From 65dbf485e401cfd04c32acd7e635b377928e4fab Mon Sep 17 00:00:00 2001 From: Zach Copley Date: Thu, 24 Feb 2011 12:52:31 -0800 Subject: [PATCH 1/6] FacebookBridge - make a huge fuss if we can't create a valid Facebookclient --- plugins/FacebookBridge/lib/facebookclient.php | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/plugins/FacebookBridge/lib/facebookclient.php b/plugins/FacebookBridge/lib/facebookclient.php index 030a75caed..80e3331824 100644 --- a/plugins/FacebookBridge/lib/facebookclient.php +++ b/plugins/FacebookBridge/lib/facebookclient.php @@ -51,7 +51,14 @@ class Facebookclient function __construct($notice) { $this->facebook = self::getFacebook(); - $this->notice = $notice; + + if (empty($this->facebook)) { + throw new FacebookApiException( + "Could not create Facebook client! Bad application ID or secret?" + ); + } + + $this->notice = $notice; $this->flink = Foreign_link::getByUserID( $notice->profile_id, @@ -89,6 +96,22 @@ class Facebookclient $secret = common_config('facebook', 'global_secret'); } + if (empty($appId)) { + common_log( + LOG_WARNING, + "Couldn't find Facebook application ID!", + __FILE__ + ); + } + + if (empty($secret)) { + common_log( + LOG_WARNING, + "Couldn't find Facebook application ID!", + __FILE__ + ); + } + return new Facebook( array( 'appId' => $appId, From c44a622449757fe13dc30ce120af07aa03ac5dc4 Mon Sep 17 00:00:00 2001 From: Zach Copley Date: Thu, 24 Feb 2011 12:59:37 -0800 Subject: [PATCH 2/6] FacebookBridge - Add lots of debug output (revert me) --- plugins/FacebookBridge/lib/facebookclient.php | 11 ++++++++ .../lib/facebookqueuehandler.php | 26 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/plugins/FacebookBridge/lib/facebookclient.php b/plugins/FacebookBridge/lib/facebookclient.php index 80e3331824..da8c95c9f2 100644 --- a/plugins/FacebookBridge/lib/facebookclient.php +++ b/plugins/FacebookBridge/lib/facebookclient.php @@ -128,6 +128,7 @@ class Facebookclient */ static function facebookBroadcastNotice($notice) { + common_debug("ZZZZZ facebookBroadcastNotice() - entered ", __FILE__); $client = new Facebookclient($notice); return $client->sendNotice(); } @@ -137,6 +138,8 @@ class Facebookclient */ function isFacebookBound() { + common_debug("ZZZZZ isFacebookBound() - entered", __FILE__); + if (empty($this->flink)) { // User hasn't setup bridging return false; @@ -175,6 +178,14 @@ class Facebookclient return true; } + common_debug( + sprintf( + "ZZZZZ isFacebookBound() - notice %d this notice does not go to Facebook", + $this->notice->id + ), + __FILE__ + ); + return false; } diff --git a/plugins/FacebookBridge/lib/facebookqueuehandler.php b/plugins/FacebookBridge/lib/facebookqueuehandler.php index 1e82ff01b1..f1e857090b 100644 --- a/plugins/FacebookBridge/lib/facebookqueuehandler.php +++ b/plugins/FacebookBridge/lib/facebookqueuehandler.php @@ -40,9 +40,35 @@ class FacebookQueueHandler extends QueueHandler function handle($notice) { + common_debug( + sprintf( + 'ZZZZZ handle() - Looking at notice %d', + $notice->id + ), + __FILE__ + ); + if ($this->_isLocal($notice)) { + + common_debug( + sprintf( + 'ZZZZZ handle() - notice %d is local; will try sending to Facebook.', + $notice->id + ), + __FILE__ + ); + return Facebookclient::facebookBroadcastNotice($notice); } + + common_debug( + sprintf( + 'ZZZZZ handle() - notice %d was not a local notice, so we wont rebroadcast it.', + $notice->id + ), + __FILE__ + ); + return true; } From de6d46ea4b2a59bdcc66333650e585703401aadb Mon Sep 17 00:00:00 2001 From: Zach Copley Date: Thu, 24 Feb 2011 13:29:56 -0800 Subject: [PATCH 3/6] FacebookBridge - dequeue messages that aren't bound for Facebook --- plugins/FacebookBridge/lib/facebookclient.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/plugins/FacebookBridge/lib/facebookclient.php b/plugins/FacebookBridge/lib/facebookclient.php index da8c95c9f2..6037ad0f2d 100644 --- a/plugins/FacebookBridge/lib/facebookclient.php +++ b/plugins/FacebookBridge/lib/facebookclient.php @@ -208,6 +208,9 @@ class Facebookclient return $this->sendGraph(); } } + + // dequeue + return true; } /* From 7d50189ec2d7523324a7e724715058c6fb91b7d3 Mon Sep 17 00:00:00 2001 From: Zach Copley Date: Thu, 24 Feb 2011 13:57:21 -0800 Subject: [PATCH 4/6] FacebookBridge - Don't hinder autoloading if the Facebook ID and secret aren't set --- .../FacebookBridge/FacebookBridgePlugin.php | 38 ++++++++----------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/plugins/FacebookBridge/FacebookBridgePlugin.php b/plugins/FacebookBridge/FacebookBridgePlugin.php index 7e17c2d7ec..37b931e23e 100644 --- a/plugins/FacebookBridge/FacebookBridgePlugin.php +++ b/plugins/FacebookBridge/FacebookBridgePlugin.php @@ -179,28 +179,22 @@ class FacebookBridgePlugin extends Plugin // Always add the admin panel route $m->connect('admin/facebook', array('action' => 'facebookadminpanel')); - // Only add these routes if an application has been setup on - // Facebook for the plugin to use. - if ($this->hasApplication()) { - - $m->connect( - 'main/facebooklogin', - array('action' => 'facebooklogin') - ); - $m->connect( - 'main/facebookfinishlogin', - array('action' => 'facebookfinishlogin') - ); - $m->connect( - 'settings/facebook', - array('action' => 'facebooksettings') - ); - $m->connect( - 'facebook/deauthorize', - array('action' => 'facebookdeauthorize') - ); - - } + $m->connect( + 'main/facebooklogin', + array('action' => 'facebooklogin') + ); + $m->connect( + 'main/facebookfinishlogin', + array('action' => 'facebookfinishlogin') + ); + $m->connect( + 'settings/facebook', + array('action' => 'facebooksettings') + ); + $m->connect( + 'facebook/deauthorize', + array('action' => 'facebookdeauthorize') + ); return true; } From 77c280a48b3c3a5582b20a2a1b7c1b0d045760c1 Mon Sep 17 00:00:00 2001 From: Zach Copley Date: Thu, 24 Feb 2011 14:21:50 -0800 Subject: [PATCH 5/6] Revert "FacebookBridge - Add lots of debug output (revert me)" This reverts commit c44a622449757fe13dc30ce120af07aa03ac5dc4. --- plugins/FacebookBridge/lib/facebookclient.php | 11 -------- .../lib/facebookqueuehandler.php | 26 ------------------- 2 files changed, 37 deletions(-) diff --git a/plugins/FacebookBridge/lib/facebookclient.php b/plugins/FacebookBridge/lib/facebookclient.php index 6037ad0f2d..516f252d98 100644 --- a/plugins/FacebookBridge/lib/facebookclient.php +++ b/plugins/FacebookBridge/lib/facebookclient.php @@ -128,7 +128,6 @@ class Facebookclient */ static function facebookBroadcastNotice($notice) { - common_debug("ZZZZZ facebookBroadcastNotice() - entered ", __FILE__); $client = new Facebookclient($notice); return $client->sendNotice(); } @@ -138,8 +137,6 @@ class Facebookclient */ function isFacebookBound() { - common_debug("ZZZZZ isFacebookBound() - entered", __FILE__); - if (empty($this->flink)) { // User hasn't setup bridging return false; @@ -178,14 +175,6 @@ class Facebookclient return true; } - common_debug( - sprintf( - "ZZZZZ isFacebookBound() - notice %d this notice does not go to Facebook", - $this->notice->id - ), - __FILE__ - ); - return false; } diff --git a/plugins/FacebookBridge/lib/facebookqueuehandler.php b/plugins/FacebookBridge/lib/facebookqueuehandler.php index f1e857090b..1e82ff01b1 100644 --- a/plugins/FacebookBridge/lib/facebookqueuehandler.php +++ b/plugins/FacebookBridge/lib/facebookqueuehandler.php @@ -40,35 +40,9 @@ class FacebookQueueHandler extends QueueHandler function handle($notice) { - common_debug( - sprintf( - 'ZZZZZ handle() - Looking at notice %d', - $notice->id - ), - __FILE__ - ); - if ($this->_isLocal($notice)) { - - common_debug( - sprintf( - 'ZZZZZ handle() - notice %d is local; will try sending to Facebook.', - $notice->id - ), - __FILE__ - ); - return Facebookclient::facebookBroadcastNotice($notice); } - - common_debug( - sprintf( - 'ZZZZZ handle() - notice %d was not a local notice, so we wont rebroadcast it.', - $notice->id - ), - __FILE__ - ); - return true; } From 55b1f3d84c7390528634db5b95396a181e13003c Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Fri, 25 Feb 2011 11:04:57 -0800 Subject: [PATCH 6/6] Scalability work on user backup stream generation. UserActivityStream -- used to create a full activity stream including subscriptions, favorites, notices, etc -- normally buffers everything into memory at once. This is infeasible for accounts with long histories of serious usage; it can take tens of seconds just to pull all records from the database, and working with them all in memory is very likely to hit resource limits. This commit adds an alternate mode for this class which avoids pulling notices until during the actual output. Instead of pre-sorting and buffering all the notices, empty spaces between the other activities are filled in with notices as we're making output. This means more smaller queries spread out during operations, and less stuff kept in memory. Callers (backupaccount action, and backupuser.php) which can stream their output pass an $outputMode param of UserActivityStream::OUTPUT_RAW, and during getString() it'll send straight to output as well as slurping the notices in this extra funky fashion. Other callers will let it default to the OUTPUT_STRING mode, which keeps the previous behavior. There should be a better way to do this, swapping out the stringer output for raw output more consitently. --- actions/backupaccount.php | 4 +- lib/useractivitystream.php | 89 ++++++++++++++++++++++++++++++++++++-- scripts/backupuser.php | 2 +- 3 files changed, 90 insertions(+), 5 deletions(-) diff --git a/actions/backupaccount.php b/actions/backupaccount.php index 928aba69ce..35458ca796 100644 --- a/actions/backupaccount.php +++ b/actions/backupaccount.php @@ -118,11 +118,13 @@ class BackupaccountAction extends Action { $cur = common_current_user(); - $stream = new UserActivityStream($cur); + $stream = new UserActivityStream($cur, true, UserActivityStream::OUTPUT_RAW); header('Content-Disposition: attachment; filename='.$cur->nickname.'.atom'); header('Content-Type: application/atom+xml; charset=utf-8'); + // @fixme atom feed logic is in getString... + // but we just want it to output to the outputter. $this->raw($stream->getString()); } diff --git a/lib/useractivitystream.php b/lib/useractivitystream.php index 53d0107aa9..d1e3e28fb8 100644 --- a/lib/useractivitystream.php +++ b/lib/useractivitystream.php @@ -29,15 +29,48 @@ class UserActivityStream extends AtomUserNoticeFeed { public $activities = array(); - function __construct($user, $indent = true) + const OUTPUT_STRING = 1; + const OUTPUT_RAW = 2; + public $outputMode = self::OUTPUT_STRING; + + /** + * + * @param User $user + * @param boolean $indent + * @param boolean $outputMode: UserActivityStream::OUTPUT_STRING to return a string, + * or UserActivityStream::OUTPUT_RAW to go to raw output. + * Raw output mode will attempt to stream, keeping less + * data in memory but will leave $this->activities incomplete. + */ + function __construct($user, $indent = true, $outputMode = UserActivityStream::OUTPUT_STRING) { parent::__construct($user, null, $indent); + $this->outputMode = $outputMode; + if ($this->outputMode == self::OUTPUT_STRING) { + // String buffering? Grab all the notices now. + $notices = $this->getNotices(); + } elseif ($this->outputMode == self::OUTPUT_RAW) { + // Raw output... need to restructure from the stringer init. + $this->xw = new XMLWriter(); + $this->xw->openURI('php://output'); + if(is_null($indent)) { + $indent = common_config('site', 'indent'); + } + $this->xw->setIndent($indent); + + // We'll fetch notices later. + $notices = array(); + } else { + throw new Exception('Invalid outputMode provided to ' . __METHOD__); + } + + // Assume that everything but notices is feasible + // to pull at once and work with in memory... $subscriptions = $this->getSubscriptions(); $subscribers = $this->getSubscribers(); $groups = $this->getGroups(); $faves = $this->getFaves(); - $notices = $this->getNotices(); $objs = array_merge($subscriptions, $subscribers, $groups, $faves, $notices); @@ -45,16 +78,44 @@ class UserActivityStream extends AtomUserNoticeFeed usort($objs, 'UserActivityStream::compareObject'); + // We'll keep these around for later, and interleave them into + // the output stream with the user's notices. foreach ($objs as $obj) { $this->activities[] = $obj->asActivity(); } } + /** + * Interleave the pre-sorted subs/groups/faves with the user's + * notices, all in reverse chron order. + */ function renderEntries() { + $end = time() + 1; foreach ($this->activities as $act) { + $start = $act->time; + + if ($this->outputMode == self::OUTPUT_RAW && $start != $end) { + // In raw mode, we haven't pre-fetched notices. + // Grab the chunks of notices between other activities. + $notices = $this->getNoticesBetween($start, $end); + foreach ($notices as $noticeAct) { + $noticeAct->asActivity()->outputTo($this, false, false); + } + } + // Only show the author sub-element if it's different from default user $act->outputTo($this, false, ($act->actor->id != $this->user->uri)); + + $end = $start; + } + + if ($this->outputMode == self::OUTPUT_RAW) { + // Grab anything after the last pre-sorted activity. + $notices = $this->getNoticesBetween(0, $end); + foreach ($notices as $noticeAct) { + $noticeAct->asActivity()->outputTo($this, false, false); + } } } @@ -121,7 +182,13 @@ class UserActivityStream extends AtomUserNoticeFeed return $faves; } - function getNotices() + /** + * + * @param int $start unix timestamp for earliest + * @param int $end unix timestamp for latest + * @return array of Notice objects + */ + function getNoticesBetween($start=0, $end=0) { $notices = array(); @@ -129,6 +196,17 @@ class UserActivityStream extends AtomUserNoticeFeed $notice->profile_id = $this->user->id; + if ($start) { + $tsstart = common_sql_date($start); + $notice->whereAdd("created >= '$tsstart'"); + } + if ($end) { + $tsend = common_sql_date($end); + $notice->whereAdd("created < '$tsend'"); + } + + $notice->orderBy('created DESC'); + if ($notice->find()) { while ($notice->fetch()) { $notices[] = clone($notice); @@ -138,6 +216,11 @@ class UserActivityStream extends AtomUserNoticeFeed return $notices; } + function getNotices() + { + return $this->getNoticesBetween(); + } + function getGroups() { $groups = array(); diff --git a/scripts/backupuser.php b/scripts/backupuser.php index 49fc1cefdc..ee2951fc8f 100644 --- a/scripts/backupuser.php +++ b/scripts/backupuser.php @@ -36,7 +36,7 @@ require_once INSTALLDIR.'/scripts/commandline.inc'; try { $user = getUser(); - $actstr = new UserActivityStream($user); + $actstr = new UserActivityStream($user, true, UserActivityStream::OUTPUT_RAW); print $actstr->getString(); } catch (Exception $e) { print $e->getMessage()."\n";