Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: resend payload of reoccuring client request #20547

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
}
}
Loading