Merge pull request #361 from ohmg-dev/316_deprecate_multimask

316 deprecate multimask field on LayerSets

Author: mradamcox

ohmg/api/schemas.py

@@ -327,7 +327,7 @@ class LayerSchema(Schema):
     extent: Optional[list]
 
     @staticmethod
-    def resolve_urls(obj):
+    def resolve_urls(obj: Layer):
         return {
             "resource": f"/layer/{obj.pk}",
             "thumbnail": obj.thumbnail.url if obj.thumbnail else "",
@@ -336,21 +336,15 @@ def resolve_urls(obj):
         }
 
     @staticmethod
-    def resolve_mask(obj):
-        if obj.layerset2 and obj.layerset2.multimask and obj.slug in obj.layerset2.multimask:
-            return obj.layerset2.multimask[obj.slug]
-        else:
-            return None
+    def resolve_mask(obj: Layer):
+        return json.loads(obj.mask.geojson) if obj.mask else None
 
     @staticmethod
-    def resolve_image_url(obj):
-        if obj.region and obj.region.file:
-            return obj.region.file.url
-        else:
-            return None
+    def resolve_image_url(obj: Layer):
+        return obj.region.file.url if obj.region and obj.region.file else None
 
     @staticmethod
-    def resolve_gcps_geojson(obj):
+    def resolve_gcps_geojson(obj: Layer):
         if not obj.region:
             logger.warning(f"[WARNING] Layer {obj.pk} has no associated region")
             return None
@@ -362,18 +356,12 @@ def resolve_gcps_geojson(obj):
         return obj.region.gcpgroup.as_geojson
 
     @staticmethod
-    def resolve_created_by(obj):
-        if obj.created_by:
-            return obj.created_by.username
-        else:
-            return ""
+    def resolve_created_by(obj: Layer):
+        return obj.created_by.username if obj.created_by else ""
 
     @staticmethod
-    def resolve_last_updated_by(obj):
-        if obj.last_updated_by:
-            return obj.last_updated_by.username
-        else:
-            return ""
+    def resolve_last_updated_by(obj: Layer):
+        return obj.last_updated_by.username if obj.last_updated_by else ""
 
 
 class LayerFullSchema(Schema):
@@ -398,11 +386,8 @@ def resolve_urls(obj):
         }
 
     @staticmethod
-    def resolve_mask(obj):
-        if obj.layerset2 and obj.layerset2.multimask and obj.slug in obj.layerset2.multimask:
-            return obj.layerset2.multimask[obj.slug]
-        else:
-            return None
+    def resolve_mask(obj: Layer):
+        return obj.mask.geojson if obj.mask else None
 
     @staticmethod
     def resolve_image_url(obj):

ohmg/core/management/commands/layerset.py

@@ -39,4 +39,4 @@ def handle(self, *args, **options):
             )
 
         if options.operation == "inspect":
-            print(ls.multimask_extent)
+            print(ls, len(ls.get_layers()))

ohmg/core/management/commands/oneoffs.py

@@ -411,22 +411,22 @@ def old_ds_to_new(file_set):
                     print("deleted")
 
         ## Mar 11th, 2026
+        ## Pushing all multimask entries directly onto Layers, now that masks are
+        ## stored on Layers, not as a collected set of GeoJSON in LayerSet.multimask
         elif operation == "add-masks-to-layers":
             from django.contrib.gis.geos import GEOSGeometry
 
             from ohmg.core.models import Layer, LayerSet
 
             for ls in LayerSet.objects.all().order_by("map__title"):
                 print(ls)
