This is an automated email from the ASF dual-hosted git repository. udo pushed a commit to branch feature/GEODE-8067 in repository https://gitbox.apache.org/repos/asf/geode.git
commit 2004f82fe73b83b81774f93af2a73ecd579c28a5 Author: Patrick Johnson <pjohn...@pivotal.io> AuthorDate: Fri May 22 07:31:59 2020 -0700 GEODE-8137 - Implement loadService. (#5136) --- .../integrationTest/resources/assembly_content.txt | 2 +- .../geode/services/module/ModuleService.java | 18 ++++ geode-modules/build.gradle | 3 + .../services/module/impl/GeodeModuleLoader.java | 2 +- ...uleService.java => JBossModuleServiceImpl.java} | 57 +++++++++++-- .../java/org/apache/geode/InvalidService.java} | 2 +- .../java/org/apache/geode/TestService.java} | 3 +- ...ceTest.java => JBossModuleServiceImplTest.java} | 95 +++++++++++++++++++++- .../module1/java/org/apache/geode/Module1.java | 6 +- .../META-INF/services/org.apache.geode.TestService | 1 + .../apache/geode}/Module2.java | 6 +- .../META-INF/services/org.apache.geode.TestService | 1 + 12 files changed, 182 insertions(+), 14 deletions(-) diff --git a/geode-assembly/src/integrationTest/resources/assembly_content.txt b/geode-assembly/src/integrationTest/resources/assembly_content.txt index 60c602b..8b92c69 100644 --- a/geode-assembly/src/integrationTest/resources/assembly_content.txt +++ b/geode-assembly/src/integrationTest/resources/assembly_content.txt @@ -975,7 +975,7 @@ javadoc/org/apache/geode/services/module/ModuleDescriptor.Builder.html javadoc/org/apache/geode/services/module/ModuleDescriptor.html javadoc/org/apache/geode/services/module/ModuleService.html javadoc/org/apache/geode/services/module/impl/GeodeModuleLoader.html -javadoc/org/apache/geode/services/module/impl/JBossModuleService.html +javadoc/org/apache/geode/services/module/impl/JBossModuleServiceImpl.html javadoc/org/apache/geode/services/module/impl/package-frame.html javadoc/org/apache/geode/services/module/impl/package-summary.html javadoc/org/apache/geode/services/module/impl/package-tree.html diff --git a/geode-common-services/src/main/java/org/apache/geode/services/module/ModuleService.java b/geode-common-services/src/main/java/org/apache/geode/services/module/ModuleService.java index cd295c0..81361be 100644 --- a/geode-common-services/src/main/java/org/apache/geode/services/module/ModuleService.java +++ b/geode-common-services/src/main/java/org/apache/geode/services/module/ModuleService.java @@ -15,6 +15,8 @@ package org.apache.geode.services.module; +import java.util.List; + import org.apache.geode.annotations.Experimental; /** @@ -33,4 +35,20 @@ public interface ModuleService { * @return true on success, false if the module could not be loaded. */ boolean loadModule(ModuleDescriptor moduleDescriptor); + + /** + * Unloads a previously loaded module. + * + * @param moduleName name of the module to be unloaded. + * @return true on success, false if the module could not be unloaded. + */ + boolean unloadModule(String moduleName); + + /** + * Loads and returns a service instance for an interface. + * + * @param service interface type to load and instantiate an implementation of. + * @return An instance of an implementation of service + */ + <T> List<T> loadService(Class<T> service); } diff --git a/geode-modules/build.gradle b/geode-modules/build.gradle index 319c0bd..17572a8 100644 --- a/geode-modules/build.gradle +++ b/geode-modules/build.gradle @@ -94,4 +94,7 @@ dependencies { implementation('org.apache.logging.log4j:log4j-core') compile('org.jboss.modules:jboss-modules') + + module1Compile(sourceSets.test.output) + module2Compile(sourceSets.test.output) } diff --git a/geode-modules/src/main/java/org/apache/geode/services/module/impl/GeodeModuleLoader.java b/geode-modules/src/main/java/org/apache/geode/services/module/impl/GeodeModuleLoader.java index 913df0c..a9d65c9 100644 --- a/geode-modules/src/main/java/org/apache/geode/services/module/impl/GeodeModuleLoader.java +++ b/geode-modules/src/main/java/org/apache/geode/services/module/impl/GeodeModuleLoader.java @@ -27,7 +27,7 @@ import org.jboss.modules.ModuleSpec; import org.apache.geode.annotations.Experimental; /** - * {@link ModuleLoader} for use by {@link JBossModuleService}. + * {@link ModuleLoader} for use by {@link JBossModuleServiceImpl}. */ @Experimental public class GeodeModuleLoader extends DelegatingModuleLoader { diff --git a/geode-modules/src/main/java/org/apache/geode/services/module/impl/JBossModuleService.java b/geode-modules/src/main/java/org/apache/geode/services/module/impl/JBossModuleServiceImpl.java similarity index 68% rename from geode-modules/src/main/java/org/apache/geode/services/module/impl/JBossModuleService.java rename to geode-modules/src/main/java/org/apache/geode/services/module/impl/JBossModuleServiceImpl.java index 9715709..c7be4c5 100644 --- a/geode-modules/src/main/java/org/apache/geode/services/module/impl/JBossModuleService.java +++ b/geode-modules/src/main/java/org/apache/geode/services/module/impl/JBossModuleServiceImpl.java @@ -17,15 +17,19 @@ package org.apache.geode.services.module.impl; import java.io.IOException; import java.util.HashMap; +import java.util.LinkedList; +import java.util.List; import java.util.Map; import java.util.jar.JarFile; import org.apache.logging.log4j.Logger; +import org.jboss.modules.DependencySpec; import org.jboss.modules.LocalDependencySpecBuilder; import org.jboss.modules.Module; import org.jboss.modules.ModuleDependencySpecBuilder; import org.jboss.modules.ModuleLoadException; import org.jboss.modules.ModuleSpec; +import org.jboss.modules.PathUtils; import org.jboss.modules.ResourceLoader; import org.jboss.modules.ResourceLoaderSpec; import org.jboss.modules.ResourceLoaders; @@ -40,7 +44,7 @@ import org.apache.geode.services.module.ModuleService; * Implementation of {@link ModuleService} using JBoss-Modules. */ @Experimental -public class JBossModuleService implements ModuleService { +public class JBossModuleServiceImpl implements ModuleService { private final Map<String, Module> modules = new HashMap<>(); @@ -48,11 +52,11 @@ public class JBossModuleService implements ModuleService { private final Logger logger; - public JBossModuleService() { + public JBossModuleServiceImpl() { this(LogService.getLogger()); } - public JBossModuleService(Logger logger) { + public JBossModuleServiceImpl(Logger logger) { this.logger = logger; } @@ -70,6 +74,7 @@ public class JBossModuleService implements ModuleService { return false; } + // Setting up new module. ModuleSpec.Builder builder = ModuleSpec.build(moduleDescriptor.getVersionedName()); builder.setVersion(Version.parse(moduleDescriptor.getVersion())); builder.addDependency(new LocalDependencySpecBuilder() @@ -77,13 +82,17 @@ public class JBossModuleService implements ModuleService { .setExport(true) .build()); + // Add dependencies to the module. moduleDescriptor.getDependedOnModules().forEach(dependency -> { logger.debug(String.format("Adding dependency on module %s", dependency)); builder.addDependency(new ModuleDependencySpecBuilder() + .setExport(true) + .setImportServices(true) .setName(dependency) .build()); }); + // Add resources to the module. try { for (String source : moduleDescriptor.getSources()) { logger.debug(String.format("Adding resource %s to module", source)); @@ -92,18 +101,24 @@ public class JBossModuleService implements ModuleService { builder.addResourceRoot(ResourceLoaderSpec.createResourceLoaderSpec(resourceLoader)); } } catch (IOException e) { - logger.error(e); + logger.error(e.getMessage(), e); return false; } + // Add dependency on the system classloader so modules can access classes that aren't in + // modules. + builder.addDependency(DependencySpec.createSystemDependencySpec(PathUtils.getPathSet(null))); + + // Build and register the ModuleSpec ModuleSpec moduleSpec = builder.create(); moduleLoader.addModuleSpec(moduleSpec); + // Load the module and add it to the modules map. try { modules.put(moduleDescriptor.getVersionedName(), moduleLoader.loadModule(moduleSpec.getName())); } catch (ModuleLoadException e) { - logger.error(e); + logger.error(e.getMessage(), e); return false; } @@ -112,4 +127,36 @@ public class JBossModuleService implements ModuleService { return true; } + + @Override + public boolean unloadModule(String moduleName) { + return false; + } + + @Override + public <T> List<T> loadService(Class<T> service) { + List<T> serviceImpls = new LinkedList<>(); + + // Iterate over all the modules looking for implementations of service. + modules.values().forEach((module) -> { + module.loadService(service).forEach((impl) -> { + // Check if class is already loaded. + // Modules with dependencies can cause duplicates without this check. + boolean duplicate = false; + for (T serviceImpl : serviceImpls) { + if (serviceImpl.getClass() == impl.getClass()) { + duplicate = true; + break; + } + } + + // If impl is not a duplicate, add it to the list to return. + if (!duplicate) { + serviceImpls.add(impl); + } + }); + }); + + return serviceImpls; + } } diff --git a/geode-modules/src/testModules/module2/java/org.apache.geode/Module2.java b/geode-modules/src/test/java/org/apache/geode/InvalidService.java similarity index 96% copy from geode-modules/src/testModules/module2/java/org.apache.geode/Module2.java copy to geode-modules/src/test/java/org/apache/geode/InvalidService.java index 9cbac0f..f6deaeb 100644 --- a/geode-modules/src/testModules/module2/java/org.apache.geode/Module2.java +++ b/geode-modules/src/test/java/org/apache/geode/InvalidService.java @@ -15,5 +15,5 @@ package org.apache.geode; -public class Module2 { +public interface InvalidService { } diff --git a/geode-modules/src/testModules/module1/java/org/apache/geode/Module1.java b/geode-modules/src/test/java/org/apache/geode/TestService.java similarity index 94% copy from geode-modules/src/testModules/module1/java/org/apache/geode/Module1.java copy to geode-modules/src/test/java/org/apache/geode/TestService.java index 684a71c..55b40e6 100644 --- a/geode-modules/src/testModules/module1/java/org/apache/geode/Module1.java +++ b/geode-modules/src/test/java/org/apache/geode/TestService.java @@ -15,5 +15,6 @@ package org.apache.geode; -public class Module1 { +public interface TestService { + String sayHello(); } diff --git a/geode-modules/src/test/java/org/apache/geode/services/module/impl/JBossModuleServiceTest.java b/geode-modules/src/test/java/org/apache/geode/services/module/impl/JBossModuleServiceImplTest.java similarity index 82% rename from geode-modules/src/test/java/org/apache/geode/services/module/impl/JBossModuleServiceTest.java rename to geode-modules/src/test/java/org/apache/geode/services/module/impl/JBossModuleServiceImplTest.java index bca62af..38bf0b3 100644 --- a/geode-modules/src/test/java/org/apache/geode/services/module/impl/JBossModuleServiceTest.java +++ b/geode-modules/src/test/java/org/apache/geode/services/module/impl/JBossModuleServiceImplTest.java @@ -18,14 +18,18 @@ package org.apache.geode.services.module.impl; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import java.util.List; + import org.jboss.modules.Module; import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.apache.geode.InvalidService; +import org.apache.geode.TestService; import org.apache.geode.services.module.ModuleDescriptor; -public class JBossModuleServiceTest { +public class JBossModuleServiceImplTest { private static final String MODULE1_PATH = System.getProperty("user.dir") + "/../libs/module1.jar"; @@ -36,11 +40,14 @@ public class JBossModuleServiceTest { private static final String MODULE4_PATH = System.getProperty("user.dir") + "/../libs/module4.jar"; - private JBossModuleService moduleService; + private static final String MODULE1_MESSAGE = "Hello from Module1!"; + private static final String MODULE2_MESSAGE = "Hello from Module2!"; + + private JBossModuleServiceImpl moduleService; @Before public void setup() { - moduleService = new JBossModuleService(); + moduleService = new JBossModuleServiceImpl(); } @After @@ -399,4 +406,86 @@ public class JBossModuleServiceTest { assertThat(module2).isNotNull(); assertThat(module2.getName()).isEqualTo(module2Descriptor.getVersionedName()); } + + @Test + public void loadServiceNoModulesLoaded() { + assertThat(moduleService.loadService(TestService.class)).isEmpty(); + } + + @Test + public void loadServiceNoModulesImplementService() { + assertThat(moduleService.loadService(InvalidService.class)).isEmpty(); + } + + @Test + public void loadServiceFromSingleModule() { + ModuleDescriptor module1Descriptor = new ModuleDescriptor.Builder("module1", "1.0") + .fromSources(MODULE1_PATH) + .build(); + moduleService.loadModule(module1Descriptor); + + List<TestService> serviceList = moduleService.loadService(TestService.class); + assertThat(serviceList.size()).isEqualTo(1); + assertThat(serviceList.get(0).sayHello()).isEqualTo(MODULE1_MESSAGE); + } + + @Test + public void loadServicesFromMultipleModules() { + ModuleDescriptor module1Descriptor = new ModuleDescriptor.Builder("module1", "1.0") + .fromSources(MODULE1_PATH) + .build(); + ModuleDescriptor module2Descriptor = new ModuleDescriptor.Builder("module2", "1.0") + .fromSources(MODULE2_PATH) + .build(); + moduleService.loadModule(module1Descriptor); + moduleService.loadModule(module2Descriptor); + + List<TestService> serviceList = moduleService.loadService(TestService.class); + assertThat(serviceList.size()).isEqualTo(2); + assertThat(serviceList.stream().map(TestService::sayHello)).contains(MODULE1_MESSAGE, + MODULE2_MESSAGE); + } + + @Test + public void loadServicesFromCompositeModule() { + ModuleDescriptor moduleDescriptor = new ModuleDescriptor.Builder("module1", "1.0") + .fromSources(MODULE1_PATH, MODULE2_PATH) + .build(); + moduleService.loadModule(moduleDescriptor); + + List<TestService> serviceList = moduleService.loadService(TestService.class); + assertThat(serviceList.size()).isEqualTo(2); + assertThat(serviceList.stream().map(TestService::sayHello)).contains(MODULE1_MESSAGE, + MODULE2_MESSAGE); + } + + @Test + public void loadServiceFromModulesWithDependencies() { + ModuleDescriptor module1Descriptor = new ModuleDescriptor.Builder("module1", "1.0") + .fromSources(MODULE1_PATH) + .build(); + moduleService.loadModule(module1Descriptor); + ModuleDescriptor module2Descriptor = new ModuleDescriptor.Builder("module2", "1.0") + .fromSources(MODULE2_PATH) + .dependsOnModules(module1Descriptor.getVersionedName()) + .build(); + moduleService.loadModule(module2Descriptor); + + List<TestService> serviceList = moduleService.loadService(TestService.class); + assertThat(serviceList.size()).isEqualTo(2); + assertThat(serviceList.stream().map(TestService::sayHello)).contains(MODULE1_MESSAGE, + MODULE2_MESSAGE); + } + + @Test + public void loadServiceFromModuleWithDuplicateContents() { + ModuleDescriptor module1Descriptor = new ModuleDescriptor.Builder("module1", "1.0") + .fromSources(MODULE1_PATH, MODULE1_PATH) + .build(); + moduleService.loadModule(module1Descriptor); + + List<TestService> serviceList = moduleService.loadService(TestService.class); + assertThat(serviceList.size()).isEqualTo(1); + assertThat(serviceList.get(0).sayHello()).isEqualTo(MODULE1_MESSAGE); + } } diff --git a/geode-modules/src/testModules/module1/java/org/apache/geode/Module1.java b/geode-modules/src/testModules/module1/java/org/apache/geode/Module1.java index 684a71c..0463781 100644 --- a/geode-modules/src/testModules/module1/java/org/apache/geode/Module1.java +++ b/geode-modules/src/testModules/module1/java/org/apache/geode/Module1.java @@ -15,5 +15,9 @@ package org.apache.geode; -public class Module1 { +public class Module1 implements TestService { + @Override + public String sayHello() { + return "Hello from Module1!"; + } } diff --git a/geode-modules/src/testModules/module1/resources/META-INF/services/org.apache.geode.TestService b/geode-modules/src/testModules/module1/resources/META-INF/services/org.apache.geode.TestService new file mode 100644 index 0000000..209c875 --- /dev/null +++ b/geode-modules/src/testModules/module1/resources/META-INF/services/org.apache.geode.TestService @@ -0,0 +1 @@ +org.apache.geode.Module1 \ No newline at end of file diff --git a/geode-modules/src/testModules/module2/java/org.apache.geode/Module2.java b/geode-modules/src/testModules/module2/java/org/apache/geode/Module2.java similarity index 86% rename from geode-modules/src/testModules/module2/java/org.apache.geode/Module2.java rename to geode-modules/src/testModules/module2/java/org/apache/geode/Module2.java index 9cbac0f..1008bbd 100644 --- a/geode-modules/src/testModules/module2/java/org.apache.geode/Module2.java +++ b/geode-modules/src/testModules/module2/java/org/apache/geode/Module2.java @@ -15,5 +15,9 @@ package org.apache.geode; -public class Module2 { +public class Module2 implements TestService { + @Override + public String sayHello() { + return "Hello from Module2!"; + } } diff --git a/geode-modules/src/testModules/module2/resources/META-INF/services/org.apache.geode.TestService b/geode-modules/src/testModules/module2/resources/META-INF/services/org.apache.geode.TestService new file mode 100644 index 0000000..6e0cde8 --- /dev/null +++ b/geode-modules/src/testModules/module2/resources/META-INF/services/org.apache.geode.TestService @@ -0,0 +1 @@ +org.apache.geode.Module2 \ No newline at end of file