Ostatus_profile throws NoProfileException from localProfile()

Some implementations of the exception handling is included here,
the actions come in a later commit.

diff --git a/plugins/OStatus/OStatusPlugin.php b/plugins/OStatus/OStatusPlugin.php
index d180a27d225aac104df095a0063238c20380e702..37099c24524bf8812fa0b98730324da282af6e97 100644 (file)
--- a/plugins/OStatus/OStatusPlugin.php
+++ b/plugins/OStatus/OStatusPlugin.php
@@ -423,7 +423,12 @@ class OStatusPlugin extends Plugin
     {
         $oprofile = $this->pullRemoteProfile($arg);
         if ($oprofile instanceof Ostatus_profile && !$oprofile->isGroup()) {
-            $profile = $oprofile->localProfile();
+            try {
+                $profile = $oprofile->localProfile();
+            } catch (NoProfileException $e) {
+                // No locally stored profile found for remote profile
+                return true;
+            }
             return false;
         } else {
             return true;

diff --git a/plugins/OStatus/classes/FeedSub.php b/plugins/OStatus/classes/FeedSub.php
index 9cf9a809111cab0f5ec34e53433e69e6cdfb92f6..83ceb35254f47f878717c5d7b3d1b504177f8e5e 100644 (file)
--- a/plugins/OStatus/classes/FeedSub.php
+++ b/plugins/OStatus/classes/FeedSub.php
@@ -264,6 +264,7 @@ class FeedSub extends Managed_DataObject
      * make sure it's inactive, unsubscribing if necessary.
      *
      * @return boolean true if the subscription is now inactive, false if still active.
+     * @throws NoProfileException in FeedSubSubscriberCount for missing Profile entries
      * @throws Exception if something goes wrong in unsubscribe() method
      */
     public function garbageCollect()
@@ -271,22 +272,23 @@ class FeedSub extends Managed_DataObject
         if ($this->sub_state == '' || $this->sub_state == 'inactive') {
             // No active PuSH subscription, we can just leave it be.
             return true;
-        } else {
-            // PuSH subscription is either active or in an indeterminate state.
-            // Check if we're out of subscribers, and if so send an unsubscribe.
-            $count = 0;
-            Event::handle('FeedSubSubscriberCount', array($this, &$count));
-
-            if ($count > 0) {
-                common_log(LOG_INFO, __METHOD__ . ': ok, ' . $count . ' user(s) left for ' . $this->getUri());
-                return false;
-            } else {
-                common_log(LOG_INFO, __METHOD__ . ': unsubscribing, no users left for ' . $this->getUri());
-                // Unsubscribe throws various Exceptions on failure
-                $this->unsubscribe();
-                return true;
-            }
         }
+
+        // PuSH subscription is either active or in an indeterminate state.
+        // Check if we're out of subscribers, and if so send an unsubscribe.
+        $count = 0;
+        Event::handle('FeedSubSubscriberCount', array($this, &$count));
+
+        if ($count > 0) {
+            common_log(LOG_INFO, __METHOD__ . ': ok, ' . $count . ' user(s) left for ' . $this->getUri());
+            return false;
+        }
+
+        common_log(LOG_INFO, __METHOD__ . ': unsubscribing, no users left for ' . $this->getUri());
+        // Unsubscribe throws various Exceptions on failure
+        $this->unsubscribe();
+
+        return true;
     }
 
     static public function renewalCheck()
@@ -505,4 +507,18 @@ class FeedSub extends Managed_DataObject
         }
         return false;
     }
+
+    public function delete($useWhere=false)
+    {
+        try {
+            $oprofile = Ostatus_profile::getKV('feeduri', $this->getUri());
+            $profile = $oprofile->localProfile();
+        } catch (NoProfileException $e) {
+            // If the Ostatus_profile has no local Profile bound to it, let's clean it out at the same time
+            $oprofile->delete();
+        } catch (NoUriException $e) {
+            // FeedSub->getUri() can throw a NoUriException, let's just go ahead and delete it
+        }
+        return parent::delete($useWhere);
+    }
 }

