372307: [Legacy] eContainer changes not properly propagated

https://bugs.eclipse.org/bugs/show_bug.cgi?id=372307

2 files changed, 222 insertions, 10 deletions

diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_372307_Test.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_372307_Test.java
new file mode 100644
index 0000000000..a39d8fda0c
--- /dev/null
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_372307_Test.java
@@ -0,0 +1,113 @@
+/**
+ * Copyright (c) 2004 - 2012 Eike Stepper (Berlin, Germany) and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *    Steve Monnier - initial API and implementation
+ */
+package org.eclipse.emf.cdo.tests.bugzilla;
+
+import org.eclipse.emf.cdo.common.id.CDOID;
+import org.eclipse.emf.cdo.eresource.CDOResource;
+import org.eclipse.emf.cdo.session.CDOSession;
+import org.eclipse.emf.cdo.tests.AbstractCDOTest;
+import org.eclipse.emf.cdo.tests.model6.A;
+import org.eclipse.emf.cdo.tests.model6.B;
+import org.eclipse.emf.cdo.tests.model6.C;
+import org.eclipse.emf.cdo.tests.model6.D;
+import org.eclipse.emf.cdo.transaction.CDOTransaction;
+import org.eclipse.emf.cdo.util.CDOUtil;
+
+import org.eclipse.emf.common.util.EList;
+
+/**
+ * @author Steve Monnier
+ */
+public class Bugzilla_372307_Test extends AbstractCDOTest
+{
+  public void testMoveElementToOtherContainer() throws Exception
+  {
+    A session1a1 = getModel6Factory().createA();
+    A session1a2 = getModel6Factory().createA();
+    D session1d = getModel6Factory().createD();
+
+    session1a1.getOwnedDs().add(session1d);
+
+    // Create model using session1
+    CDOSession session1 = openSession();
+    CDOTransaction transaction1 = session1.openTransaction();
+    CDOResource resource1 = transaction1.createResource(getResourcePath("myResource"));
+    resource1.getContents().add(session1a1);
+    resource1.getContents().add(session1a2);
+    transaction1.commit();
+
+    System.out.println("a1: " + CDOUtil.getCDOObject(session1a1));
+    System.out.println("a2: " + CDOUtil.getCDOObject(session1a2));
+    System.out.println("d:  " + CDOUtil.getCDOObject(session1d));
+
+    // Open a second session on the shared resource
+    CDOSession session2 = openSession();
+    CDOTransaction transaction2 = session2.openTransaction();
+    CDOResource resource2 = transaction2.getResource(getResourcePath("myResource"), true);
+    A session2a1 = (A)resource2.getContents().get(0);
+    A session2a2 = (A)resource2.getContents().get(1);
+    D session2d = session2a1.getOwnedDs().get(0);
+
+    // Move the D element from parent A1 to A2 in session1 and commit
+    EList<D> ownedDs = session1a2.getOwnedDs();
+    ownedDs.add(session1d);
+    commitAndSync(transaction1, transaction2);
+
+    // Validate that A1 has no content in both sessions
+    assertEquals("A1 in session1 is expected to be empty", 0, session1a1.eContents().size());
+    assertEquals("A1 in session2 is expected to be empty", 0, session2a1.eContents().size());
+
+    // Validate that A2 contains D in both sessions
+    assertEquals("A2 in session1 is expected to have contents", 1, session1a2.eContents().size());
+    assertEquals("A2 in session2 is expected to have contents", 1, session2a2.eContents().size());
+
+    CDOID id1 = CDOUtil.getCDOObject(session1a2.eContents().get(0)).cdoID();
+    CDOID id2 = CDOUtil.getCDOObject(session2a2.eContents().get(0)).cdoID();
+    assertEquals("Element contained in A1 should have the same CDOID in both sessions", id1, id2);
+
+    // Validate that the container of the D element is the same in both sessions
+    id1 = CDOUtil.getCDOObject(session1d.eContainer()).cdoID();
+    id2 = CDOUtil.getCDOObject(session2d.eContainer()).cdoID();
+    assertEquals("Container of D should have the same CDOID in both sessions", id1, id2);
+  }
+
+  public void testMoveElementToOtherContainerSingleValued() throws Exception
+  {
+    B session1a1 = getModel6Factory().createB();
+    B session1a2 = getModel6Factory().createB();
+    C session1d = getModel6Factory().createC();
+
+    session1a1.setOwnedC(session1d);
+
+    // Create model using session1
+    CDOSession session1 = openSession();
+    CDOTransaction transaction1 = session1.openTransaction();
+    CDOResource resource1 = transaction1.createResource(getResourcePath("myResource"));
+    resource1.getContents().add(session1a1);
+    resource1.getContents().add(session1a2);
+    transaction1.commit();
+
+    System.out.println("a1: " + CDOUtil.getCDOObject(session1a1));
+    System.out.println("a2: " + CDOUtil.getCDOObject(session1a2));
+    System.out.println("d:  " + CDOUtil.getCDOObject(session1d));
+
+    // Move the D element from parent A1 to A2 in session1 and commit
+    session1a2.setOwnedC(session1d);
+    transaction1.commit();
+
+    // Open a second session on the shared resource
+    CDOSession session2 = openSession();
+    CDOTransaction transaction2 = session2.openTransaction();
+    C session2d = transaction2.getObject(session1d);
+
+    assertEquals(CDOUtil.getCDOObject(session1a2).cdoID(), CDOUtil.getCDOObject(session2d.eContainer()).cdoID());
+  }
+}
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/object/CDOLegacyAdapter.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/object/CDOLegacyAdapter.java
index 36477ec3c4..3e8022a6a7 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/object/CDOLegacyAdapter.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/object/CDOLegacyAdapter.java
@@ -14,7 +14,9 @@ package org.eclipse.emf.internal.cdo.object;
 import org.eclipse.emf.cdo.CDONotification;
 import org.eclipse.emf.cdo.common.model.EMFUtil;
 import org.eclipse.emf.cdo.transaction.CDOTransaction;
