diff --git a/resources/celerity/map.php b/resources/celerity/map.php --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -45,7 +45,7 @@ 'rsrc/css/application/config/setup-issue.css' => '22270af2', 'rsrc/css/application/config/unhandled-exception.css' => '37d4f9a2', 'rsrc/css/application/conpherence/durable-column.css' => 'e2011d85', - 'rsrc/css/application/conpherence/menu.css' => '9b37a261', + 'rsrc/css/application/conpherence/menu.css' => '2c1c727c', 'rsrc/css/application/conpherence/message-pane.css' => '44154798', 'rsrc/css/application/conpherence/notification.css' => '04a6e10a', 'rsrc/css/application/conpherence/update.css' => '1099a660', @@ -515,7 +515,7 @@ 'config-options-css' => '7fedf08b', 'config-welcome-css' => '6abd79be', 'conpherence-durable-column-view' => 'e2011d85', - 'conpherence-menu-css' => '9b37a261', + 'conpherence-menu-css' => '2c1c727c', 'conpherence-message-pane-css' => '44154798', 'conpherence-notification-css' => '04a6e10a', 'conpherence-thread-manager' => 'bb928342', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1873,7 +1873,10 @@ 'PhabricatorGlobalLock' => 'infrastructure/util/PhabricatorGlobalLock.php', 'PhabricatorGlobalUploadTargetView' => 'applications/files/view/PhabricatorGlobalUploadTargetView.php', 'PhabricatorGoogleAuthProvider' => 'applications/auth/provider/PhabricatorGoogleAuthProvider.php', + 'PhabricatorHandleList' => 'applications/phid/handle/pool/PhabricatorHandleList.php', 'PhabricatorHandleObjectSelectorDataView' => 'applications/phid/handle/view/PhabricatorHandleObjectSelectorDataView.php', + 'PhabricatorHandlePool' => 'applications/phid/handle/pool/PhabricatorHandlePool.php', + 'PhabricatorHandlePoolTestCase' => 'applications/phid/handle/pool/__tests__/PhabricatorHandlePoolTestCase.php', 'PhabricatorHandleQuery' => 'applications/phid/query/PhabricatorHandleQuery.php', 'PhabricatorHarbormasterApplication' => 'applications/harbormaster/application/PhabricatorHarbormasterApplication.php', 'PhabricatorHarbormasterConfigOptions' => 'applications/harbormaster/config/PhabricatorHarbormasterConfigOptions.php', @@ -5195,6 +5198,14 @@ 'PhabricatorGlobalLock' => 'PhutilLock', 'PhabricatorGlobalUploadTargetView' => 'AphrontView', 'PhabricatorGoogleAuthProvider' => 'PhabricatorOAuth2AuthProvider', + 'PhabricatorHandleList' => array( + 'Phobject', + 'Iterator', + 'ArrayAccess', + 'Countable', + ), + 'PhabricatorHandlePool' => 'Phobject', + 'PhabricatorHandlePoolTestCase' => 'PhabricatorTestCase', 'PhabricatorHandleQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorHarbormasterApplication' => 'PhabricatorApplication', 'PhabricatorHarbormasterConfigOptions' => 'PhabricatorApplicationConfigOptions', diff --git a/src/applications/countdown/controller/PhabricatorCountdownViewController.php b/src/applications/countdown/controller/PhabricatorCountdownViewController.php --- a/src/applications/countdown/controller/PhabricatorCountdownViewController.php +++ b/src/applications/countdown/controller/PhabricatorCountdownViewController.php @@ -100,10 +100,8 @@ PhabricatorCountdown $countdown, PhabricatorActionListView $actions) { - $request = $this->getRequest(); - $viewer = $request->getUser(); - - $this->loadHandles(array($countdown->getAuthorPHID())); + $viewer = $this->getViewer(); + $handles = $viewer->loadHandles(array($countdown->getAuthorPHID())); $view = id(new PHUIPropertyListView()) ->setUser($viewer) @@ -111,7 +109,7 @@ $view->addProperty( pht('Author'), - $this->getHandle($countdown->getAuthorPHID())->renderLink()); + $handles[$countdown->getAuthorPHID()]->renderLink()); return $view; } diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -51,6 +51,7 @@ private $session = self::ATTACHABLE; private $authorities = array(); + private $handlePool; protected function readField($field) { switch ($field) { @@ -800,6 +801,26 @@ } +/* -( Handles )------------------------------------------------------------ */ + + + /** + * Get a @{class:PhabricatorHandleList} which benefits from this viewer's + * internal handle pool. + * + * @param list List of PHIDs to load. + * @return PhabricatorHandleList Handle list object. + */ + public function loadHandles(array $phids) { + if ($this->handlePool === null) { + $this->handlePool = id(new PhabricatorHandlePool()) + ->setViewer($this); + } + + return $this->handlePool->newHandleList($phids); + } + + /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/applications/phid/handle/pool/PhabricatorHandleList.php b/src/applications/phid/handle/pool/PhabricatorHandleList.php new file mode 100644 --- /dev/null +++ b/src/applications/phid/handle/pool/PhabricatorHandleList.php @@ -0,0 +1,124 @@ +loadHandles($phids); + * + * This creates a handle list object, which behaves like an array of handles. + * However, it benefits from the viewer's internal handle cache and performs + * just-in-time bulk loading. + */ +final class PhabricatorHandleList + extends Phobject + implements + Iterator, + ArrayAccess, + Countable { + + private $handlePool; + private $phids; + private $handles; + private $cursor; + + public function setHandlePool(PhabricatorHandlePool $pool) { + $this->handlePool = $pool; + return $this; + } + + public function setPHIDs(array $phids) { + $this->phids = $phids; + return $this; + } + + private function loadHandles() { + $this->handles = $this->handlePool->loadPHIDs($this->phids); + } + + private function getHandle($phid) { + if ($this->handles === null) { + $this->loadHandles(); + } + + if (empty($this->handles[$phid])) { + throw new Exception( + pht( + 'Requested handle "%s" was not loaded.', + $phid)); + } + + return $this->handles[$phid]; + } + + +/* -( Iterator )----------------------------------------------------------- */ + + + public function rewind() { + $this->cursor = 0; + } + + public function current() { + return $this->getHandle($this->phids[$this->cursor]); + } + + public function key() { + return $this->phids[$this->cursor]; + } + + public function next() { + ++$this->cursor; + } + + public function valid() { + return isset($this->phids[$this->cursor]); + } + + +/* -( ArrayAccess )-------------------------------------------------------- */ + + + public function offsetExists($offset) { + if ($this->handles === null) { + $this->loadHandles(); + } + return isset($this->handles[$offset]); + } + + public function offsetGet($offset) { + if ($this->handles === null) { + $this->loadHandles(); + } + return $this->handles[$offset]; + } + + public function offsetSet($offset, $value) { + $this->raiseImmutableException(); + } + + public function offsetUnset($offset) { + $this->raiseImmutableException(); + } + + private function raiseImmutableException() { + throw new Exception( + pht( + 'Trying to mutate a PhabricatorHandleList, but this is not permitted; '. + 'handle lists are immutable.')); + } + + +/* -( Countable )---------------------------------------------------------- */ + + + public function count() { + return count($this->phids); + } + +} diff --git a/src/applications/phid/handle/pool/PhabricatorHandlePool.php b/src/applications/phid/handle/pool/PhabricatorHandlePool.php new file mode 100644 --- /dev/null +++ b/src/applications/phid/handle/pool/PhabricatorHandlePool.php @@ -0,0 +1,75 @@ +viewer = $user; + return $this; + } + + public function getViewer() { + return $this->viewer; + } + + public function newHandleList(array $phids) { + // Mark any PHIDs we haven't loaded yet as unloaded. This will let us bulk + // load them later. + foreach ($phids as $phid) { + if (empty($this->handles[$phid])) { + $this->unloadedPHIDs[$phid] = true; + } + } + + $unique = array(); + foreach ($phids as $phid) { + $unique[$phid] = $phid; + } + + return id(new PhabricatorHandleList()) + ->setHandlePool($this) + ->setPHIDs(array_values($unique)); + } + + public function loadPHIDs(array $phids) { + $need = array(); + foreach ($phids as $phid) { + if (empty($this->handles[$phid])) { + $need[$phid] = true; + } + } + + foreach ($need as $phid => $ignored) { + if (empty($this->unloadedPHIDs[$phid])) { + throw new Exception( + pht( + 'Attempting to load PHID "%s", but it was not requested by any '. + 'handle list.', + $phid)); + } + } + + // If we need any handles, bulk load everything in the queue. + if ($need) { + $handles = id(new PhabricatorHandleQuery()) + ->setViewer($this->getViewer()) + ->withPHIDs(array_keys($this->unloadedPHIDs)) + ->execute(); + $this->handles += $handles; + $this->unloadedPHIDs = array(); + } + + return array_select_keys($this->handles, $phids); + } + +} diff --git a/src/applications/phid/handle/pool/__tests__/PhabricatorHandlePoolTestCase.php b/src/applications/phid/handle/pool/__tests__/PhabricatorHandlePoolTestCase.php new file mode 100644 --- /dev/null +++ b/src/applications/phid/handle/pool/__tests__/PhabricatorHandlePoolTestCase.php @@ -0,0 +1,58 @@ + true, + ); + } + + public function testHandlePools() { + // A lot of the batch/just-in-time/cache behavior of handle pools is not + // observable by design, so these tests don't directly cover it. + + $viewer = $this->generateNewTestUser(); + $viewer_phid = $viewer->getPHID(); + + $phids = array($viewer_phid); + + $handles = $viewer->loadHandles($phids); + + // The handle load hasn't happened yet, but we can't directly observe that. + + // Test Countable behaviors. + $this->assertEqual(1, count($handles)); + + // Test ArrayAccess behaviors. + $this->assertEqual( + array($viewer_phid), + array_keys(iterator_to_array($handles))); + $this->assertEqual(true, $handles[$viewer_phid]->isComplete()); + $this->assertEqual($viewer_phid, $handles[$viewer_phid]->getPHID()); + $this->assertTrue(isset($handles[$viewer_phid])); + $this->assertFalse(isset($handles['quack'])); + + // Test Iterator behaviors. + foreach ($handles as $key => $handle) { + $this->assertEqual($viewer_phid, $key); + $this->assertEqual($viewer_phid, $handle->getPHID()); + } + + // Do this twice to make sure the handle list is rewindable. + foreach ($handles as $key => $handle) { + $this->assertEqual($viewer_phid, $key); + $this->assertEqual($viewer_phid, $handle->getPHID()); + } + + $more_handles = $viewer->loadHandles($phids); + + // This is testing that we got back a reference to the exact same object, + // which implies the caching behavior is working correctly. + $this->assertEqual( + $handles[$viewer_phid], + $more_handles[$viewer_phid], + pht('Handles should use viewer handle pool cache.')); + } + +}