Support plugin fields in GetChange
Plugins implementing the existing ChangeAttributeFactory interface will
now get their fields populated on GET /changes/X and /changes/X/detail.
As before, the docs recommend guarding plugin-defined attribute
computation with @Options defined in a DynamicBean. Plugins that are
already doing this will see no behavior change on the Get Change
endpoints until they start passing their existing options.
Treat DynamicBeans for /detail the same as for /change without requiring
plugin authors to bind their DynamicBeans twice with two different
export names.
Change-Id: I9254d46b36ff0f359551c1ed334687fd14538973
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index 3517e22..00e153b 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -860,14 +860,14 @@
----
[[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
-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 registering it to the class in the plugin module's `configure()`
+method. The new attribute(s) will be output under a `plugin` attribute in the
+change output. This can be further controlled with an option registered in
+the Http and Ssh modules' `configure*()` methods.
The example below shows a plugin that adds two attributes (`exampleName` and
`changeValue`), to the change query output, when the query command is provided
@@ -884,7 +884,7 @@
}
}
-public class MyQueryOptions implements DynamicBean {
+public class MyChangeOptions implements DynamicBean {
@Option(name = "--all", usage = "Include plugin output")
public boolean all = false;
}
@@ -893,8 +893,11 @@
@Override
protected void configureServlets() {
bind(DynamicBean.class)
+ .annotatedWith(Exports.named(GetChange.class))
+ .to(MyChangeOptions.class);
+ bind(DynamicBean.class)
.annotatedWith(Exports.named(QueryChanges.class))
- .to(MyQueryOptions.class);
+ .to(MyChangeOptions.class);
}
}
@@ -903,12 +906,12 @@
protected void configureCommands() {
bind(DynamicBean.class)
.annotatedWith(Exports.named(Query.class))
- .to(MyQueryOptions.class);
+ .to(MyChangeOptions.class);
}
}
public class AttributeFactory implements ChangeAttributeFactory {
- protected MyQueryOptions options;
+ protected MyChangeOptions options;
public class PluginAttribute extends PluginDefinedInfo {
public String exampleName;
@@ -923,7 +926,7 @@
@Override
public PluginDefinedInfo create(ChangeData c, BeanProvider bp, String plugin) {
if (options == null) {
- options = (MyQueryOptions) bp.getDynamicBean(plugin);
+ options = (MyChangeOptions) bp.getDynamicBean(plugin);
}
if (options.all) {
return new PluginAttribute(c);
@@ -951,6 +954,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]]
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/server/DynamicOptions.java b/java/com/google/gerrit/server/DynamicOptions.java
index 2ae6899..44d3493 100644
--- a/java/com/google/gerrit/server/DynamicOptions.java
+++ b/java/com/google/gerrit/server/DynamicOptions.java
@@ -153,6 +153,17 @@
*/
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 {
@@ -199,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/changes/ChangeApiImpl.java b/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
index ad39633..f00e509 100644
--- a/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
+++ b/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
@@ -54,7 +54,6 @@
import com.google.gerrit.extensions.restapi.RestApiException;
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 +67,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;
@@ -132,7 +132,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;
@@ -180,7 +180,7 @@
PutTopic putTopic,
ChangeIncludedIn includedIn,
PostReviewers postReviewers,
- ChangeJson.Factory changeJson,
+ Provider<GetChange> getChangeProvider,
PostHashtags postHashtags,
GetHashtags getHashtags,
PutAssignee putAssignee,
@@ -226,7 +226,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;
@@ -454,7 +454,9 @@
@Override
public ChangeInfo get(EnumSet<ListChangesOption> s) throws RestApiException {
try {
- return changeJson.create(s).format(change);
+ GetChange getChange = getChangeProvider.get();
+ s.forEach(getChange::addOption);
+ return getChange.apply(change).value();
} catch (Exception e) {
throw asRestApiException("Cannot retrieve change", e);
}
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/ChangeQueryProcessor.java b/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
index 066b443..7a7f71d 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
@@ -16,12 +16,11 @@
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.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.Extension;
import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.index.QueryOptions;
import com.google.gerrit.index.query.IndexPredicate;
@@ -34,6 +33,7 @@
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;
@@ -44,7 +44,6 @@
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;
@@ -60,8 +59,6 @@
implements DynamicOptions.BeanReceiver,
DynamicOptions.BeanProvider,
PluginDefinedAttributesFactory {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
-
private final Provider<CurrentUser> userProvider;
private final ChangeNotes.Factory notesFactory;
private final ImmutableListMultimap<String, ChangeAttributeFactory> attributeFactoriesByPlugin;
@@ -136,26 +133,11 @@
@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;
+ 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/restapi/change/GetChange.java b/java/com/google/gerrit/server/restapi/change/GetChange.java
index 2decb4e..461a669 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.DynamicMap;
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 DynamicMap<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,35 @@
}
@Inject
- GetChange(ChangeJson.Factory json) {
+ GetChange(ChangeJson.Factory json, DynamicMap<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));
}
}
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/javatests/com/google/gerrit/acceptance/api/change/PluginFieldsIT.java b/javatests/com/google/gerrit/acceptance/api/change/PluginFieldsIT.java
index 88ef004..b8ec5d92 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/PluginFieldsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/PluginFieldsIT.java
@@ -36,6 +36,7 @@
import com.google.gerrit.server.DynamicOptions.DynamicBean;
import com.google.gerrit.server.change.ChangeAttributeFactory;
import com.google.gerrit.server.query.change.OutputStreamQuery;
+import com.google.gerrit.server.restapi.change.GetChange;
import com.google.gerrit.server.restapi.change.QueryChanges;
import com.google.gerrit.sshd.PluginCommandModule;
import com.google.gerrit.sshd.commands.Query;
@@ -53,6 +54,9 @@
@UseSsh
public class PluginFieldsIT extends AbstractDaemonTest {
+ private static final Gson REST_GSON = OutputFormat.JSON.newGson();
+ private static final Gson SSH_GSON = OutputStreamQuery.GSON;
+
static class MyInfo extends PluginDefinedInfo {
@Nullable String theAttribute;
@@ -135,28 +139,50 @@
@Override
protected void configureServlets() {
bind(DynamicBean.class).annotatedWith(Exports.named(QueryChanges.class)).to(MyOptions.class);
+ bind(DynamicBean.class).annotatedWith(Exports.named(GetChange.class)).to(MyOptions.class);
}
}
@Test
public void queryChangeApiWithNullAttribute() throws Exception {
- queryChangeWithNullAttribute(
+ getChangeWithNullAttribute(
id -> pluginInfoFromSingletonList(gApi.changes().query(id.toString()).get()));
}
@Test
public void queryChangeRestWithNullAttribute() throws Exception {
- queryChangeWithNullAttribute(
+ getChangeWithNullAttribute(
id -> pluginInfoFromSingletonListRest(adminRestSession.get(changeQueryUrl(id))));
}
@Test
public void queryChangeSshWithNullAttribute() throws Exception {
- queryChangeWithNullAttribute(
+ getChangeWithNullAttribute(
id -> pluginInfoFromSingletonListSsh(adminSshSession.exec(changeQueryCmd(id))));
}
- private void queryChangeWithNullAttribute(PluginInfoGetter getter) throws Exception {
+ @Test
+ public void getChangeApiWithNullAttribute() throws Exception {
+ getChangeWithNullAttribute(
+ id -> pluginInfoFromChangeInfo(gApi.changes().id(id.toString()).get()));
+ }
+
+ @Test
+ public void getChangeRestWithNullAttribute() throws Exception {
+ getChangeWithNullAttribute(
+ id -> pluginInfoFromChangeInfoRest(adminRestSession.get(changeUrl(id))));
+ }
+
+ @Test
+ public void getChangeDetailRestWithNullAttribute() throws Exception {
+ getChangeWithNullAttribute(
+ id -> pluginInfoFromChangeInfoRest(adminRestSession.get(changeDetailUrl(id))));
+ }
+
+ // No tests for /detail via the extension API, since the extension API doesn't have that method.
+ // No tests for getting a single change over SSH, since the only API is the query API.
+
+ private void getChangeWithNullAttribute(PluginInfoGetter getter) throws Exception {
Change.Id id = createChange().getChange().getId();
assertThat(getter.call(id)).isNull();
@@ -169,29 +195,48 @@
@Test
public void queryChangeApiWithSimpleAttribute() throws Exception {
- queryChangeWithSimpleAttribute(
+ getChangeWithSimpleAttribute(
id -> pluginInfoFromSingletonList(gApi.changes().query(id.toString()).get()));
}
@Test
public void queryChangeRestWithSimpleAttribute() throws Exception {
- queryChangeWithSimpleAttribute(
+ getChangeWithSimpleAttribute(
id -> pluginInfoFromSingletonListRest(adminRestSession.get(changeQueryUrl(id))));
}
@Test
public void queryChangeSshWithSimpleAttribute() throws Exception {
- queryChangeWithSimpleAttribute(
+ getChangeWithSimpleAttribute(
id -> pluginInfoFromSingletonListSsh(adminSshSession.exec(changeQueryCmd(id))));
}
- private void queryChangeWithSimpleAttribute(PluginInfoGetter getter) throws Exception {
+ @Test
+ public void getChangeApiWithSimpleAttribute() throws Exception {
+ getChangeWithSimpleAttribute(
+ id -> pluginInfoFromChangeInfo(gApi.changes().id(id.toString()).get()));
+ }
+
+ @Test
+ public void getChangeRestWithSimpleAttribute() throws Exception {
+ getChangeWithSimpleAttribute(
+ id -> pluginInfoFromChangeInfoRest(adminRestSession.get(changeUrl(id))));
+ }
+
+ @Test
+ public void getChangeDetailRestWithSimpleAttribute() throws Exception {
+ getChangeWithSimpleAttribute(
+ id -> pluginInfoFromChangeInfoRest(adminRestSession.get(changeDetailUrl(id))));
+ }
+
+ // No tests for getting a single change over SSH, since the only API is the query API.
+
+ private void getChangeWithSimpleAttribute(PluginInfoGetter getter) throws Exception {
Change.Id id = createChange().getChange().getId();
assertThat(getter.call(id)).isNull();
try (AutoCloseable ignored = installPlugin("my-plugin", SimpleAttributeModule.class)) {
- List<MyInfo> infos = getter.call(id);
- assertThat(infos).containsExactly(new MyInfo("my-plugin", "change " + id));
+ assertThat(getter.call(id)).containsExactly(new MyInfo("my-plugin", "change " + id));
}
assertThat(getter.call(id)).isNull();
@@ -199,7 +244,7 @@
@Test
public void queryChangeSshWithOption() throws Exception {
- queryChangeWithOption(
+ getChangeWithOption(
id -> pluginInfoFromSingletonListSsh(adminSshSession.exec(changeQueryCmd(id))),
(id, opts) ->
pluginInfoFromSingletonListSsh(adminSshSession.exec(changeQueryCmd(id, opts))));
@@ -207,19 +252,37 @@
@Test
public void queryChangeRestWithOption() throws Exception {
- queryChangeWithOption(
+ getChangeWithOption(
id -> pluginInfoFromSingletonListRest(adminRestSession.get(changeQueryUrl(id))),
(id, opts) ->
pluginInfoFromSingletonListRest(adminRestSession.get(changeQueryUrl(id, opts))));
}
+ @Test
+ public void getChangeRestWithOption() throws Exception {
+ getChangeWithOption(
+ id -> pluginInfoFromChangeInfoRest(adminRestSession.get(changeUrl(id))),
+ (id, opts) -> pluginInfoFromChangeInfoRest(adminRestSession.get(changeUrl(id, opts))));
+ }
+
+ @Test
+ public void getChangeDetailRestWithOption() throws Exception {
+ getChangeWithOption(
+ id -> pluginInfoFromChangeInfoRest(adminRestSession.get(changeDetailUrl(id))),
+ (id, opts) ->
+ pluginInfoFromChangeInfoRest(adminRestSession.get(changeDetailUrl(id, opts))));
+ }
+
+ // No tests for /detail via the extension API, since the extension API doesn't have that method.
+ // No tests for getting a single change over SSH, since the only API is the query API.
+
// No test for plugin-provided options over the extension API. There are currently two separate
// DynamicMap<DynamicBean> maps initialized in the SSH and HTTP injectors, and plugins have to
// define separate SSH/HTTP modules and bind their DynamicBeans in each one. To use the extension
// API, we would have to move everything into the sys injector.
// TODO(dborowitz): Determine whether this is possible without breaking existing plugins.
- private void queryChangeWithOption(
+ private void getChangeWithOption(
PluginInfoGetter getterWithoutOptions, PluginInfoGetterWithOptions getterWithOptions)
throws Exception {
Change.Id id = createChange().getChange().getId();
@@ -246,7 +309,7 @@
private String changeQueryUrl(Change.Id id, ImmutableListMultimap<String, String> opts) {
String url = "/changes/?q=" + id;
- String queryString = Joiner.on('&').withKeyValueSeparator('=').join(opts.entries());
+ String queryString = buildQueryString(opts);
if (!queryString.isEmpty()) {
url += "&" + queryString;
}
@@ -266,9 +329,44 @@
+ id;
}
+ 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());
+ }
+
private static List<MyInfo> pluginInfoFromSingletonList(List<ChangeInfo> changeInfos) {
assertThat(changeInfos).hasSize(1);
- List<PluginDefinedInfo> pluginInfo = changeInfos.get(0).plugins;
+ return pluginInfoFromChangeInfo(changeInfos.get(0));
+ }
+
+ private static List<MyInfo> pluginInfoFromChangeInfo(ChangeInfo changeInfo) {
+ List<PluginDefinedInfo> pluginInfo = changeInfo.plugins;
if (pluginInfo == null) {
return null;
}
@@ -281,15 +379,11 @@
// Don't deserialize to ChangeInfo directly, since that would treat the plugins field as
// List<PluginDefinedInfo> and ignore the unknown keys found in MyInfo.
- Gson gson = OutputFormat.JSON.newGson();
List<Map<String, Object>> changeInfos =
- gson.fromJson(res.getReader(), new TypeToken<List<Map<String, Object>>>() {}.getType());
+ REST_GSON.fromJson(
+ res.getReader(), new TypeToken<List<Map<String, Object>>>() {}.getType());
assertThat(changeInfos).hasSize(1);
- Object plugins = changeInfos.get(0).get("plugins");
- if (plugins == null) {
- return null;
- }
- return gson.fromJson(gson.toJson(plugins), new TypeToken<List<MyInfo>>() {}.getType());
+ return myInfo(changeInfos.get(0));
}
@Nullable
@@ -315,6 +409,25 @@
return gson.fromJson(gson.toJson(plugins), new TypeToken<List<MyInfo>>() {}.getType());
}
+ @Nullable
+ private List<MyInfo> pluginInfoFromChangeInfoRest(RestResponse res) throws Exception {
+ res.assertOK();
+
+ // Don't deserialize to ChangeInfo directly, since that would treat the plugins field as
+ // List<PluginDefinedInfo> and ignore the unknown keys found in MyInfo.
+ return myInfo(
+ REST_GSON.fromJson(res.getReader(), new TypeToken<Map<String, Object>>() {}.getType()));
+ }
+
+ private static List<MyInfo> myInfo(Map<String, Object> changeInfo) {
+ Object plugins = changeInfo.get("plugins");
+ if (plugins == null) {
+ return null;
+ }
+ return REST_GSON.fromJson(
+ REST_GSON.toJson(plugins), new TypeToken<List<MyInfo>>() {}.getType());
+ }
+
@FunctionalInterface
private interface PluginInfoGetter {
List<MyInfo> call(Change.Id id) throws Exception;
