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

Bug 349311

Summary: DefaultMerger should not clean DiffGroup with hidden sub diff elements
Product: [Modeling] EMFCompare Reporter: Mikaël Barbero <mikael.barbero>
Component: CoreAssignee: EMF Compare <emf.compare-inbox>
Status: CLOSED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: laurent.goubet
Version: unspecified   
Target Milestone: ---   
Hardware: PC   
OS: Linux   
Whiteboard:

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".