[ticket/15214] Address code review issues, add equal priority events test

PHPBB3-15214

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;
+			$priority_key = intval($this->template_event_priority_array[$ext_namespace][$location] ?? 0);
-			$template_events[$priority_key][] = $ext_namespace;
+			$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
-					// If debug mode is enabled, lets check for new/removed EVENT
+				//  slower, but makes developing extensions easier (no need to
-					//  templates on page load rather than at compile. This is
+				//  purge the cache when a new event template file is added)
-					//  slower, but makes developing extensions easier (no need to
+				$compiler
-					//  purge the cache when a new event template file is added)
+					->write("if (\$this->env->getLoader()->exists('@{$ext_namespace}/{$location}.html')) {\n")
-					$compiler
+					->indent();
-						->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() || $this->environment->getLoader()->exists('@' . $ext_namespace . '/' . $location . '.html'))
-				{
+			{
-					$compiler
+				$compiler
-						->write("\$previous_look_up_order = \$this->env->getNamespaceLookUpOrder();\n")
+					->write("\$previous_look_up_order = \$this->env->getNamespaceLookUpOrder();\n")
 
-						// We set the namespace lookup order to be this extension first, then the main path
+					// 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->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->loadTemplate(\$this->env->getTemplateClass('@{$ext_namespace}/{$location}.html'), '@{$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())
+			if ($this->environment->isDebug())
-				{
+			{
-					$compiler
+				$compiler
-						->outdent()
+					->outdent()
-						->write("}\n\n");
+					->write("}\n\n");
-				}
 			}
 		}
 	}

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
+		 * Allows assigning priority to template event listeners
-		 *
-		 * 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.
 		 *
 		 * @event core.twig_event_tokenparser_constructor
 		 * @var	array	template_event_priority_array	Array with template event priority assignments per extension namespace
-		 *		Usage:
-		 *		'<author>_<extension_name>' => [
-		 *			'event/<template_event_name>'		=> 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;

tests/functional/extension_template_event_order_test.php

@@ -1,19 +1,19 @@
 <?php
 /**
-*
+ *
-* This file is part of the phpBB Forum Software package.
+ * This file is part of the phpBB Forum Software package.
-*
+ *
-* @copyright (c) phpBB Limited <https://www.phpbb.com>
+ * @copyright (c) phpBB Limited <https://www.phpbb.com>
-* @license GNU General Public License, version 2 (GPL-2.0)
+ * @license GNU General Public License, version 2 (GPL-2.0)
-*
+ *
-* For full copyright and license information, please see
+ * For full copyright and license information, please see
-* the docs/CREDITS.txt file.
+ * 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.
+	 * Check extensions template event listener prioritizing
-	*/
+	 */
-	public function test_template_event_order()
+	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());
+	}
 }

tests/functional/fixtures/ext/foo/bar/styles/prosilver/template/event/index_body_forumlist_body_before.html

@@ -0,0 +1 @@
+<p id="foo_bar_forumlist_body_before">{{ lang('FOO_BAR_FORUMLIST_BODY_BEFORE') }}</p>

tests/functional/fixtures/ext/foo/foo/styles/prosilver/template/event/index_body_forumlist_body_before.html

@@ -0,0 +1 @@
+<p id="foo_foo_forumlist_body_before">{{ lang('FOO_FOO_FORUMLIST_BODY_BEFORE') }}</p>