From 48c8ae13e48a069d95806504583b134672054ebe Mon Sep 17 00:00:00 2001 From: "Christian W. Damus" Date: Wed, 5 Sep 2018 09:10:24 -0400 Subject: [PATCH 01/11] Issue #73 Execution start/finish on message ends Update alignment constraints and feed-back of message creation to enforce horizontality of non-asynchronous messages by recalculating the sending end instead of the receiving end because the receiving end is generally more significant. Implement a magnet mechanism to snap connections to the ends of an execution specification. Upon creation of a message connecting to either end of an execution specification, replace the occurrence at that end by the message end. Register a palette provider that injects magnet manager support into the selection tool and connection creation tools to support user override of the magnet snapping behaviour when drawing and moving connections, using the same keyboard modifier as for suppression of the snapping of shapes. Update a create message test to account for messages anchored on the beginning of an execution specification correctly remaining attached when the execution moves. Signed-off-by: Christian W. Damus # Conflicts: # plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/InsertMessageCommand.java # plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/spi/impl/DefaultDiagramHelper.java # plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/model/spi/DiagramHelper.java --- .../plugin.xml | 18 + .../META-INF/MANIFEST.MF | 3 +- .../figure/magnets/DefaultMagnet.java | 64 ++++ .../sequence/figure/magnets/IMagnet.java | 77 +++++ .../figure/magnets/IMagnetManager.java | 101 ++++++ .../figure/magnets/MagnetManager.java | 121 +++++++ .../plugin.xml | 10 + .../parts/ExecutionSpecificationEditPart.java | 55 ++- .../edit/parts/ISequenceEditPart.java | 45 +++ .../edit/parts/InteractionEditPart.java | 18 + .../edit/parts/LifelineBodyEditPart.java | 17 + ...stractSequenceGraphicalNodeEditPolicy.java | 78 +++-- .../edit/policies/ISequenceEditPolicy.java | 38 ++- .../policies/MessageEndpointEditPolicy.java | 42 ++- .../edit/policies/MessageFeedbackHelper.java | 177 ++++++---- .../SequenceEditPartAdapterFactory.java | 85 +++++ .../providers/SequencePaletteProvider.java | 131 +++++++ .../runtime/internal/tools/MagnetSupport.java | 77 +++++ .../tools/SequenceConnectionCreationTool.java | 51 +++ .../tools/SequencePaletteFactory.java | 55 +++ .../internal/tools/SequenceSelectionTool.java | 45 +++ .../model/commands/InsertMessageCommand.java | 119 ++++++- .../model/spi/impl/AnchorFactory.java | 245 +++++++++++++ .../model/spi/impl/DefaultDiagramHelper.java | 72 ++-- .../spi/impl/DefaultLayoutConstraints.java | 5 + .../model/spi/impl/DefaultLayoutHelper.java | 7 + .../interaction/model/spi/DiagramHelper.java | 10 +- .../model/spi/LayoutConstraints.java | 9 + .../interaction/model/spi/LayoutHelper.java | 10 + .../META-INF/MANIFEST.MF | 3 +- ...lineBodyGraphicalNodeEditPolicyUITest.java | 3 +- .../policies/tests/MessageSnappingUITest.java | 323 ++++++++++++++++++ .../runtime/tests/rules/EditorFixture.java | 204 ++++++++++- .../tests/creation/CreateMessageTest.java | 14 +- 34 files changed, 2155 insertions(+), 177 deletions(-) create mode 100644 plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/magnets/DefaultMagnet.java create mode 100644 plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/magnets/IMagnet.java create mode 100644 plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/magnets/IMagnetManager.java create mode 100644 plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/magnets/MagnetManager.java create mode 100644 plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/factories/SequenceEditPartAdapterFactory.java create mode 100644 plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/providers/SequencePaletteProvider.java create mode 100644 plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/tools/MagnetSupport.java create mode 100644 plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/tools/SequenceConnectionCreationTool.java create mode 100644 plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/tools/SequencePaletteFactory.java create mode 100644 plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/tools/SequenceSelectionTool.java create mode 100644 plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/spi/impl/AnchorFactory.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/MessageSnappingUITest.java diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.contribution/plugin.xml b/plugins/org.eclipse.papyrus.uml.diagram.sequence.contribution/plugin.xml index 8860fd8f..19e5adf3 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.contribution/plugin.xml +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.contribution/plugin.xml @@ -124,4 +124,22 @@ + + + + + + + + + + + + + diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/META-INF/MANIFEST.MF b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/META-INF/MANIFEST.MF index 57a022ac..5bb70799 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/META-INF/MANIFEST.MF +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/META-INF/MANIFEST.MF @@ -9,7 +9,8 @@ Bundle-Activator: org.eclipse.papyrus.uml.diagram.sequence.figure.internal.Activ Export-Package: org.eclipse.papyrus.uml.diagram.sequence.figure, org.eclipse.papyrus.uml.diagram.sequence.figure.anchors, org.eclipse.papyrus.uml.diagram.sequence.figure.border, - org.eclipse.papyrus.uml.diagram.sequence.figure.internal;x-internal:=true + org.eclipse.papyrus.uml.diagram.sequence.figure.internal;x-internal:=true, + org.eclipse.papyrus.uml.diagram.sequence.figure.magnets Require-Bundle: org.eclipse.core.runtime;bundle-version="[3.13.0,4.0.0)", org.eclipse.draw2d;bundle-version="[3.10.0,4.0.0)", org.eclipse.gmf.runtime.draw2d.ui;bundle-version="[1.9.0,2.0.0)", diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/magnets/DefaultMagnet.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/magnets/DefaultMagnet.java new file mode 100644 index 00000000..95f648cf --- /dev/null +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/magnets/DefaultMagnet.java @@ -0,0 +1,64 @@ +/***************************************************************************** + * Copyright (c) 2018 Christian W. Damus and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Christian W. Damus - Initial API and implementation + *****************************************************************************/ + +package org.eclipse.papyrus.uml.diagram.sequence.figure.magnets; + +import org.eclipse.draw2d.geometry.Point; + +/** + * A simple implementation of the {@linkplain IMagnet magnet} protocol. + */ +public class DefaultMagnet implements IMagnet { + + private final int strength; + + private Point location; + + /** + * Initializes me with my {@code strength}. + * + * @param strength + * my strength + */ + public DefaultMagnet(int strength) { + super(); + + this.strength = strength; + } + + @Override + public final int getStrength() { + return strength; + } + + @Override + public void setLocation(Point location) { + this.location = location; + } + + @Override + public Point getLocation() { + return location; + } + + @Override + public boolean influences(@SuppressWarnings("hiding") Point location) { + return (this.location != null) && (this.location.getDistance(location) <= 1.5 * strength); + } + + @SuppressWarnings("boxing") + @Override + public String toString() { + return String.format("DefaultMagnet at (%s, %s), strength %s", //$NON-NLS-1$ + location.preciseX(), location.preciseY(), strength); + } +} diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/magnets/IMagnet.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/magnets/IMagnet.java new file mode 100644 index 00000000..c2333171 --- /dev/null +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/magnets/IMagnet.java @@ -0,0 +1,77 @@ +/***************************************************************************** + * Copyright (c) 2018 Christian W. Damus and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Christian W. Damus - Initial API and implementation + *****************************************************************************/ + +package org.eclipse.papyrus.uml.diagram.sequence.figure.magnets; + +import org.eclipse.draw2d.geometry.Point; +import org.eclipse.draw2d.geometry.Rectangle; + +/** + * Protocol of a magnet that draws objects towards it (connection bendpoints and endpoints, shapes) + * that are being dragged about by the mouse pointer. + */ +public interface IMagnet { + /** + * Obtain the magnet's strength, which is the square radius within which it captures the mouse pointer. + * + * @return a positive strength, in pixels + */ + int getStrength(); + + /** + * Queries whether a {@code location} is within my {@linkplain #getStrength() square radius}. + * + * @param location + * a location in absolute coördinates + * @return whether I capture the {@code location} + */ + default boolean captures(Point location) { + Point self = getLocation(); + int strength = getStrength(); + int x = self.x - strength; + int y = self.y - strength; + int size = strength << 1; + + Rectangle.SINGLETON.setBounds(x, y, size, size); + + return Rectangle.SINGLETON.contains(location); + } + + /** + * Query the location of the magnet, in absolute coördinates. + * + * @return the location + */ + Point getLocation(); + + /** + * Update (move) the location of the magnet. + * + * @param location + * the new location of the magnet + */ + void setLocation(Point location); + + /** + * Determine whether a {@code location} feels my magnetic influence. This does not necessarily mean that I + * {@link #captures(Point) capture} the mouse pointer from that {@code location}. My influence could + * extend beyond my {@link #getStrength() strength}. + * + * @param location + * a location + * @return whether the {@code location} feels my influence + * @see #getStrength() + */ + boolean influences(Point location); + + // TODO: API for showing and hiding feedback (with a secondary radius within which it is activated?) +} diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/magnets/IMagnetManager.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/magnets/IMagnetManager.java new file mode 100644 index 00000000..5efb6211 --- /dev/null +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/magnets/IMagnetManager.java @@ -0,0 +1,101 @@ +/***************************************************************************** + * Copyright (c) 2018 Christian W. Damus and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Christian W. Damus - Initial API and implementation + *****************************************************************************/ + +package org.eclipse.papyrus.uml.diagram.sequence.figure.magnets; + +import java.util.Optional; +import java.util.stream.Stream; + +import org.eclipse.core.runtime.Adapters; +import org.eclipse.core.runtime.Platform; +import org.eclipse.draw2d.geometry.Point; +import org.eclipse.gef.SnapToHelper; +import org.eclipse.gef.tools.AbstractTool; +import org.eclipse.swt.SWT; + +/** + * Protocol for management of {@link IMagnet}s on a shape in the diagram editor to which connection ends snap. + * Note that we do not use this for snapping of shapes, as GEF already provides support for that via the + * {@link SnapToHelper} API which does not deal with connections but only shapes. + */ +public interface IMagnetManager { + + /** + * Key modifier for ignoring snap while dragging. It's CTRL on Mac and ALT on all other + * platforms, as defined in the GEF {@link AbstractTool} class. + */ + int MODIFIER_NO_SNAPPING = Platform.OS_MACOSX.equals(Platform.getOS()) ? SWT.CTRL : SWT.ALT; + + /** + * Add a {@code magnet} to the diagram editor. + * + * @param magnet + * the magnet to add + */ + void addMagnet(IMagnet magnet); + + /** + * Remove a {@code magnet} from the diagram editor. + * + * @param magnet + * the magnet to remove + */ + void removeMagnet(IMagnet magnet); + + /** + * Find the magnets within whose influence the given {@code location} lies, in increasing order of + * distance from that location (and so decreasing order of influence). + * + * @param location + * a location in the diagram, in absolute coördinates + * @return the magnets whose influence is felt at that {@code location} + */ + Stream getMagnets(Point location); + + /** + * Find the magnet that captures a {@code location}. + * + * @param location + * a location, in absolute coördinates + * @return the magnet that captures the {@code location} + */ + Optional getCapturingMagnet(Point location); + + /** + * Queries whether the magnet manager is currently enabled. When it is not enabled, then snapping + * connections to magnets does not happen. + * + * @return whether the magnet manager is enabled + */ + boolean isEnabled(); + + /** + * Get the magnet manager for an {@code object}, usually an edit-part or figure. There should be exactly + * one magnet manager for the diagram, maintained by a root-ish edit-part or figure, which always exists. + * + * @param object + * an object in the diagram + * @return its magnet manager. Never {@code null} + * @throws IllegalStateException + * if the {@code object} does not trace to some magnet manager + */ + static IMagnetManager get(Object object) { + IMagnetManager result = Adapters.adapt(object, IMagnetManager.class); + + if (result == null) { + throw new IllegalStateException("object has no magnet manager: " + object); //$NON-NLS-1$ + } + + return result; + } + +} diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/magnets/MagnetManager.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/magnets/MagnetManager.java new file mode 100644 index 00000000..06dca195 --- /dev/null +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/magnets/MagnetManager.java @@ -0,0 +1,121 @@ +/***************************************************************************** + * Copyright (c) 2018 Christian W. Damus and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Christian W. Damus - Initial API and implementation + *****************************************************************************/ + +package org.eclipse.papyrus.uml.diagram.sequence.figure.magnets; + +import java.util.ArrayList; +import java.util.Comparator; +import java.util.List; +import java.util.Optional; +import java.util.function.Predicate; +import java.util.function.ToDoubleFunction; +import java.util.stream.Stream; + +import org.eclipse.draw2d.geometry.Point; +import org.eclipse.gef.tools.AbstractTool.Input; + +/** + * A manager of {@link IMagnet}s. + */ +public class MagnetManager implements IMagnetManager { + + // If scale becomes a problem, we may need a quad-tree or some such + private final List magnets = new ArrayList<>(); + + private Input currentInput; + + /** + * Initializes me. + */ + public MagnetManager() { + super(); + } + + @Override + public void addMagnet(IMagnet magnet) { + magnets.add(magnet); + } + + @Override + public void removeMagnet(IMagnet magnet) { + magnets.remove(magnet); + } + + /** + * Find the magnets within whose influence the given {@code location} lies, in increasing order of + * distance from that location (and so decreasing order of influence). + * + * @param location + * a location in the diagram, in absolute coördinates + * @return the magnets whose influence is felt at that {@code location} + */ + @Override + public Stream getMagnets(Point location) { + if (!isEnabled()) { + return Stream.empty(); + } + + return magnets.stream().filter(influences(location)) + .sorted(Comparator.comparingDouble(distance(location))); + } + + /** + * Find the magnet that captures a {@code location}. + * + * @param location + * a location, in absolute coördinates + * @return the magnet that captures the {@code location} + */ + @Override + public Optional getCapturingMagnet(Point location) { + // The first magnet is the nearest, and if it isn't capturing, then none is + return getMagnets(location).findFirst().filter(captures(location)); + } + + @Override + public boolean isEnabled() { + Input input = getCurrentInput(); + return input == null || !input.isModKeyDown(MODIFIER_NO_SNAPPING); + } + + /** + * @return the currentInput + */ + public Input getCurrentInput() { + return currentInput; + } + + /** + * @param currentInput + * the currentInput to set + */ + public void setCurrentInput(Input currentInput) { + this.currentInput = currentInput; + } + + private Predicate influences(Point location) { + return magnet -> magnet.influences(location); + } + + private Predicate captures(Point location) { + return magnet -> magnet.captures(location); + } + + private ToDoubleFunction distance(Point location) { + return magnet -> magnet.getLocation().getDistance(location); + } + + public static Optional get(Object object) { + return Optional.ofNullable(IMagnetManager.get(object)).filter(MagnetManager.class::isInstance) + .map(MagnetManager.class::cast); + } +} diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/plugin.xml b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/plugin.xml index ad6c1e95..359fe1c1 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/plugin.xml +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/plugin.xml @@ -26,4 +26,14 @@ class="org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.preferences.LightweightSequenceDiagramPreferenceInitializer"> + + + + + + 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 0d560b34..c8dbc362 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 @@ -18,6 +18,8 @@ import java.util.stream.Stream; import org.eclipse.draw2d.IFigure; +import org.eclipse.draw2d.geometry.Point; +import org.eclipse.draw2d.geometry.Rectangle; import org.eclipse.emf.common.notify.Notification; import org.eclipse.emf.ecore.EObject; import org.eclipse.emf.ecore.util.EcoreUtil; @@ -31,6 +33,8 @@ import org.eclipse.gmf.runtime.notation.Shape; import org.eclipse.gmf.runtime.notation.View; import org.eclipse.papyrus.uml.diagram.sequence.figure.ExecutionSpecificationFigure; +import org.eclipse.papyrus.uml.diagram.sequence.figure.magnets.DefaultMagnet; +import org.eclipse.papyrus.uml.diagram.sequence.figure.magnets.IMagnetManager; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies.ExecutionSpecificationGraphicalNodeEditPolicy; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies.InteractionSemanticEditPolicy; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies.ResizableBorderItemPolicy; @@ -43,12 +47,32 @@ * This also means that by default, it won't listen to its bounds, and won't refresh properly * when org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies.ResizableBorderItemPolicy is used. */ -public class ExecutionSpecificationEditPart extends BorderedBorderItemEditPart { +public class ExecutionSpecificationEditPart extends BorderedBorderItemEditPart implements ISequenceEditPart { + + private DefaultMagnet startMagnet; + + private DefaultMagnet finishMagnet; public ExecutionSpecificationEditPart(View view) { super(view); } + @Override + public void deactivate() { + IMagnetManager mgr = IMagnetManager.get(this); + + if (startMagnet != null) { + mgr.removeMagnet(startMagnet); + startMagnet = null; + } + if (finishMagnet != null) { + mgr.removeMagnet(finishMagnet); + finishMagnet = null; + } + + super.deactivate(); + } + @Override protected NodeFigure createMainFigure() { NodeFigure nodeFigure = new ExecutionSpecificationFigure(); @@ -71,6 +95,8 @@ protected void refreshBounds() { if (locator != null) { locator.relocate(getFigure()); } + + updateMagnets(); // super.refreshBounds(); } @@ -123,4 +149,31 @@ private void refreshEditPartOfShape(Shape shape) { } } + protected void updateMagnets() { + IMagnetManager mgr = IMagnetManager.get(this); + + Rectangle rect = getFigure().getBounds().getCopy(); + getFigure().getParent().translateToAbsolute(rect); + + Point start = rect.getTop(); + Point finish = rect.getBottom(); + + startMagnet = createOrUpdate(mgr, startMagnet, start); + finishMagnet = createOrUpdate(mgr, finishMagnet, finish); + } + + private DefaultMagnet createOrUpdate(IMagnetManager mgr, DefaultMagnet magnet, Point location) { + DefaultMagnet result = magnet; + + if (result != null) { + // Update it + result.setLocation(location); + } else { + result = new DefaultMagnet(getLayoutConstraints().getMagnetStrength(getNotationView())); + result.setLocation(location); + mgr.addMagnet(result); + } + + return result; + } } diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/parts/ISequenceEditPart.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/parts/ISequenceEditPart.java index 755d5681..67517140 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/parts/ISequenceEditPart.java +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/parts/ISequenceEditPart.java @@ -11,11 +11,22 @@ *****************************************************************************/ package org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.parts; +import java.util.Optional; + +import org.eclipse.draw2d.IFigure; +import org.eclipse.draw2d.geometry.Point; +import org.eclipse.emf.ecore.EObject; import org.eclipse.gmf.runtime.diagram.ui.editparts.IGraphicalEditPart; +import org.eclipse.gmf.runtime.notation.Diagram; +import org.eclipse.gmf.runtime.notation.View; +import org.eclipse.papyrus.uml.diagram.sequence.figure.magnets.IMagnetManager; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.Activator; +import org.eclipse.papyrus.uml.interaction.model.MElement; +import org.eclipse.papyrus.uml.interaction.model.MInteraction; import org.eclipse.papyrus.uml.interaction.model.spi.LayoutConstraints; import org.eclipse.papyrus.uml.interaction.model.spi.LayoutConstraints.RelativePosition; import org.eclipse.papyrus.uml.interaction.model.spi.LayoutHelper; +import org.eclipse.uml2.uml.Element; /** * A mix-in interface for edit-parts in the sequence diagram that need to access the {@link #getLayoutHelper() @@ -67,4 +78,38 @@ default int getPadding(RelativePosition orientation) { return getLayoutConstraints().getPadding(orientation, getNotationView()); } + /** + * Compute a {@code location} relative in my host figure's coördinate space. + * + * @param location + * an absolute location in the diagram viewer coördinates + * @return the {@code location} in my host figure's coördinate space + */ + default Point getRelativeLocation(Point location) { + Point result = location.getCopy(); + + IFigure figure = getFigure(); + if (figure != null) { + figure.translateToRelative(result); + result.translate(figure.getBounds().getLocation().getNegated()); + } + + return result; + } + + default MInteraction getInteraction() { + View view = getNotationView(); + Diagram diagram = view.getDiagram(); + return MInteraction.getInstance(diagram); + } + + default Optional> getLogicalElement() { + EObject semantic = resolveSemanticElement(); + return Optional.ofNullable(semantic).filter(Element.class::isInstance).map(Element.class::cast) + .flatMap(getInteraction()::getElement); + } + + default IMagnetManager getMagnetManager() { + return IMagnetManager.get(this); + } } diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/parts/InteractionEditPart.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/parts/InteractionEditPart.java index 6e6b5d10..b2bae90d 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/parts/InteractionEditPart.java +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/parts/InteractionEditPart.java @@ -15,13 +15,19 @@ import org.eclipse.gmf.runtime.gef.ui.figures.NodeFigure; import org.eclipse.gmf.runtime.notation.View; import org.eclipse.papyrus.uml.diagram.sequence.figure.InteractionFigure; +import org.eclipse.papyrus.uml.diagram.sequence.figure.magnets.IMagnetManager; +import org.eclipse.papyrus.uml.diagram.sequence.figure.magnets.MagnetManager; import org.eclipse.papyrus.uml.tools.utils.UMLUtil; import org.eclipse.uml2.uml.Interaction; public class InteractionEditPart extends AbstractBorderedShapeEditPart { + private final IMagnetManager magnetManager; + public InteractionEditPart(View view) { super(view); + + this.magnetManager = new MagnetManager(); } @Override @@ -32,4 +38,16 @@ protected NodeFigure createMainFigure() { protected Interaction getInteraction() { return (Interaction)UMLUtil.resolveUMLElement(this); } + + public IMagnetManager getMagnetManager() { + return magnetManager; + } + + @Override + public Object getAdapter(@SuppressWarnings("rawtypes") Class key) { + if (key == IMagnetManager.class) { + return magnetManager; + } + return super.getAdapter(key); + } } diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/parts/LifelineBodyEditPart.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/parts/LifelineBodyEditPart.java index e5eaca2d..8ce57f31 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/parts/LifelineBodyEditPart.java +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/parts/LifelineBodyEditPart.java @@ -21,6 +21,7 @@ import java.util.Optional; import java.util.OptionalInt; +import org.eclipse.draw2d.ConnectionAnchor; import org.eclipse.draw2d.IFigure; import org.eclipse.draw2d.geometry.Dimension; import org.eclipse.draw2d.geometry.Point; @@ -28,6 +29,8 @@ import org.eclipse.emf.common.notify.Notification; import org.eclipse.emf.ecore.EAttribute; import org.eclipse.gef.EditPolicy; +import org.eclipse.gef.Request; +import org.eclipse.gef.requests.DropRequest; import org.eclipse.gmf.runtime.diagram.ui.editparts.BorderedBorderItemEditPart; import org.eclipse.gmf.runtime.diagram.ui.editparts.IBorderItemEditPart; import org.eclipse.gmf.runtime.diagram.ui.editpolicies.EditPolicyRoles; @@ -36,6 +39,7 @@ import org.eclipse.gmf.runtime.notation.Shape; import org.eclipse.gmf.runtime.notation.View; import org.eclipse.papyrus.uml.diagram.sequence.figure.LifelineBodyFigure; +import org.eclipse.papyrus.uml.diagram.sequence.figure.magnets.IMagnet; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies.InteractionSemanticEditPolicy; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies.LifelineBodyDisallowMoveAndResizeEditPolicy; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies.LifelineBodyGraphicalNodeEditPolicy; @@ -156,4 +160,17 @@ protected void createDefaultEditPolicies() { protected void addBorderItem(IFigure borderItemContainer, IBorderItemEditPart borderItemEditPart) { borderItemContainer.add(borderItemEditPart.getFigure(), new OnLineBorderItemLocator(getMainFigure())); } + + @Override + public ConnectionAnchor getTargetConnectionAnchor(Request request) { + if (request instanceof DropRequest) { + Point location = ((DropRequest)request).getLocation(); + + // Snap the location to the nearest magnet, if any + Optional magnet = getMagnetManager().getCapturingMagnet(location); + magnet.map(IMagnet::getLocation).ifPresent(location::setLocation); + } + + return super.getTargetConnectionAnchor(request); + } } 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 2634bdb2..afa72a7a 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 @@ -24,7 +24,6 @@ import org.eclipse.draw2d.ConnectionAnchor; import org.eclipse.draw2d.IFigure; import org.eclipse.draw2d.geometry.Point; -import org.eclipse.emf.ecore.EObject; import org.eclipse.gef.commands.Command; import org.eclipse.gef.commands.UnexecutableCommand; import org.eclipse.gef.editpolicies.FeedbackHelper; @@ -34,13 +33,13 @@ import org.eclipse.gmf.runtime.diagram.core.commands.SetConnectionAnchorsCommand; import org.eclipse.gmf.runtime.diagram.ui.commands.ICommandProxy; import org.eclipse.gmf.runtime.diagram.ui.editparts.ConnectionEditPart; -import org.eclipse.gmf.runtime.diagram.ui.editparts.IGraphicalEditPart; import org.eclipse.gmf.runtime.diagram.ui.editparts.INodeEditPart; import org.eclipse.gmf.runtime.diagram.ui.editpolicies.GraphicalNodeEditPolicy; import org.eclipse.gmf.runtime.diagram.ui.requests.CreateConnectionViewRequest; import org.eclipse.gmf.runtime.emf.type.core.IElementType; import org.eclipse.gmf.runtime.notation.Diagram; -import org.eclipse.gmf.runtime.notation.View; +import org.eclipse.papyrus.uml.diagram.sequence.figure.magnets.IMagnet; +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.util.CreateRequestSwitch; import org.eclipse.papyrus.uml.diagram.sequence.runtime.util.MessageUtil; @@ -94,9 +93,11 @@ public Command caseCreateConnectionViewRequest(CreateConnectionViewRequest reque } private AnchorDescriptor computeAnchoring(Point location) { - Optional> self = getLogicalElement(); - Optional lifeline = getLifeline(); + return computeAnchoring(getLogicalElement(), location); + } + private AnchorDescriptor computeAnchoring(Optional> self, Point location) { + Optional lifeline = self.flatMap(this::getLifeline); Optional> before; int offset; @@ -124,19 +125,11 @@ private AnchorDescriptor computeAnchoring(Point location) { return new AnchorDescriptor(before, offset); } - protected MInteraction getInteraction() { - View view = ((IGraphicalEditPart)getHost()).getNotationView(); - Diagram diagram = view.getDiagram(); - return MInteraction.getInstance(diagram); - } - - protected Optional> getLogicalElement() { - EObject semantic = ((IGraphicalEditPart)getHost()).resolveSemanticElement(); - return Optional.ofNullable(semantic).filter(Element.class::isInstance).map(Element.class::cast) - .flatMap(getInteraction()::getElement); + protected Optional getLifeline() { + return getLogicalElement().flatMap(this::getLifeline); } - protected Optional getLifeline() { + protected Optional getLifeline(MElement logicalElement) { SequenceDiagramSwitch> lifelineSwitch = new SequenceDiagramSwitch>() { @Override public Optional caseMLifeline(MLifeline object) { @@ -154,7 +147,7 @@ public Optional caseMMessageEnd(MMessageEnd object) { } }; - return getLogicalElement().flatMap(lifelineSwitch::doSwitch); + return lifelineSwitch.doSwitch(logicalElement); } private int getOffsetFrom(Point relativeMouse, MLifeline lifeline, @@ -183,13 +176,21 @@ public Command caseCreateConnectionViewRequest(CreateConnectionViewRequest reque MLifeline sender = start.sender; MLifeline receiver = getLifeline().get(); + MElement startBefore = start.before.orElse(sender); + int startOffset = start.offset; + Point startLocation = start.location; + Point location = request.getLocation(); + + // Snap the point to the nearest magnet, if any + Optional magnet = getMagnetManager().getCapturingMagnet(location); + magnet.map(IMagnet::getLocation).ifPresent(location::setLocation); + + int absoluteY = location.y(); + switch (start.sort) { case ASYNCH_CALL_LITERAL: case ASYNCH_SIGNAL_LITERAL: // These can slope down - Point location = request.getLocation(); - int absoluteY = location.y(); - Point startLocation = start.location; // but don't require such pointer exactitude of the user if ((absoluteY < startLocation.y()) || !getLayoutConstraints().isAsyncMessageSlope(startLocation.preciseX(), @@ -202,14 +203,37 @@ public Command caseCreateConnectionViewRequest(CreateConnectionViewRequest reque location = getRelativeLocation(location); AnchorDescriptor anchorDesc = computeAnchoring(location); - result = sender.insertMessageAfter(start.before.orElse(sender), start.offset, - receiver, anchorDesc.elementBefore.orElse(receiver), anchorDesc.offset, - start.sort, null); + result = sender.insertMessageAfter(startBefore, startOffset, receiver, + anchorDesc.elementBefore.orElse(receiver), anchorDesc.offset, start.sort, + null); break; default: - // Enforce a horizontal layout - result = sender.insertMessageAfter(start.before.orElse(sender), start.offset, - receiver, start.sort, null); + startLocation = startLocation.getCopy(); + + Optional otherMagnet = getMagnetManager().getCapturingMagnet(startLocation) + .filter(__ -> !magnet.isPresent()); // But not if the target is on a magnet + otherMagnet.map(IMagnet::getLocation).ifPresent(startLocation::setLocation); + + // Enforce a horizontal layout (subject to magnet constraints, with the + // target magnet having precendence) + if (startLocation.y() != absoluteY || otherMagnet.isPresent()) { + if (!otherMagnet.isPresent()) { + // Update the source to match the target + startLocation.setY(absoluteY); + } + + // Recompute the connection source + ISequenceEditPart sourceEP = (ISequenceEditPart)request.getSourceEditPart(); + Point relative = sourceEP.getRelativeLocation(startLocation); + AnchorDescriptor newSourceAnchorDesc = computeAnchoring( + sourceEP.getLogicalElement(), relative); + + startBefore = newSourceAnchorDesc.elementBefore.orElse(sender); + startOffset = newSourceAnchorDesc.offset; + } + + result = sender.insertMessageAfter(startBefore, startOffset, receiver, start.sort, + null); break; } @@ -229,7 +253,7 @@ protected FeedbackHelper getFeedbackHelper(CreateConnectionRequest request) { if (feedbackHelper == null) { feedbackHelper = new MessageFeedbackHelper(Mode.CREATE, - MessageUtil.isSynchronousMessageConnection(request)); + MessageUtil.isSynchronousMessageConnection(request), getMagnetManager()); Point p = request.getLocation(); connectionFeedback = createDummyConnection(request); connectionFeedback.setConnectionRouter(getDummyConnectionRouter(request)); diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/ISequenceEditPolicy.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/ISequenceEditPolicy.java index 8afccc34..a12edc37 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/ISequenceEditPolicy.java +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/ISequenceEditPolicy.java @@ -12,6 +12,8 @@ package org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies; +import java.util.Optional; + import org.eclipse.draw2d.IFigure; import org.eclipse.draw2d.geometry.Point; import org.eclipse.emf.ecore.EObject; @@ -26,9 +28,14 @@ import org.eclipse.gmf.runtime.diagram.ui.editparts.IGraphicalEditPart; import org.eclipse.gmf.runtime.notation.View; import org.eclipse.papyrus.commands.wrappers.OperationToGEFCommandWrapper; +import org.eclipse.papyrus.uml.diagram.sequence.figure.magnets.IMagnetManager; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.Activator; +import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.parts.ISequenceEditPart; +import org.eclipse.papyrus.uml.interaction.model.MElement; +import org.eclipse.papyrus.uml.interaction.model.MInteraction; import org.eclipse.papyrus.uml.interaction.model.spi.LayoutConstraints; import org.eclipse.papyrus.uml.interaction.model.spi.LayoutHelper; +import org.eclipse.uml2.uml.Element; /** * A mix-in interface for edit-policies in the sequence diagram that need to create executable commands in a @@ -84,15 +91,19 @@ static View __getHostView(ISequenceEditPolicy __this) { * @return the {@code location} in my host figure's coördinate space */ default Point getRelativeLocation(Point location) { - Point result = location.getCopy(); + return Optional.of(getHost()).filter(ISequenceEditPart.class::isInstance) + .map(ISequenceEditPart.class::cast).map(seq -> seq.getRelativeLocation(location)) + .orElseGet(() -> { + Point result = location.getCopy(); - IFigure figure = __getHostFigure(this); - if (figure != null) { - figure.translateToRelative(result); - result.translate(figure.getBounds().getLocation().getNegated()); - } + IFigure figure = __getHostFigure(this); + if (figure != null) { + figure.translateToRelative(result); + result.translate(figure.getBounds().getLocation().getNegated()); + } - return result; + return result; + }); } default int getMinimumWidth() { @@ -103,4 +114,17 @@ default int getMinimumHeight() { return getLayoutConstraints().getMinimumHeight(__getHostView(this)); } + default MInteraction getInteraction() { + return Optional.of(getHost()).filter(ISequenceEditPart.class::isInstance) + .map(ISequenceEditPart.class::cast).map(ISequenceEditPart::getInteraction).orElse(null); + } + + default Optional> getLogicalElement() { + return Optional.of(getHost()).filter(ISequenceEditPart.class::isInstance) + .map(ISequenceEditPart.class::cast).flatMap(ISequenceEditPart::getLogicalElement); + } + + default IMagnetManager getMagnetManager() { + return IMagnetManager.get(getHost()); + } } 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 772166ff..25a5e2cd 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 @@ -22,6 +22,8 @@ import com.google.common.eventbus.EventBus; +import java.util.Optional; + import org.eclipse.draw2d.ConnectionAnchor; import org.eclipse.draw2d.geometry.Point; import org.eclipse.gef.EditPart; @@ -35,6 +37,7 @@ 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.papyrus.uml.diagram.sequence.figure.magnets.IMagnet; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies.MessageFeedbackHelper.Mode; import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.util.CommandCreatedEvent; import org.eclipse.papyrus.uml.diagram.sequence.runtime.util.MessageUtil; @@ -43,7 +46,7 @@ /** * Endpoint edit-policy for management of message ends. */ -public class MessageEndpointEditPolicy extends ConnectionEndpointEditPolicy { +public class MessageEndpointEditPolicy extends ConnectionEndpointEditPolicy implements ISequenceEditPolicy { private final EventBus bus; @@ -94,7 +97,8 @@ protected FeedbackHelper getFeedbackHelper(ReconnectRequest request) { boolean synch = MessageUtil.isSynchronous(message.getMessageSort()); boolean source = request.isMovingStartAnchor(); - feedbackHelper = new MessageFeedbackHelper(source ? Mode.MOVE_SOURCE : Mode.MOVE_TARGET, synch); + feedbackHelper = new MessageFeedbackHelper(source ? Mode.MOVE_SOURCE : Mode.MOVE_TARGET, synch, + getMagnetManager()); feedbackHelper.setConnection(getConnection()); } return feedbackHelper; @@ -138,8 +142,6 @@ protected void showConnectionMoveFeedback(BendpointRequest request) { } FeedbackHelper helper = getFeedbackHelper(request); - helper.setMovingStartAnchor(true); - helper.update(getConnection().getSourceAnchor(), request.getLocation()); helper.setMovingStartAnchor(false); helper.update(getConnection().getTargetAnchor(), request.getLocation()); } @@ -175,7 +177,7 @@ protected FeedbackHelper getFeedbackHelper(BendpointRequest request) { Message message = (Message)((IGraphicalEditPart)getHost()).resolveSemanticElement(); boolean synch = MessageUtil.isSynchronous(message.getMessageSort()); - feedbackHelper = new MessageFeedbackHelper(Mode.MOVE_BOTH, synch); + feedbackHelper = new MessageFeedbackHelper(Mode.MOVE_BOTH, synch, getMagnetManager()); feedbackHelper.setConnection(getConnection()); Point grabbedAt = lastMouseLocation; @@ -208,9 +210,34 @@ protected Command getMoveConnectionCommand(BendpointRequest request) { EditPart source = connection.getSource(); EditPart target = connection.getTarget(); - // Don't modify the original because we get this command many times + // Don't modify the originals because we get this command many times Point sourceLocation = getOriginalSourceLocation(request).getCopy(); + Point targetLocation = getOriginalTargetLocation(request).getCopy(); sourceLocation.translate(0, deltaY); + targetLocation.translate(0, deltaY); + + int magnetDelta = 0; + // Target-end magnets have precedence + Optional magnet = getMagnetManager().getCapturingMagnet(targetLocation); + if (magnet.isPresent()) { + Point newTargetLocation = targetLocation.getCopy(); + newTargetLocation.setLocation(magnet.get().getLocation()); + magnetDelta = newTargetLocation.y() - targetLocation.y(); + } else { + magnet = getMagnetManager().getCapturingMagnet(sourceLocation); + if (magnet.isPresent()) { + Point newSourceLocation = sourceLocation.getCopy(); + newSourceLocation.setLocation(magnet.get().getLocation()); + magnetDelta = newSourceLocation.y() - sourceLocation.y(); + } + } + + if (magnetDelta != 0) { + deltaY = deltaY + magnetDelta; + sourceLocation.translate(0, magnetDelta); + targetLocation.translate(0, magnetDelta); + } + ReconnectRequest sourceReq = new ReconnectRequest(REQ_RECONNECT_SOURCE); sourceReq.setTargetEditPart(source); sourceReq.setConnectionEditPart(connection); @@ -218,9 +245,6 @@ protected Command getMoveConnectionCommand(BendpointRequest request) { setForce(sourceReq, true); Command updateSource = source.getCommand(sourceReq); - // Don't modify the original because we get this command many times - Point targetLocation = getOriginalTargetLocation(request).getCopy(); - targetLocation.translate(0, deltaY); ReconnectRequest targetReq = new ReconnectRequest(REQ_RECONNECT_TARGET); targetReq.setTargetEditPart(target); targetReq.setConnectionEditPart(connection); diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/MessageFeedbackHelper.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/MessageFeedbackHelper.java index e97ff8d6..47699215 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/MessageFeedbackHelper.java +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/MessageFeedbackHelper.java @@ -12,6 +12,8 @@ package org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies; +import java.util.Optional; + import org.eclipse.draw2d.Connection; import org.eclipse.draw2d.ConnectionAnchor; import org.eclipse.draw2d.IFigure; @@ -19,6 +21,8 @@ import org.eclipse.gef.editpolicies.FeedbackHelper; import org.eclipse.gmf.runtime.gef.ui.figures.NodeFigure; import org.eclipse.papyrus.uml.diagram.sequence.figure.anchors.ISequenceAnchor; +import org.eclipse.papyrus.uml.diagram.sequence.figure.magnets.IMagnet; +import org.eclipse.papyrus.uml.diagram.sequence.figure.magnets.IMagnetManager; /** * Feedback helper for messages that does not allow them to slope upwards and that ensures horizontality of @@ -29,6 +33,8 @@ class MessageFeedbackHelper extends FeedbackHelper { private final boolean synchronous; + private final IMagnetManager magnetManager; + // In the case of moving the message, where it is grabbed private Point grabbedAt; @@ -39,12 +45,15 @@ class MessageFeedbackHelper extends FeedbackHelper { * the feedback mode * @param synchronous * whether the message is synchronous, requiring a horizontal alignment + * @param magnetManager + * the contextual magnet manager */ - MessageFeedbackHelper(Mode mode, boolean synchronous) { + MessageFeedbackHelper(Mode mode, boolean synchronous, IMagnetManager magnetManager) { super(); this.mode = mode; this.synchronous = synchronous; + this.magnetManager = magnetManager; setMovingStartAnchor(mode.isMovingSource()); } @@ -55,97 +64,113 @@ void setGrabbedAt(Point point) { @Override public void update(ConnectionAnchor _anchor, Point p) { - ConnectionAnchor anchor = _anchor; - ConnectionAnchor other = getOtherAnchor(); - - if ((other instanceof ISequenceAnchor) && (anchor instanceof ISequenceAnchor)) { - ISequenceAnchor otherAnchor = (ISequenceAnchor)other; + ConnectionAnchor[] anchor = {_anchor }; + ConnectionAnchor[] other = {getOtherAnchor() }; - Point thisLocation = getLocation(anchor); - Point otherLocation = getLocation(otherAnchor); + if ((other[0] instanceof ISequenceAnchor) && (anchor[0] instanceof ISequenceAnchor)) { + Point thisLocation = getLocation(anchor[0]); + Point otherLocation = getLocation(other[0]); if (mode == Mode.MOVE_BOTH) { // We maintain the slope anyways, so synchronicity doesn't matter int deltaY = p.y() - grabbedAt.y(); if (deltaY != 0) { thisLocation.translate(0, deltaY); - anchor = recreateAnchor(anchor, thisLocation); - - if (!isMovingStartAnchor()) { - // We've moved the source and now the target, so update - // our reference point for the next drag increment - grabbedAt.translate(0, deltaY); - } - } - } else if (synchronous) { - // Constrain the message to horizontal - switch (mode) { - case CREATE: - // Force it horizontal - p.setY(otherLocation.y()); - anchor = recreateAnchor(anchor, p); - break; - case MOVE_SOURCE: - case MOVE_TARGET: - // Bring the other end along - otherLocation.setY(p.y()); - other = recreateOtherAnchor(otherAnchor, otherLocation); - if (mode.isMovingTarget()) { - getConnection().setSourceAnchor(other); - } else { - getConnection().setTargetAnchor(other); + otherLocation.translate(0, deltaY); + grabbedAt.translate(0, deltaY); // Prepare the next drag increment + + // Snap each point to the nearest magnet, if any, with priority to the + // receiving end + int magnetDelta = 0; + Optional magnet = magnetManager.getCapturingMagnet(thisLocation); + if (magnet.isPresent()) { + magnetDelta = magnet.get().getLocation().y() - thisLocation.y(); + } else { + magnet = magnetManager.getCapturingMagnet(otherLocation); + if (magnet.isPresent()) { + magnetDelta = magnet.get().getLocation().y() - otherLocation.y(); } - break; - default: - // MOVE_BOTH is handled separately - break; + } + if (magnetDelta != 0) { + // Additional adjustment + thisLocation.translate(0, magnetDelta); + otherLocation.translate(0, magnetDelta); + grabbedAt.translate(0, magnetDelta); + } + + recreateAnchor(anchor, thisLocation); + recreateOtherAnchor(other, otherLocation); + updateOtherAnchor(other); } } else { - // Don't permit the message to go back in time - int delta = mode.isMovingTarget() ? p.y() - otherLocation.y() : otherLocation.y() - p.y(); + // Snap the point to the nearest magnet, if any + Optional magnet = magnetManager.getCapturingMagnet(p); + magnet.map(IMagnet::getLocation).ifPresent(p::setLocation); - if (delta < 0) { + if (synchronous) { + // Constrain the message to horizontal switch (mode) { case CREATE: - // Force it horizontal - p.setY(otherLocation.y()); - anchor = recreateAnchor(anchor, p); - break; case MOVE_SOURCE: case MOVE_TARGET: - // Bring the other end along + // Bring the other end along (subject to magnet constraints) otherLocation.setY(p.y()); - other = recreateOtherAnchor(otherAnchor, otherLocation); - if (mode.isMovingTarget()) { - getConnection().setSourceAnchor(other); - } else { - getConnection().setTargetAnchor(other); - } + + Optional otherMagnet = magnetManager.getCapturingMagnet(otherLocation); + otherMagnet.map(IMagnet::getLocation).ifPresent(m -> { + // Don't move this end if the other is stuck to a magnet + int dy = otherLocation.y() - m.y(); + otherLocation.setLocation(m); + p.translate(0, -dy); + recreateAnchor(anchor, p); + }); + + recreateOtherAnchor(other, otherLocation); + updateOtherAnchor(other); break; default: // MOVE_BOTH is handled separately break; } - } else if (synchronous && (delta > 0)) { - switch (mode) { - case CREATE: - // Force it horizontal - p.setY(otherLocation.y()); - anchor = recreateAnchor(anchor, p); - break; - case MOVE_SOURCE: - case MOVE_TARGET: - // Let the new slope of the asynchronous message be applied - break; - default: - // MOVE_BOTH is handled separately - break; + } else { + // Don't permit the message to go back in time + int delta = mode.isMovingTarget() ? p.y() - otherLocation.y() : otherLocation.y() - p.y(); + + if (delta < 0) { + switch (mode) { + case CREATE: + // Force it horizontal + p.setY(otherLocation.y()); + recreateAnchor(anchor, p); + break; + case MOVE_SOURCE: + case MOVE_TARGET: + // Bring the other end along (subject to magnet constraints) + otherLocation.setY(p.y()); + + Optional otherMagnet = magnetManager + .getCapturingMagnet(otherLocation); + otherMagnet.map(IMagnet::getLocation).ifPresent(m -> { + // Don't move this end if the other is stuck to a magnet + int dy = otherLocation.y() - m.y(); + otherLocation.setLocation(m); + p.translate(0, -dy); + recreateAnchor(anchor, p); + }); + + recreateOtherAnchor(other, otherLocation); + updateOtherAnchor(other); + break; + default: + // MOVE_BOTH is handled separately + break; + } } } } } - super.update(anchor, p); + super.update(anchor[0], p); } private ConnectionAnchor getOtherAnchor() { @@ -160,18 +185,26 @@ static Point getLocation(ConnectionAnchor anchor) { return anchor.getLocation(ownerOrigin); } - private ConnectionAnchor recreateAnchor(ConnectionAnchor anchor, Point p) { - NodeFigure owner = (NodeFigure)anchor.getOwner(); - return mode.isMovingTarget() ? owner.getTargetConnectionAnchorAt(p) + private void recreateAnchor(ConnectionAnchor[] anchor, Point p) { + NodeFigure owner = (NodeFigure)anchor[0].getOwner(); + anchor[0] = mode.isMovingTarget() ? owner.getTargetConnectionAnchorAt(p) : owner.getSourceConnectionAnchorAt(p); } - private ConnectionAnchor recreateOtherAnchor(ConnectionAnchor anchor, Point p) { - NodeFigure owner = (NodeFigure)anchor.getOwner(); - return mode.isMovingTarget() ? owner.getSourceConnectionAnchorAt(p) + private void recreateOtherAnchor(ConnectionAnchor[] anchor, Point p) { + NodeFigure owner = (NodeFigure)anchor[0].getOwner(); + anchor[0] = mode.isMovingTarget() ? owner.getSourceConnectionAnchorAt(p) : owner.getTargetConnectionAnchorAt(p); } + void updateOtherAnchor(ConnectionAnchor[] other) { + if (mode.isMovingTarget()) { + getConnection().setSourceAnchor(other[0]); + } else { + getConnection().setTargetAnchor(other[0]); + } + } + // // Nested types // diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/factories/SequenceEditPartAdapterFactory.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/factories/SequenceEditPartAdapterFactory.java new file mode 100644 index 00000000..8923d410 --- /dev/null +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/factories/SequenceEditPartAdapterFactory.java @@ -0,0 +1,85 @@ +/***************************************************************************** + * Copyright (c) 2018 Christian W. Damus and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Christian W. Damus - Initial API and implementation + *****************************************************************************/ + +package org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.factories; + +import java.util.Optional; +import java.util.function.Function; +import java.util.function.UnaryOperator; + +import org.eclipse.core.runtime.IAdaptable; +import org.eclipse.core.runtime.IAdapterFactory; +import org.eclipse.gef.ConnectionEditPart; +import org.eclipse.gef.EditPart; +import org.eclipse.gmf.runtime.diagram.ui.editparts.DiagramEditPart; +import org.eclipse.papyrus.uml.diagram.sequence.figure.magnets.IMagnetManager; +import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.parts.InteractionEditPart; +import org.eclipse.papyrus.uml.interaction.model.spi.ViewTypes; + +/** + * Adapter factory for edit-parts in the sequence diagram. + */ +public class SequenceEditPartAdapterFactory implements IAdapterFactory { + private final Class[] adapters = {IMagnetManager.class }; + + /** + * Initializes me. + */ + public SequenceEditPartAdapterFactory() { + super(); + } + + @Override + public T getAdapter(Object adaptableObject, Class adapterType) { + EditPart self = (EditPart)adaptableObject; + T result = null; + + if (adapterType == IMagnetManager.class) { + // The interaction edit-part has this + Optional interactionEP = getInteractionEditPart(self); + result = interactionEP.map(adapt(adapterType)).orElse(null); + } + + return result; + } + + @Override + public Class[] getAdapterList() { + return adapters; + } + + Optional getInteractionEditPart(EditPart editPart) { + EditPart result; + + if (editPart instanceof DiagramEditPart) { + // Get the interaction frame from this + result = ((DiagramEditPart)editPart).getChildBySemanticHint(ViewTypes.INTERACTION); + } else { + // How to step "up" the edit-part hierarchy + UnaryOperator step = ep -> (ep instanceof ConnectionEditPart) + ? ((ConnectionEditPart)ep).getSource() + : ep.getParent(); + + for (result = editPart; result != null; result = step.apply(result)) { + if (result instanceof InteractionEditPart) { + break; + } + } + } + + return Optional.ofNullable(result); + } + + static Function adapt(Class type) { + return a -> a.getAdapter(type); + } +} diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/providers/SequencePaletteProvider.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/providers/SequencePaletteProvider.java new file mode 100644 index 00000000..18476c94 --- /dev/null +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/providers/SequencePaletteProvider.java @@ -0,0 +1,131 @@ +/***************************************************************************** + * Copyright (c) 2018 Christian W. Damus and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Christian W. Damus - Initial API and implementation + *****************************************************************************/ + +package org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.providers; + +import java.lang.reflect.Field; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.function.Consumer; + +import org.eclipse.core.runtime.IConfigurationElement; +import org.eclipse.emf.common.util.AbstractTreeIterator; +import org.eclipse.emf.common.util.TreeIterator; +import org.eclipse.gef.palette.PaletteContainer; +import org.eclipse.gef.palette.PaletteEntry; +import org.eclipse.gef.palette.PaletteRoot; +import org.eclipse.gef.palette.ToolEntry; +import org.eclipse.gef.tools.SelectionTool; +import org.eclipse.gmf.runtime.common.core.service.AbstractProvider; +import org.eclipse.gmf.runtime.common.core.service.IOperation; +import org.eclipse.gmf.runtime.diagram.ui.services.palette.IPaletteProvider; +import org.eclipse.gmf.runtime.diagram.ui.services.palette.PaletteFactory; +import org.eclipse.gmf.runtime.diagram.ui.services.palette.PaletteService; +import org.eclipse.papyrus.infra.gmfdiag.paletteconfiguration.provider.ExtendedConnectionToolEntry; +import org.eclipse.papyrus.uml.diagram.sequence.figure.magnets.IMagnetManager; +import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.Activator; +import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.tools.SequencePaletteFactory; +import org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.tools.SequenceSelectionTool; +import org.eclipse.ui.IEditorPart; + +/** + * A palette provider for sequence diagrams that injects custom tools, including + *
    + *
  • a {@link SelectionTool} that integrates with the {@linkplain IMagnetManager magnet manager} + *
  • {@linkplain PaletteFactory palette factories} for {@linkplain ToolEntry tool entries} that create + * connection creation tools that integrate with the {@linkplain IMagnetManager magnet manager}
    • + *
