Repository: django/django
Issue category: Performance Issue

Issue:
Refactoring SpatialRefSysMixin.srs for efficiency and better error handling
Description
		
(last modified by Arnaldo Govene)		
The srs property in the SpatialRefSysMixin class has several issues that can be addressed to improve code efficiency, maintainability, and adherence to best practices. The following points highlight the concerns:
1. Recursive Calls:
The property uses return self.srs within the try blocks. This introduces recursion, which could lead to stack overflow in edge cases or unexpected behavior. 
Current Code:
try:
	self._srs = gdal.SpatialReference(self.wkt)
	return self.srs # Recursive call
except Exception as e:
	msg = e
2. Error Handling:
The msg variable is overwritten in the second try block without being utilized after the first block, making error reporting ambiguous. This results in the final error message only reflecting the second exception, losing context from the first one.
Current Code:
try:
	self._srs = gdal.SpatialReference(self.wkt)
	return self.srs
except Exception as e:
	msg = e # First exception caught
try:
	self._srs = gdal.SpatialReference(self.proj4text)
	return self.srs
except Exception as e:
	msg = e # Second exception overwrites the first
Proposal:
1. Improve Error Reporting:
Capture and differentiate between errors raised during the initialization from WKT and PROJ.4. Ensure that both error messages are included in the final exception for better clarity and debugging.
2. Raplace Manual Caching with @cached_propert and Remove Recursive Calls:
To further optimize the srs property, replace the manual caching mechanism with @cached_property. This simplifies the code while retaining the benefits of caching, which is particularly important given the computational cost of creating a gdal.SpatialReference object.
Refactored Code:
from django.utils.functional import cached_property
class SpatialRefSysMixin:
	@cached_property
	def srs(self):
		"""
		Return a GDAL SpatialReference object.
		"""
		try:
			return gdal.SpatialReference(self.wkt)
		except Exception as e:
			wkt_error = f"Error initializing from WKT: {str(e)}"
		try:
			return gdal.SpatialReference(self.proj4text)
		except Exception as e:
			proj4_error = f"Error initializing from PROJ.4: {str(e)}"
		raise Exception(f"Could not get OSR SpatialReference. WKT Error: {wkt_error} | PROJ.4 Error: {proj4_error}")
Rationale for Using cached_property:
Caches the computationally expensive creation of gdal.SpatialReference, improving efficiency for repeated access.
Removes manual caching logic, making the code cleaner and easier to maintain.
Ideal for mostly static spatial reference data, avoiding unnecessary recomputation.
Forum discussion: ​https://forum.djangoproject.com/t/caching-strategy-for-spatialrefsysmixin-srs/37634/3
Refactoring SpatialRefSysMixin.srs for efficiency and better error handling
Description
		
(last modified by Arnaldo Govene)		
The srs property in the SpatialRefSysMixin class has several issues that can be addressed to improve code efficiency, maintainability, and adherence to best practices. The following points highlight the concerns:
1. Recursive Calls:
The property uses return self.srs within the try blocks. This introduces recursion, which could lead to stack overflow in edge cases or unexpected behavior. 
Current Code:
try:
	self._srs = gdal.SpatialReference(self.wkt)
	return self.srs # Recursive call
except Exception as e:
	msg = e
2. Error Handling:
The msg variable is overwritten in the second try block without being utilized after the first block, making error reporting ambiguous. This results in the final error message only reflecting the second exception, losing context from the first one.
Current Code:
try:
	self._srs = gdal.SpatialReference(self.wkt)
	return self.srs
except Exception as e:
	msg = e # First exception caught
try:
	self._srs = gdal.SpatialReference(self.proj4text)
	return self.srs
except Exception as e:
	msg = e # Second exception overwrites the first
Proposal:
1. Improve Error Reporting:
Capture and differentiate between errors raised during the initialization from WKT and PROJ.4. Ensure that both error messages are included in the final exception for better clarity and debugging.
2. Raplace Manual Caching with @cached_propert and Remove Recursive Calls:
To further optimize the srs property, replace the manual caching mechanism with @cached_property. This simplifies the code while retaining the benefits of caching, which is particularly important given the computational cost of creating a gdal.SpatialReference object.
Refactored Code:
from django.utils.functional import cached_property
class SpatialRefSysMixin:
	@cached_property
	def srs(self):
		"""
		Return a GDAL SpatialReference object.
		"""
		try:
			return gdal.SpatialReference(self.wkt)
		except Exception as e:
			wkt_error = f"Error initializing from WKT: {str(e)}"
		try:
			return gdal.SpatialReference(self.proj4text)
		except Exception as e:
			proj4_error = f"Error initializing from PROJ.4: {str(e)}"
		raise Exception(f"Could not get OSR SpatialReference. WKT Error: {wkt_error} | PROJ.4 Error: {proj4_error}")
