Skip to content

Commit

Permalink
GEODE-6174: change refid to type and make the default type to be PART…
Browse files Browse the repository at this point in the history 
…ITION

Co-authored-by: Peter Tran <ptran@pivotal.io>

* change refid to type for better UX
* add a common config validator interface and use that to validate RegionConfig
  • Loading branch information
@jinmeiliao
Aditya Anchuri authored and jinmeiliao committed Jan 28, 2019
1 parent fdb5627 commit 81884fd
Show file tree
Hide file tree
Showing 17 changed files with 268 additions and 101 deletions.
2 changes: 1 addition & 1 deletion ...org/apache/geode/management/internal/rest/ClusterManagementLocatorReconnectDunitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public void clusterManagementRestServiceStillWorksAfterLocatorReconnects() throw
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);

Expand Down
84 changes: 63 additions & 21 deletions ...ributedTest/java/org/apache/geode/management/internal/rest/RegionManagementDunitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,45 +18,48 @@
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 =
new GeodeDevRestClient("/geode-management/v2", "localhost", locator.getHttpPort(), false);
}

@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();

Expand All @@ -65,20 +68,59 @@ public void createRegion() throws Exception {
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);
}
}
2 changes: 1 addition & 1 deletion ...rationTest/java/org/apache/geode/management/internal/RegionManagementIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public static void setUpClass() throws Exception {
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);
Expand Down
2 changes: 1 addition & 1 deletion ...c/distributedTest/java/org/apache/geode/distributed/internal/CacheConfigDAODUnitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public void xmlCreatedByCCServiceCanBeLoadedByServer() {
ccService.updateCacheConfig("cluster", cc -> {
RegionConfig regionConfig = new RegionConfig();
regionConfig.setName("regionB");
regionConfig.setRefid("REPLICATE");
regionConfig.setType("REPLICATE");
cc.getRegions().add(regionConfig);
return cc;
});
Expand Down
103 changes: 47 additions & 56 deletions ...src/distributedTest/java/org/apache/geode/management/internal/api/RegionAPIDUnitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -39,19 +38,19 @@

@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());

Expand All @@ -64,23 +63,13 @@ public void createsPartitionedRegion() {
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);

Expand All @@ -89,6 +78,8 @@ public void createsPartitionedRegion() {
gfsh.executeAndAssertThat("get --key='foo' --region=" + regionName)
.statusIsSuccess()
.containsKeyValuePair("Value", "\"125\"");

locator.invoke(() -> verifyRegionPersisted(regionName, "PARTITION"));
}

@Test
Expand All @@ -97,56 +88,56 @@ public void createsReplicatedRegion() {
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);
}
}
2 changes: 2 additions & 0 deletions ...c/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
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;

Expand All @@ -58,6 +59,7 @@ public LocatorClusterManagementService(DistributionManager distributionManager,
mutators.put(RegionConfig.class, new RegionConfigMutator());

// initialize the list of validators
validators.put(RegionConfig.class, new RegionConfigValidator());
}

@VisibleForTesting
Expand Down
13 changes: 12 additions & 1 deletion ...org/apache/geode/management/internal/configuration/validators/ConfigurationValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,16 @@

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;
}

0 comments on commit 81884fd

Please sign in to comment.