feat: Add idempotency feature to detect duplicate requests due to network conditions #1190

ghost · 2023-03-16T11:09:44Z

New Pull Request Checklist

I am not disclosing a vulnerability.
I am creating this PR in reference to an issue.

Issue Description

Requests are duplicated on slow internet connections and during network interruptions.

Closes: #764

Approach

Add idempotency requestId header for client requests to enforce server side idempotency middleware on selected routes..

TODOs before merging

Add tests
Add changes to documentation (guides, repository pages, in-code descriptions)

parse-github-assistant · 2023-03-16T11:09:47Z

Thanks for opening this pull request!

🎉 We are excited about your hands-on contribution!

parse-github-assistant · 2023-03-16T13:10:01Z

I will reformat the title to use the proper commit message syntax.

mtrezza · 2023-03-16T21:41:27Z

parse/src/main/java/com/parse/ParseRESTCommand.java

+ if (masterKey != null)
+ jsonObject.put(HEADER_MASTER_KEY, masterKey);
+ requestBuilder.addHeader(HEADER_REQUEST_ID, ParseDigestUtils.md5(toDeterministicString(jsonObject)));
+ } catch (JSONException e) {


The OS should provide a UUID API that takes care of seeding. Could you use the same logic for generating the UUID as we use in the other SDKs? For example the iOS SDK and JS SDK?

I have rewritten the logic to use a UUID, as this is uniformed between SDKs.

But imo why isn't the UUID data based?, like this if the SDK calls ParseObject.save() it changes the ParseRequest which creates a new UUID, while using a data based UUID calling save won't change the UUID header.

Currently if I want the SDK to create an idempotent request I'd still set the HttpClient to use a longer timeout and increase the default max retries of requests.

I am not very good at JS, but could you direct me to where the iOS SDK implements the logic.

But imo why isn't the UUID data based?

It doesn't need to be based on object data. A UUID is a Universally Unique IDentifier, which ensures that every generated UUID is unique.

The part you'd have to take care about in the PR is that for a ParseObject.save that UUID is only generated one time and then reused if the request fails and the SDK auto-retries the request. Obviously, if you generated a new UUID with ever retry of the same request, then this wouldn't work.

The current approach works as follows if all requests were unsuccessfull:

ParseObject object = new ParseObject("GameScore");
// calls a request and retries according to the max retries (default is 4), with the same requestId header. (5 calls with the same header)
object.save();

// this second method call... calls a request and retries according to the max retries set, with the same requestId header but different than the first save() method call.
object.save();

with the same requestId header but different than the first save() method call.

Not sure what how to read that. The second save call should use a different UUID for the request ID than the first call. In fact, every save call should use its own UUID. Only auto-retries of each save call should re-use their initial UUID.

Yes, It is using exactly that. My explanation was kind of confusing.

I'll add a second test using the code in my previous comment.

parse/src/main/java/com/parse/ParseRESTCommand.java

ghost · 2023-03-17T09:59:28Z

Should I force push the first commit? or use a new commit?.

codecov · 2023-03-19T13:00:26Z

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 0.00%. Comparing base (83aec68) to head (039d197).
Report is 1 commits behind head on master.

❗ Current head 039d197 differs from pull request most recent head 76673eb

Please upload reports for the commit 76673eb to get more accurate results.

Files	Patch %	Lines
...arse/src/main/java/com/parse/ParseRESTCommand.java	0.00%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #1190    +/-   ##
=======================================
  Coverage    0.00%   0.00%            
=======================================
  Files         124     122     -2     
  Lines       10076    9973   -103     
  Branches     1359    1345    -14     
=======================================
+ Misses      10076    9973   -103

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

azlekov · 2023-05-16T07:50:10Z

Should I force push the first commit? or use a new commit?.

New commit is preferable to track changes and reviewing it easily.
Hoping to get this merged.

dplewis · 2024-06-07T17:48:36Z

@mtrezza Looks like this is ready for review

mtrezza

Lint failed

mtrezza · 2024-08-05T14:07:05Z

@parse-community/android-sdk anyone wants to pick this up and submit a new PR? It seems this was almost ready for merge, except for lint issues.

abubkr-hago · 2024-08-14T14:00:45Z

Hi, I am a new contributor, I would love to continue on this PR

rommansabbir · 2024-08-14T15:26:54Z

Hi, I am a new contributor, I would love to continue on this PR

That would be great!.

mtrezza · 2024-08-14T15:51:13Z

Sure, please go ahead.

ghost changed the title ~~Idempotent Requests Implementation~~ fix: duplicate requests Mar 16, 2023

parse-github-assistant bot changed the title ~~fix: duplicate requests~~ fix: Duplicate requests Mar 16, 2023

ghost changed the title ~~fix: Duplicate requests~~ fix: Duplicate Requests Mar 16, 2023

Idempotent Requests Implementation

a1454eb

ghost force-pushed the idempotent-requests branch from 2d150ad to a1454eb Compare March 16, 2023 14:38

mtrezza requested changes Mar 16, 2023

View reviewed changes

mtrezza changed the title ~~fix: Duplicate Requests~~ feat: Add indempotency feature to detect duplicate requests due to network conditions Mar 16, 2023

mtrezza changed the title ~~feat: Add indempotency feature to detect duplicate requests due to network conditions~~ feat: Add idempotency feature to detect duplicate requests due to network conditions Mar 16, 2023

rewrite idempotency logic

039d197

dplewis mentioned this pull request Jun 8, 2024

feat: Enable Parse.idempotency by default parse-community/Parse-SDK-JS#2164

Closed

2 tasks

Merge branch 'master' into idempotent-requests

76673eb

mtrezza requested changes Jun 11, 2024

View reviewed changes

ghost closed this Aug 3, 2024

ghost deleted the idempotent-requests branch August 3, 2024 19:15

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add idempotency feature to detect duplicate requests due to network conditions #1190

feat: Add idempotency feature to detect duplicate requests due to network conditions #1190

ghost commented Mar 16, 2023 •

edited by ghost

Loading

parse-github-assistant bot commented Mar 16, 2023 •

edited

Loading

parse-github-assistant bot commented Mar 16, 2023

mtrezza Mar 16, 2023

ghost Mar 17, 2023

ghost Mar 17, 2023 •

edited by ghost

Loading

mtrezza Mar 18, 2023

ghost Mar 18, 2023 •

edited by ghost

Loading

mtrezza Mar 18, 2023 •

edited

Loading

ghost Mar 19, 2023

ghost commented Mar 17, 2023

codecov bot commented Mar 19, 2023 •

edited

Loading

azlekov commented May 16, 2023

dplewis commented Jun 7, 2024 •

edited

Loading

mtrezza left a comment

mtrezza commented Aug 5, 2024 •

edited

Loading

abubkr-hago commented Aug 14, 2024

rommansabbir commented Aug 14, 2024

mtrezza commented Aug 14, 2024

feat: Add idempotency feature to detect duplicate requests due to network conditions #1190

feat: Add idempotency feature to detect duplicate requests due to network conditions #1190

Conversation

ghost commented Mar 16, 2023 • edited by ghost Loading

New Pull Request Checklist

Issue Description

Approach

TODOs before merging

parse-github-assistant bot commented Mar 16, 2023 • edited Loading

Thanks for opening this pull request!

parse-github-assistant bot commented Mar 16, 2023

mtrezza Mar 16, 2023

Choose a reason for hiding this comment

ghost Mar 17, 2023

Choose a reason for hiding this comment

ghost Mar 17, 2023 • edited by ghost Loading

Choose a reason for hiding this comment

mtrezza Mar 18, 2023

Choose a reason for hiding this comment

ghost Mar 18, 2023 • edited by ghost Loading

Choose a reason for hiding this comment

mtrezza Mar 18, 2023 • edited Loading

Choose a reason for hiding this comment

ghost Mar 19, 2023

Choose a reason for hiding this comment

ghost commented Mar 17, 2023

codecov bot commented Mar 19, 2023 • edited Loading

Codecov Report

azlekov commented May 16, 2023

dplewis commented Jun 7, 2024 • edited Loading

mtrezza left a comment

Choose a reason for hiding this comment

mtrezza commented Aug 5, 2024 • edited Loading

abubkr-hago commented Aug 14, 2024

rommansabbir commented Aug 14, 2024

mtrezza commented Aug 14, 2024

ghost commented Mar 16, 2023 •

edited by ghost

Loading

parse-github-assistant bot commented Mar 16, 2023 •

edited

Loading

ghost Mar 17, 2023 •

edited by ghost

Loading

ghost Mar 18, 2023 •

edited by ghost

Loading

mtrezza Mar 18, 2023 •

edited

Loading

codecov bot commented Mar 19, 2023 •

edited

Loading

dplewis commented Jun 7, 2024 •

edited

Loading

mtrezza commented Aug 5, 2024 •

edited

Loading