From a72ccf28e3a0a35d1b5780d18e0052329d04941f Mon Sep 17 00:00:00 2001 From: Ceeram Date: Sun, 13 Nov 2011 03:48:26 +0100 Subject: [PATCH 1/7] accept dot syntax for element,view and layout filename from plugins fixes #2247 --- lib/Cake/View/View.php | 37 +++++++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/lib/Cake/View/View.php b/lib/Cake/View/View.php index a1e245974..988840970 100644 --- a/lib/Cake/View/View.php +++ b/lib/Cake/View/View.php @@ -290,7 +290,8 @@ class View extends Object { * This realizes the concept of Elements, (or "partial layouts") and the $params array is used to send * data to be used in the element. Elements can be cached improving performance by using the `cache` option. * - * @param string $name Name of template file in the/app/View/Elements/ folder + * @param string $name Name of template file in the/app/View/Elements/ folder, + * or `MyPlugin.template` to use the template element from MyPlugin , will be overriden by $options['plugin'] * @param array $data Array of data to be made available to the rendered view (i.e. the Element) * @param array $options Array of options. Possible keys are: * - `cache` - Can either be `true`, to enable caching using the config in View::$elementCache. Or an array @@ -308,6 +309,8 @@ class View extends Object { if (isset($options['plugin'])) { $plugin = Inflector::camelize($options['plugin']); + } else { + list($plugin, $name) = $this->_pluginSplit($name); } if (isset($this->plugin) && !$plugin) { $plugin = $this->plugin; @@ -843,6 +846,7 @@ class View extends Object { $name = $this->view; } $name = str_replace('/', DS, $name); + list($plugin, $name) = $this->_pluginSplit($name); if (strpos($name, DS) === false && $name[0] !== '.') { $name = $this->viewPath . DS . $subDir . Inflector::underscore($name); @@ -858,8 +862,7 @@ class View extends Object { $name = $this->viewPath . DS . $subDir . $name; } } - $paths = $this->_paths($this->plugin); - + $paths = $this->_paths($plugin); $exts = $this->_getExtensions(); foreach ($exts as $ext) { foreach ($paths as $path) { @@ -882,6 +885,31 @@ class View extends Object { throw new MissingViewException(array('file' => $defaultPath . $name . $this->ext)); } + function _path($name, $plugin) { + $paths = $this->_paths($plugin); + $exts = $this->_getExtensions(); + foreach ($exts as $ext) { + foreach ($paths as $path) { + if (file_exists($path . $name . $ext)) { + return $path . $name . $ext; + } + } + } + return null; + } + + function _pluginSplit($name) { + $plugin = null; + list($first, $second) = pluginSplit($name); + if (CakePlugin::loaded($first) === true) { + $name = $second; + $plugin = $first; + } + if (isset($this->plugin) && !$plugin) { + $plugin = $this->plugin; + } + return array($plugin, $name); + } /** * Returns layout filename for this template as a string. * @@ -898,7 +926,8 @@ class View extends Object { if (!is_null($this->layoutPath)) { $subDir = $this->layoutPath . DS; } - $paths = $this->_paths($this->plugin); + list($plugin, $name) = $this->_pluginSplit($name); + $paths = $this->_paths($plugin); $file = 'Layouts' . DS . $subDir . $name; $exts = $this->_getExtensions(); From 210f8c9e2ce9acf9d5beed2e5189abf9d6dfc0d7 Mon Sep 17 00:00:00 2001 From: Ceeram Date: Sun, 13 Nov 2011 04:23:27 +0100 Subject: [PATCH 2/7] adding tests for plugindot syntax in view filenames --- lib/Cake/Test/Case/View/ViewTest.php | 32 ++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/lib/Cake/Test/Case/View/ViewTest.php b/lib/Cake/Test/Case/View/ViewTest.php index 63c9ff646..bc6f4994b 100644 --- a/lib/Cake/Test/Case/View/ViewTest.php +++ b/lib/Cake/Test/Case/View/ViewTest.php @@ -322,6 +322,24 @@ class ViewTest extends CakeTestCase { $result = $View->getLayoutFileName(); $this->assertEquals($expected, $result); + $expected = CAKE . 'Test' . DS . 'test_app' . DS . 'View' . DS .'Pages' . DS .'home.ctp'; + $result = $View->getViewFileName('TestPlugin.home'); + $this->assertEqual($expected, $result); + + CakePlugin::load('TestPlugin'); + $expected = CAKE . 'Test' . DS . 'test_app' . DS . 'View' . DS .'Pages' . DS .'home.ctp'; + $result = $View->getViewFileName('TestPlugin.home'); + $this->assertEqual($expected, $result); + + $View->viewPath = 'Tests'; + $expected = CAKE . 'Test' . DS . 'test_app' . DS . 'Plugin' . DS . 'TestPlugin' . DS . 'View' . DS .'Tests' . DS .'index.ctp'; + $result = $View->getViewFileName('TestPlugin.index'); + $this->assertEqual($expected, $result); + + $expected = CAKE . 'Test' . DS . 'test_app' . DS . 'Plugin' . DS . 'TestPlugin' . DS . 'View' . DS . 'Layouts' . DS .'default.ctp'; + $result = $View->getLayoutFileName('TestPlugin.default'); + $this->assertEqual($expected, $result); + $View->layoutPath = 'rss'; $expected = CAKE . 'Test' . DS . 'test_app' . DS . 'View' . DS . 'Layouts' . DS . 'rss' . DS . 'default.ctp'; $result = $View->getLayoutFileName(); @@ -423,6 +441,13 @@ class ViewTest extends CakeTestCase { $result = $this->View->element('plugin_element', array(), array('plugin' => 'test_plugin')); $this->assertEquals($result, 'this is the plugin element using params[plugin]'); + $result = $this->View->element('TestPlugin.plugin_element'); + $this->assertEqual($result, 'this is the plugin element using params[plugin]'); + + $result = $this->View->element('test_plugin.plugin_element'); + $this->assertPattern('/Not Found:/', $result); + $this->assertPattern('/test_plugin.plugin_element/', $result); + $this->View->plugin = 'TestPlugin'; $result = $this->View->element('test_plugin_element'); $this->assertEquals($result, 'this is the test set using View::$plugin plugin element'); @@ -430,6 +455,10 @@ class ViewTest extends CakeTestCase { $result = $this->View->element('non_existant_element'); $this->assertRegExp('/Not Found:/', $result); $this->assertRegExp('/non_existant_element/', $result); + + $result = $this->View->element('TestPlugin.plugin_element', array(), array('plugin' => 'test_plugin')); + $this->assertRegExp('/Not Found:/', $result); + $this->assertRegExp('/TestPlugin.plugin_element/', $result); } /** @@ -747,6 +776,9 @@ class ViewTest extends CakeTestCase { $result = $View->getViewFileName('index'); $this->assertRegExp('/Posts(\/|\\\)index.ctp/', $result); + $result = $View->getViewFileName('TestPlugin.index'); + $this->assertPattern('/Posts(\/|\\\)index.ctp/', $result); + $result = $View->getViewFileName('/Pages/home'); $this->assertRegExp('/Pages(\/|\\\)home.ctp/', $result); From d8cbe8a1f72ad9e425c31dc8c64fd246cbde21b9 Mon Sep 17 00:00:00 2001 From: Ceeram Date: Sun, 13 Nov 2011 04:31:34 +0100 Subject: [PATCH 3/7] Cleaning up - Adding docblock - Adding visibility keyword - Update assertion methods. - Split up tests into smaller methods. --- lib/Cake/Test/Case/View/ViewTest.php | 61 +++++++++++++++++++++------- lib/Cake/View/View.php | 24 +++++------ 2 files changed, 57 insertions(+), 28 deletions(-) diff --git a/lib/Cake/Test/Case/View/ViewTest.php b/lib/Cake/Test/Case/View/ViewTest.php index bc6f4994b..87799cfbc 100644 --- a/lib/Cake/Test/Case/View/ViewTest.php +++ b/lib/Cake/Test/Case/View/ViewTest.php @@ -297,7 +297,7 @@ class ViewTest extends CakeTestCase { * * @return void */ - public function testGetTemplate() { + public function testGetViewFileNames() { $this->Controller->plugin = null; $this->Controller->name = 'Pages'; $this->Controller->viewPath = 'Pages'; @@ -318,27 +318,37 @@ class ViewTest extends CakeTestCase { $result = $View->getViewFileName('../Posts/index'); $this->assertEquals($expected, $result); - $expected = CAKE . 'Test' . DS . 'test_app' . DS . 'View' . DS . 'Layouts' . DS .'default.ctp'; - $result = $View->getLayoutFileName(); - $this->assertEquals($expected, $result); - - $expected = CAKE . 'Test' . DS . 'test_app' . DS . 'View' . DS .'Pages' . DS .'home.ctp'; - $result = $View->getViewFileName('TestPlugin.home'); - $this->assertEqual($expected, $result); + $expected = CAKE . 'Test' . DS . 'test_app' . DS . 'View' . DS .'Pages' . DS .'page.home.ctp'; + $result = $View->getViewFileName('page.home'); + $this->assertEquals($expected, $result, 'Should not ruin files with dots.'); CakePlugin::load('TestPlugin'); $expected = CAKE . 'Test' . DS . 'test_app' . DS . 'View' . DS .'Pages' . DS .'home.ctp'; $result = $View->getViewFileName('TestPlugin.home'); - $this->assertEqual($expected, $result); + $this->assertEquals($expected, $result, 'Plugin is missing the view, cascade to app.'); $View->viewPath = 'Tests'; $expected = CAKE . 'Test' . DS . 'test_app' . DS . 'Plugin' . DS . 'TestPlugin' . DS . 'View' . DS .'Tests' . DS .'index.ctp'; $result = $View->getViewFileName('TestPlugin.index'); - $this->assertEqual($expected, $result); + $this->assertEquals($expected, $result); + } - $expected = CAKE . 'Test' . DS . 'test_app' . DS . 'Plugin' . DS . 'TestPlugin' . DS . 'View' . DS . 'Layouts' . DS .'default.ctp'; - $result = $View->getLayoutFileName('TestPlugin.default'); - $this->assertEqual($expected, $result); +/** + * Test getting layout filenames + * + * @return void + */ + public function testGetLayoutFileName() { + $this->Controller->plugin = null; + $this->Controller->name = 'Pages'; + $this->Controller->viewPath = 'Pages'; + $this->Controller->action = 'display'; + + $View = new TestView($this->Controller); + + $expected = CAKE . 'Test' . DS . 'test_app' . DS . 'View' . DS . 'Layouts' . DS .'default.ctp'; + $result = $View->getLayoutFileName(); + $this->assertEquals($expected, $result); $View->layoutPath = 'rss'; $expected = CAKE . 'Test' . DS . 'test_app' . DS . 'View' . DS . 'Layouts' . DS . 'rss' . DS . 'default.ctp'; @@ -348,7 +358,30 @@ class ViewTest extends CakeTestCase { $View->layoutPath = 'Emails' . DS . 'html'; $expected = CAKE . 'Test' . DS . 'test_app' . DS . 'View' . DS . 'Layouts' . DS . 'Emails' . DS . 'html' . DS . 'default.ctp'; $result = $View->getLayoutFileName(); + $this->assertEquals($expected, $result); + } +/** + * Test getting layout filenames for plugins. + * + * @return void + */ + public function testGetLayoutFileNamePlugin() { + $this->Controller->plugin = null; + $this->Controller->name = 'Pages'; + $this->Controller->viewPath = 'Pages'; + $this->Controller->action = 'display'; + + $View = new TestView($this->Controller); + CakePlugin::load('TestPlugin'); + + $expected = CAKE . 'Test' . DS . 'test_app' . DS . 'Plugin' . DS . 'TestPlugin' . DS . 'View' . DS . 'Layouts' . DS .'default.ctp'; + $result = $View->getLayoutFileName('TestPlugin.default'); + $this->assertEquals($expected, $result); + + $View->plugin = 'TestPlugin'; + $expected = CAKE . 'Test' . DS . 'test_app' . DS . 'Plugin' . DS . 'TestPlugin' . DS . 'View' . DS . 'Layouts' . DS .'default.ctp'; + $result = $View->getLayoutFileName('default'); $this->assertEquals($expected, $result); } @@ -442,7 +475,7 @@ class ViewTest extends CakeTestCase { $this->assertEquals($result, 'this is the plugin element using params[plugin]'); $result = $this->View->element('TestPlugin.plugin_element'); - $this->assertEqual($result, 'this is the plugin element using params[plugin]'); + $this->assertEquals($result, 'this is the plugin element using params[plugin]'); $result = $this->View->element('test_plugin.plugin_element'); $this->assertPattern('/Not Found:/', $result); diff --git a/lib/Cake/View/View.php b/lib/Cake/View/View.php index 988840970..eb2ee3be4 100644 --- a/lib/Cake/View/View.php +++ b/lib/Cake/View/View.php @@ -885,20 +885,15 @@ class View extends Object { throw new MissingViewException(array('file' => $defaultPath . $name . $this->ext)); } - function _path($name, $plugin) { - $paths = $this->_paths($plugin); - $exts = $this->_getExtensions(); - foreach ($exts as $ext) { - foreach ($paths as $path) { - if (file_exists($path . $name . $ext)) { - return $path . $name . $ext; - } - } - } - return null; - } - - function _pluginSplit($name) { +/** + * Splits a dot syntax plugin name into its plugin and filename. + * If $name does not have a dot, then index 0 will be null. + * It checks if the plugin is loaded, else filename will stay unchanged for filenames containing dot + * + * @param string $name The name you want to plugin split. + * @return array Array with 2 indexes. 0 => plugin name, 1 => filename + */ + protected function _pluginSplit($name) { $plugin = null; list($first, $second) = pluginSplit($name); if (CakePlugin::loaded($first) === true) { @@ -910,6 +905,7 @@ class View extends Object { } return array($plugin, $name); } + /** * Returns layout filename for this template as a string. * From 047e93e285489e826434fd5a84570f1fa3034137 Mon Sep 17 00:00:00 2001 From: mark_story Date: Wed, 21 Dec 2011 21:57:18 -0500 Subject: [PATCH 4/7] Clean up internal API's - There was some duplication in element() for handing plugins. - Deprecate options[plugin] for element() - Add file omitted in previous commit. --- .../Test/test_app/View/Pages/page.home.ctp | 2 ++ lib/Cake/View/View.php | 27 +++++++++++-------- 2 files changed, 18 insertions(+), 11 deletions(-) create mode 100644 lib/Cake/Test/test_app/View/Pages/page.home.ctp diff --git a/lib/Cake/Test/test_app/View/Pages/page.home.ctp b/lib/Cake/Test/test_app/View/Pages/page.home.ctp new file mode 100644 index 000000000..46f877f38 --- /dev/null +++ b/lib/Cake/Test/test_app/View/Pages/page.home.ctp @@ -0,0 +1,2 @@ +Empty page with a dot in the filename. +Used to test plugin.view and missing plugins. diff --git a/lib/Cake/View/View.php b/lib/Cake/View/View.php index eb2ee3be4..a5b096505 100644 --- a/lib/Cake/View/View.php +++ b/lib/Cake/View/View.php @@ -31,6 +31,7 @@ App::uses('ViewBlock', 'View'); * and then inserted into the selected layout. This also means you can pass data from the view to the * layout using `$this->set()` * + * * @package Cake.View * @property CacheHelper $Cache * @property FormHelper $Form @@ -291,30 +292,29 @@ class View extends Object { * data to be used in the element. Elements can be cached improving performance by using the `cache` option. * * @param string $name Name of template file in the/app/View/Elements/ folder, - * or `MyPlugin.template` to use the template element from MyPlugin , will be overriden by $options['plugin'] + * or `MyPlugin.template` to use the template element from MyPlugin. If the element + * is not found in the plugin, the normal view path cascade will be searched. * @param array $data Array of data to be made available to the rendered view (i.e. the Element) * @param array $options Array of options. Possible keys are: * - `cache` - Can either be `true`, to enable caching using the config in View::$elementCache. Or an array * If an array, the following keys can be used: * - `config` - Used to store the cached element in a custom cache configuration. * - `key` - Used to define the key used in the Cache::write(). It will be prefixed with `element_` - * - `plugin` - Load an element from a specific plugin. + * - `plugin` - Load an element from a specific plugin. This option is deprecated, see below. * - `callbacks` - Set to true to fire beforeRender and afterRender helper callbacks for this element. * Defaults to false. * @return string Rendered Element + * @deprecated The `$options['plugin']` is deprecated and will be removed in CakePHP 3.0. Use + * `Plugin.element_name` instead. */ public function element($name, $data = array(), $options = array()) { $file = $plugin = $key = null; $callbacks = false; if (isset($options['plugin'])) { - $plugin = Inflector::camelize($options['plugin']); - } else { - list($plugin, $name) = $this->_pluginSplit($name); - } - if (isset($this->plugin) && !$plugin) { - $plugin = $this->plugin; + $name = Inflector::camelize($options['plugin']) . '.' . $name; } + if (isset($options['callbacks'])) { $callbacks = $options['callbacks']; } @@ -343,7 +343,7 @@ class View extends Object { } } - $file = $this->_getElementFilename($name, $plugin); + $file = $this->_getElementFilename($name); if ($file) { if (!$this->_helpersLoaded) { @@ -384,6 +384,10 @@ class View extends Object { * * If View::$autoRender is false and no `$layout` is provided, the view will be returned bare. * + * View and layout names can point to plugin views/layouts. Using the `Plugin.view` syntax + * a plugin view/layout can be used instead of the app ones. If the chosen plugin is not found + * the view will be located along the regular view path cascade. + * * @param string $view Name of view file to use * @param string $layout Layout to use. * @return string Rendered Element @@ -955,10 +959,11 @@ class View extends Object { * Finds an element filename, returns false on failure. * * @param string $name The name of the element to find. - * @param string $plugin The plugin name the element is in. * @return mixed Either a string to the element filename or false when one can't be found. */ - protected function _getElementFileName($name, $plugin = null) { + protected function _getElementFileName($name) { + list($plugin, $name) = $this->_pluginSplit($name); + $paths = $this->_paths($plugin); $exts = $this->_getExtensions(); foreach ($exts as $ext) { From 70981d05ca48cf7e7c4cb1d00d19a36bac2b77a4 Mon Sep 17 00:00:00 2001 From: mark_story Date: Fri, 23 Dec 2011 23:56:48 -0500 Subject: [PATCH 5/7] Throw an exception when a view extends itself. --- lib/Cake/Test/Case/View/ViewTest.php | 11 +++++++++++ lib/Cake/Test/test_app/View/Posts/extend_self.ctp | 2 ++ lib/Cake/View/View.php | 3 +++ 3 files changed, 16 insertions(+) create mode 100644 lib/Cake/Test/test_app/View/Posts/extend_self.ctp diff --git a/lib/Cake/Test/Case/View/ViewTest.php b/lib/Cake/Test/Case/View/ViewTest.php index 87799cfbc..7cdcdb5bd 100644 --- a/lib/Cake/Test/Case/View/ViewTest.php +++ b/lib/Cake/Test/Case/View/ViewTest.php @@ -1147,6 +1147,17 @@ TEXT; $this->assertEquals($expected, $content); } +/** + * Make sure that extending the current view with itself causes an exception + * + * @expectedException LogicException + * @return void + */ + public function testExtendSelf() { + $this->View->layout = false; + $this->View->render('extend_self'); + } + /** * Test extend() in an element and a view. * diff --git a/lib/Cake/Test/test_app/View/Posts/extend_self.ctp b/lib/Cake/Test/test_app/View/Posts/extend_self.ctp new file mode 100644 index 000000000..3515819bc --- /dev/null +++ b/lib/Cake/Test/test_app/View/Posts/extend_self.ctp @@ -0,0 +1,2 @@ +extend('extend_self'); ?> +To infinifty and beyond. diff --git a/lib/Cake/View/View.php b/lib/Cake/View/View.php index a5b096505..ba329cdc1 100644 --- a/lib/Cake/View/View.php +++ b/lib/Cake/View/View.php @@ -633,6 +633,9 @@ class View extends Object { break; } + if ($parent == $this->_current) { + throw new LogicException(__d('cake_dev', 'You cannot have views extend themselves.')); + } $this->_parents[$this->_current] = $parent; } From 69b1c33f1f29799445c0f5e83a21ae2d213eb627 Mon Sep 17 00:00:00 2001 From: mark_story Date: Sat, 24 Dec 2011 00:03:51 -0500 Subject: [PATCH 6/7] Fix extending in loops. --- lib/Cake/Test/Case/View/ViewTest.php | 12 ++++++++++++ lib/Cake/Test/test_app/View/Posts/extend_loop.ctp | 2 ++ .../Test/test_app/View/Posts/extend_loop_inner.ctp | 2 ++ lib/Cake/View/View.php | 4 ++++ 4 files changed, 20 insertions(+) create mode 100644 lib/Cake/Test/test_app/View/Posts/extend_loop.ctp create mode 100644 lib/Cake/Test/test_app/View/Posts/extend_loop_inner.ctp diff --git a/lib/Cake/Test/Case/View/ViewTest.php b/lib/Cake/Test/Case/View/ViewTest.php index 7cdcdb5bd..fc148e1e9 100644 --- a/lib/Cake/Test/Case/View/ViewTest.php +++ b/lib/Cake/Test/Case/View/ViewTest.php @@ -1158,6 +1158,18 @@ TEXT; $this->View->render('extend_self'); } +/** + * Make sure that extending in a loop causes an exception + * + * @expectedException LogicException + * @return void + */ + public function testExtendLoop() { + $this->View->layout = false; + $this->View->render('extend_loop'); + } + + /** * Test extend() in an element and a view. * diff --git a/lib/Cake/Test/test_app/View/Posts/extend_loop.ctp b/lib/Cake/Test/test_app/View/Posts/extend_loop.ctp new file mode 100644 index 000000000..f7a07aedf --- /dev/null +++ b/lib/Cake/Test/test_app/View/Posts/extend_loop.ctp @@ -0,0 +1,2 @@ +extend('extend_loop_inner'); ?> +Outer element. diff --git a/lib/Cake/Test/test_app/View/Posts/extend_loop_inner.ctp b/lib/Cake/Test/test_app/View/Posts/extend_loop_inner.ctp new file mode 100644 index 000000000..efde05804 --- /dev/null +++ b/lib/Cake/Test/test_app/View/Posts/extend_loop_inner.ctp @@ -0,0 +1,2 @@ +extend('extend_loop'); ?> +Inner loop element. diff --git a/lib/Cake/View/View.php b/lib/Cake/View/View.php index ba329cdc1..69ba7b676 100644 --- a/lib/Cake/View/View.php +++ b/lib/Cake/View/View.php @@ -619,6 +619,7 @@ class View extends Object { * * @param string $name The view or element to 'extend' the current one with. * @return void + * @throws LogicException when you extend a view with itself or make extend loops. */ public function extend($name) { switch ($this->_currentType) { @@ -636,6 +637,9 @@ class View extends Object { if ($parent == $this->_current) { throw new LogicException(__d('cake_dev', 'You cannot have views extend themselves.')); } + if (isset($this->_parents[$parent]) && $this->_parents[$parent] == $this->_current) { + throw new LogicException(__d('cake_dev', 'You cannot have views extend in a loop.')); + } $this->_parents[$this->_current] = $parent; } From 0b9b23fe3839e502542f9b7709c8de4b29782af8 Mon Sep 17 00:00:00 2001 From: mark_story Date: Sat, 24 Dec 2011 00:13:43 -0500 Subject: [PATCH 7/7] Fix caching of paths when a plugin param is used. Plugin paths should not be cached as it makes subsequent look-ups behave incorrectly. --- lib/Cake/View/View.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/Cake/View/View.php b/lib/Cake/View/View.php index 69ba7b676..ea5c90767 100644 --- a/lib/Cake/View/View.php +++ b/lib/Cake/View/View.php @@ -1007,7 +1007,10 @@ class View extends Object { $paths = array_merge($paths, App::path('View', $plugin)); } - $this->_paths = array_unique(array_merge($paths, $viewPaths, array_keys($corePaths))); - return $this->_paths; + $paths = array_unique(array_merge($paths, $viewPaths, array_keys($corePaths))); + if ($plugin !== null) { + return $paths; + } + return $this->_paths = $paths; } }