Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F14078036
D12526.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
16 KB
Referenced Files
None
Subscribers
None
D12526.diff
View Options
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
Details
Attached
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)
Attached To
Mode
D12526: Move ManiphestTaskQuery to EdgeLogic
Attached
Detach File
Event Timeline
Log In to Comment