From 0b5c8a3b46255c23ec2d36347b3a729c91d4c066 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philipp=20H=C3=B6rist?= Date: Sun, 16 Sep 2018 13:10:47 +0200 Subject: [PATCH] Dont retract pep items on UserXEPs This leads to multiple problems 1. We cant assume only items with id='current' are stored in the node which would lead to retracting 'current' but another item would become the last published and sent to users 2. Even if we have a SingletonNode retracting the only item means the Node would be empty and offline clients would not receive the last published item on coming online, because there is no item anymore Instead we always publish an empty item from now on --- gajim/common/const.py | 6 --- gajim/common/modules/pep.py | 61 +++++++++++------------------ gajim/common/modules/user_avatar.py | 4 -- gajim/common/types.py | 3 +- gajim/gtk/profile.py | 8 ++-- gajim/roster_window.py | 8 ++-- 6 files changed, 30 insertions(+), 60 deletions(-) diff --git a/gajim/common/const.py b/gajim/common/const.py index 20641381b..0ce7c118b 100644 --- a/gajim/common/const.py +++ b/gajim/common/const.py @@ -162,12 +162,6 @@ class BookmarkStorageType(IntEnum): PUBSUB = 1 -@unique -class PEPHandlerType(IntEnum): - NOTIFY = 0 - RETRACT = 1 - - @unique class PEPEventType(IntEnum): ABSTRACT = 0 diff --git a/gajim/common/modules/pep.py b/gajim/common/modules/pep.py index 8ee110ebb..9f9d1a333 100644 --- a/gajim/common/modules/pep.py +++ b/gajim/common/modules/pep.py @@ -26,11 +26,10 @@ import nbxmpp from gajim.common import app from gajim.common.exceptions import StanzaMalformed from gajim.common.nec import NetworkIncomingEvent -from gajim.common.const import PEPHandlerType, PEPEventType +from gajim.common.const import PEPEventType from gajim.common.types import ConnectionT from gajim.common.types import PEPHandlersDict # pylint: disable=unused-import from gajim.common.types import PEPNotifyCallback -from gajim.common.types import PEPRetractCallback log = logging.getLogger('gajim.c.m.pep') @@ -64,18 +63,16 @@ class PEP: def register_pep_handler( self, namespace: str, - notify_handler: PEPNotifyCallback, - retract_handler: PEPRetractCallback) -> None: + notify_handler: PEPNotifyCallback) -> None: if namespace in self._pep_handlers: - self._pep_handlers[namespace].append( - (notify_handler, retract_handler)) + self._pep_handlers[namespace].append(notify_handler) else: - self._pep_handlers[namespace] = [(notify_handler, retract_handler)] - if notify_handler: - module_instance = notify_handler.__self__ # type: ignore - if module_instance.store_publish: - if module_instance not in self._store_publish_modules: - self._store_publish_modules.append(module_instance) + self._pep_handlers[namespace] = [notify_handler] + + module_instance = notify_handler.__self__ # type: ignore + if module_instance.store_publish: + if module_instance not in self._store_publish_modules: + self._store_publish_modules.append(module_instance) def _pep_event_received(self, _con: ConnectionT, @@ -105,9 +102,9 @@ class PEP: # Check if this is a retraction retract = items.getTag('retract') if retract is not None: - for handler in handlers: - handler[PEPHandlerType.RETRACT](jid, retract.getAttr('id')) - raise nbxmpp.NodeProcessed + id_ = retract.getAttr('id') + log.info('Received retract of id: %s', id_) + raise nbxmpp.NodeProcessed # Check if we have items items_ = items.getTags('item') @@ -115,7 +112,7 @@ class PEP: log.warning('Malformed PEP event received: %s', stanza) raise nbxmpp.NodeProcessed for handler in handlers: - handler[PEPHandlerType.NOTIFY](jid, items_[0]) + handler(jid, items_[0]) raise nbxmpp.NodeProcessed def send_stored_publish(self) -> None: @@ -165,9 +162,7 @@ class AbstractPEPModule: self._stored_publish = None self._con.get_module('PEP').register_pep_handler( - self.namespace, - self._pep_notify_received, - self._pep_retract_received) + self.namespace, self._pep_notify_received) def _pep_notify_received(self, jid: nbxmpp.JID, item: nbxmpp.Node) -> None: try: @@ -177,11 +172,14 @@ class AbstractPEPModule: return self._log.info('Received: %s %s', jid, data) - self._push_event(jid, self.pep_class(data)) - - def _pep_retract_received(self, jid: nbxmpp.JID, id_: str) -> None: - self._log.info('Retract: %s %s', jid, id_) - self._push_event(jid, self.pep_class(None)) + user_pep = self.pep_class(data) + self._notification_received(jid, user_pep) + app.nec.push_incoming_event( + PEPReceivedEvent(None, + conn=self._con, + jid=str(jid), + pep_type=self.name, + user_pep=user_pep)) def _extract_info(self, item: nbxmpp.Node) -> Any: '''To be implemented by subclasses''' @@ -191,14 +189,6 @@ class AbstractPEPModule: '''To be implemented by subclasses''' raise NotImplementedError - def _push_event(self, jid: nbxmpp.JID, user_pep: Any) -> None: - self._notification_received(jid, user_pep) - app.nec.push_incoming_event( - PEPReceivedEvent(None, conn=self._con, - jid=str(jid), - pep_type=self.name, - user_pep=user_pep)) - def _notification_received(self, jid: nbxmpp.JID, user_pep: Any) -> None: for contact in app.contacts.get_contacts(self._account, str(jid)): if user_pep: @@ -241,13 +231,6 @@ class AbstractPEPModule: self._con.get_module('PubSub').send_pb_publish( '', self.namespace, item, 'current') - def retract(self) -> None: - if not self._con.get_module('PEP').supported: - return - self.send(None) - self._con.get_module('PubSub').send_pb_retract( - '', self.namespace, 'current') - class PEPReceivedEvent(NetworkIncomingEvent): name = 'pep-received' diff --git a/gajim/common/modules/user_avatar.py b/gajim/common/modules/user_avatar.py index 594a5db1f..82d3ba1e4 100644 --- a/gajim/common/modules/user_avatar.py +++ b/gajim/common/modules/user_avatar.py @@ -152,10 +152,6 @@ class UserAvatar(AbstractPEPModule): # Not implemented yet return - def retract(self): - # Not implemented yet - return - def get_instance(*args, **kwargs): return UserAvatar(*args, **kwargs), 'UserAvatar' diff --git a/gajim/common/types.py b/gajim/common/types.py index 408859cb0..08b29eacf 100644 --- a/gajim/common/types.py +++ b/gajim/common/types.py @@ -40,8 +40,7 @@ UserTuneDataT = Optional[Tuple[str, str, str, str, str]] # PEP PEPNotifyCallback = Callable[[nbxmpp.JID, nbxmpp.Node], None] -PEPRetractCallback = Callable[[nbxmpp.JID, str], None] -PEPHandlersDict = Dict[str, List[Tuple[PEPNotifyCallback, PEPRetractCallback]]] +PEPHandlersDict = Dict[str, List[PEPNotifyCallback]] # Configpaths PathTuple = Tuple[Optional[PathLocation], str, Optional[PathType]] diff --git a/gajim/gtk/profile.py b/gajim/gtk/profile.py index 1ce792a77..ca8b34f8c 100644 --- a/gajim/gtk/profile.py +++ b/gajim/gtk/profile.py @@ -331,11 +331,9 @@ class ProfileWindow(Gtk.ApplicationWindow): transient_for=self) return vcard_, sha = self.make_vcard() - nick = vcard_.get('NICKNAME') - if nick: - app.connections[self.account].get_module('UserNickname').send(nick) - else: - app.connections[self.account].get_module('UserNickname').retract() + nick = vcard_.get('NICKNAME') or None + app.connections[self.account].get_module('UserNickname').send(nick) + if not nick: nick = app.config.get_per('accounts', self.account, 'name') app.nicks[self.account] = nick app.connections[self.account].get_module('VCardTemp').send_vcard( diff --git a/gajim/roster_window.py b/gajim/roster_window.py index d1a2abf8a..c3737b39d 100644 --- a/gajim/roster_window.py +++ b/gajim/roster_window.py @@ -2119,14 +2119,14 @@ class RosterWindow: connection.get_module('UserActivity').send( (activity, subactivity, activity_text)) else: - connection.get_module('UserActivity').retract() + connection.get_module('UserActivity').send(None) if 'mood' in pep_dict: mood = pep_dict['mood'] mood_text = pep_dict.get('mood_text', None) connection.get_module('UserMood').send((mood, mood_text)) else: - connection.get_module('UserMood').retract() + connection.get_module('UserMood').send(None) def delete_pep(self, jid, account): if jid == app.get_jid_from_account(account): @@ -3631,7 +3631,7 @@ class RosterWindow: if active: app.interface.enable_music_listener() else: - app.connections[account].get_module('UserTune').retract() + app.connections[account].get_module('UserTune').send(None) # disable music listener only if no other account uses it for acc in app.connections: if app.config.get_per('accounts', acc, 'publish_tune'): @@ -3647,7 +3647,7 @@ class RosterWindow: if active: location_listener.enable() else: - app.connections[account].get_module('UserLocation').retract() + app.connections[account].get_module('UserLocation').send(None) helpers.update_optional_features(account)