Merge pull request #5333 from rubencm/ticket/12631

[ticket/12631] Add finder.cache

phpBB/config/default/container/services.yml

@@ -12,6 +12,7 @@ imports:
     - { resource: services_feed.yml }
     - { resource: services_files.yml }
     - { resource: services_filesystem.yml }
+    - { resource: services_finder.yml }
     - { resource: services_help.yml }
     - { resource: services_http.yml }
     - { resource: services_language.yml }

phpBB/config/default/container/services_extensions.yml

@@ -5,9 +5,9 @@ services:
             - '@service_container'
             - '@dbal.conn'
             - '@config'
+            - '@finder.factory'
             - '%tables.ext%'
             - '%core.root_path%'
-            - '%core.php_ext%'
             - '@cache'
 
     ext.composer.installer:

phpBB/config/default/container/services_finder.yml

@@ -0,0 +1,8 @@
+services:
+    finder.factory:
+        class: phpbb\finder\factory
+        arguments:
+            - '@cache'
+            - '%finder.cache%'
+            - '%core.root_path%'
+            - '%core.php_ext%'

phpBB/config/development/config.yml

@@ -25,3 +25,6 @@ core:
 
     session:
         log_errors: true
+
+    finder:
+        cache: false

phpBB/config/installer/container/services_install_database.yml

@@ -7,7 +7,7 @@ services:
             - '@filesystem'
             - '%core.root_path%'
             - '%core.php_ext%'
-            - '%tables%'
+            - '%finder.cache%'
         tags:
             - { name: install_database_install, order: 1 }
 

phpBB/develop/create_schema_files.php

@@ -43,7 +43,7 @@ require($phpbb_root_path . 'phpbb/class_loader.' . $phpEx);
 $phpbb_class_loader = new \phpbb\class_loader('phpbb\\', "{$phpbb_root_path}phpbb/", $phpEx);
 $phpbb_class_loader->register();
 
-$finder = new \phpbb\finder($phpbb_root_path);
+$finder = new \phpbb\finder\finder(null, false, $phpbb_root_path, $phpEx);
 $classes = $finder->core_path('phpbb/')
 	->directory('/db/migration/data')
 	->get_classes();

phpBB/phpbb/di/extension/container_configuration.php

@@ -66,6 +66,12 @@ class container_configuration implements ConfigurationInterface
 						->booleanNode('log_errors')->defaultValue(false)->end()
 					->end()
 				->end()
+				->arrayNode('finder')
+					->addDefaultsIfNotSet()
+					->children()
+						->booleanNode('cache')->defaultValue(true)->end()
+					->end()
+				->end()
 			->end()
 		;
 		return $treeBuilder;

phpBB/phpbb/di/extension/core.php

@@ -118,6 +118,12 @@ class core extends Extension
 		{
 			$container->setParameter('session.' . $name, $value);
 		}
+
+		// Set the finder options
+		foreach ($config['finder'] as $name => $value)
+		{
+			$container->setParameter('finder.' . $name, $value);
+		}
 	}
 
 	/**

phpBB/phpbb/extension/base.php

@@ -23,7 +23,7 @@ class base implements \phpbb\extension\extension_interface
 	/** @var ContainerInterface */
 	protected $container;
 
-	/** @var \phpbb\finder */
+	/** @var \phpbb\finder\finder */
 	protected $extension_finder;
 
 	/** @var \phpbb\db\migrator */
@@ -42,12 +42,12 @@ class base implements \phpbb\extension\extension_interface
 	* Constructor
 	*
 	* @param ContainerInterface $container Container object
-	* @param \phpbb\finder $extension_finder
+	* @param \phpbb\finder\finder $extension_finder
 	* @param \phpbb\db\migrator $migrator
 	* @param string $extension_name Name of this extension (from ext.manager)
 	* @param string $extension_path Relative path to this extension
 	*/
