Merge changes from topic 'draft-ref-sharding'

* changes:
  Shard refs/draft-comments/* by change instead of account ID
  Move method for parsing ref parts (CD/ABCD/...) to RefNames
  AccountTest: Convert to Truth
  Move ChangeNoteUtil#changeRefName to RefNames

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java

@@ -20,6 +20,7 @@ import static com.google.gerrit.acceptance.PushOneCommit.FILE_NAME;
 import static com.google.gerrit.acceptance.PushOneCommit.SUBJECT;
 import static com.google.gerrit.extensions.client.ReviewerState.CC;
 import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER;
+import static com.google.gerrit.reviewdb.client.RefNames.changeMetaRef;
 import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
 import static com.google.gerrit.server.group.SystemGroupBackend.CHANGE_OWNER;
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
@@ -65,7 +66,6 @@ import com.google.gerrit.server.change.ChangeResource;
 import com.google.gerrit.server.config.AnonymousCowardNameProvider;
 import com.google.gerrit.server.git.ProjectConfig;
 import com.google.gerrit.server.group.SystemGroupBackend;
-import com.google.gerrit.server.notedb.ChangeNoteUtil;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.Util;
 import com.google.gerrit.testutil.FakeEmailSender.Message;
@@ -1099,8 +1099,7 @@ public class ChangeIT extends AbstractDaemonTest {
     try (Repository repo = repoManager.openRepository(project);
         RevWalk rw = new RevWalk(repo)) {
       RevCommit commitPatchSetCreation = rw.parseCommit(
-          repo.exactRef(ChangeNoteUtil.changeRefName(new Change.Id(c._number)))
+          repo.exactRef(changeMetaRef(new Change.Id(c._number))).getObjectId());
-              .getObjectId());
 
       assertThat(commitPatchSetCreation.getShortMessage())
           .isEqualTo("Create patch set 2");

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java

@@ -16,6 +16,7 @@ package com.google.gerrit.acceptance.rest.change;
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.TruthJUnit.assume;
+import static com.google.gerrit.reviewdb.client.RefNames.changeMetaRef;
 import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.eclipse.jgit.lib.Constants.SIGNED_OFF_BY_TAG;
 
@@ -31,7 +32,6 @@ import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.server.config.AnonymousCowardNameProvider;
-import com.google.gerrit.server.notedb.ChangeNoteUtil;
 import com.google.gerrit.testutil.ConfigSuite;
 import com.google.gerrit.testutil.TestTimeUtil;
 
@@ -122,8 +122,7 @@ public class CreateChangeIT extends AbstractDaemonTest {
     try (Repository repo = repoManager.openRepository(project);
         RevWalk rw = new RevWalk(repo)) {
       RevCommit commit = rw.parseCommit(
-          repo.exactRef(ChangeNoteUtil.changeRefName(new Change.Id(c._number)))
+          repo.exactRef(changeMetaRef(new Change.Id(c._number))).getObjectId());
-              .getObjectId());
 
       assertThat(commit.getShortMessage()).isEqualTo("Create change");
 

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/DeleteDraftPatchSetIT.java

@@ -191,7 +191,7 @@ public class DeleteDraftPatchSetIT extends AbstractDaemonTest {
   private Ref getDraftRef(TestAccount account, Change.Id changeId)
       throws Exception {
     try (Repository repo = repoManager.openRepository(allUsers)) {
-      return repo.exactRef(RefNames.refsDraftComments(account.id, changeId));
+      return repo.exactRef(RefNames.refsDraftComments(changeId, account.id));
     }
   }
 

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java

@@ -16,6 +16,7 @@ package com.google.gerrit.acceptance.server.notedb;
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.TruthJUnit.assume;
+import static com.google.gerrit.reviewdb.client.RefNames.changeMetaRef;
 
 import com.google.common.collect.ImmutableList;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -41,7 +42,6 @@ import com.google.gerrit.server.change.Rebuild;
 import com.google.gerrit.server.change.RevisionResource;
 import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.notedb.ChangeBundle;
-import com.google.gerrit.server.notedb.ChangeNoteUtil;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.notedb.NoteDbChangeState;
 import com.google.gerrit.server.schema.DisabledChangesReviewDbWrapper;
@@ -283,21 +283,20 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
     PushOneCommit.Result r = createChange();
     Change.Id id = r.getPatchSetId().getParentKey();
 
-    ObjectId changeMetaId = getMetaRef(
+    ObjectId changeMetaId = getMetaRef(project, changeMetaRef(id));
-        project, ChangeNoteUtil.changeRefName(id));
     assertThat(unwrapDb().changes().get(id).getNoteDbState()).isEqualTo(
         changeMetaId.name());
 
     putDraft(user, id, 1, "comment by user");
     ObjectId userDraftsId = getMetaRef(
-        allUsers, RefNames.refsDraftComments(user.getId(), id));
+        allUsers, RefNames.refsDraftComments(id, user.getId()));
     assertThat(unwrapDb().changes().get(id).getNoteDbState()).isEqualTo(
         changeMetaId.name()
         + "," + user.getId() + "=" + userDraftsId.name());
 
     putDraft(admin, id, 2, "comment by admin");
     ObjectId adminDraftsId = getMetaRef(
-        allUsers, RefNames.refsDraftComments(admin.getId(), id));
+        allUsers, RefNames.refsDraftComments(id, admin.getId()));
     assertThat(admin.getId().get()).isLessThan(user.getId().get());
     assertThat(unwrapDb().changes().get(id).getNoteDbState()).isEqualTo(
         changeMetaId.name()
@@ -306,7 +305,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
 
     putDraft(admin, id, 2, "revised comment by admin");
     adminDraftsId = getMetaRef(
-        allUsers, RefNames.refsDraftComments(admin.getId(), id));
+        allUsers, RefNames.refsDraftComments(id, admin.getId()));
     assertThat(unwrapDb().changes().get(id).getNoteDbState()).isEqualTo(
         changeMetaId.name()
         + "," + admin.getId() + "=" + adminDraftsId.name()

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

@@ -122,46 +122,23 @@ public final class Account {
      *              We assume that the caller has trimmed any prefix.
      */
     public static Id fromRefPart(String name) {
-      if (name == null) {
+      Integer id = RefNames.parseShardedRefPart(name);
-        return null;
+      return id != null ? new Account.Id(id) : null;
     }
 
-      String[] parts = name.split("/");
+    /**
-      int n = parts.length;
+     * Parse an Account.Id out of the last part of a ref name.
-      if (n < 2) {
+     * <p>
-        return null;
+     * The input is a ref name of the form {@code ".../1234"}, where the suffix
-      }
+     * is a non-sharded account ID. Ref names using a sharded ID should use
-
+     * {@link #fromRefPart(String)} instead for greater safety.
-      // Last 2 digits.
+     *
-      int le;
+     * @param name ref name
-      for (le = 0; le < parts[0].length(); le++) {
+     * @return account ID, or null if not numeric.
-        if (!Character.isDigit(parts[0].charAt(le))) {
+     */
-          return null;
+    public static Id fromRefSuffix(String name) {
-        }
+      Integer id = RefNames.parseRefSuffix(name);
-      }
+      return id != null ? new Account.Id(id) : null;
-      if (le != 2) {
-        return null;
-      }
-
-      // Full ID.
-      int ie;
-      for (ie = 0; ie < parts[1].length(); ie++) {
-        if (!Character.isDigit(parts[1].charAt(ie))) {
-          if (ie == 0) {
-            return null;
-          } else {
-            break;
-          }
-        }
-      }
-
-      int shard = Integer.parseInt(parts[0]);
-      int id = Integer.parseInt(parts[1].substring(0, ie));
-
-      if (id % 100 != shard) {
-        return null;
-      }
-      return new Account.Id(id);
     }
   }
 

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

