Use numpoints constraint on Zone.polygon instead of memsize

arkindex/documents/tests/test_create_elements.py

@@ -3,7 +3,7 @@ from rest_framework import status
 
 from arkindex.dataimport.models import WorkerVersion
 from arkindex.documents.models import Corpus, Element
-from arkindex.images.models import ImageServer
+from arkindex.images.models import POLYGON_MAX_POINTS, ImageServer
 from arkindex.project.aws import S3FileStatus
 from arkindex.project.tests import FixtureAPITestCase
 from arkindex.users.models import Role
@@ -149,9 +149,9 @@ class TestCreateElements(FixtureAPITestCase):
         self.assertEqual(act.worker_version, self.worker_version)
 
     def test_create_element_polygon_max_size(self):
-        # Element polygon is limited to 300 points
-        polygon = [(0, 0), *((i, i) for i in range(299, -1, -1))]
-        self.assertEqual(len(polygon), 301)
+        # Element polygon is limited to POLYGON_MAX_POINTS points
+        polygon = [(0, 0), *((i, i) for i in range(POLYGON_MAX_POINTS - 1, -1, -1))]
+        self.assertEqual(len(polygon), POLYGON_MAX_POINTS + 1)
         self.client.force_login(self.user)
         request = self.make_create_request(
             parent=str(self.vol.id),

arkindex/images/migrations/0004_polygon_postgis.py

@@ -22,9 +22,6 @@ CREATE UNIQUE INDEX zone_unique_image_polygon ON images_zone (image_id, polygon_
 # we will try with tolerances from 1 to 20 to be safe.
 MAX_TOLERANCE = 20
 
-# This was added after we removed the maximum memory size constraint from the database.
-POLYGON_MAX_MEM_SIZE = 2664
-
 
 def simplify_polygons(apps, schema_editor):
     Zone = apps.get_model('images', 'Zone')
@@ -32,17 +29,17 @@ def simplify_polygons(apps, schema_editor):
         return
 
     print('Looking for polygons exceeding the maximum allowed size…')
-    count = Zone.objects.filter(polygon__memsize__gt=POLYGON_MAX_MEM_SIZE).count()
+    count = Zone.objects.filter(polygon__memsize__gt=2664).count()
     if not count:
         return
 
     for tolerance in range(1, MAX_TOLERANCE - 1):
         print(f'Simplifying {count} polygons with tolerance {tolerance}…')
         Zone.objects \
-            .filter(polygon__memsize__gt=POLYGON_MAX_MEM_SIZE) \
+            .filter(polygon__memsize__gt=2664) \
             .update(polygon=SimplifyPreserveTopology('polygon', tolerance))
 
-        count = Zone.objects.filter(polygon__memsize__gt=POLYGON_MAX_MEM_SIZE).count()
+        count = Zone.objects.filter(polygon__memsize__gt=2664).count()
         if not count:
             print('Simplification successful!')
             return

arkindex/images/migrations/0005_polygon_index.py

@@ -3,8 +3,6 @@
 from django.contrib.gis.db.models.functions import SnapToGrid
 from django.db import migrations, models
 
-POLYGON_MAX_MEM_SIZE = 2664
-
 
 class Migration(migrations.Migration):
 
@@ -16,7 +14,7 @@ class Migration(migrations.Migration):
         migrations.AddConstraint(
             model_name='zone',
             constraint=models.CheckConstraint(
-                check=models.Q(polygon__memsize__lte=POLYGON_MAX_MEM_SIZE),
+                check=models.Q(polygon__memsize__lte=2664),
                 name='zone_polygon_size',
             ),
         ),

arkindex/images/migrations/0007_remove_max_memsize.py → arkindex/images/migrations/0007_update_zone_constraint.py

@@ -16,7 +16,7 @@ class Migration(migrations.Migration):
         ),
         migrations.AddConstraint(
             model_name='zone',
-            constraint=models.CheckConstraint(check=models.Q(polygon__memsize__gte=96), name='zone_polygon_size'),
+            constraint=models.CheckConstraint(check=models.Q(('polygon__numpoints__gte', 4), ('polygon__numpoints__lte', 300)), name='zone_polygon_size'),
         ),
         migrations.AlterModelOptions(
             name='zone',

arkindex/images/models.py

@@ -36,6 +36,8 @@ from arkindex.project.tools import bounding_box
 # and fit polygons with more than 1000 points, we chose to restrict to an arbitrary number of points and handle the
 # Postgres index errors directly to give explicit error messages to users when they are unlucky.
 # Therefore, polygons are limited to 300 points (299 distinct points as the last point must be equal to the first).
+# To clarify, we assume only raisonable raster coordinates will be stored (e.g. lower than 10000px)
+# and catch a potential B-Tree index overflow.
 POLYGON_MAX_POINTS = 300
 
 logger = logging.getLogger(__name__)
@@ -391,10 +393,10 @@ class Zone(IndexableModel):
                 check=models.Q(polygon=SnapToGrid('polygon', 1)),
                 name='zone_polygon_integer_coordinates',
             ),
-            # Restrict to at least 4 points (3+1 points for a triangle)
-            # Checking against the memory size is faster than checking the number of points
+            # Restrict the number of points to between 3+1 points for a triangle and an
+            # empirical limit of POLYGON_MAX_POINTS points due to BTree indexes.
             models.CheckConstraint(
-                check=models.Q(polygon__memsize__gte=96),
+                check=models.Q(polygon__numpoints__gte=4, polygon__numpoints__lte=POLYGON_MAX_POINTS),
                 name='zone_polygon_size',
             )
         ]

arkindex/images/tests/test_image_polygon.py

 import json
+import random
 from pathlib import Path
 
