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/21 23:28:07 UTC
[geode] branch develop updated: GEODE-6174: move exists method into
the validator (#3217)
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 bc3743e GEODE-6174: move exists method into the validator (#3217)
bc3743e is described below
commit bc3743e3106ba7ec5f4b2c9a93f59fe61a9d999f
Author: jinmeiliao <ji...@pivotal.io>
AuthorDate: Thu Feb 21 15:27:56 2019 -0800
GEODE-6174: move exists method into the validator (#3217)
Co-authored-by: Kenneth Howe <kh...@pivotal.io>
* add invalid region name validation
---
.../geode/internal/cache/RegionNameValidation.java | 4 ++--
.../internal/api/LocatorClusterManagementService.java | 3 ++-
.../configuration/mutators/ConfigurationMutator.java | 2 --
.../configuration/mutators/RegionConfigMutator.java | 6 ------
.../validators/ConfigurationValidator.java | 10 +++++++++-
.../validators/RegionConfigValidator.java | 13 +++++++++++++
.../validators/RegionConfigValidatorTest.java | 19 ++++++++++++++++++-
7 files changed, 44 insertions(+), 13 deletions(-)
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/RegionNameValidation.java b/geode-core/src/main/java/org/apache/geode/internal/cache/RegionNameValidation.java
index 380eb9c..21295a0 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/RegionNameValidation.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/RegionNameValidation.java
@@ -21,7 +21,7 @@ import java.util.regex.Pattern;
import org.apache.geode.cache.Region;
-class RegionNameValidation {
+public class RegionNameValidation {
private static final Pattern NAME_PATTERN = Pattern.compile("[aA-zZ0-9-_.]+");
@@ -29,7 +29,7 @@ class RegionNameValidation {
return NAME_PATTERN;
}
- static void validate(String name) {
+ public static void validate(String name) {
validate(name, new InternalRegionArguments());
}
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 74c35cb..69aa45e 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
@@ -93,9 +93,10 @@ public class LocatorClusterManagementService implements ClusterManagementService
}
}
+
// exit early if config element already exists in cache config
CacheConfig currentPersistedConfig = persistenceService.getCacheConfig(group, true);
- if (configurationMutator.exists(config, currentPersistedConfig)) {
+ if (validator.exists(config, currentPersistedConfig)) {
throw new EntityExistsException("cache element " + config.getId() + " already exists.");
}
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/ConfigurationMutator.java b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/ConfigurationMutator.java
index 32a96c3..5f68d45 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/ConfigurationMutator.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/ConfigurationMutator.java
@@ -31,8 +31,6 @@ import org.apache.geode.cache.configuration.CacheElement;
public interface ConfigurationMutator<T extends CacheElement> {
void add(T config, CacheConfig existing);
- boolean exists(T config, CacheConfig existing);
-
void update(T config, CacheConfig existing);
void delete(T config, CacheConfig existing);
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/RegionConfigMutator.java b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/RegionConfigMutator.java
index ec06e8e..c964164 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/RegionConfigMutator.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/RegionConfigMutator.java
@@ -18,7 +18,6 @@
package org.apache.geode.management.internal.configuration.mutators;
import org.apache.geode.cache.configuration.CacheConfig;
-import org.apache.geode.cache.configuration.CacheElement;
import org.apache.geode.cache.configuration.RegionConfig;
public class RegionConfigMutator implements ConfigurationMutator<RegionConfig> {
@@ -31,11 +30,6 @@ public class RegionConfigMutator implements ConfigurationMutator<RegionConfig> {
}
@Override
- public boolean exists(RegionConfig config, CacheConfig existing) {
- return CacheElement.exists(existing.getRegions(), config.getId());
- }
-
- @Override
public void update(RegionConfig config, CacheConfig existing) {
}
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 33ce447..5c789ee 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
@@ -14,6 +14,7 @@
*/
package org.apache.geode.management.internal.configuration.validators;
+import org.apache.geode.cache.configuration.CacheConfig;
import org.apache.geode.cache.configuration.CacheElement;
public interface ConfigurationValidator<T extends CacheElement> {
@@ -29,5 +30,12 @@ public interface ConfigurationValidator<T extends CacheElement> {
* values in the configuration object. e.g. add default values
*
*/
- public void validate(T config) throws IllegalArgumentException;
+ void validate(T config) throws IllegalArgumentException;
+
+ /**
+ * check to see if this configuration already exists
+ *
+ * @return true if this config already exists in the persisted cache configuration
+ */
+ boolean exists(T config, CacheConfig persistedConfig);
}
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/validators/RegionConfigValidator.java b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/validators/RegionConfigValidator.java
index 4caaee6..6c11efc 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/validators/RegionConfigValidator.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/validators/RegionConfigValidator.java
@@ -15,7 +15,10 @@
package org.apache.geode.management.internal.configuration.validators;
+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.internal.cache.RegionNameValidation;
public class RegionConfigValidator implements ConfigurationValidator<RegionConfig> {
public static final String DEFAULT_REGION_TYPE = "PARTITION";
@@ -23,11 +26,21 @@ public class RegionConfigValidator implements ConfigurationValidator<RegionConfi
@Override
public void validate(RegionConfig config)
throws IllegalArgumentException {
+ // validate the name
if (config.getName() == null) {
throw new IllegalArgumentException("Name of the region has to be specified.");
}
+
+ RegionNameValidation.validate(config.getName());
+
+ // validate the type
if (config.getType() == null) {
config.setType(DEFAULT_REGION_TYPE);
}
}
+
+ @Override
+ public boolean exists(RegionConfig config, CacheConfig existing) {
+ return CacheElement.exists(existing.getRegions(), config.getId());
+ }
}
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
index 6ce58c3..af74078 100644
--- 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
@@ -52,9 +52,26 @@ public class RegionConfigValidatorTest {
}
@Test
- public void noName() throws Exception {
+ public void noName() {
assertThatThrownBy(() -> validator.validate(config)).isInstanceOf(
IllegalArgumentException.class)
.hasMessageContaining("Name of the region has to be specified");
}
+
+ @Test
+ public void invalidName1() {
+ config.setName("__test");
+ assertThatThrownBy(() -> validator.validate(config)).isInstanceOf(
+ IllegalArgumentException.class)
+ .hasMessageContaining("Region names may not begin with a double-underscore");
+ }
+
+ @Test
+ public void invalidName2() {
+ config.setName("a!&b");
+ assertThatThrownBy(() -> validator.validate(config)).isInstanceOf(
+ IllegalArgumentException.class)
+ .hasMessageContaining(
+ "Region names may only be alphanumeric and may contain hyphens or underscores");
+ }
}