opendev/gerrit
Code Issues Proposed changes

Merge "Remove the Deprecated ChangeAttributeFactory Interface"

Browse Source
This commit is contained in:
Nasser Grainawi
2021-01-12 17:28:53 +00:00
committed by Gerrit Code Review
parent b97a3de076 260a910932
commit 81c4d49685
15 changed files with 12 additions and 487 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
Documentation/dev-plugins.txt
View File

@@ -1007,17 +1007,9 @@ Output:
Runtime exceptions generated by the implementors of ChangePluginDefinedInfoFactory
are encapsulated in PluginDefinedInfo objects which are part of SSH/REST query output.
==== ChangeAttributeFactory
Alternatively, there is also `ChangeAttributeFactory` which takes in one single
`ChangeData` at a time. `ChangePluginDefinedInfoFactory` should be preferred
over this as it handles many changes at once which also decreases the round-trip
time for queries resulting in performance increase for bulk queries.
Implementors of the `ChangePluginDefinedInfoFactory` and `ChangeAttributeFactory`
interfaces should check whether they need to contribute to the
link:#change-etag-computation[change ETag computation] to prevent callers using
ETags from potentially seeing outdated plugin attributes.
Implementors of the `ChangePluginDefinedInfoFactory` interface should check whether
they need to contribute to the link:#change-etag-computation[change ETag computation]
to prevent callers using ETags from potentially seeing outdated plugin attributes.
[[simple-configuration]]
== Simple Configuration in `gerrit.config`

110
java/com/google/gerrit/acceptance/AbstractPluginFieldsTest.java
View File

