diff options
author | Christian W. Damus | 2018-09-28 14:10:33 +0000 |
---|---|---|
committer | Camille Letavernier | 2018-10-04 07:40:11 +0000 |
commit | 7370e301eba4d962e04b42983c2978e33b36f94d (patch) | |
tree | 73495226b20f4e80ae0e1fcd00778483b294b5ca | |
parent | df6c48adfc6ab8930f972adef7f301694d9837cf (diff) | |
download | org.eclipse.papyrus-bugs/536486-timeDurationsOrderings.tar.gz org.eclipse.papyrus-bugs/536486-timeDurationsOrderings.tar.xz org.eclipse.papyrus-bugs/536486-timeDurationsOrderings.zip |
Bug 537571: [Sequence Diagram] Support Time Observation/Constraint as a node on an eventbugs/536486-timeDurationsOrderings
Fix problems in the location of time elements on lifelines:
- failure to refresh after undo moves a message end
- time element located on the center of the lifeline head
instead of opposite the create message when opening
a diagram
Change-Id: Ice6a31e9b4bb1e757378494a62a2714d775f47dd
Signed-off-by: Christian W. Damus <give.a.damus@gmail.com>
5 files changed, 328 insertions, 36 deletions
diff --git a/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/custom-src/org/eclipse/papyrus/uml/diagram/sequence/edit/parts/CLifeLineEditPart.java b/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/custom-src/org/eclipse/papyrus/uml/diagram/sequence/edit/parts/CLifeLineEditPart.java index 386bb347b38..d53628f7baf 100644 --- a/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/custom-src/org/eclipse/papyrus/uml/diagram/sequence/edit/parts/CLifeLineEditPart.java +++ b/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/custom-src/org/eclipse/papyrus/uml/diagram/sequence/edit/parts/CLifeLineEditPart.java @@ -294,15 +294,13 @@ public class CLifeLineEditPart extends LifelineEditPart { .filter(MessageEnd::isReceive) .filter(end -> end.getMessage().getMessageSort() == MessageSort.CREATE_MESSAGE_LITERAL); - return createEnd.map(this::getCreateMessageIncomingSide) - .filter(OptionalInt::isPresent) - .map(side -> { - getBorderedFigure().getBorderItemContainer() - .add(((IGraphicalEditPart) childEditPart).getFigure(), - new TimeElementLocator(getMainFigure(), this::getTimeElementSide)); - - return true; - }).orElseGet(() -> super.addFixedChild(childEditPart)); + return createEnd.map(__ -> { + getBorderedFigure().getBorderItemContainer() + .add(((IGraphicalEditPart) childEditPart).getFigure(), + new TimeElementLocator(getMainFigure(), this::getTimeElementSide)); + + return true; + }).orElseGet(() -> super.addFixedChild(childEditPart)); } public OptionalInt getCreateMessageIncomingSide(Point where) { @@ -350,10 +348,11 @@ public class CLifeLineEditPart extends LifelineEditPart { } int getTimeElementSide(Rectangle proposedBounds) { - int incoming = getCreateMessageIncomingSide(proposedBounds.getTopLeft()) - .orElse(PositionConstants.WEST); - - // Put the time element on the side opposite to the incoming create message - return PositionConstants.EAST_WEST ^ incoming; + OptionalInt incoming = getCreateMessageIncomingSide(proposedBounds.getTopLeft()); + return incoming.isPresent() + // Put the time element on the side opposite to the incoming create message + ? PositionConstants.EAST_WEST ^ incoming.getAsInt() + // Center it on the lifeline + : PositionConstants.CENTER; } } diff --git a/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/custom-src/org/eclipse/papyrus/uml/diagram/sequence/edit/parts/TimeElementEditPartHelper.java b/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/custom-src/org/eclipse/papyrus/uml/diagram/sequence/edit/parts/TimeElementEditPartHelper.java index f950853d680..c427bbf30c0 100644 --- a/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/custom-src/org/eclipse/papyrus/uml/diagram/sequence/edit/parts/TimeElementEditPartHelper.java +++ b/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence/custom-src/org/eclipse/papyrus/uml/diagram/sequence/edit/parts/TimeElementEditPartHelper.java @@ -15,29 +15,39 @@ package org.eclipse.papyrus.uml.diagram.sequence.edit.parts; +import java.beans.PropertyChangeEvent; +import java.beans.PropertyChangeListener; import java.util.Optional; +import java.util.concurrent.Executor; import java.util.function.Supplier; import org.eclipse.draw2d.Connection; import org.eclipse.draw2d.ConnectionAnchor; import org.eclipse.draw2d.geometry.Dimension; import org.eclipse.draw2d.geometry.Point; +import org.eclipse.draw2d.geometry.PointList; import org.eclipse.draw2d.geometry.Rectangle; import org.eclipse.gef.ConnectionEditPart; -import org.eclipse.gef.EditPart; +import org.eclipse.gef.Disposable; import org.eclipse.gmf.runtime.diagram.ui.editparts.IGraphicalEditPart; import org.eclipse.gmf.runtime.diagram.ui.figures.IBorderItemLocator; import org.eclipse.gmf.runtime.notation.NotationPackage; +import org.eclipse.papyrus.infra.core.utils.OneShotExecutor; +import org.eclipse.papyrus.infra.gmfdiag.common.helper.DiagramHelper; import org.eclipse.papyrus.infra.gmfdiag.common.utils.DiagramEditPartsUtil; +import org.eclipse.papyrus.infra.ui.util.TransactionUIHelper; import org.eclipse.uml2.uml.MessageEnd; /** * Common behaviour that time-element edit-parts can delegate. */ class TimeElementEditPartHelper { + private static final MessageTracker NONE = new MessageTracker(); private final IGraphicalEditPart owner; private final Supplier<? extends Optional<MessageEnd>> messageEndSupplier; + private final Executor executor; + private MessageTracker messageTracker = NONE; /** * Initializes me with the edit-part that I help. @@ -54,14 +64,16 @@ class TimeElementEditPartHelper { this.owner = owner; this.messageEndSupplier = messageEndSupplier; + + // Don't post redundant refreshes + this.executor = new OneShotExecutor(TransactionUIHelper.getExecutor(owner.getEditingDomain())); } boolean refreshBounds(IBorderItemLocator locator) { boolean result = false; if (locator != null) { - Optional<Point> messageEndLoc = getMessageEnd().map(this::getLocation); - + Optional<Point> messageEndLoc = Optional.ofNullable(getLocation()); if (messageEndLoc.isPresent()) { // We are fixed by a message end, then Dimension size = new Dimension( @@ -76,6 +88,11 @@ class TimeElementEditPartHelper { return result; } + // Asynchronously post a refresh of my owner edit-part + private void postRefresh() { + executor.execute(() -> DiagramHelper.refresh(owner, true)); + } + /** * Obtain the message end that is my observedor constrained event, if any. * @@ -93,25 +110,107 @@ class TimeElementEditPartHelper { * @return the location of that end relative to my parent, or {@code null} if it cannot * be determined */ - Point getLocation(MessageEnd messageEnd) { - Point result = null; - - if (messageEnd != null) { - EditPart messageEP = DiagramEditPartsUtil.getChildByEObject(messageEnd.getMessage(), - (IGraphicalEditPart) owner.getRoot().getContents(), true); - if (messageEP instanceof ConnectionEditPart) { - Connection connection = (Connection) ((ConnectionEditPart) messageEP).getFigure(); - ConnectionAnchor anchor = messageEnd.isSend() - ? connection.getSourceAnchor() - : connection.getTargetAnchor(); - if (anchor != null) { - result = anchor.getReferencePoint().getCopy(); - owner.getFigure().getParent().translateToRelative(result); - } + Point getLocation() { + return getMessage().getLocation(); + } + + private MessageTracker getMessage() { + if (!messageTracker.isValid()) { + messageTracker.dispose(); + + // Refresh our idea of what the message is + messageTracker = messageEndSupplier.get() + .map(end -> new MessageTracker(this, end)) + .filter(MessageTracker::isValid) + .orElse(NONE); + } + + return messageTracker; + } + + // + // Nested types + // + + /** + * A helper for tracking the message, if any, the end of which is linked to a time + * element to cause that time element to follow (track) the movement of the message + * end. + */ + private static final class MessageTracker implements Supplier<ConnectionEditPart>, Disposable { + private final PropertyChangeListener connectionListener = this::connectionMoved; + private final TimeElementEditPartHelper owner; + private final ConnectionEditPart connectionEP; + private final Connection connection; + private final boolean source; + private final Point anchor = new Point(); + + MessageTracker() { + super(); + + this.owner = null; + connectionEP = null; + connection = null; + source = false; + } + + MessageTracker(TimeElementEditPartHelper owner, MessageEnd end) { + super(); + + this.owner = owner; + + IGraphicalEditPart ep = DiagramEditPartsUtil.getChildByEObject(end.getMessage(), + (IGraphicalEditPart) owner.owner.getRoot().getContents(), true); + if ((ep instanceof ConnectionEditPart) && ep.isActive()) { + connectionEP = (ConnectionEditPart) ep; + connection = (Connection) connectionEP.getFigure(); + connection.addPropertyChangeListener(Connection.PROPERTY_POINTS, connectionListener); + source = end.isSend(); + anchor.setLocation(source ? connection.getPoints().getFirstPoint() : connection.getPoints().getLastPoint()); + } else { + connectionEP = null; + connection = null; + source = false; } } - return result; + @Override + public void dispose() { + if (connection != null) { + connection.removePropertyChangeListener(Connection.PROPERTY_POINTS, connectionListener); + } + } + + @Override + public ConnectionEditPart get() { + return isValid() ? connectionEP : null; + } + + public Point getLocation() { + Point result = null; + + if (isValid()) { + // If we were ever valid, we have a connection + ConnectionAnchor anchor = source ? connection.getSourceAnchor() : connection.getTargetAnchor(); + result = anchor.getReferencePoint().getCopy(); + owner.owner.getFigure().getParent().translateToRelative(result); + } + + return result; + } + + boolean isValid() { + return (connectionEP != null) && connectionEP.isActive(); + } + + private void connectionMoved(PropertyChangeEvent event) { + PointList points = (PointList) event.getNewValue(); + Point newAnchor = source ? points.getFirstPoint() : points.getLastPoint(); + if (!anchor.equals(newAnchor)) { + anchor.setLocation(newAnchor); + owner.postRefresh(); + } + } } } diff --git a/tests/junit/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence.tests/src/org/eclipse/papyrus/uml/diagram/sequence/tests/bug/BugTests.java b/tests/junit/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence.tests/src/org/eclipse/papyrus/uml/diagram/sequence/tests/bug/BugTests.java index 528c7263120..062fe1de45b 100644 --- a/tests/junit/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence.tests/src/org/eclipse/papyrus/uml/diagram/sequence/tests/bug/BugTests.java +++ b/tests/junit/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence.tests/src/org/eclipse/papyrus/uml/diagram/sequence/tests/bug/BugTests.java @@ -48,6 +48,7 @@ import org.junit.runners.Suite.SuiteClasses; DurationObservationCreationTest.class, GeneralOrderingCreationTest.class, TimeElementCreationTest.class, + TimeElementMoveTest.class, }) public class BugTests { diff --git a/tests/junit/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence.tests/src/org/eclipse/papyrus/uml/diagram/sequence/tests/bug/TimeElementCreationTest.java b/tests/junit/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence.tests/src/org/eclipse/papyrus/uml/diagram/sequence/tests/bug/TimeElementCreationTest.java index 0426ea197ec..dc4c61f571b 100644 --- a/tests/junit/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence.tests/src/org/eclipse/papyrus/uml/diagram/sequence/tests/bug/TimeElementCreationTest.java +++ b/tests/junit/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence.tests/src/org/eclipse/papyrus/uml/diagram/sequence/tests/bug/TimeElementCreationTest.java @@ -225,7 +225,6 @@ public class TimeElementCreationTest extends AbstractPapyrusTest { @Test public void createOnLifelineCreationLeftToRightCreateMessage() { EditPart lifelineEP = editor.findEditPart("thing", Lifeline.class); - EditPart messageEP = editor.findEditPart("create1", Message.class); Point llTop = getGeometry(lifelineEP, Rectangle.class).getTop(); EditPart timeEP = editor.createShape(lifelineEP, what.getElementType(), @@ -250,7 +249,6 @@ public class TimeElementCreationTest extends AbstractPapyrusTest { @Test public void createOnLifelineCreationRightToLeftCreateMessage() { EditPart lifelineEP = editor.findEditPart("whatsit", Lifeline.class); - EditPart messageEP = editor.findEditPart("create2", Message.class); Point llTop = getGeometry(lifelineEP, Rectangle.class).getTop(); EditPart timeEP = editor.createShape(lifelineEP, what.getElementType(), @@ -381,7 +379,7 @@ public class TimeElementCreationTest extends AbstractPapyrusTest { * @throws IllegalArgumentException * if the geometry {@link type} is not recognized */ - <T extends Translatable> T getGeometry(EditPart editPart, Class<T> type) { + static <T extends Translatable> T getGeometry(EditPart editPart, Class<T> type) { if (!Rectangle.class.isAssignableFrom(type) && !PointList.class.isAssignableFrom(type)) { throw new IllegalArgumentException("unrecognized geometry type: " + type.getName()); } diff --git a/tests/junit/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence.tests/src/org/eclipse/papyrus/uml/diagram/sequence/tests/bug/TimeElementMoveTest.java b/tests/junit/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence.tests/src/org/eclipse/papyrus/uml/diagram/sequence/tests/bug/TimeElementMoveTest.java new file mode 100644 index 00000000000..fa290a310f6 --- /dev/null +++ b/tests/junit/plugins/uml/diagram/org.eclipse.papyrus.uml.diagram.sequence.tests/src/org/eclipse/papyrus/uml/diagram/sequence/tests/bug/TimeElementMoveTest.java @@ -0,0 +1,195 @@ +/***************************************************************************** + * Copyright (c) 2018 Christian W. Damus, CEA LIST, and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Christian W. Damus - Initial API and implementation + * + *****************************************************************************/ + +package org.eclipse.papyrus.uml.diagram.sequence.tests.bug; + +import static org.eclipse.papyrus.uml.diagram.sequence.tests.bug.TimeElementCreationTest.getGeometry; +import static org.hamcrest.MatcherAssert.assertThat; + +import java.util.Arrays; +import java.util.Objects; +import java.util.Optional; + +import org.eclipse.draw2d.geometry.Point; +import org.eclipse.draw2d.geometry.PointList; +import org.eclipse.draw2d.geometry.Rectangle; +import org.eclipse.gef.ConnectionEditPart; +import org.eclipse.gef.EditPart; +import org.eclipse.gef.Request; +import org.eclipse.gef.RequestConstants; +import org.eclipse.gef.requests.BendpointRequest; +import org.eclipse.gef.requests.ChangeBoundsRequest; +import org.eclipse.papyrus.infra.types.core.registries.ElementTypeSetConfigurationRegistry; +import org.eclipse.papyrus.junit.framework.classification.tests.AbstractPapyrusTest; +import org.eclipse.papyrus.junit.utils.rules.ActiveDiagram; +import org.eclipse.papyrus.junit.utils.rules.PapyrusEditorFixture; +import org.eclipse.papyrus.junit.utils.rules.PluginResource; +import org.eclipse.papyrus.uml.diagram.sequence.tests.bug.TimeElementCreationTest.What; +import org.eclipse.ui.IEditorPart; +import org.eclipse.ui.IWorkbenchPage; +import org.eclipse.uml2.uml.ExecutionSpecification; +import org.eclipse.uml2.uml.Lifeline; +import org.eclipse.uml2.uml.Message; +import org.eclipse.uml2.uml.TimeConstraint; +import org.eclipse.uml2.uml.TimeObservation; +import org.hamcrest.CustomTypeSafeMatcher; +import org.hamcrest.Matcher; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; + +/** + * Regression tests for moving (with undo) of {@link TimeObservation}s and + * {@link TimeConstraint}s in the sequence diagram editor. + * + * @author Christian W. Damus + * @see <a href="http://eclip.se/537571">bug 537571</a> + */ +@PluginResource("resource/bugs/bug537571-times.di") +@ActiveDiagram("scenario") +@RunWith(Parameterized.class) +public class TimeElementMoveTest extends AbstractPapyrusTest { + private static final boolean SEND = true; + private static final boolean RECV = true; + + @Rule + public final PapyrusEditorFixture editor = new PapyrusEditorFixture(); + + private final What what; + + static { + // Kick the modeled element types registry that is needed by the tests + ElementTypeSetConfigurationRegistry.getInstance(); + } + + /** + * Initializes me. + * + * @param what + * what to create + */ + public TimeElementMoveTest(What what) { + super(); + + this.what = what; + } + + @Test + public void moveOnMessageSendOnLifeline() { + EditPart lifelineEP = editor.findEditPart("foo", Lifeline.class); + EditPart messageEP = editor.findEditPart("message1", Message.class); + PointList points = getGeometry(messageEP, PointList.class); + + EditPart timeEP = editor.createShape(lifelineEP, what.getElementType(), + points.getFirstPoint(), null); + + moveMessageAssertion(messageEP, timeEP, SEND); + } + + private void moveMessageAssertion(EditPart messageEP, EditPart timeEP, boolean sourceEnd) { + PointList messageGeom = getGeometry(messageEP, PointList.class); + int messageY = (sourceEnd ? messageGeom.getFirstPoint() : messageGeom.getLastPoint()).y(); + Rectangle original = getGeometry(timeEP, Rectangle.class); + Rectangle expected = original.getCopy(); + expected.setY(messageY - 20); + + move(messageEP, 0, -20); + + assertThat("Not moved correctly", getGeometry(timeEP, Rectangle.class), equalGeometry(expected)); + + expected.translate(0, 20); + editor.undo(); + + assertThat("Not tracked on undo", getGeometry(timeEP, Rectangle.class), equalGeometry(original)); + } + + @Test + public void moveOnMessageReceiveOnExecution() { + EditPart execEP = editor.findEditPart("exec2", ExecutionSpecification.class); + EditPart messageEP = editor.findEditPart("message4", Message.class); + Rectangle bounds = getGeometry(execEP, Rectangle.class); + PointList points = getGeometry(messageEP, PointList.class); + + EditPart timeEP = editor.createShape(execEP, what.getElementType(), + new Point(bounds.getTop().x(), points.getLastPoint().y()), null); + + moveMessageAssertion(messageEP, timeEP, RECV); + } + + // + // Test framework + // + + @Parameters(name = "{0}") + public static Iterable<Object[]> parameters() { + return TimeElementCreationTest.parameters(); + } + + @Before + public void maximizeEditor() { + IEditorPart editor = this.editor.getEditor(); + IWorkbenchPage page = editor.getSite().getPage(); + page.setPartState(page.getReference(editor), IWorkbenchPage.STATE_MAXIMIZED); + this.editor.flushDisplayEvents(); + } + + /** + * Work around the absence of an {@code equals} method in the {@link PointList} class. + * + * @param geometry + * a geometry to test for equality with an actual observed geometry + * @return the geometry matcher + */ + static Matcher<Object> equalGeometry(Object geometry) { + return new CustomTypeSafeMatcher<Object>("equals " + geometry) { + @Override + protected boolean matchesSafely(Object item) { + + return ((item instanceof PointList) && (geometry instanceof PointList)) + ? Arrays.equals(((PointList) item).toIntArray(), + ((PointList) geometry).toIntArray()) + : Objects.equals(item, geometry); + } + }; + } + + void move(EditPart editPart, int deltaX, int deltaY) { + Request move; + if (editPart instanceof ConnectionEditPart) { + ConnectionEditPart connection = (ConnectionEditPart) editPart; + Point mid = getGeometry(connection, PointList.class).getMidpoint(); + mid.translate(deltaX, deltaY); + + // Messages are moved by the gesture that creates bendpoints + BendpointRequest bendpoint = new BendpointRequest(); + bendpoint.setType(RequestConstants.REQ_CREATE_BENDPOINT); + bendpoint.setSource(connection); + bendpoint.setLocation(mid); + move = bendpoint; + } else { + ChangeBoundsRequest bounds = new ChangeBoundsRequest(RequestConstants.REQ_MOVE); + bounds.setEditParts(editPart); + bounds.setConstrainedMove(false); + bounds.setMoveDelta(new Point(deltaX, deltaY)); + move = bounds; + } + + EditPart target = editPart.getTargetEditPart(move); + editor.execute(Optional.ofNullable(target).orElse(editPart).getCommand(move)); + } +} |