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

Explores support for idempotency #1132

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Explores support for idempotency #1132

wants to merge 1 commit into from

Conversation

bart-degreed
Copy link
Contributor

@bart-degreed bart-degreed commented Dec 23, 2021

This PR adds opt-in support for idempotent requests in JsonApiDotNetCore, as defined by IETF.

In a microservices world, where at-least-once delivery is common practice, APIs must be able to handle duplicate requests gracefully. According to the HTTP standard, only POST and PATCH are non-idempotent by nature. Since in JSON:API there is no way to express "multiply the existing value of attribute X by 3", we only need to handle POST.

When a client includes the "Idempotency-Key" HTTP header in a POST Resource (or atomic:operations) request, the server stores the response before sending it back. A subsequent POST Resource request with the same key just returns the recorded response instead of executing the request. In such cases, it sends back the idempotency key too, to inform the client this was a replay.

The mechanism applies to both success and error responses. If the client wants to retry a failed request, it should resend with another key.

The tricky part is handling concurrent requests with the same key. The chosen approach here is to wrap the entire request in a transaction, so that concurrent requests block on trying to insert a record with the same primary key. This varies per database provider and isolation level, so needs more testing.
An even more scalable solution would be to use an expiring lease that is atomically obtained, but then we'd need to know the maximum duration that processing a request can take. This is problematic when non-idempotent downstream external network service calls are made from resource definitions.

References on idempotency:

References on capturing request/response bodies in ASP.NET:

@codecov
Copy link

codecov bot commented Dec 23, 2021

Codecov Report

Merging #1132 (b98933e) into master (ea3ab72) will increase coverage by 0.03%.
The diff coverage is 95.94%.

❗ Current head b98933e differs from pull request most recent head 08f8141. Consider uploading reports for the commit 08f8141 to get more accurate results

@@            Coverage Diff             @@
##           master    #1132      +/-   ##
==========================================
+ Coverage   92.61%   92.64%   +0.03%     
==========================================
  Files         242      244       +2     
  Lines        7701     7847     +146     
==========================================
+ Hits         7132     7270     +138     
- Misses        569      577       +8     
Impacted Files Coverage Δ
...nApiDotNetCore/Middleware/NoIdempotencyProvider.cs 25.00% <25.00%> (ø)
...nApiDotNetCore/Middleware/IdempotencyMiddleware.cs 97.45% <97.45%> (ø)
...AtomicOperations/EntityFrameworkCoreTransaction.cs 100.00% <100.00%> (ø)
...perations/EntityFrameworkCoreTransactionFactory.cs 100.00% <100.00%> (ø)
...Core/Configuration/ApplicationBuilderExtensions.cs 100.00% <100.00%> (ø)
...NetCore/Configuration/JsonApiApplicationBuilder.cs 100.00% <100.00%> (ø)
...JsonApiDotNetCore/Middleware/IdempotentResponse.cs 100.00% <100.00%> (ø)
src/JsonApiDotNetCore.Annotations/ArgumentGuard.cs 73.33% <0.00%> (-6.67%) ⬇️
...ApiDotNetCore.SourceGenerators/SourceCodeWriter.cs 97.47% <0.00%> (-0.09%) ⬇️
...Core/Repositories/EntityFrameworkCoreRepository.cs 99.29% <0.00%> (-0.03%) ⬇️
... and 15 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@bkoelman bkoelman force-pushed the idempotency branch 2 times, most recently from 4c0a816 to 612cbe3 Compare April 11, 2022 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants