mirror of
https://github.com/xtr-dev/payload-billing.git
synced 2025-12-10 02:43:24 +00:00
Simplify payment updates by removing race condition logic
- 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 <noreply@anthropic.com>
This commit is contained in:
@@ -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<Payment>[],
|
||||
|
||||
@@ -48,7 +48,6 @@ export interface Payment {
|
||||
| boolean
|
||||
| null;
|
||||
refunds?: (number | Refund)[] | null;
|
||||
version: number;
|
||||
updatedAt: string;
|
||||
createdAt: string;
|
||||
}
|
||||
|
||||
@@ -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<boolean> {
|
||||
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)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user