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

[Martin Marinschek]

+1

from me as well,

best regards,

Martin

On Fri, Oct 15, 2010 at 6:33 AM, Leonardo Uribe <lu4242 at gmail.com> wrote:
> Hi Andy
>
> 2010/10/13 Andy Schwartz <andy.schwartz at oracle.com>
>>
>> Here I have to mention something
>> 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 my opinion, yes.
>
>>>
>>>       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().
>>
>
> Right now, ViewHandler.deriveViewId() is used by NavigationHandler and in
> that use case it is necessary the viewId is being checked for existence, or
> in other words, if the physical resource tied to the viewId exists or not
> (for preemptive navigation). Here we have two options:
>
> - Add a method like ViewHandler.getViewDeclarationLanguageFromRawViewId
> - Add another method that do the same as deriveViewId() but do not check for
> viewId existence, to allow call it from Restore View Phase and do not break
> existing TCK tests.
>
>>
>> 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.
>>
>
> In theory just override or wrap ResourceResolver should be enough, and we
> should prevent override ExternalContext.getResource().
>
>>
>> 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 agree with this one too. In theory, the VDL is responsible to build views,
> so it should use some strategy to retrieve them (it is the "client"). Use a
> strategy per VDL seems theorically reasonable but in practice, since each
> VDL only handle files mutually exclusive with other VDL's, use a shared
> ResourceResolver per all VDL is ok in my opinion.
>
>>
>> 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.
>>
>
> In my opinion, it should be an application scope object and developers
> should be able to add wrappers through faces-config.xml inside <application>
> tag. It would be great!.
>
> Anyway, there is still open the question about if the spec should allow
> create wrapper per VDL from faces-config.xml. There are some use cases when
> this could be helpful, like allow extend facelets an add custom "attached
> objects".
>
>>
>> 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.
>>
>
> Yes it could be, but anyway we need the method existsViewId on
> ViewDeclarationLanguage (and that one will call the hypothetical method
> ResourceResolver.exists() ). Why? because the VDL should be responsible to
> indicate if the viewId is valid before check for existence. In the patch
> proposed this check is done before call ResourceResolver stuff.
>
>>
>> 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?
>>
>
> +1 on fix all that stuff before get 2.1 out.
>
> best regards,
>
> Leonardo Uribe
>
>



-- 

http://www.irian.at

Your JSF powerhouse -
JSF Consulting, Development and
Courses in English and German

Professional Support for Apache MyFaces