Remove use of RefCache in ChangeNotes

The RefCache instance stored in the ChangeNotes refs field
was passed as a nullable parameter but, effectively, always passed
as null value in all invocations form the code-base.

The refs field was also final, which means that it wasn't possible
to memoize or cache any RefCache instance on it anyway.

Remove the field and its use across the code-base, so that the
code becomes easier to read and maintain.

The cache was initially introduced in ChangeNotes with I5e02032d6
and used by the ChangeRebuilderImpl; however the implementation on
how changes are rebuilt was changed with Icc942dba25 which did not
require anymore the use of a RepoRefCache in ChangeNotes. However,
Dave forgot to remove the refs from ChangeNotes which was left unused
since then.

Release-Notes: skip
Change-Id: I729ad8738fd4ceebe72e61f086631f2d109a817a

3 files changed, 10 insertions, 19 deletions

diff --git a/java/com/google/gerrit/server/index/change/ChangeIndexer.java b/java/com/google/gerrit/server/index/change/ChangeIndexer.java
index affad63a39..82441ac4cf 100644
--- a/java/com/google/gerrit/server/index/change/ChangeIndexer.java
+++ b/java/com/google/gerrit/server/index/change/ChangeIndexer.java
@@ -528,7 +528,7 @@ public class ChangeIndexer {
   // less-contentious rebuild.
   private ChangeData newChangeData(ReviewDb db, Change change) throws OrmException {
     if (!notesMigration.readChanges()) {
-      ChangeNotes notes = changeNotesFactory.createWithAutoRebuildingDisabled(change, null);
+      ChangeNotes notes = changeNotesFactory.createWithAutoRebuildingDisabled(change);
       return changeDataFactory.create(db, notes);
     }
     return changeDataFactory.create(db, change);
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotes.java b/java/com/google/gerrit/server/notedb/ChangeNotes.java
index 3d97651969..e2d34facfe 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -188,8 +188,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
 
     public ChangeNotes createWithAutoRebuildingDisabled(
         ReviewDb db, Project.NameKey project, Change.Id changeId) throws OrmException {
-      return new ChangeNotes(args, loadChangeFromDb(db, project, changeId), true, false, null)
-          .load();
+      return new ChangeNotes(args, loadChangeFromDb(db, project, changeId), true, false).load();
     }
 
     /**
@@ -205,12 +204,11 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
 
     public ChangeNotes createForBatchUpdate(Change change, boolean shouldExist)
         throws OrmException {
-      return new ChangeNotes(args, change, shouldExist, false, null).load();
+      return new ChangeNotes(args, change, shouldExist, false).load();
     }
 
-    public ChangeNotes createWithAutoRebuildingDisabled(Change change, RefCache refs)
-        throws OrmException {
-      return new ChangeNotes(args, change, true, false, refs).load();
+    public ChangeNotes createWithAutoRebuildingDisabled(Change change) throws OrmException {
+      return new ChangeNotes(args, change, true, false).load();
     }
 
     // TODO(ekempin): Remove when database backend is deleted
@@ -478,7 +476,6 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
   }
 
   private final boolean shouldExist;
-  private final RefCache refs;
 
   private Change change;
   private ChangeNotesState state;
@@ -499,15 +496,13 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
 
   @VisibleForTesting
   public ChangeNotes(Args args, Change change) {
-    this(args, change, true, true, null);
+    this(args, change, true, true);
   }
 
-  private ChangeNotes(
-      Args args, Change change, boolean shouldExist, boolean autoRebuild, @Nullable RefCache refs) {
+  private ChangeNotes(Args args, Change change, boolean shouldExist, boolean autoRebuild) {
     super(args, change.getId(), PrimaryStorage.of(change), autoRebuild);
     this.change = new Change(change);
     this.shouldExist = shouldExist;
-    this.refs = refs;
   }
 
   public Change getChange() {
@@ -725,8 +720,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
 
   @Override
   protected ObjectId readRef(Repository repo) throws IOException {
-    Optional<RefCache> refsCache =
-        Optional.ofNullable(refs).map(Optional::of).orElse(RepoRefCache.getOptional(repo));
+    Optional<RefCache> refsCache = RepoRefCache.getOptional(repo);
     return refsCache.isPresent()
         ? refsCache.get().get(getRefName()).orElse(null)
         : super.readRef(repo);
@@ -758,10 +752,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
         // ReviewDb claims NoteDb state exists, but meta ref isn't present: fall through and
         // auto-rebuild if necessary.
       }
-      RefCache refs =
-          this.refs != null
-              ? this.refs
-              : RepoRefCache.getOptional(repo).orElse(new RepoRefCache(repo));
+      RefCache refs = RepoRefCache.getOptional(repo).orElse(new RepoRefCache(repo));
       if (!NoteDbChangeState.isChangeUpToDate(state, refs, getChangeId())) {
         return rebuildAndOpen(repo, id);
       }
diff --git a/javatests/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java b/javatests/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java
index 5ebfa9e926..2718793ac2 100644
--- a/javatests/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java
@@ -926,7 +926,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
     oldDb.changes().update(Collections.singleton(c));
 
     c = oldDb.changes().get(c.getId());
-    ChangeNotes newNotes = notesFactory.createWithAutoRebuildingDisabled(c, null);
+    ChangeNotes newNotes = notesFactory.createWithAutoRebuildingDisabled(c);
     assertThat(newNotes.getChange().getTopic()).isNotEqualTo(topic);
     assertThat(newNotes.getChange().getTopic()).isEqualTo(oldNotes.getChange().getTopic());
   }