From f1080891df3465e77a860275f7d8a083ac103d7a Mon Sep 17 00:00:00 2001 From: mark_story Date: Sat, 13 Oct 2012 15:39:34 -0400 Subject: [PATCH] Update Controller to use MergeVariablesTrait Remove a test case that handled the many edge cases with controller merge vars as those cases should be covered by tests for the trait now. --- lib/Cake/Controller/Controller.php | 83 ++------- .../TestApp/Controller/CommentsController.php | 1 - lib/Cake/Test/TestCase/AllControllerTest.php | 5 +- .../Controller/ControllerMergeVarsTest.php | 171 ------------------ .../TestCase/Controller/ControllerTest.php | 57 ++---- 5 files changed, 38 insertions(+), 279 deletions(-) delete mode 100644 lib/Cake/Test/TestCase/Controller/ControllerMergeVarsTest.php diff --git a/lib/Cake/Controller/Controller.php b/lib/Cake/Controller/Controller.php index 06a8370edf9..b02241eaa86 100644 --- a/lib/Cake/Controller/Controller.php +++ b/lib/Cake/Controller/Controller.php @@ -8,12 +8,12 @@ * * @copyright Copyright 2005-2012, Cake Software Foundation, Inc. (http://cakefoundation.org) * @link http://cakephp.org CakePHP(tm) Project - * @package Cake.Controller * @since CakePHP(tm) v 0.2.9 * @license MIT License (http://www.opensource.org/licenses/mit-license.php) */ namespace Cake\Controller; + use Cake\Core\App; use Cake\Core\Configure; use Cake\Core\Object; @@ -27,6 +27,7 @@ use Cake\Routing\Router; use Cake\Utility\ClassRegistry; use Cake\Utility\Inflector; +use Cake\Utility\MergeVariablesTrait; use Cake\View\View; /** @@ -63,6 +64,8 @@ */ class Controller extends Object implements EventListener { + use MergeVariablesTrait; + /** * The name of this controller. Controller names are plural, named after the model they manipulate. * @@ -298,15 +301,6 @@ class Controller extends Object implements EventListener { */ public $validationErrors = null; -/** - * The class name of the parent class you wish to merge with. - * Typically this is AppController, but you may wish to merge vars with a different - * parent class. - * - * @var string - */ - protected $_mergeParent = null; - /** * Instance of the Cake\Event\EventManager this controller is using * to dispatch inner events. @@ -539,71 +533,32 @@ protected function _getScaffold(Request $request) { * @return void */ protected function _mergeControllerVars() { - $pluginController = $pluginDot = null; - $mergeParent = is_subclass_of($this, $this->_mergeParent); - $pluginVars = array(); - $appVars = array(); - + $pluginDot = null; if (!empty($this->plugin)) { - $pluginController = Plugin::getNamespace($this->plugin) . '\Controller\Controller'; - if (!is_subclass_of($this, $pluginController)) { - $pluginController = null; - } $pluginDot = $this->plugin . '.'; } - - if ($pluginController) { - $merge = array('components', 'helpers'); - $this->_mergeVars($merge, $pluginController); - } - - if ($mergeParent || !empty($pluginController)) { - $appVars = get_class_vars($this->_mergeParent); - $merge = array('components', 'helpers'); - $this->_mergeVars($merge, $this->_mergeParent, true); - } - if ($this->uses === null) { $this->uses = false; } + if ($this->uses === false) { + $this->uses = []; + $this->modelClass = ''; + } if ($this->uses === true) { $this->uses = array($pluginDot . $this->modelClass); } - if (isset($appVars['uses']) && $appVars['uses'] === $this->uses) { + $oldUses = $this->uses; + $this->_mergeVars( + ['components', 'helpers', 'uses'], + [ + 'associative' => ['components', 'helpers'], + 'reverse' => ['uses'] + ] + ); + if ($this->uses === $oldUses) { array_unshift($this->uses, $pluginDot . $this->modelClass); } - if ($pluginController) { - $pluginVars = get_class_vars($pluginController); - } - if ($this->uses !== false) { - $this->_mergeUses($pluginVars); - $this->_mergeUses($appVars); - } else { - $this->uses = array(); - $this->modelClass = ''; - } - } - -/** - * Helper method for merging the $uses property together. - * - * Merges the elements not already in $this->uses into - * $this->uses. - * - * @param array $merge The data to merge in. - * @return void - */ - protected function _mergeUses($merge) { - if (!isset($merge['uses'])) { - return; - } - if ($merge['uses'] === true) { - return; - } - $this->uses = array_merge( - $this->uses, - array_diff($merge['uses'], $this->uses) - ); + $this->uses = array_unique($this->uses); } /** diff --git a/lib/Cake/Test/TestApp/Controller/CommentsController.php b/lib/Cake/Test/TestApp/Controller/CommentsController.php index e9be6011b36..ee1d590d310 100644 --- a/lib/Cake/Test/TestApp/Controller/CommentsController.php +++ b/lib/Cake/Test/TestApp/Controller/CommentsController.php @@ -23,5 +23,4 @@ */ class CommentsController extends AppController { - protected $_mergeParent = 'ControllerTestAppController'; } diff --git a/lib/Cake/Test/TestCase/AllControllerTest.php b/lib/Cake/Test/TestCase/AllControllerTest.php index efe82f2b677..6cf90ab43ad 100644 --- a/lib/Cake/Test/TestCase/AllControllerTest.php +++ b/lib/Cake/Test/TestCase/AllControllerTest.php @@ -26,7 +26,7 @@ * * @package Cake.Test.Case */ -class AllControllersTest extends \PHPUnit_Framework_TestSuite { +class AllControllerTest extends \PHPUnit_Framework_TestSuite { /** * suite method, defines tests for this suite. @@ -40,7 +40,6 @@ public static function suite() { $suite->addTestFile(CORE_TEST_CASES . DS . 'Controller/ScaffoldTest.php'); $suite->addTestFile(CORE_TEST_CASES . DS . 'Controller/PagesControllerTest.php'); $suite->addTestFile(CORE_TEST_CASES . DS . 'Controller/ComponentTest.php'); - $suite->addTestFile(CORE_TEST_CASES . DS . 'Controller/ControllerMergeVarsTest.php'); return $suite; } -} \ No newline at end of file +} diff --git a/lib/Cake/Test/TestCase/Controller/ControllerMergeVarsTest.php b/lib/Cake/Test/TestCase/Controller/ControllerMergeVarsTest.php deleted file mode 100644 index b3da3f5f671..00000000000 --- a/lib/Cake/Test/TestCase/Controller/ControllerMergeVarsTest.php +++ /dev/null @@ -1,171 +0,0 @@ - - * Copyright 2005-2012, Cake Software Foundation, Inc. (http://cakefoundation.org) - * - * Licensed under The MIT License - * Redistributions of files must retain the above copyright notice - * - * @copyright Copyright 2005-2012, Cake Software Foundation, Inc. (http://cakefoundation.org) - * @link http://book.cakephp.org/2.0/en/development/testing.html CakePHP(tm) Tests - * @package Cake.Test.Case.Controller - * @since CakePHP(tm) v 1.2.3 - * @license MIT License (http://www.opensource.org/licenses/mit-license.php) - */ -namespace Cake\Test\TestCase\Controller; -use Cake\Controller\Controller; -use Cake\Core\App; -use Cake\Core\Configure; -use Cake\Core\Object; -use Cake\Core\Plugin; -use Cake\TestSuite\TestCase; -use MergeVar\Controller\MergePostsController; -use TestApp\Controller\MergeVariablesController; - -/** - * Test Case for Controller Merging of Vars. - * - * @package Cake.Test.Case.Controller - */ -class ControllerMergeVarsTest extends TestCase { - -/** - * setUp method - * - * @return void - */ - public function setUp() { - $this->_appNamespace = Configure::read('App.namespace'); - Configure::write('App.namespace', 'TestApp'); - App::build(array( - 'Plugin' => array(CAKE . 'Test/TestApp/Plugin/') - )); - } - -/** - * tearDown method - * - * @return void - */ - public function tearDown() { - Configure::write('App.namespace', $this->_appNamespace); - App::build(); - } - -/** - * test that component settings are not duplicated when merging component settings - * - * @return void - */ - public function testComponentParamMergingNoDuplication() { - $Controller = new MergeVariablesController(); - $Controller->constructClasses(); - - $expected = array('MergeVar' => array('flag', 'otherFlag', 'redirect' => false)); - $this->assertEquals($expected, $Controller->components, 'Duplication of settings occurred. %s'); - } - -/** - * test component merges with redeclared components - * - * @return void - */ - public function testComponentMergingWithRedeclarations() { - $Controller = new MergeVariablesController(); - $Controller->components['MergeVar'] = array('remote', 'redirect' => true); - $Controller->constructClasses(); - - $expected = array('MergeVar' => array('flag', 'otherFlag', 'redirect' => true, 'remote')); - $this->assertEquals($expected, $Controller->components, 'Merging of settings is wrong. %s'); - } - -/** - * test merging of helpers array, ensure no duplication occurs - * - * @return void - */ - public function testHelperSettingMergingNoDuplication() { - $Controller = new MergeVariablesController(); - $Controller->constructClasses(); - - $expected = array('MergeVar' => array('format' => 'html', 'terse')); - $this->assertEquals($expected, $Controller->helpers, 'Duplication of settings occurred. %s'); - } - -/** - * Test that helpers declared in appcontroller come before those in the subclass - * orderwise - * - * @return void - */ - public function testHelperOrderPrecedence() { - $Controller = new MergeVariablesController(); - $Controller->helpers = array('Custom', 'Foo' => array('something')); - $Controller->constructClasses(); - - $expected = array( - 'MergeVar' => array('format' => 'html', 'terse'), - 'Custom' => null, - 'Foo' => array('something') - ); - $this->assertSame($expected, $Controller->helpers, 'Order is incorrect.'); - } - -/** - * test merging of vars with plugin - * - * @return void - */ - public function testMergeVarsWithPlugin() { - Plugin::load('MergeVar'); - $Controller = new MergePostsController(); - $Controller->components = array('Cookie' => array('ports' => 'open')); - $Controller->plugin = 'MergeVar'; - $Controller->constructClasses(); - - $expected = array( - 'MergeVar' => array('flag', 'otherFlag', 'redirect' => false), - 'Auth' => array('setting' => 'val', 'otherVal'), - 'Cookie' => array('ports' => 'open') - ); - $this->assertEquals($expected, $Controller->components, 'Components are unexpected.'); - - $expected = array( - 'MergeVar' => array('format' => 'html', 'terse'), - 'Javascript' => null - ); - $this->assertEquals($expected, $Controller->helpers, 'Helpers are unexpected.'); - - $Controller = new MergePostsController(); - $Controller->components = array(); - $Controller->plugin = 'MergeVar'; - $Controller->constructClasses(); - - $expected = array( - 'MergeVar' => array('flag', 'otherFlag', 'redirect' => false), - 'Auth' => array('setting' => 'val', 'otherVal'), - ); - $this->assertEquals($expected, $Controller->components, 'Components are unexpected.'); - } - -/** - * Ensure that _mergeControllerVars is not being greedy and merging with - * AppController when you make an instance of Controller - * - * @return void - */ - public function testMergeVarsNotGreedy() { - $Controller = new Controller(); - $Controller->components = array(); - $Controller->uses = array(); - $Controller->constructClasses(); - - $this->assertFalse(isset($Controller->Session)); - } -} diff --git a/lib/Cake/Test/TestCase/Controller/ControllerTest.php b/lib/Cake/Test/TestCase/Controller/ControllerTest.php index aeab4ee732c..7d2d5d17edf 100644 --- a/lib/Cake/Test/TestCase/Controller/ControllerTest.php +++ b/lib/Cake/Test/TestCase/Controller/ControllerTest.php @@ -155,8 +155,6 @@ class TestController extends ControllerTestAppController { */ public $uses = array('Comment'); - protected $_mergeParent = 'ControllerTestAppController'; - /** * index method * @@ -225,12 +223,6 @@ class AnotherTestController extends ControllerTestAppController { */ public $uses = false; -/** - * merge parent - * - * @var string - */ - protected $_mergeParent = 'ControllerTestAppController'; } /** @@ -747,33 +739,30 @@ public function testRedirectBeforeRedirectInControllerWithArray() { * @return void */ public function testMergeVars() { - $request = new Request('controller_posts/index'); + $request = new Request(); $TestController = new TestController($request); $TestController->constructClasses(); - $testVars = get_class_vars(__NAMESPACE__ . '\TestController'); - $appVars = get_class_vars(__NAMESPACE__ . '\ControllerTestAppController'); - - $components = is_array($appVars['components']) - ? array_merge($appVars['components'], $testVars['components']) - : $testVars['components']; - if (!in_array('Session', $components)) { - $components[] = 'Session'; - } - $helpers = is_array($appVars['helpers']) - ? array_merge($appVars['helpers'], $testVars['helpers']) - : $testVars['helpers']; - $uses = is_array($appVars['uses']) - ? array_merge($appVars['uses'], $testVars['uses']) - : $testVars['uses']; + $expected = [ + 'Html' => null, + 'Session' => null + ]; + $this->assertEquals($expected, $TestController->helpers); - $this->assertEquals(0, count(array_diff_key($TestController->helpers, array_flip($helpers)))); - $this->assertEquals(0, count(array_diff($TestController->uses, $uses))); - $this->assertEquals(count(array_diff_assoc(Hash::normalize($TestController->components), Hash::normalize($components))), 0); + $expected = [ + 'Session' => null, + 'Security' => null, + 'Cookie' => null, + ]; + $this->assertEquals($expected, $TestController->components); $expected = array('Comment', 'ControllerPost'); - $this->assertEquals($expected, $TestController->uses, '$uses was merged incorrectly, ControllerTestAppController models should be last.'); + $this->assertEquals( + $expected, + $TestController->uses, + '$uses was merged incorrectly, ControllerTestAppController models should be last.' + ); $TestController = new AnotherTestController($request); $TestController->constructClasses(); @@ -785,18 +774,6 @@ public function testMergeVars() { $this->assertFalse($testVars['uses']); $this->assertFalse(property_exists($TestController, 'ControllerPost')); - - $TestController = new ControllerCommentsController($request); - $TestController->constructClasses(); - - $appVars = get_class_vars(__NAMESPACE__ . '\ControllerTestAppController'); - $testVars = get_class_vars(__NAMESPACE__ . '\ControllerCommentsController'); - - $this->assertTrue(in_array('ControllerPost', $appVars['uses'])); - $this->assertEquals(array('ControllerPost'), $testVars['uses']); - - $this->assertTrue(isset($TestController->ControllerPost)); - $this->assertTrue(isset($TestController->ControllerComment)); } /**