Page MenuHomePhabricator
Files F14058612

D13479.id32626.diff
No OneTemporary
Actions

D13479.id32626.diff
View Options

diff --git a/src/applications/feed/story/PhabricatorFeedStory.php b/src/applications/feed/story/PhabricatorFeedStory.php
--- a/src/applications/feed/story/PhabricatorFeedStory.php
+++ b/src/applications/feed/story/PhabricatorFeedStory.php
@@ -78,10 +78,11 @@
$object_phids += $phids;
}
- $objects = id(new PhabricatorObjectQuery())
+ $object_query = id(new PhabricatorObjectQuery())
->setViewer($viewer)
- ->withPHIDs(array_keys($object_phids))
- ->execute();
+ ->withPHIDs(array_keys($object_phids));
+
+ $objects = $object_query->execute();
foreach ($key_phids as $key => $phids) {
if (!$phids) {
@@ -142,8 +143,13 @@
$handle_phids += $key_phids[$key];
}
+ // NOTE: This setParentQuery() is a little sketchy. Ideally, this whole
+ // method should be inside FeedQuery and it should be the parent query of
+ // both subqueries. We're just trying to share the workspace cache.
+
$handles = id(new PhabricatorHandleQuery())
->setViewer($viewer)
+ ->setParentQuery($object_query)
->withPHIDs(array_keys($handle_phids))
->execute();
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
@@ -1073,6 +1073,29 @@
}
+ /**
+ * Get a scalar string identifying this user.
+ *
+ * This is similar to using the PHID, but distinguishes between ominpotent
+ * and public users explicitly. This allows safe construction of cache keys
+ * or cache buckets which do not conflate public and omnipotent users.
+ *
+ * @return string Scalar identifier.
+ */
+ public function getCacheFragment() {
+ if ($this->isOmnipotent()) {
+ return 'u.omnipotent';
+ }
+
+ $phid = $this->getPHID();
+ if ($phid) {
+ return 'u.'.$phid;
+ }
+
+ return 'u.public';
+ }
+
+
/* -( Managing Handles )--------------------------------------------------- */
diff --git a/src/applications/phid/query/PhabricatorHandleQuery.php b/src/applications/phid/query/PhabricatorHandleQuery.php
--- a/src/applications/phid/query/PhabricatorHandleQuery.php
+++ b/src/applications/phid/query/PhabricatorHandleQuery.php
@@ -33,6 +33,7 @@
$object_query = id(new PhabricatorObjectQuery())
->withPHIDs($phids)
+ ->setParentQuery($this)
->requireCapabilities($this->getRequiredObjectCapabilities())
->setViewer($this->getViewer());
diff --git a/src/applications/phid/query/PhabricatorObjectQuery.php b/src/applications/phid/query/PhabricatorObjectQuery.php
--- a/src/applications/phid/query/PhabricatorObjectQuery.php
+++ b/src/applications/phid/query/PhabricatorObjectQuery.php
@@ -106,19 +106,6 @@
private function loadObjectsByPHID(array $types, array $phids) {
$results = array();
- $workspace = $this->getObjectsFromWorkspace($phids);
-
- foreach ($phids as $key => $phid) {
- if (isset($workspace[$phid])) {
- $results[$phid] = $workspace[$phid];
- unset($phids[$key]);
- }
- }
-
- if (!$phids) {
- return $results;
- }
-
$groups = array();
foreach ($phids as $phid) {
$type = phid_get_type($phid);
@@ -127,6 +114,21 @@
$in_flight = $this->getPHIDsInFlight();
foreach ($groups as $type => $group) {
+ // We check the workspace for each group, because some groups may trigger
+ // other groups to load (for example, transactions load their objects).
+ $workspace = $this->getObjectsFromWorkspace($group);
+
+ foreach ($group as $key => $phid) {
+ if (isset($workspace[$phid])) {
+ $results[$phid] = $workspace[$phid];
+ unset($group[$key]);
+ }
+ }
+
+ if (!$group) {
+ continue;
+ }
+
// Don't try to load PHIDs which are already "in flight"; this prevents
// us from recursing indefinitely if policy checks or edges form a loop.
// We will decline to load the corresponding objects.
@@ -139,7 +141,10 @@
if ($group && isset($types[$type])) {
$this->putPHIDsInFlight($group);
$objects = $types[$type]->loadObjects($this, $group);
- $results += mpull($objects, null, 'getPHID');
+
+ $map = mpull($objects, null, 'getPHID');
+ $this->putObjectsInWorkspace($map);
+ $results += $map;
}
}
diff --git a/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php
--- a/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php
+++ b/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php
@@ -406,8 +406,8 @@
*
* **Fully enrich objects pulled from the workspace.** After pulling objects
* from the workspace, you still need to load and attach any additional
- * content the query requests. Otherwise, a query might return objects without
- * requested content.
+ * content the query requests. Otherwise, a query might return objects
+ * without requested content.
*
* Generally, you do not need to update the workspace yourself: it is
* automatically populated as a side effect of objects surviving policy
@@ -419,16 +419,22 @@
* @task workspace
*/
public function putObjectsInWorkspace(array $objects) {
+ $parent = $this->getParentQuery();
+ if ($parent) {
+ $parent->putObjectsInWorkspace($objects);
+ return $this;
+ }
+
assert_instances_of($objects, 'PhabricatorPolicyInterface');
- $viewer_phid = $this->getViewer()->getPHID();
+ $viewer_fragment = $this->getViewer()->getCacheFragment();
// The workspace is scoped per viewer to prevent accidental contamination.
- if (empty($this->workspace[$viewer_phid])) {
- $this->workspace[$viewer_phid] = array();
+ if (empty($this->workspace[$viewer_fragment])) {
+ $this->workspace[$viewer_fragment] = array();
}
- $this->workspace[$viewer_phid] += $objects;
+ $this->workspace[$viewer_fragment] += $objects;
return $this;
}
@@ -445,20 +451,21 @@
* @task workspace
*/
public function getObjectsFromWorkspace(array $phids) {
- $viewer_phid = $this->getViewer()->getPHID();
+ $parent = $this->getParentQuery();
+ if ($parent) {
+ return $parent->getObjectsFromWorkspace($phids);
+ }
+
+ $viewer_fragment = $this->getViewer()->getCacheFragment();
$results = array();
foreach ($phids as $key => $phid) {
- if (isset($this->workspace[$viewer_phid][$phid])) {
- $results[$phid] = $this->workspace[$viewer_phid][$phid];
+ if (isset($this->workspace[$viewer_fragment][$phid])) {
+ $results[$phid] = $this->workspace[$viewer_fragment][$phid];
unset($phids[$key]);
}
}
- if ($phids && $this->getParentQuery()) {
- $results += $this->getParentQuery()->getObjectsFromWorkspace($phids);
- }
-
return $results;
}

File Metadata

Mime Type
text/plain
Expires
Mon, Nov 18, 1:20 PM (1 d, 17 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6729467
Default Alt Text
D13479.id32626.diff (6 KB)

Event Timeline

Phabricator · Built by Phacility