Remove buildable handle / container handle logic form Harbormaster buildable queries

Summary:
Ref T10457. We currently have these weird, out-of-place methods on Harbormaster queries that just load handles. These were written before HandlePool, and HandlePool is now more convenient, simpler, and more efficient.

Drop this stuff in favor of using handle pools off `$viewer`.

Test Plan: Looked at buildable list, looked at buildable detail, grepped for removed methods.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10457

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

src/applications/harbormaster/controller/HarbormasterBuildableViewController.php

@@ -9,8 +9,6 @@ final class HarbormasterBuildableViewController
     $buildable = id(new HarbormasterBuildableQuery())
       ->setViewer($viewer)
       ->withIDs(array($request->getURIData('id')))
-      ->needBuildableHandles(true)
-      ->needContainerHandles(true)
       ->executeOne();
     if (!$buildable) {
       return new Aphront404Response();
@@ -167,15 +165,18 @@ final class HarbormasterBuildableViewController
       ->setActionList($actions);
     $box->addPropertyList($properties);
 
-    if ($buildable->getContainerHandle() !== null) {
+    $container_phid = $buildable->getContainerPHID();
+    $buildable_phid = $buildable->getBuildablePHID();
+
+    if ($container_phid) {
       $properties->addProperty(
         pht('Container'),
-        $buildable->getContainerHandle()->renderLink());
+        $viewer->renderHandle($container_phid));
     }
 
     $properties->addProperty(
       pht('Buildable'),
-      $buildable->getBuildableHandle()->renderLink());
+      $viewer->renderHandle($buildable_phid));
 
     $properties->addProperty(
       pht('Origin'),

src/applications/harbormaster/interface/HarbormasterBuildableInterface.php

@@ -6,7 +6,6 @@ interface HarbormasterBuildableInterface {
   public function getHarbormasterContainerPHID();
 
   public function getBuildVariables();
-
   public function getAvailableBuildVariables();
 
 }

src/applications/harbormaster/phid/HarbormasterBuildablePHIDType.php

@@ -21,8 +21,7 @@ final class HarbormasterBuildablePHIDType extends PhabricatorPHIDType {
     array $phids) {
 
     return id(new HarbormasterBuildableQuery())
-      ->withPHIDs($phids)
+      ->withPHIDs($phids);
-      ->needBuildableHandles(true);
   }
 
   public function loadHandles(
@@ -30,15 +29,30 @@ final class HarbormasterBuildablePHIDType extends PhabricatorPHIDType {
     array $handles,
     array $objects) {
 
+    $viewer = $this->getViewer();
+
+    $target_phids = array();
+    foreach ($objects as $phid => $object) {
+      $target_phids[] = $object->getBuildablePHID();
+    }
+    $target_handles = $viewer->loadHandles($target_phids);
+
     foreach ($handles as $phid => $handle) {
       $buildable = $objects[$phid];
 
       $id = $buildable->getID();
-      $target = $buildable->getBuildableHandle()->getFullName();
+      $buildable_phid = $buildable->getBuildablePHID();
 
-      $handle->setURI("/B{$id}");
+      $target = $target_handles[$buildable_phid];
-      $handle->setName("B{$id}");
+      $target_name = $target->getFullName();
-      $handle->setFullName("B{$id}: ".$target);
+
+      $uri = $buildable->getURI();
+      $monogram = $buildable->getMonogram();
+
+      $handle
+        ->setURI($uri)
+        ->setName($monogram)
+        ->setFullName("{$monogram}: {$target_name}");
     }
   }
 

src/applications/harbormaster/query/HarbormasterBuildableQuery.php

@@ -10,8 +10,6 @@ final class HarbormasterBuildableQuery
   private $manualBuildables;
 
   private $needContainerObjects;
-  private $needContainerHandles;
-  private $needBuildableHandles;
   private $needBuilds;
   private $needTargets;
 
@@ -45,16 +43,6 @@ final class HarbormasterBuildableQuery
     return $this;
   }
 
-  public function needContainerHandles($need) {
-    $this->needContainerHandles = $need;
-    return $this;
-  }
-
-  public function needBuildableHandles($need) {
-    $this->needBuildableHandles = $need;
-    return $this;
-  }
-
   public function needBuilds($need) {
     $this->needBuilds = $need;
     return $this;
@@ -99,60 +87,23 @@ final class HarbormasterBuildableQuery
   }
 
   protected function didFilterPage(array $page) {
-    if ($this->needContainerObjects || $this->needContainerHandles) {
+    if ($this->needContainerObjects) {
       $container_phids = array_filter(mpull($page, 'getContainerPHID'));
 
-      if ($this->needContainerObjects) {
+      if ($container_phids) {
-        $containers = array();
+        $containers = id(new PhabricatorObjectQuery())
-
-        if ($container_phids) {
-          $containers = id(new PhabricatorObjectQuery())
-            ->setViewer($this->getViewer())
-            ->withPHIDs($container_phids)
-            ->setParentQuery($this)
-            ->execute();
-          $containers = mpull($containers, null, 'getPHID');
-        }
-
-        foreach ($page as $key => $buildable) {
-          $container_phid = $buildable->getContainerPHID();
-          $buildable->attachContainerObject(idx($containers, $container_phid));
-        }
-      }
-
-      if ($this->needContainerHandles) {
-        $handles = array();
-
-        if ($container_phids) {
-          $handles = id(new PhabricatorHandleQuery())
-            ->setViewer($this->getViewer())
-            ->withPHIDs($container_phids)
-            ->setParentQuery($this)
-            ->execute();
-        }
-
-        foreach ($page as $key => $buildable) {
-          $container_phid = $buildable->getContainerPHID();
-          $buildable->attachContainerHandle(idx($handles, $container_phid));
-        }
-      }
-    }
-
-    if ($this->needBuildableHandles) {
-      $handles = array();
-
-      $handle_phids = array_filter(mpull($page, 'getBuildablePHID'));
-      if ($handle_phids) {
-        $handles = id(new PhabricatorHandleQuery())
           ->setViewer($this->getViewer())
-          ->withPHIDs($handle_phids)
+          ->withPHIDs($container_phids)
           ->setParentQuery($this)
           ->execute();
+        $containers = mpull($containers, null, 'getPHID');
+      } else {
+        $containers = array();
       }
 
       foreach ($page as $key => $buildable) {
-        $handle_phid = $buildable->getBuildablePHID();
+        $container_phid = $buildable->getContainerPHID();
-        $buildable->attachBuildableHandle(idx($handles, $handle_phid));
+        $buildable->attachContainerObject(idx($containers, $container_phid));
       }
     }
 

src/applications/harbormaster/query/HarbormasterBuildableSearchEngine.php

@@ -58,9 +58,7 @@ final class HarbormasterBuildableSearchEngine
   }
 
   public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) {
-    $query = id(new HarbormasterBuildableQuery())
+    $query = id(new HarbormasterBuildableQuery());
-      ->needContainerHandles(true)
-      ->needBuildableHandles(true);
 
     $container_phids = $saved->getParameter('containerPHIDs', array());
     if ($container_phids) {
@@ -185,23 +183,36 @@ final class HarbormasterBuildableSearchEngine
 
     $viewer = $this->requireViewer();
 
+    $phids = array();
+    foreach ($buildables as $buildable) {
+      $phids[] = $buildable->getContainerPHID();
+      $phids[] = $buildable->getBuildablePHID();
+    }
+    $handles = $viewer->loadHandles($phids);
+
+
     $list = new PHUIObjectItemListView();
     foreach ($buildables as $buildable) {
       $id = $buildable->getID();
 
+      $container_phid = $buildable->getContainerPHID();
+      $buildable_phid = $buildable->getBuildablePHID();
+
       $item = id(new PHUIObjectItemView())
         ->setHeader(pht('Buildable %d', $buildable->getID()));
-      if ($buildable->getContainerHandle() !== null) {
+
-        $item->addAttribute($buildable->getContainerHandle()->getName());
+      if ($container_phid) {
-      }
+        $handle = $handles[$container_phid];
-      if ($buildable->getBuildableHandle() !== null) {
+        $item->addAttribute($handle->getName());
-        $item->addAttribute($buildable->getBuildableHandle()->getFullName());
       }
 
-      if ($id) {
+      if ($buildable_phid) {
-        $item->setHref("/B{$id}");
+        $handle = $handles[$buildable_phid];
+        $item->addAttribute($handle->getFullName());
       }
 
+      $item->setHref($buildable->getURI());
+
       if ($buildable->getIsManualBuildable()) {
         $item->addIcon('fa-wrench grey', pht('Manual'));
       }

src/applications/harbormaster/storage/HarbormasterBuildable.php

@@ -13,8 +13,6 @@ final class HarbormasterBuildable extends HarbormasterDAO
 
   private $buildableObject = self::ATTACHABLE;
   private $containerObject = self::ATTACHABLE;
-  private $buildableHandle = self::ATTACHABLE;
-  private $containerHandle = self::ATTACHABLE;
   private $builds = self::ATTACHABLE;
 
   const STATUS_BUILDING = 'building';
@@ -70,6 +68,10 @@ final class HarbormasterBuildable extends HarbormasterDAO
     return 'B'.$this->getID();
   }
 
+  public function getURI() {
+    return '/'.$this->getMonogram();
+  }
+
   /**
    * Returns an existing buildable for the object's PHID or creates a
    * new buildable implicitly if needed.
@@ -237,24 +239,6 @@ final class HarbormasterBuildable extends HarbormasterDAO
     return $this->assertAttached($this->containerObject);
   }
 
-  public function attachContainerHandle($container_handle) {
-    $this->containerHandle = $container_handle;
-    return $this;
-  }
-
-  public function getContainerHandle() {
-    return $this->assertAttached($this->containerHandle);
-  }
-
-  public function attachBuildableHandle($buildable_handle) {
-    $this->buildableHandle = $buildable_handle;
-    return $this;
-  }
-
-  public function getBuildableHandle() {
-    return $this->assertAttached($this->buildableHandle);
-  }
-
   public function attachBuilds(array $builds) {
     assert_instances_of($builds, 'HarbormasterBuild');
     $this->builds = $builds;