Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove default queryables form value #1473

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions eodag/plugins/search/build_search_result.py
Original file line number Diff line number Diff line change
Expand Up @@ -339,9 +339,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,
Expand Down Expand Up @@ -580,7 +577,7 @@ def discover_queryables(
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
Expand Down Expand Up @@ -654,7 +651,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")

Expand Down Expand Up @@ -872,9 +871,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())

Expand Down Expand Up @@ -961,6 +957,16 @@ def format_as_provider_keyword(
}
return format_query_params(product_type, self.config, available_properties)

@functools.lru_cache()
def fetch_data(self, url: str) -> Any:
"""
Fetches from a provider elements like constraints or forms by using the cache.

:param url: url from which the constraints can be fetched
:returns: cache containing the json file content fetched from the provider
"""
return self._fetch_data(url)

def _fetch_data(self, url: str) -> Any:
"""
fetches from a provider elements like constraints or forms.
Expand Down
16 changes: 14 additions & 2 deletions tests/resources/form.json
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
]
119 changes: 97 additions & 22 deletions tests/units/test_search_plugins.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
HTTP_REQ_TIMEOUT,
Expand Down Expand Up @@ -2342,8 +2342,10 @@ 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)
Expand All @@ -2352,7 +2354,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)

Expand All @@ -2374,32 +2376,47 @@ 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=5,
),
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=5,
),
call().raise_for_status(),
call().json(),
]
)

Expand All @@ -2419,7 +2436,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())
Expand All @@ -2441,24 +2466,74 @@ 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(
"https://ads.atmosphere.copernicus.eu/api/catalogue/v1/collections/"
"cams-europe-air-quality-reanalyses/constraints.json",
),
call(
"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
Expand Down
Loading