From f6ebe815382a61574df5f9452ee9a0ea4ae38f0c Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Sun, 21 Feb 2010 15:21:18 -0800 Subject: Performance fix for FriendFeed sup interface: MySQL query optimizer was doing a table scan on notice; explicit subquery makes it run much more efficiently, only scanning items within the period under consideration. Standard subquery should be PostgreSQL-compatible. --- actions/sup.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'actions') diff --git a/actions/sup.php b/actions/sup.php index 5daf0a1c1..4e428dfa5 100644 --- a/actions/sup.php +++ b/actions/sup.php @@ -66,10 +66,12 @@ class SupAction extends Action $divider = common_sql_date(time() - $seconds); $notice->query('SELECT profile_id, max(id) AS max_id ' . - 'FROM notice ' . + 'FROM ( ' . + 'SELECT profile_id, id FROM notice ' . ((common_config('db','type') == 'pgsql') ? 'WHERE extract(epoch from created) > (extract(epoch from now()) - ' . $seconds . ') ' : 'WHERE created > "'.$divider.'" ' ) . + ') AS latest ' . 'GROUP BY profile_id'); $updates = array(); -- cgit v1.2.3-54-g00ecf From 8ccc9e2c386824c71aca2da7ae311d2787338483 Mon Sep 17 00:00:00 2001 From: Sarven Capadisli Date: Mon, 22 Feb 2010 17:03:28 +0100 Subject: Added before and after event hooks for subscriptions content --- EVENTS.txt | 6 ++++++ actions/subscriptions.php | 43 ++++++++++++++++++++++++------------------- 2 files changed, 30 insertions(+), 19 deletions(-) (limited to 'actions') diff --git a/EVENTS.txt b/EVENTS.txt index d3c2fb7bf..c387274c0 100644 --- a/EVENTS.txt +++ b/EVENTS.txt @@ -763,3 +763,9 @@ EndFindMentions: end finding mentions in a block of text has 'mentioned' (array of mentioned profiles), 'url' (url to link as), 'title' (title of the link), 'position' (position of the text to replace), 'text' (text to replace) + +StartShowSubscriptionsContent: before showing the subscriptions content +- $action: the current action + +EndShowSubscriptionsContent: after showing the subscriptions content +- $action: the current action diff --git a/actions/subscriptions.php b/actions/subscriptions.php index 0ef31aa9f..ba6171ef4 100644 --- a/actions/subscriptions.php +++ b/actions/subscriptions.php @@ -79,32 +79,37 @@ class SubscriptionsAction extends GalleryAction function showContent() { - parent::showContent(); + if (Event::handle('StartShowSubscriptionsContent', array($this))) { + parent::showContent(); - $offset = ($this->page-1) * PROFILES_PER_PAGE; - $limit = PROFILES_PER_PAGE + 1; + $offset = ($this->page-1) * PROFILES_PER_PAGE; + $limit = PROFILES_PER_PAGE + 1; - $cnt = 0; + $cnt = 0; - if ($this->tag) { - $subscriptions = $this->user->getTaggedSubscriptions($this->tag, $offset, $limit); - } else { - $subscriptions = $this->user->getSubscriptions($offset, $limit); - } + if ($this->tag) { + $subscriptions = $this->user->getTaggedSubscriptions($this->tag, $offset, $limit); + } else { + $subscriptions = $this->user->getSubscriptions($offset, $limit); + } - if ($subscriptions) { - $subscriptions_list = new SubscriptionsList($subscriptions, $this->user, $this); - $cnt = $subscriptions_list->show(); - if (0 == $cnt) { - $this->showEmptyListMessage(); + if ($subscriptions) { + $subscriptions_list = new SubscriptionsList($subscriptions, $this->user, $this); + $cnt = $subscriptions_list->show(); + if (0 == $cnt) { + $this->showEmptyListMessage(); + } } - } - $subscriptions->free(); + $subscriptions->free(); + + $this->pagination($this->page > 1, $cnt > PROFILES_PER_PAGE, + $this->page, 'subscriptions', + array('nickname' => $this->user->nickname)); - $this->pagination($this->page > 1, $cnt > PROFILES_PER_PAGE, - $this->page, 'subscriptions', - array('nickname' => $this->user->nickname)); + + Event::handle('EndShowSubscriptionsContent', array($this)); + } } function showScripts() -- cgit v1.2.3-54-g00ecf From 5a6967db6cbe0e864c8d542700008bba99a7b095 Mon Sep 17 00:00:00 2001 From: Evan Prodromou Date: Mon, 22 Feb 2010 11:03:56 -0500 Subject: clear the site owner when profile changes --- actions/profilesettings.php | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'actions') diff --git a/actions/profilesettings.php b/actions/profilesettings.php index 0d6777879..161e35b11 100644 --- a/actions/profilesettings.php +++ b/actions/profilesettings.php @@ -285,6 +285,10 @@ class ProfilesettingsAction extends AccountSettingsAction } else { // Re-initialize language environment if it changed common_init_language(); + // Clear the site owner, in case nickname changed + if ($user->hasRole(Profile_role::OWNER)) { + User::blow('user:site_owner'); + } } } -- cgit v1.2.3-54-g00ecf From d410df040684f443d14bd921c450ca464d52c9d4 Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Tue, 23 Feb 2010 00:44:45 +0000 Subject: OStatus group delivery initial implementation. - added rel="ostatus:attention" links for group delivery - added events for plugins to override group profile/permalink pages - pulled Notice::saveGroups up to save-time so we can override; it's relatively cheap and gives us a clean list of target groups for distrib time even with customized delivery. - fixed notice::getGroups to return group objects as expected - added some doc on new parameters to Notice::saveNew - 'groups' list of group IDs to push to in place of parsing - messages that come in via PuSH and contain local group targets are delivered to local group members - messages that come in via PuSH and contain remote group targets are delivered to local members of the remote group Todo: - handle group posts that only come through Salmon - handle conflicts in case something comes in both through Salmon and PuSH - better source verification - need a cleaner interface to look up groups by URI - need a way to handle remote groups with conflicting names --- actions/apitimelinegroup.php | 3 +- classes/Notice.php | 111 +++++++++++++++++++++++-- classes/User_group.php | 18 +++- lib/distribqueuehandler.php | 14 +--- plugins/OStatus/OStatusPlugin.php | 16 ++++ plugins/OStatus/classes/Ostatus_profile.php | 75 +++++++++++++++-- plugins/OStatus/lib/hubdistribqueuehandler.php | 2 +- 7 files changed, 207 insertions(+), 32 deletions(-) (limited to 'actions') diff --git a/actions/apitimelinegroup.php b/actions/apitimelinegroup.php index 1d0c4afdd..0bb4860ea 100644 --- a/actions/apitimelinegroup.php +++ b/actions/apitimelinegroup.php @@ -176,7 +176,8 @@ class ApiTimelineGroupAction extends ApiPrivateAuthAction $atom->addEntryFromNotices($this->notices); - $this->raw($atom->getString()); + //$this->raw($atom->getString()); + print $atom->getString(); // temp hack until PuSH feeds are redone cleanly } catch (Atom10FeedException $e) { $this->serverError( diff --git a/classes/Notice.php b/classes/Notice.php index a12839d72..754c126ed 100644 --- a/classes/Notice.php +++ b/classes/Notice.php @@ -187,7 +187,14 @@ class Notice extends Memcached_DataObject * int 'location_ns' geoname namespace to interpret location_id * int 'reply_to'; notice ID this is a reply to * int 'repeat_of'; notice ID this is a repeat of - * string 'uri' permalink to notice; defaults to local notice URL + * string 'uri' unique ID for notice; defaults to local notice URL + * string 'url' permalink to notice; defaults to local notice URL + * string 'rendered' rendered HTML version of content + * array 'replies' list of profile URIs for reply delivery in + * place of extracting @-replies from content. + * array 'groups' list of group IDs to deliver to, in place of + * extracting ! tags from content + * @fixme tag override * * @return Notice * @throws ClientException @@ -342,6 +349,12 @@ class Notice extends Memcached_DataObject $notice->saveReplies(); } + if (isset($groups)) { + $notice->saveKnownGroups($groups); + } else { + $notice->saveGroups(); + } + $notice->distribute(); return $notice; @@ -692,7 +705,22 @@ class Notice extends Memcached_DataObject return $ni; } - function addToInboxes($groups, $recipients) + /** + * 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($groups=null, $recipients=null) { $ni = $this->whoGets($groups, $recipients); @@ -742,6 +770,42 @@ class Notice extends Memcached_DataObject } /** + * Record this notice to the given group inboxes for delivery. + * Overrides the regular parsing of !group markup. + * + * @param string $group_ids + * @fixme might prefer URIs as identifiers, as for replies? + * best with generalizations on user_group to support + * remote groups better. + */ + function saveKnownGroups($group_ids) + { + if (!is_array($group_ids)) { + throw new ServerException("Bad type provided to saveKnownGroups"); + } + + $groups = array(); + foreach ($group_ids as $id) { + $group = User_group::staticGet('id', $id); + if ($group) { + common_log(LOG_ERR, "Local delivery to group id $id, $group->nickname"); + $result = $this->addToGroupInbox($group); + if (!$result) { + common_log_db_error($gi, 'INSERT', __FILE__); + } + + // @fixme should we save the tags here or not? + $groups[] = clone($group); + } else { + common_log(LOG_ERR, "Local delivery to group id $id skipped, doesn't exist"); + } + } + + return $groups; + } + + /** + * Parse !group delivery and record targets into group_inbox. * @return array of Group objects */ function saveGroups() @@ -824,6 +888,19 @@ class Notice extends Memcached_DataObject return true; } + /** + * Save reply records indicating that this notice needs to be + * delivered to the local users with the given URIs. + * + * Since this is expected to be used when saving foreign-sourced + * messages, we won't deliver to any remote targets as that's the + * source service's responsibility. + * + * @fixme Unlike saveReplies() there's no mail notification here. + * Move that to distrib queue handler? + * + * @param array of unique identifier URIs for recipients + */ function saveKnownReplies($uris) { foreach ($uris as $uri) { @@ -845,6 +922,13 @@ class Notice extends Memcached_DataObject } /** + * Pull @-replies from this message's content in StatusNet markup format + * and save reply records indicating that this message needs to be + * delivered to those users. + * + * Side effect: local recipients get e-mail notifications here. + * @fixme move mail notifications to distrib? + * * @return array of integer profile IDs */ @@ -934,9 +1018,10 @@ class Notice extends Memcached_DataObject } /** - * Same calculation as saveGroups but without the saving - * @fixme merge the functions - * @return array of Group_inbox objects + * Pull list of groups this notice needs to be delivered to, + * as previously recorded by saveGroups() or saveKnownGroups(). + * + * @return array of Group objects */ function getGroups() { @@ -959,7 +1044,10 @@ class Notice extends Memcached_DataObject if ($gi->find()) { while ($gi->fetch()) { - $groups[] = clone($gi); + $group = User_group::staticGet('id', $gi->group_id); + if ($group) { + $groups[] = $group; + } } } @@ -1063,6 +1151,17 @@ class Notice extends Memcached_DataObject } } + $groups = $this->getGroups(); + + foreach ($groups as $group) { + $xs->element( + 'link', array( + 'rel' => 'ostatus:attention', + 'href' => $group->permalink() + ) + ); + } + if (!empty($this->repeat_of)) { $repeat = Notice::staticGet('id', $this->repeat_of); if (!empty($repeat)) { diff --git a/classes/User_group.php b/classes/User_group.php index 379e6b721..1382aa407 100644 --- a/classes/User_group.php +++ b/classes/User_group.php @@ -39,14 +39,24 @@ class User_group extends Memcached_DataObject function homeUrl() { - return common_local_url('showgroup', - array('nickname' => $this->nickname)); + $url = null; + if (Event::handle('StartUserGroupHomeUrl', array($this, &$url))) { + $url = common_local_url('showgroup', + array('nickname' => $this->nickname)); + } + Event::handle('EndUserGroupHomeUrl', array($this, &$url)); + return $url; } function permalink() { - return common_local_url('groupbyid', - array('id' => $this->id)); + $url = null; + if (Event::handle('StartUserGroupPermalink', array($this, &$url))) { + $url = common_local_url('groupbyid', + array('id' => $this->id)); + } + Event::handle('EndUserGroupPermalink', array($this, &$url)); + return $url; } function getNotices($offset, $limit, $since_id=null, $max_id=null) diff --git a/lib/distribqueuehandler.php b/lib/distribqueuehandler.php index c31b675c1..dc183fb36 100644 --- a/lib/distribqueuehandler.php +++ b/lib/distribqueuehandler.php @@ -69,19 +69,7 @@ class DistribQueueHandler } try { - $groups = $notice->saveGroups(); - } catch (Exception $e) { - $this->logit($notice, $e); - } - - try { - $recipients = $notice->getReplies(); - } catch (Exception $e) { - $this->logit($notice, $e); - } - - try { - $notice->addToInboxes($groups, $recipients); + $notice->addToInboxes(); } catch (Exception $e) { $this->logit($notice, $e); } diff --git a/plugins/OStatus/OStatusPlugin.php b/plugins/OStatus/OStatusPlugin.php index 061ed4bd1..472008419 100644 --- a/plugins/OStatus/OStatusPlugin.php +++ b/plugins/OStatus/OStatusPlugin.php @@ -591,6 +591,22 @@ class OStatusPlugin extends Plugin return true; } + function onStartUserGroupHomeUrl($group, &$url) + { + return $this->onStartUserGroupPermalink($group, &$url); + } + + function onStartUserGroupPermalink($group, &$url) + { + $oprofile = Ostatus_profile::staticGet('group_id', $group->id); + if ($oprofile) { + // @fixme this should probably be in the user_group table + // @fixme this uri not guaranteed to be a profile page + $url = $oprofile->uri; + return false; + } + } + function onStartShowSubscriptionsContent($action) { $user = common_current_user(); diff --git a/plugins/OStatus/classes/Ostatus_profile.php b/plugins/OStatus/classes/Ostatus_profile.php index c0e39add8..e8cc13c6c 100644 --- a/plugins/OStatus/classes/Ostatus_profile.php +++ b/plugins/OStatus/classes/Ostatus_profile.php @@ -501,6 +501,7 @@ class Ostatus_profile extends Memcached_DataObject /** * Process an incoming post activity from this remote feed. * @param Activity $activity + * @fixme break up this function, it's getting nasty long */ protected function processPost($activity) { @@ -518,7 +519,6 @@ class Ostatus_profile extends Memcached_DataObject } $oprofile = $this; } - $sourceUri = $activity->object->id; $dupe = Notice::staticGet('uri', $sourceUri); @@ -555,15 +555,76 @@ class Ostatus_profile extends Memcached_DataObject } } - // @fixme ensure that groups get handled correctly + $profile = $oprofile->localProfile(); + $params['groups'] = array(); + $params['replies'] = array(); + if ($activity->context) { + foreach ($activity->context->attention as $recipient) { + $roprofile = Ostatus_profile::staticGet('uri', $recipient); + if ($roprofile) { + if ($roprofile->isGroup()) { + // Deliver to local recipients of this remote group. + // @fixme sender verification? + $params['groups'][] = $roprofile->group_id; + continue; + } else { + // Delivery to remote users is the source service's job. + continue; + } + } + + $user = User::staticGet('uri', $recipient); + if ($user) { + // An @-reply directed to a local user. + // @fixme sender verification, spam etc? + $params['replies'][] = $recipient; + continue; + } + + // @fixme we need a uri on user_group + // $group = User_group::staticGet('uri', $recipient); + $template = common_local_url('groupbyid', array('id' => '31337')); + $template = preg_quote($template, '/'); + $template = str_replace('31337', '(\d+)', $template); + common_log(LOG_DEBUG, $template); + if (preg_match("/$template/", $recipient, $matches)) { + $id = $matches[1]; + $group = User_group::staticGet('id', $id); + if ($group) { + // Deliver to all members of this local group. + // @fixme sender verification? + if ($profile->isMember($group)) { + common_log(LOG_DEBUG, "delivering to group $id $group->nickname"); + $params['groups'][] = $group->id; + } else { + common_log(LOG_DEBUG, "not delivering to group $id $group->nickname because sender $profile->nickname is not a member"); + } + continue; + } else { + common_log(LOG_DEBUG, "not delivering to missing group $id"); + } + } else { + common_log(LOG_DEBUG, "not delivering to groups for $recipient"); + } + } + } - $saved = Notice::saveNew($oprofile->localProfile()->id, - $content, - 'ostatus', - $params); + try { + $saved = Notice::saveNew($profile->id, + $content, + 'ostatus', + $params); + } catch (Exception $e) { + common_log(LOG_ERR, "Failed saving notice entry for $sourceUri: " . $e->getMessage()); + return; + } // Record which feed this came through... - Ostatus_source::saveNew($saved, $this, 'push'); + try { + Ostatus_source::saveNew($saved, $this, 'push'); + } catch (Exception $e) { + common_log(LOG_ERR, "Failed saving ostatus_source entry for $saved->notice_id: " . $e->getMessage()); + } } /** diff --git a/plugins/OStatus/lib/hubdistribqueuehandler.php b/plugins/OStatus/lib/hubdistribqueuehandler.php index 30a427e3f..c2bd630f9 100644 --- a/plugins/OStatus/lib/hubdistribqueuehandler.php +++ b/plugins/OStatus/lib/hubdistribqueuehandler.php @@ -36,7 +36,7 @@ class HubDistribQueueHandler extends QueueHandler $this->pushUser($notice); foreach ($notice->getGroups() as $group) { - $this->pushGroup($notice, $group->group_id); + $this->pushGroup($notice, $group->id); } return true; } -- cgit v1.2.3-54-g00ecf From 1bffe424131dfa2c7a85fd9cf782e8c806326bbe Mon Sep 17 00:00:00 2001 From: Evan Prodromou Date: Tue, 23 Feb 2010 08:38:23 -0500 Subject: Drop user-only requirement for subscribe action I removed the check for local users in the subscribe button. I replaced it with a more specific check for OMB 0.1 remote profiles, which you can't use with this action. I also took the opportunity to split the handle() method into prepare() and handle(), and added PHPCS clean documentation. --- actions/subscribe.php | 132 +++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 104 insertions(+), 28 deletions(-) (limited to 'actions') diff --git a/actions/subscribe.php b/actions/subscribe.php index a90d7facd..3745311b6 100644 --- a/actions/subscribe.php +++ b/actions/subscribe.php @@ -1,7 +1,9 @@ . + * + * PHP version 5 + * + * @category Action + * @package StatusNet + * @author Evan Prodromou + * @copyright 2008-2010 StatusNet, Inc. + * @license http://www.fsf.org/licensing/licenses/agpl-3.0.html AGPLv3 + * @link http://status.net/ */ -if (!defined('STATUSNET') && !defined('LACONICA')) { exit(1); } +if (!defined('STATUSNET')) { + exit(1); +} + +/** + * Subscription action + * + * Subscribing to a profile. Does not work for OMB 0.1 remote subscriptions, + * but may work for other remote subscription protocols, like OStatus. + * + * Takes parameters: + * + * - subscribeto: a profile ID + * - token: session token to prevent CSRF attacks + * - ajax: boolean; whether to return Ajax or full-browser results + * + * Only works if the current user is logged in. + * + * @category Action + * @package StatusNet + * @author Evan Prodromou + * @copyright 2008-2010 StatusNet, Inc. + * @license http://www.fsf.org/licensing/licenses/agpl-3.0.html AGPLv3 + * @link http://status.net/ + */ class SubscribeAction extends Action { + var $user; + var $other; - function handle($args) - { - parent::handle($args); + /** + * Check pre-requisites and instantiate attributes + * + * @param Array $args array of arguments (URL, GET, POST) + * + * @return boolean success flag + */ - if (!common_logged_in()) { - $this->clientError(_('Not logged in.')); - return; - } + function prepare($args) + { + parent::prepare($args); - $user = common_current_user(); + // Only allow POST requests if ($_SERVER['REQUEST_METHOD'] != 'POST') { - common_redirect(common_local_url('subscriptions', array('nickname' => $user->nickname))); - return; + $this->clientError(_('This action only accepts POST requests.')); + return false; } - # CSRF protection + // CSRF protection $token = $this->trimmed('token'); if (!$token || $token != common_session_token()) { - $this->clientError(_('There was a problem with your session token. Try again, please.')); - return; + $this->clientError(_('There was a problem with your session token.'. + ' Try again, please.')); + return false; + } + + // Only for logged-in users + + $this->user = common_current_user(); + + if (empty($this->user)) { + $this->clientError(_('Not logged in.')); + return false; } + // Profile to subscribe to + $other_id = $this->arg('subscribeto'); - $other = User::staticGet('id', $other_id); + $this->other = Profile::staticGet('id', $other_id); - if (!$other) { - $this->clientError(_('Not a local user.')); - return; + if (empty($this->other)) { + $this->clientError(_('No such profile.')); + return false; } - $result = subs_subscribe_to($user, $other); + // OMB 0.1 doesn't have a mechanism for local-server- + // originated subscription. + + $omb01 = Remote_profile::staticGet('id', $other_id); - if (is_string($result)) { - $this->clientError($result); - return; + if (!empty($omb01)) { + $this->clientError(_('You cannot subscribe to an OMB 0.1'. + ' remote profile with this action.')); + return false; } + return true; + } + + /** + * Handle request + * + * Does the subscription and returns results. + * + * @param Array $args unused. + * + * @return void + */ + + function handle($args) + { + // Throws exception on error + + Subscription::start($this->user->getProfile(), + $this->other); + if ($this->boolean('ajax')) { $this->startHTML('text/xml;charset=utf-8'); $this->elementStart('head'); $this->element('title', null, _('Subscribed')); $this->elementEnd('head'); $this->elementStart('body'); - $unsubscribe = new UnsubscribeForm($this, $other->getProfile()); + $unsubscribe = new UnsubscribeForm($this, $this->other->getProfile()); $unsubscribe->show(); $this->elementEnd('body'); $this->elementEnd('html'); } else { - common_redirect(common_local_url('subscriptions', array('nickname' => - $user->nickname)), - 303); + $url = common_local_url('subscriptions', + array('nickname' => $this->user->nickname)); + common_redirect($url, 303); } } } -- cgit v1.2.3-54-g00ecf