From f59012862e2df09c46b3cb364e69fc6da154c94e Mon Sep 17 00:00:00 2001 From: Andrew Eisenberg Date: Thu, 24 Feb 2022 11:42:46 -0800 Subject: [PATCH 1/2] Preemptively add a version number to the query history json file Since we are now storing query history on disk, we will need to handle situations where versions change. For now, there is only version 1. In the future, we may need to make breaking changes to this format and we need the flexibility to detect and possibly handle different versions. In this case, users don't often downgrade their vscode versions, so most likely, we only need to be forward compatible. Ie- we need to handle moving from v1 to v2, but not the other way around. --- .../ql-vscode/src/query-serialization.ts | 13 ++++++- .../no-workspace/query-results.test.ts | 38 +++++++++++++++---- 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/extensions/ql-vscode/src/query-serialization.ts b/extensions/ql-vscode/src/query-serialization.ts index e5508639fd2..a242380e104 100644 --- a/extensions/ql-vscode/src/query-serialization.ts +++ b/extensions/ql-vscode/src/query-serialization.ts @@ -14,7 +14,13 @@ export async function slurpQueryHistory(fsPath: string, config: QueryHistoryConf } const data = await fs.readFile(fsPath, 'utf8'); - const queries = JSON.parse(data); + const obj = JSON.parse(data); + if (obj.version !== 1) { + void showAndLogErrorMessage(`Unsupported query history format: v${obj.version}. `); + return []; + } + + const queries = obj.queries; const parsedQueries = queries.map((q: QueryHistoryInfo) => { // Need to explicitly set prototype since reading in from JSON will not @@ -82,7 +88,10 @@ export async function splatQueryHistory(queries: QueryHistoryInfo[], fsPath: str } // remove incomplete local queries since they cannot be recreated on restart const filteredQueries = queries.filter(q => q.t === 'local' ? q.completedQuery !== undefined : true); - const data = JSON.stringify(filteredQueries, null, 2); + const data = JSON.stringify({ + version: 1, + queries: filteredQueries + }, null, 2); await fs.writeFile(fsPath, data); } catch (e) { throw new Error(`Error saving query history to ${fsPath}: ${e.message}`); diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index 05e3742d396..c64ec72d1c4 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -18,7 +18,7 @@ import { slurpQueryHistory, splatQueryHistory } from '../../query-serialization' chai.use(chaiAsPromised); const expect = chai.expect; -describe('query-results', () => { +describe.only('query-results', () => { let disposeSpy: sinon.SinonSpy; let onDidChangeQueryHistoryConfigurationSpy: sinon.SinonSpy; let mockConfig: QueryHistoryConfig; @@ -254,20 +254,30 @@ describe('query-results', () => { }); describe('splat and slurp', () => { - it('should splat and slurp query history', async () => { - const infoSuccessRaw = createMockFullQueryInfo('a', createMockQueryWithResults(`${queryPath}-a`, false, false, '/a/b/c/a', false)); - const infoSuccessInterpreted = createMockFullQueryInfo('b', createMockQueryWithResults(`${queryPath}-b`, true, true, '/a/b/c/b', false)); - const infoEarlyFailure = createMockFullQueryInfo('c', undefined, true); - const infoLateFailure = createMockFullQueryInfo('d', createMockQueryWithResults(`${queryPath}-c`, false, false, '/a/b/c/d', false)); - const infoInprogress = createMockFullQueryInfo('e'); - const allHistory = [ + + let infoSuccessRaw: LocalQueryInfo; + let infoSuccessInterpreted: LocalQueryInfo; + let infoEarlyFailure: LocalQueryInfo; + let infoLateFailure: LocalQueryInfo; + let infoInprogress: LocalQueryInfo; + let allHistory: LocalQueryInfo[]; + + beforeEach(() => { + infoSuccessRaw = createMockFullQueryInfo('a', createMockQueryWithResults(`${queryPath}-a`, false, false, '/a/b/c/a', false)); + infoSuccessInterpreted = createMockFullQueryInfo('b', createMockQueryWithResults(`${queryPath}-b`, true, true, '/a/b/c/b', false)); + infoEarlyFailure = createMockFullQueryInfo('c', undefined, true); + infoLateFailure = createMockFullQueryInfo('d', createMockQueryWithResults(`${queryPath}-c`, false, false, '/a/b/c/d', false)); + infoInprogress = createMockFullQueryInfo('e'); + allHistory = [ infoSuccessRaw, infoSuccessInterpreted, infoEarlyFailure, infoLateFailure, infoInprogress ]; + }); + it('should splat and slurp query history', async () => { // the expected results only contains the history with completed queries const expectedHistory = [ infoSuccessRaw, @@ -313,6 +323,18 @@ describe('query-results', () => { } expect(allHistoryActual.length).to.deep.eq(expectedHistory.length); }); + + it('should handle an invalid query history version', async () => { + const badPath = path.join(tmpDir.name, 'bad-query-history.json'); + fs.writeFileSync(badPath, JSON.stringify({ + version: 2, + queries: allHistory + }), 'utf8'); + + const allHistoryActual = await slurpQueryHistory(badPath, mockConfig); + // version number is invalid. Should return an empty array. + expect(allHistoryActual).to.deep.eq([]); + }); }); function createMockQueryWithResults( From f857e5ec6c74551ce5d2286c3096050c37cc3499 Mon Sep 17 00:00:00 2001 From: Andrew Eisenberg Date: Fri, 25 Feb 2022 08:00:03 -0800 Subject: [PATCH 2/2] Ensure all tests are run Co-authored-by: Charis Kyriakou --- .../src/vscode-tests/no-workspace/query-results.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts index c64ec72d1c4..0aa6843fc49 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-results.test.ts @@ -18,7 +18,7 @@ import { slurpQueryHistory, splatQueryHistory } from '../../query-serialization' chai.use(chaiAsPromised); const expect = chai.expect; -describe.only('query-results', () => { +describe('query-results', () => { let disposeSpy: sinon.SinonSpy; let onDidChangeQueryHistoryConfigurationSpy: sinon.SinonSpy; let mockConfig: QueryHistoryConfig;