Attachment 200494 Details for Bug 352977 – Patch v2 (still wrong)

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

[patch] Patch v2 (still wrong)

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

on 2011-07-28 03:19:14 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Eike Stepper

Created: 2011-07-28 03:19:14 EDT

Size: 7.96 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/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);
>+        }
>       }
>     }
> 
>@@ -1393,12 +1400,17 @@
>     {
>       // Remember current revisions
>       Map<CDOObject, CDORevision> oldRevisions = new HashMap<CDOObject, CDORevision>();
>-      for (CDOObject object : getDirtyObjects().values())
>+      InternalCDOSavepoint previousSavepoint = savepoint.getPreviousSavepoint();
>+      if (previousSavepoint != null)
>       {
>-        CDORevision oldRevision = object.cdoRevision();
>-        if (oldRevision != null)
>+        Map<CDOID, CDOObject> allDirtyObjects = previousSavepoint.getAllDirtyObjects();
>+        for (CDOObject object : allDirtyObjects.values())
>         {
>-          oldRevisions.put(object, oldRevision);
>+          CDORevision oldRevision = object.cdoRevision();
>+          if (oldRevision != null)
>+          {
>+            oldRevisions.put(object, oldRevision);
>+          }
>         }
>       }
> 
>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,20 @@
>         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);
>+        }
>+
>+        if (savepoint == this)
>+        {
>+          break;
>+        }
>       }
> 
>       return dirtyObjects;
>@@ -266,6 +276,11 @@
>         {
>           newObjects.remove(removedID);
>         }
>+
>+        if (savepoint == this)
>+        {
>+          break;
>+        }
>       }
> 
>       return newObjects;
>@@ -335,6 +350,11 @@
>         {
>           allRevisionDeltas.remove(detachedID);
>         }
>+
>+        if (savepoint == this)
>+        {
>+          break;
>+        }
>       }
> 
>       return Collections.unmodifiableMap(allRevisionDeltas);
>@@ -368,34 +388,17 @@
>         {
>           detachedObjects.remove(reattachedID);
>         }
>-      }
> 
>-      return detachedObjects;
>-    }
>-  }
>-
>-  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))
>+        if (savepoint == this)
>         {
>-          return true;
>+          break;
>         }
>       }
>-    }
> 
>-    return false;
>+      return detachedObjects;
>+    }
>   }
> 
>-  // 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 +406,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