Combine ChangeNoteUtil and CommentsInNotesUtil

Now that we have to actually inject both of these into ChangeNotes,
it was getting a little unwieldy. There was practically nothing in
ChangeNoteUtil itself anyway. Merge CommentsInNotesUtil into
ChangeNoteUtil so we only have one thing to inject.

Change-Id: I41ef88dd2911b9d007f8b3a4cf7d3d7159fa25e6

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

@@ -44,7 +44,7 @@ public abstract class AbstractChangeUpdate {
   protected final GitRepositoryManager repoManager;
   protected final ChangeControl ctl;
   protected final String anonymousCowardName;
-  protected final ChangeNoteUtil changeNoteUtil;
+  protected final ChangeNoteUtil noteUtil;
   protected final Date when;
   private final PersonIdent serverIdent;
 
@@ -56,14 +56,14 @@ public abstract class AbstractChangeUpdate {
       ChangeControl ctl,
       PersonIdent serverIdent,
       String anonymousCowardName,
-      ChangeNoteUtil changeNoteUtil,
+      ChangeNoteUtil noteUtil,
       Date when) {
     this.migration = migration;
     this.repoManager = repoManager;
     this.ctl = ctl;
     this.serverIdent = serverIdent;
     this.anonymousCowardName = anonymousCowardName;
-    this.changeNoteUtil = changeNoteUtil;
+    this.noteUtil = noteUtil;
     this.when = when;
     checkArgument(
         (ctl.getUser() instanceof IdentifiedUser)
@@ -99,7 +99,7 @@ public abstract class AbstractChangeUpdate {
   private PersonIdent newAuthorIdent() {
     CurrentUser u = getUser();
     if (u instanceof IdentifiedUser) {
-      return changeNoteUtil.newIdent(u.asIdentifiedUser().getAccount(), when,
+      return noteUtil.newIdent(u.asIdentifiedUser().getAccount(), when,
           serverIdent, anonymousCowardName);
     } else if (u instanceof InternalUser) {
       return serverIdent;
@@ -108,8 +108,7 @@ public abstract class AbstractChangeUpdate {
   }
 
   protected PersonIdent newIdent(Account author, Date when) {
-    return changeNoteUtil.newIdent(author, when, serverIdent,
+    return noteUtil.newIdent(author, when, serverIdent, anonymousCowardName);
-        anonymousCowardName);
   }
 
   /** Whether no updates have been done. */

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

@@ -78,7 +78,6 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
 
   private final AllUsersName draftsProject;
   private final Account.Id accountId;
-  private final CommentsInNotesUtil commentsUtil;
 
   // TODO: can go back to a list?
   private Map<Key, PatchLineComment> put;
@@ -91,14 +90,12 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
       GitRepositoryManager repoManager,
       NotesMigration migration,
       AllUsersName allUsers,
-      ChangeNoteUtil changeNoteUtil,
+      ChangeNoteUtil noteUtil,
-      CommentsInNotesUtil commentsUtil,
       @Assisted ChangeControl ctl,
       @Assisted Date when) {
     super(migration, repoManager, ctl, serverIdent, anonymousCowardName,
-        changeNoteUtil, when);
+        noteUtil, when);
     this.draftsProject = allUsers;
-    this.commentsUtil = commentsUtil;
     checkState(ctl.getUser().isIdentifiedUser(),
         "Current user must be identified");
     IdentifiedUser user = ctl.getUser().asIdentifiedUser();
@@ -152,7 +149,7 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
     for (Map.Entry<RevId, RevisionNoteBuilder> e : builders.entrySet()) {
       updatedRevs.add(e.getKey());
       ObjectId id = ObjectId.fromString(e.getKey().get());
-      byte[] data = e.getValue().build(commentsUtil);
+      byte[] data = e.getValue().build(noteUtil);
       if (data.length == 0) {
         rnm.noteMap.remove(id);
       } else {
@@ -197,7 +194,7 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
     // Even though reading from changes might not be enabled, we need to
     // parse any existing revision notes so we can merge them.
     return RevisionNoteMap.parse(
-        commentsUtil, ctl.getId(), rw.getObjectReader(), noteMap, true);
+        noteUtil, ctl.getId(), rw.getObjectReader(), noteMap, true);
   }
 
   @Override

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

@@ -14,21 +14,49 @@
 
 package com.google.gerrit.server.notedb;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.gerrit.server.notedb.ChangeNotes.parseException;
+import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
 import com.google.common.primitives.Ints;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.CommentRange;
+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.server.GerritPersonIdent;
+import com.google.gerrit.server.PatchLineCommentsUtil;
+import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.config.AnonymousCowardName;
 import com.google.gerrit.server.config.GerritServerId;
 import com.google.inject.Inject;
 
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.revwalk.FooterKey;
+import org.eclipse.jgit.util.GitDateFormatter;
+import org.eclipse.jgit.util.GitDateFormatter.Format;
+import org.eclipse.jgit.util.GitDateParser;
+import org.eclipse.jgit.util.MutableInteger;
+import org.eclipse.jgit.util.QuotedString;
+import org.eclipse.jgit.util.RawParseUtils;
 
+import java.io.ByteArrayOutputStream;
+import java.io.OutputStream;
+import java.io.OutputStreamWriter;
+import java.io.PrintWriter;
+import java.sql.Timestamp;
+import java.text.ParseException;
 import java.util.Date;
+import java.util.List;
+import java.util.Locale;
 
 public class ChangeNoteUtil {
   static final FooterKey FOOTER_BRANCH = new FooterKey("Branch");
@@ -45,6 +73,16 @@ public class ChangeNoteUtil {
       new FooterKey("Submitted-with");
   static final FooterKey FOOTER_TOPIC = new FooterKey("Topic");
 
+  private static final String AUTHOR = "Author";
+  private static final String BASE_PATCH_SET = "Base-for-patch-set";
+  private static final String COMMENT_RANGE = "Comment-range";
+  private static final String FILE = "File";
+  private static final String LENGTH = "Bytes";
+  private static final String PARENT = "Parent";
+  private static final String PATCH_SET = "Patch-set";
+  private static final String REVISION = "Revision";
+  private static final String UUID = "UUID";
+
   public static String changeRefName(Change.Id id) {
     StringBuilder r = new StringBuilder();
     r.append(RefNames.REFS_CHANGES);
@@ -60,10 +98,26 @@ public class ChangeNoteUtil {
     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.
+    PersonIdent newIdent = new PersonIdent(ident, t);
+    return dateFormatter.formatDate(newIdent);
+  }
+
+  private final AccountCache accountCache;
+  private final PersonIdent serverIdent;
+  private final String anonymousCowardName;
   private final String serverId;
 
   @Inject
-  ChangeNoteUtil(@GerritServerId String serverId) {
+  public ChangeNoteUtil(AccountCache accountCache,
+      @GerritPersonIdent PersonIdent serverIdent,
+      @AnonymousCowardName String anonymousCowardName,
+      @GerritServerId String serverId) {
+    this.accountCache = accountCache;
+    this.serverIdent = serverIdent;
+    this.anonymousCowardName = anonymousCowardName;
     this.serverId = serverId;
   }
 
@@ -92,4 +146,361 @@ public class ChangeNoteUtil {
     throw parseException(changeId, "invalid identity, expected <id>@%s: %s",
         serverId, email);
   }
+
+  public List<PatchLineComment> parseNote(byte[] note, MutableInteger p,
+      Change.Id changeId, Status status) throws ConfigInvalidException {
+    if (p.value >= note.length) {
+      return ImmutableList.of();
+    }
+    List<PatchLineComment> result = Lists.newArrayList();
+    int sizeOfNote = note.length;
+
+    boolean isForBase =
+        (RawParseUtils.match(note, p.value, PATCH_SET.getBytes(UTF_8))) < 0;
+
+    PatchSet.Id psId = parsePsId(note, p, changeId, isForBase ? BASE_PATCH_SET : PATCH_SET);
+
+    RevId revId =
+        new RevId(parseStringField(note, p, changeId, REVISION));
+
+    PatchLineComment c = null;
+    while (p.value < sizeOfNote) {
+      String previousFileName = c == null ?
+          null : c.getKey().getParentKey().getFileName();
+      c = parseComment(note, p, previousFileName, psId, revId,
+          isForBase, status);
+      result.add(c);
+    }
+    return result;
+  }
+
+  private PatchLineComment parseComment(byte[] note, MutableInteger curr,
+      String currentFileName, PatchSet.Id psId, RevId revId, boolean isForBase,
+      Status status) throws ConfigInvalidException {
+    Change.Id changeId = psId.getParentKey();
+
+    // Check if there is a new file.
+    boolean newFile =
+        (RawParseUtils.match(note, curr.value, FILE.getBytes(UTF_8))) != -1;
+    if (newFile) {
+      // If so, parse the new file name.
+      currentFileName = parseFilename(note, curr, changeId);
+    } else if (currentFileName == null) {
+      throw parseException(changeId, "could not parse %s", FILE);
+    }
+
+    CommentRange range = parseCommentRange(note, curr);
+    if (range == null) {
+      throw parseException(changeId, "could not parse %s", COMMENT_RANGE);
+    }
+
+    Timestamp commentTime = parseTimestamp(note, curr, changeId);
+    Account.Id aId = parseAuthor(note, curr, changeId);
+
+    boolean hasParent =
+        (RawParseUtils.match(note, curr.value, PARENT.getBytes(UTF_8))) != -1;
+    String parentUUID = null;
+    if (hasParent) {
+      parentUUID = parseStringField(note, curr, changeId, PARENT);
+    }
+
+    String uuid = parseStringField(note, curr, changeId, UUID);
+    int commentLength = parseCommentLength(note, curr, changeId);
+
+    String message = RawParseUtils.decode(
+        UTF_8, note, curr.value, curr.value + commentLength);
+    checkResult(message, "message contents", changeId);
+
+    PatchLineComment plc = new PatchLineComment(
+        new PatchLineComment.Key(new Patch.Key(psId, currentFileName), uuid),
+        range.getEndLine(), aId, parentUUID, commentTime);
+    plc.setMessage(message);
+    plc.setSide((short) (isForBase ? 0 : 1));
+    if (range.getStartCharacter() != -1) {
+      plc.setRange(range);
+    }
+    plc.setRevId(revId);
+    plc.setStatus(status);
+
+    curr.value = RawParseUtils.nextLF(note, curr.value + commentLength);
+    curr.value = RawParseUtils.nextLF(note, curr.value);
+    return plc;
+  }
+
+  private static String parseStringField(byte[] note, MutableInteger curr,
+      Change.Id changeId, String fieldName) throws ConfigInvalidException {
+    int endOfLine = RawParseUtils.nextLF(note, curr.value);
+    checkHeaderLineFormat(note, curr, fieldName, changeId);
+    int startOfField = RawParseUtils.endOfFooterLineKey(note, curr.value) + 2;
+    curr.value = endOfLine;
+    return RawParseUtils.decode(UTF_8, note, startOfField, endOfLine - 1);
+  }
+
+  /**
+   * @return a comment range. If the comment range line in the note only has
+   *    one number, we return a CommentRange with that one number as the end
+   *    line and the other fields as -1. If the comment range line in the note
+   *    contains a whole comment range, then we return a CommentRange with all
+   *    fields set. If the line is not correctly formatted, return null.
+   */
+  private static CommentRange parseCommentRange(byte[] note, MutableInteger ptr) {
+    CommentRange range = new CommentRange(-1, -1, -1, -1);
+
+    int startLine = RawParseUtils.parseBase10(note, ptr.value, ptr);
+    if (startLine == 0) {
+      range.setEndLine(0);
+      ptr.value += 1;
+      return range;
+    }
+
+    if (note[ptr.value] == '\n') {
+      range.setEndLine(startLine);
+      ptr.value += 1;
+      return range;
+    } else if (note[ptr.value] == ':') {
+      range.setStartLine(startLine);
+      ptr.value += 1;
+    } else {
+      return null;
+    }
+
+    int startChar = RawParseUtils.parseBase10(note, ptr.value, ptr);
+    if (note[ptr.value] == '-') {
+      range.setStartCharacter(startChar);
+      ptr.value += 1;
+    } else {
+      return null;
+    }
+
+    int endLine = RawParseUtils.parseBase10(note, ptr.value, ptr);
+    if (endLine == 0) {
+      return null;
+    }
+    if (note[ptr.value] == ':') {
+      range.setEndLine(endLine);
+      ptr.value += 1;
+    } else {
+      return null;
+    }
+
+    int endChar = RawParseUtils.parseBase10(note, ptr.value, ptr);
+    if (endChar == 0) {
+      return null;
+    }
+    if (note[ptr.value] == '\n') {
+      range.setEndCharacter(endChar);
+      ptr.value += 1;
+    } else {
+      return null;
+    }
+    return range;
+  }
+
+  private static PatchSet.Id parsePsId(byte[] note, MutableInteger curr,
+      Change.Id changeId, String fieldName) throws ConfigInvalidException {
+    checkHeaderLineFormat(note, curr, fieldName, changeId);
+    int startOfPsId =
+        RawParseUtils.endOfFooterLineKey(note, curr.value) + 1;
+    MutableInteger i = new MutableInteger();
+    int patchSetId =
+        RawParseUtils.parseBase10(note, startOfPsId, i);
+    int endOfLine = RawParseUtils.nextLF(note, curr.value);
+    if (i.value != endOfLine - 1) {
+      throw parseException(changeId, "could not parse %s", fieldName);
+    }
+    checkResult(patchSetId, "patchset id", changeId);
+    curr.value = endOfLine;
+    return new PatchSet.Id(changeId, patchSetId);
+  }
+
+  private static String parseFilename(byte[] note, MutableInteger curr,
+      Change.Id changeId) throws ConfigInvalidException {
+    checkHeaderLineFormat(note, curr, FILE, changeId);
+    int startOfFileName =
+        RawParseUtils.endOfFooterLineKey(note, curr.value) + 2;
+    int endOfLine = RawParseUtils.nextLF(note, curr.value);
+    curr.value = endOfLine;
+    curr.value = RawParseUtils.nextLF(note, curr.value);
+    return QuotedString.GIT_PATH.dequote(
+        RawParseUtils.decode(UTF_8, note, startOfFileName, endOfLine - 1));
+  }
+
+  private static Timestamp parseTimestamp(byte[] note, MutableInteger curr,
+      Change.Id changeId) throws ConfigInvalidException {
+    int endOfLine = RawParseUtils.nextLF(note, curr.value);
+    Timestamp commentTime;
+    String dateString =
+        RawParseUtils.decode(UTF_8, note, curr.value, endOfLine - 1);
+    try {
+      commentTime = new Timestamp(
+          GitDateParser.parse(dateString, null, Locale.US).getTime());
+    } catch (ParseException e) {
+      throw new ConfigInvalidException("could not parse comment timestamp", e);
+    }
+    curr.value = endOfLine;
+    return checkResult(commentTime, "comment timestamp", changeId);
+  }
+
+  private Account.Id parseAuthor(byte[] note, MutableInteger curr,
+      Change.Id changeId) throws ConfigInvalidException {
+    checkHeaderLineFormat(note, curr, AUTHOR, changeId);
+    int startOfAccountId =
+        RawParseUtils.endOfFooterLineKey(note, curr.value) + 2;
+    PersonIdent ident =
+        RawParseUtils.parsePersonIdent(note, startOfAccountId);
+    Account.Id aId = parseIdent(ident, changeId);
+    curr.value = RawParseUtils.nextLF(note, curr.value);
+    return checkResult(aId, "comment author", changeId);
+  }
+
+  private static int parseCommentLength(byte[] note, MutableInteger curr,
+      Change.Id changeId) throws ConfigInvalidException {
+    checkHeaderLineFormat(note, curr, LENGTH, changeId);
+    int startOfLength =
+        RawParseUtils.endOfFooterLineKey(note, curr.value) + 1;
+    MutableInteger i = new MutableInteger();
+    int commentLength =
+        RawParseUtils.parseBase10(note, startOfLength, i);
+    int endOfLine = RawParseUtils.nextLF(note, curr.value);
+    if (i.value != endOfLine-1) {
+      throw parseException(changeId, "could not parse %s", PATCH_SET);
+    }
+    curr.value = endOfLine;
+    return checkResult(commentLength, "comment length", changeId);
+  }
+
+  private static <T> T checkResult(T o, String fieldName,
+      Change.Id changeId) throws ConfigInvalidException {
+    if (o == null) {
+      throw parseException(changeId, "could not parse %s", fieldName);
+    }
+    return o;
+  }
+
+  private static int checkResult(int i, String fieldName, Change.Id changeId)
+      throws ConfigInvalidException {
+    if (i <= 0) {
+      throw parseException(changeId, "could not parse %s", fieldName);
+    }
+    return i;
+  }
+
+  private void appendHeaderField(PrintWriter writer,
+      String field, String value) {
+    writer.print(field);
+    writer.print(": ");
+    writer.print(value);
+    writer.print('\n');
+  }
+
+  private static void checkHeaderLineFormat(byte[] note, MutableInteger curr,
+      String fieldName, Change.Id changeId) throws ConfigInvalidException {
+    boolean correct =
+        RawParseUtils.match(note, curr.value, fieldName.getBytes(UTF_8)) != -1;
+    int p = curr.value + fieldName.length();
+    correct &= (p < note.length && note[p] == ':');
+    p++;
+    correct &= (p < note.length && note[p] == ' ');
+    if (!correct) {
+      throw parseException(changeId, "could not parse %s", fieldName);
+    }
+  }
+
+  public byte[] buildNote(List<PatchLineComment> comments) {
+    ByteArrayOutputStream out = new ByteArrayOutputStream();
+    buildNote(comments, out);
+    return out.toByteArray();
+  }
+
+  /**
+   * Build a note that contains the metadata for and the contents of all of the
+   * comments in the given list of comments.
+   *
+   * @param comments A list of the comments to be written to the
+   *            output stream. All of the comments in this list must have the
+   *            same side and must share the same patch set ID.
+   * @param out output stream to write to.
+   */
+  void buildNote(List<PatchLineComment> comments, OutputStream out) {
+    if (comments.isEmpty()) {
+      return;
+    }
+    OutputStreamWriter streamWriter = new OutputStreamWriter(out, UTF_8);
+    try (PrintWriter writer = new PrintWriter(streamWriter)) {
+      PatchLineComment first = comments.get(0);
+
+      short side = first.getSide();
+      PatchSet.Id psId = PatchLineCommentsUtil.getCommentPsId(first);
+      appendHeaderField(writer, side == 0
+          ? BASE_PATCH_SET
+          : PATCH_SET,
+          Integer.toString(psId.get()));
+      appendHeaderField(writer, REVISION, first.getRevId().get());
+
+      String currentFilename = null;
+
+      for (PatchLineComment c : comments) {
+        PatchSet.Id currentPsId = PatchLineCommentsUtil.getCommentPsId(c);
+        checkArgument(psId.equals(currentPsId),
+            "All comments being added must all have the same PatchSet.Id. The"
+            + "comment below does not have the same PatchSet.Id as the others "
+            + "(%s).\n%s", psId.toString(), c.toString());
+        checkArgument(side == c.getSide(),
+            "All comments being added must all have the same side. The"
+            + "comment below does not have the same side as the others "
+            + "(%s).\n%s", side, c.toString());
+        String commentFilename =
+            QuotedString.GIT_PATH.quote(c.getKey().getParentKey().getFileName());
+
+        if (!commentFilename.equals(currentFilename)) {
+          currentFilename = commentFilename;
+          writer.print("File: ");
+          writer.print(commentFilename);
+          writer.print("\n\n");
+        }
+
+        // The CommentRange field for a comment is allowed to be null.
+        // If it is indeed null, then in the first line, we simply use the line
+        // number field for a comment instead. If it isn't null, we write the
+        // comment range itself.
+        CommentRange range = c.getRange();
+        if (range != null) {
+          writer.print(range.getStartLine());
+          writer.print(':');
+          writer.print(range.getStartCharacter());
+          writer.print('-');
+          writer.print(range.getEndLine());
+          writer.print(':');
+          writer.print(range.getEndCharacter());
+        } else {
+          writer.print(c.getLine());
+        }
+        writer.print("\n");
+
+        writer.print(formatTime(serverIdent, c.getWrittenOn()));
+        writer.print("\n");
+
+        PersonIdent ident = newIdent(
+            accountCache.get(c.getAuthor()).getAccount(),
+            c.getWrittenOn(), serverIdent, anonymousCowardName);
+        String nameString = ident.getName() + " <" + ident.getEmailAddress()
+            + ">";
+        appendHeaderField(writer, AUTHOR, nameString);
+
+        String parent = c.getParentUuid();
+        if (parent != null) {
+          appendHeaderField(writer, PARENT, parent);
+        }
+
+        appendHeaderField(writer, UUID, c.getKey().get());
+
+        byte[] messageBytes = c.getMessage().getBytes(UTF_8);
+        appendHeaderField(writer, LENGTH,
+            Integer.toString(messageBytes.length));
+
+        writer.print(c.getMessage());
+        writer.print("\n\n");
+      }
+    }
+  }
 }

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

@@ -119,8 +119,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
     private final AllUsersName allUsers;
     private final Provider<InternalChangeQuery> queryProvider;
     private final ProjectCache projectCache;
-    private final ChangeNoteUtil changeNoteUtil;
+    private final ChangeNoteUtil noteUtil;
-    private final CommentsInNotesUtil commentsUtil;
 
     @VisibleForTesting
     @Inject
@@ -129,15 +128,13 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
         AllUsersName allUsers,
         Provider<InternalChangeQuery> queryProvider,
         ProjectCache projectCache,
-        ChangeNoteUtil changeNoteUtil,
+        ChangeNoteUtil noteUtil) {
-        CommentsInNotesUtil commentsUtil) {
       this.repoManager = repoManager;
       this.migration = migration;
       this.allUsers = allUsers;
       this.queryProvider = queryProvider;
       this.projectCache = projectCache;
-      this.changeNoteUtil = changeNoteUtil;
+      this.noteUtil = noteUtil;
-      this.commentsUtil = commentsUtil;
     }
 
     public ChangeNotes createChecked(ReviewDb db, Change c)
@@ -182,8 +179,8 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
           project, changeId, change.getProject());
       // TODO: Throw NoSuchChangeException when the change is not found in the
       // database
-      return new ChangeNotes(repoManager, migration, allUsers, changeNoteUtil,
+      return new ChangeNotes(repoManager, migration, allUsers, noteUtil,
-          commentsUtil, project, change).load();
+          project, change).load();
     }
 
     /**
@@ -195,13 +192,13 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
      * @return change notes
      */
     public ChangeNotes createFromIndexedChange(Change change) {
-      return new ChangeNotes(repoManager, migration, allUsers, changeNoteUtil,
+      return new ChangeNotes(repoManager, migration, allUsers, noteUtil,
-          commentsUtil, change.getProject(), change);
+          change.getProject(), change);
     }
 
     public ChangeNotes createForNew(Change change) throws OrmException {
-      return new ChangeNotes(repoManager, migration, allUsers, changeNoteUtil,
+      return new ChangeNotes(repoManager, migration, allUsers, noteUtil,
-          commentsUtil, change.getProject(), change).load();
+          change.getProject(), change).load();
     }
 
     // TODO(dborowitz): Remove when deleting index schemas <27.
@@ -210,8 +207,8 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
     checkState(!migration.readChanges(), "do not call"
         + " createFromIdOnlyWhenNotedbDisabled when notedb is enabled");
       Change change = unwrap(db).changes().get(changeId);
-      return new ChangeNotes(repoManager, migration, allUsers, changeNoteUtil,
+      return new ChangeNotes(repoManager, migration, allUsers, noteUtil,
-          commentsUtil, change.getProject(), change).load();
+          change.getProject(), change).load();
     }
 
     // TODO(ekempin): Remove when database backend is deleted
@@ -223,8 +220,8 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
         throws OrmException {
       checkState(!migration.readChanges(), "do not call"
           + " createFromChangeWhenNotedbDisabled when notedb is enabled");
-      return new ChangeNotes(repoManager, migration, allUsers, changeNoteUtil,
+      return new ChangeNotes(repoManager, migration, allUsers, noteUtil,
-          commentsUtil, change.getProject(), change).load();
+          change.getProject(), change).load();
     }
 
     public CheckedFuture<ChangeNotes, OrmException> createAsync(
@@ -244,7 +241,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
                               + " but actual project is %s",
                           project, changeId, change.getProject());
                       return new ChangeNotes(repoManager, migration, allUsers,
-                          changeNoteUtil, commentsUtil, project, change).load();
+                          noteUtil, project, change).load();
                     }
                   });
                 }
