fix: only clear spatial edges after update

Do not clear spatial edges if the added data did not actually upade a cell.
mapillary · Nov 15, 2023 · c2552fb · c2552fb
1 parent 0dd4386
commit c2552fb
Show file tree

Hide file tree

Showing 2 changed files with 88 additions and 4 deletions.
diff --git a/src/graph/GraphService.ts b/src/graph/GraphService.ts
@@ -20,6 +20,7 @@ import {
     startWith,
     tap,
     takeLast,
+    withLatestFrom,
 } from "rxjs/operators";
 
 import { FilterFunction } from "./FilterCreator";
@@ -656,11 +657,15 @@ export class GraphService {
                 first(),
                 mergeMap(
                     graph => {
+                        return graph.updateCells$(event.cellIds);
+                    }),
+                withLatestFrom(this._graph$.pipe(first())),
+                tap(
+                    ([_, graph]) => {
                         graph.resetSpatialArea();
                         graph.resetSpatialEdges();
-                        return graph.updateCells$(event.cellIds);
                     }))
-            .subscribe(cellId => { this._dataAdded$.next(cellId); });
+            .subscribe(([cellId]) => { this._dataAdded$.next(cellId); });
     };
 
     private _onDataDeleted = (event: ProviderClusterEvent): void => {

diff --git a/test/graph/GraphService.test.ts b/test/graph/GraphService.test.ts
@@ -97,6 +97,8 @@ describe("GraphService.dataAdded$", () => {
         const updateCellsSpy = spyOn(graph, "updateCells$");
         updateCellsSpy.and.returnValue(observableOf("cellId"));
 
+        const resetSpatialAreaSpy =
+            spyOn(graph, "resetSpatialArea").and.stub();
         const resetSpatialEdgesSpy =
             spyOn(graph, "resetSpatialEdges").and.stub();
 
@@ -108,6 +110,7 @@ describe("GraphService.dataAdded$", () => {
                     expect(cellId).toBe("cellId");
 
                     expect(updateCellsSpy.calls.count()).toBe(1);
+                    expect(resetSpatialAreaSpy.calls.count()).toBe(1);
                     expect(resetSpatialEdgesSpy.calls.count()).toBe(1);
 
                     done();
@@ -120,7 +123,7 @@ describe("GraphService.dataAdded$", () => {
         });
     });
 
-    it("should reset spatial edges one time for multiple updated cells", (done: Function) => {
+    it("should reset spatial edges for each updated cell", (done: Function) => {
         const dataProvider = new DataProvider();
         const api = new APIWrapper(dataProvider);
         const graph = new Graph(api);
@@ -129,6 +132,8 @@ describe("GraphService.dataAdded$", () => {
         const cellIds = ["cellId1", "cellId2"];
         updateCellsSpy.and.returnValue(observableFrom(cellIds.slice()));
 
+        const resetSpatialAreaSpy =
+            spyOn(graph, "resetSpatialArea").and.stub();
         const resetSpatialEdgesSpy =
             spyOn(graph, "resetSpatialEdges").and.stub();
 
@@ -142,7 +147,9 @@ describe("GraphService.dataAdded$", () => {
                     expect(cellIds.includes(cellId)).toBe(true);
 
                     expect(updateCellsSpy.calls.count()).toBe(1);
-                    expect(resetSpatialEdgesSpy.calls.count()).toBe(1);
+
+                    expect(resetSpatialEdgesSpy.calls.count()).toBe(count);
+                    expect(resetSpatialAreaSpy.calls.count()).toBe(count);
 
                     if (count === 2) { done(); }
                 });
@@ -154,6 +161,78 @@ describe("GraphService.dataAdded$", () => {
             cellIds: cellIds.slice(),
         });
     });
+
+    it("should not reset spatial edges when no updated cells", () => {
+        const dataProvider = new DataProvider();
+        const api = new APIWrapper(dataProvider);
+        const graph = new Graph(api);
+
+        const updateCellsSpy = spyOn(graph, "updateCells$");
+        const cellIds = ["cellId1", "cellId2"];
+        updateCellsSpy.and.returnValue(observableFrom([]));
+
+        const resetSpatialAreaSpy =
+            spyOn(graph, "resetSpatialArea").and.stub();
+        const resetSpatialEdgesSpy =
+            spyOn(graph, "resetSpatialEdges").and.stub();
+
+        const graphService = new GraphService(graph);
+
+        graphService.dataAdded$
+            .subscribe(
+                (): void => {
+                    fail();
+                });
+
+        const type: ProviderEventType = "datacreate";
+        dataProvider.fire(type, {
+            type,
+            target: dataProvider,
+            cellIds: cellIds.slice(),
+        });
+
+        expect(updateCellsSpy.calls.count()).toBe(1);
+
+        expect(resetSpatialEdgesSpy.calls.count()).toBe(0);
+        expect(resetSpatialAreaSpy.calls.count()).toBe(0);
+    });
+
+    it("should only reset spatial edges each updated cells", (done: Function) => {
+        const dataProvider = new DataProvider();
+        const api = new APIWrapper(dataProvider);
+        const graph = new Graph(api);
+
+        const updateCellsSpy = spyOn(graph, "updateCells$");
+        const cellIds = ["cellId1", "cellId2"];
+        updateCellsSpy.and.returnValue(observableFrom([cellIds[1]]));
+
+        const resetSpatialAreaSpy =
+            spyOn(graph, "resetSpatialArea").and.stub();
+        const resetSpatialEdgesSpy =
+            spyOn(graph, "resetSpatialEdges").and.stub();
+
+        const graphService = new GraphService(graph);
+
+        graphService.dataAdded$
+            .subscribe(
+                (cellId): void => {
+                    expect(cellId).toBe(cellIds[1]);
+
+                    expect(updateCellsSpy.calls.count()).toBe(1);
+
+                    expect(resetSpatialEdgesSpy.calls.count()).toBe(1);
+                    expect(resetSpatialAreaSpy.calls.count()).toBe(1);
+
+                    done();
+                });
+
+        const type: ProviderEventType = "datacreate";
+        dataProvider.fire(type, {
+            type,
+            target: dataProvider,
+            cellIds: cellIds.slice(),
+        });
+    });
 });
 
 describe("GraphService.dataDeleted$", () => {