Repository: brooklyn-server Updated Branches: refs/heads/master e601350d7 -> c782aae54
Fix DSL recursive-reference detection Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/5bc7546b Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/5bc7546b Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/5bc7546b Branch: refs/heads/master Commit: 5bc7546b9e2f5af743fad41533322c7804dd8e1b Parents: 6dafb3f Author: Aled Sage <aled.s...@gmail.com> Authored: Fri Sep 7 19:50:39 2018 +0100 Committer: Aled Sage <aled.s...@gmail.com> Committed: Fri Sep 7 19:50:39 2018 +0100 ---------------------------------------------------------------------- .../spi/dsl/BrooklynDslDeferredSupplier.java | 3 +- .../brooklyn/spi/dsl/methods/DslComponent.java | 12 ++-- .../camp/brooklyn/spi/dsl/DslYamlTest.java | 61 ++++++++++++++++++++ 3 files changed, 69 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/5bc7546b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/BrooklynDslDeferredSupplier.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/BrooklynDslDeferredSupplier.java b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/BrooklynDslDeferredSupplier.java index f7cf4cc..484aacb 100644 --- a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/BrooklynDslDeferredSupplier.java +++ b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/BrooklynDslDeferredSupplier.java @@ -131,8 +131,7 @@ public abstract class BrooklynDslDeferredSupplier<T> implements DeferredSupplier @Override public abstract Task<T> newTask(); - protected void checkAndTagForRecursiveReference(Entity targetEntity) { - String tag = toString(); + protected void checkAndTagForRecursiveReference(Entity targetEntity, String tag) { Task<?> ancestor = Tasks.current(); if (ancestor!=null) { ancestor = ancestor.getSubmittedByTask(); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/5bc7546b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslComponent.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslComponent.java b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslComponent.java index f542ff2..1040fa9 100644 --- a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslComponent.java +++ b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslComponent.java @@ -585,7 +585,8 @@ public class DslComponent extends BrooklynDslDeferredSupplier<Entity> implements } // this is always run in a new dedicated task (possibly a fake task if immediate), so no need to clear - checkAndTagForRecursiveReference(targetEntity); + String tag = "DSL:entity('"+targetEntity.getId()+"').config('"+keyName+"')"; + checkAndTagForRecursiveReference(targetEntity, tag); String keyNameS = resolveKeyName(true); ConfigKey<?> key = targetEntity.getEntityType().getConfigKey(keyNameS); @@ -777,11 +778,12 @@ public class DslComponent extends BrooklynDslDeferredSupplier<Entity> implements public Object call() throws Exception { Entity targetEntity = component.get(); - // this is always run in a new dedicated task (possibly a fake task if immediate), so no need to clear - checkAndTagForRecursiveReference(targetEntity); - int indexI = resolveIndex(immediate); + // this is always run in a new dedicated task (possibly a fake task if immediate), so no need to clear + String tag = "DSL:entity('"+targetEntity.getId()+"').location('"+indexI+"')"; + checkAndTagForRecursiveReference(targetEntity, tag); + // TODO Try repeatedly if no location(s)? Collection<Location> locations = getLocations(targetEntity); if (locations.size() < (indexI + 1)) { @@ -811,7 +813,7 @@ public class DslComponent extends BrooklynDslDeferredSupplier<Entity> implements .as(Integer.class) .context(findExecutionContext(this)) .immediately(immediately) - .description("Resolving indx from " + index) + .description("Resolving index from " + index) .get(); return result; } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/5bc7546b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslYamlTest.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslYamlTest.java index 7120150..57fee74 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslYamlTest.java @@ -38,8 +38,11 @@ import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.core.entity.EntityInternal; import org.apache.brooklyn.core.sensor.Sensors; import org.apache.brooklyn.core.test.entity.TestApplication; +import org.apache.brooklyn.core.test.entity.TestEntity; +import org.apache.brooklyn.entity.group.DynamicCluster; import org.apache.brooklyn.entity.stock.BasicApplication; import org.apache.brooklyn.entity.stock.BasicEntity; +import org.apache.brooklyn.entity.stock.BasicStartable; import org.apache.brooklyn.test.Asserts; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.exceptions.CompoundRuntimeException; @@ -857,6 +860,64 @@ public class DslYamlTest extends AbstractYamlTest { assertEquals(getConfigEventually(app, DEST), "hello world"); } + @Test + public void testDslRecursiveFails() throws Exception { + final Entity app = createAndStartApplication( + "services:", + "- type: " + BasicApplication.class.getName(), + " brooklyn.config:", + " dest: $brooklyn:config(\"dest\")"); + try { + getConfigEventually(app, DEST); + Asserts.shouldHaveFailedPreviously(); + } catch (Exception e) { + Asserts.expectedFailureContains(e, "Recursive reference DSL:entity('", "').config('dest')"); + } + } + + @Test + public void testDslRecursiveFails2() throws Exception { + final Entity app = createAndStartApplication( + "services:", + "- type: " + BasicApplication.class.getName(), + " brooklyn.config:", + " val: $brooklyn:config(\"dest\")", + " dest: $brooklyn:config(\"val\")"); + try { + getConfigEventually(app, DEST); + Asserts.shouldHaveFailedPreviously(); + } catch (Exception e) { + Asserts.expectedFailureContains(e, "Recursive reference DSL:entity('", "').config('val')"); + } + } + + @Test + public void testDslFromParentConfigNotRecursive() throws Exception { + // Broke this in https://github.com/apache/brooklyn-server/pull/971. + // It thought that the '$brooklyn:parent().config("test.value")' was the + // same each time, rather than 'parent' resolving differently each time. + + final Entity app = createAndStartApplication( + "services:", + "- type: " + BasicApplication.class.getName(), + " brooklyn.config:", + " test.value: 0", + " brooklyn.children:", + " - type: "+BasicStartable.class.getName(), + " brooklyn.config:", + " test.value: $brooklyn:parent().config(\"test.value\")", + " brooklyn.children:", + " - type: "+DynamicCluster.class.getName(), + " brooklyn.config:", + " cluster.initial.size: $brooklyn:parent().config(\"test.value\")", + " memberSpec:", + " $brooklyn:entitySpec:", + " type: "+TestEntity.class.getName()); + + BasicStartable child = (BasicStartable) Iterables.getOnlyElement(app.getChildren()); + DynamicCluster cluster = (DynamicCluster) Iterables.getOnlyElement(child.getChildren()); + assertEquals(cluster.config().get(DynamicCluster.INITIAL_SIZE), Integer.valueOf(0)); + } private static <T> T getConfigEventually(final Entity entity, final ConfigKey<T> configKey) throws Exception { // Use an executor, in case config().get() blocks forever, waiting for the config value.