This is an automated email from the ASF dual-hosted git repository.

heneveld pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git

commit 6b9837a7680ba255ce3d9b6704bd1aa050d44393
Author: Alex Heneveld <[email protected]>
AuthorDate: Wed Dec 7 22:07:17 2022 +0000

    use containing bundle as part of the search path, and prefer the search path
---
 .../classloading/BrooklynClassLoadingContext.java  |   5 +
 .../catalog/NestedRefsCatalogYamlTest.java         | 148 +++++++++++++++------
 .../nested-refs-catalog-yaml-test-2.bom.yaml       |  37 ++++++
 .../nested-refs-catalog-yaml-test-3.bom.yaml       |  36 +++++
 .../catalog/internal/BasicBrooklynCatalog.java     |  66 ++++-----
 .../BrooklynClassLoadingContextSequential.java     |  21 ++-
 .../JavaBrooklynClassLoadingContext.java           |  10 +-
 .../OsgiBrooklynClassLoadingContext.java           |   2 +
 .../resolve/entity/CatalogEntitySpecResolver.java  |   4 +-
 ...BrooklynRegisteredTypeJacksonSerialization.java |   3 +-
 .../core/typereg/BasicBrooklynTypeRegistry.java    |   2 +-
 .../brooklyn/core/typereg/RegisteredTypes.java     |  23 ++++
 12 files changed, 277 insertions(+), 80 deletions(-)

diff --git 
a/api/src/main/java/org/apache/brooklyn/api/mgmt/classloading/BrooklynClassLoadingContext.java
 
b/api/src/main/java/org/apache/brooklyn/api/mgmt/classloading/BrooklynClassLoadingContext.java
index 2441af550f..74fb0e33c2 100644
--- 
a/api/src/main/java/org/apache/brooklyn/api/mgmt/classloading/BrooklynClassLoadingContext.java
+++ 
b/api/src/main/java/org/apache/brooklyn/api/mgmt/classloading/BrooklynClassLoadingContext.java
@@ -19,13 +19,18 @@
 package org.apache.brooklyn.api.mgmt.classloading;
 
 import org.apache.brooklyn.api.mgmt.ManagementContext;
+import org.apache.brooklyn.api.typereg.OsgiBundleWithUrl;
 import org.apache.brooklyn.util.javalang.ClassLoadingContext;
 
+import java.util.Collection;
+
 /** 
  * As {@link ClassLoadingContext} but the {@link ManagementContext} is also 
available.
  */
 public interface BrooklynClassLoadingContext extends ClassLoadingContext {
 
     public ManagementContext getManagementContext();
+    // bundle search path, if defined
+    public Collection<? extends OsgiBundleWithUrl> getBundles();
 
 }
diff --git 
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/NestedRefsCatalogYamlTest.java
 
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/NestedRefsCatalogYamlTest.java
index 7e82042241..c2f88b7e76 100644
--- 
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/NestedRefsCatalogYamlTest.java
+++ 
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/NestedRefsCatalogYamlTest.java
@@ -18,49 +18,20 @@
  */
 package org.apache.brooklyn.camp.brooklyn.catalog;
 
-import com.google.common.base.Joiner;
-import com.google.common.base.Throwables;
-import com.google.common.collect.Iterables;
-import org.apache.brooklyn.api.catalog.BrooklynCatalog;
-import org.apache.brooklyn.api.entity.Entity;
-import org.apache.brooklyn.api.entity.EntitySpec;
 import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec;
-import org.apache.brooklyn.api.objs.Identifiable;
-import org.apache.brooklyn.api.typereg.BrooklynTypeRegistry;
-import org.apache.brooklyn.api.typereg.BrooklynTypeRegistry.RegisteredTypeKind;
+import org.apache.brooklyn.api.mgmt.classloading.BrooklynClassLoadingContext;
 import org.apache.brooklyn.api.typereg.RegisteredType;
 import org.apache.brooklyn.camp.brooklyn.AbstractYamlTest;
