Introduce factory methods to provide object dependencies (eclipse-edc…

…#1150)

CHANGELOG.md

@@ -12,6 +12,7 @@ in the detailed section referring to by linking pull requests or issues.
 ### Overview
 * Bugfixing DataManagementApi
 * Build improvements
+* Improvements to Dependency Resolution
 
 ### Detailed Changes
 
@@ -22,6 +23,7 @@ in the detailed section referring to by linking pull requests or issues.
 * Check to avoid duplicated module names (#1190)
 * Implement Catalog service for Data Management API (#1195)
 * Add strict body validation for REST endpoints (#1128)
+* Dependency injection using factory/provider methods (#1056)
 
 #### Changed
 

core/base/src/main/java/org/eclipse/dataspaceconnector/core/CoreServicesExtension.java

@@ -36,9 +36,9 @@
 import org.eclipse.dataspaceconnector.spi.security.PrivateKeyResolver;
 import org.eclipse.dataspaceconnector.spi.system.BaseExtension;
 import org.eclipse.dataspaceconnector.spi.system.ExecutorInstrumentation;
-import org.eclipse.dataspaceconnector.spi.system.ExecutorInstrumentationImplementation;
 import org.eclipse.dataspaceconnector.spi.system.Hostname;
 import org.eclipse.dataspaceconnector.spi.system.Inject;
+import org.eclipse.dataspaceconnector.spi.system.Provider;
 import org.eclipse.dataspaceconnector.spi.system.Provides;
 import org.eclipse.dataspaceconnector.spi.system.ServiceExtension;
 import org.eclipse.dataspaceconnector.spi.system.ServiceExtensionContext;
@@ -61,9 +61,7 @@
         ParticipantAgentService.class,
         PolicyEngine.class,
         RemoteMessageDispatcherRegistry.class,
-        RetryPolicy.class,
         RuleBindingRegistry.class,
-        ExecutorInstrumentation.class,
 })
 public class CoreServicesExtension implements ServiceExtension {
 
@@ -98,9 +96,11 @@ public class CoreServicesExtension implements ServiceExtension {
      * An optional instrumentor for {@link ExecutorService}. Used by the optional {@code micrometer} module.
      */
     @Inject(required = false)
-    private ExecutorInstrumentationImplementation executorInstrumentationImplementation;
+    private ExecutorInstrumentation executorInstrumentation;
 
     private HealthCheckServiceImpl healthCheckService;
+    private RuleBindingRegistryImpl ruleBindingRegistry;
+    private ScopeFilter scopeFilter;
 
     @Override
     public String name() {
@@ -109,37 +109,19 @@ public String name() {
 
     @Override
     public void initialize(ServiceExtensionContext context) {
-        addHttpClient(context);
-        addRetryPolicy(context);
         registerParser(context);
-        var executorInstrumentation = registerExecutorInstrumentation(context);
         var config = getHealthCheckConfig(context);
 
         // health check service
         healthCheckService = new HealthCheckServiceImpl(config, executorInstrumentation);
         context.registerService(HealthCheckService.class, healthCheckService);
 
-        // remote message dispatcher registry
-        var dispatcherRegistry = new RemoteMessageDispatcherRegistryImpl();
-        context.registerService(RemoteMessageDispatcherRegistry.class, dispatcherRegistry);
+        ruleBindingRegistry = new RuleBindingRegistryImpl();
 
-        context.registerService(CommandHandlerRegistry.class, new CommandHandlerRegistryImpl());
-
-        var agentService = new ParticipantAgentServiceImpl();
-        context.registerService(ParticipantAgentService.class, agentService);
-
-        var bindingRegistry = new RuleBindingRegistryImpl();
-        context.registerService(RuleBindingRegistry.class, bindingRegistry);
-
-        var scopeFilter = new ScopeFilter(bindingRegistry);
+        scopeFilter = new ScopeFilter(ruleBindingRegistry);
 
         var typeManager = context.getTypeManager();
         PolicyRegistrationTypes.TYPES.forEach(typeManager::registerTypes);
-
-        var policyEngine = new PolicyEngineImpl(scopeFilter);
-        context.registerService(PolicyEngine.class, policyEngine);
-
-        registerHostname(context);
     }
 
     @Override
@@ -153,45 +135,59 @@ public void shutdown() {
         ServiceExtension.super.shutdown();
     }
 
-    private HealthCheckServiceConfiguration getHealthCheckConfig(ServiceExtensionContext context) {
+    @Provider(isDefault = true)
+    public ExecutorInstrumentation defaultInstrumentation() {
+        return ExecutorInstrumentation.noop();
+    }
 
-        return HealthCheckServiceConfiguration.Builder.newInstance()
-                .livenessPeriod(Duration.ofSeconds(context.getSetting(LIVENESS_PERIOD_SECONDS_SETTING, DEFAULT_DURATION)))
-                .startupStatusPeriod(Duration.ofSeconds(context.getSetting(STARTUP_PERIOD_SECONDS_SETTING, DEFAULT_DURATION)))
-                .readinessPeriod(Duration.ofSeconds(context.getSetting(READINESS_PERIOD_SECONDS_SETTING, DEFAULT_DURATION)))
-                .readinessPeriod(Duration.ofSeconds(context.getSetting(READINESS_PERIOD_SECONDS_SETTING, DEFAULT_DURATION)))
-                .threadPoolSize(context.getSetting(THREADPOOL_SIZE_SETTING, DEFAULT_TP_SIZE))
-                .build();
+    @Provider
+    public RetryPolicy<?> retryPolicy(ServiceExtensionContext context) {
+        var maxRetries = context.getSetting(MAX_RETRIES, 5);
+        var minBackoff = context.getSetting(BACKOFF_MIN_MILLIS, 500);
+        var maxBackoff = context.getSetting(BACKOFF_MAX_MILLIS, 10_000);
+
+        return new RetryPolicy<>()
+                .withMaxRetries(maxRetries)
+                .withBackoff(minBackoff, maxBackoff, ChronoUnit.MILLIS);
     }
 
-    private void registerHostname(ServiceExtensionContext context) {
+    @Provider
+    public Hostname hostname(ServiceExtensionContext context) {
         var hostname = context.getSetting(HOSTNAME_SETTING, DEFAULT_HOSTNAME);
         if (DEFAULT_HOSTNAME.equals(hostname)) {
             context.getMonitor().warning(String.format("Settings: No setting found for key '%s'. Using default value '%s'", HOSTNAME_SETTING, DEFAULT_HOSTNAME));
         }
-        context.registerService(Hostname.class, () -> hostname);
+        return () -> hostname;
     }
 
-    private void registerParser(ServiceExtensionContext context) {
-        var resolver = context.getService(PrivateKeyResolver.class);
-        resolver.addParser(PrivateKey.class, new DefaultPrivateKeyParseFunction());
+    @Provider
+    public RemoteMessageDispatcherRegistry remoteMessageDispatcherRegistry() {
+        return new RemoteMessageDispatcherRegistryImpl();
     }
 
-    private void addRetryPolicy(ServiceExtensionContext context) {
+    @Provider
+    public CommandHandlerRegistry commandHandlerRegistry() {
+        return new CommandHandlerRegistryImpl();
+    }
 
-        var maxRetries = context.getSetting(MAX_RETRIES, 5);
-        var minBackoff = context.getSetting(BACKOFF_MIN_MILLIS, 500);
-        var maxBackoff = context.getSetting(BACKOFF_MAX_MILLIS, 10_000);
+    @Provider
+    public ParticipantAgentService participantAgentService() {
+        return new ParticipantAgentServiceImpl();
+    }
 
-        var retryPolicy = new RetryPolicy<>()
-                .withMaxRetries(maxRetries)
-                .withBackoff(minBackoff, maxBackoff, ChronoUnit.MILLIS);
+    @Provider
+    public RuleBindingRegistry ruleBindingRegistry() {
 
-        context.registerService(RetryPolicy.class, retryPolicy);
+        return ruleBindingRegistry;
+    }
 
+    @Provider
+    public PolicyEngine policyEngine() {
+        return new PolicyEngineImpl(scopeFilter);
     }
 
-    private void addHttpClient(ServiceExtensionContext context) {
+    @Provider
+    public OkHttpClient addHttpClient(ServiceExtensionContext context) {
         var builder = new OkHttpClient.Builder()
                 .connectTimeout(30, TimeUnit.SECONDS)
                 .readTimeout(30, TimeUnit.SECONDS);
@@ -201,13 +197,23 @@ private void addHttpClient(ServiceExtensionContext context) {
         var client = builder.build();
 
         context.registerService(OkHttpClient.class, client);
+        return client;
     }
 
-    private ExecutorInstrumentation registerExecutorInstrumentation(ServiceExtensionContext context) {
-        var executorInstrumentation = ofNullable((ExecutorInstrumentation) this.executorInstrumentationImplementation)
-                .orElse(ExecutorInstrumentation.noop());
-        // Register ExecutorImplementation with default noop implementation if none available
-        context.registerService(ExecutorInstrumentation.class, executorInstrumentation);
-        return executorInstrumentation;
+    private HealthCheckServiceConfiguration getHealthCheckConfig(ServiceExtensionContext context) {
+
+        return HealthCheckServiceConfiguration.Builder.newInstance()
+                .livenessPeriod(Duration.ofSeconds(context.getSetting(LIVENESS_PERIOD_SECONDS_SETTING, DEFAULT_DURATION)))
+                .startupStatusPeriod(Duration.ofSeconds(context.getSetting(STARTUP_PERIOD_SECONDS_SETTING, DEFAULT_DURATION)))
+                .readinessPeriod(Duration.ofSeconds(context.getSetting(READINESS_PERIOD_SECONDS_SETTING, DEFAULT_DURATION)))
+                .readinessPeriod(Duration.ofSeconds(context.getSetting(READINESS_PERIOD_SECONDS_SETTING, DEFAULT_DURATION)))
+                .threadPoolSize(context.getSetting(THREADPOOL_SIZE_SETTING, DEFAULT_TP_SIZE))
+                .build();
     }
+
+    private void registerParser(ServiceExtensionContext context) {
+        var resolver = context.getService(PrivateKeyResolver.class);
+        resolver.addParser(PrivateKey.class, new DefaultPrivateKeyParseFunction());
+    }
+
 }

core/base/src/test/java/org/eclipse/dataspaceconnector/core/CoreServicesExtensionTest.java

@@ -17,21 +17,22 @@
 import org.eclipse.dataspaceconnector.core.security.DefaultPrivateKeyParseFunction;
 import org.eclipse.dataspaceconnector.policy.model.PolicyRegistrationTypes;
 import org.eclipse.dataspaceconnector.spi.security.PrivateKeyResolver;
+import org.eclipse.dataspaceconnector.spi.system.ExecutorInstrumentation;
 import org.eclipse.dataspaceconnector.spi.system.ServiceExtensionContext;
+import org.eclipse.dataspaceconnector.spi.system.injection.ObjectFactory;
 import org.eclipse.dataspaceconnector.spi.types.TypeManager;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
 
 import java.security.PrivateKey;
 
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
 
+@ExtendWith(TypeManagerDependencyInjectionExtension.class)
 class CoreServicesExtensionTest {
     private CoreServicesExtension extension;
     private ServiceExtensionContext context;
@@ -51,15 +52,14 @@ void verifyDefaultPrivateKeyParserIsRegistered() {
     }
 
     @BeforeEach
-    void setUp() {
-        extension = new CoreServicesExtension();
-        typeManager = spy(new TypeManager());
-        context = mock(ServiceExtensionContext.class);
+    void setUp(ServiceExtensionContext context, ObjectFactory factory) {
         privateKeyResolverMock = mock(PrivateKeyResolver.class);
-        when(context.getSetting(eq(CoreServicesExtension.MAX_RETRIES), anyInt())).thenReturn(1);
-        when(context.getSetting(eq(CoreServicesExtension.BACKOFF_MIN_MILLIS), anyInt())).thenReturn(1);
-        when(context.getSetting(eq(CoreServicesExtension.BACKOFF_MAX_MILLIS), anyInt())).thenReturn(2);
-        when(context.getService(PrivateKeyResolver.class)).thenReturn(privateKeyResolverMock);
-        when(context.getTypeManager()).thenReturn(typeManager);
+        context.registerService(PrivateKeyResolver.class, privateKeyResolverMock);
+
+        typeManager = context.getTypeManager(); //is already a spy!
+        context.registerService(ExecutorInstrumentation.class, mock(ExecutorInstrumentation.class));
+
+        this.context = context;
+        extension = factory.constructInstance(CoreServicesExtension.class);
     }
 }

core/base/src/test/java/org/eclipse/dataspaceconnector/core/TypeManagerDependencyInjectionExtension.java

@@ -0,0 +1,29 @@
+/*
+ *  Copyright (c) 2020 - 2022 Microsoft Corporation
+ *
+ *  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:
+ *       Microsoft Corporation - initial API and implementation
+ *
+ */
+
+package org.eclipse.dataspaceconnector.core;
+
+import org.eclipse.dataspaceconnector.junit.launcher.DependencyInjectionExtension;
+import org.eclipse.dataspaceconnector.spi.types.TypeManager;
+import org.jetbrains.annotations.NotNull;
+
+import static org.mockito.Mockito.spy;
+
+// needed for one particular test, to be able to verify on the TypeManager.
+public class TypeManagerDependencyInjectionExtension extends DependencyInjectionExtension {
+    @Override
+    protected @NotNull TypeManager createTypeManager() {
+        return spy(new TypeManager());
+    }
+}