From 7734337bcb7f9e322c1d4f56ad3f514e1e4c66b7 Mon Sep 17 00:00:00 2001 From: Tim Graham Date: Wed, 21 Oct 2020 20:20:26 -0400 Subject: [PATCH] Made OracleSpatialAdapter clone geometries rather than mutate them. --- .../contrib/gis/db/backends/base/adapter.py | 5 +++ .../contrib/gis/db/backends/oracle/adapter.py | 31 ++++++++++++++----- .../gis/db/backends/postgis/adapter.py | 4 +++ docs/releases/3.2.txt | 5 +++ tests/gis_tests/distapp/tests.py | 5 +-- tests/gis_tests/geoapp/tests.py | 7 +++-- 6 files changed, 44 insertions(+), 13 deletions(-) diff --git a/django/contrib/gis/db/backends/base/adapter.py b/django/contrib/gis/db/backends/base/adapter.py index 604711eb6a..f6f271915d 100644 --- a/django/contrib/gis/db/backends/base/adapter.py +++ b/django/contrib/gis/db/backends/base/adapter.py @@ -17,3 +17,8 @@ class WKTAdapter: def __str__(self): return self.wkt + + @classmethod + def _fix_polygon(cls, poly): + # Hook for Oracle. + return poly diff --git a/django/contrib/gis/db/backends/oracle/adapter.py b/django/contrib/gis/db/backends/oracle/adapter.py index 6b19349ed2..3f96178091 100644 --- a/django/contrib/gis/db/backends/oracle/adapter.py +++ b/django/contrib/gis/db/backends/oracle/adapter.py @@ -16,17 +16,31 @@ class OracleSpatialAdapter(WKTAdapter): * Inner ring(s) - clockwise """ if isinstance(geom, Polygon): - self._fix_polygon(geom) + if self._polygon_must_be_fixed(geom): + geom = self._fix_polygon(geom) elif isinstance(geom, GeometryCollection): - self._fix_geometry_collection(geom) + if any(isinstance(g, Polygon) and self._polygon_must_be_fixed(g) for g in geom): + geom = self._fix_geometry_collection(geom) self.wkt = geom.wkt self.srid = geom.srid - def _fix_polygon(self, poly): + @staticmethod + def _polygon_must_be_fixed(poly): + return ( + not poly.empty and + ( + not poly.exterior_ring.is_counterclockwise or + any(x.is_counterclockwise for x in poly) + ) + ) + + @classmethod + def _fix_polygon(cls, poly, clone=True): """Fix single polygon orientation as described in __init__().""" - if poly.empty: - return poly + if clone: + poly = poly.clone() + if not poly.exterior_ring.is_counterclockwise: poly.exterior_ring = list(reversed(poly.exterior_ring)) @@ -36,11 +50,14 @@ class OracleSpatialAdapter(WKTAdapter): return poly - def _fix_geometry_collection(self, coll): + @classmethod + def _fix_geometry_collection(cls, coll): """ Fix polygon orientations in geometry collections as described in __init__(). """ + coll = coll.clone() for i, geom in enumerate(coll): if isinstance(geom, Polygon): - coll[i] = self._fix_polygon(geom) + coll[i] = cls._fix_polygon(geom, clone=False) + return coll diff --git a/django/contrib/gis/db/backends/postgis/adapter.py b/django/contrib/gis/db/backends/postgis/adapter.py index 84c19c7e3b..6611442cae 100644 --- a/django/contrib/gis/db/backends/postgis/adapter.py +++ b/django/contrib/gis/db/backends/postgis/adapter.py @@ -42,6 +42,10 @@ class PostGISAdapter: def __str__(self): return self.getquoted() + @classmethod + def _fix_polygon(cls, poly): + return poly + def prepare(self, conn): """ This method allows escaping the binary in the style required by the diff --git a/docs/releases/3.2.txt b/docs/releases/3.2.txt index 18d63786d2..c765c4ed63 100644 --- a/docs/releases/3.2.txt +++ b/docs/releases/3.2.txt @@ -488,6 +488,11 @@ backends. * Support for PostGIS 2.2 is removed. +* The Oracle backend now clones polygons (and geometry collections containing + polygons) before reorienting them and saving them to the database. They are + no longer mutated in place. You might notice this if you use the polygons + after a model is saved. + Dropped support for PostgreSQL 9.5 ---------------------------------- diff --git a/tests/gis_tests/distapp/tests.py b/tests/gis_tests/distapp/tests.py index 3febb98b11..dbf8c4b6fd 100644 --- a/tests/gis_tests/distapp/tests.py +++ b/tests/gis_tests/distapp/tests.py @@ -9,9 +9,7 @@ from django.db import NotSupportedError, connection from django.db.models import Exists, F, OuterRef, Q from django.test import TestCase, skipIfDBFeature, skipUnlessDBFeature -from ..utils import ( - FuncTestMixin, mysql, no_oracle, oracle, postgis, spatialite, -) +from ..utils import FuncTestMixin, mysql, oracle, postgis, spatialite from .models import ( AustraliaCity, CensusZipcode, Interstate, SouthTexasCity, SouthTexasCityFt, SouthTexasInterstate, SouthTexasZipcode, @@ -475,7 +473,6 @@ class DistanceFunctionsTests(FuncTestMixin, TestCase): with self.assertRaisesMessage(ValueError, msg): list(qs) - @no_oracle # Oracle already handles geographic distance calculation. @skipUnlessDBFeature("has_Distance_function", 'has_Transform_function') def test_distance_transform(self): """ diff --git a/tests/gis_tests/geoapp/tests.py b/tests/gis_tests/geoapp/tests.py index 4d441dcb94..fb1df6a79f 100644 --- a/tests/gis_tests/geoapp/tests.py +++ b/tests/gis_tests/geoapp/tests.py @@ -79,7 +79,7 @@ class GeoModelTest(TestCase): nullstate.save() ns = State.objects.get(name='NullState') - self.assertEqual(ply, ns.poly) + self.assertEqual(connection.ops.Adapter._fix_polygon(ply), ns.poly) # Testing the `ogr` and `srs` lazy-geometry properties. self.assertIsInstance(ns.poly.ogr, gdal.OGRGeometry) @@ -93,7 +93,10 @@ class GeoModelTest(TestCase): ply[1] = new_inner self.assertEqual(4326, ns.poly.srid) ns.save() - self.assertEqual(ply, State.objects.get(name='NullState').poly) + self.assertEqual( + connection.ops.Adapter._fix_polygon(ply), + State.objects.get(name='NullState').poly + ) ns.delete() @skipUnlessDBFeature("supports_transform")