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/01/29 18:33:12 UTC
[geode] branch develop updated: GEODE-6174: change refid to type
and make the default type to be PARTITION (#3130)
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 c97edf9 GEODE-6174: change refid to type and make the default type to be PARTITION (#3130)
c97edf9 is described below
commit c97edf9ae9ccf8964e10bf4f9b22cd853fae6d1f
Author: jinmeiliao <ji...@pivotal.io>
AuthorDate: Tue Jan 29 10:33:01 2019 -0800
GEODE-6174: change refid to type and make the default type to be PARTITION (#3130)
Co-authored-by: Peter Tran <pt...@pivotal.io>
* change refid to type for better UX
* add a common config validator interface and use that to validate RegionConfig
---
...ClusterManagementLocatorReconnectDunitTest.java | 2 +-
.../internal/rest/RegionManagementDunitTest.java | 84 ++++++++++++-----
.../internal/RegionManagementIntegrationTest.java | 2 +-
.../RegionManagementSecurityIntegrationTest.java | 2 +-
.../internal/CacheConfigDAODUnitTest.java | 2 +-
.../internal/api/RegionAPIDUnitTest.java | 103 ++++++++++-----------
.../RegionConfigMutatorIntegrationTest.java | 2 +-
.../RegionConfigRealizerIntegrationTest.java | 2 +-
.../api/LocatorClusterManagementService.java | 2 +
.../validators/ConfigurationValidator.java | 13 ++-
...onValidator.java => RegionConfigValidator.java} | 17 +++-
.../geode/cache/configuration/CacheConfigTest.java | 4 +-
.../cache/configuration/RegionConfigTest.java | 31 ++++++-
...nternalConfigurationPersistenceServiceTest.java | 2 +-
.../geode/internal/config/JAXBServiceTest.java | 2 +-
.../cli/commands/AlterRegionCommandTest.java | 2 +-
.../realizers/RegionConfigRealizerTest.java | 4 +-
.../validators/RegionConfigValidatorTest.java | 60 ++++++++++++
.../geode/cache/configuration/RegionConfig.java | 20 ++--
.../internal/api/ClusterManagementService.java | 1 +
20 files changed, 250 insertions(+), 107 deletions(-)
diff --git a/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/rest/ClusterManagementLocatorReconnectDunitTest.java b/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/rest/ClusterManagementLocatorReconnectDunitTest.java
index 2414c46..846c162 100644
--- a/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/rest/ClusterManagementLocatorReconnectDunitTest.java
+++ b/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/rest/ClusterManagementLocatorReconnectDunitTest.java
@@ -68,7 +68,7 @@ public class ClusterManagementLocatorReconnectDunitTest {
private void makeRestCallAndVerifyResult(String regionName) throws Exception {
RegionConfig regionConfig = new RegionConfig();
regionConfig.setName(regionName);
- regionConfig.setRefid("REPLICATE");
+ regionConfig.setType("REPLICATE");
ObjectMapper mapper = new ObjectMapper();
String json = mapper.writeValueAsString(regionConfig);
diff --git a/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/rest/RegionManagementDunitTest.java b/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/rest/RegionManagementDunitTest.java
index 080425a..43f62dc 100644
--- a/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/rest/RegionManagementDunitTest.java
+++ b/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/rest/RegionManagementDunitTest.java
@@ -18,29 +18,32 @@ package org.apache.geode.management.internal.rest;
import static org.assertj.core.api.Assertions.assertThat;
import com.fasterxml.jackson.databind.ObjectMapper;
-import org.junit.Before;
-import org.junit.Rule;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
import org.junit.Test;
+import org.apache.geode.cache.Cache;
import org.apache.geode.cache.Region;
import org.apache.geode.cache.configuration.CacheConfig;
+import org.apache.geode.cache.configuration.CacheElement;
import org.apache.geode.cache.configuration.RegionConfig;
import org.apache.geode.management.internal.api.ClusterManagementResult;
+import org.apache.geode.test.dunit.IgnoredException;
import org.apache.geode.test.dunit.rules.ClusterStartupRule;
import org.apache.geode.test.dunit.rules.MemberVM;
import org.apache.geode.test.junit.rules.GeodeDevRestClient;
public class RegionManagementDunitTest {
- @Rule
- public ClusterStartupRule cluster = new ClusterStartupRule();
+ @ClassRule
+ public static ClusterStartupRule cluster = new ClusterStartupRule();
- private MemberVM locator, server;
+ private static MemberVM locator, server;
- private GeodeDevRestClient restClient;
+ private static GeodeDevRestClient restClient;
- @Before
- public void before() throws Exception {
+ @BeforeClass
+ public static void beforeClass() throws Exception {
locator = cluster.startLocatorVM(0, l -> l.withHttpService());
server = cluster.startServerVM(1, locator.getPort());
restClient =
@@ -48,15 +51,15 @@ public class RegionManagementDunitTest {
}
@Test
- public void createRegion() throws Exception {
+ public void createsRegion() throws Exception {
RegionConfig regionConfig = new RegionConfig();
regionConfig.setName("customers");
- regionConfig.setRefid("REPLICATE");
+ regionConfig.setType("REPLICATE");
ObjectMapper mapper = new ObjectMapper();
String json = mapper.writeValueAsString(regionConfig);
ClusterManagementResult result =
- restClient.doPostAndAssert("/regions", json, "test", "test")
+ restClient.doPostAndAssert("/regions", json)
.hasStatusCode(201)
.getClusterManagementResult();
@@ -65,20 +68,59 @@ public class RegionManagementDunitTest {
assertThat(result.getMemberStatuses()).containsKeys("server-1").hasSize(1);
// make sure region is created
- server.invoke(() -> {
- Region region = ClusterStartupRule.getCache().getRegion("customers");
- assertThat(region).isNotNull();
- });
+ server.invoke(() -> verifyRegionCreated("customers", "REPLICATE"));
// make sure region is persisted
- locator.invoke(() -> {
- CacheConfig cacheConfig =
- ClusterStartupRule.getLocator().getConfigurationPersistenceService()
- .getCacheConfig("cluster");
- assertThat(cacheConfig.getRegions().get(0).getName()).isEqualTo("customers");
- });
+ locator.invoke(() -> verifyRegionPersisted("customers", "REPLICATE"));
// verify that additional server can be started with the cluster configuration
cluster.startServerVM(2, locator.getPort());
}
+
+ @Test
+ public void createsAPartitionedRegionByDefault() throws Exception {
+ String json = "{\"name\": \"orders\"}";
+
+ ClusterManagementResult result = restClient.doPostAndAssert("/regions", json)
+ .hasStatusCode(201)
+ .getClusterManagementResult();
+
+ assertThat(result.isSuccessfullyAppliedOnMembers()).isTrue();
+ assertThat(result.isSuccessfullyPersisted()).isTrue();
+ assertThat(result.getMemberStatuses()).containsKeys("server-1").hasSize(1);
+
+ // make sure region is created
+ server.invoke(() -> verifyRegionCreated("orders", "PARTITION"));
+
+ // make sure region is persisted
+ locator.invoke(() -> verifyRegionPersisted("orders", "PARTITION"));
+ }
+
+ @Test
+ public void noNameInConfig() throws Exception {
+ IgnoredException.addIgnoredException("Name of the region has to be specified");
+ String json = "{\"type\": \"REPLICATE\"}";
+
+ ClusterManagementResult result = restClient.doPostAndAssert("/regions", json)
+ .hasStatusCode(500)
+ .getClusterManagementResult();
+
+ assertThat(result.isSuccessfullyAppliedOnMembers()).isFalse();
+ assertThat(result.isSuccessfullyPersisted()).isFalse();
+ }
+
+ private static void verifyRegionPersisted(String regionName, String type) {
+ CacheConfig cacheConfig =
+ ClusterStartupRule.getLocator().getConfigurationPersistenceService()
+ .getCacheConfig("cluster");
+ RegionConfig regionConfig = CacheElement.findElement(cacheConfig.getRegions(), regionName);
+ assertThat(regionConfig.getType()).isEqualTo(type);
+ }
+
+ private static void verifyRegionCreated(String regionName, String type) {
+ Cache cache = ClusterStartupRule.getCache();
+ Region region = cache.getRegion(regionName);
+ assertThat(region).isNotNull();
+ assertThat(region.getAttributes().getDataPolicy().toString()).isEqualTo(type);
+ }
}
diff --git a/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/RegionManagementIntegrationTest.java b/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/RegionManagementIntegrationTest.java
index 8adf993..8d0b7f0 100644
--- a/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/RegionManagementIntegrationTest.java
+++ b/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/RegionManagementIntegrationTest.java
@@ -49,7 +49,7 @@ public class RegionManagementIntegrationTest {
public void sanityCheck() throws Exception {
RegionConfig regionConfig = new RegionConfig();
regionConfig.setName("customers");
- regionConfig.setRefid("REPLICATE");
+ regionConfig.setType("REPLICATE");
ObjectMapper mapper = new ObjectMapper();
String json = mapper.writeValueAsString(regionConfig);
diff --git a/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/RegionManagementSecurityIntegrationTest.java b/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/RegionManagementSecurityIntegrationTest.java
index 831c2e1..61fc91f 100644
--- a/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/RegionManagementSecurityIntegrationTest.java
+++ b/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/RegionManagementSecurityIntegrationTest.java
@@ -55,7 +55,7 @@ public class RegionManagementSecurityIntegrationTest {
public void before() throws JsonProcessingException {
regionConfig = new RegionConfig();
regionConfig.setName("customers");
- regionConfig.setRefid("REPLICATE");
+ regionConfig.setType("REPLICATE");
ObjectMapper mapper = new ObjectMapper();
json = mapper.writeValueAsString(regionConfig);
}
diff --git a/geode-core/src/distributedTest/java/org/apache/geode/distributed/internal/CacheConfigDAODUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/distributed/internal/CacheConfigDAODUnitTest.java
index 1f02f32..29a8ef1 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/distributed/internal/CacheConfigDAODUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/distributed/internal/CacheConfigDAODUnitTest.java
@@ -45,7 +45,7 @@ public class CacheConfigDAODUnitTest {
ccService.updateCacheConfig("cluster", cc -> {
RegionConfig regionConfig = new RegionConfig();
regionConfig.setName("regionB");
- regionConfig.setRefid("REPLICATE");
+ regionConfig.setType("REPLICATE");
cc.getRegions().add(regionConfig);
return cc;
});
diff --git a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/api/RegionAPIDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/api/RegionAPIDUnitTest.java
index b45e43a..70c5800 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/api/RegionAPIDUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/api/RegionAPIDUnitTest.java
@@ -15,22 +15,21 @@
package org.apache.geode.management.internal.api;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
-import java.util.List;
-
-import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.junit.rules.TestName;
import org.apache.geode.cache.Cache;
-import org.apache.geode.cache.DataPolicy;
import org.apache.geode.cache.Region;
import org.apache.geode.cache.RegionShortcut;
import org.apache.geode.cache.configuration.CacheConfig;
+import org.apache.geode.cache.configuration.CacheElement;
import org.apache.geode.cache.configuration.RegionConfig;
-import org.apache.geode.distributed.internal.InternalConfigurationPersistenceService;
import org.apache.geode.test.dunit.rules.ClusterStartupRule;
import org.apache.geode.test.dunit.rules.MemberVM;
import org.apache.geode.test.junit.categories.RegionsTest;
@@ -39,19 +38,19 @@ import org.apache.geode.test.junit.rules.serializable.SerializableTestName;
@Category({RegionsTest.class})
public class RegionAPIDUnitTest {
- private MemberVM locator, server;
+ private static MemberVM locator, server;
- @Rule
- public ClusterStartupRule clusterRule = new ClusterStartupRule();
+ @ClassRule
+ public static ClusterStartupRule clusterRule = new ClusterStartupRule();
@Rule
public TestName testName = new SerializableTestName();
- @Rule
- public GfshCommandRule gfsh = new GfshCommandRule();
+ @ClassRule
+ public static GfshCommandRule gfsh = new GfshCommandRule();
- @Before
- public void before() throws Exception {
+ @BeforeClass
+ public static void before() throws Exception {
locator = clusterRule.startLocatorVM(0);
server = clusterRule.startServerVM(1, locator.getPort());
@@ -64,23 +63,13 @@ public class RegionAPIDUnitTest {
locator.invoke(() -> {
RegionConfig config = new RegionConfig();
config.setName(regionName);
- config.setRefid(RegionShortcut.PARTITION.toString());
+ config.setType(RegionShortcut.PARTITION.toString());
ClusterManagementResult result = ClusterStartupRule.getLocator().getClusterManagementService()
.create(config, "cluster");
assertThat(result.isSuccessful()).isTrue();
});
-
- gfsh.executeAndAssertThat("list regions")
- .statusIsSuccess()
- .containsOutput(regionName);
-
- server.invoke(() -> {
- Cache cache = ClusterStartupRule.getCache();
- Region region = cache.getRegion(regionName);
- assertThat(region).isNotNull();
- assertThat(region.getAttributes().getDataPolicy()).isEqualTo(DataPolicy.PARTITION);
- });
+ server.invoke(() -> verifyRegionCreated(regionName, "PARTITION"));
locator.waitUntilRegionIsReadyOnExactlyThisManyServers("/" + regionName, 1);
@@ -89,6 +78,8 @@ public class RegionAPIDUnitTest {
gfsh.executeAndAssertThat("get --key='foo' --region=" + regionName)
.statusIsSuccess()
.containsKeyValuePair("Value", "\"125\"");
+
+ locator.invoke(() -> verifyRegionPersisted(regionName, "PARTITION"));
}
@Test
@@ -97,56 +88,56 @@ public class RegionAPIDUnitTest {
locator.invoke(() -> {
RegionConfig config = new RegionConfig();
config.setName(regionName);
- config.setRefid(RegionShortcut.REPLICATE.toString());
+ config.setType(RegionShortcut.REPLICATE.toString());
ClusterManagementResult result = ClusterStartupRule.getLocator().getClusterManagementService()
.create(config, "cluster");
assertThat(result.isSuccessful()).isTrue();
});
- server.invoke(() -> {
- Cache cache = ClusterStartupRule.getCache();
- Region region = cache.getRegion(regionName);
- assertThat(region).isNotNull();
- assertThat(region.getAttributes().getDataPolicy()).isEqualTo(DataPolicy.REPLICATE);
- });
+ server.invoke(() -> verifyRegionCreated(regionName, "REPLICATE"));
- gfsh.executeAndAssertThat("list regions").statusIsSuccess()
- .containsOutput(regionName);
+ locator.invoke(() -> verifyRegionPersisted(regionName, "REPLICATE"));
}
@Test
- public void createRegionPersists() {
- String regionName = testName.getMethodName();
- gfsh.executeAndAssertThat("create region --name=Dummy --type=PARTITION").statusIsSuccess();
+ public void noName() {
+ locator.invoke(() -> {
+ RegionConfig config = new RegionConfig();
+ ClusterManagementService clusterManagementService =
+ ClusterStartupRule.getLocator().getClusterManagementService();
+ assertThatThrownBy(() -> clusterManagementService.create(config, "cluster"))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("Name of the region has to be specified");
+ });
+ }
+ @Test
+ public void defaultTypeIsPartition() throws Exception {
+ String regionName = testName.getMethodName();
locator.invoke(() -> {
RegionConfig config = new RegionConfig();
config.setName(regionName);
- config.setRefid(RegionShortcut.PARTITION.toString());
ClusterManagementResult result = ClusterStartupRule.getLocator().getClusterManagementService()
.create(config, "cluster");
assertThat(result.isSuccessful()).isTrue();
});
- gfsh.executeAndAssertThat("list regions")
- .statusIsSuccess()
- .containsOutput(regionName);
+ server.invoke(() -> verifyRegionCreated(regionName, "PARTITION"));
+ locator.invoke(() -> verifyRegionPersisted(regionName, "PARTITION"));
+ }
- locator.invoke(() -> {
- InternalConfigurationPersistenceService cc =
- ClusterStartupRule.getLocator().getConfigurationPersistenceService();
- CacheConfig config = cc.getCacheConfig("cluster");
-
- List<RegionConfig> regions = config.getRegions();
- assertThat(regions).isNotEmpty();
- RegionConfig regionConfig = regions.get(1);
- assertThat(regionConfig).isNotNull();
- assertThat(regionConfig.getName()).isEqualTo(regionName);
- assertThat(regionConfig.getRefid()).isEqualTo("PARTITION");
- assertThat(regionConfig.getIndexes()).isEmpty();
- assertThat(regionConfig.getRegions()).isEmpty();
- assertThat(regionConfig.getEntries()).isEmpty();
- assertThat(regionConfig.getCustomRegionElements()).isEmpty();
- });
+ private static void verifyRegionPersisted(String regionName, String type) {
+ CacheConfig cacheConfig =
+ ClusterStartupRule.getLocator().getConfigurationPersistenceService()
+ .getCacheConfig("cluster");
+ RegionConfig regionConfig = CacheElement.findElement(cacheConfig.getRegions(), regionName);
+ assertThat(regionConfig.getType()).isEqualTo(type);
+ }
+
+ private static void verifyRegionCreated(String regionName, String type) {
+ Cache cache = ClusterStartupRule.getCache();
+ Region region = cache.getRegion(regionName);
+ assertThat(region).isNotNull();
+ assertThat(region.getAttributes().getDataPolicy().toString()).isEqualTo(type);
}
}
diff --git a/geode-core/src/integrationTest/java/org/apache/geode/management/internal/configuration/mutators/RegionConfigMutatorIntegrationTest.java b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/configuration/mutators/RegionConfigMutatorIntegrationTest.java
index eba45d4..a182652 100644
--- a/geode-core/src/integrationTest/java/org/apache/geode/management/internal/configuration/mutators/RegionConfigMutatorIntegrationTest.java
+++ b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/configuration/mutators/RegionConfigMutatorIntegrationTest.java
@@ -41,7 +41,7 @@ public class RegionConfigMutatorIntegrationTest {
@Test
public void sanity() throws Exception {
- config.setRefid("REPLICATE");
+ config.setType("REPLICATE");
config.setName("test");
CacheConfig cacheConfig =
locator.getLocator().getConfigurationPersistenceService().getCacheConfig("cluster", true);
diff --git a/geode-core/src/integrationTest/java/org/apache/geode/management/internal/configuration/realizers/RegionConfigRealizerIntegrationTest.java b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/configuration/realizers/RegionConfigRealizerIntegrationTest.java
index a391f0a..61bd0ab 100644
--- a/geode-core/src/integrationTest/java/org/apache/geode/management/internal/configuration/realizers/RegionConfigRealizerIntegrationTest.java
+++ b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/configuration/realizers/RegionConfigRealizerIntegrationTest.java
@@ -43,7 +43,7 @@ public class RegionConfigRealizerIntegrationTest {
@Test
public void sanityCheck() throws Exception {
config.setName("test");
- config.setRefid("REPLICATE");
+ config.setType("REPLICATE");
realizer.create(config, server.getCache());
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java b/geode-core/src/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java
index 0177354..17c5102 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java
@@ -41,6 +41,7 @@ import org.apache.geode.management.internal.cli.functions.UpdateCacheFunction;
import org.apache.geode.management.internal.configuration.mutators.ConfigurationMutator;
import org.apache.geode.management.internal.configuration.mutators.RegionConfigMutator;
import org.apache.geode.management.internal.configuration.validators.ConfigurationValidator;
+import org.apache.geode.management.internal.configuration.validators.RegionConfigValidator;
import org.apache.geode.management.internal.exceptions.EntityExistsException;
import org.apache.geode.management.internal.exceptions.NoMembersException;
@@ -58,6 +59,7 @@ public class LocatorClusterManagementService implements ClusterManagementService
mutators.put(RegionConfig.class, new RegionConfigMutator());
// initialize the list of validators
+ validators.put(RegionConfig.class, new RegionConfigValidator());
}
@VisibleForTesting
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/validators/ConfigurationValidator.java b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/validators/ConfigurationValidator.java
index fec118e..33ce447 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/validators/ConfigurationValidator.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/validators/ConfigurationValidator.java
@@ -18,5 +18,16 @@ import org.apache.geode.cache.configuration.CacheElement;
public interface ConfigurationValidator<T extends CacheElement> {
- public void validate(T config);
+ /**
+ * This is used to validate the configuration object passed in by the user
+ *
+ * This will be called after the ClusterManagementService received the configuration object from
+ * the api call and before passing it to the realizers and mutators.
+ *
+ *
+ * @param config the user defined configuration object. It is mutable. you can modify the
+ * values in the configuration object. e.g. add default values
+ *
+ */
+ public void validate(T config) throws IllegalArgumentException;
}
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/validators/ConfigurationValidator.java b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/validators/RegionConfigValidator.java
similarity index 62%
copy from geode-core/src/main/java/org/apache/geode/management/internal/configuration/validators/ConfigurationValidator.java
copy to geode-core/src/main/java/org/apache/geode/management/internal/configuration/validators/RegionConfigValidator.java
index fec118e..d3919dd 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/validators/ConfigurationValidator.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/validators/RegionConfigValidator.java
@@ -12,11 +12,22 @@
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/
+
package org.apache.geode.management.internal.configuration.validators;
-import org.apache.geode.cache.configuration.CacheElement;
+import org.apache.geode.cache.configuration.RegionConfig;
-public interface ConfigurationValidator<T extends CacheElement> {
+public class RegionConfigValidator implements ConfigurationValidator<RegionConfig> {
+ public static String DEFAULT_REGION_TYPE = "PARTITION";
- public void validate(T config);
+ @Override
+ public void validate(RegionConfig config)
+ throws IllegalArgumentException {
+ if (config.getName() == null) {
+ throw new IllegalArgumentException("Name of the region has to be specified.");
+ }
+ if (config.getType() == null) {
+ config.setType(DEFAULT_REGION_TYPE);
+ }
+ }
}
diff --git a/geode-core/src/test/java/org/apache/geode/cache/configuration/CacheConfigTest.java b/geode-core/src/test/java/org/apache/geode/cache/configuration/CacheConfigTest.java
index 4dbc325..40dc5e5 100644
--- a/geode-core/src/test/java/org/apache/geode/cache/configuration/CacheConfigTest.java
+++ b/geode-core/src/test/java/org/apache/geode/cache/configuration/CacheConfigTest.java
@@ -48,7 +48,7 @@ public class CacheConfigTest {
service.validateWithLocalCacheXSD();
regionConfig = new RegionConfig();
regionConfig.setName("regionA");
- regionConfig.setRefid("REPLICATE");
+ regionConfig.setType("REPLICATE");
regionXml = "<region name=\"regionA\" refid=\"REPLICATE\">";
classNameTypeXml = "<class-name>my.className</class-name>";
@@ -172,7 +172,7 @@ public class CacheConfigTest {
cacheConfig = new CacheConfig("1.0");
RegionConfig regionConfig = new RegionConfig();
regionConfig.setName("test");
- regionConfig.setRefid("REPLICATE");
+ regionConfig.setType("REPLICATE");
RegionAttributesType attributes = new RegionAttributesType();
attributes.setCacheLoader(new DeclarableType("abc.Foo"));
regionConfig.setRegionAttributes(attributes);
diff --git a/geode-core/src/test/java/org/apache/geode/cache/configuration/RegionConfigTest.java b/geode-core/src/test/java/org/apache/geode/cache/configuration/RegionConfigTest.java
index 737c04c..45efaa5 100644
--- a/geode-core/src/test/java/org/apache/geode/cache/configuration/RegionConfigTest.java
+++ b/geode-core/src/test/java/org/apache/geode/cache/configuration/RegionConfigTest.java
@@ -20,6 +20,7 @@ import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
import java.io.File;
import java.net.URL;
+import com.fasterxml.jackson.databind.ObjectMapper;
import org.apache.commons.io.FileUtils;
import org.junit.Before;
import org.junit.Test;
@@ -30,7 +31,7 @@ import org.apache.geode.internal.config.JAXBService;
public class RegionConfigTest {
private JAXBService service;
- private CacheConfig cacheConfig, master;
+ private CacheConfig master;
private RegionConfig regionConfig;
private URL xmlResource;
@@ -66,15 +67,39 @@ public class RegionConfigTest {
}
@Test
- public void checkDefaultRegionAttributesForShortcuts() throws Exception {
+ public void checkDefaultRegionAttributesForShortcuts() {
RegionShortcut[] shortcuts = RegionShortcut.values();
for (RegionShortcut shortcut : shortcuts) {
RegionConfig config = new RegionConfig();
- config.setRefid(shortcut.name());
+ config.setType(shortcut.name());
config.setName(shortcut.name());
RegionConfig masterRegion = CacheElement.findElement(master.getRegions(), shortcut.name());
assertThat(config).isEqualToComparingFieldByFieldRecursively(masterRegion);
}
}
+ @Test
+ public void invalidType() {
+ regionConfig.setName("test");
+ assertThatThrownBy(() -> regionConfig.setType("INVALID-TYPE")).isInstanceOf(
+ IllegalArgumentException.class);
+ }
+
+ @Test
+ public void correctJsonAndXml() throws Exception {
+ String json = "{\"name\":\"test\", \"type\":\"REPLICATE\"}";
+ ObjectMapper mapper = new ObjectMapper();
+ regionConfig = mapper.readValue(json, RegionConfig.class);
+ assertThat(regionConfig.getName()).isEqualTo("test");
+ assertThat(regionConfig.getType()).isEqualTo("REPLICATE");
+
+ String json2 = mapper.writeValueAsString(regionConfig);
+ assertThat(json2).contains("\"type\":\"REPLICATE\"");
+ assertThat(json2).contains("\"id\":\"test\"");
+
+ CacheConfig cacheConfig = new CacheConfig();
+ cacheConfig.getRegions().add(regionConfig);
+ String xml = service.marshall(cacheConfig);
+ assertThat(xml).contains("<region name=\"test\" refid=\"REPLICATE\"");
+ }
}
diff --git a/geode-core/src/test/java/org/apache/geode/distributed/internal/InternalConfigurationPersistenceServiceTest.java b/geode-core/src/test/java/org/apache/geode/distributed/internal/InternalConfigurationPersistenceServiceTest.java
index 574cc07..03252ee 100644
--- a/geode-core/src/test/java/org/apache/geode/distributed/internal/InternalConfigurationPersistenceServiceTest.java
+++ b/geode-core/src/test/java/org/apache/geode/distributed/internal/InternalConfigurationPersistenceServiceTest.java
@@ -82,7 +82,7 @@ public class InternalConfigurationPersistenceServiceTest {
service.updateCacheConfig("cluster", cacheConfig -> {
RegionConfig regionConfig = new RegionConfig();
regionConfig.setName("regionA");
- regionConfig.setRefid("REPLICATE");
+ regionConfig.setType("REPLICATE");
cacheConfig.getRegions().add(regionConfig);
return cacheConfig;
});
diff --git a/geode-core/src/test/java/org/apache/geode/internal/config/JAXBServiceTest.java b/geode-core/src/test/java/org/apache/geode/internal/config/JAXBServiceTest.java
index b969199..0eb0388 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/config/JAXBServiceTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/config/JAXBServiceTest.java
@@ -218,7 +218,7 @@ public class JAXBServiceTest {
RegionConfig region = new RegionConfig();
region.setName("testRegion");
- region.setRefid("REPLICATE");
+ region.setType("REPLICATE");
cache.getRegions().add(region);
}
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AlterRegionCommandTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AlterRegionCommandTest.java
index beada6c..87553e7 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AlterRegionCommandTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AlterRegionCommandTest.java
@@ -78,7 +78,7 @@ public class AlterRegionCommandTest {
cacheConfig = new CacheConfig();
existingRegionConfig = new RegionConfig();
existingRegionConfig.setName("/regionA");
- existingRegionConfig.setRefid("REPLICATE");
+ existingRegionConfig.setType("REPLICATE");
cacheConfig.getRegions().add(existingRegionConfig);
when(ccService.getCacheConfig("cluster")).thenReturn(cacheConfig);
}
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/configuration/realizers/RegionConfigRealizerTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/realizers/RegionConfigRealizerTest.java
index 55b127c..dcb3ade 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/configuration/realizers/RegionConfigRealizerTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/realizers/RegionConfigRealizerTest.java
@@ -45,7 +45,7 @@ public class RegionConfigRealizerTest {
public void createsPartitionedInCache() {
RegionConfig config = new RegionConfig();
config.setName("regionName");
- config.setRefid("PARTITION");
+ config.setType("PARTITION");
RegionConfigRealizer realizer = new RegionConfigRealizer();
realizer.create(config, cache);
@@ -61,7 +61,7 @@ public class RegionConfigRealizerTest {
public void createsReplicateInCache() {
RegionConfig config = new RegionConfig();
config.setName("regionName");
- config.setRefid("REPLICATE");
+ config.setType("REPLICATE");
RegionConfigRealizer subject = new RegionConfigRealizer();
subject.create(config, cache);
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/configuration/validators/RegionConfigValidatorTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/validators/RegionConfigValidatorTest.java
new file mode 100644
index 0000000..6ce58c3
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/validators/RegionConfigValidatorTest.java
@@ -0,0 +1,60 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.geode.management.internal.configuration.validators;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.geode.cache.configuration.RegionConfig;
+
+public class RegionConfigValidatorTest {
+
+ private RegionConfigValidator validator;
+ private RegionConfig config;
+
+ @Before
+ public void before() throws Exception {
+ validator = new RegionConfigValidator();
+ config = new RegionConfig();
+ }
+
+ @Test
+ public void noChangesWhenTypeIsSet() {
+ config.setName("regionName");
+ config.setType("REPLICATE");
+ validator.validate(config);
+ assertThat(config.getType()).isEqualTo("REPLICATE");
+ }
+
+ @Test
+ public void defaultsTypeToPartitioned() {
+ config.setName("regionName");
+ validator.validate(config);
+ assertThat(config.getType()).isEqualTo("PARTITION");
+ }
+
+ @Test
+ public void noName() throws Exception {
+ assertThatThrownBy(() -> validator.validate(config)).isInstanceOf(
+ IllegalArgumentException.class)
+ .hasMessageContaining("Name of the region has to be specified");
+ }
+}
diff --git a/geode-management/src/main/java/org/apache/geode/cache/configuration/RegionConfig.java b/geode-management/src/main/java/org/apache/geode/cache/configuration/RegionConfig.java
index d97a0d1..7c25433 100644
--- a/geode-management/src/main/java/org/apache/geode/cache/configuration/RegionConfig.java
+++ b/geode-management/src/main/java/org/apache/geode/cache/configuration/RegionConfig.java
@@ -163,13 +163,13 @@ public class RegionConfig implements CacheElement {
@XmlAttribute(name = "name", required = true)
protected String name;
@XmlAttribute(name = "refid")
- protected String refid;
+ protected String type;
public RegionConfig() {}
public RegionConfig(String name, String refid) {
this.name = name;
- this.refid = refid;
+ this.type = refid;
}
public RegionAttributesType getRegionAttributes() {
@@ -333,25 +333,25 @@ public class RegionConfig implements CacheElement {
}
/**
- * Gets the value of the refid property.
+ * Gets the value of the type property.
*
* possible object is
* {@link String }
*
*/
- public String getRefid() {
- return refid;
+ public String getType() {
+ return type;
}
/**
- * Sets the value of the refid property.
+ * Sets the value of the type property.
*
* allowed object is
* {@link String }
*
*/
- public void setRefid(String value) {
- this.refid = value;
+ public void setType(String value) {
+ this.type = value;
setShortcutAttributes();
}
@@ -360,7 +360,7 @@ public class RegionConfig implements CacheElement {
regionAttributes = new RegionAttributesType();
}
- switch (refid) {
+ switch (type) {
case "PARTITION": {
regionAttributes.setDataPolicy(RegionAttributesDataPolicy.PARTITION);
break;
@@ -487,7 +487,7 @@ public class RegionConfig implements CacheElement {
break;
}
default:
- throw new IllegalArgumentException("invalid refid " + refid);
+ throw new IllegalArgumentException("invalid type " + type);
}
}
diff --git a/geode-management/src/main/java/org/apache/geode/management/internal/api/ClusterManagementService.java b/geode-management/src/main/java/org/apache/geode/management/internal/api/ClusterManagementService.java
index b9c392e..1a88592 100644
--- a/geode-management/src/main/java/org/apache/geode/management/internal/api/ClusterManagementService.java
+++ b/geode-management/src/main/java/org/apache/geode/management/internal/api/ClusterManagementService.java
@@ -30,6 +30,7 @@ public interface ClusterManagementService {
*
* @param config this holds the configuration attributes of the element you are trying to create
* on the cluster
+ * @throws IllegalArgumentException, NoMemberException, EntityExistsException
*/
ClusterManagementResult create(CacheElement config, String group);