Make upstream callers respect "active bindings" when querying Almanac

Summary: Ref T13641. Make "active bindings" a real query and make callers that only care about active bindings only query for active bindings.

Test Plan:
  - Queried for "bindings" and "activeBindings" via Conduit.
  - Disabled/enabled devices, saw binding status update in UI.
  - Loaded Diffusion cluster layout.
  - Grepped for `needBindings()`, `getActiveBindings()`, etc.

Subscribers: yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13641

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

src/applications/almanac/constants/AlmanacDeviceStatus.php

@@ -62,6 +62,26 @@ final class AlmanacDeviceStatus
     return $result;
   }
 
+  public static function getActiveStatusList() {
+    $results = array();
+    foreach (self::newDeviceStatusMap() as $status_value => $status) {
+      if (empty($status['disabled'])) {
+        $results[] = $status_value;
+      }
+    }
+    return $results;
+  }
+
+  public static function getDisabledStatusList() {
+    $results = array();
+    foreach (self::newDeviceStatusMap() as $status_value => $status) {
+      if (!empty($status['disabled'])) {
+        $results[] = $status_value;
+      }
+    }
+    return $results;
+  }
+
   private function getDeviceStatusProperty($key, $default = null) {
     $map = self::newDeviceStatusMap();
     $properties = idx($map, $this->getValue(), array());
@@ -81,6 +101,7 @@ final class AlmanacDeviceStatus
         'icon.color' => 'grey',
         'status-tag.icon' => 'fa-times',
         'status-tag.color' => 'indigo',
+        'disabled' => true,
       ),
     );
   }

src/applications/almanac/engineextension/AlmanacBindingsSearchEngineAttachment.php

@@ -3,6 +3,17 @@
 final class AlmanacBindingsSearchEngineAttachment
   extends AlmanacSearchEngineAttachment {
 
+  private $isActive;
+
+  public function setIsActive($is_active) {
+    $this->isActive = $is_active;
+    return $this;
+  }
+
+  public function getIsActive() {
+    return $this->isActive;
+  }
+
   public function getAttachmentName() {
     return pht('Almanac Bindings');
   }
@@ -13,12 +24,24 @@ final class AlmanacBindingsSearchEngineAttachment
 
   public function willLoadAttachmentData($query, $spec) {
     $query->needProperties(true);
-    $query->needBindings(true);
+
+    if ($this->getIsActive()) {
+      $query->needBindings(true);
+    } else {
+      $query->needActiveBindings(true);
+    }
   }
 
   public function getAttachmentForObject($object, $data, $spec) {
     $bindings = array();
-    foreach ($object->getBindings() as $binding) {
+
+    if ($this->getIsActive()) {
+      $service_bindings = $object->getActiveBindings();
+    } else {
+      $service_bindings = $object->getBindings();
+    }
+
+    foreach ($service_bindings as $binding) {
       $bindings[] = $this->getAlmanacBindingDictionary($binding);
     }
 

src/applications/almanac/engineextension/AlmanacSearchEngineAttachment.php

@@ -51,6 +51,8 @@ abstract class AlmanacSearchEngineAttachment
       'phid' => $device->getPHID(),
       'name' => $device->getName(),
       'properties' => $this->getAlmanacPropertyList($device),
+      'status' => $device->getStatus(),
+      'disabled' => $device->isDisabled(),
     );
   }
 

src/applications/almanac/query/AlmanacBindingQuery.php

@@ -8,6 +8,7 @@ final class AlmanacBindingQuery
   private $servicePHIDs;
   private $devicePHIDs;
   private $interfacePHIDs;
+  private $isActive;
 
   public function withIDs(array $ids) {
     $this->ids = $ids;
@@ -34,6 +35,11 @@ final class AlmanacBindingQuery
     return $this;
   }
 
+  public function withIsActive($active) {
+    $this->isActive = $active;
+    return $this;
+  }
+
   public function newResultObject() {
     return new AlmanacBinding();
   }
@@ -95,39 +101,79 @@ final class AlmanacBindingQuery
     if ($this->ids !== null) {
       $where[] = qsprintf(
         $conn,
-        'id IN (%Ld)',
+        'binding.id IN (%Ld)',
         $this->ids);
     }
 
     if ($this->phids !== null) {
       $where[] = qsprintf(
         $conn,
-        'phid IN (%Ls)',
+        'binding.phid IN (%Ls)',
         $this->phids);
     }
 
     if ($this->servicePHIDs !== null) {
       $where[] = qsprintf(
         $conn,
-        'servicePHID IN (%Ls)',
+        'binding.servicePHID IN (%Ls)',
         $this->servicePHIDs);
     }
 
     if ($this->devicePHIDs !== null) {
       $where[] = qsprintf(
         $conn,
-        'devicePHID IN (%Ls)',
+        'binding.devicePHID IN (%Ls)',
         $this->devicePHIDs);
     }
 
     if ($this->interfacePHIDs !== null) {
       $where[] = qsprintf(
         $conn,
-        'interfacePHID IN (%Ls)',
+        'binding.interfacePHID IN (%Ls)',
         $this->interfacePHIDs);
     }
 
+    if ($this->isActive !== null) {
+      if ($this->isActive) {
+        $where[] = qsprintf(
+          $conn,
+          '(binding.isDisabled = 0) AND (device.status IN (%Ls))',
+          AlmanacDeviceStatus::getActiveStatusList());
+      } else {
+        $where[] = qsprintf(
+          $conn,
+          '(binding.isDisabled = 1) OR (device.status IN (%Ls))',
+          AlmanacDeviceStatus::getDisabledStatusList());
+      }
+    }
+
     return $where;
   }
 
