[ticket/15214] Optimize event node code and add template event order tests

PHPBB3-15214

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 = [];
+		foreach ($template_events as $events)
-
+		{
+			foreach ($events as $ext_namespace)
+			{
+				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_calls[] = function() use($compiler, $ext_namespace, $location) {
 						$compiler
 							->write("if (\$this->env->getLoader()->exists('@{$ext_namespace}/{$location}.html')) {\n")
-						->indent()
+							->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")
+							->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))
-					{
-						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();
 			}
 		}
 	}

tests/functional/extension_template_event_order_test.php

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

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 }

tests/functional/fixtures/ext/foo/bar/event/template_event_order.php

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

tests/functional/fixtures/ext/foo/bar/event/template_event_order_higher.php

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

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

@@ -0,0 +1 @@
+<li><span>{{ lang('FOO_BAR_QUICK_LINK') }}</span></li>

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 }

tests/functional/fixtures/ext/foo/foo/event/template_event_order.php

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

tests/functional/fixtures/ext/foo/foo/event/template_event_order_lower.php

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

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

@@ -0,0 +1 @@
+<li><span>{{ lang('FOO_FOO_QUICK_LINK') }}</span></li>