Introduce @DynamicItem.Final for immutable bindings
In some cases plugins would like to expose their API to other plugins,
for example to give access to internal state or to programatically
modify its configuration.
This can be potentially done currently with the usage of DynamicItem,
but that doesn't guarantee that the implementation will not be
overwritten by other plugins.
Also, one plugin may have specific expectations of where the
implementation of those interfaces must be found and
therefore exposing that constraint to Gerrit plugin load so that
only the allowed plugin is accepted for binding that item.
Introduce the @DynamicItem.Final annotation that, once assigned to a
type, requires the type to be bound at most once.
Any attempt to re-bind a DynamicItem.Final twice or from a plugin
which is not the expected one, would result in a Guice
ProvisionException.
Example:
@DynamicItem.Final(implementedByPlugin="foo-plugin")
public interface FooInterface {
String fooApi(String param);
}
Bug: Issue 338786480
Release-Notes: Introduce @DynamicItem.Final for allowing immutable dynamic bindings of interfaces to implementations in other plugins.
Change-Id: Ie7edf73e27fc6de898dbf248b9c4cda6b97e8153
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index 4fa5e68..8c1b1ce 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -144,6 +144,40 @@
This enables plugins to influence other plugins by customizing or extending the
their behaviour.
+When a plugin wants to expose an API that *must* not be further overridden by
+other plugins, it could use the additional annotation `@DynamicItem.Final` which
+also gives the option to further limit the name of the plugin that is designated
+to bind the only implementation available.
+
+For example, a `plugin B` may declare an API as `@DynamicItem.Final` which is then
+bound in its `ApiModule`.
+
+```
+ @DynamicItem.Final(implementedByPlugin = "plugin-b-impl")
+ public interface PluginAPI {}
+
+ public class PluginApiModule extends AbstractModule {
+ @Override
+ protected void configure() {
+ DynamicItem.itemOf(binder(), PluginAPI.class);
+ }
+ }
+```
+
+The above definition of the `PluginApi` would be allowed to bound only by
+the `plugin-b-impl` which would associate its implementation class.
+
+```
+public class PluginImpl implements PluginAPI {}
+
+ public class PluginImplModule extends AbstractModule {
+ @Override
+ protected void configure() {
+ DynamicItem.bind(binder(), PluginAPI.class).to(PluginImpl.class);
+ }
+ }
+```
+
*Gotchas and Limitations*:
- A `plugin A` depending on a `plugin B` (declaring a `Gerrit-ApiModule`),
diff --git a/java/com/google/gerrit/extensions/registration/DynamicItem.java b/java/com/google/gerrit/extensions/registration/DynamicItem.java
index 4464af7..4cc22f9 100644
--- a/java/com/google/gerrit/extensions/registration/DynamicItem.java
+++ b/java/com/google/gerrit/extensions/registration/DynamicItem.java
@@ -14,9 +14,12 @@
package com.google.gerrit.extensions.registration;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.gerrit.common.Nullable;
import com.google.inject.Binder;
+import com.google.inject.BindingAnnotation;
import com.google.inject.Key;
import com.google.inject.Provider;
import com.google.inject.ProvisionException;
@@ -25,6 +28,9 @@
import com.google.inject.binder.LinkedBindingBuilder;
import com.google.inject.util.Providers;
import com.google.inject.util.Types;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
import java.util.concurrent.atomic.AtomicReference;
/**
@@ -36,6 +42,15 @@
* exception is thrown.
*/
public class DynamicItem<T> {
+
+ /** Annotate a DynamicItem to be final and being bound at most once. */
+ @Target({ElementType.TYPE})
+ @Retention(RUNTIME)
+ @BindingAnnotation
+ public @interface Final {
+ String implementedByPlugin() default "";
+ }
+
/**
* Declare a singleton {@code DynamicItem<T>} with a binder.
*
diff --git a/java/com/google/gerrit/extensions/registration/PrivateInternals_DynamicTypes.java b/java/com/google/gerrit/extensions/registration/PrivateInternals_DynamicTypes.java
index 5b528cb..982ff98 100644
--- a/java/com/google/gerrit/extensions/registration/PrivateInternals_DynamicTypes.java
+++ b/java/com/google/gerrit/extensions/registration/PrivateInternals_DynamicTypes.java
@@ -14,12 +14,14 @@
package com.google.gerrit.extensions.registration;
+import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.inject.Binding;
import com.google.inject.Inject;
import com.google.inject.Injector;
import com.google.inject.Key;
+import com.google.inject.ProvisionException;
import com.google.inject.TypeLiteral;
import java.lang.reflect.ParameterizedType;
import java.util.ArrayList;
@@ -97,6 +99,26 @@
DynamicItem<Object> item = (DynamicItem<Object>) e.getValue();
for (Binding<Object> b : bindings(src, type)) {
+ Class<? super Object> rawType = type.getRawType();
+ DynamicItem.Final annotation = rawType.getAnnotation(DynamicItem.Final.class);
+ if (annotation != null) {
+ Object existingBinding = item.get();
+ if (existingBinding != null) {
+ throw new ProvisionException(
+ String.format(
+ "Attempting to bind a @DynamicItem.Final %s twice: it was already bound to %s and tried to bind again to %s",
+ rawType.getName(), existingBinding, b));
+ }
+
+ String implementedByPlugin = annotation.implementedByPlugin();
+ if (!Strings.isNullOrEmpty(implementedByPlugin)
+ && !implementedByPlugin.equals(pluginName)) {
+ throw new ProvisionException(
+ String.format(
+ "Attempting to bind a @DynamicItem.Final %s to unexpected plugin: it was supposed to be bound to %s plugin but tried bind to %s plugin",
+ rawType.getName(), implementedByPlugin, pluginName));
+ }
+ }
handles.add(item.set(b.getKey(), b.getProvider(), pluginName));
}
}
diff --git a/javatests/com/google/gerrit/extensions/BUILD b/javatests/com/google/gerrit/extensions/BUILD
index 1bb39c8..86fd295 100644
--- a/javatests/com/google/gerrit/extensions/BUILD
+++ b/javatests/com/google/gerrit/extensions/BUILD
@@ -8,6 +8,7 @@
"//java/com/google/gerrit/common:annotations",
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/extensions/common/testing:common-test-util",
+ "//java/com/google/gerrit/testing:gerrit-test-util",
"//lib:guava",
"//lib/guice",
"//lib/truth",
diff --git a/javatests/com/google/gerrit/extensions/registration/DynamicItemTest.java b/javatests/com/google/gerrit/extensions/registration/DynamicItemTest.java
new file mode 100644
index 0000000..828f6c1
--- /dev/null
+++ b/javatests/com/google/gerrit/extensions/registration/DynamicItemTest.java
@@ -0,0 +1,175 @@
+// Copyright (C) 2024 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.extensions.registration;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.common.Nullable;
+import com.google.inject.AbstractModule;
+import com.google.inject.Binder;
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.google.inject.ProvisionException;
+import com.google.inject.TypeLiteral;
+import java.util.function.Consumer;
+import org.junit.Test;
+
+public class DynamicItemTest {
+ private static final String PLUGIN_NAME = "plugin-name";
+
+ private static final String UNEXPECTED_PLUGIN_NAME = "unexpected-plugin";
+ private static final String DYNAMIC_ITEM_1 = "item-1";
+ private static final String DYNAMIC_ITEM_2 = "item-2";
+ private static final TypeLiteral<String> STRING_TYPE_LITERAL = new TypeLiteral<>() {};
+ private static final TypeLiteral<FinalItemApi> FINAL_ITEM_API_TYPE_LITERAL =
+ new TypeLiteral<>() {};
+ private static final TypeLiteral<FinalItemApiForPlugin>
+ FINAL_TARGET_PLUGIN_ITEM_API_TYPE_LITERAL = new TypeLiteral<>() {};
+
+ @DynamicItem.Final
+ private interface FinalItemApi {}
+
+ private static class FinalItemImpl implements FinalItemApi {
+ private static final FinalItemApi INSTANCE = new FinalItemImpl();
+ }
+
+ @DynamicItem.Final(implementedByPlugin = PLUGIN_NAME)
+ private interface FinalItemApiForPlugin {}
+
+ private static class FinalItemImplByPlugin implements FinalItemApiForPlugin {
+ private static final FinalItemApiForPlugin INSTANCE = new FinalItemImplByPlugin();
+ }
+
+ @Test
+ public void shouldAssignDynamicItemTwice() {
+ ImmutableMap<TypeLiteral<?>, DynamicItem<?>> bindings =
+ ImmutableMap.of(STRING_TYPE_LITERAL, DynamicItem.itemOf(String.class, null));
+
+ ImmutableList<RegistrationHandle> gerritRegistrations =
+ PrivateInternals_DynamicTypes.attachItems(
+ newInjector(
+ (binder) -> {
+ DynamicItem.itemOf(binder, String.class);
+ DynamicItem.bind(binder, String.class).toInstance(DYNAMIC_ITEM_1);
+ }),
+ PluginName.GERRIT,
+ bindings);
+ assertThat(gerritRegistrations).hasSize(1);
+ assertDynamicItem(bindings.get(STRING_TYPE_LITERAL), DYNAMIC_ITEM_1, PluginName.GERRIT);
+
+ ImmutableList<RegistrationHandle> pluginRegistrations =
+ PrivateInternals_DynamicTypes.attachItems(
+ newInjector(
+ (binder) -> DynamicItem.bind(binder, String.class).toInstance(DYNAMIC_ITEM_2)),
+ PLUGIN_NAME,
+ bindings);
+ assertThat(pluginRegistrations).hasSize(1);
+ assertDynamicItem(bindings.get(STRING_TYPE_LITERAL), DYNAMIC_ITEM_2, PLUGIN_NAME);
+ }
+
+ @Test
+ public void shouldFailToAssignFinalDynamicItemTwice() {
+ ImmutableMap<TypeLiteral<?>, DynamicItem<?>> bindings =
+ ImmutableMap.of(FINAL_ITEM_API_TYPE_LITERAL, DynamicItem.itemOf(FinalItemApi.class, null));
+
+ ImmutableList<RegistrationHandle> baseInjectorRegistrations =
+ PrivateInternals_DynamicTypes.attachItems(
+ newInjector(
+ (binder) -> {
+ DynamicItem.itemOf(binder, FinalItemApi.class);
+ DynamicItem.bind(binder, FinalItemApi.class).toInstance(FinalItemImpl.INSTANCE);
+ }),
+ PluginName.GERRIT,
+ bindings);
+ assertThat(baseInjectorRegistrations).hasSize(1);
+
+ ProvisionException ignored =
+ assertThrows(
+ ProvisionException.class,
+ () -> {
+ ImmutableList<RegistrationHandle> unused =
+ PrivateInternals_DynamicTypes.attachItems(
+ newInjector(
+ (binder) ->
+ DynamicItem.bind(binder, FinalItemApi.class)
+ .toInstance(FinalItemImpl.INSTANCE)),
+ PluginName.GERRIT,
+ bindings);
+ });
+ }
+
+ @Test
+ public void shouldFailToAssignFinalDynamicItemToDifferentPlugin() {
+ ImmutableMap<TypeLiteral<?>, DynamicItem<?>> bindings =
+ ImmutableMap.of(
+ FINAL_TARGET_PLUGIN_ITEM_API_TYPE_LITERAL,
+ DynamicItem.itemOf(FinalItemApi.class, null));
+
+ assertThrows(
+ ProvisionException.class,
+ () -> {
+ ImmutableList<RegistrationHandle> unused =
+ PrivateInternals_DynamicTypes.attachItems(
+ newInjector(
+ (binder) ->
+ DynamicItem.bind(binder, FinalItemApiForPlugin.class)
+ .toInstance(FinalItemImplByPlugin.INSTANCE)),
+ UNEXPECTED_PLUGIN_NAME,
+ bindings);
+ });
+ }
+
+ @Test
+ public void shouldAssignFinalDynamicItemToExpectedPlugin() {
+ ImmutableMap<TypeLiteral<?>, DynamicItem<?>> bindings =
+ ImmutableMap.of(
+ FINAL_TARGET_PLUGIN_ITEM_API_TYPE_LITERAL,
+ DynamicItem.itemOf(FinalItemApi.class, null));
+
+ ImmutableList<RegistrationHandle> pluginRegistrations =
+ PrivateInternals_DynamicTypes.attachItems(
+ newInjector(
+ (binder) ->
+ DynamicItem.bind(binder, FinalItemApiForPlugin.class)
+ .toInstance(FinalItemImplByPlugin.INSTANCE)),
+ PLUGIN_NAME,
+ bindings);
+ assertThat(pluginRegistrations).hasSize(1);
+ assertDynamicItem(
+ bindings.get(FINAL_TARGET_PLUGIN_ITEM_API_TYPE_LITERAL),
+ FinalItemImplByPlugin.INSTANCE,
+ PLUGIN_NAME);
+ }
+
+ private static Injector newInjector(Consumer<Binder> binding) {
+ return Guice.createInjector(
+ new AbstractModule() {
+ @Override
+ protected void configure() {
+ binding.accept(binder());
+ }
+ });
+ }
+
+ private static <T> void assertDynamicItem(
+ @Nullable DynamicItem<?> item, T itemVal, String pluginName) {
+ assertThat(item).isNotNull();
+ assertThat(item.get()).isEqualTo(itemVal);
+ assertThat(item.getPluginName()).isEqualTo(pluginName);
+ }
+}
