diff --git a/src/openeo_aggregator/backend.py b/src/openeo_aggregator/backend.py index f9283a81..ab0db071 100644 --- a/src/openeo_aggregator/backend.py +++ b/src/openeo_aggregator/backend.py @@ -656,7 +656,11 @@ def __init__( def _get_connection_and_backend_service_id( self, aggregator_service_id: str - ) -> Tuple[Union[BackendConnection, BackendConnection], str]: + ) -> Tuple[BackendConnection, str]: + """Get connection to the backend and the corresponding service ID in that backend. + + raises: ServiceNotFoundException when service_id does not exist in any of the backends. + """ backend_service_id, backend_id = ServiceIdMapping.parse_aggregator_service_id( backends=self._backends, aggregator_service_id=aggregator_service_id @@ -718,28 +722,23 @@ def merge(services, to_add): def service_info(self, user_id: str, service_id: str) -> ServiceMetadata: """https://openeo.org/documentation/1.0/developers/api/reference.html#operation/describe-service""" - # con, backend_service_id = self._get_connection_and_backend_service_id(service_id) - # with con.authenticated_from_request(request=flask.request, user=User(user_id)): - # try: - # service_json = con.get(f"/services/{backend_service_id}").json() - # except Exception as e: - # _log.debug(f"Failed to get service with ID={backend_service_id} from backend with ID={con.id}: {e!r}", exc_info=True) - # raise - # else: - # service_json["id"] = ServiceIdMapping.get_aggregator_service_id(service_json["id"], con.id) - # return ServiceMetadata.from_dict(service_json) - - for con in self._backends: - with con.authenticated_from_request(request=flask.request, user=User(user_id)): - try: - service_json = con.get(f"/services/{service_id}").json() - except Exception as e: - _log.debug(f"No service with ID={service_id} in backend with ID={con.id}: {e!r}", exc_info=True) - continue - else: - return ServiceMetadata.from_dict(service_json) - - raise ServiceNotFoundException(service_id) + con, backend_service_id = self._get_connection_and_backend_service_id(service_id) + with con.authenticated_from_request(request=flask.request, user=User(user_id)): + try: + service_json = con.get(f"/services/{backend_service_id}").json() + except (OpenEoApiError) as e: + if e.http_status_code == 404: + # Expected error + _log.debug(f"No service with ID={service_id!r} in backend with ID={con.id!r}: {e!r}", exc_info=True) + raise ServiceNotFoundException(service_id=service_id) from e + raise + except Exception as e: + _log.debug(f"Failed to get service with ID={backend_service_id} from backend with ID={con.id}: {e!r}", exc_info=True) + raise + else: + # Adapt the service ID so it points to the aggregator, with the backend ID included. + service_json["id"] = ServiceIdMapping.get_aggregator_service_id(service_json["id"], con.id) + return ServiceMetadata.from_dict(service_json) def create_service(self, user_id: str, process_graph: dict, service_type: str, api_version: str, configuration: dict) -> str: @@ -768,72 +767,50 @@ def create_service(self, user_id: str, process_graph: dict, service_type: str, a except (OpenEoRestError, OpenEoClientException) as e: raise OpenEOApiException(f"Failed to create secondary service on backend {backend_id!r}: {e!r}") - return service.service_id - - def _find_connection_with_service_id(self, user_id: str, service_id: str) -> Optional[BackendConnection]: - """Get connection for the backend that contains the service, return None if not found.""" - - # Search all services on the backends. - for con in self._backends: - with con.authenticated_from_request(request=flask.request, user=User(user_id)): - try: - _ = con.get(f"/services/{service_id}") - except OpenEoApiError as e: - if e.http_status_code == 404: - # Expected error - _log.debug(f"No service with ID={service_id!r} in backend with ID={con.id!r}: {e!r}", exc_info=True) - continue - else: - _log.warning(f"Failed to get service {service_id!r} from {con.id!r}: {e!r}", exc_info=True) - raise e - except Exception as e: - _log.warning(f"Failed to get service {service_id!r} from {con.id!r}: {e!r}", exc_info=True) - raise e - else: - return con - return None + return ServiceIdMapping.get_aggregator_service_id(service.service_id, backend_id) def remove_service(self, user_id: str, service_id: str) -> None: """https://openeo.org/documentation/1.0/developers/api/reference.html#operation/delete-service""" - con = self._find_connection_with_service_id(user_id, service_id) - if not con: - raise ServiceNotFoundException(service_id) + # Will raise ServiceNotFoundException if service_id does not exist in any of the backends. + con, backend_service_id = self._get_connection_and_backend_service_id(service_id) with con.authenticated_from_request(request=flask.request, user=User(user_id)): try: - con.delete(f"/services/{service_id}", expected_status=204) - except (OpenEoApiError, OpenEOApiException) as e: - # TODO: maybe we should just let these exception straight go to the caller without logging it here. - # Logging it here seems prudent and more consistent with the handling of unexpected exceptions below. - _log.warning(f"Failed to delete service {service_id!r} from {con.id!r}: {e!r}", exc_info=True) + con.delete(f"/services/{backend_service_id}", expected_status=204) + except (OpenEoApiError) as e: + if e.http_status_code == 404: + # Expected error + _log.debug(f"No service with ID={service_id!r} in backend with ID={con.id!r}: {e!r}", exc_info=True) + raise ServiceNotFoundException(service_id=service_id) from e + _log.warning(f"Failed to delete service {backend_service_id!r} from {con.id!r}: {e!r}", exc_info=True) raise except Exception as e: - _log.warning(f"Failed to delete service {service_id!r} from {con.id!r}: {e!r}", exc_info=True) + _log.warning(f"Failed to delete service {backend_service_id!r} from {con.id!r}: {e!r}", exc_info=True) raise OpenEOApiException( - f"Failed to delete service {service_id!r} on backend {con.id!r}: {e!r}" + f"Failed to delete service {backend_service_id!r} on backend {con.id!r}: {e!r}" ) from e def update_service(self, user_id: str, service_id: str, process_graph: dict) -> None: """https://openeo.org/documentation/1.0/developers/api/reference.html#operation/update-service""" - con = self._find_connection_with_service_id(user_id, service_id) - if not con: - raise ServiceNotFoundException(service_id) + # Will raise ServiceNotFoundException if service_id does not exist in any of the backends. + con, backend_service_id = self._get_connection_and_backend_service_id(service_id) with con.authenticated_from_request(request=flask.request, user=User(user_id)): try: json = {"process": {"process_graph": process_graph}} - con.patch(f"/services/{service_id}", json=json, expected_status=204) - except (OpenEoApiError, OpenEOApiException) as e: - # TODO: maybe we should just let these exception straight go to the caller without logging it here. - # Logging it here seems prudent and more consistent with the handling of unexpected exceptions below. - _log.warning(f"Failed to update service {service_id!r} from {con.id!r}: {e!r}", exc_info=True) + con.patch(f"/services/{backend_service_id}", json=json, expected_status=204) + except (OpenEoApiError) as e: + if e.http_status_code == 404: + # Expected error + _log.debug(f"No service with ID={backend_service_id!r} in backend with ID={con.id!r}: {e!r}", exc_info=True) + raise ServiceNotFoundException(service_id=service_id) from e raise except Exception as e: - _log.warning(f"Failed to update service {service_id!r} from {con.id!r}: {e!r}", exc_info=True) + _log.warning(f"Failed to update service {backend_service_id!r} from {con.id!r}: {e!r}", exc_info=True) raise OpenEOApiException( - f"Failed to update service {service_id!r} from {con.id!r}: {e!r}" + f"Failed to update service {backend_service_id!r} from {con.id!r}: {e!r}" ) from e diff --git a/tests/test_backend.py b/tests/test_backend.py index 1f913a2e..13de88bf 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -354,8 +354,10 @@ def test_service_info_succeeds( service_metadata_wmts_foo, service_metadata_wms_bar ): """When it gets a correct service ID, it returns the expected ServiceMetadata.""" - requests_mock.get(backend1 + "/services/wmts-foo", json=service_metadata_wmts_foo.prepare_for_json()) - requests_mock.get(backend2 + "/services/wms-bar", json=service_metadata_wms_bar.prepare_for_json()) + json_wmts_foo = service_metadata_wmts_foo.prepare_for_json() + json_wms_bar = service_metadata_wms_bar.prepare_for_json() + requests_mock.get(backend1 + "/services/wmts-foo", json=json_wmts_foo) + requests_mock.get(backend2 + "/services/wms-bar", json=json_wms_bar) processing = AggregatorProcessing(backends=multi_backend_connection, catalog=catalog, config=config) implementation = AggregatorSecondaryServices(backends=multi_backend_connection, processing=processing) api100.set_auth_bearer_token(TEST_USER_BEARER_TOKEN) @@ -363,21 +365,30 @@ def test_service_info_succeeds( # Check the expected metadata on *both* of the services. with flask_app.test_request_context(headers=headers): - actual_service1 = implementation.service_info(user_id=TEST_USER, service_id="wmts-foo") - assert actual_service1 == service_metadata_wmts_foo + actual_service1 = implementation.service_info(user_id=TEST_USER, service_id="b1-wmts-foo") + + json = dict(json_wmts_foo) + json["id"] = "b1-" + json["id"] + expected_service1 = ServiceMetadata.from_dict(json) + + assert actual_service1 == expected_service1 with flask_app.test_request_context(headers=headers): - actual_service2 = implementation.service_info(user_id=TEST_USER, service_id="wms-bar") - assert actual_service2 == service_metadata_wms_bar + actual_service2 = implementation.service_info(user_id=TEST_USER, service_id="b2-wms-bar") - def test_service_info_wrong_id( - self, flask_app, api100, multi_backend_connection, config, catalog, backend1, backend2, requests_mock, - service_metadata_wmts_foo, service_metadata_wms_bar + json = dict(json_wms_bar) + json["id"] = "b2-" + json["id"] + expected_service2 = ServiceMetadata.from_dict(json) + + assert actual_service2 == expected_service2 + + def test_service_info_wrong_backend_id( + self, flask_app, api100, multi_backend_connection, config, catalog, backend1, requests_mock, + service_metadata_wmts_foo ): """When it gets a non-existent service ID, it raises a ServiceNotFoundException.""" requests_mock.get(backend1 + "/services/wmts-foo", json=service_metadata_wmts_foo.prepare_for_json()) - requests_mock.get(backend2 + "/services/wms-bar", json=service_metadata_wms_bar.prepare_for_json()) processing = AggregatorProcessing(backends=multi_backend_connection, catalog=catalog, config=config) implementation = AggregatorSecondaryServices(backends=multi_backend_connection, processing=processing) api100.set_auth_bearer_token(TEST_USER_BEARER_TOKEN) @@ -385,7 +396,24 @@ def test_service_info_wrong_id( with flask_app.test_request_context(headers=headers): with pytest.raises(ServiceNotFoundException): - implementation.service_info(user_id=TEST_USER, service_id="doesnotexist") + implementation.service_info(user_id=TEST_USER, service_id="backenddoesnotexist-wtms-foo") + + def test_service_info_wrong_service_id( + self, flask_app, api100, multi_backend_connection, config, catalog, backend1, requests_mock, + ): + """When it gets a non-existent service ID, it raises a ServiceNotFoundException.""" + + requests_mock.get(backend1 + "/services/service-does-not-exist", status_code=404) + processing = AggregatorProcessing(backends=multi_backend_connection, catalog=catalog, config=config) + implementation = AggregatorSecondaryServices(backends=multi_backend_connection, processing=processing) + api100.set_auth_bearer_token(TEST_USER_BEARER_TOKEN) + headers = TEST_USER_AUTH_HEADER + + with flask_app.test_request_context(headers=headers): + with pytest.raises(ServiceNotFoundException): + implementation.service_info(user_id=TEST_USER, service_id="b1-service-does-not-exist") + + assert requests_mock.called def test_create_service_succeeds( self, flask_app, api100, multi_backend_connection, config, catalog, backend1, requests_mock @@ -393,13 +421,16 @@ def test_create_service_succeeds( """When it gets a correct params for a new service, it successfully creates it.""" # Set up responses for creating the service in backend 1 - expected_openeo_id = "wmts-foo" - location_backend_1 = backend1 + "/services/" + expected_openeo_id + backend_service_id = "wmts-foo" + # The aggregator should prepend the service_id with the backend_id + expected_service_id = "b1-wmts-foo" + + location_backend_1 = backend1 + "/services/" + backend_service_id process_graph = {"foo": {"process_id": "foo", "arguments": {}}} requests_mock.post( backend1 + "/services", headers={ - "OpenEO-Identifier": expected_openeo_id, + "OpenEO-Identifier": backend_service_id, "Location": location_backend_1 }, status_code=201 @@ -417,12 +448,12 @@ def test_create_service_succeeds( api_version="1.0.0", configuration={} ) - assert actual_openeo_id == expected_openeo_id + assert actual_openeo_id == expected_service_id @pytest.mark.parametrize("exception_class", [OpenEoApiError, OpenEoRestError]) def test_create_service_backend_raises_openeoapiexception( self, flask_app, api100, multi_backend_connection, config, catalog, - backend1, backend2, requests_mock, exception_class + backend1, requests_mock, exception_class ): """When the backend raises a general exception the aggregator raises an OpenEOApiException.""" @@ -483,91 +514,66 @@ def test_create_service_backend_reraises( ) def test_remove_service_succeeds( - self, flask_app, api100, multi_backend_connection, config, catalog, - backend1, backend2, requests_mock, service_metadata_wmts_foo + self, flask_app, api100, multi_backend_connection, config, catalog, backend1, requests_mock ): """When remove_service is called with an existing service ID, it removes service and returns HTTP 204.""" - # Also test that it can skip backends that don't have the service - mock_get1 = requests_mock.get( - backend1 + "/services/wmts-foo", - status_code=404 - ) - # Delete should succeed in backend2 so service should be present first. - mock_get2 = requests_mock.get( - backend2 + "/services/wmts-foo", - json=service_metadata_wmts_foo.prepare_for_json(), - status_code=200 - ) - mock_delete = requests_mock.delete(backend2 + "/services/wmts-foo", status_code=204) + mock_delete = requests_mock.delete(backend1 + "/services/wmts-foo", status_code=204) processing = AggregatorProcessing(backends=multi_backend_connection, catalog=catalog, config=config) implementation = AggregatorSecondaryServices(backends=multi_backend_connection, processing=processing) api100.set_auth_bearer_token(TEST_USER_BEARER_TOKEN) headers = TEST_USER_AUTH_HEADER with flask_app.test_request_context(headers=headers): - implementation.remove_service(user_id=TEST_USER, service_id="wmts-foo") + implementation.remove_service(user_id=TEST_USER, service_id="b1-wmts-foo") # Make sure the aggregator asked the backend to remove the service. assert mock_delete.called - # Check the other mocks were called too, just to be sure. - assert mock_get1.called - assert mock_get2.called + def test_remove_service_but_backend_id_not_found( + self, flask_app, api100, multi_backend_connection, config, catalog, + ): + """When the backend ID/prefix does not exist then the aggregator raises an ServiceNotFoundException.""" + + processing = AggregatorProcessing(backends=multi_backend_connection, catalog=catalog, config=config) + implementation = AggregatorSecondaryServices(backends=multi_backend_connection, processing=processing) + api100.set_auth_bearer_token(TEST_USER_BEARER_TOKEN) + headers = TEST_USER_AUTH_HEADER - def test_remove_service_service_id_not_found( + # Case 1: the backend doesn't even exist + with flask_app.test_request_context(headers=headers): + with pytest.raises(ServiceNotFoundException): + implementation.remove_service(user_id=TEST_USER, service_id="doesnotexist-wmts-foo") + + def test_remove_service_but_service_id_not_found( self, flask_app, api100, multi_backend_connection, config, catalog, - backend1, backend2, requests_mock, service_metadata_wmts_foo + backend1, requests_mock ): - """When the service ID does not exist then the aggregator raises an ServiceNotFoundException.""" + """When the service ID does not exist for the specified backend then the aggregator raises an ServiceNotFoundException.""" - # Neither backend has the service available, and the aggregator should detect this. - mock_get1 = requests_mock.get( - backend1 + "/services/wmts-foo", - json=service_metadata_wmts_foo.prepare_for_json(), - status_code=404 - ) - mock_get2 = requests_mock.get( - backend2 + "/services/wmts-foo", - status_code=404 - ) - - # These requests should not be executed, so check they are not called. - mock_delete1 = requests_mock.delete( - backend2 + "/services/wmts-foo", - status_code=204 - ) - mock_delete2 = requests_mock.delete( - backend2 + "/services/wmts-foo", - status_code=204 - ) processing = AggregatorProcessing(backends=multi_backend_connection, catalog=catalog, config=config) implementation = AggregatorSecondaryServices(backends=multi_backend_connection, processing=processing) api100.set_auth_bearer_token(TEST_USER_BEARER_TOKEN) headers = TEST_USER_AUTH_HEADER + # The backend exists but the service ID does not. + mock_delete1 = requests_mock.delete( + backend1 + "/services/doesnotexist", + status_code=404 + ) with flask_app.test_request_context(headers=headers): with pytest.raises(ServiceNotFoundException): - implementation.remove_service(user_id=TEST_USER, service_id="wmts-foo") + implementation.remove_service(user_id=TEST_USER, service_id="b1-doesnotexist") - assert not mock_delete1.called - assert not mock_delete2.called - # Check the other mocks were called too, just to be sure. - assert mock_get1.called - assert mock_get2.called + # This should have tried to delete it on the backend so the mock must be called. + assert mock_delete1.called def test_remove_service_backend_response_is_an_error_status( self, flask_app, api100, multi_backend_connection, config, catalog, - backend1, requests_mock, service_metadata_wmts_foo - ): - """When the backend response is an error HTTP 400/500 then the aggregator raises an OpenEoApiError.""" + backend1, requests_mock + ): + """When the backend response is an HTTP error status then the aggregator raises an OpenEoApiError.""" - # Will find it on the first backend, and it should skip the second backend so we don't add it to backend2. - requests_mock.get( - backend1 + "/services/wmts-foo", - json=service_metadata_wmts_foo.prepare_for_json(), - status_code=200 - ) requests_mock.delete(backend1 + "/services/wmts-foo", status_code=500) processing = AggregatorProcessing(backends=multi_backend_connection, catalog=catalog, config=config) implementation = AggregatorSecondaryServices(backends=multi_backend_connection, processing=processing) @@ -576,32 +582,20 @@ def test_remove_service_backend_response_is_an_error_status( with flask_app.test_request_context(headers=headers): with pytest.raises(OpenEoApiError) as e: - implementation.remove_service(user_id=TEST_USER, service_id="wmts-foo") + implementation.remove_service(user_id=TEST_USER, service_id="b1-wmts-foo") # If the backend reports HTTP 400/500, we would expect the same status code from the aggregator. # TODO: Statement above is an assumption. Is that really what we expect? assert e.value.http_status_code == 500 - # TODO: this test still fails with API version 1.0.0 def test_update_service_succeeds( self, flask_app, api100, multi_backend_connection, config, catalog, - backend1, backend2, requests_mock, service_metadata_wmts_foo + backend1, requests_mock ): """When it receives an existing service ID and a correct payload, it updates the expected service.""" - # Also test that it can skip backends that don't have the service - mock_get1 = requests_mock.get( - backend1 + "/services/wmts-foo", - status_code=404 - ) - # Update should succeed in backend2 so service should be present - mock_get2 = requests_mock.get( - backend2 + "/services/wmts-foo", - json=service_metadata_wmts_foo.prepare_for_json(), - status_code=200 - ) mock_patch = requests_mock.patch( - backend2 + "/services/wmts-foo", + backend1 + "/services/wmts-foo", status_code=204, ) processing = AggregatorProcessing(backends=multi_backend_connection, catalog=catalog, config=config) @@ -611,7 +605,7 @@ def test_update_service_succeeds( headers = TEST_USER_AUTH_HEADER with flask_app.test_request_context(headers=headers): - implementation.update_service(user_id=TEST_USER, service_id="wmts-foo", process_graph=process_graph_after) + implementation.update_service(user_id=TEST_USER, service_id="b1-wmts-foo", process_graph=process_graph_after) # Make sure the aggregator asked the backend to remove the service. assert mock_patch.called @@ -620,33 +614,31 @@ def test_update_service_succeeds( expected_process = {"process": {"process_graph": process_graph_after}} assert mock_patch.last_request.json() == expected_process - # Check the other mocks were called too, just to be sure. - assert mock_get1.called - assert mock_get2.called + def test_update_service_but_backend_id_does_not_exist( + self, flask_app, api100, multi_backend_connection, config, catalog, + ): + """When the backend ID/prefix does not exist then the aggregator raises an ServiceNotFoundException.""" - def test_update_service_service_id_not_found( + processing = AggregatorProcessing(backends=multi_backend_connection, catalog=catalog, config=config) + implementation = AggregatorSecondaryServices(backends=multi_backend_connection, processing=processing) + process_graph_after = {"bar": {"process_id": "bar", "arguments": {"arg1": "bar"}}} + api100.set_auth_bearer_token(TEST_USER_BEARER_TOKEN) + headers = TEST_USER_AUTH_HEADER + + with flask_app.test_request_context(headers=headers): + with pytest.raises(ServiceNotFoundException): + implementation.update_service(user_id=TEST_USER, service_id="doesnotexist-wmts-foo", process_graph=process_graph_after) + + def test_update_service_but_service_id_not_found( self, flask_app, api100, multi_backend_connection, config, catalog, backend1, backend2, requests_mock ): - """When the service ID does not exist then the aggregator raises an ServiceNotFoundException.""" + """When the service ID does not exist for the specified backend then the aggregator raises an ServiceNotFoundException.""" - mock_get1 = requests_mock.get( - backend1 + "/services/wmts-foo", - status_code=404 - ) - mock_get2 = requests_mock.get( - backend2 + "/services/wmts-foo", - status_code=404 - ) - # These requests should not be executed, so check they are not called. mock_patch1 = requests_mock.patch( - backend1 + "/services/wmts-foo", - status_code=204, - ) - mock_patch2 = requests_mock.patch( - backend2 + "/services/wmts-foo", - status_code=204, + backend1 + "/services/doesnotexist", + status_code=404, ) processing = AggregatorProcessing(backends=multi_backend_connection, catalog=catalog, config=config) implementation = AggregatorSecondaryServices(backends=multi_backend_connection, processing=processing) @@ -656,25 +648,16 @@ def test_update_service_service_id_not_found( with flask_app.test_request_context(headers=headers): with pytest.raises(ServiceNotFoundException): - implementation.update_service(user_id=TEST_USER, service_id="wmts-foo", process_graph=process_graph_after) + implementation.update_service(user_id=TEST_USER, service_id="b1-doesnotexist", process_graph=process_graph_after) - assert not mock_patch1.called - assert not mock_patch2.called - # Check the other mocks were called too, just to be sure. - assert mock_get1.called - assert mock_get2.called + assert mock_patch1.called def test_update_service_backend_response_is_an_error_status( self, flask_app, api100, multi_backend_connection, config, catalog, - backend1, backend2, requests_mock, service_metadata_wmts_foo + backend1, requests_mock ): - """When the backend response is an error HTTP 400/500 then the aggregator raises an OpenEoApiError.""" + """When the backend response is an HTTP error status then the aggregator raises an OpenEoApiError.""" - requests_mock.get( - backend1 + "/services/wmts-foo", - json=service_metadata_wmts_foo.prepare_for_json(), - status_code=200 - ) mock_patch = requests_mock.patch( backend1 + "/services/wmts-foo", status_code=500, @@ -687,7 +670,7 @@ def test_update_service_backend_response_is_an_error_status( with flask_app.test_request_context(headers=headers): with pytest.raises(OpenEoApiError) as e: - implementation.update_service(user_id=TEST_USER, service_id="wmts-foo", process_graph=new_process_graph) + implementation.update_service(user_id=TEST_USER, service_id="b1-wmts-foo", process_graph=new_process_graph) assert e.value.http_status_code == 500 assert mock_patch.called @@ -1674,6 +1657,29 @@ def test_parse_aggregator_job_id_fail(self, multi_backend_connection): ) +from openeo_aggregator.backend import ServiceIdMapping +class TestServiceIdMapping: + + def test_get_aggregator_job_id(self): + assert ServiceIdMapping.get_aggregator_service_id( + backend_service_id="service-x17-abc", backend_id="vito" + ) == "vito-service-x17-abc" + + def test_parse_aggregator_job_id(self, multi_backend_connection): + assert ServiceIdMapping.parse_aggregator_service_id( + backends=multi_backend_connection, aggregator_service_id="b1-serv021b" + ) == ("serv021b", "b1") + assert ServiceIdMapping.parse_aggregator_service_id( + backends=multi_backend_connection, aggregator_service_id="b2-someservice-321-ab14jh" + ) == ("someservice-321-ab14jh", "b2") + + def test_parse_aggregator_job_id_fail(self, multi_backend_connection): + with pytest.raises(ServiceNotFoundException): + ServiceIdMapping.parse_aggregator_service_id( + backends=multi_backend_connection, aggregator_service_id="b3-b6tch-j0b-o123423" + ) + + class TestAggregatorProcessing: def test_get_process_registry( self, diff --git a/tests/test_views.py b/tests/test_views.py index a1368a9f..f3c680e1 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -1335,19 +1335,6 @@ def service_metadata_wmts_foo(self): # not setting "created": This is used to test creating a service. ) - @pytest.fixture - def service_metadata_wms_bar(self): - return ServiceMetadata( - id="wms-bar", - process={"process_graph": {"bar": {"process_id": "bar", "arguments": {}}}}, - url='https://oeo.net/wms/bar', - type="WMS", - enabled=True, - configuration={"version": "0.5.8"}, - attributes={}, - title="Test WMS service" - # not setting "created": This is used to test creating a service. - ) def test_service_types_simple(self, api100, backend1, backend2, requests_mock): """Given 2 backends but only 1 backend has a single service, then the aggregator @@ -1422,64 +1409,56 @@ def test_service_types_merging(self, api100, backend1, backend2, requests_mock): assert actual_service_types == expected_service_types def test_service_info( - self, api100, backend1, backend2, requests_mock, service_metadata_wmts_foo, service_metadata_wms_bar + self, api100, backend1, requests_mock ): - """When it gets a correct service ID, it returns the expected ServiceMetadata.""" - - requests_mock.get(backend1 + "/services/wmts-foo", json=service_metadata_wmts_foo.prepare_for_json()) - requests_mock.get(backend2 + "/services/wms-bar", json=service_metadata_wms_bar.prepare_for_json()) + """When it gets a correct service ID, it returns the expected service's metadata as JSON.""" + + json_wmts_foo = { + "id": "wmts-foo", + "process": {"process_graph": {"foo": {"process_id": "foo", "arguments": {}}}}, + "url": "https://oeo.net/wmts/foo", + "type": "WMTS", + "enabled": "True", + "configuration": {"version": "0.5.8"}, + "attributes": {}, + "title": "Test WMTS service" + } + requests_mock.get(backend1 + "/services/wmts-foo", json=json_wmts_foo) api100.set_auth_bearer_token(token=TEST_USER_BEARER_TOKEN) - # Retrieve and verify the metadata for both services - resp = api100.get("/services/wmts-foo").assert_status_code(200) - actual_service1 = ServiceMetadata( - id=resp.json["id"], - process=resp.json["process"], - url=resp.json["url"], - type=resp.json["type"], - enabled=resp.json["enabled"], - configuration=resp.json["configuration"], - attributes=resp.json["attributes"], - title=resp.json["title"], - ) - assert actual_service1 == service_metadata_wmts_foo - - resp = api100.get("/services/wms-bar").assert_status_code(200) - actual_service2 = ServiceMetadata( - id=resp.json["id"], - process=resp.json["process"], - url=resp.json["url"], - type=resp.json["type"], - enabled=resp.json["enabled"], - configuration=resp.json["configuration"], - attributes=resp.json["attributes"], - title=resp.json["title"], - ) - assert actual_service2 == service_metadata_wms_bar + resp = api100.get("/services/b1-wmts-foo").assert_status_code(200) - def test_service_info_wrong_id( - self, api100, backend1, backend2, requests_mock, service_metadata_wmts_foo, service_metadata_wms_bar - ): - """When it gets a non-existent service ID, it returns HTTP Status 404, Not found.""" + expected_json_wmts_foo = dict(json_wmts_foo) + expected_json_wmts_foo["id"] = "b1-" + json_wmts_foo["id"] + assert resp.json == expected_json_wmts_foo - requests_mock.get(backend1 + "/services/wmts-foo", json=service_metadata_wmts_foo.prepare_for_json()) - requests_mock.get(backend2 + "/services/wms-bar", json=service_metadata_wms_bar.prepare_for_json()) + def test_service_info_wrong_id(self, api100): + """When it gets a non-existent service ID, the aggregator responds with HTTP 404, not found.""" api100.set_auth_bearer_token(token=TEST_USER_BEARER_TOKEN) - api100.get("/services/doesnotexist").assert_status_code(404) + # The backend ID is wrong. + api100.get("/services/doesnotexist-someservice").assert_status_code(404) + + # The backend ID exists but the service ID is wrong. + api100.get("/services/b1-doesnotexist").assert_status_code(404) def test_create_wmts(self, api100, requests_mock, backend1): + """When the payload is correct the service should be successfully created, + the service ID should be prepended with the backend ID, + and location should point to the aggregator, not to the backend directly. + """ api100.set_auth_bearer_token(TEST_USER_BEARER_TOKEN) - expected_openeo_id = 'c63d6c27-c4c2-4160-b7bd-9e32f582daec' - # The aggregator MUST NOT point to the actual instance but to its own endpoint. + backend_service_id = 'c63d6c27-c4c2-4160-b7bd-9e32f582daec' + expected_openeo_id = "b1-" + backend_service_id + + # The aggregator MUST NOT point to the backend instance but to its own endpoint. # This is handled by the openeo python driver in openeo_driver.views.services_post. expected_location = "/openeo/1.0.0/services/" + expected_openeo_id # However, backend1 must report its OWN location. - location_backend_1 = backend1 + "/services" + expected_openeo_id + location_backend_1 = backend1 + "/services" + backend_service_id process_graph = {"foo": {"process_id": "foo", "arguments": {}}} - # The process_graph/process format is slightly different between api v0.4 and v1.0 post_data = { "type": 'WMTS', "process": { @@ -1492,7 +1471,7 @@ def test_create_wmts(self, api100, requests_mock, backend1): requests_mock.post( backend1 + "/services", headers={ - "OpenEO-Identifier": expected_openeo_id, + "OpenEO-Identifier": backend_service_id, "Location": location_backend_1 }, status_code=201 @@ -1500,16 +1479,17 @@ def test_create_wmts(self, api100, requests_mock, backend1): resp = api100.post('/services', json=post_data).assert_status_code(201) - assert resp.headers['OpenEO-Identifier'] == 'c63d6c27-c4c2-4160-b7bd-9e32f582daec' + assert resp.headers['OpenEO-Identifier'] == expected_openeo_id assert resp.headers['Location'] == expected_location # ProcessGraphMissingException and ProcessGraphInvalidException are well known reasons for a bad client request. @pytest.mark.parametrize("exception_class", [ProcessGraphMissingException, ProcessGraphInvalidException]) def test_create_wmts_reports_400_client_error(self, api100, requests_mock, backend1, exception_class): - api100.set_auth_bearer_token(TEST_USER_BEARER_TOKEN) + """When the backend raises exceptions that are typically a bad request / HTTP 400, then + we expect the aggregator to return a HTTP 400 status code.""" + api100.set_auth_bearer_token(TEST_USER_BEARER_TOKEN) process_graph = {"foo": {"process_id": "foo", "arguments": {}}} - # The process_graph/process format is slightly different between api v0.4 and v1.0 post_data = { "type": 'WMTS', "process": { @@ -1532,10 +1512,12 @@ def test_create_wmts_reports_400_client_error(self, api100, requests_mock, backe # OpenEoApiError, OpenEoRestError: more general errors we can expect to lead to a HTTP 500 server error. @pytest.mark.parametrize("exception_class", [OpenEoApiError, OpenEoRestError]) def test_create_wmts_reports_500_server_error(self, api100, requests_mock, backend1, exception_class): + """When the backend raises exceptions that are typically a server error / HTTP 500, then + we expect the aggregator to return a HTTP 500 status code.""" + api100.set_auth_bearer_token(TEST_USER_BEARER_TOKEN) process_graph = {"foo": {"process_id": "foo", "arguments": {}}} - # The process_graph/process format is slightly different between api v0.4 and v1.0 post_data = { "type": 'WMTS', "process": { @@ -1554,70 +1536,51 @@ def test_create_wmts_reports_500_server_error(self, api100, requests_mock, backe assert resp.status_code == 500 def test_remove_service_succeeds( - self, api100, requests_mock, backend1, backend2, service_metadata_wmts_foo + self, api100, requests_mock, backend1 ): """When remove_service is called with an existing service ID, it removes service and returns HTTP 204.""" - api100.set_auth_bearer_token(TEST_USER_BEARER_TOKEN) - # Also test that it skips backends that don't have the service2 - mock_get1 = requests_mock.get( - backend1 + "/services/wmts-foo", - status_code=404 - ) - # Delete should succeed in backend2 so service should be present first. - mock_get2 = requests_mock.get( - backend2 + "/services/wmts-foo", - json=service_metadata_wmts_foo.prepare_for_json(), - status_code=200 - ) - mock_delete = requests_mock.delete(backend2 + "/services/wmts-foo", status_code=204) + api100.set_auth_bearer_token(TEST_USER_BEARER_TOKEN) + mock_delete = requests_mock.delete(backend1 + "/services/wmts-foo", status_code=204) - resp = api100.delete("/services/wmts-foo") + resp = api100.delete("/services/b1-wmts-foo") assert resp.status_code == 204 # Make sure the aggregator asked the backend to remove the service. assert mock_delete.called - # Verify the aggregator did query the backends to find the service. - assert mock_get1.called - assert mock_get1.called - def test_remove_service_service_id_not_found( - self, api100, backend1, backend2, requests_mock, service_metadata_wmts_foo - ): - """When the service ID does not exist then the aggregator raises an ServiceNotFoundException.""" + + def test_remove_service_but_backend_id_not_found(self, api100): + """When the service ID does not exist then the aggregator responds with HTTP 404, not found.""" + api100.set_auth_bearer_token(TEST_USER_BEARER_TOKEN) - # Neither backend has the service available, and the aggregator should detect this. - mock_get1 = requests_mock.get( + resp = api100.delete("/services/wmts-foo") + + assert resp.status_code == 404 + + def test_remove_service_but_service_id_not_found( + self, api100, backend1, requests_mock + ): + """When the service ID does not exist then the aggregator responds with HTTP 404, not found.""" + + api100.set_auth_bearer_token(TEST_USER_BEARER_TOKEN) + mock_delete = requests_mock.delete( backend1 + "/services/wmts-foo", - json=service_metadata_wmts_foo.prepare_for_json(), - status_code=404 - ) - mock_get2 = requests_mock.get( - backend2 + "/services/wmts-foo", status_code=404, - ) - mock_delete = requests_mock.delete( - backend2 + "/services/wmts-foo", - status_code=204, # deliberately avoid 404 so we know 404 comes from aggregator. ) - resp = api100.delete("/services/wmts-foo") + resp = api100.delete("/services/b1-wmts-foo") assert resp.status_code == 404 - # Verify the aggregator did not call the backend to remove the service. - assert not mock_delete.called - # Verify the aggregator did query the backends to find the service. - assert mock_get1.called - assert mock_get2.called + assert mock_delete.called def test_remove_service_backend_response_is_an_error_status( - self, api100, requests_mock, backend1, backend2, service_metadata_wmts_foo + self, api100, requests_mock, backend1, service_metadata_wmts_foo ): - """When the backend response is an error HTTP 400/500 then the aggregator raises an OpenEoApiError.""" - api100.set_auth_bearer_token(TEST_USER_BEARER_TOKEN) + """When the backend response is an error, HTTP 500, then the aggregator also responds with HTTP 500 status.""" - # Will find it on the first backend, and it should skip the second backend so we don't add it to backend2. + api100.set_auth_bearer_token(TEST_USER_BEARER_TOKEN) requests_mock.get( backend1 + "/services/wmts-foo", json=service_metadata_wmts_foo.prepare_for_json(), @@ -1634,7 +1597,7 @@ def test_remove_service_backend_response_is_an_error_status( } ) - resp = api100.delete("/services/wmts-foo") + resp = api100.delete("/services/b1-wmts-foo") assert resp.status_code == 500 # Verify the aggregator effectively asked the backend to remove the service, @@ -1642,94 +1605,68 @@ def test_remove_service_backend_response_is_an_error_status( assert mock_delete.called def test_update_service_service_succeeds( - self, api100, backend1, backend2, requests_mock, service_metadata_wmts_foo + self, api100, backend1, requests_mock, service_metadata_wmts_foo ): """When it receives an existing service ID and a correct payload, it updates the expected service.""" + api100.set_auth_bearer_token(TEST_USER_BEARER_TOKEN) - # Also test that it skips backends that don't have the service. - mock_get1 = requests_mock.get( - backend1 + "/services/wmts-foo", - status_code=404 - ) - mock_get2 = requests_mock.get( - backend2 + "/services/wmts-foo", - json=service_metadata_wmts_foo.prepare_for_json(), - status_code=200 - ) mock_patch = requests_mock.patch( - backend2 + "/services/wmts-foo", + backend1 + "/services/wmts-foo", json=service_metadata_wmts_foo.prepare_for_json(), status_code=204 ) process_graph = {"bar": {"process_id": "bar", "arguments": {"new_arg": "somevalue"}}} json_payload = {"process": {"process_graph": process_graph}} - resp = api100.patch("/services/wmts-foo", json=json_payload) + resp = api100.patch("/services/b1-wmts-foo", json=json_payload) assert resp.status_code == 204 # Make sure the aggregator asked the backend to update the service. assert mock_patch.called assert mock_patch.last_request.json() == json_payload - - # Check other mocks were called, to be sure it searched before updating. - assert mock_get1.called - assert mock_get2.called - def test_update_service_service_id_not_found( - self, api100, backend1, backend2, requests_mock, service_metadata_wmts_foo + + def test_update_service_but_backend_id_not_found( + self, api100 ): - """When the service ID does not exist then the aggregator raises an ServiceNotFoundException.""" + """When the service ID does not exist because the backend prefix is wrong, then the aggregator responds with HTTP 404, not found.""" + api100.set_auth_bearer_token(TEST_USER_BEARER_TOKEN) + process_graph = {"bar": {"process_id": "bar", "arguments": {"new_arg": "somevalue"}}} + json_payload = {"process": {"process_graph": process_graph}} - # Neither backend has the service available, and the aggregator should detect this. - mock_get1 = requests_mock.get( - backend1 + "/services/wmts-foo", + resp = api100.patch("/services/backenddoesnotexist-someservice", json=json_payload) + + assert resp.status_code == 404 + + def test_update_service_service_id_not_found( + self, api100, backend1, requests_mock, service_metadata_wmts_foo + ): + """When the service ID does not exist for the specified backend, then the aggregator responds with HTTP 404, not found.""" + + api100.set_auth_bearer_token(TEST_USER_BEARER_TOKEN) + mock_patch = requests_mock.patch( + backend1 + "/services/servicedoesnotexist", json=service_metadata_wmts_foo.prepare_for_json(), status_code=404 ) - mock_get2 = requests_mock.get( - backend2 + "/services/wmts-foo", - status_code=404, - ) - # The aggregator should not execute a HTTP patch, so we check that it does not call these two. - mock_patch1 = requests_mock.patch( - backend1 + "/services/wmts-foo", - json=service_metadata_wmts_foo.prepare_for_json(), - status_code=204 - ) - mock_patch2 = requests_mock.patch( - backend2 + "/services/wmts-foo", - json=service_metadata_wmts_foo.prepare_for_json(), - status_code=204 # deliberately avoid 404 so we know 404 comes from aggregator. - ) process_graph = {"bar": {"process_id": "bar", "arguments": {"new_arg": "somevalue"}}} json_payload = {"process": {"process_graph": process_graph}} - resp = api100.patch("/services/wmts-foo", json=json_payload) + resp = api100.patch("/services/b1-servicedoesnotexist", json=json_payload) assert resp.status_code == 404 - # Verify that the aggregator did not try to call patch on the backend. - assert not mock_patch1.called - assert not mock_patch2.called - # Verify that the aggregator asked the backend to remove the service. - assert mock_get1.called - assert mock_get2.called + assert mock_patch.called # TODO: for now, not bothering with HTTP 400 in the backend. To be decided if this is necessary. - # @pytest.mark.parametrize("backend_http_status", [400, 500]) @pytest.mark.parametrize("backend_http_status", [500]) def test_update_service_backend_response_is_an_error_status( - self, api100, backend1, backend2, requests_mock, service_metadata_wmts_foo, backend_http_status + self, api100, backend1, requests_mock, backend_http_status ): """When the backend response is an error HTTP 400/500 then the aggregator raises an OpenEoApiError.""" - api100.set_auth_bearer_token(TEST_USER_BEARER_TOKEN) - requests_mock.get( - backend1 + "/services/wmts-foo", - json=service_metadata_wmts_foo.prepare_for_json(), - status_code=200 - ) + api100.set_auth_bearer_token(TEST_USER_BEARER_TOKEN) mock_patch = requests_mock.patch( backend1 + "/services/wmts-foo", status_code=backend_http_status, @@ -1743,7 +1680,7 @@ def test_update_service_backend_response_is_an_error_status( process_graph = {"bar": {"process_id": "bar", "arguments": {"new_arg": "somevalue"}}} json_payload = {"process": {"process_graph": process_graph}} - resp = api100.patch("/services/wmts-foo", json=json_payload) + resp = api100.patch("/services/b1-wmts-foo", json=json_payload) assert resp.status_code == backend_http_status assert mock_patch.called