[470859] Update editor and release notes to reflect the feature state

* Enable recursive regions in VSM editor as experimental
* Also change corresponding validation constraint from error to warning.
* Change the experimental status of the global feature: the
compartiments are no more experimental except for the recursive cases.
* Update the property section regarding the state of the selected
ContainerMapping.

Bug: 470859
Change-Id: If054c89d3bfc022fa0cf99a83d07e11e7964f35c
Signed-off-by: Maxime Porhel <maxime.porhel@obeo.fr>

7 files changed, 137 insertions, 66 deletions

diff --git a/plugins/org.eclipse.sirius.diagram/plugin.properties b/plugins/org.eclipse.sirius.diagram/plugin.properties
index ac1e4a276f..cd7df93e73 100644
--- a/plugins/org.eclipse.sirius.diagram/plugin.properties
+++ b/plugins/org.eclipse.sirius.diagram/plugin.properties
@@ -88,9 +88,9 @@ category.siriusValidation.name = Sirius Diagram Validation Rules
 constraint.candidateExpressionExistsForRecursiveMapping.description = Validates that there is no blank SemanticCandidatesExpression for a recursive ContainerMapping
 constraint.candidateExpressionExistsForRecursiveMapping.message = The SemanticCandidatesExpression field of {0} does not accept a blank value because it is recursively imported by another ContainerMapping ({1}). This can cause infinite loop, StackOverflowError (except in case of not synchronized mapping).
 constraint.candidateExpressionExistsForRecursiveMapping.name = Blank Candidates Expression With Recursive Mapping
-constraint.compartmentRecusrion.description = Validates that Region Container Mapping should not be a Region (contained by another RegionContainer)
-constraint.compartmentRecusrion.message = The Mapping {0} should not be defined as a Region Container (ie children presentation set to horizontal stack or vertical stack) as it is itself defined as a Region (ie contained in a Region Container).
-constraint.compartmentRecusrion.name = Compartment recursivity constraint
+constraint.compartmentRecursion.description = Validates that Region Container Mapping should not be a Region (contained by another RegionContainer)
+constraint.compartmentRecursion.message = The Mapping {0} is defined as a Region Container (ie children presentation set to horizontal stack or vertical stack). It is also itself defined as a Region (ie contained in a Region Container), this could lead to unexpected empty area in the region layout.
+constraint.compartmentRecursion.name = Compartment recursivity constraint
 constraint.containersInContainerList.description = Validates that Container mapping should not be contained by a List mapping.
 constraint.containersInContainerList.message = The Mapping {0} should not be defined as a container mapping as it is contained in a Container List.
 constraint.containersInContainerList.name = Containers in Container List constraint
@@ -124,11 +124,11 @@ constraint.noSubNodesInRegionContainer.name = No sub nodes in region container c
 constraint.preconditionForDndTools.description = Drag and drop tools which accept Model or Both as source should have an explicit precondition.
 constraint.preconditionForDndTools.message = The Drop tool {0} accepts arbitrary elements from the Model Content view as drag source. You should specify a precondition to validate the nature of the dragged element.
 constraint.preconditionForDndTools.name = Check that drag and drop tools which accept the model as source have an explicit precondition.
-constraint.regionMappingCardinality.description = Validates that Region Container Mapping should not be a Region (contained by another RegionContainer)
+constraint.regionMappingCardinality.description = Validates that Region Container Mapping should have at least one Region mapping.
 constraint.regionMappingCardinality.message = The Mapping {0} is defined as a Region Container, it should have at least one Container mapping as child or reused mapping.
 constraint.regionMappingCardinality.name = Region mapping cardinality constraint
