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

diff --git a/server/src/com/vaadin/server/communication/UidlRequestHandler.java b/server/src/com/vaadin/server/communication/UidlRequestHandler.java
index f13199e9ae5cecf24f986eb8576e9459bfc83b4f..d52c5e9fe060cbb97f9a5d375f0b41a0ebd2bb90 100644 (file)
--- 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 fe7a7d42bf6daf026e4864234b76037e693790d5..60933a75c2522792df8fd474d01588590bd52095 100644 (file)
--- 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<ClientConnector> 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<ClientConnector> 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<Class<? extends ClientConnector>> 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 (file)
index 8dcd6cb..0000000
--- 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<ClientConnector>()).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");
-
-    }
-}