[314387] Failed writes on CDOObjects leave bad featureDeltas in transaction

https://bugs.eclipse.org/bugs/show_bug.cgi?id=314387

2 files changed, 37 insertions, 33 deletions

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
index 8e1c005753..97522fdd7f 100644
--- 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
@@ -1,13 +1,3 @@
-/**
- * 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;
@@ -19,6 +9,19 @@ import org.eclipse.emf.cdo.tests.model1.OrderDetail;
 import org.eclipse.emf.cdo.transaction.CDOTransaction;
 
 /**
+ * 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
+ */
+
+/**
+ * Bugzilla 293283/314387 - Failed writes on CDOObjects leave bad featureDeltas in transaction
+ * 
  * @author Caspar De Groot
  */
 public class Bugzilla_293283_Test extends AbstractCDOTest
@@ -80,7 +83,12 @@ public class Bugzilla_293283_Test extends AbstractCDOTest
 
   public void test5()
   {
-    test(Action.REMOVE);
+    test(Action.REMOVE1);
+  }
+
+  public void test6()
+  {
+    test(Action.REMOVE2);
   }
 
   private void test(Action action)
@@ -104,10 +112,14 @@ public class Bugzilla_293283_Test extends AbstractCDOTest
         order1.getOrderDetails().move(3, 0);
         break;
 
-      case REMOVE:
+      case REMOVE1:
         order1.getOrderDetails().remove(3);
         break;
 
+      case REMOVE2:
+        order1.getOrderDetails().remove(-1);
+        break;
+
       case SET:
       {
         OrderDetail newDetail = Model1Factory.eINSTANCE.createOrderDetail();
@@ -135,6 +147,6 @@ public class Bugzilla_293283_Test extends AbstractCDOTest
 
   private enum Action
   {
-    ADD, MOVE1, MOVE2, REMOVE, SET
+    ADD, MOVE1, MOVE2, REMOVE1, REMOVE2, 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 8c75319100..e9fe8b4faf 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
@@ -41,7 +41,6 @@ import org.eclipse.emf.internal.cdo.util.FSMUtil;
 import org.eclipse.net4j.util.ObjectUtil;
 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;
@@ -400,17 +399,24 @@ public final class CDOStore implements EStore
 
   public Object remove(InternalEObject eObject, EStructuralFeature feature, int index)
   {
-    if (FSMUtil.isNative(eObject))
-    {
-      checkIndexOutOfBounds(eObject, feature, index); // Bugzilla 293283
-    }
-
     InternalCDOObject cdoObject = getCDOObject(eObject);
     if (TRACER.isEnabled())
     {
       TRACER.format("remove({0}, {1}, {2})", cdoObject, feature, index); //$NON-NLS-1$
     }
 
+    // Bugzilla 293283 / 314387
+    if (feature.isMany())
+    {
+      InternalCDORevision readLockedRevision = getRevisionForReading(cdoObject);
+      CDOList list = readLockedRevision.getList(feature);
+      int size = list.size();
+      if (index < 0 || size <= index)
+      {
+        throw new IndexOutOfBoundsException("Index: " + index + ", Size: " + size);
+      }
+    }
+
     CDOFeatureDelta delta = new CDORemoveFeatureDeltaImpl(feature, index);
     InternalCDORevision revision = getRevisionForWriting(cdoObject, delta);
 
@@ -573,20 +579,6 @@ public final class CDOStore implements EStore
     return value;
   }
 
-  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 Object convertIdToObject(InternalCDOView view, EObject eObject, EStructuralFeature feature, int index,
       Object value)
   {