Populate self for command input and fix optional secondary files (#3438)

* populate self for command input and fix optional secondary files

centaur/src/main/resources/standardTestCases/cwl_input_binding_expression.test

@@ -0,0 +1,16 @@
+name: cwl_input_binding_expression
+testFormat: workflowsuccess
+workflowType: CWL
+workflowTypeVersion: v1.0
+workflowRoot: cwl_input_binding_expression
+
+files {
+  wdl: cwl_input_binding_expression/cwl_input_binding_expression.cwl
+  inputs: cwl_input_binding_expression/cwl_input_binding_expression.json
+}
+
+metadata {
+  "submittedFiles.workflowType": CWL
+  "submittedFiles.workflowTypeVersion": v1.0
+  "outputs.cwl_input_binding_expression.b": "hello world"
+}

centaur/src/main/resources/standardTestCases/cwl_input_binding_expression/cwl_input_binding_expression.cwl

@@ -0,0 +1,25 @@
+cwlVersion: v1.0
+$graph:
+- id: cwl_input_binding_expression
+  class: CommandLineTool
+  requirements:
+    - class: InlineJavascriptRequirement
+  hints:
+    DockerRequirement:
+      dockerPull: "debian:stretch-slim"
+  inputs:
+    - id: hello
+      type: string
+      inputBinding:
+        valueFrom: $(self + " world")
+        position: 1
+  outputs:
+    b:
+      type: string
+      outputBinding:
+        loadContents: true
+        glob: stdout
+        outputEval: $(self[0].contents.trim())
+  stdout: stdout
+  baseCommand: []
+  arguments: [echo]

centaur/src/main/resources/standardTestCases/cwl_input_binding_expression/cwl_input_binding_expression.json

@@ -0,0 +1,3 @@
+{
+  "hello": "hello"
+}

centaur/src/main/resources/standardTestCases/cwl_secondary_files/cwl_secondary_files.cwl

@@ -13,13 +13,27 @@ $graph:
       inputBinding:
         position: 2
       secondaryFiles: [.also]
+    - id: of
+      type: File?
+      inputBinding:
+        position: 3
+      secondaryFiles: [.also]
     - id: fs
       type:
         type: array
         items: File
         inputBinding:
-          position: 3
+          position: 4
       secondaryFiles: [.also]
+    - id: fr
+      secondaryFiles: [.also]
+      type:
+        type: record
+        fields:
+          - name: a
+            type: File
+            inputBinding:
+              position: 5
   outputs:
     the_answer:
       type: string

centaur/src/main/resources/standardTestCases/cwl_secondary_files/cwl_secondary_files.yaml

@@ -3,8 +3,15 @@ command: stat $(echo $* | sed 's/.txt/.txt.also/g') > /dev/null
 f:
   class: File
   path: "centaur/src/main/resources/standardTestCases/cwl_secondary_files/foo.txt"
+of:
+  class: File
+  path: "centaur/src/main/resources/standardTestCases/cwl_secondary_files/bim.txt"
 fs:
   - class: File
     path: "centaur/src/main/resources/standardTestCases/cwl_secondary_files/bar.txt"
   - class: File
     path: "centaur/src/main/resources/standardTestCases/cwl_secondary_files/baz.txt"
+fr:
+  a:
+    class: File
+    path: "centaur/src/main/resources/standardTestCases/cwl_secondary_files/bam.txt"

cwl/src/main/scala/cwl/CwlExpressionCommandPart.scala

@@ -22,8 +22,8 @@ case class CwlExpressionCommandPart(expr: Expression)(hasShellCommandRequirement
       case (LocalName(localName), value) => localName -> valueMapper(value)
     }
     val parameterContext = ParameterContext(
-        inputs = stringKeyMap, runtimeOption = Option(runtimeEnvironment)
-      )
+      inputs = stringKeyMap, runtimeOption = Option(runtimeEnvironment)
+    )
     ExpressionEvaluator.eval(expr, parameterContext, expressionLib) map { womValue =>
       List(InstantiatedCommand(valueMapper(womValue).valueString.shellQuote))
     }
