Merge "Make Branch.NameKey enforce full ref names"

gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Branch.java

@@ -36,9 +36,9 @@ public final class Branch {
       projectName = new Project.NameKey();
     }
 
-    public NameKey(final Project.NameKey proj, final String n) {
+    public NameKey(final Project.NameKey proj, final String branchName) {
       projectName = proj;
-      branchName = n;
+      set(branchName);
     }
 
     @Override
@@ -48,7 +48,7 @@ public final class Branch {
 
     @Override
     protected void set(String newValue) {
-      branchName = newValue;
+      branchName = RefNames.fullName(newValue);
     }
 
     @Override

gerrit-server/src/main/java/com/google/gerrit/server/args4j/ChangeIdHandler.java

@@ -57,8 +57,7 @@ public class ChangeIdHandler extends OptionHandler<Change.Id> {
     try {
       final Change.Key key = Change.Key.parse(tokens[2]);
       final Project.NameKey project = new Project.NameKey(tokens[0]);
-      final Branch.NameKey branch =
+      final Branch.NameKey branch = new Branch.NameKey(project, tokens[1]);
-          new Branch.NameKey(project, "refs/heads/" + tokens[1]);
       for (final ChangeData cd : queryProvider.get().byBranchKey(branch, key)) {
         setter.addValue(cd.getId());
         return 1;

gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeTriplet.java

@@ -21,8 +21,6 @@ import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Project;
 
-import org.eclipse.jgit.lib.Constants;
-
 @AutoValue
 public abstract class ChangeTriplet {
   public static String format(Change change) {
@@ -53,10 +51,6 @@ public abstract class ChangeTriplet {
     String branch = Url.decode(triplet.substring(t1 + 1, t2));
     String changeId = Url.decode(triplet.substring(t2 + 1));
 
-    if (!branch.startsWith(Constants.R_REFS)) {
-      branch = Constants.R_HEADS + branch;
-    }
-
     ChangeTriplet result = new AutoValue_ChangeTriplet(
         new Branch.NameKey(new Project.NameKey(project), branch),
         new Change.Key(changeId));

gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java

@@ -307,8 +307,7 @@ public class ConsistencyChecker {
       return;
     }
     if (dest == null) {
-      problem("Destination ref not found (may be new branch): "
+      problem("Destination ref not found (may be new branch): " + refName);
-          + change.getDest().get());
       return;
     }
     RevCommit tip = parseCommit(dest.getObjectId(),

gerrit-server/src/main/java/com/google/gerrit/server/util/SubmoduleSectionParser.java

@@ -101,8 +101,6 @@ public class SubmoduleSectionParser {
           // Subscription really related to this running server.
           if (branch.equals(".")) {
             branch = superProjectBranch.get();
-          } else if (!branch.startsWith(Constants.R_REFS)) {
-            branch = Constants.R_HEADS + branch;
           }
 
           final String urlExtractedPath = new URI(url).getPath();

gerrit-server/src/test/java/com/google/gerrit/server/change/ConsistencyCheckerTest.java

@@ -342,7 +342,8 @@ public class ConsistencyCheckerTest {
 
   @Test
   public void missingDestRef() throws Exception {
-    RefUpdate ru = repo.getRepository().updateRef("refs/heads/master");
+    String ref = "refs/heads/master";
+    RefUpdate ru = repo.getRepository().updateRef(ref);
     ru.setForceUpdate(true);
     assertThat(ru.delete()).isEqualTo(RefUpdate.Result.FORCED);
     Change c = insertChange();
@@ -351,7 +352,7 @@ public class ConsistencyCheckerTest {
     updatePatchSetRef(ps);
     db.patchSets().insert(singleton(ps));
 
-    assertProblems(c, "Destination ref not found (may be new branch): master");
+    assertProblems(c, "Destination ref not found (may be new branch): " + ref);
   }
 
   @Test
@@ -363,7 +364,8 @@ public class ConsistencyCheckerTest {
 
     assertProblems(c,
         "Patch set 1 (" + rev + ") is not merged into destination ref"
-        + " master (" + tip.name() + "), but change status is MERGED");
+        + " refs/heads/master (" + tip.name()
+        + "), but change status is MERGED");
   }
 
   @Test
@@ -377,7 +379,8 @@ public class ConsistencyCheckerTest {
 
     assertProblems(c,
         "Patch set 1 (" + commit.name() + ") is merged into destination ref"
-        + " master (" + commit.name() + "), but change status is NEW");
+        + " refs/heads/master (" + commit.name()
+        + "), but change status is NEW");
   }
 
   @Test
@@ -394,7 +397,8 @@ public class ConsistencyCheckerTest {
     ProblemInfo p = problems.get(0);
     assertThat(p.message).isEqualTo(
         "Patch set 1 (" + commit.name() + ") is merged into destination ref"
-        + " master (" + commit.name() + "), but change status is NEW");
+        + " refs/heads/master (" + commit.name()
+        + "), but change status is NEW");
     assertThat(p.status).isEqualTo(ProblemInfo.Status.FIXED);
     assertThat(p.outcome).isEqualTo("Marked change as merged");