Make queries for Project "X" mean "X, or any subproject of X"

Summary:
Ref T10010. I think this is the desired/expected default behavior (e.g., searching for "Maniphest" should find tasks in any subproject or sprint of that project).

I'll probably add an "exact(...)" function later to mean "only the Maniphest superproject, exactly, not any of its children".

Test Plan:
  - Added and executed unit tests.
  - Ran various queries from the web UI.
  - Got sensible-seeming results.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10010

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

src/__phutil_library_map__.php

@@ -2857,7 +2857,7 @@ phutil_register_library_map(array(
     'PhabricatorProjectListController' => 'applications/project/controller/PhabricatorProjectListController.php',
     'PhabricatorProjectListView' => 'applications/project/view/PhabricatorProjectListView.php',
     'PhabricatorProjectLockController' => 'applications/project/controller/PhabricatorProjectLockController.php',
-    'PhabricatorProjectLogicalAndDatasource' => 'applications/project/typeahead/PhabricatorProjectLogicalAndDatasource.php',
+    'PhabricatorProjectLogicalAncestorDatasource' => 'applications/project/typeahead/PhabricatorProjectLogicalAncestorDatasource.php',
     'PhabricatorProjectLogicalDatasource' => 'applications/project/typeahead/PhabricatorProjectLogicalDatasource.php',
     'PhabricatorProjectLogicalOrNotDatasource' => 'applications/project/typeahead/PhabricatorProjectLogicalOrNotDatasource.php',
     'PhabricatorProjectLogicalUserDatasource' => 'applications/project/typeahead/PhabricatorProjectLogicalUserDatasource.php',
@@ -7209,7 +7209,7 @@ phutil_register_library_map(array(
     'PhabricatorProjectListController' => 'PhabricatorProjectController',
     'PhabricatorProjectListView' => 'AphrontView',
     'PhabricatorProjectLockController' => 'PhabricatorProjectController',
-    'PhabricatorProjectLogicalAndDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
+    'PhabricatorProjectLogicalAncestorDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
     'PhabricatorProjectLogicalDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
     'PhabricatorProjectLogicalOrNotDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
     'PhabricatorProjectLogicalUserDatasource' => 'PhabricatorTypeaheadCompositeDatasource',

src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php

@@ -711,6 +711,157 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase {
       pht('Leave allowed without any permission.'));
   }
 
+
+  public function testComplexConstraints() {
+    $user = $this->createUser();
+    $user->save();
+
+    $engineering = $this->createProject($user);
+    $engineering_scan = $this->createProject($user, $engineering);
+    $engineering_warp = $this->createProject($user, $engineering);
+
+    $exploration = $this->createProject($user);
+    $exploration_diplomacy = $this->createProject($user, $exploration);
+
+    $task_engineering = $this->newTask(
+      $user,
+      array($engineering),
+      pht('Engineering Only'));
+
+    $task_exploration = $this->newTask(
+      $user,
+      array($exploration),
+      pht('Exploration Only'));
+
+    $task_warp_explore = $this->newTask(
+      $user,
+      array($engineering_warp, $exploration),
+      pht('Warp to New Planet'));
+
+    $task_diplomacy_scan = $this->newTask(
+      $user,
+      array($engineering_scan, $exploration_diplomacy),
+      pht('Scan Diplomat'));
+
+    $task_diplomacy = $this->newTask(
+      $user,
+      array($exploration_diplomacy),
+      pht('Diplomatic Meeting'));
+
+    $task_warp_scan = $this->newTask(
+      $user,
+      array($engineering_scan, $engineering_warp),
+      pht('Scan Warp Drives'));
+
+    $this->assertQueryByProjects(
+      $user,
+      array(
+        $task_engineering,
+        $task_warp_explore,
+        $task_diplomacy_scan,
+        $task_warp_scan,
+      ),
+      array($engineering),
+      pht('All Engineering'));
+
+    $this->assertQueryByProjects(
+      $user,
+      array(
+        $task_diplomacy_scan,
+        $task_warp_scan,
+      ),
+      array($engineering_scan),
+      pht('All Scan'));
+
+    $this->assertQueryByProjects(
+      $user,
+      array(
+        $task_warp_explore,
+        $task_diplomacy_scan,
+      ),
+      array($engineering, $exploration),
+      pht('Engineering + Exploration'));
+
+    // This is testing that a query for "Parent" and "Parent > Child" works
+    // properly.
+    $this->assertQueryByProjects(
+      $user,
+      array(
+        $task_diplomacy_scan,
+        $task_warp_scan,
+      ),
+      array($engineering, $engineering_scan),
+      pht('Engineering + Scan'));
+  }
+
+  private function newTask(
+    PhabricatorUser $viewer,
+    array $projects,
+    $name = null) {
+
+    $task = ManiphestTask::initializeNewTask($viewer);
+
+    if (!strlen($name)) {
+      $name = pht('Test Task');
+    }
+
+    $xactions = array();
+
+    $xactions[] = id(new ManiphestTransaction())
+      ->setTransactionType(ManiphestTransaction::TYPE_TITLE)
+      ->setNewValue($name);
+
+    if ($projects) {
+      $xactions[] = id(new ManiphestTransaction())
+        ->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
+        ->setMetadataValue(
+          'edge:type',
+          PhabricatorProjectObjectHasProjectEdgeType::EDGECONST)
+        ->setNewValue(
+          array(
+            '=' => array_fuse(mpull($projects, 'getPHID')),
+          ));
+    }
+
+    $editor = id(new ManiphestTransactionEditor())
+      ->setActor($viewer)
+      ->setContentSource(PhabricatorContentSource::newConsoleSource())
+      ->setContinueOnNoEffect(true)
+      ->applyTransactions($task, $xactions);
+
+    return $task;
+  }
+
+  private function assertQueryByProjects(
+    PhabricatorUser $viewer,
+    array $expect,
+    array $projects,
+    $label = null) {
+
+    $datasource = id(new PhabricatorProjectLogicalDatasource())
+      ->setViewer($viewer);
+
+    $project_phids = mpull($projects, 'getPHID');
+    $constraints = $datasource->evaluateTokens($project_phids);
+
+    $query = id(new ManiphestTaskQuery())
+      ->setViewer($viewer);
+
+    $query->withEdgeLogicConstraints(
+      PhabricatorProjectObjectHasProjectEdgeType::EDGECONST,
+      $constraints);
+
+    $tasks = $query->execute();
+
+    $expect_phids = mpull($expect, 'getTitle', 'getPHID');
+    ksort($expect_phids);
+
+    $actual_phids = mpull($tasks, 'getTitle', 'getPHID');
+    ksort($actual_phids);
+
+    $this->assertEqual($expect_phids, $actual_phids, $label);
+  }
+
   private function refreshProject(
     PhabricatorProject $project,
     PhabricatorUser $viewer,

src/applications/project/typeahead/PhabricatorProjectLogicalAncestorDatasource.php

@@ -0,0 +1,95 @@
+<?php
+
+final class PhabricatorProjectLogicalAncestorDatasource
+  extends PhabricatorTypeaheadCompositeDatasource {
+
+  public function getBrowseTitle() {
+    return pht('Browse Projects');
+  }
+
+  public function getPlaceholderText() {
+    return pht('Type a project name...');
+  }
+
+  public function getDatasourceApplicationClass() {
+    return 'PhabricatorProjectApplication';
+  }
+
+  public function getComponentDatasources() {
+    return array(
+      new PhabricatorProjectDatasource(),
+    );
+  }
+
+  protected function didEvaluateTokens(array $results) {
+    $phids = array();
+
+    foreach ($results as $result) {
+      if (!is_string($result)) {
+        continue;
+      }
+      $phids[] = $result;
+    }
+
+    $map = array();
+    $skip = array();
+    if ($phids) {
+      $phids = array_fuse($phids);
+      $viewer = $this->getViewer();
+
+      $all_projects = id(new PhabricatorProjectQuery())
+        ->setViewer($viewer)
+        ->withAncestorProjectPHIDs($phids)
+        ->execute();
+
+      foreach ($phids as $phid) {
+        $map[$phid][] = $phid;
+      }
+
+      foreach ($all_projects as $project) {
+        $project_phid = $project->getPHID();
+        $map[$project_phid][] = $project_phid;
+        foreach ($project->getAncestorProjects() as $ancestor) {
+          $ancestor_phid = $ancestor->getPHID();
+
+          if (isset($phids[$project_phid]) && isset($phids[$ancestor_phid])) {
+            // This is a descendant of some other project in the query, so
+            // we don't need to query for that project. This happens if a user
+            // runs a query for both "Engineering" and "Engineering > Warp
+            // Drive". We can only ever match the "Warp Drive" results, so
+            // we do not need to add the weaker "Engineering" constraint.
+            $skip[$ancestor_phid] = true;
+          }
+
+          $map[$ancestor_phid][] = $project_phid;
+        }
+      }
+    }
+
+    foreach ($results as $key => $result) {
+      if (!is_string($result)) {
+        continue;
+      }
+
+      if (empty($map[$result])) {
+        continue;
+      }
+
+      // This constraint is implied by another, stronger constraint.
+      if (isset($skip[$result])) {
+        unset($results[$key]);
+        continue;
+      }
+
+      // If we have duplicates, don't apply the second constraint.
+      $skip[$result] = true;
+
+      $results[$key] = new PhabricatorQueryConstraint(
+        PhabricatorQueryConstraint::OPERATOR_ANCESTOR,
+        $map[$result]);
+    }
+
+    return $results;
+  }
+
+}

src/applications/project/typeahead/PhabricatorProjectLogicalAndDatasource.php

@@ -1,36 +0,0 @@
-<?php
-
-final class PhabricatorProjectLogicalAndDatasource
-  extends PhabricatorTypeaheadCompositeDatasource {
-
-  public function getBrowseTitle() {
-    return pht('Browse Projects');
-  }
-
-  public function getPlaceholderText() {
-    return pht('Type a project name...');
-  }
-
-  public function getDatasourceApplicationClass() {
-    return 'PhabricatorProjectApplication';
-  }
-
-  public function getComponentDatasources() {
-    return array(
-      new PhabricatorProjectDatasource(),
-    );
-  }
-
-  protected function didEvaluateTokens(array $results) {
-    foreach ($results as $key => $result) {
-      if (is_string($result)) {
-        $results[$key] = new PhabricatorQueryConstraint(
-          PhabricatorQueryConstraint::OPERATOR_AND,
-          $result);
-      }
-    }
-
-    return $results;
-  }
-
-}

src/applications/project/typeahead/PhabricatorProjectLogicalDatasource.php

@@ -18,7 +18,7 @@ final class PhabricatorProjectLogicalDatasource
   public function getComponentDatasources() {
     return array(
       new PhabricatorProjectNoProjectsDatasource(),
-      new PhabricatorProjectLogicalAndDatasource(),
+      new PhabricatorProjectLogicalAncestorDatasource(),
       new PhabricatorProjectLogicalOrNotDatasource(),
       new PhabricatorProjectLogicalViewerDatasource(),
       new PhabricatorProjectLogicalUserDatasource(),

src/infrastructure/query/constraint/PhabricatorQueryConstraint.php

@@ -6,6 +6,7 @@ final class PhabricatorQueryConstraint extends Phobject {
   const OPERATOR_OR = 'or';
   const OPERATOR_NOT = 'not';
   const OPERATOR_NULL = 'null';
+  const OPERATOR_ANCESTOR = 'ancestor';
 
   private $operator;
   private $value;

src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php

@@ -1516,8 +1516,7 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
     $constraints = mgroup($constraints, 'getOperator');
     foreach ($constraints as $operator => $list) {
       foreach ($list as $item) {
-        $value = $item->getValue();
+        $this->edgeLogicConstraints[$edge_type][$operator][] = $item;
-        $this->edgeLogicConstraints[$edge_type][$operator][$value] = $item;
       }
     }
 
@@ -1548,6 +1547,47 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
                 $this->buildEdgeLogicTableAliasCount($alias));
             }
             break;
+          case PhabricatorQueryConstraint::OPERATOR_ANCESTOR:
+            // This is tricky. We have a query which specifies multiple
+            // projects, each of which may have an arbitrarily large number
+            // of descendants.
+
+            // Suppose the projects are "Engineering" and "Operations", and
+            // "Engineering" has subprojects X, Y and Z.
+
+            // We first use `FIELD(dst, X, Y, Z)` to produce a 0 if a row
+            // is not part of Engineering at all, or some number other than
+            // 0 if it is.
+
+            // Then we use `IF(..., idx, NULL)` to convert the 0 to a NULL and
+            // any other value to an index (say, 1) for the ancestor.
+
+            // We build these up for every ancestor, then use `COALESCE(...)`
+            // to select the non-null one, giving us an ancestor which this
+            // row is a member of.
+
+            // From there, we use `COUNT(DISTINCT(...))` to make sure that
+            // each result row is a member of all ancestors.
+            if (count($list) > 1) {
+              $idx = 1;
+              $parts = array();
+              foreach ($list as $constraint) {
+                $parts[] = qsprintf(
+                  $conn,
+                  'IF(FIELD(%T.dst, %Ls) != 0, %d, NULL)',
+                  $alias,
+                  (array)$constraint->getValue(),
+                  $idx++);
+              }
+              $parts = implode(', ', $parts);
+
+              $select[] = qsprintf(
+                $conn,
+                'COUNT(DISTINCT(COALESCE(%Q))) %T',
+                $parts,
+                $this->buildEdgeLogicTableAliasAncestor($alias));
+            }
+            break;
           default:
             break;
         }
@@ -1573,6 +1613,16 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
 
       foreach ($constraints as $operator => $list) {
         $alias = $this->getEdgeLogicTableAlias($operator, $type);
+
+        $phids = array();
+        foreach ($list as $constraint) {
+          $value = (array)$constraint->getValue();
+          foreach ($value as $v) {
+            $phids[$v] = $v;
+          }
+        }
+        $phids = array_keys($phids);
+
         switch ($operator) {
           case PhabricatorQueryConstraint::OPERATOR_NOT:
             $joins[] = qsprintf(
@@ -1586,8 +1636,9 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
               $alias,
               $type,
               $alias,
-              mpull($list, 'getValue'));
+              $phids);
             break;
+          case PhabricatorQueryConstraint::OPERATOR_ANCESTOR:
           case PhabricatorQueryConstraint::OPERATOR_AND:
           case PhabricatorQueryConstraint::OPERATOR_OR:
             // If we're including results with no matches, we have to degrade
@@ -1611,7 +1662,7 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
               $alias,
               $type,
               $alias,
-              mpull($list, 'getValue'));
+              $phids);
             break;
           case PhabricatorQueryConstraint::OPERATOR_NULL:
             $joins[] = qsprintf(
@@ -1711,6 +1762,15 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
                 count($list));
             }
             break;
+          case PhabricatorQueryConstraint::OPERATOR_ANCESTOR:
+            if (count($list) > 1) {
+              $having[] = qsprintf(
+                $conn,
+                '%T = %d',
+                $this->buildEdgeLogicTableAliasAncestor($alias),
+                count($list));
+            }
+            break;
         }
       }
     }
