[feature/extension-manager] Porting cron tasks over to the extension finder

PHPBB3-10323

phpBB/includes/cron/manager.php

@@ -33,126 +33,53 @@ class phpbb_cron_manager
 	protected $tasks = array();
 
 	/**
-	* Path to the root of directory tree with tasks.
+	* An extension manager to search for cron tasks in extensions.
-	* For bundled phpBB tasks, this is the path to includes/cron/tasks
+	* @var phpbb_extension_manager
-	* under phpBB root.
-	* @var string
 	*/
-	protected $task_path;
+	protected $extension_manager;
-
-	/**
-	* PHP file extension
-	* @var string
-	*/
-	protected $phpEx;
-
-	/**
-	* Cache driver
-	* @var phpbb_cache_driver_interface
-	*/
-	protected $cache;
 
 	/**
 	* Constructor. Loads all available tasks.
 	*
-	* Tasks will be looked up in directory tree rooted at $task_path.
+	* Tasks will be looked up in the core task directory located in
-	* Task classes will be autoloaded and must be named according to
+	* includes/cron/task/core/ and in extensions. Task classes will be
-	* autoloading naming conventions. To load cron tasks shipped with
+	* autoloaded and must be named according to autoloading naming conventions.
-	* phpbb, pass $phpbb_root_path . 'includes/cron/task' as $task_path.
 	*
-	* If $cache is given, names of found cron tasks will be cached in it
+	* Tasks in extensions must be located in a directory called cron or a subdir
-	* for one hour. Note that the cron task names are stored without
+	* of a directory called cron. The class and filename must end in a _task
-	* namespacing; if two different phbb_cron_manager instances are
+	* suffix.
-	* constructed with different $task_path arguments but the same $cache,
-	* the second instance will use task names found by the first instance.
 	*
-	* @param string $task_path                   Directory containing cron tasks
+	* @param phpbb_extension_manager $extension_manager phpBB extension manager
-	* @param string $phpEx                       PHP file extension
-	* @param phpbb_cache_driver_interface $cache Cache for task names (optional)
-	* @return void
 	*/
-	public function __construct($task_path, $phpEx, phpbb_cache_driver_interface $cache = null)
+	public function __construct(phpbb_extension_manager $extension_manager)
 	{
-		if (DIRECTORY_SEPARATOR != '/')
+		$this->extension_manager = $extension_manager;
-		{
-			// Need this on some platforms since the code elsewhere uses /
-			// to separate directory components, but PHP iterators return
-			// paths with platform-specific directory separators.
-			$task_path = str_replace('/', DIRECTORY_SEPARATOR, $task_path);
-		}
-
-		$this->task_path = $task_path;
-		$this->phpEx = $phpEx;
-		$this->cache = $cache;
 
 		$task_names = $this->find_cron_task_names();
 		$this->load_tasks($task_names);
 	}
 
 	/**
-	* Finds cron task names.
+	* Finds cron task names using the extension manager.
 	*
-	* A cron task file must follow the naming convention:
+	* All PHP files in includes/cron/task/core/ are considered tasks. Tasks
-	* includes/cron/task/$mod/$name.php.
+	* in extensions have to be located in a directory called cron or a subdir
-	* $mod is core for tasks that are part of phpbb.
+	* of a directory called cron. The class and filename must end in a _task
-	* Modifications should use their name as $mod.
+	* suffix.
-	* $name is the name of the cron task.
-	* Cron task is expected to be a class named phpbb_cron_task_${mod}_${name}.
 	*
 	* @return array		List of task names
 	*/
 	public function find_cron_task_names()
 	{
-		if ($this->cache)
+		$finder = $this->extension_manager->get_finder();
-		{
-			$task_names = $this->cache->get('_cron_tasks');
 
-			if ($task_names !== false)
+		return $finder
-			{
+			->suffix('_task')
-				return $task_names;
+			->directory('/cron')
-			}
+			->default_path('includes/cron/task/core/')
-		}
+			->default_suffix('')
-
+			->default_directory('')
-		$task_names = array();
+			->get_classes();
-		$ext = '.' . $this->phpEx;
-		$ext_length = strlen($ext);
-
-		$iterator = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($this->task_path));
-
-		foreach ($iterator as $fileinfo)
-		{
-			$file = preg_replace('#^' . preg_quote($this->task_path, '#') . '#', '', $fileinfo->getPathname());
-
-			// skip directories and files direclty in the task root path
-			if ($fileinfo->isFile() && strpos($file, DIRECTORY_SEPARATOR) !== false)
-			{
-				$task_name = str_replace(DIRECTORY_SEPARATOR, '_', substr($file, 0, -$ext_length));
-				if (substr($file, -$ext_length) == $ext && $this->is_valid_name($task_name))
-				{
-					$task_names[] = 'phpbb_cron_task_' . $task_name;
-				}
-			}
-		}
-
-		if ($this->cache)
-		{
-			$this->cache->put('_cron_tasks', $task_names, 3600);
-		}
-
-		return $task_names;
-	}
-
-	/**
-	* Checks whether $name is a valid identifier, and
-	* therefore part of valid cron task class name.
-	*
-	* @param string $name		Name to check
-	*
-	* @return bool
-	*/
-	public function is_valid_name($name)
-	{
-		return (bool) preg_match('/^[a-zA-Z][a-zA-Z0-9_]*$/', $name);
 	}
 
 	/**

tests/cron/ext/testext/cron/dummy_task.php

@@ -7,7 +7,7 @@
 *
 */
 
