diff --git a/inc/Engine/Preload/Controller/PreloadUrl.php b/inc/Engine/Preload/Controller/PreloadUrl.php index 551e63cfb1..a085e0ad8c 100755 --- a/inc/Engine/Preload/Controller/PreloadUrl.php +++ b/inc/Engine/Preload/Controller/PreloadUrl.php @@ -1,4 +1,5 @@ options->get( 'do_caching_mobile_files', false ); + if ( $this->is_already_cached( $url ) && ( ! $is_mobile || $this->is_already_cached( $url, true ) ) ) { $this->query->make_status_complete( $url ); return; } + // Should we perform a duration check? + $check_duration = ( false === get_transient( 'rocket_preload_check_duration' ) ) ? true : false; + $requests = [ [ 'url' => $url, @@ -117,6 +122,11 @@ public function preload_url( string $url ) { ] ); + if ( $check_duration ) { + $headers['blocking'] = true; + $headers['timeout'] = 20; + } + /** * Filters the arguments for the preload request. * @@ -131,19 +141,42 @@ public function preload_url( string $url ) { return; } + if ( $check_duration ) { + $start = microtime( true ); + } + wp_safe_remote_get( user_trailingslashit( $request['url'] ), $headers ); + + if ( $check_duration ) { + $duration = ( microtime( true ) - $start ); // Duration of the request. + } + /** * Filter the delay between each preload request. * * @param float $delay_between the defined delay. - * @returns float */ $delay_between = apply_filters( 'rocket_preload_delay_between_requests', 500000 ); usleep( $delay_between ); + + if ( ! $check_duration ) { + continue; + } + + $duration_transient = get_transient( 'rocket_preload_previous_requests_durations' ); + $average_duration = ( false !== $duration_transient ) ? $duration_transient : 0; + + // Update average duration. + $average_duration = ( $average_duration <= 0 ) ? $duration : ( $average_duration * 0.7 + $duration * 0.3 ); + + set_transient( 'rocket_preload_previous_requests_durations', $average_duration, 5 * MINUTE_IN_SECONDS ); + + set_transient( 'rocket_preload_check_duration', $duration, MINUTE_IN_SECONDS ); // Don't check request duration for 1 minute. + $check_duration = false; } } @@ -175,10 +208,37 @@ protected function get_mobile_user_agent_prefix() { * @return void */ public function process_pending_jobs() { - $pending_actions = $this->queue->get_pending_preload_actions(); - $count = ( (int) apply_filters( 'rocket_preload_cache_pending_jobs_cron_rows_count', 45 ) ) - count( $pending_actions ); + // Retrieve batch size limits and request timing estimation. + /** + * Get the number of pending cron job + * + * @param int $args Maximum number of job size. + */ + $max_batch_size = ( (int) apply_filters( 'rocket_preload_cache_pending_jobs_cron_rows_count', 45 ) ) - count( $pending_actions ); + + /** + * Get the number of in progress cron job + * + * @param int $args Minimum number of job size. + */ + $min_batch_size = ( (int) apply_filters( 'rocket_preload_cache_min_in_progress_jobs_count', 5 ) ); + + $average_duration = get_transient( 'rocket_preload_previous_requests_durations' ); + + /** + * Estimate batch size based on request duration. + * In case no estimation or there is an issue with the value use $min_batch_size. + */ + $next_batch_size = (int) ( ! $average_duration ? $min_batch_size : round( -5 * $average_duration + 55 ) ); + + // Limit next_batch_size. + $next_batch_size = max( $next_batch_size, $min_batch_size ); // Not lower than 5. + $next_batch_size = min( $next_batch_size, $max_batch_size ); // Not higher than 45. + $next_batch_size = max( $next_batch_size, 0 ); // Not lower than 0. + + // Get all in-progress jobs with request sent and no results. /** * Set the delay before an in-progress row is considered as outdated. * @@ -193,11 +253,11 @@ public function process_pending_jobs() { * @param int $count number of rows in batches. * @return int */ - (int) ( $count / 15 ) + (int) ( $max_batch_size / 15 ) ); - $stuck_rows = $this->query->get_outdated_in_progress_jobs( $delay ); + // Make sure the request has been sent for those jobs. $stuck_rows = array_filter( $stuck_rows, function ( $row ) use ( $pending_actions ) { @@ -210,10 +270,13 @@ function ( $row ) use ( $pending_actions ) { } ); + // Make those hanging jobs failed. foreach ( $stuck_rows as $row ) { - $this->query->make_status_failed( $row->id ); + $this->query->make_status_failed( (int) $row->id ); } - $rows = $this->query->get_pending_jobs( $count ); + + // Add new jobs in progress. + $rows = $this->query->get_pending_jobs( $next_batch_size ); foreach ( $rows as $row ) { if ( $this->is_excluded_by_filter( $row->url ) ) { @@ -221,7 +284,7 @@ function ( $row ) use ( $pending_actions ) { continue; } - $this->query->make_status_inprogress( $row->id ); + $this->query->make_status_inprogress( (int) $row->id ); $this->queue->add_job_preload_job_preload_url_async( $row->url ); } diff --git a/inc/Engine/WPRocketUninstall.php b/inc/Engine/WPRocketUninstall.php index 36f29647b5..87af2ea745 100644 --- a/inc/Engine/WPRocketUninstall.php +++ b/inc/Engine/WPRocketUninstall.php @@ -80,6 +80,8 @@ class WPRocketUninstall { 'wp_rocket_pricing_timeout', 'wp_rocket_pricing_timeout_active', 'rocket_get_refreshed_fragments_cache', + 'rocket_preload_previous_requests_durations', + 'rocket_preload_check_duration', 'wpr_user_information_timeout_active', 'wpr_user_information_timeout', ]; diff --git a/tests/Fixtures/inc/Engine/Preload/Controller/PreloadUrl/preloadUrl.php b/tests/Fixtures/inc/Engine/Preload/Controller/PreloadUrl/preloadUrl.php index 691d60c026..ec5904278a 100644 --- a/tests/Fixtures/inc/Engine/Preload/Controller/PreloadUrl/preloadUrl.php +++ b/tests/Fixtures/inc/Engine/Preload/Controller/PreloadUrl/preloadUrl.php @@ -1,7 +1,8 @@ [ + 'shouldDoNothingWhenAlreadyCached' => [ 'config' => [ + 'transient_check_duration' => 500, 'url' => 'url', 'cache_exists' => true, 'cache_mobile' => false, @@ -12,12 +13,13 @@ 'timeout' => 0.01, 'user-agent' => 'WP Rocket/Preload', 'sslverify' => false, - ] + ], ], - ] + ], ], - 'mobileNotActivatedShouldPreloadOnlyOnce' => [ + 'shouldPreloadOnlyOnceWhenMobileCacheDisabled' => [ 'config' => [ + 'transient_check_duration' => 500, 'url' => 'url', 'cache_exists' => false, 'cache_mobile' => false, @@ -28,12 +30,13 @@ 'timeout' => 0.01, 'user-agent' => 'WP Rocket/Preload', 'sslverify' => false, - ] + ], ], - ] + ], ], - 'mobileActivatedShouldPreloadTwice' => [ + 'ShouldPreloadTwiceWhenMobileCacheEnabled' => [ 'config' => [ + 'transient_check_duration' => 500, 'url' => 'url', 'cache_exists' => false, 'cache_mobile' => true, @@ -44,7 +47,7 @@ 'timeout' => 0.01, 'user-agent' => 'WP Rocket/Preload', 'sslverify' => false, - ] + ], ], 'request_mobile' => [ 'config' => [ @@ -52,8 +55,25 @@ 'timeout' => 0.01, 'user-agent' => 'user_agent', 'sslverify' => false, - ] + ], ], - ] - ] + ], + ], + 'shouldPreloadOnlyOnceWhenMobileCacheDisabledAndCheckDuration' => [ + 'config' => [ + 'transient_check_duration' => false, + 'url' => 'url', + 'cache_exists' => false, + 'cache_mobile' => false, + 'user_agent' => 'user_agent', + 'request' => [ + 'config' => [ + 'blocking' => true, + 'timeout' => 20, + 'user-agent' => 'WP Rocket/Preload', + 'sslverify' => false, + ], + ], + ], + ], ]; diff --git a/tests/Fixtures/inc/Engine/Preload/Controller/PreloadUrl/processPendingJobs.php b/tests/Fixtures/inc/Engine/Preload/Controller/PreloadUrl/processPendingJobs.php index 6d97f12d37..011a079288 100644 --- a/tests/Fixtures/inc/Engine/Preload/Controller/PreloadUrl/processPendingJobs.php +++ b/tests/Fixtures/inc/Engine/Preload/Controller/PreloadUrl/processPendingJobs.php @@ -41,34 +41,35 @@ 'shouldPassJobsInPending' => [ 'config' => [ 'outdated_jobs' => [ - $outdated_row + $outdated_row, ], 'excluded' => [ - false, - false, - true, + false, + false, + true, ], - 'rows' => 101, + 'rows' => 5, 'jobs' => [ $row1, $row2, - $row3 - ] + $row3, + ], + 'transient' => 0, ], 'expected' => [ 'outdated_jobs_id' => [ - [14] + [ 14 ], ], 'job_ids' => [ - [10], - [11], + [ 10 ], + [ 11 ], ], 'job_urls' => [ 'http://example1', 'http://example2', ], 'job_deleted' => - ['http://example3'], - ] + [ 'http://example3' ], + ], ], ]; diff --git a/tests/Fixtures/inc/Engine/Preload/Cron/Subscriber/processPendingUrls.php b/tests/Fixtures/inc/Engine/Preload/Cron/Subscriber/processPendingUrls.php index 69225f2e07..d364f77779 100644 --- a/tests/Fixtures/inc/Engine/Preload/Cron/Subscriber/processPendingUrls.php +++ b/tests/Fixtures/inc/Engine/Preload/Cron/Subscriber/processPendingUrls.php @@ -106,7 +106,7 @@ ], 'InProgressShouldNotExceedMaxQueue' => [ 'config' => [ - 'rocket_preload_cache_pending_jobs_cron_rows_count' => 3, + 'rocket_preload_cache_pending_jobs_cron_rows_count' => 5, 'manual_preload' => true, 'rocket_preload_outdated' => 1000, 'rows' => [ @@ -120,7 +120,7 @@ ], [ 'url' => 'http://example.org/test3', - 'status' => 'pending' + 'status' => 'in-progress' ], [ 'url' => 'http://example.org/test4', @@ -147,11 +147,11 @@ ], [ 'url' => 'http://example.org/test3', - 'status' => 'pending' + 'status' => 'in-progress' ], [ 'url' => 'http://example.org/test4', - 'status' => 'pending' + 'status' => 'in-progress' ], [ 'url' => 'http://example.org/test5', @@ -181,7 +181,7 @@ ], ], [ - 'exists' => false, + 'exists' => true, 'args' => [ 'hook' => 'rocket_preload_job_preload_url', 'args' => ['http://example.org/test4'] diff --git a/tests/Integration/inc/Engine/WPRocketUninstall/uninstall.php b/tests/Integration/inc/Engine/WPRocketUninstall/uninstall.php index 8bf3c1f7ba..1e88e42d13 100644 --- a/tests/Integration/inc/Engine/WPRocketUninstall/uninstall.php +++ b/tests/Integration/inc/Engine/WPRocketUninstall/uninstall.php @@ -45,6 +45,8 @@ class Test_Uninstall extends FilesystemTestCase { 'rocket_preload_complete' => null, 'rocket_preload_complete_time' => null, 'rocket_preload_errors' => null, + 'rocket_preload_previous_requests_durations' => null, + 'rocket_preload_check_duration' => null, 'rocket_database_optimization_process' => null, 'rocket_database_optimization_process_complete' => null, 'rocket_hide_deactivation_form' => null, diff --git a/tests/Unit/inc/Engine/Preload/Controller/PreloadUrl/preloadUrl.php b/tests/Unit/inc/Engine/Preload/Controller/PreloadUrl/preloadUrl.php index abc2eee2bf..ae26f31e89 100644 --- a/tests/Unit/inc/Engine/Preload/Controller/PreloadUrl/preloadUrl.php +++ b/tests/Unit/inc/Engine/Preload/Controller/PreloadUrl/preloadUrl.php @@ -1,70 +1,110 @@ options = Mockery::mock(Options_Data::class); - $this->query = $this->createMock(Cache::class); - $this->queue = Mockery::mock(Queue::class); - $this->file_system = Mockery::mock(WP_Filesystem_Direct::class); - $this->controller = Mockery::mock(PreloadUrl::class . '[get_mobile_user_agent_prefix,is_already_cached]', - [$this->options, - $this->queue, $this->query, $this->file_system])->shouldAllowMockingProtectedMethods(); + + $this->options = Mockery::mock( Options_Data::class ); + $this->query = $this->createMock( Cache::class ); + $this->queue = Mockery::mock( Queue::class ); + $this->file_system = Mockery::mock( WP_Filesystem_Direct::class ); + $this->controller = Mockery::mock( PreloadUrl::class . '[get_mobile_user_agent_prefix,is_already_cached]', + [ + $this->options, + $this->queue, + $this->query, + $this->file_system, + ] + )->shouldAllowMockingProtectedMethods(); } /** * @dataProvider configTestData */ - public function testShouldDoAsExpected($config) { - $this->options->expects()->get('do_caching_mobile_files', false)->andReturn($config['cache_mobile']); - $this->controller->expects()->is_already_cached($config['url'])->andReturn($config['cache_exists']); - $this->configureRequest($config); - $this->configureMobileRequest($config); - $this->controller->preload_url($config['url']); - } + public function testShouldDoAsExpected( $config ) { + Functions\expect( 'get_transient' ) + ->atMost() + ->once() + ->with( 'rocket_preload_check_duration' ) + ->andReturn( $config['transient_check_duration'] ); - protected function configureRequest($config) { - if($config['cache_exists']) { - return; + $this->options->expects()->get( 'do_caching_mobile_files', false ) + ->andReturn( $config['cache_mobile'] ); + $this->controller->expects()->is_already_cached( $config['url'] ) + ->andReturn( $config['cache_exists'] ); + + $this->expectDesktopRequest( $config ); + $this->expectMobileRequest( $config ); + + if ( false === $config['transient_check_duration'] ) { + Functions\expect( 'get_transient' ) + ->atMost() + ->once() + ->with( 'rocket_preload_previous_requests_durations' ) + ->andReturn( false ); + + Functions\expect( 'set_transient' ) + ->atMost() + ->once() + ->with( 'rocket_preload_previous_requests_durations', Mockery::type( 'int' ), 300 ); + + Functions\expect( 'set_transient' ) + ->atMost() + ->once() + ->with( 'rocket_preload_check_duration', Mockery::type( 'int' ), 60 ); } - Functions\expect('wp_safe_remote_get')->with($config['url'] . '/', $config['request']['config']); + $this->controller->preload_url( $config['url'] ); } - protected function configureMobileRequest($config) { - if($config['cache_exists']) { + protected function expectDesktopRequest( $config ) { + if ( $config['cache_exists'] ) { + Functions\expect( 'wp_safe_remote_get' ) + ->with( $config['url'] . '/', $config['request']['config'] ) + ->never(); + return; } - if(! $config['cache_mobile']) { + Functions\expect( 'wp_safe_remote_get' ) + ->with( $config['url'] . '/', $config['request']['config'] ); + } + + protected function expectMobileRequest( $config ) { + if ( + $config['cache_exists'] + || + ! $config['cache_mobile'] + ) { return; } - $this->controller->expects()->get_mobile_user_agent_prefix()->andReturn($config['user_agent']); - Functions\expect('wp_safe_remote_get')->with($config['url'] . '/', $config['request_mobile']['config']); + + $this->controller->expects()->get_mobile_user_agent_prefix() + ->andReturn( $config['user_agent'] ); + + Functions\expect( 'wp_safe_remote_get' ) + ->with( $config['url'] . '/', $config['request_mobile']['config'] ); } } diff --git a/tests/Unit/inc/Engine/Preload/Controller/PreloadUrl/processPendingJobs.php b/tests/Unit/inc/Engine/Preload/Controller/PreloadUrl/processPendingJobs.php index 80601709f8..a2df1ebe50 100644 --- a/tests/Unit/inc/Engine/Preload/Controller/PreloadUrl/processPendingJobs.php +++ b/tests/Unit/inc/Engine/Preload/Controller/PreloadUrl/processPendingJobs.php @@ -1,54 +1,77 @@ options = Mockery::mock(Options_Data::class); - $this->query = $this->createMock(Cache::class); - $this->queue = Mockery::mock(Queue::class); - $this->file_system = Mockery::mock(WP_Filesystem_Direct::class); - $this->controller = Mockery::mock( PreloadUrl::class . '[is_excluded_by_filter]' , [$this->options, $this->queue, $this->query, $this->file_system])->shouldAllowMockingProtectedMethods(); + + $this->options = Mockery::mock( Options_Data::class ); + $this->query = $this->createMock( Cache::class ); + $this->queue = Mockery::mock( Queue::class ); + $this->file_system = Mockery::mock( WP_Filesystem_Direct::class ); + $this->controller = Mockery::mock( PreloadUrl::class . '[is_excluded_by_filter]', [ $this->options, $this->queue, $this->query, $this->file_system ] )->shouldAllowMockingProtectedMethods(); } /** * @dataProvider configTestData */ - public function testShouldDoAsExpected($config, $expected) { - - $this->queue->expects()->get_pending_preload_actions()->andReturn([]); - Filters\expectApplied('rocket_preload_cache_pending_jobs_cron_rows_count')->with(100)->andReturn($config['rows']); - $this->query->expects(self::once())->method( 'get_outdated_in_progress_jobs' )->with()->willReturn($config['outdated_jobs']); - $this->query->expects(self::atLeastOnce())->method('make_status_failed')->withConsecutive(...$expected['outdated_jobs_id']); - $this->query->expects(self::once())->method('get_pending_jobs')->with($config['rows'])->willReturn($config['jobs']); - $this->query->expects(self::atLeastOnce())->method('make_status_inprogress')->withConsecutive(...$expected['job_ids']); - $this->query->expects(self::atLeast(0))->method('delete_by_url')->withConsecutive($expected['job_deleted']); - $this->controller->shouldReceive('is_excluded_by_filter')->zeroOrMoreTimes()->andReturnValues($config['excluded']); - foreach ($expected['job_urls'] as $url) { - $this->queue->expects()->add_job_preload_job_preload_url_async( $url ); + public function testShouldDoAsExpected( $config, $expected ) { + Functions\when( 'get_transient' ) + ->justReturn( $config['transient'] ); + + $this->queue->expects()->get_pending_preload_actions() + ->andReturn( [] ); + + Filters\expectApplied( 'rocket_preload_cache_pending_jobs_cron_rows_count' ) + ->with( 100 ) + ->andReturn( $config['rows'] ); + + $this->query->expects( self::once() ) + ->method( 'get_outdated_in_progress_jobs' ) + ->with() + ->willReturn( $config['outdated_jobs'] ); + $this->query->expects( self::atLeastOnce() ) + ->method( 'make_status_failed' ) + ->withConsecutive( ...$expected['outdated_jobs_id'] ); + $this->query->expects( self::once() ) + ->method( 'get_pending_jobs' ) + ->with( $config['rows'] ) + ->willReturn( $config['jobs'] ); + $this->query->expects( self::atLeastOnce() ) + ->method( 'make_status_inprogress' ) + ->withConsecutive( ...$expected['job_ids'] ); + $this->query->expects( self::atLeast( 0 ) ) + ->method( 'delete_by_url' ) + ->withConsecutive( $expected['job_deleted'] ); + + $this->controller->shouldReceive( 'is_excluded_by_filter' ) + ->zeroOrMoreTimes() + ->andReturnValues( $config['excluded'] ); + + foreach ( $expected['job_urls'] as $url ) { + $this->queue->expects() + ->add_job_preload_job_preload_url_async( $url ); } $this->controller->process_pending_jobs();