Replace getPagingValue() with cursor methods

Summary:
Ref T7803. Prior to this change sequence, Query classes conflated paging values (the actual thing that goes in a "x > 3" clause) with cursor values (arbitrary identifiers which track where the user is in a result list).

Although the two can sometimes be the same, the vast majority of implementations are simpler and better when object IDs are used as cursors and paging values are derived from them.

The new stuff handles this in a consistent way, so we're free to separate getPagingValue() from paging. The new method is essentially getResultCursor().

This also implements getPageCursors(), which allows queries to return directional cursors. The inability to do this was a practical limitation blocking the implementation of T7803.

Test Plan:
  - Browsed a bunch of results and paged through queries.
  - Grepped for removed methods.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7803

Differential Revision: https://secure.phabricator.com/D12383

src/applications/feed/query/PhabricatorFeedQuery.php

@@ -112,7 +112,7 @@ final class PhabricatorFeedQuery
     );
   }
 
-  protected function getPagingValue($item) {
+  protected function getResultCursor($item) {
     if ($item instanceof PhabricatorFeedStory) {
       return $item->getChronologicalKey();
     }

src/applications/maniphest/query/ManiphestTaskQuery.php

@@ -896,20 +896,14 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery {
     return array_mergev($phids);
   }
 
-  protected function getPagingValue($result) {
+  protected function getResultCursor($result) {
     $id = $result->getID();
 
-    switch ($this->groupBy) {
+    if ($this->groupBy == self::GROUP_PROJECT) {
-      case self::GROUP_NONE:
+      return rtrim($id.'.'.$result->getGroupByProjectPHID(), '.');;
-      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}'!");
     }
+
+    return $id;
   }
 
   public function getOrderableColumns() {

src/applications/notification/query/PhabricatorNotificationQuery.php

@@ -103,7 +103,7 @@ final class PhabricatorNotificationQuery
     return $this->formatWhereClause($where);
   }
 
-  protected function getPagingValue($item) {
+  protected function getResultCursor($item) {
     return $item->getChronologicalKey();
   }
 

src/applications/project/query/PhabricatorProjectQuery.php

@@ -112,15 +112,12 @@ final class PhabricatorProjectQuery
   }
 
   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');

src/applications/search/query/PhabricatorSearchDocumentQuery.php

@@ -69,7 +69,7 @@ final class PhabricatorSearchDocumentQuery
     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 '.

src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php

@@ -19,23 +19,35 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
   private $orderVector;
   private $builtinOrder;
 
-  protected function getPagingValue($result) {
+  protected function getPageCursors(array $page) {
-    if (!is_object($result)) {
+    return array(
-      // This interface can't be typehinted and PHP gets really angry if we
+      $this->getResultCursor(head($page)),
-      // call a method on a non-object, so add an explicit check here.
+      $this->getResultCursor(last($page)),
-      throw new Exception(pht('Expected object, got "%s"!', gettype($result)));
+    );
+  }
+
+  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) {
+    if ($this->beforeID !== null) {
-      $this->beforeID = $this->getPagingValue(last($page));
+      $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 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
   }
 
   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 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
     }
 
     $results = $this->execute();
+    $count = count($results);
 
     $sliced_results = $pager->sliceResults($results);
-
     if ($sliced_results) {
-      if ($pager->getBeforeID() || (count($results) > $pager->getPageSize())) {
+      list($before, $after) = $this->getPageCursors($sliced_results);
-        $pager->setNextPageID($this->getPagingValue(last($sliced_results)));
+
+      if ($pager->getBeforeID() || ($count > $limit)) {
+        $pager->setNextPageID($after);
       }
 
       if ($pager->getAfterID() ||
-         ($pager->getBeforeID() && (count($results) > $pager->getPageSize()))) {
+         ($pager->getBeforeID() && ($count > $limit))) {
-        $pager->setPrevPageID($this->getPagingValue(head($sliced_results)));
+        $pager->setPrevPageID($before);
       }
     }