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