You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by sj...@apache.org on 2015/10/14 15:55:48 UTC

[02/17] incubator-brooklyn git commit: Constraints on ConfigKeys for entities

Constraints on ConfigKeys for entities

And their validation when an entity is first managed.


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

Branch: refs/heads/master
Commit: 87b4b9b21f3427ef36bc3fc31f5bff9261782e84
Parents: f0e0ba7
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Authored: Wed Aug 26 10:58:07 2015 +0100
Committer: Sam Corbett <sa...@cloudsoftcorp.com>
Committed: Thu Oct 8 11:10:31 2015 +0100

----------------------------------------------------------------------
 .../brooklyn/core/config/BasicConfigKey.java    |  20 ++-
 .../brooklyn/core/config/ConfigConstraints.java |  90 +++++++++++
 .../core/mgmt/internal/LocalEntityManager.java  |   3 +
 .../core/config/ConfigKeyConstraintTest.java    | 148 +++++++++++++++++++
 .../org/apache/brooklyn/config/ConfigKey.java   |  12 +-
 5 files changed, 270 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/87b4b9b2/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 069936c..239e7dd 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
@@ -25,6 +25,7 @@ import java.util.Collection;
 import java.util.Map;
 import java.util.concurrent.ExecutionException;
 
+import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 import org.apache.brooklyn.api.mgmt.ExecutionContext;
@@ -40,6 +41,8 @@ import org.slf4j.LoggerFactory;
 import com.google.common.annotations.Beta;
 import com.google.common.base.Objects;
 import com.google.common.base.Preconditions;
+import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
 import com.google.common.base.Splitter;
 import com.google.common.collect.Lists;
 import com.google.common.reflect.TypeToken;
@@ -75,7 +78,8 @@ public class BasicConfigKey<T> implements ConfigKeySelfExtracting<T>, Serializab
             .description(key.getDescription())
             .defaultValue(key.getDefaultValue())
             .reconfigurable(key.isReconfigurable())
