From 572b497ec7f0ef60554769508b39b25eb109947e Mon Sep 17 00:00:00 2001 From: Chi-Hsuan Huang Date: Tue, 23 Jul 2024 19:57:20 +0800 Subject: [PATCH] Move remote logger to `./src` and enhance `fetch_latest_woocommerce_version()` logic (#49639) * Move remote logger to ./src * Change fetch_latest_woocommerce_version transient expiration to a week * Add changelog * Add retry timeout to prevent repeated attempts to connect to the API when it errors * Fix Remote_Logger * Remove class_wc_remote_logger.php * Update changelog * Rename remote logger directory * Move to internel * Move to internel * Update plugins/woocommerce/src/Internal/Loggers/RemoteLogger.php Co-authored-by: Adrian Duffell <9312929+adrianduffell@users.noreply.github.com> * Rename folder/namespace Logger -> Logging --------- Co-authored-by: Adrian Duffell <9312929+adrianduffell@users.noreply.github.com> --- .../changelog/update-remote-logging | 4 + .../includes/class-woocommerce.php | 2 - .../LoggingServiceProvider.php | 4 + .../Internal/Logging/RemoteLogger.php} | 29 ++++-- .../Internal/Logging/RemoteLoggerTest.php} | 96 ++++++++++++++----- 5 files changed, 105 insertions(+), 30 deletions(-) create mode 100644 plugins/woocommerce/changelog/update-remote-logging rename plugins/woocommerce/{includes/class-wc-remote-logger.php => src/Internal/Logging/RemoteLogger.php} (76%) rename plugins/woocommerce/tests/php/{includes/class-wc-remote-logger.php => src/Internal/Logging/RemoteLoggerTest.php} (50%) diff --git a/plugins/woocommerce/changelog/update-remote-logging b/plugins/woocommerce/changelog/update-remote-logging new file mode 100644 index 00000000000..231f49d7e53 --- /dev/null +++ b/plugins/woocommerce/changelog/update-remote-logging @@ -0,0 +1,4 @@ +Significance: patch +Type: update + +Move remote logger to `./src` and improve `fetch_latest_woocommerce_version()` logic diff --git a/plugins/woocommerce/includes/class-woocommerce.php b/plugins/woocommerce/includes/class-woocommerce.php index 978452542c0..abba659550e 100644 --- a/plugins/woocommerce/includes/class-woocommerce.php +++ b/plugins/woocommerce/includes/class-woocommerce.php @@ -28,7 +28,6 @@ use Automattic\WooCommerce\Internal\Utilities\WebhookUtil; use Automattic\WooCommerce\Internal\Admin\Marketplace; use Automattic\WooCommerce\Proxies\LegacyProxy; use Automattic\WooCommerce\Utilities\{LoggingUtil, RestApiUtil, TimeUtil}; -use Automattic\WooCommerce\Admin\WCAdminHelper; /** * Main WooCommerce Class. @@ -647,7 +646,6 @@ final class WooCommerce { include_once WC_ABSPATH . 'includes/class-wc-structured-data.php'; include_once WC_ABSPATH . 'includes/class-wc-shortcodes.php'; include_once WC_ABSPATH . 'includes/class-wc-logger.php'; - include_once WC_ABSPATH . 'includes/class-wc-remote-logger.php'; include_once WC_ABSPATH . 'includes/queue/class-wc-action-queue.php'; include_once WC_ABSPATH . 'includes/queue/class-wc-queue.php'; include_once WC_ABSPATH . 'includes/admin/marketplace-suggestions/class-wc-marketplace-updater.php'; diff --git a/plugins/woocommerce/src/Internal/DependencyManagement/ServiceProviders/LoggingServiceProvider.php b/plugins/woocommerce/src/Internal/DependencyManagement/ServiceProviders/LoggingServiceProvider.php index 58946dac50c..89f15221230 100644 --- a/plugins/woocommerce/src/Internal/DependencyManagement/ServiceProviders/LoggingServiceProvider.php +++ b/plugins/woocommerce/src/Internal/DependencyManagement/ServiceProviders/LoggingServiceProvider.php @@ -5,6 +5,7 @@ namespace Automattic\WooCommerce\Internal\DependencyManagement\ServiceProviders; use Automattic\WooCommerce\Internal\Admin\Logging\{ PageController, Settings }; use Automattic\WooCommerce\Internal\Admin\Logging\FileV2\FileController; use Automattic\WooCommerce\Internal\DependencyManagement\AbstractServiceProvider; +use Automattic\WooCommerce\Internal\Logging\RemoteLogger; /** * LoggingServiceProvider class. @@ -19,6 +20,7 @@ class LoggingServiceProvider extends AbstractServiceProvider { FileController::class, PageController::class, Settings::class, + RemoteLogger::class, ); /** @@ -37,5 +39,7 @@ class LoggingServiceProvider extends AbstractServiceProvider { ); $this->share( Settings::class ); + + $this->share( RemoteLogger::class ); } } diff --git a/plugins/woocommerce/includes/class-wc-remote-logger.php b/plugins/woocommerce/src/Internal/Logging/RemoteLogger.php similarity index 76% rename from plugins/woocommerce/includes/class-wc-remote-logger.php rename to plugins/woocommerce/src/Internal/Logging/RemoteLogger.php index 1145d4b17e8..6d9cb9cf146 100644 --- a/plugins/woocommerce/includes/class-wc-remote-logger.php +++ b/plugins/woocommerce/src/Internal/Logging/RemoteLogger.php @@ -1,6 +1,8 @@ = 3 ) { + return null; + } + if ( ! function_exists( 'plugins_api' ) ) { require_once ABSPATH . 'wp-admin/includes/plugin-install.php'; } - + // Fetch the latest version from the WordPress API. $plugin_info = plugins_api( 'plugin_information', array( 'slug' => 'woocommerce' ) ); + if ( is_wp_error( $plugin_info ) ) { + ++$retry_count; + set_transient( self::FETCH_LATEST_VERSION_RETRY, $retry_count, HOUR_IN_SECONDS ); return null; } if ( ! empty( $plugin_info->version ) ) { $latest_version = $plugin_info->version; - set_transient( $transient_key, $latest_version, DAY_IN_SECONDS ); + set_transient( self::WC_LATEST_VERSION_TRANSIENT, $latest_version, WEEK_IN_SECONDS ); + delete_transient( self::FETCH_LATEST_VERSION_RETRY ); return $latest_version; } diff --git a/plugins/woocommerce/tests/php/includes/class-wc-remote-logger.php b/plugins/woocommerce/tests/php/src/Internal/Logging/RemoteLoggerTest.php similarity index 50% rename from plugins/woocommerce/tests/php/includes/class-wc-remote-logger.php rename to plugins/woocommerce/tests/php/src/Internal/Logging/RemoteLoggerTest.php index eca1185078e..f363e75346f 100644 --- a/plugins/woocommerce/tests/php/includes/class-wc-remote-logger.php +++ b/plugins/woocommerce/tests/php/src/Internal/Logging/RemoteLoggerTest.php @@ -1,10 +1,22 @@ sut = wc_get_container()->get( RemoteLogger::class ); WC()->version = '9.2.0'; } @@ -27,6 +40,7 @@ class WC_Remote_Logger_Test extends \WC_Unit_Test_Case { $this->cleanup_filters(); delete_option( 'woocommerce_feature_remote_logging_enabled' ); + delete_transient( RemoteLogger::WC_LATEST_VERSION_TRANSIENT ); } /** @@ -39,10 +53,11 @@ class WC_Remote_Logger_Test extends \WC_Unit_Test_Case { remove_all_filters( 'option_woocommerce_allow_tracking' ); remove_all_filters( 'option_woocommerce_version' ); remove_all_filters( 'option_woocommerce_remote_variant_assignment' ); + remove_all_filters( 'plugins_api' ); } /** - * Test that remote logging is allowed when all conditions are met. + * @testdox Test that remote logging is allowed when all conditions are met. */ public function test_remote_logging_allowed() { update_option( 'woocommerce_feature_remote_logging_enabled', 'yes' ); @@ -74,12 +89,11 @@ class WC_Remote_Logger_Test extends \WC_Unit_Test_Case { 3 ); - $checker = new WC_Remote_Logger(); - $this->assertTrue( $checker->is_remote_logging_allowed() ); + $this->assertTrue( $this->sut->is_remote_logging_allowed() ); } /** - * Test that remote logging is not allowed when the feature flag is disabled. + * @testdox Test that remote logging is not allowed when the feature flag is disabled. */ public function test_remote_logging_not_allowed_feature_flag_disabled() { update_option( 'woocommerce_feature_remote_logging_enabled', 'no' ); @@ -97,14 +111,13 @@ class WC_Remote_Logger_Test extends \WC_Unit_Test_Case { } ); - set_transient( 'latest_woocommerce_version', '9.2.0', DAY_IN_SECONDS ); + set_transient( RemoteLogger::WC_LATEST_VERSION_TRANSIENT, '9.2.0', DAY_IN_SECONDS ); - $checker = new WC_Remote_Logger(); - $this->assertFalse( $checker->is_remote_logging_allowed() ); + $this->assertFalse( $this->sut->is_remote_logging_allowed() ); } /** - * Test that remote logging is not allowed when user tracking is not opted in. + * @testdox Test that remote logging is not allowed when user tracking is not opted in. */ public function test_remote_logging_not_allowed_tracking_opted_out() { update_option( 'woocommerce_feature_remote_logging_enabled', 'yes' ); @@ -121,14 +134,13 @@ class WC_Remote_Logger_Test extends \WC_Unit_Test_Case { } ); - set_transient( 'latest_woocommerce_version', '9.2.0', DAY_IN_SECONDS ); + set_transient( RemoteLogger::WC_LATEST_VERSION_TRANSIENT, '9.2.0', DAY_IN_SECONDS ); - $checker = new WC_Remote_Logger(); - $this->assertFalse( $checker->is_remote_logging_allowed() ); + $this->assertFalse( $this->sut->is_remote_logging_allowed() ); } /** - * Test that remote logging is not allowed when the WooCommerce version is outdated. + * @testdox Test that remote logging is not allowed when the WooCommerce version is outdated. */ public function test_remote_logging_not_allowed_outdated_version() { update_option( 'woocommerce_feature_remote_logging_enabled', 'yes' ); @@ -145,15 +157,14 @@ class WC_Remote_Logger_Test extends \WC_Unit_Test_Case { } ); - set_transient( 'latest_woocommerce_version', '9.2.0', DAY_IN_SECONDS ); + set_transient( RemoteLogger::WC_LATEST_VERSION_TRANSIENT, '9.2.0', DAY_IN_SECONDS ); WC()->version = '9.0.0'; - $checker = new WC_Remote_Logger(); - $this->assertFalse( $checker->is_remote_logging_allowed() ); + $this->assertFalse( $this->sut->is_remote_logging_allowed() ); } /** - * Test that remote logging is not allowed when the variant assignment is high. + * @testdox Test that remote logging is not allowed when the variant assignment is high. */ public function test_remote_logging_not_allowed_high_variant_assignment() { update_option( 'woocommerce_feature_remote_logging_enabled', 'yes' ); @@ -176,9 +187,50 @@ class WC_Remote_Logger_Test extends \WC_Unit_Test_Case { } ); - set_transient( 'latest_woocommerce_version', '9.2.0', DAY_IN_SECONDS ); + set_transient( RemoteLogger::WC_LATEST_VERSION_TRANSIENT, '9.2.0', DAY_IN_SECONDS ); - $checker = new WC_Remote_Logger(); - $this->assertFalse( $checker->is_remote_logging_allowed() ); + $this->assertFalse( $this->sut->is_remote_logging_allowed() ); + } + + /** + * @testdox Test that the fetch_latest_woocommerce_version method retries fetching the latest WooCommerce version when the API call fails. + */ + public function test_fetch_latest_woocommerce_version_retry() { + update_option( 'woocommerce_feature_remote_logging_enabled', 'yes' ); + + add_filter( + 'option_woocommerce_allow_tracking', + function () { + return 'yes'; + } + ); + add_filter( + 'option_woocommerce_remote_variant_assignment', + function () { + return 5; + } + ); + + add_filter( + 'plugins_api', // phpcs:ignore PEAR.Functions.FunctionCallSignature.MultipleArguments + function ( $result, $action, $args ) { // phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsed + return new \WP_Error(); + }, + 10, + 3 + ); + + $this->sut->is_remote_logging_allowed(); + $this->assertEquals( 1, get_transient( RemoteLogger::FETCH_LATEST_VERSION_RETRY ) ); + + $this->sut->is_remote_logging_allowed(); + $this->assertEquals( 2, get_transient( RemoteLogger::FETCH_LATEST_VERSION_RETRY ) ); + + $this->sut->is_remote_logging_allowed(); + $this->assertEquals( 3, get_transient( RemoteLogger::FETCH_LATEST_VERSION_RETRY ) ); + + // After 3 retries, the transient should not be updated. + $this->sut->is_remote_logging_allowed(); + $this->assertEquals( 3, get_transient( RemoteLogger::FETCH_LATEST_VERSION_RETRY ) ); } }