Merge branch '3.3.x'

phpBB/config/default/container/services_extensions.yml

@@ -6,6 +6,7 @@ services:
             - '@dbal.conn'
             - '@config'
             - '@finder.factory'
+            - '@router'
             - '%tables.ext%'
             - '%core.root_path%'
             - '@cache'

phpBB/phpbb/extension/manager.php

@@ -31,9 +31,11 @@ class manager
 	protected $finder_factory;
 	protected $cache;
 	protected $extensions;
+	protected $recently_changed_ext_status;
 	protected $extension_table;
 	protected $phpbb_root_path;
 	protected $cache_name;
+	protected $router;
 
 	/**
 	* Creates a manager and loads information from database
@@ -41,12 +43,14 @@ class manager
 	* @param ContainerInterface $container A container
 	* @param \phpbb\db\driver\driver_interface $db A database connection
 	* @param \phpbb\config\config $config Config object
+	* @param finder_factory $finder_factory Finder factory
+	* @param \phpbb\routing\router $router Router
 	* @param string $extension_table The name of the table holding extensions
 	* @param string $phpbb_root_path Path to the phpbb includes directory.
 	* @param \phpbb\cache\service|null $cache A cache instance or null
 	* @param string $cache_name The name of the cache variable, defaults to _ext
 	*/
-	public function __construct(ContainerInterface $container, \phpbb\db\driver\driver_interface $db, \phpbb\config\config $config, finder_factory $finder_factory, $extension_table, $phpbb_root_path, \phpbb\cache\service $cache = null, $cache_name = '_ext')
+	public function __construct(ContainerInterface $container, \phpbb\db\driver\driver_interface $db, \phpbb\config\config $config, finder_factory $finder_factory, \phpbb\routing\router $router, $extension_table, $phpbb_root_path, \phpbb\cache\service $cache = null, $cache_name = '_ext')
 	{
 		$this->cache = $cache;
 		$this->cache_name = $cache_name;
@@ -54,6 +58,7 @@ class manager
 		$this->finder_factory = $finder_factory;
 		$this->container = $container;
 		$this->db = $db;
+		$this->router = $router;
 		$this->extension_table = $extension_table;
 		$this->phpbb_root_path = $phpbb_root_path;
 
@@ -236,6 +241,12 @@ class manager
 			'ext_state'		=> serialize($state),
 		);
 
+		if ($active)
+		{
+			$this->recently_changed_ext_status[$name] = false;
+			$this->router->without_cache();
+		}
+
 		$this->update_state($name, $extension_data, $this->is_configured($name) ? 'update' : 'insert');
 
 		if ($active)
@@ -285,6 +296,12 @@ class manager
 		$state = $extension->disable_step($old_state);
 		$active = ($state !== false);
 
