Skip to content

Commit

Permalink
Issue #112 move collection_whitelist to AggregatorBackendConfig
Browse files Browse the repository at this point in the history
currently unused, so no migration path necessary
  • Loading branch information
soxofaan committed Feb 8, 2024
1 parent 793c8d9 commit 556d87e
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 31 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.

The format is roughly based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).

## [0.18.4]

- Move `collection_whitelist` config to `AggregatorBackendConfig` ([#112](https://github.com/Open-EO/openeo-aggregator/issues/112))

## [0.18.3]

- Move `auth_entitlement_check` config to `AggregatorBackendConfig` ([#112](https://github.com/Open-EO/openeo-aggregator/issues/112))
Expand Down
2 changes: 1 addition & 1 deletion src/openeo_aggregator/about.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import sys
from typing import Optional

__version__ = "0.18.3a1"
__version__ = "0.18.4a1"


def log_version_info(logger: Optional[logging.Logger] = None):
Expand Down
6 changes: 3 additions & 3 deletions src/openeo_aggregator/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@ def __init__(self, backends: MultiBackendConnection, config: AggregatorConfig):
self.backends = backends
self._memoizer = memoizer_from_config(config=config, namespace="CollectionCatalog")
self.backends.on_connections_change.add(self._memoizer.invalidate)
self._collection_whitelist: Optional[List[str]] = config.collection_whitelist

def get_all_metadata(self) -> List[dict]:
metadata, internal = self._get_all_metadata_cached()
Expand All @@ -168,6 +167,7 @@ def _get_all_metadata(self) -> Tuple[List[dict], _InternalCollectionMetadata]:
"""
# Group collection metadata by hierarchically: collection id -> backend id -> metadata
grouped = defaultdict(dict)
collection_whitelist = get_backend_config().collection_whitelist
with TimingLogger(title="Collect collection metadata from all backends", logger=_log):
for con in self.backends:
try:
Expand All @@ -180,8 +180,8 @@ def _get_all_metadata(self) -> Tuple[List[dict], _InternalCollectionMetadata]:
for collection_metadata in backend_collections:
if "id" in collection_metadata:
collection_id = collection_metadata["id"]
if self._collection_whitelist:
if collection_id not in self._collection_whitelist:
if collection_whitelist:
if collection_id not in collection_whitelist:
_log.debug(f"Skipping non-whitelisted {collection_id=} from {con.id=}")
continue
else:
Expand Down
9 changes: 5 additions & 4 deletions src/openeo_aggregator/config.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import logging
import os
from pathlib import Path
from typing import Callable, List, Union
from typing import Callable, List, Optional, Union

import attrs
from openeo_driver.config import OpenEoBackendConfig
Expand Down Expand Up @@ -57,9 +57,6 @@ class AggregatorConfig(dict):
# TTL for connection caching.
connections_cache_ttl = dict_item(default=5 * 60.0)

# List of collection ids to cover with the aggregator (when None: support union of all upstream collections)
collection_whitelist = dict_item(default=None)

# Just a config field for test purposes (while were stripping down this config class)
test_dummy = dict_item(default="alice")

Expand Down Expand Up @@ -138,6 +135,10 @@ class AggregatorBackendConfig(OpenEoBackendConfig):

auth_entitlement_check: Union[bool, dict] = False

# List of collection ids to cover with the aggregator (when None: support union of all upstream collections)
collection_whitelist: Optional[List[str]] = None


# Internal singleton
_config_getter = ConfigGetter(expected_class=AggregatorBackendConfig)

Expand Down
47 changes: 24 additions & 23 deletions tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -304,42 +304,43 @@ def test_collections_links(self, api100, requests_mock, backend1, backend2):
}

@pytest.mark.parametrize(
["config_override", "expected"],
["collection_whitelist", "expected"],
[
({"collection_whitelist": None}, {"S1", "S2", "S3", "S4"}),
({"collection_whitelist": []}, {"S1", "S2", "S3", "S4"}),
({"collection_whitelist": ["S2"]}, {"S2"}),
({"collection_whitelist": ["S4"]}, {"S4"}),
({"collection_whitelist": ["S2", "S3"]}, {"S2", "S3"}),
({"collection_whitelist": ["S2", "S999"]}, {"S2"}),
({"collection_whitelist": ["S999"]}, set()),
(None, {"S1", "S2", "S3", "S4"}),
([], {"S1", "S2", "S3", "S4"}),
(["S2"], {"S2"}),
(["S4"], {"S4"}),
(["S2", "S3"], {"S2", "S3"}),
(["S2", "S999"], {"S2"}),
(["S999"], set()),
],
)
def test_collections_whitelist(self, api100, requests_mock, backend1, backend2, expected):
def test_collections_whitelist(self, api100, requests_mock, backend1, backend2, collection_whitelist, expected):
requests_mock.get(backend1 + "/collections", json={"collections": [{"id": "S1"}, {"id": "S2"}, {"id": "S3"}]})
for cid in ["S1", "S2", "S3"]:
requests_mock.get(backend1 + f"/collections/{cid}", json={"id": cid, "title": f"b1 {cid}"})
requests_mock.get(backend2 + "/collections", json={"collections": [{"id": "S3"}, {"id": "S4"}]})
for cid in ["S3", "S4"]:
requests_mock.get(backend2 + f"/collections/{cid}", json={"id": cid, "title": f"b2 {cid}"})

res = api100.get("/collections").assert_status_code(200).json
assert set(c["id"] for c in res["collections"]) == expected
with config_overrides(collection_whitelist=collection_whitelist):
res = api100.get("/collections").assert_status_code(200).json
assert set(c["id"] for c in res["collections"]) == expected

res = api100.get("/collections/S2")
if "S2" in expected:
assert res.assert_status_code(200).json == DictSubSet({"id": "S2", "title": "b1 S2"})
else:
res.assert_error(404, "CollectionNotFound")
res = api100.get("/collections/S2")
if "S2" in expected:
assert res.assert_status_code(200).json == DictSubSet({"id": "S2", "title": "b1 S2"})
else:
res.assert_error(404, "CollectionNotFound")

res = api100.get("/collections/S3")
if "S3" in expected:
assert res.assert_status_code(200).json == DictSubSet({"id": "S3", "title": "b1 S3"})
else:
res.assert_error(404, "CollectionNotFound")
res = api100.get("/collections/S3")
if "S3" in expected:
assert res.assert_status_code(200).json == DictSubSet({"id": "S3", "title": "b1 S3"})
else:
res.assert_error(404, "CollectionNotFound")

res = api100.get("/collections/S999")
res.assert_error(404, "CollectionNotFound")
res = api100.get("/collections/S999")
res.assert_error(404, "CollectionNotFound")


class TestAuthentication:
Expand Down

0 comments on commit 556d87e

Please sign in to comment.