Merge branch 'stable-2.16' into stable-3.0

* stable-2.16:
  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

The change "Upgrade gitiles blame-cache to 0.2-7.1" is omitted from this
merge since stable-3.0 already uses 0.2-11. A separate change to upgrade
to 0.2-12 will be done later.

Change-Id: I3d4e2a91f61a5ed469d4d52878ba464b9664af6e

5 files changed, 174 insertions, 11 deletions

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 c06ec69b6e..698f7e03c0 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
@@ -28,6 +28,7 @@ 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;
@@ -35,6 +36,7 @@ import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.inject.Inject;
+import org.eclipse.jgit.revwalk.RevCommit;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -129,6 +131,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 0165eade0f..27e923ec65 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/TagsIT.java
@@ -37,6 +37,7 @@ import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.inject.Inject;
 import java.sql.Timestamp;
 import java.util.List;
+import org.eclipse.jgit.revwalk.RevCommit;
 import org.junit.Test;
 
 @NoHttpd
@@ -330,6 +331,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());