[362270] CDODeltaNotification.getNewValue() returns a CDOIDExternal instead of the EObject from the containing XMIResource

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

6 files changed, 222 insertions, 18 deletions

diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_283985_3_Test.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_283985_3_Test.java
index 2c84f5ca7e..c783f93358 100644
--- a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_283985_3_Test.java
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_283985_3_Test.java
@@ -150,7 +150,7 @@ public class Bugzilla_283985_3_Test extends AbstractCDOTest
   }
 
   /**
-   * Bugzilla 312205 - After detach-reattach-rollback, object is not present in tx
+   * Bug 312205 - After detach-reattach-rollback, object is not present in tx
    */
   public void test4()
   {
diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_362270_Test.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_362270_Test.java
new file mode 100644
index 0000000000..605a6ce581
--- /dev/null
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_362270_Test.java
@@ -0,0 +1,174 @@
+package org.eclipse.emf.cdo.tests.bugzilla;

+

+import org.eclipse.emf.cdo.CDODeltaNotification;

+import org.eclipse.emf.cdo.CDOObject;

+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.model1.Company;

+import org.eclipse.emf.cdo.tests.model1.PurchaseOrder;

+import org.eclipse.emf.cdo.tests.model1.Supplier;

+import org.eclipse.emf.cdo.transaction.CDOTransaction;

+import org.eclipse.emf.cdo.util.CDOUtil;

+

+import org.eclipse.emf.common.command.Command;

+import org.eclipse.emf.common.notify.Notification;

+import org.eclipse.emf.common.notify.impl.AdapterImpl;

+import org.eclipse.emf.common.util.URI;

+import org.eclipse.emf.ecore.EObject;

+import org.eclipse.emf.ecore.EReference;

+import org.eclipse.emf.ecore.resource.Resource;

+import org.eclipse.emf.ecore.resource.ResourceSet;

+import org.eclipse.emf.ecore.resource.impl.ResourceSetImpl;

+import org.eclipse.emf.ecore.xmi.XMIResource;

+import org.eclipse.emf.ecore.xmi.impl.XMIResourceFactoryImpl;

+import org.eclipse.emf.edit.command.AddCommand;

+import org.eclipse.emf.edit.command.RemoveCommand;

+import org.eclipse.emf.transaction.RecordingCommand;

+import org.eclipse.emf.transaction.ResourceSetChangeEvent;

+import org.eclipse.emf.transaction.ResourceSetListenerImpl;

+import org.eclipse.emf.transaction.RollbackException;

+import org.eclipse.emf.transaction.TransactionalEditingDomain;

+

+import org.eclipse.core.runtime.Status;

+

+import java.io.File;

+import java.io.IOException;

+import java.util.Collections;

+import java.util.Map;

+

+/**

+ * Test case for {@link CDODeltaNotification#getNewValue()} which must returns a local {@link EObject} contained in a

+ * {@link XMIResource} because a {@link CDOObject} stored in a {@link CDOResource} references the local {@link EObject}.

+ * 

+ * @author Esteban Dugueperoux

+ */

+public class Bugzilla_362270_Test extends AbstractCDOTest

+{

+  private final EReference SUPPLIERS = getModel1Package().getCompany_Suppliers();

+

+  private final EReference PURCHASE_ORDERS = getModel1Package().getSupplier_PurchaseOrders();

+

+  public void testNotifierNotACDOLegacyAdapter() throws Exception

+  {

+    TransactionalEditingDomain domain = TransactionalEditingDomain.Factory.INSTANCE.createEditingDomain();

+    ResourceSet resourceSet = domain.getResourceSet();

+    registerXMIFactory(resourceSet);

+

+    // 1. Create the CDOResource

+    Company obeoCompany = getModel1Factory().createCompany();

+    obeoCompany.setName("OBEO");

+    obeoCompany.setCity("Nantes");

+

+    Supplier martinSupplier = getModel1Factory().createSupplier();

+    obeoCompany.getSuppliers().add(martinSupplier);

+

+    CDOSession session = openSession();

+    CDOUtil.setLegacyModeDefault(true);

+    final CDOTransaction cdoTransaction = session.openTransaction(resourceSet);

+

+    CDOResource cdoResource = cdoTransaction.createResource(getResourcePath("/test1"));

+    cdoResource.getContents().add(obeoCompany);

+    cdoTransaction.commit();

+

+    // 2. Create the local XMI resource

+    URI localResourceURI = createXMIResource();

+

+    // Reload the local resource through the CDOTransaction!!!

+    Resource localResource = resourceSet.getResource(localResourceURI, true);

+    Company martinCompany = (Company)localResource.getContents().get(0);

+    PurchaseOrder purchaseOrder = martinCompany.getPurchaseOrders().get(0);

+

+    Command addPurchaseOrderCmd = AddCommand.create(domain, martinSupplier, PURCHASE_ORDERS, purchaseOrder);

+    domain.getCommandStack().execute(addPurchaseOrderCmd);

+

+    // 4. Commit

+    cdoTransaction.commit();

+

+    domain.addResourceSetListener(new ResourceSetListenerImpl()

+    {

+      @Override

+      public Command transactionAboutToCommit(ResourceSetChangeEvent event) throws RollbackException

+      {

+        throw new RollbackException(Status.CANCEL_STATUS);

+      }

+    });

+

+    Command removeMartinSupplierCmd = RemoveCommand.create(domain, obeoCompany, SUPPLIERS, martinSupplier);

+    domain.getCommandStack().execute(removeMartinSupplierCmd);

+

+    AssertAdapter assertAdapter = new AssertAdapter(purchaseOrder);

+    martinSupplier.eAdapters().add(assertAdapter);

+

+    // 6. rollback the previous operation

+    domain.getCommandStack().execute(new RecordingCommand(domain)

+    {

+      @Override

+      protected void doExecute()

+      {

+        cdoTransaction.rollback();

+      }

+    });

+  }

+

+  private URI createXMIResource() throws IOException

+  {

+    ResourceSet localResourceSet = new ResourceSetImpl();

+    registerXMIFactory(localResourceSet);

+

+    File localResourceFile = createTempFile("localResource", ".model1").getAbsoluteFile();

+    URI localResourceURI = URI.createFileURI(localResourceFile.getAbsolutePath());

+    Resource localResource = localResourceSet.createResource(localResourceURI);

+

+    Company martinCompany = getModel1Factory().createCompany();

+    martinCompany.setName("Martin");

+    martinCompany.setCity("Berlin");

+

+    PurchaseOrder purchaseOrder = getModel1Factory().createPurchaseOrder();

+    martinCompany.getPurchaseOrders().add(purchaseOrder);

+

+    localResource.getContents().add(martinCompany);

+    localResource.save(Collections.emptyMap());

+

+    return localResourceURI;

+  }

+

+  private void registerXMIFactory(ResourceSet resourceSet)

+  {

+    Map<String, Object> map = resourceSet.getResourceFactoryRegistry().getExtensionToFactoryMap();

+    map.put("model1", new XMIResourceFactoryImpl());

+  }

+

+  private final class AssertAdapter extends AdapterImpl

+  {

+    private EObject eObject;

+

+    private int notifyCounter;

+

+    public AssertAdapter(EObject eObject)

+    {

+      this.eObject = eObject;

+    }

+

+    @Override

+    public void notifyChanged(Notification msg)

+    {

+      if (getModel1Package().getSupplier_PurchaseOrders().equals(msg.getFeature()))

+      {

+        switch (++notifyCounter)

+        {

+        case 1:

+          assertEquals(null, msg.getNewValue());

+          break;

+

+        case 2:

+          assertEquals(eObject, msg.getNewValue());

+          break;

+

+        default:

+          fail("Only 0 or 2 calls are expected");

+        }

+      }

+    }

+  }

+}

diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOSavepointImpl.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOSavepointImpl.java
index 202994d153..e8dbf1365a 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOSavepointImpl.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOSavepointImpl.java
@@ -29,6 +29,7 @@ import org.eclipse.emf.cdo.spi.common.revision.InternalCDOFeatureDelta;
 import org.eclipse.net4j.util.collection.MultiMap;
 import org.eclipse.net4j.util.lifecycle.LifecycleUtil;
 
+import org.eclipse.emf.spi.cdo.CDOTransactionStrategy;
 import org.eclipse.emf.spi.cdo.InternalCDOSavepoint;
 import org.eclipse.emf.spi.cdo.InternalCDOTransaction;
 
@@ -436,7 +437,9 @@ public class CDOSavepointImpl extends CDOUserSavepointImpl implements InternalCD
     {
       InternalCDOTransaction transaction = getTransaction();
       LifecycleUtil.checkActive(transaction);
-      transaction.getTransactionStrategy().rollback(transaction, this);
+
+      CDOTransactionStrategy transactionStrategy = transaction.getTransactionStrategy();
+      transactionStrategy.rollback(transaction, this);
     }
   }
 }
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOTransactionImpl.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOTransactionImpl.java
index 3c1bc55774..e997f8ed64 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOTransactionImpl.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOTransactionImpl.java
@@ -1202,6 +1202,7 @@ public class CDOTransactionImpl extends CDOViewImpl implements InternalCDOTransa
         }
       }
 
