Bug 533770: [Sequence Diagram] Layout operands in a CombinedFragment

https://bugs.eclipse.org/bugs/show_bug.cgi?id=533770

- Properly set the Y coordinate of Operands in the Notation, so that the
GridManagement policy can properly compute the operands coverage
- Update the DeleteOperand test assertion, since we now expand the
preceding operand when deleting an operand (So the fragments initially
covered by the deleted operand are now covered by the other operand,
rather than the interaction)

Change-Id: Ibc7d163ebe71c14648f38a0b27f8a5f4bfab8e98
Signed-off-by: Camille Letavernier <cletavernier@eclipsesource.com>

5 files changed, 68 insertions, 27 deletions

diff --git a/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/custom-src/org/eclipse/papyrus/uml/diagram/sequence/edit/helpers/advice/InteractionOperandViewAdvice.java b/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/custom-src/org/eclipse/papyrus/uml/diagram/sequence/edit/helpers/advice/InteractionOperandViewAdvice.java
index f9298f97d59..ed1e170e541 100644
--- a/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/custom-src/org/eclipse/papyrus/uml/diagram/sequence/edit/helpers/advice/InteractionOperandViewAdvice.java
+++ b/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/custom-src/org/eclipse/papyrus/uml/diagram/sequence/edit/helpers/advice/InteractionOperandViewAdvice.java
@@ -20,6 +20,7 @@ import org.eclipse.core.commands.ExecutionException;
 import org.eclipse.core.runtime.IAdaptable;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.draw2d.geometry.Dimension;
+import org.eclipse.draw2d.geometry.Rectangle;
 import org.eclipse.emf.ecore.EStructuralFeature.Setting;
 import org.eclipse.emf.transaction.TransactionalEditingDomain;
 import org.eclipse.gmf.runtime.common.core.command.CommandResult;
@@ -29,12 +30,14 @@ import org.eclipse.gmf.runtime.emf.commands.core.command.AbstractTransactionalCo
 import org.eclipse.gmf.runtime.emf.type.core.edithelper.AbstractEditHelperAdvice;
 import org.eclipse.gmf.runtime.emf.type.core.requests.DestroyDependentsRequest;
 import org.eclipse.gmf.runtime.notation.LayoutConstraint;
+import org.eclipse.gmf.runtime.notation.Location;
 import org.eclipse.gmf.runtime.notation.Node;
 import org.eclipse.gmf.runtime.notation.NotationPackage;
 import org.eclipse.gmf.runtime.notation.Size;
 import org.eclipse.gmf.runtime.notation.View;
 import org.eclipse.papyrus.infra.emf.utils.EMFHelper;
 import org.eclipse.papyrus.infra.gmfdiag.common.adapter.NotationAndTypeAdapter;
+import org.eclipse.papyrus.uml.diagram.sequence.command.SetResizeAndLocationCommand;
 import org.eclipse.papyrus.uml.diagram.sequence.command.SetResizeCommand;
 import org.eclipse.papyrus.uml.diagram.sequence.edit.parts.InteractionOperandEditPart;
 import org.eclipse.uml2.uml.InteractionOperand;
@@ -102,19 +105,21 @@ public class InteractionOperandViewAdvice extends AbstractEditHelperAdvice {
 					return null;
 				}
 				View viewBefore = findViewBefore(viewToDestroy);
