chore: read datalayer metadata from file if missing in DB (#2494)

When we introduced the DataLayer.settings property, we did not run a
migration for existing datalayers (because there are millions in the
French server).
Now that we simplified the `DataLayer.isLoaded()` logic in the client,
we want to be sure that created DataLayer on the client have full
metadata.

umap/models.py

@@ -523,17 +523,27 @@ class DataLayer(NamedModel):
     def metadata(self, request=None):
         # Retrocompat: minimal settings for maps not saved after settings property
         # has been introduced
-        obj = self.settings or {
+        metadata = self.settings
+        if not metadata:
+            # Fallback to file for old datalayers.
+            data = json.loads(self.geojson.read().decode())
+            metadata = data.get("_umap_options")
+            if not metadata:
+                metadata = {
                     "name": self.name,
                     "displayOnLoad": self.display_on_load,
                 }
+            # Save it to prevent file reading at each map load.
+            self.settings = metadata
+            # Do not update the modified_at.
+            self.save(update_fields=["settings"])
         if self.old_id:
-            obj["old_id"] = self.old_id
+            metadata["old_id"] = self.old_id
-        obj["id"] = self.pk
+        metadata["id"] = self.pk
-        obj["permissions"] = {"edit_status": self.edit_status}
+        metadata["permissions"] = {"edit_status": self.edit_status}
-        obj["editMode"] = "advanced" if self.can_edit(request) else "disabled"
+        metadata["editMode"] = "advanced" if self.can_edit(request) else "disabled"
-        obj["_referenceVersion"] = self.reference_version
+        metadata["_referenceVersion"] = self.reference_version
-        return obj
+        return metadata
 
     def clone(self, map_inst=None):
         new = self.__class__.objects.get(pk=self.pk)

umap/static/umap/js/modules/data/layer.js

@@ -45,8 +45,6 @@ export class DataLayer extends ServerStored {
     this._features = {}
     this._geojson = null
     this._propertiesIndex = []
-    this._loaded = false // Are layer metadata loaded
-    this._dataloaded = false // Are layer data loaded
 
     this._leafletMap = leafletMap
     this.parentPane = this._leafletMap.getPane('overlayPane')
@@ -85,6 +83,7 @@ export class DataLayer extends ServerStored {
     this.connectToMap()
     this.permissions = new DataLayerPermissions(this._umap, this)
 
+    this._needsFetch = this.createdOnServer
     if (!this.createdOnServer) {
       if (this.showAtLoad()) this.show()
     }
@@ -243,7 +242,7 @@ export class DataLayer extends ServerStored {
   }
 
   dataChanged() {
-    if (!this.hasDataLoaded()) return
+    if (!this.isLoaded()) return
     this._umap.onDataLayersChanged()
     this.layer.dataChanged()
   }
@@ -252,13 +251,13 @@ export class DataLayer extends ServerStored {
     if (!geojson) return []
     const features = this.addData(geojson, sync)
     this._geojson = geojson
+    this._needsFetch = false
     this.onDataLoaded()
     this.dataChanged()
     return features
   }
 
   onDataLoaded() {
-    this._dataloaded = true
     this.renderLegend()
   }
 
@@ -268,7 +267,6 @@ export class DataLayer extends ServerStored {
     if (geojson._umap_options) this.setOptions(geojson._umap_options)
     if (this.isRemoteLayer()) await this.fetchRemoteData()
     else this.fromGeoJSON(geojson, false)
-    this._loaded = true
   }
 
   clear() {
@@ -320,7 +318,7 @@ export class DataLayer extends ServerStored {
 
   async fetchRemoteData(force) {
     if (!this.isRemoteLayer()) return
-    if (!this.hasDynamicData() && this.hasDataLoaded() && !force) return
+    if (!this.hasDynamicData() && this.isLoaded() && !force) return
     if (!this.isVisible()) return
     // Keep non proxied url for later use in Alert.
     const remoteUrl = this._umap.renderUrl(this.options.remoteData.url)
@@ -345,11 +343,7 @@ export class DataLayer extends ServerStored {
   }
 
   isLoaded() {
-    return !this.createdOnServer || this._loaded
+    return !this._needsFetch
-  }
-
-  hasDataLoaded() {
-    return this._dataloaded
   }
 
   backupOptions() {
@@ -633,8 +627,6 @@ export class DataLayer extends ServerStored {
     this.propagateDelete()
     this._leaflet_events_bk = this._leaflet_events
     this.clear()
-    delete this._loaded
-    delete this._dataloaded
   }
 
   reset() {
@@ -652,7 +644,6 @@ export class DataLayer extends ServerStored {
     this.hide()
     if (this.isRemoteLayer()) this.fetchRemoteData()
     else if (this._geojson_bk) this.fromGeoJSON(this._geojson_bk)
-    this._loaded = true
     this.show()
     this.isDirty = false
   }
@@ -1108,9 +1099,7 @@ export class DataLayer extends ServerStored {
 
   async save() {
     if (this.isDeleted) return await this.saveDelete()
-    if (!this.isLoaded()) {
+    if (!this.isLoaded()) return
-      return
-    }
     const geojson = this.umapGeoJSON()
     const formData = new FormData()
     formData.append('name', this.options.name)
@@ -1172,7 +1161,6 @@ export class DataLayer extends ServerStored {
       this.backupOptions()
       this.backupData()
       this.connectToMap()
-      this._loaded = true
       this.redraw() // Needed for reordering features
       return true
     }

umap/static/umap/js/modules/rendering/layers/classified.js

@@ -75,7 +75,7 @@ const ClassifiedMixin = {
   },
 
   renderLegend: function (container) {
-    if (!this.datalayer.hasDataLoaded()) return
+    if (!this.datalayer.isLoaded()) return
     const parent = DomUtil.create('ul', '', container)
     const items = this.getLegendItems()
     for (const [color, label] of items) {

umap/static/umap/js/modules/sync/updaters.js

@@ -55,15 +55,11 @@ export class DataLayerUpdater extends BaseUpdater {
   upsert({ value }) {
     // Upsert only happens when a new datalayer is created.
     const datalayer = this._umap.createDataLayer(value, false)
-    // Prevent the layer to get data from the server, as it will get it
-    // from the sync.
-    datalayer._loaded = true
   }
 
   update({ key, metadata, value }) {
     const datalayer = this.getDataLayerFromID(metadata.id)
     if (fieldInSchema(key)) {
-      datalayer._loaded = true
       this.updateObjectValue(datalayer, key, value)
     } else {
       console.debug(

umap/tests/integration/test_websocket_sync.py

@@ -398,7 +398,9 @@ def test_should_sync_datalayers(new_page, asgi_live_server, tilelayer):
     peerA.locator("#map").click()
 
     # Make sure this new marker is in Layer 2 for peerB
-    expect(peerB.get_by_text("Layer 2")).to_be_visible()
+    # Show features for this layer in the brower.
+    peerB.get_by_role("heading", name="Layer 2").locator(".datalayer-name").click()
+    expect(peerB.locator("li").filter(has_text="Layer 2")).to_be_visible()
     peerB.locator(".panel.left").get_by_role("button", name="Show/hide layer").nth(
         1
     ).click()