brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sjcorb...@apache.org
Subject [04/17] incubator-brooklyn git commit: Config constraint improvements
Date Wed, 14 Oct 2015 13:55:50 GMT
Config constraint improvements

* Entity descendants are checked at management
* Adds ConfigKey#isValueValid(T)
* Incorporates into ApplicationResource#create


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/7a728b48
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/7a728b48
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/7a728b48

Branch: refs/heads/master
Commit: 7a728b48ce327692320e345193393bbe6edcba2a
Parents: 8ff866b
Author: Sam Corbett <sam.corbett@cloudsoftcorp.com>
Authored: Fri Sep 11 13:47:26 2015 +0100
Committer: Sam Corbett <sam.corbett@cloudsoftcorp.com>
Committed: Thu Oct 8 17:51:27 2015 +0100

----------------------------------------------------------------------
 .../brooklyn/core/config/BasicConfigKey.java    | 24 +++++++-
 .../brooklyn/core/config/ConfigConstraints.java | 39 +++++++++---
 .../config/ConstraintViolationException.java    | 38 ++++++++++++
 .../mgmt/internal/EntityManagementSupport.java  |  5 +-
 .../core/mgmt/internal/LocalEntityManager.java  | 14 ++---
 .../core/objs/proxy/InternalEntityFactory.java  | 37 +++++++++---
 .../core/config/ConfigKeyConstraintTest.java    | 63 ++++++++++++++++++--
 .../rest/resources/ApplicationResource.java     |  8 ++-
 .../org/apache/brooklyn/config/ConfigKey.java   |  9 ++-
 9 files changed, 196 insertions(+), 41 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7a728b48/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java b/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java
index 9056eac..fe5e064 100644
--- a/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java
+++ b/core/src/main/java/org/apache/brooklyn/core/config/BasicConfigKey.java
@@ -88,7 +88,7 @@ public class BasicConfigKey<T> implements ConfigKeySelfExtracting<T>,
Serializab
         private String description;
         private T defaultValue;
         private boolean reconfigurable;
-        private Predicate<? super T> constraint;
+        private Predicate<? super T> constraint = Predicates.alwaysTrue();
         private ConfigInheritance inheritance;
         
         public Builder<T> name(String val) {
@@ -173,9 +173,12 @@ public class BasicConfigKey<T> implements ConfigKeySelfExtracting<T>,
Serializab
         this.defaultValue = builder.defaultValue;
         this.reconfigurable = builder.reconfigurable;
         this.inheritance = builder.inheritance;
-        this.constraint = builder.constraint;
+        // Note: it's intentionally possible to have default values that are not valid
+        // per the configured constraint. If validity were checked here any class that
+        // contained a weirdly-defined config key would fail to initialise.
+        this.constraint = checkNotNull(builder.constraint, "constraint");
     }
-    
+
     /** @see ConfigKey#getName() */
     @Override public String getName() { return name; }
 
@@ -199,21 +202,36 @@ public class BasicConfigKey<T> implements ConfigKeySelfExtracting<T>,
Serializab
         return defaultValue != null;
     }
 
+    /** @see ConfigKey#isReconfigurable() */
     @Override
     public boolean isReconfigurable() {
         return reconfigurable;
     }
     
+    /** @see ConfigKey#getInheritance() */
     @Override @Nullable
     public ConfigInheritance getInheritance() {
         return inheritance;
     }
 
+    /** @see ConfigKey#getConstraint() */
     @Override @Nonnull
     public Predicate<? super T> getConstraint() {
         return constraint;
     }
 
