From 6b7efda37a887d77c5b16e39fa45dc2e317067a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Mon, 19 Feb 2024 19:07:43 +0100 Subject: [PATCH 01/14] chore: replace datalayer ids with uuids --- umap/migrations/0018_datalayer_uuid.py | 34 ++++++++++++++++++++++++++ umap/models.py | 3 ++- 2 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 umap/migrations/0018_datalayer_uuid.py diff --git a/umap/migrations/0018_datalayer_uuid.py b/umap/migrations/0018_datalayer_uuid.py new file mode 100644 index 00000000..5f5f2add --- /dev/null +++ b/umap/migrations/0018_datalayer_uuid.py @@ -0,0 +1,34 @@ +# Generated by Django 4.2.8 on 2024-02-19 17:40 + +import uuid + +from django.db import migrations, models + + +def gen_uuid(apps, schema_editor): + DataLayer = apps.get_model("umap", "DataLayer") + for row in DataLayer.objects.all(): + row.uuid = uuid.uuid4() + row.save(update_fields=["uuid"]) + + + +class Migration(migrations.Migration): + dependencies = [ + ("umap", "0017_migrate_to_openstreetmap_oauth2"), + ] + + operations = [ + migrations.AddField( + model_name="datalayer", + name="uuid", + field=models.UUIDField(default=uuid.uuid4, editable=False, null=True), + ), + migrations.RunPython(gen_uuid, reverse_code=migrations.RunPython.noop), + migrations.AlterField("datalayer", name="id", field=models.CharField(max_length=100)), + migrations.AlterField( + model_name="datalayer", + name="uuid", + field=models.UUIDField(default=uuid.uuid4, editable=False, unique=True, primary_key=True), + ), + ] diff --git a/umap/models.py b/umap/models.py index 4bff5f75..9bda2979 100644 --- a/umap/models.py +++ b/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,7 @@ class DataLayer(NamedModel): (EDITORS, _("Editors only")), (OWNER, _("Owner only")), ) - + uuid = models.UUIDField(unique=True, default=uuid.uuid4, editable=False) 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) From c46f59e3dd2251014f5544cb78b30d37aad0b175 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Thu, 22 Feb 2024 11:10:52 +0100 Subject: [PATCH 02/14] chore: Use datalayers' UUIDs in the views --- umap/models.py | 3 ++- umap/urls.py | 14 ++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/umap/models.py b/umap/models.py index 9bda2979..780aeb75 100644 --- a/umap/models.py +++ b/umap/models.py @@ -374,7 +374,7 @@ class DataLayer(NamedModel): (EDITORS, _("Editors only")), (OWNER, _("Owner only")), ) - uuid = models.UUIDField(unique=True, default=uuid.uuid4, editable=False) + uuid = models.UUIDField(unique=True, primary_key=True, default=uuid.uuid4, editable=False) 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) @@ -437,6 +437,7 @@ class DataLayer(NamedModel): 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 diff --git a/umap/urls.py b/umap/urls.py index 72fff437..09ce7f24 100644 --- a/umap/urls.py +++ b/umap/urls.py @@ -21,6 +21,8 @@ from .utils import decorated_patterns admin.autodiscover() +uuid = r"[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}" + urlpatterns = [ re_path(r"^admin/", admin.site.urls), re_path("", include("social_django.urls", namespace="social")), @@ -73,17 +75,17 @@ i18n_urls = [ i18n_urls += decorated_patterns( [can_view_map, cache_control(must_revalidate=True)], re_path( - r"^datalayer/(?P\d+)/(?P[\d]+)/$", + r"^datalayer/(?P\d+)/(?P" + uuid + ")/$", views.DataLayerView.as_view(), name="datalayer_view", ), re_path( - r"^datalayer/(?P\d+)/(?P[\d]+)/versions/$", + r"^datalayer/(?P\d+)/(?P" + uuid + r")/versions/$", views.DataLayerVersions.as_view(), name="datalayer_versions", ), re_path( - r"^datalayer/(?P\d+)/(?P[\d]+)/(?P[_\w]+.geojson)$", + r"^datalayer/(?P\d+)/(?P" + uuid + r" )/(?P[_\w]+.geojson)$", views.DataLayerVersion.as_view(), name="datalayer_version", ), @@ -146,12 +148,12 @@ map_urls = [ name="datalayer_create", ), re_path( - r"^map/(?P[\d]+)/datalayer/delete/(?P\d+)/$", + r"^map/(?P[\d]+)/datalayer/delete/(?P" + uuid + r")/$", views.DataLayerDelete.as_view(), name="datalayer_delete", ), re_path( - r"^map/(?P[\d]+)/datalayer/permissions/(?P\d+)/$", + r"^map/(?P[\d]+)/datalayer/permissions/(?P" + uuid + r"\d+)/$", views.UpdateDataLayerPermissions.as_view(), name="datalayer_permissions", ), @@ -166,7 +168,7 @@ if settings.DEFAULT_FROM_EMAIL: ) datalayer_urls = [ re_path( - r"^map/(?P[\d]+)/datalayer/update/(?P\d+)/$", + r"^map/(?P[\d]+)/datalayer/update/(?P" + uuid + r")/$", views.DataLayerUpdate.as_view(), name="datalayer_update", ), From c5fd72fe2b0f13a9862bbfd13153b4dbe0e28e02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Thu, 22 Feb 2024 11:52:29 +0100 Subject: [PATCH 03/14] chore: use Django JSON serializer when calling `json.dumps` --- umap/fields.py | 3 ++- umap/tests/test_datalayer_views.py | 2 +- umap/urls.py | 2 +- umap/views.py | 11 ++++++----- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/umap/fields.py b/umap/fields.py index 4798b0b4..8874c964 100644 --- a/umap/fields.py +++ b/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 @@ class DictField(models.TextField): 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): diff --git a/umap/tests/test_datalayer_views.py b/umap/tests/test_datalayer_views.py index 3fe8a99d..c06f7d20 100644 --- a/umap/tests/test_datalayer_views.py +++ b/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() diff --git a/umap/urls.py b/umap/urls.py index 09ce7f24..62d8d141 100644 --- a/umap/urls.py +++ b/umap/urls.py @@ -85,7 +85,7 @@ i18n_urls += decorated_patterns( name="datalayer_versions", ), re_path( - r"^datalayer/(?P\d+)/(?P" + uuid + r" )/(?P[_\w]+.geojson)$", + r"^datalayer/(?P\d+)/(?P" + uuid + r")/(?P" + uuid + r"[_\w]+.geojson)$", views.DataLayerVersion.as_view(), name="datalayer_version", ), diff --git a/umap/views.py b/umap/views.py index 5c90fe97..3fd8c76d 100644 --- a/umap/views.py +++ b/umap/views.py @@ -23,6 +23,7 @@ from django.contrib.staticfiles.storage import staticfiles_storage 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 @@ class UserDownload(DetailView, SearchMixin): 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 @@ class MapsShowCase(View): } 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,7 @@ ajax_proxy = AjaxProxy.as_view() 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 +537,7 @@ class MapDetailMixin: 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 +1101,7 @@ class DataLayerUpdate(FormLessEditMixin, GZipMixin, UpdateView): # 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 From 1415f96c6f6820210197c56598dd960c4bdf843b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Thu, 22 Feb 2024 14:07:20 +0100 Subject: [PATCH 04/14] chore: fix tests --- umap/migrations/0018_datalayer_uuid.py | 3 ++- umap/templatetags/umap_tags.py | 3 ++- umap/tests/integration/test_collaborative_editing.py | 6 +++--- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/umap/migrations/0018_datalayer_uuid.py b/umap/migrations/0018_datalayer_uuid.py index 5f5f2add..9d0bbd23 100644 --- a/umap/migrations/0018_datalayer_uuid.py +++ b/umap/migrations/0018_datalayer_uuid.py @@ -25,10 +25,11 @@ class Migration(migrations.Migration): field=models.UUIDField(default=uuid.uuid4, editable=False, null=True), ), migrations.RunPython(gen_uuid, reverse_code=migrations.RunPython.noop), - migrations.AlterField("datalayer", name="id", field=models.CharField(max_length=100)), + migrations.AlterField("datalayer", name="id", field=models.CharField(max_length=100, null=True)), migrations.AlterField( model_name="datalayer", name="uuid", field=models.UUIDField(default=uuid.uuid4, editable=False, unique=True, primary_key=True), ), + # migrations.RemoveConstraint("datalayer", "") ] diff --git a/umap/templatetags/umap_tags.py b/umap/templatetags/umap_tags.py index 2349c579..b995df4f 100644 --- a/umap/templatetags/umap_tags.py +++ b/umap/templatetags/umap_tags.py @@ -3,6 +3,7 @@ from copy import copy 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, } diff --git a/umap/tests/integration/test_collaborative_editing.py b/umap/tests/integration/test_collaborative_editing.py index 29cd9b58..e22b0ee9 100644 --- a/umap/tests/integration/test_collaborative_editing.py +++ b/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) From d18a32b5d376fe99b14bb30a1914abc4c1b02a03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Thu, 22 Feb 2024 15:08:13 +0100 Subject: [PATCH 05/14] chore: use `path` rather than `re_path` in urls.py --- umap/urls.py | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/umap/urls.py b/umap/urls.py index 62d8d141..6467e733 100644 --- a/umap/urls.py +++ b/umap/urls.py @@ -21,8 +21,6 @@ from .utils import decorated_patterns admin.autodiscover() -uuid = r"[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}" - urlpatterns = [ re_path(r"^admin/", admin.site.urls), re_path("", include("social_django.urls", namespace="social")), @@ -74,18 +72,18 @@ i18n_urls = [ ] i18n_urls += decorated_patterns( [can_view_map, cache_control(must_revalidate=True)], - re_path( - r"^datalayer/(?P\d+)/(?P" + uuid + ")/$", + path( + "datalayer///", views.DataLayerView.as_view(), name="datalayer_view", ), - re_path( - r"^datalayer/(?P\d+)/(?P" + uuid + r")/versions/$", + path( + "datalayer///versions/", views.DataLayerVersions.as_view(), name="datalayer_versions", ), - re_path( - r"^datalayer/(?P\d+)/(?P" + uuid + r")/(?P" + uuid + r"[_\w]+.geojson)$", + path( + "datalayer///", views.DataLayerVersion.as_view(), name="datalayer_version", ), @@ -147,13 +145,13 @@ map_urls = [ views.DataLayerCreate.as_view(), name="datalayer_create", ), - re_path( - r"^map/(?P[\d]+)/datalayer/delete/(?P" + uuid + r")/$", + path( + "map//datalayer/delete//", views.DataLayerDelete.as_view(), name="datalayer_delete", ), - re_path( - r"^map/(?P[\d]+)/datalayer/permissions/(?P" + uuid + r"\d+)/$", + path( + "map//datalayer/permissions//", views.UpdateDataLayerPermissions.as_view(), name="datalayer_permissions", ), @@ -167,8 +165,7 @@ if settings.DEFAULT_FROM_EMAIL: ) ) datalayer_urls = [ - re_path( - r"^map/(?P[\d]+)/datalayer/update/(?P" + uuid + r")/$", + path("map//datalayer/update//", views.DataLayerUpdate.as_view(), name="datalayer_update", ), From f869d77f2c400bb56d8d279dcce98e9b468e5f93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Thu, 22 Feb 2024 15:08:36 +0100 Subject: [PATCH 06/14] chore: update migrations --- umap/migrations/0018_datalayer_uuid.py | 4 +++- umap/models.py | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/umap/migrations/0018_datalayer_uuid.py b/umap/migrations/0018_datalayer_uuid.py index 9d0bbd23..6153b1e9 100644 --- a/umap/migrations/0018_datalayer_uuid.py +++ b/umap/migrations/0018_datalayer_uuid.py @@ -25,7 +25,9 @@ class Migration(migrations.Migration): field=models.UUIDField(default=uuid.uuid4, editable=False, null=True), ), migrations.RunPython(gen_uuid, reverse_code=migrations.RunPython.noop), - migrations.AlterField("datalayer", name="id", field=models.CharField(max_length=100, null=True)), + migrations.RunSQL("ALTER TABLE umap_datalayer DROP CONSTRAINT umap_datalayer_pkey"), + # migrations.RemoveConstraint("datalayer", "id"), + migrations.AlterField("datalayer", name="id", field=models.IntegerField(null=True, blank=True)), migrations.AlterField( model_name="datalayer", name="uuid", diff --git a/umap/models.py b/umap/models.py index 780aeb75..0a2fce84 100644 --- a/umap/models.py +++ b/umap/models.py @@ -375,6 +375,7 @@ class DataLayer(NamedModel): (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) From 99d7b8a6e108f8f05996fce011ee4335ed074e48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Thu, 22 Feb 2024 15:35:18 +0100 Subject: [PATCH 07/14] chore: ruff format --- umap/migrations/0018_datalayer_uuid.py | 13 +++++++++---- umap/models.py | 4 +++- umap/urls.py | 3 ++- umap/views.py | 8 ++++++-- 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/umap/migrations/0018_datalayer_uuid.py b/umap/migrations/0018_datalayer_uuid.py index 6153b1e9..e0a655c9 100644 --- a/umap/migrations/0018_datalayer_uuid.py +++ b/umap/migrations/0018_datalayer_uuid.py @@ -12,7 +12,6 @@ def gen_uuid(apps, schema_editor): row.save(update_fields=["uuid"]) - class Migration(migrations.Migration): dependencies = [ ("umap", "0017_migrate_to_openstreetmap_oauth2"), @@ -25,13 +24,19 @@ class Migration(migrations.Migration): field=models.UUIDField(default=uuid.uuid4, editable=False, null=True), ), migrations.RunPython(gen_uuid, reverse_code=migrations.RunPython.noop), - migrations.RunSQL("ALTER TABLE umap_datalayer DROP CONSTRAINT umap_datalayer_pkey"), + migrations.RunSQL( + "ALTER TABLE umap_datalayer DROP CONSTRAINT umap_datalayer_pkey" + ), # migrations.RemoveConstraint("datalayer", "id"), - migrations.AlterField("datalayer", name="id", field=models.IntegerField(null=True, blank=True)), + migrations.AlterField( + "datalayer", name="id", field=models.IntegerField(null=True, blank=True) + ), migrations.AlterField( model_name="datalayer", name="uuid", - field=models.UUIDField(default=uuid.uuid4, editable=False, unique=True, primary_key=True), + field=models.UUIDField( + default=uuid.uuid4, editable=False, unique=True, primary_key=True + ), ), # migrations.RemoveConstraint("datalayer", "") ] diff --git a/umap/models.py b/umap/models.py index 0a2fce84..8257c8cb 100644 --- a/umap/models.py +++ b/umap/models.py @@ -374,7 +374,9 @@ class DataLayer(NamedModel): (EDITORS, _("Editors only")), (OWNER, _("Owner only")), ) - uuid = models.UUIDField(unique=True, primary_key=True, default=uuid.uuid4, editable=False) + 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")) diff --git a/umap/urls.py b/umap/urls.py index 6467e733..28564b55 100644 --- a/umap/urls.py +++ b/umap/urls.py @@ -165,7 +165,8 @@ if settings.DEFAULT_FROM_EMAIL: ) ) datalayer_urls = [ - path("map//datalayer/update//", + path( + "map//datalayer/update//", views.DataLayerUpdate.as_view(), name="datalayer_update", ), diff --git a/umap/views.py b/umap/views.py index 3fd8c76d..b43605c6 100644 --- a/umap/views.py +++ b/umap/views.py @@ -441,7 +441,9 @@ ajax_proxy = AjaxProxy.as_view() def simple_json_response(**kwargs): - return HttpResponse(json.dumps(kwargs, cls=DjangoJSONEncoder), content_type="application/json") + return HttpResponse( + json.dumps(kwargs, cls=DjangoJSONEncoder), content_type="application/json" + ) # ############## # @@ -537,7 +539,9 @@ class MapDetailMixin: geojson["properties"] = {} geojson["properties"].update(properties) geojson["properties"]["datalayers"] = self.get_datalayers() - context["map_settings"] = json.dumps(geojson, indent=settings.DEBUG, cls=DjangoJSONEncoder) + context["map_settings"] = json.dumps( + geojson, indent=settings.DEBUG, cls=DjangoJSONEncoder + ) self.set_preconnect(geojson["properties"], context) return context From ff4870730ab30466449c15c669a539bf7fd67ec3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Mon, 26 Feb 2024 11:44:23 +0100 Subject: [PATCH 08/14] chore: ensure old-format layers' versions are returned --- umap/models.py | 5 ++++- umap/tests/test_datalayer_views.py | 36 ++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/umap/models.py b/umap/models.py index 8257c8cb..f10fc876 100644 --- a/umap/models.py +++ b/umap/models.py @@ -449,7 +449,10 @@ class DataLayer(NamedModel): 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("_") diff --git a/umap/tests/test_datalayer_views.py b/umap/tests/test_datalayer_views.py index c06f7d20..5cab92aa 100644 --- a/umap/tests/test_datalayer_views.py +++ b/umap/tests/test_datalayer_views.py @@ -225,6 +225,42 @@ 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"] + + # Check we can access the old format + 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() From 51889f3238109ec3f9967c8c299ab773f07e88ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Mon, 26 Feb 2024 14:43:47 +0100 Subject: [PATCH 09/14] chore: fetch datalayer index name before dropping it. This can be helpful in situations where the name of the index is not known, as it can be with pre 1.0 deployed instance. This commit also generates the UUIDs directly using an SQL statement. --- umap/migrations/0018_datalayer_uuid.py | 27 +++++++++++++++----------- umap/tests/test_datalayer_views.py | 9 +++------ 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/umap/migrations/0018_datalayer_uuid.py b/umap/migrations/0018_datalayer_uuid.py index e0a655c9..7e9a1ee8 100644 --- a/umap/migrations/0018_datalayer_uuid.py +++ b/umap/migrations/0018_datalayer_uuid.py @@ -5,32 +5,38 @@ import uuid from django.db import migrations, models -def gen_uuid(apps, schema_editor): - DataLayer = apps.get_model("umap", "DataLayer") - for row in DataLayer.objects.all(): - row.uuid = uuid.uuid4() - row.save(update_fields=["uuid"]) - - 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), ), - migrations.RunPython(gen_uuid, reverse_code=migrations.RunPython.noop), + # Generate UUIDs for existing records + migrations.RunSQL("UPDATE umap_datalayer SET uuid = gen_random_uuid()"), + # Remove the primary key constraint migrations.RunSQL( - "ALTER TABLE umap_datalayer DROP CONSTRAINT umap_datalayer_pkey" + """ + DO $$ + BEGIN + EXECUTE 'ALTER TABLE umap_datalayer DROP CONSTRAINT ' || ( + SELECT indexname + FROM pg_indexes + WHERE tablename = 'umap_datalayer' AND indexname LIKE '%pkey' + ); + END $$; + """ ), - # migrations.RemoveConstraint("datalayer", "id"), + # 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", @@ -38,5 +44,4 @@ class Migration(migrations.Migration): default=uuid.uuid4, editable=False, unique=True, primary_key=True ), ), - # migrations.RemoveConstraint("datalayer", "") ] diff --git a/umap/tests/test_datalayer_views.py b/umap/tests/test_datalayer_views.py index 5cab92aa..d3a1125d 100644 --- a/umap/tests/test_datalayer_views.py +++ b/umap/tests/test_datalayer_views.py @@ -240,22 +240,19 @@ def test_versions_can_return_old_format(client, datalayer, map, settings): ) # 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("{}") - ) + old_format_version = "%s/%s_1440918637.geojson" % (root, datalayer.id) + datalayer.geojson.storage.save(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, + "name": "%s_1440918637.geojson" % datalayer.id, "size": 2, "at": "1440918637", } assert version in versions["versions"] - # Check we can access the old format client.get( reverse("datalayer_version", args=(map.pk, datalayer.pk, old_format_version)) ) From ffe7f18af1f80b164eb8b24617b27d3d67a4fef1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Mon, 26 Feb 2024 15:00:56 +0100 Subject: [PATCH 10/14] chore: enhance naming in tests --- umap/tests/test_datalayer_views.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/umap/tests/test_datalayer_views.py b/umap/tests/test_datalayer_views.py index d3a1125d..1ae87b8f 100644 --- a/umap/tests/test_datalayer_views.py +++ b/umap/tests/test_datalayer_views.py @@ -240,14 +240,16 @@ def test_versions_can_return_old_format(client, datalayer, map, settings): ) # store with the id prefix (rather than the uuid) - old_format_version = "%s/%s_1440918637.geojson" % (root, datalayer.id) - datalayer.geojson.storage.save(old_format_version, ContentFile("{}")) + 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": "%s_1440918637.geojson" % datalayer.id, + "name": old_format_version, "size": 2, "at": "1440918637", } From 9648c8ba42eb5833aa165e3d0ed5ad4baf2f7c26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Mon, 26 Feb 2024 20:23:34 +0100 Subject: [PATCH 11/14] chore: migrate existing local remoteURL datalayers --- umap/migrations/0018_datalayer_uuid.py | 10 +++- ...0019_migrate_internal_remote_datalayers.py | 52 +++++++++++++++++++ 2 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 umap/migrations/0019_migrate_internal_remote_datalayers.py diff --git a/umap/migrations/0018_datalayer_uuid.py b/umap/migrations/0018_datalayer_uuid.py index 7e9a1ee8..02468444 100644 --- a/umap/migrations/0018_datalayer_uuid.py +++ b/umap/migrations/0018_datalayer_uuid.py @@ -15,7 +15,9 @@ class Migration(migrations.Migration): migrations.AddField( model_name="datalayer", name="uuid", - field=models.UUIDField(default=uuid.uuid4, editable=False, null=True), + 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()"), @@ -41,7 +43,11 @@ class Migration(migrations.Migration): model_name="datalayer", name="uuid", field=models.UUIDField( - default=uuid.uuid4, editable=False, unique=True, primary_key=True + default=uuid.uuid4, + editable=False, + unique=True, + primary_key=True, + serialize=False, ), ), ] diff --git a/umap/migrations/0019_migrate_internal_remote_datalayers.py b/umap/migrations/0019_migrate_internal_remote_datalayers.py new file mode 100644 index 00000000..59f91cd8 --- /dev/null +++ b/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\d+)/(?P\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) + ] From f82897f2028ad7c0b1c91c0cf48c4e8ebe8c7011 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Mon, 26 Feb 2024 23:33:05 +0100 Subject: [PATCH 12/14] chore: make the datalayer uuid migration reversible --- umap/migrations/0018_datalayer_uuid.py | 27 ++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/umap/migrations/0018_datalayer_uuid.py b/umap/migrations/0018_datalayer_uuid.py index 02468444..08b84fe5 100644 --- a/umap/migrations/0018_datalayer_uuid.py +++ b/umap/migrations/0018_datalayer_uuid.py @@ -4,6 +4,16 @@ 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 = [ @@ -20,20 +30,12 @@ class Migration(migrations.Migration): ), ), # Generate UUIDs for existing records - migrations.RunSQL("UPDATE umap_datalayer SET uuid = gen_random_uuid()"), - # Remove the primary key constraint migrations.RunSQL( - """ - DO $$ - BEGIN - EXECUTE 'ALTER TABLE umap_datalayer DROP CONSTRAINT ' || ( - SELECT indexname - FROM pg_indexes - WHERE tablename = 'umap_datalayer' AND indexname LIKE '%pkey' - ); - END $$; - """ + "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) @@ -50,4 +52,5 @@ class Migration(migrations.Migration): serialize=False, ), ), + migrations.RunSQL(migrations.RunSQL.noop, reverse_sql=drop_index), ] From 1b41ff0ddc7e97a592fabaa5da7580bde9f960c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Thu, 29 Feb 2024 11:01:02 +0100 Subject: [PATCH 13/14] chore: Rename datalayer id to old_id --- umap/migrations/0018_datalayer_uuid.py | 8 +++++++- .../migrations/0019_migrate_internal_remote_datalayers.py | 2 +- umap/models.py | 6 +++--- umap/tests/test_datalayer_views.py | 4 ++-- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/umap/migrations/0018_datalayer_uuid.py b/umap/migrations/0018_datalayer_uuid.py index 08b84fe5..86c52ec0 100644 --- a/umap/migrations/0018_datalayer_uuid.py +++ b/umap/migrations/0018_datalayer_uuid.py @@ -9,7 +9,7 @@ BEGIN EXECUTE 'ALTER TABLE umap_datalayer DROP CONSTRAINT ' || ( SELECT indexname FROM pg_indexes - WHERE tablename = 'umap_datalayer' AND indexname LIKE '%pk' + WHERE tablename = 'umap_datalayer' AND indexname LIKE '%pk%' ); END $$; """ @@ -40,6 +40,10 @@ class Migration(migrations.Migration): migrations.AlterField( "datalayer", name="id", field=models.IntegerField(null=True, blank=True) ), + # Rename "id" to "old id" + migrations.RenameField( + model_name="datalayer", old_name="id", new_name="old_id" + ), # … to put it back on the "uuid" migrations.AlterField( model_name="datalayer", @@ -52,5 +56,7 @@ class Migration(migrations.Migration): serialize=False, ), ), + # When applying the migration backwards, we need to drop the pk index + # Before addding a new one. migrations.RunSQL(migrations.RunSQL.noop, reverse_sql=drop_index), ] diff --git a/umap/migrations/0019_migrate_internal_remote_datalayers.py b/umap/migrations/0019_migrate_internal_remote_datalayers.py index 59f91cd8..1a0bc77e 100644 --- a/umap/migrations/0019_migrate_internal_remote_datalayers.py +++ b/umap/migrations/0019_migrate_internal_remote_datalayers.py @@ -27,7 +27,7 @@ def migrate_datalayers(apps, schema_editor): remote_id = match.group("datalayer_id") map_id = match.group("map_id") try: - remote_uuid = DataLayer.objects.get(id=remote_id).uuid + remote_uuid = DataLayer.objects.get(old_id=remote_id).uuid except DataLayer.DoesNotExist: pass else: diff --git a/umap/models.py b/umap/models.py index f10fc876..aa60b334 100644 --- a/umap/models.py +++ b/umap/models.py @@ -377,7 +377,7 @@ class DataLayer(NamedModel): uuid = models.UUIDField( unique=True, primary_key=True, default=uuid.uuid4, editable=False ) - id = models.IntegerField(null=True, blank=True) + 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")) geojson = models.FileField(upload_to=upload_to, blank=True, null=True) @@ -450,8 +450,8 @@ class DataLayer(NamedModel): def is_valid_version(self, name): valid_prefixes = [name.startswith("%s_" % self.pk)] - if self.id: - valid_prefixes.append(name.startswith("%s_" % self.id)) + if self.old_id: + valid_prefixes.append(name.startswith("%s_" % self.old_id)) return any(valid_prefixes) and name.endswith(".geojson") def version_metadata(self, name): diff --git a/umap/tests/test_datalayer_views.py b/umap/tests/test_datalayer_views.py index 1ae87b8f..22ca9b95 100644 --- a/umap/tests/test_datalayer_views.py +++ b/umap/tests/test_datalayer_views.py @@ -229,7 +229,7 @@ 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.old_id = 123 # old datalayer id (now replaced by uuid) datalayer.save() datalayer.geojson.storage.save( @@ -240,7 +240,7 @@ def test_versions_can_return_old_format(client, datalayer, map, settings): ) # store with the id prefix (rather than the uuid) - old_format_version = "%s_1440918637.geojson" % datalayer.id + old_format_version = "%s_1440918637.geojson" % datalayer.old_id datalayer.geojson.storage.save( ("%s/" % root) + old_format_version, ContentFile("{}") ) From b0c1f5697944e0eab0ddcd088b505ee563fb3a3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Tue, 5 Mar 2024 11:25:37 +0100 Subject: [PATCH 14/14] chore: factorize `json.dumps` in an util function. In order to use the Django JSON Encoder by default. --- umap/fields.py | 5 +++-- umap/templatetags/umap_tags.py | 6 +++--- umap/utils.py | 7 +++++++ umap/views.py | 24 +++++++++++++----------- 4 files changed, 26 insertions(+), 16 deletions(-) diff --git a/umap/fields.py b/umap/fields.py index 8874c964..4e6506ee 100644 --- a/umap/fields.py +++ b/umap/fields.py @@ -1,10 +1,11 @@ import json import six -from django.core.serializers.json import DjangoJSONEncoder from django.db import models from django.utils.encoding import smart_str +from .utils import json_dumps + class DictField(models.TextField): """ @@ -15,7 +16,7 @@ class DictField(models.TextField): if not value: value = {} if not isinstance(value, six.string_types): - value = json.dumps(value, cls=DjangoJSONEncoder) + value = json_dumps(value) return value def from_db_value(self, value, expression, connection): diff --git a/umap/templatetags/umap_tags.py b/umap/templatetags/umap_tags.py index b995df4f..aa7a45c0 100644 --- a/umap/templatetags/umap_tags.py +++ b/umap/templatetags/umap_tags.py @@ -1,9 +1,9 @@ -import json from copy import copy from django import template from django.conf import settings -from django.core.serializers.json import DjangoJSONEncoder + +from umap.utils import json_dumps register = template.Library() @@ -26,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, cls=DjangoJSONEncoder), + "map_settings": json_dumps(map_settings), "map": map_instance, "unique_id": unique_id, } diff --git a/umap/utils.py b/umap/utils.py index 003b9b01..26cf581d 100644 --- a/umap/utils.py +++ b/umap/utils.py @@ -1,7 +1,9 @@ import gzip +import json import os from django.conf import settings +from django.core.serializers.json import DjangoJSONEncoder from django.urls import URLPattern, URLResolver, get_resolver @@ -162,3 +164,8 @@ def merge_features(reference: list, latest: list, incoming: list): merged.append(item) return merged + + +def json_dumps(obj, **kwargs): + """Utility using the Django JSON Encoder when dumping objects""" + return json.dumps(obj, cls=DjangoJSONEncoder, **kwargs) diff --git a/umap/views.py b/umap/views.py index b43605c6..ef377228 100644 --- a/umap/views.py +++ b/umap/views.py @@ -23,7 +23,6 @@ from django.contrib.staticfiles.storage import staticfiles_storage 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 ( @@ -67,7 +66,14 @@ from .forms import ( UserProfileForm, ) from .models import DataLayer, Licence, Map, Pictogram, Star, TileLayer -from .utils import ConflictError, _urls_for_js, gzip_file, is_ajax, merge_features +from .utils import ( + ConflictError, + _urls_for_js, + gzip_file, + is_ajax, + json_dumps, + merge_features, +) User = get_user_model() @@ -315,7 +321,7 @@ class UserDownload(DetailView, SearchMixin): 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, cls=DjangoJSONEncoder)) + geojson_file = io.StringIO(json_dumps(umapjson)) file_name = f"umap_backup_{map_.slug}_{map_.pk}.umap" zip_file.writestr(file_name, geojson_file.getvalue()) @@ -354,7 +360,7 @@ class MapsShowCase(View): } geojson = {"type": "FeatureCollection", "features": [make(m) for m in maps]} - return HttpResponse(smart_bytes(json.dumps(geojson, cls=DjangoJSONEncoder))) + return HttpResponse(smart_bytes(json_dumps(geojson))) showcase = MapsShowCase.as_view() @@ -441,9 +447,7 @@ ajax_proxy = AjaxProxy.as_view() def simple_json_response(**kwargs): - return HttpResponse( - json.dumps(kwargs, cls=DjangoJSONEncoder), content_type="application/json" - ) + return HttpResponse(json_dumps(kwargs), content_type="application/json") # ############## # @@ -539,9 +543,7 @@ class MapDetailMixin: geojson["properties"] = {} geojson["properties"].update(properties) geojson["properties"]["datalayers"] = self.get_datalayers() - context["map_settings"] = json.dumps( - geojson, indent=settings.DEBUG, cls=DjangoJSONEncoder - ) + context["map_settings"] = json_dumps(geojson, indent=settings.DEBUG) self.set_preconnect(geojson["properties"], context) return context @@ -1105,7 +1107,7 @@ class DataLayerUpdate(FormLessEditMixin, GZipMixin, UpdateView): # Replace the uploaded file by the merged version. self.request.FILES["geojson"].file = BytesIO( - json.dumps(merged, cls=DjangoJSONEncoder).encode("utf-8") + json_dumps(merged).encode("utf-8") ) # Mark the data to be reloaded by form_valid