Clean up variant analyses directory

Author: charisk

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-scrubber.ts

@@ -1,8 +1,9 @@
-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";
 
@@ -75,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.");
+      void extLogger.log(
+        "Cleaning up query history directories. Removing old entries.",
+      );
+
       if (!(await pathExists(queryHistoryDirs.localQueriesDirPath))) {
         void extLogger.log(
-          `Cannot scrub. Query directory does not exist: ${queryHistoryDirs.localQueriesDirPath}`,
+          `Cannot clean up query history directories. Local queries directory does not exist: ${queryHistoryDirs.localQueriesDirPath}`,
+        );
+        return;
+      }
+      if (!(await pathExists(queryHistoryDirs.variantAnalysesDirPath))) {
+        void extLogger.log(
+          `Cannot clean up query history directories. Variant analyses directory does not exist: ${queryHistoryDirs.variantAnalysesDirPath}`,
         );
         return;
       }
 
-      const baseNames = await readdir(queryHistoryDirs.localQueriesDirPath);
+      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(queryHistoryDirs.localQueriesDirPath, baseName);
+      for (const dir of allDirPaths) {
         const scrubResult = await scrubDirectory(dir, now, maxQueryTime);
         if (scrubResult.errorMsg) {
           errors.push(scrubResult.errorMsg);

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/query-history-scrubber.test.ts

@@ -23,7 +23,10 @@ describe("query history scrubber", () => {
   // 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 = dirSync({
+  const tmpLocalQueriesDir = dirSync({
+    unsafeCleanup: true,
+  });
+  const tmpVariantAnalysesDir = dirSync({
     unsafeCleanup: true,
   });
 
@@ -62,7 +65,7 @@ describe("query history scrubber", () => {
   });
 
   it("should not throw an error when the query directory does not exist", async () => {
-    registerScrubber("idontexist");
+    registerScrubber("idontexist1", "idontexist2");
 
     jest.advanceTimersByTime(ONE_HOUR_IN_MS);
     await wait();
@@ -89,21 +92,35 @@ describe("query history scrubber", () => {
     );
   });
 
-  it("should scrub directories", async () => {
+  it("should scrub local query directories", async () => {
     // create two query directories that are right around the cut off time
-    const queryDir = createMockQueryDir(
+    const localQueriesDir = createMockQueryDir(
+      tmpLocalQueriesDir.name,
       ONE_HOUR_IN_MS,
       TWO_HOURS_IN_MS,
       THREE_HOURS_IN_MS,
     );
-    registerScrubber(queryDir);
+    const variantAnalysesDir = createMockQueryDir(
+      tmpVariantAnalysesDir.name,
+      ONE_HOUR_IN_MS,
+      TWO_HOURS_IN_MS,
+      THREE_HOURS_IN_MS,
+    );
+    registerScrubber(localQueriesDir, variantAnalysesDir);
 
     jest.advanceTimersByTime(TWO_HOURS_IN_MS);
     await wait();
 
     // should have deleted only the invalid locations
     expectDirectories(
-      queryDir,
+      localQueriesDir,
+      toQueryDirName(ONE_HOUR_IN_MS),
+      toQueryDirName(TWO_HOURS_IN_MS),
+      toQueryDirName(THREE_HOURS_IN_MS),
+    );
+
+    expectDirectories(
+      variantAnalysesDir,
       toQueryDirName(ONE_HOUR_IN_MS),
       toQueryDirName(TWO_HOURS_IN_MS),
       toQueryDirName(THREE_HOURS_IN_MS),
@@ -114,7 +131,14 @@ describe("query history scrubber", () => {
 
     // nothing should have happened...yet
     expectDirectories(
-      queryDir,
+      localQueriesDir,
+      toQueryDirName(ONE_HOUR_IN_MS),
+      toQueryDirName(TWO_HOURS_IN_MS),
+      toQueryDirName(THREE_HOURS_IN_MS),
+    );
+
+    expectDirectories(
+      variantAnalysesDir,
       toQueryDirName(ONE_HOUR_IN_MS),
       toQueryDirName(TWO_HOURS_IN_MS),
       toQueryDirName(THREE_HOURS_IN_MS),
@@ -126,23 +150,24 @@ describe("query history scrubber", () => {
     // 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));
+    expectDirectories(localQueriesDir, toQueryDirName(THREE_HOURS_IN_MS));
+    expectDirectories(variantAnalysesDir, toQueryDirName(THREE_HOURS_IN_MS));
 
     // Wait until the next scrub time and the final directory is deleted
     jest.advanceTimersByTime(TWO_HOURS_IN_MS);
     await wait();
 
     // should have deleted everything
-    expectDirectories(queryDir);
+    expectDirectories(localQueriesDir);
+    expectDirectories(variantAnalysesDir);
   });
 
   function expectDirectories(queryDir: string, ...dirNames: string[]) {
     const files = readdirSync(queryDir);
     expect(files.sort()).toEqual(dirNames.sort());
   }
 
-  function createMockQueryDir(...timestamps: number[]) {
-    const dir = tmpDir.name;
+  function createMockQueryDir(dir: string, ...timestamps: number[]) {
     const queryDir = join(dir, "query");
     // create qyuery directory and fill it with some query directories
     mkdirSync(queryDir);
@@ -175,12 +200,15 @@ describe("query history scrubber", () => {
     return `query-${timestamp}`;
   }
 
-  function registerScrubber(dir: string) {
+  function registerScrubber(
+    localQueriesDirPath: string,
+    variantAnalysesDirPath: string,
+  ) {
     deregister = registerQueryHistoryScrubber(
       ONE_HOUR_IN_MS,
       TWO_HOURS_IN_MS,
       LESS_THAN_ONE_DAY,
-      { localQueriesDirPath: dir, variantAnalysesDirPath: dir },
+      { localQueriesDirPath, variantAnalysesDirPath },
       {
         removeDeletedQueries: () => {
           return Promise.resolve();