From e4bb32231ce363c940a11cfe48dcaf5e54eb52b5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20P=C3=A9ters?= <fpeters@entrouvert.com>
Date: Mon, 27 Sep 2021 13:54:21 +0200
Subject: [PATCH] opengis: add support for geojson shapes (#57280)

---
 .../migrations/0014_auto_20210927_1006.py     | 23 +++++
 passerelle/apps/opengis/models.py             | 83 +++++++++++++++---
 tests/test_opengis.py                         | 85 ++++++++++++++++---
 3 files changed, 169 insertions(+), 22 deletions(-)
 create mode 100644 passerelle/apps/opengis/migrations/0014_auto_20210927_1006.py

diff --git a/passerelle/apps/opengis/migrations/0014_auto_20210927_1006.py b/passerelle/apps/opengis/migrations/0014_auto_20210927_1006.py
new file mode 100644
index 00000000..a2090abb
--- /dev/null
+++ b/passerelle/apps/opengis/migrations/0014_auto_20210927_1006.py
@@ -0,0 +1,23 @@
+# Generated by Django 2.2.21 on 2021-09-27 08:06
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('opengis', '0013_remove_query_index_properties'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='featurecache',
+            name='bbox_lat2',
+            field=models.FloatField(blank=True, null=True),
+        ),
+        migrations.AddField(
+            model_name='featurecache',
+            name='bbox_lon2',
+            field=models.FloatField(blank=True, null=True),
+        ),
+    ]
diff --git a/passerelle/apps/opengis/models.py b/passerelle/apps/opengis/models.py
index 4d667558..eb01c4e5 100644
--- a/passerelle/apps/opengis/models.py
+++ b/passerelle/apps/opengis/models.py
@@ -23,6 +23,7 @@ import six
 from django.contrib.postgres.fields import JSONField
 from django.core.cache import cache
 from django.db import models, transaction
+from django.db.models import Q
 from django.http import HttpResponse
 from django.shortcuts import get_object_or_404
 from django.template import Context, Template
@@ -522,6 +523,7 @@ class Query(BaseQuery):
                 filters[lookup] = value
         features = features.filter(**filters)
 
+        lonmin, latmin, lonmax, latmax = None, None, None, None
         if bbox:
             try:
                 lonmin, latmin, lonmax, latmax = (float(x) for x in bbox.split(','))
@@ -529,7 +531,6 @@ class Query(BaseQuery):
                 raise APIError(
                     'Invalid bbox parameter, it must be a comma separated list of ' 'floating point numbers.'
                 )
-            features = features.filter(lon__gte=lonmin, lon__lte=lonmax, lat__gte=latmin, lat__lte=latmax)
 
         if circle:
             try:
@@ -541,7 +542,22 @@ class Query(BaseQuery):
                 )
             coords = self.get_bbox_containing_circle(center_lon, center_lat, radius)
             lonmin, latmin, lonmax, latmax = coords
-            features = features.filter(lon__gte=lonmin, lon__lte=lonmax, lat__gte=latmin, lat__lte=latmax)
+
+        if lonmin is not None:
+            # adjust lonmin, latmin, lonmax, latmax to make sure min are min and max are max
+            lonmin, lonmax = min(lonmin, lonmax), max(lonmin, lonmax)
+            latmin, latmax = min(latmin, latmax), max(latmin, latmax)
+
+            features = features.filter(
+                Q(bbox_lat2__isnull=True, lon__gte=lonmin, lon__lte=lonmax, lat__gte=latmin, lat__lte=latmax)
+                | Q(
+                    bbox_lat2__isnull=False,  # geometry != Point
+                    lon__lte=lonmax,
+                    bbox_lon2__gte=lonmin,
+                    lat__lte=latmax,
+                    bbox_lat2__gte=latmin,
+                )
+            )
 
         if q:
             features = features.filter(text__search=simplify(q))
@@ -550,9 +566,13 @@ class Query(BaseQuery):
         if circle:
             results = []
             for feature in features:
-                distance = self.get_coords_distance(feature.lat, feature.lon, center_lat, center_lon)
-                if distance < radius:
+                if feature.bbox_lat2:  # not a point
                     results.append(feature.data)
+                else:
+                    distance = self.get_coords_distance(feature.lat, feature.lon, center_lat, center_lon)
+                    if distance < radius:
+                        results.append(feature.data)
+
             data['features'] = results
         else:
             data['features'] = list(features.values_list('data', flat=True))
@@ -598,18 +618,57 @@ class Query(BaseQuery):
             template = Template(self.indexing_template)
         for feature in data['data']:
             geometry = feature.get('geometry') or {}
