Extract ChangeAttributeFactory from ChangeQueryProcessor

Introduce a new BeanProvider interface to pass to the create method.
This will allow the interface to be reused for other codepaths, namely
GetChange. Other than switching to the new class names, this should not
significantly impact plugin implementations.

Change-Id: Id318739f1e75e8f0f5981b2d2fd23993fba7e5c1

Documentation/dev-plugins.txt

@@ -863,15 +863,15 @@ boolean json;
 === Query Attributes ===
 
 Plugins can provide additional attributes to be returned in Gerrit queries by
-implementing the ChangeAttributeFactory interface and registering it to the
-ChangeQueryProcessor.ChangeAttributeFactory class in the plugin module's
-'configure()' method. The new attribute(s) will be output under a "plugin"
-attribute in the change query output. This can be further controlled with an
-option registered in the Http and Ssh modules' 'configure*()' methods.
+implementing the `ChangeAttributeFactory` interface and registering it to the
+class in the plugin module's `configure()` method. The new attribute(s) will be
+output under a `plugin` attribute in the change query output. This can be
+further controlled with an option registered in the Http and Ssh modules'
+`configure*()` methods.
 
-The example below shows a plugin that adds two attributes ('exampleName' and
-'changeValue'), to the change query output, when the query command is provided
-the --myplugin-name--all option.
+The example below shows a plugin that adds two attributes (`exampleName` and
+`changeValue`), to the change query output, when the query command is provided
+the `--myplugin-name--all` option.
 
 [source, java]
 ----
@@ -921,9 +921,9 @@ public class AttributeFactory implements ChangeAttributeFactory {
   }
 
   @Override
