Hi Wonseok, TopLink team,
I took a look at Wonseok's fix and it looks fine to me. I have the
same questions to the TopLink team as asked in the original mail. A
few remarks to the fix:
1) Wonseok's original email had a zip attachment. AFAIK, the TopLink
team can't receive emails with .zip attachment. Please always use
.jar archives. I'm attaching the content of the .zip file as a jar.
2) In MergeManager.java:
a) Consider removing the unused variable 'changeTracked'
b) Consider adding a new method
ObjectBuilder.mergeIntoObjectCascade(...),
which basically implements the logic you've added to MergeManager in
lines
409-418. MergeManager would simply call this method, as in:
...
if(registeredObject == rmiClone){
// GF#1139 Cascade merge operations even if it's
already managed
builder.mergeIntoObjectCascade(registeredObject, false,
this);
} else {
// Merge into the clone from the original, use clone as
backup as backup as anything different should be merged.
builder.mergeIntoObject(registeredObject, false,
rmiClone, this);
}
...
Note, that the new method has only a target argument, since
registeredObject == rmiClone
3) Please migrate CascadeTest to use the JUnit framework provided in
entity-persistence-tests.
4) The test class EntityA has an OneToOne and OneToMany
relationship. Please consider adding a ManyToMany relationship as
well.
5) CascadeTest is setting the OneToOne relationship only on the
owning side. Please also set the relationship on the non-owning
side.
6) You might want to make sure that the objects associated to ma1 are
clones of the objects originally added to a1.
7) Please consider renaming the field EntityA.c to EntityA.cs, as it
is a collection field.
Thanks,
-- markus.
Wonseok Kim wrote:
Hi, Tom
I'm investigating the issue 1139 "Cascade doesn't work when merging
managed entity"
https://glassfish.dev.java.net/issues/show_bug.cgi?id=1139
I don't know that there is anyone working on this. Now I'm trying to
fix this.
MergeManager.mergeChangesOfCloneIntoWorkingCopy(Object rmiClone) just
returns if the clone is already registred one.
So I modified it to traverse ForeignReferenceMapping mappings and
trigger mapping.mergeIntoObject() if cascade merge is required. See
below diffs.
Some questions:
* Is calling mapping.mergeIntoObject() with same source and target okay?
* ObjectBuilder.mergeIntoObject() also triggers PostMergeEvent. Is this
required too for this type of cascade merge? (JPA doesn't require this
event, but TopLink?)
I tested the fix with an attached testcase and it worked well. Also
there was no entity-persistence-tests failure.
Please review and instruct me.
Index:
src/java/oracle/toplink/essentials/internal/sessions/MergeManager.java
===================================================================
RCS file:
/cvs/glassfish/entity-persistence/src/java/oracle/toplink/essentials/internal/sessions/MergeManager.java,v
retrieving revision
1.6
diff -c -r1.6
MergeManager.java
***
src/java/oracle/toplink/essentials/internal/sessions/MergeManager.java
20 Apr 2006 20:32:05 -0000 1.6
---
src/java/oracle/toplink/essentials/internal/sessions/MergeManager.java
21 Sep 2006 16:01:58 -0000
***************
*** 387,402 ****
ClassDescriptor descriptor = getSession().getDescriptor(rmiClone);
Object
registeredObject =
registerObjectForMergeCloneIntoWorkingCopy(rmiClone);
- if
(registeredObject == rmiClone) {
-
//need to find better better fix. prevents merging into itself.
-
return rmiClone;
- }
-
boolean
changeTracked = false;
try {
ObjectBuilder builder = descriptor.getObjectBuilder
();
! if
(descriptor.usesVersionLocking()) {
VersionLockingPolicy policy = (VersionLockingPolicy)
descriptor.getOptimisticLockingPolicy();
if (policy.isStoredInObject()) {
Object currentValue =
builder.extractValueFromObjectForField(registeredObject,
policy.getWriteLockField(), session);
--- 387,397 ----
ClassDescriptor descriptor = getSession().getDescriptor(rmiClone);
Object
registeredObject = registerObjectForMergeCloneIntoWorkingCopy(rmiClone);
boolean
changeTracked = false;
try {
ObjectBuilder builder = descriptor.getObjectBuilder
();
! if
(registeredObject != rmiClone &&
descriptor.usesVersionLocking()) {
VersionLockingPolicy policy = (VersionLockingPolicy)
descriptor.getOptimisticLockingPolicy();
if (policy.isStoredInObject()) {
Object currentValue =
builder.extractValueFromObjectForField(registeredObject,
policy.getWriteLockField(), session);
***************
*** 410,417 ****
//
Toggle change tracking during the merge.
descriptor.getObjectChangePolicy().dissableEventProcessing(registeredObject);
! //
Merge into the clone from the original, use clone as backup as anything
different should be merged.
!
builder.mergeIntoObject(registeredObject, false, rmiClone, this);
} finally
{
descriptor.getObjectChangePolicy().enableEventProcessing(registeredObject);
}
--- 405,425 ----
//
Toggle change tracking during the merge.
descriptor.getObjectChangePolicy().dissableEventProcessing(registeredObject);
!
if(registeredObject == rmiClone){
!
// GF#1139 Cascade merge operations even if it's already managed
!
Vector mappings = descriptor.getMappings();
!
for (int index = 0; index < mappings.size(); index++) {
!
DatabaseMapping mapping = (DatabaseMapping)mappings.get(index);
!
if(mapping instanceof ForeignReferenceMapping
!
&&
((ForeignReferenceMapping)mapping).shouldMergeCascadeParts(this)){
!
mapping.mergeIntoObject(registeredObject, false, rmiClone, this);
!
}
! }
!
! }
else {
!
// Merge into the clone from the original, use clone as backup as
anything different should be merged.
!
builder.mergeIntoObject(registeredObject, false, rmiClone, this);
! }
} finally
{
descriptor.getObjectChangePolicy
().enableEventProcessing(registeredObject);
}
Thanks
--
Wonseok Kim
Senior Developer, TmaxSoft