-            if geometry.get('type') != 'Point':
-                continue
-            try:
-                lon, lat = geometry['coordinates']
-            except (KeyError, TypeError):
-                self.resource.logger.warning('invalid coordinates in geometry: %s', geometry)
+            if not geometry:
                 continue
+            if geometry.get('type') == 'Point':
+                try:
+                    lon, lat = geometry['coordinates']
+                except (KeyError, TypeError):
+                    self.resource.logger.warning('invalid coordinates in geometry: %s', geometry)
+                    continue
+                lon2, lat2 = None, None
+            else:
+                # define bbox, lat/lon as min values, bbox_lat2/bbox_lon2 as max values
+                min_lat, min_lon, max_lat, max_lon = None, None, None, None
+
+                def add_coordinates(coordinates):
+                    nonlocal min_lat, min_lon, max_lat, max_lon
+                    if not coordinates:
+                        return
+                    if not isinstance(coordinates[0], (float, int)):
+                        for child in coordinates:
+                            add_coordinates(child)
+                        return
+
+                    # position
+                    lon, lat = coordinates
+                    if min_lat is None or lat < min_lat:
+                        min_lat = lat
+                    if max_lat is None or lat > max_lat:
+                        max_lat = lat
+                    if min_lon is None or lon < min_lon:
+                        min_lon = lon
+                    if max_lon is None or lon > max_lon:
+                        max_lon = lon
+
+                add_coordinates(geometry['coordinates'])
+                lat, lon, lat2, lon2 = min_lat, min_lon, max_lat, max_lon
+
             text = ''
             if self.indexing_template:
                 context = Context(feature.get('properties', {}))
                 text = simplify(template.render(context))
-            features.append(FeatureCache(query=self, lat=lat, lon=lon, text=text, data=feature))
+            features.append(
+                FeatureCache(
+                    query=self,
+                    lat=lat,
+                    lon=lon,
+                    bbox_lat2=lat2,
+                    bbox_lon2=lon2,
+                    text=text,
+                    data=feature,
+                )
+            )
         with transaction.atomic():
             self.features.all().delete()
             FeatureCache.objects.bulk_create(features)
@@ -625,5 +684,7 @@ class FeatureCache(models.Model):
     )
     lat = models.FloatField()
     lon = models.FloatField()
+    bbox_lat2 = models.FloatField(blank=True, null=True)
+    bbox_lon2 = models.FloatField(blank=True, null=True)
     text = models.CharField(max_length=2048)
     data = JSONField()
diff --git a/tests/test_opengis.py b/tests/test_opengis.py
index 2626121b..14dbbf00 100644
--- a/tests/test_opengis.py
+++ b/tests/test_opengis.py
@@ -246,9 +246,50 @@ FAKE_GEOLOCATED_FEATURE = '''{
                 "numero": 4
             },
             "type": "Feature"
+        },
+        {
+            "geometry": {
+                "coordinates": [
+                    [
+                        [1914018, 4224644],
+                        [1914018, 4224844],
+                        [1914318, 4224944],
+                        [1914318, 4224544]
+                    ]
+                ],
+                "type": "Polygon"
+            },
+            "geometry_name": "the_geom",
+            "properties": {
+                "code_insee": 38185,
+                "code_post": 38000,
+                "nom_commune": "Grenoble",
+                "nom_square": "place trapeze"
+            },
+            "type": "Feature"
+        },
+        {
+            "geometry": {
+                "coordinates": [
+                    [
+                        [1914059, 4224699],
+                        [1914059, 4224899],
+                        [1914259, 4224699]
+                    ]
+                ],
+                "type": "Polygon"
+            },
+            "geometry_name": "the_geom",
+            "properties": {
+                "code_insee": 38185,
+                "code_post": 38000,
+                "nom_commune": "Grenoble",
+                "nom_square": "place triangle"
+            },
+            "type": "Feature"
         }
     ],
-    "totalFeatures": 4,
+    "totalFeatures": 6,
     "type": "FeatureCollection"
 }'''
 
@@ -288,6 +329,28 @@ FAKE_GEOLOCATED_FEATURE_CIRCLE = {
             },
             'geometry': {'type': 'Point', 'coordinates': [2.304, 48.8086]},
         },
