From 5dfb8e2bc418d1144c953214a40b4dd730ca148b Mon Sep 17 00:00:00 2001 From: Evan Prodromou Date: Thu, 7 Apr 2011 14:52:44 -0400 Subject: [PATCH] Use InboxNoticeStream class for inbox Move the code for inbox fetching to the InboxNoticeStream class. --- actions/all.php | 14 ++-- classes/Inbox.php | 161 +------------------------------------- classes/User.php | 29 ++++--- lib/inboxnoticestream.php | 136 ++++++++++++++++++++++++++++++++ 4 files changed, 165 insertions(+), 175 deletions(-) create mode 100644 lib/inboxnoticestream.php diff --git a/actions/all.php b/actions/all.php index 2826319c0d..74a0059346 100644 --- a/actions/all.php +++ b/actions/all.php @@ -55,17 +55,17 @@ class AllAction extends ProfileAction function prepare($args) { parent::prepare($args); - $cur = common_current_user(); - if (!empty($cur) && $cur->id == $this->user->id) { - $this->notice = $this->user->noticeInboxThreaded(($this->page-1)*NOTICES_PER_PAGE, NOTICES_PER_PAGE + 1); - } else { - $this->notice = $this->user->noticesWithFriendsThreaded(($this->page-1)*NOTICES_PER_PAGE, NOTICES_PER_PAGE + 1); - } + $stream = new InboxNoticeStream($this->user); + + $this->notice = $stream->getNotices(($this->page-1)*NOTICES_PER_PAGE, + NOTICES_PER_PAGE + 1, + null, + null); if ($this->page > 1 && $this->notice->N == 0) { // TRANS: Server error when page not found (404). - $this->serverError(_('No such page.'), $code = 404); + $this->serverError(_('No such page.'), 404); } return true; diff --git a/classes/Inbox.php b/classes/Inbox.php index feaead249b..336bba048c 100644 --- a/classes/Inbox.php +++ b/classes/Inbox.php @@ -157,168 +157,11 @@ class Inbox extends Memcached_DataObject } } - function stream($user_id, $offset, $limit, $since_id, $max_id, $own=false) - { - $inbox = Inbox::staticGet('user_id', $user_id); - - if (empty($inbox)) { - $inbox = Inbox::fromNoticeInbox($user_id); - if (empty($inbox)) { - return array(); - } else { - $inbox->encache(); - } - } - - $ids = $inbox->unpack(); - - if (!empty($since_id)) { - $newids = array(); - foreach ($ids as $id) { - if ($id > $since_id) { - $newids[] = $id; - } - } - $ids = $newids; - } - - if (!empty($max_id)) { - $newids = array(); - foreach ($ids as $id) { - if ($id <= $max_id) { - $newids[] = $id; - } - } - $ids = $newids; - } - - $ids = array_slice($ids, $offset, $limit); - - return $ids; - } - - /** - * Wrapper for Inbox::stream() and Notice::getStreamByIds() returning - * additional items up to the limit if we were short due to deleted - * notices still being listed in the inbox. - * - * This is meant to assist threaded views, and optimizes paging for - * threadness. Not ideal for very late pages, as we have to bump about - * through all previous items. - * - * Should avoid duplicates in paging, though. - * - * @param int $user_id - * @param int $offset skip past the most recent N notices (after since_id checks) - * @param int $limit - * @param mixed $since_id return only notices after but not including this id - * @param mixed $max_id return only notices up to and including this id - * @param mixed $own ignored? - * @return array of Notice objects - * - * @todo consider repacking the inbox when this happens? - * @fixme reimplement $own if we need it? - */ - function streamNoticesThreaded($user_id, $offset, $limit, $since_id, $max_id, $own=false) - { - // So what we want is: - // * slurp in the beginning of the notice list - // * filter out deleted notices - // * replace any reply notices with their conversation roots - // * filter out any duplicate conversations - // * return $limit notices after skipping $offset from the most recent - - $ids = self::stream($user_id, 0, self::MAX_NOTICES, $since_id, $max_id, $own); - - // Do a bulk lookup for the first $limit items - // Fast path when nothing's deleted. - $firstChunk = array_slice($ids, 0, $offset + $limit); - $notices = NoticeStream::getStreamByIds($firstChunk); - - assert($notices instanceof ArrayWrapper); - $items = $notices->_items; - - // Extract the latest non-deleted item in each convo - $noticeByConvo = array(); - foreach ($items as $notice) { - if (empty($noticeByConvo[$notice->conversation])) { - $noticeByConvo[$notice->conversation] = $notice; - } - } - - $wanted = count($firstChunk); // raw entry count in the inbox up to our $limit - // There were deleted notices, we'll need to look for more. - $remainder = array_slice($ids, $limit); - - for ($i = $offset + $limit; count($noticeByConvo) < $wanted && $i < count($ids); $i++) { - $notice = Notice::staticGet($ids[$i]); - if ($notice && empty($noticeByConvo[$notice->conversation])) { - $noticeByConvo[$notice->conversation] = $notice; - } - } - - $slice = array_slice($noticeByConvo, $offset, $limit, false); - return new ArrayWrapper($slice); - } - - /** - * Wrapper for Inbox::stream() and Notice::getStreamByIds() returning - * additional items up to the limit if we were short due to deleted - * notices still being listed in the inbox. - * - * The fast path (when no items are deleted) should be just as fast; the - * offset parameter is applied *before* lookups for maximum efficiency. - * - * This means offset-based paging may show duplicates, but similar behavior - * already exists when new notices are posted between page views, so we - * think people will be ok with this until id-based paging is introduced - * to the user interface. - * - * @param int $user_id - * @param int $offset skip past the most recent N notices (after since_id checks) - * @param int $limit - * @param mixed $since_id return only notices after but not including this id - * @param mixed $max_id return only notices up to and including this id - * @param mixed $own ignored? - * @return array of Notice objects - * - * @todo consider repacking the inbox when this happens? - * @fixme reimplement $own if we need it? - */ - function streamNotices($user_id, $offset, $limit, $since_id, $max_id, $own=false) - { - $ids = self::stream($user_id, $offset, self::MAX_NOTICES, $since_id, $max_id, $own); - - // Do a bulk lookup for the first $limit items - // Fast path when nothing's deleted. - $firstChunk = array_slice($ids, 0, $limit); - $notices = NoticeStream::getStreamByIds($firstChunk); - - $wanted = count($firstChunk); // raw entry count in the inbox up to our $limit - if ($notices->N >= $wanted) { - return $notices; - } - - // There were deleted notices, we'll need to look for more. - assert($notices instanceof ArrayWrapper); - $items = $notices->_items; - $remainder = array_slice($ids, $limit); - - while (count($items) < $wanted && count($remainder) > 0) { - $notice = Notice::staticGet(array_shift($remainder)); - if ($notice) { - $items[] = $notice; - } else { - } - } - return new ArrayWrapper($items); - } - /** * Saves a list of integer notice_ids into a packed blob in this object. * @param array $ids list of integer notice_ids */ - protected function pack(array $ids) + function pack(array $ids) { $this->notice_ids = call_user_func_array('pack', array_merge(array('N*'), $ids)); } @@ -326,7 +169,7 @@ class Inbox extends Memcached_DataObject /** * @return array of integer notice_ids */ - protected function unpack() + function unpack() { return unpack('N*', $this->notice_ids); } diff --git a/classes/User.php b/classes/User.php index 4fe7efc858..c968375115 100644 --- a/classes/User.php +++ b/classes/User.php @@ -478,34 +478,45 @@ class User extends Memcached_DataObject return Fave::stream($this->id, $offset, $limit, $own, $since_id, $max_id); } + function noticeInbox($offset=0, $limit=NOTICES_PER_PAGE, $since_id=0, $before_id=0) + { + $stream = new InboxNoticeStream($this); + return $stream->getNotices($offset, $limit, $since_id, $before_id); + } + + // DEPRECATED, use noticeInbox() + function noticesWithFriends($offset=0, $limit=NOTICES_PER_PAGE, $since_id=0, $before_id=0) { - return Inbox::streamNotices($this->id, $offset, $limit, $since_id, $before_id, false); + $this->noticeInbox($offset, $limit, $since_id, $before_id); } + // DEPRECATED, use noticeInbox() + function noticesWithFriendsThreaded($offset=0, $limit=NOTICES_PER_PAGE, $since_id=0, $before_id=0) { - return Inbox::streamNoticesThreaded($this->id, $offset, $limit, $since_id, $before_id, false); + $this->noticeInbox($offset, $limit, $since_id, $before_id); } - function noticeInbox($offset=0, $limit=NOTICES_PER_PAGE, $since_id=0, $before_id=0) - { - return Inbox::streamNotices($this->id, $offset, $limit, $since_id, $before_id, true); - } + // DEPRECATED, use noticeInbox() function noticeInboxThreaded($offset=0, $limit=NOTICES_PER_PAGE, $since_id=0, $before_id=0) { - return Inbox::streamNoticesThreaded($this->id, $offset, $limit, $since_id, $before_id, true); + $this->noticeInbox($offset, $limit, $since_id, $before_id); } + // DEPRECATED, use noticeInbox() + function friendsTimeline($offset=0, $limit=NOTICES_PER_PAGE, $since_id=0, $before_id=0) { - return Inbox::streamNotices($this->id, $offset, $limit, $since_id, $before_id, false); + $this->noticeInbox($offset, $limit, $since_id, $before_id); } + // DEPRECATED, use noticeInbox() + function ownFriendsTimeline($offset=0, $limit=NOTICES_PER_PAGE, $since_id=0, $before_id=0) { - return Inbox::streamNotices($this->id, $offset, $limit, $since_id, $before_id, true); + $this->noticeInbox($offset, $limit, $since_id, $before_id); } function blowFavesCache() diff --git a/lib/inboxnoticestream.php b/lib/inboxnoticestream.php new file mode 100644 index 0000000000..c7939f7977 --- /dev/null +++ b/lib/inboxnoticestream.php @@ -0,0 +1,136 @@ +. + * + * @category Cache + * @package StatusNet + * @author Evan Prodromou + * @copyright 2011 StatusNet, Inc. + * @license http://www.fsf.org/licensing/licenses/agpl-3.0.html AGPL 3.0 + * @link http://status.net/ + */ + +if (!defined('STATUSNET')) { + // This check helps protect against security problems; + // your code file can't be executed directly from the web. + exit(1); +} + +/** + * Stream of notices for the user's inbox + * + * @category General + * @package StatusNet + * @author Evan Prodromou + * @copyright 2011 StatusNet, Inc. + * @license http://www.fsf.org/licensing/licenses/agpl-3.0.html AGPL 3.0 + * @link http://status.net/ + */ + +class InboxNoticeStream extends ScopingNoticeStream +{ + /** + * Constructor + * + * @param User $user User to get a stream for + */ + function __construct($user) + { + // Note: we don't use CachingNoticeStream since RawInboxNoticeStream + // uses Inbox::staticGet(), which is cached. + parent::__construct(new RawInboxNoticeStream($user)); + } +} + +/** + * Raw stream of notices for the user's inbox + * + * @category General + * @package StatusNet + * @author Evan Prodromou + * @copyright 2011 StatusNet, Inc. + * @license http://www.fsf.org/licensing/licenses/agpl-3.0.html AGPL 3.0 + * @link http://status.net/ + */ +class RawInboxNoticeStream extends NoticeStream +{ + protected $user = null; + protected $inbox = null; + + /** + * Constructor + * + * @param User $user User to get a stream for + */ + function __construct($user) + { + $this->user = $user; + $this->inbox = Inbox::staticGet('user_id', $user->id); + } + + /** + * Get IDs in a range + * + * @param int $offset Offset from start + * @param int $limit Limit of number to get + * @param int $since_id Since this notice + * @param int $max_id Before this notice + * + * @return Array IDs found + */ + function getNoticeIds($offset, $limit, $since_id, $max_id) + { + if (empty($this->inbox)) { + $this->inbox = Inbox::fromNoticeInbox($user_id); + if (empty($this->inbox)) { + return array(); + } else { + $this->inbox->encache(); + } + } + + $ids = $this->inbox->unpack(); + + if (!empty($since_id)) { + $newids = array(); + foreach ($ids as $id) { + if ($id > $since_id) { + $newids[] = $id; + } + } + $ids = $newids; + } + + if (!empty($max_id)) { + $newids = array(); + foreach ($ids as $id) { + if ($id <= $max_id) { + $newids[] = $id; + } + } + $ids = $newids; + } + + $ids = array_slice($ids, $offset, $limit); + + return $ids; + } +} \ No newline at end of file