Revert "[492898] Store is modified even for touch modifications - when value is an external object "

This reverts commit 01c1b6a5400bfe9459cd792c09601634fff2415f.

3 files changed, 49 insertions, 214 deletions

diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_492898_Test.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_492898_Test.java
deleted file mode 100644
index 2b2f8c915e..0000000000
--- a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_492898_Test.java
+++ /dev/null
@@ -1,151 +0,0 @@
-/*******************************************************************************
- * Copyright (c) 2016 Obeo.
- * 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:
- *     Obeo - initial API and implementation
- *******************************************************************************/
-package org.eclipse.emf.cdo.tests.bugzilla;
-
-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.PurchaseOrder;
-import org.eclipse.emf.cdo.tests.model1.Supplier;
-import org.eclipse.emf.cdo.transaction.CDOTransaction;
-import org.eclipse.emf.cdo.view.CDOView;
-
-import org.eclipse.emf.common.util.URI;
-import org.eclipse.emf.ecore.EObject;
-import org.eclipse.emf.ecore.resource.Resource;
-import org.eclipse.emf.ecore.resource.ResourceSet;
-import org.eclipse.emf.ecore.xmi.impl.XMIResourceFactoryImpl;
-
-import java.util.Date;
-
-/**
- * Bug 492898 - CDOObject modifies Store even for Touch notifications.
- *
- * @author Maxime Porhel
- */
-public class Bugzilla_492898_Test extends AbstractCDOTest
-{
-  public void testTouchModificationOnReferenceToExternalObject() throws Exception
-  {
-    CDOSession session = openSession();
-    CDOTransaction transaction = session.openTransaction();
-    CDOResource resource = transaction.getOrCreateResource(getResourcePath("/test1"));
-
-    ResourceSet resourceSet = resource.getResourceSet();
-    resourceSet.getResourceFactoryRegistry().getExtensionToFactoryMap().put("xmi", new XMIResourceFactoryImpl());
-
-    URI uri = URI.createFileURI(createTempFile("tempFile", ".xmi").getCanonicalPath());
-    Resource externalResource = resourceSet.createResource(uri);
-
-    PurchaseOrder purchaseOrder = getModel1Factory().createPurchaseOrder();
-    Supplier externalSupplier = getModel1Factory().createSupplier();
-
-    resource.getContents().add(purchaseOrder);
-    externalResource.getContents().add(externalSupplier);
-
-    purchaseOrder.setDate(new Date());
-    externalSupplier.getPurchaseOrders().add(purchaseOrder);
-    externalSupplier.setName("TheExternalSupplier");
-
-    transaction.commit();
-
-    assertDirty(purchaseOrder, transaction, false);
-    assertTransient(externalSupplier);
-
-    purchaseOrder.setSupplier(externalSupplier);
-
-    assertDirty(purchaseOrder, transaction, false);
-    assertTransient(externalSupplier);
-
-    transaction.commit();
-
-    assertDirty(purchaseOrder, transaction, false);
-    assertTransient(externalSupplier);
-  }
-
-  public void testReferenceToExternalObject() throws Exception
-  {
-    CDOSession session = openSession();
-    CDOTransaction transaction = session.openTransaction();
-    CDOResource resource = transaction.getOrCreateResource(getResourcePath("/test1"));
-
-    ResourceSet resourceSet = resource.getResourceSet();
-    resourceSet.getResourceFactoryRegistry().getExtensionToFactoryMap().put("xmi", new XMIResourceFactoryImpl());
-
-    URI uri = URI.createFileURI(createTempFile("tempFile", ".xmi").getCanonicalPath());
-    Resource externalResource = resourceSet.createResource(uri);
-
-    Supplier externalSupplier = getModel1Factory().createSupplier();
-    externalSupplier.setName("TheExternalSupplier");
-    externalResource.getContents().add(externalSupplier);
-
-    PurchaseOrder purchaseOrder = getModel1Factory().createPurchaseOrder();
-    purchaseOrder.setDate(new Date());
-    purchaseOrder.setSupplier(externalSupplier);
-    resource.getContents().add(purchaseOrder);
-
-    Supplier supplier = getModel1Factory().createSupplier();
-    supplier.setName("TheSupplier");
-    resource.getContents().add(supplier);
-
-    transaction.commit();
-
-    assertDirty(purchaseOrder, transaction, false);
-    assertDirty(supplier, transaction, false);
-    assertTransient(externalSupplier);
-
-    // First modification of the reference
-    purchaseOrder.setSupplier(supplier);
-
-    assertDirty(purchaseOrder, transaction, true);
-    assertDirty(supplier, transaction, true);
-    assertTransient(externalSupplier);
-
-    // Second modification (equivalent to undo, same value than last clean revision)
-    purchaseOrder.setSupplier(externalSupplier);
-
-    assertDirty(purchaseOrder, transaction, false);
-    assertDirty(supplier, transaction, false);
-    assertTransient(externalSupplier);
-
-    transaction.commit();
-
-    assertDirty(purchaseOrder, transaction, false);
-    assertDirty(supplier, transaction, false);
-    assertTransient(externalSupplier);
-
-    purchaseOrder.setSupplier(supplier);
-
-    assertDirty(purchaseOrder, transaction, true);
-    assertDirty(supplier, transaction, true);
-    assertTransient(externalSupplier);
-
-    transaction.commit();
-
-    assertDirty(purchaseOrder, transaction, false);
-    assertDirty(supplier, transaction, false);
-    assertTransient(externalSupplier);
-  }
-
-  private static void assertDirty(EObject eObject, CDOView view, boolean dirty)
-  {
-    if (dirty)
-    {
-      assertDirty(eObject, view);
-    }
-    else
-    {
-      assertClean(eObject, view);
-    }
-
-    assertEquals(dirty, view.isDirty());
-  }
-}
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/transaction/CDOUndoDetector.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/transaction/CDOUndoDetector.java
index 43447b61b1..93ce378265 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/transaction/CDOUndoDetector.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/transaction/CDOUndoDetector.java
@@ -32,6 +32,6 @@ public interface CDOUndoDetector
 
   public static final CDOUndoDetector ALL_FEATURES = new CDOUndoDetectorImpl();
 
