From d45e0d332ef0d686766581007b1ab353c6527a66 Mon Sep 17 00:00:00 2001 From: Adam Nichols Date: Thu, 24 Oct 2024 09:40:30 -0400 Subject: [PATCH] Delete `ValidateWorkflowIdInMetadata` and associated --- .../webservice/routes/CromwellApiService.scala | 18 ++---------------- .../routes/MetadataRouteSupport.scala | 2 +- .../routes/wes/WesRouteSupport.scala | 16 +++++----------- .../services/metadata/MetadataService.scala | 1 - .../metadata/impl/MetadataServiceActor.scala | 11 ----------- 5 files changed, 8 insertions(+), 40 deletions(-) diff --git a/engine/src/main/scala/cromwell/webservice/routes/CromwellApiService.scala b/engine/src/main/scala/cromwell/webservice/routes/CromwellApiService.scala index d75eed4ccd8..ce44b53fc87 100644 --- 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 index ee2b4cdd483..0f87b1a2226 100644 --- 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 index bd5977ae853..ec5b5c32eb9 100644 --- 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 index 6c8bd4c7f59..6c50f7fad31 100644 --- 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 index bcf6346b601..245a61ade3d 100644 --- 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 =>