summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorCaspar De Groot2011-03-09 01:37:56 (EST)
committerCaspar De Groot2011-03-09 01:37:56 (EST)
commit5249b3022057ed9645f95c03b51720c3057c4e24 (patch)
tree63b42ef91600d278fdfd1eafdd7fec16e8ba223d
parent74fe3ea36ee0aa64434a68ef023e7ac1642aa61b (diff)
downloadcdo-5249b3022057ed9645f95c03b51720c3057c4e24.zip
cdo-5249b3022057ed9645f95c03b51720c3057c4e24.tar.gz
cdo-5249b3022057ed9645f95c03b51720c3057c4e24.tar.bz2
[Bug 338884] CommitIntegrityCheck does not check inclusion of normal refTargets in NEW state
https://bugs.eclipse.org/bugs/show_bug.cgi?id=338884
-rw-r--r--plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/AllConfigs.java1
-rw-r--r--plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_338884_Test.java124
-rw-r--r--plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/transaction/CDOPushTransaction.java4
-rw-r--r--plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/transaction/CDOTransaction.java4
-rw-r--r--plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/transaction/CDOTransactionImpl.java6
-rw-r--r--plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/util/CommitIntegrityCheck.java76
6 files changed, 179 insertions, 36 deletions
diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/AllConfigs.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/AllConfigs.java
index 2df911f..bfb35dd 100644
--- a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/AllConfigs.java
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/AllConfigs.java
@@ -209,5 +209,6 @@ public abstract class AllConfigs extends ConfigTestSuite
testClasses.add(Bugzilla_336314_Test.class);
testClasses.add(Bugzilla_336382_Test.class);
testClasses.add(Bugzilla_336590_Test.class);
+ testClasses.add(Bugzilla_338884_Test.class);
}
}
diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_338884_Test.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_338884_Test.java
new file mode 100644
index 0000000..b3f268c
--- /dev/null
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_338884_Test.java
@@ -0,0 +1,124 @@
+/**
+ * Copyright (c) 2004 - 2011 Eike Stepper (Berlin, Germany) and others.
+ * 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:
+ * Caspar De Groot - 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.model4.ContainedElementNoOpposite;
+import org.eclipse.emf.cdo.tests.model4.RefMultiNonContainedNPL;
+import org.eclipse.emf.cdo.tests.model4.RefSingleNonContainedNPL;
+import org.eclipse.emf.cdo.tests.model4.model4Factory;
+import org.eclipse.emf.cdo.transaction.CDOTransaction;
+import org.eclipse.emf.cdo.util.CommitException;
+import org.eclipse.emf.cdo.util.CommitIntegrityException;
+
+import java.util.Collections;
+
+/**
+ * @author Caspar De Groot
+ */
+public class Bugzilla_338884_Test extends AbstractCDOTest
+{
+ public void test_single() throws CommitException
+ {
+ CDOSession session = openSession();
+ CDOTransaction tx = session.openTransaction();
+ CDOResource resource = tx.createResource(getResourcePath("test"));
+
+ // 1. Create object A;
+ model4Factory factory = getModel4Factory();
+ RefSingleNonContainedNPL referencer = factory.createRefSingleNonContainedNPL();
+ resource.getContents().add(referencer);
+
+ // 2. Commit;
+ tx.commit();
+
+ // 3. Create object B;
+ ContainedElementNoOpposite element = factory.createContainedElementNoOpposite();
+ resource.getContents().add(element);
+
+ // 4. Create reference from object A to object B;
+ referencer.setElement(element);
+
+ // 5. Add object A to committables, but not add B;
+ tx.setCommittables(Collections.singleton(referencer));
+
+ // 6. Commit.
+ try
+ {
+ tx.commit();
+ fail("Should have thrown " + CommitException.class.getName());
+ }
+ catch (CommitException e)
+ {
+ Throwable t = e.getCause().getCause();
+ if (t instanceof CommitIntegrityException)
+ {
+ // Good
+ }
+ else
+ {
+ fail("Unexpected exception type: " + t.getClass().getName());
+ }
+ }
+
+ tx.close();
+ session.close();
+ }
+
+ public void test_multi() throws CommitException
+ {
+ CDOSession session = openSession();
+ CDOTransaction tx = session.openTransaction();
+ CDOResource resource = tx.createResource(getResourcePath("test"));
+
+ // 1. Create object A;
+ model4Factory factory = getModel4Factory();
+ RefMultiNonContainedNPL referencer = factory.createRefMultiNonContainedNPL();
+ resource.getContents().add(referencer);
+
+ // 2. Commit;
+ tx.commit();
+
+ // 3. Create object B;
+ ContainedElementNoOpposite element = factory.createContainedElementNoOpposite();
+ resource.getContents().add(element);
+
+ // 4. Create reference from object A to object B;
+ referencer.getElements().add(element);
+
+ // 5. Add object A to committables, but not add B;
+ tx.setCommittables(Collections.singleton(referencer));
+
+ // 6. Commit.
+ try
+ {
+ tx.commit();
+ fail("Should have thrown " + CommitException.class.getName());
+ }
+ catch (CommitException e)
+ {
+ Throwable t = e.getCause().getCause();
+ if (t instanceof CommitIntegrityException)
+ {
+ // Good
+ }
+ else
+ {
+ fail("Unexpected exception type: " + t.getClass().getName());
+ }
+ }
+
+ tx.close();
+ session.close();
+ }
+}
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/transaction/CDOPushTransaction.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/transaction/CDOPushTransaction.java
index 13162b0..4008096 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/transaction/CDOPushTransaction.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/transaction/CDOPushTransaction.java
@@ -685,7 +685,7 @@ public class CDOPushTransaction extends Notifier implements CDOTransaction
/**
* @since 4.0
*/
- public void setCommittables(Set<EObject> committables)
+ public void setCommittables(Set<? extends EObject> committables)
{
delegate.setCommittables(committables);
}
@@ -693,7 +693,7 @@ public class CDOPushTransaction extends Notifier implements CDOTransaction
/**
* @since 4.0
*/
- public Set<EObject> getCommittables()
+ public Set<? extends EObject> getCommittables()
{
return delegate.getCommittables();
}
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/transaction/CDOTransaction.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/transaction/CDOTransaction.java
index 9c4f5fb..23bcd6c 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/transaction/CDOTransaction.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/cdo/transaction/CDOTransaction.java
@@ -168,12 +168,12 @@ public interface CDOTransaction extends CDOView, CDOCommonTransaction, CDOUserTr
/**
* @since 4.0
*/
- public void setCommittables(Set<EObject> committables);
+ public void setCommittables(Set<? extends EObject> committables);
/**
* @since 4.0
*/
- public Set<EObject> getCommittables();
+ public Set<? extends EObject> getCommittables();
/**
* @since 4.0
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 7ad0a99..d9f7759 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
@@ -213,7 +213,7 @@ public class CDOTransactionImpl extends CDOViewImpl implements InternalCDOTransa
/**
* An optional set to specify which objects in this TX are to be committed by {@link #commit()}
*/
- private Set<EObject> committables;
+ private Set<? extends EObject> committables;
/**
* A map to hold a clean (i.e. unmodified) revision for objects that have been modified or detached.
@@ -2164,12 +2164,12 @@ public class CDOTransactionImpl extends CDOViewImpl implements InternalCDOTransa
commitComment = comment;
}
- public synchronized void setCommittables(Set<EObject> committables)
+ public synchronized void setCommittables(Set<? extends EObject> committables)
{
this.committables = committables;
}
- public synchronized Set<EObject> getCommittables()
+ public synchronized Set<? extends EObject> getCommittables()
{
return committables;
}
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/util/CommitIntegrityCheck.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/util/CommitIntegrityCheck.java
index eae943d..1a9a651 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/util/CommitIntegrityCheck.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/util/CommitIntegrityCheck.java
@@ -11,7 +11,9 @@
package org.eclipse.emf.internal.cdo.util;
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;
import org.eclipse.emf.cdo.common.id.CDOIDUtil;
import org.eclipse.emf.cdo.common.model.EMFUtil;
import org.eclipse.emf.cdo.common.revision.delta.CDOAddFeatureDelta;
@@ -162,24 +164,17 @@ public class CommitIntegrityCheck
}
else if (feat instanceof EReference)
{
- EReference ref = (EReference)feat;
- if (ref.isContainment() || hasPersistentOpposite(ref))
+ if (featureDelta instanceof CDOListFeatureDelta)
{
- // Object is dirty with respect to a containment feature or a feature with an eOpposite
- // We must ensure that any reference targets that were added/removed/set, are also
- // included in the commit
- if (featureDelta instanceof CDOListFeatureDelta)
+ for (CDOFeatureDelta innerFeatDelta : ((CDOListFeatureDelta)featureDelta).getListChanges())
{
- for (CDOFeatureDelta innerFeatDelta : ((CDOListFeatureDelta)featureDelta).getListChanges())
- {
- checkFeatureDelta(innerFeatDelta, dirtyObject);
- }
- }
- else
- {
- checkFeatureDelta(featureDelta, dirtyObject);
+ checkFeatureDelta(innerFeatDelta, dirtyObject);
}
}
+ else
+ {
+ checkFeatureDelta(featureDelta, dirtyObject);
+ }
}
}
}
@@ -201,16 +196,16 @@ public class CommitIntegrityCheck
private void checkFeatureDelta(CDOFeatureDelta featureDelta, CDOObject dirtyObject) throws CommitIntegrityException
{
- if (featureDelta instanceof CDORemoveFeatureDelta)
- {
- Object idOrObject = ((CDORemoveFeatureDelta)featureDelta).getValue();
- CDOID id = (CDOID)transaction.convertObjectToID(idOrObject);
- checkIncluded(id, "removed child of", dirtyObject);
- }
- else if (featureDelta instanceof CDOAddFeatureDelta)
+ EReference ref = (EReference)featureDelta.getFeature();
+ boolean containmentOrWithOpposite = ref.isContainment() || hasPersistentOpposite(ref);
+
+ if (featureDelta instanceof CDOAddFeatureDelta)
{
Object idOrObject = ((CDOAddFeatureDelta)featureDelta).getValue();
- checkIncluded(idOrObject, "added child of", dirtyObject);
+ if (containmentOrWithOpposite || isNew(idOrObject))
+ {
+ checkIncluded(idOrObject, "added child/refTarget of", dirtyObject);
+ }
}
else if (featureDelta instanceof CDOSetFeatureDelta)
{
@@ -220,17 +215,26 @@ public class CommitIntegrityCheck
if (oldIDOrObject != null)
{
// Old child must be included
- checkIncluded(oldID, "removed/former child of", dirtyObject);
+ checkIncluded(oldID, "removed/former child/refTarget of", dirtyObject);
}
if (newIDOrObject != null)
{
// New child must be included
newIDOrObject = transaction.convertObjectToID(newIDOrObject);
- checkIncluded(newIDOrObject, "new child of", dirtyObject);
+ if (containmentOrWithOpposite || isNew(newIDOrObject))
+ {
+ checkIncluded(newIDOrObject, "new child/refTarget of", dirtyObject);
+ }
}
}
- else if (featureDelta instanceof CDOClearFeatureDelta)
+ else if (containmentOrWithOpposite && featureDelta instanceof CDORemoveFeatureDelta)
+ {
+ Object idOrObject = ((CDORemoveFeatureDelta)featureDelta).getValue();
+ CDOID id = (CDOID)transaction.convertObjectToID(idOrObject);
+ checkIncluded(id, "removed child/refTarget of", dirtyObject);
+ }
+ else if (containmentOrWithOpposite && featureDelta instanceof CDOClearFeatureDelta)
{
EStructuralFeature feat = ((CDOClearFeatureDelta)featureDelta).getFeature();
InternalCDORevision cleanRev = transaction.getCleanRevisions().get(dirtyObject);
@@ -239,18 +243,18 @@ public class CommitIntegrityCheck
{
Object idOrObject = cleanRev.get(feat, i);
CDOID id = (CDOID)transaction.convertObjectToID(idOrObject);
- checkIncluded(id, "removed child of", dirtyObject);
+ checkIncluded(id, "removed child/refTarget of", dirtyObject);
}
}
- else if (featureDelta instanceof CDOUnsetFeatureDelta)
+ else if (containmentOrWithOpposite && featureDelta instanceof CDOUnsetFeatureDelta)
{
EStructuralFeature feat = ((CDOUnsetFeatureDelta)featureDelta).getFeature();
InternalCDORevision cleanRev = transaction.getCleanRevisions().get(dirtyObject);
Object idOrObject = cleanRev.getValue(feat);
CDOID id = (CDOID)transaction.convertObjectToID(idOrObject);
- checkIncluded(id, "removed child of", dirtyObject);
+ checkIncluded(id, "removed child/refTarget of", dirtyObject);
}
- else if (featureDelta instanceof CDOMoveFeatureDelta)
+ else if (containmentOrWithOpposite && featureDelta instanceof CDOMoveFeatureDelta)
{
// Nothing to do: a move doesn't affect the child being moved
// so that child does not need to be included
@@ -261,6 +265,20 @@ public class CommitIntegrityCheck
}
}
+ private boolean isNew(Object idOrObject)
+ {
+ if (idOrObject instanceof CDOIDTemp)
+ {
+ return true;
+ }
+ if (idOrObject instanceof EObject)
+ {
+ CDOObject obj = CDOUtil.getCDOObject((EObject)idOrObject);
+ return obj.cdoState() == CDOState.NEW;
+ }
+ return false;
+ }
+
private void checkIncluded(CDOID id, String msg, CDOObject o) throws CommitIntegrityException
{
if (id.isNull())