Make ManiphestTaskQuery more modern and safe

Summary: Ref T8637. This class has some really old parameter handling which can send `withIDs(array())` down a "fetch everything" pathway. Clean up most of it.

Test Plan: Issued every ApplicationSearch query.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8637

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

src/applications/maniphest/query/ManiphestTaskQuery.php

@@ -6,13 +6,13 @@
  */
 final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery {
 
-  private $taskIDs             = array();
+  private $taskIDs;
-  private $taskPHIDs           = array();
+  private $taskPHIDs;
-  private $authorPHIDs         = array();
+  private $authorPHIDs;
-  private $ownerPHIDs          = array();
+  private $ownerPHIDs;
   private $noOwner;
   private $anyOwner;
-  private $subscriberPHIDs     = array();
+  private $subscriberPHIDs;
   private $dateCreatedAfter;
   private $dateCreatedBefore;
   private $dateModifiedAfter;
@@ -216,75 +216,7 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery {
     $task_dao = new ManiphestTask();
     $conn = $task_dao->establishConnection('r');
 
-    $where = array();
+    $where = $this->buildWhereClause($conn);
-    $where[] = $this->buildTaskIDsWhereClause($conn);
-    $where[] = $this->buildTaskPHIDsWhereClause($conn);
-    $where[] = $this->buildStatusWhereClause($conn);
-    $where[] = $this->buildStatusesWhereClause($conn);
-    $where[] = $this->buildDependenciesWhereClause($conn);
-    $where[] = $this->buildAuthorWhereClause($conn);
-    $where[] = $this->buildOwnerWhereClause($conn);
-    $where[] = $this->buildFullTextWhereClause($conn);
-
-    if ($this->dateCreatedAfter) {
-      $where[] = qsprintf(
-        $conn,
-        'task.dateCreated >= %d',
-        $this->dateCreatedAfter);
-    }
-
-    if ($this->dateCreatedBefore) {
-      $where[] = qsprintf(
-        $conn,
-        'task.dateCreated <= %d',
-        $this->dateCreatedBefore);
-    }
-
-    if ($this->dateModifiedAfter) {
-      $where[] = qsprintf(
-        $conn,
-        'task.dateModified >= %d',
-        $this->dateModifiedAfter);
-    }
-
-    if ($this->dateModifiedBefore) {
-      $where[] = qsprintf(
-        $conn,
-        'task.dateModified <= %d',
-        $this->dateModifiedBefore);
-    }
-
-    if ($this->priorities) {
-      $where[] = qsprintf(
-        $conn,
-        'task.priority IN (%Ld)',
-        $this->priorities);
-    }
-
-    if ($this->subpriorities) {
-      $where[] = qsprintf(
-        $conn,
-        'task.subpriority IN (%Lf)',
-        $this->subpriorities);
-    }
-
-    if ($this->subpriorityMin) {
-      $where[] = qsprintf(
-        $conn,
-        'task.subpriority >= %f',
-        $this->subpriorityMin);
-    }
-
-    if ($this->subpriorityMax) {
-      $where[] = qsprintf(
-        $conn,
-        'task.subpriority <= %f',
-        $this->subpriorityMax);
-    }
-
-    $where[] = $this->buildWhereClauseParts($conn);
-
-    $where = $this->formatWhereClause($where);
 
     $group_column = '';
     switch ($this->groupBy) {
@@ -392,28 +324,101 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery {
     return $tasks;
   }
 
-  private function buildTaskIDsWhereClause(AphrontDatabaseConnection $conn) {
+  protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
-    if (!$this->taskIDs) {
+    $where = parent::buildWhereClauseParts($conn);
-      return null;
-    }
 
-    return qsprintf(
+    $where[] = $this->buildStatusWhereClause($conn);
+    $where[] = $this->buildDependenciesWhereClause($conn);
+    $where[] = $this->buildOwnerWhereClause($conn);
+    $where[] = $this->buildFullTextWhereClause($conn);
+
+    if ($this->taskIDs !== null) {
+      $where[] = qsprintf(
         $conn,
         'task.id in (%Ld)',
         $this->taskIDs);
     }
 
-  private function buildTaskPHIDsWhereClause(AphrontDatabaseConnection $conn) {
+    if ($this->taskPHIDs !== null) {
-    if (!$this->taskPHIDs) {
+      $where[] = qsprintf(
-      return null;
-    }
-
-    return qsprintf(
         $conn,
         'task.phid in (%Ls)',
         $this->taskPHIDs);
     }
 
+    if ($this->statuses !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'task.status IN (%Ls)',
+        $this->statuses);
+    }
+
+    if ($this->authorPHIDs !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'task.authorPHID in (%Ls)',
+        $this->authorPHIDs);
+    }
+
+    if ($this->dateCreatedAfter) {
+      $where[] = qsprintf(
+        $conn,
+        'task.dateCreated >= %d',
+        $this->dateCreatedAfter);
+    }
+
+    if ($this->dateCreatedBefore) {
+      $where[] = qsprintf(
+        $conn,
+        'task.dateCreated <= %d',
+        $this->dateCreatedBefore);
+    }
+
+    if ($this->dateModifiedAfter) {
+      $where[] = qsprintf(
+        $conn,
+        'task.dateModified >= %d',
+        $this->dateModifiedAfter);
+    }
+
+    if ($this->dateModifiedBefore) {
+      $where[] = qsprintf(
+        $conn,
+        'task.dateModified <= %d',
+        $this->dateModifiedBefore);
+    }
+
+    if ($this->priorities !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'task.priority IN (%Ld)',
+        $this->priorities);
+    }
+
+    if ($this->subpriorities !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'task.subpriority IN (%Lf)',
+        $this->subpriorities);
+    }
+
+    if ($this->subpriorityMin !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'task.subpriority >= %f',
+        $this->subpriorityMin);
+    }
+
+    if ($this->subpriorityMax !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'task.subpriority <= %f',
+        $this->subpriorityMax);
+    }
+
+    return $where;
+  }
+
   private function buildStatusWhereClause(AphrontDatabaseConnection $conn) {
     static $map = array(
       self::STATUS_RESOLVED   => ManiphestTaskStatus::STATUS_CLOSED_RESOLVED,
@@ -448,27 +453,6 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery {
     }
   }
 
-  private function buildStatusesWhereClause(AphrontDatabaseConnection $conn) {
-    if ($this->statuses) {
-      return qsprintf(
-        $conn,
-        'task.status IN (%Ls)',
-        $this->statuses);
-    }
-    return null;
-  }
-
-  private function buildAuthorWhereClause(AphrontDatabaseConnection $conn) {
-    if (!$this->authorPHIDs) {
-      return null;
-    }
-
-    return qsprintf(
-      $conn,
-      'task.authorPHID in (%Ls)',
-      $this->authorPHIDs);
-  }
-
   private function buildOwnerWhereClause(AphrontDatabaseConnection $conn) {
     $subclause = array();
 
@@ -590,7 +574,7 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery {
         id(new ManiphestTask())->getTableName());
     }
 
-    if ($this->subscriberPHIDs) {
+    if ($this->subscriberPHIDs !== null) {
       $joins[] = qsprintf(
         $conn_r,
         'JOIN %T e_ccs ON e_ccs.src = task.phid '.