fix(plugins): remove default queryables form value (#1473)

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

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
   }
 ]

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,24 +2470,77 @@ 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"
         params["ecmwf:variable"] = "a"
         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