Skip to content

Commit d965540

Browse files
committed
[2048] Do not reveal valued compartments on Add existing elements tool
Bug: #2048 Signed-off-by: Axel RICHARD <axel.richard@obeo.fr>
1 parent 978f686 commit d965540

File tree

14 files changed

+69
-176
lines changed

14 files changed

+69
-176
lines changed

CHANGELOG.adoc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ Now the _end_ keyword is not displayed anymore in the label of these graphical n
101101
- https://github.com/eclipse-syson/syson/issues/2058[#2058] [diagrams] Fix error when importing SysML snippets referencing unknown units which could make the resulting model inconsistent
102102
- https://github.com/eclipse-syson/syson/issues/2053[#2053] [diagrams] Prevent incoming and outgoing graphical edges of a graphical `ForkNode` or `JoinNode` to point empty space.
103103
- https://github.com/eclipse-syson/syson/issues/2059[#2059] [diagrams] Fix an issue where the _Add existing elements_ tool was not working correctly on the _action flow_ compartment of `ActionUsage` graphical nodes.
104+
- https://github.com/eclipse-syson/syson/issues/2048[#2048] [diagrams] Fix an issue where the non-empty compartments of graphical nodes were being displayed when executing the _Add existing elements_ tool.
104105

105106
=== Improvements
106107

backend/application/syson-application/src/test/java/org/eclipse/syson/application/controllers/diagrams/general/view/GVAddExistingElementsTests.java

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,6 @@ private Flux<DiagramRefreshedEventPayload> givenSubscriptionToDiagram() {
104104
return this.givenDiagramSubscription.subscribe(diagramEventInput);
105105
}
106106

