Merge "Remove the Deprecated ChangeAttributeFactory Interface"
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index f5b7282..159e2fc 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -1007,17 +1007,9 @@
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`
diff --git a/java/com/google/gerrit/acceptance/AbstractPluginFieldsTest.java b/java/com/google/gerrit/acceptance/AbstractPluginFieldsTest.java
index a91bc49..29dc6a3 100644
--- a/java/com/google/gerrit/acceptance/AbstractPluginFieldsTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractPluginFieldsTest.java
@@ -30,7 +30,6 @@
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.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 @@
}
}
- 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 @@
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 @@
}
}
- 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 @@
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 @@
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)
@@ -455,11 +356,6 @@
}
@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 @@
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;
- }
}
diff --git a/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
index e9c0136..894757b 100644
--- a/java/com/google/gerrit/pgm/util/BatchProgramModule.java
+++ b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
@@ -39,7 +39,6 @@
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 @@
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)
diff --git a/java/com/google/gerrit/server/change/ChangeAttributeFactory.java b/java/com/google/gerrit/server/change/ChangeAttributeFactory.java
deleted file mode 100644
index 663d7aa..0000000
--- a/java/com/google/gerrit/server/change/ChangeAttributeFactory.java
+++ /dev/null
@@ -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);
-}
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index 6ab0c61..02da518 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -158,17 +158,12 @@
}
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 interface AssistedFactory {
ChangeJson create(
Iterable<ListChangesOption> options,
- Optional<PluginDefinedAttributesFactory> pluginDefinedAttributesFactory,
Optional<PluginDefinedInfosFactory> pluginDefinedInfosFactory);
}
@@ -226,7 +220,6 @@
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 @@
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 @@
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 @@
}
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();
diff --git a/java/com/google/gerrit/server/change/PluginDefinedAttributesFactories.java b/java/com/google/gerrit/server/change/PluginDefinedAttributesFactories.java
index b474dab..db21f11 100644
--- a/java/com/google/gerrit/server/change/PluginDefinedAttributesFactories.java
+++ b/java/com/google/gerrit/server/change/PluginDefinedAttributesFactories.java
@@ -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,
diff --git a/java/com/google/gerrit/server/change/PluginDefinedAttributesFactory.java b/java/com/google/gerrit/server/change/PluginDefinedAttributesFactory.java
deleted file mode 100644
index 08d6ce7..0000000
--- a/java/com/google/gerrit/server/change/PluginDefinedAttributesFactory.java
+++ /dev/null
@@ -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);
-}
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index ee37d17..9308662 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -106,7 +106,6 @@
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 @@
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));
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java b/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
index 370bc75..ed1f2f1 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
@@ -17,7 +17,6 @@
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.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 @@
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 @@
IndexConfig indexConfig,
ChangeIndexCollection indexes,
ChangeIndexRewriter rewriter,
- DynamicSet<ChangeAttributeFactory> attributeFactories,
ChangeIsVisibleToPredicate.Factory changeIsVisibleToPredicateFactory,
DynamicSet<ChangePluginDefinedInfoFactory> changePluginDefinedInfoFactories) {
super(
@@ -95,14 +90,6 @@
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 @@
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;
}
diff --git a/java/com/google/gerrit/server/query/change/OutputStreamQuery.java b/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
index b931457..4922b57 100644
--- a/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
+++ b/java/com/google/gerrit/server/query/change/OutputStreamQuery.java
@@ -330,15 +330,9 @@
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;
}
diff --git a/java/com/google/gerrit/server/restapi/change/GetChange.java b/java/com/google/gerrit/server/restapi/change/GetChange.java
index 1ef3c4b..f28d2b3 100644
--- a/java/com/google/gerrit/server/restapi/change/GetChange.java
+++ b/java/com/google/gerrit/server/restapi/change/GetChange.java
@@ -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.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 @@
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 @@
}
@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 @@
}
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(
diff --git a/java/com/google/gerrit/server/restapi/change/QueryChanges.java b/java/com/google/gerrit/server/restapi/change/QueryChanges.java
index 3c8157b..cf0d4cf 100644
--- a/java/com/google/gerrit/server/restapi/change/QueryChanges.java
+++ b/java/com/google/gerrit/server/restapi/change/QueryChanges.java
@@ -187,9 +187,7 @@
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()) {
diff --git a/javatests/com/google/gerrit/acceptance/api/change/PluginFieldsIT.java b/javatests/com/google/gerrit/acceptance/api/change/PluginFieldsIT.java
index 31198d5..cebce0b 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/PluginFieldsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/PluginFieldsIT.java
@@ -16,9 +16,6 @@
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;
@@ -27,30 +24,6 @@
// 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(
id -> pluginInfosFromChangeInfos(gApi.changes().query(id.toString()).get()));
@@ -63,22 +36,6 @@
}
@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(
id -> pluginInfosFromChangeInfos(gApi.changes().query(id.toString()).get()),
@@ -108,12 +65,6 @@
}
@Test
- public void getMultipleChangesWithPluginDefinedAndChangeAttributes() throws Exception {
- getMultipleChangesWithPluginDefinedBulkAndChangeAttributes(
- () -> pluginInfosFromChangeInfos(gApi.changes().query("status:open").get()));
- }
-
- @Test
public void getMultipleChangesWithPluginDefinedAttributeInSingleCall() throws Exception {
getMultipleChangesWithPluginDefinedBulkAttributeInSingleCall(
() -> pluginInfosFromChangeInfos(gApi.changes().query("status:open").get()));
@@ -124,23 +75,4 @@
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);
- }
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/PluginFieldsIT.java b/javatests/com/google/gerrit/acceptance/rest/change/PluginFieldsIT.java
index 17bf37e..a4ec40e 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/PluginFieldsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/PluginFieldsIT.java
@@ -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;
@@ -35,41 +33,6 @@
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(
id -> pluginInfosFromChangeInfos(adminRestSession.get(changeQueryUrl(id))));
@@ -88,27 +51,6 @@
}
@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(
id -> pluginInfosFromChangeInfos(adminRestSession.get(changeQueryUrl(id))),
@@ -142,12 +84,6 @@
}
@Test
- public void getMultipleChangesWithPluginDefinedAndChangeAttributes() throws Exception {
- getMultipleChangesWithPluginDefinedBulkAndChangeAttributes(
- () -> pluginInfosFromChangeInfos(adminRestSession.get("/changes/?q=status:open")));
- }
-
- @Test
public void getMultipleChangesWithPluginDefinedAttributeInSingleCall() throws Exception {
getMultipleChangesWithPluginDefinedBulkAttributeInSingleCall(
() -> pluginInfosFromChangeInfos(adminRestSession.get("/changes/?q=status:open")));
@@ -204,24 +140,6 @@
}
@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 {
res.assertOK();
diff --git a/javatests/com/google/gerrit/acceptance/ssh/PluginChangeFieldsIT.java b/javatests/com/google/gerrit/acceptance/ssh/PluginChangeFieldsIT.java
index 38293f9..009e05d 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/PluginChangeFieldsIT.java
+++ b/javatests/com/google/gerrit/acceptance/ssh/PluginChangeFieldsIT.java
@@ -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;
@@ -41,31 +40,12 @@
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(
id -> pluginInfosFromList(adminSshSession.exec(changeQueryCmd(id))),
@@ -85,12 +65,6 @@
}
@Test
- public void getMultipleChangesWithPluginDefinedAndChangeAttributes() throws Exception {
- getMultipleChangesWithPluginDefinedBulkAndChangeAttributes(
- () -> pluginInfosFromList(adminSshSession.exec("gerrit query --format json status:open")));
- }
-
- @Test
public void getMultipleChangesWithPluginDefinedAttributeInSingleCall() throws Exception {
getMultipleChangesWithPluginDefinedBulkAttributeInSingleCall(
() -> pluginInfosFromList(adminSshSession.exec("gerrit query --format json status:open")));
@@ -116,15 +90,6 @@
}
@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 {
List<Map<String, Object>> changeAttrs = getChangeAttrs(sshOutput);
