You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@tapestry.apache.org by Geoff Callender <ge...@gmail.com> on 2007/11/26 12:27:36 UTC

T5: Edit page best practice?

Hi,

I'm looking for suggestions on how to improve this EditPerson page to  
T5 best practice.

	private Long _personId;

	@Persist
	private Person _person;

	public void onActivate(Long id) { _personId = id; }

	public Long onPassivate() { return _personId; }
	
	void onPrepareFromForm() {
		if (_person == null || _person.getId().equals(_personId)) {
			_person = getSecurityFinderService().findPerson(_personId);
		}
	}

	Object onSuccess() {
		if (...a bit of validation logic detects an error...) {
			_form.recordError(...);
			return null;
		}
		etc...
		_person = null;
		return _nextPage;
	}

	Object onActionFromCancel() {
		_person = null;
		return _previousPage;
	}

	void cleanupRender() {
		_form.clearErrors();
	}

Some notes and questions:

1. Persist - Is there an alternative?  It's used here to keep _person  
populated in case onSuccess() detects an error and redisplays the page.

2. Housekeeping - I'm nullifying _person in each method that returns a  
different page.  This seems clumsy.  Suggestions?

3. Thread-safety - is there a thread-safety issue if the user has  
opened a new window in same browser - can T5 tell them apart or do  
they share the same HttpSession?

4. onPrepareFromForm() - this looks a bit clumsy to me.  If the user  
hits Reload, or if the user hits the Back button then "pagelinks" back  
in, onPrepareFromForm() wlll not call findPerson(...).  The _person  
isn't refreshed at all!  Thoughts?

Thanks,

Geoff

Re: T5: Edit page best practice?

Posted by Angelo Chen <an...@yahoo.com.hk>.
Hi Lasitha,

I use same approach as well, don't persist, persist(flash) if you have to
inject that page and setup some value.

This does bring up another issue, easy if the page involves only one person,
you can look it up again, how about a complicated document? maybe a certain
streaming mechanism to save the entire temporary object and retrieve it
later by an ID in the onActivate event? persist back to the data store in
onSuccess? that seems easier to handle than T5's @Persist? then again maybe
T5's persist is actually doing the same thing? It might solve the
Tapestry-Hibernate's related issue of not able to roll back changes, if the
setters update only a temporary object, and this object will be used to set
the object in the data store in onSuccess, Tapestry-hibernate might work as
expected? comments?

A.C.




lasitha wrote:
> 
> 
> Ok, so it seems most of my comments boil down to: don't persist, and
> use flash/client strategies if you must :)
> 
> Let me throw something else in to break the monotony: you can
> eliminate the conditional in onSuccess() by defining an onValidate()
> handler.  If onValidate() records errors, tapestry will know not to
> call onSuccess().
> 
> Ok, hope that helps some.
> Cheers, lasitha.
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@tapestry.apache.org
> For additional commands, e-mail: users-help@tapestry.apache.org
> 
> 
> 

-- 
View this message in context: http://www.nabble.com/T5%3A-Edit-page-best-practice--tf4874654.html#a13962080
Sent from the Tapestry - User mailing list archive at Nabble.com.


---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tapestry.apache.org
For additional commands, e-mail: users-help@tapestry.apache.org


Re: T5: Edit page best practice? Really struggling...

Posted by Christian Edward Gruber <ch...@gmail.com>.
Webobjects had a nice little context variable which was built into the  
URL's and it would increment with every request within the session.   
You could tie "undo" or other functionality to it.  It was fairly  
unobtrusive.  The same mechanism could be used for conversation,  
though.  Instead of incrementing, you simply have a per-conversation  
context generated.

The problem with that approach is that we already have url  
manipulation in T5 to pass extra bits between /s as parameters.  This  
could be handled by, if optional conversatin context were active,  
additional syntax would be provided in the url with another separator  
for that.  Something like http://www.foo.com/blah/myComponent:action;context/param1 
.

Then again, I did only get 2 hours of sleep...

Christian.

On 28-Nov-07, at 6:21 PM, Howard Lewis Ship wrote:

