diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index 8a7d7d7d7c..fe2f539f09 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -7,6 +7,12 @@ final class DiffusionCommitController extends DiffusionController { private $auditAuthorityPHIDs; private $highlightedAudits; + private $commitParents; + private $commitRefs; + private $commitMerges; + private $commitErrors; + private $commitExists; + public function shouldAllowPublic() { return true; } @@ -17,6 +23,7 @@ final class DiffusionCommitController extends DiffusionController { protected function processDiffusionRequest(AphrontRequest $request) { $user = $request->getUser(); + // This controller doesn't use blob/path stuff, just pass the dictionary // in directly instead of using the AphrontRequest parsing mechanism. $data = $request->getURIMap(); @@ -45,10 +52,7 @@ final class DiffusionCommitController extends DiffusionController { )); if (!$commit) { - $exists = $this->callConduitWithDiffusionRequest( - 'diffusion.existsquery', - array('commit' => $drequest->getCommit())); - if (!$exists) { + if (!$this->getCommitExists()) { return new Aphront404Response(); } @@ -95,18 +99,6 @@ final class DiffusionCommitController extends DiffusionController { require_celerity_resource('phabricator-remarkup-css'); - $parents = $this->callConduitWithDiffusionRequest( - 'diffusion.commitparentsquery', - array('commit' => $drequest->getCommit())); - - if ($parents) { - $parents = id(new DiffusionCommitQuery()) - ->setViewer($user) - ->withRepository($repository) - ->withIdentifiers($parents) - ->execute(); - } - $headsup_view = id(new PHUIHeaderView()) ->setHeader(nonempty($commit->getSummary(), pht('Commit Detail'))); @@ -115,7 +107,6 @@ final class DiffusionCommitController extends DiffusionController { $commit_properties = $this->loadCommitProperties( $commit, $commit_data, - $parents, $audit_requests); $property_list = id(new PHUIPropertyListView()) ->setHasKeyboardShortcuts(true) @@ -148,8 +139,10 @@ final class DiffusionCommitController extends DiffusionController { $message)); $headsup_view->setTall(true); + $object_box = id(new PHUIObjectBoxView()) ->setHeader($headsup_view) + ->setFormErrors($this->getCommitErrors()) ->addPropertyList($property_list) ->addPropertyList($detail_list); @@ -216,6 +209,10 @@ final class DiffusionCommitController extends DiffusionController { 'This commit is enormous, and affects more than %d files. '. 'Changes are not shown.', $hard_limit)); + } else if (!$this->getCommitExists()) { + $content[] = $this->renderStatusMessage( + pht('Commit No Longer Exists'), + pht('This commit no longer exists in the repository.')); } else { $show_changesets = true; @@ -382,10 +379,8 @@ final class DiffusionCommitController extends DiffusionController { private function loadCommitProperties( PhabricatorRepositoryCommit $commit, PhabricatorRepositoryCommitData $data, - array $parents, array $audit_requests) { - assert_instances_of($parents, 'PhabricatorRepositoryCommit'); $viewer = $this->getRequest()->getUser(); $commit_phid = $commit->getPHID(); $drequest = $this->getDiffusionRequest(); @@ -423,11 +418,6 @@ final class DiffusionCommitController extends DiffusionController { if ($data->getCommitDetail('committerPHID')) { $phids[] = $data->getCommitDetail('committerPHID'); } - if ($parents) { - foreach ($parents as $parent) { - $phids[] = $parent->getPHID(); - } - } // NOTE: We should never normally have more than a single push log, but // it can occur naturally if a commit is pushed, then the branch it was @@ -564,39 +554,47 @@ final class DiffusionCommitController extends DiffusionController { $props['Differential Revision'] = $handles[$revision_phid]->renderLink(); } + $parents = $this->getCommitParents(); if ($parents) { - $parent_links = array(); - foreach ($parents as $parent) { - $parent_links[] = $handles[$parent->getPHID()]->renderLink(); - } - $props['Parents'] = phutil_implode_html(" \xC2\xB7 ", $parent_links); + $props['Parents'] = $viewer->renderHandleList(mpull($parents, 'getPHID')); } - $props['Branches'] = phutil_tag( - 'span', - array( - 'id' => 'commit-branches', - ), - pht('Unknown')); - $props['Tags'] = phutil_tag( - 'span', - array( - 'id' => 'commit-tags', - ), - pht('Unknown')); + if ($this->getCommitExists()) { + $props['Branches'] = phutil_tag( + 'span', + array( + 'id' => 'commit-branches', + ), + pht('Unknown')); + $props['Tags'] = phutil_tag( + 'span', + array( + 'id' => 'commit-tags', + ), + pht('Unknown')); - $callsign = $repository->getCallsign(); - $root = '/diffusion/'.$callsign.'/commit/'.$commit->getCommitIdentifier(); - Javelin::initBehavior( - 'diffusion-commit-branches', - array( - $root.'/branches/' => 'commit-branches', - $root.'/tags/' => 'commit-tags', - )); + $callsign = $repository->getCallsign(); + $root = '/diffusion/'.$callsign.'/commit/'.$commit->getCommitIdentifier(); + Javelin::initBehavior( + 'diffusion-commit-branches', + array( + $root.'/branches/' => 'commit-branches', + $root.'/tags/' => 'commit-tags', + )); + } - $refs = $this->buildRefs($drequest); + $refs = $this->getCommitRefs(); if ($refs) { - $props['References'] = $refs; + $ref_links = array(); + foreach ($refs as $ref_data) { + $ref_links[] = phutil_tag( + 'a', + array( + 'href' => $ref_data['href'], + ), + $ref_data['ref']); + } + $props['References'] = phutil_implode_html(', ', $ref_links); } if ($reverts_phids) { @@ -860,26 +858,12 @@ final class DiffusionCommitController extends DiffusionController { $drequest = $this->getDiffusionRequest(); $repository = $drequest->getRepository(); - $vcs = $repository->getVersionControlSystem(); - switch ($vcs) { - case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: - // These aren't supported under SVN. - return null; - } - - $limit = 50; - - $merges = $this->callConduitWithDiffusionRequest( - 'diffusion.mergedcommitsquery', - array( - 'commit' => $drequest->getCommit(), - 'limit' => $limit + 1, - )); - + $merges = $this->getCommitMerges(); if (!$merges) { return null; } - $merges = DiffusionPathChange::newFromConduit($merges); + + $limit = $this->getMergeDisplayLimit(); $caption = null; if (count($merges) > $limit) { @@ -961,27 +945,6 @@ final class DiffusionCommitController extends DiffusionController { return $actions; } - private function buildRefs(DiffusionRequest $request) { - // this is git-only, so save a conduit round trip and just get out of - // here if the repository isn't git - $type_git = PhabricatorRepositoryType::REPOSITORY_TYPE_GIT; - $repository = $request->getRepository(); - if ($repository->getVersionControlSystem() != $type_git) { - return null; - } - - $results = $this->callConduitWithDiffusionRequest( - 'diffusion.refsquery', - array('commit' => $request->getCommit())); - $ref_links = array(); - foreach ($results as $ref_data) { - $ref_links[] = phutil_tag('a', - array('href' => $ref_data['href']), - $ref_data['ref']); - } - return phutil_implode_html(', ', $ref_links); - } - private function buildRawDiffResponse(DiffusionRequest $drequest) { $raw_diff = $this->callConduitWithDiffusionRequest( 'diffusion.rawdiffquery', @@ -1137,4 +1100,140 @@ final class DiffusionCommitController extends DiffusionController { return $toc_view; } + private function loadCommitState() { + $viewer = $this->getViewer(); + $drequest = $this->getDiffusionRequest(); + $repository = $drequest->getRepository(); + $commit = $drequest->getCommit(); + + // TODO: We could use futures here and resolve these calls in parallel. + + $exceptions = array(); + + try { + $parent_refs = $this->callConduitWithDiffusionRequest( + 'diffusion.commitparentsquery', + array( + 'commit' => $commit, + )); + + if ($parent_refs) { + $parents = id(new DiffusionCommitQuery()) + ->setViewer($viewer) + ->withRepository($repository) + ->withIdentifiers($parent_refs) + ->execute(); + } else { + $parents = array(); + } + + $this->commitParents = $parents; + } catch (Exception $ex) { + $this->commitParents = false; + $exceptions[] = $ex; + } + + $merge_limit = $this->getMergeDisplayLimit(); + + try { + $merges = $this->callConduitWithDiffusionRequest( + 'diffusion.mergedcommitsquery', + array( + 'commit' => $commit, + 'limit' => $merge_limit + 1, + )); + + $this->commitMerges = DiffusionPathChange::newFromConduit($merges); + } catch (Exception $ex) { + $this->commitMerges = false; + $exceptions[] = $ex; + } + + + try { + if ($repository->isGit()) { + $refs = $this->callConduitWithDiffusionRequest( + 'diffusion.refsquery', + array( + 'commit' => $commit, + )); + } else { + $refs = array(); + } + + $this->commitRefs = $refs; + } catch (Exception $ex) { + $this->commitRefs = false; + $exceptions[] = $ex; + } + + if ($exceptions) { + $exists = $this->callConduitWithDiffusionRequest( + 'diffusion.existsquery', + array( + 'commit' => $commit, + )); + + if ($exists) { + $this->commitExists = true; + foreach ($exceptions as $exception) { + $this->commitErrors[] = $exception->getMessage(); + } + } else { + $this->commitExists = false; + $this->commitErrors[] = pht( + 'This commit no longer exists in the repository. It may have '. + 'been part of a branch which was deleted.'); + } + } else { + $this->commitExists = true; + $this->commitErrors = array(); + } + } + + private function getMergeDisplayLimit() { + return 50; + } + + private function getCommitExists() { + if ($this->commitExists === null) { + $this->loadCommitState(); + } + + return $this->commitExists; + } + + private function getCommitParents() { + if ($this->commitParents === null) { + $this->loadCommitState(); + } + + return $this->commitParents; + } + + private function getCommitRefs() { + if ($this->commitRefs === null) { + $this->loadCommitState(); + } + + return $this->commitRefs; + } + + private function getCommitMerges() { + if ($this->commitMerges === null) { + $this->loadCommitState(); + } + + return $this->commitMerges; + } + + private function getCommitErrors() { + if ($this->commitErrors === null) { + $this->loadCommitState(); + } + + return $this->commitErrors; + } + + }