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

sai_boorlagadda 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 ad45baa  GEODE-4856: Public API for retrieving/persisting Cluster 
Configuration (#1791)
ad45baa is described below

commit ad45baa5f4221b773797d4051ddd12f66c96aadc
Author: Sai Boorlagadda <sai.boorlaga...@gmail.com>
AuthorDate: Fri Apr 13 11:07:06 2018 -0700

    GEODE-4856: Public API for retrieving/persisting Cluster Configuration 
(#1791)
    
      fixed a bug in getCacheConfig and updateCacheConfig
      to support non-existing group
    
    Signed-off-by: Jinmei Liao <jil...@pivotal.io>
---
 .../geode/cache/configuration/CacheConfig.java     |  6 +++++
 .../InternalClusterConfigurationService.java       | 16 +++++++++--
 .../cli/commands/CreateJndiBindingCommand.java     | 14 +++++-----
 .../cli/commands/DescribeJndiBindingCommand.java   |  4 +++
 .../ExportImportClusterConfigurationCommands.java  | 17 +++++++-----
 .../InternalClusterConfigurationServiceTest.java   | 31 +++++++++++++++++++++-
 6 files changed, 72 insertions(+), 16 deletions(-)

diff --git 
a/geode-core/src/main/java/org/apache/geode/cache/configuration/CacheConfig.java
 
b/geode-core/src/main/java/org/apache/geode/cache/configuration/CacheConfig.java
index 957753d..e2b919a 100644
--- 
a/geode-core/src/main/java/org/apache/geode/cache/configuration/CacheConfig.java
+++ 
b/geode-core/src/main/java/org/apache/geode/cache/configuration/CacheConfig.java
@@ -327,6 +327,12 @@ public class CacheConfig {
   @XmlJavaTypeAdapter(CollapsedStringAdapter.class)
   protected String version;
 
+  public CacheConfig() {}
+
+  public CacheConfig(String version) {
+    this.version = version;
+  }
+
   /**
    * Gets the value of the cacheTransactionManager property.
    *
diff --git 
a/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalClusterConfigurationService.java
 
b/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalClusterConfigurationService.java
index d750fd6..459d6e3 100644
--- 
a/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalClusterConfigurationService.java
+++ 
b/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalClusterConfigurationService.java
@@ -834,10 +834,16 @@ public class InternalClusterConfigurationService 
implements ClusterConfiguration
     if (group == null) {
       group = CLUSTER_CONFIG;
     }
-    String xmlContent = getConfiguration(group).getCacheXmlContent();
+    Configuration configuration = getConfiguration(group);
+    if (configuration == null) {
+      return null;
+    }
+    String xmlContent = configuration.getCacheXmlContent();
+    // group existed, so we should create a blank one to start with
     if (xmlContent == null || xmlContent.isEmpty()) {
-      xmlContent = generateInitialXmlContent();
+      return null;
     }
+
     return jaxbService.unMarshall(xmlContent);
   }
 
@@ -849,12 +855,18 @@ public class InternalClusterConfigurationService 
implements ClusterConfiguration
     lockSharedConfiguration();
     try {
       CacheConfig cacheConfig = getCacheConfig(group);
+      if (cacheConfig == null) {
+        cacheConfig = new CacheConfig("1.0");
+      }
       cacheConfig = mutator.apply(cacheConfig);
       if (cacheConfig == null) {
         // mutator returns a null config, indicating no change needs to be 
persisted
         return;
       }
       Configuration configuration = getConfiguration(group);
+      if (configuration == null) {
+        configuration = new Configuration(group);
+      }
       configuration.setCacheXmlContent(jaxbService.marshall(cacheConfig));
       getConfigurationRegion().put(group, configuration);
     } finally {
diff --git 
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateJndiBindingCommand.java
 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateJndiBindingCommand.java
index 25bfd6e..2900bdb 100644
--- 
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateJndiBindingCommand.java
+++ 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateJndiBindingCommand.java
@@ -152,12 +152,14 @@ public class CreateJndiBindingCommand extends 
InternalGfshCommand {
 
     if (service != null) {
       CacheConfig cacheConfig = service.getCacheConfig("cluster");
-      JndiBindingsType.JndiBinding existing =
-          service.findIdentifiable(cacheConfig.getJndiBindings(), jndiName);
-      if (existing != null) {
-        throw new EntityExistsException(
-            CliStrings.format("Jndi binding with jndi-name \"{0}\" already 
exists.", jndiName),
-            ifNotExists);
+      if (cacheConfig != null) {
+        JndiBindingsType.JndiBinding existing =
+            service.findIdentifiable(cacheConfig.getJndiBindings(), jndiName);
+        if (existing != null) {
+          throw new EntityExistsException(
+              CliStrings.format("Jndi binding with jndi-name \"{0}\" already 
exists.", jndiName),
+              ifNotExists);
+        }
       }
 
       service.updateCacheConfig("cluster", config -> {
diff --git 
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeJndiBindingCommand.java
 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeJndiBindingCommand.java
index cc2f086..3e00ea2 100644
--- 
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeJndiBindingCommand.java
+++ 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeJndiBindingCommand.java
@@ -55,6 +55,10 @@ public class DescribeJndiBindingCommand extends 
InternalGfshCommand {
         (InternalClusterConfigurationService) getConfigurationService();
     if (ccService != null) {
       CacheConfig cacheConfig = ccService.getCacheConfig("cluster");
+      if (cacheConfig == null) {
+        return ResultBuilder
+            .createUserErrorResult(String.format("JNDI binding : %s not 
found", bindingName));
+      }
       List<JndiBindingsType.JndiBinding> jndiBindings = 
cacheConfig.getJndiBindings();
 
       if (jndiBindings.stream().noneMatch(b -> 
b.getJndiName().equals(bindingName)
diff --git 
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportImportClusterConfigurationCommands.java
 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportImportClusterConfigurationCommands.java
index 1deb998..f92ef8f 100644
--- 
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportImportClusterConfigurationCommands.java
+++ 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportImportClusterConfigurationCommands.java
@@ -148,13 +148,16 @@ public class ExportImportClusterConfigurationCommands 
extends InternalGfshComman
       Set<String> groupNames = sc.getConfigurationRegion().keySet();
       for (String groupName : groupNames) {
         CacheConfig cacheConfig = sc.getCacheConfig(groupName);
-        if (cacheConfig.getRegion().size() > 0 || 
cacheConfig.getAsyncEventQueue().size() > 0
-            || cacheConfig.getDiskStore().size() > 0
-            || cacheConfig.getCustomCacheElements().size() > 0
-            || cacheConfig.getJndiBindings().size() > 0 || 
cacheConfig.getGatewayReceiver() != null
-            || cacheConfig.getGatewaySender().size() > 0) {
-          return ResultBuilder.createGemFireErrorResult(
-              "Running servers have existing cluster configuration applied 
already.");
+        if (cacheConfig != null) {
+          if (cacheConfig.getRegion().size() > 0 || 
cacheConfig.getAsyncEventQueue().size() > 0
+              || cacheConfig.getDiskStore().size() > 0
+              || cacheConfig.getCustomCacheElements().size() > 0
+              || cacheConfig.getJndiBindings().size() > 0
+              || cacheConfig.getGatewayReceiver() != null
+              || cacheConfig.getGatewaySender().size() > 0) {
+            return ResultBuilder.createGemFireErrorResult(
+                "Running servers have existing cluster configuration applied 
already.");
+          }
         }
       }
 
diff --git 
a/geode-core/src/test/java/org/apache/geode/distributed/internal/InternalClusterConfigurationServiceTest.java
 
b/geode-core/src/test/java/org/apache/geode/distributed/internal/InternalClusterConfigurationServiceTest.java
index e48d1a4..ce3b87f 100644
--- 
a/geode-core/src/test/java/org/apache/geode/distributed/internal/InternalClusterConfigurationServiceTest.java
+++ 
b/geode-core/src/test/java/org/apache/geode/distributed/internal/InternalClusterConfigurationServiceTest.java
@@ -21,16 +21,20 @@ import static 
org.apache.geode.internal.config.JAXBServiceTest.setBasicValues;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doCallRealMethod;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
 
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
 import org.apache.geode.cache.Region;
+import org.apache.geode.cache.configuration.CacheConfig;
 import org.apache.geode.cache.configuration.JndiBindingsType;
 import org.apache.geode.cache.configuration.RegionConfig;
 import org.apache.geode.internal.config.JAXBServiceTest;
@@ -50,7 +54,7 @@ public class InternalClusterConfigurationServiceTest {
   public void setUp() throws Exception {
     service = spy(InternalClusterConfigurationService.class);
     configuration = new Configuration("cluster");
-    doReturn(configuration).when(service).getConfiguration(any());
+    doReturn(configuration).when(service).getConfiguration("cluster");
     doReturn(mock(Region.class)).when(service).getConfigurationRegion();
     doReturn(true).when(service).lockSharedConfiguration();
     doNothing().when(service).unlockSharedConfiguration();
@@ -166,4 +170,29 @@ public class InternalClusterConfigurationServiceTest {
     System.out.println(configuration.getCacheXmlContent());
     
assertThat(configuration.getCacheXmlContent()).doesNotContain("custom-one>");
   }
+
+  @Test
+  public void getNonExistingGroupConfigShouldReturnNull() {
+    assertThat(service.getCacheConfig("non-existing-group")).isNull();
+  }
+
+  @Test
+  public void getExistingGroupConfigShouldReturnNullIfNoXml() {
+    Configuration groupConfig = new Configuration("some-new-group");
+    doReturn(groupConfig).when(service).getConfiguration("some-new-group");
+    CacheConfig groupCacheConfig = service.getCacheConfig("some-new-group");
+    assertThat(groupCacheConfig).isNull();
+  }
+
+  @Test
+  public void updateShouldInsertIfNotExist() {
+    doCallRealMethod().when(service).updateCacheConfig(any(), any());
+    doCallRealMethod().when(service).getCacheConfig(any());
+    Region region = mock(Region.class);
+    doReturn(region).when(service).getConfigurationRegion();
+
+    service.updateCacheConfig("non-existing-group", cc -> cc);
+
+    verify(region).put(eq("non-existing-group"), any());
+  }
 }

-- 
To stop receiving notification emails like this one, please contact
sai_boorlaga...@apache.org.

Reply via email to