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

Leonardo Uribe edited comment on MYFACES-2945 at 12/8/10 3:10 PM:
------------------------------------------------------------------

Comment from Jakob Korherr (Issue MYFACES-2998 which is duplicate from here)

 Currently the FacesConfigurationProvider provides the following methods to get 
the FacesConfig data from different sources:

- getStandardFacesConfig()
- getMetaInfServicesFacesConfig()
- getAnnotationsFacesConfig()
- getClassloaderFacesConfig()
- getContextSpecifiedFacesConfig()
- getWebAppFacesConfig()

Unfortunately it also provides a method that should combine all these data: 
getFacesConfigData(). This method is here due to refactorings, but IMHO this is 
the last place where it should be put, because it gets "its own implementation" 
via its Factory and then calls all of the above methods on it. I know this was 
introduced to support wrapping the default impl, but it really is very, very 
ugly.

Because of the fact that the faces-config merging (and ordering) is a very 
complex task and there is only one "right" way to do it (per JSF 2.0 spec), 
this must not be done by a custom SPI implementation. This code should be 
provided by MyFaces only.

Thus IMO the right way to solve this problem is to move all merging and 
ordering code into its own, MyFaces-specific class. This class (and not the 
SPI) should provide the getFacesConfigData() method. This means that in 
FacesConfigurator.configure() we should first get the 
FacesConfigurationProvider SPI impl and then pass it to 
FacesConfigMerger.getFacesConfigData().

NOTE that with this approach custom FacesConfigurationProvider impls (like in 
Geronimo) do not have to deal with this complex merging and ordering stuff and 
their life is a lot easier!


      was (Author: lu4242):
    Comment from Jakob Korherr (Issue MYFACES-2998 which is duplicate from here)

 Currently the FacesConfigurationProvider provides the following methods to get 
the FacesConfig data from different sources:

- getStandardFacesConfig()
- getMetaInfServicesFacesConfig()
- getAnnotationsFacesConfig()
- getClassloaderFacesConfig()
- getContextSpecifiedFacesConfig()
- getWebAppFacesConfig()

Unfortunately it also provides a method that should combine all these data: 
getFacesConfigData(). This method is here due to refactorings, but IMHO this is 
the last place where it should be put, because it gets "its own implementation" 
via its Factory and then calls all of the above methods on it. I know this was 
introduced to support wrapping the default impl, but it really is very, very 
ugly.

Because of the fact that the faces-config merging (and ordering) is a very 
complex task and there is only one "right" way to do it (per JSF 2.0 spec), 
this must not be done by a custom SPI implementation. This code should be 
provided by MyFaces only.

Thus IMO the right way to solve this problem is to move all merging and 
ordering code into its own, MyFaces-specific class. This class (and not the 
SPI) should provide the getFacesConfigData() method. This means that in 
FacesConfigurator.configure() we should first get the 
FacesConfigurationProvider SPI impl and then pass it to 
FacesConfigMerger.getFacesConfigData().

NOTE that with this approach custom FacesConfigurationProvider impls (like in 
Geronimo) do not have to deal with this complex merging and ordering stuff and 
their life is a lot easier!
 Description    
   Currently the FacesConfigurationProvider provides the following methods to 
get the FacesConfig data from different sources: - getStandardFacesConfig() - 
getMetaInfServicesFacesConfig() - getAnnotationsFacesConfig() - 
getClassloaderFacesConfig() - getContextSpecifiedFacesConfig() - 
getWebAppFacesConfig() Unfortunately it also provides a method that should 
combine all these data: getFacesConfigData(). This method is here due to 
refactorings, but IMHO this is the last place where it should be put, because 
it gets "its own implementation" via its Factory and then calls all of the 
above methods on it. I know this was introduced to support wrapping the default 
impl, but it really is very, very ugly. Because of the fact that the 
faces-config merging (and ordering) is a very complex task and there is only 
one "right" way to do it (per JSF 2.0 spec), this must not be done by a custom 
SPI implementation. This code should be provided by MyFaces only. Thus IMO the 
right way to solve this problem is to move all merging and ordering code into 
its own, MyFaces-specific class. This class (and not the SPI) should provide 
the getFacesConfigData() method. This means that in 
FacesConfigurator.configure() we should first get the 
FacesConfigurationProvider SPI impl and then pass it to 
FacesConfigMerger.getFacesConfigData(). NOTE that with this approach custom 
FacesConfigurationProvider impls (like in Geronimo) do not have to deal with 
this complex merging and ordering stuff and their life is a lot easier!

  
> Make a way to get the FacesConfig from a provider
> -------------------------------------------------
>
>                 Key: MYFACES-2945
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2945
>             Project: MyFaces Core
>          Issue Type: Improvement
>          Components: SPI
>    Affects Versions: 2.0.2
>            Reporter: Ivan
>            Assignee: Leonardo Uribe
>             Fix For: 2.0.3-SNAPSHOT
>
>         Attachments: MYFACES-2945-2.patch, MYFACES-2945-3.patch
>
>
> Currently, MyFaces startup listener will parse the all the faces 
> configuration files and sort them on each startup time, and it will be better 
> to do it once in the deployment time, and get those data structure instances 
> from a provider. One possible way is to make those FacesConfig class 
> serializable.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to