From 6e2a18402c20b4fa93db970a46ab35ec777e0d18 Mon Sep 17 00:00:00 2001 From: Alice Kober-Sotzek Date: Wed, 29 Nov 2017 16:47:49 +0100 Subject: [PATCH] Use same timestamp for group audit log in ReviewDb and NoteDb Previously, we called TimeUtil.nowTs() both in the ReviewDb and in the NoteDb code. This resulted in different timestamps being used for the audit log entries of ReviewDb and NoteDb. Depending on various factors, the difference between those two timestamps could easily be more than just a second. In order to ensure comparability of the audit log of ReviewDb with the one of NoteDb, we now use the same timestamp for both. This breaks the extension point GroupMemberAuditListener but has the benefit that all receivers of audit events will record the same timestamps from now on. To avoid confusion, the field 'createdOn' for group creations is superseded by the field 'updatedOn'. Furthermore, not explicitly setting the timestamp for group creations is supported now. Except for tests and migrations, the automatically picked timestamp is sufficient. Change-Id: I55b8d10eb978ae1c34ee809aff45a9a4b3c8e255 --- .../gerrit/server/audit/AuditService.java | 19 +++--- .../audit/GroupMemberAuditListener.java | 12 ++-- .../gerrit/server/group/CreateGroup.java | 2 - .../group/DbGroupMemberAuditListener.java | 24 ++++--- .../gerrit/server/group/db/GroupConfig.java | 16 ++--- .../server/group/db/GroupRebuilder.java | 5 +- .../gerrit/server/group/db/GroupsUpdate.java | 64 +++++++++++++------ .../group/db/InternalGroupCreation.java | 5 -- .../server/group/db/InternalGroupUpdate.java | 7 ++ .../gerrit/server/schema/SchemaCreator.java | 2 - .../server/group/db/AuditLogReaderTest.java | 2 - .../server/group/db/GroupConfigTest.java | 3 +- 12 files changed, 96 insertions(+), 65 deletions(-) diff --git a/java/com/google/gerrit/server/audit/AuditService.java b/java/com/google/gerrit/server/audit/AuditService.java index b1ecb9b4f8..eb54fbc560 100644 --- a/java/com/google/gerrit/server/audit/AuditService.java +++ b/java/com/google/gerrit/server/audit/AuditService.java @@ -20,6 +20,7 @@ import com.google.gerrit.reviewdb.client.AccountGroupById; import com.google.gerrit.reviewdb.client.AccountGroupMember; import com.google.inject.Inject; import com.google.inject.Singleton; +import java.sql.Timestamp; import java.util.Collection; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -45,10 +46,11 @@ public class AuditService { } } - public void dispatchAddAccountsToGroup(Account.Id actor, Collection added) { + public void dispatchAddAccountsToGroup( + Account.Id actor, Collection added, Timestamp addedOn) { for (GroupMemberAuditListener auditListener : groupMemberAuditListeners) { try { - auditListener.onAddAccountsToGroup(actor, added); + auditListener.onAddAccountsToGroup(actor, added, addedOn); } catch (RuntimeException e) { log.error("failed to log add accounts to group event", e); } @@ -56,20 +58,21 @@ public class AuditService { } public void dispatchDeleteAccountsFromGroup( - Account.Id actor, Collection removed) { + Account.Id actor, Collection removed, Timestamp removedOn) { for (GroupMemberAuditListener auditListener : groupMemberAuditListeners) { try { - auditListener.onDeleteAccountsFromGroup(actor, removed); + auditListener.onDeleteAccountsFromGroup(actor, removed, removedOn); } catch (RuntimeException e) { log.error("failed to log delete accounts from group event", e); } } } - public void dispatchAddGroupsToGroup(Account.Id actor, Collection added) { + public void dispatchAddGroupsToGroup( + Account.Id actor, Collection added, Timestamp addedOn) { for (GroupMemberAuditListener auditListener : groupMemberAuditListeners) { try { - auditListener.onAddGroupsToGroup(actor, added); + auditListener.onAddGroupsToGroup(actor, added, addedOn); } catch (RuntimeException e) { log.error("failed to log add groups to group event", e); } @@ -77,10 +80,10 @@ public class AuditService { } public void dispatchDeleteGroupsFromGroup( - Account.Id actor, Collection removed) { + Account.Id actor, Collection removed, Timestamp removedOn) { for (GroupMemberAuditListener auditListener : groupMemberAuditListeners) { try { - auditListener.onDeleteGroupsFromGroup(actor, removed); + auditListener.onDeleteGroupsFromGroup(actor, removed, removedOn); } catch (RuntimeException e) { log.error("failed to log delete groups from group event", e); } diff --git a/java/com/google/gerrit/server/audit/GroupMemberAuditListener.java b/java/com/google/gerrit/server/audit/GroupMemberAuditListener.java index 9ac5994ea2..d820386529 100644 --- a/java/com/google/gerrit/server/audit/GroupMemberAuditListener.java +++ b/java/com/google/gerrit/server/audit/GroupMemberAuditListener.java @@ -18,16 +18,20 @@ import com.google.gerrit.extensions.annotations.ExtensionPoint; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroupById; import com.google.gerrit.reviewdb.client.AccountGroupMember; +import java.sql.Timestamp; import java.util.Collection; @ExtensionPoint public interface GroupMemberAuditListener { - void onAddAccountsToGroup(Account.Id actor, Collection added); + void onAddAccountsToGroup( + Account.Id actor, Collection added, Timestamp addedOn); - void onDeleteAccountsFromGroup(Account.Id actor, Collection removed); + void onDeleteAccountsFromGroup( + Account.Id actor, Collection removed, Timestamp removedOn); - void onAddGroupsToGroup(Account.Id actor, Collection added); + void onAddGroupsToGroup(Account.Id actor, Collection added, Timestamp addedOn); - void onDeleteGroupsFromGroup(Account.Id actor, Collection deleted); + void onDeleteGroupsFromGroup( + Account.Id actor, Collection deleted, Timestamp removedOn); } diff --git a/java/com/google/gerrit/server/group/CreateGroup.java b/java/com/google/gerrit/server/group/CreateGroup.java index 9dd7a2dd43..b1c8e9be95 100644 --- a/java/com/google/gerrit/server/group/CreateGroup.java +++ b/java/com/google/gerrit/server/group/CreateGroup.java @@ -17,7 +17,6 @@ package com.google.gerrit.server.group; import com.google.common.base.MoreObjects; import com.google.common.base.Strings; import com.google.common.collect.ImmutableSet; -import com.google.gerrit.common.TimeUtil; import com.google.gerrit.common.data.GlobalCapability; import com.google.gerrit.common.data.GroupDescription; import com.google.gerrit.extensions.annotations.RequiresCapability; @@ -206,7 +205,6 @@ public class CreateGroup implements RestModifyView .setGroupUUID(uuid) .setNameKey(createGroupArgs.getGroup()) .setId(groupId) - .setCreatedOn(TimeUtil.nowTs()) .build(); InternalGroupUpdate.Builder groupUpdateBuilder = InternalGroupUpdate.builder().setVisibleToAll(createGroupArgs.visibleToAll); diff --git a/java/com/google/gerrit/server/group/DbGroupMemberAuditListener.java b/java/com/google/gerrit/server/group/DbGroupMemberAuditListener.java index 2e203b3dac..d29f894cdb 100644 --- a/java/com/google/gerrit/server/group/DbGroupMemberAuditListener.java +++ b/java/com/google/gerrit/server/group/DbGroupMemberAuditListener.java @@ -15,7 +15,6 @@ package com.google.gerrit.server.group; import com.google.common.base.Joiner; -import com.google.gerrit.common.TimeUtil; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountGroupById; @@ -30,6 +29,7 @@ import com.google.gerrit.server.audit.GroupMemberAuditListener; import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.SchemaFactory; import com.google.inject.Inject; +import java.sql.Timestamp; import java.text.MessageFormat; import java.util.ArrayList; import java.util.Collection; @@ -58,10 +58,11 @@ class DbGroupMemberAuditListener implements GroupMemberAuditListener { } @Override - public void onAddAccountsToGroup(Account.Id me, Collection added) { + public void onAddAccountsToGroup( + Account.Id me, Collection added, Timestamp addedOn) { List auditInserts = new ArrayList<>(); for (AccountGroupMember m : added) { - AccountGroupMemberAudit audit = new AccountGroupMemberAudit(m, me, TimeUtil.nowTs()); + AccountGroupMemberAudit audit = new AccountGroupMemberAudit(m, me, addedOn); auditInserts.add(audit); } try (ReviewDb db = schema.open()) { @@ -73,7 +74,8 @@ class DbGroupMemberAuditListener implements GroupMemberAuditListener { } @Override - public void onDeleteAccountsFromGroup(Account.Id me, Collection removed) { + public void onDeleteAccountsFromGroup( + Account.Id me, Collection removed, Timestamp removedOn) { List auditInserts = new ArrayList<>(); List auditUpdates = new ArrayList<>(); try (ReviewDb db = schema.open()) { @@ -88,10 +90,10 @@ class DbGroupMemberAuditListener implements GroupMemberAuditListener { } if (audit != null) { - audit.removed(me, TimeUtil.nowTs()); + audit.removed(me, removedOn); auditUpdates.add(audit); } else { - audit = new AccountGroupMemberAudit(m, me, TimeUtil.nowTs()); + audit = new AccountGroupMemberAudit(m, me, removedOn); audit.removedLegacy(); auditInserts.add(audit); } @@ -105,10 +107,11 @@ class DbGroupMemberAuditListener implements GroupMemberAuditListener { } @Override - public void onAddGroupsToGroup(Account.Id me, Collection added) { + public void onAddGroupsToGroup( + Account.Id me, Collection added, Timestamp addedOn) { List includesAudit = new ArrayList<>(); for (AccountGroupById groupInclude : added) { - AccountGroupByIdAud audit = new AccountGroupByIdAud(groupInclude, me, TimeUtil.nowTs()); + AccountGroupByIdAud audit = new AccountGroupByIdAud(groupInclude, me, addedOn); includesAudit.add(audit); } try (ReviewDb db = schema.open()) { @@ -120,7 +123,8 @@ class DbGroupMemberAuditListener implements GroupMemberAuditListener { } @Override - public void onDeleteGroupsFromGroup(Account.Id me, Collection removed) { + public void onDeleteGroupsFromGroup( + Account.Id me, Collection removed, Timestamp removedOn) { final List auditUpdates = new ArrayList<>(); try (ReviewDb db = schema.open()) { for (AccountGroupById g : removed) { @@ -134,7 +138,7 @@ class DbGroupMemberAuditListener implements GroupMemberAuditListener { } if (audit != null) { - audit.removed(me, TimeUtil.nowTs()); + audit.removed(me, removedOn); auditUpdates.add(audit); } } diff --git a/java/com/google/gerrit/server/group/db/GroupConfig.java b/java/com/google/gerrit/server/group/db/GroupConfig.java index 38abd19e90..c528f8e362 100644 --- a/java/com/google/gerrit/server/group/db/GroupConfig.java +++ b/java/com/google/gerrit/server/group/db/GroupConfig.java @@ -23,6 +23,7 @@ import com.google.common.base.Strings; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import com.google.common.collect.Streams; +import com.google.gerrit.common.TimeUtil; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.RefNames; @@ -203,15 +204,10 @@ public class GroupConfig extends VersionedMetaData { String.format("Name of the group %s must be defined", groupUuid.get())); } - Timestamp createdOn; - if (groupCreation.isPresent()) { - createdOn = groupCreation.get().getCreatedOn(); - commit.setAuthor(new PersonIdent(commit.getAuthor(), createdOn)); - commit.setCommitter(new PersonIdent(commit.getCommitter(), createdOn)); - } else { - checkState(loadedGroup.isPresent(), "Cannot update non-existent group %s", groupUuid.get()); - createdOn = loadedGroup.get().getCreatedOn(); - } + Timestamp commitTimestamp = + groupUpdate.flatMap(InternalGroupUpdate::getUpdatedOn).orElseGet(TimeUtil::nowTs); + commit.setAuthor(new PersonIdent(commit.getAuthor(), commitTimestamp)); + commit.setCommitter(new PersonIdent(commit.getCommitter(), commitTimestamp)); Config config = updateGroupProperties(); @@ -223,6 +219,8 @@ public class GroupConfig extends VersionedMetaData { loadedGroup.map(InternalGroup::getSubgroups).orElseGet(ImmutableSet::of); Optional> updatedSubgroups = updateSubgroups(originalSubgroups); + Timestamp createdOn = loadedGroup.map(InternalGroup::getCreatedOn).orElse(commitTimestamp); + String commitMessage = createCommitMessage(originalMembers, updatedMembers, originalSubgroups, updatedSubgroups); commit.setMessage(commitMessage); diff --git a/java/com/google/gerrit/server/group/db/GroupRebuilder.java b/java/com/google/gerrit/server/group/db/GroupRebuilder.java index ce9ed9f361..e8a98f1a86 100644 --- a/java/com/google/gerrit/server/group/db/GroupRebuilder.java +++ b/java/com/google/gerrit/server/group/db/GroupRebuilder.java @@ -122,13 +122,13 @@ public class GroupRebuilder { .setId(bundle.id()) .setNameKey(group.getNameKey()) .setGroupUUID(group.getGroupUUID()) - .setCreatedOn(group.getCreatedOn()) .build()); InternalGroupUpdate.Builder updateBuilder = InternalGroupUpdate.builder() .setOwnerGroupUUID(group.getOwnerGroupUUID()) - .setVisibleToAll(group.isVisibleToAll()); + .setVisibleToAll(group.isVisibleToAll()) + .setUpdatedOn(group.getCreatedOn()); if (bundle.group().getDescription() != null) { updateBuilder.setDescription(group.getDescription()); } @@ -151,6 +151,7 @@ public class GroupRebuilder { for (Map.Entry> e : events.entrySet()) { InternalGroupUpdate.Builder ub = InternalGroupUpdate.builder(); e.getValue().forEach(event -> event.update().accept(ub)); + ub.setUpdatedOn(e.getKey().when()); groupConfig.setGroupUpdate(ub.build(), getAccountNameEmailFunc, getGroupNameFunc); PersonIdent currServerIdent = new PersonIdent(nowServerIdent, e.getKey().when()); diff --git a/java/com/google/gerrit/server/group/db/GroupsUpdate.java b/java/com/google/gerrit/server/group/db/GroupsUpdate.java index 7d61065445..c4d88a94fa 100644 --- a/java/com/google/gerrit/server/group/db/GroupsUpdate.java +++ b/java/com/google/gerrit/server/group/db/GroupsUpdate.java @@ -24,6 +24,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import com.google.gerrit.common.Nullable; +import com.google.gerrit.common.TimeUtil; import com.google.gerrit.common.data.GroupDescription; import com.google.gerrit.common.errors.NoSuchGroupException; import com.google.gerrit.reviewdb.client.Account; @@ -57,6 +58,7 @@ import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; import java.io.IOException; +import java.sql.Timestamp; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -203,6 +205,12 @@ public class GroupsUpdate { ReviewDb db, InternalGroupCreation groupCreation, InternalGroupUpdate groupUpdate) throws OrmException, IOException, ConfigInvalidException { if (!groupsMigration.disableGroupReviewDb()) { + if (!groupUpdate.getUpdatedOn().isPresent()) { + // Set updatedOn to a specific value so that the same timestamp is used for ReviewDb and + // NoteDb. + groupUpdate = groupUpdate.toBuilder().setUpdatedOn(TimeUtil.nowTs()).build(); + } + InternalGroup createdGroupInReviewDb = createGroupInReviewDb(ReviewDbUtil.unwrapDb(db), groupCreation, groupUpdate); @@ -243,6 +251,12 @@ public class GroupsUpdate { throws OrmException, NoSuchGroupException, IOException, ConfigInvalidException { UpdateResult reviewDbUpdateResult = null; if (!groupsMigration.disableGroupReviewDb()) { + if (!groupUpdate.getUpdatedOn().isPresent()) { + // Set updatedOn to a specific value so that the same timestamp is used for ReviewDb and + // NoteDb. + groupUpdate = groupUpdate.toBuilder().setUpdatedOn(TimeUtil.nowTs()).build(); + } + AccountGroup group = getExistingGroupFromReviewDb(ReviewDbUtil.unwrapDb(db), groupUuid); reviewDbUpdateResult = updateGroupInReviewDb(ReviewDbUtil.unwrapDb(db), group, groupUpdate); @@ -266,7 +280,8 @@ public class GroupsUpdate { // already been used to create another group db.accountGroupNames().insert(ImmutableList.of(gn)); - AccountGroup group = createAccountGroup(groupCreation); + Timestamp createdOn = groupUpdate.getUpdatedOn().orElseGet(TimeUtil::nowTs); + AccountGroup group = createAccountGroup(groupCreation, createdOn); UpdateResult updateResult = updateGroupInReviewDb(db, group, groupUpdate); return InternalGroup.create( group, @@ -277,17 +292,16 @@ public class GroupsUpdate { public static AccountGroup createAccountGroup( InternalGroupCreation groupCreation, InternalGroupUpdate groupUpdate) { - AccountGroup group = createAccountGroup(groupCreation); + Timestamp createdOn = groupUpdate.getUpdatedOn().orElseGet(TimeUtil::nowTs); + AccountGroup group = createAccountGroup(groupCreation, createdOn); applyUpdate(group, groupUpdate); return group; } - private static AccountGroup createAccountGroup(InternalGroupCreation groupCreation) { + private static AccountGroup createAccountGroup( + InternalGroupCreation groupCreation, Timestamp createdOn) { return new AccountGroup( - groupCreation.getNameKey(), - groupCreation.getId(), - groupCreation.getGroupUUID(), - groupCreation.getCreatedOn()); + groupCreation.getNameKey(), groupCreation.getId(), groupCreation.getGroupUUID(), createdOn); } private static void applyUpdate(AccountGroup group, InternalGroupUpdate groupUpdate) { @@ -350,6 +364,7 @@ public class GroupsUpdate { private ImmutableSet updateMembersInReviewDb( ReviewDb db, AccountGroup.Id groupId, InternalGroupUpdate groupUpdate) throws OrmException { + Timestamp updatedOn = groupUpdate.getUpdatedOn().orElseGet(TimeUtil::nowTs); ImmutableSet originalMembers = Groups.getMembersFromReviewDb(db, groupId).collect(toImmutableSet()); ImmutableSet updatedMembers = @@ -357,19 +372,20 @@ public class GroupsUpdate { Set addedMembers = Sets.difference(updatedMembers, originalMembers); if (!addedMembers.isEmpty()) { - addGroupMembersInReviewDb(db, groupId, addedMembers); + addGroupMembersInReviewDb(db, groupId, addedMembers, updatedOn); } Set removedMembers = Sets.difference(originalMembers, updatedMembers); if (!removedMembers.isEmpty()) { - removeGroupMembersInReviewDb(db, groupId, removedMembers); + removeGroupMembersInReviewDb(db, groupId, removedMembers, updatedOn); } return Sets.union(addedMembers, removedMembers).immutableCopy(); } private void addGroupMembersInReviewDb( - ReviewDb db, AccountGroup.Id groupId, Set newMemberIds) throws OrmException { + ReviewDb db, AccountGroup.Id groupId, Set newMemberIds, Timestamp addedOn) + throws OrmException { Set newMembers = newMemberIds .stream() @@ -378,13 +394,14 @@ public class GroupsUpdate { .collect(toImmutableSet()); if (currentUser != null) { - auditService.dispatchAddAccountsToGroup(currentUser.getAccountId(), newMembers); + auditService.dispatchAddAccountsToGroup(currentUser.getAccountId(), newMembers, addedOn); } db.accountGroupMembers().insert(newMembers); } private void removeGroupMembersInReviewDb( - ReviewDb db, AccountGroup.Id groupId, Set accountIds) throws OrmException { + ReviewDb db, AccountGroup.Id groupId, Set accountIds, Timestamp removedOn) + throws OrmException { Set membersToRemove = accountIds .stream() @@ -393,13 +410,15 @@ public class GroupsUpdate { .collect(toImmutableSet()); if (currentUser != null) { - auditService.dispatchDeleteAccountsFromGroup(currentUser.getAccountId(), membersToRemove); + auditService.dispatchDeleteAccountsFromGroup( + currentUser.getAccountId(), membersToRemove, removedOn); } db.accountGroupMembers().delete(membersToRemove); } private ImmutableSet updateSubgroupsInReviewDb( ReviewDb db, AccountGroup.Id groupId, InternalGroupUpdate groupUpdate) throws OrmException { + Timestamp updatedOn = groupUpdate.getUpdatedOn().orElseGet(TimeUtil::nowTs); ImmutableSet originalSubgroups = Groups.getSubgroupsFromReviewDb(db, groupId).collect(toImmutableSet()); ImmutableSet updatedSubgroups = @@ -407,19 +426,22 @@ public class GroupsUpdate { Set addedSubgroups = Sets.difference(updatedSubgroups, originalSubgroups); if (!addedSubgroups.isEmpty()) { - addSubgroupsInReviewDb(db, groupId, addedSubgroups); + addSubgroupsInReviewDb(db, groupId, addedSubgroups, updatedOn); } Set removedSubgroups = Sets.difference(originalSubgroups, updatedSubgroups); if (!removedSubgroups.isEmpty()) { - removeSubgroupsInReviewDb(db, groupId, removedSubgroups); + removeSubgroupsInReviewDb(db, groupId, removedSubgroups, updatedOn); } return Sets.union(addedSubgroups, removedSubgroups).immutableCopy(); } private void addSubgroupsInReviewDb( - ReviewDb db, AccountGroup.Id parentGroupId, Set subgroupUuids) + ReviewDb db, + AccountGroup.Id parentGroupId, + Set subgroupUuids, + Timestamp addedOn) throws OrmException { Set newSubgroups = subgroupUuids @@ -429,13 +451,16 @@ public class GroupsUpdate { .collect(toImmutableSet()); if (currentUser != null) { - auditService.dispatchAddGroupsToGroup(currentUser.getAccountId(), newSubgroups); + auditService.dispatchAddGroupsToGroup(currentUser.getAccountId(), newSubgroups, addedOn); } db.accountGroupById().insert(newSubgroups); } private void removeSubgroupsInReviewDb( - ReviewDb db, AccountGroup.Id parentGroupId, Set subgroupUuids) + ReviewDb db, + AccountGroup.Id parentGroupId, + Set subgroupUuids, + Timestamp removedOn) throws OrmException { Set subgroupsToRemove = subgroupUuids @@ -445,7 +470,8 @@ public class GroupsUpdate { .collect(toImmutableSet()); if (currentUser != null) { - auditService.dispatchDeleteGroupsFromGroup(currentUser.getAccountId(), subgroupsToRemove); + auditService.dispatchDeleteGroupsFromGroup( + currentUser.getAccountId(), subgroupsToRemove, removedOn); } db.accountGroupById().delete(subgroupsToRemove); } diff --git a/java/com/google/gerrit/server/group/db/InternalGroupCreation.java b/java/com/google/gerrit/server/group/db/InternalGroupCreation.java index 4b5b7c5f9b..3fcf96f7a1 100644 --- a/java/com/google/gerrit/server/group/db/InternalGroupCreation.java +++ b/java/com/google/gerrit/server/group/db/InternalGroupCreation.java @@ -16,7 +16,6 @@ package com.google.gerrit.server.group.db; import com.google.auto.value.AutoValue; import com.google.gerrit.reviewdb.client.AccountGroup; -import java.sql.Timestamp; // TODO(aliceks): Add Javadoc descriptions to this file. @AutoValue @@ -28,8 +27,6 @@ public abstract class InternalGroupCreation { public abstract AccountGroup.UUID getGroupUUID(); - public abstract Timestamp getCreatedOn(); - public static Builder builder() { return new AutoValue_InternalGroupCreation.Builder(); } @@ -42,8 +39,6 @@ public abstract class InternalGroupCreation { public abstract InternalGroupCreation.Builder setGroupUUID(AccountGroup.UUID groupUuid); - public abstract InternalGroupCreation.Builder setCreatedOn(Timestamp createdOn); - public abstract InternalGroupCreation build(); } } diff --git a/java/com/google/gerrit/server/group/db/InternalGroupUpdate.java b/java/com/google/gerrit/server/group/db/InternalGroupUpdate.java index 13f84c18c4..5758297e61 100644 --- a/java/com/google/gerrit/server/group/db/InternalGroupUpdate.java +++ b/java/com/google/gerrit/server/group/db/InternalGroupUpdate.java @@ -18,6 +18,7 @@ import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableSet; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroup; +import java.sql.Timestamp; import java.util.Optional; import java.util.Set; @@ -47,6 +48,10 @@ public abstract class InternalGroupUpdate { public abstract SubgroupModification getSubgroupModification(); + public abstract Optional getUpdatedOn(); + + public abstract Builder toBuilder(); + public static Builder builder() { return new AutoValue_InternalGroupUpdate.Builder() .setMemberModification(in -> in) @@ -71,6 +76,8 @@ public abstract class InternalGroupUpdate { abstract SubgroupModification getSubgroupModification(); + public abstract Builder setUpdatedOn(Timestamp timestamp); + public abstract InternalGroupUpdate build(); } } diff --git a/java/com/google/gerrit/server/schema/SchemaCreator.java b/java/com/google/gerrit/server/schema/SchemaCreator.java index 112fd55dc0..2004c989e8 100644 --- a/java/com/google/gerrit/server/schema/SchemaCreator.java +++ b/java/com/google/gerrit/server/schema/SchemaCreator.java @@ -17,7 +17,6 @@ package com.google.gerrit.server.schema; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.gerrit.common.Nullable; -import com.google.gerrit.common.TimeUtil; import com.google.gerrit.common.data.GroupReference; import com.google.gerrit.metrics.MetricMaker; import com.google.gerrit.reviewdb.client.Account; @@ -292,7 +291,6 @@ public class SchemaCreator { .setNameKey(new AccountGroup.NameKey(groupReference.getName())) .setId(new AccountGroup.Id(next)) .setGroupUUID(groupReference.getUUID()) - .setCreatedOn(TimeUtil.nowTs()) .build(); } diff --git a/javatests/com/google/gerrit/server/group/db/AuditLogReaderTest.java b/javatests/com/google/gerrit/server/group/db/AuditLogReaderTest.java index c985c46aca..0d1fcf96bb 100644 --- a/javatests/com/google/gerrit/server/group/db/AuditLogReaderTest.java +++ b/javatests/com/google/gerrit/server/group/db/AuditLogReaderTest.java @@ -18,7 +18,6 @@ import static com.google.common.truth.Truth.assertThat; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; -import com.google.gerrit.common.TimeUtil; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountGroupByIdAud; @@ -288,7 +287,6 @@ public final class AuditLogReaderTest extends AbstractGroupTest { .setGroupUUID(GroupUUID.make(groupName, serverIdent)) .setNameKey(new AccountGroup.NameKey(groupName)) .setId(new AccountGroup.Id(next)) - .setCreatedOn(TimeUtil.nowTs()) .build(); InternalGroupUpdate groupUpdate = authorIdent.equals(serverIdent) diff --git a/javatests/com/google/gerrit/server/group/db/GroupConfigTest.java b/javatests/com/google/gerrit/server/group/db/GroupConfigTest.java index 6b68c4761e..6240f9cae0 100644 --- a/javatests/com/google/gerrit/server/group/db/GroupConfigTest.java +++ b/javatests/com/google/gerrit/server/group/db/GroupConfigTest.java @@ -236,8 +236,7 @@ public class GroupConfigTest { return InternalGroupCreation.builder() .setGroupUUID(groupUuid) .setNameKey(groupName) - .setId(groupId) - .setCreatedOn(TimeUtil.nowTs()); + .setId(groupId); } private void populateGroupConfig(AccountGroup.UUID uuid, String fileContent) throws Exception {