You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@myfaces.apache.org by "Jakob Korherr (JIRA)" <de...@myfaces.apache.org> on 2010/12/08 13:54:01 UTC

[jira] Created: (MYFACES-2998) FacesConfig ordering must not be an SPI task

FacesConfig ordering must not be an SPI task
--------------------------------------------

                 Key: MYFACES-2998
                 URL: https://issues.apache.org/jira/browse/MYFACES-2998
             Project: MyFaces Core
          Issue Type: Task
          Components: SPI
    Affects Versions: 2.0.2
            Reporter: Jakob Korherr
            Assignee: Jakob Korherr


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!

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

[jira] Commented: (MYFACES-2998) FacesConfig ordering must not be an SPI task

Posted by "Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>.

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

Leonardo Uribe commented on MYFACES-2998:
-----------------------------------------

I have some comments about this patch, so please don't commit it yet.

> FacesConfig ordering must not be an SPI task
> --------------------------------------------
>
>                 Key: MYFACES-2998
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2998
>             Project: MyFaces Core
>          Issue Type: Task
>          Components: SPI
>    Affects Versions: 2.0.2
>            Reporter: Jakob Korherr
>            Assignee: Jakob Korherr
>         Attachments: MYFACES-2998.patch
>
>
> 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!

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

[jira] Commented: (MYFACES-2998) FacesConfig ordering must not be an SPI task

Posted by "Jakob Korherr (JIRA)" <de...@myfaces.apache.org>.

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

Jakob Korherr commented on MYFACES-2998:
----------------------------------------

If no objections, I will commit this patch soon!

> FacesConfig ordering must not be an SPI task
> --------------------------------------------
>
>                 Key: MYFACES-2998
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2998
>             Project: MyFaces Core
>          Issue Type: Task
>          Components: SPI
>    Affects Versions: 2.0.2
>            Reporter: Jakob Korherr
>            Assignee: Jakob Korherr
>         Attachments: MYFACES-2998.patch
>
>
> 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!

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