7:20 p.m.
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