feat: Add idempotency feature to detect duplicate requests due to network conditions #1190
Conversation
Thanks for opening this pull request!
|
|
I will reformat the title to use the proper commit message syntax. |
parse-github-assistant
bot
changed the title
DrMimik
force-pushed
the
idempotent-requests
branch
from
2d150ad
to
a1454eb
Compare
| if (masterKey != null) | ||
| jsonObject.put(HEADER_MASTER_KEY, masterKey); | ||
| requestBuilder.addHeader(HEADER_REQUEST_ID, ParseDigestUtils.md5(toDeterministicString(jsonObject))); | ||
| } catch (JSONException e) { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I am not very good at JS, but could you direct me to where the iOS SDK implements the logic.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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();
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
mtrezza
changed the title
mtrezza
changed the title
|
Should I force push the first commit? or use a new commit?. |
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## master #1190 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 122 122
Lines 9971 9973 +2
Branches 1345 1345
======================================
- Misses 9971 9973 +2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
New commit is preferable to track changes and reviewing it easily. |
New Pull Request Checklist
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