Move ManiphestTaskQuery to EdgeLogic

Summary:
Ref T4100. Share all edge logic code across applications.

  - Internalizes the "check that the viewer can see projects" check into edge logic.
  - Adds some convenience functions. Some of these aren't really all that convenient, but it's rare that we actually apply project constraints to queries in the applications -- and most of these callsites will go away in the long term -- so I didn't go too crazy with providing a simpler `withProjectPHIDs()` universal API or anything.

Test Plan:
  - Grepped for all affected symbols.
  - Tried to violate policies.
  - Used workboards.
  - Used normal Maniphest queries.
  - Used `maniphest.query`.
  - Verified the special grouping behavior works as expected.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4100

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

src/applications/home/controller/PhabricatorHomeMainController.php

@@ -179,7 +179,10 @@ final class PhabricatorHomeMainController extends PhabricatorHomeController {
         ->setViewer($user)
         ->withStatuses(ManiphestTaskStatus::getOpenStatusConstants())
         ->withPriorities(array($needs_triage))
-        ->withAnyProjects(mpull($projects, 'getPHID'))
+        ->withEdgeLogicPHIDs(
+          PhabricatorProjectObjectHasProjectEdgeType::EDGECONST,
+          PhabricatorQueryConstraint::OPERATOR_OR,
+          mpull($projects, 'getPHID'))
         ->needProjectPHIDs(true)
         ->setLimit(10);
       $tasks = $task_query->execute();

src/applications/maniphest/conduit/ManiphestQueryConduitAPIMethod.php

@@ -79,7 +79,10 @@ final class ManiphestQueryConduitAPIMethod extends ManiphestConduitAPIMethod {
 
     $projects = $request->getValue('projectPHIDs');
     if ($projects) {
-      $query->withAllProjects($projects);
+      $query->withEdgeLogicPHIDs(
+        PhabricatorProjectObjectHasProjectEdgeType::EDGECONST,
+        PhabricatorQueryConstraint::OPERATOR_AND,
+        $projects);
     }
 
     $ccs = $request->getValue('ccPHIDs');

src/applications/maniphest/controller/ManiphestReportController.php

@@ -411,7 +411,10 @@ final class ManiphestReportController extends ManiphestController {
       $handles = $this->loadViewerHandles($phids);
       $project_handle = $handles[$project_phid];
 
-      $query->withAnyProjects($phids);
+      $query->withEdgeLogicPHIDs(
+        PhabricatorProjectObjectHasProjectEdgeType::EDGECONST,
+        PhabricatorQueryConstraint::OPERATOR_OR,
+        $phids);
     }
 
     $tasks = $query->execute();

src/applications/maniphest/query/ManiphestTaskQuery.php

@@ -11,12 +11,7 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery {
   private $authorPHIDs         = array();
   private $ownerPHIDs          = array();
   private $includeUnowned      = null;
-  private $projectPHIDs        = array();
-  private $xprojectPHIDs       = array();
   private $subscriberPHIDs     = array();
-  private $anyProjectPHIDs     = array();
-  private $anyUserProjectPHIDs = array();
-  private $includeNoProject    = null;
   private $dateCreatedAfter;
   private $dateCreatedBefore;
   private $dateModifiedAfter;
@@ -57,7 +52,6 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery {
   private $needProjectPHIDs;
   private $blockingTasks;
   private $blockedTasks;
-  private $projectPolicyCheckFailed = false;
 
   public function withAuthors(array $authors) {
     $this->authorPHIDs = $authors;
@@ -89,39 +83,6 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery {
     return $this;
   }
 
-  public function withAllProjects(array $projects) {
-    $this->includeNoProject = false;
-    foreach ($projects as $k => $phid) {
-      if ($phid == ManiphestTaskOwner::PROJECT_NO_PROJECT) {
-        $this->includeNoProject = true;
-        unset($projects[$k]);
-      }
-    }
-    $this->projectPHIDs = $projects;
-    return $this;
-  }
-
-  /**
-   * Add an additional "all projects" constraint to existing filters.
-   *
-   * This is used by boards to supplement queries.
-   *
-   * @param list<phid> List of project PHIDs to add to any existing constraint.
-   * @return this
-   */
-  public function addWithAllProjects(array $projects) {
-    if ($this->projectPHIDs === null) {
-      $this->projectPHIDs = array();
-    }
-
-    return $this->withAllProjects(array_merge($this->projectPHIDs, $projects));
-  }
-
-  public function withoutProjects(array $projects) {
-    $this->xprojectPHIDs = $projects;
-    return $this;
-  }
-
   public function withStatus($status) {
     $this->status = $status;
     return $this;
@@ -168,16 +129,6 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery {
     return $this;
   }
 
-  public function withAnyProjects(array $projects) {
-    $this->anyProjectPHIDs = $projects;
-    return $this;
-  }
-
-  public function withAnyUserProjects(array $users) {
-    $this->anyUserProjectPHIDs = $users;
-    return $this;
-  }
-
   /**
    * True returns tasks that are blocking other tasks only.
    * False returns tasks that are not blocking other tasks only.
@@ -241,27 +192,6 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery {
   }
 
   protected function willExecute() {
-    // Make sure the user can see any projects specified in this
-    // query FIRST.
-    if ($this->projectPHIDs) {
-      $projects = id(new PhabricatorProjectQuery())
-        ->setViewer($this->getViewer())
-        ->withPHIDs($this->projectPHIDs)
-        ->execute();
-      $projects = mpull($projects, null, 'getPHID');
-      foreach ($this->projectPHIDs as $index => $phid) {
-        $project = idx($projects, $phid);
-        if (!$project) {
-          unset($this->projectPHIDs[$index]);
-          continue;
-        }
-      }
-      if (!$this->projectPHIDs) {
-        $this->projectPolicyCheckFailed = true;
-      }
-      $this->projectPHIDs = array_values($this->projectPHIDs);
-    }
-
     // If we already have an order vector, use it as provided.
     // TODO: This is a messy hack to make setOrderVector() stronger than
     // setPriority().
@@ -325,11 +255,6 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery {
   }
 
   protected function loadPage() {
-
-    if ($this->projectPolicyCheckFailed) {
-      throw new PhabricatorEmptyQueryException();
-    }
-
     $task_dao = new ManiphestTask();
     $conn = $task_dao->establishConnection('r');
 
@@ -341,10 +266,6 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery {
     $where[] = $this->buildDependenciesWhereClause($conn);
     $where[] = $this->buildAuthorWhereClause($conn);
     $where[] = $this->buildOwnerWhereClause($conn);
-    $where[] = $this->buildProjectWhereClause($conn);
-    $where[] = $this->buildAnyProjectWhereClause($conn);
-    $where[] = $this->buildAnyUserProjectWhereClause($conn);
-    $where[] = $this->buildXProjectWhereClause($conn);
     $where[] = $this->buildFullTextWhereClause($conn);
 
     if ($this->dateCreatedAfter) {
@@ -407,11 +328,6 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery {
 
     $where = $this->formatWhereClause($where);
 
-    $count = '';
-    if (count($this->projectPHIDs) > 1) {
-      $count = ', COUNT(project.dst) projectCount';
-    }
-
     $group_column = '';
     switch ($this->groupBy) {
       case self::GROUP_PROJECT:
@@ -423,9 +339,8 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery {
 
     $rows = queryfx_all(
       $conn,
-      '%Q %Q %Q FROM %T task %Q %Q %Q %Q %Q %Q',
+      '%Q %Q FROM %T task %Q %Q %Q %Q %Q %Q',
       $this->buildSelectClause($conn),
-      $count,
       $group_column,
       $task_dao->getTableName(),
       $this->buildJoinClause($conn),
@@ -689,84 +604,11 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery {
     return '('.implode(') OR (', $parts).')';
   }
 
-  private function buildProjectWhereClause(AphrontDatabaseConnection $conn) {
-    if (!$this->projectPHIDs && !$this->includeNoProject) {
-      return null;
-    }
-
-    $parts = array();
-    if ($this->projectPHIDs) {
-      $parts[] = qsprintf(
-        $conn,
-        'project.dst in (%Ls)',
-        $this->projectPHIDs);
-    }
-    if ($this->includeNoProject) {
-      $parts[] = qsprintf(
-        $conn,
-        'project.dst IS NULL');
-    }
-
-    return '('.implode(') OR (', $parts).')';
-  }
-
-  private function buildAnyProjectWhereClause(AphrontDatabaseConnection $conn) {
-    if (!$this->anyProjectPHIDs) {
-      return null;
-    }
-
-    return qsprintf(
-      $conn,
-      'anyproject.dst IN (%Ls)',
-      $this->anyProjectPHIDs);
-  }
-
-  private function buildAnyUserProjectWhereClause(
-    AphrontDatabaseConnection $conn) {
-    if (!$this->anyUserProjectPHIDs) {
-      return null;
-    }
-
-    $projects = id(new PhabricatorProjectQuery())
-      ->setViewer($this->getViewer())
-      ->withMemberPHIDs($this->anyUserProjectPHIDs)
-      ->execute();
-    $any_user_project_phids = mpull($projects, 'getPHID');
-    if (!$any_user_project_phids) {
-      throw new PhabricatorEmptyQueryException();
-    }
-
-    return qsprintf(
-      $conn,
-      'anyproject.dst IN (%Ls)',
-      $any_user_project_phids);
-  }
-
-  private function buildXProjectWhereClause(AphrontDatabaseConnection $conn) {
-    if (!$this->xprojectPHIDs) {
-      return null;
-    }
-
-    return qsprintf(
-      $conn,
-      'xproject.dst IS NULL');
-  }
-
   protected function buildJoinClauseParts(AphrontDatabaseConnection $conn_r) {
     $edge_table = PhabricatorEdgeConfig::TABLE_NAME_EDGE;
 
     $joins = array();
 
-    if ($this->projectPHIDs || $this->includeNoProject) {
-      $joins[] = qsprintf(
-        $conn_r,
-        '%Q JOIN %T project ON project.src = task.phid
-          AND project.type = %d',
-        ($this->includeNoProject ? 'LEFT' : ''),
-        $edge_table,
-        PhabricatorProjectObjectHasProjectEdgeType::EDGECONST);
-    }
-
     if ($this->shouldJoinBlockingTasks()) {
       $joins[] = qsprintf(
         $conn_r,
@@ -788,26 +630,6 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery {
         id(new ManiphestTask())->getTableName());
     }
 
-    if ($this->anyProjectPHIDs || $this->anyUserProjectPHIDs) {
-      $joins[] = qsprintf(
-        $conn_r,
-        'JOIN %T anyproject ON anyproject.src = task.phid
-          AND anyproject.type = %d',
-        $edge_table,
-        PhabricatorProjectObjectHasProjectEdgeType::EDGECONST);
-    }
-
-    if ($this->xprojectPHIDs) {
-      $joins[] = qsprintf(
-        $conn_r,
-        'LEFT JOIN %T xproject ON xproject.src = task.phid
-          AND xproject.type = %d
-          AND xproject.dst IN (%Ls)',
-        $edge_table,
-        PhabricatorProjectObjectHasProjectEdgeType::EDGECONST,
-        $this->xprojectPHIDs);
-    }
-
     if ($this->subscriberPHIDs) {
       $joins[] = qsprintf(
         $conn_r,
@@ -853,9 +675,7 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery {
   }
 
   protected function buildGroupClause(AphrontDatabaseConnection $conn_r) {
-    $joined_multiple_rows = (count($this->projectPHIDs) > 1) ||
+    $joined_multiple_rows = $this->shouldJoinBlockingTasks() ||
-                            (count($this->anyProjectPHIDs) > 1) ||
-                            $this->shouldJoinBlockingTasks() ||
                             $this->shouldJoinBlockedTasks() ||
                             ($this->shouldGroupQueryResultRows());
 
@@ -894,33 +714,32 @@ final class ManiphestTaskQuery extends PhabricatorCursorPagedPolicyAwareQuery {
    *                    construction.
    */
   private function getIgnoreGroupedProjectPHIDs() {
-    $phids = array();
+    // Maybe we should also exclude the "OPERATOR_NOT" PHIDs? It won't
-
-    if ($this->projectPHIDs) {
-      $phids[] = $this->projectPHIDs;
-    }
-
-    if (count($this->anyProjectPHIDs) == 1) {
-      $phids[] = $this->anyProjectPHIDs;
-    }
-
-    // Maybe we should also exclude the "excludeProjectPHIDs"? It won't
     // impact the results, but we might end up with a better query plan.
     // Investigate this on real data? This is likely very rare.
 
-    return array_mergev($phids);
+    $edge_types = array(
+      PhabricatorProjectObjectHasProjectEdgeType::EDGECONST,
+    );
+
+    $phids = array();
+
+    $phids[] = $this->getEdgeLogicValues(
+      $edge_types,
+      array(
+        PhabricatorQueryConstraint::OPERATOR_AND,
+      ));
+
+    $any = $this->getEdgeLogicValues(
+      $edge_types,
+      array(
+        PhabricatorQueryConstraint::OPERATOR_OR,
+      ));
+    if (count($any) == 1) {
+      $phids[] = $any;
     }
 
-  // TODO: Remove this when moving fully to edge logic.
+    return array_mergev($phids);
-  protected function buildHavingClauseParts(AphrontDatabaseConnection $conn) {
-    $having = parent::buildHavingClauseParts($conn);
-    if (count($this->projectPHIDs) > 1) {
-      $having[] = qsprintf(
-        $conn,
-        'projectCount = %d',
-        count($this->projectPHIDs));
-    }
-    return $having;
   }
 
   protected function getResultCursor($result) {

src/applications/project/controller/PhabricatorProjectBoardViewController.php

@@ -56,7 +56,6 @@ final class PhabricatorProjectBoardViewController
     $column_query = id(new PhabricatorProjectColumnQuery())
       ->setViewer($viewer)
       ->withProjectPHIDs(array($project->getPHID()));
-
     if (!$show_hidden) {
       $column_query->withStatuses(
         array(PhabricatorProjectColumn::STATUS_ACTIVE));
@@ -160,7 +159,10 @@ final class PhabricatorProjectBoardViewController
     $task_query = $engine->buildQueryFromSavedQuery($saved);
 
     $tasks = $task_query
-      ->addWithAllProjects(array($project->getPHID()))
+      ->withEdgeLogicPHIDs(
+        PhabricatorProjectObjectHasProjectEdgeType::EDGECONST,
+        PhabricatorQueryConstraint::OPERATOR_AND,
+        array($project->getPHID()))
       ->setOrderBy(ManiphestTaskQuery::ORDER_PRIORITY)
       ->setViewer($viewer)
       ->execute();

src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php

@@ -22,6 +22,7 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
   private $orderVector;
   private $builtinOrder;
   private $edgeLogicConstraints = array();
+  private $edgeLogicConstraintsAreValid = false;
 
   protected function getPageCursors(array $page) {
     return array(
@@ -1260,6 +1261,26 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
 /* -(  Edge Logic  )--------------------------------------------------------- */
 
 
+  /**
+   * Convenience method for specifying edge logic constraints with a list of
+   * PHIDs.
+   *
+   * @param const Edge constant.
+   * @param const Constraint operator.
+   * @param list<phid> List of PHIDs.
+   * @return this
+   * @task edgelogic
+   */
+  public function withEdgeLogicPHIDs($edge_type, $operator, array $phids) {
+    $constraints = array();
+    foreach ($phids as $phid) {
+      $constraints[] = new PhabricatorQueryConstraint($operator, $phid);
+    }
+
+    return $this->withEdgeLogicConstraints($edge_type, $constraints);
+  }
+
+
   /**
    * @task edgelogic
    */
@@ -1274,6 +1295,8 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
       }
     }
 
+    $this->edgeLogicConstraintsAreValid = false;
+
     return $this;
   }
 
@@ -1284,6 +1307,8 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
   public function buildEdgeLogicSelectClause(AphrontDatabaseConnection $conn) {
     $select = array();
 
+    $this->validateEdgeLogicConstraints();
+
     foreach ($this->edgeLogicConstraints as $type => $constraints) {
       foreach ($constraints as $operator => $list) {
         $alias = $this->getEdgeLogicTableAlias($operator, $type);
@@ -1508,4 +1533,81 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
   }
 
 
+  /**
+   * Select certain edge logic constraint values.
+   *
+   * @task edgelogic
+   */
+  protected function getEdgeLogicValues(
+    array $edge_types,
+    array $operators) {
+
+    $values = array();
+
+    $constraint_lists = $this->edgeLogicConstraints;
+    if ($edge_types) {
+      $constraint_lists = array_select_keys($constraint_lists, $edge_types);
+    }
+
+    foreach ($constraint_lists as $type => $constraints) {
+      if ($operators) {
+        $constraints = array_select_keys($constraints, $operators);
+      }
+      foreach ($constraints as $operator => $list) {
+        foreach ($list as $constraint) {
+          $values[] = $constraint->getValue();
+        }
+      }
+    }
+
+    return $values;
+  }
+
+
+  /**
+   * Validate edge logic constraints for the query.
+   *
+   * @return this
+   * @task edgelogic
+   */
+  private function validateEdgeLogicConstraints() {
+    if ($this->edgeLogicConstraintsAreValid) {
+      return $this;
+    }
+
+    // This should probably be more modular, eventually, but we only do
+    // project-based edge logic today.
+
+    $project_phids = $this->getEdgeLogicValues(
+      array(
+        PhabricatorProjectObjectHasProjectEdgeType::EDGECONST,
+      ),
+      array(
+        PhabricatorQueryConstraint::OPERATOR_AND,
+        PhabricatorQueryConstraint::OPERATOR_OR,
+        PhabricatorQueryConstraint::OPERATOR_NOT,
+      ));
+    if ($project_phids) {
+      $projects = id(new PhabricatorProjectQuery())
+        ->setViewer($this->getViewer())
+        ->setParentQuery($this)
+        ->withPHIDs($project_phids)
+        ->execute();
+      $projects = mpull($projects, null, 'getPHID');
+      foreach ($project_phids as $phid) {
+        if (empty($projects[$phid])) {
+          throw new PhabricatorEmptyQueryException(
+            pht(
+              'This query is constrained by a project you do not have '.
+              'permission to see.'));
+        }
+      }
+    }
+
+    $this->edgeLogicConstraintsAreValid = true;
+
+    return $this;
+  }
+
+
 }