-            .inheritance(key.getInheritance());
+            .inheritance(key.getInheritance())
+            .constraint(key.getConstraint());
     }
 
     public static class Builder<T> {
@@ -84,6 +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 ConfigInheritance inheritance;
         
         public Builder<T> name(String val) {
@@ -107,6 +112,9 @@ public class BasicConfigKey<T> implements ConfigKeySelfExtracting<T>, Serializab
         public Builder<T> inheritance(ConfigInheritance val) {
             this.inheritance = val; return this;
         }
+        public Builder<T> constraint(Predicate<? super T> constraint) {
+            this.constraint = checkNotNull(constraint, "constraint"); return this;
+        }
         public BasicConfigKey<T> build() {
             return new BasicConfigKey<T>(this);
         }
@@ -119,6 +127,7 @@ public class BasicConfigKey<T> implements ConfigKeySelfExtracting<T>, Serializab
     private T defaultValue;
     private boolean reconfigurable;
     private ConfigInheritance inheritance;
+    private Predicate<? super T> constraint;
 
     // FIXME In groovy, fields were `public final` with a default constructor; do we need the gson?
     public BasicConfigKey() { /* for gson */ }
@@ -152,6 +161,7 @@ public class BasicConfigKey<T> implements ConfigKeySelfExtracting<T>, Serializab
         
         this.defaultValue = defaultValue;
         this.reconfigurable = false;
+        this.constraint = Predicates.alwaysTrue();
     }
 
     protected BasicConfigKey(Builder<T> builder) {
@@ -162,6 +172,7 @@ public class BasicConfigKey<T> implements ConfigKeySelfExtracting<T>, Serializab
         this.defaultValue = builder.defaultValue;
         this.reconfigurable = builder.reconfigurable;
         this.inheritance = builder.inheritance;
+        this.constraint = builder.constraint;
     }
     
     /** @see ConfigKey#getName() */
@@ -196,7 +207,12 @@ public class BasicConfigKey<T> implements ConfigKeySelfExtracting<T>, Serializab
     public ConfigInheritance getInheritance() {
         return inheritance;
     }
-    
+
+    @Override @Nonnull
+    public Predicate<? super T> getConstraint() {
+        return constraint;
+    }
+
     /** @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/87b4b9b2/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
new file mode 100644
index 0000000..0ac030f
--- /dev/null
+++ b/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java
@@ -0,0 +1,90 @@
+/*
+ * 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;
+
+import java.util.List;
+
+import org.apache.brooklyn.api.entity.Entity;
+import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.entity.EntityInternal;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.base.Predicate;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+
+public class ConfigConstraints {
+
+    public static final Logger LOG = LoggerFactory.getLogger(ConfigConstraints.class);
+    private final Entity entity;
+
+    public ConfigConstraints(Entity e) {
+        this.entity = e;
+    }
+
+    /**
+     * Checks all constraints of all config keys available to an entity.
+     */
+    public static void assertValid(Entity e) {
+        Iterable<ConfigKey<?>> violations = new ConfigConstraints(e).getViolations();
+        if (!Iterables.isEmpty(violations)) {
+            throw new AssertionError("ConfigKeys violate constraints: " + violations);
+        }
+    }
+
+    public boolean isValid() {
+        return Iterables.isEmpty(getViolations());
+    }
+
+    public Iterable<ConfigKey<?>> getViolations() {
+        return validateAll();
+    }
+
+    @SuppressWarnings("unchecked")
+    private Iterable<ConfigKey<?>> validateAll() {
+        EntityInternal ei = (EntityInternal) entity;
+        List<ConfigKey<?>> violating = Lists.newArrayList();
+
+        for (ConfigKey<?> configKey : getEntityConfigKeys(entity)) {
+            // getRaw returns null if explicitly set and absent if config key was unset.
+            Object value = ei.config().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>.
+                try {
+                    Predicate<Object> po = (Predicate<Object>) configKey.getConstraint();
+                    if (!po.apply(value)) {
+                        violating.add(configKey);
+                    }
+                } catch (Exception e) {
+                    LOG.debug("Error checking constraint on {} {} ", configKey.getName(), e);
+                }
+            }
+        }
+        return violating;
+    }
+
+    private static Iterable<ConfigKey<?>> getEntityConfigKeys(Entity entity) {
+        return entity.getEntityType().getConfigKeys();
+    }
+
+}

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/87b4b9b2/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 8589eb1..f1fde20 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,6 +44,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.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;
@@ -268,6 +269,8 @@ 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));
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/87b4b9b2/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
new file mode 100644
index 0000000..130e3a7
--- /dev/null
+++ b/core/src/test/java/org/apache/brooklyn/core/config/ConfigKeyConstraintTest.java
@@ -0,0 +1,148 @@
+/*
+ * 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;
+
+import static org.testng.Assert.assertNotNull;
+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.PolicySpec;
+import org.apache.brooklyn.config.ConfigKey;
+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.testng.annotations.Test;
+
+import com.google.common.base.Predicates;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Range;
+
+public class ConfigKeyConstraintTest extends BrooklynAppUnitTestSupport {
+
+    @ImplementedBy(EntityWithNonNullConstraintImpl.class)
+    public static interface EntityWithNonNullConstraint extends TestEntity {
+        ConfigKey<Object> NON_NULL_CONFIG = ConfigKeys.builder(Object.class)
+                .name("test.conf.non-null.without-default")
+                .description("Configuration key that must not be null")
+                .constraint(Predicates.notNull())
+                .build();
+
+    }
+
+    @ImplementedBy(EntityWithNonNullConstraintWithNonNullDefaultImpl.class)
+    public static interface EntityWithNonNullConstraintWithNonNullDefault extends TestEntity {
+        ConfigKey<Object> NON_NULL_WITH_DEFAULT = ConfigKeys.builder(Object.class)
+                .name("test.conf.non-null.with-default")
+                .description("Configuration key that must not be null")
+                .defaultValue(new Object())
+                .constraint(Predicates.notNull())
+                .build();
+    }
+
+    @ImplementedBy(EntityRequiringConfigKeyInRangeImpl.class)
+    public static interface EntityRequiringConfigKeyInRange extends TestEntity {
+        ConfigKey<Integer> RANGE = ConfigKeys.builder(Integer.class)
+                .name("test.conf.range")
+                .description("Configuration key that must not be between zero and nine")
+                .defaultValue(0)
+                .constraint(Range.closed(0, 9))
+                .build();
+    }
+
+    @ImplementedBy(EntityProvidingDefaultValueForConfigKeyInRangeImpl.class)
+    public static interface EntityProvidingDefaultValueForConfigKeyInRange extends EntityRequiringConfigKeyInRange {
+        ConfigKey<Integer> REVISED_RANGE = ConfigKeys.newConfigKeyWithDefault(RANGE, -1);
+    }
+
+    public static class EntityWithNonNullConstraintImpl extends TestEntityImpl implements EntityWithNonNullConstraint {
+    }
+
+    public static class EntityWithNonNullConstraintWithNonNullDefaultImpl extends TestEntityImpl implements EntityWithNonNullConstraintWithNonNullDefault {
+    }
+
+    public static class EntityRequiringConfigKeyInRangeImpl extends TestEntityImpl implements EntityRequiringConfigKeyInRange {
+    }
+
+    public static class EntityProvidingDefaultValueForConfigKeyInRangeImpl extends TestEntityImpl implements EntityProvidingDefaultValueForConfigKeyInRange {
+    }
+
+    @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);
+            assertNotNull(t);
+        }
+    }
+
+    @Test
+    public void testNoExceptionWhenEntityHasValueForRequiredConfig() {
+        app.createAndManageChild(EntitySpec.create(EntityWithNonNullConstraint.class)
+                .configure(EntityWithNonNullConstraint.NON_NULL_CONFIG, new Object()));
+    }
+
+    @Test
+    public void testNoExceptionWhenDefaultValueIsValid() {
+        app.createAndManageChild(EntitySpec.create(EntityRequiringConfigKeyInRange.class));
+    }
+
+    @Test
+    public void testExceptionWhenSubclassSetsInvalidDefaultValue() {
+        try {
+            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);
+            assertNotNull(t);
+        }
+    }
+
+    @Test
+    public void testExceptionIsThrownWhenUserSetsNullValueToConfigWithNonNullDefault() {
+        try {
+            app.createAndManageChild(EntitySpec.create(EntityWithNonNullConstraintWithNonNullDefault.class)
+                    .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);
+            assertNotNull(t);
+        }
+    }
+
+    @Test(enabled = false)
+    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");
+        } catch (Exception e) {
+            Throwable t = Exceptions.getFirstThrowableOfType(e, AssertionError.class);
+            assertNotNull(t);
+        }
+    }
+
+}

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/87b4b9b2/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 3487bc4..9dab87d 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
@@ -20,8 +20,11 @@ package org.apache.brooklyn.config;
 
 import java.util.Collection;
 
+import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
+import com.google.common.annotations.Beta;
+import com.google.common.base.Predicate;
 import com.google.common.reflect.TypeToken;
 
 /**
@@ -79,12 +82,19 @@ public interface ConfigKey<T> {
      * @return True if the configuration can be changed at runtime.
      */
     boolean isReconfigurable();
-    
+
     /**
      * @return The inheritance model, or <code>null</code> for the default in any context.
      */
     @Nullable ConfigInheritance getInheritance();
 
+    /**
+     * @return the predicate constraining the key's value.
+     */
+    @Beta
+    @Nonnull
+    Predicate<? super T> getConstraint();
+
     /** Interface for elements which want to be treated as a config key without actually being one
      * (e.g. config attribute sensors).
      */