From df2e1a7fd9aedded50b0ba49e7858e7dfd8d1b57 Mon Sep 17 00:00:00 2001 From: Stefaan Lippens Date: Tue, 19 Sep 2023 18:17:44 +0200 Subject: [PATCH] Issue #74 eliminate flask dependency in normalize_collection_metadata The root/parent/self link functionality moved to openeo-python-driver. Avoids issue with background cache priming. --- setup.py | 2 +- src/openeo_aggregator/about.py | 2 +- src/openeo_aggregator/backend.py | 4 +- src/openeo_aggregator/metadata/merging.py | 24 ++--- tests/test_backend.py | 126 ---------------------- tests/test_views.py | 57 ++++++++++ 6 files changed, 69 insertions(+), 146 deletions(-) diff --git a/setup.py b/setup.py index 351fb91b..68997a01 100644 --- a/setup.py +++ b/setup.py @@ -28,7 +28,7 @@ "requests", "attrs", "openeo>=0.17.0", - "openeo_driver>=0.66.0.dev", + "openeo_driver>=0.67.0.dev", "flask~=2.0", "gunicorn~=20.0", "python-json-logger>=2.0.0", diff --git a/src/openeo_aggregator/about.py b/src/openeo_aggregator/about.py index bbce80b3..b5946dfe 100644 --- a/src/openeo_aggregator/about.py +++ b/src/openeo_aggregator/about.py @@ -1 +1 @@ -__version__ = "0.10.3a1" +__version__ = "0.10.4a1" diff --git a/src/openeo_aggregator/backend.py b/src/openeo_aggregator/backend.py index 4d25caa3..81cc47d8 100644 --- a/src/openeo_aggregator/backend.py +++ b/src/openeo_aggregator/backend.py @@ -195,7 +195,7 @@ def _get_all_metadata(self) -> Tuple[List[dict], _InternalCollectionMetadata]: except Exception as e: _log.error(f"Failed to merge collection metadata for {cid!r}", exc_info=True) continue - metadata = normalize_collection_metadata(metadata, app=flask.current_app) + metadata = normalize_collection_metadata(metadata) collections_metadata.append(metadata) internal_data.set_backends_for_collection(cid, by_backend.keys()) @@ -287,7 +287,7 @@ def _get_collection_metadata(self, collection_id: str) -> dict: metadata = merge_collection_metadata( by_backend=by_backend, full_metadata=True ) - return normalize_collection_metadata(metadata, app=flask.current_app) + return normalize_collection_metadata(metadata) def load_collection(self, collection_id: str, load_params: LoadParameters, env: EvalEnv) -> DriverDataCube: raise RuntimeError("openeo-aggregator does not implement concrete collection loading") diff --git a/src/openeo_aggregator/metadata/merging.py b/src/openeo_aggregator/metadata/merging.py index bc7e6d6e..7cc89019 100644 --- a/src/openeo_aggregator/metadata/merging.py +++ b/src/openeo_aggregator/metadata/merging.py @@ -30,25 +30,17 @@ DEFAULT_REPORTER = LoggerReporter(_log) -def normalize_collection_metadata(metadata: dict, app: Optional[flask.Flask] = None) -> dict: +def normalize_collection_metadata(metadata: dict) -> dict: cid = metadata.get("id", None) if cid is None: raise OpenEOApiException("Missing collection id in metadata") - if "links" not in metadata: - metadata["links"] = [] - metadata["links"] = [l for l in metadata["links"] if l.get("rel") not in ("self", "parent", "root")] - if app: - metadata["links"].append({ - "href": app.url_for("openeo.collections", _external = True), "rel": "root" - }) - metadata["links"].append({ - "href": app.url_for("openeo.collections", _external = True), "rel": "parent" - }) - metadata["links"].append({ - "href": app.url_for("openeo.collection_by_id", collection_id = cid, _external = True), "rel": "self" - }) - else: - _log.warning("Unable to provide root/parent/self links in collection metadata outside flask app context") + # Remove the root/parent/self links, so that aggregator-specific ones + # are automatically added by `_normalize_collection_metadata` from openeo-python-driver (0.67.0, 133e6aa5) + if "links" in metadata: + links_to_keep = [ln for ln in metadata.pop("links") if ln.get("rel") not in ("self", "parent", "root")] + if links_to_keep: + metadata["links"] = links_to_keep + return metadata diff --git a/tests/test_backend.py b/tests/test_backend.py index f5ed4789..1f47b4d2 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -934,38 +934,10 @@ def test_get_all_metadata_simple(self, catalog, backend1, backend2, requests_moc { "id": "S2", "summaries": {"federation:backends": ["b1"]}, - "links": [ - { - "href": "http://oeoa.test/openeo/1.1.0/collections", - "rel": "root", - }, - { - "href": "http://oeoa.test/openeo/1.1.0/collections", - "rel": "parent", - }, - { - "href": "http://oeoa.test/openeo/1.1.0/collections/S2", - "rel": "self", - }, - ], }, { "id": "S3", "summaries": {"federation:backends": ["b2"]}, - "links": [ - { - "href": "http://oeoa.test/openeo/1.1.0/collections", - "rel": "root", - }, - { - "href": "http://oeoa.test/openeo/1.1.0/collections", - "rel": "parent", - }, - { - "href": "http://oeoa.test/openeo/1.1.0/collections/S3", - "rel": "self", - }, - ], }, ] @@ -985,20 +957,6 @@ def test_get_all_metadata_common_collections_minimal( { "id": "S3", "summaries": {"federation:backends": ["b1"]}, - "links": [ - { - "href": "http://oeoa.test/openeo/1.1.0/collections", - "rel": "root", - }, - { - "href": "http://oeoa.test/openeo/1.1.0/collections", - "rel": "parent", - }, - { - "href": "http://oeoa.test/openeo/1.1.0/collections/S3", - "rel": "self", - }, - ], }, { "id": "S4", @@ -1015,38 +973,10 @@ def test_get_all_metadata_common_collections_minimal( "federation:backends": ["b1", "b2"], "provider:backend": ["b1", "b2"], }, - "links": [ - { - "href": "http://oeoa.test/openeo/1.1.0/collections", - "rel": "root", - }, - { - "href": "http://oeoa.test/openeo/1.1.0/collections", - "rel": "parent", - }, - { - "href": "http://oeoa.test/openeo/1.1.0/collections/S4", - "rel": "self", - }, - ], }, { "id": "S5", "summaries": {"federation:backends": ["b2"]}, - "links": [ - { - "href": "http://oeoa.test/openeo/1.1.0/collections", - "rel": "root", - }, - { - "href": "http://oeoa.test/openeo/1.1.0/collections", - "rel": "parent", - }, - { - "href": "http://oeoa.test/openeo/1.1.0/collections/S5", - "rel": "self", - }, - ], }, ] @@ -1161,18 +1091,6 @@ def test_get_all_metadata_common_collections_merging( "rel": "license", "href": "https://spdx.org/licenses/Apache-1.0.html", }, - { - "href": "http://oeoa.test/openeo/1.1.0/collections", - "rel": "root", - }, - { - "href": "http://oeoa.test/openeo/1.1.0/collections", - "rel": "parent", - }, - { - "href": "http://oeoa.test/openeo/1.1.0/collections/S4", - "rel": "self", - }, ], "type": "Collection", }, @@ -1222,21 +1140,11 @@ def test_get_collection_metadata_basic( assert metadata == { 'id': 'S2', 'title': "b1's S2", "summaries": {"federation:backends": ["b1"]}, - 'links': [ - {'href': 'http://oeoa.test/openeo/1.1.0/collections', 'rel': 'root'}, - {'href': 'http://oeoa.test/openeo/1.1.0/collections', 'rel': 'parent'}, - {'href': 'http://oeoa.test/openeo/1.1.0/collections/S2', 'rel': 'self'}, - ] } metadata = catalog.get_collection_metadata("S3") assert metadata == { "id": "S3", "title": "b2's S3", "summaries": {"federation:backends": ["b2"]}, - 'links': [ - {'href': 'http://oeoa.test/openeo/1.1.0/collections', 'rel': 'root'}, - {'href': 'http://oeoa.test/openeo/1.1.0/collections', 'rel': 'parent'}, - {'href': 'http://oeoa.test/openeo/1.1.0/collections/S3', 'rel': 'self'}, - ] } with pytest.raises(CollectionNotFoundException): @@ -1333,9 +1241,6 @@ def test_get_collection_metadata_merging( "links": [ {"rel": "license", "href": "https://special.license.org"}, {"rel": "license", "href": "https://propietary.license"}, - {"href": "http://oeoa.test/openeo/1.1.0/collections", "rel": "root"}, - {"href": "http://oeoa.test/openeo/1.1.0/collections", "rel": "parent"}, - {"href": "http://oeoa.test/openeo/1.1.0/collections/S2", "rel": "self"}, ], "providers": [{"name": "provider1"}, {"name": "provider2"}], "type": "Collection", @@ -1410,11 +1315,6 @@ def test_get_collection_metadata_merging_summaries( }, "id": "S2", "license": "proprietary", - "links": [ - {"href": "http://oeoa.test/openeo/1.1.0/collections", "rel": "root"}, - {"href": "http://oeoa.test/openeo/1.1.0/collections", "rel": "parent"}, - {"href": "http://oeoa.test/openeo/1.1.0/collections/S2", "rel": "self"}, - ], "stac_version": "0.9.0", "summaries": { "constellation": ["sentinel-1"], @@ -1501,11 +1401,6 @@ def test_get_collection_metadata_merging_extent( "spatial": {"bbox": [[-180, -90, 180, 90], [-10, -90, 120, 90]]}, "temporal": {"interval": [["2014-10-03T04:14:15Z", None]]}, }, - "links": [ - {"href": "http://oeoa.test/openeo/1.1.0/collections", "rel": "root"}, - {"href": "http://oeoa.test/openeo/1.1.0/collections", "rel": "parent"}, - {"href": "http://oeoa.test/openeo/1.1.0/collections/S2", "rel": "self"}, - ], "summaries": { "federation:backends": ["b1", "b2"], "provider:backend": ["b1", "b2"], @@ -1577,9 +1472,6 @@ def test_get_collection_metadata_merging_links( }, "links": [ {"href": "http://some.license", "rel": "license"}, - {"href": "http://oeoa.test/openeo/1.1.0/collections", "rel": "root"}, - {"href": "http://oeoa.test/openeo/1.1.0/collections", "rel": "parent"}, - {"href": "http://oeoa.test/openeo/1.1.0/collections/S2", "rel": "self"}, ], "summaries": { "federation:backends": ["b1", "b2"], @@ -1656,9 +1548,6 @@ def test_get_collection_metadata_merging_removes_duplicate_links( "links": [ {"href": "http://some.license", "rel": "license"}, {"href": "http://some.about.page", "rel": "about"}, - {"href": "http://oeoa.test/openeo/1.1.0/collections", "rel": "root"}, - {"href": "http://oeoa.test/openeo/1.1.0/collections", "rel": "parent"}, - {"href": "http://oeoa.test/openeo/1.1.0/collections/S2", "rel": "self"}, ], "summaries": { "federation:backends": ["b1", "b2"], @@ -1780,11 +1669,6 @@ def test_get_collection_metadata_merging_cubedimensions( "spatial": {"bbox": [[-180, -90, 180, 90]]}, "temporal": {"interval": [[None, None]]}, }, - "links": [ - {"href": "http://oeoa.test/openeo/1.1.0/collections", "rel": "root"}, - {"href": "http://oeoa.test/openeo/1.1.0/collections", "rel": "parent"}, - {"href": "http://oeoa.test/openeo/1.1.0/collections/S2", "rel": "self"}, - ], } @pytest.mark.parametrize( @@ -1860,11 +1744,6 @@ def test_get_collection_metadata_merging_bands_prefix( "spatial": {"bbox": [[-180, -90, 180, 90]]}, "temporal": {"interval": [[None, None]]}, }, - "links": [ - {"href": "http://oeoa.test/openeo/1.1.0/collections", "rel": "root"}, - {"href": "http://oeoa.test/openeo/1.1.0/collections", "rel": "parent"}, - {"href": "http://oeoa.test/openeo/1.1.0/collections/S2", "rel": "self"}, - ], } def test_get_collection_metadata_merging_with_error( @@ -1886,11 +1765,6 @@ def test_get_collection_metadata_merging_with_error( "id": "S2", "title": "b2's S2", "summaries": {"federation:backends": ["b2"]}, - "links": [ - {"href": "http://oeoa.test/openeo/1.1.0/collections", "rel": "root"}, - {"href": "http://oeoa.test/openeo/1.1.0/collections", "rel": "parent"}, - {"href": "http://oeoa.test/openeo/1.1.0/collections/S2", "rel": "self"}, - ], } # TODO: test that caching of result is different from merging without error? (#2) diff --git a/tests/test_views.py b/tests/test_views.py index e649792d..45e99f7c 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -212,6 +212,63 @@ def test_collection_full_metadata_resilience(self, api100, requests_mock, backen api100.get("/collections/S4").assert_error(404, "CollectionNotFound") # TODO: test caching of results + def test_collections_links(self, api100, requests_mock, backend1, backend2): + requests_mock.get( + backend1 + "/collections", + json={ + "collections": [ + {"id": "S1"}, + { + "id": "S2", + "links": [ + {"rel": "license", "href": "foo"}, + # These parent/root/self links should be overruled in the result + {"rel": "parent", "href": "https://s2.test/"}, + {"rel": "self", "href": "https://s2.test/S2"}, + ], + }, + ] + }, + ) + requests_mock.get(backend2 + "/collections", json={"collections": [{"id": "S3"}]}) + res = api100.get("/collections").assert_status_code(200).json + assert res == { + "collections": [ + DictSubSet( + { + "description": "S1", + "links": [ + {"rel": "root", "href": "http://oeoa.test/openeo/1.0.0/collections"}, + {"rel": "parent", "href": "http://oeoa.test/openeo/1.0.0/collections"}, + {"rel": "self", "href": "http://oeoa.test/openeo/1.0.0/collections/S1"}, + ], + } + ), + DictSubSet( + { + "description": "S2", + "links": [ + {"rel": "license", "href": "foo"}, + {"rel": "root", "href": "http://oeoa.test/openeo/1.0.0/collections"}, + {"rel": "parent", "href": "http://oeoa.test/openeo/1.0.0/collections"}, + {"rel": "self", "href": "http://oeoa.test/openeo/1.0.0/collections/S2"}, + ], + } + ), + DictSubSet( + { + "description": "S3", + "links": [ + {"rel": "root", "href": "http://oeoa.test/openeo/1.0.0/collections"}, + {"rel": "parent", "href": "http://oeoa.test/openeo/1.0.0/collections"}, + {"rel": "self", "href": "http://oeoa.test/openeo/1.0.0/collections/S3"}, + ], + } + ), + ], + "links": [], + } + class TestAuthentication: def test_credentials_oidc_default(self, api100, backend1, backend2):