Refactor remove layer

Signed-off-by: Vijayan Balasubramanian <balasvij@amazon.com>

public/components/layer_control_panel/layer_control_panel.tsx

@@ -48,6 +48,7 @@ import {
 } from '../../model/layerRenderController';
 import { MapState } from '../../model/mapState';
 import { ConfigSchema } from '../../../common/config';
+import {moveLayers} from "../../model/map/layer_operations";
 
 interface MaplibreRef {
   current: Maplibre | null;
@@ -241,7 +242,8 @@ export const LayerControlPanel = memo(
 
         const currentMaplibreLayerId = layers[prevIndex].id;
         const beforeMaplibreLayerId = beforeMaplibreLayerID(prevIndex, newIndex);
-        LayerActions.move(maplibreRef, currentMaplibreLayerId, beforeMaplibreLayerId);
+
+        moveLayers(maplibreRef.current!, currentMaplibreLayerId, beforeMaplibreLayerId);
 
         // update map layers
         const layersClone = [...layers];

public/model/documentLayerFunctions.ts

@@ -11,7 +11,7 @@ import { getMaplibreBeforeLayerId} from './layersFunctions';
 import {
   addCircleLayer,
   addLineLayer,
-  addPolygonLayer, hasLayer,
+  addPolygonLayer, hasLayer, removeLayers,
   updateCircleLayer,
   updateLineLayer,
   updatePolygonLayer,
@@ -237,12 +237,7 @@ export const DocumentLayerFunctions = {
     }
   },
   remove: (maplibreRef: MaplibreRef, layerConfig: DocumentLayerSpecification) => {
-    const layers = getCurrentStyleLayers(maplibreRef);
-    layers.forEach((layer: { id: any }) => {
-      if (layer.id.includes(layerConfig.id)) {
-        maplibreRef.current?.removeLayer(layer.id);
-      }
-    });
+    removeLayers(maplibreRef.current!, layerConfig.id, true);
   },
   hide: (maplibreRef: MaplibreRef, layerConfig: DocumentLayerSpecification) => {
     const layers = getCurrentStyleLayers(maplibreRef);

public/model/layersFunctions.ts

@@ -23,26 +23,6 @@ interface MaplibreRef {
   current: Maplibre | null;
 }
 
-export const LayerActions = {
-  move: (maplibreRef: MaplibreRef, sourceId: string, beforeId?: string) => {
-    const sourceMaplibreLayers = getLayers(maplibreRef.current!, sourceId);
-    if (!sourceMaplibreLayers) {
-      return;
-    }
-    const beforeMaplibreLayers = getLayers(maplibreRef.current!, beforeId);
-    if (!beforeMaplibreLayers || beforeMaplibreLayers.length < 1) {
-      // move to top
-      sourceMaplibreLayers.forEach((layer) => maplibreRef.current?.moveLayer(layer.id));
-      return;
-    }
-    const topOfBeforeLayer = beforeMaplibreLayers[0];
-    sourceMaplibreLayers.forEach((layer) =>
-      maplibreRef.current?.moveLayer(layer.id, topOfBeforeLayer.id)
-    );
-    return;
-  },
-};
-
 export const layersFunctionMap: { [key: string]: any } = {
   [DASHBOARDS_MAPS_LAYER_TYPE.OPENSEARCH_MAP]: OSMLayerFunctions,
   [DASHBOARDS_MAPS_LAYER_TYPE.DOCUMENTS]: DocumentLayerFunctions,

public/model/map/__mocks__/map.ts

@@ -6,17 +6,37 @@
 import { LayerSpecification } from 'maplibre-gl';
 import { MockLayer } from './layer';
 
+export type Source = any;
+
 export class MockMaplibreMap {
   private _styles: {
     layers: MockLayer[];
+    sources: Map<string, Source>;
   };
 
   constructor(layers: MockLayer[]) {
     this._styles = {
       layers: new Array<MockLayer>(...layers),
+      sources: new Map<string, Source>(),
     };
   }
 
+  public addSource(sourceId: string, source: Source) {
+    this._styles.sources.set(sourceId, source);
+  }
+
+  public getSource(sourceId: string): string | undefined {
+    return this._styles.sources.get(sourceId);
+  }
+
+  public removeSource(sourceId: string) {
+    this._styles.sources.delete(sourceId);
+  }
+
+  public getLayers(): MockLayer[] {
+    return [...this._styles.layers];
+  }
+
   getLayer(id: string): MockLayer[] {
     return this._styles.layers.filter((layer) => (layer.getProperty('id') as string).includes(id));
   }
@@ -66,4 +86,27 @@ export class MockMaplibreMap {
     });
     this._styles.layers.splice(beforeLayerIndex, 0, layer);
   }
+
+  private move(fromIndex: number, toIndex: number) {
+    const element = this._styles.layers[fromIndex];
+    this._styles.layers.splice(fromIndex, 1);
+    this._styles.layers.splice(toIndex, 0, element);
+  }
+
+  moveLayer(layerId: string, beforeId?: string) {
+    if (layerId === beforeId) {
+      return;
+    }
+    const fromIndex: number = this._styles.layers.indexOf(this.getLayer(layerId)[0]);
+    const toIndex: number = beforeId
+      ? this._styles.layers.indexOf(this.getLayer(beforeId)[0])
+      : this._styles.layers.length;
+    this.move(fromIndex, toIndex);
+  }
+
+  removeLayer(layerId: string) {
+    this._styles.layers = this.getLayers().filter(
+      (layer) => !(layer.getProperty('id') as string).includes(layerId)
+    );
+  }
 }

public/model/map/layer_operations.test.ts

@@ -6,7 +6,8 @@ import {
   addCircleLayer,
   addLineLayer,
   addPolygonLayer,
-  getLayers, hasLayer,
+  getLayers,
+  hasLayer, moveLayers, removeLayers,
   updateCircleLayer,
   updateLineLayer,
   updatePolygonLayer,
@@ -301,3 +302,60 @@ describe('get layer', () => {
     expect(hasLayer((mockMap as unknown) as Maplibre, 'layer-1')).toBe(true);
   });
 });
+
+describe('move layer', () => {
+  it('should move to top', function () {
+    const mockLayer1: MockLayer = new MockLayer('layer-1');
+    const mockLayer2: MockLayer = new MockLayer('layer-11');
+    const mockLayer3: MockLayer = new MockLayer('layer-2');
+    const mockMap: MockMaplibreMap = new MockMaplibreMap([mockLayer1, mockLayer2, mockLayer3]);
+    moveLayers((mockMap as unknown) as Maplibre, 'layer-1');
+    const reorderedLayer: string[] = mockMap.getLayers().map((layer) => layer.getProperty('id'));
+    expect(reorderedLayer).toEqual(['layer-2', 'layer-1', 'layer-11']);
+  });
+  it('should move before middle layer', function () {
+    const mockLayer1: MockLayer = new MockLayer('layer-1');
+    const mockLayer2: MockLayer = new MockLayer('layer-2');
+    const mockLayer3: MockLayer = new MockLayer('layer-3');
+    const mockMap: MockMaplibreMap = new MockMaplibreMap([mockLayer1, mockLayer2, mockLayer3]);
+    moveLayers((mockMap as unknown) as Maplibre, 'layer-1', 'layer-2');
+    const reorderedLayer: string[] = mockMap.getLayers().map((layer) => layer.getProperty('id'));
+    expect(reorderedLayer).toEqual(['layer-2', 'layer-1', 'layer-3']);
+  });
+  it('should not move if no layer is matched', function () {
+    const mockLayer1: MockLayer = new MockLayer('layer-1');
+    const mockLayer2: MockLayer = new MockLayer('layer-2');
+    const mockLayer3: MockLayer = new MockLayer('layer-3');
+    const mockMap: MockMaplibreMap = new MockMaplibreMap([mockLayer1, mockLayer2, mockLayer3]);
+    moveLayers((mockMap as unknown) as Maplibre, 'layer-4', 'layer-2');
+    const reorderedLayer: string[] = mockMap.getLayers().map((layer) => layer.getProperty('id'));
+    expect(reorderedLayer).toEqual(['layer-1', 'layer-2', 'layer-3']);
+  });
+});
+
+describe('delete layer', function () {
+  it('should delete layer without source', function () {
+    const mockLayer1: MockLayer = new MockLayer('layer-1');
+    const mockLayer2: MockLayer = new MockLayer('layer-11');
+    const mockLayer3: MockLayer = new MockLayer('layer-2');
+    const mockMap: MockMaplibreMap = new MockMaplibreMap([mockLayer1, mockLayer2, mockLayer3]);
+    mockMap.addSource('layer-1', 'geojson');
+    removeLayers((mockMap as unknown) as Maplibre, 'layer-1');
+    expect(mockMap.getLayers().length).toBe(1);
+    expect(mockMap.getSource('layer-1')).toBeDefined();
+    expect(mockMap.getLayers()[0].getProperty('id')).toBe('layer-2');
+  });
+  it('should delete layer with source', function () {
+    const mockLayer1: MockLayer = new MockLayer('layer-1');
+    const mockLayer2: MockLayer = new MockLayer('layer-11');
+    const mockLayer3: MockLayer = new MockLayer('layer-2');
+    const mockMap: MockMaplibreMap = new MockMaplibreMap([mockLayer1, mockLayer2, mockLayer3]);
+    mockMap.addSource('layer-2', 'geojson');
+    removeLayers((mockMap as unknown) as Maplibre, 'layer-2', true);
+    expect(mockMap.getLayers().length).toBe(2);
+    expect(mockMap.getSource('layer-2')).toBeUndefined();
+    expect(
+      mockMap.getLayers().filter((layer) => String(layer?.getProperty('id')) === 'layer-2')
+    ).toEqual([]);
+  });
+});

public/model/map/layer_operations.ts

@@ -21,6 +21,27 @@ export const hasLayer = (map: Maplibre, dashboardMapsLayerId: string) => {
   return false;
 };
 
+export const moveLayers = (map: Maplibre, sourceId: string, beforeId?: string) => {
+  const sourceLayers = getLayers(map, sourceId);
+  if (!sourceLayers.length) {
+    return;
+  }
+  const beforeLayers = beforeId ? getLayers(map, beforeId) : [];
+  const topOfBeforeLayer = beforeLayers.length ? beforeLayers[0].id : undefined;
+  sourceLayers.forEach((layer) => map?.moveLayer(layer.id, topOfBeforeLayer));
+  return;
+};
+
+export const removeLayers = (map: Maplibre, layerId: string, removeSource?: boolean) => {
+  getLayers(map, layerId).forEach((layer) => {
+    map.removeLayer(layer.id);
+  });
+  // client might remove source if it is used anymore.
+  if (removeSource && map.getSource(layerId)) {
+    map.removeSource(layerId);
+  }
+};
+
 export interface LineLayerSpecification {
   sourceId: string;
   visibility: string;