Pulling out parameter merging logic into a method, this allows specific typing of parameter merging (named, querystring, route). Also simplifies whitelisting of request parameters.

Tests added for merging and whitelisting.
This commit is contained in:
mark_story 2010-12-19 01:42:23 -05:00
parent 7585b2941e
commit 6b3db0a3eb
2 changed files with 216 additions and 33 deletions

View file

@ -50,6 +50,17 @@ class PaginatorComponent extends Component {
'paramType' => 'named'
);
/**
* A list of request parameters users are allowed to set. Modifying
* this list will allow users to have more influence over pagination,
* be careful with what you permit.
*
* @var array
*/
public $whitelist = array(
'limit', 'sort', 'page', 'direction'
);
/**
* Constructor
*
@ -83,6 +94,9 @@ class PaginatorComponent extends Component {
if (!is_object($object)) {
throw new MissingModelException($object);
}
$options = $this->mergeOptions($object->alias, $scope, $whitelist);
$options = array_merge(
$this->Controller->request->params,
$this->Controller->request->query,
@ -279,4 +293,49 @@ class PaginatorComponent extends Component {
}
return $object;
}
/**
* Merges the various options that Pagination uses.
* Pulls settings together from the following places:
*
* - General pagination settings
* - Model specific settings.
* - Request parameters
* - $options argument.
*
* The result of this method is the aggregate of all the option sets combined together.
*
* @param string $alias Model alias being paginated, if the general settings has a key with this value
* that key's settings will be used for pagination instead of the general ones.
* @param string $options Per call options.
* @param string $whitelist A whitelist of options that are allowed from the request parameters. Modifying
* this array will allow you to permit more or less input from the user.
* @return array Array of merged options.
*/
public function mergeOptions($alias, $options, $whitelist = array()) {
if (isset($this->settings[$alias])) {
$defaults = $this->settings[$alias];
} else {
$defaults = $this->settings;
}
if (empty($defaults['paramType'])) {
$defaults['paramType'] = 'named';
}
switch ($defaults['paramType']) {
case 'named':
$request = $this->Controller->request->params['named'];
break;
case 'querystring':
$request = $this->Controller->request->query;
break;
case 'route':
$request = $this->Controller->request->params;
unset($request['pass'], $request['named']);
}
$whitelist = array_flip(array_merge($this->whitelist, $whitelist));
$request = array_intersect_key($request, $whitelist);
return array_merge($defaults, $request, $options);
}
}

View file

