fix(requests): serialize request creation per user - #3377
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
f313701 to
dc73871
Compare
There was a problem hiding this comment.
Pull request overview
This PR addresses a race condition in request creation where multiple concurrent requests from the same user could interleave quota/duplicate checks and exceed limits. It introduces a per-user async lock so quota validation and persistence are serialized for each user within a single running instance.
Changes:
- Added a shared per-user
requestLockand wrappedMediaRequest.request()in it. - Generalized
AsyncLock.dispatch()to return the callback result (genericPromise<T>). - Added a unit test to verify concurrent quota enforcement and duplicate request rejection.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| server/utils/requestLock.ts | Introduces a shared AsyncLock instance intended to serialize request creation per user. |
| server/utils/asyncLock.ts | Updates dispatch to be generic and return the callback’s result while preserving lock release semantics. |
| server/entity/MediaRequest.ts | Wraps request creation in the per-user lock and extracts the original logic into a private helper. |
| server/entity/MediaRequest.test.ts | Adds concurrency-focused tests for quota and duplicate-request behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The quota check read its counts and then saved with nothing in between to stop a second request from the same user passing the same check, so concurrent requests could all clear a quota that only had room for one. The collection request modal submits its parts in parallel, so this was reachable from the UI. Request creation now runs under a per-user lock, which also closes the same-user duplicate and auto-request races that had the same shape. The lock is taken before any repository call, so a waiter holds no pool connection while blocked.
dc73871 to
766b2ed
Compare
Description
The quota check reads its counts and then saves, with nothing in between stopping another request from the same user passing the same check. Several requests submitted at once could all clear a quota with room for one. That is not theoretical, the collection request modal submits one request per selected part in parallel, so requesting a collection has always been able to go past the limit.
Creating a request now runs under a per-user lock, so the check and the save that follows it cannot interleave with that user's other requests. The lock also closes the same-user duplicate submit and the auto-request duplicate check, which had the same shape. It is taken before any database work, so a request waiting on it holds no pool connection.
How Has This Been Tested?
Screenshots / Logs (if applicable)
Checklist:
pnpm buildpnpm i18n:extract