state saving change proposal

[Michael Concini <mc...@gmail.com>]

All,
I've got a bug that one of our testers found where we behave differently 
from the RI.  Let me lay out the situation and the two options I have 
come up with for solutions.  This applies to at least the 1.2.x and 
2.0.x releases.

The application has a simple logout.jsp that we get to via a navigation 
rule (e.g. when outcome == "logout").  This is a simple JSP, no JSF 
tags, which contains a scriptlet that clears login data, invalidates the 
session and then redirects to a plain html page.  Its a fairly simple 
scenario that works without issue on the RI.

On MyFaces, when processing logout.jsp, we end up trying to create a new 
session because we try to save the state.  This isn't kosher since the 
response has been committed already so you'll get an 
IllegalStateException on Tomcat.

After much debugging and doing some black box testing of the RI behavior 
this week, I think I've finally figured out that the difference is that 
the RI appears to only be doing state saving if there is a JSF form tag 
on the page.  By simply adding a form tag to logout.jsp I can recreate 
the IllegalStateException.

I've got two potential solutions for this.  Either one should work but 
I'm interested to know if there is a preference in the community about 
which way to go or if there is another alternative to consider.

1) add a check to make sure that either there is session or if one needs 
to be created that the response has not been committed in 
JspStateManagerImpl.saveSerializedView().  If there is no session and 
the response has been committed, just return null back through 
StateManger.saveView() which indicates no state information to be saved.

2) do something in the form renderer to indicate that the state needs to 
be saved, maybe setting a request attribute that can be checked in 
saveView to determine whether to actually save state or just return null.

I've tested out the first option already and it does resolve the issue 
and in some initial testing does not cause any regressions elsewhere.  
Obviously a thorough testing would be done prior to actually integrating 
the change if we go that way.  I have not coded up a fix yet for the 
second option so I haven't done any testing on it yet.

As far as my preference, I'd lean a little towards the first as it seems 
to be a little less at risk of breaking anything else since we're really 
limiting the instance where we don't try to save state to only if there 
is both no valid current session and the response has already been 
committed.

The second change seems a little higher risk, but also potential higher 
reward.  This change would from what I can tell bring us into more 
consistent behavior with the RI and also have a potential slight 
performance gain by not performing state saving on JSF pages that do not 
contain form elements (as well as any non-JSF JSP pages that have been 
pushed through the JSF lifecycle due to navigation rules, etc).  The 
risk of course being that the change affects many more pages since it 
will prevent state saving on any pages that don't contain a form element.

The other question then would be whichever option we go with, should we 
apply it to both 1.2.x and 2.0.x or just to the 2.0.x releases?  I'm not 
sure that I would want to apply #2 into such a mature stream as 1.2 but 
would feel more comfortable applying #1 to both.

Apologies for the wordiness here.

Regards,
Mike

Re: state saving change proposal

[Michael Concini <mc...@gmail.com>]

Leo,

So I just did a little digging through the facelets code, and it looks 
like in the facelets case, we already are doing something similar to 
what you are proposing.  ViewHandlerImpl's writeState method looks like 
it only gets called by the encodeEnd on HtmlFormRendererBase.  So I 
think we are probably ok in the facelets case.

The problem as far as I can tell would only be one that affects JSP.  
This might actually make the change simpler as we might be able to 
re-use some of the logic from facelets and update the JSP view handler 
to function the same way.   The key appears to be in the call to 
StateWriter.getCurrentInstance().writingState(); and the subsequent 
check by the facelets VDL to StateWriter.isStateWritten() to determine 
whether or not to save the state.  We don't do such a call in the 
JSPViewHandler.

regards,
Mike

