Attachment 224742 Details for Bug 395939 – preliminary code that addresses the issue - requires rework

[patch] preliminary code that addresses the issue - requires rework

0001-Bug-395939-Detached-object-changes-are-merged-and-th.patch (text/plain), 20.03 KB, created by Tom Ware

on 2012-12-14 10:28:47 EST

(hide)

Description:

Filename:

MIME Type:

Creator: Tom Ware

Created: 2012-12-14 10:28:47 EST

Size: 20.03 KB

patch

obsolete

>From ce6954ee1ae15fb223c4c292a8b59aea9788226a Mon Sep 17 00:00:00 2001
>From: tware <tom.ware@oracle.com>
>Date: Fri, 14 Dec 2012 10:27:46 -0500
>Subject: [PATCH] Bug 395939 - Detached object changes are merged and then
> overridden from cache - preliminary fix
>
>---
> .../internal/indirection/IndirectionPolicy.java    |   10 +++
> .../indirection/TransparentIndirectionPolicy.java  |   22 +++++++
> .../indirection/UnitOfWorkValueHolder.java         |   30 +++++++++
> .../persistence/mappings/CollectionMapping.java    |   41 ++++++++-----
> .../testing/models/jpa/relationships/Order.java    |    2 +-
> .../jpa/advanced/EntityManagerJUnitTestSuite.java  |   65 ++++++++++----------
> .../RelationshipModelJUnitTestSuite.java           |   35 +++++++++++
> 7 files changed, 156 insertions(+), 49 deletions(-)
>
>diff --git a/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/indirection/IndirectionPolicy.java b/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/indirection/IndirectionPolicy.java
>index 25f2786..049ded4 100644
>--- a/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/indirection/IndirectionPolicy.java
>+++ b/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/indirection/IndirectionPolicy.java
>@@ -248,6 +248,10 @@ public abstract class IndirectionPolicy implements Cloneable, Serializable {
>         return true;
>     }
>     
>+    public boolean isAllowRefreshOfUnitOfWorkValueHolder(Object object){
>+        return true;
>+    }
>+    
>     /**
>      * INTERNAL:
>      * Initialize the indirection policy (Do nothing by default)
>@@ -352,6 +356,11 @@ public abstract class IndirectionPolicy implements Cloneable, Serializable {
>         setRealAttributeValueInObject(target, attributeValue);
>     }
>     
>+    
>+    public void setAllowRefreshOfUnitOfWorkValueHolder(Object object, boolean refreshRequired){
>+    }
>+    
>+    
>     /**
>      * INTERNAL:
>      * set the source object into QueryBasedValueHolder.
>@@ -389,6 +398,7 @@ public abstract class IndirectionPolicy implements Cloneable, Serializable {
>     public Boolean shouldUseLazyInstantiation() {
>         return false;
>     }
>+
>     /**
>      * Reset the wrapper used to store the value.
>      * This is only required if a wrapper is used.
>diff --git a/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/indirection/TransparentIndirectionPolicy.java b/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/indirection/TransparentIndirectionPolicy.java
>index 299bc70..715400f 100644
>--- a/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/indirection/TransparentIndirectionPolicy.java
>+++ b/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/indirection/TransparentIndirectionPolicy.java
>@@ -400,6 +400,16 @@ public class TransparentIndirectionPolicy extends IndirectionPolicy {
>         return attributeValue != null;
>     }
> 
>+    public boolean isAllowRefreshOfUnitOfWorkValueHolder(Object object){
>+        if (object instanceof IndirectContainer) {
>+            ValueHolderInterface valueHolder = ((IndirectContainer)object).getValueHolder();
>+            if (valueHolder instanceof UnitOfWorkValueHolder) {
>+                return ((UnitOfWorkValueHolder)valueHolder).isAllowRefresh();
>+            }
>+        }
>+        return true;
>+    }
>+    
>     public Boolean getUseLazyInstantiation() {
>         return useLazyInstantiation;
>     }
>@@ -440,6 +450,17 @@ public class TransparentIndirectionPolicy extends IndirectionPolicy {
>         super.setRealAttributeValueInObject(target, attributeValue);
>     }
>     
>+    @Override
>+    public void setAllowRefreshOfUnitOfWorkValueHolder(Object object, boolean refreshRequired){
>+        if (object instanceof IndirectContainer) {
>+            ValueHolderInterface valueHolder = ((IndirectContainer)object).getValueHolder();
>+            if (valueHolder instanceof UnitOfWorkValueHolder) {
>+                ((UnitOfWorkValueHolder)valueHolder).setAllowRefresh(refreshRequired);
>+            }
>+        }
>+        
>+    }
>+    
>     /**
>      * INTERNAL:
>      * set the source object into QueryBasedValueHolder.
>@@ -552,6 +573,7 @@ public class TransparentIndirectionPolicy extends IndirectionPolicy {
>         return objectIsInstantiated(object) || ((object instanceof IndirectCollection) && ((IndirectCollection)object).hasDeferredChanges());
>     }
> 
>+    
>     /**
>      * ADVANCED:
>      * Set the size to of container to create.  Default to using default constructor.
>diff --git a/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/indirection/UnitOfWorkValueHolder.java b/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/indirection/UnitOfWorkValueHolder.java
>index 131f7d3..348b854 100644
>--- a/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/indirection/UnitOfWorkValueHolder.java
>+++ b/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/indirection/UnitOfWorkValueHolder.java
>@@ -18,6 +18,7 @@ import org.eclipse.persistence.descriptors.ClassDescriptor;
> import org.eclipse.persistence.exceptions.ValidationException;
> import org.eclipse.persistence.indirection.*;
> import org.eclipse.persistence.mappings.*;
>+import org.eclipse.persistence.queries.ObjectLevelReadQuery;
> import org.eclipse.persistence.internal.sessions.remote.RemoteValueHolder;
> import org.eclipse.persistence.internal.sessions.AbstractSession;
> import org.eclipse.persistence.internal.sessions.UnitOfWorkImpl;
>@@ -56,6 +57,12 @@ public abstract class UnitOfWorkValueHolder extends DatabaseValueHolder implemen
>     protected transient Object relationshipSourceObject;
>     protected String sourceAttributeName;
>     protected ObjID wrappedValueHolderRemoteID;
>+    
>+    /**
>+     * Determine whether a refresh should be allowed on this valueholder during a cascade.
>+     * Refresh is allowed when in the same UnitOfWork as the initial refresh was called on
>+     */
>+    protected boolean allowRefresh = false;
> 
>     protected UnitOfWorkValueHolder(ValueHolderInterface attributeValue, Object clone, DatabaseMapping mapping, UnitOfWorkImpl unitOfWork) {
>         this.wrappedValueHolder = attributeValue;
>@@ -149,6 +156,12 @@ public abstract class UnitOfWorkValueHolder extends DatabaseValueHolder implemen
>                         }
>                         unitOfWork.log(SessionLog.FINEST, SessionLog.TRANSACTION, "instantiate_pl_relationship");
>                     }
>+               /*     if (!isAllowRefresh() && wrapped instanceof QueryBasedValueHolder){
>+                        QueryBasedValueHolder qbvh = ((QueryBasedValueHolder)wrapped);
>+                        if (qbvh.getQuery().isObjectLevelReadQuery()){
>+                            ((ObjectLevelReadQuery)qbvh.getQuery()).setShouldRefreshIdentityMapResult(false);
>+                        }
>+                    }*/
>                     if (unitOfWork.getCommitManager().isActive() || unitOfWork.wasTransactionBegunPrematurely()) {
>                         // At this point the wrapped valueholder is not triggered,
>                         // and we are in transaction.  So just trigger the
>@@ -162,6 +175,15 @@ public abstract class UnitOfWorkValueHolder extends DatabaseValueHolder implemen
>     }
> 
>     /**
>+     * Determine whether a refresh should be allowed on this valueholder during a cascade.
>+     * Refresh is allowed when in the same UnitOfWork as the initial refresh was called on
>+     */
>+    public boolean isAllowRefresh() {
>+        return allowRefresh;
>+    }
>+
>+    
>+    /**
>      * INTERNAL:
>      * Answers if this valueholder is easy to instantiate.
>      * @return true if getValue() won't trigger a database read.
>@@ -268,6 +290,14 @@ public abstract class UnitOfWorkValueHolder extends DatabaseValueHolder implemen
>         //do nothing.  nothing should be reset to null;
>     }
> 
>+    /**
>+     * Set whether a refresh should be allowed on this valueholder during a cascade.
>+     * Refresh is allowed when in the same UnitOfWork as the initial refresh was called on
>+     */
>+    public void setAllowRefresh(boolean allowRefresh) {
>+        this.allowRefresh = allowRefresh;
>+    }
>+    
>     public void setBackupValueHolder(ValueHolder backupValueHolder) {
>         this.backupValueHolder = backupValueHolder;
>     }
>diff --git a/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/mappings/CollectionMapping.java b/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/mappings/CollectionMapping.java
>index 0d77c2c..ab94559 100644
>--- a/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/mappings/CollectionMapping.java
>+++ b/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/mappings/CollectionMapping.java
>@@ -280,24 +280,29 @@ public abstract class CollectionMapping extends ForeignReferenceMapping implemen
>      */
>     public Object buildElementUnitOfWorkClone(Object element, Object parent, Integer refreshCascade, UnitOfWorkImpl unitOfWork, boolean isExisting, boolean isFromSharedCache) {
>         // optimize registration to knowledge of existence
>-        if (refreshCascade != null ){
>-            switch(refreshCascade){
>-            case ObjectBuildingQuery.CascadeAllParts : 
>-                return unitOfWork.mergeClone(element, MergeManager.CASCADE_ALL_PARTS, true);
>-            case ObjectBuildingQuery.CascadePrivateParts :
>-                return unitOfWork.mergeClone(element, MergeManager.CASCADE_PRIVATE_PARTS, true);
>-            case ObjectBuildingQuery.CascadeByMapping :
>-                return unitOfWork.mergeClone(element, MergeManager.CASCADE_BY_MAPPING, true);
>-            default:
>-                return unitOfWork.mergeClone(element, MergeManager.NO_CASCADE, true);
>-            }
>-        }else{
>-            if (isExisting) {
>-                return unitOfWork.registerExistingObject(element, isFromSharedCache);
>-            } else {// not known whether existing or not
>-                return unitOfWork.registerObject(element);
>+        if (refreshCascade != null){
>+            Object attributeValue = getAttributeValueFromObject(parent);
>+            if (getIndirectionPolicy().isAllowRefreshOfUnitOfWorkValueHolder(attributeValue)){
>+                Object returnValue = null;
>+                switch(refreshCascade){
>+                case ObjectBuildingQuery.CascadeAllParts : 
>+                    returnValue = unitOfWork.mergeClone(element, MergeManager.CASCADE_ALL_PARTS, true);
>+                case ObjectBuildingQuery.CascadePrivateParts :
>+                    returnValue =  unitOfWork.mergeClone(element, MergeManager.CASCADE_PRIVATE_PARTS, true);
>+                case ObjectBuildingQuery.CascadeByMapping :
>+                    returnValue =  unitOfWork.mergeClone(element, MergeManager.CASCADE_BY_MAPPING, true);
>+                default:
>+                    returnValue =  unitOfWork.mergeClone(element, MergeManager.NO_CASCADE, true);
>+                }
>+                getIndirectionPolicy().setAllowRefreshOfUnitOfWorkValueHolder(attributeValue, false);
>+                return returnValue;
>             }
>         }
>+        if (isExisting) {
>+            return unitOfWork.registerExistingObject(element, isFromSharedCache);
>+        } else {// not known whether existing or not
>+            return unitOfWork.registerObject(element);
>+        }
>     }
> 
>     /**
>@@ -1456,6 +1461,10 @@ public abstract class CollectionMapping extends ForeignReferenceMapping implemen
>                     Object clonedAttributeValue = this.indirectionPolicy.cloneAttribute(attributeValue, source, null, target, null, mergeManager.getSession(), false); // building clone from an original not a row. 
>                     setAttributeValueInObject(target, clonedAttributeValue);
>                 }
>+                if (!isAttributeValueInstantiated(target)){
>+                    Object attributeValue = getAttributeValueFromObject(target);
>+                    getIndirectionPolicy().setAllowRefreshOfUnitOfWorkValueHolder(attributeValue, true);
>+                }
>                 
>                 // This will occur when the clone's value has not been instantiated yet and we do not need
>                 // the refresh that attribute
>diff --git a/jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/models/jpa/relationships/Order.java b/jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/models/jpa/relationships/Order.java
>index 77a636a..6687577 100644
>--- a/jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/models/jpa/relationships/Order.java
>+++ b/jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/models/jpa/relationships/Order.java
>@@ -154,7 +154,7 @@ public class Order implements java.io.Serializable {
>         this.billedCustomer = billedCustomer; 
>     }
>     
>-    @ManyToOne(fetch=LAZY)
>+    @ManyToOne(fetch=LAZY, cascade=CascadeType.ALL)
>     public SalesPerson getSalesPerson() {
>         return salesPerson;
>     }
>diff --git a/jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/tests/jpa/advanced/EntityManagerJUnitTestSuite.java b/jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/tests/jpa/advanced/EntityManagerJUnitTestSuite.java
>index d3cea16..c10bad8 100644
>--- a/jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/tests/jpa/advanced/EntityManagerJUnitTestSuite.java
>+++ b/jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/tests/jpa/advanced/EntityManagerJUnitTestSuite.java
>@@ -12259,41 +12259,42 @@ public class EntityManagerJUnitTestSuite extends JUnitTestCase {
>     
>     // Bug 384607
>     public void testReplaceElementCollection(){
>-    	EntityManager em = createEntityManager();
>-    	beginTransaction(em);
>-    	Golfer golfer = new Golfer();
>-    	GolferPK pk = new GolferPK(102);
>-    	WorldRank rank = new WorldRank();
>-    	rank.setId(102);
>-    	golfer.setWorldRank(rank);
>-    	golfer.getSponsorDollars().put("Oracle", 10000);
>-    	golfer.setGolferPK(pk);
>-    	em.persist(golfer);
>-    	em.persist(rank);
>-    	em.flush();
>-
>-    	golfer = em.find(Golfer.class, golfer.getGolferPK());
>-    	golfer.getSponsorDollars().put("Callaway", 20000);
>-    	em.flush();
>-    	
>-    	clearCache();
>-    	em.clear();
>-    	
>-    	golfer = em.find(Golfer.class, golfer.getGolferPK());
>-    	assertTrue("Incorrect Number of sponsors", golfer.getSponsorDollars().size() == 2);
>-    	assertTrue("Missing Sponsor: Callaway", golfer.getSponsorDollars().get("Callaway").equals(20000));
>-    	assertTrue("Missing Sponsor: Oracle", golfer.getSponsorDollars().get("Oracle").equals(10000));
>+        EntityManager em = createEntityManager();
>+        beginTransaction(em);
>+        Golfer golfer = new Golfer();
>+        GolferPK pk = new GolferPK(102);
>+       WorldRank rank = new WorldRank();
>+       rank.setId(102);
>+       golfer.setWorldRank(rank);
>+       golfer.getSponsorDollars().put("Oracle", 10000);
>+       golfer.setGolferPK(pk);
>+       em.persist(golfer);
>+       em.persist(rank);
>+       em.flush();
>+
>+       golfer = em.find(Golfer.class, golfer.getGolferPK());
>+       golfer.getSponsorDollars().put("Callaway", 20000);
>+       em.flush();
>+       
>+       clearCache();
>+       em.clear();
>+    
>+       golfer = em.find(Golfer.class, golfer.getGolferPK());
>+       assertTrue("Incorrect Number of sponsors", golfer.getSponsorDollars().size() == 2);
>+       assertTrue("Missing Sponsor: Callaway", golfer.getSponsorDollars().get("Callaway").equals(20000));
>+       assertTrue("Missing Sponsor: Oracle", golfer.getSponsorDollars().get("Oracle").equals(10000));
> 
>-    	golfer = em.find(Golfer.class, golfer.getGolferPK());
>-    	golfer.setSponsorDollars(new HashMap<String, Integer>());
>-    	em.flush();
>+       golfer = em.find(Golfer.class, golfer.getGolferPK());
>+       golfer.setSponsorDollars(new HashMap<String, Integer>());
>+       em.flush();
>     	
>-    	clearCache();
>-    	em.clear();
>+       clearCache();
>+       em.clear();
>     	
>-    	golfer = em.find(Golfer.class, golfer.getGolferPK());
>-    	assertTrue("Incorrect Number of sponsors", golfer.getSponsorDollars().size() == 0);
>+       golfer = em.find(Golfer.class, golfer.getGolferPK());
>+       assertTrue("Incorrect Number of sponsors", golfer.getSponsorDollars().size() == 0);
>     	
>-    	rollbackTransaction(em);
>+       rollbackTransaction(em);
>     }
>+    
> }
>diff --git a/jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/tests/jpa/relationships/RelationshipModelJUnitTestSuite.java b/jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/tests/jpa/relationships/RelationshipModelJUnitTestSuite.java
>index cb355ef..7521ae5 100644
>--- a/jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/tests/jpa/relationships/RelationshipModelJUnitTestSuite.java
>+++ b/jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/tests/jpa/relationships/RelationshipModelJUnitTestSuite.java
>@@ -15,6 +15,7 @@
>  ******************************************************************************/
> package org.eclipse.persistence.testing.tests.jpa.relationships;
> 
>+import java.util.ArrayList;
> import java.util.Iterator;
> import java.util.Collection;
> import java.util.HashMap;
>@@ -61,10 +62,12 @@ import org.eclipse.persistence.testing.models.jpa.relationships.OrderLabel;
> import org.eclipse.persistence.testing.models.jpa.relationships.RelationshipsExamples;
> import org.eclipse.persistence.testing.models.jpa.relationships.RelationshipsTableManager;
> import org.eclipse.persistence.testing.models.jpa.relationships.Order;
>+import org.eclipse.persistence.testing.models.jpa.relationships.SalesPerson;
> import org.eclipse.persistence.testing.models.jpa.relationships.ServiceCall;
> import org.eclipse.persistence.testing.models.jpa.relationships.TestInstantiationCopyPolicy;
> 
> import org.eclipse.persistence.testing.models.jpa.relationships.Customer;
>+import org.eclipse.persistence.testing.tests.jpa.advanced.EntityManagerJUnitTestSuite;
> 
> public class RelationshipModelJUnitTestSuite extends JUnitTestCase {
>     private static Integer itemId;
>@@ -82,6 +85,7 @@ public class RelationshipModelJUnitTestSuite extends JUnitTestCase {
>         suite.setName("RelationshipModelJUnitTestSuite");
>         
>         suite.addTest(new RelationshipModelJUnitTestSuite("testSetup")); 
>+        suite.addTest(new RelationshipModelJUnitTestSuite("testMergeWithRefresh"));
>         suite.addTest(new RelationshipModelJUnitTestSuite("testCreateItem")); 
>         suite.addTest(new RelationshipModelJUnitTestSuite("testModifyItem"));
>         suite.addTest(new RelationshipModelJUnitTestSuite("testVerifyItem"));
>@@ -99,6 +103,7 @@ public class RelationshipModelJUnitTestSuite extends JUnitTestCase {
>         suite.addTest(new RelationshipModelJUnitTestSuite("testNamedQueryDoesNotExistTest"));
>         suite.addTest(new RelationshipModelJUnitTestSuite("testExcludeDefaultMappings"));
>         suite.addTest(new RelationshipModelJUnitTestSuite("testChangeSetForNewObject"));
>+        suite.addTest(new RelationshipModelJUnitTestSuite("testMergeWithRefresh"));
>         
>         return suite;
>     }
>@@ -834,5 +839,35 @@ public class RelationshipModelJUnitTestSuite extends JUnitTestCase {
>             commitTransaction(em);
>         }
>     }
>+    
>+    // Bug 
>+    public void testMergeWithRefresh(){
>+        EntityManager em = createEntityManager();
>+        beginTransaction(em);
>+        SalesPerson sp = new SalesPerson();
>+        sp.setName("Kris");
>+        em.persist(sp);
>+        Order order = new Order();
>+        order.setQuantity(1);
>+        em.persist(order);
>+        sp.getOrders().add(order);
>+        order.setSalesPerson(sp);
>+        em.flush();
>+        
>+        em.refresh(sp);
>+        
>+        em.clear();
>+        
>+        order.setQuantity(2);
>+        sp.setOrders(new ArrayList<Order>());
>+        sp.getOrders().add(order);;
>+        order.setSalesPerson(sp);
>+        
>+        order = em.merge(order);
>+        
>+        assertTrue("Wrong quantity in Order.", order.getQuantity() == 2);
>+        
>+        rollbackTransaction(em);
>+    }
> 
> }
>-- 
>1.7.9.msysgit.0
>

Actions: View | Diff

Attachments on bug 395939: 224374 | 224742