Remove 'burst' concept to simplify communication handling (#11733)

Change-Id: I74f173e69491f23dc49b1ff577a45f611b0eed1c

client/src/com/vaadin/client/ApplicationConnection.java

@@ -225,9 +225,6 @@ public class ApplicationConnection implements HasHandlers {
     /** Parameters for this application connection loaded from the web-page */
     private ApplicationConfiguration configuration;
 
-    /** List of pending variable change bursts that must be submitted in order */
-    private final ArrayList<LinkedHashMap<String, MethodInvocation>> pendingBursts = new ArrayList<LinkedHashMap<String, MethodInvocation>>();
-
     /** Timer for automatic refirect to SessionExpiredURL */
     private Timer redirectTimer;
 
@@ -1292,7 +1289,7 @@ public class ApplicationConnection implements HasHandlers {
         if (!hasActiveRequest) {
             getLogger().severe("No active request");
         }
-        // After checkForPendingVariableBursts() there may be a new active
+        // After sendInvocationsToServer() there may be a new active
         // request, so we must set hasActiveRequest to false before, not after,
         // the call. Active requests used to be tracked with an integer counter,
         // so setting it after used to work but not with the #8505 changes.
@@ -1301,7 +1298,9 @@ public class ApplicationConnection implements HasHandlers {
         webkitMaybeIgnoringRequests = false;
 
         if (isApplicationRunning()) {
-            checkForPendingVariableBursts();
+            if (sendServerRpcWhenConnectionAvailable) {
+                sendInvocationsToServer();
+            }
             runPostRequestHooks(configuration.getRootPanelId());
         }
 
@@ -1326,45 +1325,6 @@ public class ApplicationConnection implements HasHandlers {
         eventBus.fireEvent(new ResponseHandlingEndedEvent(this));
     }
 
-    /**
-     * This method is called after applying uidl change set to application.
-     * 
-     * It will clean current and queued variable change sets. And send next
-     * change set if it exists.
-     */
-    private void checkForPendingVariableBursts() {
-        cleanVariableBurst(pendingInvocations);
-        if (pendingBursts.size() > 0) {
-            for (LinkedHashMap<String, MethodInvocation> pendingBurst : pendingBursts) {
-                cleanVariableBurst(pendingBurst);
-            }
-            LinkedHashMap<String, MethodInvocation> nextBurst = pendingBursts
-                    .remove(0);
-            buildAndSendVariableBurst(nextBurst);
-        }
-    }
-
-    /**
-     * Cleans given queue of variable changes of such changes that came from
-     * components that do not exist anymore.
-     * 
-     * @param variableBurst
-     */
-    private void cleanVariableBurst(
-            LinkedHashMap<String, MethodInvocation> variableBurst) {
-        Iterator<MethodInvocation> iterator = variableBurst.values().iterator();
-        while (iterator.hasNext()) {
-            String id = iterator.next().getConnectorId();
-            if (!getConnectorMap().hasConnector(id)
-                    && !getConnectorMap().isDragAndDropPaintable(id)) {
-                // variable owner does not exist anymore
-                iterator.remove();
-                getLogger().info(
-                        "Removed variable from removed component: " + id);
-            }
-        }
-    }
-
     /**
      * Checks if the client has running or scheduled commands
      */
@@ -2778,14 +2738,7 @@ public class ApplicationConnection implements HasHandlers {
     }
 
     /**
-     * This method sends currently queued variable changes to server. It is
-     * called when immediate variable update must happen.
-     * 
-     * To ensure correct order for variable changes (due servers multithreading
-     * or network), we always wait for active request to be handler before
-     * sending a new one. If there is an active request, we will put varible
-     * "burst" to queue that will be purged after current request is handled.
-     * 
+     * Triggers a send of server RPC and legacy variable changes to the server.
      */
     public void sendPendingVariableChanges() {
         if (!deferredSendPending) {
@@ -2803,18 +2756,16 @@ public class ApplicationConnection implements HasHandlers {
     };
     private boolean deferredSendPending = false;
 
+    private boolean sendServerRpcWhenConnectionAvailable = false;
+
     private void doSendPendingVariableChanges() {
         if (isApplicationRunning()) {
             if (hasActiveRequest() || (push != null && !push.isActive())) {
-                // skip empty queues if there are pending bursts to be sent
-                if (pendingInvocations.size() > 0 || pendingBursts.size() == 0) {
-                    pendingBursts.add(pendingInvocations);
-                    pendingInvocations = new LinkedHashMap<String, MethodInvocation>();
-                    // Keep tag string short
-                    lastInvocationTag = 0;
-                }
+                // There is an active request or push is enabled but not active
+                // -> send when current request completes or push becomes active
+                sendServerRpcWhenConnectionAvailable = true;
             } else {
-                buildAndSendVariableBurst(pendingInvocations);
+                sendInvocationsToServer();
             }
         } else {
             getLogger()
@@ -2825,27 +2776,30 @@ public class ApplicationConnection implements HasHandlers {
     }
 
     /**
-     * Build the variable burst and send it to server.
-     * 
-     * When sync is forced, we also force sending of all pending variable-bursts
-     * at the same time. This is ok as we can assume that DOM will never be
-     * updated after this.
+     * Sends all pending method invocations (server RPC and legacy variable
+     * changes) to the server.
      * 
-     * @param pendingInvocations
-     *            List of RPC method invocations to send
      */
-    private void buildAndSendVariableBurst(
-            LinkedHashMap<String, MethodInvocation> pendingInvocations) {
+    private void sendInvocationsToServer() {
         boolean showLoadingIndicator = false;
         JsonArray reqJson = Json.createArray();
         if (!pendingInvocations.isEmpty()) {
             if (ApplicationConfiguration.isDebugMode()) {
-                Util.logVariableBurst(this, pendingInvocations.values());
+                Util.logMethodInvocations(this, pendingInvocations.values());
             }
 
             for (MethodInvocation invocation : pendingInvocations.values()) {
+                String connectorId = invocation.getConnectorId();
+                if (!getConnectorMap().connectorExists(connectorId)) {
+                    getLogger()
+                            .info("Not sending RPC for removed connector: "
+                                    + connectorId + ": "
+                                    + Util.getInvocationDebugString(invocation));
+                    continue;
+                }
+
                 JsonArray invocationJson = Json.createArray();
-                invocationJson.set(0, invocation.getConnectorId());
+                invocationJson.set(0, connectorId);
                 invocationJson.set(1, invocation.getInterfaceName());
                 invocationJson.set(2, invocation.getMethodName());
                 JsonArray paramJson = Json.createArray();
@@ -2888,6 +2842,7 @@ public class ApplicationConnection implements HasHandlers {
             pendingInvocations.clear();
             // Keep tag string short
             lastInvocationTag = 0;
+            sendServerRpcWhenConnectionAvailable = false;
         }
 
         String extraParams = "";

client/src/com/vaadin/client/ConnectorMap.java

@@ -101,6 +101,10 @@ public class ConnectorMap {
         return idToComponentDetail.containsKey(connectorId);
     }
 
+    boolean connectorExists(String connectorId) {
+        return hasConnector(connectorId) || isDragAndDropPaintable(connectorId);
+    }
+
     /**
      * Removes all registered connectors
      */

client/src/com/vaadin/client/Util.java

@@ -747,36 +747,46 @@ public class Util {
                             + id);
         }
         for (MethodInvocation invocation : invocations) {
-            Object[] parameters = invocation.getParameters();
-            String formattedParams = null;
-            if (ApplicationConstants.UPDATE_VARIABLE_METHOD.equals(invocation
-                    .getMethodName()) && parameters.length == 2) {
-                // name, value
-                Object value = parameters[1];
-                // TODO paintables inside lists/maps get rendered as
-                // components in the debug console
-                String formattedValue = value instanceof ServerConnector ? ((ServerConnector) value)
-                        .getConnectorId() : String.valueOf(value);
-                formattedParams = parameters[0] + " : " + formattedValue;
-            }
-            if (null == formattedParams) {
-                formattedParams = (null != parameters) ? Arrays
-                        .toString(parameters) : null;
-            }
-            getLogger().info(
-                    "\t\t" + invocation.getInterfaceName() + "."
-                            + invocation.getMethodName() + "("
-                            + formattedParams + ")");
+            getLogger().info("\t\t" + getInvocationDebugString(invocation));
+        }
+    }
+
+    /**
+     * Produces a string representation of a method invocation, suitable for
+     * debug output
+     * 
+     * @since 7.5
+     * @param invocation
+     * @return
+     */
+    static String getInvocationDebugString(MethodInvocation invocation) {
+        Object[] parameters = invocation.getParameters();
+        String formattedParams = null;
+        if (ApplicationConstants.UPDATE_VARIABLE_METHOD.equals(invocation
+                .getMethodName()) && parameters.length == 2) {
+            // name, value
+            Object value = parameters[1];
+            // TODO paintables inside lists/maps get rendered as
+            // components in the debug console
+            String formattedValue = value instanceof ServerConnector ? ((ServerConnector) value)
+                    .getConnectorId() : String.valueOf(value);
+            formattedParams = parameters[0] + " : " + formattedValue;
+        }
+        if (null == formattedParams) {
+            formattedParams = (null != parameters) ? Arrays
+                    .toString(parameters) : null;
         }
+        return invocation.getInterfaceName() + "." + invocation.getMethodName()
+                + "(" + formattedParams + ")";
     }
 
-    static void logVariableBurst(ApplicationConnection c,
-            Collection<MethodInvocation> loggedBurst) {
+    static void logMethodInvocations(ApplicationConnection c,
+            Collection<MethodInvocation> methodInvocations) {
         try {
-            getLogger().info("Variable burst to be sent to server:");
+            getLogger().info("RPC invocations to be sent to the server:");
             String curId = null;
             ArrayList<MethodInvocation> invocations = new ArrayList<MethodInvocation>();
-            for (MethodInvocation methodInvocation : loggedBurst) {
+            for (MethodInvocation methodInvocation : methodInvocations) {
                 String id = methodInvocation.getConnectorId();
 
                 if (curId == null) {
@@ -792,7 +802,8 @@ public class Util {
                 printConnectorInvocations(invocations, curId, c);
             }
         } catch (Exception e) {
-            getLogger().log(Level.SEVERE, "Error sending variable burst", e);
+            getLogger()
+                    .log(Level.SEVERE, "Error logging method invocations", e);
         }
     }
 

uitest/src/com/vaadin/tests/push/SendMultibyteCharactersTest.java

@@ -36,7 +36,7 @@ public abstract class SendMultibyteCharactersTest extends MultiBrowserTest {
 
         findElement(By.tagName("body")).click();
 
-        waitForDebugMessage("Variable burst to be sent to server:", 5);
+        waitForDebugMessage("RPC invocations to be sent to the server:", 5);
         waitForDebugMessage("Handling message from server", 10);
     }