From c12a00e554c8b4b93931b520bb94a479c5ba8706 Mon Sep 17 00:00:00 2001 From: Daniel Wiesmann Date: Wed, 6 Apr 2016 12:50:25 +0100 Subject: [PATCH] Fixed #26455 -- Allowed filtering and repairing invalid geometries. Added the IsValid and MakeValid database functions, and the isvalid lookup, all for PostGIS. Thanks Tim Graham for the review. --- .../contrib/gis/db/backends/base/features.py | 4 ++ .../gis/db/backends/base/operations.py | 8 ++-- .../gis/db/backends/mysql/operations.py | 2 +- .../gis/db/backends/oracle/operations.py | 2 +- .../gis/db/backends/postgis/operations.py | 3 ++ .../gis/db/backends/spatialite/operations.py | 2 +- django/contrib/gis/db/models/fields.py | 2 +- django/contrib/gis/db/models/functions.py | 10 ++++- django/contrib/gis/db/models/lookups.py | 15 ++++++- docs/ref/contrib/gis/functions.txt | 39 +++++++++++++++---- docs/ref/contrib/gis/geoquerysets.txt | 19 +++++++++ docs/releases/1.10.txt | 6 +++ tests/gis_tests/geoapp/test_functions.py | 19 +++++++++ tests/gis_tests/geoapp/tests.py | 7 ++++ 14 files changed, 121 insertions(+), 17 deletions(-) diff --git a/django/contrib/gis/db/backends/base/features.py b/django/contrib/gis/db/backends/base/features.py index 027baa5575..451b515625 100644 --- a/django/contrib/gis/db/backends/base/features.py +++ b/django/contrib/gis/db/backends/base/features.py @@ -64,6 +64,10 @@ class BaseSpatialFeatures(object): def supports_relate_lookup(self): return 'relate' in self.connection.ops.gis_operators + @property + def supports_isvalid_lookup(self): + return 'isvalid' in self.connection.ops.gis_operators + # For each of those methods, the class will have a property named # `has__method` (defined in __init__) which accesses connection.ops # to determine GIS method availability. diff --git a/django/contrib/gis/db/backends/base/operations.py b/django/contrib/gis/db/backends/base/operations.py index 5dafae760d..8fc51da84a 100644 --- a/django/contrib/gis/db/backends/base/operations.py +++ b/django/contrib/gis/db/backends/base/operations.py @@ -58,10 +58,10 @@ class BaseSpatialOperations(object): unsupported_functions = { 'Area', 'AsGeoJSON', 'AsGML', 'AsKML', 'AsSVG', 'BoundingCircle', 'Centroid', 'Difference', 'Distance', 'Envelope', - 'ForceRHR', 'GeoHash', 'Intersection', 'Length', 'MemSize', 'NumGeometries', - 'NumPoints', 'Perimeter', 'PointOnSurface', 'Reverse', 'Scale', - 'SnapToGrid', 'SymDifference', 'Transform', 'Translate', - 'Union', + 'ForceRHR', 'GeoHash', 'Intersection', 'IsValid', 'Length', 'MakeValid', + 'MemSize', 'NumGeometries', 'NumPoints', 'Perimeter', 'PointOnSurface', + 'Reverse', 'Scale', 'SnapToGrid', 'SymDifference', 'Transform', + 'Translate', 'Union', } # Serialization diff --git a/django/contrib/gis/db/backends/mysql/operations.py b/django/contrib/gis/db/backends/mysql/operations.py index 3aedc24040..df4ba36b49 100644 --- a/django/contrib/gis/db/backends/mysql/operations.py +++ b/django/contrib/gis/db/backends/mysql/operations.py @@ -67,7 +67,7 @@ class MySQLOperations(BaseSpatialOperations, DatabaseOperations): def unsupported_functions(self): unsupported = { 'AsGeoJSON', 'AsGML', 'AsKML', 'AsSVG', 'BoundingCircle', - 'ForceRHR', 'GeoHash', 'MemSize', + 'ForceRHR', 'GeoHash', 'IsValid', 'MakeValid', 'MemSize', 'Perimeter', 'PointOnSurface', 'Reverse', 'Scale', 'SnapToGrid', 'Transform', 'Translate', } diff --git a/django/contrib/gis/db/backends/oracle/operations.py b/django/contrib/gis/db/backends/oracle/operations.py index 2404cf9bc4..2a166f23a1 100644 --- a/django/contrib/gis/db/backends/oracle/operations.py +++ b/django/contrib/gis/db/backends/oracle/operations.py @@ -127,7 +127,7 @@ class OracleOperations(BaseSpatialOperations, DatabaseOperations): unsupported_functions = { 'AsGeoJSON', 'AsGML', 'AsKML', 'AsSVG', 'BoundingCircle', 'Envelope', - 'ForceRHR', 'GeoHash', 'MemSize', 'Scale', + 'ForceRHR', 'GeoHash', 'IsValid', 'MakeValid', 'MemSize', 'Scale', 'SnapToGrid', 'Translate', } diff --git a/django/contrib/gis/db/backends/postgis/operations.py b/django/contrib/gis/db/backends/postgis/operations.py index 4c3ba8ca50..89ddc11127 100644 --- a/django/contrib/gis/db/backends/postgis/operations.py +++ b/django/contrib/gis/db/backends/postgis/operations.py @@ -79,6 +79,7 @@ class PostGISOperations(BaseSpatialOperations, DatabaseOperations): 'disjoint': PostGISOperator(func='ST_Disjoint'), 'equals': PostGISOperator(func='ST_Equals'), 'intersects': PostGISOperator(func='ST_Intersects', geography=True), + 'isvalid': PostGISOperator(func='ST_IsValid'), 'overlaps': PostGISOperator(func='ST_Overlaps'), 'relate': PostGISOperator(func='ST_Relate'), 'touches': PostGISOperator(func='ST_Touches'), @@ -118,11 +119,13 @@ class PostGISOperations(BaseSpatialOperations, DatabaseOperations): self.geojson = prefix + 'AsGeoJson' self.gml = prefix + 'AsGML' self.intersection = prefix + 'Intersection' + self.isvalid = prefix + 'IsValid' self.kml = prefix + 'AsKML' self.length = prefix + 'Length' self.length3d = prefix + '3DLength' self.length_spheroid = prefix + 'length_spheroid' self.makeline = prefix + 'MakeLine' + self.makevalid = prefix + 'MakeValid' self.mem_size = prefix + 'mem_size' self.num_geom = prefix + 'NumGeometries' self.num_points = prefix + 'npoints' diff --git a/django/contrib/gis/db/backends/spatialite/operations.py b/django/contrib/gis/db/backends/spatialite/operations.py index f551b67bc6..f2f516109a 100644 --- a/django/contrib/gis/db/backends/spatialite/operations.py +++ b/django/contrib/gis/db/backends/spatialite/operations.py @@ -96,7 +96,7 @@ class SpatiaLiteOperations(BaseSpatialOperations, DatabaseOperations): @cached_property def unsupported_functions(self): - unsupported = {'BoundingCircle', 'ForceRHR', 'MemSize'} + unsupported = {'BoundingCircle', 'ForceRHR', 'IsValid', 'MakeValid', 'MemSize'} if self.spatial_version < (3, 1, 0): unsupported.add('SnapToGrid') if self.spatial_version < (4, 0, 0): diff --git a/django/contrib/gis/db/models/fields.py b/django/contrib/gis/db/models/fields.py index 80fe8628ce..49d86f7618 100644 --- a/django/contrib/gis/db/models/fields.py +++ b/django/contrib/gis/db/models/fields.py @@ -225,7 +225,7 @@ class GeometryField(GeoSelectFormatMixin, BaseSpatialField): returning to the caller. """ value = super(GeometryField, self).get_prep_value(value) - if isinstance(value, Expression): + if isinstance(value, (Expression, bool)): return value elif isinstance(value, (tuple, list)): geom = value[0] diff --git a/django/contrib/gis/db/models/functions.py b/django/contrib/gis/db/models/functions.py index 4f76c155f2..7355bbcbfc 100644 --- a/django/contrib/gis/db/models/functions.py +++ b/django/contrib/gis/db/models/functions.py @@ -6,7 +6,7 @@ from django.contrib.gis.measure import ( Area as AreaMeasure, Distance as DistanceMeasure, ) from django.core.exceptions import FieldError -from django.db.models import FloatField, IntegerField, TextField +from django.db.models import BooleanField, FloatField, IntegerField, TextField from django.db.models.expressions import Func, Value from django.utils import six @@ -282,6 +282,10 @@ class Intersection(OracleToleranceMixin, GeoFuncWithGeoParam): arity = 2 +class IsValid(GeoFunc): + output_field_class = BooleanField + + class Length(DistanceResultMixin, OracleToleranceMixin, GeoFunc): output_field_class = FloatField @@ -319,6 +323,10 @@ class Length(DistanceResultMixin, OracleToleranceMixin, GeoFunc): return super(Length, self).as_sql(compiler, connection) +class MakeValid(GeoFunc): + pass + + class MemSize(GeoFunc): output_field_class = IntegerField arity = 1 diff --git a/django/contrib/gis/db/models/lookups.py b/django/contrib/gis/db/models/lookups.py index a1c85b9bfe..158fb5cfa1 100644 --- a/django/contrib/gis/db/models/lookups.py +++ b/django/contrib/gis/db/models/lookups.py @@ -5,7 +5,7 @@ import re from django.core.exceptions import FieldDoesNotExist from django.db.models.constants import LOOKUP_SEP from django.db.models.expressions import Col, Expression -from django.db.models.lookups import Lookup +from django.db.models.lookups import BuiltinLookup, Lookup from django.utils import six gis_lookups = {} @@ -270,6 +270,19 @@ class IntersectsLookup(GISLookup): gis_lookups['intersects'] = IntersectsLookup +class IsValidLookup(BuiltinLookup): + lookup_name = 'isvalid' + + def as_sql(self, compiler, connection): + gis_op = connection.ops.gis_operators[self.lookup_name] + sql, params = self.process_lhs(compiler, connection) + sql = '%(func)s(%(lhs)s)' % {'func': gis_op.func, 'lhs': sql} + if not self.rhs: + sql = 'NOT ' + sql + return sql, params +gis_lookups['isvalid'] = IsValidLookup + + class OverlapsLookup(GISLookup): lookup_name = 'overlaps' gis_lookups['overlaps'] = OverlapsLookup diff --git a/docs/ref/contrib/gis/functions.txt b/docs/ref/contrib/gis/functions.txt index 04da9e78af..00dee42c57 100644 --- a/docs/ref/contrib/gis/functions.txt +++ b/docs/ref/contrib/gis/functions.txt @@ -25,13 +25,12 @@ Function's summary: ================== ======================= ====================== =================== ================== ===================== Measurement Relationships Operations Editors Output format Miscellaneous ================== ======================= ====================== =================== ================== ===================== -:class:`Area` :class:`BoundingCircle` :class:`Difference` :class:`ForceRHR` :class:`AsGeoJSON` :class:`MemSize` -:class:`Distance` :class:`Centroid` :class:`Intersection` :class:`Reverse` :class:`AsGML` :class:`NumGeometries` -:class:`Length` :class:`Envelope` :class:`SymDifference` :class:`Scale` :class:`AsKML` :class:`NumPoints` -:class:`Perimeter` :class:`PointOnSurface` :class:`Union` :class:`SnapToGrid` :class:`AsSVG` - - :class:`Transform` :class:`GeoHash` - +:class:`Area` :class:`BoundingCircle` :class:`Difference` :class:`ForceRHR` :class:`AsGeoJSON` :class:`IsValid` +:class:`Distance` :class:`Centroid` :class:`Intersection` :class:`MakeValid` :class:`AsGML` :class:`MemSize` +:class:`Length` :class:`Envelope` :class:`SymDifference` :class:`Reverse` :class:`AsKML` :class:`NumGeometries` +:class:`Perimeter` :class:`PointOnSurface` :class:`Union` :class:`Scale` :class:`AsSVG` :class:`NumPoints` + :class:`SnapToGrid` :class:`GeoHash` + :class:`Transform` :class:`Translate` ================== ======================= ====================== =================== ================== ===================== @@ -291,6 +290,18 @@ intersection between them. MySQL support was added. +``IsValid`` +=========== + +.. class:: IsValid(expr) + +.. versionadded:: 1.10 + +*Availability*: PostGIS + +Accepts a geographic field or expression and tests if the value is well formed. +Returns ``True`` if its value is a valid geometry and ``False`` otherwise. + ``Length`` ========== @@ -308,6 +319,20 @@ specify if the calculation should be based on a simple sphere (less accurate, less resource-intensive) or on a spheroid (more accurate, more resource-intensive) with the ``spheroid`` keyword argument. +``MakeValid`` +============= + +.. class:: MakeValid(expr) + +.. versionadded:: 1.10 + +*Availability*: PostGIS + +Accepts a geographic field or expression and attempts to convert the value into +a valid geometry without losing any of the input vertices. Geometries that are +already valid are returned without changes. Simple polygons might become a +multipolygon and the result might be of lower dimension than the input. + ``MemSize`` =========== diff --git a/docs/ref/contrib/gis/geoquerysets.txt b/docs/ref/contrib/gis/geoquerysets.txt index ed142d72b8..6b12c07a57 100644 --- a/docs/ref/contrib/gis/geoquerysets.txt +++ b/docs/ref/contrib/gis/geoquerysets.txt @@ -247,6 +247,25 @@ MySQL ``MBRIntersects(poly, geom)`` SpatiaLite ``Intersects(poly, geom)`` ========== ================================================= +.. fieldlookup:: isvalid + +``isvalid`` +----------- + +.. versionadded:: 1.10 + +*Availability*: PostGIS + +Tests if the geometry is valid. + +Example:: + + Zipcode.objects.filter(poly__isvalid=True) + +PostGIS equivalent:: + + SELECT ... WHERE ST_IsValid(poly) + .. fieldlookup:: overlaps ``overlaps`` diff --git a/docs/releases/1.10.txt b/docs/releases/1.10.txt index 2e97ff3d54..490aa30fb7 100644 --- a/docs/releases/1.10.txt +++ b/docs/releases/1.10.txt @@ -147,6 +147,12 @@ Minor features ` method was added. Band data can now be updated with repeated values efficiently. +* Added database functions + :class:`~django.contrib.gis.db.models.functions.IsValid` and + :class:`~django.contrib.gis.db.models.functions.MakeValid`, as well as the + :lookup:`isvalid` lookup, all for PostGIS. This allows filtering and + repairing invalid geometries on the database side. + :mod:`django.contrib.messages` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tests/gis_tests/geoapp/test_functions.py b/tests/gis_tests/geoapp/test_functions.py index 344fa03e61..c42ed95a86 100644 --- a/tests/gis_tests/geoapp/test_functions.py +++ b/tests/gis_tests/geoapp/test_functions.py @@ -233,6 +233,17 @@ class GISFunctionsTests(TestCase): expected = c.mpoly.intersection(geom) self.assertEqual(c.inter, expected) + @skipUnlessDBFeature("has_IsValid_function") + def test_isvalid(self): + valid_geom = fromstr('POLYGON((0 0, 0 1, 1 1, 1 0, 0 0))') + invalid_geom = fromstr('POLYGON((0 0, 0 1, 1 1, 1 0, 1 1, 1 0, 0 0))') + State.objects.create(name='valid', poly=valid_geom) + State.objects.create(name='invalid', poly=invalid_geom) + valid = State.objects.filter(name='valid').annotate(isvalid=functions.IsValid('poly')).first() + invalid = State.objects.filter(name='invalid').annotate(isvalid=functions.IsValid('poly')).first() + self.assertEqual(valid.isvalid, True) + self.assertEqual(invalid.isvalid, False) + @skipUnlessDBFeature("has_Area_function") def test_area_with_regular_aggregate(self): # Create projected country objects, for this test to work on all backends. @@ -249,6 +260,14 @@ class GISFunctionsTests(TestCase): result = result.sq_m self.assertAlmostEqual((result - c.mpoly.area) / c.mpoly.area, 0) + @skipUnlessDBFeature("has_MakeValid_function") + def test_make_valid(self): + invalid_geom = fromstr('POLYGON((0 0, 0 1, 1 1, 1 0, 1 1, 1 0, 0 0))') + State.objects.create(name='invalid', poly=invalid_geom) + invalid = State.objects.filter(name='invalid').annotate(repaired=functions.MakeValid('poly')).first() + self.assertEqual(invalid.repaired.valid, True) + self.assertEqual(invalid.repaired, fromstr('POLYGON((0 0, 0 1, 1 1, 1 0, 0 0))')) + @skipUnlessDBFeature("has_MemSize_function") def test_memsize(self): ptown = City.objects.annotate(size=functions.MemSize('point')).get(name='Pueblo') diff --git a/tests/gis_tests/geoapp/tests.py b/tests/gis_tests/geoapp/tests.py index f0b6f7ba65..2b215f80a9 100644 --- a/tests/gis_tests/geoapp/tests.py +++ b/tests/gis_tests/geoapp/tests.py @@ -300,6 +300,13 @@ class GeoLookupTest(TestCase): 0 ) + @skipUnlessDBFeature("supports_isvalid_lookup") + def test_isvalid_lookup(self): + invalid_geom = fromstr('POLYGON((0 0, 0 1, 1 1, 1 0, 1 1, 1 0, 0 0))') + State.objects.create(name='invalid', poly=invalid_geom) + self.assertEqual(State.objects.filter(poly__isvalid=False).count(), 1) + self.assertEqual(State.objects.filter(poly__isvalid=True).count(), State.objects.count() - 1) + @skipUnlessDBFeature("supports_left_right_lookups") def test_left_right_lookups(self): "Testing the 'left' and 'right' lookup types."