chore: replace datalayer ids with uuids

umap/fields.py

@@ -1,6 +1,7 @@
 import json
 
 import six
+from django.core.serializers.json import DjangoJSONEncoder
 from django.db import models
 from django.utils.encoding import smart_str
 
@@ -14,7 +15,7 @@ def get_prep_value(self, value):
         if not value:
             value = {}
         if not isinstance(value, six.string_types):
-            value = json.dumps(value)
+            value = json.dumps(value, cls=DjangoJSONEncoder)
         return value
 
     def from_db_value(self, value, expression, connection):

umap/migrations/0018_datalayer_uuid.py

@@ -0,0 +1,56 @@
+# Generated by Django 4.2.8 on 2024-02-19 17:40
+
+import uuid
+
+from django.db import migrations, models
+
+drop_index = """DO $$
+BEGIN
+    EXECUTE 'ALTER TABLE umap_datalayer DROP CONSTRAINT ' || (
+        SELECT indexname
+        FROM pg_indexes
+        WHERE tablename = 'umap_datalayer' AND indexname LIKE '%pk'
+    );
+END $$;
+"""
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("umap", "0017_migrate_to_openstreetmap_oauth2"),
+    ]
+
+    operations = [
+        # Add the new uuid field
+        migrations.AddField(
+            model_name="datalayer",
+            name="uuid",
+            field=models.UUIDField(
+                default=uuid.uuid4, editable=False, null=True, serialize=False
+            ),
+        ),
+        # Generate UUIDs for existing records
+        migrations.RunSQL(
+            "UPDATE umap_datalayer SET uuid = gen_random_uuid()",
+            reverse_sql=migrations.RunSQL.noop,
+        ),
+        # Remove the primary key constraint
+        migrations.RunSQL(drop_index, reverse_sql=migrations.RunSQL.noop),
+        # Drop the "id" primary key…
+        migrations.AlterField(
+            "datalayer", name="id", field=models.IntegerField(null=True, blank=True)
+        ),
+        # … to put it back on the "uuid"
+        migrations.AlterField(
+            model_name="datalayer",
+            name="uuid",
+            field=models.UUIDField(
+                default=uuid.uuid4,
+                editable=False,
+                unique=True,
+                primary_key=True,
+                serialize=False,
+            ),
+        ),
+        migrations.RunSQL(migrations.RunSQL.noop, reverse_sql=drop_index),
+    ]

umap/migrations/0019_migrate_internal_remote_datalayers.py

@@ -0,0 +1,52 @@
+# Generated by Django 4.2 on 2024-02-26 14:09
+
+import re
+
+from django.conf import settings
+from django.db import migrations
+from django.urls import NoReverseMatch, reverse
+
+# Some users hacked uMap to use another map datalayer as a remote data source.
+# This script gently handles the migration for them.
+
+
+def migrate_datalayers(apps, schema_editor):
+    DataLayer = apps.get_model("umap", "DataLayer")
+
+    datalayers = DataLayer.objects.filter(
+        settings__remoteData__url__icontains="datalayer"
+    )
+
+    for item in datalayers:
+        old_url = item.settings["remoteData"]["url"]
+        match = re.search(
+            rf"{settings.SITE_URL}/datalayer/(?P<map_id>\d+)/(?P<datalayer_id>\d+)",
+            old_url,
+        )
+        if match:
+            remote_id = match.group("datalayer_id")
+            map_id = match.group("map_id")
+            try:
+                remote_uuid = DataLayer.objects.get(id=remote_id).uuid
+            except DataLayer.DoesNotExist:
+                pass
+            else:
+                try:
+                    new_url = settings.SITE_URL + reverse(
+                        "datalayer_view", args=[map_id, remote_uuid]
+                    )
+                except NoReverseMatch:
+                    pass
+                else:
+                    item.settings["remoteData"]["url"] = new_url
+                    item.save()
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("umap", "0018_datalayer_uuid"),
+    ]
+
+    operations = [
+        migrations.RunPython(migrate_datalayers, reverse_code=migrations.RunPython.noop)
+    ]

umap/models.py

@@ -1,6 +1,7 @@
 import json
 import os
 import time
+import uuid
 
 from django.conf import settings
 from django.contrib.auth.models import User
@@ -373,7 +374,10 @@ class DataLayer(NamedModel):
         (EDITORS, _("Editors only")),
         (OWNER, _("Owner only")),
     )
-
+    uuid = models.UUIDField(
+        unique=True, primary_key=True, default=uuid.uuid4, editable=False
+    )
+    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"))
     geojson = models.FileField(upload_to=upload_to, blank=True, null=True)