@@ -383,8 +380,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
     }
   }
 
-  private final ChangeNoteUtil changeNoteUtil;
+  private final ChangeNoteUtil noteUtil;
-  private final CommentsInNotesUtil commentsUtil;
   private final Project.NameKey project;
   private final Change change;
   private ImmutableSortedMap<PatchSet.Id, PatchSet> patchSets;
@@ -406,13 +402,11 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
 
   @VisibleForTesting
   public ChangeNotes(GitRepositoryManager repoManager, NotesMigration migration,
-      AllUsersName allUsers, ChangeNoteUtil changeNoteUtil,
+      AllUsersName allUsers, ChangeNoteUtil noteUtil,
-      CommentsInNotesUtil commentsUtil, Project.NameKey project,
+      Project.NameKey project, Change change) {
-      Change change) {
     super(repoManager, migration, change != null ? change.getId() : null);
     this.allUsers = allUsers;
-    this.changeNoteUtil = changeNoteUtil;
+    this.noteUtil = noteUtil;
-    this.commentsUtil = commentsUtil;
     this.project = project;
     this.change = change != null ? new Change(change) : null;
   }
@@ -510,7 +504,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
     if (draftCommentNotes == null ||
         !author.equals(draftCommentNotes.getAuthor())) {
       draftCommentNotes = new DraftCommentNotes(repoManager, migration,
-          allUsers, commentsUtil, getChangeId(), author);
+          allUsers, noteUtil, getChangeId(), author);
       draftCommentNotes.load();
     }
   }
