Author: radu
Date: Thu Nov 10 14:19:08 2016
New Revision: 1769127
URL: http://svn.apache.org/viewvc?rev=1769127&view=rev
Log:
SLING-6258 - The PackageAdminClassLoader cannot load classes from bundles
providing older API versions
* made sure to lock-down the bundle which provides first a resource / class
from a requested package so
that subsequent calls for resolving classes / resources from the same package
will always use the same
bundle
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=1769127&r1=1769126&r2=1769127&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
Thu Nov 10 14:19:08 2016
@@ -23,6 +23,7 @@ import java.net.URL;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashSet;
+import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
@@ -31,6 +32,8 @@ import org.osgi.framework.Bundle;
import org.osgi.framework.Constants;
import org.osgi.service.packageadmin.ExportedPackage;
import org.osgi.service.packageadmin.PackageAdmin;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* The <code>PackageAdminClassLoader</code> loads
@@ -38,6 +41,8 @@ import org.osgi.service.packageadmin.Pac
*/
class PackageAdminClassLoader extends ClassLoader {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(PackageAdminClassLoader.class);
+
/** The package admin service. */
private final PackageAdmin packageAdmin;
@@ -50,6 +55,8 @@ class PackageAdminClassLoader extends Cl
/** Negative class cache. */
private Set<String> negativeClassCache = Collections.synchronizedSet(new
HashSet<String>());
+ private Map<String, Bundle> packageProviders = new ConcurrentHashMap<>();
+
/** A cache for resolved urls. */
private Map<String, URL> urlCache = new ConcurrentHashMap<String, URL>();
@@ -100,7 +107,7 @@ class PackageAdminClassLoader extends Cl
*/
private Set<Bundle> findBundlesForPackage(final String pckName) {
final ExportedPackage[] exportedPackages =
this.packageAdmin.getExportedPackages(pckName);
- Set<Bundle> bundles = new HashSet<>();
+ Set<Bundle> bundles = new LinkedHashSet<>();
if (exportedPackages != null) {
for (ExportedPackage exportedPackage : exportedPackages) {
if (!exportedPackage.isRemovalPending()) {
@@ -143,10 +150,23 @@ class PackageAdminClassLoader extends Cl
public Enumeration<URL> getResources(final String name) throws IOException
{
Enumeration<URL> e = super.getResources(name);
if ( e == null || !e.hasMoreElements() ) {
- for (Bundle bundle :
findBundlesForPackage(getPackageFromResource(name))) {
- e = bundle.getResources(name);
- if (e != null) {
- return e;
+ String packageName = getPackageFromResource(name);
+ Bundle providingBundle = packageProviders.get(packageName);
+ if (providingBundle == null) {
+ for (Bundle bundle :
findBundlesForPackage(getPackageFromResource(name))) {
+ e = bundle.getResources(name);
+ if (e != null) {
+ packageProviders.put(packageName, bundle);
+ LOGGER.debug("Marking bundle {}:{} as the provider for
API package {}.", bundle.getSymbolicName(), bundle
+ .getVersion().toString(), packageName);
+ return e;
+ }
+ }
+ } else {
+ e = providingBundle.getResources(name);
+ if (e == null) {
+ LOGGER.debug("Cannot find resources {} in bundle {}:{}
which was marked as the provider for package {}.", name,
+ providingBundle.getSymbolicName(),
providingBundle.getVersion().toString(), packageName);
}
}
}
@@ -163,12 +183,25 @@ class PackageAdminClassLoader extends Cl
}
URL url = super.findResource(name);
if ( url == null ) {
- Set<Bundle> bundles =
findBundlesForPackage(getPackageFromResource(name));
- for (Bundle bundle : bundles) {
- url = bundle.getResource(name);
- if ( url != null ) {
- urlCache.put(name, url);
- return url;
+ String packageName = getPackageFromResource(name);
+ Bundle providingBundle = packageProviders.get(packageName);
+ if (providingBundle == null) {
+ Set<Bundle> bundles =
findBundlesForPackage(getPackageFromResource(name));
+ for (Bundle bundle : bundles) {
+ url = bundle.getResource(name);
+ if (url != null) {
+ urlCache.put(name, url);
+ packageProviders.put(packageName, bundle);
+ LOGGER.debug("Marking bundle {}:{} as the provider for
API package {}.", bundle.getSymbolicName(), bundle
+ .getVersion().toString(), packageName);
+ return url;
+ }
+ }
+ } else {
+ url = providingBundle.getResource(name);
+ if (url == null) {
+ LOGGER.debug("Cannot find resource {} in bundle {}:{}
which was marked as the provider for package {}.", name,
+ providingBundle.getSymbolicName(),
providingBundle.getVersion().toString(), packageName);
}
}
}
@@ -183,19 +216,14 @@ class PackageAdminClassLoader extends Cl
if ( cachedClass != null ) {
return cachedClass;
}
- Class<?> clazz = null;
+ Class<?> clazz;
try {
clazz = super.findClass(name);
} catch (ClassNotFoundException cnfe) {
- 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
- }
+ try {
+ clazz = getClassFromBundles(name);
+ } catch (ClassNotFoundException innerCNFE) {
+ throw innerCNFE;
}
}
if ( clazz == null ) {
@@ -216,29 +244,54 @@ class PackageAdminClassLoader extends Cl
if ( negativeClassCache.contains(name) ) {
throw new ClassNotFoundException("Class not found " + name);
}
- Class<?> clazz = null;
+ String packageName = getPackageFromClassName(name);
+ Class<?> clazz;
try {
clazz = super.loadClass(name, resolve);
} catch (final ClassNotFoundException cnfe) {
- final String pckName = getPackageFromClassName(name);
- Set<Bundle> bundles = findBundlesForPackage(pckName);
+ try {
+ clazz = getClassFromBundles(name);
+ } catch (ClassNotFoundException innerCNFE) {
+ negativeClassCache.add(name);
+ this.factory.addUnresolvedPackage(packageName);
+ throw innerCNFE;
+ }
+ }
+ if ( clazz == null ) {
+ negativeClassCache.add(name);
+ this.factory.addUnresolvedPackage(packageName);
+ throw new ClassNotFoundException("Class not found " + name);
+ }
+ this.classCache.put(name, clazz);
+ return clazz;
+ }
+
+ private Class<?> getClassFromBundles(String name) throws
ClassNotFoundException {
+ Class<?> clazz = null;
+ String packageName = getPackageFromClassName(name);
+ Bundle providingBundle = packageProviders.get(packageName);
+ if (providingBundle == null) {
+ Set<Bundle> bundles = findBundlesForPackage(packageName);
for (Bundle bundle : bundles) {
try {
clazz = bundle.loadClass(name);
this.factory.addUsedBundle(bundle);
+ packageProviders.put(packageName, bundle);
+ LOGGER.debug("Marking bundle {}:{} as the provider for API
package {}.", bundle.getSymbolicName(), bundle
+ .getVersion().toString(), packageName);
break;
- } catch (final ClassNotFoundException inner) {
+ } catch (ClassNotFoundException innerCNFE) {
// do nothing; we need to loop over the bundles providing
the class' package
}
}
+ } else {
+ try {
+ clazz = providingBundle.loadClass(name);
+ } catch (ClassNotFoundException icnfe) {
+ throw new ClassNotFoundException(String.format("Cannot find
class %s in bundle %s:%s which was marked as the provider for" +
+ " package %s.", name,
providingBundle.getSymbolicName(), providingBundle.getVersion().toString(),
packageName), icnfe);
+ }
}
- if ( clazz == null ) {
- negativeClassCache.add(name);
- final String pckName = getPackageFromClassName(name);
- this.factory.addUnresolvedPackage(pckName);
- throw new ClassNotFoundException("Class not found " + name);
- }
- this.classCache.put(name, clazz);
return clazz;
}
}
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=1769127&r1=1769126&r2=1769127&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
Thu Nov 10 14:19:08 2016
@@ -16,7 +16,11 @@
*/
package org.apache.sling.commons.classloader.impl;
+import java.net.URL;
import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Enumeration;
+import java.util.Map;
import org.jmock.Expectations;
import org.jmock.Mockery;
@@ -27,6 +31,7 @@ import org.junit.Test;
import org.osgi.framework.Bundle;
import org.osgi.framework.BundleContext;
import org.osgi.framework.ServiceListener;
+import org.osgi.framework.Version;
import org.osgi.service.packageadmin.ExportedPackage;
import org.osgi.service.packageadmin.PackageAdmin;
@@ -68,6 +73,10 @@ public class ClassLoadingTest {
will(returnValue(2L));
allowing(bundle).getState();
will(returnValue(Bundle.ACTIVE));
+ allowing(bundle).getSymbolicName();
+ will(returnValue("org.apache.sling.test"));
+ allowing(bundle).getVersion();
+ will(returnValue(new Version("1.0.0")));
one(bundle).loadClass("org.apache.sling.test.A");
inSequence(sequence);
will(returnValue(java.util.Map.class));
one(bundle).loadClass("org.apache.sling.test.A");
inSequence(sequence);
@@ -92,8 +101,10 @@ public class ClassLoadingTest {
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 ExportedPackage ep3 = this.context.mock(ExportedPackage.class,
"ep3");
final Bundle bundle1 = this.context.mock(Bundle.class, "bundle1");
final Bundle bundle2 = this.context.mock(Bundle.class, "bundle2");
+ final Bundle bundle3 = this.context.mock(Bundle.class, "bundle3");
final ClassNotFoundException cnfe = new ClassNotFoundException();
this.context.checking(new Expectations() {{
allowing(bundleContext).createFilter(with(any(String.class)));
@@ -102,8 +113,10 @@ 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).getExportedPackages("org.apache.sling.test");
+
allowing(packageAdmin).getExportedPackages(with("org.apache.sling.test"));
will(returnValue(new ExportedPackage[] {ep1, ep2}));
+
allowing(packageAdmin).getExportedPackages(with("org.apache.sling.test3"));
+ will(returnValue(new ExportedPackage[] {ep3}));
allowing(ep1).getExportingBundle();
will(returnValue(bundle1));
@@ -115,27 +128,81 @@ public class ClassLoadingTest {
allowing(ep2).isRemovalPending();
will(returnValue(false));
+ allowing(ep3).getExportingBundle();
+ will(returnValue(bundle3));
+ allowing(ep3).isRemovalPending();
+ will(returnValue(false));
+
allowing(bundle1).getBundleId();
- will(returnValue(2L));
+ will(returnValue(1L));
allowing(bundle1).getState();
will(returnValue(Bundle.ACTIVE));
+ allowing(bundle1).getVersion();
+ will(returnValue(new Version("1.0.0")));
+ allowing(bundle1).getSymbolicName();
+ will(returnValue("org.apache.sling.test1"));
allowing(bundle2).getBundleId();
- will(returnValue(3L));
+ will(returnValue(2L));
allowing(bundle2).getState();
will(returnValue(Bundle.ACTIVE));
+ allowing(bundle2).getVersion();
+ will(returnValue(new Version("2.0.0")));
+ allowing(bundle2).getSymbolicName();
+ will(returnValue("org.apache.sling.test2"));
+
+ allowing(bundle3).getBundleId();
+ will(returnValue(3L));
+ allowing(bundle3).getState();
+ will(returnValue(Bundle.ACTIVE));
+ allowing(bundle3).getVersion();
+ will(returnValue(new Version("1.2.3")));
+ allowing(bundle3).getSymbolicName();
+ will(returnValue("org.apache.sling.test3"));
allowing(bundle1).loadClass("org.apache.sling.test.T1");
will(throwException(cnfe));
+ allowing(bundle1).getResource("org/apache/sling/test/T3.class");
+ will(returnValue(new
URL("jar:file:/ws/org.apache.sling.test1.jar!/org/apache/sling/test/T3.class")));
allowing(bundle2).loadClass("org.apache.sling.test.T1");
will(returnValue(ArrayList.class));
+ allowing(bundle2).loadClass("org.apache.sling.test.T2");
+ will(throwException(new ClassNotFoundException()));
+ allowing(bundle2).getResource("org/apache/sling/test/T3.class");
+ will(returnValue(null));
+ allowing(bundle2).getResources("org/apache/sling/test/T3.class");
+ will(returnValue(null));
+ allowing(bundle3).getResource("org/apache/sling/test3/T4.class");
+ will(returnValue(new
URL("jar:file:/ws/org.apache.sling.test3.jar!/org/apache/sling/test3/T4.class")));
+ allowing(bundle3).getResources("org/apache/sling/test3/T4.class");
+ will(returnValue(Collections.enumeration(new ArrayList<URL>() {{
+ add(new
URL("jar:file:/ws/org.apache.sling.test3.jar!/org/apache/sling/test3/T4.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"));
+ try {
+ cl.loadClass("org.apache.sling.test.T2");
+ } catch (Exception e) {
+ Assert.assertEquals(ClassNotFoundException.class, e.getClass());
+ }
+ Assert.assertNull(cl.getResource("org/apache/sling/test/T3.class"));
+
Assert.assertFalse(cl.getResources("org/apache/sling/test/T3.class").hasMoreElements());
+
Assert.assertEquals("jar:file:/ws/org.apache.sling.test3.jar!/org/apache/sling/test3/T4.class",
cl.getResource
+ ("org/apache/sling/test3/T4.class").toString());
+ Enumeration<URL> resourceURLs =
cl.getResources("org/apache/sling/test3/T4.class");
+ int count = 0;
+ URL lastURL = new URL("https://sling.apache.org";);
+ while (resourceURLs.hasMoreElements()) {
+ count++;
+ lastURL = resourceURLs.nextElement();
+ }
+ Assert.assertEquals(1, count);
+
Assert.assertEquals("jar:file:/ws/org.apache.sling.test3.jar!/org/apache/sling/test3/T4.class",
lastURL.toString());
}
}
