Pimp Validated with flatMap and switch to traverseU

- pimping Validated with `flatMap` means we can write much clearer code by switching to for comprehensions and avoid the use of `andThen` - using traverseU means that we don't have to produce singleton lists and labouriously combine them Leaving the use of andThen in the tests.
guardian · Oct 7, 2016 · d62b6bb · d62b6bb
1 parent 4b83d6e
commit d62b6bb
Show file tree

Hide file tree

Showing 6 changed files with 82 additions and 63 deletions.
diff --git a/magenta-lib/src/main/scala/magenta/input/RiffRaffYamlReader.scala b/magenta-lib/src/main/scala/magenta/input/RiffRaffYamlReader.scala
@@ -44,6 +44,7 @@ object RiffRaffYamlReader {
         Invalid(nelErrors.map{ case (path, validationErrors) =>
           ConfigError(s"Parsing $path", validationErrors.map(ve => ve.message).mkString(", "))
         })
+      case JsError(_) => ???
     }
   }
 }
diff --git a/magenta-lib/src/main/scala/magenta/input/models.scala b/magenta-lib/src/main/scala/magenta/input/models.scala
@@ -4,9 +4,6 @@ import cats.data.NonEmptyList
 import play.api.libs.json._
 
 case class ConfigError(context: String, message: String)
