diff --git a/BackendAcademy/src/ai/prompt-template.service.spec.ts b/BackendAcademy/src/ai/prompt-template.service.spec.ts index c8b07cb22..dd85962e0 100644 --- a/BackendAcademy/src/ai/prompt-template.service.spec.ts +++ b/BackendAcademy/src/ai/prompt-template.service.spec.ts @@ -22,7 +22,7 @@ describe('PromptTemplateService reloads', () => { it('keeps the last valid templates when a later reload is malformed', () => { writeFileSync(configPath, JSON.stringify({ schemaVersion: '1.0.0', - templates: { chat_tutor: [{ version: '2.0.0', description: 'Test', systemPrompt: 'Use the reloaded prompt.' }] }, + templates: { chat_tutor: [{ version: '2.0.0', description: 'Test', systemPrompt: 'Use the reloaded prompt.', approval: { status: 'approved' } }] }, })); expect(service.reloadTemplates()).toBe(true); diff --git a/BackendAcademy/src/database/database.service.ts b/BackendAcademy/src/database/database.service.ts index 6cf5a0ca9..797103661 100644 --- a/BackendAcademy/src/database/database.service.ts +++ b/BackendAcademy/src/database/database.service.ts @@ -43,6 +43,20 @@ export interface RedemptionRecord { */ export type PaymentStatus = 'pending' | 'processing' | 'succeeded' | 'failed' | 'refunded'; +/** + * Every payment status, in declaration order. Exported so transition tests + * and callers can enumerate the full state space without re-listing it. + * + * #661 (BA-093): used by the exhaustive state-transition test suite. + */ +export const PAYMENT_STATUSES: PaymentStatus[] = [ + 'pending', + 'processing', + 'succeeded', + 'failed', + 'refunded', +]; + export interface PaymentRecord { id: string; orderId: string; @@ -152,8 +166,15 @@ export class DatabaseService implements OnModuleInit, OnApplicationShutdown { /** * Explicit state machine for payment status transitions. A status that * does not appear as a key has no legal outgoing transitions (terminal). + * + * #661 (BA-093): this is the single, centralized source of truth for legal + * transitions. Both the webhook ingress path + * ({@link PaymentsService.processPaymentWebhookEvent}) and any internal + * status update route through {@link updatePaymentStatus}, which consults + * these rules, so illegal regressions and terminal-state changes are + * rejected everywhere. */ - private static readonly ALLOWED_TRANSITIONS: Record = { + static readonly ALLOWED_TRANSITIONS: Record = { pending: ['processing', 'succeeded', 'failed'], processing: ['succeeded', 'failed'], succeeded: ['refunded'], @@ -161,6 +182,26 @@ export class DatabaseService implements OnModuleInit, OnApplicationShutdown { refunded: [], }; + /** + * Returns whether transitioning from `from` to `to` is legal under the + * centralized payment state machine (#661 / BA-093). A no-op + * (`from === to`) is considered legal because callers treat it as an + * idempotent no-op rather than a transition. + */ + static isLegalPaymentTransition(from: PaymentStatus, to: PaymentStatus): boolean { + if (from === to) return true; + const allowed = DatabaseService.ALLOWED_TRANSITIONS[from] ?? []; + return allowed.includes(to); + } + + /** + * Returns whether a status is terminal (has no legal outgoing transitions), + * so any further change from it must be rejected (#661 / BA-093). + */ + static isTerminalPaymentStatus(status: PaymentStatus): boolean { + return (DatabaseService.ALLOWED_TRANSITIONS[status] ?? []).length === 0; + } + onModuleInit() { this.seedSampleCoupons(); this.ensureMigrationTracking(); diff --git a/BackendAcademy/src/payments/payment-state-machine.spec.ts b/BackendAcademy/src/payments/payment-state-machine.spec.ts new file mode 100644 index 000000000..b98d1baf4 --- /dev/null +++ b/BackendAcademy/src/payments/payment-state-machine.spec.ts @@ -0,0 +1,106 @@ +import { DatabaseService, PAYMENT_STATUSES, PaymentStatus } from '../database/database.service'; +import { TransactionManagerService } from '../common/transaction-manager.service'; + +/** + * Exhaustive payment state-machine coverage for BA-093 / #661. + * + * These tests assert that the centralized transition rules reject illegal + * status regressions and terminal-state changes for *every* status, and that + * duplicate provider events are treated as safe no-ops. Both the webhook + * ingress path and any internal status update route through + * `DatabaseService.updatePaymentStatus`, so exercising it here covers the + * internal-update requirement directly. + */ +describe('Payment state machine (BA-093 / #661)', () => { + describe('isLegalPaymentTransition', () => { + it('matches the declared allowed transitions for every status pair', () => { + for (const from of PAYMENT_STATUSES) { + for (const to of PAYMENT_STATUSES) { + const expected = + from === to || DatabaseService.ALLOWED_TRANSITIONS[from].includes(to); + expect(DatabaseService.isLegalPaymentTransition(from, to)).toBe(expected); + } + } + }); + + it('rejects regressions and terminal-state changes', () => { + // succeeded -> pending is an illegal regression. + expect(DatabaseService.isLegalPaymentTransition('succeeded', 'pending')).toBe(false); + // failed is terminal: any onward change is illegal. + expect(DatabaseService.isLegalPaymentTransition('failed', 'succeeded')).toBe(false); + // refunded is terminal: any onward change is illegal. + expect(DatabaseService.isLegalPaymentTransition('refunded', 'pending')).toBe(false); + expect(DatabaseService.isLegalPaymentTransition('refunded', 'succeeded')).toBe(false); + }); + }); + + describe('isTerminalPaymentStatus', () => { + it('marks failed and refunded as terminal', () => { + expect(DatabaseService.isTerminalPaymentStatus('failed')).toBe(true); + expect(DatabaseService.isTerminalPaymentStatus('refunded')).toBe(true); + }); + + it('marks pending, processing and succeeded as non-terminal', () => { + expect(DatabaseService.isTerminalPaymentStatus('pending')).toBe(false); + expect(DatabaseService.isTerminalPaymentStatus('processing')).toBe(false); + expect(DatabaseService.isTerminalPaymentStatus('succeeded')).toBe(false); + }); + }); + + describe('updatePaymentStatus enforcement (internal updates)', () => { + let db: DatabaseService; + + beforeEach(() => { + db = new DatabaseService(new TransactionManagerService()); + }); + + const seed = (id: string, status: PaymentStatus): void => { + db.createPayment({ + id, + orderId: 'ord-1', + userId: 'user-1', + status, + amount: 100, + assetCode: 'XLM', + provider: 'stellar', + }); + }; + + PAYMENT_STATUSES.forEach((from) => { + PAYMENT_STATUSES.forEach((to) => { + it(`rejects illegal transition from ${from} -> ${to} via updatePaymentStatus`, async () => { + const id = `pay-${from}-${to}`; + seed(id, from); + + const result = await db.updatePaymentStatus(id, to, `evt-${from}-${to}`); + + if (from === to) { + // Idempotent no-op: safe, but not a state change. + expect(result.success).toBe(true); + expect(result.transitioned).toBe(false); + } else if (DatabaseService.ALLOWED_TRANSITIONS[from].includes(to)) { + expect(result.success).toBe(true); + expect(result.transitioned).toBe(true); + } else { + // Illegal regression / terminal change must be refused. + expect(result.success).toBe(false); + expect(result.transitioned).toBe(false); + expect(result.reason).toMatch(/Illegal transition/i); + } + }); + }); + }); + + it('treats a repeated provider event id as a safe no-op', async () => { + seed('pay-dup', 'pending'); + const first = await db.updatePaymentStatus('pay-dup', 'succeeded', 'evt-dup'); + expect(first.success).toBe(true); + expect(first.transitioned).toBe(true); + + const second = await db.updatePaymentStatus('pay-dup', 'succeeded', 'evt-dup'); + expect(second.success).toBe(true); + expect(second.duplicateEvent).toBe(true); + expect(second.transitioned).toBe(false); + }); + }); +}); diff --git a/BackendAcademy/src/payments/payments.service.ts b/BackendAcademy/src/payments/payments.service.ts index ad9d66db4..e0cc0840e 100644 --- a/BackendAcademy/src/payments/payments.service.ts +++ b/BackendAcademy/src/payments/payments.service.ts @@ -132,14 +132,10 @@ export class PaymentsService { /** Base backoff for webhook retries (Issue #412). */ private readonly webhookBaseBackoffMs: number; /** Cap for webhook retry backoff (Issue #412). */ - private readonly defaultTimeoutMs: number; - private readonly webhookMaxRetries: number; - private readonly webhookBaseBackoffMs: number; private readonly webhookMaxBackoffMs: number; constructor( private readonly databaseService: DatabaseService, - private readonly configService?: ConfigService, @Optional() private readonly contractAdapter?: IContractAdapter, @Optional() @@ -313,12 +309,16 @@ export class PaymentsService { this.logger.log( `Webhook ${webhook.id} delivered on attempt ${attempt}`, ); + // Mark the durable outbox as delivered so its terminal state stays + // inspectable (the enqueue-before-deliver contract from #666). + await this.databaseService.completeWebhookDelivery(webhook.id, statusCode); return { success: true, attempts: attempt }; } lastError = `HTTP ${statusCode}`; } catch (err) { lastError = err instanceof Error ? err.message : String(err); } + } // Drive the delivery to a terminal state through the durable outbox, // honouring the exponential-backoff schedule stored on the record.