diff --git a/plugins/OStatus/classes/Ostatus_profile.php b/plugins/OStatus/classes/Ostatus_profile.php
index 9cf712dc7fcc4acfea1a6db9bcb973962f1f0ade..19fa20f805f11616cd60263519d9fc8f7a5cbbd5 100644 (file)
--- a/plugins/OStatus/classes/Ostatus_profile.php
+++ b/plugins/OStatus/classes/Ostatus_profile.php
@@ -63,10 +63,10 @@ class Ostatus_profile extends Managed_DataObject
             ),
             'primary key' => array('uri'),
             'unique keys' => array(
-                'ostatus_profile_profile_id_idx' => array('profile_id'),
-                'ostatus_profile_group_id_idx' => array('group_id'),
-                'ostatus_profile_peopletag_id_idx' => array('peopletag_id'),
-                'ostatus_profile_feeduri_idx' => array('feeduri'),
+                'ostatus_profile_profile_id_key' => array('profile_id'),
+                'ostatus_profile_group_id_key' => array('group_id'),
+                'ostatus_profile_peopletag_id_key' => array('peopletag_id'),
+                'ostatus_profile_feeduri_key' => array('feeduri'),
             ),
             'foreign keys' => array(
                 'ostatus_profile_profile_id_fkey' => array('profile', array('profile_id' => 'id')),
@@ -82,15 +82,17 @@ class Ostatus_profile extends Managed_DataObject
     }
 
     /**
-     * Fetch the StatusNet-side profile for this feed
+     * Fetch the locally stored profile for this feed
      * @return Profile
+     * @throws NoProfileException if it was not found
      */
     public function localProfile()
     {
-        if ($this->profile_id) {
-            return Profile::getKV('id', $this->profile_id);
+        $profile = Profile::getKV('id', $this->profile_id);
+        if ($profile instanceof Profile) {
+            return $profile;
         }
-        return null;
+        throw new NoProfileException($this->profile_id);
     }
 
     /**
@@ -247,6 +249,7 @@ class Ostatus_profile extends Managed_DataObject
      * FeedSub::garbageCollect().
      *
      * @return int
+     * @throws NoProfileException if there is no local profile for the object
      */
     public function subscriberCount()
     {
@@ -258,9 +261,10 @@ class Ostatus_profile extends Managed_DataObject
             $count = $subscribers->N;
         } else {
             $profile = $this->localProfile();
-            $count = $profile->subscriberCount();
             if ($profile->hasLocalTags()) {
                 $count = 1;
+            } else {
+                $count = $profile->subscriberCount();
             }
         }
         common_log(LOG_INFO, __METHOD__ . " SUB COUNT BEFORE: $count");
@@ -940,12 +944,16 @@ class Ostatus_profile extends Managed_DataObject
             if ($id) {
                 $group = User_group::getKV('id', $id);
                 if ($group instanceof User_group) {
-                    // Deliver to all members of this local group if allowed.
-                    $profile = $sender->localProfile();
-                    if ($profile->isMember($group)) {
-                        $groups[] = $group->id;
-                    } else {
-                        common_log(LOG_DEBUG, "Skipping reply to local group $group->nickname as sender $profile->id is not a member");
+                    try {
+                        // Deliver to all members of this local group if allowed.
+                        $profile = $sender->localProfile();
+                        if ($profile->isMember($group)) {
+                            $groups[] = $group->id;
+                        } else {
+                            common_log(LOG_DEBUG, "Skipping reply to local group $group->nickname as sender $profile->id is not a member");
+                        }
+                    } catch (NoProfileException $e) {
+                        // Sender has no profile! Do some garbage collection, please.
                     }
                     continue;
                 } else {
@@ -1227,8 +1235,10 @@ class Ostatus_profile extends Managed_DataObject
         }
 
         if ($this->isGroup()) {
+            // FIXME: throw exception for localGroup
             $self = $this->localGroup();
         } else {
+            // this throws an exception already
             $self = $this->localProfile();
         }
         if (!$self) {

diff --git a/plugins/OStatus/scripts/gcfeeds.php b/plugins/OStatus/scripts/gcfeeds.php
index 55c29238fba8854dde90bea696692df71449b17e..a4f71ab77f350a35d16271851a92849b0fe45254 100644 (file)
--- a/plugins/OStatus/scripts/gcfeeds.php
+++ b/plugins/OStatus/scripts/gcfeeds.php
@@ -31,13 +31,22 @@ require_once INSTALLDIR.'/scripts/commandline.inc';
 $feedsub = new FeedSub();
 $feedsub->find();
 while ($feedsub->fetch()) {
-    echo "{$feedsub->uri} ({$feedsub->sub_state})";
     try {
+        echo $feedsub->getUri() . " ({$feedsub->sub_state})";
         if ($feedsub->garbageCollect()) {
             echo " INACTIVE\n";
         } else {
             echo " ACTIVE\n";
         }
+    } catch (NoProfileException $e) {
+        echo " DELETED (no profile)\n";
+        $feedsub->delete();
+        continue;
+    } catch (NoUriException $e) {
+        // Probably the getUri() call
+        echo "[unknown] DELETED (no uri)\n";
+        $feedsub->delete();
+        continue;
     } catch (Exception $e) {
         echo " ERROR: {$e->getMessage()}\n";
     }

diff --git a/plugins/OStatus/scripts/update-profile-data.php b/plugins/OStatus/scripts/update-profile-data.php
index bca71b6258f2eb63a79d22276d61fa198614a206..116061a7c3bc626183a4a74d163da67bc558af5d 100644 (file)
--- a/plugins/OStatus/scripts/update-profile-data.php
+++ b/plugins/OStatus/scripts/update-profile-data.php
@@ -38,13 +38,17 @@ END_OF_HELP;
 
 require_once INSTALLDIR.'/scripts/commandline.inc';
 
-function showProfileInfo($oprofile) {
+function showProfileInfo(Ostatus_profile $oprofile) {
     if ($oprofile->isGroup()) {
         echo "group\n";
     } else {
         $profile = $oprofile->localProfile();
-        foreach (array('nickname', 'bio', 'homepage', 'location') as $field) {
-            print "  $field: {$profile->$field}\n";
+        try {
+            foreach (array('nickname', 'bio', 'homepage', 'location') as $field) {
+                print "  $field: {$profile->$field}\n";
+            }
+        } catch (NoProfileException $e) {
+            print "local profile not found";
         }
     }
     echo "\n";

diff --git a/plugins/SubMirror/SubMirrorPlugin.php b/plugins/SubMirror/SubMirrorPlugin.php
index ca797c8531405c4b14e6d182f23cd9fbedb50ecb..b0f95125f30f7defa46dedbe59273edbec1ec792 100644 (file)
--- a/plugins/SubMirror/SubMirrorPlugin.php
+++ b/plugins/SubMirror/SubMirrorPlugin.php
@@ -129,20 +129,20 @@ class SubMirrorPlugin extends Plugin
      * @param int $count in/out
      * @return mixed hook return value
      */
-    function onOstatus_profileSubscriberCount($oprofile, &$count)
+    function onOstatus_profileSubscriberCount(Ostatus_profile $oprofile, &$count)
     {
-        if (empty($oprofile) || !($oprofile instanceof Ostatus_profile)) {
-            return true;
-        }
-
-        if ($oprofile->profile_id) {
+        try {
+            $profile = $oprofile->localProfile();
             $mirror = new SubMirror();
-            $mirror->subscribed = $oprofile->profile_id;
+            $mirror->subscribed = $profile->id;
             if ($mirror->find()) {
                 while ($mirror->fetch()) {
                     $count++;
                 }
             }
+        } catch (NoProfileException $e) {
+            // We can't handle this kind of Ostatus_profile since it has no
+            // local profile
         }
 
         return true;