+				int height = deletedSize.height();
 				if (viewBefore != null) {
 					Dimension sizeOfViewBefore = getSize(viewBefore);
-					sizeOfViewBefore.expand(0, deletedSize.height());
+					sizeOfViewBefore.expand(0, height);
 					NotationAndTypeAdapter adapter = new NotationAndTypeAdapter(viewBefore.getElement(), viewBefore);
 					ICommand result = new SetResizeCommand(editingDomain, "Expand previous operand", adapter, sizeOfViewBefore);
 					result.execute(monitor, info);
 				} else {
 					View viewAfter = findViewAfter(viewToDestroy);
 					if (viewAfter != null) {
-						Dimension sizeOfViewAfter = getSize(viewAfter);
-						sizeOfViewAfter.expand(0, deletedSize.height());
+						Rectangle boundsOfViewAfter = getBounds(viewAfter);
+						boundsOfViewAfter.height += height; // Do not use expand, because it would also translate the rectangle
+						boundsOfViewAfter.y -= height; // Shift the following operand up (Used by the Grid policy to compute coverage)
 						NotationAndTypeAdapter adapter = new NotationAndTypeAdapter(viewAfter.getElement(), viewAfter);
-						ICommand result = new SetResizeCommand(editingDomain, "Expand previous operand", adapter, sizeOfViewAfter);
+						ICommand result = new SetResizeAndLocationCommand(editingDomain, "Expand previous operand", adapter, boundsOfViewAfter);
 						result.execute(monitor, info);
 					}
 				}
@@ -134,6 +139,7 @@ public class InteractionOperandViewAdvice extends AbstractEditHelperAdvice {
 			return null;
 		}
 
+		@SuppressWarnings("unchecked") // GMF is Java 1.4
 		List<View> viewSiblings = ((View) view.eContainer()).getPersistedChildren();
 
 		int index = viewSiblings.indexOf(view);
@@ -154,6 +160,7 @@ public class InteractionOperandViewAdvice extends AbstractEditHelperAdvice {
 			return null;
 		}
 
+		@SuppressWarnings("unchecked") // GMF is Java 1.4
 		List<View> viewSiblings = ((View) view.eContainer()).getPersistedChildren();
 
 		int index = viewSiblings.indexOf(view) - 1;
