Moving limit checking into a separate method, and adding tests.

Removing $scope from being passed down to the options, it previously only allowed additional conditions to be set.
Updated tests.

cake/libs/controller/components/paginator.php

@@ -77,7 +77,7 @@ class PaginatorComponent extends Component {
  * 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
  */
@@ -87,7 +87,6 @@ class PaginatorComponent extends Component {
 			$scope = $object;
 			$object = null;
 		}
-		$assoc = null;
 
 		$object = $this->_getObject($object);
 
@@ -95,8 +94,9 @@ class PaginatorComponent extends Component {
 			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;
 
@@ -111,13 +111,6 @@ class PaginatorComponent extends Component {
 			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)) {
@@ -248,20 +241,20 @@ class PaginatorComponent extends Component {
  *
  * @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 = array_merge(
-			$defaults['paramType'] = 'named';
+			array('page' => 1, 'limit' => 20, 'maxLimit' => 100, 'paramType' => 'named'),
-		}
+			$defaults
+		);
 		switch ($defaults['paramType']) {
 			case 'named':
 				$request = $this->Controller->request->params['named'];
@@ -277,7 +270,7 @@ class PaginatorComponent extends Component {
 		$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);
 	}
 
 /**
@@ -322,4 +315,19 @@ class PaginatorComponent extends Component {
 	
 		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;
+	}
 }

cake/tests/cases/libs/controller/components/paginator.test.php

@@ -582,14 +582,11 @@ class PaginatorTest extends CakeTestCase {
 				'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));
+		$result = $this->Paginator->mergeOptions('Post');
-		$this->assertEquals(10, $result['limit']);
+		$expected = array('page' => 1, 'limit' => 10, 'paramType' => 'named', 'maxLimit' => 50);
-
-		$result = $this->Paginator->mergeOptions('Post', array('sort' => 'title'));
-		$expected = array('page' => 1, 'limit' => 10, 'paramType' => 'named', 'sort' => 'title', 'maxLimit' => 50);
 		$this->assertEquals($expected, $result);
 	}
 
@@ -609,12 +606,9 @@ class PaginatorTest extends CakeTestCase {
 			'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');
 	}
 
 /**
@@ -637,12 +631,9 @@ class PaginatorTest extends CakeTestCase {
 			'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');
 	}
 
 /**
@@ -665,7 +656,7 @@ class PaginatorTest extends CakeTestCase {
 			'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);
 	}
@@ -690,7 +681,7 @@ class PaginatorTest extends CakeTestCase {
 			'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')
 		);
@@ -737,4 +728,26 @@ class PaginatorTest extends CakeTestCase {
 
 		$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']);
+	}
 }