Add Bookmarks 2 (XEP-0402) support

gajim/common/app.py

@@ -181,7 +181,6 @@
     nbxmpp.NS_JINGLE_BYTESTREAM,
     nbxmpp.NS_JINGLE_IBB,
     nbxmpp.NS_AVATAR_METADATA + '+notify',
-    nbxmpp.NS_BOOKMARKS + '+notify',
 ]
 
 # Optional features gajim supports per account

gajim/common/helpers.py

@@ -1087,6 +1087,10 @@ def update_optional_features(account=None):
             features.append(nbxmpp.NS_NICK + '+notify')
         if app.config.get_per('accounts', account_, 'subscribe_location'):
             features.append(nbxmpp.NS_LOCATION + '+notify')
+        if app.connections[account_].get_module('Bookmarks').using_bookmark_2:
+            features.append(nbxmpp.NS_BOOKMARKS_2 + '+notify')
+        elif app.connections[account_].get_module('Bookmarks').using_bookmark_1:
+            features.append(nbxmpp.NS_BOOKMARKS + '+notify')
         if not app.config.get('ignore_incoming_xhtml'):
             features.append(nbxmpp.NS_XHTML_IM)
         if app.config.get_per('accounts', account_, 'answer_receipts'):

gajim/common/modules/bookmarks.py

@@ -37,12 +37,14 @@ class Bookmarks(BaseModule):
     _nbxmpp_methods = [
         'request_bookmarks',
         'store_bookmarks',
+        'retract_bookmark',
     ]
 
     def __init__(self, con):
         BaseModule.__init__(self, con)
         self._register_pubsub_handler(self._bookmark_event_received)
         self._conversion = False
+        self._conversion_2 = False
         self._bookmarks = []
         self._join_timeouts = []
         self._request_in_progress = False
@@ -51,6 +53,10 @@ def __init__(self, con):
     def conversion(self):
         return self._conversion
 
+    @property
+    def conversion_2(self):
+        return self._conversion_2
+
     @property
     def bookmarks(self):
         return self._bookmarks
@@ -59,6 +65,14 @@ def bookmarks(self):
     def bookmarks(self, value):
         self._bookmarks = value
 
+    @property
+    def using_bookmark_1(self):
+        return self._pubsub_support() and self.conversion
+
+    @property
+    def using_bookmark_2(self):
+        return self._pubsub_support() and self.conversion_2
+
     @event_node(nbxmpp.NS_BOOKMARKS)
     def _bookmark_event_received(self, _con, stanza, properties):
         if properties.pubsub_event.retracted:
@@ -66,7 +80,7 @@ def _bookmark_event_received(self, _con, stanza, properties):
             return
 
         bookmarks = properties.pubsub_event.data
-        if properties.pubsub_event.deleted:
+        if properties.pubsub_event.deleted or properties.pubsub_event.purged:
             self._log.info('Bookmark node deleted')
             bookmarks = []
 
@@ -80,7 +94,7 @@ def _bookmark_event_received(self, _con, stanza, properties):
                               properties.jid)
             return
 
-        if not self._pubsub_support() or not self.conversion:
+        if not self.using_bookmark_1:
             return
 
         if self._request_in_progress:
@@ -93,11 +107,60 @@ def _bookmark_event_received(self, _con, stanza, properties):
         app.nec.push_incoming_event(
             NetworkEvent('bookmarks-received', account=self._account))
 
-    def pass_disco(self, info):
-        if nbxmpp.NS_BOOKMARK_CONVERSION not in info.features:
+    @event_node(nbxmpp.NS_BOOKMARKS_2)
+    def _bookmark_event_received(self, _con, stanza, properties):
+        if not properties.is_self_message:
+            self._log.warning('%s has an open access bookmarks node',
+                              properties.jid)
+            return
+
+        if not self.using_bookmark_2:
+            return
+
+        if self._request_in_progress:
+            self._log.info('Ignore update, pubsub request in progress')
             return
