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 <[email protected]> Authored: Wed Aug 26 10:58:07 2015 +0100 Committer: Sam Corbett <[email protected]> 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). */
