Skip to content

Commit

Permalink
Merge pull request #546 from crim-ca/fix-make-inputs
Browse files Browse the repository at this point in the history
  • Loading branch information
fmigneault authored Sep 14, 2023
2 parents de9fba2 + c3ab558 commit 34debdc
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 19 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
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

0 comments on commit 34debdc

Please sign in to comment.