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

Bug 317144

Summary: Notification Merge Problem
Product: [Modeling] EMF Reporter: Pascal Lehmann <pascal.lehmann>
Component: cdo.coreAssignee: Eike Stepper <stepper>
Status: CLOSED FIXED QA Contact: Eike Stepper <stepper>
Severity: normal    
Priority: P3 CC: cyril.jaquier, erwin
Version: 4.0   
Target Milestone: ---   
Hardware: All   
OS: All   
Whiteboard:
Attachments:
Description Flags
proposed patch
stepper: iplog+
Patch+Test - for future reference none

Description Pascal Lehmann CLA 2010-06-17 03:52:32 EDT 
Build Identifier: 3.0

We have the following situation:

Transaction with an Object having a list feature with the following elements: [a, b]
Now we do the following changes before we commit the changes.
- REMOVE 0
- ADD 0
- REMOVE 1

CDODeltaNotification will do a call to NotificationImpl.merge and the other views will get a merged Notifcation which looks like this:

REMOVE_MANY [0, 2], ADD 0

You can already see that the index 2 won't do any good if used (eg. Databinding), also the order of the changes is lost.

I suggest removing the merge call from CDODeltaNotifcation.

The actual Problem comes from NotificationImpl.merge, but afaik in EMF such a change can never happen. So if you think this problem should be fixed in EMF I can also create an EMF bug. The solution would probably be to merge with the latest notification for the same feature in the chain and not with the first.



Reproducible: Always

Steps to Reproduce:
1. Create a transaction and at least one view.
2. Add an object with a many feature, add an adapter to that object on the view.
3. Do the changes on that object as described above, then commit.
Comment 1 Eike Stepper CLA 2010-06-29 12:03:01 EDT 
Guys, let's discuss this on Thursday...
Comment 2 Pascal Lehmann CLA 2010-07-01 04:42:10 EDT 
Created attachment 173177 [details]
proposed patch

Here is a patch removing the call to merge in CDODeltaNotificationImpl.

1) The number of lines that you added/changed is smaller than 250.
confirmed
2) You are the only author of these changed lines.
confirmed
3) You apply the EPL to these changed lines.
confirmed
Comment 3 Eike Stepper CLA 2010-07-02 10:25:25 EDT 
Created attachment 173305 [details]
Patch+Test - for future reference
Comment 4 Eike Stepper CLA 2010-07-02 10:26:53 EDT 
We agree that this issue could/should be fixed in EMF. Until this eventually happens we are working around it in CDO.

Committed to HEAD
Comment 5 Pascal Lehmann CLA 2010-07-02 11:30:26 EDT 
If this bug is to be fixed in EMF, I propose that the merge process starts at the tail of the notification chain instead of the head, and works its way to the head until a notification for the same feature and notifier is found. If the merge can not be done, I think the merge should be aborted and the notification simply added at the end of the chain.

In the given example when the [REMOVE 1] Notification is added to the chain, the merge process would start merging at the [ADD 0] Notification and stop right there, adding the Notification simply to the end of the chain instead trying to merge it to the [REMOVE 0] Notification.

I can try to attach a patch proposal next week.
Comment 6 Eike Stepper CLA 2011-06-23 03:38:11 EDT 
Available in R20110608-1407