unit tests and fixes for what the tests found

broadinstitute · Mar 18, 2024 · 36a98e9 · 36a98e9
1 parent ac43897
commit 36a98e9
Show file tree

Hide file tree

Showing 3 changed files with 215 additions and 18 deletions.
diff --git a/src/main/scala/org/broadinstitute/dsde/firecloud/EntityService.scala b/src/main/scala/org/broadinstitute/dsde/firecloud/EntityService.scala
@@ -414,23 +414,29 @@ class EntityService(rawlsDAO: RawlsDAO, importServiceDAO: ImportServiceDAO, cwds
     // if cWDS is enabled, query cWDS for job
     val cwdsFuture: Future[ImportServiceListResponse] = if (cwdsDAO.isEnabled) {
       rawlsDAO.getWorkspace(workspaceNamespace, workspaceName)(userInfo) map { workspace =>
-        cwdsDAO.getJobV1(workspace.workspace.workspaceId, jobId)(userInfo)
+        val cwdsResponse = cwdsDAO.getJobV1(workspace.workspace.workspaceId, jobId)(userInfo)
+        logger.info(s"Found job $jobId in cWDS")
+        cwdsResponse
       }
     } else {
       Future.failed(new FireCloudExceptionWithErrorReport(ErrorReport(StatusCodes.NotFound, s"Import $jobId not found")))
     }
 
-    // if cWDS found the job, return it; else, try Import Service
-    cwdsFuture map { cwdsResponse =>
-      logger.info(s"Found job $jobId in cWDS")
-      cwdsResponse
-    } recoverWith {
-      case _ =>
-        importServiceDAO.getJob(workspaceNamespace, workspaceName, jobId)(userInfo) map { importServiceResponse =>
-          logger.info(s"Found job $jobId in Import Service")
-          importServiceResponse
+    val importServiceFuture: () => Future[ImportServiceListResponse] = () => importServiceDAO.getJob(workspaceNamespace, workspaceName, jobId)(userInfo) map { importServiceResponse =>
+      logger.info(s"Found job $jobId in Import Service")
+      importServiceResponse
+    } recover { importServiceError =>
+      logger.info(s"Job $jobId not found in either cWDS or Import Service: " + importServiceError.getClass.getName)
+      importServiceError match {
+        case fex: FireCloudExceptionWithErrorReport => throw fex
+        case   t => throw new FireCloudExceptionWithErrorReport(ErrorReport(StatusCodes.InternalServerError, t))
       }
     }
+
+    // if cWDS found the job, return it; else, try Import Service
+    cwdsFuture recoverWith {
+      case _ => importServiceFuture.apply()
+    }
   }
 
   def getEntitiesWithType(workspaceNamespace: String, workspaceName: String, userInfo: UserInfo): Future[PerRequestMessage] = {

diff --git a/src/main/scala/org/broadinstitute/dsde/firecloud/dataaccess/CwdsDAO.scala b/src/main/scala/org/broadinstitute/dsde/firecloud/dataaccess/CwdsDAO.scala
@@ -1,6 +1,7 @@
 package org.broadinstitute.dsde.firecloud.dataaccess
 
 import org.broadinstitute.dsde.firecloud.model.{AsyncImportRequest, AsyncImportResponse, ImportServiceListResponse, UserInfo}
+import org.databiosphere.workspacedata.client.ApiException
 import org.databiosphere.workspacedata.model.GenericJob
 
 trait CwdsDAO {
@@ -9,14 +10,17 @@ trait CwdsDAO {
 
   def getSupportedFormats: List[String]
 
+  @throws(classOf[ApiException])
   def listJobsV1(workspaceId: String,
                  runningOnly: Boolean
                 )(implicit userInfo: UserInfo): List[ImportServiceListResponse]
 
+  @throws(classOf[ApiException])
   def getJobV1(workspaceId: String,
                jobId: String
               )(implicit userInfo: UserInfo): ImportServiceListResponse
 
+  @throws(classOf[ApiException])
   def importV1(workspaceId: String,
                asyncImportRequest: AsyncImportRequest
               )(implicit userInfo: UserInfo): GenericJob

diff --git a/src/test/scala/org/broadinstitute/dsde/firecloud/EntityServiceSpec.scala b/src/test/scala/org/broadinstitute/dsde/firecloud/EntityServiceSpec.scala
@@ -10,14 +10,17 @@ import org.broadinstitute.dsde.firecloud.model.ModelJsonProtocol._
 import org.broadinstitute.dsde.firecloud.model.{AsyncImportRequest, EntityUpdateDefinition, FirecloudModelSchema, ImportServiceListResponse, ImportServiceResponse, ModelSchema, RequestCompleteWithErrorReport, UserInfo, WithAccessToken}
 import org.broadinstitute.dsde.firecloud.service.PerRequest.RequestComplete
 import org.broadinstitute.dsde.firecloud.service.{BaseServiceSpec, PerRequest}
-import org.broadinstitute.dsde.rawls.model.{ErrorReport, WorkspaceAccessLevels, WorkspaceBucketOptions, WorkspaceResponse, WorkspaceSubmissionStats}
+import org.broadinstitute.dsde.rawls.model.{ErrorReport, ErrorReportSource}
 import org.broadinstitute.dsde.workbench.model.google.{GcsBucketName, GcsObjectName, GcsPath}
+import org.databiosphere.workspacedata.client.ApiException
 import org.databiosphere.workspacedata.model.GenericJob
 import org.mockito.ArgumentMatchers
 import org.mockito.ArgumentMatchers.any
-import org.mockito.Mockito.{never, times, verify, when}
+import org.mockito.Mockito.{doThrow, never, times, verify, when}
 import org.parboiled.common.FileUtils
 import org.scalatest.BeforeAndAfterEach
+import org.scalatest.concurrent.ScalaFutures
+import org.scalatest.time.{Millis, Span}
 import org.scalatestplus.mockito.MockitoSugar.{mock => mockito}
 
 import java.util.UUID
@@ -26,6 +29,9 @@ import scala.concurrent.ExecutionContext.Implicits._
 import scala.concurrent.Future
 
 class EntityServiceSpec extends BaseServiceSpec with BeforeAndAfterEach {
+
+  implicit val errorReportSource: ErrorReportSource = ErrorReportSource("EntityServiceSpec")
+
   override def beforeEach(): Unit = {
     searchDao.reset()
   }
@@ -388,12 +394,193 @@ class EntityServiceSpec extends BaseServiceSpec with BeforeAndAfterEach {
   }
 
   "EntityService.getJob" - {
-    "should return correctly if job found in cWDS" is pending
-    "should return correctly if job not found in cWDS, but is found in Import Service" is pending
-    "should return correctly if cWDS errors, but is found in Import Service" is pending
-    "should return 404 if job not found in either cWDS or Import Service" is pending
-    "should return Import Service's error if job not found in cWDS and Import Service errors" is pending
-    "should return Import Service's error if cWDS errors and Import Service errors" is pending
+
+    val importServiceNotFound = new FireCloudExceptionWithErrorReport(
+      ErrorReport(
+        StatusCodes.NotFound,
+        "Import Service unit test intentional error"))
+
+    "should return correctly if job found in cWDS and not call Import Service" in {
+      val jobId = UUID.randomUUID().toString
+
+      val cwdsResponse = ImportServiceListResponse(jobId, "status1", "filename1", None)
+
+      // set up mocks
+      val cwdsDAO = mockito[MockCwdsDAO]
+      val importServiceDAO = mockito[MockImportServiceDAO]
+
+      when(cwdsDAO.isEnabled).thenReturn(true)
+      when(cwdsDAO.getJobV1(any[String], ArgumentMatchers.eq(jobId))(any[UserInfo]))
+        .thenReturn(cwdsResponse)
+      when(importServiceDAO.getJob(any[String], any[String], ArgumentMatchers.eq(jobId))(any[UserInfo]))
+        .thenReturn(Future.failed(importServiceNotFound))
+
+      // inject mocks to entity service
+      val entityService = getEntityService(mockImportServiceDAO = importServiceDAO, cwdsDAO = cwdsDAO)
+
+      // get job via entity service
+      val actual = entityService.getJob("workspaceNamespace", "workspaceName", jobId, dummyUserInfo("mytoken")).futureValue
+
+      actual shouldBe cwdsResponse
+      verify(importServiceDAO, never).getJob(any[String], any[String], any[String])(any[UserInfo])
+    }
+    "should return correctly if job not found in cWDS, but is found in Import Service" in {
+      val jobId = UUID.randomUUID().toString
+
+      val importServiceResponse = ImportServiceListResponse(jobId, "status1", "filename1", None)
+
+      // set up mocks
+      val cwdsDAO = mockito[MockCwdsDAO]
+      val importServiceDAO = mockito[MockImportServiceDAO]
+
+      when(cwdsDAO.isEnabled).thenReturn(true)
+      doThrow(new ApiException(404, "unit test intentional error"))
+        .when(cwdsDAO).getJobV1(any[String], ArgumentMatchers.eq(jobId))(any[UserInfo])
+      when(importServiceDAO.getJob(any[String], any[String], ArgumentMatchers.eq(jobId))(any[UserInfo]))
+        .thenReturn(Future.successful(importServiceResponse))
+
+      // inject mocks to entity service
+      val entityService = getEntityService(mockImportServiceDAO = importServiceDAO, cwdsDAO = cwdsDAO)
+
+      // get job via entity service
+      val actual = entityService.getJob("workspaceNamespace", "workspaceName", jobId, dummyUserInfo("mytoken")).futureValue
+
+      actual shouldBe importServiceResponse
+    }
+    "should return correctly if cWDS errors, but is found in Import Service" in {
+      val jobId = UUID.randomUUID().toString
+
+      val importServiceResponse = ImportServiceListResponse(jobId, "status1", "filename1", None)
+
+      // set up mocks
+      val cwdsDAO = mockito[MockCwdsDAO]
+      val importServiceDAO = mockito[MockImportServiceDAO]
+
+      when(cwdsDAO.isEnabled).thenReturn(true)
+      doThrow(new ApiException(500, "unit test intentional error"))
+        .when(cwdsDAO).getJobV1(any[String], ArgumentMatchers.eq(jobId))(any[UserInfo])
+      when(importServiceDAO.getJob(any[String], any[String], ArgumentMatchers.eq(jobId))(any[UserInfo]))
+        .thenReturn(Future.successful(importServiceResponse))
+
+      // inject mocks to entity service
+      val entityService = getEntityService(mockImportServiceDAO = importServiceDAO, cwdsDAO = cwdsDAO)
+
+      // get job via entity service
+      val actual = entityService.getJob("workspaceNamespace", "workspaceName", jobId, dummyUserInfo("mytoken")).futureValue
+
+      actual shouldBe importServiceResponse
+    }
+    "should return 404 if job not found in either cWDS or Import Service" in {
+      val jobId = UUID.randomUUID().toString
+
+      // set up mocks
+      val cwdsDAO = mockito[MockCwdsDAO]
+      val importServiceDAO = mockito[MockImportServiceDAO]
+
+      when(cwdsDAO.isEnabled).thenReturn(true)
+      doThrow(new ApiException(404, "cWDS unit test intentional error"))
+        .when(cwdsDAO).getJobV1(any[String], ArgumentMatchers.eq(jobId))(any[UserInfo])
+      when(importServiceDAO.getJob(any[String], any[String], ArgumentMatchers.eq(jobId))(any[UserInfo]))
+        .thenReturn(Future.failed(importServiceNotFound))
+
+      // inject mocks to entity service
+      val entityService = getEntityService(mockImportServiceDAO = importServiceDAO, cwdsDAO = cwdsDAO)
+
+      // get job via entity service
+      val getJobFuture = entityService.getJob("workspaceNamespace", "workspaceName", jobId, dummyUserInfo("mytoken"))
+
+      ScalaFutures.whenReady(getJobFuture.failed) { actual =>
+        actual shouldBe a [FireCloudExceptionWithErrorReport]
+        val fex = actual.asInstanceOf[FireCloudExceptionWithErrorReport]
+        fex.errorReport.statusCode should contain (StatusCodes.NotFound)
+      }
+    }
+    "should return Import Service's error if job not found in cWDS and Import Service errors" in {
+      val jobId = UUID.randomUUID().toString
+
+      // set up mocks
+      val cwdsDAO = mockito[MockCwdsDAO]
+      val importServiceDAO = mockito[MockImportServiceDAO]
+
+      when(cwdsDAO.isEnabled).thenReturn(true)
+      doThrow(new ApiException(404, "cWDS unit test intentional error"))
+        .when(cwdsDAO).getJobV1(any[String], ArgumentMatchers.eq(jobId))(any[UserInfo])
+      when(importServiceDAO.getJob(any[String], any[String], ArgumentMatchers.eq(jobId))(any[UserInfo]))
+        .thenReturn(Future.failed(
+          new FireCloudExceptionWithErrorReport(
+            ErrorReport(
+              StatusCodes.ImATeapot,
+              "Import Service unit test intentional error"))
+        ))
+
+      // inject mocks to entity service
+      val entityService = getEntityService(mockImportServiceDAO = importServiceDAO, cwdsDAO = cwdsDAO)
+
+      // get job via entity service
+      val getJobFuture = entityService.getJob("workspaceNamespace", "workspaceName", jobId, dummyUserInfo("mytoken"))
+
+      ScalaFutures.whenReady(getJobFuture.failed) { actual =>
+        actual shouldBe a [FireCloudExceptionWithErrorReport]
+        val fex = actual.asInstanceOf[FireCloudExceptionWithErrorReport]
+        fex.errorReport.statusCode should contain (StatusCodes.ImATeapot)
+        fex.errorReport.message shouldBe "Import Service unit test intentional error"
+      }
+    }
+    "should return Import Service's error if cWDS errors and Import Service errors" in {
+      val jobId = UUID.randomUUID().toString
+
+      // set up mocks
+      val cwdsDAO = mockito[MockCwdsDAO]
+      val importServiceDAO = mockito[MockImportServiceDAO]
+
+      when(cwdsDAO.isEnabled).thenReturn(true)
+      doThrow(new ApiException(500, "cWDS unit test intentional error"))
+        .when(cwdsDAO).getJobV1(any[String], ArgumentMatchers.eq(jobId))(any[UserInfo])
+      when(importServiceDAO.getJob(any[String], any[String], ArgumentMatchers.eq(jobId))(any[UserInfo]))
+        .thenReturn(Future.failed(
+          new FireCloudExceptionWithErrorReport(
+            ErrorReport(
+              StatusCodes.ImATeapot,
+              "Import Service unit test intentional error"))
+        ))
+
+      // inject mocks to entity service
+      val entityService = getEntityService(mockImportServiceDAO = importServiceDAO, cwdsDAO = cwdsDAO)
+
+      // get job via entity service
+      val getJobFuture = entityService.getJob("workspaceNamespace", "workspaceName", jobId, dummyUserInfo("mytoken"))
+
+      ScalaFutures.whenReady(getJobFuture.failed) { actual =>
+        actual shouldBe a [FireCloudExceptionWithErrorReport]
+        val fex = actual.asInstanceOf[FireCloudExceptionWithErrorReport]
+        fex.errorReport.statusCode should contain (StatusCodes.ImATeapot)
+        fex.errorReport.message shouldBe "Import Service unit test intentional error"
+      }
+    }
+    "should not call cWDS or Rawls if cWDS is not enabled" in {
+      val jobId = UUID.randomUUID().toString
+
+      val importServiceResponse = ImportServiceListResponse(jobId, "status1", "filename1", None)
+
+      // set up mocks
+      val cwdsDAO = mockito[MockCwdsDAO]
+      val importServiceDAO = mockito[MockImportServiceDAO]
+      val mockRawlsDAO = mockito[MockRawlsDAO]
+
+      when(cwdsDAO.isEnabled).thenReturn(false)
+      when(importServiceDAO.getJob(any[String], any[String], ArgumentMatchers.eq(jobId))(any[UserInfo]))
+        .thenReturn(Future.successful(importServiceResponse))
+
+      // inject mocks to entity service
+      val entityService = getEntityService(mockImportServiceDAO = importServiceDAO, cwdsDAO = cwdsDAO, rawlsDAO = mockRawlsDAO)
+
+      // get job via entity service
+      val actual = entityService.getJob("workspaceNamespace", "workspaceName", jobId, dummyUserInfo("mytoken")).futureValue
+
+      actual shouldBe importServiceResponse
+      verify(cwdsDAO, never).getJobV1(any[String], any[String])(any[UserInfo])
+      verify(mockRawlsDAO, never).getWorkspace(any[String], any[String])(any[UserInfo])
+    }
   }
 
   private def getEntityService(mockGoogleServicesDAO: MockGoogleServicesDAO = new MockGoogleServicesDAO,