-  public PluginDefinedInfo create(ChangeData c, ChangeQueryProcessor qp, String plugin) {
+  public PluginDefinedInfo create(ChangeData c, BeanProvider bp, String plugin) {
     if (options == null) {
-      options = (MyQueryOptions) qp.getDynamicBean(plugin);
+      options = (MyQueryOptions) bp.getDynamicBean(plugin);
     }
     if (options.all) {
       return new PluginAttribute(c);

java/com/google/gerrit/pgm/util/BatchProgramModule.java

@@ -39,6 +39,7 @@ import com.google.gerrit.server.account.externalids.ExternalIdModule;
 import com.google.gerrit.server.cache.CacheRemovalListener;
 import com.google.gerrit.server.cache.h2.H2CacheModule;
 import com.google.gerrit.server.cache.mem.DefaultMemoryCacheModule;
+import com.google.gerrit.server.change.ChangeAttributeFactory;
 import com.google.gerrit.server.change.ChangeJson;
 import com.google.gerrit.server.change.ChangeKindCacheImpl;
 import com.google.gerrit.server.change.MergeabilityCacheImpl;
@@ -72,7 +73,6 @@ import com.google.gerrit.server.project.ProjectCacheImpl;
 import com.google.gerrit.server.project.ProjectState;
 import com.google.gerrit.server.project.SubmitRuleEvaluator;
 import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.query.change.ChangeQueryProcessor;
 import com.google.gerrit.server.restapi.group.GroupModule;
 import com.google.gerrit.server.rules.DefaultSubmitRule;
 import com.google.gerrit.server.rules.IgnoreSelfApprovalRule;
@@ -112,7 +112,7 @@ public class BatchProgramModule extends FactoryModule {
     bind(new TypeLiteral<List<CommentLinkInfo>>() {})
         .toProvider(CommentLinkProvider.class)
         .in(SINGLETON);
-    bind(new TypeLiteral<DynamicMap<ChangeQueryProcessor.ChangeAttributeFactory>>() {})
+    bind(new TypeLiteral<DynamicMap<ChangeAttributeFactory>>() {})
         .toInstance(DynamicMap.emptyMap());
     bind(new TypeLiteral<DynamicMap<RestView<CommitResource>>>() {})
         .toInstance(DynamicMap.emptyMap());

java/com/google/gerrit/server/DynamicOptions.java

@@ -155,6 +155,10 @@ public class DynamicOptions {
     void setDynamicBean(String plugin, DynamicBean dynamicBean);
   }
 
+  public interface BeanProvider {
+    DynamicBean getDynamicBean(String plugin);
+  }
+
   /**
    * MergedClassloaders allow us to load classes from both plugin classloaders. Store the merged
    * classloaders in a Map to avoid creating a new classloader for each invocation. Use a

java/com/google/gerrit/server/change/ChangeAttributeFactory.java

@@ -0,0 +1,52 @@
+// Copyright (C) 2019 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.change;
+
+import com.google.gerrit.extensions.common.PluginDefinedInfo;
+import com.google.gerrit.server.DynamicOptions.BeanProvider;
+import com.google.gerrit.server.query.change.ChangeData;
+
+/**
+ * Interface for plugins to provide additional fields in {@link
+ * com.google.gerrit.extensions.common.ChangeInfo ChangeInfo}.
+ *
+ * <p>Register a {@code ChangeAttributeFactory} in a plugin {@code Module} like this:
+ *
+ * <pre>
+ * bind(ChangeAttributeFactory.class)
+ *     .annotatedWith(Exports.named("export-name"))
+ *     .to(YourClass.class);
+ * </pre>
+ *
+ * The export name can be anything unique to your plugin; the name is not exposed.
+ *
+ * <p>See the <a
+ * href="https://gerrit-review.googlesource.com/Documentation/dev-plugins.html#query_attributes">plugin
+ * developer documentation for more details and examples.
+ */
+public interface ChangeAttributeFactory {
+
+  /**
+   * Create a plugin-provided info field.
+   *
+   * <p>Typically, implementations will subclass {@code PluginDefinedInfo} to add additional fields.
+   *
+   * @param cd change.
+   * @param beanProvider provider of {@code DynamicBean}s, which may be used for reading options.
+   * @param plugin plugin name.
+   * @return the plugin's special change info.
+   */
+  PluginDefinedInfo create(ChangeData cd, BeanProvider beanProvider, String plugin);
+}

java/com/google/gerrit/server/change/ChangeJson.java

@@ -96,7 +96,6 @@ import com.google.gerrit.server.project.RemoveReviewerControl;
 import com.google.gerrit.server.project.SubmitRuleOptions;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.query.change.ChangeData.ChangedLines;
-import com.google.gerrit.server.query.change.PluginDefinedAttributesFactory;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;

java/com/google/gerrit/server/change/PluginDefinedAttributesFactory.java

@@ -1,4 +1,4 @@
-// Copyright (C) 2016 The Android Open Source Project
+// Copyright (C) 2019 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.
@@ -12,9 +12,10 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package com.google.gerrit.server.query.change;
+package com.google.gerrit.server.change;
 
 import com.google.gerrit.extensions.common.PluginDefinedInfo;
+import com.google.gerrit.server.query.change.ChangeData;
 import java.util.List;
 
 public interface PluginDefinedAttributesFactory {

java/com/google/gerrit/server/config/GerritGlobalModule.java

@@ -97,6 +97,7 @@ import com.google.gerrit.server.avatar.AvatarProvider;
 import com.google.gerrit.server.cache.CacheRemovalListener;
 import com.google.gerrit.server.change.AbandonOp;
 import com.google.gerrit.server.change.AccountPatchReviewStore;
+import com.google.gerrit.server.change.ChangeAttributeFactory;
 import com.google.gerrit.server.change.ChangeFinder;
 import com.google.gerrit.server.change.ChangeJson;
 import com.google.gerrit.server.change.ChangeKindCacheImpl;
@@ -167,7 +168,6 @@ import com.google.gerrit.server.project.ProjectState;
 import com.google.gerrit.server.project.SubmitRuleEvaluator;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.query.change.ChangeQueryBuilder;
-import com.google.gerrit.server.query.change.ChangeQueryProcessor;
 import com.google.gerrit.server.query.change.ConflictsCacheImpl;
 import com.google.gerrit.server.quota.QuotaEnforcer;
 import com.google.gerrit.server.restapi.change.SuggestReviewers;
@@ -405,7 +405,7 @@ public class GerritGlobalModule extends FactoryModule {
 
     DynamicMap.mapOf(binder(), ChangeQueryBuilder.ChangeOperatorFactory.class);
     DynamicMap.mapOf(binder(), ChangeQueryBuilder.ChangeHasOperandFactory.class);
-    DynamicMap.mapOf(binder(), ChangeQueryProcessor.ChangeAttributeFactory.class);
+    DynamicMap.mapOf(binder(), ChangeAttributeFactory.class);
 
     install(new GitwebConfig.LegacyModule(cfg));
 

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

@@ -33,6 +33,8 @@ import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.DynamicOptions;
 import com.google.gerrit.server.DynamicOptions.DynamicBean;
 import com.google.gerrit.server.account.AccountLimits;
+import com.google.gerrit.server.change.ChangeAttributeFactory;
+import com.google.gerrit.server.change.PluginDefinedAttributesFactory;
 import com.google.gerrit.server.index.change.ChangeIndexCollection;
 import com.google.gerrit.server.index.change.ChangeIndexRewriter;
 import com.google.gerrit.server.index.change.ChangeSchemaDefinitions;
@@ -55,19 +57,11 @@ import java.util.Set;
  * holding on to a single instance.
  */
 public class ChangeQueryProcessor extends QueryProcessor<ChangeData>
-    implements DynamicOptions.BeanReceiver, PluginDefinedAttributesFactory {
+    implements DynamicOptions.BeanReceiver,
+        DynamicOptions.BeanProvider,
+        PluginDefinedAttributesFactory {
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
 
-  /**
-   * Register a ChangeAttributeFactory in a config Module like this:
-   *
-   * <p>bind(ChangeAttributeFactory.class) .annotatedWith(Exports.named("export-name"))
-   * .to(YourClass.class);
-   */
-  public interface ChangeAttributeFactory {
-    PluginDefinedInfo create(ChangeData a, ChangeQueryProcessor qp, String plugin);
-  }
-
   private final Provider<CurrentUser> userProvider;
   private final ChangeNotes.Factory notesFactory;
   private final ImmutableListMultimap<String, ChangeAttributeFactory> attributeFactoriesByPlugin;
@@ -135,6 +129,7 @@ public class ChangeQueryProcessor extends QueryProcessor<ChangeData>
     dynamicBeans.put(plugin, dynamicBean);
   }
 
+  @Override
   public DynamicBean getDynamicBean(String plugin) {
     return dynamicBeans.get(plugin);
   }

javatests/com/google/gerrit/acceptance/api/change/PluginFieldsIT.java

@@ -34,7 +34,7 @@ import com.google.gerrit.extensions.common.PluginDefinedInfo;
 import com.google.gerrit.json.OutputFormat;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.server.DynamicOptions.DynamicBean;
-import com.google.gerrit.server.query.change.ChangeQueryProcessor.ChangeAttributeFactory;
+import com.google.gerrit.server.change.ChangeAttributeFactory;
 import com.google.gerrit.server.query.change.OutputStreamQuery;
 import com.google.gerrit.server.restapi.change.QueryChanges;
 import com.google.gerrit.sshd.PluginCommandModule;
@@ -92,8 +92,8 @@ public class PluginFieldsIT extends AbstractDaemonTest {
     @Override
     public void configure() {
       bind(ChangeAttributeFactory.class)
-          .annotatedWith(Exports.named("always-null"))
-          .toInstance((cd, qp, p) -> null);
+          .annotatedWith(Exports.named("null"))
+          .toInstance((cd, bp, p) -> null);
     }
   }
 
@@ -102,7 +102,7 @@ public class PluginFieldsIT extends AbstractDaemonTest {
     public void configure() {
       bind(ChangeAttributeFactory.class)
           .annotatedWith(Exports.named("simple"))
-          .toInstance((cd, qp, p) -> new MyInfo("change " + cd.getId()));
+          .toInstance((cd, bp, p) -> new MyInfo("change " + cd.getId()));
     }
   }
 
@@ -117,8 +117,8 @@ public class PluginFieldsIT extends AbstractDaemonTest {
       bind(ChangeAttributeFactory.class)
           .annotatedWith(Exports.named("simple"))
           .toInstance(
-              (cd, qp, p) -> {
-                MyOptions opts = (MyOptions) qp.getDynamicBean(p);
+              (cd, bp, p) -> {
+                MyOptions opts = (MyOptions) bp.getDynamicBean(p);
                 return opts != null ? new MyInfo("opt " + opts.opt) : null;
               });
     }