You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ji...@apache.org on 2019/02/22 21:14:06 UTC

[geode] branch develop updated: GEODE-6384: Create consistent API to retrieve instances of ClusterManagementService (#3221)

This is an automated email from the ASF dual-hosted git repository.

jinmeiliao pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/develop by this push:
     new d866bbd  GEODE-6384: Create consistent API to retrieve instances of ClusterManagementService (#3221)
d866bbd is described below

commit d866bbdb8478023507cdc2a9152d5454d6e74d60
Author: jinmeiliao <ji...@pivotal.io>
AuthorDate: Fri Feb 22 13:13:50 2019 -0800

    GEODE-6384: Create consistent API to retrieve instances of ClusterManagementService (#3221)
    
    * rename classes
    * move classes to internal packages
---
 .../integrationTest/resources/assembly_content.txt |  6 ---
 ...ClusterManagementServiceRetrievalDUnitTest.java |  6 +--
 ...a => GeodeClusterManagementServiceFactory.java} | 14 ++---
 ...ement.internal.ClusterManagementServiceFactory} |  2 +-
 .../client/ClusterManagementServiceProvider.java   | 61 ++++++++--------------
 ...nt.java => ClientClusterManagementService.java} | 18 +++----
 .../ClusterManagementServiceFactory.java}          |  8 ++-
 ...JavaClientClusterManagementServiceFactory.java} | 13 +++--
 .../RestTemplateResponseErrorHandler.java          |  2 +-
 ...ement.internal.ClusterManagementServiceFactory} |  2 +-
 ...> ClientClusterManagementServiceDUnitTest.java} |  3 +-
 11 files changed, 53 insertions(+), 82 deletions(-)

diff --git a/geode-assembly/src/integrationTest/resources/assembly_content.txt b/geode-assembly/src/integrationTest/resources/assembly_content.txt
index c2569de..a6f3323 100644
--- a/geode-assembly/src/integrationTest/resources/assembly_content.txt
+++ b/geode-assembly/src/integrationTest/resources/assembly_content.txt
@@ -701,7 +701,6 @@ javadoc/org/apache/geode/management/cli/package-frame.html
 javadoc/org/apache/geode/management/cli/package-summary.html
 javadoc/org/apache/geode/management/cli/package-tree.html
 javadoc/org/apache/geode/management/client/ClusterManagementServiceProvider.html
-javadoc/org/apache/geode/management/client/RestTemplateResponseErrorHandler.html
 javadoc/org/apache/geode/management/client/package-frame.html
 javadoc/org/apache/geode/management/client/package-summary.html
 javadoc/org/apache/geode/management/client/package-tree.html
@@ -719,11 +718,6 @@ javadoc/org/apache/geode/management/membership/package-tree.html
 javadoc/org/apache/geode/management/package-frame.html
 javadoc/org/apache/geode/management/package-summary.html
 javadoc/org/apache/geode/management/package-tree.html
-javadoc/org/apache/geode/management/spi/ClusterManagementServiceProviderFactory.html
-javadoc/org/apache/geode/management/spi/JavaClientClusterManagementProviderFactory.html
-javadoc/org/apache/geode/management/spi/package-frame.html
-javadoc/org/apache/geode/management/spi/package-summary.html
-javadoc/org/apache/geode/management/spi/package-tree.html
 javadoc/org/apache/geode/memcached/GemFireMemcachedServer.Protocol.html
 javadoc/org/apache/geode/memcached/GemFireMemcachedServer.html
 javadoc/org/apache/geode/memcached/package-frame.html
diff --git a/geode-core/src/distributedTest/java/org/apache/geode/management/api/ClusterManagementServiceRetrievalDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/management/api/ClusterManagementServiceRetrievalDUnitTest.java
index 708da0c..c83d9c6 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/management/api/ClusterManagementServiceRetrievalDUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/management/api/ClusterManagementServiceRetrievalDUnitTest.java
@@ -22,7 +22,7 @@ import org.junit.Test;
 import org.springframework.web.util.AbstractUriTemplateHandler;
 
 import org.apache.geode.management.client.ClusterManagementServiceProvider;
