Page MenuHomePhabricator
Files F13243364

D14227.diff
No OneTemporary
Actions

D14227.diff
View Options

diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php
--- a/src/applications/diffusion/controller/DiffusionCommitController.php
+++ b/src/applications/diffusion/controller/DiffusionCommitController.php
@@ -7,6 +7,12 @@
private $auditAuthorityPHIDs;
private $highlightedAudits;
+ private $commitParents;
+ private $commitRefs;
+ private $commitMerges;
+ private $commitErrors;
+ private $commitExists;
+
public function shouldAllowPublic() {
return true;
}
@@ -17,6 +23,7 @@
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 @@
));
if (!$commit) {
- $exists = $this->callConduitWithDiffusionRequest(
- 'diffusion.existsquery',
- array('commit' => $drequest->getCommit()));
- if (!$exists) {
+ if (!$this->getCommitExists()) {
return new Aphront404Response();
}
@@ -95,18 +99,6 @@
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 @@
$commit_properties = $this->loadCommitProperties(
$commit,
$commit_data,
- $parents,
$audit_requests);
$property_list = id(new PHUIPropertyListView())
->setHasKeyboardShortcuts(true)
@@ -148,8 +139,10 @@
$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 @@
'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 @@
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 @@
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 @@
$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 @@
$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 @@
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 @@
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;
+ }
+
+
}

File Metadata

Mime Type
text/plain
Expires
Fri, May 24, 3:49 AM (3 w, 3 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6281236
Default Alt Text
D14227.diff (10 KB)

Event Timeline

Phabricator · Built by Phacility