Make the bookmark stuff non-blocking

3 files changed, 113 insertions, 83 deletions

diff --git a/src/bookmark.py b/src/bookmark.py
index 8d230d26..6d271652 100644
--- a/src/bookmark.py
+++ b/src/bookmark.py
@@ -8,6 +8,8 @@ bookmark storage. It can also parse xml Elements.
 This module also defines several functions for retrieving and updating
 bookmarks, both local and remote.
 """
+
+import functools
 import logging
 from sys import version_info
 
@@ -26,7 +28,7 @@ def xml_iter(xml, tag=''):
 preferred = config.get('use_bookmarks_method', 'pep').lower()
 if preferred not in ('pep', 'privatexml'):
     preferred = 'privatexml'
-not_preferred = 'privatexml' if preferred == 'pep' else 'privatexml'
+not_preferred = 'privatexml' if preferred == 'pep' else 'pep'
 methods = ('local', preferred, not_preferred)
 
 
@@ -131,21 +133,18 @@ def save_privatexml(xmpp):
     xmpp.plugin['xep_0048'].set_bookmarks(stanza_storage('privatexml'),
             method='xep_0049')
 
-def save_remote(xmpp, method=preferred):
+def save_remote(xmpp, callback, method=preferred):
     """Save the remote bookmarks."""
     method = 'privatexml' if method != 'pep' else 'pep'
 
-    try:
-        if method is 'privatexml':
-            xmpp.plugin['xep_0048'].set_bookmarks(stanza_storage('privatexml'),
-                    method='xep_0049')
-        else:
-            xmpp.plugin['xep_0048'].set_bookmarks(stanza_storage('pep'),
-                    method='xep_0223')
-    except Exception:
-        log.error("Could not save the bookmarks:", exc_info=True)
-        return False
-    return True
+    if method is 'privatexml':
+        xmpp.plugin['xep_0048'].set_bookmarks(stanza_storage('privatexml'),
+                                              method='xep_0049',
+                                              callback=callback)
+    else:
+        xmpp.plugin['xep_0048'].set_bookmarks(stanza_storage('pep'),
+                                              method='xep_0223',
+                                              callback=callback)
 
 def save_local():
     """Save the local bookmarks."""
@@ -155,62 +154,81 @@ def save_local():
 def save(xmpp, core=None):
     """Save all the bookmarks."""
     save_local()
-    if config.get('use_remote_bookmarks', True):
-        preferred = config.get('use_bookmarks_method', 'privatexml')
-        if not save_remote(xmpp, method=preferred) and core:
+    def _cb(core, iq):
+        if iq["type"] == "error":
             core.information('Could not save bookmarks.', 'Error')
-            return False
         elif core:
             core.information('Bookmarks saved', 'Info')
-    return True
+    if config.get('use_remote_bookmarks', True):
+        preferred = config.get('use_bookmarks_method', 'privatexml')
+        cb = functools.partial(_cb, core)
+        save_remote(xmpp, cb, method=preferred)
 
-def get_pep(xmpp):
+def get_pep(xmpp, available_methods, callback):
     """Add the remotely stored bookmarks via pep to the list."""
-    try:
-        iq = xmpp.plugin['xep_0048'].get_bookmarks(method='xep_0223', block=True)
-    except:
-        return False
-    for conf in xml_iter(iq.xml, '{storage:bookmarks}conference'):
-        b = Bookmark.parse_from_element(conf, method='pep')
-        if not get_by_jid(b.jid):
-            bookmarks.append(b)
-    return True
-
-def get_privatexml(xmpp):
-    """Add the remotely stored bookmarks via privatexml to the list."""
-    try:
-        iq = xmpp.plugin['xep_0048'].get_bookmarks(method='xep_0049', block=True)
-    except:
-        return False
-    for conf in xml_iter(iq.xml, '{storage:bookmarks}conference'):
-        b = Bookmark.parse_from_element(conf, method='privatexml')
-        if not get_by_jid(b.jid):
-            bookmarks.append(b)
-    return True
+    def _cb(iq):
+        if iq["type"] == "error":
+            available_methods["pep"] = False
+        else:
+            available_methods["pep"] = True
+            for conf in xml_iter(iq.xml, '{storage:bookmarks}conference'):
+                b = Bookmark.parse_from_element(conf, method='pep')
+                if not get_by_jid(b.jid):
+                    bookmarks.append(b)
+        if callback:
+            callback()
+
+    xmpp.plugin['xep_0048'].get_bookmarks(method='xep_0223', callback=_cb)
+
+def get_privatexml(xmpp, available_methods, callback):
+    """Add the remotely stored bookmarks via privatexml to the list.
+    If both is True, we want to have the result of both methods (privatexml and pep) before calling pep"""
+    def _cb(iq):
+        if iq["type"] == "error":
+            available_methods["privatexml"] = False
+        else:
+            available_methods["privatexml"] = True
+            for conf in xml_iter(iq.xml, '{storage:bookmarks}conference'):
+                b = Bookmark.parse_from_element(conf, method='privatexml')
+                if not get_by_jid(b.jid):
+                    bookmarks.append(b)
+        if callback:
+            callback()
+
+    xmpp.plugin['xep_0048'].get_bookmarks(method='xep_0049', callback=_cb)
 
-def get_remote(xmpp):
+def get_remote(xmpp, callback):
     """Add the remotely stored bookmarks to the list."""
     if xmpp.anon:
         return
     method = config.get('use_bookmarks_method', '')
     if not method:
-        pep, privatexml = True, True
+        available_methods = {}
+        def _save_and_call_callback():
+            # If both methods returned a result, we can now call the given callback
+            if callback and "privatexml" in available_methods and "pep" in available_methods:
+                save_bookmarks_method(available_methods)
+                if callback:
+                    callback()
         for method in methods[1:]:
             if method == 'pep':
-                pep = get_pep(xmpp)
+                get_pep(xmpp, available_methods, _save_and_call_callback)
             else:
-                privatexml = get_privatexml(xmpp)
-        if pep and not privatexml:
-            config.set_and_save('use_bookmarks_method', 'pep')
-        elif privatexml and not pep:
-            config.set_and_save('use_bookmarks_method', 'privatexml')
-        elif not pep and not privatexml:
-            config.set_and_save('use_bookmarks_method', '')
+                get_privatexml(xmpp, available_methods, _save_and_call_callback)
     else:
         if method == 'pep':
-            get_pep(xmpp)
+            get_pep(xmpp, {}, callback)
         else:
-            get_privatexml(xmpp)
+            get_privatexml(xmpp, {}, callback)
+
+def save_bookmarks_method(available_methods):
+    pep, privatexml = available_methods["pep"], available_methods["privatexml"]
+    if pep and not privatexml:
+        config.set_and_save('use_bookmarks_method', 'pep')
+    elif privatexml and not pep:
+        config.set_and_save('use_bookmarks_method', 'privatexml')
+    elif not pep and not privatexml:
+        config.set_and_save('use_bookmarks_method', '')
 
 def get_local():
     """Add the locally stored bookmarks to the list."""
diff --git a/src/core/commands.py b/src/core/commands.py
index 5254464a..5f1af49c 100644
--- a/src/core/commands.py
+++ b/src/core/commands.py
@@ -6,6 +6,7 @@ import logging
 
 log = logging.getLogger(__name__)
 
+import functools
 import sys
 from datetime import datetime
 from gettext import gettext as _
@@ -440,7 +441,7 @@ def command_bookmark_local(self, arg=''):
         new_bookmarks.extend(bookmark.bookmarks)
         bookmark.bookmarks = new_bookmarks
         bookmark.save_local()
-        bookmark.save_remote(self.xmpp)
+        bookmark.save_remote(self.xmpp, None)
         self.information('Bookmarks added and saved.', 'Info')
         return
     else:
@@ -508,12 +509,13 @@ def command_bookmark(self, arg=''):
                 new_bookmarks.append(b)
         new_bookmarks.extend(bookmark.bookmarks)
         bookmark.bookmarks = new_bookmarks
-
-        if bookmark.save_remote(self.xmpp):
-            bookmark.save_local()
-            self.information("Bookmarks added.", "Info")
-        else:
-            self.information("Could not add the bookmarks.", "Info")
+        def _cb(self, iq):
+            if iq["type"] != "error":
+                bookmark.save_local()
+                self.information("Bookmarks added.", "Info")
+            else:
+                self.information("Could not add the bookmarks.", "Info")
+        bookmark.save_remote(self.xmpp, functools.partial(_cb, self))
         return
     else:
         info = safeJID(args[0])
@@ -542,14 +544,16 @@ def command_bookmark(self, arg=''):
     if password:
         bm.password = password
     bm.autojoin = autojoin
-    if bookmark.save_remote(self.xmpp):
-        self.information('Bookmark added.', 'Info')
+    def _cb(self, iq):
+        if iq["type"] != "error":
+            self.information('Bookmark added.', 'Info')
+        else:
+            self.information("Could not add the bookmarks.", "Info")
+    bookmark.save_remote(self.xmpp, functools.partial(_cb, self))
     remote = []
     for each in bookmark.bookmarks:
         if each.method in ('pep', 'privatexml'):
             remote.append(each)
-    self.information(_('Your remote bookmarks are now: %s') % remote,
-                     _('Info'))
 
 def command_bookmarks(self, arg=''):
     """/bookmarks"""
diff --git a/src/core/handlers.py b/src/core/handlers.py
index 94a0614b..2dd31abd 100644
--- a/src/core/handlers.py
+++ b/src/core/handlers.py
@@ -869,27 +869,35 @@ def on_session_start(self, event):
             self.events.trigger('send_normal_presence', pres)
             pres.send()
     bookmark.get_local()
+    def _join_initial_rooms(bookmarks):
+        """Join all rooms given in the iterator `bookmarks`"""
+        for bm in bookmarks:
+            tab = self.get_tab_by_name(bm.jid, tabs.MucTab)
+            nick = bm.nick if bm.nick else self.own_nick
+            if not tab:
+                self.open_new_room(bm.jid, nick, False)
+            self.initial_joins.append(bm.jid)
+            histo_length = config.get('muc_history_length', 20)
+            if histo_length == -1:
+                histo_length = None
+            if histo_length is not None:
+                histo_length = str(histo_length)
+            # do not join rooms that do not have autojoin
+            # but display them anyway
+            if bm.autojoin:
+                muc.join_groupchat(self, bm.jid, nick,
+                        passwd=bm.password,
+                        maxhistory=histo_length,
+                        status=self.status.message,
+                        show=self.status.show)
+    def _join_remote_only():
+        remote_bookmarks = (bm for bm in bookmark.bookmarks if (bm.method in ("pep", "privatexml")))
+        _join_initial_rooms(remote_bookmarks)
     if not self.xmpp.anon and config.get('use_remote_bookmarks', True):
-        bookmark.get_remote(self.xmpp)
-    for bm in bookmark.bookmarks:
-        tab = self.get_tab_by_name(bm.jid, tabs.MucTab)
-        nick = bm.nick if bm.nick else self.own_nick
-        if not tab:
-            self.open_new_room(bm.jid, nick, False)
-        self.initial_joins.append(bm.jid)
-        histo_length = config.get('muc_history_length', 20)
-        if histo_length == -1:
-            histo_length = None
-        if histo_length is not None:
-            histo_length = str(histo_length)
-        # do not join rooms that do not have autojoin
-        # but display them anyway
-        if bm.autojoin:
-            muc.join_groupchat(self, bm.jid, nick,
-                    passwd=bm.password,
-                    maxhistory=histo_length,
-                    status=self.status.message,
-                    show=self.status.show)
+        bookmark.get_remote(self.xmpp, _join_remote_only)
+    # join all the available bookmarks. As of yet, this is just the local
+    # ones
+    _join_initial_rooms(bookmark.bookmarks)
 
     if config.get('enable_user_nick', True):
         self.xmpp.plugin['xep_0172'].publish_nick(nick=self.own_nick, callback=dumb_callback)