diff --git a/CHANGES.rst b/CHANGES.rst index ee9e77734..01dcf5360 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -38,6 +38,9 @@ Fixes: `common-workflow-language#764 `_ and `common-workflow-language#907 `_). - Fix typing definitions for certain ``Literal`` references for proper resolution involving values stored in constants. +- Fix ``get_sane_name`` checks performed on `Process` ID and `Service` name to use ``min_len=1`` in order to allow + valid `WPS` process definition on existing servers to resolve references that are shorter than the previous default + of 3 characters. .. _changes_4.30.1: diff --git a/tests/utils.py b/tests/utils.py index 542936647..64e84a0d3 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -83,8 +83,8 @@ # pylint: disable=C0103,invalid-name,E1101,no-member MockPatch = mock._patch # noqa: W0212 - # [WPS1-URL, GetCapPathXML, [DescribePathXML], [ExecutePathXML]] - MockConfigWPS1 = Sequence[str, str, Optional[Sequence[str]], Optional[Sequence[str]]] + # [WPS1-URL, GetCapPathXML, [DescribePathXML]] + MockConfigWPS1 = Sequence[str, str, Union[Sequence[str], Dict[str, str]]] MockReturnType = TypeVar("MockReturnType") MockHttpMethod = Union[ responses.HEAD, @@ -672,6 +672,16 @@ def test_function(): def test_function(): pass + Process description ID override Mock example: + + .. code-block:: python + + @mocked_remote_server_requests_wps1( + [ "", "", {"proc-1": "", "proc-2": "", ...} ] + ) + def test_function(): + pass + The generated responses mock can be obtained as follows to add further request definitions to simulate: .. code-block:: python @@ -695,12 +705,13 @@ def test_function(mock_responses): Single level or nested 2D list/tuples of 3 elements, where each one defines: 1. WPS server URL to be mocked to simulate response contents from requests for following items. 2. Single XML file path to the expected response body of a server ``GetCapabilities`` request. - 3. List of XML file paths to one or multiple expected response body of ``DescribeProcess`` requests. + 3. List of XML file paths or data to one or multiple expected response body of ``DescribeProcess`` requests, + or mapping of desired process ID to their corresponding ``DescribeProcess`` XML file path or string data. :param mock_responses: Handle to the generated mock instance by this decorator on the first wrapped call to add more configurations. In this case, wrapper function is not returned. :param data: - Flag indicating that provided strings are the literal data instead of file references. + Flag indicating that provided strings are directly the XML data instead of file references. All server configurations must be file OR data references, no mixing between them supported. :returns: wrapper that mocks multiple WPS-1 servers and their responses with provided processes and XML contents. """ @@ -717,20 +728,33 @@ def get_xml(ref): # type: (str) -> str for test_server_wps, resource_xml_getcap, resource_xml_describe in server_configs: assert isinstance(resource_xml_getcap, str) - assert isinstance(resource_xml_describe, (set, list, tuple)) + assert isinstance(resource_xml_describe, (set, list, tuple, dict)) if not data: assert os.path.isfile(resource_xml_getcap) - assert all(os.path.isfile(file) for file in resource_xml_describe) + if isinstance(resource_xml_describe, dict): + assert all(os.path.isfile(file) for file in resource_xml_describe.values()) + else: + assert all(os.path.isfile(file) for file in resource_xml_describe) + if isinstance(resource_xml_describe, dict): + map_describe = list(resource_xml_describe.items()) + else: + map_describe = [(None, desc) for desc in resource_xml_describe] get_cap_xml = get_xml(resource_xml_getcap) version_query = "&version=1.0.0" get_cap_url = f"{test_server_wps}?service=WPS&request=GetCapabilities" all_request.add((responses.GET, get_cap_url, get_cap_xml)) all_request.add((responses.GET, get_cap_url + version_query, get_cap_xml)) - for proc_desc_xml in resource_xml_describe: + for map_desc_id, proc_desc_xml in map_describe: describe_xml = get_xml(proc_desc_xml) # assume process ID is always the first identifier (ignore input/output IDs after) proc_desc_id = re.findall("(.*)", describe_xml)[0] + if map_desc_id is not None and map_desc_id != proc_desc_id: + describe_xml = describe_xml.replace( + f"{proc_desc_id}", + f"{map_desc_id}", + ) + proc_desc_id = map_desc_id proc_desc_url = f"{test_server_wps}?service=WPS&request=DescribeProcess&identifier={proc_desc_id}" all_request.add((responses.GET, proc_desc_url, describe_xml)) all_request.add((responses.GET, proc_desc_url + version_query, describe_xml)) diff --git a/tests/wps_restapi/test_processes.py b/tests/wps_restapi/test_processes.py index 90c37ddb8..b92c36138 100644 --- a/tests/wps_restapi/test_processes.py +++ b/tests/wps_restapi/test_processes.py @@ -544,7 +544,7 @@ def test_deploy_process_success(self): for pkg in package_mock: stack.enter_context(pkg) path = "/processes" - resp = self.app.post_json(path, params=process_data, headers=self.json_headers, expect_errors=True) + resp = self.app.post_json(path, params=process_data, headers=self.json_headers, expect_errors=False) assert resp.status_code == 201 assert resp.content_type == ContentType.APP_JSON assert resp.json["processSummary"]["id"] == process_name @@ -569,6 +569,30 @@ def test_deploy_process_ogc_schema(self): assert resp.json["processSummary"]["id"] == process_name assert isinstance(resp.json["deploymentDone"], bool) and resp.json["deploymentDone"] + def test_deploy_process_short_name(self): + process_name = "x" + process_data = self.get_process_deploy_template(process_name, schema=ProcessSchema.OGC) + process_data["processDescription"]["visibility"] = Visibility.PUBLIC + process_data["processDescription"]["outputs"] = {"output": {"schema": {"type": "string"}}} + package_mock = mocked_process_package() + + with contextlib.ExitStack() as stack: + for pkg in package_mock: + stack.enter_context(pkg) + path = "/processes" + resp = self.app.post_json(path, params=process_data, headers=self.json_headers, expect_errors=False) + assert resp.status_code == 201 + assert resp.content_type == ContentType.APP_JSON + assert resp.json["processSummary"]["id"] == process_name + assert isinstance(resp.json["deploymentDone"], bool) and resp.json["deploymentDone"] + + # perform get to make sure all name checks in the chain, going through db save/load, are validated + path = f"{path}/{process_name}" + query = {"schema": ProcessSchema.OLD} + resp = self.app.get(path, headers=self.json_headers, params=query, expect_errors=False) + assert resp.status_code == 200 + assert resp.json["process"]["id"] == process_name + def test_deploy_process_bad_name(self): process_name = f"{self.fully_qualified_test_process_name()}..." process_data = self.get_process_deploy_template(process_name) diff --git a/tests/wps_restapi/test_providers.py b/tests/wps_restapi/test_providers.py index b0746ac81..1385a0bfc 100644 --- a/tests/wps_restapi/test_providers.py +++ b/tests/wps_restapi/test_providers.py @@ -625,6 +625,35 @@ def test_get_provider_process_complex_maximum_megabytes(self): assert inputs[11]["formats"][0]["maximumMegabytes"] == 200 +class WpsShortNameProviderTest(WpsProviderBase): + remote_provider_name = "x" + settings = { + "weaver.url": "https://localhost", + "weaver.wps_path": "/ows/wps", + "weaver.configuration": WeaverConfiguration.HYBRID + } + + @mocked_remote_server_requests_wps1([ + resources.TEST_REMOTE_SERVER_URL, + resources.TEST_REMOTE_SERVER_WPS1_GETCAP_XML, + {"y": resources.TEST_REMOTE_SERVER_WPS1_DESCRIBE_PROCESS_XML}, + ]) + def test_get_provider_process_description_short_names(self): + self.register_provider() + + path = f"/providers/{self.remote_provider_name}" + resp = self.app.get(path, headers=self.json_headers) + assert resp.status_code == 200 + assert resp.content_type == ContentType.APP_JSON + assert resp.json["id"] == self.remote_provider_name + + path = f"/providers/{self.remote_provider_name}/processes/y" + resp = self.app.get(path, headers=self.json_headers) + assert resp.status_code == 200 + assert resp.content_type == ContentType.APP_JSON + assert resp.json["id"] == "y" + + class WpsProviderLocalOnlyTest(WpsProviderBase): """ Validate that operations are preemptively forbidden for a local-only instance. diff --git a/weaver/processes/utils.py b/weaver/processes/utils.py index c6dc9c0b4..7fd12ca0d 100644 --- a/weaver/processes/utils.py +++ b/weaver/processes/utils.py @@ -870,8 +870,8 @@ def parse_wps_process_config(config_entry): qs_p = parse_qs(url_p.query) svc_url = get_url_without_query(url_p) # if explicit name was provided, validate it (assert fail if not), - # otherwise replace silently bad character since since is requested to be inferred - svc_name = get_sane_name(svc_name or url_p.hostname, assert_invalid=bool(svc_name)) + # otherwise replace silently bad character since it is requested to be inferred + svc_name = get_sane_name(svc_name or url_p.hostname, assert_invalid=bool(svc_name), min_len=1) svc_proc = svc_proc or qs_p.get("identifier", []) # noqa # 'identifier=a,b,c' techically allowed svc_proc = [proc.strip() for proc in svc_proc if proc.strip()] # remote empty if not isinstance(svc_name, str): @@ -919,7 +919,7 @@ def register_wps_processes_static(service_url, service_name, service_visibility, return wps_processes = [wps.describeprocess(p) for p in service_processes] or wps.processes for wps_process in wps_processes: - proc_id = f"{service_name}_{get_sane_name(wps_process.identifier)}" + proc_id = f"{service_name}_{get_sane_name(wps_process.identifier, min_len=1)}" wps_pid = wps_process.identifier proc_url = f"{service_url}?service=WPS&request=DescribeProcess&identifier={wps_pid}&version={wps.version}" svc_vis = Visibility.PUBLIC if service_visibility else Visibility.PRIVATE diff --git a/weaver/processes/wps_package.py b/weaver/processes/wps_package.py index df009273f..c2426dd15 100644 --- a/weaver/processes/wps_package.py +++ b/weaver/processes/wps_package.py @@ -244,7 +244,7 @@ def get_process_location(process_id_or_url, data_source=None): if urlparse(process_id_or_url).scheme != "": return process_id_or_url data_source_url = retrieve_data_source_url(data_source) - process_id = get_sane_name(process_id_or_url) + process_id = get_sane_name(process_id_or_url, min_len=1) process_url = sd.process_service.path.format(process_id=process_id) return f"{data_source_url}{process_url}" @@ -975,7 +975,7 @@ def get_process_identifier(process_info, package): process_id = get_any_id(process_info) if not process_id: process_id = package.get("id") - process_id = get_sane_name(process_id, assert_invalid=True) + process_id = get_sane_name(process_id, assert_invalid=True, min_len=1) return process_id diff --git a/weaver/store/mongodb.py b/weaver/store/mongodb.py index 43a56e5a2..a206c6825 100644 --- a/weaver/store/mongodb.py +++ b/weaver/store/mongodb.py @@ -113,6 +113,7 @@ def __init__(self, collection, sane_name_config=None): raise TypeError("Collection not of expected type.") self.collection = collection # type: Collection self.sane_name_config = sane_name_config or {} + self.sane_name_config.setdefault("min_len", 1) @classmethod def get_args_kwargs(cls, *args, **kwargs):