feat(sync): Let the clients set layers UUID

This make it possible to synchronize datalayers before their creation on
the server, allowing at the same time to solve issues related to them
not being saved (e.g. duplication of geometries)

We use the DataLayer._referenceVersion to track if a DataLayer has been
saved on the server.

After a save, the _referenceVersion is synched with other peers.

To pass the reference version from the server to the frontend, we have two
options:
- use a header
- populate the `options.version` field

In the case of a GET on a Datalayer, we could not use the `options` scenario
because:
- the value in the file is not up to date (it was the value the client has
 before the save)
- the python cannot change it on the fly, as the file is served by nginx

So we decided to keep using a header. But on the map view, we load all
datalayers metadatas in the map options, so here we cannot use the header
scenario, so in this specific case we had to populate `options._referenceVersion`.

At the same time, we also changed:
- Umap.options.umap_id => Umap.id
- DataLayer.umap_id => Datalayer.id
- fixed the version number returned by DataLayer.version_metadata

umap/migrations/0023_alter_datalayer_uuid.py

@@ -0,0 +1,19 @@
+# Generated by Django 5.1.2 on 2024-11-15 11:03
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("umap", "0022_add_team"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="datalayer",
+            name="uuid",
+            field=models.UUIDField(
+                editable=False, primary_key=True, serialize=False, unique=True
+            ),
+        ),
+    ]

umap/models.py

@@ -250,7 +250,7 @@ class Map(NamedModel):
                 "hash": False,
                 "scrollWheelZoom": False,
                 "noControl": True,
-                "umap_id": self.pk,
+                "id": self.pk,
                 "schema": self.extra_schema,
                 "slideshow": {},
             }
@@ -444,9 +444,7 @@ class DataLayer(NamedModel):
         (COLLABORATORS, _("Editors and team only")),
         (OWNER, _("Owner only")),
     )
-    uuid = models.UUIDField(
+    uuid = models.UUIDField(unique=True, primary_key=True, editable=False)
-        unique=True, primary_key=True, default=uuid.uuid4, editable=False
-    )
     old_id = models.IntegerField(null=True, blank=True)
     map = models.ForeignKey(Map, on_delete=models.CASCADE)
     description = models.TextField(blank=True, null=True, verbose_name=_("description"))
@@ -525,12 +523,13 @@ class DataLayer(NamedModel):
         obj["id"] = self.pk
         obj["permissions"] = {"edit_status": self.edit_status}
         obj["editMode"] = "advanced" if self.can_edit(request) else "disabled"
+        obj["_referenceVersion"] = self.reference_version
         return obj
 
     def clone(self, map_inst=None):
         new = self.__class__.objects.get(pk=self.pk)
         new._state.adding = True
-        new.pk = None
+        new.pk = uuid.uuid4()
         if map_inst:
             new.map = map_inst
         new.geojson = File(new.geojson.file.file)
@@ -543,11 +542,20 @@ class DataLayer(NamedModel):
             valid_prefixes.append(name.startswith("%s_" % self.old_id))
         return any(valid_prefixes) and name.endswith(".geojson")
 
+    def extract_version_number(self, path):
+        version = path.split(".")[0]
+        if "_" in version:
+            return version.split("_")[-1]
+        return version
+
+    @property
+    def reference_version(self):
+        return self.extract_version_number(self.geojson.path)
+
     def version_metadata(self, name):
-        els = name.split(".")[0].split("_")
         return {
             "name": name,
-            "at": els[1],
+            "at": self.extract_version_number(name),
             "size": self.geojson.storage.size(self.get_version_path(name)),
         }
 

umap/static/umap/js/modules/data/features.js

@@ -136,7 +136,7 @@ class Feature {
       subject: 'feature',
       metadata: {
         id: this.id,
-        layerId: this.datalayer?.umap_id || null,
+        layerId: this.datalayer.id,
         featureType: this.getClassName(),
       },
     }

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

