Comment 1 Caspar D. 2010-05-26 02:04:19 EDT

Created attachment 169934 [details]
Patch for 3.0, including test case

Patch and test case adopted from 2.0

Comment 2 Eike Stepper 2010-05-26 08:21:57 EDT

This didn't look like the latest patch from bug 293283. I took that one instead and committed so that I can respin another RC2.

Committed to HEAD.

Comment 3 Martin Fluegge 2010-05-26 14:26:50 EDT

This patched caused some trouble with legacy because when the checkIndexOutOfBounds() will be executed on the native object where the remove action is already performed. That'swhy the IndexOutOfBoundsException is wrongly thrown. To fix this this check should not be performed on legacy objects. I am not 100% sure why the checkIndexOutOfBounds() was introduced. Caspar, would you agree that the check is not need to legacy objects or coudl there be a reason why we need additional chekcing for legay here?

Eike commited the changes to HEAD.

Comment 4 Caspar D. 2010-05-27 04:53:04 EDT

Reopening because patch causes problems in Legacy mode,
as reported by Martin F.

Comment 5 Caspar D. 2010-05-27 05:11:09 EDT

(In reply to comment #3)
> This patched caused some trouble with legacy because when
> the checkIndexOutOfBounds() will be executed on the native
> object where the remove action is already performed.
> (snip)

My mistake was to perform the size-check on the EObject; it
should be performed on the revision instead. For non-legacy,
that comes down to the same thing because the size-check is
delegated immediately to the revision; but for legacy it
makes a difference. I'll fix this.

> I am not 100% sure why the checkIndexOutOfBounds() was
> introduced. 

Without this check, a removal with a bad index will fail
*after* the delta representing the change has been added to
the tx, which is too late because we have no mechanism for
removing deltas representing failed operations.

> Caspar, would you agree that the check is not need to
> legacy objects or coudl there be a reason why we need
> additional chekcing for legay here?

I think the CDOStore code should be as uniform as possible.
If I fix the problem, there's no reason/need to skip the
check for legacy objects.

Comment 6 Caspar D. 2010-05-27 05:12:50 EDT

Created attachment 170151 [details]
Patch for 3.0, v2

Comment 7 Eike Stepper 2010-05-27 05:45:32 EDT

Created attachment 170154 [details]
Patch v3 - ready to be committed

Comment 8 Caspar D. 2010-05-27 05:55:59 EDT

Patch and extension of unit test (case index<0) committed to HEAD.

Comment 9 Eike Stepper 2010-06-29 04:35:53 EDT

Available in 3.0 GA:
http://download.eclipse.org/modeling/emf/cdo/updates/3.0-releases/