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 20, 2024
1 parent c03981c commit 2b10030
Show file tree
Hide file tree
Showing 3 changed files with 326 additions and 6 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 a StackOverflow when expanding an item contained in rendering loop cycle

=== New Features

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,22 @@
*******************************************************************************/
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 java.util.UUID;

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 +36,16 @@
*/
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 final Logger logger = LoggerFactory.getLogger(TreeRenderer.class);

private final VariableManager variableManager;

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

List<?> rootElements = this.treeDescription.getElementsProvider().apply(this.variableManager);
List<TreeItem> childrenItems = new ArrayList<>(rootElements.size());

this.variableManager.put(ANCESTOR_IDS, new ArrayList<>());
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 @@ -71,28 +87,83 @@ private TreeItem renderTreeItem(VariableManager treeItemVariableManager) {
boolean editable = this.treeDescription.getEditableProvider().apply(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);
List<String> iconURLs = this.treeDescription.getIconURLProvider().apply(treeItemVariableManager);

if (this.loopDetected(treeItemVariableManager, id)) {
return this.renderWarningTreeItem(id, kind, label, iconURLs, 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)
.iconURL(iconURLs)
.children(childrenTreeItems)
.hasChildren(hasChildren)
.expanded(expanded)
.build();
}

private TreeItem renderWarningTreeItem(String id, String kind, StyledString label, List<String> iconURL, VariableManager currentVariableManager) {
this.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, new StyledStringFragment("<Item rendering loop> ", StyledStringFragmentStyle.newDefaultStyledStringFragmentStyle()
.foregroundColor("#ff8c00")
.build()));
StyledString newLabel = new StyledString(fragments);

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

String virtualId = "siriuscomponent://treelooprendering?renderedItemId=" + id + "&itemItem=" + UUID.nameUUIDFromBytes(path.getBytes());

return TreeItem.newTreeItem(virtualId)
.kind(kind)
.label(newLabel)
.editable(false)
.deletable(false)
.selectable(false)
.iconURL(iconURL)
.children(List.of())
.hasChildren(false)
.expanded(false)
.build();
}

/**
* 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

0 comments on commit 2b10030

Please sign in to comment.