2011-10-02 12:03:16 -07:00
|
|
|
<?php
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
|
* @task config Query Configuration
|
|
|
|
|
* @task exec Query Execution
|
|
|
|
|
* @task internal Internals
|
|
|
|
|
*/
|
2013-07-03 05:43:52 -07:00
|
|
|
final class DifferentialRevisionQuery
|
|
|
|
|
extends PhabricatorCursorPagedPolicyAwareQuery {
|
2011-10-02 12:03:16 -07:00
|
|
|
|
|
|
|
|
private $pathIDs = array();
|
|
|
|
|
|
2011-12-06 14:21:36 -08:00
|
|
|
private $authors = array();
|
2012-03-19 18:35:32 -07:00
|
|
|
private $draftAuthors = array();
|
2011-12-01 16:57:06 -08:00
|
|
|
private $ccs = array();
|
|
|
|
|
private $reviewers = array();
|
|
|
|
|
private $revIDs = array();
|
2012-01-03 21:08:12 -08:00
|
|
|
private $commitHashes = array();
|
2011-12-01 16:57:06 -08:00
|
|
|
private $phids = array();
|
2011-12-07 08:30:49 -08:00
|
|
|
private $responsibles = array();
|
2012-01-24 08:31:45 -08:00
|
|
|
private $branches = array();
|
2013-09-26 14:17:26 -07:00
|
|
|
private $repositoryPHIDs;
|
2015-03-25 10:21:56 -07:00
|
|
|
private $updatedEpochMin;
|
|
|
|
|
private $updatedEpochMax;
|
2017-08-11 08:10:28 -07:00
|
|
|
private $statuses;
|
2017-08-11 09:18:47 -07:00
|
|
|
private $isOpen;
|
2018-10-05 11:07:28 -07:00
|
|
|
private $createdEpochMin;
|
|
|
|
|
private $createdEpochMax;
|
2019-05-21 07:22:38 -07:00
|
|
|
private $noReviewers;
|
2011-12-01 16:57:06 -08:00
|
|
|
|
2011-10-02 12:03:16 -07:00
|
|
|
const ORDER_MODIFIED = 'order-modified';
|
|
|
|
|
const ORDER_CREATED = 'order-created';
|
|
|
|
|
|
2011-12-20 17:05:52 -08:00
|
|
|
private $needActiveDiffs = false;
|
|
|
|
|
private $needDiffIDs = false;
|
|
|
|
|
private $needCommitPHIDs = false;
|
2012-06-26 09:07:52 -07:00
|
|
|
private $needHashes = false;
|
2017-03-20 14:37:24 -07:00
|
|
|
private $needReviewers = false;
|
2013-10-06 17:08:14 -07:00
|
|
|
private $needReviewerAuthority;
|
2014-02-18 17:57:45 -08:00
|
|
|
private $needDrafts;
|
|
|
|
|
private $needFlags;
|
2011-10-02 12:03:16 -07:00
|
|
|
|
|
|
|
|
|
|
|
|
|
/* -( Query Configuration )------------------------------------------------ */
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
|
* Filter results to revisions which affect a Diffusion path ID in a given
|
|
|
|
|
* repository. You can call this multiple times to select revisions for
|
|
|
|
|
* several paths.
|
|
|
|
|
*
|
|
|
|
|
* @param int Diffusion repository ID.
|
|
|
|
|
* @param int Diffusion path ID.
|
|
|
|
|
* @return this
|
|
|
|
|
* @task config
|
|
|
|
|
*/
|
|
|
|
|
public function withPath($repository_id, $path_id) {
|
|
|
|
|
$this->pathIDs[] = array(
|
|
|
|
|
'repositoryID' => $repository_id,
|
|
|
|
|
'pathID' => $path_id,
|
|
|
|
|
);
|
|
|
|
|
return $this;
|
|
|
|
|
}
|
|
|
|
|
|
2011-12-01 16:57:06 -08:00
|
|
|
/**
|
2012-01-03 21:08:12 -08:00
|
|
|
* Filter results to revisions authored by one of the given PHIDs. Calling
|
|
|
|
|
* this function will clear anything set by previous calls to
|
|
|
|
|
* @{method:withAuthors}.
|
2011-12-06 14:21:36 -08:00
|
|
|
*
|
|
|
|
|
* @param array List of PHIDs of authors
|
|
|
|
|
* @return this
|
|
|
|
|
* @task config
|
|
|
|
|
*/
|
2011-12-07 08:30:49 -08:00
|
|
|
public function withAuthors(array $author_phids) {
|
|
|
|
|
$this->authors = $author_phids;
|
2011-12-06 14:21:36 -08:00
|
|
|
return $this;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
|
* Filter results to revisions which CC one of the listed people. Calling this
|
2011-12-07 08:30:49 -08:00
|
|
|
* function will clear anything set by previous calls to @{method:withCCs}.
|
2011-12-01 16:57:06 -08:00
|
|
|
*
|
2014-02-12 08:53:40 -08:00
|
|
|
* @param array List of PHIDs of subscribers.
|
2011-12-01 16:57:06 -08:00
|
|
|
* @return this
|
|
|
|
|
* @task config
|
|
|
|
|
*/
|
|
|
|
|
public function withCCs(array $cc_phids) {
|
|
|
|
|
$this->ccs = $cc_phids;
|
|
|
|
|
return $this;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
/**
|
2011-12-06 14:21:36 -08:00
|
|
|
* Filter results to revisions that have one of the provided PHIDs as
|
2011-12-01 16:57:06 -08:00
|
|
|
* reviewers. Calling this function will clear anything set by previous calls
|
2011-12-07 08:30:49 -08:00
|
|
|
* to @{method:withReviewers}.
|
2011-12-01 16:57:06 -08:00
|
|
|
*
|
|
|
|
|
* @param array List of PHIDs of reviewers
|
|
|
|
|
* @return this
|
|
|
|
|
* @task config
|
|
|
|
|
*/
|
|
|
|
|
public function withReviewers(array $reviewer_phids) {
|
2019-05-21 07:22:38 -07:00
|
|
|
if ($reviewer_phids === array()) {
|
|
|
|
|
throw new Exception(
|
|
|
|
|
pht(
|
|
|
|
|
'Empty "withReviewers()" constraint is invalid. Provide one or '.
|
|
|
|
|
'more values, or remove the constraint.'));
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
$with_none = false;
|
|
|
|
|
|
|
|
|
|
foreach ($reviewer_phids as $key => $phid) {
|
|
|
|
|
switch ($phid) {
|
|
|
|
|
case DifferentialNoReviewersDatasource::FUNCTION_TOKEN:
|
|
|
|
|
$with_none = true;
|
|
|
|
|
unset($reviewer_phids[$key]);
|
|
|
|
|
break;
|
|
|
|
|
default:
|
|
|
|
|
break;
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
$this->noReviewers = $with_none;
|
|
|
|
|
if ($reviewer_phids) {
|
|
|
|
|
$this->reviewers = array_values($reviewer_phids);
|
|
|
|
|
}
|
|
|
|
|
|
2011-12-01 16:57:06 -08:00
|
|
|
return $this;
|
|
|
|
|
}
|
|
|
|
|
|
2012-01-03 21:08:12 -08:00
|
|
|
/**
|
|
|
|
|
* Filter results to revisions that have one of the provided commit hashes.
|
|
|
|
|
* Calling this function will clear anything set by previous calls to
|
|
|
|
|
* @{method:withCommitHashes}.
|
|
|
|
|
*
|
2012-01-10 11:39:11 -08:00
|
|
|
* @param array List of pairs <Class
|
|
|
|
|
* ArcanistDifferentialRevisionHash::HASH_$type constant,
|
|
|
|
|
* hash>
|
2012-01-03 21:08:12 -08:00
|
|
|
* @return this
|
|
|
|
|
* @task config
|
|
|
|
|
*/
|
|
|
|
|
public function withCommitHashes(array $commit_hashes) {
|
|
|
|
|
$this->commitHashes = $commit_hashes;
|
|
|
|
|
return $this;
|
|
|
|
|
}
|
2011-10-02 12:03:16 -07:00
|
|
|
|
2017-08-11 08:10:28 -07:00
|
|
|
public function withStatuses(array $statuses) {
|
|
|
|
|
$this->statuses = $statuses;
|
|
|
|
|
return $this;
|
|
|
|
|
}
|
|
|
|
|
|
2017-08-11 09:18:47 -07:00
|
|
|
public function withIsOpen($is_open) {
|
|
|
|
|
$this->isOpen = $is_open;
|
|
|
|
|
return $this;
|
|
|
|
|
}
|
|
|
|
|
|
2011-10-02 12:03:16 -07:00
|
|
|
|
2012-01-24 08:31:45 -08:00
|
|
|
/**
|
|
|
|
|
* Filter results to revisions on given branches.
|
|
|
|
|
*
|
|
|
|
|
* @param list List of branch names.
|
|
|
|
|
* @return this
|
|
|
|
|
* @task config
|
|
|
|
|
*/
|
|
|
|
|
public function withBranches(array $branches) {
|
|
|
|
|
$this->branches = $branches;
|
|
|
|
|
return $this;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
2011-12-01 16:57:06 -08:00
|
|
|
/**
|
|
|
|
|
* Filter results to only return revisions whose ids are in the given set.
|
|
|
|
|
*
|
|
|
|
|
* @param array List of revision ids
|
|
|
|
|
* @return this
|
|
|
|
|
* @task config
|
|
|
|
|
*/
|
|
|
|
|
public function withIDs(array $ids) {
|
|
|
|
|
$this->revIDs = $ids;
|
|
|
|
|
return $this;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
|
* Filter results to only return revisions whose PHIDs are in the given set.
|
|
|
|
|
*
|
|
|
|
|
* @param array List of revision PHIDs
|
|
|
|
|
* @return this
|
|
|
|
|
* @task config
|
|
|
|
|
*/
|
|
|
|
|
public function withPHIDs(array $phids) {
|
|
|
|
|
$this->phids = $phids;
|
|
|
|
|
return $this;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
2011-12-07 08:30:49 -08:00
|
|
|
/**
|
|
|
|
|
* Given a set of users, filter results to return only revisions they are
|
|
|
|
|
* responsible for (i.e., they are either authors or reviewers).
|
|
|
|
|
*
|
|
|
|
|
* @param array List of user PHIDs.
|
|
|
|
|
* @return this
|
|
|
|
|
* @task config
|
|
|
|
|
*/
|
|
|
|
|
public function withResponsibleUsers(array $responsible_phids) {
|
|
|
|
|
$this->responsibles = $responsible_phids;
|
|
|
|
|
return $this;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
2013-09-26 14:17:26 -07:00
|
|
|
public function withRepositoryPHIDs(array $repository_phids) {
|
|
|
|
|
$this->repositoryPHIDs = $repository_phids;
|
|
|
|
|
return $this;
|
|
|
|
|
}
|
|
|
|
|
|
2015-03-25 10:21:56 -07:00
|
|
|
public function withUpdatedEpochBetween($min, $max) {
|
|
|
|
|
$this->updatedEpochMin = $min;
|
|
|
|
|
$this->updatedEpochMax = $max;
|
|
|
|
|
return $this;
|
|
|
|
|
}
|
|
|
|
|
|
2018-10-05 11:07:28 -07:00
|
|
|
public function withCreatedEpochBetween($min, $max) {
|
|
|
|
|
$this->createdEpochMin = $min;
|
|
|
|
|
$this->createdEpochMax = $max;
|
|
|
|
|
return $this;
|
|
|
|
|
}
|
|
|
|
|
|
2012-04-10 12:51:34 -07:00
|
|
|
|
2011-12-20 17:05:52 -08:00
|
|
|
/**
|
|
|
|
|
* Set whether or not the query should load the active diff for each
|
|
|
|
|
* revision.
|
|
|
|
|
*
|
|
|
|
|
* @param bool True to load and attach diffs.
|
|
|
|
|
* @return this
|
|
|
|
|
* @task config
|
|
|
|
|
*/
|
|
|
|
|
public function needActiveDiffs($need_active_diffs) {
|
|
|
|
|
$this->needActiveDiffs = $need_active_diffs;
|
|
|
|
|
return $this;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
|
* Set whether or not the query should load the associated commit PHIDs for
|
|
|
|
|
* each revision.
|
|
|
|
|
*
|
|
|
|
|
* @param bool True to load and attach diffs.
|
|
|
|
|
* @return this
|
|
|
|
|
* @task config
|
|
|
|
|
*/
|
|
|
|
|
public function needCommitPHIDs($need_commit_phids) {
|
|
|
|
|
$this->needCommitPHIDs = $need_commit_phids;
|
|
|
|
|
return $this;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
|
* Set whether or not the query should load associated diff IDs for each
|
|
|
|
|
* revision.
|
|
|
|
|
*
|
|
|
|
|
* @param bool True to load and attach diff IDs.
|
|
|
|
|
* @return this
|
|
|
|
|
* @task config
|
|
|
|
|
*/
|
|
|
|
|
public function needDiffIDs($need_diff_ids) {
|
|
|
|
|
$this->needDiffIDs = $need_diff_ids;
|
|
|
|
|
return $this;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
2012-06-26 09:07:52 -07:00
|
|
|
/**
|
|
|
|
|
* Set whether or not the query should load associated commit hashes for each
|
|
|
|
|
* revision.
|
|
|
|
|
*
|
|
|
|
|
* @param bool True to load and attach commit hashes.
|
|
|
|
|
* @return this
|
|
|
|
|
* @task config
|
|
|
|
|
*/
|
|
|
|
|
public function needHashes($need_hashes) {
|
|
|
|
|
$this->needHashes = $need_hashes;
|
|
|
|
|
return $this;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
2013-07-14 19:18:55 -07:00
|
|
|
/**
|
2017-03-20 14:37:24 -07:00
|
|
|
* Set whether or not the query should load associated reviewers.
|
2013-07-14 19:18:55 -07:00
|
|
|
*
|
|
|
|
|
* @param bool True to load and attach reviewers.
|
|
|
|
|
* @return this
|
|
|
|
|
* @task config
|
|
|
|
|
*/
|
2017-03-20 14:37:24 -07:00
|
|
|
public function needReviewers($need_reviewers) {
|
|
|
|
|
$this->needReviewers = $need_reviewers;
|
2013-07-14 19:18:55 -07:00
|
|
|
return $this;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
2013-10-06 17:08:14 -07:00
|
|
|
/**
|
|
|
|
|
* Request information about the viewer's authority to act on behalf of each
|
|
|
|
|
* reviewer. In particular, they have authority to act on behalf of projects
|
|
|
|
|
* they are a member of.
|
|
|
|
|
*
|
|
|
|
|
* @param bool True to load and attach authority.
|
|
|
|
|
* @return this
|
|
|
|
|
* @task config
|
|
|
|
|
*/
|
|
|
|
|
public function needReviewerAuthority($need_reviewer_authority) {
|
|
|
|
|
$this->needReviewerAuthority = $need_reviewer_authority;
|
|
|
|
|
return $this;
|
|
|
|
|
}
|
|
|
|
|
|
2014-02-18 17:57:45 -08:00
|
|
|
public function needFlags($need_flags) {
|
|
|
|
|
$this->needFlags = $need_flags;
|
|
|
|
|
return $this;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
public function needDrafts($need_drafts) {
|
|
|
|
|
$this->needDrafts = $need_drafts;
|
|
|
|
|
return $this;
|
|
|
|
|
}
|
|
|
|
|
|
2013-10-06 17:08:14 -07:00
|
|
|
|
2011-10-02 12:03:16 -07:00
|
|
|
/* -( Query Execution )---------------------------------------------------- */
|
|
|
|
|
|
|
|
|
|
|
Modernize DifferentialRevisionSearchEngine
Summary:
Ref T10939. Ref T4144. This moves the revision SearchEngine to modern code so I can add some kind of bucketing layer on top of it.
This seems to have worked pretty cleanly. One thing is that I removed the ability to search for "pending drafts":
- This was added in D1927 from a bootcamp task, was an indirect solution to a questionable problem, and almost certainly would not meet the bar today.
- Later, in D3324, we added the icons to the list. I think this is a better solution in general. In particular, it specifically addressed the query being kind of junky.
- At the time, Differential had a prebuilt "Drafts" filter. This was removed in D6347 with the move to ApplicationSearch, which simplified the large number of prebuilt filters. Although we got a lot of feedback about that, none requested that the drafts filter be restored.
Test Plan: Searched for responsible users, subscribers, orders, projects, repositories.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4144, T10939
Differential Revision: https://secure.phabricator.com/D15921
2016-05-15 10:07:58 -07:00
|
|
|
public function newResultObject() {
|
|
|
|
|
return new DifferentialRevision();
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
2011-10-02 12:03:16 -07:00
|
|
|
/**
|
|
|
|
|
* Execute the query as configured, returning matching
|
|
|
|
|
* @{class:DifferentialRevision} objects.
|
|
|
|
|
*
|
|
|
|
|
* @return list List of matching DifferentialRevision objects.
|
|
|
|
|
* @task exec
|
|
|
|
|
*/
|
2015-01-14 06:56:07 +11:00
|
|
|
protected function loadPage() {
|
Simplify construction and execution of Differential queries for "responsible" users
Summary:
Currently, when querying for responsible users (revisions where a given user is either the author or a reviewer), we do this:
- If the query passes a bunch of hard-coded special cases, use a special hard-coded UNION.
- Otherwise, use a very complicated JOIN/WHERE clause.
This is bad for several reasons:
- Tons and tons of hard-coding and special casing.
- The JOIN/WHERE clause performs very poorly for large datasets.
- (As a material consequence, the homepage issues a responsible query which barely misses the hard-coded special cases and goes down the slow path.)
Instead, //always// use the UNION strategy to execute a "responsible" query. Specifically, if we have responsible PHIDs, temporarily add them to the author list and build a normal query, then repeat for reviewers, then UNION any clauses we built.
Fixes T3377. Ref T603. Ref T2625. Depends on D6342.
There's various folklore about UNION ALL / UNION DISTINCT performance. UNION DISTINCT is simpler here and the number of rows is small, although we could use UNION ALL in the form:
SELECT * FROM ((SELECT ...) UNION ALL (SELECT ...) ORDER) GROUP LIMIT
...if we find that there's some performance benefit at some point.
Test Plan: Used DarkConsole to examine queries. Viewed home page and Differential dashboard/authors/subscribers.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603, T2625, T3377
Differential Revision: https://secure.phabricator.com/D6343
2013-07-03 05:39:09 -07:00
|
|
|
$data = $this->loadData();
|
2017-09-07 09:09:39 -07:00
|
|
|
$data = $this->didLoadRawRows($data);
|
Modernize DifferentialRevisionSearchEngine
Summary:
Ref T10939. Ref T4144. This moves the revision SearchEngine to modern code so I can add some kind of bucketing layer on top of it.
This seems to have worked pretty cleanly. One thing is that I removed the ability to search for "pending drafts":
- This was added in D1927 from a bootcamp task, was an indirect solution to a questionable problem, and almost certainly would not meet the bar today.
- Later, in D3324, we added the icons to the list. I think this is a better solution in general. In particular, it specifically addressed the query being kind of junky.
- At the time, Differential had a prebuilt "Drafts" filter. This was removed in D6347 with the move to ApplicationSearch, which simplified the large number of prebuilt filters. Although we got a lot of feedback about that, none requested that the drafts filter be restored.
Test Plan: Searched for responsible users, subscribers, orders, projects, repositories.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4144, T10939
Differential Revision: https://secure.phabricator.com/D15921
2016-05-15 10:07:58 -07:00
|
|
|
$table = $this->newResultObject();
|
2013-07-03 05:43:52 -07:00
|
|
|
return $table->loadAllFromArray($data);
|
|
|
|
|
}
|
2011-10-02 12:03:16 -07:00
|
|
|
|
2015-01-14 06:56:07 +11:00
|
|
|
protected function willFilterPage(array $revisions) {
|
2013-09-26 12:36:45 -07:00
|
|
|
$viewer = $this->getViewer();
|
|
|
|
|
|
|
|
|
|
$repository_phids = mpull($revisions, 'getRepositoryPHID');
|
|
|
|
|
$repository_phids = array_filter($repository_phids);
|
|
|
|
|
|
|
|
|
|
$repositories = array();
|
|
|
|
|
if ($repository_phids) {
|
|
|
|
|
$repositories = id(new PhabricatorRepositoryQuery())
|
|
|
|
|
->setViewer($this->getViewer())
|
|
|
|
|
->withPHIDs($repository_phids)
|
|
|
|
|
->execute();
|
|
|
|
|
$repositories = mpull($repositories, null, 'getPHID');
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
// If a revision is associated with a repository:
|
|
|
|
|
//
|
|
|
|
|
// - the viewer must be able to see the repository; or
|
|
|
|
|
// - the viewer must have an automatic view capability.
|
|
|
|
|
//
|
|
|
|
|
// In the latter case, we'll load the revision but not load the repository.
|
|
|
|
|
|
|
|
|
|
$can_view = PhabricatorPolicyCapability::CAN_VIEW;
|
|
|
|
|
foreach ($revisions as $key => $revision) {
|
|
|
|
|
$repo_phid = $revision->getRepositoryPHID();
|
|
|
|
|
if (!$repo_phid) {
|
|
|
|
|
// The revision has no associated repository. Attach `null` and move on.
|
|
|
|
|
$revision->attachRepository(null);
|
|
|
|
|
continue;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
$repository = idx($repositories, $repo_phid);
|
|
|
|
|
if ($repository) {
|
|
|
|
|
// The revision has an associated repository, and the viewer can see
|
|
|
|
|
// it. Attach it and move on.
|
|
|
|
|
$revision->attachRepository($repository);
|
|
|
|
|
continue;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if ($revision->hasAutomaticCapability($can_view, $viewer)) {
|
|
|
|
|
// The revision has an associated repository which the viewer can not
|
|
|
|
|
// see, but the viewer has an automatic capability on this revision.
|
|
|
|
|
// Load the revision without attaching a repository.
|
|
|
|
|
$revision->attachRepository(null);
|
|
|
|
|
continue;
|
|
|
|
|
}
|
|
|
|
|
|
2013-11-19 14:10:38 -08:00
|
|
|
if ($this->getViewer()->isOmnipotent()) {
|
|
|
|
|
// The viewer is omnipotent. Allow the revision to load even without
|
|
|
|
|
// a repository.
|
|
|
|
|
$revision->attachRepository(null);
|
|
|
|
|
continue;
|
|
|
|
|
}
|
|
|
|
|
|
2013-09-26 12:36:45 -07:00
|
|
|
// The revision has an associated repository, and the viewer can't see
|
|
|
|
|
// it, and the viewer has no special capabilities. Filter out this
|
|
|
|
|
// revision.
|
2013-09-27 08:43:50 -07:00
|
|
|
$this->didRejectResult($revision);
|
2013-09-26 12:36:45 -07:00
|
|
|
unset($revisions[$key]);
|
|
|
|
|
}
|
|
|
|
|
|
2013-09-27 08:43:50 -07:00
|
|
|
if (!$revisions) {
|
|
|
|
|
return array();
|
|
|
|
|
}
|
2013-09-26 12:36:45 -07:00
|
|
|
|
2013-07-03 05:43:52 -07:00
|
|
|
$table = new DifferentialRevision();
|
|
|
|
|
$conn_r = $table->establishConnection('r');
|
2011-12-20 17:05:52 -08:00
|
|
|
|
2013-07-03 05:43:52 -07:00
|
|
|
if ($this->needCommitPHIDs) {
|
Align "RevisionQuery->needCommitPHIDs()" to use Edges, not the legacy table
Summary:
Depends on D20458. Ref T13276. Although I'm not thrilled about "needCommitPHIDs()", it has a few callers, including custom fields. Allow "need + attach + get" to survive for now since they're reasonably modern, at least.
However, use edges instead of "TABLE_COMMIT" and require `need...()` + `get...()`, removing the direct `load...()`.
Also remove `RevisionQuery->withCommitPHIDs(...)`, which has no callers.
Test Plan:
- Grepped for `loadCommitPHIDs` (only two hits, the private `RevisionQuery` method).
- Called "differential.getrevision", got commits.
- Viewed a revision, saw "Commits: ...".
- Grepped for `withCommitPHIDs()`, no callers on `RevisionQuery` (some other query classes have methods with this name).
- Called "differential.query", got commits.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13276
Differential Revision: https://secure.phabricator.com/D20459
2019-04-22 12:19:54 -07:00
|
|
|
$this->loadCommitPHIDs($revisions);
|
2013-07-03 05:43:52 -07:00
|
|
|
}
|
2011-12-20 17:05:52 -08:00
|
|
|
|
2013-07-03 05:43:52 -07:00
|
|
|
$need_active = $this->needActiveDiffs;
|
|
|
|
|
$need_ids = $need_active || $this->needDiffIDs;
|
2012-06-26 09:07:52 -07:00
|
|
|
|
2013-07-03 05:43:52 -07:00
|
|
|
if ($need_ids) {
|
|
|
|
|
$this->loadDiffIDs($conn_r, $revisions);
|
2011-10-02 12:03:16 -07:00
|
|
|
}
|
|
|
|
|
|
2013-07-03 05:43:52 -07:00
|
|
|
if ($need_active) {
|
|
|
|
|
$this->loadActiveDiffs($conn_r, $revisions);
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if ($this->needHashes) {
|
|
|
|
|
$this->loadHashes($conn_r, $revisions);
|
|
|
|
|
}
|
2011-10-02 12:03:16 -07:00
|
|
|
|
2017-03-20 14:37:24 -07:00
|
|
|
if ($this->needReviewers || $this->needReviewerAuthority) {
|
2013-07-14 19:18:55 -07:00
|
|
|
$this->loadReviewers($conn_r, $revisions);
|
|
|
|
|
}
|
|
|
|
|
|
2013-07-03 05:43:52 -07:00
|
|
|
return $revisions;
|
2013-07-01 12:38:42 -07:00
|
|
|
}
|
|
|
|
|
|
2014-02-18 17:57:45 -08:00
|
|
|
protected function didFilterPage(array $revisions) {
|
|
|
|
|
$viewer = $this->getViewer();
|
|
|
|
|
|
|
|
|
|
if ($this->needFlags) {
|
|
|
|
|
$flags = id(new PhabricatorFlagQuery())
|
|
|
|
|
->setViewer($viewer)
|
|
|
|
|
->withOwnerPHIDs(array($viewer->getPHID()))
|
|
|
|
|
->withObjectPHIDs(mpull($revisions, 'getPHID'))
|
|
|
|
|
->execute();
|
|
|
|
|
$flags = mpull($flags, null, 'getObjectPHID');
|
|
|
|
|
foreach ($revisions as $revision) {
|
|
|
|
|
$revision->attachFlag(
|
|
|
|
|
$viewer,
|
|
|
|
|
idx($flags, $revision->getPHID()));
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if ($this->needDrafts) {
|
2017-01-14 09:22:51 -08:00
|
|
|
PhabricatorDraftEngine::attachDrafts(
|
|
|
|
|
$viewer,
|
|
|
|
|
$revisions);
|
2014-02-18 17:57:45 -08:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
return $revisions;
|
|
|
|
|
}
|
|
|
|
|
|
Drive Differential landing page with DifferentialRevisionQuery, simplify UI
Summary:
- Use DifferentialRevisionQuery, not DifferentialRevisionListData, to select
revisions.
- Make UI simpler (I hope?) and more flexible, similar to Maniphest. It now
shows "Active", "Revisions", "Reviews" and "Subscribed" instead of a hodge-podge
of miscellaneous stuff. All now really has all revisions, not just open
revisions.
- Allow views to be filtered and sorted more flexibly.
- Allow anonymous users to use the per-user views, just don't default them
there.
NOTE: This might have performance implications! I need some help evaluating
them.
@nh / @jungejason / @aran, can one of you run some queries agianst FB's corpus?
The "active revisions" view is built much differently now. Before, we issued two
queries:
- SELECT (open revisions you authored that need revision) UNION ALL (open
revisions you are reviewing that need review)
- SELECT (open revisions you authored that need review) UNION ALL (open
revisions you are reviewing that need revision)
These two queries generate the "Action Required" and "Waiting on Others" views,
and are available in P247.
Now, we issue only one query:
- SELECT (open revisions you authored or are reviewing)
Then we divide them into the two tables in PHP. That query is available in P246.
On the secure.phabricator.com data, this new approach seems to be much better
(like, 10x better). But the secure.phabricator.com data isn't very large. Can
someone run it against Facebook's data (using a few heavy-hitting PHIDs, like
ola or something) to make sure it won't cause a regression?
In particular:
- Run the queries and make sure the new version doesn't take too long.
- Run the queries with EXPLAIN and give me the output maybe?
Test Plan:
- Looked at different filters.
- Changed "View User" PHID.
- Changed open/all.
- Changed sort order.
- Ran EXPLAIN / select against secure.phabricator.com corpus.
Reviewers: btrahan, nh, jungejason
Reviewed By: btrahan
CC: cpiro, aran, btrahan, epriestley, jungejason, nh
Maniphest Tasks: T586
Differential Revision: 1186
2011-12-07 15:35:10 -08:00
|
|
|
private function loadData() {
|
Modernize DifferentialRevisionSearchEngine
Summary:
Ref T10939. Ref T4144. This moves the revision SearchEngine to modern code so I can add some kind of bucketing layer on top of it.
This seems to have worked pretty cleanly. One thing is that I removed the ability to search for "pending drafts":
- This was added in D1927 from a bootcamp task, was an indirect solution to a questionable problem, and almost certainly would not meet the bar today.
- Later, in D3324, we added the icons to the list. I think this is a better solution in general. In particular, it specifically addressed the query being kind of junky.
- At the time, Differential had a prebuilt "Drafts" filter. This was removed in D6347 with the move to ApplicationSearch, which simplified the large number of prebuilt filters. Although we got a lot of feedback about that, none requested that the drafts filter be restored.
Test Plan: Searched for responsible users, subscribers, orders, projects, repositories.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4144, T10939
Differential Revision: https://secure.phabricator.com/D15921
2016-05-15 10:07:58 -07:00
|
|
|
$table = $this->newResultObject();
|
2018-11-07 03:57:28 -08:00
|
|
|
$conn = $table->establishConnection('r');
|
Drive Differential landing page with DifferentialRevisionQuery, simplify UI
Summary:
- Use DifferentialRevisionQuery, not DifferentialRevisionListData, to select
revisions.
- Make UI simpler (I hope?) and more flexible, similar to Maniphest. It now
shows "Active", "Revisions", "Reviews" and "Subscribed" instead of a hodge-podge
of miscellaneous stuff. All now really has all revisions, not just open
revisions.
- Allow views to be filtered and sorted more flexibly.
- Allow anonymous users to use the per-user views, just don't default them
there.
NOTE: This might have performance implications! I need some help evaluating
them.
@nh / @jungejason / @aran, can one of you run some queries agianst FB's corpus?
The "active revisions" view is built much differently now. Before, we issued two
queries:
- SELECT (open revisions you authored that need revision) UNION ALL (open
revisions you are reviewing that need review)
- SELECT (open revisions you authored that need review) UNION ALL (open
revisions you are reviewing that need revision)
These two queries generate the "Action Required" and "Waiting on Others" views,
and are available in P247.
Now, we issue only one query:
- SELECT (open revisions you authored or are reviewing)
Then we divide them into the two tables in PHP. That query is available in P246.
On the secure.phabricator.com data, this new approach seems to be much better
(like, 10x better). But the secure.phabricator.com data isn't very large. Can
someone run it against Facebook's data (using a few heavy-hitting PHIDs, like
ola or something) to make sure it won't cause a regression?
In particular:
- Run the queries and make sure the new version doesn't take too long.
- Run the queries with EXPLAIN and give me the output maybe?
Test Plan:
- Looked at different filters.
- Changed "View User" PHID.
- Changed open/all.
- Changed sort order.
- Ran EXPLAIN / select against secure.phabricator.com corpus.
Reviewers: btrahan, nh, jungejason
Reviewed By: btrahan
CC: cpiro, aran, btrahan, epriestley, jungejason, nh
Maniphest Tasks: T586
Differential Revision: 1186
2011-12-07 15:35:10 -08:00
|
|
|
|
Simplify construction and execution of Differential queries for "responsible" users
Summary:
Currently, when querying for responsible users (revisions where a given user is either the author or a reviewer), we do this:
- If the query passes a bunch of hard-coded special cases, use a special hard-coded UNION.
- Otherwise, use a very complicated JOIN/WHERE clause.
This is bad for several reasons:
- Tons and tons of hard-coding and special casing.
- The JOIN/WHERE clause performs very poorly for large datasets.
- (As a material consequence, the homepage issues a responsible query which barely misses the hard-coded special cases and goes down the slow path.)
Instead, //always// use the UNION strategy to execute a "responsible" query. Specifically, if we have responsible PHIDs, temporarily add them to the author list and build a normal query, then repeat for reviewers, then UNION any clauses we built.
Fixes T3377. Ref T603. Ref T2625. Depends on D6342.
There's various folklore about UNION ALL / UNION DISTINCT performance. UNION DISTINCT is simpler here and the number of rows is small, although we could use UNION ALL in the form:
SELECT * FROM ((SELECT ...) UNION ALL (SELECT ...) ORDER) GROUP LIMIT
...if we find that there's some performance benefit at some point.
Test Plan: Used DarkConsole to examine queries. Viewed home page and Differential dashboard/authors/subscribers.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603, T2625, T3377
Differential Revision: https://secure.phabricator.com/D6343
2013-07-03 05:39:09 -07:00
|
|
|
$selects = array();
|
|
|
|
|
|
|
|
|
|
// NOTE: If the query includes "responsiblePHIDs", we execute it as a
|
|
|
|
|
// UNION of revisions they own and revisions they're reviewing. This has
|
|
|
|
|
// much better performance than doing it with JOIN/WHERE.
|
|
|
|
|
if ($this->responsibles) {
|
|
|
|
|
$basic_authors = $this->authors;
|
|
|
|
|
$basic_reviewers = $this->reviewers;
|
2013-10-04 20:41:50 -07:00
|
|
|
|
Simplify construction and execution of Differential queries for "responsible" users
Summary:
Currently, when querying for responsible users (revisions where a given user is either the author or a reviewer), we do this:
- If the query passes a bunch of hard-coded special cases, use a special hard-coded UNION.
- Otherwise, use a very complicated JOIN/WHERE clause.
This is bad for several reasons:
- Tons and tons of hard-coding and special casing.
- The JOIN/WHERE clause performs very poorly for large datasets.
- (As a material consequence, the homepage issues a responsible query which barely misses the hard-coded special cases and goes down the slow path.)
Instead, //always// use the UNION strategy to execute a "responsible" query. Specifically, if we have responsible PHIDs, temporarily add them to the author list and build a normal query, then repeat for reviewers, then UNION any clauses we built.
Fixes T3377. Ref T603. Ref T2625. Depends on D6342.
There's various folklore about UNION ALL / UNION DISTINCT performance. UNION DISTINCT is simpler here and the number of rows is small, although we could use UNION ALL in the form:
SELECT * FROM ((SELECT ...) UNION ALL (SELECT ...) ORDER) GROUP LIMIT
...if we find that there's some performance benefit at some point.
Test Plan: Used DarkConsole to examine queries. Viewed home page and Differential dashboard/authors/subscribers.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603, T2625, T3377
Differential Revision: https://secure.phabricator.com/D6343
2013-07-03 05:39:09 -07:00
|
|
|
try {
|
|
|
|
|
// Build the query where the responsible users are authors.
|
|
|
|
|
$this->authors = array_merge($basic_authors, $this->responsibles);
|
Modernize "Responsible Users" tokenizer and add "exact(user)" token
Summary:
Ref T10939. Fixes T9263. Ref T4144.
First, this resolves users (converting users into all packages and projects they are responsible for) earlier, so bucketing can act on that data correctly. Previously, your own blocking reviews would appear in "Must Review" but your packages/projects' would not. Now, all of them will.
Second, this adds `exact(username)` to mean "just me, not my packages/projects". You can use this along with "Bucket: By Required Action" to create a personal view of "Active Revisions" if you'd like, and ignore all your project/package reviews.
Test Plan: Queried by "me" and "exact(me)", got reasonable looking results.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4144, T9263, T10939
Differential Revision: https://secure.phabricator.com/D15925
2016-05-16 08:31:31 -07:00
|
|
|
|
Simplify construction and execution of Differential queries for "responsible" users
Summary:
Currently, when querying for responsible users (revisions where a given user is either the author or a reviewer), we do this:
- If the query passes a bunch of hard-coded special cases, use a special hard-coded UNION.
- Otherwise, use a very complicated JOIN/WHERE clause.
This is bad for several reasons:
- Tons and tons of hard-coding and special casing.
- The JOIN/WHERE clause performs very poorly for large datasets.
- (As a material consequence, the homepage issues a responsible query which barely misses the hard-coded special cases and goes down the slow path.)
Instead, //always// use the UNION strategy to execute a "responsible" query. Specifically, if we have responsible PHIDs, temporarily add them to the author list and build a normal query, then repeat for reviewers, then UNION any clauses we built.
Fixes T3377. Ref T603. Ref T2625. Depends on D6342.
There's various folklore about UNION ALL / UNION DISTINCT performance. UNION DISTINCT is simpler here and the number of rows is small, although we could use UNION ALL in the form:
SELECT * FROM ((SELECT ...) UNION ALL (SELECT ...) ORDER) GROUP LIMIT
...if we find that there's some performance benefit at some point.
Test Plan: Used DarkConsole to examine queries. Viewed home page and Differential dashboard/authors/subscribers.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603, T2625, T3377
Differential Revision: https://secure.phabricator.com/D6343
2013-07-03 05:39:09 -07:00
|
|
|
$this->reviewers = $basic_reviewers;
|
2018-11-07 03:57:28 -08:00
|
|
|
$selects[] = $this->buildSelectStatement($conn);
|
Simplify construction and execution of Differential queries for "responsible" users
Summary:
Currently, when querying for responsible users (revisions where a given user is either the author or a reviewer), we do this:
- If the query passes a bunch of hard-coded special cases, use a special hard-coded UNION.
- Otherwise, use a very complicated JOIN/WHERE clause.
This is bad for several reasons:
- Tons and tons of hard-coding and special casing.
- The JOIN/WHERE clause performs very poorly for large datasets.
- (As a material consequence, the homepage issues a responsible query which barely misses the hard-coded special cases and goes down the slow path.)
Instead, //always// use the UNION strategy to execute a "responsible" query. Specifically, if we have responsible PHIDs, temporarily add them to the author list and build a normal query, then repeat for reviewers, then UNION any clauses we built.
Fixes T3377. Ref T603. Ref T2625. Depends on D6342.
There's various folklore about UNION ALL / UNION DISTINCT performance. UNION DISTINCT is simpler here and the number of rows is small, although we could use UNION ALL in the form:
SELECT * FROM ((SELECT ...) UNION ALL (SELECT ...) ORDER) GROUP LIMIT
...if we find that there's some performance benefit at some point.
Test Plan: Used DarkConsole to examine queries. Viewed home page and Differential dashboard/authors/subscribers.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603, T2625, T3377
Differential Revision: https://secure.phabricator.com/D6343
2013-07-03 05:39:09 -07:00
|
|
|
|
2013-10-04 20:41:50 -07:00
|
|
|
// Build the query where the responsible users are reviewers, or
|
|
|
|
|
// projects they are members of are reviewers.
|
Simplify construction and execution of Differential queries for "responsible" users
Summary:
Currently, when querying for responsible users (revisions where a given user is either the author or a reviewer), we do this:
- If the query passes a bunch of hard-coded special cases, use a special hard-coded UNION.
- Otherwise, use a very complicated JOIN/WHERE clause.
This is bad for several reasons:
- Tons and tons of hard-coding and special casing.
- The JOIN/WHERE clause performs very poorly for large datasets.
- (As a material consequence, the homepage issues a responsible query which barely misses the hard-coded special cases and goes down the slow path.)
Instead, //always// use the UNION strategy to execute a "responsible" query. Specifically, if we have responsible PHIDs, temporarily add them to the author list and build a normal query, then repeat for reviewers, then UNION any clauses we built.
Fixes T3377. Ref T603. Ref T2625. Depends on D6342.
There's various folklore about UNION ALL / UNION DISTINCT performance. UNION DISTINCT is simpler here and the number of rows is small, although we could use UNION ALL in the form:
SELECT * FROM ((SELECT ...) UNION ALL (SELECT ...) ORDER) GROUP LIMIT
...if we find that there's some performance benefit at some point.
Test Plan: Used DarkConsole to examine queries. Viewed home page and Differential dashboard/authors/subscribers.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603, T2625, T3377
Differential Revision: https://secure.phabricator.com/D6343
2013-07-03 05:39:09 -07:00
|
|
|
$this->authors = $basic_authors;
|
Modernize "Responsible Users" tokenizer and add "exact(user)" token
Summary:
Ref T10939. Fixes T9263. Ref T4144.
First, this resolves users (converting users into all packages and projects they are responsible for) earlier, so bucketing can act on that data correctly. Previously, your own blocking reviews would appear in "Must Review" but your packages/projects' would not. Now, all of them will.
Second, this adds `exact(username)` to mean "just me, not my packages/projects". You can use this along with "Bucket: By Required Action" to create a personal view of "Active Revisions" if you'd like, and ignore all your project/package reviews.
Test Plan: Queried by "me" and "exact(me)", got reasonable looking results.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4144, T9263, T10939
Differential Revision: https://secure.phabricator.com/D15925
2016-05-16 08:31:31 -07:00
|
|
|
$this->reviewers = array_merge($basic_reviewers, $this->responsibles);
|
2018-11-07 03:57:28 -08:00
|
|
|
$selects[] = $this->buildSelectStatement($conn);
|
Simplify construction and execution of Differential queries for "responsible" users
Summary:
Currently, when querying for responsible users (revisions where a given user is either the author or a reviewer), we do this:
- If the query passes a bunch of hard-coded special cases, use a special hard-coded UNION.
- Otherwise, use a very complicated JOIN/WHERE clause.
This is bad for several reasons:
- Tons and tons of hard-coding and special casing.
- The JOIN/WHERE clause performs very poorly for large datasets.
- (As a material consequence, the homepage issues a responsible query which barely misses the hard-coded special cases and goes down the slow path.)
Instead, //always// use the UNION strategy to execute a "responsible" query. Specifically, if we have responsible PHIDs, temporarily add them to the author list and build a normal query, then repeat for reviewers, then UNION any clauses we built.
Fixes T3377. Ref T603. Ref T2625. Depends on D6342.
There's various folklore about UNION ALL / UNION DISTINCT performance. UNION DISTINCT is simpler here and the number of rows is small, although we could use UNION ALL in the form:
SELECT * FROM ((SELECT ...) UNION ALL (SELECT ...) ORDER) GROUP LIMIT
...if we find that there's some performance benefit at some point.
Test Plan: Used DarkConsole to examine queries. Viewed home page and Differential dashboard/authors/subscribers.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603, T2625, T3377
Differential Revision: https://secure.phabricator.com/D6343
2013-07-03 05:39:09 -07:00
|
|
|
|
|
|
|
|
// Put everything back like it was.
|
|
|
|
|
$this->authors = $basic_authors;
|
|
|
|
|
$this->reviewers = $basic_reviewers;
|
|
|
|
|
} catch (Exception $ex) {
|
|
|
|
|
$this->authors = $basic_authors;
|
|
|
|
|
$this->reviewers = $basic_reviewers;
|
|
|
|
|
throw $ex;
|
|
|
|
|
}
|
|
|
|
|
} else {
|
2018-11-07 03:57:28 -08:00
|
|
|
$selects[] = $this->buildSelectStatement($conn);
|
Simplify construction and execution of Differential queries for "responsible" users
Summary:
Currently, when querying for responsible users (revisions where a given user is either the author or a reviewer), we do this:
- If the query passes a bunch of hard-coded special cases, use a special hard-coded UNION.
- Otherwise, use a very complicated JOIN/WHERE clause.
This is bad for several reasons:
- Tons and tons of hard-coding and special casing.
- The JOIN/WHERE clause performs very poorly for large datasets.
- (As a material consequence, the homepage issues a responsible query which barely misses the hard-coded special cases and goes down the slow path.)
Instead, //always// use the UNION strategy to execute a "responsible" query. Specifically, if we have responsible PHIDs, temporarily add them to the author list and build a normal query, then repeat for reviewers, then UNION any clauses we built.
Fixes T3377. Ref T603. Ref T2625. Depends on D6342.
There's various folklore about UNION ALL / UNION DISTINCT performance. UNION DISTINCT is simpler here and the number of rows is small, although we could use UNION ALL in the form:
SELECT * FROM ((SELECT ...) UNION ALL (SELECT ...) ORDER) GROUP LIMIT
...if we find that there's some performance benefit at some point.
Test Plan: Used DarkConsole to examine queries. Viewed home page and Differential dashboard/authors/subscribers.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603, T2625, T3377
Differential Revision: https://secure.phabricator.com/D6343
2013-07-03 05:39:09 -07:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if (count($selects) > 1) {
|
2018-11-07 03:57:28 -08:00
|
|
|
$unions = null;
|
|
|
|
|
foreach ($selects as $select) {
|
|
|
|
|
if (!$unions) {
|
|
|
|
|
$unions = $select;
|
|
|
|
|
continue;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
$unions = qsprintf(
|
|
|
|
|
$conn,
|
|
|
|
|
'%Q UNION DISTINCT %Q',
|
|
|
|
|
$unions,
|
|
|
|
|
$select);
|
|
|
|
|
}
|
|
|
|
|
|
Simplify construction and execution of Differential queries for "responsible" users
Summary:
Currently, when querying for responsible users (revisions where a given user is either the author or a reviewer), we do this:
- If the query passes a bunch of hard-coded special cases, use a special hard-coded UNION.
- Otherwise, use a very complicated JOIN/WHERE clause.
This is bad for several reasons:
- Tons and tons of hard-coding and special casing.
- The JOIN/WHERE clause performs very poorly for large datasets.
- (As a material consequence, the homepage issues a responsible query which barely misses the hard-coded special cases and goes down the slow path.)
Instead, //always// use the UNION strategy to execute a "responsible" query. Specifically, if we have responsible PHIDs, temporarily add them to the author list and build a normal query, then repeat for reviewers, then UNION any clauses we built.
Fixes T3377. Ref T603. Ref T2625. Depends on D6342.
There's various folklore about UNION ALL / UNION DISTINCT performance. UNION DISTINCT is simpler here and the number of rows is small, although we could use UNION ALL in the form:
SELECT * FROM ((SELECT ...) UNION ALL (SELECT ...) ORDER) GROUP LIMIT
...if we find that there's some performance benefit at some point.
Test Plan: Used DarkConsole to examine queries. Viewed home page and Differential dashboard/authors/subscribers.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603, T2625, T3377
Differential Revision: https://secure.phabricator.com/D6343
2013-07-03 05:39:09 -07:00
|
|
|
$query = qsprintf(
|
2018-11-07 03:57:28 -08:00
|
|
|
$conn,
|
Simplify construction and execution of Differential queries for "responsible" users
Summary:
Currently, when querying for responsible users (revisions where a given user is either the author or a reviewer), we do this:
- If the query passes a bunch of hard-coded special cases, use a special hard-coded UNION.
- Otherwise, use a very complicated JOIN/WHERE clause.
This is bad for several reasons:
- Tons and tons of hard-coding and special casing.
- The JOIN/WHERE clause performs very poorly for large datasets.
- (As a material consequence, the homepage issues a responsible query which barely misses the hard-coded special cases and goes down the slow path.)
Instead, //always// use the UNION strategy to execute a "responsible" query. Specifically, if we have responsible PHIDs, temporarily add them to the author list and build a normal query, then repeat for reviewers, then UNION any clauses we built.
Fixes T3377. Ref T603. Ref T2625. Depends on D6342.
There's various folklore about UNION ALL / UNION DISTINCT performance. UNION DISTINCT is simpler here and the number of rows is small, although we could use UNION ALL in the form:
SELECT * FROM ((SELECT ...) UNION ALL (SELECT ...) ORDER) GROUP LIMIT
...if we find that there's some performance benefit at some point.
Test Plan: Used DarkConsole to examine queries. Viewed home page and Differential dashboard/authors/subscribers.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603, T2625, T3377
Differential Revision: https://secure.phabricator.com/D6343
2013-07-03 05:39:09 -07:00
|
|
|
'%Q %Q %Q',
|
2018-11-07 03:57:28 -08:00
|
|
|
$unions,
|
|
|
|
|
$this->buildOrderClause($conn, true),
|
|
|
|
|
$this->buildLimitClause($conn));
|
Simplify construction and execution of Differential queries for "responsible" users
Summary:
Currently, when querying for responsible users (revisions where a given user is either the author or a reviewer), we do this:
- If the query passes a bunch of hard-coded special cases, use a special hard-coded UNION.
- Otherwise, use a very complicated JOIN/WHERE clause.
This is bad for several reasons:
- Tons and tons of hard-coding and special casing.
- The JOIN/WHERE clause performs very poorly for large datasets.
- (As a material consequence, the homepage issues a responsible query which barely misses the hard-coded special cases and goes down the slow path.)
Instead, //always// use the UNION strategy to execute a "responsible" query. Specifically, if we have responsible PHIDs, temporarily add them to the author list and build a normal query, then repeat for reviewers, then UNION any clauses we built.
Fixes T3377. Ref T603. Ref T2625. Depends on D6342.
There's various folklore about UNION ALL / UNION DISTINCT performance. UNION DISTINCT is simpler here and the number of rows is small, although we could use UNION ALL in the form:
SELECT * FROM ((SELECT ...) UNION ALL (SELECT ...) ORDER) GROUP LIMIT
...if we find that there's some performance benefit at some point.
Test Plan: Used DarkConsole to examine queries. Viewed home page and Differential dashboard/authors/subscribers.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603, T2625, T3377
Differential Revision: https://secure.phabricator.com/D6343
2013-07-03 05:39:09 -07:00
|
|
|
} else {
|
|
|
|
|
$query = head($selects);
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-07 03:57:28 -08:00
|
|
|
return queryfx_all($conn, '%Q', $query);
|
Simplify construction and execution of Differential queries for "responsible" users
Summary:
Currently, when querying for responsible users (revisions where a given user is either the author or a reviewer), we do this:
- If the query passes a bunch of hard-coded special cases, use a special hard-coded UNION.
- Otherwise, use a very complicated JOIN/WHERE clause.
This is bad for several reasons:
- Tons and tons of hard-coding and special casing.
- The JOIN/WHERE clause performs very poorly for large datasets.
- (As a material consequence, the homepage issues a responsible query which barely misses the hard-coded special cases and goes down the slow path.)
Instead, //always// use the UNION strategy to execute a "responsible" query. Specifically, if we have responsible PHIDs, temporarily add them to the author list and build a normal query, then repeat for reviewers, then UNION any clauses we built.
Fixes T3377. Ref T603. Ref T2625. Depends on D6342.
There's various folklore about UNION ALL / UNION DISTINCT performance. UNION DISTINCT is simpler here and the number of rows is small, although we could use UNION ALL in the form:
SELECT * FROM ((SELECT ...) UNION ALL (SELECT ...) ORDER) GROUP LIMIT
...if we find that there's some performance benefit at some point.
Test Plan: Used DarkConsole to examine queries. Viewed home page and Differential dashboard/authors/subscribers.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603, T2625, T3377
Differential Revision: https://secure.phabricator.com/D6343
2013-07-03 05:39:09 -07:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
private function buildSelectStatement(AphrontDatabaseConnection $conn_r) {
|
|
|
|
|
$table = new DifferentialRevision();
|
|
|
|
|
|
2015-04-18 11:18:27 -07:00
|
|
|
$select = $this->buildSelectClause($conn_r);
|
|
|
|
|
|
|
|
|
|
$from = qsprintf(
|
Drive Differential landing page with DifferentialRevisionQuery, simplify UI
Summary:
- Use DifferentialRevisionQuery, not DifferentialRevisionListData, to select
revisions.
- Make UI simpler (I hope?) and more flexible, similar to Maniphest. It now
shows "Active", "Revisions", "Reviews" and "Subscribed" instead of a hodge-podge
of miscellaneous stuff. All now really has all revisions, not just open
revisions.
- Allow views to be filtered and sorted more flexibly.
- Allow anonymous users to use the per-user views, just don't default them
there.
NOTE: This might have performance implications! I need some help evaluating
them.
@nh / @jungejason / @aran, can one of you run some queries agianst FB's corpus?
The "active revisions" view is built much differently now. Before, we issued two
queries:
- SELECT (open revisions you authored that need revision) UNION ALL (open
revisions you are reviewing that need review)
- SELECT (open revisions you authored that need review) UNION ALL (open
revisions you are reviewing that need revision)
These two queries generate the "Action Required" and "Waiting on Others" views,
and are available in P247.
Now, we issue only one query:
- SELECT (open revisions you authored or are reviewing)
Then we divide them into the two tables in PHP. That query is available in P246.
On the secure.phabricator.com data, this new approach seems to be much better
(like, 10x better). But the secure.phabricator.com data isn't very large. Can
someone run it against Facebook's data (using a few heavy-hitting PHIDs, like
ola or something) to make sure it won't cause a regression?
In particular:
- Run the queries and make sure the new version doesn't take too long.
- Run the queries with EXPLAIN and give me the output maybe?
Test Plan:
- Looked at different filters.
- Changed "View User" PHID.
- Changed open/all.
- Changed sort order.
- Ran EXPLAIN / select against secure.phabricator.com corpus.
Reviewers: btrahan, nh, jungejason
Reviewed By: btrahan
CC: cpiro, aran, btrahan, epriestley, jungejason, nh
Maniphest Tasks: T586
Differential Revision: 1186
2011-12-07 15:35:10 -08:00
|
|
|
$conn_r,
|
2015-04-18 11:18:27 -07:00
|
|
|
'FROM %T r',
|
Drive Differential landing page with DifferentialRevisionQuery, simplify UI
Summary:
- Use DifferentialRevisionQuery, not DifferentialRevisionListData, to select
revisions.
- Make UI simpler (I hope?) and more flexible, similar to Maniphest. It now
shows "Active", "Revisions", "Reviews" and "Subscribed" instead of a hodge-podge
of miscellaneous stuff. All now really has all revisions, not just open
revisions.
- Allow views to be filtered and sorted more flexibly.
- Allow anonymous users to use the per-user views, just don't default them
there.
NOTE: This might have performance implications! I need some help evaluating
them.
@nh / @jungejason / @aran, can one of you run some queries agianst FB's corpus?
The "active revisions" view is built much differently now. Before, we issued two
queries:
- SELECT (open revisions you authored that need revision) UNION ALL (open
revisions you are reviewing that need review)
- SELECT (open revisions you authored that need review) UNION ALL (open
revisions you are reviewing that need revision)
These two queries generate the "Action Required" and "Waiting on Others" views,
and are available in P247.
Now, we issue only one query:
- SELECT (open revisions you authored or are reviewing)
Then we divide them into the two tables in PHP. That query is available in P246.
On the secure.phabricator.com data, this new approach seems to be much better
(like, 10x better). But the secure.phabricator.com data isn't very large. Can
someone run it against Facebook's data (using a few heavy-hitting PHIDs, like
ola or something) to make sure it won't cause a regression?
In particular:
- Run the queries and make sure the new version doesn't take too long.
- Run the queries with EXPLAIN and give me the output maybe?
Test Plan:
- Looked at different filters.
- Changed "View User" PHID.
- Changed open/all.
- Changed sort order.
- Ran EXPLAIN / select against secure.phabricator.com corpus.
Reviewers: btrahan, nh, jungejason
Reviewed By: btrahan
CC: cpiro, aran, btrahan, epriestley, jungejason, nh
Maniphest Tasks: T586
Differential Revision: 1186
2011-12-07 15:35:10 -08:00
|
|
|
$table->getTableName());
|
|
|
|
|
|
|
|
|
|
$joins = $this->buildJoinsClause($conn_r);
|
|
|
|
|
$where = $this->buildWhereClause($conn_r);
|
Modernize DifferentialRevisionSearchEngine
Summary:
Ref T10939. Ref T4144. This moves the revision SearchEngine to modern code so I can add some kind of bucketing layer on top of it.
This seems to have worked pretty cleanly. One thing is that I removed the ability to search for "pending drafts":
- This was added in D1927 from a bootcamp task, was an indirect solution to a questionable problem, and almost certainly would not meet the bar today.
- Later, in D3324, we added the icons to the list. I think this is a better solution in general. In particular, it specifically addressed the query being kind of junky.
- At the time, Differential had a prebuilt "Drafts" filter. This was removed in D6347 with the move to ApplicationSearch, which simplified the large number of prebuilt filters. Although we got a lot of feedback about that, none requested that the drafts filter be restored.
Test Plan: Searched for responsible users, subscribers, orders, projects, repositories.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4144, T10939
Differential Revision: https://secure.phabricator.com/D15921
2016-05-15 10:07:58 -07:00
|
|
|
$group_by = $this->buildGroupClause($conn_r);
|
2015-04-18 11:18:27 -07:00
|
|
|
$having = $this->buildHavingClause($conn_r);
|
2013-07-03 05:45:07 -07:00
|
|
|
|
|
|
|
|
$order_by = $this->buildOrderClause($conn_r);
|
|
|
|
|
|
Simplify construction and execution of Differential queries for "responsible" users
Summary:
Currently, when querying for responsible users (revisions where a given user is either the author or a reviewer), we do this:
- If the query passes a bunch of hard-coded special cases, use a special hard-coded UNION.
- Otherwise, use a very complicated JOIN/WHERE clause.
This is bad for several reasons:
- Tons and tons of hard-coding and special casing.
- The JOIN/WHERE clause performs very poorly for large datasets.
- (As a material consequence, the homepage issues a responsible query which barely misses the hard-coded special cases and goes down the slow path.)
Instead, //always// use the UNION strategy to execute a "responsible" query. Specifically, if we have responsible PHIDs, temporarily add them to the author list and build a normal query, then repeat for reviewers, then UNION any clauses we built.
Fixes T3377. Ref T603. Ref T2625. Depends on D6342.
There's various folklore about UNION ALL / UNION DISTINCT performance. UNION DISTINCT is simpler here and the number of rows is small, although we could use UNION ALL in the form:
SELECT * FROM ((SELECT ...) UNION ALL (SELECT ...) ORDER) GROUP LIMIT
...if we find that there's some performance benefit at some point.
Test Plan: Used DarkConsole to examine queries. Viewed home page and Differential dashboard/authors/subscribers.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603, T2625, T3377
Differential Revision: https://secure.phabricator.com/D6343
2013-07-03 05:39:09 -07:00
|
|
|
$limit = $this->buildLimitClause($conn_r);
|
Drive Differential landing page with DifferentialRevisionQuery, simplify UI
Summary:
- Use DifferentialRevisionQuery, not DifferentialRevisionListData, to select
revisions.
- Make UI simpler (I hope?) and more flexible, similar to Maniphest. It now
shows "Active", "Revisions", "Reviews" and "Subscribed" instead of a hodge-podge
of miscellaneous stuff. All now really has all revisions, not just open
revisions.
- Allow views to be filtered and sorted more flexibly.
- Allow anonymous users to use the per-user views, just don't default them
there.
NOTE: This might have performance implications! I need some help evaluating
them.
@nh / @jungejason / @aran, can one of you run some queries agianst FB's corpus?
The "active revisions" view is built much differently now. Before, we issued two
queries:
- SELECT (open revisions you authored that need revision) UNION ALL (open
revisions you are reviewing that need review)
- SELECT (open revisions you authored that need review) UNION ALL (open
revisions you are reviewing that need revision)
These two queries generate the "Action Required" and "Waiting on Others" views,
and are available in P247.
Now, we issue only one query:
- SELECT (open revisions you authored or are reviewing)
Then we divide them into the two tables in PHP. That query is available in P246.
On the secure.phabricator.com data, this new approach seems to be much better
(like, 10x better). But the secure.phabricator.com data isn't very large. Can
someone run it against Facebook's data (using a few heavy-hitting PHIDs, like
ola or something) to make sure it won't cause a regression?
In particular:
- Run the queries and make sure the new version doesn't take too long.
- Run the queries with EXPLAIN and give me the output maybe?
Test Plan:
- Looked at different filters.
- Changed "View User" PHID.
- Changed open/all.
- Changed sort order.
- Ran EXPLAIN / select against secure.phabricator.com corpus.
Reviewers: btrahan, nh, jungejason
Reviewed By: btrahan
CC: cpiro, aran, btrahan, epriestley, jungejason, nh
Maniphest Tasks: T586
Differential Revision: 1186
2011-12-07 15:35:10 -08:00
|
|
|
|
Simplify construction and execution of Differential queries for "responsible" users
Summary:
Currently, when querying for responsible users (revisions where a given user is either the author or a reviewer), we do this:
- If the query passes a bunch of hard-coded special cases, use a special hard-coded UNION.
- Otherwise, use a very complicated JOIN/WHERE clause.
This is bad for several reasons:
- Tons and tons of hard-coding and special casing.
- The JOIN/WHERE clause performs very poorly for large datasets.
- (As a material consequence, the homepage issues a responsible query which barely misses the hard-coded special cases and goes down the slow path.)
Instead, //always// use the UNION strategy to execute a "responsible" query. Specifically, if we have responsible PHIDs, temporarily add them to the author list and build a normal query, then repeat for reviewers, then UNION any clauses we built.
Fixes T3377. Ref T603. Ref T2625. Depends on D6342.
There's various folklore about UNION ALL / UNION DISTINCT performance. UNION DISTINCT is simpler here and the number of rows is small, although we could use UNION ALL in the form:
SELECT * FROM ((SELECT ...) UNION ALL (SELECT ...) ORDER) GROUP LIMIT
...if we find that there's some performance benefit at some point.
Test Plan: Used DarkConsole to examine queries. Viewed home page and Differential dashboard/authors/subscribers.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603, T2625, T3377
Differential Revision: https://secure.phabricator.com/D6343
2013-07-03 05:39:09 -07:00
|
|
|
return qsprintf(
|
|
|
|
|
$conn_r,
|
2015-04-18 11:18:27 -07:00
|
|
|
'(%Q %Q %Q %Q %Q %Q %Q %Q)',
|
Simplify construction and execution of Differential queries for "responsible" users
Summary:
Currently, when querying for responsible users (revisions where a given user is either the author or a reviewer), we do this:
- If the query passes a bunch of hard-coded special cases, use a special hard-coded UNION.
- Otherwise, use a very complicated JOIN/WHERE clause.
This is bad for several reasons:
- Tons and tons of hard-coding and special casing.
- The JOIN/WHERE clause performs very poorly for large datasets.
- (As a material consequence, the homepage issues a responsible query which barely misses the hard-coded special cases and goes down the slow path.)
Instead, //always// use the UNION strategy to execute a "responsible" query. Specifically, if we have responsible PHIDs, temporarily add them to the author list and build a normal query, then repeat for reviewers, then UNION any clauses we built.
Fixes T3377. Ref T603. Ref T2625. Depends on D6342.
There's various folklore about UNION ALL / UNION DISTINCT performance. UNION DISTINCT is simpler here and the number of rows is small, although we could use UNION ALL in the form:
SELECT * FROM ((SELECT ...) UNION ALL (SELECT ...) ORDER) GROUP LIMIT
...if we find that there's some performance benefit at some point.
Test Plan: Used DarkConsole to examine queries. Viewed home page and Differential dashboard/authors/subscribers.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603, T2625, T3377
Differential Revision: https://secure.phabricator.com/D6343
2013-07-03 05:39:09 -07:00
|
|
|
$select,
|
2015-04-18 11:18:27 -07:00
|
|
|
$from,
|
Simplify construction and execution of Differential queries for "responsible" users
Summary:
Currently, when querying for responsible users (revisions where a given user is either the author or a reviewer), we do this:
- If the query passes a bunch of hard-coded special cases, use a special hard-coded UNION.
- Otherwise, use a very complicated JOIN/WHERE clause.
This is bad for several reasons:
- Tons and tons of hard-coding and special casing.
- The JOIN/WHERE clause performs very poorly for large datasets.
- (As a material consequence, the homepage issues a responsible query which barely misses the hard-coded special cases and goes down the slow path.)
Instead, //always// use the UNION strategy to execute a "responsible" query. Specifically, if we have responsible PHIDs, temporarily add them to the author list and build a normal query, then repeat for reviewers, then UNION any clauses we built.
Fixes T3377. Ref T603. Ref T2625. Depends on D6342.
There's various folklore about UNION ALL / UNION DISTINCT performance. UNION DISTINCT is simpler here and the number of rows is small, although we could use UNION ALL in the form:
SELECT * FROM ((SELECT ...) UNION ALL (SELECT ...) ORDER) GROUP LIMIT
...if we find that there's some performance benefit at some point.
Test Plan: Used DarkConsole to examine queries. Viewed home page and Differential dashboard/authors/subscribers.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603, T2625, T3377
Differential Revision: https://secure.phabricator.com/D6343
2013-07-03 05:39:09 -07:00
|
|
|
$joins,
|
|
|
|
|
$where,
|
|
|
|
|
$group_by,
|
2015-04-18 11:18:27 -07:00
|
|
|
$having,
|
Simplify construction and execution of Differential queries for "responsible" users
Summary:
Currently, when querying for responsible users (revisions where a given user is either the author or a reviewer), we do this:
- If the query passes a bunch of hard-coded special cases, use a special hard-coded UNION.
- Otherwise, use a very complicated JOIN/WHERE clause.
This is bad for several reasons:
- Tons and tons of hard-coding and special casing.
- The JOIN/WHERE clause performs very poorly for large datasets.
- (As a material consequence, the homepage issues a responsible query which barely misses the hard-coded special cases and goes down the slow path.)
Instead, //always// use the UNION strategy to execute a "responsible" query. Specifically, if we have responsible PHIDs, temporarily add them to the author list and build a normal query, then repeat for reviewers, then UNION any clauses we built.
Fixes T3377. Ref T603. Ref T2625. Depends on D6342.
There's various folklore about UNION ALL / UNION DISTINCT performance. UNION DISTINCT is simpler here and the number of rows is small, although we could use UNION ALL in the form:
SELECT * FROM ((SELECT ...) UNION ALL (SELECT ...) ORDER) GROUP LIMIT
...if we find that there's some performance benefit at some point.
Test Plan: Used DarkConsole to examine queries. Viewed home page and Differential dashboard/authors/subscribers.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603, T2625, T3377
Differential Revision: https://secure.phabricator.com/D6343
2013-07-03 05:39:09 -07:00
|
|
|
$order_by,
|
|
|
|
|
$limit);
|
|
|
|
|
}
|
|
|
|
|
|
Drive Differential landing page with DifferentialRevisionQuery, simplify UI
Summary:
- Use DifferentialRevisionQuery, not DifferentialRevisionListData, to select
revisions.
- Make UI simpler (I hope?) and more flexible, similar to Maniphest. It now
shows "Active", "Revisions", "Reviews" and "Subscribed" instead of a hodge-podge
of miscellaneous stuff. All now really has all revisions, not just open
revisions.
- Allow views to be filtered and sorted more flexibly.
- Allow anonymous users to use the per-user views, just don't default them
there.
NOTE: This might have performance implications! I need some help evaluating
them.
@nh / @jungejason / @aran, can one of you run some queries agianst FB's corpus?
The "active revisions" view is built much differently now. Before, we issued two
queries:
- SELECT (open revisions you authored that need revision) UNION ALL (open
revisions you are reviewing that need review)
- SELECT (open revisions you authored that need review) UNION ALL (open
revisions you are reviewing that need revision)
These two queries generate the "Action Required" and "Waiting on Others" views,
and are available in P247.
Now, we issue only one query:
- SELECT (open revisions you authored or are reviewing)
Then we divide them into the two tables in PHP. That query is available in P246.
On the secure.phabricator.com data, this new approach seems to be much better
(like, 10x better). But the secure.phabricator.com data isn't very large. Can
someone run it against Facebook's data (using a few heavy-hitting PHIDs, like
ola or something) to make sure it won't cause a regression?
In particular:
- Run the queries and make sure the new version doesn't take too long.
- Run the queries with EXPLAIN and give me the output maybe?
Test Plan:
- Looked at different filters.
- Changed "View User" PHID.
- Changed open/all.
- Changed sort order.
- Ran EXPLAIN / select against secure.phabricator.com corpus.
Reviewers: btrahan, nh, jungejason
Reviewed By: btrahan
CC: cpiro, aran, btrahan, epriestley, jungejason, nh
Maniphest Tasks: T586
Differential Revision: 1186
2011-12-07 15:35:10 -08:00
|
|
|
|
2011-10-02 12:03:16 -07:00
|
|
|
/* -( Internals )---------------------------------------------------------- */
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
|
* @task internal
|
|
|
|
|
*/
|
2018-11-07 02:48:26 -08:00
|
|
|
private function buildJoinsClause(AphrontDatabaseConnection $conn) {
|
2011-10-02 12:03:16 -07:00
|
|
|
$joins = array();
|
|
|
|
|
if ($this->pathIDs) {
|
|
|
|
|
$path_table = new DifferentialAffectedPath();
|
|
|
|
|
$joins[] = qsprintf(
|
2018-11-07 02:48:26 -08:00
|
|
|
$conn,
|
2011-10-02 12:03:16 -07:00
|
|
|
'JOIN %T p ON p.revisionID = r.id',
|
|
|
|
|
$path_table->getTableName());
|
|
|
|
|
}
|
|
|
|
|
|
2012-01-03 21:08:12 -08:00
|
|
|
if ($this->commitHashes) {
|
|
|
|
|
$joins[] = qsprintf(
|
2018-11-07 02:48:26 -08:00
|
|
|
$conn,
|
2012-01-03 21:08:12 -08:00
|
|
|
'JOIN %T hash_rel ON hash_rel.revisionID = r.id',
|
2012-01-10 11:39:11 -08:00
|
|
|
ArcanistDifferentialRevisionHash::TABLE_NAME);
|
2012-01-03 21:08:12 -08:00
|
|
|
}
|
|
|
|
|
|
2011-12-06 14:21:36 -08:00
|
|
|
if ($this->ccs) {
|
|
|
|
|
$joins[] = qsprintf(
|
2018-11-07 02:48:26 -08:00
|
|
|
$conn,
|
2014-02-12 08:53:40 -08:00
|
|
|
'JOIN %T e_ccs ON e_ccs.src = r.phid '.
|
|
|
|
|
'AND e_ccs.type = %s '.
|
|
|
|
|
'AND e_ccs.dst in (%Ls)',
|
|
|
|
|
PhabricatorEdgeConfig::TABLE_NAME_EDGE,
|
2015-01-03 10:33:25 +11:00
|
|
|
PhabricatorObjectHasSubscriberEdgeType::EDGECONST,
|
2011-12-06 14:21:36 -08:00
|
|
|
$this->ccs);
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if ($this->reviewers) {
|
|
|
|
|
$joins[] = qsprintf(
|
2018-11-07 02:48:26 -08:00
|
|
|
$conn,
|
2019-05-21 07:22:38 -07:00
|
|
|
'LEFT JOIN %T reviewer ON reviewer.revisionPHID = r.phid
|
2017-03-20 12:35:30 -07:00
|
|
|
AND reviewer.reviewerStatus != %s
|
|
|
|
|
AND reviewer.reviewerPHID in (%Ls)',
|
|
|
|
|
id(new DifferentialReviewer())->getTableName(),
|
|
|
|
|
DifferentialReviewerStatus::STATUS_RESIGNED,
|
2011-12-06 14:21:36 -08:00
|
|
|
$this->reviewers);
|
|
|
|
|
}
|
|
|
|
|
|
2019-05-21 07:22:38 -07:00
|
|
|
if ($this->noReviewers) {
|
|
|
|
|
$joins[] = qsprintf(
|
|
|
|
|
$conn,
|
|
|
|
|
'LEFT JOIN %T no_reviewer ON no_reviewer.revisionPHID = r.phid
|
|
|
|
|
AND no_reviewer.reviewerStatus != %s',
|
|
|
|
|
id(new DifferentialReviewer())->getTableName(),
|
|
|
|
|
DifferentialReviewerStatus::STATUS_RESIGNED);
|
|
|
|
|
}
|
|
|
|
|
|
2014-02-18 16:25:16 -08:00
|
|
|
if ($this->draftAuthors) {
|
|
|
|
|
$joins[] = qsprintf(
|
2018-11-07 02:48:26 -08:00
|
|
|
$conn,
|
2017-01-13 06:36:25 -08:00
|
|
|
'JOIN %T has_draft ON has_draft.srcPHID = r.phid
|
|
|
|
|
AND has_draft.type = %s
|
|
|
|
|
AND has_draft.dstPHID IN (%Ls)',
|
|
|
|
|
PhabricatorEdgeConfig::TABLE_NAME_EDGE,
|
|
|
|
|
PhabricatorObjectHasDraftEdgeType::EDGECONST,
|
2014-02-18 16:25:16 -08:00
|
|
|
$this->draftAuthors);
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-07 02:48:26 -08:00
|
|
|
$joins[] = $this->buildJoinClauseParts($conn);
|
2011-10-02 12:03:16 -07:00
|
|
|
|
2018-11-07 02:48:26 -08:00
|
|
|
return $this->formatJoinClause($conn, $joins);
|
2011-10-02 12:03:16 -07:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
|
* @task internal
|
|
|
|
|
*/
|
2018-11-07 02:48:26 -08:00
|
|
|
protected function buildWhereClause(AphrontDatabaseConnection $conn) {
|
2011-10-02 12:03:16 -07:00
|
|
|
$where = array();
|
|
|
|
|
|
|
|
|
|
if ($this->pathIDs) {
|
|
|
|
|
$path_clauses = array();
|
|
|
|
|
$repo_info = igroup($this->pathIDs, 'repositoryID');
|
|
|
|
|
foreach ($repo_info as $repository_id => $paths) {
|
|
|
|
|
$path_clauses[] = qsprintf(
|
2018-11-07 02:48:26 -08:00
|
|
|
$conn,
|
2012-08-16 18:32:57 -07:00
|
|
|
'(p.repositoryID = %d AND p.pathID IN (%Ld))',
|
2011-10-02 12:03:16 -07:00
|
|
|
$repository_id,
|
|
|
|
|
ipull($paths, 'pathID'));
|
|
|
|
|
}
|
2018-11-07 02:48:26 -08:00
|
|
|
$path_clauses = qsprintf($conn, '%LO', $path_clauses);
|
2011-10-02 12:03:16 -07:00
|
|
|
$where[] = $path_clauses;
|
|
|
|
|
}
|
|
|
|
|
|
2011-12-06 14:21:36 -08:00
|
|
|
if ($this->authors) {
|
2011-12-01 16:57:06 -08:00
|
|
|
$where[] = qsprintf(
|
2018-11-07 02:48:26 -08:00
|
|
|
$conn,
|
2012-08-16 18:32:57 -07:00
|
|
|
'r.authorPHID IN (%Ls)',
|
2011-12-06 14:21:36 -08:00
|
|
|
$this->authors);
|
2011-12-01 16:57:06 -08:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if ($this->revIDs) {
|
|
|
|
|
$where[] = qsprintf(
|
2018-11-07 02:48:26 -08:00
|
|
|
$conn,
|
2012-08-16 18:32:57 -07:00
|
|
|
'r.id IN (%Ld)',
|
2011-12-01 16:57:06 -08:00
|
|
|
$this->revIDs);
|
|
|
|
|
}
|
|
|
|
|
|
2013-09-26 14:17:26 -07:00
|
|
|
if ($this->repositoryPHIDs) {
|
|
|
|
|
$where[] = qsprintf(
|
2018-11-07 02:48:26 -08:00
|
|
|
$conn,
|
2013-09-26 14:17:26 -07:00
|
|
|
'r.repositoryPHID IN (%Ls)',
|
|
|
|
|
$this->repositoryPHIDs);
|
|
|
|
|
}
|
|
|
|
|
|
2012-01-03 21:08:12 -08:00
|
|
|
if ($this->commitHashes) {
|
|
|
|
|
$hash_clauses = array();
|
|
|
|
|
foreach ($this->commitHashes as $info) {
|
|
|
|
|
list($type, $hash) = $info;
|
|
|
|
|
$hash_clauses[] = qsprintf(
|
2018-11-07 02:48:26 -08:00
|
|
|
$conn,
|
2012-01-03 21:08:12 -08:00
|
|
|
'(hash_rel.type = %s AND hash_rel.hash = %s)',
|
|
|
|
|
$type,
|
|
|
|
|
$hash);
|
|
|
|
|
}
|
2018-11-07 02:48:26 -08:00
|
|
|
$hash_clauses = qsprintf($conn, '%LO', $hash_clauses);
|
2012-01-03 21:08:12 -08:00
|
|
|
$where[] = $hash_clauses;
|
|
|
|
|
}
|
|
|
|
|
|
2011-12-01 16:57:06 -08:00
|
|
|
if ($this->phids) {
|
|
|
|
|
$where[] = qsprintf(
|
2018-11-07 02:48:26 -08:00
|
|
|
$conn,
|
2012-08-16 18:32:57 -07:00
|
|
|
'r.phid IN (%Ls)',
|
2011-12-01 16:57:06 -08:00
|
|
|
$this->phids);
|
|
|
|
|
}
|
|
|
|
|
|
2012-04-10 12:51:34 -07:00
|
|
|
if ($this->branches) {
|
|
|
|
|
$where[] = qsprintf(
|
2018-11-07 02:48:26 -08:00
|
|
|
$conn,
|
2012-04-10 12:51:34 -07:00
|
|
|
'r.branchName in (%Ls)',
|
|
|
|
|
$this->branches);
|
|
|
|
|
}
|
|
|
|
|
|
2015-03-25 10:21:56 -07:00
|
|
|
if ($this->updatedEpochMin !== null) {
|
|
|
|
|
$where[] = qsprintf(
|
2018-11-07 02:48:26 -08:00
|
|
|
$conn,
|
2015-03-25 10:21:56 -07:00
|
|
|
'r.dateModified >= %d',
|
|
|
|
|
$this->updatedEpochMin);
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if ($this->updatedEpochMax !== null) {
|
|
|
|
|
$where[] = qsprintf(
|
2018-11-07 02:48:26 -08:00
|
|
|
$conn,
|
2015-03-25 10:21:56 -07:00
|
|
|
'r.dateModified <= %d',
|
|
|
|
|
$this->updatedEpochMax);
|
|
|
|
|
}
|
|
|
|
|
|
2018-10-05 11:07:28 -07:00
|
|
|
if ($this->createdEpochMin !== null) {
|
|
|
|
|
$where[] = qsprintf(
|
2018-11-07 02:48:26 -08:00
|
|
|
$conn,
|
2018-10-05 11:07:28 -07:00
|
|
|
'r.dateCreated >= %d',
|
|
|
|
|
$this->createdEpochMin);
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if ($this->createdEpochMax !== null) {
|
|
|
|
|
$where[] = qsprintf(
|
2018-11-07 02:48:26 -08:00
|
|
|
$conn,
|
2018-10-05 11:07:28 -07:00
|
|
|
'r.dateCreated <= %d',
|
|
|
|
|
$this->createdEpochMax);
|
|
|
|
|
}
|
|
|
|
|
|
2017-08-11 08:10:28 -07:00
|
|
|
if ($this->statuses !== null) {
|
|
|
|
|
$where[] = qsprintf(
|
2018-11-07 02:48:26 -08:00
|
|
|
$conn,
|
2017-08-11 08:10:28 -07:00
|
|
|
'r.status in (%Ls)',
|
2017-08-11 16:27:38 -07:00
|
|
|
$this->statuses);
|
2017-08-11 08:10:28 -07:00
|
|
|
}
|
|
|
|
|
|
2017-08-11 09:18:47 -07:00
|
|
|
if ($this->isOpen !== null) {
|
|
|
|
|
if ($this->isOpen) {
|
2017-08-11 09:28:05 -07:00
|
|
|
$statuses = DifferentialLegacyQuery::getModernValues(
|
2017-08-11 09:18:47 -07:00
|
|
|
DifferentialLegacyQuery::STATUS_OPEN);
|
|
|
|
|
} else {
|
2017-08-11 09:28:05 -07:00
|
|
|
$statuses = DifferentialLegacyQuery::getModernValues(
|
2017-08-11 09:18:47 -07:00
|
|
|
DifferentialLegacyQuery::STATUS_CLOSED);
|
|
|
|
|
}
|
|
|
|
|
$where[] = qsprintf(
|
2018-11-07 02:48:26 -08:00
|
|
|
$conn,
|
2017-08-11 09:18:47 -07:00
|
|
|
'r.status in (%Ls)',
|
2017-08-11 16:27:38 -07:00
|
|
|
$statuses);
|
2017-08-11 09:18:47 -07:00
|
|
|
}
|
|
|
|
|
|
2019-05-21 07:22:38 -07:00
|
|
|
$reviewer_subclauses = array();
|
|
|
|
|
|
|
|
|
|
if ($this->noReviewers) {
|
|
|
|
|
$reviewer_subclauses[] = qsprintf(
|
|
|
|
|
$conn,
|
|
|
|
|
'no_reviewer.reviewerPHID IS NULL');
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if ($this->reviewers) {
|
|
|
|
|
$reviewer_subclauses[] = qsprintf(
|
|
|
|
|
$conn,
|
|
|
|
|
'reviewer.reviewerPHID IS NOT NULL');
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if ($reviewer_subclauses) {
|
|
|
|
|
$where[] = qsprintf($conn, '%LO', $reviewer_subclauses);
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-07 02:48:26 -08:00
|
|
|
$where[] = $this->buildWhereClauseParts($conn);
|
|
|
|
|
|
|
|
|
|
return $this->formatWhereClause($conn, $where);
|
2011-10-02 12:03:16 -07:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
|
* @task internal
|
|
|
|
|
*/
|
Modernize DifferentialRevisionSearchEngine
Summary:
Ref T10939. Ref T4144. This moves the revision SearchEngine to modern code so I can add some kind of bucketing layer on top of it.
This seems to have worked pretty cleanly. One thing is that I removed the ability to search for "pending drafts":
- This was added in D1927 from a bootcamp task, was an indirect solution to a questionable problem, and almost certainly would not meet the bar today.
- Later, in D3324, we added the icons to the list. I think this is a better solution in general. In particular, it specifically addressed the query being kind of junky.
- At the time, Differential had a prebuilt "Drafts" filter. This was removed in D6347 with the move to ApplicationSearch, which simplified the large number of prebuilt filters. Although we got a lot of feedback about that, none requested that the drafts filter be restored.
Test Plan: Searched for responsible users, subscribers, orders, projects, repositories.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4144, T10939
Differential Revision: https://secure.phabricator.com/D15921
2016-05-15 10:07:58 -07:00
|
|
|
protected function shouldGroupQueryResultRows() {
|
|
|
|
|
|
2020-09-15 17:13:21 -07:00
|
|
|
if (count($this->pathIDs) > 1) {
|
|
|
|
|
return true;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if (count($this->ccs) > 1) {
|
|
|
|
|
return true;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if (count($this->reviewers) > 1) {
|
|
|
|
|
return true;
|
|
|
|
|
}
|
2011-12-07 08:30:49 -08:00
|
|
|
|
2020-09-15 17:13:21 -07:00
|
|
|
if (count($this->commitHashes) > 1) {
|
Modernize DifferentialRevisionSearchEngine
Summary:
Ref T10939. Ref T4144. This moves the revision SearchEngine to modern code so I can add some kind of bucketing layer on top of it.
This seems to have worked pretty cleanly. One thing is that I removed the ability to search for "pending drafts":
- This was added in D1927 from a bootcamp task, was an indirect solution to a questionable problem, and almost certainly would not meet the bar today.
- Later, in D3324, we added the icons to the list. I think this is a better solution in general. In particular, it specifically addressed the query being kind of junky.
- At the time, Differential had a prebuilt "Drafts" filter. This was removed in D6347 with the move to ApplicationSearch, which simplified the large number of prebuilt filters. Although we got a lot of feedback about that, none requested that the drafts filter be restored.
Test Plan: Searched for responsible users, subscribers, orders, projects, repositories.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4144, T10939
Differential Revision: https://secure.phabricator.com/D15921
2016-05-15 10:07:58 -07:00
|
|
|
return true;
|
2011-10-02 12:03:16 -07:00
|
|
|
}
|
Modernize DifferentialRevisionSearchEngine
Summary:
Ref T10939. Ref T4144. This moves the revision SearchEngine to modern code so I can add some kind of bucketing layer on top of it.
This seems to have worked pretty cleanly. One thing is that I removed the ability to search for "pending drafts":
- This was added in D1927 from a bootcamp task, was an indirect solution to a questionable problem, and almost certainly would not meet the bar today.
- Later, in D3324, we added the icons to the list. I think this is a better solution in general. In particular, it specifically addressed the query being kind of junky.
- At the time, Differential had a prebuilt "Drafts" filter. This was removed in D6347 with the move to ApplicationSearch, which simplified the large number of prebuilt filters. Although we got a lot of feedback about that, none requested that the drafts filter be restored.
Test Plan: Searched for responsible users, subscribers, orders, projects, repositories.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4144, T10939
Differential Revision: https://secure.phabricator.com/D15921
2016-05-15 10:07:58 -07:00
|
|
|
|
2019-05-21 07:22:38 -07:00
|
|
|
if ($this->noReviewers) {
|
|
|
|
|
return true;
|
|
|
|
|
}
|
|
|
|
|
|
Modernize DifferentialRevisionSearchEngine
Summary:
Ref T10939. Ref T4144. This moves the revision SearchEngine to modern code so I can add some kind of bucketing layer on top of it.
This seems to have worked pretty cleanly. One thing is that I removed the ability to search for "pending drafts":
- This was added in D1927 from a bootcamp task, was an indirect solution to a questionable problem, and almost certainly would not meet the bar today.
- Later, in D3324, we added the icons to the list. I think this is a better solution in general. In particular, it specifically addressed the query being kind of junky.
- At the time, Differential had a prebuilt "Drafts" filter. This was removed in D6347 with the move to ApplicationSearch, which simplified the large number of prebuilt filters. Although we got a lot of feedback about that, none requested that the drafts filter be restored.
Test Plan: Searched for responsible users, subscribers, orders, projects, repositories.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4144, T10939
Differential Revision: https://secure.phabricator.com/D15921
2016-05-15 10:07:58 -07:00
|
|
|
return parent::shouldGroupQueryResultRows();
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
public function getBuiltinOrders() {
|
|
|
|
|
$orders = parent::getBuiltinOrders() + array(
|
|
|
|
|
'updated' => array(
|
|
|
|
|
'vector' => array('updated', 'id'),
|
|
|
|
|
'name' => pht('Date Updated (Latest First)'),
|
|
|
|
|
'aliases' => array(self::ORDER_MODIFIED),
|
|
|
|
|
),
|
|
|
|
|
'outdated' => array(
|
|
|
|
|
'vector' => array('-updated', '-id'),
|
|
|
|
|
'name' => pht('Date Updated (Oldest First)'),
|
|
|
|
|
),
|
|
|
|
|
);
|
|
|
|
|
|
|
|
|
|
// Alias the "newest" builtin to the historical key for it.
|
|
|
|
|
$orders['newest']['aliases'][] = self::ORDER_CREATED;
|
|
|
|
|
|
|
|
|
|
return $orders;
|
2011-10-02 12:03:16 -07:00
|
|
|
}
|
|
|
|
|
|
2015-04-11 20:26:20 -07:00
|
|
|
protected function getDefaultOrderVector() {
|
|
|
|
|
return array('updated', 'id');
|
|
|
|
|
}
|
2013-09-13 06:23:09 -07:00
|
|
|
|
2015-04-11 20:26:20 -07:00
|
|
|
public function getOrderableColumns() {
|
|
|
|
|
return array(
|
|
|
|
|
'updated' => array(
|
2017-10-23 14:59:32 -07:00
|
|
|
'table' => $this->getPrimaryTableAlias(),
|
2015-04-11 20:26:20 -07:00
|
|
|
'column' => 'dateModified',
|
|
|
|
|
'type' => 'int',
|
|
|
|
|
),
|
2017-09-06 11:48:21 -07:00
|
|
|
) + parent::getOrderableColumns();
|
2013-07-03 05:45:07 -07:00
|
|
|
}
|
|
|
|
|
|
2019-03-18 11:44:54 -07:00
|
|
|
protected function newPagingMapFromPartialObject($object) {
|
2015-04-11 20:26:20 -07:00
|
|
|
return array(
|
2019-03-18 11:44:54 -07:00
|
|
|
'id' => (int)$object->getID(),
|
|
|
|
|
'updated' => (int)$object->getDateModified(),
|
2015-04-11 20:26:20 -07:00
|
|
|
);
|
2011-10-02 12:03:16 -07:00
|
|
|
}
|
|
|
|
|
|
Align "RevisionQuery->needCommitPHIDs()" to use Edges, not the legacy table
Summary:
Depends on D20458. Ref T13276. Although I'm not thrilled about "needCommitPHIDs()", it has a few callers, including custom fields. Allow "need + attach + get" to survive for now since they're reasonably modern, at least.
However, use edges instead of "TABLE_COMMIT" and require `need...()` + `get...()`, removing the direct `load...()`.
Also remove `RevisionQuery->withCommitPHIDs(...)`, which has no callers.
Test Plan:
- Grepped for `loadCommitPHIDs` (only two hits, the private `RevisionQuery` method).
- Called "differential.getrevision", got commits.
- Viewed a revision, saw "Commits: ...".
- Grepped for `withCommitPHIDs()`, no callers on `RevisionQuery` (some other query classes have methods with this name).
- Called "differential.query", got commits.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13276
Differential Revision: https://secure.phabricator.com/D20459
2019-04-22 12:19:54 -07:00
|
|
|
private function loadCommitPHIDs(array $revisions) {
|
2012-04-04 13:13:08 -07:00
|
|
|
assert_instances_of($revisions, 'DifferentialRevision');
|
Align "RevisionQuery->needCommitPHIDs()" to use Edges, not the legacy table
Summary:
Depends on D20458. Ref T13276. Although I'm not thrilled about "needCommitPHIDs()", it has a few callers, including custom fields. Allow "need + attach + get" to survive for now since they're reasonably modern, at least.
However, use edges instead of "TABLE_COMMIT" and require `need...()` + `get...()`, removing the direct `load...()`.
Also remove `RevisionQuery->withCommitPHIDs(...)`, which has no callers.
Test Plan:
- Grepped for `loadCommitPHIDs` (only two hits, the private `RevisionQuery` method).
- Called "differential.getrevision", got commits.
- Viewed a revision, saw "Commits: ...".
- Grepped for `withCommitPHIDs()`, no callers on `RevisionQuery` (some other query classes have methods with this name).
- Called "differential.query", got commits.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13276
Differential Revision: https://secure.phabricator.com/D20459
2019-04-22 12:19:54 -07:00
|
|
|
|
|
|
|
|
if (!$revisions) {
|
|
|
|
|
return;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
$revisions = mpull($revisions, null, 'getPHID');
|
|
|
|
|
|
|
|
|
|
$edge_query = id(new PhabricatorEdgeQuery())
|
|
|
|
|
->withSourcePHIDs(array_keys($revisions))
|
|
|
|
|
->withEdgeTypes(
|
|
|
|
|
array(
|
|
|
|
|
DifferentialRevisionHasCommitEdgeType::EDGECONST,
|
|
|
|
|
));
|
|
|
|
|
$edge_query->execute();
|
|
|
|
|
|
|
|
|
|
foreach ($revisions as $phid => $revision) {
|
|
|
|
|
$commit_phids = $edge_query->getDestinationPHIDs(array($phid));
|
|
|
|
|
$revision->attachCommitPHIDs($commit_phids);
|
2011-12-20 17:05:52 -08:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
private function loadDiffIDs($conn_r, array $revisions) {
|
2012-04-04 13:13:08 -07:00
|
|
|
assert_instances_of($revisions, 'DifferentialRevision');
|
|
|
|
|
|
2011-12-20 17:05:52 -08:00
|
|
|
$diff_table = new DifferentialDiff();
|
|
|
|
|
|
|
|
|
|
$diff_ids = queryfx_all(
|
|
|
|
|
$conn_r,
|
|
|
|
|
'SELECT revisionID, id FROM %T WHERE revisionID IN (%Ld)
|
|
|
|
|
ORDER BY id DESC',
|
|
|
|
|
$diff_table->getTableName(),
|
|
|
|
|
mpull($revisions, 'getID'));
|
|
|
|
|
$diff_ids = igroup($diff_ids, 'revisionID');
|
|
|
|
|
|
|
|
|
|
foreach ($revisions as $revision) {
|
|
|
|
|
$ids = idx($diff_ids, $revision->getID(), array());
|
|
|
|
|
$ids = ipull($ids, 'id');
|
|
|
|
|
$revision->attachDiffIDs($ids);
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
private function loadActiveDiffs($conn_r, array $revisions) {
|
2012-04-04 13:13:08 -07:00
|
|
|
assert_instances_of($revisions, 'DifferentialRevision');
|
|
|
|
|
|
2011-12-20 17:05:52 -08:00
|
|
|
$diff_table = new DifferentialDiff();
|
|
|
|
|
|
|
|
|
|
$load_ids = array();
|
|
|
|
|
foreach ($revisions as $revision) {
|
|
|
|
|
$diffs = $revision->getDiffIDs();
|
|
|
|
|
if ($diffs) {
|
|
|
|
|
$load_ids[] = max($diffs);
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
$active_diffs = array();
|
|
|
|
|
if ($load_ids) {
|
|
|
|
|
$active_diffs = $diff_table->loadAllWhere(
|
|
|
|
|
'id IN (%Ld)',
|
|
|
|
|
$load_ids);
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
$active_diffs = mpull($active_diffs, null, 'getRevisionID');
|
|
|
|
|
foreach ($revisions as $revision) {
|
|
|
|
|
$revision->attachActiveDiff(idx($active_diffs, $revision->getID()));
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
2012-06-26 09:07:52 -07:00
|
|
|
private function loadHashes(
|
|
|
|
|
AphrontDatabaseConnection $conn_r,
|
|
|
|
|
array $revisions) {
|
|
|
|
|
assert_instances_of($revisions, 'DifferentialRevision');
|
|
|
|
|
|
|
|
|
|
$data = queryfx_all(
|
|
|
|
|
$conn_r,
|
|
|
|
|
'SELECT * FROM %T WHERE revisionID IN (%Ld)',
|
|
|
|
|
'differential_revisionhash',
|
|
|
|
|
mpull($revisions, 'getID'));
|
|
|
|
|
|
|
|
|
|
$data = igroup($data, 'revisionID');
|
|
|
|
|
foreach ($revisions as $revision) {
|
|
|
|
|
$hashes = idx($data, $revision->getID(), array());
|
|
|
|
|
$list = array();
|
|
|
|
|
foreach ($hashes as $hash) {
|
|
|
|
|
$list[] = array($hash['type'], $hash['hash']);
|
|
|
|
|
}
|
|
|
|
|
$revision->attachHashes($list);
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
2013-07-14 19:18:55 -07:00
|
|
|
private function loadReviewers(
|
2017-03-20 12:35:30 -07:00
|
|
|
AphrontDatabaseConnection $conn,
|
2013-07-14 19:18:55 -07:00
|
|
|
array $revisions) {
|
|
|
|
|
|
|
|
|
|
assert_instances_of($revisions, 'DifferentialRevision');
|
|
|
|
|
|
2017-03-20 12:35:30 -07:00
|
|
|
$reviewer_table = new DifferentialReviewer();
|
|
|
|
|
$reviewer_rows = queryfx_all(
|
|
|
|
|
$conn,
|
|
|
|
|
'SELECT * FROM %T WHERE revisionPHID IN (%Ls)
|
|
|
|
|
ORDER BY id ASC',
|
|
|
|
|
$reviewer_table->getTableName(),
|
|
|
|
|
mpull($revisions, 'getPHID'));
|
|
|
|
|
$reviewer_list = $reviewer_table->loadAllFromArray($reviewer_rows);
|
|
|
|
|
$reviewer_map = mgroup($reviewer_list, 'getRevisionPHID');
|
|
|
|
|
|
|
|
|
|
foreach ($reviewer_map as $key => $reviewers) {
|
|
|
|
|
$reviewer_map[$key] = mpull($reviewers, null, 'getReviewerPHID');
|
|
|
|
|
}
|
2013-07-14 19:18:55 -07:00
|
|
|
|
2013-10-06 17:08:14 -07:00
|
|
|
$viewer = $this->getViewer();
|
|
|
|
|
$viewer_phid = $viewer->getPHID();
|
2017-03-20 12:35:30 -07:00
|
|
|
|
2014-04-11 10:31:07 -07:00
|
|
|
$allow_key = 'differential.allow-self-accept';
|
|
|
|
|
$allow_self = PhabricatorEnv::getEnvConfig($allow_key);
|
2013-10-06 17:08:14 -07:00
|
|
|
|
|
|
|
|
// Figure out which of these reviewers the viewer has authority to act as.
|
|
|
|
|
if ($this->needReviewerAuthority && $viewer_phid) {
|
|
|
|
|
$authority = $this->loadReviewerAuthority(
|
|
|
|
|
$revisions,
|
2017-03-20 12:35:30 -07:00
|
|
|
$reviewer_map,
|
2013-10-06 17:08:14 -07:00
|
|
|
$allow_self);
|
|
|
|
|
}
|
|
|
|
|
|
2013-07-14 19:18:55 -07:00
|
|
|
foreach ($revisions as $revision) {
|
2017-03-20 12:35:30 -07:00
|
|
|
$reviewers = idx($reviewer_map, $revision->getPHID(), array());
|
|
|
|
|
foreach ($reviewers as $reviewer_phid => $reviewer) {
|
2013-10-06 17:08:14 -07:00
|
|
|
if ($this->needReviewerAuthority) {
|
|
|
|
|
if (!$viewer_phid) {
|
|
|
|
|
// Logged-out users never have authority.
|
|
|
|
|
$has_authority = false;
|
|
|
|
|
} else if ((!$allow_self) &&
|
|
|
|
|
($revision->getAuthorPHID() == $viewer_phid)) {
|
|
|
|
|
// The author can never have authority unless we allow self-accept.
|
|
|
|
|
$has_authority = false;
|
|
|
|
|
} else {
|
2015-05-22 17:27:56 +10:00
|
|
|
// Otherwise, look up whether the viewer has authority.
|
2013-10-06 17:08:14 -07:00
|
|
|
$has_authority = isset($authority[$reviewer_phid]);
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
$reviewer->attachAuthority($viewer, $has_authority);
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
$reviewers[$reviewer_phid] = $reviewer;
|
2013-07-14 19:18:55 -07:00
|
|
|
}
|
|
|
|
|
|
2017-03-20 15:04:23 -07:00
|
|
|
$revision->attachReviewers($reviewers);
|
2013-07-14 19:18:55 -07:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
2013-10-06 17:08:14 -07:00
|
|
|
private function loadReviewerAuthority(
|
|
|
|
|
array $revisions,
|
2017-03-20 12:35:30 -07:00
|
|
|
array $reviewers,
|
2013-10-06 17:08:14 -07:00
|
|
|
$allow_self) {
|
|
|
|
|
|
|
|
|
|
$revision_map = mpull($revisions, null, 'getPHID');
|
|
|
|
|
$viewer_phid = $this->getViewer()->getPHID();
|
|
|
|
|
|
2016-05-13 07:23:13 -07:00
|
|
|
// Find all the project/package reviewers which the user may have authority
|
|
|
|
|
// over.
|
2013-10-06 17:08:14 -07:00
|
|
|
$project_phids = array();
|
2016-05-13 07:23:13 -07:00
|
|
|
$package_phids = array();
|
2014-07-24 08:05:46 +10:00
|
|
|
$project_type = PhabricatorProjectProjectPHIDType::TYPECONST;
|
2016-05-13 07:23:13 -07:00
|
|
|
$package_type = PhabricatorOwnersPackagePHIDType::TYPECONST;
|
|
|
|
|
|
2017-03-20 12:35:30 -07:00
|
|
|
foreach ($reviewers as $revision_phid => $reviewer_list) {
|
2013-10-06 17:08:14 -07:00
|
|
|
if (!$allow_self) {
|
2017-03-20 12:35:30 -07:00
|
|
|
if ($revision_map[$revision_phid]->getAuthorPHID() == $viewer_phid) {
|
2013-10-06 17:08:14 -07:00
|
|
|
// If self-review isn't permitted, the user will never have
|
|
|
|
|
// authority over projects on revisions they authored because you
|
|
|
|
|
// can't accept your own revisions, so we don't need to load any
|
|
|
|
|
// data about these reviewers.
|
|
|
|
|
continue;
|
|
|
|
|
}
|
|
|
|
|
}
|
2017-03-20 12:35:30 -07:00
|
|
|
|
|
|
|
|
foreach ($reviewer_list as $reviewer_phid => $reviewer) {
|
|
|
|
|
$phid_type = phid_get_type($reviewer_phid);
|
2016-05-13 07:23:13 -07:00
|
|
|
if ($phid_type == $project_type) {
|
2017-03-20 12:35:30 -07:00
|
|
|
$project_phids[] = $reviewer_phid;
|
2013-10-06 17:08:14 -07:00
|
|
|
}
|
2016-05-13 07:23:13 -07:00
|
|
|
if ($phid_type == $package_type) {
|
2017-03-20 12:35:30 -07:00
|
|
|
$package_phids[] = $reviewer_phid;
|
2016-05-13 07:23:13 -07:00
|
|
|
}
|
2013-10-06 17:08:14 -07:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
2016-05-13 07:23:13 -07:00
|
|
|
// The viewer has authority over themselves.
|
|
|
|
|
$user_authority = array_fuse(array($viewer_phid));
|
|
|
|
|
|
|
|
|
|
// And over any projects they are a member of.
|
2013-10-06 17:08:14 -07:00
|
|
|
$project_authority = array();
|
|
|
|
|
if ($project_phids) {
|
|
|
|
|
$project_authority = id(new PhabricatorProjectQuery())
|
|
|
|
|
->setViewer($this->getViewer())
|
|
|
|
|
->withPHIDs($project_phids)
|
|
|
|
|
->withMemberPHIDs(array($viewer_phid))
|
|
|
|
|
->execute();
|
|
|
|
|
$project_authority = mpull($project_authority, 'getPHID');
|
2016-05-13 07:23:13 -07:00
|
|
|
$project_authority = array_fuse($project_authority);
|
2013-10-06 17:08:14 -07:00
|
|
|
}
|
|
|
|
|
|
2016-05-13 07:23:13 -07:00
|
|
|
// And over any packages they own.
|
|
|
|
|
$package_authority = array();
|
|
|
|
|
if ($package_phids) {
|
|
|
|
|
$package_authority = id(new PhabricatorOwnersPackageQuery())
|
|
|
|
|
->setViewer($this->getViewer())
|
|
|
|
|
->withPHIDs($package_phids)
|
|
|
|
|
->withAuthorityPHIDs(array($viewer_phid))
|
|
|
|
|
->execute();
|
|
|
|
|
$package_authority = mpull($package_authority, 'getPHID');
|
|
|
|
|
$package_authority = array_fuse($package_authority);
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
return $user_authority + $project_authority + $package_authority;
|
2013-10-06 17:08:14 -07:00
|
|
|
}
|
|
|
|
|
|
Lock policy queries to their applications
Summary:
While we mostly have reasonable effective object accessibility when you lock a user out of an application, it's primarily enforced at the controller level. Users can still, e.g., load the handles of objects they can't actually see. Instead, lock the queries to the applications so that you can, e.g., never load a revision if you don't have access to Differential.
This has several parts:
- For PolicyAware queries, provide an application class name method.
- If the query specifies a class name and the user doesn't have permission to use it, fail the entire query unconditionally.
- For handles, simplify query construction and count all the PHIDs as "restricted" so we get a UI full of "restricted" instead of "unknown" handles.
Test Plan:
- Added a unit test to verify I got all the class names right.
- Browsed around, logged in/out as a normal user with public policies on and off.
- Browsed around, logged in/out as a restricted user with public policies on and off. With restrictions, saw all traces of restricted apps removed or restricted.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7367
2013-10-21 17:20:27 -07:00
|
|
|
public function getQueryApplicationClass() {
|
2014-07-23 10:03:09 +10:00
|
|
|
return 'PhabricatorDifferentialApplication';
|
Lock policy queries to their applications
Summary:
While we mostly have reasonable effective object accessibility when you lock a user out of an application, it's primarily enforced at the controller level. Users can still, e.g., load the handles of objects they can't actually see. Instead, lock the queries to the applications so that you can, e.g., never load a revision if you don't have access to Differential.
This has several parts:
- For PolicyAware queries, provide an application class name method.
- If the query specifies a class name and the user doesn't have permission to use it, fail the entire query unconditionally.
- For handles, simplify query construction and count all the PHIDs as "restricted" so we get a UI full of "restricted" instead of "unknown" handles.
Test Plan:
- Added a unit test to verify I got all the class names right.
- Browsed around, logged in/out as a normal user with public policies on and off.
- Browsed around, logged in/out as a restricted user with public policies on and off. With restrictions, saw all traces of restricted apps removed or restricted.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7367
2013-10-21 17:20:27 -07:00
|
|
|
}
|
2011-10-02 12:03:16 -07:00
|
|
|
|
2015-04-18 11:18:27 -07:00
|
|
|
protected function getPrimaryTableAlias() {
|
|
|
|
|
return 'r';
|
|
|
|
|
}
|
|
|
|
|
|
2011-10-02 12:03:16 -07:00
|
|
|
}
|