-constraint.regionStyle.description = Validates that only Gradient is used as style of RegionContainer and Region mappings.
-constraint.regionStyle.message = The style {0} is not valid for its parent mapping {1}. Only the gradient style is supported for RegionContainer and Region mappings.
+constraint.regionStyle.description = Validates that only Gradient and Image are used as style of RegionContainer and Region mappings.
+constraint.regionStyle.message = The style {0} is not valid for its parent mapping {1}. Only the gradient and image styles are supported for RegionContainer and Region mappings.
 constraint.regionStyle.name = RegionContainer and Region style constraint
 constraint.representationExtensionDescriptionRepresentationName.message = {0}
 constraint.representationExtensionDescriptionRepresentationName.name = Representation extension description representation name validation
diff --git a/plugins/org.eclipse.sirius.diagram/plugin.xml b/plugins/org.eclipse.sirius.diagram/plugin.xml
index 97fe94ee5e..554ddd7579 100644
--- a/plugins/org.eclipse.sirius.diagram/plugin.xml
+++ b/plugins/org.eclipse.sirius.diagram/plugin.xml
@@ -451,14 +451,14 @@
                   id="org.eclipse.sirius.diagram.constraints.CompartmentContainmentConstraint"
                   lang="Java"
                   mode="Batch"
-                  name="%constraint.compartmentRecusrion.name"
-                  severity="ERROR"
+                  name="%constraint.compartmentRecursion.name"
+                  severity="WARNING"
                   statusCode="1">
                <message>
-                  %constraint.compartmentRecusrion.message
+                  %constraint.compartmentRecursion.message
                </message>
                <description>
-                  %constraint.compartmentRecusrion.description
+                  %constraint.compartmentRecursion.description
                </description>
             </constraint>
             <constraint
diff --git a/plugins/org.eclipse.sirius.diagram/src-core/org/eclipse/sirius/diagram/tools/internal/validation/description/constraints/ContainerMappingChildrenPresentationConstraints.java b/plugins/org.eclipse.sirius.diagram/src-core/org/eclipse/sirius/diagram/tools/internal/validation/description/constraints/ContainerMappingChildrenPresentationConstraints.java
index 4fdf0c0b01..40d3a214ab 100644
--- a/plugins/org.eclipse.sirius.diagram/src-core/org/eclipse/sirius/diagram/tools/internal/validation/description/constraints/ContainerMappingChildrenPresentationConstraints.java
+++ b/plugins/org.eclipse.sirius.diagram/src-core/org/eclipse/sirius/diagram/tools/internal/validation/description/constraints/ContainerMappingChildrenPresentationConstraints.java
@@ -18,14 +18,16 @@ import org.eclipse.sirius.business.api.query.EObjectQuery;
 import org.eclipse.sirius.diagram.business.api.query.ContainerMappingQuery;
 import org.eclipse.sirius.diagram.description.ContainerMapping;
 import org.eclipse.sirius.diagram.description.DescriptionPackage;
-import org.eclipse.sirius.diagram.description.DiagramElementMapping;
 import org.eclipse.sirius.diagram.description.NodeMapping;
 import org.eclipse.sirius.diagram.description.filter.MappingFilter;
 import org.eclipse.sirius.diagram.description.style.ContainerStyleDescription;
 import org.eclipse.sirius.diagram.description.style.FlatContainerStyleDescription;
+import org.eclipse.sirius.diagram.description.style.WorkspaceImageDescription;
 import org.eclipse.sirius.ext.base.Option;
 import org.eclipse.sirius.tools.internal.validation.AbstractConstraint;
 
