Skip to content

Commit d82fe4e

Browse files
authored
refactor(cloudwatch): simplify saveCurrentLogDataContent() #3793
Problem: saveCurrentLogDataContent() re-renders the cloudwatch logs document from the "registry", even though this command only makes sense on an existing, open editor document. Also even in the case where a document isn't open, it should just be opened instead of using the low-level registry mechanics to get the file contents. Solution: - Use `vscode.window.activeTextEditor?.document.getText()` to get the file contents. - Make the test more meanginful by opening an editor document instead of passing a URI.
1 parent d646f2a commit d82fe4e

File tree

5 files changed

+53
-40
lines changed

5 files changed

+53
-40
lines changed

src/cloudWatchLogs/activation.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
import * as vscode from 'vscode'
77
import { CLOUDWATCH_LOGS_SCHEME } from '../shared/constants'
88
import { Settings } from '../shared/settings'
9-
import { CloudWatchLogsSettings } from './cloudWatchLogsUtils'
109
import { addLogEvents } from './commands/addLogEvents'
1110
import { copyLogResource } from './commands/copyLogResource'
1211
import { saveCurrentLogDataContent } from './commands/saveCurrentLogDataContent'
@@ -22,8 +21,7 @@ import { CloudWatchLogsNode } from './explorer/cloudWatchLogsNode'
2221
import { loadAndOpenInitialLogStreamFile, LogStreamCodeLensProvider } from './document/logStreamsCodeLensProvider'
2322

2423
export async function activate(context: vscode.ExtensionContext, configuration: Settings): Promise<void> {
25-
const settings = new CloudWatchLogsSettings(configuration)
26-
const registry = new LogDataRegistry(settings)
24+
const registry = LogDataRegistry.instance
2725

2826
const documentProvider = new LogDataDocumentProvider(registry)
2927

@@ -75,10 +73,7 @@ export async function activate(context: vscode.ExtensionContext, configuration:
7573
onDidChangeCodeLensEvent: vscode.EventEmitter<void>
7674
) => addLogEvents(document, registry, headOrTail, onDidChangeCodeLensEvent)
7775
),
78-
Commands.register(
79-
'aws.saveCurrentLogDataContent',
80-
async (uri?: vscode.Uri) => await saveCurrentLogDataContent(uri, registry)
81-
),
76+
Commands.register('aws.saveCurrentLogDataContent', async () => await saveCurrentLogDataContent()),
8277
// AWS Explorer right-click action
8378
// Here instead of in ../awsexplorer/activation due to dependence on the registry.
8479
Commands.register('aws.cwl.viewLogStream', async (node: LogGroupNode) => await viewLogStream(node, registry)),

src/cloudWatchLogs/commands/saveCurrentLogDataContent.ts

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,29 +7,25 @@ import * as vscode from 'vscode'
77
import * as nls from 'vscode-nls'
88
const localize = nls.loadMessageBundle()
99

10-
import * as fs from 'fs-extra'
1110
import { SystemUtilities } from '../../shared/systemUtilities'
1211
import { isLogStreamUri, parseCloudWatchLogsUri } from '../cloudWatchLogsUtils'
13-
import { LogDataRegistry } from '../registry/logDataRegistry'
1412
import { telemetry, CloudWatchResourceType, Result } from '../../shared/telemetry/telemetry'
15-
import { generateTextFromLogEvents } from '../document/textContent'
13+
import { FileSystemCommon } from '../../srcShared/fs'
1614