@@ -30,7 +30,6 @@ import com.google.gerrit.extensions.common.PluginDefinedInfo;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.server.DynamicOptions;
import com.google.gerrit.server.DynamicOptions.DynamicBean;
import com.google.gerrit.server.change.ChangeAttributeFactory;
import com.google.gerrit.server.change.ChangePluginDefinedInfoFactory;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.restapi.change.GetChange;
@@ -40,7 +39,6 @@ import com.google.gson.Gson;
import com.google.gson.reflect.TypeToken;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import com.google.inject.Module;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
@@ -86,21 +84,6 @@ public class AbstractPluginFieldsTest extends AbstractDaemonTest {
}
}
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()));
}
}
protected static class PluginDefinedSimpleAttributeModule extends AbstractModule {
@Override
public void configure() {
@@ -170,21 +153,6 @@ public class AbstractPluginFieldsTest extends AbstractDaemonTest {
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);
}
}
public static class BulkAttributeFactoryWithOption implements ChangePluginDefinedInfoFactory {
protected MyOptions opts;
@@ -211,33 +179,6 @@ public class AbstractPluginFieldsTest extends AbstractDaemonTest {
}
}
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 getSingleChangeWithPluginDefinedBulkAttribute(BulkPluginInfoGetterWithId getter)
throws Exception {
Change.Id id = createChange().getChange().getId();
@@ -298,30 +239,6 @@ public class AbstractPluginFieldsTest extends AbstractDaemonTest {
assertThat(pluginInfos.get(changeWithInfo)).isNull();
}
protected void getMultipleChangesWithPluginDefinedBulkAndChangeAttributes(
BulkPluginInfoGetter getter) throws Exception {
Change.Id id1 = createChange().getChange().getId();
Change.Id id2 = createChange().getChange().getId();
Map<Change.Id, List<PluginDefinedInfo>> pluginInfos = getter.call();
assertThat(pluginInfos.get(id1)).isNull();
assertThat(pluginInfos.get(id2)).isNull();
try (AutoCloseable ignored =
installPlugin("my-plugin-1", PluginDefinedSimpleAttributeModule.class);
AutoCloseable ignored1 = installPlugin("my-plugin-2", SimpleAttributeModule.class)) {
pluginInfos = getter.call();
assertThat(pluginInfos.get(id1)).contains(new MyInfo("my-plugin-1", "change " + id1));
assertThat(pluginInfos.get(id1)).contains(new MyInfo("my-plugin-2", "change " + id1));
assertThat(pluginInfos.get(id2)).contains(new MyInfo("my-plugin-1", "change " + id2));
assertThat(pluginInfos.get(id2)).contains(new MyInfo("my-plugin-2", "change " + id2));
}
pluginInfos = getter.call();
assertThat(pluginInfos.get(id1)).isNull();
assertThat(pluginInfos.get(id2)).isNull();
}
protected void getMultipleChangesWithPluginDefinedBulkAttributeInSingleCall(
BulkPluginInfoGetter getter) throws Exception {
Change.Id id1 = createChange().getChange().getId();
@@ -345,22 +262,6 @@ public class AbstractPluginFieldsTest extends AbstractDaemonTest {
assertThat(pluginInfos.get(id2)).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 void getChangeWithPluginDefinedBulkAttributeOption(
BulkPluginInfoGetterWithId getterWithoutOptions,
BulkPluginInfoGetterWithIdAndOptions getterWithOptions)
@@ -454,11 +355,6 @@ public class AbstractPluginFieldsTest extends AbstractDaemonTest {
return out;
}
@FunctionalInterface
protected interface PluginInfoGetter {
List<PluginDefinedInfo> call(Change.Id id) throws Exception;
}
@FunctionalInterface
protected interface BulkPluginInfoGetter {
Map<Change.Id, List<PluginDefinedInfo>> call() throws Exception;
@@ -474,10 +370,4 @@ public class AbstractPluginFieldsTest extends AbstractDaemonTest {
Map<Change.Id, List<PluginDefinedInfo>> call(
Change.Id id, ImmutableListMultimap<String, String> pluginOptions) throws Exception;
}
@FunctionalInterface
protected interface PluginInfoGetterWithOptions {
List<PluginDefinedInfo> call(Change.Id id, ImmutableListMultimap<String, String> pluginOptions)
throws Exception;
}
}

3
java/com/google/gerrit/pgm/util/BatchProgramModule.java
View File

@@ -39,7 +39,6 @@ import com.google.gerrit.server.account.ServiceUserClassifierImpl;
import com.google.gerrit.server.account.externalids.ExternalIdModule;
import com.google.gerrit.server.cache.CacheRemovalListener;
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;
@@ -116,8 +115,6 @@ public class BatchProgramModule extends FactoryModule {
bind(new TypeLiteral<List<CommentLinkInfo>>() {})
.toProvider(CommentLinkProvider.class)
.in(SINGLETON);
bind(new TypeLiteral<DynamicSet<ChangeAttributeFactory>>() {})
.toInstance(DynamicSet.emptySet());
bind(new TypeLiteral<DynamicMap<RestView<CommitResource>>>() {})
.toInstance(DynamicMap.emptyMap());
bind(String.class)

49
java/com/google/gerrit/server/change/ChangeAttributeFactory.java
View File

@@ -1,49 +0,0 @@
// 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.
*/
@Deprecated
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);
}

25
java/com/google/gerrit/server/change/ChangeJson.java
View File

@@ -158,17 +158,12 @@ public class ChangeJson {
}
public ChangeJson create(Iterable<ListChangesOption> options) {
return factory.create(options, Optional.empty(), Optional.empty());
return factory.create(options, Optional.empty());
}
public ChangeJson create(
Iterable<ListChangesOption> options,
PluginDefinedAttributesFactory pluginDefinedAttributesFactory,
PluginDefinedInfosFactory pluginDefinedInfosFactory) {
return factory.create(
options,
Optional.of(pluginDefinedAttributesFactory),
Optional.of(pluginDefinedInfosFactory));
Iterable<ListChangesOption> options, PluginDefinedInfosFactory pluginDefinedInfosFactory) {
return factory.create(options, Optional.of(pluginDefinedInfosFactory));
}
public ChangeJson create(ListChangesOption first, ListChangesOption... rest) {
@@ -179,7 +174,6 @@ public class ChangeJson {
public interface AssistedFactory {
ChangeJson create(
Iterable<ListChangesOption> options,
Optional<PluginDefinedAttributesFactory> pluginDefinedAttributesFactory,
Optional<PluginDefinedInfosFactory> pluginDefinedInfosFactory);
}
@@ -226,7 +220,6 @@ public class ChangeJson {
private final TrackingFooters trackingFooters;
private final Metrics metrics;
private final RevisionJson revisionJson;
private final Optional<PluginDefinedAttributesFactory> pluginDefinedAttributesFactory;
private final Optional<PluginDefinedInfosFactory> pluginDefinedInfosFactory;
private final boolean includeMergeable;
private final boolean lazyLoad;
@@ -251,7 +244,6 @@ public class ChangeJson {
RevisionJson.Factory revisionJsonFactory,
@GerritServerConfig Config cfg,
@Assisted Iterable<ListChangesOption> options,
@Assisted Optional<PluginDefinedAttributesFactory> pluginDefinedAttributesFactory,
@Assisted Optional<PluginDefinedInfosFactory> pluginDefinedInfosFactory) {
this.userProvider = user;
this.changeDataFactory = cdf;
@@ -269,7 +261,6 @@ public class ChangeJson {
this.options = Sets.immutableEnumSet(options);
this.includeMergeable = MergeabilityComputationBehavior.fromConfig(cfg).includeInApi();
this.lazyLoad = containsAnyOf(this.options, REQUIRE_LAZY_LOAD);
this.pluginDefinedAttributesFactory = pluginDefinedAttributesFactory;
this.pluginDefinedInfosFactory = pluginDefinedInfosFactory;
logger.atFine().log("options = %s", options);
@@ -611,17 +602,9 @@ public class ChangeJson {
}
setSubmitter(cd, out);
if (pluginDefinedAttributesFactory.isPresent()) {
out.plugins = pluginDefinedAttributesFactory.get().create(cd);
}
if (!pluginInfos.isEmpty()) {
if (out.plugins == null) {
out.plugins = pluginInfos;
} else {
out.plugins = new ArrayList<>(out.plugins);
out.plugins.addAll(pluginInfos);
}
out.plugins = pluginInfos;
}
out.revertOf = cd.change().getRevertOf() != null ? cd.change().getRevertOf().get() : null;
out.submissionId = cd.change().getSubmissionId();

35
java/com/google/gerrit/server/change/PluginDefinedAttributesFactories.java
View File

@@ -14,55 +14,22 @@
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.collect.ImmutableListMultimap;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Change;
import com.google.gerrit.extensions.common.PluginDefinedInfo;
import com.google.gerrit.extensions.registration.Extension;
import com.google.gerrit.server.DynamicOptions.BeanProvider;
import com.google.gerrit.server.query.change.ChangeData;
import java.util.Collection;
import java.util.Objects;
import java.util.stream.Stream;
/** Static helpers for use by {@link PluginDefinedAttributesFactory} implementations. */
/** Static helpers for use by {@link PluginDefinedInfosFactory} 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;
}
public static ImmutableListMultimap<Change.Id, PluginDefinedInfo> createAll(
Collection<ChangeData> cds,
BeanProvider beanProvider,

23
java/com/google/gerrit/server/change/PluginDefinedAttributesFactory.java
View File

@@ -1,23 +0,0 @@
// 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.query.change.ChangeData;
import java.util.List;
public interface PluginDefinedAttributesFactory {
List<PluginDefinedInfo> create(ChangeData cd);
}

2
java/com/google/gerrit/server/config/GerritGlobalModule.java
View File

@@ -106,7 +106,6 @@ import com.google.gerrit.server.avatar.AvatarProvider;
import com.google.gerrit.server.cache.CacheRemovalListener;
import com.google.gerrit.server.change.AbandonOp;
import com.google.gerrit.server.change.AccountPatchReviewStore;
import com.google.gerrit.server.change.ChangeAttributeFactory;
import com.google.gerrit.server.change.ChangeETagComputation;
import com.google.gerrit.server.change.ChangeFinder;
import com.google.gerrit.server.change.ChangeJson;
@@ -441,7 +440,6 @@ public class GerritGlobalModule extends FactoryModule {
DynamicMap.mapOf(binder(), ChangeQueryBuilder.ChangeOperatorFactory.class);
DynamicMap.mapOf(binder(), ChangeQueryBuilder.ChangeHasOperandFactory.class);
DynamicMap.mapOf(binder(), ChangeQueryBuilder.ChangeIsOperandFactory.class);
DynamicSet.setOf(binder(), ChangeAttributeFactory.class);
DynamicSet.setOf(binder(), ChangePluginDefinedInfoFactory.class);
install(new GitwebConfig.LegacyModule(cfg));

25
java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
View File

@@ -17,7 +17,6 @@ package com.google.gerrit.server.query.change;
import static com.google.common.base.Preconditions.checkState;
import static com.google.gerrit.server.query.change.ChangeQueryBuilder.FIELD_LIMIT;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
import com.google.gerrit.entities.Change;
import com.google.gerrit.extensions.common.PluginDefinedInfo;
@@ -33,10 +32,8 @@ import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.DynamicOptions;
import com.google.gerrit.server.DynamicOptions.DynamicBean;
import com.google.gerrit.server.account.AccountLimits;
import com.google.gerrit.server.change.ChangeAttributeFactory;
import com.google.gerrit.server.change.ChangePluginDefinedInfoFactory;
import com.google.gerrit.server.change.PluginDefinedAttributesFactories;
import com.google.gerrit.server.change.PluginDefinedAttributesFactory;
import com.google.gerrit.server.change.PluginDefinedInfosFactory;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
import com.google.gerrit.server.index.change.ChangeIndexRewriter;
@@ -60,7 +57,6 @@ import java.util.Set;
public class ChangeQueryProcessor extends QueryProcessor<ChangeData>
implements DynamicOptions.BeanReceiver, DynamicOptions.BeanProvider, PluginDefinedInfosFactory {
private final Provider<CurrentUser> userProvider;
private final ImmutableListMultimap<String, ChangeAttributeFactory> attributeFactoriesByPlugin;
private final ChangeIsVisibleToPredicate.Factory changeIsVisibleToPredicateFactory;
private final Map<String, DynamicBean> dynamicBeans = new HashMap<>();
private final List<Extension<ChangePluginDefinedInfoFactory>>
@@ -81,7 +77,6 @@ public class ChangeQueryProcessor extends QueryProcessor<ChangeData>
IndexConfig indexConfig,
ChangeIndexCollection indexes,
ChangeIndexRewriter rewriter,
DynamicSet<ChangeAttributeFactory> attributeFactories,
ChangeIsVisibleToPredicate.Factory changeIsVisibleToPredicateFactory,
DynamicSet<ChangePluginDefinedInfoFactory> changePluginDefinedInfoFactories) {
super(
@@ -95,14 +90,6 @@ public class ChangeQueryProcessor extends QueryProcessor<ChangeData>
this.userProvider = userProvider;
this.changeIsVisibleToPredicateFactory = changeIsVisibleToPredicateFactory;
ImmutableListMultimap.Builder<String, ChangeAttributeFactory> factoriesBuilder =
ImmutableListMultimap.builder();
ImmutableListMultimap.Builder<String, ChangePluginDefinedInfoFactory> infosFactoriesBuilder =
ImmutableListMultimap.builder();
// Eagerly call Extension#get() rather than storing Extensions, since that method invokes the
// Provider on every call, which could be expensive if we invoke it once for every change.
attributeFactories.entries().forEach(e -> factoriesBuilder.put(e.getPluginName(), e.get()));
attributeFactoriesByPlugin = factoriesBuilder.build();
changePluginDefinedInfoFactories
.entries()
.forEach(e -> changePluginDefinedInfoFactoriesByPlugin.add(e));
@@ -130,18 +117,6 @@ public class ChangeQueryProcessor extends QueryProcessor<ChangeData>
return dynamicBeans.get(plugin);
}
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)));
}
public PluginDefinedInfosFactory getInfosFactory() {
return this::createPluginDefinedInfos;
}

8
java/com/google/gerrit/server/query/change/OutputStreamQuery.java
View File

@@ -330,15 +330,9 @@ public class OutputStreamQuery {
eventFactory.addDependencies(rw, c, d.change(), d.currentPatchSet());
}
c.plugins = queryProcessor.getAttributesFactory().create(d);
List<PluginDefinedInfo> pluginInfos = pluginInfosByChange.get(d.getId());
if (!pluginInfos.isEmpty()) {
if (c.plugins == null) {
c.plugins = pluginInfos;
} else {
c.plugins = new ArrayList<>(c.plugins);
c.plugins.addAll(pluginInfos);
}
c.plugins = pluginInfos;
}
return c;
}

16
java/com/google/gerrit/server/restapi/change/GetChange.java
View File

@@ -14,7 +14,6 @@
package com.google.gerrit.server.restapi.change;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.Streams;
import com.google.gerrit.entities.Change;
@@ -27,7 +26,6 @@ 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.ChangePluginDefinedInfoFactory;
import com.google.gerrit.server.change.ChangeResource;
@@ -46,7 +44,6 @@ public class GetChange
DynamicOptions.BeanReceiver,
DynamicOptions.BeanProvider {
private final ChangeJson.Factory json;
private final DynamicSet<ChangeAttributeFactory> attrFactories;
private final DynamicSet<ChangePluginDefinedInfoFactory> pdiFactories;
private final EnumSet<ListChangesOption> options = EnumSet.noneOf(ListChangesOption.class);
private final Map<String, DynamicBean> dynamicBeans = new HashMap<>();
@@ -62,12 +59,8 @@ public class GetChange
}
@Inject
GetChange(
ChangeJson.Factory json,
DynamicSet<ChangeAttributeFactory> attrFactories,
DynamicSet<ChangePluginDefinedInfoFactory> pdiFactories) {
GetChange(ChangeJson.Factory json, DynamicSet<ChangePluginDefinedInfoFactory> pdiFactories) {
this.json = json;
this.attrFactories = attrFactories;
this.pdiFactories = pdiFactories;
}
@@ -91,12 +84,7 @@ public class GetChange
}
private ChangeJson newChangeJson() {
return json.create(options, this::buildPluginInfo, this::createPluginDefinedInfos);
}
private ImmutableList<PluginDefinedInfo> buildPluginInfo(ChangeData cd) {
return PluginDefinedAttributesFactories.createAll(
cd, this, Streams.stream(attrFactories.entries()));
return json.create(options, this::createPluginDefinedInfos);
}
private ImmutableListMultimap<Change.Id, PluginDefinedInfo> createPluginDefinedInfos(

4
java/com/google/gerrit/server/restapi/change/QueryChanges.java
View File

@@ -187,9 +187,7 @@ public class QueryChanges implements RestReadView<TopLevelResource>, DynamicOpti
int cnt = queries.size();
List<QueryResult<ChangeData>> results = queryProcessor.query(qb.parse(queries));
List<List<ChangeInfo>> res =
json.create(
options, queryProcessor.getAttributesFactory(), queryProcessor.getInfosFactory())
.format(results);
json.create(options, queryProcessor.getInfosFactory()).format(results);
for (int n = 0; n < cnt; n++) {
List<ChangeInfo> info = res.get(n);
if (results.get(n).more() && !info.isEmpty()) {

68
javatests/com/google/gerrit/acceptance/api/change/PluginFieldsIT.java
View File

@@ -16,9 +16,6 @@ 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 java.util.Arrays;
import org.junit.Test;
@@ -26,30 +23,6 @@ import org.junit.Test;
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 querySingleChangeWithBulkAttribute() throws Exception {
getSingleChangeWithPluginDefinedBulkAttribute(
@@ -62,22 +35,6 @@ public class PluginFieldsIT extends AbstractPluginFieldsTest {
id -> pluginInfosFromChangeInfos(Arrays.asList(gApi.changes().id(id.toString()).get())));
}
@Test
public void queryChangeWithOption() throws Exception {
getChangeWithOption(
id -> pluginInfoFromSingletonList(gApi.changes().query(id.toString()).get()),
(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)));
}
@Test
public void queryChangeWithOptionBulkAttribute() throws Exception {
getChangeWithPluginDefinedBulkAttributeOption(
@@ -107,12 +64,6 @@ public class PluginFieldsIT extends AbstractPluginFieldsTest {
() -> pluginInfosFromChangeInfos(gApi.changes().query("status:open").get()));
}
@Test
public void getMultipleChangesWithPluginDefinedAndChangeAttributes() throws Exception {
getMultipleChangesWithPluginDefinedBulkAndChangeAttributes(
() -> pluginInfosFromChangeInfos(gApi.changes().query("status:open").get()));
}
@Test
public void getMultipleChangesWithPluginDefinedAttributeInSingleCall() throws Exception {
getMultipleChangesWithPluginDefinedBulkAttributeInSingleCall(
@@ -124,23 +75,4 @@ public class PluginFieldsIT extends AbstractPluginFieldsTest {
getChangeWithPluginDefinedBulkAttributeWithException(
id -> pluginInfosFromChangeInfos(Arrays.asList(gApi.changes().id(id.get()).get())));
}
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);
}
}

82
javatests/com/google/gerrit/acceptance/rest/change/PluginFieldsIT.java
View File

@@ -14,8 +14,6 @@
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;
@@ -34,41 +32,6 @@ 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 querySingleChangeWithBulkAttribute() throws Exception {
getSingleChangeWithPluginDefinedBulkAttribute(
@@ -87,27 +50,6 @@ public class PluginFieldsIT extends AbstractPluginFieldsTest {
id -> pluginInfoMapFromChangeInfo(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))));
}
@Test
public void pluginDefinedQueryChangeWithOption() throws Exception {
getChangeWithPluginDefinedBulkAttributeOption(
@@ -141,12 +83,6 @@ public class PluginFieldsIT extends AbstractPluginFieldsTest {
() -> pluginInfosFromChangeInfos(adminRestSession.get("/changes/?q=status:open")));
}
@Test
public void getMultipleChangesWithPluginDefinedAndChangeAttributes() throws Exception {
getMultipleChangesWithPluginDefinedBulkAndChangeAttributes(
() -> pluginInfosFromChangeInfos(adminRestSession.get("/changes/?q=status:open")));
}
@Test
public void getMultipleChangesWithPluginDefinedAttributeInSingleCall() throws Exception {
getMultipleChangesWithPluginDefinedBulkAttributeInSingleCall(
@@ -203,24 +139,6 @@ public class PluginFieldsIT extends AbstractPluginFieldsTest {
return Joiner.on('&').withKeyValueSeparator('=').join(opts.entries());
}
@Nullable
private static List<PluginDefinedInfo> pluginInfoFromSingletonList(RestResponse res)
throws Exception {
res.assertOK();
List<Map<String, Object>> changeInfos =
GSON.fromJson(res.getReader(), new TypeToken<List<Map<String, Object>>>() {}.getType());
assertThat(changeInfos).hasSize(1);
return decodeRawPluginsList(GSON, changeInfos.get(0).get("plugins"));
}
@Nullable
private List<PluginDefinedInfo> pluginInfoFromChangeInfo(RestResponse res) throws Exception {
res.assertOK();
Map<String, Object> changeInfo =
GSON.fromJson(res.getReader(), new TypeToken<Map<String, Object>>() {}.getType());
return decodeRawPluginsList(GSON, changeInfo.get("plugins"));
}
@Nullable
private Map<Change.Id, List<PluginDefinedInfo>> pluginInfoMapFromChangeInfo(RestResponse res)
throws Exception {

35
javatests/com/google/gerrit/acceptance/ssh/PluginChangeFieldsIT.java
View File

@@ -14,7 +14,6 @@
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;
@@ -40,31 +39,12 @@ public class PluginChangeFieldsIT extends AbstractPluginFieldsTest {
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 querySingleChangeWithBulkAttribute() throws Exception {
getSingleChangeWithPluginDefinedBulkAttribute(
id -> pluginInfosFromList(adminSshSession.exec(changeQueryCmd(id))));
}
@Test
public void queryChangeWithOption() throws Exception {
getChangeWithOption(
id -> pluginInfoFromSingletonList(adminSshSession.exec(changeQueryCmd(id))),
(id, opts) -> pluginInfoFromSingletonList(adminSshSession.exec(changeQueryCmd(id, opts))));
}
@Test
public void queryPluginDefinedAttributeChangeWithOption() throws Exception {
getChangeWithPluginDefinedBulkAttributeOption(
@@ -84,12 +64,6 @@ public class PluginChangeFieldsIT extends AbstractPluginFieldsTest {
() -> pluginInfosFromList(adminSshSession.exec("gerrit query --format json status:open")));
}
@Test
public void getMultipleChangesWithPluginDefinedAndChangeAttributes() throws Exception {
getMultipleChangesWithPluginDefinedBulkAndChangeAttributes(
() -> pluginInfosFromList(adminSshSession.exec("gerrit query --format json status:open")));
}
@Test
public void getMultipleChangesWithPluginDefinedAttributeInSingleCall() throws Exception {
getMultipleChangesWithPluginDefinedBulkAttributeInSingleCall(
@@ -115,15 +89,6 @@ public class PluginChangeFieldsIT extends AbstractPluginFieldsTest {
+ id;
}
@Nullable
private static List<PluginDefinedInfo> pluginInfoFromSingletonList(String sshOutput)
throws Exception {
List<Map<String, Object>> changeAttrs = getChangeAttrs(sshOutput);
assertThat(changeAttrs).hasSize(1);
return decodeRawPluginsList(GSON, changeAttrs.get(0).get("plugins"));
}
@Nullable
private static Map<Change.Id, List<PluginDefinedInfo>> pluginInfosFromList(String sshOutput)
throws Exception {