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

Substitute parse_cwl_array_type with get_cwl_io_type in processes.wps_package.WpsPackage.make_inputs #546

Merged
merged 4 commits into from
Sep 14, 2023
Merged
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
6 changes: 6 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ Fixes:
------
- Fix missing Node.js requirement in built Docker image in order to evaluate definitions that employ
`CWL` ``InlineJavascriptRequirement``, such as ``valueFrom`` employed for numeric ``Enum`` input type validation.
- Fix ``processes.wps_package.WpsPackage.make_inputs`` unable to parse multi-type `CWL` definitions due parsing
as single-type element with ``parse_cwl_array_type``. Function ``get_cwl_io_type`` is used instead to resolve any
`CWL` type combination properly.
- Fix ``get_cwl_io_type`` function that would modify the I/O definition passed as argument, which could lead to failing
`CWL` ``class`` reference resolutions later on due to different ``type`` with ``org.w3id.cwl.cwl`` prefix simplified
before ``cwltool`` had the chance to resolve them.

.. _changes_4.31.0:

Expand Down
72 changes: 55 additions & 17 deletions tests/processes/test_convert.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
"""
Unit tests of functions within :mod:`weaver.processes.convert`.
"""
import copy

# pylint: disable=R1729 # ignore non-generator representation employed for displaying test log results

import json
Expand Down Expand Up @@ -708,7 +710,7 @@ def test_cwl2wps_io_record_format():


@pytest.mark.parametrize(
"io_type, io_info",
["io_type", "io_info"],
[
(WPS_LITERAL, {"type": WPS_LITERAL}),
(WPS_COMPLEX, {"type": WPS_COMPLEX}),
Expand All @@ -727,28 +729,64 @@ def test_get_io_type_category(io_type, io_info):
assert get_io_type_category(io_info) == io_type, f"Testing: {io_info}"


@pytest.mark.parametrize("io_info, io_def", [
({"type": "string"},
CWLIODefinition(type="string")),
({"type": "int"},
CWLIODefinition(type="int")),
({"type": "float"},
CWLIODefinition(type="float")),
({"type": {"type": "enum", "symbols": ["a", "b", "c"]}},
CWLIODefinition(type="string", enum=True, symbols=["a", "b", "c"], mode=MODE.SIMPLE)),
({"type": {"type": "array", "items": "string"}},
CWLIODefinition(type="string", array=True, min_occurs=1, max_occurs=PACKAGE_ARRAY_MAX_SIZE)),
({"type": ["null", "string"]},
CWLIODefinition(type="string", null=True, min_occurs=0)),
({"type": "string?"},
CWLIODefinition(type="string", null=True, min_occurs=0)),
])
@pytest.mark.parametrize(
["io_info", "io_def"],
[
({"type": "string"},
CWLIODefinition(type="string")),
({"type": "int"},
CWLIODefinition(type="int")),
({"type": "float"},
CWLIODefinition(type="float")),
({"type": {"type": "enum", "symbols": ["a", "b", "c"]}},
CWLIODefinition(type="string", enum=True, symbols=["a", "b", "c"], mode=MODE.SIMPLE)),
({"type": {"type": "array", "items": "string"}},
CWLIODefinition(type="string", array=True, min_occurs=1, max_occurs=PACKAGE_ARRAY_MAX_SIZE)),
({"type": ["null", "string"]},
CWLIODefinition(type="string", null=True, min_occurs=0)),
({"type": "string?"},
CWLIODefinition(type="string", null=True, min_occurs=0)),
]
)
def test_get_cwl_io_type(io_info, io_def):
io_def.name = io_info["name"] = "test"
io_res = get_cwl_io_type(io_info)
assert io_res == io_def


@pytest.mark.parametrize(
["io_info", "io_def"],
[
(
{
"name": "test",
"type": "org.w3id.cwl.cwl.File",
"format": "https://www.iana.org/assignments/media-types/application/json",
"location": "/tmp/random.json",
},
CWLIODefinition(name="test", type="File")
)
]
)
def test_get_cwl_io_type_unmodified(io_info, io_def):
"""
Ensure that the input I/O details do not cause a side effect modification of the data when parsing the definition.

When :func:`get_cwl_io_type` was called with a definition containing ``type: org.w3id.cwl.cwl.File``, the resulting
parsing caused the input information to be overriden by ``type: File``. Although they are essentially equivalent
once resolved, this modification performed before :mod:`cwltool` had the time to parse the definition made it
incorrectly resolve ``class: File``, which in turn, caused :class:`cwltool.pathmapper.PathMapper` to be missing
the mapped ``location`` of provided inputs, leading to full :term:`CWL` execution failure.

.. seealso::
- https://github.com/crim-ca/weaver/pull/546
"""
io_copy = copy.deepcopy(io_info)
io_res = get_cwl_io_type(io_info)
assert io_res == io_def
assert io_info == io_copy, "Argument I/O information should not be modified from parsing."


def test_parse_cwl_array_type_explicit_invalid_item():
io_info = {
"name": "test",
Expand Down
1 change: 1 addition & 0 deletions weaver/processes/convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -1337,6 +1337,7 @@ def get_cwl_io_type(io_info, strict=True):
LOGGER.debug("I/O parsed for multiple base types")

# parse single-definition
io_info = io_info.copy()
io_info["type"] = io_type # override resolved multi-type base for more parsing
io_name = io_info["name"]
io_min_occurs = 0 if is_null else 1
Expand Down
4 changes: 2 additions & 2 deletions weaver/processes/wps_package.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,12 @@
from weaver.processes.convert import (
DEFAULT_FORMAT,
cwl2wps_io,
get_cwl_io_type,
json2wps_field,
json2wps_io,
merge_package_io,
normalize_ordered_io,
ogcapi2cwl_process,
parse_cwl_array_type,
wps2json_io,
xml_wps2cwl
)
Expand Down Expand Up @@ -1859,7 +1859,7 @@ def make_inputs(self,
# process single occurrences
input_i = input_occurs[0]
# handle as reference/data
io_def = parse_cwl_array_type(cwl_inputs_info[input_id])
io_def = get_cwl_io_type(cwl_inputs_info[input_id])
if isinstance(input_i, ComplexInput) or io_def.type in PACKAGE_COMPLEX_TYPES:
# extend array data that allow max_occur > 1
# drop invalid inputs returned as None
Expand Down
Loading