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

[Andy Schwartz]

Hey Leonardo -

Thanks for the additional details.  I wanted to respond to the first 
problem that you outline.  Will follow up on the other problems soon.  
(BTW, I haven't had a chance to look at your most recent email.  Will do 
that soon as well.)

On 10/12/10 8:20 PM, Leonardo Uribe wrote:
>
> FIRST PROBLEM: Fix Resource Resolver
>       
>     The problem happen only when suffix mapping is used.

Ah, that explains a lot. We are using prefix mappings. :-)


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

I get it now.  We've got two different ways to answer the question of 
whether a resource exist:

1.  ResourceResolver.resolveUrl(): currently only used by Facelets.
2.  ExternalContext.getResource(): used elsewhere

If these return inconsistent results, we have problems.  We can solve 
the problem by extending our use of #1 to cases where we currently rely 
on #2.


>     (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.)

Yep.  7.5.2 uses very generic terms - doesn't explicitly state how to 
check whether a viewId actually exists.

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

I am interested in what you had in mind, ie. where would we move this logic?

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

Oh, interesting.  This does seem extremely important.  We've got two 
types of viewIds:

- Raw: deriveViewId() logic has not yet been applied
- Derived: deriveViewId() has been applied

ViewHandler.getViewDeclarationLanguage() needs to be specific about what 
type of viewId is expected.  Based on the above code snippets, it looks 
like Mojarra's implementation expects raw viewIds.  MyFaces expects 
derived viewIds.  However, as you said, for the most part, Mojarra is 
passing derived viewIds into ViewHandler.getViewDeclarationLanguage().  
This means that Mojarra is repeatedly re-deriving viewIds on each 
ViewHandler.getViewDeclarationLanguage() call.   Since there can be many 
calls to this method per-request, and since checking for the physical 
existence of a view isn't always cheap, this seems bad.

Perhaps the spec needs to be updated to make it clear that 
ViewHandler.getViewDeclarationLanguage() operates on derived viewIds?

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

Right: ViewHandler.deriveViewId().


Getting back to the question of how to check whether a particular view 
exists...  I agree that at a minimum in the Facelets case we need to be 
consistent about always including the ResourceResolver in this 
decision.  The new existence check method on 
ViewDeclarationLanguageFactory() that you proposed would definitely 
help.  Actually, something like this would be tremendously helpful for 
us here (ADF Faces), since at the moment we have logic scattered across 
ResourceResolver + ExternalContext.  This split approach is particularly 
awkward since ExternalContext.getResource() is used more generically 
than ResourceResolver - ie. ExternalContext.getResource() may be called 
for arbitrary resource retrieval, whereas we are particularly interested 
in view resolving.

I wanted to mention an alternate to the ViewDeclarationLanguageFactory 
approach, though I am not sure whether it is an improvement.  In our 
case, we wouldn't mind using the ResourceResolver generically across 
VDLs, ie. we wouldn't mind seeing ResourceResolver promoted to a first 
class/VDL-agnostic API.  Our ResourceResolver implementation is used to 
serve up resources from the class path as well as from remote 
repositories (eg. a database).  This ability could be useful not just 
for Facelets, but for arbitrary VDLs.  It would be nice if we could 
implement this behavior one time in a standard/portable way rather than 
requiring a new solution for each VDL.

If we had access to an application-level (VDL-independent) 
ResourceResolver, the JSF implementations could use this to check for 
the existence of views when deriving view ids (and elsewhere) without 
having to go through the ViewDeclarationLanguageFactory.

I don't think it would be especially hard to make the ResourceResolver 
more general-purpose.  We would likely want to add a new version of this 
class outside of the Facelets packaging and possibly re-parent the 
existing Facelets ResourceResolver to the new API.  Or, at least, we 
would want to provide the ability to transparently adopt the old 
Facelets ResourceResolver to the new generic ResourceResolver.

Another small change that might be helpful... in order to answer the 
question of whether or not a resource exists, we currently need to 
literally cough up an URL.  In our case, we might be able to optimize if 
we just had to test for existence as opposed to return a reference to 
the resource.  Perhaps we could consider adding exists() method to the 
generic ResourceResolver.  By default this would just do "resolveUrl() 
!= null", but this could be optimized in other implementations.

Of course, if we generalize the ResourceResolver in this way and call 
this from ViewHandler.deriveViewId(), this does introduce a new 
assumption: that ViewDeclarationLanguages will also use the 
ResourceResolver for loading views (like Facelets does).  One case where 
this isn't necessarily true is the JSP ViewDeclarationLanguage, since 
the JSP engine itself is responsible for loading the view.  FWIW, in our 
particular case, our JSP engine actually does contain resource resolving 
logic that matches our ResourceResolver implementation.  As such, we 
would prefer to use the ResourceResolver to check for view existence 
even in the JSP case, though I imagine our requirements are a bit 
unusual in this area.

Thoughts?

 Andy