diff options
author | Caspar De Groot | 2011-03-09 01:37:56 -0500 |
---|---|---|
committer | Caspar De Groot | 2011-03-09 01:37:56 -0500 |
commit | 5249b3022057ed9645f95c03b51720c3057c4e24 (patch) | |
tree | 63b42ef91600d278fdfd1eafdd7fec16e8ba223d | |
parent | 74fe3ea36ee0aa64434a68ef023e7ac1642aa61b (diff) | |
download | cdo-5249b3022057ed9645f95c03b51720c3057c4e24.zip cdo-5249b3022057ed9645f95c03b51720c3057c4e24.tar.gz cdo-5249b3022057ed9645f95c03b51720c3057c4e24.tar.xz |
[Bug 338884] CommitIntegrityCheck does not check inclusion of normal refTargets in NEW state
https://bugs.eclipse.org/bugs/show_bug.cgi?id=338884
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()) |