-	public function __construct(ContainerInterface $container, \phpbb\finder $extension_finder, \phpbb\db\migrator $migrator, $extension_name, $extension_path)
+	public function __construct(ContainerInterface $container, \phpbb\finder\finder $extension_finder, \phpbb\db\migrator $migrator, $extension_name, $extension_path)
 	{
 		$this->container = $container;
 		$this->extension_finder = $extension_finder;

phpBB/phpbb/extension/manager.php

@@ -15,6 +15,7 @@ namespace phpbb\extension;
 
 use phpbb\exception\runtime_exception;
 use phpbb\file_downloader;
+use phpbb\finder\factory as finder_factory;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
@@ -27,8 +28,8 @@ class manager
 
 	protected $db;
 	protected $config;
+	protected $finder_factory;
 	protected $cache;
-	protected $php_ext;
 	protected $extensions;
 	protected $extension_table;
 	protected $phpbb_root_path;
@@ -42,20 +43,19 @@ class manager
 	* @param \phpbb\config\config $config Config object
 	* @param string $extension_table The name of the table holding extensions
 	* @param string $phpbb_root_path Path to the phpbb includes directory.
-	* @param string $php_ext php file extension, defaults to php
 	* @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, $extension_table, $phpbb_root_path, $php_ext = 'php', \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, $extension_table, $phpbb_root_path, \phpbb\cache\service $cache = null, $cache_name = '_ext')
 	{
 		$this->cache = $cache;
 		$this->cache_name = $cache_name;
 		$this->config = $config;
+		$this->finder_factory = $finder_factory;
 		$this->container = $container;
 		$this->db = $db;
 		$this->extension_table = $extension_table;
 		$this->phpbb_root_path = $phpbb_root_path;
-		$this->php_ext = $php_ext;
 
 		$this->extensions = ($this->cache) ? $this->cache->get($this->cache_name) : false;
 
@@ -568,14 +568,15 @@ class manager
 	}
 
 	/**
-	* Instantiates a \phpbb\finder.
+	* Instantiates a \phpbb\finder\finder.
 	*
 	* @param bool $use_all_available Should we load all extensions, or just enabled ones
-	* @return \phpbb\finder An extension finder instance
+	* @return \phpbb\finder\finder An extension finder instance
 	*/
 	public function get_finder($use_all_available = false)
 	{
-		$finder = new \phpbb\finder($this->phpbb_root_path, $this->cache, $this->php_ext, $this->cache_name . '_finder');
+		$finder = $this->finder_factory->get($this->cache_name . '_finder');
+
 		if ($use_all_available)
 		{
 			$finder->set_extensions(array_keys($this->all_available()));
@@ -584,6 +585,7 @@ class manager
 		{
 			$finder->set_extensions(array_keys($this->all_enabled()));
 		}
+
 		return $finder;
 	}
 }

phpBB/phpbb/finder/factory.php

@@ -0,0 +1,56 @@
+<?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 phpbb\finder;
+
+use phpbb\cache\service;
+
+/**
+* The finder provides a simple way to locate files in the core and a set of extensions
+*/
+class factory
+{
+	protected $cache;
+	protected $use_cache;
+	protected $phpbb_root_path;
+	protected $php_ext;
+
+	/**
+	* Creates a new finder instance with its dependencies
+	*
+	* @param service|null		$cache A cache instance or null
+	* @param bool $use_cache		Use cache or not
+	* @param string $phpbb_root_path	Path to the phpbb root directory
+	* @param string $php_ext			php file extension
+	*/
+	public function __construct(?service $cache, bool $use_cache, string $phpbb_root_path, string $php_ext)
+	{
+		$this->cache = $cache;
+		$this->use_cache = $use_cache;
+		$this->phpbb_root_path = $phpbb_root_path;
+		$this->php_ext = $php_ext;
+	}
+
+	/**
+	* The cache variable name used to store $this->cached_queries in $this->cache.
+	*
+	* Allows the use of multiple differently configured finders with the same cache.
+	*
+	* @param string $cache_name The name of the cache variable, defaults to _ext_finder
+	* @return finder New instance of finder
+	*/
+	public function get(string $cache_name = '_ext_finder'): finder
+	{
+		return new finder($this->cache, $this->use_cache, $this->phpbb_root_path, $this->php_ext, $cache_name);
+	}
+}

phpBB/phpbb/finder/finder.php

@@ -11,8 +11,9 @@
 *
 */
 
-namespace phpbb;
+namespace phpbb\finder;
 
+use phpbb\cache\service;
 use phpbb\filesystem\helper as filesystem_helper;
 
 /**
@@ -23,6 +24,7 @@ class finder
 	protected $extensions;
 	protected $phpbb_root_path;
 	protected $cache;
+	protected $use_cache;
 	protected $php_ext;
 
 	/**
@@ -50,15 +52,17 @@ class finder
 	* Creates a new finder instance with its dependencies
 	*
 	* @param string $phpbb_root_path Path to the phpbb root directory
-	* @param \phpbb\cache\service|null $cache A cache instance or null
+	* @param service|null $cache A cache instance or null
+	* @param bool $use_cache Flag whether cache should be used
 	* @param string $php_ext php file extension
 	* @param string $cache_name The name of the cache variable, defaults to
-	*                           _ext_finder
+	*							_ext_finder
 	*/
