From: Brion Vibber <brion@pobox.com>
Date: Fri, 30 Apr 2010 21:41:54 +0000 (-0700)
Subject: IM cleanup on 1.0.x branch:
X-Git-Url: https://git.mxchange.org/?a=commitdiff_plain;h=5414396a2ee9f1401d69b60969e04a1941e24e21;p=quix0rs-gnu-social.git

IM cleanup on 1.0.x branch:
* Fake_XMPP back to Queued_XMPP, refactor how we use it and don't create objects and load classes until we need them.
* fix fatal error in IM settings while waiting for a Jabber confirmation.
* Caching fix for user_im_prefs
* fix for saving multiple transport settings
* some fixes for AIM & using normalized addresses for lookups
---

diff --git a/actions/imsettings.php b/actions/imsettings.php
index 2c2606b76c..662b1063e7 100644
--- a/actions/imsettings.php
+++ b/actions/imsettings.php
@@ -133,8 +133,7 @@ class ImsettingsAction extends ConnectSettingsAction
                                              'message with further instructions. '.
                                              '(Did you add %s to your buddy list?)'),
                                              $transport_info['display'],
-                                             $transport_info['daemon_screenname'],
-                                             jabber_daemon_address()));
+                                             $transport_info['daemon_screenname']));
                     $this->hidden('screenname', $confirm->address);
                     // TRANS: Button label to cancel an IM address confirmation procedure.
                     $this->submit('cancel', _m('BUTTON','Cancel'));
@@ -163,12 +162,11 @@ class ImsettingsAction extends ConnectSettingsAction
                                               'action' =>
                                               common_local_url('imsettings')));
             $this->elementStart('fieldset', array('id' => 'settings_im_preferences'));
-            $this->element('legend', null, _('Preferences'));
+            // TRANS: Header for IM preferences form.
+            $this->element('legend', null, _('IM Preferences'));
             $this->hidden('token', common_session_token());
             $this->elementStart('table');
             $this->elementStart('tr');
-            // TRANS: Header for IM preferences form.
-            $this->element('th', null, _('IM Preferences'));
             foreach($user_im_prefs_by_transport as $transport=>$user_im_prefs)
             {
                 $this->element('th', null, $transports[$transport]['display']);
@@ -278,19 +276,20 @@ class ImsettingsAction extends ConnectSettingsAction
         $user = common_current_user();
 
         $user_im_prefs = new User_im_prefs();
+        $user_im_prefs->query('BEGIN');
         $user_im_prefs->user_id = $user->id;
         if($user_im_prefs->find() && $user_im_prefs->fetch())
         {
             $preferences = array('notify', 'updatefrompresence', 'replies', 'microid');
-            $user_im_prefs->query('BEGIN');
             do
             {
                 $original = clone($user_im_prefs);
+                $new = clone($user_im_prefs);
                 foreach($preferences as $preference)
                 {
-                    $user_im_prefs->$preference = $this->boolean($user_im_prefs->transport . '_' . $preference);
+                    $new->$preference = $this->boolean($new->transport . '_' . $preference);
                 }
-                $result = $user_im_prefs->update($original);
+                $result = $new->update($original);
 
                 if ($result === false) {
                     common_log_db_error($user, 'UPDATE', __FILE__);
@@ -299,8 +298,8 @@ class ImsettingsAction extends ConnectSettingsAction
                     return;
                 }
             }while($user_im_prefs->fetch());
-            $user_im_prefs->query('COMMIT');
         }
+        $user_im_prefs->query('COMMIT');
         // TRANS: Confirmation message for successful IM preferences save.
         $this->showForm(_('Preferences saved.'), true);
     }
diff --git a/classes/User_im_prefs.php b/classes/User_im_prefs.php
index 8ecdfe9fab..75be8969e0 100644
--- a/classes/User_im_prefs.php
+++ b/classes/User_im_prefs.php
@@ -68,4 +68,27 @@ class User_im_prefs extends Memcached_DataObject
     {
         return array(false,false);
     }
+
+    /**
+     * We have two compound keys with unique constraints:
+     * (transport, user_id) which is our primary key, and
+     * (transport, screenname) which is an additional constraint.
+     * 
+     * Currently there's not a way to represent that second key
+     * in the general keys list, so we're adding it here to the
+     * list of keys to use for caching, ensuring that it gets
+     * cleared as well when we change.
+     * 
+     * @return array of cache keys
+     */
+    function _allCacheKeys()
+    {
+        $ukeys = 'transport,screenname';
+        $uvals = $this->transport . ',' . $this->screenname;
+
+        $ckeys = parent::_allCacheKeys();
+        $ckeys[] = $this->cacheKey($this->tableName(), $ukeys, $uvals);
+        return $ckeys;
+    }
+
 }
