From a1454eb3160e174a451f2a5daba0edf880b71517 Mon Sep 17 00:00:00 2001 From: DrMimik Date: Thu, 16 Mar 2023 12:42:39 +0200 Subject: [PATCH 1/2] Idempotent Requests Implementation --- .../main/java/com/parse/ParseException.java | 2 + .../main/java/com/parse/ParseRESTCommand.java | 19 +++++++- .../com/parse/ParseRESTUserCommandTest.java | 43 +++++++++++++++++++ 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/parse/src/main/java/com/parse/ParseException.java b/parse/src/main/java/com/parse/ParseException.java index 5dc3b5f90..972a8b4aa 100644 --- a/parse/src/main/java/com/parse/ParseException.java +++ b/parse/src/main/java/com/parse/ParseException.java @@ -102,6 +102,8 @@ public class ParseException extends Exception { public static final int FILE_DELETE_ERROR = 153; /** Error code indicating that the application has exceeded its request limit. */ public static final int REQUEST_LIMIT_EXCEEDED = 155; + /** Error code indicating that the request was a duplicate and has been discarded due to idempotency rules. */ + public static final int DUPLICATE_REQUEST = 159; /** Error code indicating that the provided event name is invalid. */ public static final int INVALID_EVENT_NAME = 160; /** Error code indicating that the username is missing or empty. */ diff --git a/parse/src/main/java/com/parse/ParseRESTCommand.java b/parse/src/main/java/com/parse/ParseRESTCommand.java index 60cd1c590..1a5d25916 100644 --- a/parse/src/main/java/com/parse/ParseRESTCommand.java +++ b/parse/src/main/java/com/parse/ParseRESTCommand.java @@ -35,9 +35,10 @@ class ParseRESTCommand extends ParseRequest { /* package */ static final String HEADER_OS_VERSION = "X-Parse-OS-Version"; /* package */ static final String HEADER_INSTALLATION_ID = "X-Parse-Installation-Id"; + /* package */ static final String HEADER_REQUEST_ID = "X-Parse-Request-Id"; /* package */ static final String USER_AGENT = "User-Agent"; - private static final String HEADER_SESSION_TOKEN = "X-Parse-Session-Token"; - private static final String HEADER_MASTER_KEY = "X-Parse-Master-Key"; + /* package */ static final String HEADER_SESSION_TOKEN = "X-Parse-Session-Token"; + /* package */ static final String HEADER_MASTER_KEY = "X-Parse-Master-Key"; private static final String PARAMETER_METHOD_OVERRIDE = "_method"; // Set via Parse.initialize(Configuration) @@ -215,6 +216,20 @@ protected void addAdditionalHeaders(ParseHttpRequest.Builder requestBuilder) { if (masterKey != null) { requestBuilder.addHeader(HEADER_MASTER_KEY, masterKey); } + try { + JSONObject jsonObject = jsonParameters != null ? new JSONObject(jsonParameters.toString()) : new JSONObject(); + // using header names so we don't override a parameter with the same key name, + // using all headers to insure the requestId generated doesn't conflict with the rest of the users + if (installationId != null) + jsonObject.put(HEADER_INSTALLATION_ID, installationId); + if (sessionToken != null) + jsonObject.put(HEADER_SESSION_TOKEN, sessionToken); + if (masterKey != null) + jsonObject.put(HEADER_MASTER_KEY, masterKey); + requestBuilder.addHeader(HEADER_REQUEST_ID, ParseDigestUtils.md5(toDeterministicString(jsonObject))); + } catch (JSONException e) { + throw new RuntimeException(e.getMessage()); + } } @Override diff --git a/parse/src/test/java/com/parse/ParseRESTUserCommandTest.java b/parse/src/test/java/com/parse/ParseRESTUserCommandTest.java index a735465bf..ce5bd3eb9 100644 --- a/parse/src/test/java/com/parse/ParseRESTUserCommandTest.java +++ b/parse/src/test/java/com/parse/ParseRESTUserCommandTest.java @@ -19,6 +19,9 @@ import java.net.URL; import java.util.HashMap; import java.util.Map; +import java.util.Random; + +import org.json.JSONArray; import org.json.JSONObject; import org.junit.After; import org.junit.Before; @@ -26,6 +29,7 @@ import org.skyscreamer.jsonassert.JSONCompareMode; public class ParseRESTUserCommandTest { + private static final String ALLOWED_CHARACTERS = "0123456789qwertyuiopasdfghjklzxcvbnm"; @Before public void setUp() throws MalformedURLException { @@ -163,5 +167,44 @@ public void testOnResponseAsync() { assertEquals(200, command.getStatusCode()); } + @Test + public void testRequestIdHeader() throws Exception { + JSONArray nestedJSONArray = new JSONArray().put(true).put(1).put("test"); + JSONObject nestedJSON = + new JSONObject().put("bool", false).put("int", 2).put("string", "test"); + String sessionToken = generateRandomString(32); + String installationId = generateRandomString(32); + String masterKey = generateRandomString(32); + JSONObject json = + new JSONObject() + .put("json", nestedJSON) + .put("jsonArray", nestedJSONArray) + .put("bool", true) + .put("int", 3) + .put("string", "test"); + + String jsonString = ParseRESTCommand.toDeterministicString(json); + + JSONObject jsonAgain = new JSONObject(jsonString); + jsonAgain.put(ParseRESTCommand.HEADER_INSTALLATION_ID, installationId); + jsonAgain.put(ParseRESTCommand.HEADER_SESSION_TOKEN, sessionToken); + jsonAgain.put(ParseRESTCommand.HEADER_MASTER_KEY, masterKey); + ParseRESTCommand restCommand = new ParseRESTCommand.Builder().jsonParameters(json) + .installationId(installationId).sessionToken(sessionToken).masterKey(masterKey) + .build(); + + ParseHttpRequest.Builder builder = new ParseHttpRequest.Builder(); + restCommand.addAdditionalHeaders(builder); + assertEquals(ParseDigestUtils.md5(ParseRESTCommand.toDeterministicString(jsonAgain)), builder.build().getHeader(ParseRESTCommand.HEADER_REQUEST_ID)); + } + + private static String generateRandomString(final int sizeOfRandomString) { + final Random random = new Random(); + final StringBuilder sb = new StringBuilder(sizeOfRandomString); + for (int i = 0; i < sizeOfRandomString; ++i) + sb.append(ALLOWED_CHARACTERS.charAt(random.nextInt(ALLOWED_CHARACTERS.length()))); + return sb.toString(); + } + // endregion } From 039d197d34b90a1da32e9188584ffd0e27fb5ac4 Mon Sep 17 00:00:00 2001 From: DrMimik Date: Fri, 17 Mar 2023 11:48:58 +0200 Subject: [PATCH 2/2] rewrite idempotency logic --- .../main/java/com/parse/ParseRESTCommand.java | 22 ++----- .../com/parse/ParseRESTUserCommandTest.java | 65 ++++--------------- .../test/java/com/parse/ParseRequestTest.java | 24 +++++++ 3 files changed, 41 insertions(+), 70 deletions(-) diff --git a/parse/src/main/java/com/parse/ParseRESTCommand.java b/parse/src/main/java/com/parse/ParseRESTCommand.java index 1a5d25916..55b9114ab 100644 --- a/parse/src/main/java/com/parse/ParseRESTCommand.java +++ b/parse/src/main/java/com/parse/ParseRESTCommand.java @@ -20,6 +20,8 @@ import java.util.Collections; import java.util.Iterator; import java.util.Map; +import java.util.UUID; + import org.json.JSONArray; import org.json.JSONException; import org.json.JSONObject; @@ -37,8 +39,8 @@ class ParseRESTCommand extends ParseRequest { /* package */ static final String HEADER_INSTALLATION_ID = "X-Parse-Installation-Id"; /* package */ static final String HEADER_REQUEST_ID = "X-Parse-Request-Id"; /* package */ static final String USER_AGENT = "User-Agent"; - /* package */ static final String HEADER_SESSION_TOKEN = "X-Parse-Session-Token"; - /* package */ static final String HEADER_MASTER_KEY = "X-Parse-Master-Key"; + private static final String HEADER_SESSION_TOKEN = "X-Parse-Session-Token"; + private static final String HEADER_MASTER_KEY = "X-Parse-Master-Key"; private static final String PARAMETER_METHOD_OVERRIDE = "_method"; // Set via Parse.initialize(Configuration) @@ -50,6 +52,7 @@ class ParseRESTCommand extends ParseRequest { /* package */ String httpPath; private String installationId; private String operationSetUUID; + private final String requestId = UUID.randomUUID().toString(); private String localId; public ParseRESTCommand( @@ -216,20 +219,7 @@ protected void addAdditionalHeaders(ParseHttpRequest.Builder requestBuilder) { if (masterKey != null) { requestBuilder.addHeader(HEADER_MASTER_KEY, masterKey); } - try { - JSONObject jsonObject = jsonParameters != null ? new JSONObject(jsonParameters.toString()) : new JSONObject(); - // using header names so we don't override a parameter with the same key name, - // using all headers to insure the requestId generated doesn't conflict with the rest of the users - if (installationId != null) - jsonObject.put(HEADER_INSTALLATION_ID, installationId); - if (sessionToken != null) - jsonObject.put(HEADER_SESSION_TOKEN, sessionToken); - if (masterKey != null) - jsonObject.put(HEADER_MASTER_KEY, masterKey); - requestBuilder.addHeader(HEADER_REQUEST_ID, ParseDigestUtils.md5(toDeterministicString(jsonObject))); - } catch (JSONException e) { - throw new RuntimeException(e.getMessage()); - } + requestBuilder.addHeader(HEADER_REQUEST_ID, requestId); } @Override diff --git a/parse/src/test/java/com/parse/ParseRESTUserCommandTest.java b/parse/src/test/java/com/parse/ParseRESTUserCommandTest.java index ce5bd3eb9..517b9716e 100644 --- a/parse/src/test/java/com/parse/ParseRESTUserCommandTest.java +++ b/parse/src/test/java/com/parse/ParseRESTUserCommandTest.java @@ -19,9 +19,6 @@ import java.net.URL; import java.util.HashMap; import java.util.Map; -import java.util.Random; - -import org.json.JSONArray; import org.json.JSONObject; import org.junit.After; import org.junit.Before; @@ -29,7 +26,6 @@ import org.skyscreamer.jsonassert.JSONCompareMode; public class ParseRESTUserCommandTest { - private static final String ALLOWED_CHARACTERS = "0123456789qwertyuiopasdfghjklzxcvbnm"; @Before public void setUp() throws MalformedURLException { @@ -59,7 +55,7 @@ public void testGetCurrentUserCommand() { @Test public void testLogInUserCommand() throws Exception { ParseRESTUserCommand command = - ParseRESTUserCommand.logInUserCommand("userName", "password", true); + ParseRESTUserCommand.logInUserCommand("userName", "password", true); assertEquals("login", command.httpPath); assertEquals(ParseHttpRequest.Method.GET, command.method); @@ -72,7 +68,7 @@ public void testLogInUserCommand() throws Exception { @Test public void testResetPasswordResetCommand() throws Exception { ParseRESTUserCommand command = - ParseRESTUserCommand.resetPasswordResetCommand("test@parse.com"); + ParseRESTUserCommand.resetPasswordResetCommand("test@parse.com"); assertEquals("requestPasswordReset", command.httpPath); assertEquals(ParseHttpRequest.Method.POST, command.method); @@ -86,7 +82,7 @@ public void testSignUpUserCommand() throws Exception { JSONObject parameters = new JSONObject(); parameters.put("key", "value"); ParseRESTUserCommand command = - ParseRESTUserCommand.signUpUserCommand(parameters, "sessionToken", true); + ParseRESTUserCommand.signUpUserCommand(parameters, "sessionToken", true); assertEquals("users", command.httpPath); assertEquals(ParseHttpRequest.Method.POST, command.method); @@ -100,7 +96,7 @@ public void testServiceLogInUserCommandWithParameters() throws Exception { JSONObject parameters = new JSONObject(); parameters.put("key", "value"); ParseRESTUserCommand command = - ParseRESTUserCommand.serviceLogInUserCommand(parameters, "sessionToken", true); + ParseRESTUserCommand.serviceLogInUserCommand(parameters, "sessionToken", true); assertEquals("users", command.httpPath); assertEquals(ParseHttpRequest.Method.POST, command.method); @@ -114,7 +110,7 @@ public void testServiceLogInUserCommandWithAuthType() throws Exception { Map facebookAuthData = new HashMap<>(); facebookAuthData.put("token", "test"); ParseRESTUserCommand command = - ParseRESTUserCommand.serviceLogInUserCommand("facebook", facebookAuthData, true); + ParseRESTUserCommand.serviceLogInUserCommand("facebook", facebookAuthData, true); assertEquals("users", command.httpPath); assertEquals(ParseHttpRequest.Method.POST, command.method); @@ -136,7 +132,7 @@ public void testAddAdditionalHeaders() throws Exception { JSONObject parameters = new JSONObject(); parameters.put("key", "value"); ParseRESTUserCommand command = - ParseRESTUserCommand.signUpUserCommand(parameters, "sessionToken", true); + ParseRESTUserCommand.signUpUserCommand(parameters, "sessionToken", true); ParseHttpRequest.Builder requestBuilder = new ParseHttpRequest.Builder(); command.addAdditionalHeaders(requestBuilder); @@ -157,54 +153,15 @@ public void testOnResponseAsync() { int statusCode = 200; ParseHttpResponse response = - new ParseHttpResponse.Builder() - .setContent(new ByteArrayInputStream(content.getBytes())) - .setContentType(contentType) - .setStatusCode(statusCode) - .build(); + new ParseHttpResponse.Builder() + .setContent(new ByteArrayInputStream(content.getBytes())) + .setContentType(contentType) + .setStatusCode(statusCode) + .build(); command.onResponseAsync(response, null); assertEquals(200, command.getStatusCode()); } - @Test - public void testRequestIdHeader() throws Exception { - JSONArray nestedJSONArray = new JSONArray().put(true).put(1).put("test"); - JSONObject nestedJSON = - new JSONObject().put("bool", false).put("int", 2).put("string", "test"); - String sessionToken = generateRandomString(32); - String installationId = generateRandomString(32); - String masterKey = generateRandomString(32); - JSONObject json = - new JSONObject() - .put("json", nestedJSON) - .put("jsonArray", nestedJSONArray) - .put("bool", true) - .put("int", 3) - .put("string", "test"); - - String jsonString = ParseRESTCommand.toDeterministicString(json); - - JSONObject jsonAgain = new JSONObject(jsonString); - jsonAgain.put(ParseRESTCommand.HEADER_INSTALLATION_ID, installationId); - jsonAgain.put(ParseRESTCommand.HEADER_SESSION_TOKEN, sessionToken); - jsonAgain.put(ParseRESTCommand.HEADER_MASTER_KEY, masterKey); - ParseRESTCommand restCommand = new ParseRESTCommand.Builder().jsonParameters(json) - .installationId(installationId).sessionToken(sessionToken).masterKey(masterKey) - .build(); - - ParseHttpRequest.Builder builder = new ParseHttpRequest.Builder(); - restCommand.addAdditionalHeaders(builder); - assertEquals(ParseDigestUtils.md5(ParseRESTCommand.toDeterministicString(jsonAgain)), builder.build().getHeader(ParseRESTCommand.HEADER_REQUEST_ID)); - } - - private static String generateRandomString(final int sizeOfRandomString) { - final Random random = new Random(); - final StringBuilder sb = new StringBuilder(sizeOfRandomString); - for (int i = 0; i < sizeOfRandomString; ++i) - sb.append(ALLOWED_CHARACTERS.charAt(random.nextInt(ALLOWED_CHARACTERS.length()))); - return sb.toString(); - } - // endregion } diff --git a/parse/src/test/java/com/parse/ParseRequestTest.java b/parse/src/test/java/com/parse/ParseRequestTest.java index f3ec62478..1d833d1cf 100644 --- a/parse/src/test/java/com/parse/ParseRequestTest.java +++ b/parse/src/test/java/com/parse/ParseRequestTest.java @@ -10,8 +10,10 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -24,8 +26,11 @@ import java.io.ByteArrayInputStream; import java.io.File; import java.io.IOException; +import java.net.URL; import java.util.LinkedList; import java.util.List; +import java.util.concurrent.atomic.AtomicReference; + import org.junit.After; import org.junit.AfterClass; import org.junit.Before; @@ -121,6 +126,25 @@ public void testDownloadProgress() throws Exception { assertProgressCompletedSuccessfully(downloadProgressCallback); } + @Test + public void testIdempotencyLogic() throws Exception { + ParseHttpClient mockHttpClient = mock(ParseHttpClient.class); + AtomicReference requestIdAtomicReference = new AtomicReference<>(); + when(mockHttpClient.execute(argThat(argument -> { + assertNotNull(argument.getHeader(ParseRESTCommand.HEADER_REQUEST_ID)); + if (requestIdAtomicReference.get() == null) requestIdAtomicReference.set(argument.getHeader(ParseRESTCommand.HEADER_REQUEST_ID)); + assertEquals(argument.getHeader(ParseRESTCommand.HEADER_REQUEST_ID), requestIdAtomicReference.get()); + return true; + }))).thenThrow(new IOException()); + + ParseRESTCommand.server = new URL("http://parse.com"); + ParseRESTCommand command = new ParseRESTCommand.Builder().build(); + Task task = command.executeAsync(mockHttpClient).makeVoid(); + task.waitForCompletion(); + + verify(mockHttpClient, times(5)).execute(any(ParseHttpRequest.class)); + } + private static class TestProgressCallback implements ProgressCallback { final List history = new LinkedList<>();