From 8528d8ff34986b7ff725a3798e0bc52096728a32 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Mon, 11 Jun 2012 23:11:04 -0400 Subject: [PATCH 1/7] [ticket/10933] Initialize template context when template is constructed. There is no apparent reason for either initializing or clearing the context in set_style/set_custom_style. Initially the initialization there was added in 0501640d5db158a010741e27803191ab469834c4, for reasons that presently I do not see. This permits making context property back private. PHPBB3-10933 --- phpBB/includes/style/style.php | 2 -- phpBB/includes/template/template.php | 3 ++- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/phpBB/includes/style/style.php b/phpBB/includes/style/style.php index 6b7cd31cb3..36298b49ec 100644 --- a/phpBB/includes/style/style.php +++ b/phpBB/includes/style/style.php @@ -124,8 +124,6 @@ class phpbb_style $this->template->cachepath = $this->phpbb_root_path . 'cache/tpl_' . str_replace('_', '-', $name) . '_'; - $this->template->context = new phpbb_template_context(); - if ($template_path !== false) { $this->template->template_path = $this->locator->template_path = $template_path; diff --git a/phpBB/includes/template/template.php b/phpBB/includes/template/template.php index b7c3e00dee..7703cfc278 100644 --- a/phpBB/includes/template/template.php +++ b/phpBB/includes/template/template.php @@ -36,7 +36,7 @@ class phpbb_template * Stores template data used during template rendering. * @var phpbb_template_context */ - public $context; + private $context; /** * Path of the cache directory for the template @@ -95,6 +95,7 @@ class phpbb_template $this->user = $user; $this->locator = $locator; $this->template_path = $this->locator->template_path; + $this->context = new phpbb_template_context(); } /** From 0c18f92c0adefd87a7acb618e3fc18eb394b5f42 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Mon, 11 Jun 2012 23:15:52 -0400 Subject: [PATCH 2/7] [ticket/10933] Typo fixes PHPBB3-10933 --- phpBB/includes/style/resource_locator.php | 2 +- phpBB/includes/template/template.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/style/resource_locator.php b/phpBB/includes/style/resource_locator.php index fafa11c352..334fac21db 100644 --- a/phpBB/includes/style/resource_locator.php +++ b/phpBB/includes/style/resource_locator.php @@ -97,7 +97,7 @@ class phpbb_style_resource_locator implements phpbb_template_locator * Sets the template filenames for handles. $filename_array * should be a hash of handle => filename pairs. * - * @param array $filname_array Should be a hash of handle => filename pairs. + * @param array $filename_array Should be a hash of handle => filename pairs. */ public function set_filenames(array $filename_array) { diff --git a/phpBB/includes/template/template.php b/phpBB/includes/template/template.php index 7703cfc278..a0fb887310 100644 --- a/phpBB/includes/template/template.php +++ b/phpBB/includes/template/template.php @@ -475,7 +475,7 @@ class phpbb_template */ public function locate($files, $return_default = false, $return_full_path = true) { - // add tempalte path prefix + // add template path prefix $templates = array(); if (is_string($files)) { From 766353fe5c40e4d62c1c28f439dd369acbe4efb6 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Mon, 11 Jun 2012 23:33:28 -0400 Subject: [PATCH 3/7] [ticket/10933] Useful documentation for template locate function PHPBB3-10933 --- phpBB/includes/template/template.php | 36 ++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/template/template.php b/phpBB/includes/template/template.php index a0fb887310..c89e6ec201 100644 --- a/phpBB/includes/template/template.php +++ b/phpBB/includes/template/template.php @@ -458,8 +458,40 @@ class phpbb_template } /** - * Locates source template path, accounting for styles tree and verifying that - * the path exists. + * Obtains filesystem path for a template file. + * + * The simplest use is specifying a single template file as a string + * in the first argument. This template file should be a basename + * of a template file in the selected style, or its parent styles + * if template inheritance is being utilized. + * + * Note: "selected style" is whatever style the style resource locator + * is configured for. + * + * The return value then will be a path, relative to the current + * directory or absolute, to the template file in the selected style + * or its closest parent. + * + * If the selected style does not have the template file being searched, + * (and if inheritance is involved, none of the parents have it either), + * false will be returned. + * + * Specifying true for $return_default will cause the function to + * return the first path which was checked for existence in the event + * that the template file was not found, instead of false. + * This is the path in the selected style itself, not any of its + * parents. + * + * $files can be given an array of templates instead of a single + * template. When given an array, the function will try to resolve + * each template in the array to a path, and will return the first + * path that exists, or false if none exist. + * + * If $return_full_path is false, then instead of returning a usable + * path (when the template is found) only the template's basename + * will be returned. This can be used to check which of the templates + * specified in $files exists, provided different file names are + * used for different templates. * * @param string or array $files List of templates to locate. If there is only * one template, $files can be a string to make code easier to read. From c3fb0f359ca8e39ff9c4c3ffbd9e4e899075cd57 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Tue, 12 Jun 2012 00:41:00 -0400 Subject: [PATCH 4/7] [ticket/10933] Specify empty template path for absolute includephp test. This was probably necessary all along, and the test happened to work because state was not correctly reset between test runs and a previous test set an empty template path. PHPBB3-10933 --- tests/template/includephp_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/template/includephp_test.php b/tests/template/includephp_test.php index 626735f15f..f008a734eb 100644 --- a/tests/template/includephp_test.php +++ b/tests/template/includephp_test.php @@ -36,7 +36,7 @@ class phpbb_template_includephp_test extends phpbb_template_template_test_case $this->setup_engine(array('tpl_allow_php' => true)); - $this->style->set_custom_style('tests', $cache_dir); + $this->style->set_custom_style('tests', $cache_dir, ''); $cache_file = $this->template->cachepath . 'includephp_absolute.html.php'; $this->run_template('includephp_absolute.html', array(), array(), array(), "Path is absolute.\ntesting included php", $cache_file); From d7a626c70bdfd7aea5e10ec891230cbd2e94f862 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Mon, 25 Jun 2012 23:27:08 -0400 Subject: [PATCH 5/7] [ticket/10933] Expanded prose documentation for phpbb_extension_provider. PHPBB3-10933 --- phpBB/includes/extension/provider.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/extension/provider.php b/phpBB/includes/extension/provider.php index d0541fa007..45b55e5cab 100644 --- a/phpBB/includes/extension/provider.php +++ b/phpBB/includes/extension/provider.php @@ -16,7 +16,15 @@ if (!defined('IN_PHPBB')) } /** -* Provides a set of items found in extensions +* Provides a set of items found in extensions. +* +* This abstract class is essentially a wrapper around item-specific +* finding logic. It handles storing the extension manager via constructor +* for the finding logic to use to find the items, and provides an +* iterator interface over the items found by the finding logic. +* +* Items could be anything, for example template paths or cron task names. +* Derived classes completely define what the items are. * * @package extension */ @@ -45,7 +53,7 @@ abstract class phpbb_extension_provider implements IteratorAggregate } /** - * Finds template paths using the extension manager. + * Finds items using the extension manager. * * @return array List of task names */ From 767d09227bd848da21dc3e255b1dacd20cbad1f7 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Fri, 2 Nov 2012 18:51:35 -0400 Subject: [PATCH 6/7] [ticket/10933] Dependency inject template context. PHPBB3-10933 --- phpBB/config/services.yml | 5 ++++- phpBB/includes/bbcode.php | 2 +- phpBB/includes/functions_messenger.php | 2 +- phpBB/includes/template/template.php | 5 +++-- phpBB/install/index.php | 2 +- tests/extension/metadata_manager_test.php | 3 ++- tests/template/template_test_case.php | 2 +- tests/template/template_test_case_with_tree.php | 2 +- 8 files changed, 14 insertions(+), 9 deletions(-) diff --git a/phpBB/config/services.yml b/phpBB/config/services.yml index c3d2494952..038c8a862d 100644 --- a/phpBB/config/services.yml +++ b/phpBB/config/services.yml @@ -130,7 +130,10 @@ services: - @config - @user - @style.resource_locator - - @style.path_provider_ext + - @template_context + + template_context: + class: phpbb_template_context user: class: phpbb_user diff --git a/phpBB/includes/bbcode.php b/phpBB/includes/bbcode.php index 444446e9c3..b9ffa8091c 100644 --- a/phpBB/includes/bbcode.php +++ b/phpBB/includes/bbcode.php @@ -134,7 +134,7 @@ class bbcode $style_resource_locator = new phpbb_style_resource_locator(); $style_path_provider = new phpbb_style_extension_path_provider($phpbb_extension_manager, new phpbb_style_path_provider()); - $template = new phpbb_template($phpbb_root_path, $phpEx, $config, $user, $style_resource_locator); + $template = new phpbb_template($phpbb_root_path, $phpEx, $config, $user, $style_resource_locator, new phpbb_template_context()); $style = new phpbb_style($phpbb_root_path, $phpEx, $config, $user, $style_resource_locator, $style_path_provider, $template); $style->set_style(); $template->set_filenames(array('bbcode.html' => 'bbcode.html')); diff --git a/phpBB/includes/functions_messenger.php b/phpBB/includes/functions_messenger.php index e9073553d0..cf03de08c4 100644 --- a/phpBB/includes/functions_messenger.php +++ b/phpBB/includes/functions_messenger.php @@ -210,7 +210,7 @@ class messenger { $style_resource_locator = new phpbb_style_resource_locator(); $style_path_provider = new phpbb_style_extension_path_provider($phpbb_extension_manager, new phpbb_style_path_provider()); - $tpl = new phpbb_template($phpbb_root_path, $phpEx, $config, $user, $style_resource_locator); + $tpl = new phpbb_template($phpbb_root_path, $phpEx, $config, $user, $style_resource_locator, new phpbb_template_context()); $style = new phpbb_style($phpbb_root_path, $phpEx, $config, $user, $style_resource_locator, $style_path_provider, $tpl); $this->tpl_msg[$template_lang . $template_file] = $tpl; diff --git a/phpBB/includes/template/template.php b/phpBB/includes/template/template.php index c89e6ec201..5d3ce4c82b 100644 --- a/phpBB/includes/template/template.php +++ b/phpBB/includes/template/template.php @@ -86,8 +86,9 @@ class phpbb_template * @param string $phpbb_root_path phpBB root path * @param user $user current user * @param phpbb_template_locator $locator template locator + * @param phpbb_template_context $context template context */ - public function __construct($phpbb_root_path, $php_ext, $config, $user, phpbb_template_locator $locator) + public function __construct($phpbb_root_path, $php_ext, $config, $user, phpbb_template_locator $locator, phpbb_template_context $context) { $this->phpbb_root_path = $phpbb_root_path; $this->php_ext = $php_ext; @@ -95,7 +96,7 @@ class phpbb_template $this->user = $user; $this->locator = $locator; $this->template_path = $this->locator->template_path; - $this->context = new phpbb_template_context(); + $this->context = $context; } /** diff --git a/phpBB/install/index.php b/phpBB/install/index.php index 4f1fbee7ad..f71e5ada54 100644 --- a/phpBB/install/index.php +++ b/phpBB/install/index.php @@ -213,7 +213,7 @@ $config = new phpbb_config(array( $phpbb_style_resource_locator = new phpbb_style_resource_locator(); $phpbb_style_path_provider = new phpbb_style_path_provider(); -$template = new phpbb_template($phpbb_root_path, $phpEx, $config, $user, $phpbb_style_resource_locator); +$template = new phpbb_template($phpbb_root_path, $phpEx, $config, $user, $phpbb_style_resource_locator, new phpbb_template_context()); $phpbb_style = new phpbb_style($phpbb_root_path, $phpEx, $config, $user, $phpbb_style_resource_locator, $phpbb_style_path_provider, $template); $phpbb_style->set_ext_dir_prefix('adm/'); $phpbb_style->set_custom_style('admin', '../adm/style', ''); diff --git a/tests/extension/metadata_manager_test.php b/tests/extension/metadata_manager_test.php index 9865f14314..ce7be0dea5 100644 --- a/tests/extension/metadata_manager_test.php +++ b/tests/extension/metadata_manager_test.php @@ -43,7 +43,8 @@ class metadata_manager_test extends phpbb_database_test_case $this->phpEx, $this->config, $this->user, - new phpbb_style_resource_locator() + new phpbb_style_resource_locator(), + new phpbb_template_context() ); $this->extension_manager = new phpbb_extension_manager( diff --git a/tests/template/template_test_case.php b/tests/template/template_test_case.php index 6f76cb049d..2e6f703eb1 100644 --- a/tests/template/template_test_case.php +++ b/tests/template/template_test_case.php @@ -67,7 +67,7 @@ class phpbb_template_template_test_case extends phpbb_test_case $this->template_path = $this->test_path . '/templates'; $this->style_resource_locator = new phpbb_style_resource_locator(); $this->style_provider = new phpbb_style_path_provider(); - $this->template = new phpbb_template($phpbb_root_path, $phpEx, $config, $user, $this->style_resource_locator); + $this->template = new phpbb_template($phpbb_root_path, $phpEx, $config, $user, $this->style_resource_locator, new phpbb_template_context()); $this->style = new phpbb_style($phpbb_root_path, $phpEx, $config, $user, $this->style_resource_locator, $this->style_provider, $this->template); $this->style->set_custom_style('tests', $this->template_path, ''); } diff --git a/tests/template/template_test_case_with_tree.php b/tests/template/template_test_case_with_tree.php index 05ccb7ee55..6a226f317a 100644 --- a/tests/template/template_test_case_with_tree.php +++ b/tests/template/template_test_case_with_tree.php @@ -22,7 +22,7 @@ class phpbb_template_template_test_case_with_tree extends phpbb_template_templat $this->parent_template_path = $this->test_path . '/parent_templates'; $this->style_resource_locator = new phpbb_style_resource_locator(); $this->style_provider = new phpbb_style_path_provider(); - $this->template = new phpbb_template($phpbb_root_path, $phpEx, $config, $user, $this->style_resource_locator); + $this->template = new phpbb_template($phpbb_root_path, $phpEx, $config, $user, $this->style_resource_locator, new phpbb_template_context()); $this->style = new phpbb_style($phpbb_root_path, $phpEx, $config, $user, $this->style_resource_locator, $this->style_provider, $this->template); $this->style->set_custom_style('tests', array($this->template_path, $this->parent_template_path), ''); } From c063e3a52cb561e44368cbd854973f51f4b4b304 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Fri, 2 Nov 2012 19:28:32 -0400 Subject: [PATCH 7/7] [ticket/10933] Use inheritDoc, eliminate copy pasted docblocks. PHPBB3-10933 --- phpBB/includes/style/resource_locator.php | 69 ++--------------------- 1 file changed, 5 insertions(+), 64 deletions(-) diff --git a/phpBB/includes/style/resource_locator.php b/phpBB/includes/style/resource_locator.php index 334fac21db..8658fe4a36 100644 --- a/phpBB/includes/style/resource_locator.php +++ b/phpBB/includes/style/resource_locator.php @@ -94,10 +94,7 @@ class phpbb_style_resource_locator implements phpbb_template_locator } /** - * Sets the template filenames for handles. $filename_array - * should be a hash of handle => filename pairs. - * - * @param array $filename_array Should be a hash of handle => filename pairs. + * {@inheritDoc} */ public function set_filenames(array $filename_array) { @@ -121,14 +118,7 @@ class phpbb_style_resource_locator implements phpbb_template_locator } /** - * Determines the filename for a template handle. - * - * The filename comes from array used in a set_filenames call, - * which should have been performed prior to invoking this function. - * Return value is a file basename (without path). - * - * @param $handle string Template handle - * @return string Filename corresponding to the template handle + * {@inheritDoc} */ public function get_filename_for_handle($handle) { @@ -140,24 +130,7 @@ class phpbb_style_resource_locator implements phpbb_template_locator } /** - * Determines the source file path for a template handle without - * regard for styles tree. - * - * This function returns the path in "primary" style directory - * corresponding to the given template handle. That path may or - * may not actually exist on the filesystem. Because this function - * does not perform stat calls to determine whether the path it - * returns actually exists, it is faster than get_source_file_for_handle. - * - * Use get_source_file_for_handle to obtain the actual path that is - * guaranteed to exist (which might come from the parent style - * directory if primary style has parent styles). - * - * This function will trigger an error if the handle was never - * associated with a template file via set_filenames. - * - * @param $handle string Template handle - * @return string Path to source file path in primary style directory + * {@inheritDoc} */ public function get_virtual_source_file_for_handle($handle) { @@ -172,23 +145,7 @@ class phpbb_style_resource_locator implements phpbb_template_locator } /** - * Determines the source file path for a template handle, accounting - * for styles tree and verifying that the path exists. - * - * This function returns the actual path that may be compiled for - * the specified template handle. It will trigger an error if - * the template handle was never associated with a template path - * via set_filenames or if the template file does not exist on the - * filesystem. - * - * Use get_virtual_source_file_for_handle to just resolve a template - * handle to a path without any filesystem or styles tree checks. - * - * @param string $handle Template handle (i.e. "friendly" template name) - * @param bool $find_all If true, each root path will be checked and function - * will return array of files instead of string and will not - * trigger a error if template does not exist - * @return string Source file path + * {@inheritDoc} */ public function get_source_file_for_handle($handle, $find_all = false) { @@ -239,23 +196,7 @@ class phpbb_style_resource_locator implements phpbb_template_locator } /** - * Locates source file path, accounting for styles tree and verifying that - * the path exists. - * - * Unlike previous functions, this function works without template handle - * and it can search for more than one file. If more than one file name is - * specified, it will return location of file that it finds first. - * - * @param array $files List of files to locate. - * @param bool $return_default Determines what to return if file does not - * exist. If true, function will return location where file is - * supposed to be. If false, function will return false. - * @param bool $return_full_path If true, function will return full path - * to file. If false, function will return file name. This - * parameter can be used to check which one of set of files - * is available. - * @return string or boolean Source file path if file exists or $return_default is - * true. False if file does not exist and $return_default is false + * {@inheritDoc} */ public function get_first_file_location($files, $return_default = false, $return_full_path = true) {