diff --git a/src/applications/chatlog/PhabricatorChatLogQuery.php b/src/applications/chatlog/PhabricatorChatLogQuery.php index 02b1e1e56..06e1f1ee5 100644 --- a/src/applications/chatlog/PhabricatorChatLogQuery.php +++ b/src/applications/chatlog/PhabricatorChatLogQuery.php @@ -1,73 +1,73 @@ channels = $channels; return $this; } public function withMaximumEpoch($epoch) { $this->maximumEpoch = $epoch; return $this; } public function loadPage() { $table = new PhabricatorChatLogEvent(); $conn_r = $table->establishConnection('r'); $data = queryfx_all( $conn_r, 'SELECT * FROM %T e %Q %Q %Q', $table->getTableName(), $this->buildWhereClause($conn_r), $this->buildOrderClause($conn_r), $this->buildLimitClause($conn_r)); $logs = $table->loadAllFromArray($data); - return $this->processResults($logs); + return $logs; } private function buildWhereClause($conn_r) { $where = array(); $where[] = $this->buildPagingClause($conn_r); if ($this->maximumEpoch) { $where[] = qsprintf( $conn_r, 'epoch <= %d', $this->maximumEpoch); } if ($this->channels) { $where[] = qsprintf( $conn_r, 'channel IN (%Ls)', $this->channels); } return $this->formatWhereClause($where); } } diff --git a/src/applications/feed/PhabricatorFeedQuery.php b/src/applications/feed/PhabricatorFeedQuery.php index 2c954c8b4..30331ecaf 100644 --- a/src/applications/feed/PhabricatorFeedQuery.php +++ b/src/applications/feed/PhabricatorFeedQuery.php @@ -1,94 +1,99 @@ filterPHIDs = $phids; return $this; } public function loadPage() { $story_table = new PhabricatorFeedStoryData(); $conn = $story_table->establishConnection('r'); $data = queryfx_all( $conn, 'SELECT story.* FROM %T story %Q %Q %Q %Q %Q', $story_table->getTableName(), $this->buildJoinClause($conn), $this->buildWhereClause($conn), $this->buildGroupClause($conn), $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) { // NOTE: We perform this join unconditionally (even if we have no filter // PHIDs) to omit rows which have no story references. These story data // rows are notifications or realtime alerts. $ref_table = new PhabricatorFeedStoryReference(); return qsprintf( $conn_r, 'JOIN %T ref ON ref.chronologicalKey = story.chronologicalKey', $ref_table->getTableName()); } private function buildWhereClause(AphrontDatabaseConnection $conn_r) { $where = array(); if ($this->filterPHIDs) { $where[] = qsprintf( $conn_r, 'ref.objectPHID IN (%Ls)', $this->filterPHIDs); } $where[] = $this->buildPagingClause($conn_r); return $this->formatWhereClause($where); } private function buildGroupClause(AphrontDatabaseConnection $conn_r) { return qsprintf( $conn_r, 'GROUP BY '.($this->filterPHIDs ? 'ref.chronologicalKey' : 'story.chronologicalKey')); } protected function getPagingColumn() { return ($this->filterPHIDs ? 'ref.chronologicalKey' : 'story.chronologicalKey'); } 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 fdbb14251..0666e60a0 100644 --- a/src/applications/paste/query/PhabricatorPasteQuery.php +++ b/src/applications/paste/query/PhabricatorPasteQuery.php @@ -1,123 +1,123 @@ ids = $ids; return $this; } public function withPHIDs(array $phids) { $this->phids = $phids; return $this; } public function withAuthorPHIDs(array $phids) { $this->authorPHIDs = $phids; return $this; } public function withParentPHIDs(array $phids) { $this->parentPHIDs = $phids; return $this; } public function needContent($need_content) { $this->needContent = $need_content; return $this; } public function loadPage() { $table = new PhabricatorPaste(); $conn_r = $table->establishConnection('r'); $data = queryfx_all( $conn_r, 'SELECT paste.* FROM %T paste %Q %Q %Q', $table->getTableName(), $this->buildWhereClause($conn_r), $this->buildOrderClause($conn_r), $this->buildLimitClause($conn_r)); $pastes = $table->loadAllFromArray($data); if ($pastes && $this->needContent) { $file_phids = mpull($pastes, 'getFilePHID'); $files = id(new PhabricatorFile())->loadAllWhere( 'phid IN (%Ls)', $file_phids); $files = mpull($files, null, 'getPHID'); foreach ($pastes as $paste) { $file = idx($files, $paste->getFilePHID()); if ($file) { $paste->attachContent($file->loadFileData()); } else { $paste->attachContent(''); } } } - return $this->processResults($pastes); + return $pastes; } protected function buildWhereClause($conn_r) { $where = array(); $where[] = $this->buildPagingClause($conn_r); if ($this->ids) { $where[] = qsprintf( $conn_r, 'id IN (%Ld)', $this->ids); } if ($this->phids) { $where[] = qsprintf( $conn_r, 'phid IN (%Ls)', $this->phids); } if ($this->authorPHIDs) { $where[] = qsprintf( $conn_r, 'authorPHID IN (%Ls)', $this->authorPHIDs); } if ($this->parentPHIDs) { $where[] = qsprintf( $conn_r, 'parentPHID IN (%Ls)', $this->parentPHIDs); } return $this->formatWhereClause($where); } } diff --git a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php index 9c070455b..5ec7fd408 100644 --- a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php +++ b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php @@ -1,138 +1,137 @@ getID(); } protected function getReversePaging() { return false; } 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)); } } final public function setAfterID($object_id) { $this->afterID = $object_id; return $this; } final public function setBeforeID($object_id) { $this->beforeID = $object_id; return $this; } final protected function buildLimitClause(AphrontDatabaseConnection $conn_r) { if ($this->getRawResultLimit()) { return qsprintf($conn_r, 'LIMIT %d', $this->getRawResultLimit()); } else { return ''; } } final protected function buildPagingClause( AphrontDatabaseConnection $conn_r) { if ($this->beforeID) { return qsprintf( $conn_r, '%Q %Q %s', $this->getPagingColumn(), $this->getReversePaging() ? '<' : '>', $this->beforeID); } else if ($this->afterID) { return qsprintf( $conn_r, '%Q %Q %s', $this->getPagingColumn(), $this->getReversePaging() ? '>' : '<', $this->afterID); } return null; } final protected function buildOrderClause(AphrontDatabaseConnection $conn_r) { if ($this->beforeID) { return qsprintf( $conn_r, 'ORDER BY %Q %Q', $this->getPagingColumn(), $this->getReversePaging() ? 'DESC' : 'ASC'); } else { return qsprintf( $conn_r, 'ORDER BY %Q %Q', $this->getPagingColumn(), $this->getReversePaging() ? 'ASC' : 'DESC'); } } - 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); if ($pager->getAfterID()) { $this->setAfterID($pager->getAfterID()); } else if ($pager->getBeforeID()) { $this->setBeforeID($pager->getBeforeID()); } $results = $this->execute(); $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))); } return $sliced_results; } } diff --git a/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php index fb204dead..b33901aa2 100644 --- a/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php +++ b/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php @@ -1,278 +1,309 @@ setViewer($user) * ->withConstraint($example) * ->execute(); * * Normally, you should extend @{class:PhabricatorCursorPagedPolicyAwareQuery}, * not this class. @{class:PhabricatorCursorPagedPolicyAwareQuery} provides a * more practical interface for building usable queries against most object * types. * * NOTE: Although this class extends @{class:PhabricatorOffsetPagedQuery}, * offset paging with policy filtering is not efficient. All results must be * loaded into the application and filtered here: skipping `N` rows via offset * is an `O(N)` operation with a large constant. Prefer cursor-based paging * with @{class:PhabricatorCursorPagedPolicyAwareQuery}, which can filter far * more efficiently in MySQL. * * @task config Query Configuration * @task exec Executing Queries * @task policyimpl Policy Query Implementation */ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery { private $viewer; private $raisePolicyExceptions; private $rawResultLimit; private $capabilities; /* -( Query Configuration )------------------------------------------------ */ /** * Set the viewer who is executing the query. Results will be filtered * according to the viewer's capabilities. You must set a viewer to execute * a policy query. * * @param PhabricatorUser The viewing user. * @return this * @task config */ final public function setViewer(PhabricatorUser $viewer) { $this->viewer = $viewer; return $this; } /** * Get the query's viewer. * * @return PhabricatorUser The viewing user. * @task config */ final public function getViewer() { return $this->viewer; } /** * @task config */ final public function requireCapabilities(array $capabilities) { $this->capabilities = $capabilities; return $this; } /* -( Query Execution )---------------------------------------------------- */ /** * Execute the query, expecting a single result. This method simplifies * loading objects for detail pages or edit views. * * // Load one result by ID. * $obj = id(new ExampleQuery()) * ->setViewer($user) * ->withIDs(array($id)) * ->executeOne(); * if (!$obj) { * return new Aphront404Response(); * } * * If zero results match the query, this method returns `null`. * * If one result matches the query, this method returns that result. * * If two or more results match the query, this method throws an exception. * You should use this method only when the query constraints guarantee at * most one match (e.g., selecting a specific ID or PHID). * * If one result matches the query but it is caught by the policy filter (for * example, the user is trying to view or edit an object which exists but * which they do not have permission to see) a policy exception is thrown. * * @return mixed Single result, or null. * @task exec */ final public function executeOne() { $this->raisePolicyExceptions = true; try { $results = $this->execute(); } catch (Exception $ex) { $this->raisePolicyExceptions = false; throw $ex; } if (count($results) > 1) { throw new Exception("Expected a single result!"); } if (!$results) { return null; } return head($results); } /** * Execute the query, loading all visible results. * * @return list Result objects. * @task exec */ final public function execute() { if (!$this->viewer) { throw new Exception("Call setViewer() before execute()!"); } $results = array(); $filter = new PhabricatorPolicyFilter(); $filter->setViewer($this->viewer); if (!$this->capabilities) { $capabilities = array( PhabricatorPolicyCapability::CAN_VIEW, ); } else { $capabilities = $this->capabilities; } $filter->requireCapabilities($capabilities); $filter->raisePolicyExceptions($this->raisePolicyExceptions); $offset = (int)$this->getOffset(); $limit = (int)$this->getLimit(); $count = 0; if ($limit) { $need = $offset + $limit; } else { $need = 0; } $this->willExecute(); do { if ($need) { $this->rawResultLimit = min($need - $count, 1024); } else { $this->rawResultLimit = 0; } $page = $this->loadPage(); - $visible = $filter->apply($page); + $visible = $this->willFilterPage($page); + $visible = $filter->apply($visible); foreach ($visible as $key => $result) { ++$count; // If we have an offset, we just ignore that many results and start // storing them only once we've hit the offset. This reduces memory // requirements for large offsets, compared to storing them all and // slicing them away later. if ($count > $offset) { $results[$key] = $result; } if ($need && ($count >= $need)) { // If we have all the rows we need, break out of the paging query. break 2; } } if (!$this->rawResultLimit) { // If we don't have a load count, we loaded all the results. We do // not need to load another page. break; } if (count($page) < $this->rawResultLimit) { // If we have a load count but the unfiltered results contained fewer // objects, we know this was the last page of objects; we do not need // to load another page because we can deduce it would be empty. break; } $this->nextPage($page); } while (true); + $results = $this->didLoadResults($results); + return $results; } /* -( Policy Query Implementation )---------------------------------------- */ /** * Get the number of results @{method:loadPage} should load. If the value is * 0, @{method:loadPage} should load all available results. * * @return int The number of results to load, or 0 for all results. * @task policyimpl */ final protected function getRawResultLimit() { return $this->rawResultLimit; } /** * Hook invoked before query execution. Generally, implementations should * reset any internal cursors. * * @return void * @task policyimpl */ protected function willExecute() { return; } /** * Load a raw page of results. Generally, implementations should load objects * from the database. They should attempt to return the number of results * hinted by @{method:getRawResultLimit}. * * @return list List of filterable policy objects. * @task policyimpl */ abstract protected function loadPage(); /** * Update internal state so that the next call to @{method:loadPage} will * return new results. Generally, you should adjust a cursor position based * on the provided result page. * * @param list The current page of results. * @return void * @task policyimpl */ 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 04c579852..36aecacac 100644 --- a/src/view/control/AphrontCursorPagerView.php +++ b/src/view/control/AphrontCursorPagerView.php @@ -1,143 +1,145 @@ pageSize = max(1, $page_size); return $this; } final public function getPageSize() { return $this->pageSize; } final public function setURI(PhutilURI $uri) { $this->uri = $uri; return $this; } final public function readFromRequest(AphrontRequest $request) { $this->uri = $request->getRequestURI(); $this->afterID = $request->getStr('after'); $this->beforeID = $request->getStr('before'); return $this; } final public function setAfterID($after_id) { $this->afterID = $after_id; return $this; } final public function getAfterID() { return $this->afterID; } final public function setBeforeID($before_id) { $this->beforeID = $before_id; return $this; } final public function getBeforeID() { return $this->beforeID; } final public function setNextPageID($next_page_id) { $this->nextPageID = $next_page_id; return $this; } final public function getNextPageID() { return $this->nextPageID; } final public function setPrevPageID($prev_page_id) { $this->prevPageID = $prev_page_id; return $this; } final public function getPrevPageID() { return $this->prevPageID; } final public function sliceResults(array $results) { 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; } public function render() { if (!$this->uri) { throw new Exception( "You must call setURI() before you can call render()."); } $links = array(); - if ($this->beforeID || $this->afterID) { + if ($this->afterID || ($this->beforeID && $this->moreResults)) { $links[] = phutil_render_tag( 'a', array( 'href' => $this->uri ->alter('before', null) ->alter('after', null), ), "\xC2\xAB First"); } if ($this->prevPageID) { $links[] = phutil_render_tag( 'a', array( 'href' => $this->uri ->alter('after', null) ->alter('before', $this->prevPageID), ), "\xE2\x80\xB9 Prev"); } if ($this->nextPageID) { $links[] = phutil_render_tag( 'a', array( 'href' => $this->uri ->alter('after', $this->nextPageID) ->alter('before', null), ), "Next \xE2\x80\xBA"); } return '
'. implode('', $links). '
'; } }