You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by sa...@apache.org on 2018/04/13 18:07:11 UTC
[geode] branch develop updated: GEODE-4856: Public API for
retrieving/persisting Cluster Configuration (#1791)
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 <sa...@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 <ji...@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_boorlagadda@apache.org.