Delete ValidateWorkflowIdInMetadata and associated

broadinstitute · Oct 24, 2024 · d45e0d3 · d45e0d3
1 parent 6e21229
commit d45e0d3
Show file tree

Hide file tree

Showing 5 changed files with 8 additions and 40 deletions.
diff --git a/engine/src/main/scala/cromwell/webservice/routes/CromwellApiService.scala b/engine/src/main/scala/cromwell/webservice/routes/CromwellApiService.scala
@@ -123,7 +123,7 @@ trait CromwellApiService
       } ~
       path("workflows" / Segment / Segment / "timing") { (_, possibleWorkflowId) =>
         instrumentRequest {
-          onComplete(validateWorkflowIdInMetadata(possibleWorkflowId, serviceRegistryActor)) {
+          onComplete(validateWorkflowIdInMetadataSummaries(possibleWorkflowId, serviceRegistryActor)) {
             case Success(workflowId) => completeTimingRouteResponse(metadataLookupForTimingRoute(workflowId))
             case Failure(e: UnrecognizedWorkflowException) => e.failRequest(StatusCodes.NotFound)
             case Failure(e: InvalidWorkflowException) => e.failRequest(StatusCodes.BadRequest)
@@ -159,7 +159,7 @@ trait CromwellApiService
       path("workflows" / Segment / Segment / "releaseHold") { (_, possibleWorkflowId) =>
         post {
           instrumentRequest {
-            val response = validateWorkflowIdInMetadata(possibleWorkflowId, serviceRegistryActor) flatMap {
+            val response = validateWorkflowIdInMetadataSummaries(possibleWorkflowId, serviceRegistryActor) flatMap {
               workflowId =>
                 workflowStoreActor
                   .ask(WorkflowStoreActor.WorkflowOnHoldToSubmittedCommand(workflowId))
@@ -319,20 +319,6 @@ object CromwellApiService {
     case e: Exception => e.errorRequest(StatusCodes.InternalServerError)
   }
 
-  def validateWorkflowIdInMetadata(possibleWorkflowId: String, serviceRegistryActor: ActorRef)(implicit
-    timeout: Timeout,
-    executor: ExecutionContext
-  ): Future[WorkflowId] =
-    Try(WorkflowId.fromString(possibleWorkflowId)) match {
-      case Success(w) =>
-        serviceRegistryActor.ask(ValidateWorkflowIdInMetadata(w)).mapTo[WorkflowValidationResponse] flatMap {
-          case RecognizedWorkflowId => Future.successful(w)
-          case UnrecognizedWorkflowId => validateWorkflowIdInMetadataSummaries(possibleWorkflowId, serviceRegistryActor)
-          case FailedToCheckWorkflowId(t) => Future.failed(t)
-        }
-      case Failure(_) => Future.failed(InvalidWorkflowException(possibleWorkflowId))
-    }
-
   def validateWorkflowIdInMetadataSummaries(possibleWorkflowId: String, serviceRegistryActor: ActorRef)(implicit
     timeout: Timeout,
     executor: ExecutionContext

diff --git a/engine/src/main/scala/cromwell/webservice/routes/MetadataRouteSupport.scala b/engine/src/main/scala/cromwell/webservice/routes/MetadataRouteSupport.scala
@@ -237,7 +237,7 @@ object MetadataRouteSupport {
         case FailedToGetArchiveStatusAndEndTime(e) => Future.failed(e)
       }
 
-    validateWorkflowIdInMetadata(possibleWorkflowId, serviceRegistryActor) flatMap { id =>
+    validateWorkflowIdInMetadataSummaries(possibleWorkflowId, serviceRegistryActor) flatMap { id =>
       /*
         for requests made to one of /metadata, /logs or /outputs endpoints, perform an additional check to see
         if metadata for the workflow has been archived and deleted or not (as they interact with metadata table)

diff --git a/engine/src/main/scala/cromwell/webservice/routes/wes/WesRouteSupport.scala b/engine/src/main/scala/cromwell/webservice/routes/wes/WesRouteSupport.scala
@@ -5,7 +5,7 @@ import akka.http.scaladsl.model.{Multipart, StatusCode, StatusCodes}
 import akka.http.scaladsl.server.Directives._
 import akka.http.scaladsl.server.directives.RouteDirectives.complete
 import akka.http.scaladsl.server.{Directive1, Route}
-import akka.pattern.{ask, AskTimeoutException}
+import akka.pattern.{AskTimeoutException, ask}
 import akka.stream.ActorMaterializer
 import akka.util.Timeout
 import cats.data.NonEmptyList
@@ -16,18 +16,12 @@ import cromwell.engine.instrumentation.HttpInstrumentation
 import cromwell.engine.workflow.WorkflowManagerActor.WorkflowNotFoundException
 import cromwell.engine.workflow.workflowstore.{WorkflowStoreActor, WorkflowStoreSubmitActor}
 import cromwell.server.CromwellShutdown
-import cromwell.services.metadata.MetadataService.{
-  BuildMetadataJsonAction,
-  GetSingleWorkflowMetadataAction,
-  GetStatus,
-  MetadataServiceResponse,
-  StatusLookupFailed
-}
+import cromwell.services.metadata.MetadataService.{BuildMetadataJsonAction, GetSingleWorkflowMetadataAction, GetStatus, MetadataServiceResponse, StatusLookupFailed}
 import cromwell.services.{FailedMetadataJsonResponse, SuccessfulMetadataJsonResponse}
 import cromwell.webservice.PartialWorkflowSources
-import cromwell.webservice.WebServiceUtils.{completeResponse, materializeFormData, EnhancedThrowable}
+import cromwell.webservice.WebServiceUtils.{EnhancedThrowable, completeResponse, materializeFormData}
 import cromwell.webservice.routes.CromwellApiService
-import cromwell.webservice.routes.CromwellApiService.{validateWorkflowIdInMetadata, UnrecognizedWorkflowException}
+import cromwell.webservice.routes.CromwellApiService.{UnrecognizedWorkflowException, validateWorkflowIdInMetadataSummaries}
 import cromwell.webservice.routes.MetadataRouteSupport.{metadataBuilderActorRequest, metadataQueryRequest}
 import cromwell.webservice.routes.wes.WesResponseJsonSupport._
 import cromwell.webservice.routes.wes.WesRouteSupport.{respondWithWesError, _}
@@ -94,7 +88,7 @@ trait WesRouteSupport extends HttpInstrumentation {
               }
             },
             path("runs" / Segment / "status") { possibleWorkflowId =>
-              val response = validateWorkflowIdInMetadata(possibleWorkflowId, serviceRegistryActor).flatMap(w =>
+              val response = validateWorkflowIdInMetadataSummaries(possibleWorkflowId, serviceRegistryActor).flatMap(w =>
                 serviceRegistryActor.ask(GetStatus(w)).mapTo[MetadataServiceResponse]
               )
               // WES can also return a 401 or a 403 but that requires user auth knowledge which Cromwell doesn't currently have

diff --git a/services/src/main/scala/cromwell/services/metadata/MetadataService.scala b/services/src/main/scala/cromwell/services/metadata/MetadataService.scala
@@ -130,7 +130,6 @@ object MetadataService {
   case object RefreshSummary extends MetadataServiceAction
   case object SendMetadataTableSizeMetrics extends MetadataServiceAction
 
-  final case class ValidateWorkflowIdInMetadata(possibleWorkflowId: WorkflowId) extends MetadataServiceAction
   final case class ValidateWorkflowIdInMetadataSummaries(possibleWorkflowId: WorkflowId) extends MetadataServiceAction
   final case class FetchWorkflowMetadataArchiveStatusAndEndTime(workflowId: WorkflowId) extends MetadataServiceAction
   final case class FetchFailedJobsMetadataWithWorkflowId(workflowId: WorkflowId) extends BuildWorkflowMetadataJsonAction

diff --git a/services/src/main/scala/cromwell/services/metadata/impl/MetadataServiceActor.scala b/services/src/main/scala/cromwell/services/metadata/impl/MetadataServiceActor.scala
@@ -194,16 +194,6 @@ case class MetadataServiceActor(serviceConfig: Config, globalConfig: Config, ser
       None
     }
 
-  private def validateWorkflowIdInMetadata(possibleWorkflowId: WorkflowId, sender: ActorRef): Unit =
-    workflowWithIdExistsInMetadata(possibleWorkflowId.toString) onComplete {
-      case Success(true) => sender ! RecognizedWorkflowId
-      case Success(false) => sender ! UnrecognizedWorkflowId
-      case Failure(e) =>
-        sender ! FailedToCheckWorkflowId(
-          new RuntimeException(s"Failed lookup attempt for workflow ID $possibleWorkflowId", e)
-        )
-    }
-
   private def validateWorkflowIdInMetadataSummaries(possibleWorkflowId: WorkflowId, sender: ActorRef): Unit =
     workflowWithIdExistsInMetadataSummaries(possibleWorkflowId.toString) onComplete {
       case Success(true) => sender ! RecognizedWorkflowId
@@ -258,7 +248,6 @@ case class MetadataServiceActor(serviceConfig: Config, globalConfig: Config, ser
     case action: PutMetadataActionAndRespond => writeActor forward action
     // Assume that listen messages are directed to the write metadata actor
     case listen: Listen => writeActor forward listen
-    case v: ValidateWorkflowIdInMetadata => validateWorkflowIdInMetadata(v.possibleWorkflowId, sender())
     case v: ValidateWorkflowIdInMetadataSummaries =>
       validateWorkflowIdInMetadataSummaries(v.possibleWorkflowId, sender())
     case g: FetchWorkflowMetadataArchiveStatusAndEndTime =>