Skip to content

Commit

Permalink
fix get_cwl_io_type to avoid modification of type CWL class reference
Browse files Browse the repository at this point in the history
  • Loading branch information
fmigneault committed Sep 14, 2023
1 parent 21d03b6 commit c3ab558
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 17 deletions.
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

0 comments on commit c3ab558

Please sign in to comment.