diff options
| author | Artur Signell <artur@vaadin.com> | 2015-04-17 22:02:49 +0300 |
|---|---|---|
| committer | Artur Signell <artur@vaadin.com> | 2015-07-13 17:19:07 +0300 |
| commit | 72b36bbe2d18ee1adc037ba0d4f2093d23412c4f (patch) | |
| tree | cef1f19b770c5cdbf0835c0a223efc038ad70255 | |
| parent | 90b4c678d139fc4927811cdcad87d0b91eca98b6 (diff) | |
| download | vaadin-framework-72b36bbe2d18ee1adc037ba0d4f2093d23412c4f.tar.gz vaadin-framework-72b36bbe2d18ee1adc037ba0d4f2093d23412c4f.zip | |
Use counter in client to server messages to avoid duplicate handling (#11733)
An server message id counter is included in every client to server message and an
expected id is included in every server to client message. This makes it safe to
re-send any message when the client is not 100% sure the server has received the
message, without having to worry about double handling on the server side.
Change-Id: I840cc04829fc2491f35a0e6f98f07eaf46b1ea42
7 files changed, 156 insertions, 22 deletions
diff --git a/client/src/com/vaadin/client/ApplicationConnection.java b/client/src/com/vaadin/client/ApplicationConnection.java index 0e84392cf0..0c237cca43 100644 --- a/client/src/com/vaadin/client/ApplicationConnection.java +++ b/client/src/com/vaadin/client/ApplicationConnection.java @@ -63,14 +63,11 @@ import com.vaadin.client.ui.VNotification; import com.vaadin.client.ui.VOverlay; import com.vaadin.client.ui.ui.UIConnector; import com.vaadin.shared.AbstractComponentState; -import com.vaadin.shared.ApplicationConstants; import com.vaadin.shared.VaadinUriResolver; import com.vaadin.shared.Version; import com.vaadin.shared.communication.LegacyChangeVariablesInvocation; import com.vaadin.shared.util.SharedUtil; -import elemental.json.Json; - /** * This is the client side communication "engine", managing client-server * communication with its server side counterpart @@ -436,8 +433,8 @@ public class ApplicationConnection implements HasHandlers { public void start() { String jsonText = configuration.getUIDL(); if (jsonText == null) { - // inital UIDL not in DOM, request later - repaintAll(); + // initial UIDL not in DOM, request from server + getServerCommunicationHandler().resynchronize(); } else { // initial UIDL provided in DOM, continue as if returned by request @@ -612,17 +609,6 @@ public class ApplicationConnection implements HasHandlers { } }-*/; - public String getRepaintAllParameters() { - String parameters = ApplicationConstants.URL_PARAMETER_REPAINT_ALL - + "=1"; - return parameters; - } - - public void repaintAll() { - getServerCommunicationHandler().makeUidlRequest(Json.createArray(), - getRepaintAllParameters()); - } - /** * Requests an analyze of layouts, to find inconsistencies. Exclusively used * for debugging during development. diff --git a/client/src/com/vaadin/client/communication/ServerCommunicationHandler.java b/client/src/com/vaadin/client/communication/ServerCommunicationHandler.java index ef215220e2..d961d59372 100644 --- a/client/src/com/vaadin/client/communication/ServerCommunicationHandler.java +++ b/client/src/com/vaadin/client/communication/ServerCommunicationHandler.java @@ -66,6 +66,9 @@ public class ServerCommunicationHandler { private final String JSON_COMMUNICATION_PREFIX = "for(;;);["; private final String JSON_COMMUNICATION_SUFFIX = "]"; + private static final String REPAINT_ALL_PARAMETER = ApplicationConstants.URL_PARAMETER_REPAINT_ALL + + "=1"; + private ApplicationConnection connection; private PushConnection push; private boolean hasActiveRequest = false; @@ -80,6 +83,11 @@ public class ServerCommunicationHandler { */ private boolean webkitMaybeIgnoringRequests = false; + /** + * Counter for the messages send to the server. First sent message has id 0. + */ + private int clientToServerMessageId = 0; + public ServerCommunicationHandler() { Window.addWindowClosingHandler(new ClosingHandler() { @Override @@ -191,6 +199,8 @@ public class ServerCommunicationHandler { payload.put(ApplicationConstants.RPC_INVOCATIONS, reqInvocations); payload.put(ApplicationConstants.SERVER_SYNC_ID, getServerMessageHandler().getLastSeenServerSyncId()); + payload.put(ApplicationConstants.CLIENT_TO_SERVER_ID, + clientToServerMessageId++); getLogger() .info("Making UIDL Request with params: " + payload.toJson()); @@ -198,7 +208,7 @@ public class ServerCommunicationHandler { .translateVaadinUri(ApplicationConstants.APP_PROTOCOL_PREFIX + ApplicationConstants.UIDL_PATH + '/'); - if (extraParams.equals(connection.getRepaintAllParameters())) { + if (extraParams.equals(REPAINT_ALL_PARAMETER)) { payload.put(ApplicationConstants.RESYNCHRONIZE_ID, true); } else { uri = SharedUtil.addGetParameters(uri, extraParams); @@ -479,4 +489,45 @@ public class ServerCommunicationHandler { return connection.getLoadingIndicator(); } + /** + * Resynchronize the client side, i.e. reload all component hierarchy and + * state from the server + */ + public void resynchronize() { + makeUidlRequest(Json.createArray(), REPAINT_ALL_PARAMETER); + } + + /** + * Used internally to update what the server expects + * + * @param clientToServerMessageId + * the new client id to set + */ + public void setClientToServerMessageId(int nextExpectedId) { + if (nextExpectedId == clientToServerMessageId) { + // No op as everything matches they way it should + return; + } + + if (nextExpectedId > clientToServerMessageId) { + if (clientToServerMessageId == 0) { + // We have never sent a message to the server, so likely the + // server knows better (typical case is that we refreshed a + // @PreserveOnRefresh UI) + getLogger().info( + "Updating client-to-server id to " + nextExpectedId + + " based on server"); + } else { + getLogger().warning( + "Server expects next client-to-server id to be " + + nextExpectedId + " but we were going to use " + + clientToServerMessageId + ". Will use " + + nextExpectedId + "."); + } + clientToServerMessageId = nextExpectedId; + } else { + // Server has not yet seen all our messages + // Do nothing as they will arrive eventually + } + } } diff --git a/client/src/com/vaadin/client/communication/ServerMessageHandler.java b/client/src/com/vaadin/client/communication/ServerMessageHandler.java index a9874d99ea..718bf038f6 100644 --- a/client/src/com/vaadin/client/communication/ServerMessageHandler.java +++ b/client/src/com/vaadin/client/communication/ServerMessageHandler.java @@ -325,7 +325,7 @@ public class ServerMessageHandler { } resumeResponseHandling(lock); - connection.repaintAll(); + getServerCommunicationHandler().resynchronize(); return; } } @@ -336,6 +336,13 @@ public class ServerMessageHandler { + "Please verify that the server is up-to-date and that the response data has not been modified in transmission."); } + if (json.containsKey(ApplicationConstants.CLIENT_TO_SERVER_ID)) { + int serverNextExpected = json + .getInt(ApplicationConstants.CLIENT_TO_SERVER_ID); + getServerCommunicationHandler().setClientToServerMessageId( + serverNextExpected); + } + // Handle redirect if (json.containsKey("redirect")) { String url = json.getValueMap("redirect").getString("url"); diff --git a/server/src/com/vaadin/server/communication/ServerRpcHandler.java b/server/src/com/vaadin/server/communication/ServerRpcHandler.java index 65fb144810..85a8b341d0 100644 --- a/server/src/com/vaadin/server/communication/ServerRpcHandler.java +++ b/server/src/com/vaadin/server/communication/ServerRpcHandler.java @@ -77,6 +77,7 @@ public class ServerRpcHandler implements Serializable { private final int syncId; private final JsonObject json; private final boolean resynchronize; + private final int clientToServerMessageId; public RpcRequest(String jsonString, VaadinRequest request) { json = JsonUtil.parse(jsonString); @@ -106,7 +107,14 @@ public class ServerRpcHandler implements Serializable { } else { resynchronize = false; } - + if (json.hasKey(ApplicationConstants.CLIENT_TO_SERVER_ID)) { + clientToServerMessageId = (int) json + .getNumber(ApplicationConstants.CLIENT_TO_SERVER_ID); + } else { + getLogger() + .warning("Server message without client id received"); + clientToServerMessageId = -1; + } invocations = json.getArray(ApplicationConstants.RPC_INVOCATIONS); } @@ -149,6 +157,15 @@ public class ServerRpcHandler implements Serializable { } /** + * Gets the id of the client to server message + * + * @return the server message id + */ + public int getClientToServerId() { + return clientToServerMessageId; + } + + /** * Gets the entire request in JSON format, as it was received from the * client. * <p> @@ -199,8 +216,41 @@ public class ServerRpcHandler implements Serializable { rpcRequest.getCsrfToken())) { throw new InvalidUIDLSecurityKeyException(""); } - handleInvocations(ui, rpcRequest.getSyncId(), - rpcRequest.getRpcInvocationsData()); + + int expectedId = ui.getLastProcessedClientToServerId() + 1; + if (rpcRequest.getClientToServerId() != -1 + && rpcRequest.getClientToServerId() != expectedId) { + // Invalid message id, skip RPC processing but force a full + // re-synchronization of the client as it might have not received + // the previous response (e.g. due to a bad connection) + + // Must resync also for duplicate messages because the server might + // have generated a response for the first message but the response + // did not reach the client. When the client re-sends the message, + // it would only get an empty response (because the dirty flags have + // been cleared on the server) and would be out of sync + ui.getSession().getCommunicationManager().repaintAll(ui); + + if (rpcRequest.getClientToServerId() < expectedId) { + // Just a duplicate message due to a bad connection or similar + // It has already been handled by the server so it is safe to + // ignore + getLogger().fine( + "Ignoring old message from the client. Expected: " + + expectedId + ", got: " + + rpcRequest.getClientToServerId()); + } else { + getLogger().warning( + "Unexpected message id from the client. Expected: " + + expectedId + ", got: " + + rpcRequest.getClientToServerId()); + } + } else { + // Message id ok, process RPCs + ui.setLastProcessedClientToServerId(expectedId); + handleInvocations(ui, rpcRequest.getSyncId(), + rpcRequest.getRpcInvocationsData()); + } ui.getConnectorTracker().cleanConcurrentlyRemovedConnectorIds( rpcRequest.getSyncId()); diff --git a/server/src/com/vaadin/server/communication/UidlWriter.java b/server/src/com/vaadin/server/communication/UidlWriter.java index a4797e49aa..3d52230927 100644 --- a/server/src/com/vaadin/server/communication/UidlWriter.java +++ b/server/src/com/vaadin/server/communication/UidlWriter.java @@ -130,7 +130,10 @@ public class UidlWriter implements Serializable { .getCurrentSyncId() : -1; writer.write("\"" + ApplicationConstants.SERVER_SYNC_ID + "\": " + syncId + ", "); - + int nextClientToServerMessageId = ui + .getLastProcessedClientToServerId() + 1; + writer.write("\"" + ApplicationConstants.CLIENT_TO_SERVER_ID + + "\": " + nextClientToServerMessageId + ", "); writer.write("\"changes\" : "); JsonPaintTarget paintTarget = new JsonPaintTarget(manager, writer, diff --git a/server/src/com/vaadin/ui/UI.java b/server/src/com/vaadin/ui/UI.java index 2129db614b..28f18a6b92 100644 --- a/server/src/com/vaadin/ui/UI.java +++ b/server/src/com/vaadin/ui/UI.java @@ -260,6 +260,12 @@ public abstract class UI extends AbstractSingleComponentContainer implements this); /** + * Tracks which message from the client should come next. First message from + * the client has id 0. + */ + private int lastProcessedClientToServerId = -1; + + /** * Creates a new empty UI without a caption. The content of the UI must be * set by calling {@link #setContent(Component)} before using the UI. */ @@ -1691,4 +1697,29 @@ public abstract class UI extends AbstractSingleComponentContainer implements public String getEmbedId() { return embedId; } + + /** + * Gets the last processed server message id. + * + * Used internally for communication tracking. + * + * @return lastProcessedServerMessageId the id of the last processed server + * message + */ + public int getLastProcessedClientToServerId() { + return lastProcessedClientToServerId; + } + + /** + * Sets the last processed server message id. + * + * Used internally for communication tracking. + * + * @param lastProcessedServerMessageId + * the id of the last processed server message + */ + public void setLastProcessedClientToServerId( + int lastProcessedClientToServerId) { + this.lastProcessedClientToServerId = lastProcessedClientToServerId; + } } diff --git a/shared/src/com/vaadin/shared/ApplicationConstants.java b/shared/src/com/vaadin/shared/ApplicationConstants.java index 3431387b77..8613c9a1d2 100644 --- a/shared/src/com/vaadin/shared/ApplicationConstants.java +++ b/shared/src/com/vaadin/shared/ApplicationConstants.java @@ -133,6 +133,12 @@ public class ApplicationConstants implements Serializable { public static final String SERVER_SYNC_ID = "syncId"; /** + * The name of the parameter used to transmit the id of the client to server + * messages. + */ + public static final String CLIENT_TO_SERVER_ID = "clientId"; + + /** * Default value to use in case the security protection is disabled. */ public static final String CSRF_TOKEN_DEFAULT_VALUE = "init"; |