fix: make vector ingestion canonical and atomic
This commit is contained in:
@@ -376,32 +376,37 @@ class DatasetService:
|
||||
metadata_json=metadata,
|
||||
status=status,
|
||||
)
|
||||
db.add(dataset)
|
||||
db.add(
|
||||
DatasetVersion(
|
||||
dataset_id=dataset.id,
|
||||
version=1,
|
||||
storage_path=dataset.storage_path,
|
||||
source_version=dataset.source_version,
|
||||
observed_at=dataset.observed_at,
|
||||
valid_from=dataset.valid_from,
|
||||
valid_to=dataset.valid_to,
|
||||
checksum_sha256=dataset.checksum_sha256,
|
||||
source_metadata=dataset.source_metadata,
|
||||
provenance_metadata=dataset.provenance_metadata,
|
||||
)
|
||||
)
|
||||
db.commit()
|
||||
db.refresh(dataset)
|
||||
|
||||
if canonical_type == "vector" and vector_payload is not None and status == "ready":
|
||||
feature_class = reference_layer_name if normalized_role == "reference" else None
|
||||
VectorFeatureService.persist_geojson_features(
|
||||
db=db,
|
||||
dataset_id=dataset.id,
|
||||
payload=vector_payload,
|
||||
feature_class=feature_class,
|
||||
try:
|
||||
db.add(dataset)
|
||||
db.add(
|
||||
DatasetVersion(
|
||||
dataset_id=dataset.id,
|
||||
version=1,
|
||||
storage_path=dataset.storage_path,
|
||||
source_version=dataset.source_version,
|
||||
observed_at=dataset.observed_at,
|
||||
valid_from=dataset.valid_from,
|
||||
valid_to=dataset.valid_to,
|
||||
checksum_sha256=dataset.checksum_sha256,
|
||||
source_metadata=dataset.source_metadata,
|
||||
provenance_metadata=dataset.provenance_metadata,
|
||||
)
|
||||
)
|
||||
if canonical_type == "vector" and vector_payload is not None and status == "ready":
|
||||
feature_class = reference_layer_name if normalized_role == "reference" else None
|
||||
VectorFeatureService.persist_geojson_features(
|
||||
db=db,
|
||||
dataset_id=dataset.id,
|
||||
payload=vector_payload,
|
||||
feature_class=feature_class,
|
||||
commit=False,
|
||||
)
|
||||
db.commit()
|
||||
db.refresh(dataset)
|
||||
except Exception:
|
||||
db.rollback()
|
||||
StorageService.remove_dataset_file(storage_info["storage_path"])
|
||||
raise
|
||||
|
||||
return DatasetService._to_response(dataset)
|
||||
|
||||
|
||||
@@ -32,6 +32,7 @@ def parse_geojson_payload(raw_text: str | dict[str, Any]) -> dict[str, Any]:
|
||||
geometry_types: set[str] = set()
|
||||
geometries = []
|
||||
invalid_features = 0
|
||||
z_dimension_features = 0
|
||||
polygon_area_m2: float | None = None
|
||||
crs_assumed = None
|
||||
for feature in features:
|
||||
@@ -49,6 +50,8 @@ def parse_geojson_payload(raw_text: str | dict[str, Any]) -> dict[str, Any]:
|
||||
if not geom.is_valid:
|
||||
invalid_features += 1
|
||||
raise ValueError("Invalid geometry remains after repair")
|
||||
if geom.has_z:
|
||||
z_dimension_features += 1
|
||||
geometry_types.add(str(geom.geom_type))
|
||||
geometries.append(geom)
|
||||
|
||||
@@ -85,6 +88,8 @@ def parse_geojson_payload(raw_text: str | dict[str, Any]) -> dict[str, Any]:
|
||||
"bounds_json": bounds_json,
|
||||
"approximate_area_m2": polygon_area_m2,
|
||||
"invalid_features": invalid_features,
|
||||
"z_dimension_feature_count": z_dimension_features,
|
||||
"canonical_storage_dimension": "2D",
|
||||
"crs": crs,
|
||||
"crs_assumed": crs_assumed,
|
||||
"extracted_at": datetime.now(timezone.utc).isoformat(),
|
||||
|
||||
@@ -8,6 +8,7 @@ from geoalchemy2.shape import from_shape
|
||||
from geoalchemy2.shape import to_shape
|
||||
from shapely.geometry import mapping
|
||||
from shapely.geometry import shape
|
||||
from shapely.ops import transform as transform_geometry
|
||||
from shapely.validation import make_valid
|
||||
from sqlalchemy import Float, cast, func
|
||||
|
||||
@@ -265,6 +266,8 @@ class VectorFeatureService:
|
||||
geometry = make_valid(geometry)
|
||||
if geometry.is_empty or not geometry.is_valid:
|
||||
raise AppError(code="INVALID_GEOMETRY", message=f"Invalid feature geometry at index {index}", status_code=400)
|
||||
if geometry.has_z:
|
||||
geometry = transform_geometry(lambda x, y, z=None: (x, y), geometry)
|
||||
|
||||
properties = feature.get("properties") if isinstance(feature.get("properties"), dict) else {}
|
||||
source_feature_id = feature.get("id")
|
||||
|
||||
@@ -117,6 +117,24 @@ def test_parse_geojson_payload_returns_vector_metadata() -> None:
|
||||
assert metadata["approximate_area_m2"] >= 0.0
|
||||
|
||||
|
||||
def test_parse_geojson_payload_reports_z_dimension_for_canonical_2d_storage() -> None:
|
||||
metadata = geojson_service.parse_geojson_payload(
|
||||
{
|
||||
"type": "FeatureCollection",
|
||||
"features": [
|
||||
{
|
||||
"type": "Feature",
|
||||
"geometry": {"type": "Point", "coordinates": [5.08, 51.18, 0.0]},
|
||||
"properties": {},
|
||||
}
|
||||
],
|
||||
}
|
||||
)
|
||||
|
||||
assert metadata["z_dimension_feature_count"] == 1
|
||||
assert metadata["canonical_storage_dimension"] == "2D"
|
||||
|
||||
|
||||
def test_parse_geojson_payload_rejects_invalid_geometry() -> None:
|
||||
payload = {
|
||||
"type": "FeatureCollection",
|
||||
|
||||
@@ -5,6 +5,7 @@ from pathlib import Path
|
||||
from uuid import uuid4
|
||||
|
||||
import pytest
|
||||
from geoalchemy2.shape import to_shape
|
||||
|
||||
from app.api.routes.qa import compare_candidate_with_reference
|
||||
from app.models import Dataset, Metric, Project, QualityCheck, VectorFeature
|
||||
@@ -22,6 +23,7 @@ class FakeSession:
|
||||
self.commits = 0
|
||||
self.refreshes = []
|
||||
self.flushes = 0
|
||||
self.rollbacks = 0
|
||||
|
||||
def get(self, model, item_id):
|
||||
return self.objects.get((model, item_id))
|
||||
@@ -38,6 +40,9 @@ class FakeSession:
|
||||
def refresh(self, item) -> None:
|
||||
self.refreshes.append(item)
|
||||
|
||||
def rollback(self) -> None:
|
||||
self.rollbacks += 1
|
||||
|
||||
|
||||
def test_vector_feature_service_persists_geojson_features_with_properties() -> None:
|
||||
db = FakeSession()
|
||||
@@ -84,6 +89,34 @@ def test_vector_feature_service_persists_geojson_features_with_properties() -> N
|
||||
assert db.refreshes == []
|
||||
|
||||
|
||||
def test_vector_feature_service_normalizes_source_z_coordinates_to_canonical_2d() -> None:
|
||||
db = FakeSession()
|
||||
persisted = VectorFeatureService.persist_geojson_features(
|
||||
db=db,
|
||||
dataset_id=uuid4(),
|
||||
payload={
|
||||
"type": "FeatureCollection",
|
||||
"features": [
|
||||
{
|
||||
"type": "Feature",
|
||||
"id": "sector-3d",
|
||||
"properties": {"population_total": 100},
|
||||
"geometry": {
|
||||
"type": "Polygon",
|
||||
"coordinates": [
|
||||
[[5.0, 51.0, 0.0], [5.1, 51.0, 0.0], [5.1, 51.1, 0.0], [5.0, 51.0, 0.0]]
|
||||
],
|
||||
},
|
||||
}
|
||||
],
|
||||
},
|
||||
feature_class="population",
|
||||
)
|
||||
|
||||
assert len(persisted) == 1
|
||||
assert to_shape(persisted[0].geometry).has_z is False
|
||||
|
||||
|
||||
def test_dataset_upload_persists_vector_features(monkeypatch, tmp_path) -> None:
|
||||
project_id = uuid4()
|
||||
db = FakeSession(objects={(Project, project_id): Project(id=project_id, name="Geel")})
|
||||
@@ -141,6 +174,53 @@ def test_dataset_upload_persists_vector_features(monkeypatch, tmp_path) -> None:
|
||||
assert result.source_name == "manual"
|
||||
assert len(persisted_features) == 1
|
||||
assert persisted_features[0].dataset_id == result.id
|
||||
assert db.commits == 1
|
||||
|
||||
|
||||
def test_dataset_upload_rolls_back_dataset_and_file_when_vector_indexing_fails(monkeypatch, tmp_path) -> None:
|
||||
project_id = uuid4()
|
||||
db = FakeSession(objects={(Project, project_id): Project(id=project_id, name="Mol")})
|
||||
storage_path = tmp_path / "invalid.geojson"
|
||||
storage_path.write_text("{}", encoding="utf-8")
|
||||
|
||||
class Upload:
|
||||
filename = "invalid.geojson"
|
||||
content_type = "application/geo+json"
|
||||
|
||||
async def read(self) -> bytes:
|
||||
return b'{"type":"FeatureCollection","features":[]}'
|
||||
|
||||
monkeypatch.setattr(
|
||||
"app.services.dataset_service.StorageService.persist_dataset_file",
|
||||
lambda **_kwargs: {
|
||||
"storage_path": str(storage_path),
|
||||
"original_filename": "invalid.geojson",
|
||||
"stored_filename": "invalid.geojson",
|
||||
"content_type": "application/geo+json",
|
||||
"size_bytes": 2,
|
||||
"checksum_sha256": "0" * 64,
|
||||
},
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
VectorFeatureService,
|
||||
"persist_geojson_features",
|
||||
lambda **_kwargs: (_ for _ in ()).throw(RuntimeError("PostGIS indexing failed")),
|
||||
)
|
||||
|
||||
with pytest.raises(RuntimeError, match="PostGIS indexing failed"):
|
||||
asyncio.run(
|
||||
DatasetService.upload_dataset(
|
||||
db=db,
|
||||
project_id=project_id,
|
||||
file=Upload(),
|
||||
dataset_type="vector",
|
||||
source="user_upload",
|
||||
)
|
||||
)
|
||||
|
||||
assert db.commits == 0
|
||||
assert db.rollbacks == 1
|
||||
assert storage_path.exists() is False
|
||||
|
||||
|
||||
def test_quality_service_persists_quality_check_and_metrics() -> None:
|
||||
|
||||
Reference in New Issue
Block a user