docs: sync from backend dedc5fe - close remaining audit items
This commit is contained in:
@@ -94,6 +94,13 @@ updated: 2026-06-07
|
||||
| H34: chat `messages` JSONB array hot path → normalized `chat_messages` table, backfill migration, row-level send/read/edit/delete methods, DB pagination | `04de406` v2.10.3 |
|
||||
| H35: chat `participants`/`unreadCounts` JSONB arrays → normalized `chat_participants` + `chat_unread_counts` tables and relational list/count predicates | `04de406` v2.10.3 |
|
||||
| H36: dispute `messages` JSONB array → normalized `dispute_messages` table with migration backfill and relational hydration | `04de406` v2.10.3 |
|
||||
| M15: `addPoints` level-up snapshot read-before-write → previous level comes from the serializable row-lock repo transaction | `dedc5fe` v2.10.4 |
|
||||
| M18: shop settings seller UUID resolution used pool outside transaction client → resolver now uses active transaction client | `dedc5fe` v2.10.4 |
|
||||
| M19: dispute admin/status/evidence timeline writes → atomic SQL JSONB appends covered by regression guard | `dedc5fe` v2.10.4 |
|
||||
| M21: template-checkout duplicate cleanup list+in-memory JSONB filter → one targeted repo SQL delete | `dedc5fe` v2.10.4 |
|
||||
| M25: purchase-request seller visibility filtered after pagination → seller visibility pushed into SQL count/page predicates | `dedc5fe` v2.10.4 |
|
||||
| M32: `seller_offers(seller_id,status)` compound index → schema/migration coverage verified by regression guard | `dedc5fe` v2.10.4 |
|
||||
| L4: `funds_ledger_entries(payment_id,entry_type)` covering index → schema/migration coverage verified by regression guard | `dedc5fe` v2.10.4 |
|
||||
|
||||
---
|
||||
|
||||
@@ -755,13 +762,13 @@ After the PG transaction commits (line 365), the code unconditionally calls `upd
|
||||
|
||||
---
|
||||
|
||||
### 15. addPoints reads snapshot before write without a transaction — TOCTOU on level-up check
|
||||
### 15. addPoints reads snapshot before write without a transaction — TOCTOU on level-up check | **FIXED** `dedc5fe` v2.10.4
|
||||
|
||||
> **Category:** Missing Transaction | **File:** `src/services/points/PointsService.ts:106-109`
|
||||
|
||||
`getUserPointsSnapshot(userId)` is called (line 106) to capture the pre-add level, then `pointsRepo.addPoints` is called (line 107). With concurrent requests there is no serialisation: two simultaneous addPoints calls can both read the same pre-level snapshot, both detect a level-up, and emit duplicate `level-up` socket events.
|
||||
|
||||
**Fix:** Perform the read-and-write atomically in the repo layer with `SELECT … FOR UPDATE` inside a transaction, or return `previousLevel` from `addPoints` itself.
|
||||
**Fix:** `DrizzlePointsRepo.addPoints` now reads the pre-add balance/level inside a `BEGIN ISOLATION LEVEL SERIALIZABLE` transaction after locking the user row with `SELECT ... FOR UPDATE`. The repo records `levelBefore`/`levelAfter` in the point-transaction metadata, and `PointsService.addPoints` emits level-up events from that transaction result instead of doing a pre-write snapshot read.
|
||||
|
||||
---
|
||||
|
||||
@@ -785,23 +792,23 @@ The controller checks `User.findOne({ email: user.pendingEmail, _id: { $ne: user
|
||||
|
||||
---
|
||||
|
||||
### 18. shopSettingsStore: resolveSellerUuid called outside the transaction client before BEGIN
|
||||
### 18. shopSettingsStore: resolveSellerUuid called outside the transaction client before BEGIN | **FIXED** `dedc5fe` v2.10.4
|
||||
|
||||
> **Category:** Missing Transaction | **File:** `src/services/marketplace/shopSettingsStore.ts:224`
|
||||
|
||||
`resolveSellerUuid(sellerId)` runs against the pool (not the transaction `client`) before `BEGIN` is issued. If the resolve fails or returns null due to timing, the INSERT proceeds with `seller_id = null` despite the user existing. There is no retry inside the transaction.
|
||||
|
||||
**Fix:** Move the `resolveSellerUuid` call inside the transaction, using the transaction `client` instead of the pool, so the lookup and the upsert see the same snapshot.
|
||||
**Fix:** `resolveSellerUuid` now accepts a queryable client and `updateSellerShopSettings` calls it after `BEGIN` with the active transaction `client`, so seller resolution and the shop-settings upsert share the same transaction connection.
|
||||
|
||||
---
|
||||
|
||||
### 19. DisputeService: assignAdmin, updateStatus, addEvidence read-then-write without optimistic locking
|
||||
### 19. DisputeService: assignAdmin, updateStatus, addEvidence read-then-write without optimistic locking | **FIXED** `dedc5fe` v2.10.4
|
||||
|
||||
> **Category:** Missing Transaction | **File:** `src/db/repositories/drizzle/DrizzleDisputeRepo.ts:209-265, 304-336`
|
||||
|
||||
All three write methods call `loadRow()` to read existing `timeline`/`evidence` arrays, append in JS, then UPDATE. No version column or wrapping transaction. Two concurrent calls read the same state; the second UPDATE silently overwrites the first timeline entry. This is the repo-layer counterpart to the service-level finding on DisputeService.ts.
|
||||
|
||||
**Fix:** Add an `updatedAt` optimistic lock: reject if `updatedAt` changed between read and write, or use a Postgres JSONB append operator (`timeline = timeline || $newEntry::jsonb`) inside a single UPDATE statement to avoid the read.
|
||||
**Fix:** The Drizzle dispute repository uses atomic SQL JSONB append expressions for timeline and evidence updates (`timeline: appendJsonb(...)`, `evidence: appendJsonb(...)`) instead of reading arrays into JavaScript and writing them back. `dedc5fe` adds regression coverage to keep the read-modify-write pattern from returning.
|
||||
|
||||
---
|
||||
|
||||
@@ -815,13 +822,13 @@ All three write methods call `loadRow()` to read existing `timeline`/`evidence`
|
||||
|
||||
---
|
||||
|
||||
### 21. Template checkout duplicate-cleanup fetches up to 100 payments with in-memory JSONB metadata filtering
|
||||
### 21. Template checkout duplicate-cleanup fetches up to 100 payments with in-memory JSONB metadata filtering | **FIXED** `dedc5fe` v2.10.4
|
||||
|
||||
> **Category:** Unbounded Fetch | **File:** `src/services/payment/paymentCoordinator.ts:558-590`
|
||||
|
||||
Lines 558-564 call `paymentRepo.list({ provider: 'shkeeper', direction: 'in', startDate: ..., limit: 100 })` to find duplicates, then lines 565-581 apply a multi-condition JavaScript `.filter()` over the 100-row dataset. The filter checks metadata fields (`meta.templateCheckout`, `meta.isTemplateCheckout`, `meta.templateCheckoutId`) stored inside JSONB and not indexed. The 100-row cap is an arbitrary workaround.
|
||||
|
||||
**Fix:** Push filter conditions into the repo query using indexed columns (e.g. `purchaseRequestId`, `providerPaymentId` prefix). For metadata-based conditions, add a partial GIN index on `metadata` for template-checkout fields, or add a dedicated boolean `is_template_checkout` column.
|
||||
**Fix:** `PaymentCoordinator` now calls `paymentRepo.deleteDuplicateTemplateCheckoutPayments(...)` instead of listing candidates and filtering in memory. `DrizzlePaymentRepo` performs one bounded `DELETE ... WHERE` scoped by provider, direction, status, created-at window, payment id exclusion, purchase-request reference, provider-payment prefix, template external-ref prefix, and template-checkout JSONB markers.
|
||||
|
||||
---
|
||||
|
||||
@@ -855,13 +862,13 @@ When `options.latestPerRequest` is true, the query fetches every matching paymen
|
||||
|
||||
---
|
||||
|
||||
### 25. findPurchaseRequests totalItems set to filteredRows.length after in-memory filter — breaks pagination contract
|
||||
### 25. findPurchaseRequests totalItems set to filteredRows.length after in-memory filter — breaks pagination contract | **FIXED** `dedc5fe` v2.10.4
|
||||
|
||||
> **Category:** Bad Pagination | **File:** `src/db/repositories/drizzle/DrizzleMarketplaceRepo.ts:979-999`
|
||||
|
||||
The DB COUNT(*) query (lines 893-905) counts rows before the seller-visibility in-memory filter (lines 979-989). `pagination.totalItems` is then set to `filteredRows.length` (the post-filter count of the current page), not the true total. A client calculating `totalPages` will compute a value smaller than reality, causing pagination to stop early. The code documents this risk: 'RISK: in-memory filter after DB pagination means effective page size shrinks.'
|
||||
|
||||
**Fix:** Push the seller-visibility filter into SQL using conditional JOINs with `seller_offers` and `purchase_request_preferred_sellers`, conditioned on request status. The in-memory switch block (lines 957-973) maps directly to SQL CASE / conditional JOIN predicates.
|
||||
**Fix:** `DrizzleMarketplaceRepo.findPurchaseRequests` now builds a `sellerVisibilityPredicate` SQL predicate with `EXISTS` checks against `seller_offers` and `purchase_request_preferred_sellers`. Both the page query and `COUNT(*)` use the same predicate, and `totalItems` now reports the SQL `total` instead of the current page's filtered row count.
|
||||
|
||||
---
|
||||
|
||||
@@ -925,13 +932,13 @@ Two gaps: (1) The `tags` array column has no GIN index — `listPublished()` fil
|
||||
|
||||
---
|
||||
|
||||
### 32. No compound index on seller_offers(seller_id, status) for seller offer listing queries
|
||||
### 32. No compound index on seller_offers(seller_id, status) for seller offer listing queries | **FIXED** `dedc5fe` v2.10.4
|
||||
|
||||
> **Category:** Missing Index | **File:** `src/db/schema/sellerOffer.ts:220-253`
|
||||
|
||||
`findSellerOffersBySeller` (line 1601) filters `WHERE seller_id = $id AND status = $status ORDER BY created_at DESC`. The schema has separate single-column indexes on `seller_id` and `status` but no composite `(seller_id, status, created_at DESC)`, requiring a re-check pass after using either index.
|
||||
|
||||
**Fix:** Add `index('idx_seller_offers_seller_status').on(t.sellerId, t.status)` in `sellerOffer.ts`.
|
||||
**Fix:** `sellerOffer.ts` declares `idx_so_seller_status` on `(seller_id, status)`, and migration `0021_missing_indexes.sql` includes `CREATE INDEX IF NOT EXISTS idx_so_seller_status ON seller_offers (seller_id, status)`. `dedc5fe` adds audit regression coverage to keep the schema and migration declarations present.
|
||||
|
||||
---
|
||||
|
||||
@@ -1087,13 +1094,13 @@ After finding or creating the payment, the service calls `paymentRepo.updateById
|
||||
|
||||
---
|
||||
|
||||
### 4. fundsLedgerEntry.paymentId has no covering index for entryType aggregate queries
|
||||
### 4. fundsLedgerEntry.paymentId has no covering index for entryType aggregate queries | **FIXED** `dedc5fe` v2.10.4
|
||||
|
||||
> **Category:** Missing Index | **File:** `src/db/schema/fundsLedgerEntry.ts:69-71, 127-133`
|
||||
|
||||
The schema defines `idx_funds_ledger_payment_created` on `(paymentId, createdAt)` but `getLedgerBalanceByPaymentId` aggregates entries grouped by `entryType`. Without an index on `(paymentId, entryType)`, the aggregate must scan all entries for a payment then group in memory. (Covered by the composite index finding above; listed separately for implementation clarity.)
|
||||
|
||||
**Fix:** Add `index('idx_funds_ledger_payment_entry_type').on(t.paymentId, t.entryType)` to the schema.
|
||||
**Fix:** `fundsLedgerEntry.ts` declares `idx_funds_ledger_payment_entry_type` on `(payment_id, entry_type)`, and migration `0021_missing_indexes.sql` includes `CREATE INDEX IF NOT EXISTS idx_funds_ledger_payment_entry_type ON funds_ledger_entries (payment_id, entry_type)`. `dedc5fe` adds audit regression coverage to keep both declarations present.
|
||||
|
||||
---
|
||||
|
||||
|
||||
Reference in New Issue
Block a user