Rebase: Allow to specify options for the commit validation
When a change / patch set is created by pushing a commit for review it
is possible to specify push options. These push options are forwarded to
the commit validation listeners. This enables the commit validation
listeners to skip the validation if a certain push option was set (also
see change If9d82a37c). Push options to skip certain commit validations
are for example supported by the uploadvalidator plugin and the
code-owners plugin.
With this change we allow callers of the Rebase REST endpoint to set
validation options in the CherryPickInput that are forwarded as push
options to the commit validation listeners. This way commit validation
can also be skipped when creating a patch set via the Rebase REST
endpoint.
The same is already possible for the Create Change REST endpoint (see
change I936a8b6d6) and the Cherry Pick Revision REST endpoint (see
change I66dc4cfda).
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: If025ac25310d0c46eb34ecdd2e85eea17aef8063
Release-Notes: Add support for specifying validation options in Rebase REST endpoint
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 66b3645..7c478ae 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -7653,20 +7653,28 @@
[options="header",cols="1,^1,5"]
|===========================
-|Field Name ||Description
-|`base` |optional|
+|Field Name ||Description
+|`base` |optional|
The new parent revision. This can be a ref or a SHA-1 to a concrete patchset. +
Alternatively, a change number can be specified, in which case the current
patch set is inferred. +
Empty string is used for rebasing directly on top of the target branch,
which effectively breaks dependency towards a parent change.
-|`allow_conflicts`|optional, defaults to false|
+|`allow_conflicts` |optional, defaults to false|
If `true`, the rebase also succeeds if there are conflicts. +
If there are conflicts the file contents of the rebased patch set contain
git conflict markers to indicate the conflicts. +
Callers can find out whether there were conflicts by checking the
`contains_git_conflicts` field in the returned link:#change-info[ChangeInfo]. +
If there are conflicts the change is marked as work-in-progress.
+|`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
+options are supported depends on the installed commit validation listeners.
+Gerrit core doesn't support any validation options, but commit validation
+listeners that are implemented in plugins may. Please refer to the
+documentation of the installed plugins to learn whether they support validation
+options. Unknown validation options are silently ignored.
|===========================
[[related-change-and-commit-info]]
diff --git a/java/com/google/gerrit/extensions/api/changes/RebaseInput.java b/java/com/google/gerrit/extensions/api/changes/RebaseInput.java
index 10559a3..e9b05cc 100644
--- a/java/com/google/gerrit/extensions/api/changes/RebaseInput.java
+++ b/java/com/google/gerrit/extensions/api/changes/RebaseInput.java
@@ -14,6 +14,8 @@
package com.google.gerrit.extensions.api.changes;
+import java.util.Map;
+
public class RebaseInput {
public String base;
@@ -24,4 +26,6 @@
* to indicate the conflicts.
*/
public boolean allowConflicts;
+
+ public Map<String, String> validationOptions;
}
diff --git a/java/com/google/gerrit/server/change/RebaseChangeOp.java b/java/com/google/gerrit/server/change/RebaseChangeOp.java
index 57f94ff..a0fa8e9 100644
--- a/java/com/google/gerrit/server/change/RebaseChangeOp.java
+++ b/java/com/google/gerrit/server/change/RebaseChangeOp.java
@@ -17,8 +17,10 @@
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.gerrit.server.project.ProjectCache.illegalState;
+import static java.util.Objects.requireNonNull;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -94,6 +96,7 @@
private boolean sendEmail = true;
private boolean storeCopiedVotes = true;
private boolean matchAuthorToCommitterDate = false;
+ private ImmutableListMultimap<String, String> validationOptions = ImmutableListMultimap.of();
private CodeReviewCommit rebasedCommit;
private PatchSet.Id rebasedPatchSetId;
@@ -191,6 +194,13 @@
return this;
}
+ public RebaseChangeOp setValidationOptions(
+ ImmutableListMultimap<String, String> validationOptions) {
+ requireNonNull(validationOptions, "validationOptions may not be null");
+ this.validationOptions = validationOptions;
+ return this;
+ }
+
@Override
public void updateRepo(RepoContext ctx)
throws MergeConflictException, InvalidChangeOperationException, RestApiException, IOException,
@@ -241,6 +251,8 @@
patchSetInserter.setWorkInProgress(true);
}
+ patchSetInserter.setValidationOptions(validationOptions);
+
if (postMessage) {
patchSetInserter.setMessage(
messageForRebasedChange(rebasedPatchSetId, originalPatchSet.id(), rebasedCommit));
diff --git a/java/com/google/gerrit/server/restapi/change/Rebase.java b/java/com/google/gerrit/server/restapi/change/Rebase.java
index 1a0f2b6..835fd5a 100644
--- a/java/com/google/gerrit/server/restapi/change/Rebase.java
+++ b/java/com/google/gerrit/server/restapi/change/Rebase.java
@@ -16,8 +16,10 @@
import static com.google.gerrit.server.project.ProjectCache.illegalState;
+import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
@@ -53,6 +55,7 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
+import java.util.Map;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.ObjectReader;
@@ -126,6 +129,7 @@
.create(rsrc.getNotes(), rsrc.getPatchSet(), findBaseRev(repo, rw, rsrc, input))
.setForceContentMerge(true)
.setAllowConflicts(input.allowConflicts)
+ .setValidationOptions(getValidateOptionsAsMultimap(input.validationOptions))
.setFireRevisionCreated(true);
// TODO(dborowitz): Why no notification? This seems wrong; dig up blame.
bu.setNotify(NotifyResolver.Result.none());
@@ -246,6 +250,20 @@
return description;
}
+ private static ImmutableListMultimap<String, String> getValidateOptionsAsMultimap(
+ @Nullable Map<String, String> validationOptions) {
+ if (validationOptions == null) {
+ return ImmutableListMultimap.of();
+ }
+
+ ImmutableListMultimap.Builder<String, String> validationOptionsBuilder =
+ ImmutableListMultimap.builder();
+ validationOptions
+ .entrySet()
+ .forEach(e -> validationOptionsBuilder.put(e.getKey(), e.getValue()));
+ return validationOptionsBuilder.build();
+ }
+
public static class CurrentRevision implements RestModifyView<ChangeResource, RebaseInput> {
private final PatchSetUtil psUtil;
private final Rebase rebase;
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index dbf129a..1a79b53 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -163,7 +163,11 @@
import com.google.gerrit.server.change.ChangeMessages;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.testing.TestChangeETagComputation;
+import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.git.ChangeMessageModifier;
+import com.google.gerrit.server.git.validators.CommitValidationException;
+import com.google.gerrit.server.git.validators.CommitValidationListener;
+import com.google.gerrit.server.git.validators.CommitValidationMessage;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.index.change.ChangeIndex;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
@@ -1014,6 +1018,31 @@
}
@Test
+ public void rebaseWithValidationOptions() throws Exception {
+ // Create two changes both with the same parent
+ PushOneCommit.Result r = createChange();
+ testRepo.reset("HEAD~1");
+ PushOneCommit.Result r2 = createChange();
+
+ // Approve and submit the first change
+ RevisionApi revision = gApi.changes().id(r.getChangeId()).current();
+ revision.review(ReviewInput.approve());
+ revision.submit();
+
+ RebaseInput rebaseInput = new RebaseInput();
+ rebaseInput.validationOptions = ImmutableMap.of("key", "value");
+
+ TestCommitValidationListener testCommitValidationListener = new TestCommitValidationListener();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(testCommitValidationListener)) {
+ // Rebase the second change
+ gApi.changes().id(r2.getChangeId()).current().rebase(rebaseInput);
+ assertThat(testCommitValidationListener.receiveEvent.pushOptions)
+ .containsExactly("key", "value");
+ }
+ }
+
+ @Test
public void deleteNewChangeAsAdmin() throws Exception {
deleteChangeAsUser(admin, admin);
}
@@ -4707,4 +4736,15 @@
private void voteLabel(String changeId, String labelName, int score) throws RestApiException {
gApi.changes().id(changeId).current().review(new ReviewInput().label(labelName, score));
}
+
+ private static class TestCommitValidationListener implements CommitValidationListener {
+ public CommitReceivedEvent receiveEvent;
+
+ @Override
+ public List<CommitValidationMessage> onCommitReceived(CommitReceivedEvent receiveEvent)
+ throws CommitValidationException {
+ this.receiveEvent = receiveEvent;
+ return ImmutableList.of();
+ }
+ }
}
