[305] Fix performance issue when using EcoreUtil.delete

Bug: #305 Signed-off-by: Gwendal Daniel <gwendal.daniel@obeosoft.com>
eclipse-syson · May 21, 2024 · df68b90 · df68b90
1 parent d54338a
commit df68b90
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 10 deletions.
diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc
@@ -29,6 +29,7 @@
 - https://github.com/eclipse-syson/syson/issues/274[#274] [import] Namespace.getImportedMemberships method now prevents name collisions
 - https://github.com/eclipse-syson/syson/issues/271[#271] [diagrams] Remove non end Usages from AllocationDefinition ends compartment
 - https://github.com/eclipse-syson/syson/issues/229[#229] [diagrams] Prevent circular containment of nested parts including self containment
+- https://github.com/eclipse-syson/syson/issues/305[#305] [diagrams] Fix performance issue when using EcoreUtil.delete
 
 === Improvements
 

diff --git a/...on-tests/src/main/java/org/eclipse/syson/tests/architecture/AbstractCodingRulesTests.java b/...on-tests/src/main/java/org/eclipse/syson/tests/architecture/AbstractCodingRulesTests.java
@@ -164,6 +164,18 @@ public void noClassesShouldUseEMF() {
         rule.check(this.getClasses());
     }
 
+    public void noClassesShouldUseEcoreUtilDelete() {
+        ArchRule rule = ArchRuleDefinition.noClasses()
+                .that()
+                .resideInAPackage(this.getProjectRootPackage())
+                .should()
+                .callMethod("org.eclipse.emf.ecore.util.EcoreUtil", "delete", "org.eclipse.emf.ecore.EObject")
+                .orShould()
+                .callMethod("org.eclipse.emf.ecore.util.EcoreUtil", "delete", "org.eclipse.emf.ecore.EObject", "boolean")
+                .because("EcoreUtil.delete doesn't work well with Sysml Standard Libraries in the ResourceSet, use DeleteService instead");
+        rule.check(this.getClasses());
+    }
+
     public void noClassesShouldUseApacheCommons() {
         // @formatter:off
         ArchRule rule = ArchRuleDefinition.noClasses()

diff --git a/...-view/src/main/java/org/eclipse/syson/diagram/common/view/services/ViewCreateService.java b/...-view/src/main/java/org/eclipse/syson/diagram/common/view/services/ViewCreateService.java
@@ -19,7 +19,6 @@
 import org.eclipse.emf.ecore.EClass;
 import org.eclipse.emf.ecore.EReference;
 import org.eclipse.emf.ecore.EStructuralFeature;
-import org.eclipse.emf.ecore.util.EcoreUtil;
 import org.eclipse.sirius.components.collaborative.diagrams.api.IDiagramContext;
 import org.eclipse.sirius.components.collaborative.diagrams.api.IDiagramService;
 import org.eclipse.sirius.components.core.api.IEditingContext;
@@ -28,6 +27,7 @@
 import org.eclipse.sirius.components.diagrams.Node;
 import org.eclipse.sirius.components.diagrams.ViewCreationRequest;
 import org.eclipse.sirius.components.view.emf.diagram.api.IViewDiagramDescriptionSearchService;
+import org.eclipse.syson.services.DeleteService;
 import org.eclipse.syson.services.ElementInitializerSwitch;
 import org.eclipse.syson.sysml.AcceptActionUsage;
 import org.eclipse.syson.sysml.AllocationDefinition;
@@ -69,10 +69,13 @@ public class ViewCreateService {
 
     private final ElementInitializerSwitch elementInitializerSwitch;
 
+    private final DeleteService deleteService;
+
     public ViewCreateService(IViewDiagramDescriptionSearchService viewDiagramDescriptionSearchService, IObjectService objectService) {
         this.viewDiagramDescriptionSearchService = Objects.requireNonNull(viewDiagramDescriptionSearchService);
         this.objectService = Objects.requireNonNull(objectService);
         this.elementInitializerSwitch = new ElementInitializerSwitch();
+        this.deleteService = new DeleteService();
     }
 
     /**
@@ -441,7 +444,7 @@ public Element createAcceptActionPayload(AcceptActionUsage self, String payloadE
                 var oldParameterContent = parameterMembership.getOwnedMemberParameter();
                 if (oldParameterContent != null) {
                     // there is already a playload parameter, we need to delete it.
-                    EcoreUtil.delete(oldParameterContent);
+                    this.deleteService.deleteFromModel(oldParameterContent);
                 }
                 parameterMembership.getOwnedRelatedElement().add(referenceUsage);
                 self.getOwnedRelationship().add(parameterMembership);
@@ -517,7 +520,7 @@ public Element createAcceptActionReceiver(AcceptActionUsage self) {
         Feature oldParameterContent = parameterMembership.getOwnedMemberParameter();
         if (oldParameterContent != null) {
             // there is already an element, we need to delete this element
-            EcoreUtil.delete(oldParameterContent);
+            this.deleteService.deleteFromModel(oldParameterContent);
         }
         parameterMembership.getOwnedRelatedElement().add(referenceUsage);
         self.getOwnedRelationship().add(parameterMembership);

diff --git a/...on-view/src/main/java/org/eclipse/syson/diagram/common/view/services/ViewToolService.java b/...on-view/src/main/java/org/eclipse/syson/diagram/common/view/services/ViewToolService.java
@@ -20,7 +20,6 @@
 import java.util.Optional;
 
 import org.eclipse.emf.ecore.EClass;
-import org.eclipse.emf.ecore.util.EcoreUtil;
 import org.eclipse.sirius.components.collaborative.diagrams.api.IDiagramContext;
 import org.eclipse.sirius.components.core.api.IEditingContext;
 import org.eclipse.sirius.components.core.api.IFeedbackMessageService;
@@ -35,6 +34,7 @@
 import org.eclipse.sirius.components.representations.MessageLevel;
 import org.eclipse.sirius.components.view.diagram.DiagramDescription;
 import org.eclipse.sirius.components.view.emf.IViewRepresentationDescriptionSearchService;
+import org.eclipse.syson.services.DeleteService;
 import org.eclipse.syson.services.ElementInitializerSwitch;
 import org.eclipse.syson.services.ToolService;
 import org.eclipse.syson.sysml.ActionUsage;
@@ -75,11 +75,14 @@ public class ViewToolService extends ToolService {
 
     private final Logger logger = LoggerFactory.getLogger(ViewToolService.class);
 
+    private final DeleteService deleteService;
+
     public ViewToolService(IObjectService objectService, IRepresentationDescriptionSearchService representationDescriptionSearchService,
             IViewRepresentationDescriptionSearchService viewRepresentationDescriptionSearchService, IFeedbackMessageService feedbackMessageService) {
         super(objectService, representationDescriptionSearchService, feedbackMessageService);
         this.viewRepresentationDescriptionSearchService = Objects.requireNonNull(viewRepresentationDescriptionSearchService);
         this.elementInitializerSwitch = new ElementInitializerSwitch();
+        this.deleteService = new DeleteService();
     }
 
     /**
@@ -294,7 +297,7 @@ private Element changeOwner(Element element, Element newContainer) {
             // element aren't lost when changing its container.
             newContainer.getOwnedRelationship().add(newFeatureMembership);
             newFeatureMembership.getOwnedRelatedElement().add(element);
-            EcoreUtil.delete(owningMembership);
+            this.deleteService.deleteFromModel(owningMembership);
         }
         return element;
     }
@@ -320,7 +323,7 @@ public RequirementUsage becomeObjectiveRequirement(RequirementUsage requirement,
                     var newObjectiveMembership = SysmlFactory.eINSTANCE.createObjectiveMembership();
                     newObjectiveMembership.getOwnedRelatedElement().add(requirement);
                     newContainer.getOwnedRelationship().add(newObjectiveMembership);
-                    EcoreUtil.delete(owningMembership);
+                    this.deleteService.deleteFromModel(owningMembership);
                 }
             }
         }
@@ -335,7 +338,7 @@ public PartUsage addAsNestedPart(PartDefinition partDefinition, PartUsage partUs
             var newFeatureMembership = SysmlFactory.eINSTANCE.createFeatureMembership();
             newFeatureMembership.getOwnedRelatedElement().add(partUsage);
             partDefinition.getOwnedRelationship().add(newFeatureMembership);
-            EcoreUtil.delete(owningMembership);
+            this.deleteService.deleteFromModel(owningMembership);
         }
         return partUsage;
     }
@@ -444,7 +447,9 @@ private void moveContraintInRequirementConstraintCompartment(ConstraintUsage dro
         membership.getOwnedRelatedElement().add(droppedConstraint);
         membership.setKind(kind);
         requirement.getOwnedRelationship().add(membership);
-        EcoreUtil.delete(oldMembership);
+        if (oldMembership instanceof OwningMembership owningMembership) {
+            this.deleteService.deleteFromModel(owningMembership);
+        }
     }
 
     public Element dropElementFromDiagramInConstraintCompartment(Element droppedElement, Node droppedNode, Element targetElement, Node targetNode, IEditingContext editingContext,
@@ -456,7 +461,9 @@ public Element dropElementFromDiagramInConstraintCompartment(Element droppedElem
                 var membership = SysmlFactory.eINSTANCE.createFeatureMembership();
                 membership.getOwnedRelatedElement().add(droppedConstraint);
                 targetElement.getOwnedRelationship().add(membership);
-                EcoreUtil.delete(oldMembership);
+                if (oldMembership instanceof OwningMembership owningMembership) {
+                    this.deleteService.deleteFromModel(owningMembership);
+                }
                 this.createView(droppedElement, editingContext, diagramContext, targetNode, convertedNodes, NodeContainmentKind.CHILD_NODE);
                 diagramContext.getViewDeletionRequests().add(ViewDeletionRequest.newViewDeletionRequest().elementId(droppedNode.getId()).build());
             }
@@ -493,7 +500,9 @@ public Element reconnnectTargetCompositionEdge(Element self, Element oldTarget,
                     // reuse feature membership of the previous nested
                     var oldMembership = newTarget.eContainer();
                     featureMembership.getOwnedRelatedElement().add(newTarget);
-                    EcoreUtil.delete(oldMembership);
+                    if (oldMembership instanceof OwningMembership oldOwningMembership) {
+                        this.deleteService.deleteFromModel(oldOwningMembership);
+                    }
                 }
             }
         }