geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ivan <xhh...@gmail.com>
Subject Re: ordering tck failures in geronimo
Date Wed, 08 Dec 2010 02:05:16 GMT
Thanks for tacking this, Leonardo.
I am not sure I understand the changes. Moving methods to the
FacesConfigurationProvider is required, but it seems to me that it is not
enough. With this spi, do I still need to provide those sorting/merging/...
methods ?
The only way I could see now is that, a. extend the wrapper class b. Use
FacesConfigurationProviderFactory to get an instance of
FacesConfigurationProvider. But that seems a little strange to me ...

I also moved this thread from geronimo-tck to dev-geronimo, as current
discussion is not related to tck, and no related message in current email.
thanks.

2010/12/8 Leonardo Uribe <lu4242@gmail.com>

> 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
>>>
>>>
>>>
>>
>>
>


-- 
Ivan

Mime
View raw message