Repository: brooklyn-server Updated Branches: refs/heads/master b08c48298 -> 547feec96
BROOKLYN-482: yaml config key default vals immutable Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/15da8e50 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/15da8e50 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/15da8e50 Branch: refs/heads/master Commit: 15da8e50aa4cb8618da3948cbefa614c50877bc5 Parents: eb4992e Author: Aled Sage <[email protected]> Authored: Thu Apr 20 16:34:50 2017 +0100 Committer: Aled Sage <[email protected]> Committed: Thu Apr 20 16:43:43 2017 +0100 ---------------------------------------------------------------------- .../camp/brooklyn/ConfigParametersYamlTest.java | 67 ++++++++++++++++++++ .../brooklyn/core/objs/BasicSpecParameter.java | 26 +++++++- 2 files changed, 92 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/15da8e50/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java index f3ce7f7..53193aa 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigParametersYamlTest.java @@ -23,10 +23,12 @@ import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertNotNull; import static org.testng.Assert.assertTrue; +import java.util.Collection; import java.util.Collections; import java.util.Date; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.Callable; import org.apache.brooklyn.api.entity.Entity; @@ -67,7 +69,9 @@ import org.testng.annotations.Test; import com.google.common.base.Joiner; 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.ImmutableSet; import com.google.common.collect.Iterables; public class ConfigParametersYamlTest extends AbstractYamlRebindTest { @@ -528,6 +532,69 @@ public class ConfigParametersYamlTest extends AbstractYamlRebindTest { assertEquals(env.get("TEST"), "myDefaultParamVal", "env="+env); } + @Test + public void testDefaultValsImmutable() throws Exception { + addCatalogItems( + "brooklyn.catalog:", + " itemType: entity", + " items:", + " - id: entity-with-keys", + " item:", + " type: "+TestEntity.class.getName(), + " brooklyn.parameters:", + " - name: my.list.key", + " type: java.util.List", + " default: [\"myDefaultVal\"]", + " - name: my.set.key", + " type: "+java.util.Set.class.getName(), + " default: [\"myDefaultVal\"]", + " - name: my.collection.key", + " type: "+java.util.Collection.class.getName(), + " default: [\"myDefaultVal\"]", + " - name: my.map.key", + " type: "+java.util.Map.class.getName(), + " default: {\"myDefaultKey\":\"myDefaultVal\"}"); + + String yaml = Joiner.on("\n").join( + "services:", + "- type: entity-with-keys"); + Entity app = createStartWaitAndLogApplication(yaml); + TestEntity entity = (TestEntity) Iterables.getOnlyElement(app.getChildren()); + List<?> list = (List<?>) entity.config().get(entity.getEntityType().getConfigKey("my.list.key")); + Set<?> set = (Set<?>) entity.config().get(entity.getEntityType().getConfigKey("my.set.key")); + Collection<?> collection = (Collection<?>) entity.config().get(entity.getEntityType().getConfigKey("my.set.key")); + Map<?, ?> map = (Map<?, ?>) entity.config().get(entity.getEntityType().getConfigKey("my.map.key")); + + assertEquals(list, ImmutableList.of("myDefaultVal")); + assertEquals(set, ImmutableSet.of("myDefaultVal")); + assertEquals(collection, ImmutableList.of("myDefaultVal")); + assertEquals(map, ImmutableMap.of("myDefaultKey", "myDefaultVal")); + assertImmutable(list); + assertImmutable(set); + assertImmutable(collection); + assertImmutable(map); + } + + @SuppressWarnings("unchecked") + private void assertImmutable(Collection<?> val) { + try { + ((Collection<Object>)val).add("myNewVal"); + Asserts.shouldHaveFailedPreviously("Collection of type " + val.getClass().getName() + " was mutable"); + } catch (UnsupportedOperationException e) { + // expected - success + } + } + + @SuppressWarnings("unchecked") + private void assertImmutable(Map<?,?> val) { + try { + ((Map<Object, Object>)val).put("myNewKey", "myNewVal"); + Asserts.shouldHaveFailedPreviously("Map of type " + val.getClass().getName() + " was mutable"); + } catch (UnsupportedOperationException e) { + // expected - success + } + } + // See https://issues.apache.org/jira/browse/BROOKLYN-328 @Test public void testConfigParameterOverridingJavaConfig() throws Exception { http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/15da8e50/core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java b/core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java index 0dbc450..e6194fe 100644 --- a/core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java +++ b/core/src/main/java/org/apache/brooklyn/core/objs/BasicSpecParameter.java @@ -28,6 +28,7 @@ import java.util.Comparator; import java.util.Date; import java.util.List; import java.util.Map; +import java.util.Set; import org.apache.brooklyn.api.catalog.CatalogConfig; import org.apache.brooklyn.api.entity.Entity; @@ -52,6 +53,7 @@ import org.apache.brooklyn.core.config.ConfigKeys.InheritanceContext; import org.apache.brooklyn.core.sensor.PortAttributeSensorAndConfigKey; import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.collections.MutableMap; +import org.apache.brooklyn.util.collections.MutableSet; import org.apache.brooklyn.util.guava.Maybe; import org.apache.brooklyn.util.text.StringPredicates; import org.apache.brooklyn.util.time.Duration; @@ -290,10 +292,12 @@ public class BasicSpecParameter<T> implements SpecParameter<T>{ boolean hasType = type!=null; TypeToken typeToken = inferType(type, loader); + Object immutableDefaultValue = tryToImmutable(defaultValue, typeToken); + Builder builder = BasicConfigKey.builder(typeToken) .name(name) .description(description) - .defaultValue(defaultValue) + .defaultValue(immutableDefaultValue) .constraint(constraint) .runtimeInheritance(runtimeInheritance) .typeInheritance(typeInheritance); @@ -309,6 +313,26 @@ public class BasicSpecParameter<T> implements SpecParameter<T>{ hasType, hasDefaultValue, hasConstraints, hasRuntimeInheritance, hasTypeInheritance); } + private static Object tryToImmutable(Object val, TypeToken<?> type) { + Object result; + if (Set.class.isAssignableFrom(type.getRawType()) && val instanceof Iterable) { + result = Collections.unmodifiableSet(MutableSet.copyOf((Iterable<?>)val)); + } else if (val instanceof Iterable) { + result = Collections.unmodifiableList(MutableList.copyOf((Iterable<?>)val)); + } else if (val instanceof Map) { + result = Collections.unmodifiableMap(MutableMap.copyOf((Map<?, ?>)val)); + } else { + return val; + } + + if (type != null && !type.isAssignableFrom(result.getClass())) { + log.warn("Unable to convert parameter default value (type "+type+") to immutable"); + return val; + } else { + return result; + } + } + @SuppressWarnings({ "rawtypes" }) private static TypeToken inferType(String typeRaw, BrooklynClassLoadingContext loader) { if (typeRaw == null) return TypeToken.of(String.class);
