Skip to content

Commit

Permalink
Moving limit checking into a separate method, and adding tests.
Browse files Browse the repository at this point in the history 
Removing $scope from being passed down to the options, it previously only allowed additional conditions to be set.
Updated tests.
  • Loading branch information
@markstory
markstory committed Dec 19, 2010
1 parent 1e741de commit e9d3fcf
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 32 deletions.
40 changes: 24 additions & 16 deletions cake/libs/controller/components/paginator.php
View file
Expand Up @@ -77,7 +77,7 @@ public function __construct(ComponentCollection $collection, $settings = array()
* Handles automatic pagination of model records.
*
* @param mixed $object Model to paginate (e.g: model instance, or 'Model', or 'Model.InnerModel')
* @param mixed $scope Conditions to use while paginating
* @param mixed $scope Additional find conditions to use while paginating
* @param array $whitelist List of allowed options for paging
* @return array Model query results
*/
Expand All @@ -87,16 +87,16 @@ public function paginate($object = null, $scope = array(), $whitelist = array())
$scope = $object;
$object = null;
}
$assoc = null;

$object = $this->_getObject($object);

if (!is_object($object)) {
throw new MissingModelException($object);
}

$options = $this->mergeOptions($object->alias, $scope, $whitelist);
$options = $this->mergeOptions($object->alias, $whitelist);
$options = $this->validateSort($object, $options);
$options = $this->checkLimit($options);

$conditions = $fields = $order = $limit = $page = $recursive = null;

Expand All @@ -111,13 +111,6 @@ public function paginate($object = null, $scope = array(), $whitelist = array())
unset($options[0]);
}

$options = array_merge(array('page' => 1, 'limit' => 20, 'maxLimit' => 100), $options);
$options['limit'] = (int) $options['limit'];
if (empty($options['limit']) || $options['limit'] < 1) {
$options['limit'] = 1;
}
$options['limit'] = min((int)$options['limit'], $options['maxLimit']);

extract($options);

if (is_array($scope) && !empty($scope)) {
Expand Down Expand Up @@ -248,20 +241,20 @@ protected function _getObject($object) {
*
* @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()) {
public function mergeOptions($alias, $whitelist = array()) {
if (isset($this->settings[$alias])) {
$defaults = $this->settings[$alias];
} else {
$defaults = $this->settings;
}
if (empty($defaults['paramType'])) {
$defaults['paramType'] = 'named';
}
$defaults = array_merge(
array('page' => 1, 'limit' => 20, 'maxLimit' => 100, 'paramType' => 'named'),
$defaults
);
switch ($defaults['paramType']) {
case 'named':
$request = $this->Controller->request->params['named'];
Expand All @@ -277,7 +270,7 @@ public function mergeOptions($alias, $options, $whitelist = array()) {
$whitelist = array_flip(array_merge($this->whitelist, $whitelist));
$request = array_intersect_key($request, $whitelist);

return array_merge($defaults, $request, $options);
return array_merge($defaults, $request);
}

/**
Expand Down Expand Up @@ -322,4 +315,19 @@ public function validateSort($object, $options) {

return $options;
}

/**
* Check the limit parameter and ensure its within the maxLimit bounds.
*
* @param array $options An array of options with a limit key to be checked.
* @return array An array of options for pagination
*/
public function checkLimit($options) {
$options['limit'] = (int) $options['limit'];
if (empty($options['limit']) || $options['limit'] < 1) {
$options['limit'] = 1;
}
$options['limit'] = min((int)$options['limit'], $options['maxLimit']);
return $options;
}
}
45 changes: 29 additions & 16 deletions cake/tests/cases/libs/controller/components/paginator.test.php
View file
Expand Up @@ -582,14 +582,11 @@ function testMergeOptionsModelSpecific() {
'paramType' => 'named',
)
);
$result = $this->Paginator->mergeOptions('Silly', array());
$result = $this->Paginator->mergeOptions('Silly');
$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);
$result = $this->Paginator->mergeOptions('Post');
$expected = array('page' => 1, 'limit' => 10, 'paramType' => 'named', 'maxLimit' => 50);
$this->assertEquals($expected, $result);
}

Expand All @@ -609,12 +606,9 @@ function testMergeOptionsNamedParams() {
'maxLimit' => 100,
'paramType' => 'named',
);
$result = $this->Paginator->mergeOptions('Post', array());
$result = $this->Paginator->mergeOptions('Post');
$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');
}

/**
Expand All @@ -637,12 +631,9 @@ function testMergeOptionsQueryString() {
'maxLimit' => 100,
'paramType' => 'querystring',
);
$result = $this->Paginator->mergeOptions('Post', array());
$result = $this->Paginator->mergeOptions('Post');
$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');
}

/**
Expand All @@ -665,7 +656,7 @@ function testMergeOptionsDefaultWhiteList() {
'maxLimit' => 100,
'paramType' => 'named',
);
$result = $this->Paginator->mergeOptions('Post', array());
$result = $this->Paginator->mergeOptions('Post');
$expected = array('page' => 10, 'limit' => 10, 'maxLimit' => 100, 'paramType' => 'named');
$this->assertEquals($expected, $result);
}
Expand All @@ -690,7 +681,7 @@ function testMergeOptionsExtraWhitelist() {
'maxLimit' => 100,
'paramType' => 'named',
);
$result = $this->Paginator->mergeOptions('Post', array(), array('fields'));
$result = $this->Paginator->mergeOptions('Post', array('fields'));
$expected = array(
'page' => 10, 'limit' => 10, 'maxLimit' => 100, 'paramType' => 'named', 'fields' => array('bad.stuff')
);
Expand Down Expand Up @@ -737,4 +728,26 @@ function testValidateSortVirtualField() {

$this->assertEquals('desc', $result['order']['something']);
}

/**
* test that maxLimit is respected
*
* @return void
*/
function testCheckLimit() {
$result = $this->Paginator->checkLimit(array('limit' => 1000000, 'maxLimit' => 100));
$this->assertEquals(100, $result['limit']);

$result = $this->Paginator->checkLimit(array('limit' => 'sheep!', 'maxLimit' => 100));
$this->assertEquals(1, $result['limit']);

$result = $this->Paginator->checkLimit(array('limit' => '-1', 'maxLimit' => 100));
$this->assertEquals(1, $result['limit']);

$result = $this->Paginator->checkLimit(array('limit' => null, 'maxLimit' => 100));
$this->assertEquals(1, $result['limit']);

$result = $this->Paginator->checkLimit(array('limit' => 0, 'maxLimit' => 100));
$this->assertEquals(1, $result['limit']);
}
}

0 comments on commit e9d3fcf

Please sign in to comment.