diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensionRoutes.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensionRoutes.scala index 467b456eee..6079158f8a 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensionRoutes.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensionRoutes.scala @@ -249,7 +249,7 @@ trait GoogleExtensionRoutes extends ExtensionRoutes with SamUserDirectives with .synchronizeGroupMembers(policyId, samRequestContext = samRequestContext) .recover { // If the group sync was already done previously, then no need to return any sync report items, just return 200 - case _: GroupAlreadySynchronized => Map.empty[WorkbenchEmail, Seq[SyncReportItem]] + case exception: GroupAlreadySynchronized => Map(exception.group.email -> Seq.empty[SyncReportItem]) } .map { syncReport => StatusCodes.OK -> syncReport diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleGroupSynchronizer.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleGroupSynchronizer.scala index 2e865f1c79..972792b388 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleGroupSynchronizer.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleGroupSynchronizer.scala @@ -15,8 +15,10 @@ import org.broadinstitute.dsde.workbench.sam.util.OpenTelemetryIOUtils._ import org.broadinstitute.dsde.workbench.sam.util.SamRequestContext import org.broadinstitute.dsde.workbench.util.FutureSupport -class GroupAlreadySynchronized(errorReport: ErrorReport = ErrorReport(StatusCodes.Conflict, "Group has already been synchronized")) - extends WorkbenchExceptionWithErrorReport(errorReport) +case class GroupAlreadySynchronized( + group: WorkbenchGroup, + override val errorReport: ErrorReport = ErrorReport(StatusCodes.Conflict, "Group has already been synchronized") +) extends WorkbenchExceptionWithErrorReport(errorReport) /** This class makes sure that our google groups have the right members. * @@ -61,7 +63,7 @@ class GoogleGroupSynchronizer( if (group.version > group.lastSynchronizedVersion.getOrElse(0)) { IO.unit } else { - IO.raiseError(new GroupAlreadySynchronized) + IO.raiseError(new GroupAlreadySynchronized(group)) } members <- calculateAuthDomainIntersectionIfRequired(group, samRequestContext) subGroupSyncs <- syncSubGroupsIfRequired(group, visitedGroups, samRequestContext) diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensionRoutesSpec.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensionRoutesSpec.scala index 652f5ee441..52a7459da8 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensionRoutesSpec.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/google/GoogleExtensionRoutesSpec.scala @@ -286,7 +286,7 @@ class GoogleExtensionRoutesSpec extends GoogleExtensionRoutesSpecHelper with Sca } } - "GET /api/google/policy/{resourceTypeName}/{resourceId}/{accessPolicyName}/sync" should "200 with empty response when deduping sync requests" in { + "GET /api/google/policy/{resourceTypeName}/{resourceId}/{accessPolicyName}/sync" should "200 with group email when deduping sync requests" in { val resourceTypes = Map(resourceType.name -> resourceType) val (user, samDep, routes) = createTestUser(resourceTypes) @@ -297,6 +297,14 @@ class GoogleExtensionRoutesSpec extends GoogleExtensionRoutesSpecHelper with Sca } } + import spray.json.DefaultJsonProtocol._ + val createdPolicy = Get(s"/api/resource/${resourceType.name}/foo/policies") ~> routes.route ~> check { + status shouldEqual StatusCodes.OK + responseAs[Seq[AccessPolicyResponseEntry]] + .find(_.policyName == AccessPolicyName(resourceType.ownerRoleName.value)) + .getOrElse(fail("created policy not returned by get request")) + } + import spray.json.DefaultJsonProtocol._ import SamGoogleModelJsonSupport._ Post(s"/api/google/resource/${resourceType.name}/foo/${resourceType.ownerRoleName.value}/sync") ~> routes.route ~> check { @@ -307,7 +315,9 @@ class GoogleExtensionRoutesSpec extends GoogleExtensionRoutesSpecHelper with Sca // A duplicate call should return OK and an empty response Post(s"/api/google/resource/${resourceType.name}/foo/${resourceType.ownerRoleName.value}/sync") ~> routes.route ~> check { status shouldEqual StatusCodes.OK - responseAs[Map[WorkbenchEmail, Seq[SyncReportItem]]] shouldBe empty + responseAs[Map[WorkbenchEmail, Seq[SyncReportItem]]] shouldEqual Map( + createdPolicy.email -> Seq.empty + ) } }