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 <[email protected]>

>
>
> 2010/12/6 David Jencks <[email protected]>
>
>>
>> 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
>

Reply via email to