diff --git a/src/applications/feed/query/PhabricatorFeedQuery.php b/src/applications/feed/query/PhabricatorFeedQuery.php --- a/src/applications/feed/query/PhabricatorFeedQuery.php +++ b/src/applications/feed/query/PhabricatorFeedQuery.php @@ -112,7 +112,7 @@ ); } - protected function getPagingValue($item) { + protected function getResultCursor($item) { if ($item instanceof PhabricatorFeedStory) { return $item->getChronologicalKey(); } diff --git a/src/applications/maniphest/query/ManiphestTaskQuery.php b/src/applications/maniphest/query/ManiphestTaskQuery.php --- a/src/applications/maniphest/query/ManiphestTaskQuery.php +++ b/src/applications/maniphest/query/ManiphestTaskQuery.php @@ -896,20 +896,14 @@ return array_mergev($phids); } - protected function getPagingValue($result) { + protected function getResultCursor($result) { $id = $result->getID(); - switch ($this->groupBy) { - case self::GROUP_NONE: - case self::GROUP_STATUS: - case self::GROUP_PRIORITY: - case self::GROUP_OWNER: - return $id; - case self::GROUP_PROJECT: - return rtrim($id.'.'.$result->getGroupByProjectPHID(), '.'); - default: - throw new Exception("Unknown group query '{$this->groupBy}'!"); + if ($this->groupBy == self::GROUP_PROJECT) { + return rtrim($id.'.'.$result->getGroupByProjectPHID(), '.');; } + + return $id; } public function getOrderableColumns() { diff --git a/src/applications/notification/query/PhabricatorNotificationQuery.php b/src/applications/notification/query/PhabricatorNotificationQuery.php --- a/src/applications/notification/query/PhabricatorNotificationQuery.php +++ b/src/applications/notification/query/PhabricatorNotificationQuery.php @@ -103,7 +103,7 @@ return $this->formatWhereClause($where); } - protected function getPagingValue($item) { + protected function getResultCursor($item) { return $item->getChronologicalKey(); } diff --git a/src/applications/project/query/PhabricatorProjectQuery.php b/src/applications/project/query/PhabricatorProjectQuery.php --- a/src/applications/project/query/PhabricatorProjectQuery.php +++ b/src/applications/project/query/PhabricatorProjectQuery.php @@ -112,15 +112,12 @@ } protected function getPagingValueMap($cursor, array $keys) { + $project = $this->loadCursorObject($cursor); return array( - 'name' => $cursor, + 'name' => $project->getName(), ); } - protected function getPagingValue($result) { - return $result->getName(); - } - protected function loadPage() { $table = new PhabricatorProject(); $conn_r = $table->establishConnection('r'); diff --git a/src/applications/search/query/PhabricatorSearchDocumentQuery.php b/src/applications/search/query/PhabricatorSearchDocumentQuery.php --- a/src/applications/search/query/PhabricatorSearchDocumentQuery.php +++ b/src/applications/search/query/PhabricatorSearchDocumentQuery.php @@ -69,7 +69,7 @@ return 'PhabricatorSearchApplication'; } - protected function getPagingValue($result) { + protected function getResultCursor($result) { throw new Exception( pht( 'This query does not support cursor paging; it must be offset '. diff --git a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php --- a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php +++ b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php @@ -19,23 +19,35 @@ private $orderVector; private $builtinOrder; - protected function getPagingValue($result) { - if (!is_object($result)) { - // This interface can't be typehinted and PHP gets really angry if we - // call a method on a non-object, so add an explicit check here. - throw new Exception(pht('Expected object, got "%s"!', gettype($result))); + protected function getPageCursors(array $page) { + return array( + $this->getResultCursor(head($page)), + $this->getResultCursor(last($page)), + ); + } + + protected function getResultCursor($object) { + if (!is_object($object)) { + throw new Exception( + pht( + 'Expected object, got "%s".', + gettype($object))); } - return $result->getID(); + + return $object->getID(); } protected function nextPage(array $page) { // See getPagingViewer() for a description of this flag. $this->internalPaging = true; - if ($this->beforeID) { - $this->beforeID = $this->getPagingValue(last($page)); + if ($this->beforeID !== null) { + $page = array_reverse($page, $preserve_keys = true); + list($before, $after) = $this->getPageCursors($page); + $this->beforeID = $before; } else { - $this->afterID = $this->getPagingValue(last($page)); + list($before, $after) = $this->getPageCursors($page); + $this->afterID = $after; } } @@ -113,7 +125,9 @@ } final public function executeWithCursorPager(AphrontCursorPagerView $pager) { - $this->setLimit($pager->getPageSize() + 1); + $limit = $pager->getPageSize(); + + $this->setLimit($limit + 1); if ($pager->getAfterID()) { $this->setAfterID($pager->getAfterID()); @@ -122,17 +136,19 @@ } $results = $this->execute(); + $count = count($results); $sliced_results = $pager->sliceResults($results); - if ($sliced_results) { - if ($pager->getBeforeID() || (count($results) > $pager->getPageSize())) { - $pager->setNextPageID($this->getPagingValue(last($sliced_results))); + list($before, $after) = $this->getPageCursors($sliced_results); + + if ($pager->getBeforeID() || ($count > $limit)) { + $pager->setNextPageID($after); } if ($pager->getAfterID() || - ($pager->getBeforeID() && (count($results) > $pager->getPageSize()))) { - $pager->setPrevPageID($this->getPagingValue(head($sliced_results))); + ($pager->getBeforeID() && ($count > $limit))) { + $pager->setPrevPageID($before); } }