From 35a95d2ef5138464501b47d727c6fbd86df0c045 Mon Sep 17 00:00:00 2001 From: Laurent Fasani Date: Tue, 6 Jan 2015 11:06:13 +0100 Subject: [429659] Have EObject.eUnset() clear list for multi-valued features Update TransientStore/CDOStoreImpl.unset() to manage many feature unset. REMOVE/REMOVE_MANY notification of multi-valued features are now correcltly send at unset(). Update Bugzilla_429659_Test to test TransientStore/CDOStoreImpl.unset() with single-valued/multi-valued/settable/unsettable feature in CDO and XMI mode. bug 429659 Change-Id: I779a4ff4f8b40788071a92ee21c0f55942eb8b87 Signed-off-by: Laurent Fasani --- .../cdo/tests/bugzilla/Bugzilla_429659_Test.java | 286 ++++++++++++--------- .../eclipse/emf/internal/cdo/CDOObjectImpl.java | 29 ++- .../emf/internal/cdo/view/CDOStoreImpl.java | 16 +- 3 files changed, 204 insertions(+), 127 deletions(-) (limited to 'plugins') diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_429659_Test.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_429659_Test.java index 88f083212d..a2e7dfa7d2 100644 --- a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_429659_Test.java +++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_429659_Test.java @@ -6,160 +6,210 @@ * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * Alex Lagarde - initial API and implementation + * Laurent Fasani - 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.config.IConfig; -import org.eclipse.emf.cdo.tests.model6.ContainmentObject; -import org.eclipse.emf.cdo.tests.model6.ReferenceObject; -import org.eclipse.emf.cdo.tests.model6.Root; import org.eclipse.emf.cdo.transaction.CDOTransaction; -import org.eclipse.emf.cdo.util.DanglingIntegrityException; +import org.eclipse.emf.common.notify.Notification; +import org.eclipse.emf.common.util.URI; +import org.eclipse.emf.ecore.EObject; +import org.eclipse.emf.ecore.EStructuralFeature; +import org.eclipse.emf.ecore.resource.Resource; import org.eclipse.emf.ecore.resource.ResourceSet; -import org.eclipse.emf.transaction.RecordingCommand; -import org.eclipse.emf.transaction.TransactionalEditingDomain; +import org.eclipse.emf.ecore.resource.impl.ResourceSetImpl; +import org.eclipse.emf.ecore.util.EContentAdapter; +import org.eclipse.emf.ecore.xmi.impl.XMIResourceFactoryImpl; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; /** - * Bug 429659 - BasicEStoreEList.unset() can cause DanglingIntegrityExceptions at commit time. + * Bug 429659 - check notifications when unsetting EStructuralFeature. + * test TransientStore/CDOStoreImpl.unset() with + * single-valued/multi-valued/settable/unsettable feature in CDO + * and XMI mode. * - * @author Alex Lagarde + * @author Laurent Fasani */ public class Bugzilla_429659_Test extends AbstractCDOTest { - private CDOSession session; + public void testUnsetOnUnsettableMultiValuedFeatureXMIResource() throws Exception + { + List objectsToAdd = new ArrayList(); + objectsToAdd.add(getModel3SubpackageFactory().createClass2()); + objectsToAdd.add(getModel3SubpackageFactory().createClass2()); + performUnsetOnMultiValuedFeature(getXMIResource(), getModel3Factory().createClass1(), getModel3Package() + .getClass1_Class2(), true, objectsToAdd); + } - private CDOTransaction transaction; + public void testUnsetOnUnsettableMultiValuedFeatureCDOResource() throws Exception + { + List objectsToAdd = new ArrayList(); + objectsToAdd.add(getModel3SubpackageFactory().createClass2()); + objectsToAdd.add(getModel3SubpackageFactory().createClass2()); + performUnsetOnMultiValuedFeature(getCDOResource(), getModel3Factory().createClass1(), getModel3Package() + .getClass1_Class2(), true, objectsToAdd); + } - private Root root; + public void testUnsetOnNonUnsettableMultiValuedFeatureXMIResource() throws Exception + { + List objectsToAdd = new ArrayList(); + objectsToAdd.add(getModel6Factory().createBaseObject()); + objectsToAdd.add(getModel6Factory().createBaseObject()); + performUnsetOnMultiValuedFeature(getXMIResource(), getModel6Factory().createReferenceObject(), getModel6Package() + .getReferenceObject_ReferenceList(), false, objectsToAdd); + } - private TransactionalEditingDomain editingDomain; + public void testUnsetOnNonUnsettableMultiValuedFeatureCDOResource() throws Exception + { + List objectsToAdd = new ArrayList(); + objectsToAdd.add(getModel6Factory().createBaseObject()); + objectsToAdd.add(getModel6Factory().createBaseObject()); + performUnsetOnMultiValuedFeature(getCDOResource(), getModel6Factory().createReferenceObject(), getModel6Package() + .getReferenceObject_ReferenceList(), false, objectsToAdd); + } - @Override - protected void doSetUp() throws Exception + public void testUnsetOnUnsettableSingleValuedAttributeCDOResource() throws Exception { - super.doSetUp(); + performUnsetOnSingleValuedFeature(getCDOResource(), getModel6Factory().createEmptyStringDefaultUnsettable(), + getModel6Package().getEmptyStringDefaultUnsettable_Attribute(), true, "test"); + } - session = openSession(); - editingDomain = TransactionalEditingDomain.Factory.INSTANCE.createEditingDomain(); - ResourceSet resourceSet = editingDomain.getResourceSet(); - transaction = session.openTransaction(resourceSet); + public void testUnsetOnUnsettableSingleValuedAttributeXMIResource() throws Exception + { + performUnsetOnSingleValuedFeature(getXMIResource(), getModel6Factory().createEmptyStringDefaultUnsettable(), + getModel6Package().getEmptyStringDefaultUnsettable_Attribute(), true, "test"); + } - CDOResource resource = transaction.getOrCreateResource(getResourcePath("resource")); - root = getModel6Factory().createRoot(); - resource.getContents().add(root); + public void testUnsetOnNonUnsettableSingleValuedAttributeCDOResource() throws Exception + { + performUnsetOnSingleValuedFeature(getCDOResource(), getModel6Factory().createEmptyStringDefault(), + getModel6Package().getEmptyStringDefault_Attribute(), false, "test"); } - /** - * Ensures that when creating and then deleting an element from an empty containment list, the {@link CDOTransaction}'s new objects map is empty. - */ - @Skips(IConfig.CAPABILITY_ALL) - public void testUndoElementCreationOnEmptyList() throws Exception + public void testUnsetOnNonUnsettableSingleValuedAttributeXMIResource() throws Exception { - // Testing on an empty containment list - doTestUndoElementCreation(); + performUnsetOnSingleValuedFeature(getXMIResource(), getModel6Factory().createEmptyStringDefault(), + getModel6Package().getEmptyStringDefault_Attribute(), false, "test"); } - /** - * Ensures that when creating and then deleting an element from a non-empty containment list, the {@link CDOTransaction}'s new objects map is empty. - */ - public void testUndoElementCreationOnNonEmptyList() throws Exception + private Resource getCDOResource() { - // Add an element to the list before launching the test - editingDomain.getCommandStack().execute(new RecordingCommand(editingDomain) - { - @Override - protected void doExecute() - { - root.getListA().add(getModel6Factory().createBaseObject()); - } - }); - - doTestUndoElementCreation(); + CDOSession openSession = openSession(); + CDOTransaction transaction = openSession.openTransaction(); + Resource resourceCDO = transaction.createResource(getResourcePath("resource")); + + return resourceCDO; } - /** - * Ensures that when creating and then deleting an element from a containment list, the {@link CDOTransaction}'s new objects map is empty. - */ - private void doTestUndoElementCreation() throws Exception + private Resource getXMIResource() throws Exception { - transaction.commit(); + Resource.Factory.Registry.INSTANCE.getExtensionToFactoryMap().put("*", new XMIResourceFactoryImpl()); + ResourceSet resourceSet = new ResourceSetImpl(); + URI localURI = URI.createFileURI(createTempFile().getCanonicalPath()); + Resource localResource = resourceSet.createResource(localURI); - // Step 1: add an new element to the containment list - editingDomain.getCommandStack().execute(new RecordingCommand(editingDomain) - { - @Override - protected void doExecute() - { - root.getListA().add(getModel6Factory().createBaseObject()); - } - }); - - assertEquals(1, transaction.getNewObjects().size()); - - // Step 2: undo element creation - editingDomain.getCommandStack().undo(); - assertEquals("New elements for which creation was undone should not appear in the transaction's new objects", 0, - transaction.getNewObjects().size()); - - // Step 3: commit - transaction.commit(); + return localResource; } - /** - * Ensures that the following scenario does not lead to any {@link DanglingIntegrityException} at commit: - *
    - *
  • Create 2 objects c1 & c2
    • - *
  • Create an object referencing c2 inside c1
    • - *
  • Undo all these operations
    • - *
  • commit
    • - *
- */ - @Skips(IConfig.CAPABILITY_ALL) - public void testUndoSeveralElementsCreation() throws Exception + private void performUnsetOnSingleValuedFeature(Resource resource, EObject root, EStructuralFeature feature, + boolean unsettable, Object addedObject) throws Exception { - // Step 1: create first element (c1) - editingDomain.getCommandStack().execute(new RecordingCommand(editingDomain) + // check that Feature is unsettable or not + assertEquals(unsettable, feature.isUnsettable()); + + resource.getContents().add(root); + resource.save(Collections.emptyMap()); + + // allow notifications checking + NotificationAsserter notificationAsserter = new NotificationAsserter(); + resource.eAdapters().add(notificationAsserter); + List notifications = notificationAsserter.getNotifications(); + + root.eSet(feature, addedObject); + + // unset + notifications.clear(); + root.eUnset(feature); + + // check + assertEquals("Incorrect number of expected notifications: ", 1, notifications.size()); + assertEquals("Incorrect notification: ", unsettable ? Notification.UNSET : Notification.SET, notifications.get(0) + .getEventType()); + } + + private void performUnsetOnMultiValuedFeature(Resource resource, EObject root, EStructuralFeature feature, + boolean unsettable, List objectstoAdd) throws Exception + { + // check that feature is unsettable or not + assertEquals(unsettable, feature.isUnsettable()); + + resource.getContents().add(root); + resource.save(Collections.emptyMap()); + + // allow notifications checking + NotificationAsserter notificationAsserter = new NotificationAsserter(); + root.eAdapters().add(notificationAsserter); + List notifications = notificationAsserter.getNotifications(); + + // ----------------------------- + // Step 1: unset many elements + ((Collection)root.eGet(feature)).addAll(objectstoAdd); + + // unset + notifications.clear(); + root.eUnset(feature); + + // check + assertEquals("Incorrect number of expected notifications: ", unsettable ? 2 : 1, notifications.size()); + assertEquals("Incorrect notification: ", Notification.REMOVE_MANY, notifications.get(0).getEventType()); + if (unsettable) { - @Override - protected void doExecute() - { - root.getListA().add(getModel6Factory().createContainmentObject()); - } - }); - - // Step 2: create second element (c2) - editingDomain.getCommandStack().execute(new RecordingCommand(editingDomain) + assertEquals("Incorrect notification: ", Notification.UNSET, notifications.get(1).getEventType()); + } + + // ------------------------------------- + // Step 2: unset single element + ((Collection)root.eGet(feature)).add(objectstoAdd.get(0)); + + // unset + notifications.clear(); + root.eUnset(feature); + + // check + assertEquals("Incorrect number of expected notifications: ", unsettable ? 2 : 1, notifications.size()); + assertEquals("Incorrect notification: ", Notification.REMOVE, notifications.get(0).getEventType()); + if (unsettable) + { + assertEquals("Incorrect notification: ", Notification.UNSET, notifications.get(1).getEventType()); + } + + notifications.clear(); + root.eUnset(feature); + assertEquals("Incorrect number of expected notifications: ", unsettable ? 2 : 1, notifications.size()); + } + + private static class NotificationAsserter extends EContentAdapter + { + + private List notifications = new ArrayList(); + + @Override + public void notifyChanged(Notification notification) { - @Override - protected void doExecute() - { - root.getListA().add(getModel6Factory().createContainmentObject()); - } - }); - - // Step 3: create an element inside c1 which references c2 - editingDomain.getCommandStack().execute(new RecordingCommand(editingDomain) + super.notifyChanged(notification); + notifications.add(notification); + } + + public List getNotifications() { - @Override - protected void doExecute() - { - ReferenceObject referenceObject = getModel6Factory().createReferenceObject(); - referenceObject.setReferenceOptional(root.getListA().get(1)); - ((ContainmentObject)root.getListA().get(0)).getContainmentList().add(referenceObject); - } - }); - - // Step 4: undo the 3 previous operations (notice that commit will fail without undoing the first one) - editingDomain.getCommandStack().undo(); - editingDomain.getCommandStack().undo(); - editingDomain.getCommandStack().undo(); - - // Step 5: commit: this should not cause any exception - transaction.commit(); + return notifications; + } } } diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/CDOObjectImpl.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/CDOObjectImpl.java index 17a815a20c..f2dab8e467 100644 --- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/CDOObjectImpl.java +++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/CDOObjectImpl.java @@ -1585,16 +1585,31 @@ public class CDOObjectImpl extends MinimalEStoreEObjectImpl implements InternalC public void unset(InternalEObject eObject, EStructuralFeature feature) { - CDOObjectImpl object = (CDOObjectImpl)eObject; - Object[] settings = object.eSettings; - if (settings == null) + CDOObjectImpl cdoObject = (CDOObjectImpl)eObject; + + if (feature.isMany()) { - // Is already unset - return; + // Object object = get(eObject, feature, NO_INDEX); + Object object = cdoObject.eGet(feature); + if (object instanceof List) + { + List list = (List)object; + list.clear(); + } } + else + { - int transientIndex = getTransientFeatureIndex(eObject, feature); - settings[transientIndex] = null; + Object[] settings = cdoObject.eSettings; + if (settings == null) + { + // Is already unset + return; + } + + int transientIndex = getTransientFeatureIndex(eObject, feature); + settings[transientIndex] = null; + } } } diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/CDOStoreImpl.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/CDOStoreImpl.java index b01dacb2a3..14bf21ddae 100644 --- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/CDOStoreImpl.java +++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/view/CDOStoreImpl.java @@ -457,8 +457,20 @@ public final class CDOStoreImpl implements CDOStore TRACER.format("unset({0}, {1})", cdoObject, feature); //$NON-NLS-1$ } - CDOFeatureDelta delta = new CDOUnsetFeatureDeltaImpl(feature); - writeRevision(cdoObject, delta); + if (feature.isMany()) + { + Object object = cdoObject.eGet(feature); + if (object instanceof List) + { + List list = (List)object; + list.clear(); + } + } + else + { + CDOFeatureDelta delta = new CDOUnsetFeatureDeltaImpl(feature); + writeRevision(cdoObject, delta); + } } } -- cgit v1.2.3