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

Bug 319353

Summary: [EMF Change Model] applyAndReverse generates ChangeDescription with references to not contained objects
Product: [Modeling] EMF Reporter: Cyril Jaquier <cyril.jaquier>
Component: CoreAssignee: Ed Merks <Ed.Merks>
Status: RESOLVED INVALID QA Contact:
Severity: normal    
Priority: P3    
Version: unspecified   
Target Milestone: ---   
Hardware: PC   
OS: Linux   
Whiteboard:
Attachments:
Description Flags
Test case v1
none
Remove changes for not contained objects none

Description Cyril Jaquier CLA 2010-07-09 05:10:59 EDT 
Build Identifier: HEAD

After using applyAndReverse, I get from time to time dangling references. I'll attach a test case that show the problem.

It seems that if you build a tree and remove a node which children have been modified, applyAndReverse will still keep modifications on those children. BUT the children are nowhere contained.

Reproducible: Always
Comment 1 Cyril Jaquier CLA 2010-07-09 05:14:12 EDT 
Created attachment 173842 [details]
Test case v1

Sorry, there are some whitespace changes in the patch :-(
Comment 2 Cyril Jaquier CLA 2010-07-09 09:54:51 EDT 
Created attachment 173860 [details]
Remove changes for not contained objects

This is an attempt at fixing this bug by removing changes for object that are not contained. This patch makes my previous test case pass but breaks ManualRecorderTest#testRecord1.

Again sorry for the whitespace changes in the patch but it seems that the code format changed since 2008. I'm using the formatter template from HEAD.
Comment 3 Ed Merks CLA 2010-07-09 11:13:38 EDT 
The change recorder is properly recording the state changes and restoring them.  Both n2 and n3 are dangling references in the initial state.  If we didn't record their changes, then two applyAndReverse calls wouldn't produce the correct final state so undo-redo would be broken.  Of course in any specific application you'd be free to filter the objects, checking for recorded changes to dangling references, and remove those from the result, but I don't think the framework should just enforce that.
Comment 4 Cyril Jaquier CLA 2010-07-12 05:19:40 EDT 
Thank you Ed. Ok, it makes sense. In my case, I don't need to record such changes so I will filter them in my ChangeRecorder.

Could you just have a quick look at my patch and tell me if eliminateNotContained looks good?
Comment 5 Ed Merks CLA 2010-07-12 12:37:29 EDT 
The condition root.eContainer() == null will necessarily always be true for the result of getRootContainer for a well formed instance.  Otherwise it looks fine.
Comment 6 Cyril Jaquier CLA 2010-07-12 13:01:41 EDT 
(In reply to comment #5)
> The condition root.eContainer() == null will necessarily always be true for the
> result of getRootContainer for a well formed instance.  Otherwise it looks
> fine.

Thank you very much Ed.