AbstractChangeNotes: Don't extend VersionedMetaData

Similar to ChangeUpdate in Ib163d209, most of the VersionedMetaData
methods weren't actually being used, and using them accidentally would
lead to unexpected results.

Copy the few methods we do need into AbstractChangeNotes. This is not
a complete rethinking of the AbstractChangeNotes API, just a minimal
code change. But it does open up that possibility later if we want it.

Change-Id: I1e56a80cc8daa90fa7ac0dc985d31f22123c0a55

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

@@ -17,22 +17,23 @@ package com.google.gerrit.server.notedb;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.VersionedMetaData;
 import com.google.gwtorm.server.OrmException;
 
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevWalk;
 
 import java.io.IOException;
 
 /** View of contents at a single ref related to some change. **/
-public abstract class AbstractChangeNotes<T> extends VersionedMetaData {
+public abstract class AbstractChangeNotes<T> {
   protected final GitRepositoryManager repoManager;
   protected final NotesMigration migration;
   private final Change.Id changeId;
 
+  private ObjectId revision;
   private boolean loaded;
 
   AbstractChangeNotes(GitRepositoryManager repoManager,
@@ -46,6 +47,11 @@ public abstract class AbstractChangeNotes<T> extends VersionedMetaData {
     return changeId;
   }
 
+  /** @return revision of the metadata that was loaded. */
+  public ObjectId getRevision() {
+    return revision;
+  }
+
   public T load() throws OrmException {
     if (loaded) {
       return self();
@@ -54,8 +60,12 @@ public abstract class AbstractChangeNotes<T> extends VersionedMetaData {
       loadDefaults();
       return self();
     }
-    try (Repository repo = repoManager.openMetadataRepository(getProjectName())) {
-      load(repo);
+    try (Repository repo = repoManager.openMetadataRepository(getProjectName());
+        RevWalk walk = new RevWalk(repo)) {
+      Ref ref = repo.getRefDatabase().exactRef(getRefName());
+      ObjectId id = ref != null ? ref.getObjectId() : null;
+      revision = id != null ? walk.parseCommit(id).copy() : null;
+      onLoad(walk);
       loaded = true;
     } catch (ConfigInvalidException | IOException e) {
       throw new OrmException(e);
@@ -91,6 +101,13 @@ public abstract class AbstractChangeNotes<T> extends VersionedMetaData {
    */
   public abstract Project.NameKey getProjectName();
 
+  /** @return name of the reference storing this configuration. */
+  protected abstract String getRefName();
+
+  /** Set up the metadata, parsing any state from the loaded revision. */
+  protected abstract void onLoad(RevWalk walk)
+      throws IOException, ConfigInvalidException;
+
   @SuppressWarnings("unchecked")
   protected final T self() {
     return (T) this;

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

@@ -66,7 +66,6 @@ import com.google.inject.Provider;
 import com.google.inject.Singleton;
 
 import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.CommitBuilder;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
@@ -543,14 +542,14 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
   }
 
   @Override
-  protected void onLoad() throws IOException, ConfigInvalidException {
+  protected void onLoad(RevWalk walk)
+      throws IOException, ConfigInvalidException {
     ObjectId rev = getRevision();
     if (rev == null) {
       loadDefaults();
       return;
     }
-    try (RevWalk walk = new RevWalk(reader);
-        ChangeNotesParser parser = new ChangeNotesParser(
+    try (ChangeNotesParser parser = new ChangeNotesParser(
          project, change.getId(), rev, walk, repoManager, noteUtil)) {
       parser.parseAll();
 
@@ -610,12 +609,6 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
     hashtags = ImmutableSet.of();
   }
 
-  @Override
-  protected boolean onSave(CommitBuilder commit) {
-    throw new UnsupportedOperationException(
-        getClass().getSimpleName() + " is read-only");
-  }
-
   @Override
   public Project.NameKey getProjectName() {
     return project;

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

@@ -30,8 +30,8 @@ import com.google.inject.Inject;
 import com.google.inject.Singleton;
 
 import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.CommitBuilder;
 import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.notes.NoteMap;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
@@ -112,15 +112,16 @@ public class DraftCommentNotes extends AbstractChangeNotes<DraftCommentNotes> {
   }
 
   @Override
-  protected void onLoad() throws IOException, ConfigInvalidException {
+  protected void onLoad(RevWalk walk)
+      throws IOException, ConfigInvalidException {
     ObjectId rev = getRevision();
     if (rev == null) {
       loadDefaults();
       return;
     }
 
-    try (RevWalk walk = new RevWalk(reader)) {
     RevCommit tipCommit = walk.parseCommit(rev);
+    ObjectReader reader = walk.getObjectReader();
     revisionNoteMap = RevisionNoteMap.parse(
         noteUtil, getChangeId(), reader, NoteMap.read(reader, tipCommit),
         true);
@@ -132,14 +133,6 @@ public class DraftCommentNotes extends AbstractChangeNotes<DraftCommentNotes> {
     }
     comments = ImmutableListMultimap.copyOf(cs);
   }
-  }
-
-  @Override
-  protected boolean onSave(CommitBuilder commit) throws IOException,
-      ConfigInvalidException {
-    throw new UnsupportedOperationException(
-        getClass().getSimpleName() + " is read-only");
-  }
 
   @Override
   protected void loadDefaults() {