-  public boolean detectUndo(CDOTransaction transaction, CDORevision cleanRevision, CDORevision currentRevision,
+  public boolean detectUndo(CDOTransaction transaction, CDORevision cleanRevision, CDORevision revision,
       CDOFeatureDelta featureDelta);
 }
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOUndoDetectorImpl.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOUndoDetectorImpl.java
index 681dbc538d..bc80e635e9 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOUndoDetectorImpl.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOUndoDetectorImpl.java
@@ -11,8 +11,6 @@
 package org.eclipse.emf.internal.cdo.transaction;
 
 import org.eclipse.emf.cdo.common.id.CDOID;
-import org.eclipse.emf.cdo.common.id.CDOIDExternal;
-import org.eclipse.emf.cdo.common.id.CDOIDProvider;
 import org.eclipse.emf.cdo.common.revision.CDORevision;
 import org.eclipse.emf.cdo.common.revision.delta.CDOContainerFeatureDelta;
 import org.eclipse.emf.cdo.common.revision.delta.CDOFeatureDelta;
@@ -22,7 +20,6 @@ import org.eclipse.emf.cdo.transaction.CDOUndoDetector;
 
 import org.eclipse.net4j.util.ObjectUtil;
 
-import org.eclipse.emf.ecore.EObject;
 import org.eclipse.emf.ecore.EReference;
 import org.eclipse.emf.ecore.EStructuralFeature;
 
