Merge branch 'develop' into da_AJ-1326_queryTimeouts

broadinstitute · Dec 7, 2023 · b1558d3 · b1558d3
2 parents df38efc + 012bc9a
commit b1558d3
Show file tree

Hide file tree

Showing 15 changed files with 117 additions and 43 deletions.
diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs
@@ -20,3 +20,6 @@ b9cc35b06928aa1792fee88af0419d5087a7a800
 
 # Scala Steward: Reformat with scalafmt 3.7.12
 8448f2850081d53520aec809b0d678e0866a73c0
+
+# Scala Steward: Reformat with scalafmt 3.7.17
+a4933777d38af3b80fdf1d70bf1f5cac3b84ec9d
diff --git a/.scalafmt.conf b/.scalafmt.conf
@@ -1,4 +1,4 @@
-version = 3.7.12
+version = 3.7.17
 align = none
 align.openParenCallSite = true
 align.openParenDefnSite = true

diff --git a/automation/project/Dependencies.scala b/automation/project/Dependencies.scala
@@ -5,7 +5,7 @@ object Dependencies {
 
   val akkaV         = "2.6.8"
   val akkaHttpV     = "10.2.0"
-  val jacksonV      = "2.15.2"
+  val jacksonV      = "2.15.3"
 
   val workbenchLibsHash = "a562dff"
   val serviceTestV = s"4.2-${workbenchLibsHash}"
@@ -28,7 +28,7 @@ object Dependencies {
   val workbenchGoogle2: ModuleID = "org.broadinstitute.dsde.workbench" %% "workbench-google2" % workbenchGoogle2V exclude ("org.slf4j", "slf4j-api")
   val workbenchServiceTest: ModuleID = "org.broadinstitute.dsde.workbench" %% "workbench-service-test" % serviceTestV % "test" classifier "tests" excludeAll(workbenchExclusions :+ rawlsModelExclusion:_*)
 
-  val workspaceManager: ModuleID = "bio.terra" % "workspace-manager-client" % "0.254.950-SNAPSHOT"
+  val workspaceManager: ModuleID = "bio.terra" % "workspace-manager-client" % "0.254.967-SNAPSHOT"
   val dataRepo: ModuleID         = "bio.terra" % "datarepo-client" % "1.41.0-SNAPSHOT"
   val dataRepoJersey : ModuleID  = "org.glassfish.jersey.inject" % "jersey-hk2" % "2.32" // scala-steward:off (must match TDR)
 

diff --git a/automation/project/build.properties b/automation/project/build.properties
@@ -1 +1 @@
-sbt.version = 1.9.4
+sbt.version = 1.9.7
diff --git a/automation/src/test/scala/org/broadinstitute/dsde/test/pipeline/PipelineInjector.scala b/automation/src/test/scala/org/broadinstitute/dsde/test/pipeline/PipelineInjector.scala
@@ -31,8 +31,8 @@ trait PipelineInjector {
           seq <- json.as[Seq[UserMetadata]]
         } yield seq
         userMetadataSeq match {
-          case Right(u)    => u
-          case Left(_)     => Seq()
+          case Right(u) => u
+          case Left(_)  => Seq()
         }
       case _ => Seq()
     }

diff --git a/automation/src/test/scala/org/broadinstitute/dsde/test/pipeline/UserMetadata.scala b/automation/src/test/scala/org/broadinstitute/dsde/test/pipeline/UserMetadata.scala
@@ -42,7 +42,7 @@ case class UserMetadata(email: String, `type`: UserType, bearer: String) {
   * Companion object containing some useful methods for UserMetadata.
   */
 object UserMetadata {
-  //implicit val userMetadataDecoder: Decoder[UserMetadata] = deriveDecoder[UserMetadata]
+  // implicit val userMetadataDecoder: Decoder[UserMetadata] = deriveDecoder[UserMetadata]
   implicit val userMetadataDecoder: Decoder[UserMetadata] = (c: HCursor) =>
     for {
       email <- c.downField("email").as[String]

diff --git a/core/src/main/scala/org/broadinstitute/dsde/rawls/dataaccess/slick/DataAccess.scala b/core/src/main/scala/org/broadinstitute/dsde/rawls/dataaccess/slick/DataAccess.scala
@@ -1,7 +1,7 @@
 package org.broadinstitute.dsde.rawls.dataaccess.slick
 
 import org.broadinstitute.dsde.rawls.entities.local.LocalEntityExpressionQueries
-import org.broadinstitute.dsde.rawls.monitor.migration.{MultiregionalBucketMigrationHistory}
+import org.broadinstitute.dsde.rawls.monitor.migration.MultiregionalBucketMigrationHistory
 import slick.jdbc.JdbcProfile
 
 import javax.naming.NameNotFoundException

diff --git a/core/src/main/scala/org/broadinstitute/dsde/rawls/monitor/AvroUpsertMonitor.scala b/core/src/main/scala/org/broadinstitute/dsde/rawls/monitor/AvroUpsertMonitor.scala
@@ -21,7 +21,6 @@ import org.broadinstitute.dsde.rawls.model.{
   ImportStatuses,
   RawlsRequestContext,
   RawlsUserEmail,
-  UserInfo,
   Workspace,
   WorkspaceName
 }
@@ -342,9 +341,11 @@ class AvroUpsertMonitorActor(val pollInterval: FiniteDuration,
                                                  Option(errMsg)
               )
           }
-        case Some(_) =>
-          logger.warn(s"Received a double message delivery for import ID [${attributes.importId}]")
-          Future.unit
+        case Some(status) =>
+          logger.warn(
+            s"Received a double message delivery for import ID [${attributes.importId}] which is already in status [$status].  Acking message."
+          )
+          acknowledgeMessage(message.ackId)
         case None =>
           publishMessageToUpdateImportStatus(attributes.importId,
                                              None,

diff --git a/core/src/main/scala/org/broadinstitute/dsde/rawls/workspace/WorkspaceService.scala b/core/src/main/scala/org/broadinstitute/dsde/rawls/workspace/WorkspaceService.scala
@@ -1128,8 +1128,8 @@ class WorkspaceService(protected val ctx: RawlsRequestContext,
             newAttrs = sourceWorkspaceContext.attributes ++ destWorkspaceRequest.attributes
             destWorkspaceContext <- traceDBIOWithParent("createNewWorkspaceContext (cloneWorkspace)", ctx) { s =>
               val forceEnhancedBucketMonitoring =
-                destWorkspaceRequest.enhancedBucketLogging.exists(identity) || sourceBucketNameOption.exists(
-                  _.startsWith(s"${config.workspaceBucketNamePrefix}-secure")
+                destWorkspaceRequest.enhancedBucketLogging.exists(identity) || sourceWorkspace.bucketName.startsWith(
+                  s"${config.workspaceBucketNamePrefix}-secure"
                 )
               createNewWorkspaceContext(
                 destWorkspaceRequest.copy(authorizationDomain = Option(newAuthDomain),

diff --git a/core/src/test/scala/org/broadinstitute/dsde/rawls/jobexec/WorkflowSubmissionSpec.scala b/core/src/test/scala/org/broadinstitute/dsde/rawls/jobexec/WorkflowSubmissionSpec.scala
@@ -400,7 +400,6 @@ class WorkflowSubmissionSpec(_system: ActorSystem)
     }
   }
 
-
   it should "submit a workflow with the right zones for a regional bucket" in withDefaultTestDatabase {
     val mockExecCluster = MockShardedExecutionServiceCluster.fromDAO(new MockExecutionServiceDAO(), slickDataSource)
     val workflowSubmission = new TestWorkflowSubmission(

diff --git a/core/src/test/scala/org/broadinstitute/dsde/rawls/monitor/AvroUpsertMonitorSpec.scala b/core/src/test/scala/org/broadinstitute/dsde/rawls/monitor/AvroUpsertMonitorSpec.scala
@@ -4,7 +4,8 @@ import akka.actor.ActorSystem
 import akka.testkit.TestKit
 import cats.effect.unsafe.implicits.global
 import org.broadinstitute.dsde.rawls.dataaccess._
-import slick.TestDriverComponent
+import org.broadinstitute.dsde.rawls.dataaccess.slick.TestDriverComponent
+import org.broadinstitute.dsde.rawls.entities.EntityService
 import org.broadinstitute.dsde.rawls.google.GooglePubSubDAO.MessageRequest
 import org.broadinstitute.dsde.rawls.google.MockGooglePubSubDAO
 import org.broadinstitute.dsde.rawls.model.AttributeUpdateOperations.{
@@ -40,7 +41,6 @@ import org.scalatest.concurrent.PatienceConfiguration.{Interval, Timeout}
 import org.scalatest.flatspec.AnyFlatSpecLike
 import org.scalatest.matchers.should.Matchers
 import org.scalatestplus.mockito.MockitoSugar
-import org.broadinstitute.dsde.rawls.entities.EntityService
 
 import java.util.UUID
 import java.util.concurrent.TimeUnit
@@ -162,12 +162,10 @@ class AvroUpsertMonitorSpec(_system: ActorSystem)
     mockImportServiceDAO
   }
 
-  def setUpMockImportService(services: TestApiService) = {
+  def setUpMockImportService(services: TestApiService): ImportServiceDAO = {
     setUpPubSub(services)
 
     val mockImportServiceDAO = mock[ImportServiceDAO]
-    when(mockImportServiceDAO.getImportStatus(failImportStatusUUID, workspaceName, userInfo))
-      .thenReturn(Future.failed(new Exception("User not found")))
 
     // Start the monitor
     system.actorOf(
@@ -786,8 +784,9 @@ class AvroUpsertMonitorSpec(_system: ActorSystem)
     val timeout = 30000 milliseconds
     val interval = 250 milliseconds
 
-    // MockImportService should throw an error when getting import status
-    setUpMockImportService(services)
+    val mockImportServiceDAO = setUpMockImportService(services)
+    when(mockImportServiceDAO.getImportStatus(any[UUID], any[WorkspaceName], any[UserInfo]))
+      .thenReturn(Future.failed(new Exception("User not found")))
 
     val contents = makeOpsJsonString(100)
 
@@ -830,6 +829,46 @@ class AvroUpsertMonitorSpec(_system: ActorSystem)
 
   }
 
+  it should "ack pubsub message if message is double delivered" in withTestDataApiServices { services =>
+    val timeout = 30000 milliseconds
+    val interval = 250 milliseconds
+    val importUuid = UUID.randomUUID()
+
+    val mockImportServiceDAO = setUpMockImportService(services)
+    when(
+      mockImportServiceDAO.getImportStatus(any[UUID], any[WorkspaceName], any[UserInfo])
+    ).thenReturn(Future.successful(Some(ImportStatuses.Done)))
+
+    val contents = makeOpsJsonString(100)
+
+    // Store upsert json file
+    Await.result(
+      googleStorage
+        .createBlob(bucketName, GcsBlobName(importUuid.toString), contents.getBytes())
+        .compile
+        .drain
+        .unsafeToFuture(),
+      Duration.apply(10, TimeUnit.SECONDS)
+    )
+
+    // acks should be empty at this point
+    eventually(Timeout(scaled(timeout)), Interval(scaled(interval))) {
+      services.gpsDAO.acks shouldBe empty
+    }
+
+    // Publish message on the request topic
+    services.gpsDAO.publishMessages(
+      importReadPubSubTopic,
+      List(MessageRequest(importUuid.toString, testAttributes(importUuid)))
+    )
+
+    // check that a pubsub message was acked.
+    eventually(Timeout(scaled(timeout)), Interval(scaled(interval))) {
+      services.gpsDAO.acks should not be empty
+    }
+
+  }
+
   it should "update import status to error if upsert published to nonexistent workspace" in withTestDataApiServices {
     services =>
       val timeout = 30000 milliseconds
@@ -938,8 +977,14 @@ class AvroUpsertMonitorSpec(_system: ActorSystem)
       val timeout = 30000 milliseconds
       val interval = 250 milliseconds
 
-      // MockImportService should throw an error when getting import status
-      setUpMockImportService(services)
+      {
+        val mockImportServiceDAO = setUpMockImportService(services)
+
+        when(mockImportServiceDAO.getImportStatus(any[UUID], any[WorkspaceName], any[UserInfo]))
+          .thenReturn(Future.failed(new Exception("User not found")))
+
+        mockImportServiceDAO
+      }
 
       val contents = makeOpsJsonString(100)
 

diff --git a/core/src/test/scala/org/broadinstitute/dsde/rawls/workspace/WorkspaceServiceSpec.scala b/core/src/test/scala/org/broadinstitute/dsde/rawls/workspace/WorkspaceServiceSpec.scala
@@ -2721,6 +2721,32 @@ class WorkspaceServiceSpec
       workspace.bucketName should startWith(s"${services.workspaceServiceConfig.workspaceBucketNamePrefix}-secure")
   }
 
+  it should "clone a workspace with an enhanced bucket monitoring, resulting in the child workspace having enhanced logging even if the destination bucket location is defined" in withTestDataServices {
+    services =>
+      val baseWorkspaceName = "secure_space_for_workin"
+      val baseWorkspaceRequest = WorkspaceRequest(
+        testData.testProject1Name.value,
+        baseWorkspaceName,
+        Map.empty,
+        enhancedBucketLogging = Some(true)
+      )
+      val baseWorkspace = Await.result(services.workspaceService.createWorkspace(baseWorkspaceRequest), Duration.Inf)
+
+      val newWorkspaceName = "cloned_space"
+      val workspaceRequest =
+        WorkspaceRequest(testData.testProject1Name.value, newWorkspaceName, Map.empty, bucketLocation = Some("US"))
+
+      val workspace =
+        Await.result(services.mcWorkspaceService.cloneMultiCloudWorkspace(services.workspaceService,
+                                                                          baseWorkspace.toWorkspaceName,
+                                                                          workspaceRequest
+                     ),
+                     Duration.Inf
+        )
+
+      workspace.bucketName should startWith(s"${services.workspaceServiceConfig.workspaceBucketNamePrefix}-secure")
+  }
+
   it should "create a bucket with enhanced logging when told to, even if the parent workspace doesn't have it" in withTestDataServices {
     services =>
       val baseWorkspaceName = "secure_space_for_workin"

diff --git a/project/Dependencies.scala b/project/Dependencies.scala
@@ -50,7 +50,7 @@ object Dependencies {
   val googleCompute: ModuleID =           "com.google.apis"   % "google-api-services-compute"           % ("v1-rev20230119-" + googleV)
   val googlePubSub: ModuleID =            "com.google.apis"   % "google-api-services-pubsub"            % ("v1-rev20230112-" + googleV)
   val accessContextManager: ModuleID =    "com.google.apis"   % "google-api-services-accesscontextmanager" % ("v1-rev20230109-" + googleV)
-  val googleGuava: ModuleID =             "com.google.guava"  % "guava" % "32.1.2-jre"
+  val googleGuava: ModuleID =             "com.google.guava"  % "guava" % "32.1.3-jre"
 
   // metrics4-scala and metrics3-statsd are pulled in by workbench-metrics, which is pulled in by
   // workbench-google (workbenchGoogle variable in this file). Thus, anything that depends on workbench-google, such as
@@ -61,24 +61,24 @@ object Dependencies {
   val metricsStatsd: ModuleID =      "com.readytalk"         %  "metrics3-statsd"  % "4.2.0"
 
   val scalaLogging: ModuleID =    "com.typesafe.scala-logging"    %% "scala-logging"        % "3.9.5"
-  val jacksonCore: ModuleID =     "com.fasterxml.jackson.core"    % "jackson-core"          % "2.15.2"
+  val jacksonCore: ModuleID =     "com.fasterxml.jackson.core"    % "jackson-core"          % "2.15.3"
   val jodaTime: ModuleID =        "joda-time"                     % "joda-time"             % "2.12.5"
   val jodaConvert: ModuleID =     "org.joda"                      % "joda-convert"          % "2.2.3"
-  val typesafeConfig: ModuleID =  "com.typesafe"                  % "config"                % "1.4.2"
-  val sentryLogback: ModuleID =   "io.sentry"                     % "sentry-logback"        % "6.28.0"
-  val webjarsLocator: ModuleID =  "org.webjars"                   % "webjars-locator"       % "0.46"
+  val typesafeConfig: ModuleID =  "com.typesafe"                  % "config"                % "1.4.3"
+  val sentryLogback: ModuleID =   "io.sentry"                     % "sentry-logback"        % "6.34.0"
+  val webjarsLocator: ModuleID =  "org.webjars"                   % "webjars-locator"       % "0.50"
   val commonsJEXL: ModuleID =     "org.apache.commons"            % "commons-jexl"          % "2.1.1"
   val cats: ModuleID =            "org.typelevel"                 %% "cats-core"                 % "2.10.0"
   val logbackClassic: ModuleID =  "ch.qos.logback"                % "logback-classic"       % "1.4.14"
   val scalaUri: ModuleID =        "io.lemonlabs"                  %% "scala-uri"            % "3.0.0"
-  val scalatest: ModuleID =       "org.scalatest"                 %% "scalatest"            % "3.2.16" % "test"
+  val scalatest: ModuleID =       "org.scalatest"                 %% "scalatest"            % "3.2.17" % "test"
   val mockito: ModuleID =         "org.scalatestplus"             %% "mockito-4-2"          % "3.2.11.0" % Test
   val mockserverNetty: ModuleID = "org.mock-server"               % "mockserver-netty"      % "5.15.0" % "test"
   val breeze: ModuleID =          "org.scalanlp"                  %% "breeze"               % "1.2" % "test"
   val ficus: ModuleID =           "com.iheart"                    %% "ficus"                % "1.5.2"
-  val apacheCommonsIO: ModuleID = "commons-io"                    % "commons-io"            % "2.13.0"
-  val antlrParser: ModuleID =     "org.antlr"                     % "antlr4-runtime"        % "4.13.0"
-  val mysqlConnector: ModuleID =  "com.mysql"                         % "mysql-connector-j"  % "8.1.0"
+  val apacheCommonsIO: ModuleID = "commons-io"                    % "commons-io"            % "2.15.1"
+  val antlrParser: ModuleID =     "org.antlr"                     % "antlr4-runtime"        % "4.13.1"
+  val mysqlConnector: ModuleID =  "com.mysql"                         % "mysql-connector-j"  % "8.2.0"
   val liquibaseCore: ModuleID =   "org.liquibase"                 % "liquibase-core"        % "4.17.2"
 
   val workbenchLibsHash = "8ccaa6d"
@@ -104,11 +104,11 @@ object Dependencies {
   val workbenchOpenTelemetry: ModuleID = "org.broadinstitute.dsde.workbench" %% "workbench-opentelemetry" % workbenchOpenTelemetryV
   val workbenchOpenTelemetryTests: ModuleID = "org.broadinstitute.dsde.workbench" %% "workbench-opentelemetry" % workbenchOpenTelemetryV classifier "tests"
 
-  val googleStorageLocal: ModuleID = "com.google.cloud" % "google-cloud-nio" % "0.127.2" % "test"
+  val googleStorageLocal: ModuleID = "com.google.cloud" % "google-cloud-nio" % "0.127.7" % "test"
 
   val workbenchUtil: ModuleID = "org.broadinstitute.dsde.workbench" %% "workbench-util" % s"0.10-${workbenchLibsHash}"
 
-  val circeYAML: ModuleID = "io.circe" %% "circe-yaml" % "0.14.2"
+  val circeYAML: ModuleID = "io.circe" %% "circe-yaml" % "0.15.1"
 
   // should we prefer jakarta over javax.xml?
   def excludeJakartaActivationApi = ExclusionRule("jakarta.activation", "jakarta.activation-api")
@@ -126,11 +126,11 @@ object Dependencies {
   // "Terra Common Lib" Exclusions:
   def tclExclusions(m: ModuleID): ModuleID = m.excludeAll(excludeSpringBoot, excludeSpringAop, excludeSpringData, excludeSpringFramework, excludeOpenCensus, excludeGoogleFindBugs, excludeBroadWorkbench, excludePostgresql, excludeSnakeyaml, excludeSlf4j)
 
-  val workspaceManager = excludeJakarta("bio.terra" % "workspace-manager-client" % "0.254.950-SNAPSHOT")
+  val workspaceManager = excludeJakarta("bio.terra" % "workspace-manager-client" % "0.254.967-SNAPSHOT")
   val dataRepo = excludeJakarta("bio.terra" % "datarepo-client" % "1.379.0-SNAPSHOT")
   val resourceBufferService = excludeJakarta("bio.terra" % "terra-resource-buffer-client" % "0.4.3-SNAPSHOT")
-  val billingProfileManager = excludeJakarta("bio.terra" % "billing-profile-manager-client-javax" % "0.1.223-SNAPSHOT")
-  val terraCommonLib = tclExclusions(excludeJakarta("bio.terra" % "terra-common-lib" % "0.0.93-SNAPSHOT" classifier "plain"))
+  val billingProfileManager = excludeJakarta("bio.terra" % "billing-profile-manager-client-javax" % "0.1.236-SNAPSHOT")
+  val terraCommonLib = tclExclusions(excludeJakarta("bio.terra" % "terra-common-lib" % "0.0.95-SNAPSHOT" classifier "plain"))
   val sam: ModuleID = excludeJakarta("org.broadinstitute.dsde.workbench" %% "sam-client" % "0.1-d606036")
   val leonardo: ModuleID = "org.broadinstitute.dsde.workbench" % "leonardo-client_2.13" % "1.3.6-d0bf371"
 

diff --git a/project/build.properties b/project/build.properties
@@ -1 +1 @@
-sbt.version=1.9.4
+sbt.version=1.9.7
diff --git a/project/plugins.sbt b/project/plugins.sbt
@@ -1,12 +1,12 @@
-addSbtPlugin("com.eed3si9n" % "sbt-assembly" % "2.1.1")
+addSbtPlugin("com.eed3si9n" % "sbt-assembly" % "2.1.5")
 
-addSbtPlugin("org.scoverage" % "sbt-scoverage" % "2.0.7")
+addSbtPlugin("org.scoverage" % "sbt-scoverage" % "2.0.9")
 
-addSbtPlugin("org.scalameta" % "sbt-scalafmt" % "2.5.0")
+addSbtPlugin("org.scalameta" % "sbt-scalafmt" % "2.5.2")
 
 addSbtPlugin("com.simplytyped" % "sbt-antlr4" % "0.8.3")
 
-addSbtPlugin("ch.epfl.scala" % "sbt-scalafix" % "0.11.0")
+addSbtPlugin("ch.epfl.scala" % "sbt-scalafix" % "0.11.1")
 
 addSbtPlugin("io.spray" % "sbt-revolver" % "0.10.0")
-Original file line number
+Diff line change
@@ Expand Up / @@ -400,7 +400,6 @@ class WorkflowSubmissionSpec(_system: ActorSystem) @@
         }
       }
       it should "submit a workflow with the right zones for a regional bucket" in withDefaultTestDatabase {
         val mockExecCluster = MockShardedExecutionServiceCluster.fromDAO(new MockExecutionServiceDAO(), slickDataSource)
         val workflowSubmission = new TestWorkflowSubmission(
@@ Expand Down @@