From 880b606987e6d7350f65ce58edb13ba55b3e91ca Mon Sep 17 00:00:00 2001 From: =?utf8?q?Leif=20=C3=85strand?= Date: Fri, 14 Dec 2012 14:17:09 +0200 Subject: [PATCH] Don't remove diffstate data before cleanup (#10532) * Avoid returning unregistered but not cleaned connectors from e.g. getDirtyConnectors() and getConnector() * Add some sanity checks to ensure unregistered connectors are not used Change-Id: I1103586863d2f299d50af5058233a7b4c967d4c6 --- .../src/com/vaadin/ui/ConnectorTracker.java | 54 +++++++++++++++---- .../ChangeStateWhenReattaching.html | 32 +++++++++++ .../ChangeStateWhenReattaching.java | 51 ++++++++++++++++++ 3 files changed, 126 insertions(+), 11 deletions(-) create mode 100644 uitest/src/com/vaadin/tests/serialization/ChangeStateWhenReattaching.html create mode 100644 uitest/src/com/vaadin/tests/serialization/ChangeStateWhenReattaching.java diff --git a/server/src/com/vaadin/ui/ConnectorTracker.java b/server/src/com/vaadin/ui/ConnectorTracker.java index 91f9ac451c..d5ee43f0a0 100644 --- a/server/src/com/vaadin/ui/ConnectorTracker.java +++ b/server/src/com/vaadin/ui/ConnectorTracker.java @@ -62,6 +62,13 @@ public class ConnectorTracker implements Serializable { private Set dirtyConnectors = new HashSet(); private Set uninitializedConnectors = new HashSet(); + /** + * Connectors that have been unregistered and should be cleaned up the next + * time {@link #cleanConnectorMap()} is invoked unless they have been + * registered again. + */ + private final Set unregisteredConnectors = new HashSet(); + private boolean writingResponse = false; private UI uI; @@ -105,6 +112,8 @@ public class ConnectorTracker implements Serializable { * The connector to register. */ public void registerConnector(ClientConnector connector) { + boolean wasUnregistered = unregisteredConnectors.remove(connector); + String connectorId = connector.getConnectorId(); ClientConnector previouslyRegistered = connectorIdToConnector .get(connectorId); @@ -117,13 +126,13 @@ public class ConnectorTracker implements Serializable { } else if (previouslyRegistered != connector) { throw new RuntimeException("A connector with id " + connectorId + " is already registered!"); - } else { + } else if (!wasUnregistered) { getLogger().warning( "An already registered connector was registered again: " + connector.getClass().getSimpleName() + " (" + connectorId + ")"); } - + dirtyConnectors.add(connector); } /** @@ -152,14 +161,17 @@ public class ConnectorTracker implements Serializable { + " is not the one that was registered for that id"); } - getLogger().fine( - "Unregistered " + connector.getClass().getSimpleName() + " (" - + connectorId + ")"); - - removeFromGlobalResourceHandler(connector); - connectorIdToConnector.remove(connectorId); - uninitializedConnectors.remove(connector); - diffStates.remove(connector); + dirtyConnectors.remove(connector); + if (unregisteredConnectors.add(connector)) { + getLogger().fine( + "Unregistered " + connector.getClass().getSimpleName() + + " (" + connectorId + ")"); + } else { + getLogger().warning( + "Unregistered " + connector.getClass().getSimpleName() + + " (" + connectorId + + ") that was already unregistered."); + } } private void removeFromGlobalResourceHandler(ClientConnector connector) { @@ -221,7 +233,12 @@ public class ConnectorTracker implements Serializable { * given id */ public ClientConnector getConnector(String connectorId) { - return connectorIdToConnector.get(connectorId); + ClientConnector connector = connectorIdToConnector.get(connectorId); + // Ignore connectors that have been unregistered but not yet cleaned up + if (unregisteredConnectors.contains(connector)) { + return null; + } + return connector; } /** @@ -229,6 +246,18 @@ public class ConnectorTracker implements Serializable { * to the application. This should only be called by the framework. */ public void cleanConnectorMap() { + // Remove connectors that have been unregistered + for (ClientConnector connector : unregisteredConnectors) { + ClientConnector removedConnector = connectorIdToConnector + .remove(connector.getConnectorId()); + assert removedConnector == connector; + + removeFromGlobalResourceHandler(connector); + uninitializedConnectors.remove(connector); + diffStates.remove(connector); + } + unregisteredConnectors.clear(); + // remove detached components from paintableIdMap so they // can be GC'ed Iterator iterator = connectorIdToConnector.keySet().iterator(); @@ -423,10 +452,12 @@ public class ConnectorTracker implements Serializable { } public JSONObject getDiffState(ClientConnector connector) { + assert getConnector(connector.getConnectorId()) == connector; return diffStates.get(connector); } public void setDiffState(ClientConnector connector, JSONObject diffState) { + assert getConnector(connector.getConnectorId()) == connector; diffStates.put(connector, diffState); } @@ -537,6 +568,7 @@ public class ConnectorTracker implements Serializable { */ public void addStreamVariable(String connectorId, String variableName, StreamVariable variable) { + assert getConnector(connectorId) != null; if (pidToNameToStreamVariable == null) { pidToNameToStreamVariable = new HashMap>(); } diff --git a/uitest/src/com/vaadin/tests/serialization/ChangeStateWhenReattaching.html b/uitest/src/com/vaadin/tests/serialization/ChangeStateWhenReattaching.html new file mode 100644 index 0000000000..865f2bae0b --- /dev/null +++ b/uitest/src/com/vaadin/tests/serialization/ChangeStateWhenReattaching.html @@ -0,0 +1,32 @@ + + + + + + +New Test + + + + + + + + + + + + + + + + + + + + + + +
New Test
open/run/com.vaadin.tests.serialization.ChangeStateWhenReattaching?restartApplication
mouseClickvaadin=runcomvaadintestsserializationChangeStateWhenReattaching::/VVerticalLayout[0]/Slot[1]/VVerticalLayout[0]/Slot[0]/VButton[0]/domChild[0]/domChild[0]94,43
assertTextvaadin=runcomvaadintestsserializationChangeStateWhenReattaching::/VVerticalLayout[0]/Slot[1]/VVerticalLayout[0]/Slot[0]/VButton[0]/domChild[0]/domChild[0]
+ + diff --git a/uitest/src/com/vaadin/tests/serialization/ChangeStateWhenReattaching.java b/uitest/src/com/vaadin/tests/serialization/ChangeStateWhenReattaching.java new file mode 100644 index 0000000000..814d52048f --- /dev/null +++ b/uitest/src/com/vaadin/tests/serialization/ChangeStateWhenReattaching.java @@ -0,0 +1,51 @@ +/* + * Copyright 2012 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.serialization; + +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.Button; +import com.vaadin.ui.Button.ClickEvent; + +public class ChangeStateWhenReattaching extends AbstractTestUI { + + @Override + protected void setup(VaadinRequest request) { + Button button = new Button("Reattach and remove caption", + new Button.ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + Button button = event.getButton(); + removeComponent(button); + addComponent(button); + button.setCaption(null); + } + }); + addComponent(button); + } + + @Override + protected String getTestDescription() { + return "Clicking the button should remove its caption, even though it is also reattached."; + } + + @Override + protected Integer getTicketNumber() { + return Integer.valueOf(10532); + } + +} -- 2.39.5