> Geoff,
>
> I'm working as fast as I can ... I really need to get the Ajax stuff
> worked out.  I suspect there's a mix of guidelines and some additional
> technology that will help.  What's complicated is that you don't want
> to store state on the client because of ugly URLs and you don't want
> to store it on the server because of multiple windows issue.
>
> So a kind of conversational state would be nice, where the key into
> the HttpSession is based on a *short* key associated with each URL.
> That just adds the complication of controlling when a new conversation
> begins (is a pop-up window a new conversation?  Not always).
> Certainly Seam has solved this problem as has Spring Web Flow.
>
> On Nov 28, 2007 1:09 PM, Geoff Callender
> <ge...@gmail.com> wrote:
>> Hi Howard,
>>
>> Since one size does not fit all, I think we desperately need a
>> guideline on how to deal with each situation.  You know, a kind of
>> "cookbook", with a decision tree or a table that maps requirements to
>> standardised solutions.
>>
>> I am trying to put together just such a decision tree as I rework
>> Tapestry JumpStart from T4 to T5 and I'm really struggling, because
>> there seems to be a hole in every approach I've tried:
>>
>> - "client" persistence can be good, except it brings back ugly URLs
>> which can easily get too big and trip up the browser silently.
>> Perhaps "client:form" persistence will return to help out, but there
>> can be other considerations that steer us away from persisting on the
>> client.
>>
>> - "flash" persistence is great for display, but has hairs on it when
>> used for edit, requiring re-fetching from the DB and/or a return to
>> the DTO model.
>>
>> - "session" persistence fails to solve problems like multiple windows
>> and tabbed-browsing.
>>
>> I've jotted down a few more thoughts on this today in another thread
>> titled 'T5: Is Persist{"conversation") planned? Please?'.  I'd be
>> really grateful if you could throw in your thoughts on it.
>>
>> Regards,
>>
>> Geoff
>>
>> On 28/11/2007, at 3:54 AM, Howard Lewis Ship wrote:
>>
>>> On Nov 27, 2007 3:56 AM, Geoff Callender
>>> <ge...@gmail.com> wrote:
>>>> Thanks, Iasatha, for the considered answer.  It seems that even  
>>>> with
>>>> the brilliance of T5, the web is still a tricky area with trade- 
>>>> offs
>>>> at every turn.
>>>
>>> I agree; you can store a lot more persistent state on the server,  
>>> but
>>> then have the issue with the browser back button (not to mention
>>> resource consumption on the server), or you can store data (or
>>> prefereably, entity keys) on the client, but have to write a bit  
>>> more
>>> code.  One size does not fit all, so I just want T5 to get out of  
>>> your
>>> way on this kind of thing.
>>>
>>>
>>>>
>>>> On 27/11/2007, at 2:10 AM, lasitha wrote:
>>>>
>>>>>> 1. Persist - Is there an alternative?  It's used here to keep
>>>>>> _person
>>>>>> populated in case onSuccess() detects an error and redisplays the
>>>>>> page.
>>>>>
>>>>> Well, you could always look up the person again from the
>>>>> SecurityFinderService, but i suppose you don't want to pay for the
>>>>> lookup again...  In that case, i don't know of an alternative.
>>>>
>>>> You're right - I don't want to do a lookup again - but principally
>>>> because the aim is to redisplay the object with their changes, so a
>>>> lookup is actually counter-productive.  A second, show-stopping
>>>> reason
>>>> is that _person is a detached object utilising optimistic locking.
>>>> If
>>>> we keep refreshing the underlying object then we're messing up the
>>>> whole principal of optimistic locking.
>>>>
>>>>> Note that you probably want to use flash (perhaps even client?)
>>>>> persistence.
>>>>
>>>> Just switched to @Persist("flash") and got a couple of surprises:
>>>> - leaving the validation in onSuccess(), the page redisplays with
>>>> error but the person it shows is the latest from the DB!!!  The
>>>> problem is that _person is nullified by the time that
>>>> onPrepareFromForm() sees it so it does the lookup again.
>>>> - moving the validation out of onSuccess() and into onValidate() as
>>>> you suggested below fixes the display problem (very good) but
>>>> nonetheless _person is nullified by the time onPrepareFromForm()  
>>>> sees
>>>> it so it does the lookup again.  Non-displayed fields such as
>>>> "version" are overwritten in _person  which breaks optimistic  
>>>> locking
>>>> (very bad).
>>>>
>>>>> The form component does accept a 'context' parameter which might
>>>>> allow
>>>>> you to eliminate onActivate/onPassivate in favour of parameters to
>>>>> onPrepareFromForm() and a getter for the id.  I haven't played  
>>>>> with
>>>>> that much so if you do pursue it, please report back :)
>>>>
>>>> Not sure about this.  Without onPassivate() wouldn't I lose the  
>>>> nice
>>>> bookmarkable URL (eg. http://localhost/myapp/personedit/123)?
>>>> Without
>>>> onActivate(Long id) don't I lose the ability to navigate to this  
>>>> page
>>>> via pagelink?
>>>>
>>>>> And as a side note, shouldn't the second condition in
>>>>> onPrepareFromForm() be negated?  You want to lookup the person if
>>>>> the
>>>>> ids _don't_ match, right?
>>>>
>>>> Well spotted - you're right.
>>>>
>>>>>> 2. Housekeeping - I'm nullifying _person in each method that
>>>>>> returns a
>>>>>> different page.  This seems clumsy.  Suggestions?
>>>>>
>>>>> Why?  Tapestry will take care of basic housekeeping.  If you're
>>>>> nulling it out to force a subsequent request to refresh the  
>>>>> person's
>>>>> state, then a better alternative is to use flash persistence (or  
>>>>> no
>>>>> persistence).
>>>>>
>>>>> On a similiar note, if you set the default persistence strategy of
>>>>> the
>>>>> form to flash (via a metadata annotation), it will apply this to  
>>>>> its
>>>>> validation tracker as well.  That would allow you to eliminate
>>>>> cleanupRender().
>>>>
>>>> I tried Persist("flash") on the form and it didn't do the trick.
>>>> Looks like 5.0.7 will put flash persistence on the form's
>>>> ValidationTracker which would be good.
>>>>
>>>>>> 3. Thread-safety - is there a thread-safety issue if the user has
>>>>>> opened a new window in same browser - can T5 tell them apart or  
>>>>>> do
>>>>>> they share the same HttpSession?
>>>>>
>>>>> Yes, they will share the same session, so anything you @Persist
>>>>> could
>>>>> be a problem (unless its persisted on the client).  I don't know  
>>>>> of
>>>>> anything tap5 does to help with this, so hopefully someone more
>>>>> knowledgeable will chime in.
>>>>>
>>>>> Again, i'd reconsider just looking up the person again.
>>>>
>>>> So does anyone else know if T5 has somehow dealt with the thread-
>>>> safety issue????
>>>>
>>>> Client persistence strategy solves it if the objects are small,  
>>>> but I
>>>> note the T5 doco is quite negative on the client persistence  
>>>> strategy
>>>> (http://tapestry.apache.org/tapestry5/tapestry-core/guide/persist.html
>>>> )
>>>>
>>>>>> 4. onPrepareFromForm() - this looks a bit clumsy to me.  If the
>>>>>> user
>>>>>> hits Reload, or if the user hits the Back button then "pagelinks"
>>>>>> back
>>>>>> in, onPrepareFromForm() wlll not call findPerson(...).  The  
>>>>>> _person
>>>>>> isn't refreshed at all!  Thoughts?
>>>>>
>>>>> Using flash persistence should solve both those issues, yeah?
>>>>>
>>>>> Ok, so it seems most of my comments boil down to: don't persist,  
>>>>> and
>>>>> use flash/client strategies if you must :)
>>>>>
>>>>> Let me throw something else in to break the monotony: you can
>>>>> eliminate the conditional in onSuccess() by defining an  
>>>>> onValidate()
>>>>> handler.  If onValidate() records errors, tapestry will know not  
>>>>> to
>>>>> call onSuccess().
>>>>>
>>>>> Ok, hope that helps some.
>>>>> Cheers, lasitha.
>>>>
>>>> Thanks plenty, Iasitha.
>>>>
>>>> Geoff
>>>
>>>
>>>
>>> --
>>> Howard M. Lewis Ship
>>> Partner and Senior Architect at Feature50
>>>
>>> Creator Apache Tapestry and Apache HiveMind
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: users-unsubscribe@tapestry.apache.org
>>> For additional commands, e-mail: users-help@tapestry.apache.org
>>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: users-unsubscribe@tapestry.apache.org
>> For additional commands, e-mail: users-help@tapestry.apache.org
>>
>>
>
>
>
> -- 
> Howard M. Lewis Ship
> Partner and Senior Architect at Feature50
>
> Creator Apache Tapestry and Apache HiveMind
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@tapestry.apache.org
> For additional commands, e-mail: users-help@tapestry.apache.org
>


