From 62403329d7e0635bb6d38f69bc79d1b3a094387c Mon Sep 17 00:00:00 2001 From: Paul Latzelsperger Date: Tue, 5 Nov 2024 15:51:23 +0100 Subject: [PATCH] feat: improve handling of default values for ValueInjectionPoints --- .../edc/boot/system/DependencyGraph.java | 2 +- .../ConfigurationInjectionPoint.java | 9 +++-- .../boot/system/injection/InjectionPoint.java | 13 ++++--- .../InjectionPointDefaultServiceSupplier.java | 6 ++- .../injection/ServiceInjectionPoint.java | 11 +++--- .../system/injection/ValueInjectionPoint.java | 39 ++++++++++++------- .../lifecycle/ExtensionLifecycleManager.java | 2 +- .../injection/lifecycle/ServiceProvider.java | 9 +++-- .../ConfigurationInjectionPointTest.java | 6 +-- ...ectionPointDefaultServiceSupplierTest.java | 4 +- .../injection/ValueInjectionPointTest.java | 4 +- .../lifecycle/ServiceProviderTest.java | 5 ++- .../connector/core/CoreServicesExtension.java | 5 +++ .../DependencyInjectionExtension.java | 3 +- .../eclipse/edc/spi/system/ValueProvider.java | 20 ++++++++++ 15 files changed, 91 insertions(+), 47 deletions(-) create mode 100644 spi/common/boot-spi/src/main/java/org/eclipse/edc/spi/system/ValueProvider.java 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 index 61ab48ff0fe..eaaac2872a0 100644 --- 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> of(List 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/core/common/boot/src/main/java/org/eclipse/edc/boot/system/injection/ConfigurationInjectionPoint.java b/core/common/boot/src/main/java/org/eclipse/edc/boot/system/injection/ConfigurationInjectionPoint.java index 6f61ecf6666..f929e07342e 100644 --- a/core/common/boot/src/main/java/org/eclipse/edc/boot/system/injection/ConfigurationInjectionPoint.java +++ b/core/common/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,7 +89,7 @@ public Result setTargetValue(Object value) { * Not used here, will always return null */ @Override - public ServiceProvider getDefaultServiceProvider() { + public @Nullable ValueProvider getDefaultValueProvider() { return null; } @@ -96,7 +97,7 @@ public ServiceProvider getDefaultServiceProvider() { * Not used here */ @Override - public void setDefaultServiceProvider(ServiceProvider defaultServiceProvider) { + public void setDefaultValueProvider(ValueProvider defaultValueProvider) { } @@ -182,7 +183,7 @@ private Predicate> constructorFilter(List args) { private @NotNull List 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 index d957a8e46ec..1966829c7ac 100644 --- 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 { * * @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/core/common/boot/src/main/java/org/eclipse/edc/boot/system/injection/InjectionPointDefaultServiceSupplier.java b/core/common/boot/src/main/java/org/eclipse/edc/boot/system/injection/InjectionPointDefaultServiceSupplier.java index 2ac380421b9..86d65a8b47e 100644 --- a/core/common/boot/src/main/java/org/eclipse/edc/boot/system/injection/InjectionPointDefaultServiceSupplier.java +++ b/core/common/boot/src/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.apply(context)).orElse(null); } } diff --git a/core/common/boot/src/main/java/org/eclipse/edc/boot/system/injection/ServiceInjectionPoint.java b/core/common/boot/src/main/java/org/eclipse/edc/boot/system/injection/ServiceInjectionPoint.java index d553b53428a..d22ca516245 100644 --- a/core/common/boot/src/main/java/org/eclipse/edc/boot/system/injection/ServiceInjectionPoint.java +++ b/core/common/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 implements InjectionPoint { 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 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/core/common/boot/src/main/java/org/eclipse/edc/boot/system/injection/ValueInjectionPoint.java b/core/common/boot/src/main/java/org/eclipse/edc/boot/system/injection/ValueInjectionPoint.java index 9ef257db2a4..df7b151c187 100644 --- a/core/common/boot/src/main/java/org/eclipse/edc/boot/system/injection/ValueInjectionPoint.java +++ b/core/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 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/core/common/boot/src/main/java/org/eclipse/edc/boot/system/injection/lifecycle/ExtensionLifecycleManager.java b/core/common/boot/src/main/java/org/eclipse/edc/boot/system/injection/lifecycle/ExtensionLifecycleManager.java index cb8857680f3..9e43c820242 100644 --- a/core/common/boot/src/main/java/org/eclipse/edc/boot/system/injection/lifecycle/ExtensionLifecycleManager.java +++ b/core/common/boot/src/main/java/org/eclipse/edc/boot/system/injection/lifecycle/ExtensionLifecycleManager.java @@ -55,7 +55,7 @@ public static void bootServiceExtensions(List serviceProvider.register(context)); + serviceProviders.forEach(serviceProvider -> serviceProvider.apply(context)); } } diff --git a/core/common/boot/src/main/java/org/eclipse/edc/boot/system/injection/lifecycle/ServiceProvider.java b/core/common/boot/src/main/java/org/eclipse/edc/boot/system/injection/lifecycle/ServiceProvider.java index 0961fb274a5..9d27b461328 100644 --- a/core/common/boot/src/main/java/org/eclipse/edc/boot/system/injection/lifecycle/ServiceProvider.java +++ b/core/common/boot/src/main/java/org/eclipse/edc/boot/system/injection/lifecycle/ServiceProvider.java @@ -18,14 +18,15 @@ 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. @@ -33,11 +34,11 @@ public record ServiceProvider(ProviderMethod method, ServiceExtension extension) * @param context the service context. * @return the instantiated service. */ - public Object register(ServiceExtensionContext context) { + @Override + public Object apply(ServiceExtensionContext context) { var type = method.getReturnType(); var service = method.invoke(extension, context); context.registerService(type, service); return service; } - } diff --git a/core/common/boot/src/test/java/org/eclipse/edc/boot/system/injection/ConfigurationInjectionPointTest.java b/core/common/boot/src/test/java/org/eclipse/edc/boot/system/injection/ConfigurationInjectionPointTest.java index ca39b9003fe..9cd7773752c 100644 --- a/core/common/boot/src/test/java/org/eclipse/edc/boot/system/injection/ConfigurationInjectionPointTest.java +++ b/core/common/boot/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/core/common/boot/src/test/java/org/eclipse/edc/boot/system/injection/InjectionPointDefaultServiceSupplierTest.java b/core/common/boot/src/test/java/org/eclipse/edc/boot/system/injection/InjectionPointDefaultServiceSupplierTest.java index a7f3019253a..7116785b5c6 100644 --- a/core/common/boot/src/test/java/org/eclipse/edc/boot/system/injection/InjectionPointDefaultServiceSupplierTest.java +++ b/core/common/boot/src/test/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.apply(any())).thenReturn("service"); + injectionPoint.setDefaultValueProvider(serviceProvider); ServiceExtensionContext context = mock(); var service = supplier.provideFor(injectionPoint, context); diff --git a/core/common/boot/src/test/java/org/eclipse/edc/boot/system/injection/ValueInjectionPointTest.java b/core/common/boot/src/test/java/org/eclipse/edc/boot/system/injection/ValueInjectionPointTest.java index a170461a2a2..dc89536a36c 100644 --- a/core/common/boot/src/test/java/org/eclipse/edc/boot/system/injection/ValueInjectionPointTest.java +++ b/core/common/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/core/common/boot/src/test/java/org/eclipse/edc/boot/system/injection/lifecycle/ServiceProviderTest.java b/core/common/boot/src/test/java/org/eclipse/edc/boot/system/injection/lifecycle/ServiceProviderTest.java index ddec2408b60..8a832aef718 100644 --- a/core/common/boot/src/test/java/org/eclipse/edc/boot/system/injection/lifecycle/ServiceProviderTest.java +++ b/core/common/boot/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.apply(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/core/common/connector-core/src/main/java/org/eclipse/edc/connector/core/CoreServicesExtension.java b/core/common/connector-core/src/main/java/org/eclipse/edc/connector/core/CoreServicesExtension.java index b887cc84807..3c98660ae93 100644 --- a/core/common/connector-core/src/main/java/org/eclipse/edc/connector/core/CoreServicesExtension.java +++ b/core/common/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/core/common/junit/src/main/java/org/eclipse/edc/junit/extensions/DependencyInjectionExtension.java b/core/common/junit/src/main/java/org/eclipse/edc/junit/extensions/DependencyInjectionExtension.java index 966e27ed791..3320d39be21 100644 --- a/core/common/junit/src/main/java/org/eclipse/edc/junit/extensions/DependencyInjectionExtension.java +++ b/core/common/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.apply(c)).orElseGet(() -> mock(ip.getType()))), new InjectionPointScanner(), context ); diff --git a/spi/common/boot-spi/src/main/java/org/eclipse/edc/spi/system/ValueProvider.java b/spi/common/boot-spi/src/main/java/org/eclipse/edc/spi/system/ValueProvider.java new file mode 100644 index 00000000000..c0a1adbed3b --- /dev/null +++ b/spi/common/boot-spi/src/main/java/org/eclipse/edc/spi/system/ValueProvider.java @@ -0,0 +1,20 @@ +/* + * Copyright (c) 2024 Bayerische Motoren Werke Aktiengesellschaft (BMW AG) + * + * This program and the accompanying materials are made available under the + * terms of the Apache License, Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + * + * Contributors: + * Bayerische Motoren Werke Aktiengesellschaft (BMW AG) - initial API and implementation + * + */ + +package org.eclipse.edc.spi.system; + +@FunctionalInterface +public interface ValueProvider { + Object apply(ServiceExtensionContext context); +}