Adds ConfigurationSupportInternal.getNonBlocking ConfigConstraints uses the method to give a grace period to tasks when validating values.
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/aa5c00f6 Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/aa5c00f6 Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/aa5c00f6 Branch: refs/heads/master Commit: aa5c00f6c6b148fb192786f127d0bac19dc0e3d1 Parents: 2f9b9e1 Author: Sam Corbett <[email protected]> Authored: Thu Oct 8 17:49:47 2015 +0100 Committer: Sam Corbett <[email protected]> Committed: Thu Oct 8 17:54:43 2015 +0100 ---------------------------------------------------------------------- .../brooklyn/core/config/ConfigConstraints.java | 20 ++++--- .../brooklyn/core/entity/AbstractEntity.java | 4 ++ .../core/location/AbstractLocation.java | 6 +++ .../AbstractConfigurationSupportInternal.java | 37 +++++++++++++ .../core/objs/AbstractEntityAdjunct.java | 5 ++ .../core/objs/BrooklynObjectInternal.java | 28 +++++++--- .../core/config/ConfigKeyConstraintTest.java | 55 ++++++++++++++++---- 7 files changed, 132 insertions(+), 23 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/aa5c00f6/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 6c6445f..b1afe75 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 @@ -29,6 +29,7 @@ import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.objs.AbstractEntityAdjunct; import org.apache.brooklyn.core.objs.BrooklynObjectPredicate; import org.apache.brooklyn.core.objs.BrooklynObjectInternal; +import org.apache.brooklyn.util.guava.Maybe; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -36,6 +37,11 @@ import com.google.common.base.Predicate; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; +/** + * Checks configuration constraints on entities and their adjuncts. + * + * @since 0.9.0 + */ public abstract class ConfigConstraints<T extends BrooklynObject> { public static final Logger LOG = LoggerFactory.getLogger(ConfigConstraints.class); @@ -107,13 +113,13 @@ public abstract class ConfigConstraints<T extends BrooklynObject> { Iterable<ConfigKey<?>> configKeys = getBrooklynObjectTypeConfigKeys(); LOG.trace("Checking config keys on {}: {}", getBrooklynObject(), configKeys); for (ConfigKey<?> configKey : configKeys) { - // getRaw returns null if explicitly set and absent if config key was unset. - Object value = configInternal.getRaw(configKey).or(configKey.getDefaultValue()); - - if (value == null || value.getClass().isAssignableFrom(configKey.getType())) { - // Cast should be 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>. + // 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) { @@ -125,7 +131,7 @@ public abstract class ConfigConstraints<T extends BrooklynObject> { violating.add(configKey); } } catch (Exception e) { - LOG.debug("Error checking constraint on {} {} ", configKey.getName(), e); + LOG.debug("Error checking constraint on " + configKey.getName(), e); } } } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/aa5c00f6/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 1291587..a2e12f4 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 @@ -1214,7 +1214,11 @@ public abstract class AbstractEntity extends AbstractBrooklynObject implements E getManagementSupport().getEntityChangeListener().onConfigChanged(key); return result; + } + @Override + protected ExecutionContext getContext() { + return AbstractEntity.this.getExecutionContext(); } } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/aa5c00f6/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 1f6788c..0fd3f7b 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 @@ -33,6 +33,7 @@ import org.apache.brooklyn.api.entity.Entity; import org.apache.brooklyn.api.entity.Group; import org.apache.brooklyn.api.location.Location; import org.apache.brooklyn.api.location.LocationSpec; +import org.apache.brooklyn.api.mgmt.ExecutionContext; import org.apache.brooklyn.api.mgmt.SubscriptionContext; import org.apache.brooklyn.api.mgmt.SubscriptionHandle; import org.apache.brooklyn.api.mgmt.Task; @@ -473,6 +474,11 @@ public abstract class AbstractLocation extends AbstractBrooklynObject implements private ConfigInheritance getDefaultInheritance() { return ConfigInheritance.ALWAYS; } + + @Override + protected ExecutionContext getContext() { + return AbstractLocation.this.getManagementContext().getServerExecutionContext(); + } } public class BasicSubscriptionSupport implements SubscriptionSupportInternal { http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/aa5c00f6/core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java b/core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java index 4caa7e6..6cb8bb4 100644 --- a/core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java +++ b/core/src/main/java/org/apache/brooklyn/core/objs/AbstractConfigurationSupportInternal.java @@ -19,9 +19,17 @@ package org.apache.brooklyn.core.objs; +import java.util.concurrent.TimeUnit; +import javax.annotation.Nullable; + +import org.apache.brooklyn.api.mgmt.ExecutionContext; import org.apache.brooklyn.api.mgmt.Task; +import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.config.ConfigKey.HasConfigKey; +import org.apache.brooklyn.util.core.flags.TypeCoercions; +import org.apache.brooklyn.util.core.task.Tasks; import org.apache.brooklyn.util.guava.Maybe; +import org.apache.brooklyn.util.time.Duration; public abstract class AbstractConfigurationSupportInternal implements BrooklynObjectInternal.ConfigurationSupportInternal { @@ -41,6 +49,30 @@ public abstract class AbstractConfigurationSupportInternal implements BrooklynOb } @Override + public <T> Maybe<T> getNonBlocking(HasConfigKey<T> key) { + return getNonBlocking(key.getConfigKey()); + } + + @Override + public <T> Maybe<T> getNonBlocking(ConfigKey<T> key) { + // getRaw returns Maybe(val) if the key was explicitly set (where val can be null) + // or Absent if the config key was unset. + Object unresolved = getRaw(key).or(key.getDefaultValue()); + final Object marker = new Object(); + // Give tasks a short grace period to resolve. + Object resolved = Tasks.resolving(unresolved) + .as(Object.class) + .defaultValue(marker) + .timeout(Duration.of(5, TimeUnit.MILLISECONDS)) + .context(getContext()) + .swallowExceptions() + .get(); + return (resolved != marker) + ? TypeCoercions.tryCoerce(resolved, key.getTypeToken()) + : Maybe.<T>absent(); + } + + @Override public <T> T set(HasConfigKey<T> key, Task<T> val) { return set(key.getConfigKey(), val); } @@ -50,4 +82,9 @@ public abstract class AbstractConfigurationSupportInternal implements BrooklynOb return set(key.getConfigKey(), val); } + /** + * @return An execution context for use by {@link #getNonBlocking(ConfigKey)} + */ + @Nullable + protected abstract ExecutionContext getContext(); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/aa5c00f6/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 25c58ad..a7b00f4 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 @@ -357,6 +357,11 @@ public abstract class AbstractEntityAdjunct extends AbstractBrooklynObject imple public void refreshInheritedConfigOfChildren() { // no-op for location } + + @Override + protected ExecutionContext getContext() { + return AbstractEntityAdjunct.this.execution; + } } public <T> T getConfig(ConfigKey<T> key) { http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/aa5c00f6/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynObjectInternal.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynObjectInternal.java b/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynObjectInternal.java index 257da5a..6888c52 100644 --- a/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynObjectInternal.java +++ b/core/src/main/java/org/apache/brooklyn/core/objs/BrooklynObjectInternal.java @@ -46,11 +46,11 @@ public interface BrooklynObjectInternal extends BrooklynObject, Rebindable { @Beta public interface ConfigurationSupportInternal extends Configurable.ConfigurationSupport { - + /** - * Returns a read-only view of all the config key/value pairs on this entity, backed by a string-based map, + * Returns a read-only view of all the config key/value pairs on this entity, backed by a string-based map, * including config names that did not match anything on this entity. - * + * * TODO This method gives no information about which config is inherited versus local; * this means {@link ConfigKey#getInheritance()} cannot be respected. This is an unsolvable problem * for "config names that did not match anything on this entity". Therefore consider using @@ -58,14 +58,14 @@ public interface BrooklynObjectInternal extends BrooklynObject, Rebindable { */ @Beta ConfigBag getBag(); - + /** - * Returns a read-only view of the local (i.e. not inherited) config key/value pairs on this entity, + * Returns a read-only view of the local (i.e. not inherited) config key/value pairs on this entity, * backed by a string-based map, including config names that did not match anything on this entity. */ @Beta ConfigBag getLocalBag(); - + /** * Returns the uncoerced value for this config key, if available, not taking any default. * If there is no local value and there is an explicit inherited value, will return the inherited. @@ -92,6 +92,20 @@ public interface BrooklynObjectInternal extends BrooklynObject, Rebindable { @Beta Maybe<Object> getLocalRaw(HasConfigKey<?> key); + /** + * Attempts to coerce the value for this config key, if available, + * taking a default and {@link Maybe#absent absent} if the uncoerced + * cannot be resolved within a short timeframe. + */ + @Beta + <T> Maybe<T> getNonBlocking(ConfigKey<T> key); + + /** + * @see {@link #getNonBlocking(ConfigKey)} + */ + @Beta + <T> Maybe<T> getNonBlocking(HasConfigKey<T> key); + @Beta void addToLocalBag(Map<String, ?> vals); @@ -100,7 +114,7 @@ public interface BrooklynObjectInternal extends BrooklynObject, Rebindable { @Beta void refreshInheritedConfig(); - + @Beta void refreshInheritedConfigOfChildren(); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/aa5c00f6/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 05e0814..8a9e1c6 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 @@ -23,8 +23,11 @@ import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertNotNull; import static org.testng.Assert.fail; +import java.util.concurrent.Callable; + import org.apache.brooklyn.api.entity.EntitySpec; import org.apache.brooklyn.api.entity.ImplementedBy; +import org.apache.brooklyn.api.mgmt.Task; import org.apache.brooklyn.api.objs.BrooklynObject; import org.apache.brooklyn.api.policy.Policy; import org.apache.brooklyn.api.policy.PolicySpec; @@ -38,8 +41,11 @@ import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport; import org.apache.brooklyn.core.test.entity.TestEntity; import org.apache.brooklyn.core.test.entity.TestEntityImpl; import org.apache.brooklyn.core.test.policy.TestPolicy; +import org.apache.brooklyn.util.core.task.Tasks; 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.Test; import com.google.common.base.Predicate; @@ -164,7 +170,7 @@ public class ConfigKeyConstraintTest extends BrooklynAppUnitTestSupport { fail("Expected exception when managing entity setting invalid default value"); } catch (Exception e) { Throwable t = Exceptions.getFirstThrowableOfType(e, ConstraintViolationException.class); - assertNotNull(t); + assertNotNull(t, "Original exception was: " + Exceptions.collapseText(e)); } } @@ -176,7 +182,7 @@ public class ConfigKeyConstraintTest extends BrooklynAppUnitTestSupport { fail("Expected exception when config key set to null"); } catch (Exception e) { Throwable t = Exceptions.getFirstThrowableOfType(e, ConstraintViolationException.class); - assertNotNull(t); + assertNotNull(t, "Original exception was: " + Exceptions.collapseText(e)); } } @@ -188,7 +194,7 @@ public class ConfigKeyConstraintTest extends BrooklynAppUnitTestSupport { fail("Expected exception when managing entity with invalid config"); } catch (Exception e) { Throwable t = Exceptions.getFirstThrowableOfType(e, ConstraintViolationException.class); - assertNotNull(t); + assertNotNull(t, "Original exception was: " + Exceptions.collapseText(e)); } } @@ -196,14 +202,14 @@ public class ConfigKeyConstraintTest extends BrooklynAppUnitTestSupport { public void testExceptionWhenAppGrandchildHasInvalidConfig() { app.start(ImmutableList.of(app.newSimulatedLocation())); TestEntity testEntity = app.addChild(EntitySpec.create(TestEntity.class)); + try { testEntity.addChild(EntitySpec.create(EntityRequiringConfigKeyInRange.class) .configure(EntityRequiringConfigKeyInRange.RANGE, -1)); - try { Entities.manage(testEntity); fail("Expected exception when managing child with invalid config"); } catch (Exception e) { Throwable t = Exceptions.getFirstThrowableOfType(e, ConstraintViolationException.class); - assertNotNull(t); + assertNotNull(t, "Original exception was: " + Exceptions.collapseText(e)); } } @@ -217,7 +223,7 @@ public class ConfigKeyConstraintTest extends BrooklynAppUnitTestSupport { fail("Expected exception when validating policy with missing config"); } catch (Exception e) { Throwable t = Exceptions.getFirstThrowableOfType(e, ConstraintViolationException.class); - assertNotNull(t); + assertNotNull(t, "Original exception was: " + Exceptions.collapseText(e)); } } @@ -229,7 +235,7 @@ public class ConfigKeyConstraintTest extends BrooklynAppUnitTestSupport { fail("Expected exception when creating policy with missing config"); } catch (Exception e) { Throwable t = Exceptions.getFirstThrowableOfType(e, ConstraintViolationException.class); - assertNotNull(t); + assertNotNull(t, "Original exception was: " + Exceptions.collapseText(e)); } } @@ -241,7 +247,7 @@ public class ConfigKeyConstraintTest extends BrooklynAppUnitTestSupport { fail("Expected exception when creating enricher with invalid config"); } catch (Exception e) { Throwable t = Exceptions.getFirstThrowableOfType(e, ConstraintViolationException.class); - assertNotNull(t, "Exception was: " + Exceptions.collapseText(e)); + assertNotNull(t, "Original exception was: " + Exceptions.collapseText(e)); } } @@ -278,8 +284,39 @@ public class ConfigKeyConstraintTest extends BrooklynAppUnitTestSupport { fail("Expected exception when managing entity with incorrect config"); } catch (Exception e) { Throwable t = Exceptions.getFirstThrowableOfType(e, ConstraintViolationException.class); - assertNotNull(t); + assertNotNull(t, "Original exception was: " + Exceptions.collapseText(e)); } } + @Test + public void testQuickFutureResolved() { + // Result of task is -1, outside of the range specified by the config key. + try { + app.createAndManageChild(EntitySpec.create(EntityRequiringConfigKeyInRange.class) + .configure(EntityRequiringConfigKeyInRange.RANGE, sleepingTask(Duration.ZERO, -1))); + fail("Expected exception when managing entity with incorrect config"); + } catch (Exception e) { + Throwable t = Exceptions.getFirstThrowableOfType(e, ConstraintViolationException.class); + assertNotNull(t, "Original exception was: " + Exceptions.collapseText(e)); + } + } + + @Test + public void testSlowFutureNotResolved() { + // i.e. no exception because task is too slow to resolve. + app.createAndManageChild(EntitySpec.create(EntityRequiringConfigKeyInRange.class) + .configure(EntityRequiringConfigKeyInRange.RANGE, sleepingTask(Duration.PRACTICALLY_FOREVER, -1))); + } + + private static <T> Task<T> sleepingTask(final Duration delay, final T result) { + return Tasks.<T>builder() + .body(new Callable<T>() { + @Override public T call() throws Exception { + Time.sleep(delay); + return result; + } + }) + .build(); + } + }
