Refactor store_bookmarks()

- Add logging
- Move Node building code into own methods

gajim/common/connection.py

@@ -2347,6 +2347,7 @@ class Connection(CommonConnection, ConnectionHandlers):
         iq2 = iq.addChild(name='query', namespace=nbxmpp.NS_PRIVATE)
         iq2.addChild(name='storage', namespace='storage:bookmarks')
         self.connection.send(iq)
+        app.log('bookmarks').info('Request Bookmarks (PrivateStorage)')
 
     def _check_bookmarks_received(self):
         if not self.bookmarks:
@@ -2364,43 +2365,39 @@ class Connection(CommonConnection, ConnectionHandlers):
         if self.pubsub_supported and self.pubsub_publish_options_supported \
                 and storage_type != 'xml':
             self.send_pb_retrieve('', 'storage:bookmarks')
+            app.log('bookmarks').info('Request Bookmarks (PubSub)')
             # some server (ejabberd) are so slow to answer that we request via XML
             # if we don't get answer in the next 30 seconds
             app.idlequeue.set_alarm(self._check_bookmarks_received, 30)
         else:
             self._request_bookmarks_xml()
 
-    def store_bookmarks(self, storage_type=None):
+    def get_bookmarks_storage_node(self):
-        """
-        Send bookmarks to the storage namespace or PubSub if supported
-
-        storage_type can be set to 'pubsub' or 'xml' so store in only one method
-        else it will be stored on both
-        """
         NS_GAJIM_BM = 'xmpp:gajim.org/bookmarks'
-        if not app.account_is_connected(self.name):
+        storage_node = nbxmpp.Node(
-            return
+            tag='storage', attrs={'xmlns': 'storage:bookmarks'})
-        iq = nbxmpp.Node(tag='storage', attrs={'xmlns': 'storage:bookmarks'})
         for bm in self.bookmarks:
-            iq2 = iq.addChild(name="conference")
+            conf_node = storage_node.addChild(name="conference")
-            iq2.setAttr('jid', bm['jid'])
+            conf_node.setAttr('jid', bm['jid'])
-            iq2.setAttr('autojoin', bm['autojoin'])
+            conf_node.setAttr('autojoin', bm['autojoin'])
-            iq2.setAttr('name', bm['name'])
+            conf_node.setAttr('name', bm['name'])
-            iq2.setTag('minimize', namespace=NS_GAJIM_BM). \
+            conf_node.setTag(
-                setData(bm['minimize'])
+                'minimize', namespace=NS_GAJIM_BM).setData(bm['minimize'])
             # Only add optional elements if not empty
             # Note: need to handle both None and '' as empty
             #   thus shouldn't use "is not None"
             if bm.get('nick', None):
-                iq2.setTagData('nick', bm['nick'])
+                conf_node.setTagData('nick', bm['nick'])
             if bm.get('password', None):
-                iq2.setTagData('password', bm['password'])
+                conf_node.setTagData('password', bm['password'])
             if bm.get('print_status', None):
-                iq2.setTag('print_status', namespace=NS_GAJIM_BM). \
+                conf_node.setTag(
-                    setData(bm['print_status'])
+                    'print_status',
+                    namespace=NS_GAJIM_BM).setData(bm['print_status'])
+        return storage_node
 
-        if self.pubsub_supported and self.pubsub_publish_options_supported and\
+    @staticmethod
-                storage_type != 'xml':
+    def get_bookmark_publish_options():
         options = nbxmpp.Node(nbxmpp.NS_DATA + ' x',
                               attrs={'type': 'submit'})
         f = options.addChild('field',
@@ -2411,13 +2408,31 @@ class Connection(CommonConnection, ConnectionHandlers):
         f.setTagData('value', 'true')
         f = options.addChild('field', attrs={'var': 'pubsub#access_model'})
         f.setTagData('value', 'whitelist')
-            self.send_pb_publish('', 'storage:bookmarks', iq, 'current',
+        return options
-                                 options=options)
+
+    def store_bookmarks(self, storage_type=None):
+        """
+        Send bookmarks to the storage namespace or PubSub if supported
+
+        storage_type can be set to 'pubsub' or 'xml' so store in only one method
+        else it will be stored on both
+        """
+        if not app.account_is_connected(self.name):
+            return
+
+        storage_node = self.get_bookmarks_storage_node()
+
+        if storage_type != 'xml':
+            if self.pubsub_supported and self.pubsub_publish_options_supported:
+                self.send_pb_publish(
+                    '', 'storage:bookmarks', storage_node, 'current',
+                    options=self.get_bookmark_publish_options())
+                app.log('bookmarks').info('Bookmarks published (PubSub)')
+
         if storage_type != 'pubsub':
-            iqA = nbxmpp.Iq(typ='set')
+            iq = nbxmpp.Iq('set', nbxmpp.NS_PRIVATE, payload=storage_node)
-            iqB = iqA.addChild(name='query', namespace=nbxmpp.NS_PRIVATE)
+            self.connection.send(iq)
-            iqB.addChild(node=iq)
+            app.log('bookmarks').info('Bookmarks published (PrivateStorage)')
-            self.connection.send(iqA)
 
     def get_annotations(self):
         """

gajim/common/connection_handlers.py

@@ -1511,6 +1511,7 @@ ConnectionHandlersBase, ConnectionJingle, ConnectionIBBytestream):
     def _nec_private_storate_bookmarks_received(self, obj):
         if obj.conn.name != self.name:
             return
+        app.log('bookmarks').info('Received Bookmarks (PrivateStorage)')
         resend_to_pubsub = False
         bm_jids = [b['jid'] for b in self.bookmarks]
         for bm in obj.bookmarks:
@@ -1518,7 +1519,7 @@ ConnectionHandlersBase, ConnectionJingle, ConnectionIBBytestream):
                 self.bookmarks.append(bm)
                 # We got a bookmark that was not in pubsub
                 resend_to_pubsub = True
-        if self.pubsub_supported and resend_to_pubsub:
+        if resend_to_pubsub:
             self.store_bookmarks('pubsub')
 
     def _nec_private_storate_rosternotes_received(self, obj):

gajim/common/pubsub.py

@@ -203,6 +203,7 @@ class ConnectionPubSub:
     def _nec_pubsub_bookmarks_received(self, obj):
         if obj.conn.name != self.name:
             return
+        app.log('bookmarks').info('Received Bookmarks (PubSub)')
         bm_jids = [b['jid'] for b in self.bookmarks]
         for bm in obj.bookmarks:
             if bm['jid'] not in bm_jids: