From c601a0b1974087049b30a88391943722ee919b44 Mon Sep 17 00:00:00 2001 From: Ilyas Foo Date: Wed, 21 Feb 2024 23:22:47 +0800 Subject: [PATCH] Wrangle MarketingRecommendations as RemoteSpecsEngine (#44828) * Fix potential recursion error, add MarketingRecommendations to extend RemoteSpecsEngine, add error handling, add tests * Changelog * Added spec evaluation and return default when empty * Update plugins/woocommerce/src/Admin/Features/MarketingRecommendations/Init.php Co-authored-by: Chi-Hsuan Huang --------- Co-authored-by: Chi-Hsuan Huang --- ...-add-marketing-recommendation-remote-specs | 4 + .../MarketingRecommendations/Init.php | 65 ++++++++++++-- .../MarketingRecommendations/InitTest.php | 90 +++++++++++++++++++ 3 files changed, 150 insertions(+), 9 deletions(-) create mode 100644 plugins/woocommerce/changelog/dev-add-marketing-recommendation-remote-specs create mode 100644 plugins/woocommerce/tests/php/src/Internal/Admin/MarketingRecommendations/InitTest.php diff --git a/plugins/woocommerce/changelog/dev-add-marketing-recommendation-remote-specs b/plugins/woocommerce/changelog/dev-add-marketing-recommendation-remote-specs new file mode 100644 index 00000000000..c55c8b52e0b --- /dev/null +++ b/plugins/woocommerce/changelog/dev-add-marketing-recommendation-remote-specs @@ -0,0 +1,4 @@ +Significance: patch +Type: dev + +Wrangle MarketingRecommendations as RemoteSpecsEngine diff --git a/plugins/woocommerce/src/Admin/Features/MarketingRecommendations/Init.php b/plugins/woocommerce/src/Admin/Features/MarketingRecommendations/Init.php index dfc9e266cd4..b6511def022 100644 --- a/plugins/woocommerce/src/Admin/Features/MarketingRecommendations/Init.php +++ b/plugins/woocommerce/src/Admin/Features/MarketingRecommendations/Init.php @@ -2,13 +2,15 @@ namespace Automattic\WooCommerce\Admin\Features\MarketingRecommendations; +use Automattic\WooCommerce\Admin\RemoteSpecs\RemoteSpecsEngine; + defined( 'ABSPATH' ) || exit; /** * Marketing Recommendations engine. * This goes through the specs and gets marketing recommendations. */ -class Init { +class Init extends RemoteSpecsEngine { /** * Slug of the category specifying marketing extensions on the Woo.com store. * @@ -54,6 +56,29 @@ class Init { return $specs; } + /** + * Process specs. + * + * @param array|null $specs Marketing recommendations spec array. + * @return array + */ + protected static function evaluate_specs( array $specs = null ) { + $suggestions = array(); + $errors = array(); + + foreach ( $specs as $spec ) { + try { + $suggestions[] = self::object_to_array( $spec ); + } catch ( \Throwable $e ) { + $errors[] = $e; + } + } + + return array( + 'suggestions' => $suggestions, + 'errors' => $errors, + ); + } /** * Load recommended plugins from Woo.com @@ -61,14 +86,30 @@ class Init { * @return array */ public static function get_recommended_plugins(): array { - $specs = self::get_specs(); - $result = array(); + $specs = self::get_specs(); + $results = self::evaluate_specs( $specs ); - foreach ( $specs as $spec ) { - $result[] = self::object_to_array( $spec ); + $specs_to_return = $results['suggestions']; + $specs_to_save = null; + + if ( empty( $specs_to_return ) ) { + // When suggestions is empty, replace it with defaults and save for 3 hours. + $specs_to_save = DefaultMarketingRecommendations::get_all(); + $specs_to_return = self::evaluate_specs( $specs_to_save )['suggestions']; + } elseif ( count( $results['errors'] ) > 0 ) { + // When suggestions is not empty but has errors, save it for 3 hours. + $specs_to_save = $specs; } - return $result; + if ( $specs_to_save ) { + MarketingRecommendationsDataSourcePoller::get_instance()->set_specs_transient( $specs_to_save, 3 * HOUR_IN_SECONDS ); + } + $errors = $results['errors']; + if ( ! empty( $errors ) ) { + self::log_errors( $errors ); + } + + return $specs_to_return; } /** @@ -139,16 +180,22 @@ class Init { * This is used to convert the specs to an array so that they can be returned by the API. * * @param mixed $obj Object to convert. + * @param array &$visited Reference to an array keeping track of all seen objects to detect circular references. * @return array */ - protected static function object_to_array( $obj ) { + public static function object_to_array( $obj, &$visited = array() ) { if ( is_object( $obj ) ) { - $obj = (array) $obj; + if ( in_array( $obj, $visited, true ) ) { + // Circular reference detected. + return null; + } + $visited[] = $obj; + $obj = (array) $obj; } if ( is_array( $obj ) ) { $new = array(); foreach ( $obj as $key => $val ) { - $new[ $key ] = self::object_to_array( $val ); + $new[ $key ] = self::object_to_array( $val, $visited ); } } else { $new = $obj; diff --git a/plugins/woocommerce/tests/php/src/Internal/Admin/MarketingRecommendations/InitTest.php b/plugins/woocommerce/tests/php/src/Internal/Admin/MarketingRecommendations/InitTest.php new file mode 100644 index 00000000000..adaca4c1861 --- /dev/null +++ b/plugins/woocommerce/tests/php/src/Internal/Admin/MarketingRecommendations/InitTest.php @@ -0,0 +1,90 @@ +user = $this->factory->user->create( + array( + 'role' => 'administrator', + ) + ); + delete_option( 'woocommerce_show_marketplace_suggestions' ); + } + + /** + * Tear down. + */ + public function tearDown(): void { + parent::tearDown(); + MarketingRecommendationsDataSourcePoller::get_instance()->delete_specs_transient(); + remove_all_filters( 'transient_woocommerce_admin_' . MarketingRecommendationsDataSourcePoller::ID . '_specs' ); + } + + /** + * Test that default specs are provided when remote sources don't exist. + */ + public function test_get_default_specs() { + remove_all_filters( 'transient_woocommerce_admin_' . MarketingRecommendationsDataSourcePoller::ID . '_specs' ); + add_filter( + DataSourcePoller::FILTER_NAME, + function() { + return array(); + } + ); + $specs = Init::get_specs(); + $defaults = DefaultMarketingRecommendations::get_all(); + remove_all_filters( DataSourcePoller::FILTER_NAME ); + $this->assertEquals( $defaults, $specs ); + } + + /** + * Test that specs are read from cache when they exist. + */ + public function test_specs_transient() { + set_transient( + 'woocommerce_admin_' . MarketingRecommendationsDataSourcePoller::ID . '_specs', + array( + 'en_US' => array( + array( + 'id' => 'mock1', + ), + array( + 'id' => 'mock2', + ), + ), + ) + ); + $specs = Init::get_specs(); + $this->assertCount( 2, $specs ); + } + + /** + * Test that recursive objects does not cause an error and returns null for the recursive child. + */ + public function test_matching_suggestions() { + $node1 = new \StdClass(); + $node2 = new \StdClass(); + $node1->child = $node2; + $node2->child = $node1; + $result = Init::object_to_array( $node1 ); + $this->assertEquals( array( 'child' => array( 'child' => null ) ), $result ); + } +}