Merge "VisibleRefFilter Avoid touching the database for drafts"

gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/InProcessProtocol.java

@@ -29,13 +29,14 @@ import com.google.gerrit.server.RequestCleanup;
 import com.google.gerrit.server.config.GerritRequestModule;
 import com.google.gerrit.server.config.RequestScopedReviewDbProvider;
 import com.google.gerrit.server.git.AsyncReceiveCommits;
-import com.google.gerrit.server.git.ChangeCache;
 import com.google.gerrit.server.git.ReceiveCommits;
 import com.google.gerrit.server.git.ReceivePackInitializer;
+import com.google.gerrit.server.git.SearchingChangeCacheImpl;
 import com.google.gerrit.server.git.TagCache;
 import com.google.gerrit.server.git.TransferConfig;
 import com.google.gerrit.server.git.VisibleRefFilter;
 import com.google.gerrit.server.git.validators.UploadValidators;
+import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.project.ProjectControl;
 import com.google.gerrit.server.util.RequestContext;
@@ -211,8 +212,9 @@ class InProcessProtocol extends TestProtocol<Context> {
     private final Provider<ReviewDb> dbProvider;
     private final Provider<CurrentUser> userProvider;
     private final TagCache tagCache;
-    private final ChangeCache changeCache;
+    private final SearchingChangeCacheImpl changeCache;
     private final ProjectControl.GenericFactory projectControlFactory;
+    private final ChangeNotes.Factory changeNotesFactory;
     private final TransferConfig transferConfig;
     private final DynamicSet<PreUploadHook> preUploadHooks;
     private final UploadValidators.Factory uploadValidatorsFactory;
@@ -223,8 +225,9 @@ class InProcessProtocol extends TestProtocol<Context> {
         Provider<ReviewDb> dbProvider,
         Provider<CurrentUser> userProvider,
         TagCache tagCache,
-        ChangeCache changeCache,
+        SearchingChangeCacheImpl changeCache,
         ProjectControl.GenericFactory projectControlFactory,
+        ChangeNotes.Factory changeNotesFactory,
         TransferConfig transferConfig,
         DynamicSet<PreUploadHook> preUploadHooks,
         UploadValidators.Factory uploadValidatorsFactory,
@@ -234,6 +237,7 @@ class InProcessProtocol extends TestProtocol<Context> {
       this.tagCache = tagCache;
       this.changeCache = changeCache;
       this.projectControlFactory = projectControlFactory;
+      this.changeNotesFactory = changeNotesFactory;
       this.transferConfig = transferConfig;
       this.preUploadHooks = preUploadHooks;
       this.uploadValidatorsFactory = uploadValidatorsFactory;
@@ -260,7 +264,8 @@ class InProcessProtocol extends TestProtocol<Context> {
         up.setPackConfig(transferConfig.getPackConfig());
         up.setTimeout(transferConfig.getTimeout());
         up.setAdvertiseRefsHook(new VisibleRefFilter(
-            tagCache, changeCache, repo, ctl, dbProvider.get(), true));
+            tagCache, changeNotesFactory, changeCache, repo, ctl,
+            dbProvider.get(), true));
         List<PreUploadHook> hooks = Lists.newArrayList(preUploadHooks);
         hooks.add(uploadValidatorsFactory.create(
             ctl.getProject(), repo, "localhost-test"));

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/VisibleRefFilterIT.java

@@ -29,15 +29,17 @@ import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.edit.ChangeEditModifier;
-import com.google.gerrit.server.git.ChangeCache;
 import com.google.gerrit.server.git.ProjectConfig;
+import com.google.gerrit.server.git.SearchingChangeCacheImpl;
 import com.google.gerrit.server.git.TagCache;
 import com.google.gerrit.server.git.VisibleRefFilter;
 import com.google.gerrit.server.project.ProjectControl;
 import com.google.gerrit.server.project.Util;
 import com.google.gerrit.testutil.DisabledReviewDb;
 import com.google.inject.Inject;
+import com.google.inject.Provider;
 
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Ref;
@@ -59,11 +61,14 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
   private ProjectControl.GenericFactory projectControlFactory;
 
   @Inject
-  private ChangeCache changeCache;
+  private SearchingChangeCacheImpl changeCache;
 
   @Inject
   private TagCache tagCache;
 
+  @Inject
+  private Provider<CurrentUser> userProvider;
+
   private AccountGroup.UUID admins;
 
   private Change.Id c1;
@@ -136,6 +141,7 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
     Util.doNotInherit(cfg, Permission.READ, "refs/meta/config");
     saveProjectConfig(project, cfg);
 
+    setApiUser(user);
     assertRefs(
         "HEAD",
         r1 + "1",
@@ -171,6 +177,7 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
     allow(Permission.READ, REGISTERED_USERS, "refs/heads/master");
     deny(Permission.READ, REGISTERED_USERS, "refs/heads/branch");
 
+    setApiUser(user);
     assertRefs(
         "HEAD",
         r1 + "1",
@@ -184,6 +191,7 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
     deny(Permission.READ, REGISTERED_USERS, "refs/heads/master");
     allow(Permission.READ, REGISTERED_USERS, "refs/heads/branch");
 
+    setApiUser(user);
     assertRefs(
         r2 + "1",
         r2 + "meta",
@@ -252,6 +260,70 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
     }
   }
 
+  @Test
+  public void draftRefs() throws Exception {
+    allow(Permission.READ, REGISTERED_USERS, "refs/heads/*");
+
+    PushOneCommit.Result br = pushFactory.create(db, admin.getIdent(), testRepo)
+        .to("refs/drafts/master");
+    br.assertOkStatus();
+    Change.Id c3 = br.getChange().getId();
+    String r3 = changeRefPrefix(c3);
+
+    // Only admin can see admin's draft change.
+    setApiUser(admin);
+    assertRefs(
+        "HEAD",
+        r1 + "1",
+        r1 + "meta",
+        r2 + "1",
+        r2 + "meta",
+        r3 + "1",
+        r3 + "meta",
+        "refs/heads/branch",
+        "refs/heads/master",
+        "refs/meta/config",
+        "refs/tags/branch-tag",
+        "refs/tags/master-tag");
+
+    // user can't.
+    setApiUser(user);
+    assertRefs(
+        "HEAD",
+        r1 + "1",
+        r1 + "meta",
+        r2 + "1",
+        r2 + "meta",
+        "refs/heads/branch",
+        "refs/heads/master",
+        "refs/tags/branch-tag",
+        "refs/tags/master-tag");
+  }
+
+  @Test
+  public void noSearchingChangeCacheImpl() throws Exception {
+    allow(Permission.READ, REGISTERED_USERS, "refs/heads/*");
+
+    setApiUser(user);
+    try (Repository repo = repoManager.openRepository(project)) {
+      assertRefs(
+          repo,
+          new VisibleRefFilter(tagCache, notesFactory, null, repo,
+              projectControl(), db, true),
+          // Can't use stored values from the index so DB must be enabled.
+          false,
+          "HEAD",
+          r1 + "1",
+          r1 + "meta",
+          r2 + "1",
+          r2 + "meta",
+          "refs/heads/branch",
+          "refs/heads/master",
+          "refs/tags/branch-tag",
+          "refs/tags/master-tag");
+    }
+  }
+
   /**
    * Assert that refs seen by a non-admin user match expected.
    *
@@ -261,6 +333,18 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
    * @throws Exception
    */
   private void assertRefs(String... expectedWithMeta) throws Exception {
+    try (Repository repo = repoManager.openRepository(project)) {
+      assertRefs(
+          repo,
+          new VisibleRefFilter(tagCache, notesFactory, changeCache, repo,
+              projectControl(), new DisabledReviewDb(), true),
+          true,
+          expectedWithMeta);
+    }
+  }
+
+  private void assertRefs(Repository repo, VisibleRefFilter filter,
+      boolean disableDb, String... expectedWithMeta) throws Exception {
     List<String> expected = new ArrayList<>(expectedWithMeta.length);
     for (String r : expectedWithMeta) {
       if (notesMigration.writeChanges() || !r.endsWith(RefNames.META_SUFFIX)) {
@@ -268,17 +352,22 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
       }
     }
 
-    AcceptanceTestRequestScope.Context ctx = disableDb();
+    AcceptanceTestRequestScope.Context ctx = null;
-    try (Repository repo = repoManager.openRepository(project)) {
+    if (disableDb) {
-      ProjectControl ctl = projectControlFactory.controlFor(project,
+      ctx = disableDb();
-          identifiedUserFactory.create(user.getId()));
+    }
-      VisibleRefFilter filter = new VisibleRefFilter(
+    try {
-          tagCache, changeCache, repo, ctl, new DisabledReviewDb(), true);
       Map<String, Ref> all = repo.getAllRefs();
       assertThat(filter.filter(all, false).keySet())
           .containsExactlyElementsIn(expected);
     } finally {
+      if (disableDb) {
         enableDb(ctx);
       }
     }
   }
+
+  private ProjectControl projectControl() throws Exception {
+    return projectControlFactory.controlFor(project, userProvider.get());
+  }
+}

gerrit-httpd/src/main/java/com/google/gerrit/httpd/GitOverHttpServlet.java

@@ -25,14 +25,15 @@ import com.google.gerrit.server.AnonymousUser;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.cache.CacheModule;
 import com.google.gerrit.server.git.AsyncReceiveCommits;
-import com.google.gerrit.server.git.ChangeCache;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.git.ReceiveCommits;
+import com.google.gerrit.server.git.SearchingChangeCacheImpl;
 import com.google.gerrit.server.git.TagCache;
 import com.google.gerrit.server.git.TransferConfig;
 import com.google.gerrit.server.git.UploadPackMetricsHook;
 import com.google.gerrit.server.git.VisibleRefFilter;
 import com.google.gerrit.server.git.validators.UploadValidators;
+import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.project.ProjectControl;
 import com.google.inject.AbstractModule;
@@ -223,14 +224,18 @@ public class GitOverHttpServlet extends GitServlet {
   static class UploadFilter implements Filter {
     private final Provider<ReviewDb> db;
     private final TagCache tagCache;
-    private final ChangeCache changeCache;
+    private final ChangeNotes.Factory changeNotesFactory;
+    private final SearchingChangeCacheImpl changeCache;
     private final UploadValidators.Factory uploadValidatorsFactory;
 
     @Inject
-    UploadFilter(Provider<ReviewDb> db, TagCache tagCache, ChangeCache changeCache,
+    UploadFilter(Provider<ReviewDb> db, TagCache tagCache,
+        ChangeNotes.Factory changeNotesFactory,
+        SearchingChangeCacheImpl changeCache,
         UploadValidators.Factory uploadValidatorsFactory) {
       this.db = db;
       this.tagCache = tagCache;
+      this.changeNotesFactory = changeNotesFactory;
       this.changeCache = changeCache;
       this.uploadValidatorsFactory = uploadValidatorsFactory;
     }
@@ -255,8 +260,8 @@ public class GitOverHttpServlet extends GitServlet {
           uploadValidatorsFactory.create(pc.getProject(), repo, request.getRemoteHost());
       up.setPreUploadHook(PreUploadHookChain.newChain(
           Lists.newArrayList(up.getPreUploadHook(), uploadValidators)));
-      up.setAdvertiseRefsHook(new VisibleRefFilter(tagCache, changeCache,
+      up.setAdvertiseRefsHook(new VisibleRefFilter(tagCache, changeNotesFactory,
-          repo, pc, db.get(), true));
+          changeCache, repo, pc, db.get(), true));
 
       next.doFilter(request, response);
     }

gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java

@@ -47,6 +47,7 @@ import com.google.gerrit.server.config.GitReceivePackGroups;
 import com.google.gerrit.server.config.GitUploadPackGroups;
 import com.google.gerrit.server.git.BatchUpdate;
 import com.google.gerrit.server.git.MergeUtil;
+import com.google.gerrit.server.git.SearchingChangeCacheImpl;
 import com.google.gerrit.server.git.TagCache;
 import com.google.gerrit.server.group.GroupModule;
 import com.google.gerrit.server.mail.ReplacePatchSetSender;
@@ -120,6 +121,11 @@ public class BatchProgramModule extends FactoryModule {
     factory(PatchSetInserter.Factory.class);
     factory(RebaseChangeOp.Factory.class);
 
+    // As Reindex is a batch program, don't assume the index is available for
+    // the change cache.
+    bind(SearchingChangeCacheImpl.class).toProvider(
+        Providers.<SearchingChangeCacheImpl>of(null));
+
     bind(new TypeLiteral<Set<AccountGroup.UUID>>() {})
       .annotatedWith(GitUploadPackGroups.class)
       .toInstance(Collections.<AccountGroup.UUID> emptySet());

gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java

@@ -373,7 +373,7 @@ public class ReceiveCommits {
       GitRepositoryManager repoManager,
       TagCache tagCache,
       AccountCache accountCache,
-      ChangeCache changeCache,
+      SearchingChangeCacheImpl changeCache,
       ChangesCollection changes,
       ChangeInserter.Factory changeInserterFactory,
       CommitValidators.Factory commitValidatorsFactory,
@@ -480,8 +480,8 @@ public class ReceiveCommits {
       rp.setCheckReferencedObjectsAreReachable(
           receiveConfig.checkReferencedObjectsAreReachable);
     }
-    rp.setAdvertiseRefsHook(new VisibleRefFilter(tagCache, changeCache, repo,
+    rp.setAdvertiseRefsHook(new VisibleRefFilter(tagCache, notesFactory,
-        projectControl, db, false));
+        changeCache, repo, projectControl, db, false));
     List<AdvertiseRefsHook> advHooks = new ArrayList<>(3);
     advHooks.add(new AdvertiseRefsHook() {
       @Override

gerrit-server/src/main/java/com/google/gerrit/server/git/SearchingChangeCacheImpl.java

@@ -14,12 +14,18 @@
 
 package com.google.gerrit.server.git;
 
+import com.google.auto.value.AutoValue;
+import com.google.common.base.Function;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
+import com.google.common.collect.Lists;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
 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.server.ReviewerSet;
 import com.google.gerrit.server.cache.CacheModule;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.query.change.InternalChangeQuery;
@@ -34,6 +40,7 @@ import com.google.inject.name.Named;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.ExecutionException;
@@ -52,25 +59,62 @@ public class SearchingChangeCacheImpl
         bind(ChangeCache.class).to(SearchingChangeCacheImpl.class);
         cache(ID_CACHE,
             Project.NameKey.class,
-            new TypeLiteral<List<Change>>() {})
+            new TypeLiteral<List<CachedChange>>() {})
           .maximumWeight(0)
           .loader(Loader.class);
       }
     };
   }
 
-  private final LoadingCache<Project.NameKey, List<Change>> cache;
+  @AutoValue
+  static abstract class CachedChange {
+    // Subset of fields in ChangeData, specifically fields needed to serve
+    // VisibleRefFilter without touching the database. More can be added as
+    // necessary.
+    abstract Change change();
+    @Nullable abstract ReviewerSet reviewers();
+  }
+
+  private final LoadingCache<Project.NameKey, List<CachedChange>> cache;
+  private final ChangeData.Factory changeDataFactory;
 
   @Inject
   SearchingChangeCacheImpl(
-      @Named(ID_CACHE) LoadingCache<Project.NameKey, List<Change>> cache) {
+      @Named(ID_CACHE) LoadingCache<Project.NameKey, List<CachedChange>> cache,
+      ChangeData.Factory changeDataFactory) {
     this.cache = cache;
+    this.changeDataFactory = changeDataFactory;
   }
 
   @Override
   public List<Change> get(Project.NameKey name) {
     try {
-      return cache.get(name);
+      return Lists.transform(
+          cache.get(name),
+          new Function<CachedChange, Change>() {
+            @Override
+            public Change apply(CachedChange in) {
+              return in.change();
+            }
+          });
+    } catch (ExecutionException e) {
+      log.warn("Cannot fetch changes for " + name, e);
+      return Collections.emptyList();
+    }
+  }
+
+  // TODO(dborowitz): I think VisibleRefFilter is the only user of ChangeCache
+  // at all; probably we can just stop implementing that interface entirely.
+  public List<ChangeData> getChangeData(ReviewDb db, Project.NameKey name) {
+    try {
+      List<CachedChange> cached = cache.get(name);
+      List<ChangeData> cds = new ArrayList<>(cached.size());
+      for (CachedChange cc : cached) {
+        ChangeData cd = changeDataFactory.create(db, cc.change());
+        cd.setReviewers(cc.reviewers());
+        cds.add(cd);
+      }
+      return Collections.unmodifiableList(cds);
     } catch (ExecutionException e) {
       log.warn("Cannot fetch changes for " + name, e);
       return Collections.emptyList();
@@ -84,7 +128,7 @@ public class SearchingChangeCacheImpl
     }
   }
 
-  static class Loader extends CacheLoader<Project.NameKey, List<Change>> {
+  static class Loader extends CacheLoader<Project.NameKey, List<CachedChange>> {
     private final OneOffRequestContext requestContext;
     private final Provider<InternalChangeQuery> queryProvider;
 
@@ -96,10 +140,15 @@ public class SearchingChangeCacheImpl
     }
 
     @Override
-    public List<Change> load(Project.NameKey key) throws Exception {
+    public List<CachedChange> load(Project.NameKey key) throws Exception {
       try (AutoCloseable ctx = requestContext.open()) {
-        return Collections.unmodifiableList(
+        List<ChangeData> cds = queryProvider.get().byProject(key);
-            ChangeData.asChanges(queryProvider.get().byProject(key)));
+        List<CachedChange> result = new ArrayList<>(cds.size());
+        for (ChangeData cd : cds) {
+          result.add(new AutoValue_SearchingChangeCacheImpl_CachedChange(
+              cd.change(), cd.getReviewers()));
+        }
+        return Collections.unmodifiableList(result);
       }
     }
   }

gerrit-server/src/main/java/com/google/gerrit/server/git/VisibleRefFilter.java

@@ -16,13 +16,16 @@ package com.google.gerrit.server.git;
 
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Maps;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.reviewdb.client.Account;
 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.server.IdentifiedUser;
+import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.project.ProjectControl;
+import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gwtorm.server.OrmException;
 
 import org.eclipse.jgit.lib.Constants;
@@ -50,17 +53,24 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
       LoggerFactory.getLogger(VisibleRefFilter.class);
 
   private final TagCache tagCache;
-  private final ChangeCache changeCache;
+  private final ChangeNotes.Factory changeNotesFactory;
+  @Nullable private final SearchingChangeCacheImpl changeCache;
   private final Repository db;
   private final Project.NameKey projectName;
   private final ProjectControl projectCtl;
   private final ReviewDb reviewDb;
   private final boolean showMetadata;
 
-  public VisibleRefFilter(TagCache tagCache, ChangeCache changeCache,
+  public VisibleRefFilter(
-      Repository db, ProjectControl projectControl, ReviewDb reviewDb,
+      TagCache tagCache,
+      ChangeNotes.Factory changeNotesFactory,
+      @Nullable SearchingChangeCacheImpl changeCache,
+      Repository db,
+      ProjectControl projectControl,
+      ReviewDb reviewDb,
       boolean showMetadata) {
     this.tagCache = tagCache;
+    this.changeNotesFactory = changeNotesFactory;
     this.changeCache = changeCache;
     this.db = db;
     this.projectName = projectControl.getProject().getNameKey();
@@ -195,14 +205,21 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
   private Set<Change.Id> visibleChanges() {
     if (!showMetadata) {
       return Collections.emptySet();
+    } else if (changeCache == null) {
+      return visibleChangesByScan();
+    }
+    return visibleChangesBySearch();
   }
 
-    final Project project = projectCtl.getProject();
+  private Set<Change.Id> visibleChangesBySearch() {
+    Project project = projectCtl.getProject();
     try {
-      final Set<Change.Id> visibleChanges = new HashSet<>();
+      Set<Change.Id> visibleChanges = new HashSet<>();
-      for (Change change : changeCache.get(project.getNameKey())) {
+      for (ChangeData cd : changeCache.getChangeData(
-        if (projectCtl.controlForIndexedChange(change).isVisible(reviewDb)) {
+          reviewDb, project.getNameKey())) {
-          visibleChanges.add(change.getId());
+        if (projectCtl.controlForIndexedChange(cd.change())
+            .isVisible(reviewDb, cd)) {
+          visibleChanges.add(cd.getId());
         }
       }
       return visibleChanges;
@@ -213,6 +230,23 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
     }
   }
 
+  private Set<Change.Id> visibleChangesByScan() {
+    Project.NameKey project = projectCtl.getProject().getNameKey();
+    try {
+      Set<Change.Id> visibleChanges = new HashSet<>();
+      for (ChangeNotes cn : changeNotesFactory.scan(db, reviewDb, project)) {
+        if (projectCtl.controlFor(cn).isVisible(reviewDb)) {
+          visibleChanges.add(cn.getChangeId());
+        }
+      }
+      return visibleChanges;
+    } catch (IOException | OrmException e) {
+      log.error("Cannot load changes for project " + project
+          + ", assuming no changes are visible", e);
+      return Collections.emptySet();
+    }
+  }
+
   private static boolean isTag(Ref ref) {
     return ref.getLeaf().getName().startsWith(Constants.R_TAGS);
   }

gerrit-server/src/main/java/com/google/gerrit/server/project/ListTags.java

@@ -23,10 +23,11 @@ import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.CommonConverters;
-import com.google.gerrit.server.git.ChangeCache;
 import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.SearchingChangeCacheImpl;
 import com.google.gerrit.server.git.TagCache;
 import com.google.gerrit.server.git.VisibleRefFilter;
+import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 
@@ -52,7 +53,8 @@ public class ListTags implements RestReadView<ProjectResource> {
   private final GitRepositoryManager repoManager;
   private final Provider<ReviewDb> dbProvider;
   private final TagCache tagCache;
-  private final ChangeCache changeCache;
+  private final ChangeNotes.Factory changeNotesFactory;
+  private final SearchingChangeCacheImpl changeCache;
 
   @Option(name = "--limit", aliases = {"-n"}, metaVar = "CNT", usage = "maximum number of tags to list")
   public void setLimit(int limit) {
@@ -83,10 +85,12 @@ public class ListTags implements RestReadView<ProjectResource> {
   public ListTags(GitRepositoryManager repoManager,
       Provider<ReviewDb> dbProvider,
       TagCache tagCache,
-      ChangeCache changeCache) {
+      ChangeNotes.Factory changeNotesFactory,
+      SearchingChangeCacheImpl changeCache) {
     this.repoManager = repoManager;
     this.dbProvider = dbProvider;
     this.tagCache = tagCache;
+    this.changeNotesFactory = changeNotesFactory;
     this.changeCache = changeCache;
   }
 
@@ -147,7 +151,7 @@ public class ListTags implements RestReadView<ProjectResource> {
 
   private Map<String, Ref> visibleTags(ProjectControl control, Repository repo,
       Map<String, Ref> tags) {
-    return new VisibleRefFilter(tagCache, changeCache, repo,
+    return new VisibleRefFilter(tagCache, changeNotesFactory, changeCache, repo,
         control, dbProvider.get(), false).filter(tags, true);
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java

@@ -37,8 +37,8 @@ import com.google.gerrit.server.change.IncludedInResolver;
 import com.google.gerrit.server.config.CanonicalWebUrl;
 import com.google.gerrit.server.config.GitReceivePackGroups;
 import com.google.gerrit.server.config.GitUploadPackGroups;
-import com.google.gerrit.server.git.ChangeCache;
 import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.SearchingChangeCacheImpl;
 import com.google.gerrit.server.git.TagCache;
 import com.google.gerrit.server.git.VisibleRefFilter;
 import com.google.gerrit.server.group.SystemGroupBackend;
@@ -149,11 +149,12 @@ public class ProjectControl {
   private final CurrentUser user;
   private final ProjectState state;
   private final GitRepositoryManager repoManager;
+  private final ChangeNotes.Factory changeNotesFactory;
   private final ChangeControl.Factory changeControlFactory;
   private final PermissionCollection.Factory permissionFilter;
   private final Collection<ContributorAgreement> contributorAgreements;
   private final TagCache tagCache;
-  private final ChangeCache changeCache;
+  private final SearchingChangeCacheImpl changeCache;
 
   private List<SectionMatcher> allSections;
   private List<SectionMatcher> localSections;
@@ -167,13 +168,15 @@ public class ProjectControl {
       ProjectCache pc,
       PermissionCollection.Factory permissionFilter,
       GitRepositoryManager repoManager,
+      ChangeNotes.Factory changeNotesFactory,
       ChangeControl.Factory changeControlFactory,
       TagCache tagCache,
-      ChangeCache changeCache,
+      SearchingChangeCacheImpl changeCache,
       @CanonicalWebUrl @Nullable String canonicalWebUrl,
       @Assisted CurrentUser who,
       @Assisted ProjectState ps) {
     this.repoManager = repoManager;
+    this.changeNotesFactory = changeNotesFactory;
     this.changeControlFactory = changeControlFactory;
     this.tagCache = tagCache;
     this.changeCache = changeCache;
@@ -528,8 +531,8 @@ public class ProjectControl {
 
   boolean isMergedIntoVisibleRef(Repository repo, ReviewDb db, RevWalk rw,
       RevCommit commit, Collection<Ref> unfilteredRefs) throws IOException {
-    VisibleRefFilter filter =
+    VisibleRefFilter filter = new VisibleRefFilter(
-        new VisibleRefFilter(tagCache, changeCache, repo, this, db, true);
+        tagCache, changeNotesFactory, changeCache, repo, this, db, true);
     Map<String, Ref> m = Maps.newHashMapWithExpectedSize(unfilteredRefs.size());
     for (Ref r : unfilteredRefs) {
       m.put(r.getName(), r);

gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java

@@ -916,6 +916,10 @@ public void setPatchSets(Collection<PatchSet> patchSets) {
     this.reviewers = reviewers;
   }
 
+  public ReviewerSet getReviewers() {
+    return reviewers;
+  }
+
   public Collection<PatchLineComment> publishedComments()
       throws OrmException {
     if (publishedComments == null) {

gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java

@@ -879,7 +879,7 @@ public class RefControlTest {
 
     return new ProjectControl(Collections.<AccountGroup.UUID> emptySet(),
         Collections.<AccountGroup.UUID> emptySet(), projectCache,
-        sectionSorter, repoManager, changeControlFactory, null, null,
+        sectionSorter, repoManager, null, changeControlFactory, null, null,
         canonicalWebUrl, new MockUser(name, memberOf), newProjectState(local));
   }
 

gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/LsUserRefs.java

@@ -24,10 +24,11 @@ import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountResolver;
-import com.google.gerrit.server.git.ChangeCache;
 import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.SearchingChangeCacheImpl;
 import com.google.gerrit.server.git.TagCache;
 import com.google.gerrit.server.git.VisibleRefFilter;
+import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.project.ProjectControl;
 import com.google.gerrit.sshd.CommandMetaData;
 import com.google.gerrit.sshd.SshCommand;
@@ -59,7 +60,10 @@ public class LsUserRefs extends SshCommand {
   private TagCache tagCache;
 
   @Inject
-  private ChangeCache changeCache;
+  private ChangeNotes.Factory changeNotesFactory;
+
+  @Inject
+  private SearchingChangeCacheImpl changeCache;
 
   @Option(name = "--project", aliases = {"-p"}, metaVar = "PROJECT",
       required = true, usage = "project for which the refs should be listed")
@@ -95,9 +99,10 @@ public class LsUserRefs extends SshCommand {
     try (Repository repo = repoManager.openRepository(
         userProjectControl.getProject().getNameKey())) {
       try {
-        Map<String, Ref> refsMap =
+        Map<String, Ref> refsMap = new VisibleRefFilter(
-            new VisibleRefFilter(tagCache, changeCache, repo, userProjectControl,
+                tagCache, changeNotesFactory, changeCache, repo,
-                db, true).filter(repo.getRefDatabase().getRefs(ALL), false);
+                userProjectControl, db, true)
+            .filter(repo.getRefDatabase().getRefs(ALL), false);
 
         for (final String ref : refsMap.keySet()) {
           if (!onlyRefsHeads || ref.startsWith(RefNames.REFS_HEADS)) {

gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/Upload.java

@@ -17,13 +17,14 @@ package com.google.gerrit.sshd.commands;
 import com.google.common.collect.Lists;
 import com.google.gerrit.extensions.registration.DynamicSet;
 import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.git.ChangeCache;
+import com.google.gerrit.server.git.SearchingChangeCacheImpl;
 import com.google.gerrit.server.git.TagCache;
 import com.google.gerrit.server.git.TransferConfig;
 import com.google.gerrit.server.git.UploadPackMetricsHook;
 import com.google.gerrit.server.git.VisibleRefFilter;
 import com.google.gerrit.server.git.validators.UploadValidationException;
 import com.google.gerrit.server.git.validators.UploadValidators;
+import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.sshd.AbstractGitCommand;
 import com.google.gerrit.sshd.SshSession;
 import com.google.inject.Inject;
@@ -47,7 +48,10 @@ final class Upload extends AbstractGitCommand {
   private TagCache tagCache;
 
   @Inject
-  private ChangeCache changeCache;
+  private ChangeNotes.Factory changeNotesFactory;
+
+  @Inject
+  private SearchingChangeCacheImpl changeCache;
 
   @Inject
   private DynamicSet<PreUploadHook> preUploadHooks;
@@ -68,8 +72,10 @@ final class Upload extends AbstractGitCommand {
     }
 
     final UploadPack up = new UploadPack(repo);
-    up.setAdvertiseRefsHook(new VisibleRefFilter(tagCache, changeCache, repo,
+    up.setAdvertiseRefsHook(
-        projectControl, db, true));
+        new VisibleRefFilter(
+            tagCache, changeNotesFactory, changeCache, repo, projectControl, db,
+            true));
     up.setPackConfig(config.getPackConfig());
     up.setTimeout(config.getTimeout());
     up.setPostUploadHook(uploadMetrics);