Skip to content

Commit 20762d8

Browse files
annemirasolmalithsendiegocurbelo
authored
Add order locking to payment redirect processing (#4437)
* Add order locking to payment redirect processing * Remove extra changes from linter * Add comments * Add readme and changelog entries * Add unit test * Move unlock to finally block Co-authored-by: Malith Senaweera <[email protected]> * Add logging for when we bail because order is locked Co-authored-by: Diego Curbelo <[email protected]> * Fix spacing * Fix unit test --------- Co-authored-by: Malith Senaweera <[email protected]> Co-authored-by: Diego Curbelo <[email protected]>
1 parent f479373 commit 20762d8

File tree

4 files changed

+69
-3
lines changed

4 files changed

+69
-3
lines changed

changelog.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
* Fix - Add safety check when checking error object
5656
* Fix - Correctly handle countries without states when using the express payment methods
5757
* Update - Include extension data from block checkout when submitting an express checkout order
58+
* Fix - Add order locking when processing payment redirects, to mitigate cases of double status updates
5859

5960
= 9.5.3 - 2025-06-23 =
6061
* Fix - Reimplement mapping of Express Checkout state values to align with WooCommerce's expected state formats

includes/payment-methods/class-wc-stripe-upe-payment-gateway.php

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1616,13 +1616,21 @@ public function process_upe_redirect_payment( $order_id, $intent_id, $save_payme
16161616
return;
16171617
}
16181618

1619-
WC_Stripe_Logger::log( "Begin processing UPE redirect payment for order $order_id for the amount of {$order->get_total()}" );
1620-
16211619
try {
1620+
// First check if the order is already being processed by another request.
1621+
$locked = $this->lock_order_payment( $order );
1622+
if ( $locked ) {
1623+
WC_Stripe_Logger::log( "Skip processing UPE redirect payment for order $order_id for the amount of {$order->get_total()}, order payment is already being processed (locked)" );
1624+
return;
1625+
}
1626+
1627+
WC_Stripe_Logger::log( "Begin processing UPE redirect payment for order $order_id for the amount of {$order->get_total()}" );
1628+
16221629
$this->process_order_for_confirmed_intent( $order, $intent_id, $save_payment_method );
16231630
} catch ( Exception $e ) {
1624-
WC_Stripe_Logger::log( 'Error: ' . $e->getMessage() );
1631+
$this->unlock_order_payment( $order );
16251632

1633+
WC_Stripe_Logger::log( 'Error: ' . $e->getMessage() );
16261634
/* translators: localized exception message */
16271635
$order->update_status( OrderStatus::FAILED, sprintf( __( 'UPE payment failed: %s', 'woocommerce-gateway-stripe' ), $e->getMessage() ) );
16281636

@@ -1637,6 +1645,8 @@ public function process_upe_redirect_payment( $order_id, $intent_id, $save_payme
16371645
wp_safe_redirect( wp_sanitize_redirect( $redirect_url ) );
16381646

16391647
exit;
1648+
} finally {
1649+
$this->unlock_order_payment( $order );
16401650
}
16411651
}
16421652

readme.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,5 +166,6 @@ If you get stuck, you can ask for help in the [Plugin Forum](https://wordpress.o
166166
* Fix - Add safety check when checking error object
167167
* Fix - Correctly handle countries without states when using the express payment methods
168168
* Update - Include extension data from block checkout when submitting an express checkout order
169+
* Fix - Add order locking when processing payment redirects, to mitigate cases of double status updates
169170

170171
[See changelog for full details across versions](https://raw.githubusercontent.com/woocommerce/woocommerce-gateway-stripe/trunk/changelog.txt).

tests/phpunit/PaymentMethods/WC_Stripe_UPE_Payment_Gateway_Test.php

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,8 @@ public function set_up() {
203203
'has_pre_order',
204204
'is_subscriptions_enabled',
205205
'update_saved_payment_method',
206+
'lock_order_payment',
207+
'unlock_order_payment',
206208
]
207209
)
208210
->getMock();
@@ -237,6 +239,11 @@ public function set_up() {
237239
->will(
238240
$this->returnValue( 'cus_mock' )
239241
);
242+
$this->mock_gateway->expects( $this->any() )
243+
->method( 'unlock_order_payment' )
244+
->will(
245+
$this->returnValue( null )
246+
);
240247
}
241248

242249
public function tear_down() {
@@ -938,6 +945,11 @@ public function test_process_redirect_payment_returns_valid_response() {
938945
$payment_intent_mock['payment_method'] = $payment_method_mock;
939946
$payment_intent_mock['latest_charge'] = 'ch_mock';
940947

948+
$this->mock_gateway->expects( $this->any() )
949+
->method( 'lock_order_payment' )
950+
->will(
951+
$this->returnValue( false )
952+
);
941953
$this->mock_gateway->expects( $this->once() )
942954
->method( 'stripe_request' )
943955
->with( "payment_intents/$payment_intent_id?expand[]=payment_method" )
@@ -1051,6 +1063,48 @@ public function test_process_redirect_payment_only_runs_once() {
10511063
$this->assertEquals( OrderStatus::FAILED, $final_order->get_status() );
10521064
}
10531065

1066+
/**
1067+
* Test locking for process redirect payment.
1068+
*/
1069+
public function test_process_redirect_payment_locks_order() {
1070+
$payment_intent_id = 'pi_mock';
1071+
$payment_method_id = 'pm_mock';
1072+
$customer_id = 'cus_mock';
1073+
$order = WC_Helper_Order::create_order();
1074+
$order_id = $order->get_id();
1075+
1076+
list( $amount, $description, $metadata ) = $this->get_order_details( $order );
1077+
$order->set_payment_method( WC_Stripe_UPE_Payment_Gateway::ID );
1078+
$order->save();
1079+
1080+
$payment_method_mock = self::MOCK_CARD_PAYMENT_METHOD_TEMPLATE;
1081+
$payment_method_mock['id'] = $payment_method_id;
1082+
$payment_method_mock['customer'] = $customer_id;
1083+
$payment_method_mock['card']['exp_year'] = intval( gmdate( 'Y' ) ) + 1;
1084+
1085+
$payment_intent_mock = self::MOCK_CARD_PAYMENT_INTENT_TEMPLATE;
1086+
$payment_intent_mock['id'] = $payment_intent_id;
1087+
$payment_intent_mock['amount'] = $amount;
1088+
$payment_intent_mock['last_payment_error'] = [];
1089+
$payment_intent_mock['payment_method'] = $payment_method_mock;
1090+
$payment_intent_mock['latest_charge'] = 'ch_mock';
1091+
1092+
$this->mock_gateway->expects( $this->once() )
1093+
->method( 'lock_order_payment' )
1094+
->will(
1095+
$this->returnValue( true )
1096+
);
1097+
$this->mock_gateway->expects( $this->once() )
1098+
->method( 'unlock_order_payment' );
1099+
1100+
// Expect the process to bail early.
1101+
$this->mock_gateway->expects( $this->never() )
1102+
->method( 'stripe_request' )
1103+
->with( "payment_intents/$payment_intent_id?expand[]=payment_method" );
1104+
1105+
$this->mock_gateway->process_upe_redirect_payment( $order_id, $payment_intent_id, false );
1106+
}
1107+
10541108
/**
10551109
* Test checkout flow with setup intents.
10561110
*/

0 commit comments

Comments
 (0)