eclipse-sirius / sirius-web

Sirius Web: open-source low-code platform to define custom web applications supporting your specific visual languages
https://eclipse.dev/sirius/sirius-web.html
Eclipse Public License 2.0
79 stars 52 forks source link

Details view does not properly support "mixed" selections with different property descriptions #1224

Open pcdavid opened 2 years ago

pcdavid commented 2 years ago

Screenshots

Capture d’écran du 2022-05-18 14-45-21

Steps to reproduce

  1. Create a new project
  2. Add a "Robot Flow" and a "View"
  3. In the view, create a diagram description and a node description
  4. Expand elements in the explorer so that both the node style and some (any) element of the Robot flow are visible.
  5. Select a semantic element from the Robot, and Ctrl-click on the node style to add it to the selection => :boom:
java.util.MissingResourceException: The string resource '_UI_BorderStyle_borderColor_feature' could not be located
    at org.eclipse.emf.common.util.DelegatingResourceLocator.delegatedGetString(DelegatingResourceLocator.java:541) ~[org.eclipse.emf.common-2.21.0.jar:na]
    at org.eclipse.emf.common.util.DelegatingResourceLocator.getString(DelegatingResourceLocator.java:445) ~[org.eclipse.emf.common-2.21.0.jar:na]
    at org.eclipse.emf.edit.provider.ItemProviderAdapter.getString(ItemProviderAdapter.java:2048) ~[org.eclipse.emf.edit-2.16.0.jar:na]
    at org.eclipse.emf.edit.provider.ItemProviderAdapter.getString(ItemProviderAdapter.java:2040) ~[org.eclipse.emf.edit-2.16.0.jar:na]
    at org.eclipse.sirius.components.view.provider.NodeStyleItemProvider.addBorderColorPropertyDescriptor(NodeStyleItemProvider.java:137) ~[classes/:na]
    at org.eclipse.sirius.components.view.provider.NodeStyleItemProvider.getPropertyDescriptors(NodeStyleItemProvider.java:58) ~[classes/:na]
    at org.eclipse.sirius.components.emf.compatibility.properties.PropertiesDefaultDescriptionProvider.lambda$14(PropertiesDefaultDescriptionProvider.java:148) ~[classes/:na]
    at java.base/java.util.Optional.map(Optional.java:265) ~[na:na]
    at org.eclipse.sirius.components.emf.compatibility.properties.PropertiesDefaultDescriptionProvider.lambda$11(PropertiesDefaultDescriptionProvider.java:148) ~[classes/:na]
    at org.eclipse.sirius.components.forms.components.ForComponent.render(ForComponent.java:45) ~[classes/:na]
  1. Reverse the order: select first the node style and then add an element from Robot to the selection => :boom: with a different stack:
java.lang.ClassCastException: class fr.obeo.dsl.designer.sample.flow.impl.CompositeProcessorImpl cannot be cast to class org.eclipse.sirius.components.view.NodeStyle (fr.obeo.dsl.designer.sample.flow.impl.CompositeProcessorImpl is in unnamed module of loader 'app'; org.eclipse.sirius.components.view.NodeStyle is in unnamed module of loader org.springframework.boot.devtools.restart.classloader.RestartClassLoader @53b8ea95)
    at org.eclipse.sirius.components.emf.view.diagram.NodeStylePropertiesConfigurer.lambda$35(NodeStylePropertiesConfigurer.java:190) ~[classes/:na]
    at java.base/java.util.Optional.map(Optional.java:265) ~[na:na]
    at org.eclipse.sirius.components.emf.view.diagram.NodeStylePropertiesConfigurer.lambda$78(NodeStylePropertiesConfigurer.java:316) ~[classes/:na]
    at org.eclipse.sirius.components.forms.components.TextfieldComponent.render(TextfieldComponent.java:50) ~[classes/:na]
    at org.eclipse.sirius.components.representations.BaseRenderer.renderComponent(BaseRenderer.java:200) ~[classes/:na]

Expected behavior

When selecting multiple elements I should get one page per element, each rendered using the appropriate Form implementation registered to handle that type.

Actual behavior

pcdavid commented 2 years ago

This is caused by different parts of the code which are note fully compatible with multi-selection.

NodeStylePropertiesConfigurer which only tests for the type of the first element in the selection in its canCreatePagePredicate. If the first element is a NodeStyle, the page will be instanciated, and then its semanticElementsProvider will be invoked for every element oin the selection, but that provider assumes every element in the selection is a NodeStyle (there used to be only one, so that was always the case if canCreatePagePredicate had returned true earlier. This explains the second crash above.

The first stack is unrelated to multi-selection, but caused by https://github.com/eclipse-sirius/sirius-components/issues/1033 not having created the _UI_BorderStyle_borderColor_feature key (and probably others) when regenerating the metamodel. It is not visible in "normal" usage as NodeStylePropertiesConfigurer does not use this to create the label for the "Border Color" attribute's widget.

However since the first element in the selection is an element from Flow, NodeStylePropertiesConfigurer's canCreatePagePredicate return false, which triggers the use of the default generic/reflective rules, which use EMF's item providers.

Another related issue seen while analysing this: even after fixing the invalid semanticElementsProvider, there is an incorrect behavior when selecting first a NodeStyle and then a Flow element. Because the first selection element returns a non-empty form, the use of the default provider for the other elements in the selection where it may apply is completelyn bypassed in PropertiesEventProcessorFactory.createRepresentationEventProcessor():

Optional<FormDescription> optionalFormDescription = Optional.empty();
if (!formDescriptions.isEmpty()) {
    optionalFormDescription = new FormDescriptionAggregator().aggregate(formDescriptions, objects, this.objectService);
}
FormDescription formDescription = optionalFormDescription.orElse(this.propertiesDefaultDescriptionProvider.getFormDescription());

Here FormDescriptionAggregator finds at least one match (the custom page for the NodeStyle), so it does not try to apply the default rules for the other parts of the selection.