summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorCaspar De Groot2010-05-25 22:15:29 (EDT)
committerCaspar De Groot2010-05-25 22:15:29 (EDT)
commitec27b9d0d60cc4f13ef4450a1a11661f4c91c4f8 (patch)
tree96cc59bd1fde8e538793ecbc0031be8d72474dcc
parent82a5aed35285e639dc08c5098587b439339752ba (diff)
downloadcdo-ec27b9d0d60cc4f13ef4450a1a11661f4c91c4f8.zip
cdo-ec27b9d0d60cc4f13ef4450a1a11661f4c91c4f8.tar.gz
cdo-ec27b9d0d60cc4f13ef4450a1a11661f4c91c4f8.tar.bz2
[293283] Failed writes on CDOObjects leave bad featureDeltas in transaction
https://bugs.eclipse.org/bugs/show_bug.cgi?id=293283
-rw-r--r--plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/AllTestsAllConfigs.java2
-rw-r--r--plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_293283_Test.java140
-rw-r--r--plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/CDOStore.java32
3 files changed, 162 insertions, 12 deletions
diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/AllTestsAllConfigs.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/AllTestsAllConfigs.java
index 210ec84..7781013 100644
--- a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/AllTestsAllConfigs.java
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/AllTestsAllConfigs.java
@@ -57,6 +57,7 @@ import org.eclipse.emf.cdo.tests.bugzilla.Bugzilla_283985_CDOTest2;
import org.eclipse.emf.cdo.tests.bugzilla.Bugzilla_283985_SavePointTest;
import org.eclipse.emf.cdo.tests.bugzilla.Bugzilla_285008_Test;
import org.eclipse.emf.cdo.tests.bugzilla.Bugzilla_289932_Test;
+import org.eclipse.emf.cdo.tests.bugzilla.Bugzilla_293283_Test;
import org.eclipse.emf.cdo.tests.bugzilla.Bugzilla_294850_Test;
import org.eclipse.emf.cdo.tests.bugzilla.Bugzilla_294859_Test;
import org.eclipse.emf.cdo.tests.bugzilla.Bugzilla_299190_Test;
@@ -166,6 +167,7 @@ public abstract class AllTestsAllConfigs extends ConfigTestSuite
testClasses.add(Bugzilla_283985_CDOTest2.class);
testClasses.add(Bugzilla_283985_SavePointTest.class);
testClasses.add(Bugzilla_285008_Test.class);
+ testClasses.add(Bugzilla_293283_Test.class);
testClasses.add(Bugzilla_294850_Test.class);
testClasses.add(Bugzilla_294859_Test.class);
testClasses.add(Bugzilla_299190_Test.class);
diff --git a/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_293283_Test.java b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_293283_Test.java
new file mode 100644
index 0000000..8e1c005
--- /dev/null
+++ b/plugins/org.eclipse.emf.cdo.tests/src/org/eclipse/emf/cdo/tests/bugzilla/Bugzilla_293283_Test.java
@@ -0,0 +1,140 @@
+/**
+ * Copyright (c) 2004 - 2010 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.model1.Model1Factory;
+import org.eclipse.emf.cdo.tests.model1.Order;
+import org.eclipse.emf.cdo.tests.model1.OrderDetail;
+import org.eclipse.emf.cdo.transaction.CDOTransaction;
+
+/**
+ * @author Caspar De Groot
+ */
+public class Bugzilla_293283_Test extends AbstractCDOTest
+{
+ private CDOSession session;
+
+ private CDOTransaction tx;
+
+ private Order order1;
+
+ @Override
+ public void setUp() throws Exception
+ {
+ super.setUp();
+
+ session = openSession();
+ tx = session.openTransaction();
+ CDOResource r1 = tx.getOrCreateResource("/r1"); //$NON-NLS-1$
+ r1.getContents().clear();
+
+ Model1Factory factory = Model1Factory.eINSTANCE;
+ order1 = factory.createOrder();
+ OrderDetail detail1 = factory.createOrderDetail();
+ OrderDetail detail2 = factory.createOrderDetail();
+ order1.getOrderDetails().add(detail1);
+ order1.getOrderDetails().add(detail2);
+ r1.getContents().add(order1);
+ tx.commit();
+ }
+
+ @Override
+ public void tearDown() throws Exception
+ {
+ tx.close();
+ session.close();
+
+ super.tearDown();
+ }
+
+ public void test1()
+ {
+ test(Action.ADD);
+ }
+
+ public void test2()
+ {
+ test(Action.SET);
+ }
+
+ public void test3()
+ {
+ test(Action.MOVE1);
+ }
+
+ public void test4()
+ {
+ test(Action.MOVE2);
+ }
+
+ public void test5()
+ {
+ test(Action.REMOVE);
+ }
+
+ private void test(Action action)
+ {
+ try
+ {
+ switch (action)
+ {
+ case ADD:
+ {
+ OrderDetail newDetail = Model1Factory.eINSTANCE.createOrderDetail();
+ order1.getOrderDetails().add(3, newDetail);
+ break;
+ }
+
+ case MOVE1:
+ order1.getOrderDetails().move(0, 3);
+ break;
+
+ case MOVE2:
+ order1.getOrderDetails().move(3, 0);
+ break;
+
+ case REMOVE:
+ order1.getOrderDetails().remove(3);
+ break;
+
+ case SET:
+ {
+ OrderDetail newDetail = Model1Factory.eINSTANCE.createOrderDetail();
+ order1.getOrderDetails().set(3, newDetail);
+ break;
+ }
+ }
+
+ fail("Should have thrown " + IndexOutOfBoundsException.class.getSimpleName()); //$NON-NLS-1$
+ }
+ catch (IndexOutOfBoundsException ex)
+ {
+ // Good
+ }
+
+ try
+ {
+ tx.commit();
+ }
+ catch (Exception e)
+ {
+ fail("Should have committed cleanly, but failed with " + e.getClass().getName() + "\n" + e.getMessage());
+ }
+ }
+
+ private enum Action
+ {
+ ADD, MOVE1, MOVE2, REMOVE, SET
+ }
+}
diff --git a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/CDOStore.java b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/CDOStore.java
index db4d16d..0202bca 100644
--- a/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/CDOStore.java
+++ b/plugins/org.eclipse.emf.cdo/src/org/eclipse/emf/internal/cdo/CDOStore.java
@@ -38,6 +38,7 @@ import org.eclipse.emf.internal.cdo.util.FSMUtil;
import org.eclipse.net4j.util.ImplementationError;
import org.eclipse.net4j.util.om.trace.ContextTracer;
+import org.eclipse.emf.common.util.EList;
import org.eclipse.emf.ecore.EClass;
import org.eclipse.emf.ecore.EObject;
import org.eclipse.emf.ecore.EReference;
@@ -71,13 +72,6 @@ public final class CDOStore implements EStore
private InternalCDOView view;
- // // Used for optimization. Multiple call to CDStore will be sent like size and than add.
- // private EStructuralFeature lastLookupEFeature;
- //
- // private EStructuralFeature lastLookupEStructuralFeature;
- //
- // private Object lock = new Object();
-
/**
* @since 2.0
*/
@@ -480,6 +474,8 @@ public final class CDOStore implements EStore
public Object remove(InternalEObject eObject, EStructuralFeature feature, int index)
{
+ checkIndexOutOfBounds(eObject, feature, index); // Bugzilla 293283
+
InternalCDOObject cdoObject = getCDOObject(eObject);
if (TRACER.isEnabled())
{
@@ -488,11 +484,9 @@ public final class CDOStore implements EStore
CDOFeatureDelta delta = new CDORemoveFeatureDeltaImpl(feature, index);
InternalCDORevision revision = getRevisionForWriting(cdoObject, delta);
- Object result = revision.remove(feature, index);
- result = convertToEMF(eObject, revision, feature, index, result);
-
- return result;
+ Object oldValue = revision.remove(feature, index);
+ return convertToEMF(eObject, revision, feature, index, oldValue);
}
public void clear(InternalEObject eObject, EStructuralFeature feature)
@@ -598,6 +592,20 @@ public final class CDOStore implements EStore
}
}
+ private void checkIndexOutOfBounds(InternalEObject eObject, EStructuralFeature feature, int index)
+ {
+ // Bugzilla 293283
+ if (feature.isMany())
+ {
+ Object o = eObject.eGet(feature);
+ int size = ((EList<?>)o).size();
+ if (index >= size)
+ {
+ throw new IndexOutOfBoundsException("Index: " + index + ", Size: " + size);
+ }
+ }
+ }
+
private static InternalCDORevision getRevisionForWriting(InternalCDOObject cdoObject, CDOFeatureDelta delta)
{
CDOStateMachine.INSTANCE.write(cdoObject, delta);
@@ -609,7 +617,7 @@ public final class CDOStore implements EStore
InternalCDORevision revision = cdoObject.cdoRevision();
if (revision == null)
{
- throw new IllegalStateException("revision == null"); //$NON-NLS-1$
+ throw new IllegalStateException("revision == null");
}
return revision;