From bc95775986245508c1141e7a54c5b2e378869019 Mon Sep 17 00:00:00 2001 From: Dale du Preez Date: Fri, 16 May 2025 13:57:01 +0200 Subject: [PATCH 1/7] Enforce plugin rate limit when Stripe has rate limited us --- includes/class-wc-stripe-api.php | 84 +++++++++ tests/phpunit/test-class-wc-stripe-api.php | 197 +++++++++++++++++++++ 2 files changed, 281 insertions(+) diff --git a/includes/class-wc-stripe-api.php b/includes/class-wc-stripe-api.php index 0ce455bb3d..b8dc9d226c 100644 --- a/includes/class-wc-stripe-api.php +++ b/includes/class-wc-stripe-api.php @@ -16,6 +16,28 @@ class WC_Stripe_API { const ENDPOINT = 'https://api.stripe.com/v1/'; const STRIPE_API_VERSION = '2024-06-20'; + + /** + * Option key for cases where Stripe rate limits our live API calls. + * + * @var string + */ + public const LIVE_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY = 'wc_stripe_api_rate_limit_live'; + + /** + * Option key for cases where Stripe rate limits our test API calls. + * + * @var string + */ + public const TEST_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY = 'wc_stripe_api_rate_limit_test'; + + /** + * Duration we will use to disable Stripe API calls if we have been rate limited. + * + * @var int + */ + public const STRIPE_API_RATE_LIMIT_DURATION = 30; + /** * Secret API Key. * @@ -231,6 +253,10 @@ public static function request( $request, $api = 'charges', $method = 'POST', $w * @param string $api */ public static function retrieve( $api ) { + if ( self::is_stripe_api_rate_limited() ) { + return null; + } + WC_Stripe_Logger::log( "{$api}" ); $response = wp_safe_remote_get( @@ -243,6 +269,7 @@ public static function retrieve( $api ) { ); if ( is_wp_error( $response ) || empty( $response['body'] ) ) { + self::check_stripe_api_error_response( $response ); WC_Stripe_Logger::log( 'Error Response: ' . print_r( $response, true ) ); return new WP_Error( 'stripe_error', __( 'There was a problem connecting to the Stripe API endpoint.', 'woocommerce-gateway-stripe' ) ); } @@ -250,6 +277,63 @@ public static function retrieve( $api ) { return json_decode( $response['body'] ); } + /** + * Checks if the Stripe API for the current mode (i.e. test or live) is rate limited. + * + * @return bool True if the Stripe API is rate limited, false otherwise. + */ + public static function is_stripe_api_rate_limited() { + $rate_limit_option_key = WC_Stripe_Mode::is_test() ? self::TEST_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY : self::LIVE_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY; + + $rate_limit_expiration = get_option( $rate_limit_option_key ); + if ( ! $rate_limit_expiration ) { + return false; + } + + $now = time(); + if ( $now > $rate_limit_expiration ) { + delete_option( $rate_limit_option_key ); + return false; + } + + return true; + } + + /** + * Helper function to check error responses from Stripe and ensure we prevent unnecessary API calls, + * primarily in cases where we have been rate limited or we don't have valid keys.. + * + * @param array|WP_Error $response The response from the Stripe API. + * @return void + */ + protected static function check_stripe_api_error_response( $response ) { + // If we don't have an array for $response, return early, as we won't have an HTTP status code. + if ( ! is_array( $response ) ) { + return; + } + + // We specifically want to check $response['response']['code']. If it's not present, return early. + if ( ! isset( $response['response'] ) || ! is_array( $response['response'] ) || ! isset( $response['response']['code'] ) ) { + return; + } + + $status_code = $response['response']['code']; + + if ( 429 === $status_code ) { + // Stripe has rate limited us, so disable API calls for a period of time. + $is_test_mode = WC_Stripe_Mode::is_test(); + + $rate_limit_option_key = $is_test_mode ? self::TEST_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY : self::LIVE_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY; + update_option( $rate_limit_option_key, time() + self::STRIPE_API_RATE_LIMIT_DURATION ); + + $mode = $is_test_mode ? 'test' : 'LIVE'; + $message = "Stripe {$mode} mode API has been rate limited, disabling API calls for " . self::STRIPE_API_RATE_LIMIT_DURATION . ' seconds.'; + + error_log( 'woocommerce-gateway-stripe: WARNING: ' . $message ); + WC_Stripe_Logger::error( $message ); + } + } + /** * Send the request to Stripe's API with level 3 data generated * from the order. If the request fails due to an error related diff --git a/tests/phpunit/test-class-wc-stripe-api.php b/tests/phpunit/test-class-wc-stripe-api.php index 61a64aacf4..e14fbefe2e 100644 --- a/tests/phpunit/test-class-wc-stripe-api.php +++ b/tests/phpunit/test-class-wc-stripe-api.php @@ -40,6 +40,17 @@ public function set_up() { public function tear_down() { WC_Stripe_Helper::delete_main_stripe_settings(); WC_Stripe_API::set_secret_key( null ); + + WC_Stripe_Logger::$logger = null; + + if ( false !== has_filter( 'pre_http_request', [ $this, 'mock_429_response' ] ) ) { + remove_filter( 'pre_http_request', [ $this, 'mock_429_response' ] ); + } + + if ( false !== has_filter( 'pre_http_request', [ $this, 'throw_exception_on_http_request' ] ) ) { + remove_filter( 'pre_http_request', [ $this, 'throw_exception_on_http_request' ] ); + } + parent::tear_down(); } @@ -94,4 +105,190 @@ public function test_set_secret_key_for_mode_with_parameter() { WC_Stripe_API::set_secret_key_for_mode( 'invalid' ); $this->assertEquals( self::LIVE_SECRET_KEY, WC_Stripe_API::get_secret_key() ); } + + /** + * Provide test modes for test cases that test both test and live modes. + * + * @return array + */ + public function provide_test_modes() { + return [ + 'live mode' => [ false ], + 'test mode' => [ true ], + ]; + } + + /** + * Test WC_Stripe_API::retrieve() returns early when rate limit is active. + * + * @dataProvider provide_test_modes + * @param bool $is_test_mode Whether the test mode is true or false. + */ + public function test_retrieve_returns_early_when_rate_limit_is_active( $is_test_mode ) { + $settings = WC_Stripe_Helper::get_stripe_settings(); + $settings['testmode'] = $is_test_mode ? 'yes' : 'no'; + $settings['logging'] = 'yes'; + WC_Stripe_Helper::update_main_stripe_settings( $settings ); + + // Add this filter after we update the settings, as that code can trigger HTTP requests. + add_filter( 'pre_http_request', [ $this, 'throw_exception_on_http_request' ] ); + + $mock_logger = $this->createStub( WC_Logger_Interface::class ); + + $mock_logger->expects( $this->never() ) + ->method( 'debug' ); + + $mock_logger->expects( $this->never() ) + ->method( 'error' ); + + $now = time(); + $rate_limit_option_key = $is_test_mode ? WC_Stripe_API::TEST_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY : WC_Stripe_API::LIVE_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY; + update_option( $rate_limit_option_key, $now + 20 ); + + WC_Stripe_Logger::$logger = $mock_logger; + + $result = WC_Stripe_API::retrieve( 'account' ); + + WC_Stripe_Logger::$logger = null; + + $this->assertNull( $result ); + + remove_filter( 'pre_http_request', [ $this, 'throw_exception_on_http_request' ] ); + } + + /** + * Test WC_Stripe_API::is_stripe_api_rate_limited() returns false when no rate limit is active. + * + * @dataProvider provide_test_modes + * @param bool $is_test_mode Whether the test mode is true or false. + */ + public function test_rate_limit_check_returns_false_when_no_rate_limit_is_active( $is_test_mode ) { + $settings = WC_Stripe_Helper::get_stripe_settings(); + $settings['testmode'] = $is_test_mode ? 'yes' : 'no'; + WC_Stripe_Helper::update_main_stripe_settings( $settings ); + + $this->assertFalse( WC_Stripe_API::is_stripe_api_rate_limited() ); + } + + /** + * Test WC_Stripe_API::is_stripe_api_rate_limited() returns false and deletes the option after the rate limit expires. + * + * @dataProvider provide_test_modes + * @param bool $is_test_mode Whether the test mode is true or false. + */ + public function test_rate_limit_check_returns_false_and_deletes_option_after_rate_limit_expires( $is_test_mode ) { + $settings = WC_Stripe_Helper::get_stripe_settings(); + $settings['testmode'] = $is_test_mode ? 'yes' : 'no'; + WC_Stripe_Helper::update_main_stripe_settings( $settings ); + + $rate_limit_option_key = $is_test_mode ? WC_Stripe_API::TEST_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY : WC_Stripe_API::LIVE_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY; + update_option( $rate_limit_option_key, time() - 20 ); + + $this->assertFalse( WC_Stripe_API::is_stripe_api_rate_limited() ); + + $this->assertNull( get_option( $rate_limit_option_key, null ) ); + } + + /** + * Test WC_Stripe_API::retrieve() correctly triggers rate limiting when + * we receive a 429 response from the Stripe API. + * + * @dataProvider provide_test_modes + * @param bool $is_test_mode Whether the test mode is true or false. + */ + public function test_check_stripe_api_429_response_triggers_rate_limit( $is_test_mode ) { + $settings = WC_Stripe_Helper::get_stripe_settings(); + $settings['testmode'] = $is_test_mode ? 'yes' : 'no'; + $settings['logging'] = 'yes'; + WC_Stripe_Helper::update_main_stripe_settings( $settings ); + + $mock_logger = $this->createStub( WC_Logger_Interface::class ); + + $mock_logger->expects( $this->exactly( 2 ) ) + ->method( 'debug' ) + ->withConsecutive( + [ $this->get_expected_log_message( 'account' ) ], + [ $this->get_expected_log_prefix( 'Error Response: ' ) ], + ); + + $message_mode = $is_test_mode ? 'test' : 'LIVE'; + $mock_logger->expects( $this->once() ) + ->method( 'error' ) + ->with( + "Stripe $message_mode mode API has been rate limited, disabling API calls for " . WC_Stripe_API::STRIPE_API_RATE_LIMIT_DURATION . ' seconds.' + ); + + // Mock 429 responses from the Stripe API. + add_filter( 'pre_http_request', [ $this, 'mock_429_response' ] ); + + WC_Stripe_Logger::$logger = $mock_logger; + + $request_start_time = time(); + $result = WC_Stripe_API::retrieve( 'account' ); + $request_end_time = time(); + + // Unset the mock logger. + WC_Stripe_Logger::$logger = null; + + $this->assertInstanceOf( WP_Error::class, $result ); + $this->assertEquals( 'stripe_error', $result->get_error_code() ); + $this->assertEquals( 'There was a problem connecting to the Stripe API endpoint.', $result->get_error_message() ); + + $rate_limit_option_key = $is_test_mode ? WC_Stripe_API::TEST_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY : WC_Stripe_API::LIVE_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY; + $rate_limit_option = get_option( $rate_limit_option_key ); + $this->assertIsInt( $rate_limit_option ); + + $duration_delta = max( $request_end_time - $request_start_time, 1 ); + $this->assertEqualsWithDelta( $request_end_time + WC_Stripe_API::STRIPE_API_RATE_LIMIT_DURATION, $rate_limit_option, $duration_delta ); + + remove_filter( 'pre_http_request', [ $this, 'mock_429_response' ] ); + } + + /** + * Helper method to get the expected log message. + * + * @param string $message The message we expect to see in the log. + * @return string The expected log message. + */ + protected function get_expected_log_message( $message ) { + $expected_log_message = "\n" . '====Stripe Version: ' . WC_STRIPE_VERSION . '====' . "\n"; + $expected_log_message .= '====Stripe Plugin API Version: ' . WC_Stripe_API::STRIPE_API_VERSION . '====' . "\n"; + $expected_log_message .= '====Start Log====' . "\n" . $message . "\n" . '====End Log====' . "\n\n"; + + return $expected_log_message; + } + + /** + * Helper method to get the expected log message prefix. + * + * @param string $message The message prefix we expect to see in the log. + * @return string The expected log message prefix. + */ + protected function get_expected_log_prefix( $message ) { + $expected_log_prefix = "\n" . '====Stripe Version: ' . WC_STRIPE_VERSION . '====' . "\n"; + $expected_log_prefix .= '====Stripe Plugin API Version: ' . WC_Stripe_API::STRIPE_API_VERSION . '====' . "\n"; + $expected_log_prefix .= '====Start Log====' . "\n" . $message; + + return $this->stringStartsWith( $expected_log_prefix ); + } + + /** + * Helper method to mock an HTTP 429 response from the Stripe API. + */ + public function mock_429_response( $preempt ) { + return [ + 'response' => [ + 'code' => 429, + 'message' => 'Too many requests', + ], + 'body' => '', + ]; + } + + /** + * Helper method to throw an when triggered. + */ + public function throw_exception_on_http_request( $preempt ) { + throw new Exception( 'HTTP request should not be triggered' ); + } } From 67a158195998d47a3a24b3014e93c5c2de0657d3 Mon Sep 17 00:00:00 2001 From: Dale du Preez Date: Fri, 16 May 2025 14:33:52 +0200 Subject: [PATCH 2/7] Keep basic history so we can see if this is recurring on a site --- includes/class-wc-stripe-api.php | 23 ++++++++++++++++++--- tests/phpunit/test-class-wc-stripe-api.php | 24 +++++++++++++++++++--- 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/includes/class-wc-stripe-api.php b/includes/class-wc-stripe-api.php index b8dc9d226c..8d2c4612c7 100644 --- a/includes/class-wc-stripe-api.php +++ b/includes/class-wc-stripe-api.php @@ -22,14 +22,14 @@ class WC_Stripe_API { * * @var string */ - public const LIVE_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY = 'wc_stripe_api_rate_limit_live'; + public const LIVE_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY = 'wc_stripe_live_api_rate_limit'; /** * Option key for cases where Stripe rate limits our test API calls. * * @var string */ - public const TEST_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY = 'wc_stripe_api_rate_limit_test'; + public const TEST_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY = 'wc_stripe_test_api_rate_limit'; /** * Duration we will use to disable Stripe API calls if we have been rate limited. @@ -323,14 +323,31 @@ protected static function check_stripe_api_error_response( $response ) { // Stripe has rate limited us, so disable API calls for a period of time. $is_test_mode = WC_Stripe_Mode::is_test(); + $timestamp = time(); $rate_limit_option_key = $is_test_mode ? self::TEST_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY : self::LIVE_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY; - update_option( $rate_limit_option_key, time() + self::STRIPE_API_RATE_LIMIT_DURATION ); + update_option( $rate_limit_option_key, $timestamp + self::STRIPE_API_RATE_LIMIT_DURATION ); $mode = $is_test_mode ? 'test' : 'LIVE'; $message = "Stripe {$mode} mode API has been rate limited, disabling API calls for " . self::STRIPE_API_RATE_LIMIT_DURATION . ' seconds.'; error_log( 'woocommerce-gateway-stripe: WARNING: ' . $message ); WC_Stripe_Logger::error( $message ); + + // Store history of rate limits so we can see how often they're occurring. + $history_option_key = $rate_limit_option_key . '_history'; + $history = get_option( $history_option_key, [] ); + if ( ! is_array( $history ) ) { + $history = []; + } + // Keep a maximum of 20 rate limit history entries. + $history = array_slice( $history, -19 ); + $history[] = [ + 'timestamp' => $timestamp, + 'datetime' => gmdate( 'Y-m-d H:i:s', $timestamp ) . ' UTC', + 'duration' => self::STRIPE_API_RATE_LIMIT_DURATION, + ]; + // Note that we set autoload to false - we don't want this option to be autoloaded by default. + update_option( $history_option_key, $history, false ); } } diff --git a/tests/phpunit/test-class-wc-stripe-api.php b/tests/phpunit/test-class-wc-stripe-api.php index e14fbefe2e..0d5cbadfe2 100644 --- a/tests/phpunit/test-class-wc-stripe-api.php +++ b/tests/phpunit/test-class-wc-stripe-api.php @@ -202,6 +202,10 @@ public function test_check_stripe_api_429_response_triggers_rate_limit( $is_test $settings['logging'] = 'yes'; WC_Stripe_Helper::update_main_stripe_settings( $settings ); + $rate_limit_option_key = $is_test_mode ? WC_Stripe_API::TEST_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY : WC_Stripe_API::LIVE_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY; + $history_option_key = $rate_limit_option_key . '_history'; + update_option( $history_option_key, [], false ); + $mock_logger = $this->createStub( WC_Logger_Interface::class ); $mock_logger->expects( $this->exactly( 2 ) ) @@ -234,12 +238,26 @@ public function test_check_stripe_api_429_response_triggers_rate_limit( $is_test $this->assertEquals( 'stripe_error', $result->get_error_code() ); $this->assertEquals( 'There was a problem connecting to the Stripe API endpoint.', $result->get_error_message() ); - $rate_limit_option_key = $is_test_mode ? WC_Stripe_API::TEST_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY : WC_Stripe_API::LIVE_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY; $rate_limit_option = get_option( $rate_limit_option_key ); $this->assertIsInt( $rate_limit_option ); - $duration_delta = max( $request_end_time - $request_start_time, 1 ); - $this->assertEqualsWithDelta( $request_end_time + WC_Stripe_API::STRIPE_API_RATE_LIMIT_DURATION, $rate_limit_option, $duration_delta ); + $runtime_delta = max( $request_end_time - $request_start_time, 1 ); + $this->assertEqualsWithDelta( $request_end_time + WC_Stripe_API::STRIPE_API_RATE_LIMIT_DURATION, $rate_limit_option, $runtime_delta ); + + $history = get_option( $history_option_key, null ); + $this->assertIsArray( $history ); + $this->assertCount( 1, $history ); + + $history_entry = $history[0]; + $this->assertIsArray( $history_entry ); + $this->assertArrayHasKey( 'timestamp', $history_entry ); + $this->assertArrayHasKey( 'datetime', $history_entry ); + $this->assertArrayHasKey( 'duration', $history_entry ); + + $expected_timestamp = $rate_limit_option - WC_Stripe_API::STRIPE_API_RATE_LIMIT_DURATION; + $this->assertEquals( $expected_timestamp, $history_entry['timestamp'] ); + $this->assertEquals( gmdate( 'Y-m-d H:i:s', $expected_timestamp ) . ' UTC', $history_entry['datetime'] ); + $this->assertEquals( WC_Stripe_API::STRIPE_API_RATE_LIMIT_DURATION, $history_entry['duration'] ); remove_filter( 'pre_http_request', [ $this, 'mock_429_response' ] ); } From ab72abcbe36ceb9308cc4a4fc3f91c97f4d13aa1 Mon Sep 17 00:00:00 2001 From: Dale du Preez Date: Fri, 16 May 2025 16:09:22 +0200 Subject: [PATCH 3/7] Add explicit checks for 4xx and 5xx HTTP responses --- includes/class-wc-stripe-api.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/includes/class-wc-stripe-api.php b/includes/class-wc-stripe-api.php index 8d2c4612c7..853070330c 100644 --- a/includes/class-wc-stripe-api.php +++ b/includes/class-wc-stripe-api.php @@ -268,7 +268,13 @@ public static function retrieve( $api ) { ] ); - if ( is_wp_error( $response ) || empty( $response['body'] ) ) { + $is_error_response = is_wp_error( $response ) || empty( $response['body'] ); + // Check for 4xx and 5xx HTTP responses. + if ( ! $is_error_response && is_array( $response ) && isset( $response['response'] ) && is_array( $response['response'] ) && is_int( $response['response']['code'] ?? null ) ) { + $is_error_response = $response['response']['code'] >= 400; + } + + if ( $is_error_response ) { self::check_stripe_api_error_response( $response ); WC_Stripe_Logger::log( 'Error Response: ' . print_r( $response, true ) ); return new WP_Error( 'stripe_error', __( 'There was a problem connecting to the Stripe API endpoint.', 'woocommerce-gateway-stripe' ) ); From 06b7e62287f3c9fc5a5e3bfd5b3ec5782ea055f9 Mon Sep 17 00:00:00 2001 From: Dale du Preez Date: Fri, 16 May 2025 17:49:16 +0200 Subject: [PATCH 4/7] Separate rate checks from error handling --- includes/class-wc-stripe-api.php | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/includes/class-wc-stripe-api.php b/includes/class-wc-stripe-api.php index 853070330c..3fd8660a56 100644 --- a/includes/class-wc-stripe-api.php +++ b/includes/class-wc-stripe-api.php @@ -268,14 +268,9 @@ public static function retrieve( $api ) { ] ); - $is_error_response = is_wp_error( $response ) || empty( $response['body'] ); - // Check for 4xx and 5xx HTTP responses. - if ( ! $is_error_response && is_array( $response ) && isset( $response['response'] ) && is_array( $response['response'] ) && is_int( $response['response']['code'] ?? null ) ) { - $is_error_response = $response['response']['code'] >= 400; - } + self::check_stripe_api_error_response( $response ); - if ( $is_error_response ) { - self::check_stripe_api_error_response( $response ); + if ( is_wp_error( $response ) || empty( $response['body'] ) ) { WC_Stripe_Logger::log( 'Error Response: ' . print_r( $response, true ) ); return new WP_Error( 'stripe_error', __( 'There was a problem connecting to the Stripe API endpoint.', 'woocommerce-gateway-stripe' ) ); } From 2ec56a231a78926b3c812dec52e3bedd24407e5f Mon Sep 17 00:00:00 2001 From: Dale du Preez Date: Fri, 16 May 2025 19:27:34 +0200 Subject: [PATCH 5/7] Changelog --- changelog.txt | 1 + readme.txt | 1 + 2 files changed, 2 insertions(+) diff --git a/changelog.txt b/changelog.txt index 61f5d8ea53..d0d2669fe4 100644 --- a/changelog.txt +++ b/changelog.txt @@ -7,6 +7,7 @@ * Update - Remove BACS from the unsupported 'change payment method for subscription' page. * Fix - Fix payment method title display when new payment settings experience is enabled * Fix - Prevent styles from non-checkout pages affecting the appearance of Stripe element. +* Fix - Ensure that we apply rate limits when Stripe returns 429 responses = 9.5.0 - 2025-05-13 = * Fix - Fixes the listing of payment methods on the classic checkout when the Optimized Checkout is enabled. diff --git a/readme.txt b/readme.txt index 5e7858b583..647c953008 100644 --- a/readme.txt +++ b/readme.txt @@ -118,6 +118,7 @@ If you get stuck, you can ask for help in the [Plugin Forum](https://wordpress.o * Update - Remove BACS from the unsupported 'change payment method for subscription' page. * Fix - Fix payment method title display when new payment settings experience is enabled * Fix - Prevent styles from non-checkout pages affecting the appearance of Stripe element. +* Fix - Ensure that we apply rate limits when Stripe returns 429 responses [See changelog for full details across versions](https://raw.githubusercontent.com/woocommerce/woocommerce-gateway-stripe/trunk/changelog.txt). From 84e5af2857a5edf690d14c77916be77aa9147204 Mon Sep 17 00:00:00 2001 From: Dale du Preez Date: Fri, 16 May 2025 19:30:58 +0200 Subject: [PATCH 6/7] Improve name for duration constant --- includes/class-wc-stripe-api.php | 8 ++++---- tests/phpunit/test-class-wc-stripe-api.php | 9 +++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/includes/class-wc-stripe-api.php b/includes/class-wc-stripe-api.php index 3fd8660a56..936ae71b14 100644 --- a/includes/class-wc-stripe-api.php +++ b/includes/class-wc-stripe-api.php @@ -36,7 +36,7 @@ class WC_Stripe_API { * * @var int */ - public const STRIPE_API_RATE_LIMIT_DURATION = 30; + public const STRIPE_API_RATE_LIMIT_DURATION_IN_SECONDS = 30; /** * Secret API Key. @@ -326,10 +326,10 @@ protected static function check_stripe_api_error_response( $response ) { $timestamp = time(); $rate_limit_option_key = $is_test_mode ? self::TEST_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY : self::LIVE_MODE_STRIPE_API_RATE_LIMIT_OPTION_KEY; - update_option( $rate_limit_option_key, $timestamp + self::STRIPE_API_RATE_LIMIT_DURATION ); + update_option( $rate_limit_option_key, $timestamp + self::STRIPE_API_RATE_LIMIT_DURATION_IN_SECONDS ); $mode = $is_test_mode ? 'test' : 'LIVE'; - $message = "Stripe {$mode} mode API has been rate limited, disabling API calls for " . self::STRIPE_API_RATE_LIMIT_DURATION . ' seconds.'; + $message = "Stripe {$mode} mode API has been rate limited, disabling API calls for " . self::STRIPE_API_RATE_LIMIT_DURATION_IN_SECONDS . ' seconds.'; error_log( 'woocommerce-gateway-stripe: WARNING: ' . $message ); WC_Stripe_Logger::error( $message ); @@ -345,7 +345,7 @@ protected static function check_stripe_api_error_response( $response ) { $history[] = [ 'timestamp' => $timestamp, 'datetime' => gmdate( 'Y-m-d H:i:s', $timestamp ) . ' UTC', - 'duration' => self::STRIPE_API_RATE_LIMIT_DURATION, + 'duration' => self::STRIPE_API_RATE_LIMIT_DURATION_IN_SECONDS, ]; // Note that we set autoload to false - we don't want this option to be autoloaded by default. update_option( $history_option_key, $history, false ); diff --git a/tests/phpunit/test-class-wc-stripe-api.php b/tests/phpunit/test-class-wc-stripe-api.php index 0d5cbadfe2..05f0953b47 100644 --- a/tests/phpunit/test-class-wc-stripe-api.php +++ b/tests/phpunit/test-class-wc-stripe-api.php @@ -219,7 +219,8 @@ public function test_check_stripe_api_429_response_triggers_rate_limit( $is_test $mock_logger->expects( $this->once() ) ->method( 'error' ) ->with( - "Stripe $message_mode mode API has been rate limited, disabling API calls for " . WC_Stripe_API::STRIPE_API_RATE_LIMIT_DURATION . ' seconds.' + "Stripe $message_mode mode API has been rate limited, disabling API calls for " . + WC_Stripe_API::STRIPE_API_RATE_LIMIT_DURATION_IN_SECONDS . ' seconds.' ); // Mock 429 responses from the Stripe API. @@ -242,7 +243,7 @@ public function test_check_stripe_api_429_response_triggers_rate_limit( $is_test $this->assertIsInt( $rate_limit_option ); $runtime_delta = max( $request_end_time - $request_start_time, 1 ); - $this->assertEqualsWithDelta( $request_end_time + WC_Stripe_API::STRIPE_API_RATE_LIMIT_DURATION, $rate_limit_option, $runtime_delta ); + $this->assertEqualsWithDelta( $request_end_time + WC_Stripe_API::STRIPE_API_RATE_LIMIT_DURATION_IN_SECONDS, $rate_limit_option, $runtime_delta ); $history = get_option( $history_option_key, null ); $this->assertIsArray( $history ); @@ -254,10 +255,10 @@ public function test_check_stripe_api_429_response_triggers_rate_limit( $is_test $this->assertArrayHasKey( 'datetime', $history_entry ); $this->assertArrayHasKey( 'duration', $history_entry ); - $expected_timestamp = $rate_limit_option - WC_Stripe_API::STRIPE_API_RATE_LIMIT_DURATION; + $expected_timestamp = $rate_limit_option - WC_Stripe_API::STRIPE_API_RATE_LIMIT_DURATION_IN_SECONDS; $this->assertEquals( $expected_timestamp, $history_entry['timestamp'] ); $this->assertEquals( gmdate( 'Y-m-d H:i:s', $expected_timestamp ) . ' UTC', $history_entry['datetime'] ); - $this->assertEquals( WC_Stripe_API::STRIPE_API_RATE_LIMIT_DURATION, $history_entry['duration'] ); + $this->assertEquals( WC_Stripe_API::STRIPE_API_RATE_LIMIT_DURATION_IN_SECONDS, $history_entry['duration'] ); remove_filter( 'pre_http_request', [ $this, 'mock_429_response' ] ); } From 4e638b87ce0fa17289f030ddc4b598fe6ae3578e Mon Sep 17 00:00:00 2001 From: Dale du Preez Date: Fri, 16 May 2025 19:34:37 +0200 Subject: [PATCH 7/7] Use multiline OR for $response checks --- includes/class-wc-stripe-api.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/includes/class-wc-stripe-api.php b/includes/class-wc-stripe-api.php index 936ae71b14..0d994cf810 100644 --- a/includes/class-wc-stripe-api.php +++ b/includes/class-wc-stripe-api.php @@ -314,7 +314,11 @@ protected static function check_stripe_api_error_response( $response ) { } // We specifically want to check $response['response']['code']. If it's not present, return early. - if ( ! isset( $response['response'] ) || ! is_array( $response['response'] ) || ! isset( $response['response']['code'] ) ) { + if ( + ! isset( $response['response'] ) + || ! is_array( $response['response'] ) + || ! isset( $response['response']['code'] ) + ) { return; }