Reinstate runtime attributes

Author: danbills

backend/src/main/scala/cromwell/backend/BackendWorkflowInitializationActor.scala

@@ -1,23 +1,27 @@
 package cromwell.backend
 
-import _root_.wdl._
-import _root_.wdl.expression.PureStandardLibraryFunctions
 import akka.actor.{ActorLogging, ActorRef}
 import akka.event.LoggingReceive
+import cats.data.Validated.{Invalid, Valid, _}
+import cats.data.{NonEmptyList, ValidatedNel}
+import cats.instances.list._
+import cats.syntax.traverse._
+import common.validation.Validation._
 import cromwell.backend.BackendLifecycleActor._
 import cromwell.backend.BackendWorkflowInitializationActor._
-import cromwell.backend.async.RuntimeAttributeValidationFailures
+import cromwell.backend.async.{RuntimeAttributeValidationFailure, RuntimeAttributeValidationFailures}
 import cromwell.backend.validation.ContinueOnReturnCodeValidation
-import cromwell.core.{WorkflowMetadataKeys, WorkflowOptions}
+import cromwell.core.{NoIoFunctionSet, WorkflowMetadataKeys, WorkflowOptions}
 import cromwell.services.metadata.MetadataService.PutMetadataAction
 import cromwell.services.metadata.{MetadataEvent, MetadataKey, MetadataValue}
-import wom.callable.TaskDefinition
+import mouse.all._
+import wom.expression.WomExpression
 import wom.graph.TaskCallNode
 import wom.types.WomType
 import wom.values.WomValue
 
 import scala.concurrent.Future
