feat: improve handling of default values for ValueInjectionPoints (#4620

) * feat: improve handling of default values for ValueInjectionPoints * rename method, add javadoc
eclipse-edc · Nov 13, 2024 · cfc8e6f · cfc8e6f
1 parent 90362c9
commit cfc8e6f
Show file tree

Hide file tree

Showing 15 changed files with 102 additions and 47 deletions.
diff --git a/core/common/boot/src/main/java/org/eclipse/edc/boot/system/DependencyGraph.java b/core/common/boot/src/main/java/org/eclipse/edc/boot/system/DependencyGraph.java
@@ -121,7 +121,7 @@ public List<InjectionContainer<ServiceExtension>> of(List<ServiceExtension> exte
 
                         var defaultServiceProvider = defaultServiceProviders.get(injectionPoint.getType());
                         if (defaultServiceProvider != null) {
-                            injectionPoint.setDefaultServiceProvider(defaultServiceProvider);
+                            injectionPoint.setDefaultValueProvider(defaultServiceProvider);
                         }
                     })
                     .forEach(injectionPoint -> container.getInjectionPoints().add(injectionPoint));

diff --git a/...boot/src/main/java/org/eclipse/edc/boot/system/injection/ConfigurationInjectionPoint.java b/...boot/src/main/java/org/eclipse/edc/boot/system/injection/ConfigurationInjectionPoint.java
@@ -14,12 +14,13 @@
 
 package org.eclipse.edc.boot.system.injection;
 
-import org.eclipse.edc.boot.system.injection.lifecycle.ServiceProvider;
 import org.eclipse.edc.runtime.metamodel.annotation.Setting;
 import org.eclipse.edc.spi.result.AbstractResult;
 import org.eclipse.edc.spi.result.Result;
 import org.eclipse.edc.spi.system.ServiceExtensionContext;
