diff --git a/extensions/ql-vscode/CHANGELOG.md b/extensions/ql-vscode/CHANGELOG.md index 02a9e6c2766..e2e23a07b29 100644 --- a/extensions/ql-vscode/CHANGELOG.md +++ b/extensions/ql-vscode/CHANGELOG.md @@ -8,6 +8,7 @@ - Fix a bug where invoking _View AST_ from the file explorer would not view the selected file. Instead it would view the active editor. Also, prevent the _View AST_ from appearing if the current selection includes a directory or multiple files. [#1113](https://github.com/github/vscode-codeql/pull/1113) - Add query history items as soon as a query is run, including new icons for each history item. [#1094](https://github.com/github/vscode-codeql/pull/1094) +- Save query history items across restarts. Items will be saved for 30 days and can be overwritten by setting the `codeQL.queryHistory.ttl` configuration setting. [#1130](https://github.com/github/vscode-codeql/pull/1130) - Allow in-progress query items to be cancelled from the query history view. [#1105](https://github.com/github/vscode-codeql/pull/1105) ## 1.5.10 - 25 January 2022 diff --git a/extensions/ql-vscode/package.json b/extensions/ql-vscode/package.json index 6dd368b2f18..a0855efc572 100644 --- a/extensions/ql-vscode/package.json +++ b/extensions/ql-vscode/package.json @@ -224,6 +224,12 @@ "default": "%q on %d - %s, %r result count [%t]", "markdownDescription": "Default string for how to label query history items.\n* %t is the time of the query\n* %q is the human-readable query name\n* %f is the query file name\n* %d is the database name\n* %r is the number of results\n* %s is a status string" }, + "codeQL.queryHistory.ttl": { + "type": "number", + "default": 30, + "description": "Number of days to retain queries in the query history before being automatically deleted.", + "scope": "machine" + }, "codeQL.runningTests.additionalTestArguments": { "scope": "window", "type": "array", diff --git a/extensions/ql-vscode/src/config.ts b/extensions/ql-vscode/src/config.ts index df6ff918a76..099a60898a6 100644 --- a/extensions/ql-vscode/src/config.ts +++ b/extensions/ql-vscode/src/config.ts @@ -3,6 +3,8 @@ import { workspace, Event, EventEmitter, ConfigurationChangeEvent, Configuration import { DistributionManager } from './distribution'; import { logger } from './logging'; +const ONE_DAY_IN_MS = 24 * 60 * 60 * 1000; + /** Helper class to look up a labelled (and possibly nested) setting. */ export class Setting { name: string; @@ -54,8 +56,11 @@ const DISTRIBUTION_SETTING = new Setting('cli', ROOT_SETTING); export const CUSTOM_CODEQL_PATH_SETTING = new Setting('executablePath', DISTRIBUTION_SETTING); const INCLUDE_PRERELEASE_SETTING = new Setting('includePrerelease', DISTRIBUTION_SETTING); const PERSONAL_ACCESS_TOKEN_SETTING = new Setting('personalAccessToken', DISTRIBUTION_SETTING); + +// Query History configuration const QUERY_HISTORY_SETTING = new Setting('queryHistory', ROOT_SETTING); const QUERY_HISTORY_FORMAT_SETTING = new Setting('format', QUERY_HISTORY_SETTING); +const QUERY_HISTORY_TTL = new Setting('format', QUERY_HISTORY_SETTING); /** When these settings change, the distribution should be updated. */ const DISTRIBUTION_CHANGE_SETTINGS = [CUSTOM_CODEQL_PATH_SETTING, INCLUDE_PRERELEASE_SETTING, PERSONAL_ACCESS_TOKEN_SETTING]; @@ -71,7 +76,6 @@ export interface DistributionConfig { } // Query server configuration - const RUNNING_QUERIES_SETTING = new Setting('runningQueries', ROOT_SETTING); const NUMBER_OF_THREADS_SETTING = new Setting('numberOfThreads', RUNNING_QUERIES_SETTING); const SAVE_CACHE_SETTING = new Setting('saveCache', RUNNING_QUERIES_SETTING); @@ -106,10 +110,11 @@ export interface QueryServerConfig { } /** When these settings change, the query history should be refreshed. */ -const QUERY_HISTORY_SETTINGS = [QUERY_HISTORY_FORMAT_SETTING]; +const QUERY_HISTORY_SETTINGS = [QUERY_HISTORY_FORMAT_SETTING, QUERY_HISTORY_TTL]; export interface QueryHistoryConfig { format: string; + ttlInMillis: number; onDidChangeConfiguration: Event; } @@ -251,6 +256,13 @@ export class QueryHistoryConfigListener extends ConfigListener implements QueryH public get format(): string { return QUERY_HISTORY_FORMAT_SETTING.getValue(); } + + /** + * The configuration value is in days, but return the value in milliseconds. + */ + public get ttlInMillis(): number { + return (QUERY_HISTORY_TTL.getValue() || 30) * ONE_DAY_IN_MS; + } } export class CliConfigListener extends ConfigListener implements CliConfig { diff --git a/extensions/ql-vscode/src/contextual/locationFinder.ts b/extensions/ql-vscode/src/contextual/locationFinder.ts index a331ac97feb..6c95b6406a8 100644 --- a/extensions/ql-vscode/src/contextual/locationFinder.ts +++ b/extensions/ql-vscode/src/contextual/locationFinder.ts @@ -28,6 +28,7 @@ export interface FullLocationLink extends LocationLink { * @param dbm The database manager * @param uriString The selected source file and location * @param keyType The contextual query type to run + * @param queryStorageDir The directory to store the query results * @param progress A progress callback * @param token A CancellationToken * @param filter A function that will filter extraneous results @@ -38,6 +39,7 @@ export async function getLocationsForUriString( dbm: DatabaseManager, uriString: string, keyType: KeyType, + queryStorageDir: string, progress: ProgressCallback, token: CancellationToken, filter: (src: string, dest: string) => boolean @@ -69,6 +71,7 @@ export async function getLocationsForUriString( qs, db, initialInfo, + queryStorageDir, progress, token, templates diff --git a/extensions/ql-vscode/src/contextual/templateProvider.ts b/extensions/ql-vscode/src/contextual/templateProvider.ts index f5b2a722467..65359518971 100644 --- a/extensions/ql-vscode/src/contextual/templateProvider.ts +++ b/extensions/ql-vscode/src/contextual/templateProvider.ts @@ -42,6 +42,7 @@ export class TemplateQueryDefinitionProvider implements DefinitionProvider { private cli: CodeQLCliServer, private qs: QueryServerClient, private dbm: DatabaseManager, + private queryStorageDir: string, ) { this.cache = new CachedOperation(this.getDefinitions.bind(this)); } @@ -69,6 +70,7 @@ export class TemplateQueryDefinitionProvider implements DefinitionProvider { this.dbm, uriString, KeyType.DefinitionQuery, + this.queryStorageDir, progress, token, (src, _dest) => src === uriString @@ -84,6 +86,7 @@ export class TemplateQueryReferenceProvider implements ReferenceProvider { private cli: CodeQLCliServer, private qs: QueryServerClient, private dbm: DatabaseManager, + private queryStorageDir: string, ) { this.cache = new CachedOperation(this.getReferences.bind(this)); } @@ -116,6 +119,7 @@ export class TemplateQueryReferenceProvider implements ReferenceProvider { this.dbm, uriString, KeyType.DefinitionQuery, + this.queryStorageDir, progress, token, (src, _dest) => src === uriString @@ -136,6 +140,7 @@ export class TemplatePrintAstProvider { private cli: CodeQLCliServer, private qs: QueryServerClient, private dbm: DatabaseManager, + private queryStorageDir: string, ) { this.cache = new CachedOperation(this.getAst.bind(this)); } @@ -216,6 +221,7 @@ export class TemplatePrintAstProvider { this.qs, db, initialInfo, + this.queryStorageDir, progress, token, templates diff --git a/extensions/ql-vscode/src/extension.ts b/extensions/ql-vscode/src/extension.ts index 7de1d021a38..179e6781099 100644 --- a/extensions/ql-vscode/src/extension.ts +++ b/extensions/ql-vscode/src/extension.ts @@ -19,6 +19,7 @@ import { } from 'vscode'; import { LanguageClient } from 'vscode-languageclient'; import * as os from 'os'; +import * as fs from 'fs-extra'; import * as path from 'path'; import * as tmp from 'tmp-promise'; import { testExplorerExtensionId, TestHub } from 'vscode-test-adapter-api'; @@ -435,16 +436,22 @@ async function activateWithInstalledDistribution( ctx.subscriptions.push(queryHistoryConfigurationListener); const showResults = async (item: FullCompletedQueryInfo) => showResultsForCompletedQuery(item, WebviewReveal.Forced); + const queryStorageDir = path.join(ctx.globalStorageUri.fsPath, 'queries'); + await fs.ensureDir(queryStorageDir); + void logger.log('Initializing query history.'); const qhm = new QueryHistoryManager( qs, dbm, - ctx.extensionPath, + queryStorageDir, + ctx, queryHistoryConfigurationListener, showResults, async (from: FullCompletedQueryInfo, to: FullCompletedQueryInfo) => showResultsForComparison(from, to), ); + await qhm.readQueryHistory(); + ctx.subscriptions.push(qhm); void logger.log('Initializing results panel interface.'); const intm = new InterfaceManager(ctx, dbm, cliServer, queryServerLogger); @@ -513,10 +520,12 @@ async function activateWithInstalledDistribution( qs, databaseItem, initialInfo, + queryStorageDir, progress, source.token, ); item.completeThisQuery(completedQueryInfo); + await qhm.writeQueryHistory(); await showResultsForCompletedQuery(item as FullCompletedQueryInfo, WebviewReveal.NotForced); // Note we must update the query history view after showing results as the // display and sorting might depend on the number of results @@ -988,16 +997,16 @@ async function activateWithInstalledDistribution( void logger.log('Registering jump-to-definition handlers.'); languages.registerDefinitionProvider( { scheme: archiveFilesystemProvider.zipArchiveScheme }, - new TemplateQueryDefinitionProvider(cliServer, qs, dbm) + new TemplateQueryDefinitionProvider(cliServer, qs, dbm, queryStorageDir) ); languages.registerReferenceProvider( { scheme: archiveFilesystemProvider.zipArchiveScheme }, - new TemplateQueryReferenceProvider(cliServer, qs, dbm) + new TemplateQueryReferenceProvider(cliServer, qs, dbm, queryStorageDir) ); const astViewer = new AstViewer(); - const templateProvider = new TemplatePrintAstProvider(cliServer, qs, dbm); + const templateProvider = new TemplatePrintAstProvider(cliServer, qs, dbm, queryStorageDir); ctx.subscriptions.push(astViewer); ctx.subscriptions.push(commandRunnerWithProgress('codeQL.viewAst', async ( @@ -1036,7 +1045,7 @@ async function activateWithInstalledDistribution( } function getContextStoragePath(ctx: ExtensionContext) { - return ctx.storagePath || ctx.globalStoragePath; + return ctx.storageUri?.fsPath || ctx.globalStorageUri.fsPath; } async function initializeLogging(ctx: ExtensionContext): Promise { diff --git a/extensions/ql-vscode/src/query-history-scrubber.ts b/extensions/ql-vscode/src/query-history-scrubber.ts new file mode 100644 index 00000000000..9b2da0a5f55 --- /dev/null +++ b/extensions/ql-vscode/src/query-history-scrubber.ts @@ -0,0 +1,135 @@ +import * as fs from 'fs-extra'; +import * as os from 'os'; +import * as path from 'path'; +import { Disposable, ExtensionContext } from 'vscode'; +import { logger } from './logging'; + +const LAST_SCRUB_TIME_KEY = 'lastScrubTime'; + +type Counter = { + increment: () => void; +}; + +/** + * Registers an interval timer that will periodically check for queries old enought + * to be deleted. + * + * Note that this scrubber will clean all queries from all workspaces. It should not + * run too often and it should only run from one workspace at a time. + * + * Generally, `wakeInterval` should be significantly shorter than `throttleTime`. + * + * @param wakeInterval How often to check to see if the job should run. + * @param throttleTime How often to actually run the job. + * @param maxQueryTime The maximum age of a query before is ready for deletion. + * @param queryDirectory The directory containing all queries. + * @param ctx The extension context. + */ +export function registerQueryHistoryScubber( + wakeInterval: number, + throttleTime: number, + maxQueryTime: number, + queryDirectory: string, + ctx: ExtensionContext, + + // optional counter to keep track of how many times the scrubber has run + counter?: Counter +): Disposable { + const deregister = setInterval(scrubQueries, wakeInterval, throttleTime, maxQueryTime, queryDirectory, ctx, counter); + + return { + dispose: () => { + clearInterval(deregister); + } + }; +} + +async function scrubQueries( + throttleTime: number, + maxQueryTime: number, + queryDirectory: string, + ctx: ExtensionContext, + counter?: Counter +) { + const lastScrubTime = ctx.globalState.get(LAST_SCRUB_TIME_KEY); + const now = Date.now(); + + // If we have never scrubbed before, or if the last scrub was more than `throttleTime` ago, + // then scrub again. + if (lastScrubTime === undefined || now - lastScrubTime >= throttleTime) { + await ctx.globalState.update(LAST_SCRUB_TIME_KEY, now); + + let scrubCount = 0; // total number of directories deleted + try { + counter?.increment(); + void logger.log('Scrubbing query directory. Removing old queries.'); + if (!(await fs.pathExists(queryDirectory))) { + void logger.log(`Cannot scrub. Query directory does not exist: ${queryDirectory}`); + return; + } + + const baseNames = await fs.readdir(queryDirectory); + const errors: string[] = []; + for (const baseName of baseNames) { + const dir = path.join(queryDirectory, baseName); + const scrubResult = await scrubDirectory(dir, now, maxQueryTime); + if (scrubResult.errorMsg) { + errors.push(scrubResult.errorMsg); + } + if (scrubResult.deleted) { + scrubCount++; + } + } + + if (errors.length) { + throw new Error(os.EOL + errors.join(os.EOL)); + } + } catch (e) { + void logger.log(`Error while scrubbing queries: ${e}`); + } finally { + void logger.log(`Scrubbed ${scrubCount} old queries.`); + } + } +} + +async function scrubDirectory(dir: string, now: number, maxQueryTime: number): Promise<{ + errorMsg?: string, + deleted: boolean +}> { + const timestampFile = path.join(dir, 'timestamp'); + try { + let deleted = true; + if (!(await fs.stat(dir)).isDirectory()) { + void logger.log(` ${dir} is not a directory. Deleting.`); + await fs.remove(dir); + } else if (!(await fs.pathExists(timestampFile))) { + void logger.log(` ${dir} has no timestamp file. Deleting.`); + await fs.remove(dir); + } else if (!(await fs.stat(timestampFile)).isFile()) { + void logger.log(` ${timestampFile} is not a file. Deleting.`); + await fs.remove(dir); + } else { + const timestampText = await fs.readFile(timestampFile, 'utf8'); + const timestamp = parseInt(timestampText, 10); + + if (Number.isNaN(timestamp)) { + void logger.log(` ${dir} has invalid timestamp '${timestampText}'. Deleting.`); + await fs.remove(dir); + } else if (now - timestamp > maxQueryTime) { + void logger.log(` ${dir} is older than ${maxQueryTime / 1000} seconds. Deleting.`); + await fs.remove(dir); + } else { + void logger.log(` ${dir} is not older than ${maxQueryTime / 1000} seconds. Keeping.`); + deleted = false; + } + } + return { + deleted + }; + } catch (err) { + return { + errorMsg: ` Could not delete '${dir}': ${err}`, + deleted: false + }; + } +} diff --git a/extensions/ql-vscode/src/query-history.ts b/extensions/ql-vscode/src/query-history.ts index cd9e9983057..a7fda716c38 100644 --- a/extensions/ql-vscode/src/query-history.ts +++ b/extensions/ql-vscode/src/query-history.ts @@ -1,9 +1,11 @@ import * as path from 'path'; import { commands, + Disposable, env, Event, EventEmitter, + ExtensionContext, ProviderResult, Range, ThemeIcon, @@ -29,6 +31,7 @@ import { commandRunner } from './commandRunner'; import { assertNever } from './pure/helpers-pure'; import { FullCompletedQueryInfo, FullQueryInfo, QueryStatus } from './query-results'; import { DatabaseManager } from './databases'; +import { registerQueryHistoryScubber } from './query-history-scrubber'; /** * query-history.ts @@ -80,6 +83,9 @@ export enum SortOrder { CountDesc = 'CountDesc', } +const ONE_HOUR_IN_MS = 1000 * 60 * 60; +const TWO_HOURS_IN_MS = 1000 * 60 * 60 * 2; + /** * Tree data provider for the query history view. */ @@ -118,6 +124,7 @@ export class HistoryTreeDataProvider extends DisposableObject { title: 'Query History Item', command: 'codeQLQueryHistory.itemClicked', arguments: [element], + tooltip: element.failureReason || element.label }; // Populate the icon and the context value. We use the context value to @@ -217,6 +224,12 @@ export class HistoryTreeDataProvider extends DisposableObject { return this.history; } + set allHistory(history: FullQueryInfo[]) { + this.history = history; + this.current = history[0]; + this.refresh(); + } + refresh() { this._onDidChangeTreeData.fire(undefined); } @@ -239,17 +252,22 @@ const DOUBLE_CLICK_TIME = 500; const NO_QUERY_SELECTED = 'No query selected. Select a query history item you have already run and try again.'; +const WORKSPACE_QUERY_HISTORY_FILE = 'workspace-query-history.json'; export class QueryHistoryManager extends DisposableObject { + treeDataProvider: HistoryTreeDataProvider; treeView: TreeView; lastItemClick: { time: Date; item: FullQueryInfo } | undefined; compareWithItem: FullQueryInfo | undefined; + queryHistoryScrubber: Disposable | undefined; + private queryMetadataStorageLocation; constructor( private qs: QueryServerClient, private dbm: DatabaseManager, - extensionPath: string, - queryHistoryConfigListener: QueryHistoryConfig, + private queryStorageDir: string, + ctx: ExtensionContext, + private queryHistoryConfigListener: QueryHistoryConfig, private selectedCallback: (item: FullCompletedQueryInfo) => Promise, private doCompareCallback: ( from: FullCompletedQueryInfo, @@ -258,8 +276,14 @@ export class QueryHistoryManager extends DisposableObject { ) { super(); + // Note that we use workspace storage to hold the metadata for the query history. + // This is because the query history is specific to each workspace. + // For situations where `ctx.storageUri` is undefined (i.e., there is no workspace), + // we default to global storage. + this.queryMetadataStorageLocation = path.join((ctx.storageUri || ctx.globalStorageUri).fsPath, WORKSPACE_QUERY_HISTORY_FILE); + this.treeDataProvider = this.push(new HistoryTreeDataProvider( - extensionPath + ctx.extensionPath )); this.treeView = this.push(window.createTreeView('codeQLQueryHistory', { treeDataProvider: this.treeDataProvider, @@ -378,9 +402,15 @@ export class QueryHistoryManager extends DisposableObject { } ) ); + + // There are two configuration items that affect the query history: + // 1. The ttl for query history items. + // 2. The default label for query history items. + // When either of these change, must refresh the tree view. this.push( queryHistoryConfigListener.onDidChangeConfiguration(() => { this.treeDataProvider.refresh(); + this.registerQueryHistoryScrubber(queryHistoryConfigListener, ctx); }) ); @@ -398,6 +428,36 @@ export class QueryHistoryManager extends DisposableObject { ); }, })); + + this.registerQueryHistoryScrubber(queryHistoryConfigListener, ctx); + } + + /** + * Register and create the history scrubber. + */ + private registerQueryHistoryScrubber(queryHistoryConfigListener: QueryHistoryConfig, ctx: ExtensionContext) { + this.queryHistoryScrubber?.dispose(); + // Every hour check if we need to re-run the query history scrubber. + this.queryHistoryScrubber = this.push( + registerQueryHistoryScubber( + ONE_HOUR_IN_MS, + TWO_HOURS_IN_MS, + queryHistoryConfigListener.ttlInMillis, + this.queryStorageDir, + ctx + ) + ); + } + + async readQueryHistory(): Promise { + void logger.log(`Reading cached query history from '${this.queryMetadataStorageLocation}'.`); + const history = await FullQueryInfo.slurp(this.queryMetadataStorageLocation, this.queryHistoryConfigListener); + this.treeDataProvider.allHistory = history; + } + + async writeQueryHistory(): Promise { + const toSave = this.treeDataProvider.allHistory.filter(q => q.isCompleted()); + await FullQueryInfo.splat(toSave, this.queryMetadataStorageLocation); } async invokeCallbackOn(queryHistoryItem: FullQueryInfo) { @@ -445,14 +505,19 @@ export class QueryHistoryManager extends DisposableObject { multiSelect: FullQueryInfo[] ) { const { finalSingleItem, finalMultiSelect } = this.determineSelection(singleItem, multiSelect); - - (finalMultiSelect || [finalSingleItem]).forEach((item) => { - // Removing in progress queries is not supported yet + const toDelete = (finalMultiSelect || [finalSingleItem]); + await Promise.all(toDelete.map(async (item) => { + // Removing in progress queries is not supported. They must be cancelled first. if (item.status !== QueryStatus.InProgress) { this.treeDataProvider.remove(item); item.completedQuery?.dispose(); + + // User has explicitly asked for this query to be removed. + // We need to delete it from disk as well. + await item.completedQuery?.query.deleteQuery(); } - }); + })); + await this.writeQueryHistory(); const current = this.treeDataProvider.getCurrent(); if (current !== undefined) { await this.treeView.reveal(current, { select: true }); @@ -488,19 +553,21 @@ export class QueryHistoryManager extends DisposableObject { singleItem: FullQueryInfo, multiSelect: FullQueryInfo[] ): Promise { - if (!this.assertSingleQuery(multiSelect)) { + const { finalSingleItem, finalMultiSelect } = this.determineSelection(singleItem, multiSelect); + + if (!this.assertSingleQuery(finalMultiSelect)) { return; } const response = await window.showInputBox({ prompt: 'Label:', placeHolder: '(use default)', - value: singleItem.label, + value: finalSingleItem.label, }); // undefined response means the user cancelled the dialog; don't change anything if (response !== undefined) { // Interpret empty string response as 'go back to using default' - singleItem.initialInfo.userSpecifiedLabel = response === '' ? undefined : response; + finalSingleItem.initialInfo.userSpecifiedLabel = response === '' ? undefined : response; this.treeDataProvider.refresh(); } } @@ -509,13 +576,15 @@ export class QueryHistoryManager extends DisposableObject { singleItem: FullQueryInfo, multiSelect: FullQueryInfo[] ) { + const { finalSingleItem, finalMultiSelect } = this.determineSelection(singleItem, multiSelect); + try { - if (!singleItem.completedQuery?.didRunSuccessfully) { + if (!finalSingleItem.completedQuery?.didRunSuccessfully) { throw new Error('Please select a successful query.'); } const from = this.compareWithItem || singleItem; - const to = await this.findOtherQueryToCompare(from, multiSelect); + const to = await this.findOtherQueryToCompare(from, finalMultiSelect); if (from.isCompleted() && to?.isCompleted()) { await this.doCompareCallback(from as FullCompletedQueryInfo, to as FullCompletedQueryInfo); @@ -593,20 +662,22 @@ export class QueryHistoryManager extends DisposableObject { singleItem: FullQueryInfo, multiSelect: FullQueryInfo[] ) { - if (!this.assertSingleQuery(multiSelect)) { + const { finalSingleItem, finalMultiSelect } = this.determineSelection(singleItem, multiSelect); + + if (!this.assertSingleQuery(finalMultiSelect)) { return; } - if (!singleItem) { + if (!finalSingleItem) { throw new Error(NO_QUERY_SELECTED); } const params = new URLSearchParams({ - isQuickEval: String(!!singleItem.initialInfo.quickEvalPosition), - queryText: encodeURIComponent(await this.getQueryText(singleItem)), + isQuickEval: String(!!finalSingleItem.initialInfo.quickEvalPosition), + queryText: encodeURIComponent(await this.getQueryText(finalSingleItem)), }); const uri = Uri.parse( - `codeql:${singleItem.initialInfo.id}?${params.toString()}`, true + `codeql:${finalSingleItem.initialInfo.id}?${params.toString()}`, true ); const doc = await workspace.openTextDocument(uri); await window.showTextDocument(doc, { preview: false }); @@ -616,17 +687,20 @@ export class QueryHistoryManager extends DisposableObject { singleItem: FullQueryInfo, multiSelect: FullQueryInfo[] ) { - if (!this.assertSingleQuery(multiSelect) || !singleItem.completedQuery) { + const { finalSingleItem, finalMultiSelect } = this.determineSelection(singleItem, multiSelect); + + if (!this.assertSingleQuery(finalMultiSelect) || !finalSingleItem.completedQuery) { return; } - const query = singleItem.completedQuery.query; + + const query = finalSingleItem.completedQuery.query; const hasInterpretedResults = query.canHaveInterpretedResults(); if (hasInterpretedResults) { await this.tryOpenExternalFile( query.resultsPaths.interpretedResultsPath ); } else { - const label = singleItem.label; + const label = finalSingleItem.label; void showAndLogInformationMessage( `Query ${label} has no interpreted results.` ); @@ -637,13 +711,15 @@ export class QueryHistoryManager extends DisposableObject { singleItem: FullQueryInfo, multiSelect: FullQueryInfo[] ) { - if (!this.assertSingleQuery(multiSelect)) { + const { finalSingleItem, finalMultiSelect } = this.determineSelection(singleItem, multiSelect); + + if (!this.assertSingleQuery(finalMultiSelect)) { return; } - if (!singleItem.completedQuery) { + if (!finalSingleItem.completedQuery) { return; } - const query = singleItem.completedQuery.query; + const query = finalSingleItem.completedQuery.query; if (await query.hasCsv()) { void this.tryOpenExternalFile(query.csvPath); return; @@ -659,12 +735,14 @@ export class QueryHistoryManager extends DisposableObject { singleItem: FullQueryInfo, multiSelect: FullQueryInfo[] ) { - if (!this.assertSingleQuery(multiSelect) || !singleItem.completedQuery) { + const { finalSingleItem, finalMultiSelect } = this.determineSelection(singleItem, multiSelect); + + if (!this.assertSingleQuery(finalMultiSelect) || !finalSingleItem.completedQuery) { return; } await this.tryOpenExternalFile( - await singleItem.completedQuery.query.ensureCsvProduced(this.qs, this.dbm) + await finalSingleItem.completedQuery.query.ensureCsvProduced(this.qs, this.dbm) ); } @@ -672,15 +750,17 @@ export class QueryHistoryManager extends DisposableObject { singleItem: FullQueryInfo, multiSelect: FullQueryInfo[], ) { - if (!this.assertSingleQuery(multiSelect)) { + const { finalSingleItem, finalMultiSelect } = this.determineSelection(singleItem, multiSelect); + + if (!this.assertSingleQuery(finalMultiSelect)) { return; } - if (!singleItem.completedQuery) { + if (!finalSingleItem.completedQuery) { return; } await this.tryOpenExternalFile( - await singleItem.completedQuery.query.ensureDilPath(this.qs) + await finalSingleItem.completedQuery.query.ensureDilPath(this.qs) ); } diff --git a/extensions/ql-vscode/src/query-results.ts b/extensions/ql-vscode/src/query-results.ts index d5bb64873a7..a82e8e4bb1c 100644 --- a/extensions/ql-vscode/src/query-results.ts +++ b/extensions/ql-vscode/src/query-results.ts @@ -16,6 +16,7 @@ import { import { QueryHistoryConfig } from './config'; import { DatabaseInfo } from './pure/interface-types'; import { showAndLogErrorMessage } from './helpers'; +import { asyncFilter } from './pure/helpers-pure'; /** * A description of the information about a query @@ -190,9 +191,13 @@ export class FullQueryInfo { static async slurp(fsPath: string, config: QueryHistoryConfig): Promise { try { + if (!(await fs.pathExists(fsPath))) { + return []; + } + const data = await fs.readFile(fsPath, 'utf8'); const queries = JSON.parse(data); - return queries.map((q: FullQueryInfo) => { + const parsedQueries = queries.map((q: FullQueryInfo) => { // Need to explicitly set prototype since reading in from JSON will not // do this automatically. Note that we can't call the constructor here since @@ -215,6 +220,14 @@ export class FullQueryInfo { } return q; }); + + // filter out queries that have been deleted on disk + // most likely another workspace has deleted them because the + // queries aged out. + return asyncFilter(parsedQueries, async (q) => { + const resultsPath = q.completedQuery?.query.resultsPaths.resultsPath; + return !!resultsPath && await fs.pathExists(resultsPath); + }); } catch (e) { void showAndLogErrorMessage('Error loading query history.', { fullMessage: ['Error loading query history.', e.stack].join('\n'), @@ -234,8 +247,12 @@ export class FullQueryInfo { */ static async splat(queries: FullQueryInfo[], fsPath: string): Promise { try { - const data = JSON.stringify(queries, null, 2); - await fs.mkdirp(path.dirname(fsPath)); + if (!(await fs.pathExists(fsPath))) { + await fs.mkdir(path.dirname(fsPath), { recursive: true }); + } + // remove incomplete queries since they cannot be recreated on restart + const filteredQueries = queries.filter(q => q.completedQuery !== undefined); + const data = JSON.stringify(filteredQueries, null, 2); await fs.writeFile(fsPath, data); } catch (e) { throw new Error(`Error saving query history to ${fsPath}: ${e.message}`); @@ -253,13 +270,16 @@ export class FullQueryInfo { constructor( public readonly initialInfo: InitialQueryInfo, config: QueryHistoryConfig, - private readonly source?: CancellationTokenSource + private cancellationSource?: CancellationTokenSource // used to cancel in progress queries ) { this.setConfig(config); } cancel() { - this.source?.cancel(); + this.cancellationSource?.cancel(); + // query is no longer in progress, can delete the cancellation token source + this.cancellationSource?.dispose(); + delete this.cancellationSource; } get startTime() { @@ -342,6 +362,10 @@ export class FullQueryInfo { completeThisQuery(info: QueryWithResults) { this.completedQuery = new CompletedQueryInfo(info); + + // dispose of the cancellation token source and also ensure the source is not serialized as JSON + this.cancellationSource?.dispose(); + delete this.cancellationSource; } /** diff --git a/extensions/ql-vscode/src/run-queries.ts b/extensions/ql-vscode/src/run-queries.ts index c7997fda5ed..ee2f764679e 100644 --- a/extensions/ql-vscode/src/run-queries.ts +++ b/extensions/ql-vscode/src/run-queries.ts @@ -46,9 +46,6 @@ export const tmpDirDisposal = { } }; -// exported for testing -export const queriesDir = path.join(tmpDir.name, 'queries'); - /** * A collection of evaluation-time information about a query, * including the query itself, and where we have decided to put @@ -56,14 +53,13 @@ export const queriesDir = path.join(tmpDir.name, 'queries'); * output and results. */ export class QueryEvaluationInfo { - readonly querySaveDir: string; /** * Note that in the {@link FullQueryInfo.slurp} method, we create a QueryEvaluationInfo instance * by explicitly setting the prototype in order to avoid calling this constructor. */ constructor( - public readonly id: string, + private readonly querySaveDir: string, public readonly dbItemPath: string, private readonly databaseHasMetadataFile: boolean, public readonly queryDbscheme: string, // the dbscheme file the query expects, based on library path resolution @@ -71,7 +67,7 @@ export class QueryEvaluationInfo { public readonly metadata?: QueryMetadata, public readonly templates?: messages.TemplateDefinitions ) { - this.querySaveDir = path.join(queriesDir, this.id); + /**/ } get dilPath() { @@ -97,6 +93,16 @@ export class QueryEvaluationInfo { return path.join(this.querySaveDir, `sortedResults-${resultSetName}.bqrs`); } + /** + * Creates a file in the query directory that indicates when this query was created. + * This is important for keeping track of when queries should be removed. + */ + async createTimestampFile() { + const timestampPath = path.join(this.querySaveDir, 'timestamp'); + await fs.ensureDir(this.querySaveDir); + await fs.writeFile(timestampPath, Date.now().toString(), 'utf8'); + } + async run( qs: qsClient.QueryServerClient, upgradeQlo: string | undefined, @@ -290,6 +296,10 @@ export class QueryEvaluationInfo { await qs.cliServer.generateResultsCsv(ensureMetadataIsComplete(this.metadata), this.resultsPaths.resultsPath, this.csvPath, sourceInfo); return this.csvPath; } + + async deleteQuery(): Promise { + await fs.remove(this.querySaveDir); + } } export interface QueryWithResults { @@ -598,6 +608,7 @@ export async function compileAndRunQueryAgainstDatabase( qs: qsClient.QueryServerClient, dbItem: DatabaseItem, initialInfo: InitialQueryInfo, + queryStorageDir: string, progress: ProgressCallback, token: CancellationToken, templates?: messages.TemplateDefinitions, @@ -655,7 +666,7 @@ export async function compileAndRunQueryAgainstDatabase( const hasMetadataFile = (await dbItem.hasMetadataFile()); const query = new QueryEvaluationInfo( - initialInfo.id, + path.join(queryStorageDir, initialInfo.id), dbItem.databaseUri.fsPath, hasMetadataFile, packConfig.dbscheme, @@ -663,11 +674,13 @@ export async function compileAndRunQueryAgainstDatabase( metadata, templates ); + await query.createTimestampFile(); - const upgradeDir = await tmp.dir({ dir: upgradesTmpDir.name, unsafeCleanup: true }); + let upgradeDir: tmp.DirectoryResult | undefined; try { let upgradeQlo; if (await hasNondestructiveUpgradeCapabilities(qs)) { + upgradeDir = await tmp.dir({ dir: upgradesTmpDir.name, unsafeCleanup: true }); upgradeQlo = await compileNonDestructiveUpgrade(qs, upgradeDir, query, qlProgram, dbItem, progress, token); } else { await checkDbschemeCompatibility(cliServer, qs, query, qlProgram, dbItem, progress, token); @@ -726,7 +739,7 @@ export async function compileAndRunQueryAgainstDatabase( } } finally { try { - await upgradeDir.cleanup(); + await upgradeDir?.cleanup(); } catch (e) { void qs.logger.log(`Could not clean up the upgrades dir. Reason: ${e.message || e}`); } diff --git a/extensions/ql-vscode/src/vscode-tests/cli-integration/queries.test.ts b/extensions/ql-vscode/src/vscode-tests/cli-integration/queries.test.ts index 20a7c7932f7..9b77a172906 100644 --- a/extensions/ql-vscode/src/vscode-tests/cli-integration/queries.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/cli-integration/queries.test.ts @@ -11,7 +11,7 @@ import { DatabaseItem, DatabaseManager } from '../../databases'; import { CodeQLExtensionInterface } from '../../extension'; import { dbLoc, storagePath } from './global.helper'; import { importArchiveDatabase } from '../../databaseFetcher'; -import { compileAndRunQueryAgainstDatabase, createInitialQueryInfo } from '../../run-queries'; +import { compileAndRunQueryAgainstDatabase, createInitialQueryInfo, tmpDir } from '../../run-queries'; import { CodeQLCliServer } from '../../cli'; import { QueryServerClient } from '../../queryserver-client'; import { skipIfNoCodeQL } from '../ensureCli'; @@ -97,6 +97,7 @@ describe('Queries', function() { qs, dbItem, await mockInitialQueryInfo(queryPath), + path.join(tmpDir.name, 'mock-storage-path'), progress, token ); @@ -119,6 +120,7 @@ describe('Queries', function() { qs, dbItem, await mockInitialQueryInfo(queryPath), + path.join(tmpDir.name, 'mock-storage-path'), progress, token ); diff --git a/extensions/ql-vscode/src/vscode-tests/minimal-workspace/activation.test.ts b/extensions/ql-vscode/src/vscode-tests/minimal-workspace/activation.test.ts index ec11c773c57..f8e0bfba401 100644 --- a/extensions/ql-vscode/src/vscode-tests/minimal-workspace/activation.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/minimal-workspace/activation.test.ts @@ -15,6 +15,9 @@ describe('launching with a minimal workspace', async () => { assert(ext); }); + // Note, this test will only pass in pristine workspaces. This means that when run locally and you + // reuse an existing workspace that starts with an open ql file, this test will fail. There is + // no need to make any changes since this will still pass on CI. it('should not activate the extension at first', () => { assert(ext!.isActive === false); }); diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-history.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-history.test.ts index 0f7c20eb91a..1b6784b33f6 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/query-history.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/query-history.test.ts @@ -1,17 +1,22 @@ +import * as fs from 'fs-extra'; +import * as path from 'path'; import * as chai from 'chai'; import 'mocha'; import 'sinon-chai'; import * as vscode from 'vscode'; import * as sinon from 'sinon'; + import * as chaiAsPromised from 'chai-as-promised'; import { logger } from '../../logging'; import { QueryHistoryManager, HistoryTreeDataProvider, SortOrder } from '../../query-history'; -import { QueryEvaluationInfo, QueryWithResults } from '../../run-queries'; +import { registerQueryHistoryScubber } from '../../query-history-scrubber'; +import { QueryEvaluationInfo, QueryWithResults, tmpDir } from '../../run-queries'; import { QueryHistoryConfigListener } from '../../config'; import * as messages from '../../pure/messages'; import { QueryServerClient } from '../../queryserver-client'; import { FullQueryInfo, InitialQueryInfo } from '../../query-results'; import { DatabaseManager } from '../../databases'; +import * as tmp from 'tmp-promise'; chai.use(chaiAsPromised); const expect = chai.expect; @@ -19,6 +24,7 @@ const assert = chai.assert; describe('query-history', () => { + const mockExtensionLocation = path.join(tmpDir.name, 'mock-extension-location'); let configListener: QueryHistoryConfigListener; let showTextDocumentSpy: sinon.SinonStub; let showInformationMessageSpy: sinon.SinonStub; @@ -312,7 +318,7 @@ describe('query-history', () => { describe('HistoryTreeDataProvider', () => { let historyTreeDataProvider: HistoryTreeDataProvider; beforeEach(() => { - historyTreeDataProvider = new HistoryTreeDataProvider(vscode.Uri.file('/a/b/c').fsPath); + historyTreeDataProvider = new HistoryTreeDataProvider(vscode.Uri.file(mockExtensionLocation).fsPath); }); afterEach(() => { @@ -327,29 +333,30 @@ describe('query-history', () => { title: 'Query History Item', command: 'codeQLQueryHistory.itemClicked', arguments: [mockQuery], + tooltip: mockQuery.label, }); expect(treeItem.label).to.contain('hucairz'); expect(treeItem.contextValue).to.eq('rawResultsItem'); - expect(treeItem.iconPath).to.deep.eq(vscode.Uri.file('/a/b/c/media/drive.svg').fsPath); + expect(treeItem.iconPath).to.deep.eq(vscode.Uri.file(mockExtensionLocation + '/media/drive.svg').fsPath); }); it('should get a tree item with interpreted results', async () => { const mockQuery = createMockFullQueryInfo('a', createMockQueryWithResults(true, /* interpreted results */ true)); const treeItem = await historyTreeDataProvider.getTreeItem(mockQuery); expect(treeItem.contextValue).to.eq('interpretedResultsItem'); - expect(treeItem.iconPath).to.deep.eq(vscode.Uri.file('/a/b/c/media/drive.svg').fsPath); + expect(treeItem.iconPath).to.deep.eq(vscode.Uri.file(mockExtensionLocation + '/media/drive.svg').fsPath); }); it('should get a tree item that did not complete successfully', async () => { const mockQuery = createMockFullQueryInfo('a', createMockQueryWithResults(false), false); const treeItem = await historyTreeDataProvider.getTreeItem(mockQuery); - expect(treeItem.iconPath).to.eq(vscode.Uri.file('/a/b/c/media/red-x.svg').fsPath); + expect(treeItem.iconPath).to.eq(vscode.Uri.file(mockExtensionLocation + '/media/red-x.svg').fsPath); }); it('should get a tree item that failed before creating any results', async () => { const mockQuery = createMockFullQueryInfo('a', undefined, true); const treeItem = await historyTreeDataProvider.getTreeItem(mockQuery); - expect(treeItem.iconPath).to.eq(vscode.Uri.file('/a/b/c/media/red-x.svg').fsPath); + expect(treeItem.iconPath).to.eq(vscode.Uri.file(mockExtensionLocation + '/media/red-x.svg').fsPath); }); it('should get a tree item that is in progress', async () => { @@ -525,7 +532,9 @@ describe('query-history', () => { queryPath: 'hucairz' } as InitialQueryInfo, configListener, - {} as vscode.CancellationTokenSource + { + dispose: () => { /**/ }, + } as vscode.CancellationTokenSource ); if (queryWitbResults) { @@ -537,17 +546,196 @@ describe('query-history', () => { return fqi; } + describe('query history scrubber', () => { + let clock: sinon.SinonFakeTimers; + let deregister: vscode.Disposable | undefined; + let mockCtx: vscode.ExtensionContext; + let runCount = 0; + + const ONE_HOUR_IN_MS = 60 * 60 * 1000; + const TWO_HOURS_IN_MS = 2 * ONE_HOUR_IN_MS; + const THREE_HOURS_IN_MS = 3 * ONE_HOUR_IN_MS; + const ONE_DAY_IN_MS = 24 * ONE_HOUR_IN_MS; + // We don't want our times to align exactly with the hour, + // so we can better mimic real life + const LESS_THAN_ONE_DAY = ONE_DAY_IN_MS - 1000; + const tmpDir = tmp.dirSync({ + unsafeCleanup: true + }); + + beforeEach(() => { + clock = sandbox.useFakeTimers({ + toFake: ['setInterval', 'Date'] + }); + mockCtx = { + globalState: { + lastScrubTime: Date.now(), + get(key: string) { + if (key !== 'lastScrubTime') { + throw new Error(`Unexpected key: ${key}`); + } + return this.lastScrubTime; + }, + async update(key: string, value: any) { + if (key !== 'lastScrubTime') { + throw new Error(`Unexpected key: ${key}`); + } + this.lastScrubTime = value; + } + } + } as any as vscode.ExtensionContext; + }); + + afterEach(() => { + clock.restore(); + if (deregister) { + deregister.dispose(); + deregister = undefined; + } + }); + + it('should not throw an error when the query directory does not exist', async function() { + // because of the waits, we need to have a higher timeout on this test. + this.timeout(5000); + registerScrubber('idontexist'); + + clock.tick(ONE_HOUR_IN_MS); + await wait(); + expect(runCount, 'Should not have called the scrubber').to.eq(0); + + clock.tick(ONE_HOUR_IN_MS - 1); + await wait(); + expect(runCount, 'Should not have called the scrubber').to.eq(0); + + clock.tick(1); + await wait(); + expect(runCount, 'Should have called the scrubber once').to.eq(1); + + clock.tick(TWO_HOURS_IN_MS); + await wait(); + expect(runCount, 'Should have called the scrubber a second time').to.eq(2); + + expect((mockCtx.globalState as any).lastScrubTime).to.eq(TWO_HOURS_IN_MS * 2, 'Should have scrubbed the last time at 4 hours.'); + }); + + it('should scrub directories', async function() { + this.timeout(5000); + // create two query directories that are right around the cut off time + const queryDir = createMockQueryDir(ONE_HOUR_IN_MS, TWO_HOURS_IN_MS, THREE_HOURS_IN_MS); + registerScrubber(queryDir); + + clock.tick(TWO_HOURS_IN_MS); + await wait(); + + // should have deleted only the invalid locations + expectDirectories( + queryDir, + toQueryDirName(ONE_HOUR_IN_MS), + toQueryDirName(TWO_HOURS_IN_MS), + toQueryDirName(THREE_HOURS_IN_MS), + ); + + clock.tick(LESS_THAN_ONE_DAY); + await wait(); + + // nothing should have happened...yet + expectDirectories( + queryDir, + toQueryDirName(ONE_HOUR_IN_MS), + toQueryDirName(TWO_HOURS_IN_MS), + toQueryDirName(THREE_HOURS_IN_MS), + ); + + clock.tick(1000); + await wait(); + + // should have deleted the two older directories + // even though they have different time stamps, + // they both expire during the same scrubbing period + expectDirectories( + queryDir, + toQueryDirName(THREE_HOURS_IN_MS), + ); + + // Wait until the next scrub time and the final directory is deleted + clock.tick(TWO_HOURS_IN_MS); + await wait(); + + // should have deleted everything + expectDirectories( + queryDir + ); + }); + + function expectDirectories(queryDir: string, ...dirNames: string[]) { + const files = fs.readdirSync(queryDir); + expect(files.sort()).to.deep.eq(dirNames.sort()); + } + + function createMockQueryDir(...timestamps: number[]) { + const dir = tmpDir.name; + const queryDir = path.join(dir, 'query'); + // create qyuery directory and fill it with some query directories + fs.mkdirSync(queryDir); + + // create an invalid file + const invalidFile = path.join(queryDir, 'invalid.txt'); + fs.writeFileSync(invalidFile, 'invalid'); + + // create a directory without a timestamp file + const noTimestampDir = path.join(queryDir, 'noTimestampDir'); + fs.mkdirSync(noTimestampDir); + fs.writeFileSync(path.join(noTimestampDir, 'invalid.txt'), 'invalid'); + + // create a directory with a timestamp file, but is invalid + const invalidTimestampDir = path.join(queryDir, 'invalidTimestampDir'); + fs.mkdirSync(invalidTimestampDir); + fs.writeFileSync(path.join(invalidTimestampDir, 'timestamp'), 'invalid'); + + // create a directories with a valid timestamp files from the args + timestamps.forEach((timestamp) => { + const dir = path.join(queryDir, toQueryDirName(timestamp)); + fs.mkdirSync(dir); + fs.writeFileSync(path.join(dir, 'timestamp'), `${Date.now() + timestamp}`); + }); + + return queryDir; + } + + function toQueryDirName(timestamp: number) { + return `query-${timestamp}`; + } + + function registerScrubber(dir: string) { + deregister = registerQueryHistoryScubber( + ONE_HOUR_IN_MS, + TWO_HOURS_IN_MS, + LESS_THAN_ONE_DAY, + dir, + mockCtx, + { + increment: () => runCount++ + } + ); + } + + async function wait(ms = 500) { + return new Promise((resolve) => setTimeout(resolve, ms)); + } + }); + function createMockQueryWithResults(didRunSuccessfully = true, hasInterpretedResults = true): QueryWithResults { return { query: { - hasInterpretedResults: () => Promise.resolve(hasInterpretedResults) - } as QueryEvaluationInfo, + hasInterpretedResults: () => Promise.resolve(hasInterpretedResults), + deleteQuery: sandbox.stub(), + } as unknown as QueryEvaluationInfo, result: { resultType: didRunSuccessfully ? messages.QueryResultType.SUCCESS : messages.QueryResultType.OTHER_ERROR } as messages.EvaluationResult, - dispose: sandbox.spy(), + dispose: sandbox.spy() }; } @@ -556,6 +744,10 @@ describe('query-history', () => { {} as QueryServerClient, {} as DatabaseManager, 'xxx', + { + globalStorageUri: vscode.Uri.file(mockExtensionLocation), + extensionPath: vscode.Uri.file('/x/y/z').fsPath, + } as vscode.ExtensionContext, configListener, selectedCallback, doCompareCallback 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 85d6819a540..6228ec86fdb 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 @@ -6,13 +6,12 @@ import 'sinon-chai'; import * as sinon from 'sinon'; import * as chaiAsPromised from 'chai-as-promised'; import { FullQueryInfo, InitialQueryInfo, interpretResults } from '../../query-results'; -import { queriesDir, QueryEvaluationInfo, QueryWithResults, tmpDir } from '../../run-queries'; +import { QueryEvaluationInfo, QueryWithResults, tmpDir } from '../../run-queries'; import { QueryHistoryConfig } from '../../config'; import { EvaluationResult, QueryResultType } from '../../pure/messages'; import { DatabaseInfo, SortDirection, SortedResultSetInfo } from '../../pure/interface-types'; import { CodeQLCliServer, SourceInfo } from '../../cli'; -import { env } from 'process'; -import { CancellationTokenSource, Uri } from 'vscode'; +import { CancellationTokenSource, Uri, env } from 'vscode'; chai.use(chaiAsPromised); const expect = chai.expect; @@ -22,12 +21,15 @@ describe('query-results', () => { let onDidChangeQueryHistoryConfigurationSpy: sinon.SinonSpy; let mockConfig: QueryHistoryConfig; let sandbox: sinon.SinonSandbox; + let queryPath: string; + let cnt = 0; beforeEach(() => { sandbox = sinon.createSandbox(); disposeSpy = sandbox.spy(); onDidChangeQueryHistoryConfigurationSpy = sandbox.spy(); mockConfig = mockQueryHistoryConfig(); + queryPath = path.join(Uri.file(tmpDir.name).fsPath, `query-${cnt++}`); }); afterEach(() => { @@ -40,6 +42,7 @@ describe('query-results', () => { const date = new Date('2022-01-01T00:00:00.000Z'); const dateStr = date.toLocaleString(env.language); (fqi.initialInfo as any).start = date; + expect(fqi.interpolate('xxx')).to.eq('xxx'); expect(fqi.interpolate('%t %q %d %s %%')).to.eq(`${dateStr} hucairz a in progress %`); expect(fqi.interpolate('%t %q %d %s %%::%t %q %d %s %%')).to.eq(`${dateStr} hucairz a in progress %::${dateStr} hucairz a in progress %`); @@ -51,7 +54,7 @@ describe('query-results', () => { // from the query path expect(fqi.getQueryName()).to.eq('hucairz'); - fqi.completeThisQuery(createMockQueryWithResults()); + fqi.completeThisQuery(createMockQueryWithResults(queryPath)); // from the metadata expect(fqi.getQueryName()).to.eq('vwx'); @@ -92,7 +95,7 @@ describe('query-results', () => { // the %q from the config is now replaced by the name of the query // in the metadata - fqi.completeThisQuery(createMockQueryWithResults()); + fqi.completeThisQuery(createMockQueryWithResults(queryPath)); expect(fqi.label).to.eq('from config vwx'); // replace the config with a user specified label @@ -102,9 +105,10 @@ describe('query-results', () => { }); it('should get the getResultsPath', () => { - const fqi = createMockFullQueryInfo('a', createMockQueryWithResults()); + const query = createMockQueryWithResults(queryPath); + const fqi = createMockFullQueryInfo('a', query); const completedQuery = fqi.completedQuery!; - const expectedResultsPath = path.join(queriesDir, 'some-id/results.bqrs'); + const expectedResultsPath = path.join(queryPath, 'results.bqrs'); // from results path expect(completedQuery.getResultsPath('zxa', false)).to.eq(expectedResultsPath); @@ -121,7 +125,7 @@ describe('query-results', () => { }); it('should get the statusString', () => { - const fqi = createMockFullQueryInfo('a', createMockQueryWithResults(false)); + const fqi = createMockFullQueryInfo('a', createMockQueryWithResults(queryPath, false)); const completedQuery = fqi.completedQuery!; completedQuery.result.message = 'Tremendously'; @@ -146,7 +150,7 @@ describe('query-results', () => { it('should updateSortState', async () => { // setup - const fqi = createMockFullQueryInfo('a', createMockQueryWithResults()); + const fqi = createMockFullQueryInfo('a', createMockQueryWithResults(queryPath)); const completedQuery = fqi.completedQuery!; const spy = sandbox.spy(); @@ -162,8 +166,8 @@ describe('query-results', () => { await completedQuery.updateSortState(mockServer, 'a-result-set-name', sortState); // verify - const expectedResultsPath = path.join(queriesDir, 'some-id/results.bqrs'); - const expectedSortedResultsPath = path.join(queriesDir, 'some-id/sortedResults-a-result-set-name.bqrs'); + const expectedResultsPath = path.join(queryPath, 'results.bqrs'); + const expectedSortedResultsPath = path.join(queryPath, 'sortedResults-a-result-set-name.bqrs'); expect(spy).to.have.been.calledWith( expectedResultsPath, expectedSortedResultsPath, @@ -248,12 +252,11 @@ describe('query-results', () => { }); describe('splat and slurp', () => { - // TODO also add a test for round trip starting from file it('should splat and slurp query history', async () => { - const infoSuccessRaw = createMockFullQueryInfo('a', createMockQueryWithResults(false, false, '/a/b/c/a', false)); - const infoSuccessInterpreted = createMockFullQueryInfo('b', createMockQueryWithResults(true, true, '/a/b/c/b', false)); + 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(false, false, '/a/b/c/d', false)); + const infoLateFailure = createMockFullQueryInfo('d', createMockQueryWithResults(`${queryPath}-c`, false, false, '/a/b/c/d', false)); const infoInprogress = createMockFullQueryInfo('e'); const allHistory = [ infoSuccessRaw, @@ -263,7 +266,16 @@ describe('query-results', () => { infoInprogress ]; - const allHistoryPath = path.join(queriesDir, 'all-history.json'); + // the expected results only contains the history with completed queries + const expectedHistory = [ + infoSuccessRaw, + infoSuccessInterpreted, + infoLateFailure, + ]; + + const allHistoryPath = path.join(tmpDir.name, 'workspace-query-history.json'); + + // splat and slurp await FullQueryInfo.splat(allHistory, allHistoryPath); const allHistoryActual = await FullQueryInfo.slurp(allHistoryPath, mockConfig); @@ -287,7 +299,7 @@ describe('query-results', () => { } } }); - allHistory.forEach(info => { + expectedHistory.forEach(info => { if (info.completedQuery) { (info.completedQuery as any).dispose = undefined; } @@ -295,16 +307,27 @@ describe('query-results', () => { // make the diffs somewhat sane by comparing each element directly for (let i = 0; i < allHistoryActual.length; i++) { - expect(allHistoryActual[i]).to.deep.eq(allHistory[i]); + expect(allHistoryActual[i]).to.deep.eq(expectedHistory[i]); } - expect(allHistoryActual.length).to.deep.eq(allHistory.length); + expect(allHistoryActual.length).to.deep.eq(expectedHistory.length); }); }); - - function createMockQueryWithResults(didRunSuccessfully = true, hasInterpretedResults = true, dbPath = '/a/b/c', includeSpies = true): QueryWithResults { - const query = new QueryEvaluationInfo('some-id', - Uri.file(dbPath).fsPath, // parse the Uri to make sure it is platform-independent + function createMockQueryWithResults( + queryPath: string, + didRunSuccessfully = true, + hasInterpretedResults = true, + dbPath = '/a/b/c', + includeSpies = true + ): QueryWithResults { + // pretend that the results path exists + const resultsPath = path.join(queryPath, 'results.bqrs'); + fs.mkdirpSync(queryPath); + fs.writeFileSync(resultsPath, '', 'utf8'); + + const query = new QueryEvaluationInfo( + queryPath, + Uri.file(dbPath).fsPath, true, 'queryDbscheme', undefined, @@ -346,7 +369,9 @@ describe('query-results', () => { id: `some-id-${dbName}`, } as InitialQueryInfo, mockQueryHistoryConfig(), - {} as CancellationTokenSource + { + dispose: () => { /**/ }, + } as CancellationTokenSource ); if (queryWitbResults) { @@ -361,6 +386,7 @@ describe('query-results', () => { function mockQueryHistoryConfig(): QueryHistoryConfig { return { onDidChangeConfiguration: onDidChangeQueryHistoryConfigurationSpy, + ttlInMillis: 999999, format: 'from config %q' }; } diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/run-queries.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/run-queries.test.ts index 990f520f2dd..3b0a9f77668 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/run-queries.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/run-queries.test.ts @@ -5,7 +5,7 @@ import 'sinon-chai'; import * as sinon from 'sinon'; import * as chaiAsPromised from 'chai-as-promised'; -import { QueryEvaluationInfo, queriesDir } from '../../run-queries'; +import { QueryEvaluationInfo } from '../../run-queries'; import { Severity, compileQuery } from '../../pure/messages'; import { Uri } from 'vscode'; @@ -14,13 +14,13 @@ const expect = chai.expect; describe('run-queries', () => { it('should create a QueryEvaluationInfo', () => { - const info = createMockQueryInfo(); + const saveDir = 'query-save-dir'; + const info = createMockQueryInfo(true, saveDir); - const queryId = info.id; - expect(info.compiledQueryPath).to.eq(path.join(queriesDir, queryId, 'compiledQuery.qlo')); - expect(info.dilPath).to.eq(path.join(queriesDir, queryId, 'results.dil')); - expect(info.resultsPaths.resultsPath).to.eq(path.join(queriesDir, queryId, 'results.bqrs')); - expect(info.resultsPaths.interpretedResultsPath).to.eq(path.join(queriesDir, queryId, 'interpretedResults.sarif')); + expect(info.compiledQueryPath).to.eq(path.join(saveDir, 'compiledQuery.qlo')); + expect(info.dilPath).to.eq(path.join(saveDir, 'results.dil')); + expect(info.resultsPaths.resultsPath).to.eq(path.join(saveDir, 'results.bqrs')); + expect(info.resultsPaths.interpretedResultsPath).to.eq(path.join(saveDir, 'interpretedResults.sarif')); expect(info.dbItemPath).to.eq(Uri.file('/abc').fsPath); }); @@ -90,9 +90,9 @@ describe('run-queries', () => { }); let queryNum = 0; - function createMockQueryInfo(databaseHasMetadataFile = true) { + function createMockQueryInfo(databaseHasMetadataFile = true, saveDir = `save-dir${queryNum++}`) { return new QueryEvaluationInfo( - `save-dir${queryNum++}`, + saveDir, Uri.parse('file:///abc').fsPath, databaseHasMetadataFile, 'my-scheme', // queryDbscheme,