diff --git a/src/applications/feed/PhabricatorFeedQuery.php b/src/applications/feed/PhabricatorFeedQuery.php index 30331ecafd..6f540b6c5b 100644 --- a/src/applications/feed/PhabricatorFeedQuery.php +++ b/src/applications/feed/PhabricatorFeedQuery.php @@ -45,7 +45,7 @@ final class PhabricatorFeedQuery } protected function willFilterPage(array $data) { - return PhabricatorFeedStory::loadAllFromRows($data); + return PhabricatorFeedStory::loadAllFromRows($data, $this->getViewer()); } private function buildJoinClause(AphrontDatabaseConnection $conn_r) { diff --git a/src/applications/feed/builder/PhabricatorFeedBuilder.php b/src/applications/feed/builder/PhabricatorFeedBuilder.php index f326c32aeb..1387b9feab 100644 --- a/src/applications/feed/builder/PhabricatorFeedBuilder.php +++ b/src/applications/feed/builder/PhabricatorFeedBuilder.php @@ -44,21 +44,12 @@ final class PhabricatorFeedBuilder { $user = $this->user; $stories = $this->stories; - $handles = array(); - if ($stories) { - $handle_phids = array_mergev(mpull($stories, 'getRequiredHandlePHIDs')); - $object_phids = array_mergev(mpull($stories, 'getRequiredObjectPHIDs')); - $handles = id(new PhabricatorObjectHandleData($handle_phids)) - ->loadHandles(); - } - $null_view = new AphrontNullView(); require_celerity_resource('phabricator-feed-css'); $last_date = null; foreach ($stories as $story) { - $story->setHandles($handles); $story->setFramed($this->framed); $date = ucfirst(phabricator_relative_date($story->getEpoch(), $user)); diff --git a/src/applications/feed/constants/PhabricatorFeedStoryTypeConstants.php b/src/applications/feed/constants/PhabricatorFeedStoryTypeConstants.php index 0384d3ab3e..45a320fcf5 100644 --- a/src/applications/feed/constants/PhabricatorFeedStoryTypeConstants.php +++ b/src/applications/feed/constants/PhabricatorFeedStoryTypeConstants.php @@ -19,7 +19,6 @@ final class PhabricatorFeedStoryTypeConstants extends PhabricatorFeedConstants { - const STORY_UNKNOWN = 'PhabricatorFeedStoryUnknown'; const STORY_STATUS = 'PhabricatorFeedStoryStatus'; const STORY_DIFFERENTIAL = 'PhabricatorFeedStoryDifferential'; const STORY_PHRICTION = 'PhabricatorFeedStoryPhriction'; diff --git a/src/applications/feed/story/PhabricatorFeedStory.php b/src/applications/feed/story/PhabricatorFeedStory.php index 1dee238739..d9e5a92f1c 100644 --- a/src/applications/feed/story/PhabricatorFeedStory.php +++ b/src/applications/feed/story/PhabricatorFeedStory.php @@ -21,15 +21,17 @@ * user adding a comment) which may be represented in different forms on * different channels (like feed, notifications and realtime alerts). * - * @task load Loading Stories + * @task load Loading Stories + * @task policy Policy Implementation */ abstract class PhabricatorFeedStory implements PhabricatorPolicyInterface { private $data; private $hasViewed; - private $handles; private $framed; - private $primaryObjectPHID; + + private $handles = array(); + private $objects = array(); /* -( Loading Stories )---------------------------------------------------- */ @@ -46,7 +48,7 @@ abstract class PhabricatorFeedStory implements PhabricatorPolicyInterface { * objects. * @task load */ - public static function loadAllFromRows(array $rows) { + public static function loadAllFromRows(array $rows, PhabricatorUser $viewer) { $stories = array(); $data = id(new PhabricatorFeedStoryData())->loadAllFromArray($rows); @@ -62,42 +64,99 @@ abstract class PhabricatorFeedStory implements PhabricatorPolicyInterface { } // If the story type isn't a valid class or isn't a subclass of - // PhabricatorFeedStory, load it as PhabricatorFeedStoryUnknown. - + // PhabricatorFeedStory, decline to load it. if (!$ok) { - $class = 'PhabricatorFeedStoryUnknown'; + continue; } $key = $story_data->getChronologicalKey(); $stories[$key] = newv($class, array($story_data)); } + $object_phids = array(); + $key_phids = array(); + foreach ($stories as $key => $story) { + $phids = array(); + foreach ($story->getRequiredObjectPHIDs() as $phid) { + $phids[$phid] = true; + } + if ($story->getPrimaryObjectPHID()) { + $phids[$story->getPrimaryObjectPHID()] = true; + } + $key_phids[$key] = $phids; + $object_phids += $phids; + } + + $objects = id(new PhabricatorObjectHandleData(array_keys($object_phids))) + ->setViewer($viewer) + ->loadObjects(); + + foreach ($key_phids as $key => $phids) { + if (!$phids) { + continue; + } + $story_objects = array_select_keys($objects, array_keys($phids)); + if (count($story_objects) != count($phids)) { + // An object this story requires either does not exist or is not visible + // to the user. Decline to render the story. + unset($stories[$key]); + unset($key_phids[$key]); + continue; + } + + $stories[$key]->setObjects($story_objects); + } + + $handle_phids = array(); + foreach ($stories as $key => $story) { + foreach ($story->getRequiredHandlePHIDs() as $phid) { + $key_phids[$key][$phid] = true; + } + if ($story->getAuthorPHID()) { + $key_phids[$key][$story->getAuthorPHID()] = true; + } + $handle_phids += $key_phids[$key]; + } + + $handles = id(new PhabricatorObjectHandleData(array_keys($handle_phids))) + ->setViewer($viewer) + ->loadHandles(); + + foreach ($key_phids as $key => $phids) { + if (!$phids) { + continue; + } + $story_handles = array_select_keys($handles, array_keys($phids)); + $stories[$key]->setHandles($story_handles); + } + return $stories; } - public function getCapabilities() { - return array( - PhabricatorPolicyCapability::CAN_VIEW, - ); - } - - public function getPolicy($capability) { - return PhabricatorEnv::getEnvConfig('feed.public') - ? PhabricatorPolicies::POLICY_PUBLIC - : PhabricatorPolicies::POLICY_USER; - } - - public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { - return false; - } - - public function setPrimaryObjectPHID($primary_object_phid) { - $this->primaryObjectPHID = $primary_object_phid; + public function setObjects(array $objects) { + $this->objects = $objects; return $this; } + public function getObject($phid) { + $object = idx($this->objects, $phid); + if (!$object) { + throw new Exception( + "Story is asking for an object it did not request ('{$phid}')!"); + } + return $object; + } + + public function getPrimaryObject() { + $phid = $this->getPrimaryObjectPHID(); + if (!$phid) { + throw new Exception("Story has no primary object!"); + } + return $this->getObject($phid); + } + public function getPrimaryObjectPHID() { - return $this->primaryObjectPHID; + return null; } final public function __construct(PhabricatorFeedStoryData $data) { @@ -116,6 +175,10 @@ abstract class PhabricatorFeedStory implements PhabricatorPolicyInterface { return array(); } + public function getRequiredObjectPHIDs() { + return array(); + } + public function setHasViewed($has_viewed) { $this->hasViewed = $has_viewed; return $this; @@ -125,10 +188,6 @@ abstract class PhabricatorFeedStory implements PhabricatorPolicyInterface { return $this->hasViewed; } - public function getRequiredObjectPHIDs() { - return array(); - } - final public function setFramed($framed) { $this->framed = $framed; return $this; @@ -140,6 +199,10 @@ abstract class PhabricatorFeedStory implements PhabricatorPolicyInterface { return $this; } + final protected function getObjects() { + return $this->objects; + } + final protected function getHandles() { return $this->handles; } @@ -170,6 +233,14 @@ abstract class PhabricatorFeedStory implements PhabricatorPolicyInterface { return $this->getStoryData()->getChronologicalKey(); } + final public function getValue($key, $default = null) { + return $this->getStoryData()->getValue($key, $default); + } + + final public function getAuthorPHID() { + return $this->getStoryData()->getAuthorPHID(); + } + final protected function renderHandleList(array $phids) { $list = array(); foreach ($phids as $phid) { @@ -210,4 +281,48 @@ abstract class PhabricatorFeedStory implements PhabricatorPolicyInterface { return array(); } + +/* -( PhabricatorPolicyInterface Implementation )-------------------------- */ + + + /** + * @task policy + */ + public function getCapabilities() { + return array( + PhabricatorPolicyCapability::CAN_VIEW, + ); + } + + + /** + * @task policy + */ + public function getPolicy($capability) { + // If this story's primary object is a policy-aware object, use its policy + // to control story visiblity. + + $primary_phid = $this->getPrimaryObjectPHID(); + if (isset($this->objects[$primary_phid])) { + $object = $this->objects[$primary_phid]; + if ($object instanceof PhabricatorPolicyInterface) { + return $object->getPolicy($capability); + } + } + + // TODO: Remove this once all objects are policy-aware. For now, keep + // respecting the `feed.public` setting. + return PhabricatorEnv::getEnvConfig('feed.public') + ? PhabricatorPolicies::POLICY_PUBLIC + : PhabricatorPolicies::POLICY_USER; + } + + + /** + * @task policy + */ + public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { + return false; + } + } diff --git a/src/applications/feed/story/PhabricatorFeedStoryAggregate.php b/src/applications/feed/story/PhabricatorFeedStoryAggregate.php index 3eaf0a2994..d1926429da 100644 --- a/src/applications/feed/story/PhabricatorFeedStoryAggregate.php +++ b/src/applications/feed/story/PhabricatorFeedStoryAggregate.php @@ -24,6 +24,10 @@ abstract class PhabricatorFeedStoryAggregate extends PhabricatorFeedStory { return head($this->getAggregateStories())->getHasViewed(); } + public function getPrimaryObjectPHID() { + return head($this->getAggregateStories())->getPrimaryObjectPHID(); + } + public function getRequiredHandlePHIDs() { $phids = array(); foreach ($this->getAggregateStories() as $story) { @@ -59,6 +63,18 @@ abstract class PhabricatorFeedStoryAggregate extends PhabricatorFeedStory { final public function setAggregateStories(array $aggregate_stories) { assert_instances_of($aggregate_stories, 'PhabricatorFeedStory'); $this->aggregateStories = $aggregate_stories; + + $objects = array(); + $handles = array(); + + foreach ($this->aggregateStories as $story) { + $objects += $story->getObjects(); + $handles += $story->getHandles(); + } + + $this->setObjects($objects); + $this->setHandles($handles); + return $this; } diff --git a/src/applications/feed/story/PhabricatorFeedStoryAudit.php b/src/applications/feed/story/PhabricatorFeedStoryAudit.php index d6b50aab6a..fad627bc42 100644 --- a/src/applications/feed/story/PhabricatorFeedStoryAudit.php +++ b/src/applications/feed/story/PhabricatorFeedStoryAudit.php @@ -18,26 +18,17 @@ final class PhabricatorFeedStoryAudit extends PhabricatorFeedStory { - public function getRequiredHandlePHIDs() { - return array( - $this->getStoryData()->getAuthorPHID(), - $this->getStoryData()->getValue('commitPHID'), - ); - } - - public function getRequiredObjectPHIDs() { - return array(); + public function getPrimaryObjectPHID() { + return $this->getStoryData()->getValue('commitPHID'); } public function renderView() { - $data = $this->getStoryData(); - - $author_phid = $data->getAuthorPHID(); - $commit_phid = $data->getValue('commitPHID'); + $author_phid = $this->getAuthorPHID(); + $commit_phid = $this->getPrimaryObjectPHID(); $view = new PhabricatorFeedStoryView(); - $action = $data->getValue('action'); + $action = $this->getValue('action'); $verb = PhabricatorAuditActionConstants::getActionPastTenseVerb($action); $view->setTitle( @@ -46,9 +37,9 @@ final class PhabricatorFeedStoryAudit extends PhabricatorFeedStory { $this->linkTo($commit_phid). "."); - $view->setEpoch($data->getEpoch()); + $view->setEpoch($this->getEpoch()); - $comments = $data->getValue('content'); + $comments = $this->getValue('content'); if ($comments) { $full_size = true; } else { @@ -57,7 +48,7 @@ final class PhabricatorFeedStoryAudit extends PhabricatorFeedStory { if ($full_size) { $view->setImage($this->getHandle($author_phid)->getImageURI()); - $content = $this->renderSummary($data->getValue('content')); + $content = $this->renderSummary($this->getValue('content')); $view->appendChild($content); } else { $view->setOneLineStory(true); diff --git a/src/applications/feed/story/PhabricatorFeedStoryCommit.php b/src/applications/feed/story/PhabricatorFeedStoryCommit.php index a2144979a5..67c340c053 100644 --- a/src/applications/feed/story/PhabricatorFeedStoryCommit.php +++ b/src/applications/feed/story/PhabricatorFeedStoryCommit.php @@ -18,15 +18,14 @@ final class PhabricatorFeedStoryCommit extends PhabricatorFeedStory { - public function getRequiredHandlePHIDs() { - $data = $this->getStoryData(); + public function getPrimaryObjectPHID() { + return $this->getValue('commitPHID'); + } - return array_filter( - array( - $data->getValue('commitPHID'), - $data->getValue('authorPHID'), - $data->getValue('committerPHID'), - )); + public function getRequiredHandlePHIDs() { + return array( + $this->getValue('committerPHID'), + ); } public function renderView() { diff --git a/src/applications/feed/story/PhabricatorFeedStoryDifferential.php b/src/applications/feed/story/PhabricatorFeedStoryDifferential.php index cde2122d93..ed685fd84b 100644 --- a/src/applications/feed/story/PhabricatorFeedStoryDifferential.php +++ b/src/applications/feed/story/PhabricatorFeedStoryDifferential.php @@ -18,19 +18,8 @@ final class PhabricatorFeedStoryDifferential extends PhabricatorFeedStory { - public function getRequiredHandlePHIDs() { - $data = $this->getStoryData(); - return array( - $this->getStoryData()->getAuthorPHID(), - $data->getValue('revision_phid'), - $data->getValue('revision_author_phid'), - ); - } - - public function getRequiredObjectPHIDs() { - return array( - $this->getStoryData()->getAuthorPHID(), - ); + public function getPrimaryObjectPHID() { + return $this->getValue('revision_phid'); } public function renderView() { @@ -78,13 +67,11 @@ final class PhabricatorFeedStoryDifferential extends PhabricatorFeedStory { private function getLineForData($data) { $actor_phid = $data->getAuthorPHID(); - $owner_phid = $data->getValue('revision_author_phid'); $revision_phid = $data->getValue('revision_phid'); $action = $data->getValue('action'); $actor_link = $this->linkTo($actor_phid); $revision_link = $this->linkTo($revision_phid); - $owner_link = $this->linkTo($owner_phid); $verb = DifferentialAction::getActionPastTenseVerb($action); diff --git a/src/applications/feed/story/PhabricatorFeedStoryManiphest.php b/src/applications/feed/story/PhabricatorFeedStoryManiphest.php index 7648fbce6c..73028da9eb 100644 --- a/src/applications/feed/story/PhabricatorFeedStoryManiphest.php +++ b/src/applications/feed/story/PhabricatorFeedStoryManiphest.php @@ -19,19 +19,13 @@ final class PhabricatorFeedStoryManiphest extends PhabricatorFeedStory { - public function getRequiredHandlePHIDs() { - $data = $this->getStoryData(); - return array_filter( - array( - $this->getStoryData()->getAuthorPHID(), - $data->getValue('taskPHID'), - $data->getValue('ownerPHID'), - )); + public function getPrimaryObjectPHID() { + return $this->getValue('taskPHID'); } - public function getRequiredObjectPHIDs() { + public function getRequiredHandlePHIDs() { return array( - $this->getStoryData()->getAuthorPHID(), + $this->getValue('ownerPHID'), ); } diff --git a/src/applications/feed/story/PhabricatorFeedStoryPhriction.php b/src/applications/feed/story/PhabricatorFeedStoryPhriction.php index 8cadf1ece5..96557214f0 100644 --- a/src/applications/feed/story/PhabricatorFeedStoryPhriction.php +++ b/src/applications/feed/story/PhabricatorFeedStoryPhriction.php @@ -18,17 +18,8 @@ final class PhabricatorFeedStoryPhriction extends PhabricatorFeedStory { - public function getRequiredHandlePHIDs() { - return array( - $this->getStoryData()->getAuthorPHID(), - $this->getStoryData()->getValue('phid'), - ); - } - - public function getRequiredObjectPHIDs() { - return array( - $this->getStoryData()->getAuthorPHID(), - ); + public function getPrimaryObjectPHID() { + return $this->getValue('phid'); } public function renderView() { diff --git a/src/applications/feed/story/PhabricatorFeedStoryProject.php b/src/applications/feed/story/PhabricatorFeedStoryProject.php index 382a531c32..65faa34007 100644 --- a/src/applications/feed/story/PhabricatorFeedStoryProject.php +++ b/src/applications/feed/story/PhabricatorFeedStoryProject.php @@ -18,17 +18,8 @@ final class PhabricatorFeedStoryProject extends PhabricatorFeedStory { - public function getRequiredHandlePHIDs() { - return array( - $this->getStoryData()->getAuthorPHID(), - $this->getStoryData()->getValue('projectPHID'), - ); - } - - public function getRequiredObjectPHIDs() { - return array( - $this->getStoryData()->getAuthorPHID(), - ); + public function getPrimaryObjectPHID() { + return $this->getValue('projectPHID'); } public function renderView() { diff --git a/src/applications/feed/story/PhabricatorFeedStoryStatus.php b/src/applications/feed/story/PhabricatorFeedStoryStatus.php index d08719685f..eca8468960 100644 --- a/src/applications/feed/story/PhabricatorFeedStoryStatus.php +++ b/src/applications/feed/story/PhabricatorFeedStoryStatus.php @@ -18,16 +18,8 @@ final class PhabricatorFeedStoryStatus extends PhabricatorFeedStory { - public function getRequiredHandlePHIDs() { - return array( - $this->getStoryData()->getAuthorPHID(), - ); - } - - public function getRequiredObjectPHIDs() { - return array( - $this->getStoryData()->getAuthorPHID(), - ); + public function getPrimaryObjectPHID() { + return $this->getAuthorPHID(); } public function renderView() { diff --git a/src/applications/notification/PhabricatorNotificationQuery.php b/src/applications/notification/PhabricatorNotificationQuery.php index fcaaf62223..f0111e6152 100644 --- a/src/applications/notification/PhabricatorNotificationQuery.php +++ b/src/applications/notification/PhabricatorNotificationQuery.php @@ -20,7 +20,8 @@ * @task config Configuring the Query * @task exec Query Execution */ -final class PhabricatorNotificationQuery extends PhabricatorOffsetPagedQuery { +final class PhabricatorNotificationQuery + extends PhabricatorCursorPagedPolicyAwareQuery { private $userPHID; private $keys; @@ -60,7 +61,7 @@ final class PhabricatorNotificationQuery extends PhabricatorOffsetPagedQuery { /* -( Query Execution )---------------------------------------------------- */ - public function execute() { + public function loadPage() { if (!$this->userPHID) { throw new Exception("Call setUser() before executing the query"); } @@ -72,7 +73,7 @@ final class PhabricatorNotificationQuery extends PhabricatorOffsetPagedQuery { $data = queryfx_all( $conn, - "SELECT story.*, notif.primaryObjectPHID, notif.hasViewed FROM %T notif + "SELECT story.*, notif.hasViewed FROM %T notif JOIN %T story ON notif.chronologicalKey = story.chronologicalKey %Q ORDER BY notif.chronologicalKey DESC @@ -83,12 +84,13 @@ final class PhabricatorNotificationQuery extends PhabricatorOffsetPagedQuery { $this->buildLimitClause($conn)); $viewed_map = ipull($data, 'hasViewed', 'chronologicalKey'); - $primary_map = ipull($data, 'primaryObjectPHID', 'chronologicalKey'); - $stories = PhabricatorFeedStory::loadAllFromRows($data); + $stories = PhabricatorFeedStory::loadAllFromRows( + $data, + $this->getViewer()); + foreach ($stories as $key => $story) { $story->setHasViewed($viewed_map[$key]); - $story->setPrimaryObjectPHID($primary_map[$key]); } return $stories; diff --git a/src/applications/notification/builder/PhabricatorNotificationBuilder.php b/src/applications/notification/builder/PhabricatorNotificationBuilder.php index 9f5dc3363e..c22a346985 100644 --- a/src/applications/notification/builder/PhabricatorNotificationBuilder.php +++ b/src/applications/notification/builder/PhabricatorNotificationBuilder.php @@ -136,19 +136,9 @@ final class PhabricatorNotificationBuilder { $stories = mpull($stories, null, 'getChronologicalKey'); krsort($stories); - $handles = array(); - $objects = array(); - - if ($stories) { - $handle_phids = array_mergev(mpull($stories, 'getRequiredHandlePHIDs')); - $handles = id(new PhabricatorObjectHandleData($handle_phids)) - ->loadHandles(); - } - $null_view = new AphrontNullView(); foreach ($stories as $story) { - $story->setHandles($handles); $view = $story->renderNotificationView(); $null_view->appendChild($view); } diff --git a/src/applications/notification/controller/PhabricatorNotificationIndividualController.php b/src/applications/notification/controller/PhabricatorNotificationIndividualController.php index ada8d68c66..afb5710d31 100644 --- a/src/applications/notification/controller/PhabricatorNotificationIndividualController.php +++ b/src/applications/notification/controller/PhabricatorNotificationIndividualController.php @@ -24,6 +24,7 @@ final class PhabricatorNotificationIndividualController $user = $request->getUser(); $stories = id(new PhabricatorNotificationQuery()) + ->setViewer($user) ->setUserPHID($user->getPHID()) ->withKeys(array($request->getStr('key'))) ->execute(); diff --git a/src/applications/notification/controller/PhabricatorNotificationListController.php b/src/applications/notification/controller/PhabricatorNotificationListController.php index ac46cb22dd..aa8a033add 100644 --- a/src/applications/notification/controller/PhabricatorNotificationListController.php +++ b/src/applications/notification/controller/PhabricatorNotificationListController.php @@ -40,6 +40,7 @@ final class PhabricatorNotificationListController $pager->setOffset($request->getInt('offset')); $query = new PhabricatorNotificationQuery(); + $query->setViewer($user); $query->setUserPHID($user->getPHID()); switch ($filter) { diff --git a/src/applications/notification/controller/PhabricatorNotificationPanelController.php b/src/applications/notification/controller/PhabricatorNotificationPanelController.php index 2815101fb2..3eb87e09b3 100644 --- a/src/applications/notification/controller/PhabricatorNotificationPanelController.php +++ b/src/applications/notification/controller/PhabricatorNotificationPanelController.php @@ -25,6 +25,7 @@ final class PhabricatorNotificationPanelController $user = $request->getUser(); $query = new PhabricatorNotificationQuery(); + $query->setViewer($user); $query->setUserPHID($user->getPHID()); $query->setLimit(15);