From aded97fb235a71fa46174aa58a9d34f3369bd8d5 Mon Sep 17 00:00:00 2001 From: Ruben Vorderman Date: Wed, 1 Jul 2020 15:48:28 +0200 Subject: [PATCH] Allow nested inputs when corresponding meta setting is set to true This implements part of the spec change that handles inputs https://github.com/openwdl/wdl/pull/359 --- ...nitionElementToWomWorkflowDefinition.scala | 28 +++++++++++++------ .../graph/CallElementToGraphNode.scala | 3 +- .../graph/IfElementToGraphNode.scala | 2 ++ .../graph/ScatterElementToGraphNode.scala | 3 ++ .../WorkflowGraphElementToGraphNode.scala | 7 +++-- .../import_me.wdl | 28 +++++++++++++++++++ ..._subwf_sub_inputs_meta_enabled.inputs.json | 4 +++ ...optional_subwf_sub_inputs_meta_enabled.wdl | 14 ++++++++++ 8 files changed, 76 insertions(+), 13 deletions(-) create mode 100644 womtool/src/test/resources/validate/wdl_draft3/valid/supplied_optional_subwf_sub_inputs_meta_enabled/import_me.wdl create mode 100644 womtool/src/test/resources/validate/wdl_draft3/valid/supplied_optional_subwf_sub_inputs_meta_enabled/supplied_optional_subwf_sub_inputs_meta_enabled.inputs.json create mode 100644 womtool/src/test/resources/validate/wdl_draft3/valid/supplied_optional_subwf_sub_inputs_meta_enabled/supplied_optional_subwf_sub_inputs_meta_enabled.wdl diff --git a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/WorkflowDefinitionElementToWomWorkflowDefinition.scala b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/WorkflowDefinitionElementToWomWorkflowDefinition.scala index f41a17c5883..b1f04d902d8 100644 --- a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/WorkflowDefinitionElementToWomWorkflowDefinition.scala +++ b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/WorkflowDefinitionElementToWomWorkflowDefinition.scala @@ -4,19 +4,20 @@ import cats.syntax.validated._ import common.validation.ErrorOr.{ErrorOr, _} import wdl.model.draft3.elements.ExpressionElement.{ArrayLiteral, IdentifierLookup, SelectFirst} import wdl.model.draft3.elements._ +import wdl.model.draft3.graph.ExpressionValueConsumer.ops._ +import wdl.model.draft3.graph.expression.{FileEvaluator, TypeEvaluator, ValueEvaluator} import wdl.model.draft3.graph.{ExpressionValueConsumer, GeneratedCallFinishedHandle, GeneratedValueHandle, LinkedGraph} import wdl.shared.transforms.wdlom2wom.WomGraphMakerTools import wdl.transforms.base.linking.graph.LinkedGraphMaker +import wdl.transforms.base.wdlom2wom.graph.renaming.GraphIdentifierLookupRenamer.ops._ +import wdl.transforms.base.wdlom2wom.graph.renaming._ import wdl.transforms.base.wdlom2wom.graph.{GraphNodeMakerInputs, WorkflowGraphElementToGraphNode} +import wom.callable.MetaValueElement.MetaValueElementBoolean import wom.callable.{Callable, WorkflowDefinition} -import wom.graph.expression.AnonymousExpressionNode import wom.graph.GraphNodePort.OutputPort +import wom.graph.expression.AnonymousExpressionNode import wom.graph.{CallNode, GraphNode, WomIdentifier, Graph => WomGraph} import wom.types.WomType -import wdl.model.draft3.graph.ExpressionValueConsumer.ops._ -import wdl.model.draft3.graph.expression.{FileEvaluator, TypeEvaluator, ValueEvaluator} -import wdl.transforms.base.wdlom2wom.graph.renaming.GraphIdentifierLookupRenamer.ops._ -import wdl.transforms.base.wdlom2wom.graph.renaming._ object WorkflowDefinitionElementToWomWorkflowDefinition extends Util { @@ -33,6 +34,14 @@ object WorkflowDefinitionElementToWomWorkflowDefinition extends Util { val a: WorkflowDefinitionConvertInputs = eliminateInputDependencies(b) + val (meta, parameterMeta) = processMetaSections(a.definitionElement.metaSection, a.definitionElement.parameterMetaSection) + + val allowNestedInputs: Boolean = { + meta.get("allowNestedInputs").flatMap { + case x: MetaValueElementBoolean => Option(x.value) + case _ => None + }}.getOrElse(false) + // Make the set of workflow graph elements, including: // - Top-level graph elements // - Declarations in the inputs section @@ -45,6 +54,7 @@ object WorkflowDefinitionElementToWomWorkflowDefinition extends Util { val innerGraph: ErrorOr[WomGraph] = convertGraphElements(GraphLikeConvertInputs(graphNodeElements, Set.empty, Map.empty, a.typeAliases, a.definitionElement.name, insideAScatter = false, convertNestedScatterToSubworkflow = b.convertNestedScatterToSubworkflow, + allowNestedInputs = allowNestedInputs, a.callables)) // NB: isEmpty means "not isDefined". We specifically do NOT add defaults if the output section is defined but empty. val withDefaultOutputs: ErrorOr[WomGraph] = if (a.definitionElement.outputsSection.isEmpty) { @@ -53,8 +63,6 @@ object WorkflowDefinitionElementToWomWorkflowDefinition extends Util { innerGraph } - val (meta, parameterMeta) = processMetaSections(a.definitionElement.metaSection, a.definitionElement.parameterMetaSection) - (withDefaultOutputs map { ig => WorkflowDefinition(a.definitionElement.name, ig, meta, parameterMeta, b.definitionElement.sourceLocation) }).contextualizeErrors(s"process workflow definition '${a.definitionElement.name}'") @@ -67,6 +75,7 @@ object WorkflowDefinitionElementToWomWorkflowDefinition extends Util { workflowName: String, insideAScatter: Boolean, convertNestedScatterToSubworkflow: Boolean, + allowNestedInputs: Boolean, callables: Map[String, Callable]) def convertGraphElements(a: GraphLikeConvertInputs) @@ -84,7 +93,7 @@ object WorkflowDefinitionElementToWomWorkflowDefinition extends Util { for { linkedGraph <- LinkedGraphMaker.make(nodes = a.graphElements, seedGeneratedValueHandles ++ finished, typeAliases = a.typeAliases, callables = a.callables) - womGraph <- makeWomGraph(linkedGraph, a.seedNodes, a.externalUpstreamCalls, a.workflowName, a.insideAScatter, a.convertNestedScatterToSubworkflow, a.callables) + womGraph <- makeWomGraph(linkedGraph, a.seedNodes, a.externalUpstreamCalls, a.workflowName, a.insideAScatter, a.convertNestedScatterToSubworkflow, a.allowNestedInputs, a.callables) } yield womGraph } @@ -94,6 +103,7 @@ object WorkflowDefinitionElementToWomWorkflowDefinition extends Util { workflowName: String, insideAScatter: Boolean, convertNestedScatterToSubworkflow : Boolean, + allowNestedInputs: Boolean, callables: Map[String, Callable]) (implicit expressionValueConsumer: ExpressionValueConsumer[ExpressionElement], fileEvaluator: FileEvaluator[ExpressionElement], @@ -119,7 +129,7 @@ object WorkflowDefinitionElementToWomWorkflowDefinition extends Util { val generatedGraphNodesValidation: ErrorOr[Set[GraphNode]] = WorkflowGraphElementToGraphNode.convert( - GraphNodeMakerInputs(next, upstreamCallNodes, linkedGraph.consumedValueLookup, availableValues, linkedGraph.typeAliases, workflowName, insideAScatter, convertNestedScatterToSubworkflow, callables)) + GraphNodeMakerInputs(next, upstreamCallNodes, linkedGraph.consumedValueLookup, availableValues, linkedGraph.typeAliases, workflowName, insideAScatter, convertNestedScatterToSubworkflow, allowNestedInputs, callables)) generatedGraphNodesValidation map { nextGraphNodes: Set[GraphNode] => currentList ++ nextGraphNodes } } } diff --git a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/CallElementToGraphNode.scala b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/CallElementToGraphNode.scala index d3e456f7362..f38d4b6e68f 100644 --- a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/CallElementToGraphNode.scala +++ b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/CallElementToGraphNode.scala @@ -54,7 +54,7 @@ object CallElementToGraphNode { def supplyableInput(definition: Callable.InputDefinition): Boolean = { !definition.isInstanceOf[FixedInputDefinitionWithDefault] && - !definition.name.contains(".") // NB: Remove this check when sub-workflows allow pass-through task inputs + (!definition.name.contains(".") || a.allowNestedInputs) } def validInput(name: String, definition: Callable.InputDefinition): Boolean = { @@ -224,4 +224,5 @@ case class CallNodeMakerInputs(node: CallElement, availableTypeAliases: Map[String, WomType], workflowName: String, insideAnotherScatter: Boolean, + allowNestedInputs: Boolean, callables: Map[String, Callable]) diff --git a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/IfElementToGraphNode.scala b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/IfElementToGraphNode.scala index 230dec2642d..53742b6be80 100644 --- a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/IfElementToGraphNode.scala +++ b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/IfElementToGraphNode.scala @@ -76,6 +76,7 @@ object IfElementToGraphNode { val graphLikeConvertInputs = GraphLikeConvertInputs(graphElements.toSet, ogins, foundOuterGenerators.completionPorts, a.availableTypeAliases, a.workflowName, insideAScatter = a.insideAnotherScatter, convertNestedScatterToSubworkflow = a.convertNestedScatterToSubworkflow, + allowNestedInputs = a.allowNestedInputs, a.callables) val innerGraph: ErrorOr[Graph] = WorkflowDefinitionElementToWomWorkflowDefinition.convertGraphElements(graphLikeConvertInputs) @@ -96,4 +97,5 @@ final case class ConditionalNodeMakerInputs(node: IfElement, workflowName: String, insideAnotherScatter: Boolean, convertNestedScatterToSubworkflow : Boolean, + allowNestedInputs: Boolean, callables: Map[String, Callable]) diff --git a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/ScatterElementToGraphNode.scala b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/ScatterElementToGraphNode.scala index 2886b83c02f..621461559e5 100644 --- a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/ScatterElementToGraphNode.scala +++ b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/ScatterElementToGraphNode.scala @@ -103,6 +103,7 @@ object ScatterElementToGraphNode { val graphLikeConvertInputs = GraphLikeConvertInputs(graphElements.toSet, ogins ++ Set(womInnerGraphScatterVariableInput), foundOuterGenerators.completionPorts, a.availableTypeAliases, a.workflowName, insideAScatter = true, convertNestedScatterToSubworkflow = a.convertNestedScatterToSubworkflow, + allowNestedInputs = a.allowNestedInputs, a.callables) val innerGraph: ErrorOr[Graph] = WorkflowDefinitionElementToWomWorkflowDefinition.convertGraphElements(graphLikeConvertInputs) @@ -131,6 +132,7 @@ object ScatterElementToGraphNode { val graphLikeConvertInputs = GraphLikeConvertInputs(Set(a.node), subWorkflowInputs, Map.empty, a.availableTypeAliases, a.workflowName, insideAScatter = false, convertNestedScatterToSubworkflow = a.convertNestedScatterToSubworkflow, + allowNestedInputs = a.allowNestedInputs, a.callables) val subWorkflowGraph = WorkflowDefinitionElementToWomWorkflowDefinition.convertGraphElements(graphLikeConvertInputs) subWorkflowGraph map { WomGraphMakerTools.addDefaultOutputs(_) } @@ -188,4 +190,5 @@ final case class ScatterNodeMakerInputs(node: ScatterElement, workflowName: String, insideAnotherScatter: Boolean, convertNestedScatterToSubworkflow: Boolean, + allowNestedInputs: Boolean, callables: Map[String, Callable]) diff --git a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/WorkflowGraphElementToGraphNode.scala b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/WorkflowGraphElementToGraphNode.scala index 7c6ea313264..d72fd299dba 100644 --- a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/WorkflowGraphElementToGraphNode.scala +++ b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/graph/WorkflowGraphElementToGraphNode.scala @@ -50,15 +50,15 @@ object WorkflowGraphElementToGraphNode { result.contextualizeErrors(s"process declaration '${typeElement.toWdlV1} $name = ${expr.toWdlV1}'") case se: ScatterElement => - val scatterMakerInputs = ScatterNodeMakerInputs(se, a.upstreamCalls, a.linkableValues, a.linkablePorts, a.availableTypeAliases, a.workflowName, a.insideAScatter, a.convertNestedScatterToSubworkflow, a.callables) + val scatterMakerInputs = ScatterNodeMakerInputs(se, a.upstreamCalls, a.linkableValues, a.linkablePorts, a.availableTypeAliases, a.workflowName, a.insideAScatter, a.convertNestedScatterToSubworkflow, a.allowNestedInputs, a.callables) ScatterElementToGraphNode.convert(scatterMakerInputs) case ie: IfElement => - val ifMakerInputs = ConditionalNodeMakerInputs(ie, a.upstreamCalls, a.linkableValues, a.linkablePorts, a.availableTypeAliases, a.workflowName, a.insideAScatter, a.convertNestedScatterToSubworkflow, a.callables) + val ifMakerInputs = ConditionalNodeMakerInputs(ie, a.upstreamCalls, a.linkableValues, a.linkablePorts, a.availableTypeAliases, a.workflowName, a.insideAScatter, a.convertNestedScatterToSubworkflow, a.allowNestedInputs, a.callables) IfElementToGraphNode.convert(ifMakerInputs) case ce: CallElement => - val callNodeMakerInputs = CallNodeMakerInputs(ce, a.upstreamCalls, a.linkableValues, a.linkablePorts, a.availableTypeAliases, a.workflowName, a.insideAScatter, a.callables) + val callNodeMakerInputs = CallNodeMakerInputs(ce, a.upstreamCalls, a.linkableValues, a.linkablePorts, a.availableTypeAliases, a.workflowName, a.insideAScatter, a.allowNestedInputs, a.callables) CallElementToGraphNode.convert(callNodeMakerInputs) } @@ -81,4 +81,5 @@ final case class GraphNodeMakerInputs(node: WorkflowGraphElement, workflowName: String, insideAScatter: Boolean, convertNestedScatterToSubworkflow : Boolean, + allowNestedInputs: Boolean, callables: Map[String, Callable]) diff --git a/womtool/src/test/resources/validate/wdl_draft3/valid/supplied_optional_subwf_sub_inputs_meta_enabled/import_me.wdl b/womtool/src/test/resources/validate/wdl_draft3/valid/supplied_optional_subwf_sub_inputs_meta_enabled/import_me.wdl new file mode 100644 index 00000000000..894f838f5c1 --- /dev/null +++ b/womtool/src/test/resources/validate/wdl_draft3/valid/supplied_optional_subwf_sub_inputs_meta_enabled/import_me.wdl @@ -0,0 +1,28 @@ +version 1.0 + +workflow sub_wf { + input { + Int y + } + # Calls foo but doesn't provide an 'x'. That's fine because the input was optional, but now the outer WF cannot override it. + call foo { input: y = y } + + # No outputs, but we don't want that to be the error: + output { } + meta {allowNestedInputs: true} +} + +task foo { + input { + Int? x + Int y + } + command { + } + output { + Int z = y + } + runtime { + docker: "ubuntu:latest" + } +} diff --git a/womtool/src/test/resources/validate/wdl_draft3/valid/supplied_optional_subwf_sub_inputs_meta_enabled/supplied_optional_subwf_sub_inputs_meta_enabled.inputs.json b/womtool/src/test/resources/validate/wdl_draft3/valid/supplied_optional_subwf_sub_inputs_meta_enabled/supplied_optional_subwf_sub_inputs_meta_enabled.inputs.json new file mode 100644 index 00000000000..b77431f60f1 --- /dev/null +++ b/womtool/src/test/resources/validate/wdl_draft3/valid/supplied_optional_subwf_sub_inputs_meta_enabled/supplied_optional_subwf_sub_inputs_meta_enabled.inputs.json @@ -0,0 +1,4 @@ +{ + "supplied_optional_subwf_sub_inputs.y": 5, + "supplied_optional_subwf_sub_inputs.sub_wf.foo.x": 5 +} diff --git a/womtool/src/test/resources/validate/wdl_draft3/valid/supplied_optional_subwf_sub_inputs_meta_enabled/supplied_optional_subwf_sub_inputs_meta_enabled.wdl b/womtool/src/test/resources/validate/wdl_draft3/valid/supplied_optional_subwf_sub_inputs_meta_enabled/supplied_optional_subwf_sub_inputs_meta_enabled.wdl new file mode 100644 index 00000000000..d1d180dc887 --- /dev/null +++ b/womtool/src/test/resources/validate/wdl_draft3/valid/supplied_optional_subwf_sub_inputs_meta_enabled/supplied_optional_subwf_sub_inputs_meta_enabled.wdl @@ -0,0 +1,14 @@ +version 1.0 + +import "import_me.wdl" + +workflow supplied_optional_subwf_sub_inputs { + input { + Int y + } + # Shouldn't (strictly) be able to call this sub-workflow because the inputs are not passed through + call import_me.sub_wf {input: y=y} + meta { + allowNestedInputs: true + } +}