Merge "Fix polygerrit_plugin rule to include deps"
diff --git a/.gitignore b/.gitignore
index 8cd43dd..c0e14ab 100644
--- a/.gitignore
+++ b/.gitignore
@@ -32,6 +32,18 @@
/local.properties
/node_modules/
/package-lock.json
-/plugins/cookbook-plugin/
+/plugins/*
+!/plugins/BUILD
+!/plugins/codemirror-editor
+!/plugins/commit-message-length-validator
+!/plugins/delete-project
+!/plugins/download-commands
+!/plugins/external_plugin_deps.bzl
+!/plugins/gitiles
+!/plugins/hooks
+!/plugins/replication
+!/plugins/reviewnotes
+!/plugins/singleusergroup
+!/plugins/webhooks
/test_site
/tools/format
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index f2bd273e..1d10025 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -860,55 +860,53 @@
----
[[query_attributes]]
-=== Query Attributes ===
+=== Change 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.
+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.
-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]
----
public class Module extends AbstractModule {
@Override
protected void configure() {
- bind(ChangeAttributeFactory.class)
- .annotatedWith(Exports.named("example"))
+ // Register attribute factory.
+ DynamicSet.bind(binder(), ChangeAttributeFactory.class)
.to(AttributeFactory.class);
+
+ // Register options for GET /changes/X/change and /changes/X/detail.
+ bind(DynamicBean.class)
+ .annotatedWith(Exports.named(GetChange.class))
+ .to(MyChangeOptions.class);
+
+ // Register options for GET /changes/?q=...
+ bind(DynamicBean.class)
+ .annotatedWith(Exports.named(QueryChanges.class))
+ .to(MyChangeOptions.class);
+
+ // Register options for ssh gerrit query.
+ bind(DynamicBean.class)
+ .annotatedWith(Exports.named(Query.class))
+ .to(MyChangeOptions.class);
}
}
-public class MyQueryOptions implements DynamicBean {
+public class MyChangeOptions implements DynamicBean {
@Option(name = "--all", usage = "Include plugin output")
public boolean all = false;
}
-public static class HttpModule extends HttpPluginModule {
- @Override
- protected void configureServlets() {
- bind(DynamicBean.class)
- .annotatedWith(Exports.named(QueryChanges.class))
- .to(MyQueryOptions.class);
- }
-}
-
-public static class SshModule extends PluginCommandModule {
- @Override
- protected void configureCommands() {
- bind(DynamicBean.class)
- .annotatedWith(Exports.named(Query.class))
- .to(MyQueryOptions.class);
- }
-}
-
public class AttributeFactory implements ChangeAttributeFactory {
- protected MyQueryOptions options;
+ protected MyChangeOptions options;
public class PluginAttribute extends PluginDefinedInfo {
public String exampleName;
@@ -921,9 +919,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 = (MyChangeOptions) bp.getDynamicBean(plugin);
}
if (options.all) {
return new PluginAttribute(c);
@@ -951,6 +949,23 @@
],
...
}
+
+curl http://localhost:8080/changes/1?myplugin-name--all
+
+Output:
+
+{
+ "_number": 1,
+ ...
+ "plugins": [
+ {
+ "name": "myplugin-name",
+ "example_name": "Attribute Example",
+ "change_value": "1"
+ }
+ ],
+ ...
+}
----
[[simple-configuration]]
@@ -1661,7 +1676,7 @@
[source,java]
----
-public class HttpModule extends HttpPluginModule {
+public class HttpModule extends ServletModule {
@Override
protected void configureServlets() {
DynamicSet.bind(binder(), WebUiPlugin.class)
@@ -2205,9 +2220,9 @@
/** Register the LfsApiServlet to listen on the default LFS protocol endpoint */
import static com.google.gerrit.httpd.plugins.LfsPluginServlet.URL_REGEX;
-import com.google.gerrit.httpd.plugins.HttpPluginModule;
+import com.google.inject.servlet.ServletModule;
-public class HttpModule extends HttpPluginModule {
+public class HttpModule extends ServletModule {
@Override
protected void configureServlets() {
diff --git a/WORKSPACE b/WORKSPACE
index a984c41..b4b1a64 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -156,18 +156,18 @@
sha1 = "94ad16d728b374d65bd897625f3fbb3da223a2b6",
)
-FLOGGER_VERS = "0.3.1"
+FLOGGER_VERS = "0.4"
maven_jar(
name = "flogger",
artifact = "com.google.flogger:flogger:" + FLOGGER_VERS,
- sha1 = "585030fe1ec709760cbef997a459729fb965df0e",
+ sha1 = "9c8863dcc913b56291c0c88e6d4ca9715b43df98",
)
maven_jar(
name = "flogger-log4j-backend",
artifact = "com.google.flogger:flogger-log4j-backend:" + FLOGGER_VERS,
- sha1 = "d5085e3996bddc4b105d53b886190cc9a8811a9e",
+ sha1 = "17aa5e31daa1354187e14b6978597d630391c028",
)
maven_jar(
@@ -1054,12 +1054,12 @@
sha1 = "76716d529710fc03d1d429b43e3cedd4419f78d4",
)
-# When upgrading elasticsearch-rest-client, also upgrade http-niocore
+# When upgrading elasticsearch-rest-client, also upgrade httpcore-nio
# and httpasyncclient as necessary.
maven_jar(
name = "elasticsearch-rest-client",
- artifact = "org.elasticsearch.client:elasticsearch-rest-client:6.6.1",
- sha1 = "dc1c9284ffca28cd169fae2776c3956e90b76c00",
+ artifact = "org.elasticsearch.client:elasticsearch-rest-client:6.6.2",
+ sha1 = "2c429141e488091c358aa43b1e6873d457464c5d",
)
JACKSON_VERSION = "2.9.8"
@@ -1070,18 +1070,18 @@
sha1 = "0f5a654e4675769c716e5b387830d19b501ca191",
)
-TESTCONTAINERS_VERSION = "1.10.3"
+TESTCONTAINERS_VERSION = "1.10.7"
maven_jar(
name = "testcontainers",
artifact = "org.testcontainers:testcontainers:" + TESTCONTAINERS_VERSION,
- sha1 = "e561ce99fc616b383d85f35ce881e58e8de59ae7",
+ sha1 = "e7575fedfd010ca1ad80c8c9bf971a8057b1ff8a",
)
maven_jar(
name = "testcontainers-elasticsearch",
artifact = "org.testcontainers:elasticsearch:" + TESTCONTAINERS_VERSION,
- sha1 = "0cb114ecba0ed54a116e2be2f031bc45ca4cbfc8",
+ sha1 = "1ee43ebd81aea1f29bf60a56643bad80c134f998",
)
maven_jar(
diff --git a/java/com/google/gerrit/acceptance/AbstractPluginFieldsTest.java b/java/com/google/gerrit/acceptance/AbstractPluginFieldsTest.java
new file mode 100644
index 0000000..ccd30ab
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/AbstractPluginFieldsTest.java
@@ -0,0 +1,201 @@
+// 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.acceptance;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.common.truth.Truth.assertThat;
+import static java.util.Objects.requireNonNull;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.collect.ImmutableListMultimap;
+import com.google.gerrit.common.Nullable;
+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.reviewdb.client.Change;
+import com.google.gerrit.server.DynamicOptions.DynamicBean;
+import com.google.gerrit.server.change.ChangeAttributeFactory;
+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.Module;
+import java.util.List;
+import java.util.Objects;
+import org.kohsuke.args4j.Option;
+
+public class AbstractPluginFieldsTest extends AbstractDaemonTest {
+ protected static class MyInfo extends PluginDefinedInfo {
+ @Nullable String theAttribute;
+
+ public MyInfo(@Nullable String theAttribute) {
+ this.theAttribute = theAttribute;
+ }
+
+ MyInfo(String name, @Nullable String theAttribute) {
+ this.name = requireNonNull(name);
+ this.theAttribute = theAttribute;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (!(o instanceof MyInfo)) {
+ return false;
+ }
+ MyInfo i = (MyInfo) o;
+ return Objects.equals(name, i.name) && Objects.equals(theAttribute, i.theAttribute);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(name, theAttribute);
+ }
+
+ @Override
+ public String toString() {
+ return MoreObjects.toStringHelper(this)
+ .add("name", name)
+ .add("theAttribute", theAttribute)
+ .toString();
+ }
+ }
+
+ protected static class NullAttributeModule extends AbstractModule {
+ @Override
+ public void configure() {
+ DynamicSet.bind(binder(), ChangeAttributeFactory.class).toInstance((cd, bp, p) -> null);
+ }
+ }
+
+ protected static class SimpleAttributeModule extends AbstractModule {
+ @Override
+ public void configure() {
+ DynamicSet.bind(binder(), ChangeAttributeFactory.class)
+ .toInstance((cd, bp, p) -> new MyInfo("change " + cd.getId()));
+ }
+ }
+
+ private static class MyOptions implements DynamicBean {
+ @Option(name = "--opt")
+ private String opt;
+ }
+
+ protected static class OptionAttributeModule extends AbstractModule {
+ @Override
+ public void configure() {
+ DynamicSet.bind(binder(), ChangeAttributeFactory.class)
+ .toInstance(
+ (cd, bp, p) -> {
+ MyOptions opts = (MyOptions) bp.getDynamicBean(p);
+ return opts != null ? new MyInfo("opt " + opts.opt) : null;
+ });
+ 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();
+
+ try (AutoCloseable ignored = installPlugin("my-plugin", NullAttributeModule.class)) {
+ assertThat(getter.call(id)).isNull();
+ }
+
+ assertThat(getter.call(id)).isNull();
+ }
+
+ protected void getChangeWithSimpleAttribute(PluginInfoGetter getter) throws Exception {
+ getChangeWithSimpleAttribute(getter, SimpleAttributeModule.class);
+ }
+
+ protected void getChangeWithSimpleAttribute(
+ PluginInfoGetter getter, Class<? extends Module> moduleClass) throws Exception {
+ Change.Id id = createChange().getChange().getId();
+ assertThat(getter.call(id)).isNull();
+
+ try (AutoCloseable ignored = installPlugin("my-plugin", moduleClass)) {
+ assertThat(getter.call(id)).containsExactly(new MyInfo("my-plugin", "change " + id));
+ }
+
+ assertThat(getter.call(id)).isNull();
+ }
+
+ protected void getChangeWithOption(
+ PluginInfoGetter getterWithoutOptions, PluginInfoGetterWithOptions getterWithOptions)
+ throws Exception {
+ Change.Id id = createChange().getChange().getId();
+ assertThat(getterWithoutOptions.call(id)).isNull();
+
+ try (AutoCloseable ignored = installPlugin("my-plugin", OptionAttributeModule.class)) {
+ assertThat(getterWithoutOptions.call(id))
+ .containsExactly(new MyInfo("my-plugin", "opt null"));
+ assertThat(getterWithOptions.call(id, ImmutableListMultimap.of("my-plugin--opt", "foo")))
+ .containsExactly(new MyInfo("my-plugin", "opt foo"));
+ }
+
+ assertThat(getterWithoutOptions.call(id)).isNull();
+ }
+
+ protected static List<MyInfo> pluginInfoFromSingletonList(List<ChangeInfo> changeInfos) {
+ assertThat(changeInfos).hasSize(1);
+ return pluginInfoFromChangeInfo(changeInfos.get(0));
+ }
+
+ protected static List<MyInfo> pluginInfoFromChangeInfo(ChangeInfo changeInfo) {
+ List<PluginDefinedInfo> pluginInfo = changeInfo.plugins;
+ if (pluginInfo == null) {
+ return null;
+ }
+ return pluginInfo.stream().map(MyInfo.class::cast).collect(toImmutableList());
+ }
+
+ /**
+ * Decode {@code MyInfo}s from a raw list of maps returned from Gson.
+ *
+ * <p>This method is used instead of decoding {@code ChangeInfo} or {@code ChangAttribute}, since
+ * Gson would decode the {@code plugins} field as a {@code List<PluginDefinedInfo>}, which would
+ * return the base type and silently ignore any fields that are defined only in the subclass.
+ * Instead, decode the enclosing {@code ChangeInfo} or {@code ChangeAttribute} as a raw {@code
+ * Map<String, Object>}, and pass the {@code "plugins"} value to this method.
+ *
+ * @param gson Gson converter.
+ * @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) {
+ if (plugins == null) {
+ return null;
+ }
+ checkArgument(plugins instanceof List, "not a list: %s", plugins);
+ return gson.fromJson(gson.toJson(plugins), new TypeToken<List<MyInfo>>() {}.getType());
+ }
+
+ @FunctionalInterface
+ protected interface PluginInfoGetter {
+ List<MyInfo> call(Change.Id id) throws Exception;
+ }
+
+ @FunctionalInterface
+ protected interface PluginInfoGetterWithOptions {
+ List<MyInfo> call(Change.Id id, ImmutableListMultimap<String, String> pluginOptions)
+ throws Exception;
+ }
+}
diff --git a/java/com/google/gerrit/acceptance/BUILD b/java/com/google/gerrit/acceptance/BUILD
index 5e59007..d61cf7a 100644
--- a/java/com/google/gerrit/acceptance/BUILD
+++ b/java/com/google/gerrit/acceptance/BUILD
@@ -115,6 +115,8 @@
"//java/com/google/gerrit/server/restapi",
"//java/com/google/gerrit/server/schema",
"//java/com/google/gerrit/server/util/time",
+ "//java/com/google/gerrit/sshd",
+ "//lib:args4j",
"//lib:gson",
"//lib:guava-retrying",
"//lib:gwtorm",
diff --git a/java/com/google/gerrit/extensions/api/changes/ChangeApi.java b/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
index 2a3bf07..47ccb49 100644
--- a/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
+++ b/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
@@ -14,6 +14,7 @@
package com.google.gerrit.extensions.api.changes;
+import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.Sets;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.client.ListChangesOption;
@@ -220,7 +221,18 @@
*/
List<ReviewerInfo> reviewers() throws RestApiException;
- ChangeInfo get(EnumSet<ListChangesOption> options) throws RestApiException;
+ ChangeInfo get(
+ EnumSet<ListChangesOption> options, ImmutableListMultimap<String, String> pluginOptions)
+ throws RestApiException;
+
+ default ChangeInfo get(ImmutableListMultimap<String, String> pluginOptions)
+ throws RestApiException {
+ return get(EnumSet.noneOf(ListChangesOption.class), pluginOptions);
+ }
+
+ default ChangeInfo get(EnumSet<ListChangesOption> options) throws RestApiException {
+ return get(options, ImmutableListMultimap.of());
+ }
default ChangeInfo get(Iterable<ListChangesOption> options) throws RestApiException {
return get(Sets.newEnumSet(options, ListChangesOption.class));
@@ -488,7 +500,9 @@
}
@Override
- public ChangeInfo get(EnumSet<ListChangesOption> options) throws RestApiException {
+ public ChangeInfo get(
+ EnumSet<ListChangesOption> options, ImmutableListMultimap<String, String> pluginOptions)
+ throws RestApiException {
throw new NotImplementedException();
}
diff --git a/java/com/google/gerrit/extensions/api/changes/Changes.java b/java/com/google/gerrit/extensions/api/changes/Changes.java
index 1bafc90..bcb49de1 100644
--- a/java/com/google/gerrit/extensions/api/changes/Changes.java
+++ b/java/com/google/gerrit/extensions/api/changes/Changes.java
@@ -14,6 +14,8 @@
package com.google.gerrit.extensions.api.changes;
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.ListMultimap;
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeInput;
@@ -77,6 +79,7 @@
private int start;
private boolean isNoLimit;
private EnumSet<ListChangesOption> options = EnumSet.noneOf(ListChangesOption.class);
+ private ListMultimap<String, String> pluginOptions = ArrayListMultimap.create();
public abstract List<ChangeInfo> get() throws RestApiException;
@@ -118,6 +121,18 @@
return this;
}
+ /** Set a plugin option on the request, appending to existing options. */
+ public QueryRequest withPluginOption(String name, String value) {
+ this.pluginOptions.put(name, value);
+ return this;
+ }
+
+ /** Set a plugin option on the request, replacing existing options. */
+ public QueryRequest withPluginOptions(ListMultimap<String, String> options) {
+ this.pluginOptions = ArrayListMultimap.create(options);
+ return this;
+ }
+
public String getQuery() {
return query;
}
@@ -138,6 +153,10 @@
return options;
}
+ public ListMultimap<String, String> getPluginOptions() {
+ return pluginOptions;
+ }
+
@Override
public String toString() {
StringBuilder sb = new StringBuilder(getClass().getSimpleName()).append('{').append(query);
diff --git a/java/com/google/gerrit/extensions/registration/Extension.java b/java/com/google/gerrit/extensions/registration/Extension.java
index aaec201..1031bb0 100644
--- a/java/com/google/gerrit/extensions/registration/Extension.java
+++ b/java/com/google/gerrit/extensions/registration/Extension.java
@@ -32,7 +32,7 @@
private final @Nullable String exportName;
private final Provider<T> provider;
- protected Extension(String pluginName, Provider<T> provider) {
+ public Extension(String pluginName, Provider<T> provider) {
this(pluginName, null, provider);
}
diff --git a/java/com/google/gerrit/httpd/plugins/HttpPluginModule.java b/java/com/google/gerrit/httpd/plugins/HttpPluginModule.java
index 937b24a..279903c 100644
--- a/java/com/google/gerrit/httpd/plugins/HttpPluginModule.java
+++ b/java/com/google/gerrit/httpd/plugins/HttpPluginModule.java
@@ -15,11 +15,9 @@
package com.google.gerrit.httpd.plugins;
import com.google.gerrit.extensions.api.lfs.LfsDefinitions;
-import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.httpd.resources.Resource;
import com.google.gerrit.httpd.resources.ResourceKey;
import com.google.gerrit.httpd.resources.ResourceWeigher;
-import com.google.gerrit.server.DynamicOptions;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.plugins.ModuleGenerator;
import com.google.gerrit.server.plugins.ReloadPluginListener;
@@ -65,7 +63,5 @@
.weigher(ResourceWeigher.class);
}
});
-
- DynamicMap.mapOf(binder(), DynamicOptions.DynamicBean.class);
}
}
diff --git a/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
index 2087104..b700835 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,8 +112,8 @@
bind(new TypeLiteral<List<CommentLinkInfo>>() {})
.toProvider(CommentLinkProvider.class)
.in(SINGLETON);
- bind(new TypeLiteral<DynamicMap<ChangeQueryProcessor.ChangeAttributeFactory>>() {})
- .toInstance(DynamicMap.emptyMap());
+ bind(new TypeLiteral<DynamicSet<ChangeAttributeFactory>>() {})
+ .toInstance(DynamicSet.emptySet());
bind(new TypeLiteral<DynamicMap<RestView<CommitResource>>>() {})
.toInstance(DynamicMap.emptyMap());
bind(String.class)
diff --git a/java/com/google/gerrit/server/DynamicOptions.java b/java/com/google/gerrit/server/DynamicOptions.java
index 11d3336..44d3493 100644
--- a/java/com/google/gerrit/server/DynamicOptions.java
+++ b/java/com/google/gerrit/server/DynamicOptions.java
@@ -153,6 +153,21 @@
*/
public interface BeanReceiver {
void setDynamicBean(String plugin, DynamicBean dynamicBean);
+
+ /**
+ * Returns the class that should be used for looking up exported DynamicBean bindings from
+ * plugins. Override when a particular REST/SSH endpoint should respect DynamicBeans bound on a
+ * different endpoint. For example, {@code GetDetail} is just a synonym for a variant of {@code
+ * GetChange}, and it should respect any DynamicBeans on GetChange. GetChange}. So it should
+ * return {@code GetChange.class} from this method.
+ */
+ default Class<? extends BeanReceiver> getExportedBeanReceiver() {
+ return getClass();
+ }
+ }
+
+ public interface BeanProvider {
+ DynamicBean getDynamicBean(String plugin);
}
/**
@@ -195,9 +210,13 @@
this.bean = bean;
this.injector = injector;
beansByPlugin = new HashMap<>();
+ Class<?> beanClass =
+ (bean instanceof BeanReceiver)
+ ? ((BeanReceiver) bean).getExportedBeanReceiver()
+ : getClass();
for (String plugin : dynamicBeans.plugins()) {
Provider<DynamicBean> provider =
- dynamicBeans.byPlugin(plugin).get(bean.getClass().getCanonicalName());
+ dynamicBeans.byPlugin(plugin).get(beanClass.getCanonicalName());
if (provider != null) {
beansByPlugin.put(plugin, getDynamicBean(bean, provider.get()));
}
diff --git a/java/com/google/gerrit/server/api/BUILD b/java/com/google/gerrit/server/api/BUILD
index 910ecd3..d4f11f0 100644
--- a/java/com/google/gerrit/server/api/BUILD
+++ b/java/com/google/gerrit/server/api/BUILD
@@ -12,6 +12,8 @@
"//java/com/google/gerrit/reviewdb:server",
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/restapi",
+ "//java/com/google/gerrit/util/cli",
+ "//lib:args4j",
"//lib:guava",
"//lib:gwtorm",
"//lib:servlet-api-3_1",
diff --git a/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java b/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
index ad39633..6763f51 100644
--- a/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
+++ b/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
@@ -16,6 +16,8 @@
import static com.google.gerrit.server.api.ApiUtil.asRestApiException;
+import com.google.common.collect.ImmutableListMultimap;
+import com.google.common.collect.ListMultimap;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.api.changes.AbandonInput;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
@@ -49,12 +51,14 @@
import com.google.gerrit.extensions.common.PureRevertInfo;
import com.google.gerrit.extensions.common.RobotCommentInfo;
import com.google.gerrit.extensions.common.SuggestedReviewerInfo;
+import com.google.gerrit.extensions.registration.DynamicMap;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.server.DynamicOptions;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.StarredChangesUtil.IllegalLabelException;
-import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.change.ChangeMessageResource;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.SetPrivateOp;
@@ -68,6 +72,7 @@
import com.google.gerrit.server.restapi.change.DeleteChange;
import com.google.gerrit.server.restapi.change.DeletePrivate;
import com.google.gerrit.server.restapi.change.GetAssignee;
+import com.google.gerrit.server.restapi.change.GetChange;
import com.google.gerrit.server.restapi.change.GetHashtags;
import com.google.gerrit.server.restapi.change.GetPastAssignees;
import com.google.gerrit.server.restapi.change.GetPureRevert;
@@ -97,14 +102,18 @@
import com.google.gerrit.server.restapi.change.SubmittedTogether;
import com.google.gerrit.server.restapi.change.SuggestChangeReviewers;
import com.google.gerrit.server.restapi.change.Unignore;
+import com.google.gerrit.util.cli.CmdLineParser;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
+import com.google.inject.Injector;
import com.google.inject.Provider;
+import com.google.inject.Singleton;
import com.google.inject.assistedinject.Assisted;
import java.util.EnumSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
+import org.kohsuke.args4j.CmdLineException;
class ChangeApiImpl implements ChangeApi {
interface Factory {
@@ -132,7 +141,7 @@
private final PutTopic putTopic;
private final ChangeIncludedIn includedIn;
private final PostReviewers postReviewers;
- private final ChangeJson.Factory changeJson;
+ private final Provider<GetChange> getChangeProvider;
private final PostHashtags postHashtags;
private final GetHashtags getHashtags;
private final PutAssignee putAssignee;
@@ -157,6 +166,7 @@
private final PutMessage putMessage;
private final Provider<GetPureRevert> getPureRevertProvider;
private final StarredChangesUtil stars;
+ private final DynamicOptionParser dynamicOptionParser;
@Inject
ChangeApiImpl(
@@ -180,7 +190,7 @@
PutTopic putTopic,
ChangeIncludedIn includedIn,
PostReviewers postReviewers,
- ChangeJson.Factory changeJson,
+ Provider<GetChange> getChangeProvider,
PostHashtags postHashtags,
GetHashtags getHashtags,
PutAssignee putAssignee,
@@ -205,6 +215,7 @@
PutMessage putMessage,
Provider<GetPureRevert> getPureRevertProvider,
StarredChangesUtil stars,
+ DynamicOptionParser dynamicOptionParser,
@Assisted ChangeResource change) {
this.changeApi = changeApi;
this.revert = revert;
@@ -226,7 +237,7 @@
this.putTopic = putTopic;
this.includedIn = includedIn;
this.postReviewers = postReviewers;
- this.changeJson = changeJson;
+ this.getChangeProvider = getChangeProvider;
this.postHashtags = postHashtags;
this.getHashtags = getHashtags;
this.putAssignee = putAssignee;
@@ -251,6 +262,7 @@
this.putMessage = putMessage;
this.getPureRevertProvider = getPureRevertProvider;
this.stars = stars;
+ this.dynamicOptionParser = dynamicOptionParser;
this.change = change;
}
@@ -452,9 +464,14 @@
}
@Override
- public ChangeInfo get(EnumSet<ListChangesOption> s) throws RestApiException {
+ public ChangeInfo get(
+ EnumSet<ListChangesOption> options, ImmutableListMultimap<String, String> pluginOptions)
+ throws RestApiException {
try {
- return changeJson.create(s).format(change);
+ GetChange getChange = getChangeProvider.get();
+ options.forEach(getChange::addOption);
+ dynamicOptionParser.parseDynamicOptions(getChange, pluginOptions);
+ return getChange.apply(change).value();
} catch (Exception e) {
throw asRestApiException("Cannot retrieve change", e);
}
@@ -660,4 +677,36 @@
throw asRestApiException("Cannot parse change message " + id, e);
}
}
+
+ @Singleton
+ static class DynamicOptionParser {
+ private final CmdLineParser.Factory cmdLineParserFactory;
+ private final Injector injector;
+ private final DynamicMap<DynamicOptions.DynamicBean> dynamicBeans;
+
+ @Inject
+ DynamicOptionParser(
+ CmdLineParser.Factory cmdLineParserFactory,
+ Injector injector,
+ DynamicMap<DynamicOptions.DynamicBean> dynamicBeans) {
+ this.cmdLineParserFactory = cmdLineParserFactory;
+ this.injector = injector;
+ this.dynamicBeans = dynamicBeans;
+ }
+
+ void parseDynamicOptions(Object bean, ListMultimap<String, String> pluginOptions)
+ throws BadRequestException {
+ CmdLineParser clp = cmdLineParserFactory.create(bean);
+ DynamicOptions dynamicOptions = new DynamicOptions(bean, injector, dynamicBeans);
+ dynamicOptions.parseDynamicBeans(clp);
+ dynamicOptions.setDynamicBeans();
+ dynamicOptions.onBeanParseStart();
+ try {
+ clp.parseOptionMap(pluginOptions);
+ } catch (CmdLineException | NumberFormatException e) {
+ throw new BadRequestException(e.getMessage(), e);
+ }
+ dynamicOptions.onBeanParseEnd();
+ }
+ }
}
diff --git a/java/com/google/gerrit/server/api/changes/ChangesImpl.java b/java/com/google/gerrit/server/api/changes/ChangesImpl.java
index 0fdaeaa..acff137 100644
--- a/java/com/google/gerrit/server/api/changes/ChangesImpl.java
+++ b/java/com/google/gerrit/server/api/changes/ChangesImpl.java
@@ -30,6 +30,7 @@
import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.server.api.changes.ChangeApiImpl.DynamicOptionParser;
import com.google.gerrit.server.restapi.change.ChangesCollection;
import com.google.gerrit.server.restapi.change.CreateChange;
import com.google.gerrit.server.restapi.change.QueryChanges;
@@ -43,6 +44,7 @@
private final ChangesCollection changes;
private final ChangeApiImpl.Factory api;
private final CreateChange createChange;
+ private final DynamicOptionParser dynamicOptionParser;
private final Provider<QueryChanges> queryProvider;
@Inject
@@ -50,10 +52,12 @@
ChangesCollection changes,
ChangeApiImpl.Factory api,
CreateChange createChange,
+ DynamicOptionParser dynamicOptionParser,
Provider<QueryChanges> queryProvider) {
this.changes = changes;
this.api = api;
this.createChange = createChange;
+ this.dynamicOptionParser = dynamicOptionParser;
this.queryProvider = queryProvider;
}
@@ -120,6 +124,7 @@
for (ListChangesOption option : q.getOptions()) {
qc.addOption(option);
}
+ dynamicOptionParser.parseDynamicOptions(qc, q.getPluginOptions());
try {
List<?> result = qc.apply(TopLevelResource.INSTANCE);
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..95355cf
--- /dev/null
+++ b/java/com/google/gerrit/server/change/ChangeAttributeFactory.java
@@ -0,0 +1,48 @@
+// 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>
+ * DynamicSet.bind(binder(), ChangeAttributeFactory.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 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/change/PluginDefinedAttributesFactories.java b/java/com/google/gerrit/server/change/PluginDefinedAttributesFactories.java
new file mode 100644
index 0000000..9928125
--- /dev/null
+++ b/java/com/google/gerrit/server/change/PluginDefinedAttributesFactories.java
@@ -0,0 +1,64 @@
+// 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 static com.google.common.collect.ImmutableList.toImmutableList;
+import static java.util.concurrent.TimeUnit.MINUTES;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
+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.Objects;
+import java.util.stream.Stream;
+
+/** Static helpers for use by {@link PluginDefinedAttributesFactory} implementations. */
+public class PluginDefinedAttributesFactories {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ @Nullable
+ public static ImmutableList<PluginDefinedInfo> createAll(
+ ChangeData cd,
+ BeanProvider beanProvider,
+ Stream<Extension<ChangeAttributeFactory>> attrFactories) {
+ ImmutableList<PluginDefinedInfo> result =
+ attrFactories
+ .map(e -> tryCreate(cd, beanProvider, e.getPluginName(), e.get()))
+ .filter(Objects::nonNull)
+ .collect(toImmutableList());
+ return !result.isEmpty() ? result : null;
+ }
+
+ @Nullable
+ private static PluginDefinedInfo tryCreate(
+ ChangeData cd, BeanProvider beanProvider, String plugin, ChangeAttributeFactory attrFactory) {
+ PluginDefinedInfo pdi = null;
+ try {
+ pdi = attrFactory.create(cd, beanProvider, plugin);
+ } catch (RuntimeException ex) {
+ logger.atWarning().atMostEvery(1, MINUTES).withCause(ex).log(
+ "error populating attribute on change %s from plugin %s", cd.getId(), plugin);
+ }
+ if (pdi != null) {
+ pdi.name = plugin;
+ }
+ return pdi;
+ }
+
+ private PluginDefinedAttributesFactories() {}
+}
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..e7baa55 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -76,6 +76,7 @@
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.CmdLineParserModule;
import com.google.gerrit.server.CreateGroupPermissionSyncer;
+import com.google.gerrit.server.DynamicOptions;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountCacheImpl;
import com.google.gerrit.server.account.AccountControl;
@@ -97,6 +98,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 +169,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;
@@ -403,9 +404,10 @@
factory(UploadValidators.Factory.class);
DynamicSet.setOf(binder(), UploadValidationListener.class);
+ DynamicMap.mapOf(binder(), DynamicOptions.DynamicBean.class);
DynamicMap.mapOf(binder(), ChangeQueryBuilder.ChangeOperatorFactory.class);
DynamicMap.mapOf(binder(), ChangeQueryBuilder.ChangeHasOperandFactory.class);
- DynamicMap.mapOf(binder(), ChangeQueryProcessor.ChangeAttributeFactory.class);
+ DynamicSet.setOf(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..f9263a9 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
@@ -16,12 +16,12 @@
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.ImmutableList;
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.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.registration.Extension;
import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.index.QueryOptions;
import com.google.gerrit.index.query.IndexPredicate;
@@ -33,6 +33,9 @@
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.PluginDefinedAttributesFactories;
+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;
@@ -42,9 +45,7 @@
import com.google.gerrit.server.project.ProjectCache;
import com.google.inject.Inject;
import com.google.inject.Provider;
-import java.util.ArrayList;
import java.util.HashMap;
-import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -55,19 +56,7 @@
* holding on to a single instance.
*/
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:
- *
- * <p>bind(ChangeAttributeFactory.class) .annotatedWith(Exports.named("export-name"))
- * .to(YourClass.class);
- */
- public interface ChangeAttributeFactory {
- PluginDefinedInfo create(ChangeData a, ChangeQueryProcessor qp, String plugin);
- }
-
+ implements DynamicOptions.BeanReceiver, DynamicOptions.BeanProvider {
private final Provider<CurrentUser> userProvider;
private final ChangeNotes.Factory notesFactory;
private final ImmutableListMultimap<String, ChangeAttributeFactory> attributeFactoriesByPlugin;
@@ -92,7 +81,7 @@
ChangeIndexCollection indexes,
ChangeIndexRewriter rewriter,
ChangeNotes.Factory notesFactory,
- DynamicMap<ChangeAttributeFactory> attributeFactories,
+ DynamicSet<ChangeAttributeFactory> attributeFactories,
PermissionBackend permissionBackend,
ProjectCache projectCache,
Provider<AnonymousUser> anonymousUserProvider) {
@@ -114,7 +103,7 @@
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()));
+ attributeFactories.entries().forEach(e -> factoriesBuilder.put(e.getPluginName(), e.get()));
attributeFactoriesByPlugin = factoriesBuilder.build();
}
@@ -135,32 +124,21 @@
dynamicBeans.put(plugin, dynamicBean);
}
+ @Override
public DynamicBean getDynamicBean(String plugin) {
return dynamicBeans.get(plugin);
}
- @Override
- public List<PluginDefinedInfo> create(ChangeData cd) {
- 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) {
- // 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;
- plugins.add(pda);
- }
- }
- if (plugins.isEmpty()) {
- plugins = null;
- }
- return plugins;
+ public PluginDefinedAttributesFactory getAttributesFactory() {
+ return this::buildPluginInfo;
+ }
+
+ private ImmutableList<PluginDefinedInfo> buildPluginInfo(ChangeData cd) {
+ return PluginDefinedAttributesFactories.createAll(
+ cd,
+ this,
+ attributeFactoriesByPlugin.entries().stream()
+ .map(e -> new Extension<>(e.getKey(), e::getValue)));
}
@Override
diff --git a/java/com/google/gerrit/server/query/change/OutputStreamQuery.java b/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
index ba11db9..09ff687 100644
--- a/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
+++ b/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
@@ -75,6 +75,8 @@
JSON
}
+ public static final Gson GSON = new Gson();
+
private final GitRepositoryManager repoManager;
private final ChangeQueryBuilder queryBuilder;
private final ChangeQueryProcessor queryProcessor;
@@ -322,7 +324,7 @@
eventFactory.addDependencies(rw, c, d.change(), d.currentPatchSet());
}
- c.plugins = queryProcessor.create(d);
+ c.plugins = queryProcessor.getAttributesFactory().create(d);
return c;
}
@@ -355,7 +357,7 @@
break;
case JSON:
- out.print(new Gson().toJson(data));
+ out.print(GSON.toJson(data));
out.print('\n');
break;
}
diff --git a/java/com/google/gerrit/server/restapi/change/GetChange.java b/java/com/google/gerrit/server/restapi/change/GetChange.java
index 2decb4e..c8641d5 100644
--- a/java/com/google/gerrit/server/restapi/change/GetChange.java
+++ b/java/com/google/gerrit/server/restapi/change/GetChange.java
@@ -14,25 +14,41 @@
package com.google.gerrit.server.restapi.change;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Streams;
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.client.ListOption;
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.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
+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.ChangeJson;
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.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.util.EnumSet;
+import java.util.HashMap;
+import java.util.Map;
import org.kohsuke.args4j.Option;
-public class GetChange implements RestReadView<ChangeResource> {
+public class GetChange
+ implements RestReadView<ChangeResource>,
+ DynamicOptions.BeanReceiver,
+ DynamicOptions.BeanProvider {
private final ChangeJson.Factory json;
+ private final DynamicSet<ChangeAttributeFactory> attrFactories;
private final EnumSet<ListChangesOption> options = EnumSet.noneOf(ListChangesOption.class);
+ private final Map<String, DynamicBean> dynamicBeans = new HashMap<>();
@Option(name = "-o", usage = "Output options")
- void addOption(ListChangesOption o) {
+ public void addOption(ListChangesOption o) {
options.add(o);
}
@@ -42,16 +58,36 @@
}
@Inject
- GetChange(ChangeJson.Factory json) {
+ GetChange(ChangeJson.Factory json, DynamicSet<ChangeAttributeFactory> attrFactories) {
this.json = json;
+ this.attrFactories = attrFactories;
+ }
+
+ @Override
+ public void setDynamicBean(String plugin, DynamicOptions.DynamicBean dynamicBean) {
+ dynamicBeans.put(plugin, dynamicBean);
+ }
+
+ @Override
+ public DynamicBean getDynamicBean(String plugin) {
+ return dynamicBeans.get(plugin);
}
@Override
public Response<ChangeInfo> apply(ChangeResource rsrc) throws OrmException {
- return Response.withMustRevalidate(json.create(options).format(rsrc));
+ return Response.withMustRevalidate(newChangeJson().format(rsrc));
}
Response<ChangeInfo> apply(RevisionResource rsrc) throws OrmException {
- return Response.withMustRevalidate(json.create(options).format(rsrc));
+ return Response.withMustRevalidate(newChangeJson().format(rsrc));
+ }
+
+ private ChangeJson newChangeJson() {
+ return json.create(options, this::buildPluginInfo);
+ }
+
+ private ImmutableList<PluginDefinedInfo> buildPluginInfo(ChangeData cd) {
+ return PluginDefinedAttributesFactories.createAll(
+ cd, this, Streams.stream(attrFactories.entries()));
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/GetDetail.java b/java/com/google/gerrit/server/restapi/change/GetDetail.java
index ab75ab7..06fdd99 100644
--- a/java/com/google/gerrit/server/restapi/change/GetDetail.java
+++ b/java/com/google/gerrit/server/restapi/change/GetDetail.java
@@ -18,12 +18,14 @@
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.server.DynamicOptions;
+import com.google.gerrit.server.DynamicOptions.DynamicBean;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import org.kohsuke.args4j.Option;
-public class GetDetail implements RestReadView<ChangeResource> {
+public class GetDetail implements RestReadView<ChangeResource>, DynamicOptions.BeanReceiver {
private final GetChange delegate;
@Option(name = "-o", usage = "Output options")
@@ -47,6 +49,16 @@
}
@Override
+ public void setDynamicBean(String plugin, DynamicBean dynamicBean) {
+ delegate.setDynamicBean(plugin, dynamicBean);
+ }
+
+ @Override
+ public Class<? extends DynamicOptions.BeanReceiver> getExportedBeanReceiver() {
+ return delegate.getExportedBeanReceiver();
+ }
+
+ @Override
public Response<ChangeInfo> apply(ChangeResource rsrc) throws OrmException {
return delegate.apply(rsrc);
}
diff --git a/java/com/google/gerrit/server/restapi/change/QueryChanges.java b/java/com/google/gerrit/server/restapi/change/QueryChanges.java
index 23b3011..6176ea0 100644
--- a/java/com/google/gerrit/server/restapi/change/QueryChanges.java
+++ b/java/com/google/gerrit/server/restapi/change/QueryChanges.java
@@ -143,7 +143,8 @@
int cnt = queries.size();
List<QueryResult<ChangeData>> results = imp.query(qb.parse(queries));
- List<List<ChangeInfo>> res = json.create(options, this.imp).format(results);
+ List<List<ChangeInfo>> res =
+ json.create(options, this.imp.getAttributesFactory()).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/java/com/google/gerrit/sshd/SshModule.java b/java/com/google/gerrit/sshd/SshModule.java
index 1de14b6..c5f190e 100644
--- a/java/com/google/gerrit/sshd/SshModule.java
+++ b/java/com/google/gerrit/sshd/SshModule.java
@@ -20,10 +20,8 @@
import com.google.common.base.CharMatcher;
import com.google.common.base.Splitter;
import com.google.gerrit.extensions.registration.DynamicItem;
-import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.lifecycle.LifecycleModule;
-import com.google.gerrit.server.DynamicOptions;
import com.google.gerrit.server.PeerDaemonUser;
import com.google.gerrit.server.RemotePeer;
import com.google.gerrit.server.config.GerritConfigListener;
@@ -102,7 +100,6 @@
.annotatedWith(UniqueAnnotations.create())
.to(SshPluginStarterCallback.class);
- DynamicMap.mapOf(binder(), DynamicOptions.DynamicBean.class);
DynamicItem.itemOf(binder(), SshCreateCommandInterceptor.class);
listener().toInstance(registerInParentInjectors());
diff --git a/java/com/google/gerrit/util/cli/CmdLineParser.java b/java/com/google/gerrit/util/cli/CmdLineParser.java
index 555abc3..771451f 100644
--- a/java/com/google/gerrit/util/cli/CmdLineParser.java
+++ b/java/com/google/gerrit/util/cli/CmdLineParser.java
@@ -34,6 +34,7 @@
package com.google.gerrit.util.cli;
+import static com.google.common.base.Preconditions.checkArgument;
import static com.google.gerrit.util.cli.Localizable.localizable;
import com.google.common.base.Strings;
@@ -426,6 +427,7 @@
PrefixedOption(String prefix, Option o) {
this.prefix = prefix;
+ checkArgument(o.name().startsWith("-"), "Option name must start with '-': %s", o);
this.o = o;
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index d7083bc..9cc277f 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -52,7 +52,6 @@
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toSet;
-import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -74,10 +73,8 @@
import com.google.gerrit.common.data.LabelFunction;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.Permission;
-import com.google.gerrit.extensions.annotations.Exports;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.AddReviewerResult;
-import com.google.gerrit.extensions.api.changes.Changes.QueryRequest;
import com.google.gerrit.extensions.api.changes.DeleteReviewerInput;
import com.google.gerrit.extensions.api.changes.DeleteVoteInput;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
@@ -117,7 +114,6 @@
import com.google.gerrit.extensions.common.LabelInfo;
import com.google.gerrit.extensions.common.MergeInput;
import com.google.gerrit.extensions.common.MergePatchSetInput;
-import com.google.gerrit.extensions.common.PluginDefinedInfo;
import com.google.gerrit.extensions.common.PureRevertInfo;
import com.google.gerrit.extensions.common.RevisionInfo;
import com.google.gerrit.extensions.common.TrackingIdInfo;
@@ -150,7 +146,6 @@
import com.google.gerrit.server.index.change.IndexedChangeQuery;
import com.google.gerrit.server.project.testing.Util;
import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.query.change.ChangeQueryProcessor.ChangeAttributeFactory;
import com.google.gerrit.server.restapi.change.PostReview;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateOp;
@@ -158,15 +153,12 @@
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.gerrit.testing.FakeEmailSender.Message;
import com.google.gerrit.testing.TestTimeUtil;
-import com.google.gwtorm.server.OrmException;
-import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import java.io.IOException;
import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
-import java.util.EnumSet;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
@@ -2499,67 +2491,6 @@
assertThat(query("owner:self project:{" + project.get() + "}")).isEmpty();
}
- static class CustomAttributeModule extends AbstractModule {
- static class MyInfo extends PluginDefinedInfo {
- public String theAttribute;
- }
-
- @Override
- public void configure() {
- bind(ChangeAttributeFactory.class)
- .annotatedWith(Exports.named("my-attr"))
- .toInstance(
- (cd, qp, plugin) -> {
- try {
- String topic = cd.change().getTopic();
- if (Strings.isNullOrEmpty(topic)) {
- return null;
- }
- MyInfo myInfo = new MyInfo();
- myInfo.theAttribute = "topic: " + topic;
- return myInfo;
- } catch (OrmException e) {
- throw new RuntimeException(e);
- }
- });
- }
- }
-
- @Test
- public void queryChangeWithCustomAttributes() throws Exception {
- Change.Id id1 = createChange().getChange().getId();
- Change.Id id2 = createChange().getChange().getId();
- gApi.changes().id(id2.get()).topic("foo");
-
- Function<Change.Id, QueryRequest> byQuery =
- id ->
- gApi.changes().query(id.toString()).withOptions(EnumSet.allOf(ListChangesOption.class));
- assertThat(gApi.changes().id(id1.get()).get().plugins).isNull();
- assertThat(gApi.changes().id(id2.get()).get().plugins).isNull();
- assertThat(Iterables.getOnlyElement(byQuery.apply(id1).get()).plugins).isNull();
- assertThat(Iterables.getOnlyElement(byQuery.apply(id2).get()).plugins).isNull();
-
- try (AutoCloseable ignored = installPlugin("my-plugin", CustomAttributeModule.class)) {
- // Plugin attributes only set in queries.
- assertThat(gApi.changes().id(id1.get()).get().plugins).isNull();
- assertThat(gApi.changes().id(id2.get()).get().plugins).isNull();
-
- assertThat(Iterables.getOnlyElement(byQuery.apply(id1).get()).plugins).isNull();
- List<PluginDefinedInfo> pluginInfos =
- Iterables.getOnlyElement(byQuery.apply(id2).get()).plugins;
- assertThat(pluginInfos).hasSize(1);
- assertThat(pluginInfos.get(0)).isInstanceOf(CustomAttributeModule.MyInfo.class);
- CustomAttributeModule.MyInfo myInfo = (CustomAttributeModule.MyInfo) pluginInfos.get(0);
- assertThat(myInfo.name).isEqualTo("my-plugin");
- assertThat(myInfo.theAttribute).isEqualTo("topic: foo");
- }
-
- assertThat(gApi.changes().id(id1.get()).get().plugins).isNull();
- assertThat(gApi.changes().id(id2.get()).get().plugins).isNull();
- assertThat(Iterables.getOnlyElement(byQuery.apply(id1).get()).plugins).isNull();
- assertThat(Iterables.getOnlyElement(byQuery.apply(id2).get()).plugins).isNull();
- }
-
@Test
public void checkReviewedFlagBeforeAndAfterReview() throws Exception {
PushOneCommit.Result r = createChange();
diff --git a/javatests/com/google/gerrit/acceptance/api/change/PluginFieldsIT.java b/javatests/com/google/gerrit/acceptance/api/change/PluginFieldsIT.java
new file mode 100644
index 0000000..d5089ff
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/api/change/PluginFieldsIT.java
@@ -0,0 +1,86 @@
+// 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.acceptance.api.change;
+
+import com.google.gerrit.acceptance.AbstractPluginFieldsTest;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.extensions.annotations.Exports;
+import com.google.gerrit.server.change.ChangeAttributeFactory;
+import com.google.inject.AbstractModule;
+import org.junit.Test;
+
+@NoHttpd
+public class PluginFieldsIT extends AbstractPluginFieldsTest {
+ // No tests for /detail via the extension API, since the extension API doesn't have that method.
+
+ @Test
+ public void queryChangeWithNullAttribute() throws Exception {
+ getChangeWithNullAttribute(
+ id -> pluginInfoFromSingletonList(gApi.changes().query(id.toString()).get()));
+ }
+
+ @Test
+ public void getChangeWithNullAttribute() throws Exception {
+ getChangeWithNullAttribute(
+ id -> pluginInfoFromChangeInfo(gApi.changes().id(id.toString()).get()));
+ }
+
+ @Test
+ public void queryChangeWithSimpleAttribute() throws Exception {
+ getChangeWithSimpleAttribute(
+ id -> pluginInfoFromSingletonList(gApi.changes().query(id.toString()).get()));
+ }
+
+ @Test
+ public void getChangeWithSimpleAttribute() throws Exception {
+ getChangeWithSimpleAttribute(
+ id -> pluginInfoFromChangeInfo(gApi.changes().id(id.toString()).get()));
+ }
+
+ @Test
+ public void queryChangeWithOption() throws Exception {
+ getChangeWithOption(
+ id -> pluginInfoFromSingletonList(gApi.changes().query(id.toString()).get()),
+ (id, opts) ->
+ pluginInfoFromSingletonList(
+ gApi.changes().query(id.toString()).withPluginOptions(opts).get()));
+ }
+
+ @Test
+ public void getChangeWithOption() throws Exception {
+ getChangeWithOption(
+ id -> pluginInfoFromChangeInfo(gApi.changes().id(id.get()).get()),
+ (id, opts) -> pluginInfoFromChangeInfo(gApi.changes().id(id.get()).get(opts)));
+ }
+
+ static class SimpleAttributeWithExplicitExportModule extends AbstractModule {
+ @Override
+ public void configure() {
+ bind(ChangeAttributeFactory.class)
+ .annotatedWith(Exports.named("simple"))
+ .toInstance((cd, bp, p) -> new MyInfo("change " + cd.getId()));
+ }
+ }
+
+ @Test
+ public void getChangeWithSimpleAttributeWithExplicitExport() throws Exception {
+ // For backwards compatibility with old plugins, allow modules to bind into the
+ // DynamicSet<ChangeAttributeFactory> as if it were a DynamicMap. We only need one variant of
+ // this test to prove that the mapping works.
+ getChangeWithSimpleAttribute(
+ id -> pluginInfoFromChangeInfo(gApi.changes().id(id.toString()).get()),
+ SimpleAttributeWithExplicitExportModule.class);
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/PluginFieldsIT.java b/javatests/com/google/gerrit/acceptance/rest/change/PluginFieldsIT.java
new file mode 100644
index 0000000..649c7ae
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/rest/change/PluginFieldsIT.java
@@ -0,0 +1,151 @@
+// 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.acceptance.rest.change;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableListMultimap;
+import com.google.gerrit.acceptance.AbstractPluginFieldsTest;
+import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.json.OutputFormat;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gson.Gson;
+import com.google.gson.reflect.TypeToken;
+import java.util.List;
+import java.util.Map;
+import org.junit.Test;
+
+public class PluginFieldsIT extends AbstractPluginFieldsTest {
+ private static final Gson GSON = OutputFormat.JSON.newGson();
+
+ @Test
+ public void queryChangeWithNullAttribute() throws Exception {
+ getChangeWithNullAttribute(
+ id -> pluginInfoFromSingletonList(adminRestSession.get(changeQueryUrl(id))));
+ }
+
+ @Test
+ public void getChangeWithNullAttribute() throws Exception {
+ getChangeWithNullAttribute(id -> pluginInfoFromChangeInfo(adminRestSession.get(changeUrl(id))));
+ }
+
+ @Test
+ public void getChangeDetailWithNullAttribute() throws Exception {
+ getChangeWithNullAttribute(
+ id -> pluginInfoFromChangeInfo(adminRestSession.get(changeDetailUrl(id))));
+ }
+
+ @Test
+ public void queryChangeWithSimpleAttribute() throws Exception {
+ getChangeWithSimpleAttribute(
+ id -> pluginInfoFromSingletonList(adminRestSession.get(changeQueryUrl(id))));
+ }
+
+ @Test
+ public void getChangeWithSimpleAttribute() throws Exception {
+ getChangeWithSimpleAttribute(
+ id -> pluginInfoFromChangeInfo(adminRestSession.get(changeUrl(id))));
+ }
+
+ @Test
+ public void getChangeDetailWithSimpleAttribute() throws Exception {
+ getChangeWithSimpleAttribute(
+ id -> pluginInfoFromChangeInfo(adminRestSession.get(changeDetailUrl(id))));
+ }
+
+ @Test
+ public void queryChangeWithOption() throws Exception {
+ getChangeWithOption(
+ id -> pluginInfoFromSingletonList(adminRestSession.get(changeQueryUrl(id))),
+ (id, opts) -> pluginInfoFromSingletonList(adminRestSession.get(changeQueryUrl(id, opts))));
+ }
+
+ @Test
+ public void getChangeWithOption() throws Exception {
+ getChangeWithOption(
+ id -> pluginInfoFromChangeInfo(adminRestSession.get(changeUrl(id))),
+ (id, opts) -> pluginInfoFromChangeInfo(adminRestSession.get(changeUrl(id, opts))));
+ }
+
+ @Test
+ public void getChangeDetailWithOption() throws Exception {
+ getChangeWithOption(
+ id -> pluginInfoFromChangeInfo(adminRestSession.get(changeDetailUrl(id))),
+ (id, opts) -> pluginInfoFromChangeInfo(adminRestSession.get(changeDetailUrl(id, opts))));
+ }
+
+ private String changeQueryUrl(Change.Id id) {
+ return changeQueryUrl(id, ImmutableListMultimap.of());
+ }
+
+ private String changeQueryUrl(Change.Id id, ImmutableListMultimap<String, String> opts) {
+ String url = "/changes/?q=" + id;
+ String queryString = buildQueryString(opts);
+ if (!queryString.isEmpty()) {
+ url += "&" + queryString;
+ }
+ return url;
+ }
+
+ private String changeUrl(Change.Id id) {
+ return changeUrl(id, ImmutableListMultimap.of());
+ }
+
+ private String changeUrl(Change.Id id, ImmutableListMultimap<String, String> pluginOptions) {
+ return changeUrl(id, "", pluginOptions);
+ }
+
+ private String changeDetailUrl(Change.Id id) {
+ return changeDetailUrl(id, ImmutableListMultimap.of());
+ }
+
+ private String changeDetailUrl(
+ Change.Id id, ImmutableListMultimap<String, String> pluginOptions) {
+ return changeUrl(id, "/detail", pluginOptions);
+ }
+
+ private String changeUrl(
+ Change.Id id, String suffix, ImmutableListMultimap<String, String> pluginOptions) {
+ String url = "/changes/" + project + "~" + id + suffix;
+ String queryString = buildQueryString(pluginOptions);
+ if (!queryString.isEmpty()) {
+ url += "?" + queryString;
+ }
+ return url;
+ }
+
+ private static String buildQueryString(ImmutableListMultimap<String, String> opts) {
+ return Joiner.on('&').withKeyValueSeparator('=').join(opts.entries());
+ }
+
+ @Nullable
+ private static List<MyInfo> pluginInfoFromSingletonList(RestResponse res) throws Exception {
+ res.assertOK();
+ List<Map<String, Object>> changeInfos =
+ GSON.fromJson(res.getReader(), new TypeToken<List<Map<String, Object>>>() {}.getType());
+ assertThat(changeInfos).hasSize(1);
+ return decodeRawPluginsList(GSON, changeInfos.get(0).get("plugins"));
+ }
+
+ @Nullable
+ private List<MyInfo> 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"));
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/ssh/PluginChangeFieldsIT.java b/javatests/com/google/gerrit/acceptance/ssh/PluginChangeFieldsIT.java
new file mode 100644
index 0000000..e61e2cc
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/ssh/PluginChangeFieldsIT.java
@@ -0,0 +1,88 @@
+// 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.acceptance.ssh;
+
+import static com.google.common.truth.Truth.assertThat;
+import static java.util.stream.Collectors.joining;
+
+import com.google.common.collect.ImmutableListMultimap;
+import com.google.common.io.CharStreams;
+import com.google.gerrit.acceptance.AbstractPluginFieldsTest;
+import com.google.gerrit.acceptance.UseSsh;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.server.query.change.OutputStreamQuery;
+import com.google.gson.Gson;
+import com.google.gson.reflect.TypeToken;
+import java.io.StringReader;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Stream;
+import org.junit.Test;
+
+@UseSsh
+public class PluginChangeFieldsIT extends AbstractPluginFieldsTest {
+ // No tests for getting a single change over SSH, since the only API is the query API.
+
+ private static final Gson GSON = OutputStreamQuery.GSON;
+
+ @Test
+ public void queryChangeWithNullAttribute() throws Exception {
+ getChangeWithNullAttribute(
+ id -> pluginInfoFromSingletonList(adminSshSession.exec(changeQueryCmd(id))));
+ }
+
+ @Test
+ public void queryChangeWithSimpleAttribute() throws Exception {
+ getChangeWithSimpleAttribute(
+ id -> pluginInfoFromSingletonList(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))));
+ }
+
+ private String changeQueryCmd(Change.Id id) {
+ return changeQueryCmd(id, ImmutableListMultimap.of());
+ }
+
+ private String changeQueryCmd(Change.Id id, ImmutableListMultimap<String, String> pluginOptions) {
+ return "gerrit query --format json "
+ + pluginOptions.entries().stream()
+ .flatMap(e -> Stream.of("--" + e.getKey(), e.getValue()))
+ .collect(joining(" "))
+ + " "
+ + id;
+ }
+
+ @Nullable
+ private static List<MyInfo> pluginInfoFromSingletonList(String sshOutput) throws Exception {
+ List<Map<String, Object>> changeAttrs = new ArrayList<>();
+ for (String line : CharStreams.readLines(new StringReader(sshOutput))) {
+ Map<String, Object> changeAttr =
+ GSON.fromJson(line, new TypeToken<Map<String, Object>>() {}.getType());
+ if (!"stats".equals(changeAttr.get("type"))) {
+ changeAttrs.add(changeAttr);
+ }
+ }
+
+ assertThat(changeAttrs).hasSize(1);
+ return decodeRawPluginsList(GSON, changeAttrs.get(0).get("plugins"));
+ }
+}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
index 79170c4..504ec46 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
@@ -47,7 +47,7 @@
case V6_5:
return "docker.elastic.co/elasticsearch/elasticsearch-oss:6.5.4";
case V6_6:
- return "docker.elastic.co/elasticsearch/elasticsearch-oss:6.6.1";
+ return "docker.elastic.co/elasticsearch/elasticsearch-oss:6.6.2";
case V7_0:
return "docker.elastic.co/elasticsearch/elasticsearch-oss:7.0.0-beta1";
}
diff --git a/plugins/replication b/plugins/replication
index a4bae3f..ff75ac8 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit a4bae3f491bb3f693e2407d353d3a81ce2a5c8be
+Subproject commit ff75ac8a4c806e5f627cb755f49e08962fa6e6b0
