From fe97d19c6691ec2c9ade9427f0ba8ed31ab58c20 Mon Sep 17 00:00:00 2001 From: kasimi Date: Tue, 11 Aug 2020 07:59:10 +0200 Subject: [PATCH 1/3] [ticket/16565] Lazy-load cron.task_collection PHPBB3-16565 --- phpBB/phpbb/cron/manager.php | 50 +++++++++++++++++++++++++++++++----- 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/phpBB/phpbb/cron/manager.php b/phpBB/phpbb/cron/manager.php index 59ee693074..9464b17e76 100644 --- a/phpBB/phpbb/cron/manager.php +++ b/phpBB/phpbb/cron/manager.php @@ -15,6 +15,7 @@ namespace phpbb\cron; use phpbb\cron\task\wrapper; use phpbb\routing\helper; +use Symfony\Component\DependencyInjection\ContainerInterface; /** * Cron manager class. @@ -23,6 +24,11 @@ use phpbb\routing\helper; */ class manager { + /** + * @var ContainerInterface + */ + protected $phpbb_container; + /** * @var helper */ @@ -34,7 +40,14 @@ class manager * * @var array */ - protected $tasks = array(); + protected $tasks = []; + + /** + * Flag indicating if $this->tasks contains tasks registered in the container + * + * @var bool + */ + protected $is_initialised_from_container = false; /** * @var string @@ -49,18 +62,17 @@ class manager /** * Constructor. Loads all available tasks. * - * @param array|\Traversable $tasks Provides an iterable set of task names + * @param ContainerInterface $phpbb_container Container * @param helper $routing_helper Routing helper * @param string $phpbb_root_path Relative path to phpBB root * @param string $php_ext PHP file extension */ - public function __construct($tasks, helper $routing_helper, $phpbb_root_path, $php_ext) + public function __construct(ContainerInterface $phpbb_container, helper $routing_helper, $phpbb_root_path, $php_ext) { + $this->phpbb_container = $phpbb_container; $this->routing_helper = $routing_helper; $this->phpbb_root_path = $phpbb_root_path; $this->php_ext = $php_ext; - - $this->load_tasks($tasks); } /** @@ -79,6 +91,24 @@ class manager } } + /** + * Loads registered tasks from the container, wraps them + * and puts them into $this->tasks. + * + * @return null + */ + public function load_tasks_from_container() + { + if (!$this->is_initialised_from_container) + { + $this->is_initialised_from_container = true; + + $tasks = $this->phpbb_container->get('cron.task_collection'); + + $this->load_tasks($tasks); + } + } + /** * Finds a task that is ready to run. * @@ -90,6 +120,8 @@ class manager */ public function find_one_ready_task() { + $this->load_tasks_from_container(); + shuffle($this->tasks); foreach ($this->tasks as $task) { @@ -108,7 +140,9 @@ class manager */ public function find_all_ready_tasks() { - $tasks = array(); + $this->load_tasks_from_container(); + + $tasks = []; foreach ($this->tasks as $task) { if ($task->is_ready()) @@ -131,6 +165,8 @@ class manager */ public function find_task($name) { + $this->load_tasks_from_container(); + foreach ($this->tasks as $task) { if ($task->get_name() == $name) @@ -148,6 +184,8 @@ class manager */ public function get_tasks() { + $this->load_tasks_from_container(); + return $this->tasks; } From 0daf6ccbcb394e738630fe207f1133e0d92b3759 Mon Sep 17 00:00:00 2001 From: kasimi Date: Wed, 12 Aug 2020 12:51:59 +0200 Subject: [PATCH 2/3] [ticket/16565] Fixed cron.manager tests PHPBB3-16565 --- tests/console/cron/cron_list_test.php | 6 +++- tests/console/cron/run_test.php | 20 +++++++++-- tests/controller/common_helper_route.php | 45 +++++++++++++++++++----- tests/cron/manager_test.php | 8 ++++- tests/pagination/pagination_test.php | 8 +++-- 5 files changed, 71 insertions(+), 16 deletions(-) diff --git a/tests/console/cron/cron_list_test.php b/tests/console/cron/cron_list_test.php index 8c7424c50d..9347f5cedd 100644 --- a/tests/console/cron/cron_list_test.php +++ b/tests/console/cron/cron_list_test.php @@ -102,7 +102,11 @@ class phpbb_console_command_cron_list_test extends phpbb_test_case $pathEx ); - $this->cron_manager = new \phpbb\cron\manager($tasks, $routing_helper, $phpbb_root_path, $pathEx); + $mock_container = new phpbb_mock_container_builder(); + $mock_container->set('cron.task_collection', []); + + $this->cron_manager = new \phpbb\cron\manager($mock_container, $routing_helper, $phpbb_root_path, $pathEx); + $this->cron_manager->load_tasks($tasks); } public function get_command_tester() diff --git a/tests/console/cron/run_test.php b/tests/console/cron/run_test.php index 8402f9dd3e..af01c5242e 100644 --- a/tests/console/cron/run_test.php +++ b/tests/console/cron/run_test.php @@ -78,7 +78,11 @@ class phpbb_console_command_cron_run_test extends phpbb_database_test_case $phpEx ); - $this->cron_manager = new \phpbb\cron\manager($tasks, $routing_helper, $phpbb_root_path, $phpEx); + $mock_container = new phpbb_mock_container_builder(); + $mock_container->set('cron.task_collection', []); + + $this->cron_manager = new \phpbb\cron\manager($mock_container, $routing_helper, $phpbb_root_path, $phpEx); + $this->cron_manager->load_tasks($tasks); $this->assertSame('0', $config['cron_lock']); } @@ -154,7 +158,12 @@ class phpbb_console_command_cron_run_test extends phpbb_database_test_case $phpEx ); - $this->cron_manager = new \phpbb\cron\manager($tasks, $routing_helper, $phpbb_root_path, $phpEx); + $mock_container = new phpbb_mock_container_builder(); + $mock_container->set('cron.task_collection', []); + + $this->cron_manager = new \phpbb\cron\manager($mock_container, $routing_helper, $phpbb_root_path, $phpEx); + $this->cron_manager->load_tasks($tasks); + $command_tester = $this->get_command_tester(); $exit_status = $command_tester->execute(array('command' => $this->command_name)); @@ -197,7 +206,12 @@ class phpbb_console_command_cron_run_test extends phpbb_database_test_case $phpEx ); - $this->cron_manager = new \phpbb\cron\manager($tasks, $routing_helper, $phpbb_root_path, $phpEx); + $mock_container = new phpbb_mock_container_builder(); + $mock_container->set('cron.task_collection', []); + + $this->cron_manager = new \phpbb\cron\manager($mock_container, $routing_helper, $phpbb_root_path, $phpEx); + $this->cron_manager->load_tasks($tasks); + $command_tester = $this->get_command_tester(); $exit_status = $command_tester->execute(array('command' => $this->command_name, '--verbose' => true)); diff --git a/tests/controller/common_helper_route.php b/tests/controller/common_helper_route.php index 6945ffb7e9..d29a631285 100644 --- a/tests/controller/common_helper_route.php +++ b/tests/controller/common_helper_route.php @@ -203,13 +203,16 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_ */ public function test_helper_url_no_rewrite($route, $params, $is_amp, $session_id, $expected, $description) { + $mock_container = new phpbb_mock_container_builder(); + $mock_container->set('cron.task_collection', []); + $this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '0')); $this->routing_helper = new \phpbb\routing\helper($this->config, $this->router, $this->symfony_request, $this->request, $this->filesystem, $this->root_path, 'php'); $this->helper = new phpbb_mock_controller_helper( $this->auth, $this->cache, $this->config, - new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), + new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php'), $this->db, $this->dispatcher, $this->language, @@ -263,13 +266,16 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_ */ public function test_helper_url_with_rewrite($route, $params, $is_amp, $session_id, $expected, $description) { + $mock_container = new phpbb_mock_container_builder(); + $mock_container->set('cron.task_collection', []); + $this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '1')); $this->routing_helper = new \phpbb\routing\helper($this->config, $this->router, $this->symfony_request, $this->request, $this->filesystem, $this->root_path, 'php'); $this->helper = new phpbb_mock_controller_helper( $this->auth, $this->cache, $this->config, - new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), + new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php'), $this->db, $this->dispatcher, $this->language, @@ -323,13 +329,16 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_ */ public function test_helper_url_absolute($route, $params, $is_amp, $session_id, $expected, $description) { + $mock_container = new phpbb_mock_container_builder(); + $mock_container->set('cron.task_collection', []); + $this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '0')); $this->routing_helper = new \phpbb\routing\helper($this->config, $this->router, $this->symfony_request, $this->request, $this->filesystem, $this->root_path, 'php'); $this->helper = new phpbb_mock_controller_helper( $this->auth, $this->cache, $this->config, - new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), + new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php'), $this->db, $this->dispatcher, $this->language, @@ -383,13 +392,16 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_ */ public function test_helper_url_relative_path($route, $params, $is_amp, $session_id, $expected, $description) { + $mock_container = new phpbb_mock_container_builder(); + $mock_container->set('cron.task_collection', []); + $this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '0')); $this->routing_helper = new \phpbb\routing\helper($this->config, $this->router, $this->symfony_request, $this->request, $this->filesystem, $this->root_path, 'php'); $this->helper = new phpbb_mock_controller_helper( $this->auth, $this->cache, $this->config, - new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), + new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php'), $this->db, $this->dispatcher, $this->language, @@ -443,13 +455,16 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_ */ public function test_helper_url_network($route, $params, $is_amp, $session_id, $expected, $description) { + $mock_container = new phpbb_mock_container_builder(); + $mock_container->set('cron.task_collection', []); + $this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '0')); $this->routing_helper = new \phpbb\routing\helper($this->config, $this->router, $this->symfony_request, $this->request, $this->filesystem, $this->root_path, 'php'); $this->helper = new phpbb_mock_controller_helper( $this->auth, $this->cache, $this->config, - new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), + new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php'), $this->db, $this->dispatcher, $this->language, @@ -503,13 +518,16 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_ */ public function test_helper_url_absolute_with_rewrite($route, $params, $is_amp, $session_id, $expected, $description) { + $mock_container = new phpbb_mock_container_builder(); + $mock_container->set('cron.task_collection', []); + $this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '1')); $this->routing_helper = new \phpbb\routing\helper($this->config, $this->router, $this->symfony_request, $this->request, $this->filesystem, $this->root_path, 'php'); $this->helper = new phpbb_mock_controller_helper( $this->auth, $this->cache, $this->config, - new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), + new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php'), $this->db, $this->dispatcher, $this->language, @@ -560,13 +578,16 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_ */ public function test_helper_url_relative_path_with_rewrite($route, $params, $is_amp, $session_id, $expected, $description) { + $mock_container = new phpbb_mock_container_builder(); + $mock_container->set('cron.task_collection', []); + $this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '1')); $this->routing_helper = new \phpbb\routing\helper($this->config, $this->router, $this->symfony_request, $this->request, $this->filesystem, $this->root_path, 'php'); $this->helper = new phpbb_mock_controller_helper( $this->auth, $this->cache, $this->config, - new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), + new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php'), $this->db, $this->dispatcher, $this->language, @@ -620,13 +641,16 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_ */ public function test_helper_url_network_with_rewrite($route, $params, $is_amp, $session_id, $expected, $description) { + $mock_container = new phpbb_mock_container_builder(); + $mock_container->set('cron.task_collection', []); + $this->config = new \phpbb\config\config(['enable_mod_rewrite' => '1']); $this->routing_helper = new \phpbb\routing\helper($this->config, $this->router, $this->symfony_request, $this->request, $this->filesystem, $this->root_path, 'php'); $this->helper = new phpbb_mock_controller_helper( $this->auth, $this->cache, $this->config, - new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), + new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php'), $this->db, $this->dispatcher, $this->language, @@ -669,12 +693,15 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_ 'server_protocol' => $server_protocol, )); + $mock_container = new phpbb_mock_container_builder(); + $mock_container->set('cron.task_collection', []); + $this->routing_helper = new \phpbb\routing\helper($this->config, $this->router, $this->symfony_request, $this->request, $this->filesystem, $this->root_path, 'php'); $this->helper = new phpbb_mock_controller_helper( $this->auth, $this->cache, $this->config, - new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), + new \phpbb\cron\manager($mock_container, $this->routing_helper, $this->root_path, 'php'), $this->db, $this->dispatcher, $this->language, diff --git a/tests/cron/manager_test.php b/tests/cron/manager_test.php index f025e38cd5..0dae404a2a 100644 --- a/tests/cron/manager_test.php +++ b/tests/cron/manager_test.php @@ -102,6 +102,12 @@ class phpbb_cron_manager_test extends \phpbb_test_case $phpEx ); - return new \phpbb\cron\manager($tasks, $routing_helper, $phpbb_root_path, $phpEx); + $mock_container = new phpbb_mock_container_builder(); + $mock_container->set('cron.task_collection', []); + + $cron_manager = new \phpbb\cron\manager($mock_container, $routing_helper, $phpbb_root_path, $phpEx); + $cron_manager->load_tasks($tasks); + + return $cron_manager; } } diff --git a/tests/pagination/pagination_test.php b/tests/pagination/pagination_test.php index 38d250958d..0b2c89f829 100644 --- a/tests/pagination/pagination_test.php +++ b/tests/pagination/pagination_test.php @@ -46,7 +46,11 @@ class phpbb_pagination_pagination_test extends phpbb_template_template_test_case new \phpbb\routing\file_locator($filesystem, dirname(__FILE__) . '/') ); $resources_locator = new \phpbb\routing\resources_locator\default_resources_locator(dirname(__FILE__) . '/', PHPBB_ENVIRONMENT, $manager); - $router = new phpbb_mock_router(new phpbb_mock_container_builder(), $resources_locator, $loader, dirname(__FILE__) . '/', 'php', false); + + $mock_container = new phpbb_mock_container_builder(); + $mock_container->set('cron.task_collection', []); + + $router = new phpbb_mock_router($mock_container, $resources_locator, $loader, dirname(__FILE__) . '/', 'php', false); $request = new phpbb_mock_request(); $request->overwrite('SCRIPT_NAME', '/app.php', \phpbb\request\request_interface::SERVER); @@ -66,7 +70,7 @@ class phpbb_pagination_pagination_test extends phpbb_template_template_test_case new \phpbb\auth\auth(), new \phpbb\cache\driver\dummy(), $this->config, - new \phpbb\cron\manager([], $this->routing_helper, '', 'php'), + new \phpbb\cron\manager($mock_container, $this->routing_helper, '', 'php'), $db, new phpbb_mock_event_dispatcher(), new \phpbb\language\language(new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx)), From 76f2305b84106148aaed27f8d635a5c8812eed0e Mon Sep 17 00:00:00 2001 From: kasimi Date: Wed, 12 Aug 2020 13:03:23 +0200 Subject: [PATCH 3/3] [ticket/16565] Inject service container PHPBB3-16565 --- phpBB/config/default/container/services_cron.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/config/default/container/services_cron.yml b/phpBB/config/default/container/services_cron.yml index 70f70e355d..44030c1750 100644 --- a/phpBB/config/default/container/services_cron.yml +++ b/phpBB/config/default/container/services_cron.yml @@ -2,7 +2,7 @@ services: cron.manager: class: phpbb\cron\manager arguments: - - '@cron.task_collection' + - '@service_container' - '@routing.helper' - '%core.root_path%' - '%core.php_ext%'