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