@@ -38,7 +38,7 @@ const LAYER_MAP = LAYER_TYPES.reduce((acc, klass) => {
 }, {})
 
 export class DataLayer extends ServerStored {
-  constructor(umap, leafletMap, data) {
+  constructor(umap, leafletMap, data = {}) {
     super()
     this._umap = umap
     this.sync = umap.sync_engine.proxy(this)
@@ -51,10 +51,7 @@ export class DataLayer extends ServerStored {
 
     this._leafletMap = leafletMap
     this.parentPane = this._leafletMap.getPane('overlayPane')
-    this.pane = this._leafletMap.createPane(
+    this.pane = this._leafletMap.createPane(`datalayer${stamp(this)}`, this.parentPane)
-      `datalayer${stamp(this)}`,
-      this.parentPane
-    )
     this.pane.dataset.id = stamp(this)
     // FIXME: should be on layer
     this.renderer = L.svg({ pane: this.pane })
@@ -66,7 +63,11 @@ export class DataLayer extends ServerStored {
     }
 
     this._isDeleted = false
-    this.setUmapId(data.id)
+    this._referenceVersion = data._referenceVersion
+    // Do not save it later.
+    delete data._referenceVersion
+    data.id = data.id || crypto.randomUUID()
+
     this.setOptions(data)
 
     if (!Utils.isObject(this.options.remoteData)) {
@@ -84,7 +85,8 @@ export class DataLayer extends ServerStored {
     this.backupOptions()
     this.connectToMap()
     this.permissions = new DataLayerPermissions(this._umap, this)
-    if (!this.umap_id) {
+
+    if (!this.createdOnServer) {
       if (this.showAtLoad()) this.show()
       this.isDirty = true
     }
@@ -95,6 +97,14 @@ export class DataLayer extends ServerStored {
     if (this.isVisible()) this.propagateShow()
   }
 
+  get id() {
+    return this.options.id
+  }
+
+  get createdOnServer() {
+    return Boolean(this._referenceVersion)
+  }
+
   onDirty(status) {
     if (status) {
       // A layer can be made dirty by indirect action (like dragging layers)
@@ -121,9 +131,7 @@ export class DataLayer extends ServerStored {
   getSyncMetadata() {
     return {
       subject: 'datalayer',
-      metadata: {
+      metadata: { id: this.id },
-        id: this.umap_id || null,
-      },
     }
   }
 
@@ -160,7 +168,7 @@ export class DataLayer extends ServerStored {
   autoLoaded() {
     if (!this._umap.datalayersFromQueryString) return this.options.displayOnLoad
     const datalayerIds = this._umap.datalayersFromQueryString
-    let loadMe = datalayerIds.includes(this.umap_id.toString())
+    let loadMe = datalayerIds.includes(this.id.toString())
     if (this.options.old_id) {
       loadMe = loadMe || datalayerIds.includes(this.options.old_id.toString())
     }
@@ -215,13 +223,13 @@ export class DataLayer extends ServerStored {
   }
 
   async fetchData() {
-    if (!this.umap_id) return
+    if (!this.createdOnServer) return
     if (this._loading) return
     this._loading = true
     const [geojson, response, error] = await this._umap.server.get(this._dataUrl())
     if (!error) {
-      this._reference_version = response.headers.get('X-Datalayer-Version')
+      this.setReferenceVersion({ response, sync: false })
-      // FIXME: for now this property is set dynamically from backend
+      // FIXME: for now the _umap_options property is set dynamically from backend
       // And thus it's not in the geojson file in the server
       // So do not let all options to be reset
       // Fix is a proper migration so all datalayers settings are
@@ -257,6 +265,7 @@ export class DataLayer extends ServerStored {
 
   async fromUmapGeoJSON(geojson) {
     if (geojson._storage) geojson._umap_options = geojson._storage // Retrocompat
+    geojson._umap_options.id = this.id
     if (geojson._umap_options) this.setOptions(geojson._umap_options)
     if (this.isRemoteLayer()) await this.fetchRemoteData()
     else this.fromGeoJSON(geojson, false)
@@ -313,18 +322,13 @@ export class DataLayer extends ServerStored {
   }
 
   isLoaded() {
-    return !this.umap_id || this._loaded
+    return !this.createdOnServer || this._loaded
   }
 
   hasDataLoaded() {
     return this._dataloaded
   }
 
-  setUmapId(id) {
-    // Datalayer is null when listening creation form
-    if (!this.umap_id && id) this.umap_id = id
-  }
-
   backupOptions() {
     this._backupOptions = Utils.CopyJSON(this.options)
   }
@@ -357,8 +361,8 @@ export class DataLayer extends ServerStored {
 
   _dataUrl() {
     let url = this._umap.urls.get('datalayer_view', {
-      pk: this.umap_id,
+      pk: this.id,
-      map_id: this._umap.properties.umap_id,
+      map_id: this._umap.id,
     })
 
     // No browser cache for owners/editors.
@@ -437,7 +441,7 @@ export class DataLayer extends ServerStored {
       // otherwise the layer becomes uneditable.
       this.makeFeatures(geojson, sync)
     } catch (err) {
-      console.log('Error with DataLayer', this.umap_id)
+      console.log('Error with DataLayer', this.id)
       console.error(err)
     }
   }
@@ -524,22 +528,22 @@ export class DataLayer extends ServerStored {
 
   getDeleteUrl() {
     return this._umap.urls.get('datalayer_delete', {
-      pk: this.umap_id,
+      pk: this.id,
-      map_id: this._umap.properties.umap_id,
+      map_id: this._umap.id,
     })
   }
 
   getVersionsUrl() {
     return this._umap.urls.get('datalayer_versions', {
-      pk: this.umap_id,
+      pk: this.id,
-      map_id: this._umap.properties.umap_id,
+      map_id: this._umap.id,
     })
   }
 
   getVersionUrl(name) {
     return this._umap.urls.get('datalayer_version', {
-      pk: this.umap_id,
+      pk: this.id,
-      map_id: this._umap.properties.umap_id,
+      map_id: this._umap.id,
       name: name,
     })
   }
@@ -579,7 +583,8 @@ export class DataLayer extends ServerStored {
   }
 
   reset() {
-    if (!this.umap_id) this.erase()
+    if (!this.createdOnServer) this.erase()
+
     this.resetOptions()
     this.parentPane.appendChild(this.pane)
     if (this._leaflet_events_bk && !this._leaflet_events) {
@@ -809,7 +814,7 @@ export class DataLayer extends ServerStored {
       },
       this
     )
-    if (this.umap_id) {
+    if (this.createdOnServer) {
       const filename = `${Utils.slugify(this.options.name)}.geojson`
       const download = Utils.loadTemplate(`
         <a class="button" href="${this._dataUrl()}" download="${filename}">
@@ -1034,6 +1039,11 @@ export class DataLayer extends ServerStored {
     return this.isReadOnly() || this.isRemoteLayer()
   }
 
+  setReferenceVersion({ response, sync }) {
+    this._referenceVersion = response.headers.get('X-Datalayer-Version')
+    this.sync.update('_referenceVersion', this._referenceVersion)
+  }
+
   async save() {
     if (this.isDeleted) return await this.saveDelete()
     if (!this.isLoaded()) {
@@ -1048,14 +1058,15 @@ export class DataLayer extends ServerStored {
     // Filename support is shaky, don't do it for now.
     const blob = new Blob([JSON.stringify(geojson)], { type: 'application/json' })
     formData.append('geojson', blob)
-    const saveUrl = this._umap.urls.get('datalayer_save', {
+    const saveURL = this._umap.urls.get('datalayer_save', {
-      map_id: this._umap.properties.umap_id,
+      map_id: this._umap.id,
-      pk: this.umap_id,
+      pk: this.id,
+      created: this.createdOnServer,
     })
-    const headers = this._reference_version
+    const headers = this._referenceVersion
-      ? { 'X-Datalayer-Reference': this._reference_version }
+      ? { 'X-Datalayer-Reference': this._referenceVersion }
       : {}
-    const status = await this._trySave(saveUrl, headers, formData)
+    const status = await this._trySave(saveURL, headers, formData)
     this._geojson = geojson
     return status
   }
@@ -1090,11 +1101,12 @@ export class DataLayer extends ServerStored {
         this.fromGeoJSON(data.geojson)
         delete data.geojson
       }
-      this._reference_version = response.headers.get('X-Datalayer-Version')
+      delete data.id
-      this.sync.update('_reference_version', this._reference_version)
+      delete data._referenceVersion
-
-      this.setUmapId(data.id)
       this.updateOptions(data)
+
+      this.setReferenceVersion({ response, sync: true })
+
       this.backupOptions()
       this.backupData()
       this.connectToMap()
@@ -1105,7 +1117,7 @@ export class DataLayer extends ServerStored {
   }
 
   async saveDelete() {
-    if (this.umap_id) {
+    if (this.createdOnServer) {
       await this._umap.server.post(this.getDeleteUrl())
     }
     delete this._umap.datalayers[stamp(this)]

umap/static/umap/js/modules/permissions.js

@@ -149,7 +149,7 @@ export class MapPermissions extends ServerStored {
 
   edit() {
     if (this._umap.properties.editMode !== 'advanced') return
-    if (!this._umap.properties.umap_id) {
+    if (!this._umap.id) {
       Alert.info(translate('Please save the map first'))
       return
     }
@@ -199,13 +199,13 @@ export class MapPermissions extends ServerStored {
 
   getUrl() {
     return this._umap.urls.get('map_update_permissions', {
-      map_id: this._umap.properties.umap_id,
+      map_id: this._umap.id,
     })
   }
 
   getAttachUrl() {
     return this._umap.urls.get('map_attach_owner', {
-      map_id: this._umap.properties.umap_id,
+      map_id: this._umap.id,
     })
   }
 
@@ -262,8 +262,8 @@ export class DataLayerPermissions extends ServerStored {
 
   getUrl() {
     return this._umap.urls.get('datalayer_permissions', {
-      map_id: this._umap.properties.umap_id,
+      map_id: this._umap.id,
-      pk: this.datalayer.umap_id,
+      pk: this.datalayer.id,
     })
   }
 

umap/static/umap/js/modules/schema.js

@@ -563,4 +563,9 @@ export const SCHEMA = {
     type: Object,
     impacts: ['data'],
   },
+
+  _referenceVersion: {
+    type: Number,
+    impacts: ['data'],
+  },
 }

umap/static/umap/js/modules/share.js

@@ -61,7 +61,7 @@ export default class Share {
       translate('All data and settings of the map')
     )
     const downloadUrl = this._umap.urls.get('map_download', {
-      map_id: this._umap.properties.umap_id,
+      map_id: this._umap.id,
     })
     const link = Utils.loadTemplate(`
       <div>
@@ -205,8 +205,8 @@ class IframeExporter {
     }
     if (this.options.keepCurrentDatalayers) {
       this._umap.eachDataLayer((datalayer) => {
-        if (datalayer.isVisible() && datalayer.umap_id) {
+        if (datalayer.isVisible() && datalayer.createdOnServer) {
-          datalayers.push(datalayer.umap_id)
+          datalayers.push(datalayer.id)
         }
       })
       this.queryString.datalayers = datalayers.join(',')

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

@@ -32,8 +32,7 @@ class BaseUpdater {
   }
 
   getDataLayerFromID(layerId) {
-    if (layerId) return this._umap.getDataLayerByUmapId(layerId)
+    return this._umap.getDataLayerByUmapId(layerId)
-    return this._umap.defaultEditDataLayer()
   }
 
   applyMessage(payload) {
@@ -54,7 +53,7 @@ export class MapUpdater extends BaseUpdater {
 
 export class DataLayerUpdater extends BaseUpdater {
   upsert({ value }) {
-    // Inserts does not happen (we use multiple updates instead).
+    // Upsert only happens when a new datalayer is created.
     this._umap.createDataLayer(value, false)
     this._umap.render([])
   }

umap/static/umap/js/modules/umap.js

@@ -43,6 +43,10 @@ export default class Umap extends ServerStored {
     this.init(element, geojson)
   }
 
+  get id() {
+    return this.properties.id
+  }
+
   async init(element, geojson) {
     this.properties = Object.assign(
       {
@@ -166,7 +170,7 @@ export default class Umap extends ServerStored {
     }
 
     // Creation mode
-    if (!this.properties.umap_id) {
+    if (!this.id) {
       if (!this.properties.preview) {
         this.isDirty = true
         this.enableEdit()
@@ -285,7 +289,7 @@ export default class Umap extends ServerStored {
     }
     if (this.searchParams.has('download')) {
       const download_url = this.urls.get('map_download', {
-        map_id: this.properties.umap_id,
+        map_id: this.id,
       })
       window.location = download_url
     }
@@ -563,7 +567,7 @@ export default class Umap extends ServerStored {
   createDataLayer(options = {}, sync = true) {
     options.name =
       options.name || `${translate('Layer')} ${this.datalayersIndex.length + 1}`
-    const datalayer = new DataLayer(this, this._leafletMap, options, sync)
+    const datalayer = new DataLayer(this, this._leafletMap, options)
 
     if (sync !== false) {
       datalayer.sync.upsert(datalayer.options)
@@ -1098,7 +1102,7 @@ export default class Umap extends ServerStored {
     formData.append('name', this.properties.name)
     formData.append('center', JSON.stringify(this.geometry()))
     formData.append('settings', JSON.stringify(geojson))
-    const uri = this.urls.get('map_save', { map_id: this.properties.umap_id })
+    const uri = this.urls.get('map_save', { map_id: this.id })
     const [data, _, error] = await this.server.post(uri, {}, formData)
     // FIXME: login_required response will not be an error, so it will not
     // stop code while it should
@@ -1113,8 +1117,8 @@ export default class Umap extends ServerStored {
       return
     }
     this.properties.user = data.user
-    if (!this.properties.umap_id) {
+    if (!this.id) {
-      this.properties.umap_id = data.id
+      this.properties.id = data.id
       this.permissions.setProperties(data.permissions)
       this.permissions.commit()
       if (data.permissions?.anonymous_edit_url) {
@@ -1228,7 +1232,7 @@ export default class Umap extends ServerStored {
       this.sync.stop()
     } else {
       const ws_token_uri = this.urls.get('map_websocket_auth_token', {
-        map_id: this.properties.umap_id,
+        map_id: this.id,
       })
       await this.sync.authenticate(
         ws_token_uri,
@@ -1398,8 +1402,10 @@ export default class Umap extends ServerStored {
     this.editPanel.open({ content: container })
   }
 
-  getDataLayerByUmapId(umap_id) {
+  getDataLayerByUmapId(id) {
-    return this.findDataLayer((d) => d.umap_id === umap_id)
+    const datalayer = this.findDataLayer((d) => d.id === id)
+    if (!datalayer) throw new Error(`Can't find datalayer with id ${id}`)
+    return datalayer
   }
 
   firstVisibleDatalayer() {
@@ -1433,10 +1439,10 @@ export default class Umap extends ServerStored {
   }
 
   async star() {
-    if (!this.properties.umap_id) {
+    if (!this.id) {
       return Alert.error(translate('Please save the map first'))
     }
-    const url = this.urls.get('map_star', { map_id: this.properties.umap_id })
+    const url = this.urls.get('map_star', { map_id: this.id })
     const [data, response, error] = await this.server.post(url)
     if (error) {
       return
@@ -1530,7 +1536,7 @@ export default class Umap extends ServerStored {
     this.dialog
       .confirm(translate('Are you sure you want to delete this map?'))
       .then(async () => {
-        const url = this.urls.get('map_delete', { map_id: this.properties.umap_id })
+        const url = this.urls.get('map_delete', { map_id: this.id })
         const [data, response, error] = await this.server.post(url)
         if (data.redirect) window.location = data.redirect
       })
@@ -1542,7 +1548,7 @@ export default class Umap extends ServerStored {
         translate('Are you sure you want to clone this map and all its datalayers?')
       )
       .then(async () => {
-        const url = this.urls.get('map_clone', { map_id: this.properties.umap_id })
+        const url = this.urls.get('map_clone', { map_id: this.id })
         const [data, response, error] = await this.server.post(url)
         if (data.redirect) window.location = data.redirect
       })
@@ -1552,7 +1558,7 @@ export default class Umap extends ServerStored {
     const sendLink =
       this.properties.urls.map_send_edit_link &&
       this.urls.get('map_send_edit_link', {
-        map_id: this.properties.umap_id,
+        map_id: this.id,
       })
     await this.server.post(sendLink, {}, formData)
   }

umap/static/umap/js/modules/urls.js

@@ -25,8 +25,8 @@ export default class URLs {
   }
 
   // Update the layer if pk is passed, create otherwise.
-  datalayer_save({ map_id, pk }, ...options) {
+  datalayer_save({ map_id, pk, created }, ...options) {
-    if (pk) return this.get('datalayer_update', { map_id, pk }, ...options)
+    if (created) return this.get('datalayer_update', { map_id, pk }, ...options)
     return this.get('datalayer_create', { map_id, pk }, ...options)
   }
 }

umap/static/umap/unittests/URLs.js

@@ -8,10 +8,10 @@ import URLs from '../js/modules/urls.js'
 describe('URLs', () => {
   // Mock server URLs that will be used for testing
   const mockServerUrls = {
-    map_create: '/maps/create',
+    map_create: '/maps/create/',
-    map_update: '/maps/{map_id}/update',
+    map_update: '/maps/{map_id}/update/',
-    datalayer_create: '/maps/{map_id}/datalayers/create',
+    datalayer_create: '/maps/{map_id}/datalayers/{pk}/create/',
-    datalayer_update: '/maps/{map_id}/datalayers/{pk}/update',
+    datalayer_update: '/maps/{map_id}/datalayers/{pk}/update/',
   }
 
   let urls = new URLs(mockServerUrls)
@@ -22,37 +22,37 @@ describe('URLs', () => {
     })
 
     it('should return the correct templated URL for known urlNames', () => {
-      expect(urls.get('map_create')).to.be.equal('/maps/create')
+      expect(urls.get('map_create')).to.be.equal('/maps/create/')
-      expect(urls.get('map_update', { map_id: '123' })).to.be.equal('/maps/123/update')
+      expect(urls.get('map_update', { map_id: '123' })).to.be.equal('/maps/123/update/')
     })
 
     it('should return the correct templated URL when provided with parameters', () => {
       expect(urls.get('datalayer_update', { map_id: '123', pk: '456' })).to.be.equal(
-        '/maps/123/datalayers/456/update'
+        '/maps/123/datalayers/456/update/'
       )
     })
   })
 
   describe('map_save()', () => {
     it('should return the create URL if no map_id is provided', () => {
-      expect(urls.map_save({})).to.be.equal('/maps/create')
+      expect(urls.map_save({})).to.be.equal('/maps/create/')
     })
 
     it('should return the update URL if a map_id is provided', () => {
-      expect(urls.map_save({ map_id: '123' })).to.be.equal('/maps/123/update')
+      expect(urls.map_save({ map_id: '123' })).to.be.equal('/maps/123/update/')
     })
   })
 
   describe('datalayer_save()', () => {
-    it('should return the create URL if no pk is provided', () => {
+    it('should return the create URL if created is false', () => {
-      expect(urls.datalayer_save({ map_id: '123' })).to.be.equal(
+      expect(urls.datalayer_save({ map_id: '123', pk: '00000000-0000-0000-0000-000000000000', created: false })).to.be.equal(
-        '/maps/123/datalayers/create'
+        '/maps/123/datalayers/00000000-0000-0000-0000-000000000000/create/'
       )
     })
 
-    it('should return the update URL if a pk is provided', () => {
+    it('should return the update URL if created is true', () => {
-      expect(urls.datalayer_save({ map_id: '123', pk: '456' })).to.be.equal(
+      expect(urls.datalayer_save({ map_id: '123', pk: '00000000-0000-0000-0000-000000000000', created: true })).to.be.equal(
-        '/maps/123/datalayers/456/update'
+        '/maps/123/datalayers/00000000-0000-0000-0000-000000000000/update/'
       )
     })
   })

umap/tests/base.py

@@ -1,5 +1,6 @@
 import copy
 import json
+import uuid
 
 import factory
 from django.contrib.auth import get_user_model
@@ -113,6 +114,7 @@ class MapFactory(factory.django.DjangoModelFactory):
 
 
 class DataLayerFactory(factory.django.DjangoModelFactory):
+    uuid = factory.LazyFunction(lambda: uuid.uuid4())
     map = factory.SubFactory(MapFactory)
     name = "test datalayer"
     description = "test description"

umap/tests/integration/test_edit_datalayer.py

@@ -129,32 +129,43 @@ def test_can_change_name(live_server, openmap, page, datalayer):
 
 
 def test_can_create_new_datalayer(live_server, openmap, page, datalayer):
+    """
+    Test the creation and editing of a new datalayer.
+
+    This test verifies that:
+    1. A new datalayer can be created and saved.
+    2. The newly created datalayer appears in the UI and is saved in the database.
+    3. Editing the same datalayer updates it instead of creating a new one.
+    4. The UI reflects the changes and the database is updated correctly.
+    5. The 'dirty' state of the map is managed correctly during these operations.
+    """
+
     page.goto(
         f"{live_server.url}{openmap.get_absolute_url()}?edit&onLoadPanel=databrowser"
     )
     page.get_by_role("link", name="Manage layers").click()
     page.get_by_role("button", name="Add a layer").click()
     page.locator('input[name="name"]').click()
-    page.locator('input[name="name"]').fill("my new layer")
+    page.locator('input[name="name"]').fill("Layer A")
-    expect(page.get_by_text("my new layer")).to_be_visible()
+    expect(page.get_by_text("Layer A")).to_be_visible()
     with page.expect_response(re.compile(".*/datalayer/create/.*")):
         page.get_by_role("button", name="Save").click()
     assert DataLayer.objects.count() == 2
     saved = DataLayer.objects.last()
-    assert saved.name == "my new layer"
+    assert saved.name == "Layer A"
     expect(page.locator(".umap-is-dirty")).to_be_hidden()
     # Edit again, it should not create a new datalayer
     page.get_by_role("link", name="Manage layers").click()
     page.locator(".panel.right").get_by_title("Edit", exact=True).first.click()
     page.locator('input[name="name"]').click()
-    page.locator('input[name="name"]').fill("my new layer with a new name")
+    page.locator('input[name="name"]').fill("Layer A with a new name")
-    expect(page.get_by_text("my new layer with a new name")).to_be_visible()
+    expect(page.get_by_text("Layer A with a new name")).to_be_visible()
     page.get_by_role("button", name="Save").click()
     with page.expect_response(re.compile(".*/datalayer/update/.*")):
         page.get_by_role("button", name="Save").click()
     assert DataLayer.objects.count() == 2
     saved = DataLayer.objects.last()
-    assert saved.name == "my new layer with a new name"
+    assert saved.name == "Layer A with a new name"
     expect(page.locator(".umap-is-dirty")).to_be_hidden()
 
 

umap/tests/integration/test_import.py

@@ -92,11 +92,11 @@ def test_umap_import_from_textarea(live_server, tilelayer, page, settings):
     expect(
         page.locator('img[src="https://tile.openstreetmap.fr/hot/6/32/21.png"]')
     ).to_be_visible()
-    # Should not have imported umap_id, while in the file options
+    # Should not have imported id, while in the file options
-    assert not page.evaluate("U.MAP.properties.umap_id")
+    assert not page.evaluate("U.MAP.properties.id")
     with page.expect_response(re.compile(r".*/datalayer/create/.*")):
         page.get_by_role("button", name="Save").click()
-    assert page.evaluate("U.MAP.properties.umap_id")
+    assert page.evaluate("U.MAP.properties.id")
 
 
 def test_import_geojson_from_textarea(tilelayer, live_server, page):

umap/tests/integration/test_optimistic_merge.py

@@ -49,6 +49,7 @@ def test_created_markers_are_merged(context, live_server, tilelayer):
         "name": "test datalayer",
         "editMode": "advanced",
         "inCaption": True,
+        "id": str(datalayer.pk),
     }
 
     # Now navigate to this map from another tab
@@ -78,12 +79,14 @@ def test_created_markers_are_merged(context, live_server, tilelayer):
         sleep(1)
     # No change after the save
     expect(marker_pane_p2).to_have_count(2)
-    assert DataLayer.objects.get(pk=datalayer.pk).settings == {
+    datalayer_v2 = DataLayer.objects.get(pk=datalayer.pk)
+    assert datalayer_v2.settings == {
         "browsable": True,
         "displayOnLoad": True,
         "name": "test datalayer",
         "inCaption": True,
         "editMode": "advanced",
+        "id": str(datalayer.pk),
     }
 
     # Now create another marker in the first tab
@@ -94,7 +97,8 @@ def test_created_markers_are_merged(context, live_server, tilelayer):
         save_p1.click()
     # Should now get the other marker too
     expect(marker_pane_p1).to_have_count(3)
-    assert DataLayer.objects.get(pk=datalayer.pk).settings == {
+    datalayer_v3 = DataLayer.objects.get(pk=datalayer.pk)
+    assert datalayer_v3.settings == {
         "browsable": True,
         "displayOnLoad": True,
         "name": "test datalayer",
@@ -112,7 +116,8 @@ def test_created_markers_are_merged(context, live_server, tilelayer):
         save_p1.click()
         sleep(1)
     # Should now get the other marker too
-    assert DataLayer.objects.get(pk=datalayer.pk).settings == {
+    datalayer_v4 = DataLayer.objects.get(pk=datalayer.pk)
+    assert datalayer_v4.settings == {
         "browsable": True,
         "displayOnLoad": True,
         "name": "test datalayer",
@@ -132,7 +137,8 @@ def test_created_markers_are_merged(context, live_server, tilelayer):
         save_p2.click()
         sleep(1)
     # Should now get the other markers too
-    assert DataLayer.objects.get(pk=datalayer.pk).settings == {
+    datalayer_v5 = DataLayer.objects.get(pk=datalayer.pk)
+    assert datalayer_v5.settings == {
         "browsable": True,
         "displayOnLoad": True,
         "name": "test datalayer",

umap/tests/integration/test_websocket_sync.py

@@ -3,7 +3,7 @@ import re
 import pytest
 from playwright.sync_api import expect
 
-from umap.models import Map
+from umap.models import DataLayer, Map
 
 from ..base import DataLayerFactory, MapFactory
 
@@ -267,7 +267,7 @@ def test_websocket_connection_can_sync_cloned_polygons(
     b_polygon = peerB.locator("path")
 
     # Clone on peer B and save
-    b_polygon.click(button="right")
+    b_polygon.click(button="right", delay=200)
     peerB.get_by_role("button", name="Clone this feature").click()
 
     expect(peerB.locator("path")).to_have_count(2)
@@ -343,3 +343,75 @@ def test_websocket_connection_can_sync_late_joining_peer(
 
     # Clean up: close edit mode
     peerB.locator("body").press("Escape")
+
+
+@pytest.mark.xdist_group(name="websockets")
+def test_should_sync_datalayers(new_page, live_server, websocket_server, tilelayer):
+    map = MapFactory(name="sync", edit_status=Map.ANONYMOUS)
+    map.settings["properties"]["syncEnabled"] = True
+    map.save()
+
+    assert not DataLayer.objects.count()
+
+    # Create two tabs
+    peerA = new_page("Page A")
+    peerA.goto(f"{live_server.url}{map.get_absolute_url()}?edit")
+    peerB = new_page("Page B")
+    peerB.goto(f"{live_server.url}{map.get_absolute_url()}?edit")
+
+    # Create a new layer from peerA
+    peerA.get_by_role("link", name="Manage layers").click()
+    peerA.get_by_role("button", name="Add a layer").click()
+
+    # Check layer has been sync to peerB
+    peerB.get_by_role("button", name="Open browser").click()
+    expect(peerB.get_by_text("Layer 1")).to_be_visible()
+
+    # Draw a marker in layer 1 from peerA
+    peerA.get_by_role("link", name="Draw a marker (Ctrl+M)").click()
+    peerA.locator("#map").click()
+
+    # Check marker is visible from peerB
+    expect(peerB.locator(".leaflet-marker-icon")).to_be_visible()
+
+    # Save layer to the server
+    with peerA.expect_response(re.compile(".*/datalayer/create/.*")):
+        peerA.get_by_role("button", name="Save").click()
+
+    assert DataLayer.objects.count() == 1
+
+    # Create another layer from peerA and draw a marker on it (without saving to server)
+    peerA.get_by_role("link", name="Manage layers").click()
+    peerA.get_by_role("button", name="Add a layer").click()
+    peerA.get_by_role("link", name="Draw a marker (Ctrl+M)").click()
+    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()
+    peerB.locator(".panel.left").get_by_role("button", name="Show/hide layer").nth(
+        1
+    ).click()
+    expect(peerB.locator(".leaflet-marker-icon")).to_be_visible()
+
+    # Now draw a marker from peerB
+    peerB.get_by_role("link", name="Draw a marker (Ctrl+M)").click()
+    peerB.locator("#map").click()
+    peerB.locator('input[name="name"]').fill("marker from peerB")
+
+    # Save from peer B
+    with peerB.expect_response(re.compile(".*/datalayer/create/.*")):
+        peerB.get_by_role("button", name="Save").click()
+
+    assert DataLayer.objects.count() == 2
+
+    # Check this new marker is visible from peerA
+    peerA.get_by_role("button", name="Open browser").click()
+    peerA.locator(".panel.left").get_by_role("button", name="Show/hide layer").nth(
+        1
+    ).click()
+
+    # Now peerA saves the layer 2 to the server
+    with peerA.expect_response(re.compile(".*/datalayer/update/.*")):
+        peerA.get_by_role("button", name="Save").click()
+
+    assert DataLayer.objects.count() == 2

umap/tests/test_datalayer_views.py

@@ -1,6 +1,7 @@
 import json
 from copy import deepcopy
 from pathlib import Path
+from uuid import uuid4
 
 import pytest
 from django.core.files.base import ContentFile
@@ -105,6 +106,21 @@ def test_gzip_should_be_created_if_accepted(client, datalayer, map, post_data):
     assert Path(flat).stat().st_mtime_ns == Path(gzipped).stat().st_mtime_ns
 
 
+def test_create_datalayer(client, map, post_data):
+    uuid = str(uuid4())
+    url = reverse("datalayer_create", args=(map.pk, uuid))
+    client.login(username=map.owner.username, password="123123")
+    response = client.post(url, post_data, follow=True)
+    assert response.status_code == 200
+    new_datalayer = DataLayer.objects.get(pk=uuid)
+    assert new_datalayer.name == "name"
+    assert new_datalayer.rank == 0
+    # Test response is a json
+    data = json.loads(response.content.decode())
+    assert "id" in data
+    assert data["id"] == uuid
+
+
 def test_update(client, datalayer, map, post_data):
     url = reverse("datalayer_update", args=(map.pk, datalayer.pk))
     client.login(username=map.owner.username, password="123123")

umap/urls.py

@@ -155,8 +155,8 @@ map_urls = [
         views.MapClone.as_view(),
         name="map_clone",
     ),
-    re_path(
+    path(
-        r"^map/(?P<map_id>[\d]+)/datalayer/create/$",
+        "map/<int:map_id>/datalayer/create/<uuid:pk>/",
         views.DataLayerCreate.as_view(),
         name="datalayer_create",
     ),

umap/views.py

@@ -598,7 +598,7 @@ class MapDetailMixin(SessionMixin):
             "tilelayers": TileLayer.get_list(),
             "editMode": self.edit_mode,
             "schema": Map.extra_schema,
-            "umap_id": self.get_umap_id(),
+            "id": self.get_id(),
             "starred": self.is_starred(),
             "licences": dict((l.name, l.json) for l in Licence.objects.all()),
             "share_statuses": [
@@ -655,7 +655,7 @@ class MapDetailMixin(SessionMixin):
     def edit_mode(self):
         return "advanced"
 
-    def get_umap_id(self):
+    def get_id(self):
         return None
 
     def is_starred(self):
@@ -725,6 +725,8 @@ class MapView(MapDetailMixin, PermissionsMixin, DetailView):
         return self.object.get_absolute_url()
 
     def get_datalayers(self):
+        # When initializing datalayers from map, we cannot get the reference version
+        # the normal way, which is from the header X-Reference-Version
         return [dl.metadata(self.request) for dl in self.object.datalayer_set.all()]
 
     @property
@@ -736,7 +738,7 @@ class MapView(MapDetailMixin, PermissionsMixin, DetailView):
             edit_mode = "simple"
         return edit_mode
 
-    def get_umap_id(self):
+    def get_id(self):
         return self.object.pk
 
     def get_short_url(self):
@@ -1181,9 +1183,19 @@ class DataLayerCreate(FormLessEditMixin, GZipMixin, CreateView):
 
     def form_valid(self, form):
         form.instance.map = self.kwargs["map_inst"]
+
+        uuid = self.kwargs["pk"]
+        # Check if UUID already exists
+        if DataLayer.objects.filter(uuid=uuid).exists():
+            return HttpResponseBadRequest("UUID already exists")
+
+        form.instance.uuid = uuid
         self.object = form.save()
-        # Simple response with only metadata (including new id)
+        assert uuid == self.object.uuid
-        response = simple_json_response(**self.object.metadata(self.request))
+
+        # Simple response with only metadata
+        data = self.object.metadata(self.request)
+        response = simple_json_response(**data)
         response["X-Datalayer-Version"] = self.version
         return response