+import org.eclipse.emf.cdo.util.CDOUtil;
 
+import org.eclipse.emf.internal.cdo.CDOObjectImpl;
 import org.eclipse.emf.internal.cdo.bundle.OM;
 
 import org.eclipse.net4j.util.om.trace.ContextTracer;
@@ -22,6 +24,7 @@ import org.eclipse.net4j.util.om.trace.ContextTracer;
 import org.eclipse.emf.common.notify.Adapter;
 import org.eclipse.emf.common.notify.Notification;
 import org.eclipse.emf.common.notify.Notifier;
+import org.eclipse.emf.ecore.EReference;
 import org.eclipse.emf.ecore.EStructuralFeature;
 import org.eclipse.emf.ecore.InternalEObject;
 import org.eclipse.emf.spi.cdo.CDOStore;
@@ -88,33 +91,96 @@ public class CDOLegacyAdapter extends CDOLegacyWrapper implements Adapter.Intern
     CDOStore store = view.getStore();
     if (EMFUtil.isPersistent(feature))
     {
-      switch (msg.getEventType())
+      int eventType = msg.getEventType();
+      int position = msg.getPosition();
+      Object oldValue = msg.getOldValue();
+      Object newValue = msg.getNewValue();
+
+      switch (eventType)
       {
       case Notification.SET:
-        store.set(instance, feature, msg.getPosition(), msg.getNewValue());
+      {
+        store.set(instance, feature, position, newValue);
+        if (feature instanceof EReference)
+        {
+          EReference reference = (EReference)feature;
+          if (reference.isContainment())
+          {
+            if (oldValue != null)
+            {
+              InternalEObject oldChild = (InternalEObject)oldValue;
+              setContainer(store, oldChild, null, 0);
+            }
+
+            if (newValue != null)
+            {
+              InternalEObject newChild = (InternalEObject)newValue;
+              setContainer(store, newChild, this, reference.getFeatureID());
+            }
+          }
+        }
+
         break;
+      }
 
       case Notification.UNSET:
+      {
+        if (feature instanceof EReference)
+        {
+          EReference reference = (EReference)feature;
+          if (reference.isContainment())
+          {
+            @SuppressWarnings("unchecked")
+            List<Object> list = (List<Object>)oldValue;
+            for (Object child : list)
+            {
+              if (child != null)
+              {
+                setContainer(store, (InternalEObject)child, null, 0);
+              }
+            }
+          }
+        }
+
         store.unset(instance, feature);
         break;
+      }
 
       case Notification.MOVE:
-        // TODO Is that correct?
-        store.move(instance, feature, msg.getPosition(), (Integer)msg.getOldValue());
+        store.move(instance, feature, position, (Integer)oldValue);
         break;
 
       case Notification.ADD:
-        store.add(instance, feature, msg.getPosition(), msg.getNewValue());
+        store.add(instance, feature, position, newValue);
+        if (newValue != null && feature instanceof EReference)
+        {
+          EReference reference = (EReference)feature;
+          if (reference.isContainment())
+          {
+            InternalEObject newChild = (InternalEObject)newValue;
+            setContainer(store, newChild, this, reference.getFeatureID());
+          }
+        }
+
         break;
 
       case Notification.ADD_MANY:
       {
-        int pos = msg.getPosition();
+        int pos = position;
         @SuppressWarnings("unchecked")
-        List<Object> list = (List<Object>)msg.getNewValue();
+        List<Object> list = (List<Object>)newValue;
         for (Object object : list)
         {
           store.add(instance, feature, pos++, object);
+          if (object != null && feature instanceof EReference)
+          {
+            EReference reference = (EReference)feature;
+            if (reference.isContainment())
+            {
+              InternalEObject newChild = (InternalEObject)object;
+              setContainer(store, newChild, this, reference.getFeatureID());
+            }
+          }
         }
 
         break;
@@ -122,17 +188,33 @@ public class CDOLegacyAdapter extends CDOLegacyWrapper implements Adapter.Intern
 
       case Notification.REMOVE:
       {
-        store.remove(instance, feature, msg.getPosition());
+        InternalEObject oldChild = (InternalEObject)store.remove(instance, feature, position);
+        if (oldChild != null && feature instanceof EReference)
+        {
+          EReference reference = (EReference)feature;
+          if (reference.isContainment())
+          {
+            setContainer(store, oldChild, null, 0);
+          }
+        }
         break;
       }
 
       case Notification.REMOVE_MANY:
       {
         @SuppressWarnings("unchecked")
-        List<Object> list = (List<Object>)msg.getOldValue();
+        List<Object> list = (List<Object>)oldValue;
         for (int i = list.size() - 1; i >= 0; --i)
         {
-          store.remove(instance, feature, i);
+          InternalEObject oldChild = (InternalEObject)store.remove(instance, feature, i);
+          if (oldChild != null && feature instanceof EReference)
+          {
+            EReference reference = (EReference)feature;
+            if (reference.isContainment())
+            {
+              setContainer(store, oldChild, null, 0);
+            }
+          }
         }
 
         break;
@@ -144,6 +226,23 @@ public class CDOLegacyAdapter extends CDOLegacyWrapper implements Adapter.Intern
     }
   }
 
+  private void setContainer(CDOStore store, InternalEObject object, InternalEObject container, int containingFeatureID)
+  {
+    if (object instanceof CDOObjectImpl)
+    {
+      // Don't touch native objects
+      return;
+    }
+
+    if (FSMUtil.isTransient(CDOUtil.getCDOObject(object)))
+    {
+      // Don't touch transient objects
+      return;
+    }
+
+    store.setContainer(object, null, container, containingFeatureID);
+  }
+
   /**
    * @author Martin Flügge
    * @since 3.0