107-
108107
@BeforeEach
109108
public void setUp() {
110109
this.givenInitialServerState.initialize();
@@ -123,10 +122,9 @@ public void addExistingElementsOnDiagram() {
123122

124123
Consumer<Object> initialDiagramContentConsumer = assertRefreshedDiagramThat(diagram::set);
125124

126-
String creationToolId = diagramDescriptionIdProvider.getDiagramCreationToolId("Add existing elements");
127-
assertThat(creationToolId).as("The tool 'Add existing elements' should exist on the diagram").isNotNull();
125+
String addExistingElementsToolId = diagramDescriptionIdProvider.getDiagramCreationToolId("Add existing elements");
128126

129-
Runnable nodeCreationRunner = () -> this.nodeCreationTester.invokeTool(GeneralViewAddExistingElementsTestProjectData.EDITING_CONTEXT_ID, diagram, creationToolId);
127+
Runnable nodeCreationRunner = () -> this.nodeCreationTester.invokeTool(GeneralViewAddExistingElementsTestProjectData.EDITING_CONTEXT_ID, diagram, addExistingElementsToolId);
130128

131129
Consumer<Object> updatedDiagramConsumer = assertRefreshedDiagramThat(newDiagram -> {
132130
assertThat(newDiagram.getNodes()).as("3 nodes should be visible on the diagram").hasSize(4);
@@ -147,7 +145,6 @@ public void addExistingElementsOnDiagram() {
147145
.consumeNextWith(updatedDiagramConsumer)
148146
.thenCancel()
149147
.verify(Duration.ofSeconds(10));
150-
151148
}
152149

153150
@GivenSysONServer({ GeneralViewAddExistingElementsTestProjectData.SCRIPT_PATH })
@@ -164,15 +161,13 @@ public void addExistingElementsRecursiveOnDiagram() {
164161
Consumer<Object> initialDiagramContentConsumer = assertRefreshedDiagramThat(diagram::set);
165162

166163
String addExistingElementToolId = diagramDescriptionIdProvider.getDiagramCreationToolId("Add existing elements (recursive)");
167-
assertThat(addExistingElementToolId).as("The tool 'Add existing elements (recursive)' should exist on the diagram").isNotNull();
168164

169165
Runnable addExistingElementTool = () -> this.nodeCreationTester.invokeTool(GeneralViewAddExistingElementsTestProjectData.EDITING_CONTEXT_ID, diagram, addExistingElementToolId);
170166

171167
Consumer<Object> updatedDiagramConsumer = assertRefreshedDiagramThat(newDiagram -> {
172-
assertThat(newDiagram.getNodes()).as("6 nodes should be visible on the diagram").hasSize(7);
168+
assertThat(newDiagram.getNodes()).as("7 root nodes should be visible on the diagram").hasSize(7);
173169
assertThat(newDiagram.getEdges().stream().filter(e -> ViewModifier.Normal.equals(e.getState())).toList())
174-
.as("3 edges should be visible on the diagram")
175-
.hasSize(3)
170+
.as("3 edges should be visible on the diagram").hasSize(3)
176171
.as("The diagram should contain a composite edge between part2 and part1")
177172
.anyMatch(edge -> edge.getTargetObjectLabel().equals(PART1))
178173
.as("The diagram should contain a composite edge between action1 and action2")
@@ -340,16 +335,8 @@ private void checkRequirementUsage(Diagram newDiagram) {
340335

341336
assertThat(optRequirementNode).isPresent();
342337
assertThat(optRequirementNode.get().getChildNodes())
343-
.as("Node RequirementUsage should contain 6 children")
344-
.hasSize(8);
345-
346-
var requirementDocCompartment = this.getCompartment(optRequirementNode.get(), "doc");
347-
assertThat(requirementDocCompartment).isPresent();
348-
assertThat(requirementDocCompartment.get())
349-
.as("The doc compartment should be visible")
350-
.matches(node -> !node.getModifiers().contains(ViewModifier.Hidden))
351-
.as("The doc compartment should contain a document item")
352-
.matches(node -> node.getChildNodes().size() == 1);
338+
.as("Node RequirementUsage should contain 8 hidden compartment children").hasSize(8)
339+
.allMatch(node -> node.getModifiers().contains(ViewModifier.Hidden));
353340
}
354341

355342
private Optional<Node> getCompartment(Node node, String compartmentName) {

backend/application/syson-application/src/test/java/org/eclipse/syson/application/controllers/diagrams/general/view/GVDropFromExplorerVisibilityTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ public void dropPartFromTheExplorer() {
9898
diagramTargetId.set(diagram.getTargetObjectId());
9999
diagramId.set(diagram.getId());
100100
var partNode = new DiagramNavigator(diagram).nodeWithLabel(LabelConstants.OPEN_QUOTE + "part" + LabelConstants.CLOSE_QUOTE + LabelConstants.CR + "p1").getNode();
101-
assertThat(partNode.getChildNodes().stream().filter(node -> node.getModifiers().contains(ViewModifier.Hidden))).hasSize(9);
101+
assertThat(partNode.getChildNodes().stream().filter(node -> node.getModifiers().contains(ViewModifier.Hidden))).hasSize(10);
102102
partNodeSemanticId.set(partNode.getTargetObjectId());
103103
partNodeId.set(partNode.getId());
104104
});

backend/services/syson-diagram-services/src/main/java/org/eclipse/syson/diagram/services/DiagramQueryElementService.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -330,17 +330,17 @@ public Object infoMessage(Object self, String message) {
330330
}
331331

332332
/**
333-
* Checks if the given node represents the given feature. This especially handle can of feature chain that target inherited member. In such case, the two last segments of the chain should be used
334-
* to find the suitable node.
333+
* Checks if the given node represents the given feature. This especially handle can of feature chain that target
334+
* inherited member. In such case, the two last segments of the chain should be used to find the suitable node.
335335
*
336336
* @param endFeature
337-
* the end feature of a {@link org.eclipse.syson.sysml.ConnectorAsUsage}
337+
* the end feature of a {@link org.eclipse.syson.sysml.ConnectorAsUsage}
338338
* @param nodeToTest
339-
* the node to test
339+
* the node to test
340340
* @param cacheRendering
341-
* the current cache rendering
341+
* the current cache rendering
342342
* @param editingContext
343-
* the editing context
343+
* the editing context
344344
* @return true if the given node is valid source/target for the given node
345345
*/
346346
private boolean isValidConnectionEnd(Feature endFeature, org.eclipse.sirius.components.representations.Element nodeToTest, DiagramRenderingCache cacheRendering,

backend/views/syson-diagram-common-view/src/main/java/org/eclipse/syson/diagram/common/view/nodes/AbstractCompartmentNodeDescriptionProvider.java

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import org.eclipse.sirius.components.view.diagram.SynchronizationPolicy;
4040
import org.eclipse.sirius.components.view.diagram.UserResizableDirection;
4141
import org.eclipse.sirius.components.view.emf.diagram.ViewDiagramDescriptionConverter;
42+
import org.eclipse.syson.diagram.common.view.services.ViewNodeService;
4243
import org.eclipse.syson.diagram.common.view.services.description.ToolDescriptionService;
4344
import org.eclipse.syson.diagram.common.view.tools.CompartmentNodeToolProvider;
4445
import org.eclipse.syson.diagram.services.aql.DiagramMutationAQLService;
@@ -100,6 +101,10 @@ public void link(DiagramDescription diagramDescription, IViewDiagramElementFinde
100101
});
101102
}
102103

104+
protected IDescriptionNameGenerator getDescriptionNameGenerator() {
105+
return this.descriptionNameGenerator;
106+
}
107+
103108
/**
104109
* Implementers should provide the list of {@link NodeDescription} that can be dropped inside this compartment
105110
* {@link NodeDescription}.
@@ -140,7 +145,9 @@ protected List<INodeToolProvider> getItemCreationToolProviders() {
140145
* <code>false</code> otherwise.
141146
*/
142147
protected String isHiddenByDefaultExpression() {
143-
return AQLUtils.getSelfServiceCallExpression("isHiddenByDefault", "'" + this.eReference.getName() + "'");
148+
return ServiceMethod.of4(ViewNodeService::isHiddenByDefault).aqlSelf(AQLUtils.aqlString(this.getCompartmentName()),
149+
org.eclipse.sirius.components.diagrams.description.NodeDescription.ANCESTORS,
150+
IEditingContext.EDITING_CONTEXT, DiagramContext.DIAGRAM_CONTEXT);
144151
}
145152

146153
/**
@@ -239,10 +246,6 @@ protected DropNodeTool createCompartmentDropFromDiagramTool(IViewDiagramElementF
239246
.build();
240247
}
241248

242-
protected IDescriptionNameGenerator getDescriptionNameGenerator() {
243-
return this.descriptionNameGenerator;
244-
}
245-
246249
protected String getCompartmentLabel() {
247250
String customLabel = this.getCustomCompartmentLabel();
248251
if (customLabel != null) {

backend/views/syson-diagram-common-view/src/main/java/org/eclipse/syson/diagram/common/view/nodes/ActionFlowCompartmentNodeDescriptionProvider.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public NodeDescription create() {
7272
.defaultWidthExpression(ViewConstants.DEFAULT_NODE_WIDTH)
7373
.domainType(SysMLMetamodelHelper.buildQualifiedName(SysmlPackage.eINSTANCE.getElement()))
7474
.insideLabel(this.createInsideLabelDescription())
75-
.isHiddenByDefaultExpression("aql:true")
75+
.isHiddenByDefaultExpression(this.isHiddenByDefaultExpression())
7676
.name(this.name)
7777
.semanticCandidatesExpression(AQLConstants.AQL_SELF)
7878
.style(this.createCompartmentNodeStyle())
@@ -162,7 +162,7 @@ protected InsideLabelStyle createInsideLabelStyle() {
162162
.fontSize(12)
163163
.italic(true)
164164
.labelColor(this.colorProvider.getColor(ViewConstants.DEFAULT_LABEL_COLOR))
165-
.showIconExpression("aql:false")
165+
.showIconExpression(AQLConstants.AQL_FALSE)
166166
.withHeader(true)
167167
.build();
168168
}

backend/views/syson-diagram-common-view/src/main/java/org/eclipse/syson/diagram/common/view/nodes/InterconnectionCompartmentNodeDescriptionProvider.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,14 @@
3333
import org.eclipse.sirius.components.view.diagram.NodeToolSection;
3434
import org.eclipse.sirius.components.view.diagram.SynchronizationPolicy;
3535
import org.eclipse.sirius.components.view.diagram.UserResizableDirection;
36+
import org.eclipse.syson.diagram.common.view.services.ViewNodeService;
3637
import org.eclipse.syson.diagram.common.view.services.description.ToolConstants;
3738
import org.eclipse.syson.diagram.common.view.tools.ActionFlowCompartmentNodeToolProvider;
3839
import org.eclipse.syson.sysml.SysmlPackage;
3940
import org.eclipse.syson.util.AQLConstants;
4041
import org.eclipse.syson.util.AQLUtils;
4142
import org.eclipse.syson.util.IDescriptionNameGenerator;
43+
import org.eclipse.syson.util.ServiceMethod;
4244
import org.eclipse.syson.util.StandardDiagramsConstants;
4345
import org.eclipse.syson.util.SysMLMetamodelHelper;
4446
import org.eclipse.syson.util.ViewConstants;
@@ -66,6 +68,7 @@ public NodeDescription create() {
6668
.defaultWidthExpression(ViewConstants.DEFAULT_NODE_WIDTH)
6769
.domainType(SysMLMetamodelHelper.buildQualifiedName(SysmlPackage.eINSTANCE.getElement()))
6870
.insideLabel(this.createInsideLabelDescription())
71+
.isHiddenByDefaultExpression(this.isHiddenByDefaultExpression())
6972
.name(this.compartmentName)
7073
.preconditionExpression(
7174
AQLUtils.getSelfServiceCallExpression("isView",
@@ -162,4 +165,10 @@ protected NodeStyleDescription createCompartmentNodeStyle() {
162165
.childrenLayoutStrategy(this.diagramBuilderHelper.newFreeFormLayoutStrategyDescription().build())
163166
.build();
164167
}
168+
169+
@Override
170+
protected String isHiddenByDefaultExpression() {
171+
return ServiceMethod.of4(ViewNodeService::isHiddenByDefault).aqlSelf(AQLUtils.aqlString(this.compartmentName), org.eclipse.sirius.components.diagrams.description.NodeDescription.ANCESTORS,
172+
IEditingContext.EDITING_CONTEXT, DiagramContext.DIAGRAM_CONTEXT);
173+
}
165174
}

backend/views/syson-diagram-common-view/src/main/java/org/eclipse/syson/diagram/common/view/nodes/StateTransitionCompartmentNodeDescriptionProvider.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2024, 2025 Obeo.
2+
* Copyright (c) 2024, 2026 Obeo.
33
* This program and the accompanying materials
44
* are made available under the terms of the Eclipse Public License v2.0
55
* which accompanies this distribution, and is available at
@@ -106,7 +106,7 @@ protected InsideLabelStyle createInsideLabelStyle() {
106106
.fontSize(12)
107107
.italic(true)
108108
.labelColor(this.colorProvider.getColor(ViewConstants.DEFAULT_LABEL_COLOR))
109-
.showIconExpression("aql:false")
109+
.showIconExpression(AQLConstants.AQL_FALSE)
110110
.withHeader(true)
111111
.build();
112112
}

backend/views/syson-diagram-common-view/src/main/java/org/eclipse/syson/diagram/common/view/services/ViewNodeService.java

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import org.eclipse.emf.common.util.EList;
2525
import org.eclipse.emf.ecore.EClass;
2626
import org.eclipse.emf.ecore.EObject;
27-
import org.eclipse.emf.ecore.EStructuralFeature;
2827
import org.eclipse.sirius.components.collaborative.diagrams.DiagramContext;
2928
import org.eclipse.sirius.components.collaborative.diagrams.DiagramService;
3029
import org.eclipse.sirius.components.collaborative.diagrams.DiagramServices;
@@ -315,41 +314,38 @@ public Node revealCompartment(Node node, Element targetElement, DiagramContext d
315314
}
316315

317316
/**
318-
* Check if the compartment associated to the given {@link Element} and reference should be hidden by default.
317+
* Check if the compartment associated to the given {@link Element} should be hidden by default.
319318
*
320319
* @param self
321320
* the {@link Element} associated to the compartment node.
322-
* @param referenceName
323-
* the name of the reference associated to the compartment node.
321+
* @param compartmentName
322+
* the name of the compartment to display/hide by default
323+
* @param ancestors
324+
* the list of ancestors of the element (semantic elements corresponding to graphical ancestors). It
325+
* corresponds to a variable accessible from the variable manager.
326+
* @param editingContext
327+
* the {@link IEditingContext} of the element. It corresponds to a variable accessible from the variable
328+
* manager.
329+
* @param diagramContext
330+
* the {@link DiagramContext} of the element. It corresponds to a variable accessible from the variable
331+
* manager.
324332
* @return <code>true</code> if the compartment should be hidden by default, <code>false</code> otherwise
325333
*/
326-
public boolean isHiddenByDefault(Element self, String referenceName) {
334+
public boolean isHiddenByDefault(Element self, String compartmentName, List<Object> ancestors, IEditingContext editingContext, DiagramContext diagramContext) {
327335
boolean isHiddenByDefault = true;
328-
EStructuralFeature eStructuralFeature = self.eClass().getEStructuralFeature(referenceName);
329-
if (eStructuralFeature != null && eStructuralFeature != SysmlPackage.eINSTANCE.getDefinition_OwnedPort()) {
330-
Object referenceValue = self.eGet(eStructuralFeature);
331-
if (referenceValue instanceof List<?> referenceListValue) {
332-
isHiddenByDefault = referenceListValue.isEmpty();
333-
} else {
334-
isHiddenByDefault = referenceValue == null;
336+
// @technical-debt we should find a way to compute this compartment name here and in
337+
// InterconnectionCompartmentNodeDescriptionProvider only once.
338+
if ("GV Compartment interconnection FreeForm".equals(compartmentName)) {
339+
var exposedParts = this.getExposedElements(self, SysmlPackage.eINSTANCE.getPartUsage(), ancestors, editingContext, diagramContext);
340+
var exposedActions = this.getExposedElements(self, SysmlPackage.eINSTANCE.getActionUsage(), ancestors, editingContext, diagramContext);
341+
var exposedSatisfyRequirements = this.getExposedElements(self, SysmlPackage.eINSTANCE.getSatisfyRequirementUsage(), ancestors, editingContext, diagramContext);
342+
if (!exposedParts.isEmpty() || !exposedActions.isEmpty() || !exposedSatisfyRequirements.isEmpty()) {
343+
isHiddenByDefault = false;
335344
}
336345
}
337346
return isHiddenByDefault;
338347
}
339348

340-
/**
341-
* Check if the compartment associated to the given {@link Element} and references should be hidden by default.
342-
*
343-
* @param self
344-
* the {@link Element} associated to the compartment node.
345-
* @param referenceNames
346-
* the name of the references associated to the compartment node.
347-
* @return <code>true</code> if the compartment should be hidden by default, <code>false</code> otherwise
348-
*/
349-
public boolean isHiddenByDefault(Element self, List<String> referenceNames) {
350-
return referenceNames.stream().allMatch(referenceName -> this.isHiddenByDefault(self, referenceName));
351-
}
352-
353349
/**
354350
* Returns {@code true} if {@code parentNodeElement} is an ancestor of {@code childNodeElement}.
355351
* <p>

backend/views/syson-standard-diagrams-view/src/main/java/org/eclipse/syson/standard/diagrams/view/nodes/AllocationDefinitionEndsCompartmentNodeDescriptionProvider.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.eclipse.syson.diagram.common.view.nodes.AbstractCompartmentNodeDescriptionProvider;
2323
import org.eclipse.syson.diagram.common.view.tools.ConnectionDefinitionEndCompartmentNodeToolProvider;
2424
import org.eclipse.syson.sysml.SysmlPackage;
25+
import org.eclipse.syson.util.AQLConstants;
2526
import org.eclipse.syson.util.IDescriptionNameGenerator;
2627

2728
/**
@@ -51,4 +52,9 @@ protected List<INodeToolProvider> getItemCreationToolProviders() {
5152
creationToolProviders.add(new ConnectionDefinitionEndCompartmentNodeToolProvider());
5253
return creationToolProviders;
5354
}
55+
56+
@Override
57+
protected String isHiddenByDefaultExpression() {
58+
return AQLConstants.AQL_FALSE;
59+
}
5460
}

0 commit comments

Comments
 (0)