opendev/gerrit
Code Issues Proposed changes

Merge "Add config option to prevent group updates while migrating groups"

Browse Source
This commit is contained in:
Dave Borowitz
2017-11-16 14:02:38 +00:00
committed by Gerrit Code Review
parent 75fa62c694 809de7e70c
commit a0c379f157
3 changed files with 105 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
java/com/google/gerrit/server/group/db/GroupsUpdate.java
View File

@@ -106,6 +106,7 @@ public class GroupsUpdate {
private final MetaDataUpdateFactory metaDataUpdateFactory;
private final GitReferenceUpdated gitRefUpdated;
private final boolean writeGroupsToNoteDb;
private final boolean reviewDbUpdatesAreBlocked;
@Inject
GroupsUpdate(
@@ -143,6 +144,7 @@ public class GroupsUpdate {
// Don't flip this flag in a production setting! We only added it to spread the implementation
// of groups in NoteDb among several changes which are gradually merged.
writeGroupsToNoteDb = config.getBoolean("user", null, "writeGroupsToNoteDb", false);
reviewDbUpdatesAreBlocked = config.getBoolean("user", null, "blockReviewDbGroupUpdates", false);
}
private static MetaDataUpdateFactory getMetaDataUpdateFactory(
@@ -255,6 +257,7 @@ public class GroupsUpdate {
private InternalGroup createGroupInReviewDb(
ReviewDb db, InternalGroupCreation groupCreation, InternalGroupUpdate groupUpdate)
throws OrmException {
checkIfReviewDbUpdatesAreBlocked();
AccountGroupName gn = new AccountGroupName(groupCreation.getNameKey(), groupCreation.getId());
// first insert the group name to validate that the group name hasn't
@@ -294,6 +297,8 @@ public class GroupsUpdate {
private UpdateResult updateGroupInReviewDb(
ReviewDb db, AccountGroup group, InternalGroupUpdate groupUpdate) throws OrmException {
checkIfReviewDbUpdatesAreBlocked();
AccountGroup.NameKey originalName = group.getNameKey();
applyUpdate(group, groupUpdate);
AccountGroup.NameKey updatedName = group.getNameKey();
@@ -618,6 +623,12 @@ public class GroupsUpdate {
}
}
private void checkIfReviewDbUpdatesAreBlocked() throws OrmException {
if (reviewDbUpdatesAreBlocked) {
throw new OrmException("Updates to groups in ReviewDb are blocked");
}
}
@FunctionalInterface
private interface MetaDataUpdateFactory {
MetaDataUpdate create(

95
javatests/com/google/gerrit/acceptance/api/group/GroupRebuilderIT.java
View File

@@ -87,10 +87,12 @@ public class GroupRebuilderIT extends AbstractDaemonTest {
@Test
public void basicGroupProperties() throws Exception {
GroupInfo createdGroup = gApi.groups().create(name("group")).get();
InternalGroup reviewDbGroup = groups.getGroup(db, new AccountGroup.UUID(createdGroup.id)).get();
deleteGroupRefs(reviewDbGroup);
assertThat(removeRefState(rebuild(reviewDbGroup))).isEqualTo(roundToSecond(reviewDbGroup));
try (BlockReviewDbUpdatesForGroups ctx = new BlockReviewDbUpdatesForGroups()) {
InternalGroup reviewDbGroup =
groups.getGroup(db, new AccountGroup.UUID(createdGroup.id)).get();
deleteGroupRefs(reviewDbGroup);
assertThat(removeRefState(rebuild(reviewDbGroup))).isEqualTo(roundToSecond(reviewDbGroup));
}
}
@Test
@@ -105,46 +107,48 @@ public class GroupRebuilderIT extends AbstractDaemonTest {
gApi.groups().id(group1.id).addGroups(group2.id);
InternalGroup reviewDbGroup = groups.getGroup(db, new AccountGroup.UUID(group1.id)).get();
deleteGroupRefs(reviewDbGroup);
try (BlockReviewDbUpdatesForGroups ctx = new BlockReviewDbUpdatesForGroups()) {
InternalGroup reviewDbGroup = groups.getGroup(db, new AccountGroup.UUID(group1.id)).get();
deleteGroupRefs(reviewDbGroup);
InternalGroup noteDbGroup = rebuild(reviewDbGroup);
assertThat(removeRefState(noteDbGroup)).isEqualTo(roundToSecond(reviewDbGroup));
InternalGroup noteDbGroup = rebuild(reviewDbGroup);
assertThat(removeRefState(noteDbGroup)).isEqualTo(roundToSecond(reviewDbGroup));
ImmutableList<CommitInfo> log = log(group1);
assertThat(log).hasSize(4);
ImmutableList<CommitInfo> log = log(group1);
assertThat(log).hasSize(4);
assertThat(log.get(0)).message().isEqualTo("Create group");
assertThat(log.get(0)).author().name().isEqualTo(serverIdent.get().getName());
assertThat(log.get(0)).author().email().isEqualTo(serverIdent.get().getEmailAddress());
assertThat(log.get(0)).author().date().isEqualTo(noteDbGroup.getCreatedOn());
assertThat(log.get(0)).author().tz().isEqualTo(serverIdent.get().getTimeZoneOffset());
assertThat(log.get(0)).committer().isEqualTo(log.get(0).author);
assertThat(log.get(0)).message().isEqualTo("Create group");
assertThat(log.get(0)).author().name().isEqualTo(serverIdent.get().getName());
assertThat(log.get(0)).author().email().isEqualTo(serverIdent.get().getEmailAddress());
assertThat(log.get(0)).author().date().isEqualTo(noteDbGroup.getCreatedOn());
assertThat(log.get(0)).author().tz().isEqualTo(serverIdent.get().getTimeZoneOffset());
assertThat(log.get(0)).committer().isEqualTo(log.get(0).author);
assertThat(log.get(1))
.message()
.isEqualTo("Update group\n\nAdd: Administrator <" + admin.id + "@" + serverId + ">");
assertThat(log.get(1)).author().name().isEqualTo(admin.fullName);
assertThat(log.get(1)).author().email().isEqualTo(admin.id + "@" + serverId);
assertThat(log.get(1)).committer().hasSameDateAs(log.get(1).author);
assertThat(log.get(1))
.message()
.isEqualTo("Update group\n\nAdd: Administrator <" + admin.id + "@" + serverId + ">");
assertThat(log.get(1)).author().name().isEqualTo(admin.fullName);
assertThat(log.get(1)).author().email().isEqualTo(admin.id + "@" + serverId);
assertThat(log.get(1)).committer().hasSameDateAs(log.get(1).author);
assertThat(log.get(2))
.message()
.isEqualTo(
"Update group\n"
+ "\n"
+ ("Add: User <" + user.id + "@" + serverId + ">\n")
+ ("Add: User2 <" + user2.id + "@" + serverId + ">"));
assertThat(log.get(2)).author().name().isEqualTo(admin.fullName);
assertThat(log.get(2)).author().email().isEqualTo(admin.id + "@" + serverId);
assertThat(log.get(2)).committer().hasSameDateAs(log.get(2).author);
assertThat(log.get(2))
.message()
.isEqualTo(
"Update group\n"
+ "\n"
+ ("Add: User <" + user.id + "@" + serverId + ">\n")
+ ("Add: User2 <" + user2.id + "@" + serverId + ">"));
assertThat(log.get(2)).author().name().isEqualTo(admin.fullName);
assertThat(log.get(2)).author().email().isEqualTo(admin.id + "@" + serverId);
assertThat(log.get(2)).committer().hasSameDateAs(log.get(2).author);
assertThat(log.get(3))
.message()
.isEqualTo("Update group\n\nAdd-group: " + group2.name + " <" + group2.id + ">");
assertThat(log.get(3)).author().name().isEqualTo(admin.fullName);
assertThat(log.get(3)).author().email().isEqualTo(admin.id + "@" + serverId);
assertThat(log.get(3)).committer().hasSameDateAs(log.get(3).author);
assertThat(log.get(3))
.message()
.isEqualTo("Update group\n\nAdd-group: " + group2.name + " <" + group2.id + ">");
assertThat(log.get(3)).author().name().isEqualTo(admin.fullName);
assertThat(log.get(3)).author().email().isEqualTo(admin.id + "@" + serverId);
assertThat(log.get(3)).committer().hasSameDateAs(log.get(3).author);
}
}
private static InternalGroup removeRefState(InternalGroup group) throws Exception {
@@ -209,4 +213,19 @@ public class GroupRebuilderIT extends AbstractDaemonTest {
assertThat(commitDates).named("commit timestamps for %s", result).isOrdered();
return result.build();
}
private class BlockReviewDbUpdatesForGroups implements AutoCloseable {
BlockReviewDbUpdatesForGroups() {
blockReviewDbUpdates(true);
}
@Override
public void close() throws Exception {
blockReviewDbUpdates(false);
}
private void blockReviewDbUpdates(boolean block) {
cfg.setBoolean("user", null, "readGroupsFromNoteDb", block);
}
}
}

37
javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
View File

@@ -24,6 +24,7 @@ import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static java.util.stream.Collectors.toList;
import com.google.common.base.Throwables;
import com.google.common.cache.LoadingCache;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
@@ -56,6 +57,7 @@ import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.Account;
@@ -73,6 +75,7 @@ import com.google.gerrit.server.index.group.GroupIndexer;
import com.google.gerrit.server.index.group.StalenessChecker;
import com.google.gerrit.server.util.MagicBranch;
import com.google.gerrit.testing.ConfigSuite;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.name.Named;
import java.io.IOException;
@@ -998,6 +1001,40 @@ public class GroupsIT extends AbstractDaemonTest {
allUsers, RefNames.REFS_GROUPS + "*", Permission.READ, groupRef(REGISTERED_USERS));
}
@Test
@Sandboxed
public void blockReviewDbUpdatesOnGroupCreation() throws Exception {
assume().that(groupsInNoteDb()).isFalse();
cfg.setBoolean("user", null, "blockReviewDbGroupUpdates", true);
try {
gApi.groups().create(name("foo"));
fail("Expected RestApiException: Updates to groups in ReviewDb are blocked");
} catch (RestApiException e) {
assertWriteGroupToReviewDbBlockedException(e);
}
}
@Test
@Sandboxed
public void blockReviewDbUpdatesOnGroupUpdate() throws Exception {
assume().that(groupsInNoteDb()).isFalse();
String group1 = gApi.groups().create(name("foo")).get().id;
String group2 = gApi.groups().create(name("bar")).get().id;
cfg.setBoolean("user", null, "blockReviewDbGroupUpdates", true);
try {
gApi.groups().id(group1).addGroups(group2);
fail("Expected RestApiException: Updates to groups in ReviewDb are blocked");
} catch (RestApiException e) {
assertWriteGroupToReviewDbBlockedException(e);
}
}
private void assertWriteGroupToReviewDbBlockedException(Exception e) throws Exception {
Throwable t = Throwables.getRootCause(e);
assertThat(t).isInstanceOf(OrmException.class);
assertThat(t.getMessage()).isEqualTo("Updates to groups in ReviewDb are blocked");
}
private GroupReference groupRef(AccountGroup.UUID groupUuid) {
GroupDescription.Basic groupDescription = groupBackend.get(groupUuid);
return new GroupReference(groupDescription.getGroupUUID(), groupDescription.getName());