On Dec 6, 2010, at 5:36 PM, Ivan wrote:

> Some detailed information :
> a. org.apache.myfaces.config.impl.digester -> 
> DigesterFacesConfigUnmarshallerImpl
> b. org.apache.myfaces.config -> DefaultFacesConfigurationProvider
> c. org.apache.myfaces.spi.impl -> ServletMappingImpl
>  
> For item c, I have removed the use of ServletMappingImpl, so no need to 
> export the spi.impl package now.
> For item a, currently I use DigesterFacesConfigUnmarshallerImpl to parse the 
> faces-config.xml codes in Geronimo's own codes, as I need to query some info 
> from the parsed java objects. And currently, I do not see a strong 
> requirement for a Geronimo parser.

I have not compared the myfaces classes with the openejb jaxb tree for 
faces-config.xml, so I have no idea how plausible this idea might be.  Would it 
be possible to annotate the myfaces object tree representing faces-config.xml 
so that it could be read in by jaxb as well as by digester?

For use in geronimo, I wonder if putting some or all of this code in a fragment 
bundle hosted by the myfaces bundle would reduce the number of exports needed.

thanks
david jencks


> For item b, I created a sub classes of DefaultFacesConfigurationProvider, and 
> just overrides those get*** methods, the class is look like :
> --->
> public class GeronimoFacesConfigurationProvider extends 
> DefaultFacesConfigurationProvider {
> 
>     private FacesConfig annotationsFacesConfig;
> 
>     private List<FacesConfig> classloaderFacesConfigs;
> 
>     private List<FacesConfig> contextSpecifiedFacesConfigs;
> 
>     private FacesConfig webAppFacesConfig;
> 
>     private FacesConfig standardFacesConfig;
> 
>     public GeronimoFacesConfigurationProvider(FacesConfig 
> standardFacesConfig, FacesConfig webAppFacesConfig, FacesConfig 
> annotationsFacesConfig, List<FacesConfig> classloaderFacesConfigs,
>             List<FacesConfig> contextSpecifiedFacesConfigs) {
>         this.annotationsFacesConfig = annotationsFacesConfig;
>         this.classloaderFacesConfigs = classloaderFacesConfigs;
>         this.contextSpecifiedFacesConfigs = contextSpecifiedFacesConfigs;
>         this.webAppFacesConfig = webAppFacesConfig;
>         this.standardFacesConfig = standardFacesConfig;
>     }
> 
>     @Override
>     protected FacesConfig getAnnotationsFacesConfig(ExternalContext ectx, 
> boolean metadataComplete) {
>         return annotationsFacesConfig;
>     }
> 
>     @Override
>     protected List<FacesConfig> getClassloaderFacesConfig(ExternalContext 
> externalContext) {
>         return classloaderFacesConfigs;
>     }
> 
>     @Override
>     protected List<FacesConfig> 
> getContextSpecifiedFacesConfig(ExternalContext externalContext) {
>         return contextSpecifiedFacesConfigs;
>     }
> 
>     @Override
>     protected FacesConfig getStandardFacesConfig(ExternalContext 
> externalContext) {
>         return standardFacesConfig;
>     }
> 
>     @Override
>     protected FacesConfig getWebAppFacesConfig(ExternalContext 
> externalContext) {
>         return webAppFacesConfig;
>     }
> 
> }
> <---
> Thoughts ?
> thanks.
> 
> 2010/12/7 Leonardo Uribe <lu4...@gmail.com>
> Hi
> 
> First of all, I want to note that if the default algorithm for 
> FacesConfigResourceProvider is not able to find a.faces-config.xml and 
> c.faces-config.xml, that means the Thread Context Class Loader needs to be 
> fixed, because is not taking into account jar files under WEB-INF/lib.
> 
> 2010/12/6 Ivan <xhh...@gmail.com>
> 
> 
> 2010/12/6 David Jencks <david_jen...@yahoo.com>
> 
> On Dec 5, 2010, at 7:44 PM, Ivan wrote:
> 
> > I am wondering whether the myfaces bundle could also export the spi.impl 
> > package, I hope to take advantage of some codes there, e.g. 
> > DefaultFacesConfigurationProvider, it contains some sort algorithm.
> > thanks.
> >
> 
> 
> Why export spi.impl package? the idea to have an spi api and impl is allow 
> impl package to change and let api stable to prevent break code later, right? 
> If something should be exposed from DefaultFacesConfigurationProvider (for 
> example adding some methods on FacesConfigurationProvider), it should be 
> discussed first.
> 
> I agree to expose this two:
> 
> 
> +                            
> org.apache.myfaces.config.impl.digester.elements;version="${project.version}",
> +                            
> org.apache.myfaces.config.impl.digester;version="${project.version}
> 
> because theorically the code there will not change (only between different 
> versions of JSF), but the other ones:
> 
> 
> +                            
> org.apache.myfaces.spi.impl;version="${project.version}",
> +                            
> org.apache.myfaces.config;version="${project.version}",
> 
> needs some argumentation first.
>  
> I'd say if you need to expose the default implementation of the spi then 
> there is something wrong with the interface design.  If the sorting is 
> universal then perhaps it should not be in a class implemented by a service 
> provider?
> 
> 
> Ok, as long as I undersand there is no reason why expose sorting algorithm to 
> the container. Also, allow a different parser for FacesConfig was discussed 
> before. I remember on MYFACES-2945 that it was proposed an interface with 
> this methods:
> 
> public abstract class FacesConfigurationProvider 
> { 
>     public abstract FacesConfig getStandardFacesConfig(ExternalContext ectx); 
>      
>     public abstract FacesConfig getMetaInfServicesFacesConfig(ExternalContext 
> ectx); 
>      
>     public abstract FacesConfig getAnnotationsFacesConfig(ExternalContext 
> ectx, boolean metadataComplete); 
>      
>     public abstract List<FacesConfig> 
> getClassloaderFacesConfig(ExternalContext ectx); 
>      
>     public abstract List<FacesConfig> 
> getContextSpecifiedFacesConfig(ExternalContext ectx); 
>      
>     public abstract FacesConfig getWebAppFacesConfig(ExternalContext ectx); 
>      
> } 
> 
> But finally the final form was this:
> 
> public abstract class FacesConfigurationProvider 
> { 
>     
>     public abstract FacesConfigData getFacesConfigData(ExternalContext ectx); 
>      
> } 
> 
> And this comment was added:
> 
> "...After some attempts, the structure of the solution is not too different 
> from the previous patch, because after all in 
> DefaultFacesConfigurationProvider it is necessary to put all previous code 
> plus ordering and feeding of FacesConfig files...."
> 
> Note the first form allows to define an custom parser, because to retrieve 
> FacesConfig you should locate them first and then parse them, but the second 
> one does not.
> 
>    Yes, in Geronimo, we have a sepearted algorthim for sorting. Currently, I 
> still use the implementation from MyFaces to do it, maybe in the future I 
> could move them to Geronimo side or it will be abstracted from current 
> default spi provider. I also need to provide a mock external context to make 
> it work. or I might need to copy some codes from myfaces.
>   Another thing is that, I use the digester xml parser from MyFaces to parse 
> all the faces configuration files, while we using jaxb tree in the past. This 
> could be avoid, just did not want to add some codes to convert two 
> FacesConfig objects. 
> 
> The idea behind refactor org.apache.myfaces.config.element package is give 
> some base classes from myfaces, to allow use a different parser. Those 
> changes were committed, but to allow that it is still necessary to commit 
> some methods on FacesConfigurationProvider to do the "bridge".
>  
> regards,
> 
> Leonardo Uribe
> 
>   Thanks.
> 
> 
> thanks
> david jencks
> 
> >
> > --
> > Ivan
> 
> 
> 
> 
> -- 
> Ivan
> 
> 
> 
> 
> -- 
> Ivan

Reply via email to