From d8ae0f5d75ea63b9045a26d9509aa9a9a1c3239a Mon Sep 17 00:00:00 2001 From: toxyy Date: Sun, 3 Dec 2023 16:32:02 -0500 Subject: [PATCH 01/16] [ticket/15214] Add event & functionality for assigning template event priority Event added to allow template events to be assigned priority per extension, event location chosen so that it only fires once. Twig node event class refactored to allow template event priority assignment, compile calls are deferred until all locations are processed per extension namespace. Priority precedence mirrors Symfony priority, with higher numbers being placed at the beginning of the array. Duplicate priority assignment will currently have the later events compiled before the others. PHPBB3-15214 --- .../default/container/services_twig.yml | 1 + phpBB/phpbb/template/twig/extension.php | 9 +++- phpBB/phpbb/template/twig/node/event.php | 51 +++++++++++++++++-- .../phpbb/template/twig/tokenparser/event.php | 28 +++++++++- 4 files changed, 80 insertions(+), 9 deletions(-) diff --git a/phpBB/config/default/container/services_twig.yml b/phpBB/config/default/container/services_twig.yml index 5547ca2608..f229814bcc 100644 --- a/phpBB/config/default/container/services_twig.yml +++ b/phpBB/config/default/container/services_twig.yml @@ -40,6 +40,7 @@ services: - '@template_context' - '@template.twig.environment' - '@language' + - '@dispatcher' tags: - { name: twig.extension } diff --git a/phpBB/phpbb/template/twig/extension.php b/phpBB/phpbb/template/twig/extension.php index 95c710ad7e..fcf1ce52cc 100644 --- a/phpBB/phpbb/template/twig/extension.php +++ b/phpBB/phpbb/template/twig/extension.php @@ -26,18 +26,23 @@ class extension extends \Twig\Extension\AbstractExtension /** @var \phpbb\language\language */ protected $language; + /** @var \phpbb\event\dispatcher_interface */ + protected $phpbb_dispatcher; + /** * Constructor * * @param \phpbb\template\context $context * @param \phpbb\template\twig\environment $environment * @param \phpbb\language\language $language + * @param \phpbb\event\dispatcher_interface $phpbb_dispatcher */ - public function __construct(\phpbb\template\context $context, \phpbb\template\twig\environment $environment, $language) + public function __construct(\phpbb\template\context $context, \phpbb\template\twig\environment $environment, $language, \phpbb\event\dispatcher_interface $phpbb_dispatcher) { $this->context = $context; $this->environment = $environment; $this->language = $language; + $this->phpbb_dispatcher = $phpbb_dispatcher; } /** @@ -62,7 +67,7 @@ class extension extends \Twig\Extension\AbstractExtension new \phpbb\template\twig\tokenparser\includeparser, new \phpbb\template\twig\tokenparser\includejs, new \phpbb\template\twig\tokenparser\includecss, - new \phpbb\template\twig\tokenparser\event($this->environment), + new \phpbb\template\twig\tokenparser\event($this->environment, $this->phpbb_dispatcher), new \phpbb\template\twig\tokenparser\includephp($this->environment), new \phpbb\template\twig\tokenparser\php($this->environment), ); diff --git a/phpBB/phpbb/template/twig/node/event.php b/phpBB/phpbb/template/twig/node/event.php index 9766750500..e871b39b9f 100644 --- a/phpBB/phpbb/template/twig/node/event.php +++ b/phpBB/phpbb/template/twig/node/event.php @@ -21,12 +21,16 @@ class event extends \Twig\Node\Node */ protected $listener_directory = 'event/'; - /** @var \Twig\Environment */ + /** @var \phpbb\template\twig\environment */ protected $environment; - public function __construct(\Twig\Node\Expression\AbstractExpression $expr, \phpbb\template\twig\environment $environment, $lineno, $tag = null) + /** @var array */ + protected $template_event_priority_array; + + public function __construct(\Twig\Node\Expression\AbstractExpression $expr, \phpbb\template\twig\environment $environment, $lineno, $tag = null, $template_event_priority_array = []) { $this->environment = $environment; + $this->template_event_priority_array = $template_event_priority_array; parent::__construct(array('expr' => $expr), array(), $lineno, $tag); } @@ -42,17 +46,26 @@ class event extends \Twig\Node\Node $location = $this->listener_directory . $this->getNode('expr')->getAttribute('name'); + $compiler_steps = []; + foreach ($this->environment->get_phpbb_extensions() as $ext_namespace => $ext_path) { $ext_namespace = str_replace('/', '_', $ext_namespace); + if (isset($this->template_event_priority_array[$ext_namespace][$location])) + { + $priority_key = $this->template_event_priority_array[$ext_namespace][$location]; + } + + $compiler_calls = []; + 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 + $compiler_calls[] = fn() => $compiler ->write("if (\$this->env->getLoader()->exists('@{$ext_namespace}/{$location}.html')) {\n") ->indent() ; @@ -60,7 +73,7 @@ class event extends \Twig\Node\Node if ($this->environment->isDebug() || $this->environment->getLoader()->exists('@' . $ext_namespace . '/' . $location . '.html')) { - $compiler + $compiler_calls[] = fn() => $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 @@ -72,11 +85,39 @@ class event extends \Twig\Node\Node if ($this->environment->isDebug()) { - $compiler + $compiler_calls[] = fn() => $compiler ->outdent() ->write("}\n\n") ; } + + if (!empty($compiler_calls)) + { + if (isset($priority_key)) + { + if (array_key_exists($priority_key, $compiler_steps)) + { + array_splice($compiler_steps, $priority_key, 0, [$compiler_calls]); + } + else + { + $compiler_steps[$priority_key] = $compiler_calls; + } + } + else + { + array_unshift($compiler_steps, $compiler_calls); + } + } + } + + krsort($compiler_steps); + foreach ($compiler_steps as $ext_namespace_steps) + { + foreach ($ext_namespace_steps as $step) + { + $step(); + } } } } diff --git a/phpBB/phpbb/template/twig/tokenparser/event.php b/phpBB/phpbb/template/twig/tokenparser/event.php index 7b9742cc95..3dadc7e3b3 100644 --- a/phpBB/phpbb/template/twig/tokenparser/event.php +++ b/phpBB/phpbb/template/twig/tokenparser/event.php @@ -18,14 +18,38 @@ 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; + /** * Constructor * * @param \phpbb\template\twig\environment $environment */ - public function __construct(\phpbb\template\twig\environment $environment) + public function __construct(\phpbb\template\twig\environment $environment, \phpbb\event\dispatcher_interface $phpbb_dispatcher = null) { $this->environment = $environment; + $this->phpbb_dispatcher = $phpbb_dispatcher; + + $template_event_priority_array = []; + + /** + * Allow assigning priority to template events + * + * @event core.twig_tokenparser_constructor + * @var array template_event_priority_array Array with template event priority assignments per extension namespace + * @since 3.3.12-RC1 + */ + if ($this->phpbb_dispatcher) + { + $vars = array('template_event_priority_array'); + extract($this->phpbb_dispatcher->trigger_event('core.twig_tokenparser_constructor', compact($vars))); + } + + $this->template_event_priority_array = $template_event_priority_array; } /** @@ -42,7 +66,7 @@ class event extends \Twig\TokenParser\AbstractTokenParser $stream = $this->parser->getStream(); $stream->expect(\Twig\Token::BLOCK_END_TYPE); - return new \phpbb\template\twig\node\event($expr, $this->environment, $token->getLine(), $this->getTag()); + return new \phpbb\template\twig\node\event($expr, $this->environment, $token->getLine(), $this->getTag(), $this->template_event_priority_array); } /** From 12fd385a4cfcbdd4ef3bc2776af713c7a9585152 Mon Sep 17 00:00:00 2001 From: toxyy Date: Sun, 3 Dec 2023 16:52:15 -0500 Subject: [PATCH 02/16] [ticket/15214] Test fix for test_helper_url_no_rewrite Add new dispatch parameter to the template\twig\extension call PHPBB3-15214 --- tests/controller/common_helper_route.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/controller/common_helper_route.php b/tests/controller/common_helper_route.php index be755593c7..6b01a13144 100644 --- a/tests/controller/common_helper_route.php +++ b/tests/controller/common_helper_route.php @@ -119,6 +119,7 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_ $cache_path = $phpbb_root_path . 'cache/twig'; $context = new \phpbb\template\context(); $loader = new \phpbb\template\twig\loader($this->filesystem, ''); + $this->dispatcher = new \phpbb\event\dispatcher($container); $twig = new \phpbb\template\twig\environment( $this->config, $this->filesystem, @@ -126,7 +127,7 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_ $cache_path, null, $loader, - new \phpbb\event\dispatcher($container), + $this->dispatcher, array( 'cache' => false, 'debug' => false, @@ -134,7 +135,7 @@ abstract class phpbb_controller_common_helper_route extends phpbb_database_test_ 'autoescape' => false, ) ); - $this->template = new phpbb\template\twig\twig($this->phpbb_path_helper, $this->config, $context, $twig, $cache_path, $this->user, array(new \phpbb\template\twig\extension($context, $twig, $this->user))); + $this->template = new phpbb\template\twig\twig($this->phpbb_path_helper, $this->config, $context, $twig, $cache_path, $this->user, array(new \phpbb\template\twig\extension($context, $twig, $this->user, $this->dispatcher))); $twig->setLexer(new \phpbb\template\twig\lexer($twig)); $this->extension_manager = new phpbb_mock_extension_manager( From 2c1e43a834d22ad82cd73451917e15f54b6b30d6 Mon Sep 17 00:00:00 2001 From: toxyy Date: Sun, 3 Dec 2023 16:57:24 -0500 Subject: [PATCH 03/16] [ticket/15214] Test fix for test_bbcode_firstpass Add new dispatch parameter to the template\twig\extension call PHPBB3-15214 --- tests/email/email_parsing_test.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/email/email_parsing_test.php b/tests/email/email_parsing_test.php index a812f88fba..85b9e490f3 100644 --- a/tests/email/email_parsing_test.php +++ b/tests/email/email_parsing_test.php @@ -68,6 +68,7 @@ class phpbb_email_parsing_test extends phpbb_test_case $phpbb_container->set('ext.manager', $extension_manager); $context = new \phpbb\template\context(); + $dispatcher = new \phpbb\event\dispatcher($phpbb_container); $twig = new \phpbb\template\twig\environment( $config, $filesystem, @@ -75,7 +76,7 @@ class phpbb_email_parsing_test extends phpbb_test_case $cache_path, null, new \phpbb\template\twig\loader($filesystem, ''), - new \phpbb\event\dispatcher($phpbb_container), + $dispatcher, array( 'cache' => false, 'debug' => false, @@ -83,7 +84,7 @@ class phpbb_email_parsing_test extends phpbb_test_case 'autoescape' => false, ) ); - $twig_extension = new \phpbb\template\twig\extension($context, $twig, $lang); + $twig_extension = new \phpbb\template\twig\extension($context, $twig, $lang, $dispatcher); $phpbb_container->set('template.twig.extensions.phpbb', $twig_extension); $twig_extensions_collection = new \phpbb\di\service_collection($phpbb_container); From c7d16febefd3eae6de188885fdca0c6d8f11a938 Mon Sep 17 00:00:00 2001 From: toxyy Date: Sun, 3 Dec 2023 17:30:29 -0500 Subject: [PATCH 04/16] [ticket/15214] Add fixes for various other tests Add new dispatch parameter to the template\twig\extension calls PHPBB3-15214 --- tests/extension/metadata_manager_test.php | 2 +- tests/template/extension_test.php | 5 +++-- tests/template/template_allfolder_test.php | 5 +++-- tests/template/template_events_test.php | 5 +++-- tests/template/template_includecss_test.php | 5 +++-- tests/template/template_test_case.php | 5 +++-- tests/template/template_test_case_with_tree.php | 5 +++-- 7 files changed, 19 insertions(+), 13 deletions(-) diff --git a/tests/extension/metadata_manager_test.php b/tests/extension/metadata_manager_test.php index 2357d593b5..535512e9ed 100644 --- a/tests/extension/metadata_manager_test.php +++ b/tests/extension/metadata_manager_test.php @@ -112,7 +112,7 @@ class phpbb_extension_metadata_manager_test extends phpbb_database_test_case $lang = new \phpbb\language\language($lang_loader); $this->user = new \phpbb\user($lang, '\phpbb\datetime'); - $this->template = new phpbb\template\twig\twig($phpbb_path_helper, $this->config, $context, $twig, $cache_path, $this->user, array(new \phpbb\template\twig\extension($context, $twig, $this->user))); + $this->template = new phpbb\template\twig\twig($phpbb_path_helper, $this->config, $context, $twig, $cache_path, $this->user, array(new \phpbb\template\twig\extension($context, $twig, $this->user, $phpbb_dispatcher))); $twig->setLexer(new \phpbb\template\twig\lexer($twig)); } diff --git a/tests/template/extension_test.php b/tests/template/extension_test.php index 6308928325..759fd6e27d 100644 --- a/tests/template/extension_test.php +++ b/tests/template/extension_test.php @@ -74,6 +74,7 @@ class phpbb_template_extension_test extends phpbb_template_template_test_case $cache_path = $phpbb_root_path . 'cache/twig'; $context = new \phpbb\template\context(); $loader = new \phpbb\template\twig\loader($filesystem); + $dispatcher = new \phpbb\event\dispatcher($phpbb_container); $twig = new \phpbb\template\twig\environment( $config, $filesystem, @@ -81,7 +82,7 @@ class phpbb_template_extension_test extends phpbb_template_template_test_case $cache_path, null, $loader, - new \phpbb\event\dispatcher($phpbb_container), + $dispatcher, array( 'cache' => false, 'debug' => false, @@ -97,7 +98,7 @@ class phpbb_template_extension_test extends phpbb_template_template_test_case $cache_path, $this->user, [ - new \phpbb\template\twig\extension($context, $twig, $this->lang), + new \phpbb\template\twig\extension($context, $twig, $this->lang, $dispatcher), new \phpbb\template\twig\extension\avatar(), new \phpbb\template\twig\extension\config($config), new \phpbb\template\twig\extension\username(), diff --git a/tests/template/template_allfolder_test.php b/tests/template/template_allfolder_test.php index 9765b32845..534c0a84ea 100644 --- a/tests/template/template_allfolder_test.php +++ b/tests/template/template_allfolder_test.php @@ -60,6 +60,7 @@ class phpbb_template_allfolder_test extends phpbb_template_template_test_case $cache_path = $phpbb_root_path . 'cache/twig'; $context = new \phpbb\template\context(); $loader = new \phpbb\template\twig\loader(new \phpbb\filesystem\filesystem(), ''); + $dispatcher = new \phpbb\event\dispatcher($container); $twig = new \phpbb\template\twig\environment( $config, $filesystem, @@ -67,7 +68,7 @@ class phpbb_template_allfolder_test extends phpbb_template_template_test_case $cache_path, $this->extension_manager, $loader, - new \phpbb\event\dispatcher($container), + $dispatcher, array( 'cache' => false, 'debug' => false, @@ -75,7 +76,7 @@ class phpbb_template_allfolder_test extends phpbb_template_template_test_case 'autoescape' => false, ) ); - $this->template = new \phpbb\template\twig\twig($path_helper, $config, $context, $twig, $cache_path, $this->user, array(new \phpbb\template\twig\extension($context, $twig, $this->user)), $this->extension_manager); + $this->template = new \phpbb\template\twig\twig($path_helper, $config, $context, $twig, $cache_path, $this->user, array(new \phpbb\template\twig\extension($context, $twig, $this->user, $dispatcher)), $this->extension_manager); $twig->setLexer(new \phpbb\template\twig\lexer($twig)); $this->template_path = $this->test_path . '/templates'; diff --git a/tests/template/template_events_test.php b/tests/template/template_events_test.php index aa3f772111..d41a70b372 100644 --- a/tests/template/template_events_test.php +++ b/tests/template/template_events_test.php @@ -154,6 +154,7 @@ Zeta test event in all', $cache_path = $phpbb_root_path . 'cache/twig'; $context = new \phpbb\template\context(); $loader = new \phpbb\template\twig\loader(new \phpbb\filesystem\filesystem(), ''); + $dispatcher = new \phpbb\event\dispatcher($container); $twig = new \phpbb\template\twig\environment( $config, $filesystem, @@ -161,7 +162,7 @@ Zeta test event in all', $cache_path, $this->extension_manager, $loader, - new \phpbb\event\dispatcher($container), + $dispatcher, array( 'cache' => false, 'debug' => false, @@ -169,7 +170,7 @@ Zeta test event in all', 'autoescape' => false, ) ); - $this->template = new \phpbb\template\twig\twig($path_helper, $config, $context, $twig, $cache_path, $this->user, array(new \phpbb\template\twig\extension($context, $twig, $this->user)), $this->extension_manager); + $this->template = new \phpbb\template\twig\twig($path_helper, $config, $context, $twig, $cache_path, $this->user, array(new \phpbb\template\twig\extension($context, $twig, $this->user, $dispatcher)), $this->extension_manager); $twig->setLexer(new \phpbb\template\twig\lexer($twig)); $this->template->set_custom_style(((!empty($style_names)) ? $style_names : 'silver'), array($this->template_path)); diff --git a/tests/template/template_includecss_test.php b/tests/template/template_includecss_test.php index c767986f85..8d2d21a220 100644 --- a/tests/template/template_includecss_test.php +++ b/tests/template/template_includecss_test.php @@ -46,6 +46,7 @@ class phpbb_template_template_includecss_test extends phpbb_template_template_te $cache_path = $phpbb_root_path . 'cache/twig'; $context = new \phpbb\template\context(); $loader = new \phpbb\template\twig\loader(new \phpbb\filesystem\filesystem(), ''); + $dispatcher = new \phpbb\event\dispatcher($container); $twig = new \phpbb\template\twig\environment( $config, $filesystem, @@ -53,7 +54,7 @@ class phpbb_template_template_includecss_test extends phpbb_template_template_te $cache_path, null, $loader, - new \phpbb\event\dispatcher($container), + $dispatcher, array( 'cache' => false, 'debug' => false, @@ -68,7 +69,7 @@ class phpbb_template_template_includecss_test extends phpbb_template_template_te $twig, $cache_path, $this->user, - array(new \phpbb\template\twig\extension($context, $twig, $this->user)), + array(new \phpbb\template\twig\extension($context, $twig, $this->user, $dispatcher)), new phpbb_mock_extension_manager( __DIR__ . '/', array( diff --git a/tests/template/template_test_case.php b/tests/template/template_test_case.php index 1cb6cbe799..6d819e8842 100644 --- a/tests/template/template_test_case.php +++ b/tests/template/template_test_case.php @@ -98,6 +98,7 @@ class phpbb_template_template_test_case extends phpbb_test_case $cache_path = $phpbb_root_path . 'cache/twig'; $context = new \phpbb\template\context(); $loader = new \phpbb\template\twig\loader(new \phpbb\filesystem\filesystem(), ''); + $dispatcher = new \phpbb\event\dispatcher($container); $twig = new \phpbb\template\twig\environment( $config, $filesystem, @@ -105,7 +106,7 @@ class phpbb_template_template_test_case extends phpbb_test_case $cache_path, null, $loader, - new \phpbb\event\dispatcher($container), + $dispatcher, array( 'cache' => false, 'debug' => false, @@ -113,7 +114,7 @@ class phpbb_template_template_test_case extends phpbb_test_case 'autoescape' => false, ) ); - $this->template = new phpbb\template\twig\twig($path_helper, $config, $context, $twig, $cache_path, $this->user, array(new \phpbb\template\twig\extension($context, $twig, $this->user))); + $this->template = new phpbb\template\twig\twig($path_helper, $config, $context, $twig, $cache_path, $this->user, array(new \phpbb\template\twig\extension($context, $twig, $this->user, $dispatcher))); $twig->setLexer(new \phpbb\template\twig\lexer($twig)); $this->template->set_custom_style('tests', $this->template_path); } diff --git a/tests/template/template_test_case_with_tree.php b/tests/template/template_test_case_with_tree.php index 4a1764053b..339b5d2feb 100644 --- a/tests/template/template_test_case_with_tree.php +++ b/tests/template/template_test_case_with_tree.php @@ -41,6 +41,7 @@ class phpbb_template_template_test_case_with_tree extends phpbb_template_templat $cache_path = $phpbb_root_path . 'cache/twig'; $context = new \phpbb\template\context(); $loader = new \phpbb\template\twig\loader(new \phpbb\filesystem\filesystem(), ''); + $dispatcher = new \phpbb\event\dispatcher($container); $twig = new \phpbb\template\twig\environment( $config, $filesystem, @@ -48,7 +49,7 @@ class phpbb_template_template_test_case_with_tree extends phpbb_template_templat $cache_path, null, $loader, - new \phpbb\event\dispatcher($container), + $dispatcher, array( 'cache' => false, 'debug' => false, @@ -56,7 +57,7 @@ class phpbb_template_template_test_case_with_tree extends phpbb_template_templat 'autoescape' => false, ) ); - $this->template = new phpbb\template\twig\twig($this->phpbb_path_helper, $config, $context, $twig, $cache_path, $this->user, array(new \phpbb\template\twig\extension($context, $twig, $this->user))); + $this->template = new phpbb\template\twig\twig($this->phpbb_path_helper, $config, $context, $twig, $cache_path, $this->user, array(new \phpbb\template\twig\extension($context, $twig, $this->user, $dispatcher))); $twig->setLexer(new \phpbb\template\twig\lexer($twig)); $this->template->set_custom_style('tests', array($this->template_path, $this->parent_template_path)); } From 8d6d016a5a7089e6ff39f4657da24c183978f11a Mon Sep 17 00:00:00 2001 From: toxyy Date: Sun, 3 Dec 2023 17:44:38 -0500 Subject: [PATCH 05/16] [ticket/15214] Replace arrow functions with anonymous functions Arrow functions aren't added until PHP 7.4, so we can't use them yet. Anonymous functions have been added since PHP 5.3 PHPBB3-15214 --- phpBB/phpbb/template/twig/node/event.php | 36 ++++++++++++++---------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/phpBB/phpbb/template/twig/node/event.php b/phpBB/phpbb/template/twig/node/event.php index e871b39b9f..c3700459f0 100644 --- a/phpBB/phpbb/template/twig/node/event.php +++ b/phpBB/phpbb/template/twig/node/event.php @@ -65,30 +65,36 @@ class event extends \Twig\Node\Node // 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_calls[] = fn() => $compiler - ->write("if (\$this->env->getLoader()->exists('@{$ext_namespace}/{$location}.html')) {\n") - ->indent() - ; + $compiler_calls[] = function() use($compiler, $ext_namespace, $location) { + $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_calls[] = fn() => $compiler - ->write("\$previous_look_up_order = \$this->env->getNamespaceLookUpOrder();\n") + $compiler_calls[] = function() use($compiler, $ext_namespace, $location) { + $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('@{$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('@{$ext_namespace}/{$location}.html')->display(\$context);\n") + ->write("\$this->env->setNamespaceLookUpOrder(\$previous_look_up_order);\n") + ; + }; } if ($this->environment->isDebug()) { - $compiler_calls[] = fn() => $compiler - ->outdent() - ->write("}\n\n") - ; + $compiler_calls[] = function() use($compiler) { + $compiler + ->outdent() + ->write("}\n\n") + ; + }; } if (!empty($compiler_calls)) From 31fb27e8d88a1ad51ce1dbdeb5845054e14262bb Mon Sep 17 00:00:00 2001 From: toxyy Date: Mon, 4 Dec 2023 15:31:13 -0500 Subject: [PATCH 06/16] [ticket/15214] Move setting $priority_key to a more appropriate place It doesn't need to be set at the top and makes more sense to set it at the bottom where it's used. Also removes a redundant check PHPBB3-15214 --- phpBB/phpbb/template/twig/node/event.php | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/phpBB/phpbb/template/twig/node/event.php b/phpBB/phpbb/template/twig/node/event.php index c3700459f0..9276efaa90 100644 --- a/phpBB/phpbb/template/twig/node/event.php +++ b/phpBB/phpbb/template/twig/node/event.php @@ -52,11 +52,6 @@ class event extends \Twig\Node\Node { $ext_namespace = str_replace('/', '_', $ext_namespace); - if (isset($this->template_event_priority_array[$ext_namespace][$location])) - { - $priority_key = $this->template_event_priority_array[$ext_namespace][$location]; - } - $compiler_calls = []; if ($this->environment->isDebug()) @@ -99,8 +94,10 @@ class event extends \Twig\Node\Node if (!empty($compiler_calls)) { - if (isset($priority_key)) + if (isset($this->template_event_priority_array[$ext_namespace][$location])) { + $priority_key = $this->template_event_priority_array[$ext_namespace][$location]; + if (array_key_exists($priority_key, $compiler_steps)) { array_splice($compiler_steps, $priority_key, 0, [$compiler_calls]); From e1f597caff4aae0cade51ff04246c563951862a9 Mon Sep 17 00:00:00 2001 From: toxyy Date: Tue, 5 Dec 2023 03:07:12 -0500 Subject: [PATCH 07/16] [ticket/15214] Provide usage example within event docblock Adds similar usage examples like the event core.permissions has PHPBB3-15214 --- phpBB/phpbb/template/twig/tokenparser/event.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/phpBB/phpbb/template/twig/tokenparser/event.php b/phpBB/phpbb/template/twig/tokenparser/event.php index 3dadc7e3b3..bfc2c573fa 100644 --- a/phpBB/phpbb/template/twig/tokenparser/event.php +++ b/phpBB/phpbb/template/twig/tokenparser/event.php @@ -40,7 +40,15 @@ class event extends \Twig\TokenParser\AbstractTokenParser * Allow assigning priority to template events * * @event core.twig_tokenparser_constructor - * @var array template_event_priority_array Array with template event priority assignments per extension namespace + * @var array template_event_priority_array Array with template event priority assignments per extension namespace, usage: + * '_' => array( + * 'event/' => priority_number, + * ), + * Example: + * 'phpbb_viglink' => array( + * 'event/acp_help_phpbb_stats_after' => 80, + * 'event/overall_footer_after' => 100, + * ), * @since 3.3.12-RC1 */ if ($this->phpbb_dispatcher) From 64b9aa2543bff0f4f4dad962c66a5933ea61a4e3 Mon Sep 17 00:00:00 2001 From: toxyy Date: Tue, 5 Dec 2023 03:32:57 -0500 Subject: [PATCH 08/16] [ticket/15214] Update block, restart tests Make docblock look a bit cleaner and restart the tests PHPBB3-15214 --- phpBB/phpbb/template/twig/tokenparser/event.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/phpBB/phpbb/template/twig/tokenparser/event.php b/phpBB/phpbb/template/twig/tokenparser/event.php index bfc2c573fa..7c11d32b69 100644 --- a/phpBB/phpbb/template/twig/tokenparser/event.php +++ b/phpBB/phpbb/template/twig/tokenparser/event.php @@ -38,9 +38,10 @@ class event extends \Twig\TokenParser\AbstractTokenParser /** * Allow assigning priority to template events - * + *f * @event core.twig_tokenparser_constructor - * @var array template_event_priority_array Array with template event priority assignments per extension namespace, usage: + * @var array template_event_priority_array Array with template event priority assignments per extension namespace + * Usage: * '_' => array( * 'event/' => priority_number, * ), From 9603924e5c20aa0f0cffa418e79f55056beab777 Mon Sep 17 00:00:00 2001 From: rxu Date: Fri, 8 Dec 2023 18:47:56 +0700 Subject: [PATCH 09/16] [ticket/15214] Optimize event node code and add template event order tests PHPBB3-15214 --- phpBB/phpbb/template/twig/node/event.php | 83 ++++++------------ .../extension_template_event_order_test.php | 86 +++++++++++++++++++ .../fixtures/ext/foo/bar/config/services.yml | 6 ++ .../foo/bar/event/template_event_order.php | 38 ++++++++ .../bar/event/template_event_order_higher.php | 38 ++++++++ .../navbar_header_quick_links_after.html | 1 + .../fixtures/ext/foo/foo/config/services.yml | 5 ++ .../foo/foo/event/template_event_order.php | 38 ++++++++ .../foo/event/template_event_order_lower.php | 38 ++++++++ .../navbar_header_quick_links_after.html | 1 + 10 files changed, 277 insertions(+), 57 deletions(-) create mode 100644 tests/functional/extension_template_event_order_test.php create mode 100644 tests/functional/fixtures/ext/foo/bar/event/template_event_order.php create mode 100644 tests/functional/fixtures/ext/foo/bar/event/template_event_order_higher.php create mode 100644 tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/event/navbar_header_quick_links_after.html create mode 100644 tests/functional/fixtures/ext/foo/foo/event/template_event_order.php create mode 100644 tests/functional/fixtures/ext/foo/foo/event/template_event_order_lower.php create mode 100644 tests/functional/fixtures/ext/foo/foo/styles/prosilver/template/event/navbar_header_quick_links_after.html diff --git a/phpBB/phpbb/template/twig/node/event.php b/phpBB/phpbb/template/twig/node/event.php index 9276efaa90..265c609fbc 100644 --- a/phpBB/phpbb/template/twig/node/event.php +++ b/phpBB/phpbb/template/twig/node/event.php @@ -46,80 +46,49 @@ class event extends \Twig\Node\Node $location = $this->listener_directory . $this->getNode('expr')->getAttribute('name'); - $compiler_steps = []; + $template_events = []; + // 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; + } + krsort($template_events); - $compiler_calls = []; - - if ($this->environment->isDebug()) + foreach ($template_events as $events) + { + foreach ($events as $ext_namespace) { - // 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_calls[] = function() use($compiler, $ext_namespace, $location) { - $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')) + { + 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 ($this->environment->isDebug() || $this->environment->getLoader()->exists('@' . $ext_namespace . '/' . $location . '.html')) - { - $compiler_calls[] = function() use($compiler, $ext_namespace, $location) { $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('@{$ext_namespace}/{$location}.html')->display(\$context);\n") - ->write("\$this->env->setNamespaceLookUpOrder(\$previous_look_up_order);\n") - ; - }; - } + ->write("\$this->env->setNamespaceLookUpOrder(\$previous_look_up_order);\n"); - if ($this->environment->isDebug()) - { - $compiler_calls[] = function() use($compiler) { - $compiler - ->outdent() - ->write("}\n\n") - ; - }; - } - - if (!empty($compiler_calls)) - { - if (isset($this->template_event_priority_array[$ext_namespace][$location])) - { - $priority_key = $this->template_event_priority_array[$ext_namespace][$location]; - - if (array_key_exists($priority_key, $compiler_steps)) + if ($this->environment->isDebug()) { - array_splice($compiler_steps, $priority_key, 0, [$compiler_calls]); - } - else - { - $compiler_steps[$priority_key] = $compiler_calls; + $compiler + ->outdent() + ->write("}\n\n"); } } - else - { - array_unshift($compiler_steps, $compiler_calls); - } - } - } - - krsort($compiler_steps); - foreach ($compiler_steps as $ext_namespace_steps) - { - foreach ($ext_namespace_steps as $step) - { - $step(); } } } diff --git a/tests/functional/extension_template_event_order_test.php b/tests/functional/extension_template_event_order_test.php new file mode 100644 index 0000000000..c931c95bc1 --- /dev/null +++ b/tests/functional/extension_template_event_order_test.php @@ -0,0 +1,86 @@ + +* @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 +*/ +class phpbb_functional_extension_template_event_order_test extends phpbb_functional_test_case +{ + protected $phpbb_extension_manager; + + static private $helper; + + static protected $fixtures = [ + './', + ]; + + static public function setUpBeforeClass(): void + { + parent::setUpBeforeClass(); + + self::$helper = new phpbb_test_case_helpers(__CLASS__); + self::$helper->copy_ext_fixtures(__DIR__ . '/fixtures/ext/', self::$fixtures); + } + + static public function tearDownAfterClass(): void + { + parent::tearDownAfterClass(); + + self::$helper->restore_original_ext_dir(); + } + + protected function setUp(): void + { + parent::setUp(); + + $this->phpbb_extension_manager = $this->get_extension_manager(); + + $this->purge_cache(); + } + + /** + * Check a controller for extension foo/bar. + */ + public function test_template_event_order() + { + global $phpbb_root_path; + + $this->phpbb_extension_manager->enable('foo/bar'); + $this->phpbb_extension_manager->enable('foo/foo'); + $crawler = self::request('GET', 'index.php'); + $quick_links_menu = $crawler->filter('ul[role="menu"]')->eq(0); + $quick_links_menu_nodes_count = (int) $quick_links_menu->filter('li')->count(); + // Ensure foo/foo template event goes before foo/bar one + $this->assertStringContainsString('FOO_FOO_QUICK_LINK', $quick_links_menu->filter('li')->eq($quick_links_menu_nodes_count - 2)->filter('span')->text()); + $this->assertStringContainsString('FOO_BAR_QUICK_LINK', $quick_links_menu->filter('li')->eq($quick_links_menu_nodes_count - 1)->filter('span')->text()); + + // Change template events order to default, put foo/bar event before foo/foo one + $this->phpbb_extension_manager->disable('foo/bar'); + $this->phpbb_extension_manager->disable('foo/foo'); + $this->assertTrue(copy(__DIR__ . '/fixtures/ext/foo/bar/event/template_event_order_higher.php', $phpbb_root_path . 'ext/foo/bar/event/template_event_order.php')); + $this->assertTrue(copy(__DIR__ . '/fixtures/ext/foo/foo/event/template_event_order_lower.php', $phpbb_root_path . 'ext/foo/foo/event/template_event_order.php')); + $this->phpbb_extension_manager->enable('foo/bar'); + $this->phpbb_extension_manager->enable('foo/foo'); + $this->purge_cache(); + sleep(3); + $crawler = self::request('GET', 'index.php'); + $quick_links_menu = $crawler->filter('ul[role="menu"]')->eq(0); + $quick_links_menu_nodes_count = (int) $quick_links_menu->filter('li')->count(); + // Ensure foo/foo template event goes before foo/bar one + $this->assertStringContainsString('FOO_BAR_QUICK_LINK', $quick_links_menu->filter('li')->eq($quick_links_menu_nodes_count - 2)->filter('span')->text()); + $this->assertStringContainsString('FOO_FOO_QUICK_LINK', $quick_links_menu->filter('li')->eq($quick_links_menu_nodes_count - 1)->filter('span')->text()); + + $this->phpbb_extension_manager->purge('foo/bar'); + $this->phpbb_extension_manager->purge('foo/foo'); + } +} diff --git a/tests/functional/fixtures/ext/foo/bar/config/services.yml b/tests/functional/fixtures/ext/foo/bar/config/services.yml index 495c775a1f..cac5f9cd76 100644 --- a/tests/functional/fixtures/ext/foo/bar/config/services.yml +++ b/tests/functional/fixtures/ext/foo/bar/config/services.yml @@ -14,7 +14,13 @@ services: class: foo\bar\event\permission tags: - { name: event.listener } + foo_bar.listener.user_setup: class: foo\bar\event\user_setup tags: - { name: event.listener } + + foo_bar.listener.template_event_order: + class: foo\bar\event\template_event_order + tags: + - { name: event.listener } diff --git a/tests/functional/fixtures/ext/foo/bar/event/template_event_order.php b/tests/functional/fixtures/ext/foo/bar/event/template_event_order.php new file mode 100644 index 0000000000..9cb6a9c71b --- /dev/null +++ b/tests/functional/fixtures/ext/foo/bar/event/template_event_order.php @@ -0,0 +1,38 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +namespace foo\bar\event; + +/** +* Event listener +*/ +use Symfony\Component\EventDispatcher\EventSubscriberInterface; + +class template_event_order implements EventSubscriberInterface +{ + static public function getSubscribedEvents() + { + return array( + 'core.twig_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['foo_bar'] = [ + 'event/navbar_header_quick_links_after' => -1, + ]; + $event['template_event_priority_array'] = $template_event_priority_array; + } +} diff --git a/tests/functional/fixtures/ext/foo/bar/event/template_event_order_higher.php b/tests/functional/fixtures/ext/foo/bar/event/template_event_order_higher.php new file mode 100644 index 0000000000..43d5c05be3 --- /dev/null +++ b/tests/functional/fixtures/ext/foo/bar/event/template_event_order_higher.php @@ -0,0 +1,38 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +namespace foo\bar\event; + +/** +* Event listener +*/ +use Symfony\Component\EventDispatcher\EventSubscriberInterface; + +class template_event_order implements EventSubscriberInterface +{ + static public function getSubscribedEvents() + { + return array( + 'core.twig_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['foo_bar'] = [ + 'event/navbar_header_quick_links_after' => 1, + ]; + $event['template_event_priority_array'] = $template_event_priority_array; + } +} diff --git a/tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/event/navbar_header_quick_links_after.html b/tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/event/navbar_header_quick_links_after.html new file mode 100644 index 0000000000..40be5d2b38 --- /dev/null +++ b/tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/event/navbar_header_quick_links_after.html @@ -0,0 +1 @@ +
  • {{ lang('FOO_BAR_QUICK_LINK') }}
    • \ No newline at end of file diff --git a/tests/functional/fixtures/ext/foo/foo/config/services.yml b/tests/functional/fixtures/ext/foo/foo/config/services.yml index b3c7719715..fc10b65e56 100644 --- a/tests/functional/fixtures/ext/foo/foo/config/services.yml +++ b/tests/functional/fixtures/ext/foo/foo/config/services.yml @@ -1,3 +1,8 @@ services: foo_foo.controller: class: foo\foo\controller\controller + + foo_foo.listener.template_event_order: + class: foo\foo\event\template_event_order + tags: + - { name: event.listener } diff --git a/tests/functional/fixtures/ext/foo/foo/event/template_event_order.php b/tests/functional/fixtures/ext/foo/foo/event/template_event_order.php new file mode 100644 index 0000000000..cba0340e16 --- /dev/null +++ b/tests/functional/fixtures/ext/foo/foo/event/template_event_order.php @@ -0,0 +1,38 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +namespace foo\foo\event; + +/** +* Event listener +*/ +use Symfony\Component\EventDispatcher\EventSubscriberInterface; + +class template_event_order implements EventSubscriberInterface +{ + static public function getSubscribedEvents() + { + return array( + 'core.twig_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['foo_bar'] = [ + 'event/navbar_header_quick_links_after' => 1, + ]; + $event['template_event_priority_array'] = $template_event_priority_array; + } +} diff --git a/tests/functional/fixtures/ext/foo/foo/event/template_event_order_lower.php b/tests/functional/fixtures/ext/foo/foo/event/template_event_order_lower.php new file mode 100644 index 0000000000..6cb1566417 --- /dev/null +++ b/tests/functional/fixtures/ext/foo/foo/event/template_event_order_lower.php @@ -0,0 +1,38 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +namespace foo\foo\event; + +/** +* Event listener +*/ +use Symfony\Component\EventDispatcher\EventSubscriberInterface; + +class template_event_order implements EventSubscriberInterface +{ + static public function getSubscribedEvents() + { + return array( + 'core.twig_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['foo_bar'] = [ + 'event/navbar_header_quick_links_after' => -1, + ]; + $event['template_event_priority_array'] = $template_event_priority_array; + } +} diff --git a/tests/functional/fixtures/ext/foo/foo/styles/prosilver/template/event/navbar_header_quick_links_after.html b/tests/functional/fixtures/ext/foo/foo/styles/prosilver/template/event/navbar_header_quick_links_after.html new file mode 100644 index 0000000000..932c93caff --- /dev/null +++ b/tests/functional/fixtures/ext/foo/foo/styles/prosilver/template/event/navbar_header_quick_links_after.html @@ -0,0 +1 @@ +
  • {{ lang('FOO_FOO_QUICK_LINK') }}
    • \ No newline at end of file From 1e2532a9a93916995339d460f3cc4bb3aaf9e564 Mon Sep 17 00:00:00 2001 From: rxu Date: Sun, 10 Dec 2023 00:37:48 +0700 Subject: [PATCH 10/16] [ticket/15214] Fix test foo/foo extension listener PHPBB3-15214 --- .../fixtures/ext/foo/foo/event/template_event_order.php | 2 +- .../fixtures/ext/foo/foo/event/template_event_order_lower.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functional/fixtures/ext/foo/foo/event/template_event_order.php b/tests/functional/fixtures/ext/foo/foo/event/template_event_order.php index cba0340e16..28652c23fc 100644 --- a/tests/functional/fixtures/ext/foo/foo/event/template_event_order.php +++ b/tests/functional/fixtures/ext/foo/foo/event/template_event_order.php @@ -30,7 +30,7 @@ class template_event_order implements EventSubscriberInterface public function set_template_event_priority($event) { $template_event_priority_array = $event['template_event_priority_array']; - $template_event_priority_array['foo_bar'] = [ + $template_event_priority_array['foo_foo'] = [ 'event/navbar_header_quick_links_after' => 1, ]; $event['template_event_priority_array'] = $template_event_priority_array; diff --git a/tests/functional/fixtures/ext/foo/foo/event/template_event_order_lower.php b/tests/functional/fixtures/ext/foo/foo/event/template_event_order_lower.php index 6cb1566417..e360209481 100644 --- a/tests/functional/fixtures/ext/foo/foo/event/template_event_order_lower.php +++ b/tests/functional/fixtures/ext/foo/foo/event/template_event_order_lower.php @@ -30,7 +30,7 @@ class template_event_order implements EventSubscriberInterface public function set_template_event_priority($event) { $template_event_priority_array = $event['template_event_priority_array']; - $template_event_priority_array['foo_bar'] = [ + $template_event_priority_array['foo_foo'] = [ 'event/navbar_header_quick_links_after' => -1, ]; $event['template_event_priority_array'] = $template_event_priority_array; From 648cd688cac37f709790a64db6b234987cbf24aa Mon Sep 17 00:00:00 2001 From: toxyy Date: Sat, 9 Dec 2023 13:19:13 -0500 Subject: [PATCH 11/16] [ticket/15214] Move if statement to a better place Putting this here will have the loop run the same amount of times as default. Without this, the loop runs too many extra times. PHPBB3-15214 --- phpBB/phpbb/template/twig/node/event.php | 52 ++++++++++++------------ 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/phpBB/phpbb/template/twig/node/event.php b/phpBB/phpbb/template/twig/node/event.php index 265c609fbc..dfb89f1dc9 100644 --- a/phpBB/phpbb/template/twig/node/event.php +++ b/phpBB/phpbb/template/twig/node/event.php @@ -51,9 +51,12 @@ class event extends \Twig\Node\Node // 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; + if ($this->environment->isDebug() || $this->environment->getLoader()->exists('@' . $ext_namespace . '/' . $location . '.html')) + { + $ext_namespace = str_replace('/', '_', $ext_namespace); + $priority_key = $this->template_event_priority_array[$ext_namespace][$location] ?? 0; + $template_events[$priority_key][] = $ext_namespace; + } } krsort($template_events); @@ -61,33 +64,30 @@ class event extends \Twig\Node\Node { foreach ($events as $ext_namespace) { - if ($this->environment->isDebug() || $this->environment->getLoader()->exists('@' . $ext_namespace . '/' . $location . '.html')) + 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("\$previous_look_up_order = \$this->env->getNamespaceLookUpOrder();\n") + ->write("if (\$this->env->getLoader()->exists('@{$ext_namespace}/{$location}.html')) {\n") + ->indent(); + } - // 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('@{$ext_namespace}/{$location}.html')->display(\$context);\n") - ->write("\$this->env->setNamespaceLookUpOrder(\$previous_look_up_order);\n"); + $compiler + ->write("\$previous_look_up_order = \$this->env->getNamespaceLookUpOrder();\n") - if ($this->environment->isDebug()) - { - $compiler - ->outdent() - ->write("}\n\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('@{$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"); } } } From 3714202dd9786bb7bdfb86cac3e5a567c4462176 Mon Sep 17 00:00:00 2001 From: toxyy Date: Sat, 9 Dec 2023 14:50:13 -0500 Subject: [PATCH 12/16] [ticket/15214] Move $ext_namespace str_replace to work Added this when I initially tested the if block move. but I forgot to add it in the actual commit. If this isn't moved, no template events work, and the tests fail. PHPBB3-15214 --- phpBB/phpbb/template/twig/node/event.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/phpbb/template/twig/node/event.php b/phpBB/phpbb/template/twig/node/event.php index dfb89f1dc9..6d8940a91a 100644 --- a/phpBB/phpbb/template/twig/node/event.php +++ b/phpBB/phpbb/template/twig/node/event.php @@ -51,9 +51,9 @@ class event extends \Twig\Node\Node // 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); if ($this->environment->isDebug() || $this->environment->getLoader()->exists('@' . $ext_namespace . '/' . $location . '.html')) { - $ext_namespace = str_replace('/', '_', $ext_namespace); $priority_key = $this->template_event_priority_array[$ext_namespace][$location] ?? 0; $template_events[$priority_key][] = $ext_namespace; } From 96ce8e7363c3a92a2ee74754415ffc9b754aecdb Mon Sep 17 00:00:00 2001 From: rxu Date: Sun, 10 Dec 2023 09:41:42 +0700 Subject: [PATCH 13/16] [ticket/15214] Fix tests PHPBB3-15214 --- tests/functional/extension_controller_test.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/functional/extension_controller_test.php b/tests/functional/extension_controller_test.php index aa408b3b56..da1a6e18ae 100644 --- a/tests/functional/extension_controller_test.php +++ b/tests/functional/extension_controller_test.php @@ -28,6 +28,7 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c 'foo/bar/styles/prosilver/template/', 'foo/foo/config/', 'foo/foo/controller/', + 'foo/foo/event/', ); static public function setUpBeforeClass(): void From 50d4e6ba340d7909406c88889391bb3d702f4c84 Mon Sep 17 00:00:00 2001 From: rxu Date: Sun, 10 Dec 2023 14:02:30 +0700 Subject: [PATCH 14/16] [ticket/15214] Fix Windows tests Purge Twig compiled cache in Windows. Set appropriate folder access control options to do that. PHPBB3-15214 --- .github/workflows/tests.yml | 4 +--- tests/functional/extension_controller_test.php | 7 +++++++ tests/functional/extension_global_lang_test.php | 6 ++++-- tests/functional/extension_module_test.php | 10 ++++++++-- tests/functional/extension_permission_lang_test.php | 9 +++++++++ .../functional/extension_template_event_order_test.php | 7 +++++++ tests/functional/metadata_manager_test.php | 1 + tests/test_framework/phpbb_functional_test_case.php | 2 +- 8 files changed, 38 insertions(+), 8 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index c8c597c01d..a47b67baad 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -543,9 +543,7 @@ jobs: New-WebHandler -Name "PHP-FastCGI" -Path "*.php" -Modules FastCgiModule -ScriptProcessor "C:\tools\php\php-cgi.exe" -Verb '*' -ResourceType Either iisreset NET START W3SVC - mkdir "${env:GITHUB_WORKSPACE}\phpBB\cache\test" - mkdir "${env:GITHUB_WORKSPACE}\phpBB\cache\installer" - icacls "${env:GITHUB_WORKSPACE}\phpBB\cache" /grant Users:F /T + icacls "${env:GITHUB_WORKSPACE}\phpBB\cache" /grant "Users:(OI)(CI)F" /T icacls "${env:GITHUB_WORKSPACE}\phpBB\files" /grant Users:F /T icacls "${env:GITHUB_WORKSPACE}\phpBB\store" /grant Users:F /T icacls "${env:GITHUB_WORKSPACE}\phpBB\images\avatars\upload" /grant Users:F /T diff --git a/tests/functional/extension_controller_test.php b/tests/functional/extension_controller_test.php index da1a6e18ae..0b95b916ad 100644 --- a/tests/functional/extension_controller_test.php +++ b/tests/functional/extension_controller_test.php @@ -55,6 +55,13 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c $this->purge_cache(); } + protected function tearDown(): void + { + $this->purge_cache(); + + parent::tearDown(); + } + /** * Check a controller for extension foo/bar. */ diff --git a/tests/functional/extension_global_lang_test.php b/tests/functional/extension_global_lang_test.php index 8c9c38937b..ae254b3d84 100644 --- a/tests/functional/extension_global_lang_test.php +++ b/tests/functional/extension_global_lang_test.php @@ -54,9 +54,9 @@ class phpbb_functional_extension_global_lang_test extends phpbb_functional_test_ protected function tearDown(): void { - parent::tearDown(); - $this->purge_cache(); + + parent::tearDown(); } public function test_load_extension_lang_globally() @@ -71,5 +71,7 @@ class phpbb_functional_extension_global_lang_test extends phpbb_functional_test_ // language from ext/foo/bar/language/en/foo_global.php $this->assertStringContainsString('Overwritten by foo', $crawler->filter('.skiplink')->text()); + + $this->phpbb_extension_manager->purge('foo/bar'); } } diff --git a/tests/functional/extension_module_test.php b/tests/functional/extension_module_test.php index 8c676361e6..92e116460a 100644 --- a/tests/functional/extension_module_test.php +++ b/tests/functional/extension_module_test.php @@ -40,6 +40,14 @@ class phpbb_functional_extension_module_test extends phpbb_functional_test_case self::$helper->restore_original_ext_dir(); } + protected function tearDown(): void + { + $this->phpbb_extension_manager->purge('foo/bar'); + $this->purge_cache(); + + parent::tearDown(); + } + protected function setUp(): void { global $db; @@ -131,7 +139,5 @@ class phpbb_functional_extension_module_test extends phpbb_functional_test_case $link = $crawler->selectLink('UCP_FOOBAR_TITLE')->link()->getUri(); $crawler = self::request('GET', substr($link, strpos($link, 'ucp.'))); $this->assertStringContainsString('UCP Extension Template Test Passed!', $crawler->filter('#content')->text()); - - $this->phpbb_extension_manager->purge('foo/bar'); } } diff --git a/tests/functional/extension_permission_lang_test.php b/tests/functional/extension_permission_lang_test.php index 47adc3f87e..3aae7685d8 100644 --- a/tests/functional/extension_permission_lang_test.php +++ b/tests/functional/extension_permission_lang_test.php @@ -41,6 +41,13 @@ class phpbb_functional_extension_permission_lang_test extends phpbb_functional_t self::$helper->restore_original_ext_dir(); } + protected function tearDown(): void + { + $this->purge_cache(); + + parent::tearDown(); + } + protected function setUp(): void { parent::setUp(); @@ -82,5 +89,7 @@ class phpbb_functional_extension_permission_lang_test extends phpbb_functional_t // language from ext/foo/bar/language/en/permissions_foo.php $this->assertStringContainsString('Can view foobar', $crawler->filter('body')->text()); + + $this->phpbb_extension_manager->purge('foo/bar'); } } diff --git a/tests/functional/extension_template_event_order_test.php b/tests/functional/extension_template_event_order_test.php index c931c95bc1..c77cb7b89f 100644 --- a/tests/functional/extension_template_event_order_test.php +++ b/tests/functional/extension_template_event_order_test.php @@ -39,6 +39,13 @@ class phpbb_functional_extension_template_event_order_test extends phpbb_functio self::$helper->restore_original_ext_dir(); } + protected function tearDown(): void + { + $this->purge_cache(); + + parent::tearDown(); + } + protected function setUp(): void { parent::setUp(); diff --git a/tests/functional/metadata_manager_test.php b/tests/functional/metadata_manager_test.php index 5ef67836c9..5dff5d6227 100644 --- a/tests/functional/metadata_manager_test.php +++ b/tests/functional/metadata_manager_test.php @@ -26,6 +26,7 @@ class phpbb_functional_metadata_manager_test extends phpbb_functional_test_case protected function tearDown(): void { + $this->phpbb_extension_manager->purge('foo/bar'); $this->purge_cache(); parent::tearDown(); diff --git a/tests/test_framework/phpbb_functional_test_case.php b/tests/test_framework/phpbb_functional_test_case.php index a90008c22e..90cbb40626 100644 --- a/tests/test_framework/phpbb_functional_test_case.php +++ b/tests/test_framework/phpbb_functional_test_case.php @@ -980,7 +980,7 @@ class phpbb_functional_test_case extends phpbb_test_case // Any output before the doc type means there was an error $content = self::get_content(); self::assertStringNotContainsString('[phpBB Debug]', $content); - self::assertStringStartsWith(' Date: Sun, 10 Dec 2023 11:47:36 -0500 Subject: [PATCH 15/16] [ticket/15214] Fix tests again Adding per rxu's recommendation PHPBB3-15214 --- .github/workflows/tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index a47b67baad..0fa9a70701 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -544,6 +544,9 @@ jobs: iisreset NET START W3SVC icacls "${env:GITHUB_WORKSPACE}\phpBB\cache" /grant "Users:(OI)(CI)F" /T + mkdir "${env:GITHUB_WORKSPACE}\phpBB\cache\test" + mkdir "${env:GITHUB_WORKSPACE}\phpBB\cache\test\twig" + mkdir "${env:GITHUB_WORKSPACE}\phpBB\cache\installer" icacls "${env:GITHUB_WORKSPACE}\phpBB\files" /grant Users:F /T icacls "${env:GITHUB_WORKSPACE}\phpBB\store" /grant Users:F /T icacls "${env:GITHUB_WORKSPACE}\phpBB\images\avatars\upload" /grant Users:F /T From 01ae4ed1f7477d35bfba473885bdef2dc7ee5989 Mon Sep 17 00:00:00 2001 From: rxu Date: Sun, 10 Dec 2023 23:24:11 +0700 Subject: [PATCH 16/16] [ticket/15214] Fix Windows tests PHPBB3-15214 --- .github/workflows/tests.yml | 3 +-- tests/functional/extension_controller_test.php | 7 ------- tests/functional/extension_global_lang_test.php | 7 ------- tests/functional/extension_module_test.php | 1 - tests/functional/extension_permission_lang_test.php | 7 ------- tests/functional/extension_template_event_order_test.php | 7 ------- tests/functional/metadata_manager_test.php | 1 - tests/test_framework/phpbb_functional_test_case.php | 3 +++ 8 files changed, 4 insertions(+), 32 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 0fa9a70701..c8c597c01d 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -543,10 +543,9 @@ jobs: New-WebHandler -Name "PHP-FastCGI" -Path "*.php" -Modules FastCgiModule -ScriptProcessor "C:\tools\php\php-cgi.exe" -Verb '*' -ResourceType Either iisreset NET START W3SVC - icacls "${env:GITHUB_WORKSPACE}\phpBB\cache" /grant "Users:(OI)(CI)F" /T mkdir "${env:GITHUB_WORKSPACE}\phpBB\cache\test" - mkdir "${env:GITHUB_WORKSPACE}\phpBB\cache\test\twig" mkdir "${env:GITHUB_WORKSPACE}\phpBB\cache\installer" + icacls "${env:GITHUB_WORKSPACE}\phpBB\cache" /grant Users:F /T icacls "${env:GITHUB_WORKSPACE}\phpBB\files" /grant Users:F /T icacls "${env:GITHUB_WORKSPACE}\phpBB\store" /grant Users:F /T icacls "${env:GITHUB_WORKSPACE}\phpBB\images\avatars\upload" /grant Users:F /T diff --git a/tests/functional/extension_controller_test.php b/tests/functional/extension_controller_test.php index 0b95b916ad..da1a6e18ae 100644 --- a/tests/functional/extension_controller_test.php +++ b/tests/functional/extension_controller_test.php @@ -55,13 +55,6 @@ class phpbb_functional_extension_controller_test extends phpbb_functional_test_c $this->purge_cache(); } - protected function tearDown(): void - { - $this->purge_cache(); - - parent::tearDown(); - } - /** * Check a controller for extension foo/bar. */ diff --git a/tests/functional/extension_global_lang_test.php b/tests/functional/extension_global_lang_test.php index ae254b3d84..58d4547662 100644 --- a/tests/functional/extension_global_lang_test.php +++ b/tests/functional/extension_global_lang_test.php @@ -52,13 +52,6 @@ class phpbb_functional_extension_global_lang_test extends phpbb_functional_test_ $this->purge_cache(); } - protected function tearDown(): void - { - $this->purge_cache(); - - parent::tearDown(); - } - public function test_load_extension_lang_globally() { $this->phpbb_extension_manager->enable('foo/bar'); diff --git a/tests/functional/extension_module_test.php b/tests/functional/extension_module_test.php index 92e116460a..6f48d6122a 100644 --- a/tests/functional/extension_module_test.php +++ b/tests/functional/extension_module_test.php @@ -43,7 +43,6 @@ class phpbb_functional_extension_module_test extends phpbb_functional_test_case protected function tearDown(): void { $this->phpbb_extension_manager->purge('foo/bar'); - $this->purge_cache(); parent::tearDown(); } diff --git a/tests/functional/extension_permission_lang_test.php b/tests/functional/extension_permission_lang_test.php index 3aae7685d8..6099cdcf73 100644 --- a/tests/functional/extension_permission_lang_test.php +++ b/tests/functional/extension_permission_lang_test.php @@ -41,13 +41,6 @@ class phpbb_functional_extension_permission_lang_test extends phpbb_functional_t self::$helper->restore_original_ext_dir(); } - protected function tearDown(): void - { - $this->purge_cache(); - - parent::tearDown(); - } - protected function setUp(): void { parent::setUp(); diff --git a/tests/functional/extension_template_event_order_test.php b/tests/functional/extension_template_event_order_test.php index c77cb7b89f..c931c95bc1 100644 --- a/tests/functional/extension_template_event_order_test.php +++ b/tests/functional/extension_template_event_order_test.php @@ -39,13 +39,6 @@ class phpbb_functional_extension_template_event_order_test extends phpbb_functio self::$helper->restore_original_ext_dir(); } - protected function tearDown(): void - { - $this->purge_cache(); - - parent::tearDown(); - } - protected function setUp(): void { parent::setUp(); diff --git a/tests/functional/metadata_manager_test.php b/tests/functional/metadata_manager_test.php index 5dff5d6227..2c885c5f8f 100644 --- a/tests/functional/metadata_manager_test.php +++ b/tests/functional/metadata_manager_test.php @@ -27,7 +27,6 @@ class phpbb_functional_metadata_manager_test extends phpbb_functional_test_case protected function tearDown(): void { $this->phpbb_extension_manager->purge('foo/bar'); - $this->purge_cache(); parent::tearDown(); } diff --git a/tests/test_framework/phpbb_functional_test_case.php b/tests/test_framework/phpbb_functional_test_case.php index 90cbb40626..3bb99d13cc 100644 --- a/tests/test_framework/phpbb_functional_test_case.php +++ b/tests/test_framework/phpbb_functional_test_case.php @@ -123,6 +123,9 @@ class phpbb_functional_test_case extends phpbb_test_case // Close the database connections again this test $this->db->sql_close(); } + + $this->purge_cache(); + sleep(3); // Give it some time to delete all the files correctly } /**