Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorEike Stepper2011-07-27 02:52:16 -0400
committerEike Stepper2011-07-27 02:52:16 -0400
commit00290eb0a8dc80527fdf6aa93e6e670124a1e346 (patch)
tree69016cb36d9404dc4b5c0b75aafed2f0e0f13b72
parenta446533db04677db8686a47de128143ffd6c52f3 (diff)
downloadcdo-00290eb0a8dc80527fdf6aa93e6e670124a1e346.tar.gz
cdo-00290eb0a8dc80527fdf6aa93e6e670124a1e346.tar.xz
cdo-00290eb0a8dc80527fdf6aa93e6e670124a1e346.zip
[353167] CDOSavePoint and Reattachment issue
https://bugs.eclipse.org/bugs/show_bug.cgi?id=353167
-rw-r--r--plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/TransactionTest.java59
-rw-r--r--plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOSavepointImpl.java169
-rw-r--r--plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOTransactionImpl.java5
-rw-r--r--plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/spi/cdo/InternalCDOSavepoint.java2
4 files changed, 146 insertions, 89 deletions
diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/TransactionTest.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/TransactionTest.java
index 1056a47bec..c4daa1b6c9 100644
--- a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/TransactionTest.java
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/TransactionTest.java
@@ -527,6 +527,9 @@ public class TransactionTest extends AbstractCDOTest
}
}
+ /**
+ * Bug 353167.
+ */
public void testReattachCommit() throws Exception
{
Company company = getModel1Factory().createCompany();
@@ -542,6 +545,9 @@ public class TransactionTest extends AbstractCDOTest
transaction.commit();
}
+ /**
+ * Bug 353167.
+ */
public void testReattachModifyCommit() throws Exception
{
{
@@ -565,4 +571,57 @@ public class TransactionTest extends AbstractCDOTest
Company company2 = (Company)resource2.getContents().get(0);
assertEquals("ESC", company2.getName());
}
+
+ /**
+ * Bug 353167.
+ */
+ public void testReattachCommitWithSavepoints() 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();
+
+ resource.getContents().remove(company);
+ transaction.setSavepoint();
+
+ resource.getContents().add(company);
+ transaction.setSavepoint();
+ transaction.commit();
+ }
+
+ /**
+ * Bug 353167.
+ */
+ public void testReattachModifyCommitWithSavepoints() 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();
+
+ resource.getContents().remove(company);
+ // transaction.setSavepoint();
+
+ resource.getContents().add(company);
+ transaction.setSavepoint();
+
+ company.setName("ESC");
+
+ transaction.commit();
+ }
+
+ CDOSession session2 = openSession();
+ CDOTransaction transaction2 = session2.openTransaction();
+ CDOResource resource2 = transaction2.getOrCreateResource(getResourcePath("/test1"));
+ Company company2 = (Company)resource2.getContents().get(0);
+ assertEquals("ESC", company2.getName());
+ }
}
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 db69033adc..ab878152ee 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
@@ -35,7 +35,6 @@ import org.eclipse.emf.spi.cdo.InternalCDOTransaction;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
-import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
@@ -55,9 +54,8 @@ public class CDOSavepointImpl extends CDOUserSavepointImpl implements InternalCD
private Map<CDOID, CDOObject> newObjects = new HashMap<CDOID, CDOObject>();
- private Map<CDOID, CDOObject> dirtyObjects = new HashMap<CDOID, CDOObject>();
-
- private ConcurrentMap<CDOID, CDORevisionDelta> revisionDeltas = new ConcurrentHashMap<CDOID, CDORevisionDelta>();
+ // Bug 283985 (Re-attachment)
+ private Map<CDOID, CDOObject> reattachedObjects = new HashMap<CDOID, CDOObject>();
private Map<CDOID, CDOObject> detachedObjects = new HashMap<CDOID, CDOObject>()
{
@@ -68,24 +66,19 @@ public class CDOSavepointImpl extends CDOUserSavepointImpl implements InternalCD
{
synchronized (transaction)
{
- sharedDetachedObjects.add(key);
- dirtyObjects.remove(key);
baseNewObjects.remove(key);
newObjects.remove(key);
+ reattachedObjects.remove(key);
+ dirtyObjects.remove(key);
revisionDeltas.remove(key);
return super.put(key, object);
}
}
};
- // Bug 283985 (Re-attachment)
- private Map<CDOID, CDOObject> reattachedObjects = new HashMap<CDOID, CDOObject>();
+ private Map<CDOID, CDOObject> dirtyObjects = new HashMap<CDOID, CDOObject>();
- /**
- * Contains all persistent CDOIDs that were removed. The same instance is shared between all save points that belong
- * to the same transaction.
- */
- private Set<CDOID> sharedDetachedObjects;
+ private ConcurrentMap<CDOID, CDORevisionDelta> revisionDeltas = new ConcurrentHashMap<CDOID, CDORevisionDelta>();
private boolean wasDirty;
@@ -93,16 +86,7 @@ public class CDOSavepointImpl extends CDOUserSavepointImpl implements InternalCD
{
super(transaction, lastSavepoint);
this.transaction = transaction;
-
wasDirty = transaction.isDirty();
- if (lastSavepoint == null)
- {
- sharedDetachedObjects = new HashSet<CDOID>();
- }
- else
- {
- sharedDetachedObjects = lastSavepoint.getSharedDetachedObjects();
- }
}
@Override
@@ -177,9 +161,16 @@ public class CDOSavepointImpl extends CDOUserSavepointImpl implements InternalCD
return dirtyObjects;
}
+ @Deprecated
public Set<CDOID> getSharedDetachedObjects()
{
- return sharedDetachedObjects;
+ throw new UnsupportedOperationException();
+ }
+
+ @Deprecated
+ public void recalculateSharedDetachedObjects()
+ {
+ throw new UnsupportedOperationException();
}
public ConcurrentMap<CDOID, CDORevisionDelta> getRevisionDeltas()
@@ -266,27 +257,14 @@ public class CDOSavepointImpl extends CDOUserSavepointImpl implements InternalCD
return Collections.unmodifiableMap(getNewObjects());
}
- if (getSharedDetachedObjects().size() == 0)
- {
- MultiMap.ListBased<CDOID, CDOObject> newObjects = new MultiMap.ListBased<CDOID, CDOObject>();
- for (InternalCDOSavepoint savepoint = this; savepoint != null; savepoint = savepoint.getPreviousSavepoint())
- {
- newObjects.getDelegates().add(savepoint.getNewObjects());
- }
-
- return newObjects;
- }
-
Map<CDOID, CDOObject> newObjects = new HashMap<CDOID, CDOObject>();
- for (InternalCDOSavepoint savepoint = this; savepoint != null; savepoint = savepoint.getPreviousSavepoint())
+ for (InternalCDOSavepoint savepoint = getFirstSavePoint(); savepoint != null; savepoint = savepoint
+ .getNextSavepoint())
{
- for (Entry<CDOID, CDOObject> entry : savepoint.getNewObjects().entrySet())
+ newObjects.putAll(savepoint.getNewObjects());
+ for (CDOID removedID : savepoint.getDetachedObjects().keySet())
{
- CDOID id = entry.getKey();
- if (!getSharedDetachedObjects().contains(id))
- {
- newObjects.put(id, entry.getValue());
- }
+ newObjects.remove(removedID);
}
}
@@ -329,35 +307,37 @@ public class CDOSavepointImpl extends CDOUserSavepointImpl implements InternalCD
}
// We need to combined the result for all delta in different Savepoint
- Map<CDOID, CDORevisionDelta> revisionDeltas = new HashMap<CDOID, CDORevisionDelta>();
+ Map<CDOID, CDORevisionDelta> allRevisionDeltas = new HashMap<CDOID, CDORevisionDelta>();
for (InternalCDOSavepoint savepoint = getFirstSavePoint(); savepoint != null; savepoint = savepoint
.getNextSavepoint())
{
- for (Entry<CDOID, CDORevisionDelta> entry : savepoint.getRevisionDeltas().entrySet())
+ for (CDORevisionDelta revisionDelta : savepoint.getRevisionDeltas().values())
{
- // Skipping new or detached objects
- CDOID id = entry.getKey();
- if (isNewObject(id) || getSharedDetachedObjects().contains(id))
- {
- continue;
- }
-
- CDORevisionDeltaImpl revisionDelta = (CDORevisionDeltaImpl)revisionDeltas.get(id);
- if (revisionDelta == null)
- {
- revisionDeltas.put(id, entry.getValue().copy());
- }
- else
+ CDOID id = revisionDelta.getID();
+ if (!isNewObject(id))
{
- for (CDOFeatureDelta delta : entry.getValue().getFeatureDeltas())
+ CDORevisionDeltaImpl oldRevisionDelta = (CDORevisionDeltaImpl)allRevisionDeltas.get(id);
+ if (oldRevisionDelta == null)
+ {
+ allRevisionDeltas.put(id, revisionDelta.copy());
+ }
+ else
{
- revisionDelta.addFeatureDelta(((InternalCDOFeatureDelta)delta).copy());
+ for (CDOFeatureDelta delta : revisionDelta.getFeatureDeltas())
+ {
+ oldRevisionDelta.addFeatureDelta(((InternalCDOFeatureDelta)delta).copy());
+ }
}
}
}
+
+ for (CDOID detachedID : savepoint.getDetachedObjects().keySet())
+ {
+ allRevisionDeltas.remove(detachedID);
+ }
}
- return Collections.unmodifiableMap(revisionDeltas);
+ return Collections.unmodifiableMap(allRevisionDeltas);
}
}
@@ -371,42 +351,26 @@ public class CDOSavepointImpl extends CDOUserSavepointImpl implements InternalCD
}
Map<CDOID, CDOObject> detachedObjects = new HashMap<CDOID, CDOObject>();
- Set<CDOID> reattachedObjectIDs = new HashSet<CDOID>(); // Bug 283985 (Re-attachment)
- for (InternalCDOSavepoint savepoint = this; savepoint != null; savepoint = savepoint.getPreviousSavepoint())
+ for (InternalCDOSavepoint savepoint = getFirstSavePoint(); savepoint != null; savepoint = savepoint
+ .getNextSavepoint())
{
- reattachedObjectIDs.addAll(savepoint.getReattachedObjects().keySet());
-
for (Entry<CDOID, CDOObject> entry : savepoint.getDetachedObjects().entrySet())
{
- CDOID id = entry.getKey();
- if (!isNewObject(id))
+ CDOID detachedID = entry.getKey();
+ if (!isNewObject(detachedID))
{
- // Bug 283985 (Re-attachment):
- // Object is only included if it was not reattached in a later savepoint
- if (!reattachedObjectIDs.contains(id))
- {
- detachedObjects.put(id, entry.getValue());
- }
+ CDOObject detachedObject = entry.getValue();
+ detachedObjects.put(detachedID, detachedObject);
}
}
- }
-
- return detachedObjects;
- }
- }
- public void recalculateSharedDetachedObjects()
- {
- synchronized (transaction)
- {
- sharedDetachedObjects.clear();
- for (InternalCDOSavepoint savepoint = this; savepoint != null; savepoint = savepoint.getPreviousSavepoint())
- {
- for (CDOID id : savepoint.getDetachedObjects().keySet())
+ for (CDOID reattachedID : savepoint.getReattachedObjects().keySet())
{
- sharedDetachedObjects.add(id);
+ detachedObjects.remove(reattachedID);
}
}
+
+ return detachedObjects;
}
}
@@ -431,6 +395,41 @@ public class CDOSavepointImpl extends CDOUserSavepointImpl implements InternalCD
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)
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 5893f2daca..5f1608b8c5 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
@@ -1053,7 +1053,7 @@ public class CDOTransactionImpl extends CDOViewImpl implements InternalCDOTransa
private boolean isObjectDetached(CDOID id)
{
- return lastSavepoint.getSharedDetachedObjects().contains(id);
+ return lastSavepoint.getAllDetachedObjects().containsKey(id);
}
/**
@@ -1225,8 +1225,6 @@ public class CDOTransactionImpl extends CDOViewImpl implements InternalCDOTransa
private void loadSavepoint(CDOSavepoint savepoint, Set<CDOID> idsOfNewObjectWithDeltas)
{
- lastSavepoint.recalculateSharedDetachedObjects();
-
Map<CDOID, CDOObject> dirtyObjects = getDirtyObjects();
Map<CDOID, CDOObject> newObjMaps = getNewObjects();
Map<CDOID, CDORevision> newBaseRevision = getBaseNewObjects();
@@ -1732,7 +1730,6 @@ public class CDOTransactionImpl extends CDOViewImpl implements InternalCDOTransa
lastSavepoint = firstSavepoint;
firstSavepoint.clear();
firstSavepoint.setNextSavepoint(null);
- firstSavepoint.getSharedDetachedObjects().clear();
cleanRevisions.clear();
dirty = false;
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/spi/cdo/InternalCDOSavepoint.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/spi/cdo/InternalCDOSavepoint.java
index 7f42eb937d..eb708fcb8e 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/spi/cdo/InternalCDOSavepoint.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/spi/cdo/InternalCDOSavepoint.java
@@ -33,8 +33,10 @@ public interface InternalCDOSavepoint extends CDOSavepoint, InternalCDOUserSavep
public void clear();
+ @Deprecated
public Set<CDOID> getSharedDetachedObjects();
+ @Deprecated
public void recalculateSharedDetachedObjects();
/**

Back to the top