Fix issues where Drydock queries didn't work correctly with empty arrays

Summary: Ref T2015.  This fixes issues where the Drydock queries wouldn't filter (or throw an exception) when passed empty arrays for their `with` methods.  In addition, this also adds `array_unique` to the resource and lease subqueries so that we don't pull in a bunch of stuff if logs or leases have the same related objects.

Test Plan: Tested it by using DarkConsole on the log controller.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: joshuaspence, Korvin, epriestley

Maniphest Tasks: T2015

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

src/applications/drydock/query/DrydockBlueprintQuery.php

@@ -51,21 +51,21 @@ final class DrydockBlueprintQuery extends DrydockQuery {
   protected function buildWhereClause(AphrontDatabaseConnection $conn_r) {
     $where = array();
 
-    if ($this->ids) {
+    if ($this->ids !== null) {
       $where[] = qsprintf(
         $conn_r,
         'id IN (%Ld)',
         $this->ids);
     }
 
-    if ($this->phids) {
+    if ($this->phids !== null) {
       $where[] = qsprintf(
         $conn_r,
         'phid IN (%Ls)',
         $this->phids);
     }
 
-    if ($this->datasourceQuery) {
+    if ($this->datasourceQuery !== null) {
       $where[] = qsprintf(
         $conn_r,
         'blueprintName LIKE %>',

src/applications/drydock/query/DrydockLeaseQuery.php

@@ -47,7 +47,7 @@ final class DrydockLeaseQuery extends DrydockQuery {
       $resources = id(new DrydockResourceQuery())
         ->setParentQuery($this)
         ->setViewer($this->getViewer())
-        ->withIDs($resource_ids)
+        ->withIDs(array_unique($resource_ids))
         ->execute();
     } else {
       $resources = array();
@@ -71,35 +71,35 @@ final class DrydockLeaseQuery extends DrydockQuery {
   protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
     $where = parent::buildWhereClauseParts($conn);
 
-    if ($this->resourceIDs) {
+    if ($this->resourceIDs !== null) {
       $where[] = qsprintf(
         $conn,
         'resourceID IN (%Ld)',
         $this->resourceIDs);
     }
 
-    if ($this->ids) {
+    if ($this->ids !== null) {
       $where[] = qsprintf(
         $conn,
         'id IN (%Ld)',
         $this->ids);
     }
 
-    if ($this->phids) {
+    if ($this->phids !== null) {
       $where[] = qsprintf(
         $conn,
         'phid IN (%Ls)',
         $this->phids);
     }
 
-    if ($this->statuses) {
+    if ($this->statuses !== null) {
       $where[] = qsprintf(
         $conn,
         'status IN (%Ld)',
         $this->statuses);
     }
 
-    if ($this->datasourceQuery) {
+    if ($this->datasourceQuery !== null) {
       $where[] = qsprintf(
         $conn,
         'id = %d',

src/applications/drydock/query/DrydockLogQuery.php

@@ -36,7 +36,7 @@ final class DrydockLogQuery extends DrydockQuery {
       $resources = id(new DrydockResourceQuery())
         ->setParentQuery($this)
         ->setViewer($this->getViewer())
-        ->withIDs($resource_ids)
+        ->withIDs(array_unique($resource_ids))
         ->execute();
     } else {
       $resources = array();
@@ -59,7 +59,7 @@ final class DrydockLogQuery extends DrydockQuery {
       $leases = id(new DrydockLeaseQuery())
         ->setParentQuery($this)
         ->setViewer($this->getViewer())
-        ->withIDs($lease_ids)
+        ->withIDs(array_unique($lease_ids))
         ->execute();
     } else {
       $leases = array();
@@ -91,14 +91,14 @@ final class DrydockLogQuery extends DrydockQuery {
   protected function buildWhereClause(AphrontDatabaseConnection $conn_r) {
     $where = array();
 
-    if ($this->resourceIDs) {
+    if ($this->resourceIDs !== null) {
       $where[] = qsprintf(
         $conn_r,
         'resourceID IN (%Ld)',
         $this->resourceIDs);
     }
 
-    if ($this->leaseIDs) {
+    if ($this->leaseIDs !== null) {
       $where[] = qsprintf(
         $conn_r,
         'leaseID IN (%Ld)',

src/applications/drydock/query/DrydockLogSearchEngine.php

@@ -24,22 +24,38 @@ final class DrydockLogSearchEngine extends PhabricatorApplicationSearchEngine {
   }
 
   public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) {
+    $resource_phids = $saved->getParameter('resourcePHIDs', array());
+    $lease_phids = $saved->getParameter('leasePHIDs', array());
 
-    // TODO: Change logs to use PHIDs instead of IDs.
+     // TODO: Change logs to use PHIDs instead of IDs.
-    $resource_ids = id(new DrydockResourceQuery())
+    $resource_ids = array();
-      ->setViewer(PhabricatorUser::getOmnipotentUser())
+    $lease_ids = array();
-      ->withPHIDs($saved->getParameter('resourcePHIDs', array()))
-      ->execute();
-    $resource_ids = mpull($resource_ids, 'getID');
-    $lease_ids = id(new DrydockLeaseQuery())
-      ->setViewer(PhabricatorUser::getOmnipotentUser())
-      ->withPHIDs($saved->getParameter('leasePHIDs', array()))
-      ->execute();
-    $lease_ids = mpull($lease_ids, 'getID');
 
-    return id(new DrydockLogQuery())
+    if ($resource_phids) {
-      ->withResourceIDs($resource_ids)
+      $resource_ids = id(new DrydockResourceQuery())
-      ->withLeaseIDs($lease_ids);
+        ->setViewer(PhabricatorUser::getOmnipotentUser())
+        ->withPHIDs($resource_phids)
+        ->execute();
+      $resource_ids = mpull($resource_ids, 'getID');
+    }
+
+    if ($lease_phids) {
+      $lease_ids = id(new DrydockLeaseQuery())
+        ->setViewer(PhabricatorUser::getOmnipotentUser())
+        ->withPHIDs($lease_phids)
+        ->execute();
+      $lease_ids = mpull($lease_ids, 'getID');
+    }
+
+    $query = new DrydockLogQuery();
+    if ($resource_ids) {
+      $query->withResourceIDs($resource_ids);
+    }
+    if ($lease_ids) {
+      $query->withLeaseIDs($lease_ids);
+    }
+
+    return $query;
   }
 
   public function buildSearchForm(

src/applications/drydock/query/DrydockResourceQuery.php

@@ -59,42 +59,42 @@ final class DrydockResourceQuery extends DrydockQuery {
   protected function buildWhereClause(AphrontDatabaseConnection $conn_r) {
     $where = array();
 
-    if ($this->ids) {
+    if ($this->ids !== null) {
       $where[] = qsprintf(
         $conn_r,
         'id IN (%Ld)',
         $this->ids);
     }
 
-    if ($this->phids) {
+    if ($this->phids !== null) {
       $where[] = qsprintf(
         $conn_r,
         'phid IN (%Ls)',
         $this->phids);
     }
 
-    if ($this->types) {
+    if ($this->types !== null) {
       $where[] = qsprintf(
         $conn_r,
         'type IN (%Ls)',
         $this->types);
     }
 
-    if ($this->statuses) {
+    if ($this->statuses !== null) {
       $where[] = qsprintf(
         $conn_r,
         'status IN (%Ls)',
         $this->statuses);
     }
 
-    if ($this->blueprintPHIDs) {
+    if ($this->blueprintPHIDs !== null) {
       $where[] = qsprintf(
         $conn_r,
         'blueprintPHID IN (%Ls)',
         $this->blueprintPHIDs);
     }
 
-    if ($this->datasourceQuery) {
+    if ($this->datasourceQuery !== null) {
       $where[] = qsprintf(
         $conn_r,
         'name LIKE %>',