Skip to content

Commit

Permalink
Add filtered classloader.
Browse files Browse the repository at this point in the history
  • Loading branch information
alesj committed Mar 19, 2012
1 parent e73d5dd commit 88e7e7b
Show file tree
Hide file tree
Showing 3 changed files with 179 additions and 8 deletions.
@@ -0,0 +1,129 @@
/*
* JBoss, Home of Professional Open Source
* Copyright 2012, Red Hat Middleware LLC, and individual contributors
* by the @authors tag. See the copyright.txt in the distribution for a
* full listing of individual contributors.
*
* Licensed 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.
*/
package org.jboss.shrinkwrap.api.classloader;

import java.io.IOException;
import java.lang.reflect.Method;
import java.net.URL;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashSet;
import java.util.Set;

/**
* Filter delegate classloader.
*
* @author <a href="mailto:ales.justin@jboss.org">Ales Justin</a>
*/
public class FilteredClassLoader extends ClassLoader {
private static final Method getPackage;
private final ClassLoader delegate;
private final Set<String> excludes;
private final Set<String> suffixes = new HashSet<String>();

static {
try {
getPackage = ClassLoader.class.getDeclaredMethod("getPackage", String.class);
getPackage.setAccessible(true);
} catch (NoSuchMethodException e) {
throw new IllegalArgumentException("Error getting getPackage method.", e);
}
}

public FilteredClassLoader(String... excludes) {
this(null, excludes);
}

public FilteredClassLoader(ClassLoader delegate, String... excludes) {
if (delegate == null)
delegate = getSystemClassLoader();
this.delegate = delegate;
this.excludes = new HashSet<String>();
for (String ex : excludes) {
String path = toPath(ex);
this.excludes.add(path.endsWith("/") || path.endsWith("*") ? path : (path + "/"));
}
// add default suffixes
addSuffix(".class");
addSuffix(".xml");
addSuffix(".properties");
}

public void addSuffix(String suffix) {
suffixes.add(suffix);
}

protected static String toPath(String pckg) {
return pckg.replace(".", "/");
}

protected boolean isExcluded(String name) {
boolean match;
for (String suffix : suffixes) {
match = name.endsWith(suffix);
if (match) {
name = name.substring(0, name.length() - suffix.length());
break;
}
}
name = toPath(name);
name = name.substring(0, name.lastIndexOf("/") + 1); // cut off the name

for (String exclude : excludes) {
if (exclude.endsWith("*")) {
if (name.startsWith(exclude.substring(0, exclude.length() - 1)))
return true;
} else {
if (name.equals(exclude))
return true;
}
}

return false;
}

@Override
protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
if (name.startsWith("java.") || isExcluded(name) == false) {
Class<?> clazz = delegate.loadClass(name);
if (resolve)
resolveClass(clazz);
return clazz;
}

throw new ClassNotFoundException("Cannot load excluded class: " + name);
}

@Override
public URL getResource(String name) {
return isExcluded(name) ? null : delegate.getResource(name);
}

@Override
public Enumeration<URL> getResources(String name) throws IOException {
return isExcluded(name) ? Collections.enumeration(Collections.<URL>emptySet()) : delegate.getResources(name);
}

@Override
protected Package getPackage(String name) {
try {
return isExcluded(name) ? null : (Package) getPackage.invoke(delegate, name);
} catch (Exception e) {
throw new IllegalArgumentException(e);
}
}
}
Expand Up @@ -18,6 +18,7 @@

