diff --git a/src/applications/base/controller/PhabricatorController.php b/src/applications/base/controller/PhabricatorController.php --- a/src/applications/base/controller/PhabricatorController.php +++ b/src/applications/base/controller/PhabricatorController.php @@ -440,10 +440,6 @@ return $this; } - protected function getLoadedHandles() { - return $this->handles; - } - protected function loadViewerHandles(array $phids) { return id(new PhabricatorHandleQuery()) ->setViewer($this->getRequest()->getUser()) @@ -471,7 +467,7 @@ } return implode_selected_handle_links($style_map[$style], - $this->getLoadedHandles(), + $this->handles, array_filter($phids)); } diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php --- a/src/applications/herald/adapter/HeraldAdapter.php +++ b/src/applications/herald/adapter/HeraldAdapter.php @@ -1074,8 +1074,9 @@ return $map; } - public function renderRuleAsText(HeraldRule $rule, array $handles) { - assert_instances_of($handles, 'PhabricatorObjectHandle'); + public function renderRuleAsText( + HeraldRule $rule, + PhabricatorHandleList $handles) { require_celerity_resource('herald-css'); @@ -1150,7 +1151,7 @@ private function renderConditionAsText( HeraldCondition $condition, - array $handles) { + PhabricatorHandleList $handles) { $field_type = $condition->getFieldName(); @@ -1170,7 +1171,7 @@ private function renderActionAsText( HeraldAction $action, - array $handles) { + PhabricatorHandleList $handles) { $rule_global = HeraldRuleTypeConfig::RULE_TYPE_GLOBAL; $action_type = $action->getAction(); @@ -1183,7 +1184,7 @@ private function renderConditionValueAsText( HeraldCondition $condition, - array $handles) { + PhabricatorHandleList $handles) { $value = $condition->getValue(); if (!is_array($value)) { @@ -1220,7 +1221,7 @@ break; default: foreach ($value as $index => $val) { - $handle = idx($handles, $val); + $handle = $handles->getHandleIfExists($val); if ($handle) { $value[$index] = $handle->renderLink(); } @@ -1233,7 +1234,7 @@ private function renderActionTargetAsText( HeraldAction $action, - array $handles) { + PhabricatorHandleList $handles) { $target = $action->getTarget(); if (!is_array($target)) { @@ -1245,7 +1246,7 @@ $target[$index] = PhabricatorFlagColor::getColorName($val); break; default: - $handle = idx($handles, $val); + $handle = $handles->getHandleIfExists($val); if ($handle) { $target[$index] = $handle->renderLink(); } diff --git a/src/applications/herald/controller/HeraldRuleViewController.php b/src/applications/herald/controller/HeraldRuleViewController.php --- a/src/applications/herald/controller/HeraldRuleViewController.php +++ b/src/applications/herald/controller/HeraldRuleViewController.php @@ -115,7 +115,7 @@ $viewer = $this->getRequest()->getUser(); - $this->loadHandles(HeraldAdapter::getHandlePHIDs($rule)); + $handles = $viewer->loadHandles(HeraldAdapter::getHandlePHIDs($rule)); $view = id(new PHUIPropertyListView()) ->setUser($viewer) @@ -152,8 +152,8 @@ $view->addSectionHeader( pht('Rule Description'), PHUIPropertyListView::ICON_SUMMARY); - $view->addTextContent( - $adapter->renderRuleAsText($rule, $this->getLoadedHandles())); + + $view->addTextContent($adapter->renderRuleAsText($rule, $handles)); } return $view; diff --git a/src/applications/maniphest/controller/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/ManiphestTaskDetailController.php --- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php +++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php @@ -83,11 +83,12 @@ } $phids = array_keys($phids); + $handles = $user->loadHandles($phids); + // TODO: This is double-loading because we have a separate call to + // renderHandlesForPHIDs(). Clean this up in the next pass. $this->loadHandles($phids); - $handles = $this->getLoadedHandles(); - $info_view = null; if ($parent_task) { $info_view = new PHUIInfoView(); @@ -100,7 +101,7 @@ $info_view->appendChild(hsprintf( 'Created a subtask of %s', - $this->getHandle($parent_task->getPHID())->renderLink())); + $handles[$parent_task->getPHID()]->renderLink())); } else if ($workflow == 'create') { $info_view = new PHUIInfoView(); $info_view->setSeverity(PHUIInfoView::SEVERITY_NOTICE); @@ -327,7 +328,7 @@ $header = $this->buildHeaderView($task); $properties = $this->buildPropertyView( - $task, $field_list, $edges, $actions); + $task, $field_list, $edges, $actions, $handles); $description = $this->buildDescriptionView($task, $engine); if (!$user->isLoggedIn()) { @@ -443,7 +444,8 @@ ManiphestTask $task, PhabricatorCustomFieldList $field_list, array $edges, - PhabricatorActionListView $actions) { + PhabricatorActionListView $actions, + $handles) { $viewer = $this->getRequest()->getUser(); @@ -455,8 +457,8 @@ $view->addProperty( pht('Assigned To'), $task->getOwnerPHID() - ? $this->getHandle($task->getOwnerPHID())->renderLink() - : phutil_tag('em', array(), pht('None'))); + ? $handles[$task->getOwnerPHID()]->renderLink() + : phutil_tag('em', array(), pht('None'))); $view->addProperty( pht('Priority'), @@ -464,7 +466,7 @@ $view->addProperty( pht('Author'), - $this->getHandle($task->getAuthorPHID())->renderLink()); + $handles[$task->getAuthorPHID()]->renderLink()); $source = $task->getOriginalEmailSource(); if ($source) { @@ -491,7 +493,6 @@ ); $revisions_commits = array(); - $handles = $this->getLoadedHandles(); $commit_phids = array_keys( $edges[ManiphestTaskHasCommitEdgeType::EDGECONST]); @@ -505,7 +506,7 @@ foreach ($commit_phids as $phid) { $revisions_commits[$phid] = $handles[$phid]->renderLink(); $revision_phid = key($drev_edges[$phid][$commit_drev]); - $revision_handle = idx($handles, $revision_phid); + $revision_handle = $handles->getHandleIfExists($revision_phid); if ($revision_handle) { $task_drev = ManiphestTaskHasRevisionEdgeType::EDGECONST; unset($edges[$task_drev][$revision_phid]); diff --git a/src/applications/phid/handle/pool/PhabricatorHandleList.php b/src/applications/phid/handle/pool/PhabricatorHandleList.php --- a/src/applications/phid/handle/pool/PhabricatorHandleList.php +++ b/src/applications/phid/handle/pool/PhabricatorHandleList.php @@ -57,6 +57,20 @@ } + /** + * Get a handle from this list if it exists. + * + * This has similar semantics to @{function:idx}. + */ + public function getHandleIfExists($phid, $default = null) { + if ($this->handles === null) { + $this->loadHandles(); + } + + return idx($this->handles, $phid, $default); + } + + /* -( Iterator )----------------------------------------------------------- */