Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 349311 - DefaultMerger should not clean DiffGroup with hidden sub diff elements
Summary: DefaultMerger should not clean DiffGroup with hidden sub diff elements
Status: CLOSED FIXED
Alias: None
Product: EMFCompare
Classification: Modeling
Component: Core (show other bugs)
Version: unspecified   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: EMF Compare CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-14 08:49 EDT by Mikaël Barbero CLA
Modified: 2011-07-22 03:06 EDT (History)
1 user (show)

See Also:

Attachments
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 Mikaël Barbero CLA 2011-06-14 08:49:44 EDT 
The cleanDiffGroup(DiffGroup) method is cleaning (removing from its container) an "empty" DiffGroup. From its point of view, an empty DiffGroup is a group with no visible (i.e. not hidden) DiffElement. 

It causes NPE and other IllegalArguments exception when trying to apply a set of merge operations from an AbstractDiffExtension because a parent group can be removed from its container at a given step.

It should remove a DiffGroup only if subDiffElements list is empty and not if there is no more visible diff elements.

Mika
Comment 1 Mikaël Barbero CLA 2011-06-15 03:43:47 EDT 
The patch is to change the method DefaultMerger#cleanDiffGroup(DiffGroup) from 

protected void cleanDiffGroup(DiffGroup diffGroup) {
	if (diffGroup != null && diffGroup.getSubchanges() == 0) {
		final EObject parent = diffGroup.eContainer();
		if (parent instanceof DiffGroup) {
			EcoreUtil.remove(diffGroup);
			cleanDiffGroup((DiffGroup)parent);
		}
	}
}

to

protected void cleanDiffGroup(DiffGroup diffGroup) {
	if (diffGroup != null && diffGroup.getSubDiffElements().isEmpty()) { // This line is different
		final EObject parent = diffGroup.eContainer();
		if (parent instanceof DiffGroup) {
			EcoreUtil.remove(diffGroup);
			cleanDiffGroup((DiffGroup)parent);
		}
	}
}
Comment 2 Laurent Goubet CLA 2011-07-22 03:06:54 EDT 
This had been fixed on master with "diffGroup.getSubDiffElements().size() == 0".