Bug 566184 - EModelService#move cannot move within the same container

An element cannot be added to the same parent without removing it first.
Using ECollections.move to move the element in the same container.

Change-Id: Iddf6333450b02a6630803196b2ffa3025f0a578e
Also-by: Lars Vogel <Lars.Vogel@vogella.com>
Signed-off-by: Rolf Theunissen <rolf.theunissen@gmail.com>
diff --git a/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/ModelServiceImpl.java b/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/ModelServiceImpl.java
index dd61a72..6a7f60d 100644
--- a/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/ModelServiceImpl.java
+++ b/bundles/org.eclipse.e4.ui.workbench/src/org/eclipse/e4/ui/internal/workbench/ModelServiceImpl.java
@@ -72,6 +72,7 @@
 import org.eclipse.e4.ui.workbench.modeling.EPartService;
 import org.eclipse.e4.ui.workbench.modeling.EPlaceholderResolver;
 import org.eclipse.e4.ui.workbench.modeling.ElementMatcher;
+import org.eclipse.emf.common.util.ECollections;
 import org.eclipse.emf.ecore.EObject;
 import org.eclipse.emf.ecore.util.EcoreUtil;
 import org.osgi.framework.Bundle;
@@ -717,7 +718,10 @@
 		int curIndex = curParent.getChildren().indexOf(element);
 
 		// Move the model element
-		if (index == -1) {
+		if (newParent == curParent) {
+			int target = index != -1 ? index : newParent.getChildren().size() - 1;
+			ECollections.move(newParent.getChildren(), target, element);
+		} else if (index == -1) {
 			newParent.getChildren().add(element);
 		} else {
 			newParent.getChildren().add(index, element);
diff --git a/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/application/EModelServiceTest.java b/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/application/EModelServiceTest.java
index 74f4b90..df61e32 100644
--- a/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/application/EModelServiceTest.java
+++ b/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/application/EModelServiceTest.java
@@ -274,6 +274,50 @@
 	}
 
 	@Test
+	public void testMoveWithoutIndexWithOneOtherElementInTheSameContainer() {
+		var window = ems.createModelElement(MWindow.class);
+		var part1 = ems.createModelElement(MPart.class);
+		var part2 = ems.createModelElement(MPart.class);
+		window.getChildren().add(part1);
+		window.getChildren().add(part2);
+		var modelService = applicationContext.get(EModelService.class);
+		modelService.move(part1, window);
+		assertSame(part1, window.getChildren().get(1));
+	}
+
+	@Test
+	public void testMoveWithIndexWithTwoOtherElementInTheSameContainer() {
+		var window = ems.createModelElement(MWindow.class);
+		var part1 = ems.createModelElement(MPart.class);
+		var part2 = ems.createModelElement(MPart.class);
+		var part3 = ems.createModelElement(MPart.class);
+		window.getChildren().add(part1);
+		window.getChildren().add(part2);
+		window.getChildren().add(part3);
+		window.setSelectedElement(part2);
+		var modelService = applicationContext.get(EModelService.class);
+		modelService.move(part1, window, 1);
+		assertSame(part1, window.getChildren().get(1));
+		assertSame(part2, window.getSelectedElement());
+	}
+
+	@Test
+	public void testMoveWithIndexWithTwoOtherElementInTheSameContainerKeepSelection() {
+		var window = ems.createModelElement(MWindow.class);
+		var part1 = ems.createModelElement(MPart.class);
+		var part2 = ems.createModelElement(MPart.class);
+		var part3 = ems.createModelElement(MPart.class);
+		window.getChildren().add(part1);
+		window.getChildren().add(part2);
+		window.getChildren().add(part3);
+		window.setSelectedElement(part1);
+		var modelService = applicationContext.get(EModelService.class);
+		modelService.move(part1, window, 1);
+		assertSame(part1, window.getChildren().get(1));
+		assertSame(part1, window.getSelectedElement());
+	}
+
+	@Test
 	public void testCountRenderableChildren_WithWindows() {
 		var window = ems.createModelElement(MWindow.class);
 		application.getChildren().add(window);