+import com.google.common.collect.Iterables;
+
 /**
  * Container children presentation related validation rules.
  * 
@@ -220,8 +222,8 @@ public class ContainerMappingChildrenPresentationConstraints extends AbstractCon
      * Mapping filter nodes on Regions validation rules.
      */
     private IStatus validateMappingFilterOnRegions(IValidationContext ctx, MappingFilter filter) {
-        for (DiagramElementMapping mapping : filter.getMappings()) {
-            if (mapping instanceof ContainerMapping && new ContainerMappingQuery((ContainerMapping) mapping.eContainer()).isRegionContainer()) {
+        for (ContainerMapping mapping : Iterables.filter(filter.getMappings(), ContainerMapping.class)) {
+            if (new ContainerMappingQuery(mapping).isRegion()) {
                 return ctx.createFailureStatus(new Object[] { filter });
             }
         }
@@ -229,14 +231,15 @@ public class ContainerMappingChildrenPresentationConstraints extends AbstractCon
     }
 
     /**
-     * RegionContainer and Region mappings only support the Gradient style.
+     * RegionContainer and Region mappings only support the Gradient and Image styles.
      */
     private IStatus validateStyle(IValidationContext ctx, ContainerStyleDescription style, ContainerMapping containerMapping) {
         ContainerMappingQuery query = new ContainerMappingQuery(containerMapping);
-        if (!(style instanceof FlatContainerStyleDescription) && (query.isRegionContainer() || query.isRegion())) {
-            return ctx.createFailureStatus(new Object[] { style, containerMapping });
+        if (query.isRegionContainer() || query.isRegion()) {
+            if (!(style instanceof FlatContainerStyleDescription || style instanceof WorkspaceImageDescription)) {
+                return ctx.createFailureStatus(new Object[] { style, containerMapping });
+            }
         }
         return ctx.createSuccessStatus();
     }
-
 }
diff --git a/plugins/org.eclipse.sirius.doc/doc/Release_Notes.html b/plugins/org.eclipse.sirius.doc/doc/Release_Notes.html
index 538dfffeb6..844dc88a9a 100644
--- a/plugins/org.eclipse.sirius.doc/doc/Release_Notes.html
+++ b/plugins/org.eclipse.sirius.doc/doc/Release_Notes.html
@@ -98,6 +98,22 @@
 				<code>BorderSizeComputationExpression</code> for new style is now 1, except for 
 				<em>Worskpace Image</em> where the value is 0.
 			</li>
+			<li><span class="label label-info">Modified</span> The support for containers with vertical or horizontal 
+				<em>compartments</em> on diagrams was introduced in Sirius 3.0.0 as an experimental feature. Several improvements and corrections have been done around the behaviors of this feature which is now ready to have its experimental state removed. Validation rules and tooltips in the VSM editor are here to help the specifier create a consistent mapping structure.
+			</li>
+			<li><span class="label label-info">Modified</span> <span class="label label-info">Experimental</span> New, experimental support for recursive 
+				<em>compartments</em> on diagrams: the 
+				<em>children presentation</em> property of a 
+				<em>Container Mapping</em>, which is defined as a 
+				<em>compartment</em> (its parent 
+				<em>ContainerMapping</em> has a 
+				<em>VerticalStack</em> or 
+				<em>HorizontalStack</em> 
+				<em>children presentation</em>), can now also be set to 
+				<em>Vertical Stack</em> and 
+				<em>Horizontal Stack</em>. It allows to define complex stacks of 
+				<em>compartments</em> mixing both stack directions. While working reliably, this support is marked experimental in Sirius 3.1.0 as there are still some behaviors which do not behave as expected. If you make use of this feature, be aware that some details may still change in future releases, including 3.1.x maintenance releases.
+			</li>
 		</ul>
 		<h3 id="DeveloperVisibleChanges">Developer-Visible Changes</h3>
 		<h4 id="Partialsupportforinternationalization">Partial support for internationalization</h4>
@@ -1816,4 +1832,4 @@
 			</li>
 		</ul>
 	</body>
-</html>
\ No newline at end of file
+</html>
diff --git a/plugins/org.eclipse.sirius.doc/doc/Release_Notes.textile b/plugins/org.eclipse.sirius.doc/doc/Release_Notes.textile
index 06a6cb9dbe..0f6a6cde87 100644
--- a/plugins/org.eclipse.sirius.doc/doc/Release_Notes.textile
+++ b/plugins/org.eclipse.sirius.doc/doc/Release_Notes.textile
@@ -22,6 +22,8 @@ Same sample since Sirius 3.1:
 h3. Specifier-Visible Changes
 
 * <span class="label label-info">Modified</span> You have now really the capability to set the @BorderSizeComputationExpression@ as 0 for style of @ContainerMapping@. Before Sirius 3.1.0, a size of 0 is interpreted as 1 at runtime. For compatibility reason, all existing @BorderSizeComputationExpression@ equals to 0 for style of @ContainerMapping@ are automatically set to 1 (ditto for results of this expression in representations file for container elements). For @ContainerMapping@, the default value of @BorderSizeComputationExpression@ for new style is now 1, except for _Worskpace Image_ where the value is 0.
+* <span class="label label-info">Modified</span> The support for containers with vertical or horizontal _compartments_ on diagrams was introduced in Sirius 3.0.0 as an experimental feature. Several improvements and corrections have been done around the behaviors of this feature which is now ready to have its experimental state removed. Validation rules and tooltips in the VSM editor are here to help the specifier create a consistent mapping structure.
+* <span class="label label-info">Modified</span> <span class="label label-info">Experimental</span> New, experimental support for recursive _compartments_ on diagrams: the _children presentation_ property of a _Container Mapping_, which is defined as a _compartment_ (its parent _ContainerMapping_ has a _VerticalStack_ or _HorizontalStack_ _children presentation_), can now also be set to _Vertical Stack_ and _Horizontal Stack_. It allows to define complex stacks of _compartments_ mixing both stack directions. While working reliably, this support is marked experimental in Sirius 3.1.0 as there are still some behaviors which do not behave as expected. If you make use of this feature, be aware that some details may still change in future releases, including 3.1.x maintenance releases.
 
 h3. Developer-Visible Changes
 
diff --git a/plugins/org.eclipse.sirius.editor.diagram/src/org/eclipse/sirius/diagram/editor/properties/section/description/containermapping/ContainerMappingChildrenPresentationPropertySectionSpec.java b/plugins/org.eclipse.sirius.editor.diagram/src/org/eclipse/sirius/diagram/editor/properties/section/description/containermapping/ContainerMappingChildrenPresentationPropertySectionSpec.java
index 08656bc49d..9abf5d8e3a 100644
--- a/plugins/org.eclipse.sirius.editor.diagram/src/org/eclipse/sirius/diagram/editor/properties/section/description/containermapping/ContainerMappingChildrenPresentationPropertySectionSpec.java
+++ b/plugins/org.eclipse.sirius.editor.diagram/src/org/eclipse/sirius/diagram/editor/properties/section/description/containermapping/ContainerMappingChildrenPresentationPropertySectionSpec.java
@@ -20,78 +20,114 @@ import org.eclipse.sirius.ext.base.Options;
 import org.eclipse.swt.graphics.Font;
 import org.eclipse.swt.graphics.FontData;
 import org.eclipse.swt.widgets.Button;
-import org.eclipse.swt.widgets.Composite;
-import org.eclipse.ui.views.properties.tabbed.TabbedPropertySheetPage;
 
 /**
  * A section for the childrenPresentation property of a ContainerMapping object.
  * 
- * It expose only the FreeForm and List capabilities but not the experimental
- * region support.
+ * It is able to mark as experimental or disable the compartment capabilities to
+ * help the user to produce a valid VSM.
  */
 public class ContainerMappingChildrenPresentationPropertySectionSpec extends ContainerMappingChildrenPresentationPropertySection {
 
     @Override
-    public void createControls(Composite parent, TabbedPropertySheetPage tabbedPropertySheetPage) {
-        super.createControls(parent, tabbedPropertySheetPage);
+    protected void makeWrittable() {
+        super.makeWrittable();
+
+        if (button != null && eObject instanceof ContainerMapping) {
+            ContainerMapping containerMapping = (ContainerMapping) eObject;
+            Option<String> experimentalStackButtons = shouldMarkCompartmentsExperimental(containerMapping);
+            Option<String> disabledStackButtons = shouldDisableCompartments(containerMapping);
+
+            updateControls(experimentalStackButtons, disabledStackButtons);
+        }
+    }
+
+    private Option<String> shouldMarkCompartmentsExperimental(ContainerMapping containerMapping) {
+        boolean experimental = false;
+        StringBuilder message = new StringBuilder("Experimental");
+        experimental = new ContainerMappingQuery(containerMapping).isRegion();
+        if (experimental) {
+            message.append(" - the mapping will produce containers which will be Region and RegionContainer");
+        }
 
+        for (ContainerMapping subContainerMapping : containerMapping.getAllContainerMappings()) {
+            experimental = experimental || new ContainerMappingQuery(subContainerMapping).isRegionContainer();
+
+            if (experimental) {
+                message.append(" - the children mapping will produce containers which will be Region and RegionContainer");
+                break;
+            }
+        }
+        message.append(".");
+        return experimental ? Options.newSome(message.toString()) : Options.<String> newNone();
+    }
+
+    private Option<String> shouldDisableCompartments(ContainerMapping containerMapping) {
+        String message = null;
+        if (!containerMapping.getAllNodeMappings().isEmpty()) {
+            message = "The mapping contains node mappings, it cannot be a RegionContainer mapping.";
+        }
+        return Options.newSome(message);
+    }
+
+    private void updateControls(Option<String> experimentalStackButtons, Option<String> disabledStackButtons) {
         String hStackButtonText = getText(ContainerLayout.HORIZONTAL_STACK);
         String vStackButtonText = getText(ContainerLayout.VERTICAL_STACK);
+
+        // Update buttons
+        boolean needsLayout = false;
         for (Button b : button) {
             if (hStackButtonText.equals(b.getText()) || vStackButtonText.equals(b.getText())) {
-                b.setToolTipText("Experimental");
-                Font f = b.getFont();
-                if (f != null && f.getFontData().length > 0) {
-                    FontData fontData = f.getFontData()[0];
-                    Font italic = SiriusEditor.getFontRegistry().getItalic(fontData.getName());
-                    if (italic != null) {
-                        b.setFont(italic);
-                    }
-                }
+                needsLayout = updateButton(b, experimentalStackButtons, disabledStackButtons) || needsLayout;
             }
         }
-    }
 
-    @Override
-    protected void makeWrittable() {
-        super.makeWrittable();
+        // Update group
+        updateGroup(disabledStackButtons);
 
-        if (button != null && eObject instanceof ContainerMapping) {
-            Option<String> stackButtonDisabled = shouldDisableCompartiments((ContainerMapping) eObject);
+        // If the section has been created with italic buttons, it might require
+        // layout be able to display the buttons with normal font.
+        if (needsLayout) {
+            composite.layout();
+        }
+    }
 
-            String hStackButtonText = getText(ContainerLayout.HORIZONTAL_STACK);
-            String vStackButtonText = getText(ContainerLayout.VERTICAL_STACK);
-            for (Button b : button) {
-                if (hStackButtonText.equals(b.getText()) || vStackButtonText.equals(b.getText())) {
-                    b.setEnabled(!stackButtonDisabled.some());
+    private boolean updateButton(Button b, Option<String> experimentalStackButtons, Option<String> disabledStackButtons) {
+        boolean needsLayout = false;
+        if (experimentalStackButtons.some()) {
+            b.setToolTipText(experimentalStackButtons.get());
+            Font f = b.getFont();
+            if (f != null && f.getFontData().length > 0) {
+                FontData fontData = f.getFontData()[0];
+                Font italic = SiriusEditor.getFontRegistry().getItalic(fontData.getName());
+                if (italic != null) {
+                    b.setFont(italic);
                 }
             }
-
-            if (group != null) {
-                if (stackButtonDisabled.some()) {
-                    group.setToolTipText(stackButtonDisabled.get());
-                } else {
-                    group.setToolTipText(null);
+        } else {
+            b.setToolTipText(null);
+            Font f = b.getFont();
+            if (f != null && f.getFontData().length > 0) {
+                FontData fontData = f.getFontData()[0];
+                Font normal = SiriusEditor.getFontRegistry().get(fontData.getName());
+                if (normal != null) {
+                    b.setFont(normal);
+                    needsLayout = true;
                 }
             }
         }
-    }
 
-    private Option<String> shouldDisableCompartiments(ContainerMapping containerMapping) {
-        ContainerMappingQuery query = new ContainerMappingQuery(containerMapping);
+        b.setEnabled(!disabledStackButtons.some());
+        return needsLayout;
+    }
 
-        String message = query.isRegion() ?  "A Region mapping cannot be a RegionContainer mapping.": null;
-        if (message == null) {
-            for (ContainerMapping subContainer : containerMapping.getAllContainerMappings()) {
-                ContainerMappingQuery subQuery = new ContainerMappingQuery(subContainer);
-                if (message == null && subQuery.isRegionContainer()) {
-                message = "The mapping contains RegionContainer sub mappings, they cannot be Region and RegionContainer.";
-                }
+    private void updateGroup(Option<String> disabledStackButtons) {
+        if (group != null) {
+            if (disabledStackButtons.some()) {
+                group.setToolTipText(disabledStackButtons.get());
+            } else {
+                group.setToolTipText(null);
             }
         }
-        if (message == null && !containerMapping.getAllNodeMappings().isEmpty()) {
-            message = "The mapping contains node mappings, it cannot be a RegionContainer mapping.";
-        }
-        return Options.newSome(message);
     }
 }
diff --git a/plugins/org.eclipse.sirius.tests.junit/src/org/eclipse/sirius/tests/unit/api/mappings/CompartmentsTests.java b/plugins/org.eclipse.sirius.tests.junit/src/org/eclipse/sirius/tests/unit/api/mappings/CompartmentsTests.java
index a67fcf4e88..df168ed90e 100644
--- a/plugins/org.eclipse.sirius.tests.junit/src/org/eclipse/sirius/tests/unit/api/mappings/CompartmentsTests.java
+++ b/plugins/org.eclipse.sirius.tests.junit/src/org/eclipse/sirius/tests/unit/api/mappings/CompartmentsTests.java
@@ -113,14 +113,28 @@ public class CompartmentsTests extends SiriusDiagramTestCase {
         changeChildrenPresentation(regionMapping, ContainerLayout.VERTICAL_STACK);
 
         diagnostic = diagnostician.validate(representation.getDescription());
-        assertEquals("The VSM is not valid, a region container mapping should not contain another region container mapping", Diagnostic.ERROR, diagnostic.getSeverity());
+        checkDiagnostic(diagnostic);
+
         undo();
 
         // set mapping to be a horizontal stack container mapping
         changeChildrenPresentation(regionMapping, ContainerLayout.HORIZONTAL_STACK);
         diagnostic = diagnostician.validate(representation.getDescription());
-        assertEquals("The VSM is not valid, a region container mapping should not contain another region container mapping", Diagnostic.ERROR, diagnostic.getSeverity());
+        checkDiagnostic(diagnostic);
+    }
 
+    private void checkDiagnostic(Diagnostic diagnostic) {
+        assertEquals("The VSM is not valid, there should be 2 errors and 1 warning.", Diagnostic.ERROR, diagnostic.getSeverity());
+        int errors = 0;
+        int warnings = 0;
+        for (Diagnostic d : diagnostic.getChildren()) {
+            if (Diagnostic.ERROR == d.getSeverity()) {
+                errors++;
+            } else if (Diagnostic.WARNING == d.getSeverity()) {
+                warnings++;
+            }
+        }
+        assertTrue("The VSM is not valid, a region container mapping can now contains other regions containers but user must be warned.", errors == 2 && warnings == 1);
     }
 
     /**