From 52bc6c7e167050f2bbf241fee2859daad7c128d3 Mon Sep 17 00:00:00 2001 From: Axel RICHARD Date: Thu, 2 Oct 2025 11:05:38 +0200 Subject: [PATCH] [1567] Fix Sub-parts are not displayed inside Package nodes Bug: https://github.com/eclipse-syson/syson/issues/1567 Signed-off-by: Axel RICHARD --- CHANGELOG.adoc | 2 + .../general/view/GVDropFromDiagramTests.java | 2 +- .../diagrams/general/view/GVPackageTests.java | 161 ++++++++++++++++++ backend/releng/syson-test-coverage/pom.xml | 5 + .../view/services/ViewFilterSwitch.java | 56 ++++-- .../common/view/services/ViewNodeService.java | 16 +- .../pages/release-notes/2025.10.0.adoc | 2 + 7 files changed, 231 insertions(+), 13 deletions(-) create mode 100644 backend/application/syson-application/src/test/java/org/eclipse/syson/application/controllers/diagrams/general/view/GVPackageTests.java diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 8a1b1a412..dd244fc90 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -85,6 +85,8 @@ Now it has been fixed in Sirius Web 2025.8.4, `SysONUpdateLibraryExecutor` has b - https://github.com/eclipse-syson/syson/issues/1546[#1546] [diagrams] Fix drag and drop of a graphical node from the diagram background into a _Package_ graphical node. - https://github.com/eclipse-syson/syson/issues/1550[#1550] [diagrams] Fix an issue where the _action flow_ compartment was not revealed when the _New Start Action_ was executed. - https://github.com/eclipse-syson/syson/issues/1547[#1547] [diagrams] Fix an issue where the _Delete from diagram_ action did nothing on elements inside a _Package_ in diagrams. +- https://github.com/eclipse-syson/syson/issues/1567[#1567] [diagrams] Fix an issue where a `PartUsage` contained in a `PartUsage` contained in a `Package` was not displayed, even though it was exposed in the `ViewUsage` corresponding to the diagram. +`PartUsage` is just one example, but the same issue could appear with any other `Element` contained in `Element` contained in a `Package`. === Improvements diff --git a/backend/application/syson-application/src/test/java/org/eclipse/syson/application/controllers/diagrams/general/view/GVDropFromDiagramTests.java b/backend/application/syson-application/src/test/java/org/eclipse/syson/application/controllers/diagrams/general/view/GVDropFromDiagramTests.java index bb8b3c201..f05c32168 100644 --- a/backend/application/syson-application/src/test/java/org/eclipse/syson/application/controllers/diagrams/general/view/GVDropFromDiagramTests.java +++ b/backend/application/syson-application/src/test/java/org/eclipse/syson/application/controllers/diagrams/general/view/GVDropFromDiagramTests.java @@ -75,7 +75,7 @@ public void beforeEach() { this.givenInitialServerState.initialize(); } - @DisplayName("Given a diagram with some nodes, when a node is dropped in another one, then the diagram is updated") + @DisplayName("GIVEN a diagram with some nodes, WHEN a node is dropped in another one, THEN the diagram is updated") @Sql(scripts = { GeneralViewWithTopNodesTestProjectData.SCRIPT_PATH }, executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD, config = @SqlConfig(transactionMode = SqlConfig.TransactionMode.ISOLATED)) @Sql(scripts = { "/scripts/cleanup.sql" }, executionPhase = Sql.ExecutionPhase.AFTER_TEST_METHOD, config = @SqlConfig(transactionMode = SqlConfig.TransactionMode.ISOLATED)) diff --git a/backend/application/syson-application/src/test/java/org/eclipse/syson/application/controllers/diagrams/general/view/GVPackageTests.java b/backend/application/syson-application/src/test/java/org/eclipse/syson/application/controllers/diagrams/general/view/GVPackageTests.java new file mode 100644 index 000000000..53ea8fd21 --- /dev/null +++ b/backend/application/syson-application/src/test/java/org/eclipse/syson/application/controllers/diagrams/general/view/GVPackageTests.java @@ -0,0 +1,161 @@ +/******************************************************************************* + * Copyright (c) 2025 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.syson.application.controllers.diagrams.general.view; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.eclipse.sirius.components.diagrams.tests.DiagramEventPayloadConsumer.assertRefreshedDiagramThat; + +import java.time.Duration; +import java.util.List; +import java.util.UUID; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Consumer; + +import org.eclipse.sirius.components.collaborative.diagrams.dto.DiagramEventInput; +import org.eclipse.sirius.components.collaborative.diagrams.dto.DiagramRefreshedEventPayload; +import org.eclipse.sirius.components.diagrams.tests.navigation.DiagramNavigator; +import org.eclipse.sirius.components.view.emf.diagram.IDiagramIdProvider; +import org.eclipse.sirius.web.tests.services.api.IGivenInitialServerState; +import org.eclipse.syson.AbstractIntegrationTests; +import org.eclipse.syson.application.controllers.diagrams.testers.ToolTester; +import org.eclipse.syson.application.data.GeneralViewWithTopNodesTestProjectData; +import org.eclipse.syson.services.diagrams.DiagramDescriptionIdProvider; +import org.eclipse.syson.services.diagrams.api.IGivenDiagramDescription; +import org.eclipse.syson.services.diagrams.api.IGivenDiagramSubscription; +import org.eclipse.syson.standard.diagrams.view.SDVDescriptionNameGenerator; +import org.eclipse.syson.sysml.SysmlPackage; +import org.eclipse.syson.util.IDescriptionNameGenerator; +import org.eclipse.syson.util.SysONRepresentationDescriptionIdentifiers; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.jdbc.Sql; +import org.springframework.test.context.jdbc.SqlConfig; +import org.springframework.transaction.annotation.Transactional; + +import reactor.core.publisher.Flux; +import reactor.test.StepVerifier; + +/** + * Tests the nodes inside the a PAckage nodes in the General View diagram. + * + * @author arichard + */ +@Transactional +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) +public class GVPackageTests extends AbstractIntegrationTests { + + @Autowired + private IGivenInitialServerState givenInitialServerState; + + @Autowired + private IGivenDiagramSubscription givenDiagramSubscription; + + @Autowired + private IGivenDiagramDescription givenDiagramDescription; + + @Autowired + private IDiagramIdProvider diagramIdProvider; + + @Autowired + private ToolTester toolTester; + + private final IDescriptionNameGenerator descriptionNameGenerator = new SDVDescriptionNameGenerator(); + + private Flux givenSubscriptionToDiagram() { + var diagramEventInput = new DiagramEventInput(UUID.randomUUID(), + GeneralViewWithTopNodesTestProjectData.EDITING_CONTEXT_ID, + GeneralViewWithTopNodesTestProjectData.GraphicalIds.DIAGRAM_ID); + var flux = this.givenDiagramSubscription.subscribe(diagramEventInput); + return flux; + } + + @BeforeEach + public void beforeEach() { + this.givenInitialServerState.initialize(); + } + + @DisplayName("GIVEN a diagram with a Package node, WHEN a Part node and a Sub-Part node are created, THEN the Part node and the Sub-Part node are visible inside the Package, on the same level, with an edge between them") + @Sql(scripts = { GeneralViewWithTopNodesTestProjectData.SCRIPT_PATH }, executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD, + config = @SqlConfig(transactionMode = SqlConfig.TransactionMode.ISOLATED)) + @Sql(scripts = { "/scripts/cleanup.sql" }, executionPhase = Sql.ExecutionPhase.AFTER_TEST_METHOD, config = @SqlConfig(transactionMode = SqlConfig.TransactionMode.ISOLATED)) + @Test + public void testCreateSubPartInPackage() { + var flux = this.givenSubscriptionToDiagram(); + + var diagramDescription = this.givenDiagramDescription.getDiagramDescription(GeneralViewWithTopNodesTestProjectData.EDITING_CONTEXT_ID, + SysONRepresentationDescriptionIdentifiers.GENERAL_VIEW_DIAGRAM_DESCRIPTION_ID); + var diagramDescriptionIdProvider = new DiagramDescriptionIdProvider(diagramDescription, this.diagramIdProvider); + + var newPartToolId = diagramDescriptionIdProvider.getNodeCreationToolId(this.descriptionNameGenerator.getNodeName(SysmlPackage.eINSTANCE.getPackage()), "New Part"); + assertThat(newPartToolId).as("The tool 'New Part' should exist on the Package").isNotNull(); + + var newSubPartToolId = diagramDescriptionIdProvider.getNodeCreationToolId(this.descriptionNameGenerator.getNodeName(SysmlPackage.eINSTANCE.getPartUsage()), "New Part"); + assertThat(newPartToolId).as("The tool 'New Part' should exist on the PartUsage").isNotNull(); + + var diagramId = new AtomicReference(); + var packageNodeId = new AtomicReference(); + var partNodeId = new AtomicReference(); + + Consumer initialDiagramContentConsumer = assertRefreshedDiagramThat(diag -> { + diagramId.set(diag.getId()); + + var packageNode = new DiagramNavigator(diag).nodeWithLabel("Package").getNode(); + packageNodeId.set(packageNode.getId()); + + assertThat(packageNode.getChildNodes()).hasSize(0); + + assertThat(new DiagramNavigator(diag).findDiagramEdgeCount()).isEqualTo(3); + + }); + + Runnable newPartUsageTool = () -> this.toolTester.invokeTool(GeneralViewWithTopNodesTestProjectData.EDITING_CONTEXT_ID, diagramId.get(), packageNodeId.get(), newPartToolId, + List.of()); + + Consumer updatedDiagramContentConsumerAfterNewPart = assertRefreshedDiagramThat(diag -> { + var packageNode = new DiagramNavigator(diag).nodeWithLabel("Package").getNode(); + + var partNode = new DiagramNavigator(diag).nodeWithLabel("Package").childNodeWithLabel("\u00ABpart\u00BB\npart1").getNode(); + partNodeId.set(partNode.getId()); + + assertThat(packageNode.getChildNodes()).hasSize(1); + assertThat(packageNode.getChildNodes().get(0)).isEqualTo(partNode); + + assertThat(new DiagramNavigator(diag).findDiagramEdgeCount()).isEqualTo(3); + + }); + + Runnable newSubPartUsageTool = () -> this.toolTester.invokeTool(GeneralViewWithTopNodesTestProjectData.EDITING_CONTEXT_ID, diagramId.get(), partNodeId.get(), newSubPartToolId, + List.of()); + + Consumer updatedDiagramContentConsumerAfterNewSubPart = assertRefreshedDiagramThat(diag -> { + var packageNode = new DiagramNavigator(diag).nodeWithLabel("Package").getNode(); + + assertThat(packageNode.getChildNodes()).hasSize(2); + + // a new edge exists between the Part and the sub-Part inside the Package node + assertThat(new DiagramNavigator(diag).findDiagramEdgeCount()).isEqualTo(4); + }); + + StepVerifier.create(flux) + .consumeNextWith(initialDiagramContentConsumer) + .then(newPartUsageTool) + .consumeNextWith(updatedDiagramContentConsumerAfterNewPart) + .then(newSubPartUsageTool) + .consumeNextWith(updatedDiagramContentConsumerAfterNewSubPart) + .thenCancel() + .verify(Duration.ofSeconds(10)); + } +} diff --git a/backend/releng/syson-test-coverage/pom.xml b/backend/releng/syson-test-coverage/pom.xml index 5bec0e337..d7a3bb8f9 100644 --- a/backend/releng/syson-test-coverage/pom.xml +++ b/backend/releng/syson-test-coverage/pom.xml @@ -90,6 +90,11 @@ syson-sysml-validation 2025.8.9 + + org.eclipse.syson + syson-common-view + 2025.8.9 + org.eclipse.syson syson-diagram-common-view diff --git a/backend/views/syson-diagram-common-view/src/main/java/org/eclipse/syson/diagram/common/view/services/ViewFilterSwitch.java b/backend/views/syson-diagram-common-view/src/main/java/org/eclipse/syson/diagram/common/view/services/ViewFilterSwitch.java index ad281bed8..768b2fda0 100644 --- a/backend/views/syson-diagram-common-view/src/main/java/org/eclipse/syson/diagram/common/view/services/ViewFilterSwitch.java +++ b/backend/views/syson-diagram-common-view/src/main/java/org/eclipse/syson/diagram/common/view/services/ViewFilterSwitch.java @@ -22,6 +22,7 @@ import org.eclipse.syson.sysml.AttributeUsage; import org.eclipse.syson.sysml.Documentation; import org.eclipse.syson.sysml.Element; +import org.eclipse.syson.sysml.Namespace; import org.eclipse.syson.sysml.NamespaceImport; import org.eclipse.syson.sysml.Package; import org.eclipse.syson.sysml.PortUsage; @@ -75,13 +76,13 @@ public ViewFilterSwitch(ViewDefinitionKind kind, List exposedElements, public Boolean defaultCase(EObject object) { Boolean displayAsTreeNode = Boolean.FALSE; if (ViewDefinitionKind.isGeneralView(this.kind)) { - displayAsTreeNode = Boolean.TRUE; + displayAsTreeNode = this.parentElement == null || (this.isDirectNestedNode(object)); } else if (ViewDefinitionKind.isActionFlowView(this.kind)) { displayAsTreeNode = object instanceof ActionUsage || object instanceof ActionDefinition; } else if (ViewDefinitionKind.isStateTransitionView(this.kind)) { displayAsTreeNode = object instanceof StateUsage || object instanceof StateDefinition; } else { - displayAsTreeNode = !this.isNestedNode(object); + displayAsTreeNode = !this.isIndirectNestedNode(object); } return displayAsTreeNode; } @@ -90,7 +91,7 @@ public Boolean defaultCase(EObject object) { public Boolean caseAttributeUsage(AttributeUsage object) { // For AttributeUsages we don't want nested Nodes, no matter the type of ViewDefinition. // The sub AttributeUsages are displayed in a compartment list called "attributes". - return !this.isNestedNode(object) && !ViewDefinitionKind.isActionFlowView(this.kind) && !ViewDefinitionKind.isStateTransitionView(this.kind); + return !this.isIndirectNestedNode(object) && !ViewDefinitionKind.isActionFlowView(this.kind) && !ViewDefinitionKind.isStateTransitionView(this.kind); } @Override @@ -103,18 +104,51 @@ public Boolean caseDocumentation(Documentation object) { public Boolean casePortUsage(PortUsage object) { // For PortUsages we don't want nested Nodes, no matter the type of ViewDefinition. // The sub PortUsages are displayed in a compartment list called "ports" and/or as border nodes. - return !this.isNestedNode(object) && !ViewDefinitionKind.isActionFlowView(this.kind) && !ViewDefinitionKind.isStateTransitionView(this.kind); + return !this.isIndirectNestedNode(object) && !ViewDefinitionKind.isActionFlowView(this.kind) && !ViewDefinitionKind.isStateTransitionView(this.kind); } - private boolean isNestedNode(EObject object) { - boolean isNestedNode = false; - for (Element exposedElement : this.exposedElements) { - isNestedNode = !Objects.equals(exposedElement, object) && (this.parentElement == null && EMFUtils.isAncestor(exposedElement, object)); - if (isNestedNode) { - break; + /** + * Check if the given object is an indirect nested node of the parentElement of this class. + * + * @param object + * the given object + * @return true if the given object is an indirect node of the parentElement of this class, + * false otherwise. + */ + private boolean isIndirectNestedNode(EObject object) { + boolean isDirectNestedNode = false; + if (this.parentElement instanceof Namespace elt && !elt.getOwnedMember().contains(object)) { + isDirectNestedNode = true; + } else { + for (Element exposedElement : this.exposedElements) { + if (Objects.equals(exposedElement, object)) { + continue; + } else if (this.parentElement == null && EMFUtils.isAncestor(exposedElement, object)) { + isDirectNestedNode = true; + } + if (isDirectNestedNode) { + break; + } } } - return isNestedNode; + return isDirectNestedNode; } + /** + * Check if the given object is a direct nested node of the parentElement of this class. + * + * @param object + * the given object + * @return true if the given object is a direct nested node of the parentElement of this class, + * false otherwise. + */ + private boolean isDirectNestedNode(EObject object) { + boolean isDirectNestedNode = false; + if (this.parentElement instanceof Namespace elt && elt.getOwnedMember().contains(object)) { + isDirectNestedNode = true; + } else if (this.parentElement instanceof Package) { + isDirectNestedNode = true; + } + return isDirectNestedNode; + } } diff --git a/backend/views/syson-diagram-common-view/src/main/java/org/eclipse/syson/diagram/common/view/services/ViewNodeService.java b/backend/views/syson-diagram-common-view/src/main/java/org/eclipse/syson/diagram/common/view/services/ViewNodeService.java index 7c40b35b2..6e1f258ea 100644 --- a/backend/views/syson-diagram-common-view/src/main/java/org/eclipse/syson/diagram/common/view/services/ViewNodeService.java +++ b/backend/views/syson-diagram-common-view/src/main/java/org/eclipse/syson/diagram/common/view/services/ViewNodeService.java @@ -137,7 +137,7 @@ public List getExposedElements(Element element, Element parent, EClass exposedElements.addAll(viewUsage.getExposedElement()); } var filteredExposedElements = exposedElements.stream() - .filter(elt -> elt != null && Objects.equals(elt.eClass(), domainType) && (parent == null || parent.getOwnedElement().contains(elt))) + .filter(elt -> this.isTypeOf(elt, domainType) && (parent == null || (!Objects.equals(parent, elt) && EMFUtils.isAncestor(parent, elt)))) .toList(); elementsToExpose.addAll(filteredExposedElements); // if it is not a General View, we don't want to display nested nodes as tree (i.e. sibling nodes + @@ -686,4 +686,18 @@ protected List getAncestors(Node selectedNode, Diagram diagram, IEditing } return ancestors; } + + /** + * Check if the given {@link Element}'s {@link EClass} is the same than the given domainType. + * + * @param element + * the given {@link Element}. + * @param domainType + * the given domainType as an {@link EClass}. + * @return true if the given {@link Element}'s {@link EClass} is the same than the given domainType, + * false otherwise. + */ + protected boolean isTypeOf(Element element, EClass domainType) { + return element != null && Objects.equals(element.eClass(), domainType); + } } diff --git a/doc/content/modules/user-manual/pages/release-notes/2025.10.0.adoc b/doc/content/modules/user-manual/pages/release-notes/2025.10.0.adoc index b94140200..3958078ab 100644 --- a/doc/content/modules/user-manual/pages/release-notes/2025.10.0.adoc +++ b/doc/content/modules/user-manual/pages/release-notes/2025.10.0.adoc @@ -76,6 +76,8 @@ Now it is only available on SysML model elements. - Fix drag and drop of a graphical node from the diagram background into a _Package_ graphical node. - Fix an issue where the _action flow_ compartment was not revealed when the _New Start Action_ was executed. - Fix an issue where the _Delete from diagram_ action did nothing on elements inside a _Package_ in diagrams. +- Fix an issue where a `PartUsage` contained in a `PartUsage` contained in a `Package` was not displayed, even though it was exposed in the `ViewUsage` corresponding to the diagram. +`PartUsage` is just one example, but the same issue could appear with any other `Element` contained in `Element` contained in a `Package`. == Improvements