Rationale for Using cached_property:
Caches the computationally expensive creation of gdal.SpatialReference, improving efficiency for repeated access.
Removes manual caching logic, making the code cleaner and easier to maintain.
Ideal for mostly static spatial reference data, avoiding unnecessary recomputation.
Forum discussion: ​https://forum.djangoproject.com/t/caching-strategy-for-spatialrefsysmixin-srs/37634/3


Hints:
['Thank you, agree the error overwrite could be improved, the caching logic is also due a refresh', 1735805940.0]
['Thank you, agree the error overwrite could be improved, the caching logic is also due a refresh', 1735805940.0]

Patch:
diff --git a/django/contrib/gis/db/backends/base/models.py b/django/contrib/gis/db/backends/base/models.py
index 589c872da611..38309f0e5dea 100644
--- a/django/contrib/gis/db/backends/base/models.py
+++ b/django/contrib/gis/db/backends/base/models.py
@@ -1,4 +1,5 @@
 from django.contrib.gis import gdal
+from django.utils.functional import cached_property
 
 
 class SpatialRefSysMixin:
@@ -7,35 +8,26 @@ class SpatialRefSysMixin:
     SpatialRefSys objects to reduce redundant code.
     """
 
-    @property
+    @cached_property
     def srs(self):
         """
         Return a GDAL SpatialReference object.
         """
-        # TODO: Is caching really necessary here?  Is complexity worth it?
-        if hasattr(self, "_srs"):
-            # Returning a clone of the cached SpatialReference object.
-            return self._srs.clone()
-        else:
-            # Attempting to cache a SpatialReference object.
-
-            # Trying to get from WKT first.
-            try:
-                self._srs = gdal.SpatialReference(self.wkt)
-                return self.srs
-            except Exception as e:
-                msg = e
-
-            try:
-                self._srs = gdal.SpatialReference(self.proj4text)
-                return self.srs
-            except Exception as e:
-                msg = e
-
-            raise Exception(
-                "Could not get OSR SpatialReference from WKT: %s\nError:\n%s"
-                % (self.wkt, msg)
-            )
+        try:
+            return gdal.SpatialReference(self.wkt)
+        except Exception as e:
+            wkt_error = e
+
+        try:
+            return gdal.SpatialReference(self.proj4text)
+        except Exception as e:
+            proj4_error = e
+
+        raise Exception(
+            "Could not get OSR SpatialReference.\n"
+            f"Error for WKT '{self.wkt}': {wkt_error}\n"
+            f"Error for PROJ.4 '{self.proj4text}': {proj4_error}"
+        )
 
     @property
     def ellipsoid(self):


Test patch:
diff --git a/tests/gis_tests/test_spatialrefsys.py b/tests/gis_tests/test_spatialrefsys.py
index 512fd217c375..b87dcf8b9293 100644
--- a/tests/gis_tests/test_spatialrefsys.py
+++ b/tests/gis_tests/test_spatialrefsys.py
@@ -1,5 +1,6 @@
 import re
 
+from django.contrib.gis.db.backends.base.models import SpatialRefSysMixin
 from django.db import connection
 from django.test import TestCase, skipUnlessDBFeature
 from django.utils.functional import cached_property
@@ -147,3 +148,17 @@ def test_add_entry(self):
         self.assertTrue(
             self.SpatialRefSys.get_spheroid(srs.wkt).startswith("SPHEROID[")
         )
+
+    def test_srs_with_invalid_wkt_and_proj4(self):
+        class MockSpatialRefSys(SpatialRefSysMixin):
+            def __init__(self, wkt=None, proj4text=None):
+                self.wkt = wkt
+                self.proj4text = proj4text
+
+        with self.assertRaisesMessage(
+            Exception,
+            "Could not get OSR SpatialReference.\n"
+            "Error for WKT 'INVALID_WKT': Corrupt data.\n"
+            "Error for PROJ.4 '+proj=invalid': Corrupt data.",
+        ):
+            MockSpatialRefSys(wkt="INVALID_WKT", proj4text="+proj=invalid").srs