Hi!

Very interesting problem!
In my oppinion is clearly a bug!

On 1/22/06, Paul Benedict <[EMAIL PROTECTED] > wrote:
>
> >>I can't think of a good reason it wouldn't call validate() too, like any
> other request, just
> because the action was canceled.  Like I said, maybe someone can come up
> with a reasonable
> explanation for that behavior, but I can't see it :)
>
> There is a legitimate case: when an form can be cancelled, you do want to
> skip client-side and
> server-side validation. That's just fine because in these case you do want
> to call the cancelled()
> method from DispatchAction, dump out any state you collected, and go to
> the appropriate forward.
>
> I want to make sure we don't lose focus here. The problem is not that
> cancelled doesn't validate,
> but that ANY action can have its validation bypassed. This is not
> semantically correct for some
> actions, especially GET actions whose sole job is to retrieve data without
> any state except for
> the request parameters they receive. Usually people cancel form POST
> input, but not GET input
> because those could be links in emails, bookmarks, etc. Explain the
> semantics of canceling a
> bookmark or a link from an email? It's pretty difficult to think of a
> general case.
>
> So, as I've said before, I do believe there needs to be a switch that
> determines when by-passing
> validation IS A VALID USE CASE. Having that feature always be on is
> logically wrong and creates a
> world where many stateless requests which use form validation to defend
> against bad user input is
> suddenly defeated.


In my oppinion it isn't ahndled correctly and I don't think that it should
be solved by configuration
script.
If the user presses Cancel then the processing should go on another path.
It doesn't make sense to populate and to validate if cancel was pressed.
I would like to go one step further and say that it doesn't make sense to
call execute either!
If cancel was pressed a cancel() method should be called.
Currently in an Action's execute you would handle cancellation like this:

if (isCancelled(request)) {
  // process the cancellation
      // clear data from session
      // forward to the "cancel"
}
// do the normal logic of the execute.

If we look at the processing of the cancellation the code looks exactly like
you had
another execute() in the Action's "normal" execute.  It does some processing
and then
it forwards somewhere else. (Beside this Struts relies on the
developer to put in the if (isCancelled(request)) otherwise it will just
call execute()
which is a problem in my oppinion).
So it would make sense to have a execute-like cancel() method that is called
if the Action was cancelled just like in DispatchAction.

Cancelable Actions (independently on the Action type: normal Actions,
DispatchActions) could even implement a Cancelable interface with a cancel
method.

However for Actions when you traditionaly have only one execute method I
would always
have a CancelAction and in its execute method I would handle the
cancellation.

This cancel feature was probably added with DispatchAction in mind(for which
it works almost perfectly (it shouldn't populate the form in my oppinion))
and to support in Action too they added
this isCancelled() hack which migt look cool but hacks usually break things
:-)



> Paul



Tamas

Reply via email to