Re: [jsr-314-open-mirror] [jsr-314-open] 490-XmlViews Processing JSPX files as Facelets

Hi These are the answers to the questions asked (answer inside the mail sent could make things more confusing): FIRST PROBLEM: Fix Resource Resolver AS >> In what way (the ResourceResolver) does not work? AS >> It this is broken, it definitely needs to be fixed. AS >> We need more details on exactly what behavior is broken. The problem happen only when suffix mapping is used. In few words, When this mapping is used, it is required to check if a physical resource exists, usually calling ExternalContext.getResource. Since the algorithm described on the spec does not take into account ResourceResolver, a http 404 not found response is sent back. (The reference that needs to be fixed on the spec is JSF 2.0 section 7.5.2 Default ViewHandler Implementation, when it describe the requeriments of ViewHandler.deriveViewId.) AS >> What is the issue # for this problem? AS >> Is this a Mojarra issue? MyFaces? Spec? All of the above? The issue in MyFaces is this one: https://issues.apache.org/jira/browse/MYFACES-2628 It was first mentioned on this mailing list under the topic: [jsr-314-open] javax.faces.view.facelets.ResourceResolver cannot be fully overriden Then it was added some comments on this spec issue: https://javaserverfaces-spec-public.dev.java.net/issues/show_bug.cgi?id=781 It was also found this issue on Mojarra https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=1567 Maybe we should create an independent issue for this one on the spec issue tracker AS >> We are using it (ResourceResolver) here. Now I am getting worried that AS >> maybe our code is broken. :-) Though Jason also blogged on this topic here: AS >> http://blogs.steeplesoft.com/2010/05/putting-facelets-in-a-jar/ AS >> Which makes me think that this must be minimally working. I think you are using ResourceResolver with applications that uses prefix mapping. The hack on the blog works but by accident. Try the same example, but without configure ResourceResolver and it will work, because we are overriding ExternalContext.getResource, which it is used to check if a resource exists or not. The proposal at first view, to solve this one is add a method on ViewDeclarationLanguageFactory called existsViewId(), so the algorithm on ViewHandler.deriveViewId just delegate to this one and that one delegate to different ViewDeclarationLanguage implementations. But it seems better to move all logic from section 7.5.2 ViewHandler.deriveViewId. Here I have to mention something I said on this discussion: [jsr-314-open] Should the ViewDeclarationLanguage be responsible to indicate if a viewId can be created? On mojarra, this is the code on MultiViewHandler.getViewDeclarationLanguage: public ViewDeclarationLanguage getViewDeclarationLanguage(FacesContext context, String viewId) { String actualViewId = derivePhysicalViewId(context, viewId, false); return vdlFactory.getViewDeclarationLanguage(actualViewId); } Now on myfaces is this: @Override public ViewDeclarationLanguage getViewDeclarationLanguage( FacesContext context, String viewId) { // return a suitable ViewDeclarationLanguage implementation for the given viewId return _vdlFactory.getViewDeclarationLanguage(viewId); } The difference is subtle, but very, very important. This method is called from many locations, but only once (from RestoreViewPhase) it is passed the "raw" viewId. To "derive" the physical viewId it is required to know if prefix or suffix mapping is used and if suffix mapping is used, try to check if a view "resource" exists or not with different derived ids with the extensions configured ".xhtml .jsp". In this case, MyFaces "derive" the physical viewId before call getViewDeclarationLanguage from RestoreViewPhase. In theory, we should call from RestoreViewPhase algorithm a method exposed by ViewHandler. I'm not have clear the details yet. SECOND PROBLEM: Fix suffix definition for vdls AS >> This seems like a good thing to do. Are you thinking that we'll add a new AS >> hook on ViewDeclarationLanguage that allows the VDL implementation to decide AS >> whether or not it can handle a particular view id? In theory, both myfaces and mojarra has already that, but it is left as an implementation detail. Take a look at ViewDeclarationLanguageFactory implementation and you'll see some code to decide which VDL should handle the viewId identified (note handle a viewId is different to check if the viewId exists, which it is required for the first one). AS >> Do we have a spec issue logged for this topic? Not yet. The intention is create one based on all previous discussion. THIRD PROBLEM: Fix specific suffix handling for facelets vdl BS>>>>>> In the JAR case, it doesn't care about the VDL at all, it is simply BS>>>>>> loading the requested resources out of the JAR regardless of extension. LU>>>>So, what's the point about have this configuration on a JAR, if there will be no LU>>>>resources using such extensions inside it?. If all resources used by facelets are LU>>>>inside the WAR or in the webapp directory, shouldn't that param be configured on LU>>>>web.xml or only in faces config application resource (WEB-INF/faces-config.xml). LU>>>>It's more, if I have the xml configuration proposed on JAR META-INF/faces-config.xml, LU>>>>what someone could expect is the configuration applies to case 1 (composite LU>>>>component resource files), and not globally, which it is the intention. AS>>I don't completely follow this. Could you explain? The proposal "as is" has a problem. It we have the configuration on a JAR, the way the mappings are written does not suggest which resources applies to it, or in other words, it is not clear if the resources found outside the JAR are also affected or not. As I said before, a JAR file could contain resources that could be used by facelets in two situations: 1. On META-INF/resources/[localePrefix/][libraryName/]resourceName as a composite component resource file. 2. On any directory but it should be created a class that implements javax.faces.view.facelets.ResourceResolver, so it can get the resource properly. If I put that configuration on a JAR, my first thought is this configuration only applies to the resources inside the JAR. Since ResourceResolver is affected by "global mappings", this rule should only applies to composite component definitions and related facelets, right?. But the intention is affect the "global mapping", since the resources will be loaded through ResourceResolver (I don't know your particular use case, so I'm supposing here). It this is true, it has sense to put it on a JAR, but that condition must be made explicit, using a more "suggestive" name and it should be possible to define a custom ResourceResolver from faces-config file, otherwise it will not be possible to wrap more than 2 instances (the default and the one from the web config param). LU >>>> All three issues are related. All of them should be solved. Delaying these issues LU >>>> with partial hacks only makes things worst and even harder to understand. AS >> I agree that all three issues should be solved. FWIW, I think that #3 can be AS >> solved independently of #1 and #2. I don't quite see why these issues should AS >> be tightly coupled. I also don't think that the proposal to fix #3 is a hack, AS >> but if others think that it is, we of course need to resolve these concerns. From my point of view, this one is a particular case of the second one. Why? because if you add this on faces-config.xml: <faces-config-extension> <facelets-processing> <file-extension>.jspx</file-extension> <process-as>jspx</process-as> </facelets-processing> <facelets-processing> <file-extension>.view.xml</file-extension> <process-as>xml</process-as> </facelets-processing> </faces-config-extension> You are not only saying "... process this suffix in mode ....", you also are implicitly mapping the suffix to facelets VDL. The mode is just a configuration property that the VDL must take into account to process a viewId with this suffix. Why don't do something like this (maybe you can find better names for the xml): <faces-config-extension> <vdl-configuration> <vdl-id>facelets</vdl-id> <vdl-class>xxx.yyy.zzz.MyCustomFaceletsVDLWrapper</vdl-class> <vdl-global-resource-mapping> <file-extension>.jspx</file-extension> <vdl-resource-mapping-config> <!-- Saved on key/value map or something like that --> <process-as>jspx</process-as> </vdl-resource-mapping-config> </vdl-global-resource-mapping> <vdl-global-resource-mapping> <file-extension>.view.xml</file-extension> <vdl-resource-mapping-config> <!-- Saved on key/value map or something like that --> <process-as>xml</process-as> </vdl-resource-mapping-config> </vdl-global-resource-mapping> </vdl-configuration> </faces-config-extension> It is almost the same as we have before but with the following differences: 1. Every VDL has an id (like with RenderKit) 2. There is an option to provide custom properties, that allow pass the "mode". 3. <vdl-global-resource-mapping> tag says "... all resources that match this mapping in the webapp will be processed by this VDL using these configuration params ..." 4. (Optional) Allow to provide a wrapper for a VDL implementation, allowing to extend it (maybe useful for gracelets????). Did you see the relationship between all three problems? - All of them require changes on ViewDeclarationLanguage/ViewDeclarationLanguageFactory. - Problem (1) is enough simple to solve, because it just require some code relocation that already exists, but it is necessary to update the spec. - Problem (2) is the general case of (3), so we just need to change the current patch a little bit. - To make (3) and use it as expected we need (1) fixed. I'm willing to contribute with this issue doing some code if necessary (I'll try it to see how far I can go). Suggestions are welcome. best regards, Leonardo Uribe