@@ -163,6 +163,11 @@ public final class Change {
       return new Change.Id(Integer.parseInt(id));
     }
 
+    public static Id fromRefPart(String ref) {
+      Integer id = RefNames.parseShardedRefPart(ref);
+      return id != null ? new Change.Id(id) : null;
+    }
+
     static int startIndex(String ref) {
       if (ref == null || !ref.startsWith(REFS_CHANGES)) {
         return -1;

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

@@ -14,7 +14,6 @@
 
 package com.google.gerrit.reviewdb.client;
 
-
 /** Constants and utilities for Gerrit-specific ref names. */
 public class RefNames {
   public static final String REFS = "refs/";
@@ -79,6 +78,21 @@ public class RefNames {
     return ref;
   }
 
+  public static String changeMetaRef(Change.Id id) {
+    StringBuilder r = new StringBuilder();
+    r.append(REFS_CHANGES);
+    int n = id.get();
+    int m = n % 100;
+    if (m < 10) {
+      r.append('0');
+    }
+    r.append(m);
+    r.append('/');
+    r.append(n);
+    r.append(META_SUFFIX);
+    return r.toString();
+  }
+
   public static String refsUsers(Account.Id accountId) {
     StringBuilder r = new StringBuilder();
     r.append(REFS_USERS);
@@ -93,15 +107,15 @@ public class RefNames {
     return r.toString();
   }
 
-  public static String refsDraftComments(Account.Id accountId,
+  public static String refsDraftComments(Change.Id changeId,
-      Change.Id changeId) {
+      Account.Id accountId) {
-    StringBuilder r = buildRefsPrefix(REFS_DRAFT_COMMENTS, accountId.get());
+    StringBuilder r = buildRefsPrefix(REFS_DRAFT_COMMENTS, changeId.get());
-    r.append(changeId.get());
+    r.append(accountId.get());
     return r.toString();
   }
 
-  public static String refsDraftCommentsPrefix(Account.Id accountId) {
+  public static String refsDraftCommentsPrefix(Change.Id changeId) {
-    return buildRefsPrefix(REFS_DRAFT_COMMENTS, accountId.get()).toString();
+    return buildRefsPrefix(REFS_DRAFT_COMMENTS, changeId.get()).toString();
   }
 
   public static String refsStarredChanges(Change.Id changeId,
@@ -160,6 +174,69 @@ public class RefNames {
       .toString();
   }
 
+  static Integer parseShardedRefPart(String name) {
+    if (name == null) {
+      return null;
+    }
+
+    String[] parts = name.split("/");
+    int n = parts.length;
+    if (n < 2) {
+      return null;
+    }
+
+    // Last 2 digits.
+    int le;
+    for (le = 0; le < parts[0].length(); le++) {
+      if (!Character.isDigit(parts[0].charAt(le))) {
+        return null;
+      }
+    }
+    if (le != 2) {
+      return null;
+    }
+
+    // Full ID.
+    int ie;
+    for (ie = 0; ie < parts[1].length(); ie++) {
+      if (!Character.isDigit(parts[1].charAt(ie))) {
+        if (ie == 0) {
+          return null;
+        } else {
+          break;
+        }
+      }
+    }
+
+    int shard = Integer.parseInt(parts[0]);
+    int id = Integer.parseInt(parts[1].substring(0, ie));
+
+    if (id % 100 != shard) {
+      return null;
+    }
+    return id;
+  }
+
+  static Integer parseRefSuffix(String name) {
+    if (name == null) {
+      return null;
+    }
+    int i = name.length();
+    while (i > 0) {
+      char c = name.charAt(i - 1);
+      if (c == '/') {
+        break;
+      } else if (!Character.isDigit(c)) {
+        return null;
+      }
+      i--;
+    }
+    if (i == 0) {
+      return null;
+    }
+    return Integer.valueOf(name.substring(i, name.length()));
+  }
+
   private RefNames() {
   }
 }

gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/AccountTest.java

@@ -14,69 +14,48 @@
 
 package com.google.gerrit.reviewdb.client;
 
-import static org.junit.Assert.assertEquals;
+import static com.google.common.truth.Truth.assertThat;
-import static org.junit.Assert.assertNull;
+import static com.google.gerrit.reviewdb.client.Account.Id.fromRef;
+import static com.google.gerrit.reviewdb.client.Account.Id.fromRefPart;
+import static com.google.gerrit.reviewdb.client.Account.Id.fromRefSuffix;
 
 import org.junit.Test;
 
 public class AccountTest {
   @Test
   public void parseRefName() {
-    assertRef(1, "refs/users/01/1");
+    assertThat(fromRef("refs/users/01/1")).isEqualTo(id(1));
-    assertRef(1, "refs/users/01/1-drafts");
+    assertThat(fromRef("refs/users/01/1-drafts")).isEqualTo(id(1));
-    assertRef(1, "refs/users/01/1-drafts/2");
+    assertThat(fromRef("refs/users/01/1-drafts/2")).isEqualTo(id(1));
-    assertRef(1, "refs/users/01/1/edit/2");
+    assertThat(fromRef("refs/users/01/1/edit/2")).isEqualTo(id(1));
 
-    assertNotRef(null);
+    assertThat(fromRef(null)).isNull();
-    assertNotRef("");
+    assertThat(fromRef("")).isNull();
 
     // Invalid characters.
-    assertNotRef("refs/users/01a/1");
+    assertThat(fromRef("refs/users/01a/1")).isNull();
-    assertNotRef("refs/users/01/a1");
+    assertThat(fromRef("refs/users/01/a1")).isNull();
 
     // Mismatched shard.
-    assertNotRef("refs/users/01/23");
+    assertThat(fromRef("refs/users/01/23")).isNull();
 
     // Shard too short.
-    assertNotRef("refs/users/1/1");
+    assertThat(fromRef("refs/users/1/1")).isNull();
   }
 
   @Test
   public void parseRefNameParts() {
-    assertRefPart(1, "01/1");
+    assertThat(fromRefPart("01/1")).isEqualTo(id(1));
-    assertRefPart(1, "01/1-drafts");
+    assertThat(fromRefPart("ab/cd")).isNull();
-    assertRefPart(1, "01/1-drafts/2");
-
-    assertNotRefPart(null);
-    assertNotRefPart("");
-
-    // This method assumes that the common prefix "refs/users/" will be removed.
-    assertNotRefPart("refs/users/01/1");
-
-    // Invalid characters.
-    assertNotRefPart("01a/1");
-    assertNotRefPart("01/a1");
-
-    // Mismatched shard.
-    assertNotRefPart("01/23");
-
-    // Shard too short.
-    assertNotRefPart("1/1");
   }
 
-  private static void assertRef(int accountId, String refName) {
+  @Test
-    assertEquals(new Account.Id(accountId), Account.Id.fromRef(refName));
+  public void parseRefSuffix() {
+    assertThat(fromRefSuffix("12/34")).isEqualTo(id(34));
+    assertThat(fromRefSuffix("ab/cd")).isNull();
   }
 
-  private static void assertNotRef(String refName) {
+  private Account.Id id(int n) {
-    assertNull(Account.Id.fromRef(refName));
+    return new Account.Id(n);
-  }
-
-  private static void assertRefPart(int accountId, String refName) {
-    assertEquals(new Account.Id(accountId), Account.Id.fromRefPart(refName));
-  }
-
-  private static void assertNotRefPart(String refName) {
-    assertNull(Account.Id.fromRefPart(refName));
   }
 }

gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/RefNamesTest.java

@@ -15,6 +15,8 @@
 package com.google.gerrit.reviewdb.client;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.reviewdb.client.RefNames.parseShardedRefPart;
+import static com.google.gerrit.reviewdb.client.RefNames.parseRefSuffix;
 
 import org.junit.Test;
 
@@ -38,14 +40,14 @@ public class RefNamesTest {
 
   @Test
   public void refsDraftComments() throws Exception {
-    assertThat(RefNames.refsDraftComments(accountId, changeId))
+    assertThat(RefNames.refsDraftComments(changeId, accountId))
-      .isEqualTo("refs/draft-comments/23/1011123/67473");
+      .isEqualTo("refs/draft-comments/73/67473/1011123");
   }
 
   @Test
   public void refsDraftCommentsPrefix() throws Exception {
-    assertThat(RefNames.refsDraftCommentsPrefix(accountId))
+    assertThat(RefNames.refsDraftCommentsPrefix(changeId))
-      .isEqualTo("refs/draft-comments/23/1011123/");
+      .isEqualTo("refs/draft-comments/73/67473/");
   }
 
   @Test
@@ -65,4 +67,42 @@ public class RefNamesTest {
     assertThat(RefNames.refsEdit(accountId, changeId, psId))
       .isEqualTo("refs/users/23/1011123/edit-67473/42");
   }
+
+  @Test
+  public void testParseShardedRefsPart() throws Exception {
+    assertThat(parseShardedRefPart("01/1")).isEqualTo(1);
+    assertThat(parseShardedRefPart("01/1-drafts")).isEqualTo(1);
+    assertThat(parseShardedRefPart("01/1-drafts/2")).isEqualTo(1);
+
+    assertThat(parseShardedRefPart(null)).isNull();
+    assertThat(parseShardedRefPart("")).isNull();
+
+    // Prefix not stripped.
+    assertThat(parseShardedRefPart("refs/users/01/1")).isNull();
+
+    // Invalid characters.
+    assertThat(parseShardedRefPart("01a/1")).isNull();
+    assertThat(parseShardedRefPart("01/a1")).isNull();
+
+    // Mismatched shard.
+    assertThat(parseShardedRefPart("01/23")).isNull();
+
+    // Shard too short.
+    assertThat(parseShardedRefPart("1/1")).isNull();
+  }
+
+  @Test
+  public void testParseRefSuffix() throws Exception {
+    assertThat(parseRefSuffix("1/2/34")).isEqualTo(34);
+    assertThat(parseRefSuffix("/34")).isEqualTo(34);
+
+    assertThat(parseRefSuffix(null)).isNull();
+    assertThat(parseRefSuffix("")).isNull();
+    assertThat(parseRefSuffix("34")).isNull();
+    assertThat(parseRefSuffix("12/ab")).isNull();
+    assertThat(parseRefSuffix("12/a4")).isNull();
+    assertThat(parseRefSuffix("12/4a")).isNull();
+    assertThat(parseRefSuffix("a4")).isNull();
+    assertThat(parseRefSuffix("4a")).isNull();
+  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/PatchLineCommentsUtil.java

@@ -23,7 +23,6 @@ import com.google.common.collect.ComparisonChain;
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
 import com.google.common.collect.Ordering;
 import com.google.gerrit.extensions.client.Side;
 import com.google.gerrit.extensions.common.CommentInfo;
@@ -52,7 +51,6 @@ import org.eclipse.jgit.lib.BatchRefUpdate;
 import org.eclipse.jgit.lib.NullProgressMonitor;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Ref;
-import org.eclipse.jgit.lib.RefDatabase;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.transport.ReceiveCommand;
@@ -62,8 +60,6 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
-import java.util.Map;
-import java.util.Set;
 
 /**
  * Utility functions to manipulate PatchLineComments.
@@ -169,8 +165,8 @@ public class PatchLineCommentsUtil {
     }
 
     List<PatchLineComment> comments = Lists.newArrayList();
-    for (String refSuffix : getDraftRefs(notes.getChangeId()).keySet()) {
+    for (Ref ref : getDraftRefs(notes.getChangeId())) {
-      Account.Id account = Account.Id.fromRefPart(refSuffix);
+      Account.Id account = Account.Id.fromRefSuffix(ref.getName());
       if (account != null) {
         comments.addAll(draftByChangeAuthor(db, notes, account));
       }
@@ -199,8 +195,8 @@ public class PatchLineCommentsUtil {
     List<PatchLineComment> comments = Lists.newArrayList();
     comments.addAll(publishedByPatchSet(db, notes, psId));
 
-    for (String refSuffix : getDraftRefs(notes.getChangeId()).keySet()) {
+    for (Ref ref : getDraftRefs(notes.getChangeId())) {
-      Account.Id account = Account.Id.fromRefPart(refSuffix);
+      Account.Id account = Account.Id.fromRefSuffix(ref.getName());
       if (account != null) {
         comments.addAll(draftByPatchSetAuthor(db, psId, account, notes));
       }
@@ -278,18 +274,25 @@ public class PatchLineCommentsUtil {
       return sort(db.patchComments().draftByAuthor(author).toList());
     }
 
-    Set<String> refNames =
-        getRefNamesAllUsers(RefNames.refsDraftCommentsPrefix(author));
     List<PatchLineComment> comments = Lists.newArrayList();
-    for (String refName : refNames) {
+    try (Repository repo = repoManager.openRepository(allUsers)) {
-      Change.Id changeId = Change.Id.parse(refName);
+      for (String refName : repo.getRefDatabase()
+          .getRefs(RefNames.REFS_DRAFT_COMMENTS).keySet()) {
+        Account.Id accountId = Account.Id.fromRefSuffix(refName);
+        Change.Id changeId = Change.Id.fromRefPart(refName);
+        if (accountId == null || changeId == null) {
+          continue;
+        }
         // Avoid loading notes for all affected changes just to be able to auto-
-      // rebuild. This is only used in a corner case in the search codepath, so
+        // rebuild. This is only used in a corner case in the search codepath,
-      // returning slightly stale values is ok.
+        // so returning slightly stale values is ok.
         DraftCommentNotes notes =
             draftFactory.createWithAutoRebuildingDisabled(changeId, author);
         comments.addAll(notes.load().getComments().values());
       }
+    } catch (IOException e) {
+      throw new OrmException(e);
+    }
     return sort(comments);
   }
 
@@ -314,7 +317,7 @@ public class PatchLineCommentsUtil {
     try (Repository repo = repoManager.openRepository(allUsers);
         RevWalk rw = new RevWalk(repo)) {
       BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate();
-      for (Ref ref : getDraftRefs(repo, changeId).values()) {
+      for (Ref ref : getDraftRefs(repo, changeId)) {
         bru.addCommand(new ReceiveCommand(
             ref.getObjectId(), ObjectId.zeroId(), ref.getName()));
       }
@@ -372,16 +375,7 @@ public class PatchLineCommentsUtil {
     return c.getRevId();
   }
 
-  private Set<String> getRefNamesAllUsers(String prefix) throws OrmException {
+  public Collection<Ref> getDraftRefs(Change.Id changeId)
-    try (Repository repo = repoManager.openRepository(allUsers)) {
-      RefDatabase refDb = repo.getRefDatabase();
-      return refDb.getRefs(prefix).keySet();
-    } catch (IOException e) {
-      throw new OrmException(e);
-    }
-  }
-
-  public Map<String, Ref> getDraftRefs(Change.Id changeId)
       throws OrmException {
     try (Repository repo = repoManager.openRepository(allUsers)) {
       return getDraftRefs(repo, changeId);
@@ -390,17 +384,10 @@ public class PatchLineCommentsUtil {
     }
   }
 
-  private Map<String, Ref> getDraftRefs(Repository repo,
+  private Collection<Ref> getDraftRefs(Repository repo, Change.Id changeId)
-      final Change.Id changeId) throws IOException {
+      throws IOException {
-    final String suffix = "/" + changeId.get();
+    return repo.getRefDatabase().getRefs(
-    return Maps.filterKeys(
+        RefNames.refsDraftCommentsPrefix(changeId)).values();
-        repo.getRefDatabase().getRefs(RefNames.REFS_DRAFT_COMMENTS),
-        new Predicate<String>() {
-          @Override
-          public boolean apply(String input) {
-            return input.endsWith(suffix);
-          }
-        });
   }
 
   private static List<PatchLineComment> sort(List<PatchLineComment> comments) {

gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java

@@ -221,7 +221,7 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
 
   @Override
   protected String getRefName() {
-    return RefNames.refsDraftComments(accountId, getId());
+    return RefNames.refsDraftComments(getId(), accountId);
   }
 
   @Override

gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java

@@ -31,7 +31,6 @@ import com.google.gerrit.reviewdb.client.Patch;
 import com.google.gerrit.reviewdb.client.PatchLineComment;
 import com.google.gerrit.reviewdb.client.PatchLineComment.Status;
 import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.reviewdb.client.RevId;
 import com.google.gerrit.reviewdb.server.ReviewDbUtil;
 import com.google.gerrit.server.GerritPersonIdent;
@@ -88,21 +87,6 @@ public class ChangeNoteUtil {
   private static final String UUID = "UUID";
   private static final String TAG = FOOTER_TAG.getName();
 
-  public static String changeRefName(Change.Id id) {
-    StringBuilder r = new StringBuilder();
-    r.append(RefNames.REFS_CHANGES);
-    int n = id.get();
-    int m = n % 100;
-    if (m < 10) {
-      r.append('0');
-    }
-    r.append(m);
-    r.append('/');
-    r.append(n);
-    r.append(RefNames.META_SUFFIX);
-    return r.toString();
-  }
-
   public static String formatTime(PersonIdent ident, Timestamp t) {
     GitDateFormatter dateFormatter = new GitDateFormatter(Format.DEFAULT);
     // TODO(dborowitz): Use a ThreadLocal or use Joda.

gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java

@@ -17,6 +17,7 @@ package com.google.gerrit.server.notedb;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
+import static com.google.gerrit.reviewdb.client.RefNames.changeMetaRef;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
@@ -530,7 +531,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
 
   @Override
   protected String getRefName() {
-    return ChangeNoteUtil.changeRefName(getChangeId());
+    return changeMetaRef(getChangeId());
   }
 
   public PatchSet getCurrentPatchSet() {

gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeRebuilderImpl.java

@@ -18,6 +18,7 @@ import static com.google.common.base.MoreObjects.firstNonNull;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
+import static com.google.gerrit.reviewdb.client.RefNames.changeMetaRef;
 import static com.google.gerrit.server.PatchLineCommentsUtil.setCommentRevId;
 import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_HASHTAGS;
 import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PATCH_SET;
@@ -401,7 +402,7 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
 
   private List<HashtagsEvent> getHashtagsEvents(Change change,
       NoteDbUpdateManager manager) throws IOException {
-    String refName = ChangeNoteUtil.changeRefName(change.getId());
+    String refName = changeMetaRef(change.getId());
     ObjectId old = manager.getChangeRepo().getObjectId(refName);
     if (old == null) {
       return Collections.emptyList();
@@ -460,7 +461,7 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
 
   private void deleteRef(Change change, Repository repo,
       ChainedReceiveCommands cmds) throws IOException {
-    String refName = ChangeNoteUtil.changeRefName(change.getId());
+    String refName = changeMetaRef(change.getId());
     ObjectId old = cmds.getObjectId(repo, refName);
     if (old != null) {
       cmds.add(new ReceiveCommand(old, ObjectId.zeroId(), refName));

gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java

@@ -18,6 +18,7 @@ import static com.google.common.base.MoreObjects.firstNonNull;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
+import static com.google.gerrit.reviewdb.client.RefNames.changeMetaRef;
 import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_BRANCH;
 import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_CHANGE_ID;
 import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_COMMIT;
@@ -480,7 +481,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
 
   @Override
   protected String getRefName() {
-    return ChangeNoteUtil.changeRefName(getId());
+    return changeMetaRef(getId());
   }
 
   @Override

gerrit-server/src/main/java/com/google/gerrit/server/notedb/DraftCommentNotes.java

@@ -110,7 +110,7 @@ public class DraftCommentNotes extends AbstractChangeNotes<DraftCommentNotes> {
 
   @Override
   protected String getRefName() {
-    return RefNames.refsDraftComments(author, getChangeId());
+    return RefNames.refsDraftComments(getChangeId(), author);
   }
 
   @Override

gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbChangeState.java

@@ -16,6 +16,7 @@ package com.google.gerrit.server.notedb;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.gerrit.reviewdb.client.RefNames.changeMetaRef;
 
 import com.google.auto.value.AutoValue;
 import com.google.common.annotations.VisibleForTesting;
@@ -166,7 +167,7 @@ public class NoteDbChangeState {
   }
 
   public boolean isChangeUpToDate(Repository changeRepo) throws IOException {
-    Ref ref = changeRepo.exactRef(ChangeNoteUtil.changeRefName(changeId));
+    Ref ref = changeRepo.exactRef(changeMetaRef(changeId));
     if (ref == null) {
       return changeMetaId.equals(ObjectId.zeroId());
     }
@@ -176,7 +177,7 @@ public class NoteDbChangeState {
   public boolean areDraftsUpToDate(Repository draftsRepo, Account.Id accountId)
       throws IOException {
     Ref ref = draftsRepo.exactRef(
-        RefNames.refsDraftComments(accountId, changeId));
+        RefNames.refsDraftComments(changeId, accountId));
     if (ref == null) {
       return !draftIds.containsKey(accountId);
     }

gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java

@@ -258,12 +258,10 @@ public class NoteDbUpdateManager {
     for (ReceiveCommand cmd : allUsersRepo.cmds.getCommands().values()) {
       String r = cmd.getRefName();
       if (r.startsWith(REFS_DRAFT_COMMENTS)) {
-        Account.Id accountId =
+        Change.Id changeId =
-            Account.Id.fromRefPart(r.substring(REFS_DRAFT_COMMENTS.length()));
+            Change.Id.fromRefPart(r.substring(REFS_DRAFT_COMMENTS.length()));
-        checkDraftRef(accountId != null, r);
+        Account.Id accountId = Account.Id.fromRefSuffix(r);
-        int s = r.lastIndexOf('/');
+        checkDraftRef(accountId != null && changeId != null, r);
-        checkDraftRef(s >= 0 && s < r.length() - 1, r);
-        Change.Id changeId = Change.Id.parse(r.substring(s + 1));
         draftIds.put(changeId, accountId, cmd.getNewId());
       }
     }

gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java

@@ -15,8 +15,8 @@
 package com.google.gerrit.server.notedb;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.reviewdb.client.RefNames.changeMetaRef;
 import static com.google.gerrit.reviewdb.client.RefNames.refsDraftComments;
-import static com.google.gerrit.server.notedb.ChangeNoteUtil.changeRefName;
 import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC;
 import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
 import static com.google.gerrit.testutil.TestChanges.incrementPatchSet;
@@ -526,7 +526,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
   @Test
   public void emptyChangeUpdate() throws Exception {
     Change c = newChange();
-    Ref initial = repo.exactRef(changeRefName(c.getId()));
+    Ref initial = repo.exactRef(changeMetaRef(c.getId()));
     assertThat(initial).isNotNull();
 
     // Empty update doesn't create a new commit.
@@ -534,7 +534,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
     update.commit();
     assertThat(update.getResult()).isNull();
 
-    Ref updated = repo.exactRef(changeRefName(c.getId()));
+    Ref updated = repo.exactRef(changeMetaRef(c.getId()));
     assertThat(updated.getObjectId()).isEqualTo(initial.getObjectId());
   }
 
@@ -1930,8 +1930,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
     update.putComment(comment);
     update.commit();
 
-    assertThat(repo.exactRef(changeRefName(c.getId()))).isNotNull();
+    assertThat(repo.exactRef(changeMetaRef(c.getId()))).isNotNull();
-    String draftRef = refsDraftComments(otherUser.getAccountId(), c.getId());
+    String draftRef = refsDraftComments(c.getId(), otherUser.getAccountId());
     assertThat(exactRefAllUsers(draftRef)).isNull();
   }
 
@@ -1953,7 +1953,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
     update.putComment(draft);
     update.commit();
 
-    String draftRef = refsDraftComments(otherUser.getAccountId(), c.getId());
+    String draftRef = refsDraftComments(c.getId(), otherUser.getAccountId());
     ObjectId old = exactRefAllUsers(draftRef);
     assertThat(old).isNotNull();
 
@@ -2127,7 +2127,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
     update.putComment(comment2);
     update.commit();
 
-    String refName = refsDraftComments(otherUserId, c.getId());
+    String refName = refsDraftComments(c.getId(), otherUserId);
     ObjectId oldDraftId = exactRefAllUsers(refName);
 
     update = newUpdate(c, otherUser);

gerrit-server/src/test/java/com/google/gerrit/testutil/NoteDbChecker.java

@@ -20,12 +20,12 @@ import com.google.common.base.Joiner;
 import com.google.common.collect.Iterables;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.reviewdb.server.ReviewDbUtil;
 import com.google.gerrit.server.PatchLineCommentsUtil;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.notedb.ChangeBundle;
-import com.google.gerrit.server.notedb.ChangeNoteUtil;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.notedb.ChangeRebuilder;
 import com.google.gerrit.server.schema.DisabledChangesReviewDbWrapper;
@@ -118,8 +118,7 @@ public class NoteDbChecker {
   public void assertNoChangeRef(Project.NameKey project, Change.Id changeId)
       throws Exception {
     try (Repository repo = repoManager.openRepository(project)) {
-      assertThat(repo.exactRef(ChangeNoteUtil.changeRefName(changeId)))
+      assertThat(repo.exactRef(RefNames.changeMetaRef(changeId))).isNull();
-          .isNull();
     }
   }