PR Review

revert back to a bulk update on GeoJSON data initial load
manage a removal of layer by index with auto-reload intervals

Author: pakb

src/store/plugins/load-geojson-style-and-data.plugin.js

@@ -39,8 +39,10 @@ async function autoReloadData(store, geoJsonLayer) {
             const { data } = await load(geoJsonLayer.geoJsonUrl)
             const layerCopy = geoJsonLayer.clone()
             layerCopy.geoJsonData = data
-            await store.dispatch('updateLayer', {
-                layer: layerCopy,
+            // we update through the action updateLayers, so that if multiple copies of the same GeoJSON layer are present,
+            // they will all be updated with the fresh data
+            store.dispatch('updateLayers', {
+                layers: [layerCopy],
                 ...dispatcher,
             })
             log.debug(`Data reloaded according to updateDelay for layer ${geoJsonLayer.id}`)
@@ -51,11 +53,18 @@ async function autoReloadData(store, geoJsonLayer) {
     }, geoJsonLayer.updateDelay)
 }
 
+function clearAutoReload(layerId) {
+    log.debug(`Removing auto-reload of data for layer ${layerId} as it was removed from the map`)
+    clearInterval(intervalsByLayerId[layerId])
+    delete intervalsByLayerId[layerId]
+}
+
 /**
  * @param {GeoAdminGeoJsonLayer} geoJsonLayer
  * @returns {Promise<GeoAdminGeoJsonLayer>}
  */
-async function loadDataAndStyle(store, geoJsonLayer) {
+async function loadDataAndStyle(geoJsonLayer) {
+    log.debug(`Loading data/style for added GeoJSON layer`, geoJsonLayer)
     const clone = geoJsonLayer.clone()
     try {
         const [{ data: style }, { data }] = await Promise.all([
@@ -68,19 +77,13 @@ async function loadDataAndStyle(store, geoJsonLayer) {
         clone.geoJsonData = data
         clone.geoJsonStyle = style
         clone.isLoading = false
-        await store.dispatch('updateLayer', {
-            layer: clone,
-            ...dispatcher,
-        })
+        return clone
     } catch (error) {
         log.error(`Error while fetching GeoJSON data/style for layer ${geoJsonLayer?.id}`, error)
         clone.isLoading = false
         clone.errorKey = 'loading_error_network_failure'
         clone.hasError = true
-        await store.dispatch('updateLayer', {
-            layer: clone,
-            ...dispatcher,
-        })
+        return clone
     }
 }
 
@@ -92,18 +95,34 @@ async function loadDataAndStyle(store, geoJsonLayer) {
  */
 export default function loadGeojsonStyleAndData(store) {
     store.subscribe((mutation) => {
-        const addLayersSubscriber = (layers) => {
-            layers
+        const addLayersSubscriber = async (layers) => {
+            const geoJsonLayers = layers
                 .filter((layer) => layer instanceof GeoAdminGeoJsonLayer)
+                // filtering out multiple active layer entries for the same GeoJSON data
+                // (only one request to get the data is necessary for all entries)
+                .filter(
+                    (geoJsonLayer, index, self) =>
+                        // checking that the index of the first layer matching our ID is the same index as the layer
+                        // we are currently looping through, filtering it out otherwise (it's a duplicate)
+                        self.indexOf(self.find((layer) => layer.id === geoJsonLayer.id)) === index
+                )
+
+            const updatedLayers = await Promise.all(
+                geoJsonLayers
+                    .filter((layer) => layer.isLoading)
+                    .map((layer) => loadDataAndStyle(layer))
+            )
+            if (updatedLayers.length > 0) {
+                store.dispatch('updateLayers', { layers: updatedLayers, ...dispatcher })
+            }
+
+            // after the initial load is done,
+            // going through all layers that have an update delay and launching the routine to reload their data
+            geoJsonLayers
+                .filter((layer) => layer.updateDelay > 0)
                 .forEach((layer) => {
-                    if (layer.isLoading) {
-                        log.debug(`Loading data/style for added GeoJSON layer`, layer)
-                        loadDataAndStyle(store, layer)
-                    }
-                    if (layer.updateDelay > 0) {
-                        log.debug('starting auto-reload of data for layer', layer)
-                        autoReloadData(store, layer)
-                    }
+                    log.debug('starting auto-reload of data for layer', layer)
+                    autoReloadData(store, layer)
                 })
         }
         if (mutation.type === 'addLayer') {
@@ -113,12 +132,19 @@ export default function loadGeojsonStyleAndData(store) {
             addLayersSubscriber(mutation.payload.layers)
         }
         if (mutation.type === 'removeLayerWithId' && intervalsByLayerId[mutation.payload.layerId]) {
-            log.debug(
-                `Removing auto-reload of data for layer ${mutation.payload.layerId} as it was removed from the map`
-            )
             // when a layer is removed, if a matching interval is found, we clear it
-            clearInterval(intervalsByLayerId[mutation.payload.layerId])
-            delete intervalsByLayerId[mutation.payload.layerId]
+            clearAutoReload(mutation.payload.layerId)
+        }
+        if (mutation.type === 'removeLayerByIndex') {
+            // As we come after the work has been done,
+            // we cannot get the layer ID removed from the store from the mutation's payload.
+            // So we instead go through all intervals, and clear any that has no matching layer in the active layers
+            Object.keys(intervalsByLayerId)
+                .filter(
+                    (layerId) =>
+                        !store.state.layers.activeLayers.some((layer) => layer.id === layerId)
+                )
+                .forEach((layerId) => clearAutoReload(layerId))
         }
     })
 }