diff --git a/phpBB/config/default/container/services_extensions.yml b/phpBB/config/default/container/services_extensions.yml index 2c04716683..8999fc8871 100644 --- a/phpBB/config/default/container/services_extensions.yml +++ b/phpBB/config/default/container/services_extensions.yml @@ -6,6 +6,7 @@ services: - '@dbal.conn' - '@config' - '@finder.factory' + - '@router' - '%tables.ext%' - '%core.root_path%' - '@cache' diff --git a/phpBB/phpbb/extension/manager.php b/phpBB/phpbb/extension/manager.php index 275043d7e8..bc7eb7984a 100644 --- a/phpBB/phpbb/extension/manager.php +++ b/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']; } diff --git a/phpBB/phpbb/routing/router.php b/phpBB/phpbb/routing/router.php index 18285c06d5..8e789d3ffa 100644 --- a/phpBB/phpbb/routing/router.php +++ b/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); diff --git a/tests/dbal/migrator_test.php b/tests/dbal/migrator_test.php index e75b45522e..809351fd19 100644 --- a/tests/dbal/migrator_test.php +++ b/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 diff --git a/tests/extension/manager_test.php b/tests/extension/manager_test.php index 14b6a9c57f..8c1dea8fc6 100644 --- a/tests/extension/manager_test.php +++ b/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 diff --git a/tests/extension/metadata_manager_test.php b/tests/extension/metadata_manager_test.php index e4ae09064e..3ab113c465 100644 --- a/tests/extension/metadata_manager_test.php +++ b/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 diff --git a/tests/mock/dummy_router.php b/tests/mock/dummy_router.php new file mode 100644 index 0000000000..5da71abc9f --- /dev/null +++ b/tests/mock/dummy_router.php @@ -0,0 +1,31 @@ + +* @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(); + } +} diff --git a/tests/mock/extension_manager.php b/tests/mock/extension_manager.php index 31412559c3..3f549700de 100644 --- a/tests/mock/extension_manager.php +++ b/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(); } } diff --git a/tests/test_framework/phpbb_functional_test_case.php b/tests/test_framework/phpbb_functional_test_case.php index e94e233b22..e21e06073b 100644 --- a/tests/test_framework/phpbb_functional_test_case.php +++ b/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)