From d5172ea8089caa1e52122af88751192051543519 Mon Sep 17 00:00:00 2001 From: Blake Geno Date: Mon, 16 Sep 2024 16:22:36 -0400 Subject: [PATCH] clean up --- .../webservice/SubmissionApiServiceSpec.scala | 23 +++++---- .../webservice/WorkspaceApiServiceSpec.scala | 49 +++++++++++++------ 2 files changed, 47 insertions(+), 25 deletions(-) diff --git a/core/src/test/scala/org/broadinstitute/dsde/rawls/webservice/SubmissionApiServiceSpec.scala b/core/src/test/scala/org/broadinstitute/dsde/rawls/webservice/SubmissionApiServiceSpec.scala index d8c7ebdf8c..40fe885607 100644 --- a/core/src/test/scala/org/broadinstitute/dsde/rawls/webservice/SubmissionApiServiceSpec.scala +++ b/core/src/test/scala/org/broadinstitute/dsde/rawls/webservice/SubmissionApiServiceSpec.scala @@ -1,15 +1,16 @@ package org.broadinstitute.dsde.rawls.webservice import akka.actor.{ActorRef, PoisonPill} +import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport import akka.http.scaladsl.model._ import akka.http.scaladsl.model.headers.OAuth2BearerToken import akka.http.scaladsl.server.Route import akka.http.scaladsl.server.Route.{seal => sealRoute} -import akka.http.scaladsl.testkit.RouteTestTimeout +import akka.http.scaladsl.testkit.{RouteTestTimeout, ScalatestRouteTest} import akka.testkit.TestProbe import io.opentelemetry.context.Context import org.apache.commons.lang3.RandomStringUtils -import org.broadinstitute.dsde.rawls.WorkspaceAccessDeniedException +import org.broadinstitute.dsde.rawls.{TestExecutionContext, WorkspaceAccessDeniedException} import org.broadinstitute.dsde.rawls.dataaccess._ import org.broadinstitute.dsde.rawls.dataaccess.slick.{ReadWriteAction, TestData, WorkflowAuditStatusRecord} import org.broadinstitute.dsde.rawls.google.MockGooglePubSubDAO @@ -20,9 +21,12 @@ import org.broadinstitute.dsde.rawls.model.WorkspaceJsonSupport._ import org.broadinstitute.dsde.rawls.model._ import org.broadinstitute.dsde.rawls.openam.MockUserInfoDirectives import org.broadinstitute.dsde.rawls.submissions.SubmissionsService +import org.broadinstitute.dsde.rawls.util.MockitoTestUtils import org.broadinstitute.dsde.workbench.model.WorkbenchEmail import org.joda.time.DateTime -import org.mockito.Mockito.when +import org.mockito.Mockito.{verify, when} +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers import org.scalatest.prop.TableDrivenPropertyChecks import org.scalatest.time.{Seconds, Span} import spray.json.DefaultJsonProtocol._ @@ -1458,7 +1462,7 @@ class SubmissionApiServiceSpec extends ApiServiceSpec with TableDrivenPropertyCh List("userComment" -> "user comment updated".toJson): _* ) ) ~> - service.baseApiRoutes(Context.root()) ~> + service.testRoutes ~> check { status should be(StatusCodes.NoContent) } @@ -1478,14 +1482,14 @@ class SubmissionApiServiceSpec extends ApiServiceSpec with TableDrivenPropertyCh List("userComment" -> "user comment updated".toJson): _* ) ) ~> - service.baseApiRoutes(Context.root()) ~> + service.testRoutes ~> check { status should be(StatusCodes.NotFound) } } it should "return 403 when access is denied" in { - val wsName = testData.wsName + val wsName = WorkspaceName("ns", "n") val submissionId = UUID.randomUUID().toString val submissionsService = mock[SubmissionsService] val update = UserCommentUpdateOperation("user comment updated") @@ -1498,12 +1502,13 @@ class SubmissionApiServiceSpec extends ApiServiceSpec with TableDrivenPropertyCh List("userComment" -> "user comment updated".toJson): _* ) ) ~> - service.baseApiRoutes(Context.root()) ~> - check { testResult: RouteTestResult => + service.testRoutes ~> + check { val response = responseAs[String] status should be(StatusCodes.Forbidden) - response should include("insufficient permissions to perform operation on myNamespace/myWorkspace") + response should include("insufficient permissions to perform operation on") } + verify(submissionsService).updateSubmissionUserComment(wsName, submissionId, update) } } diff --git a/core/src/test/scala/org/broadinstitute/dsde/rawls/webservice/WorkspaceApiServiceSpec.scala b/core/src/test/scala/org/broadinstitute/dsde/rawls/webservice/WorkspaceApiServiceSpec.scala index 2a5484fa0e..1b2c902bb5 100644 --- a/core/src/test/scala/org/broadinstitute/dsde/rawls/webservice/WorkspaceApiServiceSpec.scala +++ b/core/src/test/scala/org/broadinstitute/dsde/rawls/webservice/WorkspaceApiServiceSpec.scala @@ -5,6 +5,7 @@ import akka.http.scaladsl.model.{StatusCodes, _} import akka.http.scaladsl.model.headers._ import akka.http.scaladsl.testkit.ScalatestRouteTest import bio.terra.workspace.model.{ErrorReport => _} +import io.opentelemetry.context.Context import org.broadinstitute.dsde.rawls.{RawlsException, RawlsExceptionWithErrorReport, TestExecutionContext} import org.broadinstitute.dsde.rawls.model.Attributable.AttributeMap import org.broadinstitute.dsde.rawls.model.AttributeUpdateOperations._ @@ -109,14 +110,13 @@ class WorkspaceApiServiceSpec val workspaceService = mock[WorkspaceService] val mcWorkspaceService = mock[MultiCloudWorkspaceService] val workspace = testData.workspace - val newWorkspace = WorkspaceRequest( namespace = workspace.namespace, name = workspace.name, Map.empty ) val details = WorkspaceDetails(workspace, Set()) - when(mcWorkspaceService.createMultiCloudOrRawlsWorkspace(any, any, any)) + when(mcWorkspaceService.createMultiCloudOrRawlsWorkspace(newWorkspace, workspaceService)) .thenReturn(Future.successful(details)) val service = new MockApiService( workspaceServiceConstructor = _ => workspaceService, @@ -129,12 +129,7 @@ class WorkspaceApiServiceSpec val response = responseAs[WorkspaceDetails] response.name shouldBe workspace.name } - verify(mcWorkspaceService).createMultiCloudOrRawlsWorkspace( - newWorkspace, - workspaceService, - null - // FIXME: should be: RawlsRequestContext(service.userInfo, Some(Context.root())) - ) + verify(mcWorkspaceService).createMultiCloudOrRawlsWorkspace(newWorkspace, workspaceService, null) } private val tagsTestParameters = Table[String, Option[String], Option[Int]]( @@ -173,20 +168,20 @@ class WorkspaceApiServiceSpec val responseWorkspace = WorkspaceResponse(None, None, None, None, details, None, None, None, None, None) val response: JsObject = responseWorkspace.toJson.asJsObject val workspaceService = mock[WorkspaceService] - when(workspaceService.getWorkspaceById(any, any, any)).thenReturn(Future.successful(response)) + val params = WorkspaceFieldSpecs(Some(Set("a", "b", "c"))) + when(workspaceService.getWorkspaceById(workspace.workspaceId, params)).thenReturn(Future.successful(response)) val service = new MockApiService( workspaceServiceConstructor = _ => workspaceService, multiCloudWorkspaceServiceConstructor = _ => mcWorkspaceService ) - Get(s"/workspaces/id/${workspace.workspaceId}") ~> + Get(s"/workspaces/id/${workspace.workspaceId}?fields=a,b,c") ~> service.testRoutes ~> check { status shouldBe StatusCodes.OK val resp = responseAs[WorkspaceResponse] resp shouldBe responseWorkspace } - // TODO: be more specific - verify(workspaceService).getWorkspaceById(any, any, any) + verify(workspaceService).getWorkspaceById(workspace.workspaceId, params, null) } it should "get a workspace by name and namespace from the workspace service" in { @@ -196,7 +191,8 @@ class WorkspaceApiServiceSpec val responseWorkspace = WorkspaceResponse(None, None, None, None, details, None, None, None, None, None) val response: JsObject = responseWorkspace.toJson.asJsObject val workspaceService = mock[WorkspaceService] - when(workspaceService.getWorkspace(any, any, any)).thenReturn(Future.successful(response)) + when(workspaceService.getWorkspace(workspace.toWorkspaceName, WorkspaceFieldSpecs(None))) + .thenReturn(Future.successful(response)) val service = new MockApiService( workspaceServiceConstructor = _ => workspaceService, multiCloudWorkspaceServiceConstructor = _ => mcWorkspaceService @@ -208,9 +204,30 @@ class WorkspaceApiServiceSpec val resp = responseAs[WorkspaceResponse] resp shouldBe responseWorkspace } - // TODO: be more specific - verify(workspaceService).getWorkspace(ArgumentMatchers.eq(workspace.toWorkspaceName), any, any) - // todo: also test with params (fields) - WorkspaceFieldSpecs.fromQueryParams(allParams, "fields") + verify(workspaceService).getWorkspace(workspace.toWorkspaceName, WorkspaceFieldSpecs(None), null) + } + + it should "pass the fields parameter when getting a workspace by name and namespace" in { + val mcWorkspaceService = mock[MultiCloudWorkspaceService] + val workspace = testData.workspace + val details = WorkspaceDetails(workspace, Set()) + val responseWorkspace = WorkspaceResponse(None, None, None, None, details, None, None, None, None, None) + val response: JsObject = responseWorkspace.toJson.asJsObject + val workspaceService = mock[WorkspaceService] + val params = WorkspaceFieldSpecs(Some(Set("a", "b", "c"))) + when(workspaceService.getWorkspace(workspace.toWorkspaceName, params)).thenReturn(Future.successful(response)) + val service = new MockApiService( + workspaceServiceConstructor = _ => workspaceService, + multiCloudWorkspaceServiceConstructor = _ => mcWorkspaceService + ) + Get(s"/workspaces/${workspace.namespace}/${workspace.name}?fields=a,b,c") ~> + service.testRoutes ~> + check { + status shouldBe StatusCodes.OK + val resp = responseAs[WorkspaceResponse] + resp shouldBe responseWorkspace + } + verify(workspaceService).getWorkspace(workspace.toWorkspaceName, params, null) } it should "update the workspace by name and namespace" in {