Remove dead ScanningChangeCacheImpl

The ScanningChangeCacheImpl is now dead code. With VisibleRefFilter
using SearchingChangeCacheImpl directly the ScanningChangeCacheImpl
is no longer referenced by the server. This also lets us get rid of
the ChangeCache interface.

This cleanup of dead code enables further optimization of the
VisibleRefFilter -> SearchingChangeCacheImpl path.

Change-Id: I07144006358a70915ea0bf10ce04a8b826f22095

gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java

@@ -60,9 +60,9 @@ import com.google.gerrit.server.config.DownloadConfig;
 import com.google.gerrit.server.config.GerritGlobalModule;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.RestCacheAdminModule;
-import com.google.gerrit.server.git.ChangeCacheImplModule;
 import com.google.gerrit.server.git.GarbageCollectionModule;
 import com.google.gerrit.server.git.ReceiveCommitsExecutorModule;
+import com.google.gerrit.server.git.SearchingChangeCacheImpl;
 import com.google.gerrit.server.git.WorkQueue;
 import com.google.gerrit.server.index.DummyIndexModule;
 import com.google.gerrit.server.index.IndexModule;
@@ -348,7 +348,7 @@ public class Daemon extends SiteProgram {
     modules.add(new DiffExecutorModule());
     modules.add(new MimeUtil2Module());
     modules.add(cfgInjector.getInstance(GerritGlobalModule.class));
-    modules.add(new ChangeCacheImplModule(slave));
+    modules.add(new SearchingChangeCacheImpl.Module());
     modules.add(new InternalAccountDirectory.Module());
     modules.add(new DefaultCacheFactory.Module());
     if (emailModule != null) {

gerrit-pgm/src/main/java/com/google/gerrit/pgm/RebuildNoteDb.java

@@ -211,7 +211,6 @@ public class RebuildNoteDb extends SiteProgram {
       @Override
       public void configure() {
         install(dbInjector.getInstance(BatchProgramModule.class));
-        install(SearchingChangeCacheImpl.module());
         DynamicSet.bind(binder(), GitReferenceUpdatedListener.class).to(
             ReindexAfterUpdate.class);
         install(new DummyIndexModule());

gerrit-pgm/src/main/java/com/google/gerrit/pgm/Reindex.java

@@ -26,7 +26,6 @@ import com.google.gerrit.pgm.util.SiteProgram;
 import com.google.gerrit.pgm.util.ThreadLimiter;
 import com.google.gerrit.server.change.ChangeResource;
 import com.google.gerrit.server.config.GerritServerConfig;
-import com.google.gerrit.server.git.ScanningChangeCacheImpl;
 import com.google.gerrit.server.index.Index;
 import com.google.gerrit.server.index.IndexDefinition;
 import com.google.gerrit.server.index.IndexModule;
@@ -124,9 +123,6 @@ public class Reindex extends SiteProgram {
         throw new IllegalStateException("unsupported index.type");
     }
     modules.add(indexModule);
-    // Scan changes from git instead of relying on the secondary index, as we
-    // will have just deleted the old (possibly corrupt) index.
-    modules.add(ScanningChangeCacheImpl.module());
     modules.add(dbInjector.getInstance(BatchProgramModule.class));
     modules.add(new FactoryModule() {
       @Override

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

@@ -1,24 +0,0 @@
-// Copyright (C) 2015 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.git;
-
-import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.Project;
-
-import java.util.List;
-
-public interface ChangeCache {
-  List<Change> get(Project.NameKey name);
-}

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

@@ -1,38 +0,0 @@
-// Copyright (C) 2015 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.git;
-
-import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
-import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.inject.AbstractModule;
-
-public class ChangeCacheImplModule extends AbstractModule {
-  private final boolean slave;
-
-  public ChangeCacheImplModule(boolean slave) {
-    this.slave = slave;
-  }
-
-  @Override
-  protected void configure() {
-    if (slave) {
-      install(ScanningChangeCacheImpl.module());
-    } else {
-      install(SearchingChangeCacheImpl.module());
-      DynamicSet.bind(binder(), GitReferenceUpdatedListener.class)
-          .to(SearchingChangeCacheImpl.class);
-    }
-  }
-}

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

@@ -1,109 +0,0 @@
-// Copyright (C) 2015 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.git;
-
-import static com.google.gerrit.server.git.SearchingChangeCacheImpl.ID_CACHE;
-
-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.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.server.cache.CacheModule;
-import com.google.gerrit.server.notedb.ChangeNotes;
-import com.google.gerrit.server.util.ManualRequestContext;
-import com.google.gerrit.server.util.OneOffRequestContext;
-import com.google.inject.Inject;
-import com.google.inject.Module;
-import com.google.inject.Singleton;
-import com.google.inject.TypeLiteral;
-import com.google.inject.name.Named;
-
-import org.eclipse.jgit.lib.Repository;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import java.util.Collections;
-import java.util.List;
-import java.util.concurrent.ExecutionException;
-
-@Singleton
-public class ScanningChangeCacheImpl implements ChangeCache {
-  private static final Logger log =
-      LoggerFactory.getLogger(ScanningChangeCacheImpl.class);
-
-  public static Module module() {
-    return new CacheModule() {
-      @Override
-      protected void configure() {
-        bind(ChangeCache.class).to(ScanningChangeCacheImpl.class);
-        cache(ID_CACHE,
-            Project.NameKey.class,
-            new TypeLiteral<List<Change>>() {})
-          .maximumWeight(0)
-          .loader(Loader.class);
-      }
-    };
-  }
-
-  private final LoadingCache<Project.NameKey, List<Change>> cache;
-
-  @Inject
-  ScanningChangeCacheImpl(
-      @Named(ID_CACHE) LoadingCache<Project.NameKey, List<Change>> cache) {
-    this.cache = cache;
-  }
-
-  @Override
-  public List<Change> get(Project.NameKey name) {
-    try {
-      return cache.get(name);
-    } catch (ExecutionException e) {
-      log.warn("Cannot fetch changes for " + name, e);
-      return Collections.emptyList();
-    }
-  }
-
-  static class Loader extends CacheLoader<Project.NameKey, List<Change>> {
-    private final GitRepositoryManager repoManager;
-    private final ChangeNotes.Factory notesFactory;
-    private final OneOffRequestContext requestContext;
-
-    @Inject
-    Loader(GitRepositoryManager repoManager,
-        ChangeNotes.Factory notesFactory,
-        OneOffRequestContext requestContext) {
-      this.repoManager = repoManager;
-      this.notesFactory = notesFactory;
-      this.requestContext = requestContext;
-    }
-
-    @Override
-    public List<Change> load(Project.NameKey key) throws Exception {
-      try (Repository repo = repoManager.openRepository(key);
-          ManualRequestContext ctx = requestContext.open()) {
-        return Lists.transform(
-            notesFactory.scan(repo, ctx.getReviewDbProvider().get(), key),
-            new Function<ChangeNotes, Change>() {
-              @Override
-              public Change apply(ChangeNotes notes) {
-                return notes.getChange();
-              }
-            });
-      }
-    }
-  }
-}

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

@@ -15,12 +15,11 @@
 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.extensions.registration.DynamicSet;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RefNames;
@@ -31,7 +30,6 @@ import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.query.change.InternalChangeQuery;
 import com.google.gerrit.server.util.OneOffRequestContext;
 import com.google.inject.Inject;
-import com.google.inject.Module;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
 import com.google.inject.TypeLiteral;
@@ -46,24 +44,24 @@ import java.util.List;
 import java.util.concurrent.ExecutionException;
 
 @Singleton
-public class SearchingChangeCacheImpl
+public class SearchingChangeCacheImpl implements GitReferenceUpdatedListener {
-    implements ChangeCache, GitReferenceUpdatedListener {
   private static final Logger log =
       LoggerFactory.getLogger(SearchingChangeCacheImpl.class);
   static final String ID_CACHE = "changes";
 
-  public static Module module() {
+  public static class Module extends CacheModule {
-    return new CacheModule() {
+    @Override
-      @Override
+    protected void configure() {
-      protected void configure() {
+      cache(ID_CACHE,
-        bind(ChangeCache.class).to(SearchingChangeCacheImpl.class);
+          Project.NameKey.class,
-        cache(ID_CACHE,
+          new TypeLiteral<List<CachedChange>>() {})
-            Project.NameKey.class,
+        .maximumWeight(0)
-            new TypeLiteral<List<CachedChange>>() {})
+        .loader(Loader.class);
-          .maximumWeight(0)
+
-          .loader(Loader.class);
+      bind(SearchingChangeCacheImpl.class);
-      }
+      DynamicSet.bind(binder(), GitReferenceUpdatedListener.class)
-    };
+          .to(SearchingChangeCacheImpl.class);
+    }
   }
 
   @AutoValue
@@ -86,25 +84,6 @@ public class SearchingChangeCacheImpl
     this.changeDataFactory = changeDataFactory;
   }
 
-  @Override
-  public List<Change> get(Project.NameKey name) {
-    try {
-      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);

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

@@ -44,11 +44,11 @@ import com.google.gerrit.server.config.GerritServerId;
 import com.google.gerrit.server.config.SitePath;
 import com.google.gerrit.server.config.TrackingFooters;
 import com.google.gerrit.server.config.TrackingFootersProvider;
-import com.google.gerrit.server.git.ChangeCacheImplModule;
 import com.google.gerrit.server.git.ChangeUpdateExecutor;
 import com.google.gerrit.server.git.GarbageCollection;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.git.PerThreadRequestScope;
+import com.google.gerrit.server.git.SearchingChangeCacheImpl;
 import com.google.gerrit.server.git.SendEmailExecutor;
 import com.google.gerrit.server.index.IndexModule.IndexType;
 import com.google.gerrit.server.index.change.ChangeSchemaDefinitions;
@@ -142,7 +142,7 @@ public class InMemoryModule extends FactoryModule {
     });
     bind(MetricMaker.class).to(DisabledMetricMaker.class);
     install(cfgInjector.getInstance(GerritGlobalModule.class));
-    install(new ChangeCacheImplModule(false));
+    install(new SearchingChangeCacheImpl.Module());
     factory(GarbageCollection.Factory.class);
 
     bindScope(RequestScoped.class, PerThreadRequestScope.REQUEST);

gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java

@@ -43,10 +43,10 @@ import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.GerritServerConfigModule;
 import com.google.gerrit.server.config.RestCacheAdminModule;
 import com.google.gerrit.server.config.SitePath;
-import com.google.gerrit.server.git.ChangeCacheImplModule;
 import com.google.gerrit.server.git.GarbageCollectionModule;
 import com.google.gerrit.server.git.GitRepositoryManagerModule;
 import com.google.gerrit.server.git.ReceiveCommitsExecutorModule;
+import com.google.gerrit.server.git.SearchingChangeCacheImpl;
 import com.google.gerrit.server.git.WorkQueue;
 import com.google.gerrit.server.index.IndexModule;
 import com.google.gerrit.server.index.IndexModule.IndexType;
@@ -303,7 +303,7 @@ public class WebAppInitializer extends GuiceServletContextListener
     modules.add(new DiffExecutorModule());
     modules.add(new MimeUtil2Module());
     modules.add(cfgInjector.getInstance(GerritGlobalModule.class));
-    modules.add(new ChangeCacheImplModule(false));
+    modules.add(new SearchingChangeCacheImpl.Module());
     modules.add(new InternalAccountDirectory.Module());
     modules.add(new DefaultCacheFactory.Module());
     modules.add(new SmtpEmailSender.Module());