Skip to content

Commit

Permalink
[931] Removing an edge does not affect position of other edges
Browse files Browse the repository at this point in the history
Bug: #931
Signed-off-by: Guillaume Coutable <[email protected]>
  • Loading branch information
gcoutable committed Sep 7, 2022
1 parent 1725b14 commit 15f6948
Show file tree
Hide file tree
Showing 18 changed files with 360 additions and 26 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ To integrate the "Details" view inside a workbench, one must now use the new `De
- https://github.com/eclipse-sirius/sirius-components/issues/1268[#1268] [form] Use literal instead of name for Enum label & newValue
- https://github.com/eclipse-sirius/sirius-components/issues/1308[#1308] [workbench] Fix the vertical overflow issue on the view header
- https://github.com/eclipse-sirius/sirius-components/issues/1306[#1306] [form] Fix unable to set None value from a Select widget from View Form
- https://github.com/eclipse-sirius/sirius-components/issues/931[#931] [layout] When the delete tool of an edge is used, the position of other edges are not affected.

=== Improvements

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ private Diagram doRender(String label, Object targetObject, IEditingContext edit
.diagramDescription(diagramDescription)
.viewCreationRequests(viewCreationRequests)
.viewDeletionRequests(viewDeletionRequests)
.previousDiagram(optionalPreviousDiagram);
.previousDiagram(optionalPreviousDiagram)
.diagramEvent(optionalDiagramElementEvent);
//@formatter:on

DiagramComponentProps props = builder.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import org.eclipse.sirius.components.diagrams.description.DiagramDescription;
import org.eclipse.sirius.components.diagrams.description.EdgeDescription;
import org.eclipse.sirius.components.diagrams.description.NodeDescription;
import org.eclipse.sirius.components.diagrams.events.RemoveEdgeEvent;
import org.eclipse.sirius.components.representations.Failure;
import org.eclipse.sirius.components.representations.IStatus;
import org.eclipse.sirius.components.representations.Success;
Expand Down Expand Up @@ -116,11 +117,13 @@ private void handleDelete(One<IPayload> payloadSink, Many<ChangeDescription> cha
List<String> errors = new ArrayList<>();
boolean atLeastOneOk = false;
Diagram diagram = diagramContext.getDiagram();
List<String> deletedEdgeIds = new ArrayList<>();
for (String edgeId : diagramInput.getEdgeIds()) {
var optionalElement = this.diagramQueryService.findEdgeById(diagram, edgeId);
if (optionalElement.isPresent()) {
IStatus status = this.invokeDeleteEdgeTool(optionalElement.get(), editingContext, diagramContext, diagramInput.getDeletionPolicy());
if (status instanceof Success) {
deletedEdgeIds.add(edgeId);
atLeastOneOk = true;
} else {
errors.add(((Failure) status).getMessage());
Expand All @@ -145,6 +148,8 @@ private void handleDelete(One<IPayload> payloadSink, Many<ChangeDescription> cha
}
}

RemoveEdgeEvent removeEdgeEvent = new RemoveEdgeEvent(deletedEdgeIds);
diagramContext.setDiagramEvent(removeEdgeEvent);
this.sendResponse(payloadSink, changeDescriptionSink, errors, atLeastOneOk, diagramContext, diagramInput);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
import org.eclipse.sirius.components.diagrams.description.DiagramDescription;
import org.eclipse.sirius.components.diagrams.description.EdgeDescription;
import org.eclipse.sirius.components.diagrams.description.NodeDescription;
import org.eclipse.sirius.components.diagrams.events.IDiagramEvent;
import org.eclipse.sirius.components.diagrams.events.RemoveEdgeEvent;
import org.eclipse.sirius.components.diagrams.tests.TestDiagramBuilder;
import org.eclipse.sirius.components.diagrams.tests.TestDiagramDescriptionBuilder;
import org.eclipse.sirius.components.representations.IRepresentationDescription;
Expand Down Expand Up @@ -143,5 +145,10 @@ public void testEdgeSemanticDeletionFromDiagram() {

IPayload payload = payloadSink.asMono().block();
assertThat(payload).isInstanceOf(DeleteFromDiagramSuccessPayload.class);

IDiagramEvent diagramEvent = diagramContext.getDiagramEvent();
assertThat(diagramEvent).isInstanceOf(RemoveEdgeEvent.class);
RemoveEdgeEvent removeEdgeEvent = (RemoveEdgeEvent) diagramEvent;
assertThat(removeEdgeEvent.getEdgeIds()).contains(EDGE_ID);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public void testRemoveEdgeDoesNotAffectUnrelated() {
JsonBasedEditingContext editingContext = new JsonBasedEditingContext(path);

TestDiagramCreationService diagramCreationService = this.createDiagramCreationService(diagram);
Optional<Diagram> optionalRefreshedDiagram = diagramCreationService.performRefresh(editingContext, diagram);
Optional<Diagram> optionalRefreshedDiagram = diagramCreationService.performRefresh(editingContext, diagram, null);
assertThat(optionalRefreshedDiagram).isPresent();
Diagram refreshedDiagram = optionalRefreshedDiagram.get();
assertThat(refreshedDiagram.getNodes()).hasSize(2);
Expand Down Expand Up @@ -195,8 +195,9 @@ public void testSimpleDiagramLayout() throws IOException {
JsonBasedEditingContext editingContext = new JsonBasedEditingContext(path);

TestDiagramCreationService diagramCreationService = this.createDiagramCreationService(diagram);
IDiagramEvent diagramEvent = new SinglePositionEvent(Position.at(300, 100));

Optional<Diagram> optionalRefreshedDiagram = diagramCreationService.performRefresh(editingContext, diagram);
Optional<Diagram> optionalRefreshedDiagram = diagramCreationService.performRefresh(editingContext, diagram, diagramEvent);
assertThat(optionalRefreshedDiagram).isNotEmpty();
Diagram refreshedDiagram = optionalRefreshedDiagram.get();

Expand Down Expand Up @@ -228,7 +229,6 @@ public void testSimpleDiagramLayout() throws IOException {
assertThat(secondToFirstEdge.getTargetAnchorRelativePosition()).isEqualTo(Ratio.of(0.1, 0.9));
assertThat(secondToFirstEdge.getRoutingPoints()).isEmpty();

IDiagramEvent diagramEvent = new SinglePositionEvent(Position.at(300, 100));
Diagram layoutedDiagram = diagramCreationService.performLayout(editingContext, refreshedDiagram, diagramEvent);

Optional<Node> optionalLayoutedThirdParent = this.getNode(layoutedDiagram.getNodes(), "Third Parent"); //$NON-NLS-1$
Expand Down Expand Up @@ -266,7 +266,8 @@ public void testCreateEdgeDoesNotAffectOtherEdges() {
JsonBasedEditingContext editingContext = new JsonBasedEditingContext(path);

TestDiagramCreationService diagramCreationService = this.createDiagramCreationService(initialLayoutedDiagram);
Optional<Diagram> optionalRefreshedDiagram = diagramCreationService.performRefresh(editingContext, initialLayoutedDiagram);
DoublePositionEvent diagramEvent = new DoublePositionEvent(Position.at(20, 20), Position.at(60, 30));
Optional<Diagram> optionalRefreshedDiagram = diagramCreationService.performRefresh(editingContext, initialLayoutedDiagram, diagramEvent);
assertThat(optionalRefreshedDiagram).isPresent();
Diagram refreshedDiagram = optionalRefreshedDiagram.get();
assertThat(refreshedDiagram.getEdges()).hasSize(2);
Expand All @@ -275,7 +276,7 @@ public void testCreateEdgeDoesNotAffectOtherEdges() {
assertThat(refreshedDiagram.getEdges().get(1).getSourceId()).isEqualTo(first.get().getId());
assertThat(refreshedDiagram.getEdges().get(1).getTargetId()).isEqualTo(first.get().getId());

Diagram layoutedDiagram = diagramCreationService.performLayout(editingContext, refreshedDiagram, new DoublePositionEvent(Position.at(20, 20), Position.at(60, 30)));
Diagram layoutedDiagram = diagramCreationService.performLayout(editingContext, refreshedDiagram, diagramEvent);
assertThat(layoutedDiagram.getEdges().get(1).getSourceAnchorRelativePosition().getX()).isGreaterThanOrEqualTo(0).isLessThanOrEqualTo(1);
assertThat(layoutedDiagram.getEdges().get(1).getSourceAnchorRelativePosition().getY()).isGreaterThanOrEqualTo(0).isLessThanOrEqualTo(1);
assertThat(layoutedDiagram.getEdges().get(1).getTargetAnchorRelativePosition().getX()).isGreaterThanOrEqualTo(0).isLessThanOrEqualTo(1);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
/*******************************************************************************
* Copyright (c) 2022 Obeo.
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v2.0
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Obeo - initial API and implementation
*******************************************************************************/
package org.eclipse.sirius.components.diagrams.layout.incremental;

import static org.assertj.core.api.Assertions.assertThat;
import static org.eclipse.sirius.components.diagrams.tests.DiagramAssertions.assertThat;

import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;

import org.eclipse.sirius.components.core.api.IEditingContext;
import org.eclipse.sirius.components.core.api.IRepresentationDescriptionSearchService;
import org.eclipse.sirius.components.diagrams.Diagram;
import org.eclipse.sirius.components.diagrams.Edge;
import org.eclipse.sirius.components.diagrams.Ratio;
import org.eclipse.sirius.components.diagrams.description.DiagramDescription;
import org.eclipse.sirius.components.diagrams.events.RemoveEdgeEvent;
import org.eclipse.sirius.components.diagrams.layout.ELKLayoutedDiagramProvider;
import org.eclipse.sirius.components.diagrams.layout.IELKDiagramConverter;
import org.eclipse.sirius.components.diagrams.layout.LayoutConfiguratorRegistry;
import org.eclipse.sirius.components.diagrams.layout.LayoutService;
import org.eclipse.sirius.components.diagrams.layout.incremental.provider.ImageSizeProvider;
import org.eclipse.sirius.components.diagrams.layout.incremental.provider.NodeSizeProvider;
import org.eclipse.sirius.components.diagrams.layout.services.DefaultTestDiagramDescriptionProvider;
import org.eclipse.sirius.components.diagrams.layout.services.TestDiagramCreationService;
import org.eclipse.sirius.components.diagrams.layout.services.TestLayoutObjectService;
import org.eclipse.sirius.components.diagrams.tests.builder.JsonBasedEditingContext;
import org.eclipse.sirius.components.diagrams.tests.builder.TestLayoutDiagramBuilder;
import org.eclipse.sirius.components.representations.IRepresentationDescription;
import org.junit.jupiter.api.Test;

/**
* Used to test the layout of edges.
*
* @author gcoutable
*/
public class EdgeLayoutTests {
private static final Path PATH_TO_EDITING_CONTEXTS = Paths.get("src", "test", "resources", "editing-contexts"); //$NON-NLS-1$//$NON-NLS-2$//$NON-NLS-3$//$NON-NLS-4$

private TestLayoutObjectService objectService = new TestLayoutObjectService();

private DefaultTestDiagramDescriptionProvider defaultTestDiagramDescriptionProvider = new DefaultTestDiagramDescriptionProvider(this.objectService);

private TestDiagramCreationService createDiagramCreationService(Diagram diagram) {
IRepresentationDescriptionSearchService.NoOp representationDescriptionSearchService = new IRepresentationDescriptionSearchService.NoOp() {
@Override
public Optional<IRepresentationDescription> findById(IEditingContext editingContext, String representationDescriptionId) {
DiagramDescription diagramDescription = EdgeLayoutTests.this.defaultTestDiagramDescriptionProvider.getDefaultDiagramDescription(diagram);
return Optional.of(diagramDescription);
}
};

NodeSizeProvider nodeSizeProvider = new NodeSizeProvider(new ImageSizeProvider());
IncrementalLayoutEngine incrementalLayoutEngine = new IncrementalLayoutEngine(nodeSizeProvider);

LayoutService layoutService = new LayoutService(new IELKDiagramConverter.NoOp(), new IncrementalLayoutDiagramConverter(), new LayoutConfiguratorRegistry(List.of()),
new ELKLayoutedDiagramProvider(), new IncrementalLayoutedDiagramProvider(), representationDescriptionSearchService, incrementalLayoutEngine);

return new TestDiagramCreationService(this.objectService, representationDescriptionSearchService, layoutService);
}

@Test
public void testRemoveEdgesBetweenTwoSameElements() {
String firstNode = "First"; //$NON-NLS-1$
String secondNode = "Second"; //$NON-NLS-1$

// @formatter:off
Diagram diagram = TestLayoutDiagramBuilder.diagram("Root") //$NON-NLS-1$
.nodes()
.rectangleNode(firstNode).at(10, 10).of(40, 210).and()
.rectangleNode(secondNode).at(100, 10).of(40, 210).and()
.and()
.edge("E1 - removed") //$NON-NLS-1$
.from(firstNode).at(0.75, 1.0 / 7.0)
.to(secondNode).at(0.25, 1.0 / 7.0)
.and()
.edge("E2") //$NON-NLS-1$
.from(firstNode).at(0.75, 2.0 / 7.0)
.to(secondNode).at(0.25, 2.0 / 7.0)
.and()
.edge("E3 - removed") //$NON-NLS-1$
.from(firstNode).at(0.75, 3.0 / 7.0)
.to(secondNode).at(0.25, 3.0 / 7.0)
.and()
.edge("E4") //$NON-NLS-1$
.from(firstNode).at(0.75, 4.0 / 7.0)
.to(secondNode).at(0.25, 4.0 / 7.0)
.and()
.edge("E5 - removed") //$NON-NLS-1$
.from(firstNode).at(0.75, 5.0 / 7.0)
.to(secondNode).at(0.25, 5.0 / 7.0)
.and()
.edge("E6") //$NON-NLS-1$
.from(firstNode).at(0.75, 6.0 / 7.0)
.to(secondNode).at(0.25, 6.0 / 7.0)
.and()
.build();
// @formatter:on

Path path = Paths.get(PATH_TO_EDITING_CONTEXTS.toString(), "testRemoveEdgeWithMultipleEdgeBetweenSameElement"); //$NON-NLS-1$
JsonBasedEditingContext editingContext = new JsonBasedEditingContext(path);

List<String> removedEdgeIds = new ArrayList<>();
// @formatter:off
diagram.getEdges().stream()
.filter(edge -> {
return edge.getCenterLabel().getText().contains(" - removed"); //$NON-NLS-1$
})
.map(Edge::getId)
.forEach(removedEdgeIds::add);
// @formatter:on
RemoveEdgeEvent removeEdgeEvent = new RemoveEdgeEvent(removedEdgeIds);

TestDiagramCreationService diagramCreationService = this.createDiagramCreationService(diagram);
Optional<Diagram> optionalRefreshedDiagram = diagramCreationService.performRefresh(editingContext, diagram, removeEdgeEvent);
assertThat(optionalRefreshedDiagram).isPresent();
Diagram refreshedDiagram = optionalRefreshedDiagram.get();
List<Edge> refreshedEdges = refreshedDiagram.getEdges();
assertThat(refreshedEdges).hasSize(3);

// Each remaining edge has the id for index 0, 1 and 2 but keeps its position
// @formatter:off
assertThat(refreshedEdges.get(0))
.hasId(diagram.getEdges().get(0).getId())
.hasSourceAnchorRelativePositionRatio(Ratio.of(0.75, 2.0 / 7.0))
.hasTargetAnchorRelativePositionRatio(Ratio.of(0.25, 2.0 / 7.0));
assertThat(refreshedEdges.get(1))
.hasId(diagram.getEdges().get(1).getId())
.hasSourceAnchorRelativePositionRatio(Ratio.of(0.75, 4.0 / 7.0))
.hasTargetAnchorRelativePositionRatio(Ratio.of(0.25, 4.0 / 7.0));
assertThat(refreshedEdges.get(2))
.hasId(diagram.getEdges().get(2).getId())
.hasSourceAnchorRelativePositionRatio(Ratio.of(0.75, 6.0 / 7.0))
.hasTargetAnchorRelativePositionRatio(Ratio.of(0.25, 6.0 / 7.0));
// @formatter:on
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -255,12 +255,12 @@ private Diagram createParentResizedDiagram() {

private Diagram createNewNode(Diagram diagram, IEditingContext editingContext, Position eventCreationPosition) {
TestDiagramCreationService diagramCreationService = this.createDiagramCreationService(diagram);
IDiagramEvent diagramEvent = new SinglePositionEvent(eventCreationPosition);

Optional<Diagram> optionalRefreshedDiagram = diagramCreationService.performRefresh(editingContext, diagram);
Optional<Diagram> optionalRefreshedDiagram = diagramCreationService.performRefresh(editingContext, diagram, diagramEvent);
assertThat(optionalRefreshedDiagram).isNotEmpty();
Diagram refreshedDiagram = optionalRefreshedDiagram.get();

IDiagramEvent diagramEvent = new SinglePositionEvent(eventCreationPosition);
return diagramCreationService.performLayout(editingContext, refreshedDiagram, diagramEvent);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public TestDiagramCreationService(IObjectService objectService, IRepresentationD
this.layoutService = Objects.requireNonNull(layoutService);
}

public Optional<Diagram> performRefresh(IEditingContext editingContext, Diagram previousDiagram) {
public Optional<Diagram> performRefresh(IEditingContext editingContext, Diagram previousDiagram, IDiagramEvent diagramEvent) {
var optionalObject = this.objectService.getObject(editingContext, previousDiagram.getTargetObjectId());

// @formatter:off
Expand All @@ -62,14 +62,15 @@ public Optional<Diagram> performRefresh(IEditingContext editingContext, Diagram
if (optionalObject.isPresent() && optionalDiagramDescription.isPresent()) {
Object object = optionalObject.get();
DiagramDescription diagramDescription = optionalDiagramDescription.get();
Diagram refreshedDiagram = this.doRefresh(previousDiagram.getLabel(), object, editingContext, diagramDescription, Optional.of(previousDiagram));
Diagram refreshedDiagram = this.doRefresh(previousDiagram.getLabel(), object, editingContext, diagramDescription, Optional.of(previousDiagram), Optional.ofNullable(diagramEvent));
return Optional.of(refreshedDiagram);

}
return Optional.empty();
}

private Diagram doRefresh(String label, Object targetObject, IEditingContext editingContext, DiagramDescription diagramDescription, Optional<Diagram> optionalPreviousDiagram) {
private Diagram doRefresh(String label, Object targetObject, IEditingContext editingContext, DiagramDescription diagramDescription, Optional<Diagram> optionalPreviousDiagram,
Optional<IDiagramEvent> optionalDiagramEvent) {
VariableManager variableManager = new VariableManager();
variableManager.put(DiagramDescription.LABEL, label);
variableManager.put(VariableManager.SELF, targetObject);
Expand All @@ -81,7 +82,8 @@ private Diagram doRefresh(String label, Object targetObject, IEditingContext edi
.diagramDescription(diagramDescription)
.viewCreationRequests(List.of())
.viewDeletionRequests(List.of())
.previousDiagram(optionalPreviousDiagram);
.previousDiagram(optionalPreviousDiagram)
.diagramEvent(optionalDiagramEvent);
//@formatter:on

DiagramComponentProps props = builder.build();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
{
"name": "diag:Root",
"children": [
{
"name": "rect:First",
"children": [
{
"name": "edge:Second",
"children": []
},
{
"name": "edge:Second",
"children": []
},
{
"name": "edge:Second",
"children": []
}
]
},
{
"name": "rect:Second",
"children": []
}
]
}
Loading

0 comments on commit 15f6948

Please sign in to comment.