[1048] Improve TransitionUsage textual import with implicit source

Bug: #1048
Signed-off-by: Arthur Daussy <arthur.daussy@obeo.fr>

Author: adaussy

CHANGELOG.adoc

@@ -21,7 +21,7 @@
 - https://github.com/eclipse-syson/syson/issues/1083[#1083] [metamodel] Fix an issue where resolving against "unrestricted" name did not work
 - https://github.com/eclipse-syson/syson/issues/1075[#1075] [import] Fix a ClassCastException thrown while importing a model with a name conflict.
 - https://github.com/eclipse-syson/syson/issues/1091[#1091] [export] Fix `FeatureReferenceExpression` export for elements that should be exported as qualified names.
-Exporting a SysML model containing a `FeatureReferenceExpression` now correctly procudes qualified names where it should.
+Exporting a SysML model containing a `FeatureReferenceExpression` now correctly produces qualified names where it should.
 
 === Improvements
 
@@ -39,6 +39,7 @@ A new dra & drop tool is available on the diagram, allowing moving `Satisfy Requ
 - https://github.com/eclipse-syson/syson/issues/1042[#1042] [import] Improve textual import of `SuccessionAsUsage` which define a new target action directly after the 'then' keyword.
 - https://github.com/eclipse-syson/syson/issues/1045[#1045] [export] Improve textual export by properly handle named `SuccessionAsUsage`.
 - https://github.com/eclipse-syson/syson/issues/1080[#1080] [import] Improve textual import and resolution against `ConjugatedPorts`.
+- https://github.com/eclipse-syson/syson/issues/1048[#1048] [import] Improve textual import of `TransitionUsage` by handling _implicit_ source property.
 
 === New features
 

backend/application/syson-application/src/test/java/org/eclipse/syson/application/imports/ImportSysMLModelTest.java

@@ -25,6 +25,7 @@
 import org.eclipse.syson.AbstractIntegrationTests;
 import org.eclipse.syson.application.configuration.SysMLEditingContextProcessor;
 import org.eclipse.syson.services.UtilService;
+import org.eclipse.syson.sysml.ActionUsage;
 import org.eclipse.syson.sysml.ConjugatedPortDefinition;
 import org.eclipse.syson.sysml.Feature;
 import org.eclipse.syson.sysml.Membership;
@@ -34,7 +35,9 @@
 import org.eclipse.syson.sysml.Redefinition;
 import org.eclipse.syson.sysml.ReferenceUsage;
 import org.eclipse.syson.sysml.Specialization;
+import org.eclipse.syson.sysml.Succession;
 import org.eclipse.syson.sysml.SuccessionAsUsage;
+import org.eclipse.syson.sysml.TransitionUsage;
 import org.eclipse.syson.sysml.Type;
 import org.eclipse.syson.sysml.helper.EMFUtils;
 import org.eclipse.syson.sysml.upload.SysMLExternalResourceLoaderService;
@@ -71,7 +74,7 @@ public void setUp() {
     }
 
     @Test
-    @DisplayName("Given a model with duplicated names, when importing the model, then the resolution of name shoud match the closest matching element")
+    @DisplayName("Given a model with duplicated names, when importing the model, then the resolution of name should match the closest matching element")
     public void checkNameResolutionProcessWithDuplicatedName() throws IOException {
         var input = """
                 package p1 {
@@ -99,7 +102,7 @@ public void checkNameResolutionProcessWithDuplicatedName() throws IOException {
     }
 
     @Test
-    @DisplayName("Given a model with a Redefintion, when importing the model, then the resolution of name should special Redefinition rules")
+    @DisplayName("Given a model with a Redefintion, when importing the model, then the resolution of name should specialize Redefinition rules")
     public void checkNameResolutionProcessInRedifinition() throws IOException {
         var input = """
                 package p1 {
@@ -129,7 +132,7 @@ public void checkNameResolutionProcessInRedifinition() throws IOException {
     }
 
     @Test
-    @DisplayName("Given a model with a reference to an invalid type, when importing the model then the resolution of name shoud not set a reference with an incompatible target")
+    @DisplayName("Given a model with a reference to an invalid type, when importing the model, then the resolution of name shoud not set a reference with an incompatible target")
     public void checkNameResolutionProcessWithInvalidTargetName() throws IOException {
         var input = """
                 package p1 {
@@ -156,7 +159,78 @@ public void checkNameResolutionProcessWithInvalidTargetName() throws IOException
     }
 
     @Test
-    @DisplayName("Given a model with PortDefinitions, when importing the model, then a conjugated port is create for each conjugated ports")
+    @DisplayName("Given a model with a TransitionUsage, when importing the model, then a TransitionUsage should be created.")
+    public void checkSimpleTransitionUsage() throws IOException {
+        var input = """
+                action def A1 {
+                    private import ScalarValues::Integer;
+                    action a1 ;
+                    action a2 ;
+                    attribute x : Integer;
+                    succession S first a1 if x == 1 then a2;
+                }""";
+
+        this.checker.checkImportedModel(resource -> {
+            TransitionUsage transitionUsage = EMFUtils.allContainedObjectOfType(resource, TransitionUsage.class)
+                    .filter(t -> "S".equals(t.getDeclaredName()))
+                    .findFirst().get();
+
+            ActionUsage source = transitionUsage.getSource();
+            assertThat(source).isNotNull().matches(s -> "a1".equals(s.getName()));
+            ActionUsage target = transitionUsage.getTarget();
+            assertThat(target).isNotNull().matches(t -> "a2".equals(t.getName()));
+
+            Succession succession = transitionUsage.getSuccession();
+            assertThat(succession).isNotNull();
+
+        }).check(input);
+    }
+
+    @Test
+    @DisplayName("Given a model with TransitionUsage with an implicit source, when importing the model, then the implicit source should be correctly resolved.")
+    public void checkTransitionWithImplicitSource() throws IOException {
+        var input = """
+                   action def A1 {
+                    private import ScalarValues::Integer;
+                    attribute x : Integer;
+                    action a1;
+                    action a2;
+                    action a3;
+                    decide d;
+                        if x == 1 then a1;
+                        if x == 2 then a2;
+                        else a3;
+                }""";
+
+        this.checker.checkImportedModel(resource -> {
+            List<TransitionUsage> transitionUsages = EMFUtils.allContainedObjectOfType(resource, TransitionUsage.class)
+                    .toList();
+
+            assertThat(transitionUsages).hasSize(3);
+
+            TransitionUsage t1 = transitionUsages.get(0);
+            ActionUsage source1 = t1.getSource();
+            assertThat(source1).isNotNull().matches(s -> "d".equals(s.getName()));
+            ActionUsage target1 = t1.getTarget();
+            assertThat(target1).isNotNull().matches(t -> "a1".equals(t.getName()));
+
+            TransitionUsage t2 = transitionUsages.get(1);
+            ActionUsage source2 = t2.getSource();
+            assertThat(source2).isNotNull().matches(s -> "d".equals(s.getName()));
+            ActionUsage target2 = t2.getTarget();
+            assertThat(target2).isNotNull().matches(t -> "a2".equals(t.getName()));
+
+            TransitionUsage t3 = transitionUsages.get(2);
+            ActionUsage source3 = t3.getSource();
+            assertThat(source3).isNotNull().matches(s -> "d".equals(s.getName()));
+            ActionUsage target3 = t3.getTarget();
+            assertThat(target3).isNotNull().matches(t -> "a3".equals(t.getName()));
+
+        }).check(input);
+    }
+
+    @Test
+    @DisplayName("Given a model with PortDefinitions, when importing the model, then a conjugated port is created for each conjugated ports")
     public void checkConjugatedPortCreation() throws IOException {
         var input = """
                 package Conjugated {
@@ -172,7 +246,6 @@ public void checkConjugatedPortCreation() throws IOException {
                     }
                 }""";
 
-
         this.checker.checkImportedModel(resource -> {
             PortDefinition tempPort = EMFUtils.allContainedObjectOfType(resource, PortDefinition.class)
                     .filter(pod -> "TempPort".equals(pod.getDeclaredName()))

backend/application/syson-application/src/test/resources/part_changes.json

@@ -86,7 +86,9 @@
         "unioningType": [],
         "chainingFeature": [],
         "endOwningType": null,
-        "featureTarget": null,
+        "featureTarget": {
+            "@id":"a4f51a38-bfeb-4e0d-a870-55f8fe90405e"
+        },
         "featuringType": [],
         "ownedFeatureChaining": [],
         "ownedFeatureInverting": [],

backend/application/syson-application/src/test/resources/simple_project.json

@@ -283,7 +283,9 @@
         "unioningType": [],
         "chainingFeature": [],
         "endOwningType": null,
-        "featureTarget": null,
+        "featureTarget": {
+            "@id": "a4f51a38-bfeb-4e0d-a870-55f8fe90405e"
+        },
         "featuringType": [],
         "ownedFeatureChaining": [],
         "ownedFeatureInverting": [],

backend/application/syson-application/src/test/resources/simple_project_changes.json

@@ -312,7 +312,9 @@
             "unioningType": [],
             "chainingFeature": [],
             "endOwningType": null,
-            "featureTarget": null,
+            "featureTarget": {
+                "@id":"a4f51a38-bfeb-4e0d-a870-55f8fe90405e"
+            },
             "featuringType": [],
             "ownedFeatureChaining": [],
             "ownedFeatureInverting": [],

backend/application/syson-sysml-import/src/main/java/org/eclipse/syson/sysml/ASTTransformer.java

@@ -83,11 +83,23 @@ public Resource convertResource(final InputStream input, final ResourceSet resou
                 this.astTreeParser.resolveAllReference(proxiedReferences);
                 this.logger.info("End of references resolving");
 
+                this.postResolvingFixingPhase(rootSysmlObjects);
             }
         }
         return result;
     }
 
+    private void postResolvingFixingPhase(List<EObject> rootSysmlObjects) {
+        for (EObject root : rootSysmlObjects) {
+            this.fixTransitionUsageImplicitSource(root);
+        }
+    }
+
+    private void fixTransitionUsageImplicitSource(EObject root) {
+        List<TransitionUsage> transitionUsages = EMFUtils.allContainedObjectOfType(root, TransitionUsage.class).toList();
+        this.fixImplicitTransitionSourceFeature(transitionUsages);
+    }
+
     private void preResolvingFixingPhase(List<EObject> rootSysmlObjects) {
         for (EObject root : rootSysmlObjects) {
             this.fixSuccessionUsageImplicitSource(root);
@@ -171,7 +183,7 @@ private void fixImplicitSourceFeature(List<SuccessionAsUsage> succesionAsUsage)
                     ReferenceUsage refUsage = SysmlFactory.eINSTANCE.createReferenceUsage();
                     EList<Element> ownedRelatedElements = owningFeatureMembershit.getOwnedRelatedElement();
 
-                    Membership previousMembershipFeature = this.computePreviousFeatureMembership(suc);
+                    Membership previousMembershipFeature = this.computePreviousFeatureMembership(suc, m -> m.eClass() == SysmlPackage.eINSTANCE.getMembership());
                     if (previousMembershipFeature != null) {
                         // For implicit source that targets an element of the standard library, we need to keep a
                         // "virtual link" to the previous feature membership to identify the source of SuccessionAsUsage
@@ -186,16 +198,39 @@ private void fixImplicitSourceFeature(List<SuccessionAsUsage> succesionAsUsage)
                 });
     }
 
-    private Membership computePreviousFeatureMembership(SuccessionAsUsage successionAsUsage) {
-        Type owningType = successionAsUsage.getOwningType();
+    private void fixImplicitTransitionSourceFeature(List<TransitionUsage> transitionUsages) {
+        transitionUsages.stream().filter(this::hasImplicitSourceFeature)
+                .forEach(transition -> {
+                    // The specification define the computation of the sourceFeature of TransitionUsage by:
+                    // Return the Feature to be used as the source of the succession of this TransitionUsage, which is
+                    // the first ownedMember of the TransitionUsage that is a Feature not owned via a FeatureMembership
+                    // whose featureTarget is an ActionUsage.
+                    // The current implementation of SysIDE providing the AST does create such element. This fix aims to
+                    // provide a workaround.
+                    Membership previousFeature = this.computePreviousFeatureMembership(transition, m -> this.isValidPreviousFeature(m));
+                    if (previousFeature != null && previousFeature.getMemberElement() instanceof ActionUsage actionUsage) {
+                        Membership membership = SysmlFactory.eINSTANCE.createMembership();
+                        membership.setMemberElement(actionUsage);
+                        transition.getOwnedRelationship().add(0, membership);
+                    }
+                });
+    }
+
+    private boolean isValidPreviousFeature(Membership m) {
+        Element memberElement = m.getMemberElement();
+        return memberElement instanceof ActionUsage && !(memberElement instanceof TransitionUsage);
+    }
+
+    private Membership computePreviousFeatureMembership(Feature testedFeature, java.util.function.Predicate<Membership> filter) {
+        Type owningType = testedFeature.getOwningType();
         if (owningType != null) {
             EList<Membership> ownedMemberships = owningType.getOwnedMembership();
-            int index = ownedMemberships.indexOf(successionAsUsage.getOwningMembership());
+            int index = ownedMemberships.indexOf(testedFeature.getOwningMembership());
             if (index > 0) {
                 ListIterator<Membership> iterator = ownedMemberships.subList(0, index).listIterator(index);
                 while (iterator.hasPrevious()) {
                     Membership previous = iterator.previous();
-                    if (previous.getMemberElement() instanceof Feature) {
+                    if (previous.getMemberElement() instanceof Feature && (filter == null || filter.test(previous))) {
                         return previous;
                     }
                 }
@@ -233,6 +268,11 @@ private boolean hasImplicitSourceFeature(SuccessionAsUsage successionUsage) {
         return false;
     }
 
+    private boolean hasImplicitSourceFeature(TransitionUsage transitionUsage) {
+        ActionUsage source = transitionUsage.getSource();
+        return source == null;
+    }
+
     private boolean hasImplicitTargetFeature(SuccessionAsUsage successionUsage) {
         EList<Feature> ends = successionUsage.getConnectorEnd();
         if (ends.size() > 1) {