From 7ba09e60c6220614b090b288451903284df2436e Mon Sep 17 00:00:00 2001 From: Opeyemi Ibrahim Date: Wed, 21 Aug 2024 09:31:31 +0100 Subject: [PATCH 1/9] Add period check and environment check to wamr_up methods :closes: #6850 --- .../PerformanceHints/WarmUp/Controller.php | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/inc/Engine/Common/PerformanceHints/WarmUp/Controller.php b/inc/Engine/Common/PerformanceHints/WarmUp/Controller.php index 5bffd9b05d..4a4e7a3c74 100644 --- a/inc/Engine/Common/PerformanceHints/WarmUp/Controller.php +++ b/inc/Engine/Common/PerformanceHints/WarmUp/Controller.php @@ -66,6 +66,14 @@ public function __construct( array $factories, Options_Data $options, APIClient * @return void */ public function warm_up_home(): void { + if ( 'local' === wp_get_environment_type() ) { + return; + } + + if ( $this->user->is_license_expired_grace_period() ) { + return; + } + if ( (bool) $this->options->get( 'remove_unused_css', 0 ) ) { return; } @@ -88,6 +96,10 @@ public function warm_up(): void { return; } + if ( $this->user->is_license_expired_grace_period() ) { + return; + } + if ( (bool) $this->options->get( 'remove_unused_css', 0 ) ) { return; } @@ -109,10 +121,6 @@ public function warm_up(): void { * @return array */ public function fetch_links(): array { - if ( $this->user->is_license_expired_grace_period() ) { - return []; - } - $user_agent = 'WP Rocket/Pre-fetch Home Links Mozilla/5.0 (iPhone; CPU iPhone OS 9_1 like Mac OS X) AppleWebKit/601.1.46 (KHTML, like Gecko) Version/9.0 Mobile/13B143 Safari/601.1'; $home_url = home_url(); From 6a6f33474550b91fcb758aa6a231986b71422910 Mon Sep 17 00:00:00 2001 From: Opeyemi Ibrahim Date: Wed, 21 Aug 2024 10:27:27 +0100 Subject: [PATCH 2/9] Modify tests --- .../WarmUp/Controller/fetchLinks.php | 15 --------------- .../WarmUp/Subscriber/warmUp.php | 17 +++++++++++++++++ .../WarmUp/Subscriber/warmUp.php | 7 +++++-- .../WarmUp/Controller/fetchLinks.php | 3 --- 4 files changed, 22 insertions(+), 20 deletions(-) diff --git a/tests/Fixtures/inc/Engine/Common/PerformanceHints/WarmUp/Controller/fetchLinks.php b/tests/Fixtures/inc/Engine/Common/PerformanceHints/WarmUp/Controller/fetchLinks.php index b87b5b1d41..0abf3937fb 100644 --- a/tests/Fixtures/inc/Engine/Common/PerformanceHints/WarmUp/Controller/fetchLinks.php +++ b/tests/Fixtures/inc/Engine/Common/PerformanceHints/WarmUp/Controller/fetchLinks.php @@ -11,21 +11,6 @@ $html_with_rss_feed_rest_api = 'RSS FeedRest APIHello World 4Hello World 5Hello World 6Hello World 7Hello World 8Hello World 9Rich Dad Poor DadBuy (He came to set the captives free) - Rebecca BrownHome'; return [ - 'shouldReturnEmptyWhenLicenseExpired' => [ - 'config' => [ - 'license_expired' => true, - 'headers' => [ - 'user-agent' => 'WP Rocket/Pre-fetch Home Links Mozilla/5.0 (iPhone; CPU iPhone OS 9_1 like Mac OS X) AppleWebKit/601.1.46 (KHTML, like Gecko) Version/9.0 Mobile/13B143 Safari/601.1', - 'timeout' => 60, - ], - 'response' => [ - 'response' => [ - 'code' => 500, - ], - ], - ], - 'expected' => [], - ], 'shouldReturnEmptyWhenNot200' => [ 'config' => [ 'license_expired' => false, diff --git a/tests/Fixtures/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUp.php b/tests/Fixtures/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUp.php index 892aaad7b6..5c14bb2383 100644 --- a/tests/Fixtures/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUp.php +++ b/tests/Fixtures/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUp.php @@ -5,6 +5,7 @@ 'wp_env' => 'production', 'remove_unused_css' => 0, 'is_allowed' => [1], + 'license_expired' => false, 'links' => [ 'http://example.com/link1', 'http://example.com/link2', @@ -12,11 +13,25 @@ ], 'expected' => 2, ], + 'testShouldNotCallSendToSaasWhenLicenseExpired' => [ + 'config' => [ + 'wp_env' => 'production', + 'remove_unused_css' => 0, + 'is_allowed' => [1], + 'license_expired' => true, + 'links' => [ + 'http://example.com/link1', + 'http://example.com/link2', + ], + ], + 'expected' => 0, + ], 'testShouldNotCallSendToSaasWhenLocalEnv' => [ 'config' => [ 'wp_env' => 'local', 'remove_unused_css' => 0, 'is_allowed' => [1], + 'license_expired' => false, 'links' => [ 'http://example.com/link1', 'http://example.com/link2', @@ -28,6 +43,7 @@ 'config' => [ 'wp_env' => 'production', 'remove_unused_css' => 1, + 'license_expired' => false, 'is_allowed' => [1], 'links' => [ 'http://example.com/link1', @@ -40,6 +56,7 @@ 'config' => [ 'wp_env' => 'production', 'remove_unused_css' => 0, + 'license_expired' => false, 'is_allowed' => [], 'links' => [ 'http://example.com/link1', diff --git a/tests/Integration/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUp.php b/tests/Integration/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUp.php index f8ba9d85ab..436dce0f71 100644 --- a/tests/Integration/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUp.php +++ b/tests/Integration/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUp.php @@ -39,16 +39,19 @@ public function testShouldDoExpected( $config, $expected ) { $options->shouldReceive('get') ->with('remove_unused_css', 0) ->andReturn($config['remove_unused_css']); + + $user->shouldReceive( 'is_license_expired_grace_period' ) + ->once() + ->andReturn( $config['license_expired'] ); } $queue->shouldReceive('add_job_warmup_url') ->times($expected); - add_action('rocket_job_warmup', [$controller, 'warm_up']); do_action('rocket_job_warmup'); - + remove_action('rocket_job_warmup', [$controller, 'warm_up']); } } diff --git a/tests/Unit/inc/Engine/Common/PerformanceHints/WarmUp/Controller/fetchLinks.php b/tests/Unit/inc/Engine/Common/PerformanceHints/WarmUp/Controller/fetchLinks.php index b8786206b6..b4817a5a01 100644 --- a/tests/Unit/inc/Engine/Common/PerformanceHints/WarmUp/Controller/fetchLinks.php +++ b/tests/Unit/inc/Engine/Common/PerformanceHints/WarmUp/Controller/fetchLinks.php @@ -33,9 +33,6 @@ protected function setUp(): void { * @dataProvider configTestData */ public function testShouldReturnExpected( $config, $expected ) { - $this->user->shouldReceive( 'is_license_expired_grace_period' ) - ->once() - ->andReturn( $config['license_expired'] ); Functions\when( 'home_url' )->alias( function ( $link = '' ) { From ebcbbb371b45764ebf60bdda7e899e9ecb9a2a44 Mon Sep 17 00:00:00 2001 From: Opeyemi Ibrahim Date: Wed, 21 Aug 2024 11:00:58 +0100 Subject: [PATCH 3/9] Add test for home page warm up --#6850 --- .../WarmUp/Subscriber/warmUpHome.php | 53 +++++++++++++++++ .../WarmUp/Subscriber/warmUpHome.php | 57 +++++++++++++++++++ 2 files changed, 110 insertions(+) create mode 100644 tests/Fixtures/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUpHome.php create mode 100644 tests/Integration/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUpHome.php diff --git a/tests/Fixtures/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUpHome.php b/tests/Fixtures/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUpHome.php new file mode 100644 index 0000000000..1c18725374 --- /dev/null +++ b/tests/Fixtures/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUpHome.php @@ -0,0 +1,53 @@ + [ + 'config' => [ + 'wp_env' => 'production', + 'remove_unused_css' => 0, + 'factories' => [1], + 'license_expired' => false, + 'home_url' => 'http://example.com/', + ], + 'expected' => 1, + ], + 'testShouldNotCallSendToSaasWhenLicenseExpired' => [ + 'config' => [ + 'wp_env' => 'production', + 'remove_unused_css' => 0, + 'factories' => [1], + 'license_expired' => true, + 'home_url' => 'http://example.com/', + ], + 'expected' => 0, + ], + 'testShouldNotCallSendToSaasWhenLocalEnv' => [ + 'config' => [ + 'wp_env' => 'local', + 'remove_unused_css' => 0, + 'factories' => [1], + 'license_expired' => false, + 'home_url' => 'http://example.com/', + ], + 'expected' => 0, + ], + 'testShouldNotCallSendToSaasWhenRemoveUnusedCssEnabled' => [ + 'config' => [ + 'wp_env' => 'production', + 'remove_unused_css' => 1, + 'license_expired' => false, + 'factories' => [1], + 'home_url' => 'http://example.com/', + ], + 'expected' => 0, + ], + 'testShouldNotCallSendToSaasWhenFactoriesAreEmpty' => [ + 'config' => [ + 'wp_env' => 'production', + 'remove_unused_css' => 0, + 'license_expired' => false, + 'factories' => [], + 'home_url' => 'http://example.com/', + ], + 'expected' => 0, + ], +]; diff --git a/tests/Integration/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUpHome.php b/tests/Integration/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUpHome.php new file mode 100644 index 0000000000..5fb64510ef --- /dev/null +++ b/tests/Integration/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUpHome.php @@ -0,0 +1,57 @@ +makePartial(); + + $controller->shouldReceive('send_to_saas') + ->andReturn($config['home_url']); + + Functions\expect( 'wp_get_environment_type' )->andReturn($config['wp_env']); + + $queue->shouldReceive('add_job_warmup') + ->times($expected); + + if ( 'local' !== $config['wp_env'] ) { + $options->shouldReceive('get') + ->with('remove_unused_css', 0) + ->andReturn($config['remove_unused_css']); + + $user->shouldReceive( 'is_license_expired_grace_period' ) + ->once() + ->andReturn( $config['license_expired'] ); + } + + add_action('rocket_after_clear_performance_hints_data', [$controller, 'warm_up_home']); + + do_action('rocket_after_clear_performance_hints_data'); + + remove_action('rocket_after_clear_performance_hints_data', [$controller, 'warm_up_home']); + } +} From 196cc465a838700eea58150d71e6553727ef361e Mon Sep 17 00:00:00 2001 From: Opeyemi Ibrahim Date: Thu, 22 Aug 2024 11:01:46 +0100 Subject: [PATCH 4/9] Add to general test --- .../Common/PerformanceHints/WarmUp/Subscriber/warmUpHome.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/Integration/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUpHome.php b/tests/Integration/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUpHome.php index 5fb64510ef..42a6b454f0 100644 --- a/tests/Integration/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUpHome.php +++ b/tests/Integration/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUpHome.php @@ -14,10 +14,8 @@ /** * Test class covering \WP_Rocket\Engine\Common\PerformanceHints\WarmUp\Subscriber::warm_up_home * - * @group PerformanceHints - * @group WarmUp */ -class Test_warmUpHome extends TestCase { +class Test_WarmUpHome extends TestCase { /** * Test should do expected. * From 826789b950d838f5292855afa66032b88cce5c94 Mon Sep 17 00:00:00 2001 From: Opeyemi Ibrahim Date: Thu, 22 Aug 2024 11:50:04 +0100 Subject: [PATCH 5/9] revert test group changes --- .../Common/PerformanceHints/WarmUp/Subscriber/warmUpHome.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/Integration/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUpHome.php b/tests/Integration/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUpHome.php index 42a6b454f0..df9703bc1c 100644 --- a/tests/Integration/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUpHome.php +++ b/tests/Integration/inc/Engine/Common/PerformanceHints/WarmUp/Subscriber/warmUpHome.php @@ -14,6 +14,8 @@ /** * Test class covering \WP_Rocket\Engine\Common\PerformanceHints\WarmUp\Subscriber::warm_up_home * + * @group PerformanceHints + * @group WarmUp */ class Test_WarmUpHome extends TestCase { /** From 76d0b1a2d6f729e7d087afc9095e172966244eb5 Mon Sep 17 00:00:00 2001 From: Opeyemi Ibrahim Date: Thu, 22 Aug 2024 14:36:10 +0100 Subject: [PATCH 6/9] Modify logic check --- .../PerformanceHints/WarmUp/Controller.php | 33 +++++++++---------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/inc/Engine/Common/PerformanceHints/WarmUp/Controller.php b/inc/Engine/Common/PerformanceHints/WarmUp/Controller.php index 4a4e7a3c74..fd77f7dd62 100644 --- a/inc/Engine/Common/PerformanceHints/WarmUp/Controller.php +++ b/inc/Engine/Common/PerformanceHints/WarmUp/Controller.php @@ -60,21 +60,26 @@ public function __construct( array $factories, Options_Data $options, APIClient $this->queue = $queue; } + /** + * Should terminate early if true. + * + * @return bool + */ + private function should_terminate_early(): bool { + return ( + 'local' === wp_get_environment_type() || + $this->user->is_license_expired_grace_period() || + (bool) $this->options->get( 'remove_unused_css', 0 ) + ); + } + /** * Send home URL for warm up. * * @return void */ public function warm_up_home(): void { - if ( 'local' === wp_get_environment_type() ) { - return; - } - - if ( $this->user->is_license_expired_grace_period() ) { - return; - } - - if ( (bool) $this->options->get( 'remove_unused_css', 0 ) ) { + if ( $this->should_terminate_early() ) { return; } @@ -92,15 +97,7 @@ public function warm_up_home(): void { * @return void */ public function warm_up(): void { - if ( 'local' === wp_get_environment_type() ) { - return; - } - - if ( $this->user->is_license_expired_grace_period() ) { - return; - } - - if ( (bool) $this->options->get( 'remove_unused_css', 0 ) ) { + if ( $this->should_terminate_early() ) { return; } From 58be56f07c6f3b22fa12aae636f5e9826fe9c0d2 Mon Sep 17 00:00:00 2001 From: Opeyemi Ibrahim Date: Fri, 23 Aug 2024 10:10:51 +0100 Subject: [PATCH 7/9] Changed method name --- inc/Engine/Common/PerformanceHints/WarmUp/Controller.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/inc/Engine/Common/PerformanceHints/WarmUp/Controller.php b/inc/Engine/Common/PerformanceHints/WarmUp/Controller.php index fd77f7dd62..57c23d34c4 100644 --- a/inc/Engine/Common/PerformanceHints/WarmUp/Controller.php +++ b/inc/Engine/Common/PerformanceHints/WarmUp/Controller.php @@ -65,7 +65,7 @@ public function __construct( array $factories, Options_Data $options, APIClient * * @return bool */ - private function should_terminate_early(): bool { + private function is_allowed(): bool { return ( 'local' === wp_get_environment_type() || $this->user->is_license_expired_grace_period() || @@ -79,7 +79,7 @@ private function should_terminate_early(): bool { * @return void */ public function warm_up_home(): void { - if ( $this->should_terminate_early() ) { + if ( $this->is_allowed() ) { return; } @@ -97,7 +97,7 @@ public function warm_up_home(): void { * @return void */ public function warm_up(): void { - if ( $this->should_terminate_early() ) { + if ( $this->is_allowed() ) { return; } From f3e2f16dcab2a0cab8ff2de040211ac238f48b25 Mon Sep 17 00:00:00 2001 From: Opeyemi Ibrahim Date: Fri, 23 Aug 2024 10:48:01 +0100 Subject: [PATCH 8/9] invert method return for clear understanding --- inc/Engine/Common/PerformanceHints/WarmUp/Controller.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/inc/Engine/Common/PerformanceHints/WarmUp/Controller.php b/inc/Engine/Common/PerformanceHints/WarmUp/Controller.php index 57c23d34c4..1e22faee92 100644 --- a/inc/Engine/Common/PerformanceHints/WarmUp/Controller.php +++ b/inc/Engine/Common/PerformanceHints/WarmUp/Controller.php @@ -66,7 +66,7 @@ public function __construct( array $factories, Options_Data $options, APIClient * @return bool */ private function is_allowed(): bool { - return ( + return !( 'local' === wp_get_environment_type() || $this->user->is_license_expired_grace_period() || (bool) $this->options->get( 'remove_unused_css', 0 ) @@ -79,7 +79,7 @@ private function is_allowed(): bool { * @return void */ public function warm_up_home(): void { - if ( $this->is_allowed() ) { + if ( ! $this->is_allowed() ) { return; } @@ -97,7 +97,7 @@ public function warm_up_home(): void { * @return void */ public function warm_up(): void { - if ( $this->is_allowed() ) { + if ( ! $this->is_allowed() ) { return; } From 13417d7add58edce2277a58de3587ae0d26b5a62 Mon Sep 17 00:00:00 2001 From: Opeyemi Ibrahim Date: Fri, 23 Aug 2024 10:50:36 +0100 Subject: [PATCH 9/9] fix lint error --- inc/Engine/Common/PerformanceHints/WarmUp/Controller.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/inc/Engine/Common/PerformanceHints/WarmUp/Controller.php b/inc/Engine/Common/PerformanceHints/WarmUp/Controller.php index 1e22faee92..73019b2517 100644 --- a/inc/Engine/Common/PerformanceHints/WarmUp/Controller.php +++ b/inc/Engine/Common/PerformanceHints/WarmUp/Controller.php @@ -66,7 +66,7 @@ public function __construct( array $factories, Options_Data $options, APIClient * @return bool */ private function is_allowed(): bool { - return !( + return ! ( 'local' === wp_get_environment_type() || $this->user->is_license_expired_grace_period() || (bool) $this->options->get( 'remove_unused_css', 0 )