+  protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) {
+    $joins = parent::buildJoinClauseParts($conn);
+
+    if ($this->shouldJoinDeviceTable()) {
+      $device_table = new AlmanacDevice();
+      $joins[] = qsprintf(
+        $conn,
+        'JOIN %R device ON binding.devicePHID = device.phid',
+        $device_table);
+    }
+
+    return $joins;
+  }
+
+  private function shouldJoinDeviceTable() {
+    if ($this->isActive !== null) {
+      return true;
+    }
+
+    return false;
+  }
+
+  protected function getPrimaryTableAlias() {
+    return 'binding';
+  }
+
 }

src/applications/almanac/query/AlmanacServiceQuery.php

@@ -12,6 +12,7 @@ final class AlmanacServiceQuery
   private $nameSuffix;
 
   private $needBindings;
+  private $needActiveBindings;
 
   public function withIDs(array $ids) {
     $this->ids = $ids;
@@ -59,6 +60,11 @@ final class AlmanacServiceQuery
     return $this;
   }
 
+  public function needActiveBindings($need_active) {
+    $this->needActiveBindings = $need_active;
+    return $this;
+  }
+
   public function newResultObject() {
     return new AlmanacService();
   }
@@ -160,18 +166,35 @@ final class AlmanacServiceQuery
   }
 
   protected function didFilterPage(array $services) {
-    if ($this->needBindings) {
+    $need_all = $this->needBindings;
+    $need_active = $this->needActiveBindings;
+
+    $need_any = ($need_all || $need_active);
+    $only_active = ($need_active && !$need_all);
+
+    if ($need_any) {
       $service_phids = mpull($services, 'getPHID');
-      $bindings = id(new AlmanacBindingQuery())
+
+      $bindings_query = id(new AlmanacBindingQuery())
         ->setViewer($this->getViewer())
         ->withServicePHIDs($service_phids)
-        ->needProperties($this->getNeedProperties())
-        ->execute();
+        ->needProperties($this->getNeedProperties());
+
+      if ($only_active) {
+        $bindings_query->withIsActive(true);
+      }
+
+      $bindings = $bindings_query->execute();
       $bindings = mgroup($bindings, 'getServicePHID');
 
       foreach ($services as $service) {
         $service_bindings = idx($bindings, $service->getPHID(), array());
-        $service->attachBindings($service_bindings);
+
+        if ($only_active) {
+          $service->attachActiveBindings($service_bindings);
+        } else {
+          $service->attachBindings($service_bindings);
+        }
       }
     }
 

src/applications/almanac/storage/AlmanacDevice.php

@@ -282,6 +282,10 @@ final class AlmanacDevice
         ->setKey('status')
         ->setType('map<string, wild>')
         ->setDescription(pht('Device status information.')),
+      id(new PhabricatorConduitSearchFieldSpecification())
+        ->setKey('disabled')
+        ->setType('bool')
+        ->setDescription(pht('True if device is disabled.')),
     );
   }
 
@@ -294,6 +298,7 @@ final class AlmanacDevice
         'value' => $status->getValue(),
         'name' => $status->getName(),
       ),
+      'disabled' => $this->isDisabled(),
     );
   }
 

src/applications/almanac/storage/AlmanacService.php

