diff --git a/extensions/query-history/package.json b/extensions/query-history/package.json index 9f6bcfb0fc..3d82305b64 100644 --- a/extensions/query-history/package.json +++ b/extensions/query-history/package.json @@ -190,7 +190,7 @@ }, "dependencies": {}, "devDependencies": { - "@microsoft/azdata-test": "^1.5.2", + "@microsoft/azdata-test": "^2.0.0", "@microsoft/vscodetestcover": "^1.2.1", "@types/mocha": "^7.0.2", "@types/node": "^12.11.7", diff --git a/extensions/query-history/src/queryHistoryProvider.ts b/extensions/query-history/src/queryHistoryProvider.ts index 7cd254a1a1..c822f1f729 100644 --- a/extensions/query-history/src/queryHistoryProvider.ts +++ b/extensions/query-history/src/queryHistoryProvider.ts @@ -5,7 +5,6 @@ import * as vscode from 'vscode'; import * as azdata from 'azdata'; -import { EOL } from 'os'; import { QueryHistoryItem } from './queryHistoryItem'; import { removeNewLines } from './utils'; import { CAPTURE_ENABLED_CONFIG_SECTION, ITEM_SELECTED_COMMAND_ID, QUERY_HISTORY_CONFIG_SECTION } from './constants'; @@ -24,26 +23,44 @@ export class QueryHistoryProvider implements vscode.TreeDataProvider = new Map(); + constructor() { this._disposables.push(azdata.queryeditor.registerQueryEventListener({ onQueryEvent: async (type: azdata.queryeditor.QueryEventType, document: azdata.queryeditor.QueryDocument, args: azdata.ResultSetSummary | string | undefined, queryInfo?: azdata.queryeditor.QueryInfo) => { - if (this._captureEnabled && queryInfo && type === 'queryStop') { - const textDocuments = vscode.workspace.textDocuments; - // We need to compare URIs, but the event Uri comes in as string so while it should be in the same format as - // the textDocument uri.toString() we parse it into a vscode.Uri first to be absolutely sure. - const textDocument = textDocuments.find(e => e.uri.toString() === vscode.Uri.parse(document.uri).toString()); - if (!textDocument) { - // If we couldn't find the document then we can't get the text so just log the error and move on - console.error(`Couldn't find text document with URI ${document.uri} for query event`); - return; + if (this._captureEnabled && queryInfo) { + if (type === 'queryStop') { + const connectionProfile = await azdata.connection.getConnection(document.uri); + const isSuccess = queryInfo.messages.find(m => m.isError) ? false : true; + // Add to the front of the list so the new item appears at the top + const queryText = this.queryTextMappings.get(document.uri); + if (queryText === undefined) { + console.error(`Couldn't find query text for URI ${document.uri.toString()}`); + return; + } + this.queryTextMappings.delete(document.uri); + this._queryHistoryItems.unshift({ queryText, connectionProfile, timestamp: new Date(), isSuccess }); + this._onDidChangeTreeData.fire(undefined); + } else if (type === 'queryStart') { + // We get the text and save it on queryStart because we want to get the query text immediately when + // the query is started but then only add the item when it finishes (so that we can properly determine the success of the execution). + // This avoids a race condition with the text being modified during execution and ending up with the query text at the end being + // different than when it started. + const textEditor = vscode.window.activeTextEditor; + // We need to compare URIs, but the event Uri comes in as string so while it should be in the same format as + // the textDocument uri.toString() we parse it into a vscode.Uri first to be absolutely sure. + if (textEditor?.document.uri.toString() !== vscode.Uri.parse(document.uri).toString()) { + // If we couldn't find the document then we can't get the text so just log the error and move on + console.error(`Active text editor ${textEditor?.document.uri} does not match URI ${document.uri} for query event`); + return; + } + // Get the text from the current selection - or the entire document if there isn't a selection (mimicking what STS is doing itself) + const queryText = textEditor.document.getText(textEditor.selection.isEmpty ? undefined : textEditor.selection) ?? ''; + this.queryTextMappings.set(document.uri, queryText); } - // Combine all the text from the batches back together - const queryText = queryInfo.batchRanges.map(r => textDocument.getText(r) ?? '').join(EOL); - const connectionProfile = await azdata.connection.getConnection(document.uri); - const isSuccess = queryInfo.messages.find(m => m.isError) ? false : true; - // Add to the front of the list so the new item appears at the top - this._queryHistoryItems.unshift({ queryText, connectionProfile, timestamp: new Date(), isSuccess }); - this._onDidChangeTreeData.fire(undefined); } } })); diff --git a/extensions/query-history/src/test/queryHistoryProvider.test.ts b/extensions/query-history/src/test/queryHistoryProvider.test.ts index 9465823078..0239425deb 100644 --- a/extensions/query-history/src/test/queryHistoryProvider.test.ts +++ b/extensions/query-history/src/test/queryHistoryProvider.test.ts @@ -11,7 +11,6 @@ import * as sinon from 'sinon'; import * as azdataTest from '@microsoft/azdata-test'; import { QueryHistoryProvider } from '../queryHistoryProvider'; import { QueryHistoryItem } from '../queryHistoryItem'; -import { EOL } from 'os'; describe('QueryHistoryProvider', () => { @@ -57,44 +56,51 @@ describe('QueryHistoryProvider', () => { }); it('queryStop events cause children to be added', async function () { - await fireQueryEventAndWaitForRefresh('queryStop', { uri: testUri.toString() }, { messages: [], batchRanges: [] }); + setupTextEditorMock('SELECT 1'); + await fireQueryStartAndStopAndWaitForRefresh(testUri); const children = testProvider.getChildren(); should(children).length(1, 'Should have one child after adding item'); - await fireQueryEventAndWaitForRefresh('queryStop', { uri: testUri.toString() }, { messages: [], batchRanges: [] }); + await fireQueryStartAndStopAndWaitForRefresh(testUri); should(children).length(2, 'Should have two children after adding another item'); }); - it('multiple ranges are combined', async function () { - const rangeWithContent1: azdataTest.mocks.vscode.RangeWithContent = { range: new vscode.Range(new vscode.Position(0, 0), new vscode.Position(2, 0)), content: 'SELECT 1' }; - const rangeWithContent2: azdataTest.mocks.vscode.RangeWithContent = { range: new vscode.Range(new vscode.Position(3, 0), new vscode.Position(3, 5)), content: 'SELECT 2' }; - const textDocumentMock = azdataTest.mocks.vscode.createTextDocumentMock(testUri, [rangeWithContent1, rangeWithContent2]); - textDocumentSandbox.restore(); - textDocumentSandbox.replaceGetter(vscode.workspace, 'textDocuments', () => [textDocumentMock.object]); - await fireQueryEventAndWaitForRefresh('queryStop', { uri: testUri.toString() }, { - messages: [], - batchRanges: [rangeWithContent1.range, rangeWithContent2.range] - }); + it('no selection records entire text', async function () { + const content = 'SELECT 1\nSELECT 2'; + setupTextEditorMock(content); + await fireQueryStartAndStopAndWaitForRefresh(testUri); const children = testProvider.getChildren(); should(children).length(1, 'Should have one child after adding item'); - should(children[0].queryText).be.equal(`${rangeWithContent1.content}${EOL}${rangeWithContent2.content}`, 'item content should be combined from both source ranges'); + should(children[0].queryText).be.equal(content, 'item content should be full text content'); + }); + + it('active selection records only selected text', async function () { + const rangeWithContent1: azdataTest.mocks.vscode.RangeWithContent = { range: new vscode.Range(new vscode.Position(0, 0), new vscode.Position(2, 0)), content: 'SELECT 1' }; + const rangeWithContent2: azdataTest.mocks.vscode.RangeWithContent = { range: new vscode.Range(new vscode.Position(3, 0), new vscode.Position(3, 5)), content: 'SELECT 2' }; + setupTextEditorMock([rangeWithContent1, rangeWithContent2], [new vscode.Selection(rangeWithContent1.range.start, rangeWithContent1.range.end)]); + await fireQueryStartAndStopAndWaitForRefresh(testUri); + const children = testProvider.getChildren(); + should(children).length(1, 'Should have one child after adding item'); + should(children[0].queryText).be.equal(rangeWithContent1.content, 'item content should be only active selection'); }); it('event with errors is marked as error', async function () { + setupTextEditorMock('SELECT 1'); const message1: azdata.queryeditor.QueryMessage = { message: 'Message 1', isError: false }; const message2: azdata.queryeditor.QueryMessage = { message: 'Error message', isError: true }; const message3: azdata.queryeditor.QueryMessage = { message: 'Message 2', isError: false }; - await fireQueryEventAndWaitForRefresh('queryStop', { uri: testUri.toString() }, { messages: [ message1, message2, message3 ], batchRanges: []}); + await fireQueryStartAndStopAndWaitForRefresh(testUri, { messages: [message1, message2, message3], batchRanges: [] }); const children = testProvider.getChildren(); should(children).length(1, 'Should have one child after adding item'); should(children[0].isSuccess).be.false('Event with errors should have error icon'); }); it('event without errors is marked as success', async function () { + setupTextEditorMock('SELECT 1'); const message1: azdata.queryeditor.QueryMessage = { message: 'Message 1', isError: false }; const message2: azdata.queryeditor.QueryMessage = { message: 'Message 2', isError: false }; const message3: azdata.queryeditor.QueryMessage = { message: 'Message 3', isError: false }; - await fireQueryEventAndWaitForRefresh('queryStop', { uri: testUri.toString() }, { messages: [ message1, message2, message3 ], batchRanges: []}); + await fireQueryStartAndStopAndWaitForRefresh(testUri, { messages: [message1, message2, message3], batchRanges: [] }); const children = testProvider.getChildren(); should(children).length(1, 'Should have one child after adding item'); should(children[0].isSuccess).be.true('Event without errors should have check icon'); @@ -102,14 +108,15 @@ describe('QueryHistoryProvider', () => { it('queryStop events from unknown document are ignored', async function () { const unknownUri = vscode.Uri.parse('untitled://query2'); + const queryDocumentMock = azdataTest.mocks.azdata.queryeditor.createQueryDocumentMock(unknownUri.toString()); // Since we didn't find the text document we'll never update the item list so add a timeout since that event will never fire - await fireQueryEventAndWaitForRefresh('queryStop', { uri: unknownUri.toString() }, { messages: [], batchRanges: [] }, 2000); + await fireQueryEventAndWaitForRefresh('queryStop', queryDocumentMock.object, { messages: [], batchRanges: [] }, 2000); const children = testProvider.getChildren(); should(children).length(0, 'Should not have any children'); }); it('can clear all with one child', async function () { - await fireQueryEventAndWaitForRefresh('queryStop', { uri: testUri.toString() }, { messages: [], batchRanges: [] }); + await fireQueryStartAndStopAndWaitForRefresh(testUri); let children = testProvider.getChildren(); should(children).length(1, 'Should have one child after adding item'); @@ -119,9 +126,9 @@ describe('QueryHistoryProvider', () => { }); it('can clear all with multiple children', async function () { - await fireQueryEventAndWaitForRefresh('queryStop', { uri: testUri.toString() }, { messages: [], batchRanges: [] }); - await fireQueryEventAndWaitForRefresh('queryStop', { uri: testUri.toString() }, { messages: [], batchRanges: [] }); - await fireQueryEventAndWaitForRefresh('queryStop', { uri: testUri.toString() }, { messages: [], batchRanges: [] }); + await fireQueryStartAndStopAndWaitForRefresh(testUri); + await fireQueryStartAndStopAndWaitForRefresh(testUri); + await fireQueryStartAndStopAndWaitForRefresh(testUri); let children = testProvider.getChildren(); should(children).length(3, 'Should have 3 children after adding item'); @@ -138,7 +145,7 @@ describe('QueryHistoryProvider', () => { }); it('delete item that doesn\'t exist doesn\'t throw', async function () { - await fireQueryEventAndWaitForRefresh('queryStop', { uri: testUri.toString() }, { messages: [], batchRanges: [] }); + await fireQueryStartAndStopAndWaitForRefresh(testUri); let children = testProvider.getChildren(); should(children).length(1, 'Should have 1 child initially'); @@ -149,9 +156,9 @@ describe('QueryHistoryProvider', () => { }); it('can delete single item', async function () { - await fireQueryEventAndWaitForRefresh('queryStop', { uri: testUri.toString() }, { messages: [], batchRanges: [] }); - await fireQueryEventAndWaitForRefresh('queryStop', { uri: testUri.toString() }, { messages: [], batchRanges: [] }); - await fireQueryEventAndWaitForRefresh('queryStop', { uri: testUri.toString() }, { messages: [], batchRanges: [] }); + await fireQueryStartAndStopAndWaitForRefresh(testUri); + await fireQueryStartAndStopAndWaitForRefresh(testUri); + await fireQueryStartAndStopAndWaitForRefresh(testUri); const firstChildren = testProvider.getChildren(); should(firstChildren).length(3, 'Should have 3 children initially'); @@ -175,22 +182,36 @@ describe('QueryHistoryProvider', () => { }); it('pausing capture causes children not to be added', async function () { - await fireQueryEventAndWaitForRefresh('queryStop', { uri: testUri.toString() }, { messages: [], batchRanges: [] }); + await fireQueryStartAndStopAndWaitForRefresh(testUri); const children = testProvider.getChildren(); should(children).length(1, 'Should have one child after adding initial item'); await testProvider.setCaptureEnabled(false); // Add timeout since the item is never added, thus never triggering the event - await fireQueryEventAndWaitForRefresh('queryStop', { uri: testUri.toString() }, { messages: [], batchRanges: [] }, 2000); + await fireQueryStartAndStopAndWaitForRefresh(testUri, { messages: [], batchRanges: [] }, 2000); should(children).length(1, 'Should still have 1 child after adding item when capture paused'); await testProvider.setCaptureEnabled(true); - await fireQueryEventAndWaitForRefresh('queryStop', { uri: testUri.toString() }, { messages: [], batchRanges: [] }); + await fireQueryStartAndStopAndWaitForRefresh(testUri); should(children).length(2, 'Should have 2 child after adding item when capture was resumed'); }); + function setupTextEditorMock(content: string | azdataTest.mocks.vscode.RangeWithContent[], selections?: vscode.Selection[] | undefined): void { + const textDocumentMock = azdataTest.mocks.vscode.createTextDocumentMock(testUri, content); + const textEditorMock = azdataTest.mocks.vscode.createTextEditorMock(textDocumentMock.object, selections); + textDocumentSandbox.replaceGetter(vscode.window, 'activeTextEditor', () => textEditorMock.object); + } + + async function fireQueryStartAndStopAndWaitForRefresh(uri: vscode.Uri, queryInfo: azdata.queryeditor.QueryInfo = { messages: [], batchRanges: [] }, timeoutMs?: number): Promise { + const queryDocumentMock = azdataTest.mocks.azdata.queryeditor.createQueryDocumentMock(uri.toString()); + // First queryStart message to record text. QueryInfo is always empty for this. + testListener.onQueryEvent('queryStart', queryDocumentMock.object, undefined, { messages: [], batchRanges: [] }); + // Fire queryStop message to trigger creation of the history node + await fireQueryEventAndWaitForRefresh('queryStop', queryDocumentMock.object, queryInfo, timeoutMs); + } + async function fireQueryEventAndWaitForRefresh(type: azdata.queryeditor.QueryEventType, document: azdata.queryeditor.QueryDocument, queryInfo: azdata.queryeditor.QueryInfo, timeoutMs?: number): Promise { await waitForItemRefresh(() => testListener.onQueryEvent(type, document, undefined, queryInfo), timeoutMs); } diff --git a/extensions/query-history/yarn.lock b/extensions/query-history/yarn.lock index 2e8d42cf9f..82fa68d950 100644 --- a/extensions/query-history/yarn.lock +++ b/extensions/query-history/yarn.lock @@ -228,10 +228,10 @@ "@jridgewell/resolve-uri" "^3.0.3" "@jridgewell/sourcemap-codec" "^1.4.10" -"@microsoft/azdata-test@^1.5.2": - version "1.5.2" - resolved "https://registry.yarnpkg.com/@microsoft/azdata-test/-/azdata-test-1.5.2.tgz#0a750eed6c686eaf98e63db580f119916a083d6a" - integrity sha512-m1WVNxkCqU8nkaRB0Q/DEq1wHej0Ev3bfyEmsPgX9znymY7MD9c1l6kRoHtPPmsYYDEjcCxO2QXUJAsNUeAKwg== +"@microsoft/azdata-test@^2.0.0": + version "2.0.0" + resolved "https://registry.yarnpkg.com/@microsoft/azdata-test/-/azdata-test-2.0.0.tgz#5b7d4ba0114ebd5ccae7d88e6982c847b1e76b06" + integrity sha512-vXfmBoc6hlbrdIqXLfbLEzw/cj5CJfpvw1mmMhO6hpSGSYqa1pm7e3dXn3fAbTjiumdSnLN1iIuOba1ggz24nA== dependencies: http-proxy-agent "^2.1.0" https-proxy-agent "^2.2.4"