This content has been marked as final.
Show 2 replies
-
1. Re: Temp include/exclude filter set in Module
adrian.brock Jul 16, 2008 8:48 AM (in response to alesj)Isn't this just missing a check for whether to recurse into a virtual file?
Index: classloading-vfs/src/main/org/jboss/classloading/plugins/vfs/VFSResourceVisitor.java =================================================================== --- classloading-vfs/src/main/org/jboss/classloading/plugins/vfs/VFSResourceVisitor.java (revision 75758) +++ classloading-vfs/src/main/org/jboss/classloading/plugins/vfs/VFSResourceVisitor.java (working copy) @@ -145,6 +145,10 @@ public boolean accepts(VirtualFile file) { + ResourceContext ctx = new ResourceContext(file.toURL(), file.getName(), classLoader); + if (filter.accepts(ctx) == false) + return false; + // This is our current root if (file.equals(root)) return true;
For ease of use it might be better to create a specific api
so people can use the simple filter for accepting things like .class files
and a seperate api for which directories/jars to enterIndex: classloading-vfs/src/main/org/jboss/classloading/plugins/vfs/VFSResourceVisitor.java =================================================================== --- classloading-vfs/src/main/org/jboss/classloading/plugins/vfs/VFSResourceVisitor.java (revision 75758) +++ classloading-vfs/src/main/org/jboss/classloading/plugins/vfs/VFSResourceVisitor.java (working copy) @@ -145,6 +145,10 @@ public boolean accepts(VirtualFile file) { + ResourceContext ctx = new ResourceContext(file.toURL(), file.getName(), classLoader); + if (filter.isRecurse(ctx) == false) + return false; + // This is our current root if (file.equals(root)) return true; Index: classloading/src/main/org/jboss/classloading/spi/visitor/ResourceFilter.java =================================================================== --- classloading/src/main/org/jboss/classloading/spi/visitor/ResourceFilter.java (revision 75758) +++ classloading/src/main/org/jboss/classloading/spi/visitor/ResourceFilter.java (working copy) @@ -36,4 +36,12 @@ * @return true to visit the resource */ boolean accepts(ResourceContext resource); + + /** + * Controls whether to recurse into a particular resource + * + * @param resource the resource context + * @return true to recurse into the resource + */ + boolean isRecurse(ResourceContext resource); }
Or for even better ease of use it is probably better to split the two types of filtering
classloading/src/main/org/jboss/classloading/spi/dependency/Module.java/** * Visit the resources in this module using the given filter(s) * * Typically the filter is used to determine which types of files to visit, e.g. .class files * while the recurseFilter determines which jars/directories to recurse into * * @param visitor the visitor * @param filter the filter * @param recurseFilter the recursion filter (null means recurse into everything) */ public void visit(ResourceVisitor visitor, ResourceFilter filter, ResourceFilter recurseFilter) { throw new UnsupportedOperationException("The module " + getContextName() + " does not support filtering: " + getClass().getName()); }
The last two code snippets are just "ease of use" so a filter can concentrate
on "doing one thing well" and different filters can be mixed and matched for
different purposes. The filter implementation could do this itself.
It is the checking for recursion in VFSResourceVisitor.accepts() that is missing
(the first patch). -
2. Re: Temp include/exclude filter set in Module
adrian.brock Jul 16, 2008 8:56 AM (in response to alesj)The reason I mention using a different api, beside "ease of use"
is that currently the filters are only currently invoked to check whether
the vist() callback should be invoked, so introducing a different api
for the new behaviour with a backward compatiblity method/** * Visit the resources in this module * using the given filter * * @param visitor the visitor * @param filter the filter */ public void visit(ResourceVisitor visitor, ResourceFilter filter) { - throw new UnsupportedOperationException("The module " + getContextName() + " does not support filtering: " + getClass().getName()); + visit(visitor, filter, null); }
would also avoid unintended changes to existing code.