You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by ri...@apache.org on 2018/09/11 23:00:58 UTC
brooklyn-server git commit: Fix DSL recursive-reference detection
Repository: brooklyn-server
Updated Branches:
refs/heads/1.0.0-M1 8f47a8e70 -> ab2f98b49
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/ab2f98b4
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/ab2f98b4
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/ab2f98b4
Branch: refs/heads/1.0.0-M1
Commit: ab2f98b4920ba96d5778233c189d298e61f80c23
Parents: 8f47a8e
Author: Aled Sage <al...@gmail.com>
Authored: Fri Sep 7 19:50:39 2018 +0100
Committer: Richard Downer <ri...@apache.org>
Committed: Tue Sep 11 12:12:21 2018 +0000
----------------------------------------------------------------------
.../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/ab2f98b4/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/ab2f98b4/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/ab2f98b4/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.