Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 432986

Summary: Several Diagram specific concepts are no more proposed in the VSM editor.
Product: [Modeling] Sirius Reporter: Maxime Porhel <maxime.porhel>
Component: DiagramAssignee: Pierre-Charles David <pierre-charles.david>
Status: CLOSED FIXED QA Contact: Julien Dupont <julien.dupont>
Severity: critical    
Priority: P3 CC: julien.dupont, pierre-charles.david
Version: 1.0.0M7Keywords: triaged
Target Milestone: 1.0.0M7   
Hardware: PC   
OS: Windows NT   
Whiteboard:
Bug Depends on: 427803    
Bug Blocks:    
Attachments:
Description Flags
NewChildDescriptorTest.java
none
The result of the test program when run on Sirius 1.0.0M6
none
The result of the test program when run on the current master
none
Draft of a patch that enables EMF's native child extender mechanism
none
The result of the test program when run on master with the patch applied
none
NewChildDescriptorTest.java
none
The result of the test program when run on Sirius 1.0.0M6
none
The result of the test program when run on the current master
none
NewChildDescriptorTest.java
none
The result of the test program when run on Sirius 1.0.0M6
none
The result of the test program when run on the current master
none
The result of the test program when run on master with the patch applied
none
The result of the test program when run on the current master none

Description Maxime Porhel CLA 2014-04-17 04:53:03 EDT
Since Bug 427803, several Diagram specific context are no more proposed by the VSM editor: Navigation, CreateView, CreateEdgeView, ... (other types will be listed later in a comment).

Furthermore, several "new child" menus seems duplicated and the order has changed. Diagram specific menus are now provided after the core menus : Diagram is a dialect, but before Bug 427803, its concepts and menus were defined by the core plugins. We should avoid duplication and add a priority notion on menu builders or sort the menu regarding their label.
Comment 1 Maxime Porhel CLA 2014-04-17 04:57:44 EDT
The Sample Reflective Ecore Model Editor does not propose the lacking types.
Comment 2 Pierre-Charles David CLA 2014-04-17 08:10:01 EDT
We might want to use EMF's own mechanism for this: http://ed-merks.blogspot.com/2008/01/creating-children-you-didnt-know.html

Our own code to support this predates this feature in EMF, but now even the most ancient EMF version we support provides this. Maybe we could remove most/all of the DialectUIServices.provide* methods.
Comment 3 Maxime Porhel CLA 2014-04-18 02:30:22 EDT
Steps to reproduce on Sequence diagram
# Create two Sequence diagrams with 2 lifelines, 1 execution on each and a message from the execution to the other lifeline.
# Launch the arrange all on both diagrams
# Close one of the diagram
# Move one of the message before its execution (reconnect)
# Reopen the second diagram
# Try to move m1 up (a few pixels)

-> If the bug is present, m1 does not move but the execution and the other events below are shifted down.
Comment 4 Maxime Porhel CLA 2014-04-18 02:31:50 EDT
Wrong comment (now added on Bug 432936)

