using validated types

broadinstitute · May 25, 2017 · dcd6bdd · dcd6bdd
1 parent 8399271
commit dcd6bdd
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 22 deletions.
diff --git a/core/src/main/scala/cromwell/core/package.scala b/core/src/main/scala/cromwell/core/package.scala
@@ -1,7 +1,12 @@
 package cromwell
 
+import cats.data.Validated._
+import cats.data.{NonEmptyList, ValidatedNel}
 import wdl4s.values.WdlValue
 
+import scala.util.{Failure, Success, Try}
+import scala.language.implicitConversions
+
 package object core {
   type LocallyQualifiedName = String
   type FullyQualifiedName = String
@@ -10,4 +15,21 @@ package object core {
   type CallOutputs = Map[LocallyQualifiedName, JobOutput]
   type HostInputs = Map[String, WdlValue]
   type EvaluatedRuntimeAttributes = Map[String, WdlValue]
+  type StringEither[A] = Either[NonEmptyList[String], A]
+  type StringValidated[A] = ValidatedNel[String, A]
+
+  implicit def tryToEither[A](trySomething: Try[A]): StringEither[A] = trySomething match {
+    case Success(options) => Right(options)
+    case Failure(err) => Left(NonEmptyList(err.getMessage, Nil))
+  }
+
+  def eitherToTry[A](validatedString: StringEither[A]): Try[A] = validatedString match {
+    case Right(options) => Success(options)
+    case Left(err) => Failure(new RuntimeException(s"Error(s): ${err.toList.mkString(",")}"))
+  }
+
+  implicit def tryToValidated[A](trySomething: Try[A]): StringValidated[A] = trySomething match {
+    case Success(options) => Valid(options)
+    case Failure(err) => invalidNel(err.getMessage)
+  }
 }
diff --git a/engine/src/main/scala/cromwell/webservice/CromwellApiService.scala b/engine/src/main/scala/cromwell/webservice/CromwellApiService.scala
@@ -1,13 +1,17 @@
 package cromwell.webservice
 
 import akka.actor._
-import cats.data.NonEmptyList
+import cats.data.{NonEmptyList, Validated}
+import cats.syntax.cartesian._
+import cats.syntax.either._
+import cats.data.Validated._
 import com.typesafe.config.{Config, ConfigFactory}
-import cromwell.core.{WorkflowId, WorkflowOptions, WorkflowOptionsJson, WorkflowSourceFilesCollection}
+import cromwell.core.{StringEither, WorkflowId, WorkflowOptions, WorkflowOptionsJson, WorkflowSourceFilesCollection}
 import cromwell.engine.backend.BackendConfiguration
 import cromwell.services.metadata.MetadataService._
 import cromwell.webservice.WorkflowJsonSupport._
 import cromwell.webservice.metadata.MetadataBuilderActor
+import cromwell.core._
 import spray.http.MediaTypes._
 import spray.http._
 import spray.httpx.SprayJsonSupport._
@@ -147,23 +151,28 @@ trait CromwellApiService extends HttpService with PerRequestCreator {
       }
     }
 