diff --git a/classes/statusnet.ini b/classes/statusnet.ini
index d13fdfa526..b57d862263 100644
--- a/classes/statusnet.ini
+++ b/classes/statusnet.ini
@@ -647,8 +647,10 @@ modified = 384
 [user_im_prefs__keys]
 user_id = K
 transport = K
-transport = U
-screenname = U
+; There's another unique index on (transport, screenname)
+; but we have no way to represent a compound index other than
+; the primary key in here. To ensure proper cache purging,
+; we need to tweak the class.
 
 [user_urlshortener_prefs]
 user_id = 129
diff --git a/lib/implugin.php b/lib/implugin.php
index 7302859a47..7125aaee8d 100644
--- a/lib/implugin.php
+++ b/lib/implugin.php
@@ -107,10 +107,15 @@ abstract class ImPlugin extends Plugin
      * receive a raw message
      * Raw IM data is taken from the incoming queue, and passed to this function.
      * It should parse the raw message and call handle_incoming()
+     * 
+     * Returning false may CAUSE REPROCESSING OF THE QUEUE ITEM, and should
+     * be used for temporary failures only. For permanent failures such as
+     * unrecognized addresses, return true to indicate your processing has
+     * completed.
      *
      * @param object $data raw IM data
      *
-     * @return boolean success value
+     * @return boolean true if processing completed, false for temporary failures
      */
     abstract function receive_raw_message($data);
 
