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
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index 721d8db..3517e22 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -863,15 +863,15 @@
=== 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 @@
}
@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);
diff --git a/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
index 2087104..494da86 100644
--- a/java/com/google/gerrit/pgm/util/BatchProgramModule.java
+++ b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
@@ -39,6 +39,7 @@
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.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 @@
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());
diff --git a/java/com/google/gerrit/server/DynamicOptions.java b/java/com/google/gerrit/server/DynamicOptions.java
index 11d3336..2ae6899 100644
--- a/java/com/google/gerrit/server/DynamicOptions.java
+++ b/java/com/google/gerrit/server/DynamicOptions.java
@@ -155,6 +155,10 @@
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
diff --git a/java/com/google/gerrit/server/change/ChangeAttributeFactory.java b/java/com/google/gerrit/server/change/ChangeAttributeFactory.java
new file mode 100644
index 0000000..1f91853
--- /dev/null
+++ b/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);
+}
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index e975c3d..fdb51c4 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -96,7 +96,6 @@
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;
diff --git a/java/com/google/gerrit/server/query/change/PluginDefinedAttributesFactory.java b/java/com/google/gerrit/server/change/PluginDefinedAttributesFactory.java
similarity index 82%
rename from java/com/google/gerrit/server/query/change/PluginDefinedAttributesFactory.java
rename to java/com/google/gerrit/server/change/PluginDefinedAttributesFactory.java
index a795025..08d6ce7 100644
--- a/java/com/google/gerrit/server/query/change/PluginDefinedAttributesFactory.java
+++ b/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 {
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 711efaf..aa06e5b 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -97,6 +97,7 @@
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.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 @@
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));
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java b/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
index d15d0dd..066b443 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
@@ -33,6 +33,8 @@
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 @@
* 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 @@
dynamicBeans.put(plugin, dynamicBean);
}
+ @Override
public DynamicBean getDynamicBean(String plugin) {
return dynamicBeans.get(plugin);
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/PluginFieldsIT.java b/javatests/com/google/gerrit/acceptance/api/change/PluginFieldsIT.java
index e495c99..88ef004 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/PluginFieldsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/PluginFieldsIT.java
@@ -34,7 +34,7 @@
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 @@
@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 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 @@
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;
});
}
