From 2d79229083437a10bb16c4eb8bff393506f9c887 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 26 May 2017 09:43:40 -0700 Subject: [PATCH] Modernize FeedQuery a little bit Summary: Ref T12762. Updates some conventions and methods. This has no (meaningful) behavioral changes. Test Plan: - Grepped for `setFilterPHIDs()`. - Viewed main feed, user feed, project feed. - Called `feed.query`. Reviewers: chad Reviewed By: chad Maniphest Tasks: T12762 Differential Revision: https://secure.phabricator.com/D18027 --- .../conduit/FeedQueryConduitAPIMethod.php | 10 ++-- .../feed/query/PhabricatorFeedQuery.php | 60 +++++++++---------- .../query/PhabricatorFeedSearchEngine.php | 2 +- ...PhabricatorPeopleProfileViewController.php | 2 +- .../PhabricatorProjectProfileController.php | 2 +- 5 files changed, 37 insertions(+), 39 deletions(-) diff --git a/src/applications/feed/conduit/FeedQueryConduitAPIMethod.php b/src/applications/feed/conduit/FeedQueryConduitAPIMethod.php index 661b70a7ab..bddc4f5921 100644 --- a/src/applications/feed/conduit/FeedQueryConduitAPIMethod.php +++ b/src/applications/feed/conduit/FeedQueryConduitAPIMethod.php @@ -67,16 +67,16 @@ final class FeedQueryConduitAPIMethod extends FeedConduitAPIMethod { if (!$limit) { $limit = $this->getDefaultLimit(); } - $filter_phids = $request->getValue('filterPHIDs'); - if (!$filter_phids) { - $filter_phids = array(); - } $query = id(new PhabricatorFeedQuery()) ->setLimit($limit) - ->setFilterPHIDs($filter_phids) ->setViewer($user); + $filter_phids = $request->getValue('filterPHIDs'); + if ($filter_phids) { + $query->withFilterPHIDs($filter_phids); + } + $after = $request->getValue('after'); if (strlen($after)) { $query->setAfterID($after); diff --git a/src/applications/feed/query/PhabricatorFeedQuery.php b/src/applications/feed/query/PhabricatorFeedQuery.php index 13cfb266ed..7335a6d5b1 100644 --- a/src/applications/feed/query/PhabricatorFeedQuery.php +++ b/src/applications/feed/query/PhabricatorFeedQuery.php @@ -6,7 +6,7 @@ final class PhabricatorFeedQuery private $filterPHIDs; private $chronologicalKeys; - public function setFilterPHIDs(array $phids) { + public function withFilterPHIDs(array $phids) { $this->filterPHIDs = $phids; return $this; } @@ -16,50 +16,46 @@ final class PhabricatorFeedQuery return $this; } + public function newResultObject() { + return new PhabricatorFeedStoryData(); + } + protected 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)); - - return $data; + // NOTE: We return raw rows from this method, which is a little unusual. + return $this->loadStandardPageRows($this->newResultObject()); } protected function willFilterPage(array $data) { return PhabricatorFeedStory::loadAllFromRows($data, $this->getViewer()); } - protected function buildJoinClause(AphrontDatabaseConnection $conn_r) { + protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) { + $joins = parent::buildJoinClauseParts($conn); + // 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, + $joins[] = qsprintf( + $conn, 'JOIN %T ref ON ref.chronologicalKey = story.chronologicalKey', $ref_table->getTableName()); + + return $joins; } - protected function buildWhereClause(AphrontDatabaseConnection $conn_r) { - $where = array(); + protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { + $where = parent::buildWhereClauseParts($conn); - if ($this->filterPHIDs) { + if ($this->filterPHIDs !== null) { $where[] = qsprintf( - $conn_r, + $conn, 'ref.objectPHID IN (%Ls)', $this->filterPHIDs); } - if ($this->chronologicalKeys) { + if ($this->chronologicalKeys !== null) { // NOTE: We want to use integers in the query so we can take advantage // of keys, but can't use %d on 32-bit systems. Make sure all the keys // are integers and then format them raw. @@ -73,21 +69,19 @@ final class PhabricatorFeedQuery } $where[] = qsprintf( - $conn_r, + $conn, 'ref.chronologicalKey IN (%Q)', implode(', ', $keys)); } - $where[] = $this->buildPagingClause($conn_r); - - return $this->formatWhereClause($where); + return $where; } - protected function buildGroupClause(AphrontDatabaseConnection $conn_r) { - if ($this->filterPHIDs) { - return qsprintf($conn_r, 'GROUP BY ref.chronologicalKey'); + protected function buildGroupClause(AphrontDatabaseConnection $conn) { + if ($this->filterPHIDs !== null) { + return qsprintf($conn, 'GROUP BY ref.chronologicalKey'); } else { - return qsprintf($conn_r, 'GROUP BY story.chronologicalKey'); + return qsprintf($conn, 'GROUP BY story.chronologicalKey'); } } @@ -120,6 +114,10 @@ final class PhabricatorFeedQuery return $item['chronologicalKey']; } + protected function getPrimaryTableAlias() { + return 'story'; + } + public function getQueryApplicationClass() { return 'PhabricatorFeedApplication'; } diff --git a/src/applications/feed/query/PhabricatorFeedSearchEngine.php b/src/applications/feed/query/PhabricatorFeedSearchEngine.php index b8caf60ae7..43a0b716ca 100644 --- a/src/applications/feed/query/PhabricatorFeedSearchEngine.php +++ b/src/applications/feed/query/PhabricatorFeedSearchEngine.php @@ -56,7 +56,7 @@ final class PhabricatorFeedSearchEngine $phids = array_mergev($phids); if ($phids) { - $query->setFilterPHIDs($phids); + $query->withFilterPHIDs($phids); } return $query; diff --git a/src/applications/people/controller/PhabricatorPeopleProfileViewController.php b/src/applications/people/controller/PhabricatorPeopleProfileViewController.php index 9db106081d..7bcab34b17 100644 --- a/src/applications/people/controller/PhabricatorPeopleProfileViewController.php +++ b/src/applications/people/controller/PhabricatorPeopleProfileViewController.php @@ -229,7 +229,7 @@ final class PhabricatorPeopleProfileViewController $viewer) { $query = new PhabricatorFeedQuery(); - $query->setFilterPHIDs( + $query->withFilterPHIDs( array( $user->getPHID(), )); diff --git a/src/applications/project/controller/PhabricatorProjectProfileController.php b/src/applications/project/controller/PhabricatorProjectProfileController.php index e47cc678bc..d9fe019751 100644 --- a/src/applications/project/controller/PhabricatorProjectProfileController.php +++ b/src/applications/project/controller/PhabricatorProjectProfileController.php @@ -73,7 +73,7 @@ final class PhabricatorProjectProfileController $stories = id(new PhabricatorFeedQuery()) ->setViewer($viewer) - ->setFilterPHIDs( + ->withFilterPHIDs( array( $project->getPHID(), ))