Refactor order locking to use order meta instead of transients (#3573)

* Use order meta instead of transients to lock order payments * Add unit tests * Update inline comments * edit inline comment * Add changelog entry * Fix issues with phpunit tests comparing versions of an order that didn't contain locking metadata * Update includes/abstracts/abstract-wc-stripe-payment-gateway.php Co-authored-by: Wesley Rosa <[email protected]> * fix small whitespace --------- Co-authored-by: Wesley Rosa <[email protected]>
woocommerce · Nov 7, 2024 · 61385aa · 61385aa
1 parent e799735
commit 61385aa
Show file tree

Hide file tree

Showing 5 changed files with 77 additions and 14 deletions.
diff --git a/changelog.txt b/changelog.txt
@@ -2,6 +2,7 @@
 
 = 8.8.2 - xxxx-xx-xx =
 * Fix - Prevent marking renewal orders as processing/completed multiple times due to handling the Stripe webhook in parallel.
+* Dev - Refactor lock_order_payment() to use order meta instead of transients.
 
 = 8.8.1 - 2024-10-28 =
 * Tweak - Disables APMs when using the legacy checkout experience due Stripe deprecation by October 29, 2024.

diff --git a/includes/abstracts/abstract-wc-stripe-payment-gateway.php b/includes/abstracts/abstract-wc-stripe-payment-gateway.php
@@ -1672,17 +1672,25 @@ private function get_intent( $intent_type, $intent_id ) {
 	 * @return bool            A flag that indicates whether the order is already locked.
 	 */
 	public function lock_order_payment( $order, $intent = null ) {
-		$order_id       = $order->get_id();
-		$transient_name = 'wc_stripe_processing_intent_' . $order_id;
-		$processing     = get_transient( $transient_name );
+		$order->read_meta_data( true );
 
-		// Block the process if the same intent is already being handled.
-		if ( '-1' === $processing || ( isset( $intent->id ) && $processing === $intent->id ) ) {
-			return true;
+		$existing_lock = $order->get_meta( '_stripe_lock_payment', true );
+
+		if ( $existing_lock ) {
+			$parts         = explode( '|', $existing_lock ); // Format is: "{expiry_timestamp}" or "{expiry_timestamp}|{pi_xxxx}" if an intent is passed.
+			$expiration    = (int) $parts[0];
+			$locked_intent = ! empty( $parts[1] ) ? $parts[1] : '';
+
+			// If the lock is still active, return true.
+			if ( time() <= $expiration && ( empty( $intent ) || empty( $locked_intent ) || ( $intent->id ?? '' ) === $locked_intent ) ) {
+				return true;
+			}
 		}
 
-		// Save the new intent as a transient, eventually overwriting another one.
-		set_transient( $transient_name, empty( $intent ) ? '-1' : $intent->id, 5 * MINUTE_IN_SECONDS );
+		$new_lock = ( time() + 5 * MINUTE_IN_SECONDS ) . ( isset( $intent->id ) ? '|' . $intent->id : '' );
+
+		$order->update_meta_data( '_stripe_lock_payment', $new_lock );
+		$order->save_meta_data();
 
 		return false;
 	}
@@ -1694,8 +1702,8 @@ public function lock_order_payment( $order, $intent = null ) {
 	 * @param WC_Order $order The order that is being unlocked.
 	 */
 	public function unlock_order_payment( $order ) {
-		$order_id = $order->get_id();
-		delete_transient( 'wc_stripe_processing_intent_' . $order_id );
+		$order->delete_meta_data( '_stripe_lock_payment' );
+		$order->save_meta_data();
 	}
 
 	/**

diff --git a/readme.txt b/readme.txt
@@ -112,5 +112,6 @@ If you get stuck, you can ask for help in the [Plugin Forum](https://wordpress.o
 
 = 8.8.2 - xxxx-xx-xx =
 * Fix - Prevent marking renewal orders as processing/completed multiple times due to handling the Stripe webhook in parallel.
+* Dev - Refactor lock_order_payment() to use order meta instead of transients.
 
 [See changelog for all versions](https://raw.githubusercontent.com/woocommerce/woocommerce-gateway-stripe/trunk/changelog.txt).
diff --git a/tests/phpunit/test-class-wc-stripe-upe-payment-gateway.php b/tests/phpunit/test-class-wc-stripe-upe-payment-gateway.php
@@ -1768,8 +1768,11 @@ public function test_subscription_renewal_is_successful() {
 
 		list( $amount, $description, $metadata ) = $this->get_order_details( $order );
 		$order->set_payment_method( WC_Stripe_UPE_Payment_Gateway::ID );
+		$order->update_meta_data( '_stripe_lock_payment', ( time() + MINUTE_IN_SECONDS ) ); // To assist with comparing expected order objects, set an existing lock.
 		$order->save();
 
+		$order = wc_get_order( $order_id );
+
 		$payment_method_mock                     = self::MOCK_CARD_PAYMENT_METHOD_TEMPLATE;
 		$payment_method_mock['id']               = $payment_method_id;
 		$payment_method_mock['customer']         = $customer_id;
@@ -1799,7 +1802,7 @@ public function test_subscription_renewal_is_successful() {
 		$this->mock_gateway->expects( $this->once() )
 			->method( 'create_and_confirm_intent_for_off_session' )
 			->with(
-				wc_get_order( $order_id ),
+				$order,
 				$prepared_source,
 				$amount
 			)
@@ -1820,7 +1823,7 @@ public function test_subscription_renewal_is_successful() {
 			->method( 'get_latest_charge_from_intent' )
 			->willReturn( $this->array_to_object( $charge ) );
 
-		$this->mock_gateway->process_subscription_payment( $amount, wc_get_order( $order_id ), false, false );
+		$this->mock_gateway->process_subscription_payment( $amount, $order, false, false );
 
 		$final_order = wc_get_order( $order_id );
 		$note        = wc_get_order_notes(
@@ -1859,8 +1862,11 @@ public function test_subscription_renewal_checks_payment_method_authorization()
 
 		list( $amount, $description, $metadata ) = $this->get_order_details( $order );
 		$order->set_payment_method( WC_Stripe_UPE_Payment_Gateway::ID );
+		$order->update_meta_data( '_stripe_lock_payment', ( time() + MINUTE_IN_SECONDS ) ); // To assist with comparing expected order objects, set an existing lock.
 		$order->save();
 
+		$order = wc_get_order( $order_id );
+
 		$payment_method_mock                     = self::MOCK_CARD_PAYMENT_METHOD_TEMPLATE;
 		$payment_method_mock['id']               = $payment_method_id;
 		$payment_method_mock['customer']         = $customer_id;
@@ -1898,7 +1904,7 @@ public function test_subscription_renewal_checks_payment_method_authorization()
 		$this->mock_gateway->expects( $this->once() )
 			->method( 'create_and_confirm_intent_for_off_session' )
 			->with(
-				wc_get_order( $order_id ),
+				$order,
 				$prepared_source,
 				$amount
 			)
@@ -1919,7 +1925,7 @@ public function test_subscription_renewal_checks_payment_method_authorization()
 			->method( 'get_latest_charge_from_intent' )
 			->willReturn( $this->array_to_object( $charge ) );
 
-		$this->mock_gateway->process_subscription_payment( $amount, wc_get_order( $order_id ), false, false );
+		$this->mock_gateway->process_subscription_payment( $amount, $order, false, false );
 
 		$final_order = wc_get_order( $order_id );
 		$note        = wc_get_order_notes(

diff --git a/tests/phpunit/test-wc-stripe-payment-gateway.php b/tests/phpunit/test-wc-stripe-payment-gateway.php
@@ -568,4 +568,51 @@ public function test_get_balance_transaction_id_from_charge() {
 
 		$this->assertEquals( null, $this->gateway->get_balance_transaction_id_from_charge( null ) );
 	}
+
+	/**
+	 * Tests for `lock_order_payment` method.
+	 */
+	public function test_lock_order_payment() {
+		$order_1 = WC_Helper_Order::create_order();
+		$locked  = $this->gateway->lock_order_payment( $order_1 );
+
+		$this->assertFalse( $locked );
+		$current_lock = $order_1->get_meta( '_stripe_lock_payment' );
+		$this->assertEqualsWithDelta( (int) $current_lock, ( time() + 5 * MINUTE_IN_SECONDS ), 3 );
+
+		$locked = $this->gateway->lock_order_payment( $order_1 );
+		$this->assertTrue( $locked );
+
+		// lock with an intent ID.
+		$order_2   = WC_Helper_Order::create_order();
+		$intent_id = 'pi_123intent';
+
+		$locked       = $this->gateway->lock_order_payment( $order_2, $intent_id );
+		$current_lock = $order_2->get_meta( '_stripe_lock_payment' );
+
+		$this->assertFalse( $locked );
+		$locked = $this->gateway->lock_order_payment( $order_2, $intent_id );
+		$this->assertTrue( $locked );
+		$locked = $this->gateway->lock_order_payment( $order_2 ); // test that you don't need to pass the intent ID to check lock.
+		$this->assertTrue( $locked );
+
+		// test expired locks.
+		$order_3 = WC_Helper_Order::create_order();
+		$order_3->update_meta_data( '_stripe_lock_payment', time() - 1 );
+		$order_3->save_meta_data();
+
+		$locked       = $this->gateway->lock_order_payment( $order_3, $intent_id );
+		$current_lock = $order_3->get_meta( '_stripe_lock_payment' );
+
+		$this->assertFalse( $locked );
+		$this->assertEqualsWithDelta( (int) $current_lock, ( time() + 5 * MINUTE_IN_SECONDS ), 3 );
+
+		// test two instances of the same order, one locked and one not.
+		$order_4   = WC_Helper_Order::create_order();
+		$dup_order = wc_get_order( $order_4->get_id() );
+
+		$this->gateway->lock_order_payment( $order_4 );
+		$dup_locked = $this->gateway->lock_order_payment( $dup_order );
+		$this->assertTrue( $dup_locked ); // Confirms lock from $order_4 prevents payment on $dup_order.
+	}
 }