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-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