Merge changes I2fb1cd15,I78afe991,Ifdbac1bc,I0f693c63,I378d3845, ...
* changes:
CreateTag: Trim revision that is provided in input
CreateTag: Allow revision in input to be empty
RefUtil#parseBaseRevision: Do not log an error if baseRevision is invalid
InvalidRevisionException: Include invalid revision into message
CreateBranch: Trim revision that is provided in input
CreateBranch: Allow revision in input to be empty
CreateBranchIT: Add tests that set revision in the input
diff --git a/java/com/google/gerrit/server/project/RefUtil.java b/java/com/google/gerrit/server/project/RefUtil.java
index 1dac751..5d6379a 100644
--- a/java/com/google/gerrit/server/project/RefUtil.java
+++ b/java/com/google/gerrit/server/project/RefUtil.java
@@ -19,6 +19,7 @@
import com.google.common.collect.Iterables;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -46,16 +47,15 @@
try {
ObjectId revid = repo.resolve(baseRevision);
if (revid == null) {
- throw new InvalidRevisionException();
+ throw new InvalidRevisionException(baseRevision);
}
return revid;
} catch (IOException err) {
logger.atSevere().withCause(err).log(
"Cannot resolve \"%s\" in project \"%s\"", baseRevision, projectName.get());
- throw new InvalidRevisionException();
+ throw new InvalidRevisionException(baseRevision);
} catch (RevisionSyntaxException err) {
- logger.atSevere().withCause(err).log("Invalid revision syntax \"%s\"", baseRevision);
- throw new InvalidRevisionException();
+ throw new InvalidRevisionException(baseRevision);
}
}
@@ -66,7 +66,7 @@
try {
rw.markStart(rw.parseCommit(revid));
} catch (IncorrectObjectTypeException err) {
- throw new InvalidRevisionException();
+ throw new InvalidRevisionException(revid.name());
}
RefDatabase refDb = repo.getRefDatabase();
Iterable<Ref> refs =
@@ -86,11 +86,11 @@
rw.checkConnectivity();
return rw;
} catch (IncorrectObjectTypeException | MissingObjectException err) {
- throw new InvalidRevisionException();
+ throw new InvalidRevisionException(revid.name());
} catch (IOException err) {
logger.atSevere().withCause(err).log(
"Repository \"%s\" may be corrupt; suggest running git fsck", repo.getDirectory());
- throw new InvalidRevisionException();
+ throw new InvalidRevisionException(revid.name());
}
}
@@ -125,8 +125,8 @@
public static final String MESSAGE = "Invalid Revision";
- InvalidRevisionException() {
- super(MESSAGE);
+ InvalidRevisionException(@Nullable String invalidRevision) {
+ super(MESSAGE + ": " + invalidRevision);
}
}
}
diff --git a/java/com/google/gerrit/server/restapi/project/CreateBranch.java b/java/com/google/gerrit/server/restapi/project/CreateBranch.java
index 56948c1..67213c5 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateBranch.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateBranch.java
@@ -16,6 +16,7 @@
import static com.google.gerrit.entities.RefNames.isConfigRef;
+import com.google.common.base.Strings;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.RefNames;
@@ -93,7 +94,10 @@
if (input.ref != null && !ref.equals(input.ref)) {
throw new BadRequestException("ref must match URL");
}
- if (input.revision == null) {
+ if (input.revision != null) {
+ input.revision = input.revision.trim();
+ }
+ if (Strings.isNullOrEmpty(input.revision)) {
input.revision = Constants.HEAD;
}
while (ref.startsWith("/")) {
diff --git a/java/com/google/gerrit/server/restapi/project/CreateTag.java b/java/com/google/gerrit/server/restapi/project/CreateTag.java
index dca6e9a..8fdf5e4 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateTag.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateTag.java
@@ -88,7 +88,10 @@
if (input.ref != null && !ref.equals(input.ref)) {
throw new BadRequestException("ref must match URL");
}
- if (input.revision == null) {
+ if (input.revision != null) {
+ input.revision = input.revision.trim();
+ }
+ if (Strings.isNullOrEmpty(input.revision)) {
input.revision = Constants.HEAD;
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java b/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
index e5ef5ba..85d383e 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
@@ -36,9 +36,11 @@
import com.google.gerrit.extensions.api.projects.BranchInfo;
import com.google.gerrit.extensions.api.projects.BranchInput;
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.RestApiException;
import com.google.inject.Inject;
+import org.eclipse.jgit.revwalk.RevCommit;
import org.junit.Before;
import org.junit.Test;
@@ -146,6 +148,116 @@
"Not allowed to create group branch.");
}
+ @Test
+ public void createWithRevision() throws Exception {
+ RevCommit revision = projectOperations.project(project).getHead("master");
+
+ // update master so that points to a different revision than the revision on which we create the
+ // new branch
+ pushTo("refs/heads/master");
+ assertThat(projectOperations.project(project).getHead("master")).isNotEqualTo(revision);
+
+ BranchInput input = new BranchInput();
+ input.revision = revision.name();
+ BranchInfo created = branch(testBranch).create(input).get();
+ assertThat(created.ref).isEqualTo(testBranch.branch());
+ assertThat(created.revision).isEqualTo(revision.name());
+ assertThat(projectOperations.project(project).getHead(testBranch.branch())).isEqualTo(revision);
+ }
+
+ @Test
+ public void createWithoutSpecifyingRevision() throws Exception {
+ // If revision is not specified, the branch is created based on HEAD, which points to master.
+ RevCommit expectedRevision = projectOperations.project(project).getHead("master");
+
+ BranchInput input = new BranchInput();
+ input.revision = null;
+ BranchInfo created = branch(testBranch).create(input).get();
+ assertThat(created.ref).isEqualTo(testBranch.branch());
+ assertThat(created.revision).isEqualTo(expectedRevision.name());
+ assertThat(projectOperations.project(project).getHead(testBranch.branch()))
+ .isEqualTo(expectedRevision);
+ }
+
+ @Test
+ public void createWithEmptyRevision() throws Exception {
+ // If revision is not specified, the branch is created based on HEAD, which points to master.
+ RevCommit expectedRevision = projectOperations.project(project).getHead("master");
+
+ BranchInput input = new BranchInput();
+ input.revision = "";
+ BranchInfo created = branch(testBranch).create(input).get();
+ assertThat(created.ref).isEqualTo(testBranch.branch());
+ assertThat(created.revision).isEqualTo(expectedRevision.name());
+ assertThat(projectOperations.project(project).getHead(testBranch.branch()))
+ .isEqualTo(expectedRevision);
+ }
+
+ @Test
+ public void createRevisionIsTrimmed() throws Exception {
+ RevCommit revision = projectOperations.project(project).getHead("master");
+
+ BranchInput input = new BranchInput();
+ input.revision = "\t" + revision.name();
+ BranchInfo created = branch(testBranch).create(input).get();
+ assertThat(created.ref).isEqualTo(testBranch.branch());
+ assertThat(created.revision).isEqualTo(revision.name());
+ assertThat(projectOperations.project(project).getHead(testBranch.branch())).isEqualTo(revision);
+ }
+
+ @Test
+ public void createWithBranchNameAsRevision() throws Exception {
+ RevCommit expectedRevision = projectOperations.project(project).getHead("master");
+
+ BranchInput input = new BranchInput();
+ input.revision = "master";
+ BranchInfo created = branch(testBranch).create(input).get();
+ assertThat(created.ref).isEqualTo(testBranch.branch());
+ assertThat(created.revision).isEqualTo(expectedRevision.name());
+ assertThat(projectOperations.project(project).getHead(testBranch.branch()))
+ .isEqualTo(expectedRevision);
+ }
+
+ @Test
+ public void createWithFullBranchNameAsRevision() throws Exception {
+ RevCommit expectedRevision = projectOperations.project(project).getHead("master");
+
+ BranchInput input = new BranchInput();
+ input.revision = "refs/heads/master";
+ BranchInfo created = branch(testBranch).create(input).get();
+ assertThat(created.ref).isEqualTo(testBranch.branch());
+ assertThat(created.revision).isEqualTo(expectedRevision.name());
+ assertThat(projectOperations.project(project).getHead(testBranch.branch()))
+ .isEqualTo(expectedRevision);
+ }
+
+ @Test
+ public void cannotCreateWithNonExistingBranchNameAsRevision() throws Exception {
+ assertCreateFails(
+ testBranch,
+ "refs/heads/non-existing",
+ BadRequestException.class,
+ "invalid revision \"refs/heads/non-existing\"");
+ }
+
+ @Test
+ public void cannotCreateWithNonExistingRevision() throws Exception {
+ assertCreateFails(
+ testBranch,
+ "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef",
+ BadRequestException.class,
+ "invalid revision \"deadbeefdeadbeefdeadbeefdeadbeefdeadbeef\"");
+ }
+
+ @Test
+ public void cannotCreateWithInvalidRevision() throws Exception {
+ assertCreateFails(
+ testBranch,
+ "invalid\trevision",
+ BadRequestException.class,
+ "invalid revision \"invalid\trevision\"");
+ }
+
private void blockCreateReference() throws Exception {
projectOperations
.project(project)
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java b/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java
index 3d1a148..3becb81 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java
@@ -41,6 +41,7 @@
import com.google.inject.Inject;
import java.sql.Timestamp;
import java.util.List;
+import org.eclipse.jgit.revwalk.RevCommit;
import org.junit.Test;
@NoHttpd
@@ -357,6 +358,53 @@
assertThat(thrown).hasMessageThat().contains("Invalid base revision");
}
+ @Test
+ public void noBaseRevision() throws Exception {
+ grantTagPermissions();
+
+ // If revision is not specified, the tag is created based on HEAD, which points to master.
+ RevCommit expectedRevision = projectOperations.project(project).getHead("master");
+
+ TagInput input = new TagInput();
+ input.ref = "test";
+ input.revision = null;
+
+ TagInfo result = tag(input.ref).create(input).get();
+ assertThat(result.ref).isEqualTo(R_TAGS + input.ref);
+ assertThat(result.revision).isEqualTo(expectedRevision.name());
+ }
+
+ @Test
+ public void emptyBaseRevision() throws Exception {
+ grantTagPermissions();
+
+ // If revision is not specified, the tag is created based on HEAD, which points to master.
+ RevCommit expectedRevision = projectOperations.project(project).getHead("master");
+
+ TagInput input = new TagInput();
+ input.ref = "test";
+ input.revision = "";
+
+ TagInfo result = tag(input.ref).create(input).get();
+ assertThat(result.ref).isEqualTo(R_TAGS + input.ref);
+ assertThat(result.revision).isEqualTo(expectedRevision.name());
+ }
+
+ @Test
+ public void baseRevisionIsTrimmed() throws Exception {
+ grantTagPermissions();
+
+ RevCommit revision = projectOperations.project(project).getHead("master");
+
+ TagInput input = new TagInput();
+ input.ref = "test";
+ input.revision = "\t" + revision.name();
+
+ TagInfo result = tag(input.ref).create(input).get();
+ assertThat(result.ref).isEqualTo(R_TAGS + input.ref);
+ assertThat(result.revision).isEqualTo(revision.name());
+ }
+
private void assertTagList(FluentIterable<String> expected, List<TagInfo> actual)
throws Exception {
assertThat(actual).hasSize(expected.size());
