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

Bug 336729

Summary: CDOSetFeatureDelta doesn't have oldValue
Product: [Modeling] EMF Reporter: Gábor Nagy <gnagy>
Component: cdo.coreAssignee: Pascal Lehmann <pascal.lehmann>
Status: CLOSED FIXED QA Contact: Eike Stepper <stepper>
Severity: normal    
Priority: P3 CC: cbateman
Version: 4.0Flags: stepper: review+
Target Milestone: ---   
Hardware: All   
OS: All   
Whiteboard:
Attachments:
Description Flags
patch v1
none
Patch v2 - ready to be committed none

Description Gábor Nagy CLA 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 CLA 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 CLA 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 CLA 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 CLA 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 CLA 2011-02-10 10:28:53 EST 
Committed patch v2
Comment 6 Eike Stepper CLA 2011-06-23 03:42:02 EDT 
Available in R20110608-1407