OStatus: consolidate the low-level notice save code between Salmon and PuSH input paths. Validation etc remains at higher levels.

plugins/OStatus/OStatusPlugin.php

@@ -314,7 +314,7 @@ class OStatusPlugin extends Plugin
     {
         $oprofile = Ostatus_profile::staticGet('feeduri', $feedsub->uri);
         if ($oprofile) {
-            $oprofile->processFeed($feed);
+            $oprofile->processFeed($feed, 'push');
         } else {
             common_log(LOG_DEBUG, "No ostatus profile for incoming feed $feedsub->uri");
         }

plugins/OStatus/classes/Ostatus_profile.php

@@ -488,7 +488,7 @@ class Ostatus_profile extends Memcached_DataObject
      *
      * @param DOMDocument $feed
      */
-    public function processFeed($feed)
+    public function processFeed($feed, $source)
     {
         $entries = $feed->getElementsByTagNameNS(Activity::ATOM, 'entry');
         if ($entries->length == 0) {
@@ -498,7 +498,7 @@ class Ostatus_profile extends Memcached_DataObject
 
         for ($i = 0; $i < $entries->length; $i++) {
             $entry = $entries->item($i);
-            $this->processEntry($entry, $feed);
+            $this->processEntry($entry, $feed, $source);
         }
     }
 
@@ -508,15 +508,12 @@ class Ostatus_profile extends Memcached_DataObject
      * @param DOMElement $entry
      * @param DOMElement $feed for context
      */
-    protected function processEntry($entry, $feed)
+    public function processEntry($entry, $feed, $source)
     {
         $activity = new Activity($entry, $feed);
 
-        $debug = var_export($activity, true);
-        common_log(LOG_DEBUG, $debug);
-
         if ($activity->verb == ActivityVerb::POST) {
-            $this->processPost($activity);
+            $this->processPost($activity, $source);
         } else {
             common_log(LOG_INFO, "Ignoring activity with unrecognized verb $activity->verb");
         }
@@ -525,35 +522,47 @@ class Ostatus_profile extends Memcached_DataObject
     /**
      * Process an incoming post activity from this remote feed.
      * @param Activity $activity
+     * @param string $method 'push' or 'salmon'
+     * @return mixed saved Notice or false
      * @fixme break up this function, it's getting nasty long
      */
-    protected function processPost($activity)
+    public function processPost($activity, $method)
     {
         if ($this->isGroup()) {
+            // A group feed will contain posts from multiple authors.
             // @fixme validate these profiles in some way!
             $oprofile = self::ensureActorProfile($activity);
+            if ($oprofile->isGroup()) {
+                // Groups can't post notices in StatusNet.
+                common_log(LOG_WARNING, "OStatus: skipping post with group listed as author: $oprofile->uri in feed from $this->uri");
+                return false;
+            }
         } else {
+            // Individual user feeds may contain only posts from themselves.
+            // Authorship is validated against the profile URI on upper layers,
+            // through PuSH setup or Salmon signature checks.
             $actorUri = self::getActorProfileURI($activity);
             if ($actorUri == $this->uri) {
                 // @fixme check if profile info has changed and update it
             } else {
-                // @fixme drop or reject the messages once we've got the canonical profile URI recorded sanely
+                common_log(LOG_WARNING, "OStatus: skipping post with bad author: got $actorUri expected $this->uri");
-                common_log(LOG_INFO, "OStatus: Warning: non-group post with unexpected author: $actorUri expected $this->uri");
+                return false;
-                //return;
             }
             $oprofile = $this;
         }
+
+        // The id URI will be used as a unique identifier for for the notice,
+        // protecting against duplicate saves. It isn't required to be a URL;
+        // tag: URIs for instance are found in Google Buzz feeds.
         $sourceUri = $activity->object->id;
-
         $dupe = Notice::staticGet('uri', $sourceUri);
-
         if ($dupe) {
             common_log(LOG_INFO, "OStatus: ignoring duplicate post: $sourceUri");
-            return;
+            return false;
         }
 
+        // We'll also want to save a web link to the original notice, if provided.
         $sourceUrl = null;
-
         if ($activity->object->link) {
             $sourceUrl = $activity->object->link;
         } else if ($activity->link) {
@@ -563,103 +572,126 @@ class Ostatus_profile extends Memcached_DataObject
         }
 
         // Get (safe!) HTML and text versions of the content
-
+        $rendered = $this->purify($activity->object->content);
-        require_once(INSTALLDIR.'/extlib/HTMLPurifier/HTMLPurifier.auto.php');
-
-        $html = $activity->object->content;
-
-        $purifier = new HTMLPurifier();
-
-        $rendered = $purifier->purify($html);
-
         $content = html_entity_decode(strip_tags($rendered));
 
-        $params = array('is_local' => Notice::REMOTE_OMB,
+        $options = array('is_local' => Notice::REMOTE_OMB,
                         'url' => $sourceUrl,
                         'uri' => $sourceUri,
-                        'rendered' => $rendered);
+                        'rendered' => $rendered,
+                        'replies' => array(),
+                        'groups' => array());
 
-        $location = $activity->context->location;
+        // Check for optional attributes...
 
-        if ($location) {
+        if (!empty($activity->time)) {
-            $params['lat'] = $location->lat;
+            $options['created'] = common_sql_date($activity->time);
-            $params['lon'] = $location->lon;
-            if ($location->location_id) {
-                $params['location_ns'] = $location->location_ns;
-                $params['location_id'] = $location->location_id;
-            }
         }
 
-        $profile = $oprofile->localProfile();
-        $params['groups'] = array();
-        $params['replies'] = array();
         if ($activity->context) {
-            foreach ($activity->context->attention as $recipient) {
+            // Any individual or group attn: targets?
-                $roprofile = Ostatus_profile::staticGet('uri', $recipient);
+            $replies = $activity->context->attention;
-                if ($roprofile) {
+            $options['groups'] = $this->filterReplies($oprofile, $replies);
-                    if ($roprofile->isGroup()) {
+            $options['replies'] = $replies;
-                        // 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);
+            // Maintain direct reply associations
-                if ($user) {
+            // @fixme what about conversation ID?
-                    // An @-reply directed to a local user.
+            if (!empty($activity->context->replyToID)) {
-                    // @fixme sender verification, spam etc?
+                $orig = Notice::staticGet('uri',
-                    $params['replies'][] = $recipient;
+                                          $activity->context->replyToID);
-                    continue;
+                if (!empty($orig)) {
+                    $options['reply_to'] = $orig->id;
                 }
+            }
 
-                // @fixme we need a uri on user_group
+            $location = $activity->context->location;
-                // $group = User_group::staticGet('uri', $recipient);
+            if ($location) {
-                $template = common_local_url('groupbyid', array('id' => '31337'));
+                $options['lat'] = $location->lat;
-                $template = preg_quote($template, '/');
+                $options['lon'] = $location->lon;
-                $template = str_replace('31337', '(\d+)', $template);
+                if ($location->location_id) {
-                common_log(LOG_DEBUG, $template);
+                    $options['location_ns'] = $location->location_ns;
-                if (preg_match("/$template/", $recipient, $matches)) {
+                    $options['location_id'] = $location->location_id;
-                    $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");
                 }
             }
         }
 
         try {
-            $saved = Notice::saveNew($profile->id,
+            $saved = Notice::saveNew($oprofile->profile_id,
                                      $content,
                                      'ostatus',
-                                     $params);
+                                     $options);
+            if ($saved) {
+                Ostatus_source::saveNew($saved, $this, $method);
+            }
         } catch (Exception $e) {
-            common_log(LOG_ERR, "Failed saving notice entry for $sourceUri: " . $e->getMessage());
+            common_log(LOG_ERR, "OStatus save of remote message $sourceUri failed: " . $e->getMessage());
-            return;
+            throw $e;
         }
+        common_log(LOG_INFO, "OStatus saved remote message $sourceUri as notice id $saved->id");
+        return $saved;
+    }
 
-        // Record which feed this came through...
+    /**
-        try {
+     * Clean up HTML
-            Ostatus_source::saveNew($saved, $this, 'push');
+     */
-        } catch (Exception $e) {
+    protected function purify($html)
-            common_log(LOG_ERR, "Failed saving ostatus_source entry for $saved->notice_id: " . $e->getMessage());
+    {
+        // @fixme disable caching or set a sane temp dir
+        require_once(INSTALLDIR.'/extlib/HTMLPurifier/HTMLPurifier.auto.php');
+        $purifier = new HTMLPurifier();
+        return $purifier->purify($html);
+    }
+
+    /**
+     * Filters a list of recipient ID URIs to just those for local delivery.
+     * @param Ostatus_profile local profile of sender
+     * @param array in/out &$attention_uris set of URIs, will be pruned on output
+     * @return array of group IDs
+     */
+    protected function filterReplies($sender, &$attention_uris)
+    {
+        $groups = array();
+        $replies = array();
+        foreach ($attention_uris as $recipient) {
+            // Is the recipient a local user?
+            $user = User::staticGet('uri', $recipient);
+            if ($user) {
+                // @fixme sender verification, spam etc?
+                $replies[] = $recipient;
+                continue;
+            }
+
+            // Is the recipient a remote group?
+            $oprofile = Ostatus_profile::staticGet('uri', $recipient);
+            if ($oprofile) {
+                if ($oprofile->isGroup()) {
+                    // Deliver to local members of this remote group.
+                    // @fixme sender verification?
+                    $groups[] = $oprofile->group_id;
+                }
+                continue;
+            }
+
+            // Is the recipient a local group?
+            // @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);
+            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 if allowed.
+                    if ($sender->localProfile()->isMember($group)) {
+                        $groups[] = $group->id;
+                    }
+                    continue;
+                }
+            }
         }
+        $attention_uris = $replies;
+        return $groups;
     }
 
     /**

plugins/OStatus/lib/salmonaction.php

@@ -185,54 +185,6 @@ class SalmonAction extends Action
     function saveNotice()
     {
         $oprofile = $this->ensureProfile();
-
+        return $oprofile->processPost($this->act, 'salmon');
-        // Get (safe!) HTML and text versions of the content
-
-        require_once(INSTALLDIR.'/extlib/HTMLPurifier/HTMLPurifier.auto.php');
-
-        $html = $this->act->object->content;
-
-        $purifier = new HTMLPurifier();
-
-        $rendered = $purifier->purify($html);
-
-        $content = html_entity_decode(strip_tags($rendered));
-
-        $options = array('is_local' => Notice::REMOTE_OMB,
-                         'uri' => $this->act->object->id,
-                         'url' => $this->act->object->link,
-                         'rendered' => $rendered,
-                         'replies' => $this->act->context->attention);
-
-        if (!empty($this->act->context->location)) {
-            $options['lat'] = $location->lat;
-            $options['lon'] = $location->lon;
-            if ($location->location_id) {
-                $options['location_ns'] = $location->location_ns;
-                $options['location_id'] = $location->location_id;
-            }
-        }
-
-        if (!empty($this->act->context->replyToID)) {
-            $orig = Notice::staticGet('uri',
-                                      $this->act->context->replyToID);
-            if (!empty($orig)) {
-                $options['reply_to'] = $orig->id;
-            }
-        }
-
-        if (!empty($this->act->time)) {
-            $options['created'] = common_sql_date($this->act->time);
-        }
-
-        $saved = Notice::saveNew($oprofile->profile_id,
-                                 $content,
-                                 'ostatus+salmon',
-                                 $options);
-
-        // Record that this was saved through a validated Salmon source
-        // @fixme actually do the signature validation!
-        Ostatus_source::saveNew($saved, $oprofile, 'salmon');
-        return $saved;
     }
 }