+      // Rollback all detached objects
       Map<CDOID, CDOObject> detachedObjectsMap = itrSavepoint.getDetachedObjects();
       if (!detachedObjectsMap.isEmpty())
       {
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/AbstractCDOView.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/AbstractCDOView.java
index be274143c9..acad1f21d4 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/AbstractCDOView.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/AbstractCDOView.java
@@ -19,6 +19,7 @@ import org.eclipse.emf.cdo.common.branch.CDOBranch;
 import org.eclipse.emf.cdo.common.branch.CDOBranchPoint;
 import org.eclipse.emf.cdo.common.commit.CDOChangeSetData;
 import org.eclipse.emf.cdo.common.id.CDOID;
+import org.eclipse.emf.cdo.common.id.CDOIDExternal;
 import org.eclipse.emf.cdo.common.id.CDOIDUtil;
 import org.eclipse.emf.cdo.common.model.CDOClassifierRef;
 import org.eclipse.emf.cdo.common.model.CDOModelUtil;
@@ -693,26 +694,49 @@ public abstract class AbstractCDOView extends Lifecycle implements InternalCDOVi
       return lastLookupObject;
     }
 
-    // Needed for recursive call to getObject. (from createObject/cleanObject/getResource/getObject)
-    InternalCDOObject localLookupObject = objects.get(id);
-    if (localLookupObject == null)
-    {
-      if (!loadOnDemand)
-      {
-        return null;
-      }
+    lastLookupID = null;
+    lastLookupObject = null;
+    InternalCDOObject localLookupObject = null;
 
-      excludeNewObject(id);
-      localLookupObject = createObject(id);
+    if (id.isExternal())
+    {
+      URI uri = URI.createURI(((CDOIDExternal)id).getURI());
+      ResourceSet resourceSet = getResourceSet();
 
-      // CDOResource have a special way to register to the view.
-      if (!CDOModelUtil.isResource(localLookupObject.eClass()))
+      localLookupObject = (InternalCDOObject)CDOUtil.getCDOObject(resourceSet.getEObject(uri, loadOnDemand));
+      if (localLookupObject == null)
       {
-        registerObject(localLookupObject);
+        if (!loadOnDemand)
+        {
+          return null;
+        }
+
+        throw new ObjectNotFoundException(id, this);
       }
-      else if (id.equals(getSession().getRepositoryInfo().getRootResourceID()))
+    }
+    else
+    {
+      // Needed for recursive call to getObject. (from createObject/cleanObject/getResource/getObject)
+      localLookupObject = objects.get(id);
+      if (localLookupObject == null)
       {
-        setRootResource((CDOResourceImpl)localLookupObject);
+        if (!loadOnDemand)
+        {
+          return null;
+        }
+
+        excludeNewObject(id);
+        localLookupObject = createObject(id);
+
+        // CDOResource have a special way to register to the view.
+        if (!CDOModelUtil.isResource(localLookupObject.eClass()))
+        {
+          registerObject(localLookupObject);
+        }
+        else if (id.equals(getSession().getRepositoryInfo().getRootResourceID()))
+        {
+          setRootResource((CDOResourceImpl)localLookupObject);
+        }
       }
     }
 
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/CDOStateMachine.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/CDOStateMachine.java
index 1bd7a4d3cb..80ea51a1a4 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/CDOStateMachine.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/CDOStateMachine.java
@@ -11,6 +11,7 @@
  */
 package org.eclipse.emf.internal.cdo.view;
 
+import org.eclipse.emf.cdo.CDOObject;
 import org.eclipse.emf.cdo.CDOState;
 import org.eclipse.emf.cdo.common.id.CDOID;
 import org.eclipse.emf.cdo.common.id.CDOIDTemp;
@@ -690,7 +691,8 @@ public final class CDOStateMachine extends FiniteStateMachine<CDOState, CDOEvent
       }
 
       // Add the object to the set of reattached objects
-      transaction.getLastSavepoint().getReattachedObjects().put(id, object);
+      Map<CDOID, CDOObject> reattachedObjects = transaction.getLastSavepoint().getReattachedObjects();
+      reattachedObjects.put(id, object);
     }
   }