Change classloading order in ClassLoaderUtils
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/2654305d Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/2654305d Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/2654305d Branch: refs/heads/master Commit: 2654305d4f4bc5f9d1a45bffb363996c4eb6e6fc Parents: a555bea Author: Aled Sage <[email protected]> Authored: Thu Jul 7 14:34:04 2016 +0100 Committer: Aled Sage <[email protected]> Committed: Sat Jul 9 10:58:36 2016 +0100 ---------------------------------------------------------------------- .../brooklyn/camp/brooklyn/RebindOsgiTest.java | 63 ++++++++++++++++++++ .../brooklyn/util/core/ClassLoaderUtils.java | 63 +++++++++++++++++--- .../mgmt/persist/XmlMementoSerializerTest.java | 8 +++ .../util/core/ClassLoaderUtilsTest.java | 20 +++++++ 4 files changed, 145 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2654305d/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/RebindOsgiTest.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/RebindOsgiTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/RebindOsgiTest.java index 752b066..46387da 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/RebindOsgiTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/RebindOsgiTest.java @@ -37,6 +37,7 @@ import org.apache.brooklyn.core.mgmt.ha.OsgiManager; import org.apache.brooklyn.core.mgmt.internal.LocalManagementContext; import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; import org.apache.brooklyn.core.mgmt.osgi.OsgiStandaloneTest; +import org.apache.brooklyn.core.mgmt.osgi.OsgiVersionMoreEntityTest; import org.apache.brooklyn.core.sensor.Sensors; import org.apache.brooklyn.core.test.entity.TestEntity; import org.apache.brooklyn.util.core.osgi.Osgis; @@ -321,6 +322,62 @@ public class RebindOsgiTest extends AbstractYamlRebindTest { assertEquals(entity2.getCatalogItemId(), appSymbolicName+":"+appVersion); } + /** + * Installs a newer version of the bundle than that used in the catalog. Then creates an + * app with that catalog item, and rebinds. Confirms that we use our explicit version, rather + * that the newer version available in the OSGi container. + */ + @Test + public void testUsesCatalogBundleVersion() throws Exception { + String bundleV1Url = OsgiVersionMoreEntityTest.BROOKLYN_TEST_MORE_ENTITIES_V1_URL; + String bundleV2Url = OsgiVersionMoreEntityTest.BROOKLYN_TEST_MORE_ENTITIES_V2_URL; + String bundleEntityType = "org.apache.brooklyn.test.osgi.entities.more.MoreEntity"; + String bundleObjectType = "org.apache.brooklyn.test.osgi.entities.more.MoreObject"; + String v1Version = "0.1.0"; + + installBundle(mgmt(), bundleV2Url); + bundleUrlsToInstallOnRebind.add(bundleV2Url); + + String appSymbolicName = "my.catalog.app.id.load"; + String appVersion = "0.1.0"; + String appCatalogFormat = Joiner.on("\n").join( + "brooklyn.catalog:", + " id: " + appSymbolicName, + " version: " + appVersion, + " itemType: entity", + " libraries:", + " - " + bundleV1Url, + " item:", + " type: " + bundleEntityType, + " brooklyn.config:", + " my.conf:", + " $brooklyn:object:", + " type: " + bundleObjectType, + " object.fields:", + " val: myEntityVal"); + + Iterables.getOnlyElement(addCatalogItems(String.format(appCatalogFormat, appVersion))); + + String appBlueprintYaml = Joiner.on("\n").join( + "location: localhost\n", + "services:", + "- type: " + CatalogUtils.getVersionedId(appSymbolicName, appVersion)); + origApp = (StartableApplication) createAndStartApplication(appBlueprintYaml); + Entity origEntity = Iterables.getOnlyElement(origApp.getChildren()); + Object configVal = origEntity.config().get(ConfigKeys.newConfigKey(Object.class, "my.conf")); + assertBundleVersionOf(Entities.deproxy(origEntity), v1Version); + assertBundleVersionOf(configVal, v1Version); + + // Rebind + rebind(); + + // Ensure entity/config is loaded from the explicit catalog version + Entity newEntity = Iterables.getOnlyElement(newApp.getChildren()); + Object newConfigVal = newEntity.config().get(ConfigKeys.newConfigKey(Object.class, "my.conf")); + assertBundleVersionOf(Entities.deproxy(newEntity), v1Version); + assertBundleVersionOf(newConfigVal, v1Version); + } + // TODO Does not do rebind; the config isn't there after rebind. // Need to reproduce that in a simpler use-case. @Test @@ -395,4 +452,10 @@ public class RebindOsgiTest extends AbstractYamlRebindTest { Framework framework = osgiManager.getFramework(); return Osgis.install(framework, bundleUrl); } + + protected void assertBundleVersionOf(Object obj, String expectedVersion) { + assertNotNull(obj); + Class<?> clazz = (obj instanceof Class) ? (Class<?>)obj : obj.getClass(); + assertEquals(Osgis.getBundleOf(clazz).get().getVersion().toString(), expectedVersion); + } } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2654305d/core/src/main/java/org/apache/brooklyn/util/core/ClassLoaderUtils.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/util/core/ClassLoaderUtils.java b/core/src/main/java/org/apache/brooklyn/util/core/ClassLoaderUtils.java index bc46b60..b46f68d 100644 --- a/core/src/main/java/org/apache/brooklyn/util/core/ClassLoaderUtils.java +++ b/core/src/main/java/org/apache/brooklyn/util/core/ClassLoaderUtils.java @@ -101,12 +101,44 @@ public class ClassLoaderUtils { this.mgmt = checkNotNull(mgmt, "mgmt"); } + /** + * Loads the given class, handle OSGi bundles. The class could be in one of the following formats: + * <ul> + * <li>{@code <classname>}, such as {@code com.google.common.net.HostAndPort} + * <li>{@code <bunde-symbolicName>:<classname>}, such as {@code com.google.guava:com.google.common.net.HostAndPort} + * <li>{@code <bunde-symbolicName>:<bundle-version>:<classname>}, such as {@code com.google.guava:16.0.1:com.google.common.net.HostAndPort} + * </ul> + * + * The classloading order is as follows: + * <ol> + * <li>If the class explicitly states the bundle name and version, then load from that. + * <li>Otherwise try to load from the catalog's classloader. This is so we respect any + * {@code libraries} supplied in the catalog metadata, and can thus handle updating + * catalog versions. It also means we can try our best to handle a catalog that + * uses a different bundle version from something that ships with Brooklyn. + * <li>The white-listed bundles (i.e. those that ship with Brooklyn). We prefer the + * version of the bundle that Brooklyn depends on, rather than taking the highest + * version installed (e.g. Karaf has Guava 16.0.1 and 18.0; we want the former, which + * Brooklyn uses). + * <li>The classloader passed in. Normally this is a boring unhelpful classloader (e.g. + * obtained from {@code callingClass.getClassLoader()}), so won't work. But it's up + * to the caller if they pass in something more useful. + * <li>The {@link ManagementContext#getCatalogClassLoader()}. Again, this is normally not helpful. + * We instead would prefer the specific catalog item's classloader (which we tried earlier). + * <li>If we were given a bundle name without a version, then finally try just using the + * most recent version of the bundle that is available in the OSGi container. + * </ol> + * + * The list of "white-listed bundles" are controlled using the system property named + * {@link #WHITE_LIST_KEY}, defaulting to all {@code org.apache.brooklyn.*} bundles. + */ public Class<?> loadClass(String name) throws ClassNotFoundException { + String symbolicName; + String version; + String className; + if (looksLikeBundledClassName(name)) { String[] arr = name.split(CLASS_NAME_DELIMITER); - String symbolicName; - String version; - String className; if (arr.length > 3) { throw new IllegalStateException("'" + name + "' doesn't look like a class name and contains too many colons to be parsed as bundle:version:class triple."); } else if (arr.length == 3) { @@ -120,9 +152,17 @@ public class ClassLoaderUtils { } else { throw new IllegalStateException("'" + name + "' contains a bundle:version:class delimiter, but only one of those specified"); } - return loadClass(symbolicName, version, className); + } else { + symbolicName = null; + version = null; + className = name; } + if (symbolicName != null && version != null) { + // Very explicit; do as we're told! + return loadClass(symbolicName, version, className); + } + if (entity != null && mgmt != null) { String catalogItemId = entity.getCatalogItemId(); if (catalogItemId != null) { @@ -130,7 +170,7 @@ public class ClassLoaderUtils { if (item != null) { BrooklynClassLoadingContext loader = CatalogUtils.newClassLoadingContext(mgmt, item); try { - return loader.loadClass(name); + return loader.loadClass(className); } catch (IllegalStateException e) { ClassNotFoundException cnfe = Exceptions.getFirstThrowableOfType(e, ClassNotFoundException.class); NoClassDefFoundError ncdfe = Exceptions.getFirstThrowableOfType(e, NoClassDefFoundError.class); @@ -146,11 +186,16 @@ public class ClassLoaderUtils { } } + Class<?> cls = tryLoadFromBundleWhiteList(name); + if (cls != null) { + return cls; + } + try { // Used instead of callingClass.getClassLoader().loadClass(...) as it could be null (only for bootstrap classes) // Note that Class.forName(name, false, classLoader) doesn't seem to like us returning a // class with a different name from that intended (e.g. stripping off an OSGi prefix). - return classLoader.loadClass(name); + return classLoader.loadClass(className); } catch (ClassNotFoundException e) { } @@ -161,9 +206,9 @@ public class ClassLoaderUtils { } } - Class<?> cls = tryLoadFromBundleWhiteList(name); - if (cls != null) { - return cls; + if (symbolicName != null) { + // Finally fall back to loading from any version of the bundle + return loadClass(symbolicName, version, className); } else { throw new ClassNotFoundException("Class " + name + " not found on the application class path, nor in the bundle white list."); } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2654305d/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/XmlMementoSerializerTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/XmlMementoSerializerTest.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/XmlMementoSerializerTest.java index 9251354..e26983b 100644 --- a/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/XmlMementoSerializerTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/XmlMementoSerializerTest.java @@ -43,11 +43,13 @@ import org.apache.brooklyn.api.mgmt.rebind.mementos.BrooklynMementoPersister.Loo import org.apache.brooklyn.api.objs.BrooklynObject; import org.apache.brooklyn.api.objs.BrooklynObjectType; import org.apache.brooklyn.api.policy.Policy; +import org.apache.brooklyn.api.sensor.AttributeSensor; import org.apache.brooklyn.api.sensor.Enricher; import org.apache.brooklyn.api.sensor.Feed; import org.apache.brooklyn.api.typereg.RegisteredType; import org.apache.brooklyn.core.catalog.internal.CatalogItemBuilder; import org.apache.brooklyn.core.catalog.internal.CatalogItemDtoAbstract; +import org.apache.brooklyn.core.entity.Attributes; import org.apache.brooklyn.core.entity.Entities; import org.apache.brooklyn.core.location.SimulatedLocation; import org.apache.brooklyn.core.mgmt.ha.OsgiManager; @@ -263,6 +265,12 @@ public class XmlMementoSerializerTest { } @Test + public void testSensorWithTypeToken() throws Exception { + AttributeSensor<Map<String, Object>> obj = Attributes.SERVICE_NOT_UP_DIAGNOSTICS; + assertSerializeAndDeserialize(obj); + } + + @Test public void testClass() throws Exception { Class<?> t = XmlMementoSerializer.class; assertSerializeAndDeserialize(t); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2654305d/core/src/test/java/org/apache/brooklyn/util/core/ClassLoaderUtilsTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/util/core/ClassLoaderUtilsTest.java b/core/src/test/java/org/apache/brooklyn/util/core/ClassLoaderUtilsTest.java index 3f2a8e1..98a93ef 100644 --- a/core/src/test/java/org/apache/brooklyn/util/core/ClassLoaderUtilsTest.java +++ b/core/src/test/java/org/apache/brooklyn/util/core/ClassLoaderUtilsTest.java @@ -41,6 +41,8 @@ import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; +import com.google.common.collect.ImmutableList; + public class ClassLoaderUtilsTest { private LocalManagementContext mgmt; @@ -144,6 +146,24 @@ public class ClassLoaderUtilsTest { assertFalse(clu.isBundleWhiteListed(getBundle(mgmt, "com.google.guava"))); } + /** + * When two guava versions installed, want us to load from the *brooklyn* version rather than + * a newer version that happens to be in Karaf. + */ + @Test(groups={"Integration"}) + public void testLoadsFromRightGuavaVersion() throws Exception { + mgmt = LocalManagementContextForTests.builder(true).disableOsgi(false).build(); + ClassLoaderUtils clu = new ClassLoaderUtils(getClass(), mgmt); + + String bundleUrl = "http://search.maven.org/remotecontent?filepath=com/google/guava/guava/18.0/guava-18.0.jar";; + Bundle bundle = installBundle(mgmt, bundleUrl); + String bundleName = bundle.getSymbolicName(); + + String classname = bundleName + ":" + ImmutableList.class.getName(); + Class<?> clazz = clu.loadClass(classname); + assertEquals(clazz, ImmutableList.class); + } + private Bundle installBundle(ManagementContext mgmt, String bundleUrl) throws Exception { OsgiManager osgiManager = ((ManagementContextInternal)mgmt).getOsgiManager().get(); Framework framework = osgiManager.getFramework();
