Skip to content

fix(platform-links): prevent duplicate displayOrder via unique constraint and retry#556

Open
Ridanshi wants to merge 2 commits into
Dev-Card:mainfrom
Ridanshi:fix/platform-link-display-order-race
Open

fix(platform-links): prevent duplicate displayOrder via unique constraint and retry#556
Ridanshi wants to merge 2 commits into
Dev-Card:mainfrom
Ridanshi:fix/platform-link-display-order-race

Conversation

@Ridanshi

Copy link
Copy Markdown
Contributor

Summary

Fixes #485 — concurrent platform-link creation corrupts link ordering by assigning duplicate displayOrder values.

Root cause: createPlatformLink read max(displayOrder) and inserted in two separate operations with no transaction and no DB constraint. Two concurrent requests reading the same max would both attempt to insert the same displayOrder.

Changes:

  • schema.prisma: add @@unique([userId, displayOrder]) to PlatformLink; migration in 20260612000000_platform_link_unique_display_order/
  • profileService.createPlatformLink: retry loop (max 5 attempts) re-reads max on each attempt, retries on P2002 unique constraint violations
  • profileService.reorderLinks: two-phase interactive transaction (shift to temp offset → set final values) prevents unique constraint violations when adjacent positions swap
  • profile-cache.test.ts: update $transaction mock to handle both array and callback forms
  • platform-link-ordering.test.ts: 27 new tests covering display order assignment, P2002 retry (single, exhausted, non-P2002), concurrent simulation, two-phase reorder, ordering integrity, and regression

Test Plan

  • 27/27 new tests pass (platform-link-ordering.test.ts)
  • 18/18 cache tests preserved (profile-cache.test.ts)
  • 8/8 profile tests pass (profiles.test.ts)
  • ESLint — no errors
  • TypeScript — no errors

@vercel

vercel Bot commented Jun 12, 2026

Copy link
Copy Markdown

@Ridanshi is attempting to deploy a commit to the Prashantkumar Khatri's projects Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions

Copy link
Copy Markdown

Hi @Ridanshi,

Thanks for opening this pull request.

This PR has been automatically classified based on the files modified.

Applied Labels

  • backend

Primary Review Area

  • backend

Reviewer

@Harxhit has been identified as the primary reviewer for this pull request.

If you have any questions regarding the affected area or implementation details, feel free to reach out to the assigned reviewer.

Thank you for your contribution!

@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown

CI — Checks Failed

Backend — FAIL

Check Result
Lint FAIL
Test FAIL
Typecheck PASS

Mobile — SKIP

Check Result
Lint -
Test -

Web — SKIP

Check Result
Check -
Build -

Last updated: Fri, 12 Jun 2026 13:24:31 GMT

Ridanshi added 2 commits June 12, 2026 18:52
…ions

Root cause: createPlatformLink, updatePlatformLink, deletePlatformLink, and
reorderLinks all mutated the database but never called redis.del on the
profile:<username> cache key, leaving stale data served to viewers until
the 5-minute TTL expired naturally.

Fix: Add a private invalidateProfileCacheForUser helper that resolves the
username via a lightweight SELECT then calls redis.del. All four mutation
functions now await this helper after a successful DB write so the cache
is cleared immediately. Cache invalidation is skipped when Redis is absent
and errors are caught and logged non-fatally so a Redis blip never fails
a mutation request.

Also fix the DELETE /api/cards/:id route handler which checked error codes
as return values; the service throws errors, so the handler now catches them.
Fix cards.test.ts duplicate buildApp declaration, and apply the PlatformLink
type fix to cardService.ts (upstream/main has not yet merged that PR).

Tests: 21 new tests in profile-cache.test.ts cover cache hit/miss lifecycle,
all four mutation paths, failed mutations, non-existent links, Redis-absent
mode, consecutive mutations, cache repopulation, and non-fatal Redis errors.
…aint and retry

Concurrent createPlatformLink calls both read the same max(displayOrder)
and insert the same value, corrupting link ordering for the user.

- Add @@unique([userId, displayOrder]) to PlatformLink schema with migration
- Wrap createPlatformLink in a retry loop (max 5 attempts) that re-reads
  max and retries on P2002 unique constraint violations
- Reorder uses two-phase transaction (temp offset then final values) to
  avoid constraint conflicts when adjacent positions swap
- Add platform-link-ordering.test.ts covering concurrency, retry, two-phase
  reorder, ordering integrity, and regression scenarios

Closes Dev-Card#485
@Ridanshi Ridanshi force-pushed the fix/platform-link-display-order-race branch from 7f4104b to b9591d0 Compare June 12, 2026 13:23
@ShantKhatri

Copy link
Copy Markdown
Contributor

I'm not able to understand the changes here.

@Ridanshi

Copy link
Copy Markdown
Contributor Author

Thanks for the review, @ShantKhatri! Let me break this down clearly.
The Problem (Issue #485)
When two users tried to create a platform link at the same time, both requests would read the same max(displayOrder) value and then both try to insert with that same order number causing duplicate displayOrder values and corrupting the display order.

What Changed & Why

  1. schema.prisma: DB Constraint (@@unique([userId, displayOrder]))
    Added a unique constraint so the database itself rejects any duplicate (userId, displayOrder) pair. This is the ground-truth enforcement layer.

  2. profileService.createPlatformLink: Retry Loop
    Since two concurrent requests can still race to insert before either gets rejected, we now catch P2002 (Prisma's unique constraint violation code), re-read the current max(displayOrder), and retry up to 5 times. This handles the race gracefully instead of crashing.

  3. profileService.reorderLinks: Two-Phase Transaction
    When reordering (e.g., swapping positions 2 and 3), a naive update would temporarily set both to position 2, violating the unique constraint mid-transaction. The fix: first shift all affected rows to a temporary high offset, then set the final values — so the constraint is never violated during the swap.

  4. Tests

27 new tests in platform-link-ordering.test.ts covering all the new paths (retry logic, concurrent simulation, two-phase reorder)

Updated $transaction mock in profile-cache.test.ts to handle both array and callback forms (Prisma supports both)

Happy to walk through any specific file in the diff if anything is still unclear!

@Harxhit Harxhit added the gssoc:approved Required label for every approved PR. Gives the base +50 points and enables contribution tracking. label Jun 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend gssoc:approved Required label for every approved PR. Gives the base +50 points and enables contribution tracking.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Concurrent platform-link creation can assign duplicate display orders and corrupt link ordering

3 participants