diff --git a/core/edr_provider.py b/core/edr_provider.py index eba54880..95af6a79 100644 --- a/core/edr_provider.py +++ b/core/edr_provider.py @@ -181,8 +181,18 @@ def fields(self): return self.get_fields() # ----------------------------------------------------------- instances - def get_instances(self): - """List transducer-deployment instance identifiers.""" + def instances(self): + """List transducer-deployment instance identifiers. + + Named for pygeoapi's EDR contract, not ours: ``get_collection_edr_ + instances`` calls ``p.instances()`` and ``p.instance(id)``, and + ``BaseEDRProvider`` *returns* (rather than raises) a + ``NotImplementedError`` instance from both. A provider that spells + these ``get_instances``/``get_instance`` therefore does not override + anything -- /instances iterates the NotImplementedError object and + 500s, and /instances/{id}/... validates against a truthy object, so + any id at all is accepted. + """ if not self.instance_field: return [] rows = self._fetch( @@ -192,9 +202,9 @@ def get_instances(self): ) return [str(row["iid"]) for row in rows] - def get_instance(self, instance): + def instance(self, instance): """Validate an instance identifier.""" - return instance in set(self.get_instances()) + return str(instance) in set(self.instances()) # ------------------------------------------------------------ queries def locations( diff --git a/tests/features/edr-water-data.feature b/tests/features/edr-water-data.feature index 9e21f56b..08cd063f 100644 --- a/tests/features/edr-water-data.feature +++ b/tests/features/edr-water-data.feature @@ -1,4 +1,4 @@ -@backend @edr +@backend @edr @production Feature: OGC API - EDR delivery of water-level and water-chemistry data As a consumer of Bureau observational data I want to query groundwater levels and water chemistry through the standard diff --git a/tests/features/environment.py b/tests/features/environment.py index 4d0f6903..2a7af12d 100644 --- a/tests/features/environment.py +++ b/tests/features/environment.py @@ -527,6 +527,15 @@ def add_edr_water_data(context, session, well, deployment): lex_term = "(SELECT term FROM lexicon_term LIMIT 1)" + # Wells seed as 'draft', but ogc_water_chemistry gates on the *thing's* + # release status as well as the sample's, so a draft well publishes no + # chemistry at all. Promote this one well -- the fixture exists to give + # the EDR collections something to serve. + session.execute( + text("UPDATE thing SET release_status = 'public' WHERE id = :tid"), + {"tid": well.id}, + ) + # Promote the seeded transducer data to public and give the deployment a # bounded window + recording interval so it reads as an EDR instance. session.execute( @@ -595,6 +604,37 @@ def add_edr_water_data(context, session, well, deployment): {"sid": sid, "pid": pid, "dt": dt, "val": value, "st": status}, ) + # ogc_water_chemistry is built from the legacy NMA_* chemistry tables + # (d9e0f1a2b3c4), not from observation: nothing populates the + # observation -> sample -> parameter chain with analyte data. Seeding + # only observations left the EDR chemistry collection empty, which is + # why its scenarios failed with 400 (pH not a known parameter) and 204. + for public_release, ph_value in ((True, 7.1), (False, 99.0)): + sample_info_id = session.execute( + text( + 'INSERT INTO "NMA_Chemistry_SampleInfo" ' + '(thing_id, "CollectionDate", "PublicRelease", ' + '"nma_SamplePointID") ' + "VALUES (:tid, '2022-06-01T00:00:00Z', :pub, 'EDR-TEST') " + "RETURNING id" + ), + {"tid": well.id, "pub": public_release}, + ).scalar() + session.execute( + text( + 'INSERT INTO "NMA_FieldParameters" ' + '(chemistry_sample_info_id, "FieldParameter", "SampleValue", ' + "\"Units\") VALUES (:csi, 'pH', :val, 'std units')" + ), + {"csi": sample_info_id, "val": ph_value}, + ) + + session.commit() + + # Materialized view: without a refresh the rows just inserted are + # invisible to every chemistry query. + session.execute(text("REFRESH MATERIALIZED VIEW ogc_water_chemistry")) + session.execute(text("REFRESH MATERIALIZED VIEW ogc_internal_water_chemistry")) session.commit() diff --git a/tests/test_edr_provider.py b/tests/test_edr_provider.py index f7ba87dc..9f60fa4c 100644 --- a/tests/test_edr_provider.py +++ b/tests/test_edr_provider.py @@ -30,3 +30,48 @@ def test_station_properties_omits_thing_type_when_the_view_lacks_it(): properties = _provider(False)._station_properties({"station_name": "NM-28368"}) assert properties == {"name": "NM-28368"} + + +# ---------------------------------------------------------------- instances + + +def test_provider_implements_pygeoapis_instance_contract(): + """The method names pygeoapi actually calls. + + BaseEDRProvider.instances/instance *return* a NotImplementedError rather + than raising one, so a provider that spells these get_instances/ + get_instance overrides nothing and fails silently at the API layer: + /instances iterates the NotImplementedError object (TypeError -> 500), and + /instances/{id}/... validates the id against a truthy object, accepting + anything. + """ + from pygeoapi.provider.base_edr import BaseEDRProvider + + for name in ("instances", "instance"): + assert name in WaterEDRProvider.__dict__, ( + f"WaterEDRProvider must override {name}() -- pygeoapi calls that " + "name, and the base implementation returns a NotImplementedError " + "object instead of raising." + ) + assert getattr(WaterEDRProvider, name) is not getattr(BaseEDRProvider, name) + + +def test_instances_are_empty_without_an_instance_field(): + # ogc_water_chemistry has no deployments, so its provider declares no + # instance_field and must report an empty list rather than querying. + provider = object.__new__(WaterEDRProvider) + provider.instance_field = None + + assert provider.instances() == [] + + +def test_instance_validation_compares_as_strings(monkeypatch): + # instances() reports identifiers as strings; the id arrives from the URL + # as a string too, but an int must not slip through as valid. + provider = object.__new__(WaterEDRProvider) + provider.instance_field = "deployment_id" + monkeypatch.setattr(WaterEDRProvider, "instances", lambda self: ["7", "9"]) + + assert provider.instance("7") is True + assert provider.instance(7) is True + assert provider.instance("8") is False