Hello Gordon, thanks for review.
Currently EntityManagerImpl is using UoW.mergeCloneWithReferences() instead
of mergeClone(). So mergeCloneWithReferences() should be overrided as
follows. right?
public Object mergeCloneWithReferences(Object rmiClone, int
cascadePolicy) {
if (rmiClone == null) {
return null;
}
ClassDescriptor descriptor = getDescriptor(rmiClone);
if ((descriptor == null) || descriptor.isAggregateDescriptor() ||
descriptor.isAggregateCollectionDescriptor()) {
if (cascadePolicy == MergeManager.CASCADE_BY_MAPPING){
throw new IllegalArgumentException(
ExceptionLocalization.buildMessage("not_an_entity", new
Object[]{rmiClone}));
}
return rmiClone;
}
//CR#2272
// logDebugMessage(rmiClone, "merge_clone_with_references");
ObjectBuilder builder = descriptor.getObjectBuilder();
Object implementation = builder.unwrapObject(rmiClone, this);
MergeManager manager = new MergeManager(this);
manager.mergeCloneIntoWorkingCopyForceCascade();
manager.setCascadePolicy(cascadePolicy);
Object mergedObject = manager.mergeChanges(implementation, null);
if (isSmartMerge()) {
return builder.wrapObject(mergedObject, this);
} else {
return mergedObject;
}
}
I'm curious, Why mergeCloneWithReferences() code is different from
mergeClone()?
It doesn't use SessionProfiler and handleException() as you see.
Also, to override this private logDebugMessage() should have protected
access.
And the force-cascade logic which was in MergeManager can go to
ObjectBuilder for avoding a duplicate like below.
public void mergeIntoObject(Object target, boolean isUnInitialized,
Object source, 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(target != source
|| (
mergeManager.shouldMergeCloneIntoWorkingCopyForceCascade()
&& mapping instanceof ForeignReferenceMapping
&&
((ForeignReferenceMapping)mapping).shouldMergeCascadeParts(mergeManager))){
// GF#1139 Cascade merge operations even if it's already
managed
mapping.mergeIntoObject(target, isUnInitialized, source,
mergeManager);
}
}
...
}
What do you think?
-Wonseok
On 9/25/06, Gordon Yorke <gordon.yorke_at_oracle.com> wrote:
>
> Hello Wonseok,
> A slight change will have to be made to your code as it is not
> backward compatible with current TopLink behaviour. Instead of altering the
> MergeManger to always cascade the merge and duplicating code already found
> within the ObjectBuilder the RepeatableWriteUnitOfWork (which is specific to
> EJB3.0) should request the cascaded merge from the MergeManager while the
> UnitOfWork continues to request the abortive merge.
>
> The following method should be added to the RepeatableWriteUnitOfWork :
> /**
> * INTERNAL:
> * Merge the attributes of the clone into the unit of work copy.
> */
> public Object mergeClone(Object rmiClone, int cascadeDepth) {
> if (rmiClone == null) {
> return null;
> }
>
> //CR#2272
> logDebugMessage(rmiClone, "merge_clone");
>
> startOperationProfile(SessionProfiler.Merge);
> ObjectBuilder builder =
> getDescriptor(rmiClone).getObjectBuilder();
> Object implementation = builder.unwrapObject(rmiClone, this);
>
> MergeManager manager = new MergeManager(this);
> manager.mergeCloneIntoWorkingCopyForceCascade();
> manager.setCascadePolicy(cascadeDepth);
>
> Object merged = null;
> try {
> merged = manager.mergeChanges(implementation, null);
> } catch (RuntimeException exception) {
> merged = handleException(exception);
> }
> endOperationProfile(SessionProfiler.Merge);
>
> return merged;
> }
> Then the MergeManager should have the following method and corresponding
> property added:
> /**
> * This can be used by the user for merging clones from RMI into the
> unit of work.
> */
> public void mergeCloneIntoWorkingCopyForceCascade() {
> setMergePolicy(CLONE_INTO_WORKING_COPY_FORCE_CASCADE);
> }
>
> Then update the MergeManager to check for this setting in two places:
> mergeClone():
> } else if (shouldMergeCloneIntoWorkingCopy() ||
> shouldMergeCloneWithReferencesIntoWorkingCopy() ||
> shouldMergeCloneIntoWorkingCopyForceCascade()) {
> mergedObject = mergeChangesOfCloneIntoWorkingCopy(object);
> and mergeChangesOfCloneIntoWorkingCopy() :
> if (registeredObject== rmiClone && !
> shouldMergeCloneIntoWorkingCopyForceCascade() ){
> return rmiClone;
> }
>
>
> These changes preserve existing TopLink behaviour while updating the
> functionality for EJB 3.0 support and the code continues to use the
> current TopLink patterns for method call sequence.
> --Gordon
>
> -----Original Message-----
> *From:* Wonseok Kim [mailto:guruwons_at_gmail.com]
> *Sent:* Sunday, September 24, 2006 10:19 PM
> *To:* persistence_at_glassfish.dev.java.net
> *Subject:* Re: Fix for issue 1139 "cascade merge"
>
> Hi, Gordon and team
>
> Please review the attached fix.
> * The fix is almost same as the first fix.
> * A new test is added to entity-persistence-tests and the test is using
> cmp3/advanced models - Project and Employee.
>
> Diffs and the added test are as follows:
>
> Index:
> src/java/oracle/toplink/essentials/testing/models/cmp3/advanced/Employee.java
> ===================================================================
> RCS file:
> /cvs/glassfish/entity-persistence-tests/src/java/oracle/toplink/essentials/testing/models/cmp3/advanced/Employee.java,v
> retrieving revision 1.10
> diff -c -r1.10 Employee.java
> ***
> src/java/oracle/toplink/essentials/testing/models/cmp3/advanced/Employee.java
> 8 Sep 2006 18:53:37 -0000 1.10
> ---
> src/java/oracle/toplink/essentials/testing/models/cmp3/advanced/Employee.java
> 23 Sep 2006 07:06:58 -0000
> ***************
> *** 205,211 ****
> this.manager = manager;
> }
>
> ! @ManyToMany(cascade=PERSIST)
> @JoinTable(
> name="CMP3_EMP_PROJ",
> // Default for the project side and specify for the employee
> side
> --- 205,211 ----
> this.manager = manager;
> }
>
> ! @ManyToMany(cascade={PERSIST, MERGE})
> @JoinTable(
> name="CMP3_EMP_PROJ",
> // Default for the project side and specify for the employee
> side
> Index:
> src/java/oracle/toplink/essentials/testing/models/cmp3/advanced/Project.java
> ===================================================================
> RCS file:
> /cvs/glassfish/entity-persistence-tests/src/java/oracle/toplink/essentials/testing/models/cmp3/advanced/Project.java,v
>
> retrieving revision 1.3
> diff -c -r1.3 Project.java
> ***
> src/java/oracle/toplink/essentials/testing/models/cmp3/advanced/Project.java
> 20 Jan 2006 15:57:15 -0000 1.3
> ---
> src/java/oracle/toplink/essentials/testing/models/cmp3/advanced/Project.java
> 23 Sep 2006 07:06:58 -0000
> ***************
> *** 111,117 ****
> this.description = description;
> }
>
> ! @OneToOne
> @JoinColumn(name="LEADER_ID")
> public Employee getTeamLeader() {
> return teamLeader;
> --- 111,117 ----
> this.description = description;
> }
>
> ! @OneToOne(cascade = {CascadeType.MERGE})
> @JoinColumn(name="LEADER_ID")
> public Employee getTeamLeader() {
> return teamLeader;
> Index:
> src/java/oracle/toplink/essentials/testing/tests/FullRegressionTestSuite.java
> ===================================================================
> RCS file:
> /cvs/glassfish/entity-persistence-tests/src/java/oracle/toplink/essentials/testing/tests/FullRegressionTestSuite.java,v
> retrieving revision 1.16
> diff -c -r1.16 FullRegressionTestSuite.java
> ***
> src/java/oracle/toplink/essentials/testing/tests/FullRegressionTestSuite.java
> 8 Sep 2006 18:53:38 -0000 1.16
> ---
> src/java/oracle/toplink/essentials/testing/tests/FullRegressionTestSuite.java
> 23 Sep 2006 07:06:59 -0000
> ***************
> *** 24,38 ****
> import junit.framework.TestSuite;
> import junit.framework.Test;
>
> ! import
> oracle.toplink.essentials.testing.tests.cmp3.advanced.NamedNativeQueryJUnitTest;
> ! import
> oracle.toplink.essentials.testing.tests.cmp3.advanced.CallbackEventJUnitTestSuite
> ;
> ! import
> oracle.toplink.essentials.testing.tests.cmp3.advanced.EntityManagerJUnitTestSuite
> ;
> ! import
> oracle.toplink.essentials.testing.tests.cmp3.advanced.SQLResultSetMappingTestSuite
> ;
> ! import
> oracle.toplink.essentials.testing.tests.cmp3.advanced.JoinedAttributeAdvancedJunitTest;
> ! import
> oracle.toplink.essentials.testing.tests.cmp3.advanced.ReportQueryMultipleReturnTestSuite
> ;
> ! import
> oracle.toplink.essentials.testing.tests.cmp3.advanced.ExtendedPersistenceContextJUnitTestSuite
> ;
> ! import
> oracle.toplink.essentials.testing.tests.cmp3.advanced.ReportQueryConstructorExpressionTestSuite
> ;
> ! import
> oracle.toplink.essentials.testing.tests.cmp3.advanced.OptimisticConcurrencyJUnitTestSuite;
>
> import
> oracle.toplink.essentials.testing.tests.cmp3.advanced.compositepk.AdvancedCompositePKJunitTest
> ;
>
> --- 24,30 ----
> import junit.framework.TestSuite;
> import junit.framework.Test;
>
> ! import oracle.toplink.essentials.testing.tests.cmp3.advanced.*;
>
> import
> oracle.toplink.essentials.testing.tests.cmp3.advanced.compositepk.AdvancedCompositePKJunitTest
> ;
>
> ***************
> *** 80,85 ****
> --- 72,78 ----
> fullSuite.addTest(ExtendedPersistenceContextJUnitTestSuite.suite
> ());
> fullSuite.addTest(
> ReportQueryConstructorExpressionTestSuite.suite());
> fullSuite.addTest(OptimisticConcurrencyJUnitTestSuite.suite ());
> + fullSuite.addTest(CascadeOperationJUnitTestSuite.suite());
>
> // Advanced - compositepk model
> fullSuite.addTest(AdvancedCompositePKJunitTest.suite());
>
>
> src/java/oracle/toplink/essentials/testing/tests/cmp3/advanced/CascadeOperationJUnitTestSuite.java
> package oracle.toplink.essentials.testing.tests.cmp3.advanced;
>
> import oracle.toplink.essentials.testing.framework.junit.JUnitTestCase;
> import
> oracle.toplink.essentials.testing.models.cmp3.advanced.AdvancedTableCreator;
> import oracle.toplink.essentials.testing.models.cmp3.advanced.Project;
> import oracle.toplink.essentials.testing.models.cmp3.advanced.Employee;
> import oracle.toplink.essentials.tools.schemaframework.SchemaManager;
> import junit.framework.Test;
> import junit.framework.TestSuite;
> import junit.extensions.TestSetup ;
>
> import javax.persistence.EntityManager;
>
> public class CascadeOperationJUnitTestSuite extends JUnitTestCase {
>
> public CascadeOperationJUnitTestSuite() {
> super();
> }
>
> public CascadeOperationJUnitTestSuite(String name) {
> super(name);
> }
>
> public static Test suite() {
> TestSuite suite = new TestSuite(
> CascadeOperationJUnitTestSuite.class);
>
> return new TestSetup(suite) {
> protected void setUp(){
> SchemaManager schemaManager = new SchemaManager(
> JUnitTestCase.getServerSession ());
> new AdvancedTableCreator().replaceTables(
> JUnitTestCase.getServerSession(), schemaManager);
> }
>
> protected void tearDown(){
> clearCache();
> }
> };
> }
>
> // JUnit framework will automatically execute all methods starting
> with test...
>
> // Test cascade merge on a detached entity
> public void testCascadeMergeDetached() {
> // setup
> Project p1 = new Project();
> p1.setName("p1");
> Project p2 = new Project();
> p1.setName("p2");
> Employee e1 = new Employee("e1", "");
> Employee e2 = new Employee("e2", "");
>
> EntityManager em = createEntityManager();
> em.getTransaction().begin();
> try {
> em.persist(p1);
> em.persist(p2);
> em.persist(e1);
> em.persist(e2);
>
> em.getTransaction().commit();
> em.clear();
> } catch (RuntimeException re){
> em.getTransaction().rollback();
> throw re;
> }
> // end of setup
>
> //p1,p2,e1,e2 are detached
>
> // associate relationships
> //p1 -> e1 (one-to-one)
> p1.setTeamLeader(e1);
> //e1 -> e2 (one-to-many)
> e1.addManagedEmployee(e2);
> //e2 -> p2 (many-to-many)
> e2.addProject(p2);
> p2.addTeamMember(e2);
>
> em.getTransaction().begin();
> try {
> Project mp1 = em.merge(p1); // cascade merge
> assertTrue(em.contains(mp1));
> assertTrue("Managed instance and detached instance must not be
> same", mp1 != p1);
>
> Employee me1 = mp1.getTeamLeader();
> assertTrue("Cascade merge failed", em.contains(me1));
> assertTrue("Managed instance and detached instance must not be
> same", me1 != e1);
>
> Employee me2 = me1.getManagedEmployees().iterator().next();
> assertTrue("Cascade merge failed", em.contains(me2));
> assertTrue("Managed instance and detached instance must not be
> same", me2 != e2);
>
> Project mp2 = me2.getProjects().iterator().next();
> assertTrue("Cascade merge failed", em.contains(mp2));
> assertTrue("Managed instance and detached instance must not be
> same", mp2 != p2);
>
> em.getTransaction().commit();
> em.clear();
> } catch (RuntimeException re){
> em.getTransaction().rollback();
> throw re;
> }
>
> }
>
> // Test cascade merge on a managed entity
> // Test for GF#1139 - Cascade doesn't work when merging managed entity
> public void testCascadeMergeManaged() {
> // setup
> Project p1 = new Project();
> p1.setName("p1");
> Project p2 = new Project();
> p1.setName("p2");
> Employee e1 = new Employee("e1", "");
> Employee e2 = new Employee("e2", "");
>
> EntityManager em = createEntityManager();
> em.getTransaction().begin();
> try {
> em.persist(p1);
> em.persist(p2);
> em.persist(e1);
> em.persist(e2);
>
> em.getTransaction().commit();
> em.clear();
> } catch (RuntimeException re){
> em.getTransaction().rollback();
> throw re;
> }
> // end of setup
>
> //p1,p2,e1,e2 are detached
>
> em.getTransaction().begin();
> try {
> Project mp1 = em.merge(p1);
> assertTrue(em.contains(mp1));
> assertTrue("Managed instance and detached instance must not be
> same", mp1 != p1);
>
> //p1 -> e1 (one-to-one)
> mp1.setTeamLeader (e1);
> mp1 = em.merge(mp1); // merge again - trigger cascade merge
>
> Employee me1 = mp1.getTeamLeader();
> assertTrue("Cascade merge failed", em.contains(me1));
> assertTrue("Managed instance and detached instance must not be
> same", me1 != e1);
>
> //e1 -> e2 (one-to-many)
> me1.addManagedEmployee(e2);
> me1 = em.merge(me1); // merge again - trigger cascade merge
>
> Employee me2 = me1.getManagedEmployees().iterator().next();
> assertTrue("Cascade merge failed", em.contains(me2));
> assertTrue("Managed instance and detached instance must not be
> same", me2 != e2);
>
> //e2 -> p2 (many-to-many)
> me2.addProject(p2);
> p2.addTeamMember(me2);
> me2 = em.merge(me2); // merge again - trigger cascade merge
>
> Project mp2 = me2.getProjects().iterator().next();
> assertTrue("Cascade merge failed", em.contains(mp2));
> assertTrue("Managed instance and detached instance must not be
> same", mp2 != p2);
>
> em.getTransaction().commit();
> em.clear();
> } catch (RuntimeException re){
> em.getTransaction().rollback();
> throw re;
> }
> }
>
> public static void main(String[] args) {
> // Now run JUnit.
> junit.swingui.TestRunner.main(args);
> }
> }
>
> Thanks
> -Wonseok
>
> On 9/22/06, Gordon Yorke <gordon.yorke_at_oracle.com > wrote:
> >
> > Wonseok, Markus,
> > I think the original proposal was closer to a better solution.
> > Adding another method to ObjectBuilder adds a second merge path that will
> > need to be maintained in the future.
> > I think that if we refactor MergeManager a bit we can have
> > MergeManger not perform the 'short-circuit' when called from a
> > RepeatableWriteUnitOfWork. This will isolate the cascade merging behaviour
> > to the EJB30 implemetation.
> > --Gordon
> >
>
>