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