From 1d7543c7787ff66f60f863f98fd551e23b994e30 Mon Sep 17 00:00:00 2001 From: rxu Date: Sun, 15 Jun 2025 01:00:37 +0700 Subject: [PATCH] [ticket/15214] Address code review issues, add equal priority events test PHPBB3-15214 --- phpBB/phpbb/template/twig/node/event.php | 60 +++++++++---------- .../phpbb/template/twig/tokenparser/event.php | 39 ++---------- .../extension_template_event_order_test.php | 44 +++++++++----- .../index_body_forumlist_body_before.html | 1 + .../index_body_forumlist_body_before.html | 1 + 5 files changed, 64 insertions(+), 81 deletions(-) create mode 100644 tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/event/index_body_forumlist_body_before.html create mode 100644 tests/functional/fixtures/ext/foo/foo/styles/prosilver/template/event/index_body_forumlist_body_before.html diff --git a/phpBB/phpbb/template/twig/node/event.php b/phpBB/phpbb/template/twig/node/event.php index 4b506377a2..1334fe2ad2 100644 --- a/phpBB/phpbb/template/twig/node/event.php +++ b/phpBB/phpbb/template/twig/node/event.php @@ -46,49 +46,47 @@ class event extends \Twig\Node\Node $location = $this->listener_directory . $this->getNode('expr')->getAttribute('name'); - $template_events = []; + $template_event_listeners = []; // Group and sort extension template events in according to their priority (0 by default if not set) foreach ($this->environment->get_phpbb_extensions() as $ext_namespace => $ext_path) { $ext_namespace = str_replace('/', '_', $ext_namespace); - $priority_key = $this->template_event_priority_array[$ext_namespace][$location] ?? 0; - $template_events[$priority_key][] = $ext_namespace; + $priority_key = intval($this->template_event_priority_array[$ext_namespace][$location] ?? 0); + $template_event_listeners[$priority_key][] = $ext_namespace; } - krsort($template_events); + krsort($template_event_listeners); - foreach ($template_events as $events) + $template_event_listeners = array_merge(...$template_event_listeners); + foreach ($template_event_listeners as $ext_namespace) { - foreach ($events as $ext_namespace) + if ($this->environment->isDebug()) { - if ($this->environment->isDebug()) - { - // If debug mode is enabled, lets check for new/removed EVENT - // templates on page load rather than at compile. This is - // slower, but makes developing extensions easier (no need to - // purge the cache when a new event template file is added) - $compiler - ->write("if (\$this->env->getLoader()->exists('@{$ext_namespace}/{$location}.html')) {\n") - ->indent(); - } + // If debug mode is enabled, lets check for new/removed EVENT + // templates on page load rather than at compile. This is + // slower, but makes developing extensions easier (no need to + // purge the cache when a new event template file is added) + $compiler + ->write("if (\$this->env->getLoader()->exists('@{$ext_namespace}/{$location}.html')) {\n") + ->indent(); + } - if ($this->environment->isDebug() || $this->environment->getLoader()->exists('@' . $ext_namespace . '/' . $location . '.html')) - { - $compiler - ->write("\$previous_look_up_order = \$this->env->getNamespaceLookUpOrder();\n") + if ($this->environment->isDebug() || $this->environment->getLoader()->exists('@' . $ext_namespace . '/' . $location . '.html')) + { + $compiler + ->write("\$previous_look_up_order = \$this->env->getNamespaceLookUpOrder();\n") - // We set the namespace lookup order to be this extension first, then the main path - ->write("\$this->env->setNamespaceLookUpOrder(array('{$ext_namespace}', '__main__'));\n") - ->write("\$this->env->loadTemplate(\$this->env->getTemplateClass('@{$ext_namespace}/{$location}.html'), '@{$ext_namespace}/{$location}.html')->display(\$context);\n") - ->write("\$this->env->setNamespaceLookUpOrder(\$previous_look_up_order);\n"); - } + // We set the namespace lookup order to be this extension first, then the main path + ->write("\$this->env->setNamespaceLookUpOrder(array('{$ext_namespace}', '__main__'));\n") + ->write("\$this->env->loadTemplate(\$this->env->getTemplateClass('@{$ext_namespace}/{$location}.html'), '@{$ext_namespace}/{$location}.html')->display(\$context);\n") + ->write("\$this->env->setNamespaceLookUpOrder(\$previous_look_up_order);\n"); + } - if ($this->environment->isDebug()) - { - $compiler - ->outdent() - ->write("}\n\n"); - } + if ($this->environment->isDebug()) + { + $compiler + ->outdent() + ->write("}\n\n"); } } } diff --git a/phpBB/phpbb/template/twig/tokenparser/event.php b/phpBB/phpbb/template/twig/tokenparser/event.php index 4514e4c698..d5a6cd46e3 100644 --- a/phpBB/phpbb/template/twig/tokenparser/event.php +++ b/phpBB/phpbb/template/twig/tokenparser/event.php @@ -18,9 +18,6 @@ class event extends \Twig\TokenParser\AbstractTokenParser /** @var \phpbb\template\twig\environment */ protected $environment; - /** @var \phpbb\event\dispatcher_interface */ - protected $phpbb_dispatcher; - /** @var array */ protected $template_event_priority_array; @@ -32,49 +29,21 @@ class event extends \Twig\TokenParser\AbstractTokenParser public function __construct(\phpbb\template\twig\environment $environment) { $this->environment = $environment; - $this->phpbb_dispatcher = $this->environment->get_phpbb_dispatcher(); + $phpbb_dispatcher = $this->environment->get_phpbb_dispatcher(); $template_event_priority_array = []; /** - * Allow assigning priority to template events - * - * The higher number - the higher tempate event listener priority value is. - * In case of equal priority values, corresponding template event listeners will be handled in default compilation order. - * If not set, template event listener priority will be assigned to the value of 0. + * Allows assigning priority to template event listeners * * @event core.twig_event_tokenparser_constructor * @var array template_event_priority_array Array with template event priority assignments per extension namespace - * Usage: - * '_' => [ - * 'event/' => priority_number, - * ], - * - * Example: - * class template_event_order implements EventSubscriberInterface - * { - * static public function getSubscribedEvents() - * { - * return [ - * 'core.twig_event_tokenparser_constructor' => 'set_template_event_priority', - * ]; - * } - * - * public function set_template_event_priority($event) - * { - * $template_event_priority_array = $event['template_event_priority_array']; - * $template_event_priority_array['vendor_name'] = [ - * 'event/navbar_header_quick_links_after' => -1, - * ]; - * $event['template_event_priority_array'] = $template_event_priority_array; - * } - * } * * @since 4.0.0-a1 */ - if ($this->phpbb_dispatcher) + if ($phpbb_dispatcher) { $vars = ['template_event_priority_array']; - extract($this->phpbb_dispatcher->trigger_event('core.twig_event_tokenparser_constructor', compact($vars))); + extract($phpbb_dispatcher->trigger_event('core.twig_event_tokenparser_constructor', compact($vars))); } $this->template_event_priority_array = $template_event_priority_array; diff --git a/tests/functional/extension_template_event_order_test.php b/tests/functional/extension_template_event_order_test.php index 44586fc719..197efce746 100644 --- a/tests/functional/extension_template_event_order_test.php +++ b/tests/functional/extension_template_event_order_test.php @@ -1,19 +1,19 @@ -* @license GNU General Public License, version 2 (GPL-2.0) -* -* For full copyright and license information, please see -* the docs/CREDITS.txt file. -* -*/ + * + * 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. + * + */ /** -* @group functional -*/ + * @group functional + */ class phpbb_functional_extension_template_event_order_test extends phpbb_functional_test_case { static private $helper; @@ -58,9 +58,9 @@ class phpbb_functional_extension_template_event_order_test extends phpbb_functio } /** - * Check a controller for extension foo/bar. - */ - public function test_template_event_order() + * Check extensions template event listener prioritizing + */ + public function test_different_template_event_priority() { global $phpbb_root_path; @@ -88,4 +88,18 @@ class phpbb_functional_extension_template_event_order_test extends phpbb_functio $this->assertStringContainsString('FOO_BAR_QUICK_LINK', $quick_links_menu->filter('li')->eq($quick_links_menu_nodes_count - 4)->filter('span')->text()); $this->assertStringContainsString('FOO_FOO_QUICK_LINK', $quick_links_menu->filter('li')->eq($quick_links_menu_nodes_count - 3)->filter('span')->text()); } + + /** + * Check extensions template event listener equal (default - 0) priority rendering + * Should render in the order of reading listener files from the filesystem + */ + public function test_same_template_event_priority() + { + global $phpbb_root_path; + + $crawler = self::request('GET', 'index.php'); + // Ensure foo/bar template event goes before foo/foo one (assuming they have been read from the filesystem in alphabetical order) + $this->assertStringContainsString('FOO_BAR_FORUMLIST_BODY_BEFORE', $crawler->filter('p[id*="forumlist_body_before"]')->eq(0)->text()); + $this->assertStringContainsString('FOO_FOO_FORUMLIST_BODY_BEFORE', $crawler->filter('p[id*="forumlist_body_before"]')->eq(1)->text()); + } } diff --git a/tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/event/index_body_forumlist_body_before.html b/tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/event/index_body_forumlist_body_before.html new file mode 100644 index 0000000000..2071af8433 --- /dev/null +++ b/tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/event/index_body_forumlist_body_before.html @@ -0,0 +1 @@ +

{{ lang('FOO_BAR_FORUMLIST_BODY_BEFORE') }}

\ No newline at end of file diff --git a/tests/functional/fixtures/ext/foo/foo/styles/prosilver/template/event/index_body_forumlist_body_before.html b/tests/functional/fixtures/ext/foo/foo/styles/prosilver/template/event/index_body_forumlist_body_before.html new file mode 100644 index 0000000000..3b76634e18 --- /dev/null +++ b/tests/functional/fixtures/ext/foo/foo/styles/prosilver/template/event/index_body_forumlist_body_before.html @@ -0,0 +1 @@ +

{{ lang('FOO_FOO_FORUMLIST_BODY_BEFORE') }}

\ No newline at end of file