Merge pull request github#3376 from github/robertbrignull/dont-model-twice

Don't send methods to AutoModel more than once

Author: robertbrignull

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

@@ -22,7 +22,13 @@ export function getCandidates(
   mode: Mode,
   methods: readonly Method[],
   modeledMethodsBySignature: Record<string, readonly ModeledMethod[]>,
+  processedByAutoModelMethods: Set<string>,
 ): MethodSignature[] {
+  // Filter out any methods already processed by auto-model
+  methods = methods.filter(
+    (m) => !processedByAutoModelMethods.has(m.signature),
+  );
+
   // Sort the same way as the UI so we send the first ones listed in the UI first
   const grouped = groupMethods(methods, mode);
   const sortedGroupNames = sortGroupNames(grouped);

extensions/ql-vscode/src/model-editor/auto-modeler.ts

@@ -58,6 +58,7 @@ export class AutoModeler {
     packageName: string,
     methods: readonly Method[],
     modeledMethods: Record<string, readonly ModeledMethod[]>,
+    processedByAutoModelMethods: Set<string>,
     mode: Mode,
   ): Promise<void> {
     if (this.jobs.has(packageName)) {
@@ -72,6 +73,7 @@ export class AutoModeler {
         packageName,
         methods,
         modeledMethods,
+        processedByAutoModelMethods,
         mode,
         cancellationTokenSource,
       );
@@ -105,6 +107,7 @@ export class AutoModeler {
     packageName: string,
     methods: readonly Method[],
     modeledMethods: Record<string, readonly ModeledMethod[]>,
+    processedByAutoModelMethods: Set<string>,
     mode: Mode,
     cancellationTokenSource: CancellationTokenSource,
   ): Promise<void> {
@@ -114,7 +117,12 @@ export class AutoModeler {
 
     await withProgress(async (progress) => {
       // Fetch the candidates to send to the model
-      const allCandidateMethods = getCandidates(mode, methods, modeledMethods);
+      const allCandidateMethods = getCandidates(
+        mode,
+        methods,
+        modeledMethods,
+        processedByAutoModelMethods,
+      );
 
       // If there are no candidates, there is nothing to model and we just return
       if (allCandidateMethods.length === 0) {

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

@@ -655,11 +655,17 @@ export class ModelEditorView extends AbstractWebview<
       this.databaseItem,
       methodSignatures,
     );
+    const processedByAutoModelMethods =
+      this.modelingStore.getProcessedByAutoModelMethods(
+        this.databaseItem,
+        methodSignatures,
+      );
     const mode = this.modelingStore.getMode(this.databaseItem);
     await this.autoModeler.startModeling(
       packageName,
       methods,
       modeledMethods,
+      processedByAutoModelMethods,
       mode,
     );
   }

extensions/ql-vscode/src/model-editor/modeling-store.ts

@@ -344,6 +344,22 @@ export class ModelingStore extends DisposableObject {
     });
   }
 
+  public getProcessedByAutoModelMethods(
+    dbItem: DatabaseItem,
+    methodSignatures?: string[],
+  ): Set<string> {
+    const processedByAutoModelMethods =
+      this.getState(dbItem).processedByAutoModelMethods;
+    if (!methodSignatures) {
+      return processedByAutoModelMethods;
+    }
+    return new Set(
+      Array.from(processedByAutoModelMethods).filter((x) =>
+        methodSignatures.includes(x),
+      ),
+    );
+  }
+
   public addProcessedByAutoModelMethods(
     dbItem: DatabaseItem,
     processedByAutoModelMethods: string[],

extensions/ql-vscode/test/unit-tests/model-editor/auto-model.test.ts

@@ -116,7 +116,12 @@ describe("getCandidates", () => {
         },
       ],
     };
-    const candidates = getCandidates(Mode.Application, methods, modeledMethods);
+    const candidates = getCandidates(
+      Mode.Application,
+      methods,
+      modeledMethods,
+      new Set(),
+    );
     expect(candidates.length).toEqual(0);
   });
 
@@ -136,7 +141,37 @@ describe("getCandidates", () => {
       },
     ];
     const modeledMethods = {};
-    const candidates = getCandidates(Mode.Application, methods, modeledMethods);
+    const candidates = getCandidates(
+      Mode.Application,
+      methods,
+      modeledMethods,
+      new Set(),
+    );
+    expect(candidates.length).toEqual(0);
+  });
+
+  it("doesn't return methods that are already processed by auto model", () => {
+    const methods: Method[] = [
+      {
+        library: "my.jar",
+        signature: "org.my.A#x()",
+        endpointType: EndpointType.Method,
+        packageName: "org.my",
+        typeName: "A",
+        methodName: "x",
+        methodParameters: "()",
+        supported: false,
+        supportedType: "none",
+        usages: [],
+      },
+    ];
+    const modeledMethods = {};
+    const candidates = getCandidates(
+      Mode.Application,
+      methods,
+      modeledMethods,
+      new Set(["org.my.A#x()"]),
+    );
     expect(candidates.length).toEqual(0);
   });
 
@@ -155,7 +190,12 @@ describe("getCandidates", () => {
       usages: [],
     });
     const modeledMethods = {};
-    const candidates = getCandidates(Mode.Application, methods, modeledMethods);
+    const candidates = getCandidates(
+      Mode.Application,
+      methods,
+      modeledMethods,
+      new Set(),
+    );
     expect(candidates.length).toEqual(1);
   });
 });