3 files changed, 285 insertions, 120 deletions
diff --git a/phpBB/includes/template/locator.php b/phpBB/includes/template/locator.php
new file mode 100644
index 0000000000..961783e635
--- /dev/null
+++ b/phpBB/includes/template/locator.php
@@ -0,0 +1,258 @@
+<?php
+/**
+*
+* @package phpBB3
+* @copyright (c) 2011 phpBB Group
+* @license http://opensource.org/licenses/gpl-license.php GNU Public License
+*
+*/
+
+/**
+* @ignore
+*/
+if (!defined('IN_PHPBB'))
+{
+	exit;
+}
+
+
+/**
+* Template locator. Maintains mapping from template handles to source paths.
+*
+* Template locator is aware of template inheritance, and can return actual
+* filesystem paths (i.e., the "primary" template or the "parent" template)
+* depending on what files exist.
+*
+* @package phpBB3
+*/
+class phpbb_template_locator
+{
+	/**
+	* @var string Path to directory that templates are stored in.
+	*/
+	private $root = '';
+
+	/**
+	* @var string Path to parent/fallback template directory.
+	*/
+	private $inherit_root = '';
+
+	/**
+	* @var array Map from handles to source template file paths.
+	* Normally it only contains paths for handles that are used
+	* (or are likely to be used) by the page being rendered and not
+	* all templates that exist on the filesystem.
+	*/
+	private $files = array();
+
+	/**
+	* @var array Map from handles to source template file names.
+	* Covers the same data as $files property but maps to basenames
+	* instead of paths.
+	*/
+	private $filenames = array();
+
+	/**
+	* @var array Map from handles to parent/fallback source template
+	* file paths. Covers the same data as $files.
+	*/
+	private $files_inherit = array();
+
+	private $orig_tpl_inherits_id;
+
+	/**
+	* Set template location.
+	* @param string $style_name Name of style from which templates are to be taken
+	*/
+	public function set_template_path($style_name)
+	{
+		$relative_template_root = $this->relative_template_root_for_style($style_name);
+		$template_root = $this->phpbb_root_path . $relative_template_root;
+		if (!file_exists($template_root))
+		{
+			trigger_error('template locator: Template path could not be found: ' . $relative_template_root, E_USER_ERROR);
+		}
+
+		$this->root = $template_root;
+
+		if ($this->orig_tpl_inherits_id === null)
+		{
+			$this->orig_tpl_inherits_id = $this->user->theme['template_inherits_id'];
+		}
+
+		$this->user->theme['template_inherits_id'] = $this->orig_tpl_inherits_id;
+
+		if ($this->user->theme['template_inherits_id'])
+		{
+			$this->inherit_root = $this->phpbb_root_path . $this->relative_template_root_for_style($this->user->theme['template_inherit_path']);
+		}
+	}
+
+	/**
+	* Converts a style name to relative (to board root) path to
+	* the style's template files.
+	*
+	* @param $style_name string Style name
+	* @return string Path to style template files
+	*/
+	private function relative_template_root_for_style($style_name)
+	{
+		return 'styles/' . $style_name . '/template';
+	}
+
+	/**
+	* Set custom template location (able to use directory outside of phpBB).
+	*
+	* Note: Templates are still compiled to phpBB's cache directory.
+	*
+	* @param string $template_path Path to template directory
+	* @param string $template_name Name of template
+	* @param string|bool $fallback_template_path Path to fallback template, or false to disable fallback
+	*/
+	public function set_custom_template($template_path, $template_name, $fallback_template_path = false)
+	{
+		// Make sure $template_path has no ending slash
+		if (substr($template_path, -1) == '/')
+		{
+			$template_path = substr($template_path, 0, -1);
+		}
+
+		$this->root = $template_path;
+
+		if ($fallback_template_path !== false)
+		{
+			if (substr($fallback_template_path, -1) == '/')
+			{
+				$fallback_template_path = substr($fallback_template_path, 0, -1);
+			}
+
+			$this->inherit_root = $fallback_template_path;
+			$this->orig_tpl_inherits_id = true;
+		}
+		else
+		{
+			$this->orig_tpl_inherits_id = false;
+		}
+	}
+
+	/**
+	* 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.
+	*/
+	public function set_filenames(array $filename_array)
+	{
+		foreach ($filename_array as $handle => $filename)
+		{
+			if (empty($filename))
+			{
+				trigger_error("template locator: set_filenames: Empty filename specified for $handle", E_USER_ERROR);
+			}
+
+			$this->filename[$handle] = $filename;
+			$this->files[$handle] = $this->root . '/' . $filename;
+
+			if ($this->inherit_root)
+			{
+				$this->files_inherit[$handle] = $this->inherit_root . '/' . $filename;
+			}
+		}
+	}
+
+	/**
+	* 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
+	*/
+	public function get_filename_for_handle($handle)
+	{
+		if (!isset($this->filename[$handle]))
+		{
+			trigger_error("template->_tpl_load(): No file specified for handle $handle", E_USER_ERROR);
+		}
+		return $this->filename[$handle];
+	}
+
+	/**
+	* Determines the source file path for a template handle without
+	* regard for template inheritance.
+	*
+	* This function returns the path in "primary" template 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/fallback
+	* template directory if template inheritance is used).
+	*
+	* 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 template directory
+	*/
+	public function get_virtual_source_file_for_handle($handle)
+	{
+		// If we don't have a file assigned to this handle, die.
+		if (!isset($this->files[$handle]))
+		{
+			trigger_error("template locator: No file specified for handle $handle", E_USER_ERROR);
+		}
+
+		$source_file = $this->files[$handle];
+		return $source_file;
+	}
+
+	/**
+	* Determines the source file path for a template handle, accounting
+	* for template inheritance 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 inheritance checks.
+	*
+	* @param string $handle Template handle (i.e. "friendly" template name)
+	* @return string Source file path
+	*/
+	public function get_source_file_for_handle($handle)
+	{
+		// If we don't have a file assigned to this handle, die.
+		if (!isset($this->files[$handle]))
+		{
+			trigger_error("template locator: No file specified for handle $handle", E_USER_ERROR);
+		}
+
+		$source_file = $this->files[$handle];
+
+		// Try and open template for reading
+		if (!file_exists($source_file))
+		{
+			if (isset($this->files_inherit[$handle]) && $this->files_inherit[$handle])
+			{
+				$parent_source_file = $this->files_inherit[$handle];
+				if (!file_exists($parent_source_file))
+				{
+					trigger_error("template locator: Neither $source_file nor $parent_source_file exist", E_USER_ERROR);
+				}
+				$source_file = $parent_source_file;
+			}
+			else
+			{
+				trigger_error("template locator: File $source_file does not exist", E_USER_ERROR);
+			}
+		}
+		return $source_file;
+	}
+}
diff --git a/phpBB/includes/template/template.php b/phpBB/includes/template/template.php
index d2d035b3b5..4ac15ae500 100644
--- a/phpBB/includes/template/template.php
+++ b/phpBB/includes/template/template.php
@@ -38,28 +38,10 @@ class phpbb_template
 	private $context;
 
 	/**
-	* @var string Root dir for template.
-	*/
-	private $root = '';
-
-	/**
 	* @var string Path of the cache directory for the template
 	*/
 	public $cachepath = '';
 
