Merge pull request #3395 from github/robertbrignull/set-modeled-and-modified-methods

Set modeled methods and modified methods together

Author: robertbrignull

extensions/ql-vscode/src/common/interface-types.ts

@@ -529,9 +529,10 @@ interface SetMethodsMessage {
   methods: Method[];
 }
 
-interface SetModeledMethodsMessage {
-  t: "setModeledMethods";
+interface SetModeledAndModifiedMethodsMessage {
+  t: "setModeledAndModifiedMethods";
   methods: Record<string, ModeledMethod[]>;
+  modifiedMethodSignatures: string[];
 }
 
 interface SetModifiedMethodsMessage {
@@ -647,7 +648,7 @@ interface SetModelEvaluationRunMessage {
 export type ToModelEditorMessage =
   | SetExtensionPackStateMessage
   | SetMethodsMessage
-  | SetModeledMethodsMessage
+  | SetModeledAndModifiedMethodsMessage
   | SetModifiedMethodsMessage
   | SetInProgressMethodsMessage
   | SetProcessedByAutoModelMethodsMessage

extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts

@@ -126,10 +126,7 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider<
           this.databaseItem,
           msg.methodSignature,
           msg.modeledMethods,
-        );
-        this.modelingStore.addModifiedMethod(
-          this.databaseItem,
-          msg.methodSignature,
+          true,
         );
         break;
       }
@@ -164,7 +161,7 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider<
 
   private registerToModelingEvents(): void {
     this.push(
-      this.modelingEvents.onModeledMethodsChanged(async (e) => {
+      this.modelingEvents.onModeledAndModifiedMethodsChanged(async (e) => {
         if (this.webviewView && e.isActiveDb && this.method) {
           const modeledMethods = e.modeledMethods[this.method.signature];
           if (modeledMethods) {
@@ -174,17 +171,10 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider<
               modeledMethods,
             });
           }
-        }
-      }),
-    );
 
-    this.push(
-      this.modelingEvents.onModifiedMethodsChanged(async (e) => {
-        if (this.webviewView && e.isActiveDb && this.method) {
-          const isModified = e.modifiedMethods.has(this.method.signature);
           await this.postMessage({
             t: "setMethodModified",
-            isModified,
+            isModified: e.modifiedMethodSignatures.has(this.method.signature),
           });
         }
       }),

extensions/ql-vscode/src/model-editor/methods-usage/methods-usage-panel.ts

@@ -102,7 +102,7 @@ export class MethodsUsagePanel extends DisposableObject {
     );
 
     this.push(
-      this.modelingEvents.onModifiedMethodsChanged(async (event) => {
+      this.modelingEvents.onModeledAndModifiedMethodsChanged(async (event) => {
         if (event.isActiveDb) {
           await this.handleStateChangeEvent();
         }

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

@@ -910,22 +910,12 @@ export class ModelEditorView extends AbstractWebview<
     );
 
     this.push(
-      this.modelingEvents.onModeledMethodsChanged(async (event) => {
+      this.modelingEvents.onModeledAndModifiedMethodsChanged(async (event) => {
         if (event.dbUri === this.databaseItem.databaseUri.toString()) {
           await this.postMessage({
-            t: "setModeledMethods",
+            t: "setModeledAndModifiedMethods",
             methods: event.modeledMethods,
-          });
-        }
-      }),
-    );
-
-    this.push(
-      this.modelingEvents.onModifiedMethodsChanged(async (event) => {
-        if (event.dbUri === this.databaseItem.databaseUri.toString()) {
-          await this.postMessage({
-            t: "setModifiedMethods",
-            methodSignatures: [...event.modifiedMethods],
+            modifiedMethodSignatures: [...event.modifiedMethodSignatures],
           });
         }
       }),
@@ -981,11 +971,10 @@ export class ModelEditorView extends AbstractWebview<
   }
 
   private addModeledMethods(modeledMethods: Record<string, ModeledMethod[]>) {
-    this.modelingStore.addModeledMethods(this.databaseItem, modeledMethods);
-
-    this.modelingStore.addModifiedMethods(
+    this.modelingStore.addModeledMethods(
       this.databaseItem,
-      new Set(Object.keys(modeledMethods)),
+      modeledMethods,
+      true,
     );
   }
 
@@ -1008,8 +997,8 @@ export class ModelEditorView extends AbstractWebview<
       this.databaseItem,
       signature,
       methods,
+      true,
     );
-    this.modelingStore.addModifiedMethod(this.databaseItem, signature);
   }
 
   private async updateModelEvaluationRun(run: ModelEvaluationRunState) {

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

@@ -24,14 +24,9 @@ interface ModeChangedEvent {
   readonly isActiveDb: boolean;
 }
 
-interface ModeledMethodsChangedEvent {
+interface ModeledAndModifiedMethodsChangedEvent {
   readonly modeledMethods: Readonly<Record<string, ModeledMethod[]>>;
-  readonly dbUri: string;
-  readonly isActiveDb: boolean;
-}
-
-interface ModifiedMethodsChangedEvent {
-  readonly modifiedMethods: ReadonlySet<string>;
+  readonly modifiedMethodSignatures: ReadonlySet<string>;
   readonly dbUri: string;
   readonly isActiveDb: boolean;
 }
@@ -77,8 +72,7 @@ export class ModelingEvents extends DisposableObject {
   public readonly onMethodsChanged: AppEvent<MethodsChangedEvent>;
   public readonly onHideModeledMethodsChanged: AppEvent<HideModeledMethodsChangedEvent>;
   public readonly onModeChanged: AppEvent<ModeChangedEvent>;
-  public readonly onModeledMethodsChanged: AppEvent<ModeledMethodsChangedEvent>;
-  public readonly onModifiedMethodsChanged: AppEvent<ModifiedMethodsChangedEvent>;
+  public readonly onModeledAndModifiedMethodsChanged: AppEvent<ModeledAndModifiedMethodsChangedEvent>;
   public readonly onSelectedMethodChanged: AppEvent<SelectedMethodChangedEvent>;
   public readonly onInProgressMethodsChanged: AppEvent<InProgressMethodsChangedEvent>;
   public readonly onProcessedByAutoModelMethodsChanged: AppEvent<ProcessedByAutoModelMethodsChangedEvent>;
@@ -92,8 +86,7 @@ export class ModelingEvents extends DisposableObject {
   private readonly onMethodsChangedEventEmitter: AppEventEmitter<MethodsChangedEvent>;
   private readonly onHideModeledMethodsChangedEventEmitter: AppEventEmitter<HideModeledMethodsChangedEvent>;
   private readonly onModeChangedEventEmitter: AppEventEmitter<ModeChangedEvent>;
-  private readonly onModeledMethodsChangedEventEmitter: AppEventEmitter<ModeledMethodsChangedEvent>;
-  private readonly onModifiedMethodsChangedEventEmitter: AppEventEmitter<ModifiedMethodsChangedEvent>;
+  private readonly onModeledAndModifiedMethodsChangedEventEmitter: AppEventEmitter<ModeledAndModifiedMethodsChangedEvent>;
   private readonly onSelectedMethodChangedEventEmitter: AppEventEmitter<SelectedMethodChangedEvent>;
   private readonly onInProgressMethodsChangedEventEmitter: AppEventEmitter<InProgressMethodsChangedEvent>;
   private readonly onProcessedByAutoModelMethodsChangedEventEmitter: AppEventEmitter<ProcessedByAutoModelMethodsChangedEvent>;
@@ -133,17 +126,11 @@ export class ModelingEvents extends DisposableObject {
     );
     this.onModeChanged = this.onModeChangedEventEmitter.event;
 
-    this.onModeledMethodsChangedEventEmitter = this.push(
-      app.createEventEmitter<ModeledMethodsChangedEvent>(),
+    this.onModeledAndModifiedMethodsChangedEventEmitter = this.push(
+      app.createEventEmitter<ModeledAndModifiedMethodsChangedEvent>(),
     );
-    this.onModeledMethodsChanged =
-      this.onModeledMethodsChangedEventEmitter.event;
-
-    this.onModifiedMethodsChangedEventEmitter = this.push(
-      app.createEventEmitter<ModifiedMethodsChangedEvent>(),
-    );
-    this.onModifiedMethodsChanged =
-      this.onModifiedMethodsChangedEventEmitter.event;
+    this.onModeledAndModifiedMethodsChanged =
+      this.onModeledAndModifiedMethodsChangedEventEmitter.event;
 
     this.onSelectedMethodChangedEventEmitter = this.push(
       app.createEventEmitter<SelectedMethodChangedEvent>(),
@@ -223,25 +210,15 @@ export class ModelingEvents extends DisposableObject {
     });
   }
 
-  public fireModeledMethodsChangedEvent(
+  public fireModeledAndModifiedMethodsChangedEvent(
     modeledMethods: Record<string, ModeledMethod[]>,
+    modifiedMethodSignatures: ReadonlySet<string>,
     dbUri: string,
     isActiveDb: boolean,
   ) {
-    this.onModeledMethodsChangedEventEmitter.fire({
+    this.onModeledAndModifiedMethodsChangedEventEmitter.fire({
       modeledMethods,
-      dbUri,
-      isActiveDb,
-    });
-  }
-
-  public fireModifiedMethodsChangedEvent(
-    modifiedMethods: ReadonlySet<string>,
-    dbUri: string,
-    isActiveDb: boolean,
-  ) {
-    this.onModifiedMethodsChangedEventEmitter.fire({
-      modifiedMethods,
+      modifiedMethodSignatures,
       dbUri,
       isActiveDb,
     });

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

@@ -227,8 +227,9 @@ export class ModelingStore extends DisposableObject {
   public addModeledMethods(
     dbItem: DatabaseItem,
     methods: Record<string, ModeledMethod[]>,
+    setModified: boolean,
   ) {
-    this.changeModeledMethods(dbItem, (state) => {
+    this.changeModeledAndModifiedMethods(dbItem, (state) => {
       const newModeledMethods = {
         ...methods,
         // Keep all methods that are already modeled in some form in the state
@@ -239,14 +240,22 @@ export class ModelingStore extends DisposableObject {
         ),
       };
       state.modeledMethods = newModeledMethods;
+
+      if (setModified) {
+        const newModifiedMethods = new Set([
+          ...state.modifiedMethodSignatures,
+          ...new Set(Object.keys(methods)),
+        ]);
+        state.modifiedMethodSignatures = newModifiedMethods;
+      }
     });
   }
 
   public setModeledMethods(
     dbItem: DatabaseItem,
     methods: Record<string, ModeledMethod[]>,
   ) {
-    this.changeModeledMethods(dbItem, (state) => {
+    this.changeModeledAndModifiedMethods(dbItem, (state) => {
       state.modeledMethods = { ...methods };
     });
   }
@@ -255,45 +264,28 @@ export class ModelingStore extends DisposableObject {
     dbItem: DatabaseItem,
     signature: string,
     modeledMethods: ModeledMethod[],
+    setModified: boolean,
   ) {
-    this.changeModeledMethods(dbItem, (state) => {
+    this.changeModeledAndModifiedMethods(dbItem, (state) => {
       const newModeledMethods = { ...state.modeledMethods };
       newModeledMethods[signature] = modeledMethods;
       state.modeledMethods = newModeledMethods;
-    });
-  }
-
-  public setModifiedMethods(
-    dbItem: DatabaseItem,
-    methodSignatures: Set<string>,
-  ) {
-    this.changeModifiedMethods(dbItem, (state) => {
-      state.modifiedMethodSignatures = new Set(methodSignatures);
-    });
-  }
 
-  public addModifiedMethods(
-    dbItem: DatabaseItem,
-    methodSignatures: Iterable<string>,
-  ) {
-    this.changeModifiedMethods(dbItem, (state) => {
-      const newModifiedMethods = new Set([
-        ...state.modifiedMethodSignatures,
-        ...methodSignatures,
-      ]);
-      state.modifiedMethodSignatures = newModifiedMethods;
+      if (setModified) {
+        const newModifiedMethods = new Set([
+          ...state.modifiedMethodSignatures,
+          signature,
+        ]);
+        state.modifiedMethodSignatures = newModifiedMethods;
+      }
     });
   }
 
-  public addModifiedMethod(dbItem: DatabaseItem, methodSignature: string) {
-    this.addModifiedMethods(dbItem, [methodSignature]);
-  }
-
   public removeModifiedMethods(
     dbItem: DatabaseItem,
     methodSignatures: string[],
   ) {
-    this.changeModifiedMethods(dbItem, (state) => {
+    this.changeModeledAndModifiedMethods(dbItem, (state) => {
       const newModifiedMethods = Array.from(
         state.modifiedMethodSignatures,
       ).filter((s) => !methodSignatures.includes(s));
@@ -447,31 +439,17 @@ export class ModelingStore extends DisposableObject {
     );
   }
 
-  private changeModifiedMethods(
+  private changeModeledAndModifiedMethods(
     dbItem: DatabaseItem,
     updateState: (state: InternalDbModelingState) => void,
   ) {
     const state = this.getState(dbItem);
 
     updateState(state);
 
-    this.modelingEvents.fireModifiedMethodsChangedEvent(
-      state.modifiedMethodSignatures,
-      dbItem.databaseUri.toString(),
-      dbItem.databaseUri.toString() === this.activeDb,
-    );
-  }
-
-  private changeModeledMethods(
-    dbItem: DatabaseItem,
-    updateState: (state: InternalDbModelingState) => void,
-  ) {
-    const state = this.getState(dbItem);
-
-    updateState(state);
-
-    this.modelingEvents.fireModeledMethodsChangedEvent(
+    this.modelingEvents.fireModeledAndModifiedMethodsChangedEvent(
       state.modeledMethods,
+      state.modifiedMethodSignatures,
       dbItem.databaseUri.toString(),
       dbItem.databaseUri.toString() === this.activeDb,
     );

extensions/ql-vscode/src/view/model-editor/ModelEditor.tsx

@@ -198,8 +198,9 @@ export function ModelEditor({
           case "setMethods":
             setMethods(msg.methods);
             break;
-          case "setModeledMethods":
+          case "setModeledAndModifiedMethods":
             setModeledMethods(msg.methods);
+            setModifiedSignatures(new Set(msg.modifiedMethodSignatures));
             break;
           case "setModifiedMethods":
             setModifiedSignatures(new Set(msg.methodSignatures));

extensions/ql-vscode/test/__mocks__/model-editor/modelingEventsMock.ts

@@ -7,8 +7,7 @@ export function createMockModelingEvents({
   onMethodsChanged = jest.fn(),
   onHideModeledMethodsChanged = jest.fn(),
   onModeChanged = jest.fn(),
-  onModeledMethodsChanged = jest.fn(),
-  onModifiedMethodsChanged = jest.fn(),
+  onModeledAndModifiedMethodsChanged = jest.fn(),
   onInProgressMethodsChanged = jest.fn(),
   onProcessedByAutoModelMethodsChanged = jest.fn(),
   onRevealInModelEditor = jest.fn(),
@@ -20,8 +19,7 @@ export function createMockModelingEvents({
   onMethodsChanged?: ModelingEvents["onMethodsChanged"];
   onHideModeledMethodsChanged?: ModelingEvents["onHideModeledMethodsChanged"];
   onModeChanged?: ModelingEvents["onModeChanged"];
-  onModeledMethodsChanged?: ModelingEvents["onModeledMethodsChanged"];
-  onModifiedMethodsChanged?: ModelingEvents["onModifiedMethodsChanged"];
+  onModeledAndModifiedMethodsChanged?: ModelingEvents["onModeledAndModifiedMethodsChanged"];
   onInProgressMethodsChanged?: ModelingEvents["onInProgressMethodsChanged"];
   onProcessedByAutoModelMethodsChanged?: ModelingEvents["onProcessedByAutoModelMethodsChanged"];
   onRevealInModelEditor?: ModelingEvents["onRevealInModelEditor"];
@@ -34,8 +32,7 @@ export function createMockModelingEvents({
     onMethodsChanged,
     onHideModeledMethodsChanged,
     onModeChanged,
-    onModeledMethodsChanged,
-    onModifiedMethodsChanged,
+    onModeledAndModifiedMethodsChanged,
     onInProgressMethodsChanged,
     onProcessedByAutoModelMethodsChanged,
     onRevealInModelEditor,