feat(ServiceExtensionContext): freeze the ServiceExtensionContext before the prepare phase (#2023)

Author: wolf4ood

core/common/boot/src/main/java/org/eclipse/dataspaceconnector/boot/system/DefaultServiceExtensionContext.java

@@ -39,6 +39,7 @@ public class DefaultServiceExtensionContext implements ServiceExtensionContext {
 
     private final Map<Class<?>, Object> services = new HashMap<>();
     private final List<ConfigurationExtension> configurationExtensions;
+    private boolean isReadOnly = false;
     private String connectorId;
     private Config config;
 
@@ -51,6 +52,16 @@ public DefaultServiceExtensionContext(TypeManager typeManager, Monitor monitor,
         registerService(Clock.class, Clock.systemUTC());
     }
 
+    @Override
+    public Config getConfig(String path) {
+        return config.getConfig(path);
+    }
+
+    @Override
+    public void freeze() {
+        isReadOnly = true;
+    }
+
     @Override
     public String getConnectorId() {
         return connectorId;
@@ -80,6 +91,9 @@ public <T> T getService(Class<T> type, boolean isOptional) {
 
     @Override
     public <T> void registerService(Class<T> type, T service) {
+        if (isReadOnly) {
+            throw new EdcException("Cannot register service " + type.getName() + ", the ServiceExtensionContext is in read-only mode.");
+        }
         if (hasService(type)) {
             getMonitor().warning("A service of the type " + type.getCanonicalName() + " was already registered and has now been replaced with a " + service.getClass().getSimpleName() + " instance.");
         }
@@ -96,11 +110,6 @@ public void initialize() {
         connectorId = getSetting("edc.connector.name", "edc-" + UUID.randomUUID());
     }
 
-    @Override
-    public Config getConfig(String path) {
-        return config.getConfig(path);
-    }
-
     // this method exists so that getting env vars can be mocked during testing
     protected Map<String, String> getEnvironmentVariables() {
         return System.getenv();
@@ -118,5 +127,4 @@ private Config loadConfig() {
 
         return config.merge(environmentConfig).merge(systemPropertyConfig);
     }
-
 }

core/common/boot/src/main/java/org/eclipse/dataspaceconnector/boot/system/ExtensionLoader.java

@@ -66,6 +66,8 @@ public static void bootServiceExtensions(List<InjectionContainer<ServiceExtensio
                 .map(ExtensionLifecycleManager::provide)
                 .collect(Collectors.toList());
 
+        context.freeze();
+
         var preparedExtensions = lifeCycles.stream().map(ExtensionLifecycleManager::prepare).collect(Collectors.toList());
         preparedExtensions.forEach(ExtensionLifecycleManager::start);
     }

core/common/boot/src/test/java/org/eclipse/dataspaceconnector/boot/system/DefaultServiceExtensionContextTest.java

@@ -15,6 +15,7 @@
 
 package org.eclipse.dataspaceconnector.boot.system;
 
+import org.eclipse.dataspaceconnector.spi.EdcException;
 import org.eclipse.dataspaceconnector.spi.monitor.Monitor;
 import org.eclipse.dataspaceconnector.spi.system.ConfigurationExtension;
 import org.eclipse.dataspaceconnector.spi.system.configuration.Config;
@@ -30,13 +31,14 @@
 import java.util.Map;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 class DefaultServiceExtensionContextTest {
 
-    private DefaultServiceExtensionContext context;
     private final ConfigurationExtension configuration = mock(ConfigurationExtension.class);
+    private DefaultServiceExtensionContext context;
 
     @BeforeEach
     void setUp() {
@@ -167,4 +169,16 @@ void loadConfig_systemPropOverwritesEnvVar() {
         }
     }
 
+    @Test
+    void registerService_throwsWhenFrozen() {
+        when(configuration.getConfig()).thenReturn(ConfigFactory.empty());
+        context.initialize();
+
+        context.freeze();
+        assertThatThrownBy(() -> context.registerService(Object.class, new Object() {
+        })).isInstanceOf(EdcException.class).hasMessageStartingWith("Cannot register service");
+
+
+    }
+
 }

extensions/common/junit/src/test/java/org/eclipse/dataspaceconnector/junit/extensions/EdcExtensionTest.java

@@ -15,12 +15,16 @@
 package org.eclipse.dataspaceconnector.junit.extensions;
 
 import okhttp3.OkHttpClient;
+import org.eclipse.dataspaceconnector.junit.testfixtures.MockVault;
+import org.eclipse.dataspaceconnector.spi.EdcException;
+import org.eclipse.dataspaceconnector.spi.security.Vault;
 import org.eclipse.dataspaceconnector.spi.system.MonitorExtension;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.startsWith;
 import static org.mockito.Mockito.atLeastOnce;
 import static org.mockito.Mockito.mock;
@@ -31,6 +35,7 @@ class EdcExtensionTest {
 
     private OkHttpClient testClient;
 
+
     @BeforeEach
     void setUp(EdcExtension extension) {
         testClient = mock(OkHttpClient.class);
@@ -46,4 +51,11 @@ void registerServiceMock_serviceAlreadyExists(EdcExtension extension) {
         assertThat(extension.getContext().getService(OkHttpClient.class)).isEqualTo(testClient);
         verify(mockedMonitor, atLeastOnce()).warning(startsWith("TestServiceExtensionContext: A service mock was registered for type okhttp3.OkHttpClient"));
     }
-}
+
+    @Test
+    void registerServiceMock_serviceContextReadOnlyMode(EdcExtension extension) {
+        assertThatThrownBy(() -> extension.getContext().registerService(Vault.class, new MockVault()))
+                .isInstanceOf(EdcException.class)
+                .hasMessageStartingWith("Cannot register service");
+    }
+}

spi/common/core-spi/src/main/java/org/eclipse/dataspaceconnector/spi/system/ServiceExtensionContext.java

@@ -26,6 +26,14 @@
  */
 public interface ServiceExtensionContext extends SettingResolver {
 
+    /**
+     * Freeze the context. It should mark the ServiceExtensionContext as read-only, preventing the registration
+     * of new services after the initialization phase
+     */
+    default void freeze() {
+
+    }
+
     /**
      * Fetches the unique ID of the connector. If the {@code dataspaceconnector.connector.name} config value has been set, that value is returned; otherwise  a random
      * name is chosen.