GEODE-420: Reverting checkAttributes change in AbstractDistributionConfig.java.
Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/6179a698 Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/6179a698 Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/6179a698 Branch: refs/heads/develop Commit: 6179a6988afd75f1441d65c737b2e4ef3b9d11fe Parents: 31cbd2f Author: Udo Kohlmeyer <[email protected]> Authored: Wed Aug 17 06:00:12 2016 +1000 Committer: Udo Kohlmeyer <[email protected]> Committed: Wed Aug 17 06:00:12 2016 +1000 ---------------------------------------------------------------------- .../internal/AbstractDistributionConfig.java | 44 +------------ .../internal/DistributionConfigImpl.java | 67 ++++++++++++++++++++ .../gemfire/distributed/LocatorDUnitTest.java | 1 - .../internal/DistributionConfigJUnitTest.java | 3 - 4 files changed, 69 insertions(+), 46 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/6179a698/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/AbstractDistributionConfig.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/AbstractDistributionConfig.java b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/AbstractDistributionConfig.java index ce835c5..282a239 100644 --- a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/AbstractDistributionConfig.java +++ b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/AbstractDistributionConfig.java @@ -464,9 +464,6 @@ public abstract class AbstractDistributionConfig extends AbstractConfig implemen @ConfigAttributeChecker(name = SSL_ENABLED_COMPONENTS) protected SSLEnabledComponent[] checkLegacySSLWhenSSLEnabledComponentsSet(SSLEnabledComponent[] value) { for (SSLEnabledComponent component : value) { - if (!isAliasCorrectlyConfiguredForComponents(component)) { - throw new IllegalArgumentException(LocalizedStrings.AbstractDistributionConfig_SSL_ENABLED_COMPONENTS_INVALID_ALIAS_OPTIONS.toLocalizedString()); - } switch (component) { case ALL: case CLUSTER: @@ -498,43 +495,7 @@ public abstract class AbstractDistributionConfig extends AbstractConfig implemen return value; } - private boolean isAliasCorrectlyConfiguredForComponents(final SSLEnabledComponent component) { - switch (component) { - case ALL: { - //If the default alias is not set, then check that all the other component aliases are set - if (StringUtils.isEmpty(getSSLDefaultAlias())) { - boolean correctAlias = true; - correctAlias &= isAliasCorrectlyConfiguredForComponents(SSLEnabledComponent.CLUSTER); - correctAlias &= isAliasCorrectlyConfiguredForComponents(SSLEnabledComponent.GATEWAY); - correctAlias &= isAliasCorrectlyConfiguredForComponents(SSLEnabledComponent.HTTP_SERVICE); - correctAlias &= isAliasCorrectlyConfiguredForComponents(SSLEnabledComponent.JMX); - correctAlias &= isAliasCorrectlyConfiguredForComponents(SSLEnabledComponent.LOCATOR); - correctAlias &= isAliasCorrectlyConfiguredForComponents(SSLEnabledComponent.SERVER); - return correctAlias; - } - } - case CLUSTER: { - return StringUtils.isEmpty(getClusterSSLAlias()) ? true : (getSSLEnabledComponents().length > 1 ? !StringUtils.isEmpty(getSSLDefaultAlias()) : true); - } - case GATEWAY: { - return StringUtils.isEmpty(getGatewaySSLAlias()) ? true : (getSSLEnabledComponents().length > 1 ? !StringUtils.isEmpty(getSSLDefaultAlias()) : true); - } - case HTTP_SERVICE: { - return StringUtils.isEmpty(getHTTPServiceSSLAlias()) ? true : (getSSLEnabledComponents().length > 1 ? !StringUtils.isEmpty(getSSLDefaultAlias()) : true); - } - case JMX: { - return StringUtils.isEmpty(getJMXManagerSSLAlias()) ? true : (getSSLEnabledComponents().length > 1 ? !StringUtils.isEmpty(getSSLDefaultAlias()) : true); - } - case LOCATOR: { - return StringUtils.isEmpty(getLocatorSSLAlias()) ? true : (getSSLEnabledComponents().length > 1 ? !StringUtils.isEmpty(getSSLDefaultAlias()) : true); - } - case SERVER: { - return StringUtils.isEmpty(getServerSSLAlias()) ? true : (getSSLEnabledComponents().length > 1 ? !StringUtils.isEmpty(getSSLDefaultAlias()) : true); - } - default: - return false; - } - } + // AbstractConfig overriding methods @@ -597,8 +558,7 @@ public abstract class AbstractDistributionConfig extends AbstractConfig implemen throw new InternalGemFireException("the attribute setter must have one and only one parametter"); } - //Moved this to the outside loop to complete the setting of configuration before checking their validity. - // checkAttribute(attName, attValue); + checkAttribute(attName, attValue); try { setter.invoke(this, attValue); } catch (Exception e) { http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/6179a698/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfigImpl.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfigImpl.java b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfigImpl.java index 7c2d551..26263d3 100644 --- a/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfigImpl.java +++ b/geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfigImpl.java @@ -35,11 +35,13 @@ import java.util.Map; import java.util.Properties; import java.util.Set; +import org.apache.commons.lang.StringUtils; import org.apache.geode.redis.GeodeRedisServer; import com.gemstone.gemfire.GemFireConfigException; import com.gemstone.gemfire.GemFireIOException; import com.gemstone.gemfire.InternalGemFireException; +import com.gemstone.gemfire.distributed.ConfigurationProperties; import com.gemstone.gemfire.distributed.DistributedSystem; import com.gemstone.gemfire.internal.ConfigSource; import com.gemstone.gemfire.internal.i18n.LocalizedStrings; @@ -919,11 +921,76 @@ public class DistributionConfigImpl extends AbstractDistributionConfig implement // Make attributes writeable only this.modifiable = true; validateConfigurationProperties(props); + validateSSLEnabledComponentsConfiguration(); // Make attributes read only this.modifiable = false; } + private void validateSSLEnabledComponentsConfiguration() { + Object value = null; + try { + Method method = getters.get(ConfigurationProperties.SSL_ENABLED_COMPONENTS); + if (method != null) { + value = method.invoke(this, new Object[] {}); + } + } catch (Exception e) { + if (e instanceof RuntimeException) { + throw (RuntimeException) e; + } + if (e.getCause() instanceof RuntimeException) { + throw (RuntimeException) e.getCause(); + } else { + throw new InternalGemFireException("error invoking getter for property" + ConfigurationProperties.SSL_ENABLED_COMPONENTS); + } + } + SSLEnabledComponent[] sslEnabledComponents = (SSLEnabledComponent[]) value; + for (SSLEnabledComponent sslEnabledComponent : sslEnabledComponents) { + if (!isAliasCorrectlyConfiguredForComponents(sslEnabledComponent)) { + throw new IllegalArgumentException(LocalizedStrings.AbstractDistributionConfig_SSL_ENABLED_COMPONENTS_INVALID_ALIAS_OPTIONS.toLocalizedString()); + } + } + + } + + private boolean isAliasCorrectlyConfiguredForComponents(final SSLEnabledComponent component) { + switch (component) { + case ALL: { + //If the default alias is not set, then check that all the other component aliases are set + if (StringUtils.isEmpty(getSSLDefaultAlias())) { + boolean correctAlias = true; + correctAlias &= isAliasCorrectlyConfiguredForComponents(SSLEnabledComponent.CLUSTER); + correctAlias &= isAliasCorrectlyConfiguredForComponents(SSLEnabledComponent.GATEWAY); + correctAlias &= isAliasCorrectlyConfiguredForComponents(SSLEnabledComponent.HTTP_SERVICE); + correctAlias &= isAliasCorrectlyConfiguredForComponents(SSLEnabledComponent.JMX); + correctAlias &= isAliasCorrectlyConfiguredForComponents(SSLEnabledComponent.LOCATOR); + correctAlias &= isAliasCorrectlyConfiguredForComponents(SSLEnabledComponent.SERVER); + return correctAlias; + } + } + case CLUSTER: { + return StringUtils.isEmpty(getClusterSSLAlias()) ? true : (getSSLEnabledComponents().length > 1 ? !StringUtils.isEmpty(getSSLDefaultAlias()) : true); + } + case GATEWAY: { + return StringUtils.isEmpty(getGatewaySSLAlias()) ? true : (getSSLEnabledComponents().length > 1 ? !StringUtils.isEmpty(getSSLDefaultAlias()) : true); + } + case HTTP_SERVICE: { + return StringUtils.isEmpty(getHTTPServiceSSLAlias()) ? true : (getSSLEnabledComponents().length > 1 ? !StringUtils.isEmpty(getSSLDefaultAlias()) : true); + } + case JMX: { + return StringUtils.isEmpty(getJMXManagerSSLAlias()) ? true : (getSSLEnabledComponents().length > 1 ? !StringUtils.isEmpty(getSSLDefaultAlias()) : true); + } + case LOCATOR: { + return StringUtils.isEmpty(getLocatorSSLAlias()) ? true : (getSSLEnabledComponents().length > 1 ? !StringUtils.isEmpty(getSSLDefaultAlias()) : true); + } + case SERVER: { + return StringUtils.isEmpty(getServerSSLAlias()) ? true : (getSSLEnabledComponents().length > 1 ? !StringUtils.isEmpty(getSSLDefaultAlias()) : true); + } + default: + return false; + } + } + /** * Here we will validate the correctness of the set properties as per the CheckAttributeChecker annotations defined in #AbstractDistributionConfig * @param props http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/6179a698/geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorDUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorDUnitTest.java b/geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorDUnitTest.java index 9fe4e8f..2bff100 100755 --- a/geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorDUnitTest.java +++ b/geode-core/src/test/java/com/gemstone/gemfire/distributed/LocatorDUnitTest.java @@ -26,7 +26,6 @@ import java.util.List; import java.util.Properties; import java.util.Set; -import com.sun.xml.internal.ws.api.FeatureListValidatorAnnotation; import org.junit.Test; import org.junit.experimental.categories.Category; http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/6179a698/geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/DistributionConfigJUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/DistributionConfigJUnitTest.java b/geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/DistributionConfigJUnitTest.java index ad10f0c..aa65a41 100644 --- a/geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/DistributionConfigJUnitTest.java +++ b/geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/DistributionConfigJUnitTest.java @@ -279,7 +279,6 @@ public class DistributionConfigJUnitTest { @Test(expected = UnmodifiableException.class) public void testSetUnmodifiableAttributeObject() { config.setAttributeObject(ARCHIVE_DISK_SPACE_LIMIT, 0, ConfigSource.api()); - config.checkAttribute(ARCHIVE_DISK_SPACE_LIMIT, 0); } @Test @@ -291,7 +290,6 @@ public class DistributionConfigJUnitTest { @Test(expected = IllegalArgumentException.class) public void testOutOfRangeAttributeObject() { config.setAttributeObject(HTTP_SERVICE_PORT, -1, ConfigSource.api()); - config.checkAttribute(HTTP_SERVICE_PORT, -1); } @Test @@ -318,7 +316,6 @@ public class DistributionConfigJUnitTest { config.modifiable = true; // config.setStartLocator(address); config.setAttributeObject(START_LOCATOR,address,ConfigSource.api()); - config.checkAttribute(START_LOCATOR,address); } @Test
