Hm. Apologies, I haven't been following this thread very closely.
Preface: I realize JPA compatibility isn't necessarily a current goal,
but hear me out. :)
The way I read the JPA spec is that @PrePersist is supposed to be
called immediately before database insert.
The problem seems to be a mismatch between the "cayenne way" and the
"JPA" way. In cayenne, you register your object, then make your
changes (especially relationships), then commit to the DB. In JPA,
you would setup the entire bean structure (including relationships)
before calling EntityManager.persist, which results directly in a DB
commit. Ie, persist is like context.registerNewObject() +
context.commitChanges() rolled into one, except the scope is limited a
single object and associated changes. I don't have any qualms about
adding additional lifecycle listeners to make cayenne more useful in
general and/or to address the impedence with JPA, but if we're going
to keep lifecycle listeners with the same names as the JPA ones, we
should also keep the same semantics, which means calling PrePersist
immediately before insert. Otherwise, we'll confuse users new to
cayenne but familiar with JPA. So, I would propose something more like:
postRegister <- called when a non-persisted object is registered with
a data context. You could even have a preRegister, but that's probably
overkill.
prePersist <- just before DB insert
postPersist <- just after DB insert
preUpdate < - just before DB update of existing object
postUpdate <- just after DB update
preRemove <- just before DB delete
postRemove <-just after DB delete
postLoad <- just after a committed obj. is fetched from the DB.
Alternatively, as mentioned below we could rename all callbacks to
align with the "cayenne way". But, again, if we're going to keep the
JPA names, we should keep the behavior as JPA-like as possible.
Robert
On Sep 23, 2009, at 9/233:14 PM , Andrus Adamchik wrote:
Hi Andrey,
Thanks for your comments. This made me tweak my initial suggestion,
resulting in something that hopefully makes more sense. So here is
an attempt to show the full callback picture with suggested changes
(grouping things by the object pre-commit state):
COMMITTED:
postLoad
NEW:
prePersist (called after registration in context)
preInsert (POTENTIAL NEW CALLBACK; called before commit) [1]
postPersist (called after commit)
MODIFIED:
preUpdate (called before commit)
postUpdate (called after commit)
DELETED:
preRemove (called before deleting in context, and before delete
rules are evaluated)
preUpdate (POTENTIAL REMOVED CALLBACK; called before commit) [2]
postRemove (called after commit)
postUpdate (POTENTIAL REMOVED CALLBACK; called after commit) [3]
[1] We definitely need this callback here for the new objects right
before commit. My suggestion of reusing "preUpdate" here was based
on the fact that all my pre-commit code for NEW and MODIFIED objects
is exactly the same, so it seems that from implementation standpoint
the event is almost identical. On the other hand, "postUpdate" is
indeed redundant for NEW objects. So using pre/postUpdate for NEW
does break the symmetry...
[2] While there's no overlap with a real "preUpdate" in possible
callback functionality here, and we need to remove this one, this
leaves a gap (just like with NEW objects) that needs to be filled
with some "preCommitDeleted" callback.
[3] Definitely needs to be removed. It is being passed TRANSIENT
objects and is really out of place (though still JPA compatible :-)).
I guess completely renaming callbacks to be aligned with Cayenne
(rather than JPA) lifecycle could be a good thing (persist -> insert|
create, remove -> delete, etc.), and may go a long way in clarifying
what is called and when. It is the end of a long day here, so I am
not ready to suggest any better naming right away, but regardless of
the naming, if we just make the changes above, I think we can
provide the existing users a clear set of instructions on how to
migrate existing callbacks to the new set. FWIW I'll be able to try
it myself on a large application without waiting for the B1 release.
Andrus
On Sep 23, 2009, at 8:53 PM, Andrey Razumovsky wrote:
Hi Andrus,
I also suffered missing "preInsert" listener a number of times.
But, I think
the moment of when listeners are called (and which of them are
called)
should be easily guessed by what is happening to the object.
1. call pre/postUpdate for new objects. So new objects will have
pre/postPersist as well as pre/postUpdate callbacks. Of course
the main
motivation is that preUpdate is called before save, not after
context
insert, so all the relationships are in place.
So there will be four calls in total at object creation. I don't
think it is
transparent that pre- and postUpdate will be called - after all, no
updates
are ever made!
2. stop calling pre/postUpdate for removed objects. This is
causing grief
when we need to access relationships of a deleted object.
I'm confused. Are pre/postUpdate methods really called for deleted
objects?
If this is so, it definitely must be stopped I think. Only pre/
postRemove
should be called..
What will be wrong if we just add new type of event - preInsert?
Users
aren't obligated to implement any listener class, after all. I'm
afraid the
changes you suggest will break a lot of existing code
Thanks,
