From 45c1aa778974a915660cbed3233529e854edb94e Mon Sep 17 00:00:00 2001 From: anesson-cs <113022843+anesson-cs@users.noreply.github.com> Date: Mon, 3 Feb 2025 14:56:03 +0100 Subject: [PATCH] fix(plugins): remove default queryables form value (#1473) --- eodag/plugins/search/build_search_result.py | 18 ++- tests/resources/form.json | 16 ++- tests/units/test_search_plugins.py | 121 ++++++++++++++++---- 3 files changed, 122 insertions(+), 33 deletions(-) diff --git a/eodag/plugins/search/build_search_result.py b/eodag/plugins/search/build_search_result.py index ec368849f..16444bd2d 100644 --- a/eodag/plugins/search/build_search_result.py +++ b/eodag/plugins/search/build_search_result.py @@ -341,9 +341,6 @@ class ECMWFSearch(PostJsonSearch): """ def __init__(self, provider: str, config: PluginConfig) -> None: - # cache fetching method - self.fetch_data = functools.lru_cache()(self._fetch_data) - config.metadata_mapping = { **keywords_to_mdt(ECMWF_KEYWORDS + COP_DS_KEYWORDS, "ecmwf"), **config.metadata_mapping, @@ -576,13 +573,13 @@ def discover_queryables( getattr(self.config, "discover_queryables", {}).get("constraints_url", ""), **kwargs, ) - constraints: list[dict[str, Any]] = self.fetch_data(constraints_url) + constraints: list[dict[str, Any]] = self._fetch_data(constraints_url) form_url = format_metadata( getattr(self.config, "discover_queryables", {}).get("form_url", ""), **kwargs, ) - form = self.fetch_data(form_url) + form: list[dict[str, Any]] = self._fetch_data(form_url) formated_kwargs = self.format_as_provider_keyword( product_type, processed_kwargs @@ -639,7 +636,7 @@ def discover_queryables( return self.queryables_from_metadata_mapping(product_type) if "{" in values_url: values_url = values_url.format(productType=provider_product_type) - data = self.fetch_data(values_url) + data = self._fetch_data(values_url) available_values = data["constraints"] required_keywords = data.get("required", []) @@ -656,7 +653,9 @@ def discover_queryables( "completionTimeFromAscendingNode", "geom", } - and keyword.replace("ecmwf:", "") not in available_values + and keyword not in [f["name"] for f in form] + and keyword.replace("ecmwf:", "") + not in set(list(available_values.keys()) + [f["name"] for f in form]) ): raise ValidationError(f"{keyword} is not a queryable parameter") @@ -874,9 +873,6 @@ def queryables_by_form( if fields and (comment := fields[0].get("comment")): prop["description"] = comment - if d := details.get("default"): - default = default or (d[0] if fields else d) - if name == "area" and isinstance(default, dict): default = list(default.values()) @@ -979,7 +975,7 @@ def _fetch_data(self, url: str) -> Any: else None ) timeout = getattr(self.config, "timeout", DEFAULT_SEARCH_TIMEOUT) - return fetch_json(url, auth=auth, timeout=timeout) + return functools.lru_cache()(fetch_json)(url, auth=auth, timeout=timeout) def normalize_results( self, results: RawSearchResult, **kwargs: Any diff --git a/tests/resources/form.json b/tests/resources/form.json index b18d35fe1..738a35545 100644 --- a/tests/resources/form.json +++ b/tests/resources/form.json @@ -73,8 +73,20 @@ "name": "data_format", "type": "StringChoiceWidget", "details": { - "values": ["grib", "netcdf"] + "values": ["grib", "netcdf"], + "default": ["grib"] }, - "id": 8 + "id": 8, + "required": true + }, + { + "name": "download_format", + "type": "StringChoiceWidget", + "details": { + "values": ["unarchived", "zip"], + "default": ["unarchived"] + }, + "id": 9, + "required": true } ] diff --git a/tests/units/test_search_plugins.py b/tests/units/test_search_plugins.py index b447acb0c..e05e2f051 100644 --- a/tests/units/test_search_plugins.py +++ b/tests/units/test_search_plugins.py @@ -40,7 +40,7 @@ from eodag.api.product import AssetsDict from eodag.api.product.metadata_mapping import get_queryable_from_provider from eodag.utils import deepcopy -from eodag.utils.exceptions import UnsupportedProductType +from eodag.utils.exceptions import UnsupportedProductType, ValidationError from tests.context import ( DEFAULT_MISSION_START_DATE, DEFAULT_SEARCH_TIMEOUT, @@ -2343,8 +2343,11 @@ def test_plugins_search_ecmwfsearch_with_custom_producttype(self): except Exception: assert eoproduct.properties[param] == self.custom_query_params[param] - @mock.patch("eodag.utils.requests.requests.sessions.Session.get", autospec=True) - def test_plugins_search_ecmwfsearch_discover_queryables(self, mock_requests_get): + @mock.patch( + "eodag.plugins.search.build_search_result.ECMWFSearch._fetch_data", + autospec=True, + ) + def test_plugins_search_ecmwfsearch_discover_queryables_ok(self, mock__fetch_data): constraints_path = os.path.join(TEST_RESOURCES_PATH, "constraints.json") with open(constraints_path) as f: constraints = json.load(f) @@ -2353,7 +2356,7 @@ def test_plugins_search_ecmwfsearch_discover_queryables(self, mock_requests_get) form_path = os.path.join(TEST_RESOURCES_PATH, "form.json") with open(form_path) as f: form = json.load(f) - mock_requests_get.return_value.json.side_effect = [constraints, form] + mock__fetch_data.side_effect = [constraints, form] product_type_config = {"missionStartDate": "2001-01-01T00:00:00Z"} setattr(self.search_plugin.config, "product_type_config", product_type_config) @@ -2375,32 +2378,49 @@ def test_plugins_search_ecmwfsearch_discover_queryables(self, mock_requests_get) default_values.pop("metadata_mapping", None) params = deepcopy(default_values) params["productType"] = "CAMS_EU_AIR_QUALITY_RE" + # set a parameter among the required ones of the form file with a default value in this form but not among the + # ones of the constraints file to an empty value to check if its associated queryable has no default value + eodag_formatted_data_format = "ecmwf:data_format" + provider_data_format = eodag_formatted_data_format.replace("ecmwf:", "") + self.assertIn(eodag_formatted_data_format, default_values) + self.assertIn(provider_data_format, [param["name"] for param in form]) + data_format_in_form = [ + param for param in form if param["name"] == provider_data_format + ][0] + self.assertTrue(data_format_in_form.get("required", False)) + self.assertIsNotNone( + data_format_in_form.get("details", {}).get("default", None) + ) + for constraint in constraints: + self.assertNotIn(provider_data_format, constraint) + params[eodag_formatted_data_format] = "" + + # use a parameter among the ones of the form file but not among the ones of the constraints file + # and of provider default configuration to check if an error is raised, which is supposed to not happen + eodag_formatted_download_format = "ecmwf:download_format" + provider_download_format = eodag_formatted_download_format.replace("ecmwf:", "") + self.assertNotIn(eodag_formatted_download_format, default_values) + self.assertIn(provider_download_format, [param["name"] for param in form]) + for constraint in constraints: + self.assertNotIn(provider_data_format, constraint) + params[eodag_formatted_download_format] = "foo" queryables = self.search_plugin.discover_queryables(**params) + # no error was raised, as expected self.assertIsNotNone(queryables) - mock_requests_get.assert_has_calls( + mock__fetch_data.assert_has_calls( [ call( mock.ANY, "https://ads.atmosphere.copernicus.eu/api/catalogue/v1/collections/" "cams-europe-air-quality-reanalyses/constraints.json", - headers=USER_AGENT, - auth=None, - timeout=DEFAULT_SEARCH_TIMEOUT, ), - call().raise_for_status(), - call().json(), call( mock.ANY, "https://ads.atmosphere.copernicus.eu/api/catalogue/v1/collections/" "cams-europe-air-quality-reanalyses/form.json", - headers=USER_AGENT, - auth=None, - timeout=DEFAULT_SEARCH_TIMEOUT, ), - call().raise_for_status(), - call().json(), ] ) @@ -2420,7 +2440,15 @@ def test_plugins_search_ecmwfsearch_discover_queryables(self, mock_requests_get) "CAMS_EU_AIR_QUALITY_RE" ].items(): queryable = queryables.get(property) - if queryable is not None: + # a special case for eodag_formatted_data_format queryable is required + # as its default value has been overwritten by an empty value + if queryable is not None and property == eodag_formatted_data_format: + self.assertEqual( + PydanticUndefined, queryable.__metadata__[0].get_default() + ) + # queryables with empty default values are required + self.assertTrue(queryable.__metadata__[0].is_required()) + elif queryable is not None: self.assertEqual(default_value, queryable.__metadata__[0].get_default()) # queryables with default values are not required self.assertFalse(queryable.__metadata__[0].is_required()) @@ -2442,7 +2470,8 @@ def test_plugins_search_ecmwfsearch_discover_queryables(self, mock_requests_get) ) # reset mock - mock_requests_get.reset_mock() + mock__fetch_data.reset_mock() + mock__fetch_data.side_effect = [constraints, form] # with additional param params = deepcopy(default_values) params["productType"] = "CAMS_EU_AIR_QUALITY_RE" @@ -2450,16 +2479,68 @@ def test_plugins_search_ecmwfsearch_discover_queryables(self, mock_requests_get) queryables = self.search_plugin.discover_queryables(**params) self.assertIsNotNone(queryables) - # mock not called because cached values are used - mock_requests_get.assert_not_called() + # cached values are not used to make the set of unit tests work then the mock is called again + mock__fetch_data.assert_has_calls( + [ + call( + mock.ANY, + "https://ads.atmosphere.copernicus.eu/api/catalogue/v1/collections/" + "cams-europe-air-quality-reanalyses/constraints.json", + ), + call( + mock.ANY, + "https://ads.atmosphere.copernicus.eu/api/catalogue/v1/collections/" + "cams-europe-air-quality-reanalyses/form.json", + ), + ] + ) - self.assertEqual(11, len(queryables)) + self.assertEqual(12, len(queryables)) # default properties called in function arguments are added and must be default values of the queryables queryable = queryables.get("ecmwf:variable") if queryable is not None: self.assertEqual("a", queryable.__metadata__[0].get_default()) self.assertFalse(queryable.__metadata__[0].is_required()) + @mock.patch( + "eodag.plugins.search.build_search_result.ECMWFSearch._fetch_data", + autospec=True, + ) + def test_plugins_search_ecmwfsearch_discover_queryables_ko(self, mock__fetch_data): + constraints_path = os.path.join(TEST_RESOURCES_PATH, "constraints.json") + with open(constraints_path) as f: + constraints = json.load(f) + form_path = os.path.join(TEST_RESOURCES_PATH, "form.json") + with open(form_path) as f: + form = json.load(f) + mock__fetch_data.side_effect = [constraints, form] + + default_values = deepcopy( + getattr(self.search_plugin.config, "products", {}).get( + "CAMS_EU_AIR_QUALITY_RE", {} + ) + ) + default_values.pop("metadata_mapping", None) + params = deepcopy(default_values) + params["productType"] = "CAMS_EU_AIR_QUALITY_RE" + + # use a wrong parameter, e.g. it is not among the ones of the form file, not among + # the ones of the constraints file and not among the ones of default provider configuration + wrong_queryable = "foo" + self.assertNotIn(wrong_queryable, default_values) + self.assertNotIn(wrong_queryable, [param["name"] for param in form]) + for constraint in constraints: + self.assertNotIn(wrong_queryable, constraint) + params[wrong_queryable] = "bar" + + # Test the function, expecting ValidationError to be raised + with self.assertRaises(ValidationError) as context: + self.search_plugin.discover_queryables(**params) + self.assertEqual( + f"{wrong_queryable} is not a queryable parameter for {self.provider}", + str(context.exception), + ) + @mock.patch("eodag.utils.requests.requests.sessions.Session.get", autospec=True) def test_plugins_search_ecmwf_search_wekeo_discover_queryables( self, mock_requests_get