+import org.eclipse.edc.spi.system.ValueProvider;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
@@ -88,15 +89,15 @@ public Result<Void> setTargetValue(Object value) {
      * Not used here, will always return null
      */
     @Override
-    public ServiceProvider getDefaultServiceProvider() {
+    public @Nullable ValueProvider getDefaultValueProvider() {
         return null;
     }
 
     /**
      * Not used here
      */
     @Override
-    public void setDefaultServiceProvider(ServiceProvider defaultServiceProvider) {
+    public void setDefaultValueProvider(ValueProvider defaultValueProvider) {
 
     }
 
@@ -182,7 +183,7 @@ private Predicate<Constructor<?>> constructorFilter(List<FieldValue> args) {
     private @NotNull List<FieldValue> resolveSettingsFields(ServiceExtensionContext context, Field[] fields) {
         return injectionPointsFrom(fields)
                 .map(ip -> {
-                    var val = ip.resolve(context, null /*the default supplier arg is not used anyway*/);
+                    var val = ip.resolve(context, new InjectionPointDefaultServiceSupplier());
                     var fieldName = ip.getTargetField().getName();
                     return new FieldValue(fieldName, val);
                 })

diff --git a/core/common/boot/src/main/java/org/eclipse/edc/boot/system/injection/InjectionPoint.java b/core/common/boot/src/main/java/org/eclipse/edc/boot/system/injection/InjectionPoint.java
@@ -14,9 +14,9 @@
 
 package org.eclipse.edc.boot.system.injection;
 
-import org.eclipse.edc.boot.system.injection.lifecycle.ServiceProvider;
 import org.eclipse.edc.spi.result.Result;
 import org.eclipse.edc.spi.system.ServiceExtensionContext;
+import org.eclipse.edc.spi.system.ValueProvider;
 import org.jetbrains.annotations.Nullable;
 
 import java.util.List;
@@ -57,22 +57,23 @@ public interface InjectionPoint<T> {
      *
      * @return the default service provider if any, null otherwise
      */
-    @Nullable ServiceProvider getDefaultServiceProvider();
+    @Nullable ValueProvider getDefaultValueProvider();
 
     /**
      * Sets the default service provider.
      *
-     * @param defaultServiceProvider the default service provider
+     * @param defaultValueProvider the default service provider
      */
-    void setDefaultServiceProvider(ServiceProvider defaultServiceProvider);
+    void setDefaultValueProvider(ValueProvider defaultValueProvider);
 
     /**
      * Resolves the value for an injected field from either the context or a default service supplier. For some injection points,
      * this may also return a (statically declared) default value.
      *
      * @param context                The {@link ServiceExtensionContext} from which the value is resolved.
-     * @param defaultServiceSupplier Some service dynamically resolve a default value in case the actual value isn't found on the context.
-     * @return The resolved value, or null if the injected field is not required..
+     * @param defaultServiceSupplier Provider for default values that can be resolved statically (e.g. through an annotation attribute) or
+     *                               dynamically (e.g. by instantiating an object or resolving from context).
+     * @return The resolved value, or null if the injected field is not required.
      * @throws EdcInjectionException in case the value could not be resolved
      */
     Object resolve(ServiceExtensionContext context, DefaultServiceSupplier defaultServiceSupplier);

diff --git a/...main/java/org/eclipse/edc/boot/system/injection/InjectionPointDefaultServiceSupplier.java b/...main/java/org/eclipse/edc/boot/system/injection/InjectionPointDefaultServiceSupplier.java
@@ -17,6 +17,8 @@
 import org.eclipse.edc.spi.system.ServiceExtensionContext;
 import org.jetbrains.annotations.Nullable;
 
+import static java.util.Optional.ofNullable;
+
 /**
  * Supplies the default {@link org.eclipse.edc.boot.system.injection.lifecycle.ServiceProvider} that has been stored in
  * the {@link InjectionPoint}
@@ -25,11 +27,11 @@ public class InjectionPointDefaultServiceSupplier implements DefaultServiceSuppl
 
     @Override
     public @Nullable Object provideFor(InjectionPoint<?> injectionPoint, ServiceExtensionContext context) {
-        var defaultService = injectionPoint.getDefaultServiceProvider();
+        var defaultService = injectionPoint.getDefaultValueProvider();
         if (injectionPoint.isRequired() && defaultService == null) {
             throw new EdcInjectionException("No default provider for required service " + injectionPoint.getType());
         }
-        return defaultService == null ? null : defaultService.register(context);
+        return ofNullable(defaultService).map(vp -> vp.get(context)).orElse(null);
     }
 
 }
diff --git a/...ommon/boot/src/main/java/org/eclipse/edc/boot/system/injection/ServiceInjectionPoint.java b/...ommon/boot/src/main/java/org/eclipse/edc/boot/system/injection/ServiceInjectionPoint.java
@@ -14,10 +14,11 @@
 
 package org.eclipse.edc.boot.system.injection;
 
-import org.eclipse.edc.boot.system.injection.lifecycle.ServiceProvider;
 import org.eclipse.edc.spi.result.Result;
 import org.eclipse.edc.spi.system.ServiceExtension;
 import org.eclipse.edc.spi.system.ServiceExtensionContext;
+import org.eclipse.edc.spi.system.ValueProvider;
+import org.jetbrains.annotations.Nullable;
 
 import java.lang.reflect.Field;
 import java.util.List;
@@ -36,7 +37,7 @@ public class ServiceInjectionPoint<T> implements InjectionPoint<T> {
     private final T instance;
     private final Field injectedField;
     private final boolean isRequired;
-    private ServiceProvider defaultServiceProvider;
+    private ValueProvider defaultServiceProvider;
 
     public ServiceInjectionPoint(T instance, Field injectedField) {
         this(instance, injectedField, true);
@@ -75,13 +76,13 @@ public Result<Void> setTargetValue(Object value) {
     }
 
     @Override
-    public ServiceProvider getDefaultServiceProvider() {
+    public @Nullable ValueProvider getDefaultValueProvider() {
         return defaultServiceProvider;
     }
 
     @Override
-    public void setDefaultServiceProvider(ServiceProvider defaultServiceProvider) {
-        this.defaultServiceProvider = defaultServiceProvider;
+    public void setDefaultValueProvider(ValueProvider defaultValueProvider) {
+        this.defaultServiceProvider = defaultValueProvider;
 
     }
 

diff --git a/.../common/boot/src/main/java/org/eclipse/edc/boot/system/injection/ValueInjectionPoint.java b/.../common/boot/src/main/java/org/eclipse/edc/boot/system/injection/ValueInjectionPoint.java
@@ -14,15 +14,18 @@
 
 package org.eclipse.edc.boot.system.injection;
 
-import org.eclipse.edc.boot.system.injection.lifecycle.ServiceProvider;
 import org.eclipse.edc.runtime.metamodel.annotation.Setting;
 import org.eclipse.edc.spi.result.Result;
 import org.eclipse.edc.spi.system.ServiceExtensionContext;
+import org.eclipse.edc.spi.system.ValueProvider;
+import org.jetbrains.annotations.Nullable;
 
 import java.lang.reflect.Field;
 import java.util.List;
 import java.util.Map;
 
+import static java.util.Optional.ofNullable;
+
 /**
  * Injection point for configuration values ("settings"). Configuration values must be basic data types and be annotated
  * with {@link Setting}, for example:
@@ -94,22 +97,25 @@ public Result<Void> setTargetValue(Object value) {
     }
 
     /**
-     * Not used here, always returns null;
+     * Returns a {@link ValueProvider} that takes the annotation's {@link Setting#defaultValue()} attribute or null
      *
-     * @return always {@code null}
+     * @return a nullable default value provider
      */
     @Override
-    public ServiceProvider getDefaultServiceProvider() {
+    public @Nullable ValueProvider getDefaultValueProvider() {
+        if (!Setting.NULL.equals(annotationValue.defaultValue())) {
+            return context -> annotationValue.defaultValue();
+        }
         return null;
     }
 
     /**
      * Not used here
      *
-     * @param defaultServiceProvider Ignored
+     * @param defaultValueProvider Ignored
      */
     @Override
-    public void setDefaultServiceProvider(ServiceProvider defaultServiceProvider) {
+    public void setDefaultValueProvider(ValueProvider defaultValueProvider) {
 
     }
 
@@ -125,15 +131,20 @@ public Object resolve(ServiceExtensionContext context, DefaultServiceSupplier de
         }
 
         // not found in config, but there is a default value
-        var def = annotationValue.defaultValue();
-        if (def != null && !def.trim().equals(Setting.NULL)) {
-            var msg = "Config value: no setting found for '%s', falling back to default value '%s'".formatted(key, def);
-            if (annotationValue.warnOnMissingConfig()) {
-                context.getMonitor().warning(msg);
-            } else {
-                context.getMonitor().debug(msg);
+        var def = ofNullable(defaultServiceSupplier)
+                .map(s -> s.provideFor(this, context))
+                .map(Object::toString);
+        if (def.isPresent()) {
+            var defaultValue = def.get();
+            if (!defaultValue.trim().equals(Setting.NULL)) {
+                var msg = "Config value: no setting found for '%s', falling back to default value '%s'".formatted(key, defaultValue);
+                if (annotationValue.warnOnMissingConfig()) {
+                    context.getMonitor().warning(msg);
+                } else {
+                    context.getMonitor().debug(msg);
+                }
+                return parseEntry(defaultValue, type);
             }
-            return parseEntry(def, type);
         }
 
         // neither in config, nor default val

diff --git a/.../main/java/org/eclipse/edc/boot/system/injection/lifecycle/ExtensionLifecycleManager.java b/.../main/java/org/eclipse/edc/boot/system/injection/lifecycle/ExtensionLifecycleManager.java
@@ -55,7 +55,7 @@ public static void bootServiceExtensions(List<InjectionContainer<ServiceExtensio
 
             var serviceProviders = container.getServiceProviders();
             if (serviceProviders != null) {
-                serviceProviders.forEach(serviceProvider -> serviceProvider.register(context));
+                serviceProviders.forEach(serviceProvider -> serviceProvider.get(context));
             }
         }
 

diff --git a/...n/boot/src/main/java/org/eclipse/edc/boot/system/injection/lifecycle/ServiceProvider.java b/...n/boot/src/main/java/org/eclipse/edc/boot/system/injection/lifecycle/ServiceProvider.java
@@ -18,26 +18,27 @@
 import org.eclipse.edc.runtime.metamodel.annotation.Provider;
 import org.eclipse.edc.spi.system.ServiceExtension;
 import org.eclipse.edc.spi.system.ServiceExtensionContext;
+import org.eclipse.edc.spi.system.ValueProvider;
 
 /**
  * Represent a service provider, that's a method annotated with the {@link Provider}
  *
- * @param method the provider method.
+ * @param method    the provider method.
  * @param extension the extension in which the method is contained.
  */
-public record ServiceProvider(ProviderMethod method, ServiceExtension extension) {
+public record ServiceProvider(ProviderMethod method, ServiceExtension extension) implements ValueProvider {
 
     /**
      * Call the method and register the service.
      *
      * @param context the service context.
      * @return the instantiated service.
      */
-    public Object register(ServiceExtensionContext context) {
+    @Override
+    public Object get(ServiceExtensionContext context) {
         var type = method.getReturnType();
         var service = method.invoke(extension, context);
         context.registerService(type, service);
         return service;
     }
-
 }
diff --git a/.../src/test/java/org/eclipse/edc/boot/system/injection/ConfigurationInjectionPointTest.java b/.../src/test/java/org/eclipse/edc/boot/system/injection/ConfigurationInjectionPointTest.java
@@ -57,12 +57,12 @@ void setTargetValue() throws IllegalAccessException {
     }
 
     @Test
-    void getDefaultServiceProvider() {
-        assertThat(injectionPoint.getDefaultServiceProvider()).isNull();
+    void getDefaultValueProvider() {
+        assertThat(injectionPoint.getDefaultValueProvider()).isNull();
     }
 
     @Test
-    void setDefaultServiceProvider() {
+    void setDefaultValueProvider() {
         //noop
     }
 

diff --git a/.../java/org/eclipse/edc/boot/system/injection/InjectionPointDefaultServiceSupplierTest.java b/.../java/org/eclipse/edc/boot/system/injection/InjectionPointDefaultServiceSupplierTest.java
@@ -32,8 +32,8 @@ class InjectionPointDefaultServiceSupplierTest {
     void shouldRegisterDefaultService() {
         var injectionPoint = new ServiceInjectionPoint<>("any", mock());
         ServiceProvider serviceProvider = mock();
-        when(serviceProvider.register(any())).thenReturn("service");
-        injectionPoint.setDefaultServiceProvider(serviceProvider);
+        when(serviceProvider.get(any())).thenReturn("service");
+        injectionPoint.setDefaultValueProvider(serviceProvider);
         ServiceExtensionContext context = mock();
 
         var service = supplier.provideFor(injectionPoint, context);

diff --git a/...mon/boot/src/test/java/org/eclipse/edc/boot/system/injection/ValueInjectionPointTest.java b/...mon/boot/src/test/java/org/eclipse/edc/boot/system/injection/ValueInjectionPointTest.java
@@ -52,7 +52,7 @@ void isRequired() {
 
         @Test
         void getDefaultServiceProvider() {
-            assertThat(getInjectionPoint().getDefaultServiceProvider()).isNull();
+            assertThat(getInjectionPoint().getDefaultValueProvider()).isNull();
         }
 
         @Test
@@ -78,7 +78,7 @@ void resolve_whenRequired_andNotFound_hasDefault() {
             when(contextMock.getConfig()).thenReturn(config);
             when(contextMock.getMonitor()).thenReturn(mock());
             var ip = createInjectionPoint(getInjectionPoint(), "requiredValWithDefault", ExtensionWithConfigValue.class);
-            assertThat(ip.resolve(contextMock, mock())).isEqualTo(ExtensionWithConfigValue.DEFAULT_VALUE);
+            assertThat(ip.resolve(contextMock, new InjectionPointDefaultServiceSupplier())).isEqualTo(ExtensionWithConfigValue.DEFAULT_VALUE);
         }
 
         @Test

diff --git a/...ot/src/test/java/org/eclipse/edc/boot/system/injection/lifecycle/ServiceProviderTest.java b/...ot/src/test/java/org/eclipse/edc/boot/system/injection/lifecycle/ServiceProviderTest.java
@@ -37,7 +37,7 @@ void shouldRegisterService() throws NoSuchMethodException {
         ServiceExtensionContext context = mock();
         var serviceProvider = new ServiceProvider(providerMethod, extension);
 
-        var registered = serviceProvider.register(context);
+        var registered = serviceProvider.get(context);
 
         assertThat(registered).isEqualTo(service);
         verify(context).registerService(TestService.class, service);
@@ -52,5 +52,6 @@ public TestService testService() {
 
     }
 
-    private interface TestService {}
+    private interface TestService {
+    }
 }
diff --git a/...on/connector-core/src/main/java/org/eclipse/edc/connector/core/CoreServicesExtension.java b/...on/connector-core/src/main/java/org/eclipse/edc/connector/core/CoreServicesExtension.java
@@ -58,12 +58,17 @@
 public class CoreServicesExtension implements ServiceExtension {
 
     public static final String NAME = "Core Services";
+
     @Setting(description = "The name of the claim key used to determine the participant identity", defaultValue = DEFAULT_IDENTITY_CLAIM_KEY)
     public static final String EDC_AGENT_IDENTITY_KEY = "edc.agent.identity.key";
+
     private static final String DEFAULT_EDC_HOSTNAME = "localhost";
+
     public static final String EDC_HOSTNAME = "edc.hostname";
+
     @Setting(description = "Connector hostname, which e.g. is used in referer urls", defaultValue = DEFAULT_EDC_HOSTNAME, key = EDC_HOSTNAME, warnOnMissingConfig = true)
     public static String hostname;
+
     @Inject
     private EventExecutorServiceContainer eventExecutorServiceContainer;
 

diff --git a/...on/junit/src/main/java/org/eclipse/edc/junit/extensions/DependencyInjectionExtension.java b/...on/junit/src/main/java/org/eclipse/edc/junit/extensions/DependencyInjectionExtension.java
@@ -27,6 +27,7 @@
 import org.junit.jupiter.api.extension.ParameterResolutionException;
 import org.junit.jupiter.api.extension.ParameterResolver;
 
+import static java.util.Optional.ofNullable;
 import static org.eclipse.edc.util.types.Cast.cast;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
@@ -51,7 +52,7 @@ public void beforeEach(ExtensionContext extensionContext) {
         context = spy(super.createServiceExtensionContext(ConfigFactory.empty()));
         context.initialize();
         factory = new ReflectiveObjectFactory(
-                new InjectorImpl((ip, c) -> mock(ip.getType())),
+                new InjectorImpl((ip, c) -> ofNullable(ip.getDefaultValueProvider()).map(vp -> vp.get(c)).orElseGet(() -> mock(ip.getType()))),
                 new InjectionPointScanner(),
                 context
         );