struts-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Niall Pemberton" <niall.pember...@gmail.com>
Subject Re: svn commit: r552396 - /struts/struts1/trunk/core/src/main/java/org/apache/struts/chain/commands/AbstractPopulateActionForm.java
Date Fri, 06 Jul 2007 04:58:55 GMT
On 7/6/07, Paul Benedict <pbenedict@apache.org> wrote:
> The ticket issue shows the conversation starting without a form and then
> growing to include the form. So I believe it was appropriate to address
> both situations, which I think are okay.

I had missed that in the comments :(

> First, in all the years of
> development of Struts, I have never come across one user mail or program
> code where people are asking to retrieve values on cancellation. Please
> find one for me so I can be corrected. In fact, I can't think of one
> framework which allows this anyway. I put this in the "bad practice"
> department and believe this should not be supported in anyway -- and the
> current situation is an oversight. The action is canceled, the form is
> part of the action, and so the form has no job which is very much
> implied by the canceling of validation.
>
> Now granted, there seems to be concern here with you and Martin.
> However, I think this is a justified change, and going from one version
> to the next does not have to be 100% backwards compatible .. just about
> 98 or 99%, in my opinion. Remember, this codebase has no 2.0 version --
> that's done by the WW port -- so if things are going to move forward,
> and perhaps dream to fix some rather odd use cases, I find this
> appropriate just as much as Struts 2.1 alters some functionality of
> Struts 2.0.

IMO people who want to move forward should use Struts2 - Struts 1
shouldn't break compatibilty lightly - I see no great benefit to
changing how this has worked for 6 years and just pain for anyone who
has happened to rely on it. Also with the Command Chain introduced in
Struts 1.3 its possible to easily provide both new functionality and
compatibility with different Command implementations. So I'm still
against this change as it is now.

Niall

> Paul
>
> Niall Pemberton wrote:
> > I don't have a problem with setting the "cancellation" attribute where
> > there is no form - but this changes goes further than whats mentioned
> > in STR-1674 with the form no longer being populated when "cancelled".
> > This breaks compatibility with previous Struts versions and I'm
> > against it.
> >
> > Niall
> >
> > On 7/2/07, pbenedict@apache.org <pbenedict@apache.org> wrote:
> >> Author: pbenedict
> >> Date: Sun Jul  1 21:03:17 2007
> >> New Revision: 552396
> >>
> >> URL: http://svn.apache.org/viewvc?view=rev&rev=552396
> >> Log:
> >> STR-1674: Check cancellation first and possibly skip population
> >>
> >> Modified:
> >>
> >> struts/struts1/trunk/core/src/main/java/org/apache/struts/chain/commands/AbstractPopulateActionForm.java
> >>
> >>
> >> Modified:
> >> struts/struts1/trunk/core/src/main/java/org/apache/struts/chain/commands/AbstractPopulateActionForm.java
> >>
> >> URL:
> >> http://svn.apache.org/viewvc/struts/struts1/trunk/core/src/main/java/org/apache/struts/chain/commands/AbstractPopulateActionForm.java?view=diff&rev=552396&r1=552395&r2=552396
> >>
> >> ==============================================================================
> >>
> >> ---
> >> struts/struts1/trunk/core/src/main/java/org/apache/struts/chain/commands/AbstractPopulateActionForm.java
> >> (original)
> >> +++
> >> struts/struts1/trunk/core/src/main/java/org/apache/struts/chain/commands/AbstractPopulateActionForm.java
> >> Sun Jul  1 21:03:17 2007
> >> @@ -45,21 +45,26 @@
> >>       */
> >>      public boolean execute(ActionContext actionCtx)
> >>          throws Exception {
> >> -        // Is there a form bean for this request?
> >> +
> >> +        ActionConfig actionConfig = actionCtx.getActionConfig();
> >>          ActionForm actionForm = actionCtx.getActionForm();
> >>
> >> +        // First determine if the request was cancelled
> >> +        handleCancel(actionCtx, actionConfig, actionForm);
> >> +
> >> +        // Is there a form bean for this request?
> >>          if (actionForm == null) {
> >>              return (false);
> >>          }
> >>
> >> -        // Reset the form bean property values
> >> -        ActionConfig actionConfig = actionCtx.getActionConfig();
> >> +        // If request is cancelled, form manipulation is prevented
> >> +        if (actionCtx.getCancelled().booleanValue()) {
> >> +            return (false);
> >> +        }
> >>
> >> +        // Reset and repopulate the form bean property values
> >>          reset(actionCtx, actionConfig, actionForm);
> >> -
> >>          populate(actionCtx, actionConfig, actionForm);
> >> -
> >> -        handleCancel(actionCtx, actionConfig, actionForm);
> >>
> >>          return (false);
> >>      }

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


Mime
View raw message