Skip to content

Commit

Permalink
[3958] Prevent Stackoverflow when rendering a twice an expanded TreeItem
Browse files Browse the repository at this point in the history
This commit provide a partial fix for the given bug. It only prevents
rendering twice in the same tree path and expanded element because it
would cause an infinite loop.

A fix still need to be implemented in the "Expand All" action to prevent
stackoverfow when rendering a infinite loop with non unique ID.

Bug: eclipse-sirius#3958
Signed-off-by: Arthur Daussy <arthur.daussy@obeo.fr>
  • Loading branch information
adaussy committed Sep 18, 2024
1 parent c03981c commit 25495ff
Show file tree
Hide file tree
Showing 4 changed files with 369 additions and 12 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ Both `IInput` and `IDomainEvent` implement `ICause` and will thus be used to ind
- https://github.com/eclipse-sirius/sirius-web/issues/3954[#3954] [diagram] Ensure separator bar is displayed immediately after dynamically adding list child
- https://github.com/eclipse-sirius/sirius-web/issues/4001[#4001] [diagram] Fix an issue with label text overlapped by icon
- https://github.com/eclipse-sirius/sirius-web/issues/3965[#3965] [diagram] Fix an issue where the label height was not evaluated to define the node size on the first layout

- https://github.com/eclipse-sirius/sirius-web/issues/3958[#3958] [tree] Partial fix of stack overflow when invoking "Expand All" on a domain

=== New Features

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,21 @@
*******************************************************************************/
package org.eclipse.sirius.components.trees.renderer;

import static java.util.stream.Collectors.joining;

import java.util.ArrayList;
import java.util.List;
import java.util.Objects;

import org.eclipse.sirius.components.core.api.labels.StyledString;
import org.eclipse.sirius.components.core.api.labels.StyledStringFragment;
import org.eclipse.sirius.components.core.api.labels.StyledStringFragmentStyle;
import org.eclipse.sirius.components.representations.VariableManager;
import org.eclipse.sirius.components.trees.Tree;
import org.eclipse.sirius.components.trees.TreeItem;
import org.eclipse.sirius.components.trees.description.TreeDescription;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Renderer used to create the tree from its description and some variables.
Expand All @@ -30,10 +35,20 @@
*/
public class TreeRenderer {

public static final String ANCESTOR_IDS = "ancestorIds";

public static final String INDEX = "index";

public static final String EXPANDED = "expanded";

public static final String ACTIVE_FILTER_IDS = "activeFilterIds";

private static final Logger LOGGER = LoggerFactory.getLogger(TreeRenderer.class);

private static final StyledStringFragment ITEM_RENDERING_LOOP_FRAGMENT = new StyledStringFragment("<Item rendering loop> ", StyledStringFragmentStyle.newDefaultStyledStringFragmentStyle()
.foregroundColor("#ff8c00")
.build());

private final VariableManager variableManager;

private final TreeDescription treeDescription;
Expand All @@ -50,9 +65,11 @@ public Tree render() {

List<?> rootElements = this.treeDescription.getElementsProvider().apply(this.variableManager);
List<TreeItem> childrenItems = new ArrayList<>(rootElements.size());
int index = 0;
for (Object rootElement : rootElements) {
VariableManager rootElementVariableManager = this.variableManager.createChild();
rootElementVariableManager.put(VariableManager.SELF, rootElement);
rootElementVariableManager.put(INDEX, index++);
childrenItems.add(this.renderTreeItem(rootElementVariableManager));
}

Expand All @@ -72,27 +89,75 @@ private TreeItem renderTreeItem(VariableManager treeItemVariableManager) {
boolean deletable = this.treeDescription.getDeletableProvider().apply(treeItemVariableManager);
boolean selectable = this.treeDescription.getSelectableProvider().apply(treeItemVariableManager);
List<String> iconURL = this.treeDescription.getIconURLProvider().apply(treeItemVariableManager);
Boolean hasChildren = this.treeDescription.getHasChildrenProvider().apply(treeItemVariableManager);

if (this.loopDetected(treeItemVariableManager, id)) {
return this.renderWarningTreeItem(id, kind, label, iconURL, treeItemVariableManager);
}

Boolean hasChildren = this.treeDescription.getHasChildrenProvider().apply(treeItemVariableManager);
List<?> children = this.treeDescription.getChildrenProvider().apply(treeItemVariableManager);
List<TreeItem> childrenTreeItems = new ArrayList<>(children.size());

int childIndex = 0;
boolean expanded = !children.isEmpty();
for (Object child : children) {
VariableManager childVariableManager = treeItemVariableManager.createChild();
List<Object> childAncestors = new ArrayList<Object>(treeItemVariableManager.get(ANCESTOR_IDS, List.class).orElse(List.of()));
childAncestors.add(id);
childVariableManager.put(ANCESTOR_IDS, childAncestors);
childVariableManager.put(VariableManager.SELF, child);
childVariableManager.put(INDEX, childIndex++);
childrenTreeItems.add(this.renderTreeItem(childVariableManager));
}
return TreeItem.newTreeItem(id)
return TreeItem.newTreeItem(id).kind(kind).label(label).editable(editable).deletable(deletable).selectable(selectable).iconURL(iconURL).children(childrenTreeItems).hasChildren(hasChildren)
.expanded(expanded).build();
}

private TreeItem renderWarningTreeItem(String id, String kind, StyledString label, List<String> iconURL, VariableManager mng) {
LOGGER.warn("Possible infinite loop encountered during tree rendering. Item with id '{}' has already been encountered as ancestors", id);

List<StyledStringFragment> fragments = new ArrayList<>(label.styledStringFragments());
fragments.add(0, ITEM_RENDERING_LOOP_FRAGMENT);
StyledString newLabel = new StyledString(fragments);
List<String> newIcons = new ArrayList<>(iconURL);
newIcons.add("/icons/svg/warning-overlay.svg");

List<Object> idPath = mng.get(ANCESTOR_IDS, List.class).orElse(new ArrayList<>());
idPath.add(id);
String path = idPath.stream().map(Object::toString).collect(joining("::")).toString() + "#" + mng.get(INDEX, Integer.class);

// This would need to be encoded to avoid getting a problem with id containing #
String virtualId = "siriuscomponent://treelooprendering?itemId=" + id + "&path=" + path;

// @formatter:off
return TreeItem.newTreeItem(virtualId)
.kind(kind)
.label(label)
.editable(editable)
.deletable(deletable)
.selectable(selectable)
.iconURL(iconURL)
.children(childrenTreeItems)
.hasChildren(hasChildren)
.expanded(expanded)
.label(newLabel)
.editable(false)
.deletable(false)
.selectable(false)
.iconURL(newIcons)
.children(List.of())
.hasChildren(false)
.expanded(false)
.build();
// @formatter:on
}

/**
* Checks if the id of the current item has already been used by its ancestors.
*
* @param treeItemVariableManager
* the VariableManager
* @param id
* the current TreeItem id
* @return <code>true</code> if a loop is detected
*/
private boolean loopDetected(VariableManager treeItemVariableManager, String id) {
// @formatter:off
return treeItemVariableManager.get(ANCESTOR_IDS, List.class)
.orElse(List.of()).stream()
.anyMatch(ancestorId -> Objects.equals(ancestorId, id));
// @formatter:on
}
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading

0 comments on commit 25495ff

Please sign in to comment.