You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by al...@apache.org on 2016/11/10 14:02:03 UTC

[1/4] brooklyn-server git commit: Fix resolving a sensor of an entity in the context of another entity.

Repository: brooklyn-server
Updated Branches:
  refs/heads/master f25fff728 -> 6a4ca6486


Fix resolving a sensor of an entity in the context of another entity.

The ExecutionContext will tag the task with the entity it belongs to. As it is tagged by the execution context no need to tag it explicitly.
The context of the task should be the entity we are resolving the sensor against, not the context of the current task.


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/6be89c16
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/6be89c16
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/6be89c16

Branch: refs/heads/master
Commit: 6be89c168829326dcaea02fccea086366b43823b
Parents: 3dbe9cc
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Authored: Fri Oct 14 16:53:04 2016 +0300
Committer: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Committed: Wed Nov 9 17:57:07 2016 +0200

----------------------------------------------------------------------
 .../brooklyn/camp/brooklyn/spi/dsl/methods/DslComponent.java      | 3 ---
 .../test/java/org/apache/brooklyn/camp/brooklyn/dsl/DslTest.java  | 2 +-
 .../java/org/apache/brooklyn/util/core/task/ValueResolver.java    | 3 +--
 3 files changed, 2 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/6be89c16/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 5e42fbe..ddbeb72 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
@@ -101,7 +101,6 @@ public class DslComponent extends BrooklynDslDeferredSupplier<Entity> {
         return TaskBuilder.<Entity>builder()
                 .displayName(toString())
                 .tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
-                .tagIfNotNull(BrooklynTaskTags.getTargetOrContextEntityTag(Tasks.current()))
                 .body(new EntityInScopeFinder(scopeComponent, scope, componentId))
                 .build();
     }
