Page MenuHomePhabricator
Files F15415098

D14910.diff
No OneTemporary
Actions

D14910.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -2857,7 +2857,7 @@
'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 @@
'PhabricatorProjectListController' => 'PhabricatorProjectController',
'PhabricatorProjectListView' => 'AphrontView',
'PhabricatorProjectLockController' => 'PhabricatorProjectController',
- 'PhabricatorProjectLogicalAndDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
+ 'PhabricatorProjectLogicalAncestorDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
'PhabricatorProjectLogicalDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
'PhabricatorProjectLogicalOrNotDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
'PhabricatorProjectLogicalUserDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
diff --git a/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php b/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
--- a/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
+++ b/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
@@ -711,6 +711,157 @@
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,
diff --git a/src/applications/project/typeahead/PhabricatorProjectLogicalAncestorDatasource.php b/src/applications/project/typeahead/PhabricatorProjectLogicalAncestorDatasource.php
new file mode 100644
--- /dev/null
+++ b/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;
+ }
+
+}
diff --git a/src/applications/project/typeahead/PhabricatorProjectLogicalAndDatasource.php b/src/applications/project/typeahead/PhabricatorProjectLogicalAndDatasource.php
deleted file mode 100644
--- a/src/applications/project/typeahead/PhabricatorProjectLogicalAndDatasource.php
+++ /dev/null
@@ -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;
- }
-
-}
diff --git a/src/applications/project/typeahead/PhabricatorProjectLogicalDatasource.php b/src/applications/project/typeahead/PhabricatorProjectLogicalDatasource.php
--- a/src/applications/project/typeahead/PhabricatorProjectLogicalDatasource.php
+++ b/src/applications/project/typeahead/PhabricatorProjectLogicalDatasource.php
@@ -18,7 +18,7 @@
public function getComponentDatasources() {
return array(
new PhabricatorProjectNoProjectsDatasource(),
- new PhabricatorProjectLogicalAndDatasource(),
+ new PhabricatorProjectLogicalAncestorDatasource(),
new PhabricatorProjectLogicalOrNotDatasource(),
new PhabricatorProjectLogicalViewerDatasource(),
new PhabricatorProjectLogicalUserDatasource(),
diff --git a/src/infrastructure/query/constraint/PhabricatorQueryConstraint.php b/src/infrastructure/query/constraint/PhabricatorQueryConstraint.php
--- a/src/infrastructure/query/constraint/PhabricatorQueryConstraint.php
+++ b/src/infrastructure/query/constraint/PhabricatorQueryConstraint.php
@@ -6,6 +6,7 @@
const OPERATOR_OR = 'or';
const OPERATOR_NOT = 'not';
const OPERATOR_NULL = 'null';
+ const OPERATOR_ANCESTOR = 'ancestor';
private $operator;
private $value;
diff --git a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php
--- a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php
+++ b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php
@@ -1516,8 +1516,7 @@
$constraints = mgroup($constraints, 'getOperator');
foreach ($constraints as $operator => $list) {
foreach ($list as $item) {
- $value = $item->getValue();
- $this->edgeLogicConstraints[$edge_type][$operator][$value] = $item;
+ $this->edgeLogicConstraints[$edge_type][$operator][] = $item;
}
}
@@ -1548,6 +1547,47 @@
$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 @@
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 @@
$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 @@
$alias,
$type,
$alias,
- mpull($list, 'getValue'));
+ $phids);
break;
case PhabricatorQueryConstraint::OPERATOR_NULL:
$joins[] = qsprintf(
@@ -1711,6 +1762,15 @@
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 @@
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 @@
return $alias.'_count';
}
+ /**
+ * @task edgelogic
+ */
+ private function buildEdgeLogicTableAliasAncestor($alias) {
+ return $alias.'_ancestor';
+ }
+
/**
* Select certain edge logic constraint values.
@@ -1781,7 +1849,10 @@
}
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 @@
PhabricatorQueryConstraint::OPERATOR_AND,
PhabricatorQueryConstraint::OPERATOR_OR,
PhabricatorQueryConstraint::OPERATOR_NOT,
+ PhabricatorQueryConstraint::OPERATOR_ANCESTOR,
));
if ($project_phids) {
$projects = id(new PhabricatorProjectQuery())

File Metadata

Mime Type
text/plain
Expires
Mar 21 2025, 3:59 AM (7 w, 8 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7387914
Default Alt Text
D14910.diff (16 KB)

Event Timeline

Phabricator · Built by Phacility