Add ability to specify base commit in Create Change API
The Create Change API currently supports the ability to specify
a `base_change`, and the change which you create will use that
existing change as its parent. However, it has no support for
specifying a commit sha1 which already exists on the target
branch.
This change introduces a new `base_commit` field, which takes
a 40 character sha1 as its value. It also introduces processing
to validate that the given sha1 is valid and exists on the
target branch. In also includes documentation and tests for the
above behavior.
Bug: Issue 8857
Change-Id: I7f1daa008b9a8d144c745327495dac65f581a5bf
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 9a09836..97a09cf 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -5734,7 +5734,11 @@
Whether the new change should be set to work in progress.
|`base_change` |optional|
A link:#change-id[\{change-id\}] that identifies the base change for a create
-change operation.
+change operation. Mutually exclusive with `base_commit`.
+|`base_commit` |optional|
+A 40-digit hex SHA-1 of the commit which will be the parent commit of the newly
+created change. If set, it must be a merged commit on the destination branch.
+Mutually exclusive with `base_change`.
|`new_branch` |optional, default to `false`|
Allow creating a new branch when set to `true`.
|`merge` |optional|
diff --git a/java/com/google/gerrit/extensions/common/ChangeInput.java b/java/com/google/gerrit/extensions/common/ChangeInput.java
index c8e7bca..36dd8f2 100644
--- a/java/com/google/gerrit/extensions/common/ChangeInput.java
+++ b/java/com/google/gerrit/extensions/common/ChangeInput.java
@@ -30,6 +30,7 @@
public Boolean isPrivate;
public Boolean workInProgress;
public String baseChange;
+ public String baseCommit;
public Boolean newBranch;
public MergeInput merge;
diff --git a/java/com/google/gerrit/server/restapi/change/CreateChange.java b/java/com/google/gerrit/server/restapi/change/CreateChange.java
index d8aa880..9f86e46 100644
--- a/java/com/google/gerrit/server/restapi/change/CreateChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CreateChange.java
@@ -81,6 +81,7 @@
import java.util.List;
import java.util.TimeZone;
import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.errors.InvalidObjectIdException;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
@@ -180,6 +181,10 @@
}
}
+ if (input.baseChange != null && input.baseCommit != null) {
+ throw new BadRequestException("only provide one of base_change or base_commit");
+ }
+
ProjectResource rsrc = projectsCollection.parse(input.project);
boolean privateByDefault = rsrc.getProjectState().is(BooleanProjectConfig.PRIVATE_BY_DEFAULT);
boolean isPrivate = input.isPrivate == null ? privateByDefault : input.isPrivate;
@@ -205,6 +210,7 @@
RevWalk rw = new RevWalk(reader)) {
ObjectId parentCommit;
List<String> groups;
+ Ref destRef = git.getRefDatabase().exactRef(refName);
if (input.baseChange != null) {
List<ChangeNotes> notes = changeFinder.find(input.baseChange);
if (notes.size() != 1) {
@@ -219,8 +225,21 @@
PatchSet ps = psUtil.current(db.get(), change);
parentCommit = ObjectId.fromString(ps.getRevision().get());
groups = ps.getGroups();
+ } else if (input.baseCommit != null) {
+ try {
+ parentCommit = ObjectId.fromString(input.baseCommit);
+ } catch (InvalidObjectIdException e) {
+ throw new UnprocessableEntityException(
+ String.format("Base %s doesn't represent a valid SHA-1", input.baseCommit));
+ }
+ RevCommit parentRevCommit = rw.parseCommit(parentCommit);
+ RevCommit destRefRevCommit = rw.parseCommit(destRef.getObjectId());
+ if (!rw.isMergedInto(parentRevCommit, destRefRevCommit)) {
+ throw new BadRequestException(
+ String.format("Commit %s doesn't exist on ref %s", input.baseCommit, refName));
+ }
+ groups = Collections.emptyList();
} else {
- Ref destRef = git.getRefDatabase().exactRef(refName);
if (destRef != null) {
if (Boolean.TRUE.equals(input.newBranch)) {
throw new ResourceConflictException(
@@ -231,8 +250,7 @@
if (Boolean.TRUE.equals(input.newBranch)) {
parentCommit = null;
} else {
- throw new UnprocessableEntityException(
- String.format("Branch %s does not exist.", refName));
+ throw new BadRequestException("Must provide a destination branch");
}
}
groups = Collections.emptyList();
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
index ac17ec8..e510e25 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
@@ -208,6 +208,36 @@
}
@Test
+ public void createChangeWithParentCommit() throws Exception {
+ Map<String, PushOneCommit.Result> setup =
+ changeInTwoBranches("foo", "foo.txt", "bar", "bar.txt");
+ ChangeInput input = newChangeInput(ChangeStatus.NEW);
+ input.baseCommit = setup.get("master").getCommit().getId().name();
+ assertCreateSucceeds(input);
+ }
+
+ @Test
+ public void createChangeWithBadParentCommitFails() throws Exception {
+ ChangeInput input = newChangeInput(ChangeStatus.NEW);
+ input.baseCommit = "notasha1";
+ assertCreateFails(
+ input, UnprocessableEntityException.class, "Base notasha1 doesn't represent a valid SHA-1");
+ }
+
+ @Test
+ public void createChangeWithParentCommitOnWrongBranchFails() throws Exception {
+ Map<String, PushOneCommit.Result> setup =
+ changeInTwoBranches("foo", "foo.txt", "bar", "bar.txt");
+ ChangeInput input = newChangeInput(ChangeStatus.NEW);
+ input.branch = "foo";
+ input.baseCommit = setup.get("bar").getCommit().getId().name();
+ assertCreateFails(
+ input,
+ BadRequestException.class,
+ String.format("Commit %s doesn't exist on ref refs/heads/foo", input.baseCommit));
+ }
+
+ @Test
public void createChangeWithoutAccessToParentCommitFails() throws Exception {
Map<String, PushOneCommit.Result> results =
changeInTwoBranches("invisible-branch", "a.txt", "visible-branch", "b.txt");
