Meta section [BA-5797] (broadinstitute#5053)

* WIP * WIP * WIP * WIP * Fixing code review comment, and fixing a failing test for draft3-transforms * Supporting only what is necessary in terms of an io.circe JSON encoder for the meta values * Converting to io.circe JSON values correctly * Fixing code review comments
AlekseiLitkovetc · Jul 15, 2019 · 1fed0a4 · 1fed0a4
1 parent 5e8157a
commit 1fed0a4
Show file tree

Hide file tree

Showing 13 changed files with 123 additions and 58 deletions.
diff --git a/cwl/src/main/scala/cwl/Workflow.scala b/cwl/src/main/scala/cwl/Workflow.scala
@@ -16,7 +16,7 @@ import cwl.command.ParentName
 import shapeless._
 import shapeless.syntax.singleton._
 import wom.SourceFileLocation
-import wom.callable.WorkflowDefinition
+import wom.callable.{MetaValueElement, WorkflowDefinition}
 import wom.executable.Executable
 import wom.expression.{IoFunctionSet, ValueAsAnExpression}
 import wom.graph.GraphNodePort.{GraphNodeOutputPort, OutputPort}
@@ -188,8 +188,8 @@ case class Workflow private(
 
   def womDefinition(validator: RequirementsValidator, expressionLib: ExpressionLib): Checked[WorkflowDefinition] = {
     val name: String = Paths.get(id).getFileName.toString
-    val meta: Map[String, String] = Map.empty
-    val paramMeta: Map[String, String] = Map.empty
+    val meta: Map[String, MetaValueElement.MetaValueElementString] = Map.empty
+    val paramMeta: Map[String, MetaValueElement.MetaValueElementString] = Map.empty
     val lexInfo : Option[SourceFileLocation] = None
 
     womGraph(name, validator, expressionLib).map(graph =>

diff --git a/services/src/main/scala/cromwell/services/womtool/models/MetaValueElementJsonSupport.scala b/services/src/main/scala/cromwell/services/womtool/models/MetaValueElementJsonSupport.scala
@@ -0,0 +1,39 @@
+package cromwell.services.womtool.models
+
+import io.circe.{Encoder, Json}
+
+import wom.callable.MetaValueElement
+import wom.callable.MetaValueElement._
+
+object MetaValueElementJsonSupport {
+
+  // We only implement the encoder, because currently, the decoder is not required.
+  implicit val metaValueElementEncoder: Encoder[MetaValueElement] = new Encoder[MetaValueElement] {
+    final def apply(a : MetaValueElement) : Json = {
+      a match {
+        case MetaValueElementNull =>
+          Json.Null
+
+        case MetaValueElementBoolean(b) =>
+          Json.fromBoolean(b)
+
+        case MetaValueElementFloat(x) =>
+          Json.fromDouble(x).get
+
+        case MetaValueElementInteger(a) =>
+          Json.fromInt(a)
+
+        case MetaValueElementString(s) =>
+          Json.fromString(s)
+
+        case MetaValueElementArray(vec) =>
+          Json.fromValues(vec.map(apply))
+
+        case MetaValueElementObject(m) =>
+          Json.fromFields(m.map{ case (key, value) =>
+                            key -> apply(value)
+                          })
+      }
+    }
+  }
+}
diff --git a/services/src/main/scala/cromwell/services/womtool/models/WorkflowDescription.scala b/services/src/main/scala/cromwell/services/womtool/models/WorkflowDescription.scala
@@ -4,12 +4,12 @@ import io.circe.{Decoder, Encoder, HCursor}
 import io.circe.generic.semiauto.deriveEncoder
 import wom.RuntimeAttributesKeys
 import wom.callable.Callable.{InputDefinition, InputDefinitionWithDefault, OutputDefinition}
-import wom.callable.{Callable, CallableTaskDefinition, WorkflowDefinition}
+import wom.callable.{Callable, CallableTaskDefinition, MetaValueElement, WorkflowDefinition}
 import wom.executable.WomBundle
 
-// `meta`, `parameterMeta` will need updating to support compound types - issue #4746
-case class WorkflowDescription(
-                                valid: Boolean,
+import MetaValueElementJsonSupport._
+
+case class WorkflowDescription( valid: Boolean,
                                 errors: List[String],
                                 validWorkflow: Boolean,
                                 name: String,
@@ -18,10 +18,9 @@ case class WorkflowDescription(
                                 images: List[String],
                                 submittedDescriptorType: Map[String, String],
                                 importedDescriptorTypes: List[Map[String, String]],
-                                meta: Map[String, String],
-                                parameterMeta: Map[String, String],
-                                isRunnableWorkflow: Boolean
-                              )
+                                meta: Map[String, MetaValueElement],
+                                parameterMeta: Map[String, MetaValueElement],
+                                isRunnableWorkflow: Boolean)
 
 case object WorkflowDescription {
 
@@ -76,8 +75,8 @@ case object WorkflowDescription {
                                inputErrors: List[String],
                                name: String, submittedDescriptorType: Map[String, String],
                                inputs: List[InputDefinition], outputs: List[OutputDefinition],
-                               meta: Map[String, String],
-                               parameterMeta: Map[String, String],
+                               meta: Map[String, MetaValueElement],
+                               parameterMeta: Map[String, MetaValueElement],
                                images: List[String],
                                isRunnableWorkflow: Boolean
                              ): WorkflowDescription = {
@@ -136,8 +135,8 @@ case object WorkflowDescription {
             images: List[String] = List.empty,
             submittedDescriptorType: Map[String, String] = Map.empty,
             importedDescriptorTypes: List[Map[String, String]] = List.empty,
-            meta: Map[String, String] = Map.empty,
-            parameterMeta: Map[String, String] = Map.empty,
+            meta: Map[String, MetaValueElement] = Map.empty,
+            parameterMeta: Map[String, MetaValueElement] = Map.empty,
             isRunnableWorkflow: Boolean = false): WorkflowDescription = {
     new WorkflowDescription(valid, errors, validWorkflow, name, inputs, outputs, images, submittedDescriptorType, importedDescriptorTypes, meta, parameterMeta, isRunnableWorkflow)
   }

diff --git a/...c/main/scala/wdl/transforms/draft2/wdlom2wom/WdlDraft2WomCommandTaskDefinitionMaker.scala b/...c/main/scala/wdl/transforms/draft2/wdlom2wom/WdlDraft2WomCommandTaskDefinitionMaker.scala
@@ -6,11 +6,12 @@ import wdl.draft2.model.{AstTools, WdlTask, WdlWomExpression}
 import wdl.draft2.parser.WdlParser.Terminal
 import wom.SourceFileLocation
 import wom.callable.Callable.{OverridableInputDefinitionWithDefault, OptionalInputDefinition, RequiredInputDefinition}
-import wom.callable.{Callable, CallableTaskDefinition, CommandTaskDefinition}
+import wom.callable.{Callable, CallableTaskDefinition, CommandTaskDefinition, MetaValueElement}
 import wom.graph.LocalName
 import wom.transforms.WomCommandTaskDefinitionMaker
 import wom.types.WomOptionalType
 
+
 object WdlDraft2WomCommandTaskDefinitionMaker extends WomCommandTaskDefinitionMaker[WdlTask] {
 
   override def toWomTaskDefinition(wdlTask: WdlTask): ErrorOr[CommandTaskDefinition] = {
@@ -27,12 +28,21 @@ object WdlDraft2WomCommandTaskDefinitionMaker extends WomCommandTaskDefinitionMa
     // Figure out the start line of the workflow in the source file
     val t: Option[Terminal] = AstTools.findTerminals(wdlTask.ast).headOption
 
+    // Draft-2 only support string values. It does not support composite values, or
+    // anything
+    def stringifyMetaValues(meta: Map[String, String]): Map[String, MetaValueElement] = {
+      meta map {
+        case (key, value) =>
+          key -> MetaValueElement.MetaValueElementString(value)
+      }
+    }
+
     CallableTaskDefinition(
       name = wdlTask.fullyQualifiedName,
       commandTemplateBuilder = Function.const(wdlTask.commandTemplate.validNel),
       runtimeAttributes = wdlTask.runtimeAttributes.toWomRuntimeAttributes(wdlTask),
-      meta = wdlTask.meta,
-      parameterMeta = wdlTask.parameterMeta,
+      meta = stringifyMetaValues(wdlTask.meta),
+      parameterMeta = stringifyMetaValues(wdlTask.parameterMeta),
       outputs = wdlTask.outputs.map(_.womOutputDefinition).toList,
       inputs = womInputs,
       adHocFileCreation = Set.empty,

diff --git a/.../src/main/scala/wdl/transforms/draft2/wdlom2wom/WdlDraft2WomWorkflowDefinitionMaker.scala b/.../src/main/scala/wdl/transforms/draft2/wdlom2wom/WdlDraft2WomWorkflowDefinitionMaker.scala
@@ -3,7 +3,7 @@ package wdl.transforms.draft2.wdlom2wom
 import common.validation.ErrorOr.ErrorOr
 import wdl.draft2.model.{AstTools, Scope, WdlWorkflow}
 import wdl.draft2.parser.WdlParser.Terminal
-import wom.callable.WorkflowDefinition
+import wom.callable.{MetaValueElement, WorkflowDefinition}
 import wom.transforms.WomWorkflowDefinitionMaker
 import wom.transforms.WomGraphMaker.ops._
 import wom.SourceFileLocation
@@ -15,12 +15,19 @@ object WdlDraft2WomWorkflowDefinitionMaker extends WomWorkflowDefinitionMaker[Wd
     // Figure out the start line of the workflow in the source file
     val t: Terminal = AstTools.findTerminals(wdlWorkflow.ast).head
 
+    def stringifyMetaValues(meta: Map[String, String]): Map[String, MetaValueElement] = {
+      meta map {
+        case (key, value) =>
+          key -> MetaValueElement.MetaValueElementString(value)
+      }
+    }
+
     (wdlWorkflow: Scope).toWomGraph(Set.empty, Map.empty, preserveIndexForOuterLookups = true, isASubworkflow: Boolean) map { wg =>
       WorkflowDefinition(
         wdlWorkflow.fullyQualifiedName,
         wg,
-        wdlWorkflow.meta,
-        wdlWorkflow.parameterMeta,
+        stringifyMetaValues(wdlWorkflow.meta),
+        stringifyMetaValues(wdlWorkflow.parameterMeta),
         Some(SourceFileLocation(t.getLine)))
     }
   }

diff --git a/wdl/transforms/draft3/src/test/cases/task_with_metas2.wdl b/wdl/transforms/draft3/src/test/cases/task_with_metas2.wdl
@@ -13,5 +13,12 @@ task task_with_metas2 {
   meta {
     author: "John Doe"
     email: "[email protected]"
+    b : true
+    zipcode: 94043
+    f : 1.3
+    numbers : [1, 2, 3]
+    extras: {
+      house : "With porch",
+      cat : "Lucy" }
   }
 }
diff --git a/...transforms/draft3/src/test/scala/wdl/draft3/transforms/ast2wdlom/WdlFileToWdlomSpec.scala b/...transforms/draft3/src/test/scala/wdl/draft3/transforms/ast2wdlom/WdlFileToWdlomSpec.scala
@@ -385,7 +385,17 @@ object WdlFileToWdlomSpec {
             runtimeSection = None,
             metaSection = Some(MetaSectionElement(
               Map("author" -> MetaValueElementString("John Doe"),
-                  "email" -> MetaValueElementString("[email protected]")))),
+                  "email" -> MetaValueElementString("[email protected]"),
+                  "b" -> MetaValueElementBoolean(true),
+                  "zipcode" -> MetaValueElementInteger(94043),
+                  "f" -> MetaValueElementFloat(1.3),
+                  "numbers" -> MetaValueElementArray(Vector(MetaValueElementInteger(1),
+                                                            MetaValueElementInteger(2),
+                                                            MetaValueElementInteger(3))),
+                  "extras" -> MetaValueElementObject(
+                    Map( "house" -> MetaValueElementString("With porch"),
+                         "cat" -> MetaValueElementString("Lucy")))
+              ))),
             parameterMetaSection = None,
             sourceLocation = Some(SourceFileLocation(3))
           ))),

diff --git a/wdl/transforms/draft3/src/test/scala/wdl/draft3/transforms/wdlom2wom/WdlFileToWomSpec.scala b/wdl/transforms/draft3/src/test/scala/wdl/draft3/transforms/wdlom2wom/WdlFileToWomSpec.scala
@@ -12,7 +12,7 @@ import wdl.model.draft3.elements.CommandPartElement.StringCommandPartElement
 import wdl.model.draft3.elements.ExpressionElement.StringLiteral
 import wdl.transforms.base.wdlom2wom._
 import wom.callable.Callable.{FixedInputDefinitionWithDefault, OptionalInputDefinition}
-import wom.callable.MetaValueElement.{MetaValueElementBoolean, MetaValueElementObject}
+import wom.callable.MetaValueElement._
 import wom.callable.{CallableTaskDefinition, WorkflowDefinition}
 import wom.executable.WomBundle
 import wom.graph.expression.{ExposedExpressionNode, TaskCallInputExpressionNode}
@@ -164,7 +164,16 @@ class WdlFileToWomSpec extends FlatSpec with Matchers {
   private def validateMetaSection(b: WomBundle): Assertion = {
     val task = b.primaryCallable.get.asInstanceOf[CallableTaskDefinition]
 
-    task.meta should be (Map("author" -> "John Doe",
-                             "email" -> "[email protected]"))
+    task.meta should be (Map("author" -> MetaValueElementString("John Doe"),
+                             "email" -> MetaValueElementString("[email protected]"),
+                             "b" -> MetaValueElementBoolean(true),
+                             "zipcode" -> MetaValueElementInteger(94043),
+                             "f" -> MetaValueElementFloat(1.3),
+                             "numbers" -> MetaValueElementArray(Vector(MetaValueElementInteger(1),
+                                                                       MetaValueElementInteger(2),
+                                                                       MetaValueElementInteger(3))),
+                             "extras" -> MetaValueElementObject(Map("house" -> MetaValueElementString("With porch"),
+                                                                    "cat" -> MetaValueElementString("Lucy")))
+                         ))
   }
 }
diff --git a/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/Util.scala b/wdl/transforms/new-base/src/main/scala/wdl/transforms/base/wdlom2wom/Util.scala
@@ -1,25 +1,14 @@
 package wdl.transforms.base.wdlom2wom
 
 import wdl.model.draft3.elements.{MetaSectionElement, ParameterMetaSectionElement}
-import wom.callable.MetaValueElement
 
 trait Util {
 
   def processMetaSections(meta: Option[MetaSectionElement], parameterMeta: Option[ParameterMetaSectionElement]) = {
-
-    def stringifyMetaValues(meta: Map[String, MetaValueElement]): Map[String, String] = {
-      meta map {
-        case (key, MetaValueElement.MetaValueElementString(value)) =>
-          key -> value
-        case (key, other) =>
-          key -> s"Compound types not yet supported, see #4746. String approximation: ${other.toString}"
-      }
-    }
-
     val metaMap = meta.map(_.meta).getOrElse(Map.empty)
     val parameterMetaMap = parameterMeta.map(_.metaAttributes).getOrElse(Map.empty)
 
-    (stringifyMetaValues(metaMap), stringifyMetaValues(parameterMetaMap))
+    (metaMap, parameterMetaMap)
   }
 
 }
diff --git a/wom/src/main/scala/wom/callable/CommandTaskDefinition.scala b/wom/src/main/scala/wom/callable/CommandTaskDefinition.scala
@@ -55,8 +55,8 @@ object CommandTaskDefinition {
   */
 sealed trait TaskDefinition extends Callable {
   def runtimeAttributes: RuntimeAttributes
-  def meta: Map[String, String]
-  def parameterMeta: Map[String, String]
+  def meta: Map[String, MetaValueElement]
+  def parameterMeta: Map[String, MetaValueElement]
 
   /**
     * Transform the Callable TaskDefinition to an ExecutableCallable that can be executed on its own.
@@ -137,8 +137,8 @@ sealed trait CommandTaskDefinition extends TaskDefinition {
 final case class CallableTaskDefinition(name: String,
                                         commandTemplateBuilder: WomEvaluatedCallInputs => ErrorOr[Seq[CommandPart]],
                                         runtimeAttributes: RuntimeAttributes,
-                                        meta: Map[String, String],
-                                        parameterMeta: Map[String, String],
+                                        meta: Map[String, MetaValueElement],
+                                        parameterMeta: Map[String, MetaValueElement],
                                         outputs: List[Callable.OutputDefinition],
                                         inputs: List[_ <: Callable.InputDefinition],
                                         adHocFileCreation: Set[ContainerizedInputExpression],
@@ -198,8 +198,8 @@ sealed trait ExpressionTaskDefinition extends TaskDefinition {
 final case class CallableExpressionTaskDefinition(name: String,
                                                   evaluate: (Map[String, WomValue], IoFunctionSet, List[OutputPort]) => Checked[Map[OutputPort, WomValue]],
                                                   runtimeAttributes: RuntimeAttributes,
-                                                  meta: Map[String, String],
-                                                  parameterMeta: Map[String, String],
+                                                  meta: Map[String, MetaValueElement],
+                                                  parameterMeta: Map[String, MetaValueElement],
                                                   outputs: List[Callable.OutputDefinition],
                                                   inputs: List[_ <: Callable.InputDefinition],
                                                   prefixSeparator: String = ".",

diff --git a/wom/src/main/scala/wom/callable/WorkflowDefinition.scala b/wom/src/main/scala/wom/callable/WorkflowDefinition.scala
@@ -6,8 +6,8 @@ import wom.graph.{CommandCallNode, Graph}
 
 final case class WorkflowDefinition(name: String,
                                     innerGraph: Graph,
-                                    meta: Map[String, String],
-                                    parameterMeta: Map[String, String],
+                                    meta: Map[String, MetaValueElement],
+                                    parameterMeta: Map[String, MetaValueElement],
                                     override val sourceLocation : Option[SourceFileLocation]) extends ExecutableCallable {
 
   override lazy val toString = s"[Workflow $name]"

diff --git a/wom/src/test/resources/three_step/test.wdl b/wom/src/test/resources/three_step/test.wdl
@@ -37,4 +37,4 @@ workflow three_step {
   call wc {
     input: in_file = ps.procs
   }
-}
+}
diff --git a/womtool/src/main/scala/womtool/wom2wdlom/WomToWdlom.scala b/womtool/src/main/scala/womtool/wom2wdlom/WomToWdlom.scala
@@ -14,8 +14,7 @@ import wdl.model.draft3.elements.CommandPartElement.{PlaceholderCommandPartEleme
 import wdl.model.draft3.elements.ExpressionElement.ExpressionLiteralElement
 import wom.callable.Callable._
 import wom.callable.Callable.OutputDefinition
-import wom.callable.{CallableTaskDefinition, WorkflowDefinition}
-import wom.callable.MetaValueElement.MetaValueElementString
+import wom.callable.{CallableTaskDefinition, MetaValueElement, WorkflowDefinition}
 import wom.executable.WomBundle
 import wom.expression.WomExpression
 import wom.graph._
@@ -65,22 +64,18 @@ object WomToWdlom {
       }
     }
 
-  def mapToMetaSectionElement: CheckedAtoB[Map[String, String], Option[MetaSectionElement]] =
-    CheckedAtoB.fromCheck { a: Map[String, String] =>
+  def mapToMetaSectionElement: CheckedAtoB[Map[String, MetaValueElement], Option[MetaSectionElement]] =
+    CheckedAtoB.fromCheck { a: Map[String, MetaValueElement] =>
       if (a.nonEmpty)
-        Some(MetaSectionElement(a map { case (key, value) =>
-          key -> MetaValueElementString(value) // draft-2: strings only
-        })).validNelCheck
+        Some(MetaSectionElement(a)).validNelCheck
       else
         None.validNelCheck
     }
 
-  def mapToParameterMetaSectionElement: CheckedAtoB[Map[String, String], Option[ParameterMetaSectionElement]] =
-    CheckedAtoB.fromCheck { a: Map[String, String] =>
+  def mapToParameterMetaSectionElement: CheckedAtoB[Map[String, MetaValueElement], Option[ParameterMetaSectionElement]] =
+    CheckedAtoB.fromCheck { a: Map[String, MetaValueElement] =>
       if (a.nonEmpty)
-        Some(ParameterMetaSectionElement(a map { case (key, value) =>
-          key -> MetaValueElementString(value) // draft-2: strings only
-        })).validNelCheck
+        Some(ParameterMetaSectionElement(a)).validNelCheck
       else
         None.validNelCheck
     }
-Original file line number
+Diff line change
@@ Expand Up / @@ -37,4 +37,4 @@ workflow three_step { @@
       call wc {
         input: in_file = ps.procs
       }
-    }
+    }