@@ -21,6 +21,7 @@ final class AlmanacService
 
   private $almanacProperties = self::ATTACHABLE;
   private $bindings = self::ATTACHABLE;
+  private $activeBindings = self::ATTACHABLE;
   private $serviceImplementation = self::ATTACHABLE;
 
   public static function initializeNewService($type) {
@@ -91,23 +92,36 @@ final class AlmanacService
   }
 
   public function getActiveBindings() {
-    $bindings = $this->getBindings();
-
-    // Filter out disabled bindings.
-    foreach ($bindings as $key => $binding) {
-      if ($binding->getIsDisabled()) {
-        unset($bindings[$key]);
-      }
-    }
-
-    return $bindings;
+    return $this->assertAttached($this->activeBindings);
   }
 
   public function attachBindings(array $bindings) {
+    $active_bindings = array();
+    foreach ($bindings as $key => $binding) {
+      // Filter out disabled bindings.
+      if ($binding->getIsDisabled()) {
+        continue;
+      }
+
+      // Filter out bindings to disabled devices.
+      if ($binding->getDevice()->isDisabled()) {
+        continue;
+      }
+
+      $active_bindings[$key] = $binding;
+    }
+
+    $this->attachActiveBindings($active_bindings);
+
     $this->bindings = $bindings;
     return $this;
   }
 
+  public function attachActiveBindings(array $bindings) {
+    $this->activeBindings = $bindings;
+    return $this;
+  }
+
   public function getServiceImplementation() {
     return $this->assertAttached($this->serviceImplementation);
   }
@@ -289,6 +303,9 @@ final class AlmanacService
         ->setAttachmentKey('properties'),
       id(new AlmanacBindingsSearchEngineAttachment())
         ->setAttachmentKey('bindings'),
+      id(new AlmanacBindingsSearchEngineAttachment())
+        ->setIsActive(true)
+        ->setAttachmentKey('activeBindings'),
     );
   }
 

src/applications/almanac/view/AlmanacBindingTableView.php

