From 9b6633698c5abda07df3102b62c37cfb0847e1ea Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Tue, 29 Oct 2013 11:27:26 +0100 Subject: [PATCH] Group discovery from text functions polished Also removed the entirely unused saveGroups function. Now avoiding multiGet and using listFind in Profile->getGroups() so we don't have to deal with ArrayWrapper. --- actions/usergroups.php | 7 +- classes/Notice.php | 73 +-------------- classes/Profile.php | 18 +++- classes/User_group.php | 44 +++++++-- lib/apiaction.php | 2 +- lib/command.php | 2 +- lib/groupsnav.php | 4 +- lib/profileaction.php | 9 +- lib/toselector.php | 2 +- lib/util.php | 146 ++++++++++++++---------------- plugins/OStatus/OStatusPlugin.php | 2 +- scripts/createsim.php | 2 +- 12 files changed, 137 insertions(+), 174 deletions(-) diff --git a/actions/usergroups.php b/actions/usergroups.php index 9ffc8e0ab3..b705afe068 100644 --- a/actions/usergroups.php +++ b/actions/usergroups.php @@ -137,12 +137,11 @@ class UsergroupsAction extends ProfileAction $groups = $this->user->getGroups($offset, $limit); - if ($groups) { + if ($groups instanceof User_group) { $gl = new GroupList($groups, $this->user, $this); $cnt = $gl->show(); - if (0 == $cnt) { - $this->showEmptyListMessage(); - } + } else { + $this->showEmptyListMessage(); } $this->pagination($this->page > 1, $cnt > GROUPS_PER_PAGE, diff --git a/classes/Notice.php b/classes/Notice.php index 4e5a83a859..c5cde618ca 100644 --- a/classes/Notice.php +++ b/classes/Notice.php @@ -410,9 +410,8 @@ class Notice extends Managed_DataObject $notice->url = $url; // Get the groups here so we can figure out replies and such - if (!isset($groups)) { - $groups = self::groupsFromText($notice->content, $profile); + $groups = User_group::idsFromText($notice->content, $profile); } $reply = null; @@ -1154,7 +1153,7 @@ class Notice extends Managed_DataObject $groups = array(); foreach (array_unique($group_ids) as $id) { $group = User_group::getKV('id', $id); - if ($group) { + if ($group instanceof User_group) { common_log(LOG_ERR, "Local delivery to group id $id, $group->nickname"); $result = $this->addToGroupInbox($group); if (!$result) { @@ -1181,48 +1180,7 @@ class Notice extends Managed_DataObject return $groups; } - /** - * Parse !group delivery and record targets into group_inbox. - * @return array of Group objects - */ - function saveGroups() - { - // Don't save groups for repeats - - if (!empty($this->repeat_of)) { - return array(); - } - - $profile = $this->getProfile(); - - $groups = self::groupsFromText($this->content, $profile); - - /* Add them to the database */ - - foreach ($groups as $group) { - /* XXX: remote groups. */ - - if (empty($group)) { - continue; - } - - - if ($profile->isMember($group)) { - - $result = $this->addToGroupInbox($group); - - if (!$result) { - common_log_db_error($gi, 'INSERT', __FILE__); - } - - $groups[] = clone($group); - } - } - - return $groups; - } - - function addToGroupInbox($group) + function addToGroupInbox(User_group $group) { $gi = Group_inbox::pkeyGet(array('group_id' => $group->id, 'notice_id' => $this->id)); @@ -1445,7 +1403,7 @@ class Notice extends Managed_DataObject /** * Pull list of groups this notice needs to be delivered to, - * as previously recorded by saveGroups() or saveKnownGroups(). + * as previously recorded by saveKnownGroups(). * * @return array of Group objects */ @@ -2500,29 +2458,6 @@ class Notice extends Managed_DataObject return false; } - static function groupsFromText($text, $profile) - { - $groups = array(); - - /* extract all !group */ - $count = preg_match_all('/(?:^|\s)!(' . Nickname::DISPLAY_FMT . ')/', - strtolower($text), - $match); - - if (!$count) { - return $groups; - } - - foreach (array_unique($match[1]) as $nickname) { - $group = User_group::getForNickname($nickname, $profile); - if ($group instanceof User_group && $profile->isMember($group)) { - $groups[] = $group->id; - } - } - - return $groups; - } - public function getParent() { $parent = Notice::getKV('id', $this->reply_to); diff --git a/classes/Profile.php b/classes/Profile.php index 50cbeb3ac4..4e0632ba0a 100644 --- a/classes/Profile.php +++ b/classes/Profile.php @@ -221,9 +221,8 @@ class Profile extends Managed_DataObject function isMember($group) { $groups = $this->getGroups(0, null); - $gs = $groups->fetchAll(); - foreach ($gs as $g) { - if ($group->id == $g->id) { + while ($groups instanceof User_group && $groups->fetch()) { + if ($groups->id == $group->id) { return true; } } @@ -272,7 +271,18 @@ class Profile extends Managed_DataObject $ids = array_slice($ids, $offset, $limit); } - return User_group::multiGet('id', $ids); + try { + return User_group::listFind('id', $ids); + } catch (NoResultException $e) { + return null; // throw exception when we handle it everywhere + } + } + + function getGroupCount() { + $groups = $this->getGroups(0, null); + return $groups instanceof User_group + ? $groups->N + : 0; } function isTagged($peopletag) diff --git a/classes/User_group.php b/classes/User_group.php index 6f3c7038a7..22b8d0bc04 100644 --- a/classes/User_group.php +++ b/classes/User_group.php @@ -418,14 +418,14 @@ class User_group extends Managed_DataObject return true; } - static function getForNickname($nickname, $profile=null) + static function getForNickname($nickname, Profile $profile=null) { $nickname = Nickname::normalize($nickname); // Are there any matching remote groups this profile's in? - if ($profile) { + if ($profile instanceof Profile) { $group = $profile->getGroups(0, null); - while ($group->fetch()) { + while ($group instanceof User_group && $group->fetch()) { if ($group->nickname == $nickname) { // @fixme is this the best way? return clone($group); @@ -434,13 +434,12 @@ class User_group extends Managed_DataObject } // If not, check local groups. - $group = Local_group::getKV('nickname', $nickname); - if (!empty($group)) { + if ($group instanceof Local_group) { return User_group::getKV('id', $group->group_id); } $alias = Group_alias::getKV('alias', $nickname); - if (!empty($alias)) { + if ($alias instanceof Group_alias) { return User_group::getKV('id', $alias->group_id); } return null; @@ -822,4 +821,37 @@ class User_group extends Managed_DataObject return ($this->join_policy == self::JOIN_POLICY_MODERATE && $this->force_scope == 1); } + + static function groupsFromText($text, Profile $profile) + { + $groups = array(); + + /* extract all !group */ + $count = preg_match_all('/(?:^|\s)!(' . Nickname::DISPLAY_FMT . ')/', + strtolower($text), + $match); + + if (!$count) { + return $groups; + } + + foreach (array_unique($match[1]) as $nickname) { + $group = self::getForNickname($nickname, $profile); + if ($group instanceof User_group && $profile->isMember($group)) { + $groups[] = clone($group); + } + } + + return $groups; + } + + static function idsFromText($text, Profile $profile) + { + $ids = array(); + $groups = self::groupsFromText($text, $profile); + foreach ($groups as $group) { + $ids[$group->id] = true; + } + return array_keys($ids); + } } diff --git a/lib/apiaction.php b/lib/apiaction.php index 481dcf1b95..d151f4c51f 100644 --- a/lib/apiaction.php +++ b/lib/apiaction.php @@ -225,7 +225,7 @@ class ApiAction extends Action } $twitter_user['profile_image_url_original'] = $origurl; - $twitter_user['groups_count'] = $profile->getGroups(0, null)->N; + $twitter_user['groups_count'] = $profile->getGroupCount(); foreach (array('linkcolor', 'backgroundcolor') as $key) { $twitter_user[$key] = Profile_prefs::getConfigData($profile, 'theme', $key); } diff --git a/lib/command.php b/lib/command.php index 04c2e57ec8..d15fe43ce1 100644 --- a/lib/command.php +++ b/lib/command.php @@ -980,7 +980,7 @@ class GroupsCommand extends Command { $group = $this->user->getGroups(); $groups=array(); - while ($group->fetch()) { + while ($group instanceof User_group && $group->fetch()) { $groups[]=$group->nickname; } if(count($groups)==0){ diff --git a/lib/groupsnav.php b/lib/groupsnav.php index 8439ab6399..8e5d1a39fb 100644 --- a/lib/groupsnav.php +++ b/lib/groupsnav.php @@ -58,7 +58,7 @@ class GroupsNav extends MoreMenu function haveGroups() { - return (!empty($this->groups) && ($this->groups->N > 0)); + return ($this->groups instanceof User_group && $this->groups->N > 0); } function tag() @@ -70,7 +70,7 @@ class GroupsNav extends MoreMenu { $items = array(); - while ($this->groups->fetch()) { + while ($this->groups instanceof User_group && $this->groups->fetch()) { $items[] = array('placeholder', array('nickname' => $this->groups->nickname, 'mainpage' => $this->groups->homeUrl()), diff --git a/lib/profileaction.php b/lib/profileaction.php index ea33af326a..b2990bd249 100644 --- a/lib/profileaction.php +++ b/lib/profileaction.php @@ -270,13 +270,12 @@ class ProfileAction extends Action $this->text($this->profile->getGroups(0, null)->N); $this->elementEnd('h2'); - if ($groups) { + if ($groups instanceof User_group) { $gml = new GroupMiniList($groups, $this->profile, $this); $cnt = $gml->show(); - if ($cnt == 0) { - // TRANS: Text for user user group membership statistics if user is not a member of any group. - $this->element('p', null, _('(None)')); - } + } else { + // TRANS: Text for user user group membership statistics if user is not a member of any group. + $this->element('p', null, _('(None)')); } Event::handle('EndShowGroupsMiniList', array($this)); diff --git a/lib/toselector.php b/lib/toselector.php index b01948ed62..6d424dc259 100644 --- a/lib/toselector.php +++ b/lib/toselector.php @@ -94,7 +94,7 @@ class ToSelector extends Widget $groups = $this->user->getGroups(); - while ($groups->fetch()) { + while ($groups instanceof User_group && $groups->fetch()) { $value = 'group:'.$groups->id; if (($this->to instanceof User_group) && $this->to->id == $groups->id) { $default = $value; diff --git a/lib/util.php b/lib/util.php index d124045724..bf3250e0c1 100644 --- a/lib/util.php +++ b/lib/util.php @@ -320,7 +320,7 @@ function common_set_user($user) } else if (is_string($user)) { $nickname = $user; $user = User::getKV('nickname', $nickname); - } else if (!($user instanceof User)) { + } else if (!$user instanceof User) { return false; } @@ -433,7 +433,7 @@ function common_remembered_user() $user = User::getKV('id', $rm->user_id); - if (!$user) { + if (!$user instanceof User) { common_log(LOG_WARNING, 'No such user for rememberme: ' . $rm->user_id); common_forgetme(); return null; @@ -488,8 +488,8 @@ function common_current_user() common_ensure_session(); $id = isset($_SESSION['userid']) ? $_SESSION['userid'] : false; if ($id) { - $user = User::getKV($id); - if ($user) { + $user = User::getKV('id', $id); + if ($user instanceof User) { $_cur = $user; return $_cur; } @@ -584,13 +584,10 @@ function common_canonical_email($email) * @param Notice $notice in whose context we're working * @return string partially rendered HTML */ -function common_render_content($text, $notice) +function common_render_content($text, Notice $notice) { $r = common_render_text($text); - $id = $notice->profile_id; $r = common_linkify_mentions($r, $notice); - $r = preg_replace_callback('/(^|[\s\.\,\:\;]+)!(' . Nickname::DISPLAY_FMT . ')/', - function ($m) { return "{$m[1]}!".common_group_link($id, $m[2]); }, $r); return $r; } @@ -677,35 +674,39 @@ function common_linkify_mention($mention) */ function common_find_mentions($text, $notice) { - $mentions = array(); - - $sender = Profile::getKV('id', $notice->profile_id); - - if (empty($sender)) { - return $mentions; + try { + $sender = Profile::getKV('id', $notice->profile_id); + } catch (NoProfileException $e) { + return array(); } + $mentions = array(); + if (Event::handle('StartFindMentions', array($sender, $text, &$mentions))) { // Get the context of the original notice, if any - $originalAuthor = null; - $originalNotice = null; - $originalMentions = array(); + $origAuthor = null; + $origNotice = null; + $origMentions = array(); // Is it a reply? - if (!empty($notice) && !empty($notice->reply_to)) { - $originalNotice = Notice::getKV('id', $notice->reply_to); - if (!empty($originalNotice)) { - $originalAuthor = Profile::getKV('id', $originalNotice->profile_id); + if ($notice instanceof Notice) { + try { + $origNotice = $notice->getParent(); + $origAuthor = $origNotice->getProfile(); - $ids = $originalNotice->getReplies(); + $ids = $origNotice->getReplies(); foreach ($ids as $id) { $repliedTo = Profile::getKV('id', $id); - if (!empty($repliedTo)) { - $originalMentions[$repliedTo->nickname] = $repliedTo; + if ($repliedTo instanceof Profile) { + $origMentions[$repliedTo->nickname] = $repliedTo; } } + } catch (NoProfileException $e) { + common_log(LOG_WARNING, sprintf('Notice %d author profile id %d does not exist', $origNotice->id, $origNotice->profile_id)); + } catch (ServerException $e) { + common_log(LOG_WARNING, __METHOD__ . ' got exception: ' . $e->getMessage()); } } @@ -723,19 +724,19 @@ function common_find_mentions($text, $notice) // Start with conversation context, then go to // sender context. - if (!empty($originalAuthor) && $originalAuthor->nickname == $nickname) { - $mentioned = $originalAuthor; - } else if (!empty($originalMentions) && - array_key_exists($nickname, $originalMentions)) { - $mentioned = $originalMentions[$nickname]; + if ($origAuthor instanceof Profile && $origAuthor->nickname == $nickname) { + $mentioned = $origAuthor; + } else if (!empty($origMentions) && + array_key_exists($nickname, $origMentions)) { + $mentioned = $origMentions[$nickname]; } else { $mentioned = common_relative_profile($sender, $nickname); } - if (!empty($mentioned)) { + if ($mentioned instanceof Profile) { $user = User::getKV('id', $mentioned->id); - if ($user) { + if ($user instanceof User) { $url = common_local_url('userbyid', array('id' => $user->id)); } else { $url = $mentioned->profileurl; @@ -757,26 +758,42 @@ function common_find_mentions($text, $notice) // @#tag => mention of all subscriptions tagged 'tag' preg_match_all('/(?:^|[\s\.\,\:\;]+)@#([\pL\pN_\-\.]{1,64})/', - $text, - $hmatches, - PREG_OFFSET_CAPTURE); - + $text, $hmatches, PREG_OFFSET_CAPTURE); foreach ($hmatches[1] as $hmatch) { - $tag = common_canonical_tag($hmatch[0]); $plist = Profile_list::getByTaggerAndTag($sender->id, $tag); - if (!empty($plist) && !$plist->private) { - $tagged = $sender->getTaggedSubscribers($tag); - - $url = common_local_url('showprofiletag', - array('tagger' => $sender->nickname, - 'tag' => $tag)); - - $mentions[] = array('mentioned' => $tagged, - 'text' => $hmatch[0], - 'position' => $hmatch[1], - 'url' => $url); + if (!$plist instanceof Profile_list || $plist->private) { + continue; } + $tagged = $sender->getTaggedSubscribers($tag); + + $url = common_local_url('showprofiletag', + array('tagger' => $sender->nickname, + 'tag' => $tag)); + + $mentions[] = array('mentioned' => $tagged, + 'text' => $hmatch[0], + 'position' => $hmatch[1], + 'url' => $url); + } + + preg_match_all('/(?:^|[\s\.\,\:\;]+)!(' . Nickname::DISPLAY_FMT . ')/', + $text, $hmatches, PREG_OFFSET_CAPTURE); + foreach ($hmatches[1] as $hmatch) { + $nickname = Nickname::normalize($hmatch[0]); + $group = User_group::getForNickname($nickname, $sender); + + if (!$group instanceof User_group || !$sender->isMember($group)) { + continue; + } + + $profile = $group->getProfile(); + + $mentions[] = array('mentioned' => $profile, + 'text' => $hmatch[0], + 'position' => $hmatch[1], + 'url' => $group->permalink, + 'title' => $group->getFancyName()); } Event::handle('EndFindMentions', array($sender, $text, &$mentions)); @@ -1146,35 +1163,6 @@ function common_valid_profile_tag($str) return preg_match('/^[A-Za-z0-9_\-\.]{1,64}$/', $str); } -/** - * - * @param $sender_id - * @param $nickname - * @return - * @access private - */ -function common_group_link($sender_id, $nickname) -{ - $sender = Profile::getKV($sender_id); - $group = User_group::getForNickname($nickname, $sender); - if ($sender && $group && $sender->isMember($group)) { - $attrs = array('href' => $group->permalink(), - 'class' => 'url'); - if (!empty($group->fullname)) { - $attrs['title'] = $group->getFancyName(); - } - $xs = new XMLStringer(); - $xs->elementStart('span', 'vcard'); - $xs->elementStart('a', $attrs); - $xs->element('span', 'fn nickname group', $nickname); - $xs->elementEnd('a'); - $xs->elementEnd('span'); - return $xs->getString(); - } else { - return $nickname; - } -} - /** * Resolve an ambiguous profile nickname reference, checking in following order: * - profiles that $sender subscribes to @@ -1222,7 +1210,7 @@ function common_relative_profile($sender, $nickname, $dt=null) return $recipient; } // If this is a local user, try to find a local user with that nickname. - $sender = User::getKV($sender->id); + $sender = User::getKV('id', $sender->id); if ($sender instanceof User) { $recipient_user = User::getKV('nickname', $nickname); if ($recipient_user instanceof User) { @@ -2020,8 +2008,8 @@ function common_profile_uri($profile) if (!empty($profile)) { if (Event::handle('StartCommonProfileURI', array($profile, &$uri))) { - $user = User::getKV($profile->id); - if (!empty($user)) { + $user = User::getKV('id', $profile->id); + if ($user instanceof User) { $uri = $user->uri; } Event::handle('EndCommonProfileURI', array($profile, &$uri)); diff --git a/plugins/OStatus/OStatusPlugin.php b/plugins/OStatus/OStatusPlugin.php index 84f2a5e9ca..29dde3119e 100644 --- a/plugins/OStatus/OStatusPlugin.php +++ b/plugins/OStatus/OStatusPlugin.php @@ -337,7 +337,7 @@ class OStatusPlugin extends Plugin * @param array &$mention in/out param: set of found mentions * @return boolean hook return value */ - function onEndFindMentions($sender, $text, &$mentions) + function onEndFindMentions(Profile $sender, $text, &$mentions) { $matches = array(); diff --git a/scripts/createsim.php b/scripts/createsim.php index e0ad74c49e..7fc2ed1d90 100644 --- a/scripts/createsim.php +++ b/scripts/createsim.php @@ -163,7 +163,7 @@ function newNotice($i, $tagmax) if ($in_group == 0) { $groups = $user->getGroups(); - if ($groups->N > 0) { + if ($groups instanceof User_group) { $gval = rand(0, $groups->N - 1); $groups->fetch(); // go to 0th for ($i = 0; $i < $gval; $i++) {