---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tapestry.apache.org
For additional commands, e-mail: users-help@tapestry.apache.org


Re: T5: Edit page best practice? Really struggling...

Posted by Howard Lewis Ship <hl...@gmail.com>.
Geoff,

I'm working as fast as I can ... I really need to get the Ajax stuff
worked out.  I suspect there's a mix of guidelines and some additional
technology that will help.  What's complicated is that you don't want
to store state on the client because of ugly URLs and you don't want
to store it on the server because of multiple windows issue.

So a kind of conversational state would be nice, where the key into
the HttpSession is based on a *short* key associated with each URL.
That just adds the complication of controlling when a new conversation
begins (is a pop-up window a new conversation?  Not always).
Certainly Seam has solved this problem as has Spring Web Flow.

On Nov 28, 2007 1:09 PM, Geoff Callender
<ge...@gmail.com> wrote:
> Hi Howard,
>
> Since one size does not fit all, I think we desperately need a
> guideline on how to deal with each situation.  You know, a kind of
> "cookbook", with a decision tree or a table that maps requirements to
> standardised solutions.
>
> I am trying to put together just such a decision tree as I rework
> Tapestry JumpStart from T4 to T5 and I'm really struggling, because
> there seems to be a hole in every approach I've tried:
>
> - "client" persistence can be good, except it brings back ugly URLs
> which can easily get too big and trip up the browser silently.
> Perhaps "client:form" persistence will return to help out, but there
> can be other considerations that steer us away from persisting on the
> client.
>
> - "flash" persistence is great for display, but has hairs on it when
> used for edit, requiring re-fetching from the DB and/or a return to
> the DTO model.
>
> - "session" persistence fails to solve problems like multiple windows
> and tabbed-browsing.
>
> I've jotted down a few more thoughts on this today in another thread
> titled 'T5: Is Persist{"conversation") planned? Please?'.  I'd be
> really grateful if you could throw in your thoughts on it.
>
> Regards,
>
> Geoff
>
> On 28/11/2007, at 3:54 AM, Howard Lewis Ship wrote:
>
> > On Nov 27, 2007 3:56 AM, Geoff Callender
> > <ge...@gmail.com> wrote:
> >> Thanks, Iasatha, for the considered answer.  It seems that even with
> >> the brilliance of T5, the web is still a tricky area with trade-offs
> >> at every turn.
> >
> > I agree; you can store a lot more persistent state on the server, but
> > then have the issue with the browser back button (not to mention
> > resource consumption on the server), or you can store data (or
> > prefereably, entity keys) on the client, but have to write a bit more
> > code.  One size does not fit all, so I just want T5 to get out of your
> > way on this kind of thing.
> >
> >
> >>
> >> On 27/11/2007, at 2:10 AM, lasitha wrote:
> >>
> >>>> 1. Persist - Is there an alternative?  It's used here to keep
> >>>> _person
> >>>> populated in case onSuccess() detects an error and redisplays the
> >>>> page.
> >>>
> >>> Well, you could always look up the person again from the
> >>> SecurityFinderService, but i suppose you don't want to pay for the
> >>> lookup again...  In that case, i don't know of an alternative.
> >>
> >> You're right - I don't want to do a lookup again - but principally
> >> because the aim is to redisplay the object with their changes, so a
> >> lookup is actually counter-productive.  A second, show-stopping
> >> reason
> >> is that _person is a detached object utilising optimistic locking.
> >> If
> >> we keep refreshing the underlying object then we're messing up the
> >> whole principal of optimistic locking.
> >>
> >>> Note that you probably want to use flash (perhaps even client?)
> >>> persistence.
> >>
> >> Just switched to @Persist("flash") and got a couple of surprises:
> >> - leaving the validation in onSuccess(), the page redisplays with
> >> error but the person it shows is the latest from the DB!!!  The
> >> problem is that _person is nullified by the time that
> >> onPrepareFromForm() sees it so it does the lookup again.
> >> - moving the validation out of onSuccess() and into onValidate() as
> >> you suggested below fixes the display problem (very good) but
> >> nonetheless _person is nullified by the time onPrepareFromForm() sees
> >> it so it does the lookup again.  Non-displayed fields such as
> >> "version" are overwritten in _person  which breaks optimistic locking
> >> (very bad).
> >>
> >>> The form component does accept a 'context' parameter which might
> >>> allow
> >>> you to eliminate onActivate/onPassivate in favour of parameters to
> >>> onPrepareFromForm() and a getter for the id.  I haven't played with
> >>> that much so if you do pursue it, please report back :)
> >>
> >> Not sure about this.  Without onPassivate() wouldn't I lose the nice
> >> bookmarkable URL (eg. http://localhost/myapp/personedit/123)?
> >> Without
> >> onActivate(Long id) don't I lose the ability to navigate to this page
> >> via pagelink?
> >>
> >>> And as a side note, shouldn't the second condition in
> >>> onPrepareFromForm() be negated?  You want to lookup the person if
> >>> the
> >>> ids _don't_ match, right?
> >>
> >> Well spotted - you're right.
> >>
> >>>> 2. Housekeeping - I'm nullifying _person in each method that
> >>>> returns a
> >>>> different page.  This seems clumsy.  Suggestions?
> >>>
> >>> Why?  Tapestry will take care of basic housekeeping.  If you're
> >>> nulling it out to force a subsequent request to refresh the person's
> >>> state, then a better alternative is to use flash persistence (or no
> >>> persistence).
> >>>
> >>> On a similiar note, if you set the default persistence strategy of
> >>> the
> >>> form to flash (via a metadata annotation), it will apply this to its
> >>> validation tracker as well.  That would allow you to eliminate
> >>> cleanupRender().
> >>
> >> I tried Persist("flash") on the form and it didn't do the trick.
> >> Looks like 5.0.7 will put flash persistence on the form's
> >> ValidationTracker which would be good.
> >>
> >>>> 3. Thread-safety - is there a thread-safety issue if the user has
> >>>> opened a new window in same browser - can T5 tell them apart or do
> >>>> they share the same HttpSession?
> >>>
> >>> Yes, they will share the same session, so anything you @Persist
> >>> could
> >>> be a problem (unless its persisted on the client).  I don't know of
> >>> anything tap5 does to help with this, so hopefully someone more
> >>> knowledgeable will chime in.
> >>>
> >>> Again, i'd reconsider just looking up the person again.
> >>
> >> So does anyone else know if T5 has somehow dealt with the thread-
> >> safety issue????
> >>
> >> Client persistence strategy solves it if the objects are small, but I
> >> note the T5 doco is quite negative on the client persistence strategy
> >> (http://tapestry.apache.org/tapestry5/tapestry-core/guide/persist.html
> >> )
> >>
> >>>> 4. onPrepareFromForm() - this looks a bit clumsy to me.  If the
> >>>> user
> >>>> hits Reload, or if the user hits the Back button then "pagelinks"
> >>>> back
> >>>> in, onPrepareFromForm() wlll not call findPerson(...).  The _person
> >>>> isn't refreshed at all!  Thoughts?
> >>>
> >>> Using flash persistence should solve both those issues, yeah?
> >>>
> >>> Ok, so it seems most of my comments boil down to: don't persist, and
> >>> use flash/client strategies if you must :)
> >>>
> >>> Let me throw something else in to break the monotony: you can
> >>> eliminate the conditional in onSuccess() by defining an onValidate()
> >>> handler.  If onValidate() records errors, tapestry will know not to
> >>> call onSuccess().
> >>>
> >>> Ok, hope that helps some.
> >>> Cheers, lasitha.
> >>
> >> Thanks plenty, Iasitha.
> >>
> >> Geoff
> >
> >
> >
> > --
> > Howard M. Lewis Ship
> > Partner and Senior Architect at Feature50
> >
> > Creator Apache Tapestry and Apache HiveMind
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: users-unsubscribe@tapestry.apache.org
> > For additional commands, e-mail: users-help@tapestry.apache.org
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@tapestry.apache.org
> For additional commands, e-mail: users-help@tapestry.apache.org
>
>



