1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-22 13:30:55 +01:00

Fix pagination of fulltext search results

Summary:
Fixes T8285. Fulltext search relies on an underlying engine which can not realistically use cursor paging. This is unusual and creates some oddness.

Tweak a few numbers -- and how offsets are handled -- to separate the filtered offset and unfiltered offset.

Test Plan:
  - Set page size to 2.
  - Ran a query.
  - Paged forward and backward through results sensibly, seeing the full result set.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8285

Differential Revision: https://secure.phabricator.com/D17667
This commit is contained in:
epriestley 2017-04-12 14:48:18 -07:00
parent a7ebfc12c0
commit 4bf968148c
3 changed files with 23 additions and 18 deletions

View file

@ -9148,7 +9148,7 @@ phutil_register_library_map(array(
'PhabricatorSearchDocument' => 'PhabricatorSearchDAO', 'PhabricatorSearchDocument' => 'PhabricatorSearchDAO',
'PhabricatorSearchDocumentField' => 'PhabricatorSearchDAO', 'PhabricatorSearchDocumentField' => 'PhabricatorSearchDAO',
'PhabricatorSearchDocumentFieldType' => 'Phobject', 'PhabricatorSearchDocumentFieldType' => 'Phobject',
'PhabricatorSearchDocumentQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorSearchDocumentQuery' => 'PhabricatorPolicyAwareQuery',
'PhabricatorSearchDocumentRelationship' => 'PhabricatorSearchDAO', 'PhabricatorSearchDocumentRelationship' => 'PhabricatorSearchDAO',
'PhabricatorSearchDocumentTypeDatasource' => 'PhabricatorTypeaheadDatasource', 'PhabricatorSearchDocumentTypeDatasource' => 'PhabricatorTypeaheadDatasource',
'PhabricatorSearchEditController' => 'PhabricatorSearchBaseController', 'PhabricatorSearchEditController' => 'PhabricatorSearchBaseController',

View file

@ -1,10 +1,11 @@
<?php <?php
final class PhabricatorSearchDocumentQuery final class PhabricatorSearchDocumentQuery
extends PhabricatorCursorPagedPolicyAwareQuery { extends PhabricatorPolicyAwareQuery {
private $savedQuery; private $savedQuery;
private $objectCapabilities; private $objectCapabilities;
private $unfilteredOffset;
public function withSavedQuery(PhabricatorSavedQuery $query) { public function withSavedQuery(PhabricatorSavedQuery $query) {
$this->savedQuery = $query; $this->savedQuery = $query;
@ -20,11 +21,27 @@ final class PhabricatorSearchDocumentQuery
if ($this->objectCapabilities) { if ($this->objectCapabilities) {
return $this->objectCapabilities; return $this->objectCapabilities;
} }
return $this->getRequiredCapabilities(); return $this->getRequiredCapabilities();
} }
protected function willExecute() {
$this->unfilteredOffset = 0;
}
protected function loadPage() { protected function loadPage() {
$phids = $this->loadDocumentPHIDsWithoutPolicyChecks(); // NOTE: The offset and limit information in the inherited properties of
// this object represent a policy-filtered offset and limit, but the
// underlying query engine needs an unfiltered offset and limit. We keep
// track of an unfiltered result offset internally.
$query = id(clone($this->savedQuery))
->setParameter('offset', $this->unfilteredOffset)
->setParameter('limit', $this->getRawResultLimit());
$phids = PhabricatorSearchService::executeSearch($query);
$this->unfilteredOffset += count($phids);
$handles = id(new PhabricatorHandleQuery()) $handles = id(new PhabricatorHandleQuery())
->setViewer($this->getViewer()) ->setViewer($this->getViewer())
@ -69,25 +86,13 @@ final class PhabricatorSearchDocumentQuery
return $handles; return $handles;
} }
public function loadDocumentPHIDsWithoutPolicyChecks() {
$query = id(clone($this->savedQuery))
->setParameter('offset', $this->getOffset())
->setParameter('limit', $this->getRawResultLimit());
return PhabricatorSearchService::executeSearch($query);
}
public function getQueryApplicationClass() { public function getQueryApplicationClass() {
return 'PhabricatorSearchApplication'; return 'PhabricatorSearchApplication';
} }
protected function getResultCursor($result) {
throw new Exception(
pht(
'This query does not support cursor paging; it must be offset paged.'));
}
protected function nextPage(array $page) { protected function nextPage(array $page) {
$this->setOffset($this->getOffset() + count($page)); // We already updated the internal offset in `loadPage()` after loading
// results, so we do not need to make any additional state updates here.
return $this; return $this;
} }

View file

@ -55,7 +55,7 @@ final class PHUIPagerView extends AphrontView {
} }
public function willShowPagingControls() { public function willShowPagingControls() {
return $this->hasMorePages; return $this->hasMorePages || $this->getOffset();
} }
public function getHasMorePages() { public function getHasMorePages() {