-	public function __construct($phpbb_root_path = '', \phpbb\cache\service $cache = null, $php_ext = 'php', $cache_name = '_ext_finder')
+	public function __construct(?service $cache, bool $use_cache, string $phpbb_root_path, string $php_ext, string $cache_name = '_ext_finder')
 	{
 		$this->phpbb_root_path = $phpbb_root_path;
 		$this->cache = $cache;
+		$this->use_cache = $use_cache;
 		$this->php_ext = $php_ext;
 		$this->cache_name = $cache_name;
 
@@ -81,7 +85,7 @@ class finder
 	*
 	* @param array $extensions		A list of extensions that should be searched as well
 	* @param bool $replace_list		Should the list be emptied before adding the extensions
-	* @return \phpbb\finder This object for chaining calls
+	* @return \phpbb\finder\finder This object for chaining calls
 	*/
 	public function set_extensions(array $extensions, $replace_list = true)
 	{
@@ -101,7 +105,7 @@ class finder
 	* Sets a core path to be searched in addition to extensions
 	*
 	* @param string $core_path The path relative to phpbb_root_path
-	* @return \phpbb\finder This object for chaining calls
+	* @return \phpbb\finder\finder This object for chaining calls
 	*/
 	public function core_path($core_path)
 	{
@@ -117,7 +121,7 @@ class finder
 	* file extension is automatically added to suffixes.
 	*
 	* @param string $suffix A filename suffix
-	* @return \phpbb\finder This object for chaining calls
+	* @return \phpbb\finder\finder This object for chaining calls
 	*/
 	public function suffix($suffix)
 	{
@@ -134,7 +138,7 @@ class finder
 	* file extension is automatically added to suffixes.
 	*
 	* @param string $extension_suffix A filename suffix
-	* @return \phpbb\finder This object for chaining calls
+	* @return \phpbb\finder\finder This object for chaining calls
 	*/
 	public function extension_suffix($extension_suffix)
 	{
@@ -150,7 +154,7 @@ class finder
 	* file extension is automatically added to suffixes.
 	*
 	* @param string $core_suffix A filename suffix
-	* @return \phpbb\finder This object for chaining calls
+	* @return \phpbb\finder\finder This object for chaining calls
 	*/
 	public function core_suffix($core_suffix)
 	{
@@ -162,7 +166,7 @@ class finder
 	* Sets the prefix all files found in extensions and core must match
 	*
 	* @param string $prefix A filename prefix
-	* @return \phpbb\finder This object for chaining calls
+	* @return \phpbb\finder\finder This object for chaining calls
 	*/
 	public function prefix($prefix)
 	{
@@ -175,7 +179,7 @@ class finder
 	* Sets a prefix all files found in extensions must match
 	*
 	* @param string $extension_prefix A filename prefix
-	* @return \phpbb\finder This object for chaining calls
+	* @return \phpbb\finder\finder This object for chaining calls
 	*/
 	public function extension_prefix($extension_prefix)
 	{
@@ -187,7 +191,7 @@ class finder
 	* Sets a prefix all files found in the core path must match
 	*
 	* @param string $core_prefix A filename prefix
-	* @return \phpbb\finder This object for chaining calls
+	* @return \phpbb\finder\finder This object for chaining calls
 	*/
 	public function core_prefix($core_prefix)
 	{
@@ -202,7 +206,7 @@ class finder
 	* the current directory.
 	*
 	* @param string $directory
-	* @return \phpbb\finder This object for chaining calls
+	* @return \phpbb\finder\finder This object for chaining calls
 	*/
 	public function directory($directory)
 	{
@@ -215,7 +219,7 @@ class finder
 	* Sets a directory all files found in extensions must be contained in
 	*
 	* @param string $extension_directory
-	* @return \phpbb\finder This object for chaining calls
+	* @return \phpbb\finder\finder This object for chaining calls
 	*/
 	public function extension_directory($extension_directory)
 	{
@@ -227,7 +231,7 @@ class finder
 	* Sets a directory all files found in the core path must be contained in
 	*
 	* @param string $core_directory
-	* @return \phpbb\finder This object for chaining calls
+	* @return \phpbb\finder\finder This object for chaining calls
 	*/
 	public function core_directory($core_directory)
 	{
@@ -424,7 +428,7 @@ class finder
 		$this->query['is_dir'] = $is_dir;
 		$query = md5(serialize($this->query) . serialize($extensions));
 
-		if (!defined('DEBUG') && $cache && isset($this->cached_queries[$query]))
+		if ($this->use_cache && $cache && isset($this->cached_queries[$query]))
 		{
 			return $this->cached_queries[$query];
 		}

phpBB/phpbb/install/module/install_database/task/create_schema_file.php

@@ -45,6 +45,11 @@ class create_schema_file extends \phpbb\install\task_base
 	 */
 	protected $php_ext;
 
+	/**
+	 * @var bool
+	 */
+	protected $finder_cache;
+
 	/**
 	 * Constructor
 	 *
@@ -53,12 +58,14 @@ class create_schema_file extends \phpbb\install\task_base
 	 * @param \phpbb\filesystem\filesystem_interface				$filesystem			Filesystem service
 	 * @param string												$phpbb_root_path	Path phpBB's root
 	 * @param string												$php_ext			Extension of PHP files
+	 * @param bool													$finder_cache		Flag whether to cache finder
 	 */
 	public function __construct(\phpbb\install\helper\config $config,
 								\phpbb\install\helper\database $db_helper,
 								\phpbb\filesystem\filesystem_interface $filesystem,
 								$phpbb_root_path,
-								$php_ext)
+								$php_ext,
+								$finder_cache)
 	{
 		$dbms = $db_helper->get_available_dbms($config->get('dbms'));
 		$dbms = $dbms[$config->get('dbms')]['DRIVER'];
@@ -78,6 +85,7 @@ class create_schema_file extends \phpbb\install\task_base
 		$this->filesystem		= $filesystem;
 		$this->phpbb_root_path	= $phpbb_root_path;
 		$this->php_ext			= $php_ext;
+		$this->finder_cache		= $finder_cache;
 
 		parent::__construct(true);
 	}
@@ -117,7 +125,8 @@ class create_schema_file extends \phpbb\install\task_base
 				include ($this->phpbb_root_path . 'includes/constants.' . $this->php_ext);
 			}
 
-			$finder = new \phpbb\finder($this->phpbb_root_path, null, $this->php_ext);
+			$finder_factory = new \phpbb\finder\factory(null, $this->finder_cache, $this->phpbb_root_path, $this->php_ext);
+			$finder = $finder_factory->get();
 			$migrator_classes = $finder->core_path('phpbb/db/migration/data/')->get_classes();
 			$factory = new \phpbb\db\tools\factory();
 			$db_tools = $factory->get($this->db, true);

tests/dbal/migrator_test.php

@@ -54,6 +54,8 @@ class phpbb_dbal_migrator_test extends phpbb_database_test_case
 
 		$this->config = new \phpbb\config\db($this->db, new phpbb_mock_cache, 'phpbb_config');
 
+		$finder_factory = $this->createMock('\phpbb\finder\factory');
+
 		$tools = array(
 			new \phpbb\db\migration\tool\config($this->config),
 		);
@@ -80,9 +82,9 @@ class phpbb_dbal_migrator_test extends phpbb_database_test_case
 			$container,
 			$this->db,
 			$this->config,
+			$finder_factory,
 			'phpbb_ext',
 			__DIR__ . '/../../phpBB/',
-			'php',
 			null
 		);
 	}

tests/extension/finder_test.php

@@ -15,7 +15,7 @@ class phpbb_extension_finder_test extends phpbb_test_case
 {
 	/** @var \phpbb\extension\manager */
 	protected $extension_manager;
-	/** @var \phpbb\finder */
+	/** @var \phpbb\finder\finder */
 	protected $finder;
 
 	protected function setUp(): void
@@ -243,7 +243,7 @@ class phpbb_extension_finder_test extends phpbb_test_case
 	public function test_get_classes_create_cache()
 	{
 		$cache = new phpbb_mock_cache;
-		$finder = new \phpbb\finder(__DIR__ . '/', $cache, 'php', '_custom_cache_name');
+		$finder = new \phpbb\finder\finder($cache, false, __DIR__ . '/', 'php', '_custom_cache_name');
 		$finder->set_extensions(array_keys($this->extension_manager->all_enabled()));
 		$files = $finder->suffix('_class.php')->get_files();
 
@@ -282,13 +282,15 @@ class phpbb_extension_finder_test extends phpbb_test_case
 			'is_dir' => false,
 		);
 
-		$finder = new \phpbb\finder(
+		$finder = new \phpbb\finder\finder(
-			__DIR__ . '/',
 			new phpbb_mock_cache(array(
 				'_ext_finder' => array(
 					md5(serialize($query)) => array('file_name' => 'extension'),
 				),
-			))
+			)),
+			false,
+			__DIR__ . '/',
+			'_ext_finder'
 		);
 		$finder->set_extensions(array_keys($this->extension_manager->all_enabled()));
 

tests/extension/manager_test.php

@@ -147,13 +147,14 @@ class phpbb_extension_manager_test extends phpbb_database_test_case
 
 	protected function create_extension_manager($with_cache = true)
 	{
+		$phpbb_root_path = __DIR__ . './../../phpBB/';
+		$php_ext = 'php';
 
 		$config = new \phpbb\config\config(array('version' => PHPBB_VERSION));
 		$db = $this->new_dbal();
 		$factory = new \phpbb\db\tools\factory();
+		$finder_factory = new \phpbb\finder\factory(null, false, $phpbb_root_path, $php_ext);
 		$db_tools = $factory->get($db);
-		$phpbb_root_path = __DIR__ . './../../phpBB/';
-		$php_ext = 'php';
 		$table_prefix = 'phpbb_';
 
 		$container = new phpbb_mock_container_builder();
@@ -177,9 +178,9 @@ class phpbb_extension_manager_test extends phpbb_database_test_case
 			$container,
 			$db,
 			$config,
+			$finder_factory,
 			'phpbb_ext',
 			__DIR__ . '/',
-			$php_ext,
 			($with_cache) ? new \phpbb\cache\service(new phpbb_mock_cache(), $config, $db, $phpbb_root_path, $php_ext) : null
 		);
 	}

tests/extension/metadata_manager_test.php

@@ -42,6 +42,7 @@ class phpbb_extension_metadata_manager_test extends phpbb_database_test_case
 		$this->db = $this->new_dbal();
 		$factory = new \phpbb\db\tools\factory();
 		$this->db_tools = $factory->get($this->db);
+		$finder_factory = $this->createMock('\phpbb\finder\factory');
 		$this->phpbb_root_path = __DIR__ . '/';
 		$this->phpEx = 'php';
 
@@ -97,9 +98,9 @@ class phpbb_extension_metadata_manager_test extends phpbb_database_test_case
 			$container,
 			$this->db,
 			$this->config,
+			$finder_factory,
 			'phpbb_ext',
 			$this->phpbb_root_path,
-			$this->phpEx,
 			$this->cache
 		);
 

tests/mock/container_builder.php

@@ -23,6 +23,7 @@ class phpbb_mock_container_builder implements ContainerInterface
 		$this->setParameter('debug.load_time', false);
 		$this->setParameter('session.log_errors', false);
 		$this->setParameter('session.force_sid', true);
+		$this->setParameter('finder.cache', false);
 	}
 
 	/**

tests/mock/extension_manager.php

@@ -19,11 +19,11 @@ 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->php_ext = 'php';
 		$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);
 	}
 }

tests/test_framework/phpbb_database_test_case.php

@@ -74,7 +74,7 @@ abstract class phpbb_database_test_case extends TestCase
 
 		$setup_extensions = static::setup_extensions();
 
-		$finder = new \phpbb\finder($phpbb_root_path, null, $phpEx);
+		$finder = new \phpbb\finder\finder(null, false, $phpbb_root_path, $phpEx);
 		$finder->core_path('phpbb/db/migration/data/');
 		if (!empty($setup_extensions))
 		{

tests/test_framework/phpbb_database_test_connection_manager.php

@@ -365,7 +365,7 @@ class phpbb_database_test_connection_manager
 		{
 			global $phpbb_root_path, $phpEx, $table_prefix;
 
-			$finder = new \phpbb\finder($phpbb_root_path, null, $phpEx);
+			$finder = new \phpbb\finder\finder(null, false, $phpbb_root_path, $phpEx);
 			$classes = $finder->core_path('phpbb/db/migration/data/')
 				->get_classes();
 

tests/test_framework/phpbb_functional_test_case.php

@@ -239,6 +239,7 @@ class phpbb_functional_test_case extends phpbb_test_case
 		$config = new \phpbb\config\config(array('version' => PHPBB_VERSION));
 		$db = $this->get_db();
 		$factory = new \phpbb\db\tools\factory();
+		$finder_factory = new \phpbb\finder\factory(null, false, $phpbb_root_path, $phpEx);
 		$db_tools = $factory->get($db);
 
 		$container = new phpbb_mock_container_builder();
@@ -262,9 +263,9 @@ class phpbb_functional_test_case extends phpbb_test_case
 			$container,
 			$db,
 			$config,
+			$finder_factory,
 			self::$config['table_prefix'] . 'ext',
 			__DIR__ . '/',
-			$phpEx,
 			new \phpbb\cache\service($this->get_cache_driver(), $config, $this->db, $phpbb_root_path, $phpEx)
 		);