Validate config().set(ConfigKey<T>, T) on entities, adjuncts and locations
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/9ba5391d Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/9ba5391d Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/9ba5391d Branch: refs/heads/master Commit: 9ba5391d4db5ed0c2b3a6ea867810276d8b73e1e Parents: aa5c00f Author: Sam Corbett <[email protected]> Authored: Fri Oct 9 16:57:33 2015 +0100 Committer: Sam Corbett <[email protected]> Committed: Fri Oct 9 16:57:33 2015 +0100 ---------------------------------------------------------------------- .../brooklyn/core/config/ConfigConstraints.java | 69 +++++++++++++------- .../brooklyn/core/entity/AbstractEntity.java | 2 + .../core/location/AbstractLocation.java | 2 + .../core/objs/AbstractEntityAdjunct.java | 2 + .../core/config/ConfigKeyConstraintTest.java | 27 ++++++++ 5 files changed, 80 insertions(+), 22 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9ba5391d/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java b/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java index b1afe75..1a273f6 100644 --- a/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java +++ b/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java @@ -19,13 +19,16 @@ package org.apache.brooklyn.core.config; +import java.util.Collections; import java.util.Iterator; import java.util.List; import org.apache.brooklyn.api.entity.Entity; +import org.apache.brooklyn.api.location.Location; import org.apache.brooklyn.api.objs.BrooklynObject; import org.apache.brooklyn.api.objs.EntityAdjunct; import org.apache.brooklyn.config.ConfigKey; +import org.apache.brooklyn.core.location.AbstractLocation; import org.apache.brooklyn.core.objs.AbstractEntityAdjunct; import org.apache.brooklyn.core.objs.BrooklynObjectPredicate; import org.apache.brooklyn.core.objs.BrooklynObjectInternal; @@ -74,6 +77,18 @@ public abstract class ConfigConstraints<T extends BrooklynObject> { } } + public static <T> void assertValid(Entity entity, ConfigKey<T> key, T value) { + if (!new EntityConfigConstraints(entity).isValueValid(key, value)) { + throw new ConstraintViolationException("Invalid value for " + key + " on " + entity + ": " + value); + } + } + + public static <T> void assertValid(Location location, ConfigKey<T> key, T value) { + if (!new LocationConfigConstraints(location).isValueValid(key, value)) { + throw new ConstraintViolationException("Invalid value for " + key + " on " + location + ": " + value); + } + } + private static String errorMessage(BrooklynObject object, Iterable<ConfigKey<?>> violations) { StringBuilder message = new StringBuilder("Error configuring ") .append(object.getDisplayName()) @@ -97,10 +112,6 @@ public abstract class ConfigConstraints<T extends BrooklynObject> { abstract Iterable<ConfigKey<?>> getBrooklynObjectTypeConfigKeys(); - public boolean isValid() { - return Iterables.isEmpty(getViolations()); - } - public Iterable<ConfigKey<?>> getViolations() { return validateAll(); } @@ -108,36 +119,39 @@ public abstract class ConfigConstraints<T extends BrooklynObject> { @SuppressWarnings("unchecked") private Iterable<ConfigKey<?>> validateAll() { List<ConfigKey<?>> violating = Lists.newLinkedList(); - BrooklynObjectInternal.ConfigurationSupportInternal configInternal = getConfigurationSupportInternal(); - Iterable<ConfigKey<?>> configKeys = getBrooklynObjectTypeConfigKeys(); LOG.trace("Checking config keys on {}: {}", getBrooklynObject(), configKeys); for (ConfigKey<?> configKey : configKeys) { + BrooklynObjectInternal.ConfigurationSupportInternal configInternal = getConfigurationSupportInternal(); // getNonBlocking method coerces the value to the config key's type. Maybe<?> maybeValue = configInternal.getNonBlocking(configKey); if (maybeValue.isPresent()) { - Object value = maybeValue.get(); - try { - // Cast is safe because the author of the constraint on the config key had to - // keep its type to Predicte<? super T>, where T is ConfigKey<T>. - Predicate<Object> po = (Predicate<Object>) configKey.getConstraint(); - boolean isValid; - if (po instanceof BrooklynObjectPredicate) { - isValid = BrooklynObjectPredicate.class.cast(po).apply(value, brooklynObject); - } else { - isValid = po.apply(value); - } - if (!isValid) { - violating.add(configKey); - } - } catch (Exception e) { - LOG.debug("Error checking constraint on " + configKey.getName(), e); + // Cast is safe because the author of the constraint on the config key had to + // keep its type to Predicte<? super T>, where T is ConfigKey<T>. + ConfigKey<Object> ck = (ConfigKey<Object>) configKey; + if (!isValueValid(ck, maybeValue.get())) { + violating.add(configKey); } } } return violating; } + @SuppressWarnings("unchecked") + <V> boolean isValueValid(ConfigKey<V> configKey, V value) { + try { + Predicate<? super V> po = configKey.getConstraint(); + if (po instanceof BrooklynObjectPredicate) { + return BrooklynObjectPredicate.class.cast(po).apply(value, brooklynObject); + } else { + return po.apply(value); + } + } catch (Exception e) { + LOG.debug("Error checking constraint on " + configKey.getName(), e); + } + return true; + } + private BrooklynObjectInternal.ConfigurationSupportInternal getConfigurationSupportInternal() { return ((BrooklynObjectInternal) brooklynObject).config(); } @@ -168,4 +182,15 @@ public abstract class ConfigConstraints<T extends BrooklynObject> { } } + private static class LocationConfigConstraints extends ConfigConstraints<Location> { + public LocationConfigConstraints(Location brooklynObject) { + super(brooklynObject); + } + + @Override + Iterable<ConfigKey<?>> getBrooklynObjectTypeConfigKeys() { + return Collections.emptyList(); + } + } + } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9ba5391d/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java b/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java index a2e12f4..49afc60 100644 --- a/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java +++ b/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java @@ -58,6 +58,7 @@ import org.apache.brooklyn.config.ConfigKey.HasConfigKey; import org.apache.brooklyn.core.BrooklynFeatureEnablement; import org.apache.brooklyn.core.BrooklynLogging; import org.apache.brooklyn.core.catalog.internal.CatalogUtils; +import org.apache.brooklyn.core.config.ConfigConstraints; import org.apache.brooklyn.core.config.render.RendererHints; import org.apache.brooklyn.core.enricher.AbstractEnricher; import org.apache.brooklyn.core.entity.internal.EntityConfigMap; @@ -1144,6 +1145,7 @@ public abstract class AbstractEntity extends AbstractBrooklynObject implements E @Override public <T> T set(ConfigKey<T> key, T val) { + ConfigConstraints.assertValid(AbstractEntity.this, key, val); return setConfigInternal(key, val); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9ba5391d/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java b/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java index 0fd3f7b..98a7e38 100644 --- a/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java +++ b/core/src/main/java/org/apache/brooklyn/core/location/AbstractLocation.java @@ -47,6 +47,7 @@ import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.config.ConfigKey.HasConfigKey; import org.apache.brooklyn.core.BrooklynFeatureEnablement; import org.apache.brooklyn.core.config.BasicConfigKey; +import org.apache.brooklyn.core.config.ConfigConstraints; import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.core.internal.storage.BrooklynStorage; import org.apache.brooklyn.core.internal.storage.Reference; @@ -400,6 +401,7 @@ public abstract class AbstractLocation extends AbstractBrooklynObject implements @Override public <T> T set(ConfigKey<T> key, T val) { + ConfigConstraints.assertValid(AbstractLocation.this, key, val); T result = configBag.put(key, val); onChanged(); return result; http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9ba5391d/core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java b/core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java index a7b00f4..3fc2839 100644 --- a/core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java +++ b/core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java @@ -42,6 +42,7 @@ import org.apache.brooklyn.api.sensor.Sensor; import org.apache.brooklyn.api.sensor.SensorEventListener; import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.config.ConfigMap; +import org.apache.brooklyn.core.config.ConfigConstraints; import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.core.enricher.AbstractEnricher; import org.apache.brooklyn.core.entity.Entities; @@ -298,6 +299,7 @@ public abstract class AbstractEntityAdjunct extends AbstractBrooklynObject imple @SuppressWarnings("unchecked") @Override public <T> T set(ConfigKey<T> key, T val) { + ConfigConstraints.assertValid(entity, key, val); if (entity != null && isRunning()) { doReconfigureConfig(key, val); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/9ba5391d/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyConstraintTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyConstraintTest.java b/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyConstraintTest.java index 8a9e1c6..e3bfe2d 100644 --- a/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyConstraintTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyConstraintTest.java @@ -27,6 +27,7 @@ import java.util.concurrent.Callable; import org.apache.brooklyn.api.entity.EntitySpec; import org.apache.brooklyn.api.entity.ImplementedBy; +import org.apache.brooklyn.api.location.Location; import org.apache.brooklyn.api.mgmt.Task; import org.apache.brooklyn.api.objs.BrooklynObject; import org.apache.brooklyn.api.policy.Policy; @@ -35,9 +36,12 @@ import org.apache.brooklyn.api.sensor.EnricherSpec; import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.enricher.AbstractEnricher; import org.apache.brooklyn.core.entity.Entities; +import org.apache.brooklyn.core.entity.factory.ApplicationBuilder; import org.apache.brooklyn.core.objs.BrooklynObjectPredicate; import org.apache.brooklyn.core.policy.AbstractPolicy; import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport; +import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests; +import org.apache.brooklyn.core.test.entity.TestApplication; import org.apache.brooklyn.core.test.entity.TestEntity; import org.apache.brooklyn.core.test.entity.TestEntityImpl; import org.apache.brooklyn.core.test.policy.TestPolicy; @@ -46,6 +50,7 @@ import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.net.Networking; import org.apache.brooklyn.util.time.Duration; import org.apache.brooklyn.util.time.Time; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import com.google.common.base.Predicate; @@ -319,4 +324,26 @@ public class ConfigKeyConstraintTest extends BrooklynAppUnitTestSupport { .build(); } + // Supplies an entity, a policy and a location. + @DataProvider(name = "brooklynObjects") + public Object[][] createBrooklynObjects() { + TestApplication app = ApplicationBuilder.newManagedApp(EntitySpec.create(TestApplication.class), LocalManagementContextForTests.newInstance()); + EntityRequiringConfigKeyInRange entity = app.createAndManageChild(EntitySpec.create(EntityRequiringConfigKeyInRange.class) + .configure(EntityRequiringConfigKeyInRange.RANGE, 5)); + Policy policy = entity.policies().add(PolicySpec.create(TestPolicy.class)); + Location location = app.newSimulatedLocation(); + return new Object[][]{{entity}, {policy}, {location}}; + } + + @Test(dataProvider = "brooklynObjects") + public void testCannotUpdateConfigToInvalidValue(BrooklynObject object) { + try { + object.config().set(EntityRequiringConfigKeyInRange.RANGE, -1); + fail("Expected exception when calling config().set with invalid value on " + object); + } catch (Exception e) { + Throwable t = Exceptions.getFirstThrowableOfType(e, ConstraintViolationException.class); + assertNotNull(t, "Original exception was: " + Exceptions.collapseText(e)); + } + } + }