@@ -35,7 +35,7 @@ case class CwlExpressionCommandPart(expr: Expression)(hasShellCommandRequirement
   */
 abstract class CommandLineBindingCommandPart(commandLineBinding: CommandLineBinding)(hasShellCommandRequirement: Boolean, expressionLib: ExpressionLib) extends CommandPart {
 
-  
+
   private lazy val prefixAsString = commandLineBinding.prefix.getOrElse("")
   private lazy val prefixAsList = commandLineBinding.prefix.toList
   private lazy val separate = commandLineBinding.effectiveSeparate
@@ -44,32 +44,52 @@ abstract class CommandLineBindingCommandPart(commandLineBinding: CommandLineBind
     if (separate) prefixAsList :+ value else List(s"$prefixAsString$value")
   }
 
+  /**
+    * Value bound to this command part.
+    * InputCommandLineBindingCommandPart has one
+    * ArgumentCommandLineBindingCommandPart does not
+    */
+  def boundValue: Option[WomValue]
+
+  // If the bound value is defined but contains an empty optional value, we should not evaluate the valueFrom
+  // Conformance test "stage-unprovided-file" tests this behavior
+  private lazy val evaluateValueFrom = boundValue.forall {
+    case WomOptionalValue(_, None) => false
+    case _ => true
+  }
+
   override def instantiate(inputsMap: Map[LocalName, WomValue],
                            functions: IoFunctionSet,
                            valueMapper: (WomValue) => WomValue,
                            runtimeEnvironment: RuntimeEnvironment): ErrorOr[List[InstantiatedCommand]] = {
-      val stringInputsMap = inputsMap map {
-        case (LocalName(localName), value) => localName -> valueMapper(value)
-      }
-      val parameterContext = ParameterContext(inputs = stringInputsMap, runtimeOption = Option(runtimeEnvironment))
+    lazy val stringInputsMap = inputsMap map {
+      case (LocalName(localName), value) => localName -> valueMapper(value)
+    }
+
+    lazy val parameterContext = ParameterContext(
+      inputs = stringInputsMap,
+      runtimeOption = Option(runtimeEnvironment),
+      self = boundValue.getOrElse(ParameterContext.EmptySelf)
+    )
 
-    val evaluatedValueFrom = commandLineBinding.optionalValueFrom map {
-      case StringOrExpression.Expression(expression) => ExpressionEvaluator.eval(expression, parameterContext, expressionLib) map valueMapper
-      case StringOrExpression.String(string) => WomString(string).validNel
+    val evaluatedValueFrom = commandLineBinding.optionalValueFrom flatMap {
+      case StringOrExpression.Expression(expression) if evaluateValueFrom => Option(ExpressionEvaluator.eval(expression, parameterContext, expressionLib) map valueMapper)
+      case StringOrExpression.String(string) if evaluateValueFrom => Option(WomString(string).validNel)
+      case _ => None
     }
 
     val evaluatedWomValue: Checked[WomValue] = evaluatedValueFrom.orElse(boundValue.map(_.validNel)) match {
       case Some(womValue) => womValue.map(valueMapper).toEither
       case None => "Command line binding has no valueFrom field and no bound value".invalidNelCheck
     }
-    
+
     def applyShellQuote(value: String): String = commandLineBinding.shellQuote match {
       // Only honor shellQuote = false if ShellCommandRequirement is enabled.
       // Conformance test "Test that shell directives are not interpreted."
       case Some(false) if hasShellCommandRequirement => value
       case _ => value.shellQuote
     }
-    
+
     def processValue(womValue: WomValue): List[String] = womValue match {
       case WomOptionalValue(_, Some(value)) => processValue(valueMapper(value))
       case WomOptionalValue(_, None) => List.empty
@@ -105,8 +125,6 @@ abstract class CommandLineBindingCommandPart(commandLineBinding: CommandLineBind
 
     evaluatedWomValue map { v => processValue(v) map applyShellQuote map (InstantiatedCommand(_)) } toValidated
   }
-
-  def boundValue: Option[WomValue]
 }
 
 case class InputCommandLineBindingCommandPart(commandLineBinding: InputCommandLineBinding, associatedValue: WomValue)

cwl/src/main/scala/cwl/InputParameter.scala

@@ -1,5 +1,6 @@
 package cwl
 
+import cats.syntax.either._
 import cats.syntax.traverse._
 import cats.syntax.validated._
 import common.validation.ErrorOr._
@@ -8,7 +9,7 @@ import shapeless.Poly1
 import wom.callable.Callable.InputDefinition.InputValueMapper
 import wom.expression.IoFunctionSet
 import wom.types.{WomSingleFileType, WomType}
-import wom.values.{WomArray, WomMaybePopulatedFile, WomValue}
+import wom.values.{WomArray, WomMaybePopulatedFile, WomObject, WomObjectLike, WomOptionalValue, WomValue}
 
 trait InputParameter {
   def id: String
@@ -122,6 +123,19 @@ object InputParameter {
             } yield updated
 
           case WomArray(_, values) => values.toList.traverse(populateFiles).map(WomArray(_))
+          case WomOptionalValue(_, Some(innerValue)) => populateFiles(innerValue).map(WomOptionalValue(_))
+          case obj: WomObjectLike =>
+            // Map the values
+            obj.values.toList.traverse[ErrorOr, (String, WomValue)]({
+              case (key, value) => populateFiles(value).map(key -> _)
+            })
+              .map(_.toMap)
+              // transform to Either so we can flatMap
+              .toEither
+              // Validate new types are still valid w.r.t the object
+              .flatMap(WomObject.withTypeChecked(_, obj.womObjectTypeLike))
+              // re-transform to ErrorOr
+              .toValidated
           case womValue: WomValue =>
             womValue.valid
         }

cwl/src/main/scala/cwl/ParameterContext.scala

@@ -4,6 +4,10 @@ import wom.callable.RuntimeEnvironment
 import wom.types.WomNothingType
 import wom.values.{WomOptionalValue, WomValue}
 
+object ParameterContext {
+  val EmptySelf = WomOptionalValue(WomNothingType, None)
+}
+
 case class ParameterContext(inputs: Map[String, WomValue] = Map.empty,
-                            self: WomValue = WomOptionalValue(WomNothingType, None),
+                            self: WomValue = ParameterContext.EmptySelf,
                             runtimeOption: Option[RuntimeEnvironment] = None)

src/bin/travis/resources/local_conformance_expected_failures.txt

@@ -114,6 +114,6 @@
 114
 #115
 #116
-117
+#117
 #118
 119

wom/src/main/scala/wom/values/WomObject.scala

@@ -1,5 +1,8 @@
 package wom.values
 
+import cats.data.{NonEmptyList, Validated}
+import common.Checked
+import common.validation.ErrorOr.ErrorOr
 import wom.TsvSerializable
 import wom.types._
 import wom.util.FileUtil
@@ -75,7 +78,15 @@ object WomObject {
 
   def withType(values: Map[String, Any], objectTypeLike: WomObjectTypeLike) = {
     import common.validation.Validation._
-    objectTypeLike.validateAndCoerceValues(values).map(new WomObject(_, objectTypeLike)).toTry.get
+    withTypeErrorOr(values, objectTypeLike).toTry.get
+  }
+
+  def withTypeErrorOr(values: Map[String, Any], objectTypeLike: WomObjectTypeLike): ErrorOr[WomObject] = {
+    objectTypeLike.validateAndCoerceValues(values).map(new WomObject(_, objectTypeLike))
+  }
+
+  def withTypeChecked(values: Map[String, Any], objectTypeLike: WomObjectTypeLike): Checked[WomObject] = {
+    withTypeErrorOr(values, objectTypeLike).toEither
   }
 
 }