Merge pull request #2118 from github/charisk/variant-analysis-scrubbing

Clean up variant analyses from query history

Author: charisk

extensions/ql-vscode/src/extension.ts

@@ -131,6 +131,7 @@ import { ExtensionApp } from "./common/vscode/vscode-app";
 import { RepositoriesFilterSortStateWithIds } from "./pure/variant-analysis-filter-sort";
 import { DbModule } from "./databases/db-module";
 import { redactableError } from "./pure/errors";
+import { QueryHistoryDirs } from "./query-history/query-history-dirs";
 
 /**
  * extension.ts
@@ -649,13 +650,18 @@ async function activateWithInstalledDistribution(
   );
 
   void extLogger.log("Initializing query history.");
+  const queryHistoryDirs: QueryHistoryDirs = {
+    localQueriesDirPath: queryStorageDir,
+    variantAnalysesDirPath: variantAnalysisStorageDir,
+  };
+
   const qhm = new QueryHistoryManager(
     qs,
     dbm,
     localQueryResultsView,
     variantAnalysisManager,
     evalLogViewer,
-    queryStorageDir,
+    queryHistoryDirs,
     ctx,
     queryHistoryConfigurationListener,
     labelProvider,

extensions/ql-vscode/src/pure/files.ts

@@ -67,3 +67,8 @@ export function pathsEqual(
   }
   return path1 === path2;
 }
+
+export async function readDirFullPaths(path: string): Promise<string[]> {
+  const baseNames = await readdir(path);
+  return baseNames.map((baseName) => join(path, baseName));
+}

extensions/ql-vscode/src/query-history/query-history-dirs.ts

@@ -0,0 +1,4 @@
+export interface QueryHistoryDirs {
+  localQueriesDirPath: string;
+  variantAnalysesDirPath: string;
+}

extensions/ql-vscode/src/query-history/query-history-manager.ts

@@ -65,6 +65,7 @@ import { VariantAnalysisHistoryItem } from "./variant-analysis-history-item";
 import { getTotalResultCount } from "../variant-analysis/shared/variant-analysis";
 import { HistoryTreeDataProvider } from "./history-tree-data-provider";
 import { redactableError } from "../pure/errors";
+import { QueryHistoryDirs } from "./query-history-dirs";
 
 /**
  * query-history-manager.ts
@@ -139,7 +140,7 @@ export class QueryHistoryManager extends DisposableObject {
     private readonly localQueriesResultsView: ResultsView,
     private readonly variantAnalysisManager: VariantAnalysisManager,
     private readonly evalLogViewer: EvalLogViewer,
-    private readonly queryStorageDir: string,
+    private readonly queryHistoryDirs: QueryHistoryDirs,
     ctx: ExtensionContext,
     private readonly queryHistoryConfigListener: QueryHistoryConfig,
     private readonly labelProvider: HistoryItemLabelProvider,
@@ -389,7 +390,7 @@ export class QueryHistoryManager extends DisposableObject {
         ONE_HOUR_IN_MS,
         TWO_HOURS_IN_MS,
         queryHistoryConfigListener.ttlInMillis,
-        this.queryStorageDir,
+        this.queryHistoryDirs,
         qhm,
         ctx,
       ),

extensions/ql-vscode/src/query-history/query-history-scrubber.ts

@@ -1,8 +1,10 @@
-import { pathExists, readdir, stat, remove, readFile } from "fs-extra";
+import { pathExists, stat, remove, readFile } from "fs-extra";
 import { EOL } from "os";
 import { join } from "path";
 import { Disposable, ExtensionContext } from "vscode";
 import { extLogger } from "../common";
+import { readDirFullPaths } from "../pure/files";
+import { QueryHistoryDirs } from "./query-history-dirs";
 import { QueryHistoryManager } from "./query-history-manager";
 
 const LAST_SCRUB_TIME_KEY = "lastScrubTime";
@@ -23,14 +25,14 @@ type Counter = {
  * @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 queryHistoryDirs The directories containing all query history information.
  * @param ctx The extension context.
  */
 export function registerQueryHistoryScrubber(
   wakeInterval: number,
   throttleTime: number,
   maxQueryTime: number,
-  queryDirectory: string,
+  queryHistoryDirs: QueryHistoryDirs,
   qhm: QueryHistoryManager,
   ctx: ExtensionContext,
 
@@ -42,7 +44,7 @@ export function registerQueryHistoryScrubber(
     wakeInterval,
     throttleTime,
     maxQueryTime,
-    queryDirectory,
+    queryHistoryDirs,
     qhm,
     ctx,
     counter,
@@ -58,7 +60,7 @@ export function registerQueryHistoryScrubber(
 async function scrubQueries(
   throttleTime: number,
   maxQueryTime: number,
-  queryDirectory: string,
+  queryHistoryDirs: QueryHistoryDirs,
   qhm: QueryHistoryManager,
   ctx: ExtensionContext,
   counter?: Counter,
@@ -74,18 +76,33 @@ async function scrubQueries(
     let scrubCount = 0; // total number of directories deleted
     try {
       counter?.increment();
-      void extLogger.log("Scrubbing query directory. Removing old queries.");
-      if (!(await pathExists(queryDirectory))) {
+      void extLogger.log(
+        "Cleaning up query history directories. Removing old entries.",
+      );
+
+      if (!(await pathExists(queryHistoryDirs.localQueriesDirPath))) {
+        void extLogger.log(
+          `Cannot clean up query history directories. Local queries directory does not exist: ${queryHistoryDirs.localQueriesDirPath}`,
+        );
+        return;
+      }
+      if (!(await pathExists(queryHistoryDirs.variantAnalysesDirPath))) {
         void extLogger.log(
-          `Cannot scrub. Query directory does not exist: ${queryDirectory}`,
+          `Cannot clean up query history directories. Variant analyses directory does not exist: ${queryHistoryDirs.variantAnalysesDirPath}`,
         );
         return;
       }
 
-      const baseNames = await readdir(queryDirectory);
+      const localQueryDirPaths = await readDirFullPaths(
+        queryHistoryDirs.localQueriesDirPath,
+      );
+      const variantAnalysisDirPaths = await readDirFullPaths(
+        queryHistoryDirs.variantAnalysesDirPath,
+      );
+      const allDirPaths = [...localQueryDirPaths, ...variantAnalysisDirPaths];
+
       const errors: string[] = [];
-      for (const baseName of baseNames) {
-        const dir = join(queryDirectory, baseName);
+      for (const dir of allDirPaths) {
         const scrubResult = await scrubDirectory(dir, now, maxQueryTime);
         if (scrubResult.errorMsg) {
           errors.push(scrubResult.errorMsg);

extensions/ql-vscode/test/factories/query-history/query-history-dirs.ts

@@ -0,0 +1,14 @@
+import { QueryHistoryDirs } from "../../../src/query-history/query-history-dirs";
+
+export function createMockQueryHistoryDirs({
+  localQueriesDirPath = "mock-local-queries-dir-path",
+  variantAnalysesDirPath = "mock-variant-analyses-dir-path",
+}: {
+  localQueriesDirPath?: string;
+  variantAnalysesDirPath?: string;
+} = {}): QueryHistoryDirs {
+  return {
+    localQueriesDirPath,
+    variantAnalysesDirPath,
+  };
+}

extensions/ql-vscode/test/unit-tests/pure/files.test.ts

@@ -4,6 +4,7 @@ import {
   gatherQlFiles,
   getDirectoryNamesInsidePath,
   pathsEqual,
+  readDirFullPaths,
 } from "../../../src/pure/files";
 
 describe("files", () => {
@@ -100,6 +101,20 @@ describe("files", () => {
       expect(result).toEqual(["sub-folder"]);
     });
   });
+
+  describe("readDirFullPaths", () => {
+    it("should return all files with full path", async () => {
+      const file1 = join(dataDir, "compute-default-strings.ql");
+      const file2 = join(dataDir, "multiple-result-sets.ql");
+      const file3 = join(dataDir, "query.ql");
+
+      const paths = await readDirFullPaths(dataDir);
+
+      expect(paths.some((path) => path === file1)).toBe(true);
+      expect(paths.some((path) => path === file2)).toBe(true);
+      expect(paths.some((path) => path === file3)).toBe(true);
+    });
+  });
 });
 
 describe("pathsEqual", () => {

extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/history-tree-data-provider.test.ts

@@ -26,6 +26,7 @@ import {
   SortOrder,
 } from "../../../../src/query-history/history-tree-data-provider";
 import { QueryHistoryManager } from "../../../../src/query-history/query-history-manager";
+import { createMockQueryHistoryDirs } from "../../../factories/query-history/query-history-dirs";
 
 describe("HistoryTreeDataProvider", () => {
   const mockExtensionLocation = join(tmpDir.name, "mock-extension-location");
@@ -425,7 +426,7 @@ describe("HistoryTreeDataProvider", () => {
       localQueriesResultsViewStub,
       variantAnalysisManagerStub,
       {} as EvalLogViewer,
-      "xxx",
+      createMockQueryHistoryDirs(),
       {
         globalStorageUri: vscode.Uri.file(mockExtensionLocation),
         extensionPath: vscode.Uri.file("/x/y/z").fsPath,

extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/query-history-manager.test.ts

@@ -26,6 +26,7 @@ import { QuickPickItem, TextEditor } from "vscode";
 import { WebviewReveal } from "../../../../src/interface-utils";
 import * as helpers from "../../../../src/helpers";
 import { mockedObject } from "../../utils/mocking.helpers";
+import { createMockQueryHistoryDirs } from "../../../factories/query-history/query-history-dirs";
 
 describe("QueryHistoryManager", () => {
   const mockExtensionLocation = join(tmpDir.name, "mock-extension-location");
@@ -1150,7 +1151,7 @@ describe("QueryHistoryManager", () => {
       localQueriesResultsViewStub,
       variantAnalysisManagerStub,
       {} as EvalLogViewer,
-      "xxx",
+      createMockQueryHistoryDirs(),
       {
         globalStorageUri: vscode.Uri.file(mockExtensionLocation),
         extensionPath: vscode.Uri.file("/x/y/z").fsPath,

extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/query-history-scrubber.test.ts

@@ -181,7 +181,7 @@ describe("query history scrubber", () => {
       ONE_HOUR_IN_MS,
       TWO_HOURS_IN_MS,
       LESS_THAN_ONE_DAY,
-      dir,
+      { localQueriesDirPath: dir, variantAnalysesDirPath: dir },
       mockedObject<QueryHistoryManager>({
         removeDeletedQueries: () => {
           return Promise.resolve();