-import org.apache.brooklyn.camp.brooklyn.ConfigYamlTest;
-import org.apache.brooklyn.config.ConfigKey;
-import org.apache.brooklyn.core.catalog.internal.BasicBrooklynCatalog;
-import org.apache.brooklyn.core.catalog.internal.CatalogUtils;
-import org.apache.brooklyn.core.config.ConfigKeys;
-import org.apache.brooklyn.core.entity.Dumper;
-import org.apache.brooklyn.core.mgmt.BrooklynTags;
-import org.apache.brooklyn.core.mgmt.BrooklynTags.SpecSummary;
-import org.apache.brooklyn.core.test.entity.TestEntity;
-import org.apache.brooklyn.core.test.entity.TestEntityImpl;
-import org.apache.brooklyn.core.typereg.RegisteredTypes;
-import org.apache.brooklyn.core.workflow.WorkflowBasicTest;
-import org.apache.brooklyn.entity.stock.BasicApplication;
-import org.apache.brooklyn.entity.stock.BasicEntity;
-import org.apache.brooklyn.entity.stock.BasicEntityImpl;
+import 
org.apache.brooklyn.core.mgmt.classloading.OsgiBrooklynClassLoadingContext;
+import org.apache.brooklyn.core.typereg.RegisteredTypeLoadingContexts;
 import org.apache.brooklyn.test.Asserts;
-import org.apache.brooklyn.util.collections.MutableList;
-import org.apache.brooklyn.util.collections.MutableSet;
 import org.apache.brooklyn.util.core.ResourceUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.testng.Assert;
 import org.testng.annotations.Test;
 
 import java.util.Collection;
-import java.util.List;
 import java.util.Map;
