From f5ad3a10c9f449a9cddcd0d94a323e91a096faad Mon Sep 17 00:00:00 2001 From: Henri Sara Date: Wed, 6 Sep 2017 15:22:20 +0300 Subject: Fix making components visible by push (#9934) Fix connector tracker cleanup for the case where a component is hidden by a request and is made visible again by push. This fixes a regression caused by #9305. Fixes #9905 --- .../main/java/com/vaadin/server/VaadinSession.java | 2 +- .../vaadin/server/communication/UidlWriter.java | 1 + .../main/java/com/vaadin/ui/ConnectorTracker.java | 24 ++++- .../application/MissingHierarchyDetection.java | 43 --------- .../TableRemovedQuicklySendsInvalidRpcCalls.java | 5 + .../tests/push/MakeComponentVisibleWithPush.java | 104 +++++++++++++++++++++ .../application/MissingHierarchyDetectionTest.java | 7 +- .../push/MakeComponentVisibleWithPushTest.java | 41 ++++++++ 8 files changed, 173 insertions(+), 54 deletions(-) create mode 100644 uitest/src/main/java/com/vaadin/tests/push/MakeComponentVisibleWithPush.java create mode 100644 uitest/src/test/java/com/vaadin/tests/push/MakeComponentVisibleWithPushTest.java diff --git a/server/src/main/java/com/vaadin/server/VaadinSession.java b/server/src/main/java/com/vaadin/server/VaadinSession.java index 671b35ae66..26b9cfa7a1 100644 --- a/server/src/main/java/com/vaadin/server/VaadinSession.java +++ b/server/src/main/java/com/vaadin/server/VaadinSession.java @@ -1028,7 +1028,7 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable { } } try { - ui.getConnectorTracker().cleanConnectorMap(); + ui.getConnectorTracker().cleanConnectorMap(false); } catch (AssertionError | Exception e) { getLogger().log(Level.SEVERE, "Exception while cleaning connector map for ui " diff --git a/server/src/main/java/com/vaadin/server/communication/UidlWriter.java b/server/src/main/java/com/vaadin/server/communication/UidlWriter.java index d27fc3f0ae..92b6e8e8bd 100644 --- a/server/src/main/java/com/vaadin/server/communication/UidlWriter.java +++ b/server/src/main/java/com/vaadin/server/communication/UidlWriter.java @@ -321,6 +321,7 @@ public class UidlWriter implements Serializable { writePerformanceData(ui, writer); } finally { uiConnectorTracker.setWritingResponse(false); + uiConnectorTracker.cleanConnectorMap(true); } } diff --git a/server/src/main/java/com/vaadin/ui/ConnectorTracker.java b/server/src/main/java/com/vaadin/ui/ConnectorTracker.java index c41c0d0fd5..54fb5c5822 100644 --- a/server/src/main/java/com/vaadin/ui/ConnectorTracker.java +++ b/server/src/main/java/com/vaadin/ui/ConnectorTracker.java @@ -71,7 +71,7 @@ public class ConnectorTracker implements Serializable { /** * Connectors that have been unregistered and should be cleaned up the next - * time {@link #cleanConnectorMap()} is invoked unless they have been + * time {@link #cleanConnectorMap(boolean)} is invoked unless they have been * registered again. */ private final Set unregisteredConnectors = new HashSet<>(); @@ -262,14 +262,30 @@ public class ConnectorTracker implements Serializable { return null; } + /** + * Cleans the connector map from all connectors that are no longer attached + * to the application if there are dirty connectors or the force flag is + * true. This should only be called by the framework. + * + * @param force + * {@code true} to force cleaning + * @since + */ + public void cleanConnectorMap(boolean force) { + if (force || !dirtyConnectors.isEmpty()) { + cleanConnectorMap(); + } + } + /** * Cleans the connector map from all connectors that are no longer attached * to the application. This should only be called by the framework. + * + * @deprecated use {@link #cleanConnectorMap(boolean)} instead */ + @Deprecated public void cleanConnectorMap() { - if (!unregisteredConnectors.isEmpty()) { - removeUnregisteredConnectors(); - } + removeUnregisteredConnectors(); cleanStreamVariables(); diff --git a/uitest/src/main/java/com/vaadin/tests/application/MissingHierarchyDetection.java b/uitest/src/main/java/com/vaadin/tests/application/MissingHierarchyDetection.java index 2b85318ea4..0c91814f89 100644 --- a/uitest/src/main/java/com/vaadin/tests/application/MissingHierarchyDetection.java +++ b/uitest/src/main/java/com/vaadin/tests/application/MissingHierarchyDetection.java @@ -15,15 +15,7 @@ */ package com.vaadin.tests.application; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.logging.Handler; -import java.util.logging.LogRecord; -import java.util.logging.Logger; - import com.vaadin.server.VaadinRequest; -import com.vaadin.server.VaadinSession; import com.vaadin.tests.components.AbstractReindeerTestUIWithLog; import com.vaadin.ui.Button; import com.vaadin.ui.Button.ClickEvent; @@ -39,8 +31,6 @@ public class MissingHierarchyDetection extends AbstractReindeerTestUIWithLog { private CssLayout normalLayout = new CssLayout( new Label("Normal layout child")); - private List pendingErrors = Collections - .synchronizedList(new ArrayList<>()); public class BrokenCssLayout extends CssLayout implements SelectiveRenderer { @@ -59,29 +49,6 @@ public class MissingHierarchyDetection extends AbstractReindeerTestUIWithLog { @Override protected void setup(VaadinRequest request) { - // Catch log messages so we can see if there is an error - Logger vaadinSessionLogger = Logger - .getLogger(VaadinSession.class.getName()); - vaadinSessionLogger.addHandler(new Handler() { - - @Override - public void publish(LogRecord record) { - if (record.getThrown() instanceof AssertionError) { - pendingErrors.add(record); - vaadinSessionLogger.removeHandler(this); - } - } - - @Override - public void flush() { - - } - - @Override - public void close() throws SecurityException { - - } - }); addComponent(brokenLayout); addComponent(normalLayout); addComponent(new Button("Toggle properly", new Button.ClickListener() { @@ -97,16 +64,6 @@ public class MissingHierarchyDetection extends AbstractReindeerTestUIWithLog { toggle(false); } })); - addComponent(new Button("Check for errors", new Button.ClickListener() { - @Override - public void buttonClick(ClickEvent event) { - if (!pendingErrors.isEmpty()) { - log(pendingErrors.remove(0).getThrown().getMessage()); - } else { - log("No errors"); - } - } - })); } private void toggle(boolean properly) { diff --git a/uitest/src/main/java/com/vaadin/tests/components/table/TableRemovedQuicklySendsInvalidRpcCalls.java b/uitest/src/main/java/com/vaadin/tests/components/table/TableRemovedQuicklySendsInvalidRpcCalls.java index ae3667ff29..8e00357b56 100644 --- a/uitest/src/main/java/com/vaadin/tests/components/table/TableRemovedQuicklySendsInvalidRpcCalls.java +++ b/uitest/src/main/java/com/vaadin/tests/components/table/TableRemovedQuicklySendsInvalidRpcCalls.java @@ -90,6 +90,11 @@ public class TableRemovedQuicklySendsInvalidRpcCalls return tracker.getConnector(connectorId); } + @Override + public void cleanConnectorMap(boolean force) { + tracker.cleanConnectorMap(force); + } + @Override public void cleanConnectorMap() { tracker.cleanConnectorMap(); diff --git a/uitest/src/main/java/com/vaadin/tests/push/MakeComponentVisibleWithPush.java b/uitest/src/main/java/com/vaadin/tests/push/MakeComponentVisibleWithPush.java new file mode 100644 index 0000000000..2bc93317e8 --- /dev/null +++ b/uitest/src/main/java/com/vaadin/tests/push/MakeComponentVisibleWithPush.java @@ -0,0 +1,104 @@ +package com.vaadin.tests.push; + +import java.util.ArrayList; +import java.util.List; + +import com.vaadin.annotations.Push; +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.Button; +import com.vaadin.ui.Grid; +import com.vaadin.ui.UI; +import com.vaadin.ui.VerticalLayout; + +@Push +public class MakeComponentVisibleWithPush extends AbstractTestUI { + + private VerticalLayout rootLayout; + private Grid grid; + private SearchThread searchThread; + private List data; + + @Override + protected void setup(VaadinRequest request) { + data = new ArrayList<>(); + + rootLayout = new VerticalLayout(); + setContent(rootLayout); + + grid = new Grid(); + grid.addColumn(Person::getName); + grid.setVisible(false); + rootLayout.addComponent(grid); + + Button doUpdateButton = new Button("Do Update", event -> { + try { + doUpdate(); + } catch (InterruptedException e) { + } + }); + + rootLayout.addComponent(doUpdateButton); + } + + private void doUpdate() throws InterruptedException { + + cancelSuggestThread(); + + grid.setVisible(false); + grid.setItems(data); + + UI ui = UI.getCurrent(); + searchThread = new SearchThread(ui); + searchThread.start(); + + } + + class SearchThread extends Thread { + private UI ui; + + public SearchThread(UI ui) { + this.ui = ui; + } + + @Override + public void run() { + data = new ArrayList(data); + data.add(new Person("Person " + (data.size() + 1))); + + if (!searchThread.isInterrupted()) { + ui.access(() -> { + grid.setItems(data); + grid.setVisible(true); + }); + } + } + + } + + private void cancelSuggestThread() { + + if ((searchThread != null) && !searchThread.isInterrupted()) { + searchThread.interrupt(); + searchThread = null; + } + } + + class Person { + + private String name; + + public Person(String name) { + this.name = name; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + } + +} diff --git a/uitest/src/test/java/com/vaadin/tests/application/MissingHierarchyDetectionTest.java b/uitest/src/test/java/com/vaadin/tests/application/MissingHierarchyDetectionTest.java index 80eb0735da..64b999b7d5 100644 --- a/uitest/src/test/java/com/vaadin/tests/application/MissingHierarchyDetectionTest.java +++ b/uitest/src/test/java/com/vaadin/tests/application/MissingHierarchyDetectionTest.java @@ -44,11 +44,6 @@ public class MissingHierarchyDetectionTest extends SingleBrowserTest { ButtonElement toggleImproperly = $(ButtonElement.class) .caption("Toggle improperly").first(); toggleImproperly.click(); - - $(ButtonElement.class).caption("Check for errors").first().click(); - Assert.assertTrue( - "No error was logged for the missing hierarchy change event", - getLogRow(0).contains( - "is no longer visible to the client, but no corresponding hierarchy change was sent.")); + assertSystemNotification(); } } diff --git a/uitest/src/test/java/com/vaadin/tests/push/MakeComponentVisibleWithPushTest.java b/uitest/src/test/java/com/vaadin/tests/push/MakeComponentVisibleWithPushTest.java new file mode 100644 index 0000000000..80bedff193 --- /dev/null +++ b/uitest/src/test/java/com/vaadin/tests/push/MakeComponentVisibleWithPushTest.java @@ -0,0 +1,41 @@ +/* + * Copyright 2000-2016 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.push; + +import org.junit.Assert; +import org.junit.Test; + +import com.vaadin.testbench.elements.ButtonElement; +import com.vaadin.testbench.elements.GridElement; +import com.vaadin.tests.tb3.SingleBrowserTest; + +public class MakeComponentVisibleWithPushTest extends SingleBrowserTest { + + @Test + public void showingHiddenComponentByPushWorks() { + setDebug(true); + openTestURL(); + + $(ButtonElement.class).first().click(); + Assert.assertEquals("Unexpected row count", 1, + $(GridElement.class).first().getRowCount()); + $(ButtonElement.class).first().click(); + Assert.assertEquals("Unexpected row count", 2, + $(GridElement.class).first().getRowCount()); + + assertNoErrorNotifications(); + } +} -- cgit v1.2.3