From 843e3ae8494f50109106b28ceccace177ca9648e Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 19 Jan 2020 16:03:32 +0100 Subject: [PATCH 1/4] [ticket/16314] Improve handling of page_footer output for controllers Moved the common template output to controller helper and added garbage_collection handling to kernel_terminate_subscriber. PHPBB3-16314 --- phpBB/config/default/container/services.yml | 9 + phpBB/includes/functions.php | 54 +---- phpBB/phpbb/controller/helper.php | 176 +++++++++++++-- .../event/kernel_terminate_subscriber.php | 4 +- tests/controller/common_helper_route.php | 200 ++++++++++++++++-- tests/pagination/pagination_test.php | 22 +- 6 files changed, 372 insertions(+), 93 deletions(-) diff --git a/phpBB/config/default/container/services.yml b/phpBB/config/default/container/services.yml index 2d4720029d..739341ab49 100644 --- a/phpBB/config/default/container/services.yml +++ b/phpBB/config/default/container/services.yml @@ -85,12 +85,21 @@ services: controller.helper: class: phpbb\controller\helper arguments: + - '@auth' + - '@cache.driver' + - '@cron.manager' + - '@dbal.conn' + - '@dispatcher' + - '@language' - '@template' - '@user' - '@config' - '@symfony_request' - '@request' - '@routing.helper' + - '%core.adm_relative_path%' + - '%core.php_ext%' + - '%debug.sql_explain%' controller.resolver: class: phpbb\controller\resolver diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 9759eabb5a..eb667eed7e 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -4403,8 +4403,7 @@ function phpbb_generate_debug_output(\phpbb\db\driver\driver_interface $db, \php */ function page_footer($run_cron = true, $display_template = true, $exit_handler = true) { - global $db, $config, $template, $user, $auth, $cache, $phpEx; - global $request, $phpbb_dispatcher, $phpbb_admin_path; + global $phpbb_dispatcher, $phpbb_container, $template; // A listener can set this variable to `true` when it overrides this function $page_footer_override = false; @@ -4426,55 +4425,10 @@ function page_footer($run_cron = true, $display_template = true, $exit_handler = return; } - phpbb_check_and_display_sql_report($request, $auth, $db); + /** @var \phpbb\controller\helper $controller_helper */ + $controller_helper = $phpbb_container->get('controller.helper'); - $template->assign_vars(array( - 'DEBUG_OUTPUT' => phpbb_generate_debug_output($db, $config, $auth, $user, $phpbb_dispatcher), - 'TRANSLATION_INFO' => (!empty($user->lang['TRANSLATION_INFO'])) ? $user->lang['TRANSLATION_INFO'] : '', - 'CREDIT_LINE' => $user->lang('POWERED_BY', 'phpBB® Forum Software © phpBB Limited'), - - 'U_ACP' => ($auth->acl_get('a_') && !empty($user->data['is_registered'])) ? append_sid("{$phpbb_admin_path}index.$phpEx", false, true, $user->session_id) : '') - ); - - // Call cron-type script - $call_cron = false; - if (!defined('IN_CRON') && !$config['use_system_cron'] && $run_cron && !$config['board_disable'] && !$user->data['is_bot'] && !$cache->get('_cron.lock_check')) - { - $call_cron = true; - $time_now = (!empty($user->time_now) && is_int($user->time_now)) ? $user->time_now : time(); - - // Any old lock present? - if (!empty($config['cron_lock'])) - { - $cron_time = explode(' ', $config['cron_lock']); - - // If 1 hour lock is present we do not call cron.php - if ($cron_time[0] + 3600 >= $time_now) - { - $call_cron = false; - } - } - } - - // Call cron job? - if ($call_cron) - { - global $phpbb_container; - - /* @var $cron \phpbb\cron\manager */ - $cron = $phpbb_container->get('cron.manager'); - $task = $cron->find_one_ready_task(); - - if ($task) - { - $url = $task->get_url(); - $template->assign_var('RUN_CRON_TASK', 'cron'); - } - else - { - $cache->put('_cron.lock_check', true, 60); - } - } + $controller_helper->display_footer($run_cron); /** * Execute code and/or modify output before displaying the template. diff --git a/phpBB/phpbb/controller/helper.php b/phpBB/phpbb/controller/helper.php index 58a4a492f8..8e8cd135f6 100644 --- a/phpBB/phpbb/controller/helper.php +++ b/phpBB/phpbb/controller/helper.php @@ -13,6 +13,18 @@ namespace phpbb\controller; +use phpbb\auth\auth; +use \phpbb\cache\driver\driver_interface as cache_interface; +use phpbb\config\config; +use phpbb\cron\manager; +use phpbb\db\driver\driver_interface; +use phpbb\event\dispatcher; +use phpbb\language\language; +use phpbb\request\request_interface; +use phpbb\routing\helper as routing_helper; +use phpbb\symfony_request; +use phpbb\template\template; +use phpbb\user; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; @@ -22,53 +34,101 @@ use Symfony\Component\Routing\Generator\UrlGeneratorInterface; */ class helper { + /** @var auth */ + protected $auth; + + /** @var cache_interface */ + protected $cache; + + /** @var manager */ + protected $cron_manager; + + /** @var driver_interface */ + protected $db; + + /** @var dispatcher */ + protected $dispatcher; + + /** @var language */ + protected $language; + /** * Template object - * @var \phpbb\template\template + * @var template */ protected $template; /** * User object - * @var \phpbb\user + * @var user */ protected $user; /** * config object - * @var \phpbb\config\config + * @var config */ protected $config; - /* @var \phpbb\symfony_request */ + /* @var symfony_request */ protected $symfony_request; - /* @var \phpbb\request\request_interface */ + /* @var request_interface */ protected $request; /** - * @var \phpbb\routing\helper + * @var routing_helper */ protected $routing_helper; + /** @var string */ + protected $admin_path; + + /** @var string */ + protected $php_ext; + + /** @var bool $sql_explain */ + protected $sql_explain; + /** * Constructor * - * @param \phpbb\template\template $template Template object - * @param \phpbb\user $user User object - * @param \phpbb\config\config $config Config object - * @param \phpbb\symfony_request $symfony_request Symfony Request object - * @param \phpbb\request\request_interface $request phpBB request object - * @param \phpbb\routing\helper $routing_helper Helper to generate the routes + * @param auth $auth Auth object + * @param cache_interface $cache + * @param manager $cron_manager + * @param driver_interface $db Dbal object + * @param dispatcher $dispatcher + * @param language $language + * @param template $template Template object + * @param user $user User object + * @param config $config Config object + * @param symfony_request $symfony_request Symfony Request object + * @param request_interface $request phpBB request object + * @param routing_helper $routing_helper Helper to generate the routes + * @param string $admin_path Admin path + * @param string $php_ext PHP extension + * @param bool $sql_explain Flag whether to display sql explain */ - public function __construct(\phpbb\template\template $template, \phpbb\user $user, \phpbb\config\config $config, \phpbb\symfony_request $symfony_request, \phpbb\request\request_interface $request, \phpbb\routing\helper $routing_helper) + public function __construct(auth $auth, cache_interface $cache, manager $cron_manager, driver_interface $db, + dispatcher $dispatcher, language $language, template $template, user $user, config $config, + symfony_request $symfony_request, request_interface $request, routing_helper $routing_helper, + $admin_path, $php_ext, $sql_explain = false) { + $this->auth = $auth; + $this->cache = $cache; + $this->cron_manager = $cron_manager; + $this->db = $db; + $this->dispatcher = $dispatcher; + $this->language = $language; $this->template = $template; $this->user = $user; $this->config = $config; $this->symfony_request = $symfony_request; $this->request = $request; $this->routing_helper = $routing_helper; + $this->admin_path = $admin_path; + $this->php_ext = $php_ext; + $this->sql_explain = $sql_explain; } /** @@ -92,7 +152,7 @@ class helper 'body' => $template_file, )); - page_footer(true, false, false); + $this->display_footer(); $headers = !empty($this->user->data['is_bot']) ? array('X-PHPBB-IS-BOT' => 'yes') : array(); @@ -142,8 +202,8 @@ class helper public function message($message, array $parameters = array(), $title = 'INFORMATION', $code = 200) { array_unshift($parameters, $message); - $message_text = call_user_func_array(array($this->user, 'lang'), $parameters); - $message_title = $this->user->lang($title); + $message_text = call_user_func_array(array($this->language, 'lang'), $parameters); + $message_title = $this->language->lang($title); if ($this->request->is_ajax()) { @@ -174,7 +234,7 @@ class helper * * @param int $time time in seconds, when redirection should occur * @param string $url the URL where the user should be redirected - * @return null + * @return void */ public function assign_meta_refresh_var($time, $url) { @@ -192,4 +252,86 @@ class helper { return generate_board_url(true) . $this->request->escape($this->symfony_request->getRequestUri(), true); } + + /** + * Handle display actions for footer, e.g. SQL report and credit line + * + * @param bool $run_cron Flag whether cron should be run + * + * @return void + */ + public function display_footer($run_cron = true) + { + $this->display_sql_report(); + + $this->template->assign_vars([ + 'DEBUG_OUTPUT' => phpbb_generate_debug_output($this->db, $this->config, $this->auth, $this->user, $this->dispatcher), + 'TRANSLATION_INFO' => $this->language->is_set('TRANSLATION_INFO') ? $this->language->lang('TRANSLATION_INFO') : '', + 'CREDIT_LINE' => $this->language->lang('POWERED_BY', 'phpBB® Forum Software © phpBB Limited'), + + 'U_ACP' => ($this->auth->acl_get('a_') && !empty($this->user->data['is_registered'])) ? append_sid("{$this->admin_path}index.{$this->php_ext}", false, true, $this->user->session_id) : '', + ]); + + if ($run_cron) + { + $this->set_cron_task(); + } + } + + /** + * Display SQL report + * + * @return void + */ + protected function display_sql_report() + { + if ($this->sql_explain && $this->request->variable('explain', false) && $this->auth->acl_get('a_')) + { + $this->db->sql_report('display'); + } + } + + /** + * Set cron task for footer + * + * @return void + */ + protected function set_cron_task() + { + // Call cron-type script + $call_cron = false; + if (!defined('IN_CRON') && !$this->config['use_system_cron'] && !$this->config['board_disable'] && !$this->user->data['is_bot'] && !$this->cache->get('_cron.lock_check')) + { + $call_cron = true; + $time_now = (!empty($this->user->time_now) && is_int($this->user->time_now)) ? $this->user->time_now : time(); + + // Any old lock present? + if (!empty($this->config['cron_lock'])) + { + $cron_time = explode(' ', $this->config['cron_lock']); + + // If 1 hour lock is present we do not set a cron task + if ($cron_time[0] + 3600 >= $time_now) + { + $call_cron = false; + } + } + } + + // Call cron job? + if ($call_cron) + { + $task = $this->cron_manager->find_one_ready_task(); + + if ($task) + { + $url = $task->get_url(); + $this->template->assign_var('RUN_CRON_TASK', 'cron'); + } + else + { + $this->cache->put('_cron.lock_check', true, 60); + } + } + } } diff --git a/phpBB/phpbb/event/kernel_terminate_subscriber.php b/phpBB/phpbb/event/kernel_terminate_subscriber.php index f0d0a2f595..eb7b16a7aa 100644 --- a/phpBB/phpbb/event/kernel_terminate_subscriber.php +++ b/phpBB/phpbb/event/kernel_terminate_subscriber.php @@ -25,10 +25,12 @@ class kernel_terminate_subscriber implements EventSubscriberInterface * primarily cleanup stuff. * * @param PostResponseEvent $event - * @return null + * @return void */ public function on_kernel_terminate(PostResponseEvent $event) { + garbage_collection(); + exit_handler(); } diff --git a/tests/controller/common_helper_route.php b/tests/controller/common_helper_route.php index bdaf8ee682..a357eddfe9 100644 --- a/tests/controller/common_helper_route.php +++ b/tests/controller/common_helper_route.php @@ -13,21 +13,33 @@ use Symfony\Component\Routing\Generator\UrlGeneratorInterface; -abstract class phpbb_controller_common_helper_route extends phpbb_test_case +abstract class phpbb_controller_common_helper_route extends phpbb_database_test_case { - protected $root_path; - private $user; - private $config; - private $template; - private $extension_manager; - private $request; - private $symfony_request; - private $provider; - private $filesystem; - private $phpbb_path_helper; - private $helper; - private $router; - private $routing_helper; + protected $root_path; + private $auth; + private $cache; + private $db; + private $dispatcher; + private $language; + private $admin_path; + private $php_ext; + private $user; + private $config; + private $template; + private $extension_manager; + private $request; + private $symfony_request; + private $provider; + private $filesystem; + private $phpbb_path_helper; + private $helper; + private $router; + private $routing_helper; + + public function getDataSet() + { + return $this->createXMLDataSet(dirname(__FILE__) . '/../fixtures/empty.xml'); + } public function setUp(): void { @@ -141,6 +153,13 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case ); $resources_locator = new \phpbb\routing\resources_locator\default_resources_locator(dirname(__FILE__) . '/', PHPBB_ENVIRONMENT, $this->extension_manager); $this->router = new phpbb_mock_router($container, $resources_locator, $loader, dirname(__FILE__) . '/', 'php', false); + $this->auth = new \phpbb\auth\auth(); + $this->cache = new \phpbb\cache\driver\dummy(); + $this->db = $this->new_dbal(); + $this->dispatcher = new phpbb_mock_event_dispatcher(); + $this->language = new \phpbb\language\language(new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx)); + $this->admin_path = $phpbb_root_path . 'adm/'; + $this->php_ext = $phpEx; // Set correct current phpBB root path $this->root_path = $this->get_phpbb_root_path(); @@ -186,7 +205,22 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case { $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->template, $this->user, $this->config, $this->symfony_request, $this->request, $this->routing_helper); + $this->helper = new phpbb_mock_controller_helper( + $this->auth, + $this->cache, + new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), + $this->db, + $this->dispatcher, + $this->language, + $this->template, + $this->user, + $this->config, + $this->symfony_request, + $this->request, + $this->routing_helper, + $this->admin_path, + $this->php_ext + ); static::assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id), $description); } @@ -230,7 +264,22 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case { $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->template, $this->user, $this->config, $this->symfony_request, $this->request, $this->routing_helper); + $this->helper = new phpbb_mock_controller_helper( + $this->auth, + $this->cache, + new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), + $this->db, + $this->dispatcher, + $this->language, + $this->template, + $this->user, + $this->config, + $this->symfony_request, + $this->request, + $this->routing_helper, + $this->admin_path, + $this->php_ext + ); static::assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id), $description); } @@ -274,7 +323,22 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case { $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->template, $this->user, $this->config, $this->symfony_request, $this->request, $this->routing_helper); + $this->helper = new phpbb_mock_controller_helper( + $this->auth, + $this->cache, + new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), + $this->db, + $this->dispatcher, + $this->language, + $this->template, + $this->user, + $this->config, + $this->symfony_request, + $this->request, + $this->routing_helper, + $this->admin_path, + $this->php_ext + ); static::assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id, UrlGeneratorInterface::ABSOLUTE_URL), $description); } @@ -318,7 +382,22 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case { $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->template, $this->user, $this->config, $this->symfony_request, $this->request, $this->routing_helper); + $this->helper = new phpbb_mock_controller_helper( + $this->auth, + $this->cache, + new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), + $this->db, + $this->dispatcher, + $this->language, + $this->template, + $this->user, + $this->config, + $this->symfony_request, + $this->request, + $this->routing_helper, + $this->admin_path, + $this->php_ext + ); static::assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id, UrlGeneratorInterface::RELATIVE_PATH), $description); } @@ -362,7 +441,22 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case { $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->template, $this->user, $this->config, $this->symfony_request, $this->request, $this->routing_helper); + $this->helper = new phpbb_mock_controller_helper( + $this->auth, + $this->cache, + new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), + $this->db, + $this->dispatcher, + $this->language, + $this->template, + $this->user, + $this->config, + $this->symfony_request, + $this->request, + $this->routing_helper, + $this->admin_path, + $this->php_ext + ); static::assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id, UrlGeneratorInterface::NETWORK_PATH), $description); } @@ -406,7 +500,22 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case { $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->template, $this->user, $this->config, $this->symfony_request, $this->request, $this->routing_helper); + $this->helper = new phpbb_mock_controller_helper( + $this->auth, + $this->cache, + new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), + $this->db, + $this->dispatcher, + $this->language, + $this->template, + $this->user, + $this->config, + $this->symfony_request, + $this->request, + $this->routing_helper, + $this->admin_path, + $this->php_ext + ); static::assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id, UrlGeneratorInterface::ABSOLUTE_URL), $description); } @@ -447,7 +556,22 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case { $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->template, $this->user, $this->config, $this->symfony_request, $this->request, $this->routing_helper); + $this->helper = new phpbb_mock_controller_helper( + $this->auth, + $this->cache, + new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), + $this->db, + $this->dispatcher, + $this->language, + $this->template, + $this->user, + $this->config, + $this->symfony_request, + $this->request, + $this->routing_helper, + $this->admin_path, + $this->php_ext + ); static::assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id, UrlGeneratorInterface::RELATIVE_PATH), $description); } @@ -491,7 +615,22 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case { $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->template, $this->user, $this->config, $this->symfony_request, $this->request, $this->routing_helper); + $this->helper = new phpbb_mock_controller_helper( + $this->auth, + $this->cache, + new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), + $this->db, + $this->dispatcher, + $this->language, + $this->template, + $this->user, + $this->config, + $this->symfony_request, + $this->request, + $this->routing_helper, + $this->admin_path, + $this->php_ext + ); static::assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id, UrlGeneratorInterface::NETWORK_PATH), $description); } @@ -523,7 +662,22 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case )); $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->template, $this->user, $this->config, $this->symfony_request, $this->request, $this->routing_helper); + $this->helper = new phpbb_mock_controller_helper( + $this->auth, + $this->cache, + new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), + $this->db, + $this->dispatcher, + $this->language, + $this->template, + $this->user, + $this->config, + $this->symfony_request, + $this->request, + $this->routing_helper, + $this->admin_path, + $this->php_ext + ); static::assertEquals($expected, $this->helper->route('controller1', array(), false, false, $type)); } } diff --git a/tests/pagination/pagination_test.php b/tests/pagination/pagination_test.php index 073ba1d5cd..7ee7bc14d7 100644 --- a/tests/pagination/pagination_test.php +++ b/tests/pagination/pagination_test.php @@ -42,7 +42,6 @@ class phpbb_pagination_pagination_test extends phpbb_template_template_test_case $filesystem = new \phpbb\filesystem\filesystem(); $manager = new phpbb_mock_extension_manager(dirname(__FILE__) . '/', array()); - $loader = new \Symfony\Component\Routing\Loader\YamlFileLoader( new \phpbb\routing\file_locator($filesystem, dirname(__FILE__) . '/') ); @@ -58,8 +57,27 @@ class phpbb_pagination_pagination_test extends phpbb_template_template_test_case $request ); + $db = $this->getMockBuilder('\phpbb\db\driver\mysqli') + ->disableOriginalConstructor() + ->getMock(); + $this->routing_helper = new \phpbb\routing\helper($this->config, $router, $symfony_request, $request, $filesystem, '', 'php'); - $this->helper = new phpbb_mock_controller_helper($this->template, $this->user, $this->config, $symfony_request, $request, $this->routing_helper); + $this->helper = new phpbb_mock_controller_helper( + new \phpbb\auth\auth(), + new \phpbb\cache\driver\dummy(), + new \phpbb\cron\manager([], $this->routing_helper, '', 'php'), + $db, + new phpbb_mock_event_dispatcher(), + new \phpbb\language\language(new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx)), + $this->template, + $this->user, + $this->config, + $symfony_request, + $request, + $this->routing_helper, + '', + 'php' + ); $this->pagination = new \phpbb\pagination($this->template, $this->user, $this->helper, $phpbb_dispatcher); } From 2bfea200079d83925b39a1d18a49f242bad2920b Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 19 Jan 2020 21:43:07 +0100 Subject: [PATCH 2/4] [ticket/16314] Create correct adm relative path for controller helper PHPBB3-16314 --- phpBB/config/default/container/services.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/config/default/container/services.yml b/phpBB/config/default/container/services.yml index 739341ab49..eb884ace5f 100644 --- a/phpBB/config/default/container/services.yml +++ b/phpBB/config/default/container/services.yml @@ -97,7 +97,7 @@ services: - '@symfony_request' - '@request' - '@routing.helper' - - '%core.adm_relative_path%' + - '%core.root_path%%core.adm_relative_path%' - '%core.php_ext%' - '%debug.sql_explain%' From 8bdad7d73e6c0876110bee033bfa382d16856c1d Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 20 Jan 2020 17:28:37 +0100 Subject: [PATCH 3/4] [ticket/16314] Reorder controller helper constructor parameters PHPBB3-16314 --- phpBB/config/default/container/services.yml | 11 +-- phpBB/phpbb/controller/helper.php | 55 ++++++-------- tests/controller/common_helper_route.php | 83 ++++++++++++--------- tests/pagination/pagination_test.php | 11 +-- 4 files changed, 81 insertions(+), 79 deletions(-) diff --git a/phpBB/config/default/container/services.yml b/phpBB/config/default/container/services.yml index eb884ace5f..9e2af7df23 100644 --- a/phpBB/config/default/container/services.yml +++ b/phpBB/config/default/container/services.yml @@ -87,17 +87,18 @@ services: arguments: - '@auth' - '@cache.driver' + - '@config' - '@cron.manager' - '@dbal.conn' - '@dispatcher' - '@language' - - '@template' - - '@user' - - '@config' - - '@symfony_request' - '@request' - '@routing.helper' - - '%core.root_path%%core.adm_relative_path%' + - '@symfony_request' + - '@template' + - '@user' + - '%core.root_path%' + - '%core.adm_relative_path%' - '%core.php_ext%' - '%debug.sql_explain%' diff --git a/phpBB/phpbb/controller/helper.php b/phpBB/phpbb/controller/helper.php index 8e8cd135f6..8afdbd9c8d 100644 --- a/phpBB/phpbb/controller/helper.php +++ b/phpBB/phpbb/controller/helper.php @@ -40,6 +40,9 @@ class helper /** @var cache_interface */ protected $cache; + /** @var config */ + protected $config; + /** @var manager */ protected $cron_manager; @@ -52,34 +55,20 @@ class helper /** @var language */ protected $language; - /** - * Template object - * @var template - */ - protected $template; + /* @var request_interface */ + protected $request; - /** - * User object - * @var user - */ - protected $user; - - /** - * config object - * @var config - */ - protected $config; + /** @var routing_helper */ + protected $routing_helper; /* @var symfony_request */ protected $symfony_request; - /* @var request_interface */ - protected $request; + /** @var template */ + protected $template; - /** - * @var routing_helper - */ - protected $routing_helper; + /** @var user */ + protected $user; /** @var string */ protected $admin_path; @@ -95,23 +84,25 @@ class helper * * @param auth $auth Auth object * @param cache_interface $cache + * @param config $config Config object * @param manager $cron_manager - * @param driver_interface $db Dbal object + * @param driver_interface $db DBAL object * @param dispatcher $dispatcher * @param language $language - * @param template $template Template object - * @param user $user User object - * @param config $config Config object - * @param symfony_request $symfony_request Symfony Request object * @param request_interface $request phpBB request object * @param routing_helper $routing_helper Helper to generate the routes + * @param symfony_request $symfony_request Symfony Request object + * @param template $template Template object + * @param user $user User object + * @param string $root_path phpBB root path * @param string $admin_path Admin path * @param string $php_ext PHP extension * @param bool $sql_explain Flag whether to display sql explain */ - public function __construct(auth $auth, cache_interface $cache, manager $cron_manager, driver_interface $db, - dispatcher $dispatcher, language $language, template $template, user $user, config $config, - symfony_request $symfony_request, request_interface $request, routing_helper $routing_helper, + public function __construct(auth $auth, cache_interface $cache, config $config, manager $cron_manager, + driver_interface $db, dispatcher $dispatcher, language $language, + request_interface $request, routing_helper $routing_helper, + symfony_request $symfony_request, template $template, user $user, $root_path, $admin_path, $php_ext, $sql_explain = false) { $this->auth = $auth; @@ -126,7 +117,7 @@ class helper $this->symfony_request = $symfony_request; $this->request = $request; $this->routing_helper = $routing_helper; - $this->admin_path = $admin_path; + $this->admin_path = $root_path . $admin_path; $this->php_ext = $php_ext; $this->sql_explain = $sql_explain; } @@ -166,7 +157,7 @@ class helper * @param array $params String or array of additional url parameters * @param bool $is_amp Is url using & (true) or & (false) * @param string|bool $session_id Possibility to use a custom session id instead of the global one - * @param bool|string $reference_type The type of reference to be generated (one of the constants) + * @param int $reference_type The type of reference to be generated (one of the constants) * @return string The URL already passed through append_sid() */ public function route($route, array $params = array(), $is_amp = true, $session_id = false, $reference_type = UrlGeneratorInterface::ABSOLUTE_PATH) diff --git a/tests/controller/common_helper_route.php b/tests/controller/common_helper_route.php index a357eddfe9..6945ffb7e9 100644 --- a/tests/controller/common_helper_route.php +++ b/tests/controller/common_helper_route.php @@ -158,7 +158,7 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_ $this->db = $this->new_dbal(); $this->dispatcher = new phpbb_mock_event_dispatcher(); $this->language = new \phpbb\language\language(new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx)); - $this->admin_path = $phpbb_root_path . 'adm/'; + $this->admin_path = 'adm/'; $this->php_ext = $phpEx; // Set correct current phpBB root path @@ -208,16 +208,17 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_ $this->helper = new phpbb_mock_controller_helper( $this->auth, $this->cache, + $this->config, new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), $this->db, $this->dispatcher, $this->language, - $this->template, - $this->user, - $this->config, - $this->symfony_request, $this->request, $this->routing_helper, + $this->symfony_request, + $this->template, + $this->user, + $this->root_path, $this->admin_path, $this->php_ext ); @@ -267,16 +268,17 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_ $this->helper = new phpbb_mock_controller_helper( $this->auth, $this->cache, + $this->config, new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), $this->db, $this->dispatcher, $this->language, - $this->template, - $this->user, - $this->config, - $this->symfony_request, $this->request, $this->routing_helper, + $this->symfony_request, + $this->template, + $this->user, + $this->root_path, $this->admin_path, $this->php_ext ); @@ -326,16 +328,17 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_ $this->helper = new phpbb_mock_controller_helper( $this->auth, $this->cache, + $this->config, new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), $this->db, $this->dispatcher, $this->language, - $this->template, - $this->user, - $this->config, - $this->symfony_request, $this->request, $this->routing_helper, + $this->symfony_request, + $this->template, + $this->user, + $this->root_path, $this->admin_path, $this->php_ext ); @@ -385,16 +388,17 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_ $this->helper = new phpbb_mock_controller_helper( $this->auth, $this->cache, + $this->config, new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), $this->db, $this->dispatcher, $this->language, - $this->template, - $this->user, - $this->config, - $this->symfony_request, $this->request, $this->routing_helper, + $this->symfony_request, + $this->template, + $this->user, + $this->root_path, $this->admin_path, $this->php_ext ); @@ -444,16 +448,17 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_ $this->helper = new phpbb_mock_controller_helper( $this->auth, $this->cache, + $this->config, new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), $this->db, $this->dispatcher, $this->language, - $this->template, - $this->user, - $this->config, - $this->symfony_request, $this->request, $this->routing_helper, + $this->symfony_request, + $this->template, + $this->user, + $this->root_path, $this->admin_path, $this->php_ext ); @@ -503,16 +508,17 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_ $this->helper = new phpbb_mock_controller_helper( $this->auth, $this->cache, + $this->config, new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), $this->db, $this->dispatcher, $this->language, - $this->template, - $this->user, - $this->config, - $this->symfony_request, $this->request, $this->routing_helper, + $this->symfony_request, + $this->template, + $this->user, + $this->root_path, $this->admin_path, $this->php_ext ); @@ -559,16 +565,17 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_ $this->helper = new phpbb_mock_controller_helper( $this->auth, $this->cache, + $this->config, new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), $this->db, $this->dispatcher, $this->language, - $this->template, - $this->user, - $this->config, - $this->symfony_request, $this->request, $this->routing_helper, + $this->symfony_request, + $this->template, + $this->user, + $this->root_path, $this->admin_path, $this->php_ext ); @@ -618,16 +625,17 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_ $this->helper = new phpbb_mock_controller_helper( $this->auth, $this->cache, + $this->config, new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), $this->db, $this->dispatcher, $this->language, - $this->template, - $this->user, - $this->config, - $this->symfony_request, $this->request, $this->routing_helper, + $this->symfony_request, + $this->template, + $this->user, + $this->root_path, $this->admin_path, $this->php_ext ); @@ -665,16 +673,17 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_ $this->helper = new phpbb_mock_controller_helper( $this->auth, $this->cache, + $this->config, new \phpbb\cron\manager([], $this->routing_helper, $this->root_path, 'php'), $this->db, $this->dispatcher, $this->language, - $this->template, - $this->user, - $this->config, - $this->symfony_request, $this->request, $this->routing_helper, + $this->symfony_request, + $this->template, + $this->user, + $this->root_path, $this->admin_path, $this->php_ext ); diff --git a/tests/pagination/pagination_test.php b/tests/pagination/pagination_test.php index 7ee7bc14d7..38d250958d 100644 --- a/tests/pagination/pagination_test.php +++ b/tests/pagination/pagination_test.php @@ -65,17 +65,18 @@ class phpbb_pagination_pagination_test extends phpbb_template_template_test_case $this->helper = new phpbb_mock_controller_helper( new \phpbb\auth\auth(), new \phpbb\cache\driver\dummy(), + $this->config, new \phpbb\cron\manager([], $this->routing_helper, '', 'php'), $db, new phpbb_mock_event_dispatcher(), new \phpbb\language\language(new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx)), - $this->template, - $this->user, - $this->config, - $symfony_request, $request, $this->routing_helper, - '', + $symfony_request, + $this->template, + $this->user, + $phpbb_root_path, + 'adm/', 'php' ); $this->pagination = new \phpbb\pagination($this->template, $this->user, $this->helper, $phpbb_dispatcher); From e6624c1fd54dd29af8769b548e635154b973088d Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 2 Feb 2020 13:48:46 +0100 Subject: [PATCH 4/4] [ticket/16314] Deprecate phpbb_check_and_display_sql_report PHPBB3-16314 --- phpBB/includes/functions.php | 10 ++++++---- phpBB/phpbb/controller/helper.php | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index eb667eed7e..a0369caf33 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -4315,15 +4315,17 @@ function page_header($page_title = '', $display_online_list = false, $item_id = * @param \phpbb\request\request_interface $request Request object * @param \phpbb\auth\auth $auth Auth object * @param \phpbb\db\driver\driver_interface $db Database connection + * + * @deprecated 3.3.1 (To be removed: 4.0.0-a1); use controller helper's display_sql_report() */ function phpbb_check_and_display_sql_report(\phpbb\request\request_interface $request, \phpbb\auth\auth $auth, \phpbb\db\driver\driver_interface $db) { global $phpbb_container; - if ($phpbb_container->getParameter('debug.sql_explain') && $request->variable('explain', false) && $auth->acl_get('a_')) - { - $db->sql_report('display'); - } + /** @var \phpbb\controller\helper $controller_helper */ + $controller_helper = $phpbb_container->get('controller.helper'); + + $controller_helper->display_sql_report(); } /** diff --git a/phpBB/phpbb/controller/helper.php b/phpBB/phpbb/controller/helper.php index 8afdbd9c8d..1b07ab9f7d 100644 --- a/phpBB/phpbb/controller/helper.php +++ b/phpBB/phpbb/controller/helper.php @@ -274,7 +274,7 @@ class helper * * @return void */ - protected function display_sql_report() + public function display_sql_report() { if ($this->sql_explain && $this->request->variable('explain', false) && $this->auth->acl_get('a_')) {