-    def partialSourcesToSourceCollections(partialSources: Try[PartialWorkflowSources], allowNoInputs: Boolean): Try[Seq[WorkflowSourceFilesCollection]] = {
-      partialSources flatMap {
-        case PartialWorkflowSources(Some(wdlSource), workflowInputs, workflowInputsAux, workflowOptions, labels, wdlDependencies) =>
-          //The order of addition allows for the expected override of colliding keys.
-          val sortedInputAuxes = workflowInputsAux.toSeq.sortBy(_._1).map(x => Option(x._2))
-          val wfInputs: Try[Seq[WdlJson]] = if (workflowInputs.isEmpty) {
-            if (allowNoInputs) Success(Vector("{}")) else Failure(new IllegalArgumentException("No inputs were provided"))
-          } else Success(workflowInputs map { workflowInputSet =>
-            mergeMaps(Seq(Option(workflowInputSet)) ++ sortedInputAuxes).toString
-          })
-          val wfOptions = WorkflowOptions.fromJsonString(workflowOptions.getOrElse("{}")) match {
-            case Failure(err) => Failure(new IllegalArgumentException(s"Invalid workflow options provided: ${err.getMessage}"))
-            case Success(x) => Success(x)
-          }
-          wfInputs.map(_.map(x => WorkflowSourceFilesCollection(wdlSource, x, wfOptions.get.asPrettyJson, labels.getOrElse("{}"), wdlDependencies)))
-        case other => Failure(new IllegalArgumentException(s"Incomplete workflow submission: $other"))
+    def partialSourcesToSourceCollections(partialSources: Try[PartialWorkflowSources], allowNoInputs: Boolean): StringEither[Seq[WorkflowSourceFilesCollection]] = {
+
+      def validateInputs(pws: PartialWorkflowSources): Validated[NonEmptyList[WdlJson], Seq[WdlJson]] =
+        (pws.workflowInputs.isEmpty, allowNoInputs) match {
+          case (true, true) => Valid(Vector("{}")).toValidatedNel
+          case (true, false) => invalidNel("No inputs were provided")
+          case _ => {
+            val sortedInputAuxes = pws.workflowInputsAux.toSeq.sortBy { case (index, _) => index } map { case(_, inputJson) => Option(inputJson) }
+            Valid(pws.workflowInputs map { workflowInputSet: WdlJson => mergeMaps(Seq(Option(workflowInputSet)) ++ sortedInputAuxes).toString }).toValidatedNel
+        }
       }
+
+      def validateOptions(options: Option[WorkflowOptionsJson]): Validated[NonEmptyList[String], WorkflowOptions] =
+        WorkflowOptions.fromJsonString(options.getOrElse("{}")) leftMap { _ map { i => s"Invalid workflow options provided: $i" } }
+
+      for {
+        partialSource <- tryToEither(partialSources)
+        wdlSource <- partialSource.wdlSource.toRight(NonEmptyList(s"Incomplete workflow submission: $partialSource", Nil))
+        workflowSourceFilesCollection <- (validateInputs(partialSource) |@| validateOptions(partialSource.workflowOptions) ).map { (wfInputs, wfOptions) =>
+          wfInputs.map(x => WorkflowSourceFilesCollection(wdlSource, x, wfOptions.asPrettyJson, partialSource.customLabels.getOrElse("{}"), partialSource.zippedImports))
+        }.toEither
+      } yield workflowSourceFilesCollection
     }
 
     def fromSubmitRoute(formData: MultipartFormData, allowNoInputs: Boolean): Try[Seq[WorkflowSourceFilesCollection]] = {
@@ -185,7 +194,7 @@ trait CromwellApiService extends HttpService with PerRequestCreator {
           throw new IllegalArgumentException(s"Unexpected body part name: ${bodyPart.name.getOrElse("None")}")
         }
       })
-      partialSourcesToSourceCollections(partialSources, allowNoInputs)
+      eitherToTry(partialSourcesToSourceCollections(partialSources, allowNoInputs))
     }
   }
 

diff --git a/engine/src/test/scala/cromwell/webservice/CromwellApiServiceSpec.scala b/engine/src/test/scala/cromwell/webservice/CromwellApiServiceSpec.scala
@@ -163,7 +163,7 @@ class CromwellApiServiceSpec extends FlatSpec with ScalatestRouteTest with Match
         assertResult(
           s"""{
               |  "status": "fail",
-              |  "message": "Unexpected body part name: incorrectParameter"
+              |  "message": "Error(s): Unexpected body part name: incorrectParameter"
               |}""".stripMargin) {
           responseAs[String]
         }
@@ -195,7 +195,7 @@ class CromwellApiServiceSpec extends FlatSpec with ScalatestRouteTest with Match
 
   it should "return 400 for a workflow submission with malformed workflow woptions json" in {
     val options = s"""
-                  |{"read_from_cache": true"
+                  |{"read_from_cache": "true"
                   |""".stripMargin
 
     val bodyParts = Map("wdlSource" -> BodyPart(HelloWorld.wdlSource()), "workflowOptions" -> BodyPart(options))
@@ -255,7 +255,7 @@ class CromwellApiServiceSpec extends FlatSpec with ScalatestRouteTest with Match
         assertResult(
           s"""{
               |  "status": "fail",
-              |  "message": "No inputs were provided"
+              |  "message": "Error(s): No inputs were provided"
               |}""".stripMargin) {
           responseAs[String]
         }