You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by "David Jencks (JIRA)" <de...@myfaces.apache.org> on 2010/12/07 01:44:09 UTC
[jira] Created: (MYFACES-2995) Make method of determinine app
context in FactoryFinder pluggable
Make method of determinine app context in FactoryFinder pluggable
-----------------------------------------------------------------
Key: MYFACES-2995
URL: https://issues.apache.org/jira/browse/MYFACES-2995
Project: MyFaces Core
Issue Type: Improvement
Components: General
Affects Versions: 2.0.3-SNAPSHOT
Reporter: David Jencks
As discussed on dev list.... geronimo would like to explicitly mark component boundaries rather than relying on the TCCL changing.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (MYFACES-2995) Make method of determinine app
context in FactoryFinder pluggable
Posted by "Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2995?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Leonardo Uribe updated MYFACES-2995:
------------------------------------
Resolution: Fixed
Fix Version/s: 2.0.5-SNAPSHOT
Assignee: Leonardo Uribe
Status: Resolved (was: Patch Available)
Ok, I'm closing it as fixed, since I already did the changes suggested.
> Make method of determinine app context in FactoryFinder pluggable
> -----------------------------------------------------------------
>
> Key: MYFACES-2995
> URL: https://issues.apache.org/jira/browse/MYFACES-2995
> Project: MyFaces Core
> Issue Type: Improvement
> Components: SPI
> Affects Versions: 2.0.3-SNAPSHOT
> Reporter: David Jencks
> Assignee: Leonardo Uribe
> Fix For: 2.0.5-SNAPSHOT
>
> Attachments: MYFACES-2995-FactoryFinderProvider-1.patch, MYFACES-2995.patch
>
>
> As discussed on dev list.... geronimo would like to explicitly mark component boundaries rather than relying on the TCCL changing.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (MYFACES-2995) Make method of determinine app
context in FactoryFinder pluggable
Posted by "Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2995?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12996886#comment-12996886 ]
Leonardo Uribe commented on MYFACES-2995:
-----------------------------------------
I thought a lot about FactoryFinder and how we can "extend" it. I'll do a resume, describing the problem in detail and providing some alternatives. This could be a little bit redundant, but it is necessary to gain a better understanding about what's going on and the direction to take.
FactoryFinder is a class with three methods:
public final class FactoryFinder
{
public static Object getFactory(String factoryName) throws FacesException {...}
public static void setFactory(String factoryName, String implName) {...}
public static void releaseFactories() throws FacesException {...}
}
The javadoc describe the intention of FactoryFinder class:
"... FactoryFinder implements the standard discovery algorithm for all factory objects specified in the JavaServer Faces APIs. For a given factory class name, a corresponding implementation class is searched for based on the following algorithm...."
On the javadoc there is more information from the point of view of the author (classloading stuff and so on), but just ignore it for a moment, because it only makes things confusing.
In few words, this class allows to find JSF factory classes. The necessary information to create factory instances is loaded on initialization time, but which locations contains such information (for more information see JSF 2.0 spec section 11.4.2) (we are only interested in jsf factories initialization information) ?
- Look factories on META-INF/services/[factoryClassName]
- Look META-INF/faces-config.xml or META-INF/[prefix].faces-config.xml
- Look the files pointed by javax.faces.CONFIG_FILES web config param (note WEB-INF/web.xml is taken into consideration)
- Look the applicationFacesConfig on WEB-INF/faces-config.xml
Based on the previous facts, the first conclusion to take into account arise: Configuration information is gathered per "web context". What is a "web context"? In simple terms, is the "space" where a web application is deployed. Let's suppose an EAR file with two WAR files: a.war and b.war. Both contains different "web applications" and when are deployed has different "web context", so both can provide different factory configuration, because both has different WEB-INF/web.xml and WEB-INF/faces-config.xml files.
Now, given a request, how the web container identify a "web context"? At start, it receives the request information and based on that it decides which web application should process it. After that, it assign to a thread from is thread pool to be processed and the control is passed to the proper filters/servlets.
So, if we don't have a servlet context/portlet context/whatever context, how to identify a "web context"? The answer is using the thread, but the one who knows how to do that is the web container, not the jsf implementation.
The existing problem is caused by a "shortcut" taken to make things easier. Instead use the current "thread", it is taken as advantage the fact that each web application deployed has a different classloader. That is true for a lot of application servers, so the current implementation of FactoryFinder is based on that fact too and has worked well since the beginning.
Now let's examine in detail how a "single classloader per EAR" option could work. If the EAR has two WAR files (a.war and b.war), we have two web context, and the initialization code is executed twice. When all FactoryFinder methods are called?
- FactoryFinder.setFactory is called on initialization
- FactoryFinder.releaseFactories is called on shutdown
- FactoryFinder.getFactory is called after initialization configuration is done but before shutdown call to FactoryFinder.setFactory .
Remember all methods of FactoryFinder are static.
One possible solution could be:
1. Create a class called FactoryFinderProvider, that has the same three method but in a non static version.
2. A singleton component is provided that holds the information of the FactoryFinderProviderFactory. This one works per classloader, so the singleton is implemented using an static variable. To configure it, the static method should be called when the "classloader realm" is initialized, before any web context is started (the WAR is deployed). Usually the EAR is started as a single entity, so this should occur when the EAR starts, but before the WAR files are started (or the web context are created). The singleton will be responsible to decide which FactoryFinderProvider should be used, based on the current thread information.
3. Add utility methods to retrieve the required objects and call the methods using reflection from javax.faces.FactoryFinder
I provided a patch a patch for this one (MYFACES-2995-FactoryFinderProvider-1.patch)
If no objections I'll commit this code soon.
> Make method of determinine app context in FactoryFinder pluggable
> -----------------------------------------------------------------
>
> Key: MYFACES-2995
> URL: https://issues.apache.org/jira/browse/MYFACES-2995
> Project: MyFaces Core
> Issue Type: Improvement
> Components: SPI
> Affects Versions: 2.0.3-SNAPSHOT
> Reporter: David Jencks
> Attachments: MYFACES-2995-FactoryFinderProvider-1.patch, MYFACES-2995.patch
>
>
> As discussed on dev list.... geronimo would like to explicitly mark component boundaries rather than relying on the TCCL changing.
--
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (MYFACES-2995) Make method of determinine app
context in FactoryFinder pluggable
Posted by "Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2995?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12996973#comment-12996973 ]
Leonardo Uribe commented on MYFACES-2995:
-----------------------------------------
The example of the EAR is just a "help" to understand the problem, but it is not a limitation. The important concept is FactoryFinderProviderFactory instance is an static var that should be initialized when the "classloader context" is initialized. For example, if I have 5 EAR files and all share the same myfaces implementation (by some specific classloading configuration done by the server), the container should initialize this instance before deploy/start any of the 5 EAR.
I was thinking on make some changes on FactoryFinderProviderFactory, removing the code that set FactoryFinder._initialized to false. Instead, check if FactoryFinder has been initialized and if that so, log a warning indicating the instance will not be changed.
I'll be pending of your review so we can close this issue.
> Make method of determinine app context in FactoryFinder pluggable
> -----------------------------------------------------------------
>
> Key: MYFACES-2995
> URL: https://issues.apache.org/jira/browse/MYFACES-2995
> Project: MyFaces Core
> Issue Type: Improvement
> Components: SPI
> Affects Versions: 2.0.3-SNAPSHOT
> Reporter: David Jencks
> Attachments: MYFACES-2995-FactoryFinderProvider-1.patch, MYFACES-2995.patch
>
>
> As discussed on dev list.... geronimo would like to explicitly mark component boundaries rather than relying on the TCCL changing.
--
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (MYFACES-2995) Make method of determinine app
context in FactoryFinder pluggable
Posted by "David Jencks (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2995?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12996947#comment-12996947 ]
David Jencks commented on MYFACES-2995:
---------------------------------------
Since it's been about 2 1/2 months before this issue was really responded to I'd appreciate a few days to review your patch before you commit it. At the moment I don't understand how a per-ear object could possibly be of any use. IMO depending on the app server either a classloader based strategy as is used at present will work fine or the app server will need to use something else for every ear. There is little chance that one app server will use different classloading strategies for different ears and want to therefore use different code to distinguish jsf contexts for different ears.
However I may have misunderstood your comments as I have not yet had a chance to look at your patch.
> Make method of determinine app context in FactoryFinder pluggable
> -----------------------------------------------------------------
>
> Key: MYFACES-2995
> URL: https://issues.apache.org/jira/browse/MYFACES-2995
> Project: MyFaces Core
> Issue Type: Improvement
> Components: SPI
> Affects Versions: 2.0.3-SNAPSHOT
> Reporter: David Jencks
> Attachments: MYFACES-2995-FactoryFinderProvider-1.patch, MYFACES-2995.patch
>
>
> As discussed on dev list.... geronimo would like to explicitly mark component boundaries rather than relying on the TCCL changing.
--
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (MYFACES-2995) Make method of determinine app
context in FactoryFinder pluggable
Posted by "David Jencks (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2995?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12999056#comment-12999056 ]
David Jencks commented on MYFACES-2995:
---------------------------------------
This looks ok to me although I don't like the amount of reflection that much. A few comments:
- I don't see why FactoryFinderProvider is an abstract class rather than an interface.
- I don't see why the api _FactoryFinderProviderFactory needs to know anything about FactoryFinderProviderFactory. Since you are already using reflection to poke around in the api, why not have FactoryFinderProviderFactory just set an object field in the api class directly?
- I think I would make the tccl and optional replacement strategies more similar by putting the tccl based code in a FactoryFinderProvider like object set into _FactoryFinderProviderFactory by default.
I haven't tried this with geronimo yet but I think it will work.
> Make method of determinine app context in FactoryFinder pluggable
> -----------------------------------------------------------------
>
> Key: MYFACES-2995
> URL: https://issues.apache.org/jira/browse/MYFACES-2995
> Project: MyFaces Core
> Issue Type: Improvement
> Components: SPI
> Affects Versions: 2.0.3-SNAPSHOT
> Reporter: David Jencks
> Attachments: MYFACES-2995-FactoryFinderProvider-1.patch, MYFACES-2995.patch
>
>
> As discussed on dev list.... geronimo would like to explicitly mark component boundaries rather than relying on the TCCL changing.
--
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Commented: (MYFACES-2995) Make method of determinine app
context in FactoryFinder pluggable
Posted by "Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2995?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12999067#comment-12999067 ]
Leonardo Uribe commented on MYFACES-2995:
-----------------------------------------
DJ >> This looks ok to me although I don't like the amount of reflection that much. A few comments:
Well, it is unavoidable.
DJ >> I don't see why FactoryFinderProvider is an abstract class rather than an interface.
Good point. Yes, it could be an interface instead an abstract class, because it is not expected this one could change in the future. I'll do the necessary changes.
DJ >> why not have FactoryFinderProviderFactory just set an object field in the api class directly?
It is not possible to do that because it introduces a circular dependency between myfaces-api and myfaces-impl.
DJ >> I think I would make the tccl and optional replacement strategies more similar by putting the tccl based code in a FactoryFinderProvider like object set into _FactoryFinderProviderFactory by default.
Yes, it could be but I think it is better to let the code as is in the patch, because it is easy to compare the javadoc against the code, and it is clear if no FactoryFinderProvider implementation is provided, just do the default algorithm. Other reason to take into account is do not use reflection if it is not necessary.
DJ >> I haven't tried this with geronimo yet but I think it will work.
So can we close this issue as fixed? (after commit the code with some changed)
> Make method of determinine app context in FactoryFinder pluggable
> -----------------------------------------------------------------
>
> Key: MYFACES-2995
> URL: https://issues.apache.org/jira/browse/MYFACES-2995
> Project: MyFaces Core
> Issue Type: Improvement
> Components: SPI
> Affects Versions: 2.0.3-SNAPSHOT
> Reporter: David Jencks
> Attachments: MYFACES-2995-FactoryFinderProvider-1.patch, MYFACES-2995.patch
>
>
> As discussed on dev list.... geronimo would like to explicitly mark component boundaries rather than relying on the TCCL changing.
--
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Updated: (MYFACES-2995) Make method of determinine app
context in FactoryFinder pluggable
Posted by "David Jencks (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-2995?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
David Jencks updated MYFACES-2995:
----------------------------------
Status: Patch Available (was: Open)
> Make method of determinine app context in FactoryFinder pluggable
> -----------------------------------------------------------------
>
> Key: MYFACES-2995
> URL: https://issues.apache.org/jira/browse/MYFACES-2995
> Project: MyFaces Core
> Issue Type: Improvement
> Components: General
> Affects Versions: 2.0.3-SNAPSHOT
> Reporter: David Jencks
>
> As discussed on dev list.... geronimo would like to explicitly mark component boundaries rather than relying on the TCCL changing.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.