@@ -436,6 +440,7 @@ def metadata(self, user=None, request=None):
 
     def clone(self, map_inst=None):
         new = self.__class__.objects.get(pk=self.pk)
+        new._state.adding = True
         new.pk = None
         if map_inst:
             new.map = map_inst
@@ -444,7 +449,10 @@ def clone(self, map_inst=None):
         return new
 
     def is_valid_version(self, name):
-        return name.startswith("%s_" % self.pk) and name.endswith(".geojson")
+        valid_prefixes = [name.startswith("%s_" % self.pk)]
+        if self.id:
+            valid_prefixes.append(name.startswith("%s_" % self.id))
+        return any(valid_prefixes) and name.endswith(".geojson")
 
     def version_metadata(self, name):
         els = name.split(".")[0].split("_")

umap/templatetags/umap_tags.py

@@ -3,6 +3,7 @@
 
 from django import template
 from django.conf import settings
+from django.core.serializers.json import DjangoJSONEncoder
 
 register = template.Library()
 
@@ -25,7 +26,7 @@ def map_fragment(map_instance, **kwargs):
     page = kwargs.pop("page", None) or ""
     unique_id = prefix + str(page) + "_" + str(map_instance.pk)
     return {
-        "map_settings": json.dumps(map_settings),
+        "map_settings": json.dumps(map_settings, cls=DjangoJSONEncoder),
         "map": map_instance,
         "unique_id": unique_id,
     }

umap/tests/integration/test_collaborative_editing.py

@@ -98,7 +98,7 @@ def test_collaborative_editing_create_markers(context, live_server, tilelayer):
         "name": "test datalayer",
         "inCaption": True,
         "editMode": "advanced",
-        "id": datalayer.pk,
+        "id": str(datalayer.pk),
         "permissions": {"edit_status": 1},
     }
 
@@ -116,7 +116,7 @@ def test_collaborative_editing_create_markers(context, live_server, tilelayer):
         "name": "test datalayer",
         "inCaption": True,
         "editMode": "advanced",
-        "id": datalayer.pk,
+        "id": str(datalayer.pk),
         "permissions": {"edit_status": 1},
     }
     expect(marker_pane_p1).to_have_count(4)
@@ -136,7 +136,7 @@ def test_collaborative_editing_create_markers(context, live_server, tilelayer):
         "name": "test datalayer",
         "inCaption": True,
         "editMode": "advanced",
-        "id": datalayer.pk,
+        "id": str(datalayer.pk),
         "permissions": {"edit_status": 1},
     }
     expect(marker_pane_p2).to_have_count(5)

umap/tests/test_datalayer_views.py

@@ -111,7 +111,7 @@ def test_update(client, datalayer, map, post_data):
     # Test response is a json
     j = json.loads(response.content.decode())
     assert "id" in j
-    assert datalayer.pk == j["id"]
+    assert str(datalayer.pk) == j["id"]
     assert j["browsable"] is True
     assert Path(modified_datalayer.geojson.path).exists()
 
@@ -225,6 +225,41 @@ def test_versions_should_return_versions(client, datalayer, map, settings):
     assert version in versions["versions"]
 
 
+def test_versions_can_return_old_format(client, datalayer, map, settings):
+    map.share_status = Map.PUBLIC
+    map.save()
+    root = datalayer.storage_root()
+    datalayer.id = 123  # old datalayer id (now replaced by uuid)
+    datalayer.save()
+
+    datalayer.geojson.storage.save(
+        "%s/%s_1440924889.geojson" % (root, datalayer.pk), ContentFile("{}")
+    )
+    datalayer.geojson.storage.save(
+        "%s/%s_1440923687.geojson" % (root, datalayer.pk), ContentFile("{}")
+    )
+
+    # store with the id prefix (rather than the uuid)
+    old_format_version = "%s_1440918637.geojson" % datalayer.id
+    datalayer.geojson.storage.save(
+        ("%s/" % root) + old_format_version, ContentFile("{}")
+    )
+
+    url = reverse("datalayer_versions", args=(map.pk, datalayer.pk))
+    versions = json.loads(client.get(url).content.decode())
+    assert len(versions["versions"]) == 4
+    version = {
+        "name": old_format_version,
+        "size": 2,
+        "at": "1440918637",
+    }
+    assert version in versions["versions"]
+
+    client.get(
+        reverse("datalayer_version", args=(map.pk, datalayer.pk, old_format_version))
+    )
+
+
 def test_version_should_return_one_version_geojson(client, datalayer, map):
     map.share_status = Map.PUBLIC
     map.save()

umap/urls.py

