336729 – CDOSetFeatureDelta doesn't have oldValue

Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 336729 - CDOSetFeatureDelta doesn't have oldValue

Summary: CDOSetFeatureDelta doesn't have oldValue

Status:	CLOSED FIXED

Alias:	None

Product:	EMF
Classification:	Modeling
Component:	cdo.core (show other bugs)
Version:	4.0
Hardware:	All All

Importance:	P3 normal (vote)
Target Milestone:	---
Assignee:	Pascal Lehmann
QA Contact:	Eike Stepper

URL:
Whiteboard:
Keywords:

Depends on:
Blocks:

Reported:	2011-02-09 11:29 EST by Gábor Nagy
Modified:	2011-06-23 03:42 EDT (History)
CC List:	1 user (show)

See Also:

Flags:	stepper: review+

Attachments
patch v1 (3.62 KB, patch) 2011-02-10 10:14 EST, Pascal Lehmann	no flags	Details \| Diff
Patch v2 - ready to be committed (3.64 KB, patch) 2011-02-10 10:19 EST, Eike Stepper	no flags	Details \| Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Gábor Nagy

2011-02-09 11:29:12 EST

Build Identifier: 

We'd like to provide the user with a diff view of changes before committing. However, when CDOSetFeatureDeltaImpl is constructed in CDOStoreImpl,set(), it is not passed in the oldValue.

I can see that oldValue comes from revision.set(), which needs the delta, so there is a bit of a chicken and an egg problem, however wouldn't it work if the old value for the delta was retrieved from the feature like this?

Object originalValue = feature.isMany() ? ((List<?>)eObject.eGet(feature)).get(index) : eObject.eGet(feature);
CDOFeatureDelta delta = new CDOSetFeatureDeltaImpl(feature, index, value, originalValue);
...

Reproducible: Always

Comment 1 Eike Stepper

2011-02-10 02:55:07 EST

Pascal, I have the feeling that we never serialize the oldValue. Do you think it's generally reasonable that we remember it locally?

Comment 2 Pascal Lehmann

2011-02-10 10:14:15 EST

Created attachment 188688 [details]
patch v1

Yes, the oldValue is never serialized, so I think it doesn't hurt keeping it in the featureDelta in the dirty transaction.

The patch puts the oldValue in the CDOSetFeatureDelta, also updates the oldValue for Notfications and probably fixes a notification problem for set operations on lists.

Comment 3 Eike Stepper

2011-02-10 10:19:37 EST

Created attachment 188690 [details]
Patch v2 - ready to be committed

Just a minor reformat ;-)

Comment 4 Pascal Lehmann

2011-02-10 10:25:55 EST

Committed revision 7054:
- trunk/plugins/org.eclipse.emf.cdo
- trunk/plugins/org.eclipse.emf.cdo.common

Comment 5 Pascal Lehmann

2011-02-10 10:28:53 EST

Committed patch v2

Comment 6 Eike Stepper

2011-06-23 03:42:02 EDT

Available in R20110608-1407