brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sjcorb...@apache.org
Subject [01/17] incubator-brooklyn git commit: Validation of config constraints on policies and enrichers.
Date Wed, 14 Oct 2015 13:55:47 GMT
Repository: incubator-brooklyn
Updated Branches:
  refs/heads/master 206d78256 -> 2a004d939


Validation of config constraints on policies and enrichers.


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

Branch: refs/heads/master
Commit: 8ff866b0fad456f5c856f59738935e9f765d8959
Parents: 87b4b9b
Author: Sam Corbett <sam.corbett@cloudsoftcorp.com>
Authored: Fri Aug 28 11:37:33 2015 +0100
Committer: Sam Corbett <sam.corbett@cloudsoftcorp.com>
Committed: Thu Oct 8 11:10:31 2015 +0100

----------------------------------------------------------------------
 .../brooklyn/core/config/BasicConfigKey.java    |  1 +
 .../brooklyn/core/config/ConfigConstraints.java | 75 ++++++++++++++++----
 .../brooklyn/core/entity/AbstractEntity.java    |  2 -
 .../core/objs/AbstractEntityAdjunct.java        |  2 +-
 .../core/objs/proxy/InternalPolicyFactory.java  | 16 +++--
 .../core/config/ConfigKeyConstraintTest.java    | 67 ++++++++++++++---
 6 files changed, 130 insertions(+), 33 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/8ff866b0/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 239e7dd..9056eac 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
@@ -112,6 +112,7 @@ public class BasicConfigKey<T> implements ConfigKeySelfExtracting<T>,
Serializab
         public Builder<T> inheritance(ConfigInheritance val) {
             this.inheritance = val; return this;
         }
+        @Beta
         public Builder<T> constraint(Predicate<? super T> constraint) {
             this.constraint = checkNotNull(constraint, "constraint"); return this;
         }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/8ff866b0/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 0ac030f..ce6f39b 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
@@ -22,8 +22,11 @@ package org.apache.brooklyn.core.config;
 import java.util.List;
 
 import org.apache.brooklyn.api.entity.Entity;
