From 2272cc244d04ff974cb3ef23538a6fc7a1cee277 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Thu, 6 Mar 2014 04:36:00 +0100 Subject: [PATCH] Removed Inbox from core (unused since 4b2a66ed29091209c05d74755e42f96265c846ce) Added the following FIXME: How should a Twitter user get their Inbox filled with foreign tweets? Every imported Twitter user has a profile in the Profile table, so we could setup a Subscription entry for each of those, meaning they get collected in the InboxNoticeStream... But this would mean a lot of unnecessary entries and listings that generally just point to the locked down Twitter service. Let's figure out a good relation so we can connect any profile to any imported foreign notice, so it shows up in the "all" feed. --- classes/Inbox.php | 128 ------------------ classes/Notice.php | 27 ---- classes/User.php | 14 -- db/core.php | 1 - lib/distribqueuehandler.php | 8 -- lib/inboxnoticestream.php | 3 +- lib/inboxtagcloudsection.php | 68 ++++------ .../TwitterBridge/lib/tweetinqueuehandler.php | 5 +- plugins/TwitterBridge/scripts/streamtest.php | 6 +- scripts/upgrade.php | 45 ------ 10 files changed, 34 insertions(+), 271 deletions(-) delete mode 100644 classes/Inbox.php diff --git a/classes/Inbox.php b/classes/Inbox.php deleted file mode 100644 index 429d2ae02b..0000000000 --- a/classes/Inbox.php +++ /dev/null @@ -1,128 +0,0 @@ -. - * - * @category Data - * @package StatusNet - * @author Evan Prodromou - * @copyright 2009 StatusNet Inc. - * @license http://www.fsf.org/licensing/licenses/agpl-3.0.html GNU Affero General Public License version 3.0 - * @link http://status.net/ - */ - -class Inbox extends Managed_DataObject { - const BOXCAR = 128; - const MAX_NOTICES = 1024; - - ###START_AUTOCODE - /* the code below is auto generated do not remove the above tag */ - - public $__table = 'inbox'; // table name - public $user_id; // int(4) primary_key not_null - public $notice_ids; // blob - - /* the code above is auto generated do not remove the tag below */ - ###END_AUTOCODE - - public static function schemaDef() - { - return array( - 'fields' => array( - 'user_id' => array('type' => 'int', 'not null' => true, 'description' => 'user receiving the notice'), - 'notice_ids' => array('type' => 'blob', 'description' => 'packed list of notice ids'), - ), - 'primary key' => array('user_id'), - 'foreign keys' => array( - 'inbox_user_id_fkey' => array('user', array('user_id' => 'id')), - ), - ); - } - - /** - * Append the given notice to the given user's inbox. - * Caching updates are managed for the inbox itself. - * - * If the notice is already in this inbox, the second - * add will be silently dropped. - * - * @param int @user_id - * @param int $notice_id - * @return boolean success - */ - static function insertNotice(Notice $notice, $user_id) - { - // Going straight to the DB rather than trusting our caching - // during an update. Note: not using DB_DataObject::staticGet, - // which is unsafe to use directly (in-process caching causes - // memory leaks, which accumulate in queue processes). - $inbox = new Inbox(); - $inbox->get('user_id', $user_id); - - if (empty($inbox)) { - return false; - } - - $ids = $inbox->unpack(); - if (in_array(intval($notice->id), $ids)) { - // Already in there, we probably re-ran some inbox adds - // due to an error. Skip the dupe silently. - return true; - } - - $result = $inbox->query(sprintf('UPDATE inbox '. - 'SET notice_ids = concat(cast(0x%08x as binary(4)), '. - 'SUBSTR(notice_ids, 1, %d)) '. - 'WHERE user_id = %d', - $notice->id, - 4 * (self::MAX_NOTICES - 1), - $user_id)); - - if ($result !== false) { - self::blow('inbox:user_id:%d', $user_id); - } - - return $result; - } - - static function bulkInsert(Notice $notice, array $user_ids) - { - foreach ($user_ids as $user_id) - { - self::insertNotice($notice, $user_id); - } - } - - /** - * Saves a list of integer notice_ids into a packed blob in this object. - * @param array $ids list of integer notice_ids - */ - function pack(array $ids) - { - $this->notice_ids = call_user_func_array('pack', array_merge(array('N*'), $ids)); - } - - /** - * @return array of integer notice_ids - */ - function unpack() - { - return unpack('N*', $this->notice_ids); - } -} diff --git a/classes/Notice.php b/classes/Notice.php index f0a7a85bff..3fea531081 100644 --- a/classes/Notice.php +++ b/classes/Notice.php @@ -1064,33 +1064,6 @@ class Notice extends Managed_DataObject return $ni; } - /** - * Adds this notice to the inboxes of each local user who should receive - * it, based on author subscriptions, group memberships, and @-replies. - * - * Warning: running a second time currently will make items appear - * multiple times in users' inboxes. - * - * @fixme make more robust against errors - * @fixme break up massive deliveries to smaller background tasks - * - * @param array $groups optional list of Group objects; - * if left empty, will be loaded from group_inbox records - * @param array $recipient optional list of reply profile ids - * if left empty, will be loaded from reply records - */ - function addToInboxes(array $groups=null, array $recipients=null) - { - $ni = $this->whoGets($groups, $recipients); - - $ids = array_keys($ni); - - // Bulk insert - Inbox::bulkInsert($this, $ids); - - return; - } - function getSubscribedUsers() { $user = new User(); diff --git a/classes/User.php b/classes/User.php index 540965209d..2929e1d332 100644 --- a/classes/User.php +++ b/classes/User.php @@ -325,20 +325,6 @@ class User extends Managed_DataObject return false; } - // Everyone gets an inbox - - $inbox = new Inbox(); - - $inbox->user_id = $user->id; - $inbox->notice_ids = ''; - - $result = $inbox->insert(); - - if (!$result) { - common_log_db_error($inbox, 'INSERT', __FILE__); - return false; - } - // Everyone is subscribed to themself $subscription = new Subscription(); diff --git a/db/core.php b/db/core.php index b820b3971e..5a420cc2b3 100644 --- a/db/core.php +++ b/db/core.php @@ -80,7 +80,6 @@ $classes = array('Schema_version', 'Location_namespace', 'Login_token', 'User_location_prefs', - 'Inbox', 'User_im_prefs', 'Conversation', 'Local_group', diff --git a/lib/distribqueuehandler.php b/lib/distribqueuehandler.php index a7519c1d50..4a671fe802 100644 --- a/lib/distribqueuehandler.php +++ b/lib/distribqueuehandler.php @@ -58,19 +58,11 @@ class DistribQueueHandler * If this function indicates failure, a warning will be logged * and the item is placed back in the queue to be re-run. * - * @fixme addToInboxes is known to fail sometimes with large recipient sets - * * @param Notice $notice * @return boolean true on success, false on failure */ function handle($notice) { - try { - $notice->addToInboxes(); - } catch (Exception $e) { - $this->logit($notice, $e); - } - try { $notice->sendReplyNotifications(); } catch (Exception $e) { diff --git a/lib/inboxnoticestream.php b/lib/inboxnoticestream.php index 0eb791d70a..a8633314d5 100644 --- a/lib/inboxnoticestream.php +++ b/lib/inboxnoticestream.php @@ -57,8 +57,7 @@ class InboxNoticeStream extends ScopingNoticeStream if ($scoped === null) { $scoped = Profile::current(); } - // Note: we don't use CachingNoticeStream since RawInboxNoticeStream - // uses Inbox::getKV(), which is cached. + // FIXME: we don't use CachingNoticeStream - but maybe we should? parent::__construct(new RawInboxNoticeStream($target), $scoped); } } diff --git a/lib/inboxtagcloudsection.php b/lib/inboxtagcloudsection.php index 05c688fc14..d19f76366d 100644 --- a/lib/inboxtagcloudsection.php +++ b/lib/inboxtagcloudsection.php @@ -42,6 +42,8 @@ if (!defined('STATUSNET') && !defined('LACONICA')) { */ class InboxTagCloudSection extends TagCloudSection { + const MAX_NOTICES = 1024; // legacy value for "Inbox" table size when that existed + protected $target = null; function __construct($out=null, Profile $target) @@ -60,52 +62,42 @@ class InboxTagCloudSection extends TagCloudSection { $profile = Profile::current(); - $keypart = sprintf('Inbox:notice_tag:%d:%d', $this->target, - $profile instanceof Profile ? $profile->id : 0); + $stream = new InboxNoticeStream($this->target, $profile); - $tag = Memcached_DataObject::cacheGet($keypart); + $ids = $stream->getNoticeIds(0, self::MAX_NOTICES, null, null); - if ($tag === false) { + if (empty($ids)) { + $tag = array(); + } else { + $weightexpr = common_sql_weight('notice_tag.created', common_config('tag', 'dropoff')); + // @fixme should we use the cutoff too? Doesn't help with indexing per-user. - $stream = new InboxNoticeStream($this->target, $profile); + $qry = 'SELECT notice_tag.tag, '. + $weightexpr . ' as weight ' . + 'FROM notice_tag JOIN notice ' . + 'ON notice_tag.notice_id = notice.id ' . + 'WHERE notice.id in (' . implode(',', $ids) . ')'. + 'GROUP BY notice_tag.tag ' . + 'ORDER BY weight DESC '; - $ids = $stream->getNoticeIds(0, Inbox::MAX_NOTICES, null, null); + $limit = TAGS_PER_SECTION; + $offset = 0; - if (empty($ids)) { - $tag = array(); + if (common_config('db','type') == 'pgsql') { + $qry .= ' LIMIT ' . $limit . ' OFFSET ' . $offset; } else { - $weightexpr = common_sql_weight('notice_tag.created', common_config('tag', 'dropoff')); - // @fixme should we use the cutoff too? Doesn't help with indexing per-user. - - $qry = 'SELECT notice_tag.tag, '. - $weightexpr . ' as weight ' . - 'FROM notice_tag JOIN notice ' . - 'ON notice_tag.notice_id = notice.id ' . - 'WHERE notice.id in (' . implode(',', $ids) . ')'. - 'GROUP BY notice_tag.tag ' . - 'ORDER BY weight DESC '; - - $limit = TAGS_PER_SECTION; - $offset = 0; - - if (common_config('db','type') == 'pgsql') { - $qry .= ' LIMIT ' . $limit . ' OFFSET ' . $offset; - } else { - $qry .= ' LIMIT ' . $offset . ', ' . $limit; - } - - $t = new Notice_tag(); - - $t->query($qry); - - $tag = array(); - - while ($t->fetch()) { - $tag[] = clone($t); - } + $qry .= ' LIMIT ' . $offset . ', ' . $limit; } - Memcached_DataObject::cacheSet($keypart, $tag, 3600); + $t = new Notice_tag(); + + $t->query($qry); + + $tag = array(); + + while ($t->fetch()) { + $tag[] = clone($t); + } } return new ArrayWrapper($tag); diff --git a/plugins/TwitterBridge/lib/tweetinqueuehandler.php b/plugins/TwitterBridge/lib/tweetinqueuehandler.php index cc4d2f47d4..3aa9e5817d 100644 --- a/plugins/TwitterBridge/lib/tweetinqueuehandler.php +++ b/plugins/TwitterBridge/lib/tweetinqueuehandler.php @@ -55,9 +55,8 @@ class TweetInQueueHandler extends QueueHandler if ($flink instanceof Foreign_link) { common_log(LOG_DEBUG, "TweetInQueueHandler - Got flink so add notice ". $notice->id." to inbox ".$flink->user_id); - // @fixme this should go through more regular channels? - Inbox::insertNotice($notice, $flink->user_id); - }else { + // FIXME: How should a Twitter user get their Inbox filled with foreign tweets? + } else { common_log(LOG_DEBUG, "TweetInQueueHandler - No flink found for foreign user ".$receiver); } } diff --git a/plugins/TwitterBridge/scripts/streamtest.php b/plugins/TwitterBridge/scripts/streamtest.php index c42fa38f10..4e8340bb3f 100644 --- a/plugins/TwitterBridge/scripts/streamtest.php +++ b/plugins/TwitterBridge/scripts/streamtest.php @@ -170,11 +170,7 @@ $stream->hookEvent('status', function($data, $context) { $importer = new TwitterImport(); printf("\timporting..."); $notice = $importer->importStatus($data); - if ($notice instanceof Notice) { - global $myuser; - Inbox::insertNotice($notice, $myuser->id); - printf(" %s\n", $notice->id); - } else { + if (!$notice instanceof Notice) { printf(" FAIL\n"); } } diff --git a/scripts/upgrade.php b/scripts/upgrade.php index a6c7ec40ff..9ae95e2562 100644 --- a/scripts/upgrade.php +++ b/scripts/upgrade.php @@ -42,7 +42,6 @@ function main() fixupNoticeRendered(); fixupNoticeConversation(); initConversation(); - initInbox(); fixupGroupURI(); initGroupProfileId(); @@ -198,50 +197,6 @@ function initConversation() printfnq("DONE.\n"); } -function initInbox() -{ - printfnq("Ensuring all users have an inbox..."); - - $user = new User(); - $user->whereAdd('not exists (select user_id from inbox where user_id = user.id)'); - $user->orderBy('id'); - - if ($user->find()) { - - while ($user->fetch()) { - - try { - $notice = new Notice(); - - $notice->selectAdd(); - $notice->selectAdd('id'); - $notice->joinAdd(array('profile_id', 'subscription:subscribed')); - $notice->whereAdd('subscription.subscriber = ' . $user->id); - $notice->whereAdd('notice.created >= subscription.created'); - - $ids = array(); - - if ($notice->find()) { - while ($notice->fetch()) { - $ids[] = $notice->id; - } - } - - $notice = null; - - $inbox = new Inbox(); - $inbox->user_id = $user->id; - $inbox->pack($ids); - $inbox->insert(); - } catch (Exception $e) { - printv("Error initializing inbox: " . $e->getMessage()); - } - } - } - - printfnq("DONE.\n"); -} - function initGroupProfileId() { printfnq("Ensuring all User_group entries have a Profile and profile_id...");