feat(spp_api_v2_gis): add OGC API Features GIS endpoints#71
feat(spp_api_v2_gis): add OGC API Features GIS endpoints#71
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new module, Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Semgrep OSS found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 19.0 #71 +/- ##
==========================================
+ Coverage 55.79% 58.09% +2.29%
==========================================
Files 162 188 +26
Lines 9291 10797 +1506
==========================================
+ Hits 5184 6272 +1088
- Misses 4107 4525 +418
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive GIS API module with OGC-compliant endpoints, which is a significant and well-structured feature addition. The code is generally of high quality, with good separation of concerns into services, routers, and schemas. The inclusion of extensive tests, performance considerations like caching, and robust error handling is commendable.
My review has identified a couple of areas for improvement:
- A metadata inconsistency in the module's development status.
- A potential performance issue (N+1 query) in a computed field method.
Addressing these points will further enhance the quality and robustness of this new module.
Note: Security Review did not run due to the size of the PR.
spp_api_v2_gis/__manifest__.py
Outdated
| "author": "OpenSPP.org", | ||
| "website": "https://github.com/OpenSPP/OpenSPP2", | ||
| "license": "LGPL-3", | ||
| "development_status": "Production/Stable", |
There was a problem hiding this comment.
The development_status is set to Production/Stable, but this contradicts the Alpha status mentioned in the README.rst file and the PR description. To avoid confusion, these should be consistent. Given the context, changing this to Alpha seems appropriate.
| "development_status": "Production/Stable", | |
| "development_status": "Alpha", |
| def _compute_area_sqkm(self): | ||
| """Compute area in square kilometers from geometry using PostGIS. | ||
|
|
||
| Uses ST_Area with geography type for accurate area calculation | ||
| in square meters, then converts to square kilometers. | ||
| """ | ||
| for rec in self: | ||
| if not rec.geometry or not rec.id: | ||
| rec.area_sqkm = 0.0 | ||
| continue | ||
|
|
||
| try: | ||
| # Use PostGIS ST_Area with geography cast for accurate measurement | ||
| # Geography type automatically uses spheroid calculations | ||
| query = """ | ||
| SELECT ST_Area(ST_Transform(geometry::geometry, 4326)::geography) / 1000000.0 as area_sqkm | ||
| FROM spp_gis_geofence | ||
| WHERE id = %s | ||
| """ | ||
| self.env.cr.execute(query, (rec.id,)) | ||
| result = self.env.cr.fetchone() | ||
| rec.area_sqkm = result[0] if result else 0.0 | ||
| except Exception as e: | ||
| _logger.warning("Failed to compute area for geofence %s: %s", rec.name, str(e)) | ||
| rec.area_sqkm = 0.0 |
There was a problem hiding this comment.
The current implementation of _compute_area_sqkm executes a SQL query for each record within the loop, which can lead to performance issues (N+1 problem) when creating or updating multiple geofences at once. This can be optimized by refactoring to use a single batch query to compute the areas for all records in the recordset.
def _compute_area_sqkm(self):
"""Compute area in square kilometers from geometry using PostGIS.
Uses ST_Area with geography type for accurate area calculation
in square meters, then converts to square kilometers.
"""
recs_to_compute = self.filtered(lambda r: r.geometry and r.id)
# Set area to 0 for records without geometry or not yet in DB
for rec in self - recs_to_compute:
rec.area_sqkm = 0.0
if not recs_to_compute:
return
try:
query = """
SELECT id, ST_Area(ST_Transform(geometry::geometry, 4326)::geography) / 1000000.0 as area
FROM spp_gis_geofence
WHERE id = ANY(%s)
"""
self.env.cr.execute(query, (list(recs_to_compute.ids),))
area_map = {res["id"]: res["area"] for res in self.env.cr.dictfetchall()}
for rec in recs_to_compute:
rec.area_sqkm = area_map.get(rec.id, 0.0)
except Exception as e:
_logger.warning("Failed to compute area for geofences: %s", str(e))
for rec in recs_to_compute:
rec.area_sqkm = 0.0
1 participant
Summary
New module providing OGC API Features-compliant GIS endpoints:
Dependencies: spp_api_v2, spp_gis, spp_gis_report, spp_area, spp_hazard, spp_vocabulary, spp_statistic, spp_aggregation
Test plan
./scripts/test_single_module.sh spp_api_v2_gis