Merge pull request #3018 from cakephp/3.0-paginator-revisit

3.0 paginator revisit

src/Controller/Component/PaginatorComponent.php

@@ -16,7 +16,9 @@ namespace Cake\Controller\Component;
 
 use Cake\Controller\Component;
 use Cake\Controller\ComponentRegistry;
+use Cake\Datasource\RepositoryInterface;
 use Cake\Error;
+use Cake\ORM\Query;
 use Cake\ORM\Table;
 
 /**
@@ -126,65 +128,57 @@ class PaginatorComponent extends Component {
  *
  * Would paginate using the `find('popular')` method.
  *
- * @param Table $object The table to paginate.
+ * You can also pass an already created instance of a query to this method:
+ *
+ * {{{
+ * $query = $this->Articles->find('popular')->matching('Tags', function($q) {
+ *	return $q->where(['name' => 'CakePHP'])
+ * });
+ * $results = $paginator->paginate($query);
+ * }}}
+ *
+ * @param Cake\Datasource\RepositoryInterface|Cake\ORM\Query $object The table or query to paginate.
  * @param array $settings The settings/configuration used for pagination.
  * @return array Query results
  * @throws \Cake\Error\NotFoundException
  */
 	public function paginate($object, array $settings = []) {
-		$alias = $object->alias();
+		if ($object instanceof Query) {
+			$query = $object;
+			$object = $query->repository();
+		}
 
+		$alias = $object->alias();
 		$options = $this->mergeOptions($alias, $settings);
 		$options = $this->validateSort($object, $options);
 		$options = $this->checkLimit($options);
 
-		$conditions = $fields = $limit = $page = null;
-		$order = [];
+		$options += ['page' => 1];
+		$options['page'] = intval($options['page']) < 1 ? 1 : (int)$options['page'];
 
-		if (!isset($options['conditions'])) {
-			$options['conditions'] = [];
-		}
+		$type = !empty($options['findType']) ? $options['findType'] : 'all';
+		unset($options['findType'], $options['maxLimit']);
 
-		extract($options);
-		$extra = array_diff_key($options, compact(
-			'conditions', 'fields', 'order', 'limit', 'page'
-		));
-
-		$type = 'all';
-		if (!empty($extra['findType'])) {
-			$type = $extra['findType'];
-		}
-		unset($extra['findType'], $extra['maxLimit']);
-
-		if (intval($page) < 1) {
-			$page = 1;
+		if (empty($query)) {
+			$query = $object->find($type);
 		}
-		$page = $options['page'] = (int)$page;
-
-		$parameters = compact('conditions', 'fields', 'order', 'limit', 'page');
-		$query = $object->find($type, array_merge($parameters, $extra));
 
+		$query->applyOptions($options);
 		$results = $query->all();
 		$numResults = count($results);
+		$count = $numResults ? $query->count() : 0;
 
 		$defaults = $this->getDefaults($alias, $settings);
 		unset($defaults[0]);
-		$count = 0;
-
-		if ($numResults) {
-			$count = $query->count();
-		}
 
+		$page = $options['page'];
+		$limit = $options['limit'];
 		$pageCount = intval(ceil($count / $limit));
 		$requestedPage = $page;
 		$page = max(min($page, $pageCount), 1);
 		$request = $this->_registry->getController()->request;
 
-		if (!is_array($order)) {
-			$order = (array)$order;
-		}
-
-		reset($order);
+		$order = (array)$options['order'];
 		$sortDefault = $directionDefault = false;
 		if (!empty($defaults['order']) && count($defaults['order']) == 1) {
 			$sortDefault = key($defaults['order']);
@@ -207,7 +201,7 @@ class PaginatorComponent extends Component {
 		);
 
 		if (!isset($request['paging'])) {
-			$request['paging'] = array();
+			$request['paging'] = [];
 		}
 		$request['paging'] = array_merge(
 			(array)$request['paging'],

src/Controller/Controller.php

@@ -656,7 +656,8 @@ class Controller extends Object implements EventListener {
  *
  * This method will also make the PaginatorHelper available in the view.
  *
- * @param Table|string $object Table to paginate (e.g: Table instance, or 'Model')
+ * @param Table|string|Query $object Table to paginate
+ * (e.g: Table instance, 'TableName' or a Query object)
  * @return ORM\ResultSet Query results
  * @link http://book.cakephp.org/3.0/en/controllers.html#Controller::paginate
  */

src/ORM/Query.php

@@ -426,7 +426,7 @@ class Query extends DatabaseQuery {
  * - having: Maps to the having method
  * - contain: Maps to the contain options for eager loading
  * - join: Maps to the join method
- * - join: Maps to the page method
+ * - page: Maps to the page method
  *
  * @return \Cake\ORM\Query
  */

tests/TestCase/Controller/Component/PaginatorComponentTest.php

@@ -46,7 +46,14 @@ class PaginatorComponentTest extends TestCase {
  *
  * @var array
  */
-	public $fixtures = array('core.post', 'core.author');
+	public $fixtures = array('core.post');
+
+/**
+ * Don't load data for fixtures for all tests
+ *
+ * @var boolean
+ */
+	public $autoFixtures = false;
 
 /**
  * setup
@@ -118,17 +125,18 @@ class PaginatorComponentTest extends TestCase {
 		$query = $this->_getMockFindQuery();
 		$table->expects($this->once())
 			->method('find')
-			->with('all', [
-				'conditions' => [],
+			->with('all')
+			->will($this->returnValue($query));
+
+		$query->expects($this->once())
+			->method('applyOptions')
+			->with([
 				'contain' => ['PaginatorAuthor'],
-				'fields' => null,
 				'group' => 'PaginatorPosts.published',
 				'limit' => 10,
 				'order' => ['PaginatorPosts.id' => 'ASC'],
 				'page' => 1,
-			])
-			->will($this->returnValue($query));
-
+			]);
 		$this->Paginator->paginate($table, $settings);
 	}
 
@@ -173,14 +181,15 @@ class PaginatorComponentTest extends TestCase {
 
 		$table->expects($this->once())
 			->method('find')
-			->with('all', [
-				'conditions' => [],
-				'fields' => null,
+			->with('all')
+			->will($this->returnValue($query));
+		$query->expects($this->once())
+			->method('applyOptions')
+			->with([
 				'limit' => 10,
 				'page' => 1,
 				'order' => ['PaginatorPosts.id' => 'DESC']
-			])
-			->will($this->returnValue($query));
+			]);
 
 		$this->Paginator->paginate($table, $settings);
 	}
@@ -201,14 +210,15 @@ class PaginatorComponentTest extends TestCase {
 
 		$table->expects($this->once())
 			->method('find')
-			->with('all', [
-				'conditions' => [],
-				'fields' => null,
+			->with('all')
+			->will($this->returnValue($query));
+		$query->expects($this->once())
+			->method('applyOptions')
+			->with([
 				'limit' => 10,
 				'page' => 1,
 				'order' => ['PaginatorPosts.id' => 'DESC']
-			])
-			->will($this->returnValue($query));
+			]);
 
 		$this->Paginator->paginate($table, $settings);
 		$this->assertEquals('PaginatorPosts.id', $this->request->params['paging']['PaginatorPosts']['sortDefault']);
@@ -370,14 +380,14 @@ class PaginatorComponentTest extends TestCase {
 
 		$table->expects($this->once())
 			->method('find')
-			->with('all', [
-				'fields' => null,
+			->with('all')
+			->will($this->returnValue($query));
+		$query->expects($this->once())->method('applyOptions')
+			->with([
 				'limit' => 20,
-				'conditions' => [],
 				'page' => 1,
-				'order' => ['PaginatorPosts.id' => 'asc'],
-			])
-			->will($this->returnValue($query));
+				'order' => ['PaginatorPosts.id' => 'asc']
+			]);
 
 		$this->request->query = [
 			'page' => 1,
@@ -415,6 +425,7 @@ class PaginatorComponentTest extends TestCase {
  * @return void
  */
 	public function testOutOfRangePageNumberGetsClamped() {
+		$this->loadFixtures('Post');
 		$this->request->query['page'] = 3000;
 
 		$table = TableRegistry::get('PaginatorPosts');
@@ -628,6 +639,7 @@ class PaginatorComponentTest extends TestCase {
  * @return void
  */
 	public function testPaginateCustomFind() {
+		$this->loadFixtures('Post');
 		$idExtractor = function ($result) {
 			$ids = [];
 			foreach ($result as $record) {
@@ -677,6 +689,7 @@ class PaginatorComponentTest extends TestCase {
  * @return void
  */
 	public function testPaginateCustomFindFieldsArray() {
+		$this->loadFixtures('Post');
 		$table = TableRegistry::get('PaginatorPosts');
 		$data = array('author_id' => 3, 'title' => 'Fourth Article', 'body' => 'Article Body, unpublished', 'published' => 'N');
 		$table->save(new \Cake\ORM\Entity($data));
@@ -718,19 +731,79 @@ class PaginatorComponentTest extends TestCase {
 		$query = $this->_getMockFindQuery();
 		$table->expects($this->once())
 			->method('find')
-			->with('published', [
-				'conditions' => [],
-				'order' => [],
-				'limit' => 2,
-				'fields' => null,
-				'page' => 1,
-			])
+			->with('published')
 			->will($this->returnValue($query));
 
+		$query->expects($this->once())->method('applyOptions')
+			->with(['limit' => 2, 'page' => 1, 'order' => []]);
 		$this->Paginator->paginate($table, $settings);
 	}
 
 /**
+ * Tests that it is possible to pass an already made query object to
+ * paginate()
+ *
+ * @return void
+ */
+	public function testPaginateQuery() {
+		$this->request->query = array('page' => '-1');
+		$settings = array(
+			'PaginatorPosts' => array(
+				'contain' => array('PaginatorAuthor'),
+				'maxLimit' => 10,
+				'group' => 'PaginatorPosts.published',
+				'order' => array('PaginatorPosts.id' => 'ASC')
+			)
+		);
+		$table = $this->_getMockPosts(['find']);
+		$query = $this->_getMockFindQuery($table);
+		$table->expects($this->never())->method('find');
+		$query->expects($this->once())
+			->method('applyOptions')
+			->with([
+				'contain' => ['PaginatorAuthor'],
+				'group' => 'PaginatorPosts.published',
+				'limit' => 10,
+				'order' => ['PaginatorPosts.id' => 'ASC'],
+				'page' => 1,
+			]);
+		$this->Paginator->paginate($query, $settings);
+	}
+
+/**
+ * Tests that passing a query object with a limit clause set will
+ * overwrite it with the passed defaults.
+ *
+ * @return void
+ */
+	public function testPaginateQueryWithLimit() {
+		$this->request->query = array('page' => '-1');
+		$settings = array(
+			'PaginatorPosts' => array(
+				'contain' => array('PaginatorAuthor'),
+				'maxLimit' => 10,
+				'limit' => 5,
+				'group' => 'PaginatorPosts.published',
+				'order' => array('PaginatorPosts.id' => 'ASC')
+			)
+		);
+		$table = $this->_getMockPosts(['find']);
+		$query = $this->_getMockFindQuery($table);
+		$query->limit(2);
+		$table->expects($this->never())->method('find');
+		$query->expects($this->once())
+			->method('applyOptions')
+			->with([
+				'contain' => ['PaginatorAuthor'],
+				'group' => 'PaginatorPosts.published',
+				'limit' => 5,
+				'order' => ['PaginatorPosts.id' => 'ASC'],
+				'page' => 1,
+			]);
+		$this->Paginator->paginate($query, $settings);
+	}
+
+/**
  * Helper method for making mocks.
  *
  * @param array $methods
@@ -740,7 +813,18 @@ class PaginatorComponentTest extends TestCase {
 		return $this->getMock(
 			'TestApp\Model\Table\PaginatorPostsTable',
 			$methods,
-			[['connection' => ConnectionManager::get('test'), 'alias' => 'PaginatorPosts']]
+			[[
+				'connection' => ConnectionManager::get('test'),
+				'alias' => 'PaginatorPosts',
+				'schema' => [
+					'id' => ['type' => 'integer'],
+					'author_id' => ['type' => 'integer', 'null' => false],
+					'title' => ['type' => 'string', 'null' => false],
+					'body' => 'text',
+					'published' => ['type' => 'string', 'length' => 1, 'default' => 'N'],
+					'_constraints' => ['primary' => ['type' => 'primary', 'columns' => ['id']]]
+				]
+			]]
 		);
 	}
 
@@ -749,9 +833,9 @@ class PaginatorComponentTest extends TestCase {
  *
  * @return Query
  */
-	protected function _getMockFindQuery() {
+	protected function _getMockFindQuery($table = null) {
 		$query = $this->getMockBuilder('Cake\ORM\Query')
-			->setMethods(['total', 'all', 'count'])
+			->setMethods(['total', 'all', 'count', 'applyOptions'])
 			->disableOriginalConstructor()
 			->getMock();
 
@@ -768,6 +852,7 @@ class PaginatorComponentTest extends TestCase {
 			->method('count')
 			->will($this->returnValue(2));
 
+		$query->repository($table);
 		return $query;
 	}