@@ -35,49 +32,48 @@ import java.util.List;
  */
 public class CDOUndoDetectorImpl implements CDOUndoDetector
 {
-  public boolean detectUndo(CDOTransaction transaction, CDORevision cleanRevision, CDORevision currentRevision,
+  public boolean detectUndo(CDOTransaction transaction, CDORevision cleanRevision, CDORevision revision,
       CDOFeatureDelta featureDelta)
   {
     EStructuralFeature feature = featureDelta.getFeature();
-    InternalCDORevision cleanInternalRevision = (InternalCDORevision)cleanRevision;
-    InternalCDORevision currentInternalRevision = (InternalCDORevision)currentRevision;
+    InternalCDORevision revision1 = (InternalCDORevision)cleanRevision;
+    InternalCDORevision revision2 = (InternalCDORevision)revision;
 
-    if (ignore(feature, cleanInternalRevision, currentInternalRevision))
+    if (ignore(feature, revision1, revision2))
     {
       return false;
     }
 
     if (feature == CDOContainerFeatureDelta.CONTAINER_FEATURE)
     {
-      return detectUndoContainer(transaction, cleanInternalRevision, currentInternalRevision);
+      return detectUndoContainer(revision1, revision2);
     }
 
-    Object cleanValue = cleanInternalRevision.getValue(feature);
-    Object currentValue = currentInternalRevision.getValue(feature);
+    Object value1 = revision1.getValue(feature);
+    Object value2 = revision2.getValue(feature);
 
     if (feature instanceof EReference)
     {
       if (feature.isMany())
       {
-        List<?> cleanList = (List<?>)cleanValue;
-        List<?> currentList = (List<?>)currentValue;
+        List<?> list1 = (List<?>)value1;
+        List<?> list2 = (List<?>)value2;
 
-        int cleanSize = size(cleanList);
-        int currentSize = size(currentList);
+        int size1 = size(list1);
+        int size2 = size(list2);
 
-        if (cleanSize != currentSize)
+        if (size1 != size2)
         {
           return false;
         }
 
-        if (cleanSize != 0)
+        if (size1 != 0)
         {
-          for (Iterator<?> cleanIterator = cleanList.iterator(), currentIterator = currentList.iterator(); //
-          cleanIterator.hasNext();)
+          for (Iterator<?> it1 = list1.iterator(), it2 = list2.iterator(); it1.hasNext();)
           {
-            Object cleanID = cleanIterator.next();
-            Object currentID = currentIterator.next();
-            if (!equalReference(transaction, cleanID, currentID))
+            Object id1 = getID(it1.next());
+            Object id2 = getID(it2.next());
+            if (id1 != id2)
             {
               return false;
             }
@@ -87,43 +83,37 @@ public class CDOUndoDetectorImpl implements CDOUndoDetector
         return true;
       }
 
-      return equalReference(transaction, cleanValue, currentValue);
+      value1 = getID(value1);
+      value2 = getID(value2);
+      return value1 == value2;
     }
 
-    return ObjectUtil.equals(cleanValue, currentValue);
+    return ObjectUtil.equals(value1, value2);
   }
 
-  /**
-   * @deprecated As of CDO 4.5 {@link #detectUndo(CDOTransaction, CDORevision, CDORevision, CDOFeatureDelta)} is called.
-   */
-  @Deprecated
-  protected boolean detectUndoContainer(InternalCDORevision cleanRevision, InternalCDORevision currentRevision)
-  {
-    throw new UnsupportedOperationException();
-  }
-
-  protected boolean detectUndoContainer(CDOTransaction transaction, InternalCDORevision cleanRevision,
-      InternalCDORevision currentRevision)
+  protected boolean detectUndoContainer(InternalCDORevision revision1, InternalCDORevision revision2)
   {
-    CDOID cleanResourceID = cleanRevision.getResourceID();
-    CDOID currentResourceID = currentRevision.getResourceID();
-    if (cleanResourceID != currentResourceID)
+    CDOID resourceID1 = revision1.getResourceID();
+    CDOID resourceID2 = revision2.getResourceID();
+    if (resourceID1 != resourceID2)
     {
       return false;
     }
 
-    int cleanContainingFeatureID = cleanRevision.getContainingFeatureID();
-    int currentContainingFeatureID = currentRevision.getContainingFeatureID();
-
-    if (cleanContainingFeatureID != currentContainingFeatureID)
+    int containingFeatureID1 = revision1.getContainingFeatureID();
+    int containingFeatureID2 = revision2.getContainingFeatureID();
+    if (containingFeatureID1 != containingFeatureID2)
     {
       return false;
     }
 
-    Object cleanContainerID = cleanRevision.getContainerID();
-    Object currentContainerID = currentRevision.getContainerID();
+    Object c1 = revision1.getContainerID();
+    Object c2 = revision2.getContainerID();
 
-    if (!equalReference(transaction, cleanContainerID, currentContainerID))
+    // Potentially most expensive check because of EObject/ID conversion in getID()
+    Object containerID1 = getID(c1);
+    Object containerID2 = getID(c2);
+    if (containerID1 != containerID2)
     {
       return false;
     }
@@ -131,24 +121,21 @@ public class CDOUndoDetectorImpl implements CDOUndoDetector
     return true;
   }
 
-  protected boolean ignore(EStructuralFeature feature, InternalCDORevision cleanRevision,
-      InternalCDORevision currentRevision)
+  protected boolean ignore(EStructuralFeature feature, InternalCDORevision revision1, InternalCDORevision revision2)
   {
-    return feature.isMany() && !cleanRevision.isUnchunked() && !currentRevision.isUnchunked();
+    return feature.isMany() && !revision1.isUnchunked() && !revision2.isUnchunked();
   }
 
-  private static boolean equalReference(CDOTransaction transaction, Object cleanValue, Object currentValue)
+  private static Object getID(Object value)
   {
-    if (currentValue instanceof EObject && cleanValue instanceof CDOIDExternal)
-    {
-      CDOID id = ((CDOIDProvider)transaction).provideCDOID(currentValue);
-      if (id != null)
-      {
-        currentValue = id;
-      }
-    }
-
-    return cleanValue == currentValue;
+    // TODO Write tests to see if EObject instead of CDOID instances need special handling
+    // CDOID id = CDOIDUtil.getCDOID(value);
+    // if (id != null)
+    // {
+    // return id;
+    // }
+
+    return value;
   }
 
   private static int size(List<?> list)
@@ -166,7 +153,7 @@ public class CDOUndoDetectorImpl implements CDOUndoDetector
    */
   public static final class NoFeatures implements CDOUndoDetector
   {
-    public boolean detectUndo(CDOTransaction transaction, CDORevision cleanRevision, CDORevision currentRevision,
+    public boolean detectUndo(CDOTransaction transaction, CDORevision revision1, CDORevision revision2,
         CDOFeatureDelta featureDelta)
     {
       return false;
@@ -179,15 +166,14 @@ public class CDOUndoDetectorImpl implements CDOUndoDetector
   public static final class SingleValuedFeatures extends CDOUndoDetectorImpl
   {
     @Override
-    protected boolean ignore(EStructuralFeature feature, InternalCDORevision cleanRevision,
-        InternalCDORevision currentRevision)
+    protected boolean ignore(EStructuralFeature feature, InternalCDORevision revision1, InternalCDORevision revision2)
     {
       if (feature.isMany())
       {
         return false;
       }
 
-      return super.ignore(feature, cleanRevision, currentRevision);
+      return super.ignore(feature, revision1, revision2);
     }
   }
 }