From f7add6f25f37780fde2269b254c237caea9ef98d Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Mon, 24 May 2010 07:47:15 -0700 Subject: [PATCH 1/8] Handle funky notice deletion cases more gracefully: if we already have a deleted_notice entry, don't freak out when we try to save it again on the second try. --- classes/Notice.php | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/classes/Notice.php b/classes/Notice.php index e173a24690..d85c8cd33a 100644 --- a/classes/Notice.php +++ b/classes/Notice.php @@ -97,15 +97,20 @@ class Notice extends Memcached_DataObject // For auditing purposes, save a record that the notice // was deleted. - $deleted = new Deleted_notice(); + // @fixme we have some cases where things get re-run and so the + // insert fails. + $deleted = Deleted_notice::staticGet('id', $this->id); + if (!$deleted) { + $deleted = new Deleted_notice(); - $deleted->id = $this->id; - $deleted->profile_id = $this->profile_id; - $deleted->uri = $this->uri; - $deleted->created = $this->created; - $deleted->deleted = common_sql_now(); + $deleted->id = $this->id; + $deleted->profile_id = $this->profile_id; + $deleted->uri = $this->uri; + $deleted->created = $this->created; + $deleted->deleted = common_sql_now(); - $deleted->insert(); + $deleted->insert(); + } // Clear related records From 8d8751472766d1d6b0f89616152503ad35f65ab0 Mon Sep 17 00:00:00 2001 From: Zach Copley Date: Fri, 21 May 2010 05:03:23 +0000 Subject: [PATCH 2/8] Upgrade to latest old REST API library (0.1.0) --- plugins/Facebook/facebook/facebook.php | 74 ++++++++++--------- .../facebook/facebookapi_php5_restlib.php | 56 +------------- 2 files changed, 42 insertions(+), 88 deletions(-) diff --git a/plugins/Facebook/facebook/facebook.php b/plugins/Facebook/facebook/facebook.php index 440706cbc3..76696c1d55 100644 --- a/plugins/Facebook/facebook/facebook.php +++ b/plugins/Facebook/facebook/facebook.php @@ -45,7 +45,9 @@ class Facebook { public $user; public $profile_user; public $canvas_user; + public $ext_perms = array(); protected $base_domain; + /* * Create a Facebook client like this: * @@ -104,17 +106,17 @@ class Facebook { * * For nitty-gritty details of when each of these is used, check out * http://wiki.developers.facebook.com/index.php/Verifying_The_Signature - * - * @param bool resolve_auth_token convert an auth token into a session */ - public function validate_fb_params($resolve_auth_token=true) { + public function validate_fb_params() { $this->fb_params = $this->get_valid_fb_params($_POST, 48 * 3600, 'fb_sig'); // note that with preload FQL, it's possible to receive POST params in // addition to GET, so use a different prefix to differentiate them if (!$this->fb_params) { $fb_params = $this->get_valid_fb_params($_GET, 48 * 3600, 'fb_sig'); - $fb_post_params = $this->get_valid_fb_params($_POST, 48 * 3600, 'fb_post_sig'); + $fb_post_params = $this->get_valid_fb_params($_POST, + 48 * 3600, // 48 hours + 'fb_post_sig'); $this->fb_params = array_merge($fb_params, $fb_post_params); } @@ -128,6 +130,9 @@ class Facebook { $this->fb_params['canvas_user'] : null; $this->base_domain = isset($this->fb_params['base_domain']) ? $this->fb_params['base_domain'] : null; + $this->ext_perms = isset($this->fb_params['ext_perms']) ? + explode(',', $this->fb_params['ext_perms']) + : array(); if (isset($this->fb_params['session_key'])) { $session_key = $this->fb_params['session_key']; @@ -141,13 +146,11 @@ class Facebook { $this->set_user($user, $session_key, $expires); - } - // if no Facebook parameters were found in the GET or POST variables, - // then fall back to cookies, which may have cached user information - // Cookies are also used to receive session data via the Javascript API - else if ($cookies = - $this->get_valid_fb_params($_COOKIE, null, $this->api_key)) { - + } else if ($cookies = + $this->get_valid_fb_params($_COOKIE, null, $this->api_key)) { + // if no Facebook parameters were found in the GET or POST variables, + // then fall back to cookies, which may have cached user information + // Cookies are also used to receive session data via the Javascript API $base_domain_cookie = 'base_domain_' . $this->api_key; if (isset($_COOKIE[$base_domain_cookie])) { $this->base_domain = $_COOKIE[$base_domain_cookie]; @@ -160,25 +163,6 @@ class Facebook { $cookies['session_key'], $expires); } - // finally, if we received no parameters, but the 'auth_token' GET var - // is present, then we are in the middle of auth handshake, - // so go ahead and create the session - else if ($resolve_auth_token && isset($_GET['auth_token']) && - $session = $this->do_get_session($_GET['auth_token'])) { - if ($this->generate_session_secret && - !empty($session['secret'])) { - $session_secret = $session['secret']; - } - - if (isset($session['base_domain'])) { - $this->base_domain = $session['base_domain']; - } - - $this->set_user($session['uid'], - $session['session_key'], - $session['expires'], - isset($session_secret) ? $session_secret : null); - } return !empty($this->fb_params); } @@ -309,11 +293,28 @@ class Facebook { // require_add and require_install have been removed. // see http://developer.facebook.com/news.php?blog=1&story=116 for more details - public function require_login() { - if ($user = $this->get_loggedin_user()) { + public function require_login($required_permissions = '') { + $user = $this->get_loggedin_user(); + $has_permissions = true; + + if ($required_permissions) { + $this->require_frame(); + $permissions = array_map('trim', explode(',', $required_permissions)); + foreach ($permissions as $permission) { + if (!in_array($permission, $this->ext_perms)) { + $has_permissions = false; + break; + } + } + } + + if ($user && $has_permissions) { return $user; } - $this->redirect($this->get_login_url(self::current_url(), $this->in_frame())); + + $this->redirect( + $this->get_login_url(self::current_url(), $this->in_frame(), + $required_permissions)); } public function require_frame() { @@ -342,10 +343,11 @@ class Facebook { return $page . '?' . http_build_query($params); } - public function get_login_url($next, $canvas) { + public function get_login_url($next, $canvas, $req_perms = '') { $page = self::get_facebook_url().'/login.php'; - $params = array('api_key' => $this->api_key, - 'v' => '1.0'); + $params = array('api_key' => $this->api_key, + 'v' => '1.0', + 'req_perms' => $req_perms); if ($next) { $params['next'] = $next; diff --git a/plugins/Facebook/facebook/facebookapi_php5_restlib.php b/plugins/Facebook/facebook/facebookapi_php5_restlib.php index fa1088cd00..e249a326b2 100755 --- a/plugins/Facebook/facebook/facebookapi_php5_restlib.php +++ b/plugins/Facebook/facebook/facebookapi_php5_restlib.php @@ -569,7 +569,7 @@ function toggleDisplay(id, type) { return $this->call_method('facebook.events.invite', array('eid' => $eid, 'uids' => $uids, - 'personal_message', $personal_message)); + 'personal_message' => $personal_message)); } /** @@ -1350,53 +1350,6 @@ function toggleDisplay(id, type) { ); } - /** - * Dashboard API - */ - - /** - * Set the news for the specified user. - * - * @param int $uid The user for whom you are setting news for - * @param string $news Text of news to display - * - * @return bool Success - */ - public function dashboard_setNews($uid, $news) { - return $this->call_method('facebook.dashboard.setNews', - array('uid' => $uid, - 'news' => $news) - ); - } - - /** - * Get the current news of the specified user. - * - * @param int $uid The user to get the news of - * - * @return string The text of the current news for the user - */ - public function dashboard_getNews($uid) { - return json_decode( - $this->call_method('facebook.dashboard.getNews', - array('uid' => $uid) - ), true); - } - - /** - * Set the news for the specified user. - * - * @param int $uid The user you are clearing the news of - * - * @return bool Success - */ - public function dashboard_clearNews($uid) { - return $this->call_method('facebook.dashboard.clearNews', - array('uid' => $uid) - ); - } - - /** * Creates a note with the specified title and content. @@ -2005,7 +1958,7 @@ function toggleDisplay(id, type) { * @return array A list of strings describing any compile errors for the * submitted FBML */ - function profile_setFBML($markup, + public function profile_setFBML($markup, $uid=null, $profile='', $profile_action='', @@ -3267,9 +3220,8 @@ function toggleDisplay(id, type) { } else { $get['v'] = '1.0'; } - if (isset($this->use_ssl_resources) && - $this->use_ssl_resources) { - $post['return_ssl_resources'] = true; + if (isset($this->use_ssl_resources)) { + $post['return_ssl_resources'] = (bool) $this->use_ssl_resources; } return array($get, $post); } From 777ca74500025c616ae689f9a92b3233cf8466f7 Mon Sep 17 00:00:00 2001 From: Zach Copley Date: Mon, 24 May 2010 21:25:21 +0000 Subject: [PATCH 3/8] Upgrade Facebook posting: - Use FQL to check for publish stream permission instead of old REST API - Better error handling, especially for error code 100 - More logging / better log messages --- plugins/Facebook/facebookutil.php | 320 +++++++++++++++++++++++------- 1 file changed, 247 insertions(+), 73 deletions(-) diff --git a/plugins/Facebook/facebookutil.php b/plugins/Facebook/facebookutil.php index 045891649c..e52a3deaef 100644 --- a/plugins/Facebook/facebookutil.php +++ b/plugins/Facebook/facebookutil.php @@ -81,114 +81,288 @@ function isFacebookBound($notice, $flink) { function facebookBroadcastNotice($notice) { $facebook = getFacebook(); - $flink = Foreign_link::getByUserID($notice->profile_id, FACEBOOK_SERVICE); + $flink = Foreign_link::getByUserID( + $notice->profile_id, + FACEBOOK_SERVICE + ); if (isFacebookBound($notice, $flink)) { // Okay, we're good to go, update the FB status - $status = null; $fbuid = $flink->foreign_id; $user = $flink->getUser(); - $attachments = $notice->attachments(); try { - // Get the status 'verb' (prefix) the user has set + // Check permissions - // XXX: Does this call count against our per user FB request limit? - // If so we should consider storing verb elsewhere or not storing + common_debug( + 'FacebookPlugin - checking for publish_stream permission for user ' + . "$user->nickname ($user->id), Facebook UID: $fbuid" + ); - $prefix = trim($facebook->api_client->data_getUserPreference(FACEBOOK_NOTICE_PREFIX, - $fbuid)); + // NOTE: $facebook->api_client->users_hasAppPermission('publish_stream', $fbuid) + // has been returning bogus results, so we're using FQL to check for + // publish_stream permission now - $status = "$prefix $notice->content"; + $fql = "SELECT publish_stream FROM permissions WHERE uid = $fbuid"; + $result = $facebook->api_client->fql_query($fql); - common_debug("FacebookPlugin - checking for publish_stream permission for user $user->id"); + $canPublish = 0; - $can_publish = $facebook->api_client->users_hasAppPermission('publish_stream', - $fbuid); + if (!empty($result)) { + $canPublish = $result[0]['publish_stream']; + } - common_debug("FacebookPlugin - checking for status_update permission for user $user->id"); + if ($canPublish == 1) { + common_debug( + "FacebookPlugin - $user->nickname ($user->id), Facebook UID: $fbuid " + . 'has publish_stream permission.' + ); + } else { + common_debug( + "FacebookPlugin - $user->nickname ($user->id), Facebook UID: $fbuid " + . 'does NOT have publish_stream permission. Facebook ' + . 'returned: ' . var_export($result, true) + ); + } - $can_update = $facebook->api_client->users_hasAppPermission('status_update', - $fbuid); - if (!empty($attachments) && $can_publish == 1) { - $fbattachment = format_attachments($attachments); - $facebook->api_client->stream_publish($status, $fbattachment, - null, null, $fbuid); - common_log(LOG_INFO, - "FacebookPlugin - Posted notice $notice->id w/attachment " . - "to Facebook user's stream (fbuid = $fbuid)."); - } elseif ($can_update == 1 || $can_publish == 1) { - $facebook->api_client->users_setStatus($status, $fbuid, false, true); - common_log(LOG_INFO, - "FacebookPlugin - Posted notice $notice->id to Facebook " . - "as a status update (fbuid = $fbuid)."); + common_debug( + 'FacebookPlugin - checking for status_update permission for user ' + . "$user->nickname ($user->id), Facebook UID: $fbuid. " + ); + + $canUpdate = $facebook->api_client->users_hasAppPermission( + 'status_update', + $fbuid + ); + + if ($canUpdate == 1) { + common_debug( + "FacebookPlugin - $user->nickname ($user->id), Facebook UID: $fbuid " + . 'has status_update permission.' + ); + } else { + common_debug( + "FacebookPlugin - $user->nickname ($user->id), Facebook UID: $fbuid " + .'does NOT have status_update permission. Facebook ' + . 'returned: ' . var_export($can_publish, true) + ); + } + + // Post to Facebook + + if ($notice->hasAttachments() && $canPublish == 1) { + publishStream($notice, $user, $fbuid); + } elseif ($canUpdate == 1 || $canPublish == 1) { + statusUpdate($notice, $user, $fbuid); } else { $msg = "FacebookPlugin - Not sending notice $notice->id to Facebook " . - "because user $user->nickname hasn't given the " . + "because user $user->nickname has not given the " . 'Facebook app \'status_update\' or \'publish_stream\' permission.'; common_log(LOG_WARNING, $msg); } // Finally, attempt to update the user's profile box - if ($can_publish == 1 || $can_update == 1) { - updateProfileBox($facebook, $flink, $notice); + if ($canPublish == 1 || $canUpdate == 1) { + updateProfileBox($facebook, $flink, $notice, $user); } } catch (FacebookRestClientException $e) { - - $code = $e->getCode(); - - $msg = "FacebookPlugin - Facebook returned error code $code: " . - $e->getMessage() . ' - ' . - "Unable to update Facebook status (notice $notice->id) " . - "for $user->nickname (user id: $user->id)!"; - - common_log(LOG_WARNING, $msg); - - if ($code == 100 || $code == 200 || $code == 250) { - - // 100 The account is 'inactive' (probably - this is not well documented) - // 200 The application does not have permission to operate on the passed in uid parameter. - // 250 Updating status requires the extended permission status_update or publish_stream. - // see: http://wiki.developers.facebook.com/index.php/Users.setStatus#Example_Return_XML - - remove_facebook_app($flink); - - } else if ($code == 341) { - // 341 Feed action request limit reached - Unable to update Facebook status - // Reposting immediately probably won't work, so drop the message for now. :( - - common_log(LOG_ERR, "Facebook rate limit hit: dropping notice $notice->id"); - return true; - } else { - - // Try sending again later. - // - // @fixme at the moment, returning false here could lead to an infinite loop - // if the error condition isn't actually transitory. - // - // Temporarily throwing an exception to kill the process so it'll hit our - // retry limits. - throw new Exception("Facebook error $code on notice $notice->id"); - - return false; - } - + return handleFacebookError($e, $notice, $flink); } } return true; - } -function updateProfileBox($facebook, $flink, $notice) { - $fbaction = new FacebookAction($output = 'php://output', - $indent = null, $facebook, $flink); +function handleFacebookError($e, $notice, $flink) +{ + $fbuid = $flink->foreign_id; + $user = $flink->getUser(); + $code = $e->getCode(); + $errmsg = $e->getMessage(); + + // XXX: Check for any others? + switch($code) { + case 100: // Invalid parameter + $msg = "FacebookPlugin - Facebook claims notice %d was posted with an invalid parameter (error code 100):" + . "\"%s\" (Notice details: nickname=%s, user ID=%d, Facebook ID=%d, notice content=\"%s\"). " + . "Removing notice from the Facebook queue for safety."; + common_log( + LOG_ERROR, sprintf( + $msg, + $notice->id, + $errmsg, + $user->nickname, + $user->id, + $fbuid, + $notice->content + ) + ); + return true; + break; + case 200: // Permissions error + case 250: // Updating status requires the extended permission status_update + remove_facebook_app($flink); + return true; // dequeue + break; + case 341: // Feed action request limit reached + $msg = "FacebookPlugin - User %s (User ID=%d, Facebook ID=%d) has exceeded " + . "his/her limit for posting notices to Facebook today. Dequeuing " + . "notice %d."; + common_log( + LOG_INFO, sprintf( + $msg, + $user->nickname, + $user->id, + $fbuid, + $notice->id + ) + ); + // @fixme: We want to rety at a later time when the throttling has expired + // instead of just giving up. + return true; + break; + default: + $msg = "FacebookPlugin - Facebook returned an error we don't know how to deal with while trying to " + . "post notice %d. Error code: %d, error message: \"%s\". (Notice details: " + . "nickname=%s, user ID=%d, Facebook ID=%d, notice content=\"%s\"). Re-queueing " + . "notice, and will try to send again later."; + common_log( + LOG_ERROR, sprintf( + $msg, + $notice->id, + $code, + $errmsg, + $user->nickname, + $user->id, + $fbuid, + $notice->content + ) + ); + // Re-queue and try again later + return false; + break; + } +} + +function statusUpdate($notice, $user, $fbuid) +{ + common_debug( + "FacebookPlugin - Attempting to post notice $notice->id " + . "as a status update for $user->nickname ($user->id), " + . "Facebook UID: $fbuid" + ); + + $text = formatNotice($notice, $user, $fbuid); + + $facebook = getFacebook(); + $result = $facebook->api_client->users_setStatus( + $text, + $fbuid, + false, + true + ); + + common_debug('Facebook returned: ' . var_export($result, true)); + + common_log( + LOG_INFO, + "FacebookPlugin - Posted notice $notice->id as a status " + . "update for $user->nickname ($user->id), " + . "Facebook UID: $fbuid" + ); +} + +function publishStream($notice, $user, $fbuid) +{ + common_debug( + "FacebookPlugin - Attempting to post notice $notice->id " + . "as stream item with attachment for $user->nickname ($user->id), " + . "Facebook UID: $fbuid" + ); + + $text = formatNotice($notice, $user, $fbuid); + $fbattachment = format_attachments($notice->attachments()); + + $facebook = getFacebook(); + $facebook->api_client->stream_publish( + $text, + $fbattachment, + null, + null, + $fbuid + ); + + common_debug('Facebook returned: ' . var_export($result, true)); + + common_log( + LOG_INFO, + "FacebookPlugin - Posted notice $notice->id as a stream " + . "item with attachment for $user->nickname ($user->id), " + . "Facebook UID: $fbuid" + ); +} + + +function formatNotice($notice, $user, $fbuid) +{ + // Get the status 'verb' the user has set, if any + + common_debug( + "FacebookPlugin - Looking to see if $user->nickname ($user->id), " + . "Facebook UID: $fbuid has set a verb for Facebook posting..." + ); + + $facebook = getFacebook(); + $verb = trim( + $facebook->api_client->data_getUserPreference( + FACEBOOK_NOTICE_PREFIX, + $fbuid + ) + ); + + common_debug("Facebook returned " . var_export($verb, true)); + + $text = null; + + if (!empty($verb)) { + common_debug("FacebookPlugin - found a verb: $verb"); + $text = trim($verb) . ' ' . $notice->content; + } else { + common_debug("FacebookPlugin - no verb found."); + $text = $notice->content; + } + + return $text; +} + +function updateProfileBox($facebook, $flink, $notice, $user) { + + $facebook = getFacebook(); + $fbaction = new FacebookAction( + $output = 'php://output', + $indent = null, + $facebook, + $flink + ); + + common_debug( + 'FacebookPlugin - Attempting to update profile box with ' + . "content from notice $notice->id for $user->nickname ($user->id)" + . "Facebook UID: $fbuid" + ); + $fbaction->updateProfileBox($notice); + + common_debug( + 'FacebookPlugin - finished updating profile box for ' + . "$user->nickname ($user->id) Facebook UID: $fbuid" + ); + } function format_attachments($attachments) From 1f3a16bbfb41b366bea05f5ba05bb41f44108ab8 Mon Sep 17 00:00:00 2001 From: Zach Copley Date: Mon, 24 May 2010 22:41:34 +0000 Subject: [PATCH 4/8] Clear up warnings I introduced by refactoring Facebook posting --- plugins/Facebook/facebookutil.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/plugins/Facebook/facebookutil.php b/plugins/Facebook/facebookutil.php index e52a3deaef..d573c34ac1 100644 --- a/plugins/Facebook/facebookutil.php +++ b/plugins/Facebook/facebookutil.php @@ -147,7 +147,7 @@ function facebookBroadcastNotice($notice) common_debug( "FacebookPlugin - $user->nickname ($user->id), Facebook UID: $fbuid " .'does NOT have status_update permission. Facebook ' - . 'returned: ' . var_export($can_publish, true) + . 'returned: ' . var_export($canPublish, true) ); } @@ -297,8 +297,6 @@ function publishStream($notice, $user, $fbuid) $fbuid ); - common_debug('Facebook returned: ' . var_export($result, true)); - common_log( LOG_INFO, "FacebookPlugin - Posted notice $notice->id as a stream " @@ -307,7 +305,6 @@ function publishStream($notice, $user, $fbuid) ); } - function formatNotice($notice, $user, $fbuid) { // Get the status 'verb' the user has set, if any @@ -350,6 +347,8 @@ function updateProfileBox($facebook, $flink, $notice, $user) { $flink ); + $fbuid = $flink->foreign_id; + common_debug( 'FacebookPlugin - Attempting to update profile box with ' . "content from notice $notice->id for $user->nickname ($user->id)" From 9cde924bb3f928d9df0519a9a6b995633eb45789 Mon Sep 17 00:00:00 2001 From: Zach Copley Date: Mon, 24 May 2010 23:27:53 +0000 Subject: [PATCH 5/8] Accidentally used the wrong log level (LOG ERROR instead of LOG_ERR) --- plugins/Facebook/facebookutil.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/Facebook/facebookutil.php b/plugins/Facebook/facebookutil.php index d573c34ac1..0f24f5441e 100644 --- a/plugins/Facebook/facebookutil.php +++ b/plugins/Facebook/facebookutil.php @@ -192,7 +192,7 @@ function handleFacebookError($e, $notice, $flink) . "\"%s\" (Notice details: nickname=%s, user ID=%d, Facebook ID=%d, notice content=\"%s\"). " . "Removing notice from the Facebook queue for safety."; common_log( - LOG_ERROR, sprintf( + LOG_ERR, sprintf( $msg, $notice->id, $errmsg, @@ -232,7 +232,7 @@ function handleFacebookError($e, $notice, $flink) . "nickname=%s, user ID=%d, Facebook ID=%d, notice content=\"%s\"). Re-queueing " . "notice, and will try to send again later."; common_log( - LOG_ERROR, sprintf( + LOG_ERR, sprintf( $msg, $notice->id, $code, @@ -351,7 +351,7 @@ function updateProfileBox($facebook, $flink, $notice, $user) { common_debug( 'FacebookPlugin - Attempting to update profile box with ' - . "content from notice $notice->id for $user->nickname ($user->id)" + . "content from notice $notice->id for $user->nickname ($user->id), " . "Facebook UID: $fbuid" ); From 09dab2ce5ae819c73d7984822d418c43f1fba223 Mon Sep 17 00:00:00 2001 From: Zach Copley Date: Tue, 25 May 2010 15:40:38 +0000 Subject: [PATCH 6/8] Dequeue notice when we hit any Facebook error. --- plugins/Facebook/facebookutil.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/plugins/Facebook/facebookutil.php b/plugins/Facebook/facebookutil.php index 0f24f5441e..c7b0f02c31 100644 --- a/plugins/Facebook/facebookutil.php +++ b/plugins/Facebook/facebookutil.php @@ -229,8 +229,8 @@ function handleFacebookError($e, $notice, $flink) default: $msg = "FacebookPlugin - Facebook returned an error we don't know how to deal with while trying to " . "post notice %d. Error code: %d, error message: \"%s\". (Notice details: " - . "nickname=%s, user ID=%d, Facebook ID=%d, notice content=\"%s\"). Re-queueing " - . "notice, and will try to send again later."; + . "nickname=%s, user ID=%d, Facebook ID=%d, notice content=\"%s\"). Removing notice " + . "from the Facebook queue for safety."; common_log( LOG_ERR, sprintf( $msg, @@ -243,8 +243,7 @@ function handleFacebookError($e, $notice, $flink) $notice->content ) ); - // Re-queue and try again later - return false; + return true; // dequeue break; } } From f98609204fb9b5966b9e4c9e4bf8bf605656c31c Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Tue, 25 May 2010 11:36:42 -0700 Subject: [PATCH 7/8] Backing out locale switch change to see if this affects our mystery memory leak. Revert "Locale switch cleanup: use common_switch_locale() which is safer for updating gettext state. Also moved a few calls to reduce chance of hitting an exception before switching back." This reverts commit 74a89b1fc37067d91d31bd66922053361eb4e616. --- lib/mail.php | 20 ++++++++++---------- lib/util.php | 17 ----------------- plugins/Facebook/facebookutil.php | 6 +++--- plugins/TwitterBridge/twitter.php | 6 +++--- 4 files changed, 16 insertions(+), 33 deletions(-) diff --git a/lib/mail.php b/lib/mail.php index ab5742e33d..f45b2d333f 100644 --- a/lib/mail.php +++ b/lib/mail.php @@ -224,6 +224,9 @@ function mail_subscribe_notify_profile($listenee, $other) if ($other->hasRight(Right::EMAILONSUBSCRIBE) && $listenee->email && $listenee->emailnotifysub) { + // use the recipient's localization + common_init_locale($listenee->language); + $profile = $listenee->getProfile(); $name = $profile->getBestName(); @@ -233,9 +236,6 @@ function mail_subscribe_notify_profile($listenee, $other) $recipients = $listenee->email; - // use the recipient's localization - common_switch_locale($listenee->language); - $headers = _mail_prepare_headers('subscribe', $listenee->nickname, $other->nickname); $headers['From'] = mail_notify_from(); $headers['To'] = $name . ' <' . $listenee->email . '>'; @@ -277,7 +277,7 @@ function mail_subscribe_notify_profile($listenee, $other) common_local_url('emailsettings')); // reset localization - common_switch_locale(); + common_init_locale(); mail_send($recipients, $headers, $body); } } @@ -479,7 +479,7 @@ function mail_confirm_sms($code, $nickname, $address) function mail_notify_nudge($from, $to) { - common_switch_locale($to->language); + common_init_locale($to->language); // TRANS: Subject for 'nudge' notification email $subject = sprintf(_('You\'ve been nudged by %s'), $from->nickname); @@ -497,7 +497,7 @@ function mail_notify_nudge($from, $to) $from->nickname, common_local_url('all', array('nickname' => $to->nickname)), common_config('site', 'name')); - common_switch_locale(); + common_init_locale(); $headers = _mail_prepare_headers('nudge', $to->nickname, $from->nickname); @@ -531,7 +531,7 @@ function mail_notify_message($message, $from=null, $to=null) return true; } - common_switch_locale($to->language); + common_init_locale($to->language); // TRANS: Subject for direct-message notification email $subject = sprintf(_('New private message from %s'), $from->nickname); @@ -555,7 +555,7 @@ function mail_notify_message($message, $from=null, $to=null) $headers = _mail_prepare_headers('message', $to->nickname, $from->nickname); - common_switch_locale(); + common_init_locale(); return mail_to_user($to, $subject, $body, $headers); } @@ -583,7 +583,7 @@ function mail_notify_fave($other, $user, $notice) $bestname = $profile->getBestName(); - common_switch_locale($other->language); + common_init_locale($other->language); // TRANS: Subject for favorite notification email $subject = sprintf(_('%s (@%s) added your notice as a favorite'), $bestname, $user->nickname); @@ -611,7 +611,7 @@ function mail_notify_fave($other, $user, $notice) $headers = _mail_prepare_headers('fave', $other->nickname, $user->nickname); - common_switch_locale(); + common_init_locale(); mail_to_user($other, $subject, $body, $headers); } diff --git a/lib/util.php b/lib/util.php index 59d5132ec6..eed61d0291 100644 --- a/lib/util.php +++ b/lib/util.php @@ -34,14 +34,6 @@ function common_user_error($msg, $code=400) $err->showPage(); } -/** - * This should only be used at setup; processes switching languages - * to send text to other users should use common_switch_locale(). - * - * @param string $language Locale language code (optional; empty uses - * current user's preference or site default) - * @return mixed success - */ function common_init_locale($language=null) { if(!$language) { @@ -58,15 +50,6 @@ function common_init_locale($language=null) return $ok; } -/** - * Initialize locale and charset settings and gettext with our message catalog, - * using the current user's language preference or the site default. - * - * This should generally only be run at framework initialization; code switching - * languages at runtime should call common_switch_language(). - * - * @access private - */ function common_init_language() { mb_internal_encoding('UTF-8'); diff --git a/plugins/Facebook/facebookutil.php b/plugins/Facebook/facebookutil.php index c7b0f02c31..9c35276b76 100644 --- a/plugins/Facebook/facebookutil.php +++ b/plugins/Facebook/facebookutil.php @@ -461,12 +461,12 @@ function remove_facebook_app($flink) function mail_facebook_app_removed($user) { + common_init_locale($user->language); + $profile = $user->getProfile(); $site_name = common_config('site', 'name'); - common_switch_locale($user->language); - $subject = sprintf( _m('Your %1$s Facebook application access has been disabled.', $site_name)); @@ -480,7 +480,7 @@ function mail_facebook_app_removed($user) "re-installing the %2\$s Facebook application.\n\nRegards,\n\n%2\$s"), $user->nickname, $site_name); - common_switch_locale(); + common_init_locale(); return mail_to_user($user, $subject, $body); } diff --git a/plugins/TwitterBridge/twitter.php b/plugins/TwitterBridge/twitter.php index 896eee2dac..21adc7a908 100644 --- a/plugins/TwitterBridge/twitter.php +++ b/plugins/TwitterBridge/twitter.php @@ -335,9 +335,9 @@ function remove_twitter_link($flink) function mail_twitter_bridge_removed($user) { - $profile = $user->getProfile(); + common_init_locale($user->language); - common_switch_locale($user->language); + $profile = $user->getProfile(); $subject = sprintf(_m('Your Twitter bridge has been disabled.')); @@ -354,7 +354,7 @@ function mail_twitter_bridge_removed($user) common_local_url('twittersettings'), common_config('site', 'name')); - common_switch_locale(); + common_init_locale(); return mail_to_user($user, $subject, $body); } From 3d4ce6f10b94d487e8eff89f689fba22327634f0 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Tue, 25 May 2010 12:31:16 -0700 Subject: [PATCH 8/8] Revert "Backing out locale switch change to see if this affects our mystery memory leak." This reverts commit f98609204fb9b5966b9e4c9e4bf8bf605656c31c. --- lib/mail.php | 20 ++++++++++---------- lib/util.php | 17 +++++++++++++++++ plugins/Facebook/facebookutil.php | 6 +++--- plugins/TwitterBridge/twitter.php | 6 +++--- 4 files changed, 33 insertions(+), 16 deletions(-) diff --git a/lib/mail.php b/lib/mail.php index f45b2d333f..ab5742e33d 100644 --- a/lib/mail.php +++ b/lib/mail.php @@ -224,9 +224,6 @@ function mail_subscribe_notify_profile($listenee, $other) if ($other->hasRight(Right::EMAILONSUBSCRIBE) && $listenee->email && $listenee->emailnotifysub) { - // use the recipient's localization - common_init_locale($listenee->language); - $profile = $listenee->getProfile(); $name = $profile->getBestName(); @@ -236,6 +233,9 @@ function mail_subscribe_notify_profile($listenee, $other) $recipients = $listenee->email; + // use the recipient's localization + common_switch_locale($listenee->language); + $headers = _mail_prepare_headers('subscribe', $listenee->nickname, $other->nickname); $headers['From'] = mail_notify_from(); $headers['To'] = $name . ' <' . $listenee->email . '>'; @@ -277,7 +277,7 @@ function mail_subscribe_notify_profile($listenee, $other) common_local_url('emailsettings')); // reset localization - common_init_locale(); + common_switch_locale(); mail_send($recipients, $headers, $body); } } @@ -479,7 +479,7 @@ function mail_confirm_sms($code, $nickname, $address) function mail_notify_nudge($from, $to) { - common_init_locale($to->language); + common_switch_locale($to->language); // TRANS: Subject for 'nudge' notification email $subject = sprintf(_('You\'ve been nudged by %s'), $from->nickname); @@ -497,7 +497,7 @@ function mail_notify_nudge($from, $to) $from->nickname, common_local_url('all', array('nickname' => $to->nickname)), common_config('site', 'name')); - common_init_locale(); + common_switch_locale(); $headers = _mail_prepare_headers('nudge', $to->nickname, $from->nickname); @@ -531,7 +531,7 @@ function mail_notify_message($message, $from=null, $to=null) return true; } - common_init_locale($to->language); + common_switch_locale($to->language); // TRANS: Subject for direct-message notification email $subject = sprintf(_('New private message from %s'), $from->nickname); @@ -555,7 +555,7 @@ function mail_notify_message($message, $from=null, $to=null) $headers = _mail_prepare_headers('message', $to->nickname, $from->nickname); - common_init_locale(); + common_switch_locale(); return mail_to_user($to, $subject, $body, $headers); } @@ -583,7 +583,7 @@ function mail_notify_fave($other, $user, $notice) $bestname = $profile->getBestName(); - common_init_locale($other->language); + common_switch_locale($other->language); // TRANS: Subject for favorite notification email $subject = sprintf(_('%s (@%s) added your notice as a favorite'), $bestname, $user->nickname); @@ -611,7 +611,7 @@ function mail_notify_fave($other, $user, $notice) $headers = _mail_prepare_headers('fave', $other->nickname, $user->nickname); - common_init_locale(); + common_switch_locale(); mail_to_user($other, $subject, $body, $headers); } diff --git a/lib/util.php b/lib/util.php index eed61d0291..59d5132ec6 100644 --- a/lib/util.php +++ b/lib/util.php @@ -34,6 +34,14 @@ function common_user_error($msg, $code=400) $err->showPage(); } +/** + * This should only be used at setup; processes switching languages + * to send text to other users should use common_switch_locale(). + * + * @param string $language Locale language code (optional; empty uses + * current user's preference or site default) + * @return mixed success + */ function common_init_locale($language=null) { if(!$language) { @@ -50,6 +58,15 @@ function common_init_locale($language=null) return $ok; } +/** + * Initialize locale and charset settings and gettext with our message catalog, + * using the current user's language preference or the site default. + * + * This should generally only be run at framework initialization; code switching + * languages at runtime should call common_switch_language(). + * + * @access private + */ function common_init_language() { mb_internal_encoding('UTF-8'); diff --git a/plugins/Facebook/facebookutil.php b/plugins/Facebook/facebookutil.php index 9c35276b76..c7b0f02c31 100644 --- a/plugins/Facebook/facebookutil.php +++ b/plugins/Facebook/facebookutil.php @@ -461,12 +461,12 @@ function remove_facebook_app($flink) function mail_facebook_app_removed($user) { - common_init_locale($user->language); - $profile = $user->getProfile(); $site_name = common_config('site', 'name'); + common_switch_locale($user->language); + $subject = sprintf( _m('Your %1$s Facebook application access has been disabled.', $site_name)); @@ -480,7 +480,7 @@ function mail_facebook_app_removed($user) "re-installing the %2\$s Facebook application.\n\nRegards,\n\n%2\$s"), $user->nickname, $site_name); - common_init_locale(); + common_switch_locale(); return mail_to_user($user, $subject, $body); } diff --git a/plugins/TwitterBridge/twitter.php b/plugins/TwitterBridge/twitter.php index 21adc7a908..896eee2dac 100644 --- a/plugins/TwitterBridge/twitter.php +++ b/plugins/TwitterBridge/twitter.php @@ -335,10 +335,10 @@ function remove_twitter_link($flink) function mail_twitter_bridge_removed($user) { - common_init_locale($user->language); - $profile = $user->getProfile(); + common_switch_locale($user->language); + $subject = sprintf(_m('Your Twitter bridge has been disabled.')); $site_name = common_config('site', 'name'); @@ -354,7 +354,7 @@ function mail_twitter_bridge_removed($user) common_local_url('twittersettings'), common_config('site', 'name')); - common_init_locale(); + common_switch_locale(); return mail_to_user($user, $subject, $body); }