Skip to content

Commit ae647c6

Browse files
committed
fix: address review findings in geofence model
- Batch _compute_area_sqkm into single SQL query (was N+1) - Use psycopg2.sql.Identifier for safe table name interpolation - Narrow exception catch in to_geojson to specific types - Prefetch tag_ids and create_uid in to_geojson_collection - Add spp_gis. prefix to geofence ACL model_id references - Remove redundant field guard in spp_api_v2_gis properties override
1 parent 7edc4b3 commit ae647c6

File tree

3 files changed

+34
-30
lines changed

3 files changed

+34
-30
lines changed

spp_api_v2_gis/models/geofence.py

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,8 @@ class GisGeofence(models.Model):
1919
)
2020

2121
def _get_geojson_properties(self):
22-
"""Extend properties with incident info when spp_hazard is installed."""
22+
"""Extend properties with incident info from spp_hazard."""
2323
props = super()._get_geojson_properties()
24-
if "incident_id" in self._fields:
25-
props["incident_id"] = (
26-
self.incident_id.uuid if self.incident_id and hasattr(self.incident_id, "uuid") else None
27-
)
28-
props["incident_name"] = self.incident_id.name if self.incident_id else None
24+
props["incident_id"] = self.incident_id.uuid if self.incident_id and hasattr(self.incident_id, "uuid") else None
25+
props["incident_name"] = self.incident_id.name if self.incident_id else None
2926
return props

spp_gis/models/geofence.py

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import uuid
77

88
import psycopg2
9+
from psycopg2 import sql
910
from shapely.geometry import mapping
1011

1112
from odoo import _, api, fields, models
@@ -103,25 +104,28 @@ def _compute_area_sqkm(self):
103104
Uses ST_Area with geography type for accurate area calculation
104105
in square meters, then converts to square kilometers.
105106
"""
106-
for rec in self:
107-
if not rec.geometry or not rec.id:
108-
rec.area_sqkm = 0.0
109-
continue
110-
111-
try:
112-
# Use PostGIS ST_Area with geography cast for accurate measurement
113-
# Geography type automatically uses spheroid calculations
114-
query = """
115-
SELECT ST_Area(ST_Transform(geometry::geometry, 4326)::geography) / 1000000.0 as area_sqkm
116-
FROM %s
117-
WHERE id = %%s
118-
"""
119-
# Use self._table for the table name instead of hardcoding
120-
self.env.cr.execute(query % self._table, (rec.id,))
121-
result = self.env.cr.fetchone()
122-
rec.area_sqkm = result[0] if result else 0.0
123-
except psycopg2.Error as e:
124-
_logger.warning("Failed to compute area for geofence %s: %s", rec.id, str(e))
107+
records_with_geom = self.filtered(lambda r: r.geometry and r.id)
108+
records_without = self - records_with_geom
109+
110+
for rec in records_without:
111+
rec.area_sqkm = 0.0
112+
113+
if not records_with_geom:
114+
return
115+
116+
try:
117+
# Batch query: compute area for all records with geometry in one roundtrip
118+
query = sql.SQL(
119+
"SELECT id, ST_Area(ST_Transform(geometry::geometry, 4326)::geography) / 1000000.0 "
120+
"FROM {} WHERE id IN %s"
121+
).format(sql.Identifier(self._table))
122+
self.env.cr.execute(query, (tuple(records_with_geom.ids),))
123+
results = dict(self.env.cr.fetchall())
124+
for rec in records_with_geom:
125+
rec.area_sqkm = results.get(rec.id, 0.0)
126+
except psycopg2.Error as e:
127+
_logger.warning("Failed to compute area for geofences %s: %s", records_with_geom.ids, str(e))
128+
for rec in records_with_geom:
125129
rec.area_sqkm = 0.0
126130

127131
@api.constrains("name", "active")
@@ -170,7 +174,7 @@ def to_geojson(self):
170174
# Convert shapely geometry to GeoJSON
171175
try:
172176
geometry_dict = mapping(self.geometry)
173-
except Exception as e:
177+
except (ValueError, TypeError, AttributeError) as e:
174178
_logger.warning("Failed to convert geometry to GeoJSON for geofence %s: %s", self.id, str(e))
175179
geometry_dict = None
176180

@@ -207,6 +211,9 @@ def to_geojson_collection(self):
207211
Returns:
208212
dict: GeoJSON FeatureCollection with all features
209213
"""
214+
# Prefetch related fields to avoid N+1 queries on singletons
215+
self.mapped("tag_ids.name")
216+
self.mapped("create_uid.name")
210217
features = [rec.to_geojson() for rec in self]
211218
return {
212219
"type": "FeatureCollection",

spp_gis/security/ir.model.access.csv

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ access_spp_raster_layer_admin,Raster Layer Admin,spp_gis.model_spp_gis_raster_la
66
access_spp_raster_layer_type_admin,Raster Layer Type Admin,spp_gis.model_spp_gis_raster_layer_type,spp_security.group_spp_admin,1,1,1,1
77
access_spp_data_layer_read,Data Layer Read,spp_gis.model_spp_gis_data_layer,spp_registry.group_registry_read,1,0,0,0
88
access_spp_raster_layer_read,Raster Layer Read,spp_gis.model_spp_gis_raster_layer,spp_registry.group_registry_read,1,0,0,0
9-
access_spp_gis_geofence_admin,Geofence Admin,model_spp_gis_geofence,spp_security.group_spp_admin,1,1,1,1
10-
access_spp_gis_geofence_manager,Geofence Manager,model_spp_gis_geofence,spp_registry.group_registry_manager,1,1,1,1
11-
access_spp_gis_geofence_officer,Geofence Officer,model_spp_gis_geofence,spp_registry.group_registry_officer,1,1,1,0
12-
access_spp_gis_geofence_read,Geofence Read,model_spp_gis_geofence,spp_registry.group_registry_read,1,0,0,0
9+
access_spp_gis_geofence_admin,Geofence Admin,spp_gis.model_spp_gis_geofence,spp_security.group_spp_admin,1,1,1,1
10+
access_spp_gis_geofence_manager,Geofence Manager,spp_gis.model_spp_gis_geofence,spp_registry.group_registry_manager,1,1,1,1
11+
access_spp_gis_geofence_officer,Geofence Officer,spp_gis.model_spp_gis_geofence,spp_registry.group_registry_officer,1,1,1,0
12+
access_spp_gis_geofence_read,Geofence Read,spp_gis.model_spp_gis_geofence,spp_registry.group_registry_read,1,0,0,0

0 commit comments

Comments
 (0)