From b68eba074b1912c9ecc28faee10c7770523be39d Mon Sep 17 00:00:00 2001 From: guilhermeblanco Date: Wed, 16 Jan 2008 20:51:36 +0000 Subject: [PATCH] Fixed count bug in Doctrine_Pager that was wrong counting the total of results found. Added 3 new methods: Doctrine_Pager::getExecuted (checks if the Pager was already executed), Doctrine_Pager_Layout::execute (handy access to execute Pager query without having to access Doctrine_Pager instance) and Doctrine_Pager_Layout::processPage (processes the template of a given page and returns the parsed string) --- lib/Doctrine/Pager.php | 145 +++++++++++++++++++-------- lib/Doctrine/Pager/Exception.php | 4 +- lib/Doctrine/Pager/Layout.php | 75 +++++++++++--- lib/Doctrine/Pager/Range.php | 4 +- lib/Doctrine/Pager/Range/Jumping.php | 31 +++--- lib/Doctrine/Pager/Range/Sliding.php | 54 +++++----- 6 files changed, 217 insertions(+), 96 deletions(-) diff --git a/lib/Doctrine/Pager.php b/lib/Doctrine/Pager.php index 42297a776..868481e42 100644 --- a/lib/Doctrine/Pager.php +++ b/lib/Doctrine/Pager.php @@ -28,8 +28,8 @@ * @subpackage Pager * @license http://www.opensource.org/licenses/lgpl-license.php LGPL * @version $Revision$ - * @link www.phpdoctrine.com - * @since 1.0 + * @link www.phpdoctrine.org + * @since 0.9 */ class Doctrine_Pager { @@ -58,25 +58,30 @@ class Doctrine_Pager */ protected $_lastPage; + /** + * @var boolean $_executed Pager was initialized (called "execute" at least once) + */ + protected $_executed; + /** * __construct * * @param mixed $query Accepts either a Doctrine_Query object or a string - * (which does the Doctrine_Query class creation). + * (which does the Doctrine_Query class creation). * @param int $page Current page * @param int $maxPerPage Maximum itens per page * @return void */ public function __construct($query, $page, $maxPerPage = 0) { - $this->_setQuery($query); + $this->_setExecuted(false); - $this->_setMaxPerPage($maxPerPage); + $this->_setQuery($query); $this->_setPage($page); - $this->_initialize(); + $this->setMaxPerPage($maxPerPage); } @@ -85,15 +90,18 @@ class Doctrine_Pager * * Initialize Pager object calculating number of results * + * @param $params Optional parameters to Doctrine_Query::execute * @return void */ - protected function _initialize() + protected function _initialize($params = array()) { // retrieve the number of items found - $count = $this->getQuery()->count(); + $count = $this->getQuery()->count($params); $this->_setNumResults($count); $this->_adjustOffset(); + + $this->_setExecuted(true); } @@ -106,16 +114,45 @@ class Doctrine_Pager */ protected function _adjustOffset() { - // Define new total of pages - $this->_setLastPage( - max(1, ceil($this->getNumResults() / $this->getMaxPerPage())) - ); - $offset = ($this->getPage() - 1) * $this->getMaxPerPage(); + if (!$this->getExecuted()) { + // Define new total of pages + $this->_setLastPage( + max(1, ceil($this->getNumResults() / $this->getMaxPerPage())) + ); + $offset = ($this->getPage() - 1) * $this->getMaxPerPage(); - // Assign new offset and limit to Doctrine_Query object - $p = $this->getQuery(); - $p->offset($offset); - $p->limit($this->getMaxPerPage()); + // Assign new offset and limit to Doctrine_Query object + $p = $this->getQuery(); + $p->offset($offset); + $p->limit($this->getMaxPerPage()); + } + } + + + /** + * getExecuted + * + * Returns the check if Pager was already executed at least once + * + * @return boolen Pager was executed + */ + public function getExecuted() + { + return $this->_executed; + } + + + /** + * _setExecuted + * + * Defines if Pager was already executed + * + * @param $executed Pager was executed + * @return void + */ + protected function _setExecuted($executed) + { + $this->_executed = $executed; } @@ -128,7 +165,13 @@ class Doctrine_Pager */ public function getNumResults() { - return $this->_numResults; + if ($this->getExecuted()) { + return $this->_numResults; + } + + throw new Doctrine_Pager_Exception( + 'Cannot retrieve the number of results of a not yet executed Pager query' + ); } @@ -168,7 +211,13 @@ class Doctrine_Pager */ public function getLastPage() { - return $this->_lastPage; + if ($this->getExecuted()) { + return $this->_lastPage; + } + + throw new Doctrine_Pager_Exception( + 'Cannot retrieve the last page number of a not yet executed Pager query' + ); } @@ -212,7 +261,13 @@ class Doctrine_Pager */ public function getNextPage() { - return min($this->getPage() + 1, $this->getLastPage()); + if ($this->getExecuted()) { + return $this->_lastPage; + } + + throw new Doctrine_Pager_Exception( + 'Cannot retrieve the last page number of a not yet executed Pager query' + );return min($this->getPage() + 1, $this->getLastPage()); } @@ -225,7 +280,13 @@ class Doctrine_Pager */ public function getPreviousPage() { - return max($this->getPage() - 1, $this->getFirstPage()); + if ($this->getExecuted()) { + return max($this->getPage() - 1, $this->getFirstPage()); + } + + throw new Doctrine_Pager_Exception( + 'Cannot retrieve the previous page number of a not yet executed Pager query' + ); } @@ -238,7 +299,13 @@ class Doctrine_Pager */ public function haveToPaginate() { - return $this->getNumResults() > $this->getMaxPerPage(); + if ($this->getExecuted()) { + return $this->getNumResults() > $this->getMaxPerPage(); + } + + throw new Doctrine_Pager_Exception( + 'Cannot know if it is necessary to paginate a not yet executed Pager query' + ); } @@ -253,7 +320,7 @@ class Doctrine_Pager public function setPage($page) { $this->_setPage($page); - $this->_adjustOffset(); + $this->_setExecuted(false); } @@ -294,21 +361,6 @@ class Doctrine_Pager * @return void */ public function setMaxPerPage($max) - { - $this->_setMaxPerPage($max); - $this->_adjustOffset(); - } - - - /** - * _setMaxPerPage - * - * Defines the maximum number of itens per page - * - * @param $max maximum number of itens per page - * @return void - */ - private function _setMaxPerPage($max) { if ($max > 0) { $this->_maxPerPage = $max; @@ -317,6 +369,8 @@ class Doctrine_Pager } else { $this->_maxPerPage = abs($max); } + + $this->_setExecuted(false); } @@ -339,7 +393,7 @@ class Doctrine_Pager * Defines the maximum number of itens per page * * @param $query Accepts either a Doctrine_Query object or a string - * (which does the Doctrine_Query class creation). + * (which does the Doctrine_Query class creation). * @return void */ protected function _setQuery($query) @@ -354,15 +408,18 @@ class Doctrine_Pager /** * execute - * executes the query and populates the data set * - * @param $params Optional parameters to Doctrine_Query::execute - * @param $hydrationMode Hyddration Mode of Doctrine_Query::execute - * returned ResultSet. Doctrine::Default is FETCH_RECORD - * @return Doctrine_Collection the root collection + * Executes the query, populates the collection and then return it + * + * @param $params Optional parameters to Doctrine_Query::execute + * @param $hydrationMode Hydration Mode of Doctrine_Query::execute + * returned ResultSet. Doctrine::Default is FETCH_RECORD + * @return Doctrine_Collection The root collection */ public function execute($params = array(), $hydrationMode = Doctrine::FETCH_RECORD) { + $this->_initialize($params); + return $this->getQuery()->execute($params, $hydrationMode); } } diff --git a/lib/Doctrine/Pager/Exception.php b/lib/Doctrine/Pager/Exception.php index 2eee6f723..2e2368ec2 100644 --- a/lib/Doctrine/Pager/Exception.php +++ b/lib/Doctrine/Pager/Exception.php @@ -30,8 +30,8 @@ Doctrine::autoload('Doctrine_Exception'); * @subpackage Pager * @license http://www.opensource.org/licenses/lgpl-license.php LGPL * @version $Revision$ - * @link www.phpdoctrine.com - * @since 1.0 + * @link www.phpdoctrine.org + * @since 0.9 */ class Doctrine_Pager_Exception extends Doctrine_Exception { } \ No newline at end of file diff --git a/lib/Doctrine/Pager/Layout.php b/lib/Doctrine/Pager/Layout.php index 8170e57d5..6ab277a6e 100644 --- a/lib/Doctrine/Pager/Layout.php +++ b/lib/Doctrine/Pager/Layout.php @@ -30,8 +30,8 @@ Doctrine::autoload('Doctrine_Pager_Range'); * @subpackage Pager * @license http://www.opensource.org/licenses/lgpl-license.php LGPL * @version $Revision$ - * @link www.phpdoctrine.com - * @since 1.0 + * @link www.phpdoctrine.org + * @since 0.9 */ class Doctrine_Pager_Layout { @@ -120,6 +120,22 @@ class Doctrine_Pager_Layout } + /** + * execute + * + * Handy method to execute the query without need to retrieve the Pager instance + * + * @param $params Optional parameters to Doctrine_Query::execute + * @param $hydrationMode Hydration Mode of Doctrine_Query::execute + * returned ResultSet. Doctrine::Default is FETCH_RECORD + * @return Doctrine_Collection The root collection + */ + public function execute($params = array(), $hydrationMode = Doctrine::FETCH_RECORD) + { + return $this->getPager()->execute($params, $hydrationMode); + } + + /** * getPagerRange * @@ -255,8 +271,8 @@ class Doctrine_Pager_Layout { $this->_separatorTemplate = $separatorTemplate; } - - + + /** * addMaskReplacement * @@ -279,8 +295,8 @@ class Doctrine_Pager_Layout ); } } - - + + /** * removeMaskReplacement * @@ -333,10 +349,8 @@ class Doctrine_Pager_Layout for ($i = 0, $l = count($range); $i < $l; $i++) { // Define some optional mask values $options['page_number'] = $range[$i]; - $options['page'] = $range[$i]; // Handy assignment for URLs - $options['url'] = $this->_parseUrl($options); - $str .= $this->_parseTemplate($options); + $str .= $this->processPage($options); // Apply separator between pages if ($i < $l - 1) { @@ -352,8 +366,39 @@ class Doctrine_Pager_Layout echo $str; } + /** - * simply calls display, and returns the output. + * processPage + * + * Parses the template and returns the string of a processed page + * + * @param array Optional parameters to be applied in template and url mask + * @return string Processed template for the given page + */ + public function processPage($options = array()) + { + // Check if at least basic options are defined + if (!isset($options['page_number'])) { + throw new Doctrine_Pager_Exception( + 'Cannot process template of the given page. ' . + 'Missing at least one of needed parameters: \'page\' or \'page_number\'' + ); + + // Should never reach here + return ''; + } + + // Assign "page" options index if not defined yet + if (!isset($this->_maskReplacements['page']) && !isset($options['page'])) { + $options['page'] = $options['page_number']; + } + + return $this->_parseTemplate($options); + } + + + /** + * Simply calls display, and returns the output. */ public function __toString() { @@ -366,14 +411,15 @@ class Doctrine_Pager_Layout * * Process the template of a given page and return the processed template * - * @param $options Optional parameters to be applied in template and url mask + * @param array Optional parameters to be applied in template and url mask * @return string */ protected function _parseTemplate($options = array()) { $str = ''; - if (isset($options['page_number']) && $options['page_number'] == $this->getPager()->getPage()) { + // If given page is the current active one + if ($options['page_number'] == $this->getPager()->getPage()) { $str = $this->_parseMaskReplacements($this->getSelectedTemplate()); } @@ -382,8 +428,11 @@ class Doctrine_Pager_Layout $str = $this->_parseMaskReplacements($this->getTemplate()); } + // Defining "url" options index to allow {%url} mask + $options['url'] = $this->_parseUrl($options); + $replacements = array(); - + foreach ($options as $k => $v) { $replacements['{%'.$k.'}'] = $v; } diff --git a/lib/Doctrine/Pager/Range.php b/lib/Doctrine/Pager/Range.php index a91eb8a17..2b584583f 100644 --- a/lib/Doctrine/Pager/Range.php +++ b/lib/Doctrine/Pager/Range.php @@ -28,8 +28,8 @@ * @subpackage Pager * @license http://www.opensource.org/licenses/lgpl-license.php LGPL * @version $Revision$ - * @link www.phpdoctrine.com - * @since 1.0 + * @link www.phpdoctrine.org + * @since 0.9 */ abstract class Doctrine_Pager_Range { diff --git a/lib/Doctrine/Pager/Range/Jumping.php b/lib/Doctrine/Pager/Range/Jumping.php index 09507a380..20de79d35 100644 --- a/lib/Doctrine/Pager/Range/Jumping.php +++ b/lib/Doctrine/Pager/Range/Jumping.php @@ -30,8 +30,8 @@ Doctrine::autoload('Doctrine_Pager_Range'); * @subpackage Pager * @license http://www.opensource.org/licenses/lgpl-license.php LGPL * @version $Revision$ - * @link www.phpdoctrine.com - * @since 1.0 + * @link www.phpdoctrine.org + * @since 0.9 */ class Doctrine_Pager_Range_Jumping extends Doctrine_Pager_Range { @@ -95,19 +95,26 @@ class Doctrine_Pager_Range_Jumping extends Doctrine_Pager_Range public function rangeAroundPage() { $pager = $this->getPager(); - $page = $pager->getPage(); - // Define initial assignments for StartPage and EndPage - $startPage = $page - ($page - 1) % $this->getChunkLength(); - $endPage = ($startPage + $this->getChunkLength()) - 1; + if ($pager->getExecuted()) { + $page = $pager->getPage(); - // Check for EndPage out-range - if ($endPage > $pager->getLastPage()) { - $endPage = $pager->getLastPage(); + // Define initial assignments for StartPage and EndPage + $startPage = $page - ($page - 1) % $this->getChunkLength(); + $endPage = ($startPage + $this->getChunkLength()) - 1; + + // Check for EndPage out-range + if ($endPage > $pager->getLastPage()) { + $endPage = $pager->getLastPage(); + } + + // No need to check for out-range in start, it will never happens + + return range($startPage, $endPage); } - // No need to check for out-range in start, it will never happens - - return range($startPage, $endPage); + throw new Doctrine_Pager_Exception( + 'Cannot retrieve the range around the page of a not yet executed Pager query' + ); } } diff --git a/lib/Doctrine/Pager/Range/Sliding.php b/lib/Doctrine/Pager/Range/Sliding.php index 7daca9e8d..6a0032f0b 100644 --- a/lib/Doctrine/Pager/Range/Sliding.php +++ b/lib/Doctrine/Pager/Range/Sliding.php @@ -30,8 +30,8 @@ Doctrine::autoload('Doctrine_Pager_Range'); * @subpackage Pager * @license http://www.opensource.org/licenses/lgpl-license.php LGPL * @version $Revision$ - * @link www.phpdoctrine.com - * @since 1.0 + * @link www.phpdoctrine.org + * @since 0.9 */ class Doctrine_Pager_Range_Sliding extends Doctrine_Pager_Range { @@ -100,29 +100,37 @@ class Doctrine_Pager_Range_Sliding extends Doctrine_Pager_Range public function rangeAroundPage() { $pager = $this->getPager(); - $page = $pager->getPage(); - $pages = $pager->getLastPage(); - $chunk = $this->getChunkLength(); - if ($chunk > $pages) { - $chunk = $pages; + if ($pager->getExecuted()) { + $page = $pager->getPage(); + $pages = $pager->getLastPage(); + + $chunk = $this->getChunkLength(); + + if ($chunk > $pages) { + $chunk = $pages; + } + + $chunkStart = $page - (floor($chunk / 2)); + $chunkEnd = $page + (ceil($chunk / 2)-1); + + if ($chunkStart < 1) { + $adjust = 1 - $chunkStart; + $chunkStart = 1; + $chunkEnd = $chunkEnd + $adjust; + } + + if ($chunkEnd > $pages) { + $adjust = $chunkEnd - $pages; + $chunkStart = $chunkStart - $adjust; + $chunkEnd = $pages; + } + + return range($chunkStart, $chunkEnd); } - $chunkStart = $page - (floor($chunk / 2)); - $chunkEnd = $page + (ceil($chunk / 2)-1); - - if ($chunkStart < 1) { - $adjust = 1 - $chunkStart; - $chunkStart = 1; - $chunkEnd = $chunkEnd + $adjust; - } - if ($chunkEnd > $pages) { - $adjust = $chunkEnd - $pages; - $chunkStart = $chunkStart - $adjust; - $chunkEnd = $pages; - } - - return range($chunkStart, $chunkEnd); - + throw new Doctrine_Pager_Exception( + 'Cannot retrieve the range around the page of a not yet executed Pager query' + ); } }