+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.entity.EntityInternal;
+import org.apache.brooklyn.core.objs.AbstractEntityAdjunct;
+import org.apache.brooklyn.core.objs.BrooklynObjectInternal;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -31,25 +34,35 @@ import com.google.common.base.Predicate;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 
-public class ConfigConstraints {
+public abstract class ConfigConstraints<T extends BrooklynObject> {
 
     public static final Logger LOG = LoggerFactory.getLogger(ConfigConstraints.class);
-    private final Entity entity;
 
-    public ConfigConstraints(Entity e) {
-        this.entity = e;
-    }
+    private final T brooklynObject;
 
     /**
      * Checks all constraints of all config keys available to an entity.
      */
-    public static void assertValid(Entity e) {
-        Iterable<ConfigKey<?>> violations = new ConfigConstraints(e).getViolations();
+    public static void assertValid(Entity entity) {
+        Iterable<ConfigKey<?>> violations = new EntityConfigConstraints(entity).getViolations();
+        if (!Iterables.isEmpty(violations)) {
+            throw new ConstraintViolationException("ConfigKeys violate constraints: " + violations);
+        }
+    }
+
+    public static void assertValid(EntityAdjunct adjunct) {
+        Iterable<ConfigKey<?>> violations = new EntityAdjunctConstraints(adjunct).getViolations();
         if (!Iterables.isEmpty(violations)) {
-            throw new AssertionError("ConfigKeys violate constraints: " + violations);
+            throw new ConstraintViolationException("ConfigKeys violate constraints: " + violations);
         }
     }
 
+    public ConfigConstraints(T brooklynObject) {
+        this.brooklynObject = brooklynObject;
+    }
+
+    abstract Iterable<ConfigKey<?>> getBrooklynObjectTypeConfigKeys();
+
     public boolean isValid() {
         return Iterables.isEmpty(getViolations());
     }
@@ -60,12 +73,14 @@ public class ConfigConstraints {
 
     @SuppressWarnings("unchecked")
     private Iterable<ConfigKey<?>> validateAll() {
-        EntityInternal ei = (EntityInternal) entity;
         List<ConfigKey<?>> violating = Lists.newArrayList();
+        BrooklynObjectInternal.ConfigurationSupportInternal configInternal = getConfigurationSupportInternal();
 
-        for (ConfigKey<?> configKey : getEntityConfigKeys(entity)) {
+        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 = ei.config().getRaw(configKey).or(configKey.getDefaultValue());
+            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
@@ -83,8 +98,40 @@ public class ConfigConstraints {
         return violating;
     }
 
-    private static Iterable<ConfigKey<?>> getEntityConfigKeys(Entity entity)
{
-        return entity.getEntityType().getConfigKeys();
+    private BrooklynObjectInternal.ConfigurationSupportInternal getConfigurationSupportInternal()
{
+        return ((BrooklynObjectInternal) brooklynObject).config();
+    }
+
+    protected T getBrooklynObject() {
+        return brooklynObject;
+    }
+
+    private static class EntityConfigConstraints extends ConfigConstraints<Entity>
{
+        public EntityConfigConstraints(Entity brooklynObject) {
+            super(brooklynObject);
+        }
+
+        @Override
+        Iterable<ConfigKey<?>> getBrooklynObjectTypeConfigKeys() {
+            return getBrooklynObject().getEntityType().getConfigKeys();
+        }
+    }
+
+    private static class EntityAdjunctConstraints extends ConfigConstraints<EntityAdjunct>
{
+        public EntityAdjunctConstraints(EntityAdjunct brooklynObject) {
+            super(brooklynObject);
+        }
+
+        @Override
+        Iterable<ConfigKey<?>> getBrooklynObjectTypeConfigKeys() {
+            return ((AbstractEntityAdjunct) getBrooklynObject()).getAdjunctType().getConfigKeys();
+        }
+    }
+
+    public static class ConstraintViolationException extends RuntimeException {
+        public ConstraintViolationException(String message) {
+            super(message);
+        }
     }
 
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/8ff866b0/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 d91b28f..eb919a7 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
@@ -1551,8 +1551,6 @@ public abstract class AbstractEntity extends AbstractBrooklynObject
implements E
      */
     protected ToStringHelper toStringHelper() {
         return Objects.toStringHelper(this).omitNullValues().add("id", getId());
-//            make output more concise by suppressing display name
-//            .add("name", getDisplayName());
     }
     
     // -------- INITIALIZATION --------------

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/8ff866b0/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 8200ea8..3434471 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
@@ -426,7 +426,7 @@ public abstract class AbstractEntityAdjunct extends AbstractBrooklynObject
imple
     
     protected abstract void onChanged();
     
-    protected AdjunctType getAdjunctType() {
+    public AdjunctType getAdjunctType() {
         return adjunctType;
     }
     

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/8ff866b0/core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalPolicyFactory.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalPolicyFactory.java
b/core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalPolicyFactory.java
index aaee778..370d6fc 100644
--- a/core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalPolicyFactory.java
+++ b/core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalPolicyFactory.java
@@ -20,13 +20,17 @@ package org.apache.brooklyn.core.objs.proxy;
 
 import java.util.Map;
 
+import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
+import org.apache.brooklyn.api.objs.BrooklynObject;
+import org.apache.brooklyn.api.objs.EntityAdjunct;
 import org.apache.brooklyn.api.policy.Policy;
 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.api.sensor.Feed;
 import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.config.ConfigConstraints;
 import org.apache.brooklyn.core.enricher.AbstractEnricher;
 import org.apache.brooklyn.core.entity.AbstractEntity;
 import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
@@ -98,15 +102,15 @@ public class InternalPolicyFactory extends InternalFactory {
         if (spec.getFlags().containsKey("parent")) {
             throw new IllegalArgumentException("Spec's flags must not contain parent; use
spec.parent() instead for "+spec);
         }
-        
+
         try {
             Class<? extends T> clazz = spec.getType();
-            
+
             T pol = construct(clazz, spec.getFlags());
 
-            if (spec.getDisplayName()!=null)
+            if (spec.getDisplayName()!=null) {
                 ((AbstractPolicy)pol).setDisplayName(spec.getDisplayName());
-            
+            }
             if (spec.getCatalogItemId()!=null) {
                 ((AbstractPolicy)pol).setCatalogItemId(spec.getCatalogItemId());
             }
@@ -125,6 +129,7 @@ public class InternalPolicyFactory extends InternalFactory {
             for (Map.Entry<ConfigKey<?>, Object> entry : spec.getConfig().entrySet())
{
                 pol.config().set((ConfigKey)entry.getKey(), entry.getValue());
             }
+            ConfigConstraints.assertValid(pol);
             ((AbstractPolicy)pol).init();
             
             return pol;
@@ -166,6 +171,7 @@ public class InternalPolicyFactory extends InternalFactory {
             for (Map.Entry<ConfigKey<?>, Object> entry : spec.getConfig().entrySet())
{
                 enricher.config().set((ConfigKey)entry.getKey(), entry.getValue());
             }
+            ConfigConstraints.assertValid(enricher);
             ((AbstractEnricher)enricher).init();
             
             return enricher;
@@ -174,7 +180,7 @@ public class InternalPolicyFactory extends InternalFactory {
             throw Exceptions.propagate(e);
         }
     }
-    
+
     /**
      * Constructs a new-style policy (fails if no no-arg constructor).
      */

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/8ff866b0/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 130e3a7..6636944 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
@@ -24,17 +24,22 @@ import static org.testng.Assert.fail;
 
 import org.apache.brooklyn.api.entity.EntitySpec;
 import org.apache.brooklyn.api.entity.ImplementedBy;
+import org.apache.brooklyn.api.policy.Policy;
 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.policy.AbstractPolicy;
 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.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.Predicates;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Range;
 
 public class ConfigKeyConstraintTest extends BrooklynAppUnitTestSupport {
@@ -46,7 +51,6 @@ public class ConfigKeyConstraintTest extends BrooklynAppUnitTestSupport
{
                 .description("Configuration key that must not be null")
                 .constraint(Predicates.notNull())
                 .build();
-
     }
 
     @ImplementedBy(EntityWithNonNullConstraintWithNonNullDefaultImpl.class)
@@ -86,13 +90,30 @@ public class ConfigKeyConstraintTest extends BrooklynAppUnitTestSupport
{
     public static class EntityProvidingDefaultValueForConfigKeyInRangeImpl extends TestEntityImpl
implements EntityProvidingDefaultValueForConfigKeyInRange {
     }
 
+    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")
+                .constraint(Predicates.notNull())
+                .build();
+    }
+
+    public static class EnricherWithConfigConstraint extends AbstractEnricher {
+        public static final ConfigKey<String> PATTERN = ConfigKeys.builder(String.class)
+                .name("test.enricher.regex")
+                .description("Must match a valid IPv4 address")
+                .constraint(Predicates.containsPattern(Networking.VALID_IP_ADDRESS_REGEX))
+                .build();
+    }
+
+
     @Test
     public void testExceptionWhenEntityHasNullConfig() {
         try {
             app.createAndManageChild(EntitySpec.create(EntityWithNonNullConstraint.class));
             fail("Expected exception when managing entity with missing config");
         } catch (Exception e) {
-            Throwable t = Exceptions.getFirstThrowableOfType(e, AssertionError.class);
+            Throwable t = Exceptions.getFirstThrowableOfType(e, ConstraintViolationException.class);
             assertNotNull(t);
         }
     }
@@ -114,7 +135,7 @@ public class ConfigKeyConstraintTest extends BrooklynAppUnitTestSupport
{
             app.createAndManageChild(EntitySpec.create(EntityProvidingDefaultValueForConfigKeyInRange.class));
             fail("Expected exception when managing entity setting invalid default value");
         } catch (Exception e) {
-            Throwable t = Exceptions.getFirstThrowableOfType(e, AssertionError.class);
+            Throwable t = Exceptions.getFirstThrowableOfType(e, ConstraintViolationException.class);
             assertNotNull(t);
         }
     }
@@ -126,23 +147,47 @@ public class ConfigKeyConstraintTest extends BrooklynAppUnitTestSupport
{
                     .configure(EntityWithNonNullConstraintWithNonNullDefault.NON_NULL_WITH_DEFAULT,
(Object) null));
             fail("Expected exception when config key set to null");
         } catch (Exception e) {
-            Throwable t = Exceptions.getFirstThrowableOfType(e, AssertionError.class);
+            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() {
+        Policy p = mgmt.getEntityManager().createPolicy(PolicySpec.create(TestPolicy.class)
+                .configure(EntityWithNonNullConstraint.NON_NULL_CONFIG, (Object) null));
+        try {
+            ConfigConstraints.assertValid(p);
+            fail("Expected exception when validating policy with missing config");
+        } catch (Exception e) {
+            Throwable t = Exceptions.getFirstThrowableOfType(e, ConstraintViolationException.class);
+            assertNotNull(t);
+        }
+    }
+
+    @Test
     public void testExceptionWhenPolicyHasNullConfig() {
-        app.createAndManageChild(EntitySpec.create(TestEntity.class)
-                .policy(PolicySpec.create(TestPolicy.class)
-                        .configure(EntityWithNonNullConstraint.NON_NULL_CONFIG, (Object)
null)));
         try {
-            app.start(ImmutableList.of(app.newSimulatedLocation()));
-            fail("Expected exception when starting entity with policy with missing config");
+            mgmt.getEntityManager().createPolicy(PolicySpec.create(PolicyWithConfigConstraint.class)
+                    .configure(PolicyWithConfigConstraint.NON_NULL_CONFIG, (Object) null));
+            fail("Expected exception when creating policy with missing config");
         } catch (Exception e) {
-            Throwable t = Exceptions.getFirstThrowableOfType(e, AssertionError.class);
+            Throwable t = Exceptions.getFirstThrowableOfType(e, ConstraintViolationException.class);
             assertNotNull(t);
         }
     }
 
+    @Test
+    public void testExceptionWhenEnricherHasInvalidConfig() {
+        try {
+            mgmt.getEntityManager().createEnricher(EnricherSpec.create(EnricherWithConfigConstraint.class)
+                    .configure(EnricherWithConfigConstraint.PATTERN, "123.123.256.10"));
+            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));
+        }
+    }
+
 }


Mime
View raw message