geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jakob Korherr <>
Subject Re: ordering tck failures in geronimo
Date Thu, 09 Dec 2010 10:01:51 GMT

I called it ugly, because of its implementation code in
DefaultFacesConfigurationProvider: The method is already inside of a
FacesConfigurationProvider, but it does this:

FacesConfigurationProvider provider = FacesConfigurationProviderFactory.

...and then calls all the other methods of FacesConfigurationProvider
on this provider instance.

As I said, I know this is this way, because FacesConfigurationProvider
can be wrapped, but IMHO it is really ugly.

The method used on MYFACES-2998 was a first approach to solve this
problem in a better way. However, I really like those two of your

1) Leo #2: Create another SPI interface for getFacesConfigData()
(please suggest a name for it, maybe
FacesConfigurationMergerProvider?) and remove this method form
2) Ivan: In a few words: let getFacesConfigData() on
FacesConfigurationProvider, but provide an
AbstractFacesConfigurationProvider which implements the merging and
sorting to let custom SPI impls take advantage of it.

At first, I really liked Ivan's proposal, but after thinking more
about it, it is not consistent with what we have right now (we do not
provide any other Abstract-SPI impl) and we would have the huge
merging and sorting code all in the SPI(-api) package, but IMO it
should really go into o.a.m.config.

Thus I think that a new FacesConfigurationMergerProvider-SPI as Leo
proposed is the best choice here. Note that for this SPI it is good
practice to use other SPI impls.

I will provide a patch for it soon and then we can discuss it further!


2010/12/9 Ivan <>:
> my 2 cents, it seems for me less urgly ...
> a. For the FacesConfigurationProvider , it is better to have only one method
> getFacesConfigData()
> b. Create another abstract class AbstractFacesConfigurationProvider which
> extends FacesConfigurationProvider, and define those proctected methods of
> get***, also place those sorting/merging codes there.
> c. In the DefaultFacesConfigurationProvider, it only implements those get***
> methods.
> thanks.
> 2010/12/9 Leonardo Uribe <>
>> Hi
>> I agree with Jakob about faces-config merging and ordering algorithm
>> should not be exposed by MyFaces. Why is it not enough?. At this point it is
>> not clear the reasons. Note in this moment ordering and sorting algoritm it
>> is not being exposed by FacesConfigurationProvider interface.
>> Other different thing is FacesConfigurationProvider.getFacesConfigData().
>> The intention of this method is provide a way to get a Serializable object
>> that represents all config information required to initialize MyFaces and
>> allow container to store it temporally somewere. In this way it is possible
>> to deploy and undeploy an application more quickly, because if "nothing
>> changes"(no added dependencies, no update from faces-config.xml files or
>> classes) this information is always the same.
>> On MYFACES-2945 and previous discussions it was proposed two different
>> options:
>> 1. Add getFacesConfigData()
>> 2. Add a set of methods to retrieve FacesConfig objects instead.
>>     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);
>> The first option has the advantage that it fill the initial requeriment
>> without add more complexity to the problem. The second one seems to be more
>> structured and opens the possibility to do other things like how override
>> MyFaces parsing for faces-config.xml files like it is being discussed. If
>> the container parse faces-config.xml files, with the previous methods it is
>> possible to prevent parse the same files twice.
>> My first intention, as is shown on MYFACES-2945 was that
>> FacesConfigurationProvider does not included getFacesConfigData(), because
>> it is possible to fill the initial objective with these methods, but finally
>> it was agreed the first option looks better, right?
>> Now I see that on MYFACES-2998 that fact is questioned:
>> JK>> Unfortunately it also provides a method that should combine all these
>> data: getFacesConfigData().
>> JK>> This method is here due to refactorings, but IMHO this is the last
>> place where it should be put,
>> JK>> because it gets "its own implementation" via its Factory and then
>> calls all of the above methods on
>> JK>> it. I know this was introduced to support wrapping the default impl,
>> but it really is very, very ugly.
>> In few words, why does it looks ugly? or with other words, what can we do
>> to make it cleaner? remove it? or just provide another SPI interface and put
>> that method there? In practice, getFacesConfigData() merges all FacesConfig
>> information, and "on the way" it does order applicationFacesConfig files
>> (the ones obtained from getClassloaderFacesConfig() and
>> getContextSpecifiedFacesConfig() ) . To do that it requires to call all six
>> methods from FacesConfigurationProvider, there is no other way, so I don't
>> see why do that is considered ugly.
>> At this moment we have the following courses of action:
>> 1. Remove FacesConfigurationResource interface partially, because it is
>> still too inmature and let it for myfaces core 2.0.4.
>> 2. Create another SPI interface for getFacesConfigData() (please suggest a
>> name for it, maybe FacesConfigurationMergerProvider?) and remove this method
>> form FacesConfigurationResource. Apply the patch on MYFACES-2998 seems to be
>> in this direction, but forget the reason why it is wanted to expose
>> getFacesConfigData() to the container.
>> 3. Apply something like MYFACES-2998 patch, and refactor this one later in
>> myfaces core 2.0.4
>> Suggestions are welcome.
>> regards,
>> Leonardo Uribe
> --
> Ivan

Jakob Korherr


View raw message