Mailing-List: contact dev-help@myfaces.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "MyFaces Development" <dev@myfaces.apache.org>
Date: Thu, 24 Feb 2011 10:20:38 +0000 (UTC)
From: "Jakob Korherr (JIRA)" <dev@myfaces.apache.org>
To: dev@myfaces.apache.org
Message-ID: 
 <949187602.13210.1298542838454.JavaMail.tomcat@hel.zones.apache.org>
In-Reply-To: 
 <275025381.7913.1298370398682.JavaMail.tomcat@hel.zones.apache.org>
Subject: [jira] Commented: (MYFACES-3051) Use multiple ClassLoaders to find
 resources (not only ContextClassLoader)
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ 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