PatchLineComment: Replace RevId with ObjectId

This change does not affect Comment's field names or types, since that
type is used in the NoteDb JSON storage format, so it needs to remain
backwards compatible. It's possible we could change this and then use a
custom type adapter in Gson, but that may be more effort than it's
worth.

See Iff5644e2 context on removing RevId usages.

Change-Id: If054aca4867f5868e108616e7524b15f5c52de55

java/com/google/gerrit/reviewdb/client/Comment.java

@@ -16,9 +16,11 @@ package com.google.gerrit.reviewdb.client;
 
 import com.google.common.base.MoreObjects;
 import com.google.common.base.MoreObjects.ToStringHelper;
+import com.google.gerrit.common.Nullable;
 import java.sql.Timestamp;
 import java.util.Comparator;
 import java.util.Objects;
+import org.eclipse.jgit.lib.AnyObjectId;
 
 /**
  * This class represents inline comments in NoteDb. This means it determines the JSON format for
@@ -192,7 +194,9 @@ public class Comment {
   public Range range;
   public String tag;
 
-  // Hex commit SHA1 of the commit of the patchset to which this comment applies.
+  // Hex commit SHA1 of the commit of the patchset to which this comment applies. Other classes call
+  // this "commitId", but this class uses the old ReviewDb term "revId", and this field name is
+  // serialized into JSON in NoteDb, so it can't easily be changed.
   public String revId;
   public String serverId;
   public boolean unresolved;
@@ -250,8 +254,8 @@ public class Comment {
     this.range = range != null ? range.asCommentRange() : null;
   }
 
-  public void setRevId(RevId revId) {
-    this.revId = revId != null ? revId.get() : null;
+  public void setRevId(@Nullable AnyObjectId revId) {
+    this.revId = revId != null ? revId.name() : null;
   }
 
   public void setRealAuthor(Account.Id id) {

java/com/google/gerrit/reviewdb/client/PatchLineComment.java

@@ -18,6 +18,8 @@ import com.google.gerrit.common.Nullable;
 import com.google.gerrit.extensions.client.Comment.Range;
 import java.sql.Timestamp;
 import java.util.Objects;
+import org.eclipse.jgit.lib.AnyObjectId;
+import org.eclipse.jgit.lib.ObjectId;
 
 /**
  * A comment left by a user on a specific line of a {@link Patch}.
@@ -68,7 +70,7 @@ public final class PatchLineComment {
       plc.setRange(new CommentRange(r.startLine, r.startChar, r.endLine, r.endChar));
     }
     plc.setTag(c.tag);
-    plc.setRevId(new RevId(c.revId));
+    plc.setCommitId(ObjectId.fromString(c.revId));
     plc.setStatus(status);
     plc.setRealAuthor(c.getRealAuthor().getId());
     plc.setUnresolved(c.unresolved);
@@ -110,8 +112,8 @@ public final class PatchLineComment {
   /** True if this comment requires further action. */
   protected boolean unresolved;
 
-  /** The RevId for the commit to which this comment is referring. */
-  protected RevId revId;
+  /** The ID of the commit to which this comment is referring. */
+  protected ObjectId commitId;
 
   protected PatchLineComment() {}
 
