D12526.id.diff
No OneTemporary
Actions

Size

16 KB

Referenced Files

None

Subscribers

None

D12526.id.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

Mime Type: text/plain
Expires: Sat, May 11, 9:34 AM (1 w, 5 d ago)
Storage Engine: blob
Storage Format: Encrypted (AES-256-CBC)
Storage Handle: 6281404
Default Alt Text: D12526.id.diff (16 KB)

D12526.id.diffNo OneTemporaryActions

D12526.id.diffView Options

File Metadata

Event Timeline

D12526.id.diff
No OneTemporary
Actions

D12526.id.diff
View Options