diff --git a/flow-server/src/main/java/com/vaadin/flow/component/internal/UIInternals.java b/flow-server/src/main/java/com/vaadin/flow/component/internal/UIInternals.java index 402a7cec5e7..d1f4f337c84 100644 --- a/flow-server/src/main/java/com/vaadin/flow/component/internal/UIInternals.java +++ b/flow-server/src/main/java/com/vaadin/flow/component/internal/UIInternals.java @@ -382,13 +382,7 @@ public void setSession(VaadinSession session) { + "."); } else { if (session == null) { - try { - ComponentUtil.onComponentDetach(ui); - ui.getChildren().forEach(ComponentUtil::onComponentDetach); - } catch (Exception e) { - getLogger().warn("Error while detaching UI from session", - e); - } + ui.getElement().getNode().setParent(null); // Disable push when the UI is detached. Otherwise the // push connection and possibly VaadinSession will live on. ui.getPushConfiguration().setPushMode(PushMode.DISABLED); @@ -397,7 +391,9 @@ public void setSession(VaadinSession session) { this.session = session; } - if (session != null) { + if (session != null) + + { ComponentUtil.onComponentAttach(ui, true); } } @@ -853,8 +849,9 @@ public void addComponentDependencies( } else { // In npm mode, add external JavaScripts directly to the page. dependencies.getJavaScripts().stream() - .filter(js -> UrlUtil.isExternal(js.value())).forEach(js -> page - .addJavaScript(js.value(), js.loadMode())); + .filter(js -> UrlUtil.isExternal(js.value())) + .forEach(js -> page.addJavaScript(js.value(), + js.loadMode())); dependencies.getJsModules().stream() .filter(js -> UrlUtil.isExternal(js.value())) .forEach(js -> page.addJsModule(js.value(), js.loadMode())); diff --git a/flow-server/src/main/java/com/vaadin/flow/internal/StateNode.java b/flow-server/src/main/java/com/vaadin/flow/internal/StateNode.java index b5b6f1d6409..84209ea622c 100644 --- a/flow-server/src/main/java/com/vaadin/flow/internal/StateNode.java +++ b/flow-server/src/main/java/com/vaadin/flow/internal/StateNode.java @@ -365,8 +365,8 @@ protected void setTree(StateTree tree) { } /** - * Removes the node from its parent and unlinks the node (and children) - * from the state tree. + * Removes the node from its parent and unlinks the node (and children) from + * the state tree. */ public void removeFromTree() { visitNodeTree(StateNode::reset); @@ -676,10 +676,10 @@ private void doSetTree(StateTree tree) { if (owner instanceof StateTree) { throw new IllegalStateException( - "Can't move a node from one state tree to another. " + - "If this is intentional, first remove the " + - "node from its current state tree by calling " + - "removeFromTree"); + "Can't move a node from one state tree to another. " + + "If this is intentional, first remove the " + + "node from its current state tree by calling " + + "removeFromTree"); } owner = tree; } diff --git a/flow-server/src/main/java/com/vaadin/flow/internal/StateTree.java b/flow-server/src/main/java/com/vaadin/flow/internal/StateTree.java index 56c0b84c959..5eddf66c85d 100644 --- a/flow-server/src/main/java/com/vaadin/flow/internal/StateTree.java +++ b/flow-server/src/main/java/com/vaadin/flow/internal/StateTree.java @@ -45,6 +45,7 @@ public class StateTree implements NodeOwner { private final class RootNode extends StateNode { + private RootNode(Class[] features) { super(features); @@ -57,14 +58,22 @@ private RootNode(Class[] features) { @Override public void setParent(StateNode parent) { - throw new IllegalStateException( - "Can't set the parent of the tree root"); + if (parent == null) { + super.setParent(null); + isRootAttached = false; + } else { + throw new IllegalStateException( + "Can't set the parent of the tree root"); + } } @Override public boolean isAttached() { - // Root is always attached - return true; + // the method is called from the super class (which is bad: call + // overridden method at the class init phase), so there the field + // from enclosing class is used because it's already initialized at + // the class constructor call. + return isRootAttached; } } @@ -135,6 +144,11 @@ public interface ExecutionRegistration extends Registration { private final UIInternals uiInternals; + // This field actually belongs to RootNode class but it can'be moved there + // because its method isAttached() is called before the RootNode class is + // initialization is done. + private boolean isRootAttached = true; + /** * Creates a new state tree with a set of features defined for the root * node. diff --git a/flow-server/src/test/java/com/vaadin/flow/component/UITest.java b/flow-server/src/test/java/com/vaadin/flow/component/UITest.java index 1c1fba960dc..1c75fc5442a 100644 --- a/flow-server/src/test/java/com/vaadin/flow/component/UITest.java +++ b/flow-server/src/test/java/com/vaadin/flow/component/UITest.java @@ -21,6 +21,7 @@ import com.vaadin.flow.component.page.History; import com.vaadin.flow.component.page.History.HistoryStateChangeEvent; import com.vaadin.flow.dom.Element; +import com.vaadin.flow.dom.ElementDetachEvent; import com.vaadin.flow.dom.Node; import com.vaadin.flow.dom.NodeVisitor; import com.vaadin.flow.dom.impl.AbstractTextElementStateProvider; @@ -179,7 +180,8 @@ private static void initUI(UI ui, String initialLocation, routeConfiguration.update(() -> { routeConfiguration.getHandledRegistry().clean(); Arrays.asList(RootNavigationTarget.class, - FooBarNavigationTarget.class).forEach(routeConfiguration::setAnnotatedRoute); + FooBarNavigationTarget.class) + .forEach(routeConfiguration::setAnnotatedRoute); }); ui.doInit(request, 0); @@ -198,7 +200,8 @@ private static void initUI(UI ui, String initialLocation, @Test public void scrollAttribute() { UI ui = new UI(); - Assert.assertNull("'scroll' attribute shouldn't be set for the " + Assert.assertNull( + "'scroll' attribute shouldn't be set for the " + "UI element which represents 'body' tag", ui.getElement().getAttribute("scroll")); } @@ -263,8 +266,8 @@ public void locationAfterClientNavigation() History history = ui.getPage().getHistory(); - history.getHistoryStateChangeHandler().onHistoryStateChange( - new HistoryStateChangeEvent(history, null, + history.getHistoryStateChangeHandler() + .onHistoryStateChange(new HistoryStateChangeEvent(history, null, new Location("foo/bar"), NavigationTrigger.HISTORY)); assertEquals("foo/bar", @@ -364,6 +367,29 @@ public void unsetSession_detachEventIsFiredForUIChildren() assertEquals(childComponent, events.get(0).getSource()); } + @Test + public void unserSession_datachEventIsFiredForElements() { + UI ui = createTestUI(); + + List events = new ArrayList<>(); + + ui.getElement().addDetachListener(events::add); + initUI(ui, "", null); + + Component childComponent = new AttachableComponent(); + ui.add(childComponent); + childComponent.getElement().addDetachListener(events::add); + + ui.getSession().access(() -> ui.getInternals().setSession(null)); + + // Unlock to run pending access tasks + ui.getSession().unlock(); + + assertEquals(2, events.size()); + assertEquals(childComponent.getElement(), events.get(0).getSource()); + assertEquals(ui.getElement(), events.get(1).getSource()); + } + @Test public void beforeClientResponse_regularOrder() { UI ui = createTestUI(); @@ -708,9 +734,8 @@ public void accessLaterRunnable_attachedUnlockedUi_runnableIsRun() { public void accessLaterRunnable_detachedUiNoHandler_throws() { UI ui = createTestUI(); - SerializableRunnable wrapped = ui - .accessLater(() -> Assert.fail("Action should never run"), - null); + SerializableRunnable wrapped = ui.accessLater( + () -> Assert.fail("Action should never run"), null); wrapped.run(); } @@ -720,9 +745,9 @@ public void accessLaterRunnable_detachedUi_detachHandlerCalled() { UI ui = createTestUI(); - SerializableRunnable wrapped = ui - .accessLater(() -> Assert.fail("Action should never run"), - runCount::incrementAndGet); + SerializableRunnable wrapped = ui.accessLater( + () -> Assert.fail("Action should never run"), + runCount::incrementAndGet); assertEquals("Handler should not yet have run", 0, runCount.get()); @@ -766,9 +791,8 @@ public void accessLaterConsumer_attachedUnlockedUi_runnableIsRun() { public void accessLaterConsumer_detachedUiNoHandler_throws() { UI ui = createTestUI(); - SerializableConsumer wrapped = ui - .accessLater(value -> Assert.fail("Action should never run"), - null); + SerializableConsumer wrapped = ui.accessLater( + value -> Assert.fail("Action should never run"), null); wrapped.accept(null); } @@ -778,9 +802,9 @@ public void accessLaterConsumer_detachedUi_detachHandlerCalled() { UI ui = createTestUI(); - SerializableConsumer wrapped = ui - .accessLater(value -> Assert.fail("Action should never run"), - runCount::incrementAndGet); + SerializableConsumer wrapped = ui.accessLater( + value -> Assert.fail("Action should never run"), + runCount::incrementAndGet); assertEquals("Handler should not yet have run", 0, runCount.get()); diff --git a/flow-server/src/test/java/com/vaadin/flow/internal/StateTreeTest.java b/flow-server/src/test/java/com/vaadin/flow/internal/StateTreeTest.java index 57775c9e5ad..a2db7c1e0da 100644 --- a/flow-server/src/test/java/com/vaadin/flow/internal/StateTreeTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/internal/StateTreeTest.java @@ -24,6 +24,7 @@ import java.util.List; import java.util.Optional; import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; import org.apache.commons.lang3.SerializationUtils; @@ -93,7 +94,7 @@ protected void populateJson(JsonObject json, } @Test - public void testRootNodeState() { + public void rootNodeState() { StateNode rootNode = tree.getRootNode(); Assert.assertNull("Root node should have no parent", @@ -109,10 +110,23 @@ public void testRootNodeState() { } @Test(expected = IllegalStateException.class) - public void testRootNode_setParent_throws() { + public void rootNode_setStateNodeAsParent_throws() { tree.getRootNode().setParent(new StateNode()); } + @Test + public void rootNode_setNullAsParent_nodeIsDetached() { + AtomicInteger detachCount = new AtomicInteger(); + Assert.assertTrue(tree.hasNode(tree.getRootNode())); + tree.getRootNode() + .addDetachListener(() -> detachCount.incrementAndGet()); + tree.getRootNode().setParent(null); + Assert.assertEquals(1, detachCount.get()); + Assert.assertFalse(tree.getRootNode().isAttached()); + + Assert.assertFalse(tree.hasNode(tree.getRootNode())); + } + @Test public void attachedNodeIsAttached() { StateNode node = StateNodeTest.createEmptyNode();