From 5ca419174a6ca96316f91864b973f6c1679e9860 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 21 Jul 2013 07:03:10 -0700 Subject: [PATCH] Proide application-based handle loads; load Slowvote handles from the application Summary: Ref T2715. This is pretty straightforward, I think. Notes: - Long term, I want to replace `PhabricatorObjectHandleData` with `PhabricatorObjectQuery` and `PhabricatorHandleQuery`. The former's name is a relic of old Facebook stuff and unusual now that everything else uses normal queries. - I simplified the amount of work applications need to do in order to populate handles. The should just need to set names and URIs in most cases. Test Plan: Used `phid.lookup` and `phid.query` to load slowvote handles. Browsed around to load other handles. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T2715 Differential Revision: https://secure.phabricator.com/D6508 --- src/__phutil_library_map__.php | 3 + .../phid/PhabricatorObjectHandle.php | 34 ++++++++-- .../handle/PhabricatorObjectHandleData.php | 40 +++++------ .../phid/query/PhabricatorHandleQuery.php | 66 +++++++++++++++++++ .../phid/type/PhabricatorPHIDType.php | 34 +++++++++- .../phid/PhabricatorSlowvotePHIDTypePoll.php | 14 +++- 6 files changed, 161 insertions(+), 30 deletions(-) create mode 100644 src/applications/phid/query/PhabricatorHandleQuery.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 49a9294311..4fc0b5febb 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1166,6 +1166,7 @@ phutil_register_library_map(array( 'PhabricatorGlobalLock' => 'infrastructure/util/PhabricatorGlobalLock.php', 'PhabricatorGlobalUploadTargetView' => 'applications/files/view/PhabricatorGlobalUploadTargetView.php', 'PhabricatorHandleObjectSelectorDataView' => 'applications/phid/handle/view/PhabricatorHandleObjectSelectorDataView.php', + 'PhabricatorHandleQuery' => 'applications/phid/query/PhabricatorHandleQuery.php', 'PhabricatorHash' => 'infrastructure/util/PhabricatorHash.php', 'PhabricatorHashTestCase' => 'infrastructure/util/__tests__/PhabricatorHashTestCase.php', 'PhabricatorHeaderView' => 'view/layout/PhabricatorHeaderView.php', @@ -3144,6 +3145,7 @@ phutil_register_library_map(array( 'PhabricatorGestureExample' => 'PhabricatorUIExample', 'PhabricatorGlobalLock' => 'PhutilLock', 'PhabricatorGlobalUploadTargetView' => 'AphrontView', + 'PhabricatorHandleQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorHashTestCase' => 'PhabricatorTestCase', 'PhabricatorHeaderView' => 'AphrontView', 'PhabricatorHelpController' => 'PhabricatorController', @@ -3270,6 +3272,7 @@ phutil_register_library_map(array( 'PhabricatorOAuthServerTestCase' => 'PhabricatorTestCase', 'PhabricatorOAuthServerTestController' => 'PhabricatorOAuthServerController', 'PhabricatorOAuthServerTokenController' => 'PhabricatorAuthController', + 'PhabricatorObjectHandle' => 'PhabricatorPolicyInterface', 'PhabricatorObjectHandleStatus' => 'PhabricatorObjectHandleConstants', 'PhabricatorObjectItemListExample' => 'PhabricatorUIExample', 'PhabricatorObjectItemListView' => 'AphrontTagView', diff --git a/src/applications/phid/PhabricatorObjectHandle.php b/src/applications/phid/PhabricatorObjectHandle.php index 392a703572..2047efc206 100644 --- a/src/applications/phid/PhabricatorObjectHandle.php +++ b/src/applications/phid/PhabricatorObjectHandle.php @@ -1,6 +1,7 @@ name === null) { + return pht('Unknown Object (%s)', $this->getTypeName()); + } return $this->name; } @@ -99,9 +103,8 @@ final class PhabricatorObjectHandle { } public function getTypeName() { - $types = PhabricatorPHIDType::getAllTypes(); - if (isset($types[$this->getType()])) { - return $types[$this->getType()]->getTypeName(); + if ($this->getPHIDType()) { + return $this->getPHIDType()->getTypeName(); } static $map = array( @@ -223,4 +226,27 @@ final class PhabricatorObjectHandle { return $name; } + protected function getPHIDType() { + $types = PhabricatorPHIDType::getAllTypes(); + return idx($types, $this->getType()); + } + + +/* -( PhabricatorPolicyInterface )----------------------------------------- */ + + + public function getCapabilities() { + return array( + PhabricatorPolicyCapability::CAN_VIEW, + ); + } + + public function getPolicy($capability) { + return PhabricatorPolicies::POLICY_PUBLIC; + } + + public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { + return false; + } + } diff --git a/src/applications/phid/handle/PhabricatorObjectHandleData.php b/src/applications/phid/handle/PhabricatorObjectHandleData.php index b348850cc7..4cdf5ccc6a 100644 --- a/src/applications/phid/handle/PhabricatorObjectHandleData.php +++ b/src/applications/phid/handle/PhabricatorObjectHandleData.php @@ -241,9 +241,23 @@ final class PhabricatorObjectHandleData { } public function loadHandles() { - $all_objects = $this->loadObjects(); - $types = phid_group_by_type($this->phids); + $application_handles = id(new PhabricatorHandleQuery()) + ->setViewer($this->viewer) + ->withPHIDs($this->phids) + ->execute(); + + // TODO: Move the rest of this into Applications. + + $phid_map = array_fuse($this->phids); + foreach ($application_handles as $handle) { + if ($handle->isComplete()) { + unset($phid_map[$handle->getPHID()]); + } + } + + $all_objects = $this->loadObjects(); + $types = phid_group_by_type($phid_map); $handles = array(); @@ -674,26 +688,6 @@ final class PhabricatorObjectHandleData { } break; - - case PhabricatorSlowvotePHIDTypePoll::TYPECONST: - foreach ($phids as $phid) { - $handle = new PhabricatorObjectHandle(); - $handle->setPHID($phid); - $handle->setType($type); - if (empty($objects[$phid])) { - $handle->setName('Unknown Slowvote'); - } else { - $poll = $objects[$phid]; - $handle->setName('V'.$poll->getID()); - $handle->setFullName( - 'V'.$poll->getID().': '.$poll->getQuestion()); - $handle->setURI('/V'.$poll->getID()); - $handle->setComplete(true); - } - $handles[$phid] = $handle; - } - break; - case PhabricatorPHIDConstants::PHID_TYPE_MCRO: foreach ($phids as $phid) { $handle = new PhabricatorObjectHandle(); @@ -821,6 +815,6 @@ final class PhabricatorObjectHandleData { } } - return $handles; + return $handles + $application_handles; } } diff --git a/src/applications/phid/query/PhabricatorHandleQuery.php b/src/applications/phid/query/PhabricatorHandleQuery.php new file mode 100644 index 0000000000..3cdcfb8e5a --- /dev/null +++ b/src/applications/phid/query/PhabricatorHandleQuery.php @@ -0,0 +1,66 @@ +phids = $phids; + return $this; + } + + public function loadPage() { + $types = PhabricatorPHIDType::getAllTypes(); + + $phids = $this->phids; + if (!$phids) { + return array(); + } + + $objects = id(new PhabricatorObjectQuery()) + ->withPHIDs($phids) + ->setViewer($this->getViewer()) + ->execute(); + + $groups = array(); + foreach ($phids as $phid) { + $type = phid_get_type($phid); + $groups[$type][] = $phid; + } + + $results = array(); + foreach ($groups as $type => $phid_group) { + $handles = array(); + foreach ($phid_group as $key => $phid) { + if (isset($handles[$phid])) { + unset($phid_group[$key]); + // The input had a duplicate PHID; just skip it. + continue; + } + $handles[$phid] = id(new PhabricatorObjectHandle()) + ->setType($type) + ->setPHID($phid); + if (isset($objects[$phid])) { + $handles[$phid]->setComplete(true); + } + } + + if (isset($types[$type])) { + $type_objects = array_select_keys($objects, $phid_group); + if ($type_objects) { + $have_object_phids = array_keys($type_objects); + $types[$type]->loadHandles( + $this, + array_select_keys($handles, $have_object_phids), + $type_objects); + } + } + + $results += $handles; + } + + return $results; + } + +} diff --git a/src/applications/phid/type/PhabricatorPHIDType.php b/src/applications/phid/type/PhabricatorPHIDType.php index bae42fd53d..47e1351922 100644 --- a/src/applications/phid/type/PhabricatorPHIDType.php +++ b/src/applications/phid/type/PhabricatorPHIDType.php @@ -12,7 +12,39 @@ abstract class PhabricatorPHIDType { abstract public function loadObjects( PhabricatorObjectQuery $query, array $phids); - abstract public function loadHandles(array $phids, array $objects); + + + /** + * Populate provided handles with application-specific data, like titles and + * URIs. + * + * NOTE: The `$handles` and `$objects` lists are guaranteed to be nonempty + * and have the same keys: subclasses are expected to load information only + * for handles with visible objects. + * + * Because of this guarantee, a safe implementation will typically look like* + * + * foreach ($handles as $phid => $handle) { + * $object = $objects[$phid]; + * + * $handle->setStuff($object->getStuff()); + * // ... + * } + * + * In general, an implementation should call `setName()` and `setURI()` on + * each handle at a minimum. See @{class:PhabricatorObjectHandle} for other + * handle properties. + * + * @param PhabricatorHandleQuery Issuing query object. + * @param list Handles to populate with data. + * @param list Objects for these PHIDs loaded by + * @{method:loadObjects()}. + * @return void + */ + abstract public function loadHandles( + PhabricatorHandleQuery $query, + array $handles, + array $objects); public function canLoadNamedObject($name) { return false; diff --git a/src/applications/slowvote/phid/PhabricatorSlowvotePHIDTypePoll.php b/src/applications/slowvote/phid/PhabricatorSlowvotePHIDTypePoll.php index 56470d07af..6eaf8d3230 100644 --- a/src/applications/slowvote/phid/PhabricatorSlowvotePHIDTypePoll.php +++ b/src/applications/slowvote/phid/PhabricatorSlowvotePHIDTypePoll.php @@ -26,8 +26,18 @@ final class PhabricatorSlowvotePHIDTypePoll extends PhabricatorPHIDType { ->execute(); } - public function loadHandles(array $phids, array $objects) { - throw new Exception("TODO"); + public function loadHandles( + PhabricatorHandleQuery $query, + array $handles, + array $objects) { + + foreach ($handles as $phid => $handle) { + $poll = $objects[$phid]; + + $handle->setName('V'.$poll->getID()); + $handle->setFullName('V'.$poll->getID().': '.$poll->getQuestion()); + $handle->setURI('/V'.$poll->getID()); + } } public function canLoadNamedObject($name) {