Merge pull request #6037 from kasimi/ticket/16565

[ticket/16565] Lazy-load cron.task_collection

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%'

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;
 	}
 

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()

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));
 

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,

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;
 	}
 }

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)),