-import scala.util.{Failure, Success, Try}
+import scala.util.Try
 
 object BackendWorkflowInitializationActor {
 
@@ -32,6 +36,66 @@ object BackendWorkflowInitializationActor {
   case class InitializationSuccess(backendInitializationData: Option[BackendInitializationData]) extends InitializationResponse
   case class InitializationFailed(reason: Throwable) extends Exception with InitializationResponse
 
+  /**
+    * This method calls into `runtimeAttributeValidators` to validate runtime attribute expressions.
+    * The initialization-time validators try to fully `evaluate` (not `evaluateType`) the runtime attribute expression.
+    * Because these validators do full evaluations but at initialization time we don't have the actual call inputs, it's
+    * possible a legitimate runtime attribute expression evaluation could fail to evaluate.  These validators must
+    * therefore give runtime attribute expressions which failed to evaluate the benefit of the doubt and pass them.
+    *
+    * Perhaps a better way of doing initialization-time runtime attribute validation:
+    *
+    * - Currently `default_runtime_attributes` are values and not expressions.  These should all be typechecked
+    *   irrespective of whether there is actual fallback to using their values due to missing task attributes.
+    *
+    * - It appears it should be possible to fully typecheck runtime attribute expressions at workflow initialization
+    *   time if task (not call!) declarations and inputs are considered.  We have all tasks at workflow initialization
+    *   time, and the task declarations know their types.  We won't actually have a call until all of its inputs are
+    *   available, but tasks are available in the `WdlNamespace`.  A task-aware type evaluation might look like:
+    *
+    *   {{{
+    *     val task: Task = ???
+    *     val expression: WdlExpression = ???
+    *     def lookup(name: String): WomType = task.declarations.find(_.name == name).get.womType
+    *     val expressionType = expression.evaluateType(lookup, OnlyPureFunctions)
+    *   }}}
+    *
+    * - Assuming the previous two checks pass, an attempt should be made to evaluate the expression.  It's possible
+    *   the expression could be a function of call inputs or declarations which are unavailable at initialization time,
+    *   so these failed evaluations must continue to be accepted as possibly correct expressions.
+    *
+    * - Assuming the expression evaluation succeeds, it should be possible to do real business logic validation.
+    *
+    * - It would be nice to memoize as much of the work that gets done here as possible so it doesn't have to all be
+    *   repeated when the various `FooRuntimeAttributes` classes are created, in the spirit of #1076.
+    */
+    def validateRuntimeAttributes(
+                                   taskName: String,
+                                   defaultRuntimeAttributes: Map[String, WomValue],
+                                   runtimeAttributes: Map[String, WomExpression],
+                                   runtimeAttributeValidators: Map[String, Option[WomExpression] => Boolean]
+                                 ): ValidatedNel[RuntimeAttributeValidationFailure, Unit] = {
+
+      //This map append will overwrite default key/values with runtime settings upon key collisions
+      val lookups = defaultRuntimeAttributes.mapValues(_.asWomExpression) ++ runtimeAttributes
+
+      runtimeAttributeValidators.toList.traverse[WishKindProjectorWorked, Unit]{
+        case (attributeName, validator) =>
+          val runtimeAttributeValue: Option[WomExpression] = lookups.get(attributeName)
+          validator(runtimeAttributeValue).fold(
+            validNel(()),
+            Invalid(NonEmptyList.of(RuntimeAttributeValidationFailure(taskName, attributeName, runtimeAttributeValue)))
+          )
+      }.map(_ => ())
+  }
+
+  //This exists because when a type (in this case Validated[A,B]) has 2 type parameters it can't figure
+  //out how to form a shape F[X], where X could be either A or B.  In this case we want an Applicative
+  //for ValidatedNel where the error type is bound to RuntimeAttributeValidationFailure.
+  //Ideally we could use Scala Compiler plugin "Kind projector" to do this inline but Scaladoc
+  //did not work with it :(.
+  private type WishKindProjectorWorked[A] = ValidatedNel[RuntimeAttributeValidationFailure, A]
+
 }
 
 /**
@@ -53,15 +117,14 @@ trait BackendWorkflowInitializationActor extends BackendWorkflowLifecycleActor w
     * declarations will fail evaluation and return `true` from this predicate, even if the type could be determined
     * to be wrong with consideration of task declarations or inputs.
     */
-  protected def wdlTypePredicate(valueRequired: Boolean, predicate: WomType => Boolean)(wdlExpressionMaybe: Option[WomValue]): Boolean = {
-    wdlExpressionMaybe match {
+  protected def womTypePredicate(valueRequired: Boolean, predicate: WomType => Boolean)(womExpressionMaybe: Option[WomExpression]): Boolean = {
+    womExpressionMaybe match {
       case None => !valueRequired
-      case Some(wdlExpression: WdlExpression) =>
-        wdlExpression.evaluate(NoLookup, PureStandardLibraryFunctions) map (_.womType) match {
-          case Success(womType) => predicate(womType)
-          case Failure(_) => true // If we can't evaluate it, we'll let it pass for now...
+      case Some(womExpression: WomExpression) =>
+        womExpression.evaluateValue(Map.empty, NoIoFunctionSet) map (_.womType) match {
+          case Valid(womType) => predicate(womType)
+          case Invalid(_) => true // If we can't evaluate it, we'll let it pass for now...
         }
-      case Some(womValue) => predicate(womValue.womType)
     }
   }
 
@@ -70,11 +133,11 @@ trait BackendWorkflowInitializationActor extends BackendWorkflowLifecycleActor w
     * between evaluation failures due to missing call inputs or evaluation failures due to malformed expressions, and will
     * return `true` in both cases.
     */
-  protected def continueOnReturnCodePredicate(valueRequired: Boolean)(wdlExpressionMaybe: Option[WomValue]): Boolean = {
-    ContinueOnReturnCodeValidation.default(configurationDescriptor.backendRuntimeConfig).validateOptionalExpression(wdlExpressionMaybe)
+  protected def continueOnReturnCodePredicate(valueRequired: Boolean)(womExpressionMaybe: Option[WomValue]): Boolean = {
+    ContinueOnReturnCodeValidation.default(configurationDescriptor.backendRuntimeConfig).validateOptionalWomValue(womExpressionMaybe)
   }
 
-  protected def runtimeAttributeValidators: Map[String, Option[WomValue] => Boolean]
+  protected def runtimeAttributeValidators: Map[String, Option[WomExpression] => Boolean]
 
   // FIXME: If a workflow executes jobs using multiple backends,
   // each backend will try to write its own workflow root and override any previous one.
@@ -85,66 +148,6 @@ trait BackendWorkflowInitializationActor extends BackendWorkflowLifecycleActor w
 
   protected def coerceDefaultRuntimeAttributes(options: WorkflowOptions): Try[Map[String, WomValue]]
 
-  /**
-    * This method calls into `runtimeAttributeValidators` to validate runtime attribute expressions.
-    * The initialization-time validators try to fully `evaluate` (not `evaluateType`) the runtime attribute expression.
-    * Because these validators do full evaluations but at initialization time we don't have the actual call inputs, it's
-    * possible a legitimate runtime attribute expression evaluation could fail to evaluate.  These validators must
-    * therefore give runtime attribute expressions which failed to evaluate the benefit of the doubt and pass them.
-    *
-    * Perhaps a better way of doing initialization-time runtime attribute validation:
-    *
-    * - Currently `default_runtime_attributes` are values and not expressions.  These should all be typechecked
-    *   irrespective of whether there is actual fallback to using their values due to missing task attributes.
-    *
-    * - It appears it should be possible to fully typecheck runtime attribute expressions at workflow initialization
-    *   time if task (not call!) declarations and inputs are considered.  We have all tasks at workflow initialization
-    *   time, and the task declarations know their types.  We won't actually have a call until all of its inputs are
-    *   available, but tasks are available in the `WdlNamespace`.  A task-aware type evaluation might look like:
-    *
-    *   {{{
-    *     val task: Task = ???
-    *     val expression: WdlExpression = ???
-    *     def lookup(name: String): WomType = task.declarations.find(_.name == name).get.womType
-    *     val expressionType = expression.evaluateType(lookup, OnlyPureFunctions)
-    *   }}}
-    *
-    * - Assuming the previous two checks pass, an attempt should be made to evaluate the expression.  It's possible
-    *   the expression could be a function of call inputs or declarations which are unavailable at initialization time,
-    *   so these failed evaluations must continue to be accepted as possibly correct expressions.
-    *
-    * - Assuming the expression evaluation succeeds, it should be possible to do real business logic validation.
-    *
-    * - It would be nice to memoize as much of the work that gets done here as possible so it doesn't have to all be
-    *   repeated when the various `FooRuntimeAttributes` classes are created, in the spirit of #1076.
-    */
-  private def validateRuntimeAttributes: Future[Unit] = {
-
-    coerceDefaultRuntimeAttributes(workflowDescriptor.workflowOptions) match {
-      case Success(_) =>
-
-//        def defaultRuntimeAttribute(name: String): Option[WomValue] = {
-//          defaultRuntimeAttributes.get(name)
-//        }
-
-        def badRuntimeAttrsForTask(task: TaskDefinition) = {
-          // TODO WOM: https://github.com/broadinstitute/cromwell/issues/2606
-//          runtimeAttributeValidators map { case (attributeName, validator) =>
-//            val value = task.runtimeAttributes.attributes.get(attributeName) orElse defaultRuntimeAttribute(attributeName)
-//            attributeName -> ((value, validator(value)))
-//          } collect {
-//            case (name, (value, false)) => RuntimeAttributeValidationFailure(task.name, name, value)
-//          }
-          Seq.empty
-        }
-
-        calls map { _.callable } flatMap badRuntimeAttrsForTask match {
-          case errors if errors.isEmpty => Future.successful(())
-          case errors => Future.failed(RuntimeAttributeValidationFailures(errors.toList))
-        }
-      case Failure(t) => Future.failed(t)
-    }
-  }
 
   def receive: Receive = LoggingReceive {
     case Initialize => performActionThenRespond(initSequence(), onFailure = InitializationFailed)
@@ -154,11 +157,17 @@ trait BackendWorkflowInitializationActor extends BackendWorkflowLifecycleActor w
   /**
     * Our predefined sequence to run during preStart
     */
-  final def initSequence(): Future[InitializationSuccess] = for {
-    _ <- validateRuntimeAttributes
-    _ <- validate()
-    data <- beforeAll()
-  } yield InitializationSuccess(data)
+  final def initSequence(): Future[InitializationSuccess] =
+    for {
+      defaultRuntimeAttributes <- coerceDefaultRuntimeAttributes(workflowDescriptor.workflowOptions) |> Future.fromTry _
+      taskList = calls.toList.map(_.callable).map(t => t.name -> t.runtimeAttributes.attributes)
+      _ <- taskList.
+            traverse[WishKindProjectorWorked, Unit]{
+              case (name, runtimeAttributes) => validateRuntimeAttributes(name, defaultRuntimeAttributes, runtimeAttributes, runtimeAttributeValidators)
+            }.toFuture(errors => RuntimeAttributeValidationFailures(errors.toList))
+      _ <- validate()
+      data <- beforeAll()
+    } yield InitializationSuccess(data)
 
   /**
     * Abort all initializations.
@@ -176,3 +185,4 @@ trait BackendWorkflowInitializationActor extends BackendWorkflowLifecycleActor w
   def validate(): Future[Unit]
 
 }
+

backend/src/main/scala/cromwell/backend/async/KnownJobFailureException.scala

@@ -2,7 +2,8 @@ package cromwell.backend.async
 
 import cromwell.core.path.Path
 import common.exception.ThrowableAggregation
-import wom.values.WomValue
+import cromwell.core.NoIoFunctionSet
+import wom.expression.WomExpression
 
 abstract class KnownJobFailureException extends Exception {
   def stderrPath: Option[Path]
@@ -29,14 +30,14 @@ final case class StderrNonEmpty(jobTag: String, stderrLength: Long, stderrPath:
 object RuntimeAttributeValidationFailure {
   def apply(jobTag: String,
             runtimeAttributeName: String,
-            runtimeAttributeValue: Option[WomValue]): RuntimeAttributeValidationFailure = RuntimeAttributeValidationFailure(jobTag, runtimeAttributeName, runtimeAttributeValue, None)
+            runtimeAttributeValue: Option[WomExpression]): RuntimeAttributeValidationFailure = RuntimeAttributeValidationFailure(jobTag, runtimeAttributeName, runtimeAttributeValue, None)
 }
 
 final case class RuntimeAttributeValidationFailure private (jobTag: String,
                                                             runtimeAttributeName: String,
-                                                            runtimeAttributeValue: Option[WomValue],
+                                                            runtimeAttributeValue: Option[WomExpression],
                                                             stderrPath: Option[Path]) extends KnownJobFailureException {
-  override def getMessage = s"Task $jobTag has an invalid runtime attribute $runtimeAttributeName = ${runtimeAttributeValue map { _.valueString} getOrElse "!! NOT FOUND !!"}"
+  override def getMessage = s"Task $jobTag has an invalid runtime attribute $runtimeAttributeName = ${runtimeAttributeValue map { _.evaluateValue(Map.empty, NoIoFunctionSet)} getOrElse "!! NOT FOUND !!"}"
 }
 
 final case class RuntimeAttributeValidationFailures(throwables: List[RuntimeAttributeValidationFailure]) extends KnownJobFailureException with ThrowableAggregation {

backend/src/main/scala/cromwell/backend/standard/StandardInitializationActor.scala

@@ -7,6 +7,7 @@ import cromwell.backend.wfs.WorkflowPathBuilder
 import cromwell.backend.{BackendConfigurationDescriptor, BackendInitializationData, BackendWorkflowDescriptor, BackendWorkflowInitializationActor}
 import cromwell.core.WorkflowOptions
 import cromwell.core.path.{DefaultPathBuilder, PathBuilder}
+import wom.expression.WomExpression
 import wom.graph.TaskCallNode
 import wom.values.WomValue
 
@@ -71,7 +72,7 @@ class StandardInitializationActor(val standardParams: StandardInitializationActo
   def runtimeAttributesBuilder: StandardValidatedRuntimeAttributesBuilder =
       StandardValidatedRuntimeAttributesBuilder.default(configurationDescriptor.backendRuntimeConfig)
 
-  override protected lazy val runtimeAttributeValidators: Map[String, (Option[WomValue]) => Boolean] = {
+  override protected lazy val runtimeAttributeValidators: Map[String, (Option[WomExpression]) => Boolean] = {
     runtimeAttributesBuilder.validatorMap
   }
 

backend/src/main/scala/cromwell/backend/validation/RuntimeAttributesValidation.scala

@@ -1,13 +1,16 @@
 package cromwell.backend.validation
 
+import cats.data.Validated.{Invalid, Valid}
 import cats.data.{NonEmptyList, Validated}
 import cats.syntax.validated._
 import com.typesafe.config.Config
 import cromwell.backend.{MemorySize, RuntimeAttributeDefinition}
 import common.validation.ErrorOr._
+import cromwell.core.NoIoFunctionSet
 import org.slf4j.Logger
 import wdl.expression.PureStandardLibraryFunctions
 import wdl.{NoLookup, WdlExpression}
+import wom.expression.WomExpression
 import wom.types._
 import wom.values._
 
@@ -312,7 +315,7 @@ trait RuntimeAttributesValidation[ValidatedType] {
   /**
     * Runs this validation on the value matching key.
     *
-    * NOTE: The values passed to this method should already be evaluated instances of WomValue, and not WdlExpression.
+    * NOTE: The values passed to this method should already be evaluated instances of WomValue, and not WomExpression.
     *
     * @param values The full set of values.
     * @return The error or valid value for this key.
@@ -341,7 +344,7 @@ trait RuntimeAttributesValidation[ValidatedType] {
     * @param wdlExpressionMaybe The optional expression.
     * @return True if the expression may be evaluated.
     */
-  def validateOptionalExpression(wdlExpressionMaybe: Option[WomValue]): Boolean = {
+  def validateOptionalWomValue(wdlExpressionMaybe: Option[WomValue]): Boolean = {
     wdlExpressionMaybe match {
       case None => staticDefaultOption.isDefined || validateNone.isValid
       case Some(wdlExpression: WdlExpression) =>
@@ -353,6 +356,16 @@ trait RuntimeAttributesValidation[ValidatedType] {
     }
   }
 
+  def validateOptionalWomExpression(womExpressionMaybe: Option[WomExpression]): Boolean = {
+    womExpressionMaybe match {
+      case None => staticDefaultOption.isDefined || validateNone.isValid
+      case Some(womExpression) =>
+        womExpression.evaluateValue(Map.empty, NoIoFunctionSet) match {
+          case Valid(womValue) => validateExpression.applyOrElse(womValue, (_: Any) => false)
+          case Invalid(_) => true // If we can't evaluate it, we'll let it pass for now...
+        }
+    }
+  }
   /**
     * Used to convert this instance to a `RuntimeAttributeDefinition`.
     *

backend/src/main/scala/cromwell/backend/validation/ValidatedRuntimeAttributesBuilder.scala

@@ -6,6 +6,7 @@ import cromwell.backend.RuntimeAttributeDefinition
 import common.exception.MessageAggregation
 import common.validation.ErrorOr._
 import org.slf4j.Logger
+import wom.expression.WomExpression
 import wom.types.WomType
 import wom.values.WomValue
 
@@ -35,9 +36,9 @@ trait ValidatedRuntimeAttributesBuilder {
   /**
     * Returns validators suitable for BackendWorkflowInitializationActor.runtimeAttributeValidators.
     */
-  final lazy val validatorMap: Map[String, (Option[WomValue]) => Boolean] = {
+  final lazy val validatorMap: Map[String, (Option[WomExpression]) => Boolean] = {
     validations.map(validation =>
-      validation.key -> validation.validateOptionalExpression _
+      validation.key -> validation.validateOptionalWomExpression _
     ).toMap
   }