Do we need DaoUtils?
|
|
dxxvi
|
Hi Matt,
I look at the DaoUtils class and ... to my understanding, this class's methods are used to get the object id of an object by searching for methods with @Id or @EmbeddedId annotations. Then if the objId is not null, it thinks that this object was persisted already and then calls the merge method of the entityManager. I think it's not right in the case of composite ids. Because with composite id, the id is not null although that object is new. Anyway, do we need to know when to use persist and when to use merge? karldmoore from Spring forum told me about this and it seems to me that we can use merge all the time. I haven't tried it yet because I'm still having trouble with persisting an object with a composite id. Anybody has an example of using composite id with JPA? Matt, I appreciate your AppFuse. It helps me alot. Thank you. PS: I'm talking about JPA in AppFuse 2.0M3. |
Bryan Noll
|
I'm reading and responding to that thread too. If you can do
semantically what the 'save' method is doing (insert if new, update if modified) without the use of DaoUtils and what it does, I'm all for it. I simply could not get it to work that way when I was implementing that code, regardless of what the documentation says. For example, a quick modification to do what you're talking about is to : Change GenericDaoJpa.save from: public void save(T object) { Object objId = DaoUtils.getPersistentId(object); if (objId == null) { this.entityManager.persist(object); } else { this.entityManager.merge(object); } } ... to: public void save(T object) { this.entityManager.merge(object); } When I do that (and do the same thing for UniversalDaoJpa.save), and then run mvn test the jpa-hibernate module, I get the following: UniversalDaoTest failure: http://rafb.net/p/epCRgU65.html UserDaoTest failure: http://rafb.net/p/Wb4dR394.html If you look at these tests, you'll see that in both cases, it's doing this... assertNotNull(user.getId()); ... aka... making the assumption that the merge method knows how to insert a new object (one without an id yet) into the database. So, this either doesn't work exactly how the documentation seems to indicate it should, or I've done something wrong in the test (maybe in terms of extending org.springframework.test.jpa.AbstractJpaTests and the way it deals with transactions). I would like to change that method to simply call entityManager.merge myself, but don't see how unless there is something wrong with how the tests are working. I tried pretty hard to make it work like you want when I wrote, and it just didn't for me. Hopefully someone can shed some light on it so we can get rid of a bunch of this code. --Bryan dxxvi wrote: > Hi Matt, > > I look at the DaoUtils class and ... to my understanding, this class's > methods are used to get the object id of an object by searching for methods > with @Id or @EmbeddedId annotations. Then if the objId is not null, it > thinks that this object was persisted already and then calls the merge > method of the entityManager. I think it's not right in the case of composite > ids. Because with composite id, the id is not null although that object is > new. > > Anyway, do we need to know when to use persist and when to use merge? > karldmoore from Spring forum told me about this and it seems to me that we > can use merge all the time. I haven't tried it yet because I'm still having > trouble with persisting an object with a composite id. > > Anybody has an example of using composite id with JPA? > > Matt, I appreciate your AppFuse. It helps me alot. Thank you. > > PS: I'm talking about JPA in AppFuse 2.0M3. > --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
||||||||||||||||
|
Bryan Noll
|
For anyone wanting a cross-reference to the other thread too...
http://forum.springframework.org/showthread.php?t=36064 Bryan Noll wrote: > I'm reading and responding to that thread too. If you can do > semantically what the 'save' method is doing (insert if new, update if > modified) without the use of DaoUtils and what it does, I'm all for it. > > I simply could not get it to work that way when I was implementing > that code, regardless of what the documentation says. > For example, a quick modification to do what you're talking about is to : > > Change GenericDaoJpa.save from: > > public void save(T object) { > Object objId = DaoUtils.getPersistentId(object); > if (objId == null) { > this.entityManager.persist(object); > } else { > this.entityManager.merge(object); > } > } > > ... to: > > public void save(T object) { > this.entityManager.merge(object); > } > > When I do that (and do the same thing for UniversalDaoJpa.save), and > then run mvn test the jpa-hibernate module, I get the following: > > UniversalDaoTest failure: http://rafb.net/p/epCRgU65.html > UserDaoTest failure: http://rafb.net/p/Wb4dR394.html > > If you look at these tests, you'll see that in both cases, it's doing > this... > > assertNotNull(user.getId()); > > ... aka... making the assumption that the merge method knows how to > insert a new object (one without an id yet) into the database. > > So, this either doesn't work exactly how the documentation seems to > indicate it should, or I've done something wrong in the test (maybe in > terms of extending org.springframework.test.jpa.AbstractJpaTests and > the way it deals with transactions). I would like to change that > method to simply call entityManager.merge myself, but don't see how > unless there is something wrong with how the tests are working. > I tried pretty hard to make it work like you want when I wrote, and it > just didn't for me. Hopefully someone can shed some light on it so we > can get rid of a bunch of this code. > > --Bryan > > > > > > dxxvi wrote: >> Hi Matt, >> >> I look at the DaoUtils class and ... to my understanding, this class's >> methods are used to get the object id of an object by searching for >> methods >> with @Id or @EmbeddedId annotations. Then if the objId is not null, it >> thinks that this object was persisted already and then calls the merge >> method of the entityManager. I think it's not right in the case of >> composite >> ids. Because with composite id, the id is not null although that >> object is >> new. >> >> Anyway, do we need to know when to use persist and when to use merge? >> karldmoore from Spring forum told me about this and it seems to me >> that we >> can use merge all the time. I haven't tried it yet because I'm still >> having >> trouble with persisting an object with a composite id. >> >> Anybody has an example of using composite id with JPA? >> >> Matt, I appreciate your AppFuse. It helps me alot. Thank you. >> >> PS: I'm talking about JPA in AppFuse 2.0M3. >> > --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
||||||||||||||||
|
Bryan Noll
|
Some javascript/style in this post has been disabled (why?)
Furthermore...According to this (http://www.hibernate.org/hib_docs/entitymanager/reference/en/html_single/#d0e897)... Merging vs. saveOrUpdate/saveOrUpdateCopy Merging in EJB3 is similar to the saveOrUpdateCopy() method in native Hibernate. However, it is not the same as the saveOrUpdate() method, the given instance is not reattached with the persistence context, but a managed instance is returned by the merge() method. So, let's go look at what saveOrUpdateCopy says: org.hibernate.Session doesn't have that method, but org.hibernate.classic.Session does (http://www.hibernate.org/hib_docs/v3/api/org/hibernate/classic/Session.html)... All it tells us is that it's been deprecated and that we should go look at Session.merge, which takes us back to org.hibernate.Session http://www.hibernate.org/hib_docs/v3/api/org/hibernate/Session.html#merge(java.lang.Object) ... which tells us this: Copy the state of the given object onto the persistent object with the same identifier. If there is no persistent instance currently associated with the session, it will be loaded. Return the persistent instance. If the given instance is unsaved, save a copy of and return it as a newly persistent instance. The given instance does not become associated with the session. This operation cascades to associated instances if the association is mapped with cascade="merge". The semantics of this method are defined by JSR-220. Evidently, the important part here must be "The given instance does not become associated with the session." At any rate, that definition is not the same as the one for Session.saveOrUpdate, as in: http://www.hibernate.org/hib_docs/v3/api/org/hibernate/Session.html#saveOrUpdate(java.lang.Object) Either save() or update() the given instance, depending upon the value of its identifier property. By default the instance is always saved. This behaviour may be adjusted by specifying an unsaved-value attribute of the identifier property mapping. This operation cascades to associated instances if the association is mapped with cascade="save-update". Just to make this message already longer than it is, here is the javadoc description of the deprecated saveOrUpdateCopy method. Copy the state of the given object onto the persistent object with the same identifier. If there is no persistent instance currently associated with the session, it will be loaded. Return the persistent instance. If the given instance is unsaved or does not exist in the database, save it and return it as a newly persistent instance. Otherwise, the given instance does not become associated with the session. My brain hurts a little more every time I re-read one of these javadoc comments. Bryan Noll wrote: For anyone wanting a cross-reference to the other thread too... |
||||||||||||||||
|
Bryan Noll
|
Some javascript/style in this post has been disabled (why?)
Allright... ignore all my long winded crap, I re-read the stuff enough
times that I got it. In order to get this to work, we'd have to change
the UniversalDao (and UniversalDaoJpa of course) to return the result
of the entityManager.save method. (The same obviously applies to
UserDao and UserDaoJpa.)EG... UniversalDao.save should become: public Object save(Object o); UniversalDaoJpa.save should become: public Object save(Object o) { return this.entityManager.merge(o); } Line 41 of UniversalDaoTest should change from: universalDao.save(user); ... to: user = (User)universalDao.save(user); That makes the test past. AppFuse folks... what do you all think about making this modification? I say we should do it, but this also means that to stay consistent we'd have to modify the Dao interface and implementation of the 'save' method in the hibernate module and the iBatis module as well. I don't see that this would cause a problem. Let me know what you think. Bryan Noll wrote: Furthermore... |
||||||||||||||||
|
Bryan Noll
|
Some javascript/style in this post has been disabled (why?)
One more tiny wrench in this issue...Session.saveOrUpdate (what is being employed in UniversalDaoHibernate.save) returns void. EntityManager.merge (what is being employed in UniversalDaoJpa.save) returns an object. Session.merge (an option that we're currently not using in any *DaoHibernate implementations, returns an object as well). This means we'd have to switch from Session.saveOrUpdate to Session.merge, which doesn't bother me. Looking at the iBatis implementation UniversalDaoiBatis.save, it seems like simply returning the 'object' param instead of void should do the trick here. I'll try all this stuff to make sure it works how I think it ought to and make sure the tests are passing. Assuming they do, is everyone alright with me making these changes? Please let me know if you see some ramifications that I don't. Thanks... Bryan Bryan Noll wrote: Allright... ignore all my long winded crap, I re-read the stuff enough times that I got it. In order to get this to work, we'd have to change the UniversalDao (and UniversalDaoJpa of course) to return the result of the entityManager.save method. (The same obviously applies to UserDao and UserDaoJpa.) |
||||||||||||||||
mraible
|
I believe we can leave these methods as void because even if an object
is returned, we don't need to pass it back to the user. Thanks to pass-by-reference, the object they passed into the method will get manipulated by Hibernate, JPA, etc. Matt On 3/13/07, Bryan Noll <[hidden email]> wrote: > > One more tiny wrench in this issue... > > Session.saveOrUpdate (what is being employed in UniversalDaoHibernate.save) > returns void. > > EntityManager.merge (what is being employed in UniversalDaoJpa.save) > returns an object. > > Session.merge (an option that we're currently not using in any > *DaoHibernate implementations, returns an object as well). > > > This means we'd have to switch from Session.saveOrUpdate to Session.merge, > which doesn't bother me. > > Looking at the iBatis implementation UniversalDaoiBatis.save, it seems like > simply returning the 'object' param instead of void should do the trick > here. I'll try all this stuff to make sure it works how I think it ought to > and make sure the tests are passing. > > Assuming they do, is everyone alright with me making these changes? Please > let me know if you see some ramifications that I don't. > > Thanks... > > Bryan > > > Bryan Noll wrote: > Allright... ignore all my long winded crap, I re-read the stuff enough > times that I got it. In order to get this to work, we'd have to change the > UniversalDao (and UniversalDaoJpa of course) to return the result of the > entityManager.save method. (The same obviously applies to UserDao and > UserDaoJpa.) > > EG... > > UniversalDao.save should become: > > public Object save(Object o); > > > UniversalDaoJpa.save should become: > > public Object save(Object o) { > return this.entityManager.merge(o); > } > > Line 41 of UniversalDaoTest should change from: > > universalDao.save(user); > > ... to: > > user = (User)universalDao.save(user); > > > That makes the test past. > > > AppFuse folks... what do you all think about making this modification? I > say we should do it, but this also means that to stay consistent we'd have > to modify the Dao interface and implementation of the 'save' method in the > hibernate module and the iBatis module as well. I don't see that this would > cause a problem. Let me know what you think. > > Bryan Noll wrote: > Furthermore... > > According to this > (http://www.hibernate.org/hib_docs/entitymanager/reference/en/html_single/#d0e897)... > > Merging vs. saveOrUpdate/saveOrUpdateCopy > > Merging in EJB3 is similar to the saveOrUpdateCopy() method in native > Hibernate. However, it is not the same as the saveOrUpdate() method, the > given instance is not reattached with the persistence context, but a managed > instance is returned by the merge() method. > > So, let's go look at what saveOrUpdateCopy says: > > org.hibernate.Session doesn't have that method, but > org.hibernate.classic.Session does > (http://www.hibernate.org/hib_docs/v3/api/org/hibernate/classic/Session.html)... > > All it tells us is that it's been deprecated and that we should go look at > Session.merge, which takes us back to org.hibernate.Session > > > http://www.hibernate.org/hib_docs/v3/api/org/hibernate/Session.html#merge(java.lang.Object) > > ... which tells us this: > > Copy the state of the given object onto the persistent object with the same > identifier. If there is no persistent instance currently associated with the > session, it will be loaded. Return the persistent instance. If the given > instance is unsaved, save a copy of and return it as a newly persistent > instance. The given instance does not become associated with the session. > This operation cascades to associated instances if the association is mapped > with cascade="merge". > > The semantics of this method are defined by JSR-220. > > > Evidently, the important part here must be "The given instance does not > become associated with the session." At any rate, that definition is not > the same as the one for Session.saveOrUpdate, as in: > > http://www.hibernate.org/hib_docs/v3/api/org/hibernate/Session.html#saveOrUpdate(java.lang.Object) > > Either save() or update() the given instance, depending upon the value of > its identifier property. By default the instance is always saved. This > behaviour may be adjusted by specifying an unsaved-value attribute of the > identifier property mapping. This operation cascades to associated instances > if the association is mapped with cascade="save-update". > > > > Just to make this message already longer than it is, here is the javadoc > description of the deprecated saveOrUpdateCopy method. > > Copy the state of the given object onto the persistent object with the same > identifier. If there is no persistent instance currently associated with the > session, it will be loaded. Return the persistent instance. If the given > instance is unsaved or does not exist in the database, save it and return it > as a newly persistent instance. Otherwise, the given instance does not > become associated with the session. > > > My brain hurts a little more every time I re-read one of these javadoc > comments. > > > Bryan Noll wrote: > For anyone wanting a cross-reference to the other thread too... > > http://forum.springframework.org/showthread.php?t=36064 > > Bryan Noll wrote: > > I'm reading and responding to that thread too. If you can do semantically > what the 'save' method is doing (insert if new, update if modified) without > the use of DaoUtils and what it does, I'm all for it. > > I simply could not get it to work that way when I was implementing that > code, regardless of what the documentation says. > For example, a quick modification to do what you're talking about is to : > > Change GenericDaoJpa.save from: > > public void save(T object) { > Object objId = DaoUtils.getPersistentId(object); > if (objId == null) { > this.entityManager.persist(object); > } else { > this.entityManager.merge(object); > } > } > > ... to: > > public void save(T object) { > this.entityManager.merge(object); > } > > When I do that (and do the same thing for UniversalDaoJpa.save), and then > run mvn test the jpa-hibernate module, I get the following: > > UniversalDaoTest failure: http://rafb.net/p/epCRgU65.html > UserDaoTest failure: http://rafb.net/p/Wb4dR394.html > > If you look at these tests, you'll see that in both cases, it's doing > this... > > assertNotNull(user.getId()); > > ... aka... making the assumption that the merge method knows how to insert > a new object (one without an id yet) into the database. > > So, this either doesn't work exactly how the documentation seems to > indicate it should, or I've done something wrong in the test (maybe in terms > of extending org.springframework.test.jpa.AbstractJpaTests > and the way it deals with transactions). I would like to change that method > to simply call entityManager.merge myself, but don't see how unless there is > something wrong with how the tests are working. > I tried pretty hard to make it work like you want when I wrote, and it just > didn't for me. Hopefully someone can shed some light on it so we can get > rid of a bunch of this code. > > --Bryan > > > > > > dxxvi wrote: > > Hi Matt, > > I look at the DaoUtils class and ... to my understanding, this class's > methods are used to get the object id of an object by searching for methods > with @Id or @EmbeddedId annotations. Then if the objId is not null, it > thinks that this object was persisted already and then calls the merge > method of the entityManager. I think it's not right in the case of > composite > ids. Because with composite id, the id is not null although that object is > new. > > Anyway, do we need to know when to use persist and when to use merge? > karldmoore from Spring forum told me about this and it seems to me that we > can use merge all the time. I haven't tried it yet because I'm still having > trouble with persisting an object with a composite id. > > Anybody has an example of using composite id with JPA? > > Matt, I appreciate your AppFuse. It helps me alot. Thank you. > > PS: I'm talking about JPA in AppFuse 2.0M3. > > > -- http://raibledesigns.com --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
||||||||||||||||
|
Bryan Noll
|
inline...
Matt Raible wrote: > I believe we can leave these methods as void because even if an object > is returned, we don't need to pass it back to the user. Thanks to > pass-by-reference, the object they passed into the method will get > manipulated by Hibernate, JPA, etc. What you describe in this last sentence is exactly what is not happening with JPA, and the JPA spec defines no way to make it happen. Basically what we'd be doing is to tweak the api a touch (returning something instead of void, and my contention is that there's a low impact way to get this to work for all 3 persistence frameworks) in order to use JPA as it was intended (aka... without the DaoUtils class... I would be able to remove it entirely). I'm gonna make the changes, and create a patch whether we decide to go with or not, so anyone who wants can see exactly what is being modified. --Bryan > > Matt > > On 3/13/07, Bryan Noll <[hidden email]> wrote: >> >> One more tiny wrench in this issue... >> >> Session.saveOrUpdate (what is being employed in >> UniversalDaoHibernate.save) >> returns void. >> >> EntityManager.merge (what is being employed in UniversalDaoJpa.save) >> returns an object. >> >> Session.merge (an option that we're currently not using in any >> *DaoHibernate implementations, returns an object as well). >> >> >> This means we'd have to switch from Session.saveOrUpdate to >> Session.merge, >> which doesn't bother me. >> >> Looking at the iBatis implementation UniversalDaoiBatis.save, it >> seems like >> simply returning the 'object' param instead of void should do the trick >> here. I'll try all this stuff to make sure it works how I think it >> ought to >> and make sure the tests are passing. >> >> Assuming they do, is everyone alright with me making these changes? >> Please >> let me know if you see some ramifications that I don't. >> >> Thanks... >> >> Bryan >> >> >> Bryan Noll wrote: >> Allright... ignore all my long winded crap, I re-read the stuff enough >> times that I got it. In order to get this to work, we'd have to >> change the >> UniversalDao (and UniversalDaoJpa of course) to return the result of the >> entityManager.save method. (The same obviously applies to UserDao and >> UserDaoJpa.) >> >> EG... >> >> UniversalDao.save should become: >> >> public Object save(Object o); >> >> >> UniversalDaoJpa.save should become: >> >> public Object save(Object o) { >> return this.entityManager.merge(o); >> } >> >> Line 41 of UniversalDaoTest should change from: >> >> universalDao.save(user); >> >> ... to: >> >> user = (User)universalDao.save(user); >> >> >> That makes the test past. >> >> >> AppFuse folks... what do you all think about making this >> modification? I >> say we should do it, but this also means that to stay consistent we'd >> have >> to modify the Dao interface and implementation of the 'save' method >> in the >> hibernate module and the iBatis module as well. I don't see that >> this would >> cause a problem. Let me know what you think. >> >> Bryan Noll wrote: >> Furthermore... >> >> According to this >> (http://www.hibernate.org/hib_docs/entitymanager/reference/en/html_single/#d0e897)... >> >> >> Merging vs. saveOrUpdate/saveOrUpdateCopy >> >> Merging in EJB3 is similar to the saveOrUpdateCopy() method in native >> Hibernate. However, it is not the same as the saveOrUpdate() method, the >> given instance is not reattached with the persistence context, but a >> managed >> instance is returned by the merge() method. >> >> So, let's go look at what saveOrUpdateCopy says: >> >> org.hibernate.Session doesn't have that method, but >> org.hibernate.classic.Session does >> (http://www.hibernate.org/hib_docs/v3/api/org/hibernate/classic/Session.html)... >> >> >> All it tells us is that it's been deprecated and that we should go >> look at >> Session.merge, which takes us back to org.hibernate.Session >> >> >> http://www.hibernate.org/hib_docs/v3/api/org/hibernate/Session.html#merge(java.lang.Object) >> >> >> ... which tells us this: >> >> Copy the state of the given object onto the persistent object with >> the same >> identifier. If there is no persistent instance currently associated >> with the >> session, it will be loaded. Return the persistent instance. If the given >> instance is unsaved, save a copy of and return it as a newly persistent >> instance. The given instance does not become associated with the >> session. >> This operation cascades to associated instances if the association is >> mapped >> with cascade="merge". >> >> The semantics of this method are defined by JSR-220. >> >> >> Evidently, the important part here must be "The given instance does not >> become associated with the session." At any rate, that definition is >> not >> the same as the one for Session.saveOrUpdate, as in: >> >> http://www.hibernate.org/hib_docs/v3/api/org/hibernate/Session.html#saveOrUpdate(java.lang.Object) >> >> >> Either save() or update() the given instance, depending upon the >> value of >> its identifier property. By default the instance is always saved. This >> behaviour may be adjusted by specifying an unsaved-value attribute of >> the >> identifier property mapping. This operation cascades to associated >> instances >> if the association is mapped with cascade="save-update". >> >> >> >> Just to make this message already longer than it is, here is the >> javadoc >> description of the deprecated saveOrUpdateCopy method. >> >> Copy the state of the given object onto the persistent object with >> the same >> identifier. If there is no persistent instance currently associated >> with the >> session, it will be loaded. Return the persistent instance. If the given >> instance is unsaved or does not exist in the database, save it and >> return it >> as a newly persistent instance. Otherwise, the given instance does not >> become associated with the session. >> >> >> My brain hurts a little more every time I re-read one of these javadoc >> comments. >> >> >> Bryan Noll wrote: >> For anyone wanting a cross-reference to the other thread too... >> >> http://forum.springframework.org/showthread.php?t=36064 >> >> Bryan Noll wrote: >> >> I'm reading and responding to that thread too. If you can do >> semantically >> what the 'save' method is doing (insert if new, update if modified) >> without >> the use of DaoUtils and what it does, I'm all for it. >> >> I simply could not get it to work that way when I was implementing that >> code, regardless of what the documentation says. >> For example, a quick modification to do what you're talking about is >> to : >> >> Change GenericDaoJpa.save from: >> >> public void save(T object) { >> Object objId = DaoUtils.getPersistentId(object); >> if (objId == null) { >> this.entityManager.persist(object); >> } else { >> this.entityManager.merge(object); >> } >> } >> >> ... to: >> >> public void save(T object) { >> this.entityManager.merge(object); >> } >> >> When I do that (and do the same thing for UniversalDaoJpa.save), and >> then >> run mvn test the jpa-hibernate module, I get the following: >> >> UniversalDaoTest failure: http://rafb.net/p/epCRgU65.html >> UserDaoTest failure: http://rafb.net/p/Wb4dR394.html >> >> If you look at these tests, you'll see that in both cases, it's doing >> this... >> >> assertNotNull(user.getId()); >> >> ... aka... making the assumption that the merge method knows how to >> insert >> a new object (one without an id yet) into the database. >> >> So, this either doesn't work exactly how the documentation seems to >> indicate it should, or I've done something wrong in the test (maybe >> in terms >> of extending org.springframework.test.jpa.AbstractJpaTests >> and the way it deals with transactions). I would like to change that >> method >> to simply call entityManager.merge myself, but don't see how unless >> there is >> something wrong with how the tests are working. >> I tried pretty hard to make it work like you want when I wrote, and >> it just >> didn't for me. Hopefully someone can shed some light on it so we can >> get >> rid of a bunch of this code. >> >> --Bryan >> >> >> >> >> >> dxxvi wrote: >> >> Hi Matt, >> >> I look at the DaoUtils class and ... to my understanding, this class's >> methods are used to get the object id of an object by searching for >> methods >> with @Id or @EmbeddedId annotations. Then if the objId is not null, it >> thinks that this object was persisted already and then calls the merge >> method of the entityManager. I think it's not right in the case of >> composite >> ids. Because with composite id, the id is not null although that >> object is >> new. >> >> Anyway, do we need to know when to use persist and when to use merge? >> karldmoore from Spring forum told me about this and it seems to me >> that we >> can use merge all the time. I haven't tried it yet because I'm still >> having >> trouble with persisting an object with a composite id. >> >> Anybody has an example of using composite id with JPA? >> >> Matt, I appreciate your AppFuse. It helps me alot. Thank you. >> >> PS: I'm talking about JPA in AppFuse 2.0M3. >> >> >> > > --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
||||||||||||||||
|
mraible
|
Fricken JPA... figures it'd cause issues like this. Oh well, changing
the signatures might be necessary. One thing we might want to do is do an M4 release before you commit this. We don't want to break anyone's applications that are depending on the snapshot. Matt On 3/13/07, Bryan Noll <[hidden email]> wrote: > inline... > > Matt Raible wrote: > > I believe we can leave these methods as void because even if an object > > is returned, we don't need to pass it back to the user. Thanks to > > pass-by-reference, the object they passed into the method will get > > manipulated by Hibernate, JPA, etc. > > What you describe in this last sentence is exactly what is not happening > with JPA, and the JPA spec defines no way to make it happen. Basically > what we'd be doing is to tweak the api a touch (returning something > instead of void, and my contention is that there's a low impact way to > get this to work for all 3 persistence frameworks) in order to use JPA > as it was intended (aka... without the DaoUtils class... I would be able > to remove it entirely). > > > I'm gonna make the changes, and create a patch whether we decide to go > with or not, so anyone who wants can see exactly what is being modified. > > --Bryan > > > > Matt > > > > On 3/13/07, Bryan Noll <[hidden email]> wrote: > >> > >> One more tiny wrench in this issue... > >> > >> Session.saveOrUpdate (what is being employed in > >> UniversalDaoHibernate.save) > >> returns void. > >> > >> EntityManager.merge (what is being employed in UniversalDaoJpa.save) > >> returns an object. > >> > >> Session.merge (an option that we're currently not using in any > >> *DaoHibernate implementations, returns an object as well). > >> > >> > >> This means we'd have to switch from Session.saveOrUpdate to > >> Session.merge, > >> which doesn't bother me. > >> > >> Looking at the iBatis implementation UniversalDaoiBatis.save, it > >> seems like > >> simply returning the 'object' param instead of void should do the trick > >> here. I'll try all this stuff to make sure it works how I think it > >> ought to > >> and make sure the tests are passing. > >> > >> Assuming they do, is everyone alright with me making these changes? > >> Please > >> let me know if you see some ramifications that I don't. > >> > >> Thanks... > >> > >> Bryan > >> > >> > >> Bryan Noll wrote: > >> Allright... ignore all my long winded crap, I re-read the stuff enough > >> times that I got it. In order to get this to work, we'd have to > >> change the > >> UniversalDao (and UniversalDaoJpa of course) to return the result of the > >> entityManager.save method. (The same obviously applies to UserDao and > >> UserDaoJpa.) > >> > >> EG... > >> > >> UniversalDao.save should become: > >> > >> public Object save(Object o); > >> > >> > >> UniversalDaoJpa.save should become: > >> > >> public Object save(Object o) { > >> return this.entityManager.merge(o); > >> } > >> > >> Line 41 of UniversalDaoTest should change from: > >> > >> universalDao.save(user); > >> > >> ... to: > >> > >> user = (User)universalDao.save(user); > >> > >> > >> That makes the test past. > >> > >> > >> AppFuse folks... what do you all think about making this > >> modification? I > >> say we should do it, but this also means that to stay consistent we'd > >> have > >> to modify the Dao interface and implementation of the 'save' method > >> in the > >> hibernate module and the iBatis module as well. I don't see that > >> this would > >> cause a problem. Let me know what you think. > >> > >> Bryan Noll wrote: > >> Furthermore... > >> > >> According to this > >> (http://www.hibernate.org/hib_docs/entitymanager/reference/en/html_single/#d0e897)... > >> > >> > >> Merging vs. saveOrUpdate/saveOrUpdateCopy > >> > >> Merging in EJB3 is similar to the saveOrUpdateCopy() method in native > >> Hibernate. However, it is not the same as the saveOrUpdate() method, the > >> given instance is not reattached with the persistence context, but a > >> managed > >> instance is returned by the merge() method. > >> > >> So, let's go look at what saveOrUpdateCopy says: > >> > >> org.hibernate.Session doesn't have that method, but > >> org.hibernate.classic.Session does > >> (http://www.hibernate.org/hib_docs/v3/api/org/hibernate/classic/Session.html)... > >> > >> > >> All it tells us is that it's been deprecated and that we should go > >> look at > >> Session.merge, which takes us back to org.hibernate.Session > >> > >> > >> http://www.hibernate.org/hib_docs/v3/api/org/hibernate/Session.html#merge(java.lang.Object) > >> > >> > >> ... which tells us this: > >> > >> Copy the state of the given object onto the persistent object with > >> the same > >> identifier. If there is no persistent instance currently associated > >> with the > >> session, it will be loaded. Return the persistent instance. If the given > >> instance is unsaved, save a copy of and return it as a newly persistent > >> instance. The given instance does not become associated with the > >> session. > >> This operation cascades to associated instances if the association is > >> mapped > >> with cascade="merge". > >> > >> The semantics of this method are defined by JSR-220. > >> > >> > >> Evidently, the important part here must be "The given instance does not > >> become associated with the session." At any rate, that definition is > >> not > >> the same as the one for Session.saveOrUpdate, as in: > >> > >> http://www.hibernate.org/hib_docs/v3/api/org/hibernate/Session.html#saveOrUpdate(java.lang.Object) > >> > >> > >> Either save() or update() the given instance, depending upon the > >> value of > >> its identifier property. By default the instance is always saved. This > >> behaviour may be adjusted by specifying an unsaved-value attribute of > >> the > >> identifier property mapping. This operation cascades to associated > >> instances > >> if the association is mapped with cascade="save-update". > >> > >> > >> > >> Just to make this message already longer than it is, here is the > >> javadoc > >> description of the deprecated saveOrUpdateCopy method. > >> > >> Copy the state of the given object onto the persistent object with > >> the same > >> identifier. If there is no persistent instance currently associated > >> with the > >> session, it will be loaded. Return the persistent instance. If the given > >> instance is unsaved or does not exist in the database, save it and > >> return it > >> as a newly persistent instance. Otherwise, the given instance does not > >> become associated with the session. > >> > >> > >> My brain hurts a little more every time I re-read one of these javadoc > >> comments. > >> > >> > >> Bryan Noll wrote: > >> For anyone wanting a cross-reference to the other thread too... > >> > >> http://forum.springframework.org/showthread.php?t=36064 > >> > >> Bryan Noll wrote: > >> > >> I'm reading and responding to that thread too. If you can do > >> semantically > >> what the 'save' method is doing (insert if new, update if modified) > >> without > >> the use of DaoUtils and what it does, I'm all for it. > >> > >> I simply could not get it to work that way when I was implementing that > >> code, regardless of what the documentation says. > >> For example, a quick modification to do what you're talking about is > >> to : > >> > >> Change GenericDaoJpa.save from: > >> > >> public void save(T object) { > >> Object objId = DaoUtils.getPersistentId(object); > >> if (objId == null) { > >> this.entityManager.persist(object); > >> } else { > >> this.entityManager.merge(object); > >> } > >> } > >> > >> ... to: > >> > >> public void save(T object) { > >> this.entityManager.merge(object); > >> } > >> > >> When I do that (and do the same thing for UniversalDaoJpa.save), and > >> then > >> run mvn test the jpa-hibernate module, I get the following: > >> > >> UniversalDaoTest failure: http://rafb.net/p/epCRgU65.html > >> UserDaoTest failure: http://rafb.net/p/Wb4dR394.html > >> > >> If you look at these tests, you'll see that in both cases, it's doing > >> this... > >> > >> assertNotNull(user.getId()); > >> > >> ... aka... making the assumption that the merge method knows how to > >> insert > >> a new object (one without an id yet) into the database. > >> > >> So, this either doesn't work exactly how the documentation seems to > >> indicate it should, or I've done something wrong in the test (maybe > >> in terms > >> of extending org.springframework.test.jpa.AbstractJpaTests > >> and the way it deals with transactions). I would like to change that > >> method > >> to simply call entityManager.merge myself, but don't see how unless > >> there is > >> something wrong with how the tests are working. > >> I tried pretty hard to make it work like you want when I wrote, and > >> it just > >> didn't for me. Hopefully someone can shed some light on it so we can > >> get > >> rid of a bunch of this code. > >> > >> --Bryan > >> > >> > >> > >> > >> > >> dxxvi wrote: > >> > >> Hi Matt, > >> > >> I look at the DaoUtils class and ... to my understanding, this class's > >> methods are used to get the object id of an object by searching for > >> methods > >> with @Id or @EmbeddedId annotations. Then if the objId is not null, it > >> thinks that this object was persisted already and then calls the merge > >> method of the entityManager. I think it's not right in the case of > >> composite > >> ids. Because with composite id, the id is not null although that > >> object is > >> new. > >> > >> Anyway, do we need to know when to use persist and when to use merge? > >> karldmoore from Spring forum told me about this and it seems to me > >> that we > >> can use merge all the time. I haven't tried it yet because I'm still > >> having > >> trouble with persisting an object with a composite id. > >> > >> Anybody has an example of using composite id with JPA? > >> > >> Matt, I appreciate your AppFuse. It helps me alot. Thank you. > >> > >> PS: I'm talking about JPA in AppFuse 2.0M3. > >> > >> > >> > > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [hidden email] > For additional commands, e-mail: [hidden email] > > -- http://raibledesigns.com --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
||||||||||||||||
|
Bryan Noll
|
OK... I don't have a problem with that. Just as long as it makes it
into the 2.0 final. I always thought all that crap I had to do in DaoUtils felt like a hack. It'll be good to get confusing, unnecessary, reflection-intensive code out of there. Looking at that class would be enough to make me want to use .NET if I was a newcomer. --Bryan Matt Raible wrote: > Fricken JPA... figures it'd cause issues like this. Oh well, changing > the signatures might be necessary. One thing we might want to do is > do an M4 release before you commit this. We don't want to break > anyone's applications that are depending on the snapshot. > > Matt > > On 3/13/07, Bryan Noll <[hidden email]> wrote: >> inline... >> >> Matt Raible wrote: >> > I believe we can leave these methods as void because even if an object >> > is returned, we don't need to pass it back to the user. Thanks to >> > pass-by-reference, the object they passed into the method will get >> > manipulated by Hibernate, JPA, etc. >> >> What you describe in this last sentence is exactly what is not happening >> with JPA, and the JPA spec defines no way to make it happen. Basically >> what we'd be doing is to tweak the api a touch (returning something >> instead of void, and my contention is that there's a low impact way to >> get this to work for all 3 persistence frameworks) in order to use JPA >> as it was intended (aka... without the DaoUtils class... I would be able >> to remove it entirely). >> >> >> I'm gonna make the changes, and create a patch whether we decide to go >> with or not, so anyone who wants can see exactly what is being modified. >> >> --Bryan >> > >> > Matt >> > >> > On 3/13/07, Bryan Noll <[hidden email]> wrote: >> >> >> >> One more tiny wrench in this issue... >> >> >> >> Session.saveOrUpdate (what is being employed in >> >> UniversalDaoHibernate.save) >> >> returns void. >> >> >> >> EntityManager.merge (what is being employed in UniversalDaoJpa.save) >> >> returns an object. >> >> >> >> Session.merge (an option that we're currently not using in any >> >> *DaoHibernate implementations, returns an object as well). >> >> >> >> >> >> This means we'd have to switch from Session.saveOrUpdate to >> >> Session.merge, >> >> which doesn't bother me. >> >> >> >> Looking at the iBatis implementation UniversalDaoiBatis.save, it >> >> seems like >> >> simply returning the 'object' param instead of void should do the >> trick >> >> here. I'll try all this stuff to make sure it works how I think it >> >> ought to >> >> and make sure the tests are passing. >> >> >> >> Assuming they do, is everyone alright with me making these changes? >> >> Please >> >> let me know if you see some ramifications that I don't. >> >> >> >> Thanks... >> >> >> >> Bryan >> >> >> >> >> >> Bryan Noll wrote: >> >> Allright... ignore all my long winded crap, I re-read the stuff >> enough >> >> times that I got it. In order to get this to work, we'd have to >> >> change the >> >> UniversalDao (and UniversalDaoJpa of course) to return the result >> of the >> >> entityManager.save method. (The same obviously applies to UserDao >> and >> >> UserDaoJpa.) >> >> >> >> EG... >> >> >> >> UniversalDao.save should become: >> >> >> >> public Object save(Object o); >> >> >> >> >> >> UniversalDaoJpa.save should become: >> >> >> >> public Object save(Object o) { >> >> return this.entityManager.merge(o); >> >> } >> >> >> >> Line 41 of UniversalDaoTest should change from: >> >> >> >> universalDao.save(user); >> >> >> >> ... to: >> >> >> >> user = (User)universalDao.save(user); >> >> >> >> >> >> That makes the test past. >> >> >> >> >> >> AppFuse folks... what do you all think about making this >> >> modification? I >> >> say we should do it, but this also means that to stay consistent we'd >> >> have >> >> to modify the Dao interface and implementation of the 'save' method >> >> in the >> >> hibernate module and the iBatis module as well. I don't see that >> >> this would >> >> cause a problem. Let me know what you think. >> >> >> >> Bryan Noll wrote: >> >> Furthermore... >> >> >> >> According to this >> >> >> (http://www.hibernate.org/hib_docs/entitymanager/reference/en/html_single/#d0e897)... >> >> >> >> >> >> >> Merging vs. saveOrUpdate/saveOrUpdateCopy >> >> >> >> Merging in EJB3 is similar to the saveOrUpdateCopy() method in >> native >> >> Hibernate. However, it is not the same as the saveOrUpdate() >> method, the >> >> given instance is not reattached with the persistence context, but a >> >> managed >> >> instance is returned by the merge() method. >> >> >> >> So, let's go look at what saveOrUpdateCopy says: >> >> >> >> org.hibernate.Session doesn't have that method, but >> >> org.hibernate.classic.Session does >> >> >> (http://www.hibernate.org/hib_docs/v3/api/org/hibernate/classic/Session.html)... >> >> >> >> >> >> >> All it tells us is that it's been deprecated and that we should go >> >> look at >> >> Session.merge, which takes us back to org.hibernate.Session >> >> >> >> >> >> >> http://www.hibernate.org/hib_docs/v3/api/org/hibernate/Session.html#merge(java.lang.Object) >> >> >> >> >> >> >> ... which tells us this: >> >> >> >> Copy the state of the given object onto the persistent object with >> >> the same >> >> identifier. If there is no persistent instance currently associated >> >> with the >> >> session, it will be loaded. Return the persistent instance. If the >> given >> >> instance is unsaved, save a copy of and return it as a newly >> persistent >> >> instance. The given instance does not become associated with the >> >> session. >> >> This operation cascades to associated instances if the association is >> >> mapped >> >> with cascade="merge". >> >> >> >> The semantics of this method are defined by JSR-220. >> >> >> >> >> >> Evidently, the important part here must be "The given instance >> does not >> >> become associated with the session." At any rate, that definition is >> >> not >> >> the same as the one for Session.saveOrUpdate, as in: >> >> >> >> >> http://www.hibernate.org/hib_docs/v3/api/org/hibernate/Session.html#saveOrUpdate(java.lang.Object) >> >> >> >> >> >> >> Either save() or update() the given instance, depending upon the >> >> value of >> >> its identifier property. By default the instance is always saved. >> This >> >> behaviour may be adjusted by specifying an unsaved-value attribute of >> >> the >> >> identifier property mapping. This operation cascades to associated >> >> instances >> >> if the association is mapped with cascade="save-update". >> >> >> >> >> >> >> >> Just to make this message already longer than it is, here is the >> >> javadoc >> >> description of the deprecated saveOrUpdateCopy method. >> >> >> >> Copy the state of the given object onto the persistent object with >> >> the same >> >> identifier. If there is no persistent instance currently associated >> >> with the >> >> session, it will be loaded. Return the persistent instance. If the >> given >> >> instance is unsaved or does not exist in the database, save it and >> >> return it >> >> as a newly persistent instance. Otherwise, the given instance does >> not >> >> become associated with the session. >> >> >> >> >> >> My brain hurts a little more every time I re-read one of these >> javadoc >> >> comments. >> >> >> >> >> >> Bryan Noll wrote: >> >> For anyone wanting a cross-reference to the other thread too... >> >> >> >> http://forum.springframework.org/showthread.php?t=36064 >> >> >> >> Bryan Noll wrote: >> >> >> >> I'm reading and responding to that thread too. If you can do >> >> semantically >> >> what the 'save' method is doing (insert if new, update if modified) >> >> without >> >> the use of DaoUtils and what it does, I'm all for it. >> >> >> >> I simply could not get it to work that way when I was >> implementing that >> >> code, regardless of what the documentation says. >> >> For example, a quick modification to do what you're talking about is >> >> to : >> >> >> >> Change GenericDaoJpa.save from: >> >> >> >> public void save(T object) { >> >> Object objId = DaoUtils.getPersistentId(object); >> >> if (objId == null) { >> >> this.entityManager.persist(object); >> >> } else { >> >> this.entityManager.merge(object); >> >> } >> >> } >> >> >> >> ... to: >> >> >> >> public void save(T object) { >> >> this.entityManager.merge(object); >> >> } >> >> >> >> When I do that (and do the same thing for UniversalDaoJpa.save), and >> >> then >> >> run mvn test the jpa-hibernate module, I get the following: >> >> >> >> UniversalDaoTest failure: http://rafb.net/p/epCRgU65.html >> >> UserDaoTest failure: http://rafb.net/p/Wb4dR394.html >> >> >> >> If you look at these tests, you'll see that in both cases, it's >> doing >> >> this... >> >> >> >> assertNotNull(user.getId()); >> >> >> >> ... aka... making the assumption that the merge method knows how to >> >> insert >> >> a new object (one without an id yet) into the database. >> >> >> >> So, this either doesn't work exactly how the documentation seems to >> >> indicate it should, or I've done something wrong in the test (maybe >> >> in terms >> >> of extending org.springframework.test.jpa.AbstractJpaTests >> >> and the way it deals with transactions). I would like to change that >> >> method >> >> to simply call entityManager.merge myself, but don't see how unless >> >> there is >> >> something wrong with how the tests are working. >> >> I tried pretty hard to make it work like you want when I wrote, and >> >> it just >> >> didn't for me. Hopefully someone can shed some light on it so we can >> >> get >> >> rid of a bunch of this code. >> >> >> >> --Bryan >> >> >> >> >> >> >> >> >> >> >> >> dxxvi wrote: >> >> >> >> Hi Matt, >> >> >> >> I look at the DaoUtils class and ... to my understanding, this >> class's >> >> methods are used to get the object id of an object by searching for >> >> methods >> >> with @Id or @EmbeddedId annotations. Then if the objId is not >> null, it >> >> thinks that this object was persisted already and then calls the >> merge >> >> method of the entityManager. I think it's not right in the case of >> >> composite >> >> ids. Because with composite id, the id is not null although that >> >> object is >> >> new. >> >> >> >> Anyway, do we need to know when to use persist and when to use >> merge? >> >> karldmoore from Spring forum told me about this and it seems to me >> >> that we >> >> can use merge all the time. I haven't tried it yet because I'm still >> >> having >> >> trouble with persisting an object with a composite id. >> >> >> >> Anybody has an example of using composite id with JPA? >> >> >> >> Matt, I appreciate your AppFuse. It helps me alot. Thank you. >> >> >> >> PS: I'm talking about JPA in AppFuse 2.0M3. >> >> >> >> >> >> >> > >> > >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: [hidden email] >> For additional commands, e-mail: [hidden email] >> >> > > --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
||||||||||||||||
|
Bryan Noll
|
In reply to this post
by Bryan Noll
Some javascript/style in this post has been disabled (why?)
inlineBryan Noll wrote: One more tiny wrench in this issue... This ended up not being necessary... simply returning the value of the user input param (since Hibernate deals with it in a pass-by-reference way) works. Even smaller change that I originally thought.
|
||||||||||||||||
|
Bryan Noll
|
Some javascript/style in this post has been disabled (why?)
Here's the patch (for mods beneath the data directory only). Please
check the JIRA comments out for a succinct summary of the ripple effect
this will have.http://issues.appfuse.org/browse/APF-695 Thanks... Bryan Bryan Noll wrote: inline |
||||||||||||||||
|
Bryan Noll
|
Some javascript/style in this post has been disabled (why?)
So... I'm going to go ahead and continue what may be a record for the longest mailing list thread with replies from the same person ever.It dawned on me tonight on the drive home that instead of changing the bajillion files it looks like we'd have to change with the way I proposed in the JIRA, why don't I just make the save method in the Jpa Dao's work the way we want them to, like this: public void save(T object) { object = this.entityManager.merge(object); } I haven't tried it yet (my laptop with the code on it is in my truck, and I'm done with computers for the night as soon as I send this), but will in the morning. If it doesn't have any adverse side affects, I'm gonna go this route. And since this route only involves changing a couple files instead of 14,000 of them (and no semantic or api modifications), I don't think we'll need to wait until after the M4 release has occurred. On 3/13/07, Bryan Noll <[hidden email]> wrote:
|
||||||||||||||||
|
Bryan Noll
|
Some javascript/style in this post has been disabled (why?)
So, I'm smoking crack. Started thinking in C++ terms for some reason.
This doesn't work... so we'll have to make the verbose modifications as
per the JIRA. I agree with Matt that it should wait until after M4.(http://javadude.com/articles/passbyvalue.htm) Bryan Noll wrote: So... I'm going to go ahead and continue what may be a record for the longest mailing list thread with replies from the same person ever. |
||||||||||||||||
| Free Embeddable Forum Powered by Nabble | Help |