@@ -1729,6 +1789,7 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
           case PhabricatorQueryConstraint::OPERATOR_NOT:
           case PhabricatorQueryConstraint::OPERATOR_AND:
           case PhabricatorQueryConstraint::OPERATOR_OR:
+          case PhabricatorQueryConstraint::OPERATOR_ANCESTOR:
             if (count($list) > 1) {
               return true;
             }
@@ -1758,6 +1819,13 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
     return $alias.'_count';
   }
 
+  /**
+   * @task edgelogic
+   */
+  private function buildEdgeLogicTableAliasAncestor($alias) {
+    return $alias.'_ancestor';
+  }
+
 
   /**
    * Select certain edge logic constraint values.
@@ -1781,7 +1849,10 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
       }
       foreach ($constraints as $operator => $list) {
         foreach ($list as $constraint) {
-          $values[] = $constraint->getValue();
+          $value = (array)$constraint->getValue();
+          foreach ($value as $v) {
+            $values[] = $v;
+          }
         }
       }
     }
@@ -1812,6 +1883,7 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
         PhabricatorQueryConstraint::OPERATOR_AND,
         PhabricatorQueryConstraint::OPERATOR_OR,
         PhabricatorQueryConstraint::OPERATOR_NOT,
+        PhabricatorQueryConstraint::OPERATOR_ANCESTOR,
       ));
     if ($project_phids) {
       $projects = id(new PhabricatorProjectQuery())