@ -20,6 +20,7 @@
* @license MIT License (http://www.opensource.org/licenses/mit-license.php)
*/
App::import('Controller', 'Controller', false);
App::import('Component', 'Paginator');
App::import('Core', array('CakeRequest', 'CakeResponse'));
/**
@ -210,6 +211,20 @@ class PaginatorTest extends CakeTestCase {
*/
public $fixtures = array('core.post', 'core.comment');
/**
* setup
*
* @return void
*/
function setUp() {
parent::setUp();
$this->request = new CakeRequest('controller_posts/index');
$this->request->params['pass'] = $this->request->params['named'] = array();
$this->Controller = new Controller($this->request);
$this->Paginator = new PaginatorComponent($this->getMock('ComponentCollection'), array());
$this->Paginator->Controller = $this->Controller;
}
/**
* testPaginate method
*
@ -217,10 +232,7 @@ class PaginatorTest extends CakeTestCase {
* @return void
*/
function testPaginate() {
$request = new CakeRequest('controller_posts/index');
$request->params['pass'] = $request->params['named'] = array();
$Controller = new PaginatorTestController($request);
$Controller = new PaginatorTestController($this->request);
$Controller->uses = array('PaginatorControllerPost', 'PaginatorControllerComment');
$Controller->passedArgs[] = '1';
$Controller->params['url'] = array();
@ -306,10 +318,7 @@ class PaginatorTest extends CakeTestCase {
* @return void
*/
function testPaginateExtraParams() {
$request = new CakeRequest('controller_posts/index');
$request->params['pass'] = $request->params['named'] = array();
$Controller = new PaginatorTestController($request);
$Controller = new PaginatorTestController($this->request);
$Controller->uses = array('PaginatorControllerPost', 'PaginatorControllerComment');
$Controller->passedArgs[] = '1';
@ -352,7 +361,7 @@ class PaginatorTest extends CakeTestCase {
$this->assertEqual($Controller->PaginatorControllerPost->lastQuery['limit'], 12);
$this->assertEqual($paging['options']['limit'], 12);
$Controller = new PaginatorTestController($request);
$Controller = new PaginatorTestController($this->request);
$Controller->uses = array('ControllerPaginateModel');
$Controller->params['url'] = array();
$Controller->constructClasses();
@ -400,10 +409,7 @@ class PaginatorTest extends CakeTestCase {
* @return void
*/
public function testPaginatePassedArgs() {
$request = new CakeRequest('controller_posts/index');
$request->params['pass'] = $request->params['named'] = array();
$Controller = new PaginatorTestController($request);
$Controller = new PaginatorTestController($this->request);
$Controller->uses = array('PaginatorControllerPost');
$Controller->passedArgs[] = array('1', '2', '3');
$Controller->params['url'] = array();
@ -440,10 +446,7 @@ class PaginatorTest extends CakeTestCase {
* @return void
*/
function testPaginateSpecialType() {
$request = new CakeRequest('controller_posts/index');
$request->params['pass'] = $request->params['named'] = array();
$Controller = new PaginatorTestController($request);
$Controller = new PaginatorTestController($this->request);
$Controller->uses = array('PaginatorControllerPost', 'PaginatorControllerComment');
$Controller->passedArgs[] = '1';
$Controller->params['url'] = array();
@ -469,10 +472,7 @@ class PaginatorTest extends CakeTestCase {
* @return void
*/
function testDefaultPaginateParams() {
$request = new CakeRequest('controller_posts/index');
$request->params['pass'] = $request->params['named'] = array();
$Controller = new PaginatorTestController($request);
$Controller = new PaginatorTestController($this->request);
$Controller->modelClass = 'PaginatorControllerPost';
$Controller->params['url'] = array();
$Controller->constructClasses();
@ -491,10 +491,7 @@ class PaginatorTest extends CakeTestCase {
* @return void
*/
function testPaginateOrderVirtualField() {
$request = new CakeRequest('controller_posts/index');
$request->params['pass'] = $request->params['named'] = array();
$Controller = new PaginatorTestController($request);
$Controller = new PaginatorTestController($this->request);
$Controller->uses = array('PaginatorControllerPost', 'PaginatorControllerComment');
$Controller->params['url'] = array();
$Controller->constructClasses();
@ -522,10 +519,7 @@ class PaginatorTest extends CakeTestCase {
* @expectedException MissingModelException
*/
function testPaginateMissingModel() {
$request = new CakeRequest('controller_posts/index');
$request->params['pass'] = $request->params['named'] = array();
$Controller = new PaginatorTestController($request);
$Controller = new PaginatorTestController($this->request);
$Controller->constructClasses();
$Controller->Paginator->paginate('MissingModel');
}
@ -537,10 +531,7 @@ class PaginatorTest extends CakeTestCase {
* @access public
*/
function testPaginateMaxLimit() {
$request = new CakeRequest('controller_posts/index');
$request->params['pass'] = $request->params['named'] = array();
$Controller = new Controller($request);
$Controller = new Controller($this->request);
$Controller->uses = array('PaginatorControllerPost', 'ControllerComment');
$Controller->passedArgs[] = '1';
@ -568,4 +559,137 @@ class PaginatorTest extends CakeTestCase {
$result = $Controller->paginate('PaginatorControllerPost');
$this->assertEqual($Controller->params['paging']['PaginatorControllerPost']['options']['limit'], 2000);
}
/**
* test that option merging prefers specific models
*
* @return void
*/
function testMergeOptionsModelSpecific() {
$this->Paginator->settings = array(
'page' => 1,
'limit' => 20,
'maxLimit' => 100,
'paramType' => 'named',
'Post' => array(
'page' => 1,
'limit' => 10,
'maxLimit' => 50,
'paramType' => 'named',
)
);
$result = $this->Paginator->mergeOptions('Silly', array());
$this->assertEquals($this->Paginator->settings, $result);
$result = $this->Paginator->mergeOptions('Silly', array('limit' => 10));
$this->assertEquals(10, $result['limit']);
$result = $this->Paginator->mergeOptions('Post', array('sort' => 'title'));
$expected = array('page' => 1, 'limit' => 10, 'paramType' => 'named', 'sort' => 'title', 'maxLimit' => 50);
$this->assertEquals($expected, $result);
}
/**
* test mergeOptions with named params.
*
* @return void
*/
function testMergeOptionsNamedParams() {
$this->request->params['named'] = array(
'page' => 10,
'limit' => 10
);
$this->Paginator->settings = array(
'page' => 1,
'limit' => 20,
'maxLimit' => 100,
'paramType' => 'named',
);
$result = $this->Paginator->mergeOptions('Post', array());
$expected = array('page' => 10, 'limit' => 10, 'maxLimit' => 100, 'paramType' => 'named');
$this->assertEquals($expected, $result);
$result = $this->Paginator->mergeOptions('Post', array('page' => 100));
$this->assertEquals(100, $result['page'], 'Passed options should replace request params');
}
/**
* test merging options from the querystring.
*
* @return void
*/
function testMergeOptionsQueryString() {
$this->request->params['named'] = array(
'page' => 10,
'limit' => 10
);
$this->request->query = array(
'page' => 99,
'limit' => 75
);
$this->Paginator->settings = array(
'page' => 1,
'limit' => 20,
'maxLimit' => 100,
'paramType' => 'querystring',
);
$result = $this->Paginator->mergeOptions('Post', array());
$expected = array('page' => 99, 'limit' => 75, 'maxLimit' => 100, 'paramType' => 'querystring');
$this->assertEquals($expected, $result);
$result = $this->Paginator->mergeOptions('Post', array('page' => 100));
$this->assertEquals(100, $result['page'], 'Passed options should replace request params');
}
/**
* test that the default whitelist doesn't let people screw with things they should not be allowed to.
*
* @return void
*/
function testMergeOptionsDefaultWhiteList() {
$this->request->params['named'] = array(
'page' => 10,
'limit' => 10,
'fields' => array('bad.stuff'),
'recursive' => 1000,
'conditions' => array('bad.stuff'),
'contain' => array('bad')
);
$this->Paginator->settings = array(
'page' => 1,
'limit' => 20,
'maxLimit' => 100,
'paramType' => 'named',
);
$result = $this->Paginator->mergeOptions('Post', array());
$expected = array('page' => 10, 'limit' => 10, 'maxLimit' => 100, 'paramType' => 'named');
$this->assertEquals($expected, $result);
}
/**
* test that modifying the whitelist works.
*
* @return void
*/
function testMergeOptionsExtraWhitelist() {
$this->request->params['named'] = array(
'page' => 10,
'limit' => 10,
'fields' => array('bad.stuff'),
'recursive' => 1000,
'conditions' => array('bad.stuff'),
'contain' => array('bad')
);
$this->Paginator->settings = array(
'page' => 1,
'limit' => 20,
'maxLimit' => 100,
'paramType' => 'named',
);
$result = $this->Paginator->mergeOptions('Post', array(), array('fields'));
$expected = array(
'page' => 10, 'limit' => 10, 'maxLimit' => 100, 'paramType' => 'named', 'fields' => array('bad.stuff')
);
$this->assertEquals($expected, $result);
}
}