mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-25 16:22:43 +01:00
Allow policy-aware queries to prefilter results
Summary: Provides a simple way for policy-aware queries to pre-filter results without needing to maintain separate cursors, and fixes a bunch of filter-related edge cases. - For reverse-paged cursor queries, we previously reversed each individual set of results. If the final result set is built out of multiple pages, it's in the wrong order overall, with each page in the correct order in sequence. Instead, reverse everything at the end. This also simplifies construction of queries. - `AphrontCursorPagerView` would always render a "<< First" link when paging backward, even if we were on the first page of results. - Add a filtering hook to let queries perform in-application pre-policy filtering as simply as possible (i.e., without maintaing their own cursors over the result sets). Test Plan: Made feed randomly prefilter half the results, and paged forward and backward. Observed correct result ordering, pagination, and next/previous links. Reviewers: btrahan, vrana Reviewed By: btrahan CC: aran Differential Revision: https://secure.phabricator.com/D3787
This commit is contained in:
parent
0b9101f3c5
commit
51c4b199d0
6 changed files with 50 additions and 13 deletions
|
@ -46,7 +46,7 @@ final class PhabricatorChatLogQuery
|
|||
|
||||
$logs = $table->loadAllFromArray($data);
|
||||
|
||||
return $this->processResults($logs);
|
||||
return $logs;
|
||||
}
|
||||
|
||||
private function buildWhereClause($conn_r) {
|
||||
|
|
|
@ -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'];
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -81,7 +81,7 @@ final class PhabricatorPasteQuery
|
|||
}
|
||||
}
|
||||
|
||||
return $this->processResults($pastes);
|
||||
return $pastes;
|
||||
}
|
||||
|
||||
protected function buildWhereClause($conn_r) {
|
||||
|
|
|
@ -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)));
|
||||
}
|
||||
|
||||
|
|
|
@ -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<wild> Results from `loadPage()`.
|
||||
* @return list<PhabricatorPolicyInterface> 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<PhabricatorPolicyInterface> Query results.
|
||||
* @return list<PhabricatorPolicyInterface> Final results.
|
||||
* @task policyimpl
|
||||
*/
|
||||
protected function didLoadResults(array $results) {
|
||||
return $results;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -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(
|
||||
|
|
Loading…
Reference in a new issue