You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@tapestry.apache.org by David Ezzio <de...@ysoft.com> on 2004/03/27 19:02:28 UTC
Mutable Persistent Page Properties (2nd attempt)
For some reason, my first posting of this note ended up in the archives
but never got reflected back out to the mailing list.
--
Hi,
I'm a newbie to Tapestry. So far, very impressed both with the code and
the book. Good job. But I am running into one question that I haven't
found the answer to.
There seems to be a bug or some confusion (perhaps on my part) about
mutable persistent page properties such as the ToDoList property in the
ToDo example.
In several places, the documentation says that the attribute should be
set after it is changed to ensure that HttpSession.setAttribute is
called. Everyone seems to agree that making the call to setAttribute is
required to support clustering. So far, so good.
The problem arises with the claim in some places that mutable objects
such as List and Map are copied when set in the page properties, when in
fact they are not. As a result, the HttpSession.setAttribute method is
never called for mutable collection objects unless a new collection
object is assigned.
In the example, when the enhanced ToDo class's setToDoList method is
called, the fireObserver occurs, which results in
PageRecorder.observeChange being called, which results in
SessionPageRecorder.recordChange being called.
In recordChange, the logic simply gets the old value and then takes two
steps either of which prevent the HttpSession.setAttribute method from
being called.
First
if (newValue == oldValue) return;
Since the List object is passed by reference and not by value, the two
references are the same, and the method returns.
Second, in the next code block of this method, it does a check on equals
which would also result in a return if execution passed the first check.
Consequently, HttpSession.setAttribute is not being called in the case
of the ToDo example as a result of the call to setToDoList in the
formSubmit listener.
Fixing this bug is an interesting question. I happen to agree with the
existing behavior that does not copy List, Map, and Set objects. In
many cases, they will not be modified. Certainly there is no reason to
copy the immutable objects.
Given that, then I would think the most obvious fix is that
HttpSession.setAttribute is always called if the fireObservedChange
method is called. I don't see a point for the checking in recordChange
method.
Best wishes,
David Ezzio
---------------------------------------------------------------------
To unsubscribe, e-mail: tapestry-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tapestry-user-help@jakarta.apache.org
Re: Mutable Persistent Page Properties (2nd attempt)
Posted by David Ezzio <de...@ysoft.com>.
Excellent! The joys of open source software: bugs are fixed!
David
Howard M. Lewis Ship wrote:
> I slipped in a fix for this problem right under the wire for the 3.0 final release.
>
> http://nagoya.apache.org/bugzilla/show_bug.cgi?id=28454
>
> The joys of having a test suite!
>
>
> --
> Howard M. Lewis Ship
> Independent J2EE / Open-Source Java Consultant
> Creator, Tapestry: Java Web Components
> Creator, HiveMind
> http://howardlewisship.com
>
>
>
>>-----Original Message-----
>>From: stephen smithstone [mailto:skullboxhouse@lchost.co.uk]
>>Sent: Monday, April 19, 2004 6:41 PM
>>To: Tapestry users
>>Subject: Re: Mutable Persistent Page Properties (2nd attempt)
>>
>>
>>but further down in the todo list example it sets the new list
>>due to the proplem with mutable objects
>>
>>On 27 Mar 2004, at 18:02, David Ezzio wrote:
>>
>>
>>>For some reason, my first posting of this note ended up in the
>>>archives but never got reflected back out to the mailing list.
>>>
>>>--
>>>
>>>Hi,
>>>
>>>I'm a newbie to Tapestry. So far, very impressed both with
>>
>>the code
>>
>>>and the book. Good job. But I am running into one question that I
>>>haven't found the answer to.
>>>
>>>There seems to be a bug or some confusion (perhaps on my
>>
>>part) about
>>
>>>mutable persistent page properties such as the ToDoList property in
>>>the ToDo example.
>>>
>>>In several places, the documentation says that the
>>
>>attribute should be
>>
>>>set after it is changed to ensure that HttpSession.setAttribute is
>>>called. Everyone seems to agree that making the call to
>>
>>setAttribute
>>
>>>is required to support clustering. So far, so good.
>>>
>>>The problem arises with the claim in some places that
>>
>>mutable objects
>>
>>>such as List and Map are copied when set in the page
>>
>>properties, when
>>
>>>in fact they are not. As a result, the HttpSession.setAttribute
>>>method is never called for mutable collection objects unless a new
>>>collection object is assigned.
>>>
>>>In the example, when the enhanced ToDo class's setToDoList
>>
>>method is
>>
>>>called, the fireObserver occurs, which results in
>>>PageRecorder.observeChange being called, which results in
>>>SessionPageRecorder.recordChange being called.
>>>
>>>In recordChange, the logic simply gets the old value and then takes
>>>two steps either of which prevent the
>>
>>HttpSession.setAttribute method
>>
>>>from being called.
>>>
>>>First
>>>
>>> if (newValue == oldValue) return;
>>>
>>>Since the List object is passed by reference and not by
>>
>>value, the two
>>
>>>references are the same, and the method returns.
>>>
>>>Second, in the next code block of this method, it does a check on
>>>equals which would also result in a return if execution passed the
>>>first check.
>>>
>>>Consequently, HttpSession.setAttribute is not being called
>>
>>in the case
>>
>>>of the ToDo example as a result of the call to setToDoList in the
>>>formSubmit listener.
>>>
>>>Fixing this bug is an interesting question. I happen to agree with
>>>the existing behavior that does not copy List, Map, and Set
>>
>>objects.
>>
>>>In many cases, they will not be modified. Certainly there is no
>>>reason to copy the immutable objects.
>>>
>>>Given that, then I would think the most obvious fix is that
>>>HttpSession.setAttribute is always called if the fireObservedChange
>>>method is called. I don't see a point for the checking in
>>>recordChange method.
>>>
>>>
>>>
>>>Best wishes,
>>>
>>>David Ezzio
>>>
>>>
>>>
>>>
>>
>>---------------------------------------------------------------------
>>
>>>To unsubscribe, e-mail: tapestry-user-unsubscribe@jakarta.apache.org
>>>For additional commands, e-mail:
>>
>>tapestry-user-help@jakarta.apache.org
>>
>>>
>>
>>---------------------------------------------------------------------
>>To unsubscribe, e-mail: tapestry-user-unsubscribe@jakarta.apache.org
>>For additional commands, e-mail: tapestry-user-help@jakarta.apache.org
>>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tapestry-user-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tapestry-user-help@jakarta.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: tapestry-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tapestry-user-help@jakarta.apache.org
RE: Mutable Persistent Page Properties (2nd attempt)
Posted by "Howard M. Lewis Ship" <hl...@comcast.net>.
I slipped in a fix for this problem right under the wire for the 3.0 final release.
http://nagoya.apache.org/bugzilla/show_bug.cgi?id=28454
The joys of having a test suite!
--
Howard M. Lewis Ship
Independent J2EE / Open-Source Java Consultant
Creator, Tapestry: Java Web Components
Creator, HiveMind
http://howardlewisship.com
> -----Original Message-----
> From: stephen smithstone [mailto:skullboxhouse@lchost.co.uk]
> Sent: Monday, April 19, 2004 6:41 PM
> To: Tapestry users
> Subject: Re: Mutable Persistent Page Properties (2nd attempt)
>
>
> but further down in the todo list example it sets the new list
> due to the proplem with mutable objects
>
> On 27 Mar 2004, at 18:02, David Ezzio wrote:
>
> > For some reason, my first posting of this note ended up in the
> > archives but never got reflected back out to the mailing list.
> >
> > --
> >
> > Hi,
> >
> > I'm a newbie to Tapestry. So far, very impressed both with
> the code
> > and the book. Good job. But I am running into one question that I
> > haven't found the answer to.
> >
> > There seems to be a bug or some confusion (perhaps on my
> part) about
> > mutable persistent page properties such as the ToDoList property in
> > the ToDo example.
> >
> > In several places, the documentation says that the
> attribute should be
> > set after it is changed to ensure that HttpSession.setAttribute is
> > called. Everyone seems to agree that making the call to
> setAttribute
> > is required to support clustering. So far, so good.
> >
> > The problem arises with the claim in some places that
> mutable objects
> > such as List and Map are copied when set in the page
> properties, when
> > in fact they are not. As a result, the HttpSession.setAttribute
> > method is never called for mutable collection objects unless a new
> > collection object is assigned.
> >
> > In the example, when the enhanced ToDo class's setToDoList
> method is
> > called, the fireObserver occurs, which results in
> > PageRecorder.observeChange being called, which results in
> > SessionPageRecorder.recordChange being called.
> >
> > In recordChange, the logic simply gets the old value and then takes
> > two steps either of which prevent the
> HttpSession.setAttribute method
> > from being called.
> >
> > First
> >
> > if (newValue == oldValue) return;
> >
> > Since the List object is passed by reference and not by
> value, the two
> > references are the same, and the method returns.
> >
> > Second, in the next code block of this method, it does a check on
> > equals which would also result in a return if execution passed the
> > first check.
> >
> > Consequently, HttpSession.setAttribute is not being called
> in the case
> > of the ToDo example as a result of the call to setToDoList in the
> > formSubmit listener.
> >
> > Fixing this bug is an interesting question. I happen to agree with
> > the existing behavior that does not copy List, Map, and Set
> objects.
> > In many cases, they will not be modified. Certainly there is no
> > reason to copy the immutable objects.
> >
> > Given that, then I would think the most obvious fix is that
> > HttpSession.setAttribute is always called if the fireObservedChange
> > method is called. I don't see a point for the checking in
> > recordChange method.
> >
> >
> >
> > Best wishes,
> >
> > David Ezzio
> >
> >
> >
> >
> ---------------------------------------------------------------------
> > To unsubscribe, e-mail: tapestry-user-unsubscribe@jakarta.apache.org
> > For additional commands, e-mail:
> tapestry-user-help@jakarta.apache.org
> >
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tapestry-user-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tapestry-user-help@jakarta.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: tapestry-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tapestry-user-help@jakarta.apache.org
Re: Mutable Persistent Page Properties (2nd attempt)
Posted by stephen smithstone <sk...@lchost.co.uk>.
but further down in the todo list example it sets the new list
due to the proplem with mutable objects
On 27 Mar 2004, at 18:02, David Ezzio wrote:
> For some reason, my first posting of this note ended up in the
> archives but never got reflected back out to the mailing list.
>
> --
>
> Hi,
>
> I'm a newbie to Tapestry. So far, very impressed both with the code
> and the book. Good job. But I am running into one question that I
> haven't found the answer to.
>
> There seems to be a bug or some confusion (perhaps on my part) about
> mutable persistent page properties such as the ToDoList property in
> the ToDo example.
>
> In several places, the documentation says that the attribute should be
> set after it is changed to ensure that HttpSession.setAttribute is
> called. Everyone seems to agree that making the call to setAttribute
> is required to support clustering. So far, so good.
>
> The problem arises with the claim in some places that mutable objects
> such as List and Map are copied when set in the page properties, when
> in fact they are not. As a result, the HttpSession.setAttribute
> method is never called for mutable collection objects unless a new
> collection object is assigned.
>
> In the example, when the enhanced ToDo class's setToDoList method is
> called, the fireObserver occurs, which results in
> PageRecorder.observeChange being called, which results in
> SessionPageRecorder.recordChange being called.
>
> In recordChange, the logic simply gets the old value and then takes
> two steps either of which prevent the HttpSession.setAttribute method
> from being called.
>
> First
>
> if (newValue == oldValue) return;
>
> Since the List object is passed by reference and not by value, the two
> references are the same, and the method returns.
>
> Second, in the next code block of this method, it does a check on
> equals which would also result in a return if execution passed the
> first check.
>
> Consequently, HttpSession.setAttribute is not being called in the case
> of the ToDo example as a result of the call to setToDoList in the
> formSubmit listener.
>
> Fixing this bug is an interesting question. I happen to agree with
> the existing behavior that does not copy List, Map, and Set objects.
> In many cases, they will not be modified. Certainly there is no
> reason to copy the immutable objects.
>
> Given that, then I would think the most obvious fix is that
> HttpSession.setAttribute is always called if the fireObservedChange
> method is called. I don't see a point for the checking in
> recordChange method.
>
>
>
> Best wishes,
>
> David Ezzio
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tapestry-user-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tapestry-user-help@jakarta.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: tapestry-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tapestry-user-help@jakarta.apache.org
Re: Mutable Persistent Page Properties (2nd attempt)
Posted by David Ezzio <de...@ysoft.com>.
Hi David,
Well, at least you looked at it. :)
Here's the issue in a nutshell (I am using Beta 3, so perhaps the
problem has been fixed.) fireObserver must be called when persistent
page properties are changed because this is the mechanism that tells
Tapestry to store the attribute in the HttpSession object. This is not
a big deal if you use Tapestry's auto-enhancment for abstract
properties. The enhancement added code calls the fireObserver method.
The problem is that calling fireObserver does nothing in the Hangman1
example, since the recordChange method checks to see if the object to be
set is the same object as is in the HttpSession already. For Tapestry
to work as advertised in TIA, something has to change. Either Tapesty
must make copies of the mutable objects when it stores them in the page,
or the page recorder can not assume that checking the reference is
sufficient to detect a change in the object.
In other words, this line of code in SessionPageRecorder.recordChange:
if (newValue == oldValue) return;
must go away. It works only if the objects that contain the values of
the persistent page properties are themselves immutable.
David
David Moran wrote:
> I went back and looked at your original post and I was not able to follow
> what you were talking about with the fireObserved event, but my
> understanding from the TIA book is that the setAttribute for the mutable
> objects (which includes the engine itself) is called by the framework at the
> end Engine.service() method if the objects have changed.
>
> -----Original Message-----
> From: David Ezzio [mailto:dezzio@ysoft.com]
> Sent: Tuesday, April 13, 2004 7:12 AM
> To: Tapestry users
> Subject: Re: Mutable Persistent Page Properties (2nd attempt)
>
>
> I rather thought that someone would address this question. Is it poorly
> framed, or what???
>
> David Ezzio wrote:
>
>
>>For some reason, my first posting of this note ended up in the archives
>>but never got reflected back out to the mailing list.
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tapestry-user-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tapestry-user-help@jakarta.apache.org
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tapestry-user-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tapestry-user-help@jakarta.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: tapestry-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tapestry-user-help@jakarta.apache.org
RE: Mutable Persistent Page Properties (2nd attempt)
Posted by David Moran <dm...@nc.rr.com>.
I went back and looked at your original post and I was not able to follow
what you were talking about with the fireObserved event, but my
understanding from the TIA book is that the setAttribute for the mutable
objects (which includes the engine itself) is called by the framework at the
end Engine.service() method if the objects have changed.
-----Original Message-----
From: David Ezzio [mailto:dezzio@ysoft.com]
Sent: Tuesday, April 13, 2004 7:12 AM
To: Tapestry users
Subject: Re: Mutable Persistent Page Properties (2nd attempt)
I rather thought that someone would address this question. Is it poorly
framed, or what???
David Ezzio wrote:
> For some reason, my first posting of this note ended up in the archives
> but never got reflected back out to the mailing list.
>
---------------------------------------------------------------------
To unsubscribe, e-mail: tapestry-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tapestry-user-help@jakarta.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: tapestry-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tapestry-user-help@jakarta.apache.org
Re: Mutable Persistent Page Properties (2nd attempt)
Posted by David Ezzio <de...@ysoft.com>.
I rather thought that someone would address this question. Is it poorly
framed, or what???
David Ezzio wrote:
> For some reason, my first posting of this note ended up in the archives
> but never got reflected back out to the mailing list.
>
---------------------------------------------------------------------
To unsubscribe, e-mail: tapestry-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tapestry-user-help@jakarta.apache.org