Page MenuHomePhabricator

D12526.diff
No OneTemporary

D12526.diff

diff --git a/src/applications/home/controller/PhabricatorHomeMainController.php b/src/applications/home/controller/PhabricatorHomeMainController.php
--- a/src/applications/home/controller/PhabricatorHomeMainController.php
+++ b/src/applications/home/controller/PhabricatorHomeMainController.php
@@ -179,7 +179,10 @@
->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();
diff --git a/src/applications/maniphest/conduit/ManiphestQueryConduitAPIMethod.php b/src/applications/maniphest/conduit/ManiphestQueryConduitAPIMethod.php
--- a/src/applications/maniphest/conduit/ManiphestQueryConduitAPIMethod.php
+++ b/src/applications/maniphest/conduit/ManiphestQueryConduitAPIMethod.php
@@ -79,7 +79,10 @@
$projects = $request->getValue('projectPHIDs');
if ($projects) {
- $query->withAllProjects($projects);
+ $query->withEdgeLogicPHIDs(
+ PhabricatorProjectObjectHasProjectEdgeType::EDGECONST,
+ PhabricatorQueryConstraint::OPERATOR_AND,
+ $projects);
}
$ccs = $request->getValue('ccPHIDs');
diff --git a/src/applications/maniphest/controller/ManiphestReportController.php b/src/applications/maniphest/controller/ManiphestReportController.php
--- a/src/applications/maniphest/controller/ManiphestReportController.php
+++ b/src/applications/maniphest/controller/ManiphestReportController.php
@@ -411,7 +411,10 @@
$handles = $this->loadViewerHandles($phids);
$project_handle = $handles[$project_phid];
- $query->withAnyProjects($phids);
+ $query->withEdgeLogicPHIDs(
+ PhabricatorProjectObjectHasProjectEdgeType::EDGECONST,
+ PhabricatorQueryConstraint::OPERATOR_OR,
+ $phids);
}
$tasks = $query->execute();
diff --git a/src/applications/maniphest/query/ManiphestTaskQuery.php b/src/applications/maniphest/query/ManiphestTaskQuery.php
--- a/src/applications/maniphest/query/ManiphestTaskQuery.php
+++ b/src/applications/maniphest/query/ManiphestTaskQuery.php
@@ -11,12 +11,7 @@
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 @@
private $needProjectPHIDs;
private $blockingTasks;
private $blockedTasks;
- private $projectPolicyCheckFailed = false;
public function withAuthors(array $authors) {
$this->authorPHIDs = $authors;
@@ -89,39 +83,6 @@
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 @@
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 @@
}
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 @@
}
protected function loadPage() {
-
- if ($this->projectPolicyCheckFailed) {
- throw new PhabricatorEmptyQueryException();
- }
-
$task_dao = new ManiphestTask();
$conn = $task_dao->establishConnection('r');
@@ -341,10 +266,6 @@
$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 @@
$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 @@
$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 @@
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 @@
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 @@
}
protected function buildGroupClause(AphrontDatabaseConnection $conn_r) {
- $joined_multiple_rows = (count($this->projectPHIDs) > 1) ||
- (count($this->anyProjectPHIDs) > 1) ||
- $this->shouldJoinBlockingTasks() ||
+ $joined_multiple_rows = $this->shouldJoinBlockingTasks() ||
$this->shouldJoinBlockedTasks() ||
($this->shouldGroupQueryResultRows());
@@ -894,35 +714,34 @@
* construction.
*/
private function getIgnoreGroupedProjectPHIDs() {
+ // Maybe we should also exclude the "OPERATOR_NOT" PHIDs? 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.
+
+ $edge_types = array(
+ PhabricatorProjectObjectHasProjectEdgeType::EDGECONST,
+ );
+
$phids = array();
- if ($this->projectPHIDs) {
- $phids[] = $this->projectPHIDs;
- }
+ $phids[] = $this->getEdgeLogicValues(
+ $edge_types,
+ array(
+ PhabricatorQueryConstraint::OPERATOR_AND,
+ ));
- if (count($this->anyProjectPHIDs) == 1) {
- $phids[] = $this->anyProjectPHIDs;
+ $any = $this->getEdgeLogicValues(
+ $edge_types,
+ array(
+ PhabricatorQueryConstraint::OPERATOR_OR,
+ ));
+ if (count($any) == 1) {
+ $phids[] = $any;
}
- // 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);
}
- // TODO: Remove this when moving fully to edge logic.
- 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) {
$id = $result->getID();
diff --git a/src/applications/project/controller/PhabricatorProjectBoardViewController.php b/src/applications/project/controller/PhabricatorProjectBoardViewController.php
--- a/src/applications/project/controller/PhabricatorProjectBoardViewController.php
+++ b/src/applications/project/controller/PhabricatorProjectBoardViewController.php
@@ -56,7 +56,6 @@
$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 @@
$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();
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
@@ -22,6 +22,7 @@
private $orderVector;
private $builtinOrder;
private $edgeLogicConstraints = array();
+ private $edgeLogicConstraintsAreValid = false;
protected function getPageCursors(array $page) {
return array(
@@ -1261,6 +1262,26 @@
/**
+ * 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
*/
public function withEdgeLogicConstraints($edge_type, array $constraints) {
@@ -1274,6 +1295,8 @@
}
}
+ $this->edgeLogicConstraintsAreValid = false;
+
return $this;
}
@@ -1284,6 +1307,8 @@
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 @@
}
+ /**
+ * 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;
+ }
+
+
}

File Metadata

Mime Type
text/plain
Expires
Sat, Nov 23, 1:49 AM (17 h, 8 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6773862
Default Alt Text
D12526.diff (16 KB)

Event Timeline