Skip to content

Commit

Permalink
fix: Break loop in visitor (#13370)
Browse files Browse the repository at this point in the history
Do not constantly add children
of a node that has already had
all children added once.

Fixes #13366
  • Loading branch information
caalador authored and vaadin-bot committed Mar 28, 2022
1 parent fdf901e commit f2616d2
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -705,15 +705,19 @@ void visitNodeTreeBottomUp(Consumer<StateNode> visitor) {
// not done inside loop to please Sonarcube
forEachChild(stack::addFirst);
StateNode previousParent = this;
Set<StateNode> childrenAdded = new HashSet<>();

while (!stack.isEmpty()) {
StateNode current = stack.getFirst();
assert current != null;
if (current == previousParent) {
visitor.accept(stack.removeFirst());
previousParent = current.getParent();
} else if (childrenAdded.contains(current)) {
previousParent = current;
} else {
current.forEachChild(stack::addFirst);
childrenAdded.add(current);
previousParent = current;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.vaadin.flow.internal;

import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
Expand Down Expand Up @@ -296,6 +297,35 @@ public void nodeTreeOnAttach_bottomUpTraversing_correctOrder() {
data.removeLast()));
}

@Test
public void nodeTreeOnAttach_bottomUpTraversing_brokenParentInChildDoesNotEndInLoop()
throws NoSuchFieldException, IllegalAccessException {
// Set data is used to track the node during debug see
// TestStateNode.toString
TestStateNode root = new TestStateNode();
root.setData(0);
List<Integer> count = new ArrayList<>();

final Field parent = StateNode.class.getDeclaredField("parent");
parent.setAccessible(true);

TestStateNode childOfRoot = new TestStateNode();
childOfRoot.setData(1);

TestStateNode child = new TestStateNode();
child.setData(2);
setParent(child, childOfRoot);

parent.set(child, null);

setParent(childOfRoot, root);

root.visitNodeTreeBottomUp(node -> count.add(1));

Assert.assertEquals("Each node should be visited once", 3,
count.size());
}

@Test
public void attachListener_onSetParent_listenerTriggered() {
StateNode root = new TestStateTree().getRootNode();
Expand Down

0 comments on commit f2616d2

Please sign in to comment.