-object ConfigError {
-  def nel(context: String, message: String) = NonEmptyList.of(ConfigError(context, message))
-}
 
 case class RiffRaffDeployConfig(
   stacks: Option[List[String]],

diff --git a/magenta-lib/src/main/scala/magenta/input/resolver/DeploymentResolver.scala b/magenta-lib/src/main/scala/magenta/input/resolver/DeploymentResolver.scala
@@ -3,28 +3,29 @@ package magenta.input.resolver
 import cats.data.Validated.{Invalid, Valid}
 import cats.data.{NonEmptyList => NEL, Validated, ValidatedNel}
 import cats.syntax.cartesian._
-import cats.instances.all._
+import cats.syntax.traverse._
+import cats.instances.list._
 import magenta.input.{ConfigError, Deployment, DeploymentOrTemplate, RiffRaffDeployConfig}
 
 object DeploymentResolver {
   def resolve(config: RiffRaffDeployConfig): ValidatedNel[ConfigError, List[Deployment]] = {
-    config.deployments.map { case (label, rawDeployment) =>
-      applyTemplates(label, rawDeployment, config.templates).andThen { templated =>
-        resolveDeployment(label, templated, config.stacks, config.regions)
-      }.andThen { deployment =>
-        validateDependencies(label, deployment, config.deployments)
-      }.map(List(_))
-    }.reduceLeft(_ combine _)
+    config.deployments.traverseU[ValidatedNel[ConfigError, Deployment]] { case (label, rawDeployment) =>
+      for {
+        templated <- applyTemplates(label, rawDeployment, config.templates)
+        deployment <- resolveDeployment(label, templated, config.stacks, config.regions)
+        validatedDeployment <- validateDependencies(label, deployment, config.deployments)
+      } yield validatedDeployment
+    }
   }
 
   /**
     * Validates and resolves a templated deployment by merging its
     * deployment attributes with any globally defined properties.
     */
   private[input] def resolveDeployment(label: String, templated: DeploymentOrTemplate, globalStacks: Option[List[String]], globalRegions: Option[List[String]]): ValidatedNel[ConfigError, Deployment] = {
-    (Validated.fromOption(templated.`type`, ConfigError.nel(label, "No type field provided")) |@|
-      Validated.fromOption(templated.stacks.orElse(globalStacks), ConfigError.nel(label, "No stacks provided")) |@|
-      Validated.fromOption(templated.regions.orElse(globalRegions), ConfigError.nel(label, "No regions provided"))) map { (deploymentType, stacks, regions) =>
+    (Validated.fromOption(templated.`type`, NEL.of(ConfigError(label, "No type field provided"))) |@|
+      Validated.fromOption(templated.stacks.orElse(globalStacks), NEL.of(ConfigError(label, "No stacks provided"))) |@|
+      Validated.fromOption(templated.regions.orElse(globalRegions), NEL.of(ConfigError(label, "No regions provided")))) map { (deploymentType, stacks, regions) =>
       Deployment(
         name = label,
         `type` = deploymentType,
@@ -48,10 +49,13 @@ object DeploymentResolver {
       case None =>
         Valid(template)
       case Some(parentTemplateName) =>
-        Validated.fromOption(templates.flatMap(_.get(parentTemplateName)),
-          ConfigError.nel(templateName, s"Template with name $parentTemplateName does not exist")).andThen { parentTemplate =>
-          applyTemplates(parentTemplateName, parentTemplate, templates)
-        }.map { resolvedParent =>
+        for {
+          parentTemplate <- {
+            Validated.fromOption(templates.flatMap(_.get(parentTemplateName)),
+              NEL.of(ConfigError(templateName, s"Template with name $parentTemplateName does not exist")))
+          }
+          resolvedParent <- applyTemplates(parentTemplateName, parentTemplate, templates)
+        } yield {
           DeploymentOrTemplate(
             `type` = template.`type`.orElse(resolvedParent.`type`),
             template = None,
@@ -76,7 +80,7 @@ object DeploymentResolver {
       case Nil =>
         Valid(deployment)
       case missingDependencies =>
-        Invalid(ConfigError.nel(label, missingDependencies.mkString(s"Missing deployment dependencies ", ", ", "")))
+        Invalid(NEL.of(ConfigError(label, missingDependencies.mkString(s"Missing deployment dependencies ", ", ", ""))))
     }
   }
 }
diff --git a/magenta-lib/src/main/scala/magenta/input/resolver/Resolver.scala b/magenta-lib/src/main/scala/magenta/input/resolver/Resolver.scala
@@ -3,6 +3,7 @@ package magenta.input.resolver
 import cats.data.Validated.{Invalid, Valid}
 import cats.data.{ValidatedNel, NonEmptyList => NEL}
 import cats.instances.all._
+import cats.syntax.traverse._
 import magenta.artifact.S3Artifact
 import magenta.deployment_type.DeploymentType
 import magenta.graph.{DeploymentTasks, Graph}
@@ -13,46 +14,53 @@ object Resolver {
   def resolve(yamlConfig: String, deploymentResources: DeploymentResources, parameters: DeployParameters,
     deploymentTypes: Seq[DeploymentType], artifact: S3Artifact): ValidatedNel[ConfigError, Graph[DeploymentTasks]] = {
 
-    resolveDeploymentGraph(yamlConfig).andThen { graph =>
-      val flattenedGraph = DeploymentGraphActionFlattening.flattenActions(graph)
-      val deploymentTaskGraph = flattenedGraph.map { deployment =>
-        TaskResolver.resolve(deployment, deploymentResources, parameters, deploymentTypes, artifact)
-      }
+    for {
+      deploymentGraph <- resolveDeploymentGraph(yamlConfig)
+      taskGraph <- buildTaskGraph(deploymentResources, parameters, deploymentTypes, artifact, deploymentGraph)
+    } yield taskGraph
+  }
 
-      if (deploymentTaskGraph.nodes.values.exists(_.isInvalid)) {
-        Invalid(deploymentTaskGraph.toList.collect { case Invalid(errors) => errors }.reduce(_ concat _))
-      } else {
-        // we know they are all good
-        Valid(deploymentTaskGraph.map(_.toOption.get))
+  def resolveDeploymentGraph(yamlConfig: String): ValidatedNel[ConfigError, Graph[Deployment]] = {
+    for {
+      config <- RiffRaffYamlReader.fromString(yamlConfig)
+      deployments <- DeploymentResolver.resolve(config)
+      validatedDeployments <- deployments.traverseU[ValidatedNel[ConfigError, Deployment]]{deployment =>
+        DeploymentTypeResolver.validateDeploymentType(deployment, DeploymentType.all)
       }
-    }
+      userSelectedDeployments = validatedDeployments
+      graph = buildParallelisedGraph(userSelectedDeployments)
+    } yield graph
   }
 
-  def resolveDeploymentGraph(yamlConfig: String): ValidatedNel[ConfigError, Graph[Deployment]] = {
-    val validatedDeployments = RiffRaffYamlReader.fromString(yamlConfig).andThen { config =>
-      DeploymentResolver.resolve(config)
-    }.andThen { deployments =>
-      deployments.map { deployment =>
-        DeploymentTypeResolver.validateDeploymentType(deployment, DeploymentType.all).map(List(_))
-      }.reduceLeft(_ combine _)
+  private def buildTaskGraph(deploymentResources: DeploymentResources, parameters: DeployParameters,
+    deploymentTypes: Seq[DeploymentType], artifact: S3Artifact, graph: Graph[Deployment]) = {
+
+    val flattenedGraph = DeploymentGraphActionFlattening.flattenActions(graph)
+    val deploymentTaskGraph = flattenedGraph.map { deployment =>
+      TaskResolver.resolve(deployment, deploymentResources, parameters, deploymentTypes, artifact)
     }
 
-    validatedDeployments.map { deployments =>
-      // TODO: this is a placeholder for when we filter previews in the UI
-      val userSelectedDeployments = deployments
+    if (deploymentTaskGraph.nodes.values.exists(_.isInvalid)) {
+      Invalid(deploymentTaskGraph.toList.collect { case Invalid(errors) => errors }.reduce(_ concat _))
+    } else {
+      // we know they are all good
+      Valid(deploymentTaskGraph.map(_.toOption.get))
+    }
+  }
 
-      val stacks = userSelectedDeployments.flatMap(_.stacks.toList).distinct
-      val regions = userSelectedDeployments.flatMap(_.regions.toList).distinct
 
-      val stackRegionGraphs: List[Graph[Deployment]] = for {
-        stack <- stacks
-        region <- regions
-        deploymentsForStackAndRegion = filterDeployments(userSelectedDeployments, stack, region)
-        graphForStackAndRegion = DeploymentGraphBuilder.buildGraph(deploymentsForStackAndRegion)
-      } yield graphForStackAndRegion
+  private def buildParallelisedGraph(userSelectedDeployments: List[Deployment]) = {
+    val stacks = userSelectedDeployments.flatMap(_.stacks.toList).distinct
+    val regions = userSelectedDeployments.flatMap(_.regions.toList).distinct
 
-      stackRegionGraphs.reduceLeft(_ joinParallel _)
-    }
+    val stackRegionGraphs: List[Graph[Deployment]] = for {
+      stack <- stacks
+      region <- regions
+      deploymentsForStackAndRegion = filterDeployments(userSelectedDeployments, stack, region)
+      graphForStackAndRegion = DeploymentGraphBuilder.buildGraph(deploymentsForStackAndRegion)
+    } yield graphForStackAndRegion
+
+    stackRegionGraphs.reduceLeft(_ joinParallel _)
   }
 
   private[resolver] def filterDeployments(deployments: List[Deployment], stack: String, region: String): List[Deployment] = {

diff --git a/magenta-lib/src/main/scala/magenta/input/resolver/package.scala b/magenta-lib/src/main/scala/magenta/input/resolver/package.scala
@@ -0,0 +1,9 @@
+package magenta.input
+
+import cats.data.Validated
+
+package object resolver {
+  implicit class RichValidated[E, A](validated: Validated[E, A]) {
+    def flatMap[EE >: E, B](f: A => Validated[EE, B]): Validated[EE, B] = validated.andThen(f)
+  }
+}
diff --git a/magenta-lib/src/test/scala/magenta/input/resolver/DeploymentResolverTest.scala b/magenta-lib/src/test/scala/magenta/input/resolver/DeploymentResolverTest.scala
@@ -20,7 +20,7 @@ class DeploymentResolverTest extends FlatSpec with ShouldMatchers with Validated
       """.stripMargin
     val yaml = RiffRaffYamlReader.fromString(yamlString)
     val deployments = yaml.andThen(DeploymentResolver.resolve).valid
-    deployments.toList.size should be (1)
+    deployments.size should be (1)
     deployments.head should have (
       'type ("testType"),
       'stacks (NEL.of("testStack")),
@@ -46,7 +46,7 @@ class DeploymentResolverTest extends FlatSpec with ShouldMatchers with Validated
       """.stripMargin
     val yaml = RiffRaffYamlReader.fromString(yamlString)
     val deployments = yaml.andThen(DeploymentResolver.resolve).valid
-    deployments.toList.size should be (1)
+    deployments.size should be (1)
     deployments.head should have (
       'type ("testType"),
       'stacks (NEL.of("stack1", "stack2")),
@@ -75,7 +75,7 @@ class DeploymentResolverTest extends FlatSpec with ShouldMatchers with Validated
       """.stripMargin
     val yaml = RiffRaffYamlReader.fromString(yamlString)
     val deployments = yaml.andThen(DeploymentResolver.resolve).valid
-    deployments.toList.size should be (1)
+    deployments.size should be (1)
     deployments.head should have (
       'type ("testType"),
       'stacks (NEL.of("testStack")),
@@ -106,7 +106,7 @@ class DeploymentResolverTest extends FlatSpec with ShouldMatchers with Validated
       """.stripMargin
     val yaml = RiffRaffYamlReader.fromString(yamlString)
     val deployments = yaml.andThen(DeploymentResolver.resolve).valid
-    deployments.toList.size should be (1)
+    deployments.size should be (1)
     deployments.head should have (
       'type ("testType"),
       'stacks (NEL.of("testStack")),
@@ -136,7 +136,7 @@ class DeploymentResolverTest extends FlatSpec with ShouldMatchers with Validated
       """.stripMargin
     val yaml = RiffRaffYamlReader.fromString(yamlString)
     val deployments = yaml.andThen(DeploymentResolver.resolve).valid
-    deployments.toList.size should be (1)
+    deployments.size should be (1)
     deployments.head should have(
       'stacks (NEL.of("deployment-stack")),
       'regions (NEL.of("deployment-region"))
@@ -159,7 +159,7 @@ class DeploymentResolverTest extends FlatSpec with ShouldMatchers with Validated
       """.stripMargin
     val yaml = RiffRaffYamlReader.fromString(yamlString)
     val deployments = yaml.andThen(DeploymentResolver.resolve).valid
-    deployments.toList.size should be (1)
+    deployments.size should be (1)
     deployments.head should have(
       'stacks (NEL.of("template-stack")),
       'regions (NEL.of("template-region"))
@@ -180,7 +180,7 @@ class DeploymentResolverTest extends FlatSpec with ShouldMatchers with Validated
       """.stripMargin
     val yaml = RiffRaffYamlReader.fromString(yamlString)
     val deployments = yaml.andThen(DeploymentResolver.resolve).valid
-    deployments.toList.size should be (1)
+    deployments.size should be (1)
     deployments.head should have(
       'stacks (NEL.of("global-stack")),
       'regions (NEL.of("global-region"))
@@ -206,7 +206,7 @@ class DeploymentResolverTest extends FlatSpec with ShouldMatchers with Validated
       """.stripMargin
     val yaml = RiffRaffYamlReader.fromString(yamlString)
     val deployments = yaml.andThen(DeploymentResolver.resolve).valid
-    deployments.toList.size should be (1)
+    deployments.size should be (1)
     deployments.head should have(
       'stacks (NEL.of("nested-template-stack")),
       'regions (NEL.of("template-region"))
@@ -242,7 +242,7 @@ class DeploymentResolverTest extends FlatSpec with ShouldMatchers with Validated
       """.stripMargin
     val yaml = RiffRaffYamlReader.fromString(yamlString)
     val deployments = yaml.andThen(DeploymentResolver.resolve).valid
-    deployments.toList.size should be (1)
+    deployments.size should be (1)
     val deployment = deployments.head
     deployment.parameters.size should be(6)
     deployment.parameters should contain("nestedParameter" -> JsNumber(1984))
@@ -270,7 +270,7 @@ class DeploymentResolverTest extends FlatSpec with ShouldMatchers with Validated
       """.stripMargin
     val yaml = RiffRaffYamlReader.fromString(yamlString)
     val deployments = yaml.andThen(DeploymentResolver.resolve).valid
-    deployments.toList.size should be (1)
+    deployments.size should be (1)
     deployments.head should have(
       'app ("templateApp"),
       'actions (Some(List("templateAction"))),
@@ -303,7 +303,7 @@ class DeploymentResolverTest extends FlatSpec with ShouldMatchers with Validated
       """.stripMargin
     val yaml = RiffRaffYamlReader.fromString(yamlString)
     val deployments = yaml.andThen(DeploymentResolver.resolve).valid
-    deployments.toList.size should be (4)
+    deployments.size should be (4)
     val deployment = deployments.find(_.name == "test").get
     deployment.dependencies should be(List("deployment-dep"))
   }
@@ -330,7 +330,7 @@ class DeploymentResolverTest extends FlatSpec with ShouldMatchers with Validated
       """.stripMargin
     val yaml = RiffRaffYamlReader.fromString(yamlString)
     val deployments = yaml.andThen(DeploymentResolver.resolve).valid
-    deployments.toList.size should be (3)
+    deployments.size should be (3)
     val deployment = deployments.find(_.name == "test").get
     deployment.dependencies should be(List("template-dep"))
   }
@@ -354,7 +354,7 @@ class DeploymentResolverTest extends FlatSpec with ShouldMatchers with Validated
       """.stripMargin
     val yaml = RiffRaffYamlReader.fromString(yamlString)
     val deployments = yaml.andThen(DeploymentResolver.resolve).valid
-    deployments.toList.size should be (2)
+    deployments.size should be (2)
     val deployment = deployments.find(_.name == "test").get
     deployment.dependencies should be(List("nested-dep"))
   }