Merge pull request #2508 from ptitlazy/ticket/12597

[ticket/12597] Command for executing all available cron tasks

* ptitlazy/ticket/12597: (27 commits)
  [ticket/12597] Reformating an acp message
  [ticket/12597] Modifiying acp message
  [ticket/12597] Fix wrong global variable name
  [ticket/12597] Typo correction
  [ticket/12597] Typo corrections
  [ticket/12597] Changing place of lock release in execute() method
  [ticket/12597] Fix test file
  [ticket/12597] Typing corrections and improvement of code consistency
  [ticket/12597] Fix various mistakes
  [ticket/12597] Fix various mistakes
  [ticket/12597] Fix visibilty of two functions in run.php
  [ticket/12597] Refactoring and test improving
  [ticket/12597] Fix language key name
  [ticket/12597] Fix coding style and typing mistakes
  [ticket/12597] Changes name of command  cron:run-all to cron:run.
  [ticket/12597] Fix various problems
  [ticket/12597] Fix constructor bug and servral doc blocs
  [ticket/12597] Modification of return statuses and of test files
  [ticket/12597] Fix misplaced release of db lock
  [ticket/12597] Change EXECUTE to RUN in language
  ...

phpBB/config/console.yml

@@ -46,6 +46,15 @@ services:
         tags:
             - { name: console.command }
 
+    console.command.cron.run:
+        class: phpbb\console\command\cron\run
+        arguments:
+            - @cron.manager
+            - @cron.lock_db
+            - @user
+        tags:
+            - { name: console.command }
+
     console.command.db.migrate:
         class: phpbb\console\command\db\migrate
         arguments:

phpBB/cron.php

@@ -37,25 +37,6 @@ function output_image()
 	flush();
 }
 
-function do_cron($cron_lock, $run_tasks)
-{
-	global $config;
-
-	foreach ($run_tasks as $task)
-	{
-		if (defined('DEBUG') && $config['use_system_cron'])
-		{
-			echo "[phpBB cron] Running task '{$task->get_name()}'\n";
-		}
-
-		$task->run();
-	}
-
-	// Unloading cache and closing db after having done the dirty work.
-	$cron_lock->release();
-	garbage_collection();
-}
-
 // Thanks to various fatal errors and lack of try/finally, it is quite easy to leave
 // the cron lock locked, especially when working on cron-related code.
 //
@@ -63,47 +44,36 @@ function do_cron($cron_lock, $run_tasks)
 //
 // If DEBUG is defined and cron lock cannot be obtained, a message will be printed.
 
-if (!$config['use_system_cron'])
-{
-	$cron_type = request_var('cron_type', '');
+$cron_type = request_var('cron_type', '');
 
-	// Comment this line out for debugging so the page does not return an image.
-	output_image();
-}
+// Comment this line out for debugging so the page does not return an image.
+output_image();
 
 $cron_lock = $phpbb_container->get('cron.lock_db');
 if ($cron_lock->acquire())
 {
 	$cron = $phpbb_container->get('cron.manager');
 
-	if ($config['use_system_cron'])
+	$task = $cron->find_task($cron_type);
+	if ($task)
 	{
-		$run_tasks = $cron->find_all_ready_tasks();
-	}
-	else
-	{
-		// If invalid task is specified, empty $run_tasks is passed to do_cron which then does nothing
-		$run_tasks = array();
-		$task = $cron->find_task($cron_type);
-		if ($task)
+		if ($task->is_parametrized())
 		{
-			if ($task->is_parametrized())
-			{
-				$task->parse_parameters($request);
-			}
-			if ($task->is_ready())
-			{
-				$run_tasks = array($task);
-			}
+			$task->parse_parameters($request);
+		}
+		if ($task->is_ready())
+		{
+			$task->run();
+			garbage_collection();
 		}
 	}
+	$cron_lock->release();
 
-	do_cron($cron_lock, $run_tasks);
 }
 else
 {
 	if (defined('DEBUG'))
 	{
-		echo "Could not obtain cron lock.\n";
+		echo $user->lang('CRON_LOCK_ERROR') . "\n";
 	}
 }

phpBB/language/en/acp/board.php

@@ -461,7 +461,7 @@ $lang = array_merge($lang, array(
 	'UPLOAD_ICONS_PATH'			=> 'Extension group icons storage path',
 	'UPLOAD_ICONS_PATH_EXPLAIN'	=> 'Path under your phpBB root directory, e.g. <samp>images/upload_icons</samp>.',
 	'USE_SYSTEM_CRON'		=> 'Run periodic tasks from system cron',
-	'USE_SYSTEM_CRON_EXPLAIN'		=> 'When off, phpBB will arrange for periodic tasks to be run automatically. When on, phpBB will not schedule any periodic tasks by itself; a system administrator must arrange for <code>cron.php</code> to be invoked by the system cron facility at regular intervals (e.g. every 5 minutes).',
+	'USE_SYSTEM_CRON_EXPLAIN'		=> 'When off, phpBB will arrange for periodic tasks to be run automatically. When on, phpBB will not schedule any periodic tasks by itself; a system administrator must arrange for <code>cron:run</code> command to be run by the system cron facility at regular intervals (e.g. every 5 minutes).',
 ));
 
 // Security Settings