On 6/11/2010 12:29 PM, Leonardo Uribe wrote:
> Hi
>
> In theory, if there is no form there is no state saved, so the ajax 
> received will not have the state and no postback could be sent. Ok, if 
> the view state is saved in some different way by other alternate ajax 
> component, the point in common is that any component that writes the 
> state should call viewHandler.writeState() method (and 
> stateManager.writeState() method).
>
> Instead do something on the renderer, I think it is possible to do 
> something on viewHandler.writeState() or stateManager.writeState() and 
> keep the same proposed behavior. In jsf 2.0, we have to keep into 
> account the method getViewState().
>
> regards,
>
> Leonardo Uribe
>
> 2010/6/11 Werner Punz <werner.punz@gmail.com 
> <ma...@gmail.com>>
>
>     What about ajax and http get which perse do not post a form in all
>     cases
>     but can post data back into the lifecycle?
>
>     OK,http get is handled, but still there is ajax which can trigger
>     a  post without a form.
>
>
>
>     Werner
>
>
>     Am 11.06.10 16:13, schrieb Martin Marinschek:
>
>         Hi Mike,
>
>             2) do something in the form renderer to indicate that the
>             state needs to be
>             saved, maybe setting a request attribute that can be
>             checked in saveView to
>             determine whether to actually save state or just return null.
>
>
>         I would lean towards option 2 - I don´t see how the saved
>         state would
>         be used at all if there is no form on the page - there cannot be a
>         postback to it, so this memory is anyways wasted.
>
>         best regards,
>
>         Martin
>
>
>
>

Re: state saving change proposal

[Leonardo Uribe <lu...@gmail.com>]

Hi

In theory, if there is no form there is no state saved, so the ajax received
will not have the state and no postback could be sent. Ok, if the view state
is saved in some different way by other alternate ajax component, the point
in common is that any component that writes the state should call
viewHandler.writeState() method (and stateManager.writeState() method).

Instead do something on the renderer, I think it is possible to do something
on viewHandler.writeState() or stateManager.writeState() and keep the same
proposed behavior. In jsf 2.0, we have to keep into account the method
getViewState().

regards,

Leonardo Uribe

2010/6/11 Werner Punz <we...@gmail.com>

> What about ajax and http get which perse do not post a form in all cases
> but can post data back into the lifecycle?
>
> OK,http get is handled, but still there is ajax which can trigger
> a  post without a form.
>
>
>
> Werner
>
>
> Am 11.06.10 16:13, schrieb Martin Marinschek:
>
>  Hi Mike,
>>
>>  2) do something in the form renderer to indicate that the state needs to
>>> be
>>> saved, maybe setting a request attribute that can be checked in saveView
>>> to
>>> determine whether to actually save state or just return null.
>>>
>>
>> I would lean towards option 2 - I don´t see how the saved state would
>> be used at all if there is no form on the page - there cannot be a
>> postback to it, so this memory is anyways wasted.
>>
>> best regards,
>>
>> Martin
>>
>>
>
>

Re: state saving change proposal

[Werner Punz <we...@gmail.com>]

What about ajax and http get which perse do not post a form in all cases
but can post data back into the lifecycle?

OK,http get is handled, but still there is ajax which can trigger
a  post without a form.



Werner


Am 11.06.10 16:13, schrieb Martin Marinschek:
> Hi Mike,
>
>> 2) do something in the form renderer to indicate that the state needs to be
>> saved, maybe setting a request attribute that can be checked in saveView to
>> determine whether to actually save state or just return null.
>
> I would lean towards option 2 - I don´t see how the saved state would
> be used at all if there is no form on the page - there cannot be a
> postback to it, so this memory is anyways wasted.
>
> best regards,
>
> Martin
>

Re: state saving change proposal

[Martin Marinschek <mm...@apache.org>]

Hi Mike,

> 2) do something in the form renderer to indicate that the state needs to be
> saved, maybe setting a request attribute that can be checked in saveView to
> determine whether to actually save state or just return null.

I would lean towards option 2 - I don´t see how the saved state would
be used at all if there is no form on the page - there cannot be a
postback to it, so this memory is anyways wasted.

best regards,

Martin

-- 

http://www.irian.at

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

Professional Support for Apache MyFaces