diff --git a/docs/docs/api/hybrid-payment-handler.md b/docs/docs/api/hybrid-payment-handler.md index 8cd583d..270dc10 100644 --- a/docs/docs/api/hybrid-payment-handler.md +++ b/docs/docs/api/hybrid-payment-handler.md @@ -55,6 +55,8 @@ On iOS, the merchant ID is resolved from the app entitlements by default. Use `a Build the request yourself or use [createPaymentRequest](/docs/api/utils#createpaymentrequestoptions) for a single line item with defaults. +Only one payment can be in flight at a time. A call made while another payment is still in progress, including one started by `usePaymentCheckout`, resolves immediately with `{ success: false, error: 'A payment is already in progress' }` and does not affect the payment in flight. This applies on iOS and Android. + --- ### canMakePayments(usingNetworks) diff --git a/docs/docs/api/use-payment-checkout.md b/docs/docs/api/use-payment-checkout.md index 647377c..58a035a 100644 --- a/docs/docs/api/use-payment-checkout.md +++ b/docs/docs/api/use-payment-checkout.md @@ -68,7 +68,10 @@ interface UsePaymentCheckoutReturn { - Returns `null` and sets `error` when the cart is empty. - Returns `PaymentResult` (including `{ success: false, error }`) when the native flow completes with a failure result. - Returns `null` if an exception is thrown while starting/processing the payment. -- **reset()** — Clears `isProcessing`, `result`, and `error` (does not clear cart). + - Returns `{ success: false, error: 'A payment is already in progress' }` without opening a second sheet if a payment is still in flight: + - If the payment in flight was started by this hook, the hook state is not changed. + - If it was started elsewhere (`HybridPaymentHandler` or another `usePaymentCheckout`), the native handler rejects the call and the hook records it like any failed payment, in `result` and `error`. +- **reset()** — Clears `result` and `error` (does not clear cart). It does not clear `isProcessing` while a payment is in flight; `isProcessing` becomes `false` when that payment settles. ## Example diff --git a/package/android/src/main/java/com/margelo/nitro/pay/Constants.kt b/package/android/src/main/java/com/margelo/nitro/pay/Constants.kt index c81db8c..de01fd7 100644 --- a/package/android/src/main/java/com/margelo/nitro/pay/Constants.kt +++ b/package/android/src/main/java/com/margelo/nitro/pay/Constants.kt @@ -40,6 +40,9 @@ object PaymentConstants { // Supported networks (lowercase for comparison) val SUPPORTED_NETWORKS = listOf("visa", "mastercard", "amex", "discover") + // Errors (keep in sync with iOS `ErrorMessage`) + const val ERROR_PAYMENT_IN_PROGRESS = "A payment is already in progress" + // Logging tags const val TAG_PAYMENT_HANDLER = "HybridPaymentHandler" const val TAG_GOOGLE_PAY_BUTTON = "HybridGooglePayButton" diff --git a/package/android/src/main/java/com/margelo/nitro/pay/HybridPaymentHandler.kt b/package/android/src/main/java/com/margelo/nitro/pay/HybridPaymentHandler.kt index 6f5c475..4de4e9a 100644 --- a/package/android/src/main/java/com/margelo/nitro/pay/HybridPaymentHandler.kt +++ b/package/android/src/main/java/com/margelo/nitro/pay/HybridPaymentHandler.kt @@ -27,7 +27,7 @@ class HybridPaymentHandler : HybridPaymentHandlerSpec(), ActivityEventListener { ?: throw IllegalStateException("Application context is not ReactApplicationContext") } - private var paymentPromise: Promise? = null + private val pendingPayment = PendingPayment() private var currentEnvironment: Int = WalletConstants.ENVIRONMENT_TEST init { @@ -71,7 +71,14 @@ class HybridPaymentHandler : HybridPaymentHandlerSpec(), ActivityEventListener { override fun startPayment(request: PaymentRequest): Promise { val promise = Promise() - paymentPromise = promise + + // Reject overlaps without touching the in-flight payment, so its + // promise still settles and no second Google Pay sheet is requested. + if (!pendingPayment.tryBegin { result -> promise.resolve(result) }) { + Log.w(PaymentConstants.TAG_PAYMENT_HANDLER, PaymentConstants.ERROR_PAYMENT_IN_PROGRESS) + promise.resolve(PaymentMapper.createErrorResult(PaymentConstants.ERROR_PAYMENT_IN_PROGRESS)) + return promise + } try { val environment = determineEnvironment(request.googlePayEnvironment) @@ -193,14 +200,12 @@ class HybridPaymentHandler : HybridPaymentHandlerSpec(), ActivityEventListener { private fun handlePaymentSuccess(paymentData: PaymentData) { Log.d(PaymentConstants.TAG_PAYMENT_HANDLER, "Payment successful") val result = PaymentMapper.mapPaymentDataToResult(paymentData) - paymentPromise?.resolve(result) - paymentPromise = null + pendingPayment.settle(result) } private fun handlePaymentError(errorMessage: String) { val result = PaymentMapper.createErrorResult(errorMessage) - paymentPromise?.resolve(result) - paymentPromise = null + pendingPayment.settle(result) } private fun logPaymentRequest(request: org.json.JSONObject, environment: Int) { diff --git a/package/android/src/main/java/com/margelo/nitro/pay/PendingPayment.kt b/package/android/src/main/java/com/margelo/nitro/pay/PendingPayment.kt new file mode 100644 index 0000000..adfca9b --- /dev/null +++ b/package/android/src/main/java/com/margelo/nitro/pay/PendingPayment.kt @@ -0,0 +1,31 @@ +package com.margelo.nitro.pay + +import java.util.concurrent.atomic.AtomicReference + +/** + * Holds the settle callback of the single in-flight payment. + * + * `startPayment` runs on the JS thread while `onActivityResult` runs on the + * main thread, so claiming and settling go through one atomic slot. + */ +internal class PendingPayment { + private val settleCallback = AtomicReference<((T) -> Unit)?>(null) + + /** + * Claims the slot for a new payment. + * Returns `false` without touching the in-flight payment if one exists. + */ + fun tryBegin(settle: (T) -> Unit): Boolean { + return settleCallback.compareAndSet(null, settle) + } + + /** + * Settles the in-flight payment once and frees the slot. + * Returns `false` if there was nothing to settle. + */ + fun settle(result: T): Boolean { + val settle = settleCallback.getAndSet(null) ?: return false + settle(result) + return true + } +} diff --git a/package/android/src/test/java/com/margelo/nitro/pay/PendingPaymentTest.kt b/package/android/src/test/java/com/margelo/nitro/pay/PendingPaymentTest.kt new file mode 100644 index 0000000..5ccb022 --- /dev/null +++ b/package/android/src/test/java/com/margelo/nitro/pay/PendingPaymentTest.kt @@ -0,0 +1,88 @@ +package com.margelo.nitro.pay + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import java.util.concurrent.CountDownLatch +import java.util.concurrent.atomic.AtomicInteger + +class PendingPaymentTest { + + @Test + fun settlesTheClaimedPayment() { + val pending = PendingPayment() + val settled = mutableListOf() + + assertTrue(pending.tryBegin { settled.add(it) }) + assertTrue(pending.settle("done")) + + assertEquals(listOf("done"), settled) + } + + @Test + fun rejectsOverlapWithoutReplacingTheInFlightPayment() { + val pending = PendingPayment() + val first = mutableListOf() + val second = mutableListOf() + + assertTrue(pending.tryBegin { first.add(it) }) + assertFalse(pending.tryBegin { second.add(it) }) + pending.settle("done") + + assertEquals(listOf("done"), first) + assertEquals(emptyList(), second) + } + + @Test + fun settlesOnlyOnce() { + val pending = PendingPayment() + val settled = mutableListOf() + + pending.tryBegin { settled.add(it) } + + assertTrue(pending.settle("first")) + assertFalse(pending.settle("second")) + assertEquals(listOf("first"), settled) + } + + @Test + fun settleWithoutPaymentIsNoOp() { + val pending = PendingPayment() + + assertFalse(pending.settle("orphan")) + } + + @Test + fun acceptsNewPaymentAfterSettle() { + val pending = PendingPayment() + val settled = mutableListOf() + + pending.tryBegin { settled.add("a:$it") } + pending.settle("1") + + assertTrue(pending.tryBegin { settled.add("b:$it") }) + pending.settle("2") + + assertEquals(listOf("a:1", "b:2"), settled) + } + + @Test + fun concurrentBeginsClaimTheSlotOnce() { + val pending = PendingPayment() + val threads = 16 + val gate = CountDownLatch(1) + val claims = AtomicInteger(0) + + val workers = (0 until threads).map { + Thread { + gate.await() + if (pending.tryBegin { }) claims.incrementAndGet() + }.apply { start() } + } + gate.countDown() + workers.forEach { it.join() } + + assertEquals(1, claims.get()) + } +} diff --git a/package/src/__tests__/paymentHandler.test.ts b/package/src/__tests__/paymentHandler.test.ts new file mode 100644 index 0000000..15ec6ef --- /dev/null +++ b/package/src/__tests__/paymentHandler.test.ts @@ -0,0 +1,110 @@ +import { act, renderHook } from '@testing-library/react-native' + +const IN_PROGRESS_RESULT = { + success: false, + error: 'A payment is already in progress', +} + +const mockInstances: object[] = [] +const mockPresentations: Array<(result: any) => void> = [] + +jest.mock('react-native-nitro-modules', () => ({ + NitroModules: { + createHybridObject: () => { + // Mirrors the per-instance in-progress guard of the native handlers. + let inFlight = false + const instance = { + payServiceStatus: () => ({ + canMakePayments: true, + canSetupCards: true, + }), + startPayment: () => { + if (inFlight) { + return Promise.resolve({ + success: false, + error: 'A payment is already in progress', + }) + } + inFlight = true + return new Promise((resolve) => { + mockPresentations.push((result) => { + inFlight = false + resolve(result) + }) + }) + }, + } + mockInstances.push(instance) + return instance + }, + }, + getHostComponent: () => 'HostComponent', +})) + +// Required after the mock state above exists: importing the package creates +// the handler immediately. +const { HybridPaymentHandler, usePaymentCheckout, createPaymentRequest } = + require('../index') as typeof import('../index') + +const settlePresentations = () => + mockPresentations.splice(0).forEach((settle) => settle({ success: true })) + +describe('shared PaymentHandler instance', () => { + const directRequest = createPaymentRequest({ amount: 10, label: 'Direct' }) + + const renderCheckoutWithItem = () => { + const rendered = renderHook(() => usePaymentCheckout({})) + act(() => { + rendered.result.current.addItem('Order', 5) + }) + return rendered + } + + afterEach(settlePresentations) + + it('uses one native handler for the hook and HybridPaymentHandler', () => { + renderHook(() => usePaymentCheckout({})) + + expect(mockInstances).toHaveLength(1) + expect(mockInstances[0]).toBe(HybridPaymentHandler) + }) + + it('rejects a direct call while the hook payment is in flight', async () => { + const { result } = renderCheckoutWithItem() + + let hookPayment!: Promise + let directResult: any + await act(async () => { + hookPayment = result.current.startPayment() + directResult = await HybridPaymentHandler.startPayment(directRequest) + }) + + expect(mockPresentations).toHaveLength(1) + expect(directResult).toEqual(IN_PROGRESS_RESULT) + + await act(async () => { + settlePresentations() + await hookPayment + }) + + await expect(hookPayment).resolves.toEqual({ success: true }) + }) + + it('records the native in-progress result when a direct payment is in flight', async () => { + const { result } = renderCheckoutWithItem() + + const directPayment = HybridPaymentHandler.startPayment(directRequest) + let hookResult: any + await act(async () => { + hookResult = await result.current.startPayment() + }) + + expect(mockPresentations).toHaveLength(1) + expect(hookResult).toEqual(IN_PROGRESS_RESULT) + expect(result.current.result).toEqual(IN_PROGRESS_RESULT) + expect(result.current.error?.message).toBe(IN_PROGRESS_RESULT.error) + + settlePresentations() + await expect(directPayment).resolves.toEqual({ success: true }) + }) +}) diff --git a/package/src/hooks/__tests__/usePaymentCheckout.integration.test.ts b/package/src/hooks/__tests__/usePaymentCheckout.integration.test.ts index aae2439..be72e0d 100644 --- a/package/src/hooks/__tests__/usePaymentCheckout.integration.test.ts +++ b/package/src/hooks/__tests__/usePaymentCheckout.integration.test.ts @@ -14,6 +14,19 @@ jest.mock('react-native-nitro-modules', () => ({ import { usePaymentCheckout } from '../usePaymentCheckout' +const IN_PROGRESS_RESULT = { + success: false, + error: 'A payment is already in progress', +} + +function createDeferred() { + let resolve!: (value: T) => void + const promise = new Promise((res) => { + resolve = res + }) + return { promise, resolve } +} + describe('usePaymentCheckout integration', () => { beforeEach(() => { mockPayServiceStatus.mockReturnValue({ @@ -245,4 +258,95 @@ describe('usePaymentCheckout integration', () => { expect(result.current.result).toBeNull() expect(result.current.isProcessing).toBe(false) }) + + it('starts one native payment when startPayment is called twice synchronously', async () => { + const native = createDeferred() + mockStartPayment.mockReturnValueOnce(native.promise) + const { result } = renderCheckout() + + act(() => { + result.current.addItem('Order', 5) + }) + + let first!: Promise + let second: any + await act(async () => { + first = result.current.startPayment() + second = await result.current.startPayment() + }) + + expect(mockStartPayment).toHaveBeenCalledTimes(1) + expect(second).toEqual(IN_PROGRESS_RESULT) + expect(result.current.isProcessing).toBe(true) + expect(result.current.error).toBeNull() + expect(result.current.result).toBeNull() + + const nativeSuccess = { success: true, transactionId: 'tx_1' } + await act(async () => { + native.resolve(nativeSuccess) + await first + }) + + await expect(first).resolves.toEqual(nativeSuccess) + expect(result.current.result).toEqual(nativeSuccess) + expect(result.current.isProcessing).toBe(false) + }) + + it('keeps the in-flight guard when reset() runs during a payment', async () => { + const native = createDeferred() + mockStartPayment.mockReturnValueOnce(native.promise) + const { result } = renderCheckout() + + act(() => { + result.current.addItem('Order', 5) + }) + + let first!: Promise + act(() => { + first = result.current.startPayment() + }) + act(() => { + result.current.reset() + }) + + expect(result.current.isProcessing).toBe(true) + + let second: any + await act(async () => { + second = await result.current.startPayment() + }) + + expect(second).toEqual(IN_PROGRESS_RESULT) + expect(mockStartPayment).toHaveBeenCalledTimes(1) + + await act(async () => { + native.resolve({ success: true }) + await first + }) + + expect(result.current.isProcessing).toBe(false) + }) + + it('accepts a new payment once the previous one settles', async () => { + mockStartPayment + .mockRejectedValueOnce(new Error('Native failure')) + .mockResolvedValueOnce({ success: true }) + const { result } = renderCheckout() + + act(() => { + result.current.addItem('Order', 5) + }) + + await act(async () => { + await result.current.startPayment() + }) + + let paymentResult: any = null + await act(async () => { + paymentResult = await result.current.startPayment() + }) + + expect(mockStartPayment).toHaveBeenCalledTimes(2) + expect(paymentResult).toEqual({ success: true }) + }) }) diff --git a/package/src/hooks/usePaymentCheckout.ts b/package/src/hooks/usePaymentCheckout.ts index 8e199f4..f3d3995 100644 --- a/package/src/hooks/usePaymentCheckout.ts +++ b/package/src/hooks/usePaymentCheckout.ts @@ -1,9 +1,5 @@ -import { useState, useCallback, useMemo, useEffect } from 'react' -import { NitroModules } from 'react-native-nitro-modules' -import type { PaymentHandler } from '../specs/PaymentHandler.nitro' - -const HybridPaymentHandler = - NitroModules.createHybridObject('PaymentHandler') +import { useState, useCallback, useMemo, useEffect, useRef } from 'react' +import { getPaymentHandler, PAYMENT_IN_PROGRESS_ERROR } from '../paymentHandler' import type { PaymentRequest, PaymentResult, @@ -183,9 +179,12 @@ export function usePaymentCheckout( const [result, setResult] = useState(null) const [error, setError] = useState(null) + // Set synchronously, so two calls in the same frame still see each other. + const isPaymentInFlight = useRef(false) + useEffect(() => { try { - const paymentStatus = HybridPaymentHandler.payServiceStatus() + const paymentStatus = getPaymentHandler().payServiceStatus() setStatus(paymentStatus) } catch (err) { console.error('Failed to check payment status:', err) @@ -270,19 +269,24 @@ export function usePaymentCheckout( }, []) const startPayment = useCallback(async (): Promise => { + if (isPaymentInFlight.current) { + return { success: false, error: PAYMENT_IN_PROGRESS_ERROR } + } + if (items.length === 0) { const emptyCartError = new Error('Cart is empty') setError(emptyCartError) return null } + isPaymentInFlight.current = true setIsProcessing(true) setResult(null) setError(null) try { const paymentResult = - await HybridPaymentHandler.startPayment(paymentRequest) + await getPaymentHandler().startPayment(paymentRequest) setResult(paymentResult) @@ -297,12 +301,12 @@ export function usePaymentCheckout( setError(paymentError) return null } finally { + isPaymentInFlight.current = false setIsProcessing(false) } }, [items, paymentRequest]) const reset = useCallback(() => { - setIsProcessing(false) setResult(null) setError(null) }, []) diff --git a/package/src/index.ts b/package/src/index.ts index 2f9438a..36c91b9 100644 --- a/package/src/index.ts +++ b/package/src/index.ts @@ -1,4 +1,4 @@ -import { NitroModules, getHostComponent } from 'react-native-nitro-modules' +import { getHostComponent } from 'react-native-nitro-modules' import type { ApplePayButtonProps, ApplePayButtonMethods, @@ -7,10 +7,9 @@ import type { GooglePayButtonProps, GooglePayButtonMethods, } from './specs/GooglePayButton.nitro' -import type { PaymentHandler } from './specs/PaymentHandler.nitro' +import { getPaymentHandler } from './paymentHandler' -export const HybridPaymentHandler = - NitroModules.createHybridObject('PaymentHandler') +export const HybridPaymentHandler = getPaymentHandler() export type { PaymentHandler } from './specs/PaymentHandler.nitro' diff --git a/package/src/paymentHandler.ts b/package/src/paymentHandler.ts new file mode 100644 index 0000000..bbd8df7 --- /dev/null +++ b/package/src/paymentHandler.ts @@ -0,0 +1,24 @@ +import { NitroModules } from 'react-native-nitro-modules' +import type { PaymentHandler } from './specs/PaymentHandler.nitro' + +/** + * Error returned when `startPayment` is called while another payment is + * still in flight. Matches the message used by the native handlers. + */ +export const PAYMENT_IN_PROGRESS_ERROR = 'A payment is already in progress' + +let paymentHandler: PaymentHandler | undefined + +/** + * Returns the package-wide `PaymentHandler` instance. + * + * The native in-progress guard lives on the instance, so the hook and the + * exported `HybridPaymentHandler` must share one. + */ +export function getPaymentHandler(): PaymentHandler { + if (paymentHandler === undefined) { + paymentHandler = + NitroModules.createHybridObject('PaymentHandler') + } + return paymentHandler +}