Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GEODE-6174: move exists method into the validator #3217

Merged
merged 2 commits into from Feb 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -21,15 +21,15 @@

import org.apache.geode.cache.Region;

class RegionNameValidation {
public class RegionNameValidation {

private static final Pattern NAME_PATTERN = Pattern.compile("[aA-zZ0-9-_.]+");

static Pattern getNamePattern() {
return NAME_PATTERN;
}

static void validate(String name) {
public static void validate(String name) {
validate(name, new InternalRegionArguments());
}

Expand Down
Expand Up @@ -93,9 +93,10 @@ public ClusterManagementResult create(CacheElement config, String group) {
}
}


// 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.");
}

Expand Down
Expand Up @@ -31,8 +31,6 @@
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);
Expand Down
Expand Up @@ -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> {
Expand All @@ -30,11 +29,6 @@ public void add(RegionConfig configElement, CacheConfig existingConfig) {
existingConfig.getRegions().add(configElement);
}

@Override
public boolean exists(RegionConfig config, CacheConfig existing) {
return CacheElement.exists(existing.getRegions(), config.getId());
}

@Override
public void update(RegionConfig config, CacheConfig existing) {

Expand Down
Expand Up @@ -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> {
Expand All @@ -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);
}
Expand Up @@ -15,19 +15,32 @@

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

@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());
}
}
Expand Up @@ -52,9 +52,26 @@ public void defaultsTypeToPartitioned() {
}

@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");
}
}