You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ra...@apache.org on 2016/11/08 18:30:46 UTC

svn commit: r1768759 - in /sling/trunk/bundles/commons/classloader/src: main/java/org/apache/sling/commons/classloader/impl/PackageAdminClassLoader.java test/java/org/apache/sling/commons/classloader/impl/ClassLoadingTest.java

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"));
+    }
 }