From 9f1104229332369ff80fc0fb6867dea6077f08d7 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 24 Mar 2019 13:56:35 -0700 Subject: [PATCH 1/2] (stable) Fix an unusual internal cursor in Conpherence Summary: See . Conpherence calls `setAfterID()` and `setBeforeID()` directly on a subquery, but these methods no longer exist. Use a pager instead. This code probably shouldn't exist (we should use some other approach to fetch this data in most cases) but that's a larger change. Test Plan: Sent messages in a Conpherence thread. Before: fatal; after: success. Viewed the Conphrence menu, loaded threads, etc. Reviewers: amckinley Reviewed By: amckinley Differential Revision: https://secure.phabricator.com/D20318 --- .../query/ConpherenceThreadQuery.php | 34 +++++++++++++------ 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/src/applications/conpherence/query/ConpherenceThreadQuery.php b/src/applications/conpherence/query/ConpherenceThreadQuery.php index 5cd6489d65..9c6682a8a7 100644 --- a/src/applications/conpherence/query/ConpherenceThreadQuery.php +++ b/src/applications/conpherence/query/ConpherenceThreadQuery.php @@ -285,23 +285,35 @@ private function loadCoreHandles( } private function loadTransactionsAndHandles(array $conpherences) { - $query = id(new ConpherenceTransactionQuery()) - ->setViewer($this->getViewer()) - ->withObjectPHIDs(array_keys($conpherences)) - ->needHandles(true); + // NOTE: This is older code which has been modernized to the minimum + // standard required by T13266. It probably isn't the best available + // approach to the problems it solves. + + $limit = $this->getTransactionLimit(); + if ($limit) { + // fetch an extra for "show older" scenarios + $limit = $limit + 1; + } else { + $limit = 0xFFFF; + } + + $pager = id(new AphrontCursorPagerView()) + ->setPageSize($limit); // We have to flip these for the underlying query class. The semantics of // paging are tricky business. if ($this->afterTransactionID) { - $query->setBeforeID($this->afterTransactionID); + $pager->setBeforeID($this->afterTransactionID); } else if ($this->beforeTransactionID) { - $query->setAfterID($this->beforeTransactionID); + $pager->setAfterID($this->beforeTransactionID); } - if ($this->getTransactionLimit()) { - // fetch an extra for "show older" scenarios - $query->setLimit($this->getTransactionLimit() + 1); - } - $transactions = $query->execute(); + + $transactions = id(new ConpherenceTransactionQuery()) + ->setViewer($this->getViewer()) + ->withObjectPHIDs(array_keys($conpherences)) + ->needHandles(true) + ->executeWithCursorPager($pager); + $transactions = mgroup($transactions, 'getObjectPHID'); foreach ($conpherences as $phid => $conpherence) { $current_transactions = idx($transactions, $phid, array()); From d3df0a49f92a8be2f5b8a030af2f8fa5471cc45d Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 25 Mar 2019 07:27:24 -0700 Subject: [PATCH 2/2] (stable) Correct use of the paging API in Phame Summary: Ref T13266. This callsite is using the older API; swap it to use pagers. Test Plan: Viewed a Phame blog post with siblings, saw the previous/next posts linked. Reviewers: amckinley Reviewed By: amckinley Subscribers: nicolast Maniphest Tasks: T13263, T13266 Differential Revision: https://secure.phabricator.com/D20319 --- .../controller/post/PhamePostViewController.php | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/applications/phame/controller/post/PhamePostViewController.php b/src/applications/phame/controller/post/PhamePostViewController.php index 11d94d2f94..4fb01c4def 100644 --- a/src/applications/phame/controller/post/PhamePostViewController.php +++ b/src/applications/phame/controller/post/PhamePostViewController.php @@ -304,6 +304,15 @@ private function buildCommentForm(PhamePost $post, $timeline) { private function loadAdjacentPosts(PhamePost $post) { $viewer = $this->getViewer(); + $pager = id(new AphrontCursorPagerView()) + ->setPageSize(1); + + $prev_pager = id(clone $pager) + ->setAfterID($post->getID()); + + $next_pager = id(clone $pager) + ->setBeforeID($post->getID()); + $query = id(new PhamePostQuery()) ->setViewer($viewer) ->withVisibility(array(PhameConstants::VISIBILITY_PUBLISHED)) @@ -311,12 +320,10 @@ private function loadAdjacentPosts(PhamePost $post) { ->setLimit(1); $prev = id(clone $query) - ->setAfterID($post->getID()) - ->execute(); + ->executeWithCursorPager($prev_pager); $next = id(clone $query) - ->setBeforeID($post->getID()) - ->execute(); + ->executeWithCursorPager($next_pager); return array(head($prev), head($next)); }