Community
Participate
Working Groups
Created attachment 244476 [details] Test case to reproduce Step to reproduce: * Make sure the Preferences > Sirius > Auto Refresh is disabled. * Import the modified ecore sample modeler and the given test case in the same workspace. * Open the "package entities" diagram * Make sure the diagram is "unsynchronized". The EReference edge mapping has been modified to be "unsynchronizable" * Create a new reference between NewEClass5 and NewEClass6. * The two other references between NewEClass1-NewEClass2 and NewEClass3-NewEClass4 are re-created as well. Those two edges have been deleted from the diagram without deleting the EReference within the semantic model.
It seems that all edges matching the same mapping than the one we create with the tool are creates. In org.eclipse.sirius.diagram.business.internal.helper.task.CreateDEdgeTask.execute() l.120 we call dDiagramSynchronizer.createEdgeMapping(...) and next we call org.eclipse.sirius.diagram.business.internal.experimental.sync.DDiagramSynchronizer.createDEdgeOnMapping(...) which can creates several DEdge (by calling next org.eclipse.sirius.diagram.business.internal.experimental.sync.DDiagramSynchronizer.createEdgeCandidates(...). It doesn't make sense here because we create a single edge from the edge creation tool.
Created attachment 244542 [details] modified sample ecore modeler
Created attachment 244543 [details] Test case to reproduce
See https://git.eclipse.org/r/#/c/29048/ for a fix.
I've set the main target to 2.0 (i.e. master), but it could probably be also backported to 1.0.x.
Merged as 1a83d2ee6df562a5f490aa52567c12db2794c122 .
This commits makes several regressions on SWTBot Sequence tests suite (in SequenceExecutionBasicAndReturnMessageTest for example). I reverted it on master.
I tried in debug, both version does not create the same DEdgeSpec on a Sequence Diagram: the sourceNode is not the same.
See https://git.eclipse.org/r/#/c/30051/ for a new version of the correction. Sequence post refresh extension should not flag elements created by tools as externally changes. Then layout will conserve the location/size set by the tool.
It seems that there is a bug in the EdgeMappings refresh: it is not able to delete some edge or to detect there are invalid.
In facts this also comes from our proposed correction and the sample I am using: the edge creation tool has some extra source/target mappings and so the sourceView/targetViews enable the tool but are not compatible with the mapping definition. So if we create a candidate as this, the next refresh will not be able to delete the produced DEdge. And if this deletion is corrected, in manual refresh, the created Edge will not be consistent with the modeler defined in the odesign.
The real issue comes from computeEdgeCandidatesWithDomain and computeEdgeCandidatesWithoutDomain called from org.eclipse.sirius.diagram.business.internal.experimental.sync.DEdgeSynchronizerHelper.computeNowEdgeCandidates(EdgeMapping, Map<DiagramElementMapping, Collection<EdgeTarget>>). > if (tool || new DiagramElementMappingQuery(mapping).isSynchronizedAndCreateElement(diagram)) { The synchronization state well return false in our case (unsynchronized diagram with unsynchonizable maping) but the tool field has been set to true and cause the creation of all candidates. A proper solution would consist in restricting the candidates to the semantic elements just created by the tool. The AbstractSynchronizerHelper.setTool() seems to be called only from the CreateEdgeTask and CreateViewTask (and only for the DEdgeSynchonizerHelper of the DDiagramSynchronizer). However the 'tool' field is used during candidates computation of both DNodeSynchronizerHelper and DEdgeSynchronizerHelper. Furthermore, the DEdge creation code should be the same in CreateEdgeTask and CreateViewTask.
The current proposed patch on gerrit is not acceptable as it create an edge candidate from the current mapping and clicked source and target views, but in several cases, those clicked views correponds to extra mappings and cannot be used as real candidates for the edge creation. Note also that the current behavior of some tools creates several graphical elements in a magical way, but to correctly handle the auto/manual refresh and the different synchronization modes, we should probably not have this magic. We should ask the specifiers to add CreateView, CreateEdgeView model operation to explicitely define the vies to create (which view, which mapping, which real container/source/target).