(In reply to Maxime Porhel from comment #3)
> Steps to reproduce on Sequence diagram
> # Create two Sequence diagrams with 2 lifelines, 1 execution on each and a
> message from the execution to the other lifeline.
> # Launch the arrange all on both diagrams
> # Close one of the diagram
> # Move one of the message before its execution (reconnect)
> # Reopen the second diagram
> # Try to move m1 up (a few pixels)
> 
> -> If the bug is present, m1 does not move but the execution and the other
> events below are shifted down.
Comment 5 Pierre-Charles David CLA 2014-04-24 10:53:04 EDT
It looks like enabling the Child Creation Extender as mentioned in Ed's blog fixes the issue, but we have a lot of customized (@not-generated) code in the impacted classes, so it is not clear which ones would be impacted but are ignored by the regen. Also, the number of possible elements in the VSM make it difficult to be sure we have fixed all the cases. I'll see if we can write some automated tool to help.
Comment 6 Pierre-Charles David CLA 2014-04-25 05:31:42 EDT
Created attachment 242317 [details]
NewChildDescriptorTest.java

Attached is an automated test that outputs a sorted list of all the "newChildDescriptors" available for all Sirius types available in a VSM editor. It is implemented as a JUnit test to be run as a  "JUnit Plug-in Test" to be sure it is executed in the correct runtime context (with all the various extension points and registries correctly setup).
Comment 7 Pierre-Charles David CLA 2014-04-25 05:32:32 EDT
Created attachment 242318 [details]
The result of the test program when run on Sirius 1.0.0M6
Comment 8 Pierre-Charles David CLA 2014-04-25 05:35:26 EDT
Created attachment 242319 [details]
The result of the test program when run on the current master

Comparing the attached result when run on master to the one when run on M6 shows that we lost (among others) diagram.description.tool.CreateEdgeView, diagram.description.tool.CreateView and diagram.description.tool.Navigation which were mentioned in the issue description. There are many other differences, which probably require some deeper investigation.
Comment 9 Pierre-Charles David CLA 2014-04-25 05:36:28 EDT
Created attachment 242320 [details]
Draft of a patch that enables EMF's native child extender mechanism
Comment 10 Pierre-Charles David CLA 2014-04-25 05:39:15 EDT
Created attachment 242321 [details]
The result of the test program when run on master with the patch applied

With the patch applied, the test program produces the exact same output as what we had with Sirius 1.0.0M6, before the effective split of the diagram metamodel out of the core plug-ins.
Comment 11 Pierre-Charles David CLA 2014-04-29 08:55:34 EDT
Created attachment 242476 [details]
NewChildDescriptorTest.java

New version of the test program which avoids duplicate entries in the report.
Comment 12 Pierre-Charles David CLA 2014-04-29 08:56:17 EDT
Created attachment 242477 [details]
The result of the test program when run on Sirius 1.0.0M6

Updated version of the test result for M6 without the duplications.
Comment 13 Pierre-Charles David CLA 2014-04-29 08:57:16 EDT
Created attachment 242478 [details]
The result of the test program when run on the current master

Updated version of the result of the test program on current master (without the child extender patch), without the duplications.
Comment 14 Pierre-Charles David CLA 2014-04-29 09:06:06 EDT
Comparing the two reports, the elements we have lost (i.e. that can no longer be created using the VSM editor's menus) are:

Inside viewpoint.description.DecorationDescriptionsSet:
- diagram.description.MappingBasedDecoration

Inside viewpoint.description.Environment:
- diagram.description.tool.BehaviorTool
- diagram.description.tool.ContainerCreationDescription
- diagram.description.tool.ContainerDropDescription
- diagram.description.tool.DeleteElementDescription
- diagram.description.tool.DiagramCreationDescription
- diagram.description.tool.DiagramNavigationDescription
- diagram.description.tool.DirectEditLabel
- diagram.description.tool.DoubleClickDescription
- diagram.description.tool.EdgeCreationDescription
- diagram.description.tool.NodeCreationDescription
- diagram.description.tool.ReconnectEdgeDescription
- diagram.description.tool.RequestDescription
- diagram.description.tool.ToolGroup

Inside most ContainerModelOperation and also Case/SwitchChild:
- diagram.description.tool.CreateEdgeView
- diagram.description.tool.CreateView
- diagram.description.tool.Navigation

The environement is never actually visible to the user inside the VSM editor. For the other cases, I confirm that we lost MappingBasedDecoration, CreateEdgeView, CreateView and Navigation. It looks like the test program's results are reliable, so for now I'll assume there are no other things lost.

With only these 4 elements to handle, enabling the child extender mechanism seems overkill, especially this close to M7. I'll see if I can recover them "manually".
Comment 15 Pierre-Charles David CLA 2014-04-29 09:30:40 EDT
org.eclipse.sirius.ui.business.api.dialect.DialectUIServices already provides some extension mechanism. In fact it provides 5 different ones, each narrowly targeted towards extending the provider of a specific kind of element. None of these correspond to the context we need for the elements we lost. We may need to enable EMF's mechanism, otherwise it means either adding 1 or 2 new ad hoc extensions methods (which are API), or create a new one which is more generic, but would basically reproduce what EMF does.
Comment 16 Pierre-Charles David CLA 2014-04-29 10:22:42 EDT
See https://git.eclipse.org/r/#/c/25733/
Comment 17 Pierre-Charles David CLA 2014-04-29 10:35:29 EDT
There is a bug in the new version of the test program: I had to comment out the reference to org.eclipse.sirius.diagram.description.DescriptionPackage.eINSTANCE when trying to run the test under M6 (where the package did not exist), and forgot to uncomment it when running the test under M6 and master.
Comment 18 Pierre-Charles David CLA 2014-04-29 10:53:45 EDT
Created attachment 242493 [details]
NewChildDescriptorTest.java

New version of the test program with the diagram package re-enabled.
Comment 19 Pierre-Charles David CLA 2014-04-29 10:54:56 EDT
Created attachment 242494 [details]
The result of the test program when run on Sirius 1.0.0M6

New version of the M6 log produced by the fixed test.
Comment 20 Pierre-Charles David CLA 2014-04-29 10:56:05 EDT
Created attachment 242495 [details]
The result of the test program when run on the current master

New version of the current master (without the patch) log produced by the fixed test.
Comment 21 Pierre-Charles David CLA 2014-04-29 11:02:31 EDT
Created attachment 242496 [details]
The result of the test program when run on master with the patch applied

New version of the log with the patch applied and the fixed test program.
Comment 22 Pierre-Charles David CLA 2014-04-29 11:03:08 EDT
Created attachment 242497 [details]
The result of the test program when run on the current master

New version of the log for master (no patch) with the fixed test program.
Comment 23 Pierre-Charles David CLA 2014-05-03 04:31:56 EDT
Fixed by 4dc0c302144f9edf86f06a5ccfbe0236cedc78f5. Note that there are other regressions in the VSM editor caused by the diagram split: menu items order and grouping, property sections ordering and customization. They are less critical and will be treated after M7.
Comment 24 Pierre-Charles David CLA 2014-05-12 03:46:14 EDT
Available in Sirius 1.0.0M7 (see https://wiki.eclipse.org/Sirius/1.0.0M7 & http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/tag/?id=v1.0.0M7).