+ */ +public class SequencePaletteProvider extends AbstractProvider implements IPaletteProvider { + + private static Consumer connectionToolEntryDelegator; + + /** + * Initializes me. + */ + public SequencePaletteProvider() { + super(); + } + + @Override + public boolean provides(IOperation operation) { + // Papyrus palette service never invokes this. Only filtering in the plugin.xml is supported + return true; + } + + @Override + public void contributeToPalette(IEditorPart editor, Object content, PaletteRoot root, + @SuppressWarnings("rawtypes") Map predefinedEntries) { + + for (Iterator iter = allContents(root); iter.hasNext();) { + PaletteEntry next = iter.next(); + if ((next instanceof ToolEntry) && PaletteService.TOOL_SELECTION.equals(next.getId())) { + ((ToolEntry)next).setToolClass(SequenceSelectionTool.class); + } else if (next instanceof ExtendedConnectionToolEntry) { + ExtendedConnectionToolEntry connection = (ExtendedConnectionToolEntry)next; + setToolFactory(connection); + } + } + } + + @Override + public void setContributions(IConfigurationElement configElement) { + // We don't need to know this + } + + @SuppressWarnings("serial") + TreeIterator allContents(PaletteRoot root) { + return new AbstractTreeIterator(root) { + @Override + protected Iterator getChildren(@SuppressWarnings("hiding") Object object) { + if (object instanceof PaletteContainer) { + @SuppressWarnings("unchecked") + List entries = ((PaletteContainer)object).getChildren(); + return entries.iterator(); + } + return Collections.emptyIterator(); + } + }; + } + + private static void setToolFactory(ExtendedConnectionToolEntry connectionToolEntry) { + getConnectionToolEntryDelegator().accept(connectionToolEntry); + } + + private static Consumer getConnectionToolEntryDelegator() { + if (connectionToolEntryDelegator == null) { + try { + @SuppressWarnings("restriction") + Field factoryField = org.eclipse.gmf.runtime.diagram.ui.internal.services.palette.PaletteToolEntry.class + .getDeclaredField("factory"); //$NON-NLS-1$ + factoryField.setAccessible(true); + connectionToolEntryDelegator = tool -> { + try { + PaletteFactory delegate = (PaletteFactory)factoryField.get(tool); + factoryField.set(tool, new SequencePaletteFactory(delegate)); + } catch (Exception e) { + // Shouldn't be happening if we found the field and made it accessible + connectionToolEntryDelegator = Objects::requireNonNull; + } + }; + } catch (Exception e) { + Activator.log.error("Cannot inject magnet support into connection tools.", e); //$NON-NLS-1$ + connectionToolEntryDelegator = Objects::requireNonNull; + } + } + + return connectionToolEntryDelegator; + } +} diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/tools/MagnetSupport.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/tools/MagnetSupport.java new file mode 100644 index 00000000..333e3ecc --- /dev/null +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/tools/MagnetSupport.java @@ -0,0 +1,77 @@ +/***************************************************************************** + * Copyright (c) 2018 Christian W. Damus and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Christian W. Damus - Initial API and implementation + *****************************************************************************/ + +package org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.tools; + +import java.util.Optional; +import java.util.function.Supplier; + +import org.eclipse.gef.EditPartViewer; +import org.eclipse.gef.RootEditPart; +import org.eclipse.gef.tools.AbstractTool.Input; +import org.eclipse.papyrus.uml.diagram.sequence.figure.magnets.IMagnetManager; +import org.eclipse.papyrus.uml.diagram.sequence.figure.magnets.MagnetManager; + +/** + * Support for integration of palette tools with the {@linkplain IMagnetManager magnet manager}. + */ +class MagnetSupport { + private final Supplier viewerSupplier; + + private final Supplier basicInputSupplier; + + private Optional magnetManager = Optional.empty(); + + private Input currentInput; + + /** + * Initializes me with my viewer supplier and basic input supplier. + * + * @param viewSupplier + * supplier of the contextual edit-part viewer + * @param basicInputSupplier + * basic supplier of the tool's input + */ + MagnetSupport(Supplier viewerSupplier, Supplier basicInputSupplier) { + super(); + + this.viewerSupplier = viewerSupplier; + this.basicInputSupplier = basicInputSupplier; + } + + /** + * Called upon deactivation of my owner tool. + */ + void deactivate() { + magnetManager.ifPresent(mgr -> mgr.setCurrentInput(null)); + magnetManager = Optional.empty(); + currentInput = null; + } + + /** + * Called by my owner tool to get its current input. + */ + Input getCurrentInput() { + if (currentInput == null) { + currentInput = basicInputSupplier.get(); + } + + if (!magnetManager.isPresent()) { + magnetManager = Optional.ofNullable(viewerSupplier.get()).map(EditPartViewer::getRootEditPart) + .map(RootEditPart::getContents).flatMap(MagnetManager::get); + magnetManager.ifPresent(mgr -> mgr.setCurrentInput(currentInput)); + } + + return currentInput; + } + +} diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/tools/SequenceConnectionCreationTool.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/tools/SequenceConnectionCreationTool.java new file mode 100644 index 00000000..599187ce --- /dev/null +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/tools/SequenceConnectionCreationTool.java @@ -0,0 +1,51 @@ +/***************************************************************************** + * Copyright (c) 2018 Christian W. Damus and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Christian W. Damus - Initial API and implementation + *****************************************************************************/ + +package org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.tools; + +import java.util.List; + +import org.eclipse.gmf.runtime.emf.type.core.IElementType; +import org.eclipse.papyrus.infra.gmfdiag.common.service.palette.AspectUnspecifiedTypeConnectionTool; +import org.eclipse.papyrus.uml.diagram.sequence.figure.magnets.IMagnetManager; + +/** + * Custom connection creation tool that integrates with the {@linkplain IMagnetManager magnet manager}. + */ +public class SequenceConnectionCreationTool extends AspectUnspecifiedTypeConnectionTool { + + private final MagnetSupport magnetSupport = new MagnetSupport(this::getCurrentViewer, + super::getCurrentInput); + + /** + * Initializes me with my element types. + * + * @param elementTypes + * my supported element types + */ + public SequenceConnectionCreationTool(List elementTypes) { + super(elementTypes); + } + + @Override + public void deactivate() { + magnetSupport.deactivate(); + + super.deactivate(); + } + + @Override + protected Input getCurrentInput() { + return magnetSupport.getCurrentInput(); + } + +} diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/tools/SequencePaletteFactory.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/tools/SequencePaletteFactory.java new file mode 100644 index 00000000..dc4716e7 --- /dev/null +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/tools/SequencePaletteFactory.java @@ -0,0 +1,55 @@ +/***************************************************************************** + * Copyright (c) 2018 Christian W. Damus and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Christian W. Damus - Initial API and implementation + *****************************************************************************/ + +package org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.tools; + +import org.eclipse.gef.Tool; +import org.eclipse.gmf.runtime.diagram.ui.services.palette.PaletteFactory; +import org.eclipse.papyrus.infra.gmfdiag.common.service.palette.AspectUnspecifiedTypeConnectionTool; +import org.eclipse.papyrus.uml.diagram.sequence.figure.magnets.IMagnetManager; + +/** + * Custom palette factory that substitutes Papyrus tools with alternate implementations that support + * integration with the {@linkplain IMagnetManager magnet manager}. + */ +public final class SequencePaletteFactory extends PaletteFactory.Adapter { + private final PaletteFactory delegate; + + /** + * Initializes me with the default factory for tools described by my owning palette model entry. + * + * @param delegate + * a palette factory delegate + */ + public SequencePaletteFactory(PaletteFactory delegate) { + super(); + + this.delegate = delegate; + } + + @Override + public Tool createTool(String toolId) { + Tool result = delegate.createTool(toolId); + + if (result instanceof AspectUnspecifiedTypeConnectionTool) { + AspectUnspecifiedTypeConnectionTool connectionTool = (AspectUnspecifiedTypeConnectionTool)result; + result = new SequenceConnectionCreationTool(connectionTool.getElementTypes()); + } + + return result; + } + + @Override + public Object getTemplate(String templateId) { + return delegate.getTemplate(templateId); + } +} diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/tools/SequenceSelectionTool.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/tools/SequenceSelectionTool.java new file mode 100644 index 00000000..5f926282 --- /dev/null +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/tools/SequenceSelectionTool.java @@ -0,0 +1,45 @@ +/***************************************************************************** + * Copyright (c) 2018 Christian W. Damus and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Christian W. Damus - Initial API and implementation + *****************************************************************************/ + +package org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.tools; + +import org.eclipse.gmf.runtime.diagram.ui.services.palette.SelectionToolEx; +import org.eclipse.papyrus.uml.diagram.sequence.figure.magnets.IMagnetManager; + +/** + * Custom selection tool that integrates with the {@linkplain IMagnetManager magnet manager}. + */ +public class SequenceSelectionTool extends SelectionToolEx { + + private final MagnetSupport magnetSupport = new MagnetSupport(this::getCurrentViewer, + super::getCurrentInput); + + /** + * Initializes me. + */ + public SequenceSelectionTool() { + super(); + } + + @Override + public void deactivate() { + magnetSupport.deactivate(); + + super.deactivate(); + } + + @Override + protected Input getCurrentInput() { + return magnetSupport.getCurrentInput(); + } + +} 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 3119b10f..83719b43 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 @@ -12,7 +12,9 @@ package org.eclipse.papyrus.uml.interaction.internal.model.commands; +import static java.lang.Integer.MAX_VALUE; import static java.lang.Math.max; +import static org.eclipse.papyrus.uml.interaction.graph.util.CrossReferenceUtil.invertSingle; import static org.eclipse.papyrus.uml.interaction.graph.util.Suppliers.compose; import java.util.ArrayList; @@ -27,6 +29,14 @@ import org.eclipse.emf.common.command.Command; import org.eclipse.emf.common.command.UnexecutableCommand; +import org.eclipse.emf.common.util.EList; +import org.eclipse.emf.ecore.EObject; +import org.eclipse.emf.ecore.EReference; +import org.eclipse.emf.edit.command.AddCommand; +import org.eclipse.emf.edit.command.CommandParameter; +import org.eclipse.emf.edit.command.DeleteCommand; +import org.eclipse.emf.edit.command.MoveCommand; +import org.eclipse.emf.edit.command.SetCommand; import org.eclipse.gmf.runtime.notation.Shape; import org.eclipse.gmf.runtime.notation.View; import org.eclipse.papyrus.uml.interaction.graph.Vertex; @@ -39,14 +49,17 @@ 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.MOccurrence; import org.eclipse.papyrus.uml.interaction.model.spi.DeferredAddCommand; import org.eclipse.papyrus.uml.interaction.model.spi.LayoutHelper; import org.eclipse.papyrus.uml.interaction.model.spi.SemanticHelper; import org.eclipse.uml2.uml.Element; +import org.eclipse.uml2.uml.ExecutionSpecification; import org.eclipse.uml2.uml.Message; import org.eclipse.uml2.uml.MessageEnd; import org.eclipse.uml2.uml.MessageSort; import org.eclipse.uml2.uml.NamedElement; +import org.eclipse.uml2.uml.OccurrenceSpecification; import org.eclipse.uml2.uml.Operation; import org.eclipse.uml2.uml.Signal; import org.eclipse.uml2.uml.UMLPackage; @@ -200,12 +213,14 @@ protected Command createCommand() { } // Send: Is there actually an execution occurrence here? - int llTop = layoutHelper().getBottom(getTarget().getDiagramView().get()); - int where = beforeSend == getTarget() ? sendOffset + int llTopSend = layoutHelper().getBottom(getTarget().getDiagramView().get()); + int whereSend = beforeSend == getTarget() ? sendOffset // For executions also the top to allow for messages to anchor within it - : beforeSend.getTop().orElse(llTop) - llTop + sendOffset; - Optional sendingExec = getTarget().elementAt(where).filter(MExecution.class::isInstance) - .map(MExecution.class::cast); + : beforeSend.getTop().orElse(llTopSend) - llTopSend + sendOffset; + Optional sendingExec = getTarget().elementAt(whereSend).flatMap(this::getExecution) + .filter(exec -> // + ((exec.getBottom().orElse(-1) - llTopSend) >= whereSend) // + && ((exec.getTop().orElse(MAX_VALUE) - llTopSend) <= whereSend)); Vertex sender = sendingExec.map(this::vertex).orElseGet(this::vertex); if (sender == null || sender.getDiagramView() == null) { return UnexecutableCommand.INSTANCE; @@ -214,10 +229,10 @@ protected Command createCommand() { // Receive: Is there actually an execution occurrence here? // In case of create or destruction messages, this should not connect to execution occurrences, // because they connect to lifeline-header/new destruction occurrence - llTop = layoutHelper().getBottom(receiver.getDiagramView().get()); - where = beforeRecv == receiver ? recvOffset + int llTopRecv = layoutHelper().getBottom(receiver.getDiagramView().get()); + int whereRecv = beforeRecv == receiver ? recvOffset // For executions also the top to allow for messages to anchor within it - : beforeRecv.getTop().orElse(llTop) - llTop + recvOffset; + : beforeRecv.getTop().orElse(llTopRecv) - llTopRecv + recvOffset; Optional receivingExec; switch (sort) { case CREATE_MESSAGE_LITERAL: @@ -225,20 +240,28 @@ protected Command createCommand() { receivingExec = Optional.empty(); break; default: - receivingExec = this.receiver.elementAt(where).filter(MExecution.class::isInstance) - .map(MExecution.class::cast); + receivingExec = this.receiver.elementAt(whereRecv).flatMap(this::getExecution).filter(exec -> // + ((exec.getBottom().orElse(-1) - llTopRecv) >= whereRecv) // + && ((exec.getTop().orElse(MAX_VALUE) - llTopRecv) <= whereRecv)); break; } + @SuppressWarnings("hiding") Vertex receiver = receivingExec.map(this::vertex).orElseGet(() -> vertex(this.receiver)); if (receiver == null || receiver.getDiagramView() == null) { return UnexecutableCommand.INSTANCE; } - OptionalInt sendReferenceY = layoutHelper().getBottom(sendReference); + OptionalInt sendReferenceY = beforeSend == getTarget() ? layoutHelper().getBottom(sendReference) + // Reference from the top of an execution specification to allow + // connections within its extent + : layoutHelper().getTop(sendReference); if (!sendReferenceY.isPresent()) { return UnexecutableCommand.INSTANCE; } - OptionalInt recvReferenceY = layoutHelper().getBottom(recvReference); + OptionalInt recvReferenceY = beforeRecv == this.receiver ? layoutHelper().getBottom(recvReference) + // Reference from the top of an execution specification to allow + // connections within its extent + : layoutHelper().getTop(recvReference); if (!recvReferenceY.isPresent()) { return UnexecutableCommand.INSTANCE; } @@ -334,7 +357,8 @@ protected Command createCommand() { /* create message */ result = result.chain(diagramHelper().createMessageConnector(resultCommand, // senderView, () -> sendReferenceY.getAsInt() + sendOffset, // - receiverView, () -> recvReferenceY.getAsInt() + recvOffset)); + receiverView, () -> recvReferenceY.getAsInt() + recvOffset, // + this::replace)); switch (sort) { case CREATE_MESSAGE_LITERAL: @@ -405,7 +429,6 @@ protected Command createCommand() { } return result; - } private int relativeTopOfBefore() { @@ -432,4 +455,72 @@ private CreationParameters endParams(MElement insertionPoint) UMLPackage.Literals.INTERACTION__FRAGMENT) : CreationParameters.after(insertionPoint.getElement()); } + + private Optional replace(OccurrenceSpecification occurrence, MessageEnd msgEnd) { + Optional execSpec = getExecution(occurrence); + return execSpec.map(exec -> { + boolean isStart = exec.getStart() == occurrence; + + EObject owner = occurrence.eContainer(); + EReference containment = occurrence.eContainmentFeature(); + int index = containment.isMany() ? ((EList)owner.eGet(containment)).indexOf(occurrence) + : CommandParameter.NO_INDEX; + + Command result = DeleteCommand.create(getEditingDomain(), occurrence); + + if (msgEnd.eContainer() == owner) { + result = result + .chain(MoveCommand.create(getEditingDomain(), owner, containment, msgEnd, index)); + } else { + result = result + .chain(AddCommand.create(getEditingDomain(), owner, containment, msgEnd, index)); + } + + if (isStart) { + result = result.chain(SetCommand.create(getEditingDomain(), exec, + UMLPackage.Literals.EXECUTION_SPECIFICATION__START, msgEnd)); + } else { + result = result.chain(SetCommand.create(getEditingDomain(), exec, + UMLPackage.Literals.EXECUTION_SPECIFICATION__FINISH, msgEnd)); + } + + return result; + }); + } + + Optional getExecution(MElement interactionElement) { + Optional result = Optional.empty(); + + if (interactionElement instanceof MExecution) { + // It already is an execution + result = Optional.of((MExecution)interactionElement); + } else if (interactionElement instanceof MOccurrence) { + MOccurrence occurrence = (MOccurrence)interactionElement; + result = occurrence.getStartedExecution(); + if (!result.isPresent()) { + result = occurrence.getFinishedExecution(); + } + } + + return result; + } + + /** + * Query the {@link ExecutionSpecification} that is started or finished by an {@code occurrence} + * specification. + * + * @param occurrence + * an occurrence in the interaction + * @return the started or finished execution + */ + Optional getExecution(OccurrenceSpecification occurrence) { + Optional result = invertSingle(occurrence, + UMLPackage.Literals.EXECUTION_SPECIFICATION__START, ExecutionSpecification.class); + if (!result.isPresent()) { + result = invertSingle(occurrence, UMLPackage.Literals.EXECUTION_SPECIFICATION__FINISH, + ExecutionSpecification.class); + } + return result; + } + } diff --git a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/spi/impl/AnchorFactory.java b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/spi/impl/AnchorFactory.java new file mode 100644 index 00000000..41240871 --- /dev/null +++ b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/spi/impl/AnchorFactory.java @@ -0,0 +1,245 @@ +/***************************************************************************** + * Copyright (c) 2018 Christian W. Damus and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Christian W. Damus - Initial API and implementation + *****************************************************************************/ + +package org.eclipse.papyrus.uml.interaction.internal.model.spi.impl; + +import org.eclipse.gmf.runtime.notation.Anchor; +import org.eclipse.gmf.runtime.notation.Connector; +import org.eclipse.gmf.runtime.notation.IdentityAnchor; +import org.eclipse.gmf.runtime.notation.NotationPackage; +import org.eclipse.gmf.runtime.notation.Shape; +import org.eclipse.papyrus.uml.interaction.model.spi.LayoutHelper; +import org.eclipse.papyrus.uml.interaction.model.spi.ViewTypes; + +/** + * A black-box factory for connection anchors in the sequence diagram. + */ +class AnchorFactory { + + private NullAnchorBuilder nullAnchor; + + private LifelineBodyAnchorBuilder lifelineBodyBuilder; + + private LifelineHeadAnchorBuilder lifelineHeadBuilder; + + private ExecutionSpecificationAnchorBuilder execSpecBuilder; + + private final Connector connector; + + private final LayoutHelper layout; + + /** + * Initializes me with the {@code connector} for which I create anchors and a {@code layout} helper for + * geometric calculations. + * + * @param connector + * my connector + * @param layout + * my layout helper + */ + AnchorFactory(Connector connector, LayoutHelper layout) { + super(); + + this.connector = connector; + this.layout = layout; + } + + public AnchorBuilder builderFor(Shape shape) { + AnchorBuilder result; + + if (shape.getType() == null) { + result = defaultAnchorBuilder(); + } else { + switch (shape.getType()) { + case ViewTypes.LIFELINE_BODY: + result = lifelineBodyBuilder(); + break; + case ViewTypes.LIFELINE_HEADER: + result = lifelineHeadBuilder(); + break; + case ViewTypes.EXECUTION_SPECIFICATION: + result = executionSpecificationBuilder(); + break; + default: + result = defaultAnchorBuilder(); + } + } + + return result; + } + + public AnchorBuilder defaultAnchorBuilder() { + if (nullAnchor == null) { + nullAnchor = new NullAnchorBuilder(); + } + return nullAnchor; + } + + public AnchorBuilder lifelineBodyBuilder() { + if (lifelineBodyBuilder == null) { + lifelineBodyBuilder = new LifelineBodyAnchorBuilder(); + } + return lifelineBodyBuilder; + } + + public AnchorBuilder lifelineHeadBuilder() { + if (lifelineHeadBuilder == null) { + lifelineHeadBuilder = new LifelineHeadAnchorBuilder(); + } + return lifelineHeadBuilder; + } + + public AnchorBuilder executionSpecificationBuilder() { + if (execSpecBuilder == null) { + execSpecBuilder = new ExecutionSpecificationAnchorBuilder(); + } + return execSpecBuilder; + } + + public static boolean isExecutionSpecificationStart(Anchor anchor) { + return (anchor instanceof IdentityAnchor) && "start".equals(((IdentityAnchor)anchor).getId()); + } + + public static boolean isExecutionSpecificationFinish(Anchor anchor) { + return (anchor instanceof IdentityAnchor) && "end".equals(((IdentityAnchor)anchor).getId()); + } + + // + // Nested types + // + + public abstract class AnchorBuilder { + boolean isConnectionSource; + + Shape sourceView; + + Shape targetView; + + int distance; + + private AnchorBuilder() { + super(); + } + + public AnchorBuilder from(Shape view) { + sourceView = view; + return this; + } + + public AnchorBuilder to(Shape view) { + targetView = view; + return this; + } + + public AnchorBuilder sourceEnd() { + isConnectionSource = true; + return this; + } + + public AnchorBuilder targetEnd() { + isConnectionSource = false; + return this; + } + + boolean isLeftToRight() { + return layout.getLeft(sourceView) <= layout.getLeft(targetView); + } + + Shape getAnchorView() { + return isConnectionSource ? sourceView : targetView; + } + + public AnchorBuilder at(int yOffset) { + this.distance = yOffset; + return this; + } + + public final Anchor build() { + IdentityAnchor result = isConnectionSource + ? (IdentityAnchor)connector.createSourceAnchor(NotationPackage.Literals.IDENTITY_ANCHOR) + : (IdentityAnchor)connector.createTargetAnchor(NotationPackage.Literals.IDENTITY_ANCHOR); + result.setId(computeIdentity()); + sourceView = null; + targetView = null; + return result; + } + + protected abstract String computeIdentity(); + } + + private final class NullAnchorBuilder extends AnchorBuilder { + NullAnchorBuilder() { + super(); + } + + @Override + public String computeIdentity() { + return ""; //$NON-NLS-1$ + } + } + + private final class LifelineBodyAnchorBuilder extends AnchorBuilder { + private LifelineBodyAnchorBuilder() { + super(); + } + + @Override + public String computeIdentity() { + return Integer.toString(distance); + } + } + + private final class LifelineHeadAnchorBuilder extends AnchorBuilder { + private LifelineHeadAnchorBuilder() { + super(); + } + + @Override + public String computeIdentity() { + String result; + + // Which direction? + if (isLeftToRight() == isConnectionSource) { + result = "right;" + distance; + } else { + result = "left;" + distance; + } + + return result; + } + } + + private final class ExecutionSpecificationAnchorBuilder extends AnchorBuilder { + private ExecutionSpecificationAnchorBuilder() { + super(); + } + + @Override + public String computeIdentity() { + String result; + + // Is it the top or bottom? + if (distance < 1) { + result = "start"; + } else if (distance >= layout.getHeight(getAnchorView())) { + result = "end"; + } else // Which direction? + if (isLeftToRight() == isConnectionSource) { + result = "right;" + distance; + } else { + result = "left;" + distance; + } + + return result; + } + } +} 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 79521500..952320e0 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,10 +14,14 @@ import static org.eclipse.papyrus.uml.interaction.graph.util.Suppliers.compose; +import java.util.Optional; +import java.util.function.BiFunction; import java.util.function.IntSupplier; import java.util.function.Supplier; import org.eclipse.emf.common.command.Command; +import org.eclipse.emf.common.command.CommandWrapper; +import org.eclipse.emf.common.command.IdentityCommand; import org.eclipse.emf.ecore.EObject; import org.eclipse.emf.edit.command.DeleteCommand; import org.eclipse.emf.edit.domain.EditingDomain; @@ -26,7 +30,6 @@ import org.eclipse.gmf.runtime.notation.Compartment; import org.eclipse.gmf.runtime.notation.Connector; import org.eclipse.gmf.runtime.notation.Diagram; -import org.eclipse.gmf.runtime.notation.IdentityAnchor; import org.eclipse.gmf.runtime.notation.LayoutConstraint; import org.eclipse.gmf.runtime.notation.Node; import org.eclipse.gmf.runtime.notation.NotationFactory; @@ -46,6 +49,7 @@ import org.eclipse.uml2.uml.Lifeline; import org.eclipse.uml2.uml.Message; import org.eclipse.uml2.uml.MessageEnd; +import org.eclipse.uml2.uml.OccurrenceSpecification; import org.eclipse.uml2.uml.UMLPackage; /** @@ -55,7 +59,6 @@ */ public class DefaultDiagramHelper implements DiagramHelper { - @SuppressWarnings("unused") private final EditingDomain editingDomain; private final Supplier semanticHelper; @@ -134,7 +137,7 @@ public CreationCommand createLifelineShape(Supplier l Shape body = (Shape)result.createChild(NotationPackage.Literals.SHAPE); body.setType(ViewTypes.LIFELINE_BODY); Size bodySize = NotationFactory.eINSTANCE.createSize(); - bodySize.setWidth(layoutHelper().getConstraints().getMinimumHeight(ViewTypes.LIFELINE_BODY)); + bodySize.setWidth(layoutHelper().getConstraints().getMinimumWidth(ViewTypes.LIFELINE_BODY)); bodySize.setHeight(layoutHelper().getConstraints().getMinimumHeight(ViewTypes.LIFELINE_BODY)); body.setLayoutConstraint(bodySize); @@ -235,7 +238,8 @@ public CreationCommand createDestructionOccurrenceShape(Supplier message, Supplier source, - IntSupplier sourceY, Supplier target, IntSupplier targetY) { + IntSupplier sourceY, Supplier target, IntSupplier targetY, + BiFunction> collisionHandler) { // TODO: The Logical Model is required to have no dependencies on the diagram editor, // but this is usually the responsibility of a View Provider. That should be okay ... @@ -247,36 +251,18 @@ public Command createMessageConnector(Supplier message, Supplier message, Supplier replace = Optional.empty(); + if (AnchorFactory.isExecutionSpecificationStart(created.getTargetAnchor())) { + // Replace the execution occurrence start occurrence + ExecutionSpecification exec = (ExecutionSpecification)target.get().getElement(); + replace = collisionHandler.apply(exec.getStart(), message.get().getReceiveEvent()); + } else if (AnchorFactory.isExecutionSpecificationFinish(created.getSourceAnchor())) { + // Replace the execution occurrence start occurrence + ExecutionSpecification exec = (ExecutionSpecification)source.get().getElement(); + replace = collisionHandler.apply(exec.getFinish(), message.get().getSendEvent()); + } + + // If we don't have a replace command, we still need something to execute + return replace.orElse(IdentityCommand.INSTANCE); + } + }; + result = result.chain(deferredCollisionHandler); + } + + return result; } @Override diff --git a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/spi/impl/DefaultLayoutConstraints.java b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/spi/impl/DefaultLayoutConstraints.java index ca5d2085..85b7d86e 100644 --- a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/spi/impl/DefaultLayoutConstraints.java +++ b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/spi/impl/DefaultLayoutConstraints.java @@ -152,6 +152,11 @@ public boolean isAsyncMessageSlope(double x1, double y1, double x2, double y2) { return slope >= 3.0; } + @Override + public int getMagnetStrength(View view) { + return 15; + } + @SuppressWarnings("boxing") private static Map loadXOffsets() { Map result = new HashMap<>(); diff --git a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/spi/impl/DefaultLayoutHelper.java b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/spi/impl/DefaultLayoutHelper.java index 761080cf..2e0dc5c0 100644 --- a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/spi/impl/DefaultLayoutHelper.java +++ b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/spi/impl/DefaultLayoutHelper.java @@ -386,6 +386,13 @@ public int getBottom(Shape shape) { return result; } + @Override + public int getHeight(Shape shape) { + int bottom = getBottom(shape); + int top = getTop(shape); + return ((bottom == DEFAULT_BOTTOM) || (top == DEFAULT_TOP)) ? DEFAULT_HEIGHT : bottom - top; + } + @Override public int getYPosition(Anchor anchor, Shape anchoredOn) { // Anchors are point locations, so the bottom is the top is the Y position. diff --git a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/model/spi/DiagramHelper.java b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/model/spi/DiagramHelper.java index e98e7534..36c3dd05 100644 --- a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/model/spi/DiagramHelper.java +++ b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/model/spi/DiagramHelper.java @@ -12,6 +12,8 @@ package org.eclipse.papyrus.uml.interaction.model.spi; +import java.util.Optional; +import java.util.function.BiFunction; import java.util.function.IntSupplier; import java.util.function.Supplier; @@ -26,6 +28,7 @@ import org.eclipse.uml2.uml.Lifeline; import org.eclipse.uml2.uml.Message; import org.eclipse.uml2.uml.MessageEnd; +import org.eclipse.uml2.uml.OccurrenceSpecification; /** * Protocol for a pluggable utility that provides creation of the visual elements of a sequence diagram. All @@ -126,11 +129,16 @@ CreationCommand createDestructionOccurrenceShape(Supplier message, // Supplier source, IntSupplier sourceY, // - Supplier target, IntSupplier targetY); + Supplier target, IntSupplier targetY, // + BiFunction> collisionHandler); /** * Obtain a command to delete a given {@code connector}. diff --git a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/model/spi/LayoutConstraints.java b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/model/spi/LayoutConstraints.java index fa817f69..aff37020 100644 --- a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/model/spi/LayoutConstraints.java +++ b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/model/spi/LayoutConstraints.java @@ -200,6 +200,15 @@ public interface Modifiers { */ boolean isAsyncMessageSlope(double x1, double y1, double x2, double y2); + /** + * Query the strength of a magnet attached to the given {@code view} + * + * @param view + * a notation view in the diagram that supports magnets + * @return the magnet strength + */ + int getMagnetStrength(View view); + /** * Applies a {@link Modifiers modifier} to the given viewType and returns the resulting key. * diff --git a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/model/spi/LayoutHelper.java b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/model/spi/LayoutHelper.java index ce0a5184..38a47d7a 100644 --- a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/model/spi/LayoutHelper.java +++ b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/model/spi/LayoutHelper.java @@ -69,6 +69,15 @@ public interface LayoutHelper { */ int getBottom(Shape shape); + /** + * Queries the height of a {@code shape}. + * + * @param shape + * a shape in the diagram + * @return its height + */ + int getHeight(Shape shape); + /** * Queries the vertical position (y-coördinate) of an {@code anchor} in the sequence diagram. * @@ -351,4 +360,5 @@ default int toRelativeY(Shape shape, int y) { * @return the pluggable layout constraints */ LayoutConstraints getConstraints(); + } diff --git a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/META-INF/MANIFEST.MF b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/META-INF/MANIFEST.MF index 1b0b11e0..4f52c332 100644 --- a/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/META-INF/MANIFEST.MF +++ b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/META-INF/MANIFEST.MF @@ -18,6 +18,7 @@ Require-Bundle: org.eclipse.core.runtime;bundle-version="[3.13.0,4.0.0)", org.eclipse.papyrus.infra.core.sashwindows.di, org.eclipse.gef, org.eclipse.gmf.runtime.emf.type.core, - org.eclipse.papyrus.infra.gmfdiag.common + org.eclipse.papyrus.infra.gmfdiag.common, + org.eclipse.papyrus.uml.diagram.sequence.figure;bundle-version="[1.0.0,2.0.0)" Bundle-Description: Tests of the core run-time APIs of the sequence diagram. Export-Package: org.eclipse.papyrus.uml.diagram.sequence.runtime.tests.matchers;version="1.0.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/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 55ecdcbb..fcac6dfb 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 @@ -110,7 +110,8 @@ public void attemptSlopedSyncMessage() { EditPart messageEP = createConnection(SequenceElementTypes.Sync_Message_Edge, at(sendX, 115), at(recvX, 130)); - assertThat("Message should be horizontal", messageEP, runs(sendX, 115, recvX, 115, 2)); + assertThat("Message should be horizontal to receive location", messageEP, + runs(sendX, 130, recvX, 130, 2)); } @Test 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 new file mode 100644 index 00000000..4fc69ad6 --- /dev/null +++ b/tests/org.eclipse.papyrus.uml.diagram.sequence.runtime.tests/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/tests/MessageSnappingUITest.java @@ -0,0 +1,323 @@ +/***************************************************************************** + * Copyright (c) 2018 Christian W. Damus and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Christian W. Damus - Initial API and implementation + *****************************************************************************/ + +package org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies.tests; + +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.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; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assume.assumeThat; + +import java.util.Arrays; +import java.util.function.Function; + +import org.eclipse.emf.common.command.Command; +import org.eclipse.emf.ecore.EObject; +import org.eclipse.emf.edit.command.MoveCommand; +import org.eclipse.emf.edit.domain.EditingDomain; +import org.eclipse.gef.EditPart; +import org.eclipse.gmf.runtime.diagram.ui.commands.SetBoundsCommand; +import org.eclipse.gmf.runtime.emf.core.util.EObjectAdapter; +import org.eclipse.gmf.runtime.notation.Location; +import org.eclipse.gmf.runtime.notation.Node; +import org.eclipse.papyrus.commands.wrappers.GMFtoGEFCommandWrapper; +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.rules.EditorFixture; +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.Interaction; +import org.eclipse.uml2.uml.InteractionFragment; +import org.eclipse.uml2.uml.Message; +import org.eclipse.uml2.uml.UMLPackage; +import org.hamcrest.CoreMatchers; +import org.hamcrest.Matcher; +import org.junit.Before; +import org.junit.Ignore; +import org.junit.Test; +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. + * + * @author Christian W. Damus + */ +@SuppressWarnings("restriction") +@ModelResource("two-lifelines.di") +@Maximized +@RunWith(Parameterized.class) +public class MessageSnappingUITest extends AbstractGraphicalEditPolicyUITest { + // 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 boolean EXEC_START = true; + private static final int EXEC_START_Y = 145; + + private static final boolean EXEC_FINISH = false; + private static final int EXEC_HEIGHT = 60; + private static final int EXEC_FINISH_Y = EXEC_START_Y + EXEC_HEIGHT; + + private final EditorFixture.Modifiers modifiers; + private final Function, Matcher> modifiersMatcherFunction; + private ExecutionSpecification exec; + + /** + * Initializes me. + * + * @param withSnap + * whether to allow snapping ({@code true}) or suppress it + * ({@code false}) + * @param snapString + * a string representation of {@code withSnap} + */ + public MessageSnappingUITest(boolean withSnap, String snapString) { + super(); + + this.modifiers = withSnap ? editor.unmodified() : editor.modifierKey(MODIFIER_NO_SNAPPING); + modifiersMatcherFunction = withSnap ? CoreMatchers::is : CoreMatchers::not; + } + + @Test + public void createSyncCallMessage() { + EditPart messageEP = editor.with(modifiers, + () -> createConnection(SequenceElementTypes.Sync_Message_Edge, + at(LL1_BODY_X, withinMagnet(EXEC_START)), + at(LL2_BODY_X, withinMagnet(EXEC_START)))); + + // The receiving end snaps to the exec start and the sending end matches + assertThat(messageEP, withModifiers(runs(LL1_BODY_X, EXEC_START_Y, LL2_BODY_X, EXEC_START_Y, 1))); + + // The message receive event starts the execution + Message message = (Message) messageEP.getAdapter(EObject.class); + assertThat(exec.getStart(), withModifiers(is(message.getReceiveEvent()))); + } + + @Test + public void createAsyncCallMessage() { + EditPart messageEP = editor.with(modifiers, + () -> createConnection(SequenceElementTypes.Async_Message_Edge, at(LL1_BODY_X, 120), + at(LL2_BODY_X, withinMagnet(EXEC_START)))); + + // The receiving end snaps to the exec start. The sending end doesn't match + assertThat(messageEP, withModifiers(runs(LL1_BODY_X, 120, LL2_BODY_X, EXEC_START_Y, 1))); + + // The message receive event starts the execution + Message message = (Message) messageEP.getAdapter(EObject.class); + assertThat(exec.getStart(), withModifiers(is(message.getReceiveEvent()))); + } + + @Test + public void createReplyMessage() { + EditPart messageEP = editor.with(modifiers, + () -> createConnection(SequenceElementTypes.Reply_Message_Edge, + at(LL2_BODY_X, withinMagnet(EXEC_FINISH)), + at(LL1_BODY_X, withinMagnet(EXEC_FINISH)))); + + // The sending end snaps to the exec start and the receiving end matches + assertThat(messageEP, withModifiers(runs(LL2_BODY_X, EXEC_FINISH_Y, LL1_BODY_X, EXEC_FINISH_Y, 1))); + + // The message send event finishes the execution + Message message = (Message) messageEP.getAdapter(EObject.class); + assertThat(exec.getFinish(), withModifiers(is(message.getSendEvent()))); + } + + @Test + public void moveSyncCallMessage() { + EditPart messageEP = createConnection(SequenceElementTypes.Sync_Message_Edge, at(LL1_BODY_X, 120), + at(LL2_BODY_X, 120)); + assumeThat(messageEP, runs(LL1_BODY_X, 120, LL2_BODY_X, 120)); + + ensureMessageEndsFirst(messageEP); + + // The receiving end snaps to the exec start and the sending end matches + final int midMessage = (LL1_BODY_X + LL2_BODY_X) / 2; + editor.with(modifiers, + () -> editor.moveSelection(at(midMessage, 120), at(midMessage, withinMagnet(EXEC_START)))); + assertThat(messageEP, withModifiers(runs(LL1_BODY_X, EXEC_START_Y, LL2_BODY_X, EXEC_START_Y, 1))); + } + + @Ignore("Logical model not calculating bottom of execution correctly.") + @Test + public void moveReplyMessage() { + EditPart messageEP = createConnection(SequenceElementTypes.Reply_Message_Edge, at(LL2_BODY_X, 240), + at(LL1_BODY_X, 240)); + assumeThat(messageEP, runs(LL2_BODY_X, 240, LL1_BODY_X, 240)); + + ensureMessageEndsLast(messageEP); + + // The sending end snaps to the exec start and the receiving end matches + final int midMessage = (LL1_BODY_X + LL2_BODY_X) / 2; + editor.with(modifiers, + () -> editor.moveSelection(at(midMessage, 240), at(midMessage, withinMagnet(EXEC_FINISH)))); + assertThat(messageEP, withModifiers(runs(LL2_BODY_X, EXEC_FINISH_Y, LL1_BODY_X, EXEC_FINISH_Y, 1))); + } + + /** + * Verify that magnets are updated when the size of an execution occurrence + * changes. + */ + @Test + public void magnetUpdatesOnSize() { + assumeThat("Only makes sense with snapping enabled", "magnets suppressed", + withModifiers(is("magnets suppressed"))); + + // First, extend the bottom edge of the execution specification + int newBottomY = EXEC_FINISH_Y + 100; + editor.moveSelection(at(LL2_BODY_X, EXEC_FINISH_Y), at(LL2_BODY_X, newBottomY)); + + EditPart messageEP = editor.with(modifiers, + () -> createConnection(SequenceElementTypes.Reply_Message_Edge, // + at(LL2_BODY_X, withinMagnet(newBottomY, EXEC_FINISH)), + at(LL1_BODY_X, withinMagnet(newBottomY, EXEC_FINISH)))); + assertThat("No snap: infer that magnet not moved", messageEP, + runs(LL2_BODY_X, newBottomY, LL1_BODY_X, newBottomY, 1)); + } + + /** + * Verify that magnets are updated when the location of an execution occurrence + * changes. + */ + @Test + public void magnetUpdatesOnLocation() { + assumeThat("Only makes sense with snapping enabled", "magnets suppressed", + withModifiers(is("magnets suppressed"))); + + // First, move the execution specification down. The edit-part doesn't (yet) + // have a policy for moving it (only for resize) so be direct about it instead + // of automating the selection tool + EditPart execEP = getLastCreatedEditPart(); + Node execView = (Node) execEP.getModel(); + Location location = (Location) execView.getLayoutConstraint(); + SetBoundsCommand command = new SetBoundsCommand(editor.getDiagramEditPart().getEditingDomain(), + "Move execution down", new EObjectAdapter(execView), + at(location.getX(), location.getY() + 100)); + editor.getDiagramEditPart().getDiagramEditDomain().getDiagramCommandStack() + .execute(GMFtoGEFCommandWrapper.wrap(command)); + + int newTopY = EXEC_START_Y + 100; + + EditPart messageEP = editor.with(modifiers, + () -> createConnection(SequenceElementTypes.Reply_Message_Edge, // + at(LL2_BODY_X, withinMagnet(newTopY, EXEC_START)), + at(LL1_BODY_X, withinMagnet(newTopY, EXEC_START)))); + assertThat("No snap: infer that magnet not moved", messageEP, + runs(LL2_BODY_X, newTopY, LL1_BODY_X, newTopY, 1)); + } + + // + // Test framework + // + + @Parameters(name = "{1}") + public static Iterable parameters() { + return Arrays.asList(new Object[][] { // + { true, "snap to magnet" }, // + { false, "suppress snapping" }, // + }); + } + + @Before + public void createExecutionSpecification() { + EditPart exec = createShape(SequenceElementTypes.Behavior_Execution_Shape, + at(LL2_BODY_X, EXEC_START_Y), sized(0, EXEC_HEIGHT)); + assumeThat("Execution specification not created", exec, notNullValue()); + + this.exec = (ExecutionSpecification) exec.getAdapter(EObject.class); + } + + @SuppressWarnings("unchecked") + Matcher withModifiers(Matcher matcher) { + return (Matcher) modifiersMatcherFunction.apply(matcher); + } + + void ensureMessageEndsFirst(EditPart connectionEP) { + Command adjust = null; + + EditingDomain domain = editor.getEditingDomain(); + Message message = (Message) connectionEP.getAdapter(EObject.class); + InteractionFragment send = (InteractionFragment) message.getSendEvent(); + InteractionFragment recv = (InteractionFragment) message.getReceiveEvent(); + Interaction interaction = message.getInteraction(); + + int sendIndex = interaction.getFragments().indexOf(send); + if (sendIndex != 0) { + Command move = MoveCommand.create(domain, interaction, + UMLPackage.Literals.INTERACTION__FRAGMENT, send, 0); + adjust = move; + } + int recvIndex = interaction.getFragments().indexOf(recv); + if (recvIndex != 1) { + Command move = MoveCommand.create(domain, interaction, + UMLPackage.Literals.INTERACTION__FRAGMENT, recv, 1); + if (adjust == null) { + adjust = move; + } else { + adjust = adjust.chain(move); + } + } + if (adjust != null) { + domain.getCommandStack().execute(adjust); + } + } + + void ensureMessageEndsLast(EditPart connectionEP) { + Command adjust = null; + + EditingDomain domain = editor.getEditingDomain(); + Message message = (Message) connectionEP.getAdapter(EObject.class); + InteractionFragment send = (InteractionFragment) message.getSendEvent(); + InteractionFragment recv = (InteractionFragment) message.getReceiveEvent(); + Interaction interaction = message.getInteraction(); + int secondLast = interaction.getFragments().size() - 2; + int last = interaction.getFragments().size() - 1; + + int sendIndex = interaction.getFragments().indexOf(send); + if (sendIndex != secondLast) { + Command move = MoveCommand.create(domain, interaction, + UMLPackage.Literals.INTERACTION__FRAGMENT, send, secondLast); + adjust = move; + } + int recvIndex = interaction.getFragments().indexOf(recv); + if (recvIndex != last) { + Command move = MoveCommand.create(domain, interaction, + UMLPackage.Literals.INTERACTION__FRAGMENT, recv, last); + if (adjust == null) { + adjust = move; + } else { + adjust = adjust.chain(move); + } + } + if (adjust != null) { + domain.getCommandStack().execute(adjust); + editor.flushDisplayEvents(); + } + } + + static int withinMagnet(boolean execStart) { + return withinMagnet(execStart ? EXEC_START_Y : EXEC_FINISH_Y, execStart); + } + + static int withinMagnet(int y, boolean execStart) { + return execStart ? y - 9 : y + 9; + } +} 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 8b24ec14..543fde76 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 @@ -22,6 +22,9 @@ import java.util.HashSet; import java.util.Optional; import java.util.Set; +import java.util.function.Consumer; +import java.util.function.IntConsumer; +import java.util.function.IntSupplier; import java.util.function.Supplier; import java.util.stream.Stream; @@ -37,10 +40,10 @@ import org.eclipse.gef.ConnectionEditPart; import org.eclipse.gef.EditPart; import org.eclipse.gef.EditPartViewer; +import org.eclipse.gef.tools.ConnectionCreationTool; import org.eclipse.gef.tools.SelectionTool; import org.eclipse.gmf.runtime.diagram.ui.editparts.DiagramEditPart; import org.eclipse.gmf.runtime.diagram.ui.parts.DiagramEditor; -import org.eclipse.gmf.runtime.diagram.ui.services.palette.SelectionToolEx; import org.eclipse.gmf.runtime.emf.type.core.IElementType; import org.eclipse.gmf.runtime.notation.Diagram; import org.eclipse.papyrus.infra.core.sasheditor.di.contentprovider.utils.IPageUtils; @@ -48,7 +51,6 @@ import org.eclipse.papyrus.infra.core.sasheditor.editor.IPage; import org.eclipse.papyrus.infra.core.sasheditor.editor.ISashWindowsContainer; import org.eclipse.papyrus.infra.core.sashwindows.di.service.IPageManager; -import org.eclipse.papyrus.infra.gmfdiag.common.service.palette.AspectUnspecifiedTypeConnectionTool; import org.eclipse.papyrus.infra.gmfdiag.common.service.palette.AspectUnspecifiedTypeCreationTool; import org.eclipse.papyrus.uml.interaction.tests.rules.ModelFixture; import org.eclipse.swt.SWT; @@ -79,6 +81,9 @@ public class EditorFixture extends ModelFixture { private IEditorPart editor; private boolean isMaximized; + private int mouseButton = 1; + private int modifierKeys = 0; + /** * Initializes me. * @@ -289,7 +294,8 @@ public EditPart createShape(IElementType type, Point location, Dimension size) { tool.mouseMove(new MouseEvent(mouse), viewer); // Start the click - mouse.button = 1; + mouse.button = mouseButton; + mouse.stateMask = modifierKeys; mouse.type = SWT.MouseDown; tool.mouseDown(new MouseEvent(mouse), viewer); @@ -365,8 +371,7 @@ public EditPart createConnection(IElementType type, Point start, Point finish) { private void drawConnection(IElementType type, Point start, Point finish, boolean complete) { DiagramEditPart diagram = getDiagramEditPart(); EditPartViewer viewer = diagram.getViewer(); - AspectUnspecifiedTypeConnectionTool tool = new AspectUnspecifiedTypeConnectionTool( - singletonList(type)); + ConnectionCreationTool tool = createConnectionTool(type); Event mouse = new Event(); mouse.display = editor.getSite().getShell().getDisplay(); @@ -382,7 +387,8 @@ private void drawConnection(IElementType type, Point start, Point finish, boolea tool.mouseMove(new MouseEvent(mouse), viewer); // Click - mouse.button = 1; + mouse.button = mouseButton; + mouse.stateMask = modifierKeys; mouse.type = SWT.MouseDown; tool.mouseDown(new MouseEvent(mouse), viewer); mouse.type = SWT.MouseUp; @@ -400,7 +406,8 @@ private void drawConnection(IElementType type, Point start, Point finish, boolea flushDisplayEvents(); if (complete) { - mouse.button = 1; + mouse.button = mouseButton; + mouse.stateMask = modifierKeys; mouse.type = SWT.MouseDown; tool.mouseDown(new MouseEvent(mouse), viewer); mouse.type = SWT.MouseUp; @@ -433,7 +440,7 @@ public void hoverConnection(IElementType type, Point start, Point finish) { public void escape() { DiagramEditPart diagram = getDiagramEditPart(); EditPartViewer viewer = diagram.getViewer(); - SelectionTool tool = new SelectionToolEx(); + SelectionTool tool = createSelectionTool(); Event key = new Event(); key.display = editor.getSite().getShell().getDisplay(); @@ -465,7 +472,7 @@ public void moveSelection(Point start, Point finish) { DiagramEditPart diagram = getDiagramEditPart(); EditPartViewer viewer = diagram.getViewer(); - SelectionTool tool = new SelectionToolEx(); + SelectionTool tool = createSelectionTool(); Event mouse = new Event(); mouse.display = editor.getSite().getShell().getDisplay(); @@ -481,7 +488,8 @@ public void moveSelection(Point start, Point finish) { tool.mouseMove(new MouseEvent(mouse), viewer); // Click to select - mouse.button = 1; + mouse.button = mouseButton; + mouse.stateMask = modifierKeys; mouse.type = SWT.MouseDown; tool.mouseDown(new MouseEvent(mouse), viewer); mouse.type = SWT.MouseUp; @@ -531,6 +539,10 @@ public void moveSelection(Point start, Point finish) { // Utilities // + public final EditingDomain getEditingDomain() { + return getDiagramEditPart().getEditingDomain(); + } + public final void flushDisplayEvents() { Display display = Display.getCurrent(); while (display.readAndDispatch()) { @@ -598,4 +610,176 @@ public static Dimension sized(int width, int height) { return new Dimension(width, height); } + /** + * Run an {@code action} with {@code modifiers}. + * + * @param modifiers + * the modifiers to apply to the {@code action} + * @param action + * an action to run under the influence of the {@code modifiers} + */ + public final void with(Modifiers modifiers, Runnable action) { + modifiers.apply(); + + try { + action.run(); + } finally { + modifiers.unapply(); + } + } + + /** + * Run an {@code action} computing a result with {@code modifiers}. + * + * @param modifiers + * the modifiers to apply to the {@code action} + * @param action + * an action to run under the influence of the {@code modifiers} + */ + public final T with(Modifiers modifiers, Supplier action) { + final T result; + + modifiers.apply(); + + try { + result = action.get(); + } finally { + modifiers.unapply(); + } + + return result; + } + + /** + * Obtain modifiers applying a mouse button to mouse events. + * + * @param mouseButton + * the mouse button + * @return the mouse button modifiers + */ + public Modifiers mouseButton(int mouseButton) { + return ModifiersImpl.withInt( // + () -> setMouseButton(mouseButton), // + this::setMouseButton); + } + + private int setMouseButton(int mouseButton) { + int result = this.mouseButton; + this.mouseButton = mouseButton; + return result; + } + + /** + * Obtain modifiers applying a modifier key to mouse and keyboard events. + * + * @param modifierKey + * the modifier key code + * @return the modifier key modifiers + */ + public Modifiers modifierKey(int modifierKey) { + return ModifiersImpl.withInt( // + () -> setModifierKeys(this.modifierKeys | modifierKey), // + this::setModifierKeys); + } + + private int setModifierKeys(int modifierKeys) { + int result = this.modifierKeys; + this.modifierKeys = modifierKeys; + return result; + } + + /** + * Absence of modifiers, useful for clients that require some kind of modifiers + * instance, even if actual modifiers are not needed. + * + * @return a modifiers implementation that does nothing + */ + public Modifiers unmodified() { + return ModifiersImpl.with(this::pass, this::pass); + } + + private void pass() { + // Pass + } + + @SuppressWarnings("restriction") + private SelectionTool createSelectionTool() { + return new org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.tools.SequenceSelectionTool(); + } + + @SuppressWarnings("restriction") + private ConnectionCreationTool createConnectionTool(IElementType type) { + return new org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.tools.SequenceConnectionCreationTool( + singletonList(type)); + } + + // + // Nested types + // + + /** + * Protocol for modifiers rules for interaction gestures in the diagram. + */ + public interface Modifiers { + /** Applies my modifiers. */ + void apply(); + + /** Removes my modifiers. */ + void unapply(); + + /** + * Wrap another {@code modifiers} around me. + * + * @param modifiers + * other modifiers to apply around mine + * @return the composed modifiers + */ + default Modifiers and(Modifiers modifiers) { + return ModifiersImpl.with( // + () -> { + modifiers.apply(); + this.apply(); + }, () -> { + this.unapply(); + modifiers.unapply(); + }); + } + } + + private static final class ModifiersImpl implements Modifiers { + private final Runnable apply; + private final Runnable unapply; + + private ModifiersImpl(Runnable apply, Runnable unapply) { + super(); + + this.apply = apply; + this.unapply = unapply; + } + + static Modifiers with(Runnable apply, Runnable unapply) { + return new ModifiersImpl(apply, unapply); + } + + static Modifiers with(Supplier apply, Consumer unapply) { + @SuppressWarnings("unchecked") + final T[] holder = (T[]) new Object[1]; + return with(() -> holder[0] = apply.get(), () -> unapply.accept(holder[0])); + } + + static Modifiers withInt(IntSupplier apply, IntConsumer unapply) { + final int[] holder = { 0 }; + return with(() -> holder[0] = apply.getAsInt(), () -> unapply.accept(holder[0])); + } + + @Override + public void apply() { + apply.run(); + } + + @Override + public void unapply() { + unapply.run(); + } + } } diff --git a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/CreateMessageTest.java b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/CreateMessageTest.java index 9f08ed77..50cbd31d 100644 --- a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/CreateMessageTest.java +++ b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/CreateMessageTest.java @@ -6,6 +6,7 @@ import org.eclipse.gmf.runtime.notation.View; import org.eclipse.papyrus.uml.interaction.internal.model.impl.LogicalModelPlugin; import org.eclipse.papyrus.uml.interaction.model.CreationCommand; +import org.eclipse.papyrus.uml.interaction.model.MExecution; import org.eclipse.papyrus.uml.interaction.model.MInteraction; import org.eclipse.papyrus.uml.interaction.model.MLifeline; import org.eclipse.papyrus.uml.interaction.model.spi.DiagramHelper; @@ -121,13 +122,20 @@ public void creationMessage() { assertEquals(3, interaction().getMessages().size()); assertEquals(MessageSort.CREATE_MESSAGE_LITERAL, interaction().getMessages().get(2).getElement().getMessageSort()); - assertEquals(getLifelineBodyTop(interaction().getLifelines().get(0)) + 10, - interaction().getMessages().get(2).getTop().getAsInt()); + + /* + * we created this message sending from the start of an execution, so if that execution was nudged + * down, then the message will have followed. Therefore, assert the message location based on the + * sending execution + */ + MExecution exec = lifeline1.getExecutions().get(0); + assertEquals(exec.getTop().getAsInt(), interaction().getMessages().get(2).getTop().getAsInt()); assertEquals(lifeline1Top, interaction().getLifelines().get(0).getTop().getAsInt()); assertEquals(lifeline3Top, interaction().getLifelines().get(2).getTop().getAsInt()); - int nudgedLifeline2Top = interaction().getMessages().get(2).getTop().getAsInt() + /* we drew the creation message to the middle of the side of the lifeline head */ + int nudgedLifeline2Top = getLifelineBodyTop(interaction().getLifelines().get(0)) + 10 // - (lifeline2Header / 2); assertEquals(nudgedLifeline2Top, interaction().getLifelines().get(1).getTop().getAsInt()); From 1c125a9859b664bb09a6da3edd9849ab7fd751ad Mon Sep 17 00:00:00 2001 From: "Christian W. Damus" Date: Thu, 6 Sep 2018 16:50:48 -0400 Subject: [PATCH 02/11] Fix issue #73: Semantic ordering of fragments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix the semantic ordering of fragments on creation of a message, inserting new message ends more correctly relative to the existing interaction fragments in the interaction’s ordered fragments list. This removes the need for work-arounds in the tests that post facto adjusted semantic ordering. --- .../model/commands/InsertMessageCommand.java | 33 ++++--- .../internal/model/commands/ModelCommand.java | 85 +++++++++++++++++++ .../policies/tests/MessageSnappingUITest.java | 73 ---------------- 3 files changed, 106 insertions(+), 85 deletions(-) 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 83719b43..98b478c1 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 @@ -293,14 +293,24 @@ protected Command createCommand() { break; } - MElement sendInsertionPoint = normalizeFragmentInsertionPoint(beforeSend); - MElement recvInsertionPoint = normalizeFragmentInsertionPoint(beforeRecv); + // Determine the semantic elements after which to insert each message end + List> timeline = getTimeline(getTarget().getInteraction()); + int absoluteSendY = sendReferenceY.getAsInt() + sendOffset; + int absoluteRecvY = recvReferenceY.getAsInt() + recvOffset; + Optional> sendInsert = getInsertionPoint(timeline, absoluteSendY); + Optional> recvInsert = getInsertionPoint(timeline, absoluteRecvY); SemanticHelper semantics = semanticHelper(); - CreationParameters sendParams = endParams(sendInsertionPoint); + CreationParameters sendParams = endParams( + () -> sendInsert.map(MElement::getElement).map(Element.class::cast).orElse(null)); CreationCommand sendEvent = semantics.createMessageOccurrence(sendParams); CreationParameters recvParams = syncMessage ? CreationParameters.after(sendEvent) - : endParams(recvInsertionPoint); + // If the insertion point is the same or unspecified, then follow the send event + : !recvInsert.isPresent() || recvInsert.equals(sendInsert) + ? CreationParameters.after(sendEvent) + // Otherwise, it is specific + : endParams(() -> recvInsert.map(MElement::getElement).map(Element.class::cast) + .orElse(null)); CreationCommand recvEvent; switch (sort) { case DELETE_MESSAGE_LITERAL: @@ -416,8 +426,8 @@ protected Command createCommand() { // Now we have commands to add the message specification. But, first we must make // room for it in the diagram. Nudge the element that will follow the new receive event int spaceRequired = 2 * sendOffset; - MElement distanceFrom = sendInsertionPoint; - Optional makeSpace = getTarget().following(sendInsertionPoint).map(el -> { + MElement distanceFrom = normalizeFragmentInsertionPoint(beforeSend); + Optional makeSpace = getTarget().following(distanceFrom).map(el -> { OptionalInt distance = el.verticalDistance(distanceFrom); return distance.isPresent() ? el.nudge(max(0, spaceRequired - distance.getAsInt())) : null; @@ -448,12 +458,11 @@ private static void findElementsBelow(int yPosition, List elementsBelow)); } - private CreationParameters endParams(MElement insertionPoint) { - return insertionPoint instanceof MLifeline - // Just append to the fragments in that case - ? CreationParameters.in(insertionPoint.getInteraction().getElement(), - UMLPackage.Literals.INTERACTION__FRAGMENT) - : CreationParameters.after(insertionPoint.getElement()); + private CreationParameters endParams(Supplier insertionPoint) { + CreationParameters result = CreationParameters.in(getTarget().getInteraction().getElement(), + UMLPackage.Literals.INTERACTION__FRAGMENT); + result.setInsertBefore(insertionPoint); + return result; } private Optional replace(OccurrenceSpecification occurrence, MessageEnd msgEnd) { diff --git a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/ModelCommand.java b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/ModelCommand.java index da4aa164..ce85870c 100644 --- a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/ModelCommand.java +++ b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/ModelCommand.java @@ -12,7 +12,13 @@ package org.eclipse.papyrus.uml.interaction.internal.model.commands; +import java.lang.reflect.Proxy; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; import java.util.Optional; +import java.util.OptionalInt; +import java.util.stream.Collectors; import org.eclipse.emf.common.command.Command; import org.eclipse.emf.common.command.CommandWrapper; @@ -24,6 +30,7 @@ import org.eclipse.papyrus.uml.interaction.internal.model.impl.MInteractionImpl; import org.eclipse.papyrus.uml.interaction.model.MElement; import org.eclipse.papyrus.uml.interaction.model.MExecution; +import org.eclipse.papyrus.uml.interaction.model.MInteraction; import org.eclipse.papyrus.uml.interaction.model.MMessage; import org.eclipse.papyrus.uml.interaction.model.MMessageEnd; import org.eclipse.papyrus.uml.interaction.model.MOccurrence; @@ -139,6 +146,84 @@ protected MElement normalizeFragmentInsertionPoint(MElement insertionPoint return result; } + /** + * Get the global y-ordered timeline of interaction fragments. + * + * @param interaction + * an interaction + * @return the totality of its interaction fragments, ordered in time from top to bottom Y position + */ + protected static List> getTimeline(MInteraction interaction) { + return interaction.getElement().getFragments().stream().map(interaction::getElement) + .filter(Optional::isPresent).map(Optional::get) // + .sorted(compareByTop()) // + .collect(Collectors.toList()); + } + + private static Comparator> compareByTop() { + return Comparator.comparingInt(el -> el.getTop().orElse(-1)); + } + + /** + * Find the interaction fragment in a {@code timeline} before which an interaction fragment should be + * inserted that would be at the given absolute Y location. + * + * @param timeline + * a time-ordered (from top to bottom in the Y axis) sequence of interaction fragments + * @param the + * position on the Y axis at which a new interaction fragment is to be inserted + * @return the interaction fragment before which to insert the new fragment in the semantic model, or an + * empty optional if the new fragment is to be the last fragment in the interaction + */ + protected static Optional> getInsertionPoint( + List> timeline, int yPosition) { + + final MElement searchToken = ySearch(yPosition); + int size = timeline.size(); + int index = Collections.binarySearch(timeline, searchToken, compareByTop()); + + if (index >= 0) { + // Easy case: follow the fragment currently at that Y position, which means insert + // before the first element following that is at a greater position + while ((index < size) && (timeline.get(index).getTop().orElse(-1) == yPosition)) { + index++; + } + } else { + // The search found an "insertion point", which is where the fragment is before which + // the new one should be inserted + index = -index - 1; + } + + return (index < size) + // Insert before this one + ? Optional.of(timeline.get(index)) + // Append to the end + : Optional.empty(); + } + + /** + * Obtain a fake logical element representing an interaction fragment having a given {@code top} Y + * position to be use as a key for searching an ordered timeline. + * + * @param top + * the Y position being searched, to publish as the "top" of a fake element + * @return the search term + * @see #getTimeline(MInteraction) + * @see #getInsertionPoint(List, int) + */ + @SuppressWarnings("unchecked") + private static MElement ySearch(int top) { + return (MElement)Proxy.newProxyInstance(MElement.class.getClassLoader(), + new Class[] {MElement.class }, (proxy, method, args) -> { + switch (method.getName()) { + case "getTop": //$NON-NLS-1$ + return OptionalInt.of(top); + default: + return null; + } + }); + } + @Override public Command chain(Command next) { return CompoundModelCommand.compose(getEditingDomain(), this, next); 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 4fc69ad6..6498ebfb 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 @@ -24,10 +24,7 @@ import java.util.Arrays; import java.util.function.Function; -import org.eclipse.emf.common.command.Command; import org.eclipse.emf.ecore.EObject; -import org.eclipse.emf.edit.command.MoveCommand; -import org.eclipse.emf.edit.domain.EditingDomain; import org.eclipse.gef.EditPart; import org.eclipse.gmf.runtime.diagram.ui.commands.SetBoundsCommand; import org.eclipse.gmf.runtime.emf.core.util.EObjectAdapter; @@ -40,10 +37,7 @@ 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.Interaction; -import org.eclipse.uml2.uml.InteractionFragment; import org.eclipse.uml2.uml.Message; -import org.eclipse.uml2.uml.UMLPackage; import org.hamcrest.CoreMatchers; import org.hamcrest.Matcher; import org.junit.Before; @@ -147,8 +141,6 @@ public void moveSyncCallMessage() { at(LL2_BODY_X, 120)); assumeThat(messageEP, runs(LL1_BODY_X, 120, LL2_BODY_X, 120)); - ensureMessageEndsFirst(messageEP); - // The receiving end snaps to the exec start and the sending end matches final int midMessage = (LL1_BODY_X + LL2_BODY_X) / 2; editor.with(modifiers, @@ -163,8 +155,6 @@ public void moveReplyMessage() { at(LL1_BODY_X, 240)); assumeThat(messageEP, runs(LL2_BODY_X, 240, LL1_BODY_X, 240)); - ensureMessageEndsLast(messageEP); - // The sending end snaps to the exec start and the receiving end matches final int midMessage = (LL1_BODY_X + LL2_BODY_X) / 2; editor.with(modifiers, @@ -250,69 +240,6 @@ Matcher withModifiers(Matcher matcher) { return (Matcher) modifiersMatcherFunction.apply(matcher); } - void ensureMessageEndsFirst(EditPart connectionEP) { - Command adjust = null; - - EditingDomain domain = editor.getEditingDomain(); - Message message = (Message) connectionEP.getAdapter(EObject.class); - InteractionFragment send = (InteractionFragment) message.getSendEvent(); - InteractionFragment recv = (InteractionFragment) message.getReceiveEvent(); - Interaction interaction = message.getInteraction(); - - int sendIndex = interaction.getFragments().indexOf(send); - if (sendIndex != 0) { - Command move = MoveCommand.create(domain, interaction, - UMLPackage.Literals.INTERACTION__FRAGMENT, send, 0); - adjust = move; - } - int recvIndex = interaction.getFragments().indexOf(recv); - if (recvIndex != 1) { - Command move = MoveCommand.create(domain, interaction, - UMLPackage.Literals.INTERACTION__FRAGMENT, recv, 1); - if (adjust == null) { - adjust = move; - } else { - adjust = adjust.chain(move); - } - } - if (adjust != null) { - domain.getCommandStack().execute(adjust); - } - } - - void ensureMessageEndsLast(EditPart connectionEP) { - Command adjust = null; - - EditingDomain domain = editor.getEditingDomain(); - Message message = (Message) connectionEP.getAdapter(EObject.class); - InteractionFragment send = (InteractionFragment) message.getSendEvent(); - InteractionFragment recv = (InteractionFragment) message.getReceiveEvent(); - Interaction interaction = message.getInteraction(); - int secondLast = interaction.getFragments().size() - 2; - int last = interaction.getFragments().size() - 1; - - int sendIndex = interaction.getFragments().indexOf(send); - if (sendIndex != secondLast) { - Command move = MoveCommand.create(domain, interaction, - UMLPackage.Literals.INTERACTION__FRAGMENT, send, secondLast); - adjust = move; - } - int recvIndex = interaction.getFragments().indexOf(recv); - if (recvIndex != last) { - Command move = MoveCommand.create(domain, interaction, - UMLPackage.Literals.INTERACTION__FRAGMENT, recv, last); - if (adjust == null) { - adjust = move; - } else { - adjust = adjust.chain(move); - } - } - if (adjust != null) { - domain.getCommandStack().execute(adjust); - editor.flushDisplayEvents(); - } - } - static int withinMagnet(boolean execStart) { return withinMagnet(execStart ? EXEC_START_Y : EXEC_FINISH_Y, execStart); } From 1f60803a6c94393aca01f545384da5a3c37503d8 Mon Sep 17 00:00:00 2001 From: "Christian W. Damus" Date: Fri, 7 Sep 2018 09:05:54 -0400 Subject: [PATCH 03/11] Issue #73: Empiricism in the tests Measure the actual top and bottom of the execution specification as actually rendered differently on different platforms, instead of assuming that it ends up exactly where requested via the API. --- .../policies/tests/MessageSnappingUITest.java | 54 ++++++++++++++----- 1 file changed, 41 insertions(+), 13 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/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 6498ebfb..aae110f4 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 @@ -16,7 +16,9 @@ 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; +import static org.eclipse.papyrus.uml.interaction.tests.matchers.NumberMatchers.isNear; import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.CoreMatchers.notNullValue; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assume.assumeThat; @@ -24,8 +26,11 @@ import java.util.Arrays; import java.util.function.Function; +import org.eclipse.draw2d.IFigure; +import org.eclipse.draw2d.geometry.Rectangle; import org.eclipse.emf.ecore.EObject; import org.eclipse.gef.EditPart; +import org.eclipse.gef.GraphicalEditPart; import org.eclipse.gmf.runtime.diagram.ui.commands.SetBoundsCommand; import org.eclipse.gmf.runtime.emf.core.util.EObjectAdapter; import org.eclipse.gmf.runtime.notation.Location; @@ -69,11 +74,12 @@ public class MessageSnappingUITest extends AbstractGraphicalEditPolicyUITest { private static final boolean EXEC_FINISH = false; private static final int EXEC_HEIGHT = 60; - private static final int EXEC_FINISH_Y = EXEC_START_Y + EXEC_HEIGHT; private final EditorFixture.Modifiers modifiers; private final Function, Matcher> modifiersMatcherFunction; private ExecutionSpecification exec; + private int execTop; + private int execBottom; /** * Initializes me. @@ -99,7 +105,7 @@ public void createSyncCallMessage() { at(LL2_BODY_X, withinMagnet(EXEC_START)))); // The receiving end snaps to the exec start and the sending end matches - assertThat(messageEP, withModifiers(runs(LL1_BODY_X, EXEC_START_Y, LL2_BODY_X, EXEC_START_Y, 1))); + assertThat(messageEP, withModifiers(runs(LL1_BODY_X, execTop, LL2_BODY_X, execTop, 1))); // The message receive event starts the execution Message message = (Message) messageEP.getAdapter(EObject.class); @@ -113,7 +119,7 @@ public void createAsyncCallMessage() { at(LL2_BODY_X, withinMagnet(EXEC_START)))); // The receiving end snaps to the exec start. The sending end doesn't match - assertThat(messageEP, withModifiers(runs(LL1_BODY_X, 120, LL2_BODY_X, EXEC_START_Y, 1))); + assertThat(messageEP, withModifiers(runs(LL1_BODY_X, 120, LL2_BODY_X, execTop, 1))); // The message receive event starts the execution Message message = (Message) messageEP.getAdapter(EObject.class); @@ -128,7 +134,7 @@ public void createReplyMessage() { at(LL1_BODY_X, withinMagnet(EXEC_FINISH)))); // The sending end snaps to the exec start and the receiving end matches - assertThat(messageEP, withModifiers(runs(LL2_BODY_X, EXEC_FINISH_Y, LL1_BODY_X, EXEC_FINISH_Y, 1))); + assertThat(messageEP, withModifiers(runs(LL2_BODY_X, execBottom, LL1_BODY_X, execBottom, 1))); // The message send event finishes the execution Message message = (Message) messageEP.getAdapter(EObject.class); @@ -145,7 +151,7 @@ public void moveSyncCallMessage() { final int midMessage = (LL1_BODY_X + LL2_BODY_X) / 2; editor.with(modifiers, () -> editor.moveSelection(at(midMessage, 120), at(midMessage, withinMagnet(EXEC_START)))); - assertThat(messageEP, withModifiers(runs(LL1_BODY_X, EXEC_START_Y, LL2_BODY_X, EXEC_START_Y, 1))); + assertThat(messageEP, withModifiers(runs(LL1_BODY_X, execTop, LL2_BODY_X, execTop, 1))); } @Ignore("Logical model not calculating bottom of execution correctly.") @@ -159,7 +165,7 @@ public void moveReplyMessage() { final int midMessage = (LL1_BODY_X + LL2_BODY_X) / 2; editor.with(modifiers, () -> editor.moveSelection(at(midMessage, 240), at(midMessage, withinMagnet(EXEC_FINISH)))); - assertThat(messageEP, withModifiers(runs(LL2_BODY_X, EXEC_FINISH_Y, LL1_BODY_X, EXEC_FINISH_Y, 1))); + assertThat(messageEP, withModifiers(runs(LL2_BODY_X, execBottom, LL1_BODY_X, execBottom, 1))); } /** @@ -171,9 +177,12 @@ public void magnetUpdatesOnSize() { assumeThat("Only makes sense with snapping enabled", "magnets suppressed", withModifiers(is("magnets suppressed"))); - // First, extend the bottom edge of the execution specification - int newBottomY = EXEC_FINISH_Y + 100; - editor.moveSelection(at(LL2_BODY_X, EXEC_FINISH_Y), at(LL2_BODY_X, newBottomY)); + // First, extend the bottom edge of the execution specification. Have to + // subtract 1 to grab on (inside-ish) the actual bottom edge + editor.moveSelection(at(LL2_BODY_X, execBottom - 1), at(LL2_BODY_X, execBottom + 100)); + + int newBottomY = getBottom(getLastCreatedEditPart()); + assumeThat("Execution not stretched", newBottomY, not(isNear(execBottom, 5))); EditPart messageEP = editor.with(modifiers, () -> createConnection(SequenceElementTypes.Reply_Message_Edge, // @@ -204,7 +213,7 @@ public void magnetUpdatesOnLocation() { editor.getDiagramEditPart().getDiagramEditDomain().getDiagramCommandStack() .execute(GMFtoGEFCommandWrapper.wrap(command)); - int newTopY = EXEC_START_Y + 100; + int newTopY = execTop + 100; EditPart messageEP = editor.with(modifiers, () -> createConnection(SequenceElementTypes.Reply_Message_Edge, // @@ -233,6 +242,11 @@ public void createExecutionSpecification() { assumeThat("Execution specification not created", exec, notNullValue()); this.exec = (ExecutionSpecification) exec.getAdapter(EObject.class); + + // Get the top and bottom of the execution as realized in the diagram editor, + // which may differ slightly from the expectation in a platform-dependent way + this.execTop = getTop(exec); + this.execBottom = getBottom(exec); } @SuppressWarnings("unchecked") @@ -240,11 +254,25 @@ Matcher withModifiers(Matcher matcher) { return (Matcher) modifiersMatcherFunction.apply(matcher); } - static int withinMagnet(boolean execStart) { - return withinMagnet(execStart ? EXEC_START_Y : EXEC_FINISH_Y, execStart); + int withinMagnet(boolean execStart) { + return withinMagnet(execStart ? execTop : execBottom, execStart); } - static int withinMagnet(int y, boolean execStart) { + int withinMagnet(int y, boolean execStart) { return execStart ? y - 9 : y + 9; } + + static int getTop(EditPart editPart) { + IFigure figure = ((GraphicalEditPart) editPart).getFigure(); + Rectangle bounds = figure.getBounds().getCopy(); + figure.getParent().translateToAbsolute(bounds); + return bounds.y(); + } + + static int getBottom(EditPart editPart) { + IFigure figure = ((GraphicalEditPart) editPart).getFigure(); + Rectangle bounds = figure.getBounds().getCopy(); + figure.getParent().translateToAbsolute(bounds); + return bounds.bottom(); + } } From e35e2f582c1f28f47b87af902bb4dda7e0acfa39 Mon Sep 17 00:00:00 2001 From: "Christian W. Damus" Date: Fri, 7 Sep 2018 09:31:56 -0400 Subject: [PATCH 04/11] Issue #73: Empirical message grabbing Get the actual location at which to grab the message to move it in message moving tests, to account for platform variability. --- .../policies/tests/MessageSnappingUITest.java | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 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/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 aae110f4..7327151e 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 @@ -13,6 +13,7 @@ package org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies.tests; 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.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; @@ -26,9 +27,12 @@ import java.util.Arrays; import java.util.function.Function; +import org.eclipse.draw2d.Connection; import org.eclipse.draw2d.IFigure; +import org.eclipse.draw2d.geometry.Point; import org.eclipse.draw2d.geometry.Rectangle; import org.eclipse.emf.ecore.EObject; +import org.eclipse.gef.ConnectionEditPart; import org.eclipse.gef.EditPart; import org.eclipse.gef.GraphicalEditPart; import org.eclipse.gmf.runtime.diagram.ui.commands.SetBoundsCommand; @@ -145,12 +149,15 @@ public void createReplyMessage() { public void moveSyncCallMessage() { EditPart messageEP = createConnection(SequenceElementTypes.Sync_Message_Edge, at(LL1_BODY_X, 120), at(LL2_BODY_X, 120)); - assumeThat(messageEP, runs(LL1_BODY_X, 120, LL2_BODY_X, 120)); + Point midMessage = getMessageGrabPoint(messageEP); // The receiving end snaps to the exec start and the sending end matches - final int midMessage = (LL1_BODY_X + LL2_BODY_X) / 2; editor.with(modifiers, - () -> editor.moveSelection(at(midMessage, 120), at(midMessage, withinMagnet(EXEC_START)))); + () -> 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))); + assertThat(messageEP, withModifiers(runs(LL1_BODY_X, execTop, LL2_BODY_X, execTop, 1))); } @@ -159,12 +166,11 @@ public void moveSyncCallMessage() { public void moveReplyMessage() { EditPart messageEP = createConnection(SequenceElementTypes.Reply_Message_Edge, at(LL2_BODY_X, 240), at(LL1_BODY_X, 240)); - assumeThat(messageEP, runs(LL2_BODY_X, 240, LL1_BODY_X, 240)); + Point midMessage = getMessageGrabPoint(messageEP); // The sending end snaps to the exec start and the receiving end matches - final int midMessage = (LL1_BODY_X + LL2_BODY_X) / 2; editor.with(modifiers, - () -> editor.moveSelection(at(midMessage, 240), at(midMessage, withinMagnet(EXEC_FINISH)))); + () -> editor.moveSelection(midMessage, at(midMessage.x(), withinMagnet(EXEC_FINISH)))); assertThat(messageEP, withModifiers(runs(LL2_BODY_X, execBottom, LL1_BODY_X, execBottom, 1))); } @@ -275,4 +281,9 @@ static int getBottom(EditPart editPart) { figure.getParent().translateToAbsolute(bounds); return bounds.bottom(); } + + static Point getMessageGrabPoint(EditPart editPart) { + Connection connection = (Connection) ((ConnectionEditPart) editPart).getFigure(); + return connection.getPoints().getMidpoint(); + } } From f39f2eb960dd88dac39ffe33125ce7b0c91cc3b0 Mon Sep 17 00:00:00 2001 From: "Christian W. Damus" Date: Wed, 5 Sep 2018 14:45:38 -0400 Subject: [PATCH 05/11] Fix issue #343 Restrict message end movement Implement the requirement to not allow movement of a message end to result in the other end being moved also to maintain semantic integrity. Thus, only the end grabbed by the user can be moved and then only if the resulting message (the other end always staying put) would be valid. Signed-off-by: Christian W. Damus # Conflicts: # plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/AbstractSequenceGraphicalNodeEditPolicy.java --- ...stractSequenceGraphicalNodeEditPolicy.java | 92 ++++++++++++++++--- ...LifelineHeaderGraphicalNodeEditPolicy.java | 12 ++- .../edit/policies/MessageFeedbackHelper.java | 67 +++++++------- .../internal/util/WeakEventBusDelegator.java | 62 +++++++++++++ ...lineBodyGraphicalNodeEditPolicyUITest.java | 5 +- .../tests/MessageReconnectionUITest.java | 17 ++-- 6 files changed, 195 insertions(+), 60 deletions(-) create mode 100644 plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/util/WeakEventBusDelegator.java 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 afa72a7a..83880717 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 @@ -17,6 +17,9 @@ import static org.eclipse.papyrus.uml.interaction.model.util.LogicalModelPredicates.above; import static org.eclipse.papyrus.uml.interaction.model.util.LogicalModelPredicates.below; +import com.google.common.eventbus.EventBus; +import com.google.common.eventbus.Subscribe; + import java.util.Iterator; import java.util.Optional; @@ -41,6 +44,7 @@ import org.eclipse.papyrus.uml.diagram.sequence.figure.magnets.IMagnet; 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.util.WeakEventBusDelegator; import org.eclipse.papyrus.uml.diagram.sequence.runtime.util.CreateRequestSwitch; import org.eclipse.papyrus.uml.diagram.sequence.runtime.util.MessageUtil; import org.eclipse.papyrus.uml.interaction.model.CreationCommand; @@ -63,6 +67,8 @@ */ public abstract class AbstractSequenceGraphicalNodeEditPolicy extends GraphicalNodeEditPolicy implements ISequenceEditPolicy { + private final EventBus bus = new EventBus(); + /** * Initializes me. */ @@ -84,8 +90,8 @@ public Command caseCreateConnectionViewRequest(CreateConnectionViewRequest reque Point location = getRelativeLocation(mouse); AnchorDescriptor anchorDesc = computeAnchoring(location); - Command result = new StartMessageCommand(lifeline, mouse, anchorDesc.elementBefore, - anchorDesc.offset, getSort(messageType)); + StartMessageCommand result = new StartMessageCommand(bus, lifeline, mouse, + anchorDesc.elementBefore, anchorDesc.offset, getSort(messageType)); request.setStartCommand(result); return result; } @@ -252,8 +258,12 @@ protected FeedbackHelper getFeedbackHelper(CreateConnectionRequest request) { } if (feedbackHelper == null) { - feedbackHelper = new MessageFeedbackHelper(Mode.CREATE, + MessageFeedbackHelper helper = new MessageFeedbackHelper(Mode.CREATE, MessageUtil.isSynchronousMessageConnection(request), getMagnetManager()); + helper.setEventBus(bus); + + feedbackHelper = helper; + Point p = request.getLocation(); connectionFeedback = createDummyConnection(request); connectionFeedback.setConnectionRouter(getDummyConnectionRouter(request)); @@ -266,6 +276,11 @@ protected FeedbackHelper getFeedbackHelper(CreateConnectionRequest request) { return feedbackHelper; } + @Override + protected void showCreationFeedback(CreateConnectionRequest request) { + super.showCreationFeedback(request); + } + @Override protected Command getReconnectSourceCommand(ReconnectRequest request) { Command result = super.getReconnectSourceCommand(request); @@ -370,32 +385,67 @@ Optional getEnd(ConnectionEditPart connection, boolean source) { // Nested types // - static class StartMessageCommand extends Command { - final MLifeline sender; + /** + * A pseudo-command tracking the details of the start location and anchoring of a message to be created. + */ + class StartMessageCommand extends Command { + private final MLifeline sender; - final Point location; + private final MessageSort sort; - final Optional> before; + private Point location; - final int offset; + private Optional> before; - final MessageSort sort; + private int offset; /** * Initializes me. */ - StartMessageCommand(MLifeline sender, Point absoluteMouse, Optional> before, int offset, - MessageSort sort) { + StartMessageCommand(EventBus bus, MLifeline sender, Point absoluteMouse, Optional> before, + int offset, MessageSort sort) { this.sender = sender; this.location = absoluteMouse; this.before = before; this.offset = offset; this.sort = sort; + + bus.register(new LocationUpdateHandler(bus, this)); + } + + MLifeline sender() { + return sender; + } + + MessageSort sort() { + return sort; + } + + Optional> before() { + return before; + } + + int offset() { + return offset; } + void updateLocation(Point absoluteLocation) { + if (!absoluteLocation.equals(this.location)) { + this.location = absoluteLocation; + Point relativeLocation = getRelativeLocation(absoluteLocation); + + AnchorDescriptor anchor = computeAnchoring(relativeLocation); + before = anchor.elementBefore; + offset = anchor.offset; + } + } } + /** + * A description of the anchoring, in Logical Model terms, of an end of a message to a lifeline + * or an execution specification on a lifeline. + */ private static class AnchorDescriptor { final Optional> elementBefore; @@ -407,6 +457,26 @@ private static class AnchorDescriptor { this.elementBefore = elementBefore; this.offset = offset; } + } + + /** + * Event handler for location updates posted on the event bus, which forwards them to a + * {@link StartMessageCommand} as long as that command is still viable. This weak reference indirection is + * required because the lifecycle of the {@link StartMessageCommand} is uncontrolled; there is no point in + * GEF at which it is or can be disposed. + */ + private static final class LocationUpdateHandler extends WeakEventBusDelegator { + LocationUpdateHandler(EventBus bus, StartMessageCommand delegate) { + super(bus, delegate); + } + + @Subscribe + public void updateLocation(Point location) { + StartMessageCommand delegate = get(); + if (delegate != null) { + delegate.updateLocation(location); + } + } } } diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/LifelineHeaderGraphicalNodeEditPolicy.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/LifelineHeaderGraphicalNodeEditPolicy.java index 4080923a..71f028e4 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/LifelineHeaderGraphicalNodeEditPolicy.java +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/LifelineHeaderGraphicalNodeEditPolicy.java @@ -33,18 +33,20 @@ public class LifelineHeaderGraphicalNodeEditPolicy extends GraphicalNodeEditPoli protected Command getConnectionCompleteCommand(CreateConnectionRequest request) { return new CreateRequestSwitch() { @Override - public Command caseCreateConnectionViewRequest(CreateConnectionViewRequest request) { + public Command caseCreateConnectionViewRequest( + @SuppressWarnings("hiding") CreateConnectionViewRequest request) { + StartMessageCommand start = (StartMessageCommand)request.getStartCommand(); org.eclipse.emf.common.command.Command result; - MLifeline sender = start.sender; + MLifeline sender = start.sender(); MInteraction interaction = sender.getInteraction(); MLifeline receiver = interaction.getLifeline(getHost().getAdapter(Lifeline.class)).get(); - switch (start.sort) { + switch (start.sort()) { case CREATE_MESSAGE_LITERAL: - result = sender.insertMessageAfter(start.before.orElse(sender), start.offset, - receiver, start.sort, null); + result = sender.insertMessageAfter(start.before().orElse(sender), start.offset(), + receiver, start.sort(), null); break; default: result = UnexecutableCommand.INSTANCE; diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/MessageFeedbackHelper.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/MessageFeedbackHelper.java index 47699215..3d6f9767 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/MessageFeedbackHelper.java +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/MessageFeedbackHelper.java @@ -12,6 +12,8 @@ package org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies; +import com.google.common.eventbus.EventBus; + import java.util.Optional; import org.eclipse.draw2d.Connection; @@ -35,6 +37,8 @@ class MessageFeedbackHelper extends FeedbackHelper { private final IMagnetManager magnetManager; + private EventBus bus; + // In the case of moving the message, where it is grabbed private Point grabbedAt; @@ -107,12 +111,13 @@ public void update(ConnectionAnchor _anchor, Point p) { Optional magnet = magnetManager.getCapturingMagnet(p); magnet.map(IMagnet::getLocation).ifPresent(p::setLocation); - if (synchronous) { + // Don't permit the message to go back in time if it's asynchronous + int delta = mode.isMovingTarget() ? p.y() - otherLocation.y() : otherLocation.y() - p.y(); + + if (synchronous || (delta < 0)) { // Constrain the message to horizontal switch (mode) { case CREATE: - case MOVE_SOURCE: - case MOVE_TARGET: // Bring the other end along (subject to magnet constraints) otherLocation.setY(p.y()); @@ -128,44 +133,24 @@ public void update(ConnectionAnchor _anchor, Point p) { recreateOtherAnchor(other, otherLocation); updateOtherAnchor(other); break; + case MOVE_SOURCE: + case MOVE_TARGET: + // Force it horizontal + p.setY(otherLocation.y()); + recreateAnchor(anchor, p); + break; default: // MOVE_BOTH is handled separately break; } + } + } + + if (bus != null) { + if (isMovingStartAnchor()) { + bus.post(thisLocation); } else { - // Don't permit the message to go back in time - int delta = mode.isMovingTarget() ? p.y() - otherLocation.y() : otherLocation.y() - p.y(); - - if (delta < 0) { - switch (mode) { - case CREATE: - // Force it horizontal - p.setY(otherLocation.y()); - recreateAnchor(anchor, p); - break; - case MOVE_SOURCE: - case MOVE_TARGET: - // Bring the other end along (subject to magnet constraints) - otherLocation.setY(p.y()); - - Optional otherMagnet = magnetManager - .getCapturingMagnet(otherLocation); - otherMagnet.map(IMagnet::getLocation).ifPresent(m -> { - // Don't move this end if the other is stuck to a magnet - int dy = otherLocation.y() - m.y(); - otherLocation.setLocation(m); - p.translate(0, -dy); - recreateAnchor(anchor, p); - }); - - recreateOtherAnchor(other, otherLocation); - updateOtherAnchor(other); - break; - default: - // MOVE_BOTH is handled separately - break; - } - } + bus.post(otherLocation); } } } @@ -205,6 +190,16 @@ void updateOtherAnchor(ConnectionAnchor[] other) { } } + /** + * Assign a bus to which I post source location updates. + * + * @param bus + * the event bus + */ + void setEventBus(EventBus bus) { + this.bus = bus; + } + // // Nested types // diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/util/WeakEventBusDelegator.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/util/WeakEventBusDelegator.java new file mode 100644 index 00000000..d31152d8 --- /dev/null +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/util/WeakEventBusDelegator.java @@ -0,0 +1,62 @@ +/***************************************************************************** + * Copyright (c) 2018 Christian W. Damus and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Christian W. Damus - Initial API and implementation + *****************************************************************************/ + +package org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.util; + +import com.google.common.eventbus.EventBus; + +import java.lang.ref.WeakReference; +import java.util.function.Supplier; + +/** + * An {@link EventBus} subscriber that maintains a weak reference to the real object that handles events (this + * delegator forwarding events to it). This enables registration of objects whose lifecycle is not tracked, + * for which there is no "disposal" or other termination protocol at which the object can then be unregistered + * from the event bus. + */ +public abstract class WeakEventBusDelegator implements Supplier { + private final EventBus bus; + + private final WeakReference delegate; + + /** + * Initializes me. + * + * @param bus + * the event bus to which the client will register me + * @param delegate + * the delegate that I track, weakly, to which I will forward events from the bus + */ + public WeakEventBusDelegator(EventBus bus, T delegate) { + super(); + + this.bus = bus; + this.delegate = new WeakReference(delegate); + } + + /** + * Obtain the delegate to which to forward an event from the bus. If the result is {@code null}, then I + * will already have unregistered myself because my delegate is no longer viable. + */ + @Override + public final T get() { + T result = delegate.get(); + + if (result == null) { + // The reference has been cleared, so unregister + bus.unregister(this); + } + + return result; + } + +} 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 fcac6dfb..1f815176 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 @@ -89,7 +89,10 @@ public void attemptBackwardSlopedAsyncMessage() { EditPart messageEP = createConnection(SequenceElementTypes.Async_Message_Edge, at(sendX, 130), at(recvX, 115)); - assertThat("Message should be horizontal", messageEP, runs(sendX, 130, recvX, 130, 2)); + // The target to which the user draw the message should have priority over the + // source + assertThat("Message should be horizontal to the target", messageEP, + runs(sendX, 115, recvX, 115, 2)); } @Test 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 53860b44..3e4c4779 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 @@ -78,8 +78,8 @@ public MessageReconnectionUITest(boolean rightToLeft, String direction, boolean @Test public void moveAsyncEnd() { - EditPart messageEP = createConnection(SequenceElementTypes.Async_Message_Edge, at(sendX, INITIAL_Y), - at(recvX, INITIAL_Y)); + EditPart messageEP = createConnection(SequenceElementTypes.Async_Message_Edge, // + at(sendX, INITIAL_Y), at(recvX, INITIAL_Y)); assumeThat(messageEP, runs(sendX, INITIAL_Y, recvX, INITIAL_Y)); @@ -92,8 +92,9 @@ public void moveAsyncEnd() { if (moveSource) { if (moveDown) { - expectedSendY = y; - expectedRecvY = y; + // Cannot slope up, so this end doesn't move + expectedSendY = INITIAL_Y; + expectedRecvY = INITIAL_Y; } else { expectedSendY = y; expectedRecvY = INITIAL_Y; @@ -103,8 +104,9 @@ public void moveAsyncEnd() { expectedSendY = INITIAL_Y; expectedRecvY = y; } else { - expectedSendY = y; - expectedRecvY = y; + // Cannot slope up, so this end doesn't move + expectedSendY = INITIAL_Y; + expectedRecvY = INITIAL_Y; } } @@ -126,7 +128,8 @@ public void moveSyncEnd() { editor.moveSelection(at(x, INITIAL_Y), at(x, y)); - assertThat(messageEP, runs(sendX, y, recvX, y)); + assertThat("Was able to move sync message end", messageEP, + runs(sendX, INITIAL_Y, recvX, INITIAL_Y)); } // From 0fbbe3b599b667926da7ee54782f9233eeb1c029 Mon Sep 17 00:00:00 2001 From: "Christian W. Damus" Date: Mon, 10 Sep 2018 08:48:35 -0400 Subject: [PATCH 06/11] Issue #343 No connection to LL heads Do not permit connection to the lifeline head. Signed-off-by: Christian W. Damus --- ...LifelineHeaderGraphicalNodeEditPolicy.java | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/LifelineHeaderGraphicalNodeEditPolicy.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/LifelineHeaderGraphicalNodeEditPolicy.java index 71f028e4..0e9b1b9e 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/LifelineHeaderGraphicalNodeEditPolicy.java +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.runtime/src/org/eclipse/papyrus/uml/diagram/sequence/runtime/internal/edit/policies/LifelineHeaderGraphicalNodeEditPolicy.java @@ -12,15 +12,22 @@ package org.eclipse.papyrus.uml.diagram.sequence.runtime.internal.edit.policies; import org.eclipse.emf.common.command.UnexecutableCommand; +import org.eclipse.emf.ecore.EObject; import org.eclipse.gef.commands.Command; import org.eclipse.gef.requests.CreateConnectionRequest; +import org.eclipse.gef.requests.ReconnectRequest; +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.papyrus.uml.diagram.sequence.runtime.internal.edit.policies.AbstractSequenceGraphicalNodeEditPolicy.StartMessageCommand; import org.eclipse.papyrus.uml.diagram.sequence.runtime.util.CreateRequestSwitch; import org.eclipse.papyrus.uml.interaction.model.MInteraction; import org.eclipse.papyrus.uml.interaction.model.MLifeline; +import org.eclipse.uml2.uml.GeneralOrdering; import org.eclipse.uml2.uml.Lifeline; +import org.eclipse.uml2.uml.Message; +import org.eclipse.uml2.uml.MessageSort; +import org.eclipse.uml2.uml.util.UMLSwitch; /** * The {@code LifelineHeaderGraphicalNodeEditPolicy} type. @@ -58,4 +65,73 @@ public Command caseCreateConnectionViewRequest( }.doSwitch(request); } + /** + * Lifeline heads can never be the source of a message or general ordering. + */ + @Override + protected Command getReconnectSourceCommand(ReconnectRequest request) { + EObject semanticElement = null; + + if (request.getConnectionEditPart() instanceof IGraphicalEditPart) { + semanticElement = ((IGraphicalEditPart)request.getConnectionEditPart()).resolveSemanticElement(); + } + + if (semanticElement == null) { + return org.eclipse.gef.commands.UnexecutableCommand.INSTANCE; + } + + return new UMLSwitch() { + @Override + public Command caseMessage(Message object) { + return org.eclipse.gef.commands.UnexecutableCommand.INSTANCE; + } + + @Override + public Command caseGeneralOrdering(GeneralOrdering object) { + return org.eclipse.gef.commands.UnexecutableCommand.INSTANCE; + } + + @Override + public Command defaultCase(EObject object) { + return LifelineHeaderGraphicalNodeEditPolicy.super.getReconnectSourceCommand(request); + } + }.doSwitch(semanticElement); + } + + /** + * Lifeline heads can never be the target of a general ordering or a message that isn't a create message. + */ + @Override + protected Command getReconnectTargetCommand(ReconnectRequest request) { + EObject semanticElement = null; + + if (request.getConnectionEditPart() instanceof IGraphicalEditPart) { + semanticElement = ((IGraphicalEditPart)request.getConnectionEditPart()).resolveSemanticElement(); + } + + if (semanticElement == null) { + return org.eclipse.gef.commands.UnexecutableCommand.INSTANCE; + } + + return new UMLSwitch() { + @Override + public Command caseMessage(Message object) { + if (object.getMessageSort() != MessageSort.CREATE_MESSAGE_LITERAL) { + return org.eclipse.gef.commands.UnexecutableCommand.INSTANCE; + } + + return null; // Up to the default case + } + + @Override + public Command caseGeneralOrdering(GeneralOrdering object) { + return org.eclipse.gef.commands.UnexecutableCommand.INSTANCE; + } + + @Override + public Command defaultCase(EObject object) { + return LifelineHeaderGraphicalNodeEditPolicy.super.getReconnectTargetCommand(request); + } + }.doSwitch(semanticElement); + } } From 5bc6937ef99365a248e9a14fb3c0a07f32081e9c Mon Sep 17 00:00:00 2001 From: "Christian W. Damus" Date: Mon, 10 Sep 2018 10:45:07 -0400 Subject: [PATCH 07/11] Issue #343 Msg end ordering for executions When drawing a message replaces an execution occurrence specification that starts or finishes an execution specification, the ordering of the message ends would end up reversed in the interaction fragments (the message being received before it is sent) and also (in the case of starting the execution specification) after the execution specification, itself. Signed-off-by: Christian W. Damus --- .../model/commands/InsertMessageCommand.java | 21 +++---------------- .../internal/model/commands/ModelCommand.java | 8 +++---- 2 files changed, 7 insertions(+), 22 deletions(-) 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 98b478c1..23154df2 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 @@ -29,13 +29,8 @@ import org.eclipse.emf.common.command.Command; import org.eclipse.emf.common.command.UnexecutableCommand; -import org.eclipse.emf.common.util.EList; import org.eclipse.emf.ecore.EObject; -import org.eclipse.emf.ecore.EReference; -import org.eclipse.emf.edit.command.AddCommand; -import org.eclipse.emf.edit.command.CommandParameter; import org.eclipse.emf.edit.command.DeleteCommand; -import org.eclipse.emf.edit.command.MoveCommand; import org.eclipse.emf.edit.command.SetCommand; import org.eclipse.gmf.runtime.notation.Shape; import org.eclipse.gmf.runtime.notation.View; @@ -470,21 +465,11 @@ private Optional replace(OccurrenceSpecification occurrence, MessageEnd return execSpec.map(exec -> { boolean isStart = exec.getStart() == occurrence; - EObject owner = occurrence.eContainer(); - EReference containment = occurrence.eContainmentFeature(); - int index = containment.isMany() ? ((EList)owner.eGet(containment)).indexOf(occurrence) - : CommandParameter.NO_INDEX; - + // The message end was inserted already in the right index of the + // fragments list, relative to the execution specification. So, + // just delete the execution occurrence that is being replaced Command result = DeleteCommand.create(getEditingDomain(), occurrence); - if (msgEnd.eContainer() == owner) { - result = result - .chain(MoveCommand.create(getEditingDomain(), owner, containment, msgEnd, index)); - } else { - result = result - .chain(AddCommand.create(getEditingDomain(), owner, containment, msgEnd, index)); - } - if (isStart) { result = result.chain(SetCommand.create(getEditingDomain(), exec, UMLPackage.Literals.EXECUTION_SPECIFICATION__START, msgEnd)); diff --git a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/ModelCommand.java b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/ModelCommand.java index ce85870c..471236ad 100644 --- a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/ModelCommand.java +++ b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/ModelCommand.java @@ -183,10 +183,10 @@ protected static Optional> getInsertionPoint( int index = Collections.binarySearch(timeline, searchToken, compareByTop()); if (index >= 0) { - // Easy case: follow the fragment currently at that Y position, which means insert - // before the first element following that is at a greater position - while ((index < size) && (timeline.get(index).getTop().orElse(-1) == yPosition)) { - index++; + // Easy case: precede the fragments currently at that Y position, which means insert + // before the first element following that is at this position + while ((index > 0) && (timeline.get(index - 1).getTop().orElse(-1) == yPosition)) { + index--; } } else { // The search found an "insertion point", which is where the fragment is before which From eaa5ad7007876326977eba7bef88ca8f670a3332 Mon Sep 17 00:00:00 2001 From: "Christian W. Damus" Date: Mon, 10 Sep 2018 17:43:44 -0400 Subject: [PATCH 08/11] Issue #343 Message anchoring on move Fix some mistakes in the anchoring of message ends to lifelines and execution specifications when moving messages such that a message end moves from an execution specification to a lifeline or vice-versa. Signed-off-by: Christian W. Damus --- .../figure/ExecutionSpecificationFigure.java | 9 +++++ ...stractSequenceGraphicalNodeEditPolicy.java | 16 +++++++-- .../policies/MessageEndpointEditPolicy.java | 36 +++++++++++++++++++ 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/ExecutionSpecificationFigure.java b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/ExecutionSpecificationFigure.java index 926f0f20..8da990be 100644 --- a/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/ExecutionSpecificationFigure.java +++ b/plugins/org.eclipse.papyrus.uml.diagram.sequence.figure/src/org/eclipse/papyrus/uml/diagram/sequence/figure/ExecutionSpecificationFigure.java @@ -25,6 +25,7 @@ import org.eclipse.papyrus.uml.diagram.sequence.figure.anchors.ExecutionSpecificationEndAnchor; import org.eclipse.papyrus.uml.diagram.sequence.figure.anchors.ExecutionSpecificationSideAnchor; import org.eclipse.papyrus.uml.diagram.sequence.figure.anchors.ExecutionSpecificationStartAnchor; +import org.eclipse.papyrus.uml.diagram.sequence.figure.anchors.ISequenceAnchor; public class ExecutionSpecificationFigure extends NodeFigure { @Override @@ -115,4 +116,12 @@ protected ConnectionAnchor createAnchor(PrecisionPoint p) { return super.createAnchor(p); } + + @Override + public String getConnectionAnchorTerminal(ConnectionAnchor c) { + if (c instanceof ISequenceAnchor) { + return ((ISequenceAnchor)c).getTerminal(); + } + return super.getConnectionAnchorTerminal(c); + } } 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 83880717..1e3925a9 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 @@ -281,12 +281,18 @@ protected void showCreationFeedback(CreateConnectionRequest request) { super.showCreationFeedback(request); } + @SuppressWarnings("boxing") @Override protected Command getReconnectSourceCommand(ReconnectRequest request) { Command result = super.getReconnectSourceCommand(request); ConnectionEditPart connectionEP = (ConnectionEditPart)request.getConnectionEditPart(); + Optional message = Optional.of(connectionEP).map(ConnectionEditPart::resolveSemanticElement) + .filter(Message.class::isInstance).map(Message.class::cast); + + // Of course, we don't mess with the other end of an asynchronous message + if (!isForce(request) + && message.map(Message::getMessageSort).map(MessageUtil::isSynchronous).orElse(false)) { - if (!isForce(request)) { // Need feedback constraints in addition to semantic constraints Connection connection = (Connection)connectionEP.getFigure(); Point targetLocation = getLocation(connection.getTargetAnchor()); @@ -323,12 +329,18 @@ protected Optional constrainReconnection(ReconnectRequest request, MMes return result; } + @SuppressWarnings("boxing") @Override protected Command getReconnectTargetCommand(ReconnectRequest request) { Command result = super.getReconnectTargetCommand(request); ConnectionEditPart connectionEP = (ConnectionEditPart)request.getConnectionEditPart(); + Optional message = Optional.of(connectionEP).map(ConnectionEditPart::resolveSemanticElement) + .filter(Message.class::isInstance).map(Message.class::cast); + + // Of course, we don't mess with the other end of an asynchronous message + if (!isForce(request) + && message.map(Message::getMessageSort).map(MessageUtil::isSynchronous).orElse(false)) { - if (!isForce(request)) { // Need feedback constraints in addition to semantic constraints Connection connection = (Connection)connectionEP.getFigure(); Point sourceLocation = getLocation(connection.getSourceAnchor()); 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 25a5e2cd..90473674 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 @@ -22,6 +22,7 @@ import com.google.common.eventbus.EventBus; +import java.util.Collections; import java.util.Optional; import org.eclipse.draw2d.ConnectionAnchor; @@ -243,6 +244,10 @@ protected Command getMoveConnectionCommand(BendpointRequest request) { sourceReq.setConnectionEditPart(connection); sourceReq.setLocation(sourceLocation); setForce(sourceReq, true); + + // In case the result of the drag moves the source end to a different edit-part + // (for example, a different execution specification) + source = retargetRequest(source, sourceReq); Command updateSource = source.getCommand(sourceReq); ReconnectRequest targetReq = new ReconnectRequest(REQ_RECONNECT_TARGET); @@ -250,6 +255,9 @@ protected Command getMoveConnectionCommand(BendpointRequest request) { targetReq.setConnectionEditPart(connection); targetReq.setLocation(targetLocation); setForce(targetReq, true); + + // In case the result of the drag moves the target end to a different edit-part + target = retargetRequest(target, targetReq); Command updateTarget = target.getCommand(targetReq); // Never update just one end @@ -261,4 +269,32 @@ protected Command getMoveConnectionCommand(BendpointRequest request) { return result.unwrap(); } + + /** + * Recompute the target of a re-connection request in case the connection end is being dragged to a + * different edit-part than where it connects to currently. + * + * @param editPart + * the original connection source/target edit-part that is the intended target edit-part of the + * {@code request} + * @param request + * the request to be sent + * @return the best available edit-part to which to target the {@code request}, which if different to the + * original {@code editPart} will already be set as the {@code request}'s + * {@link ReconnectRequest#setTargetEditPart(EditPart) target} + */ + protected EditPart retargetRequest(EditPart editPart, ReconnectRequest request) { + EditPart result = editPart; + + EditPart resolvedTarget = editPart.getViewer().findObjectAtExcluding(request.getLocation(), + Collections.singleton(request.getConnectionEditPart()), + ep -> ep.getTargetEditPart(request) != null); + if (resolvedTarget != null && resolvedTarget != editPart) { + // Ask this one for the command + request.setTargetEditPart(resolvedTarget); + result = resolvedTarget; + } + + return result; + } } From 03af9e0f6661004dce718267e5bc4bc3f3ad2ce0 Mon Sep 17 00:00:00 2001 From: Philip Langer Date: Wed, 12 Sep 2018 10:22:28 +0200 Subject: [PATCH 09/11] Add tests for semantic ordering after creation on top Adds tests for issues #343 and #347. To enable more efficient adding of testing, this change also refactors the tests a bit and extracts common methods into dedicated model fixture. Fix wrong name of receive of message 2 Improve ModelEditFixture.isSemanticallyBefore() Signed-off-by: Philip Langer --- .../model/tests/ModelEditFixture.java | 88 ++++++++ .../model/tests/SeqDCustomTests.java | 2 + .../tests/creation/CreateExecutionTest.java | 12 +- .../tests/creation/CreateMessageTest.java | 45 +--- .../tests/creation/CreateMessageTestB.java | 45 +--- ...cOrderAfterCreationOfElementOnTopTest.java | 196 ++++++++++++++++++ .../SemanticOrderAfterCreationTest.notation | 54 +++++ .../SemanticOrderAfterCreationTest.uml | 17 ++ .../tests/deletion/BasicDeletionTest.java | 7 +- .../deletion/CreationMessageDeletionTest.java | 14 +- .../tests/deletion/DeleteExecutionTest.java | 12 +- .../deletion/DeletionMessageDeletionTest.java | 14 +- .../tests/LogicalModelPredicatesTest.java | 47 +---- 13 files changed, 411 insertions(+), 142 deletions(-) create mode 100644 tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/ModelEditFixture.java create mode 100644 tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/SemanticOrderAfterCreationOfElementOnTopTest.java create mode 100644 tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/SemanticOrderAfterCreationTest.notation create mode 100644 tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/SemanticOrderAfterCreationTest.uml diff --git a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/ModelEditFixture.java b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/ModelEditFixture.java new file mode 100644 index 00000000..cb39cd47 --- /dev/null +++ b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/ModelEditFixture.java @@ -0,0 +1,88 @@ +/***************************************************************************** + * 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: + * Philip Langer - Initial API and implementation + *****************************************************************************/ +package org.eclipse.papyrus.uml.interaction.model.tests; + +import java.util.List; +import java.util.Optional; + +import org.eclipse.gmf.runtime.notation.View; +import org.eclipse.papyrus.uml.interaction.internal.model.impl.LogicalModelPlugin; +import org.eclipse.papyrus.uml.interaction.model.MElement; +import org.eclipse.papyrus.uml.interaction.model.MExecution; +import org.eclipse.papyrus.uml.interaction.model.MInteraction; +import org.eclipse.papyrus.uml.interaction.model.MLifeline; +import org.eclipse.papyrus.uml.interaction.model.MMessage; +import org.eclipse.papyrus.uml.interaction.model.MOccurrence; +import org.eclipse.papyrus.uml.interaction.model.spi.DiagramHelper; +import org.eclipse.papyrus.uml.interaction.model.spi.LayoutHelper; +import org.eclipse.papyrus.uml.interaction.tests.rules.ModelFixture.Edit; +import org.eclipse.uml2.uml.Element; +import org.eclipse.uml2.uml.ExecutionSpecification; +import org.eclipse.uml2.uml.InteractionFragment; +import org.eclipse.uml2.uml.Message; + +@SuppressWarnings("restriction") +public class ModelEditFixture extends Edit { + + public DiagramHelper diagramHelper() { + return LogicalModelPlugin.getInstance().getDiagramHelper(this.getEditingDomain()); + } + + public LayoutHelper layoutHelper() { + return LogicalModelPlugin.getInstance().getLayoutHelper(this.getEditingDomain()); + } + + public int getLifelineBodyTop(MLifeline lifeline) { + View shape = lifeline.getDiagramView().orElse(null); + return layoutHelper().getTop(diagramHelper().getLifelineBodyShape(shape)); + } + + public MInteraction getMInteraction() { + return MInteraction.getInstance(getInteraction(), getSequenceDiagram().get()); + } + + public Optional getMMessage(Message message) { + return getMInteraction().getMessage(message); + } + + public Optional getMExecution(ExecutionSpecification execution) { + Optional> element = getMInteraction() + .getElement(execution); + if (element.isPresent() && element.get() instanceof MExecution) { + return Optional.of((MExecution)element.get()); + } + return Optional.empty(); + } + + public > T getElement(String qnFormat, String name, Class type) { + String qName = String.format(qnFormat, name); + + Element element = getElement(qName); + return Optional.ofNullable(element).flatMap(getMInteraction()::getElement).filter(type::isInstance) + .map(type::cast).orElseThrow(() -> new AssertionError("no such element: " + name)); //$NON-NLS-1$ + } + + @SuppressWarnings("unchecked") + public > T getElement(String qnFormat, String name) { + return (T)getElement(qnFormat, name, MElement.class); + } + + @SuppressWarnings({"boxing" }) + public Object isSemanticallyBefore(MOccurrence one, + MOccurrence other) { + List fragments = getMInteraction().getElement().getFragments(); + int indexOfOne = fragments.indexOf(one.getElement()); + int indexOfOther = fragments.indexOf(other.getElement()); + return indexOfOne >= 0 && indexOfOther >= 0 && indexOfOther > indexOfOne; + } + +} diff --git a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/SeqDCustomTests.java b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/SeqDCustomTests.java index 8cf2e1f4..c8b3b761 100644 --- a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/SeqDCustomTests.java +++ b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/SeqDCustomTests.java @@ -17,6 +17,7 @@ import org.eclipse.papyrus.uml.interaction.model.tests.creation.CreateExecutionTest; import org.eclipse.papyrus.uml.interaction.model.tests.creation.CreateMessageTest; import org.eclipse.papyrus.uml.interaction.model.tests.creation.CreateMessageTestB; +import org.eclipse.papyrus.uml.interaction.model.tests.creation.SemanticOrderAfterCreationOfElementOnTopTest; import org.eclipse.papyrus.uml.interaction.model.tests.deletion.BasicDeletionTest; import org.eclipse.papyrus.uml.interaction.model.tests.deletion.CreationMessageDeletionTest; import org.eclipse.papyrus.uml.interaction.model.tests.deletion.DeleteExecutionTest; @@ -37,6 +38,7 @@ CreateMessageTest.class, CreateMessageTestB.class, // CreateExecutionTest.class, DeleteExecutionTest.class, // CreationMessageDeletionTest.class, BasicDeletionTest.class, DeletionMessageDeletionTest.class, // + SemanticOrderAfterCreationOfElementOnTopTest.class, // LogicalModelAdapterTest.class, // LogicalModelPredicatesTest.class, // }) diff --git a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/CreateExecutionTest.java b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/CreateExecutionTest.java index ca4c070e..a123673e 100644 --- a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/CreateExecutionTest.java +++ b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/CreateExecutionTest.java @@ -19,10 +19,9 @@ import org.eclipse.papyrus.uml.interaction.model.CreationCommand; import org.eclipse.papyrus.uml.interaction.model.MInteraction; import org.eclipse.papyrus.uml.interaction.model.MLifeline; -import org.eclipse.papyrus.uml.interaction.tests.rules.ModelFixture; +import org.eclipse.papyrus.uml.interaction.model.tests.ModelEditFixture; import org.eclipse.papyrus.uml.interaction.tests.rules.ModelResource; import org.eclipse.uml2.uml.ExecutionSpecification; -import org.junit.Assert; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -34,7 +33,7 @@ public class CreateExecutionTest { private static final int EXECUTION_HEIGHT = 20; @Rule - public final ModelFixture.Edit model = new ModelFixture.Edit(); + public final ModelEditFixture model = new ModelEditFixture(); private MInteraction interaction; @@ -56,16 +55,13 @@ public void before() { private MInteraction interaction() { if (interaction == null) { - interaction = MInteraction.getInstance(model.getInteraction(), model.getSequenceDiagram().get()); + interaction = model.getMInteraction(); } return interaction; } private void execute(Command command) { - if (!command.canExecute()) { - Assert.fail("Command not executable"); //$NON-NLS-1$ - } - command.execute(); + model.execute(command); /* force reinit after change */ interaction = null; } diff --git a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/CreateMessageTest.java b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/CreateMessageTest.java index 50cbd31d..6ebf71d9 100644 --- a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/CreateMessageTest.java +++ b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/CreateMessageTest.java @@ -3,29 +3,23 @@ import static org.junit.Assert.assertEquals; import org.eclipse.emf.common.command.Command; -import org.eclipse.gmf.runtime.notation.View; -import org.eclipse.papyrus.uml.interaction.internal.model.impl.LogicalModelPlugin; import org.eclipse.papyrus.uml.interaction.model.CreationCommand; import org.eclipse.papyrus.uml.interaction.model.MExecution; import org.eclipse.papyrus.uml.interaction.model.MInteraction; import org.eclipse.papyrus.uml.interaction.model.MLifeline; -import org.eclipse.papyrus.uml.interaction.model.spi.DiagramHelper; -import org.eclipse.papyrus.uml.interaction.model.spi.LayoutHelper; -import org.eclipse.papyrus.uml.interaction.tests.rules.ModelFixture; +import org.eclipse.papyrus.uml.interaction.model.tests.ModelEditFixture; import org.eclipse.papyrus.uml.interaction.tests.rules.ModelResource; import org.eclipse.uml2.uml.Message; import org.eclipse.uml2.uml.MessageSort; -import org.junit.Assert; import org.junit.Before; import org.junit.Rule; import org.junit.Test; -@SuppressWarnings("restriction") @ModelResource({"CreateMessageTesting.uml", "CreateMessageTesting.notation" }) public class CreateMessageTest { @Rule - public final ModelFixture.Edit model = new ModelFixture.Edit(); + public final ModelEditFixture model = new ModelEditFixture(); private MInteraction interaction; @@ -57,8 +51,8 @@ public void before() { lifeline2Top = interaction().getLifelines().get(1).getTop().getAsInt(); lifeline3Top = interaction().getLifelines().get(2).getTop().getAsInt(); - lifeline2Header = getLifelineBodyTop(interaction().getLifelines().get(1)) - lifeline2Top; - lifeline3Header = getLifelineBodyTop(interaction().getLifelines().get(2)) - lifeline3Top; + lifeline2Header = model.getLifelineBodyTop(interaction().getLifelines().get(1)) - lifeline2Top; + lifeline3Header = model.getLifelineBodyTop(interaction().getLifelines().get(2)) - lifeline3Top; message1Top = interaction().getMessages().get(0).getTop().getAsInt(); message2Top = interaction().getMessages().get(1).getTop().getAsInt(); @@ -74,36 +68,19 @@ public void before() { .getAsInt(); } - private DiagramHelper diagramHelper() { - return LogicalModelPlugin.getInstance().getDiagramHelper(model.getEditingDomain()); - } - - private LayoutHelper layoutHelper() { - return LogicalModelPlugin.getInstance().getLayoutHelper(model.getEditingDomain()); - } - - private int getLifelineBodyTop(MLifeline lifeline) { - View shape = lifeline.getDiagramView().orElse(null); - return layoutHelper()// - .getTop(diagramHelper().getLifelineBodyShape(shape)); + private void execute(Command command) { + model.execute(command); + /* force reinit after change */ + interaction = null; } private MInteraction interaction() { if (interaction == null) { - interaction = MInteraction.getInstance(model.getInteraction(), model.getSequenceDiagram().get()); + interaction = model.getMInteraction(); } return interaction; } - private void execute(Command remove) { - if (!remove.canExecute()) { - Assert.fail("Command not executable"); //$NON-NLS-1$ - } - remove.execute(); - /* force reinit after change */ - interaction = null; - } - @Test public void creationMessage() { /* setup */ @@ -135,7 +112,7 @@ public void creationMessage() { assertEquals(lifeline3Top, interaction().getLifelines().get(2).getTop().getAsInt()); /* we drew the creation message to the middle of the side of the lifeline head */ - int nudgedLifeline2Top = getLifelineBodyTop(interaction().getLifelines().get(0)) + 10 // + int nudgedLifeline2Top = model.getLifelineBodyTop(interaction().getLifelines().get(0)) + 10 // - (lifeline2Header / 2); assertEquals(nudgedLifeline2Top, interaction().getLifelines().get(1).getTop().getAsInt()); @@ -177,7 +154,7 @@ public void nestedCreationMessage() { assertEquals(4, interaction().getMessages().size()); assertEquals(MessageSort.CREATE_MESSAGE_LITERAL, interaction().getMessages().get(3).getElement().getMessageSort()); - assertEquals(getLifelineBodyTop(interaction().getLifelines().get(1)) + 5, + assertEquals(model.getLifelineBodyTop(interaction().getLifelines().get(1)) + 5, interaction().getMessages().get(3).getTop().getAsInt()); assertEquals(lifeline1Top, interaction().getLifelines().get(0).getTop().getAsInt()); diff --git a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/CreateMessageTestB.java b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/CreateMessageTestB.java index c8626e77..e2872f07 100644 --- a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/CreateMessageTestB.java +++ b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/CreateMessageTestB.java @@ -3,28 +3,22 @@ import static org.junit.Assert.assertEquals; import org.eclipse.emf.common.command.Command; -import org.eclipse.gmf.runtime.notation.View; -import org.eclipse.papyrus.uml.interaction.internal.model.impl.LogicalModelPlugin; import org.eclipse.papyrus.uml.interaction.model.CreationCommand; import org.eclipse.papyrus.uml.interaction.model.MInteraction; import org.eclipse.papyrus.uml.interaction.model.MLifeline; -import org.eclipse.papyrus.uml.interaction.model.spi.DiagramHelper; -import org.eclipse.papyrus.uml.interaction.model.spi.LayoutHelper; -import org.eclipse.papyrus.uml.interaction.tests.rules.ModelFixture; +import org.eclipse.papyrus.uml.interaction.model.tests.ModelEditFixture; import org.eclipse.papyrus.uml.interaction.tests.rules.ModelResource; import org.eclipse.uml2.uml.Message; import org.eclipse.uml2.uml.MessageSort; -import org.junit.Assert; import org.junit.Before; import org.junit.Rule; import org.junit.Test; -@SuppressWarnings("restriction") @ModelResource({"CreateMessageTestingB.uml", "CreateMessageTestingB.notation" }) public class CreateMessageTestB { @Rule - public final ModelFixture.Edit model = new ModelFixture.Edit(); + public final ModelEditFixture model = new ModelEditFixture(); private MInteraction interaction; @@ -57,8 +51,8 @@ public void before() { lifeline3Top = interaction().getLifelines().get(2).getTop().getAsInt(); lifeline4Top = interaction().getLifelines().get(3).getTop().getAsInt(); - lifeline2Header = getLifelineBodyTop(interaction().getLifelines().get(1)) - lifeline2Top; - lifeline4Header = getLifelineBodyTop(interaction().getLifelines().get(3)) - lifeline4Top; + lifeline2Header = model.getLifelineBodyTop(interaction().getLifelines().get(1)) - lifeline2Top; + lifeline4Header = model.getLifelineBodyTop(interaction().getLifelines().get(3)) - lifeline4Top; message1Top = interaction().getMessages().get(0).getTop().getAsInt(); message2Top = interaction().getMessages().get(4).getTop().getAsInt(); @@ -67,36 +61,19 @@ public void before() { message5Top = interaction().getMessages().get(3).getTop().getAsInt(); } - private DiagramHelper diagramHelper() { - return LogicalModelPlugin.getInstance().getDiagramHelper(model.getEditingDomain()); - } - - private LayoutHelper layoutHelper() { - return LogicalModelPlugin.getInstance().getLayoutHelper(model.getEditingDomain()); - } - - private int getLifelineBodyTop(MLifeline lifeline) { - View shape = lifeline.getDiagramView().orElse(null); - return layoutHelper()// - .getTop(diagramHelper().getLifelineBodyShape(shape)); + private void execute(Command command) { + model.execute(command); + /* force reinit after change */ + interaction = null; } private MInteraction interaction() { if (interaction == null) { - interaction = MInteraction.getInstance(model.getInteraction(), model.getSequenceDiagram().get()); + interaction = model.getMInteraction(); } return interaction; } - private void execute(Command remove) { - if (!remove.canExecute()) { - Assert.fail("Command not executable"); //$NON-NLS-1$ - } - remove.execute(); - /* force reinit after change */ - interaction = null; - } - @Test public void creationMessage1() { /* setup */ @@ -115,7 +92,7 @@ public void creationMessage1() { assertEquals(6, interaction().getMessages().size()); assertEquals(MessageSort.CREATE_MESSAGE_LITERAL, interaction().getMessages().get(5).getElement().getMessageSort()); - assertEquals(getLifelineBodyTop(interaction().getLifelines().get(0)) + 10, + assertEquals(model.getLifelineBodyTop(interaction().getLifelines().get(0)) + 10, interaction().getMessages().get(5).getTop().getAsInt()); assertEquals(lifeline1Top, interaction().getLifelines().get(0).getTop().getAsInt()); @@ -156,7 +133,7 @@ public void creationMessage2() { assertEquals(6, interaction().getMessages().size()); assertEquals(MessageSort.CREATE_MESSAGE_LITERAL, interaction().getMessages().get(5).getElement().getMessageSort()); - assertEquals(getLifelineBodyTop(interaction().getLifelines().get(2)) + 10, + assertEquals(model.getLifelineBodyTop(interaction().getLifelines().get(2)) + 10, interaction().getMessages().get(5).getTop().getAsInt()); assertEquals(lifeline1Top, interaction().getLifelines().get(0).getTop().getAsInt()); diff --git a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/SemanticOrderAfterCreationOfElementOnTopTest.java b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/SemanticOrderAfterCreationOfElementOnTopTest.java new file mode 100644 index 00000000..0e7648ef --- /dev/null +++ b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/SemanticOrderAfterCreationOfElementOnTopTest.java @@ -0,0 +1,196 @@ +package org.eclipse.papyrus.uml.interaction.model.tests.creation; + +import static org.eclipse.uml2.uml.UMLPackage.Literals.ACTION_EXECUTION_SPECIFICATION; +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; + +import java.util.Arrays; +import java.util.List; + +import org.eclipse.emf.common.command.Command; +import org.eclipse.papyrus.uml.interaction.model.CreationCommand; +import org.eclipse.papyrus.uml.interaction.model.MExecution; +import org.eclipse.papyrus.uml.interaction.model.MInteraction; +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.tests.ModelEditFixture; +import org.eclipse.papyrus.uml.interaction.tests.rules.ModelResource; +import org.eclipse.uml2.uml.ExecutionSpecification; +import org.eclipse.uml2.uml.Message; +import org.eclipse.uml2.uml.MessageSort; +import org.junit.Rule; +import org.junit.Test; + +@SuppressWarnings("nls") +@ModelResource({"SemanticOrderAfterCreationTest.uml", "SemanticOrderAfterCreationTest.notation" }) +public class SemanticOrderAfterCreationOfElementOnTopTest { + + private static final String QN = "343-test::Interaction::%s"; + + @Rule + public final ModelEditFixture model = new ModelEditFixture(); + + private MInteraction interaction; + + private MInteraction interaction() { + if (interaction == null) { + interaction = model.getMInteraction(); + } + return interaction; + } + + private void execute(Command command) { + model.execute(command); + /* force reinit after change */ + interaction = null; + } + + @Test + @SuppressWarnings({"boxing" }) + public void addMessageOnTopFromLifeline2ToLifeline3() { + /* setup */ + MLifeline lifeline2 = interaction().getLifelines().get(1); + MLifeline lifeline3 = interaction().getLifelines().get(2); + CreationCommand command = lifeline2.insertMessageAfter(lifeline2, 10, lifeline3, + MessageSort.SYNCH_CALL_LITERAL, null); + + /* act */ + execute(command); + + /* assert: message ends of added message are sorted in before existing messages */ + MMessage createdMessage = model.getMMessage(command.get()).get(); + MMessageEnd createdSend = createdMessage.getSend().get(); + MMessageEnd createdReceive = createdMessage.getReceive().get(); + MMessageEnd msg1send = model.getElement(QN, "1s", MMessageEnd.class); + MMessageEnd msg1receive = model.getElement(QN, "1r", MMessageEnd.class); + MMessageEnd msg2send = model.getElement(QN, "2s", MMessageEnd.class); + MMessageEnd msg2receive = model.getElement(QN, "2r", MMessageEnd.class); + + List otherEnds = Arrays.asList(msg1send, msg1receive, msg2send, msg2receive); + otherEnds.forEach(other -> assertThat(model.isSemanticallyBefore(createdSend, other), is(true))); + otherEnds.forEach(other -> assertThat(model.isSemanticallyBefore(createdReceive, other), is(true))); + } + + @Test + @SuppressWarnings({"boxing" }) + public void addMessageAfterFirstExistingMessageFromLifeline2ToLifeline3() { + /* setup */ + MLifeline lifeline2 = interaction().getLifelines().get(1); + MLifeline lifeline3 = interaction().getLifelines().get(2); + CreationCommand command = lifeline2.insertMessageAfter(interaction.getMessages().get(0), 10, + lifeline3, MessageSort.SYNCH_CALL_LITERAL, null); + + /* act */ + execute(command); + + /* assert: message ends of added message are sorted in before existing messages */ + MMessage createdMessage = model.getMMessage(command.get()).get(); + MMessageEnd createdSend = createdMessage.getSend().get(); + MMessageEnd createdReceive = createdMessage.getReceive().get(); + MMessageEnd msg1send = model.getElement(QN, "1s", MMessageEnd.class); + MMessageEnd msg1receive = model.getElement(QN, "1r", MMessageEnd.class); + MMessageEnd msg2send = model.getElement(QN, "2s", MMessageEnd.class); + MMessageEnd msg2receive = model.getElement(QN, "2r", MMessageEnd.class); + + List endsBefore = Arrays.asList(msg1send, msg1receive); + endsBefore.forEach(end -> assertThat(model.isSemanticallyBefore(end, createdSend), is(true))); + endsBefore.forEach(end -> assertThat(model.isSemanticallyBefore(end, createdReceive), is(true))); + + List endsAfter = Arrays.asList(msg2send, msg2receive); + endsAfter.forEach(end -> assertThat(model.isSemanticallyBefore(createdSend, end), is(true))); + endsAfter.forEach(end -> assertThat(model.isSemanticallyBefore(createdReceive, end), is(true))); + } + + @Test + public void addExecutionOnTopOfLifeline1() { + MLifeline lifeline1 = interaction().getLifelines().get(0); + addAndAssertExecutionOnTopOfLifeline(lifeline1); + } + + @Test + public void addExecutionOnTopOfLifeline2() { + MLifeline lifeline2 = interaction().getLifelines().get(1); + addAndAssertExecutionOnTopOfLifeline(lifeline2); + } + + @Test + public void addExecutionOnTopOfLifeline3() { + MLifeline lifeline3 = interaction().getLifelines().get(2); + addAndAssertExecutionOnTopOfLifeline(lifeline3); + } + + @SuppressWarnings("boxing") + private void addAndAssertExecutionOnTopOfLifeline(MLifeline lifeline) { + /* setup */ + CreationCommand command = lifeline.insertExecutionAfter(lifeline, 0, 20, + ACTION_EXECUTION_SPECIFICATION); + + /* act */ + execute(command); + + /* assert: message ends of added message are sorted in before existing messages */ + MExecution createdExecution = model.getMExecution(command.get()).get(); + MOccurrence createdStart = createdExecution.getStart().get(); + MOccurrence createdFinish = createdExecution.getFinish().get(); + MMessageEnd msg1send = model.getElement(QN, "1s", MMessageEnd.class); + MMessageEnd msg1receive = model.getElement(QN, "1r", MMessageEnd.class); + MMessageEnd msg2send = model.getElement(QN, "2s", MMessageEnd.class); + MMessageEnd msg2receive = model.getElement(QN, "2r", MMessageEnd.class); + + List otherEnds = Arrays.asList(msg1send, msg1receive, msg2send, msg2receive); + otherEnds.forEach(other -> assertThat(model.isSemanticallyBefore(createdStart, other), is(true))); + otherEnds.forEach(other -> assertThat(model.isSemanticallyBefore(createdFinish, other), is(true))); + } + + @Test + @SuppressWarnings({"boxing" }) + public void addMessageAndExecutionOnTopOfLifeline3() { + /* first command to create message */ + /* setup */ + MLifeline lifeline2 = interaction().getLifelines().get(1); + MLifeline lifeline3 = interaction().getLifelines().get(2); + CreationCommand command = lifeline2.insertMessageAfter(lifeline2, 20, lifeline3, + MessageSort.SYNCH_CALL_LITERAL, null); + + /* act */ + execute(command); + + /* second command to create execution */ + /* setup */ + CreationCommand command2 = lifeline3.insertExecutionAfter(lifeline3, 0, 20, + ACTION_EXECUTION_SPECIFICATION); + + /* act */ + execute(command2); + + /* assert: message ends of added message are sorted in before existing messages */ + MMessage createdMessage = model.getMMessage(command.get()).get(); + MMessageEnd createdSend = createdMessage.getSend().get(); + MMessageEnd createdReceive = createdMessage.getReceive().get(); + + MMessageEnd msg1send = model.getElement(QN, "1s", MMessageEnd.class); + MMessageEnd msg1receive = model.getElement(QN, "1r", MMessageEnd.class); + MMessageEnd msg2send = model.getElement(QN, "2s", MMessageEnd.class); + MMessageEnd msg2receive = model.getElement(QN, "2r", MMessageEnd.class); + + List otherEnds = Arrays.asList(msg1send, msg1receive, msg2send, msg2receive); + otherEnds.forEach(other -> assertThat(model.isSemanticallyBefore(createdSend, other), is(true))); + otherEnds.forEach(other -> assertThat(model.isSemanticallyBefore(createdReceive, other), is(true))); + + /* + * assert: message ends of added message and all other messages are sorted in after created execution + */ + MExecution createdExecution = model.getMExecution(command2.get()).get(); + MOccurrence createdStart = createdExecution.getStart().get(); + MOccurrence createdFinish = createdExecution.getFinish().get(); + + List createdEnds = Arrays.asList(createdSend, createdReceive); + createdEnds.forEach(other -> assertThat(model.isSemanticallyBefore(createdStart, other), is(true))); + createdEnds.forEach(other -> assertThat(model.isSemanticallyBefore(createdFinish, other), is(true))); + otherEnds.forEach(other -> assertThat(model.isSemanticallyBefore(createdStart, other), is(true))); + otherEnds.forEach(other -> assertThat(model.isSemanticallyBefore(createdFinish, other), is(true))); + } + +} diff --git a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/SemanticOrderAfterCreationTest.notation b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/SemanticOrderAfterCreationTest.notation new file mode 100644 index 00000000..8a46b136 --- /dev/null +++ b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/SemanticOrderAfterCreationTest.notation @@ -0,0 +1,54 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/SemanticOrderAfterCreationTest.uml b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/SemanticOrderAfterCreationTest.uml new file mode 100644 index 00000000..28953234 --- /dev/null +++ b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/SemanticOrderAfterCreationTest.uml @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + + + + diff --git a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/deletion/BasicDeletionTest.java b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/deletion/BasicDeletionTest.java index ecf17a37..6d5fd9cf 100644 --- a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/deletion/BasicDeletionTest.java +++ b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/deletion/BasicDeletionTest.java @@ -15,7 +15,7 @@ import org.eclipse.papyrus.uml.interaction.model.MLifeline; import org.eclipse.papyrus.uml.interaction.model.MMessage; import org.eclipse.papyrus.uml.interaction.model.spi.RemovalCommand; -import org.eclipse.papyrus.uml.interaction.tests.rules.ModelFixture; +import org.eclipse.papyrus.uml.interaction.model.tests.ModelEditFixture; import org.eclipse.papyrus.uml.interaction.tests.rules.ModelResource; import org.eclipse.uml2.uml.Element; import org.junit.Assert; @@ -26,7 +26,7 @@ public class BasicDeletionTest { @Rule - public final ModelFixture.Edit model = new ModelFixture.Edit(); + public final ModelEditFixture model = new ModelEditFixture(); private MInteraction interaction; @@ -48,7 +48,7 @@ private static IntSupplier fail(String msg) { private MInteraction interaction() { if (interaction == null) { - interaction = MInteraction.getInstance(model.getInteraction(), model.getSequenceDiagram().get()); + interaction = model.getMInteraction(); } return interaction; } @@ -70,7 +70,6 @@ private void executeAndAssertRemoval(RemovalCommand remove, /* force reinit after change */ interaction = null; - } @SuppressWarnings("unchecked") diff --git a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/deletion/CreationMessageDeletionTest.java b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/deletion/CreationMessageDeletionTest.java index 8845415b..30727279 100644 --- a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/deletion/CreationMessageDeletionTest.java +++ b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/deletion/CreationMessageDeletionTest.java @@ -5,9 +5,8 @@ import org.eclipse.emf.common.command.Command; import org.eclipse.papyrus.uml.interaction.model.MInteraction; import org.eclipse.papyrus.uml.interaction.model.MMessage; -import org.eclipse.papyrus.uml.interaction.tests.rules.ModelFixture; +import org.eclipse.papyrus.uml.interaction.model.tests.ModelEditFixture; import org.eclipse.papyrus.uml.interaction.tests.rules.ModelResource; -import org.junit.Assert; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -16,7 +15,7 @@ public class CreationMessageDeletionTest { @Rule - public final ModelFixture.Edit model = new ModelFixture.Edit(); + public final ModelEditFixture model = new ModelEditFixture(); private MInteraction interaction; @@ -64,16 +63,13 @@ public void before() { private MInteraction interaction() { if (interaction == null) { - interaction = MInteraction.getInstance(model.getInteraction(), model.getSequenceDiagram().get()); + interaction = model.getMInteraction(); } return interaction; } - private void execute(Command remove) { - if (!remove.canExecute()) { - Assert.fail("Command not executable"); //$NON-NLS-1$ - } - remove.execute(); + private void execute(Command command) { + model.execute(command); /* force reinit after change */ interaction = null; } diff --git a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/deletion/DeleteExecutionTest.java b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/deletion/DeleteExecutionTest.java index a6f73900..e8f5631a 100644 --- a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/deletion/DeleteExecutionTest.java +++ b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/deletion/DeleteExecutionTest.java @@ -19,9 +19,8 @@ import org.eclipse.papyrus.uml.interaction.model.MInteraction; import org.eclipse.papyrus.uml.interaction.model.MLifeline; import org.eclipse.papyrus.uml.interaction.model.MMessage; -import org.eclipse.papyrus.uml.interaction.tests.rules.ModelFixture; +import org.eclipse.papyrus.uml.interaction.model.tests.ModelEditFixture; import org.eclipse.papyrus.uml.interaction.tests.rules.ModelResource; -import org.junit.Assert; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -31,7 +30,7 @@ public class DeleteExecutionTest { @Rule - public final ModelFixture.Edit model = new ModelFixture.Edit(); + public final ModelEditFixture model = new ModelEditFixture(); private MInteraction interaction; @@ -89,16 +88,13 @@ public void before() { private MInteraction interaction() { if (interaction == null) { - interaction = MInteraction.getInstance(model.getInteraction(), model.getSequenceDiagram().get()); + interaction = model.getMInteraction(); } return interaction; } private void execute(Command command) { - if (!command.canExecute()) { - Assert.fail("Command not executable"); //$NON-NLS-1$ - } - command.execute(); + model.execute(command); /* force reinit after change */ interaction = null; } diff --git a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/deletion/DeletionMessageDeletionTest.java b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/deletion/DeletionMessageDeletionTest.java index e43ac40e..d1eb2860 100644 --- a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/deletion/DeletionMessageDeletionTest.java +++ b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/deletion/DeletionMessageDeletionTest.java @@ -5,9 +5,8 @@ import org.eclipse.emf.common.command.Command; import org.eclipse.papyrus.uml.interaction.model.MInteraction; import org.eclipse.papyrus.uml.interaction.model.MMessage; -import org.eclipse.papyrus.uml.interaction.tests.rules.ModelFixture; +import org.eclipse.papyrus.uml.interaction.model.tests.ModelEditFixture; import org.eclipse.papyrus.uml.interaction.tests.rules.ModelResource; -import org.junit.Assert; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -16,7 +15,7 @@ public class DeletionMessageDeletionTest { @Rule - public final ModelFixture.Edit model = new ModelFixture.Edit(); + public final ModelEditFixture model = new ModelEditFixture(); private MInteraction interaction; @@ -40,16 +39,13 @@ public class DeletionMessageDeletionTest { private MInteraction interaction() { if (interaction == null) { - interaction = MInteraction.getInstance(model.getInteraction(), model.getSequenceDiagram().get()); + interaction = model.getMInteraction(); } return interaction; } - private void execute(Command remove) { - if (!remove.canExecute()) { - Assert.fail("Command not executable"); //$NON-NLS-1$ - } - remove.execute(); + private void execute(Command command) { + model.execute(command); /* force reinit after change */ interaction = null; } diff --git a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/util/tests/LogicalModelPredicatesTest.java b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/util/tests/LogicalModelPredicatesTest.java index c0e9ee6d..1f91b7e9 100644 --- a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/util/tests/LogicalModelPredicatesTest.java +++ b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/util/tests/LogicalModelPredicatesTest.java @@ -17,78 +17,53 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; -import java.util.Optional; - import org.eclipse.papyrus.uml.interaction.model.MElement; -import org.eclipse.papyrus.uml.interaction.model.MInteraction; +import org.eclipse.papyrus.uml.interaction.model.tests.ModelEditFixture; import org.eclipse.papyrus.uml.interaction.model.util.LogicalModelPredicates; -import org.eclipse.papyrus.uml.interaction.tests.rules.ModelFixture; import org.eclipse.papyrus.uml.interaction.tests.rules.ModelResource; -import org.eclipse.uml2.uml.Element; -import org.junit.Before; import org.junit.Rule; import org.junit.Test; /** * Unit tests for the {@link LogicalModelPredicates} class. */ +@SuppressWarnings({"boxing", "nls" }) @ModelResource({"AnchorsModel.di", "AnchorsModel.uml", "AnchorsModel.notation" }) public class LogicalModelPredicatesTest { - @Rule - public final ModelFixture model = new ModelFixture.Edit(); + private static final String QN = "AnchorsModel::ExecutionSpecificationAnchors::%s"; - private MInteraction interaction; + @Rule + public final ModelEditFixture model = new ModelEditFixture(); @Test public void above_int() { - MElement requestRecv = getElement("request-recv"); + MElement requestRecv = model.getElement(QN, "request-recv"); assertThat(above(150).test(requestRecv), is(true)); assertThat(above(50).test(requestRecv), is(false)); } @Test public void above_MElement() { - MElement requestRecv = getElement("request-recv"); - MElement replySend = getElement("reply-send"); + MElement requestRecv = model.getElement(QN, "request-recv"); + MElement replySend = model.getElement(QN, "reply-send"); assertThat(above(replySend).test(requestRecv), is(true)); assertThat(above(requestRecv).test(replySend), is(false)); } @Test public void below_int() { - MElement requestRecv = getElement("request-recv"); + MElement requestRecv = model.getElement(QN, "request-recv"); assertThat(below(50).test(requestRecv), is(true)); assertThat(below(150).test(requestRecv), is(false)); } @Test public void below_MElement() { - MElement requestRecv = getElement("request-recv"); - MElement replySend = getElement("reply-send"); + MElement requestRecv = model.getElement(QN, "request-recv"); + MElement replySend = model.getElement(QN, "reply-send"); assertThat(below(requestRecv).test(replySend), is(true)); assertThat(below(replySend).test(requestRecv), is(false)); } - // - // Test framework - // - - @Before - public void setup() { - interaction = MInteraction.getInstance(model.getInteraction(), model.getSequenceDiagram().get()); - } - - public > T getElement(String name, Class type) { - String qName = String.format("AnchorsModel::ExecutionSpecificationAnchors::%s", name); - - Element element = model.getElement(qName); - return Optional.ofNullable(element).flatMap(interaction::getElement).filter(type::isInstance) - .map(type::cast).orElseThrow(() -> new AssertionError("no such element: " + name)); - } - - @SuppressWarnings("unchecked") - public > T getElement(String name) { - return (T)getElement(name, MElement.class); - } } From a6d4e7a3a49053bc743648b0579ef8561ae5e6f5 Mon Sep 17 00:00:00 2001 From: "Christian W. Damus" Date: Wed, 12 Sep 2018 08:52:25 -0400 Subject: [PATCH 10/11] Issue #343 code review comments Address comments from code review of tests contribution for issue #343. Signed-off-by: Christian W. Damus --- .../model/tests/ModelEditFixture.java | 76 +++++++++++++++---- ...cOrderAfterCreationOfElementOnTopTest.java | 66 ++++++++-------- 2 files changed, 94 insertions(+), 48 deletions(-) diff --git a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/ModelEditFixture.java b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/ModelEditFixture.java index cb39cd47..d33afead 100644 --- a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/ModelEditFixture.java +++ b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/ModelEditFixture.java @@ -29,6 +29,9 @@ import org.eclipse.uml2.uml.ExecutionSpecification; import org.eclipse.uml2.uml.InteractionFragment; import org.eclipse.uml2.uml.Message; +import org.hamcrest.Description; +import org.hamcrest.Matcher; +import org.hamcrest.TypeSafeDiagnosingMatcher; @SuppressWarnings("restriction") public class ModelEditFixture extends Edit { @@ -55,34 +58,81 @@ public Optional getMMessage(Message message) { } public Optional getMExecution(ExecutionSpecification execution) { - Optional> element = getMInteraction() - .getElement(execution); - if (element.isPresent() && element.get() instanceof MExecution) { - return Optional.of((MExecution)element.get()); - } - return Optional.empty(); + Optional> result = getMInteraction().getElement(execution); + return result.filter(MExecution.class::isInstance).map(MExecution.class::cast); } - public > T getElement(String qnFormat, String name, Class type) { - String qName = String.format(qnFormat, name); + public > T getElement(Class type, String qnFormat, + String... name) { + String qName = String.format(qnFormat, (Object[])name); Element element = getElement(qName); return Optional.ofNullable(element).flatMap(getMInteraction()::getElement).filter(type::isInstance) - .map(type::cast).orElseThrow(() -> new AssertionError("no such element: " + name)); //$NON-NLS-1$ + .map(type::cast).orElseThrow(() -> new AssertionError("no such element: " + qName)); //$NON-NLS-1$ } @SuppressWarnings("unchecked") - public > T getElement(String qnFormat, String name) { - return (T)getElement(qnFormat, name, MElement.class); + public > T getElement(String qnFormat, String... name) { + return (T)getElement(MElement.class, qnFormat, name); } - @SuppressWarnings({"boxing" }) - public Object isSemanticallyBefore(MOccurrence one, + public boolean isSemanticallyBefore(MOccurrence one, MOccurrence other) { + List fragments = getMInteraction().getElement().getFragments(); int indexOfOne = fragments.indexOf(one.getElement()); int indexOfOther = fragments.indexOf(other.getElement()); return indexOfOne >= 0 && indexOfOther >= 0 && indexOfOther > indexOfOne; } + public > Matcher isSemanticallyAfter( + MOccurrence other) { + + return new TypeSafeDiagnosingMatcher() { + + @Override + protected boolean matchesSafely(T item, Description mismatchDescription) { + boolean result = isSemanticallyBefore(other, item); + if (!result) { + mismatchDescription.appendValue(item); + mismatchDescription.appendText(" semantically precedes "); //$NON-NLS-1$ + mismatchDescription.appendValue(other); + } + return result; + } + + @Override + public void describeTo(Description description) { + description.appendText("semantically follows "); //$NON-NLS-1$ + description.appendValue(other); + } + + }; + } + + public > Matcher isSemanticallyBefore( + MOccurrence other) { + + return new TypeSafeDiagnosingMatcher() { + + @Override + protected boolean matchesSafely(T item, Description mismatchDescription) { + boolean result = isSemanticallyBefore(item, other); + if (!result) { + mismatchDescription.appendValue(item); + mismatchDescription.appendText(" semantically follows "); //$NON-NLS-1$ + mismatchDescription.appendValue(other); + } + return result; + } + + @Override + public void describeTo(Description description) { + description.appendText("semantically precedes "); //$NON-NLS-1$ + description.appendValue(other); + } + + }; + } + } diff --git a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/SemanticOrderAfterCreationOfElementOnTopTest.java b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/SemanticOrderAfterCreationOfElementOnTopTest.java index 0e7648ef..9ea21ce1 100644 --- a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/SemanticOrderAfterCreationOfElementOnTopTest.java +++ b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/SemanticOrderAfterCreationOfElementOnTopTest.java @@ -1,7 +1,7 @@ package org.eclipse.papyrus.uml.interaction.model.tests.creation; import static org.eclipse.uml2.uml.UMLPackage.Literals.ACTION_EXECUTION_SPECIFICATION; -import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.everyItem; import static org.junit.Assert.assertThat; import java.util.Arrays; @@ -48,7 +48,6 @@ private void execute(Command command) { } @Test - @SuppressWarnings({"boxing" }) public void addMessageOnTopFromLifeline2ToLifeline3() { /* setup */ MLifeline lifeline2 = interaction().getLifelines().get(1); @@ -63,18 +62,17 @@ public void addMessageOnTopFromLifeline2ToLifeline3() { MMessage createdMessage = model.getMMessage(command.get()).get(); MMessageEnd createdSend = createdMessage.getSend().get(); MMessageEnd createdReceive = createdMessage.getReceive().get(); - MMessageEnd msg1send = model.getElement(QN, "1s", MMessageEnd.class); - MMessageEnd msg1receive = model.getElement(QN, "1r", MMessageEnd.class); - MMessageEnd msg2send = model.getElement(QN, "2s", MMessageEnd.class); - MMessageEnd msg2receive = model.getElement(QN, "2r", MMessageEnd.class); + MMessageEnd msg1send = model.getElement(MMessageEnd.class, QN, "1s"); + MMessageEnd msg1receive = model.getElement(MMessageEnd.class, QN, "1r"); + MMessageEnd msg2send = model.getElement(MMessageEnd.class, QN, "2s"); + MMessageEnd msg2receive = model.getElement(MMessageEnd.class, QN, "2r"); List otherEnds = Arrays.asList(msg1send, msg1receive, msg2send, msg2receive); - otherEnds.forEach(other -> assertThat(model.isSemanticallyBefore(createdSend, other), is(true))); - otherEnds.forEach(other -> assertThat(model.isSemanticallyBefore(createdReceive, other), is(true))); + assertThat(otherEnds, everyItem(model.isSemanticallyAfter(createdSend))); + assertThat(otherEnds, everyItem(model.isSemanticallyAfter(createdReceive))); } @Test - @SuppressWarnings({"boxing" }) public void addMessageAfterFirstExistingMessageFromLifeline2ToLifeline3() { /* setup */ MLifeline lifeline2 = interaction().getLifelines().get(1); @@ -89,18 +87,18 @@ public void addMessageAfterFirstExistingMessageFromLifeline2ToLifeline3() { MMessage createdMessage = model.getMMessage(command.get()).get(); MMessageEnd createdSend = createdMessage.getSend().get(); MMessageEnd createdReceive = createdMessage.getReceive().get(); - MMessageEnd msg1send = model.getElement(QN, "1s", MMessageEnd.class); - MMessageEnd msg1receive = model.getElement(QN, "1r", MMessageEnd.class); - MMessageEnd msg2send = model.getElement(QN, "2s", MMessageEnd.class); - MMessageEnd msg2receive = model.getElement(QN, "2r", MMessageEnd.class); + MMessageEnd msg1send = model.getElement(MMessageEnd.class, QN, "1s"); + MMessageEnd msg1receive = model.getElement(MMessageEnd.class, QN, "1r"); + MMessageEnd msg2send = model.getElement(MMessageEnd.class, QN, "2s"); + MMessageEnd msg2receive = model.getElement(MMessageEnd.class, QN, "2r"); List endsBefore = Arrays.asList(msg1send, msg1receive); - endsBefore.forEach(end -> assertThat(model.isSemanticallyBefore(end, createdSend), is(true))); - endsBefore.forEach(end -> assertThat(model.isSemanticallyBefore(end, createdReceive), is(true))); + assertThat(endsBefore, everyItem(model.isSemanticallyBefore(createdSend))); + assertThat(endsBefore, everyItem(model.isSemanticallyBefore(createdReceive))); List endsAfter = Arrays.asList(msg2send, msg2receive); - endsAfter.forEach(end -> assertThat(model.isSemanticallyBefore(createdSend, end), is(true))); - endsAfter.forEach(end -> assertThat(model.isSemanticallyBefore(createdReceive, end), is(true))); + assertThat(endsAfter, everyItem(model.isSemanticallyAfter(createdSend))); + assertThat(endsAfter, everyItem(model.isSemanticallyAfter(createdReceive))); } @Test @@ -121,7 +119,6 @@ public void addExecutionOnTopOfLifeline3() { addAndAssertExecutionOnTopOfLifeline(lifeline3); } - @SuppressWarnings("boxing") private void addAndAssertExecutionOnTopOfLifeline(MLifeline lifeline) { /* setup */ CreationCommand command = lifeline.insertExecutionAfter(lifeline, 0, 20, @@ -134,18 +131,17 @@ private void addAndAssertExecutionOnTopOfLifeline(MLifeline lifeline) { MExecution createdExecution = model.getMExecution(command.get()).get(); MOccurrence createdStart = createdExecution.getStart().get(); MOccurrence createdFinish = createdExecution.getFinish().get(); - MMessageEnd msg1send = model.getElement(QN, "1s", MMessageEnd.class); - MMessageEnd msg1receive = model.getElement(QN, "1r", MMessageEnd.class); - MMessageEnd msg2send = model.getElement(QN, "2s", MMessageEnd.class); - MMessageEnd msg2receive = model.getElement(QN, "2r", MMessageEnd.class); + MMessageEnd msg1send = model.getElement(MMessageEnd.class, QN, "1s"); + MMessageEnd msg1receive = model.getElement(MMessageEnd.class, QN, "1r"); + MMessageEnd msg2send = model.getElement(MMessageEnd.class, QN, "2s"); + MMessageEnd msg2receive = model.getElement(MMessageEnd.class, QN, "2r"); List otherEnds = Arrays.asList(msg1send, msg1receive, msg2send, msg2receive); - otherEnds.forEach(other -> assertThat(model.isSemanticallyBefore(createdStart, other), is(true))); - otherEnds.forEach(other -> assertThat(model.isSemanticallyBefore(createdFinish, other), is(true))); + assertThat(otherEnds, everyItem(model.isSemanticallyAfter(createdStart))); + assertThat(otherEnds, everyItem(model.isSemanticallyAfter(createdFinish))); } @Test - @SuppressWarnings({"boxing" }) public void addMessageAndExecutionOnTopOfLifeline3() { /* first command to create message */ /* setup */ @@ -170,14 +166,14 @@ public void addMessageAndExecutionOnTopOfLifeline3() { MMessageEnd createdSend = createdMessage.getSend().get(); MMessageEnd createdReceive = createdMessage.getReceive().get(); - MMessageEnd msg1send = model.getElement(QN, "1s", MMessageEnd.class); - MMessageEnd msg1receive = model.getElement(QN, "1r", MMessageEnd.class); - MMessageEnd msg2send = model.getElement(QN, "2s", MMessageEnd.class); - MMessageEnd msg2receive = model.getElement(QN, "2r", MMessageEnd.class); + MMessageEnd msg1send = model.getElement(MMessageEnd.class, QN, "1s"); + MMessageEnd msg1receive = model.getElement(MMessageEnd.class, QN, "1r"); + MMessageEnd msg2send = model.getElement(MMessageEnd.class, QN, "2s"); + MMessageEnd msg2receive = model.getElement(MMessageEnd.class, QN, "2r"); List otherEnds = Arrays.asList(msg1send, msg1receive, msg2send, msg2receive); - otherEnds.forEach(other -> assertThat(model.isSemanticallyBefore(createdSend, other), is(true))); - otherEnds.forEach(other -> assertThat(model.isSemanticallyBefore(createdReceive, other), is(true))); + assertThat(otherEnds, everyItem(model.isSemanticallyAfter(createdSend))); + assertThat(otherEnds, everyItem(model.isSemanticallyAfter(createdReceive))); /* * assert: message ends of added message and all other messages are sorted in after created execution @@ -187,10 +183,10 @@ public void addMessageAndExecutionOnTopOfLifeline3() { MOccurrence createdFinish = createdExecution.getFinish().get(); List createdEnds = Arrays.asList(createdSend, createdReceive); - createdEnds.forEach(other -> assertThat(model.isSemanticallyBefore(createdStart, other), is(true))); - createdEnds.forEach(other -> assertThat(model.isSemanticallyBefore(createdFinish, other), is(true))); - otherEnds.forEach(other -> assertThat(model.isSemanticallyBefore(createdStart, other), is(true))); - otherEnds.forEach(other -> assertThat(model.isSemanticallyBefore(createdFinish, other), is(true))); + assertThat(createdEnds, everyItem(model.isSemanticallyAfter(createdStart))); + assertThat(createdEnds, everyItem(model.isSemanticallyAfter(createdFinish))); + assertThat(otherEnds, everyItem(model.isSemanticallyAfter(createdStart))); + assertThat(otherEnds, everyItem(model.isSemanticallyAfter(createdFinish))); } } From 66011de8b5573e3a99d5bc0e82a3d03e620cc14e Mon Sep 17 00:00:00 2001 From: "Christian W. Damus" Date: Wed, 12 Sep 2018 10:57:44 -0400 Subject: [PATCH 11/11] Issue #343: Insertion problems MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix the semantic insertion order of interaction fragments in the case of creating an execution specification. Includes fixing a test that didn’t account for tear-down of the logical model when the model is changed. Also adding the Papyrus di resources to git for ease of reference to the notation in a Papyrus run-time workbench. Signed-off-by: Christian W. Damus --- .../commands/InsertExecutionCommand.java | 28 ++++++++-------- .../model/commands/InsertMessageCommand.java | 7 +++- .../internal/model/commands/ModelCommand.java | 32 +++++++------------ .../tests/creation/CreateExecutionTesting.di | 2 ++ .../tests/creation/CreateExecutionTesting.uml | 2 +- ...cOrderAfterCreationOfElementOnTopTest.java | 7 ++-- .../SemanticOrderAfterCreationTest.di | 2 ++ 7 files changed, 40 insertions(+), 40 deletions(-) create mode 100644 tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/CreateExecutionTesting.di create mode 100644 tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/SemanticOrderAfterCreationTest.di diff --git a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/InsertExecutionCommand.java b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/InsertExecutionCommand.java index 5afd0af3..9ad4fd4e 100644 --- a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/InsertExecutionCommand.java +++ b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/InsertExecutionCommand.java @@ -12,9 +12,9 @@ package org.eclipse.papyrus.uml.interaction.internal.model.commands; +import java.util.List; import java.util.Optional; import java.util.OptionalInt; -import java.util.function.Function; import org.eclipse.emf.common.command.Command; import org.eclipse.emf.common.command.UnexecutableCommand; @@ -146,22 +146,19 @@ protected Command createCommand() { return UnexecutableCommand.INSTANCE; } - MElement insertionPoint = normalizeFragmentInsertionPoint(before); - Function paramsFactory = CreationParameters::after; - - if (insertionPoint instanceof MLifeline) { - // Insert before the first covering fragment, if any - Optional> covering = ((MLifeline)insertionPoint) - .following(insertionPoint); - if (covering.isPresent()) { - insertionPoint = covering.get(); - paramsFactory = CreationParameters::before; - } - } + // Determine the semantic element before which to insert the execution and its + // start and finish occurrences + List> timeline = getTimeline(getTarget().getInteraction()); + int absoluteExecY = referenceY.getAsInt() + offset; + Optional> insertAt = getInsertionPoint(timeline, absoluteExecY) + .map(this::normalizeFragmentInsertionPoint); SemanticHelper semantics = semanticHelper(); - CreationParameters execParams = paramsFactory.apply(insertionPoint.getElement()); + CreationParameters execParams = CreationParameters.in(getTarget().getInteraction().getElement(), + UMLPackage.Literals.INTERACTION__FRAGMENT); execParams.setEClass(eClass); + execParams.setInsertBefore( + () -> insertAt.map(MElement::getElement).map(Element.class::cast).orElse(null)); resultCommand = semantics.createExecutionSpecification(specification, execParams); CreationParameters startParams = CreationParameters.before(resultCommand); CreationCommand start = semantics.createStart(resultCommand, startParams); @@ -180,7 +177,8 @@ protected Command createCommand() { // Now we have commands to add the execution specification. But, first we must make // room for it in the diagram. Nudge the element that will follow the new execution - Optional makeSpace = createNudgeCommand(insertionPoint, execParams.isInsertBefore()); + Optional makeSpace = insertAt + .flatMap(insertionPoint -> createNudgeCommand(insertionPoint, execParams.isInsertBefore())); if (makeSpace.isPresent()) { result = makeSpace.get().chain(result); } 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 23154df2..3c5bfd24 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 @@ -421,7 +421,12 @@ protected Command createCommand() { // Now we have commands to add the message specification. But, first we must make // room for it in the diagram. Nudge the element that will follow the new receive event int spaceRequired = 2 * sendOffset; - MElement distanceFrom = normalizeFragmentInsertionPoint(beforeSend); + // If inserting after the start occurrence of an execution specification, + // then actually insert after the execution, itself, so that it can span + // the new message end + MElement distanceFrom = (MElement)Optional.of(beforeSend) + .filter(MOccurrence.class::isInstance).map(MOccurrence.class::cast) + .flatMap(MOccurrence::getStartedExecution).orElse(beforeSend); Optional makeSpace = getTarget().following(distanceFrom).map(el -> { OptionalInt distance = el.verticalDistance(distanceFrom); return distance.isPresent() ? el.nudge(max(0, spaceRequired - distance.getAsInt())) diff --git a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/ModelCommand.java b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/ModelCommand.java index 471236ad..3d0e365a 100644 --- a/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/ModelCommand.java +++ b/plugins/org.eclipse.papyrus.uml.interaction.model/src/org/eclipse/papyrus/uml/interaction/internal/model/commands/ModelCommand.java @@ -29,11 +29,9 @@ import org.eclipse.papyrus.uml.interaction.internal.model.impl.MElementImpl; import org.eclipse.papyrus.uml.interaction.internal.model.impl.MInteractionImpl; import org.eclipse.papyrus.uml.interaction.model.MElement; -import org.eclipse.papyrus.uml.interaction.model.MExecution; import org.eclipse.papyrus.uml.interaction.model.MInteraction; 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.DiagramHelper; import org.eclipse.papyrus.uml.interaction.model.spi.LayoutHelper; import org.eclipse.papyrus.uml.interaction.model.spi.SemanticHelper; @@ -116,33 +114,23 @@ protected MElement normalizeFragmentInsertionPoint(MElement insertionPoint MElement result = insertionPoint; if (insertionPoint instanceof MMessage) { - // After the message is received, if it is received + // Before the message is sent, if it is sent MMessage message = (MMessage)insertionPoint; - Optional end = message.getReceive(); + Optional end = message.getSend(); if (!end.isPresent()) { - end = message.getSend(); + end = message.getReceive(); } if (end.isPresent()) { result = end.get(); } } else if (insertionPoint instanceof MMessageEnd) { MMessageEnd end = (MMessageEnd)insertionPoint; - if (end.isSend()) { - // After the message is received, if it is received + if (end.isReceive()) { + // Before the message is sent, if it is sent result = end.getOtherEnd().orElse(end); } } - if (result instanceof MOccurrence) { - // Insert after the execution specification that it starts (if any) - // so that the new fragment can be grouped in that occurrence - MOccurrence occurrence = (MOccurrence)result; - Optional exec = occurrence.getStartedExecution(); - if (exec.isPresent()) { - result = exec.get(); - } - } - return result; } @@ -183,10 +171,12 @@ protected static Optional> getInsertionPoint( int index = Collections.binarySearch(timeline, searchToken, compareByTop()); if (index >= 0) { - // Easy case: precede the fragments currently at that Y position, which means insert - // before the first element following that is at this position - while ((index > 0) && (timeline.get(index - 1).getTop().orElse(-1) == yPosition)) { - index--; + // Easy case: precede the first fragment at a greater Y position than this element. + // This handles, for example, the common case of inserting an execution specification + // after a message receive end at offset 0, which then should not result in the + // execution semantically preceding that message end + while ((index < size) && (timeline.get(index).getTop().orElse(-1) == yPosition)) { + index++; } } else { // The search found an "insertion point", which is where the fragment is before which diff --git a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/CreateExecutionTesting.di b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/CreateExecutionTesting.di new file mode 100644 index 00000000..bf9abab3 --- /dev/null +++ b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/CreateExecutionTesting.di @@ -0,0 +1,2 @@ + + diff --git a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/CreateExecutionTesting.uml b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/CreateExecutionTesting.uml index bf9664bc..f46656f1 100644 --- a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/CreateExecutionTesting.uml +++ b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/CreateExecutionTesting.uml @@ -15,7 +15,7 @@ - + diff --git a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/SemanticOrderAfterCreationOfElementOnTopTest.java b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/SemanticOrderAfterCreationOfElementOnTopTest.java index 9ea21ce1..d182111b 100644 --- a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/SemanticOrderAfterCreationOfElementOnTopTest.java +++ b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/SemanticOrderAfterCreationOfElementOnTopTest.java @@ -153,8 +153,11 @@ public void addMessageAndExecutionOnTopOfLifeline3() { /* act */ execute(command); - /* second command to create execution */ - /* setup */ + /* + * second command to create execution. The first command invalidated the logical model, so we need to + * find lifeline3 again + */ + lifeline3 = interaction().getLifelines().get(2); CreationCommand command2 = lifeline3.insertExecutionAfter(lifeline3, 0, 20, ACTION_EXECUTION_SPECIFICATION); diff --git a/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/SemanticOrderAfterCreationTest.di b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/SemanticOrderAfterCreationTest.di new file mode 100644 index 00000000..bf9abab3 --- /dev/null +++ b/tests/org.eclipse.papyrus.uml.interaction.model.tests/src/org/eclipse/papyrus/uml/interaction/model/tests/creation/SemanticOrderAfterCreationTest.di @@ -0,0 +1,2 @@ + +