@@ -56,21 +56,41 @@ final class AlmanacBindingTableView extends AphrontView {
 
     $icon_active = id(new PHUIIconView())
       ->setIcon('fa-check')
+      ->setColor('green')
       ->addSigil('has-tooltip')
       ->setMetadata(
         array(
           'tip' => pht('Active'),
         ));
 
+    $icon_device_disabled = id(new PHUIIconView())
+      ->setIcon('fa-times')
+      ->setColor('grey')
+      ->addSigil('has-tooltip')
+      ->setMetadata(
+        array(
+          'tip' => pht('Device Disabled'),
+        ));
+
     $rows = array();
     foreach ($bindings as $binding) {
       $addr = $binding->getInterface()->getAddress();
       $port = $binding->getInterface()->getPort();
 
+      $device = $binding->getDevice();
+      if ($device->isDisabled()) {
+        $binding_icon = $icon_device_disabled;
+      } else if ($binding->getIsDisabled()) {
+        $binding_icon = $icon_disabled;
+      } else {
+        $binding_icon = $icon_active;
+      }
+
       $rows[] = array(
         $binding->getID(),
-        ($binding->getIsDisabled() ? $icon_disabled : $icon_active),
+        $binding_icon,
         $handles->renderHandle($binding->getServicePHID()),
+
         $handles->renderHandle($binding->getDevicePHID()),
         $handles->renderHandle($binding->getInterface()->getNetworkPHID()),
         $binding->getInterface()->renderDisplayAddress(),

src/applications/diffusion/management/DiffusionRepositoryStorageManagementPanel.php

@@ -89,7 +89,7 @@ final class DiffusionRepositoryStorageManagementPanel
             AlmanacClusterRepositoryServiceType::SERVICETYPE,
           ))
         ->withPHIDs(array($service_phid))
-        ->needBindings(true)
+        ->needActiveBindings(true)
         ->executeOne();
       if (!$service) {
         // TODO: Viewer may not have permission to see the service, or it may
@@ -104,7 +104,7 @@ final class DiffusionRepositoryStorageManagementPanel
 
     $rows = array();
     if ($service) {
-      $bindings = $service->getBindings();
+      $bindings = $service->getActiveBindings();
       $bindings = mgroup($bindings, 'getDevicePHID');
 
       // This is an unusual read which always comes from the master.
@@ -117,29 +117,19 @@ final class DiffusionRepositoryStorageManagementPanel
 
       $versions = mpull($versions, null, 'getDevicePHID');
 
-      // List enabled devices first, then sort devices in each group by name.
       $sort = array();
       foreach ($bindings as $key => $binding_group) {
-        $all_disabled = $this->isDisabledGroup($binding_group);
-
         $sort[$key] = id(new PhutilSortVector())
-          ->addInt($all_disabled ? 1 : 0)
           ->addString(head($binding_group)->getDevice()->getName());
       }
       $sort = msortv($sort, 'getSelf');
       $bindings = array_select_keys($bindings, array_keys($sort)) + $bindings;
 
       foreach ($bindings as $binding_group) {
-        $all_disabled = $this->isDisabledGroup($binding_group);
         $any_binding = head($binding_group);
 
-        if ($all_disabled) {
-          $binding_icon = 'fa-times grey';
-          $binding_tip = pht('Disabled');
-        } else {
-          $binding_icon = 'fa-folder-open green';
-          $binding_tip = pht('Active');
-        }
+        $binding_icon = 'fa-folder-open green';
+        $binding_tip = pht('Active');
 
         $binding_icon = id(new PHUIIconView())
           ->setIcon($binding_icon)
@@ -376,17 +366,4 @@ final class DiffusionRepositoryStorageManagementPanel
     return $box_view;
   }
 
-
-  private function isDisabledGroup(array $binding_group) {
-    assert_instances_of($binding_group, 'AlmanacBinding');
-
-    foreach ($binding_group as $binding) {
-      if (!$binding->getIsDisabled()) {
-        return false;
-      }
-    }
-
-    return true;
-  }
-
 }

src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php

@@ -34,7 +34,7 @@ final class DrydockAlmanacServiceHostBlueprintImplementation
     DrydockBlueprint $blueprint,
     DrydockLease $lease) {
     $services = $this->loadServices($blueprint);
-    $bindings = $this->loadAllBindings($services);
+    $bindings = $this->getActiveBindings($services);
 
     if (!$bindings) {
       // If there are no devices bound to the services for this blueprint,
@@ -222,7 +222,7 @@ final class DrydockAlmanacServiceHostBlueprintImplementation
         ->setViewer($viewer)
         ->withPHIDs($service_phids)
         ->withServiceTypes($this->getAlmanacServiceTypes())
-        ->needBindings(true)
+        ->needActiveBindings(true)
         ->execute();
       $services = mpull($services, null, 'getPHID');
 
@@ -242,9 +242,9 @@ final class DrydockAlmanacServiceHostBlueprintImplementation
     return $this->services;
   }
 
-  private function loadAllBindings(array $services) {
+  private function getActive(array $services) {
     assert_instances_of($services, 'AlmanacService');
-    $bindings = array_mergev(mpull($services, 'getBindings'));
+    $bindings = array_mergev(mpull($services, 'getActiveBindings'));
     return mpull($bindings, null, 'getPHID');
   }
 
@@ -271,15 +271,10 @@ final class DrydockAlmanacServiceHostBlueprintImplementation
       $allocated_phids = array_fuse($allocated_phids);
 
       $services = $this->loadServices($blueprint);
-      $bindings = $this->loadAllBindings($services);
+      $bindings = $this->getActiveBindings($services);
 
       $free = array();
       foreach ($bindings as $binding) {
-        // Don't consider disabled bindings to be available.
-        if ($binding->getIsDisabled()) {
-          continue;
-        }
-
         if (empty($allocated_phids[$binding->getPHID()])) {
           $free[] = $binding;
         }

src/applications/repository/management/PhabricatorRepositoryManagementClusterizeWorkflow.php

@@ -61,7 +61,7 @@ final class PhabricatorRepositoryManagementClusterizeWorkflow
           array(
             AlmanacClusterRepositoryServiceType::SERVICETYPE,
           ))
-        ->needBindings(true)
+        ->needActiveBindings(true)
         ->executeOne();
       if (!$service) {
         throw new PhutilArgumentUsageException(

src/applications/repository/storage/PhabricatorRepository.php

@@ -2109,7 +2109,7 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
       throw new Exception(
         pht(
           'The Almanac service for this repository is not bound to any '.
-          'interfaces.'));
+          'active interfaces.'));
     }
 
     $uris = array();
@@ -2531,7 +2531,7 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
     $service = id(new AlmanacServiceQuery())
       ->setViewer(PhabricatorUser::getOmnipotentUser())
       ->withPHIDs(array($service_phid))
-      ->needBindings(true)
+      ->needActiveBindings(true)
       ->needProperties(true)
       ->executeOne();
     if (!$service) {