myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jakob Korherr (JIRA)" <...@myfaces.apache.org>
Subject [jira] Commented: (MYFACES-3051) Use multiple ClassLoaders to find resources (not only ContextClassLoader)
Date Thu, 24 Feb 2011 10:20:38 GMT

    [ 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

        

Mime
View raw message