+from django.db.utils import IntegrityError
 from django.urls import reverse
 from rest_framework import status
 
 from arkindex.dataimport.models import WorkerVersion
 from arkindex.images.managers import PolygonTooLargeError
-from arkindex.images.models import Image, Zone
+from arkindex.images.models import POLYGON_MAX_POINTS, Image, Zone
 from arkindex.project.gis import ensure_linear_ring
 from arkindex.project.tests import FixtureAPITestCase
 
@@ -28,9 +30,24 @@ class TestImagePolygon(FixtureAPITestCase):
         super().setUp()
         self.client.force_login(self.superuser)
 
-    def test_polygon_164_pts(self):
-        polygon = [(0, 0), *((i, i) for i in range(162, -1, -1))]
-        self.assertEqual(len(polygon), 164)
+    def test_polygon_num_pts_constraint(self):
+        """
+        Polygon maximum allowed number of points is empirically set to POLYGON_MAX_POINTS
+        """
+        # Create a polygon with POLYGON_MAX_POINTS + 1 points
+        polygon = [(0, 0), *((i, i) for i in range(POLYGON_MAX_POINTS - 1, -1, -1))]
+        self.assertEqual(len(polygon), POLYGON_MAX_POINTS + 1)
+        self.assertEqual(polygon[0], polygon[-1])
+        with self.assertRaisesRegex(IntegrityError, '"images_zone" violates check constraint "zone_polygon_size"'):
+            Zone(image=self.image, polygon=polygon).save()
+
+    def test_polygon_POLYGON_MAX_POINTS_pts(self):
+        """
+        Ensure a Polygon of POLYGON_MAX_POINTS points with a rather good entropy can be saved
+        """
+        random.seed(1337)
+        polygon = [(0, 0), *((random.randint(0, 100000), i) for i in range(1, POLYGON_MAX_POINTS - 1)), (0, 0)]
+        self.assertEqual(len(polygon), POLYGON_MAX_POINTS)
         self.assertEqual(polygon[0], polygon[-1])
         Zone(image=self.image, polygon=polygon).save()
 

arkindex/project/gis.py

@@ -4,6 +4,7 @@ from itertools import groupby
 from django.contrib.gis.db.models.fields import BaseSpatialField, LineStringField
 from django.contrib.gis.db.models.functions import GeoFuncMixin, GeomOutputGeoFunc
 from django.contrib.gis.db.models.functions import MemSize as MemSizeFunc
+from django.contrib.gis.db.models.functions import NumPoints as NumPointsFunc
 from django.contrib.gis.db.models.lookups import EqualsLookup
 from django.contrib.gis.geos import GEOSGeometry, LineString, Polygon
 from django.db.models import BooleanField, Transform
@@ -65,12 +66,24 @@ class IsClosed(GeoFuncMixin, Transform):
     output_field = BooleanField()
 
 
+@BaseSpatialField.register_lookup
+class NumPoints(NumPointsFunc, Transform):
+    """
+    Number of points in a geometry.
+    Provided by Django, but modified for use as a Transform in a queryset:
+    Zone.objects.filter(polygon__numpoints=42)
+
+    https://postgis.net/docs/ST_NumPoints.html
+    https://docs.djangoproject.com/en/3.1/ref/contrib/gis/functions/#numpoints
+    """
+    lookup_name = 'numpoints'
+
+
 @BaseSpatialField.register_lookup
 class MemSize(MemSizeFunc, Transform):
     """
     Size, in bytes, of a geometry.
-    Provided by Django, but modified for use as a Transform in a queryset:
-    Zone.objects.filter(polygon__memsize=42)
+    This lookup is not used anymore except for migrations.
 
     https://postgis.net/docs/ST_MemSize.html
     https://docs.djangoproject.com/en/3.1/ref/contrib/gis/functions/#memsize

arkindex/project/tests/test_drf_linear_ring_field.py

@@ -3,6 +3,7 @@ from unittest import TestCase
 from django.contrib.gis.geos import LinearRing, Point
 from rest_framework.serializers import ValidationError
 
+from arkindex.images.models import POLYGON_MAX_POINTS
 from arkindex.project.serializer_fields import LinearRingField, PointField
 
 
@@ -80,19 +81,23 @@ class TestLinearRingSerializerField(TestCase):
             LinearRingField().run_validation([[1, 2]])
 
     def test_linear_ring_max_size(self):
-        # 0,0 + 298 points from 299,299 to 1,1
-        coords = [[0, 0]] + [[i, i] for i in range(299, 1, -1)]
+        """
+        Polygon maximum allowed number of points is empirically set to POLYGON_MAX_POINTS
+        """
+        # Create a polygon with POLYGON_MAX_POINTS - 1 points
+        coords = [(0, 0), *((i, i) for i in range(POLYGON_MAX_POINTS - 2, 0, -1))]
+        self.assertEqual(len(coords), POLYGON_MAX_POINTS - 1)
         self.assertEqual(
             LinearRingField().run_validation(coords).coords,
             LinearRing(coords + [[0, 0]]).coords,
         )
 
-        # 300th point, should fail if it is not equal to the first one
-        coords.append([299, 299])
-        with self.assertRaisesRegex(ValidationError, '300 points are allowed only when the first and last points are equal.'):
+        # Next point makes the validation fail if it is not equal to the first one
+        coords.append([POLYGON_MAX_POINTS + 1, POLYGON_MAX_POINTS + 1])
+        with self.assertRaisesRegex(ValidationError, f'{POLYGON_MAX_POINTS} points are allowed only when the first and last points are equal.'):
             LinearRingField().run_validation(coords)
 
-        coords[299] = [0, 0]
+        coords[-1] = [0, 0]
         self.assertEqual(
             LinearRingField().run_validation(coords).coords,
             LinearRing(coords).coords,