Tapestry
  1. Tapestry
  2. TAPESTRY-1972

Client persistence bug - user typing lost

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Invalid
    • Affects Version/s: 5.0.6
    • Fix Version/s: None
    • Component/s: tapestry-core
    • Labels:
      None
    • Environment:
      Safari, OS X, JBoss 4.2.1

      Description

      The user loses the changes they typed into an "input" or "edit" page if an error is recorded by onSuccess() AND the page is dealing with a client persisted object. The page is redisplayed OK, with error, but with the previous values!!!

      The user's changes aren't lost if
      (a) I move the work that finds the error from onSuccess() into onValidate() - this fixes all TextField components but does not fix Select, DateField, checkbox, or expansions; or
      (b) I add a simple client-persisted field to the page - remarkably this fixes; or
      (c) I replace client persistence with session persistence.

      To illustrate the problem, it's just like the example from http://tapestry.apache.org/tapestry5/tapestry-core/guide/validation.html, which contains these excerpts:

      @Persist
      private String _userName;

      private String _password;

      String onSuccess()
      {
      if (!_authenticator.isValid(_userName, _password))

      { _form.recordError(_passwordField, "Invalid user name or password."); return null; }

      return "PostLogin";
      }

      except that instead of a single field, _userName, I am persisting a whole object, _user:

      @Persist("client")
      private User _user;

      and in the template we refer to its fields, eg:

      <input t:type="TextField" t:id="firstName" value="user.firstName" ...

      If the user types a value into firstName, but the onSuccess() method records an error, then the user loses what they typed.

      However, if I add another field to the page then everything works!

      @Persist("client")
      private String _aField;

      <input t:type="TextField" t:id="aField" value="aField" ...

      Alternatively, it works if I do either of the other 2 things - use session persistence or move all logic into onValidate().

        Issue Links

          Activity

          Hide
          Geoff Callender added a comment -

          Here's a fuller example. As it stands it demonstrates option (a). To try option (b) just un-comment _note and its getters and setters in the java and the one line in the template. To try option (c) just change @Persist("client") to @Persist.

          <html xmlns:t="http://tapestry.apache.org/schema/tapestry_5_0_0.xsd">
          <t:form t:id="form">
          To cause a server-side validation error, type Joe into first name.<br/>
          <t:errors /><br/>

          Salutation: <select t:id="salutation" t:type="Select" value="dude.salutation" model="salutations"/> ... $

          {dude.salutation}

          <br/>
          First Name: <input t:id="firstName" t:type="TextField" value="dude.firstName" size="20" validate="required,maxLength=20" /> ... $

          {dude.firstName}

          <br/>
          Last Name: <input t:id="lastName" t:type="TextField" value="dude.lastName" size="20" validate="required,maxLength=20" /> ... $

          {dude.lastName}

          <br/>
          Expiry Date: <input t:id="expiryDate" t:type="DateField" value="dude.expiryDate" format="$

          {dateFieldFormat}

          "/> ... $

          {dude.expiryDate}

          <br/>
          Active: <input t:id="active" t:type="checkbox" value="dude.active"/> ... $

          {dude.active}

          <br/>
          <!--
          Note: <input t:id="note" t:type="TextField" value="note" size="20" validate="required,maxLength=20" /> ... $

          {note}

          <br/>
          -->

          <br/>
          <input type="submit" value="Save" class="positive"/><br/>
          <t:pagelink page="Start">To Start</t:pagelink>
          </t:form>
          </html>

          package xyz.pages;

          import java.util.Date;

          import xyz.entity.Dude;

          import org.apache.tapestry.annotations.Component;
          import org.apache.tapestry.annotations.Persist;
          import org.apache.tapestry.corelib.components.Form;

          public class DudeEdit {

          @Persist("client")
          private Dude _dude;

          // @Persist("client")
          // private String _note;

          @Component(id = "form")
          private Form _form;

          void setupRender() {
          if (!_form.getHasErrors())

          { _dude = new Dude("Mr", "John", "Citizen", true, new Date()); }

          }

          void onValidate() {
          // Imitate an error
          if (_dude.getFirstName().equals("Joe"))

          { _form.recordError("First name cannot be Joe."); return; }

          }

          String onSuccess()

          { return "Start"; }

          void cleanupRender()

          { _form.clearErrors(); }

          public String[] getSalutations()

          { return Dude.SALUTATIONS; }

          public String getDateFieldFormat()

          { return "%d/%m/%Y"; }

          public Dude getDude()

          { return _dude; }

          public void setDude(Dude dude)

          { _dude = dude; }

          // public String getNote()

          { // return _note; // }

          //
          // public void setNote(String note)

          { // _note = note; // }

          }

          package xyz.entity;

          import java.io.Serializable;
          import java.util.Date;

          @SuppressWarnings("serial")
          public class Dude implements Serializable {
          static public final String[] SALUTATIONS =

          { "", "Ms", "Mrs", "Mr", "Dr", "Prof" }

          ;

          private String _salutation;
          private String _firstName;
          private String _lastName;
          private boolean _active;
          private Date _expiryDate;

          public Dude() {

          }

          public Dude(String salutation, String firstName, String lastName, boolean active, Date expiryDate)

          { _salutation = salutation; _firstName = firstName; _lastName = lastName; _active = active; _expiryDate = expiryDate; }

          public String getFirstName()

          { return _firstName; }

          public void setFirstName(String firstName)

          { _firstName = firstName; }

          public String getLastName()

          { return _lastName; }

          public void setLastName(String lastName)

          { _lastName = lastName; }

          public boolean isActive()

          { return _active; }

          public void setActive(boolean active)

          { _active = active; }

          public String getSalutation()

          { return _salutation; }

          public void setSalutation(String salutation)

          { _salutation = salutation; }

          public Date getExpiryDate()

          { return _expiryDate; }

          public void setExpiryDate(Date expiryDate)

          { _expiryDate = expiryDate; }

          }

          Show
          Geoff Callender added a comment - Here's a fuller example. As it stands it demonstrates option (a). To try option (b) just un-comment _note and its getters and setters in the java and the one line in the template. To try option (c) just change @Persist("client") to @Persist. <html xmlns:t="http://tapestry.apache.org/schema/tapestry_5_0_0.xsd"> <t:form t:id="form"> To cause a server-side validation error, type Joe into first name.<br/> <t:errors /><br/> Salutation: <select t:id="salutation" t:type="Select" value="dude.salutation" model="salutations"/> ... $ {dude.salutation} <br/> First Name: <input t:id="firstName" t:type="TextField" value="dude.firstName" size="20" validate="required,maxLength=20" /> ... $ {dude.firstName} <br/> Last Name: <input t:id="lastName" t:type="TextField" value="dude.lastName" size="20" validate="required,maxLength=20" /> ... $ {dude.lastName} <br/> Expiry Date: <input t:id="expiryDate" t:type="DateField" value="dude.expiryDate" format="$ {dateFieldFormat} "/> ... $ {dude.expiryDate} <br/> Active: <input t:id="active" t:type="checkbox" value="dude.active"/> ... $ {dude.active} <br/> <!-- Note: <input t:id="note" t:type="TextField" value="note" size="20" validate="required,maxLength=20" /> ... $ {note} <br/> --> <br/> <input type="submit" value="Save" class="positive"/><br/> <t:pagelink page="Start">To Start</t:pagelink> </t:form> </html> package xyz.pages; import java.util.Date; import xyz.entity.Dude; import org.apache.tapestry.annotations.Component; import org.apache.tapestry.annotations.Persist; import org.apache.tapestry.corelib.components.Form; public class DudeEdit { @Persist("client") private Dude _dude; // @Persist("client") // private String _note; @Component(id = "form") private Form _form; void setupRender() { if (!_form.getHasErrors()) { _dude = new Dude("Mr", "John", "Citizen", true, new Date()); } } void onValidate() { // Imitate an error if (_dude.getFirstName().equals("Joe")) { _form.recordError("First name cannot be Joe."); return; } } String onSuccess() { return "Start"; } void cleanupRender() { _form.clearErrors(); } public String[] getSalutations() { return Dude.SALUTATIONS; } public String getDateFieldFormat() { return "%d/%m/%Y"; } public Dude getDude() { return _dude; } public void setDude(Dude dude) { _dude = dude; } // public String getNote() { // return _note; // } // // public void setNote(String note) { // _note = note; // } } package xyz.entity; import java.io.Serializable; import java.util.Date; @SuppressWarnings("serial") public class Dude implements Serializable { static public final String[] SALUTATIONS = { "", "Ms", "Mrs", "Mr", "Dr", "Prof" } ; private String _salutation; private String _firstName; private String _lastName; private boolean _active; private Date _expiryDate; public Dude() { } public Dude(String salutation, String firstName, String lastName, boolean active, Date expiryDate) { _salutation = salutation; _firstName = firstName; _lastName = lastName; _active = active; _expiryDate = expiryDate; } public String getFirstName() { return _firstName; } public void setFirstName(String firstName) { _firstName = firstName; } public String getLastName() { return _lastName; } public void setLastName(String lastName) { _lastName = lastName; } public boolean isActive() { return _active; } public void setActive(boolean active) { _active = active; } public String getSalutation() { return _salutation; } public void setSalutation(String salutation) { _salutation = salutation; } public Date getExpiryDate() { return _expiryDate; } public void setExpiryDate(Date expiryDate) { _expiryDate = expiryDate; } }
          Hide
          Howard M. Lewis Ship added a comment -

          I think this is related to TAPESTRY-1941. It may be a duplicate.

          Show
          Howard M. Lewis Ship added a comment - I think this is related to TAPESTRY-1941 . It may be a duplicate.
          Hide
          Geoff Callender added a comment -

          Further testing found that when clientValidation="false" then the symptoms will also occur when a validator in the template returns an error.

          Show
          Geoff Callender added a comment - Further testing found that when clientValidation="false" then the symptoms will also occur when a validator in the template returns an error.
          Hide
          Howard M. Lewis Ship added a comment -

          By the time the Form invokes onSuccess(), it will have cleared the ValidationTracker.

          A work around would be do use the onValidate() event handler method instead of onSuccess(); you can get the Form instance and query it to see if it has validation errors.

          The question is: where is the correct place to clear the ValidationTracker after the success event is fired? Tricky.

          Show
          Howard M. Lewis Ship added a comment - By the time the Form invokes onSuccess(), it will have cleared the ValidationTracker. A work around would be do use the onValidate() event handler method instead of onSuccess(); you can get the Form instance and query it to see if it has validation errors. The question is: where is the correct place to clear the ValidationTracker after the success event is fired? Tricky.
          Hide
          Howard M. Lewis Ship added a comment -

          Perhap the right thing would be to not allow errors to be recorded from a "success" event handler method; require that errors be recorded earlier, in a "validate" event handler as your "workaround" states.

          Show
          Howard M. Lewis Ship added a comment - Perhap the right thing would be to not allow errors to be recorded from a "success" event handler method; require that errors be recorded earlier, in a "validate" event handler as your "workaround" states.
          Hide
          Geoff Callender added a comment -

          I'm tending to agree.

          An alternative that works - ie. that allows onSuccess() to return null, redisplaying the page with errors AND WITHOUT undoing your input - is to use @Persist("flash") on the object or fields of the page. For tidiness you might also nullify them in cleanupRender(). However, that alternative seems clunky and less-than-obvious to the maintenance programmer who will follow.

          In the meantime I confess I'm growing used to putting everything that could create error messages into onValidate(). The following actually reads quite sanely!

          void onValidateForm()
          {
          if (!_authenticator.isValid(_userName, _password))

          { _form.recordError(_passwordField, "Invalid user name or password."); }

          }

          String onSuccess()

          { return "PostLogin"; }
          Show
          Geoff Callender added a comment - I'm tending to agree. An alternative that works - ie. that allows onSuccess() to return null, redisplaying the page with errors AND WITHOUT undoing your input - is to use @Persist("flash") on the object or fields of the page. For tidiness you might also nullify them in cleanupRender(). However, that alternative seems clunky and less-than-obvious to the maintenance programmer who will follow. In the meantime I confess I'm growing used to putting everything that could create error messages into onValidate(). The following actually reads quite sanely! void onValidateForm() { if (!_authenticator.isValid(_userName, _password)) { _form.recordError(_passwordField, "Invalid user name or password."); } } String onSuccess() { return "PostLogin"; }
          Hide
          Howard M. Lewis Ship added a comment -

          Use the onValidate() method to record such errors; by the time the success event is triggered, the ValidationTracker has been cleared.

          Show
          Howard M. Lewis Ship added a comment - Use the onValidate() method to record such errors; by the time the success event is triggered, the ValidationTracker has been cleared.
          Hide
          Vjeran Marcinko added a comment -

          Is there some special reasons why ValidationTracker input values have to be cleared after success event (or any other)?

          My experience is that in large majority of cases, whenever someone stays on the same form page after submit, regardless whether it was error or not, I want to see all those input values still there.
          And it all works except when error are caught in onSuccess

          And I think people forget that some actions throw checked exceptions.
          Example is saving user:

          UserDao.save(User user) throws UsernameReservedException;

          I think most of the people would assume to call this final action after all possible fields have been validated, but there is no way one can find out if "username" has been reserved without calling service layer action beforehand. Normal place to call this action would be in onSuccess method. And error still have to be caught and displayed in case of exception thrown (I hope none will seriously suggest that right aproach would be to separate checking of username in separate service layer action to be called in onValidate).

          Other cases when input should be kept is when I display search result table on same page where search form is. This way it is great to keep all those values presented to člet user know what search criteria have been.

          Maybe in some rare cases one would want to stay on same page and clear Validationtracker input values, but then he could call explicitly validationTracker.clear() or whatever.
          I agree that this whole problem can possibly be overcome by persisting form model instance, but that is neither the simpliest nor righteouss aproach IMO.

          Thoughts?

          Show
          Vjeran Marcinko added a comment - Is there some special reasons why ValidationTracker input values have to be cleared after success event (or any other)? My experience is that in large majority of cases, whenever someone stays on the same form page after submit, regardless whether it was error or not, I want to see all those input values still there. And it all works except when error are caught in onSuccess And I think people forget that some actions throw checked exceptions. Example is saving user: UserDao.save(User user) throws UsernameReservedException; I think most of the people would assume to call this final action after all possible fields have been validated, but there is no way one can find out if "username" has been reserved without calling service layer action beforehand. Normal place to call this action would be in onSuccess method. And error still have to be caught and displayed in case of exception thrown (I hope none will seriously suggest that right aproach would be to separate checking of username in separate service layer action to be called in onValidate). Other cases when input should be kept is when I display search result table on same page where search form is. This way it is great to keep all those values presented to člet user know what search criteria have been. Maybe in some rare cases one would want to stay on same page and clear Validationtracker input values, but then he could call explicitly validationTracker.clear() or whatever. I agree that this whole problem can possibly be overcome by persisting form model instance, but that is neither the simpliest nor righteouss aproach IMO. Thoughts?

            People

            • Assignee:
              Howard M. Lewis Ship
              Reporter:
              Geoff Callender
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development