diff options
| -rw-r--r-- | phpBB/config/default/container/services_cron.yml | 13 | ||||
| -rw-r--r-- | phpBB/config/default/routing/cron.yml | 3 | ||||
| -rw-r--r-- | phpBB/config/default/routing/routing.yml | 4 | ||||
| -rw-r--r-- | phpBB/cron.php | 67 | ||||
| -rw-r--r-- | phpBB/phpbb/cron/controller/cron.php | 38 | ||||
| -rw-r--r-- | phpBB/phpbb/cron/event/cron_runner_listener.php | 101 | ||||
| -rw-r--r-- | phpBB/phpbb/cron/manager.php | 21 | ||||
| -rw-r--r-- | phpBB/phpbb/cron/task/wrapper.php | 47 | ||||
| -rw-r--r-- | tests/console/cron/cron_list_test.php | 29 | ||||
| -rw-r--r-- | tests/console/cron/run_test.php | 87 | ||||
| -rw-r--r-- | tests/cron/manager_test.php | 28 | ||||
| -rw-r--r-- | tests/functional/controllers_compatibility_test.php | 8 | ||||
| -rw-r--r-- | tests/functional/prune_shadow_topic_test.php | 11 |
13 files changed, 374 insertions, 83 deletions
diff --git a/phpBB/config/default/container/services_cron.yml b/phpBB/config/default/container/services_cron.yml index 0b95a96190..fc176c0d29 100644 --- a/phpBB/config/default/container/services_cron.yml +++ b/phpBB/config/default/container/services_cron.yml @@ -3,6 +3,7 @@ services: class: phpbb\cron\manager arguments: - '@cron.task_collection' + - '@routing.helper' - '%core.root_path%' - '%core.php_ext%' @@ -13,6 +14,18 @@ services: - '@config' - '@dbal.conn' + cron.controller: + class: phpbb\cron\controller\cron + + cron.event_listener: + class: phpbb\cron\event\cron_runner_listener + arguments: + - '@cron.lock_db' + - '@cron.manager' + - '@request' + tags: + - { name: kernel.event_subscriber } + # ----- Cron tasks ----- cron.task_collection: class: phpbb\di\service_collection diff --git a/phpBB/config/default/routing/cron.yml b/phpBB/config/default/routing/cron.yml new file mode 100644 index 0000000000..5a634166fa --- /dev/null +++ b/phpBB/config/default/routing/cron.yml @@ -0,0 +1,3 @@ +phpbb_cron_run: + path: /{cron_type} + defaults: { _controller: cron.controller:handle } diff --git a/phpBB/config/default/routing/routing.yml b/phpBB/config/default/routing/routing.yml index f381f024ad..199c5229b0 100644 --- a/phpBB/config/default/routing/routing.yml +++ b/phpBB/config/default/routing/routing.yml @@ -8,6 +8,10 @@ # instantiate the 'foo_service' service and call the 'method' method. # +phpbb_cron_routing: + resource: cron.yml + prefix: /cron + phpbb_feed_routing: resource: feed.yml prefix: /feed diff --git a/phpBB/cron.php b/phpBB/cron.php index 2f519947aa..58261429a2 100644 --- a/phpBB/cron.php +++ b/phpBB/cron.php @@ -11,10 +11,11 @@ * */ +use Symfony\Component\HttpFoundation\RedirectResponse; + /** */ define('IN_PHPBB', true); -define('IN_CRON', true); $phpbb_root_path = (defined('PHPBB_ROOT_PATH')) ? PHPBB_ROOT_PATH : './'; $phpEx = substr(strrchr(__FILE__, '.'), 1); include($phpbb_root_path . 'common.' . $phpEx); @@ -23,62 +24,14 @@ include($phpbb_root_path . 'common.' . $phpEx); $user->session_begin(false); $auth->acl($user->data); -function output_image() -{ - // Output transparent gif - header('Cache-Control: no-cache'); - header('Content-type: image/gif'); - header('Content-length: 43'); - - echo base64_decode('R0lGODlhAQABAIAAAP///wAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw=='); - - // Flush here to prevent browser from showing the page as loading while - // running cron. - flush(); -} - -// 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. -// -// Attempt to alleviate the problem by doing setup outside of the lock as much as possible. - $cron_type = $request->variable('cron_type', ''); -// Comment this line out for debugging so the page does not return an image. -output_image(); - -/* @var $cron_lock \phpbb\lock\db */ -$cron_lock = $phpbb_container->get('cron.lock_db'); -if ($cron_lock->acquire()) -{ - /* @var $cron \phpbb\cron\manager */ - $cron = $phpbb_container->get('cron.manager'); - - $task = $cron->find_task($cron_type); - if ($task) - { - /** - * This event enables you to catch the task before it runs - * - * @event core.cron_run_before - * @var \phpbb\cron\task\wrapper task Current Cron task - * @since 3.1.8-RC1 - */ - $vars = array( - 'task', - ); - extract($phpbb_dispatcher->trigger_event('core.cron_run_before', compact($vars))); - - if ($task->is_parametrized()) - { - $task->parse_parameters($request); - } - if ($task->is_ready()) - { - $task->run(); - } - } - $cron_lock->release(); -} +$get_params_array = $request->get_super_global(\phpbb\request\request_interface::GET); -garbage_collection(); +/** @var \phpbb\controller\helper $controller_helper */ +$controller_helper = $phpbb_container->get('controller.helper'); +$response = new RedirectResponse( + $controller_helper->route('phpbb_cron_run', $get_params_array), + 301 +); +$response->send(); diff --git a/phpBB/phpbb/cron/controller/cron.php b/phpBB/phpbb/cron/controller/cron.php new file mode 100644 index 0000000000..d71136ee5d --- /dev/null +++ b/phpBB/phpbb/cron/controller/cron.php @@ -0,0 +1,38 @@ +<?php +/** + * + * This file is part of the phpBB Forum Software package. + * + * @copyright (c) phpBB Limited <https://www.phpbb.com> + * @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\cron\controller; + +use Symfony\Component\HttpFoundation\Response; + +/** + * Controller for running cron jobs + */ +class cron +{ + /** + * Handles CRON requests + * + * @return Response + */ + public function handle($cron_type) + { + $response = new Response(); + $response->headers->set('Cache-Control', 'no-cache'); + $response->headers->set('Content-type', 'image/gif'); + $response->headers->set('Content-length', '43'); + $response->setContent(base64_decode('R0lGODlhAQABAIAAAP///wAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw==')); + + return $response; + } +} diff --git a/phpBB/phpbb/cron/event/cron_runner_listener.php b/phpBB/phpbb/cron/event/cron_runner_listener.php new file mode 100644 index 0000000000..323ac966ac --- /dev/null +++ b/phpBB/phpbb/cron/event/cron_runner_listener.php @@ -0,0 +1,101 @@ +<?php +/** + * + * This file is part of the phpBB Forum Software package. + * + * @copyright (c) phpBB Limited <https://www.phpbb.com> + * @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\cron\event; + +use phpbb\cron\manager; +use phpbb\lock\db; +use phpbb\request\request_interface; +use Symfony\Component\EventDispatcher\EventSubscriberInterface; +use Symfony\Component\HttpKernel\KernelEvents; +use Symfony\Component\HttpKernel\Event\PostResponseEvent; + +/** + * Event listener that executes cron tasks, after the response was served + */ +class cron_runner_listener implements EventSubscriberInterface +{ + /** + * @var \phpbb\lock\db + */ + private $cron_lock; + + /** + * @var \phpbb\cron\manager + */ + private $cron_manager; + + /** + * @var \phpbb\request\request_interface + */ + private $request; + + /** + * Constructor + * + * @param db $lock + * @param manager $manager + * @param request_interface $request + */ + public function __construct(db $lock, manager $manager, request_interface $request) + { + $this->cron_lock = $lock; + $this->cron_manager = $manager; + $this->request = $request; + } + + /** + * Runs the cron job after the response was sent + */ + public function on_kernel_terminate(PostResponseEvent $event) + { + $request = $event->getRequest(); + $controller_name = $request->get('_route'); + + if ($controller_name !== 'phpbb_cron_run') + { + return; + } + + $cron_type = $request->get('cron_type'); + + if ($this->cron_lock->acquire()) + { + $task = $this->cron_manager->find_task($cron_type); + if ($task) + { + if ($task->is_parametrized()) + { + $task->parse_parameters($this->request); + } + + if ($task->is_ready()) + { + $task->run(); + } + + $this->cron_lock->release(); + } + } + } + + /** + * {@inheritdoc} + */ + static public function getSubscribedEvents() + { + return array( + KernelEvents::TERMINATE => 'on_kernel_terminate', + ); + } +} diff --git a/phpBB/phpbb/cron/manager.php b/phpBB/phpbb/cron/manager.php index 9bd30a0a5b..59ee693074 100644 --- a/phpBB/phpbb/cron/manager.php +++ b/phpBB/phpbb/cron/manager.php @@ -13,6 +13,9 @@ namespace phpbb\cron; +use phpbb\cron\task\wrapper; +use phpbb\routing\helper; + /** * Cron manager class. * @@ -21,6 +24,11 @@ namespace phpbb\cron; class manager { /** + * @var helper + */ + protected $routing_helper; + + /** * Set of \phpbb\cron\task\wrapper objects. * Array holding all tasks that have been found. * @@ -28,18 +36,27 @@ class manager */ protected $tasks = array(); + /** + * @var string + */ protected $phpbb_root_path; + + /** + * @var string + */ protected $php_ext; /** * Constructor. Loads all available tasks. * * @param array|\Traversable $tasks Provides an iterable set of task names + * @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, $phpbb_root_path, $php_ext) + public function __construct($tasks, helper $routing_helper, $phpbb_root_path, $php_ext) { + $this->routing_helper = $routing_helper; $this->phpbb_root_path = $phpbb_root_path; $this->php_ext = $php_ext; @@ -142,6 +159,6 @@ class manager */ public function wrap_task(\phpbb\cron\task\task $task) { - return new \phpbb\cron\task\wrapper($task, $this->phpbb_root_path, $this->php_ext); + return new wrapper($task, $this->routing_helper, $this->phpbb_root_path, $this->php_ext); } } diff --git a/phpBB/phpbb/cron/task/wrapper.php b/phpBB/phpbb/cron/task/wrapper.php index 8a4a8b1f0c..4dc3a7fb95 100644 --- a/phpBB/phpbb/cron/task/wrapper.php +++ b/phpBB/phpbb/cron/task/wrapper.php @@ -13,14 +13,32 @@ namespace phpbb\cron\task; +use phpbb\routing\helper; + /** * Cron task wrapper class. * Enhances cron tasks with convenience methods that work identically for all tasks. */ class wrapper { + /** + * @var helper + */ + protected $routing_helper; + + /** + * @var task + */ protected $task; + + /** + * @var string + */ protected $phpbb_root_path; + + /** + * @var string + */ protected $php_ext; /** @@ -28,13 +46,15 @@ class wrapper * * Wraps a task $task, which must implement cron_task interface. * - * @param \phpbb\cron\task\task $task The cron task to wrap. - * @param string $phpbb_root_path Relative path to phpBB root - * @param string $php_ext PHP file extension + * @param task $task The cron task to wrap. + * @param helper $routing_helper Routing helper for route generation + * @param string $phpbb_root_path Relative path to phpBB root + * @param string $php_ext PHP file extension */ - public function __construct(\phpbb\cron\task\task $task, $phpbb_root_path, $php_ext) + public function __construct(task $task, helper $routing_helper, $phpbb_root_path, $php_ext) { $this->task = $task; + $this->routing_helper = $routing_helper; $this->phpbb_root_path = $phpbb_root_path; $this->php_ext = $php_ext; } @@ -49,7 +69,7 @@ class wrapper */ public function is_parametrized() { - return $this->task instanceof \phpbb\cron\task\parametrized; + return $this->task instanceof parametrized; } /** @@ -76,22 +96,13 @@ class wrapper */ public function get_url() { - $name = $this->get_name(); + $params['cron_type'] = $this->get_name(); if ($this->is_parametrized()) { - $params = $this->task->get_parameters(); - $extra = ''; - foreach ($params as $key => $value) - { - $extra .= '&' . $key . '=' . urlencode($value); - } + $params = array_merge($params, $this->task->get_parameters()); } - else - { - $extra = ''; - } - $url = append_sid($this->phpbb_root_path . 'cron.' . $this->php_ext, 'cron_type=' . $name . $extra); - return $url; + + return $this->routing_helper->route('phpbb_cron_run', $params); } /** diff --git a/tests/console/cron/cron_list_test.php b/tests/console/cron/cron_list_test.php index 3bbe2078ba..9cc029200a 100644 --- a/tests/console/cron/cron_list_test.php +++ b/tests/console/cron/cron_list_test.php @@ -74,7 +74,34 @@ class phpbb_console_command_cron_list_test extends phpbb_test_case $task->set_name('command' . $i); $i++; } - $this->cron_manager = new \phpbb\cron\manager($tasks, $phpbb_root_path, $pathEx); + + $mock_config = new \phpbb\config\config(array( + 'force_server_vars' => false, + 'enable_mod_rewrite' => '', + )); + + $mock_router = $this->getMockBuilder('\phpbb\routing\router') + ->setMethods(array('setContext', 'generate')) + ->disableOriginalConstructor() + ->getMock(); + $mock_router->method('setContext') + ->willReturn(true); + $mock_router->method('generate') + ->willReturn('foobar'); + + $request = new \phpbb\request\request(); + + $routing_helper = new \phpbb\routing\helper( + $mock_config, + $mock_router, + new \phpbb\symfony_request($request), + $request, + new \phpbb\filesystem\filesystem(), + $phpbb_root_path, + $pathEx + ); + + $this->cron_manager = new \phpbb\cron\manager($tasks, $routing_helper, $phpbb_root_path, $pathEx); } public function get_command_tester() diff --git a/tests/console/cron/run_test.php b/tests/console/cron/run_test.php index b4a0203325..8f5be00a80 100644 --- a/tests/console/cron/run_test.php +++ b/tests/console/cron/run_test.php @@ -50,7 +50,34 @@ class phpbb_console_command_cron_run_test extends phpbb_database_test_case $tasks = array( $this->task, ); - $this->cron_manager = new \phpbb\cron\manager($tasks, $phpbb_root_path, $phbEx); + + $mock_config = new \phpbb\config\config(array( + 'force_server_vars' => false, + 'enable_mod_rewrite' => '', + )); + + $mock_router = $this->getMockBuilder('\phpbb\routing\router') + ->setMethods(array('setContext', 'generate')) + ->disableOriginalConstructor() + ->getMock(); + $mock_router->method('setContext') + ->willReturn(true); + $mock_router->method('generate') + ->willReturn('foobar'); + + $request = new \phpbb\request\request(); + + $routing_helper = new \phpbb\routing\helper( + $mock_config, + $mock_router, + new \phpbb\symfony_request($request), + $request, + new \phpbb\filesystem\filesystem(), + $phpbb_root_path, + $phpEx + ); + + $this->cron_manager = new \phpbb\cron\manager($tasks, $routing_helper, $phpbb_root_path, $phpEx); $this->assertSame('0', $config['cron_lock']); } @@ -96,7 +123,34 @@ class phpbb_console_command_cron_run_test extends phpbb_database_test_case { $tasks = array( ); - $this->cron_manager = new \phpbb\cron\manager($tasks, $phpbb_root_path, $phpEx); + + $mock_config = new \phpbb\config\config(array( + 'force_server_vars' => false, + 'enable_mod_rewrite' => '', + )); + + $mock_router = $this->getMockBuilder('\phpbb\routing\router') + ->setMethods(array('setContext', 'generate')) + ->disableOriginalConstructor() + ->getMock(); + $mock_router->method('setContext') + ->willReturn(true); + $mock_router->method('generate') + ->willReturn('foobar'); + + $request = new \phpbb\request\request(); + + $routing_helper = new \phpbb\routing\helper( + $mock_config, + $mock_router, + new \phpbb\symfony_request($request), + $request, + new \phpbb\filesystem\filesystem(), + $phpbb_root_path, + $phpEx + ); + + $this->cron_manager = new \phpbb\cron\manager($tasks, $routing_helper, $phpbb_root_path, $phpEx); $command_tester = $this->get_command_tester(); $exit_status = $command_tester->execute(array('command' => $this->command_name)); @@ -109,7 +163,34 @@ class phpbb_console_command_cron_run_test extends phpbb_database_test_case { $tasks = array( ); - $this->cron_manager = new \phpbb\cron\manager($tasks, $phpbb_root_path, $phpEx); + + $mock_config = new \phpbb\config\config(array( + 'force_server_vars' => false, + 'enable_mod_rewrite' => '', + )); + + $mock_router = $this->getMockBuilder('\phpbb\routing\router') + ->setMethods(array('setContext', 'generate')) + ->disableOriginalConstructor() + ->getMock(); + $mock_router->method('setContext') + ->willReturn(true); + $mock_router->method('generate') + ->willReturn('foobar'); + + $request = new \phpbb\request\request(); + + $routing_helper = new \phpbb\routing\helper( + $mock_config, + $mock_router, + new \phpbb\symfony_request($request), + $request, + new \phpbb\filesystem\filesystem(), + $phpbb_root_path, + $phpEx + ); + + $this->cron_manager = new \phpbb\cron\manager($tasks, $routing_helper, $phpbb_root_path, $phpEx); $command_tester = $this->get_command_tester(); $exit_status = $command_tester->execute(array('command' => $this->command_name, '--verbose' => true)); diff --git a/tests/cron/manager_test.php b/tests/cron/manager_test.php index f4dd69b19b..98814b59de 100644 --- a/tests/cron/manager_test.php +++ b/tests/cron/manager_test.php @@ -75,6 +75,32 @@ class phpbb_cron_manager_test extends \phpbb_test_case { global $phpbb_root_path, $phpEx; - return new \phpbb\cron\manager($tasks, $phpbb_root_path, $phpEx); + $mock_config = new \phpbb\config\config(array( + 'force_server_vars' => false, + 'enable_mod_rewrite' => '', + )); + + $mock_router = $this->getMockBuilder('\phpbb\routing\router') + ->setMethods(array('setContext', 'generate')) + ->disableOriginalConstructor() + ->getMock(); + $mock_router->method('setContext') + ->willReturn(true); + $mock_router->method('generate') + ->willReturn('foobar'); + + $request = new \phpbb\request\request(); + + $routing_helper = new \phpbb\routing\helper( + $mock_config, + $mock_router, + new \phpbb\symfony_request($request), + $request, + new \phpbb\filesystem\filesystem(), + $phpbb_root_path, + $phpEx + ); + + return new \phpbb\cron\manager($tasks, $routing_helper, $phpbb_root_path, $phpEx); } } diff --git a/tests/functional/controllers_compatibility_test.php b/tests/functional/controllers_compatibility_test.php index 9499888a1a..36a34aa7c8 100644 --- a/tests/functional/controllers_compatibility_test.php +++ b/tests/functional/controllers_compatibility_test.php @@ -37,6 +37,13 @@ class phpbb_functional_controllers_compatibility_test extends phpbb_functional_t $this->assert301('feed.php?t=1', 'app.php/feed/topic/1'); } + public function test_cron_compatibility() + { + $this->assert301('cron.php?cron_type=foo', 'app.php/cron/foo'); + $this->assert301('cron.php?cron_type=foo&bar=foobar', 'app.php/cron/foo?bar=foobar'); + $this->assert301('cron.php?cron_type=foo&bar=foobar&who=me', 'app.php/cron/foo?bar=foobar&who=me'); + } + protected function assert301($from, $to) { self::$client->followRedirects(false); @@ -44,6 +51,7 @@ class phpbb_functional_controllers_compatibility_test extends phpbb_functional_t // Fix sid issues $location = self::$client->getResponse()->getHeader('Location'); + $location = str_replace('&', '&', $location); $location = preg_replace('#sid=[^&]+(&(amp;)?)?#', '', $location); if (substr($location, -1) === '?') { diff --git a/tests/functional/prune_shadow_topic_test.php b/tests/functional/prune_shadow_topic_test.php index c014119b98..39bf223f93 100644 --- a/tests/functional/prune_shadow_topic_test.php +++ b/tests/functional/prune_shadow_topic_test.php @@ -130,7 +130,16 @@ class phpbb_functional_prune_shadow_topic_test extends phpbb_functional_test_cas $crawler = self::request('GET', "viewforum.php?f={$this->data['forums']['Prune Shadow']}&sid={$this->sid}"); $this->assertNotEmpty($crawler->filter('img')->last()->attr('src')); - self::request('GET', "cron.php?cron_type=cron.task.core.prune_shadow_topics&f={$this->data['forums']['Prune Shadow']}&sid={$this->sid}", array(), false); + self::request('GET', "app.php/cron/cron.task.core.prune_shadow_topics?f={$this->data['forums']['Prune Shadow']}&sid={$this->sid}", array(), false); + + // Try to ensure that the cron can actually run before we start to wait for it + sleep(1); + $cron_lock = new \phpbb\lock\db('cron_lock', new \phpbb\config\db($this->db, new \phpbb\cache\driver\dummy(), 'phpbb_config'), $this->db); + while (!$cron_lock->acquire()) + { + // do nothing + } + $cron_lock->release(); $this->assert_forum_details($this->data['forums']['Prune Shadow'], array( 'forum_posts_approved' => 0, |