AJ-1749: respect importService.enabled for listing jobs (#1361)

broadinstitute · May 17, 2024 · 8eb272a · 8eb272a
1 parent 875bbb8
commit 8eb272a
Show file tree

Hide file tree

Showing 7 changed files with 49 additions and 4 deletions.
diff --git a/src/main/scala/org/broadinstitute/dsde/firecloud/Boot.scala b/src/main/scala/org/broadinstitute/dsde/firecloud/Boot.scala
@@ -98,7 +98,7 @@ object Boot extends App with LazyLogging {
     // can be disabled
     val agoraDAO: AgoraDAO = whenEnabled[AgoraDAO](FireCloudConfig.Agora.enabled, new HttpAgoraDAO(FireCloudConfig.Agora))
     val googleServicesDAO: GoogleServicesDAO = whenEnabled[GoogleServicesDAO](FireCloudConfig.GoogleCloud.enabled, new HttpGoogleServicesDAO(FireCloudConfig.GoogleCloud.priceListUrl, GooglePriceList(GooglePrices(FireCloudConfig.GoogleCloud.defaultStoragePriceList, UsTieredPriceItem(FireCloudConfig.GoogleCloud.defaultEgressPriceList)), "v1", "1")))
-    val importServiceDAO: ImportServiceDAO = whenEnabled[ImportServiceDAO](FireCloudConfig.ImportService.enabled, new HttpImportServiceDAO)
+    val importServiceDAO: ImportServiceDAO = whenEnabled[ImportServiceDAO](FireCloudConfig.ImportService.enabled, new HttpImportServiceDAO(FireCloudConfig.ImportService.enabled))
     val shibbolethDAO: ShibbolethDAO = whenEnabled[ShibbolethDAO](FireCloudConfig.Shibboleth.enabled, new HttpShibbolethDAO)
     val cwdsDAO: CwdsDAO = whenEnabled[CwdsDAO](FireCloudConfig.Cwds.enabled, new HttpCwdsDAO(FireCloudConfig.Cwds.enabled, FireCloudConfig.Cwds.supportedFormats))
 

diff --git a/src/main/scala/org/broadinstitute/dsde/firecloud/EntityService.scala b/src/main/scala/org/broadinstitute/dsde/firecloud/EntityService.scala
@@ -287,7 +287,11 @@ class EntityService(rawlsDAO: RawlsDAO, importServiceDAO: ImportServiceDAO, cwds
 
     for {
       // get jobs from Import Service
-      importServiceJobs <- importServiceDAO.listJobs(workspaceNamespace, workspaceName, runningOnly)(userInfo)
+      importServiceJobs <- if (importServiceDAO.isEnabled) {
+        importServiceDAO.listJobs(workspaceNamespace, workspaceName, runningOnly)(userInfo)
+      } else {
+        Future.successful(List.empty)
+      }
       // get jobs from cWDS
       cwdsJobs <- if (cwdsDAO.isEnabled) {
         rawlsDAO.getWorkspace(workspaceNamespace, workspaceName)(userInfo) map { workspace =>

diff --git a/src/main/scala/org/broadinstitute/dsde/firecloud/dataaccess/HttpImportServiceDAO.scala b/src/main/scala/org/broadinstitute/dsde/firecloud/dataaccess/HttpImportServiceDAO.scala
@@ -18,11 +18,13 @@ import spray.json.DefaultJsonProtocol._
 import scala.concurrent.{ExecutionContext, Future}
 import scala.concurrent.duration._
 
-class HttpImportServiceDAO(implicit val system: ActorSystem, implicit val materializer: Materializer, implicit val executionContext: ExecutionContext)
+class HttpImportServiceDAO(enabled: Boolean = true)(implicit val system: ActorSystem, implicit val materializer: Materializer, implicit val executionContext: ExecutionContext)
   extends ImportServiceDAO with RestJsonClient with SprayJsonSupport {
 
   implicit val errorReportSource: ErrorReportSource = ErrorReportSource("FireCloud")
 
+  override def isEnabled: Boolean = enabled
+
   override def importJob(workspaceNamespace: String, workspaceName: String, importRequest: AsyncImportRequest, isUpsert: Boolean)(implicit userInfo: UserInfo): Future[PerRequestMessage] = {
     doImport(workspaceNamespace, workspaceName, isUpsert, importRequest)
   }
@@ -105,5 +107,4 @@ class HttpImportServiceDAO(implicit val system: ActorSystem, implicit val materi
     }
   }
 
-
 }
diff --git a/src/main/scala/org/broadinstitute/dsde/firecloud/dataaccess/ImportServiceDAO.scala b/src/main/scala/org/broadinstitute/dsde/firecloud/dataaccess/ImportServiceDAO.scala
@@ -13,6 +13,8 @@ object ImportServiceFiletypes {
 
 trait ImportServiceDAO {
 
+  def isEnabled: Boolean
+
   def importJob(workspaceNamespace: String,
                 workspaceName: String,
                 importRequest: AsyncImportRequest,

diff --git a/src/test/scala/org/broadinstitute/dsde/firecloud/EntityServiceSpec.scala b/src/test/scala/org/broadinstitute/dsde/firecloud/EntityServiceSpec.scala
@@ -238,6 +238,7 @@ class EntityServiceSpec extends BaseServiceSpec with BeforeAndAfterEach {
       )
 
       when(importServiceDAO.listJobs(any[String], any[String], any[Boolean])(any[UserInfo])).thenReturn(Future.successful(importServiceResponse))
+      when(importServiceDAO.isEnabled).thenReturn(true)
       when(cwdsDAO.listJobsV1(any[String], any[Boolean])(any[UserInfo])).thenReturn(cwdsResponse)
       when(cwdsDAO.isEnabled).thenReturn(false)
 
@@ -256,12 +257,46 @@ class EntityServiceSpec extends BaseServiceSpec with BeforeAndAfterEach {
       actual should contain theSameElementsAs importServiceResponse
     }
 
+    "should not call Import Service if Import Service is not enabled" in {
+      // set up mocks
+      val importServiceDAO = mockito[MockImportServiceDAO]
+      val cwdsDAO = mockito[MockCwdsDAO]
+      val rawlsDAO = mockito[MockRawlsDAO]
+
+      val importServiceResponse = List(
+        ImportServiceListResponse("jobId1", "status1", "filetype1", None),
+        ImportServiceListResponse("jobId2", "status2", "filetype2", None)
+      )
+      val cwdsResponse = List(
+        ImportServiceListResponse("jobId3", "status3", "filetype3", None),
+        ImportServiceListResponse("jobId4", "status4", "filetype4", None)
+      )
+
+      when(importServiceDAO.listJobs(any[String], any[String], any[Boolean])(any[UserInfo])).thenReturn(Future.successful(importServiceResponse))
+      when(importServiceDAO.isEnabled).thenReturn(false)
+      when(cwdsDAO.listJobsV1(any[String], any[Boolean])(any[UserInfo])).thenReturn(cwdsResponse)
+      when(cwdsDAO.isEnabled).thenReturn(true)
+
+      // inject mocks to entity service
+      val entityService = getEntityService(mockImportServiceDAO = importServiceDAO, cwdsDAO = cwdsDAO)
+
+      // list jobs via entity service
+      val actual = entityService.listJobs("workspaceNamespace", "workspaceName", runningOnly = true, dummyUserInfo("mytoken")).futureValue
+
+      // verify Import Service list-jobs was NOT called
+      verify(importServiceDAO, never).listJobs(any[String], any[String], any[Boolean])(any[UserInfo])
+
+      // verify the response only contains cWDS jobs
+      actual should contain theSameElementsAs cwdsResponse
+    }
+
     def listJobsTestImpl(importServiceResponse: List[ImportServiceListResponse], cwdsResponse: List[ImportServiceListResponse]) = {
       // set up mocks
       val importServiceDAO = mockito[MockImportServiceDAO]
       val cwdsDAO = mockito[MockCwdsDAO]
 
       when(importServiceDAO.listJobs(any[String], any[String], any[Boolean])(any[UserInfo])).thenReturn(Future.successful(importServiceResponse))
+      when(importServiceDAO.isEnabled).thenReturn(true)
       when(cwdsDAO.listJobsV1(any[String], any[Boolean])(any[UserInfo])).thenReturn(cwdsResponse)
       when(cwdsDAO.isEnabled).thenReturn(true)
 

diff --git a/src/test/scala/org/broadinstitute/dsde/firecloud/dataaccess/MockImportServiceDAO.scala b/src/test/scala/org/broadinstitute/dsde/firecloud/dataaccess/MockImportServiceDAO.scala
@@ -49,4 +49,6 @@ class MockImportServiceDAO extends ImportServiceDAO {
   : Future[ImportServiceListResponse] = {
     Future.successful(ImportServiceListResponse(jobId, "status", "filetype", None))
   }
+
+  override def isEnabled: Boolean = true
 }
diff --git a/src/test/scala/org/broadinstitute/dsde/firecloud/webservice/WorkspaceApiServiceJobSpec.scala b/src/test/scala/org/broadinstitute/dsde/firecloud/webservice/WorkspaceApiServiceJobSpec.scala
@@ -60,6 +60,7 @@ class WorkspaceApiServiceJobSpec extends BaseServiceSpec with WorkspaceApiServic
           s"should call ImportServiceDAO.listJobs with running_only=$runningOnly" in {
             // reset mock invocation counts and configure its return value
             clearInvocations(mockitoImportServiceDAO)
+            when(mockitoImportServiceDAO.isEnabled).thenReturn(true)
             when(mockitoImportServiceDAO.listJobs(any[String], any[String], any[Boolean])(any[UserInfo])).thenReturn(
               Future.successful(importList))
             // execute the route