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


The following commit(s) were added to refs/heads/feature/GEODE-8067 by this 
push:
     new 387f21b  GEODE-8137 - Implement loadService. (#5136)
387f21b is described below

commit 387f21b49404d834dd9da5cc0a153ff1201256c4
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 139424e..4c9d066 100644
--- a/geode-assembly/src/integrationTest/resources/assembly_content.txt
+++ b/geode-assembly/src/integrationTest/resources/assembly_content.txt
@@ -969,7 +969,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

Reply via email to