+		if (!$active)
+		{
+			$this->recently_changed_ext_status[$name] = true;
+			$this->router->without_cache();
+		}
+
 		$extension_data = array(
 			'ext_active'	=> $active,
 			'ext_state'		=> serialize($state),
@@ -497,6 +514,13 @@ class manager
 	*/
 	public function is_enabled($name)
 	{
+		// The extension has just been enabled and so is not loaded. When asking if it is enabled or
+		// not we should answer no to stay consistent with the status at the beginning of the request.
+		if (isset($this->recently_changed_ext_status[$name]))
+		{
+			return $this->recently_changed_ext_status[$name];
+		}
+
 		return isset($this->extensions[$name]['ext_active']) && $this->extensions[$name]['ext_active'];
 	}
 
@@ -508,6 +532,13 @@ class manager
 	*/
 	public function is_disabled($name)
 	{
+		// The extension has just been disabled and so is still loaded. When asking if it is disabled or
+		// not we should answer yes to stay consistent with the status at the beginning of the request.
+		if (isset($this->recently_changed_ext_status[$name]))
+		{
+			return $this->recently_changed_ext_status[$name];
+		}
+
 		return isset($this->extensions[$name]['ext_active']) && !$this->extensions[$name]['ext_active'];
 	}
 

phpBB/phpbb/routing/router.php

@@ -92,6 +92,11 @@ class router implements RouterInterface
 	 */
 	protected $debug_url_matcher;
 
+	/**
+	 * @var bool
+	 */
+	protected $use_cache;
+
 	/**
 	 * Construct method
 	 *
@@ -113,6 +118,7 @@ class router implements RouterInterface
 		$this->cache_dir			= $cache_dir;
 		$this->debug_url_generator	= $debug_url_generator;
 		$this->debug_url_matcher    = $debug_url_matcher;
+		$this->use_cache			= true;
 	}
 
 	/**
@@ -192,6 +198,22 @@ class router implements RouterInterface
 		return $this->get_matcher()->match($pathinfo);
 	}
 
+	/**
+	 * Enables the use of a cached URL generator and matcher
+	 */
+	public function with_cache()
+	{
+		$this->use_cache = true;
+	}
+
+	/**
+	 * Disables the use of a cached URL generator and matcher
+	 */
+	public function without_cache()
+	{
+		$this->use_cache = false;
+	}
+
 	/**
 	 * Gets the UrlMatcher instance associated with this Router.
 	 *
@@ -214,6 +236,12 @@ class router implements RouterInterface
 	 */
 	protected function create_dumped_url_matcher()
 	{
+		if (!$this->use_cache)
+		{
+			$this->create_new_url_matcher();
+			return;
+		}
+
 		try
 		{
 			$cache = new ConfigCache("{$this->cache_dir}url_matcher.{$this->php_ext}", $this->debug_url_matcher);
@@ -262,6 +290,12 @@ class router implements RouterInterface
 	 */
 	protected function create_dumped_url_generator()
 	{
+		if (!$this->use_cache)
+		{
+			$this->create_new_url_generator();
+			return;
+		}
+
 		try
 		{
 			$cache = new ConfigCache("{$this->cache_dir}url_generator.{$this->php_ext}", $this->debug_url_generator);

tests/dbal/migrator_test.php

@@ -83,6 +83,7 @@ class phpbb_dbal_migrator_test extends phpbb_database_test_case
 			$this->db,
 			$this->config,
 			$finder_factory,
+			new phpbb_mock_dummy_router(),
 			'phpbb_ext',
 			__DIR__ . '/../../phpBB/',
 			null

tests/extension/manager_test.php

@@ -30,6 +30,7 @@ class phpbb_extension_manager_test extends phpbb_database_test_case
 	{
 		parent::setUp();
 
+		$this->db = null;
 		$this->extension_manager = $this->create_extension_manager();
 	}
 
@@ -95,7 +96,12 @@ class phpbb_extension_manager_test extends phpbb_database_test_case
 
 		$this->assertEquals(array('vendor2/foo'), array_keys($this->extension_manager->all_enabled()));
 		$this->extension_manager->enable('vendor2/bar');
-		$this->assertEquals(array('vendor2/bar', 'vendor2/foo'), array_keys($this->extension_manager->all_enabled()));
+
+		// We should not display the extension as being enabled in the same request
+		$this->assertEquals(array('vendor2/foo'), array_keys($this->extension_manager->all_enabled()));
+		// With a different request we should see the extension as being disabled
+		$this->assertEquals(array('vendor2/bar', 'vendor2/foo'), array_keys($this->create_extension_manager()->all_enabled()));
+
 		$this->assertEquals(array('vendor/moo', 'vendor2/bar', 'vendor2/foo'), array_keys($this->extension_manager->all_configured()));
 
 		$this->assertEquals(4, vendor2\bar\ext::$state);
@@ -119,7 +125,12 @@ class phpbb_extension_manager_test extends phpbb_database_test_case
 
 		$this->assertEquals(array('vendor2/foo'), array_keys($this->extension_manager->all_enabled()));
 		$this->extension_manager->disable('vendor2/foo');
-		$this->assertEquals(array(), array_keys($this->extension_manager->all_enabled()));
+
+		// We should still display the extension as being enabled in the current request
+		$this->assertEquals(array('vendor2/foo'), array_keys($this->extension_manager->all_enabled()));
+		// With a different request we should see the extension as being disabled
+		$this->assertEquals(array(), array_keys($this->create_extension_manager()->all_enabled()));
+
 		$this->assertEquals(array('vendor/moo', 'vendor2/foo'), array_keys($this->extension_manager->all_configured()));
 
 		$this->assertTrue(vendor2\foo\ext::$disabled);
@@ -179,6 +190,7 @@ class phpbb_extension_manager_test extends phpbb_database_test_case
 			$db,
 			$config,
 			$finder_factory,
+			new phpbb_mock_dummy_router(),
 			'phpbb_ext',
 			__DIR__ . '/',
 			($with_cache) ? new \phpbb\cache\service(new phpbb_mock_cache(), $config, $db, $phpbb_root_path, $php_ext) : null

tests/extension/metadata_manager_test.php

@@ -99,6 +99,7 @@ class phpbb_extension_metadata_manager_test extends phpbb_database_test_case
 			$this->db,
 			$this->config,
 			$finder_factory,
+			new phpbb_mock_dummy_router(),
 			'phpbb_ext',
 			$this->phpbb_root_path,
 			$this->cache

tests/mock/dummy_router.php

@@ -0,0 +1,31 @@
+<?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.
+*
+*/
+
+class phpbb_mock_dummy_router extends phpbb_mock_router
+{
+	public function __construct()
+	{
+	}
+
+	public function get_matcher()
+	{
+		$this->create_new_url_matcher();
+		return parent::get_matcher();
+	}
+
+	public function get_generator()
+	{
+		$this->create_new_url_generator();
+		return parent::get_generator();
+	}
+}

tests/mock/extension_manager.php

@@ -11,6 +11,7 @@
 *
 */
 
+
 class phpbb_mock_extension_manager extends \phpbb\extension\manager
 {
 	public function __construct($phpbb_root_path, $extensions = array(), $container = null)
@@ -20,10 +21,9 @@ class phpbb_mock_extension_manager extends \phpbb\extension\manager
 		$lang = new \phpbb\language\language(new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx));
 		$this->phpbb_root_path = $phpbb_root_path;
 		$this->extensions = $extensions;
-		$this->filesystem = new \phpbb\filesystem\filesystem();
 		$this->container = $container;
 		$this->config = new \phpbb\config\config(array());
-		$this->user = new \phpbb\user($lang,'\phpbb\datetime');
 		$this->finder_factory = new \phpbb\finder\factory(null, false, $this->phpbb_root_path, $phpEx);
+		$this->router = new phpbb_mock_dummy_router();
 	}
 }

tests/test_framework/phpbb_functional_test_case.php

@@ -264,6 +264,7 @@ class phpbb_functional_test_case extends phpbb_test_case
 			$db,
 			$config,
 			$finder_factory,
+			new phpbb_mock_dummy_router(),
 			self::$config['table_prefix'] . 'ext',
 			__DIR__ . '/',
 			new \phpbb\cache\service($this->get_cache_driver(), $config, $this->db, $phpbb_root_path, $phpEx)