From 4e70c8999f571ef68543bfb46bba3d76da60a221 Mon Sep 17 00:00:00 2001
From: Robert <robertbrignull@github.com>
Date: Mon, 14 Aug 2023 15:35:21 +0100
Subject: [PATCH] Only update the details view when data has changed

---
 .../model-details-data-provider.ts            | 23 +++++--
 .../model-details-data-provider.test.ts       | 66 +++++++++++++++++++
 2 files changed, 83 insertions(+), 6 deletions(-)
 create mode 100644 extensions/ql-vscode/test/vscode-tests/no-workspace/data-extensions-editor/model-details/model-details-data-provider.test.ts

diff --git a/extensions/ql-vscode/src/data-extensions-editor/model-details/model-details-data-provider.ts b/extensions/ql-vscode/src/data-extensions-editor/model-details/model-details-data-provider.ts
index abf59e1c4..bb2ab4324 100644
--- a/extensions/ql-vscode/src/data-extensions-editor/model-details/model-details-data-provider.ts
+++ b/extensions/ql-vscode/src/data-extensions-editor/model-details/model-details-data-provider.ts
@@ -34,16 +34,27 @@ export class ModelDetailsDataProvider
     return this.onDidChangeTreeDataEmitter.event;
   }
 
+  /**
+   * Update the data displayed in the tree view.
+   *
+   * Will only trigger an update if the data has changed. This relies on
+   * object identity, so be sure to not mutate the data passed to this
+   * method and instead always pass new objects/arrays.
+   */
   public async setState(
     externalApiUsages: ExternalApiUsage[],
     databaseItem: DatabaseItem,
   ): Promise<void> {
-    this.externalApiUsages = externalApiUsages;
-    this.databaseItem = databaseItem;
-    this.sourceLocationPrefix = await this.databaseItem.getSourceLocationPrefix(
-      this.cliServer,
-    );
-    this.onDidChangeTreeDataEmitter.fire();
+    if (
+      this.externalApiUsages !== externalApiUsages ||
+      this.databaseItem !== databaseItem
+    ) {
+      this.externalApiUsages = externalApiUsages;
+      this.databaseItem = databaseItem;
+      this.sourceLocationPrefix =
+        await this.databaseItem.getSourceLocationPrefix(this.cliServer);
+      this.onDidChangeTreeDataEmitter.fire();
+    }
   }
 
   getTreeItem(item: ModelDetailsTreeViewItem): TreeItem {
diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/data-extensions-editor/model-details/model-details-data-provider.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/data-extensions-editor/model-details/model-details-data-provider.test.ts
new file mode 100644
index 000000000..62a44e9ac
--- /dev/null
+++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/data-extensions-editor/model-details/model-details-data-provider.test.ts
@@ -0,0 +1,66 @@
+import { CodeQLCliServer } from "../../../../../src/codeql-cli/cli";
+import { ExternalApiUsage } from "../../../../../src/data-extensions-editor/external-api-usage";
+import { ModelDetailsDataProvider } from "../../../../../src/data-extensions-editor/model-details/model-details-data-provider";
+import { DatabaseItem } from "../../../../../src/databases/local-databases";
+import { mockedObject } from "../../../utils/mocking.helpers";
+
+describe("ModelDetailsDataProvider", () => {
+  const mockCliServer = mockedObject<CodeQLCliServer>({});
+
+  describe("setState", () => {
+    it("should emit onDidChangeTreeData event when state has changed", async () => {
+      const externalApiUsages: ExternalApiUsage[] = [];
+      const dbItem = mockedObject<DatabaseItem>({
+        getSourceLocationPrefix: () => "test",
+      });
+
+      const dataProvider = new ModelDetailsDataProvider(mockCliServer);
+      await dataProvider.setState(externalApiUsages, dbItem);
+
+      const onDidChangeTreeDataListener = jest.fn();
+      dataProvider.onDidChangeTreeData(onDidChangeTreeDataListener);
+
+      await dataProvider.setState(externalApiUsages, dbItem);
+
+      expect(onDidChangeTreeDataListener).toHaveBeenCalledTimes(0);
+    });
+
+    it("should emit onDidChangeTreeData event when externalApiUsages has changed", async () => {
+      const externalApiUsages1: ExternalApiUsage[] = [];
+      const externalApiUsages2: ExternalApiUsage[] = [];
+      const dbItem = mockedObject<DatabaseItem>({
+        getSourceLocationPrefix: () => "test",
+      });
+
+      const dataProvider = new ModelDetailsDataProvider(mockCliServer);
+      await dataProvider.setState(externalApiUsages1, dbItem);
+
+      const onDidChangeTreeDataListener = jest.fn();
+      dataProvider.onDidChangeTreeData(onDidChangeTreeDataListener);
+
+      await dataProvider.setState(externalApiUsages2, dbItem);
+
+      expect(onDidChangeTreeDataListener).toHaveBeenCalledTimes(1);
+    });
+
+    it("should emit onDidChangeTreeData event when dbItem has changed", async () => {
+      const externalApiUsages: ExternalApiUsage[] = [];
+      const dbItem1 = mockedObject<DatabaseItem>({
+        getSourceLocationPrefix: () => "test",
+      });
+      const dbItem2 = mockedObject<DatabaseItem>({
+        getSourceLocationPrefix: () => "test",
+      });
+
+      const dataProvider = new ModelDetailsDataProvider(mockCliServer);
+      await dataProvider.setState(externalApiUsages, dbItem1);
+
+      const onDidChangeTreeDataListener = jest.fn();
+      dataProvider.onDidChangeTreeData(onDidChangeTreeDataListener);
+
+      await dataProvider.setState(externalApiUsages, dbItem2);
+
+      expect(onDidChangeTreeDataListener).toHaveBeenCalledTimes(1);
+    });
+  });
+});