-import org.apache.geode.management.internal.ClusterManagementClient;
+import org.apache.geode.management.internal.ClientClusterManagementService;
 import org.apache.geode.test.dunit.rules.ClusterStartupRule;
 import org.apache.geode.test.dunit.rules.MemberVM;
 
@@ -42,8 +42,8 @@ public class ClusterManagementServiceRetrievalDUnitTest {
 
     final String url = String.format("http://localhost:%d", locator.getHttpPort());
     server1.invoke(() -> {
-      ClusterManagementClient cms =
-          (ClusterManagementClient) ClusterManagementServiceProvider.getService();
+      ClientClusterManagementService cms =
+          (ClientClusterManagementService) ClusterManagementServiceProvider.getService();
       assertThat(
           ((AbstractUriTemplateHandler) cms.getRestTemplate().getUriTemplateHandler()).getBaseUrl())
               .isEqualTo(url);
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/api/GeodeClusterManagementServiceProviderFactory.java b/geode-core/src/main/java/org/apache/geode/management/internal/api/GeodeClusterManagementServiceFactory.java
similarity index 90%
rename from geode-core/src/main/java/org/apache/geode/management/internal/api/GeodeClusterManagementServiceProviderFactory.java
rename to geode-core/src/main/java/org/apache/geode/management/internal/api/GeodeClusterManagementServiceFactory.java
index ce8d9c7..d013e43 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/api/GeodeClusterManagementServiceProviderFactory.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/api/GeodeClusterManagementServiceFactory.java
@@ -35,20 +35,20 @@ import org.apache.geode.internal.cache.GemFireCacheImpl;
 import org.apache.geode.internal.logging.LogService;
 import org.apache.geode.management.api.ClusterManagementService;
 import org.apache.geode.management.client.ClusterManagementServiceProvider;
-import org.apache.geode.management.internal.ClusterManagementClient;
+import org.apache.geode.management.internal.ClientClusterManagementService;
+import org.apache.geode.management.internal.ClusterManagementServiceFactory;
+import org.apache.geode.management.internal.JavaClientClusterManagementServiceFactory;
 import org.apache.geode.management.internal.cli.domain.MemberInformation;
 import org.apache.geode.management.internal.cli.functions.GetMemberInformationFunction;
-import org.apache.geode.management.spi.ClusterManagementServiceProviderFactory;
-import org.apache.geode.management.spi.JavaClientClusterManagementProviderFactory;
 
 /**
- * An implementation of {@link ClusterManagementServiceProviderFactory} which can be used in any
+ * An implementation of {@link ClusterManagementServiceFactory} which can be used in any
  * context where Geode is running (client, server or locator). It will attempt to determine the
  * address of the {@code ClusterManagementService} when using the {@code create()} call. Otherwise
  * an explicit can also be used.
  */
-public class GeodeClusterManagementServiceProviderFactory extends
-    JavaClientClusterManagementProviderFactory {
+public class GeodeClusterManagementServiceFactory extends
+    JavaClientClusterManagementServiceFactory {
 
   @Immutable
   private static final GetMemberInformationFunction MEMBER_INFORMATION_FUNCTION =
@@ -76,7 +76,7 @@ public class GeodeClusterManagementServiceProviderFactory extends
               .keySet();
       String serviceAddress = getHttpServiceAddress(locatorsWithClusterConfig);
 
-      return new ClusterManagementClient(serviceAddress);
+      return new ClientClusterManagementService(serviceAddress);
     }
 
     ClientCache clientCache = ClientCacheFactory.getAnyInstance();
diff --git a/geode-core/src/main/resources/META-INF/services/org.apache.geode.management.spi.ClusterManagementServiceProviderFactory b/geode-core/src/main/resources/META-INF/services/org.apache.geode.management.internal.ClusterManagementServiceFactory
similarity index 97%
rename from geode-core/src/main/resources/META-INF/services/org.apache.geode.management.spi.ClusterManagementServiceProviderFactory
rename to geode-core/src/main/resources/META-INF/services/org.apache.geode.management.internal.ClusterManagementServiceFactory
index ec5fe49..047244a 100644
--- a/geode-core/src/main/resources/META-INF/services/org.apache.geode.management.spi.ClusterManagementServiceProviderFactory
+++ b/geode-core/src/main/resources/META-INF/services/org.apache.geode.management.internal.ClusterManagementServiceFactory
@@ -13,4 +13,4 @@
 # the License.
 #
 
-org.apache.geode.management.internal.api.GeodeClusterManagementServiceProviderFactory
+org.apache.geode.management.internal.api.GeodeClusterManagementServiceFactory
diff --git a/geode-management/src/main/java/org/apache/geode/management/client/ClusterManagementServiceProvider.java b/geode-management/src/main/java/org/apache/geode/management/client/ClusterManagementServiceProvider.java
index c378d46..9c006c8 100644
--- a/geode-management/src/main/java/org/apache/geode/management/client/ClusterManagementServiceProvider.java
+++ b/geode-management/src/main/java/org/apache/geode/management/client/ClusterManagementServiceProvider.java
@@ -15,23 +15,23 @@
 
 package org.apache.geode.management.client;
 
-import java.util.ArrayList;
-import java.util.List;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.ServiceLoader;
 
 import org.springframework.http.client.ClientHttpRequestFactory;
 
 import org.apache.geode.annotations.Experimental;
 import org.apache.geode.management.api.ClusterManagementService;
-import org.apache.geode.management.spi.ClusterManagementServiceProviderFactory;
+import org.apache.geode.management.internal.ClusterManagementServiceFactory;
 
 /**
  * Top-level entry point for client interaction with the {@link ClusterManagementService}. A user
  * can create an instance of the {@code ClusterManagementService} (CMS for short) in several ways,
  * each providing various level of control or expediency.
  * <p/>
- * Calling {@code getFactory(context)} will return an explicit instance of {@link
- * ClusterManagementServiceProviderFactory}. Methods on this factory can then be called to create or
+ * Calling {@code getServiceFactory(context)} will return an explicit instance of {@link
+ * ClusterManagementServiceFactory}. Methods on this factory can then be called to create or
  * retrieve instances of a CMS. New {@link ClusterManagementServiceProvider}s can be written if
  * specific customization or parameterization is required.
  * <p/>
@@ -64,56 +64,37 @@ public class ClusterManagementServiceProvider {
   public static final String JAVA_CLIENT_CONTEXT = "java-client";
   public static final String GEODE_CONTEXT = "geode";
 
-  private static List<ClusterManagementServiceProviderFactory> providerFactories = null;
+  private static Map<String, ClusterManagementServiceFactory> serviceFactories = null;
 
   public static ClusterManagementService getService() {
-    ClusterManagementServiceProviderFactory factory;
-    try {
-      factory = getFactory(GEODE_CONTEXT);
-      try {
-        ClusterManagementService cms = factory.create();
-        return cms;
-      } catch (IllegalStateException ex) {
-        // Ignored
-      }
-    } catch (IllegalArgumentException iex) {
-      // Ig
-    } catch (Exception iex) {
-      iex.printStackTrace();
-    }
-
-    throw new IllegalStateException(
-        "Unable to get ClusterManagementService using any of the default contexts");
+    return getServiceFactory(GEODE_CONTEXT).create();
   }
 
   public static ClusterManagementService getService(String clusterUrl) {
-    return getFactory(JAVA_CLIENT_CONTEXT).create(clusterUrl);
+    return getServiceFactory(JAVA_CLIENT_CONTEXT).create(clusterUrl);
   }
 
   public static ClusterManagementService getService(ClientHttpRequestFactory requestFactory) {
-    return getFactory(JAVA_CLIENT_CONTEXT).create(requestFactory);
+    return getServiceFactory(JAVA_CLIENT_CONTEXT).create(requestFactory);
   }
 
-  public static synchronized ClusterManagementServiceProviderFactory getFactory(String context) {
-    if (providerFactories == null) {
-      loadClusterManagementServiceProviderFactories();
+  private static synchronized ClusterManagementServiceFactory getServiceFactory(String context) {
+    if (serviceFactories == null) {
+      loadClusterManagementServiceFactories();
+    }
+    ClusterManagementServiceFactory factory = serviceFactories.get(context);
+    if (factory == null) {
+      throw new IllegalArgumentException("Did not find provider for context: " + context);
     }
-
-    ClusterManagementServiceProviderFactory factory = providerFactories.stream()
-        .filter(x -> x.getContext().equalsIgnoreCase(context))
-        .findFirst()
-        .orElseThrow(
-            () -> new IllegalArgumentException("Did not find provider for context: " + context));
-
     return factory;
   }
 
-  private static void loadClusterManagementServiceProviderFactories() {
-    providerFactories = new ArrayList<>();
+  private static void loadClusterManagementServiceFactories() {
+    serviceFactories = new HashMap<>();
 
-    for (ClusterManagementServiceProviderFactory factory : ServiceLoader
-        .load(ClusterManagementServiceProviderFactory.class)) {
-      providerFactories.add(factory);
+    for (ClusterManagementServiceFactory factory : ServiceLoader
+        .load(ClusterManagementServiceFactory.class)) {
+      serviceFactories.put(factory.getContext(), factory);
     }
   }
 }
diff --git a/geode-management/src/main/java/org/apache/geode/management/internal/ClusterManagementClient.java b/geode-management/src/main/java/org/apache/geode/management/internal/ClientClusterManagementService.java
similarity index 84%
rename from geode-management/src/main/java/org/apache/geode/management/internal/ClusterManagementClient.java
rename to geode-management/src/main/java/org/apache/geode/management/internal/ClientClusterManagementService.java
index 5eeb647..e6980b6 100644
--- a/geode-management/src/main/java/org/apache/geode/management/internal/ClusterManagementClient.java
+++ b/geode-management/src/main/java/org/apache/geode/management/internal/ClientClusterManagementService.java
@@ -22,21 +22,20 @@ import org.springframework.web.client.RestTemplate;
 import org.springframework.web.util.DefaultUriTemplateHandler;
 import sun.reflect.generics.reflectiveObjects.NotImplementedException;
 
-import org.apache.geode.annotations.Experimental;
 import org.apache.geode.cache.configuration.CacheElement;
 import org.apache.geode.management.api.ClusterManagementResult;
 import org.apache.geode.management.api.ClusterManagementService;
 import org.apache.geode.management.api.RestfulEndpoint;
-import org.apache.geode.management.client.RestTemplateResponseErrorHandler;
 
 /**
  * Implementation of {@link ClusterManagementService} interface which represents the cluster
  * management service as used by a Java client.
  * <p/>
  * In order to manipulate Geode components (Regions, etc.) clients can construct instances of {@link
- * CacheElement}s and call the corresponding {@link ClusterManagementClient#create(CacheElement,
- * String)}, {@link ClusterManagementClient#delete(CacheElement, String)} or {@link
- * ClusterManagementClient#update(CacheElement, String)} method. The returned {@link
+ * CacheElement}s and call the corresponding
+ * {@link ClientClusterManagementService#create(CacheElement,
+ * String)}, {@link ClientClusterManagementService#delete(CacheElement, String)} or {@link
+ * ClientClusterManagementService#update(CacheElement, String)} method. The returned {@link
  * ClusterManagementResult} will contain all necessary information about the outcome of the call.
  * This will include the result of persisting the config as part of the cluster configuration as
  * well as creating the actual component in the cluster.
@@ -44,25 +43,24 @@ import org.apache.geode.management.client.RestTemplateResponseErrorHandler;
  * All create calls are idempotent and will not return an error if the desired component already
  * exists.
  */
-@Experimental
-public class ClusterManagementClient implements ClusterManagementService {
+public class ClientClusterManagementService implements ClusterManagementService {
 
   private static final ResponseErrorHandler DEFAULT_ERROR_HANDLER =
       new RestTemplateResponseErrorHandler();
 
   private final RestTemplate restTemplate;
 
-  private ClusterManagementClient() {
+  private ClientClusterManagementService() {
     restTemplate = new RestTemplate();
     restTemplate.setErrorHandler(DEFAULT_ERROR_HANDLER);
   }
 
-  public ClusterManagementClient(ClientHttpRequestFactory requestFactory) {
+  public ClientClusterManagementService(ClientHttpRequestFactory requestFactory) {
     this();
     this.restTemplate.setRequestFactory(requestFactory);
   }
 
-  public ClusterManagementClient(String clusterUrl) {
+  public ClientClusterManagementService(String clusterUrl) {
     this();
     DefaultUriTemplateHandler templateHandler = new DefaultUriTemplateHandler();
     templateHandler.setBaseUrl(clusterUrl);
diff --git a/geode-management/src/main/java/org/apache/geode/management/spi/ClusterManagementServiceProviderFactory.java b/geode-management/src/main/java/org/apache/geode/management/internal/ClusterManagementServiceFactory.java
similarity index 90%
rename from geode-management/src/main/java/org/apache/geode/management/spi/ClusterManagementServiceProviderFactory.java
rename to geode-management/src/main/java/org/apache/geode/management/internal/ClusterManagementServiceFactory.java
index 44136f2..4489905 100644
--- a/geode-management/src/main/java/org/apache/geode/management/spi/ClusterManagementServiceProviderFactory.java
+++ b/geode-management/src/main/java/org/apache/geode/management/internal/ClusterManagementServiceFactory.java
@@ -13,7 +13,7 @@
  * the License.
  */
 
-package org.apache.geode.management.spi;
+package org.apache.geode.management.internal;
 
 import org.springframework.http.client.ClientHttpRequestFactory;
 
@@ -23,12 +23,10 @@ import org.apache.geode.management.api.ClusterManagementService;
  * A Server Provider interface which should be implemented to create instances of {@link
  * ClusterManagementService} for a given context.
  */
-public interface ClusterManagementServiceProviderFactory {
+public interface ClusterManagementServiceFactory {
 
   /**
-   * Returns a list of the contexts which this provider supports.
-   *
-   * @return a list of names of supported contexts of this factory
+   * Returns the context of the factory
    */
   String getContext();
 
diff --git a/geode-management/src/main/java/org/apache/geode/management/spi/JavaClientClusterManagementProviderFactory.java b/geode-management/src/main/java/org/apache/geode/management/internal/JavaClientClusterManagementServiceFactory.java
similarity index 81%
rename from geode-management/src/main/java/org/apache/geode/management/spi/JavaClientClusterManagementProviderFactory.java
rename to geode-management/src/main/java/org/apache/geode/management/internal/JavaClientClusterManagementServiceFactory.java
index 666ce51..9acbd7c 100644
--- a/geode-management/src/main/java/org/apache/geode/management/spi/JavaClientClusterManagementProviderFactory.java
+++ b/geode-management/src/main/java/org/apache/geode/management/internal/JavaClientClusterManagementServiceFactory.java
@@ -13,22 +13,21 @@
  * the License.
  */
 
-package org.apache.geode.management.spi;
+package org.apache.geode.management.internal;
 
 import org.springframework.http.client.ClientHttpRequestFactory;
 
 import org.apache.geode.management.api.ClusterManagementService;
 import org.apache.geode.management.client.ClusterManagementServiceProvider;
-import org.apache.geode.management.internal.ClusterManagementClient;
 
 /**
- * A Service Provider factory which will be used to create {@link ClusterManagementService}
+ * A Service factory which will be used to create {@link ClusterManagementService}
  * instances from pure Java code - i.e. the Cluster Management endpoint URL <b>cannot</b> be
  * inferred from the implied runtime context but needs to be specifically configured using a given
  * URL or {@code ClientHttpRequestFactory}.
  */
-public class JavaClientClusterManagementProviderFactory
-    implements ClusterManagementServiceProviderFactory {
+public class JavaClientClusterManagementServiceFactory
+    implements ClusterManagementServiceFactory {
 
   @Override
   public String getContext() {
@@ -44,11 +43,11 @@ public class JavaClientClusterManagementProviderFactory
 
   @Override
   public ClusterManagementService create(String clusterUrl) {
-    return new ClusterManagementClient(clusterUrl);
+    return new ClientClusterManagementService(clusterUrl);
   }
 
   @Override
   public ClusterManagementService create(ClientHttpRequestFactory requestFactory) {
-    return new ClusterManagementClient(requestFactory);
+    return new ClientClusterManagementService(requestFactory);
   }
 }
diff --git a/geode-management/src/main/java/org/apache/geode/management/client/RestTemplateResponseErrorHandler.java b/geode-management/src/main/java/org/apache/geode/management/internal/RestTemplateResponseErrorHandler.java
similarity index 96%
rename from geode-management/src/main/java/org/apache/geode/management/client/RestTemplateResponseErrorHandler.java
rename to geode-management/src/main/java/org/apache/geode/management/internal/RestTemplateResponseErrorHandler.java
index 6ae39c0..ef3272a 100644
--- a/geode-management/src/main/java/org/apache/geode/management/client/RestTemplateResponseErrorHandler.java
+++ b/geode-management/src/main/java/org/apache/geode/management/internal/RestTemplateResponseErrorHandler.java
@@ -13,7 +13,7 @@
  * the License.
  */
 
-package org.apache.geode.management.client;
+package org.apache.geode.management.internal;
 
 import java.io.IOException;
 
diff --git a/geode-management/src/main/resources/META-INF/services/org.apache.geode.management.spi.ClusterManagementServiceProviderFactory b/geode-management/src/main/resources/META-INF/services/org.apache.geode.management.internal.ClusterManagementServiceFactory
similarity index 90%
rename from geode-management/src/main/resources/META-INF/services/org.apache.geode.management.spi.ClusterManagementServiceProviderFactory
rename to geode-management/src/main/resources/META-INF/services/org.apache.geode.management.internal.ClusterManagementServiceFactory
index 5863340..6fa7b09 100644
--- a/geode-management/src/main/resources/META-INF/services/org.apache.geode.management.spi.ClusterManagementServiceProviderFactory
+++ b/geode-management/src/main/resources/META-INF/services/org.apache.geode.management.internal.ClusterManagementServiceFactory
@@ -14,4 +14,4 @@
 #
 
 
-org.apache.geode.management.spi.JavaClientClusterManagementProviderFactory
+org.apache.geode.management.internal.JavaClientClusterManagementServiceFactory
diff --git a/geode-web-management/src/distributedTest/java/org/apache/geode/management/client/ClusterManagementClientDUnitTest.java b/geode-web-management/src/distributedTest/java/org/apache/geode/management/client/ClientClusterManagementServiceDUnitTest.java
similarity index 96%
rename from geode-web-management/src/distributedTest/java/org/apache/geode/management/client/ClusterManagementClientDUnitTest.java
rename to geode-web-management/src/distributedTest/java/org/apache/geode/management/client/ClientClusterManagementServiceDUnitTest.java
index 06a6f4f..eeba0cc 100644
--- a/geode-web-management/src/distributedTest/java/org/apache/geode/management/client/ClusterManagementClientDUnitTest.java
+++ b/geode-web-management/src/distributedTest/java/org/apache/geode/management/client/ClientClusterManagementServiceDUnitTest.java
@@ -34,6 +34,7 @@ import org.springframework.web.context.WebApplicationContext;
 import org.apache.geode.cache.configuration.RegionConfig;
 import org.apache.geode.management.api.ClusterManagementResult;
 import org.apache.geode.management.api.ClusterManagementService;
+import org.apache.geode.management.internal.RestTemplateResponseErrorHandler;
 import org.apache.geode.management.internal.rest.BaseLocatorContextLoader;
 import org.apache.geode.management.internal.rest.PlainLocatorContextLoader;
 import org.apache.geode.test.dunit.rules.ClusterStartupRule;
@@ -43,7 +44,7 @@ import org.apache.geode.test.dunit.rules.MemberVM;
 @ContextConfiguration(locations = {"classpath*:WEB-INF/geode-management-servlet.xml"},
     loader = PlainLocatorContextLoader.class)
 @WebAppConfiguration
-public class ClusterManagementClientDUnitTest {
+public class ClientClusterManagementServiceDUnitTest {
   private static final ResponseErrorHandler ERROR_HANDLER = new RestTemplateResponseErrorHandler();
 
   @Autowired