From d00de495bcf513c2b59242a1b1aedf16d8475b98 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 23 Nov 2016 09:01:03 -0800 Subject: [PATCH] Separate "browse ref -> commit" and "commit -> upstream" hardpoints from `arc browse` workflow Summary: Ref T10895. Currently, the "browse as a commit" code does these lookups, but the code can't be reused. For T10895, I want to introduce "browse as revision", but it will need to do the same ref lookup. Separate this as a hardpoint so the code can be shared via hardpoint/ref infrastructure. Test Plan: Ran `arc browse master` and similar commands. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10895 Differential Revision: https://secure.phabricator.com/D16929 --- src/__phutil_library_map__.php | 4 + .../ArcanistBrowseCommitHardpointLoader.php | 81 ++++++++++++++++++ ...ArcanistBrowseCommitURIHardpointLoader.php | 82 ++++++++----------- src/browse/ref/ArcanistBrowseRef.php | 8 ++ .../ArcanistCommitUpstreamHardpointLoader.php | 59 +++++++++++++ src/loader/ArcanistHardpointLoader.php | 22 ++++- src/ref/ArcanistCommitRef.php | 18 ++++ 7 files changed, 224 insertions(+), 50 deletions(-) create mode 100644 src/browse/loader/ArcanistBrowseCommitHardpointLoader.php create mode 100644 src/loader/ArcanistCommitUpstreamHardpointLoader.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index bc0c78cc..51f87d36 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -42,6 +42,7 @@ phutil_register_library_map(array( 'ArcanistBraceFormattingXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistBraceFormattingXHPASTLinterRuleTestCase.php', 'ArcanistBranchRef' => 'ref/ArcanistBranchRef.php', 'ArcanistBranchWorkflow' => 'workflow/ArcanistBranchWorkflow.php', + 'ArcanistBrowseCommitHardpointLoader' => 'browse/loader/ArcanistBrowseCommitHardpointLoader.php', 'ArcanistBrowseCommitURIHardpointLoader' => 'browse/loader/ArcanistBrowseCommitURIHardpointLoader.php', 'ArcanistBrowseObjectNameURIHardpointLoader' => 'browse/loader/ArcanistBrowseObjectNameURIHardpointLoader.php', 'ArcanistBrowsePathURIHardpointLoader' => 'browse/loader/ArcanistBrowsePathURIHardpointLoader.php', @@ -84,6 +85,7 @@ phutil_register_library_map(array( 'ArcanistCommentStyleXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistCommentStyleXHPASTLinterRule.php', 'ArcanistCommentStyleXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistCommentStyleXHPASTLinterRuleTestCase.php', 'ArcanistCommitRef' => 'ref/ArcanistCommitRef.php', + 'ArcanistCommitUpstreamHardpointLoader' => 'loader/ArcanistCommitUpstreamHardpointLoader.php', 'ArcanistCommitWorkflow' => 'workflow/ArcanistCommitWorkflow.php', 'ArcanistCompilerLintRenderer' => 'lint/renderer/ArcanistCompilerLintRenderer.php', 'ArcanistComposerLinter' => 'lint/linter/ArcanistComposerLinter.php', @@ -480,6 +482,7 @@ phutil_register_library_map(array( 'ArcanistBraceFormattingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistBranchRef' => 'ArcanistRef', 'ArcanistBranchWorkflow' => 'ArcanistFeatureWorkflow', + 'ArcanistBrowseCommitHardpointLoader' => 'ArcanistHardpointLoader', 'ArcanistBrowseCommitURIHardpointLoader' => 'ArcanistBrowseURIHardpointLoader', 'ArcanistBrowseObjectNameURIHardpointLoader' => 'ArcanistBrowseURIHardpointLoader', 'ArcanistBrowsePathURIHardpointLoader' => 'ArcanistBrowseURIHardpointLoader', @@ -522,6 +525,7 @@ phutil_register_library_map(array( 'ArcanistCommentStyleXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistCommentStyleXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistCommitRef' => 'ArcanistRef', + 'ArcanistCommitUpstreamHardpointLoader' => 'ArcanistHardpointLoader', 'ArcanistCommitWorkflow' => 'ArcanistWorkflow', 'ArcanistCompilerLintRenderer' => 'ArcanistLintRenderer', 'ArcanistComposerLinter' => 'ArcanistLinter', diff --git a/src/browse/loader/ArcanistBrowseCommitHardpointLoader.php b/src/browse/loader/ArcanistBrowseCommitHardpointLoader.php new file mode 100644 index 00000000..696cc567 --- /dev/null +++ b/src/browse/loader/ArcanistBrowseCommitHardpointLoader.php @@ -0,0 +1,81 @@ +getQuery(); + + $api = $query->getRepositoryAPI(); + if (!$api) { + return array(); + } + + $repository_ref = $query->getRepositoryRef(); + if (!$repository_ref) { + return array(); + } + $repository_phid = $repository_ref->getPHID(); + + $commit_map = array(); + foreach ($refs as $key => $ref) { + $is_commit = $ref->hasType( + ArcanistBrowseCommitURIHardpointLoader::BROWSETYPE); + + $token = $ref->getToken(); + + if ($token === '.') { + // Git resolves "." like HEAD, but we want to treat it as "browse the + // current directory" instead in all cases. + continue; + } + + if ($token === null) { + if ($is_commit) { + $token = $api->getHeadCommit(); + } else { + continue; + } + } + + try { + $commit = $api->getCanonicalRevisionName($token); + if ($commit) { + $commit_map[$commit][] = $key; + } + } catch (Exception $ex) { + // Ignore anything we can't resolve. + } + } + + if (!$commit_map) { + return array(); + } + + $results = array(); + foreach ($commit_map as $commit_identifier => $ref_keys) { + foreach ($ref_keys as $key) { + $commit_ref = id(new ArcanistCommitRef()) + ->setCommitHash($commit_identifier); + $results[$key][] = $commit_ref; + } + } + + return $results; + } + +} diff --git a/src/browse/loader/ArcanistBrowseCommitURIHardpointLoader.php b/src/browse/loader/ArcanistBrowseCommitURIHardpointLoader.php index af783e5b..1ea8f92c 100644 --- a/src/browse/loader/ArcanistBrowseCommitURIHardpointLoader.php +++ b/src/browse/loader/ArcanistBrowseCommitURIHardpointLoader.php @@ -34,69 +34,55 @@ final class ArcanistBrowseCommitURIHardpointLoader } public function loadHardpoints(array $refs, $hardpoint) { - $api = $this->getQuery()->getRepositoryAPI(); + $query = $this->getQuery(); + + $api = $query->getRepositoryAPI(); if (!$api) { return array(); } - $repository_ref = $this->getQuery()->getRepositoryRef(); + $repository_ref = $query->getRepositoryRef(); if (!$repository_ref) { return array(); } - $repository_phid = $repository_ref->getPHID(); - $refs = $this->getRefsWithSupportedTypes($refs); - $commit_map = array(); - foreach ($refs as $key => $ref) { - $is_commit = $ref->hasType('commit'); - - $token = $ref->getToken(); - - if ($token === '.') { - // Git resolves "." like HEAD, but we want to treat it as "browse the - // current directory" instead in all cases. - continue; - } - - if ($token === null) { - if ($is_commit) { - $token = $api->getHeadCommit(); - } else { - continue; - } - } - - try { - $commit = $api->getCanonicalRevisionName($token); - if ($commit) { - $commit_map[$commit][] = $key; - } - } catch (Exception $ex) { - // Ignore anything we can't resolve. - } - } - - if (!$commit_map) { + if (!$refs) { return array(); } - $commit_info = $this->resolveCall( - 'diffusion.querycommits', - array( - 'repositoryPHID' => $repository_phid, - 'names' => array_keys($commit_map), - )); + $this->newQuery($refs) + ->needHardpoints( + array( + 'commitRefs', + )) + ->execute(); + + $commit_refs = array(); + foreach ($refs as $key => $ref) { + foreach ($ref->getCommitRefs() as $commit_ref) { + $commit_refs[] = $commit_ref; + } + } + + $this->newQuery($commit_refs) + ->needHardpoints( + array( + 'upstream', + )) + ->execute(); $results = array(); - foreach ($commit_info['identifierMap'] as $commit_key => $commit_phid) { - foreach ($commit_map[$commit_key] as $key) { - $commit_uri = $commit_info['data'][$commit_phid]['uri']; - - $results[$key][] = id(new ArcanistBrowseURIRef()) - ->setURI($commit_uri) - ->setType('commit'); + foreach ($refs as $key => $ref) { + $commit_refs = $ref->getCommitRefs(); + foreach ($commit_refs as $commit_ref) { + $uri = $commit_ref->getURI(); + if ($uri !== null) { + $results[$key][] = id(new ArcanistBrowseURIRef()) + ->setURI() + ->setType(self::BROWSETYPE); + } } } diff --git a/src/browse/ref/ArcanistBrowseRef.php b/src/browse/ref/ArcanistBrowseRef.php index db176154..f07c4b2c 100644 --- a/src/browse/ref/ArcanistBrowseRef.php +++ b/src/browse/ref/ArcanistBrowseRef.php @@ -13,6 +13,10 @@ final class ArcanistBrowseRef public function defineHardpoints() { return array( + 'commitRefs' => array( + 'type' => 'ArcanistCommitRef', + 'vector' => true, + ), 'uris' => array( 'type' => 'ArcanistBrowseURIRef', 'vector' => true, @@ -61,4 +65,8 @@ final class ArcanistBrowseRef return $this->getHardpoint('uris'); } + public function getCommitRefs() { + return $this->getHardpoint('commitRefs'); + } + } diff --git a/src/loader/ArcanistCommitUpstreamHardpointLoader.php b/src/loader/ArcanistCommitUpstreamHardpointLoader.php new file mode 100644 index 00000000..4f7c251e --- /dev/null +++ b/src/loader/ArcanistCommitUpstreamHardpointLoader.php @@ -0,0 +1,59 @@ +getQuery(); + + $repository_ref = $query->getRepositoryRef(); + if (!$repository_ref) { + return array_fill_keys(array_keys($refs), null); + } + $repository_phid = $repository_ref->getPHID(); + + $commit_map = array(); + foreach ($refs as $key => $ref) { + $hash = $ref->getCommitHash(); + $commit_map[$hash][] = $key; + } + + $commit_info = $this->resolveCall( + 'diffusion.querycommits', + array( + 'repositoryPHID' => $repository_phid, + 'names' => array_keys($commit_map), + )); + + $results = array(); + foreach ($commit_map as $hash => $keys) { + $commit_phid = idx($commit_info['identifierMap'], $hash); + if ($commit_phid) { + $commit_data = idx($commit_info['data'], $commit_phid); + } else { + $commit_data = null; + } + + foreach ($keys as $key) { + $results[$key] = $commit_data; + } + } + + return $results; + } + +} diff --git a/src/loader/ArcanistHardpointLoader.php b/src/loader/ArcanistHardpointLoader.php index f74e45a8..467fdebf 100644 --- a/src/loader/ArcanistHardpointLoader.php +++ b/src/loader/ArcanistHardpointLoader.php @@ -25,10 +25,28 @@ abstract class ArcanistHardpointLoader } final protected function newQuery(array $refs) { - return id(new ArcanistRefQuery()) - ->setRepositoryAPI($this->getQuery()->getRepositoryAPI()) + $result = id(new ArcanistRefQuery()) ->setConduitEngine($this->getQuery()->getConduitEngine()) ->setRefs($refs); + + $query = $this->getQuery(); + + $repository_api = $query->getRepositoryAPI(); + if ($repository_api) { + $result->setRepositoryAPI($repository_api); + } + + $repository_ref = $query->getRepositoryRef(); + if ($repository_ref) { + $result->setRepositoryRef($repository_ref); + } + + $working_ref = $query->getWorkingCopyRef(); + if ($working_ref) { + $result->setWorkingCopyRef($working_ref); + } + + return $result; } final public function getLoaderKey() { diff --git a/src/ref/ArcanistCommitRef.php b/src/ref/ArcanistCommitRef.php index 38a3b3c7..8058c74a 100644 --- a/src/ref/ArcanistCommitRef.php +++ b/src/ref/ArcanistCommitRef.php @@ -7,6 +7,7 @@ final class ArcanistCommitRef private $treeHash; private $commitEpoch; private $authorEpoch; + private $upstream; public function getRefIdentifier() { return pht('Commit %s', $this->getCommitHash()); @@ -17,6 +18,9 @@ final class ArcanistCommitRef 'message' => array( 'type' => 'string', ), + 'upstream' => array( + 'type' => 'wild', + ), ); } @@ -73,4 +77,18 @@ final class ArcanistCommitRef return $this->getHardpoint('message'); } + public function getURI() { + return $this->getUpstreamProperty('uri'); + } + + private function getUpstreamProperty($key, $default = null) { + $upstream = $this->getHardpoint('upstream'); + + if (!$upstream) { + return $default; + } + + return idx($upstream, $key, $default); + } + }