[3/5] brooklyn-server git commit: apply the fix for previous failing tests and https://issues.apache.org/jira/browse/BROOKLYN-445

[sv...@apache.org]

apply the fix for previous failing tests and https://issues.apache.org/jira/browse/BROOKLYN-445

do not use a context catalog item id.  this is a significant change and release notes should be updated,
but it doesn't seem to break any of our tests; presumably most catalog items are already refactoring
so each item is declared as its own item.


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

Branch: refs/heads/master
Commit: 9c38c94eeeb5a3de2a082f81546440980b1abb5c
Parents: edaf054
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Tue Feb 28 12:55:24 2017 +0000
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Tue Feb 28 12:55:24 2017 +0000

----------------------------------------------------------------------
 .../brooklyn/api/internal/AbstractBrooklynObjectSpec.java |  8 ++++++--
 .../brooklyn/core/objs/proxy/InternalEntityFactory.java   | 10 ++++++++--
 2 files changed, 14 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/9c38c94e/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java
----------------------------------------------------------------------
diff --git a/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java b/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java
index e725885..6ca5f9d 100644
--- a/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java
+++ b/api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java
@@ -75,7 +75,6 @@ public abstract class AbstractBrooklynObjectSpec<T,SpecT extends AbstractBrookly
     protected AbstractBrooklynObjectSpec(Class<? extends T> type) {
         checkValidType(type);
         this.type = type;
-        this.catalogItemId = ApiObjectsFactory.get().getCatalogItemIdFromContext();
     }
     
     @SuppressWarnings("unchecked")
@@ -98,13 +97,18 @@ public abstract class AbstractBrooklynObjectSpec<T,SpecT extends AbstractBrookly
         return self();
     }
     
+    /** Set the catalog item ID that defined this object, also used for searching for type and resources referenced */
+    // since https://issues.apache.org/jira/browse/BROOKLYN-445 this must no longer be used to indicate
+    // a caller-context catalog item that should be used for search purposes;
+    // if that behaviour is desired, the child should be refactored to be its own item in the catalog BOM
+    // (or TODO we add a separate field to record other catalog item IDs that could be applied for searching, see below)
     public SpecT catalogItemId(String val) {
         catalogItemId = val;
         return self();
     }
     // TODO in many places (callers to this method) we prefer a wrapper item ID;
     // that is right, because the wrapper's defn will refer to the wrapped,
-    // but we might need also to collect the item ID's so that *all* can be searched.
+    // but we might need also to collect the item ID's so that *all* can be searched, see #catalogItemId.
     // e.g. if R3 references R2 which references R1 any one of these might supply config keys 
     // referencing resources or types in their local bundles. 
     @Beta

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/9c38c94e/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 401b312..6090c5b 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
@@ -271,8 +271,14 @@ public class InternalEntityFactory extends InternalFactory {
     }
 
     private void addSpecParameters(EntitySpec<?> spec, EntityDynamicType edType) {
-        // if coming from a catalog item, the spec list of parameters is canonical, 
-        // ie it has just the right ones from the class, so clear the parameters on the type
+        // if coming from a catalog item, parsed by CAMP, then the spec list of parameters is canonical,
+        // the parent item has had its config keys set as parameters here with those non-inheritable
+        // via type definition removed, so wipe those on the EDT to make sure non-inheritable ones are removed; 
+        // OTOH if item is blank, it was set as a java type, not inheriting it,
+        // and the config keys on the dynamic type are the correct ones to use, and usually there is nothing in spec.parameters,
+        // except what is being added programmatically.
+        // (this logic could get confused if catalog item ID referred to some runtime-inherited context,
+        // but those semantics should no longer be used -- https://issues.apache.org/jira/browse/BROOKLYN-445)
         if (Strings.isNonBlank(spec.getCatalogItemId())) {
             edType.clearConfigKeys();
         }