From 37f3a3d558ba55a085c9ee5427948b572c197bc3 Mon Sep 17 00:00:00 2001 From: Eric Helgeson Date: Sat, 16 Jan 2010 11:56:07 -0500 Subject: Missed change when refactoring groups. Thanks macno --- actions/apigroupjoin.php | 2 +- actions/apigroupleave.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/actions/apigroupjoin.php b/actions/apigroupjoin.php index 3309d63e7..374cf83df 100644 --- a/actions/apigroupjoin.php +++ b/actions/apigroupjoin.php @@ -145,7 +145,7 @@ class ApiGroupJoinAction extends ApiAuthAction switch($this->format) { case 'xml': - $this->show_single_xml_group($this->group); + $this->showSingleXmlGroup($this->group); break; case 'json': $this->showSingleJsonGroup($this->group); diff --git a/actions/apigroupleave.php b/actions/apigroupleave.php index 6f8d40527..9848ece05 100644 --- a/actions/apigroupleave.php +++ b/actions/apigroupleave.php @@ -131,7 +131,7 @@ class ApiGroupLeaveAction extends ApiAuthAction switch($this->format) { case 'xml': - $this->show_single_xml_group($this->group); + $this->showSingleXmlGroup($this->group); break; case 'json': $this->showSingleJsonGroup($this->group); -- cgit v1.2.3-54-g00ecf From bb16898b1c73073e0442de72f2af133a3bd39713 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Thu, 4 Feb 2010 14:50:20 -0800 Subject: Inbox::streamNotices() with deletion compensation: inbox paging should more or less work with deleted items now. No change in efficiency for the common case where nothing's deleted: does the same bulk fetch of just the notices we think we'll need as before, then if we turned up short keeps checking one by one until we've filled up to our $limit. This can leave us with overlap between pages, but we already have that when new messages come in between clicks; seems to be the lesser of evils versus not getting a 'before' button. More permanent fix for that will be to switch timeline paging in the UI to use notice IDs. --- classes/Inbox.php | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++--- classes/User.php | 14 ++++-------- 2 files changed, 67 insertions(+), 13 deletions(-) diff --git a/classes/Inbox.php b/classes/Inbox.php index 26b27d2b5..029410799 100644 --- a/classes/Inbox.php +++ b/classes/Inbox.php @@ -32,6 +32,7 @@ require_once INSTALLDIR.'/classes/Memcached_DataObject.php'; class Inbox extends Memcached_DataObject { const BOXCAR = 128; + const MAX_NOTICES = 1024; ###START_AUTOCODE /* the code below is auto generated do not remove the above tag */ @@ -81,7 +82,7 @@ class Inbox extends Memcached_DataObject $ni->selectAdd(); $ni->selectAdd('notice_id'); $ni->orderBy('notice_id DESC'); - $ni->limit(0, 1024); + $ni->limit(0, self::MAX_NOTICES); if ($ni->find()) { while($ni->fetch()) { @@ -115,9 +116,11 @@ class Inbox extends Memcached_DataObject $result = $inbox->query(sprintf('UPDATE inbox '. 'set notice_ids = concat(cast(0x%08x as binary(4)), '. - 'substr(notice_ids, 1, 4092)) '. + 'substr(notice_ids, 1, %d)) '. 'WHERE user_id = %d', - $notice_id, $user_id)); + $notice_id, + 4 * (self::MAX_NOTICES - 1), + $user_id)); if ($result) { self::blow('inbox:user_id:%d', $user_id); @@ -173,4 +176,61 @@ class Inbox extends Memcached_DataObject 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. + * + * 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 $since obsolete/ignored + * @param mixed $own ignored? + * @return array of Notice objects + * + * @todo consider repacking the inbox when this happens? + */ + function streamNotices($user_id, $offset, $limit, $since_id, $max_id, $since, $own=false) + { + $ids = self::stream($user_id, $offset, self::MAX_NOTICES, $since_id, $max_id, $since, $own); + + // Do a bulk lookup for the first $limit items + // Fast path when nothing's deleted. + $firstChunk = array_slice($ids, 0, $limit); + $notices = Notice::getStreamByIds($firstChunk); + + $wanted = count($firstChunk); // raw entry count in the inbox up to our $limit + if ($notices->N >= $wanted) { + common_log(LOG_DEBUG, __METHOD__ . " ok got $wanted items"); + return $notices; + } + + common_log(LOG_DEBUG, __METHOD__ . " got $notices->N of $wanted items, getting more"); + // 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) { + common_log(LOG_DEBUG, __METHOD__ . " got another one"); + $items[] = $notice; + } else { + common_log(LOG_DEBUG, __METHOD__ . " skipping another deleted one"); + } + } + return new ArrayWrapper($items); + } } diff --git a/classes/User.php b/classes/User.php index 0ab816b57..72c3f39e9 100644 --- a/classes/User.php +++ b/classes/User.php @@ -502,28 +502,22 @@ class User extends Memcached_DataObject function noticesWithFriends($offset=0, $limit=NOTICES_PER_PAGE, $since_id=0, $before_id=0, $since=null) { - $ids = Inbox::stream($this->id, $offset, $limit, $since_id, $before_id, $since, false); - return Notice::getStreamByIds($ids); + return Inbox::streamNotices($this->id, $offset, $limit, $since_id, $before_id, $since, false); } function noticeInbox($offset=0, $limit=NOTICES_PER_PAGE, $since_id=0, $before_id=0, $since=null) { - $ids = Inbox::stream($this->id, $offset, $limit, $since_id, $before_id, $since, true); - return Notice::getStreamByIds($ids); + return Inbox::streamNotices($this->id, $offset, $limit, $since_id, $before_id, $since, true); } function friendsTimeline($offset=0, $limit=NOTICES_PER_PAGE, $since_id=0, $before_id=0, $since=null) { - $ids = Inbox::stream($this->id, $offset, $limit, $since_id, $before_id, $since, false); - - return Notice::getStreamByIds($ids); + return Inbox::streamNotices($this->id, $offset, $limit, $since_id, $before_id, $since, false); } function ownFriendsTimeline($offset=0, $limit=NOTICES_PER_PAGE, $since_id=0, $before_id=0, $since=null) { - $ids = Inbox::stream($this->id, $offset, $limit, $since_id, $before_id, $since, true); - - return Notice::getStreamByIds($ids); + return Inbox::streamNotices($this->id, $offset, $limit, $since_id, $before_id, $since, true); } function blowFavesCache() -- cgit v1.2.3-54-g00ecf From 4502bea9a86fe5992eb9b359d90f0c1f004998c1 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Thu, 4 Feb 2010 15:16:27 -0800 Subject: drop debug messages from inbox deletion fix --- classes/Inbox.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/classes/Inbox.php b/classes/Inbox.php index 029410799..be62611a1 100644 --- a/classes/Inbox.php +++ b/classes/Inbox.php @@ -212,11 +212,9 @@ class Inbox extends Memcached_DataObject $wanted = count($firstChunk); // raw entry count in the inbox up to our $limit if ($notices->N >= $wanted) { - common_log(LOG_DEBUG, __METHOD__ . " ok got $wanted items"); return $notices; } - common_log(LOG_DEBUG, __METHOD__ . " got $notices->N of $wanted items, getting more"); // There were deleted notices, we'll need to look for more. assert($notices instanceof ArrayWrapper); $items = $notices->_items; @@ -225,10 +223,8 @@ class Inbox extends Memcached_DataObject while (count($items) < $wanted && count($remainder) > 0) { $notice = Notice::staticGet(array_shift($remainder)); if ($notice) { - common_log(LOG_DEBUG, __METHOD__ . " got another one"); $items[] = $notice; } else { - common_log(LOG_DEBUG, __METHOD__ . " skipping another deleted one"); } } return new ArrayWrapper($items); -- cgit v1.2.3-54-g00ecf