@@ -167,18 +174,31 @@ public class InteractionOperandViewAdvice extends AbstractEditHelperAdvice {
 	/**
 	 * @param view
 	 * @return
-	 * 		A new {@link Dimension} instance representing the model size of the {@link View},
+	 * 		A new {@link Rectangle} instance representing the model bounds of the {@link View},
 	 *         or <code>null</code> if the View doesn't have a size.
 	 */
-	private Dimension getSize(View view) {
+	private Rectangle getBounds(View view) {
 		if (view instanceof Node) {
 			Node node = (Node) view;
 			LayoutConstraint constraint = node.getLayoutConstraint();
+			Rectangle bounds = new Rectangle();
 			if (constraint instanceof Size) {
 				Size size = (Size) constraint;
-				return new Dimension(size.getWidth(), size.getHeight());
+				bounds.setSize(size.getWidth(), size.getHeight());
 			}
+			if (constraint instanceof Location) {
+				Location location = (Location) constraint;
+				bounds.setLocation(location.getX(), location.getY());
+			}
+			return bounds;
 		}
 		return null;
 	}
+
+
+
+	private Dimension getSize(View viewToDestroy) {
+		Rectangle bounds = getBounds(viewToDestroy);
+		return bounds == null ? null : bounds.getSize();
+	}
 }
diff --git a/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/custom-src/org/eclipse/papyrus/uml/diagram/sequence/edit/policies/CombinedCreationEditPolicy.java b/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/custom-src/org/eclipse/papyrus/uml/diagram/sequence/edit/policies/CombinedCreationEditPolicy.java
index 7b48b580c57..25c0fab4fb9 100644
--- a/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/custom-src/org/eclipse/papyrus/uml/diagram/sequence/edit/policies/CombinedCreationEditPolicy.java
+++ b/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/custom-src/org/eclipse/papyrus/uml/diagram/sequence/edit/policies/CombinedCreationEditPolicy.java
@@ -42,6 +42,7 @@ import org.eclipse.gmf.runtime.notation.View;
 import org.eclipse.papyrus.infra.gmfdiag.common.adapter.NotationAndTypeAdapter;
 import org.eclipse.papyrus.infra.gmfdiag.common.editpolicies.DefaultCreationEditPolicy;
 import org.eclipse.papyrus.infra.services.edit.utils.RequestParameterConstants;
+import org.eclipse.papyrus.uml.diagram.sequence.command.SetResizeAndLocationCommand;
 import org.eclipse.papyrus.uml.diagram.sequence.command.SetResizeCommand;
 import org.eclipse.papyrus.uml.diagram.sequence.edit.parts.InteractionOperandEditPart;
 
@@ -135,9 +136,9 @@ public class CombinedCreationEditPolicy extends DefaultCreationEditPolicy {
 
 			// We get the size from the mouse cursor location to the bottom of the existing operand
 			int height = targetOperandBounds.getBottom().y() - locationToOperand.y();
-
-			Dimension size = new Dimension(-1, height);
-			ICommand setBoundsCommand = new SetResizeCommand(editingDomain, "Set dimension", descriptor, size);
+			int distanceToCompartmentTop = compartmentFigure.getBounds().getTopLeft().getNegated().translate(locationToCompartment).y;
+			Rectangle bounds = new Rectangle(0, distanceToCompartmentTop, -1, height);
+			ICommand setBoundsCommand = new SetResizeAndLocationCommand(editingDomain, "Set dimension", descriptor, bounds);
 
 			// Also reduce the size of the existing operand, to avoid shifting the entire operands stack
 			View view = targetOperandPart.getNotationView();
diff --git a/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/custom-src/org/eclipse/papyrus/uml/diagram/sequence/edit/policies/CombinedFragmentResizeEditPolicy.java b/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/custom-src/org/eclipse/papyrus/uml/diagram/sequence/edit/policies/CombinedFragmentResizeEditPolicy.java
index 7b60f062f58..f2481a6d22c 100644
--- a/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/custom-src/org/eclipse/papyrus/uml/diagram/sequence/edit/policies/CombinedFragmentResizeEditPolicy.java
+++ b/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/custom-src/org/eclipse/papyrus/uml/diagram/sequence/edit/policies/CombinedFragmentResizeEditPolicy.java
@@ -208,7 +208,7 @@ public class CombinedFragmentResizeEditPolicy extends ResizableEditPolicyEx {
 
 	protected ChangeBoundsRequest getResizeAboveRequest(MoveSeparatorRequest request) {
 		ChangeBoundsRequest requestAbove = new ChangeBoundsRequest(RequestConstants.REQ_RESIZE);
-		requestAbove.setMoveDelta(request.getMoveDelta());
+		requestAbove.setMoveDelta(new Point(0, 0));
 		requestAbove.setSizeDelta(new Dimension(request.getMoveDelta().x, request.getMoveDelta().y));
 		requestAbove.setResizeDirection(PositionConstants.SOUTH);
 		requestAbove.setLocation(request.getLocation());
@@ -218,8 +218,9 @@ public class CombinedFragmentResizeEditPolicy extends ResizableEditPolicyEx {
 
 	protected ChangeBoundsRequest getResizeBelowRequest(MoveSeparatorRequest request) {
 		ChangeBoundsRequest requestBelow = new ChangeBoundsRequest(RequestConstants.REQ_RESIZE);
-		requestBelow.setMoveDelta(request.getMoveDelta());
-		requestBelow.setSizeDelta(new Dimension(-request.getMoveDelta().x, -request.getMoveDelta().y));
+		Point sizeDelta = request.getMoveDelta().getNegated();
+		requestBelow.setSizeDelta(new Dimension(sizeDelta.x, sizeDelta.y));
+		requestBelow.setMoveDelta(request.getMoveDelta().getCopy());
 		requestBelow.setResizeDirection(PositionConstants.NORTH);
 		requestBelow.setLocation(request.getLocation());
 		requestBelow.setEditParts(getOperandBelow(request.getSeparatorIndex()));
diff --git a/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/custom-src/org/eclipse/papyrus/uml/diagram/sequence/referencialgrilling/ResizeOperandEditPolicy.java b/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/custom-src/org/eclipse/papyrus/uml/diagram/sequence/referencialgrilling/ResizeOperandEditPolicy.java
index 06b48ed0b96..9bc2572c095 100644
--- a/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/custom-src/org/eclipse/papyrus/uml/diagram/sequence/referencialgrilling/ResizeOperandEditPolicy.java
+++ b/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/custom-src/org/eclipse/papyrus/uml/diagram/sequence/referencialgrilling/ResizeOperandEditPolicy.java
@@ -16,7 +16,10 @@ package org.eclipse.papyrus.uml.diagram.sequence.referencialgrilling;
 
 import java.util.List;
 
+import org.eclipse.draw2d.IFigure;
 import org.eclipse.draw2d.geometry.Dimension;
+import org.eclipse.draw2d.geometry.Point;
+import org.eclipse.draw2d.geometry.PrecisionRectangle;
 import org.eclipse.emf.transaction.TransactionalEditingDomain;
 import org.eclipse.gef.Request;
 import org.eclipse.gef.RequestConstants;
@@ -26,12 +29,11 @@ import org.eclipse.gef.requests.ChangeBoundsRequest;
 import org.eclipse.gmf.runtime.common.core.command.CompositeCommand;
 import org.eclipse.gmf.runtime.common.core.command.ICommand;
 import org.eclipse.gmf.runtime.diagram.ui.commands.ICommandProxy;
-import org.eclipse.gmf.runtime.diagram.ui.editparts.GraphicalEditPart;
 import org.eclipse.gmf.runtime.diagram.ui.editparts.IGraphicalEditPart;
 import org.eclipse.gmf.runtime.emf.core.util.EObjectAdapter;
 import org.eclipse.gmf.runtime.notation.View;
 import org.eclipse.papyrus.infra.gmfdiag.common.helper.NotationHelper;
-import org.eclipse.papyrus.uml.diagram.sequence.command.SetResizeCommand;
+import org.eclipse.papyrus.uml.diagram.sequence.command.SetResizeAndLocationCommand;
 
 
 /**
@@ -90,16 +92,28 @@ public class ResizeOperandEditPolicy extends GraphicalEditPolicy {
 	}
 
 	private void updateCurrentChildSize(CompositeCommand compositeCommand, ChangeBoundsRequest changeBoundsRequest, TransactionalEditingDomain editingDomain, Object currentEditPart) {
-		GraphicalEditPart operandPart = (GraphicalEditPart) currentEditPart;
+		IGraphicalEditPart operandPart = (IGraphicalEditPart) currentEditPart;
 		View shapeView = NotationHelper.findView(operandPart);
-		Dimension size = operandPart.getFigure().getSize();
+		// Dimension size = operandPart.getFigure().getSize();
+		// Point location = operandPart.getFigure().getBounds().getLocation().getCopy();
 
-		// Take zoom into account; the request contains absolute mouse coordinates delta.
 		Dimension sizeDelta = changeBoundsRequest.getSizeDelta().getCopy();
-		operandPart.getFigure().translateToRelative(sizeDelta);
-		size.expand(sizeDelta.width, sizeDelta.height);
+		Point moveDelta = changeBoundsRequest.getMoveDelta().getCopy();
 
-		ICommand setBoundsCommand = new SetResizeCommand(editingDomain, "Resize Operands", new EObjectAdapter(shapeView), size);
+		// Take zoom into account; the request contains absolute mouse coordinates delta.
+		IFigure operandFigure = operandPart.getFigure();
+		PrecisionRectangle bounds = new PrecisionRectangle(operandFigure.getBounds());
+		operandFigure.translateToAbsolute(bounds);
+		bounds.resize(sizeDelta);
+		bounds.translate(moveDelta);
+		operandFigure.translateToRelative(bounds);
+
+		// Set the new bounds, relative to the parent (CombinedFragment), to make
+		// sure the notation is consistent with the visuals. We should get x = 0; y = sizeOf(Cf_Label) + Sum(sizeOf(previousOperands))
+		IFigure cfFigure = ((IGraphicalEditPart) operandPart.getParent()).getFigure();
+		bounds.translate(cfFigure.getBounds().getTopLeft().getNegated());
+
+		ICommand setBoundsCommand = new SetResizeAndLocationCommand(editingDomain, "Resize Operands", new EObjectAdapter(shapeView), bounds);
 		compositeCommand.add(setBoundsCommand);
 	}
 
diff --git a/tests/junit/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence.tests/src/org/eclipse/papyrus/uml/diagram/sequence/tests/bug/CombinedFragmentRegressionTest.java b/tests/junit/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence.tests/src/org/eclipse/papyrus/uml/diagram/sequence/tests/bug/CombinedFragmentRegressionTest.java
index 5cdd09f531b..88adfd68049 100644
--- a/tests/junit/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence.tests/src/org/eclipse/papyrus/uml/diagram/sequence/tests/bug/CombinedFragmentRegressionTest.java
+++ b/tests/junit/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence.tests/src/org/eclipse/papyrus/uml/diagram/sequence/tests/bug/CombinedFragmentRegressionTest.java
@@ -1,6 +1,6 @@
 /*****************************************************************************
  * Copyright (c) 2018 Christian W. Damus and others.
- * 
+ *
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -8,7 +8,7 @@
  *
  * Contributors:
  *   Christian W. Damus - Initial API and implementation
- *   
+ *
  *****************************************************************************/
 
 package org.eclipse.papyrus.uml.diagram.sequence.tests.bug;
@@ -81,7 +81,7 @@ import org.junit.Test;
 /**
  * Regression tests specifically for {@link CombinedFragment}s in the sequence diagram
  * editor, especially those tracked under the umbrella of <a href="http://eclip.se/533670">bug 533670</a>.
- * 
+ *
  * @author Christian W. Damus
  * @see <a href="http://eclip.se/533670">bug 533670</a>
  */
@@ -258,7 +258,7 @@ public class CombinedFragmentRegressionTest extends AbstractPapyrusTest {
 	/**
 	 * Verify that the deletion of the only (or last remaining) interaction operand
 	 * in a combined fragment results in deletion of the combined fragment, also.
-	 * 
+	 *
 	 * @see <a href="http://eclip.se/533683">bug 533683</a>
 	 */
 	@Test
@@ -283,7 +283,7 @@ public class CombinedFragmentRegressionTest extends AbstractPapyrusTest {
 	/**
 	 * Verify that the deletion of an interaction operand that leaves at least one
 	 * remaining in a combined fragment does not result in deletion of the combined fragment.
-	 * 
+	 *
 	 * @see <a href="http://eclip.se/533683">bug 533683</a>
 	 */
 	@Test
@@ -304,13 +304,18 @@ public class CombinedFragmentRegressionTest extends AbstractPapyrusTest {
 
 		editor.delete(operandEP);
 
+		editor.flushDisplayEvents();
+
 		combinedFragmentEP = editor.findEditPart(cfrag);
 
 		assertThat("Combined fragment was deleted", cfrag.getEnclosingInteraction(), is(interaction));
 		assertThat("Combined fragment no longer presented in diagram", combinedFragmentEP, notNullValue());
 
+		InteractionOperand firstOperand = cfrag.getOperands().get(0);
+		// The CombinedFragment still covers the same area, so fragments should
+		// now be owned by the other operand
 		assertThat("Fragments of deleted operand not retained",
-				interaction.getFragments(), hasItems(deleteSend, deleted));
+				firstOperand.getFragments(), hasItems(deleteSend, deleted));
 	}
 
 	/**