Remove Controller->getLoadedHandles()

Summary: Ref T7689. Removes this part of the `Controller->loadHandles()` + `Controller->getLoadedHandles()` mechanism.

Test Plan:
  - Viewed Herald transcripts.
  - Viewed Maniphest tasks with attached revisions and commits.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7689

Differential Revision: https://secure.phabricator.com/D12204

src/applications/base/controller/PhabricatorController.php

@@ -440,10 +440,6 @@ abstract class PhabricatorController extends AphrontController {
     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 @@ abstract class PhabricatorController extends AphrontController {
     }
 
     return implode_selected_handle_links($style_map[$style],
-      $this->getLoadedHandles(),
+      $this->handles,
       array_filter($phids));
   }
 

src/applications/herald/adapter/HeraldAdapter.php

@@ -1074,8 +1074,9 @@ abstract class HeraldAdapter {
     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 @@ abstract class HeraldAdapter {
 
   private function renderConditionAsText(
     HeraldCondition $condition,
-    array $handles) {
+    PhabricatorHandleList $handles) {
 
     $field_type = $condition->getFieldName();
 
@@ -1170,7 +1171,7 @@ abstract class HeraldAdapter {
 
   private function renderActionAsText(
     HeraldAction $action,
-    array $handles) {
+    PhabricatorHandleList $handles) {
     $rule_global = HeraldRuleTypeConfig::RULE_TYPE_GLOBAL;
 
     $action_type = $action->getAction();
@@ -1183,7 +1184,7 @@ abstract class HeraldAdapter {
 
   private function renderConditionValueAsText(
     HeraldCondition $condition,
-    array $handles) {
+    PhabricatorHandleList $handles) {
 
     $value = $condition->getValue();
     if (!is_array($value)) {
@@ -1220,7 +1221,7 @@ abstract class HeraldAdapter {
         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 @@ abstract class HeraldAdapter {
 
   private function renderActionTargetAsText(
     HeraldAction $action,
-    array $handles) {
+    PhabricatorHandleList $handles) {
 
     $target = $action->getTarget();
     if (!is_array($target)) {
@@ -1245,7 +1246,7 @@ abstract class HeraldAdapter {
           $target[$index] = PhabricatorFlagColor::getColorName($val);
           break;
         default:
-          $handle = idx($handles, $val);
+          $handle = $handles->getHandleIfExists($val);
           if ($handle) {
             $target[$index] = $handle->renderLink();
           }

src/applications/herald/controller/HeraldRuleViewController.php

@@ -115,7 +115,7 @@ final class HeraldRuleViewController extends HeraldController {
 
     $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 @@ final class HeraldRuleViewController extends HeraldController {
       $view->addSectionHeader(
         pht('Rule Description'),
         PHUIPropertyListView::ICON_SUMMARY);
-      $view->addTextContent(
-        $adapter->renderRuleAsText($rule, $this->getLoadedHandles()));
+
+      $view->addTextContent($adapter->renderRuleAsText($rule, $handles));
     }
 
     return $view;

src/applications/maniphest/controller/ManiphestTaskDetailController.php

@@ -83,11 +83,12 @@ final class ManiphestTaskDetailController extends ManiphestController {
     }
 
     $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 @@ final class ManiphestTaskDetailController extends ManiphestController {
 
       $info_view->appendChild(hsprintf(
         'Created a subtask of <strong>%s</strong>',
-        $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 @@ final class ManiphestTaskDetailController extends ManiphestController {
 
     $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 @@ final class ManiphestTaskDetailController extends ManiphestController {
     ManiphestTask $task,
     PhabricatorCustomFieldList $field_list,
     array $edges,
-    PhabricatorActionListView $actions) {
+    PhabricatorActionListView $actions,
+    $handles) {
 
     $viewer = $this->getRequest()->getUser();
 
@@ -455,8 +457,8 @@ final class ManiphestTaskDetailController extends ManiphestController {
     $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 @@ final class ManiphestTaskDetailController extends ManiphestController {
 
     $view->addProperty(
       pht('Author'),
-      $this->getHandle($task->getAuthorPHID())->renderLink());
+      $handles[$task->getAuthorPHID()]->renderLink());
 
     $source = $task->getOriginalEmailSource();
     if ($source) {
@@ -491,7 +493,6 @@ final class ManiphestTaskDetailController extends ManiphestController {
     );
 
     $revisions_commits = array();
-    $handles = $this->getLoadedHandles();
 
     $commit_phids = array_keys(
       $edges[ManiphestTaskHasCommitEdgeType::EDGECONST]);
@@ -505,7 +506,7 @@ final class ManiphestTaskDetailController extends ManiphestController {
       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]);

src/applications/phid/handle/pool/PhabricatorHandleList.php

@@ -57,6 +57,20 @@ final class PhabricatorHandleList
   }
 
 
+  /**
+   * 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  )----------------------------------------------------------- */