Attachment 200450 Details for Bug 352977 – Fix v1 (incomplete)

Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

[patch] Fix v1 (incomplete)

clipboard.txt (text/plain), 7.12 KB, created by Eike Stepper

on 2011-07-27 09:52:26 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Eike Stepper

Created: 2011-07-27 09:52:26 EDT

Size: 7.12 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.emf.cdo.tests
>Index: src/org/eclipse/emf/cdo/tests/TransactionTest.java
>===================================================================
>--- src/org/eclipse/emf/cdo/tests/TransactionTest.java	(revision 8821)
>+++ src/org/eclipse/emf/cdo/tests/TransactionTest.java	(working copy)
>@@ -624,4 +624,58 @@
>     Company company2 = (Company)resource2.getContents().get(0);
>     assertEquals("ESC", company2.getName());
>   }
>+
>+  /**
>+   * Bug 352977.
>+   */
>+  public void testModifyDetachCommit() throws Exception
>+  {
>+    {
>+      Company company = getModel1Factory().createCompany();
>+
>+      CDOSession session = openSession();
>+      CDOTransaction transaction = session.openTransaction();
>+      CDOResource resource = transaction.getOrCreateResource(getResourcePath("/test1"));
>+      resource.getContents().add(company);
>+      transaction.commit();
>+
>+      company.setName("ESC");
>+      resource.getContents().remove(company);
>+      transaction.commit();
>+    }
>+
>+    CDOSession session2 = openSession();
>+    CDOTransaction transaction2 = session2.openTransaction();
>+    CDOResource resource2 = transaction2.getOrCreateResource(getResourcePath("/test1"));
>+    assertEquals(0, resource2.getContents().size());
>+  }
>+
>+  /**
>+   * Bug 352977.
>+   */
>+  public void testModifyDetachWithSavepoints() throws Exception
>+  {
>+    {
>+      Company company = getModel1Factory().createCompany();
>+
>+      CDOSession session = openSession();
>+      CDOTransaction transaction = session.openTransaction();
>+      CDOResource resource = transaction.getOrCreateResource(getResourcePath("/test1"));
>+      resource.getContents().add(company);
>+      transaction.commit();
>+
>+      company.setName("ESC");
>+      transaction.setSavepoint();
>+
>+      resource.getContents().remove(company);
>+      // transaction.setSavepoint();
>+
>+      transaction.commit();
>+    }
>+
>+    CDOSession session2 = openSession();
>+    CDOTransaction transaction2 = session2.openTransaction();
>+    CDOResource resource2 = transaction2.getOrCreateResource(getResourcePath("/test1"));
>+    assertEquals(0, resource2.getContents().size());
>+  }
> }
>#P org.eclipse.emf.cdo
>Index: src/org/eclipse/emf/internal/cdo/object/CDOObjectMerger.java
>===================================================================
>--- src/org/eclipse/emf/internal/cdo/object/CDOObjectMerger.java	(revision 8817)
>+++ src/org/eclipse/emf/internal/cdo/object/CDOObjectMerger.java	(working copy)
>@@ -32,7 +32,8 @@
>    */
>   public synchronized void merge(InternalCDOObject object, CDORevisionDelta delta)
>   {
>-    InternalCDORevision revision = object.cdoRevision().copy();
>+    InternalCDORevision oldRevision = object.cdoRevision();
>+    InternalCDORevision revision = oldRevision.copy();
>     object.cdoInternalSetRevision(revision);
> 
>     // NEW object should stay that state.
>Index: src/org/eclipse/emf/internal/cdo/transaction/CDOTransactionImpl.java
>===================================================================
>--- src/org/eclipse/emf/internal/cdo/transaction/CDOTransactionImpl.java	(revision 8821)
>+++ src/org/eclipse/emf/internal/cdo/transaction/CDOTransactionImpl.java	(working copy)
>@@ -1294,11 +1294,18 @@
>         Map<CDOID, CDOObject> map = isNew ? newObjMaps : dirtyObjects;
>         InternalCDOObject object = (InternalCDOObject)map.get(id);
> 
>-        // Change state of the objects
>-        merger.merge(object, delta);
>+        if (object != null)
>+        {
>+          // Change state of the objects
>+          merger.merge(object, delta);
> 
>-        // Load the object from revision to EObject
>-        object.cdoInternalPostLoad();
>+          // Load the object from revision to EObject
>+          object.cdoInternalPostLoad();
>+        }
>+        else
>+        {
>+          removeObject(id);
>+        }
>       }
>     }
> 
>Index: src/org/eclipse/emf/internal/cdo/transaction/CDOSavepointImpl.java
>===================================================================
>--- src/org/eclipse/emf/internal/cdo/transaction/CDOSavepointImpl.java	(revision 8821)
>+++ src/org/eclipse/emf/internal/cdo/transaction/CDOSavepointImpl.java	(working copy)
>@@ -235,10 +235,15 @@
>         return Collections.unmodifiableMap(getDirtyObjects());
>       }
> 
>-      MultiMap.ListBased<CDOID, CDOObject> dirtyObjects = new MultiMap.ListBased<CDOID, CDOObject>();
>-      for (InternalCDOSavepoint savepoint = this; savepoint != null; savepoint = savepoint.getPreviousSavepoint())
>+      Map<CDOID, CDOObject> allDirtyObjects = new HashMap<CDOID, CDOObject>();
>+      for (InternalCDOSavepoint savepoint = getFirstSavePoint(); savepoint != null; savepoint = savepoint
>+          .getNextSavepoint())
>       {
>-        dirtyObjects.getDelegates().add(savepoint.getDirtyObjects());
>+        allDirtyObjects.putAll(savepoint.getDirtyObjects());
>+        for (CDOID removedID : savepoint.getDetachedObjects().keySet())
>+        {
>+          allDirtyObjects.remove(removedID);
>+        }
>       }
> 
>       return dirtyObjects;
>@@ -374,28 +379,6 @@
>     }
>   }
> 
>-  public boolean isNewObject(CDOID id)
>-  {
>-    if (id.isTemporary())
>-    {
>-      return true;
>-    }
>-
>-    synchronized (transaction)
>-    {
>-      for (InternalCDOSavepoint savepoint = this; savepoint != null; savepoint = savepoint.getPreviousSavepoint())
>-      {
>-        if (savepoint.getNewObjects().containsKey(id))
>-        {
>-          return true;
>-        }
>-      }
>-    }
>-
>-    return false;
>-  }
>-
>-  // TODO Not sure if this new implementation is needed. The existing one passes all tests.
>   // public boolean isNewObject(CDOID id)
>   // {
>   // if (id.isTemporary())
>@@ -403,33 +386,55 @@
>   // return true;
>   // }
>   //
>-  // boolean isNew = false;
>-  // boolean wasNew = false;
>   // synchronized (transaction)
>   // {
>   // for (InternalCDOSavepoint savepoint = this; savepoint != null; savepoint = savepoint.getPreviousSavepoint())
>   // {
>   // if (savepoint.getNewObjects().containsKey(id))
>   // {
>-  // isNew = true;
>-  // wasNew = true;
>-  // }
>-  //
>-  // if (isNew && savepoint.getDetachedObjects().containsKey(id))
>-  // {
>-  // isNew = false;
>-  // }
>-  //
>-  // if (!isNew && wasNew && savepoint.getReattachedObjects().containsKey(id))
>-  // {
>-  // isNew = true;
>+  // return true;
>   // }
>   // }
>   // }
>   //
>-  // return isNew;
>+  // return false;
>   // }
> 
>+  // TODO Not sure if this new implementation is needed. The existing one passes all tests.
>+  public boolean isNewObject(CDOID id)
>+  {
>+    if (id.isTemporary())
>+    {
>+      return true;
>+    }
>+
>+    boolean isNew = false;
>+    boolean wasNew = false;
>+    synchronized (transaction)
>+    {
>+      for (InternalCDOSavepoint savepoint = this; savepoint != null; savepoint = savepoint.getPreviousSavepoint())
>+      {
>+        if (savepoint.getNewObjects().containsKey(id))
>+        {
>+          isNew = true;
>+          wasNew = true;
>+        }
>+
>+        if (isNew && savepoint.getDetachedObjects().containsKey(id))
>+        {
>+          isNew = false;
>+        }
>+
>+        if (!isNew && wasNew && savepoint.getReattachedObjects().containsKey(id))
>+        {
>+          isNew = true;
>+        }
>+      }
>+    }
>+
>+    return isNew;
>+  }
>+
>   public void rollback()
>   {
>     synchronized (transaction)

Actions: View | Diff

Attachments on bug 352977: 200450 | 200494 | 205900