Kalle,
I'm really glad you asked this question because it got me thinking,
and I've realised I'm putting too much into onActivate(..). The
problem is that onActivate() is called regardless of whether the user
clicks on submit or an ActionLink. The database activity in
onActivate() is not usually necessary for an ActionLink.
I think this is better...
* Use onActivate() to read in the context and nothing more.
* Use setupRender() to set up display-only objects and fields. You can
also use getters if they have low overhead or you take care to lazy-
load.
* Use onPrepare() to set up editable objects and fields. onPrepare()
is triggered by form render and form submit. If there's more than one
form then use multiple onPrepares, eg. onPrepareForFormX(),
onPrepareForFormY().
So the examples become...
UserCreate.java:
void onPrepare() {
// Instantiate a User for the page data to overlay.
_user = new User();
_user.setActive(true);
}
UserEdit.java:
Long onPassivate() {
return _userId;
}
void onActivate(Long id) {
_userId = id;
}
void onPrepare() {
try {
_user = getSecurityFinderService().findUser(_userId);
}
catch (DoesNotExistException e) {
// Handle null user in the template
}
}
void setupRender() {
_userRoles =
getSecurityFinderService().findUserRolesShallowishByUser(_userId);
}
I've almost finished reworking JumpStart to suit this pattern and it
works much better. I'll release a few days from now as version 4.5.2.
The previous technique is a hangover from when I thought onActivate()
should validate the context and throw an exception if it was invalid.
I changed my mind on that quite a while ago and wrote this:
http://jumpstart.doublenegative.com.au:8080/jumpstart/examples/infrastructure/handlingabadcontext/1
Thanks again,
Geoff
On 03/09/2009, at 4:53 AM, Kalle Korhonen wrote:
Thanks Geoff, no can't see any major issues. Agree on the
initialization of roles in setupRender() since they really are used
for rendering only. Heavy use of lazy getter-based initialization has
gotten me into trouble before, but I may revisit my coding best
practice. And that's really the only thing I was after - to review my
understanding and practices against others'.
Kalle
On Wed, Sep 2, 2009 at 8:12 AM, Geoff
Callender<geoff.callender.jumpst...@gmail.com> wrote:
To set the scene: in the EditUser example, the user is displayed in
a form
and the user roles are displayed below it in a grid with
ActionLinks for
View, Edit, and Delete on each row.
The reason it is OK to get the user roles in setupRender() is
because they
are not editable - all we need is the id for the context of each
ActionLink
in the row. If you come back to this screen and hit a link then it
will
still work.
Actually, if the user roles were editable you'd probably get them in
onPrepare() rather than onActivate(), just as in the EditableLoop1
example.
I can't recall why I preferred onPrepare() over onActivate() but I
think it
was because it's called exactly as often as it is needed whereas
onActivate() is called often.
If a ValueEncoder is used with the loop then it becomes OK to get the
entities in setupRender() and encode them in onPrepare(). This is
demonstrated in EditableLoopUsingEncoder1.
Back in the user form, user.salutation is chosen from a Select
list. The
list doesn't need to be built in onActivate() either. It's done in
getSalutations() and works just fine with the Back button.
Can you see a hole in this that I've missed?
Geoff
http://jumpstart.doublenegative.com.au:8080/jumpstart/
On 02/09/2009, at 11:56 PM, Kalle Korhonen wrote:
On Tue, Sep 1, 2009 at 11:30 PM, Geoff
Callender<geoff.callender.jumpst...@gmail.com> wrote:
The key to it is this snippet: "if the stuff you are setting up
is not
needed for component event requests, consider putting it
elsewhere". If I
understand your example correctly, the object you are creating IS
needed
for
a component event request so DO put it in onActivate(...).
Yes, that's just the thing. Whether it's entities or translators or
anything, pretty much all of that "stuff" is needed for event
requests. Can you come up with a good example for initializing
something that is safe to do in setupRender()? Obviously if that
object is really needed just for rendering (as the operation name
suggests) then it's the right place for it, but those cases are few
and far between. Even in your example, the userRoles are most
certainly needed in event requests - obviously you can just return
error "user doesn't have the proper role for the operation" but it'd
be more usable to just do that in onActivate as well.
But with edit, if you want optimistic locking then you have to
include
the
entity's version attribute in the form:
in which case if you submit, press Back, then submit, you'll get an
exception thrown by the persistence mechanism about optimistic
locking -
it
tells us that the User has changed since the page was first
displayed.
It's
correct, and all you do is hit refresh and try again. If you
don't want
optimistic locking then don't put the entity's version attribute
in the
form.
Agree completely, that's a good pattern to follow.
Kalle
On 02/09/2009, at 4:03 PM, Kalle Korhonen wrote:
On Tue, Sep 1, 2009 at 9:50 PM, Geoff
Callender<geoff.callender.jumpst...@gmail.com> wrote:
Good question. Yes, it does still seem to me to be best
practice and
no,
I
don't see it breaking the back button. Can you give an example?
Assuming you use something else than session or client
persistence and
you initialize (create) an object, set it as a value of some page
property in your setupRender(), then submit the form, press the
back
button and re-submit the formit, the object will be null
(because it
was initialized in setupRender() that was never invoked rather
than
onActivate()).
Kalle
On 01/09/2009, at 8:37 AM, Kalle Korhonen wrote:
Hey Geoff,
I recall reading at some point that you had recommended not
using
onActivate() for all initialization purposes, and sure enough,
I dug
it up and at
http://jumpstart.doublenegative.com.au:8080/jumpstart/examples/navigation/onactivateandonpassivate/3
you say "It can be tempting to put lots of setup code into
onActivate(...). However, if the stuff you are setting up is not
needed for component event requests, consider putting it
elsewhere,
such as setupRender() or getter methods."
Does that still reflect your current understanding of the best
practices? The caveat with using setupRender() (or anything else
except for onActivate()) is that even for non-ajax
applications, it
"breaks the back button", i.e. if a user goes back in history
and say
re-submits a form (for one reason or another) the objects
required by
the page/form are not initialized. Obviously it depends on the
case
what the application should do, but you loose half the
benefits of
redirect-after-post if your require a refresh before a page is
usable
again. Do you simple prefer rendering an an error in the back
button/direct form submit case or do you generally do all
initialization in onActivate()?
Kalle
On Tue, Aug 4, 2009 at 7:59 PM, Geoff
Callender<geoff.callender.jumpst...@gmail.com> wrote:
Hi all,
JumpStart 4.4 is now available. It's a tidying up release:
the
structure's
a bit neater and it uses the latest OpenEJB, ie. 3.1.1.
Use it live:
http://jumpstart.doublenegative.com.au:8080/jumpstart/
or download it:
http://jumpstart.doublenegative.com.au
And if someone can figure how to get Jetty/OpenEJB in Eclipse
to use
the
libs and classes in the WAR instead of having to be spoon-fed a
classpath,
then please let me know. I suspect it's a class-loading issue
that
might
soon be solved by http://code.google.com/p/embed-openejb-in-eclipse/
.
Cheers,
Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tapestry.apache.org
For additional commands, e-mail: users-h...@tapestry.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tapestry.apache.org
For additional commands, e-mail: users-h...@tapestry.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tapestry.apache.org
For additional commands, e-mail: users-h...@tapestry.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tapestry.apache.org
For additional commands, e-mail: users-h...@tapestry.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tapestry.apache.org
For additional commands, e-mail: users-h...@tapestry.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tapestry.apache.org
For additional commands, e-mail: users-h...@tapestry.apache.org