17-
export async function saveCurrentLogDataContent(uri: vscode.Uri | undefined, registry: LogDataRegistry): Promise<void> {
15+
/** Prompts the user to select a file location to save the currently visible "aws-cwl:" document to. */
16+
export async function saveCurrentLogDataContent(): Promise<void> {
1817
let result: Result = 'Succeeded'
1918
let resourceType: CloudWatchResourceType = 'logStream' // Default to stream if it fails to find URI
2019

2120
try {
21+
// Get URI from active editor.
22+
const uri = vscode.window.activeTextEditor?.document.uri
2223
if (!uri) {
23-
// No URI = used command palette as entrypoint, attempt to get URI from active editor
24-
// should work correctly under any normal circumstances since the action only appears in command palette when the editor is a CloudWatch Logs editor
25-
uri = vscode.window.activeTextEditor?.document.uri
26-
if (!uri) {
27-
throw new Error()
28-
}
24+
throw new Error()
2925
}
26+
3027
resourceType = isLogStreamUri(uri) ? 'logStream' : 'logGroup'
31-
const cachedLogEvents = registry.fetchCachedLogEvents(uri)
32-
const content: string = generateTextFromLogEvents(cachedLogEvents, { timestamps: true }).text
28+
const content = vscode.window.activeTextEditor?.document.getText()
3329
const workspaceDir = vscode.workspace.workspaceFolders
3430
? vscode.workspace.workspaceFolders[0].uri
3531
: vscode.Uri.file(SystemUtilities.getHomeDirectory())
@@ -47,10 +43,9 @@ export async function saveCurrentLogDataContent(uri: vscode.Uri | undefined, reg
4743
},
4844
})
4945

50-
if (selectedUri) {
46+
if (selectedUri && content) {
5147
try {
52-
await fs.writeFile(selectedUri.fsPath, content)
53-
// TODO: Open file and close virtual doc? Is this possible?
48+
await FileSystemCommon.instance.writeFile(selectedUri, content)
5449
} catch (e) {
5550
result = 'Failed'
5651
const err = e as Error

src/cloudWatchLogs/registry/logDataRegistry.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { DefaultCloudWatchLogsClient } from '../../shared/clients/cloudWatchLogs
1010
import { waitTimeout } from '../../shared/utilities/timeoutUtils'
1111
import { Messages } from '../../shared/utilities/messages'
1212
import { pageableToCollection } from '../../shared/utilities/collectionUtils'
13+
import { Settings } from '../../shared/settings'
1314
// TODO: Add debug logging statements
1415

1516
/** Uri as a string */
@@ -20,8 +21,14 @@ export type UriString = string
2021
export class LogDataRegistry {
2122
private readonly _onDidChange: vscode.EventEmitter<vscode.Uri> = new vscode.EventEmitter<vscode.Uri>()
2223

24+
static #instance: LogDataRegistry
25+
26+
public static get instance() {
27+
return (this.#instance ??= new this())
28+
}
29+
2330
public constructor(
24-
public readonly configuration: CloudWatchLogsSettings,
31+
public readonly configuration: CloudWatchLogsSettings = new CloudWatchLogsSettings(Settings.instance),
2532
private readonly registry: Map<UriString, CloudWatchLogsData> = new Map()
2633
) {}
2734

src/test/cloudWatchLogs/commands/saveCurrentLogDataContent.test.ts

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,31 @@ import * as fs from 'fs-extra'
1010

1111
import { createURIFromArgs } from '../../../cloudWatchLogs/cloudWatchLogsUtils'
1212
import { saveCurrentLogDataContent } from '../../../cloudWatchLogs/commands/saveCurrentLogDataContent'
13-
import { CloudWatchLogsEvent, LogDataRegistry } from '../../../cloudWatchLogs/registry/logDataRegistry'
1413
import { fileExists, makeTemporaryToolkitFolder, readFileAsString } from '../../../shared/filesystemUtilities'
1514
import { getTestWindow } from '../../shared/vscode/window'
15+
import {
16+
CloudWatchLogsGroupInfo,
17+
CloudWatchLogsParameters,
18+
CloudWatchLogsResponse,
19+
LogDataRegistry,
20+
} from '../../../cloudWatchLogs/registry/logDataRegistry'
21+
import { assertTextEditorContains } from '../../testUtil'
22+
23+
async function testFilterLogEvents(
24+
logGroupInfo: CloudWatchLogsGroupInfo,
25+
apiParameters: CloudWatchLogsParameters,
26+
nextToken?: string
27+
): Promise<CloudWatchLogsResponse> {
28+
return {
29+
events: [
30+
{ message: 'The first log event', logStreamName: 'stream1', timestamp: 1675451113 },
31+
{ message: 'The second log event', logStreamName: 'stream2', timestamp: 1675451114 },
32+
],
33+
}
34+
}
1635

1736
describe('saveCurrentLogDataContent', async function () {
1837
let filename: string
19-
let fakeRegistry: LogDataRegistry
2038
let tempDir: string
2139
const expectedText = `1970-01-20T09:24:11.113+00:00\tThe first log event
2240
1970-01-20T09:24:11.114+00:00\tThe second log event
@@ -25,15 +43,6 @@ describe('saveCurrentLogDataContent', async function () {
2543
beforeEach(async function () {
2644
tempDir = await makeTemporaryToolkitFolder()
2745
filename = path.join(tempDir, 'bobLoblawsLawB.log')
28-
fakeRegistry = {
29-
fetchCachedLogEvents: (uri: vscode.Uri) => {
30-
const events: CloudWatchLogsEvent[] = [
31-
{ message: 'The first log event', logStreamName: 'stream1', timestamp: 1675451113 },
32-
{ message: 'The second log event', logStreamName: 'stream2', timestamp: 1675451114 },
33-
]
34-
return events
35-
},
36-
} as any as LogDataRegistry
3746
})
3847

3948
afterEach(async function () {
@@ -47,19 +56,22 @@ describe('saveCurrentLogDataContent', async function () {
4756
streamName: 's',
4857
}
4958
const uri = createURIFromArgs(logGroupInfo, {})
59+
LogDataRegistry.instance.registerInitialLog(uri, testFilterLogEvents)
60+
await LogDataRegistry.instance.fetchNextLogEvents(uri)
61+
vscode.window.showTextDocument(uri)
62+
await assertTextEditorContains(expectedText, false) // Wait for document provider.
5063

5164
getTestWindow().onDidShowDialog(d => d.selectItem(vscode.Uri.file(filename)))
52-
await saveCurrentLogDataContent(uri, fakeRegistry)
65+
await saveCurrentLogDataContent()
5366

5467
assert.ok(await fileExists(filename))
5568
assert.strictEqual(await readFileAsString(filename), expectedText)
5669
})
5770

5871
it('does not do anything if the URI is invalid', async function () {
5972
getTestWindow().onDidShowDialog(d => d.selectItem(vscode.Uri.file(filename)))
60-
await saveCurrentLogDataContent(vscode.Uri.parse(`notCloudWatch:hahahaha`), fakeRegistry)
73+
vscode.window.showTextDocument(vscode.Uri.parse(`notCloudWatch:hahahaha`))
74+
await saveCurrentLogDataContent()
6175
assert.strictEqual(await fileExists(filename), false)
6276
})
63-
64-
// TODO: Add test for fs.writeFile failure. Apparently `fs.chmod` doesn't work on Windows?
6577
})

src/test/testUtil.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ export const assertTelemetryCurried =
226226
* the document must match exactly to the text editor at some point, otherwise this
227227
* function will timeout.
228228
*/
229-
export async function assertTextEditorContains(contents: string): Promise<void | never> {
229+
export async function assertTextEditorContains(contents: string, exact: boolean = true): Promise<void | never> {
230230
const editor = await waitUntil(
231231
async () => {
232232
if (vscode.window.activeTextEditor?.document.getText() === contents) {
@@ -244,7 +244,11 @@ export async function assertTextEditorContains(contents: string): Promise<void |
244244
const actual = vscode.window.activeTextEditor.document
245245
const documentName = actual.uri.toString(true)
246246
const message = `Document "${documentName}" contained "${actual.getText()}", expected: "${contents}"`
247-
assert.strictEqual(actual.getText(), contents, message)
247+
if (exact) {
248+
assert.strictEqual(actual.getText(), contents, message)
249+
} else {
250+
assert(actual.getText().includes(contents), message)
251+
}
248252
}
249253
}
250254

0 commit comments

Comments
 (0)