Allow rebase with a secondary email via REST
Update the rebase REST API to add a new optional 'committer_email'.
It provides the flexibility to choose a secondary email of the
current user or latest uploader (when on_behalf_of_uploader=true)
instead of their preferred email to perform the rebase. This option
is not supported when rebasing a chain of changes. UI will be updated
in a followup change.
Release-Notes: rebase can be performed with a secondary email using REST API
Change-Id: I4007b18b460fdd341fa0abc205ea861634575a44
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index ea95680..5026c5f 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -1472,6 +1472,9 @@
Optionally, the parent revision (of the oldest ancestor to be rebased) can be changed to another
change, revision or branch through the link:#rebase-input[RebaseInput] entity.
+Providing a `committer_email` through the link:#rebase-input[RebaseInput] entity is not supported
+when rebasing a chain.
+
If the chain is outdated, i.e., there's a change that depends on an old revision of its parent, the
result is the same as individually rebasing all outdated changes on top of their parent's latest
revision before running the rebase chain action.
@@ -8343,6 +8346,10 @@
In addition, rebasing on behalf of the uploader is only supported for the
current patch set of a change. +
If the caller is the uploader this flag is ignored and a normal rebase is done.
+|`committer_email`|optional|
+Rebase is committed using this email address. Only the registered emails
+of the calling user or uploader (when `on_behalf_of_uploader` is `true`) are
+considered valid. This option is not supported when rebasing a chain.
|`validation_options` |optional|
Map with key-value pairs that are forwarded as options to the commit validation
listeners (e.g. can be used to skip certain validations). Which validation
diff --git a/java/com/google/gerrit/extensions/api/changes/RebaseInput.java b/java/com/google/gerrit/extensions/api/changes/RebaseInput.java
index 07e65d0..42dea8d 100644
--- a/java/com/google/gerrit/extensions/api/changes/RebaseInput.java
+++ b/java/com/google/gerrit/extensions/api/changes/RebaseInput.java
@@ -51,4 +51,12 @@
public boolean onBehalfOfUploader;
public Map<String, String> validationOptions;
+
+ /**
+ * Rebase will be committed using this email address. Only the registered emails of the calling
+ * user or uploader (when onBehalfOfUploader is true) are considered valid.
+ *
+ * <p>This option is not supported when rebasing a chain.
+ */
+ public String committerEmail;
}
diff --git a/java/com/google/gerrit/server/change/RebaseChangeOp.java b/java/com/google/gerrit/server/change/RebaseChangeOp.java
index f46196f..de3b7d5 100644
--- a/java/com/google/gerrit/server/change/RebaseChangeOp.java
+++ b/java/com/google/gerrit/server/change/RebaseChangeOp.java
@@ -400,6 +400,10 @@
return rebasedCommit;
}
+ public PatchSet getOriginalPatchSet() {
+ return originalPatchSet;
+ }
+
public PatchSet.Id getPatchSetId() {
checkState(rebasedPatchSetId != null, "getPatchSetId() only valid after updateRepo");
return rebasedPatchSetId;
diff --git a/java/com/google/gerrit/server/change/RebaseUtil.java b/java/com/google/gerrit/server/change/RebaseUtil.java
index 48b052f..47a1e11 100644
--- a/java/com/google/gerrit/server/change/RebaseUtil.java
+++ b/java/com/google/gerrit/server/change/RebaseUtil.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.change;
import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableSet;
import com.google.common.flogger.FluentLogger;
import com.google.common.primitives.Ints;
import com.google.gerrit.common.Nullable;
@@ -38,6 +39,7 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.ChangePermission;
+import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.RefPermission;
@@ -45,6 +47,7 @@
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.server.update.BatchUpdate;
+import com.google.gerrit.server.util.time.TimeUtil;
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.io.IOException;
@@ -62,39 +65,38 @@
private final Provider<PersonIdent> serverIdent;
private final IdentifiedUser.GenericFactory userFactory;
private final PermissionBackend permissionBackend;
- private final ChangeResource.Factory changeResourceFactory;
private final GitRepositoryManager repoManager;
private final Provider<InternalChangeQuery> queryProvider;
private final ChangeNotes.Factory notesFactory;
private final PatchSetUtil psUtil;
private final RebaseChangeOp.Factory rebaseFactory;
+ private final Provider<CurrentUser> self;
@Inject
RebaseUtil(
@GerritPersonIdent Provider<PersonIdent> serverIdent,
IdentifiedUser.GenericFactory userFactory,
PermissionBackend permissionBackend,
- ChangeResource.Factory changeResourceFactory,
GitRepositoryManager repoManager,
Provider<InternalChangeQuery> queryProvider,
ChangeNotes.Factory notesFactory,
PatchSetUtil psUtil,
- RebaseChangeOp.Factory rebaseFactory) {
+ RebaseChangeOp.Factory rebaseFactory,
+ Provider<CurrentUser> self) {
this.serverIdent = serverIdent;
this.userFactory = userFactory;
this.permissionBackend = permissionBackend;
- this.changeResourceFactory = changeResourceFactory;
this.repoManager = repoManager;
this.queryProvider = queryProvider;
this.notesFactory = notesFactory;
this.psUtil = psUtil;
this.rebaseFactory = rebaseFactory;
+ this.self = self;
}
/**
- * Checks that the uploader has permissions to create a new patch set and creates a new {@link
- * RevisionResource} that contains the uploader (aka the impersonated user) as the current user
- * which can be used for {@link BatchUpdate} to do the rebase on behalf of the uploader.
+ * Checks that the uploader has permissions to create a new patch set as the current user which
+ * can be used for {@link BatchUpdate} to do the rebase on behalf of the uploader.
*
* <p>The following permissions are required for the uploader:
*
@@ -137,10 +139,8 @@
*
* @param rsrc the revision resource that should be rebased
* @param rebaseInput the request input containing options for the rebase
- * @return revision resource that contains the uploader (aka the impersonated user) as the current
- * user which can be used for {@link BatchUpdate} to do the rebase on behalf of the uploader
*/
- public RevisionResource onBehalfOf(RevisionResource rsrc, RebaseInput rebaseInput)
+ public void checkCanRebaseOnBehalfOf(RevisionResource rsrc, RebaseInput rebaseInput)
throws IOException, PermissionBackendException, BadRequestException,
ResourceConflictException {
if (rebaseInput.allowConflicts) {
@@ -208,9 +208,6 @@
}
}
}
-
- return new RevisionResource(
- changeResourceFactory.create(rsrc.getNotes(), uploader), rsrc.getPatchSet());
}
private void checkPermissionForUploader(
@@ -538,23 +535,77 @@
return baseId;
}
- public RebaseChangeOp getRebaseOp(RevisionResource revRsrc, RebaseInput input, ObjectId baseRev) {
+ public RebaseChangeOp getRebaseOp(
+ RevWalk rw,
+ RevisionResource revRsrc,
+ RebaseInput input,
+ ObjectId baseRev,
+ IdentifiedUser rebaseAsUser)
+ throws ResourceConflictException, PermissionBackendException, IOException {
return applyRebaseInputToOp(
- rebaseFactory.create(revRsrc.getNotes(), revRsrc.getPatchSet(), baseRev), input);
+ rw,
+ rebaseFactory.create(revRsrc.getNotes(), revRsrc.getPatchSet(), baseRev),
+ input,
+ rebaseAsUser);
}
public RebaseChangeOp getRebaseOp(
- RevisionResource revRsrc, RebaseInput input, Change.Id baseChange) {
+ RevWalk rw,
+ RevisionResource revRsrc,
+ RebaseInput input,
+ Change.Id baseChange,
+ IdentifiedUser rebaseAsUser)
+ throws ResourceConflictException, PermissionBackendException, IOException {
return applyRebaseInputToOp(
- rebaseFactory.create(revRsrc.getNotes(), revRsrc.getPatchSet(), baseChange), input);
+ rw,
+ rebaseFactory.create(revRsrc.getNotes(), revRsrc.getPatchSet(), baseChange),
+ input,
+ rebaseAsUser);
}
- private RebaseChangeOp applyRebaseInputToOp(RebaseChangeOp op, RebaseInput input) {
- return op.setForceContentMerge(true)
- .setAllowConflicts(input.allowConflicts)
- .setMergeStrategy(input.strategy)
- .setValidationOptions(
- ValidationOptionsUtil.getValidateOptionsAsMultimap(input.validationOptions))
- .setFireRevisionCreated(true);
+ private RebaseChangeOp applyRebaseInputToOp(
+ RevWalk rw, RebaseChangeOp op, RebaseInput input, IdentifiedUser rebaseAsUser)
+ throws ResourceConflictException, PermissionBackendException, IOException {
+ RebaseChangeOp rebaseChangeOp =
+ op.setForceContentMerge(true)
+ .setAllowConflicts(input.allowConflicts)
+ .setMergeStrategy(input.strategy)
+ .setValidationOptions(
+ ValidationOptionsUtil.getValidateOptionsAsMultimap(input.validationOptions))
+ .setFireRevisionCreated(true);
+
+ String originalPatchSetCommitterEmail =
+ rw.parseCommit(rebaseChangeOp.getOriginalPatchSet().commitId())
+ .getCommitterIdent()
+ .getEmailAddress();
+
+ if (input.committerEmail != null) {
+ if (!self.get().hasSameAccountId(rebaseAsUser)
+ && !input.committerEmail.equals(rebaseAsUser.getAccount().preferredEmail())
+ && !input.committerEmail.equals(originalPatchSetCommitterEmail)
+ && !permissionBackend.currentUser().test(GlobalPermission.VIEW_SECONDARY_EMAILS)) {
+ throw new ResourceConflictException(
+ String.format(
+ "Cannot rebase using committer email '%s'. It can only be done using the "
+ + "preferred email or the committer email of the uploader",
+ input.committerEmail));
+ }
+
+ ImmutableSet<String> emails = rebaseAsUser.getEmailAddresses();
+ if (!emails.contains(input.committerEmail)) {
+ throw new ResourceConflictException(
+ String.format(
+ "Cannot rebase using committer email '%s' as it is not a registered "
+ + "email of the user on whose behalf the rebase operation is performed",
+ input.committerEmail));
+ }
+ rebaseChangeOp.setCommitterIdent(
+ new PersonIdent(
+ rebaseAsUser.getName(),
+ input.committerEmail,
+ TimeUtil.now(),
+ serverIdent.get().getZoneId()));
+ }
+ return rebaseChangeOp;
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/Rebase.java b/java/com/google/gerrit/server/restapi/change/Rebase.java
index 167f784..98a3f83 100644
--- a/java/com/google/gerrit/server/restapi/change/Rebase.java
+++ b/java/com/google/gerrit/server/restapi/change/Rebase.java
@@ -29,6 +29,7 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.webui.UiAction;
+import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.change.ChangeResource;
@@ -68,6 +69,7 @@
private final ProjectCache projectCache;
private final PatchSetUtil patchSetUtil;
private final RebaseMetrics rebaseMetrics;
+ private final IdentifiedUser.GenericFactory userFactory;
@Inject
public Rebase(
@@ -78,7 +80,8 @@
PermissionBackend permissionBackend,
ProjectCache projectCache,
PatchSetUtil patchSetUtil,
- RebaseMetrics rebaseMetrics) {
+ RebaseMetrics rebaseMetrics,
+ IdentifiedUser.GenericFactory userFactory) {
this.updateFactory = updateFactory;
this.repoManager = repoManager;
this.rebaseUtil = rebaseUtil;
@@ -87,16 +90,20 @@
this.projectCache = projectCache;
this.patchSetUtil = patchSetUtil;
this.rebaseMetrics = rebaseMetrics;
+ this.userFactory = userFactory;
}
@Override
public Response<ChangeInfo> apply(RevisionResource rsrc, RebaseInput input)
throws UpdateException, RestApiException, IOException, PermissionBackendException {
-
+ IdentifiedUser rebaseAsUser;
if (input.onBehalfOfUploader && !rsrc.getPatchSet().uploader().equals(rsrc.getAccountId())) {
+ rebaseAsUser =
+ userFactory.runAs(/*remotePeer= */ null, rsrc.getPatchSet().uploader(), rsrc.getUser());
rsrc.permissions().check(ChangePermission.REBASE_ON_BEHALF_OF_UPLOADER);
- rsrc = rebaseUtil.onBehalfOf(rsrc, input);
+ rebaseUtil.checkCanRebaseOnBehalfOf(rsrc, input);
} else {
+ rebaseAsUser = rsrc.getUser().asIdentifiedUser();
input.onBehalfOfUploader = false;
rsrc.permissions().check(ChangePermission.REBASE);
}
@@ -113,14 +120,16 @@
ObjectReader reader = oi.newReader();
RevWalk rw = CodeReviewCommit.newRevWalk(reader);
BatchUpdate bu =
- updateFactory.create(change.getProject(), rsrc.getUser(), TimeUtil.now())) {
+ updateFactory.create(change.getProject(), rebaseAsUser, TimeUtil.now())) {
rebaseUtil.verifyRebasePreconditions(rw, rsrc.getNotes(), rsrc.getPatchSet());
RebaseChangeOp rebaseOp =
rebaseUtil.getRebaseOp(
+ rw,
rsrc,
input,
- rebaseUtil.parseOrFindBaseRevision(repo, rw, permissionBackend, rsrc, input, true));
+ rebaseUtil.parseOrFindBaseRevision(repo, rw, permissionBackend, rsrc, input, true),
+ rebaseAsUser);
// TODO(dborowitz): Why no notification? This seems wrong; dig up blame.
bu.setNotify(NotifyResolver.Result.none());
diff --git a/java/com/google/gerrit/server/restapi/change/RebaseChain.java b/java/com/google/gerrit/server/restapi/change/RebaseChain.java
index 343fb72..76c5253 100644
--- a/java/com/google/gerrit/server/restapi/change/RebaseChain.java
+++ b/java/com/google/gerrit/server/restapi/change/RebaseChain.java
@@ -36,6 +36,7 @@
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.change.ChangeResource;
@@ -89,6 +90,7 @@
private final PatchSetUtil patchSetUtil;
private final ChangeJson.Factory json;
private final RebaseMetrics rebaseMetrics;
+ private final IdentifiedUser.GenericFactory userFactory;
@Inject
RebaseChain(
@@ -103,7 +105,8 @@
ProjectCache projectCache,
PatchSetUtil patchSetUtil,
ChangeJson.Factory json,
- RebaseMetrics rebaseMetrics) {
+ RebaseMetrics rebaseMetrics,
+ IdentifiedUser.GenericFactory userFactory) {
this.repoManager = repoManager;
this.getRelatedChangesUtil = getRelatedChangesUtil;
this.changeDataFactory = changeDataFactory;
@@ -116,11 +119,18 @@
this.patchSetUtil = patchSetUtil;
this.json = json;
this.rebaseMetrics = rebaseMetrics;
+ this.userFactory = userFactory;
}
@Override
public Response<RebaseChainInfo> apply(ChangeResource tipRsrc, RebaseInput input)
throws IOException, PermissionBackendException, RestApiException, UpdateException {
+ IdentifiedUser rebaseAsUser;
+ if (input.committerEmail != null) {
+ // TODO: committer_email can be supported if all changes in the chain
+ // belong to the same uploader. It can be attempted in future as needed.
+ throw new BadRequestException("committer_email is not supported when rebasing a chain");
+ }
if (input.onBehalfOfUploader) {
tipRsrc.permissions().check(ChangePermission.REBASE_ON_BEHALF_OF_UPLOADER);
if (input.allowConflicts) {
@@ -160,10 +170,14 @@
new RevisionResource(changeResourceFactory.create(changeData, user), ps);
if (input.onBehalfOfUploader
&& !revRsrc.getPatchSet().uploader().equals(revRsrc.getAccountId())) {
- revRsrc = rebaseUtil.onBehalfOf(revRsrc, input);
+ rebaseAsUser =
+ userFactory.runAs(
+ /*remotePeer= */ null, revRsrc.getPatchSet().uploader(), revRsrc.getUser());
+ rebaseUtil.checkCanRebaseOnBehalfOf(revRsrc, input);
revRsrc.permissions().check(ChangePermission.REBASE_ON_BEHALF_OF_UPLOADER);
anyRebaseOnBehalfOfUploader = true;
} else {
+ rebaseAsUser = revRsrc.getUser().asIdentifiedUser();
revRsrc.permissions().check(ChangePermission.REBASE);
}
rebaseUtil.verifyRebasePreconditions(rw, changeData.notes(), ps);
@@ -177,7 +191,7 @@
if (currentBase(rw, ps).equals(desiredBase)) {
isUpToDate = true;
} else {
- rebaseOp = rebaseUtil.getRebaseOp(revRsrc, input, desiredBase);
+ rebaseOp = rebaseUtil.getRebaseOp(rw, revRsrc, input, desiredBase, rebaseAsUser);
}
} else {
if (ancestorsAreUpToDate) {
@@ -187,7 +201,8 @@
isUpToDate = currentBase(rw, ps).equals(latestCommittedBase);
}
if (!isUpToDate) {
- rebaseOp = rebaseUtil.getRebaseOp(revRsrc, input, chain.get(i - 1).id());
+ rebaseOp =
+ rebaseUtil.getRebaseOp(rw, revRsrc, input, chain.get(i - 1).id(), rebaseAsUser);
}
}
@@ -196,7 +211,7 @@
continue;
}
ancestorsAreUpToDate = false;
- bu.addOp(revRsrc.getChange().getId(), revRsrc.getUser(), rebaseOp);
+ bu.addOp(revRsrc.getChange().getId(), rebaseAsUser, rebaseOp);
rebaseOps.put(revRsrc.getChange().getId(), rebaseOp);
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/RebaseChainOnBehalfOfUploaderIT.java b/javatests/com/google/gerrit/acceptance/api/change/RebaseChainOnBehalfOfUploaderIT.java
index 7d1ddfc..297579c 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/RebaseChainOnBehalfOfUploaderIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/RebaseChainOnBehalfOfUploaderIT.java
@@ -100,6 +100,22 @@
}
@Test
+ public void cannotRebaseOnBehalfOfUploaderWithCommitterEmail() throws Exception {
+ Account.Id uploader = accountOperations.newAccount().create();
+ Change.Id changeId = changeOperations.newChange().owner(uploader).create();
+ RebaseInput rebaseInput = new RebaseInput();
+ rebaseInput.onBehalfOfUploader = true;
+ rebaseInput.committerEmail = "admin@example.com";
+ BadRequestException exception =
+ assertThrows(
+ BadRequestException.class,
+ () -> gApi.changes().id(changeId.get()).rebaseChain(rebaseInput));
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo("committer_email is not supported when rebasing a chain");
+ }
+
+ @Test
public void rebaseChangeOnBehalfOfUploader_withRebasePermission() throws Exception {
testRebaseChainOnBehalfOfUploader(Permission.REBASE);
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java b/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java
index d9b079a..c637916 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java
@@ -63,6 +63,7 @@
import com.google.gerrit.extensions.common.RevisionInfo;
import com.google.gerrit.extensions.events.WorkInProgressStateChangedListener;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
@@ -144,6 +145,82 @@
}
@Test
+ public void rebaseWithCommitterEmail() throws Exception {
+ // Create three changes with the same parent
+ PushOneCommit.Result r1 = createChange();
+ testRepo.reset("HEAD~1");
+ PushOneCommit.Result r2 = createChange();
+ testRepo.reset("HEAD~1");
+ PushOneCommit.Result r3 = createChange();
+
+ // Create new user with a secondary email and with permission to rebase
+ Account.Id userWithSecondaryEmail =
+ accountOperations
+ .newAccount()
+ .preferredEmail("preferred@domain.org")
+ .addSecondaryEmail("secondary@domain.org")
+ .create();
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.REBASE).ref("refs/heads/master").group(REGISTERED_USERS))
+ .update();
+
+ // Approve and submit the r1
+ RevisionApi revision = gApi.changes().id(r1.getChangeId()).current();
+ revision.review(ReviewInput.approve());
+ revision.submit();
+
+ // Rebase r2 as the new user with its primary email
+ RebaseInput ri = new RebaseInput();
+ ri.committerEmail = "preferred@domain.org";
+ requestScopeOperations.setApiUser(userWithSecondaryEmail);
+ rebaseCallWithInput.call(r2.getChangeId(), ri);
+ assertThat(r2.getChange().getCommitter().getEmailAddress()).isEqualTo(ri.committerEmail);
+
+ // Approve and submit the r3
+ requestScopeOperations.setApiUser(admin.id());
+ revision = gApi.changes().id(r3.getChangeId()).current();
+ revision.review(ReviewInput.approve());
+ revision.submit();
+
+ // Rebase r2 as the new user with its secondary email
+ ri = new RebaseInput();
+ ri.committerEmail = "secondary@domain.org";
+ requestScopeOperations.setApiUser(userWithSecondaryEmail);
+ rebaseCallWithInput.call(r2.getChangeId(), ri);
+ assertThat(r2.getChange().getCommitter().getEmailAddress()).isEqualTo(ri.committerEmail);
+ }
+
+ @Test
+ public void cannotRebaseWithInvalidCommitterEmail() throws Exception {
+ // Create two changes both with the same parent
+ PushOneCommit.Result c1 = createChange();
+ testRepo.reset("HEAD~1");
+ PushOneCommit.Result c2 = createChange();
+
+ // Approve and submit the first change
+ RevisionApi revision = gApi.changes().id(c1.getChangeId()).current();
+ revision.review(ReviewInput.approve());
+ revision.submit();
+
+ // Rebase the second change with invalid committer email
+ RebaseInput ri = new RebaseInput();
+ ri.committerEmail = "invalid@example.com";
+ ResourceConflictException thrown =
+ assertThrows(
+ ResourceConflictException.class,
+ () -> rebaseCallWithInput.call(c2.getChangeId(), ri));
+ assertThat(thrown)
+ .hasMessageThat()
+ .isEqualTo(
+ String.format(
+ "Cannot rebase using committer email '%s' as it is not a registered email of "
+ + "the user on whose behalf the rebase operation is performed",
+ ri.committerEmail));
+ }
+
+ @Test
public void rebaseAbandonedChange() throws Exception {
PushOneCommit.Result r = createChange();
String changeId = r.getChangeId();
@@ -1096,6 +1173,72 @@
assertThat(thrown).hasMessageThat().contains("The whole chain is already up to date.");
}
+ @Override
+ @Test
+ public void rebaseWithCommitterEmail() throws Exception {
+ // Create changes with the following hierarchy:
+ // * HEAD
+ // * r1
+ // * r2
+
+ PushOneCommit.Result r1 = createChange();
+ testRepo.reset("HEAD~1");
+ PushOneCommit.Result r2 = createChange();
+
+ // Approve and submit the first change
+ RevisionApi revision = gApi.changes().id(r1.getChangeId()).current();
+ revision.review(ReviewInput.approve());
+ revision.submit();
+
+ // Create new user with a secondary email and with permission to rebase
+ Account.Id userWithSecondaryEmail =
+ accountOperations
+ .newAccount()
+ .preferredEmail("preferred@domain.org")
+ .addSecondaryEmail("secondary@domain.org")
+ .create();
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.REBASE).ref("refs/heads/master").group(REGISTERED_USERS))
+ .update();
+
+ // Rebase the chain through r2 with the new user and with its secondary email.
+ RebaseInput ri = new RebaseInput();
+ ri.committerEmail = "secondary@domain.org";
+ requestScopeOperations.setApiUser(userWithSecondaryEmail);
+ BadRequestException exception =
+ assertThrows(
+ BadRequestException.class, () -> gApi.changes().id(r2.getChangeId()).rebaseChain(ri));
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo("committer_email is not supported when rebasing a chain");
+ }
+
+ @Override
+ @Test
+ public void cannotRebaseWithInvalidCommitterEmail() throws Exception {
+ // Create two changes both with the same parent
+ PushOneCommit.Result c1 = createChange();
+ testRepo.reset("HEAD~1");
+ PushOneCommit.Result c2 = createChange();
+
+ // Approve and submit the first change
+ RevisionApi revision = gApi.changes().id(c1.getChangeId()).current();
+ revision.review(ReviewInput.approve());
+ revision.submit();
+
+ // Rebase the second change with invalid committer email
+ RebaseInput ri = new RebaseInput();
+ ri.committerEmail = "invalid@example.com";
+ BadRequestException exception =
+ assertThrows(
+ BadRequestException.class, () -> gApi.changes().id(c2.getChangeId()).rebaseChain(ri));
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo("committer_email is not supported when rebasing a chain");
+ }
+
@Test
public void rebaseChain() throws Exception {
// Create changes with the following hierarchy:
diff --git a/javatests/com/google/gerrit/acceptance/api/change/RebaseOnBehalfOfUploaderIT.java b/javatests/com/google/gerrit/acceptance/api/change/RebaseOnBehalfOfUploaderIT.java
index 968c1f7..319c0cd 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/RebaseOnBehalfOfUploaderIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/RebaseOnBehalfOfUploaderIT.java
@@ -17,6 +17,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth8.assertThat;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowCapability;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowLabel;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
@@ -36,6 +37,7 @@
import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.Change;
@@ -100,6 +102,103 @@
}
@Test
+ public void rebaseOnBehalfOfUploaderWithCommitterEmail() throws Exception {
+ allowPermissionToAllUsers(Permission.REBASE);
+
+ String uploaderPreferredEmail = "uploader.preferred@example.com";
+ String uploaderSecondaryEmail = "uploader.secondary@example.com";
+ Account.Id uploader =
+ accountOperations
+ .newAccount()
+ .preferredEmail(uploaderPreferredEmail)
+ .addSecondaryEmail(uploaderSecondaryEmail)
+ .create();
+ Account.Id approver = admin.id();
+ Account.Id rebaser = accountOperations.newAccount().create();
+
+ projectOperations
+ .allProjectsForUpdate()
+ .add(allowCapability(GlobalCapability.VIEW_SECONDARY_EMAILS).group(REGISTERED_USERS))
+ .update();
+
+ // Create two changes both with the same parent.
+ requestScopeOperations.setApiUser(uploader);
+ Change.Id changeToBeTheNewBase =
+ changeOperations.newChange().project(project).owner(uploader).create();
+ Change.Id changeToBeRebased =
+ changeOperations.newChange().project(project).owner(uploader).create();
+
+ // Approve and submit the change that will be the new base for the change that will be rebased.
+ requestScopeOperations.setApiUser(approver);
+ gApi.changes().id(changeToBeTheNewBase.get()).current().review(ReviewInput.approve());
+ gApi.changes().id(changeToBeTheNewBase.get()).current().submit();
+
+ // Rebase the second change on behalf of the uploader
+ requestScopeOperations.setApiUser(rebaser);
+ RebaseInput rebaseInput = new RebaseInput();
+ rebaseInput.onBehalfOfUploader = true;
+ rebaseInput.committerEmail = uploaderSecondaryEmail;
+ gApi.changes().id(changeToBeRebased.get()).rebase(rebaseInput);
+
+ assertThat(
+ gApi.changes()
+ .id(changeToBeRebased.get())
+ .get()
+ .getCurrentRevision()
+ .commit
+ .committer
+ .email)
+ .isEqualTo(uploaderSecondaryEmail);
+ }
+
+ @Test
+ public void cannotRebaseOnBehalfOfUploaderWithCommitterEmailWithoutViewSecondaryEmails()
+ throws Exception {
+ allowPermissionToAllUsers(Permission.REBASE);
+
+ String uploaderPreferredEmail = "uploader.preferred@example.com";
+ String uploaderSecondaryEmail = "uploader.secondary@example.com";
+ Account.Id uploader =
+ accountOperations
+ .newAccount()
+ .preferredEmail(uploaderPreferredEmail)
+ .addSecondaryEmail(uploaderSecondaryEmail)
+ .create();
+ Account.Id approver = admin.id();
+ Account.Id rebaser = accountOperations.newAccount().create();
+
+ // Create two changes both with the same parent.
+ requestScopeOperations.setApiUser(uploader);
+ Change.Id changeToBeTheNewBase =
+ changeOperations.newChange().project(project).owner(uploader).create();
+ Change.Id changeToBeRebased =
+ changeOperations.newChange().project(project).owner(uploader).create();
+
+ // Approve and submit the change that will be the new base for the change that will be rebased.
+ requestScopeOperations.setApiUser(approver);
+ gApi.changes().id(changeToBeTheNewBase.get()).current().review(ReviewInput.approve());
+ gApi.changes().id(changeToBeTheNewBase.get()).current().submit();
+
+ // Rebase the second change on behalf of the uploader
+ requestScopeOperations.setApiUser(rebaser);
+ RebaseInput rebaseInput = new RebaseInput();
+ rebaseInput.onBehalfOfUploader = true;
+ rebaseInput.committerEmail = uploaderSecondaryEmail;
+
+ ResourceConflictException exception =
+ assertThrows(
+ ResourceConflictException.class,
+ () -> gApi.changes().id(changeToBeRebased.get()).rebase(rebaseInput));
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo(
+ String.format(
+ "Cannot rebase using committer email '%s'. It can only be done using "
+ + "the preferred email or the committer email of the uploader",
+ uploaderSecondaryEmail));
+ }
+
+ @Test
public void cannotRebaseNonCurrentPatchSetOnBehalfOfUploader() throws Exception {
Account.Id uploader = accountOperations.newAccount().create();
Change.Id changeId = changeOperations.newChange().owner(uploader).create();
