[
https://issues.apache.org/jira/browse/BROOKLYN-445?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15978782#comment-15978782
]
ASF GitHub Bot commented on BROOKLYN-445:
-----------------------------------------
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/338#discussion_r112694452
--- Diff:
api/src/main/java/org/apache/brooklyn/api/internal/AbstractBrooklynObjectSpec.java
---
@@ -91,26 +96,39 @@ public String toString() {
}
protected abstract void checkValidType(Class<? extends T> type);
-
+
public SpecT displayName(String val) {
displayName = val;
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)
+
+ /**
+ * Set the catalog item ID that defined this object;
+ * 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;
+ * instead use {@link #catalogItemIdAndSearchPath}.
+ */
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, 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.
+
+ /**
+ * Set the immediate catalog item ID of this object, and the search
path of other catalog items used to define it.
+ */
+ public synchronized SpecT catalogItemIdAndSearchPath(String
catalogItemId, Collection<String> searchPath) {
+ if (catalogItemId != null) {
--- End diff --
what is the reason for this behaviour on null? ignoring searchPath seems
weird.
> Search path and meaning of catalogItemId is overloaded
> ------------------------------------------------------
>
> Key: BROOKLYN-445
> URL: https://issues.apache.org/jira/browse/BROOKLYN-445
> Project: Brooklyn
> Issue Type: Bug
> Affects Versions: 0.10.0
> Reporter: Alex Heneveld
>
> We use catalogItemId on a spec or BrooklynObject for two things:
> (1) Record the catalog item that was defined to create an item
> (2) Find the search path to use when looking up resources and/or creating
> other specs
> Most of the time these two are the same, e.g. an entity comes from catalog
> item `cassandra-node:1.0` and the implementation should use the bundles
> defined against that to resolve scripts etc. The catalogItemId is the only
> record of the catalog-entry that was used to define the entity; the entity
> spec reduces it to the java type which might tell you one bundle but won't
> tell you of other library entries from the definition.
> However in some cases we seem to want the context catalogItemId to be
> inherited, e.g. we reference a stock type like `VanillaServer` as a child but
> supply config pointing at scripts/images in the parent's bundle. This is
> currently achieved by a line in AbstractBrooklynObjectSpec's constructor
> which sets the catalogItemId from the thread context entity's catalog item
> ID. However there are a couple problems with this:
> (A) We can't tell if the `catalogItemId` is the definition of an entity (it
> usually is, but sometimes might be inherited), so we can't tell what was used
> to create an entity
> (B) A child's search behaviour differs depending whether that child comes
> from another catalogItemId (which will overwrite the inherited context
> catalogItemId) or a stock item (e.g. a java type is passed to the spec
> constructor)
> (C) When setting the EntityDynamicType we can't tell whether to clear the
> config keys set there; in InternalEntityFactory.addSpecParameters we need to
> know whether the spec extends the Java type defining it (in which case CAMP
> code in BasicSpecParameter.initializeSpecWithExplicitParameters has filtered
> out those keys which are not type-definition inheritable and set the
> spec.parameters, so the EntityDynamicType's keys should be cleared) or not
> (in which case spec.parameters won't normally be set and EDT.configKeys
> should not be cleared). Currently it checks whether there is a
> catalogItemId, and assume it is set iff the spec is extending (former case);
> this assumption fails if catalogItemId is inherited from context.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)