diff --git a/java/com/google/gerrit/reviewdb/client/AccountGroupByIdAud.java b/java/com/google/gerrit/reviewdb/client/AccountGroupByIdAud.java index b058f04ed8..33955c4dc5 100644 --- a/java/com/google/gerrit/reviewdb/client/AccountGroupByIdAud.java +++ b/java/com/google/gerrit/reviewdb/client/AccountGroupByIdAud.java @@ -93,6 +93,14 @@ public final class AccountGroupByIdAud { return key; } + public AccountGroup.Id getGroupId() { + return key.getParentKey(); + } + + public AccountGroup.UUID getIncludeUUID() { + return key.getIncludeUUID(); + } + public boolean isActive() { return removedOn == null; } @@ -106,6 +114,10 @@ public final class AccountGroupByIdAud { return addedBy; } + public Timestamp getAddedOn() { + return key.getAddedOn(); + } + public Account.Id getRemovedBy() { return removedBy; } diff --git a/java/com/google/gerrit/reviewdb/client/AccountGroupMemberAudit.java b/java/com/google/gerrit/reviewdb/client/AccountGroupMemberAudit.java index 5b838ce210..9968b7dd1a 100644 --- a/java/com/google/gerrit/reviewdb/client/AccountGroupMemberAudit.java +++ b/java/com/google/gerrit/reviewdb/client/AccountGroupMemberAudit.java @@ -93,6 +93,14 @@ public final class AccountGroupMemberAudit { return key; } + public AccountGroup.Id getGroupId() { + return key.getGroupId(); + } + + public Account.Id getMemberId() { + return key.getParentKey(); + } + public boolean isActive() { return removedOn == null; } @@ -111,6 +119,10 @@ public final class AccountGroupMemberAudit { return addedBy; } + public Timestamp getAddedOn() { + return key.getAddedOn(); + } + public Account.Id getRemovedBy() { return removedBy; } diff --git a/java/com/google/gerrit/server/group/GetAuditLog.java b/java/com/google/gerrit/server/group/GetAuditLog.java index 89a0ebc650..ebada0b092 100644 --- a/java/com/google/gerrit/server/group/GetAuditLog.java +++ b/java/com/google/gerrit/server/group/GetAuditLog.java @@ -31,6 +31,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.account.AccountLoader; import com.google.gerrit.server.account.GroupBackend; import com.google.gerrit.server.account.GroupCache; +import com.google.gerrit.server.group.db.Groups; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; @@ -47,6 +48,7 @@ public class GetAuditLog implements RestReadView { private final GroupCache groupCache; private final GroupJson groupJson; private final GroupBackend groupBackend; + private final Groups groups; @Inject public GetAuditLog( @@ -54,12 +56,14 @@ public class GetAuditLog implements RestReadView { AccountLoader.Factory accountLoaderFactory, GroupCache groupCache, GroupJson groupJson, - GroupBackend groupBackend) { + GroupBackend groupBackend, + Groups groups) { this.db = db; this.accountLoaderFactory = accountLoaderFactory; this.groupCache = groupCache; this.groupJson = groupJson; this.groupBackend = groupBackend; + this.groups = groups; } @Override @@ -76,14 +80,12 @@ public class GetAuditLog implements RestReadView { List auditEvents = new ArrayList<>(); for (AccountGroupMemberAudit auditEvent : - db.get().accountGroupMembersAudit().byGroup(group.getId()).toList()) { - AccountInfo member = accountLoader.get(auditEvent.getKey().getParentKey()); + groups.getMembersAudit(db.get(), group.getGroupUUID())) { + AccountInfo member = accountLoader.get(auditEvent.getMemberId()); auditEvents.add( GroupAuditEventInfo.createAddUserEvent( - accountLoader.get(auditEvent.getAddedBy()), - auditEvent.getKey().getAddedOn(), - member)); + accountLoader.get(auditEvent.getAddedBy()), auditEvent.getAddedOn(), member)); if (!auditEvent.isActive()) { auditEvents.add( @@ -93,8 +95,8 @@ public class GetAuditLog implements RestReadView { } for (AccountGroupByIdAud auditEvent : - db.get().accountGroupByIdAud().byGroup(group.getId()).toList()) { - AccountGroup.UUID includedGroupUUID = auditEvent.getKey().getIncludeUUID(); + groups.getSubgroupsAudit(db.get(), group.getGroupUUID())) { + AccountGroup.UUID includedGroupUUID = auditEvent.getIncludeUUID(); Optional includedGroup = groupCache.get(includedGroupUUID); GroupInfo member; if (includedGroup.isPresent()) { diff --git a/java/com/google/gerrit/server/group/db/Groups.java b/java/com/google/gerrit/server/group/db/Groups.java index 53051c1842..9ea78befaf 100644 --- a/java/com/google/gerrit/server/group/db/Groups.java +++ b/java/com/google/gerrit/server/group/db/Groups.java @@ -16,6 +16,7 @@ package com.google.gerrit.server.group.db; import static com.google.common.collect.ImmutableSet.toImmutableSet; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Streams; @@ -23,7 +24,9 @@ import com.google.gerrit.common.errors.NoSuchGroupException; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountGroupById; +import com.google.gerrit.reviewdb.client.AccountGroupByIdAud; import com.google.gerrit.reviewdb.client.AccountGroupMember; +import com.google.gerrit.reviewdb.client.AccountGroupMemberAudit; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.config.GerritServerConfig; @@ -279,4 +282,46 @@ public class Groups { .distinct() .filter(groupUuid -> !AccountGroup.isInternalGroup(groupUuid)); } + + /** + * Returns the membership audit records for a given group. + * + * @param db the {@code ReviewDb} instance to use for lookups + * @param groupUuid the UUID of the group + * @return the audit records, in arbitrary order; empty if the group does not exist + * @throws OrmException if an error occurs while reading from ReviewDb + */ + public List getMembersAudit(ReviewDb db, AccountGroup.UUID groupUuid) + throws OrmException { + if (readFromNoteDb) { + // TODO(dborowitz): Implement. + throw new OrmException("Audit logs not yet implemented in NoteDb"); + } + Optional group = getGroupFromReviewDb(db, groupUuid); + if (!group.isPresent()) { + return ImmutableList.of(); + } + return db.accountGroupMembersAudit().byGroup(group.get().getId()).toList(); + } + + /** + * Returns the subgroup audit records for a given group. + * + * @param db the {@code ReviewDb} instance to use for lookups + * @param groupUuid the UUID of the group + * @return the audit records, in arbitrary order; empty if the group does not exist + * @throws OrmException if an error occurs while reading from ReviewDb + */ + public List getSubgroupsAudit(ReviewDb db, AccountGroup.UUID groupUuid) + throws OrmException { + if (readFromNoteDb) { + // TODO(dborowitz): Implement. + throw new OrmException("Audit logs not yet implemented in NoteDb"); + } + Optional group = getGroupFromReviewDb(db, groupUuid); + if (!group.isPresent()) { + return ImmutableList.of(); + } + return db.accountGroupByIdAud().byGroup(group.get().getId()).toList(); + } } diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java index 3ac975d720..d0368c5749 100644 --- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java +++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java @@ -15,6 +15,7 @@ package com.google.gerrit.acceptance.api.group; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.TruthJUnit.assume; import static com.google.gerrit.acceptance.GitUtil.fetch; import static com.google.gerrit.acceptance.api.group.GroupAssert.assertGroupInfo; import static com.google.gerrit.acceptance.rest.account.AccountAssert.assertAccountInfos; @@ -722,6 +723,7 @@ public class GroupsIT extends AbstractDaemonTest { @Test public void getAuditLog() throws Exception { + assume().that(cfg.getBoolean("user", null, "readGroupsFromNoteDb", false)).isFalse(); GroupApi g = gApi.groups().create(name("group")); List auditEvents = g.auditLog(); assertThat(auditEvents).hasSize(1);