Issue #84, Implement improvements from code review

src/openeo_aggregator/backend.py

@@ -677,14 +677,20 @@ def _get_connection_and_backend_service_id(
 
     def service_types(self) -> dict:
         """https://openeo.org/documentation/1.0/developers/api/reference.html#operation/list-service-types"""
-        cached_info = self._memoizer.get_or_call(key=("_get_service_types",), callback=self._get_service_types)
+        cached_info = self._get_service_types_cached()
         # Convert the cached results back to the format that service_types should return.
         return {name: data["service_type"] for name, data, in cached_info.items()}
 
-    def _find_backend_id_for_service_type(self, service_type: str) -> Optional[str]:
+    def _find_backend_id_for_service_type(self, service_type: str) -> str:
         """Returns the ID of the backend that provides the service_type or None if no backend supports that service."""
-        cached_info = self._memoizer.get_or_call(key=("_get_service_types",), callback=self._get_service_types)
-        return cached_info.get(service_type, {}).get("backend_id")
+        cached_info = self._get_service_types_cached()
+        backend_id = cached_info.get(service_type, {}).get("backend_id")
+        if backend_id is None:
+            raise ServiceUnsupportedException(service_type)
+        return backend_id
+
+    def _get_service_types_cached(self):
+        return self._memoizer.get_or_call(key=("service_types"), callback=self._get_service_types)
 
     def _get_service_types(self) -> dict:
         """Returns a dict that maps the service name to another dict that contains 2 items:
@@ -726,12 +732,6 @@ def _get_service_types(self) -> dict:
         # TODO: Issue #85 data about backend capabilities could be added to the service_types data structure as well.
         service_types = {}
 
-        # TODO: Instead of merge: prefix each type with backend-id? #83
-        def merge(services: dict, backend_id: str, new_service: dict):
-            for name, data in new_service.items():
-                if name.lower() not in {k.lower() for k in services.keys()}:
-                    services[name] = dict(backend_id=backend_id, service_type=data)
-
         # Collect all service types from the backends.
         for con in self._backends:
             # TODO: skip back-ends that do not support secondary services. https://github.com/Open-EO/openeo-aggregator/issues/78#issuecomment-1326180557
@@ -742,8 +742,16 @@ def merge(services: dict, backend_id: str, new_service: dict):
                 _log.warning(f"Failed to get service_types from {con.id}: {e!r}", exc_info=True)
                 continue
             # TODO #1 smarter merging:  parameter differences?
-            merge(service_types, con.id, types_to_add)
-
+            # TODO: Instead of merge: prefix each type with backend-id? #83
+            for name, data in types_to_add.items():
+                if name.lower() not in {k.lower() for k in service_types.keys()}:
+                    service_types[name] = dict(backend_id=con.id, service_type=data)
+                else:
+                    conflicting_backend = service_types[name]["backend_id"]
+                    _log.warning(
+                        f'Conflicting secondary service types: "{name}" is present in more than one backend, ' +
+                        f'already found in backend: {conflicting_backend}'
+                    )
         return service_types
 
     def list_services(self, user_id: str) -> List[ServiceMetadata]:
@@ -811,8 +819,6 @@ def create_service(self, user_id: str, process_graph: dict, service_type: str, a
             backend_id = "sentinelhub"
         else:
             backend_id = self._find_backend_id_for_service_type(service_type)
-            if backend_id is None:
-                raise ServiceUnsupportedException(service_type)
         process_graph = self._processing.preprocess_process_graph(process_graph, backend_id=backend_id)
 
         con = self._backends.get_connection(backend_id)

tests/test_backend.py

@@ -1,4 +1,5 @@
 import datetime as dt
+import logging
 
 import pytest
 
@@ -244,6 +245,49 @@ def test_service_types_multiple_backends(self, multi_backend_connection, config,
         expected_service_types.update(service_type_2)
         assert actual_service_types == expected_service_types
 
+    def test_service_types_warns_about_duplicate_service(self, multi_backend_connection, config, catalog,
+        backend1, backend2, requests_mock, caplog
+    ):
+        """
+        Given 2 backends which have conflicting service types,
+        then the aggregator lists only the service type from the first backend
+        and it logs a warning about the conflicting types.
+        """
+        caplog.set_level(logging.WARNING)
+        service_type_1 = {
+            "WMS": {
+                "title": "OGC Web Map Service",
+                "configuration": {},
+                "process_parameters": [],
+                "links": []
+            }
+        }
+        service_type_2 = {
+            "WMS": {
+                "title": "A duplicate OGC Web Map Service",
+                "configuration": {},
+                "process_parameters": [],
+                "links": []
+            }
+        }
+        requests_mock.get(backend1 + "/service_types", json=service_type_1)
+        requests_mock.get(backend2 + "/service_types", json=service_type_2)
+        processing = AggregatorProcessing(backends=multi_backend_connection, catalog=catalog, config=config)
+        implementation = AggregatorSecondaryServices(backends=multi_backend_connection, processing=processing, config=config)
+
+        actual_service_types = implementation.service_types()
+
+        # There were duplicate service types:
+        # Therefore it should find only one service type, and the log should contain a warning.
+        expected_service_types = dict(service_type_1)
+        assert actual_service_types == expected_service_types
+
+        expected_log_message = (
+            'Conflicting secondary service types: "WMS" is present in more than one backend, ' +
+            'already found in backend: b1'
+        )
+        assert expected_log_message in caplog.text
+
     @pytest.fixture
     def service_metadata_wmts_foo(self):
         return ServiceMetadata(