@@ -185,9 +190,12 @@ abstract class ImPlugin extends Plugin
      */
     function get_user_im_prefs_from_screenname($screenname)
     {
-        if($user_im_prefs = User_im_prefs::pkeyGet( array('transport' => $this->transport, 'screenname' => $screenname) )){
+        $user_im_prefs = User_im_prefs::pkeyGet(
+            array('transport' => $this->transport,
+                  'screenname' => $this->normalize($screenname)));
+        if ($user_im_prefs) {
             return $user_im_prefs;
-        }else{
+        } else {
             return false;
         }
     }
@@ -203,9 +211,9 @@ abstract class ImPlugin extends Plugin
     function get_screenname($user)
     {
         $user_im_prefs = $this->get_user_im_prefs_from_user($user);
-        if($user_im_prefs){
+        if ($user_im_prefs) {
             return $user_im_prefs->screenname;
-        }else{
+        } else {
             return false;
         }
     }
@@ -220,9 +228,12 @@ abstract class ImPlugin extends Plugin
      */
     function get_user_im_prefs_from_user($user)
     {
-        if($user_im_prefs = User_im_prefs::pkeyGet( array('transport' => $this->transport, 'user_id' => $user->id) )){
+        $user_im_prefs = User_im_prefs::pkeyGet(
+            array('transport' => $this->transport,
+                  'user_id' => $user->id));
+        if ($user_im_prefs){
             return $user_im_prefs;
-        }else{
+        } else {
             return false;
         }
     }
diff --git a/plugins/Aim/AimPlugin.php b/plugins/Aim/AimPlugin.php
index 3855d1fb05..30da1dbc79 100644
--- a/plugins/Aim/AimPlugin.php
+++ b/plugins/Aim/AimPlugin.php
@@ -126,6 +126,11 @@ class AimPlugin extends ImPlugin
         return true;
     }
 
+    /**
+     * Accept a queued input message.
+     *
+     * @return true if processing completed, false if message should be reprocessed
+     */
     function receive_raw_message($message)
     {
         $info=Aim::getMessageInfo($message);
@@ -133,7 +138,9 @@ class AimPlugin extends ImPlugin
         $user = $this->get_user($from);
         $notice_text = $info['message'];
 
-        return $this->handle_incoming($from, $notice_text);
+        $this->handle_incoming($from, $notice_text);
+
+        return true;
     }
 
     function initialize(){
diff --git a/plugins/Aim/README b/plugins/Aim/README
index 0465917383..7d486a0366 100644
--- a/plugins/Aim/README
+++ b/plugins/Aim/README
@@ -6,7 +6,7 @@ add "addPlugin('aim',
     array('setting'=>'value', 'setting2'=>'value2', ...);"
 to the bottom of your config.php
 
-The daemon included with this plugin must be running. It will be started by
+scripts/imdaemon.php included with StatusNet must be running. It will be started by
 the plugin along with their other daemons when you run scripts/startdaemons.sh.
 See the StatusNet README for more about queuing and daemons.
 
diff --git a/plugins/Xmpp/Fake_XMPP.php b/plugins/Xmpp/Fake_XMPP.php
deleted file mode 100644
index 0f7cfd3b4d..0000000000
--- a/plugins/Xmpp/Fake_XMPP.php
+++ /dev/null
@@ -1,114 +0,0 @@
-<?php
-/**
- * StatusNet, the distributed open-source microblogging tool
- *
- * Instead of sending XMPP messages, retrieve the raw XML that would be sent
- *
- * PHP version 5
- *
- * LICENCE: This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Affero General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Affero General Public License for more details.
- *
- * You should have received a copy of the GNU Affero General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *
- * @category  Network
- * @package   StatusNet
- * @author    Brion Vibber <brion@status.net>
- * @copyright 2010 StatusNet, Inc.
- * @license   http://www.fsf.org/licensing/licenses/agpl-3.0.html GNU Affero General Public License version 3.0
- * @link      http://status.net/
- */
-
-if (!defined('STATUSNET') && !defined('LACONICA')) {
-    exit(1);
-}
-
-class Fake_XMPP extends XMPPHP_XMPP
-{
-    public $would_be_sent = null;
-
-	/**
-	 * Constructor
-	 *
-	 * @param string  $host
-	 * @param integer $port
-	 * @param string  $user
-	 * @param string  $password
-	 * @param string  $resource
-	 * @param string  $server
-	 * @param boolean $printlog
-	 * @param string  $loglevel
-	 */
-	public function __construct($host, $port, $user, $password, $resource, $server = null, $printlog = false, $loglevel = null)
-	{
-        parent::__construct($host, $port, $user, $password, $resource, $server, $printlog, $loglevel);
-
-        // We use $host to connect, but $server to build JIDs if specified.
-        // This seems to fix an upstream bug where $host was used to build
-        // $this->basejid, never seen since it isn't actually used in the base
-        // classes.
-        if (!$server) {
-            $server = $this->host;
-        }
-        $this->basejid = $this->user . '@' . $server;
-
-        // Normally the fulljid is filled out by the server at resource binding
-        // time, but we need to do it since we're not talking to a real server.
-        $this->fulljid = "{$this->basejid}/{$this->resource}";
-    }
-
-    /**
-     * Send a formatted message to the outgoing queue for later forwarding
-     * to a real XMPP connection.
-     *
-     * @param string $msg
-     */
-    public function send($msg, $timeout=NULL)
-    {
-        $this->would_be_sent = $msg;
-    }
-
-    //@{
-    /**
-     * Stream i/o functions disabled; only do output
-     */
-    public function connect($timeout = 30, $persistent = false, $sendinit = true)
-    {
-        throw new Exception("Can't connect to server from fake XMPP.");
-    }
-
-    public function disconnect()
-    {
-        throw new Exception("Can't connect to server from fake XMPP.");
-    }
-
-    public function process()
-    {
-        throw new Exception("Can't read stream from fake XMPP.");
-    }
-
-    public function processUntil($event, $timeout=-1)
-    {
-        throw new Exception("Can't read stream from fake XMPP.");
-    }
-
-    public function read()
-    {
-        throw new Exception("Can't read stream from fake XMPP.");
-    }
-
-    public function readyToProcess()
-    {
-        throw new Exception("Can't read stream from fake XMPP.");
-    }
-    //@}
-}
-
diff --git a/plugins/Xmpp/Queued_XMPP.php b/plugins/Xmpp/Queued_XMPP.php
new file mode 100644
index 0000000000..73eff22467
--- /dev/null
+++ b/plugins/Xmpp/Queued_XMPP.php
@@ -0,0 +1,121 @@
+<?php
+/**
+ * StatusNet, the distributed open-source microblogging tool
+ *
+ * Queue-mediated proxy class for outgoing XMPP messages.
+ *
+ * PHP version 5
+ *
+ * LICENCE: This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * @category  Network
+ * @package   StatusNet
+ * @author    Brion Vibber <brion@status.net>
+ * @copyright 2010 StatusNet, Inc.
+ * @license   http://www.fsf.org/licensing/licenses/agpl-3.0.html GNU Affero General Public License version 3.0
+ * @link      http://status.net/
+ */
+
+if (!defined('STATUSNET') && !defined('LACONICA')) {
+    exit(1);
+}
+
+class Queued_XMPP extends XMPPHP_XMPP
+{
+    /**
+     * Reference to the XmppPlugin object we're hooked up to.
+     */
+    public $plugin;
+
+	/**
+	 * Constructor
+	 *
+     * @param XmppPlugin $plugin
+	 * @param string  $host
+	 * @param integer $port
+	 * @param string  $user
+	 * @param string  $password
+	 * @param string  $resource
+	 * @param string  $server
+	 * @param boolean $printlog
+	 * @param string  $loglevel
+	 */
+	public function __construct($plugin, $host, $port, $user, $password, $resource, $server = null, $printlog = false, $loglevel = null)
+	{
+        $this->plugin = $plugin;
+
+        parent::__construct($host, $port, $user, $password, $resource, $server, $printlog, $loglevel);
+
+        // We use $host to connect, but $server to build JIDs if specified.
+        // This seems to fix an upstream bug where $host was used to build
+        // $this->basejid, never seen since it isn't actually used in the base
+        // classes.
+        if (!$server) {
+            $server = $this->host;
+        }
+        $this->basejid = $this->user . '@' . $server;
+
+        // Normally the fulljid is filled out by the server at resource binding
+        // time, but we need to do it since we're not talking to a real server.
+        $this->fulljid = "{$this->basejid}/{$this->resource}";
+    }
+
+    /**
+     * Send a formatted message to the outgoing queue for later forwarding
+     * to a real XMPP connection.
+     *
+     * @param string $msg
+     */
+    public function send($msg, $timeout=NULL)
+    {
+        $this->plugin->enqueue_outgoing_raw($msg);
+    }
+
+    //@{
+    /**
+     * Stream i/o functions disabled; only do output
+     */
+    public function connect($timeout = 30, $persistent = false, $sendinit = true)
+    {
+        throw new Exception("Can't connect to server from fake XMPP.");
+    }
+
+    public function disconnect()
+    {
+        throw new Exception("Can't connect to server from fake XMPP.");
+    }
+
+    public function process()
+    {
+        throw new Exception("Can't read stream from fake XMPP.");
+    }
+
+    public function processUntil($event, $timeout=-1)
+    {
+        throw new Exception("Can't read stream from fake XMPP.");
+    }
+
+    public function read()
+    {
+        throw new Exception("Can't read stream from fake XMPP.");
+    }
+
+    public function readyToProcess()
+    {
+        throw new Exception("Can't read stream from fake XMPP.");
+    }
+    //@}
+
+}
+
diff --git a/plugins/Xmpp/XmppPlugin.php b/plugins/Xmpp/XmppPlugin.php
index 03bf47feac..a2521536bc 100644
--- a/plugins/Xmpp/XmppPlugin.php
+++ b/plugins/Xmpp/XmppPlugin.php
@@ -60,8 +60,6 @@ class XmppPlugin extends ImPlugin
 
     public $transport = 'xmpp';
 
-    protected $fake_xmpp;
-
     function getDisplayName(){
         return _m('XMPP/Jabber/GTalk');
     }
@@ -292,7 +290,7 @@ class XmppPlugin extends ImPlugin
             require_once 'XMPP.php';
             return false;
         case 'Sharing_XMPP':
-        case 'Fake_XMPP':
+        case 'Queued_XMPP':
             require_once $dir . '/'.$cls.'.php';
             return false;
         case 'XmppManager':
@@ -317,9 +315,7 @@ class XmppPlugin extends ImPlugin
 
     function send_message($screenname, $body)
     {
-        $this->fake_xmpp->message($screenname, $body, 'chat');
-        $this->enqueue_outgoing_raw($this->fake_xmpp->would_be_sent);
-        return true;
+        $this->queuedConnection()->message($screenname, $body, 'chat');
     }
 
     function send_notice($screenname, $notice)
@@ -327,8 +323,7 @@ class XmppPlugin extends ImPlugin
         $msg   = $this->format_notice($notice);
         $entry = $this->format_entry($notice);
         
-        $this->fake_xmpp->message($screenname, $msg, 'chat', null, $entry);
-        $this->enqueue_outgoing_raw($this->fake_xmpp->would_be_sent);
+        $this->queuedConnection()->message($screenname, $msg, 'chat', null, $entry);
         return true;
     }
 
@@ -385,10 +380,19 @@ class XmppPlugin extends ImPlugin
             return true;
         }
 
-        return $this->handle_incoming($from, $pl['body']);
+        $this->handle_incoming($from, $pl['body']);
+        
+        return true;
     }
 
-    function initialize(){
+    /**
+     * Build a queue-proxied XMPP interface object. Any outgoing messages
+     * will be run back through us for enqueing rather than sent directly.
+     * 
+     * @return Queued_XMPP
+     * @throws Exception if server settings are invalid.
+     */
+    function queuedConnection(){
         if(!isset($this->server)){
             throw new Exception("must specify a server");
         }
@@ -402,7 +406,7 @@ class XmppPlugin extends ImPlugin
             throw new Exception("must specify a password");
         }
 
-        $this->fake_xmpp = new Fake_XMPP($this->host ?
+        return new Queued_XMPP($this, $this->host ?
                                     $this->host :
                                     $this->server,
                                     $this->port,
@@ -415,7 +419,6 @@ class XmppPlugin extends ImPlugin
                                     $this->debug ?
                                     XMPPHP_Log::LEVEL_VERBOSE :  null
                                     );
-        return true;
     }
 
     function onPluginVersion(&$versions)