Thanks Markus for reviewing the fix.
I'll try to migrate the test into entity-persistence-tests framework.
I added a cascade-merge method into ObjectBuilder as Markus commented. There
are other cascadeXXX methods in ObjectBuilder, so I named it
"cascadeMergeIntoObject".
/**
* INTERNAL:
* Cascade merge of the contents of a registered object, this merge
algorthim is dependent on the merge manager.
*/
public void cascadeMergeIntoObject(Object object, boolean
isUnInitialized, MergeManager mergeManager) {
// PERF: Avoid synchronized enumerator as is concurrency bottleneck.
Vector mappings = getDescriptor().getMappings();
for (int index = 0; index < mappings.size(); index++) {
DatabaseMapping mapping = (DatabaseMapping)mappings.get(index);
if(mapping instanceof ForeignReferenceMapping
&&
((ForeignReferenceMapping)mapping).shouldMergeCascadeParts(mergeManager)){
mapping.mergeIntoObject(object, isUnInitialized, object,
mergeManager);
}
}
}
cascade merge worked well for one-to-one, one-to-many and many-to-many
relationships.
Thanks
- Wonseok
On 9/22/06, Markus Fuchs <Markus.Fuchs_at_sun.com> wrote:
>
> 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
>
>
>