[ 
https://issues.apache.org/jira/browse/MYFACES-3051?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12998794#comment-12998794
 ] 

Jakob Korherr commented on MYFACES-3051:
----------------------------------------

OK, I see your point, Leo, but I think the general idea of a MyFacesClassLoader 
is good enough to follow this approach. It comes with the advantage that we do 
not have to think about which ClassLoader may be the right one in different 
scenarios (OSGi vs. non-OSGi, MyFaces resources vs. webapp resource). 
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).

Fortunately, I think the problems you're seeing can be fixed easily:

>It is ok, but note if we are using myfaces-bundle, we'll scan on the same 
>classloader twice (because api and impl are the same bundle classloader)

The solution is to change MyFacesClassLoader to check if api and impl 
ClassLoader are actually the same and only use one in this case.

>This code will not work correctly:
>
>    @Override
>    public Enumeration<URL> getResources(String s) throws IOException
>    {
>        // use all 3 classloaders and merge without duplicates
>        Set<URL> urls = new HashSet<URL>(); // no duplicates
>
>        // context classloader
>        urls.addAll(Collections.list(super.getResources(s)));
>
>        // api classlaoder
>        urls.addAll(Collections.list(apiClassLoader.getResources(s)));
>
>        // impl classlaoder
>        urls.addAll(Collections.list(implClassLoader.getResources(s)));
>
>        return Collections.enumeration(urls);

Actually, it will work correctly. Take a closer look at the comments: "use all 
3 classloaders and merge WITHOUT duplicates" and "no duplicates". You see, the 
method uses a Set<URL> in order to automatically remove duplicate URLs.

In addition, MYFACES-3044 was solved by the MyFacesClassLoader already. So why 
should we remove it?

I'll provide a new patch to address the points mentioned.

> 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.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

        

Reply via email to