myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Leonardo Uribe <lu4...@gmail.com>
Subject Re: ordering tck failures in geronimo
Date Tue, 07 Dec 2010 19:31:24 GMT
Hi

2010/12/7 David Jencks <david_jencks@yahoo.com>

>
> On Dec 7, 2010, at 10:12 AM, Leonardo Uribe wrote:
>
> Hi
>
> 2010/12/6 Ivan <xhhsld@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_jencks@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)
>
>
I have never tried, but what I understand is only annotations with
@Retention(RetentionPolicy.SOURCE) are discarded by the compiler. But if it
is possible to add jaxb as an optional dependency, yes, myfaces could
include those annotations. Anyway in this moment it is just a possibility,
so I will not believe it until I see it. Note JSF 2.0 is JDK 1.5 compatible.
That means we should take care on keep MyFaces working in 1.5.

regards,

Leonardo Uribe


> 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 <lu4242@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 <xhhsld@gmail.com>
>>>
>>>>
>>>>
>>>> 2010/12/6 David Jencks <david_jencks@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
>>
>>
>>
>
>

Mime
View raw message