BatchUpdate: Support executing ops by different users
RebaseChain uses BatchUpdate to rebase the changes of a chain within a
single transaction.
For RebaseChain we want to support rebasing on behalf of the uploader.
For this each RebaseChangeOp needs to be executed by the user that is
the uploader of the change that is being rebased. Since the changes in
the chain may have different uploaders, RebaseChangeOp's in the same
BatchUpdate may need to be executed by different users. To make this
possible we allow specifying a user when adding ops to BatchUpdate that
should be used to execute the BatchUpdateOp.
The user on whom's behalf an op is executed is provided via the Context.
This means we need to create a new Context instance whenever an op is
executed, at least if the user for an op differs from the user that was
used to execute the previous op.
Change updates for the same change, patch set and user are still
squashed together if they are executed consecutively. This is enabled by
reusing the ChangeContextImpl for a BatchUpdateOp if it has the same
change and user as the previously execute BatchUpdateOp (so that
ChangeContext#getUpdate can return the same ChangeUpdate instance if it
is invoked multiple times for the same patch set).
Release-Notes: skip
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I91b502171e8f08135fcee972b05faab3f901c7a1
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/java/com/google/gerrit/server/update/BatchUpdate.java b/java/com/google/gerrit/server/update/BatchUpdate.java
index 2572271..745c918 100644
--- a/java/com/google/gerrit/server/update/BatchUpdate.java
+++ b/java/com/google/gerrit/server/update/BatchUpdate.java
@@ -23,6 +23,7 @@
import static java.util.stream.Collectors.toMap;
import static java.util.stream.Collectors.toSet;
+import com.google.auto.value.AutoValue;
import com.google.common.base.Throwables;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableList;
@@ -50,6 +51,7 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
+import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.extensions.events.AttentionSetObserver;
@@ -243,6 +245,12 @@
}
class ContextImpl implements Context {
+ private final CurrentUser contextUser;
+
+ ContextImpl(@Nullable CurrentUser contextUser) {
+ this.contextUser = contextUser != null ? contextUser : user;
+ }
+
@Override
public RepoView getRepoView() throws IOException {
return BatchUpdate.this.getRepoView();
@@ -270,7 +278,7 @@
@Override
public CurrentUser getUser() {
- return user;
+ return contextUser;
}
@Override
@@ -281,6 +289,10 @@
}
private class RepoContextImpl extends ContextImpl implements RepoContext {
+ RepoContextImpl(@Nullable CurrentUser contextUser) {
+ super(contextUser);
+ }
+
@Override
public ObjectInserter getInserter() throws IOException {
return getRepoView().getInserterWrapper();
@@ -310,7 +322,8 @@
private boolean deleted;
- ChangeContextImpl(ChangeNotes notes) {
+ ChangeContextImpl(@Nullable CurrentUser contextUser, ChangeNotes notes) {
+ super(contextUser);
this.notes = requireNonNull(notes);
defaultUpdates = new TreeMap<>(comparing(PatchSet.Id::get));
distinctUpdates = ArrayListMultimap.create();
@@ -334,7 +347,7 @@
}
private ChangeUpdate getNewChangeUpdate(PatchSet.Id psId) {
- ChangeUpdate u = changeUpdateFactory.create(notes, user, when);
+ ChangeUpdate u = changeUpdateFactory.create(notes, getUser(), getWhen());
if (newChanges.containsKey(notes.getChangeId())) {
u.setAllowWriteToNewRef(true);
}
@@ -356,7 +369,9 @@
private class PostUpdateContextImpl extends ContextImpl implements PostUpdateContext {
private final Map<Change.Id, ChangeData> changeDatas;
- PostUpdateContextImpl(Map<Change.Id, ChangeData> changeDatas) {
+ PostUpdateContextImpl(
+ @Nullable CurrentUser contextUser, Map<Change.Id, ChangeData> changeDatas) {
+ super(contextUser);
this.changeDatas = changeDatas;
}
@@ -385,6 +400,7 @@
}
private final GitRepositoryManager repoManager;
+ private final AccountCache accountCache;
private final ChangeData.Factory changeDataFactory;
private final ChangeNotes.Factory changeNotesFactory;
private final ChangeUpdate.Factory changeUpdateFactory;
@@ -397,10 +413,10 @@
private final Instant when;
private final ZoneId zoneId;
- private final ListMultimap<Change.Id, BatchUpdateOp> ops =
+ private final ListMultimap<Change.Id, OpData<BatchUpdateOp>> ops =
MultimapBuilder.linkedHashKeys().arrayListValues().build();
private final Map<Change.Id, Change> newChanges = new HashMap<>();
- private final List<RepoOnlyOp> repoOnlyOps = new ArrayList<>();
+ private final List<OpData<RepoOnlyOp>> repoOnlyOps = new ArrayList<>();
private final Map<Change.Id, NotifyHandling> perChangeNotifyHandling = new HashMap<>();
private RepoView repoView;
@@ -419,6 +435,7 @@
BatchUpdate(
GitRepositoryManager repoManager,
@GerritPersonIdent PersonIdent serverIdent,
+ AccountCache accountCache,
ChangeData.Factory changeDataFactory,
ChangeNotes.Factory changeNotesFactory,
ChangeUpdate.Factory changeUpdateFactory,
@@ -430,6 +447,7 @@
@Assisted CurrentUser user,
@Assisted Instant when) {
this.repoManager = repoManager;
+ this.accountCache = accountCache;
this.changeDataFactory = changeDataFactory;
this.changeNotesFactory = changeNotesFactory;
this.changeUpdateFactory = changeUpdateFactory;
@@ -549,48 +567,72 @@
toMap(entry -> BranchNameKey.create(project, entry.getKey()), Map.Entry::getValue));
}
+ /**
+ * Adds a {@link BatchUpdate} for a change.
+ *
+ * <p>The op is executed by the user for which the {@link BatchUpdate} has been created.
+ */
public BatchUpdate addOp(Change.Id id, BatchUpdateOp op) {
checkArgument(!(op instanceof InsertChangeOp), "use insertChange");
requireNonNull(op);
- ops.put(id, op);
+ ops.put(id, OpData.create(op, user));
return this;
}
+ /** Adds a {@link BatchUpdate} for a change that should be executed by the given context user. */
+ public BatchUpdate addOp(Change.Id id, CurrentUser contextUser, BatchUpdateOp op) {
+ checkArgument(!(op instanceof InsertChangeOp), "use insertChange");
+ requireNonNull(op);
+ ops.put(id, OpData.create(op, contextUser));
+ return this;
+ }
+
+ /**
+ * Adds a {@link RepoOnlyOp}.
+ *
+ * <p>The op is executed by the user for which the {@link BatchUpdate} has been created.
+ */
public BatchUpdate addRepoOnlyOp(RepoOnlyOp op) {
checkArgument(!(op instanceof BatchUpdateOp), "use addOp()");
- repoOnlyOps.add(op);
+ repoOnlyOps.add(OpData.create(op, user));
+ return this;
+ }
+
+ /** Adds a {@link RepoOnlyOp} that should be executed by the given context user. */
+ public BatchUpdate addRepoOnlyOp(CurrentUser contextUser, RepoOnlyOp op) {
+ checkArgument(!(op instanceof BatchUpdateOp), "use addOp()");
+ repoOnlyOps.add(OpData.create(op, contextUser));
return this;
}
public BatchUpdate insertChange(InsertChangeOp op) throws IOException {
- Context ctx = new ContextImpl();
+ Context ctx = new ContextImpl(user);
Change c = op.createChange(ctx);
checkArgument(
!newChanges.containsKey(c.getId()), "only one op allowed to create change %s", c.getId());
newChanges.put(c.getId(), c);
- ops.get(c.getId()).add(0, op);
+ ops.get(c.getId()).add(0, OpData.create(op, user));
return this;
}
private void executeUpdateRepo() throws UpdateException, RestApiException {
try {
logDebug("Executing updateRepo on %d ops", ops.size());
- RepoContextImpl ctx = new RepoContextImpl();
- for (Map.Entry<Change.Id, BatchUpdateOp> op : ops.entries()) {
+ for (Map.Entry<Change.Id, OpData<BatchUpdateOp>> e : ops.entries()) {
+ BatchUpdateOp op = e.getValue().op();
+ RepoContextImpl ctx = new RepoContextImpl(e.getValue().user());
try (TraceContext.TraceTimer ignored =
TraceContext.newTimer(
- op.getValue().getClass().getSimpleName() + "#updateRepo",
- Metadata.builder()
- .projectName(project.get())
- .changeId(op.getKey().get())
- .build())) {
- op.getValue().updateRepo(ctx);
+ op.getClass().getSimpleName() + "#updateRepo",
+ Metadata.builder().projectName(project.get()).changeId(e.getKey().get()).build())) {
+ op.updateRepo(ctx);
}
}
logDebug("Executing updateRepo on %d RepoOnlyOps", repoOnlyOps.size());
- for (RepoOnlyOp op : repoOnlyOps) {
- op.updateRepo(ctx);
+ for (OpData<RepoOnlyOp> opData : repoOnlyOps) {
+ RepoContextImpl ctx = new RepoContextImpl(opData.user());
+ opData.op().updateRepo(ctx);
}
if (onSubmitValidators != null && !getRefUpdates().isEmpty()) {
@@ -599,7 +641,7 @@
// first update's executeRefUpdates has finished, hence after first repo's refs have been
// updated, which is too late.
onSubmitValidators.validate(
- project, ctx.getRevWalk().getObjectReader(), repoView.getCommands());
+ project, getRepoView().getRevWalk().getObjectReader(), repoView.getCommands());
}
} catch (Exception e) {
Throwables.throwIfInstanceOf(e, RestApiException.class);
@@ -613,12 +655,14 @@
}
}
- private void fireAttentionSetUpdateEvents(PostUpdateContext ctx) {
+ private void fireAttentionSetUpdateEvents(Map<Change.Id, ChangeData> changeDatas) {
for (ProjectChangeKey key : attentionSetUpdates.keySet()) {
- ChangeData change = ctx.getChangeData(key.projectName(), key.changeId());
- AccountState account = ctx.getAccount();
+ ChangeData change =
+ changeDatas.computeIfAbsent(
+ key.changeId(), id -> changeDataFactory.create(key.projectName(), key.changeId()));
for (AttentionSetUpdate update : attentionSetUpdates.get(key)) {
- attentionSetObserver.fire(change, account, update, ctx.getWhen());
+ attentionSetObserver.fire(
+ change, accountCache.getEvenIfMissing(update.account()), update, when);
}
}
}
@@ -709,31 +753,45 @@
}
handle.manager.setRefLogMessage(refLogMessage);
handle.manager.setPushCertificate(pushCert);
- for (Map.Entry<Change.Id, Collection<BatchUpdateOp>> e : ops.asMap().entrySet()) {
+ for (Map.Entry<Change.Id, Collection<OpData<BatchUpdateOp>>> e : ops.asMap().entrySet()) {
Change.Id id = e.getKey();
- ChangeContextImpl ctx = newChangeContext(id);
boolean dirty = false;
+ boolean deleted = false;
+ List<ChangeUpdate> changeUpdates = new ArrayList<>();
+ ChangeContextImpl ctx = null;
logDebug(
"Applying %d ops for change %s: %s",
e.getValue().size(),
id,
lazy(() -> e.getValue().stream().map(op -> op.getClass().getName()).collect(toSet())));
- for (BatchUpdateOp op : e.getValue()) {
+ for (OpData<BatchUpdateOp> opData : e.getValue()) {
+ if (ctx == null) {
+ ctx = newChangeContext(opData.user(), id);
+ } else if (!ctx.getUser().equals(opData.user())) {
+ ctx.defaultUpdates.values().forEach(changeUpdates::add);
+ ctx.distinctUpdates.values().forEach(changeUpdates::add);
+ ctx = newChangeContext(opData.user(), id);
+ }
try (TraceContext.TraceTimer ignored =
TraceContext.newTimer(
- op.getClass().getSimpleName() + "#updateChange",
+ opData.getClass().getSimpleName() + "#updateChange",
Metadata.builder().projectName(project.get()).changeId(id.get()).build())) {
- dirty |= op.updateChange(ctx);
+ dirty |= opData.op().updateChange(ctx);
+ deleted |= ctx.deleted;
}
}
+ if (ctx != null) {
+ ctx.defaultUpdates.values().forEach(changeUpdates::add);
+ ctx.distinctUpdates.values().forEach(changeUpdates::add);
+ }
+
if (!dirty) {
logDebug("No ops reported dirty, short-circuiting");
handle.setResult(id, ChangeResult.SKIPPED);
continue;
}
- ctx.defaultUpdates.values().forEach(handle.manager::add);
- ctx.distinctUpdates.values().forEach(handle.manager::add);
- if (ctx.deleted) {
+ changeUpdates.forEach(handle.manager::add);
+ if (deleted) {
logDebug("Change %s was deleted", id);
handle.manager.deleteChange(id);
handle.setResult(id, ChangeResult.DELETED);
@@ -744,7 +802,7 @@
return handle;
}
- private ChangeContextImpl newChangeContext(Change.Id id) {
+ private ChangeContextImpl newChangeContext(@Nullable CurrentUser contextUser, Change.Id id) {
logDebug("Opening change %s for update", id);
Change c = newChanges.get(id);
boolean isNew = c != null;
@@ -757,27 +815,30 @@
logDebug("Change %s is new", id);
}
ChangeNotes notes = changeNotesFactory.createForBatchUpdate(c, !isNew);
- return new ChangeContextImpl(notes);
+ return new ChangeContextImpl(contextUser, notes);
}
private void executePostOps(Map<Change.Id, ChangeData> changeDatas) throws Exception {
- PostUpdateContextImpl ctx = new PostUpdateContextImpl(changeDatas);
- for (BatchUpdateOp op : ops.values()) {
+ for (OpData<BatchUpdateOp> opData : ops.values()) {
+ PostUpdateContextImpl ctx = new PostUpdateContextImpl(opData.user(), changeDatas);
try (TraceContext.TraceTimer ignored =
- TraceContext.newTimer(op.getClass().getSimpleName() + "#postUpdate", Metadata.empty())) {
- op.postUpdate(ctx);
+ TraceContext.newTimer(
+ opData.getClass().getSimpleName() + "#postUpdate", Metadata.empty())) {
+ opData.op().postUpdate(ctx);
}
}
- for (RepoOnlyOp op : repoOnlyOps) {
+ for (OpData<RepoOnlyOp> opData : repoOnlyOps) {
+ PostUpdateContextImpl ctx = new PostUpdateContextImpl(opData.user(), changeDatas);
try (TraceContext.TraceTimer ignored =
- TraceContext.newTimer(op.getClass().getSimpleName() + "#postUpdate", Metadata.empty())) {
- op.postUpdate(ctx);
+ TraceContext.newTimer(
+ opData.getClass().getSimpleName() + "#postUpdate", Metadata.empty())) {
+ opData.op().postUpdate(ctx);
}
}
try (TraceContext.TraceTimer ignored =
TraceContext.newTimer("fireAttentionSetUpdates#postUpdate", Metadata.empty())) {
- fireAttentionSetUpdateEvents(ctx);
+ fireAttentionSetUpdateEvents(changeDatas);
}
}
@@ -808,4 +869,18 @@
logger.atFine().log(msg, arg1, arg2, arg3);
}
}
+
+ /** Data needed to execute a {@link RepoOnlyOp} or a {@link BatchUpdateOp}. */
+ @AutoValue
+ abstract static class OpData<T extends RepoOnlyOp> {
+ /** Op that should be executed. */
+ abstract T op();
+
+ /** User that should be used to execute the {@link #op}. */
+ abstract CurrentUser user();
+
+ static <T extends RepoOnlyOp> OpData<T> create(T op, CurrentUser user) {
+ return new AutoValue_BatchUpdate_OpData<>(op, user);
+ }
+ }
}
diff --git a/javatests/com/google/gerrit/server/update/BatchUpdateTest.java b/javatests/com/google/gerrit/server/update/BatchUpdateTest.java
index 07159b7..3066d9e 100644
--- a/javatests/com/google/gerrit/server/update/BatchUpdateTest.java
+++ b/javatests/com/google/gerrit/server/update/BatchUpdateTest.java
@@ -38,6 +38,7 @@
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountManager;
import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.change.AddReviewersOp;
@@ -46,6 +47,7 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.notedb.Sequences;
import com.google.gerrit.server.patch.DiffSummary;
import com.google.gerrit.server.patch.DiffSummaryKey;
@@ -55,6 +57,8 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.name.Named;
+import java.util.ArrayList;
+import java.util.List;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
@@ -97,6 +101,7 @@
@Inject private DynamicSet<AttentionSetListener> attentionSetListeners;
@Inject private AccountManager accountManager;
@Inject private AuthRequest.Factory authRequestFactory;
+ @Inject private IdentifiedUser.GenericFactory userFactory;
@Rule public final MockitoRule mockito = MockitoJUnit.rule();
@@ -374,6 +379,157 @@
assertThat(diffSummaryCache.asMap()).hasSize(cacheSizeBefore + 1);
}
+ @Test
+ public void executeOpsWithDifferentUsers() throws Exception {
+ Change.Id changeId = createChange();
+
+ ObjectId oldHead =
+ repo.getRepository().exactRef(RefNames.changeMetaRef(changeId)).getObjectId();
+
+ CurrentUser defaultUser = user.get();
+ IdentifiedUser user1 =
+ userFactory.create(
+ accountManager.authenticate(authRequestFactory.createForUser("user1")).getAccountId());
+ IdentifiedUser user2 =
+ userFactory.create(
+ accountManager.authenticate(authRequestFactory.createForUser("user2")).getAccountId());
+
+ TestOp testOp1 = new TestOp().addReviewer(defaultUser.getAccountId());
+ TestOp testOp2 = new TestOp().addReviewer(user1.getAccountId());
+ TestOp testOp3 = new TestOp().addReviewer(user2.getAccountId());
+
+ try (BatchUpdate bu = batchUpdateFactory.create(project, defaultUser, TimeUtil.now())) {
+ bu.addOp(changeId, user1, testOp1);
+ bu.addOp(changeId, user2, testOp2);
+ bu.addOp(changeId, testOp3);
+ bu.execute();
+ }
+
+ assertThat(testOp1.updateRepoUser).isEqualTo(user1);
+ assertThat(testOp1.updateChangeUser).isEqualTo(user1);
+ assertThat(testOp1.postUpdateUser).isEqualTo(user1);
+
+ assertThat(testOp2.updateRepoUser).isEqualTo(user2);
+ assertThat(testOp2.updateChangeUser).isEqualTo(user2);
+ assertThat(testOp2.postUpdateUser).isEqualTo(user2);
+
+ assertThat(testOp3.updateRepoUser).isEqualTo(defaultUser);
+ assertThat(testOp3.updateChangeUser).isEqualTo(defaultUser);
+ assertThat(testOp3.postUpdateUser).isEqualTo(defaultUser);
+
+ // Verify that we got one meta commit per op.
+ RevCommit metaCommitForTestOp3 =
+ repo.getRepository()
+ .parseCommit(
+ repo.getRepository().exactRef(RefNames.changeMetaRef(changeId)).getObjectId());
+ assertThat(metaCommitForTestOp3.getAuthorIdent().getEmailAddress())
+ .isEqualTo(String.format("%s@gerrit", defaultUser.getAccountId()));
+ assertThat(metaCommitForTestOp3.getFullMessage())
+ .startsWith(
+ "Update patch set 1\n"
+ + "\n"
+ + "Patch-set: 1\n"
+ + String.format(
+ "Reviewer: Gerrit User %s <%s@gerrit>\n",
+ user2.getAccountId(), user2.getAccountId())
+ + "Attention:");
+
+ RevCommit metaCommitForTestOp2 =
+ repo.getRepository().parseCommit(metaCommitForTestOp3.getParent(0));
+ assertThat(metaCommitForTestOp2.getAuthorIdent().getEmailAddress())
+ .isEqualTo(String.format("%s@gerrit", user2.getAccountId()));
+ assertThat(metaCommitForTestOp2.getFullMessage())
+ .startsWith(
+ "Update patch set 1\n"
+ + "\n"
+ + "Patch-set: 1\n"
+ + String.format(
+ "Reviewer: Gerrit User %s <%s@gerrit>\n",
+ user1.getAccountId(), user1.getAccountId())
+ + "Attention:");
+
+ RevCommit metaCommitForTestOp1 =
+ repo.getRepository().parseCommit(metaCommitForTestOp2.getParent(0));
+ assertThat(metaCommitForTestOp1.getAuthorIdent().getEmailAddress())
+ .isEqualTo(String.format("%s@gerrit", user1.getAccountId()));
+ assertThat(metaCommitForTestOp1.getFullMessage())
+ .isEqualTo(
+ "Update patch set 1\n"
+ + "\n"
+ + "Patch-set: 1\n"
+ + String.format(
+ "Reviewer: Gerrit User %s <%s@gerrit>\n",
+ defaultUser.getAccountId(), defaultUser.getAccountId()));
+
+ assertThat(metaCommitForTestOp1.getParent(0)).isEqualTo(oldHead);
+ }
+
+ @Test
+ public void executeOpsWithSameUser() throws Exception {
+ Change.Id changeId = createChange();
+
+ ObjectId oldHead =
+ repo.getRepository().exactRef(RefNames.changeMetaRef(changeId)).getObjectId();
+
+ CurrentUser defaultUser = user.get();
+ IdentifiedUser user1 =
+ userFactory.create(
+ accountManager.authenticate(authRequestFactory.createForUser("user1")).getAccountId());
+ IdentifiedUser user2 =
+ userFactory.create(
+ accountManager.authenticate(authRequestFactory.createForUser("user2")).getAccountId());
+
+ TestOp testOp1 = new TestOp().addReviewer(user1.getAccountId());
+ TestOp testOp2 = new TestOp().addReviewer(user2.getAccountId());
+
+ try (BatchUpdate bu = batchUpdateFactory.create(project, defaultUser, TimeUtil.now())) {
+ bu.addOp(changeId, defaultUser, testOp1);
+ bu.addOp(changeId, testOp2);
+ bu.execute();
+ }
+
+ assertThat(testOp1.updateRepoUser).isEqualTo(defaultUser);
+ assertThat(testOp1.updateChangeUser).isEqualTo(defaultUser);
+ assertThat(testOp1.postUpdateUser).isEqualTo(defaultUser);
+
+ assertThat(testOp2.updateRepoUser).isEqualTo(defaultUser);
+ assertThat(testOp2.updateChangeUser).isEqualTo(defaultUser);
+ assertThat(testOp2.postUpdateUser).isEqualTo(defaultUser);
+
+ // Verify that we got a single meta commit (updates of both ops squashed into one commit).
+ RevCommit metaCommit =
+ repo.getRepository()
+ .parseCommit(
+ repo.getRepository().exactRef(RefNames.changeMetaRef(changeId)).getObjectId());
+ assertThat(metaCommit.getAuthorIdent().getEmailAddress())
+ .isEqualTo(String.format("%s@gerrit", defaultUser.getAccountId()));
+ assertThat(metaCommit.getFullMessage())
+ .startsWith(
+ "Update patch set 1\n"
+ + "\n"
+ + "Patch-set: 1\n"
+ + String.format(
+ "Reviewer: Gerrit User %s <%s@gerrit>\n",
+ user1.getAccountId(), user1.getAccountId())
+ + String.format(
+ "Reviewer: Gerrit User %s <%s@gerrit>\n",
+ user2.getAccountId(), user2.getAccountId())
+ + "Attention:");
+
+ assertThat(metaCommit.getParent(0)).isEqualTo(oldHead);
+ }
+
+ private Change.Id createChange() throws Exception {
+ Change.Id id = Change.id(sequences.nextChangeId());
+ try (BatchUpdate bu = batchUpdateFactory.create(project, user.get(), TimeUtil.now())) {
+ bu.insertChange(
+ changeInserterFactory.create(
+ id, repo.commit().message("Change").insertChangeId().create(), "refs/heads/master"));
+ bu.execute();
+ }
+ return id;
+ }
+
private Change.Id createChangeWithUpdates(int totalUpdates) throws Exception {
checkArgument(totalUpdates > 0);
checkArgument(totalUpdates <= MAX_UPDATES);
@@ -468,4 +624,38 @@
return true;
}
}
+
+ private static class TestOp implements BatchUpdateOp {
+ CurrentUser updateRepoUser;
+ CurrentUser updateChangeUser;
+ CurrentUser postUpdateUser;
+
+ private List<Account.Id> reviewersToAdd = new ArrayList<>();
+
+ TestOp addReviewer(Account.Id accountId) {
+ reviewersToAdd.add(accountId);
+ return this;
+ }
+
+ @Override
+ public void updateRepo(RepoContext ctx) {
+ updateRepoUser = ctx.getUser();
+ }
+
+ @Override
+ public boolean updateChange(ChangeContext ctx) {
+ updateChangeUser = ctx.getUser();
+
+ reviewersToAdd.forEach(
+ accountId ->
+ ctx.getUpdate(ctx.getChange().currentPatchSetId())
+ .putReviewer(accountId, ReviewerStateInternal.REVIEWER));
+ return true;
+ }
+
+ @Override
+ public void postUpdate(PostUpdateContext ctx) {
+ postUpdateUser = ctx.getUser();
+ }
+ }
}
