diff options
author | Edwin Kempin <ekempin@google.com> | 2019-12-06 13:40:55 +0000 |
---|---|---|
committer | Gerrit Code Review <noreply-gerritcodereview@google.com> | 2019-12-06 13:40:55 +0000 |
commit | 8d6a9984018d163b29f82c05657903099041f21b (patch) | |
tree | 0f16eaa556838ab722e407fc122f129d2e48b88a | |
parent | f4f8d2e41cecb09b2e8e1a0d91d41f8db073b398 (diff) | |
parent | bb1aaa14a732f91631db9ced3d3154767c6704df (diff) |
Merge changes I2fb1cd15,I78afe991,Ifdbac1bc,I0f693c63,I378d3845, ... into stable-2.16
* 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
Upgrade gitiles blame-cache to 0.2-7.1
6 files changed, 176 insertions, 13 deletions
@@ -768,10 +768,10 @@ maven_jar( maven_jar( name = "blame-cache", - artifact = "com/google/gitiles:blame-cache:0.2-7", + artifact = "com/google/gitiles:blame-cache:0.2-7.1", attach_source = False, repository = GERRIT, - sha1 = "8170f33b8b1db6f55e41d7069fa050a4d102a62b", + sha1 = "73915991bb7472a730102ab01ca68776a52466fd", ) # Keep this version of Soy synchronized with the version used in Gitiles. diff --git a/java/com/google/gerrit/server/project/RefUtil.java b/java/com/google/gerrit/server/project/RefUtil.java index 9f1fa4a21c..4e08137a1c 100644 --- a/java/com/google/gerrit/server/project/RefUtil.java +++ b/java/com/google/gerrit/server/project/RefUtil.java @@ -19,6 +19,7 @@ import static org.eclipse.jgit.lib.Constants.R_TAGS; import com.google.common.collect.Iterables; import com.google.common.flogger.FluentLogger; +import com.google.gerrit.common.Nullable; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.RefNames; @@ -46,16 +47,15 @@ public class RefUtil { 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 @@ public class RefUtil { 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 @@ public class RefUtil { 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 class RefUtil { 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 62106e8c1d..ec1f56e718 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 @@ package com.google.gerrit.server.restapi.project; import static com.google.gerrit.reviewdb.client.RefNames.isConfigRef; +import com.google.common.base.Strings; import com.google.common.flogger.FluentLogger; import com.google.gerrit.extensions.api.projects.BranchInfo; import com.google.gerrit.extensions.api.projects.BranchInput; @@ -91,7 +92,10 @@ public class CreateBranch 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 e72deaf255..855a7c7429 100644 --- a/java/com/google/gerrit/server/restapi/project/CreateTag.java +++ b/java/com/google/gerrit/server/restapi/project/CreateTag.java @@ -87,7 +87,10 @@ public class CreateTag implements RestCollectionCreateView<ProjectResource, TagR 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 df896864e5..c49a62b29b 100644 --- a/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java @@ -28,12 +28,14 @@ import com.google.gerrit.extensions.api.projects.BranchApi; 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.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.RefNames; +import org.eclipse.jgit.revwalk.RevCommit; import org.junit.Before; import org.junit.Test; @@ -127,6 +129,112 @@ public class CreateBranchIT extends AbstractDaemonTest { "Not allowed to create group branch."); } + @Test + public void createWithRevision() throws Exception { + RevCommit revision = getRemoteHead(project, "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(getRemoteHead(project, "master")).isNotEqualTo(revision); + + BranchInput input = new BranchInput(); + input.revision = revision.name(); + BranchInfo created = branch(testBranch).create(input).get(); + assertThat(created.ref).isEqualTo(testBranch.get()); + assertThat(created.revision).isEqualTo(revision.name()); + assertThat(getRemoteHead(project, testBranch.getShortName())).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 = getRemoteHead(project, "master"); + + BranchInput input = new BranchInput(); + input.revision = null; + BranchInfo created = branch(testBranch).create(input).get(); + assertThat(created.ref).isEqualTo(testBranch.get()); + assertThat(created.revision).isEqualTo(expectedRevision.name()); + assertThat(getRemoteHead(project, testBranch.getShortName())).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 = getRemoteHead(project, "master"); + + BranchInput input = new BranchInput(); + input.revision = ""; + BranchInfo created = branch(testBranch).create(input).get(); + assertThat(created.ref).isEqualTo(testBranch.get()); + assertThat(created.revision).isEqualTo(expectedRevision.name()); + assertThat(getRemoteHead(project, testBranch.getShortName())).isEqualTo(expectedRevision); + } + + @Test + public void createRevisionIsTrimmed() throws Exception { + RevCommit revision = getRemoteHead(project, "master"); + + BranchInput input = new BranchInput(); + input.revision = "\t" + revision.name(); + BranchInfo created = branch(testBranch).create(input).get(); + assertThat(created.ref).isEqualTo(testBranch.get()); + assertThat(created.revision).isEqualTo(revision.name()); + assertThat(getRemoteHead(project, testBranch.getShortName())).isEqualTo(revision); + } + + @Test + public void createWithBranchNameAsRevision() throws Exception { + RevCommit expectedRevision = getRemoteHead(project, "master"); + + BranchInput input = new BranchInput(); + input.revision = "master"; + BranchInfo created = branch(testBranch).create(input).get(); + assertThat(created.ref).isEqualTo(testBranch.get()); + assertThat(created.revision).isEqualTo(expectedRevision.name()); + assertThat(getRemoteHead(project, testBranch.getShortName())).isEqualTo(expectedRevision); + } + + @Test + public void createWithFullBranchNameAsRevision() throws Exception { + RevCommit expectedRevision = getRemoteHead(project, "master"); + + BranchInput input = new BranchInput(); + input.revision = "refs/heads/master"; + BranchInfo created = branch(testBranch).create(input).get(); + assertThat(created.ref).isEqualTo(testBranch.get()); + assertThat(created.revision).isEqualTo(expectedRevision.name()); + assertThat(getRemoteHead(project, testBranch.getShortName())).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 { block("refs/*", Permission.CREATE, ANONYMOUS_USERS); } diff --git a/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java b/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java index d4edc0de95..610ce92e23 100644 --- a/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java @@ -35,6 +35,7 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import java.sql.Timestamp; import java.util.List; +import org.eclipse.jgit.revwalk.RevCommit; import org.junit.Test; @NoHttpd @@ -326,6 +327,53 @@ public class TagsIT extends AbstractDaemonTest { tag(input.ref).create(input); } + @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 = getRemoteHead(project, "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 = getRemoteHead(project, "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 = getRemoteHead(project, "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()); |