From ee809e5985d7a438e031c40bed160ac78ae8d0bc Mon Sep 17 00:00:00 2001 From: =?utf8?q?Johannes=20Dahlstr=C3=B6m?= Date: Mon, 21 Oct 2013 16:16:37 +0300 Subject: [PATCH] Revert broken fix and test (#12446) The timeoutInterval variable needs to be part of per-UI state for the code to work correctly. Storing it in a servlet-global MetadataWriter instance is both semantically wrong and a race condition. Also, the test currently throws NPE if assertions are enabled. This reverts commit f595d057951523baf35a797692376dfa5de0bc8d. Change-Id: Ia6d1383b2c112b86ce60c75a3ebb9f10da545d4a --- .../communication/UidlRequestHandler.java | 5 +- .../server/communication/UidlWriter.java | 32 ++--- .../server/communication/UidlWriterTest.java | 125 ------------------ 3 files changed, 14 insertions(+), 148 deletions(-) delete mode 100644 server/tests/src/com/vaadin/server/communication/UidlWriterTest.java diff --git a/server/src/com/vaadin/server/communication/UidlRequestHandler.java b/server/src/com/vaadin/server/communication/UidlRequestHandler.java index f13199e9ae..d52c5e9fe0 100644 --- a/server/src/com/vaadin/server/communication/UidlRequestHandler.java +++ b/server/src/com/vaadin/server/communication/UidlRequestHandler.java @@ -56,7 +56,8 @@ public class UidlRequestHandler extends SynchronizedRequestHandler implements private ServerRpcHandler rpcHandler = new ServerRpcHandler(); - private UidlWriter uidlWriter = new UidlWriter(); + public UidlRequestHandler() { + } @Override public boolean synchronizedHandleRequest(VaadinSession session, @@ -145,7 +146,7 @@ public class UidlRequestHandler extends SynchronizedRequestHandler implements JSONException { openJsonMessage(writer, response); - uidlWriter.write(ui, writer, repaintAll, false); + new UidlWriter().write(ui, writer, repaintAll, false); closeJsonMessage(writer); } diff --git a/server/src/com/vaadin/server/communication/UidlWriter.java b/server/src/com/vaadin/server/communication/UidlWriter.java index fe7a7d42bf..60933a75c2 100644 --- a/server/src/com/vaadin/server/communication/UidlWriter.java +++ b/server/src/com/vaadin/server/communication/UidlWriter.java @@ -51,14 +51,6 @@ import com.vaadin.ui.UI; */ public class UidlWriter implements Serializable { - private LegacyUidlWriter legacyUidlWriter = new LegacyUidlWriter(); - private SharedStateWriter sharedStateWriter = new SharedStateWriter(); - private ConnectorTypeWriter connectorTypeWriter = new ConnectorTypeWriter(); - private ConnectorHierarchyWriter connectorHierarchyWriter = new ConnectorHierarchyWriter(); - private ClientRpcWriter clientRpcWriter = new ClientRpcWriter(); - private MetadataWriter metadataWriter = new MetadataWriter(); - private ResourceWriter resourceWriter = new ResourceWriter(); - /** * Writes a JSON object containing all pending changes to the given UI. * @@ -87,12 +79,13 @@ public class UidlWriter implements Serializable { // to write out session.getService().runPendingAccessTasks(session); + ArrayList dirtyVisibleConnectors = ui + .getConnectorTracker().getDirtyVisibleConnectors(); + LegacyCommunicationManager manager = session.getCommunicationManager(); // Paints components + ConnectorTracker uiConnectorTracker = ui.getConnectorTracker(); getLogger().log(Level.FINE, "* Creating response to client"); - ConnectorTracker uiConnectorTracker = ui.getConnectorTracker(); - ArrayList dirtyVisibleConnectors = uiConnectorTracker - .getDirtyVisibleConnectors(); getLogger().log( Level.FINE, "Found " + dirtyVisibleConnectors.size() @@ -107,13 +100,10 @@ public class UidlWriter implements Serializable { try { writer.write("\"changes\" : "); - LegacyCommunicationManager manager = session - .getCommunicationManager(); - JsonPaintTarget paintTarget = new JsonPaintTarget(manager, writer, !repaintAll); - legacyUidlWriter.write(ui, writer, paintTarget); + new LegacyUidlWriter().write(ui, writer, paintTarget); paintTarget.close(); writer.write(", "); // close changes @@ -130,7 +120,7 @@ public class UidlWriter implements Serializable { // processing. writer.write("\"state\":"); - sharedStateWriter.write(ui, writer); + new SharedStateWriter().write(ui, writer); writer.write(", "); // close states // TODO This should be optimized. The type only needs to be @@ -139,7 +129,7 @@ public class UidlWriter implements Serializable { // widget mapping writer.write("\"types\":"); - connectorTypeWriter.write(ui, writer, paintTarget); + new ConnectorTypeWriter().write(ui, writer, paintTarget); writer.write(", "); // close states // Send update hierarchy information to the client. @@ -150,7 +140,7 @@ public class UidlWriter implements Serializable { // child to 0 children) writer.write("\"hierarchy\":"); - connectorHierarchyWriter.write(ui, writer); + new ConnectorHierarchyWriter().write(ui, writer); writer.write(", "); // close hierarchy // send server to client RPC calls for components in the UI, in call @@ -160,7 +150,7 @@ public class UidlWriter implements Serializable { // which they were performed, remove the calls from components writer.write("\"rpc\" : "); - clientRpcWriter.write(ui, writer); + new ClientRpcWriter().write(ui, writer); writer.write(", "); // close rpc uiConnectorTracker.markAllConnectorsClean(); @@ -170,11 +160,11 @@ public class UidlWriter implements Serializable { SystemMessages messages = ui.getSession().getService() .getSystemMessages(ui.getLocale(), null); // TODO hilightedConnector - metadataWriter.write(ui, writer, repaintAll, async, messages); + new MetadataWriter().write(ui, writer, repaintAll, async, messages); writer.write(", "); writer.write("\"resources\" : "); - resourceWriter.write(ui, writer, paintTarget); + new ResourceWriter().write(ui, writer, paintTarget); Collection> usedClientConnectors = paintTarget .getUsedClientConnectors(); diff --git a/server/tests/src/com/vaadin/server/communication/UidlWriterTest.java b/server/tests/src/com/vaadin/server/communication/UidlWriterTest.java deleted file mode 100644 index 8dcd6cbdf4..0000000000 --- a/server/tests/src/com/vaadin/server/communication/UidlWriterTest.java +++ /dev/null @@ -1,125 +0,0 @@ -/* - * Copyright 2000-2013 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.server.communication; - -import java.io.IOException; -import java.io.StringWriter; -import java.util.ArrayList; -import java.util.Locale; - -import org.easymock.EasyMock; -import org.json.JSONException; -import org.json.JSONObject; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; - -import com.vaadin.server.ClientConnector; -import com.vaadin.server.LegacyCommunicationManager; -import com.vaadin.server.SystemMessages; -import com.vaadin.server.VaadinRequest; -import com.vaadin.server.VaadinService; -import com.vaadin.server.VaadinSession; -import com.vaadin.server.WrappedSession; -import com.vaadin.ui.ConnectorTracker; -import com.vaadin.ui.UI; - -/** - * - * @since - * @author Vaadin Ltd - */ -public class UidlWriterTest { - - private UI ui; - private UidlWriter uidlWriter; - - @Before - public void setUp() { - SystemMessages messages = EasyMock.createNiceMock(SystemMessages.class); - EasyMock.expect(messages.isSessionExpiredNotificationEnabled()) - .andReturn(true).anyTimes(); - EasyMock.replay(messages); - - VaadinService service = EasyMock.createNiceMock(VaadinService.class); - EasyMock.expect( - service.getSystemMessages(EasyMock.anyObject(Locale.class), - EasyMock.anyObject(VaadinRequest.class))) - .andReturn(messages).anyTimes(); - EasyMock.replay(service); - - LegacyCommunicationManager manager = EasyMock - .createNiceMock(LegacyCommunicationManager.class); - EasyMock.replay(manager); - - WrappedSession wrappedSession = EasyMock - .createNiceMock(WrappedSession.class); - EasyMock.expect(wrappedSession.getMaxInactiveInterval()).andReturn(100) - .times(3).andReturn(200); - - EasyMock.replay(wrappedSession); - - VaadinSession session = EasyMock.createNiceMock(VaadinSession.class); - EasyMock.expect(session.getService()).andReturn(service).anyTimes(); - EasyMock.expect(session.getCommunicationManager()).andReturn(manager) - .anyTimes(); - EasyMock.expect(session.getSession()).andReturn(wrappedSession) - .anyTimes(); - EasyMock.replay(session); - - ConnectorTracker tracker = EasyMock - .createNiceMock(ConnectorTracker.class); - EasyMock.expect(tracker.getDirtyVisibleConnectors()) - .andReturn(new ArrayList()).anyTimes(); - EasyMock.replay(tracker); - - ui = EasyMock.createNiceMock(UI.class); - EasyMock.expect(ui.getSession()).andReturn(session).anyTimes(); - EasyMock.expect(ui.getConnectorTracker()).andReturn(tracker).anyTimes(); - EasyMock.replay(ui); - - uidlWriter = new UidlWriter(); - } - - @Test - public void testMetadataWriterState() throws IOException, JSONException { - - Assert.assertEquals( - "Metadata should contain redirect interval on first write", - 115, getRedirect(uidl(false, false)).optInt("interval")); - Assert.assertNull( - "Metadata should not contain redirect interval on second write", - getRedirect(uidl(false, false))); - Assert.assertEquals( - "Metadata should contain redirect interval on repaintAll", 115, - getRedirect(uidl(true, false)).optInt("interval")); - Assert.assertEquals( - "Metadata should contain redirect interval when changed in session", - 215, getRedirect(uidl(false, false)).optInt("interval")); - } - - private JSONObject uidl(boolean repaintAll, boolean async) - throws IOException, JSONException { - StringWriter writer = new StringWriter(); - uidlWriter.write(ui, writer, repaintAll, async); - return new JSONObject("{" + writer.toString() + "}"); - } - - private JSONObject getRedirect(JSONObject json) throws JSONException { - return json.getJSONObject("meta").optJSONObject("timedRedirect"); - - } -} -- 2.39.5