Skip to content

Commit

Permalink
CWL schema definition updates and fixes for Workflow steps and requir…
Browse files Browse the repository at this point in the history
…ements strict validation
  • Loading branch information
fmigneault committed May 14, 2024
1 parent d4eadd2 commit 3f008d0
Show file tree
Hide file tree
Showing 7 changed files with 176 additions and 53 deletions.
21 changes: 16 additions & 5 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,22 @@ Changes

Changes:
--------
- No change.

Fixes:
------
- No change.
- Add `CWL` ``MultipleInputFeatureRequirement`` support.
- Add `CWL` ``SubworkflowFeatureRequirement`` support.
- Add `CWL` ``Workflow`` explicit schema validation of its ``steps``.
- Remove "unknown" definitions in `CWL` ``requirements``. Only fully defined and resolved definitions will be allowed.
If an unsupported `CWL` requirement by `Weaver` must be provided (but is a valid definition supported by ``cwltool``),
it must now be provided through ``hints`` to succeed schema validation.

Fixes:
------
- Fix invalid `CWL` schema definition for ``ScatterFeatureRequirement`` that directly
contained the corresponding fields ``scatter`` and ``scatterMethod``, instead of the expected
definition within a `Workflow Step <https://www.commonwl.org/v1.2/Workflow.html#WorkflowStep>`_.
- Fix `CWL` ``requirements`` schema definition using ``OneOf`` and the ``discriminator`` property that could sometime
drop a definition when it only contained an empty mapping ``{}``, and that the corresponding requirement allows it.
- Fix ``weaver.wps_restapi.colander_extras.AnyOfKeywordSchema`` not allowing distinct `JSON` structure ``type`` to be
combined simultaneously.

.. _changes_5.3.0:

Expand Down
40 changes: 39 additions & 1 deletion tests/wps_restapi/test_colander_extras.py
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,24 @@ class OneOfTwoMap(ce.OneOfKeywordSchema):
])


def test_oneof_discriminator():
class Cat(ce.PermissiveMappingSchema):
animal_type = ce.ExtendedSchemaNode(colander.String(), validator=colander.OneOf(["cat"]))

class Dog(ce.PermissiveMappingSchema):
animal_type = ce.ExtendedSchemaNode(colander.String(), validator=colander.OneOf(["dog"]))

class Animal(ce.OneOfKeywordSchema):
discriminator = "animal_type"
_one_of = [Cat(), Dog()]

schema = Animal()
schema.deserialize({"animal_type": "cat"})
schema.deserialize({"animal_type": "dog"})
with pytest.raises(colander.Invalid):
schema.deserialize({"animal_type": "bird"})


def test_oneof_optional_default_with_nested_required():
"""
Using ``oneOf`` keyword that is optional with default, its required subnodes must resolve to the provided default.
Expand Down Expand Up @@ -1086,7 +1104,7 @@ class VarMap(ce.ExtendedMappingSchema):
VarMap().deserialize({"random": "abc"})


def test_variable_not_additional_properties():
def test_variable_no_additional_properties():
class VarMap(ce.StrictMappingSchema):
var_1 = ce.ExtendedSchemaNode(colander.String(), variable="<var-1>")

Expand All @@ -1098,6 +1116,26 @@ class VarMap(ce.StrictMappingSchema):
with pytest.raises(colander.Invalid, match=err):
VarMap().deserialize({"random": "abc", "other": 1})

VarMap().deserialize({"random": "abc"})


def test_mapping_no_additional_properties():
class Map(ce.StrictMappingSchema):
field = ce.ExtendedSchemaNode(colander.String())

class Derived(Map):
other = ce.ExtendedSchemaNode(colander.String())

with pytest.raises(colander.Invalid):
Map().deserialize({"random": "abc"})
with pytest.raises(colander.Invalid):
Map().deserialize({"field": "abc", "other": "bad"})
Map().deserialize({"field": "abc"})

with pytest.raises(colander.Invalid):
Derived().deserialize({"field": "abc", "random": "bad"})
Derived().deserialize({"field": "abc", "other": "bad"})


def test_media_type_pattern():
test_schema = sd.MediaType
Expand Down
29 changes: 29 additions & 0 deletions tests/wps_restapi/test_swagger_definitions.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,20 @@
import glob
import os
import uuid
from typing import TYPE_CHECKING

import colander
import pytest

from weaver.formats import ContentType
from weaver.utils import load_file
from weaver.wps_restapi import swagger_definitions as sd

if TYPE_CHECKING:
from weaver.typedefs import CWL

TEST_DIR = os.path.dirname(os.path.dirname(__file__))


def test_process_id_with_version_tag_deploy_invalid():
"""
Expand Down Expand Up @@ -169,3 +178,23 @@ def test_execute_input_inline_object_invalid(test_data, expect_result):
else:
result = schema.deserialize(test_data)
assert result == expect_result