phpBB/language/en/acp/common.php

@@ -223,8 +223,14 @@ $lang = array_merge($lang, array(
 
 	'BACK'					=> 'Back',
 
+	'CLI_DESCRIPTION_CRON_RUN'		=> 'Runs all available cron tasks.',
+	'CLI_DESCRIPTION_CRON_RUN_ARGUMENT_1'	=> 'Name of the task to be run',
+
 	'COLOUR_SWATCH'			=> 'Web-safe colour swatch',
 	'CONFIG_UPDATED'		=> 'Configuration updated successfully.',
+	'CRON_LOCK_ERROR'		=> 'Could not obtain cron lock.',
+	'CRON_NO_SUCH_TASK'		=> 'No such cron task',
+	'CRON_NO_TASK'			=> 'No task to be run',
 
 	'DEACTIVATE'				=> 'Deactivate',
 	'DIRECTORY_DOES_NOT_EXIST'	=> 'The entered path “%s” does not exist.',
@@ -285,6 +291,7 @@ $lang = array_merge($lang, array(
 	'REMIND'							=> 'Remind',
 	'RESYNC'							=> 'Resynchronise',
 
+	'RUNNING_TASK'			=> 'Running task: %s.',
 	'SELECT_ANONYMOUS'		=> 'Select anonymous user',
 	'SELECT_OPTION'			=> 'Select option',
 

phpBB/phpbb/console/command/cron/run.php

@@ -0,0 +1,176 @@
+<?php
+/**
+*
+* This file is part of the phpBB Forum Software package.
+*
+* @copyright (c) phpBB Limited 
+* @license GNU General Public License, version 2 (GPL-2.0)
+*
+* For full copyright and license information, please see
+* the docs/CREDITS.txt file.
+*
+*/
+
+namespace phpbb\console\command\cron;
+
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Input\InputArgument;
+use Symfony\Component\Console\Input\InputOption;
+use Symfony\Component\Console\Output\OutputInterface;
+
+class run extends \phpbb\console\command\command
+{
+	/** @var \phpbb\cron\manager */
+	protected $cron_manager;
+
+	/** @var \phpbb\lock\db */
+	protected $lock_db;
+
+	/** @var \phpbb\user */
+	protected $user;
+
+	/**
+	* Construct method
+	*
+	* @param \phpbb\cron\manager $cron_manager The cron manager containing
+	*		the cron tasks to be executed.
+	* @param \phpbb\lock\db $lock_db The lock for accessing database.
+	* @param \phobb\user $user The user object (used to get language information)
+	*/
+	public function __construct(\phpbb\cron\manager $cron_manager, \phpbb\lock\db $lock_db, \phpbb\user $user)
+	{
+		$this->cron_manager = $cron_manager;
+		$this->lock_db = $lock_db;
+		$this->user = $user;
+		parent::__construct();
+	}
+
+	/**
+	* Sets the command name and description
+	*
+	* @return null
+	*/
+	protected function configure()
+	{
+		$this
+			->setName('cron:run')
+			->setDescription($this->user->lang('CLI_DESCRIPTION_CRON_RUN'))
+			->addArgument('name', InputArgument::OPTIONAL, $this->user->lang('CLI_DESCRIPTION_CRON_RUN_ARGUMENT_1'))
+		;
+	}
+
+	/**
+	* Executes the command cron:run.
+	*
+	* Tries to acquire the cron lock, then if no argument has been given runs all ready cron tasks.
+	* If the cron lock can not be obtained, an error message is printed
+	*		and the exit status is set to 1.
+	* If the verbose option is specified, each start of a task is printed.
+	*		Otherwise there is no output.
+	* If an argument is given to the command, only the task whose name matches the 
+	*		argument will be started. If verbose option is specified,
+	*		an info message containing the name of the task is printed.
+	* If no task matches the argument given, an error message is printed
+	*		and the exit status is set to 2.
+	*
+	* @param InputInterface $input The input stream used to get the argument and verboe option.
+	* @param OutputInterface $output The output stream, used for printing verbose-mode and error information.
+	*
+	* @return int 0 if all is ok, 1 if a lock error occured and 2 if no task matching the argument was found.
+	*/
+	protected function execute(InputInterface $input, OutputInterface $output)
+	{
+		if ($this->lock_db->acquire())
+		{
+			$task_name = $input->getArgument('name');
+			if ($task_name)
+			{
+				$exit_status = $this->run_one($input, $output, $task_name);
+			}
+			else
+			{
+				$exit_status = $this->run_all($input, $output);
+			}
+
+			$this->lock_db->release();
+			return $exit_status;
+		}
+		else
+		{
+			$output->writeln('<error>' . $this->user->lang('CRON_LOCK_ERROR') . '</error>');
+			return 1;
+		}
+	}
+
+	/*
+	* Executes all ready cron tasks.
+	*
+	* If verbose mode is set, an info message will be printed if there is no task to
+	*		be run, or else for each starting task.
+	*
+	* @see execute
+	* @param InputInterface $input The input stream used to get the argument and verbose option.
+	* @param OutputInterface $output The output stream, used for printing verbose-mode and error information.
+	* @return int 0
+	*/
+	protected function run_all(InputInterface $input, OutputInterface $output)
+	{
+		$run_tasks = $this->cron_manager->find_all_ready_tasks();
+
+		if ($run_tasks)
+		{
+			foreach ($run_tasks as $task)
+			{
+				if ($input->getOption('verbose'))
+				{
+					$output->writeln('<info>' . $this->user->lang('RUNNING_TASK', $task->get_name()) . '</info>');
+				}
+
+				$task->run();
+			}
+		}
+		else
+		{
+			if ($input->getOption('verbose'))
+			{
+				$output->writeln('<info>' . $this->user->lang('CRON_NO_TASK') . '</info>');
+			}
+		}
+
+		return 0;
+	}
+
+	/*
+	* Executes a given cron task, if it is ready.
+	*
+	* If there is a task whose name matches $task_name, it is run and 0 is returned.
+	*		and if verbose mode is set, print an info message with the name of the task.
+	* If there is no task matching $task_name, the function prints an error message
+	*		and returns with status 2.
+	*
+	* @see execute
+	* @param string $task_name The name of the task that should be run.
+	* @param InputInterface $input The input stream used to get the argument and verbose option.
+	* @param OutputInterface $output The output stream, used for printing verbose-mode and error information.
+	* @return int 0 if all is well, 2 if no task matches $task_name.
+	*/
+	protected function run_one(InputInterface $input, OutputInterface $output, $task_name)
+	{
+		$task = $this->cron_manager->find_task($task_name);
+		if ($task)
+		{
+			if ($input->getOption('verbose'))
+			{
+				$output->writeln('<info>' . $this->user->lang('RUNNING_TASK', $task_name) . '</info>');
+			}
+
+			$task->run();
+			return 0;
+		}
+		else
+		{
+			$output->writeln('<error>' . $this->user->lang('CRON_NO_SUCH_TASK') . '</error>');
+			return 2;
+		}
+	}
+}

tests/console/cron/fixtures/config.xml

@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<dataset>
+	<table name="phpbb_config">
+		<column>config_name</column>
+		<column>config_value</column>
+		<column>is_dynamic</column>
+	</table>
+</dataset>

tests/console/cron/run_test.php

@@ -0,0 +1,157 @@
+<?php
+/**
+*
+* This file is part of the phpBB Forum Software package.
+*
+* @copyright (c) phpBB Limited 
+* @license GNU General Public License, version 2 (GPL-2.0)
+*
+* For full copyright and license information, please see
+* the docs/CREDITS.txt file.
+*
+*/
+
+use Symfony\Component\Console\Application;
+use Symfony\Component\Console\Tester\CommandTester;
+use phpbb\console\command\cron\run;
+
+require_once dirname(__FILE__) . '/tasks/simple.php';
+
+class phpbb_console_command_cron_run_test extends phpbb_database_test_case
+{
+	protected $db;
+	protected $config;
+	protected $lock;
+	protected $user;
+	protected $cron_manager;
+	protected $command_name;
+	protected $task;
+
+	public function getDataSet()
+	{
+		return $this->createXMLDataSet(dirname(__FILE__) . '/fixtures/config.xml');
+	}
+
+	public function setUp()
+	{
+		global $db, $config, $phpbb_root_path, $phpEx;
+
+		$db = $this->db = $this->new_dbal();
+		$config = $this->config = new \phpbb\config\config(array('cron_lock' => '0'));
+		set_config(null, null, null, $this->config);
+		$this->lock = new \phpbb\lock\db('cron_lock', $this->config, $this->db);
+
+		$this->user = $this->getMock('\phpbb\user');
+		$this->user->method('lang')->will($this->returnArgument(0));
+
+		$this->task = new phpbb_cron_task_simple();
+		$tasks = array(
+			$this->task,
+		);
+		$this->cron_manager = new \phpbb\cron\manager($tasks, $phpbb_root_path, $phbEx);
+
+		$this->assertSame('0', $config['cron_lock']);
+	}
+
+	public function test_normal_use()
+	{
+		$command_tester = $this->get_command_tester();
+		$exit_status = $command_tester->execute(array('command' => $this->command_name));
+
+		$this->assertSame('', $command_tester->getDisplay());
+		$this->assertSame(true, $this->task->executed);
+		$this->assertSame(0, $exit_status);
+		$this->assertSame(false, $this->lock->owns_lock());
+	}
+
+	public function test_verbose_mode()
+	{
+		$command_tester = $this->get_command_tester();
+		$exit_status = $command_tester->execute(array('command' => $this->command_name, '--verbose' => true));
+
+		$this->assertContains('RUNNING_TASK', $command_tester->getDisplay());
+		$this->assertSame(true, $this->task->executed);
+		$this->assertSame(0, $exit_status);
+		$this->assertSame(false, $this->lock->owns_lock());
+	}
+
+	public function test_error_lock()
+	{
+		$this->lock->acquire();
+		$command_tester = $this->get_command_tester();
+		$exit_status = $command_tester->execute(array('command' => $this->command_name));
+
+		$this->assertContains('CRON_LOCK_ERROR', $command_tester->getDisplay());
+		$this->assertSame(false, $this->task->executed);
+		$this->assertSame(1, $exit_status);
+	}
+
+	public function test_no_task()
+	{
+		$tasks = array(
+		);
+		$this->cron_manager = new \phpbb\cron\manager($tasks, $phpbb_root_path, $phpEx);
+		$command_tester = $this->get_command_tester();
+		$exit_status = $command_tester->execute(array('command' => $this->command_name));
+
+		$this->assertSame('', $command_tester->getDisplay());
+		$this->assertSame(0, $exit_status);
+		$this->assertSame(false, $this->lock->owns_lock());
+	}
+
+	public function test_no_task_verbose()
+	{
+		$tasks = array(
+		);
+		$this->cron_manager = new \phpbb\cron\manager($tasks, $phpbb_root_path, $phpEx);
+		$command_tester = $this->get_command_tester();
+		$exit_status = $command_tester->execute(array('command' => $this->command_name, '--verbose' => true));
+
+		$this->assertContains('CRON_NO_TASK', $command_tester->getDisplay());
+		$this->assertSame(0, $exit_status);
+		$this->assertSame(false, $this->lock->owns_lock());
+	}
+
+	public function test_arg_valid()
+	{
+		$command_tester = $this->get_command_tester();
+		$exit_status = $command_tester->execute(array('command' => $this->command_name, 'name' => 'phpbb_cron_task_simple'));
+
+		$this->assertSame('', $command_tester->getDisplay());
+		$this->assertSame(true, $this->task->executed);
+		$this->assertSame(0, $exit_status);
+		$this->assertSame(false, $this->lock->owns_lock());
+	}
+
+	public function test_arg_invalid()
+	{
+		$command_tester = $this->get_command_tester();
+		$exit_status = $command_tester->execute(array('command' => $this->command_name, 'name' => 'foo'));
+
+		$this->assertContains('CRON_NO_SUCH_TASK', $command_tester->getDisplay());
+		$this->assertSame(false, $this->task->executed);
+		$this->assertSame(2, $exit_status);
+		$this->assertSame(false, $this->lock->owns_lock());
+	}
+
+	public function test_arg_valid_verbose()
+	{
+		$command_tester = $this->get_command_tester();
+		$exit_status = $command_tester->execute(array('command' => $this->command_name, 'name' => 'phpbb_cron_task_simple', '--verbose' => true));
+
+		$this->assertContains('RUNNING_TASK', $command_tester->getDisplay());
+		$this->assertSame(true, $this->task->executed);
+		$this->assertSame(0, $exit_status);
+		$this->assertSame(false, $this->lock->owns_lock());
+	}
+
+	public function get_command_tester()
+	{
+		$application = new Application();
+		$application->add(new run($this->cron_manager, $this->lock, $this->user));
+
+		$command = $application->find('cron:run');
+		$this->command_name = $command->getName();
+		return new CommandTester($command);
+	}
+}

tests/console/cron/tasks/simple.php

@@ -0,0 +1,27 @@
+<?php
+/**
+*
+* This file is part of the phpBB Forum Software package.
+*
+* @copyright (c) phpBB Limited 
+* @license GNU General Public License, version 2 (GPL-2.0)
+*
+* For full copyright and license information, please see
+* the docs/CREDITS.txt file.
+*
+*/
+
+class phpbb_cron_task_simple extends \phpbb\cron\task\base
+{
+	public $executed = false;
+
+	public function get_name()
+	{
+		return get_class($this);
+	}
+
+	public function run()
+	{
+		$this->executed = true;
+	}
+}