From a89bec0b9d45ab36389a9b21845266227ca30c91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philipp=20H=C3=B6rist?= Date: Sun, 3 Feb 2019 23:02:32 +0100 Subject: [PATCH] Refactor Bookmarks - Simplify modules because nbxmpp handles more stuff --- gajim/common/connection.py | 4 +- gajim/common/modules/bookmarks.py | 337 +++++++++--------------------- gajim/groupchat_control.py | 2 +- gajim/gtk/bookmarks.py | 34 ++- gajim/gtk/discovery.py | 2 +- gajim/gtk/start_chat.py | 4 +- gajim/gui_interface.py | 5 +- gajim/gui_menu_builder.py | 31 ++- gajim/roster_window.py | 12 +- 9 files changed, 145 insertions(+), 286 deletions(-) diff --git a/gajim/common/connection.py b/gajim/common/connection.py index 50a44e8f6..9bcf1049f 100644 --- a/gajim/common/connection.py +++ b/gajim/common/connection.py @@ -1497,7 +1497,7 @@ class Connection(CommonConnection, ConnectionHandlers): self.get_module('VCardTemp').request_vcard() # Get bookmarks - self.get_module('Bookmarks').get_bookmarks() + self.get_module('Bookmarks').request_bookmarks() # Get annotations self.get_module('Annotations').get_annotations() @@ -1625,7 +1625,7 @@ class Connection(CommonConnection, ConnectionHandlers): # ask our VCard self.get_module('VCardTemp').request_vcard() - self.get_module('Bookmarks').get_bookmarks() + self.get_module('Bookmarks').request_bookmarks() self.get_module('Annotations').get_annotations() self.get_module('Blocking').get_blocking_list() diff --git a/gajim/common/modules/bookmarks.py b/gajim/common/modules/bookmarks.py index 6a636482a..4ba704102 100644 --- a/gajim/common/modules/bookmarks.py +++ b/gajim/common/modules/bookmarks.py @@ -15,96 +15,97 @@ # XEP-0048: Bookmarks from typing import Any -from typing import Dict from typing import List from typing import Optional import logging import copy -from collections import OrderedDict import nbxmpp +from nbxmpp.structs import BookmarkData +from nbxmpp.const import BookmarkStoreType from gi.repository import GLib from gajim.common import app -from gajim.common import helpers -from gajim.common.const import PEPEventType from gajim.common.nec import NetworkEvent -from gajim.common.exceptions import StanzaMalformed -from gajim.common.modules.pep import AbstractPEPModule -from gajim.common.modules.pep import AbstractPEPData -from gajim.common.modules.util import from_xs_boolean -from gajim.common.modules.util import to_xs_boolean +from gajim.common.modules.base import BaseModule +from gajim.common.modules.util import event_node log = logging.getLogger('gajim.c.m.bookmarks') -class BookmarksData(AbstractPEPData): +class Bookmarks(BaseModule): - type_ = PEPEventType.BOOKMARKS - - -class Bookmarks(AbstractPEPModule): - - name = 'storage' - namespace = 'storage:bookmarks' - pep_class = BookmarksData - store_publish = False - _log = log + _nbxmpp_extends = 'Bookmarks' + _nbxmpp_methods = [ + 'request_bookmarks', + 'store_bookmarks', + ] def __init__(self, con): - AbstractPEPModule.__init__(self, con) - - self.bookmarks = {} - self.conversion = False + BaseModule.__init__(self, con) + self._register_pubsub_handler(self._bookmark_event_received) + self._conversion = False + self._bookmarks = [] self._join_timeouts = [] self._request_in_progress = False - def pass_disco(self, from_, _identities, features, _data, _node): - if nbxmpp.NS_BOOKMARK_CONVERSION not in features: - return - self.conversion = True - log.info('Discovered Bookmarks Conversion: %s', from_) + @property + def conversion(self): + return self._conversion - def _extract_info(self, item): - storage = item.getTag('storage', namespace=self.namespace) - if storage is None: - raise StanzaMalformed('No storage node') - return storage + @property + def bookmarks(self): + return self._bookmarks - def _notification_received(self, jid: nbxmpp.JID, user_pep: Any) -> None: - if self._request_in_progress: - log.info('Ignore update, pubsub request in progress') - return + @bookmarks.setter + def bookmarks(self, value): + self._bookmarks = value - if not self._con.get_own_jid().bareMatch(jid): - log.warning('%s has an open access bookmarks node', jid) + @event_node(nbxmpp.NS_BOOKMARKS) + def _bookmark_event_received(self, _con, _stanza, properties): + bookmarks = properties.pubsub_event.data + + if not properties.is_self_message: + log.warning('%s has an open access bookmarks node', properties.jid) return if not self._pubsub_support() or not self.conversion: return - old_bookmarks = self._convert_to_set(self.bookmarks) - self.bookmarks = self._parse_bookmarks(user_pep.data) + if self._request_in_progress: + log.info('Ignore update, pubsub request in progress') + return + + old_bookmarks = self._convert_to_set(self._bookmarks) + self._bookmarks = bookmarks self._act_on_changed_bookmarks(old_bookmarks) app.nec.push_incoming_event( NetworkEvent('bookmarks-received', account=self._account)) + def pass_disco(self, from_, _identities, features, _data, _node): + if nbxmpp.NS_BOOKMARK_CONVERSION not in features: + return + self._conversion = True + log.info('Discovered Bookmarks Conversion: %s', from_) + def _act_on_changed_bookmarks(self, old_bookmarks): - new_bookmarks = self._convert_to_set(self.bookmarks) + new_bookmarks = self._convert_to_set(self._bookmarks) changed = new_bookmarks - old_bookmarks if not changed: return join = [jid for jid, autojoin in changed if autojoin] + bookmarks = [] for jid in join: log.info('Schedule autojoin in 10s for: %s', jid) + bookmarks.append(self.get_bookmark_from_jid(jid)) # If another client creates a MUC, the MUC is locked until the # configuration is finished. Give the user some time to finish # the configuration. timeout_id = GLib.timeout_add_seconds( - 10, self._join_with_timeout, join) + 10, self._join_with_timeout, bookmarks) self._join_timeouts.append(timeout_id) # TODO: leave mucs @@ -113,256 +114,122 @@ class Bookmarks(AbstractPEPModule): @staticmethod def _convert_to_set(bookmarks): set_ = set() - for jid in bookmarks: - set_.add((jid, bookmarks[jid]['autojoin'])) + for bookmark in bookmarks: + set_.add((bookmark.jid, bookmark.autojoin)) return set_ + def get_bookmark_from_jid(self, jid): + for bookmark in self._bookmarks: + if bookmark.jid == jid: + return bookmark + def get_sorted_bookmarks(self, short_name=False): # This returns a sorted by name copy of the bookmarks - sorted_bookmarks = {} - for jid, bookmarks in self.bookmarks.items(): - bookmark_copy = copy.deepcopy(bookmarks) - if not bookmark_copy['name']: + sorted_bookmarks = [] + for bookmark in self._bookmarks: + bookmark_copy = copy.deepcopy(bookmark) + if not bookmark_copy.name: # No name was given for this bookmark # Use the first part of JID instead - name = jid.split("@")[0] - bookmark_copy['name'] = name + name = bookmark_copy.jid.split("@")[0] + bookmark_copy = bookmark_copy._replace(name=name) if short_name: - name = bookmark_copy['name'] + name = bookmark_copy.name name = (name[:42] + '..') if len(name) > 42 else name - bookmark_copy['name'] = name + bookmark_copy = bookmark_copy._replace(name=name) - sorted_bookmarks[jid] = bookmark_copy - return OrderedDict( - sorted(sorted_bookmarks.items(), - key=lambda bookmark: bookmark[1]['name'].lower())) + sorted_bookmarks.append(bookmark_copy) + + sorted_bookmarks.sort(key=lambda x: x.name.lower()) + return sorted_bookmarks def _pubsub_support(self) -> bool: return (self._con.get_module('PEP').supported and self._con.get_module('PubSub').publish_options) - def get_bookmarks(self): + def request_bookmarks(self): if not app.account_is_connected(self._account): return + self._request_in_progress = True + type_ = BookmarkStoreType.PRIVATE if self._pubsub_support() and self.conversion: - self._request_pubsub_bookmarks() - else: - self._request_private_bookmarks() + type_ = BookmarkStoreType.PUBSUB - def _request_pubsub_bookmarks(self) -> None: - log.info('Request Bookmarks (PubSub)') - self._request_in_progress = True - self._con.get_module('PubSub').send_pb_retrieve( - '', 'storage:bookmarks', cb=self._bookmarks_received) + self._nbxmpp('Bookmarks').request_bookmarks( + type_, callback=self._bookmarks_received) - def _request_private_bookmarks(self) -> None: - self._request_in_progress = True - iq = nbxmpp.Iq(typ='get') - query = iq.addChild(name='query', namespace=nbxmpp.NS_PRIVATE) - query.addChild(name='storage', namespace='storage:bookmarks') - log.info('Request Bookmarks (PrivateStorage)') - self._con.connection.SendAndCallForResponse( - iq, self._bookmarks_received, {}) - - def _bookmarks_received(self, _con, stanza): + def _bookmarks_received(self, bookmarks): self._request_in_progress = False - if not nbxmpp.isResultNode(stanza): - log.info('No bookmarks found: %s', stanza.getError()) - else: - log.info('Received Bookmarks') - storage = self._get_storage_node(stanza) - if storage is not None: - self.bookmarks = self._parse_bookmarks(storage) - self.auto_join_bookmarks() - + self._bookmarks = bookmarks + self.auto_join_bookmarks() app.nec.push_incoming_event( NetworkEvent('bookmarks-received', account=self._account)) - @staticmethod - def _get_storage_node(stanza): - node = stanza.getTag('pubsub', namespace=nbxmpp.NS_PUBSUB) - if node is None: - node = stanza.getTag('event', namespace=nbxmpp.NS_PUBSUB_EVENT) - if node is None: - # Private Storage - query = stanza.getQuery() - if query is None: - return - storage = query.getTag('storage', - namespace=nbxmpp.NS_BOOKMARKS) - if storage is None: - return - return storage - - items_node = node.getTag('items') - if items_node is None: - return - if items_node.getAttr('node') != nbxmpp.NS_BOOKMARKS: - return - - item_node = items_node.getTag('item') - if item_node is None: - return - - storage = item_node.getTag('storage', namespace=nbxmpp.NS_BOOKMARKS) - if storage is None: - return - return storage - - @staticmethod - def _parse_bookmarks(storage: nbxmpp.Node) -> Dict[str, Dict[str, Any]]: - bookmarks = {} - confs = storage.getTags('conference') - for conf in confs: - autojoin_val = conf.getAttr('autojoin') - if not autojoin_val: # not there (it's optional) - autojoin_val = False - - try: - jid = helpers.parse_jid(conf.getAttr('jid')) - except helpers.InvalidFormat: - log.warning('Invalid JID: %s, ignoring it', - conf.getAttr('jid')) - continue - - bookmark = { - 'name': conf.getAttr('name'), - 'password': conf.getTagData('password'), - 'nick': conf.getTagData('nick'), - } - - try: - bookmark['autojoin'] = from_xs_boolean(autojoin_val) - except ValueError as error: - log.warning(error) - continue - - log.debug('Found Bookmark: %s', jid) - bookmarks[jid] = bookmark - - return bookmarks - - @staticmethod - def _build_storage_node(bookmarks): - storage_node = nbxmpp.Node( - tag='storage', attrs={'xmlns': 'storage:bookmarks'}) - for jid, bm in bookmarks.items(): - conf_node = storage_node.addChild(name="conference") - conf_node.setAttr('jid', jid) - conf_node.setAttr('autojoin', to_xs_boolean(bm['autojoin'])) - conf_node.setAttr('name', bm['name']) - # 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): - conf_node.setTagData('nick', bm['nick']) - if bm.get('password', None): - conf_node.setTagData('password', bm['password']) - return storage_node - - def _build_node(self, _data): - pass - - @staticmethod - def get_bookmark_publish_options() -> nbxmpp.Node: - options = nbxmpp.Node(nbxmpp.NS_DATA + ' x', - attrs={'type': 'submit'}) - field = options.addChild('field', - attrs={'var': 'FORM_TYPE', 'type': 'hidden'}) - field.setTagData('value', nbxmpp.NS_PUBSUB_PUBLISH_OPTIONS) - field = options.addChild('field', attrs={'var': 'pubsub#access_model'}) - field.setTagData('value', 'whitelist') - return options - def store_bookmarks(self): if not app.account_is_connected(self._account): return - storage_node = self._build_storage_node(self.bookmarks) + type_ = BookmarkStoreType.PRIVATE if self._pubsub_support() and self.conversion: - self._pubsub_store(storage_node) - else: - self._private_store(storage_node) + type_ = BookmarkStoreType.PUBSUB + + self._nbxmpp('Bookmarks').store_bookmarks(self._bookmarks, type_) app.nec.push_incoming_event( NetworkEvent('bookmarks-received', account=self._account)) - def _pubsub_store(self, storage_node: nbxmpp.Node) -> None: - self._con.get_module('PubSub').send_pb_publish( - '', 'storage:bookmarks', storage_node, 'current', - options=self.get_bookmark_publish_options(), - cb=self._pubsub_store_result) - log.info('Publish Bookmarks (PubSub)') - - def _private_store(self, storage_node: nbxmpp.Node) -> None: - iq = nbxmpp.Iq('set', nbxmpp.NS_PRIVATE, payload=storage_node) - log.info('Publish Bookmarks (PrivateStorage)') - self._con.connection.SendAndCallForResponse( - iq, self._private_store_result) - - @staticmethod - def _pubsub_store_result(_con, stanza): - if not nbxmpp.isResultNode(stanza): - log.error('Error: %s', stanza.getError()) - return - - @staticmethod - def _private_store_result(stanza: nbxmpp.Iq) -> None: - if not nbxmpp.isResultNode(stanza): - log.error('Error: %s', stanza.getError()) - return - - def _join_with_timeout(self, bookmarks: Optional[List[str]] = None) -> None: + def _join_with_timeout(self, bookmarks: List[Any]) -> None: self._join_timeouts.pop(0) self.auto_join_bookmarks(bookmarks) - def auto_join_bookmarks(self, bookmarks: Optional[List[str]] = None) -> None: + def auto_join_bookmarks(self, bookmarks: Optional[List[Any]] = None) -> None: if app.is_invisible(self._account): return - if bookmarks is None: - bookmarks = self.bookmarks.keys() - for jid in bookmarks: - bookmark = self.bookmarks[jid] - if bookmark['autojoin']: + if bookmarks is None: + bookmarks = self._bookmarks + + for bookmark in bookmarks: + if bookmark.autojoin: # Only join non-opened groupchats. Opened one are already # auto-joined on re-connection - if jid not in app.gc_connected[self._account]: + if bookmark.jid not in app.gc_connected[self._account]: # we are not already connected - log.info('Autojoin Bookmark: %s', jid) - minimize = app.config.get_per('rooms', jid, + log.info('Autojoin Bookmark: %s', bookmark.jid) + minimize = app.config.get_per('rooms', bookmark.jid, 'minimize_on_autojoin', True) app.interface.join_gc_room( - self._account, jid, bookmark['nick'], - bookmark['password'], minimize=minimize) + self._account, bookmark.jid, bookmark.nick, + bookmark.password, minimize=minimize) def add_bookmark(self, name, jid, autojoin, password, nick): - self.bookmarks[jid] = { - 'name': name, - 'autojoin': autojoin, - 'password': password, - 'nick': nick, - } - + bookmark = BookmarkData(jid=jid, + name=name, + autojoin=autojoin, + password=password, + nick=nick) + self._bookmarks.append(bookmark) self.store_bookmarks() def remove(self, jid: str, publish: bool = True) -> None: - if self.bookmarks.pop(jid, None) is None: + bookmark = self.get_bookmark_from_jid(jid) + if bookmark is None: return + self.bookmark.remove(bookmark) if publish: self.store_bookmarks() def get_name_from_bookmark(self, jid: str) -> str: fallback = jid.split('@')[0] - try: - return self.bookmarks[jid]['name'] or fallback - except KeyError: + bookmark = self.get_bookmark_from_jid(jid) + if bookmark is None: return fallback + return bookmark.name or fallback def is_bookmark(self, jid: str) -> bool: - return jid in self.bookmarks + return self.get_bookmark_from_jid(jid) is not None def purge_pubsub_bookmarks(self) -> None: log.info('Purge/Delete Bookmarks on PubSub, ' diff --git a/gajim/groupchat_control.py b/gajim/groupchat_control.py index 48a0069ca..3192abbf5 100644 --- a/gajim/groupchat_control.py +++ b/gajim/groupchat_control.py @@ -468,7 +468,7 @@ class GroupchatControl(ChatControlBase): # Bookmarks con = app.connections[self.account] - bookmarked = self.room_jid in con.get_module('Bookmarks').bookmarks + bookmarked = con.get_module('Bookmarks').is_bookmark(self.room_jid) self._get_action('bookmark-').set_enabled(self.is_connected and not bookmarked) diff --git a/gajim/gtk/bookmarks.py b/gajim/gtk/bookmarks.py index 407cc8137..4fb50abf9 100644 --- a/gajim/gtk/bookmarks.py +++ b/gajim/gtk/bookmarks.py @@ -16,6 +16,7 @@ from enum import IntEnum from gi.repository import Gtk from gi.repository import Gdk +from nbxmpp.structs import BookmarkData from gajim.common import app from gajim.common import helpers @@ -57,14 +58,14 @@ class ManageBookmarksWindow: con = app.connections[account] bookmarks = con.get_module('Bookmarks').get_sorted_bookmarks() - for jid, bookmark in bookmarks.items(): + for bookmark in bookmarks: self.treestore.append(iter_, [account, - bookmark['name'], - jid, - bookmark['autojoin'], - bookmark['password'], - bookmark['nick'], - bookmark['name']]) + bookmark.name, + bookmark.jid, + bookmark.autojoin, + bookmark.password, + bookmark.nick, + bookmark.name]) self.view = self.xml.get_object('bookmarks_treeview') self.view.set_model(self.treestore) @@ -190,20 +191,17 @@ class ManageBookmarksWindow: for account in self.treestore: acct = account[1] con = app.connections[acct] - con.get_module('Bookmarks').bookmarks = {} + bookmarks = [] for bm in account.iterchildren(): # create the bookmark-dict - bmdict = { - 'name': bm[Row.ROOM_NAME], - 'autojoin': bm[Row.AUTOJOIN], - 'password': bm[Row.PASSWORD], - 'nick': bm[Row.NICK], - } - - jid = bm[Row.ROOM_JID] - con.get_module('Bookmarks').bookmarks[jid] = bmdict - + bookmark = BookmarkData(jid=bm[Row.ROOM_JID], + name=bm[Row.ROOM_NAME], + autojoin=bm[Row.AUTOJOIN], + password=bm[Row.PASSWORD], + nick=bm[Row.NICK]) + bookmarks.append(bookmark) + con.get_module('Bookmarks').bookmarks = bookmarks con.get_module('Bookmarks').store_bookmarks() self.window.destroy() diff --git a/gajim/gtk/discovery.py b/gajim/gtk/discovery.py index d2c0446f4..72d093d68 100644 --- a/gajim/gtk/discovery.py +++ b/gajim/gtk/discovery.py @@ -1707,7 +1707,7 @@ class MucBrowser(AgentBrowser): return room_jid = model[iter_][0] - if room_jid in con.get_module('Bookmarks').bookmarks: + if con.get_module('Bookmarks').is_bookmark(room_jid): ErrorDialog( _('Bookmark already set'), _('Group Chat "%s" is already in your bookmarks.') % room_jid, diff --git a/gajim/gtk/start_chat.py b/gajim/gtk/start_chat.py index 7016b10e3..49a4aaff9 100644 --- a/gajim/gtk/start_chat.py +++ b/gajim/gtk/start_chat.py @@ -92,8 +92,8 @@ class StartChatDialog(Gtk.ApplicationWindow): con = app.connections[account] bookmarks = con.get_module('Bookmarks').bookmarks groupchats = {} - for jid, bookmark in bookmarks.items(): - groupchats[jid] = bookmark['name'] + for bookmark in bookmarks: + groupchats[bookmark.jid] = bookmark.name for jid in app.contacts.get_gc_list(account): if jid in groupchats: diff --git a/gajim/gui_interface.py b/gajim/gui_interface.py index f8eec07c5..e32ea0d98 100644 --- a/gajim/gui_interface.py +++ b/gajim/gui_interface.py @@ -1552,11 +1552,10 @@ class Interface: return con = app.connections[account] - bookmarks = con.get_module('Bookmarks').bookmarks - bookmark = bookmarks.get(room_jid, None) + bookmark = con.get_module('Bookmarks').get_bookmark_from_jid(room_jid) if bookmark is not None: app.interface.join_gc_room( - account, room_jid, bookmark['nick'], bookmark['password']) + account, bookmark.jid, bookmark.nick, bookmark.password) return try: diff --git a/gajim/gui_menu_builder.py b/gajim/gui_menu_builder.py index ef67df5ff..89c8df691 100644 --- a/gajim/gui_menu_builder.py +++ b/gajim/gui_menu_builder.py @@ -167,14 +167,13 @@ show_bookmarked=False, force_resource=False): r_jids = [] # list of room jids for account in connected_accounts: con = app.connections[account] - boomarks = con.get_module('Bookmarks').bookmarks - for jid in boomarks.keys(): - if jid in r_jids: + for bookmark in con.get_module('Bookmarks').bookmarks: + if bookmark.jid in r_jids: continue - if jid not in app.gc_connected[account] or not \ - app.gc_connected[account][jid]: - rooms2.append((jid, account)) - r_jids.append(jid) + if bookmark.jid not in app.gc_connected[account] or not \ + app.gc_connected[account][bookmark.jid]: + rooms2.append((bookmark.jid, account)) + r_jids.append(bookmark.jid) if not rooms2: return @@ -736,7 +735,7 @@ def get_groupchat_menu(control_id, account, jid): def get_bookmarks_menu(account, rebuild=False): con = app.connections[account] - boomarks = con.get_module('Bookmarks').get_sorted_bookmarks(short_name=True) + bookmarks = con.get_module('Bookmarks').get_sorted_bookmarks(short_name=True) menu = Gio.Menu() @@ -749,19 +748,17 @@ def get_bookmarks_menu(account, rebuild=False): # Build Bookmarks section = Gio.Menu() - for jid, bookmark in boomarks.items(): - name = bookmark['name'] - + for bookmark in bookmarks: action = 'app.{}-activate-bookmark'.format(account) - menuitem = Gio.MenuItem.new(name, action) + menuitem = Gio.MenuItem.new(bookmark.name, action) # Create Variant Dict dict_ = {'account': GLib.Variant('s', account), - 'jid': GLib.Variant('s', jid)} - if bookmark['nick']: - dict_['nick'] = GLib.Variant('s', bookmark['nick']) - if bookmark['password']: - dict_['password'] = GLib.Variant('s', bookmark['password']) + 'jid': GLib.Variant('s', bookmark.jid)} + if bookmark.nick: + dict_['nick'] = GLib.Variant('s', bookmark.nick) + if bookmark.password: + dict_['password'] = GLib.Variant('s', bookmark.password) variant_dict = GLib.Variant('a{sv}', dict_) menuitem.set_action_and_target_value(action, variant_dict) diff --git a/gajim/roster_window.py b/gajim/roster_window.py index 450df2f3f..d6ea092e2 100644 --- a/gajim/roster_window.py +++ b/gajim/roster_window.py @@ -2706,9 +2706,9 @@ class RosterWindow: ### FIXME: order callbacks in itself... ################################################################################ - def on_bookmark_menuitem_activate(self, widget, account, jid, bookmark): + def on_bookmark_menuitem_activate(self, widget, account, bookmark): app.interface.join_gc_room( - account, jid, bookmark['nick'], bookmark['password']) + account, bookmark.jid, bookmark.nick, bookmark.password) def on_info(self, widget, contact, account): """ @@ -5421,15 +5421,13 @@ class RosterWindow: bookmarks = con.get_module('Bookmarks').get_sorted_bookmarks( short_name=True) - for jid, bookmark in bookmarks.items(): - name = bookmark['name'] - + for bookmark in bookmarks: # Do not use underline. - item = Gtk.MenuItem.new_with_label(name) + item = Gtk.MenuItem.new_with_label(bookmark.name) item.set_use_underline(False) item.connect( 'activate', self.on_bookmark_menuitem_activate, - account, jid, bookmark) + account, bookmark) gc_sub_menu.append(item) def show_appropriate_context_menu(self, event, iters):