From f68469bb935bda8df3cca7c6318a0f981b195b42 Mon Sep 17 00:00:00 2001 From: Remi Schnekenburger Date: Wed, 26 Dec 2018 13:48:25 +0100 Subject: [PATCH 1/4] Issue #389: Update self message router and feeback - update the self router to not use bendpoints - update feedback to switch from self to oblique and vice versa - update management of start/end messages for execution specifications - add some tests for self messages corner cases - add some (weird) refresh method for tests, to get them green on remote build. working fine locally PDE and maven, but not on remote build server - update anchors on executions to manage automatically the sides. Change-Id: Ieb479d5b1109c4f4b4a76c0df5e45b48c73192d2 Signed-off-by: Remi Schnekenburger --- .../sequence/figure/MessageFigure.java | 9 +- .../ExecutionSpecificationEndAnchor.java | 7 + .../ExecutionSpecificationSideAnchor.java | 3 +- .../ExecutionSpecificationStartAnchor.java | 7 + .../figure/anchors/LifelineBodyAnchor.java | 28 +- .../parts/ExecutionSpecificationEditPart.java | 7 +- .../internal/edit/parts/MessageEditPart.java | 56 +++ ...stractSequenceGraphicalNodeEditPolicy.java | 67 ++- .../policies/LifelineBodyDropEditPolicy.java | 12 + .../policies/MessageEndpointEditPolicy.java | 105 ++++ .../internal/locators/SelfMessageRouter.java | 66 +++ .../sequence/runtime/util/MessageUtil.java | 16 + .../model/commands/InsertMessageCommand.java | 12 +- .../model/commands/PendingChildData.java | 11 + .../model/commands/SetCoveredCommand.java | 138 ++++- .../model/commands/SetOwnerCommand.java | 145 +++++- .../model/spi/impl/DefaultDiagramHelper.java | 91 ++-- .../interaction/model/util/Executions.java | 18 +- .../AbstractGraphicalEditPolicyUITest.java | 3 + .../tests/ExecutionAnchorsUITest.java | 2 + ...ficationGraphicalNodeEditPolicyUITest.java | 38 +- .../tests/ExecutionWithSelfMoveUITest.java | 128 +++++ .../tests/LifelineSwitchingUITest.java | 471 +++++++++++++++++- .../tests/SelfMessageCreationUITest.java | 27 +- .../tests/SelfMessageReconnectionUITest.java | 250 +++++----- .../edit/policies/tests/execution-self.di | 2 + .../policies/tests/execution-self.notation | 42 ++ .../edit/policies/tests/execution-self.uml | 16 + .../runtime/tests/rules/EditorFixture.java | 14 + 29 files changed, 1544 insertions(+), 247 deletions(-) create mode 100644 plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/locators/SelfMessageRouter.java create mode 100644 tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/ExecutionWithSelfMoveUITest.java create mode 100644 tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/execution-self.di create mode 100644 tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/execution-self.notation create mode 100644 tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/execution-self.uml diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/MessageFigure.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/MessageFigure.java index 4c562b39..aa587019 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/MessageFigure.java +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/MessageFigure.java @@ -48,15 +48,18 @@ public ConnectionAnchor getConnectionAnchor(String terminal) { @Override public void validate() { - super.validate(); - + if (isValid()) { + return; + } MessageDirection direction = computeDirection(); if (getSourceAnchor() instanceof ISideAnchor) { - ((ISideAnchor)getSourceAnchor()).setConnectionSide(direction.getExecutionSide(true)); + int side = direction.getExecutionSide(true); + ((ISideAnchor)getSourceAnchor()).setConnectionSide(side); } if (getTargetAnchor() instanceof ISideAnchor) { ((ISideAnchor)getTargetAnchor()).setConnectionSide(direction.getExecutionSide(false)); } + super.validate(); } MessageDirection computeDirection() { diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/ExecutionSpecificationEndAnchor.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/ExecutionSpecificationEndAnchor.java index d8ad0eed..616a1433 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/ExecutionSpecificationEndAnchor.java +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/ExecutionSpecificationEndAnchor.java @@ -36,6 +36,13 @@ public void setConnectionSide(int side) { } } + @Override + public Point getReferencePoint() { + Rectangle body = getOwner().getBounds().getCopy(); + getOwner().translateToAbsolute(body); + return body.getBottomLeft(); + } + @Override public Point getLocation(Point reference) { Rectangle body = getOwner().getBounds().getCopy(); diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/ExecutionSpecificationSideAnchor.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/ExecutionSpecificationSideAnchor.java index cb898acf..ddc89f47 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/ExecutionSpecificationSideAnchor.java +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/ExecutionSpecificationSideAnchor.java @@ -12,6 +12,7 @@ package org.eclipse.papyrus.uml.diagram.sequence.figure.anchors; import static org.eclipse.draw2d.PositionConstants.LEFT; +import static org.eclipse.draw2d.PositionConstants.RIGHT; import org.eclipse.core.runtime.Assert; import org.eclipse.draw2d.AbstractConnectionAnchor; @@ -29,7 +30,7 @@ public class ExecutionSpecificationSideAnchor extends AbstractConnectionAnchor i public ExecutionSpecificationSideAnchor(IFigure figure, int height) { super(figure); - this.side = LEFT; + this.side = RIGHT; this.height = height; } diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/ExecutionSpecificationStartAnchor.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/ExecutionSpecificationStartAnchor.java index 0f840396..124a1ed7 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/ExecutionSpecificationStartAnchor.java +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/ExecutionSpecificationStartAnchor.java @@ -36,6 +36,13 @@ public void setConnectionSide(int side) { } } + @Override + public Point getReferencePoint() { + Rectangle body = getOwner().getBounds().getCopy(); + getOwner().translateToAbsolute(body); + return body.getTop(); + } + @Override public Point getLocation(Point reference) { Rectangle body = getOwner().getBounds().getCopy(); diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/LifelineBodyAnchor.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/LifelineBodyAnchor.java index 7326fad2..24cb89f3 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/LifelineBodyAnchor.java +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/LifelineBodyAnchor.java @@ -25,17 +25,14 @@ */ public class LifelineBodyAnchor extends AbstractConnectionAnchor implements ISequenceAnchor { - private final LifelineBodyFigure lifelinebodyFigure; private int distance; - public LifelineBodyAnchor(LifelineBodyFigure lifelinebodyFigure, int distance) { super(lifelinebodyFigure); // We actually attach the anchor to the BodyFigure of the Lifeline // Note: this causes issues for policies/interaction, because the body is far away // from the Lifeline's Bounds. However, we're only interested in rendering here, and this works fine this.distance = distance; - this.lifelinebodyFigure = lifelinebodyFigure; } @Override @@ -45,13 +42,24 @@ public String getTerminal() { @Override public Point getLocation(Point reference) { - Rectangle body = lifelinebodyFigure.getBounds().getCopy(); - lifelinebodyFigure.translateToAbsolute(body); - int boundedHeight = Math.min(distance, body.height); - int realHeight = (int)Math.round(boundedHeight * getScale(lifelinebodyFigure)); - Point location = new Point(0, realHeight); - location.translate(body.getTopLeft()); - return location; + return getReferencePoint(); + } + + @Override + public Point getReferencePoint() { + if (getOwner() == null) { + return null; + } else { + // reference point is the top of the lifeline + // useful for the router + Rectangle body = getOwner().getBounds().getCopy(); + getOwner().translateToAbsolute(body); + int boundedHeight = Math.min(distance, body.height); + int realHeight = (int)Math.round(boundedHeight * getScale(getOwner())); + Point location = new Point(1, realHeight); + location.translate(body.getTopLeft()); + return location; + } } /** diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/parts/ExecutionSpecificationEditPart.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/parts/ExecutionSpecificationEditPart.java index c2dcda24..c83382f3 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/parts/ExecutionSpecificationEditPart.java +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/parts/ExecutionSpecificationEditPart.java @@ -24,6 +24,7 @@ import org.eclipse.emf.ecore.util.EcoreUtil; import org.eclipse.gef.DragTracker; import org.eclipse.gef.EditPart; +import org.eclipse.gef.EditPartViewer; import org.eclipse.gef.EditPolicy; import org.eclipse.gef.Request; import org.eclipse.gmf.runtime.diagram.ui.editparts.BorderedBorderItemEditPart; @@ -136,7 +137,11 @@ private void refreshLifeline(MLifeline lifeline) { } private void refreshEditPartOfShape(Shape shape) { - Object lifelineEditPart = getViewer().getEditPartRegistry().get(shape); + EditPartViewer viewer = getViewer(); + if (viewer == null) { + return; + } + Object lifelineEditPart = viewer.getEditPartRegistry().get(shape); if (lifelineEditPart instanceof EditPart) { EditPart editPart = (EditPart)lifelineEditPart; List lifelineEditpartChildren = editPart.getChildren(); diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/parts/MessageEditPart.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/parts/MessageEditPart.java index 9a370407..6da4167f 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/parts/MessageEditPart.java +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/parts/MessageEditPart.java @@ -16,11 +16,14 @@ import com.google.common.eventbus.EventBus; +import java.beans.PropertyChangeEvent; import java.util.List; import java.util.stream.Stream; import org.eclipse.draw2d.Connection; import org.eclipse.draw2d.ConnectionAnchor; +import org.eclipse.draw2d.ConnectionLayer; +import org.eclipse.draw2d.ConnectionRouter; import org.eclipse.draw2d.Graphics; import org.eclipse.draw2d.PolygonDecoration; import org.eclipse.draw2d.PolylineDecoration; @@ -31,32 +34,43 @@ import org.eclipse.gef.DragTracker; import org.eclipse.gef.EditPart; import org.eclipse.gef.EditPolicy; +import org.eclipse.gef.LayerConstants; import org.eclipse.gef.Request; import org.eclipse.gmf.runtime.diagram.ui.editparts.ConnectionNodeEditPart; import org.eclipse.gmf.runtime.diagram.ui.editpolicies.EditPolicyRoles; +import org.eclipse.gmf.runtime.draw2d.ui.internal.figures.ConnectionLayerEx; import org.eclipse.gmf.runtime.draw2d.ui.mapmode.IMapMode; import org.eclipse.gmf.runtime.notation.ArrowType; import org.eclipse.gmf.runtime.notation.IdentityAnchor; +import org.eclipse.gmf.runtime.notation.NotationPackage; +import org.eclipse.gmf.runtime.notation.Routing; +import org.eclipse.gmf.runtime.notation.RoutingStyle; import org.eclipse.gmf.runtime.notation.Shape; import org.eclipse.gmf.runtime.notation.View; import org.eclipse.papyrus.uml.diagram.sequence.figure.MessageFigure; import org.eclipse.papyrus.uml.diagram.sequence.figure.magnets.ConnectionFigureMagnetHelper; +import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.Activator; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies.LogicalModelElementSemanticEditPolicy; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies.MessageBendpointsEditPolicy; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies.MessageEndpointEditPolicy; +import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.locators.SelfMessageRouter; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.tools.MessageMoveTracker; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.tools.SequenceConnectionSelectionTracker; import org.eclipse.papyrus.uml.interaction.model.MInteraction; import org.eclipse.papyrus.uml.interaction.model.MLifeline; +import org.eclipse.papyrus.uml.interaction.model.spi.ViewTypes; import org.eclipse.uml2.uml.Message; import org.eclipse.uml2.uml.UMLPackage; +@SuppressWarnings("restriction") public class MessageEditPart extends ConnectionNodeEditPart implements ISequenceEditPart { private final EventBus bus = new EventBus(); private ConnectionFigureMagnetHelper magnetHelper; + private SelfMessageRouter selfMessageRouter; + public MessageEditPart(View view) { super(view); } @@ -93,6 +107,11 @@ protected void handleNotificationEvent(Notification notification) { refreshLifelineIfHeightOrPositionHasChanged(notification); } + @Override + protected void refreshBendpoints() { + // nothing to refresh, as bendpoints are not used + } + protected void updateArrowDecoration() { updateArrowDecoration(getMessageFigure()); } @@ -224,6 +243,43 @@ protected void refreshRouterChange() { // No bendpoints in messages to refresh } + @Override + protected void handlePropertyChangeEvent(PropertyChangeEvent event) { + + } + + @Override + protected void installRouter() { + ConnectionLayer cLayer = (ConnectionLayer)getLayer(LayerConstants.CONNECTION_LAYER); + RoutingStyle style = (RoutingStyle)((View)getModel()) + .getStyle(NotationPackage.Literals.ROUTING_STYLE); + + if (style != null && cLayer instanceof ConnectionLayerEx) { + + ConnectionLayerEx cLayerEx = (ConnectionLayerEx)cLayer; + Routing routing = style.getRouting(); + if (Routing.MANUAL_LITERAL == routing) { + getConnectionFigure().setConnectionRouter(cLayerEx.getObliqueRouter()); + } else if (Routing.RECTILINEAR_LITERAL == routing) { + getConnectionFigure().setConnectionRouter(getSelfMessageRouter()); + } else if (Routing.TREE_LITERAL == routing) { + getConnectionFigure().setConnectionRouter(cLayerEx.getTreeRouter()); + } + + } + + refreshRouterChange(); + } + + private ConnectionRouter getSelfMessageRouter() { + if (selfMessageRouter == null) { + int minimumWidth = Activator.getDefault().getLayoutConstraints(getEditingDomain()) + .getMinimumWidth(ViewTypes.MESSAGE); + selfMessageRouter = new SelfMessageRouter(minimumWidth); + } + return selfMessageRouter; + } + @Override protected void refreshRoutingStyles() { // Routing styles are not supported because we do not support bendpoints diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/AbstractSequenceGraphicalNodeEditPolicy.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/AbstractSequenceGraphicalNodeEditPolicy.java index 82023992..957dd461 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/AbstractSequenceGraphicalNodeEditPolicy.java +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/AbstractSequenceGraphicalNodeEditPolicy.java @@ -37,10 +37,14 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import org.eclipse.draw2d.ConnectionLayer; +import org.eclipse.draw2d.ConnectionRouter; import org.eclipse.draw2d.geometry.Point; import org.eclipse.emf.common.command.CommandWrapper; import org.eclipse.emf.ecore.EClass; import org.eclipse.emf.transaction.TransactionalEditingDomain; +import org.eclipse.gef.EditPart; +import org.eclipse.gef.LayerConstants; import org.eclipse.gef.Request; import org.eclipse.gef.commands.Command; import org.eclipse.gef.commands.UnexecutableCommand; @@ -51,8 +55,10 @@ import org.eclipse.gmf.runtime.diagram.ui.editparts.IGraphicalEditPart; import org.eclipse.gmf.runtime.diagram.ui.editpolicies.GraphicalNodeEditPolicy; import org.eclipse.gmf.runtime.diagram.ui.requests.CreateConnectionViewRequest; +import org.eclipse.gmf.runtime.draw2d.ui.internal.figures.ConnectionLayerEx; import org.eclipse.gmf.runtime.emf.type.core.IElementType; import org.eclipse.gmf.runtime.notation.Connector; +import org.eclipse.gmf.runtime.notation.Routing; import org.eclipse.gmf.runtime.notation.View; import org.eclipse.papyrus.infra.emf.gmf.command.GMFtoEMFCommandWrapper; import org.eclipse.papyrus.infra.gmfdiag.common.commands.SelectAndExecuteCommand; @@ -61,6 +67,7 @@ import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.Messages; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.parts.ISequenceEditPart; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies.MessageFeedbackHelper.Mode; +import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.locators.SelfMessageRouter; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.util.WeakEventBusDelegator; import org.eclipse.papyrus.uml.diagram.sequence.runtime.util.CreateRequestSwitch; import org.eclipse.papyrus.uml.diagram.sequence.runtime.util.MessageUtil; @@ -91,10 +98,13 @@ * * @author Christian W. Damus */ +@SuppressWarnings("restriction") public abstract class AbstractSequenceGraphicalNodeEditPolicy extends GraphicalNodeEditPolicy implements ISequenceEditPolicy { private final EventBus bus = new EventBus(); + private SelfMessageRouter selfMessageRouter; + /** * Initializes me. */ @@ -276,7 +286,7 @@ public Command caseCreateConnectionViewRequest(CreateConnectionViewRequest reque otherMagnet.map(IMagnet::getLocation).ifPresent(startLocation::setLocation); // Enforce a horizontal layout (subject to magnet constraints, with the - // target magnet having precendence) + // target magnet having precedence) if (startLocation.y() != absoluteY || otherMagnet.isPresent()) { if (!otherMagnet.isPresent()) { // Update the source to match the target @@ -387,21 +397,70 @@ protected FeedbackHelper getFeedbackHelper(CreateConnectionRequest request) { Point p = request.getLocation(); connectionFeedback = createDummyConnection(request); - connectionFeedback.setConnectionRouter(getDummyConnectionRouter(request)); + updateFeedbackRouter(request); connectionFeedback.setSourceAnchor(getSourceConnectionAnchor(request)); feedbackHelper.setConnection(connectionFeedback); addFeedback(connectionFeedback); feedbackHelper.update(null, p); + } else { + // only a potential update on the router (rectilinear to oblique and vice versa) + Point p = request.getLocation(); + updateFeedbackRouter(request); + feedbackHelper.update(null, p); } return feedbackHelper; } + private void updateFeedbackRouter(CreateConnectionRequest request) { + ConnectionRouter dummyConnectionRouter = getDummyConnectionRouter(request); + connectionFeedback.setConnectionRouter(dummyConnectionRouter); + + } + @Override protected void showCreationFeedback(CreateConnectionRequest request) { super.showCreationFeedback(request); } + @Override + protected ConnectionRouter getDummyConnectionRouter(CreateConnectionRequest ccr) { + // return a rectilinear router for connexion on same element, and an oblique one when source and + // target are different + EditPart source = ccr.getSourceEditPart(); + EditPart target = ccr.getTargetEditPart(); + Routing routingVal = Routing.MANUAL_LITERAL; + + if (source == target) { + routingVal = Routing.RECTILINEAR_LITERAL; + } else { + // TODO check for source or target contained in the parent list of the other ? + // will only implement oblique there currently + + } + + ConnectionLayer cLayer = (ConnectionLayer)getLayer(LayerConstants.CONNECTION_LAYER); + if (cLayer instanceof ConnectionLayerEx) { + ConnectionLayerEx cLayerEx = (ConnectionLayerEx)cLayer; + if (routingVal == Routing.MANUAL_LITERAL) { + return cLayerEx.getObliqueRouter(); + } else if (routingVal == Routing.RECTILINEAR_LITERAL) { + return getSelfMessageRouter(); + } else if (routingVal == Routing.TREE_LITERAL) { + return cLayerEx.getTreeRouter(); + } + } + return super.getDummyConnectionRouter(ccr); + } + + private ConnectionRouter getSelfMessageRouter() { + if (selfMessageRouter == null) { + int minimumWidth = getLayoutConstraints().getMinimumWidth(ViewTypes.MESSAGE); + selfMessageRouter = new SelfMessageRouter(minimumWidth); + } + return selfMessageRouter; + } + @Override protected Command getReconnectSourceCommand(ReconnectRequest request) { ConnectionEditPart connectionEP = (ConnectionEditPart)request.getConnectionEditPart(); @@ -451,7 +510,9 @@ protected boolean isAttachingToOtherLifeline(MMessageEnd end) { protected Optional constrainReconnection(ReconnectRequest request, MMessageEnd end) { - + // Disallow semantic reordering below the next element on the lifeline. + // But if this is a self-message, then don't worry about crossing over the + // other end, because it's also moving Optional lifeline = getLifeline(); // The lifeline under the pointer boolean selfMessage = end.getOtherEnd().flatMap(MMessageEnd::getCovered).equals(lifeline); boolean wasSelfMessage = end.getOtherEnd().flatMap(MMessageEnd::getCovered).equals(end.getCovered()); diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/LifelineBodyDropEditPolicy.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/LifelineBodyDropEditPolicy.java index 7ec7d976..d8710fa5 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/LifelineBodyDropEditPolicy.java +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/LifelineBodyDropEditPolicy.java @@ -30,7 +30,9 @@ import org.eclipse.gmf.runtime.diagram.ui.requests.DropObjectsRequest; import org.eclipse.papyrus.uml.interaction.model.MExecution; import org.eclipse.papyrus.uml.interaction.model.MLifeline; +import org.eclipse.papyrus.uml.interaction.model.MOccurrence; import org.eclipse.papyrus.uml.interaction.model.util.Optionals; +import org.eclipse.uml2.uml.Element; import org.eclipse.uml2.uml.ExecutionSpecification; import org.eclipse.uml2.uml.Lifeline; @@ -83,6 +85,16 @@ protected Command getDropExecutionCommand(MExecution execution, DropObjectsReque OptionalInt top = flatMap(execution.getTop(), t -> map(deltaY, y -> t + y)); OptionalInt bottom = flatMap(execution.getBottom(), b -> map(deltaY, y -> b + y)); + // check if there is a change of lifeline, in this case, opt for the start event cover change + // this allows for nice handling of self messages. + if (lifeline.isPresent() && !lifeline.get().equals(execution.getOwner())) { + + Optional> start = execution.getStart(); + if (start.isPresent()) { + return lifeline.map(ll -> wrap(start.get().setCovered(ll, top))).orElse(null); + } + } + // default behavior, use the set owner API. return lifeline.map(ll -> wrap(execution.setOwner(ll, top, bottom))).orElse(null); } diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/MessageEndpointEditPolicy.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/MessageEndpointEditPolicy.java index 0e51f0f4..3b49c92d 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/MessageEndpointEditPolicy.java +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/MessageEndpointEditPolicy.java @@ -34,10 +34,13 @@ import java.util.OptionalInt; import org.eclipse.draw2d.ConnectionAnchor; +import org.eclipse.draw2d.ConnectionLayer; import org.eclipse.draw2d.ConnectionLocator; +import org.eclipse.draw2d.ConnectionRouter; import org.eclipse.draw2d.geometry.Point; import org.eclipse.emf.ecore.EObject; import org.eclipse.gef.EditPart; +import org.eclipse.gef.LayerConstants; import org.eclipse.gef.Request; import org.eclipse.gef.commands.Command; import org.eclipse.gef.commands.CompoundCommand; @@ -49,11 +52,14 @@ import org.eclipse.gef.requests.ReconnectRequest; import org.eclipse.gmf.runtime.diagram.ui.editparts.ConnectionEditPart; import org.eclipse.gmf.runtime.diagram.ui.editparts.IGraphicalEditPart; +import org.eclipse.gmf.runtime.draw2d.ui.internal.figures.ConnectionLayerEx; import org.eclipse.gmf.runtime.notation.Node; +import org.eclipse.gmf.runtime.notation.Routing; import org.eclipse.papyrus.uml.diagram.sequence.figure.magnets.IMagnet; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.parts.LifelineHeaderEditPart; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies.MessageFeedbackHelper.Mode; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.handles.SequenceConnectionEndpointHandle; +import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.locators.SelfMessageRouter; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.util.CommandCreatedEvent; import org.eclipse.papyrus.uml.diagram.sequence.runtime.util.MessageUtil; import org.eclipse.papyrus.uml.interaction.model.MDestruction; @@ -66,6 +72,7 @@ /** * Endpoint edit-policy for management of message ends. */ +@SuppressWarnings("restriction") public class MessageEndpointEditPolicy extends ConnectionEndpointEditPolicy implements ISequenceEditPolicy { private final EventBus bus; @@ -78,8 +85,14 @@ public class MessageEndpointEditPolicy extends ConnectionEndpointEditPolicy impl private ConnectionAnchor originalTarget; + private ConnectionRouter originalRouter; + private MessageFeedbackHelper feedbackHelper; + private SelfMessageRouter selfMessageRouter; + + private Object originalConstraint; + /** * Initializes me with my event bus. * @@ -121,11 +134,72 @@ protected FeedbackHelper getFeedbackHelper(ReconnectRequest request) { boolean source = request.isMovingStartAnchor(); feedbackHelper = new MessageFeedbackHelper(source ? Mode.MOVE_SOURCE : Mode.MOVE_TARGET, synch, getMagnetManager(), getLayoutConstraints()); + updateFeedbackRouter(request); feedbackHelper.setConnection(getConnection()); + } else { + // only a potential update on the router (rectilinear to oblique and vice versa) + Point p = request.getLocation(); + updateFeedbackRouter(request); + feedbackHelper.update(null, p); + } return feedbackHelper; } + private void updateFeedbackRouter(ReconnectRequest request) { + ConnectionRouter connectionRouter = getConnectionRouter(request); + getConnection().setConnectionRouter(connectionRouter); + + } + + protected ConnectionRouter getConnectionRouter(ReconnectRequest rr) { + // return a rectilinear router for connexion on same element, and an oblique one when source and + // target are different + EditPart source; + EditPart target; + if (rr.isMovingStartAnchor()) { + source = rr.getTarget(); + target = rr.getConnectionEditPart().getTarget(); + } else { + source = rr.getConnectionEditPart().getSource(); + target = rr.getTarget(); + } + + return getConnectionRouter(source, target); + } + + protected ConnectionRouter getConnectionRouter(EditPart source, EditPart target) { + + Routing routingVal = Routing.MANUAL_LITERAL; + + if (source == target) { + routingVal = Routing.RECTILINEAR_LITERAL; + } else { + // TODO check for source or target contained in the parent list of the other ? + // will only implement oblique there currently + } + ConnectionLayer cLayer = (ConnectionLayer)getLayer(LayerConstants.CONNECTION_LAYER); + if (cLayer instanceof ConnectionLayerEx) { + ConnectionLayerEx cLayerEx = (ConnectionLayerEx)cLayer; + if (routingVal == Routing.MANUAL_LITERAL) { + return cLayerEx.getObliqueRouter(); + } else if (routingVal == Routing.RECTILINEAR_LITERAL) { + return getSelfMessageRouter(); + } else if (routingVal == Routing.TREE_LITERAL) { + return cLayerEx.getTreeRouter(); + } + } + return cLayer.getConnectionRouter(); + } + + private ConnectionRouter getSelfMessageRouter() { + if (selfMessageRouter == null) { + int minimumWidth = getLayoutConstraints().getMinimumWidth(ViewTypes.MESSAGE); + selfMessageRouter = new SelfMessageRouter(minimumWidth); + } + return selfMessageRouter; + } + @Override public void showSourceFeedback(Request request) { if (REQ_CREATE_BENDPOINT.equals(request.getType())) { @@ -149,6 +223,25 @@ protected void eraseConnectionMoveFeedback(ReconnectRequest request) { super.eraseConnectionMoveFeedback(request); lastMouseLocation = null; feedbackHelper = null; + + getConnection().setConnectionRouter(originalRouter); + getConnection().setRoutingConstraint(originalConstraint); + + originalRouter = null; + originalConstraint = null; + } + + @Override + protected void showConnectionMoveFeedback(ReconnectRequest request) { + if (originalRouter == null) { + originalRouter = getConnection().getConnectionRouter(); + if (originalConstraint == null) { + // This one may be null, so this is saved only once with the original router. + originalConstraint = getConnection().getRoutingConstraint(); + } + } + + super.showConnectionMoveFeedback(request); } protected void showConnectionMoveFeedback(BendpointRequest request) { @@ -166,6 +259,14 @@ protected void showConnectionMoveFeedback(BendpointRequest request) { } setUpdatedTargetLocation(request, getLocation(getConnection().getTargetAnchor())); + if (originalRouter == null) { + originalRouter = getConnection().getConnectionRouter(); + if (originalConstraint == null) { + // This one may be null, so this is saved only once with the original router. + originalConstraint = getConnection().getRoutingConstraint(); + } + } + FeedbackHelper helper = getFeedbackHelper(request); helper.setMovingStartAnchor(false); helper.update(getConnection().getTargetAnchor(), request.getLocation()); @@ -190,10 +291,14 @@ protected void eraseConnectionMoveFeedback(BendpointRequest request) { getConnection().setSourceAnchor(originalSource); getConnection().setTargetAnchor(originalTarget); + getConnection().setConnectionRouter(originalRouter); + getConnection().setRoutingConstraint(originalConstraint); lastMouseLocation = null; originalSource = null; originalTarget = null; + originalRouter = null; + originalConstraint = null; feedbackHelper = null; } diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/locators/SelfMessageRouter.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/locators/SelfMessageRouter.java new file mode 100644 index 00000000..19e511c8 --- /dev/null +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/locators/SelfMessageRouter.java @@ -0,0 +1,66 @@ +/***************************************************************************** + * Copyright (c) 2018 EclipseSource 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 + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * EclipseSource - Initial API and implementation + * + *****************************************************************************/ + +package org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.locators; + +import org.eclipse.draw2d.Connection; +import org.eclipse.draw2d.ConnectionAnchor; +import org.eclipse.draw2d.geometry.Point; +import org.eclipse.draw2d.geometry.PointList; +import org.eclipse.gmf.runtime.draw2d.ui.internal.routers.ObliqueRouter; +import org.eclipse.gmf.runtime.draw2d.ui.internal.routers.OrthogonalRouter; + +/** + * Specific router used for + */ +@SuppressWarnings("restriction") +public class SelfMessageRouter extends ObliqueRouter implements OrthogonalRouter { + + private int space; + + public SelfMessageRouter(int space) { + this.space = space; + } + + @Override + protected PointList calculateBendPoints(Connection conn) { + ConnectionAnchor source = conn.getSourceAnchor(); + ConnectionAnchor target = conn.getTargetAnchor(); + + PointList points = new PointList(4); + + if (source == null || target == null) { + return points; + } + + Point sourceLoc = source.getLocation(source.getReferencePoint()); + Point targetLoc = target.getLocation(target.getReferencePoint()); + + // ensure the segment stays vertical and not oblique. + int sourceX = sourceLoc.x(); + int targetX = targetLoc.x(); + int vSegmentX = Math.min(sourceX + space(), targetX + space()); + + points.addPoint(sourceLoc); + points.addPoint(new Point(vSegmentX, sourceLoc.y())); + points.addPoint(new Point(vSegmentX, targetLoc.y())); + points.addPoint(targetLoc); + + return points; + } + + private int space() { + return space; + } + +} diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/util/MessageUtil.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/util/MessageUtil.java index 8c983e0d..37983ae5 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/util/MessageUtil.java +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/util/MessageUtil.java @@ -22,12 +22,15 @@ import java.util.function.Predicate; import org.eclipse.core.runtime.IAdaptable; +import org.eclipse.gef.ConnectionEditPart; import org.eclipse.gef.requests.CreateConnectionRequest; import org.eclipse.gmf.runtime.diagram.ui.requests.CreateConnectionViewRequest; import org.eclipse.gmf.runtime.diagram.ui.requests.CreateUnspecifiedTypeConnectionRequest; import org.eclipse.gmf.runtime.emf.type.core.IElementType; import org.eclipse.gmf.runtime.emf.type.core.IHintedType; +import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.parts.MessageEditPart; import org.eclipse.papyrus.uml.interaction.graph.util.CrossReferenceUtil; +import org.eclipse.papyrus.uml.interaction.model.util.Optionals; import org.eclipse.papyrus.uml.service.types.element.UMLElementTypes; import org.eclipse.papyrus.uml.service.types.utils.ElementUtil; import org.eclipse.uml2.uml.ExecutionSpecification; @@ -124,6 +127,19 @@ public static boolean isSynchronousMessage(IElementType messageType) { return isSynchronous(getSort(messageType)); } + @SuppressWarnings("boxing") + public static boolean isSynchronousMessage(ConnectionEditPart editPart) { + Optional connection = Optionals.as(Optional.ofNullable(editPart), + MessageEditPart.class); + + return connection.map(MessageEditPart::resolveSemanticElement)// + .filter(Message.class::isInstance)// + .map(Message.class::cast) // + .map(Message::getMessageSort) // + .map(MessageUtil::isSynchronous) // + .orElse(false); + } + public static boolean isSynchronous(MessageSort messageSort) { // Is it *not* an asynchronous message type? switch (messageSort) { diff --git a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/InsertMessageCommand.java b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/InsertMessageCommand.java index d23cb360..3b2977fe 100644 --- a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/InsertMessageCommand.java +++ b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/InsertMessageCommand.java @@ -599,8 +599,7 @@ private Command createAdditionalSyncMessageCommands(Supplier messageC if (isSelfMessage()) { actualReceiverY = () -> senderY.getAsInt() + layoutConstraints().getMinimumHeight(ViewTypes.MESSAGE); - actualSenderY = () -> senderY.getAsInt() - + layoutConstraints().getMinimumHeight(ViewTypes.MESSAGE); + actualSenderY = () -> senderY.getAsInt(); } else { actualReceiverY = receiverY; actualSenderY = senderY; @@ -629,7 +628,11 @@ private Command createAdditionalSyncMessageCommands(Supplier messageC if (executionCreationParameter.isCreateReply()) { additionalCommands.addAll(createAutomaticReplyMessageCommands(sendEvent, execution, - executionShape, actualSenderY, actualReceiverY)); + executionShape, () -> actualReceiverY.getAsInt() + execMinHeight, + isSelfMessage() + ? () -> actualReceiverY.getAsInt() + execMinHeight + + layoutConstraints().getMinimumHeight(ViewTypes.MESSAGE) + : () -> actualReceiverY.getAsInt() + execMinHeight)); } else { CreationCommand finish = semanticHelper().createFinish(execution, CreationParameters.after(execution)); @@ -676,8 +679,7 @@ private List createAutomaticReplyMessageCommands(Supplier s int execMinHeight = layoutConstraints().getMinimumHeight(ViewTypes.EXECUTION_SPECIFICATION); Command replyMessageView = diagramHelper().createMessageConnector(replyMessage, executionShape, - () -> senderY.getAsInt() + execMinHeight, () -> receiverShape, - () -> receiverY.getAsInt() + execMinHeight, null); + () -> senderY.getAsInt(), () -> receiverShape, () -> receiverY.getAsInt(), null); commands.add(replyMessageView); return commands; diff --git a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/PendingChildData.java b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/PendingChildData.java index 198b47f0..c7eee7a4 100644 --- a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/PendingChildData.java +++ b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/PendingChildData.java @@ -12,11 +12,15 @@ package org.eclipse.papyrus.uml.interaction.internal.model.commands; +import static org.eclipse.papyrus.uml.interaction.model.util.LogicalModelPredicates.equalTo; + import java.util.List; import java.util.Optional; +import java.util.function.Predicate; import org.eclipse.papyrus.uml.interaction.model.MElement; import org.eclipse.papyrus.uml.interaction.model.MExecution; +import org.eclipse.papyrus.uml.interaction.model.MLifeline; import org.eclipse.uml2.uml.Element; /** @@ -84,4 +88,11 @@ static void setPendingChild(MElement owner, MElement> Predicate changesContainer(MLifeline owner) { + return self -> { + Optional> pendingParent = getPendingOwner(self); + return pendingParent.filter(equalTo(owner)).isPresent(); + }; + } + } diff --git a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/SetCoveredCommand.java b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/SetCoveredCommand.java index 4256f09a..67e29189 100644 --- a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/SetCoveredCommand.java +++ b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/SetCoveredCommand.java @@ -48,6 +48,7 @@ import org.eclipse.papyrus.uml.interaction.model.MMessage; import org.eclipse.papyrus.uml.interaction.model.MMessageEnd; import org.eclipse.papyrus.uml.interaction.model.MOccurrence; +import org.eclipse.papyrus.uml.interaction.model.spi.ViewTypes; import org.eclipse.papyrus.uml.interaction.model.util.Lifelines; import org.eclipse.papyrus.uml.interaction.model.util.Optionals; import org.eclipse.uml2.uml.Classifier; @@ -136,6 +137,25 @@ protected Command doCreateCommand() { int newYPosition = yPosition.orElseGet(() -> getTarget().getTop().getAsInt()); + // enhance new position in case of message being transformed into a self message or back to a non self + // message + if (isTargetEnd()) { + if (isBecomingSelfMessage()) { + newYPosition += layoutHelper().getConstraints().getMinimumHeight(ViewTypes.MESSAGE); + } else if (isBecomingNonSelfMessage()) { + // retrieve source end position + Optional sourceEnd = Optionals.as(Optional.of(getTarget()), MMessageEnd.class) + .flatMap(MMessageEnd::getOtherEnd); + Optional message = Optionals.as(Optional.of(getTarget()), MMessageEnd.class) + .map(MMessageEnd::getOwner); + if (message.isPresent() && message.get().isSynchronous()) { + newYPosition = sourceEnd.map(MMessageEnd::getTop).orElse(yPosition).getAsInt(); + } else { + newYPosition = yPosition.getAsInt(); + } + } + } + // Can't put an interaction fragment on a lifeline that is already destroyed if (!existsAt(lifeline, newYPosition)) { return bomb(); @@ -184,7 +204,7 @@ && getTarget().getExecution().filter(splitsExecution(lifeline)).isPresent()) { result = bomb(); } else { // Handle a dependent execution - result = dependencies(getTarget()).map(chaining(result)).orElse(result); + result = dependencies(getTarget(), newYPosition).map(chaining(result)).orElse(result); // Handle message details if (getTarget() instanceof MMessageEnd) { @@ -197,7 +217,36 @@ && getTarget().getExecution().filter(splitsExecution(lifeline)).isPresent()) { return result; } - protected Optional dependencies(MOccurrence occurrence) { + private boolean isTargetEnd() { + Optional end = Optionals.as(Optional.of(getTarget()), MMessageEnd.class); + Optional receive = end.map(MMessageEnd::getOwner).flatMap(MMessage::getReceive); + return end.isPresent() && end.equals(receive); + } + + protected boolean isBecomingSelfMessage() { + return isChangingLifeline() && willBeSelfMessage() && !wasSelfMessage(); + } + + protected boolean isBecomingNonSelfMessage() { + return isChangingPosition() && wasSelfMessage() && !willBeSelfMessage(); + } + + protected boolean wasSelfMessage() { + Optional otherLifeline = Optionals.as(Optional.of(getTarget()), MMessageEnd.class) + .flatMap(MMessageEnd::getOtherEnd).flatMap(MMessageEnd::getCovered); + Optional endlifeline = Optionals.as(Optional.of(getTarget()), MMessageEnd.class) + .flatMap(MMessageEnd::getCovered); + return endlifeline.isPresent() && endlifeline.equals(otherLifeline); + } + + protected boolean willBeSelfMessage() { + Optional otherEnd = Optionals.as(Optional.of(getTarget()), MMessageEnd.class) + .flatMap(MMessageEnd::getOtherEnd); + return (otherEnd.flatMap(MMessageEnd::getCovered).map(MLifeline::getElement).orElse(null) == lifeline + .getElement()); + } + + protected Optional dependencies(MOccurrence occurrence, int newYPosition) { List result = new ArrayList<>(); Consumer commandSink = cmd -> Optional.ofNullable(cmd).ifPresent(result::add); @@ -214,11 +263,11 @@ protected Optional dependencies(MOccurrence occurren if (!isThisMessageEndBeingDisconnected) { // Either we aren't disconnecting the message end, or it's not a message // end that we're moving - occurrence.getStartedExecution() - .map(exec -> exec.setOwner(lifeline, yPosition, OptionalInt.empty())) + occurrence.getStartedExecution().filter(exec -> !hasDependency(exec, SetOwnerCommand.class)) + .map(exec -> exec.setOwner(lifeline, OptionalInt.of(newYPosition), OptionalInt.empty())) .ifPresent(result::add); - occurrence.getFinishedExecution() - .map(exec -> exec.setOwner(lifeline, OptionalInt.empty(), yPosition)) + occurrence.getFinishedExecution().filter(exec -> !hasDependency(exec, SetOwnerCommand.class)) + .map(exec -> exec.setOwner(lifeline, OptionalInt.empty(), OptionalInt.of(newYPosition))) .ifPresent(result::add); } @@ -226,12 +275,12 @@ protected Optional dependencies(MOccurrence occurren if ((occurrence instanceof MMessageEnd) && !(occurrence instanceof MDestruction)) { MMessageEnd end = (MMessageEnd)occurrence; if (isChangingLifeline() || isChangingPosition()) { - collectMessageDependencies(end, isDisconnectingMessageEnd, commandSink); + collectMessageDependencies(end, isDisconnectingMessageEnd, newYPosition, commandSink); } } else if (occurrence instanceof MExecutionOccurrence) { MExecutionOccurrence execOcc = (MExecutionOccurrence)occurrence; if (isChangingLifeline() || isChangingPosition()) { - collectExecutionOccurrenceDependencies(execOcc, commandSink); + collectExecutionOccurrenceDependencies(execOcc, newYPosition, commandSink); } } @@ -246,10 +295,11 @@ protected Optional dependencies(MOccurrence occurren * @param isDisconnecting * whether the {@code end} is being disconnected from the start/finish of an execution * specification + * @param newYPosition * @param commandSink * accumulator of dependency commands (accepts {@code null} values) */ - private void collectMessageDependencies(MMessageEnd end, boolean isDisconnecting, + private void collectMessageDependencies(MMessageEnd end, boolean isDisconnecting, int newYPosition, Consumer commandSink) { MMessage message = end.getOwner(); @@ -261,7 +311,6 @@ private void collectMessageDependencies(MMessageEnd end, boolean isDisconnecting Connector connector = edge.get(); Shape lifelineBody = getLifelineBody().get(); - int newYPosition = yPosition.orElseGet(() -> end.getTop().getAsInt()); // Handle attached execution specification, unless we're detaching from a message end Optional replacement = isDisconnecting ? Optional.empty() @@ -286,13 +335,13 @@ private void collectMessageDependencies(MMessageEnd end, boolean isDisconnecting commandSink.accept(defer( () -> reconnectSource(connector, newAttachedShape.get(), newYPosition).orElse(null))); Optional otherEnd = end.getOtherEnd(); - otherEnd.flatMap(this::handleSelfMessageChange).ifPresent(commandSink); + otherEnd.flatMap(oEnd -> handleSelfMessageChange(oEnd, newYPosition)).ifPresent(commandSink); otherEnd.flatMap(this::handleOppositeSendOrReplyMessage).ifPresent(commandSink); } else if (end.isReceive()) { commandSink.accept(defer( () -> reconnectTarget(connector, newAttachedShape.get(), newYPosition).orElse(null))); Optional otherEnd = end.getOtherEnd(); - otherEnd.flatMap(this::handleSelfMessageChange).ifPresent(commandSink); + otherEnd.flatMap(oEnd -> handleSelfMessageChange(oEnd, newYPosition)).ifPresent(commandSink); otherEnd.flatMap(this::handleOppositeSendOrReplyMessage).ifPresent(commandSink); } // else don't know what to do with it } @@ -320,15 +369,62 @@ private void collectMessageDependencies(MMessageEnd end, boolean isDisconnecting map(exec.getBottom(), b -> b + deltaY))).ifPresent(commandSink); } else { Optional otherCovered = other.flatMap(MMessageEnd::getCovered); - OptionalInt otherNewY = map(otherY, y -> y + deltaY); // Track this end + final OptionalInt otherNewY; + if (end.isStart()) { + if (wasSelfMessage() && willBeSelfMessage()) { + otherNewY = OptionalInt.of(PendingVerticalExtentData + .getPendingTop(end.getExecution().get()).getAsInt() + - layoutHelper().getConstraints().getMinimumHeight(ViewTypes.MESSAGE)); + } else { + otherNewY = PendingVerticalExtentData.getPendingTop(end.getExecution().get()); + } + + } else if (end.isFinish()) { + OptionalInt tmp = PendingVerticalExtentData + .getPendingBottom(end.getExecution().get()); + if (otherEnd.isReceive()) { + // add an additional room for the self message + if (willBeSelfMessage()) { + tmp = OptionalInt.of(tmp.getAsInt() + layoutHelper().getConstraints() + .getMinimumHeight(ViewTypes.MESSAGE)); + } + } + otherNewY = tmp; + } else { + OptionalInt tmp = yPosition; + // only a simple occurrence on the execution. Move using delta + if (otherEnd.isReceive()) { + // add an additional room for the self message + if (isBecomingSelfMessage() && !wasSelfMessage()) { + tmp = OptionalInt.of(tmp.getAsInt() + layoutHelper().getConstraints() + .getMinimumHeight(ViewTypes.MESSAGE)); + } + } + otherNewY = tmp; + } otherCovered.map(ll -> otherEnd.setCovered(ll, otherNewY)).ifPresent(commandSink); } } else if (shouldTrackOppositeEnd(end)) { Optional otherCovered = otherEnd.getCovered(); // Track this end otherCovered.map(ll -> otherEnd.setCovered(ll, yPosition)).ifPresent(commandSink); + } else { + Optional otherCovered = other.flatMap(MMessageEnd::getCovered); + // only a simple occurrence on the execution. Move using delta + if (otherEnd.isReceive()) { + // add an additional room for the self message + if (isBecomingSelfMessage() && !wasSelfMessage()) { + // ensure the position has a least the space for self messages, but it could be bigger + int minLayout = yPosition.getAsInt() + + layoutHelper().getConstraints().getMinimumHeight(ViewTypes.MESSAGE); + int currentOtherPos = otherEnd.getTop().getAsInt(); + OptionalInt otherNewY = OptionalInt.of(Math.max(minLayout, currentOtherPos)); + otherCovered.map(ll -> otherEnd.setCovered(ll, otherNewY)).ifPresent(commandSink); + } + } + } } } @@ -419,7 +515,7 @@ private Optional getFinishMessage(Optional execution) { .map(MMessageEnd.class::cast).map(MMessageEnd::getOwner); } - Optional handleSelfMessageChange(MMessageEnd otherEnd) { + Optional handleSelfMessageChange(MMessageEnd otherEnd, int newYPosition) { Optional result; // If the message end opposite one being moved to this lifeline is already on that lifeline, @@ -448,14 +544,14 @@ Optional handleSelfMessageChange(MMessageEnd otherEnd) { Command bend; if (otherEnd.isSend()) { // We're reconnecting the source end, but we need to maintain the gap - int newYPosition = currentYPosition - height; - Shape newAttachShape = executionShapeAt(lifeline, newYPosition).orElse(lifelineBody); + // int newYPosition = currentYPosition - height; + Shape newAttachShape = executionShapeAt(lifeline, currentYPosition).orElse(lifelineBody); bend = reconnectSource(connector, newAttachShape, currentYPosition) .orElse(IdentityCommand.INSTANCE); } else { - int newYPosition = currentYPosition + height; - Shape newAttachShape = executionShapeAt(lifeline, newYPosition).orElse(lifelineBody); - bend = reconnectTarget(connector, newAttachShape, newYPosition) + int newReceivePosition = Math.max(newYPosition, currentYPosition); + Shape newAttachShape = executionShapeAt(lifeline, newReceivePosition).orElse(lifelineBody); + bend = reconnectTarget(connector, newAttachShape, newReceivePosition) .orElse(IdentityCommand.INSTANCE); } @@ -478,7 +574,7 @@ Optional handleSelfMessageChange(MMessageEnd otherEnd) { * @param commandSink * accumulator of dependency commands (accepts {@code null} values) */ - private void collectExecutionOccurrenceDependencies(MExecutionOccurrence occurrence, + private void collectExecutionOccurrenceDependencies(MExecutionOccurrence occurrence, int newYPosition, Consumer commandSink) { boolean shouldReplace = (occurrence.isStart() || occurrence.isFinish()) @@ -486,7 +582,7 @@ private void collectExecutionOccurrenceDependencies(MExecutionOccurrence occurre if (shouldReplace) { // Replace it by a message end? But not if we exist because of separation // of a message end from the execution in the first place - Optional end = getMessageEnd(yPosition); + Optional end = getMessageEnd(OptionalInt.of(newYPosition)); end.map(occurrence::replaceBy).ifPresent(commandSink); } } diff --git a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/SetOwnerCommand.java b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/SetOwnerCommand.java index b2a75420..33be053d 100644 --- a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/SetOwnerCommand.java +++ b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/SetOwnerCommand.java @@ -34,7 +34,11 @@ import org.eclipse.papyrus.uml.interaction.model.MElement; import org.eclipse.papyrus.uml.interaction.model.MExecution; import org.eclipse.papyrus.uml.interaction.model.MLifeline; +import org.eclipse.papyrus.uml.interaction.model.MMessage; +import org.eclipse.papyrus.uml.interaction.model.MMessageEnd; import org.eclipse.papyrus.uml.interaction.model.MOccurrence; +import org.eclipse.papyrus.uml.interaction.model.spi.ViewTypes; +import org.eclipse.papyrus.uml.interaction.model.util.Executions; import org.eclipse.papyrus.uml.interaction.model.util.Lifelines; import org.eclipse.papyrus.uml.interaction.model.util.Optionals; import org.eclipse.papyrus.uml.interaction.model.util.SequenceDiagramSwitch; @@ -49,9 +53,9 @@ public class SetOwnerCommand extends ModelCommandWithDependencies newOwner; - private final OptionalInt top; + private OptionalInt top; - private final OptionalInt bottom; + private OptionalInt bottom; // The element on the lifeline that we may need to nudge, if the new owner is a lifeline private final Optional> nudgeElement; @@ -79,10 +83,65 @@ public SetOwnerCommand(MElementImpl element, MElement element.getBottom().orElse(0)))); + // ensure message starting the execution would still have some room + fixLocation(); + // Publish this ownership change in the dependency context for other commands to find PendingChildData.setPendingChild(newOwner, element); // And vertical extent change - PendingVerticalExtentData.setPendingVerticalExtent(element, top, bottom); + PendingVerticalExtentData.setPendingVerticalExtent(element, this.top, this.bottom); + } + + private void fixLocation() { + MElement target = getTarget(); + if (MExecution.class.isInstance(target)) { + MExecution execution = MExecution.class.cast(target); + boolean wasSelfMessage = wasStartingSelfMessage(execution); + boolean isSelfMessage = isStartingSelfMessage(execution); + + if (!wasSelfMessage && isSelfMessage) { + if (top.isPresent()) { + // check if there is already a setCoveredCommand on the starting message + if (!execution.getStart().filter(occ -> hasDependency(occ, SetCoveredCommand.class)) + .isPresent()) { + top = OptionalInt.of(top.getAsInt() + + layoutHelper().getConstraints().getMinimumHeight(ViewTypes.MESSAGE)); + } + } + if (bottom.isPresent()) { + if (!execution.getStart().filter(occ -> hasDependency(occ, SetCoveredCommand.class)) + .isPresent()) { + bottom = OptionalInt.of(bottom.getAsInt() + + layoutHelper().getConstraints().getMinimumHeight(ViewTypes.MESSAGE)); + } + } else { + // bottom should still be set. + // to compute it, compute original height + new top position + if (top.isPresent()) { + bottom = OptionalInt.of(top.getAsInt() + execution.getBottom().getAsInt() + - execution.getTop().getAsInt()); + } + } + } else if (wasSelfMessage && !isSelfMessage) { + if (top.isPresent()) { + if (!execution.getStart().filter(occ -> hasDependency(occ, SetCoveredCommand.class)) + .isPresent()) { + top = OptionalInt.of(top.getAsInt() + - layoutHelper().getConstraints().getMinimumHeight(ViewTypes.MESSAGE)); + } + } + if (bottom.isPresent()) { + if (!execution.getStart().filter(occ -> hasDependency(occ, SetCoveredCommand.class)) + .isPresent()) { + bottom = OptionalInt.of(bottom.getAsInt() + - layoutHelper().getConstraints().getMinimumHeight(ViewTypes.MESSAGE)); + } + } else { + bottom = OptionalInt.of(top.getAsInt() + execution.getBottom().getAsInt() + - execution.getTop().getAsInt()); + } + } + } } protected boolean isChangingOwner() { @@ -186,13 +245,61 @@ protected Optional dependencies(MExecution execution, MLifeline lifelin // that *will be* spanned do not; they only must be reattached per the step below result = execution.getOccurrences().stream() .map(occ -> occ.setCovered(lifeline, topMapping.apply(occ.getTop()))).reduce(chaining()); - } else if (isChangingPosition() && !isChangingOwner()) { - // We are reshaping it. Refresh nested executions, if necessary - result = execution.getNestedExecutions().stream() - .filter(PendingVerticalExtentData.spannedBy(execution).negate()) - .peek(nested -> setPendingNested(PendingNestedData.NO_EXECUTION, nested)) - .map(nested -> nested.setOwner(lifeline, nested.getTop(), nested.getBottom())) - .filter(Objects::nonNull).reduce(chaining()); + } else if (isChangingPosition()) { + if (!isChangingOwner()) { + // We are reshaping it. Refresh nested executions, if necessary + result = execution.getNestedExecutions().stream() + .filter(PendingVerticalExtentData.spannedBy(execution).negate()) + .peek(nested -> setPendingNested(PendingNestedData.NO_EXECUTION, nested)) + .map(nested -> nested.setOwner(lifeline, nested.getTop(), nested.getBottom())) + .filter(Objects::nonNull).reduce(chaining()); + } else { + // we are moving and changing owner. Receive event of the finish message may also be move down + // from some pixels because of becoming a self message. + int deltaTop = map(this.top, t -> t - execution.getTop().getAsInt()).orElse(0); + UnaryOperator topMapping = deltaTop == 0 ? UnaryOperator.identity() + : top_ -> map(top_, t -> t + deltaTop); + + result = execution.getOccurrences().stream() + .filter(occ -> !occ.equals(execution.getFinish().orElse(null))) + .map(occ -> occ.setCovered(lifeline, topMapping.apply(occ.getTop()))) + .filter(Objects::nonNull).reduce(chaining()); + boolean isSelfMessage = isStartingSelfMessage(execution); + boolean wasSelfMessage = wasStartingSelfMessage(execution); + if (isSelfMessage && !wasSelfMessage) { + UnaryOperator topFinishMapping = deltaTop == 0 ? UnaryOperator.identity() + : top_ -> map(top_, t -> t + deltaTop + + layoutHelper().getConstraints().getMinimumHeight(ViewTypes.MESSAGE)); + final Optional last = execution.getFinish() + .map(occ -> occ.setCovered(lifeline, topFinishMapping.apply(occ.getTop()))); + if (result.isPresent()) { + if (last.isPresent()) { + result.get().chain(last.get()); + } + } else { + result = last; + } + } else if (!isSelfMessage && wasSelfMessage) { + UnaryOperator topFinishMapping = deltaTop == 0 ? UnaryOperator.identity() + : top_ -> map(top_, t -> t// + deltaTop + - layoutHelper().getConstraints().getMinimumHeight(ViewTypes.MESSAGE)); + final Optional last = execution.getFinish() + .map(occ -> occ.setCovered(lifeline, topFinishMapping.apply(occ.getTop()))); + if (result.isPresent()) { + if (last.isPresent()) { + result.get().chain(last.get()); + } + } else { + result = last; + } + } else { + UnaryOperator topFinishMapping = deltaTop == 0 ? UnaryOperator.identity() + : top_ -> map(top_, t -> t + deltaTop); + final Optional last = execution.getFinish() + .map(occ -> occ.setCovered(lifeline, topFinishMapping.apply(occ.getTop()))); + result.map(cc -> last.map(l -> cc.chain(l))).orElseGet(() -> last); + } + } } // Note that nested executions will be handled implicitly by either their start or finish. @@ -210,6 +317,24 @@ protected Optional dependencies(MExecution execution, MLifeline lifelin return result; } + private boolean isStartingSelfMessage(MExecution execution) { + Optional sendStartLL = Executions.getStartMessage(execution).flatMap(MMessage::getSend) + .flatMap(MMessageEnd::getCovered); + return as(Optional.of(newOwner), MLifeline.class).map(MElement::getElement) + .equals(sendStartLL.map(MElement::getElement)); + } + + private boolean wasStartingSelfMessage(MExecution execution) { + Optional sendStartLL = Executions.getStartMessage(execution).flatMap(MMessage::getSend) + .flatMap(MMessageEnd::getCovered); + if (!sendStartLL.isPresent()) { + return false; + } + return sendStartLL.equals(Executions.getStartMessage(execution).flatMap(MMessage::getReceive) + .flatMap(MMessageEnd::getCovered)); + + } + protected void ensurePadding() { // From which element do we need to ensure padding? MElement padFrom = getTarget(); diff --git a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/spi/impl/DefaultDiagramHelper.java b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/spi/impl/DefaultDiagramHelper.java index 18276a7e..03eae68a 100644 --- a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/spi/impl/DefaultDiagramHelper.java +++ b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/spi/impl/DefaultDiagramHelper.java @@ -14,7 +14,6 @@ import static org.eclipse.papyrus.uml.interaction.graph.util.Suppliers.compose; -import java.util.Arrays; import java.util.Collections; import java.util.Optional; import java.util.function.BiFunction; @@ -46,7 +45,6 @@ import org.eclipse.gmf.runtime.notation.Shape; import org.eclipse.gmf.runtime.notation.Size; import org.eclipse.gmf.runtime.notation.View; -import org.eclipse.gmf.runtime.notation.datatype.RelativeBendpoint; import org.eclipse.papyrus.uml.interaction.model.CreationCommand; import org.eclipse.papyrus.uml.interaction.model.CreationParameters; import org.eclipse.papyrus.uml.interaction.model.spi.DeferredCreateCommand; @@ -346,31 +344,30 @@ public CreationCommand createMessageConnector(Supplier messa : targetView); if (selfMessage) { // A self message is fixed at the minimal gap if it is of a synchronous sort - int minTargetDistance = sourceDistance - + layoutHelper().getConstraints().getMinimumHeight(result); + // warning, source distance and target distance may be in 2 different local dimensions + // so the computation has to be done in absolute coordinates then translated back to local + // dimensions + + int minTargetDistance = sourceY.getAsInt() + + layoutHelper().getConstraints().getMinimumHeight(result) + - layoutHelper() + .getTop(ViewTypes.DESTRUCTION_SPECIFICATION.equals(targetView.getType()) + ? (Shape)targetView.eContainer() // Calculate relative to the lifeline + : targetView); targetDistance = syncMessage ? minTargetDistance : Math.max(targetDistance, minTargetDistance); } anchorFactory.builderFor(targetView).from(sourceView).to(targetView).at(targetDistance) .targetEnd().build(); - // We need to have a bendpoints list, even if it's empty - RelativeBendpoints bendpoints = (RelativeBendpoints)result - .createBendpoints(NotationPackage.Literals.RELATIVE_BENDPOINTS); - // Specific routing for self-messages if (selfMessage) { result.setRouting(Routing.RECTILINEAR_LITERAL); - - int xOffset = layoutHelper().getConstraints().getMinimumWidth(result); - int yOffset = layoutHelper().getConstraints().getMinimumHeight(result); - RelativeBendpoint bp1 = new RelativeBendpoint(0, 0, 0, -yOffset); - RelativeBendpoint bp2 = new RelativeBendpoint(xOffset, 0, xOffset, -yOffset); - RelativeBendpoint bp3 = new RelativeBendpoint(xOffset, yOffset, xOffset, 0); - RelativeBendpoint bp4 = new RelativeBendpoint(0, yOffset, 0, 0); - bendpoints.setPoints(Arrays.asList(bp1, bp2, bp3, bp4)); } + RelativeBendpoints bendpoints = NotationFactory.eINSTANCE.createRelativeBendpoints(); + result.setBendpoints(bendpoints); + // create a decoration node to be seen by CSS rules DecorationNode nameLabel = (DecorationNode)result .createChild(NotationPackage.Literals.DECORATION_NODE); @@ -430,27 +427,6 @@ protected Command createCommand() { public Command configureSelfMessageConnector(Message message, Connector messageView) { Command result = SetCommand.create(editingDomain, messageView, NotationPackage.Literals.ROUTING_STYLE__ROUTING, Routing.RECTILINEAR_LITERAL); - - // Compute the target anchor - Shape source = (Shape)messageView.getSource(); - AnchorFactory anchorFactory = new AnchorFactory(messageView, layoutHelper()); - int sourceDistance = layoutHelper().getYPosition(messageView.getSourceAnchor(), source) - - layoutHelper().getTop(source); - int targetDistance = sourceDistance + layoutHelper().getConstraints().getMinimumHeight(messageView); - String id = anchorFactory.builderFor(source).from(source).to(source).at(targetDistance).targetEnd() - .computeIdentity(); - result = result.chain(SetCommand.create(editingDomain, messageView.getTargetAnchor(), - NotationPackage.Literals.IDENTITY_ANCHOR__ID, id)); - - int xOffset = layoutHelper().getConstraints().getMinimumWidth(messageView); - int yOffset = layoutHelper().getConstraints().getMinimumHeight(messageView); - RelativeBendpoint bp1 = new RelativeBendpoint(0, 0, 0, -yOffset); - RelativeBendpoint bp2 = new RelativeBendpoint(xOffset, 0, xOffset, -yOffset); - RelativeBendpoint bp3 = new RelativeBendpoint(xOffset, yOffset, xOffset, 0); - RelativeBendpoint bp4 = new RelativeBendpoint(0, yOffset, 0, 0); - result = result.chain(SetCommand.create(editingDomain, messageView.getBendpoints(), - NotationPackage.Literals.RELATIVE_BENDPOINTS__POINTS, Arrays.asList(bp1, bp2, bp3, bp4))); - return result; } @@ -458,8 +434,10 @@ public Command configureSelfMessageConnector(Message message, Connector messageV public Command configureStraightMessageConnector(Message message, Connector messageView) { Command result = SetCommand.create(editingDomain, messageView, NotationPackage.Literals.ROUTING_STYLE__ROUTING, Routing.MANUAL_LITERAL); - result = result.chain(SetCommand.create(editingDomain, messageView.getBendpoints(), - NotationPackage.Literals.RELATIVE_BENDPOINTS__POINTS, Collections.EMPTY_LIST)); + if (messageView.getBendpoints() != null) { + result = result.chain(SetCommand.create(editingDomain, messageView.getBendpoints(), + NotationPackage.Literals.RELATIVE_BENDPOINTS__POINTS, Collections.EMPTY_LIST)); + } return result; } @@ -477,6 +455,24 @@ public Command reconnectSource(Connector connector, Shape newSource, int yPositi result = result.chain(SetCommand.create(editingDomain, anchor, NotationPackage.Literals.IDENTITY_ANCHOR__ID, newID)); + // if message changes from self message to standard, or from standard to self, the routing style must + // be updated. + // compute the future value of self message + Lifeline sourceLL = getLifeline(newSource); + Lifeline targetLL = getLifeline(connector.getTarget()); + boolean selfMessage = (sourceLL != null) && (sourceLL == targetLL); + + Routing newRouting = Routing.MANUAL_LITERAL; + if (selfMessage) { + newRouting = Routing.RECTILINEAR_LITERAL; + } + + // should force? + if (newRouting != connector.getRouting()) { + result.chain(SetCommand.create(editingDomain, connector, + NotationPackage.Literals.ROUTING_STYLE__ROUTING, newRouting)); + } + return result; } @@ -506,6 +502,23 @@ public Command reconnectTarget(Connector connector, Shape newTarget, int yPositi result = result.chain(SetCommand.create(editingDomain, anchor, NotationPackage.Literals.IDENTITY_ANCHOR__ID, newID)); + // if message changes from self message to standard, or from standard to self, the routing style must + // be updated. + // compute the future value of self message + Lifeline sourceLL = getLifeline(connector.getSource()); + Lifeline targetLL = getLifeline(newTarget); + boolean selfMessage = (sourceLL != null) && (sourceLL == targetLL); + + Routing newRouting = Routing.MANUAL_LITERAL; + if (selfMessage) { + newRouting = Routing.RECTILINEAR_LITERAL; + } + // should force? + if (newRouting != connector.getRouting()) { + result.chain(SetCommand.create(editingDomain, connector, + NotationPackage.Literals.ROUTING_STYLE__ROUTING, newRouting)); + } + return result; } diff --git a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/model/util/Executions.java b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/model/util/Executions.java index 303beb54..a76face6 100644 --- a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/model/util/Executions.java +++ b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/model/util/Executions.java @@ -18,6 +18,8 @@ import org.eclipse.papyrus.uml.interaction.model.MElement; import org.eclipse.papyrus.uml.interaction.model.MExecution; import org.eclipse.papyrus.uml.interaction.model.MLifeline; +import org.eclipse.papyrus.uml.interaction.model.MMessage; +import org.eclipse.papyrus.uml.interaction.model.MMessageEnd; import org.eclipse.papyrus.uml.interaction.model.spi.ViewTypes; import org.eclipse.uml2.uml.Element; @@ -46,6 +48,16 @@ protected static Optional parentLifelineView(Optional view) { return parentLifelineView(as(view.map(EObject::eContainer), View.class)); } + public static Optional getStartMessage(MExecution execution) { + return execution.getStart().filter(MMessageEnd.class::isInstance).map(MMessageEnd.class::cast) + .map(MMessageEnd::getOwner); + } + + public static Optional getFinishMessage(MExecution execution) { + return execution.getFinish().filter(MMessageEnd.class::isInstance).map(MMessageEnd.class::cast) + .map(MMessageEnd::getOwner); + } + public static Optional executionAt(MLifeline lifeline, int absoluteY) { return executionAt(lifeline, absoluteY, alwaysFalse()); } @@ -61,8 +73,10 @@ public static Optional executionAt(MLifeline lifeline, int absoluteY .max(LogicalModelOrdering.vertically()); return elseMaybe(movingToLifeline, // Easy case - () -> lifeline.getExecutions().stream().filter(spanning) - .max(LogicalModelOrdering.vertically())); + () -> lifeline.getExecutions().stream().filter(spanning).filter(t -> { + Optional> pendingOwner = PendingChildData.getPendingOwner(t); + return !pendingOwner.isPresent() || pendingOwner.get().equals(lifeline); + }).max(LogicalModelOrdering.vertically())); } public static Optional executionShapeAt(MLifeline lifeline, int absoluteY) { diff --git a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/AbstractGraphicalEditPolicyUITest.java b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/AbstractGraphicalEditPolicyUITest.java index cbf719eb..916357e4 100644 --- a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/AbstractGraphicalEditPolicyUITest.java +++ b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/AbstractGraphicalEditPolicyUITest.java @@ -64,6 +64,9 @@ public abstract class AbstractGraphicalEditPolicyUITest { /** The width of an execution specification. */ protected static final int EXEC_WIDTH = 10; + /** The default height of an execution specification. */ + protected static final int EXEC_HEIGHT = 40; + /** Constant for the source end of a connection edit-part. */ protected static final int SOURCE_END = 0; diff --git a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/ExecutionAnchorsUITest.java b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/ExecutionAnchorsUITest.java index 422468c6..97db0bfd 100644 --- a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/ExecutionAnchorsUITest.java +++ b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/ExecutionAnchorsUITest.java @@ -135,6 +135,8 @@ public void findFixtures() { @After public void checkAfter() { editor.undo(); + editor.forceRefresh(); + editor.flushDisplayEvents(); assertEquals(request_source, getSource(requestEP)); assertEquals(request_target, getTarget(requestEP)); diff --git a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/ExecutionSpecificationGraphicalNodeEditPolicyUITest.java b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/ExecutionSpecificationGraphicalNodeEditPolicyUITest.java index 74ce5f35..808bbc10 100644 --- a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/ExecutionSpecificationGraphicalNodeEditPolicyUITest.java +++ b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/ExecutionSpecificationGraphicalNodeEditPolicyUITest.java @@ -31,19 +31,20 @@ import org.junit.runners.Parameterized.Parameters; /** - * Integration test cases for the - * {@link ExecutionSpecificationGraphicalNodeEditPolicy} class. + * Integration test cases for the {@link ExecutionSpecificationGraphicalNodeEditPolicy} class. * * @author Christian W. Damus */ -@SuppressWarnings("restriction") @ModelResource("one-exec.di") @Maximized @RunWith(Parameterized.class) public class ExecutionSpecificationGraphicalNodeEditPolicyUITest extends AbstractGraphicalEditPolicyUITest { + private static final int MESSAGE_Y = 195; + @ClassRule - public static LightweightSeqDPrefs prefs = new LightweightSeqDPrefs().dontCreateExecutionsForSyncMessages(); + public static LightweightSeqDPrefs prefs = new LightweightSeqDPrefs() + .dontCreateExecutionsForSyncMessages(); // Horizontal position of the first lifeline's body private static final int LIFELINE_1_BODY_X = 121; @@ -52,6 +53,7 @@ public class ExecutionSpecificationGraphicalNodeEditPolicyUITest extends Abstrac private static final int LIFELINE_2_BODY_X = 281; private static final boolean SENDER = true; // source = true + private static final boolean RECEIVER = !SENDER; private final int sendX; @@ -61,7 +63,8 @@ public class ExecutionSpecificationGraphicalNodeEditPolicyUITest extends Abstrac /** * Initializes me. */ - public ExecutionSpecificationGraphicalNodeEditPolicyUITest(boolean rightToLeft, String direction) { + public ExecutionSpecificationGraphicalNodeEditPolicyUITest(boolean rightToLeft, + @SuppressWarnings("unused") String direction) { super(); if (rightToLeft) { @@ -75,16 +78,18 @@ public ExecutionSpecificationGraphicalNodeEditPolicyUITest(boolean rightToLeft, @Test public void createAsyncMessage() { - EditPart messageEP = createConnection(SequenceElementTypes.Async_Message_Edge, at(sendX, 195), at(recvX, 195)); + EditPart messageEP = createConnection(SequenceElementTypes.Async_Message_Edge, at(sendX, MESSAGE_Y), + at(recvX, MESSAGE_Y)); - assertThat(messageEP, runs(x(SENDER), 195, x(RECEIVER), 195, 2)); + assertThat(messageEP, runs(x(SENDER), MESSAGE_Y, x(RECEIVER), MESSAGE_Y, 2)); } @Test public void createSyncMessage() { - EditPart messageEP = createConnection(SequenceElementTypes.Sync_Message_Edge, at(sendX, 195), at(recvX, 195)); + EditPart messageEP = createConnection(SequenceElementTypes.Sync_Message_Edge, at(sendX, MESSAGE_Y), + at(recvX, MESSAGE_Y)); - assertThat(messageEP, runs(x(SENDER), 195, x(RECEIVER), 195, 2)); + assertThat(messageEP, runs(x(SENDER), MESSAGE_Y, x(RECEIVER), MESSAGE_Y, 2)); } // @@ -94,19 +99,18 @@ public void createSyncMessage() { @Parameters(name = "{1}") public static Iterable parameters() { return Arrays.asList(new Object[][] { // - { false, "left-to-right" }, // - { true, "right-to-left" }, // + {false, "left-to-right" }, // + {true, "right-to-left" }, // }); } /** - * Compute an adjusted {@code x} coördinate based the test message's - * directionality and whether that end is at an execution specification or a - * lifeline stem. - * - * @param source whether this is the source end of the message + * Compute an adjusted {@code x} coordinate based the test message's directionality and whether that end + * is at an execution specification or a lifeline stem. * - * @return the adjusted X coördinate + * @param source + * whether this is the source end of the message + * @return the adjusted X coordinate */ int x(boolean source) { if (sendX > recvX) { diff --git a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/ExecutionWithSelfMoveUITest.java b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/ExecutionWithSelfMoveUITest.java new file mode 100644 index 00000000..4651aa87 --- /dev/null +++ b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/ExecutionWithSelfMoveUITest.java @@ -0,0 +1,128 @@ +/***************************************************************************** + * Copyright (c) 2018 EclipseSource 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 + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * EclipseSource - Initial API and implementation + * + *****************************************************************************/ + +package org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies.tests; + +import static org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.matchers.GEFMatchers.EditParts.runs; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.MatcherAssert.assertThat; + +import org.eclipse.draw2d.geometry.Point; +import org.eclipse.draw2d.geometry.PointList; +import org.eclipse.draw2d.geometry.Rectangle; +import org.eclipse.gef.EditPart; +import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.rules.AutoFixture; +import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.rules.AutoFixtureRule; +import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.rules.Maximized; +import org.eclipse.papyrus.uml.interaction.tests.rules.ModelResource; +import org.eclipse.uml2.uml.ExecutionSpecification; +import org.eclipse.uml2.uml.Lifeline; +import org.eclipse.uml2.uml.Message; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; + +/** + * UI test for moving executions with self start / finish messages. + */ +@ModelResource("execution-self.di") +@Maximized +public class ExecutionWithSelfMoveUITest extends AbstractGraphicalEditPolicyUITest { + + @Rule + public final AutoFixtureRule autoFixtures = new AutoFixtureRule(this); + + @AutoFixture("Lifeline1") + private Lifeline lifeline1; + + @AutoFixture + private EditPart lifeline1EP; + + @AutoFixture("request") + private Message request; + + @AutoFixture + private EditPart requestEP; + + @AutoFixture("reply") + private Message reply; + + @AutoFixture + private EditPart replyEP; + + @AutoFixture("Execution1") + private ExecutionSpecification execution1; + + @AutoFixture + private EditPart execution1EP; + + private Rectangle execution1Rect; + + private PointList replyPoints; + + private PointList requestPoints; + + @Before + public void updateFixtures() { + execution1Rect = getBounds(execution1EP); + requestPoints = getPoints(requestEP).getCopy(); + replyPoints = getPoints(replyEP).getCopy(); + } + + @Test + public void moveDownExecution() { + int delta = +50; + + Point grabAt = execution1Rect.getCenter(); + Point dropAt = execution1Rect.getCenter().translate(0, delta); + editor.moveSelection(grabAt, dropAt); + + // check result + Rectangle exec1EPNew = execution1Rect.getCopy().translate(new Point(0, delta)); + EditPart exec1EP = findEditPart("execution-self::Interaction1::Execution1", false); + assertThat(getBounds(exec1EP), equalTo(exec1EPNew)); + PointList newRequestPoints = requestPoints.getCopy(); + newRequestPoints.translate(0, delta); + assertThat(requestEP, runs(exec1EPNew.getCenter().x(), exec1EPNew.getTop().y() - 20, + exec1EPNew.getTopRight().x(), exec1EPNew.getTopRight().y())); + + PointList newReplyPoints = replyPoints.getCopy(); + newReplyPoints.translate(0, delta); + assertThat(replyEP, runs(exec1EPNew.getBottomRight().x(), exec1EPNew.getBottomRight().y(), + exec1EPNew.getCenter().x(), exec1EPNew.getBottom().y() + 20)); + } + + @Test + public void moveUpExecution() { + int delta = -50; + + Point grabAt = execution1Rect.getCenter(); + Point dropAt = execution1Rect.getCenter().translate(0, delta); + editor.moveSelection(grabAt, dropAt); + + // check result + Rectangle exec1EPNew = execution1Rect.getCopy().translate(new Point(0, delta)); + EditPart exec1EP = findEditPart("execution-self::Interaction1::Execution1", false); + assertThat(getBounds(exec1EP), equalTo(exec1EPNew)); + PointList newRequestPoints = requestPoints.getCopy(); + newRequestPoints.translate(0, delta); + assertThat(requestEP, runs(exec1EPNew.getCenter().x(), exec1EPNew.getTop().y() - 20, + exec1EPNew.getTopRight().x(), exec1EPNew.getTopRight().y())); + + PointList newReplyPoints = replyPoints.getCopy(); + newReplyPoints.translate(0, delta); + assertThat(replyEP, runs(exec1EPNew.getBottomRight().x(), exec1EPNew.getBottomRight().y(), + exec1EPNew.getCenter().x(), exec1EPNew.getBottom().y() + 20)); + } + +} diff --git a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/LifelineSwitchingUITest.java b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/LifelineSwitchingUITest.java index 2f40b561..5f48f49d 100644 --- a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/LifelineSwitchingUITest.java +++ b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/LifelineSwitchingUITest.java @@ -91,9 +91,6 @@ public class LifelineSwitchingUITest extends AbstractGraphicalEditPolicyUITest { // Width of the destruction X shape private static final int DESTRUCTION_WIDTH = 20; - // Width of an execution specification - private static final int EXEC_WIDTH = 10; - // Vertical gap between send and receive of a self-message private static final int SELF_MESSAGE_HEIGHT = 20; @@ -139,10 +136,10 @@ public void switchLifeline() { protected void switchLifeline(VerificationMode mode) { editor.with(editor.allowSemanticReordering(), - () -> editor.moveSelection(at(getGrabX(), mesgY), at(newRecvX, mesgY))); + () -> editor.moveSelection(at(getGrabX(), getGrabY()), at(getReleaseX(), getReleaseY()))); // Verify the new visuals - mode.verify(messageEP, runs(sendX, mesgY, getNewRecvX(), mesgY)); + mode.verify(messageEP, runs(sendX, mesgY, getNewRecvX(), getNewRecvY())); // And the semantics MMessage msg = getRequestMessage(); @@ -160,6 +157,19 @@ public void undoSwitchLifeline() { protected void undoSwitchLifeline(VerificationMode mode) { editor.undo(); + // Verify the old visuals + mode.verify(messageEP, runs(sendX, mesgY, getGrabX(), getGrabY())); + + // And the old semantics + MMessage msg = getRequestMessage(); + mode.verify("Sender changed", msg.getSender(), is(getSender())); + mode.verify("Receiver not reverted", msg.getReceiver(), not(getReceiver())); + mode.verify("Wrong receiver", msg.getReceiver(), is(getOriginalReceiver())); + } + + protected void undoSwitchSelfLifeline(VerificationMode mode) { + editor.undo(); + // Verify the old visuals mode.verify(messageEP, runs(sendX, mesgY, getGrabX(), mesgY)); @@ -171,6 +181,30 @@ protected void undoSwitchLifeline(VerificationMode mode) { } } + public static class MessageSelfNoExecution extends MessageNoExecution { + + @Override + public int getReleaseX() { + return sendX; + } + + @Override + public int getNewRecvX() { + return sendX; + } + + @Override + public int getNewRecvY() { + return mesgY + 20; + } + + @Override + Optional getReceiver() { + return getSender(); + } + + } + public static class MessageNoReply extends MessageNoExecution { @ClassRule @@ -182,7 +216,7 @@ protected void switchLifeline(VerificationMode mode) { // Verify the new visuals mode.verify(getExecutionEditPart(), - isBounded(isNear(getNewRecvX(), 5), isNear(mesgY), anything(), anything())); + isBounded(isNear(getNewRecvX(), 5), isNear(getNewRecvY()), anything(), anything())); // And the semantics MExecution exec = requireExecution(); @@ -235,6 +269,29 @@ int getNewRecvX() { } + public static class MessageSelfNoReply extends MessageNoReply { + + @Override + public int getReleaseX() { + return sendX; + } + + @Override + public int getNewRecvX() { + return sendX; + } + + @Override + public int getNewRecvY() { + return mesgY + 20; + } + + @Override + Optional getReceiver() { + return getSender(); + } + } + public static class MessageWithReply extends MessageNoReply { @ClassRule @@ -282,6 +339,251 @@ EditPart getReplyEditPart() { } } + public static class MessageSelfWithReply extends MessageNoExecution { + + @Override + public int getReleaseX() { + return sendX; + } + + @Override + public int getNewRecvY() { + return mesgY + 20; + } + + @Override + Optional getReceiver() { + return getSender(); + } + + @Override + MLifeline requireReceiver() { + return requireLifeline("Lifeline1"); + } + + @ClassRule + public static LightweightSeqDPrefs prefs = new LightweightSeqDPrefs().createRepliesForSyncCalls(); + + @Override + protected void switchLifeline(VerificationMode mode) { + super.switchLifeline(ASSERT); + + // Verify the new visuals + mode.verify(getExecutionEditPart(), isBounded(isNear(getReleaseX(), 5), isNear(getNewRecvY()), + is(EXEC_WIDTH), is(EXEC_HEIGHT))); + + // And the semantics + MExecution exec = requireExecution(); + mode.verify("Owner not changed", exec.getOwner(), not(requireOriginalReceiver())); + mode.verify("Wrong owner", exec.getOwner(), is(requireReceiver())); + MOccurrence finish = verifying(mode, exec.getFinish(), "No finish occurrence"); + mode.verify("Finish coverage lost", finish.getCovered().isPresent(), is(true)); + MLifeline covered = finish.getCovered().get(); + mode.verify("Finish coverage not changed", covered, not(requireOriginalReceiver())); + mode.verify("Wrong finish coverage", covered, is(requireReceiver())); + + // check the reply + MMessage msg = getReplyMessage(); + mode.verify("Sender changed", msg.getSender(), is(getSender())); + mode.verify("Receiver not changed", msg.getReceiver(), not(getOriginalReceiver())); + mode.verify("Wrong receiver", msg.getReceiver(), is(getReceiver())); + mode.verify(getReplyEditPart(), runs(getNewRecvX(), getNewRecvY() + EXEC_HEIGHT, getReleaseX(), + getNewRecvY() + EXEC_HEIGHT + SELF_MESSAGE_HEIGHT)); + + } + + @Override + protected void undoSwitchLifeline(VerificationMode mode) { + super.undoSwitchLifeline(ASSERT); + + // Verify the old visuals + mode.verify(getExecutionEditPart(), + isBounded(isNear(getGrabX(), 5), isNear(mesgY), anything(), anything())); + + // And the old semantics + MExecution exec = requireExecution(); + mode.verify("Owner not reverted", exec.getOwner(), is(requireOriginalReceiver())); + MOccurrence finish = verifying(mode, exec.getFinish(), "No finish occurrence"); + mode.verify("Finish coverage lost", finish.getCovered().isPresent(), is(true)); + MLifeline covered = finish.getCovered().get(); + mode.verify("Finish coverage not reverted", covered, is(requireOriginalReceiver())); + } + + // + // Test framework + // + + EditPart getExecutionEditPart() { + Optional editPart = Optional.of(messageEP).map(ConnectionEditPart.class::cast) + .map(ConnectionEditPart::getTarget) + .filter(ExecutionSpecificationEditPart.class::isInstance); + return assuming(editPart, "No execution edit-part created"); + } + + @Override + int getGrabX() { + return super.getGrabX() - (EXEC_WIDTH / 2); + } + + @Override + int getNewRecvX() { + return sendX + (EXEC_WIDTH / 2); + } + + // + // Test framework + // + + EditPart getReplyEditPart() { + @SuppressWarnings("unchecked") + Optional editPart = editor.getDiagramEditPart().getConnections().stream().skip(1) + .findFirst(); + return assuming(editPart, "No execution edit-part created"); + } + } + + public static class MessageSelfToUnSelfWithReply extends MessageNoExecution { + + @ClassRule + public static LightweightSeqDPrefs prefs = new LightweightSeqDPrefs().createRepliesForSyncCalls(); + + @Override + protected int getInitialSenderX() { + return LIFELINE_1_BODY_X; + } + + @Override + protected int getInitialSenderY() { + return mesgY; + } + + @Override + protected int getInitialReceiverX() { + return LIFELINE_1_BODY_X; + } + + @Override + protected int getInitialReceiverY() { + return mesgY + 20; + } + + @Override + public int getReleaseX() { + return LIFELINE_2_BODY_X; + } + + @Override + int getGrabX() { + return getInitialReceiverX() + (EXEC_WIDTH / 2); + } + + @Override + int getGrabY() { + return getInitialReceiverY(); + } + + @Override + public int getNewRecvY() { + return mesgY; + } + + @Override + int getNewRecvX() { + return LIFELINE_2_BODY_X - (EXEC_WIDTH / 2); + } + + @Override + MLifeline requireReceiver() { + return requireLifeline("Lifeline2"); + } + + @Override + Optional getReceiver() { + return getLifeline("Lifeline2"); + } + + @Override + Optional getOriginalReceiver() { + return getLifeline("Lifeline1"); + } + + @Override + Optional getSender() { + return getLifeline("Lifeline1"); + } + + @Override + MLifeline requireOriginalReceiver() { + return requireLifeline("Lifeline1"); + } + + @Override + protected void switchLifeline(VerificationMode mode) { + super.switchLifeline(ASSERT); + + // Verify the new visuals + mode.verify(getExecutionEditPart(), isBounded(isNear(getReleaseX(), 5), isNear(getNewRecvY()), + is(EXEC_WIDTH), is(EXEC_HEIGHT))); + + // And the semantics + MExecution exec = requireExecution(); + mode.verify("Owner not changed", exec.getOwner(), not(requireOriginalReceiver())); + mode.verify("Wrong owner", exec.getOwner(), is(requireReceiver())); + MOccurrence finish = verifying(mode, exec.getFinish(), "No finish occurrence"); + mode.verify("Finish coverage lost", finish.getCovered().isPresent(), is(true)); + MLifeline covered = finish.getCovered().get(); + mode.verify("Finish coverage not changed", covered, not(requireOriginalReceiver())); + mode.verify("Wrong finish coverage", covered, is(requireReceiver())); + + // check the reply + MMessage msg = getReplyMessage(); + mode.verify("Sender changed", msg.getSender(), is(getReceiver())); + mode.verify("Wrong receiver", msg.getReceiver(), is(getSender())); + mode.verify(getReplyEditPart(), runs(getNewRecvX(), getNewRecvY() + EXEC_HEIGHT, + getInitialReceiverX(), getNewRecvY() + EXEC_HEIGHT)); + + } + + @Override + protected void undoSwitchLifeline(VerificationMode mode) { + super.undoSwitchLifeline(ASSERT); + + // Verify the old visuals + mode.verify(getExecutionEditPart(), isBounded(isNear(getGrabX() - EXEC_WIDTH, 5), + isNear(getGrabY()), is(EXEC_WIDTH), is(EXEC_HEIGHT))); + + // And the old semantics + MExecution exec = requireExecution(); + mode.verify("Owner not reverted", exec.getOwner(), is(requireOriginalReceiver())); + MOccurrence finish = verifying(mode, exec.getFinish(), "No finish occurrence"); + mode.verify("Finish coverage lost", finish.getCovered().isPresent(), is(true)); + MLifeline covered = finish.getCovered().get(); + mode.verify("Finish coverage not reverted", covered, is(requireOriginalReceiver())); + } + + // + // Test framework + // + + EditPart getExecutionEditPart() { + Optional editPart = Optional.of(messageEP).map(ConnectionEditPart.class::cast) + .map(ConnectionEditPart::getTarget) + .filter(ExecutionSpecificationEditPart.class::isInstance); + return assuming(editPart, "No execution edit-part created"); + } + + // + // Test framework + // + + EditPart getReplyEditPart() { + @SuppressWarnings("unchecked") + Optional editPart = editor.getDiagramEditPart().getConnections().stream().skip(1) + .findFirst(); + return assuming(editPart, "No execution edit-part created"); + } + } + public static class DeleteMessage extends MessageNoExecution { @Override @@ -579,15 +881,160 @@ int getNewRecvX() { } + @RunWith(JUnit4.class) + public static class ExecutionSwitchLifeline extends LifelineSwitchingUITest { + + @ClassRule + public static LightweightSeqDPrefs prefs = new LightweightSeqDPrefs().createRepliesForSyncCalls(); + + @Test + public void switchLifeline() { + switchLifeline(ASSERT); + } + + protected void switchLifeline(VerificationMode mode) { + editor.with(editor.allowSemanticReordering(), + () -> editor.moveSelection(at(getGrabX(), getGrabY()), at(getReleaseX(), getReleaseY()))); + + // Verify the new visuals + mode.verify(messageEP, runs(sendX, mesgY, getNewRecvX(), getNewRecvY())); + + // And the semantics + MMessage msg = getRequestMessage(); + mode.verify("Sender changed", msg.getSender(), is(getSender())); + mode.verify("Receiver not changed", msg.getReceiver(), not(getOriginalReceiver())); + mode.verify("Wrong receiver", msg.getReceiver(), is(getReceiver())); + + mode.verify(getExecutionEditPart(), isBounded(isNear(getReleaseX(), 5), isNear(getNewRecvY()), + is(EXEC_WIDTH), is(EXEC_HEIGHT))); + + // And the semantics + MExecution exec = requireExecution(); + mode.verify("Owner not changed", exec.getOwner(), not(requireOriginalReceiver())); + mode.verify("Wrong owner", exec.getOwner(), is(requireReceiver())); + MOccurrence finish = verifying(mode, exec.getFinish(), "No finish occurrence"); + mode.verify("Finish coverage lost", finish.getCovered().isPresent(), is(true)); + MLifeline covered = finish.getCovered().get(); + mode.verify("Finish coverage not changed", covered, not(requireOriginalReceiver())); + mode.verify("Wrong finish coverage", covered, is(requireReceiver())); + + // check the reply + MMessage reply = getReplyMessage(); + mode.verify("Sender changed", reply.getSender(), is(getSender())); + mode.verify("Receiver not changed", reply.getReceiver(), not(getOriginalReceiver())); + mode.verify("Wrong receiver", reply.getReceiver(), is(getReceiver())); + mode.verify(getReplyEditPart(), runs(getNewRecvX(), getNewRecvY() + EXEC_HEIGHT, getReleaseX(), + getNewRecvY() + EXEC_HEIGHT + SELF_MESSAGE_HEIGHT)); + } + + @Override + public void createMessage() { + messageEP = createConnection(getMessageType(), at(getInitialSenderX(), getInitialSenderY()), + at(getInitialReceiverX(), getInitialReceiverY())); + } + + @Override + int getNewRecvX() { + return sendX + EXEC_WIDTH / 2; + } + + @Override + int getNewRecvY() { + return mesgY + 20; + } + + @Override + int getGrabX() { + return super.getGrabX(); + } + + @Override + int getGrabY() { + return mesgY + EXEC_HEIGHT / 2; + } + + @Override + public int getReleaseY() { + return getGrabY(); + } + + @Override + public int getReleaseX() { + return sendX; + } + + @Override + Optional getReceiver() { + return getLifeline("Lifeline1"); + } + + @Override + MLifeline requireReceiver() { + return requireLifeline("Lifeline1"); + } + + // + // Test framework + // + + EditPart getExecutionEditPart() { + Optional editPart = Optional.of(messageEP).map(ConnectionEditPart.class::cast) + .map(ConnectionEditPart::getTarget) + .filter(ExecutionSpecificationEditPart.class::isInstance); + return assuming(editPart, "No execution edit-part created"); + } + + EditPart getReplyEditPart() { + @SuppressWarnings("unchecked") + Optional editPart = editor.getDiagramEditPart().getConnections().stream().skip(1) + .findFirst(); + return assuming(editPart, "No execution edit-part created"); + } + + } // // Test framework // @Before public void createMessage() { - messageEP = createConnection(getMessageType(), at(sendX, mesgY), at(recvX, mesgY)); + messageEP = createConnection(getMessageType(), at(getInitialSenderX(), getInitialSenderY()), + at(getInitialReceiverX(), getInitialReceiverY())); + + if (moveTarget()) { + assumeThat(messageEP, runs(getInitialSenderX(), getInitialSenderY(), getGrabX(), getGrabY())); + } else { + assumeThat(messageEP, runs(getGrabX(), getGrabY(), getInitialReceiverX(), getInitialReceiverY())); + } + + } - assumeThat(messageEP, runs(sendX, mesgY, getGrabX(), mesgY)); + protected boolean moveTarget() { + return true; + } + + protected int getInitialReceiverY() { + return mesgY; + } + + protected int getInitialReceiverX() { + return recvX; + } + + protected int getInitialSenderY() { + return mesgY; + } + + protected int getInitialSenderX() { + return sendX; + } + + public int getReleaseX() { + return newRecvX; + } + + public int getReleaseY() { + return mesgY; } IElementType getMessageType() { @@ -598,10 +1045,18 @@ int getGrabX() { return recvX; } + int getGrabY() { + return mesgY; + } + int getNewRecvX() { return newRecvX; } + int getNewRecvY() { + return mesgY; + } + MInteraction getInteraction() { return assuming(editor.getSequenceDiagram().map(MInteraction::getInstance), "No logical model"); } diff --git a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/SelfMessageCreationUITest.java b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/SelfMessageCreationUITest.java index c4c43707..bf0d0b60 100644 --- a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/SelfMessageCreationUITest.java +++ b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/SelfMessageCreationUITest.java @@ -12,20 +12,29 @@ package org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies.tests; +import static org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.matchers.GEFMatchers.EditParts.isBounded; import static org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.matchers.GEFMatchers.EditParts.runs; import static org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.rules.EditorFixture.at; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.MatcherAssert.assertThat; import java.util.Arrays; +import org.eclipse.emf.ecore.EObject; +import org.eclipse.gef.ConnectionEditPart; import org.eclipse.gef.EditPart; import org.eclipse.gmf.runtime.emf.type.core.IElementType; +import org.eclipse.gmf.runtime.notation.View; +import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.parts.ExecutionSpecificationEditPart; +import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.parts.MessageEditPart; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.providers.SequenceElementTypes; import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.rules.LightweightSeqDPrefs; import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.rules.Maximized; import org.eclipse.papyrus.uml.diagram.sequence.runtime.util.MessageUtil; import org.eclipse.papyrus.uml.interaction.tests.rules.ModelResource; +import org.eclipse.uml2.uml.Message; import org.eclipse.uml2.uml.MessageSort; import org.junit.Before; import org.junit.ClassRule; @@ -79,6 +88,7 @@ public SelfMessageCreationUITest(MessageSort sort, CreationMode mode) { this.mode = mode; messageX = mode == CreationMode.ON_EXECUTION ? LIFELINE_2_BODY_X : LIFELINE_1_BODY_X; messageY = Y_POSITION + (mode.isTall() ? CUSTOM_SPAN : 0); + } @Test @@ -124,7 +134,22 @@ private void doCreateSelfMessage() { default: if (mode != CreationMode.WITH_EXECUTION) { assertThat(messageEP, runs(x(), top(), x(), bottom(), 2)); - } // TODO: Assert the self-message shape with execution when we draw it properly + } else { + // messageEP is the sync message. + assertThat(messageEP, runs(x(), top(), x() + 5, bottom())); + + // then find the execution linked to this message + assertThat(messageEP, instanceOf(MessageEditPart.class)); + EditPart executionEP = ((MessageEditPart)messageEP).getTarget(); + assertThat(executionEP, instanceOf(ExecutionSpecificationEditPart.class)); + assertThat(executionEP, isBounded(x() - 5, bottom(), 10, 40)); // default sizes + ConnectionEditPart replyEP = (ConnectionEditPart)((ExecutionSpecificationEditPart)executionEP) + .getSourceConnections().get(0); + EObject reply = ((View)replyEP.getModel()).getElement(); + assertThat(reply, instanceOf(Message.class)); + assertThat(((Message)reply).getMessageSort(), equalTo(MessageSort.REPLY_LITERAL)); + assertThat(replyEP, runs(x() + 5, bottom() + 40, x(), bottom() + 40 + MINIMUM_SPAN)); + } break; } diff --git a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/SelfMessageReconnectionUITest.java b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/SelfMessageReconnectionUITest.java index 9f27d604..69b7aa16 100644 --- a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/SelfMessageReconnectionUITest.java +++ b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/SelfMessageReconnectionUITest.java @@ -67,11 +67,13 @@ public class SelfMessageReconnectionUITest extends AbstractGraphicalEditPolicyUI private static final int LIFELINE_2_BODY_X = 281; private static final int SEND_Y = 195; + private static final int RECV_Y = SEND_Y + 20; private static final int SELF_MESSAGE_WIDTH = 40; private final MessageSort messageSort; + private final ReconnectionMode reconnectionMode; private EditPart messageEP; @@ -97,55 +99,55 @@ public void reconnectSelfMessage() { Modifiers modifiers = editor.unmodified(); switch (reconnectionMode) { - case SLIDE_SEND_UP: - grabY = SEND_Y; - dropX = LIFELINE_1_BODY_X; - delta = -10; - dropY = SEND_Y + delta; - break; - case SLIDE_SEND_DOWN: - grabY = SEND_Y; - dropX = LIFELINE_1_BODY_X; - dropY = SEND_Y + delta; - break; - case SLIDE_RECEIVE_UP: - dropX = LIFELINE_1_BODY_X; - delta = -10; - dropY = RECV_Y + delta; - break; - case SLIDE_RECEIVE_DOWN: - dropX = LIFELINE_1_BODY_X; - dropY = RECV_Y + delta; - break; - case MOVE_UP: - // Grab the spine of the message - grabX = LIFELINE_1_BODY_X + SELF_MESSAGE_WIDTH; - grabY = (SEND_Y + RECV_Y) / 2; - dropX = grabX; - delta = -10; - dropY = grabY + delta; - break; - case MOVE_DOWN: - // Grab the spine of the message - grabX = LIFELINE_1_BODY_X + SELF_MESSAGE_WIDTH; - grabY = (SEND_Y + RECV_Y) / 2; - dropX = grabX; - dropY = grabY + delta; - break; - case RECEIVE_ON_OTHER_LIFELINE: - dropX = LIFELINE_2_BODY_X; - dropY = RECV_Y; - modifiers = editor.allowSemanticReordering(); - break; - case RECEIVE_ON_SENDING_LIFELINE: - grabX = LIFELINE_2_BODY_X - 5; - grabY = SEND_Y + (isSynchronous(messageSort) ? 0 : 15); - dropX = LIFELINE_1_BODY_X; - dropY = SEND_Y; // The editor should enforce the minimum gap - modifiers = editor.allowSemanticReordering(); - break; - default: - throw new IllegalStateException("Unsupported reconnection mode: " + reconnectionMode); + case SLIDE_SEND_UP: + grabY = SEND_Y; + dropX = LIFELINE_1_BODY_X; + delta = -10; + dropY = SEND_Y + delta; + break; + case SLIDE_SEND_DOWN: + grabY = SEND_Y; + dropX = LIFELINE_1_BODY_X; + dropY = SEND_Y + delta; + break; + case SLIDE_RECEIVE_UP: + dropX = LIFELINE_1_BODY_X; + delta = -10; + dropY = RECV_Y + delta; + break; + case SLIDE_RECEIVE_DOWN: + dropX = LIFELINE_1_BODY_X; + dropY = RECV_Y + delta; + break; + case MOVE_UP: + // Grab the spine of the message + grabX = LIFELINE_1_BODY_X + SELF_MESSAGE_WIDTH; + grabY = (SEND_Y + RECV_Y) / 2; + dropX = grabX; + delta = -10; + dropY = grabY + delta; + break; + case MOVE_DOWN: + // Grab the spine of the message + grabX = LIFELINE_1_BODY_X + SELF_MESSAGE_WIDTH; + grabY = (SEND_Y + RECV_Y) / 2; + dropX = grabX; + dropY = grabY + delta; + break; + case RECEIVE_ON_OTHER_LIFELINE: + dropX = LIFELINE_2_BODY_X; + dropY = RECV_Y; + modifiers = editor.allowSemanticReordering(); + break; + case RECEIVE_ON_SENDING_LIFELINE: + grabX = LIFELINE_2_BODY_X - 5; + grabY = SEND_Y + (isSynchronous(messageSort) ? 0 : 15); + dropX = LIFELINE_1_BODY_X; + dropY = SEND_Y; // The editor should enforce the minimum gap + modifiers = editor.allowSemanticReordering(); + break; + default: + throw new IllegalStateException("Unsupported reconnection mode: " + reconnectionMode); } Point grabAt = at(grabX, grabY); @@ -153,61 +155,59 @@ public void reconnectSelfMessage() { editor.with(modifiers, () -> editor.moveSelection(grabAt, dropAt)); switch (reconnectionMode) { - case SLIDE_SEND_UP: - if (async) { - assertThat("Send end not moved as expected", messageEP, - runs(sendX(), dropY, recvX(), RECV_Y, 2)); - } else { - assertThat("Synchronous message was reshaped", messageEP, - runs(sendX(), SEND_Y, recvX(), RECV_Y, 2)); - } - break; - case SLIDE_SEND_DOWN: - case SLIDE_RECEIVE_UP: - if (async) { - assertThat("Minimum gap not maintained", messageEP, - runs(sendX(), SEND_Y, recvX(), RECV_Y, 2)); - } else { - assertThat("Synchronous message was reshaped", messageEP, - runs(sendX(), SEND_Y, recvX(), RECV_Y, 2)); + case SLIDE_SEND_UP: + if (async) { + assertThat("Send end not moved as expected", messageEP, + runs(sendX(), dropY, recvX(), RECV_Y, 2)); + } else { + assertThat("Synchronous message was reshaped", messageEP, + runs(sendX(), SEND_Y, recvX(), RECV_Y, 2)); + } + break; + case SLIDE_SEND_DOWN: + case SLIDE_RECEIVE_UP: + if (async) { + assertThat("Minimum gap not maintained", messageEP, + runs(sendX(), SEND_Y, recvX(), RECV_Y, 2)); + } else { + assertThat("Synchronous message was reshaped", messageEP, + runs(sendX(), SEND_Y, recvX(), RECV_Y, 2)); + } + break; + case SLIDE_RECEIVE_DOWN: + if (async) { + assertThat("Receive end not moved as expected", messageEP, + runs(sendX(), SEND_Y, recvX(), dropY, 2)); + } else { + assertThat("Synchronous message was reshaped", messageEP, + runs(sendX(), SEND_Y, recvX(), RECV_Y, 2)); + } + break; + case MOVE_UP: + case MOVE_DOWN: + assertThat("Message not moved as expected", messageEP, + runs(sendX(), SEND_Y + delta, recvX(), RECV_Y + delta, 2)); + break; + case RECEIVE_ON_OTHER_LIFELINE: { + assertThat("Lifeline and slope incorrect", messageEP, runs(sendX(), SEND_Y, recvX(), + // Async messages may slope; others snap to horizontal + async ? RECV_Y : SEND_Y, 2)); + + Routing routing = getRouting(messageEP); + assertThat("Lifeline is not oblique", routing, is(Routing.MANUAL_LITERAL)); + RelativeBendpoints bendpoints = getBendpoints(messageEP); + assertThat("Message has bendpoints", (List)bendpoints.getPoints(), + not(hasItem(anything()))); + break; } - break; - case SLIDE_RECEIVE_DOWN: - if (async) { - assertThat("Receive end not moved as expected", messageEP, - runs(sendX(), SEND_Y, recvX(), dropY, 2)); - } else { - assertThat("Synchronous message was reshaped", messageEP, + case RECEIVE_ON_SENDING_LIFELINE: { + assertThat("Self-message send and receive not separate by correct gap", messageEP, runs(sendX(), SEND_Y, recvX(), RECV_Y, 2)); + + Routing routing = getRouting(messageEP); + assertThat("Lifeline is not rectilinear", routing, is(Routing.RECTILINEAR_LITERAL)); + break; } - break; - case MOVE_UP: - case MOVE_DOWN: - assertThat("Message not moved as expected", messageEP, - runs(sendX(), SEND_Y + delta, recvX(), RECV_Y + delta, 2)); - break; - case RECEIVE_ON_OTHER_LIFELINE: { - assertThat("Lifeline and slope incorrect", messageEP, runs(sendX(), SEND_Y, recvX(), - // Async messages may slope; others snap to horizontal - async ? RECV_Y : SEND_Y, 2)); - - Routing routing = getRouting(messageEP); - assertThat("Lifeline is not oblique", routing, is(Routing.MANUAL_LITERAL)); - RelativeBendpoints bendpoints = getBendpoints(messageEP); - assertThat("Message has bendpoints", (List) bendpoints.getPoints(), - not(hasItem(anything()))); - break; - } - case RECEIVE_ON_SENDING_LIFELINE: { - assertThat("Self-message send and receive not separate by correct gap", messageEP, - runs(sendX(), SEND_Y, recvX(), RECV_Y, 2)); - - Routing routing = getRouting(messageEP); - assertThat("Lifeline is not rectilinear", routing, is(Routing.RECTILINEAR_LITERAL)); - RelativeBendpoints bendpoints = getBendpoints(messageEP); - assertThat("Wrong number of message bendpoints", bendpoints.getPoints().size(), is(4)); - break; - } } } @@ -223,7 +223,7 @@ public static Iterable parameters() { for (MessageSort sort : Arrays.asList(MessageSort.SYNCH_CALL_LITERAL, MessageSort.ASYNCH_CALL_LITERAL)) { for (ReconnectionMode mode : ReconnectionMode.values()) { - result.add(new Object[] { sort, mode }); + result.add(new Object[] {sort, mode }); } } @@ -234,23 +234,22 @@ public static Iterable parameters() { public void createMessage() { IElementType type = SequenceElementTypes.getMessageType(messageSort); switch (reconnectionMode) { - default: // the majority of cases are self-messages - // Drop connection at the same Y coördinate; the editor adds the internal gap - messageEP = createConnection(type, at(LIFELINE_1_BODY_X, SEND_Y), - at(LIFELINE_1_BODY_X, SEND_Y)); - break; - case RECEIVE_ON_SENDING_LIFELINE: - int receiveY = SEND_Y + (isSynchronous(messageSort) ? 0 : 15); - messageEP = createConnection(type, at(LIFELINE_1_BODY_X, SEND_Y), - at(LIFELINE_2_BODY_X, receiveY)); - break; + default: // the majority of cases are self-messages + // Drop connection at the same Y coördinate; the editor adds the internal gap + messageEP = createConnection(type, at(LIFELINE_1_BODY_X, SEND_Y), + at(LIFELINE_1_BODY_X, SEND_Y)); + break; + case RECEIVE_ON_SENDING_LIFELINE: + int receiveY = SEND_Y + (isSynchronous(messageSort) ? 0 : 15); + messageEP = createConnection(type, at(LIFELINE_1_BODY_X, SEND_Y), + at(LIFELINE_2_BODY_X, receiveY)); + break; } } /** - * Compute an adjusted {@code x} coördinate from which the message is expected - * to be sent, based whether the send end is on an execution specification or on - * the lifeline stem. + * Compute an adjusted {@code x} coördinate from which the message is expected to be sent, based whether + * the send end is on an execution specification or on the lifeline stem. * * @return the adjusted send end X coördinate */ @@ -259,27 +258,26 @@ int sendX() { } /** - * Compute an adjusted {@code x} coördinate at which the message is expected to - * be received, based whether the receive end is on an execution specification - * or on the lifeline stem. + * Compute an adjusted {@code x} coördinate at which the message is expected to be received, based whether + * the receive end is on an execution specification or on the lifeline stem. * * @return the adjusted receive end X coördinate */ int recvX() { switch (reconnectionMode) { - default: // the majority of cases are self-messages - return LIFELINE_1_BODY_X; - case RECEIVE_ON_OTHER_LIFELINE: - return LIFELINE_2_BODY_X - 5; + default: // the majority of cases are self-messages + return LIFELINE_1_BODY_X; + case RECEIVE_ON_OTHER_LIFELINE: + return LIFELINE_2_BODY_X - 5; } } Routing getRouting(EditPart editPart) { - return ((Connector) editPart.getModel()).getRouting(); + return ((Connector)editPart.getModel()).getRouting(); } RelativeBendpoints getBendpoints(EditPart editPart) { - return Optional.ofNullable(((Connector) editPart.getModel()).getBendpoints()) + return Optional.ofNullable(((Connector)editPart.getModel()).getBendpoints()) .filter(RelativeBendpoints.class::isInstance).map(RelativeBendpoints.class::cast) .orElse(null); } @@ -304,8 +302,8 @@ enum ReconnectionMode { /** Re-connect a receiving end of a self-message to some other lifeline. */ RECEIVE_ON_OTHER_LIFELINE, /** - * Re-connect the receiving end of a message between two lifelines to the - * sending lifeline, to make it a self-message. + * Re-connect the receiving end of a message between two lifelines to the sending lifeline, to make it + * a self-message. */ RECEIVE_ON_SENDING_LIFELINE; } diff --git a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/execution-self.di b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/execution-self.di new file mode 100644 index 00000000..8c549eec --- /dev/null +++ b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/execution-self.di @@ -0,0 +1,2 @@ + + diff --git a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/execution-self.notation b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/execution-self.notation new file mode 100644 index 00000000..118eace9 --- /dev/null +++ b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/execution-self.notation @@ -0,0 +1,42 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/execution-self.uml b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/execution-self.uml new file mode 100644 index 00000000..7240ba95 --- /dev/null +++ b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/execution-self.uml @@ -0,0 +1,16 @@ + + + + + + + + + + + + + + + + diff --git a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/tests/rules/EditorFixture.java b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/tests/rules/EditorFixture.java index fb44b54d..b09c0d29 100644 --- a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/tests/rules/EditorFixture.java +++ b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/tests/rules/EditorFixture.java @@ -46,6 +46,7 @@ import org.eclipse.emf.ecore.EObject; import org.eclipse.emf.ecore.resource.ResourceSet; import org.eclipse.emf.edit.domain.EditingDomain; +import org.eclipse.gef.ConnectionEditPart; import org.eclipse.gef.EditPart; import org.eclipse.gef.EditPartViewer; import org.eclipse.gef.Request; @@ -451,6 +452,8 @@ protected CreateConnectionRequest createTargetRequest() { return null; } + forceRefresh(); + // Find the new edit-part assertThat("No unsepecified-type request", request[0], notNullValue()); CreateConnectionViewAndElementRequest createRequest = (CreateConnectionViewAndElementRequest)request[0] @@ -463,6 +466,15 @@ protected CreateConnectionRequest createTargetRequest() { return result; } + @SuppressWarnings("unchecked") + public void forceRefresh() { + getDiagramEditPart().getConnections().stream().map(ConnectionEditPart.class::cast) + .forEach(cep -> ((ConnectionEditPart)cep).getFigure().invalidate()); + getDiagramEditPart().getConnections().stream().map(ConnectionEditPart.class::cast) + .forEach(cep -> ((ConnectionEditPart)cep).getFigure().validate()); + getDiagramEditPart().refresh(); + } + /** * Operate the mouse pointer as though to create a new connection in the current diagram, but do not click * to complete it. @@ -611,6 +623,8 @@ public void drag(Point start, Point finish) { tool.mouseUp(new MouseEvent(mouse), viewer); flushDisplayEvents(); + + forceRefresh(); } /** From 83a6417ef7425dea175012602e0def0249b83dd7 Mon Sep 17 00:00:00 2001 From: "Christian W. Damus" Date: Wed, 26 Dec 2018 15:58:44 -0500 Subject: [PATCH 2/4] Issue #389: Fix test assumption violations on Mac MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix several new test assumption violations on Mac platform, along with some compiler warnings in files changed for that reason. Note that one of the LifelineSwitchingUITest cases that was being skipped on an assumption violation is now (accurately) showing a regression on all platforms (a message that becomes a self-message now doesn’t have any vertical extent where it does on the master branch). Also fix the definition of some sub-suites of the LifelineSwitchingUITest that were making wrong assertions/assumptions. Those tests now pass instead of skip. Signed-off-by: Christian W. Damus --- ...tionSpecificationDragEditPolicyUITest.java | 3 + .../tests/ExecutionWithSelfMoveUITest.java | 6 ++ ...lineBodyGraphicalNodeEditPolicyUITest.java | 44 +++++++--- .../tests/LifelineSwitchingUITest.java | 88 +++++++++---------- .../tests/MessageExecutionUITest.java | 43 ++++----- .../tests/MessageReconnectionUITest.java | 49 +++++++---- .../tests/SelfMessageCreationUITest.java | 12 +++ 7 files changed, 149 insertions(+), 96 deletions(-) diff --git a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/ExecutionSpecificationDragEditPolicyUITest.java b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/ExecutionSpecificationDragEditPolicyUITest.java index 75174933..1ced9715 100644 --- a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/ExecutionSpecificationDragEditPolicyUITest.java +++ b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/ExecutionSpecificationDragEditPolicyUITest.java @@ -278,6 +278,9 @@ public static class WithMessagesAttached extends AbstractGraphicalEditPolicyUITe @Rule public final AutoFixtureRule autoFixtures = new AutoFixtureRule(this); + @ClassRule + public static TestRule tolerance = GEFMatchers.defaultTolerance(1); + @AutoFixture("exec1") private ExecutionSpecification exec; diff --git a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/ExecutionWithSelfMoveUITest.java b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/ExecutionWithSelfMoveUITest.java index 4651aa87..8d95bf43 100644 --- a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/ExecutionWithSelfMoveUITest.java +++ b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/ExecutionWithSelfMoveUITest.java @@ -21,6 +21,7 @@ import org.eclipse.draw2d.geometry.PointList; import org.eclipse.draw2d.geometry.Rectangle; import org.eclipse.gef.EditPart; +import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.matchers.GEFMatchers; import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.rules.AutoFixture; import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.rules.AutoFixtureRule; import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.rules.Maximized; @@ -29,8 +30,10 @@ import org.eclipse.uml2.uml.Lifeline; import org.eclipse.uml2.uml.Message; import org.junit.Before; +import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TestRule; /** * UI test for moving executions with self start / finish messages. @@ -39,6 +42,9 @@ @Maximized public class ExecutionWithSelfMoveUITest extends AbstractGraphicalEditPolicyUITest { + @ClassRule + public static TestRule tolerance = GEFMatchers.defaultTolerance(1); + @Rule public final AutoFixtureRule autoFixtures = new AutoFixtureRule(this); diff --git a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/LifelineBodyGraphicalNodeEditPolicyUITest.java b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/LifelineBodyGraphicalNodeEditPolicyUITest.java index e1203117..4bf8384c 100644 --- a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/LifelineBodyGraphicalNodeEditPolicyUITest.java +++ b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/LifelineBodyGraphicalNodeEditPolicyUITest.java @@ -24,30 +24,34 @@ import org.eclipse.gef.EditPart; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies.LifelineBodyGraphicalNodeEditPolicy; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.providers.SequenceElementTypes; +import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.matchers.GEFMatchers; import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.matchers.GEFMatchers.Figures; import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.rules.LightweightSeqDPrefs; import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.rules.Maximized; import org.eclipse.papyrus.uml.interaction.tests.rules.ModelResource; import org.junit.ClassRule; import org.junit.Test; +import org.junit.rules.TestRule; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameters; /** - * Integration test cases for the {@link LifelineBodyGraphicalNodeEditPolicy} - * class. + * Integration test cases for the {@link LifelineBodyGraphicalNodeEditPolicy} class. * * @author Christian W. Damus */ -@SuppressWarnings("restriction") @ModelResource("two-lifelines.di") @Maximized @RunWith(Parameterized.class) public class LifelineBodyGraphicalNodeEditPolicyUITest extends AbstractGraphicalEditPolicyUITest { @ClassRule - public static LightweightSeqDPrefs prefs = new LightweightSeqDPrefs().dontCreateExecutionsForSyncMessages(); + public static LightweightSeqDPrefs prefs = new LightweightSeqDPrefs() + .dontCreateExecutionsForSyncMessages(); + + @ClassRule + public static TestRule tolerance = GEFMatchers.defaultTolerance(1); // Horizontal position of the first lifeline's body private static final int LIFELINE_1_BODY_X = 121; @@ -60,7 +64,12 @@ public class LifelineBodyGraphicalNodeEditPolicyUITest extends AbstractGraphical private final int recvX; /** - * Initializes me. + * Initializes me with my test parameters. + * + * @param rightToLeft + * whether to draw the message from right-to-left (otherwise, left-to-right) + * @param direction + * human-readable interpretation of the {@code leftToRight} parameter */ public LifelineBodyGraphicalNodeEditPolicyUITest(boolean rightToLeft, String direction) { super(); @@ -76,21 +85,24 @@ public LifelineBodyGraphicalNodeEditPolicyUITest(boolean rightToLeft, String dir @Test public void createAsyncMessage() { - EditPart messageEP = createConnection(SequenceElementTypes.Async_Message_Edge, at(sendX, 125), at(recvX, 125)); + EditPart messageEP = createConnection(SequenceElementTypes.Async_Message_Edge, at(sendX, 125), + at(recvX, 125)); assertThat(messageEP, runs(sendX, 125, recvX, 125, 2)); } @Test public void createSlopedAsyncMessage() { - EditPart messageEP = createConnection(SequenceElementTypes.Async_Message_Edge, at(sendX, 125), at(recvX, 140)); + EditPart messageEP = createConnection(SequenceElementTypes.Async_Message_Edge, at(sendX, 125), + at(recvX, 140)); assertThat("Message should be sloped", messageEP, runs(sendX, 125, recvX, 140, 2)); } @Test public void attemptBackwardSlopedAsyncMessage_allowed() { - EditPart messageEP = createConnection(SequenceElementTypes.Async_Message_Edge, at(sendX, 140), at(recvX, 138)); + EditPart messageEP = createConnection(SequenceElementTypes.Async_Message_Edge, at(sendX, 140), + at(recvX, 138)); // The target to which the user draw the message should have priority over the // source @@ -106,7 +118,8 @@ public void attemptBackwardSlopedAsyncMessage_disallowed() { @Test public void createCrossedAsyncMessages() { - EditPart messageEP = createConnection(SequenceElementTypes.Async_Message_Edge, at(sendX, 150), at(recvX, 150)); + EditPart messageEP = createConnection(SequenceElementTypes.Async_Message_Edge, at(sendX, 150), + at(recvX, 150)); assumeThat(messageEP, runs(sendX, 150, recvX, 150, 2)); @@ -117,14 +130,17 @@ public void createCrossedAsyncMessages() { @Test public void attemptSlopedSyncMessage() { - EditPart messageEP = createConnection(SequenceElementTypes.Sync_Message_Edge, at(sendX, 115), at(recvX, 130)); + EditPart messageEP = createConnection(SequenceElementTypes.Sync_Message_Edge, at(sendX, 115), + at(recvX, 130)); - assertThat("Message should be horizontal to receive location", messageEP, runs(sendX, 130, recvX, 130, 2)); + assertThat("Message should be horizontal to receive location", messageEP, + runs(sendX, 130, recvX, 130, 2)); } @Test public void asyncMessageLessThanSlopeThreshold() { - EditPart messageEP = createConnection(SequenceElementTypes.Async_Message_Edge, at(sendX, 115), at(recvX, 119)); + EditPart messageEP = createConnection(SequenceElementTypes.Async_Message_Edge, at(sendX, 115), + at(recvX, 119)); assertThat("Message should be horizontal", messageEP, isHorizontal()); } @@ -172,8 +188,8 @@ public void backwardSlopeFeedback_disallowed() { @Parameters(name = "{1}") public static Iterable parameters() { return Arrays.asList(new Object[][] { // - { false, "left-to-right" }, // - { true, "right-to-left" }, // + {false, "left-to-right" }, // + {true, "right-to-left" }, // }); } } diff --git a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/LifelineSwitchingUITest.java b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/LifelineSwitchingUITest.java index 5f48f49d..82500a50 100644 --- a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/LifelineSwitchingUITest.java +++ b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/LifelineSwitchingUITest.java @@ -46,6 +46,7 @@ import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.parts.DestructionSpecificationEditPart; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.parts.ExecutionSpecificationEditPart; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.providers.SequenceElementTypes; +import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.matchers.GEFMatchers; import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.rules.LightweightSeqDPrefs; import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.rules.Maximized; import org.eclipse.papyrus.uml.interaction.internal.model.SequenceDiagramPackage; @@ -65,6 +66,7 @@ import org.junit.ClassRule; import org.junit.Test; import org.junit.experimental.runners.Enclosed; +import org.junit.rules.TestRule; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -73,12 +75,15 @@ * * @author Christian W. Damus */ -@SuppressWarnings("restriction") +@SuppressWarnings({"restriction", "hiding" }) @ModelResource("three-lifelines.di") @Maximized @RunWith(Enclosed.class) public class LifelineSwitchingUITest extends AbstractGraphicalEditPolicyUITest { + @ClassRule + public static TestRule tolerance = GEFMatchers.defaultTolerance(1); + // Horizontal position of the first lifeline's body private static final int LIFELINE_1_BODY_X = 121; @@ -216,7 +221,7 @@ protected void switchLifeline(VerificationMode mode) { // Verify the new visuals mode.verify(getExecutionEditPart(), - isBounded(isNear(getNewRecvX(), 5), isNear(getNewRecvY()), anything(), anything())); + isBounded(isNear(getNewExecX()), isNear(getNewRecvY()), anything(), anything())); // And the semantics MExecution exec = requireExecution(); @@ -267,6 +272,9 @@ int getNewRecvX() { return super.getNewRecvX() - (EXEC_WIDTH / 2); } + int getNewExecX() { + return getNewRecvX(); + } } public static class MessageSelfNoReply extends MessageNoReply { @@ -278,7 +286,7 @@ public int getReleaseX() { @Override public int getNewRecvX() { - return sendX; + return sendX + (EXEC_WIDTH / 2); } @Override @@ -286,6 +294,12 @@ public int getNewRecvY() { return mesgY + 20; } + @Override + int getNewExecX() { + // In this case, the message attaches on the right side, not the left, so account for that + return super.getNewExecX() - EXEC_WIDTH; + } + @Override Optional getReceiver() { return getSender(); @@ -341,26 +355,6 @@ EditPart getReplyEditPart() { public static class MessageSelfWithReply extends MessageNoExecution { - @Override - public int getReleaseX() { - return sendX; - } - - @Override - public int getNewRecvY() { - return mesgY + 20; - } - - @Override - Optional getReceiver() { - return getSender(); - } - - @Override - MLifeline requireReceiver() { - return requireLifeline("Lifeline1"); - } - @ClassRule public static LightweightSeqDPrefs prefs = new LightweightSeqDPrefs().createRepliesForSyncCalls(); @@ -420,6 +414,21 @@ EditPart getExecutionEditPart() { return assuming(editPart, "No execution edit-part created"); } + @Override + public int getReleaseX() { + return sendX; + } + + @Override + public int getNewRecvY() { + return mesgY + 20; + } + + @Override + Optional getReceiver() { + return getSender(); + } + @Override int getGrabX() { return super.getGrabX() - (EXEC_WIDTH / 2); @@ -492,11 +501,6 @@ int getNewRecvX() { return LIFELINE_2_BODY_X - (EXEC_WIDTH / 2); } - @Override - MLifeline requireReceiver() { - return requireLifeline("Lifeline2"); - } - @Override Optional getReceiver() { return getLifeline("Lifeline2"); @@ -512,11 +516,6 @@ Optional getSender() { return getLifeline("Lifeline1"); } - @Override - MLifeline requireOriginalReceiver() { - return requireLifeline("Lifeline1"); - } - @Override protected void switchLifeline(VerificationMode mode) { super.switchLifeline(ASSERT); @@ -866,7 +865,7 @@ public void createMessage() { // The connections all exist already. Just locate the request message MMessage request = requireMessage("request"); messageEP = requireEditPart(request); - assumeThat(messageEP, runs(sendX, mesgY, getGrabX(), mesgY)); + assumeThat(messageEP, runs(sendX, mesgY, getGrabX(), mesgY, 2)); } @Override @@ -968,11 +967,6 @@ Optional getReceiver() { return getLifeline("Lifeline1"); } - @Override - MLifeline requireReceiver() { - return requireLifeline("Lifeline1"); - } - // // Test framework // @@ -1078,28 +1072,32 @@ MLifeline requireLifeline(String name) { return asserting(getLifeline(name), "No such lifeline: " + name); } + MLifeline requireLifeline(Optional lifeline) { + return asserting(lifeline, "No such lifeline"); + } + Optional getSender() { return getLifeline("Lifeline1"); } - MLifeline requireSender() { - return requireLifeline("Lifeline1"); + final MLifeline requireSender() { + return requireLifeline(getSender()); } Optional getReceiver() { return getLifeline("Lifeline3"); } - MLifeline requireReceiver() { - return requireLifeline("Lifeline3"); + final MLifeline requireReceiver() { + return requireLifeline(getReceiver()); } Optional getOriginalReceiver() { return getLifeline("Lifeline2"); } - MLifeline requireOriginalReceiver() { - return requireLifeline("Lifeline2"); + final MLifeline requireOriginalReceiver() { + return requireLifeline(getOriginalReceiver()); } Optional getExecution() { diff --git a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/MessageExecutionUITest.java b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/MessageExecutionUITest.java index 52d6da75..a0c768b5 100644 --- a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/MessageExecutionUITest.java +++ b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/MessageExecutionUITest.java @@ -29,6 +29,7 @@ import org.eclipse.gef.EditPart; import org.eclipse.papyrus.infra.gmfdiag.common.utils.DiagramEditPartsUtil; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.providers.SequenceElementTypes; +import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.matchers.GEFMatchers; import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.rules.LightweightSeqDPrefs; import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.rules.Maximized; import org.eclipse.papyrus.uml.interaction.tests.rules.ModelResource; @@ -39,14 +40,13 @@ import org.junit.Before; import org.junit.ClassRule; import org.junit.Test; +import org.junit.rules.TestRule; /** - * Integration test cases for disconnection of messages from execution - * specification start/finish. + * Integration test cases for disconnection of messages from execution specification start/finish. * * @author Christian W. Damus */ -@SuppressWarnings("restriction") @ModelResource("two-lifelines.di") @Maximized public class MessageExecutionUITest extends AbstractGraphicalEditPolicyUITest { @@ -55,19 +55,25 @@ public class MessageExecutionUITest extends AbstractGraphicalEditPolicyUITest { public static LightweightSeqDPrefs prefs = new LightweightSeqDPrefs().createExecutionsForSyncMessages() .createRepliesForSyncCalls(); + @ClassRule + public static TestRule tolerance = GEFMatchers.defaultTolerance(1); + // Horizontal position of the first lifeline's body private static final int LL1_BODY_X = 121; // Horizontal position of the second lifeline's body private static final int LL2_BODY_X = 281; - private static final int EXEC_WIDTH = 10; - private EditPart requestEP; + private Message request; + private EditPart execEP; + private ExecutionSpecification exec; + private EditPart replyEP; + private Message reply; /** @@ -146,8 +152,8 @@ public void moveExecutionStart() { editor.drag(grabAt, dropStart); - assertThat("Request message not moved", requestEP, runs(LL1_BODY_X, dropStart.y, - LL2_BODY_X - (EXEC_WIDTH / 2), dropStart.y, RESIZE_TOLERANCE)); + assertThat("Request message not moved", requestEP, + runs(LL1_BODY_X, dropStart.y, LL2_BODY_X - (EXEC_WIDTH / 2), dropStart.y, RESIZE_TOLERANCE)); int oldBottom = execBounds.bottom(); execBounds.setY(dropStart.y); @@ -171,8 +177,7 @@ public void moveExecutionFinish() { LL1_BODY_X, dropFinish.y, RESIZE_TOLERANCE)); execBounds.setHeight(dropFinish.y - execBounds.y); - assertThat("Execution not moved or resized", execEP, - isBounded(isRect(execBounds, RESIZE_TOLERANCE))); + assertThat("Execution not moved or resized", execEP, isBounded(isRect(execBounds, RESIZE_TOLERANCE))); } @Test @@ -197,8 +202,7 @@ public void disconnectExecutionStart() { assertThat("Execution not moved/resized", execEP, isBounded(isRect(execBounds, RESIZE_TOLERANCE))); MessageEnd requestRecv = request.getReceiveEvent(); - assertThat("Incorrect semantic ordering", exec.getStart(), - editor.semanticallyPrecedes(requestRecv)); + assertThat("Incorrect semantic ordering", exec.getStart(), editor.semanticallyPrecedes(requestRecv)); } @Test @@ -218,8 +222,7 @@ public void disconnectExecutionFinish() { runs(LL2_BODY_X - (EXEC_WIDTH / 2), msgY, LL1_BODY_X, msgY, RESIZE_TOLERANCE)); execBounds.setHeight(dropFinish.y - execBounds.y); - assertThat("Execution not moved or resized", execEP, - isBounded(isRect(execBounds, RESIZE_TOLERANCE))); + assertThat("Execution not moved or resized", execEP, isBounded(isRect(execBounds, RESIZE_TOLERANCE))); MessageEnd replySend = reply.getSendEvent(); assertThat("Incorrect semantic ordering", exec.getFinish(), editor.semanticallyFollows(replySend)); @@ -235,25 +238,23 @@ public void createSyncMessage() { at(LL2_BODY_X, 200)); assumeThat("Request message not created", requestEP, notNullValue()); - request = (Message) requestEP.getAdapter(EObject.class); + request = (Message)requestEP.getAdapter(EObject.class); - exec = ((OccurrenceSpecification) request.getReceiveEvent()).getCovered().getCoveredBys().stream() + exec = ((OccurrenceSpecification)request.getReceiveEvent()).getCovered().getCoveredBys().stream() .filter(ExecutionSpecification.class::isInstance).map(ExecutionSpecification.class::cast) .findFirst().orElse(null); - execEP = (exec == null) - ? null + execEP = (exec == null) ? null : DiagramEditPartsUtil.getChildByEObject(exec, editor.getDiagramEditPart(), false); assumeThat("Execution not created", execEP, notNullValue()); - reply = ((MessageEnd) exec.getFinish()).getMessage(); - replyEP = (reply == null) - ? null + reply = ((MessageEnd)exec.getFinish()).getMessage(); + replyEP = (reply == null) ? null : DiagramEditPartsUtil.getChildByEObject(reply, editor.getDiagramEditPart(), true); assumeThat("Reply message not created", replyEP, notNullValue()); } static Point getMessageGrabPoint(EditPart editPart) { - Connection connection = (Connection) ((ConnectionEditPart) editPart).getFigure(); + Connection connection = (Connection)((ConnectionEditPart)editPart).getFigure(); return connection.getPoints().getMidpoint(); } } diff --git a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/MessageReconnectionUITest.java b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/MessageReconnectionUITest.java index c792a80f..0586755c 100644 --- a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/MessageReconnectionUITest.java +++ b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/MessageReconnectionUITest.java @@ -22,30 +22,35 @@ import org.eclipse.gef.EditPart; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies.LifelineBodyGraphicalNodeEditPolicy; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.providers.SequenceElementTypes; +import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.matchers.GEFMatchers; import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.rules.LightweightSeqDPrefs; import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.rules.Maximized; import org.eclipse.papyrus.uml.interaction.tests.rules.ModelResource; import org.junit.ClassRule; import org.junit.Test; +import org.junit.rules.TestRule; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameters; /** - * Integration test cases for the {@link LifelineBodyGraphicalNodeEditPolicy} - * class's message re-connection behaviour. + * Integration test cases for the {@link LifelineBodyGraphicalNodeEditPolicy} class's message re-connection + * behaviour. * * @author Christian W. Damus */ -@SuppressWarnings("restriction") @ModelResource("two-lifelines.di") @Maximized @RunWith(Parameterized.class) public class MessageReconnectionUITest extends AbstractGraphicalEditPolicyUITest { - + + @ClassRule + public static LightweightSeqDPrefs prefs = new LightweightSeqDPrefs() + .dontCreateExecutionsForSyncMessages(); + @ClassRule - public static LightweightSeqDPrefs prefs = new LightweightSeqDPrefs().dontCreateExecutionsForSyncMessages(); - + public static TestRule tolerance = GEFMatchers.defaultTolerance(1); + // Horizontal position of the first lifeline's body private static final int LIFELINE_1_BODY_X = 121; @@ -64,6 +69,19 @@ public class MessageReconnectionUITest extends AbstractGraphicalEditPolicyUITest /** * Initializes me. + * + * @param rightToLeft + * whether to draw the message from right to left (otherwise, left to right) + * @param direction + * human-presentable representation of the {@code rightToLeft} parameter + * @param moveSource + * whether the move the source end of the message (otherwise, the target) + * @param whichEnd + * human-presentable representation of the {@code moveSource} parameter + * @param rightToLeft + * whether to move the message end down in the diagram (otherwise, up) + * @param whichWay + * human-presentable representation of the {@code moveDown} parameter */ public MessageReconnectionUITest(boolean rightToLeft, String direction, boolean moveSource, String whichEnd, boolean moveDown, String whichWay) { @@ -134,8 +152,7 @@ public void moveSyncEnd() { editor.moveSelection(at(x, INITIAL_Y), at(x, y)); - assertThat("Was able to move sync message end", messageEP, - runs(sendX, INITIAL_Y, recvX, INITIAL_Y)); + assertThat("Was able to move sync message end", messageEP, runs(sendX, INITIAL_Y, recvX, INITIAL_Y)); } // @@ -145,14 +162,14 @@ public void moveSyncEnd() { @Parameters(name = "{1}, {3}, {5}") public static Iterable parameters() { return Arrays.asList(new Object[][] { // - { false, "left-to-right", false, "target", false, "up" }, // - { false, "left-to-right", false, "target", true, "down" }, // - { false, "left-to-right", true, "source", false, "up" }, // - { false, "left-to-right", true, "source", true, "down" }, // - { true, "right-to-left", false, "target", false, "up" }, // - { true, "right-to-left", false, "target", true, "down" }, // - { true, "right-to-left", true, "source", false, "up" }, // - { true, "right-to-left", true, "source", true, "down" }, // + {false, "left-to-right", false, "target", false, "up" }, // + {false, "left-to-right", false, "target", true, "down" }, // + {false, "left-to-right", true, "source", false, "up" }, // + {false, "left-to-right", true, "source", true, "down" }, // + {true, "right-to-left", false, "target", false, "up" }, // + {true, "right-to-left", false, "target", true, "down" }, // + {true, "right-to-left", true, "source", false, "up" }, // + {true, "right-to-left", true, "source", true, "down" }, // }); } diff --git a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/SelfMessageCreationUITest.java b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/SelfMessageCreationUITest.java index bf0d0b60..c908616b 100644 --- a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/SelfMessageCreationUITest.java +++ b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/SelfMessageCreationUITest.java @@ -19,6 +19,7 @@ import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assume.assumeThat; import java.util.Arrays; @@ -30,6 +31,7 @@ import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.parts.ExecutionSpecificationEditPart; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.parts.MessageEditPart; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.providers.SequenceElementTypes; +import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.matchers.GEFMatchers; import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.rules.LightweightSeqDPrefs; import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.rules.Maximized; import org.eclipse.papyrus.uml.diagram.sequence.runtime.util.MessageUtil; @@ -39,6 +41,7 @@ import org.junit.Before; import org.junit.ClassRule; import org.junit.Test; +import org.junit.rules.TestRule; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameters; @@ -57,6 +60,9 @@ public class SelfMessageCreationUITest extends AbstractGraphicalEditPolicyUITest public static LightweightSeqDPrefs prefs = new LightweightSeqDPrefs() .dontCreateExecutionsForSyncMessages(); + @ClassRule + public static TestRule tolerance = GEFMatchers.defaultTolerance(1); + // Horizontal position of the first lifeline's body private static final int LIFELINE_1_BODY_X = 121; @@ -132,6 +138,12 @@ private void doCreateSelfMessage() { } break; default: + if (messageEP == null) { + assumeThat("Should fail to get a message edit-part only for create message", messageSort, + equalTo(MessageSort.CREATE_MESSAGE_LITERAL)); + return; // Unreachable + } + if (mode != CreationMode.WITH_EXECUTION) { assertThat(messageEP, runs(x(), top(), x(), bottom(), 2)); } else { From 26342bd1566545dbd242f7a3f44fcf0ebbb4f493 Mon Sep 17 00:00:00 2001 From: "Christian W. Damus" Date: Thu, 27 Dec 2018 12:05:52 -0500 Subject: [PATCH 3/4] Issue #389: Fix test assumption violation on Linux Fix the remaining test assumption violation that occurs on Linux (not on Mac), including on the build server, which happened to mask a test failure that signals an actual regression in the shape of self-messages. Signed-off-by: Christian W. Damus --- .../tests/LifelineSwitchingUITest.java | 65 ++++++++++++++++--- 1 file changed, 56 insertions(+), 9 deletions(-) diff --git a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/LifelineSwitchingUITest.java b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/LifelineSwitchingUITest.java index 82500a50..32ea6164 100644 --- a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/LifelineSwitchingUITest.java +++ b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/LifelineSwitchingUITest.java @@ -34,6 +34,7 @@ import org.eclipse.draw2d.geometry.Dimension; import org.eclipse.draw2d.geometry.Point; +import org.eclipse.draw2d.geometry.PointList; import org.eclipse.emf.ecore.EObject; import org.eclipse.gef.ConnectionEditPart; import org.eclipse.gef.EditPart; @@ -47,6 +48,8 @@ import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.parts.ExecutionSpecificationEditPart; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.providers.SequenceElementTypes; import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.matchers.GEFMatchers; +import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.rules.AutoFixture; +import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.rules.AutoFixtureRule; import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.rules.LightweightSeqDPrefs; import org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.rules.Maximized; import org.eclipse.papyrus.uml.interaction.internal.model.SequenceDiagramPackage; @@ -64,6 +67,7 @@ import org.junit.AssumptionViolatedException; import org.junit.Before; import org.junit.ClassRule; +import org.junit.Rule; import org.junit.Test; import org.junit.experimental.runners.Enclosed; import org.junit.rules.TestRule; @@ -789,13 +793,46 @@ IElementType getMessageType() { @ModelResource("execution-busy.di") public static class ExecutionSpanningOccurrences extends MessageNoExecution { - private static final int LIFELINE_4_BODY_X = 596; + @Rule + public final AutoFixtureRule autoFixtures = new AutoFixtureRule(this); - private final int m2Y = 173; + @AutoFixture + private EditPart requestEP; - private final int m3Y = 198; + @AutoFixture + private PointList requestGeom; - private final int m4Y = 223; + @AutoFixture + private EditPart m2EP; + + @AutoFixture + private PointList m2Geom; + + @AutoFixture + private EditPart m3EP; + + @AutoFixture + private PointList m3Geom; + + @AutoFixture + private EditPart m4EP; + + @AutoFixture + private PointList m4Geom; + + @AutoFixture + private EditPart replyEP; + + @AutoFixture + private PointList replyGeom; + + private int LIFELINE_4_BODY_X; + + private int m2Y; + + private int m3Y; + + private int m4Y; @Override protected void switchLifeline(VerificationMode mode) { @@ -863,19 +900,29 @@ protected void undoSwitchLifeline(VerificationMode mode) { @Override public void createMessage() { // The connections all exist already. Just locate the request message - MMessage request = requireMessage("request"); - messageEP = requireEditPart(request); - assumeThat(messageEP, runs(sendX, mesgY, getGrabX(), mesgY, 2)); + messageEP = requestEP; + mesgY = requestGeom.getLastPoint().y(); + + // Get select geometric parameters + LIFELINE_4_BODY_X = m3Geom.getLastPoint().x(); + m2Y = m2Geom.getLastPoint().y(); + m3Y = m3Geom.getLastPoint().y(); + m4Y = m4Geom.getLastPoint().y(); } @Override int getGrabX() { - return super.getGrabX() - (EXEC_WIDTH / 2); + return requestGeom.getLastPoint().x(); + } + + @Override + public int getReleaseX() { + return m2Geom.getLastPoint().x(); } @Override int getNewRecvX() { - return super.getNewRecvX() - (EXEC_WIDTH / 2); + return m2Geom.getLastPoint().x() - (EXEC_WIDTH / 2); } } From 0ca848336a0bc210e2b34d9bdb3173f7ff29c894 Mon Sep 17 00:00:00 2001 From: Remi Schnekenburger Date: Wed, 2 Jan 2019 14:10:37 +0100 Subject: [PATCH 4/4] Issue #389: self message management - simple fixes based on PR review - fix transformation messages to self messages when relevant during move of an execution - Add a similar behavior between moving the end of a sync message starting an execution to another lifeline and the move of the execution itself. Change-Id: I19c9b868f016401fa71a9c5f65c82093fe9ddd8b Signed-off-by: Remi Schnekenburger --- .../ExecutionSpecificationEndAnchor.java | 2 +- .../ExecutionSpecificationSideAnchor.java | 2 +- .../internal/edit/parts/MessageEditPart.java | 9 ++++++++- ...AbstractSequenceGraphicalNodeEditPolicy.java | 10 +++------- .../policies/LifelineBodyDropEditPolicy.java | 17 ++++++++++------- .../model/commands/SetCoveredCommand.java | 11 ++++------- .../model/commands/SetOwnerCommand.java | 16 ++++++++++++++++ .../model/spi/impl/DefaultDiagramHelper.java | 4 ++-- .../tests/ExecutionWithSelfMoveUITest.java | 9 ++++----- .../policies/tests/MessageSnappingUITest.java | 9 +++++++-- 10 files changed, 56 insertions(+), 33 deletions(-) diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/ExecutionSpecificationEndAnchor.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/ExecutionSpecificationEndAnchor.java index 616a1433..1744dde0 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/ExecutionSpecificationEndAnchor.java +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/ExecutionSpecificationEndAnchor.java @@ -40,7 +40,7 @@ public void setConnectionSide(int side) { public Point getReferencePoint() { Rectangle body = getOwner().getBounds().getCopy(); getOwner().translateToAbsolute(body); - return body.getBottomLeft(); + return body.getBottom(); } @Override diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/ExecutionSpecificationSideAnchor.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/ExecutionSpecificationSideAnchor.java index ddc89f47..f0e131d9 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/ExecutionSpecificationSideAnchor.java +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/anchors/ExecutionSpecificationSideAnchor.java @@ -30,7 +30,7 @@ public class ExecutionSpecificationSideAnchor extends AbstractConnectionAnchor i public ExecutionSpecificationSideAnchor(IFigure figure, int height) { super(figure); - this.side = RIGHT; + this.side = RIGHT; // by default, anchors are located on the right. this.height = height; } diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/parts/MessageEditPart.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/parts/MessageEditPart.java index 6da4167f..cc29b786 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/parts/MessageEditPart.java +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/parts/MessageEditPart.java @@ -151,6 +151,9 @@ private void refreshLifeline(MLifeline lifeline) { } private void refreshEditPartOfShape(Shape shape) { + if(getViewer() == null) { + return; + } Object lifelineEditPart = getViewer().getEditPartRegistry().get(shape); if (lifelineEditPart instanceof EditPart) { EditPart editPart = (EditPart)lifelineEditPart; @@ -245,7 +248,11 @@ protected void refreshRouterChange() { @Override protected void handlePropertyChangeEvent(PropertyChangeEvent event) { - + // Avoid calling super, as it only handles the refresh of the routing property. The super method + // refreshes the router based on the routing style of the notation model. In the case of feedback, the + // router may be different (getting from a self message to a normal message for example). So refresh + // should be avoided in this case, and connection router shall be updated on notation model change + // only. The feedback router is handled in the feedback related methods. } @Override diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/AbstractSequenceGraphicalNodeEditPolicy.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/AbstractSequenceGraphicalNodeEditPolicy.java index 957dd461..6ef4e11a 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/AbstractSequenceGraphicalNodeEditPolicy.java +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/AbstractSequenceGraphicalNodeEditPolicy.java @@ -14,7 +14,6 @@ import static java.lang.Math.abs; import static org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies.PrivateRequestUtils.getUpdatedSourceLocation; -import static org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies.PrivateRequestUtils.getUpdatedTargetLocation; import static org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies.PrivateRequestUtils.isAllowSemanticReordering; import static org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies.PrivateRequestUtils.isForce; import static org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.util.CommandUtil.injectViewInto; @@ -479,15 +478,12 @@ protected Command getReconnectSourceCommand(ReconnectRequest request) { OptionalInt yPosition = OptionalInt.of(y); org.eclipse.emf.common.command.Command result = null; + // Check for semantic re-ordering if we're not connecting to an execution - if (!getExecutionFinish(lifeline.get(), request.getLocation()).isPresent() - // Or if we're moving both ends and the other is not connecting to an execution - && !(isForce(request) && message.getReceiver() - .flatMap(rcvr -> getExecutionStart(rcvr, getUpdatedTargetLocation(request))) - .isPresent())) { + if (!getExecutionFinish(lifeline.get(), request.getLocation()).isPresent()) { + // should ensure the execution start is nudged result = getNudgeObstacleCommand(request, message.getSend().get(), y); } - // If the message didn't have a send end, we wouldn't be reconnecting it MMessageEnd sourceEnd = message.getSend().get(); org.eclipse.emf.common.command.Command createFinish = null; diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/LifelineBodyDropEditPolicy.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/LifelineBodyDropEditPolicy.java index d8710fa5..4e9a53e8 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/LifelineBodyDropEditPolicy.java +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/LifelineBodyDropEditPolicy.java @@ -79,21 +79,24 @@ protected Command getDropExecutionCommand(MExecution execution, DropObjectsReque return UnexecutableCommand.INSTANCE; } - Optional changeBounds = Optional.ofNullable(getChangeBoundsRequest(request)) - .map(ChangeBoundsRequest::getMoveDelta); - OptionalInt deltaY = mapToInt(changeBounds, Point::y); - OptionalInt top = flatMap(execution.getTop(), t -> map(deltaY, y -> t + y)); - OptionalInt bottom = flatMap(execution.getBottom(), b -> map(deltaY, y -> b + y)); - // check if there is a change of lifeline, in this case, opt for the start event cover change // this allows for nice handling of self messages. + // note that this forbids the execution to change lifeline and move along the new lifeline if (lifeline.isPresent() && !lifeline.get().equals(execution.getOwner())) { Optional> start = execution.getStart(); if (start.isPresent()) { - return lifeline.map(ll -> wrap(start.get().setCovered(ll, top))).orElse(null); + return lifeline.map(ll -> wrap(start.get().setCovered(ll, execution.getTop()))).orElse(null); } } + + // if not, let the execution move along the line. + Optional changeBounds = Optional.ofNullable(getChangeBoundsRequest(request)) + .map(ChangeBoundsRequest::getMoveDelta); + OptionalInt deltaY = mapToInt(changeBounds, Point::y); + OptionalInt top = flatMap(execution.getTop(), t -> map(deltaY, y -> t + y)); + OptionalInt bottom = flatMap(execution.getBottom(), b -> map(deltaY, y -> b + y)); + // default behavior, use the set owner API. return lifeline.map(ll -> wrap(execution.setOwner(ll, top, bottom))).orElse(null); } diff --git a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/SetCoveredCommand.java b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/SetCoveredCommand.java index 67e29189..59f3ed20 100644 --- a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/SetCoveredCommand.java +++ b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/SetCoveredCommand.java @@ -224,11 +224,11 @@ private boolean isTargetEnd() { } protected boolean isBecomingSelfMessage() { - return isChangingLifeline() && willBeSelfMessage() && !wasSelfMessage(); + return willBeSelfMessage() && !wasSelfMessage(); } protected boolean isBecomingNonSelfMessage() { - return isChangingPosition() && wasSelfMessage() && !willBeSelfMessage(); + return wasSelfMessage() && !willBeSelfMessage(); } protected boolean wasSelfMessage() { @@ -335,13 +335,11 @@ private void collectMessageDependencies(MMessageEnd end, boolean isDisconnecting commandSink.accept(defer( () -> reconnectSource(connector, newAttachedShape.get(), newYPosition).orElse(null))); Optional otherEnd = end.getOtherEnd(); - otherEnd.flatMap(oEnd -> handleSelfMessageChange(oEnd, newYPosition)).ifPresent(commandSink); otherEnd.flatMap(this::handleOppositeSendOrReplyMessage).ifPresent(commandSink); } else if (end.isReceive()) { commandSink.accept(defer( () -> reconnectTarget(connector, newAttachedShape.get(), newYPosition).orElse(null))); Optional otherEnd = end.getOtherEnd(); - otherEnd.flatMap(oEnd -> handleSelfMessageChange(oEnd, newYPosition)).ifPresent(commandSink); otherEnd.flatMap(this::handleOppositeSendOrReplyMessage).ifPresent(commandSink); } // else don't know what to do with it } @@ -373,7 +371,7 @@ private void collectMessageDependencies(MMessageEnd end, boolean isDisconnecting // Track this end final OptionalInt otherNewY; if (end.isStart()) { - if (wasSelfMessage() && willBeSelfMessage()) { + if (!wasSelfMessage() && willBeSelfMessage()) { otherNewY = OptionalInt.of(PendingVerticalExtentData .getPendingTop(end.getExecution().get()).getAsInt() - layoutHelper().getConstraints().getMinimumHeight(ViewTypes.MESSAGE)); @@ -415,7 +413,7 @@ private void collectMessageDependencies(MMessageEnd end, boolean isDisconnecting // only a simple occurrence on the execution. Move using delta if (otherEnd.isReceive()) { // add an additional room for the self message - if (isBecomingSelfMessage() && !wasSelfMessage()) { + if (isBecomingSelfMessage()) { // ensure the position has a least the space for self messages, but it could be bigger int minLayout = yPosition.getAsInt() + layoutHelper().getConstraints().getMinimumHeight(ViewTypes.MESSAGE); @@ -544,7 +542,6 @@ Optional handleSelfMessageChange(MMessageEnd otherEnd, int newYPosition Command bend; if (otherEnd.isSend()) { // We're reconnecting the source end, but we need to maintain the gap - // int newYPosition = currentYPosition - height; Shape newAttachShape = executionShapeAt(lifeline, currentYPosition).orElse(lifelineBody); bend = reconnectSource(connector, newAttachShape, currentYPosition) .orElse(IdentityCommand.INSTANCE); diff --git a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/SetOwnerCommand.java b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/SetOwnerCommand.java index 33be053d..3c78cdbb 100644 --- a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/SetOwnerCommand.java +++ b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/SetOwnerCommand.java @@ -245,6 +245,22 @@ protected Optional dependencies(MExecution execution, MLifeline lifelin // that *will be* spanned do not; they only must be reattached per the step below result = execution.getOccurrences().stream() .map(occ -> occ.setCovered(lifeline, topMapping.apply(occ.getTop()))).reduce(chaining()); + + if (isStartingSelfMessage(execution)) { + // specific case of a self message starting the execution => it should also be moved of delta + final Optional moveInitial = execution.getStart().map(MMessageEnd.class::cast) + .flatMap(MMessageEnd::getOtherEnd) + .map(occ -> occ.setCovered(lifeline, topMapping.apply(occ.getTop()))); + + if (moveInitial.isPresent()) { + if (result.isPresent()) { + result.get().chain(moveInitial.get()); + } else { + result = moveInitial; + } + } + } + } else if (isChangingPosition()) { if (!isChangingOwner()) { // We are reshaping it. Refresh nested executions, if necessary diff --git a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/spi/impl/DefaultDiagramHelper.java b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/spi/impl/DefaultDiagramHelper.java index 03eae68a..33c2b5a9 100644 --- a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/spi/impl/DefaultDiagramHelper.java +++ b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/spi/impl/DefaultDiagramHelper.java @@ -469,7 +469,7 @@ public Command reconnectSource(Connector connector, Shape newSource, int yPositi // should force? if (newRouting != connector.getRouting()) { - result.chain(SetCommand.create(editingDomain, connector, + result = result.chain(SetCommand.create(editingDomain, connector, NotationPackage.Literals.ROUTING_STYLE__ROUTING, newRouting)); } @@ -515,7 +515,7 @@ public Command reconnectTarget(Connector connector, Shape newTarget, int yPositi } // should force? if (newRouting != connector.getRouting()) { - result.chain(SetCommand.create(editingDomain, connector, + result = result.chain(SetCommand.create(editingDomain, connector, NotationPackage.Literals.ROUTING_STYLE__ROUTING, newRouting)); } diff --git a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/ExecutionWithSelfMoveUITest.java b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/ExecutionWithSelfMoveUITest.java index 8d95bf43..098b064c 100644 --- a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/ExecutionWithSelfMoveUITest.java +++ b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/ExecutionWithSelfMoveUITest.java @@ -110,7 +110,7 @@ public void moveDownExecution() { @Test public void moveUpExecution() { - int delta = -50; + int delta = -30; Point grabAt = execution1Rect.getCenter(); Point dropAt = execution1Rect.getCenter().translate(0, delta); @@ -118,15 +118,14 @@ public void moveUpExecution() { // check result Rectangle exec1EPNew = execution1Rect.getCopy().translate(new Point(0, delta)); - EditPart exec1EP = findEditPart("execution-self::Interaction1::Execution1", false); - assertThat(getBounds(exec1EP), equalTo(exec1EPNew)); PointList newRequestPoints = requestPoints.getCopy(); newRequestPoints.translate(0, delta); assertThat(requestEP, runs(exec1EPNew.getCenter().x(), exec1EPNew.getTop().y() - 20, exec1EPNew.getTopRight().x(), exec1EPNew.getTopRight().y())); - PointList newReplyPoints = replyPoints.getCopy(); - newReplyPoints.translate(0, delta); + EditPart exec1EP = findEditPart("execution-self::Interaction1::Execution1", false); + assertThat(getBounds(exec1EP), equalTo(exec1EPNew)); + assertThat(replyEP, runs(exec1EPNew.getBottomRight().x(), exec1EPNew.getBottomRight().y(), exec1EPNew.getCenter().x(), exec1EPNew.getBottom().y() + 20)); } diff --git a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/MessageSnappingUITest.java b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/MessageSnappingUITest.java index 2e6b1daa..0466ff13 100644 --- a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/MessageSnappingUITest.java +++ b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/MessageSnappingUITest.java @@ -14,6 +14,7 @@ import static org.eclipse.papyrus.uml.diagram.sequence.figure.magnets.IMagnetManager.MODIFIER_NO_SNAPPING; import static org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.matchers.GEFMatchers.isPoint; +import static org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.matchers.GEFMatchers.EditParts.isBounded; import static org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.matchers.GEFMatchers.EditParts.runs; import static org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.rules.EditorFixture.at; import static org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.rules.EditorFixture.sized; @@ -184,17 +185,20 @@ public void createReplyMessage() { @Test public void moveSyncCallMessage() { + int execTop = getTop(execEP); + int size = getBottom(execEP) - execTop; EditPart messageEP = createConnection(SequenceElementTypes.Sync_Message_Edge, at(LL1_BODY_X, 120), at(LL2_BODY_X, 120)); Point midMessage = getMessageGrabPoint(messageEP); // The receiving end snaps to the exec start and the sending end matches - editor.with(modifiers, + editor.with(modifiers.and(editor.allowSemanticReordering()), () -> editor.moveSelection(midMessage, at(midMessage.x(), withinMagnet(EXEC_START)))); Point newMessageMidpoint = getMessageGrabPoint(messageEP); assumeThat("Message not moved", newMessageMidpoint, not(isPoint(midMessage.x(), midMessage.y(), 5))); - int execTop = getTop(execEP); + // check exec position + assertThat(execEP, isBounded(left(LL2_BODY_X), execTop, EXEC_WIDTH, size)); assertThat(messageEP, withModifiers(runs(LL1_BODY_X, execTop, left(LL2_BODY_X), execTop, 1))); } @@ -205,6 +209,7 @@ public void moveReplyMessage() { Point midMessage = getMessageGrabPoint(messageEP); // The sending end snaps to the exec start and the receiving end matches + // only if the semantic reordering is activated. Otherwise, it will only push down the exec spec editor.with(modifiers, () -> editor.moveSelection(midMessage, at(midMessage.x(), withinMagnet(EXEC_FINISH)))); int execBottom = getBottom(execEP);