brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aleds...@apache.org
Subject [08/50] git commit: expand create-entity-from-spec logic so that it does two passes, one to build up the hierarchy, and one to run initializers and policies (still simpler than rebind, but more complicated than it was, to support child specs with initial
Date Wed, 09 Jul 2014 21:46:21 GMT
expand create-entity-from-spec logic so that it does two passes, one to build up the hierarchy,
and one to run initializers and policies
(still simpler than rebind, but more complicated than it was, to support child specs with
initializers which try to access their application)


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/57b26125
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/57b26125
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/57b26125

Branch: refs/heads/master
Commit: 57b261258efaef59f3def667da523a51b483c6e4
Parents: 79e5cd9
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Authored: Fri Jul 4 14:15:53 2014 +0100
Committer: Aled Sage <aled.sage@gmail.com>
Committed: Wed Jul 9 22:34:42 2014 +0100

----------------------------------------------------------------------
 .../entity/proxying/InternalEntityFactory.java  | 175 +++++++++++--------
 1 file changed, 104 insertions(+), 71 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/57b26125/core/src/main/java/brooklyn/entity/proxying/InternalEntityFactory.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/proxying/InternalEntityFactory.java b/core/src/main/java/brooklyn/entity/proxying/InternalEntityFactory.java
index 92a9845..4f8381c 100644
--- a/core/src/main/java/brooklyn/entity/proxying/InternalEntityFactory.java
+++ b/core/src/main/java/brooklyn/entity/proxying/InternalEntityFactory.java
@@ -146,6 +146,26 @@ public class InternalEntityFactory {
     }
 
     public <T extends Entity> T createEntity(EntitySpec<T> spec) {
+        /* Order is important here. Changed Jul 2014 when supporting children in spec.
+         * (Previously was much simpler, and parent was set right after running initializers;
and there were no children.)
+         * <p>
+         * It seems we need access to the parent (indeed the root application) when running
some initializers (esp children initializers).
+         * <p>
+         * Now we do two passes, so that hierarchy is fully populated before initialization
and policies.
+         * (This is needed where some config or initializer might reference another entity
by its ID, e.g. yaml $brooklyn:component("id"). 
+         * Initialization is done in parent-first order with depth-first children traversal.
+         */
+
+        // (maps needed because we need the spec, and we need to keep the AbstractEntity
to call init, not a proxy)
+        Map<String,Entity> entitiesByEntityId = MutableMap.of();
+        Map<String,EntitySpec<?>> specsByEntityId = MutableMap.of();
+        
+        T entity = createEntityAndDescendantsUninitialized(spec, entitiesByEntityId, specsByEntityId);
+        initEntityAndDescendants(entity.getId(), entitiesByEntityId, specsByEntityId);
+        return entity;
+    }
+    
+    protected <T extends Entity> T createEntityAndDescendantsUninitialized(EntitySpec<T>
spec, Map<String,Entity> entitiesByEntityId, Map<String,EntitySpec<?>> specsByEntityId)
{
         if (spec.getFlags().containsKey("parent") || spec.getFlags().containsKey("owner"))
{
             throw new IllegalArgumentException("Spec's flags must not contain parent or owner;
use spec.parent() instead for "+spec);
         }
@@ -168,7 +188,24 @@ public class InternalEntityFactory {
             ((AbstractEntity)entity).setManagementContext(managementContext);
             managementContext.prePreManage(entity);
 
-            initEntity(entity, spec);
+            loadUnitializedEntity(entity, spec);
+
+            entitiesByEntityId.put(entity.getId(), entity);
+            specsByEntityId.put(entity.getId(), spec);
+
+            for (EntitySpec<?> childSpec : spec.getChildren()) {
+                if (childSpec.getParent()!=null) {
+                    if (!childSpec.getParent().equals(entity)) {
+                        throw new IllegalStateException("Spec "+childSpec+" has parent "+childSpec.getParent()+"
defined, "
+                            + "but it is defined as a child of "+entity);
+                    }
+                    log.warn("Child spec "+childSpec+" is already set with parent "+entity+";
how did this happen?!");
+                }
+                childSpec.parent(entity);
+                Entity child = createEntityAndDescendantsUninitialized(childSpec, entitiesByEntityId,
specsByEntityId);
+                entity.addChild(child);
+            }
+            
             return entity;
             
         } catch (Exception e) {
@@ -177,7 +214,7 @@ public class InternalEntityFactory {
     }
     
     @SuppressWarnings({ "unchecked", "rawtypes" })
-    public <T extends Entity> T initEntity(final T entity, final EntitySpec<T>
spec) {
+    protected <T extends Entity> T loadUnitializedEntity(final T entity, final EntitySpec<T>
spec) {
         try {
             if (spec.getDisplayName()!=null)
                 ((AbstractEntity)entity).setDisplayName(spec.getDisplayName());
@@ -189,81 +226,12 @@ public class InternalEntityFactory {
                 ((EntityLocal)entity).setConfig((ConfigKey)entry.getKey(), entry.getValue());
             }
 
-            /* Order is important here. Changed Jul 2014 when supporting children in spec.
-             * (Previously parent was set *after* running initializers, and there were no
children.)
-             * <p>
-             * We may want access to the parent when running initializers (or in children
initializers).
-             * <p>
-             * Currently initializers will be run before children are added.
-             * on the basis that it is more likely children will want to assume parent is
initialized,
-             * than initializers will want access to children.
-             * <p>
-             * If this is not good enough we could do an additional pass where all children
are built up
-             * before running any initializers.
-             */
             Entity parent = spec.getParent();
             if (parent != null) {
                 parent = (parent instanceof AbstractEntity) ? ((AbstractEntity)parent).getProxyIfAvailable()
: parent;
                 entity.setParent(parent);
             }
-
-            /* Marked transient so that the task is not needlessly kept around at the highest
level.
-             * Note that the task is not normally visible in the GUI, because 
-             * (a) while it is running, the entity is parentless (and so not in the tree);
-             * and (b) when it is completed it is GC'd, as it is transient.
-             * However task info is available via the API if you know its ID,
-             * and if better subtask querying is available it will be picked up as a background
task 
-             * of the parent entity creating this child entity
-             * (note however such subtasks are currently filtered based on parent entity
so is excluded).
-             */
-            ((EntityInternal)entity).getExecutionContext().submit(Tasks.builder().dynamic(false).name("Entity
initialization").tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
-                    .body(new Runnable() {
-                @Override
-                public void run() {
-                    ((AbstractEntity)entity).init();
-                    for (EntityInitializer initializer: spec.getInitializers())
-                        initializer.apply((EntityInternal)entity);
-                    /* 31 Mar 2014, moved initialization (above) into this task: primarily
for consistency and traceability on failure.
-                     * TBC whether this is good/bad/indifferent. My (Alex) opinion is that
whether it is done in a subtask 
-                     * should be the same as whether enricher/policy/etc (below) is done
subtasks, which is was added recently
-                     * in 249c96fbb18bd9d763029475e0a3dc251c01b287. @nakomis can you give
exact reason code below is needed in a task
-                     * commit message said was to do with wiring up yaml sensors and policies
-- which makes sense but specifics would be handy!
-                     * and would let me know if there is any reason to do / not_do the initializer
code above also here! 
-                     */
-                    
-                    for (Enricher enricher : spec.getEnrichers()) {
-                        entity.addEnricher(enricher);
-                    }
-                    
-                    for (EnricherSpec<?> enricherSpec : spec.getEnricherSpecs()) {
-                        entity.addEnricher(policyFactory.createEnricher(enricherSpec));
-                    }
-                    
-                    for (Policy policy : spec.getPolicies()) {
-                        entity.addPolicy((AbstractPolicy)policy);
-                    }
-                    
-                    for (PolicySpec<?> policySpec : spec.getPolicySpecs()) {
-                        entity.addPolicy(policyFactory.createPolicy(policySpec));
-                    }
-                    
-                    ((AbstractEntity)entity).addLocations(spec.getLocations());
-                }
-            }).build()).get();
             
-            for (EntitySpec<?> childSpec : spec.getChildren()) {
-                if (childSpec.getParent()!=null) {
-                    if (!childSpec.getParent().equals(entity)) {
-                        throw new IllegalStateException("Spec "+childSpec+" has parent "+childSpec.getParent()+"
defined, "
-                            + "but it is defined as a child of "+entity);
-                    }
-                    log.warn("Child spec "+childSpec+" is already set with parent "+entity+";
how did this happen?!");
-                }
-                childSpec.parent(entity);
-                Entity child = createEntity(childSpec);
-                entity.addChild(child);
-            }
-
             return entity;
             
         } catch (Exception e) {
@@ -271,6 +239,71 @@ public class InternalEntityFactory {
         }
     }
     
+    protected <T extends Entity> void initEntityAndDescendants(String entityId, final
Map<String,Entity> entitiesByEntityId, final Map<String,EntitySpec<?>> specsByEntityId)
{
+        final Entity entity = entitiesByEntityId.get(entityId);
+        final EntitySpec<?> spec = specsByEntityId.get(entityId);
+        
+        if (entity==null || spec==null) {
+            log.debug("Skipping initialization of "+entityId+" found as child of entity being
initialized, "
+                + "but this child is not one we created; likely it was created by an initializer,
"
+                + "and thus it should be already fully initialized.");
+            return;
+        }
+        
+        /* Marked transient so that the task is not needlessly kept around at the highest
level.
+         * Note that the task is not normally visible in the GUI, because 
+         * (a) while it is running, the entity is parentless (and so not in the tree);
+         * and (b) when it is completed it is GC'd, as it is transient.
+         * However task info is available via the API if you know its ID,
+         * and if better subtask querying is available it will be picked up as a background
task 
+         * of the parent entity creating this child entity
+         * (note however such subtasks are currently filtered based on parent entity so is
excluded).
+         */
+        ((EntityInternal)entity).getExecutionContext().submit(Tasks.builder().dynamic(false).name("Entity
initialization")
+                .tag(BrooklynTaskTags.tagForContextEntity(entity))
+                .tag(BrooklynTaskTags.TRANSIENT_TASK_TAG)
+                .body(new Runnable() {
+            @Override
+            public void run() {
+                ((AbstractEntity)entity).init();
+                
+                ((AbstractEntity)entity).addLocations(spec.getLocations());
+
+                for (EntityInitializer initializer: spec.getInitializers())
+                    initializer.apply((EntityInternal)entity);
+                /* 31 Mar 2014, moved initialization (above) into this task: primarily for
consistency and traceability on failure.
+                 * TBC whether this is good/bad/indifferent. My (Alex) opinion is that whether
it is done in a subtask 
+                 * should be the same as whether enricher/policy/etc (below) is done subtasks,
which is was added recently
+                 * in 249c96fbb18bd9d763029475e0a3dc251c01b287. @nakomis can you give exact
reason code below is needed in a task
+                 * commit message said was to do with wiring up yaml sensors and policies
-- which makes sense but specifics would be handy!
+                 * and would let me know if there is any reason to do / not_do the initializer
code above also here! 
+                 */
+                
+                for (Enricher enricher : spec.getEnrichers()) {
+                    entity.addEnricher(enricher);
+                }
+                
+                for (EnricherSpec<?> enricherSpec : spec.getEnricherSpecs()) {
+                    entity.addEnricher(policyFactory.createEnricher(enricherSpec));
+                }
+                
+                for (Policy policy : spec.getPolicies()) {
+                    entity.addPolicy((AbstractPolicy)policy);
+                }
+                
+                for (PolicySpec<?> policySpec : spec.getPolicySpecs()) {
+                    entity.addPolicy(policyFactory.createPolicy(policySpec));
+                }
+                                
+                for (Entity child: entity.getChildren()) {
+                    // right now descendants are initialized depth-first (see the getUnchecked()
call below)
+                    // they could be done in parallel, but OTOH initializers should be very
quick
+                    initEntityAndDescendants(child.getId(), entitiesByEntityId, specsByEntityId);
+                }
+            }
+        }).build()).getUnchecked();        
+    }
+    
     /**
      * Constructs an entity (if new-style, calls no-arg constructor; if old-style, uses spec
to pass in config).
      */


Mime
View raw message