Complex input parsing (#3127)

* Non primitive types input parsing
broadinstitute · Jan 12, 2018 · d6b391c · d6b391c
1 parent 796ad7f
commit d6b391c
Show file tree

Hide file tree

Showing 8 changed files with 161 additions and 129 deletions.
diff --git a/cwl/src/main/scala-2.11/cwl/CwlExecutableValidation.scala b/cwl/src/main/scala-2.11/cwl/CwlExecutableValidation.scala
@@ -27,9 +27,9 @@ object CwlExecutableValidation {
   // Decodes the input file, and build the ParsedInputMap
   private val inputCoercionFunction: InputParsingFunction =
     inputFile => {
-      yaml.parser.parse(inputFile).flatMap(_.as[Map[String, MyriadInputValue]]) match {
+      yaml.parser.parse(inputFile).flatMap(_.as[Map[String, Json]]) match {
         case Left(error) => error.getMessage.invalidNelCheck[ParsedInputMap]
-        case Right(inputValue) => inputValue.map({ case (key, value) => key -> value.fold(CwlInputCoercion) }).validNelCheck
+        case Right(inputValue) => inputValue.map({ case (key, value) => key -> value.foldWith(CwlInputCoercion) }).validNelCheck
       }
     }
 

diff --git a/cwl/src/main/scala-2.12/cwl/CwlExecutableValidation.scala b/cwl/src/main/scala-2.12/cwl/CwlExecutableValidation.scala
@@ -6,7 +6,7 @@ import common.validation.ErrorOr.ErrorOr
 import io.circe.generic.auto._
 import io.circe.literal._
 import io.circe.shapes._
-import io.circe.{Decoder, yaml}
+import io.circe.{Decoder, Json, yaml}
 import wom.callable.{CallableTaskDefinition, ExecutableCallable, ExecutableTaskDefinition}
 import wom.executable.Executable
 import wom.executable.Executable.{InputParsingFunction, ParsedInputMap}
@@ -22,9 +22,9 @@ object CwlExecutableValidation {
   // Decodes the input file, and build the ParsedInputMap
   private val inputCoercionFunction: InputParsingFunction =
     inputFile => {
-      yaml.parser.parse(inputFile).flatMap(_.as[Map[String, MyriadInputValue]]) match {
+      yaml.parser.parse(inputFile).flatMap(_.as[Map[String, Json]]) match {
         case Left(error) => error.getMessage.invalidNelCheck[ParsedInputMap]
-        case Right(inputValue) => inputValue.map({ case (key, value) => key -> value.fold(CwlInputCoercion) }).validNelCheck
+        case Right(inputValue) => inputValue.map({ case (key, value) => key -> value.foldWith(CwlInputCoercion) }).validNelCheck
       }
     }
 

diff --git a/cwl/src/main/scala/cwl/CwlInputCoercion.scala b/cwl/src/main/scala/cwl/CwlInputCoercion.scala
@@ -0,0 +1,67 @@
+package cwl
+
+import cats.instances.list._
+import cats.syntax.traverse._
+import cats.syntax.validated._
+import cats.syntax.option._
+import common.validation.ErrorOr.ErrorOr
+import common.validation.Validation._
+import io.circe.{Json, JsonNumber, JsonObject}
+import io.circe._
+import io.circe.generic.auto._
+import io.circe.refined._
+import io.circe.literal._
+import wom.executable.Executable.DelayedCoercionFunction
+import wom.types._
+import wom.values._
+
+// With recursive types we could let circe parse it for us, but until we figure that out just parse it as Json and
+// manually check for File / Directory structures
+private [cwl] object CwlInputCoercion extends Json.Folder[DelayedCoercionFunction] {
+  import cwl.decoder._
+  implicit val fileD = implicitly[Decoder[File]]
+  implicit val directoryD = implicitly[Decoder[Directory]]
+
+  private def simpleCoercion(value: Any)(womType: WomType) = womType.coerceRawValue(value).toErrorOr
+
+  override def onNull = womType => { WomOptionalValue.none(womType).validNel }
+  override def onBoolean(value: Boolean) = simpleCoercion(value)
+  override def onNumber(value: JsonNumber) = {
+    case WomFloatType => WomFloat(value.toDouble).validNel
+    case WomIntegerType => value.toInt.map(WomInteger.apply).toValidNel(s"$value is not a valid Int")
+    case other => other.coerceRawValue(value.toString).toErrorOr
+  }
+  override def onString(value: String) = simpleCoercion(value)
+
+  override def onArray(value: Vector[Json]) = {
+    case womArrayType: WomArrayType =>
+      value.toList
+        .traverse[ErrorOr, WomValue](_.foldWith(this).apply(womArrayType.memberType))
+        .map {
+          WomArray(womArrayType, _)
+        }
+
+    case other => s"Cannot convert an array input value into a non array type: $other".invalidNel
+  }
+
+  override def onObject(value: JsonObject) = {
+    // CWL files are represented as Json objects, so this could be a file 
+    case WomSingleFileType | WomMaybePopulatedFileType if value.toMap.get("class").flatMap(_.asString).contains("File") =>
+      Json.fromJsonObject(value).as[File] match {
+        case Left(errors) => errors.message.invalidNel
+        case Right(file) => file.asWomValue
+      }
+    case WomMaybeListedDirectoryType | WomUnlistedDirectoryType if value.toMap.get("class").flatMap(_.asString).contains("Directory") =>
+      Json.fromJsonObject(value).as[Directory] match {
+        case Left(errors) => errors.message.invalidNel
+        case Right(directory) => directory.asWomValue
+      }
+    case WomObjectType =>
+      val foldedMap = value.toList.traverse[ErrorOr, (String, WomValue)]({
+        case (k, v) => v.foldWith(this).apply(WomAnyType).map(k -> _)
+      }).map(_.toMap[String, WomValue])
+
+      foldedMap.map(WomObject.apply)
+    case other => s"Cannot convert an array input value into a non array type: $other".invalidNel
+  }
+}
diff --git a/cwl/src/main/scala/cwl/CwlInputParsingPoly.scala b/cwl/src/main/scala/cwl/CwlInputParsingPoly.scala
diff --git a/cwl/src/main/scala/cwl/CwlType.scala b/cwl/src/main/scala/cwl/CwlType.scala
@@ -4,7 +4,7 @@ import cats.instances.list._
 import cats.instances.option._
 import cats.syntax.traverse._
 import cats.syntax.validated._
-import common.validation.ErrorOr._
+import common.validation.ErrorOr.{ErrorOr, _}
 import common.validation.Validation._
 import eu.timepit.refined._
 import shapeless.Poly1

diff --git a/cwl/src/main/scala/cwl/TypeAliases.scala b/cwl/src/main/scala/cwl/TypeAliases.scala
@@ -41,23 +41,6 @@ trait TypeAliases {
       CwlAny :+:
       CNil
 
-  // TODO WOM: Record Schema as well as Directories are not included because they're not supported yet, although they should eventually be.
-  // Removing them saves some compile time when building decoders for this type (in CwlInputParsing)
-  type MyriadInputValuePrimitives =
-    String :+:
-      Int :+:
-      Long :+:
-      FileOrDirectory :+:
-      Float :+:
-      Double :+:
-      Boolean :+:
-      CNil
-
-  type MyriadInputValue =
-    MyriadInputValuePrimitives :+:
-      Array[MyriadInputValuePrimitives] :+:
-      CNil
-
   type MyriadInputType =
     MyriadInputInnerType :+:
       Array[MyriadInputInnerType] :+:

diff --git a/cwl/src/test/scala/cwl/CwlInputValidationSpec.scala b/cwl/src/test/scala/cwl/CwlInputValidationSpec.scala
@@ -8,6 +8,7 @@ import shapeless.Coproduct
 import wom.expression.WomExpression
 import wom.graph.Graph.ResolvedExecutableInput
 import wom.graph.GraphNodePort
+import wom.types.{WomArrayType, WomStringType}
 import wom.values._
 
 class CwlInputValidationSpec extends FlatSpec with Matchers with TableDrivenPropertyChecks with BeforeAndAfterAll {
@@ -33,6 +34,24 @@ class CwlInputValidationSpec extends FlatSpec with Matchers with TableDrivenProp
         | w5: double
         | w6: float
         | w7: boolean
+        | w8:
+        |   type:
+        |     type: array
+        |     items:
+        |       type: array
+        |       items: string
+        | # enable this when InputRecordSchemas are enabled      
+        | #w9:
+        | #  type:
+        | #    name: w9
+        | #    type: record
+        | #    fields:
+        | #    - name: w9a
+        | #      type: record
+        | #      fields: 
+        | #      - name: w9aa
+        | #        type: string  
+        | w10: Directory
         |steps: []    
         |outputs: []
       """.stripMargin
@@ -53,14 +72,19 @@ class CwlInputValidationSpec extends FlatSpec with Matchers with TableDrivenProp
     case Right(womDef) => womDef.graph
   }
 
-  lazy val w0OutputPort = graph.inputNodes.find(_.localName == "w0").getOrElse(fail("Failed to find an input node for w0")).singleOutputPort
-  lazy val w1OutputPort = graph.inputNodes.find(_.localName == "w1").getOrElse(fail("Failed to find an input node for w1")).singleOutputPort
-  lazy val w2OutputPort = graph.inputNodes.find(_.localName == "w2").getOrElse(fail("Failed to find an input node for w2")).singleOutputPort
-  lazy val w3OutputPort = graph.inputNodes.find(_.localName == "w3").getOrElse(fail("Failed to find an input node for w3")).singleOutputPort
-  lazy val w4OutputPort = graph.inputNodes.find(_.localName == "w4").getOrElse(fail("Failed to find an input node for w4")).singleOutputPort
-  lazy val w5OutputPort = graph.inputNodes.find(_.localName == "w5").getOrElse(fail("Failed to find an input node for w5")).singleOutputPort
-  lazy val w6OutputPort = graph.inputNodes.find(_.localName == "w6").getOrElse(fail("Failed to find an input node for w6")).singleOutputPort
-  lazy val w7OutputPort = graph.inputNodes.find(_.localName == "w7").getOrElse(fail("Failed to find an input node for w7")).singleOutputPort
+  def getOutputPort(n: Int) = graph.inputNodes.find(_.localName == s"w$n").getOrElse(fail(s"Failed to find an input node for w$n")).singleOutputPort
+
+  lazy val w0OutputPort = getOutputPort(0)
+  lazy val w1OutputPort = getOutputPort(1)
+  lazy val w2OutputPort = getOutputPort(2)
+  lazy val w3OutputPort = getOutputPort(3)
+  lazy val w4OutputPort = getOutputPort(4)
+  lazy val w5OutputPort = getOutputPort(5)
+  lazy val w6OutputPort = getOutputPort(6)
+  lazy val w7OutputPort = getOutputPort(7)
+  lazy val w8OutputPort = getOutputPort(8)
+//  lazy val w9OutputPort = getOutputPort(9)
+  lazy val w10OutputPort = getOutputPort(10)
 
   def validate(inputFile: String): Map[GraphNodePort.OutputPort, ResolvedExecutableInput] = {
     cwlWorkflow.womExecutable(AcceptAllRequirements, Option(inputFile)) match {
@@ -75,12 +99,30 @@ class CwlInputValidationSpec extends FlatSpec with Matchers with TableDrivenProp
         w1:
           class: File
           path: my_file.txt
+          secondaryFiles:
+            - class: File
+              path: secondaryFile.txt
         w2: hello !
         w3: 3
         w4: 4
         w5: 5.1
         w6: 6.1
         w7: true
+        w8: [
+          ["a", "b"],
+          ["c", "d"]
+        ]
+        w9: 
+          w9a:
+            w9aa: "hello"
+        w10:
+          class: Directory
+          location: "directory_location"
+          listing:
+            - class: Directory
+              path: "innerDirectory"
+            - class: File
+              location: "innerFile"
       """.stripMargin
 
     val validInputs = validate(inputFile).map {
@@ -89,15 +131,45 @@ class CwlInputValidationSpec extends FlatSpec with Matchers with TableDrivenProp
 
     // w0 has no input value in the input file, so it should fallback to the default value
     // TODO WOM: when we have string value for wom expression, check that it's "hi !"
-    validInputs(w0OutputPort.name).select[WomExpression].isDefined shouldBe true
+    validInputs(w0OutputPort.name).select[WomExpression].get.sourceString shouldBe "\"hi w0 !\""
     validInputs(w1OutputPort.name) shouldBe
-      Coproduct[ResolvedExecutableInput](WomMaybePopulatedFile("my_file.txt"): WomValue)
+      Coproduct[ResolvedExecutableInput](WomMaybePopulatedFile(
+        valueOption = Option("my_file.txt"),
+        secondaryFiles = List(WomMaybePopulatedFile("secondaryFile.txt"))
+      ): WomValue)
     validInputs(w2OutputPort.name) shouldBe Coproduct[ResolvedExecutableInput](WomString("hello !"): WomValue)
     validInputs(w3OutputPort.name) shouldBe Coproduct[ResolvedExecutableInput](WomInteger(3): WomValue)
     validInputs(w4OutputPort.name) shouldBe Coproduct[ResolvedExecutableInput](WomInteger(4): WomValue)
-    validInputs(w5OutputPort.name) shouldBe Coproduct[ResolvedExecutableInput](WomFloat(5.1F): WomValue)
-    validInputs(w6OutputPort.name) shouldBe Coproduct[ResolvedExecutableInput](WomFloat(6.1F): WomValue)
+    validInputs(w5OutputPort.name) shouldBe Coproduct[ResolvedExecutableInput](WomFloat(5.1D): WomValue)
+    validInputs(w6OutputPort.name) shouldBe Coproduct[ResolvedExecutableInput](WomFloat(6.1D): WomValue)
     validInputs(w7OutputPort.name) shouldBe Coproduct[ResolvedExecutableInput](WomBoolean(true): WomValue)
+    validInputs(w8OutputPort.name) shouldBe Coproduct[ResolvedExecutableInput](
+      WomArray(
+        WomArrayType(WomArrayType(WomStringType)),
+        List(
+          WomArray(WomArrayType(WomStringType), List(WomString("a"), WomString("b"))),
+          WomArray(WomArrayType(WomStringType), List(WomString("c"), WomString("d")))
+        )
+      ): WomValue
+    )
+    // Enable this when InputRecordSchema is enabled
+//    validInputs(w9OutputPort.name) shouldBe Coproduct[ResolvedExecutableInput](
+//      WomObject(
+//        Map(
+//          "w9a" -> WomObject(Map("w9aa" -> WomString("hello")))
+//        )
+//      ): WomValue
+//    )
+    validInputs(w10OutputPort.name) shouldBe
+      Coproduct[ResolvedExecutableInput](WomMaybeListedDirectory(
+        valueOption = Option("directory_location"),
+        listingOption = Option(
+          List(
+            WomMaybeListedDirectory("innerDirectory"),
+            WomMaybePopulatedFile("innerFile")
+          )
+        )
+      ): WomValue)
   }
 
   it should "not validate when required inputs are missing" in {
@@ -114,7 +186,9 @@ class CwlInputValidationSpec extends FlatSpec with Matchers with TableDrivenProp
         "Required workflow input 'w4' not specified",
         "Required workflow input 'w5' not specified",
         "Required workflow input 'w6' not specified",
-        "Required workflow input 'w7' not specified"
+        "Required workflow input 'w7' not specified",
+        "Required workflow input 'w8' not specified",
+        "Required workflow input 'w10' not specified"
       )
     }
   }

diff --git a/wom/src/main/scala/wom/expression/WomExpression.scala b/wom/src/main/scala/wom/expression/WomExpression.scala
@@ -21,7 +21,7 @@ trait WomExpression {
   * It looks and acts like an expression, but it's really just a value.
   */
 final case class ValueAsAnExpression(value: WomValue) extends WomExpression {
-  override def sourceString: String = value.toWomString
+  override def sourceString: String = value.valueString
   override def evaluateValue(inputValues: Map[String, WomValue], ioFunctionSet: IoFunctionSet): ErrorOr[WomValue] = Valid(value)
   override def evaluateType(inputTypes: Map[String, WomType]): ErrorOr[WomType] = Valid(value.womType)
   override def evaluateFiles(inputTypes: Map[String, WomValue], ioFunctionSet: IoFunctionSet, coerceTo: WomType): ErrorOr[Set[WomFile]] = Valid(Set.empty)