import java.io.Closeable;
import java.io.FileNotFoundException;
import java.io.FilterInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
Expand All @@ -40,6 +41,7 @@
*
* @author <a href="mailto:aslak@redhat.com">Aslak Knutsen</a>
* @author <a href="mailto:andrew.rubinger@jboss.org">ALR</a>
* @author <a href="mailto:ales.justin@jboss.org">Ales Justin</a>
*/
public class ShrinkWrapClassLoader extends URLClassLoader implements Closeable {
// -------------------------------------------------------------------------------------||
Expand Down Expand Up @@ -130,7 +132,7 @@ public InputStream getInputStream() throws IOException {
if (node == null) {
// We've asked for a path that doesn't exist
throw new FileNotFoundException("Requested path: " + path + " does not exist in "
+ archive.toString());
+ archive.toString());
}

final Asset asset = node.getAsset();
Expand All @@ -145,7 +147,7 @@ public InputStream getInputStream() throws IOException {
synchronized (this) {
openedStreams.add(input);
}
return input;
return new InputStreamWrapper(input);

This comment has been minimized.

Copy link
@ALRubinger

ALRubinger Mar 19, 2012

Why are these changes needed to SWCL? The stream returned is already Closeable and we don't need to return a filtered impl (by contract)...

This comment has been minimized.

Copy link
@alesj

alesj Mar 19, 2012

Author Owner

Afais, if a stream is already closed -- as it should be, in any decent IO code,
it should be removed from openedStreams -- otherwise a 2nd call to close can happen,
which might not be ok, depending on the actual close impl.

This comment has been minimized.

Copy link
@ALRubinger

ALRubinger Mar 19, 2012

If the Closeable doesn't support double-close without error, it's broken by contract:

http://docs.oracle.com/javase/1.5.0/docs/api/java/io/Closeable.html#close()

" If the stream is already closed then invoking this method has no effect."

This comment has been minimized.

Copy link
@alesj

alesj Mar 19, 2012

Author Owner

You're also leaking resource.
e.g. I close the stream manually, hence expecting it's a done-deal, but then profiler says it's not ;-)

This comment has been minimized.

Copy link
@ALRubinger

ALRubinger via email Mar 19, 2012

This comment has been minimized.

Copy link
@alesj

alesj Mar 19, 2012

Author Owner

I didn't see it, I'm just thinking out-loud and in theory. :-)


}

Expand Down Expand Up @@ -175,4 +177,17 @@ public void close() throws IOException {
openedStreams.clear();
}
}
}

private class InputStreamWrapper extends FilterInputStream {
private InputStreamWrapper(InputStream delegate) {
super(delegate);
}

public void close() throws IOException {
synchronized (this) {
openedStreams.remove(in);
}
super.close();
}
}
}
Expand Up @@ -28,9 +28,12 @@
import org.jboss.shrinkwrap.api.GenericArchive;
import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.asset.Asset;
import org.jboss.shrinkwrap.api.asset.EmptyAsset;
import org.jboss.shrinkwrap.api.asset.StringAsset;
import org.jboss.shrinkwrap.api.classloader.FilteredClassLoader;
import org.jboss.shrinkwrap.api.classloader.ShrinkWrapClassLoader;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.jboss.shrinkwrap.impl.base.MockArchive;
import org.jboss.shrinkwrap.impl.base.io.IOUtil;
import org.junit.After;
import org.junit.Assert;
Expand All @@ -42,6 +45,7 @@
*
* @author <a href="mailto:aslak@redhat.com">Aslak Knutsen</a>
* @author <a href="mailto:andrew.rubinger@jboss.org">ALR</a>
* @author <a href="mailto:ales.justin@jboss.org">Ales Justin</a>
* @version $Revision: $
*/
public class ShrinkWrapClassLoaderTestCase {
Expand All @@ -64,7 +68,7 @@ public class ShrinkWrapClassLoaderTestCase {
* Archive to be read via a {@link ShrinkWrapClassLoaderTestCase#shrinkWrapClassLoader}
*/
private static final JavaArchive archive = ShrinkWrap.create(JavaArchive.class).addClass(
applicationClassLoaderClass);
applicationClassLoaderClass);

// -------------------------------------------------------------------------------------||
// Instance Members -------------------------------------------------------------------||
Expand Down Expand Up @@ -114,7 +118,7 @@ public void closeClassLoader() {
public void shouldBeAbleToLoadClassFromArchive() throws ClassNotFoundException {
// Load the test class from the CL
final Class<?> loadedTestClass = Class.forName(applicationClassLoaderClass.getName(), false,
shrinkWrapClassLoader);
shrinkWrapClassLoader);

final ClassLoader loadedTestClassClassLoader = loadedTestClass.getClassLoader();
log.info("Got " + loadedTestClass + " from " + loadedTestClassClassLoader);
Expand All @@ -123,10 +127,10 @@ public void shouldBeAbleToLoadClassFromArchive() throws ClassNotFoundException {
Assert.assertNotNull("Test class could not be found via the ClassLoader", loadedTestClass);

Assert.assertSame("Test class should have been loaded via the archive ClassLoader", shrinkWrapClassLoader,
loadedTestClassClassLoader);
loadedTestClassClassLoader);

Assert.assertNotSame("Class Loaded from the CL should not be the same as the one on the appCL",
loadedTestClass, applicationClassLoaderClass);
loadedTestClass, applicationClassLoaderClass);

}

Expand Down Expand Up @@ -219,6 +223,29 @@ public void shouldBeAbleToLoadAResourceFromArchiveMultipleTimes() throws Excepti
IOUtil.copyWithClose(resource.openStream(), new ByteArrayOutputStream());
}

