Improve performance of selecting scripts

Increase performance by only notifying GUI about changes in selection
when there really is a change. It removes extra processing from all
event listeners that act on selection state change.

src/application/Context/State/Selection/UserSelection.ts

@@ -109,6 +109,9 @@ export class UserSelection implements IUserSelection {
       .getAllScripts()
       .filter((script) => !this.scripts.exists(script.id))
       .map((script) => new SelectedScript(script, false));
+    if (scriptsToSelect.length === 0) {
+      return;
+    }
     for (const script of scriptsToSelect) {
       this.scripts.addItem(script);
     }
@@ -116,6 +119,9 @@ export class UserSelection implements IUserSelection {
   }
 
   public deselectAll(): void {
+    if (this.scripts.length === 0) {
+      return;
+    }
     const selectedScriptIds = this.scripts.getItems().map((script) => script.id);
     for (const scriptId of selectedScriptIds) {
       this.scripts.removeItem(scriptId);
@@ -127,20 +133,35 @@ export class UserSelection implements IUserSelection {
     if (!scripts || scripts.length === 0) {
       throw new Error('Scripts are empty. Use deselectAll() if you want to deselect everything');
     }
-    // Unselect from selected scripts
+    let totalChanged = 0;
-    if (this.scripts.length !== 0) {
+    totalChanged += this.unselectMissingWithoutNotifying(scripts);
-      this.scripts.getItems()
+    totalChanged += this.selectNewWithoutNotifying(scripts);
-        .filter((existing) => !scripts.some((script) => existing.id === script.id))
+    if (totalChanged > 0) {
-        .map((script) => script.id)
+      this.changed.notify(this.scripts.getItems());
-        .forEach((scriptId) => this.scripts.removeItem(scriptId));
     }
-    // Select from unselected scripts
+  }
+
+  private unselectMissingWithoutNotifying(scripts: readonly IScript[]): number {
+    if (this.scripts.length === 0 || scripts.length === 0) {
+      return 0;
+    }
+    const existingItems = this.scripts.getItems();
+    const missingIds = existingItems
+      .filter((existing) => !scripts.some((script) => existing.id === script.id))
+      .map((script) => script.id);
+    for (const id of missingIds) {
+      this.scripts.removeItem(id);
+    }
+    return missingIds.length;
+  }
+
+  private selectNewWithoutNotifying(scripts: readonly IScript[]): number {
     const unselectedScripts = scripts
       .filter((script) => !this.scripts.exists(script.id))
       .map((script) => new SelectedScript(script, false));
-    for (const toSelect of unselectedScripts) {
+    for (const newScript of unselectedScripts) {
-      this.scripts.addItem(toSelect);
+      this.scripts.addItem(newScript);
     }
-    this.changed.notify(this.scripts.getItems());
+    return unselectedScripts.length;
   }
 }

tests/unit/application/Context/State/Selection/UserSelection.spec.ts

@@ -38,72 +38,134 @@ describe('UserSelection', () => {
         .expectFinalScripts(scripts);
     });
   });
-  describe('deselectAll removes all items', () => {
+  describe('deselectAll', () => {
-    // arrange
+    describe('removes existing items', () => {
-    const allScripts = [
+      // arrange
-      new SelectedScriptStub('s1', false),
+      const allScripts = [
-      new SelectedScriptStub('s2', false),
+        new SelectedScriptStub('s1', false),
-      new SelectedScriptStub('s3', false),
+        new SelectedScriptStub('s2', false),
-      new SelectedScriptStub('s4', false),
+        new SelectedScriptStub('s3', false),
-    ];
+        new SelectedScriptStub('s4', false),
-    const selectedScripts = allScripts.filter(
+      ];
-      (s) => ['s1', 's2', 's3'].includes(s.id),
+      const selectedScripts = allScripts.filter(
-    );
+        (s) => ['s1', 's2', 's3'].includes(s.id),
-    new UserSelectionTestRunner()
+      );
-      .withSelectedScripts(selectedScripts)
+      new UserSelectionTestRunner()
-      .withCategory(1, allScripts.map((s) => s.script))
+        .withSelectedScripts(selectedScripts)
-    // act
+        .withCategory(1, allScripts.map((s) => s.script))
-      .run((sut) => {
+      // act
-        sut.deselectAll();
+        .run((sut) => {
-      })
+          sut.deselectAll();
-    // assert
+        })
-      .expectTotalFiredEvents(1)
+      // assert
-      .expectFinalScripts([])
+        .expectTotalFiredEvents(1)
-      .expectFinalScriptsInEvent(0, []);
+        .expectFinalScripts([])
+        .expectFinalScriptsInEvent(0, []);
+    });
+    describe('does not notify if nothing is selected', () => {
+      new UserSelectionTestRunner()
+      // arrange
+        .withSelectedScripts([])
+      // act
+        .run((sut) => {
+          sut.deselectAll();
+        })
+      // assert
+        .expectTotalFiredEvents(0);
+    });
   });
-  describe('selectOnly selects expected', () => {
+  describe('selectAll', () => {
-    // arrange
+    describe('selects as expected', () => {
-    const allScripts = [
+      // arrange
-      new SelectedScriptStub('s1', false),
+      const expected = [
-      new SelectedScriptStub('s2', false),
+        new SelectedScriptStub('s1', false),
-      new SelectedScriptStub('s3', false),
+        new SelectedScriptStub('s2', false),
-      new SelectedScriptStub('s4', false),
+      ];
-    ];
+      new UserSelectionTestRunner()
-    const selectedScripts = allScripts.filter(
+        .withSelectedScripts([])
-      (s) => ['s1', 's2', 's3'].includes(s.id),
+        .withCategory(1, expected.map((s) => s.script))
-    );
+      // act
-    const scriptsToSelect = allScripts.filter(
+        .run((sut) => {
-      (s) => ['s2', 's3', 's4'].includes(s.id),
+          sut.selectAll();
-    );
+        })
-    new UserSelectionTestRunner()
+      // assert
-      .withSelectedScripts(selectedScripts)
+        .expectTotalFiredEvents(1)
-      .withCategory(1, allScripts.map((s) => s.script))
+        .expectFinalScripts(expected)
-    // act
+        .expectFinalScriptsInEvent(0, expected);
-      .run((sut) => {
+    });
-        sut.selectOnly(scriptsToSelect.map((s) => s.script));
+    describe('does not notify if nothing new is selected', () => {
-      })
+      const allScripts = [new ScriptStub('s1'), new ScriptStub('s2')];
-    // assert
+      const selectedScripts = allScripts.map((s) => s.toSelectedScript(false));
-      .expectTotalFiredEvents(1)
+      new UserSelectionTestRunner()
-      .expectFinalScripts(scriptsToSelect)
+      // arrange
-      .expectFinalScriptsInEvent(0, scriptsToSelect);
+        .withSelectedScripts(selectedScripts)
+        .withCategory(0, allScripts)
+      // act
+        .run((sut) => {
+          sut.selectAll();
+        })
+      // assert
+        .expectTotalFiredEvents(0);
+    });
   });
-  describe('selectAll selects as expected', () => {
+  describe('selectOnly', () => {
-    // arrange
+    describe('selects as expected', () => {
-    const expected = [
+      // arrange
-      new SelectedScriptStub('s1', false),
+      const allScripts = [
-      new SelectedScriptStub('s2', false),
+        new SelectedScriptStub('s1', false),
-    ];
+        new SelectedScriptStub('s2', false),
-    new UserSelectionTestRunner()
+        new SelectedScriptStub('s3', false),
-      .withSelectedScripts([])
+        new SelectedScriptStub('s4', false),
-      .withCategory(1, expected.map((s) => s.script))
+      ];
-    // act
+      const getScripts = (...ids: string[]) => allScripts.filter((s) => ids.includes(s.id));
-      .run((sut) => {
+      const testCases = [
-        sut.selectAll();
+        {
-      })
+          name: 'adds as expected',
-    // assert
+          preSelected: getScripts('s1'),
-      .expectTotalFiredEvents(1)
+          toSelect: getScripts('s1', 's2'),
-      .expectFinalScripts(expected)
+        },
-      .expectFinalScriptsInEvent(0, expected);
+        {
+          name: 'removes as expected',
+          preSelected: getScripts('s1', 's2', 's3'),
+          toSelect: getScripts('s1'),
+        },
+        {
+          name: 'adds and removes as expected',
+          preSelected: getScripts('s1', 's2', 's3'),
+          toSelect: getScripts('s2', 's3', 's4'),
+        },
+      ];
+      for (const testCase of testCases) {
+        describe(testCase.name, () => {
+          new UserSelectionTestRunner()
+            .withSelectedScripts(testCase.preSelected)
+            .withCategory(1, testCase.toSelect.map((s) => s.script))
+          // act
+            .run((sut) => {
+              sut.selectOnly(testCase.toSelect.map((s) => s.script));
+            })
+          // assert
+            .expectTotalFiredEvents(1)
+            .expectFinalScripts(testCase.toSelect)
+            .expectFinalScriptsInEvent(0, testCase.toSelect);
+        });
+      }
+    });
+    describe('does not notify if selection does not change', () => {
+      const allScripts = [new ScriptStub('s1'), new ScriptStub('s2'), new ScriptStub('s3')];
+      const toSelect = [allScripts[0], allScripts[1]];
+      const preSelected = toSelect.map((s) => s.toSelectedScript(false));
+      new UserSelectionTestRunner()
+      // arrange
+        .withSelectedScripts(preSelected)
+        .withCategory(0, allScripts)
+      // act
+        .run((sut) => {
+          sut.selectOnly(toSelect);
+        })
+      // assert
+        .expectTotalFiredEvents(0);
+    });
   });
   describe('addOrUpdateSelectedScript', () => {
     describe('adds when item does not exist', () => {