-class phpbb_cron_task_testmod_dummy_task extends phpbb_cron_task_base
+class phpbb_ext_testext_cron_dummy_task extends phpbb_cron_task_base
 {
 	public static $was_run = 0;
 

tests/cron/includes/cron/task/core/dummy_task.php

@@ -0,0 +1,23 @@
+<?php
+/**
+*
+* @package testing
+* @copyright (c) 2010 phpBB Group
+* @license http://opensource.org/licenses/gpl-license.php GNU Public License
+*
+*/
+
+class phpbb_cron_task_core_dummy_task extends phpbb_cron_task_base
+{
+	public static $was_run = 0;
+
+	public function run()
+	{
+		self::$was_run++;
+	}
+
+	public function should_run()
+	{
+		return true;
+	}
+}

tests/cron/includes/cron/task/core/second_dummy_task.php

@@ -7,7 +7,7 @@
 *
 */
 
-class phpbb_cron_task_testmod_second_dummy_task extends phpbb_cron_task_base
+class phpbb_cron_task_core_second_dummy_task extends phpbb_cron_task_base
 {
 	public static $was_run = 0;
 

tests/cron/manager_test.php

@@ -7,25 +7,35 @@
 *
 */
 
-require_once dirname(__FILE__) . '/../mock/cache.php';
+require_once dirname(__FILE__) . '/../mock/extension_manager.php';
-require_once dirname(__FILE__) . '/task/testmod/dummy_task.php';
+require_once dirname(__FILE__) . '/includes/cron/task/core/dummy_task.php';
-require_once dirname(__FILE__) . '/task/testmod/second_dummy_task.php';
+require_once dirname(__FILE__) . '/includes/cron/task/core/second_dummy_task.php';
-require_once dirname(__FILE__) . '/task2/testmod/simple_ready.php';
+require_once dirname(__FILE__) . '/ext/testext/cron/dummy_task.php';
-require_once dirname(__FILE__) . '/task2/testmod/simple_not_runnable.php';
+require_once dirname(__FILE__) . '/root2/includes/cron/task/core/simple_ready.php';
-require_once dirname(__FILE__) . '/task2/testmod/simple_should_not_run.php';
+require_once dirname(__FILE__) . '/root2/includes/cron/task/core/simple_not_runnable.php';
+require_once dirname(__FILE__) . '/root2/includes/cron/task/core/simple_should_not_run.php';
 
 class phpbb_cron_manager_test extends PHPUnit_Framework_TestCase
 {
 	public function setUp()
 	{
-		$this->manager = new phpbb_cron_manager(dirname(__FILE__) . '/task/', 'php');
+		$this->extension_manager = new phpbb_mock_extension_manager(
-		$this->task_name = 'phpbb_cron_task_testmod_dummy_task';
+			dirname(__FILE__) . '/',
+			array(
+				'testext' => array(
+					'ext_name'		=> 'testext',
+					'ext_active'	=> true,
+					'ext_path'		=> dirname(__FILE__) . '/ext/testext/'
+				),
+			));
+		$this->manager = new phpbb_cron_manager($this->extension_manager);
+		$this->task_name = 'phpbb_cron_task_core_dummy_task';
 	}
 
 	public function test_manager_finds_shipped_tasks()
 	{
 		$tasks = $this->manager->find_cron_task_names();
-		$this->assertEquals(2, sizeof($tasks));
+		$this->assertEquals(3, sizeof($tasks));
 	}
 
 	public function test_manager_finds_shipped_task_by_name()
@@ -45,7 +55,7 @@ class phpbb_cron_manager_test extends PHPUnit_Framework_TestCase
 	public function test_manager_finds_all_ready_tasks()
 	{
 		$tasks = $this->manager->find_all_ready_tasks();
-		$this->assertEquals(2, sizeof($tasks));
+		$this->assertEquals(3, sizeof($tasks));
 	}
 
 	public function test_manager_finds_one_ready_task()
@@ -54,21 +64,12 @@ class phpbb_cron_manager_test extends PHPUnit_Framework_TestCase
 		$this->assertInstanceOf('phpbb_cron_task_wrapper', $task);
 	}
 
-	public function test_manager_finds_all_ready_tasks_cached()
-	{
-		$cache = new phpbb_mock_cache(array('_cron_tasks' => array($this->task_name)));
-		$manager = new phpbb_cron_manager(dirname(__FILE__) . '/../../phpBB/', 'php', $cache);
-
-		$tasks = $manager->find_all_ready_tasks();
-		$this->assertEquals(1, sizeof($tasks));
-	}
-
 	public function test_manager_finds_only_ready_tasks()
 	{
-		$manager = new phpbb_cron_manager(dirname(__FILE__) . '/task2/', 'php');
+		$manager = new phpbb_cron_manager(new phpbb_mock_extension_manager(dirname(__FILE__) . '/root2/'));
 		$tasks = $manager->find_all_ready_tasks();
 		$task_names = $this->tasks_to_names($tasks);
-		$this->assertEquals(array('phpbb_cron_task_testmod_simple_ready'), $task_names);
+		$this->assertEquals(array('phpbb_cron_task_core_simple_ready'), $task_names);
 	}
 
 	private function tasks_to_names($tasks)

tests/cron/root2/includes/cron/task/core/simple_not_runnable.php

@@ -1,6 +1,6 @@
 <?php
 
-class phpbb_cron_task_testmod_simple_not_runnable extends phpbb_cron_task_base
+class phpbb_cron_task_core_simple_not_runnable extends phpbb_cron_task_base
 {
 	public function run()
 	{

tests/cron/root2/includes/cron/task/core/simple_ready.php

@@ -0,0 +1,8 @@
+<?php
+
+class phpbb_cron_task_core_simple_ready extends phpbb_cron_task_base
+{
+	public function run()
+	{
+	}
+}

tests/cron/root2/includes/cron/task/core/simple_should_not_run.php

@@ -1,6 +1,6 @@
 <?php
 
-class phpbb_cron_task_testmod_simple_should_not_run extends phpbb_cron_task_base
+class phpbb_cron_task_core_simple_should_not_run extends phpbb_cron_task_base
 {
 	public function run()
 	{

tests/cron/task2/testmod/simple_ready.php

@@ -1,8 +0,0 @@
-<?php
-
-class phpbb_cron_task_testmod_simple_ready extends phpbb_cron_task_base
-{
-	public function run()
-	{
-	}
-}

tests/extension/finder_test.php

@@ -17,20 +17,22 @@ class phpbb_extension_finder_test extends phpbb_test_case
 
 	public function setUp()
 	{
-		$this->extension_manager = new phpbb_mock_extension_manager(array(
+		$this->extension_manager = new phpbb_mock_extension_manager(
-			'foo' => array(
+			dirname(__FILE__) . '/',
-				'ext_name' => 'foo',
+			array(
-				'ext_active' => '1',
+				'foo' => array(
-				'ext_path' => dirname(__FILE__) . '/ext/foo/',
+					'ext_name' => 'foo',
-			),
+					'ext_active' => '1',
-			'bar' => array(
+					'ext_path' => dirname(__FILE__) . '/ext/foo/',
-				'ext_name' => 'bar',
+				),
-				'ext_active' => '1',
+				'bar' => array(
-				'ext_path' => dirname(__FILE__) . '/ext/bar/',
+					'ext_name' => 'bar',
-			),
+					'ext_active' => '1',
-		));
+					'ext_path' => dirname(__FILE__) . '/ext/bar/',
+				),
+			));
 
-		$this->finder = new phpbb_extension_finder($this->extension_manager, dirname(__FILE__) . '/');
+		$this->finder = $this->extension_manager->get_finder();
 	}
 
 	public function test_suffix_get_classes()

tests/mock/extension_manager.php

@@ -9,8 +9,10 @@
 
 class phpbb_mock_extension_manager extends phpbb_extension_manager
 {
-	public function __construct($extensions = array())
+	public function __construct($phpbb_root_path, $extensions = array())
 	{
+		$this->phpbb_root_path = $phpbb_root_path;
+		$this->phpEx = '.php';
 		$this->extensions = $extensions;
 	}
 }