-- 
Howard M. Lewis Ship
Partner and Senior Architect at Feature50

Creator Apache Tapestry and Apache HiveMind

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tapestry.apache.org
For additional commands, e-mail: users-help@tapestry.apache.org


Re: T5: Edit page best practice? Really struggling...

Posted by Geoff Callender <ge...@gmail.com>.
Hi Howard,

Since one size does not fit all, I think we desperately need a  
guideline on how to deal with each situation.  You know, a kind of  
"cookbook", with a decision tree or a table that maps requirements to  
standardised solutions.

I am trying to put together just such a decision tree as I rework  
Tapestry JumpStart from T4 to T5 and I'm really struggling, because  
there seems to be a hole in every approach I've tried:

- "client" persistence can be good, except it brings back ugly URLs  
which can easily get too big and trip up the browser silently.   
Perhaps "client:form" persistence will return to help out, but there  
can be other considerations that steer us away from persisting on the  
client.

- "flash" persistence is great for display, but has hairs on it when  
used for edit, requiring re-fetching from the DB and/or a return to  
the DTO model.

- "session" persistence fails to solve problems like multiple windows  
and tabbed-browsing.

I've jotted down a few more thoughts on this today in another thread  
titled 'T5: Is Persist{"conversation") planned? Please?'.  I'd be  
really grateful if you could throw in your thoughts on it.

Regards,

Geoff

On 28/11/2007, at 3:54 AM, Howard Lewis Ship wrote:

