Tapestry 5
  1. Tapestry 5
  2. TAP5-374

Persistent (@Persist) fields not set correctly between requests if they are initialised from pageAttached() method

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 5.0.15, 5.0.16
    • Fix Version/s: 5.1.0.1
    • Component/s: tapestry-core
    • Labels:
      None

      Description

      I am seeing unexpected behaviour when using persistent fields and the
      page lifecycle method, pageAttached(), (T 5.0.15).

      I have a persistent field,

      @Persist
      private Map myMap;

      I also have a page lifecycle method,

      void pageAttached() {
      if (myMap == null)

      { myMap = new HashMap(); }

      }

      I expect that when the page first loads and myMap is null then myMap
      will be initialised to an empty map, thereafter I expect anything I put
      into the map to persist across requests.

      What I actually see is that, if the above pageAttached method is present
      then any data that I put into myMap does not persist across requests
      (the map is always empty). I was not expecting this.

      If I do not initialise myMap in pageAttached then data does persist.

      By design or bug?

        Activity

        Hide
        Joel Halbert added a comment -

        Cool. I will test this out on my old code base once released.

        Show
        Joel Halbert added a comment - Cool. I will test this out on my old code base once released.
        Hide
        Howard M. Lewis Ship added a comment -

        From PageImpl:

        public void persistFieldChange(ComponentResources resources, String fieldName, Object newValue)

        { if (!loadComplete) throw new RuntimeException(StructureMessages.persistChangeBeforeLoadComplete()); persistentFieldManager.postChange(name, resources, fieldName, newValue); }

        public Object getFieldChange(String nestedId, String fieldName)

        { if (fieldBundle == null) fieldBundle = persistentFieldManager.gatherChanges(name); return fieldBundle.getValue(nestedId, fieldName); }

        My guess is that this could be caused by having some other persistent field in the page. This first field forces the fieldBundle() to gather values. You initialization code sets the property to a new Map(). Then getFieldChange() is invoked for the property and your change isn't there, just resetting the value back to null.

        Anyway, breaking pageAttached() into two phases will help: first all persisted fields will be rolled back, then pageAttached() logic can set default values for persisted fields (which will be properly accounted for in the session from that point on).

        Show
        Howard M. Lewis Ship added a comment - From PageImpl: public void persistFieldChange(ComponentResources resources, String fieldName, Object newValue) { if (!loadComplete) throw new RuntimeException(StructureMessages.persistChangeBeforeLoadComplete()); persistentFieldManager.postChange(name, resources, fieldName, newValue); } public Object getFieldChange(String nestedId, String fieldName) { if (fieldBundle == null) fieldBundle = persistentFieldManager.gatherChanges(name); return fieldBundle.getValue(nestedId, fieldName); } My guess is that this could be caused by having some other persistent field in the page. This first field forces the fieldBundle() to gather values. You initialization code sets the property to a new Map(). Then getFieldChange() is invoked for the property and your change isn't there, just resetting the value back to null. Anyway, breaking pageAttached() into two phases will help: first all persisted fields will be rolled back, then pageAttached() logic can set default values for persisted fields (which will be properly accounted for in the session from that point on).
        Hide
        Howard M. Lewis Ship added a comment -

        Something's bugging me about this once because looking at the code I'm not sure why its failing.

        Show
        Howard M. Lewis Ship added a comment - Something's bugging me about this once because looking at the code I'm not sure why its failing.
        Hide
        Joel Halbert added a comment -

        sounds good.

        --------------------------------------------------
        From: "Howard M. Lewis Ship (JIRA)" <jira@apache.org>
        Sent: Tuesday, January 27, 2009 12:42 AM
        To: <joel@su3analytics.com>
        Subject: [jira] Commented: (TAP5-374) Persistent (@Persist) fields not set
        correctly between requests if they are initialised from pageAttached()
        method

        Show
        Joel Halbert added a comment - sounds good. -------------------------------------------------- From: "Howard M. Lewis Ship (JIRA)" <jira@apache.org> Sent: Tuesday, January 27, 2009 12:42 AM To: <joel@su3analytics.com> Subject: [jira] Commented: ( TAP5-374 ) Persistent (@Persist) fields not set correctly between requests if they are initialised from pageAttached() method
        Hide
        Howard M. Lewis Ship added a comment -

        I believe what's happening is that when the pageAttached() lifecycle method is invoked, the page is still in unattached state, i.e., so that it can ignore any changes to persistent fields caused by rolling back of the externally pesistent field values. Thus the change to myMap is ignored as if it was the result of rolling back the value stored in the session.

        So, probably, we need to split pageAttached() into two notifications: one for Tapestry generated code (i.e., an internalPageAttached()) and one for use code (pageAttached).

        Show
        Howard M. Lewis Ship added a comment - I believe what's happening is that when the pageAttached() lifecycle method is invoked, the page is still in unattached state, i.e., so that it can ignore any changes to persistent fields caused by rolling back of the externally pesistent field values. Thus the change to myMap is ignored as if it was the result of rolling back the value stored in the session. So, probably, we need to split pageAttached() into two notifications: one for Tapestry generated code (i.e., an internalPageAttached()) and one for use code (pageAttached).
        Hide
        Howard M. Lewis Ship added a comment -

        It's possible there is a bug here; Tapestry unable to distinguish between your change (which should be persisted) and the normal rolling back of field values from the session.

        Show
        Howard M. Lewis Ship added a comment - It's possible there is a bug here; Tapestry unable to distinguish between your change (which should be persisted) and the normal rolling back of field values from the session.
        Hide
        Joel Halbert added a comment -

        please refer to my last comment, thx.

        Show
        Joel Halbert added a comment - please refer to my last comment, thx.
        Hide
        Joel Halbert added a comment -

        But in the above example myMap is a persistent field that we want initialised on a per request basis, with a different Map instance each time, thus surely the correct place to do so is on pageAttached? (given that pageLoaded is not request specific and pageAttached is).

        Either way, given that we check for myMap being null before initialising it , it does not explain why it causes data not to persist between requests.

        Show
        Joel Halbert added a comment - But in the above example myMap is a persistent field that we want initialised on a per request basis, with a different Map instance each time, thus surely the correct place to do so is on pageAttached? (given that pageLoaded is not request specific and pageAttached is). Either way, given that we check for myMap being null before initialising it , it does not explain why it causes data not to persist between requests.
        Hide
        Howard M. Lewis Ship added a comment -

        pageAttached() is called for each request, I think the behavior you want is from pageLoaded().

        Show
        Howard M. Lewis Ship added a comment - pageAttached() is called for each request, I think the behavior you want is from pageLoaded().

          People

          • Assignee:
            Howard M. Lewis Ship
            Reporter:
            Joel Halbert
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development