@@ -72,18 +72,18 @@
 ]
 i18n_urls += decorated_patterns(
     [can_view_map, cache_control(must_revalidate=True)],
-    re_path(
-        r"^datalayer/(?P<map_id>\d+)/(?P<pk>[\d]+)/$",
+    path(
+        "datalayer/<int:map_id>/<uuid:pk>/",
         views.DataLayerView.as_view(),
         name="datalayer_view",
     ),
-    re_path(
-        r"^datalayer/(?P<map_id>\d+)/(?P<pk>[\d]+)/versions/$",
+    path(
+        "datalayer/<int:map_id>/<uuid:pk>/versions/",
         views.DataLayerVersions.as_view(),
         name="datalayer_versions",
     ),
-    re_path(
-        r"^datalayer/(?P<map_id>\d+)/(?P<pk>[\d]+)/(?P<name>[_\w]+.geojson)$",
+    path(
+        "datalayer/<int:map_id>/<uuid:pk>/<str:name>",
         views.DataLayerVersion.as_view(),
         name="datalayer_version",
     ),
@@ -145,13 +145,13 @@
         views.DataLayerCreate.as_view(),
         name="datalayer_create",
     ),
-    re_path(
-        r"^map/(?P<map_id>[\d]+)/datalayer/delete/(?P<pk>\d+)/$",
+    path(
+        "map/<int:map_id>/datalayer/delete/<uuid:pk>/",
         views.DataLayerDelete.as_view(),
         name="datalayer_delete",
     ),
-    re_path(
-        r"^map/(?P<map_id>[\d]+)/datalayer/permissions/(?P<pk>\d+)/$",
+    path(
+        "map/<int:map_id>/datalayer/permissions/<uuid:pk>/",
         views.UpdateDataLayerPermissions.as_view(),
         name="datalayer_permissions",
     ),
@@ -165,8 +165,8 @@
         )
     )
 datalayer_urls = [
-    re_path(
-        r"^map/(?P<map_id>[\d]+)/datalayer/update/(?P<pk>\d+)/$",
+    path(
+        "map/<int:map_id>/datalayer/update/<uuid:pk>/",
         views.DataLayerUpdate.as_view(),
         name="datalayer_update",
     ),

umap/views.py

@@ -23,6 +23,7 @@
 from django.core.exceptions import PermissionDenied
 from django.core.mail import send_mail
 from django.core.paginator import EmptyPage, PageNotAnInteger, Paginator
+from django.core.serializers.json import DjangoJSONEncoder
 from django.core.signing import BadSignature, Signer
 from django.core.validators import URLValidator, ValidationError
 from django.http import (
@@ -314,7 +315,7 @@ def render_to_response(self, context, *args, **kwargs):
         with zipfile.ZipFile(zip_buffer, "a", zipfile.ZIP_DEFLATED, False) as zip_file:
             for map_ in self.get_maps():
                 umapjson = map_.generate_umapjson(self.request)
-                geojson_file = io.StringIO(json.dumps(umapjson))
+                geojson_file = io.StringIO(json.dumps(umapjson, cls=DjangoJSONEncoder))
                 file_name = f"umap_backup_{map_.slug}_{map_.pk}.umap"
                 zip_file.writestr(file_name, geojson_file.getvalue())
 
@@ -353,7 +354,7 @@ def make(m):
             }
 
         geojson = {"type": "FeatureCollection", "features": [make(m) for m in maps]}
-        return HttpResponse(smart_bytes(json.dumps(geojson)))
+        return HttpResponse(smart_bytes(json.dumps(geojson, cls=DjangoJSONEncoder)))
 
 
 showcase = MapsShowCase.as_view()
@@ -440,7 +441,9 @@ def get(self, *args, **kwargs):
 
 
 def simple_json_response(**kwargs):
-    return HttpResponse(json.dumps(kwargs), content_type="application/json")
+    return HttpResponse(
+        json.dumps(kwargs, cls=DjangoJSONEncoder), content_type="application/json"
+    )
 
 
 # ############## #
@@ -536,7 +539,9 @@ def get_context_data(self, **kwargs):
             geojson["properties"] = {}
         geojson["properties"].update(properties)
         geojson["properties"]["datalayers"] = self.get_datalayers()
-        context["map_settings"] = json.dumps(geojson, indent=settings.DEBUG)
+        context["map_settings"] = json.dumps(
+            geojson, indent=settings.DEBUG, cls=DjangoJSONEncoder
+        )
         self.set_preconnect(geojson["properties"], context)
         return context
 
@@ -1100,7 +1105,7 @@ def post(self, request, *args, **kwargs):
 
             # Replace the uploaded file by the merged version.
             self.request.FILES["geojson"].file = BytesIO(
-                json.dumps(merged).encode("utf-8")
+                json.dumps(merged, cls=DjangoJSONEncoder).encode("utf-8")
             )
 
             # Mark the data to be reloaded by form_valid