> On Nov 27, 2007 3:56 AM, Geoff Callender
> <ge...@gmail.com> wrote:
>> Thanks, Iasatha, for the considered answer.  It seems that even with
>> the brilliance of T5, the web is still a tricky area with trade-offs
>> at every turn.
>
> I agree; you can store a lot more persistent state on the server, but
> then have the issue with the browser back button (not to mention
> resource consumption on the server), or you can store data (or
> prefereably, entity keys) on the client, but have to write a bit more
> code.  One size does not fit all, so I just want T5 to get out of your
> way on this kind of thing.
>
>
>>
>> On 27/11/2007, at 2:10 AM, lasitha wrote:
>>
>>>> 1. Persist - Is there an alternative?  It's used here to keep  
>>>> _person
>>>> populated in case onSuccess() detects an error and redisplays the
>>>> page.
>>>
>>> Well, you could always look up the person again from the
>>> SecurityFinderService, but i suppose you don't want to pay for the
>>> lookup again...  In that case, i don't know of an alternative.
>>
>> You're right - I don't want to do a lookup again - but principally
>> because the aim is to redisplay the object with their changes, so a
>> lookup is actually counter-productive.  A second, show-stopping  
>> reason
>> is that _person is a detached object utilising optimistic locking.   
>> If
>> we keep refreshing the underlying object then we're messing up the
>> whole principal of optimistic locking.
>>
>>> Note that you probably want to use flash (perhaps even client?)
>>> persistence.
>>
>> Just switched to @Persist("flash") and got a couple of surprises:
>> - leaving the validation in onSuccess(), the page redisplays with
>> error but the person it shows is the latest from the DB!!!  The
>> problem is that _person is nullified by the time that
>> onPrepareFromForm() sees it so it does the lookup again.
>> - moving the validation out of onSuccess() and into onValidate() as
>> you suggested below fixes the display problem (very good) but
>> nonetheless _person is nullified by the time onPrepareFromForm() sees
>> it so it does the lookup again.  Non-displayed fields such as
>> "version" are overwritten in _person  which breaks optimistic locking
>> (very bad).
>>
>>> The form component does accept a 'context' parameter which might  
>>> allow
>>> you to eliminate onActivate/onPassivate in favour of parameters to
>>> onPrepareFromForm() and a getter for the id.  I haven't played with
>>> that much so if you do pursue it, please report back :)
>>
>> Not sure about this.  Without onPassivate() wouldn't I lose the nice
>> bookmarkable URL (eg. http://localhost/myapp/personedit/123)?   
>> Without
>> onActivate(Long id) don't I lose the ability to navigate to this page
>> via pagelink?
>>
>>> And as a side note, shouldn't the second condition in
>>> onPrepareFromForm() be negated?  You want to lookup the person if  
>>> the
>>> ids _don't_ match, right?
>>
>> Well spotted - you're right.
>>
>>>> 2. Housekeeping - I'm nullifying _person in each method that
>>>> returns a
>>>> different page.  This seems clumsy.  Suggestions?
>>>
>>> Why?  Tapestry will take care of basic housekeeping.  If you're
>>> nulling it out to force a subsequent request to refresh the person's
>>> state, then a better alternative is to use flash persistence (or no
>>> persistence).
>>>
>>> On a similiar note, if you set the default persistence strategy of  
>>> the
>>> form to flash (via a metadata annotation), it will apply this to its
>>> validation tracker as well.  That would allow you to eliminate
>>> cleanupRender().
>>
>> I tried Persist("flash") on the form and it didn't do the trick.
>> Looks like 5.0.7 will put flash persistence on the form's
>> ValidationTracker which would be good.
>>
>>>> 3. Thread-safety - is there a thread-safety issue if the user has
>>>> opened a new window in same browser - can T5 tell them apart or do
>>>> they share the same HttpSession?
>>>
>>> Yes, they will share the same session, so anything you @Persist  
>>> could
>>> be a problem (unless its persisted on the client).  I don't know of
>>> anything tap5 does to help with this, so hopefully someone more
>>> knowledgeable will chime in.
>>>
>>> Again, i'd reconsider just looking up the person again.
>>
>> So does anyone else know if T5 has somehow dealt with the thread-
>> safety issue????
>>
>> Client persistence strategy solves it if the objects are small, but I
>> note the T5 doco is quite negative on the client persistence strategy
>> (http://tapestry.apache.org/tapestry5/tapestry-core/guide/persist.html 
>> )
>>
>>>> 4. onPrepareFromForm() - this looks a bit clumsy to me.  If the  
>>>> user
>>>> hits Reload, or if the user hits the Back button then "pagelinks"
>>>> back
>>>> in, onPrepareFromForm() wlll not call findPerson(...).  The _person
>>>> isn't refreshed at all!  Thoughts?
>>>
>>> Using flash persistence should solve both those issues, yeah?
>>>
>>> Ok, so it seems most of my comments boil down to: don't persist, and
>>> use flash/client strategies if you must :)
>>>
>>> Let me throw something else in to break the monotony: you can
>>> eliminate the conditional in onSuccess() by defining an onValidate()
>>> handler.  If onValidate() records errors, tapestry will know not to
>>> call onSuccess().
>>>
>>> Ok, hope that helps some.
>>> Cheers, lasitha.
>>
>> Thanks plenty, Iasitha.
>>
>> Geoff
>
>
>
> -- 
> Howard M. Lewis Ship
> Partner and Senior Architect at Feature50
>
> Creator Apache Tapestry and Apache HiveMind
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@tapestry.apache.org
> For additional commands, e-mail: users-help@tapestry.apache.org
>


---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tapestry.apache.org
For additional commands, e-mail: users-help@tapestry.apache.org


Re: T5: Edit page best practice?

