Skip to content

Commit

Permalink
feat: resend payload of reoccuring client request (#20547)
Browse files Browse the repository at this point in the history
* feat: resend payload of reoccuring client request

If the client request contains
the previous id and exactly the
same message content respond with
the same payload as previously.

Closes #20506

* Rename exception, remove initial value.

---------

Co-authored-by: Teppo Kurki <[email protected]>
  • Loading branch information
caalador and tepi authored Nov 27, 2024
1 parent 380b15f commit 14b016c
Show file tree
Hide file tree
Showing 5 changed files with 144 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,8 @@ public List<Object> getParameters() {

private byte[] lastProcessedMessageHash = null;

private String lastRequestResponse;

private String contextRootRelativePath;

private String appId;
Expand Down Expand Up @@ -305,6 +307,25 @@ public void setLastProcessedClientToServerId(
this.lastProcessedMessageHash = lastProcessedMessageHash;
}

/**
* Sets the response created for the last UIDL request.
*
* @param lastRequestResponse
* The request that was sent for the last UIDL request.
*/
public void setLastRequestResponse(String lastRequestResponse) {
this.lastRequestResponse = lastRequestResponse;
}

/**
* Returns the response created for the last UIDL request.
*
* @return The request that was sent for the last UIDL request.
*/
public String getLastRequestResponse() {
return lastRequestResponse;
}

/**
* Gets the server sync id.
* <p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,19 @@ public ResynchronizationRequiredException() {
}
}

/**
* Exception thrown when the client side re-sends the same request.
*/
public static class ClientResentPayloadException extends RuntimeException {

/**
* Default constructor for the exception.
*/
public ClientResentPayloadException() {
super();
}
}

/**
* Reads JSON containing zero or more serialized RPC calls (including legacy
* variable changes) and executes the calls.
Expand Down Expand Up @@ -317,9 +330,11 @@ public void handleRpc(UI ui, String message, VaadinRequest request)
* situation is most likely triggered by a timeout or such
* causing a message to be resent.
*/
getLogger().info(
"Ignoring old duplicate message from the client. Expected: "
+ expectedId + ", got: " + requestId);
getLogger().debug(
"Received old duplicate message from the client. Expected: "
+ expectedId + ", got: " + requestId
+ ". Resending previous response.");
throw new ClientResentPayloadException();
} else if (rpcRequest.isUnloadBeaconRequest()) {
getLogger().debug(
"Ignoring unexpected message id from the client on UNLOAD request. "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import com.vaadin.flow.server.VaadinService;
import com.vaadin.flow.server.VaadinSession;
import com.vaadin.flow.server.communication.ServerRpcHandler.InvalidUIDLSecurityKeyException;
import com.vaadin.flow.server.communication.ServerRpcHandler.ClientResentPayloadException;
import com.vaadin.flow.server.communication.ServerRpcHandler.ResynchronizationRequiredException;
import com.vaadin.flow.server.dau.DAUUtils;
import com.vaadin.flow.server.dau.DauEnforcementException;
Expand Down Expand Up @@ -134,8 +135,10 @@ public Optional<ResponseWriter> synchronizedHandleRequest(
StringWriter stringWriter = new StringWriter();

try {
getRpcHandler(session).handleRpc(uI, requestBody, request);
getRpcHandler().handleRpc(uI, requestBody, request);
writeUidl(uI, stringWriter, false);
} catch (ClientResentPayloadException e) {
stringWriter.write(uI.getInternals().getLastRequestResponse());
} catch (JsonException e) {
getLogger().error("Error writing JSON to response", e);
// Refresh on client side
Expand Down Expand Up @@ -176,6 +179,7 @@ void writeUidl(UI ui, Writer writer, boolean resync) throws IOException {

// some dirt to prevent cross site scripting
String responseString = "for(;;);[" + uidl.toJson() + "]";
ui.getInternals().setLastRequestResponse(responseString);
writer.write(responseString);
}

Expand Down Expand Up @@ -208,7 +212,7 @@ public boolean handleSessionExpired(VaadinRequest request,
return true;
}

private ServerRpcHandler getRpcHandler(VaadinSession session) {
private ServerRpcHandler getRpcHandler() {
ServerRpcHandler handler = rpcHandler.get();
if (handler == null) {
rpcHandler.compareAndSet(null, createRpcHandler());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.vaadin.flow.server.communication;

import java.io.IOException;
import java.io.Reader;
import java.io.StringReader;

import org.junit.Assert;
Expand Down Expand Up @@ -99,9 +98,9 @@ public void handleRpc_resynchronize_throwsExceptionAndDirtiesTreeAndClearsDepend
Mockito.verify(dependencyList).clearPendingSendToClient();
}

@Test
public void handleRpc_duplicateMessage_doNotThrow()
throws InvalidUIDLSecurityKeyException, IOException {
@Test(expected = ServerRpcHandler.ClientResentPayloadException.class)
public void handleRpc_duplicateMessage_throwsResendPayload()
throws InvalidUIDLSecurityKeyException {
String msg = "{\"" + ApplicationConstants.CLIENT_TO_SERVER_ID + "\":1}";
ServerRpcHandler handler = new ServerRpcHandler();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

import java.io.IOException;
import java.io.OutputStream;
import java.io.Reader;
import java.io.StringWriter;
import java.util.Collections;
import java.util.Optional;
Expand All @@ -31,6 +30,7 @@
import org.mockito.Mockito;

import com.vaadin.flow.component.UI;
import com.vaadin.flow.function.DeploymentConfiguration;
import com.vaadin.flow.server.DefaultDeploymentConfiguration;
import com.vaadin.flow.server.HandlerHelper.RequestType;
import com.vaadin.flow.server.MockVaadinContext;
Expand All @@ -46,6 +46,7 @@
import com.vaadin.flow.server.startup.ApplicationConfiguration;
import com.vaadin.flow.shared.ApplicationConstants;
import com.vaadin.pro.licensechecker.dau.EnforcementException;
import com.vaadin.tests.util.MockUI;

import elemental.json.JsonObject;
import elemental.json.impl.JsonUtil;
Expand Down Expand Up @@ -128,9 +129,73 @@ public void writeSessionExpired_whenUINotFound() throws IOException {
responseContent);
}

@Test
public void clientRequestsPreviousIdAndPayload_resendPreviousResponse()
throws IOException {

UI ui = getUi();
VaadinSession session = ui.getSession();
VaadinService service = session.getService();
DeploymentConfiguration conf = Mockito
.mock(DeploymentConfiguration.class);
Mockito.when(service.getDeploymentConfiguration()).thenReturn(conf);
Mockito.when(conf.isRequestTiming()).thenReturn(false);

String requestBody = """
{
"csrfToken": "d1f44a6f-bbe5-4493-a8a9-3f5f234a2a93",
"rpc": [
{
"type": "mSync",
"node": 12,
"feature": 1,
"property": "value",
"value": "a"
},
{
"type": "event",
"node": 12,
"event": "change",
"data": {}
}
],
"syncId": 0,
"clientId": 0
}
""";
Mockito.when(request.getService()).thenReturn(service);
Mockito.when(conf.isSyncIdCheckEnabled()).thenReturn(true);

Optional<SynchronizedRequestHandler.ResponseWriter> result = handler
.synchronizedHandleRequest(session, request, response,
requestBody);
Assert.assertTrue("ResponseWriter should be present",
result.isPresent());
result.get().writeResponse();
String responseContent = CommunicationUtil
.getStringWhenWriteString(outputStream);

// Init clean response
response = Mockito.mock(VaadinResponse.class);
outputStream = Mockito.mock(OutputStream.class);
Mockito.when(response.getOutputStream()).thenReturn(outputStream);

result = handler.synchronizedHandleRequest(session, request, response,
requestBody);
Assert.assertTrue("ResponseWriter should be present",
result.isPresent());
result.get().writeResponse();
String resendResponseContent = CommunicationUtil
.getStringWhenWriteString(outputStream);

// response shouldn't contain async
Assert.assertEquals("Server should send same content again",
responseContent, resendResponseContent);
}

@Test
public void should_modifyUidl_when_MPR() throws Exception {
UI ui = mock(UI.class);
UI ui = getUi();

UidlRequestHandler handler = spy(new UidlRequestHandler());
StringWriter writer = new StringWriter();
Expand All @@ -151,7 +216,7 @@ public void should_modifyUidl_when_MPR() throws Exception {

@Test
public void should_changeURL_when_v7LocationProvided() throws Exception {
UI ui = mock(UI.class);
UI ui = getUi();

UidlRequestHandler handler = spy(new UidlRequestHandler());
StringWriter writer = new StringWriter();
Expand All @@ -172,7 +237,7 @@ public void should_changeURL_when_v7LocationProvided() throws Exception {
@Test
public void should_updateHash_when_v7LocationNotProvided()
throws Exception {
UI ui = mock(UI.class);
UI ui = getUi();

UidlRequestHandler handler = spy(new UidlRequestHandler());
StringWriter writer = new StringWriter();
Expand All @@ -192,7 +257,7 @@ public void should_updateHash_when_v7LocationNotProvided()

@Test
public void should_not_modify_non_MPR_Uidl() throws Exception {
UI ui = mock(UI.class);
UI ui = getUi();

UidlRequestHandler handler = spy(new UidlRequestHandler());
StringWriter writer = new StringWriter();
Expand All @@ -217,7 +282,7 @@ public void should_not_modify_non_MPR_Uidl() throws Exception {
@Test
public void should_not_update_browser_history_if_no_hash_in_location()
throws Exception {
UI ui = mock(UI.class);
UI ui = getUi();

UidlRequestHandler handler = spy(new UidlRequestHandler());
StringWriter writer = new StringWriter();
Expand Down Expand Up @@ -351,4 +416,29 @@ private JsonObject getUidlWithNoHashInLocation() {
// @formatter:on
}

/**
* Mock ui with session.
*
* @return
*/
private static UI getUi() {
VaadinService service = mock(VaadinService.class);
VaadinSession session = new VaadinSession(service) {
@Override
public boolean hasLock() {
return true;
}

@Override
public VaadinService getService() {
return service;
}
};

UI ui = new MockUI(session);

when(service.findUI(Mockito.any())).thenReturn(ui);

return ui;
}
}

0 comments on commit 14b016c

Please sign in to comment.