-                if ls.multimask:
-                    for k, v in ls.multimask.items():
-                        print(k)
-                        ## there SHOULD only be one layer here, but per ticket
-                        ## #308 some regions got duplicated, and some of the
-                        ## duplicates got georeferenced :(. Can't clean all of
-                        ## that up now, so... using filter here
-                        layers = Layer.objects.filter(slug=k, region__document__map=ls.map)
-                        for layer in layers:
-                            layer.mask = GEOSGeometry(json.dumps(v["geometry"]))
-                            layer.save(skip_map_lookup_update=True, set_extent=False)
-                print("")
+                for layer in ls.get_layers():
+                    print(f"  {layer.slug}")
+                    if ls.multimask and layer.slug in ls.multimask:
+                        print("  -> setting mask from layerset multimask")
+                        mask_geom = ls.multimask[layer.slug]
+                        layer.mask = GEOSGeometry(json.dumps(mask_geom["geometry"]))
+                    else:
+                        print("  -> setting mask to None")
+                        layer.mask = None
+                    layer.save(skip_map_lookup_update=True, set_extent=False)

ohmg/core/models/layer.py

@@ -1,3 +1,4 @@
+import json
 import logging
 import urllib.parse
 from pathlib import Path
@@ -103,6 +104,19 @@ def file_url_encoded(self):
         no COG exists, return None."""
         return urllib.parse.quote(self.file_url, safe="")
 
+    @property
+    def mask_geojson_feature(self) -> Union[dict | None]:
+        """Returns a GeoJSON feature representation of the Layer mask, or None if no mask"""
+        return (
+            {
+                "type": "Feature",
+                "geometry": json.loads(self.mask.geojson),
+                "properties": {"layer": self.slug},
+            }
+            if self.mask
+            else None
+        )
+
     def create_xyz_url(self) -> Union[str, None]:
         file_url = get_file_url(self)
         if file_url:
@@ -145,16 +159,7 @@ def set_layerset(self, layerset):
 
         # make sure to clean up the existing multimask in the current vrs if necessary
         existing_obj = LayerSet.objects.get(pk=self.layerset2.pk) if self.layerset2 else None
-        delete_existing = False
-        if existing_obj:
-            if existing_obj.multimask and self.slug in existing_obj.multimask:
-                del existing_obj.multimask[self.slug]
-                existing_obj.save(update_fields=["multimask"])
-                logger.info(
-                    f"Layer {self.pk} removed from existing multimask in LayerSet {existing_obj.pk}"
-                )
-            if existing_obj.get_layers().count() == 1:
-                delete_existing = True
+        delete_existing = existing_obj and existing_obj.get_layers().count() == 1
         self.layerset2 = layerset
         self.save(update_fields=["layerset2"])
         logger.info(f"Layer {self.pk} added to LayerSet {self.layerset2} ({self.layerset2.pk})")

ohmg/core/models/layerset.py

@@ -1,11 +1,10 @@
-import json
 import logging
 import urllib.parse
 from typing import TYPE_CHECKING, Iterable, Union
 
 from django.conf import settings
 from django.contrib.gis.db import models
-from django.contrib.gis.geos import GEOSGeometry, MultiPolygon, Polygon
+from django.contrib.gis.geos import MultiPolygon, Polygon
 from django.contrib.postgres.fields import ArrayField
 from django.utils.functional import cached_property
 from django.utils.safestring import mark_safe
@@ -112,67 +111,25 @@ def create_xyz_url(self) -> Union[str, None]:
 
     @property
     def multimask_extent(self):
-        """Calculate an extent based on all layers in this layerset's
-        multimask. If there is no multimask, return None."""
-        extent = None
-        if self.multimask:
-            feature_polygons = []
-            for v in self.multimask.values():
-                poly = Polygon(v["geometry"]["coordinates"][0])
-                feature_polygons.append(poly)
-            if len(feature_polygons) > 0:
-                extent = MultiPolygon(feature_polygons, srid=4326).extent
-        return extent
+        """Calculate an extent based any existing masks of layers in this LayerSet.
+        If no layer have masks then return None."""
+        feature_polygons = []
+        for feat in self.multimask_geojson["features"]:
+            poly = Polygon(feat["geometry"]["coordinates"][0])
+            feature_polygons.append(poly)
+        return (
+            MultiPolygon(feature_polygons, srid=4326).extent if len(feature_polygons) > 0 else None
+        )
 
     @property
-    def multimask_geojson(self):
-        if self.multimask:
-            multimask_geojson = {"type": "FeatureCollection", "features": []}
-            for layer, geojson in self.multimask.items():
-                geojson["properties"] = {"layer": layer}
-                multimask_geojson["features"].append(geojson)
-            return multimask_geojson
-        else:
-            return None
-
-    def validate_multimask_geojson(self, multimask_geojson):
-        errors = []
-        for feature in multimask_geojson["features"]:
-            lyr = feature["properties"]["layer"]
-            try:
-                geom_str = json.dumps(feature["geometry"])
-                g = GEOSGeometry(geom_str)
-                if not g.valid:
-                    logger.warning(f"{self} | invalid mask: {lyr} - {g.valid_reason}")
-                    errors.append((lyr, g.valid_reason))
-            except Exception as e:
-                logger.warning(f"{self} | improper GeoJSON in multimask")
-                errors.append((lyr, e))
-        return errors
-
-    def update_multimask_from_geojson(self, multimask_geojson):
-        from .layer import Layer
-
-        errors = self.validate_multimask_geojson(multimask_geojson)
-        if errors:
-            return errors
-
-        if multimask_geojson["features"]:
-            self.multimask = {}
-            for feature in multimask_geojson["features"]:
-                layer_slug = feature["properties"]["layer"]
-                self.multimask[feature["properties"]["layer"]] = feature
-
-                ## future patch: save mask directly to layers
-                layer = Layer.objects.get(slug=layer_slug, region__document__map=self.map)
-                new_mask = GEOSGeometry(json.dumps(feature["geometry"]))
-                if new_mask != layer.mask:
-                    logger.debug(f"updating mask on layer {layer.slug} ({layer.pk})")
-                    layer.mask = new_mask
-                    layer.save(skip_map_lookup_update=True, set_extent=False)
-        else:
-            self.multimask = None
-        self.save(update_fields=["multimask"])
+    def multimask_geojson(self) -> dict:
+        """Collect all masks from layers in this layerset and return as GeoJSON Feature Collection"""
+        fc = {"type": "FeatureCollection", "features": []}
+        for layer in self.get_layers():
+            mask_geojson = layer.mask_geojson_feature
+            if mask_geojson:
+                fc["features"].append(mask_geojson)
+        return fc
 
     def save(self, set_tilejson: bool = False, *args, **kwargs):
         if self._state.adding is False:

ohmg/core/models/map.py

@@ -232,8 +232,8 @@ def stats(self):
             # make sure 0/0 appears at the very bottom, then 0/1, 0/2, etc.
             mm_percent = main_lyrs_ct * 0.000001
         mm_display = f"0/{main_lyrs_ct}"
-        if main_layerset and main_layerset.multimask is not None:
-            mm_ct = len(main_layerset.multimask)
+        if main_layerset:
+            mm_ct = len(main_layerset.multimask_geojson["features"])
             mm_todo = main_lyrs_ct - mm_ct
             if mm_ct > 0 and main_lyrs_ct > 0:
                 mm_display = f"{mm_ct}/{main_lyrs_ct}"
@@ -393,8 +393,8 @@ def update_item_lookup(self):
             # make sure 0/0 appears at the very bottom, then 0/1, 0/2, etc.
             multimask_rank = main_lyrs_ct * 0.000001
 
-        if main_layerset and main_layerset.multimask is not None:
-            multimask_ct = len(main_layerset.multimask)
+        if main_layerset:
+            multimask_ct = len(main_layerset.multimask_geojson["features"])
             if multimask_ct > 0 and main_lyrs_ct > 0:
                 pct = multimask_ct / main_lyrs_ct
                 multimask_rank += pct * 0.000001

ohmg/core/signals.py

@@ -52,13 +52,6 @@ def handle_layer_deletion(sender, instance, **kwargs):
     except Exception as e:
         logger.warning(f"instance.region.gcpgroup.delete(): {e}")
 
-    # remove layer mask from layerset if present
-    if instance.layerset2:
-        if instance.layerset2.multimask:
-            if instance.slug in instance.layerset2.multimask:
-                del instance.layerset2.multimask[instance.slug]
-                instance.layerset2.save()
-
     # remove layerset if this was the last layer attached to it
     if instance.layerset2:
         if instance.layerset2.get_layers().count() == 0:

ohmg/core/views.py

@@ -4,7 +4,7 @@
 
 from django.conf import settings
 from django.contrib.auth.decorators import login_required
-from django.contrib.gis.geos import Polygon
+from django.contrib.gis.geos import GEOSGeometry, Polygon
 from django.http import FileResponse, Http404, JsonResponse
 from django.shortcuts import HttpResponse, get_object_or_404, redirect, render
 from django.utils.decorators import method_decorator
@@ -415,11 +415,7 @@ def get(self, request, pk, derivative, resource=""):
 
 
 class LayerSetView(View):
-    @method_decorator(
-        validate_post_request(
-            operations=["bulk-classify-layers", "check-for-existing-mask", "set-mask"]
-        )
-    )
+    @method_decorator(validate_post_request(operations=["bulk-classify-layers", "set-mask"]))
     def post(self, request):
         body = json.loads(request.body)
         operation = body.get("operation")
@@ -442,31 +438,44 @@ def post(self, request):
             else:
                 return JsonResponseSuccess("Layers classified successfully.")
 
-        if operation == "check-for-existing-mask":
-            r = get_object_or_404(Layer, pk=payload.get("resource-id"))
-
-            if r.layerset2:
-                if not r.layerset2.category.slug == payload.get("category"):
-                    if r.layerset2.multimask and r.slug in r.layerset2.multimask:
-                        return JsonResponseFail(
-                            f"Layer already in {r.layerset2.category} multimask.",
-                            payload=payload,
-                        )
-
-            return JsonResponseSuccess(payload=payload)
-
         if operation == "set-mask":
-            try:
-                layerset = LayerSet.objects.get(
-                    map_id=payload["map-id"], category__slug=payload["category"]
-                )
-                errors = layerset.update_multimask_from_geojson(payload["multimask-geojson"])
-                if errors:
-                    return JsonResponseFail("; ".join([f"\n-- {i[0]}: {i[1]}" for i in errors]))
+            layerset = LayerSet.objects.get(
+                map_id=payload["map-id"], category__slug=payload["category"]
+            )
+
+            ## first validate the incoming geojson for each mask
+            errors = []
+            geom_lookup = {}
+            for feature in payload["multimask-geojson"]["features"]:
+                lyr_slug = feature["properties"]["layer"]
+                try:
+                    geom_str = json.dumps(feature["geometry"])
+                    g = GEOSGeometry(geom_str)
+                    if not g.valid:
+                        logger.warning(f"{layerset} | invalid mask: {lyr_slug} - {g.valid_reason}")
+                        errors.append((lyr_slug, g.valid_reason))
+                    geom_lookup[lyr_slug] = g
+                except Exception as e:
+                    logger.warning(f"{self} | improper GeoJSON in mask")
+                    errors.append((lyr_slug, e))
+            if errors:
+                return JsonResponseFail("; ".join([f"\n-- {i[0]}: {i[1]}" for i in errors]))
+
+            for layer in layerset.get_layers():
+                # if there is no mask to set and the layer doesn't need a mask removed, skip
+                if layer.mask is None and layer.slug not in geom_lookup:
+                    continue
+                # update all layers with provided masks
+                if layer.slug in geom_lookup:
+                    if layer.mask != geom_lookup[layer.slug]:
+                        layer.mask = geom_lookup[layer.slug]
+                        logger.debug(f"updating mask on layer {layer.slug} ({layer.pk})")
+                # remove mask from any layers that previously had one but has since been deleted
                 else:
-                    return JsonResponseSuccess()
-            except LayerSet.DoesNotExist:
-                return JsonResponseNotFound()
+                    logger.debug(f"removing mask from layer {layer.slug} ({layer.pk})")
+                    layer.mask = None
+                layer.save(set_extent=False, skip_map_lookup_update=True)
+            return JsonResponseSuccess()
 
 
 class LayersetDerivativeView(View):