From d112b6c15d8e8d9876d5f8f0800921729bbc9b77 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 27 Jan 2014 17:14:21 -0800 Subject: [PATCH] Add `diffusion.querycommits` and deprecate `diffusion.getcommits` Summary: Fixes T4344. `diffusion.getcommits` is nasty old bad news. Implement a modern query method. This method provides limit/paging in a somewhat abstract way so it's sort of ultramodern, but I didn't want the default behavior to return a million rows. I'll probably move more stuff toward this over time, now that cursor paging is pervasive. Here, we needed extra metadata (the identifier map) anyway. Test Plan: Used console to execute command. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T4344 Differential Revision: https://secure.phabricator.com/D8077 --- src/__phutil_library_map__.php | 2 + .../conduit/method/ConduitAPIMethod.php | 60 +++++++++++++- ...ConduitAPI_diffusion_getcommits_Method.php | 10 ++- ...nduitAPI_diffusion_querycommits_Method.php | 83 +++++++++++++++++++ .../diffusion/query/DiffusionCommitQuery.php | 5 +- 5 files changed, 156 insertions(+), 4 deletions(-) create mode 100644 src/applications/diffusion/conduit/ConduitAPI_diffusion_querycommits_Method.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 311b68e461..ebb752f4d4 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -168,6 +168,7 @@ phutil_register_library_map(array( 'ConduitAPI_diffusion_lastmodifiedquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_lastmodifiedquery_Method.php', 'ConduitAPI_diffusion_looksoon_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_looksoon_Method.php', 'ConduitAPI_diffusion_mergedcommitsquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_mergedcommitsquery_Method.php', + 'ConduitAPI_diffusion_querycommits_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_querycommits.php', 'ConduitAPI_diffusion_rawdiffquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_rawdiffquery_Method.php', 'ConduitAPI_diffusion_readmequery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_readmequery_Method.php', 'ConduitAPI_diffusion_refsquery_Method' => 'applications/diffusion/conduit/ConduitAPI_diffusion_refsquery_Method.php', @@ -2637,6 +2638,7 @@ phutil_register_library_map(array( 'ConduitAPI_diffusion_lastmodifiedquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method', 'ConduitAPI_diffusion_looksoon_Method' => 'ConduitAPI_diffusion_Method', 'ConduitAPI_diffusion_mergedcommitsquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method', + 'ConduitAPI_diffusion_querycommits_Method' => 'ConduitAPI_diffusion_Method', 'ConduitAPI_diffusion_rawdiffquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method', 'ConduitAPI_diffusion_readmequery_Method' => 'ConduitAPI_diffusion_abstractquery_Method', 'ConduitAPI_diffusion_refsquery_Method' => 'ConduitAPI_diffusion_abstractquery_Method', diff --git a/src/applications/conduit/method/ConduitAPIMethod.php b/src/applications/conduit/method/ConduitAPIMethod.php index e549f5755b..87604dc6d2 100644 --- a/src/applications/conduit/method/ConduitAPIMethod.php +++ b/src/applications/conduit/method/ConduitAPIMethod.php @@ -1,9 +1,8 @@ 'optional string', + 'after' => 'optional string', + 'limit' => 'optional int (default = 100)', + ); + } + + + /** + * @task pager + */ + protected function newPager(ConduitAPIRequest $request) { + $limit = $request->getValue('limit', 100); + $limit = min(1000, $limit); + $limit = max(1, $limit); + + $pager = id(new AphrontCursorPagerView()) + ->setPageSize($limit); + + $before_id = $request->getValue('before'); + if ($before_id !== null) { + $pager->setBeforeID($before_id); + } + + $after_id = $request->getValue('after'); + if ($after_id !== null) { + $pager->setAfterID($after_id); + } + + return $pager; + } + + + /** + * @task pager + */ + protected function addPagerResults( + array $results, + AphrontCursorPagerView $pager) { + + $results['cursor'] = array( + 'limit' => $pager->getPageSize(), + 'after' => $pager->getNextPageID(), + 'before' =>$pager->getPrevPageID(), + ); + + return $results; + } + + /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/applications/diffusion/conduit/ConduitAPI_diffusion_getcommits_Method.php b/src/applications/diffusion/conduit/ConduitAPI_diffusion_getcommits_Method.php index 60782ba91d..1ccc6a50a6 100644 --- a/src/applications/diffusion/conduit/ConduitAPI_diffusion_getcommits_Method.php +++ b/src/applications/diffusion/conduit/ConduitAPI_diffusion_getcommits_Method.php @@ -7,7 +7,15 @@ final class ConduitAPI_diffusion_getcommits_Method extends ConduitAPI_diffusion_Method { public function getMethodDescription() { - return "Retrieve Diffusion commit information."; + return pht('Retrieve Diffusion commit information.'); + } + + public function getMethodStatus() { + return self::METHOD_STATUS_DEPRECATED; + } + + public function getMethodStatusDescription() { + return pht('Obsoleted by diffusion.querycommits.'); } public function defineParamTypes() { diff --git a/src/applications/diffusion/conduit/ConduitAPI_diffusion_querycommits_Method.php b/src/applications/diffusion/conduit/ConduitAPI_diffusion_querycommits_Method.php new file mode 100644 index 0000000000..d4df1673ef --- /dev/null +++ b/src/applications/diffusion/conduit/ConduitAPI_diffusion_querycommits_Method.php @@ -0,0 +1,83 @@ +'; + } + + public function defineParamTypes() { + return array( + 'ids' => 'optional list', + 'phids' => 'optional list', + 'names' => 'optional list', + 'repositoryPHID' => 'optional phid', + ) + $this->getPagerParamTypes(); + } + + public function defineErrorTypes() { + return array(); + } + + protected function execute(ConduitAPIRequest $request) { + $query = id(new DiffusionCommitQuery()) + ->setViewer($request->getUser()); + + $repository_phid = $request->getValue('repositoryPHID'); + if ($repository_phid) { + $repository = id(new PhabricatorRepositoryQuery()) + ->setViewer($request->getUser()) + ->withPHIDs(array($repository_phid)) + ->executeOne(); + if ($repository) { + $query->withRepository($repository); + } + } + + $names = $request->getValue('names'); + if ($names) { + $query->withIdentifiers($names); + } + + $ids = $request->getValue('ids'); + if ($ids) { + $query->withIDs($ids); + } + + $phids = $request->getValue('phids'); + if ($phids) { + $query->withPHIDs($phids); + } + + $pager = $this->newPager($request); + $commits = $query->executeWithCursorPager($pager); + + $map = $query->getIdentifierMap(); + $map = mpull($map, 'getPHID'); + + $data = array(); + foreach ($commits as $commit) { + $data[$commit->getPHID()] = array( + 'id' => $commit->getID(), + 'phid' => $commit->getPHID(), + 'repositoryPHID' => $commit->getRepository()->getPHID(), + 'identifier' => $commit->getCommitIdentifier(), + 'epoch' => $commit->getEpoch(), + 'isImporting' => !$commit->isImported(), + ); + } + + $result = array( + 'data' => $data, + 'identifierMap' => nonempty($map, (object)array()), + ); + + return $this->addPagerResults($result, $pager); + } + +} diff --git a/src/applications/diffusion/query/DiffusionCommitQuery.php b/src/applications/diffusion/query/DiffusionCommitQuery.php index b5e7145e9b..3d3e48b29e 100644 --- a/src/applications/diffusion/query/DiffusionCommitQuery.php +++ b/src/applications/diffusion/query/DiffusionCommitQuery.php @@ -259,7 +259,8 @@ final class DiffusionCommitQuery // If we discarded all possible identifiers (e.g., they all referenced // bogus repositories or were all too short), make sure the query finds // nothing. - throw new PhabricatorEmptyQueryException('No commit identifiers.'); + throw new PhabricatorEmptyQueryException( + pht('No commit identifiers.')); } $where[] = '('.implode(' OR ', $sql).')'; @@ -286,6 +287,8 @@ final class DiffusionCommitQuery $this->repositoryIDs); } + $where[] = $this->buildPagingClause($conn_r); + return $this->formatWhereClause($where); }