+        {
+            'type': 'Feature',
+            'properties': {
+                'in-circle': True,
+                'in-bbox': True,
+            },
+            'geometry': {
+                'type': 'Polygon',
+                'coordinates': [[[2.304, 48.8086], [2.304, 49.8086], [1.304, 48.8086]]],
+            },
+        },
+        {
+            'type': 'Feature',
+            'properties': {
+                'in-circle': False,
+                'in-bbox': False,
+            },
+            'geometry': {
+                'type': 'Polygon',
+                'coordinates': [[[-2.304, 48.8086], [-2.304, 49.8086], [-1.304, 48.8086]]],
+            },
+        },
     ],
 }
 
@@ -599,7 +662,7 @@ def test_opengis_query_cache_update(mocked_get, app, connector, query):
 
     assert mocked_get.call_args[1]['params']['filter'] == query.filter_expression
     assert mocked_get.call_args[1]['params']['typenames'] == query.typename
-    assert FeatureCache.objects.count() == 4
+    assert FeatureCache.objects.count() == 6
 
     feature = FeatureCache.objects.get(lon=1914059.51, lat=4224699.2)
     assert feature.data['properties']['code_post'] == 38000
@@ -622,7 +685,7 @@ def test_opengis_query_q_endpoint(mocked_get, app, connector, query):
     assert feature_data == feature.data
 
     resp = app.get(endpoint + '?bbox=1914041,4224660,1914060,4224670')
-    assert len(resp.json['features']) == 1
+    assert len(resp.json['features']) == 2
 
     resp = app.get(endpoint + '?bbox=wrong')
     assert resp.json['err'] == 1
@@ -632,7 +695,7 @@ def test_opengis_query_q_endpoint(mocked_get, app, connector, query):
 def test_opengis_query_cache_update_change(mocked_get, app, connector, query):
     mocked_get.side_effect = geoserver_geolocated_responses
     query.update_cache()
-    assert FeatureCache.objects.count() == 4
+    assert FeatureCache.objects.count() == 6
 
     def new_response(url, **kwargs):
         if kwargs['params'].get('request') == 'GetCapabilities':
@@ -669,7 +732,7 @@ def test_opengis_query_cache_update_jobs(mocked_get, app, connector, query):
     assert not FeatureCache.objects.exists()
 
     connector.jobs()
-    assert FeatureCache.objects.count() == 4
+    assert FeatureCache.objects.count() == 6
     job.refresh_from_db()
     assert job.status == 'completed'
 
@@ -704,7 +767,7 @@ def test_opengis_query_cache_update_daily(mocked_get, app, connector, query):
     mocked_get.side_effect = geoserver_geolocated_responses
     assert not FeatureCache.objects.exists()
     call_command('cron', 'daily')
-    assert FeatureCache.objects.count() == 4
+    assert FeatureCache.objects.count() == 6
 
 
 @mock.patch('passerelle.utils.Request.get')
@@ -729,7 +792,7 @@ def test_opengis_query_text_search(mocked_get, app, connector, query):
     query.update_cache()
 
     resp = app.get(endpoint + '?q=grenoble')
-    assert len(resp.json['features']) == 4
+    assert len(resp.json['features']) == 6
 
     resp = app.get(endpoint + '?q=victor')
     assert len(resp.json['features']) == 2
@@ -746,7 +809,7 @@ def test_opengis_query_text_search(mocked_get, app, connector, query):
     query.update_cache()
 
     resp = app.get(endpoint + '?q=plop')
-    assert len(resp.json['features']) == 4
+    assert len(resp.json['features']) == 6
 
 
 @mock.patch('passerelle.utils.Request.get')
@@ -762,12 +825,12 @@ def test_opengis_query_circle_filtering(mocked_get, app, connector, query):
     bbox = Query.get_bbox_containing_circle(center_lon, center_lat, float(radius))
     resp = app.get(endpoint + '?bbox=' + ','.join((str(x) for x in bbox)))
     features = resp.json['features']
-    assert len(features) == 3
+    assert len(features) == 4
     assert all(feature['properties']['in-circle'] or feature['properties']['in-bbox'] for feature in features)
 
     resp = app.get(endpoint + '?circle=%s,%s,%s' % (center_lon, center_lat, radius))
     features = resp.json['features']
-    assert len(features) == 2
+    assert len(features) == 3
     assert all(feature['properties']['in-circle'] for feature in features)
 
 
@@ -778,7 +841,7 @@ def test_opengis_query_data_filter(mocked_get, app, connector, query):
     query.update_cache()
 
     resp = app.get(endpoint + '?property:code_insee=38185')
-    assert len(resp.json['features']) == 4
+    assert len(resp.json['features']) == 6
 
     resp = app.get(endpoint + '?property:nom_voie=place victor hugo')
     assert len(resp.json['features']) == 2
-- 
2.33.0

