Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15312060
D13922.id33598.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
12 KB
Referenced Files
None
Subscribers
None
D13922.id33598.diff
View Options
diff --git a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php
--- a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php
+++ b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php
@@ -20,7 +20,7 @@
$owned_packages = id(new PhabricatorOwnersPackageQuery())
->setViewer($user)
- ->withOwnerPHIDs(array($user->getPHID()))
+ ->withAuthorityPHIDs(array($user->getPHID()))
->execute();
foreach ($owned_packages as $package) {
$phids[$package->getPHID()] = true;
diff --git a/src/applications/owners/conduit/OwnersQueryConduitAPIMethod.php b/src/applications/owners/conduit/OwnersQueryConduitAPIMethod.php
--- a/src/applications/owners/conduit/OwnersQueryConduitAPIMethod.php
+++ b/src/applications/owners/conduit/OwnersQueryConduitAPIMethod.php
@@ -141,7 +141,7 @@
$query = id(new PhabricatorOwnersPackageQuery())
->setViewer($request->getUser());
- $query->withOwnerPHIDs(array($request->getValue('userAffiliated')));
+ $query->withAuthorityPHIDs(array($request->getValue('userAffiliated')));
$packages = $query->execute();
} else if ($is_owner_query) {
diff --git a/src/applications/owners/controller/PhabricatorOwnersDetailController.php b/src/applications/owners/controller/PhabricatorOwnersDetailController.php
--- a/src/applications/owners/controller/PhabricatorOwnersDetailController.php
+++ b/src/applications/owners/controller/PhabricatorOwnersDetailController.php
@@ -14,6 +14,7 @@
->setViewer($viewer)
->withIDs(array($request->getURIData('id')))
->needPaths(true)
+ ->needOwners(true)
->executeOne();
if (!$package) {
return new Aphront404Response();
@@ -150,8 +151,7 @@
$view = id(new PHUIPropertyListView())
->setUser($viewer);
- // TODO: needOwners() this on the Query.
- $owners = $package->loadOwners();
+ $owners = $package->getOwners();
if ($owners) {
$owner_list = $viewer->renderHandleList(mpull($owners, 'getUserPHID'));
} else {
diff --git a/src/applications/owners/controller/PhabricatorOwnersEditController.php b/src/applications/owners/controller/PhabricatorOwnersEditController.php
--- a/src/applications/owners/controller/PhabricatorOwnersEditController.php
+++ b/src/applications/owners/controller/PhabricatorOwnersEditController.php
@@ -17,6 +17,7 @@
// TODO: Support this capability.
// PhabricatorPolicyCapability::CAN_EDIT,
))
+ ->needOwners(true)
->executeOne();
if (!$package) {
return new Aphront404Response();
@@ -30,8 +31,7 @@
$e_name = true;
$v_name = $package->getName();
- // TODO: Pull these off needOwners() on the Query.
- $v_owners = mpull($package->loadOwners(), 'getUserPHID');
+ $v_owners = mpull($package->getOwners(), 'getUserPHID');
$v_auditing = $package->getAuditingEnabled();
$v_description = $package->getDescription();
diff --git a/src/applications/owners/query/PhabricatorOwnersPackageQuery.php b/src/applications/owners/query/PhabricatorOwnersPackageQuery.php
--- a/src/applications/owners/query/PhabricatorOwnersPackageQuery.php
+++ b/src/applications/owners/query/PhabricatorOwnersPackageQuery.php
@@ -6,21 +6,37 @@
private $ids;
private $phids;
private $ownerPHIDs;
+ private $authorityPHIDs;
private $repositoryPHIDs;
+ private $paths;
private $namePrefix;
- private $needPaths;
private $controlMap = array();
private $controlResults;
+ private $needPaths;
+ private $needOwners;
+
+
/**
- * Owners are direct owners, and members of owning projects.
+ * Query owner PHIDs exactly. This does not expand authorities, so a user
+ * PHID will not match projects the user is a member of.
*/
public function withOwnerPHIDs(array $phids) {
$this->ownerPHIDs = $phids;
return $this;
}
+ /**
+ * Query owner authority. This will expand authorities, so a user PHID will
+ * match both packages they own directly and packages owned by a project they
+ * are a member of.
+ */
+ public function withAuthorityPHIDs(array $phids) {
+ $this->authorityPHIDs = $phids;
+ return $this;
+ }
+
public function withPHIDs(array $phids) {
$this->phids = $phids;
return $this;
@@ -36,6 +52,11 @@
return $this;
}
+ public function withPaths(array $paths) {
+ $this->paths = $paths;
+ return $this;
+ }
+
public function withControl($repository_phid, array $paths) {
if (empty($this->controlMap[$repository_phid])) {
$this->controlMap[$repository_phid] = array();
@@ -62,6 +83,11 @@
return $this;
}
+ public function needOwners($need_owners) {
+ $this->needOwners = $need_owners;
+ return $this;
+ }
+
public function newResultObject() {
return new PhabricatorOwnersPackage();
}
@@ -88,6 +114,19 @@
}
}
+ if ($this->needOwners) {
+ $package_ids = mpull($packages, 'getID');
+
+ $owners = id(new PhabricatorOwnersOwner())->loadAllWhere(
+ 'packageID IN (%Ld)',
+ $package_ids);
+ $owners = mgroup($owners, 'getPackageID');
+
+ foreach ($packages as $package) {
+ $package->attachOwners(idx($owners, $package->getID(), array()));
+ }
+ }
+
if ($this->controlMap) {
$this->controlResults += mpull($packages, null, 'getID');
}
@@ -98,14 +137,14 @@
protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) {
$joins = parent::buildJoinClauseParts($conn);
- if ($this->ownerPHIDs !== null) {
+ if ($this->shouldJoinOwnersTable()) {
$joins[] = qsprintf(
$conn,
'JOIN %T o ON o.packageID = p.id',
id(new PhabricatorOwnersOwner())->getTableName());
}
- if ($this->shouldJoinOwnersPathTable()) {
+ if ($this->shouldJoinPathTable()) {
$joins[] = qsprintf(
$conn,
'JOIN %T rpath ON rpath.packageID = p.id',
@@ -139,21 +178,26 @@
$this->repositoryPHIDs);
}
- if ($this->ownerPHIDs !== null) {
- $base_phids = $this->ownerPHIDs;
-
- $projects = id(new PhabricatorProjectQuery())
- ->setViewer($this->getViewer())
- ->withMemberPHIDs($base_phids)
- ->execute();
- $project_phids = mpull($projects, 'getPHID');
-
- $all_phids = array_merge($base_phids, $project_phids);
+ if ($this->authorityPHIDs !== null) {
+ $authority_phids = $this->expandAuthority($this->authorityPHIDs);
+ $where[] = qsprintf(
+ $conn,
+ 'o.userPHID IN (%Ls)',
+ $authority_phids);
+ }
+ if ($this->ownerPHIDs !== null) {
$where[] = qsprintf(
$conn,
'o.userPHID IN (%Ls)',
- $all_phids);
+ $this->ownerPHIDs);
+ }
+
+ if ($this->paths !== null) {
+ $where[] = qsprintf(
+ $conn,
+ 'rpath.path IN (%Ls)',
+ $this->getFragmentsForPaths($this->paths));
}
if (strlen($this->namePrefix)) {
@@ -168,12 +212,7 @@
if ($this->controlMap) {
$clauses = array();
foreach ($this->controlMap as $repository_phid => $paths) {
- $fragments = array();
- foreach ($paths as $path) {
- foreach (PhabricatorOwnersPackage::splitPath($path) as $fragment) {
- $fragments[$fragment] = $fragment;
- }
- }
+ $fragments = $this->getFragmentsForPaths($paths);
$clauses[] = qsprintf(
$conn,
@@ -188,11 +227,11 @@
}
protected function shouldGroupQueryResultRows() {
- if ($this->shouldJoinOwnersPathTable()) {
+ if ($this->shouldJoinOwnersTable()) {
return true;
}
- if ($this->ownerPHIDs) {
+ if ($this->shouldJoinPathTable()) {
return true;
}
@@ -236,11 +275,27 @@
return 'p';
}
- private function shouldJoinOwnersPathTable() {
+ private function shouldJoinOwnersTable() {
+ if ($this->ownerPHIDs !== null) {
+ return true;
+ }
+
+ if ($this->authorityPHIDs !== null) {
+ return true;
+ }
+
+ return false;
+ }
+
+ private function shouldJoinPathTable() {
if ($this->repositoryPHIDs !== null) {
return true;
}
+ if ($this->paths !== null) {
+ return true;
+ }
+
if ($this->controlMap) {
return true;
}
@@ -248,6 +303,28 @@
return false;
}
+ private function expandAuthority(array $phids) {
+ $projects = id(new PhabricatorProjectQuery())
+ ->setViewer($this->getViewer())
+ ->withMemberPHIDs($phids)
+ ->execute();
+ $project_phids = mpull($projects, 'getPHID');
+
+ return array_fuse($phids) + array_fuse($project_phids);
+ }
+
+ private function getFragmentsForPaths(array $paths) {
+ $fragments = array();
+
+ foreach ($paths as $path) {
+ foreach (PhabricatorOwnersPackage::splitPath($path) as $fragment) {
+ $fragments[$fragment] = $fragment;
+ }
+ }
+
+ return $fragments;
+ }
+
/* -( Path Control )------------------------------------------------------- */
diff --git a/src/applications/owners/query/PhabricatorOwnersPackageSearchEngine.php b/src/applications/owners/query/PhabricatorOwnersPackageSearchEngine.php
--- a/src/applications/owners/query/PhabricatorOwnersPackageSearchEngine.php
+++ b/src/applications/owners/query/PhabricatorOwnersPackageSearchEngine.php
@@ -18,29 +18,37 @@
protected function buildCustomSearchFields() {
return array(
id(new PhabricatorSearchDatasourceField())
- ->setLabel(pht('Owners'))
- ->setKey('ownerPHIDs')
- ->setAliases(array('owner', 'owners'))
+ ->setLabel(pht('Authority'))
+ ->setKey('authorityPHIDs')
+ ->setAliases(array('authority', 'authorities'))
->setDatasource(new PhabricatorProjectOrUserDatasource()),
id(new PhabricatorSearchDatasourceField())
->setLabel(pht('Repositories'))
->setKey('repositoryPHIDs')
->setAliases(array('repository', 'repositories'))
->setDatasource(new DiffusionRepositoryDatasource()),
+ id(new PhabricatorSearchStringListField())
+ ->setLabel(pht('Paths'))
+ ->setKey('paths')
+ ->setAliases(array('path')),
);
}
protected function buildQueryFromParameters(array $map) {
$query = $this->newQuery();
- if ($map['ownerPHIDs']) {
- $query->withOwnerPHIDs($map['ownerPHIDs']);
+ if ($map['authorityPHIDs']) {
+ $query->withAuthorityPHIDs($map['authorityPHIDs']);
}
if ($map['repositoryPHIDs']) {
$query->withRepositoryPHIDs($map['repositoryPHIDs']);
}
+ if ($map['paths']) {
+ $query->withPaths($map['paths']);
+ }
+
return $query;
}
@@ -52,7 +60,7 @@
$names = array();
if ($this->requireViewer()->isLoggedIn()) {
- $names['owned'] = pht('Owned');
+ $names['authority'] = pht('Owned');
}
$names += array(
@@ -69,9 +77,9 @@
switch ($query_key) {
case 'all':
return $query;
- case 'owned':
+ case 'authority':
return $query->setParameter(
- 'ownerPHIDs',
+ 'authorityPHIDs',
array($this->requireViewer()->getPHID()));
}
diff --git a/src/applications/owners/storage/PhabricatorOwnersPackage.php b/src/applications/owners/storage/PhabricatorOwnersPackage.php
--- a/src/applications/owners/storage/PhabricatorOwnersPackage.php
+++ b/src/applications/owners/storage/PhabricatorOwnersPackage.php
@@ -14,10 +14,13 @@
protected $mailKey;
private $paths = self::ATTACHABLE;
+ private $owners = self::ATTACHABLE;
public static function initializeNewPackage(PhabricatorUser $actor) {
return id(new PhabricatorOwnersPackage())
- ->setAuditingEnabled(0);
+ ->setAuditingEnabled(0)
+ ->attachPaths(array())
+ ->attachOwners(array());
}
public function getCapabilities() {
@@ -249,6 +252,16 @@
return $this->assertAttached($this->paths);
}
+ public function attachOwners(array $owners) {
+ assert_instances_of($owners, 'PhabricatorOwnersOwner');
+ $this->owners = $owners;
+ return $this;
+ }
+
+ public function getOwners() {
+ return $this->assertAttached($this->owners);
+ }
+
/* -( PhabricatorApplicationTransactionInterface )------------------------- */
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Fri, Mar 7, 1:31 PM (4 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7332751
Default Alt Text
D13922.id33598.diff (12 KB)
Attached To
Mode
D13922: Further modernize OwnersPackageQuery
Attached
Detach File
Event Timeline
Log In to Comment