Merge pull request #2794 from github/koesie10/improve-model-editor

Improve responsiveness of model editor

Author: koesie10

extensions/ql-vscode/src/local-queries/run-query.ts

@@ -1,9 +1,7 @@
 import { CancellationToken } from "vscode";
-import { CodeQLCliServer } from "../codeql-cli/cli";
 import { ProgressCallback } from "../common/vscode/progress";
 import { DatabaseItem } from "../databases/local-databases";
 import { CoreCompletedQuery, QueryRunner } from "../query-server";
-import { createLockFileForStandardQuery } from "./standard-queries";
 import { TeeLogger, showAndLogExceptionWithTelemetry } from "../common/logging";
 import { QueryResultType } from "../query-server/new-messages";
 import { extLogger } from "../common/logging/vscode";
@@ -12,7 +10,6 @@ import { redactableError } from "../common/errors";
 import { basename } from "path";
 
 type RunQueryOptions = {
-  cliServer: CodeQLCliServer;
   queryRunner: QueryRunner;
   databaseItem: DatabaseItem;
   queryPath: string;
@@ -21,11 +18,9 @@ type RunQueryOptions = {
   extensionPacks: string[] | undefined;
   progress: ProgressCallback;
   token: CancellationToken;
-  createLockFile: boolean;
 };
 
 export async function runQuery({
-  cliServer,
   queryRunner,
   databaseItem,
   queryPath,
@@ -34,18 +29,7 @@ export async function runQuery({
   extensionPacks,
   progress,
   token,
-  createLockFile,
 }: RunQueryOptions): Promise<CoreCompletedQuery | undefined> {
-  let cleanupLockFile: (() => Promise<void>) | undefined = undefined;
-  if (createLockFile) {
-    // Create a lock file for the query. This is required to resolve dependencies and library path for the query.
-    const { cleanup } = await createLockFileForStandardQuery(
-      cliServer,
-      queryPath,
-    );
-    cleanupLockFile = cleanup;
-  }
-
   // Create a query run to execute
   const queryRun = queryRunner.createQueryRun(
     databaseItem.databaseUri.fsPath,
@@ -68,8 +52,6 @@ export async function runQuery({
     new TeeLogger(queryRunner.logger, queryRun.outputDir.logPath),
   );
 
-  await cleanupLockFile?.();
-
   if (completedQuery.resultType !== QueryResultType.SUCCESS) {
     void showAndLogExceptionWithTelemetry(
       extLogger,

extensions/ql-vscode/src/model-editor/auto-model-codeml-queries.ts

@@ -75,7 +75,6 @@ export async function runAutoModelQueries({
 
   // Run the actual query
   const completedQuery = await runQuery({
-    cliServer,
     queryRunner,
     databaseItem,
     queryPath,
@@ -84,7 +83,6 @@ export async function runAutoModelQueries({
     extensionPacks,
     progress,
     token: cancellationTokenSource.token,
-    createLockFile: false,
   });
 
   if (!completedQuery) {

extensions/ql-vscode/src/model-editor/extension-pack-picker.ts

@@ -18,13 +18,12 @@ import {
 } from "./extension-pack-name";
 import { autoPickExtensionsDirectory } from "./extensions-workspace-folder";
 
-const maxStep = 3;
-
 export async function pickExtensionPack(
   cliServer: Pick<CodeQLCliServer, "resolveQlpacks">,
   databaseItem: Pick<DatabaseItem, "name" | "language">,
   logger: NotificationLogger,
   progress: ProgressCallback,
+  maxStep: number,
 ): Promise<ExtensionPack | undefined> {
   progress({
     message: "Resolving extension packs...",

extensions/ql-vscode/src/model-editor/external-api-usage-queries.ts

@@ -92,7 +92,6 @@ export async function runExternalApiQueries(
 
   // Run the actual query
   const completedQuery = await runQuery({
-    cliServer,
     queryRunner,
     databaseItem,
     queryPath,
@@ -106,8 +105,6 @@ export async function runExternalApiQueries(
         message: update.message,
       }),
     token,
-    // We need to create a lock file, because the query is inside our own pack
-    createLockFile: true,
   });
 
   if (!completedQuery) {

extensions/ql-vscode/src/model-editor/flow-model-queries.ts

@@ -120,7 +120,6 @@ async function runSingleFlowQuery(
 
   // Run the query
   const completedQuery = await runQuery({
-    cliServer,
     queryRunner,
     databaseItem,
     queryPath,
@@ -134,7 +133,6 @@ async function runSingleFlowQuery(
         maxStep: 4000,
       }),
     token,
-    createLockFile: false,
   });
 
   if (!completedQuery) {

extensions/ql-vscode/src/model-editor/model-editor-module.ts

@@ -102,6 +102,8 @@ export class ModelEditorModule extends DisposableObject {
 
         return withProgress(
           async (progress) => {
+            const maxStep = 4;
+
             if (!(await this.cliServer.cliConstraints.supportsQlpacksKind())) {
               void showAndLogErrorMessage(
                 this.app.logger,
@@ -125,18 +127,31 @@ export class ModelEditorModule extends DisposableObject {
               db,
               this.app.logger,
               progress,
+              maxStep,
             );
             if (!modelFile) {
               return;
             }
 
+            progress({
+              message: "Installing dependencies...",
+              step: 3,
+              maxStep,
+            });
+
             // Create new temporary directory for query files and pack dependencies
             const queryDir = (await dir({ unsafeCleanup: true })).path;
             const success = await setUpPack(this.cliServer, queryDir, language);
             if (!success) {
               return;
             }
 
+            progress({
+              message: "Opening editor...",
+              step: 4,
+              maxStep,
+            });
+
             const view = new ModelEditorView(
               this.ctx,
               this.app,

extensions/ql-vscode/src/model-editor/model-editor-view.ts

@@ -484,6 +484,7 @@ export class ModelEditorView extends AbstractWebview<
         addedDatabase,
         this.app.logger,
         progress,
+        3,
       );
       if (!modelFile) {
         return;

extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/extension-pack-picker.test.ts

@@ -34,6 +34,7 @@ describe("pickExtensionPack", () => {
   let workspaceFolder: WorkspaceFolder;
 
   const logger = createMockLogger();
+  const maxStep = 4;
 
   beforeEach(async () => {
     tmpDir = (
@@ -98,7 +99,13 @@ describe("pickExtensionPack", () => {
     const cliServer = mockCliServer(qlPacks);
 
     expect(
-      await pickExtensionPack(cliServer, databaseItem, logger, progress),
+      await pickExtensionPack(
+        cliServer,
+        databaseItem,
+        logger,
+        progress,
+        maxStep,
+      ),
     ).toEqual(autoExtensionPack);
     expect(cliServer.resolveQlpacks).toHaveBeenCalledTimes(1);
     expect(cliServer.resolveQlpacks).toHaveBeenCalledWith(
@@ -173,7 +180,13 @@ describe("pickExtensionPack", () => {
     const cliServer = mockCliServer({});
 
     expect(
-      await pickExtensionPack(cliServer, databaseItem, logger, progress),
+      await pickExtensionPack(
+        cliServer,
+        databaseItem,
+        logger,
+        progress,
+        maxStep,
+      ),
     ).toEqual({
       path: newPackDir,
       yamlPath: join(newPackDir, "codeql-pack.yml"),
@@ -241,7 +254,13 @@ describe("pickExtensionPack", () => {
     const cliServer = mockCliServer({});
 
     expect(
-      await pickExtensionPack(cliServer, databaseItem, logger, progress),
+      await pickExtensionPack(
+        cliServer,
+        databaseItem,
+        logger,
+        progress,
+        maxStep,
+      ),
     ).toEqual({
       path: newPackDir,
       yamlPath: join(newPackDir, "codeql-pack.yml"),
@@ -277,7 +296,13 @@ describe("pickExtensionPack", () => {
     });
 
     expect(
-      await pickExtensionPack(cliServer, databaseItem, logger, progress),
+      await pickExtensionPack(
+        cliServer,
+        databaseItem,
+        logger,
+        progress,
+        maxStep,
+      ),
     ).toEqual(undefined);
     expect(logger.showErrorMessage).toHaveBeenCalledTimes(1);
     expect(logger.showErrorMessage).toHaveBeenCalledWith(
@@ -296,7 +321,13 @@ describe("pickExtensionPack", () => {
     });
 
     expect(
-      await pickExtensionPack(cliServer, databaseItem, logger, progress),
+      await pickExtensionPack(
+        cliServer,
+        databaseItem,
+        logger,
+        progress,
+        maxStep,
+      ),
     ).toEqual(undefined);
     expect(logger.showErrorMessage).toHaveBeenCalledTimes(1);
     expect(logger.showErrorMessage).toHaveBeenCalledWith(
@@ -317,7 +348,13 @@ describe("pickExtensionPack", () => {
     await outputFile(join(tmpDir.path, "codeql-pack.yml"), dumpYaml("java"));
 
     expect(
-      await pickExtensionPack(cliServer, databaseItem, logger, progress),
+      await pickExtensionPack(
+        cliServer,
+        databaseItem,
+        logger,
+        progress,
+        maxStep,
+      ),
     ).toEqual(undefined);
     expect(logger.showErrorMessage).toHaveBeenCalledTimes(1);
     expect(logger.showErrorMessage).toHaveBeenCalledWith(
@@ -348,7 +385,13 @@ describe("pickExtensionPack", () => {
     );
 
     expect(
-      await pickExtensionPack(cliServer, databaseItem, logger, progress),
+      await pickExtensionPack(
+        cliServer,
+        databaseItem,
+        logger,
+        progress,
+        maxStep,
+      ),
     ).toEqual(undefined);
     expect(logger.showErrorMessage).toHaveBeenCalledTimes(1);
     expect(logger.showErrorMessage).toHaveBeenCalledWith(
@@ -382,7 +425,13 @@ describe("pickExtensionPack", () => {
     );
 
     expect(
-      await pickExtensionPack(cliServer, databaseItem, logger, progress),
+      await pickExtensionPack(
+        cliServer,
+        databaseItem,
+        logger,
+        progress,
+        maxStep,
+      ),
     ).toEqual(undefined);
     expect(logger.showErrorMessage).toHaveBeenCalledTimes(1);
     expect(logger.showErrorMessage).toHaveBeenCalledWith(
@@ -433,7 +482,13 @@ describe("pickExtensionPack", () => {
     };
 
     expect(
-      await pickExtensionPack(cliServer, databaseItem, logger, progress),
+      await pickExtensionPack(
+        cliServer,
+        databaseItem,
+        logger,
+        progress,
+        maxStep,
+      ),
     ).toEqual(extensionPack);
   });
 });