-import java.util.Set;
-
-import static com.google.common.base.Preconditions.checkNotNull;
-import static org.testng.Assert.*;
 
 
 public class NestedRefsCatalogYamlTest extends AbstractYamlTest {
@@ -73,23 +44,126 @@ public class NestedRefsCatalogYamlTest extends 
AbstractYamlTest {
     }
 
     @Test
-    public void testManyAddCatalogItems() throws Exception {
+    public void testPreferHighestVersion() {
+        addCatalogItems("brooklyn.catalog:\n" +
+                "  bundle: Bv2\n" +
+                "  version: \"2\"\n" +
+                "  items:\n" +
+                "    - id: A\n" +
+                "      item:\n" +
+                "        name: Av2\n" +
+                "        type: 
org.apache.brooklyn.entity.stock.BasicEntity\n");
+        addCatalogItems("brooklyn.catalog:\n" +
+                "  bundle: Bv1\n" +
+                "  version: \"1\"\n" +
+                "  items:\n" +
+                "    - id: A\n" +
+                "      item:\n" +
+                "        name: Av1\n" +
+                "        type: 
org.apache.brooklyn.entity.stock.BasicEntity\n");
 
-        /* test that invoke-effector doesn't cause confusion */
+        AbstractBrooklynObjectSpec<?, ?> x = 
mgmt().getTypeRegistry().createSpec(
+                mgmt().getTypeRegistry().get("A"), null, null);
 
-//        WorkflowBasicTest.addWorkflowStepTypes(mgmt());
+        Asserts.assertEquals(x.getDisplayName(), "Av2");
+    }
+
+    @Test
+    public void testPreferSameBundle() {
+        addCatalogItems("brooklyn.catalog:\n" +
+                "  bundle: Bv2\n" +
+                "  version: \"2\"\n" +
+                "  items:\n" +
+                "    - id: A\n" +
+                "      item:\n" +
+                "        name: Av2\n" +
+                "        type: 
org.apache.brooklyn.entity.stock.BasicEntity\n");
+        addCatalogItems("brooklyn.catalog:\n" +
+                "  bundle: Bv1\n" +
+                "  version: \"1\"\n" +
+                "  items:\n" +
+                "    - id: A\n" +
+                "      item:\n" +
+                "        name: Av1\n" +
+                "        type: org.apache.brooklyn.entity.stock.BasicEntity\n" 
+
+                "    - id: X\n" +
+                "      item:\n" +
+                "        type: A");
+        AbstractBrooklynObjectSpec<?, ?> x = 
mgmt().getTypeRegistry().createSpec(
+                mgmt().getTypeRegistry().get("X"), null, null);
+
+        Asserts.assertEquals(x.getDisplayName(), "Av1");
+    }
+
+    @Test
+    public void testPreferSameBundleComplex() {
+        addCatalogItems("brooklyn.catalog:\n" +
+                "  bundle: Bv2\n" +
+                "  version: \"2\"\n" +
+                "  items:\n" +
+                "    - id: A\n" +
+                "      item:\n" +
+                "        name: Av2\n" +
+                "        type: org.apache.brooklyn.entity.stock.BasicEntity\n"+
+                "    - id: Z\n" +
+                "      item:\n" +
+                "        name: Z\n" +
+                "        type: 
org.apache.brooklyn.entity.stock.BasicEntity\n");
+        addCatalogItems("brooklyn.catalog:\n" +
+                "  bundle: Bv1\n" +
+                "  version: \"1\"\n" +
+                "  items:\n" +
+                "    - id: A\n" +
+                "      item:\n" +
+                "        name: Av1\n" +
+                "        type: Z\n" +
+                "    - id: X\n" +
+                "      item:\n" +
+                "        type: A");
+        AbstractBrooklynObjectSpec<?, ?> x = 
mgmt().getTypeRegistry().createSpec(
+                mgmt().getTypeRegistry().get("X"), null, null);
+
+        Asserts.assertEquals(x.getDisplayName(), "Av1");
+    }
+
+    /* test that invoke-effector doesn't cause confusion */
+    void doTestAddAfterWorkflow(String url) {
 
+        // add invoke-effector bean via a bundle
+//        WorkflowBasicTest.addWorkflowStepTypes(mgmt());
         Collection<RegisteredType> typesW = 
addCatalogItems(ResourceUtils.create(this).getResourceAsString("classpath://nested-refs-catalog-yaml-workflow-test.bom.yaml"));
         Map<RegisteredType, Collection<Throwable>> resultW = 
mgmt().getCatalog().validateTypes(typesW);
         if (!resultW.isEmpty()) {
             Asserts.fail("Should be empty: " + resultW);
         }
 
-        Collection<RegisteredType> types = 
addCatalogItems(ResourceUtils.create(this).getResourceAsString("classpath://nested-refs-catalog-yaml-test.bom.yaml"));
+        // now add it as a type, in various ways
+        Collection<RegisteredType> types = 
addCatalogItems(ResourceUtils.create(this).getResourceAsString(url));
+
+        // should prefer from local bundle
+        Asserts.assertEquals(mgmt().getTypeRegistry().get("invoke-effector", 
RegisteredTypeLoadingContexts.loader(
+                new OsgiBrooklynClassLoadingContext(mgmt(), 
types.iterator().next().getId(), null))).getContainingBundle(), 
"test-bundle:0.1.0-SNAPSHOT");
+
+        // which should allow it to validate
         Map<RegisteredType, Collection<Throwable>> result = 
mgmt().getCatalog().validateTypes(types);
         if (!result.isEmpty()) {
-            Asserts.fail("Should be empty: " + result);
+            Asserts.fail("Failed validation should be empty: " + result);
         }
     }
 
+    @Test
+    public void testAddAmbiguousCatalogItemsPreferEntitySpecAsParent() throws 
Exception {
+        
doTestAddAfterWorkflow("classpath://nested-refs-catalog-yaml-test.bom.yaml");
+    }
+
+    @Test
+    public void testAddAmbiguousCatalogItemsPreferFromSameBundle() throws 
Exception {
+        
doTestAddAfterWorkflow("classpath://nested-refs-catalog-yaml-test-2.bom.yaml");
+    }
+
+    @Test(groups = "Broken")  // need to defer loading until all items 
pre-parsed, or correct loading later
+    public void 
testAddAmbiguousCatalogItemsPreferFromSameBundleEvenIfDefinedLater() throws 
Exception {
+        
doTestAddAfterWorkflow("classpath://nested-refs-catalog-yaml-test-3.bom.yaml");
+    }
+
 }
diff --git 
a/camp/camp-brooklyn/src/test/resources/nested-refs-catalog-yaml-test-2.bom.yaml
 
b/camp/camp-brooklyn/src/test/resources/nested-refs-catalog-yaml-test-2.bom.yaml
new file mode 100644
index 0000000000..1cd1c22b11
--- /dev/null
+++ 
b/camp/camp-brooklyn/src/test/resources/nested-refs-catalog-yaml-test-2.bom.yaml
@@ -0,0 +1,37 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#  http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+brooklyn.catalog:
+  bundle: test-bundle
+  version: "0.1.0-SNAPSHOT"
+  items:
+
+    - id: invoke-effector
+      item:
+        type: org.apache.brooklyn.entity.stock.BasicEntity
+
+    - id: child
+      item:
+        type: invoke-effector
+
+    - id: parent
+      item:
+        type: org.apache.brooklyn.entity.stock.BasicEntity
+        brooklyn.children:
+          - type: child
+
diff --git 
a/camp/camp-brooklyn/src/test/resources/nested-refs-catalog-yaml-test-3.bom.yaml
 
b/camp/camp-brooklyn/src/test/resources/nested-refs-catalog-yaml-test-3.bom.yaml
new file mode 100644
index 0000000000..bb3b64b44f
--- /dev/null
+++ 
b/camp/camp-brooklyn/src/test/resources/nested-refs-catalog-yaml-test-3.bom.yaml
@@ -0,0 +1,36 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#  http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+brooklyn.catalog:
+  bundle: test-bundle
+  version: "0.1.0-SNAPSHOT"
+  items:
+
+    - id: child
+      item:
+        type: invoke-effector
+
+    - id: parent
+      item:
+        type: org.apache.brooklyn.entity.stock.BasicEntity
+        brooklyn.children:
+          - type: child
+
+    - id: invoke-effector
+      item:
+        type: org.apache.brooklyn.entity.stock.BasicEntity
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java
 
b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java
index 59ae974454..35f60b23ab 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java
@@ -778,22 +778,8 @@ public class BasicBrooklynCatalog implements 
BrooklynCatalog {
             }
         }
         PlanInterpreterInferringType planInterpreter = new 
PlanInterpreterInferringType(id, item, sourceYaml, itemType, format,
-                (containingBundle instanceof CatalogBundle ? 
((CatalogBundle)containingBundle) : null), libraryBundles,
-                null, resultLegacyFormat).resolve();
-
-        if (!planInterpreter.isResolved()) {
-            // don't throw yet, we may be able to add it in an unresolved state
-            resolutionError = Exceptions.create("Could not resolve definition 
of item"
-                + (Strings.isNonBlank(id) ? " '"+id+"'" : 
Strings.isNonBlank(symbolicName) ? " '"+symbolicName+"'" : 
Strings.isNonBlank(name) ? " '"+name+"'" : "")
-                // better not to show yaml, takes up lots of space, and with 
multiple plan transformers there might be multiple errors;
-                // some of the errors themselves may reproduce it
-                // (ideally in future we'll be able to return typed errors 
with caret position of error)
-//                + ":\n"+sourceYaml
-                , 
MutableList.<Exception>of().appendIfNotNull(resolutionError).appendAll(planInterpreter.getErrors()));
-        }
-
-        // might be null
-        itemType = planInterpreter.getCatalogItemType();
+                containingBundle, libraryBundles,
+                null, resultLegacyFormat);
 
         Map<?, ?> itemAsMap = planInterpreter.getItem();
         // the "plan yaml" includes the services: ... or brooklyn.policies: 
... outer key,
@@ -925,10 +911,27 @@ public class BasicBrooklynCatalog implements 
BrooklynCatalog {
         final String deprecated = getFirstAs(catalogMetadata, String.class, 
"deprecated").orNull();
         final Boolean catalogDeprecated = 
Boolean.valueOf(setFromItemIfUnset(deprecated, itemAsMap, "deprecated"));
 
-        // run again now that we know the ID to catch recursive definitions 
and possibly other mistakes (itemType inconsistency?)
-        planInterpreter = planInterpreter.setId(id).resolve();
-        if (resolutionError==null && !planInterpreter.isResolved()) {
-            resolutionError = new IllegalStateException("Plan resolution for 
"+id+" breaks after id and itemType are set; is there a recursive reference or 
other type inconsistency?\n"+sourceYaml);
+        planInterpreter.resolve();
+        if (!planInterpreter.isResolved()) {
+            // don't throw yet, we may be able to add it in an unresolved state
+            resolutionError = Exceptions.create("Could not resolve definition 
of item"
+                            + (Strings.isNonBlank(id) ? " '"+id+"'" : 
Strings.isNonBlank(symbolicName) ? " '"+symbolicName+"'" : 
Strings.isNonBlank(name) ? " '"+name+"'" : "")
+                    // better not to show yaml, takes up lots of space, and 
with multiple plan transformers there might be multiple errors;
+                    // some of the errors themselves may reproduce it
+                    // (ideally in future we'll be able to return typed errors 
with caret position of error)
+//                + ":\n"+sourceYaml
+                    , 
MutableList.<Exception>of().appendIfNotNull(resolutionError).appendAll(planInterpreter.getErrors()));
+        }
+
+        // might be null
+        itemType = planInterpreter.getCatalogItemType();
+
+        // run again if ID has just been learned, to catch recursive 
definitions and possibly other mistakes (itemType inconsistency?)
+        if (!Objects.equal(id, planInterpreter.itemId)) {
+            planInterpreter.setId(id).resolve();
+            if (resolutionError == null && !planInterpreter.isResolved()) {
+                resolutionError = new IllegalStateException("Plan resolution 
for " + id + " breaks after id and itemType are set; is there a recursive 
reference or other type inconsistency?\n" + sourceYaml);
+            }
         }
         if (throwOnError && resolutionError!=null) {
             // if there was an error, throw it here
@@ -993,14 +996,14 @@ public class BasicBrooklynCatalog implements 
BrooklynCatalog {
                 // which normally has "type: " prefixed
                 sourcePlanYaml = planInterpreter.itemYaml;
             }
-            
 
+            Set<OsgiBundleWithUrl> searchBundles = 
MutableSet.<OsgiBundleWithUrl>of().putIfNotNull(containingBundle).putAll(libraryBundles);
             BasicRegisteredType type = createYetUnsavedRegisteredTypeInstance(
                     
BrooklynObjectType.of(planInterpreter.catalogItemType).getSpecType()!=null ? 
RegisteredTypeKind.SPEC
                             : 
planInterpreter.catalogItemType==CatalogItemType.BEAN ? RegisteredTypeKind.BEAN
                             : RegisteredTypeKind.UNRESOLVED,
                     symbolicName, version,
-                    containingBundle, libraryBundles, 
+                    containingBundle, searchBundles,
                     displayName, description, catalogIconUrl, 
catalogDeprecated, sourcePlanYaml, 
                     tags, aliases, catalogDisabled, superTypes, format);
 
@@ -1036,7 +1039,7 @@ public class BasicBrooklynCatalog implements 
BrooklynCatalog {
     private BasicRegisteredType createYetUnsavedRegisteredTypeInstance(
             RegisteredTypeKind kind,
             String symbolicName, String version,
-            ManagedBundle containingBundle, Collection<CatalogBundle> 
libraryBundles,
+            ManagedBundle containingBundle, Collection<OsgiBundleWithUrl> 
libraryBundles,
             String displayName, String description,
             String catalogIconUrl, final Boolean catalogDeprecated, String 
sourcePlanYaml, Set<Object> tags, List<String> aliases,
             Boolean catalogDisabled, MutableList<Object> superTypes, String 
format) {
@@ -1294,7 +1297,7 @@ public class BasicBrooklynCatalog implements 
BrooklynCatalog {
         final Map<?,?> item;
         final String itemYaml;
         final String format;
-        final CatalogBundle containingBundle;
+        final OsgiBundleWithUrl containingBundle;
         final Collection<CatalogBundle> libraryBundles;
         final List<CatalogItemDtoAbstract<?, ?>> itemsDefinedSoFar;
         RegisteredTypeLoadingContext constraint;
@@ -1307,7 +1310,7 @@ public class BasicBrooklynCatalog implements 
BrooklynCatalog {
         List<Exception> transformerErrors = MutableList.of();
 
         public PlanInterpreterInferringType(@Nullable String itemId, Object 
itemDefinitionParsedToStringOrMap, String itemYaml, @Nullable CatalogItemType 
optionalCiType, @Nullable String format,
-                                            CatalogBundle containingBundle, 
Collection<CatalogBundle> libraryBundles,
+                                            OsgiBundleWithUrl 
containingBundle, Collection<CatalogBundle> libraryBundles,
                                             RegisteredTypeLoadingContext 
constraint, List<CatalogItemDtoAbstract<?,?>> itemsDefinedSoFar) {
             // ID is useful to prevent recursive references (possibly only 
supported for entities?)
             this.itemId = itemId;
@@ -1348,7 +1351,7 @@ public class BasicBrooklynCatalog implements 
BrooklynCatalog {
             this.itemsDefinedSoFar = itemsDefinedSoFar;
         }
 
-        public PlanInterpreterInferringType resolve() {
+        public void resolve() {
             try {
                 currentlyResolvingType.set(Strings.isBlank(itemId) ? itemYaml 
: itemId);
 
@@ -1367,7 +1370,7 @@ public class BasicBrooklynCatalog implements 
BrooklynCatalog {
                         resolved = false;
                         attemptLegacySpecTransformersForVariousSpecTypes();
                     }
-                    return this;
+                    return;
                 }
 
                 // for now, these are the lowest-priority errors (reported 
after the others)
@@ -1382,7 +1385,7 @@ public class BasicBrooklynCatalog implements 
BrooklynCatalog {
                         planYaml = itemYaml;
                         resolved = true;
                     }
-                    return this;
+                    return;
                 }
 
                 // couldn't resolve it with the plan transformers; retry with 
legacy "spec" transformers.
@@ -1393,7 +1396,7 @@ public class BasicBrooklynCatalog implements 
BrooklynCatalog {
                     attemptLegacySpecTransformersForVariousSpecTypes();
                 }
 
-                return this;
+                return;
             } finally {
                 currentlyResolvingType.remove();
             }
@@ -1427,8 +1430,7 @@ public class BasicBrooklynCatalog implements 
BrooklynCatalog {
             try {
                 suspicionOfABean = false;
 
-                Set<? extends OsgiBundleWithUrl> searchBundles = 
MutableSet.copyOf(libraryBundles)
-                        .putIfNotNull(containingBundle);
+                Set<OsgiBundleWithUrl> searchBundles = 
MutableSet.<OsgiBundleWithUrl>of().putIfNotNull(containingBundle).putAll(libraryBundles);
                 BrooklynClassLoadingContext loader = new 
OsgiBrooklynClassLoadingContext(mgmt, null, searchBundles);
                 if (catalogItemType == null) {
                     // attempt to detect whether it is a bean
@@ -1988,7 +1990,7 @@ public class BasicBrooklynCatalog implements 
BrooklynCatalog {
             // try spec instantiation if we know the BO Type (no point 
otherwise)
             resultT = RegisteredTypes.copyResolved(RegisteredTypeKind.SPEC, 
typeToValidate);
             try {
-                resultO = 
((BasicBrooklynTypeRegistry)mgmt.getTypeRegistry()).createSpec(resultT, 
constraint, boType.getSpecType());
+                resultO = mgmt.getTypeRegistry().createSpec(resultT, 
constraint, boType.getSpecType());
             } catch (Exception e) {
                 Exceptions.propagateIfFatal(e);
                 specError = e;
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/mgmt/classloading/BrooklynClassLoadingContextSequential.java
 
b/core/src/main/java/org/apache/brooklyn/core/mgmt/classloading/BrooklynClassLoadingContextSequential.java
index ddd5c5b364..8e7d29a3a8 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/mgmt/classloading/BrooklynClassLoadingContextSequential.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/mgmt/classloading/BrooklynClassLoadingContextSequential.java
@@ -18,12 +18,10 @@
  */
 package org.apache.brooklyn.core.mgmt.classloading;
 
-import java.net.URL;
-import java.util.List;
-import java.util.Set;
-
+import com.google.common.base.Objects;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
 import org.apache.brooklyn.api.mgmt.classloading.BrooklynClassLoadingContext;
+import org.apache.brooklyn.api.typereg.OsgiBundleWithUrl;
 import org.apache.brooklyn.util.collections.MutableList;
 import org.apache.brooklyn.util.collections.MutableSet;
 import org.apache.brooklyn.util.exceptions.Exceptions;
@@ -31,7 +29,10 @@ import org.apache.brooklyn.util.guava.Maybe;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.google.common.base.Objects;
+import java.net.URL;
+import java.util.Collection;
+import java.util.List;
+import java.util.Set;
 
 public final class BrooklynClassLoadingContextSequential extends 
AbstractBrooklynClassLoadingContext {
 
@@ -46,7 +47,15 @@ public final class BrooklynClassLoadingContextSequential 
extends AbstractBrookly
         for (BrooklynClassLoadingContext target: targets)
             add(target);
     }
-    
+
+    @Override
+    public Collection<? extends OsgiBundleWithUrl> getBundles() {
+        MutableSet<OsgiBundleWithUrl> result = MutableSet.of();
+        primaries.forEach(c -> result.addAll(c.getBundles()));
+        secondaries.forEach(c -> result.addAll(c.getBundles()));
+        return result;
+    }
+
     public void add(BrooklynClassLoadingContext target) {
         if (target instanceof BrooklynClassLoadingContextSequential) {
             for (BrooklynClassLoadingContext targetN: 
((BrooklynClassLoadingContextSequential)target).primaries )
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/mgmt/classloading/JavaBrooklynClassLoadingContext.java
 
b/core/src/main/java/org/apache/brooklyn/core/mgmt/classloading/JavaBrooklynClassLoadingContext.java
index 17c1573c15..1e82705681 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/mgmt/classloading/JavaBrooklynClassLoadingContext.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/mgmt/classloading/JavaBrooklynClassLoadingContext.java
@@ -22,8 +22,11 @@ import static 
com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
 
 import java.net.URL;
+import java.util.Collection;
+import java.util.Collections;
 
 import org.apache.brooklyn.api.mgmt.ManagementContext;
+import org.apache.brooklyn.api.typereg.OsgiBundleWithUrl;
 import org.apache.brooklyn.core.mgmt.persist.DeserializingClassRenamesProvider;
 import org.apache.brooklyn.util.core.ClassLoaderUtils;
 import org.apache.brooklyn.util.exceptions.Exceptions;
@@ -48,7 +51,12 @@ public class JavaBrooklynClassLoadingContext extends 
AbstractBrooklynClassLoadin
     public static JavaBrooklynClassLoadingContext create(ClassLoader loader) {
         return new JavaBrooklynClassLoadingContext(null, checkNotNull(loader, 
"loader"));
     }
-    
+
+    @Override
+    public Collection<? extends OsgiBundleWithUrl> getBundles() {
+        return Collections.emptySet();
+    }
+
     /**
      * At least one of mgmt or loader must not be null.
      * @deprecated since 0.7.0 only for legacy catalog items which provide a 
non-osgi loader; see {@link #newDefault(ManagementContext)}
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/mgmt/classloading/OsgiBrooklynClassLoadingContext.java
 
b/core/src/main/java/org/apache/brooklyn/core/mgmt/classloading/OsgiBrooklynClassLoadingContext.java
index c410f9e511..46a83e58bf 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/mgmt/classloading/OsgiBrooklynClassLoadingContext.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/mgmt/classloading/OsgiBrooklynClassLoadingContext.java
@@ -30,9 +30,11 @@ import org.apache.brooklyn.core.entity.EntityInternal;
 import org.apache.brooklyn.core.mgmt.entitlement.Entitlements;
 import org.apache.brooklyn.core.mgmt.ha.OsgiManager;
 import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
+import org.apache.brooklyn.util.collections.MutableList;
 import org.apache.brooklyn.util.guava.Maybe;
 
 import com.google.common.base.Objects;
+import org.apache.brooklyn.util.osgi.VersionedName;
 
 public class OsgiBrooklynClassLoadingContext extends 
AbstractBrooklynClassLoadingContext {
 
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/resolve/entity/CatalogEntitySpecResolver.java
 
b/core/src/main/java/org/apache/brooklyn/core/resolve/entity/CatalogEntitySpecResolver.java
index e04be5fb4d..8cff6a1899 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/resolve/entity/CatalogEntitySpecResolver.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/resolve/entity/CatalogEntitySpecResolver.java
@@ -62,7 +62,7 @@ public class CatalogEntitySpecResolver extends 
AbstractEntitySpecResolver {
     @Override
     protected boolean canResolve(String type, BrooklynClassLoadingContext 
loader) {
         String localType = getLocalType(type);
-        RegisteredType item = mgmt.getTypeRegistry().get(localType);
+        RegisteredType item = mgmt.getTypeRegistry().get(localType, 
RegisteredTypeLoadingContexts.loader(loader));
         if (item==null) {
             item = loadUpgrade(localType);
         }
@@ -78,7 +78,7 @@ public class CatalogEntitySpecResolver extends 
AbstractEntitySpecResolver {
     @Override
     public EntitySpec<?> resolve(String type, BrooklynClassLoadingContext 
loader, Set<String> parentEncounteredTypes) {
         String localType = getLocalType(type);
-        RegisteredType item = mgmt.getTypeRegistry().get(localType, 
RegisteredTypeLoadingContexts.spec(Entity.class));
+        RegisteredType item = mgmt.getTypeRegistry().get(localType, 
RegisteredTypeLoadingContexts.withLoader(RegisteredTypeLoadingContexts.spec(Entity.class),
 loader));
         boolean upgradeRequired = item==null;
         
         if (upgradeRequired) {
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BrooklynRegisteredTypeJacksonSerialization.java
 
b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BrooklynRegisteredTypeJacksonSerialization.java
index 5bd425a57c..31e0cab4c7 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BrooklynRegisteredTypeJacksonSerialization.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BrooklynRegisteredTypeJacksonSerialization.java
@@ -45,6 +45,7 @@ import 
org.apache.brooklyn.core.mgmt.internal.LocalManagementContext;
 import 
org.apache.brooklyn.core.resolve.jackson.AsPropertyIfAmbiguous.AsPropertyButNotIfFieldConflictTypeDeserializer;
 import 
org.apache.brooklyn.core.resolve.jackson.AsPropertyIfAmbiguous.AsPropertyIfAmbiguousTypeSerializer;
 import 
org.apache.brooklyn.core.resolve.jackson.AsPropertyIfAmbiguous.HasBaseType;
+import org.apache.brooklyn.core.typereg.RegisteredTypeLoadingContexts;
 import org.apache.brooklyn.util.core.flags.BrooklynTypeNameResolution;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 
@@ -124,7 +125,7 @@ public class BrooklynRegisteredTypeJacksonSerialization {
             }
 
             if (allowRegisteredTypes && mgmt!=null) {
-                RegisteredType rt = mgmt.getTypeRegistry().get(id);
+                RegisteredType rt = mgmt.getTypeRegistry().get(id, 
RegisteredTypeLoadingContexts.loader(loader));
                 if (rt != null) {
                     return new BrooklynJacksonType(rt);
                 }
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java
 
b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java
index 766841775a..2cefe89a98 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java
@@ -207,7 +207,7 @@ public class BasicBrooklynTypeRegistry implements 
BrooklynTypeRegistry {
         }
         
         if (!Iterables.isEmpty(types)) {
-            RegisteredType type = RegisteredTypes.getBestVersion(types);
+            RegisteredType type = 
RegisteredTypes.getBestVersionInContext(types, context);
             if (type!=null) return Maybe.of(type);
         }
         
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java 
b/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java
index f8fec6e810..5a416b84c1 100644
--- a/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java
+++ b/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java
@@ -52,6 +52,7 @@ import org.apache.brooklyn.core.mgmt.BrooklynTags;
 import org.apache.brooklyn.core.mgmt.BrooklynTags.NamedStringTag;
 import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
 import 
org.apache.brooklyn.core.mgmt.classloading.JavaBrooklynClassLoadingContext;
+import 
org.apache.brooklyn.core.mgmt.classloading.OsgiBrooklynClassLoadingContext;
 import org.apache.brooklyn.core.objs.BrooklynObjectInternal;
 import 
org.apache.brooklyn.core.typereg.JavaClassNameTypePlanTransformer.JavaClassNameTypeImplementationPlan;
 import org.apache.brooklyn.util.collections.Jsonya;
@@ -59,6 +60,7 @@ import org.apache.brooklyn.util.core.task.Tasks;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.guava.Maybe;
 import org.apache.brooklyn.util.guava.Maybe.Absent;
+import org.apache.brooklyn.util.osgi.VersionedName;
 import org.apache.brooklyn.util.stream.Streams;
 import org.apache.brooklyn.util.text.NaturalOrderComparator;
 import org.apache.brooklyn.util.text.Strings;
@@ -518,6 +520,27 @@ public class RegisteredTypes {
         if (types==null || !types.iterator().hasNext()) return null;
         return 
Ordering.from(RegisteredTypeNameThenBestFirstComparator.INSTANCE).min(types);
     }
+
+    public static RegisteredType 
getBestVersionInContext(Iterable<RegisteredType> types, 
RegisteredTypeLoadingContext context) {
+        if (types==null) return null;
+        Iterator<RegisteredType> ti = types.iterator();
+        if (!ti.hasNext()) return null;
+        RegisteredType first = ti.next();
+        if (!ti.hasNext()) return first;  //only
+
+        Collection<? extends OsgiBundleWithUrl> preferredBundles = 
context.getLoader().getBundles();
+        for (OsgiBundleWithUrl b: preferredBundles) {
+            Iterable<RegisteredType> typesInBundle = Iterables.filter(types, t 
-> Objects.equal(
+                    VersionedName.fromString(t.getContainingBundle()), 
b.getVersionedName()));
+            if (typesInBundle.iterator().hasNext()) {
+                if (log.isTraceEnabled()) log.trace("Preferring 
"+typesInBundle+" from "+types+" because its bundle is in the explicit loader 
list");
+                types = typesInBundle;
+                break;
+            }
+        }
+
+        return getBestVersion(types);
+    }
     
     /** by name, then with disabled, deprecated first, then by increasing 
version */ 
     public static class RegisteredTypeNameThenWorstFirstComparator implements 
Comparator<RegisteredType> {


Reply via email to