Posted by Howard Lewis Ship <hl...@gmail.com>.
On Nov 27, 2007 3:56 AM, Geoff Callender
<ge...@gmail.com> wrote:
> Thanks, Iasatha, for the considered answer.  It seems that even with
> the brilliance of T5, the web is still a tricky area with trade-offs
> at every turn.

I agree; you can store a lot more persistent state on the server, but
then have the issue with the browser back button (not to mention
resource consumption on the server), or you can store data (or
prefereably, entity keys) on the client, but have to write a bit more
code.  One size does not fit all, so I just want T5 to get out of your
way on this kind of thing.


>
> On 27/11/2007, at 2:10 AM, lasitha wrote:
>
> >> 1. Persist - Is there an alternative?  It's used here to keep _person
> >> populated in case onSuccess() detects an error and redisplays the
> >> page.
> >
> > Well, you could always look up the person again from the
> > SecurityFinderService, but i suppose you don't want to pay for the
> > lookup again...  In that case, i don't know of an alternative.
>
> You're right - I don't want to do a lookup again - but principally
> because the aim is to redisplay the object with their changes, so a
> lookup is actually counter-productive.  A second, show-stopping reason
> is that _person is a detached object utilising optimistic locking.  If
> we keep refreshing the underlying object then we're messing up the
> whole principal of optimistic locking.
>
> > Note that you probably want to use flash (perhaps even client?)
> > persistence.
>
> Just switched to @Persist("flash") and got a couple of surprises:
> - leaving the validation in onSuccess(), the page redisplays with
> error but the person it shows is the latest from the DB!!!  The
> problem is that _person is nullified by the time that
> onPrepareFromForm() sees it so it does the lookup again.
> - moving the validation out of onSuccess() and into onValidate() as
> you suggested below fixes the display problem (very good) but
> nonetheless _person is nullified by the time onPrepareFromForm() sees
> it so it does the lookup again.  Non-displayed fields such as
> "version" are overwritten in _person  which breaks optimistic locking
> (very bad).
>
> > The form component does accept a 'context' parameter which might allow
> > you to eliminate onActivate/onPassivate in favour of parameters to
> > onPrepareFromForm() and a getter for the id.  I haven't played with
> > that much so if you do pursue it, please report back :)
>
> Not sure about this.  Without onPassivate() wouldn't I lose the nice
> bookmarkable URL (eg. http://localhost/myapp/personedit/123)?  Without
> onActivate(Long id) don't I lose the ability to navigate to this page
> via pagelink?
>
> > And as a side note, shouldn't the second condition in
> > onPrepareFromForm() be negated?  You want to lookup the person if the
> > ids _don't_ match, right?
>
> Well spotted - you're right.
>
> >> 2. Housekeeping - I'm nullifying _person in each method that
> >> returns a
> >> different page.  This seems clumsy.  Suggestions?
> >
> > Why?  Tapestry will take care of basic housekeeping.  If you're
> > nulling it out to force a subsequent request to refresh the person's
> > state, then a better alternative is to use flash persistence (or no
> > persistence).
> >
> > On a similiar note, if you set the default persistence strategy of the
> > form to flash (via a metadata annotation), it will apply this to its
> > validation tracker as well.  That would allow you to eliminate
> > cleanupRender().
>
> I tried Persist("flash") on the form and it didn't do the trick.
> Looks like 5.0.7 will put flash persistence on the form's
> ValidationTracker which would be good.
>
> >> 3. Thread-safety - is there a thread-safety issue if the user has
> >> opened a new window in same browser - can T5 tell them apart or do
> >> they share the same HttpSession?
> >
> > Yes, they will share the same session, so anything you @Persist could
> > be a problem (unless its persisted on the client).  I don't know of
> > anything tap5 does to help with this, so hopefully someone more
> > knowledgeable will chime in.
> >
> > Again, i'd reconsider just looking up the person again.
>
> So does anyone else know if T5 has somehow dealt with the thread-
> safety issue????
>
> Client persistence strategy solves it if the objects are small, but I
> note the T5 doco is quite negative on the client persistence strategy
> (http://tapestry.apache.org/tapestry5/tapestry-core/guide/persist.html)
>
> >> 4. onPrepareFromForm() - this looks a bit clumsy to me.  If the user
> >> hits Reload, or if the user hits the Back button then "pagelinks"
> >> back
> >> in, onPrepareFromForm() wlll not call findPerson(...).  The _person
> >> isn't refreshed at all!  Thoughts?
> >
> > Using flash persistence should solve both those issues, yeah?
> >
> > Ok, so it seems most of my comments boil down to: don't persist, and
> > use flash/client strategies if you must :)
> >
> > Let me throw something else in to break the monotony: you can
> > eliminate the conditional in onSuccess() by defining an onValidate()
> > handler.  If onValidate() records errors, tapestry will know not to
> > call onSuccess().
> >
> > Ok, hope that helps some.
> > Cheers, lasitha.
>
> Thanks plenty, Iasitha.
>
> Geoff



-- 
Howard M. Lewis Ship
Partner and Senior Architect at Feature50

Creator Apache Tapestry and Apache HiveMind

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tapestry.apache.org
For additional commands, e-mail: users-help@tapestry.apache.org


Re: T5: Edit page best practice?

Posted by Geoff Callender <ge...@gmail.com>.
Thanks, Iasatha, for the considered answer.  It seems that even with  
the brilliance of T5, the web is still a tricky area with trade-offs  
at every turn.

On 27/11/2007, at 2:10 AM, lasitha wrote:

>> 1. Persist - Is there an alternative?  It's used here to keep _person
>> populated in case onSuccess() detects an error and redisplays the  
>> page.
>
> Well, you could always look up the person again from the
> SecurityFinderService, but i suppose you don't want to pay for the
> lookup again...  In that case, i don't know of an alternative.

You're right - I don't want to do a lookup again - but principally  
because the aim is to redisplay the object with their changes, so a  
lookup is actually counter-productive.  A second, show-stopping reason  
is that _person is a detached object utilising optimistic locking.  If  
we keep refreshing the underlying object then we're messing up the  
whole principal of optimistic locking.

> Note that you probably want to use flash (perhaps even client?)  
> persistence.

Just switched to @Persist("flash") and got a couple of surprises:
- leaving the validation in onSuccess(), the page redisplays with  
error but the person it shows is the latest from the DB!!!  The  
problem is that _person is nullified by the time that  
onPrepareFromForm() sees it so it does the lookup again.
- moving the validation out of onSuccess() and into onValidate() as  
you suggested below fixes the display problem (very good) but  
nonetheless _person is nullified by the time onPrepareFromForm() sees  
it so it does the lookup again.  Non-displayed fields such as  
"version" are overwritten in _person  which breaks optimistic locking  
(very bad).

> The form component does accept a 'context' parameter which might allow
> you to eliminate onActivate/onPassivate in favour of parameters to
> onPrepareFromForm() and a getter for the id.  I haven't played with
> that much so if you do pursue it, please report back :)

