[
https://issues.apache.org/jira/browse/MYFACES-3051?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12998877#comment-12998877
]
Leonardo Uribe commented on MYFACES-3051:
-----------------------------------------
JK >> ClassLoaderUtils basically does the same already, however with a lot of
static methods and not with a custom ClassLoader impl (which would be easier
IMO).
That's exactly the reason why it is not necessary such wrapper. In this case,
all those methods does not change, it always will be the same.
JK >> Actually, it will work correctly. Take a closer look at the comments:
"use all 3 classloaders and merge WITHOUT duplicates" and "no duplicates".
JK >> You see, the method uses a Set<URL> in order to automatically remove
duplicate URLs.
Of course, but all that preprocessing is in fact unnecessary, and the ordering
of the resources is lost. In this case the order is relevant. Look the javadoc
of Clasloader. It says:
"... The search order is described in the documentation for
getResource(String). ..."
Look the code on Apache ibatis (ClassLoaderWrapper). Note deliberately such
class does not extends from Classloader.
If we have a class that we are not expecting to override it or "consume" it in
some way, it does not have sense to put it in external context. At this time,
to deal with OSGi we have provided multiple interfaces, so MyFaces can "ask for
help" to OSGi to locate resources (faces-config stuff, META-INF/services
factory or annotation scanning). Thinking on MYFACES-3044, the best is suggest
OSGi users to create a custom ResourceHandler that can locate resources. Really
we don't have to do anything. If we do something, we could provide another SPI
interface to help our default ResourceHandler implementation to load resource
files. That sounds better, because we are in control of what's going on.
In conclusion, there is not enough justification to do this change. From my
point of view, we are adding more unnecessary complexity to something that
should be simple and clear. The hack with OSGi (try TCCL first then bundle
class loader though someclass.getClassloader()) it is widely used, so really we
are not simplifying thing here. Based on this reasoning I have to object this
patch.
> Use multiple ClassLoaders to find resources (not only ContextClassLoader)
> -------------------------------------------------------------------------
>
> Key: MYFACES-3051
> URL: https://issues.apache.org/jira/browse/MYFACES-3051
> Project: MyFaces Core
> Issue Type: Bug
> Affects Versions: 2.0.4
> Environment: OSGi
> Reporter: Jakob Korherr
> Assignee: Jakob Korherr
> Attachments: MYFACES-3051-first-draft.patch,
> MYFACES-3051-impl-and-shared-2.patch, MYFACES-3051-impl-and-shared.patch
>
>
> In most parts of the code we only use the ContextClassLoader to find Classes
> and Resources. However, in some parts we also use the ClassLoader of the
> current Class or of a specific Class (e.g. to use myfaces-api and/or
> myfaces-impl ClassLoader, see ApplicationImpl.getResourceBundle(),
> BeanValidator.postSetValidationGroups(), ResourceHandlerImpl.getBundle() or
> FactoryFinder for example).
> IMO we should unify this code and maybe provide a custom ClassLoader that
> encapsulates three ClassLoaders (ContextClassLoader, myfaces-api and
> myfaces-impl). This most certainly would solve a lot of OSGi related problems.
> WDYT?
--
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
