Hi 2010/12/7 David Jencks <[email protected]>
> > On Dec 7, 2010, at 10:12 AM, Leonardo Uribe wrote: > > Hi > > 2010/12/6 Ivan <[email protected]> > > 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 <[email protected]> > >> >> >> 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 <[email protected]> >> >>> 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 >>>> >>> >>> >> >> >> -- >> Ivan >> >> >> > >
