aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMáté Bartus <mate.bartus@gmail.com>2016-03-18 22:57:02 +0100
committerMarc Alexander <admin@m-a-styles.de>2019-05-06 21:45:15 +0200
commit7a173877b7244f4ab6a8ff7b6fa0d6450027751b (patch)
tree5f820d58f33a9e972815007dfc79494669d7e702
parent41956a8b9050fa6e1e301bfae51d947e6f13c068 (diff)
downloadforums-7a173877b7244f4ab6a8ff7b6fa0d6450027751b.tar
forums-7a173877b7244f4ab6a8ff7b6fa0d6450027751b.tar.gz
forums-7a173877b7244f4ab6a8ff7b6fa0d6450027751b.tar.bz2
forums-7a173877b7244f4ab6a8ff7b6fa0d6450027751b.tar.xz
forums-7a173877b7244f4ab6a8ff7b6fa0d6450027751b.zip
[ticket/14542] Move cron to controller
PHPBB3-14542
-rw-r--r--phpBB/config/default/container/services_cron.yml13
-rw-r--r--phpBB/config/default/routing/cron.yml3
-rw-r--r--phpBB/config/default/routing/routing.yml4
-rw-r--r--phpBB/cron.php67
-rw-r--r--phpBB/phpbb/cron/controller/cron.php38
-rw-r--r--phpBB/phpbb/cron/event/cron_runner_listener.php101
-rw-r--r--phpBB/phpbb/cron/manager.php21
-rw-r--r--phpBB/phpbb/cron/task/wrapper.php47
-rw-r--r--tests/console/cron/cron_list_test.php29
-rw-r--r--tests/console/cron/run_test.php87
-rw-r--r--tests/cron/manager_test.php28
-rw-r--r--tests/functional/controllers_compatibility_test.php8
-rw-r--r--tests/functional/prune_shadow_topic_test.php11
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 d7f6388536..70f70e355d 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 .= '&amp;' . $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 fdc9a05cb2..99291ec215 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 76f8c753bf..d204eb90d1 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('&amp;', '&', $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,