Author: radu Date: Tue Nov 8 18:30:46 2016 New Revision: 1768759 URL: http://svn.apache.org/viewvc?rev=1768759&view=rev Log: SLING-6258 - The PackageAdminClassLoader cannot load classes from bundles providing older API versions
* check all bundles providing the same API package when trying to solve a class / resource and return the first non-null result; this will always return the class / resource with the highest API version Modified: sling/trunk/bundles/commons/classloader/src/main/java/org/apache/sling/commons/classloader/impl/PackageAdminClassLoader.java sling/trunk/bundles/commons/classloader/src/test/java/org/apache/sling/commons/classloader/impl/ClassLoadingTest.java Modified: sling/trunk/bundles/commons/classloader/src/main/java/org/apache/sling/commons/classloader/impl/PackageAdminClassLoader.java URL: http://svn.apache.org/viewvc/sling/trunk/bundles/commons/classloader/src/main/java/org/apache/sling/commons/classloader/impl/PackageAdminClassLoader.java?rev=1768759&r1=1768758&r2=1768759&view=diff ============================================================================== --- sling/trunk/bundles/commons/classloader/src/main/java/org/apache/sling/commons/classloader/impl/PackageAdminClassLoader.java (original) +++ sling/trunk/bundles/commons/classloader/src/main/java/org/apache/sling/commons/classloader/impl/PackageAdminClassLoader.java Tue Nov 8 18:30:46 2016 @@ -98,16 +98,20 @@ class PackageAdminClassLoader extends Cl * @param pckName The package name. * @return The bundle or <code>null</code> */ - private Bundle findBundleForPackage(final String pckName) { - final ExportedPackage exportedPackage = this.packageAdmin.getExportedPackage(pckName); - Bundle bundle = null; - if (exportedPackage != null && !exportedPackage.isRemovalPending() ) { - bundle = exportedPackage.getExportingBundle(); - if ( !this.isBundleActive(bundle) ) { - bundle = null; + private Set<Bundle> findBundlesForPackage(final String pckName) { + final ExportedPackage[] exportedPackages = this.packageAdmin.getExportedPackages(pckName); + Set<Bundle> bundles = new HashSet<>(); + if (exportedPackages != null) { + for (ExportedPackage exportedPackage : exportedPackages) { + if (!exportedPackage.isRemovalPending()) { + Bundle bundle = exportedPackage.getExportingBundle(); + if (isBundleActive(bundle)) { + bundles.add(bundle); + } + } } } - return bundle; + return bundles; } /** @@ -123,7 +127,7 @@ class PackageAdminClassLoader extends Cl /** * Return the package from a class. - * @param resource The class name. + * @param name The class name. * @return The package name. */ private String getPackageFromClassName(final String name) { @@ -139,9 +143,11 @@ class PackageAdminClassLoader extends Cl public Enumeration<URL> getResources(final String name) throws IOException { Enumeration<URL> e = super.getResources(name); if ( e == null || !e.hasMoreElements() ) { - final Bundle bundle = this.findBundleForPackage(getPackageFromResource(name)); - if ( bundle != null ) { + for (Bundle bundle : findBundlesForPackage(getPackageFromResource(name))) { e = bundle.getResources(name); + if (e != null) { + return e; + } } } return e; @@ -157,11 +163,12 @@ class PackageAdminClassLoader extends Cl } URL url = super.findResource(name); if ( url == null ) { - final Bundle bundle = this.findBundleForPackage(getPackageFromResource(name)); - if ( bundle != null ) { + Set<Bundle> bundles = findBundlesForPackage(getPackageFromResource(name)); + for (Bundle bundle : bundles) { url = bundle.getResource(name); if ( url != null ) { urlCache.put(name, url); + return url; } } } @@ -180,10 +187,15 @@ class PackageAdminClassLoader extends Cl try { clazz = super.findClass(name); } catch (ClassNotFoundException cnfe) { - final Bundle bundle = this.findBundleForPackage(getPackageFromClassName(name)); - if ( bundle != null ) { - clazz = bundle.loadClass(name); - this.factory.addUsedBundle(bundle); + Set<Bundle> bundles = findBundlesForPackage(getPackageFromClassName(name)); + for (Bundle bundle : bundles) { + try { + clazz = bundle.loadClass(name); + this.factory.addUsedBundle(bundle); + break; + } catch (ClassNotFoundException icnfe) { + // do nothing; we need to loop over the bundles providing the class' package + } } } if ( clazz == null ) { @@ -209,15 +221,14 @@ class PackageAdminClassLoader extends Cl clazz = super.loadClass(name, resolve); } catch (final ClassNotFoundException cnfe) { final String pckName = getPackageFromClassName(name); - final Bundle bundle = this.findBundleForPackage(pckName); - if ( bundle != null ) { + Set<Bundle> bundles = findBundlesForPackage(pckName); + for (Bundle bundle : bundles) { try { clazz = bundle.loadClass(name); this.factory.addUsedBundle(bundle); + break; } catch (final ClassNotFoundException inner) { - negativeClassCache.add(name); - this.factory.addUnresolvedPackage(pckName); - throw inner; + // do nothing; we need to loop over the bundles providing the class' package } } } Modified: sling/trunk/bundles/commons/classloader/src/test/java/org/apache/sling/commons/classloader/impl/ClassLoadingTest.java URL: http://svn.apache.org/viewvc/sling/trunk/bundles/commons/classloader/src/test/java/org/apache/sling/commons/classloader/impl/ClassLoadingTest.java?rev=1768759&r1=1768758&r2=1768759&view=diff ============================================================================== --- sling/trunk/bundles/commons/classloader/src/test/java/org/apache/sling/commons/classloader/impl/ClassLoadingTest.java (original) +++ sling/trunk/bundles/commons/classloader/src/test/java/org/apache/sling/commons/classloader/impl/ClassLoadingTest.java Tue Nov 8 18:30:46 2016 @@ -16,6 +16,8 @@ */ package org.apache.sling.commons.classloader.impl; +import java.util.ArrayList; + import org.jmock.Expectations; import org.jmock.Mockery; import org.jmock.Sequence; @@ -56,8 +58,8 @@ public class ClassLoadingTest { will(returnValue(null)); allowing(bundleContext).addServiceListener(with(any(ServiceListener.class)), with(any(String.class))); allowing(bundleContext).removeServiceListener(with(any(ServiceListener.class))); - allowing(packageAdmin).getExportedPackage("org.apache.sling.test"); - will(returnValue(ep)); + allowing(packageAdmin).getExportedPackages("org.apache.sling.test"); + will(returnValue(new ExportedPackage[] {ep})); allowing(ep).getExportingBundle(); will(returnValue(bundle)); allowing(ep).isRemovalPending(); @@ -84,4 +86,56 @@ public class ClassLoadingTest { final Class<?> c3 = cl.loadClass("org.apache.sling.test.A"); Assert.assertEquals("java.util.Map", c3.getName()); } + + @Test public void testLoading_SLING_6258() throws Exception { + final BundleContext bundleContext = this.context.mock(BundleContext.class); + final PackageAdmin packageAdmin = this.context.mock(PackageAdmin.class); + final ExportedPackage ep1 = this.context.mock(ExportedPackage.class, "ep1"); + final ExportedPackage ep2 = this.context.mock(ExportedPackage.class, "ep2"); + final Bundle bundle1 = this.context.mock(Bundle.class, "bundle1"); + final Bundle bundle2 = this.context.mock(Bundle.class, "bundle2"); + final ClassNotFoundException cnfe = new ClassNotFoundException(); + this.context.checking(new Expectations() {{ + allowing(bundleContext).createFilter(with(any(String.class))); + will(returnValue(null)); + allowing(bundleContext).getServiceReferences(with(any(String.class)), with((String)null)); + will(returnValue(null)); + allowing(bundleContext).addServiceListener(with(any(ServiceListener.class)), with(any(String.class))); + allowing(bundleContext).removeServiceListener(with(any(ServiceListener.class))); + allowing(packageAdmin).getExportedPackages("org.apache.sling.test"); + will(returnValue(new ExportedPackage[] {ep1, ep2})); + + allowing(ep1).getExportingBundle(); + will(returnValue(bundle1)); + allowing(ep1).isRemovalPending(); + will(returnValue(false)); + + allowing(ep2).getExportingBundle(); + will(returnValue(bundle2)); + allowing(ep2).isRemovalPending(); + will(returnValue(false)); + + allowing(bundle1).getBundleId(); + will(returnValue(2L)); + allowing(bundle1).getState(); + will(returnValue(Bundle.ACTIVE)); + + + allowing(bundle2).getBundleId(); + will(returnValue(3L)); + allowing(bundle2).getState(); + will(returnValue(Bundle.ACTIVE)); + + allowing(bundle1).loadClass("org.apache.sling.test.T1"); + will(throwException(cnfe)); + + allowing(bundle2).loadClass("org.apache.sling.test.T1"); + will(returnValue(ArrayList.class)); + + }}); + DynamicClassLoaderManagerImpl manager = new DynamicClassLoaderManagerImpl(bundleContext, packageAdmin, null, + new DynamicClassLoaderManagerFactory(bundleContext, packageAdmin)); + final ClassLoader cl = manager.getDynamicClassLoader(); + Assert.assertEquals(ArrayList.class, cl.loadClass("org.apache.sling.test.T1")); + } }