@@ -137,7 +139,7 @@ public final class PatchLineComment {
     side = o.side;
     message = o.message;
     parentUuid = o.parentUuid;
-    revId = o.revId;
+    commitId = o.commitId;
     if (o.range != null) {
       range =
           new CommentRange(
@@ -232,12 +234,12 @@ public final class PatchLineComment {
     return range;
   }
 
-  public void setRevId(RevId rev) {
-    revId = rev;
+  public void setCommitId(AnyObjectId commitId) {
+    this.commitId = commitId.copy();
   }
 
-  public RevId getRevId() {
-    return revId;
+  public ObjectId getCommitId() {
+    return commitId;
   }
 
   public void setTag(String tag) {
@@ -266,7 +268,7 @@ public final class PatchLineComment {
             message,
             serverId,
             unresolved);
-    c.setRevId(revId);
+    c.setRevId(commitId);
     c.setRange(range);
     c.lineNbr = lineNbr;
     c.parentUuid = parentUuid;
@@ -289,7 +291,7 @@ public final class PatchLineComment {
           && Objects.equals(message, c.getMessage())
           && Objects.equals(parentUuid, c.getParentUuid())
           && Objects.equals(range, c.getRange())
-          && Objects.equals(revId, c.getRevId())
+          && Objects.equals(commitId, c.getCommitId())
           && Objects.equals(tag, c.getTag())
           && Objects.equals(unresolved, c.getUnresolved());
     }
@@ -316,7 +318,7 @@ public final class PatchLineComment {
     builder.append("message=").append(Objects.toString(message, "")).append(',');
     builder.append("parentUuid=").append(Objects.toString(parentUuid, "")).append(',');
     builder.append("range=").append(Objects.toString(range, "")).append(',');
-    builder.append("revId=").append(revId != null ? revId.get() : "").append(',');
+    builder.append("revId=").append(commitId != null ? commitId.name() : "").append(',');
     builder.append("tag=").append(Objects.toString(tag, "")).append(',');
     builder.append("unresolved=").append(unresolved);
     builder.append('}');

java/com/google/gerrit/server/notedb/LegacyChangeNoteRead.java

@@ -23,7 +23,6 @@ import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Comment;
 import com.google.gerrit.reviewdb.client.CommentRange;
 import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.RevId;
 import com.google.gerrit.server.config.GerritServerId;
 import com.google.inject.Inject;
 import java.sql.Timestamp;
@@ -34,6 +33,7 @@ import java.util.List;
 import java.util.Locale;
 import java.util.Set;
 import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.util.GitDateParser;
 import org.eclipse.jgit.util.MutableInteger;
@@ -77,7 +77,8 @@ public class LegacyChangeNoteRead {
     byte[] bpsb = ChangeNoteUtil.BASE_PATCH_SET.getBytes(UTF_8);
     byte[] bpn = ChangeNoteUtil.PARENT_NUMBER.getBytes(UTF_8);
 
-    RevId revId = new RevId(parseStringField(note, p, changeId, ChangeNoteUtil.REVISION));
+    ObjectId commitId =
+        ObjectId.fromString(parseStringField(note, p, changeId, ChangeNoteUtil.REVISION));
     String fileName = null;
     PatchSet.Id psId = null;
     boolean isForBase = false;
@@ -105,7 +106,7 @@ public class LegacyChangeNoteRead {
             ChangeNoteUtil.BASE_PATCH_SET);
       }
 
-      Comment c = parseComment(note, p, fileName, psId, revId, isForBase, parentNumber);
+      Comment c = parseComment(note, p, fileName, psId, commitId, isForBase, parentNumber);
       fileName = c.key.filename;
       if (!seen.add(c.key)) {
         throw parseException(changeId, "multiple comments for %s in note", c.key);
@@ -120,7 +121,7 @@ public class LegacyChangeNoteRead {
       MutableInteger curr,
       String currentFileName,
       PatchSet.Id psId,
-      RevId revId,
+      ObjectId commitId,
       boolean isForBase,
       Integer parentNumber)
       throws ConfigInvalidException {
@@ -189,7 +190,7 @@ public class LegacyChangeNoteRead {
     c.lineNbr = range.getEndLine();
     c.parentUuid = parentUUID;
     c.tag = tag;
-    c.setRevId(revId);
+    c.setRevId(commitId);
     if (raId != null) {
       c.setRealAuthor(raId);
     }

javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java

@@ -663,7 +663,7 @@ public class ChangeNotesStateTest extends GerritBaseTests {
             "message 1",
             "serverId",
             false);
-    c1.setRevId(new RevId("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"));
+    c1.setRevId(ObjectId.fromString("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"));
     String c1Json = Serializer.GSON.toJson(c1);
 
     Comment c2 =
@@ -675,7 +675,7 @@ public class ChangeNotesStateTest extends GerritBaseTests {
             "message 2",
             "serverId",
             true);
-    c2.setRevId(new RevId("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"));
+    c2.setRevId(ObjectId.fromString("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"));
     String c2Json = Serializer.GSON.toJson(c2);
 
     assertRoundTrip(