-        self._conversion = True
-        self._log.info('Discovered Bookmarks Conversion: %s', info.jid)
+
+        old_bookmarks = self._convert_to_set(self._bookmarks)
+
+        if properties.pubsub_event.deleted or properties.pubsub_event.purged:
+            self._log.info('Bookmark node deleted')
+            self._bookmarks = []
+
+        elif properties.pubsub_event.retracted:
+            jid = properties.pubsub_event.id
+            self._log.info('Retract: %s', jid)
+            bookmark = self.get_bookmark_from_jid(jid)
+            if bookmark is not None:
+                try:
+                    self._bookmarks.remove(bookmark)
+                except KeyError:
+                    pass
+
+        elif properties.pubsub_event.data is None:
+            self._log.warning('Invalid bookmark data')
+            self._log.warning(stanza)
+            return
+
+        else:
+            new_bookmark = properties.pubsub_event.data
+            old_bookmark = self.get_bookmark_from_jid(new_bookmark.jid)
+            if old_bookmark is not None:
+                self._bookmarks.remove(old_bookmark)
+            self._bookmarks.append(new_bookmark)
+
+        self._act_on_changed_bookmarks(old_bookmarks)
+        app.nec.push_incoming_event(
+            NetworkEvent('bookmarks-received', account=self._account))
+
+    def pass_disco(self, info):
+        if nbxmpp.NS_BOOKMARKS_COMPAT in info.features:
+            self._conversion_2 = True
+            self._log.info('Discovered Bookmarks Conversion 2: %s', info.jid)
+
+        elif nbxmpp.NS_BOOKMARK_CONVERSION in info.features:
+            self._conversion = True
+            self._log.info('Discovered Bookmarks Conversion: %s', info.jid)
 
     def _act_on_changed_bookmarks(self, old_bookmarks):
         new_bookmarks = self._convert_to_set(self._bookmarks)
@@ -142,8 +205,11 @@ def request_bookmarks(self):
 
         self._request_in_progress = True
         type_ = BookmarkStoreType.PRIVATE
-        if self._pubsub_support() and self.conversion:
-            type_ = BookmarkStoreType.PUBSUB
+        if self._pubsub_support():
+            if self.conversion:
+                type_ = BookmarkStoreType.PUBSUB_BOOKMARK_1
+            if self._conversion_2:
+                type_ = BookmarkStoreType.PUBSUB_BOOKMARK_2
 
         self._nbxmpp('Bookmarks').request_bookmarks(
             type_, callback=self._bookmarks_received)
@@ -159,15 +225,48 @@ def _bookmarks_received(self, bookmarks):
         app.nec.push_incoming_event(
             NetworkEvent('bookmarks-received', account=self._account))
 
-    def store_bookmarks(self):
+    def store_difference(self, bookmarks):
+        if self.using_bookmark_2:
+            retract, add_or_modify = self._determine_changed_bookmarks(
+                bookmarks, self._bookmarks)
+
+            for bookmark in retract:
+                self.remove(str(bookmark.jid))
+
+            if add_or_modify:
+                self.store_bookmarks(add_or_modify)
+            self._bookmarks = bookmarks
+
+        else:
+            self._bookmarks = bookmarks
+            self.store_bookmarks()
+
+    def store_bookmarks(self, bookmarks=None):
         if not app.account_is_connected(self._account):
             return
 
         type_ = BookmarkStoreType.PRIVATE
-        if self._pubsub_support() and self.conversion:
-            type_ = BookmarkStoreType.PUBSUB
+        if self._pubsub_support():
+            if self.conversion:
+                type_ = BookmarkStoreType.PUBSUB_BOOKMARK_1
+            if self.conversion_2:
+                type_ = BookmarkStoreType.PUBSUB_BOOKMARK_2
+
+        self._nbxmpp('Bookmarks').store_bookmarks(bookmarks or self._bookmarks,
+                                                  type_)
+
+        app.nec.push_incoming_event(
+            NetworkEvent('bookmarks-received', account=self._account))
+
+    def store_bookmark(self, bookmark):
+        if not app.account_is_connected(self._account):
+            return
+
+        if not self.using_bookmark_2:
+            return
 
-        self._nbxmpp('Bookmarks').store_bookmarks(self._bookmarks, type_)
+        self._nbxmpp('Bookmarks').store_bookmarks(
+            [bookmark], BookmarkStoreType.PUBSUB_BOOKMARK_2)
 
         app.nec.push_incoming_event(
             NetworkEvent('bookmarks-received', account=self._account))
@@ -199,19 +298,35 @@ def auto_join_bookmarks(self, bookmarks: Optional[List[Any]] = None) -> None:
     def modify(self, jid: str, **kwargs: Dict[str, str]) -> None:
         bookmark = self.get_bookmark_from_jid(jid)
         if bookmark is None:
-            bookmark = BookmarkData(jid=jid, **kwargs)
-            self._bookmarks.append(bookmark)
-            self._log.info('Add new bookmark: %s', bookmark)
+            return
+
+        new_bookmark = bookmark._replace(**kwargs)
+        if new_bookmark == bookmark:
+            # No change happened
+            return
+        self._log.info('Modify bookmark: %s %s', jid, kwargs)
+        self._bookmarks.remove(bookmark)
+        self._bookmarks.append(new_bookmark)
+
+        if self.using_bookmark_2:
+            self.store_bookmark(new_bookmark)
         else:
