Merge "Make RevisionNote public for checks plugin"

java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java

diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java b/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
index 18aab18..d15d0dd 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
@@ -16,9 +16,10 @@
 
 import static com.google.common.base.Preconditions.checkState;
 import static com.google.gerrit.server.query.change.ChangeQueryBuilder.FIELD_LIMIT;
+import static java.util.concurrent.TimeUnit.MINUTES;
 
-import com.google.common.collect.HashMultimap;
-import com.google.common.collect.Multimap;
+import com.google.common.collect.ImmutableListMultimap;
+import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.extensions.common.PluginDefinedInfo;
 import com.google.gerrit.extensions.registration.DynamicMap;
 import com.google.gerrit.index.IndexConfig;
@@ -55,6 +56,8 @@
  */
 public class ChangeQueryProcessor extends QueryProcessor<ChangeData>
     implements DynamicOptions.BeanReceiver, PluginDefinedAttributesFactory {
+  private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
   /**
    * Register a ChangeAttributeFactory in a config Module like this:
    *
@@ -67,9 +70,7 @@
 
   private final Provider<CurrentUser> userProvider;
   private final ChangeNotes.Factory notesFactory;
-  private final DynamicMap<ChangeAttributeFactory> attributeFactories;
-  private final Multimap<String, ChangeAttributeFactory> attributeFactoriesByPlugin =
-      HashMultimap.create();
+  private final ImmutableListMultimap<String, ChangeAttributeFactory> attributeFactoriesByPlugin;
   private final PermissionBackend permissionBackend;
   private final ProjectCache projectCache;
   private final Provider<AnonymousUser> anonymousUserProvider;
@@ -105,11 +106,16 @@
         () -> limitsFactory.create(userProvider.get()).getQueryLimit());
     this.userProvider = userProvider;
     this.notesFactory = notesFactory;
-    this.attributeFactories = attributeFactories;
     this.permissionBackend = permissionBackend;
     this.projectCache = projectCache;
     this.anonymousUserProvider = anonymousUserProvider;
-    setupAttributeFactories();
+
+    ImmutableListMultimap.Builder<String, ChangeAttributeFactory> factoriesBuilder =
+        ImmutableListMultimap.builder();
+    // Eagerly call Extension#get() rather than storing Extensions, since that method invokes the
+    // Provider on every call, which could be expensive if we invoke it once for every change.
+    attributeFactories.forEach(e -> factoriesBuilder.put(e.getPluginName(), e.get()));
+    attributeFactoriesByPlugin = factoriesBuilder.build();
   }
 
   @Override
@@ -133,25 +139,18 @@
     return dynamicBeans.get(plugin);
   }
 
-  public void setupAttributeFactories() {
-    for (String plugin : attributeFactories.plugins()) {
-      for (Provider<ChangeAttributeFactory> provider :
-          attributeFactories.byPlugin(plugin).values()) {
-        attributeFactoriesByPlugin.put(plugin, provider.get());
-      }
-    }
-  }
-
   @Override
   public List<PluginDefinedInfo> create(ChangeData cd) {
-    List<PluginDefinedInfo> plugins = new ArrayList<>(attributeFactories.plugins().size());
+    List<PluginDefinedInfo> plugins = new ArrayList<>(attributeFactoriesByPlugin.size());
     for (Map.Entry<String, ChangeAttributeFactory> e : attributeFactoriesByPlugin.entries()) {
       String plugin = e.getKey();
       PluginDefinedInfo pda = null;
       try {
         pda = e.getValue().create(cd, this, plugin);
       } catch (RuntimeException ex) {
-        /* Eat runtime exceptions so that queries don't fail. */
+        // Log once a minute, to avoid spamming logs with one stack trace per change.
+        logger.atWarning().atMostEvery(1, MINUTES).withCause(ex).log(
+            "error populating attribute on change %s from plugin %s", cd.getId(), plugin);
       }
       if (pda != null) {
         pda.name = plugin;