@pytest.mark.parametrize(
"cwl_path",
glob.glob(
"**/*.cwl",
root_dir=TEST_DIR,
recursive=True,
)
)
def test_cwl_package(cwl_path):
# type: (str) -> None
"""
Test that our :term:`CWL` schema definition works with the many examples used for testsing.
"""
cwl_path = os.path.join(TEST_DIR, cwl_path)
cwl = load_file(cwl_path) # type: CWL
cwl_check = sd.CWL().deserialize(cwl)
cwl_check.pop("$schema", None) # our definition injects this reference
assert cwl_check == cwl
2 changes: 1 addition & 1 deletion weaver/processes/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,8 @@ class OpenSearchField(Constants):
CWL_REQUIREMENT_PROCESS_GENERATOR = get_args(CWL_RequirementProcessGeneratorType)[0]
CWL_REQUIREMENT_RESOURCE = get_args(CWL_RequirementResourceType)[0]
CWL_REQUIREMENT_SCATTER = get_args(CWL_RequirementScatterFeatureType)[0]
CWL_REQUIREMENT_STEP_INPUT_EXPRESSION = get_args(CWL_RequirementStepInputExpressionType)[0]
CWL_REQUIREMENT_SECRETS = get_args(CWL_RequirementSecretsType)[0]
CWL_REQUIREMENT_STEP_INPUT_EXPRESSION = get_args(CWL_RequirementStepInputExpressionType)[0]
CWL_REQUIREMENT_SUBWORKFLOW = get_args(CWL_RequirementSubworkflowFeatureType)[0]
CWL_REQUIREMENT_TIME_LIMIT = get_args(CWL_RequirementToolTimeLimitType)[0]
# default is to reuse, employed to explicitly disable
Expand Down
47 changes: 34 additions & 13 deletions weaver/wps_restapi/colander_extras.py
Original file line number Diff line number Diff line change
Expand Up @@ -2012,22 +2012,43 @@ def __init__(self, *args, **kwargs):
if isinstance(discriminator_spec, dict) and "mapping" not in discriminator_spec:
mapping = {}
for node in self.get_keyword_items():
node_fields = [getattr(field, "example", colander.null) for field in node.children
if field.name == discriminator_spec["propertyName"]]
node_fields = [
field for field in node.children
if field.name == discriminator_spec["propertyName"]
]
if len(node_fields) != 1:
continue
example = node_fields[0]
example = getattr(node_fields[0], "example", colander.null)
values = getattr(node_fields[0], "validator", StringOneOf([colander.null]))
discriminator_value = colander.null
if example is not colander.null:
if example in mapping:
raise SchemaNodeTypeError(
"Keyword schema '{}' of type '{}' specification with 'discriminator' attempts "
"to refer to duplicate example values '{}' between '{}' and '{}'".format(
schema_name, keyword, example,
_get_node_name(mapping[example], schema_name=True),
_get_node_name(node, schema_name=True),
)
discriminator_value = example
elif (
isinstance(values, colander.OneOf)
and len(values.choices) == 1
and isinstance(values.choices[0], str)
):
discriminator_value = values.choices[0]
if not discriminator_value:
raise SchemaNodeTypeError(
"Keyword schema '{}' of type '{}' specification with 'discriminator' "
"could not resolve any value defining the discriminator for nested "
"field '{}' in schema '{}'.".format(
schema_name, keyword,
discriminator_spec["propertyName"],
_get_node_name(node_fields[0], schema_name=True),
)
mapping[example] = node
)
if discriminator_value in mapping:
raise SchemaNodeTypeError(
"Keyword schema '{}' of type '{}' specification with 'discriminator' attempts "
"to refer to duplicate example values '{}' between '{}' and '{}'".format(
schema_name, keyword, discriminator_value,
_get_node_name(mapping[discriminator_value], schema_name=True),
_get_node_name(node, schema_name=True),
)
)
mapping[discriminator_value] = node
discriminator_spec["mapping"] = mapping
if not (
isinstance(discriminator_spec, dict)
Expand Down Expand Up @@ -2259,7 +2280,7 @@ class AnyRequired(AnyKeywordSchema):
- :class:`NotKeywordSchema`
"""
_keyword_schemas_only_object = False
_keyword_schemas_same_struct = True
_keyword_schemas_same_struct = False
_keyword = "_any_of"

@classmethod
Expand Down

0 comments on commit 3f008d0

Please sign in to comment.