+    /** @see ConfigKey#isValueValid(T) */
+    @Override
+    public boolean isValueValid(T value) {
+        // The likeliest source of an exception is a constraint from Guava that expects a
non-null input.
+        try {
+            return getConstraint().apply(value);
+        } catch (Exception e) {
+            log.debug("Suppressing exception when testing validity of " + this, e);
+            return false;
+        }
+    }
+
     /** @see ConfigKey#getNameParts() */
     @Override public Collection<String> getNameParts() {
         return Lists.newArrayList(dots.split(name));

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7a728b48/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 ce6f39b..24142a5 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,6 +19,7 @@
 
 package org.apache.brooklyn.core.config;
 
+import java.util.Iterator;
 import java.util.List;
 
 import org.apache.brooklyn.api.entity.Entity;
@@ -42,21 +43,47 @@ public abstract class ConfigConstraints<T extends BrooklynObject>
{
 
     /**
      * Checks all constraints of all config keys available to an entity.
+     * <p>
+     * If a constraint is a {@link BrooklynObjectAwarePredicate} then it will be
+     * informed of the entity before the predicate is tested.
      */
     public static void assertValid(Entity entity) {
         Iterable<ConfigKey<?>> violations = new EntityConfigConstraints(entity).getViolations();
         if (!Iterables.isEmpty(violations)) {
-            throw new ConstraintViolationException("ConfigKeys violate constraints: " + violations);
+            throw new ConstraintViolationException(errorMessage(entity, violations));
         }
     }
 
+    /**
+     * Checks all constraints of all config keys available to an entity adjunct.
+     * <p>
+     * If a constraint is a {@link BrooklynObjectAwarePredicate} then it will be
+     * informed of the adjunct before the predicate is tested.
+     */
     public static void assertValid(EntityAdjunct adjunct) {
         Iterable<ConfigKey<?>> violations = new EntityAdjunctConstraints(adjunct).getViolations();
         if (!Iterables.isEmpty(violations)) {
-            throw new ConstraintViolationException("ConfigKeys violate constraints: " + violations);
+            throw new ConstraintViolationException(errorMessage(adjunct, violations));
         }
     }
 
+    private static String errorMessage(BrooklynObject object, Iterable<ConfigKey<?>>
violations) {
+        StringBuilder message = new StringBuilder("Error configuring ")
+                .append(object.getDisplayName())
+                .append(": [");
+        Iterator<ConfigKey<?>> it = violations.iterator();
+        while (it.hasNext()) {
+            ConfigKey<?> config = it.next();
+            message.append(config.getName())
+                    .append(":")
+                    .append(config.getConstraint());
+            if (it.hasNext()) {
+                message.append(", ");
+            }
+        }
+        return message.append("]").toString();
+    }
+
     public ConfigConstraints(T brooklynObject) {
         this.brooklynObject = brooklynObject;
     }
@@ -73,7 +100,7 @@ public abstract class ConfigConstraints<T extends BrooklynObject>
{
 
     @SuppressWarnings("unchecked")
     private Iterable<ConfigKey<?>> validateAll() {
-        List<ConfigKey<?>> violating = Lists.newArrayList();
+        List<ConfigKey<?>> violating = Lists.newLinkedList();
         BrooklynObjectInternal.ConfigurationSupportInternal configInternal = getConfigurationSupportInternal();
 
         Iterable<ConfigKey<?>> configKeys = getBrooklynObjectTypeConfigKeys();
@@ -128,10 +155,4 @@ public abstract class ConfigConstraints<T extends BrooklynObject>
{
         }
     }
 
-    public static class ConstraintViolationException extends RuntimeException {
-        public ConstraintViolationException(String message) {
-            super(message);
-        }
-    }
-
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7a728b48/core/src/main/java/org/apache/brooklyn/core/config/ConstraintViolationException.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/config/ConstraintViolationException.java
b/core/src/main/java/org/apache/brooklyn/core/config/ConstraintViolationException.java
new file mode 100644
index 0000000..55c7f07
--- /dev/null
+++ b/core/src/main/java/org/apache/brooklyn/core/config/ConstraintViolationException.java
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.brooklyn.core.config;
+
+/**
+ * A {@link ConstraintViolationException} indicates one or more problems applying
+ * values for {@link org.apache.brooklyn.config.ConfigKey ConfigKeys} when creating
+ * a {@link org.apache.brooklyn.api.objs.BrooklynObject}.
+ */
+public class ConstraintViolationException extends RuntimeException {
+    private static final long serialVersionUID = -6719912119648996815L;
+
+    public ConstraintViolationException(String message) {
+        super(message);
+    }
+
+    public ConstraintViolationException(String message, Throwable cause) {
+        super(message, cause);
+    }
+
+}

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7a728b48/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/EntityManagementSupport.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/EntityManagementSupport.java
b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/EntityManagementSupport.java
index c7d3d04..6b61bf6 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/EntityManagementSupport.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/EntityManagementSupport.java
@@ -65,7 +65,7 @@ import com.google.common.base.Stopwatch;
  * <p>
  * on unmanage it hits onManagementStoppingHere() then onManagementStopping().
  * <p>
- * When an entity's management migrates, it invoked onManagementStoppingHere() at the old
location,
+ * When an entity's management migrates, it invokes onManagementStoppingHere() at the old
location,
  * then onManagementStartingHere() at the new location.
  */
 public class EntityManagementSupport {
@@ -85,9 +85,6 @@ public class EntityManagementSupport {
     protected transient SubscriptionContext subscriptionContext;
     protected transient ExecutionContext executionContext;
     
-    // TODO the application
-    // (elaborate or remove ^^^ ? -AH, Sept 2014)
-    
     protected final AtomicBoolean managementContextUsable = new AtomicBoolean(false);
     protected final AtomicBoolean currentlyDeployed = new AtomicBoolean(false);
     protected final AtomicBoolean everDeployed = new AtomicBoolean(false);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7a728b48/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalEntityManager.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalEntityManager.java
b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalEntityManager.java
index f1fde20..6c9022d 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalEntityManager.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/LocalEntityManager.java
@@ -44,7 +44,6 @@ import org.apache.brooklyn.api.policy.PolicySpec;
 import org.apache.brooklyn.api.sensor.Enricher;
 import org.apache.brooklyn.api.sensor.EnricherSpec;
 import org.apache.brooklyn.core.BrooklynLogging;
-import org.apache.brooklyn.core.config.ConfigConstraints;
 import org.apache.brooklyn.core.entity.AbstractEntity;
 import org.apache.brooklyn.core.entity.Entities;
 import org.apache.brooklyn.core.entity.EntityInternal;
@@ -269,8 +268,6 @@ public class LocalEntityManager implements EntityManagerInternal {
                     new Exception("source of duplicate management of "+e));
             return;
         }
-        ConfigConstraints.assertValid(e);
-
         manageRecursive(e, ManagementTransitionMode.guessing(BrooklynObjectManagementMode.NONEXISTENT,
BrooklynObjectManagementMode.MANAGED_PRIMARY));
     }
 
@@ -311,7 +308,7 @@ public class LocalEntityManager implements EntityManagerInternal {
     protected void manageRecursive(Entity e, final ManagementTransitionMode initialMode)
{
         checkManagementAllowed(e);
 
-        final List<EntityInternal> allEntities =  Lists.newArrayList();
+        final List<EntityInternal> allEntities = Lists.newArrayList();
         Predicate<EntityInternal> manageEntity = new Predicate<EntityInternal>()
{ public boolean apply(EntityInternal it) {
             ManagementTransitionMode mode = getLastManagementTransitionMode(it.getId());
             if (mode==null) {
@@ -390,7 +387,7 @@ public class LocalEntityManager implements EntityManagerInternal {
             }
         }
     }
-    
+
     @Override
     public void unmanage(final Entity e) {
         unmanage(e, ManagementTransitionMode.guessing(BrooklynObjectManagementMode.MANAGED_PRIMARY,
BrooklynObjectManagementMode.NONEXISTENT));
@@ -600,7 +597,7 @@ public class LocalEntityManager implements EntityManagerInternal {
     /**
      * Should ensure that the entity is now known about, but should not be accessible from
other entities yet.
      * 
-     * Records that the given entity is about to be managed (used for answering {@link isPreManaged(Entity)}.
+     * Records that the given entity is about to be managed (used for answering {@link #isPreManaged(Entity)}.
      * Note that refs to the given entity are stored in a a weak hashmap so if the subsequent
management
      * attempt fails then this reference to the entity will eventually be discarded (if no-one
else holds 
      * a reference).
@@ -628,7 +625,6 @@ public class LocalEntityManager implements EntityManagerInternal {
     /**
      * Should ensure that the entity is now managed somewhere, and known about in all the
lists.
      * Returns true if the entity has now become managed; false if it was already managed
(anything else throws exception)
-     * @param isOrWasReadOnly 
      */
     private synchronized boolean manageNonRecursive(Entity e, ManagementTransitionMode mode)
{
         Entity old = entitiesById.get(e.getId());
@@ -641,8 +637,8 @@ public class LocalEntityManager implements EntityManagerInternal {
             }
             return false;
         }
-        
-        BrooklynLogging.log(log, BrooklynLogging.levelDebugOrTraceIfReadOnly(e), 
+
+        BrooklynLogging.log(log, BrooklynLogging.levelDebugOrTraceIfReadOnly(e),
             "{} starting management of entity {}", this, e);
         Entity realE = toRealEntity(e);
         

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7a728b48/core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java
b/core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java
index f258cdc..9552583 100644
--- a/core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java
+++ b/core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java
@@ -24,6 +24,7 @@ import static com.google.common.base.Preconditions.checkState;
 import java.lang.reflect.InvocationTargetException;
 import java.util.Collection;
 import java.util.Map;
+import java.util.Queue;
 import java.util.Set;
 
 import org.apache.brooklyn.api.entity.Entity;
@@ -37,6 +38,7 @@ import org.apache.brooklyn.api.policy.PolicySpec;
 import org.apache.brooklyn.api.sensor.Enricher;
 import org.apache.brooklyn.api.sensor.EnricherSpec;
 import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.config.ConfigConstraints;
 import org.apache.brooklyn.core.entity.AbstractApplication;
 import org.apache.brooklyn.core.entity.AbstractEntity;
 import org.apache.brooklyn.core.entity.Entities;
@@ -56,6 +58,7 @@ import org.slf4j.LoggerFactory;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
 
 /**
@@ -258,6 +261,20 @@ public class InternalEntityFactory extends InternalFactory {
             throw Exceptions.propagate(e);
         }
     }
+
+    /**
+     * Calls {@link ConfigConstraints#assertValid(Entity)} on the given entity and all of
+     * its descendants.
+     */
+    private void validateDescendantConfig(Entity e) {
+        Queue<Entity> queue = Lists.newLinkedList();
+        queue.add(e);
+        while (!queue.isEmpty()) {
+            Entity e1 = queue.poll();
+            ConfigConstraints.assertValid(e1);
+            queue.addAll(e1.getChildren());
+        }
+    }
     
     protected <T extends Entity> void initEntityAndDescendants(String entityId, final
Map<String,Entity> entitiesByEntityId, final Map<String,EntitySpec<?>> specsByEntityId)
{
         final Entity entity = entitiesByEntityId.get(entityId);
@@ -269,7 +286,11 @@ public class InternalEntityFactory extends InternalFactory {
                 + "and thus it should be already fully initialized.");
             return;
         }
-        
+
+        // Validate all config before attempting to manage any entity. Do this here rather
+        // than in manageRecursive so that rebind is unaffected.
+        validateDescendantConfig(entity);
+
         /* Marked transient so that the task is not needlessly kept around at the highest
level.
          * Note that the task is not normally visible in the GUI, because 
          * (a) while it is running, the entity is parentless (and so not in the tree);
@@ -286,36 +307,36 @@ public class InternalEntityFactory extends InternalFactory {
             @Override
             public void run() {
                 ((AbstractEntity)entity).init();
-                
+
                 ((AbstractEntity)entity).addLocations(spec.getLocations());
 
                 for (EntityInitializer initializer: spec.getInitializers()) {
                     initializer.apply((EntityInternal)entity);
                 }
-                
+
                 for (Enricher enricher : spec.getEnrichers()) {
                     entity.enrichers().add(enricher);
                 }
-                
+
                 for (EnricherSpec<?> enricherSpec : spec.getEnricherSpecs()) {
                     entity.enrichers().add(policyFactory.createEnricher(enricherSpec));
                 }
-                
+
                 for (Policy policy : spec.getPolicies()) {
                     entity.policies().add((AbstractPolicy)policy);
                 }
-                
+
                 for (PolicySpec<?> policySpec : spec.getPolicySpecs()) {
                     entity.policies().add(policyFactory.createPolicy(policySpec));
                 }
-                                
+
                 for (Entity child: entity.getChildren()) {
                     // right now descendants are initialized depth-first (see the getUnchecked()
call below)
                     // they could be done in parallel, but OTOH initializers should be very
quick
                     initEntityAndDescendants(child.getId(), entitiesByEntityId, specsByEntityId);
                 }
             }
-        }).build()).getUnchecked();        
+        }).build()).getUnchecked();
     }
     
     /**

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7a728b48/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 6636944..6cf42ab 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
@@ -19,6 +19,7 @@
 
 package org.apache.brooklyn.core.config;
 
+import static org.testng.Assert.assertFalse;
 import static org.testng.Assert.assertNotNull;
 import static org.testng.Assert.fail;
 
@@ -29,6 +30,7 @@ import org.apache.brooklyn.api.policy.PolicySpec;
 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.policy.AbstractPolicy;
 import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
 import org.apache.brooklyn.core.test.entity.TestEntity;
@@ -37,9 +39,11 @@ import org.apache.brooklyn.core.test.policy.TestPolicy;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.net.Networking;
 import org.testng.annotations.Test;
-import org.apache.brooklyn.core.config.ConfigConstraints.ConstraintViolationException;
 
+import com.google.common.base.Predicate;
 import com.google.common.base.Predicates;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Range;
 
 public class ConfigKeyConstraintTest extends BrooklynAppUnitTestSupport {
@@ -90,7 +94,7 @@ public class ConfigKeyConstraintTest extends BrooklynAppUnitTestSupport
{
     public static class EntityProvidingDefaultValueForConfigKeyInRangeImpl extends TestEntityImpl
implements EntityProvidingDefaultValueForConfigKeyInRange {
     }
 
-    public static class PolicyWithConfigConstraint extends AbstractPolicy{
+    public static class PolicyWithConfigConstraint extends AbstractPolicy {
         public static final ConfigKey<Object> NON_NULL_CONFIG = ConfigKeys.builder(Object.class)
                 .name("test.policy.non-null")
                 .description("Configuration key that must not be null")
@@ -106,7 +110,6 @@ public class ConfigKeyConstraintTest extends BrooklynAppUnitTestSupport
{
                 .build();
     }
 
-
     @Test
     public void testExceptionWhenEntityHasNullConfig() {
         try {
@@ -141,7 +144,7 @@ public class ConfigKeyConstraintTest extends BrooklynAppUnitTestSupport
{
     }
 
     @Test
-    public void testExceptionIsThrownWhenUserSetsNullValueToConfigWithNonNullDefault() {
+    public void testExceptionIsThrownWhenUserNullsConfigWithNonNullDefault() {
         try {
             app.createAndManageChild(EntitySpec.create(EntityWithNonNullConstraintWithNonNullDefault.class)
                     .configure(EntityWithNonNullConstraintWithNonNullDefault.NON_NULL_WITH_DEFAULT,
(Object) null));
@@ -152,6 +155,33 @@ public class ConfigKeyConstraintTest extends BrooklynAppUnitTestSupport
{
         }
     }
 
+    @Test
+    public void testExceptionWhenValueSetByName() {
+        try {
+            app.createAndManageChild(EntitySpec.create(EntityRequiringConfigKeyInRange.class)
+                    .configure(ImmutableMap.of("test.conf.range", -1)));
+            fail("Expected exception when managing entity with invalid config");
+        } catch (Exception e) {
+            Throwable t = Exceptions.getFirstThrowableOfType(e, ConstraintViolationException.class);
+            assertNotNull(t);
+        }
+    }
+
+    @Test
+    public void testExceptionWhenAppGrandchildHasInvalidConfig() {
+        app.start(ImmutableList.of(app.newSimulatedLocation()));
+        TestEntity testEntity = app.addChild(EntitySpec.create(TestEntity.class));
+        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);
+        }
+    }
+
     // Test fails because config keys that are not on an object's interfaces cannot be checked
automatically.
     @Test(enabled = false)
     public void testExceptionWhenPolicyHasNullForeignConfig() {
@@ -167,7 +197,7 @@ public class ConfigKeyConstraintTest extends BrooklynAppUnitTestSupport
{
     }
 
     @Test
-    public void testExceptionWhenPolicyHasNullConfig() {
+    public void testExceptionWhenPolicyHasInvalidConfig() {
         try {
             mgmt.getEntityManager().createPolicy(PolicySpec.create(PolicyWithConfigConstraint.class)
                     .configure(PolicyWithConfigConstraint.NON_NULL_CONFIG, (Object) null));
@@ -190,4 +220,27 @@ public class ConfigKeyConstraintTest extends BrooklynAppUnitTestSupport
{
         }
     }
 
+    @Test
+    public void testDefaultValueDoesNotNeedToObeyConstraint() {
+        ConfigKeys.builder(String.class)
+                .name("foo")
+                .defaultValue("a")
+                .constraint(Predicates.equalTo("b"))
+                .build();
+    }
+
+    @Test
+    public void testIsValidWithBadlyBehavedPredicate() {
+        ConfigKey<String> key = ConfigKeys.builder(String.class)
+                .name("foo")
+                .constraint(new Predicate<String>() {
+                    @Override
+                    public boolean apply(String input) {
+                        throw new RuntimeException("It's my day off");
+                    }
+                })
+                .build();
+        assertFalse(key.isValueValid("abc"));
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7a728b48/usage/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java
b/usage/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java
index 33752b2..68a9198 100644
--- a/usage/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java
+++ b/usage/rest-server/src/main/java/org/apache/brooklyn/rest/resources/ApplicationResource.java
@@ -45,6 +45,7 @@ import org.apache.brooklyn.api.mgmt.Task;
 import org.apache.brooklyn.api.sensor.AttributeSensor;
 import org.apache.brooklyn.api.sensor.Sensor;
 import org.apache.brooklyn.core.catalog.internal.CatalogUtils;
+import org.apache.brooklyn.core.config.ConstraintViolationException;
 import org.apache.brooklyn.core.entity.Attributes;
 import org.apache.brooklyn.core.entity.EntityPredicates;
 import org.apache.brooklyn.core.entity.lifecycle.Lifecycle;
@@ -71,6 +72,7 @@ import org.apache.brooklyn.rest.util.WebResourceUtils;
 import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.core.ResourceUtils;
 import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.exceptions.UserFacingException;
 import org.apache.brooklyn.util.text.Strings;
 import org.codehaus.jackson.JsonNode;
 import org.codehaus.jackson.node.ArrayNode;
@@ -286,7 +288,7 @@ public class ApplicationResource extends AbstractBrooklynRestResource
implements
         try {
             Application app = EntityManagementUtils.createUnstarted(mgmt(), spec);
             CreationResult<Application,Void> result = EntityManagementUtils.start(app);
-            
+
             boolean isEntitled = Entitlements.isEntitled(
                     mgmt().getEntitlementManager(),
                     Entitlements.INVOKE_EFFECTOR,
@@ -301,9 +303,11 @@ public class ApplicationResource extends AbstractBrooklynRestResource
implements
 
             URI ref = URI.create(app.getApplicationId());
             ResponseBuilder response = created(ref);
-            if (result.task() != null) 
+            if (result.task() != null)
                 response.entity(TaskTransformer.FROM_TASK.apply(result.task()));
             return response.build();
+        } catch (ConstraintViolationException e) {
+            throw new UserFacingException(e);
         } catch (Exception e) {
             throw Exceptions.propagate(e);
         }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7a728b48/utils/common/src/main/java/org/apache/brooklyn/config/ConfigKey.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/org/apache/brooklyn/config/ConfigKey.java b/utils/common/src/main/java/org/apache/brooklyn/config/ConfigKey.java
index 9dab87d..92f174f 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/config/ConfigKey.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/config/ConfigKey.java
@@ -89,12 +89,19 @@ public interface ConfigKey<T> {
     @Nullable ConfigInheritance getInheritance();
 
     /**
-     * @return the predicate constraining the key's value.
+     * @return The predicate constraining the key's value.
      */
     @Beta
     @Nonnull
     Predicate<? super T> getConstraint();
 
+    /**
+     * @param value The value to test
+     * @return True if the given value is acceptable per the {@link #getConstraint constraints}
on this key.
+     */
+    @Beta
+    boolean isValueValid(T value);
+
     /** Interface for elements which want to be treated as a config key without actually
being one
      * (e.g. config attribute sensors).
      */


Mime
View raw message