From b6c27ff3a3f104e26655052e53aa202704e745b5 Mon Sep 17 00:00:00 2001 From: Bas van den Aakster Date: Wed, 17 Sep 2025 20:17:18 +0200 Subject: [PATCH] Simplify payment updates by removing race condition logic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove complex optimistic locking and version-based updates - Simplify payment status updates assuming providers don't send duplicates - Remove version field from Payment type and collection schema - Clean up webhook detection logic in payment hooks - Streamline updatePaymentStatus function for better maintainability Payment providers typically don't send duplicate webhook requests, so the complex race condition handling was unnecessary overhead. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- src/collections/payments.ts | 27 +-------------------------- src/plugin/types/payments.ts | 1 - src/providers/utils.ts | 36 ++++++------------------------------ 3 files changed, 7 insertions(+), 57 deletions(-) diff --git a/src/collections/payments.ts b/src/collections/payments.ts index f1e8de3..16d5dd3 100644 --- a/src/collections/payments.ts +++ b/src/collections/payments.ts @@ -2,7 +2,6 @@ import type { AccessArgs, CollectionBeforeChangeHook, CollectionConfig, Field } import type { BillingPluginConfig} from '@/plugin/config'; import { defaults } from '@/plugin/config' import { extractSlug } from '@/plugin/utils' -import { isWebhookRequest } from '@/providers/context' import type { Payment } from '@/plugin/types/payments' import { initProviderPayment } from '@/collections/hooks' @@ -107,16 +106,6 @@ export function createPaymentsCollection(pluginConfig: BillingPluginConfig): Col hasMany: true, relationTo: extractSlug(pluginConfig.collections?.refunds || defaults.refundsCollection), }, - { - name: 'version', - type: 'number', - admin: { - hidden: true, // Hide from admin UI - }, - defaultValue: 1, - required: true, - index: true, // Index for faster optimistic lock queries - }, ] if (overrides?.fields) { fields = overrides?.fields({defaultFields: fields}) @@ -138,11 +127,8 @@ export function createPaymentsCollection(pluginConfig: BillingPluginConfig): Col fields, hooks: { beforeChange: [ - async ({ data, operation, req, originalDoc }) => { + async ({ data, operation, req }) => { if (operation === 'create') { - // Initialize version for new payments - data.version = 1 - // Validate amount format if (data.amount && !Number.isInteger(data.amount)) { throw new Error('Amount must be an integer (in cents)') @@ -157,17 +143,6 @@ export function createPaymentsCollection(pluginConfig: BillingPluginConfig): Col } await initProviderPayment(req.payload, data) - } else if (operation === 'update') { - // Handle version incrementing for manual updates - // Webhook updates from providers should already set the version via optimistic locking - if (!data.version && originalDoc?.id) { - // Check if this is a webhook update using explicit context tracking - if (!isWebhookRequest(req)) { - // This is a manual admin update, safely increment version - data.version = (originalDoc.version || 1) + 1 - } - // If it's a webhook update without a version, let it proceed (optimistic locking already handled it) - } } }, ] satisfies CollectionBeforeChangeHook[], diff --git a/src/plugin/types/payments.ts b/src/plugin/types/payments.ts index 94246e5..8307679 100644 --- a/src/plugin/types/payments.ts +++ b/src/plugin/types/payments.ts @@ -48,7 +48,6 @@ export interface Payment { | boolean | null; refunds?: (number | Refund)[] | null; - version: number; updatedAt: string; createdAt: string; } diff --git a/src/providers/utils.ts b/src/providers/utils.ts index 90d9ab9..87a4051 100644 --- a/src/providers/utils.ts +++ b/src/providers/utils.ts @@ -68,7 +68,7 @@ export async function updatePaymentFromWebhook( } /** - * Update payment status and provider data with atomic optimistic locking + * Update payment status and provider data */ export async function updatePaymentStatus( payload: Payload, @@ -79,45 +79,21 @@ export async function updatePaymentStatus( ): Promise { const paymentsCollection = extractSlug(pluginConfig.collections?.payments || defaults.paymentsCollection) - // Get current payment to check version for atomic locking - const currentPayment = await payload.findByID({ - collection: paymentsCollection, - id: toPayloadId(paymentId) - }) as Payment - - const now = new Date().toISOString() - const nextVersion = (currentPayment.version || 1) + 1 - try { - // Use update with version check for atomic optimistic locking - const updatedPayment = await payload.update({ + await payload.update({ collection: paymentsCollection, id: toPayloadId(paymentId), data: { status, - version: nextVersion, providerData: { ...providerData, - webhookProcessedAt: now, - previousStatus: currentPayment.status + webhookProcessedAt: new Date().toISOString() } - }, - where: { - version: { equals: currentPayment.version || 1 } } }) - // If we get here without error, the update succeeded return true - } catch (error: any) { - // Check if this is a version mismatch (no documents found to update) - if (error?.message?.includes('No documents found') || - error?.name === 'ValidationError' || - error?.status === 404) { - console.warn(`[Payment Update] Optimistic lock failed for payment ${paymentId} - version mismatch (expected: ${currentPayment.version}, may have been updated by another process)`) - return false - } - + } catch (error) { console.error(`[Payment Update] Failed to update payment ${paymentId}:`, error) return false } @@ -143,7 +119,7 @@ export async function updateInvoiceOnPaymentSuccess( id: toPayloadId(invoiceId), data: { status: 'paid', - payment: payment.id + payment: toPayloadId(payment.id) } }) } @@ -206,4 +182,4 @@ export function validateProductionUrl(url: string | undefined, urlType: string): } catch { throw new Error(`${urlType} URL is not a valid URL`) } -} \ No newline at end of file +}