@@ -556,9 +550,8 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
       return;
     }
     try (RevWalk walk = new RevWalk(reader);
-        ChangeNotesParser parser = new ChangeNotesParser(project,
+        ChangeNotesParser parser = new ChangeNotesParser(
-            change.getId(), rev, walk, repoManager, changeNoteUtil,
+            project, change.getId(), rev, walk, repoManager, noteUtil)) {
-            commentsUtil)) {
       parser.parseAll();
 
       if (parser.status != null) {

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

@@ -114,8 +114,7 @@ class ChangeNotesParser implements AutoCloseable {
   PatchSet.Id currentPatchSetId;
   RevisionNoteMap revisionNoteMap;
 
-  private final ChangeNoteUtil changeNoteUtil;
+  private final ChangeNoteUtil noteUtil;
-  private final CommentsInNotesUtil commentsUtil;
   private final Change.Id id;
   private final ObjectId tip;
   private final RevWalk walk;
@@ -127,14 +126,13 @@ class ChangeNotesParser implements AutoCloseable {
 
   ChangeNotesParser(Project.NameKey project, Change.Id changeId, ObjectId tip,
       RevWalk walk, GitRepositoryManager repoManager,
-      ChangeNoteUtil changeNoteUtil, CommentsInNotesUtil commentsUtil)
+      ChangeNoteUtil noteUtil)
       throws RepositoryNotFoundException, IOException {
     this.id = changeId;
     this.tip = tip;
     this.walk = walk;
     this.repo = repoManager.openMetadataRepository(project);
-    this.changeNoteUtil = changeNoteUtil;
+    this.noteUtil = noteUtil;
-    this.commentsUtil = commentsUtil;
     approvals = Maps.newHashMap();
     reviewers = Maps.newLinkedHashMap();
     allPastReviewers = Lists.newArrayList();
@@ -505,7 +503,7 @@ class ChangeNotesParser implements AutoCloseable {
     ObjectReader reader = walk.getObjectReader();
     RevCommit tipCommit = walk.parseCommit(tip);
     revisionNoteMap = RevisionNoteMap.parse(
-        commentsUtil, id, reader, NoteMap.read(reader, tipCommit), false);
+        noteUtil, id, reader, NoteMap.read(reader, tipCommit), false);
     Map<RevId, RevisionNote> rns = revisionNoteMap.revisionNotes;
 
     for (Map.Entry<RevId, RevisionNote> e : rns.entrySet()) {
@@ -544,7 +542,7 @@ class ChangeNotesParser implements AutoCloseable {
       labelVoteStr = line.substring(0, s);
       PersonIdent ident = RawParseUtils.parsePersonIdent(line.substring(s + 1));
       checkFooter(ident != null, FOOTER_LABEL, line);
-      accountId = changeNoteUtil.parseIdent(ident, id);
+      accountId = noteUtil.parseIdent(ident, id);
     } else {
       labelVoteStr = line;
       accountId = committerId;
@@ -582,7 +580,7 @@ class ChangeNotesParser implements AutoCloseable {
       label = line.substring(1, s);
       PersonIdent ident = RawParseUtils.parsePersonIdent(line.substring(s + 1));
       checkFooter(ident != null, FOOTER_LABEL, line);
-      accountId = changeNoteUtil.parseIdent(ident, id);
+      accountId = noteUtil.parseIdent(ident, id);
     } else {
       label = line.substring(1);
       accountId = committerId;
@@ -665,7 +663,7 @@ class ChangeNotesParser implements AutoCloseable {
           PersonIdent ident =
               RawParseUtils.parsePersonIdent(line.substring(c2 + 2));
           checkFooter(ident != null, FOOTER_SUBMITTED_WITH, line);
-          label.appliedBy = changeNoteUtil.parseIdent(ident, id);
+          label.appliedBy = noteUtil.parseIdent(ident, id);
         } else {
           label.label = line.substring(c + 2);
         }
@@ -683,7 +681,7 @@ class ChangeNotesParser implements AutoCloseable {
         && a.getEmailAddress().equals(c.getEmailAddress())) {
       return null;
     }
-    return changeNoteUtil.parseIdent(commit.getAuthorIdent(), id);
+    return noteUtil.parseIdent(commit.getAuthorIdent(), id);
   }
 
   private void parseReviewer(ReviewerStateInternal state, String line)
@@ -692,7 +690,7 @@ class ChangeNotesParser implements AutoCloseable {
     if (ident == null) {
       throw invalidFooter(state.getFooterKey(), line);
     }
-    Account.Id accountId = changeNoteUtil.parseIdent(ident, id);
+    Account.Id accountId = noteUtil.parseIdent(ident, id);
     if (!reviewers.containsKey(accountId)) {
       reviewers.put(accountId, state);
     }

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

@@ -100,7 +100,6 @@ public class ChangeUpdate extends AbstractChangeUpdate {
   }
 
   private final AccountCache accountCache;
-  private final CommentsInNotesUtil commentsUtil;
   private final ChangeDraftUpdate.Factory draftUpdateFactory;
   private final NoteDbUpdateManager.Factory updateManagerFactory;
 
@@ -136,11 +135,10 @@ public class ChangeUpdate extends AbstractChangeUpdate {
       ChangeDraftUpdate.Factory draftUpdateFactory,
       ProjectCache projectCache,
       @Assisted ChangeControl ctl,
-      CommentsInNotesUtil commentsUtil,
+      ChangeNoteUtil noteUtil) {
-      ChangeNoteUtil changeNoteUtil) {
     this(serverIdent, anonymousCowardName, repoManager, migration, accountCache,
         updateManagerFactory, draftUpdateFactory,
-        projectCache, ctl, serverIdent.getWhen(), commentsUtil, changeNoteUtil);
+        projectCache, ctl, serverIdent.getWhen(), noteUtil);
   }
 
   @AssistedInject
@@ -155,13 +153,12 @@ public class ChangeUpdate extends AbstractChangeUpdate {
       ProjectCache projectCache,
       @Assisted ChangeControl ctl,
       @Assisted Date when,
-      CommentsInNotesUtil commentsUtil,
+      ChangeNoteUtil noteUtil) {
-      ChangeNoteUtil changeNoteUtil) {
     this(serverIdent, anonymousCowardName, repoManager, migration, accountCache,
         updateManagerFactory, draftUpdateFactory, ctl,
         when,
         projectCache.get(getProjectName(ctl)).getLabelTypes().nameComparator(),
-        commentsUtil, changeNoteUtil);
+        noteUtil);
   }
 
   private static Project.NameKey getProjectName(ChangeControl ctl) {
@@ -180,12 +177,10 @@ public class ChangeUpdate extends AbstractChangeUpdate {
       @Assisted ChangeControl ctl,
       @Assisted Date when,
       @Assisted Comparator<String> labelNameComparator,
-      CommentsInNotesUtil commentsUtil,
+      ChangeNoteUtil noteUtil) {
-      ChangeNoteUtil changeNoteUtil) {
     super(migration, repoManager, ctl, serverIdent,
-        anonymousCowardName, changeNoteUtil, when);
+        anonymousCowardName, noteUtil, when);
     this.accountCache = accountCache;
-    this.commentsUtil = commentsUtil;
     this.draftUpdateFactory = draftUpdateFactory;
     this.updateManagerFactory = updateManagerFactory;
 
@@ -378,7 +373,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
 
     for (Map.Entry<RevId, RevisionNoteBuilder> e : builders.entrySet()) {
       ObjectId data = inserter.insert(
-          OBJ_BLOB, e.getValue().build(commentsUtil));
+          OBJ_BLOB, e.getValue().build(noteUtil));
       rnm.noteMap.set(ObjectId.fromString(e.getKey().get()), data);
     }
 
@@ -404,7 +399,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
     // Even though reading from changes might not be enabled, we need to
     // parse any existing revision notes so we can merge them.
     return RevisionNoteMap.parse(
-        commentsUtil, ctl.getId(), rw.getObjectReader(), noteMap, false);
+        noteUtil, ctl.getId(), rw.getObjectReader(), noteMap, false);
   }
 
   private void checkComments(Map<RevId, RevisionNote> existingNotes,

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

@@ -1,451 +0,0 @@
-// Copyright (C) 2014 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.server.notedb;
-
-import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.gerrit.server.notedb.ChangeNotes.parseException;
-import static java.nio.charset.StandardCharsets.UTF_8;
-
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Lists;
-import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.CommentRange;
-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.RevId;
-import com.google.gerrit.server.GerritPersonIdent;
-import com.google.gerrit.server.PatchLineCommentsUtil;
-import com.google.gerrit.server.account.AccountCache;
-import com.google.gerrit.server.config.AnonymousCowardName;
-import com.google.inject.Inject;
-import com.google.inject.Singleton;
-
-import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.PersonIdent;
-import org.eclipse.jgit.util.GitDateFormatter;
-import org.eclipse.jgit.util.GitDateFormatter.Format;
-import org.eclipse.jgit.util.GitDateParser;
-import org.eclipse.jgit.util.MutableInteger;
-import org.eclipse.jgit.util.QuotedString;
-import org.eclipse.jgit.util.RawParseUtils;
-
-import java.io.ByteArrayOutputStream;
-import java.io.OutputStream;
-import java.io.OutputStreamWriter;
-import java.io.PrintWriter;
-import java.sql.Timestamp;
-import java.text.ParseException;
-import java.util.List;
-import java.util.Locale;
-
-/**
- * Utility functions to parse PatchLineComments out of a note byte array and
- * store a list of PatchLineComments in the form of a note (in a byte array).
- **/
-@Singleton
-public class CommentsInNotesUtil {
-  private static final String AUTHOR = "Author";
-  private static final String BASE_PATCH_SET = "Base-for-patch-set";
-  private static final String COMMENT_RANGE = "Comment-range";
-  private static final String FILE = "File";
-  private static final String LENGTH = "Bytes";
-  private static final String PARENT = "Parent";
-  private static final String PATCH_SET = "Patch-set";
-  private static final String REVISION = "Revision";
-  private static final String UUID = "UUID";
-
-  public static String formatTime(PersonIdent ident, Timestamp t) {
-    GitDateFormatter dateFormatter = new GitDateFormatter(Format.DEFAULT);
-    // TODO(dborowitz): Use a ThreadLocal or use Joda.
-    PersonIdent newIdent = new PersonIdent(ident, t);
-    return dateFormatter.formatDate(newIdent);
-  }
-
-  private final AccountCache accountCache;
-  private final PersonIdent serverIdent;
-  private final String anonymousCowardName;
-  private final ChangeNoteUtil changeNoteUtil;
-
-  @Inject
-  public CommentsInNotesUtil(AccountCache accountCache,
-      @GerritPersonIdent PersonIdent serverIdent,
-      @AnonymousCowardName String anonymousCowardName,
-      ChangeNoteUtil changeNoteUtil) {
-    this.accountCache = accountCache;
-    this.serverIdent = serverIdent;
-    this.anonymousCowardName = anonymousCowardName;
-    this.changeNoteUtil = changeNoteUtil;
-  }
-
-  public List<PatchLineComment> parseNote(byte[] note, MutableInteger p,
-      Change.Id changeId, Status status) throws ConfigInvalidException {
-    if (p.value >= note.length) {
-      return ImmutableList.of();
-    }
-    List<PatchLineComment> result = Lists.newArrayList();
-    int sizeOfNote = note.length;
-
-    boolean isForBase =
-        (RawParseUtils.match(note, p.value, PATCH_SET.getBytes(UTF_8))) < 0;
-
-    PatchSet.Id psId = parsePsId(note, p, changeId, isForBase ? BASE_PATCH_SET : PATCH_SET);
-
-    RevId revId =
-        new RevId(parseStringField(note, p, changeId, REVISION));
-
-    PatchLineComment c = null;
-    while (p.value < sizeOfNote) {
-      String previousFileName = c == null ?
-          null : c.getKey().getParentKey().getFileName();
-      c = parseComment(note, p, previousFileName, psId, revId,
-          isForBase, status);
-      result.add(c);
-    }
-    return result;
-  }
-
-  private PatchLineComment parseComment(byte[] note, MutableInteger curr,
-      String currentFileName, PatchSet.Id psId, RevId revId, boolean isForBase,
-      Status status) throws ConfigInvalidException {
-    Change.Id changeId = psId.getParentKey();
-
-    // Check if there is a new file.
-    boolean newFile =
-        (RawParseUtils.match(note, curr.value, FILE.getBytes(UTF_8))) != -1;
-    if (newFile) {
-      // If so, parse the new file name.
-      currentFileName = parseFilename(note, curr, changeId);
-    } else if (currentFileName == null) {
-      throw parseException(changeId, "could not parse %s", FILE);
-    }
-
-    CommentRange range = parseCommentRange(note, curr);
-    if (range == null) {
-      throw parseException(changeId, "could not parse %s", COMMENT_RANGE);
-    }
-
-    Timestamp commentTime = parseTimestamp(note, curr, changeId);
-    Account.Id aId = parseAuthor(note, curr, changeId);
-
-    boolean hasParent =
-        (RawParseUtils.match(note, curr.value, PARENT.getBytes(UTF_8))) != -1;
-    String parentUUID = null;
-    if (hasParent) {
-      parentUUID = parseStringField(note, curr, changeId, PARENT);
-    }
-
-    String uuid = parseStringField(note, curr, changeId, UUID);
-    int commentLength = parseCommentLength(note, curr, changeId);
-
-    String message = RawParseUtils.decode(
-        UTF_8, note, curr.value, curr.value + commentLength);
-    checkResult(message, "message contents", changeId);
-
-    PatchLineComment plc = new PatchLineComment(
-        new PatchLineComment.Key(new Patch.Key(psId, currentFileName), uuid),
-        range.getEndLine(), aId, parentUUID, commentTime);
-    plc.setMessage(message);
-    plc.setSide((short) (isForBase ? 0 : 1));
-    if (range.getStartCharacter() != -1) {
-      plc.setRange(range);
-    }
-    plc.setRevId(revId);
-    plc.setStatus(status);
-
-    curr.value = RawParseUtils.nextLF(note, curr.value + commentLength);
-    curr.value = RawParseUtils.nextLF(note, curr.value);
-    return plc;
-  }
-
-  private static String parseStringField(byte[] note, MutableInteger curr,
-      Change.Id changeId, String fieldName) throws ConfigInvalidException {
-    int endOfLine = RawParseUtils.nextLF(note, curr.value);
-    checkHeaderLineFormat(note, curr, fieldName, changeId);
-    int startOfField = RawParseUtils.endOfFooterLineKey(note, curr.value) + 2;
-    curr.value = endOfLine;
-    return RawParseUtils.decode(UTF_8, note, startOfField, endOfLine - 1);
-  }
-
-  /**
-   * @return a comment range. If the comment range line in the note only has
-   *    one number, we return a CommentRange with that one number as the end
-   *    line and the other fields as -1. If the comment range line in the note
-   *    contains a whole comment range, then we return a CommentRange with all
-   *    fields set. If the line is not correctly formatted, return null.
-   */
-  private static CommentRange parseCommentRange(byte[] note, MutableInteger ptr) {
-    CommentRange range = new CommentRange(-1, -1, -1, -1);
-
-    int startLine = RawParseUtils.parseBase10(note, ptr.value, ptr);
-    if (startLine == 0) {
-      range.setEndLine(0);
-      ptr.value += 1;
-      return range;
-    }
-
-    if (note[ptr.value] == '\n') {
-      range.setEndLine(startLine);
-      ptr.value += 1;
-      return range;
-    } else if (note[ptr.value] == ':') {
-      range.setStartLine(startLine);
-      ptr.value += 1;
-    } else {
-      return null;
-    }
-
-    int startChar = RawParseUtils.parseBase10(note, ptr.value, ptr);
-    if (note[ptr.value] == '-') {
-      range.setStartCharacter(startChar);
-      ptr.value += 1;
-    } else {
-      return null;
-    }
-
-    int endLine = RawParseUtils.parseBase10(note, ptr.value, ptr);
-    if (endLine == 0) {
-      return null;
-    }
-    if (note[ptr.value] == ':') {
-      range.setEndLine(endLine);
-      ptr.value += 1;
-    } else {
-      return null;
-    }
-
-    int endChar = RawParseUtils.parseBase10(note, ptr.value, ptr);
-    if (endChar == 0) {
-      return null;
-    }
-    if (note[ptr.value] == '\n') {
-      range.setEndCharacter(endChar);
-      ptr.value += 1;
-    } else {
-      return null;
-    }
-    return range;
-  }
-
-  private static PatchSet.Id parsePsId(byte[] note, MutableInteger curr,
-      Change.Id changeId, String fieldName) throws ConfigInvalidException {
-    checkHeaderLineFormat(note, curr, fieldName, changeId);
-    int startOfPsId =
-        RawParseUtils.endOfFooterLineKey(note, curr.value) + 1;
-    MutableInteger i = new MutableInteger();
-    int patchSetId =
-        RawParseUtils.parseBase10(note, startOfPsId, i);
-    int endOfLine = RawParseUtils.nextLF(note, curr.value);
-    if (i.value != endOfLine - 1) {
-      throw parseException(changeId, "could not parse %s", fieldName);
-    }
-    checkResult(patchSetId, "patchset id", changeId);
-    curr.value = endOfLine;
-    return new PatchSet.Id(changeId, patchSetId);
-  }
-
-  private static String parseFilename(byte[] note, MutableInteger curr,
-      Change.Id changeId) throws ConfigInvalidException {
-    checkHeaderLineFormat(note, curr, FILE, changeId);
-    int startOfFileName =
-        RawParseUtils.endOfFooterLineKey(note, curr.value) + 2;
-    int endOfLine = RawParseUtils.nextLF(note, curr.value);
-    curr.value = endOfLine;
-    curr.value = RawParseUtils.nextLF(note, curr.value);
-    return QuotedString.GIT_PATH.dequote(
-        RawParseUtils.decode(UTF_8, note, startOfFileName, endOfLine - 1));
-  }
-
-  private static Timestamp parseTimestamp(byte[] note, MutableInteger curr,
-      Change.Id changeId) throws ConfigInvalidException {
-    int endOfLine = RawParseUtils.nextLF(note, curr.value);
-    Timestamp commentTime;
-    String dateString =
-        RawParseUtils.decode(UTF_8, note, curr.value, endOfLine - 1);
-    try {
-      commentTime = new Timestamp(
-          GitDateParser.parse(dateString, null, Locale.US).getTime());
-    } catch (ParseException e) {
-      throw new ConfigInvalidException("could not parse comment timestamp", e);
-    }
-    curr.value = endOfLine;
-    return checkResult(commentTime, "comment timestamp", changeId);
-  }
-
-  private Account.Id parseAuthor(byte[] note, MutableInteger curr,
-      Change.Id changeId) throws ConfigInvalidException {
-    checkHeaderLineFormat(note, curr, AUTHOR, changeId);
-    int startOfAccountId =
-        RawParseUtils.endOfFooterLineKey(note, curr.value) + 2;
-    PersonIdent ident =
-        RawParseUtils.parsePersonIdent(note, startOfAccountId);
-    Account.Id aId = changeNoteUtil.parseIdent(ident, changeId);
-    curr.value = RawParseUtils.nextLF(note, curr.value);
-    return checkResult(aId, "comment author", changeId);
-  }
-
-  private static int parseCommentLength(byte[] note, MutableInteger curr,
-      Change.Id changeId) throws ConfigInvalidException {
-    checkHeaderLineFormat(note, curr, LENGTH, changeId);
-    int startOfLength =
-        RawParseUtils.endOfFooterLineKey(note, curr.value) + 1;
-    MutableInteger i = new MutableInteger();
-    int commentLength =
-        RawParseUtils.parseBase10(note, startOfLength, i);
-    int endOfLine = RawParseUtils.nextLF(note, curr.value);
-    if (i.value != endOfLine-1) {
-      throw parseException(changeId, "could not parse %s", PATCH_SET);
-    }
-    curr.value = endOfLine;
-    return checkResult(commentLength, "comment length", changeId);
-  }
-
-  private static <T> T checkResult(T o, String fieldName,
-      Change.Id changeId) throws ConfigInvalidException {
-    if (o == null) {
-      throw parseException(changeId, "could not parse %s", fieldName);
-    }
-    return o;
-  }
-
-  private static int checkResult(int i, String fieldName, Change.Id changeId)
-      throws ConfigInvalidException {
-    if (i <= 0) {
-      throw parseException(changeId, "could not parse %s", fieldName);
-    }
-    return i;
-  }
-
-  private void appendHeaderField(PrintWriter writer,
-      String field, String value) {
-    writer.print(field);
-    writer.print(": ");
-    writer.print(value);
-    writer.print('\n');
-  }
-
-  private static void checkHeaderLineFormat(byte[] note, MutableInteger curr,
-      String fieldName, Change.Id changeId) throws ConfigInvalidException {
-    boolean correct =
-        RawParseUtils.match(note, curr.value, fieldName.getBytes(UTF_8)) != -1;
-    int p = curr.value + fieldName.length();
-    correct &= (p < note.length && note[p] == ':');
-    p++;
-    correct &= (p < note.length && note[p] == ' ');
-    if (!correct) {
-      throw parseException(changeId, "could not parse %s", fieldName);
-    }
-  }
-
-  public byte[] buildNote(List<PatchLineComment> comments) {
-    ByteArrayOutputStream out = new ByteArrayOutputStream();
-    buildNote(comments, out);
-    return out.toByteArray();
-  }
-
-  /**
-   * Build a note that contains the metadata for and the contents of all of the
-   * comments in the given list of comments.
-   *
-   * @param comments A list of the comments to be written to the
-   *            output stream. All of the comments in this list must have the
-   *            same side and must share the same patch set ID.
-   * @param out output stream to write to.
-   */
-  void buildNote(List<PatchLineComment> comments, OutputStream out) {
-    if (comments.isEmpty()) {
-      return;
-    }
-    OutputStreamWriter streamWriter = new OutputStreamWriter(out, UTF_8);
-    try (PrintWriter writer = new PrintWriter(streamWriter)) {
-      PatchLineComment first = comments.get(0);
-
-      short side = first.getSide();
-      PatchSet.Id psId = PatchLineCommentsUtil.getCommentPsId(first);
-      appendHeaderField(writer, side == 0
-          ? BASE_PATCH_SET
-          : PATCH_SET,
-          Integer.toString(psId.get()));
-      appendHeaderField(writer, REVISION, first.getRevId().get());
-
-      String currentFilename = null;
-
-      for (PatchLineComment c : comments) {
-        PatchSet.Id currentPsId = PatchLineCommentsUtil.getCommentPsId(c);
-        checkArgument(psId.equals(currentPsId),
-            "All comments being added must all have the same PatchSet.Id. The"
-            + "comment below does not have the same PatchSet.Id as the others "
-            + "(%s).\n%s", psId.toString(), c.toString());
-        checkArgument(side == c.getSide(),
-            "All comments being added must all have the same side. The"
-            + "comment below does not have the same side as the others "
-            + "(%s).\n%s", side, c.toString());
-        String commentFilename =
-            QuotedString.GIT_PATH.quote(c.getKey().getParentKey().getFileName());
-
-        if (!commentFilename.equals(currentFilename)) {
-          currentFilename = commentFilename;
-          writer.print("File: ");
-          writer.print(commentFilename);
-          writer.print("\n\n");
-        }
-
-        // The CommentRange field for a comment is allowed to be null.
-        // If it is indeed null, then in the first line, we simply use the line
-        // number field for a comment instead. If it isn't null, we write the
-        // comment range itself.
-        CommentRange range = c.getRange();
-        if (range != null) {
-          writer.print(range.getStartLine());
-          writer.print(':');
-          writer.print(range.getStartCharacter());
-          writer.print('-');
-          writer.print(range.getEndLine());
-          writer.print(':');
-          writer.print(range.getEndCharacter());
-        } else {
-          writer.print(c.getLine());
-        }
-        writer.print("\n");
-
-        writer.print(formatTime(serverIdent, c.getWrittenOn()));
-        writer.print("\n");
-
-        PersonIdent ident =
-            changeNoteUtil.newIdent(accountCache.get(c.getAuthor()).getAccount(),
-                c.getWrittenOn(), serverIdent, anonymousCowardName);
-        String nameString = ident.getName() + " <" + ident.getEmailAddress()
-            + ">";
-        appendHeaderField(writer, AUTHOR, nameString);
-
-        String parent = c.getParentUuid();
-        if (parent != null) {
-          appendHeaderField(writer, PARENT, parent);
-        }
-
-        appendHeaderField(writer, UUID, c.getKey().get());
-
-        byte[] messageBytes = c.getMessage().getBytes(UTF_8);
-        appendHeaderField(writer, LENGTH,
-            Integer.toString(messageBytes.length));
-
-        writer.print(c.getMessage());
-        writer.print("\n\n");
-      }
-    }
-  }
-}

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

@@ -48,40 +48,40 @@ public class DraftCommentNotes extends AbstractChangeNotes<DraftCommentNotes> {
     private final GitRepositoryManager repoManager;
     private final NotesMigration migration;
     private final AllUsersName draftsProject;
-    private final CommentsInNotesUtil commentsUtil;
+    private final ChangeNoteUtil noteUtil;
 
     @VisibleForTesting
     @Inject
     public Factory(GitRepositoryManager repoManager,
         NotesMigration migration,
         AllUsersName allUsers,
-        CommentsInNotesUtil commentsUtil) {
+        ChangeNoteUtil noteUtil) {
       this.repoManager = repoManager;
       this.migration = migration;
       this.draftsProject = allUsers;
-      this.commentsUtil = commentsUtil;
+      this.noteUtil = noteUtil;
     }
 
     public DraftCommentNotes create(Change.Id changeId, Account.Id accountId) {
       return new DraftCommentNotes(repoManager, migration, draftsProject,
-          commentsUtil, changeId, accountId);
+          noteUtil, changeId, accountId);
     }
   }
 
   private final AllUsersName draftsProject;
-  private final CommentsInNotesUtil commentsUtil;
+  private final ChangeNoteUtil noteUtil;
   private final Account.Id author;
 
   private ImmutableListMultimap<RevId, PatchLineComment> comments;
   private RevisionNoteMap revisionNoteMap;
 
   DraftCommentNotes(GitRepositoryManager repoManager, NotesMigration migration,
-      AllUsersName draftsProject, CommentsInNotesUtil commentsUtil,
+      AllUsersName draftsProject, ChangeNoteUtil noteUtil, Change.Id changeId,
-      Change.Id changeId, Account.Id author) {
+      Account.Id author) {
     super(repoManager, migration, changeId);
     this.draftsProject = draftsProject;
     this.author = author;
-    this.commentsUtil = commentsUtil;
+    this.noteUtil = noteUtil;
   }
 
   RevisionNoteMap getRevisionNoteMap() {
@@ -122,7 +122,7 @@ public class DraftCommentNotes extends AbstractChangeNotes<DraftCommentNotes> {
     try (RevWalk walk = new RevWalk(reader)) {
       RevCommit tipCommit = walk.parseCommit(rev);
       revisionNoteMap = RevisionNoteMap.parse(
-          commentsUtil, getChangeId(), reader, NoteMap.read(reader, tipCommit),
+          noteUtil, getChangeId(), reader, NoteMap.read(reader, tipCommit),
           true);
       Multimap<RevId, PatchLineComment> cs = ArrayListMultimap.create();
       for (RevisionNote rn : revisionNoteMap.revisionNotes.values()) {

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

@@ -63,7 +63,7 @@ class RevisionNote {
   final ImmutableList<PatchLineComment> comments;
   final String pushCert;
 
-  RevisionNote(CommentsInNotesUtil commentsUtil, Change.Id changeId,
+  RevisionNote(ChangeNoteUtil noteUtil, Change.Id changeId,
       ObjectReader reader, ObjectId noteId, boolean draftsOnly)
       throws ConfigInvalidException, IOException {
     byte[] bytes = reader.open(noteId, OBJ_BLOB).getCachedBytes(MAX_NOTE_SZ);
@@ -79,6 +79,6 @@ class RevisionNote {
         ? PatchLineComment.Status.DRAFT
         : PatchLineComment.Status.PUBLISHED;
     comments = ImmutableList.copyOf(
-        commentsUtil.parseNote(bytes, p, changeId, status));
+        noteUtil.parseNote(bytes, p, changeId, status));
   }
 }

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

@@ -91,7 +91,7 @@ class RevisionNoteBuilder {
     this.pushCert = pushCert;
   }
 
-  byte[] build(CommentsInNotesUtil commentsUtil) {
+  byte[] build(ChangeNoteUtil noteUtil) {
     ByteArrayOutputStream out = new ByteArrayOutputStream();
     if (pushCert != null) {
       byte[] certBytes = pushCert.getBytes(UTF_8);
@@ -107,7 +107,7 @@ class RevisionNoteBuilder {
       }
     }
     Collections.sort(all, PLC_ORDER);
-    commentsUtil.buildNote(all, out);
+    noteUtil.buildNote(all, out);
     return out.toByteArray();
   }
 

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

@@ -31,13 +31,13 @@ class RevisionNoteMap {
   final NoteMap noteMap;
   final ImmutableMap<RevId, RevisionNote> revisionNotes;
 
-  static RevisionNoteMap parse(CommentsInNotesUtil commentsUtil,
+  static RevisionNoteMap parse(ChangeNoteUtil noteUtil,
       Change.Id changeId, ObjectReader reader, NoteMap noteMap,
       boolean draftsOnly) throws ConfigInvalidException, IOException {
     Map<RevId, RevisionNote> result = new HashMap<>();
     for (Note note : noteMap) {
       RevisionNote rn = new RevisionNote(
-          commentsUtil, changeId, reader, note.getData(), draftsOnly);
+          noteUtil, changeId, reader, note.getData(), draftsOnly);
       result.put(new RevId(note.name()), rn);
     }
     return new RevisionNoteMap(noteMap, ImmutableMap.copyOf(result));

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

@@ -108,10 +108,7 @@ public abstract class AbstractChangeNotesTest extends GerritBaseTests {
   protected AllUsersName allUsers;
 
   @Inject
-  protected ChangeNoteUtil changeNoteUtil;
+  protected ChangeNoteUtil noteUtil;
-
-  @Inject
-  protected CommentsInNotesUtil commentsUtil;
 
   private Injector injector;
   private String systemTimeZone;
@@ -207,8 +204,8 @@ public abstract class AbstractChangeNotesTest extends GerritBaseTests {
   }
 
   protected ChangeNotes newNotes(Change c) throws OrmException {
-    return new ChangeNotes(repoManager, MIGRATION, allUsers, changeNoteUtil,
+    return new ChangeNotes(repoManager, MIGRATION, allUsers, noteUtil,
-        commentsUtil, c.getProject(), c).load();
+        c.getProject(), c).load();
   }
 
   protected static SubmitRecord submitRecord(String status,

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

@@ -415,7 +415,7 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest {
   }
 
   private RevCommit writeCommit(String body) throws Exception {
-    return writeCommit(body, changeNoteUtil.newIdent(
+    return writeCommit(body, noteUtil.newIdent(
         changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent,
         "Anonymous Coward"));
   }
@@ -462,6 +462,6 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest {
   private ChangeNotesParser newParser(ObjectId tip) throws Exception {
     Change c = newChange();
     return new ChangeNotesParser(c.getProject(), c.getId(), tip, walk,
-        repoManager, changeNoteUtil, commentsUtil);
+        repoManager, noteUtil);
   }
 }

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

@@ -825,7 +825,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
         + "File: a.txt\n"
         + "\n"
         + "1:2-3:4\n"
-        + CommentsInNotesUtil.formatTime(serverIdent, ts) + "\n"
+        + ChangeNoteUtil.formatTime(serverIdent, ts) + "\n"
         + "Author: Change Owner <1@gerrit>\n"
         + "UUID: uuid1\n"
         + "Bytes: 7\n"
@@ -910,8 +910,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
 
     try (RevWalk rw = new RevWalk(repo)) {
       try (ChangeNotesParser notesWithComments = new ChangeNotesParser(project,
-          c.getId(), commitWithComments.copy(), rw, repoManager, changeNoteUtil,
+          c.getId(), commitWithComments.copy(), rw, repoManager, noteUtil)) {
-          commentsUtil)) {
         notesWithComments.parseAll();
         ImmutableListMultimap<PatchSet.Id, PatchSetApproval> approvals1 =
             notesWithComments.buildApprovals();
@@ -923,7 +922,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
     try (RevWalk rw = new RevWalk(repo)) {
       try (ChangeNotesParser notesWithApprovals = new ChangeNotesParser(project,
           c.getId(), commitWithApprovals.copy(), rw, repoManager,
-          changeNoteUtil, commentsUtil)) {
+          noteUtil)) {
         notesWithApprovals.parseAll();
         ImmutableListMultimap<PatchSet.Id, PatchSetApproval> approvals2 =
             notesWithApprovals.buildApprovals();
@@ -1168,14 +1167,14 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
           + "File: file1\n"
           + "\n"
           + "1:1-2:1\n"
-          + CommentsInNotesUtil.formatTime(serverIdent, time1) + "\n"
+          + ChangeNoteUtil.formatTime(serverIdent, time1) + "\n"
           + "Author: Other Account <2@gerrit>\n"
           + "UUID: uuid1\n"
           + "Bytes: 9\n"
           + "comment 1\n"
           + "\n"
           + "2:1-3:1\n"
-          + CommentsInNotesUtil.formatTime(serverIdent, time2) + "\n"
+          + ChangeNoteUtil.formatTime(serverIdent, time2) + "\n"
           + "Author: Other Account <2@gerrit>\n"
           + "UUID: uuid2\n"
           + "Bytes: 9\n"
@@ -1184,7 +1183,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
           + "File: file2\n"
           + "\n"
           + "3:0-4:1\n"
-          + CommentsInNotesUtil.formatTime(serverIdent, time3) + "\n"
+          + ChangeNoteUtil.formatTime(serverIdent, time3) + "\n"
           + "Author: Other Account <2@gerrit>\n"
           + "UUID: uuid3\n"
           + "Bytes: 9\n"
@@ -1238,14 +1237,14 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
           + "File: file1\n"
           + "\n"
           + "1:1-2:1\n"
-          + CommentsInNotesUtil.formatTime(serverIdent, time1) + "\n"
+          + ChangeNoteUtil.formatTime(serverIdent, time1) + "\n"
           + "Author: Other Account <2@gerrit>\n"
           + "UUID: uuid1\n"
           + "Bytes: 9\n"
           + "comment 1\n"
           + "\n"
           + "2:1-3:1\n"
-          + CommentsInNotesUtil.formatTime(serverIdent, time2) + "\n"
+          + ChangeNoteUtil.formatTime(serverIdent, time2) + "\n"
           + "Author: Other Account <2@gerrit>\n"
           + "UUID: uuid2\n"
           + "Bytes: 9\n"

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

@@ -34,7 +34,6 @@ import com.google.gerrit.server.notedb.ChangeDraftUpdate;
 import com.google.gerrit.server.notedb.ChangeNoteUtil;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.notedb.ChangeUpdate;
-import com.google.gerrit.server.notedb.CommentsInNotesUtil;
 import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gwtorm.server.OrmException;
@@ -105,7 +104,6 @@ public class TestChanges {
             stubChangeControl(
                 repoManager, migration, c, allUsers,
                 injector.getInstance(ChangeNoteUtil.class),
-                injector.getInstance(CommentsInNotesUtil.class),
                 user),
             TimeUtil.nowTs(), Ordering.<String> natural());
 
@@ -139,15 +137,15 @@ public class TestChanges {
 
   private static ChangeControl stubChangeControl(
       GitRepositoryManager repoManager, NotesMigration migration,
-      Change c, AllUsersName allUsers, ChangeNoteUtil changeNoteUtil,
+      Change c, AllUsersName allUsers, ChangeNoteUtil noteUtil,
-      CommentsInNotesUtil commentsUtil, CurrentUser user) throws OrmException {
+      CurrentUser user) throws OrmException {
     ChangeControl ctl = EasyMock.createMock(ChangeControl.class);
     expect(ctl.getChange()).andStubReturn(c);
     expect(ctl.getProject()).andStubReturn(new Project(c.getProject()));
     expect(ctl.getUser()).andStubReturn(user);
     ChangeNotes notes =
-        new ChangeNotes(repoManager, migration, allUsers, changeNoteUtil,
+        new ChangeNotes(repoManager, migration, allUsers, noteUtil,
-            commentsUtil, c.getProject(), c).load();
+            c.getProject(), c).load();
     expect(ctl.getNotes()).andStubReturn(notes);
     expect(ctl.getId()).andStubReturn(c.getId());
     EasyMock.replay(ctl);