Not sure about this.  Without onPassivate() wouldn't I lose the nice  
bookmarkable URL (eg. http://localhost/myapp/personedit/123)?  Without  
onActivate(Long id) don't I lose the ability to navigate to this page  
via pagelink?

> And as a side note, shouldn't the second condition in
> onPrepareFromForm() be negated?  You want to lookup the person if the
> ids _don't_ match, right?

Well spotted - you're right.

>> 2. Housekeeping - I'm nullifying _person in each method that  
>> returns a
>> different page.  This seems clumsy.  Suggestions?
>
> Why?  Tapestry will take care of basic housekeeping.  If you're
> nulling it out to force a subsequent request to refresh the person's
> state, then a better alternative is to use flash persistence (or no
> persistence).
>
> On a similiar note, if you set the default persistence strategy of the
> form to flash (via a metadata annotation), it will apply this to its
> validation tracker as well.  That would allow you to eliminate
> cleanupRender().

I tried Persist("flash") on the form and it didn't do the trick.   
Looks like 5.0.7 will put flash persistence on the form's  
ValidationTracker which would be good.

>> 3. Thread-safety - is there a thread-safety issue if the user has
>> opened a new window in same browser - can T5 tell them apart or do
>> they share the same HttpSession?
>
> Yes, they will share the same session, so anything you @Persist could
> be a problem (unless its persisted on the client).  I don't know of
> anything tap5 does to help with this, so hopefully someone more
> knowledgeable will chime in.
>
> Again, i'd reconsider just looking up the person again.

So does anyone else know if T5 has somehow dealt with the thread- 
safety issue????

Client persistence strategy solves it if the objects are small, but I  
note the T5 doco is quite negative on the client persistence strategy
(http://tapestry.apache.org/tapestry5/tapestry-core/guide/persist.html)

>> 4. onPrepareFromForm() - this looks a bit clumsy to me.  If the user
>> hits Reload, or if the user hits the Back button then "pagelinks"  
>> back
>> in, onPrepareFromForm() wlll not call findPerson(...).  The _person
>> isn't refreshed at all!  Thoughts?
>
> Using flash persistence should solve both those issues, yeah?
>
> Ok, so it seems most of my comments boil down to: don't persist, and
> use flash/client strategies if you must :)
>
> Let me throw something else in to break the monotony: you can
> eliminate the conditional in onSuccess() by defining an onValidate()
> handler.  If onValidate() records errors, tapestry will know not to
> call onSuccess().
>
> Ok, hope that helps some.
> Cheers, lasitha.

Thanks plenty, Iasitha.

Geoff

Re: T5: Edit page best practice?

Posted by lasitha <la...@gmail.com>.
On Nov 26, 2007 4:57 PM, Geoff Callender
<ge...@gmail.com> wrote:
> Hi,
>
> I'm looking for suggestions on how to improve this EditPerson page to
> T5 best practice.

Hello Geoff.  See comments below, though i can't make any claims to
best-practice myself :)

>   private Long _personId;
>
>   @Persist
>   private Person _person;
>
>   public void onActivate(Long id) { _personId = id; }
>
>   public Long onPassivate() { return _personId; }
>
>   void onPrepareFromForm() {
>     if (_person == null || _person.getId().equals(_personId)) {
>       _person = getSecurityFinderService().findPerson(_personId);
>     }
>   }
>
>   Object onSuccess() {
>     if (...a bit of validation logic detects an error...) {
>       _form.recordError(...);
>       return null;
>     }
>     etc...
>     _person = null;
>     return _nextPage;
>   }
>
>   Object onActionFromCancel() {
>     _person = null;
>     return _previousPage;
>   }
>
>   void cleanupRender() {
>     _form.clearErrors();
>   }
>
> Some notes and questions:
>
> 1. Persist - Is there an alternative?  It's used here to keep _person
> populated in case onSuccess() detects an error and redisplays the page.

Well, you could always look up the person again from the
SecurityFinderService, but i suppose you don't want to pay for the
lookup again...  In that case, i don't know of an alternative.

Note that you probably want to use flash (perhaps even client?) persistence.

The form component does accept a 'context' parameter which might allow
you to eliminate onActivate/onPassivate in favour of parameters to
onPrepareFromForm() and a getter for the id.  I haven't played with
that much so if you do pursue it, please report back :)

