Skip to content

Commit

Permalink
Issue #74 eliminate flask dependency in normalize_collection_metadata
Browse files Browse the repository at this point in the history 
The root/parent/self link functionality moved to openeo-python-driver.
Avoids issue with background cache priming.
  • Loading branch information
@soxofaan
soxofaan committed Sep 21, 2023
1 parent cfc09cf commit df2e1a7
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 146 deletions.
2 changes: 1 addition & 1 deletion setup.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion src/openeo_aggregator/about.py
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = "0.10.3a1"
__version__ = "0.10.4a1"
4 changes: 2 additions & 2 deletions src/openeo_aggregator/backend.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand Down Expand Up @@ -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")
Expand Down
24 changes: 8 additions & 16 deletions src/openeo_aggregator/metadata/merging.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
126 changes: 0 additions & 126 deletions tests/test_backend.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
],
},
]

Expand All @@ -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",
Expand All @@ -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",
},
],
},
]

Expand Down Expand Up @@ -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",
},
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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"],
Expand Down Expand Up @@ -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"],
Expand Down Expand Up @@ -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"],
Expand Down Expand Up @@ -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"],
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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)

Expand Down
57 changes: 57 additions & 0 deletions tests/test_views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down

0 comments on commit df2e1a7

Please sign in to comment.