-	/**
-	* @var array Hash of handle => file path pairs
-	*/
-	public $files = array();
-
-	/**
-	* @var array Hash of handle => filename pairs
-	*/
-	public $filename = array();
-
-	public $files_inherit = array();
-	public $inherit_root = '';
-
 	public $orig_tpl_inherits_id;
 
 	/**
@@ -83,6 +65,11 @@ class phpbb_template
 	private $user;
 
 	/**
+	* @var locator template locator
+	*/
+	private $locator;
+
+	/**
 	* Constructor.
 	*
 	* @param string $phpbb_root_path phpBB root path
@@ -94,6 +81,7 @@ class phpbb_template
 		$this->phpEx = $phpEx;
 		$this->config = $config;
 		$this->user = $user;
+		$this->locator = new phpbb_template_locator();
 	}
 
 	/**
@@ -101,23 +89,12 @@ class phpbb_template
 	*/
 	public function set_template()
 	{
-		$template_path = $this->user->theme['template_path'];
+		$template_path = $style_name = $this->user->theme['template_path'];
+		$this->locator->set_template_path($style_name);
+
 		if (file_exists($this->phpbb_root_path . 'styles/' . $template_path . '/template'))
 		{
-			$this->root = $this->phpbb_root_path . 'styles/' . $template_path . '/template';
 			$this->cachepath = $this->phpbb_root_path . 'cache/tpl_' . str_replace('_', '-', $template_path) . '_';
-
-			if ($this->orig_tpl_inherits_id === null)
-			{
-				$this->orig_tpl_inherits_id = $this->user->theme['template_inherits_id'];
-			}
-
-			$this->user->theme['template_inherits_id'] = $this->orig_tpl_inherits_id;
-
-			if ($this->user->theme['template_inherits_id'])
-			{
-				$this->inherit_root = $this->phpbb_root_path . 'styles/' . $this->user->theme['template_inherit_path'] . '/template';
-			}
 		}
 		else
 		{
@@ -138,32 +115,13 @@ class phpbb_template
 	* @param string $template_name Name of template
 	* @param string $fallback_template_path Path to fallback template
 	*/
-	public function set_custom_template($template_path, $template_name, $fallback_template_path = false)
+	public function set_custom_template($template_path, $style_name, $fallback_template_path = false)
 	{
-		// Make sure $template_path has no ending slash
-		if (substr($template_path, -1) == '/')
-		{
-			$template_path = substr($template_path, 0, -1);
-		}
+		$this->locator->set_custom_template($template_path, $style_name, $fallback_template_path);
+		$template_name = $style_name;
 
-		$this->root = $template_path;
 		$this->cachepath = $this->phpbb_root_path . 'cache/ctpl_' . str_replace('_', '-', $template_name) . '_';
 
-		if ($fallback_template_path !== false)
-		{
-			if (substr($fallback_template_path, -1) == '/')
-			{
-				$fallback_template_path = substr($fallback_template_path, 0, -1);
-			}
-
-			$this->inherit_root = $fallback_template_path;
-			$this->orig_tpl_inherits_id = true;
-		}
-		else
-		{
-			$this->orig_tpl_inherits_id = false;
-		}
-
 		$this->context = new phpbb_template_context();
 
 		return true;
@@ -176,21 +134,7 @@ class phpbb_template
 	*/
 	public function set_filenames(array $filename_array)
 	{
-		foreach ($filename_array as $handle => $filename)
-		{
-			if (empty($filename))
-			{
-				trigger_error("template->set_filenames: Empty filename specified for $handle", E_USER_ERROR);
-			}
-
-			$this->filename[$handle] = $filename;
-			$this->files[$handle] = $this->root . '/' . $filename;
-
-			if ($this->inherit_root)
-			{
-				$this->files_inherit[$handle] = $this->inherit_root . '/' . $filename;
-			}
-		}
+		$this->locator->set_filenames($filename_array);
 
 		return true;
 	}
@@ -328,31 +272,25 @@ class phpbb_template
 	*/
 	private function _tpl_load($handle)
 	{
-		if (!isset($this->filename[$handle]))
-		{
-			trigger_error("template->_tpl_load(): No file specified for handle $handle", E_USER_ERROR);
-		}
+		$virtual_source_file = $this->locator->get_virtual_source_file_for_handle($handle);
+		$source_file = null;
 
 		// reload this setting to have the values they had when this object was initialised
 		// using set_template or set_custom_template, they might otherwise have been overwritten
 		// by other template class instances in between.
 		$this->user->theme['template_inherits_id'] = $this->orig_tpl_inherits_id;
 
-		$compiled_path = $this->cachepath . str_replace('/', '.', $this->filename[$handle]) . '.' . $this->phpEx;
+		$compiled_path = $this->cachepath . str_replace('/', '.', $virtual_source_file) . '.' . $this->phpEx;
 
 		$recompile = defined('DEBUG_EXTRA') ||
 			!file_exists($compiled_path) ||
 			@filesize($compiled_path) === 0 ||
-			($this->config['load_tplcompile'] && @filemtime($compiled_path) < @filemtime($this->files[$handle]));
+			($this->config['load_tplcompile'] && @filemtime($compiled_path) < @filemtime($source_file));
 
 		if (!$recompile && $this->config['load_tplcompile'])
 		{
-			// No way around it: we need to check inheritance here
-			if ($this->user->theme['template_inherits_id'] && !file_exists($this->files[$handle]))
-			{
-				$this->files[$handle] = $this->files_inherit[$handle];
-			}
-			$recompile = (@filemtime($compiled_path) < @filemtime($this->files[$handle])) ? true : false;
+			$source_file = $this->locator->get_source_file_for_handle($handle);
+			$recompile = (@filemtime($compiled_path) < @filemtime($source_file)) ? true : false;
 		}
 
 		// Recompile page if the original template is newer, otherwise load the compiled version
@@ -361,14 +299,11 @@ class phpbb_template
 			return new phpbb_template_renderer_include($compiled_path, $this);
 		}
 
-		// Inheritance - we point to another template file for this one.
-		if (isset($this->user->theme['template_inherits_id']) && $this->user->theme['template_inherits_id'] && !file_exists($this->files[$handle]))
+		if ($source_file === null)
 		{
-			$this->files[$handle] = $this->files_inherit[$handle];
+			$source_file = $this->locator->get_source_file_for_handle($handle);
 		}
 
-		$source_file = $this->_source_file_for_handle($handle);
-
 		$compile = new phpbb_template_compile($this->config['tpl_allow_php']);
 
 		$output_file = $this->_compiled_file_for_handle($handle);
@@ -389,36 +324,14 @@ class phpbb_template
 	}
 
 	/**
-	* Resolves template handle $handle to source file path.
-	* @param string $handle Template handle (i.e. "friendly" template name)
-	* @return string Source file path
-	*/
-	private function _source_file_for_handle($handle)
-	{
-		// If we don't have a file assigned to this handle, die.
-		if (!isset($this->files[$handle]))
-		{
-			trigger_error("_source_file_for_handle(): No file specified for handle $handle", E_USER_ERROR);
-		}
-
-		$source_file = $this->files[$handle];
-
-		// Try and open template for reading
-		if (!file_exists($source_file))
-		{
-			trigger_error("_source_file_for_handle(): File $source_file does not exist", E_USER_ERROR);
-		}
-		return $source_file;
-	}
-
-	/**
 	* Determines compiled file path for handle $handle.
 	* @param string $handle Template handle (i.e. "friendly" template name)
 	* @return string Compiled file path
 	*/
 	private function _compiled_file_for_handle($handle)
 	{
-		$compiled_file = $this->cachepath . str_replace('/', '.', $this->filename[$handle]) . '.' . $this->phpEx;
+		$source_file = $this->locator->get_filename_for_handle($handle);
+		$compiled_file = $this->cachepath . str_replace('/', '.', $source_file) . '.' . $this->phpEx;
 		return $compiled_file;
 	}
 
@@ -496,15 +409,9 @@ class phpbb_template
 	*/
 	public function _tpl_include($filename, $include = true)
 	{
-		$handle = $filename;
-		$this->filename[$handle] = $filename;
-		$this->files[$handle] = $this->root . '/' . $filename;
-		if ($this->inherit_root)
-		{
-			$this->files_inherit[$handle] = $this->inherit_root . '/' . $filename;
-		}
+		$this->locator->set_filenames(array($filename => $filename));
 
-		$renderer = $this->_tpl_load($handle);
+		$renderer = $this->_tpl_load($filename);
 
 		if ($renderer)
 		{
diff --git a/tests/template/template_test.php b/tests/template/template_test.php
index 2a5a53637d..44baeaf8f0 100644
--- a/tests/template/template_test.php
+++ b/tests/template/template_test.php
@@ -256,7 +256,7 @@ class phpbb_template_template_test extends phpbb_template_template_test_case
 		$this->template->set_filenames(array('test' => $filename));
 		$this->assertFileNotExists($this->template_path . '/' . $filename, 'Testing missing file, file cannot exist');
 
-		$expecting = sprintf('_source_file_for_handle(): File %s does not exist', realpath($this->template_path . '/../') . '/templates/' . $filename);
+		$expecting = sprintf('template locator: File %s does not exist', realpath($this->template_path . '/../') . '/templates/' . $filename);
 		$this->setExpectedTriggerError(E_USER_ERROR, $expecting);
 
 		$this->display('test');
@@ -264,7 +264,7 @@ class phpbb_template_template_test extends phpbb_template_template_test_case
 
 	public function test_empty_file()
 	{
-		$expecting = 'template->set_filenames: Empty filename specified for test';
+		$expecting = 'template locator: set_filenames: Empty filename specified for test';
 
 		$this->setExpectedTriggerError(E_USER_ERROR, $expecting);
 		$this->template->set_filenames(array('test' => ''));