Skip to content

Commit cae696c

Browse files
authored
CORE-282 top groups (#1641)
1 parent ef29cc4 commit cae696c

File tree

10 files changed

+259
-55
lines changed

10 files changed

+259
-55
lines changed

src/main/resources/swagger/api-docs.yaml

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,10 +155,17 @@ paths:
155155
parameters:
156156
- name: email
157157
in: path
158-
description: Email address of user to check
158+
description: Email address (or user id) of user to check
159159
required: true
160160
schema:
161161
type: string
162+
- name: groupsContributingToMostMembershipsLimit
163+
in: query
164+
description: The number of groups to return that contribute to the most google group memberships. Default is 0.
165+
schema:
166+
type: integer
167+
minimum: 1
168+
maximum: 100
162169
responses:
163170
200:
164171
description: user summary information
@@ -3427,6 +3434,14 @@ paths:
34273434
- Users
34283435
summary: Gets a combined SamUser, attributes, allowances, terms of service, enterprise features state of the user, used to reduce api calls.
34293436
operationId: getSamUserCombinedState
3437+
parameters:
3438+
- name: groupsContributingToMostMembershipsLimit
3439+
in: query
3440+
description: The number of groups to return that contribute to the most google group memberships. Default is 0.
3441+
schema:
3442+
type: integer
3443+
minimum: 1
3444+
maximum: 100
34303445
responses:
34313446
200:
34323447
description: user exists
@@ -4992,6 +5007,20 @@ components:
49925007
additional details about the user, such as enterprise features they have access to example: '{ "enterpriseFeatures": ["feature1", "feature2"] }'
49935008
groupMemberhipCounts:
49945009
$ref: '#/components/schemas/GroupMembershipCounts'
5010+
groupsContributingToMostMemberships:
5011+
type: array
5012+
items:
5013+
$ref: '#/components/schemas/GroupMembershipCount'
5014+
GroupMembershipCount:
5015+
type: object
5016+
properties:
5017+
group:
5018+
description: unlikely to be used. Name of an internal sam group, not a managed-group which is what most users usually belong to, those will show up in policies
5019+
type: string
5020+
policy:
5021+
$ref: '#/components/schemas/PolicyIdentifiers'
5022+
attributedMembershipCount:
5023+
type: integer
49955024
SamUserRegistrationRequest:
49965025
type: object
49975026
properties:

src/main/scala/org/broadinstitute/dsde/workbench/sam/api/AdminRoutes.scala

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,10 @@ trait AdminRoutes extends SecurityDirectives with SamRequestContextDirectives wi
5353
requireAdminResourceAction(SamResourceActions.adminReadSummaryInformation, userType, samUser, samRequestContext) {
5454
val workbenchEmail = WorkbenchEmail(email)
5555
getWithTelemetry(samRequestContext, emailParam(workbenchEmail)) {
56-
complete {
57-
userService.getSamUserCombinedState(workbenchEmail, samRequestContext, resourceService)
56+
parameters("groupsContributingToMostMembershipsLimit".as[Int].?) { topGroupsLimit =>
57+
complete {
58+
userService.getSamUserCombinedState(workbenchEmail, topGroupsLimit.getOrElse(0), samRequestContext, resourceService)
59+
}
5860
}
5961
}
6062
}

src/main/scala/org/broadinstitute/dsde/workbench/sam/api/UserRoutesV2.scala

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,10 @@ trait UserRoutesV2 extends SamUserDirectives with SamRequestContextDirectives wi
8080
val samRequestContext = samRequestContextWithoutUser.copy(samUser = Some(samUser))
8181
pathEndOrSingleSlash {
8282
get {
83-
complete {
84-
userService.getSamUserCombinedState(samUser, samRequestContext, resourceService)
83+
parameters("groupsContributingToMostMembershipsLimit".as[Int].?) { topGroupsLimit =>
84+
complete {
85+
userService.getSamUserCombinedState(samUser, topGroupsLimit.getOrElse(0), samRequestContext, resourceService)
86+
}
8587
}
8688
}
8789
}

src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/DirectoryDAO.scala

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,4 +207,15 @@ trait DirectoryDAO {
207207
resourceTypeName: ResourceTypeName,
208208
samRequestContext: SamRequestContext
209209
): IO[Set[FullyQualifiedResourceId]]
210+
211+
/** List the top groups contributing to the most memberships for a user.
212+
* @param samUser
213+
* the user to list groups for
214+
* @param limit
215+
* the maximum number of groups to return
216+
* @param samRequestContext
217+
* @return
218+
* a map of group to membership count
219+
*/
220+
def listGroupsContributingToMostMemberships(samUser: SamUser, limit: Int, samRequestContext: SamRequestContext): IO[Map[WorkbenchGroupIdentity, Int]]
210221
}

src/main/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresDirectoryDAO.scala

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -732,6 +732,48 @@ class PostgresDirectoryDAO(protected val writeDbRef: DbReference, protected val
732732
}
733733
}
734734

735+
override def listGroupsContributingToMostMemberships(
736+
samUser: SamUser,
737+
limit: Int,
738+
samRequestContext: SamRequestContext
739+
): IO[Map[WorkbenchGroupIdentity, Int]] = if (limit <= 0) {
740+
IO.pure(Map.empty)
741+
} else {
742+
readOnlyTransaction("listGroupsContributingToMostMemberships", samRequestContext) { implicit session =>
743+
val f = GroupMemberFlatTable.syntax("f")
744+
val g = GroupTable.syntax("g")
745+
val p = PolicyTable.syntax("p")
746+
val r = ResourceTable.syntax("r")
747+
val rt = ResourceTypeTable.syntax("rt")
748+
749+
val query = samsql"""with group_counts as (
750+
select ${f.lastGroupMembershipElement} as group_id, count(${f.groupId}) AS membership_count
751+
from ${GroupMemberFlatTable as f}
752+
join ${GroupTable as g} on ${f.groupId} = ${g.id}
753+
where ${g.synchronizedDate} is not null
754+
and ${f.memberUserId} = ${samUser.id}
755+
group by ${f.lastGroupMembershipElement}
756+
order by membership_count desc
757+
limit $limit
758+
)
759+
select gc.membership_count, ${g.result.name}, ${p.result.name}, ${r.result.name}, ${rt.result.name}
760+
from group_counts gc
761+
join ${GroupTable as g} on gc.group_id = ${g.id}
762+
left join ${PolicyTable as p} on ${p.groupId} = ${g.id}
763+
left join ${ResourceTable as r} on ${p.resourceId} = ${r.id}
764+
left join ${ResourceTypeTable as rt} on ${r.resourceTypeId} = ${rt.id}
765+
"""
766+
767+
query
768+
.map { rs =>
769+
resultSetToGroupIdentity(rs, g, p, r, rt) -> rs.int("membership_count")
770+
}
771+
.list()
772+
.apply()
773+
.toMap
774+
}
775+
}
776+
735777
override def countDirectSynchronizedGroupMemberships(samUser: SamUser, samRequestContext: SamRequestContext): IO[Int] =
736778
readOnlyTransaction("countDirectSynchronizedGroupMemberships", samRequestContext) { implicit session =>
737779
val query = samsql"""select count(distinct g.id) directMembershipCount
Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package org.broadinstitute.dsde.workbench.sam.model.api
22

3-
import org.broadinstitute.dsde.workbench.sam.model.{FullyQualifiedResourceId, TermsOfServiceDetails}
3+
import org.broadinstitute.dsde.workbench.model.WorkbenchIdentityJsonSupport.WorkbenchGroupNameFormat
4+
import org.broadinstitute.dsde.workbench.model.{WorkbenchGroupIdentity, WorkbenchGroupName}
5+
import org.broadinstitute.dsde.workbench.sam.model.{FullyQualifiedPolicyId, FullyQualifiedResourceId, PolicyIdentifiers, TermsOfServiceDetails}
46
import spray.json.DefaultJsonProtocol._
57
import spray.json._
68
import org.broadinstitute.dsde.workbench.sam.model.api.GroupMembershipCounts.GroupMembershipCountsFormat
@@ -9,14 +11,35 @@ import org.broadinstitute.dsde.workbench.sam.model.api.SamUserAttributes.SamUser
911
import org.broadinstitute.dsde.workbench.sam.model.api.SamJsonSupport._
1012

1113
object SamUserCombinedStateResponse {
12-
implicit val SamUserResponseFormat: RootJsonFormat[SamUserCombinedStateResponse] = jsonFormat7(SamUserCombinedStateResponse.apply)
14+
implicit val GroupMembershipCountFormat: RootJsonFormat[GroupMembershipCount] = jsonFormat3(GroupMembershipCount.apply)
15+
implicit val SamUserResponseFormat: RootJsonFormat[SamUserCombinedStateResponse] = jsonFormat8(SamUserCombinedStateResponse.apply)
1316
}
17+
18+
final case class GroupMembershipCount(
19+
group: Option[WorkbenchGroupName],
20+
policy: Option[PolicyIdentifiers],
21+
attributedMembershipCount: Int
22+
)
23+
object GroupMembershipCount {
24+
def apply(group: WorkbenchGroupIdentity, attributedMembershipCount: Int): GroupMembershipCount =
25+
group match {
26+
case groupName: WorkbenchGroupName => GroupMembershipCount(Option(groupName), None, attributedMembershipCount)
27+
case policyId: FullyQualifiedPolicyId =>
28+
GroupMembershipCount(
29+
None,
30+
Option(PolicyIdentifiers(policyId.accessPolicyName, policyId.resource.resourceTypeName, policyId.resource.resourceId)),
31+
attributedMembershipCount
32+
)
33+
}
34+
}
35+
1436
final case class SamUserCombinedStateResponse(
1537
samUser: SamUser,
1638
allowances: SamUserAllowances,
1739
attributes: Option[SamUserAttributes],
1840
termsOfServiceDetails: TermsOfServiceDetails,
1941
groupMembershipCounts: GroupMembershipCounts,
2042
additionalDetails: Map[String, JsValue],
21-
favoriteResources: Set[FullyQualifiedResourceId]
43+
favoriteResources: Set[FullyQualifiedResourceId],
44+
groupsContributingToMostMemberships: Option[List[GroupMembershipCount]]
2245
)

src/main/scala/org/broadinstitute/dsde/workbench/sam/service/UserService.scala

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package org.broadinstitute.dsde.workbench.sam
22
package service
33

44
import akka.http.scaladsl.model.StatusCodes
5+
import cats.data.OptionT
56
import cats.effect.IO
67
import com.typesafe.scalalogging.LazyLogging
78
import org.apache.commons.codec.binary.Hex
@@ -519,24 +520,35 @@ class UserService(
519520
def countIndirectPublicGroupMemberships(samUser: SamUser, samRequestContext: SamRequestContext): IO[Int] =
520521
directoryDAO.countIndirectPublicGroupMemberships(samUser, samRequestContext)
521522

523+
def listGroupsContributingToMostMemberships(samUser: SamUser, limit: Int, samRequestContext: SamRequestContext): IO[Map[WorkbenchGroupIdentity, Int]] =
524+
directoryDAO.listGroupsContributingToMostMemberships(samUser, limit, samRequestContext)
525+
522526
def getSamUserCombinedState(
523527
workbenchEmail: WorkbenchEmail,
528+
topGroupsLimit: Int,
524529
samRequestContext: SamRequestContext,
525530
resourceService: ResourceService
526531
): IO[SamUserCombinedStateResponse] =
527532
for {
528-
samUserOption <- getUserFromEmail(workbenchEmail, samRequestContext)
529-
samUser = samUserOption.getOrElse(throw new WorkbenchException("user not found"))
530-
combinedState <- getSamUserCombinedState(samUser, samRequestContext, resourceService)
533+
samUser <- OptionT(getUserFromEmail(workbenchEmail, samRequestContext))
534+
.orElseF(directoryDAO.loadUser(WorkbenchUserId(workbenchEmail.value), samRequestContext))
535+
.getOrRaise(new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.NotFound, "user not found")))
536+
combinedState <- getSamUserCombinedState(samUser, topGroupsLimit, samRequestContext, resourceService)
531537
} yield combinedState
532538

533-
def getSamUserCombinedState(samUser: SamUser, samRequestContext: SamRequestContext, resourceService: ResourceService): IO[SamUserCombinedStateResponse] =
539+
def getSamUserCombinedState(
540+
samUser: SamUser,
541+
topGroupsLimit: Int,
542+
samRequestContext: SamRequestContext,
543+
resourceService: ResourceService
544+
): IO[SamUserCombinedStateResponse] =
534545
for {
535546
allowances <- getUserAllowances(samUser, samRequestContext)
536547
maybeAttributes <- getUserAttributes(samUser.id, samRequestContext)
537548
directGroupMemberships <- countDirectSynchronizedGroupMemberships(samUser, samRequestContext)
538549
indirectGroupMemberships <- countIndirectSynchronizedGroupMemberships(samUser, samRequestContext)
539550
indirectPublicMemberships <- countIndirectPublicGroupMemberships(samUser, samRequestContext)
551+
topGroups <- listGroupsContributingToMostMemberships(samUser, topGroupsLimit, samRequestContext)
540552
termsOfServiceDetails <- tosService.getTermsOfServiceDetailsForUser(samUser.id, samRequestContext)
541553
enterpriseFeatures <- resourceService
542554
.listResourcesFlat(
@@ -560,7 +572,11 @@ class UserService(
560572
indirectPublic = indirectPublicMemberships
561573
),
562574
Map("enterpriseFeatures" -> enterpriseFeatures.toJson),
563-
favoriteResources
575+
favoriteResources,
576+
topGroups.map { case (group, count) => GroupMembershipCount(group, count) }.toList match {
577+
case Nil => None
578+
case list => Option(list)
579+
}
564580
)
565581

566582
}

src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/MockDirectoryDAO.scala

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,4 +520,10 @@ class MockDirectoryDAO(val groups: mutable.Map[WorkbenchGroupIdentity, Workbench
520520
samRequestContext: SamRequestContext
521521
): IO[Set[FullyQualifiedResourceId]] =
522522
IO.pure(userFavoriteResources.getOrElse(userId, Set.empty).filter(_.resourceTypeName == resourceTypeName))
523+
524+
override def listGroupsContributingToMostMemberships(
525+
samUser: SamUser,
526+
limit: Int,
527+
samRequestContext: SamRequestContext
528+
): IO[Map[WorkbenchGroupIdentity, Int]] = ???
523529
}

src/test/scala/org/broadinstitute/dsde/workbench/sam/dataAccess/PostgresDirectoryDAOSpec.scala

Lines changed: 83 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,9 @@ import org.broadinstitute.dsde.workbench.sam.db.tables.TosTable
1313
import org.broadinstitute.dsde.workbench.sam.matchers.TimeMatchers
1414
import org.broadinstitute.dsde.workbench.sam.model._
1515
import org.broadinstitute.dsde.workbench.sam.model.api.{AdminUpdateUserRequest, SamUser, SamUserAttributes}
16-
import org.broadinstitute.dsde.workbench.sam.{Generator, RetryableAnyFreeSpec, TestSupport}
16+
import org.broadinstitute.dsde.workbench.sam.{Generator, TestSupport}
1717
import org.scalatest.Inside.inside
18+
import org.scalatest.freespec.AnyFreeSpec
1819
import org.scalatest.matchers.should.Matchers
1920
import org.scalatest.{BeforeAndAfterEach, OptionValues}
2021

@@ -23,7 +24,7 @@ import java.time.temporal.ChronoUnit
2324
import java.util.{Date, UUID}
2425
import scala.concurrent.duration._
2526

26-
class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with BeforeAndAfterEach with TimeMatchers with OptionValues {
27+
class PostgresDirectoryDAOSpec extends AnyFreeSpec with Matchers with BeforeAndAfterEach with TimeMatchers with OptionValues {
2728
val dao = new PostgresDirectoryDAO(TestSupport.dbRef, TestSupport.dbRef)
2829
val policyDAO = new PostgresAccessPolicyDAO(TestSupport.dbRef, TestSupport.dbRef)
2930
val azureManagedResourceGroupDAO = new PostgresAzureManagedResourceGroupDAO(TestSupport.dbRef, TestSupport.dbRef)
@@ -2222,5 +2223,85 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B
22222223
loadedFavoriteResources should contain theSameElementsAs Set(otherResource.fullyQualifiedId)
22232224
}
22242225
}
2226+
2227+
"listGroupsContributingToMostMemberships" - {
2228+
"list groups contributing to most memberships" in {
2229+
assume(databaseEnabled, databaseEnabledClue)
2230+
val otherUser = Generator.genWorkbenchUserBoth.sample.get
2231+
dao.createUser(defaultUser, samRequestContext).unsafeRunSync()
2232+
dao.createUser(otherUser, samRequestContext).unsafeRunSync()
2233+
2234+
/*
2235+
create `drop` + `limit` groups each containing defaultUser, these are leaf groups
2236+
for each leaf group, create `multiplier` * `index` parent groups, these are parent groups
2237+
the result of the list call using `limit` should exclude the lowest `drop` indexed groups
2238+
the counts for the remaining groups should be 1 (for the leaf group) + `index` * `multiplier` (for the parent groups)
2239+
*/
2240+
val multiplier = 5
2241+
val limit = 3
2242+
val drop = 3
2243+
val testGroups = for {
2244+
index <- 1 to limit + drop
2245+
} yield {
2246+
val leafUuid = UUID.randomUUID().toString
2247+
val leafGroup = BasicWorkbenchGroup(WorkbenchGroupName(leafUuid), Set(defaultUser.id), WorkbenchEmail(leafUuid))
2248+
dao.createGroup(leafGroup, samRequestContext = samRequestContext).unsafeRunSync()
2249+
dao.updateSynchronizedDateAndVersion(leafGroup, samRequestContext).unsafeRunSync()
2250+
for (_ <- 1 to multiplier * index) {
2251+
val parentUuid = UUID.randomUUID().toString
2252+
val parentGroup = BasicWorkbenchGroup(WorkbenchGroupName(parentUuid), Set(leafGroup.id), WorkbenchEmail(parentUuid))
2253+
dao.createGroup(parentGroup, samRequestContext = samRequestContext).unsafeRunSync()
2254+
dao.updateSynchronizedDateAndVersion(parentGroup, samRequestContext).unsafeRunSync()
2255+
}
2256+
(leafGroup, index)
2257+
}
2258+
2259+
dao.addGroupMember(testGroups.last._1.id, otherUser.id, samRequestContext).unsafeRunSync()
2260+
2261+
val result = dao.listGroupsContributingToMostMemberships(defaultUser, limit, samRequestContext).unsafeRunSync()
2262+
result should be(testGroups.drop(drop).map { case (group, index) => group.id -> (1 + index * multiplier) }.toMap)
2263+
2264+
// check that another user gives different results
2265+
val otherResult = dao.listGroupsContributingToMostMemberships(otherUser, limit, samRequestContext).unsafeRunSync()
2266+
otherResult should be(Map(testGroups.last._1.id -> (1 + (limit + drop) * multiplier)))
2267+
}
2268+
2269+
"list policies and groups" in {
2270+
assume(databaseEnabled, databaseEnabledClue)
2271+
dao.createUser(defaultUser, samRequestContext).unsafeRunSync()
2272+
2273+
val policy = defaultPolicy.copy(members = Set(defaultUser.id))
2274+
policyDAO.createResourceType(resourceType, samRequestContext).unsafeRunSync()
2275+
policyDAO.createResource(defaultResource, samRequestContext).unsafeRunSync()
2276+
policyDAO.createPolicy(policy, samRequestContext).unsafeRunSync()
2277+
dao.updateSynchronizedDateAndVersion(policy, samRequestContext).unsafeRunSync()
2278+
2279+
val groupUuid = UUID.randomUUID().toString
2280+
val group = BasicWorkbenchGroup(WorkbenchGroupName(groupUuid), Set(defaultUser.id), WorkbenchEmail(groupUuid))
2281+
dao.createGroup(group, samRequestContext = samRequestContext).unsafeRunSync()
2282+
dao.updateSynchronizedDateAndVersion(group, samRequestContext).unsafeRunSync()
2283+
2284+
val result = dao.listGroupsContributingToMostMemberships(defaultUser, 10, samRequestContext).unsafeRunSync()
2285+
result should be(Map(policy.id -> 1, group.id -> 1))
2286+
}
2287+
2288+
"ignores unsynchronized groups" in {
2289+
assume(databaseEnabled, databaseEnabledClue)
2290+
dao.createUser(defaultUser, samRequestContext).unsafeRunSync()
2291+
2292+
val leafUuid = UUID.randomUUID().toString
2293+
val leafGroup = BasicWorkbenchGroup(WorkbenchGroupName(leafUuid), Set(defaultUser.id), WorkbenchEmail(leafUuid))
2294+
dao.createGroup(leafGroup, samRequestContext = samRequestContext).unsafeRunSync()
2295+
dao.updateSynchronizedDateAndVersion(leafGroup, samRequestContext).unsafeRunSync()
2296+
2297+
val parentUuid = UUID.randomUUID().toString
2298+
val parentGroup = BasicWorkbenchGroup(WorkbenchGroupName(parentUuid), Set(leafGroup.id), WorkbenchEmail(parentUuid))
2299+
dao.createGroup(parentGroup, samRequestContext = samRequestContext).unsafeRunSync()
2300+
// not synchronized
2301+
2302+
val result = dao.listGroupsContributingToMostMemberships(defaultUser, 10, samRequestContext).unsafeRunSync()
2303+
result should be(Map(leafGroup.id -> 1))
2304+
}
2305+
}
22252306
}
22262307
}

0 commit comments

Comments
 (0)