diff --git a/plugins/woocommerce/changelog/fix-duplicate-spec-evaluation b/plugins/woocommerce/changelog/fix-duplicate-spec-evaluation new file mode 100644 index 00000000000..299eaf7c9d6 --- /dev/null +++ b/plugins/woocommerce/changelog/fix-duplicate-spec-evaluation @@ -0,0 +1,4 @@ +Significance: patch +Type: fix + +Fix duplicate spec evaluation in evaluate_specs() diff --git a/plugins/woocommerce/src/Admin/Features/PaymentGatewaySuggestions/EvaluateSuggestion.php b/plugins/woocommerce/src/Admin/Features/PaymentGatewaySuggestions/EvaluateSuggestion.php index 45d609da940..4b614367199 100644 --- a/plugins/woocommerce/src/Admin/Features/PaymentGatewaySuggestions/EvaluateSuggestion.php +++ b/plugins/woocommerce/src/Admin/Features/PaymentGatewaySuggestions/EvaluateSuggestion.php @@ -13,6 +13,13 @@ use Automattic\WooCommerce\Admin\RemoteSpecs\RuleProcessors\RuleEvaluator; * Evaluates the spec and returns the evaluated suggestion. */ class EvaluateSuggestion { + /** + * Stores memoized results of evaluate_specs. + * + * @var array + */ + protected static $memo = array(); + /** * Evaluates the spec and returns the suggestion. * @@ -58,6 +65,12 @@ class EvaluateSuggestion { * @return array The visible suggestions and errors. */ public static function evaluate_specs( $specs, $logger_args = array() ) { + $specs_key = self::get_memo_key( $specs ); + + if ( isset( self::$memo[ $specs_key ] ) ) { + return self::$memo[ $specs_key ]; + } + $suggestions = array(); $errors = array(); @@ -72,9 +85,43 @@ class EvaluateSuggestion { } } - return array( + $result = array( 'suggestions' => $suggestions, 'errors' => $errors, ); + + // Memoize results, with a fail safe to prevent unbounded memory growth. + // This limit is unlikely to be reached under normal circumstances. + if ( count( self::$memo ) > 50 ) { + self::reset_memo(); + } + self::$memo[ $specs_key ] = $result; + + return $result; + } + + /** + * Resets the memoized results. Useful for testing. + */ + public static function reset_memo() { + self::$memo = array(); + } + + /** + * Returns a memoization key for the given specs. + * + * @param array $specs The specs to generate a key for. + * + * @return string The memoization key. + */ + private static function get_memo_key( $specs ) { + $data = wp_json_encode( $specs ); + + if ( function_exists( 'hash' ) && in_array( 'xxh3', hash_algos(), true ) ) { + // Use xxHash (xxh3) if available. + return hash( 'xxh3', $data ); + } + // Fall back to CRC32. + return (string) crc32( $data ); } } diff --git a/plugins/woocommerce/tests/legacy/unit-tests/woocommerce-admin/features/payment-gateway-suggestions/evaluate-suggestion.php b/plugins/woocommerce/tests/legacy/unit-tests/woocommerce-admin/features/payment-gateway-suggestions/evaluate-suggestion.php index 15fee01caea..3b0a70952c6 100644 --- a/plugins/woocommerce/tests/legacy/unit-tests/woocommerce-admin/features/payment-gateway-suggestions/evaluate-suggestion.php +++ b/plugins/woocommerce/tests/legacy/unit-tests/woocommerce-admin/features/payment-gateway-suggestions/evaluate-suggestion.php @@ -323,6 +323,31 @@ class WC_Admin_Tests_PaymentGatewaySuggestions_EvaluateSuggestion extends WC_Uni remove_filter( 'woocommerce_admin_remote_specs_evaluator_should_log', '__return_true' ); } + /** + * Test that the memo is set correctly. + */ + public function test_memo_set_correctly() { + $specs = array( + array( + 'id' => 'test-gateway-1', + 'is_visible' => true, + ), + array( + 'id' => 'test-gateway-2', + 'is_visible' => false, + ), + ); + + $result = TestableEvaluateSuggestion::evaluate_specs( $specs ); + $memo = TestableEvaluateSuggestion::get_memo_for_tests(); + + $this->assertCount( 1, $memo ); + $memo_key = array_keys( $memo )[0]; + $this->assertEquals( $result, $memo[ $memo_key ] ); + $this->assertCount( 1, $result['suggestions'] ); + $this->assertEquals( 'test-gateway-1', $result['suggestions'][0]->id ); + } + /** * Overrides the WC logger. * @@ -359,3 +384,19 @@ class WC_Admin_Tests_PaymentGatewaySuggestions_EvaluateSuggestion extends WC_Uni ); } } + +//phpcs:disable Generic.Files.OneObjectStructurePerFile.MultipleFound, Squiz.Classes.ClassFileName.NoMatch, Suin.Classes.PSR4.IncorrectClassName +/** + * TestableEvaluateSuggestion class. + */ +class TestableEvaluateSuggestion extends EvaluateSuggestion { + /** + * Get the memo for testing. + * + * @return array + */ + public static function get_memo_for_tests() { + return self::$memo; + } +} +//phpcs:enable Generic.Files.OneObjectStructurePerFile.MultipleFound, Squiz.Classes.ClassFileName.NoMatch, Suin.Classes.PSR4.IncorrectClassName diff --git a/plugins/woocommerce/tests/legacy/unit-tests/woocommerce-admin/features/payment-gateway-suggestions/payment-gateway-suggestions.php b/plugins/woocommerce/tests/legacy/unit-tests/woocommerce-admin/features/payment-gateway-suggestions/payment-gateway-suggestions.php index 728c78ce286..bb2ef0f93f1 100644 --- a/plugins/woocommerce/tests/legacy/unit-tests/woocommerce-admin/features/payment-gateway-suggestions/payment-gateway-suggestions.php +++ b/plugins/woocommerce/tests/legacy/unit-tests/woocommerce-admin/features/payment-gateway-suggestions/payment-gateway-suggestions.php @@ -25,7 +25,7 @@ class WC_Admin_Tests_PaymentGatewaySuggestions_Init extends WC_Unit_Test_Case { delete_option( 'woocommerce_show_marketplace_suggestions' ); add_filter( 'transient_woocommerce_admin_' . PaymentGatewaySuggestionsDataSourcePoller::ID . '_specs', - function( $value ) { + function ( $value ) { if ( $value ) { return $value; } @@ -37,6 +37,8 @@ class WC_Admin_Tests_PaymentGatewaySuggestions_Init extends WC_Unit_Test_Case { ); } ); + + EvaluateSuggestion::reset_memo(); } /** @@ -57,7 +59,7 @@ class WC_Admin_Tests_PaymentGatewaySuggestions_Init extends WC_Unit_Test_Case { remove_all_filters( 'transient_woocommerce_admin_' . PaymentGatewaySuggestionsDataSourcePoller::ID . '_specs' ); add_filter( DataSourcePoller::FILTER_NAME, - function() { + function () { return array(); } ); @@ -242,7 +244,7 @@ class WC_Admin_Tests_PaymentGatewaySuggestions_Init extends WC_Unit_Test_Case { add_filter( 'locale', - function( $_locale ) { + function () { return 'zh_TW'; } ); @@ -364,5 +366,4 @@ class WC_Admin_Tests_PaymentGatewaySuggestions_Init extends WC_Unit_Test_Case { // Clean up. delete_option( PaymentGatewaySuggestions::RECOMMENDED_PAYMENT_PLUGINS_DISMISS_OPTION ); } - } diff --git a/plugins/woocommerce/tests/php/src/Admin/ShippingPartnerSuggestions/DefaultShippingPartnersTest.php b/plugins/woocommerce/tests/php/src/Admin/Features/ShippingPartnerSuggestions/DefaultShippingPartnersTest.php similarity index 97% rename from plugins/woocommerce/tests/php/src/Admin/ShippingPartnerSuggestions/DefaultShippingPartnersTest.php rename to plugins/woocommerce/tests/php/src/Admin/Features/ShippingPartnerSuggestions/DefaultShippingPartnersTest.php index ecea313caa7..44b15c965f6 100644 --- a/plugins/woocommerce/tests/php/src/Admin/ShippingPartnerSuggestions/DefaultShippingPartnersTest.php +++ b/plugins/woocommerce/tests/php/src/Admin/Features/ShippingPartnerSuggestions/DefaultShippingPartnersTest.php @@ -1,7 +1,7 @@ mock_logger = $this->getMockBuilder( 'WC_Logger_Interface' )->getMock(); add_filter( 'woocommerce_logging_class', array( $this, 'override_wc_logger' ) ); + + EvaluateSuggestion::reset_memo(); } /** @@ -91,7 +94,7 @@ class ShippingPartnerSuggestionsTest extends WC_Unit_Test_Case { remove_all_filters( 'transient_woocommerce_admin_' . ShippingPartnerSuggestionsDataSourcePoller::ID . '_specs' ); add_filter( DataSourcePoller::FILTER_NAME, - function() { + function () { return array(); } ); diff --git a/plugins/woocommerce/tests/php/src/Internal/Admin/WCPayPromotion/DefaultPromotionsTest.php b/plugins/woocommerce/tests/php/src/Internal/Admin/WCPayPromotion/DefaultPromotionsTest.php index 893cc7a9753..29351fce779 100644 --- a/plugins/woocommerce/tests/php/src/Internal/Admin/WCPayPromotion/DefaultPromotionsTest.php +++ b/plugins/woocommerce/tests/php/src/Internal/Admin/WCPayPromotion/DefaultPromotionsTest.php @@ -29,6 +29,8 @@ class DefaultPromotionsTest extends WC_Unit_Test_Case { update_option( 'woocommerce_store_address', 'foo' ); update_option( 'active_plugins', array( 'foo/foo.php' ) ); + + EvaluateSuggestion::reset_memo(); } /** diff --git a/plugins/woocommerce/tests/php/src/Internal/Admin/WCPayPromotion/InitTest.php b/plugins/woocommerce/tests/php/src/Internal/Admin/WCPayPromotion/InitTest.php index 3a51a919fa3..cfe7b606f69 100644 --- a/plugins/woocommerce/tests/php/src/Internal/Admin/WCPayPromotion/InitTest.php +++ b/plugins/woocommerce/tests/php/src/Internal/Admin/WCPayPromotion/InitTest.php @@ -26,7 +26,7 @@ class InitTest extends WC_Unit_Test_Case { delete_option( 'woocommerce_show_marketplace_suggestions' ); add_filter( 'transient_woocommerce_admin_' . WCPayPromotionDataSourcePoller::ID . '_specs', - function( $value ) { + function ( $value ) { if ( $value ) { return $value; } @@ -38,6 +38,8 @@ class InitTest extends WC_Unit_Test_Case { ); } ); + + EvaluateSuggestion::reset_memo(); } /** @@ -59,7 +61,7 @@ class InitTest extends WC_Unit_Test_Case { remove_all_filters( 'transient_woocommerce_admin_' . WCPayPromotionDataSourcePoller::ID . '_specs' ); add_filter( DataSourcePoller::FILTER_NAME, - function() { + function () { return array(); } );