Merge "Use same timestamp for group audit log in ReviewDb and NoteDb"

This commit is contained in:
Alice Kober-Sotzek
2017-11-30 15:11:37 +00:00
committed by Gerrit Code Review
12 changed files with 96 additions and 65 deletions

View File

@@ -20,6 +20,7 @@ import com.google.gerrit.reviewdb.client.AccountGroupById;
import com.google.gerrit.reviewdb.client.AccountGroupMember; import com.google.gerrit.reviewdb.client.AccountGroupMember;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Singleton; import com.google.inject.Singleton;
import java.sql.Timestamp;
import java.util.Collection; import java.util.Collection;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
@@ -45,10 +46,11 @@ public class AuditService {
} }
} }
public void dispatchAddAccountsToGroup(Account.Id actor, Collection<AccountGroupMember> added) { public void dispatchAddAccountsToGroup(
Account.Id actor, Collection<AccountGroupMember> added, Timestamp addedOn) {
for (GroupMemberAuditListener auditListener : groupMemberAuditListeners) { for (GroupMemberAuditListener auditListener : groupMemberAuditListeners) {
try { try {
auditListener.onAddAccountsToGroup(actor, added); auditListener.onAddAccountsToGroup(actor, added, addedOn);
} catch (RuntimeException e) { } catch (RuntimeException e) {
log.error("failed to log add accounts to group event", e); log.error("failed to log add accounts to group event", e);
} }
@@ -56,20 +58,21 @@ public class AuditService {
} }
public void dispatchDeleteAccountsFromGroup( public void dispatchDeleteAccountsFromGroup(
Account.Id actor, Collection<AccountGroupMember> removed) { Account.Id actor, Collection<AccountGroupMember> removed, Timestamp removedOn) {
for (GroupMemberAuditListener auditListener : groupMemberAuditListeners) { for (GroupMemberAuditListener auditListener : groupMemberAuditListeners) {
try { try {
auditListener.onDeleteAccountsFromGroup(actor, removed); auditListener.onDeleteAccountsFromGroup(actor, removed, removedOn);
} catch (RuntimeException e) { } catch (RuntimeException e) {
log.error("failed to log delete accounts from group event", e); log.error("failed to log delete accounts from group event", e);
} }
} }
} }
public void dispatchAddGroupsToGroup(Account.Id actor, Collection<AccountGroupById> added) { public void dispatchAddGroupsToGroup(
Account.Id actor, Collection<AccountGroupById> added, Timestamp addedOn) {
for (GroupMemberAuditListener auditListener : groupMemberAuditListeners) { for (GroupMemberAuditListener auditListener : groupMemberAuditListeners) {
try { try {
auditListener.onAddGroupsToGroup(actor, added); auditListener.onAddGroupsToGroup(actor, added, addedOn);
} catch (RuntimeException e) { } catch (RuntimeException e) {
log.error("failed to log add groups to group event", e); log.error("failed to log add groups to group event", e);
} }
@@ -77,10 +80,10 @@ public class AuditService {
} }
public void dispatchDeleteGroupsFromGroup( public void dispatchDeleteGroupsFromGroup(
Account.Id actor, Collection<AccountGroupById> removed) { Account.Id actor, Collection<AccountGroupById> removed, Timestamp removedOn) {
for (GroupMemberAuditListener auditListener : groupMemberAuditListeners) { for (GroupMemberAuditListener auditListener : groupMemberAuditListeners) {
try { try {
auditListener.onDeleteGroupsFromGroup(actor, removed); auditListener.onDeleteGroupsFromGroup(actor, removed, removedOn);
} catch (RuntimeException e) { } catch (RuntimeException e) {
log.error("failed to log delete groups from group event", e); log.error("failed to log delete groups from group event", e);
} }

View File

@@ -18,16 +18,20 @@ import com.google.gerrit.extensions.annotations.ExtensionPoint;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroupById; import com.google.gerrit.reviewdb.client.AccountGroupById;
import com.google.gerrit.reviewdb.client.AccountGroupMember; import com.google.gerrit.reviewdb.client.AccountGroupMember;
import java.sql.Timestamp;
import java.util.Collection; import java.util.Collection;
@ExtensionPoint @ExtensionPoint
public interface GroupMemberAuditListener { public interface GroupMemberAuditListener {
void onAddAccountsToGroup(Account.Id actor, Collection<AccountGroupMember> added); void onAddAccountsToGroup(
Account.Id actor, Collection<AccountGroupMember> added, Timestamp addedOn);
void onDeleteAccountsFromGroup(Account.Id actor, Collection<AccountGroupMember> removed); void onDeleteAccountsFromGroup(
Account.Id actor, Collection<AccountGroupMember> removed, Timestamp removedOn);
void onAddGroupsToGroup(Account.Id actor, Collection<AccountGroupById> added); void onAddGroupsToGroup(Account.Id actor, Collection<AccountGroupById> added, Timestamp addedOn);
void onDeleteGroupsFromGroup(Account.Id actor, Collection<AccountGroupById> deleted); void onDeleteGroupsFromGroup(
Account.Id actor, Collection<AccountGroupById> deleted, Timestamp removedOn);
} }

View File

@@ -17,7 +17,6 @@ package com.google.gerrit.server.group;
import com.google.common.base.MoreObjects; import com.google.common.base.MoreObjects;
import com.google.common.base.Strings; import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSet; 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.GlobalCapability;
import com.google.gerrit.common.data.GroupDescription; import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.extensions.annotations.RequiresCapability; import com.google.gerrit.extensions.annotations.RequiresCapability;
@@ -206,7 +205,6 @@ public class CreateGroup implements RestModifyView<TopLevelResource, GroupInput>
.setGroupUUID(uuid) .setGroupUUID(uuid)
.setNameKey(createGroupArgs.getGroup()) .setNameKey(createGroupArgs.getGroup())
.setId(groupId) .setId(groupId)
.setCreatedOn(TimeUtil.nowTs())
.build(); .build();
InternalGroupUpdate.Builder groupUpdateBuilder = InternalGroupUpdate.Builder groupUpdateBuilder =
InternalGroupUpdate.builder().setVisibleToAll(createGroupArgs.visibleToAll); InternalGroupUpdate.builder().setVisibleToAll(createGroupArgs.visibleToAll);

View File

@@ -15,7 +15,6 @@
package com.google.gerrit.server.group; package com.google.gerrit.server.group;
import com.google.common.base.Joiner; 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.Account;
import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountGroupById; 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.OrmException;
import com.google.gwtorm.server.SchemaFactory; import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject; import com.google.inject.Inject;
import java.sql.Timestamp;
import java.text.MessageFormat; import java.text.MessageFormat;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collection; import java.util.Collection;
@@ -58,10 +58,11 @@ class DbGroupMemberAuditListener implements GroupMemberAuditListener {
} }
@Override @Override
public void onAddAccountsToGroup(Account.Id me, Collection<AccountGroupMember> added) { public void onAddAccountsToGroup(
Account.Id me, Collection<AccountGroupMember> added, Timestamp addedOn) {
List<AccountGroupMemberAudit> auditInserts = new ArrayList<>(); List<AccountGroupMemberAudit> auditInserts = new ArrayList<>();
for (AccountGroupMember m : added) { for (AccountGroupMember m : added) {
AccountGroupMemberAudit audit = new AccountGroupMemberAudit(m, me, TimeUtil.nowTs()); AccountGroupMemberAudit audit = new AccountGroupMemberAudit(m, me, addedOn);
auditInserts.add(audit); auditInserts.add(audit);
} }
try (ReviewDb db = schema.open()) { try (ReviewDb db = schema.open()) {
@@ -73,7 +74,8 @@ class DbGroupMemberAuditListener implements GroupMemberAuditListener {
} }
@Override @Override
public void onDeleteAccountsFromGroup(Account.Id me, Collection<AccountGroupMember> removed) { public void onDeleteAccountsFromGroup(
Account.Id me, Collection<AccountGroupMember> removed, Timestamp removedOn) {
List<AccountGroupMemberAudit> auditInserts = new ArrayList<>(); List<AccountGroupMemberAudit> auditInserts = new ArrayList<>();
List<AccountGroupMemberAudit> auditUpdates = new ArrayList<>(); List<AccountGroupMemberAudit> auditUpdates = new ArrayList<>();
try (ReviewDb db = schema.open()) { try (ReviewDb db = schema.open()) {
@@ -88,10 +90,10 @@ class DbGroupMemberAuditListener implements GroupMemberAuditListener {
} }
if (audit != null) { if (audit != null) {
audit.removed(me, TimeUtil.nowTs()); audit.removed(me, removedOn);
auditUpdates.add(audit); auditUpdates.add(audit);
} else { } else {
audit = new AccountGroupMemberAudit(m, me, TimeUtil.nowTs()); audit = new AccountGroupMemberAudit(m, me, removedOn);
audit.removedLegacy(); audit.removedLegacy();
auditInserts.add(audit); auditInserts.add(audit);
} }
@@ -105,10 +107,11 @@ class DbGroupMemberAuditListener implements GroupMemberAuditListener {
} }
@Override @Override
public void onAddGroupsToGroup(Account.Id me, Collection<AccountGroupById> added) { public void onAddGroupsToGroup(
Account.Id me, Collection<AccountGroupById> added, Timestamp addedOn) {
List<AccountGroupByIdAud> includesAudit = new ArrayList<>(); List<AccountGroupByIdAud> includesAudit = new ArrayList<>();
for (AccountGroupById groupInclude : added) { for (AccountGroupById groupInclude : added) {
AccountGroupByIdAud audit = new AccountGroupByIdAud(groupInclude, me, TimeUtil.nowTs()); AccountGroupByIdAud audit = new AccountGroupByIdAud(groupInclude, me, addedOn);
includesAudit.add(audit); includesAudit.add(audit);
} }
try (ReviewDb db = schema.open()) { try (ReviewDb db = schema.open()) {
@@ -120,7 +123,8 @@ class DbGroupMemberAuditListener implements GroupMemberAuditListener {
} }
@Override @Override
public void onDeleteGroupsFromGroup(Account.Id me, Collection<AccountGroupById> removed) { public void onDeleteGroupsFromGroup(
Account.Id me, Collection<AccountGroupById> removed, Timestamp removedOn) {
final List<AccountGroupByIdAud> auditUpdates = new ArrayList<>(); final List<AccountGroupByIdAud> auditUpdates = new ArrayList<>();
try (ReviewDb db = schema.open()) { try (ReviewDb db = schema.open()) {
for (AccountGroupById g : removed) { for (AccountGroupById g : removed) {
@@ -134,7 +138,7 @@ class DbGroupMemberAuditListener implements GroupMemberAuditListener {
} }
if (audit != null) { if (audit != null) {
audit.removed(me, TimeUtil.nowTs()); audit.removed(me, removedOn);
auditUpdates.add(audit); auditUpdates.add(audit);
} }
} }

View File

@@ -23,6 +23,7 @@ import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets; import com.google.common.collect.Sets;
import com.google.common.collect.Streams; 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.Account;
import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.RefNames; 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())); String.format("Name of the group %s must be defined", groupUuid.get()));
} }
Timestamp createdOn; Timestamp commitTimestamp =
if (groupCreation.isPresent()) { groupUpdate.flatMap(InternalGroupUpdate::getUpdatedOn).orElseGet(TimeUtil::nowTs);
createdOn = groupCreation.get().getCreatedOn(); commit.setAuthor(new PersonIdent(commit.getAuthor(), commitTimestamp));
commit.setAuthor(new PersonIdent(commit.getAuthor(), createdOn)); commit.setCommitter(new PersonIdent(commit.getCommitter(), commitTimestamp));
commit.setCommitter(new PersonIdent(commit.getCommitter(), createdOn));
} else {
checkState(loadedGroup.isPresent(), "Cannot update non-existent group %s", groupUuid.get());
createdOn = loadedGroup.get().getCreatedOn();
}
Config config = updateGroupProperties(); Config config = updateGroupProperties();
@@ -223,6 +219,8 @@ public class GroupConfig extends VersionedMetaData {
loadedGroup.map(InternalGroup::getSubgroups).orElseGet(ImmutableSet::of); loadedGroup.map(InternalGroup::getSubgroups).orElseGet(ImmutableSet::of);
Optional<ImmutableSet<AccountGroup.UUID>> updatedSubgroups = updateSubgroups(originalSubgroups); Optional<ImmutableSet<AccountGroup.UUID>> updatedSubgroups = updateSubgroups(originalSubgroups);
Timestamp createdOn = loadedGroup.map(InternalGroup::getCreatedOn).orElse(commitTimestamp);
String commitMessage = String commitMessage =
createCommitMessage(originalMembers, updatedMembers, originalSubgroups, updatedSubgroups); createCommitMessage(originalMembers, updatedMembers, originalSubgroups, updatedSubgroups);
commit.setMessage(commitMessage); commit.setMessage(commitMessage);

View File

@@ -122,13 +122,13 @@ public class GroupRebuilder {
.setId(bundle.id()) .setId(bundle.id())
.setNameKey(group.getNameKey()) .setNameKey(group.getNameKey())
.setGroupUUID(group.getGroupUUID()) .setGroupUUID(group.getGroupUUID())
.setCreatedOn(group.getCreatedOn())
.build()); .build());
InternalGroupUpdate.Builder updateBuilder = InternalGroupUpdate.Builder updateBuilder =
InternalGroupUpdate.builder() InternalGroupUpdate.builder()
.setOwnerGroupUUID(group.getOwnerGroupUUID()) .setOwnerGroupUUID(group.getOwnerGroupUUID())
.setVisibleToAll(group.isVisibleToAll()); .setVisibleToAll(group.isVisibleToAll())
.setUpdatedOn(group.getCreatedOn());
if (bundle.group().getDescription() != null) { if (bundle.group().getDescription() != null) {
updateBuilder.setDescription(group.getDescription()); updateBuilder.setDescription(group.getDescription());
} }
@@ -151,6 +151,7 @@ public class GroupRebuilder {
for (Map.Entry<Key, Collection<Event>> e : events.entrySet()) { for (Map.Entry<Key, Collection<Event>> e : events.entrySet()) {
InternalGroupUpdate.Builder ub = InternalGroupUpdate.builder(); InternalGroupUpdate.Builder ub = InternalGroupUpdate.builder();
e.getValue().forEach(event -> event.update().accept(ub)); e.getValue().forEach(event -> event.update().accept(ub));
ub.setUpdatedOn(e.getKey().when());
groupConfig.setGroupUpdate(ub.build(), getAccountNameEmailFunc, getGroupNameFunc); groupConfig.setGroupUpdate(ub.build(), getAccountNameEmailFunc, getGroupNameFunc);
PersonIdent currServerIdent = new PersonIdent(nowServerIdent, e.getKey().when()); PersonIdent currServerIdent = new PersonIdent(nowServerIdent, e.getKey().when());

View File

@@ -24,6 +24,7 @@ import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets; import com.google.common.collect.Sets;
import com.google.gerrit.common.Nullable; import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.data.GroupDescription; import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.errors.NoSuchGroupException; import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.reviewdb.client.Account; 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.Inject;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;
import java.io.IOException; import java.io.IOException;
import java.sql.Timestamp;
import java.util.Objects; import java.util.Objects;
import java.util.Optional; import java.util.Optional;
import java.util.Set; import java.util.Set;
@@ -203,6 +205,12 @@ public class GroupsUpdate {
ReviewDb db, InternalGroupCreation groupCreation, InternalGroupUpdate groupUpdate) ReviewDb db, InternalGroupCreation groupCreation, InternalGroupUpdate groupUpdate)
throws OrmException, IOException, ConfigInvalidException { throws OrmException, IOException, ConfigInvalidException {
if (!groupsMigration.disableGroupReviewDb()) { 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 = InternalGroup createdGroupInReviewDb =
createGroupInReviewDb(ReviewDbUtil.unwrapDb(db), groupCreation, groupUpdate); createGroupInReviewDb(ReviewDbUtil.unwrapDb(db), groupCreation, groupUpdate);
@@ -243,6 +251,12 @@ public class GroupsUpdate {
throws OrmException, NoSuchGroupException, IOException, ConfigInvalidException { throws OrmException, NoSuchGroupException, IOException, ConfigInvalidException {
UpdateResult reviewDbUpdateResult = null; UpdateResult reviewDbUpdateResult = null;
if (!groupsMigration.disableGroupReviewDb()) { 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); AccountGroup group = getExistingGroupFromReviewDb(ReviewDbUtil.unwrapDb(db), groupUuid);
reviewDbUpdateResult = updateGroupInReviewDb(ReviewDbUtil.unwrapDb(db), group, groupUpdate); reviewDbUpdateResult = updateGroupInReviewDb(ReviewDbUtil.unwrapDb(db), group, groupUpdate);
@@ -266,7 +280,8 @@ public class GroupsUpdate {
// already been used to create another group // already been used to create another group
db.accountGroupNames().insert(ImmutableList.of(gn)); 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); UpdateResult updateResult = updateGroupInReviewDb(db, group, groupUpdate);
return InternalGroup.create( return InternalGroup.create(
group, group,
@@ -277,17 +292,16 @@ public class GroupsUpdate {
public static AccountGroup createAccountGroup( public static AccountGroup createAccountGroup(
InternalGroupCreation groupCreation, InternalGroupUpdate groupUpdate) { InternalGroupCreation groupCreation, InternalGroupUpdate groupUpdate) {
AccountGroup group = createAccountGroup(groupCreation); Timestamp createdOn = groupUpdate.getUpdatedOn().orElseGet(TimeUtil::nowTs);
AccountGroup group = createAccountGroup(groupCreation, createdOn);
applyUpdate(group, groupUpdate); applyUpdate(group, groupUpdate);
return group; return group;
} }
private static AccountGroup createAccountGroup(InternalGroupCreation groupCreation) { private static AccountGroup createAccountGroup(
InternalGroupCreation groupCreation, Timestamp createdOn) {
return new AccountGroup( return new AccountGroup(
groupCreation.getNameKey(), groupCreation.getNameKey(), groupCreation.getId(), groupCreation.getGroupUUID(), createdOn);
groupCreation.getId(),
groupCreation.getGroupUUID(),
groupCreation.getCreatedOn());
} }
private static void applyUpdate(AccountGroup group, InternalGroupUpdate groupUpdate) { private static void applyUpdate(AccountGroup group, InternalGroupUpdate groupUpdate) {
@@ -350,6 +364,7 @@ public class GroupsUpdate {
private ImmutableSet<Account.Id> updateMembersInReviewDb( private ImmutableSet<Account.Id> updateMembersInReviewDb(
ReviewDb db, AccountGroup.Id groupId, InternalGroupUpdate groupUpdate) throws OrmException { ReviewDb db, AccountGroup.Id groupId, InternalGroupUpdate groupUpdate) throws OrmException {
Timestamp updatedOn = groupUpdate.getUpdatedOn().orElseGet(TimeUtil::nowTs);
ImmutableSet<Account.Id> originalMembers = ImmutableSet<Account.Id> originalMembers =
Groups.getMembersFromReviewDb(db, groupId).collect(toImmutableSet()); Groups.getMembersFromReviewDb(db, groupId).collect(toImmutableSet());
ImmutableSet<Account.Id> updatedMembers = ImmutableSet<Account.Id> updatedMembers =
@@ -357,19 +372,20 @@ public class GroupsUpdate {
Set<Account.Id> addedMembers = Sets.difference(updatedMembers, originalMembers); Set<Account.Id> addedMembers = Sets.difference(updatedMembers, originalMembers);
if (!addedMembers.isEmpty()) { if (!addedMembers.isEmpty()) {
addGroupMembersInReviewDb(db, groupId, addedMembers); addGroupMembersInReviewDb(db, groupId, addedMembers, updatedOn);
} }
Set<Account.Id> removedMembers = Sets.difference(originalMembers, updatedMembers); Set<Account.Id> removedMembers = Sets.difference(originalMembers, updatedMembers);
if (!removedMembers.isEmpty()) { if (!removedMembers.isEmpty()) {
removeGroupMembersInReviewDb(db, groupId, removedMembers); removeGroupMembersInReviewDb(db, groupId, removedMembers, updatedOn);
} }
return Sets.union(addedMembers, removedMembers).immutableCopy(); return Sets.union(addedMembers, removedMembers).immutableCopy();
} }
private void addGroupMembersInReviewDb( private void addGroupMembersInReviewDb(
ReviewDb db, AccountGroup.Id groupId, Set<Account.Id> newMemberIds) throws OrmException { ReviewDb db, AccountGroup.Id groupId, Set<Account.Id> newMemberIds, Timestamp addedOn)
throws OrmException {
Set<AccountGroupMember> newMembers = Set<AccountGroupMember> newMembers =
newMemberIds newMemberIds
.stream() .stream()
@@ -378,13 +394,14 @@ public class GroupsUpdate {
.collect(toImmutableSet()); .collect(toImmutableSet());
if (currentUser != null) { if (currentUser != null) {
auditService.dispatchAddAccountsToGroup(currentUser.getAccountId(), newMembers); auditService.dispatchAddAccountsToGroup(currentUser.getAccountId(), newMembers, addedOn);
} }
db.accountGroupMembers().insert(newMembers); db.accountGroupMembers().insert(newMembers);
} }
private void removeGroupMembersInReviewDb( private void removeGroupMembersInReviewDb(
ReviewDb db, AccountGroup.Id groupId, Set<Account.Id> accountIds) throws OrmException { ReviewDb db, AccountGroup.Id groupId, Set<Account.Id> accountIds, Timestamp removedOn)
throws OrmException {
Set<AccountGroupMember> membersToRemove = Set<AccountGroupMember> membersToRemove =
accountIds accountIds
.stream() .stream()
@@ -393,13 +410,15 @@ public class GroupsUpdate {
.collect(toImmutableSet()); .collect(toImmutableSet());
if (currentUser != null) { if (currentUser != null) {
auditService.dispatchDeleteAccountsFromGroup(currentUser.getAccountId(), membersToRemove); auditService.dispatchDeleteAccountsFromGroup(
currentUser.getAccountId(), membersToRemove, removedOn);
} }
db.accountGroupMembers().delete(membersToRemove); db.accountGroupMembers().delete(membersToRemove);
} }
private ImmutableSet<AccountGroup.UUID> updateSubgroupsInReviewDb( private ImmutableSet<AccountGroup.UUID> updateSubgroupsInReviewDb(
ReviewDb db, AccountGroup.Id groupId, InternalGroupUpdate groupUpdate) throws OrmException { ReviewDb db, AccountGroup.Id groupId, InternalGroupUpdate groupUpdate) throws OrmException {
Timestamp updatedOn = groupUpdate.getUpdatedOn().orElseGet(TimeUtil::nowTs);
ImmutableSet<AccountGroup.UUID> originalSubgroups = ImmutableSet<AccountGroup.UUID> originalSubgroups =
Groups.getSubgroupsFromReviewDb(db, groupId).collect(toImmutableSet()); Groups.getSubgroupsFromReviewDb(db, groupId).collect(toImmutableSet());
ImmutableSet<AccountGroup.UUID> updatedSubgroups = ImmutableSet<AccountGroup.UUID> updatedSubgroups =
@@ -407,19 +426,22 @@ public class GroupsUpdate {
Set<AccountGroup.UUID> addedSubgroups = Sets.difference(updatedSubgroups, originalSubgroups); Set<AccountGroup.UUID> addedSubgroups = Sets.difference(updatedSubgroups, originalSubgroups);
if (!addedSubgroups.isEmpty()) { if (!addedSubgroups.isEmpty()) {
addSubgroupsInReviewDb(db, groupId, addedSubgroups); addSubgroupsInReviewDb(db, groupId, addedSubgroups, updatedOn);
} }
Set<AccountGroup.UUID> removedSubgroups = Sets.difference(originalSubgroups, updatedSubgroups); Set<AccountGroup.UUID> removedSubgroups = Sets.difference(originalSubgroups, updatedSubgroups);
if (!removedSubgroups.isEmpty()) { if (!removedSubgroups.isEmpty()) {
removeSubgroupsInReviewDb(db, groupId, removedSubgroups); removeSubgroupsInReviewDb(db, groupId, removedSubgroups, updatedOn);
} }
return Sets.union(addedSubgroups, removedSubgroups).immutableCopy(); return Sets.union(addedSubgroups, removedSubgroups).immutableCopy();
} }
private void addSubgroupsInReviewDb( private void addSubgroupsInReviewDb(
ReviewDb db, AccountGroup.Id parentGroupId, Set<AccountGroup.UUID> subgroupUuids) ReviewDb db,
AccountGroup.Id parentGroupId,
Set<AccountGroup.UUID> subgroupUuids,
Timestamp addedOn)
throws OrmException { throws OrmException {
Set<AccountGroupById> newSubgroups = Set<AccountGroupById> newSubgroups =
subgroupUuids subgroupUuids
@@ -429,13 +451,16 @@ public class GroupsUpdate {
.collect(toImmutableSet()); .collect(toImmutableSet());
if (currentUser != null) { if (currentUser != null) {
auditService.dispatchAddGroupsToGroup(currentUser.getAccountId(), newSubgroups); auditService.dispatchAddGroupsToGroup(currentUser.getAccountId(), newSubgroups, addedOn);
} }
db.accountGroupById().insert(newSubgroups); db.accountGroupById().insert(newSubgroups);
} }
private void removeSubgroupsInReviewDb( private void removeSubgroupsInReviewDb(
ReviewDb db, AccountGroup.Id parentGroupId, Set<AccountGroup.UUID> subgroupUuids) ReviewDb db,
AccountGroup.Id parentGroupId,
Set<AccountGroup.UUID> subgroupUuids,
Timestamp removedOn)
throws OrmException { throws OrmException {
Set<AccountGroupById> subgroupsToRemove = Set<AccountGroupById> subgroupsToRemove =
subgroupUuids subgroupUuids
@@ -445,7 +470,8 @@ public class GroupsUpdate {
.collect(toImmutableSet()); .collect(toImmutableSet());
if (currentUser != null) { if (currentUser != null) {
auditService.dispatchDeleteGroupsFromGroup(currentUser.getAccountId(), subgroupsToRemove); auditService.dispatchDeleteGroupsFromGroup(
currentUser.getAccountId(), subgroupsToRemove, removedOn);
} }
db.accountGroupById().delete(subgroupsToRemove); db.accountGroupById().delete(subgroupsToRemove);
} }

View File

@@ -16,7 +16,6 @@ package com.google.gerrit.server.group.db;
import com.google.auto.value.AutoValue; import com.google.auto.value.AutoValue;
import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountGroup;
import java.sql.Timestamp;
// TODO(aliceks): Add Javadoc descriptions to this file. // TODO(aliceks): Add Javadoc descriptions to this file.
@AutoValue @AutoValue
@@ -28,8 +27,6 @@ public abstract class InternalGroupCreation {
public abstract AccountGroup.UUID getGroupUUID(); public abstract AccountGroup.UUID getGroupUUID();
public abstract Timestamp getCreatedOn();
public static Builder builder() { public static Builder builder() {
return new AutoValue_InternalGroupCreation.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 setGroupUUID(AccountGroup.UUID groupUuid);
public abstract InternalGroupCreation.Builder setCreatedOn(Timestamp createdOn);
public abstract InternalGroupCreation build(); public abstract InternalGroupCreation build();
} }
} }

View File

@@ -18,6 +18,7 @@ import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountGroup;
import java.sql.Timestamp;
import java.util.Optional; import java.util.Optional;
import java.util.Set; import java.util.Set;
@@ -47,6 +48,10 @@ public abstract class InternalGroupUpdate {
public abstract SubgroupModification getSubgroupModification(); public abstract SubgroupModification getSubgroupModification();
public abstract Optional<Timestamp> getUpdatedOn();
public abstract Builder toBuilder();
public static Builder builder() { public static Builder builder() {
return new AutoValue_InternalGroupUpdate.Builder() return new AutoValue_InternalGroupUpdate.Builder()
.setMemberModification(in -> in) .setMemberModification(in -> in)
@@ -71,6 +76,8 @@ public abstract class InternalGroupUpdate {
abstract SubgroupModification getSubgroupModification(); abstract SubgroupModification getSubgroupModification();
public abstract Builder setUpdatedOn(Timestamp timestamp);
public abstract InternalGroupUpdate build(); public abstract InternalGroupUpdate build();
} }
} }

View File

@@ -17,7 +17,6 @@ package com.google.gerrit.server.schema;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.Nullable; import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.data.GroupReference; import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.metrics.MetricMaker; import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
@@ -292,7 +291,6 @@ public class SchemaCreator {
.setNameKey(new AccountGroup.NameKey(groupReference.getName())) .setNameKey(new AccountGroup.NameKey(groupReference.getName()))
.setId(new AccountGroup.Id(next)) .setId(new AccountGroup.Id(next))
.setGroupUUID(groupReference.getUUID()) .setGroupUUID(groupReference.getUUID())
.setCreatedOn(TimeUtil.nowTs())
.build(); .build();
} }

View File

@@ -18,7 +18,6 @@ import static com.google.common.truth.Truth.assertThat;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets; 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.Account;
import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountGroupByIdAud; import com.google.gerrit.reviewdb.client.AccountGroupByIdAud;
@@ -288,7 +287,6 @@ public final class AuditLogReaderTest extends AbstractGroupTest {
.setGroupUUID(GroupUUID.make(groupName, serverIdent)) .setGroupUUID(GroupUUID.make(groupName, serverIdent))
.setNameKey(new AccountGroup.NameKey(groupName)) .setNameKey(new AccountGroup.NameKey(groupName))
.setId(new AccountGroup.Id(next)) .setId(new AccountGroup.Id(next))
.setCreatedOn(TimeUtil.nowTs())
.build(); .build();
InternalGroupUpdate groupUpdate = InternalGroupUpdate groupUpdate =
authorIdent.equals(serverIdent) authorIdent.equals(serverIdent)

View File

@@ -236,8 +236,7 @@ public class GroupConfigTest {
return InternalGroupCreation.builder() return InternalGroupCreation.builder()
.setGroupUUID(groupUuid) .setGroupUUID(groupUuid)
.setNameKey(groupName) .setNameKey(groupName)
.setId(groupId) .setId(groupId);
.setCreatedOn(TimeUtil.nowTs());
} }
private void populateGroupConfig(AccountGroup.UUID uuid, String fileContent) throws Exception { private void populateGroupConfig(AccountGroup.UUID uuid, String fileContent) throws Exception {