@Test(expected = ClassNotFoundException.class)
public void shouldNotBeAbleToLoadFilteredClass() throws Exception {
JavaArchive jar = ShrinkWrap.create(JavaArchive.class);
jar.addClass(MockArchive.class);
FilteredClassLoader fcl = new FilteredClassLoader(LoadedTestClass.class.getPackage().getName());
ShrinkWrapClassLoader cl = new ShrinkWrapClassLoader(fcl, jar);
cl.loadClass(LoadedTestClass.class.getName());
}

@Test
public void shouldNotBeAbleToGetFilteredResource() throws Exception {
JavaArchive jar = ShrinkWrap.create(JavaArchive.class);
jar.addClass(MockArchive.class);
jar.addAsResource(EmptyAsset.INSTANCE, "org/jboss/acme/foobar.xml");
String excluded = "org/jboss/shrinkwrap/impl/base/asset";
FilteredClassLoader fcl = new FilteredClassLoader(excluded);
ShrinkWrapClassLoader cl = new ShrinkWrapClassLoader(fcl, jar);
String name = excluded + "/Test.properties";
URL resource = cl.getResource(name);
Assert.assertNull(resource);
Assert.assertNotNull(cl.getResource("org/jboss/acme/foobar.xml"));
}

// -------------------------------------------------------------------------------------||
// Internal Helper Methods ------------------------------------------------------------||
// -------------------------------------------------------------------------------------||
Expand All @@ -234,4 +261,4 @@ private static String getResourceNameOfClass(final Class<?> clazz) {
sb.append(".class");
return sb.toString();
}
}
}

2 comments on commit 88e7e7b

@ALRubinger
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit represents a well-tested, simple filtered CL implementation which is likely to be useful in concert with SWCL.

A concern I have is that this is well out-of-scope for the SW project. What does this mean in terms of greater/easier configuration for the resources that get filtered in the CL? What about a switch to go to modular, or child-first delegation? This opens the doors to us providing ClassLoader implementations (in the API, no less) that are beyond what the SW project is intended to supply.

A more natural solution to me seems to have a slim CL thing build atop/delegate to/use the SWCL for loading of classes in a SW archive, and handle all other elements like the delegation model and filtering etc on its own. So what's preventing us from using something like JBCL or jboss-modules in Arquillian/Weld to wrap up the intentionally-simple SWCL?

@alesj
Copy link
Owner Author

@alesj alesj commented on 88e7e7b Mar 19, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, perhaps what we need it a new SW sub-project, which could handle diff classloading rules/notions;
e.g. as you mentioned, JBCL, Modules, OSGI, ...
Or this might be an over-kill, as you already have super-fast envs which use this CL systems,
hence we would just be re-inventing the wheel.

Where my FCL is a simple and good-enough solution to a real problem I just stumbled upon.

Embedded testing w/o such system CL filtering can hide actual CL problems,
making it pretty useless, unless you then also test the same stuff in real env.
Luckily this is the case with Weld

But we still use Embedded for quick&dirty tests, and knowing that the CL works as it should, is crucial.
e.g. that we don't slip in some bean class by mistake, etc

Please sign in to comment.