Page MenuHomePhabricator
Files F14422170

D12202.id29325.diff
No OneTemporary
Actions

D12202.id29325.diff
View Options

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<phid> 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 @@
+<?php
+
+/**
+ * A list of object handles.
+ *
+ * This is a convenience class which behaves like an array but makes working
+ * with handles more convenient, improves their caching and batching semantics,
+ * and provides some utility behavior.
+ *
+ * Load a handle list by calling `loadHandles()` on a `$viewer`:
+ *
+ * $handles = $viewer->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 @@
+<?php
+
+/**
+ * Coordinates loading object handles.
+ *
+ * This is a low-level piece of plumbing which code will not normally interact
+ * with directly. For discussion of the handle pool mechanism, see
+ * @{class:PhabricatorHandleList}.
+ */
+final class PhabricatorHandlePool extends Phobject {
+
+ private $viewer;
+ private $handles = array();
+ private $unloadedPHIDs = array();
+
+ public function setViewer(PhabricatorUser $user) {
+ $this->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 @@
+<?php
+
+final class PhabricatorHandlePoolTestCase extends PhabricatorTestCase {
+
+ protected function getPhabricatorTestCaseConfiguration() {
+ return array(
+ self::PHABRICATOR_TESTCONFIG_BUILD_STORAGE_FIXTURES => 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.'));
+ }
+
+}

File Metadata

Mime Type
text/plain
Expires
Thu, Dec 26, 7:26 AM (12 h, 17 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6928730
Default Alt Text
D12202.id29325.diff (12 KB)

Event Timeline

Phabricator · Built by Phacility