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..2f4072bb 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; @@ -42,6 +41,7 @@ import org.eclipse.draw2d.geometry.Point; import org.eclipse.emf.common.command.CommandWrapper; import org.eclipse.emf.ecore.EClass; +import org.eclipse.emf.ecore.EObject; import org.eclipse.emf.transaction.TransactionalEditingDomain; import org.eclipse.gef.EditPart; import org.eclipse.gef.LayerConstants; @@ -479,15 +479,17 @@ 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())) { + // 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()) + */) { + // 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; @@ -573,6 +575,16 @@ protected Command getReconnectTargetCommand(ReconnectRequest request) { && (lifeline.get().getElement() != message.getSender().get().getElement())) { // Doesn't matter where the mouse pointer is y = flatMapToInt(message.getSend(), MElement::getTop).orElse(y); + } else if (isForce(request)) { + Optional sourceLL = message.getSender().map(MElement::getElement); + Optional targetLL = message.getReceiver().map(MElement::getElement); + if (sourceLL.isPresent() && sourceLL.equals(targetLL)) { + // this is a self message, target position should be the new source position + the current + // height of message + int bottom = message.getBottom().getAsInt(); + int top = message.getTop().getAsInt(); + y = y + 0; + } } OptionalInt yPosition = OptionalInt.of(y); 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..cdaad916 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,15 @@ 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(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(oEnd -> handleSelfMessageChange(oEnd, + // newYPosition)).ifPresent(commandSink); otherEnd.flatMap(this::handleOppositeSendOrReplyMessage).ifPresent(commandSink); } // else don't know what to do with it } @@ -373,7 +375,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 +417,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 +546,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);