2011-10-02 12:03:16 -07:00
|
|
|
<?php
|
|
|
|
|
|
|
|
/**
|
|
|
|
* Flexible query API for Differential revisions. Example:
|
|
|
|
*
|
|
|
|
* // Load open revisions
|
|
|
|
* $revisions = id(new DifferentialRevisionQuery())
|
|
|
|
* ->withStatus(DifferentialRevisionQuery::STATUS_OPEN)
|
|
|
|
* ->execute();
|
|
|
|
*
|
|
|
|
* @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();
|
|
|
|
|
2013-08-07 14:53:49 +01:00
|
|
|
private $status = 'status-any';
|
|
|
|
const STATUS_ANY = 'status-any';
|
|
|
|
const STATUS_OPEN = 'status-open';
|
|
|
|
const STATUS_ACCEPTED = 'status-accepted';
|
|
|
|
const STATUS_NEEDS_REVIEW = 'status-needs-review';
|
|
|
|
const STATUS_NEEDS_REVISION = 'status-needs-revision';
|
2013-11-25 17:39:24 -08:00
|
|
|
const STATUS_CLOSED = 'status-closed';
|
2013-08-07 14:53:49 +01:00
|
|
|
const STATUS_ABANDONED = 'status-abandoned';
|
2011-10-02 12:03:16 -07:00
|
|
|
|
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();
|
2015-01-30 11:51:16 -08:00
|
|
|
private $commitPHIDs = 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;
|
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 $needRelationships = false;
|
|
|
|
private $needActiveDiffs = false;
|
|
|
|
private $needDiffIDs = false;
|
|
|
|
private $needCommitPHIDs = false;
|
2012-06-26 09:07:52 -07:00
|
|
|
private $needHashes = false;
|
2013-07-14 19:18:55 -07:00
|
|
|
private $needReviewerStatus = 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
|
|
|
|
2013-07-03 05:45:07 -07:00
|
|
|
private $buildingGlobalOrder;
|
|
|
|
|
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) {
|
|
|
|
$this->reviewers = $reviewer_phids;
|
|
|
|
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
|
|
|
|
2015-01-30 11:51:16 -08:00
|
|
|
/**
|
|
|
|
* Filter results to revisions that have one of the provided PHIDs as
|
|
|
|
* commits. Calling this function will clear anything set by previous calls
|
|
|
|
* to @{method:withCommitPHIDs}.
|
|
|
|
*
|
|
|
|
* @param array List of PHIDs of commits
|
|
|
|
* @return this
|
|
|
|
* @task config
|
|
|
|
*/
|
|
|
|
public function withCommitPHIDs(array $commit_phids) {
|
|
|
|
$this->commitPHIDs = $commit_phids;
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
2011-10-02 12:03:16 -07:00
|
|
|
/**
|
|
|
|
* Filter results to revisions with a given status. Provide a class constant,
|
2014-07-23 10:03:09 +10:00
|
|
|
* such as `DifferentialRevisionQuery::STATUS_OPEN`.
|
2011-10-02 12:03:16 -07:00
|
|
|
*
|
|
|
|
* @param const Class STATUS constant, like STATUS_OPEN.
|
|
|
|
* @return this
|
|
|
|
* @task config
|
|
|
|
*/
|
|
|
|
public function withStatus($status_constant) {
|
|
|
|
$this->status = $status_constant;
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
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;
|
|
|
|
}
|
|
|
|
|
2012-04-10 12:51:34 -07:00
|
|
|
|
2011-10-02 12:03:16 -07:00
|
|
|
|
|
|
|
/**
|
|
|
|
* Set whether or not the query will load and attach relationships.
|
|
|
|
*
|
|
|
|
* @param bool True to load and attach relationships.
|
|
|
|
* @return this
|
|
|
|
* @task config
|
|
|
|
*/
|
|
|
|
public function needRelationships($need_relationships) {
|
|
|
|
$this->needRelationships = $need_relationships;
|
|
|
|
return $this;
|
|
|
|
}
|
|
|
|
|
|
|
|
|
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
|
|
|
/**
|
|
|
|
* Set whether or not the query should load associated reviewer status.
|
|
|
|
*
|
|
|
|
* @param bool True to load and attach reviewers.
|
|
|
|
* @return this
|
|
|
|
* @task config
|
|
|
|
*/
|
|
|
|
public function needReviewerStatus($need_reviewer_status) {
|
|
|
|
$this->needReviewerStatus = $need_reviewer_status;
|
|
|
|
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();
|
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
|
|
|
$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->needRelationships) {
|
|
|
|
$this->loadRelationships($conn_r, $revisions);
|
|
|
|
}
|
2012-01-24 08:31:45 -08:00
|
|
|
|
2013-07-03 05:43:52 -07:00
|
|
|
if ($this->needCommitPHIDs) {
|
|
|
|
$this->loadCommitPHIDs($conn_r, $revisions);
|
|
|
|
}
|
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
|
|
|
|
2013-10-06 17:08:14 -07:00
|
|
|
if ($this->needReviewerStatus || $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();
|
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 = $table->establishConnection('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
|
|
|
$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;
|
|
|
|
$selects[] = $this->buildSelectStatement($conn_r);
|
|
|
|
|
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);
|
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[] = $this->buildSelectStatement($conn_r);
|
|
|
|
|
|
|
|
// 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 {
|
|
|
|
$selects[] = $this->buildSelectStatement($conn_r);
|
|
|
|
}
|
|
|
|
|
|
|
|
if (count($selects) > 1) {
|
2013-07-03 05:45:07 -07:00
|
|
|
$this->buildingGlobalOrder = true;
|
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(
|
|
|
|
$conn_r,
|
|
|
|
'%Q %Q %Q',
|
|
|
|
implode(' UNION DISTINCT ', $selects),
|
2013-07-03 05:45:07 -07:00
|
|
|
$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
|
|
|
$this->buildLimitClause($conn_r));
|
|
|
|
} else {
|
|
|
|
$query = head($selects);
|
|
|
|
}
|
|
|
|
|
|
|
|
return queryfx_all($conn_r, '%Q', $query);
|
|
|
|
}
|
|
|
|
|
|
|
|
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
|
|
|
|
|
|
|
$this->buildingGlobalOrder = false;
|
|
|
|
$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
|
|
|
|
*/
|
|
|
|
private function buildJoinsClause($conn_r) {
|
|
|
|
$joins = array();
|
|
|
|
if ($this->pathIDs) {
|
|
|
|
$path_table = new DifferentialAffectedPath();
|
|
|
|
$joins[] = qsprintf(
|
|
|
|
$conn_r,
|
|
|
|
'JOIN %T p ON p.revisionID = r.id',
|
|
|
|
$path_table->getTableName());
|
|
|
|
}
|
|
|
|
|
2012-01-03 21:08:12 -08:00
|
|
|
if ($this->commitHashes) {
|
|
|
|
$joins[] = qsprintf(
|
|
|
|
$conn_r,
|
|
|
|
'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(
|
|
|
|
$conn_r,
|
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(
|
|
|
|
$conn_r,
|
2013-10-05 13:48:45 -07:00
|
|
|
'JOIN %T e_reviewers ON e_reviewers.src = r.phid '.
|
|
|
|
'AND e_reviewers.type = %s '.
|
|
|
|
'AND e_reviewers.dst in (%Ls)',
|
|
|
|
PhabricatorEdgeConfig::TABLE_NAME_EDGE,
|
2015-01-01 14:43:26 +11:00
|
|
|
DifferentialRevisionHasReviewerEdgeType::EDGECONST,
|
2011-12-06 14:21:36 -08:00
|
|
|
$this->reviewers);
|
|
|
|
}
|
|
|
|
|
2014-02-18 16:25:16 -08:00
|
|
|
if ($this->draftAuthors) {
|
|
|
|
$joins[] = qsprintf(
|
|
|
|
$conn_r,
|
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);
|
|
|
|
}
|
|
|
|
|
2015-01-30 11:51:16 -08:00
|
|
|
if ($this->commitPHIDs) {
|
|
|
|
$joins[] = qsprintf(
|
|
|
|
$conn_r,
|
|
|
|
'JOIN %T commits ON commits.revisionID = r.id',
|
|
|
|
DifferentialRevision::TABLE_COMMIT);
|
|
|
|
}
|
|
|
|
|
2015-04-18 11:18:27 -07:00
|
|
|
$joins[] = $this->buildJoinClauseParts($conn_r);
|
2011-10-02 12:03:16 -07:00
|
|
|
|
2015-04-18 11:18:27 -07:00
|
|
|
return $this->formatJoinClause($joins);
|
2011-10-02 12:03:16 -07:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
* @task internal
|
|
|
|
*/
|
Make buildWhereClause() a method of AphrontCursorPagedPolicyAwareQuery
Summary:
Ref T4100. Ref T5595.
To support a unified "Projects:" query across all applications, a future diff is going to add a set of "Edge Logic" capabilities to `PolicyAwareQuery` which write the required SELECT, JOIN, WHERE, HAVING and GROUP clauses for you.
With the addition of "Edge Logic", we'll have three systems which may need to build components of query claues: ordering/paging, customfields/applicationsearch, and edge logic.
For most clauses, queries don't currently call into the parent explicitly to get default components. I want to move more query construction logic up the class tree so it can be shared.
For most methods, this isn't a problem, but many subclasses define a `buildWhereClause()`. Make all such definitions protected and consistent.
This causes no behavioral changes.
Test Plan: Ran `arc unit --everything`, which does a pretty through job of verifying this statically.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: yelirekim, hach-que, epriestley
Maniphest Tasks: T4100, T5595
Differential Revision: https://secure.phabricator.com/D12453
2015-04-18 07:08:30 -07:00
|
|
|
protected function buildWhereClause(AphrontDatabaseConnection $conn_r) {
|
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(
|
|
|
|
$conn_r,
|
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'));
|
|
|
|
}
|
|
|
|
$path_clauses = '('.implode(' OR ', $path_clauses).')';
|
|
|
|
$where[] = $path_clauses;
|
|
|
|
}
|
|
|
|
|
2011-12-06 14:21:36 -08:00
|
|
|
if ($this->authors) {
|
2011-12-01 16:57:06 -08:00
|
|
|
$where[] = qsprintf(
|
|
|
|
$conn_r,
|
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(
|
|
|
|
$conn_r,
|
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(
|
|
|
|
$conn_r,
|
|
|
|
'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(
|
|
|
|
$conn_r,
|
|
|
|
'(hash_rel.type = %s AND hash_rel.hash = %s)',
|
|
|
|
$type,
|
|
|
|
$hash);
|
|
|
|
}
|
|
|
|
$hash_clauses = '('.implode(' OR ', $hash_clauses).')';
|
|
|
|
$where[] = $hash_clauses;
|
|
|
|
}
|
|
|
|
|
2015-01-30 11:51:16 -08:00
|
|
|
if ($this->commitPHIDs) {
|
|
|
|
$where[] = qsprintf(
|
|
|
|
$conn_r,
|
|
|
|
'commits.commitPHID IN (%Ls)',
|
|
|
|
$this->commitPHIDs);
|
|
|
|
}
|
|
|
|
|
2011-12-01 16:57:06 -08:00
|
|
|
if ($this->phids) {
|
|
|
|
$where[] = qsprintf(
|
|
|
|
$conn_r,
|
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(
|
|
|
|
$conn_r,
|
|
|
|
'r.branchName in (%Ls)',
|
|
|
|
$this->branches);
|
|
|
|
}
|
|
|
|
|
2015-03-25 10:21:56 -07:00
|
|
|
if ($this->updatedEpochMin !== null) {
|
|
|
|
$where[] = qsprintf(
|
|
|
|
$conn_r,
|
|
|
|
'r.dateModified >= %d',
|
|
|
|
$this->updatedEpochMin);
|
|
|
|
}
|
|
|
|
|
|
|
|
if ($this->updatedEpochMax !== null) {
|
|
|
|
$where[] = qsprintf(
|
|
|
|
$conn_r,
|
|
|
|
'r.dateModified <= %d',
|
|
|
|
$this->updatedEpochMax);
|
|
|
|
}
|
|
|
|
|
2015-06-17 11:25:01 -07:00
|
|
|
// NOTE: Although the status constants are integers in PHP, the column is a
|
|
|
|
// string column in MySQL, and MySQL won't use keys on string columns if
|
|
|
|
// you put integers in the query.
|
|
|
|
|
2011-10-02 12:03:16 -07:00
|
|
|
switch ($this->status) {
|
|
|
|
case self::STATUS_ANY:
|
|
|
|
break;
|
|
|
|
case self::STATUS_OPEN:
|
|
|
|
$where[] = qsprintf(
|
|
|
|
$conn_r,
|
2015-06-17 11:25:01 -07:00
|
|
|
'r.status IN (%Ls)',
|
2013-11-25 17:39:24 -08:00
|
|
|
DifferentialRevisionStatus::getOpenStatuses());
|
2011-10-02 12:03:16 -07:00
|
|
|
break;
|
2012-06-28 15:26:00 -07:00
|
|
|
case self::STATUS_NEEDS_REVIEW:
|
|
|
|
$where[] = qsprintf(
|
|
|
|
$conn_r,
|
2015-06-17 11:25:01 -07:00
|
|
|
'r.status IN (%Ls)',
|
2012-06-28 15:26:00 -07:00
|
|
|
array(
|
2014-07-23 10:03:09 +10:00
|
|
|
ArcanistDifferentialRevisionStatus::NEEDS_REVIEW,
|
2012-06-28 15:26:00 -07:00
|
|
|
));
|
|
|
|
break;
|
2013-08-07 14:53:49 +01:00
|
|
|
case self::STATUS_NEEDS_REVISION:
|
|
|
|
$where[] = qsprintf(
|
|
|
|
$conn_r,
|
2015-06-17 11:25:01 -07:00
|
|
|
'r.status IN (%Ls)',
|
2013-08-07 14:53:49 +01:00
|
|
|
array(
|
2014-07-23 10:03:09 +10:00
|
|
|
ArcanistDifferentialRevisionStatus::NEEDS_REVISION,
|
2013-08-07 14:53:49 +01:00
|
|
|
));
|
|
|
|
break;
|
2012-01-25 14:50:34 -08:00
|
|
|
case self::STATUS_ACCEPTED:
|
|
|
|
$where[] = qsprintf(
|
|
|
|
$conn_r,
|
2015-06-17 11:25:01 -07:00
|
|
|
'r.status IN (%Ls)',
|
2012-01-25 14:50:34 -08:00
|
|
|
array(
|
|
|
|
ArcanistDifferentialRevisionStatus::ACCEPTED,
|
|
|
|
));
|
|
|
|
break;
|
2012-04-23 17:40:57 -07:00
|
|
|
case self::STATUS_CLOSED:
|
2012-01-12 16:56:07 -08:00
|
|
|
$where[] = qsprintf(
|
|
|
|
$conn_r,
|
2015-06-17 11:25:01 -07:00
|
|
|
'r.status IN (%Ls)',
|
2013-11-25 17:39:24 -08:00
|
|
|
DifferentialRevisionStatus::getClosedStatuses());
|
2012-01-12 16:56:07 -08:00
|
|
|
break;
|
2012-03-06 16:42:41 -08:00
|
|
|
case self::STATUS_ABANDONED:
|
|
|
|
$where[] = qsprintf(
|
|
|
|
$conn_r,
|
2015-06-17 11:25:01 -07:00
|
|
|
'r.status IN (%Ls)',
|
2012-03-06 16:42:41 -08:00
|
|
|
array(
|
|
|
|
ArcanistDifferentialRevisionStatus::ABANDONED,
|
|
|
|
));
|
|
|
|
break;
|
2011-10-02 12:03:16 -07:00
|
|
|
default:
|
|
|
|
throw new Exception(
|
2015-05-22 17:27:56 +10:00
|
|
|
pht("Unknown revision status filter constant '%s'!", $this->status));
|
2011-10-02 12:03:16 -07:00
|
|
|
}
|
|
|
|
|
2015-04-18 11:18:27 -07:00
|
|
|
$where[] = $this->buildWhereClauseParts($conn_r);
|
2013-07-03 05:43:52 -07:00
|
|
|
return $this->formatWhereClause($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() {
|
|
|
|
|
2011-12-07 08:30:49 -08:00
|
|
|
$join_triggers = array_merge(
|
|
|
|
$this->pathIDs,
|
|
|
|
$this->ccs,
|
2014-02-12 08:53:40 -08:00
|
|
|
$this->reviewers);
|
2011-12-07 08:30:49 -08: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
|
|
|
if (count($join_triggers) > 1) {
|
|
|
|
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
|
|
|
|
|
|
|
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() {
|
|
|
|
$primary = ($this->buildingGlobalOrder ? null : 'r');
|
2013-09-13 06:23:09 -07:00
|
|
|
|
2015-04-11 20:26:20 -07:00
|
|
|
return array(
|
|
|
|
'id' => array(
|
|
|
|
'table' => $primary,
|
|
|
|
'column' => 'id',
|
|
|
|
'type' => 'int',
|
|
|
|
'unique' => true,
|
|
|
|
),
|
|
|
|
'updated' => array(
|
|
|
|
'table' => $primary,
|
|
|
|
'column' => 'dateModified',
|
|
|
|
'type' => 'int',
|
|
|
|
),
|
2013-09-13 06:23:09 -07:00
|
|
|
);
|
2013-07-03 05:45:07 -07:00
|
|
|
}
|
|
|
|
|
2015-04-11 20:26:20 -07:00
|
|
|
protected function getPagingValueMap($cursor, array $keys) {
|
|
|
|
$revision = $this->loadCursorObject($cursor);
|
|
|
|
return array(
|
|
|
|
'id' => $revision->getID(),
|
|
|
|
'updated' => $revision->getDateModified(),
|
|
|
|
);
|
2011-10-02 12:03:16 -07:00
|
|
|
}
|
|
|
|
|
2011-12-20 17:05:52 -08:00
|
|
|
private function loadRelationships($conn_r, array $revisions) {
|
2012-04-04 13:13:08 -07:00
|
|
|
assert_instances_of($revisions, 'DifferentialRevision');
|
2013-10-05 13:48:45 -07:00
|
|
|
|
2015-01-01 14:43:26 +11:00
|
|
|
$type_reviewer = DifferentialRevisionHasReviewerEdgeType::EDGECONST;
|
2015-01-03 10:33:25 +11:00
|
|
|
$type_subscriber = PhabricatorObjectHasSubscriberEdgeType::EDGECONST;
|
2014-02-12 08:53:40 -08:00
|
|
|
|
2013-10-05 13:48:45 -07:00
|
|
|
$edges = id(new PhabricatorEdgeQuery())
|
|
|
|
->withSourcePHIDs(mpull($revisions, 'getPHID'))
|
2014-02-12 08:53:40 -08:00
|
|
|
->withEdgeTypes(array($type_reviewer, $type_subscriber))
|
2013-10-04 19:57:15 -07:00
|
|
|
->setOrder(PhabricatorEdgeQuery::ORDER_OLDEST_FIRST)
|
2013-10-05 13:48:45 -07:00
|
|
|
->execute();
|
|
|
|
|
2014-02-12 08:53:40 -08:00
|
|
|
$type_map = array(
|
|
|
|
DifferentialRevision::RELATION_REVIEWER => $type_reviewer,
|
|
|
|
DifferentialRevision::RELATION_SUBSCRIBED => $type_subscriber,
|
|
|
|
);
|
|
|
|
|
2011-12-20 17:05:52 -08:00
|
|
|
foreach ($revisions as $revision) {
|
2014-02-12 08:53:40 -08:00
|
|
|
$data = array();
|
|
|
|
foreach ($type_map as $rel_type => $edge_type) {
|
|
|
|
$revision_edges = $edges[$revision->getPHID()][$edge_type];
|
|
|
|
foreach ($revision_edges as $dst_phid => $edge_data) {
|
|
|
|
$data[] = array(
|
|
|
|
'relation' => $rel_type,
|
|
|
|
'objectPHID' => $dst_phid,
|
|
|
|
'reasonPHID' => null,
|
|
|
|
);
|
|
|
|
}
|
2013-10-05 13:48:45 -07:00
|
|
|
}
|
|
|
|
|
|
|
|
$revision->attachRelationships($data);
|
2011-12-20 17:05:52 -08:00
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
private function loadCommitPHIDs($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
|
|
|
$commit_phids = queryfx_all(
|
|
|
|
$conn_r,
|
|
|
|
'SELECT * FROM %T WHERE revisionID IN (%Ld)',
|
|
|
|
DifferentialRevision::TABLE_COMMIT,
|
|
|
|
mpull($revisions, 'getID'));
|
|
|
|
$commit_phids = igroup($commit_phids, 'revisionID');
|
|
|
|
foreach ($revisions as $revision) {
|
|
|
|
$phids = idx($commit_phids, $revision->getID(), array());
|
|
|
|
$phids = ipull($phids, 'commitPHID');
|
|
|
|
$revision->attachCommitPHIDs($phids);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
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(
|
|
|
|
AphrontDatabaseConnection $conn_r,
|
|
|
|
array $revisions) {
|
|
|
|
|
|
|
|
assert_instances_of($revisions, 'DifferentialRevision');
|
2015-01-01 14:43:26 +11:00
|
|
|
$edge_type = DifferentialRevisionHasReviewerEdgeType::EDGECONST;
|
2013-07-14 19:18:55 -07:00
|
|
|
|
|
|
|
$edges = id(new PhabricatorEdgeQuery())
|
|
|
|
->withSourcePHIDs(mpull($revisions, 'getPHID'))
|
|
|
|
->withEdgeTypes(array($edge_type))
|
|
|
|
->needEdgeData(true)
|
2013-10-04 19:57:15 -07:00
|
|
|
->setOrder(PhabricatorEdgeQuery::ORDER_OLDEST_FIRST)
|
2013-07-14 19:18:55 -07:00
|
|
|
->execute();
|
|
|
|
|
2013-10-06 17:08:14 -07:00
|
|
|
$viewer = $this->getViewer();
|
|
|
|
$viewer_phid = $viewer->getPHID();
|
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,
|
|
|
|
$edges,
|
|
|
|
$allow_self);
|
|
|
|
}
|
|
|
|
|
2013-07-14 19:18:55 -07:00
|
|
|
foreach ($revisions as $revision) {
|
|
|
|
$revision_edges = $edges[$revision->getPHID()][$edge_type];
|
|
|
|
$reviewers = array();
|
2013-10-06 17:08:14 -07:00
|
|
|
foreach ($revision_edges as $reviewer_phid => $edge) {
|
2016-12-13 10:14:26 -08:00
|
|
|
$reviewer = new DifferentialReviewerProxy(
|
|
|
|
$reviewer_phid,
|
|
|
|
$edge['data']);
|
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
|
|
|
}
|
|
|
|
|
|
|
|
$revision->attachReviewerStatus($reviewers);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
2013-10-06 17:08:14 -07:00
|
|
|
private function loadReviewerAuthority(
|
|
|
|
array $revisions,
|
|
|
|
array $edges,
|
|
|
|
$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;
|
|
|
|
|
2015-01-01 14:43:26 +11:00
|
|
|
$edge_type = DifferentialRevisionHasReviewerEdgeType::EDGECONST;
|
2013-10-06 17:08:14 -07:00
|
|
|
foreach ($edges as $src => $types) {
|
|
|
|
if (!$allow_self) {
|
|
|
|
if ($revision_map[$src]->getAuthorPHID() == $viewer_phid) {
|
|
|
|
// 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;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
$edge_data = idx($types, $edge_type, array());
|
|
|
|
foreach ($edge_data as $dst => $data) {
|
2016-05-13 07:23:13 -07:00
|
|
|
$phid_type = phid_get_type($dst);
|
|
|
|
if ($phid_type == $project_type) {
|
2013-10-06 17:08:14 -07:00
|
|
|
$project_phids[] = $dst;
|
|
|
|
}
|
2016-05-13 07:23:13 -07:00
|
|
|
if ($phid_type == $package_type) {
|
|
|
|
$package_phids[] = $dst;
|
|
|
|
}
|
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
|
|
|
}
|