Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15198466
D14227.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
10 KB
Referenced Files
None
Subscribers
None
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
Details
Attached
Mime Type
text/plain
Expires
Mon, Feb 24, 3:09 AM (14 h, 27 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7188578
Default Alt Text
D14227.diff (10 KB)
Attached To
Mode
D14227: Improve Diffusion behavior for no-longer-existing commits
Attached
Detach File
Event Timeline
Log In to Comment