[Git][gajim/gajim][master] Dont retract pep items on UserXEPs

Philipp Hörist pushed to branch master at gajim / gajim

Commits: 7018d5be by Philipp Hörist at 2018-09-16T11:35:55Z 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

- - - - -

6 changed files:

- gajim/common/const.py - gajim/common/modules/pep.py - gajim/common/modules/user_avatar.py - gajim/common/types.py - gajim/gtk/profile.py - gajim/roster_window.py

Changes:

===================================== gajim/common/const.py ===================================== --- a/gajim/common/const.py +++ b/gajim/common/const.py @@ -163,12 +163,6 @@ class BookmarkStorageType(IntEnum):

@unique -class PEPHandlerType(IntEnum): - NOTIFY = 0 - RETRACT = 1 - - -@unique class PEPEventType(IntEnum): ABSTRACT = 0 ACTIVITY = 1

===================================== gajim/common/modules/pep.py ===================================== --- 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'

===================================== gajim/common/modules/user_avatar.py ===================================== --- 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'

===================================== gajim/common/types.py ===================================== --- 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]]

===================================== gajim/gtk/profile.py ===================================== --- 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(

===================================== gajim/roster_window.py ===================================== --- 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)

View it on GitLab: https://dev.gajim.org/gajim/gajim/commit/7018d5be0ee93eeb25b1bf45d91fd4507d4...