Allow plugins to provide bulk PluginDefinedInfos in query results
Create a ChangePluginDefinedInfoFactory interface to allow list of
ChangeData for plugins attribute to get bulk data to provide additional
attributes to be output in a change query result. A common use case for
this is for a class implementing ChangePluginDefinedInfoFactory to
handle many changes instead of a single change, for which all have
plugin-specific results added in the query output. This results into
decrease of roundtrip time for the queries and increases the performance
for bulk queries.
Change-Id: I294f29557e98d6ff4f4d82ff3d83c4a743aa622d
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index b4ae469..8f81775 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -864,13 +864,15 @@
[[query_attributes]]
=== Change Attributes ===
+==== ChangePluginDefinedInfoFactory
+
Plugins can provide additional attributes to be returned from the Get Change and
-Query Change APIs by implementing implementing the `ChangeAttributeFactory`
-interface and adding it to the `DynamicSet` in the plugin module's `configure()`
-method. The new attribute(s) will be output under a `plugin` attribute in the
-change output. This can be further controlled by registering a class containing
-@Option declarations as a `DynamicBean`, annotated with the with HTTP/SSH
-commands on which the options should be available.
+Query Change APIs by implementing the `ChangePluginDefinedInfoFactory` interface
+and adding it to the `DynamicSet` in the plugin module's `configure()` method.
+The new attribute(s) will be output under a `plugin` attribute in the change
+output. This can be further controlled by registering a class containing @Option
+declarations as a `DynamicBean`, annotated with the HTTP/SSH commands on
+which the options should be available.
The example below shows a plugin that adds two attributes (`exampleName` and
`changeValue`), to the change query output, when the query command is provided
@@ -882,7 +884,7 @@
@Override
protected void configure() {
// Register attribute factory.
- DynamicSet.bind(binder(), ChangeAttributeFactory.class)
+ DynamicSet.bind(binder(), ChangePluginDefinedInfoFactory.class)
.to(AttributeFactory.class);
// Register options for GET /changes/X/change and /changes/X/detail.
@@ -907,7 +909,7 @@
public boolean all = false;
}
-public class AttributeFactory implements ChangeAttributeFactory {
+public class AttributeFactory implements ChangePluginDefinedInfoFactory {
protected MyChangeOptions options;
public class PluginAttribute extends PluginDefinedInfo {
@@ -921,14 +923,17 @@
}
@Override
- public PluginDefinedInfo create(ChangeData c, BeanProvider bp, String plugin) {
+ public Map<Change.Id, PluginDefinedInfo> createPluginDefinedInfos(
+ Collection<ChangeData> cds, BeanProvider bp, String plugin) {
if (options == null) {
options = (MyChangeOptions) bp.getDynamicBean(plugin);
}
+ Map<Change.Id, PluginDefinedInfo> out = new HashMap<>();
if (options.all) {
- return new PluginAttribute(c);
+ cds.forEach(cd -> out.put(cd.getId(), new PluginAttribute(cd)));
+ return out;
}
- return null;
+ return ImmutableMap.of();
}
}
----
@@ -970,10 +975,20 @@
}
----
-Implementors of the `ChangeAttributeFactory` interface should check whether
-they need to contribute to the link:#change-etag-computation[change ETag
-computation] to prevent callers using ETags from potentially seeing outdated
-plugin attributes.
+Runtime exceptions generated by the implementors of ChangePluginDefinedInfoFactory
+are encapsulated in PluginDefinedInfo objects which are part of SSH/REST query output.
+
+==== ChangeAttributeFactory
+
+Alternatively, there is also `ChangeAttributeFactory` which takes in one single
+`ChangeData` at a time. `ChangePluginDefinedInfoFactory` should be preferred
+over this as it handles many changes at once which also decreases the round-trip
+time for queries resulting in performance increase for bulk queries.
+
+Implementors of the `ChangePluginDefinedInfoFactory` and `ChangeAttributeFactory`
+interfaces should check whether they need to contribute to the
+link:#change-etag-computation[change ETag computation] to prevent callers using
+ETags from potentially seeing outdated plugin attributes.
[[simple-configuration]]
== Simple Configuration in `gerrit.config`
diff --git a/java/com/google/gerrit/acceptance/AbstractPluginFieldsTest.java b/java/com/google/gerrit/acceptance/AbstractPluginFieldsTest.java
index 020602b..a91bc49 100644
--- a/java/com/google/gerrit/acceptance/AbstractPluginFieldsTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractPluginFieldsTest.java
@@ -21,26 +21,36 @@
import com.google.common.base.MoreObjects;
import com.google.common.collect.ImmutableListMultimap;
+import com.google.gerrit.acceptance.testsuite.change.ChangeOperations;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Change;
import com.google.gerrit.extensions.annotations.Exports;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.PluginDefinedInfo;
import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.server.DynamicOptions;
import com.google.gerrit.server.DynamicOptions.DynamicBean;
import com.google.gerrit.server.change.ChangeAttributeFactory;
+import com.google.gerrit.server.change.ChangePluginDefinedInfoFactory;
+import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.restapi.change.GetChange;
import com.google.gerrit.server.restapi.change.QueryChanges;
import com.google.gerrit.sshd.commands.Query;
import com.google.gson.Gson;
import com.google.gson.reflect.TypeToken;
import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
import com.google.inject.Module;
+import java.util.Collection;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
import java.util.Objects;
import org.kohsuke.args4j.Option;
public class AbstractPluginFieldsTest extends AbstractDaemonTest {
+ @Inject private ChangeOperations changeOperations;
+
protected static class MyInfo extends PluginDefinedInfo {
@Nullable String theAttribute;
@@ -91,6 +101,70 @@
}
}
+ protected static class PluginDefinedSimpleAttributeModule extends AbstractModule {
+ @Override
+ public void configure() {
+ DynamicSet.bind(binder(), ChangePluginDefinedInfoFactory.class)
+ .toInstance(
+ (cds, bp, p) -> {
+ Map<Change.Id, PluginDefinedInfo> out = new HashMap<>();
+ cds.forEach(cd -> out.put(cd.getId(), new MyInfo("change " + cd.getId())));
+ return out;
+ });
+ }
+ }
+
+ protected static class PluginDefinedBulkExceptionModule extends AbstractModule {
+ @Override
+ protected void configure() {
+ DynamicSet.bind(binder(), ChangePluginDefinedInfoFactory.class)
+ .toInstance(
+ (cds, bp, p) -> {
+ throw new RuntimeException("Sample Exception");
+ });
+ }
+ }
+
+ protected static class PluginDefinedChangesByCommitBulkAttributeModule extends AbstractModule {
+ @Override
+ public void configure() {
+ DynamicSet.bind(binder(), ChangePluginDefinedInfoFactory.class)
+ .toInstance(
+ (cds, bp, p) -> {
+ Map<Change.Id, PluginDefinedInfo> out = new HashMap<>();
+ cds.forEach(
+ cd ->
+ out.put(
+ cd.getId(),
+ !cd.commitMessage().contains("no-info")
+ ? new MyInfo("change " + cd.getId())
+ : null));
+ return out;
+ });
+ }
+ }
+
+ protected static class PluginDefinedSingleCallBulkAttributeModule extends AbstractModule {
+ @Override
+ public void configure() {
+ DynamicSet.bind(binder(), ChangePluginDefinedInfoFactory.class)
+ .to(SingleCallBulkFactoryAttribute.class);
+ }
+ }
+
+ protected static class SingleCallBulkFactoryAttribute implements ChangePluginDefinedInfoFactory {
+ public static int timesCreateCalled = 0;
+
+ @Override
+ public Map<Change.Id, PluginDefinedInfo> createPluginDefinedInfos(
+ Collection<ChangeData> cds, DynamicOptions.BeanProvider beanProvider, String plugin) {
+ timesCreateCalled++;
+ Map<Change.Id, PluginDefinedInfo> out = new HashMap<>();
+ cds.forEach(cd -> out.put(cd.getId(), new MyInfo("change " + cd.getId())));
+ return out;
+ }
+ }
+
private static class MyOptions implements DynamicBean {
@Option(name = "--opt")
private String opt;
@@ -111,6 +185,32 @@
}
}
+ public static class BulkAttributeFactoryWithOption implements ChangePluginDefinedInfoFactory {
+ protected MyOptions opts;
+
+ @Override
+ public Map<Change.Id, PluginDefinedInfo> createPluginDefinedInfos(
+ Collection<ChangeData> cds, DynamicOptions.BeanProvider beanProvider, String plugin) {
+ if (opts == null) {
+ opts = (MyOptions) beanProvider.getDynamicBean(plugin);
+ }
+ Map<Change.Id, PluginDefinedInfo> out = new HashMap<>();
+ cds.forEach(cd -> out.put(cd.getId(), new MyInfo("opt " + opts.opt)));
+ return out;
+ }
+ }
+
+ protected static class PluginDefinedOptionAttributeModule extends AbstractModule {
+ @Override
+ public void configure() {
+ DynamicSet.bind(binder(), ChangePluginDefinedInfoFactory.class)
+ .to(BulkAttributeFactoryWithOption.class);
+ bind(DynamicBean.class).annotatedWith(Exports.named(Query.class)).to(MyOptions.class);
+ bind(DynamicBean.class).annotatedWith(Exports.named(QueryChanges.class)).to(MyOptions.class);
+ bind(DynamicBean.class).annotatedWith(Exports.named(GetChange.class)).to(MyOptions.class);
+ }
+ }
+
protected void getChangeWithNullAttribute(PluginInfoGetter getter) throws Exception {
Change.Id id = createChange().getChange().getId();
assertThat(getter.call(id)).isNull();
@@ -138,6 +238,113 @@
assertThat(getter.call(id)).isNull();
}
+ protected void getSingleChangeWithPluginDefinedBulkAttribute(BulkPluginInfoGetterWithId getter)
+ throws Exception {
+ Change.Id id = createChange().getChange().getId();
+
+ Map<Change.Id, List<PluginDefinedInfo>> pluginInfos = getter.call(id);
+ assertThat(pluginInfos.get(id)).isNull();
+
+ try (AutoCloseable ignored =
+ installPlugin("my-plugin", PluginDefinedSimpleAttributeModule.class)) {
+ pluginInfos = getter.call(id);
+ assertThat(pluginInfos.get(id)).containsExactly(new MyInfo("my-plugin", "change " + id));
+ }
+
+ pluginInfos = getter.call(id);
+ assertThat(pluginInfos.get(id)).isNull();
+ }
+
+ protected void getMultipleChangesWithPluginDefinedBulkAttribute(BulkPluginInfoGetter getter)
+ throws Exception {
+ Change.Id id1 = createChange().getChange().getId();
+ Change.Id id2 = createChange().getChange().getId();
+
+ Map<Change.Id, List<PluginDefinedInfo>> pluginInfos = getter.call();
+ assertThat(pluginInfos.get(id1)).isNull();
+ assertThat(pluginInfos.get(id2)).isNull();
+
+ try (AutoCloseable ignored =
+ installPlugin("my-plugin", PluginDefinedSimpleAttributeModule.class)) {
+ pluginInfos = getter.call();
+ assertThat(pluginInfos.get(id1)).containsExactly(new MyInfo("my-plugin", "change " + id1));
+ assertThat(pluginInfos.get(id2)).containsExactly(new MyInfo("my-plugin", "change " + id2));
+ }
+
+ pluginInfos = getter.call();
+ assertThat(pluginInfos.get(id1)).isNull();
+ assertThat(pluginInfos.get(id2)).isNull();
+ }
+
+ protected void getChangesByCommitMessageWithPluginDefinedBulkAttribute(
+ BulkPluginInfoGetter getter) throws Exception {
+ Change.Id changeWithNoInfo = changeOperations.newChange().commitMessage("no-info").create();
+ Change.Id changeWithInfo = changeOperations.newChange().commitMessage("info").create();
+
+ Map<Change.Id, List<PluginDefinedInfo>> pluginInfos = getter.call();
+ assertThat(pluginInfos.get(changeWithNoInfo)).isNull();
+ assertThat(pluginInfos.get(changeWithInfo)).isNull();
+
+ try (AutoCloseable ignored =
+ installPlugin("my-plugin", PluginDefinedChangesByCommitBulkAttributeModule.class)) {
+ pluginInfos = getter.call();
+ assertThat(pluginInfos.get(changeWithNoInfo)).isNull();
+ assertThat(pluginInfos.get(changeWithInfo))
+ .containsExactly(new MyInfo("my-plugin", "change " + changeWithInfo));
+ }
+
+ pluginInfos = getter.call();
+ assertThat(pluginInfos.get(changeWithNoInfo)).isNull();
+ assertThat(pluginInfos.get(changeWithInfo)).isNull();
+ }
+
+ protected void getMultipleChangesWithPluginDefinedBulkAndChangeAttributes(
+ BulkPluginInfoGetter getter) throws Exception {
+ Change.Id id1 = createChange().getChange().getId();
+ Change.Id id2 = createChange().getChange().getId();
+
+ Map<Change.Id, List<PluginDefinedInfo>> pluginInfos = getter.call();
+ assertThat(pluginInfos.get(id1)).isNull();
+ assertThat(pluginInfos.get(id2)).isNull();
+
+ try (AutoCloseable ignored =
+ installPlugin("my-plugin-1", PluginDefinedSimpleAttributeModule.class);
+ AutoCloseable ignored1 = installPlugin("my-plugin-2", SimpleAttributeModule.class)) {
+ pluginInfos = getter.call();
+ assertThat(pluginInfos.get(id1)).contains(new MyInfo("my-plugin-1", "change " + id1));
+ assertThat(pluginInfos.get(id1)).contains(new MyInfo("my-plugin-2", "change " + id1));
+ assertThat(pluginInfos.get(id2)).contains(new MyInfo("my-plugin-1", "change " + id2));
+ assertThat(pluginInfos.get(id2)).contains(new MyInfo("my-plugin-2", "change " + id2));
+ }
+
+ pluginInfos = getter.call();
+ assertThat(pluginInfos.get(id1)).isNull();
+ assertThat(pluginInfos.get(id2)).isNull();
+ }
+
+ protected void getMultipleChangesWithPluginDefinedBulkAttributeInSingleCall(
+ BulkPluginInfoGetter getter) throws Exception {
+ Change.Id id1 = createChange().getChange().getId();
+ Change.Id id2 = createChange().getChange().getId();
+ int timesCalled = SingleCallBulkFactoryAttribute.timesCreateCalled;
+
+ Map<Change.Id, List<PluginDefinedInfo>> pluginInfos = getter.call();
+ assertThat(pluginInfos.get(id1)).isNull();
+ assertThat(pluginInfos.get(id2)).isNull();
+
+ try (AutoCloseable ignored =
+ installPlugin("my-plugin", PluginDefinedSingleCallBulkAttributeModule.class)) {
+ pluginInfos = getter.call();
+ assertThat(pluginInfos.get(id1)).containsExactly(new MyInfo("my-plugin", "change " + id1));
+ assertThat(pluginInfos.get(id2)).containsExactly(new MyInfo("my-plugin", "change " + id2));
+ assertThat(SingleCallBulkFactoryAttribute.timesCreateCalled).isEqualTo(timesCalled + 1);
+ }
+
+ pluginInfos = getter.call();
+ assertThat(pluginInfos.get(id1)).isNull();
+ assertThat(pluginInfos.get(id2)).isNull();
+ }
+
protected void getChangeWithOption(
PluginInfoGetter getterWithoutOptions, PluginInfoGetterWithOptions getterWithOptions)
throws Exception {
@@ -154,17 +361,61 @@
assertThat(getterWithoutOptions.call(id)).isNull();
}
- protected static List<MyInfo> pluginInfoFromSingletonList(List<ChangeInfo> changeInfos) {
+ protected void getChangeWithPluginDefinedBulkAttributeOption(
+ BulkPluginInfoGetterWithId getterWithoutOptions,
+ BulkPluginInfoGetterWithIdAndOptions getterWithOptions)
+ throws Exception {
+ Change.Id id = createChange().getChange().getId();
+ assertThat(getterWithoutOptions.call(id).get(id)).isNull();
+
+ try (AutoCloseable ignored =
+ installPlugin("my-plugin", PluginDefinedOptionAttributeModule.class)) {
+ assertThat(getterWithoutOptions.call(id).get(id))
+ .containsExactly(new MyInfo("my-plugin", "opt null"));
+ assertThat(
+ getterWithOptions.call(id, ImmutableListMultimap.of("my-plugin--opt", "foo")).get(id))
+ .containsExactly(new MyInfo("my-plugin", "opt foo"));
+ }
+
+ assertThat(getterWithoutOptions.call(id).get(id)).isNull();
+ }
+
+ protected void getChangeWithPluginDefinedBulkAttributeWithException(
+ BulkPluginInfoGetterWithId getter) throws Exception {
+ Change.Id id = createChange().getChange().getId();
+ assertThat(getter.call(id).get(id)).isNull();
+
+ try (AutoCloseable ignored =
+ installPlugin("my-plugin", PluginDefinedBulkExceptionModule.class)) {
+ PluginDefinedInfo errorInfo = new PluginDefinedInfo();
+ List<PluginDefinedInfo> outputInfos = getter.call(id).get(id);
+ assertThat(outputInfos).hasSize(1);
+ assertThat(outputInfos.get(0).name).isEqualTo("my-plugin");
+ assertThat(outputInfos.get(0).message).isEqualTo("Something went wrong in plugin: my-plugin");
+ }
+
+ assertThat(getter.call(id).get(id)).isNull();
+ }
+
+ protected static List<PluginDefinedInfo> pluginInfoFromSingletonList(
+ List<ChangeInfo> changeInfos) {
assertThat(changeInfos).hasSize(1);
return pluginInfoFromChangeInfo(changeInfos.get(0));
}
- protected static List<MyInfo> pluginInfoFromChangeInfo(ChangeInfo changeInfo) {
+ protected static List<PluginDefinedInfo> pluginInfoFromChangeInfo(ChangeInfo changeInfo) {
List<PluginDefinedInfo> pluginInfo = changeInfo.plugins;
if (pluginInfo == null) {
return null;
}
- return pluginInfo.stream().map(MyInfo.class::cast).collect(toImmutableList());
+ return pluginInfo.stream().map(PluginDefinedInfo.class::cast).collect(toImmutableList());
+ }
+
+ protected static Map<Change.Id, List<PluginDefinedInfo>> pluginInfosFromChangeInfos(
+ List<ChangeInfo> changeInfos) {
+ Map<Change.Id, List<PluginDefinedInfo>> out = new HashMap<>();
+ changeInfos.forEach(ci -> out.put(Change.id(ci._number), pluginInfoFromChangeInfo(ci)));
+ return out;
}
/**
@@ -180,7 +431,8 @@
* @param plugins list of {@code MyInfo} objects, each as a raw map returned from Gson.
* @return decoded list of {@code MyInfo}s.
*/
- protected static List<MyInfo> decodeRawPluginsList(Gson gson, @Nullable Object plugins) {
+ protected static List<PluginDefinedInfo> decodeRawPluginsList(
+ Gson gson, @Nullable Object plugins) {
if (plugins == null) {
return null;
}
@@ -188,14 +440,44 @@
return gson.fromJson(gson.toJson(plugins), new TypeToken<List<MyInfo>>() {}.getType());
}
+ protected static Map<Change.Id, List<PluginDefinedInfo>> getPluginInfosFromChangeInfos(
+ Gson gson, List<Map<String, Object>> changeInfos) {
+ Map<Change.Id, List<PluginDefinedInfo>> out = new HashMap<>();
+ changeInfos.forEach(
+ change -> {
+ Double changeId =
+ (Double)
+ (change.get("number") != null ? change.get("number") : change.get("_number"));
+ out.put(
+ Change.id(changeId.intValue()), decodeRawPluginsList(gson, change.get("plugins")));
+ });
+ return out;
+ }
+
@FunctionalInterface
protected interface PluginInfoGetter {
- List<MyInfo> call(Change.Id id) throws Exception;
+ List<PluginDefinedInfo> call(Change.Id id) throws Exception;
+ }
+
+ @FunctionalInterface
+ protected interface BulkPluginInfoGetter {
+ Map<Change.Id, List<PluginDefinedInfo>> call() throws Exception;
+ }
+
+ @FunctionalInterface
+ protected interface BulkPluginInfoGetterWithId {
+ Map<Change.Id, List<PluginDefinedInfo>> call(Change.Id id) throws Exception;
+ }
+
+ @FunctionalInterface
+ protected interface BulkPluginInfoGetterWithIdAndOptions {
+ Map<Change.Id, List<PluginDefinedInfo>> call(
+ Change.Id id, ImmutableListMultimap<String, String> pluginOptions) throws Exception;
}
@FunctionalInterface
protected interface PluginInfoGetterWithOptions {
- List<MyInfo> call(Change.Id id, ImmutableListMultimap<String, String> pluginOptions)
+ List<PluginDefinedInfo> call(Change.Id id, ImmutableListMultimap<String, String> pluginOptions)
throws Exception;
}
}
diff --git a/java/com/google/gerrit/extensions/common/PluginDefinedInfo.java b/java/com/google/gerrit/extensions/common/PluginDefinedInfo.java
index e6fef0f..69bfa2c 100644
--- a/java/com/google/gerrit/extensions/common/PluginDefinedInfo.java
+++ b/java/com/google/gerrit/extensions/common/PluginDefinedInfo.java
@@ -16,4 +16,5 @@
public class PluginDefinedInfo {
public String name;
+ public String message;
}
diff --git a/java/com/google/gerrit/server/change/ChangeAttributeFactory.java b/java/com/google/gerrit/server/change/ChangeAttributeFactory.java
index 95355cf..663d7aa 100644
--- a/java/com/google/gerrit/server/change/ChangeAttributeFactory.java
+++ b/java/com/google/gerrit/server/change/ChangeAttributeFactory.java
@@ -32,6 +32,7 @@
* href="https://gerrit-review.googlesource.com/Documentation/dev-plugins.html#query_attributes">plugin
* developer documentation for more details and examples.
*/
+@Deprecated
public interface ChangeAttributeFactory {
/**
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index 014955c9..8323cfd 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -39,6 +39,7 @@
import com.google.common.base.Joiner;
import com.google.common.base.MoreObjects;
import com.google.common.base.Throwables;
+import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ListMultimap;
@@ -67,6 +68,7 @@
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
import com.google.gerrit.extensions.common.LabelInfo;
+import com.google.gerrit.extensions.common.PluginDefinedInfo;
import com.google.gerrit.extensions.common.ProblemInfo;
import com.google.gerrit.extensions.common.ReviewerUpdateInfo;
import com.google.gerrit.extensions.common.RevisionInfo;
@@ -156,13 +158,17 @@
}
public ChangeJson create(Iterable<ListChangesOption> options) {
- return factory.create(options, Optional.empty());
+ return factory.create(options, Optional.empty(), Optional.empty());
}
public ChangeJson create(
Iterable<ListChangesOption> options,
- PluginDefinedAttributesFactory pluginDefinedAttributesFactory) {
- return factory.create(options, Optional.of(pluginDefinedAttributesFactory));
+ PluginDefinedAttributesFactory pluginDefinedAttributesFactory,
+ PluginDefinedInfosFactory pluginDefinedInfosFactory) {
+ return factory.create(
+ options,
+ Optional.of(pluginDefinedAttributesFactory),
+ Optional.of(pluginDefinedInfosFactory));
}
public ChangeJson create(ListChangesOption first, ListChangesOption... rest) {
@@ -173,7 +179,8 @@
public interface AssistedFactory {
ChangeJson create(
Iterable<ListChangesOption> options,
- Optional<PluginDefinedAttributesFactory> pluginDefinedAttributesFactory);
+ Optional<PluginDefinedAttributesFactory> pluginDefinedAttributesFactory,
+ Optional<PluginDefinedInfosFactory> pluginDefinedInfosFactory);
}
@Singleton
@@ -220,6 +227,7 @@
private final Metrics metrics;
private final RevisionJson revisionJson;
private final Optional<PluginDefinedAttributesFactory> pluginDefinedAttributesFactory;
+ private final Optional<PluginDefinedInfosFactory> pluginDefinedInfosFactory;
private final boolean includeMergeable;
private final boolean lazyLoad;
@@ -243,7 +251,8 @@
RevisionJson.Factory revisionJsonFactory,
@GerritServerConfig Config cfg,
@Assisted Iterable<ListChangesOption> options,
- @Assisted Optional<PluginDefinedAttributesFactory> pluginDefinedAttributesFactory) {
+ @Assisted Optional<PluginDefinedAttributesFactory> pluginDefinedAttributesFactory,
+ @Assisted Optional<PluginDefinedInfosFactory> pluginDefinedInfosFactory) {
this.userProvider = user;
this.changeDataFactory = cdf;
this.permissionBackend = permissionBackend;
@@ -261,6 +270,7 @@
this.includeMergeable = MergeabilityComputationBehavior.fromConfig(cfg).includeInApi();
this.lazyLoad = containsAnyOf(this.options, REQUIRE_LAZY_LOAD);
this.pluginDefinedAttributesFactory = pluginDefinedAttributesFactory;
+ this.pluginDefinedInfosFactory = pluginDefinedInfosFactory;
logger.atFine().log("options = %s", options);
}
@@ -279,12 +289,12 @@
}
public ChangeInfo format(ChangeData cd) {
- return format(cd, Optional.empty(), true);
+ return format(cd, Optional.empty(), true, getPluginInfos(cd));
}
public ChangeInfo format(RevisionResource rsrc) {
ChangeData cd = changeDataFactory.create(rsrc.getNotes());
- return format(cd, Optional.of(rsrc.getPatchSet().id()), true);
+ return format(cd, Optional.of(rsrc.getPatchSet().id()), true, getPluginInfos(cd));
}
public List<List<ChangeInfo>> format(List<QueryResult<ChangeData>> in)
@@ -293,8 +303,10 @@
accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS));
List<List<ChangeInfo>> res = new ArrayList<>(in.size());
Map<Change.Id, ChangeInfo> cache = Maps.newHashMapWithExpectedSize(in.size());
+ ImmutableListMultimap<Change.Id, PluginDefinedInfo> pluginInfosByChange =
+ getPluginInfos(in.stream().flatMap(e -> e.entities().stream()).collect(toList()));
for (QueryResult<ChangeData> r : in) {
- List<ChangeInfo> infos = toChangeInfos(r.entities(), cache);
+ List<ChangeInfo> infos = toChangeInfos(r.entities(), cache, pluginInfosByChange);
if (!infos.isEmpty() && r.more()) {
infos.get(infos.size() - 1)._moreChanges = true;
}
@@ -309,8 +321,9 @@
accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS));
ensureLoaded(in);
List<ChangeInfo> out = new ArrayList<>(in.size());
+ ImmutableListMultimap<Change.Id, PluginDefinedInfo> pluginInfosByChange = getPluginInfos(in);
for (ChangeData cd : in) {
- out.add(format(cd, Optional.empty(), false));
+ out.add(format(cd, Optional.empty(), false, pluginInfosByChange.get(cd.getId())));
}
accountLoader.fill();
return out;
@@ -326,7 +339,8 @@
}
return checkOnly(changeDataFactory.create(project, id));
}
- return format(changeDataFactory.create(notes), Optional.empty(), true);
+ ChangeData cd = changeDataFactory.create(notes);
+ return format(cd, Optional.empty(), true, getPluginInfos(cd));
}
private static Collection<SubmitRequirementInfo> requirementsFor(ChangeData cd) {
@@ -358,15 +372,18 @@
}
private ChangeInfo format(
- ChangeData cd, Optional<PatchSet.Id> limitToPsId, boolean fillAccountLoader) {
+ ChangeData cd,
+ Optional<PatchSet.Id> limitToPsId,
+ boolean fillAccountLoader,
+ List<PluginDefinedInfo> pluginInfosForChange) {
try {
if (fillAccountLoader) {
accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS));
- ChangeInfo res = toChangeInfo(cd, limitToPsId);
+ ChangeInfo res = toChangeInfo(cd, limitToPsId, pluginInfosForChange);
accountLoader.fill();
return res;
}
- return toChangeInfo(cd, limitToPsId);
+ return toChangeInfo(cd, limitToPsId, pluginInfosForChange);
} catch (PatchListNotAvailableException
| GpgException
| IOException
@@ -404,7 +421,9 @@
}
private List<ChangeInfo> toChangeInfos(
- List<ChangeData> changes, Map<Change.Id, ChangeInfo> cache) {
+ List<ChangeData> changes,
+ Map<Change.Id, ChangeInfo> cache,
+ ImmutableListMultimap<Change.Id, PluginDefinedInfo> pluginInfosByChange) {
try (Timer0.Context ignored = metrics.toChangeInfosLatency.start()) {
List<ChangeInfo> changeInfos = new ArrayList<>(changes.size());
for (int i = 0; i < changes.size(); i++) {
@@ -425,7 +444,7 @@
// Compute and cache if possible
try {
ensureLoaded(Collections.singleton(cd));
- info = format(cd, Optional.empty(), false);
+ info = format(cd, Optional.empty(), false, pluginInfosByChange.get(cd.getId()));
changeInfos.add(info);
if (isCacheable) {
cache.put(Change.id(info._number), info);
@@ -480,14 +499,18 @@
return info;
}
- private ChangeInfo toChangeInfo(ChangeData cd, Optional<PatchSet.Id> limitToPsId)
+ private ChangeInfo toChangeInfo(
+ ChangeData cd,
+ Optional<PatchSet.Id> limitToPsId,
+ List<PluginDefinedInfo> pluginInfosForChange)
throws PatchListNotAvailableException, GpgException, PermissionBackendException, IOException {
try (Timer0.Context ignored = metrics.toChangeInfoLatency.start()) {
- return toChangeInfoImpl(cd, limitToPsId);
+ return toChangeInfoImpl(cd, limitToPsId, pluginInfosForChange);
}
}
- private ChangeInfo toChangeInfoImpl(ChangeData cd, Optional<PatchSet.Id> limitToPsId)
+ private ChangeInfo toChangeInfoImpl(
+ ChangeData cd, Optional<PatchSet.Id> limitToPsId, List<PluginDefinedInfo> pluginInfos)
throws PatchListNotAvailableException, GpgException, PermissionBackendException, IOException {
ChangeInfo out = new ChangeInfo();
CurrentUser user = userProvider.get();
@@ -589,6 +612,15 @@
if (pluginDefinedAttributesFactory.isPresent()) {
out.plugins = pluginDefinedAttributesFactory.get().create(cd);
}
+
+ if (!pluginInfos.isEmpty()) {
+ if (out.plugins == null) {
+ out.plugins = pluginInfos;
+ } else {
+ out.plugins = new ArrayList<>(out.plugins);
+ out.plugins.addAll(pluginInfos);
+ }
+ }
out.revertOf = cd.change().getRevertOf() != null ? cd.change().getRevertOf().get() : null;
out.submissionId = cd.change().getSubmissionId();
out.cherryPickOfChange =
@@ -819,4 +851,16 @@
}
return map;
}
+
+ private List<PluginDefinedInfo> getPluginInfos(ChangeData cd) {
+ return getPluginInfos(Collections.singleton(cd)).get(cd.getId());
+ }
+
+ private ImmutableListMultimap<Change.Id, PluginDefinedInfo> getPluginInfos(
+ Collection<ChangeData> cds) {
+ if (pluginDefinedInfosFactory.isPresent()) {
+ return pluginDefinedInfosFactory.get().createPluginDefinedInfos(cds);
+ }
+ return ImmutableListMultimap.of();
+ }
}
diff --git a/java/com/google/gerrit/server/change/ChangePluginDefinedInfoFactory.java b/java/com/google/gerrit/server/change/ChangePluginDefinedInfoFactory.java
new file mode 100644
index 0000000..c6ceb61
--- /dev/null
+++ b/java/com/google/gerrit/server/change/ChangePluginDefinedInfoFactory.java
@@ -0,0 +1,52 @@
+// Copyright (C) 2020 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.entities.Change;
+import com.google.gerrit.extensions.common.PluginDefinedInfo;
+import com.google.gerrit.server.DynamicOptions.BeanProvider;
+import com.google.gerrit.server.query.change.ChangeData;
+import java.util.Collection;
+import java.util.Map;
+
+/**
+ * Interface for plugins to provide additional fields in {@link
+ * com.google.gerrit.extensions.common.ChangeInfo ChangeInfo}.
+ *
+ * <p>Register a {@code ChangePluginDefinedInfoFactory} in a plugin {@code Module} like this:
+ *
+ * <pre>
+ * DynamicSet.bind(binder(), ChangePluginDefinedInfoFactory.class).to(YourClass.class);
+ * </pre>
+ *
+ * <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 ChangePluginDefinedInfoFactory {
+
+ /**
+ * Create a plugin-provided info field for each of the provided {@link ChangeData}s.
+ *
+ * <p>Typically, implementations will subclass {@code PluginDefinedInfo} to add additional fields.
+ *
+ * @param cds changes.
+ * @param beanProvider provider of {@code DynamicBean}s, which may be used for reading options.
+ * @param plugin plugin name.
+ * @return map of the plugin's special info for each change
+ */
+ Map<Change.Id, PluginDefinedInfo> createPluginDefinedInfos(
+ Collection<ChangeData> cds, BeanProvider beanProvider, String plugin);
+}
diff --git a/java/com/google/gerrit/server/change/PluginDefinedAttributesFactories.java b/java/com/google/gerrit/server/change/PluginDefinedAttributesFactories.java
index 9928125..b474dab 100644
--- a/java/com/google/gerrit/server/change/PluginDefinedAttributesFactories.java
+++ b/java/com/google/gerrit/server/change/PluginDefinedAttributesFactories.java
@@ -18,12 +18,15 @@
import static java.util.concurrent.TimeUnit.MINUTES;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableListMultimap;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
+import com.google.gerrit.entities.Change;
import com.google.gerrit.extensions.common.PluginDefinedInfo;
import com.google.gerrit.extensions.registration.Extension;
import com.google.gerrit.server.DynamicOptions.BeanProvider;
import com.google.gerrit.server.query.change.ChangeData;
+import java.util.Collection;
import java.util.Objects;
import java.util.stream.Stream;
@@ -60,5 +63,44 @@
return pdi;
}
+ public static ImmutableListMultimap<Change.Id, PluginDefinedInfo> createAll(
+ Collection<ChangeData> cds,
+ BeanProvider beanProvider,
+ Stream<Extension<ChangePluginDefinedInfoFactory>> infoFactories) {
+ ImmutableListMultimap.Builder<Change.Id, PluginDefinedInfo> pluginInfosByChangeBuilder =
+ ImmutableListMultimap.builder();
+ infoFactories.forEach(
+ e -> tryCreate(cds, beanProvider, e.getPluginName(), e.get(), pluginInfosByChangeBuilder));
+ ImmutableListMultimap<Change.Id, PluginDefinedInfo> result = pluginInfosByChangeBuilder.build();
+ return result;
+ }
+
+ private static void tryCreate(
+ Collection<ChangeData> cds,
+ BeanProvider beanProvider,
+ String plugin,
+ ChangePluginDefinedInfoFactory infoFactory,
+ ImmutableListMultimap.Builder<Change.Id, PluginDefinedInfo> pluginInfosByChangeBuilder) {
+ try {
+ infoFactory
+ .createPluginDefinedInfos(cds, beanProvider, plugin)
+ .forEach(
+ (id, pdi) -> {
+ if (pdi != null) {
+ pdi.name = plugin;
+ pluginInfosByChangeBuilder.put(id, pdi);
+ }
+ });
+ } catch (RuntimeException ex) {
+ /* Propagate runtime exceptions as structured API data types so that queries don't fail. */
+ logger.atWarning().atMostEvery(1, MINUTES).withCause(ex).log(
+ "error populating attribute on changes from plugin %s", plugin);
+ PluginDefinedInfo errorInfo = new PluginDefinedInfo();
+ errorInfo.name = plugin;
+ errorInfo.message = "Something went wrong in plugin: " + plugin;
+ cds.forEach(cd -> pluginInfosByChangeBuilder.put(cd.getId(), errorInfo));
+ }
+ }
+
private PluginDefinedAttributesFactories() {}
}
diff --git a/java/com/google/gerrit/server/change/PluginDefinedInfosFactory.java b/java/com/google/gerrit/server/change/PluginDefinedInfosFactory.java
new file mode 100644
index 0000000..db57e29
--- /dev/null
+++ b/java/com/google/gerrit/server/change/PluginDefinedInfosFactory.java
@@ -0,0 +1,42 @@
+// Copyright (C) 2020 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.common.collect.ImmutableListMultimap;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.extensions.common.PluginDefinedInfo;
+import com.google.gerrit.server.query.change.ChangeData;
+import java.util.Collection;
+
+/**
+ * Interface to generate {@code PluginDefinedInfo}s from registered {@code
+ * ChangePluginDefinedInfoFactory}s.
+ *
+ * <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 PluginDefinedInfosFactory {
+
+ /**
+ * Create a plugin-provided info field from all the plugins for each of the provided {@link
+ * ChangeData}s.
+ *
+ * @param cds changes.
+ * @return map of the all plugin's special infos for each change.
+ */
+ ImmutableListMultimap<Change.Id, PluginDefinedInfo> createPluginDefinedInfos(
+ Collection<ChangeData> cds);
+}
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 22d02d2..78dd38c 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -110,6 +110,7 @@
import com.google.gerrit.server.change.ChangeFinder;
import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.change.ChangeKindCacheImpl;
+import com.google.gerrit.server.change.ChangePluginDefinedInfoFactory;
import com.google.gerrit.server.change.LabelsJson;
import com.google.gerrit.server.change.MergeabilityCacheImpl;
import com.google.gerrit.server.change.ReviewerSuggestion;
@@ -437,6 +438,7 @@
DynamicMap.mapOf(binder(), ChangeQueryBuilder.ChangeHasOperandFactory.class);
DynamicMap.mapOf(binder(), ChangeQueryBuilder.ChangeIsOperandFactory.class);
DynamicSet.setOf(binder(), ChangeAttributeFactory.class);
+ DynamicSet.setOf(binder(), ChangePluginDefinedInfoFactory.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 40c0477..370bc75 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
@@ -19,6 +19,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
+import com.google.gerrit.entities.Change;
import com.google.gerrit.extensions.common.PluginDefinedInfo;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.registration.Extension;
@@ -33,15 +34,20 @@
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.ChangePluginDefinedInfoFactory;
import com.google.gerrit.server.change.PluginDefinedAttributesFactories;
import com.google.gerrit.server.change.PluginDefinedAttributesFactory;
+import com.google.gerrit.server.change.PluginDefinedInfosFactory;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
import com.google.gerrit.server.index.change.ChangeIndexRewriter;
import com.google.gerrit.server.index.change.ChangeSchemaDefinitions;
import com.google.gerrit.server.index.change.IndexedChangeQuery;
import com.google.inject.Inject;
import com.google.inject.Provider;
+import java.util.ArrayList;
+import java.util.Collection;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -52,11 +58,13 @@
* holding on to a single instance.
*/
public class ChangeQueryProcessor extends QueryProcessor<ChangeData>
- implements DynamicOptions.BeanReceiver, DynamicOptions.BeanProvider {
+ implements DynamicOptions.BeanReceiver, DynamicOptions.BeanProvider, PluginDefinedInfosFactory {
private final Provider<CurrentUser> userProvider;
private final ImmutableListMultimap<String, ChangeAttributeFactory> attributeFactoriesByPlugin;
private final ChangeIsVisibleToPredicate.Factory changeIsVisibleToPredicateFactory;
private final Map<String, DynamicBean> dynamicBeans = new HashMap<>();
+ private final List<Extension<ChangePluginDefinedInfoFactory>>
+ changePluginDefinedInfoFactoriesByPlugin = new ArrayList<>();
static {
// It is assumed that basic rewrites do not touch visibleto predicates.
@@ -74,7 +82,8 @@
ChangeIndexCollection indexes,
ChangeIndexRewriter rewriter,
DynamicSet<ChangeAttributeFactory> attributeFactories,
- ChangeIsVisibleToPredicate.Factory changeIsVisibleToPredicateFactory) {
+ ChangeIsVisibleToPredicate.Factory changeIsVisibleToPredicateFactory,
+ DynamicSet<ChangePluginDefinedInfoFactory> changePluginDefinedInfoFactories) {
super(
metricMaker,
ChangeSchemaDefinitions.INSTANCE,
@@ -88,10 +97,15 @@
ImmutableListMultimap.Builder<String, ChangeAttributeFactory> factoriesBuilder =
ImmutableListMultimap.builder();
+ ImmutableListMultimap.Builder<String, ChangePluginDefinedInfoFactory> infosFactoriesBuilder =
+ 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.entries().forEach(e -> factoriesBuilder.put(e.getPluginName(), e.get()));
attributeFactoriesByPlugin = factoriesBuilder.build();
+ changePluginDefinedInfoFactories
+ .entries()
+ .forEach(e -> changePluginDefinedInfoFactoriesByPlugin.add(e));
}
@Override
@@ -128,6 +142,17 @@
.map(e -> new Extension<>(e.getKey(), e::getValue)));
}
+ public PluginDefinedInfosFactory getInfosFactory() {
+ return this::createPluginDefinedInfos;
+ }
+
+ @Override
+ public ImmutableListMultimap<Change.Id, PluginDefinedInfo> createPluginDefinedInfos(
+ Collection<ChangeData> cds) {
+ return PluginDefinedAttributesFactories.createAll(
+ cds, this, changePluginDefinedInfoFactoriesByPlugin.stream());
+ }
+
@Override
protected Predicate<ChangeData> enforceVisibility(Predicate<ChangeData> pred) {
return new AndChangeSource(
diff --git a/java/com/google/gerrit/server/query/change/OutputStreamQuery.java b/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
index 02e8434..b931457 100644
--- a/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
+++ b/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
@@ -17,11 +17,14 @@
import static com.google.common.base.Preconditions.checkState;
import static java.nio.charset.StandardCharsets.UTF_8;
+import com.google.common.collect.ImmutableListMultimap;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.LabelTypes;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.extensions.common.PluginDefinedInfo;
import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.index.query.QueryResult;
import com.google.gerrit.server.DynamicOptions;
@@ -97,6 +100,8 @@
private OutputStream outputStream = DisabledOutputStream.INSTANCE;
private PrintWriter out;
+ private ImmutableListMultimap<Change.Id, PluginDefinedInfo> pluginInfosByChange =
+ ImmutableListMultimap.of();
@Inject
OutputStreamQuery(
@@ -207,6 +212,7 @@
Map<Project.NameKey, Repository> repos = new HashMap<>();
Map<Project.NameKey, RevWalk> revWalks = new HashMap<>();
QueryResult<ChangeData> results = queryProcessor.query(queryBuilder.parse(queryString));
+ pluginInfosByChange = queryProcessor.createPluginDefinedInfos(results.entities());
try {
for (ChangeData d : results.entities()) {
show(buildChangeAttribute(d, repos, revWalks));
@@ -325,6 +331,15 @@
}
c.plugins = queryProcessor.getAttributesFactory().create(d);
+ List<PluginDefinedInfo> pluginInfos = pluginInfosByChange.get(d.getId());
+ if (!pluginInfos.isEmpty()) {
+ if (c.plugins == null) {
+ c.plugins = pluginInfos;
+ } else {
+ c.plugins = new ArrayList<>(c.plugins);
+ c.plugins.addAll(pluginInfos);
+ }
+ }
return c;
}
diff --git a/java/com/google/gerrit/server/restapi/change/GetChange.java b/java/com/google/gerrit/server/restapi/change/GetChange.java
index c28741b..1ef3c4b 100644
--- a/java/com/google/gerrit/server/restapi/change/GetChange.java
+++ b/java/com/google/gerrit/server/restapi/change/GetChange.java
@@ -15,7 +15,9 @@
package com.google.gerrit.server.restapi.change;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.Streams;
+import com.google.gerrit.entities.Change;
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.client.ListOption;
import com.google.gerrit.extensions.common.ChangeInfo;
@@ -27,11 +29,13 @@
import com.google.gerrit.server.DynamicOptions.DynamicBean;
import com.google.gerrit.server.change.ChangeAttributeFactory;
import com.google.gerrit.server.change.ChangeJson;
+import com.google.gerrit.server.change.ChangePluginDefinedInfoFactory;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.PluginDefinedAttributesFactories;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
+import java.util.Collection;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.Map;
@@ -43,6 +47,7 @@
DynamicOptions.BeanProvider {
private final ChangeJson.Factory json;
private final DynamicSet<ChangeAttributeFactory> attrFactories;
+ private final DynamicSet<ChangePluginDefinedInfoFactory> pdiFactories;
private final EnumSet<ListChangesOption> options = EnumSet.noneOf(ListChangesOption.class);
private final Map<String, DynamicBean> dynamicBeans = new HashMap<>();
@@ -57,9 +62,13 @@
}
@Inject
- GetChange(ChangeJson.Factory json, DynamicSet<ChangeAttributeFactory> attrFactories) {
+ GetChange(
+ ChangeJson.Factory json,
+ DynamicSet<ChangeAttributeFactory> attrFactories,
+ DynamicSet<ChangePluginDefinedInfoFactory> pdiFactories) {
this.json = json;
this.attrFactories = attrFactories;
+ this.pdiFactories = pdiFactories;
}
@Override
@@ -82,11 +91,17 @@
}
private ChangeJson newChangeJson() {
- return json.create(options, this::buildPluginInfo);
+ return json.create(options, this::buildPluginInfo, this::createPluginDefinedInfos);
}
private ImmutableList<PluginDefinedInfo> buildPluginInfo(ChangeData cd) {
return PluginDefinedAttributesFactories.createAll(
cd, this, Streams.stream(attrFactories.entries()));
}
+
+ private ImmutableListMultimap<Change.Id, PluginDefinedInfo> createPluginDefinedInfos(
+ Collection<ChangeData> cds) {
+ return PluginDefinedAttributesFactories.createAll(
+ cds, this, Streams.stream(pdiFactories.entries()));
+ }
}
diff --git a/java/com/google/gerrit/server/restapi/change/QueryChanges.java b/java/com/google/gerrit/server/restapi/change/QueryChanges.java
index 878e714..0fec476 100644
--- a/java/com/google/gerrit/server/restapi/change/QueryChanges.java
+++ b/java/com/google/gerrit/server/restapi/change/QueryChanges.java
@@ -165,7 +165,9 @@
int cnt = queries.size();
List<QueryResult<ChangeData>> results = queryProcessor.query(qb.parse(queries));
List<List<ChangeInfo>> res =
- json.create(options, queryProcessor.getAttributesFactory()).format(results);
+ json.create(
+ options, queryProcessor.getAttributesFactory(), queryProcessor.getInfosFactory())
+ .format(results);
for (int n = 0; n < cnt; n++) {
List<ChangeInfo> info = res.get(n);
if (results.get(n).more() && !info.isEmpty()) {
diff --git a/javatests/com/google/gerrit/acceptance/api/change/PluginFieldsIT.java b/javatests/com/google/gerrit/acceptance/api/change/PluginFieldsIT.java
index d5089ff..31198d5 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/PluginFieldsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/PluginFieldsIT.java
@@ -19,6 +19,7 @@
import com.google.gerrit.extensions.annotations.Exports;
import com.google.gerrit.server.change.ChangeAttributeFactory;
import com.google.inject.AbstractModule;
+import java.util.Arrays;
import org.junit.Test;
@NoHttpd
@@ -50,6 +51,18 @@
}
@Test
+ public void querySingleChangeWithBulkAttribute() throws Exception {
+ getSingleChangeWithPluginDefinedBulkAttribute(
+ id -> pluginInfosFromChangeInfos(gApi.changes().query(id.toString()).get()));
+ }
+
+ @Test
+ public void getSingleChangeWithBulkAttribute() throws Exception {
+ getSingleChangeWithPluginDefinedBulkAttribute(
+ id -> pluginInfosFromChangeInfos(Arrays.asList(gApi.changes().id(id.toString()).get())));
+ }
+
+ @Test
public void queryChangeWithOption() throws Exception {
getChangeWithOption(
id -> pluginInfoFromSingletonList(gApi.changes().query(id.toString()).get()),
@@ -65,6 +78,53 @@
(id, opts) -> pluginInfoFromChangeInfo(gApi.changes().id(id.get()).get(opts)));
}
+ @Test
+ public void queryChangeWithOptionBulkAttribute() throws Exception {
+ getChangeWithPluginDefinedBulkAttributeOption(
+ id -> pluginInfosFromChangeInfos(gApi.changes().query(id.toString()).get()),
+ (id, opts) ->
+ pluginInfosFromChangeInfos(
+ gApi.changes().query(id.toString()).withPluginOptions(opts).get()));
+ }
+
+ @Test
+ public void getChangeWithOptionBulkAttribute() throws Exception {
+ getChangeWithPluginDefinedBulkAttributeOption(
+ id -> pluginInfosFromChangeInfos(Arrays.asList(gApi.changes().id(id.get()).get())),
+ (id, opts) ->
+ pluginInfosFromChangeInfos(Arrays.asList(gApi.changes().id(id.get()).get(opts))));
+ }
+
+ @Test
+ public void queryMultipleChangesWithPluginDefinedAttribute() throws Exception {
+ getMultipleChangesWithPluginDefinedBulkAttribute(
+ () -> pluginInfosFromChangeInfos(gApi.changes().query("status:open").get()));
+ }
+
+ @Test
+ public void queryChangesByCommitMessageWithPluginDefinedBulkAttribute() throws Exception {
+ getChangesByCommitMessageWithPluginDefinedBulkAttribute(
+ () -> pluginInfosFromChangeInfos(gApi.changes().query("status:open").get()));
+ }
+
+ @Test
+ public void getMultipleChangesWithPluginDefinedAndChangeAttributes() throws Exception {
+ getMultipleChangesWithPluginDefinedBulkAndChangeAttributes(
+ () -> pluginInfosFromChangeInfos(gApi.changes().query("status:open").get()));
+ }
+
+ @Test
+ public void getMultipleChangesWithPluginDefinedAttributeInSingleCall() throws Exception {
+ getMultipleChangesWithPluginDefinedBulkAttributeInSingleCall(
+ () -> pluginInfosFromChangeInfos(gApi.changes().query("status:open").get()));
+ }
+
+ @Test
+ public void getChangeWithPluginDefinedException() throws Exception {
+ getChangeWithPluginDefinedBulkAttributeWithException(
+ id -> pluginInfosFromChangeInfos(Arrays.asList(gApi.changes().id(id.get()).get())));
+ }
+
static class SimpleAttributeWithExplicitExportModule extends AbstractModule {
@Override
public void configure() {
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/PluginFieldsIT.java b/javatests/com/google/gerrit/acceptance/rest/change/PluginFieldsIT.java
index 7649316..17bf37e 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/PluginFieldsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/PluginFieldsIT.java
@@ -22,9 +22,11 @@
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Change;
+import com.google.gerrit.extensions.common.PluginDefinedInfo;
import com.google.gerrit.json.OutputFormat;
import com.google.gson.Gson;
import com.google.gson.reflect.TypeToken;
+import java.util.Arrays;
import java.util.List;
import java.util.Map;
import org.junit.Test;
@@ -68,6 +70,24 @@
}
@Test
+ public void querySingleChangeWithBulkAttribute() throws Exception {
+ getSingleChangeWithPluginDefinedBulkAttribute(
+ id -> pluginInfosFromChangeInfos(adminRestSession.get(changeQueryUrl(id))));
+ }
+
+ @Test
+ public void pluginDefinedGetChangeWithSimpleAttribute() throws Exception {
+ getSingleChangeWithPluginDefinedBulkAttribute(
+ id -> pluginInfoMapFromChangeInfo(adminRestSession.get(changeUrl(id))));
+ }
+
+ @Test
+ public void pluginDefinedGetChangeDetailWithSimpleAttribute() throws Exception {
+ getSingleChangeWithPluginDefinedBulkAttribute(
+ id -> pluginInfoMapFromChangeInfo(adminRestSession.get(changeDetailUrl(id))));
+ }
+
+ @Test
public void queryChangeWithOption() throws Exception {
getChangeWithOption(
id -> pluginInfoFromSingletonList(adminRestSession.get(changeQueryUrl(id))),
@@ -88,6 +108,57 @@
(id, opts) -> pluginInfoFromChangeInfo(adminRestSession.get(changeDetailUrl(id, opts))));
}
+ @Test
+ public void pluginDefinedQueryChangeWithOption() throws Exception {
+ getChangeWithPluginDefinedBulkAttributeOption(
+ id -> pluginInfosFromChangeInfos(adminRestSession.get(changeQueryUrl(id))),
+ (id, opts) -> pluginInfosFromChangeInfos(adminRestSession.get(changeQueryUrl(id, opts))));
+ }
+
+ @Test
+ public void pluginDefinedGetChangeWithOption() throws Exception {
+ getChangeWithPluginDefinedBulkAttributeOption(
+ id -> pluginInfoMapFromChangeInfo(adminRestSession.get(changeUrl(id))),
+ (id, opts) -> pluginInfoMapFromChangeInfo(adminRestSession.get(changeUrl(id, opts))));
+ }
+
+ @Test
+ public void pluginDefinedGetChangeDetailWithOption() throws Exception {
+ getChangeWithPluginDefinedBulkAttributeOption(
+ id -> pluginInfoMapFromChangeInfo(adminRestSession.get(changeDetailUrl(id))),
+ (id, opts) -> pluginInfoMapFromChangeInfo(adminRestSession.get(changeDetailUrl(id, opts))));
+ }
+
+ @Test
+ public void queryMultipleChangesWithPluginDefinedAttribute() throws Exception {
+ getMultipleChangesWithPluginDefinedBulkAttribute(
+ () -> pluginInfosFromChangeInfos(adminRestSession.get("/changes/?q=status:open")));
+ }
+
+ @Test
+ public void queryChangesByCommitMessageWithPluginDefinedBulkAttribute() throws Exception {
+ getChangesByCommitMessageWithPluginDefinedBulkAttribute(
+ () -> pluginInfosFromChangeInfos(adminRestSession.get("/changes/?q=status:open")));
+ }
+
+ @Test
+ public void getMultipleChangesWithPluginDefinedAndChangeAttributes() throws Exception {
+ getMultipleChangesWithPluginDefinedBulkAndChangeAttributes(
+ () -> pluginInfosFromChangeInfos(adminRestSession.get("/changes/?q=status:open")));
+ }
+
+ @Test
+ public void getMultipleChangesWithPluginDefinedAttributeInSingleCall() throws Exception {
+ getMultipleChangesWithPluginDefinedBulkAttributeInSingleCall(
+ () -> pluginInfosFromChangeInfos(adminRestSession.get("/changes/?q=status:open")));
+ }
+
+ @Test
+ public void getChangeWithPluginDefinedException() throws Exception {
+ getChangeWithPluginDefinedBulkAttributeWithException(
+ id -> pluginInfoMapFromChangeInfo(adminRestSession.get(changeUrl(id))));
+ }
+
private String changeQueryUrl(Change.Id id) {
return changeQueryUrl(id, ImmutableListMultimap.of());
}
@@ -133,7 +204,8 @@
}
@Nullable
- private static List<MyInfo> pluginInfoFromSingletonList(RestResponse res) throws Exception {
+ private static List<PluginDefinedInfo> pluginInfoFromSingletonList(RestResponse res)
+ throws Exception {
res.assertOK();
List<Map<String, Object>> changeInfos =
GSON.fromJson(res.getReader(), new TypeToken<List<Map<String, Object>>>() {}.getType());
@@ -142,10 +214,28 @@
}
@Nullable
- private List<MyInfo> pluginInfoFromChangeInfo(RestResponse res) throws Exception {
+ private List<PluginDefinedInfo> pluginInfoFromChangeInfo(RestResponse res) throws Exception {
res.assertOK();
Map<String, Object> changeInfo =
GSON.fromJson(res.getReader(), new TypeToken<Map<String, Object>>() {}.getType());
return decodeRawPluginsList(GSON, changeInfo.get("plugins"));
}
+
+ @Nullable
+ private Map<Change.Id, List<PluginDefinedInfo>> pluginInfoMapFromChangeInfo(RestResponse res)
+ throws Exception {
+ res.assertOK();
+ Map<String, Object> changeInfo =
+ GSON.fromJson(res.getReader(), new TypeToken<Map<String, Object>>() {}.getType());
+ return getPluginInfosFromChangeInfos(GSON, Arrays.asList(changeInfo));
+ }
+
+ @Nullable
+ private Map<Change.Id, List<PluginDefinedInfo>> pluginInfosFromChangeInfos(RestResponse res)
+ throws Exception {
+ res.assertOK();
+ List<Map<String, Object>> changeInfos =
+ GSON.fromJson(res.getReader(), new TypeToken<List<Map<String, Object>>>() {}.getType());
+ return getPluginInfosFromChangeInfos(GSON, changeInfos);
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/ssh/PluginChangeFieldsIT.java b/javatests/com/google/gerrit/acceptance/ssh/PluginChangeFieldsIT.java
index 4bf7c19..38293f9 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/PluginChangeFieldsIT.java
+++ b/javatests/com/google/gerrit/acceptance/ssh/PluginChangeFieldsIT.java
@@ -23,6 +23,7 @@
import com.google.gerrit.acceptance.UseSsh;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Change;
+import com.google.gerrit.extensions.common.PluginDefinedInfo;
import com.google.gerrit.server.query.change.OutputStreamQuery;
import com.google.gson.Gson;
import com.google.gson.reflect.TypeToken;
@@ -52,12 +53,55 @@
}
@Test
+ public void querySingleChangeWithBulkAttribute() throws Exception {
+ getSingleChangeWithPluginDefinedBulkAttribute(
+ id -> pluginInfosFromList(adminSshSession.exec(changeQueryCmd(id))));
+ }
+
+ @Test
public void queryChangeWithOption() throws Exception {
getChangeWithOption(
id -> pluginInfoFromSingletonList(adminSshSession.exec(changeQueryCmd(id))),
(id, opts) -> pluginInfoFromSingletonList(adminSshSession.exec(changeQueryCmd(id, opts))));
}
+ @Test
+ public void queryPluginDefinedAttributeChangeWithOption() throws Exception {
+ getChangeWithPluginDefinedBulkAttributeOption(
+ id -> pluginInfosFromList(adminSshSession.exec(changeQueryCmd(id))),
+ (id, opts) -> pluginInfosFromList(adminSshSession.exec(changeQueryCmd(id, opts))));
+ }
+
+ @Test
+ public void queryMultipleChangesWithPluginDefinedAttribute() throws Exception {
+ getMultipleChangesWithPluginDefinedBulkAttribute(
+ () -> pluginInfosFromList(adminSshSession.exec("gerrit query --format json status:open")));
+ }
+
+ @Test
+ public void queryChangesByCommitMessageWithPluginDefinedBulkAttribute() throws Exception {
+ getChangesByCommitMessageWithPluginDefinedBulkAttribute(
+ () -> pluginInfosFromList(adminSshSession.exec("gerrit query --format json status:open")));
+ }
+
+ @Test
+ public void getMultipleChangesWithPluginDefinedAndChangeAttributes() throws Exception {
+ getMultipleChangesWithPluginDefinedBulkAndChangeAttributes(
+ () -> pluginInfosFromList(adminSshSession.exec("gerrit query --format json status:open")));
+ }
+
+ @Test
+ public void getMultipleChangesWithPluginDefinedAttributeInSingleCall() throws Exception {
+ getMultipleChangesWithPluginDefinedBulkAttributeInSingleCall(
+ () -> pluginInfosFromList(adminSshSession.exec("gerrit query --format json status:open")));
+ }
+
+ @Test
+ public void getChangeWithPluginDefinedException() throws Exception {
+ getChangeWithPluginDefinedBulkAttributeWithException(
+ id -> pluginInfosFromList(adminSshSession.exec(changeQueryCmd(id))));
+ }
+
private String changeQueryCmd(Change.Id id) {
return changeQueryCmd(id, ImmutableListMultimap.of());
}
@@ -72,7 +116,22 @@
}
@Nullable
- private static List<MyInfo> pluginInfoFromSingletonList(String sshOutput) throws Exception {
+ private static List<PluginDefinedInfo> pluginInfoFromSingletonList(String sshOutput)
+ throws Exception {
+ List<Map<String, Object>> changeAttrs = getChangeAttrs(sshOutput);
+
+ assertThat(changeAttrs).hasSize(1);
+ return decodeRawPluginsList(GSON, changeAttrs.get(0).get("plugins"));
+ }
+
+ @Nullable
+ private static Map<Change.Id, List<PluginDefinedInfo>> pluginInfosFromList(String sshOutput)
+ throws Exception {
+ List<Map<String, Object>> changeAttrs = getChangeAttrs(sshOutput);
+ return getPluginInfosFromChangeInfos(GSON, changeAttrs);
+ }
+
+ private static List<Map<String, Object>> getChangeAttrs(String sshOutput) throws Exception {
List<Map<String, Object>> changeAttrs = new ArrayList<>();
for (String line : CharStreams.readLines(new StringReader(sshOutput))) {
Map<String, Object> changeAttr =
@@ -81,8 +140,6 @@
changeAttrs.add(changeAttr);
}
}
-
- assertThat(changeAttrs).hasSize(1);
- return decodeRawPluginsList(GSON, changeAttrs.get(0).get("plugins"));
+ return changeAttrs;
}
}