-            modified_bookmark = bookmark._replace(**kwargs)
-            if modified_bookmark == bookmark:
-                # No change happened
-                return
-            self._log.info('Modify bookmark: %s %s', jid, kwargs)
-            self._bookmarks.remove(bookmark)
-            self._bookmarks.append(modified_bookmark)
+            self.store_bookmarks()
+
+    def add_or_modify(self, jid: str, **kwargs: Dict[str, str]) -> None:
+        bookmark = self.get_bookmark_from_jid(jid)
+        if bookmark is not None:
+            self.modify(jid, **kwargs)
+            return
 
-        self.store_bookmarks()
+        new_bookmark = BookmarkData(jid=jid, **kwargs)
+        self._bookmarks.append(new_bookmark)
+        self._log.info('Add new bookmark: %s', new_bookmark)
+
+        if self.using_bookmark_2:
+            self.store_bookmark(new_bookmark)
+        else:
+            self.store_bookmarks()
 
     def remove(self, jid: str, publish: bool = True) -> None:
         bookmark = self.get_bookmark_from_jid(jid)
@@ -219,7 +334,27 @@ def remove(self, jid: str, publish: bool = True) -> None:
             return
         self._bookmarks.remove(bookmark)
         if publish:
-            self.store_bookmarks()
+            if self.using_bookmark_2:
+                self._nbxmpp('Bookmarks').retract_bookmark(jid)
+            else:
+                self.store_bookmarks()
+
+    @staticmethod
+    def _determine_changed_bookmarks(new_bookmarks, old_bookmarks):
+        new_jids = [bookmark.jid for bookmark in new_bookmarks]
+        new_bookmarks = set(new_bookmarks)
+        old_bookmarks = set(old_bookmarks)
+
+        retract = []
+        add_or_modify = []
+        changed_bookmarks = new_bookmarks.symmetric_difference(old_bookmarks)
+
+        for bookmark in changed_bookmarks:
+            if bookmark.jid not in new_jids:
+                retract.append(bookmark)
+            if bookmark in new_bookmarks:
+                add_or_modify.append(bookmark)
+        return retract, add_or_modify
 
     def get_name_from_bookmark(self, jid: str) -> str:
         bookmark = self.get_bookmark_from_jid(jid)

gajim/common/modules/discovery.py

@@ -20,8 +20,9 @@
 
 from gajim.common import app
 from gajim.common.nec import NetworkIncomingEvent
-from gajim.common.modules.base import BaseModule
 from gajim.common.nec import NetworkEvent
+from gajim.common.modules.base import BaseModule
+from gajim.common.helpers import update_optional_features
 
 
 class Discovery(BaseModule):
@@ -106,6 +107,8 @@ def _account_info_received(self, result):
         if 'urn:xmpp:pep-vcard-conversion:0' in result.features:
             self._con.avatar_conversion = True
 
+        update_optional_features()
+
     def discover_server_info(self):
         # Calling this method starts the connect_maschine()
         server = self._con.get_own_jid().getDomain()

gajim/common/modules/muc.py

@@ -688,10 +688,11 @@ def _room_join_complete(self, muc_data):
         self._remove_rejoin_timeout(muc_data.jid)
 
         # We successfully joined a MUC, set add bookmark with autojoin
-        self._con.get_module('Bookmarks').modify(muc_data.jid,
-                                                 autojoin=True,
-                                                 password=muc_data.password,
-                                                 nick=muc_data.nick)
+        self._con.get_module('Bookmarks').add_or_modify(
+            muc_data.jid,
+            autojoin=True,
+            password=muc_data.password,
+            nick=muc_data.nick)
 
     def _on_voice_request(self, _con, _stanza, properties):
         if not properties.is_voice_request:

gajim/gtk/bookmarks.py

@@ -19,6 +19,7 @@
 from gi.repository import Gdk
 from nbxmpp.structs import BookmarkData
 from nbxmpp.protocol import validate_resourcepart
+from nbxmpp.protocol import JID
 
 from gajim.common import app
 from gajim.common.helpers import validate_jid
@@ -142,7 +143,8 @@ def _on_apply_clicked(self, _button):
         for row in self._ui.bookmarks_store:
             if not row[Column.ADDRESS]:
                 continue
-            bookmark = BookmarkData(jid=row[Column.ADDRESS],
+
+            bookmark = BookmarkData(jid=JID(row[Column.ADDRESS]),
                                     name=row[Column.NAME],
                                     autojoin=row[Column.AUTOJOIN],
                                     password=row[Column.PASSWORD],
@@ -150,8 +152,7 @@ def _on_apply_clicked(self, _button):
             bookmarks.append(bookmark)
 
         con = app.connections[self.account]
-        con.get_module('Bookmarks').bookmarks = bookmarks
-        con.get_module('Bookmarks').store_bookmarks()
+        con.get_module('Bookmarks').store_difference(bookmarks)
         self.destroy()
 
     @staticmethod