@@ -373,7 +372,6 @@ public class DslComponent extends BrooklynDslDeferredSupplier<Entity> {
             return Tasks.builder()
                     .displayName("retrieving config for "+keyName)
                     .tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
-                    .tagIfNotNull(BrooklynTaskTags.getTargetOrContextEntityTag(Tasks.current()))
                     .dynamic(false)
                     .body(new Callable<Object>() {
                         @Override
@@ -454,7 +452,6 @@ public class DslComponent extends BrooklynDslDeferredSupplier<Entity> {
             return Tasks.<Sensor<?>>builder()
                     .displayName("looking up sensor for "+sensorName)
                     .dynamic(false)
-                    .tagIfNotNull(BrooklynTaskTags.getTargetOrContextEntityTag(Tasks.current()))
                     .body(new Callable<Sensor<?>>() {
                         @Override
                         public Sensor<?> call() throws Exception {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/6be89c16/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/dsl/DslTest.java
----------------------------------------------------------------------
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/dsl/DslTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/dsl/DslTest.java
index aa6ec0e..3f31805 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/dsl/DslTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/dsl/DslTest.java
@@ -272,7 +272,7 @@ public class DslTest extends BrooklynAppUnitTestSupport {
     }
 
     // Different from testParentConcurrent() only in the execution context the task is submitted in (global vs app)
-    @Test(invocationCount=10, groups="Broken") //fails ~4 times
+    @Test(invocationCount=10)
     public void testResolveInDifferentContext() throws InterruptedException, ExecutionException {
         final TestEntity entity = app.createAndManageChild(EntitySpec.create(TestEntity.class));
         Task<Maybe<Entity>> result = app.getExecutionContext().submit(new Callable<Maybe<Entity>>() {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/6be89c16/core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java b/core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java
index 4c3cbe2..9ae6893 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java
@@ -375,8 +375,7 @@ public class ValueResolver<T> implements DeferredSupplier<T> {
                     TaskBuilder<Object> tb = Tasks.<Object>builder()
                             .body(callable)
                             .displayName("Resolving dependent value")
-                            .description(description)
-                            .tagIfNotNull(BrooklynTaskTags.getTargetOrContextEntityTag(Tasks.current()));
+                            .description(description);
                     if (isTransientTask) tb.tag(BrooklynTaskTags.TRANSIENT_TASK_TAG);
                     
                     // Note that immediate resolution is handled by using ImmediateSupplier (using an instanceof check), 


[2/4] brooklyn-server git commit: Test executing a task in a context different from its own

Posted by al...@apache.org.
Test executing a task in a context different from its own

When executing a task which is already tagged with a context entity in the execution context of another task, the task ends up with two context entities in its tags. It's indeterministic which one will be available through BrooklynTaskTags.getContextEntity. Add tests to cover the expected behaviour (which are failing atm) - the task should always have a single context entity, the one explicitly applied on the task before submitting.


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/3dbe9cc2
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/3dbe9cc2
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/3dbe9cc2

Branch: refs/heads/master
Commit: 3dbe9cc2b347ea59bdbe5d7c749e83204fbbcaeb
Parents: fd00978
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Authored: Fri Oct 14 15:43:41 2016 +0300
Committer: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Committed: Wed Nov 9 17:57:07 2016 +0200

----------------------------------------------------------------------
 .../brooklyn/camp/brooklyn/dsl/DslTest.java     | 36 ++++++++++++++++++++
 .../brooklyn/util/core/task/TasksTest.java      | 31 +++++++++++++++++
 2 files changed, 67 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3dbe9cc2/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/dsl/DslTest.java
----------------------------------------------------------------------
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/dsl/DslTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/dsl/DslTest.java
index 1d9c4ae..aa6ec0e 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/dsl/DslTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/dsl/DslTest.java
@@ -24,6 +24,7 @@ import java.util.Collection;
 import java.util.NoSuchElementException;
 import java.util.Random;
 import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
 
@@ -38,6 +39,7 @@ import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.config.ConfigKeys;
 import org.apache.brooklyn.core.entity.EntityInternal;
 import org.apache.brooklyn.core.objs.BasicSpecParameter;
+import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
 import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
 import org.apache.brooklyn.core.test.entity.TestApplication;
 import org.apache.brooklyn.core.test.entity.TestEntity;
@@ -269,6 +271,40 @@ public class DslTest extends BrooklynAppUnitTestSupport {
         }
     }
 
+    // Different from testParentConcurrent() only in the execution context the task is submitted in (global vs app)
+    @Test(invocationCount=10, groups="Broken") //fails ~4 times
+    public void testResolveInDifferentContext() throws InterruptedException, ExecutionException {
+        final TestEntity entity = app.createAndManageChild(EntitySpec.create(TestEntity.class));
+        Task<Maybe<Entity>> result = app.getExecutionContext().submit(new Callable<Maybe<Entity>>() {
+            @Override
+            public Maybe<Entity> call() throws Exception {
+                BrooklynDslDeferredSupplier<?> dsl = BrooklynDslCommon.self();
+                return Tasks.resolving(dsl).as(Entity.class)
+                        .context(entity)
+                        .timeout(ValueResolver.NON_BLOCKING_WAIT)
+                        .getMaybe();
+            }
+        });
+        assertEquals(result.get().get(), entity);
+    }
+
+    @Test(invocationCount=10, groups="Broken") //fails ~4 times
+    public void testTaskContext() {
+        final TestEntity entity = app.createAndManageChild(EntitySpec.create(TestEntity.class));
+        // context entity here = none
+        Task<Entity> task = Tasks.<Entity>builder()
+            .tag(BrooklynTaskTags.tagForContextEntity(entity))
+            .body(new Callable<Entity>() {
+                @Override
+                public Entity call() throws Exception {
+                    // context entity here = entity
+                    return BrooklynTaskTags.getContextEntity(Tasks.current());
+                }
+            }).build();
+        Task<Entity> result = app.getExecutionContext().submit(task);
+        assertEquals(result.getUnchecked(), entity);
+    }
+
     protected void runConcurrentWorker(Supplier<Runnable> taskSupplier) {
         Collection<Task<?>> results = new ArrayList<>();
         for (int i = 0; i < MAX_PARALLEL_RESOLVERS; i++) {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3dbe9cc2/core/src/test/java/org/apache/brooklyn/util/core/task/TasksTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/util/core/task/TasksTest.java b/core/src/test/java/org/apache/brooklyn/util/core/task/TasksTest.java
index e1952c2..cc37443 100644
--- a/core/src/test/java/org/apache/brooklyn/util/core/task/TasksTest.java
+++ b/core/src/test/java/org/apache/brooklyn/util/core/task/TasksTest.java
@@ -22,13 +22,19 @@ import static org.apache.brooklyn.core.sensor.DependentConfiguration.attributeWh
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertTrue;
 
+import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.Callable;
 
+import org.apache.brooklyn.api.entity.Entity;
+import org.apache.brooklyn.api.entity.EntitySpec;
 import org.apache.brooklyn.api.mgmt.ExecutionContext;
 import org.apache.brooklyn.api.mgmt.Task;
 import org.apache.brooklyn.core.entity.EntityFunctions;
+import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
+import org.apache.brooklyn.core.mgmt.BrooklynTaskTags.WrappedEntity;
 import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
 import org.apache.brooklyn.core.test.entity.TestApplication;
 import org.apache.brooklyn.core.test.entity.TestEntity;
@@ -180,4 +186,29 @@ public class TasksTest extends BrooklynAppUnitTestSupport {
         assertTrue(t.get(Duration.TEN_SECONDS));
     }
 
+    @Test(groups="Broken")
+    public void testSingleExecutionContextEntity() {
+        final TestEntity entity = app.createAndManageChild(EntitySpec.create(TestEntity.class));
+        // context entity here = none
+        Task<Collection<Entity>> task = Tasks.<Collection<Entity>>builder()
+            .tag(BrooklynTaskTags.tagForContextEntity(entity))
+            .body(new Callable<Collection<Entity>>() {
+                @Override
+                public Collection<Entity> call() throws Exception {
+                    Collection<Entity> context = new ArrayList<>();
+                    for (Object tag : Tasks.current().getTags()) {
+                        if (tag instanceof WrappedEntity) {
+                            WrappedEntity wrapped = (WrappedEntity)tag;
+                            if (BrooklynTaskTags.CONTEXT_ENTITY.equals(wrapped.wrappingType)) {
+                                context.add(wrapped.entity);
+                            }
+                        }
+                    }
+                    return context;
+                }
+            }).build();
+        Task<Collection<Entity>> result = app.getExecutionContext().submit(task);
+        assertEquals(result.getUnchecked(), ImmutableList.of(entity));
+    }
+
 }


[3/4] brooklyn-server git commit: Don't allow multiple context entities in a task's tags.

Posted by al...@apache.org.
Don't allow multiple context entities in a task's tags.

The context entity should be set by internal code at a single place. Remove all places where we are setting the context additionally and add a warning at runtime if detected. To be changed to an exception after a release or two.


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/0019473f
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/0019473f
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/0019473f

Branch: refs/heads/master
Commit: 0019473fb2cdd5dffee1115873afe59357b20e88
Parents: 6be89c1
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Authored: Fri Oct 14 20:17:39 2016 +0300
Committer: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Committed: Wed Nov 9 17:57:57 2016 +0200

----------------------------------------------------------------------
 .../brooklyn/camp/brooklyn/dsl/DslTest.java     | 21 +----
 .../location/internal/LocationConfigMap.java    | 21 ++++-
 .../brooklyn/core/mgmt/BrooklynTaskTags.java    | 12 +++
 .../core/objs/proxy/InternalEntityFactory.java  |  1 -
 .../util/core/config/ResolvingConfigBag.java    | 22 ++++-
 .../util/core/task/BasicExecutionContext.java   | 57 ++++++++++--
 .../brooklyn/util/core/task/TasksTest.java      | 91 +++++++++++++++-----
 .../system_service/SystemServiceEnricher.java   |  1 -
 8 files changed, 170 insertions(+), 56 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/0019473f/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/dsl/DslTest.java
----------------------------------------------------------------------
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/dsl/DslTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/dsl/DslTest.java
index 3f31805..ae55379 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/dsl/DslTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/dsl/DslTest.java
@@ -273,27 +273,10 @@ public class DslTest extends BrooklynAppUnitTestSupport {
 
     // Different from testParentConcurrent() only in the execution context the task is submitted in (global vs app)
     @Test(invocationCount=10)
-    public void testResolveInDifferentContext() throws InterruptedException, ExecutionException {
-        final TestEntity entity = app.createAndManageChild(EntitySpec.create(TestEntity.class));
-        Task<Maybe<Entity>> result = app.getExecutionContext().submit(new Callable<Maybe<Entity>>() {
-            @Override
-            public Maybe<Entity> call() throws Exception {
-                BrooklynDslDeferredSupplier<?> dsl = BrooklynDslCommon.self();
-                return Tasks.resolving(dsl).as(Entity.class)
-                        .context(entity)
-                        .timeout(ValueResolver.NON_BLOCKING_WAIT)
-                        .getMaybe();
-            }
-        });
-        assertEquals(result.get().get(), entity);
-    }
-
-    @Test(invocationCount=10, groups="Broken") //fails ~4 times
     public void testTaskContext() {
         final TestEntity entity = app.createAndManageChild(EntitySpec.create(TestEntity.class));
         // context entity here = none
         Task<Entity> task = Tasks.<Entity>builder()
-            .tag(BrooklynTaskTags.tagForContextEntity(entity))
             .body(new Callable<Entity>() {
                 @Override
                 public Entity call() throws Exception {
@@ -301,14 +284,14 @@ public class DslTest extends BrooklynAppUnitTestSupport {
                     return BrooklynTaskTags.getContextEntity(Tasks.current());
                 }
             }).build();
-        Task<Entity> result = app.getExecutionContext().submit(task);
+        Task<Entity> result = entity.getExecutionContext().submit(task);
         assertEquals(result.getUnchecked(), entity);
     }
 
     protected void runConcurrentWorker(Supplier<Runnable> taskSupplier) {
         Collection<Task<?>> results = new ArrayList<>();
         for (int i = 0; i < MAX_PARALLEL_RESOLVERS; i++) {
-            Task<?> result = mgmt.getExecutionManager().submit(taskSupplier.get());
+            Task<?> result = app.getExecutionContext().submit(taskSupplier.get());
             results.add(result);
         }
         for (Task<?> result : results) {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/0019473f/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationConfigMap.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationConfigMap.java b/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationConfigMap.java
index 4c82188..b975e1d 100644
--- a/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationConfigMap.java
+++ b/core/src/main/java/org/apache/brooklyn/core/location/internal/LocationConfigMap.java
@@ -24,13 +24,18 @@ import java.util.Map;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
+import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.location.Location;
 import org.apache.brooklyn.api.mgmt.ExecutionContext;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
 import org.apache.brooklyn.api.objs.BrooklynObject;
 import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.config.internal.AbstractConfigMapImpl;
+import org.apache.brooklyn.core.entity.EntityInternal;
 import org.apache.brooklyn.core.location.AbstractLocation;
+import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
+import org.apache.brooklyn.core.objs.AbstractEntityAdjunct;
+import org.apache.brooklyn.util.core.task.Tasks;
 import org.apache.brooklyn.util.guava.Maybe;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -53,10 +58,18 @@ public class LocationConfigMap extends AbstractConfigMapImpl<Location> {
 
     @Override
     protected ExecutionContext getExecutionContext(BrooklynObject bo) {
-        if (bo==null) return null;
-        ManagementContext mgmt = ((AbstractLocation)bo).getManagementContext();
-        if (mgmt==null) return null;
-        return mgmt.getServerExecutionContext();
+        // Support DSL in location config. DSL expects to be resolved in the execution context of an entity.
+        // Since there's no location-entity relation try to infer it from the context of the caller.
+        Entity contextEntity = BrooklynTaskTags.getTargetOrContextEntity(Tasks.current());
+        if (contextEntity != null) {
+            return ((EntityInternal)contextEntity).getExecutionContext();
+        } else {
+            log.debug("No resolving context found, will use global execution context. Could lead to NPE on DSL resolving.");
+            if (bo==null) return null;
+            ManagementContext mgmt = ((AbstractLocation)bo).getManagementContext();
+            if (mgmt==null) return null;
+            return mgmt.getServerExecutionContext();
+        }
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/0019473f/core/src/main/java/org/apache/brooklyn/core/mgmt/BrooklynTaskTags.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/BrooklynTaskTags.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/BrooklynTaskTags.java
index e49bee5..68e62d7 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/BrooklynTaskTags.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/BrooklynTaskTags.java
@@ -35,7 +35,9 @@ import org.apache.brooklyn.api.mgmt.ExecutionManager;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
 import org.apache.brooklyn.api.mgmt.Task;
 import org.apache.brooklyn.api.mgmt.entitlement.EntitlementContext;
+import org.apache.brooklyn.core.mgmt.internal.AbstractManagementContext;
 import org.apache.brooklyn.util.core.config.ConfigBag;
+import org.apache.brooklyn.util.core.task.DynamicTasks;
 import org.apache.brooklyn.util.core.task.TaskTags;
 import org.apache.brooklyn.util.core.task.Tasks;
 import org.apache.brooklyn.util.guava.Maybe;
@@ -112,6 +114,16 @@ public class BrooklynTaskTags extends TaskTags {
     public static final String CALLER_ENTITY = "callerEntity";
     public static final String TARGET_ENTITY = "targetEntity";
     
+    /**
+     * Marks a task as running in the context of the entity. This means
+     * resolving any relative/context sensitive values against that entity.
+     * Using the entity in APIs where it is implicit - a prominent example
+     * being {@link DynamicTasks}.
+     *
+     * The result from the call should be used only when reading tags (for example
+     * to compare whether the tag already exists). The only place where the value is
+     * added to the entity tags is {@link AbstractManagementContext#getExecutionContext(Entity)}.
+     */
     public static WrappedEntity tagForContextEntity(Entity entity) {
         return new WrappedEntity(CONTEXT_ENTITY, entity);
     }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/0019473f/core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java b/core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java
index 8a64a7d..cb53990 100644
--- a/core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java
+++ b/core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java
@@ -320,7 +320,6 @@ public class InternalEntityFactory extends InternalFactory {
          * TODO It would be nice if these schedule tasks were grouped in a bucket! 
          */
         ((EntityInternal)entity).getExecutionContext().submit(Tasks.builder().dynamic(false).displayName("Entity initialization")
-                .tag(BrooklynTaskTags.tagForContextEntity(entity))
                 .tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
                 .body(new Runnable() {
             @Override

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/0019473f/core/src/main/java/org/apache/brooklyn/util/core/config/ResolvingConfigBag.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/config/ResolvingConfigBag.java b/core/src/main/java/org/apache/brooklyn/util/core/config/ResolvingConfigBag.java
index 7c6263f..a038516 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/config/ResolvingConfigBag.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/config/ResolvingConfigBag.java
@@ -20,13 +20,18 @@ package org.apache.brooklyn.util.core.config;
 
 import java.util.Map;
 
+import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.mgmt.ExecutionContext;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
 import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.entity.EntityInternal;
+import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
 import org.apache.brooklyn.util.core.task.DeferredSupplier;
 import org.apache.brooklyn.util.core.task.Tasks;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.guava.Maybe;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import com.google.common.annotations.Beta;
 import com.google.common.base.Function;
@@ -37,7 +42,10 @@ import com.google.common.collect.Sets;
  * As for {@link ConfigBag}, but resolves values that are of type {@link DeferredSupplier}.
  */
 @Beta
+// TODO Check if this is still needed after the changes in https://github.com/apache/brooklyn-server/pull/340.
+//      The PR now provides the execution context and resolves. Still there are cases which we need the transformer here, why?
 public class ResolvingConfigBag extends ConfigBag {
+    private static final Logger log = LoggerFactory.getLogger(ResolvingConfigBag.class);
 
     // Relies on various getters all delegating to a few common methods.
 
@@ -64,14 +72,24 @@ public class ResolvingConfigBag extends ConfigBag {
                 @Override public Object apply(Object input) {
                     if (input instanceof DeferredSupplier<?>) {
                         try {
-                            ExecutionContext exec = mgmt.getServerExecutionContext();
-                            return Tasks.resolveValue(input, Object.class, exec);
+                            return Tasks.resolveValue(input, Object.class, getExecutionContext());
                         } catch (Exception e) {
                             throw Exceptions.propagate(e);
                         }
                     }
                     return input;
                 }
+
+                protected ExecutionContext getExecutionContext() {
+                    // TODO Transformer is cached so on next getConfig call the context could be different - something to watch out for.
+                    Entity contextEntity = BrooklynTaskTags.getTargetOrContextEntity(Tasks.current());
+                    if (contextEntity instanceof EntityInternal) {
+                        return ((EntityInternal)contextEntity).getExecutionContext();
+                    } else {
+                        log.debug("No entity context found, will use global execution context. Could lead to NPE on DSL resolving in location config.");
+                        return mgmt.getServerExecutionContext();
+                    }
+                }
             };
         }
         return transformer;

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/0019473f/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java b/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java
index 74e0ddd..b220552 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionContext.java
@@ -40,6 +40,7 @@ import org.apache.brooklyn.core.entity.EntityInternal;
 import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
 import org.apache.brooklyn.core.mgmt.BrooklynTaskTags.WrappedEntity;
 import org.apache.brooklyn.core.mgmt.entitlement.Entitlements;
+import org.apache.brooklyn.util.collections.MutableMap;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -101,16 +102,23 @@ public class BasicExecutionContext extends AbstractExecutionContext {
         if (task instanceof TaskAdaptable<?> && !(task instanceof Task<?>)) 
             return submitInternal(propertiesQ, ((TaskAdaptable<?>)task).asTask());
         
-        Map properties = propertiesQ;
-        if (properties.get("tags")==null) properties.put("tags", new ArrayList()); 
-        Collection taskTags = (Collection)properties.get("tags");
+        Map properties = MutableMap.copyOf(propertiesQ);
+        Collection taskTags;
+        if (properties.get("tags")==null) {
+            taskTags = new ArrayList();
+        } else {
+            taskTags = new ArrayList((Collection)properties.get("tags"));
+        }
+        properties.put("tags", taskTags);
         
         // FIXME some of this is brooklyn-specific logic, should be moved to a BrooklynExecContext subclass;
         // the issue is that we want to ensure that cross-entity calls switch execution contexts;
         // previously it was all very messy how that was handled (and it didn't really handle it in many cases)
         if (task instanceof Task<?>) taskTags.addAll( ((Task<?>)task).getTags() ); 
         Entity target = BrooklynTaskTags.getWrappedEntityOfType(taskTags, BrooklynTaskTags.TARGET_ENTITY);
-        
+
+        checkUserSuppliedContext(task, taskTags);
+
         if (target!=null && !tags.contains(BrooklynTaskTags.tagForContextEntity(target))) {
             // task is switching execution context boundaries
             /* 
@@ -158,7 +166,7 @@ public class BasicExecutionContext extends AbstractExecutionContext {
                 }).build());
             }
         }
-        
+
         EntitlementContext entitlementContext = BrooklynTaskTags.getEntitlement(taskTags);
         if (entitlementContext==null)
         entitlementContext = Entitlements.getEntitlementContext();
@@ -167,7 +175,7 @@ public class BasicExecutionContext extends AbstractExecutionContext {
         }
 
         taskTags.addAll(tags);
-        
+
         if (Tasks.current()!=null && BrooklynTaskTags.isTransient(Tasks.current()) 
                 && !taskTags.contains(BrooklynTaskTags.NON_TRANSIENT_TASK_TAG) && !taskTags.contains(BrooklynTaskTags.TRANSIENT_TASK_TAG)) {
             // tag as transient if submitter is transient, unless explicitly tagged as non-transient
@@ -203,11 +211,46 @@ public class BasicExecutionContext extends AbstractExecutionContext {
             throw new IllegalArgumentException("Unhandled task type: task="+task+"; type="+(task!=null ? task.getClass() : "null"));
         }
     }
-    
+
     private void registerPerThreadExecutionContext() { perThreadExecutionContext.set(this); }
 
     private void clearPerThreadExecutionContext() { perThreadExecutionContext.remove(); }
 
+    private void checkUserSuppliedContext(Object task, Collection<Object> taskTags) {
+        Entity taskContext = BrooklynTaskTags.getWrappedEntityOfType(taskTags, BrooklynTaskTags.CONTEXT_ENTITY);
+        Entity defaultContext = BrooklynTaskTags.getWrappedEntityOfType(tags, BrooklynTaskTags.CONTEXT_ENTITY);
+        if (taskContext != null) {
+            if (log.isWarnEnabled()) {
+                String msg = "Deprecated since 0.10.0. Task " + task + " is submitted for execution but has context " +
+                        "entity (" + taskContext + ") tag set by the caller. ";
+                if (taskContext != defaultContext) {
+                    msg += "The context entity of the execution context (" + this + ") the task is submitted on is " +
+                            defaultContext + " which is different. This will cause any of them to be used at random at " +
+                            "runtime. ";
+                    if (task instanceof BasicTask) {
+                        msg += "Fixing the context entity to the latter. ";
+                    }
+                }
+                msg += "Setting the context entity by the caller is not allowed. See the documentation on " +
+                        "BrooklynTaskTags.tagForContextEntity(Entity) method for more details. Future Apache Brooklyn " +
+                        "releases will throw an exception instead of logging a warning.";
+
+                /**
+                 * @deprecated since 0.10.0
+                 */
+                // Should we rate limit?
+                log.warn(msg);
+            }
+
+            WrappedEntity contextTag = BrooklynTaskTags.tagForContextEntity(taskContext);
+            while(taskTags.remove(contextTag)) {};
+            if (task instanceof BasicTask) {
+                Set<?> mutableTags = BasicTask.class.cast(task).getMutableTags();
+                mutableTags.remove(contextTag);
+            }
+        }
+    }
+
     @Override
     public boolean isShutdown() {
         return getExecutionManager().isShutdown();

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/0019473f/core/src/test/java/org/apache/brooklyn/util/core/task/TasksTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/util/core/task/TasksTest.java b/core/src/test/java/org/apache/brooklyn/util/core/task/TasksTest.java
index cc37443..990e7f7 100644
--- a/core/src/test/java/org/apache/brooklyn/util/core/task/TasksTest.java
+++ b/core/src/test/java/org/apache/brooklyn/util/core/task/TasksTest.java
@@ -38,9 +38,6 @@ import org.apache.brooklyn.core.mgmt.BrooklynTaskTags.WrappedEntity;
 import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
 import org.apache.brooklyn.core.test.entity.TestApplication;
 import org.apache.brooklyn.core.test.entity.TestEntity;
-import org.apache.brooklyn.util.core.task.TaskInternal;
-import org.apache.brooklyn.util.core.task.Tasks;
-import org.apache.brooklyn.util.core.task.ValueResolver;
 import org.apache.brooklyn.util.guava.Functionals;
 import org.apache.brooklyn.util.repeat.Repeater;
 import org.apache.brooklyn.util.time.Duration;
@@ -186,29 +183,79 @@ public class TasksTest extends BrooklynAppUnitTestSupport {
         assertTrue(t.get(Duration.TEN_SECONDS));
     }
 
-    @Test(groups="Broken")
-    public void testSingleExecutionContextEntity() {
+    @Test
+    public void testSingleExecutionContextEntityWithTask() {
+        // Should cause an exception to be thrown in future releases. For now will log a warning.
+        // Until then make sure the task is tagged only with the context of the executor.
         final TestEntity entity = app.createAndManageChild(EntitySpec.create(TestEntity.class));
-        // context entity here = none
-        Task<Collection<Entity>> task = Tasks.<Collection<Entity>>builder()
+        Task<Void> task = Tasks.<Void>builder()
             .tag(BrooklynTaskTags.tagForContextEntity(entity))
-            .body(new Callable<Collection<Entity>>() {
-                @Override
-                public Collection<Entity> call() throws Exception {
-                    Collection<Entity> context = new ArrayList<>();
-                    for (Object tag : Tasks.current().getTags()) {
-                        if (tag instanceof WrappedEntity) {
-                            WrappedEntity wrapped = (WrappedEntity)tag;
-                            if (BrooklynTaskTags.CONTEXT_ENTITY.equals(wrapped.wrappingType)) {
-                                context.add(wrapped.entity);
-                            }
-                        }
+            .body(new AssertContextRunnable(ImmutableList.of(app))).build();
+        app.getExecutionContext().submit(task).getUnchecked();
+    }
+
+    @Test
+    public void testSingleExecutionContextEntityWithTaskAndExternalFlags() {
+        // Should cause an exception to be thrown in future releases. For now will log a warning.
+        // Until then make sure the task is tagged only with the context of the executor.
+        final TestEntity entity = app.createAndManageChild(EntitySpec.create(TestEntity.class));
+        Task<Void> task = Tasks.<Void>builder()
+            .body(new AssertContextRunnable(ImmutableList.of(app))).build();
+        ImmutableMap<String,?> flags = ImmutableMap.of(
+                "tags", ImmutableList.of(BrooklynTaskTags.tagForContextEntity(entity)));
+        app.getExecutionContext().submit(flags, task).getUnchecked();
+    }
+
+    @Test
+    public void testSingleExecutionContextEntityWithCallable() {
+        // Should cause an exception to be thrown in future releases. For now will log a warning.
+        // Until then make sure the task is tagged only with the context of the executor.
+        final TestEntity entity = app.createAndManageChild(EntitySpec.create(TestEntity.class));
+        Callable<Void> task = new Callable<Void>() {
+            @Override
+            public Void call() throws Exception {
+                new AssertContextRunnable(ImmutableList.of(app)).run();
+                return null;
+            }
+        };
+
+        ImmutableMap<String,?> flags = ImmutableMap.of(
+                "tags", ImmutableList.of(BrooklynTaskTags.tagForContextEntity(entity)));
+        app.getExecutionContext().submit(flags, task).getUnchecked();
+    }
+
+    @Test
+    public void testSingleExecutionContextEntityWithRunnable() {
+        // Should cause an exception to be thrown in future releases. For now will log a warning.
+        // Until then make sure the task is tagged only with the context of the executor.
+        final TestEntity entity = app.createAndManageChild(EntitySpec.create(TestEntity.class));
+        Runnable task = new AssertContextRunnable(ImmutableList.of(app));
+        ImmutableMap<String,?> flags = ImmutableMap.of(
+                "tags", ImmutableList.of(BrooklynTaskTags.tagForContextEntity(entity)));
+        app.getExecutionContext().submit(flags, task).getUnchecked();
+    }
+
+    private static class AssertContextRunnable implements Runnable {
+        private Collection<?> expectedContext;
+
+        public AssertContextRunnable(Collection<?> expectedContext) {
+            this.expectedContext = expectedContext;
+        }
+
+        @Override
+        public void run() {
+            Collection<Entity> context = new ArrayList<>();
+            for (Object tag : Tasks.current().getTags()) {
+                if (tag instanceof WrappedEntity) {
+                    WrappedEntity wrapped = (WrappedEntity)tag;
+                    if (BrooklynTaskTags.CONTEXT_ENTITY.equals(wrapped.wrappingType)) {
+                        context.add(wrapped.entity);
                     }
-                    return context;
                 }
-            }).build();
-        Task<Collection<Entity>> result = app.getExecutionContext().submit(task);
-        assertEquals(result.getUnchecked(), ImmutableList.of(entity));
+            }
+            assertEquals(context, expectedContext, "Found " + context + ", expected " + expectedContext);
+        }
+        
     }
 
 }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/0019473f/software/base/src/main/java/org/apache/brooklyn/entity/system_service/SystemServiceEnricher.java
----------------------------------------------------------------------
diff --git a/software/base/src/main/java/org/apache/brooklyn/entity/system_service/SystemServiceEnricher.java b/software/base/src/main/java/org/apache/brooklyn/entity/system_service/SystemServiceEnricher.java
index 26c0fdb..c018d3f 100644
--- a/software/base/src/main/java/org/apache/brooklyn/entity/system_service/SystemServiceEnricher.java
+++ b/software/base/src/main/java/org/apache/brooklyn/entity/system_service/SystemServiceEnricher.java
@@ -110,7 +110,6 @@ public class SystemServiceEnricher extends AbstractEnricher implements Enricher
                 .description("Update system service")
                 .add(installerTask)
                 .add(udpateTask)
-                .tag(BrooklynTaskTags.tagForContextEntity(entity))
                 .tag(BrooklynTaskTags.NON_TRANSIENT_TASK_TAG)
                 .build();
 


[4/4] brooklyn-server git commit: This closes #385

Posted by al...@apache.org.
This closes #385


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/6a4ca648
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/6a4ca648
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/6a4ca648

Branch: refs/heads/master
Commit: 6a4ca6486861b2ef7388992bdef074470d900d2c
Parents: f25fff7 0019473
Author: Aled Sage <al...@gmail.com>
Authored: Thu Nov 10 14:00:38 2016 +0000
Committer: Aled Sage <al...@gmail.com>
Committed: Thu Nov 10 14:00:38 2016 +0000

----------------------------------------------------------------------
 .../brooklyn/spi/dsl/methods/DslComponent.java  |  3 -
 .../brooklyn/camp/brooklyn/dsl/DslTest.java     | 21 ++++-
 .../location/internal/LocationConfigMap.java    | 21 ++++-
 .../brooklyn/core/mgmt/BrooklynTaskTags.java    | 12 +++
 .../core/objs/proxy/InternalEntityFactory.java  |  1 -
 .../util/core/config/ResolvingConfigBag.java    | 22 ++++-
 .../util/core/task/BasicExecutionContext.java   | 57 +++++++++++--
 .../brooklyn/util/core/task/ValueResolver.java  |  3 +-
 .../brooklyn/util/core/task/TasksTest.java      | 84 +++++++++++++++++++-
 .../system_service/SystemServiceEnricher.java   |  1 -
 10 files changed, 201 insertions(+), 24 deletions(-)
----------------------------------------------------------------------