Made OracleSpatialAdapter clone geometries rather than mutate them.

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")