diff --git a/src/applications/chatlog/PhabricatorChatLogQuery.php b/src/applications/chatlog/PhabricatorChatLogQuery.php index 02b1e1e56c..06e1f1ee5b 100644 --- a/src/applications/chatlog/PhabricatorChatLogQuery.php +++ b/src/applications/chatlog/PhabricatorChatLogQuery.php @@ -46,7 +46,7 @@ final class PhabricatorChatLogQuery $logs = $table->loadAllFromArray($data); - return $this->processResults($logs); + return $logs; } private function buildWhereClause($conn_r) { diff --git a/src/applications/feed/PhabricatorFeedQuery.php b/src/applications/feed/PhabricatorFeedQuery.php index 2c954c8b49..30331ecafd 100644 --- a/src/applications/feed/PhabricatorFeedQuery.php +++ b/src/applications/feed/PhabricatorFeedQuery.php @@ -41,9 +41,11 @@ final class PhabricatorFeedQuery $this->buildOrderClause($conn), $this->buildLimitClause($conn)); - $results = PhabricatorFeedStory::loadAllFromRows($data); + return $data; + } - return $this->processResults($results); + protected function willFilterPage(array $data) { + return PhabricatorFeedStory::loadAllFromRows($data); } private function buildJoinClause(AphrontDatabaseConnection $conn_r) { @@ -88,7 +90,10 @@ final class PhabricatorFeedQuery } protected function getPagingValue($item) { - return $item->getChronologicalKey(); + if ($item instanceof PhabricatorFeedStory) { + return $item->getChronologicalKey(); + } + return $item['chronologicalKey']; } } diff --git a/src/applications/paste/query/PhabricatorPasteQuery.php b/src/applications/paste/query/PhabricatorPasteQuery.php index fdbb14251a..0666e60a08 100644 --- a/src/applications/paste/query/PhabricatorPasteQuery.php +++ b/src/applications/paste/query/PhabricatorPasteQuery.php @@ -81,7 +81,7 @@ final class PhabricatorPasteQuery } } - return $this->processResults($pastes); + return $pastes; } protected function buildWhereClause($conn_r) { diff --git a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php index 9c070455bb..5ec7fd408e 100644 --- a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php +++ b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php @@ -40,7 +40,7 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery protected function nextPage(array $page) { if ($this->beforeID) { - $this->beforeID = $this->getPagingValue(head($page)); + $this->beforeID = $this->getPagingValue(last($page)); } else { $this->afterID = $this->getPagingValue(last($page)); } @@ -102,14 +102,13 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery } } - final protected function processResults(array $results) { + final protected function didLoadResults(array $results) { if ($this->beforeID) { $results = array_reverse($results, $preserve_keys = true); } return $results; } - final public function executeWithCursorPager(AphrontCursorPagerView $pager) { $this->setLimit($pager->getPageSize() + 1); @@ -123,12 +122,12 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery $sliced_results = $pager->sliceResults($results); - if ($this->beforeID || (count($results) > $pager->getPageSize())) { + if ($pager->getBeforeID() || (count($results) > $pager->getPageSize())) { $pager->setNextPageID($this->getPagingValue(last($sliced_results))); } - if ($this->afterID || - ($this->beforeID && (count($results) > $pager->getPageSize()))) { + if ($pager->getAfterID() || + ($pager->getBeforeID() && (count($results) > $pager->getPageSize()))) { $pager->setPrevPageID($this->getPagingValue(head($sliced_results))); } diff --git a/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php index fb204dead1..b33901aa2f 100644 --- a/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php +++ b/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php @@ -188,7 +188,8 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery { $page = $this->loadPage(); - $visible = $filter->apply($page); + $visible = $this->willFilterPage($page); + $visible = $filter->apply($visible); foreach ($visible as $key => $result) { ++$count; @@ -222,6 +223,8 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery { $this->nextPage($page); } while (true); + $results = $this->didLoadResults($results); + return $results; } @@ -275,4 +278,32 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery { */ abstract protected function nextPage(array $page); + + /** + * Hook for applying a page filter prior to the privacy filter. This allows + * you to drop some items from the result set without creating problems with + * pagination or cursor updates. + * + * @param list Results from `loadPage()`. + * @return list Objects for policy filtering. + * @task policyimpl + */ + protected function willFilterPage(array $page) { + return $page; + } + + + /** + * Hook for applying final adjustments before results are returned. This is + * used by @{class:PhabricatorCursorPagedPolicyAwareQuery} to reverse results + * that are queried during reverse paging. + * + * @param list Query results. + * @return list Final results. + * @task policyimpl + */ + protected function didLoadResults(array $results) { + return $results; + } + } diff --git a/src/view/control/AphrontCursorPagerView.php b/src/view/control/AphrontCursorPagerView.php index 04c5798523..36aecacac6 100644 --- a/src/view/control/AphrontCursorPagerView.php +++ b/src/view/control/AphrontCursorPagerView.php @@ -25,6 +25,7 @@ final class AphrontCursorPagerView extends AphrontView { private $nextPageID; private $prevPageID; + private $moreResults; private $uri; @@ -89,6 +90,7 @@ final class AphrontCursorPagerView extends AphrontView { if (count($results) > $this->getPageSize()) { $offset = ($this->beforeID ? count($results) - $this->getPageSize() : 0); $results = array_slice($results, $offset, $this->getPageSize(), true); + $this->moreResults = true; } return $results; } @@ -101,7 +103,7 @@ final class AphrontCursorPagerView extends AphrontView { $links = array(); - if ($this->beforeID || $this->afterID) { + if ($this->afterID || ($this->beforeID && $this->moreResults)) { $links[] = phutil_render_tag( 'a', array(