From 10cdc55cf70b3bce0f9663901f257e3258b76327 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 14 Dec 2015 06:11:03 -0800 Subject: [PATCH] Give "owners.search" a "paths" attachment and a default "owners" value Summary: Ref T9964. - Add a "paths" attachment for fetching paths. - Always load owners. We will need this to do policy checks in the future, anyway, and this data is not large, is very useful, and is reasonable to load unconditionally. Test Plan: - Queried packages via API. - Edited packages (paths, owners). - Created a package. Reviewers: chad Reviewed By: chad Maniphest Tasks: T9964 Differential Revision: https://secure.phabricator.com/D14775 --- src/__phutil_library_map__.php | 2 + .../PhabricatorOwnersDetailController.php | 1 - .../PhabricatorOwnersPackageEditEngine.php | 3 +- ...bricatorOwnersPackageTransactionEditor.php | 9 ++--- ...catorOwnersPathsSearchEngineAttachment.php | 35 +++++++++++++++++ .../query/PhabricatorOwnersPackageQuery.php | 39 ++++++++----------- .../storage/PhabricatorOwnersPackage.php | 38 +++++++++++++++++- ...atorPasteContentSearchEngineAttachment.php | 2 +- 8 files changed, 95 insertions(+), 34 deletions(-) create mode 100644 src/applications/owners/engineextension/PhabricatorOwnersPathsSearchEngineAttachment.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 4a7b76ff43..60defb1e98 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2616,6 +2616,7 @@ phutil_register_library_map(array( 'PhabricatorOwnersPackageTransactionQuery' => 'applications/owners/query/PhabricatorOwnersPackageTransactionQuery.php', 'PhabricatorOwnersPath' => 'applications/owners/storage/PhabricatorOwnersPath.php', 'PhabricatorOwnersPathsController' => 'applications/owners/controller/PhabricatorOwnersPathsController.php', + 'PhabricatorOwnersPathsSearchEngineAttachment' => 'applications/owners/engineextension/PhabricatorOwnersPathsSearchEngineAttachment.php', 'PhabricatorOwnersSchemaSpec' => 'applications/owners/storage/PhabricatorOwnersSchemaSpec.php', 'PhabricatorOwnersSearchField' => 'applications/owners/searchfield/PhabricatorOwnersSearchField.php', 'PhabricatorPHDConfigOptions' => 'applications/config/option/PhabricatorPHDConfigOptions.php', @@ -6846,6 +6847,7 @@ phutil_register_library_map(array( 'PhabricatorOwnersPackageTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'PhabricatorOwnersPath' => 'PhabricatorOwnersDAO', 'PhabricatorOwnersPathsController' => 'PhabricatorOwnersController', + 'PhabricatorOwnersPathsSearchEngineAttachment' => 'PhabricatorSearchEngineAttachment', 'PhabricatorOwnersSchemaSpec' => 'PhabricatorConfigSchemaSpec', 'PhabricatorOwnersSearchField' => 'PhabricatorSearchTokenizerField', 'PhabricatorPHDConfigOptions' => 'PhabricatorApplicationConfigOptions', diff --git a/src/applications/owners/controller/PhabricatorOwnersDetailController.php b/src/applications/owners/controller/PhabricatorOwnersDetailController.php index 7dd4bdb6d6..6193e5b367 100644 --- a/src/applications/owners/controller/PhabricatorOwnersDetailController.php +++ b/src/applications/owners/controller/PhabricatorOwnersDetailController.php @@ -14,7 +14,6 @@ final class PhabricatorOwnersDetailController ->setViewer($viewer) ->withIDs(array($request->getURIData('id'))) ->needPaths(true) - ->needOwners(true) ->executeOne(); if (!$package) { return new Aphront404Response(); diff --git a/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php b/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php index 0294c66279..0b6b5988ec 100644 --- a/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php +++ b/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php @@ -18,8 +18,7 @@ final class PhabricatorOwnersPackageEditEngine } protected function newObjectQuery() { - return id(new PhabricatorOwnersPackageQuery()) - ->needOwners(true); + return id(new PhabricatorOwnersPackageQuery()); } protected function getObjectCreateTitleText($object) { diff --git a/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php b/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php index cb989f77cb..e267a0a0a9 100644 --- a/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php +++ b/src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php @@ -32,8 +32,7 @@ final class PhabricatorOwnersPackageTransactionEditor case PhabricatorOwnersPackageTransaction::TYPE_NAME: return $object->getName(); case PhabricatorOwnersPackageTransaction::TYPE_OWNERS: - // TODO: needOwners() this on the Query. - $phids = mpull($object->loadOwners(), 'getUserPHID'); + $phids = mpull($object->getOwners(), 'getUserPHID'); $phids = array_values($phids); return $phids; case PhabricatorOwnersPackageTransaction::TYPE_AUDITING: @@ -125,8 +124,7 @@ final class PhabricatorOwnersPackageTransactionEditor $old = $xaction->getOldValue(); $new = $xaction->getNewValue(); - // TODO: needOwners this - $owners = $object->loadOwners(); + $owners = $object->getOwners(); $owners = mpull($owners, null, 'getUserPHID'); $rem = array_diff($old, $new); @@ -222,8 +220,7 @@ final class PhabricatorOwnersPackageTransactionEditor } protected function getMailCC(PhabricatorLiskDAO $object) { - // TODO: needOwners() this - return mpull($object->loadOwners(), 'getUserPHID'); + return mpull($object->getOwners(), 'getUserPHID'); } protected function buildReplyHandler(PhabricatorLiskDAO $object) { diff --git a/src/applications/owners/engineextension/PhabricatorOwnersPathsSearchEngineAttachment.php b/src/applications/owners/engineextension/PhabricatorOwnersPathsSearchEngineAttachment.php new file mode 100644 index 0000000000..bce6a76b60 --- /dev/null +++ b/src/applications/owners/engineextension/PhabricatorOwnersPathsSearchEngineAttachment.php @@ -0,0 +1,35 @@ +needPaths(true); + } + + public function getAttachmentForObject($object, $data, $spec) { + $paths = $object->getPaths(); + + $list = array(); + foreach ($paths as $path) { + $list[] = array( + 'repositoryPHID' => $path->getRepositoryPHID(), + 'path' => $path->getPath(), + 'isExcluded' => (bool)$path->getExcluded(), + ); + } + + return array( + 'paths' => $list, + ); + } + +} diff --git a/src/applications/owners/query/PhabricatorOwnersPackageQuery.php b/src/applications/owners/query/PhabricatorOwnersPackageQuery.php index ef6a1e1a67..4c2569ad5d 100644 --- a/src/applications/owners/query/PhabricatorOwnersPackageQuery.php +++ b/src/applications/owners/query/PhabricatorOwnersPackageQuery.php @@ -16,7 +16,6 @@ final class PhabricatorOwnersPackageQuery private $controlResults; private $needPaths; - private $needOwners; /** @@ -89,11 +88,6 @@ final class PhabricatorOwnersPackageQuery return $this; } - public function needOwners($need_owners) { - $this->needOwners = $need_owners; - return $this; - } - public function newResultObject() { return new PhabricatorOwnersPackage(); } @@ -103,13 +97,27 @@ final class PhabricatorOwnersPackageQuery } protected function loadPage() { - return $this->loadStandardPage(new PhabricatorOwnersPackage()); + return $this->loadStandardPage($this->newResultObject()); + } + + protected function willFilterPage(array $packages) { + $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())); + } + + return $packages; } protected function didFilterPage(array $packages) { - if ($this->needPaths) { - $package_ids = mpull($packages, 'getID'); + $package_ids = mpull($packages, 'getID'); + if ($this->needPaths) { $paths = id(new PhabricatorOwnersPath())->loadAllWhere( 'packageID IN (%Ld)', $package_ids); @@ -120,19 +128,6 @@ final class PhabricatorOwnersPackageQuery } } - 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'); } diff --git a/src/applications/owners/storage/PhabricatorOwnersPackage.php b/src/applications/owners/storage/PhabricatorOwnersPackage.php index 52b8529179..210bc2c855 100644 --- a/src/applications/owners/storage/PhabricatorOwnersPackage.php +++ b/src/applications/owners/storage/PhabricatorOwnersPackage.php @@ -273,6 +273,17 @@ final class PhabricatorOwnersPackage return mpull($this->getOwners(), 'getUserPHID'); } + public function isOwnerPHID($phid) { + if (!$phid) { + return false; + } + + $owner_phids = $this->getOwnerPHIDs(); + $owner_phids = array_fuse($owner_phids); + + return isset($owner_phids[$phid]); + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ @@ -290,11 +301,19 @@ final class PhabricatorOwnersPackage } public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { + switch ($capability) { + case PhabricatorPolicyCapability::CAN_VIEW: + if ($this->isOwnerPHID($viewer->getPHID())) { + return true; + } + break; + } + return false; } public function describeAutomaticCapability($capability) { - return null; + return pht('Owners of a package may always view it.'); } @@ -384,19 +403,34 @@ final class PhabricatorOwnersPackage 'type' => 'string', 'description' => pht('Active or archived status of the package.'), ), + 'owners' => array( + 'type' => 'list>', + 'description' => pht('List of package owners.'), + ), ); } public function getFieldValuesForConduit() { + $owner_list = array(); + foreach ($this->getOwners() as $owner) { + $owner_list[] = array( + 'ownerPHID' => $owner->getUserPHID(), + ); + } + return array( 'name' => $this->getName(), 'description' => $this->getDescription(), 'status' => $this->getStatus(), + 'owners' => $owner_list, ); } public function getConduitSearchAttachments() { - return array(); + return array( + id(new PhabricatorOwnersPathsSearchEngineAttachment()) + ->setAttachmentKey('paths'), + ); } } diff --git a/src/applications/paste/engineextension/PhabricatorPasteContentSearchEngineAttachment.php b/src/applications/paste/engineextension/PhabricatorPasteContentSearchEngineAttachment.php index 2d459e5501..34addc663d 100644 --- a/src/applications/paste/engineextension/PhabricatorPasteContentSearchEngineAttachment.php +++ b/src/applications/paste/engineextension/PhabricatorPasteContentSearchEngineAttachment.php @@ -17,7 +17,7 @@ final class PhabricatorPasteContentSearchEngineAttachment public function getAttachmentForObject($object, $data, $spec) { return array( - 'data' => $object->getRawContent(), + 'content' => $object->getRawContent(), ); } -- 2.51.2