From 3df9985aff1c3b7044a45f35e0f57c6268c9b38f Mon Sep 17 00:00:00 2001 From: Charles Gagnon Date: Mon, 3 Oct 2022 18:21:40 -0700 Subject: [PATCH] Add loading text to query history view (#20717) * Add max entries to query history * Update query history README * Add loading text to query history view * add comment --- extensions/query-history/package.json | 6 +++ extensions/query-history/package.nls.json | 1 + extensions/query-history/src/constants.ts | 3 ++ extensions/query-history/src/main.ts | 3 +- .../query-history/src/queryHistoryProvider.ts | 44 +++++++++++++++-- .../src/test/queryHistoryProvider.test.ts | 47 +++++++++---------- 6 files changed, 75 insertions(+), 29 deletions(-) diff --git a/extensions/query-history/package.json b/extensions/query-history/package.json index a45ca427cd..9d9b5f8bd1 100644 --- a/extensions/query-history/package.json +++ b/extensions/query-history/package.json @@ -189,6 +189,12 @@ "viewsWelcome": [ { "view": "queryHistory", + "when": "queryHistory.loading", + "contents": "%queryHistory.loading%" + }, + { + "view": "queryHistory", + "when": "queryHistory.noEntries", "contents": "%queryHistory.noEntries%" } ], diff --git a/extensions/query-history/package.nls.json b/extensions/query-history/package.nls.json index 3e46da8fbe..ee4cbd7b21 100644 --- a/extensions/query-history/package.nls.json +++ b/extensions/query-history/package.nls.json @@ -13,6 +13,7 @@ "queryHistory.disableCapture": "Pause Query History Capture", "queryHistory.enableCapture": "Start Query History Capture", "queryHistory.noEntries": "No queries to display", + "queryHistory.loading": "Loading saved entries...", "queryHistory.maxEntries": "Maximum number of entries to store. 0 means unlimited entries are stored. Increasing this limit may impact performance, especially if persistence is enabled.", "queryHistory.openStorageFolder": "Open storage folder" } diff --git a/extensions/query-history/src/constants.ts b/extensions/query-history/src/constants.ts index aba6e3a7f5..a9a72e4622 100644 --- a/extensions/query-history/src/constants.ts +++ b/extensions/query-history/src/constants.ts @@ -11,3 +11,6 @@ export const MAX_ENTRIES_CONFIG_SECTION = 'maxEntries'; export const ITEM_SELECTED_COMMAND_ID = 'queryHistory.itemSelected'; +export const CONTEXT_LOADING = 'queryHistory.loading'; +export const CONTEXT_NOENTRIES = 'queryHistory.noEntries'; + diff --git a/extensions/query-history/src/main.ts b/extensions/query-history/src/main.ts index 40a03abe6a..fe5055271f 100644 --- a/extensions/query-history/src/main.ts +++ b/extensions/query-history/src/main.ts @@ -7,7 +7,7 @@ import * as azdata from 'azdata'; import * as vscode from 'vscode'; import { DOUBLE_CLICK_ACTION_CONFIG_SECTION, ITEM_SELECTED_COMMAND_ID, QUERY_HISTORY_CONFIG_SECTION } from './constants'; import { QueryHistoryItem } from './queryHistoryItem'; -import { QueryHistoryProvider } from './queryHistoryProvider'; +import { QueryHistoryProvider, setLoadingContext } from './queryHistoryProvider'; import { promises as fs } from 'fs'; let lastSelectedItem: { item: QueryHistoryItem | undefined, time: number | undefined } = { @@ -29,6 +29,7 @@ export async function activate(context: vscode.ExtensionContext): Promise console.error(`Error creating query history global storage folder ${context.globalStorageUri.fsPath}. ${err}`); } } + await setLoadingContext(true); const treeDataProvider = new QueryHistoryProvider(context, storageUri); context.subscriptions.push(treeDataProvider); const treeView = vscode.window.createTreeView('queryHistory', { diff --git a/extensions/query-history/src/queryHistoryProvider.ts b/extensions/query-history/src/queryHistoryProvider.ts index 285cd1d1de..f19054792d 100644 --- a/extensions/query-history/src/queryHistoryProvider.ts +++ b/extensions/query-history/src/queryHistoryProvider.ts @@ -7,7 +7,7 @@ import * as vscode from 'vscode'; import * as azdata from 'azdata'; import { QueryHistoryItem } from './queryHistoryItem'; import { debounce, removeNewLines } from './utils'; -import { CAPTURE_ENABLED_CONFIG_SECTION, ITEM_SELECTED_COMMAND_ID, MAX_ENTRIES_CONFIG_SECTION, PERSIST_HISTORY_CONFIG_SECTION, QUERY_HISTORY_CONFIG_SECTION } from './constants'; +import { CAPTURE_ENABLED_CONFIG_SECTION, CONTEXT_LOADING, CONTEXT_NOENTRIES, ITEM_SELECTED_COMMAND_ID, MAX_ENTRIES_CONFIG_SECTION, PERSIST_HISTORY_CONFIG_SECTION, QUERY_HISTORY_CONFIG_SECTION } from './constants'; import * as fs from 'fs'; import * as path from 'path'; import * as crypto from 'crypto'; @@ -35,6 +35,8 @@ export class QueryHistoryProvider implements vscode.TreeDataProvider | undefined = undefined; + private _disposables: vscode.Disposable[] = []; private writeHistoryFileWorker: (() => void) | undefined; @@ -48,7 +50,7 @@ export class QueryHistoryProvider implements vscode.TreeDataProvider { if (e.affectsConfiguration(QUERY_HISTORY_CONFIG_SECTION) || e.affectsConfiguration(MAX_ENTRIES_CONFIG_SECTION)) { await this.updateConfigurationValues(); @@ -174,8 +176,12 @@ export class QueryHistoryProvider implements vscode.TreeDataProvider { this._queryHistoryItems = []; this.writeHistoryFile(); + await this.updateNoEntriesContext(); this._onDidChangeTreeData.fire(undefined); } public async deleteItem(item: QueryHistoryItem): Promise { this._queryHistoryItems = this._queryHistoryItems.filter(n => n !== item); this.writeHistoryFile(); + await this.updateNoEntriesContext(); this._onDidChangeTreeData.fire(undefined); } @@ -209,7 +217,8 @@ export class QueryHistoryProvider implements vscode.TreeDataProvider { + await this._initPromise; // We only have top level items return this._queryHistoryItems; } @@ -227,6 +236,10 @@ export class QueryHistoryProvider implements vscode.TreeDataProvider { + // Only show the "No Entries" text if there's no loaded entries - otherwise it will show the text until + // the tree view actually displays the items. + // Note that we only have to call this when deleting items, not adding, since that's the only time outside + // the initial load that we may end up with 0 items in the list. + return vscode.commands.executeCommand('setContext', CONTEXT_NOENTRIES, this._queryHistoryItems.length === 0); + } +} + +/** + * Sets the 'queryHistory.loaded' context, which is used to display the loading message in the tree view + * while entries are being loaded from disk. + * @param loading The loaded state to set + * @returns A promise that completes when the setContext command has been executed + */ +export async function setLoadingContext(loading: boolean): Promise { + return vscode.commands.executeCommand('setContext', CONTEXT_LOADING, loading); } diff --git a/extensions/query-history/src/test/queryHistoryProvider.test.ts b/extensions/query-history/src/test/queryHistoryProvider.test.ts index 60eb139d3f..315d8a26b1 100644 --- a/extensions/query-history/src/test/queryHistoryProvider.test.ts +++ b/extensions/query-history/src/test/queryHistoryProvider.test.ts @@ -28,9 +28,8 @@ describe('QueryHistoryProvider', () => { textDocumentSandbox.replaceGetter(vscode.workspace, 'textDocuments', () => [azdataTest.mocks.vscode.createTextDocumentMock(testUri).object]); const getConnectionStub = sinon.stub(azdata.connection, 'getConnection'); getConnectionStub.resolves({}); - // const getConfigurationStub = sinon.stub(vscode.workspace, 'getConfiguration') const contextMock = azdataTest.mocks.vscode.createExtensionContextMock(); - testProvider = new QueryHistoryProvider(contextMock.object); + testProvider = new QueryHistoryProvider(contextMock.object, contextMock.object.globalStorageUri); // Disable persistence during tests await testProvider.setPersistenceEnabled(false); }); @@ -39,14 +38,14 @@ describe('QueryHistoryProvider', () => { sinon.restore(); }); - it('There should be no children initially', function () { - const children = testProvider.getChildren(); + it('There should be no children initially', async function () { + const children = await testProvider.getChildren(); should(children).length(0); }); it('Clearing empty list does not throw', async function () { await testProvider.clearAll(); - const children = testProvider.getChildren(); + const children = await testProvider.getChildren(); should(children).length(0); }); @@ -54,7 +53,7 @@ describe('QueryHistoryProvider', () => { const types: azdata.queryeditor.QueryEventType[] = ['executionPlan', 'queryStart', 'queryUpdate', 'visualize']; for (const type of types) { await fireQueryEventAndWaitForRefresh(type, { uri: testUri.toString() }, { messages: [], batchRanges: [] }, 2000); - const children = testProvider.getChildren(); + const children = await testProvider.getChildren(); should(children).length(0, `Should have no children after ${type} event`); } }); @@ -62,7 +61,7 @@ describe('QueryHistoryProvider', () => { it('queryStop events cause children to be added', async function () { setupTextEditorMock('SELECT 1'); await fireQueryStartAndStopAndWaitForRefresh(testUri); - const children = testProvider.getChildren(); + const children = await testProvider.getChildren(); should(children).length(1, 'Should have one child after adding item'); await fireQueryStartAndStopAndWaitForRefresh(testUri); @@ -73,7 +72,7 @@ describe('QueryHistoryProvider', () => { const content = 'SELECT 1\nSELECT 2'; setupTextEditorMock(content); await fireQueryStartAndStopAndWaitForRefresh(testUri); - const children = testProvider.getChildren(); + const children = await testProvider.getChildren(); should(children).length(1, 'Should have one child after adding item'); should(children[0].queryText).be.equal(content, 'item content should be full text content'); }); @@ -83,7 +82,7 @@ describe('QueryHistoryProvider', () => { 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(); + const children = await 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'); }); @@ -94,7 +93,7 @@ describe('QueryHistoryProvider', () => { const message2: azdata.queryeditor.QueryMessage = { message: 'Error message', isError: true }; const message3: azdata.queryeditor.QueryMessage = { message: 'Message 2', isError: false }; await fireQueryStartAndStopAndWaitForRefresh(testUri, { messages: [message1, message2, message3], batchRanges: [] }); - const children = testProvider.getChildren(); + const children = await 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'); }); @@ -105,7 +104,7 @@ describe('QueryHistoryProvider', () => { const message2: azdata.queryeditor.QueryMessage = { message: 'Message 2', isError: false }; const message3: azdata.queryeditor.QueryMessage = { message: 'Message 3', isError: false }; await fireQueryStartAndStopAndWaitForRefresh(testUri, { messages: [message1, message2, message3], batchRanges: [] }); - const children = testProvider.getChildren(); + const children = await 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'); }); @@ -115,17 +114,17 @@ describe('QueryHistoryProvider', () => { 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', queryDocumentMock.object, { messages: [], batchRanges: [] }, 2000); - const children = testProvider.getChildren(); + const children = await testProvider.getChildren(); should(children).length(0, 'Should not have any children'); }); it('can clear all with one child', async function () { await fireQueryStartAndStopAndWaitForRefresh(testUri); - let children = testProvider.getChildren(); + let children = await testProvider.getChildren(); should(children).length(1, 'Should have one child after adding item'); await waitForItemRefresh(() => testProvider.clearAll()); - children = testProvider.getChildren(); + children = await testProvider.getChildren(); should(children).length(0, 'Should have no children after clearing'); }); @@ -133,29 +132,29 @@ describe('QueryHistoryProvider', () => { await fireQueryStartAndStopAndWaitForRefresh(testUri); await fireQueryStartAndStopAndWaitForRefresh(testUri); await fireQueryStartAndStopAndWaitForRefresh(testUri); - let children = testProvider.getChildren(); + let children = await testProvider.getChildren(); should(children).length(3, 'Should have 3 children after adding item'); await waitForItemRefresh(() => testProvider.clearAll()); - children = testProvider.getChildren(); + children = await testProvider.getChildren(); should(children).length(0, 'Should have no children after clearing'); }); it('delete item when no items doesn\'t throw', async function () { const testItem: QueryHistoryItem = { queryText: 'SELECT 1', connectionProfile: azdataTest.stubs.azdata.createConnectionProfile(), timestamp: new Date().toLocaleString(), isSuccess: true }; await waitForItemRefresh(() => testProvider.deleteItem(testItem)); - const children = testProvider.getChildren(); + const children = await testProvider.getChildren(); should(children).length(0, 'Should have no children after deleting item'); }); it('delete item that doesn\'t exist doesn\'t throw', async function () { await fireQueryStartAndStopAndWaitForRefresh(testUri); - let children = testProvider.getChildren(); + let children = await testProvider.getChildren(); should(children).length(1, 'Should have 1 child initially'); const testItem: QueryHistoryItem = { queryText: 'SELECT 1', connectionProfile: azdataTest.stubs.azdata.createConnectionProfile(), timestamp: new Date().toLocaleString(), isSuccess: true }; await waitForItemRefresh(() => testProvider.deleteItem(testItem)); - children = testProvider.getChildren(); + children = await testProvider.getChildren(); should(children).length(1, 'Should still have 1 child after deleting item'); }); @@ -163,31 +162,31 @@ describe('QueryHistoryProvider', () => { await fireQueryStartAndStopAndWaitForRefresh(testUri); await fireQueryStartAndStopAndWaitForRefresh(testUri); await fireQueryStartAndStopAndWaitForRefresh(testUri); - const firstChildren = testProvider.getChildren(); + const firstChildren = await testProvider.getChildren(); should(firstChildren).length(3, 'Should have 3 children initially'); let itemToDelete: QueryHistoryItem = firstChildren[1]; await waitForItemRefresh(() => testProvider.deleteItem(itemToDelete)); - const secondChildren = testProvider.getChildren(); + const secondChildren = await testProvider.getChildren(); should(secondChildren).length(2, 'Should still have 2 child after deleting item'); should(secondChildren[0]).be.equal(firstChildren[0], 'First item should still exist after deleting first item'); should(secondChildren[1]).be.equal(firstChildren[2], 'Second item should still exist after deleting first item'); itemToDelete = secondChildren[0]; await waitForItemRefresh(() => testProvider.deleteItem(itemToDelete)); - const thirdChildren = testProvider.getChildren(); + const thirdChildren = await testProvider.getChildren(); should(thirdChildren).length(1, 'Should still have 1 child after deleting item'); should(thirdChildren[0]).be.equal(secondChildren[1], 'Second item should still exist after deleting second item'); itemToDelete = thirdChildren[0]; await waitForItemRefresh(() => testProvider.deleteItem(itemToDelete)); - const fourthChildren = testProvider.getChildren(); + const fourthChildren = await testProvider.getChildren(); should(fourthChildren).length(0, 'Should have no children after deleting all items'); }); it('pausing capture causes children not to be added', async function () { await fireQueryStartAndStopAndWaitForRefresh(testUri); - const children = testProvider.getChildren(); + const children = await testProvider.getChildren(); should(children).length(1, 'Should have one child after adding initial item'); await testProvider.setCaptureEnabled(false);