jsr344-experts@javaserverfaces-spec-public.java.net

[jsr344-experts] [758-ViewActions] Resolution Attempt One

From: Ed Burns <edward.burns_at_oracle.com>
Date: Thu, 28 Jul 2011 13:07:57 -0700

>>>>> On Tue, 28 Jun 2011 18:55:17 -0500, Leonardo Uribe <lu4242_at_gmail.com> said:

DA> First, the ActionListener#processAction() always calls the
DA> FacesContext#renderResponse() method before returning. To allow for
DA> sequential execution of the actions, this method has to be
DA> temporarily disabled. If the behavior of this method could be aware
DA> of the view action requirements, we wouldn't need this hack.

[...]

EB> <f:viewAction>.  Therefore, unless someone can come up with a
EB> cleaner option, we must specify that the renderResponse() be made
EB> temporarily to be a no-op, as you do now.

LU> "... sequential execution of the actions ..." sounds suspicious. In
LU> practice, in previous JSF versions that use case is not considered.
LU> Usually there is only one action per request, but in this case I don't
LU> see how can be possible multiple actions, because at the end only one
LU> navigation will take effect.

The current implementation will call
NavigationHandler.handleNavigation() for each action in the order in
which they appear in the view. The last one wins.

LU> If we have many f:viewAction on the same
LU> view, it should be mutually exclusive or do not have "action" set, and
LU> the tag syntax should enforce this. In other words, it should be a
LU> container tag of f:viewAction that specify which one will be choosen
LU> from multiple f:viewAction with "action" property set. One idea that
LU> comes to my mind is how "switch" works in java:

LU> <f:switchAction value="...">
LU> <f:viewAction case="..." action="..."/>
LU> <!-- .... -->
LU> <f:viewAction case="default" action="..."/>
LU> </f:switchAction>

While this <f:switchAction> is seductively appealing, I don't want to
include it in the spec. It's yet another way to declare navigation and
we already have two: 1. explicit navigation rules 2. implicit
navigation.

DA> Second, if the navigation that is matched is a non-redirect
DA> navigation to a different view id, it's necessary to rewind the
DA> lifecycle and replay it on the new view Id [1]. There's an important
DA> reason for this requirement. We need the view actions on the target
DA> view ID to be invoked. If we didn't rewind the lifecycle, then the
DA> view ID would be immediately rendered. That breaks the requirements.

DA> Now, we could solve #2 a couple different ways.

DA> Alt #1: We could imply a redirect for any navigation rule matched by
DA> a view action. That would certainly solve our problem of having to
DA> manually manipulate the lifecycle. The only downside to this
DA> approach is that the developer may intend to have an internal
DA> forward (server-side redirect) so as to cloak the URL in the
DA> navigation bar. I don't know how important of a requirement that
EB> is.

EB> I think we should go with this option.  The <f:viewAction> usage
EB> contract must clearly state that a redirect will occur if a match
EB> occurs.

LU> Sounds good, after all that's similar to a button click. In theory, to
LU> check this a call to navigationHandler.getNavigationCase() and then a
LU> check to isRedirect() will do the job.

Good point. Nice that we are using ConfigurableNavigationHandler in the
spec itself now.

AS> The only attribute that seems specific to that component is
AS> "onPostback", because metadata action processed for both postback
AS> and get requests. Also, I suggest to append "phase" attribute for
AS> all command components, not only to ViewActions. For example,
AS> command processed after validation phase suitable to verify user
AS> input without model update.

EB> While I understand the rationale for the "phase" attribute on existing
EB> components, I can't abide introducing that complexity in this case.
EB>

IO> As <f:viewAction> is close to <h:commandButton/Link> (It's an
IO> ActionSource2 anyway), I think:

IO> - We should use the 'rendered' and 'disabled' attributes instead of
IO> an 'if'. While I can see the beauty of the 'if', especially as
IO> nothing "gets rendered", I think most developers are used to use
IO> 'rendered' to "disable" a component.

IO> - viewAction should have an 'actionListener'-attribute too. (And it
IO> should also allow for "child-listener-components"

EB> I was uncertain about the naming of the "if" attribute as well.
EB> Perhaps we should go with "rendered".

LU> In theory there is not difference between both, but in my mind
LU> "rendered" property is tied to the visual part of the component. I
LU> personally like "if".

Noted, but I'm still going to write the spec as "rendered" now.

LU> In theory if f:viewAction is ActionSource2 this code should be
LU> valid:

LU> <f:viewAction ...>
LU>     <f:actionListener ... />
LU> </f:viewAction ...>

LU> But checking the interface, there are two methods:
LU>
LU>     public boolean isImmediate();
LU>
LU>     public void setImmediate(boolean immediate);

LU> that does not have sense for f:viewAction, since this is activated
LU> as soon as the metadata is built (restore view phase). Even if use
LU> ActionEvent and ActionListener interfaces has sense, it is not an
LU> ActionSource2 fully compliant from a strict point of view.

EB> Doch, you can see from the <f:viewAction> vdldocs, which were taken
EB> from the <s:viewAction> vdldocs, that there is an immediate
EB> attribute.

EB> http://javaserverfaces-spec-public.java.net/nonav/snapshots/jsf-spec-2.2-SNAPSHOT-20110616/vdldocs/facelets/f/viewAction.html

EB> so immediate has sense, since this works in a similar way as any
EB> other command button/link.

EB> Here are the conclusions from this message.
EB>
EB> 1. Continue to do the "wrap the FacesContext to make renderResponse a
EB> no-op before calling ActionListener.processAction()" thing.
EB>
EB> 2. Clearly state that a redirect must happen in the event of a
EB> navigation rule match.
EB>
EB> 3. Rename "if" attribute to "rendered".
EB>
EB> 4. Allow ActionListener children.

LU> Two questions:

LU> 1. If f:viewAction implements ActionSource2, shouldn't implement
LU> actionListener property too?

Yes. I thought that was implied by my "4. Allow ActionListener
children" but it is better to explicitly state it.

LU> 2. Isn't "immediate" the same thing as "phase" property? after all,
LU> "immediate" specify if the action should be on "apply request value"
LU> or "invoke application" phase

Yes, it is, but it is better to not introduce a new name for it.

Ed

-- 
| edward.burns_at_oracle.com | office: +1 407 458 0017
| homepage:               | http://ridingthecrest.com/