And as a side note, shouldn't the second condition in
onPrepareFromForm() be negated?  You want to lookup the person if the
ids _don't_ match, right?

> 2. Housekeeping - I'm nullifying _person in each method that returns a
> different page.  This seems clumsy.  Suggestions?

Why?  Tapestry will take care of basic housekeeping.  If you're
nulling it out to force a subsequent request to refresh the person's
state, then a better alternative is to use flash persistence (or no
persistence).

On a similiar note, if you set the default persistence strategy of the
form to flash (via a metadata annotation), it will apply this to its
validation tracker as well.  That would allow you to eliminate
cleanupRender().

> 3. Thread-safety - is there a thread-safety issue if the user has
> opened a new window in same browser - can T5 tell them apart or do
> they share the same HttpSession?

Yes, they will share the same session, so anything you @Persist could
be a problem (unless its persisted on the client).  I don't know of
anything tap5 does to help with this, so hopefully someone more
knowledgeable will chime in.

Again, i'd reconsider just looking up the person again.

> 4. onPrepareFromForm() - this looks a bit clumsy to me.  If the user
> hits Reload, or if the user hits the Back button then "pagelinks" back
> in, onPrepareFromForm() wlll not call findPerson(...).  The _person
> isn't refreshed at all!  Thoughts?

Using flash persistence should solve both those issues, yeah?

Ok, so it seems most of my comments boil down to: don't persist, and
use flash/client strategies if you must :)

Let me throw something else in to break the monotony: you can
eliminate the conditional in onSuccess() by defining an onValidate()
handler.  If onValidate() records errors, tapestry will know not to
call onSuccess().

Ok, hope that helps some.
Cheers, lasitha.

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@tapestry.apache.org
For additional commands, e-mail: users-help@tapestry.apache.org


Re: T5: Edit page best practice?

Posted by Geoff Callender <ge...@gmail.com>.
Hi,

Here's my new attempt at incorporating T5 best practice from this  
thread into my EditPerson page.  I'm looking for suggestions on how to  
improve it further.

	private Long _personId;
	
	@Persist("client")
	// Persistence is needed here because this is a detached Entity  
Bean.  When we call the service to
	// accept our changes, it will need its id and version fields intact  
to be able to do optimistic
	// locking check and a successful merge. "client" is used so that  
even if you use the Back button
	// to get to this page, we will have the right Person object.
	private Person _person;
	
	@Persist("client")
	private boolean _needsRefresh = true;

	void onActivate(Long id) throws Exception { _personId = id; }

	Long onPassivate() { return _person.getId(); }

	void setupRender() throws Exception {
		if (_needsRefresh) {
			_person = getPersonService().findPerson(_personId);
		}
		_needsRefresh = true;
	}

	void onValidate() throws Exception {
		_needsRefresh = false;
		if (...a bit of validation logic detects an error...) {
			_form.recordError(...);
		}
	}
	
	Object onSuccess() {
		try {
			getPersonService().changePerson(_person);
			_nextPage.onActivate(_personId);
			_needsRefresh = true;
			return _nextPage;
		}
		catch (Exception e) {
			_form.recordError(ExceptionUtil.getRootCause(e));
			return null;
		}
	}

	void cleanupRender() {
		   _form.clearErrors();
	}
	
Some notes and questions:

1. Detached object - Person is a detached object.  I am avoiding  
refreshing it every time in setupRender() because a) it would  
overwrite the user's changes, and b) it would defeat optimistic  
locking: if someone else has changed the object then I do want  
getPersonService().changePerson(_person) to reject the transaction  
when Save is pressed.

2. _needsRefresh - it's just a method of preventing the refresh when  
we re-display due to error.  This seems clumsy.  Suggestions?

3. Thread-safety - I'm using "client" persistence to avoid the whole  
thread-safety issue caused by the user opening a new window or tabs in  
same browser (T5 can't tell them apart so they share the same  
HttpSession).

Comments please!

Thanks,

Geoff


On 26/11/2007, at 10:27 PM, Geoff Callender wrote:

> Hi,
>
> I'm looking for suggestions on how to improve this EditPerson page  
> to T5 best practice.
>
> 	private Long _personId;
>
> 	@Persist
> 	private Person _person;
>
> 	public void onActivate(Long id) { _personId = id; }
>
> 	public Long onPassivate() { return _personId; }
> 	
> 	void onPrepareFromForm() {
> 		if (_person == null || _person.getId().equals(_personId)) {
> 			_person = getSecurityFinderService().findPerson(_personId);
> 		}
> 	}
>
> 	Object onSuccess() {
> 		if (...a bit of validation logic detects an error...) {
> 			_form.recordError(...);
> 			return null;
> 		}
> 		etc...
> 		_person = null;
> 		return _nextPage;
> 	}
>
> 	Object onActionFromCancel() {
> 		_person = null;
> 		return _previousPage;
> 	}
>
> 	void cleanupRender() {
> 		_form.clearErrors();
> 	}
>
> Some notes and questions:
>
> 1. Persist - Is there an alternative?  It's used here to keep  
> _person populated in case onSuccess() detects an error and  
> redisplays the page.
>
> 2. Housekeeping - I'm nullifying _person in each method that returns  
> a different page.  This seems clumsy.  Suggestions?
>
> 3. Thread-safety - is there a thread-safety issue if the user has  
> opened a new window in same browser - can T5 tell them apart or do  
> they share the same HttpSession?
>
> 4. onPrepareFromForm() - this looks a bit clumsy to me.  If the user  
> hits Reload, or if the user hits the Back button then "pagelinks"  
> back in, onPrepareFromForm() wlll not call findPerson(...).  The  
> _person isn't refreshed at all!  Thoughts?
>
> Thanks,
>
> Geoff