From 520a5f92953d350880355dbe46217d2b41edd2bd Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Wed, 31 Aug 2011 17:49:48 -0400 Subject: [PATCH] [feature/extension-manager] Refactoring the structure of extension provider PHPBB3-10323 --- phpBB/common.php | 4 +- phpBB/cron.php | 2 +- phpBB/includes/bbcode.php | 2 +- phpBB/includes/cron/{ => task}/provider.php | 4 +- phpBB/includes/extension/provider.php | 13 ++- phpBB/includes/functions_messenger.php | 2 +- .../template/extension_path_provider.php | 110 ++++++++++++++++++ phpBB/includes/template/path_provider.php | 74 +++--------- .../template/path_provider_interface.php | 52 +++++++++ phpBB/includes/template/template.php | 2 +- ...ovider_test.php => task_provider_test.php} | 4 +- tests/template/template_inheritance_test.php | 2 +- tests/template/template_test_case.php | 2 +- 13 files changed, 198 insertions(+), 75 deletions(-) rename phpBB/includes/cron/{ => task}/provider.php (91%) create mode 100644 phpBB/includes/template/extension_path_provider.php create mode 100644 phpBB/includes/template/path_provider_interface.php rename tests/cron/{provider_test.php => task_provider_test.php} (84%) diff --git a/phpBB/common.php b/phpBB/common.php index 279d5c481d..1b54c07e88 100644 --- a/phpBB/common.php +++ b/phpBB/common.php @@ -131,7 +131,7 @@ set_config_count(null, null, null, $config); $phpbb_extension_manager = new phpbb_extension_manager($db, EXT_TABLE, $phpbb_root_path, ".$phpEx", $cache->get_driver()); $phpbb_template_locator = new phpbb_template_locator(); -$phpbb_template_path_provider = new phpbb_template_path_provider($phpbb_extension_manager); +$phpbb_template_path_provider = new phpbb_template_extension_path_provider($phpbb_extension_manager, new phpbb_template_path_provider()); $template = new phpbb_template($phpbb_root_path, $phpEx, $config, $user, $phpbb_template_locator, $phpbb_template_path_provider); // Add own hook handler @@ -145,5 +145,5 @@ foreach ($cache->obtain_hooks() as $hook) if (!$config['use_system_cron']) { - $cron = new phpbb_cron_manager(new phpbb_cron_provider($phpbb_extension_manager), $cache->get_driver()); + $cron = new phpbb_cron_manager(new phpbb_cron_task_provider($phpbb_extension_manager), $cache->get_driver()); } diff --git a/phpBB/cron.php b/phpBB/cron.php index 6633a6c3fd..4f246048da 100644 --- a/phpBB/cron.php +++ b/phpBB/cron.php @@ -62,7 +62,7 @@ function do_cron($cron_lock, $run_tasks) if ($config['use_system_cron']) { - $cron = new phpbb_cron_manager(new phpbb_cron_provider($phpbb_extension_manager), $cache->get_driver()); + $cron = new phpbb_cron_manager(new phpbb_cron_task_provider($phpbb_extension_manager), $cache->get_driver()); } else { diff --git a/phpBB/includes/bbcode.php b/phpBB/includes/bbcode.php index 3addb54110..929ea45c9b 100644 --- a/phpBB/includes/bbcode.php +++ b/phpBB/includes/bbcode.php @@ -134,7 +134,7 @@ class bbcode $this->template_bitfield = new bitfield($user->theme['bbcode_bitfield']); $template_locator = new phpbb_template_locator(); - $template_path_provider = new phpbb_template_path_provider($phpbb_extension_manager); + $template_path_provider = new phpbb_template_extension_path_provider($phpbb_extension_manager, new phpbb_template_path_provider()); $template = new phpbb_template($phpbb_root_path, $phpEx, $config, $user, $template_locator, $template_path_provider); $template->set_template(); $template_locator->set_filenames(array('bbcode.html' => 'bbcode.html')); diff --git a/phpBB/includes/cron/provider.php b/phpBB/includes/cron/task/provider.php similarity index 91% rename from phpBB/includes/cron/provider.php rename to phpBB/includes/cron/task/provider.php index 42552e1d8e..3f07bac051 100644 --- a/phpBB/includes/cron/provider.php +++ b/phpBB/includes/cron/task/provider.php @@ -22,7 +22,7 @@ if (!defined('IN_PHPBB')) * * @package phpBB3 */ -class phpbb_cron_provider extends phpbb_extension_provider +class phpbb_cron_task_provider extends phpbb_extension_provider { /** * Finds cron task names using the extension manager. @@ -35,7 +35,7 @@ class phpbb_cron_provider extends phpbb_extension_provider * * @return array List of task names */ - public function find() + protected function find() { $finder = $this->extension_manager->get_finder(); diff --git a/phpBB/includes/extension/provider.php b/phpBB/includes/extension/provider.php index 9b5ec56d30..3939c2ef07 100644 --- a/phpBB/includes/extension/provider.php +++ b/phpBB/includes/extension/provider.php @@ -24,9 +24,9 @@ abstract class phpbb_extension_provider implements IteratorAggregate { /** * Array holding all found items - * @var array + * @var array|null */ - protected $items = array(); + protected $items = null; /** * An extension manager to search for items in extensions @@ -42,8 +42,6 @@ abstract class phpbb_extension_provider implements IteratorAggregate public function __construct(phpbb_extension_manager $extension_manager) { $this->extension_manager = $extension_manager; - - $this->items = $this->find(); } /** @@ -51,7 +49,7 @@ abstract class phpbb_extension_provider implements IteratorAggregate * * @return array List of task names */ - abstract function find(); + abstract protected function find(); /** * Retrieve an iterator over all items @@ -60,6 +58,11 @@ abstract class phpbb_extension_provider implements IteratorAggregate */ public function getIterator() { + if ($this->items === null) + { + $this->items = $this->find(); + } + return new ArrayIterator($this->items); } } diff --git a/phpBB/includes/functions_messenger.php b/phpBB/includes/functions_messenger.php index 2a206af7a9..1149d03347 100644 --- a/phpBB/includes/functions_messenger.php +++ b/phpBB/includes/functions_messenger.php @@ -194,7 +194,7 @@ class messenger if (!isset($this->tpl_msg[$template_lang . $template_file])) { $template_locator = new phpbb_template_locator(); - $template_path_provider = new phpbb_template_path_provider($phpbb_extension_manager); + $template_path_provider = new phpbb_template_extension_path_provider($phpbb_extension_manager, new phpbb_template_path_provider()); $this->tpl_msg[$template_lang . $template_file] = new phpbb_template($phpbb_root_path, $phpEx, $config, $user, $template_locator, $template_path_provider); $tpl = &$this->tpl_msg[$template_lang . $template_file]; diff --git a/phpBB/includes/template/extension_path_provider.php b/phpBB/includes/template/extension_path_provider.php new file mode 100644 index 0000000000..0cef590baa --- /dev/null +++ b/phpBB/includes/template/extension_path_provider.php @@ -0,0 +1,110 @@ +base_paths = $base_paths; + } + + /** + * Defines a prefix to use for template paths in extensions + * + * @param string $ext_dir_prefix The prefix including trailing slash + * @return null + */ + public function set_ext_dir_prefix($ext_dir_prefix) + { + $this->ext_dir_prefix = $ext_dir_prefix; + } + + /** + * Finds template paths using the extension manager + * + * Finds paths with the same name (e.g. styles/prosilver/template/) in all + * active extensions. Then appends the actual template paths based in the + * current working directory. + * + * @return array List of template paths + */ + public function find() + { + $directories = array(); + + $finder = $this->extension_manager->get_finder(); + foreach ($this->base_paths as $path) + { + if ($path && !phpbb_is_absolute($path)) + { + $directories = array_merge($directories, $finder + ->directory('/' . $this->ext_dir_prefix . $path) + ->get_directories() + ); + } + } + + foreach ($this->base_paths as $path) + { + $directories[] = $path; + } + + return $directories; + } + + /** + * Overwrites the current template names and paths + * + * @param array $templates An associative map from template names to paths. + * The first element is the main template. + * If the path is false, it will be generated from + * the supplied name. + * @return null + */ + public function set_templates(array $templates) + { + $this->base_paths->set_templates($templates); + } + + /** + * Retrieves the path to the main template passed into set_templates() + * + * @return string Main template path + */ + public function get_main_template_path() + { + return $this->base_paths->get_main_template_path(); + } +} diff --git a/phpBB/includes/template/path_provider.php b/phpBB/includes/template/path_provider.php index 69ac68bdb7..e6c8b34e15 100644 --- a/phpBB/includes/template/path_provider.php +++ b/phpBB/includes/template/path_provider.php @@ -22,68 +22,19 @@ if (!defined('IN_PHPBB')) * * @package phpBB3 */ -class phpbb_template_path_provider extends phpbb_extension_provider +class phpbb_template_path_provider implements IteratorAggregate, phpbb_template_path_provider_interface { protected $main_template_name = ''; - protected $templates = array(); - protected $ext_dir_prefix = ''; + protected $paths = array(); /** - * Constructor stores extension manager - * - * @param phpbb_extension_manager $extension_manager phpBB extension manager - */ - public function __construct(phpbb_extension_manager $extension_manager = null) - { - // no super call to avoid find() call - $this->extension_manager = $extension_manager; - } - - /** - * Defines a prefix to use for template paths in extensions + * Ignores the extension dir prefix * * @param string $ext_dir_prefix The prefix including trailing slash * @return null */ public function set_ext_dir_prefix($ext_dir_prefix) { - $this->ext_dir_prefix = $ext_dir_prefix; - } - - /** - * Finds template paths using the extension manager - * - * Finds paths with the same name (e.g. styles/prosilver/template/) in all - * active extensions. Then appends the actual template paths based in the - * current working directory. - * - * @return array List of template paths - */ - public function find() - { - $directories = array(); - - if ($this->extension_manager) - { - $finder = $this->extension_manager->get_finder(); - foreach ($this->templates as $name => $path) - { - if ($path && !phpbb_is_absolute($path)) - { - $directories = array_merge($directories, $finder - ->directory('/' . $this->ext_dir_prefix . $path) - ->get_directories() - ); - } - } - } - - foreach ($this->templates as $name => $path) - { - $directories[] = $path; - } - - return $directories; } /** @@ -97,7 +48,7 @@ class phpbb_template_path_provider extends phpbb_extension_provider */ public function set_templates(array $templates) { - $this->templates = array(); + $this->paths; foreach ($templates as $name => $path) { @@ -106,13 +57,10 @@ class phpbb_template_path_provider extends phpbb_extension_provider $path = $this->template_root_for_style($name); } - $this->templates[$name] = $path; + $this->paths[] = $path; } - reset($this->templates); - $this->main_template_path = current($this->templates); - - $this->items = $this->find(); + $this->main_template_path = $this->paths[0]; } /** @@ -136,4 +84,14 @@ class phpbb_template_path_provider extends phpbb_extension_provider { return 'styles/' . $style_name . '/template'; } + + /** + * Retrieve an iterator over all template paths + * + * @return ArrayIterator An iterator for the array of template paths + */ + public function getIterator() + { + return new ArrayIterator($this->paths); + } } diff --git a/phpBB/includes/template/path_provider_interface.php b/phpBB/includes/template/path_provider_interface.php new file mode 100644 index 0000000000..800d7697d9 --- /dev/null +++ b/phpBB/includes/template/path_provider_interface.php @@ -0,0 +1,52 @@ +phpbb_root_path = $phpbb_root_path; $this->phpEx = $phpEx; diff --git a/tests/cron/provider_test.php b/tests/cron/task_provider_test.php similarity index 84% rename from tests/cron/provider_test.php rename to tests/cron/task_provider_test.php index 662bb34c13..5565d0f64c 100644 --- a/tests/cron/provider_test.php +++ b/tests/cron/task_provider_test.php @@ -9,7 +9,7 @@ require_once dirname(__FILE__) . '/../mock/extension_manager.php'; -class phpbb_cron_provider_test extends PHPUnit_Framework_TestCase +class phpbb_cron_task_provider_test extends PHPUnit_Framework_TestCase { public function setUp() { @@ -22,7 +22,7 @@ class phpbb_cron_provider_test extends PHPUnit_Framework_TestCase 'ext_path' => 'ext/testext/' ), )); - $this->provider = new phpbb_cron_provider($this->extension_manager); + $this->provider = new phpbb_cron_task_provider($this->extension_manager); } public function test_manager_finds_shipped_tasks() diff --git a/tests/template/template_inheritance_test.php b/tests/template/template_inheritance_test.php index 3b75498828..93b01ae381 100644 --- a/tests/template/template_inheritance_test.php +++ b/tests/template/template_inheritance_test.php @@ -69,7 +69,7 @@ class phpbb_template_template_inheritance_test extends phpbb_template_template_t $this->template_path = dirname(__FILE__) . '/templates'; $this->parent_template_path = dirname(__FILE__) . '/parent_templates'; $this->template_locator = new phpbb_template_locator(); - $this->template_provider = new phpbb_template_path_provider(new phpbb_mock_extension_manager(dirname(__FILE__) . '/')); + $this->template_provider = new phpbb_template_path_provider(); $this->template = new phpbb_template($phpbb_root_path, $phpEx, $config, $user, $this->template_locator, $this->template_provider); $this->template->set_custom_template($this->template_path, 'tests', $this->parent_template_path); } diff --git a/tests/template/template_test_case.php b/tests/template/template_test_case.php index c0944f7f5a..4e36912e01 100644 --- a/tests/template/template_test_case.php +++ b/tests/template/template_test_case.php @@ -59,7 +59,7 @@ class phpbb_template_template_test_case extends phpbb_test_case $this->template_path = dirname(__FILE__) . '/templates'; $this->template_locator = new phpbb_template_locator(); - $this->template_provider = new phpbb_template_path_provider(new phpbb_mock_extension_manager(dirname(__FILE__) . '/')); + $this->template_provider = new phpbb_template_path_provider(); $this->template = new phpbb_template($phpbb_root_path, $phpEx, $config, $user, $this->template_locator, $this->template_provider); $this->template->set_custom_template($this->template_path, 'tests'); }