Groups: Add method to read audit logs

This has the effect of breaking GetAuditLog when run under NoteDb,
rather than returning possibly inconsistent data. This will be fixed in
a followup, which is intended to be submitted at the same time.

Preserve the existing behavior of arbitrary ordering based on whatever
comes out of the database. The only downstream caller is GetAuditLog,
which needs to do its own sorting anyway (since it merges events of
different types).

The methods take a UUID instead of an ID, which requires an extra
lookup in ReviewDb. But in NoteDb the UUID is what we want, so this is
the interface that makes sense long term.

This change doesn't implement the byGroupAccount/byGroupInclude methods.
Those methods are only used by the DbGroupMemberAuditListener, which is
used to populate the audit tables in the first place. In the near future
this class will go away entirely; it only affects ReviewDb, since audit
records are stored automatically in NoteDb.

Change-Id: Idbc0cbed5517135c7705a6470d4d1c27d1bd6437

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;
   }

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;
   }

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<GroupResource> {
   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<GroupResource> {
       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<GroupResource> {
     List<GroupAuditEventInfo> 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<GroupResource> {
     }
 
     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<InternalGroup> includedGroup = groupCache.get(includedGroupUUID);
       GroupInfo member;
       if (includedGroup.isPresent()) {

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<AccountGroupMemberAudit> getMembersAudit(ReviewDb db, AccountGroup.UUID groupUuid)
+      throws OrmException {
+    if (readFromNoteDb) {
+      // TODO(dborowitz): Implement.
+      throw new OrmException("Audit logs not yet implemented in NoteDb");
+    }
+    Optional<AccountGroup> 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<AccountGroupByIdAud> getSubgroupsAudit(ReviewDb db, AccountGroup.UUID groupUuid)
+      throws OrmException {
+    if (readFromNoteDb) {
+      // TODO(dborowitz): Implement.
+      throw new OrmException("Audit logs not yet implemented in NoteDb");
+    }
+    Optional<AccountGroup> group = getGroupFromReviewDb(db, groupUuid);
+    if (!group.isPresent()) {
+      return ImmutableList.of();
+    }
+    return db.accountGroupByIdAud().byGroup(group.get().getId()).toList();
+  }
 }

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<? extends GroupAuditEventInfo> auditEvents = g.auditLog();
     assertThat(auditEvents).hasSize(1);