On Dec 7, 2010, at 10:12 AM, Leonardo Uribe wrote:
> Hi
>
> 2010/12/6 Ivan <xhh...@gmail.com>
>
> So, at least could you please help to add the export the
> org.apache.myfaces.config package in 2.0.3, that will make my life easier
> now. And remove it once those methods are moved to FacesConfigurationProvider
> in the future.
>
> Instead do that, it is better to add them now. (I committed it on
> MYFACES-2945) It is a change already studied, so I don't see any problem
> doing it. I did some small changes too, so it could be good if you try it to
> see if everything is ok (I'll wait for your response) .
>
> 2010/12/7 David Jencks <david_jen...@yahoo.com>
>
>
> 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.
>
>
> I don't think so, because do that requires to add a dependency to jaxb on
> myfaces impl, and we have one xml library dependency already
> (commons-digester).
Classes containing annotations that aren't available at runtime should still
load fine, so this would be a compile time dependency but optional at runtime.
It would possibly make the commons-digester dependency optional at runtime as
well (at least with non-myfaces additional code that uses jaxb)
thanks
david jencks
>
> regards,
>
> Leonardo URibe
>
> 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
>
>
