diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index a2f19a3fd1..1f674a3548 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -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 diff --git a/packages/trees/backend/sirius-components-trees/src/main/java/org/eclipse/sirius/components/trees/renderer/TreeRenderer.java b/packages/trees/backend/sirius-components-trees/src/main/java/org/eclipse/sirius/components/trees/renderer/TreeRenderer.java index 17a891a872..30dc35999e 100644 --- a/packages/trees/backend/sirius-components-trees/src/main/java/org/eclipse/sirius/components/trees/renderer/TreeRenderer.java +++ b/packages/trees/backend/sirius-components-trees/src/main/java/org/eclipse/sirius/components/trees/renderer/TreeRenderer.java @@ -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. @@ -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; @@ -50,9 +62,13 @@ public Tree render() { List rootElements = this.treeDescription.getElementsProvider().apply(this.variableManager); List 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)); } @@ -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 iconURL = this.treeDescription.getIconURLProvider().apply(treeItemVariableManager); - Boolean hasChildren = this.treeDescription.getHasChildrenProvider().apply(treeItemVariableManager); + List 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 childrenTreeItems = new ArrayList<>(children.size()); + int childIndex = 0; boolean expanded = !children.isEmpty(); for (Object child : children) { VariableManager childVariableManager = treeItemVariableManager.createChild(); + List childAncestors = new ArrayList(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 iconURL, VariableManager currentVariableManager) { + this.logger.warn("Possible infinite loop encountered during tree rendering. Item with id '{}' has already been encountered as ancestors", id); + + List fragments = new ArrayList<>(label.styledStringFragments()); + fragments.add(0, new StyledStringFragment(" ", StyledStringFragmentStyle.newDefaultStyledStringFragmentStyle() + .foregroundColor("#ff8c00") + .build())); + StyledString newLabel = new StyledString(fragments); + + List 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 true 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 + } } diff --git a/packages/trees/backend/sirius-components-trees/src/test/java/org/eclipse/sirius/components/trees/renderer/TreeRendererTests.java b/packages/trees/backend/sirius-components-trees/src/test/java/org/eclipse/sirius/components/trees/renderer/TreeRendererTests.java new file mode 100644 index 0000000000..eecbc81b3f --- /dev/null +++ b/packages/trees/backend/sirius-components-trees/src/test/java/org/eclipse/sirius/components/trees/renderer/TreeRendererTests.java @@ -0,0 +1,249 @@ +/******************************************************************************* + * Copyright (c) 2024 Obeo. + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Obeo - initial API and implementation + *******************************************************************************/ +package org.eclipse.sirius.components.trees.renderer; + +import static java.util.stream.Collectors.joining; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Optional; +import java.util.Set; + +import org.eclipse.sirius.components.core.api.labels.StyledString; +import org.eclipse.sirius.components.core.api.labels.StyledStringFragment; +import org.eclipse.sirius.components.representations.Success; +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.junit.jupiter.api.Test; + +/** + * Test class for the {@link TreeRenderer}. + * + * @author Arthur Daussy + */ +public class TreeRendererTests { + + private static final String ITEM_RENDERING_LOOP_LABEL = " root"; + + private static final String CHILD1X1_ID = "child1x1"; + + private static final String CHILD2_ID = "child2"; + + private static final String CHILD1_ID = "child1"; + + private static final String ROOT_LABEL = "root"; + + private static final String NODE_KIND = "node"; + + private static final String FAKE_ID = "FakeId"; + + @Test + public void basicRenderingOneRoot() { + + TreeNode root = new TreeNode(ROOT_LABEL); + TreeDescription description = this.createDescription(root); + VariableManager variableManager = new VariableManager(); + Tree tree = new TreeRenderer(variableManager, description).render(); + + assertEquals(FAKE_ID, tree.getLabel()); + assertEquals(1, tree.getChildren().size()); + this.assertTreeNode(tree.getChildren().get(0), ROOT_LABEL, 0); + + } + + @Test + public void basicRenderingSmallTree() { + + TreeNode root = new TreeNode(ROOT_LABEL); + + TreeNode child1 = root.createContainementChildren(CHILD1_ID); + TreeNode child2 = root.createContainementChildren(CHILD2_ID); + + TreeNode child1x1 = child1.createContainementChildren(CHILD1X1_ID); + + child2.addReferenceChildren(child1x1); + + TreeDescription description = this.createDescription(root); + VariableManager variableManager = new VariableManager(); + Tree tree = new TreeRenderer(variableManager, description).render(); + + assertEquals(FAKE_ID, tree.getLabel()); + assertEquals(1, tree.getChildren().size()); + TreeItem rootTreeItem = tree.getChildren().get(0); + this.assertTreeNode(rootTreeItem, ROOT_LABEL, 2); + + TreeItem child1TreeItem = rootTreeItem.getChildren().get(0); + this.assertTreeNode(child1TreeItem, CHILD1_ID, 1); + + TreeItem child1x1TreeItem = child1TreeItem.getChildren().get(0); + this.assertTreeNode(child1x1TreeItem, CHILD1X1_ID, 0); + + TreeItem child2TreeItem = rootTreeItem.getChildren().get(1); + this.assertTreeNode(child2TreeItem, CHILD2_ID, 1); + + TreeItem refChild1x1TreeItem = child2TreeItem.getChildren().get(0); + this.assertTreeNode(refChild1x1TreeItem, CHILD1X1_ID, 0); + + } + + /** + * This test aims to check that the TreeRendering do not throw a StackOverflow exception when a tree to be rendered + * has loops. It also verify that the virtual nodes used to report a problem to the user all have unique ids. + */ + @Test + public void checkAvoidStackOverFlow() { + TreeNode root = new TreeNode(ROOT_LABEL); + + TreeNode child1 = root.createContainementChildren(CHILD1_ID); + TreeNode child2 = root.createContainementChildren(CHILD2_ID); + + child1.addReferenceChildren(root); + child1.addReferenceChildren(root); + child2.addReferenceChildren(root); + + TreeDescription description = this.createDescription(root); + VariableManager variableManager = new VariableManager(); + Tree tree = new TreeRenderer(variableManager, description).render(); + + assertEquals(FAKE_ID, tree.getLabel()); + assertEquals(1, tree.getChildren().size()); + TreeItem rootTreeItem = tree.getChildren().get(0); + this.assertTreeNode(rootTreeItem, ROOT_LABEL, 2); + + TreeItem child1TreeItem = rootTreeItem.getChildren().get(0); + this.assertTreeNode(child1TreeItem, CHILD1_ID, 2); + + TreeItem refRoot1x1 = child1TreeItem.getChildren().get(0); + this.assertTreeNode(refRoot1x1, ITEM_RENDERING_LOOP_LABEL, 0); + TreeItem refRoot1x2 = child1TreeItem.getChildren().get(1); + this.assertTreeNode(refRoot1x2, ITEM_RENDERING_LOOP_LABEL, 0); + + TreeItem child2TreeItem = rootTreeItem.getChildren().get(1); + this.assertTreeNode(child2TreeItem, CHILD2_ID, 1); + + TreeItem refRoot2 = child2TreeItem.getChildren().get(0); + this.assertTreeNode(refRoot2, ITEM_RENDERING_LOOP_LABEL, 0); + + // All ids are unique + assertEquals(3, Set.of(refRoot1x1.getId(), refRoot1x2.getId(), refRoot2.getId()).size()); + + } + + private TreeDescription createDescription(TreeNode root) { + // @formatter:off + return TreeDescription.newTreeDescription(FAKE_ID) + .canCreatePredicate(v -> true) + .childrenProvider(v -> this.getSelfNode(v).getChildren()) + .deletableProvider(v -> false) + .deleteHandler(v -> new Success()) + .editableProvider(v -> true).elementsProvider(v -> List.of(root)) + .hasChildrenProvider(v -> !this.getSelfNode(v).getChildren().isEmpty()) + .iconURLProvider(v -> List.of()) + .idProvider(v -> FAKE_ID) + .kindProvider(v -> NODE_KIND) + .label("Fake tree") + .labelProvider(v -> StyledString.of(v.get(VariableManager.SELF, TreeNode.class).map(TreeNode::getId).orElse(FAKE_ID))) + .parentObjectProvider(v -> this.getSelfNode(v).getParent()) + .renameHandler((v, name) -> new Success()) + .selectableProvider(v -> true) + .targetObjectIdProvider(v -> v.get(VariableManager.SELF, TreeNode.class).map(TreeNode::getId).orElse(FAKE_ID)) + .treeItemIdProvider(v -> this.getSelfNode(v).getId()) + .treeItemObjectProvider(v -> root.search(v.get("treeItemId", String.class).get(), new HashSet<>())) + .build(); + // @formatter:on + + } + + private void assertTreeNode(TreeItem treeItem, String expectedSimpleLabel, int exepectedNbChildren) { + assertEquals(expectedSimpleLabel, treeItem.getLabel().styledStringFragments().stream().map(StyledStringFragment::text).collect(joining())); + assertEquals(NODE_KIND, treeItem.getKind()); + assertEquals(exepectedNbChildren, treeItem.getChildren().size()); + } + + private TreeNode getSelfNode(VariableManager mng) { + return mng.get(VariableManager.SELF, TreeNode.class).get(); + } + + /** + * Item used to create simple tree. + * + * @author Arthur Daussy + */ + private static final class TreeNode { + + private final String id; + + private final List children = new ArrayList<>(); + + private TreeNode parent; + + private TreeNode(String id) { + super(); + this.id = id; + } + + public TreeNode getParent() { + return this.parent; + } + + public void setParent(TreeNode parent) { + this.parent = parent; + } + + public TreeNode createContainementChildren(String childId) { + TreeNode child = new TreeNode(childId); + this.children.add(child); + child.setParent(this); + return child; + } + + public void addReferenceChildren(TreeNode node) { + this.children.add(node); + } + + public String getId() { + return this.id; + } + + public List getChildren() { + return this.children; + } + + public Optional search(String searchId, Set searchNodeId) { + Optional result = Optional.empty(); + if (searchNodeId.contains(this.id)) { + result = Optional.empty(); + } else if (this.id.equals(searchId)) { + result = Optional.of(this); + } else { + searchNodeId.add(this.id); + + for (TreeNode child : this.children) { + result = child.search(searchId, searchNodeId); + if (result.isPresent()) { + return result; + } + } + + } + + return result; + } + + } + +}