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
This commit is contained in:
Philipp Hörist 2018-09-16 13:10:47 +02:00 committed by Philipp Hörist
parent f00d8087ad
commit 0b5c8a3b46
6 changed files with 30 additions and 60 deletions

View File

@ -162,12 +162,6 @@ class BookmarkStorageType(IntEnum):
PUBSUB = 1 PUBSUB = 1
@unique
class PEPHandlerType(IntEnum):
NOTIFY = 0
RETRACT = 1
@unique @unique
class PEPEventType(IntEnum): class PEPEventType(IntEnum):
ABSTRACT = 0 ABSTRACT = 0

View File

@ -26,11 +26,10 @@ import nbxmpp
from gajim.common import app from gajim.common import app
from gajim.common.exceptions import StanzaMalformed from gajim.common.exceptions import StanzaMalformed
from gajim.common.nec import NetworkIncomingEvent 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 ConnectionT
from gajim.common.types import PEPHandlersDict # pylint: disable=unused-import from gajim.common.types import PEPHandlersDict # pylint: disable=unused-import
from gajim.common.types import PEPNotifyCallback from gajim.common.types import PEPNotifyCallback
from gajim.common.types import PEPRetractCallback
log = logging.getLogger('gajim.c.m.pep') log = logging.getLogger('gajim.c.m.pep')
@ -64,18 +63,16 @@ class PEP:
def register_pep_handler( def register_pep_handler(
self, self,
namespace: str, namespace: str,
notify_handler: PEPNotifyCallback, notify_handler: PEPNotifyCallback) -> None:
retract_handler: PEPRetractCallback) -> None:
if namespace in self._pep_handlers: if namespace in self._pep_handlers:
self._pep_handlers[namespace].append( self._pep_handlers[namespace].append(notify_handler)
(notify_handler, retract_handler))
else: else:
self._pep_handlers[namespace] = [(notify_handler, retract_handler)] self._pep_handlers[namespace] = [notify_handler]
if notify_handler:
module_instance = notify_handler.__self__ # type: ignore module_instance = notify_handler.__self__ # type: ignore
if module_instance.store_publish: if module_instance.store_publish:
if module_instance not in self._store_publish_modules: if module_instance not in self._store_publish_modules:
self._store_publish_modules.append(module_instance) self._store_publish_modules.append(module_instance)
def _pep_event_received(self, def _pep_event_received(self,
_con: ConnectionT, _con: ConnectionT,
@ -105,9 +102,9 @@ class PEP:
# Check if this is a retraction # Check if this is a retraction
retract = items.getTag('retract') retract = items.getTag('retract')
if retract is not None: if retract is not None:
for handler in handlers: id_ = retract.getAttr('id')
handler[PEPHandlerType.RETRACT](jid, retract.getAttr('id')) log.info('Received retract of id: %s', id_)
raise nbxmpp.NodeProcessed raise nbxmpp.NodeProcessed
# Check if we have items # Check if we have items
items_ = items.getTags('item') items_ = items.getTags('item')
@ -115,7 +112,7 @@ class PEP:
log.warning('Malformed PEP event received: %s', stanza) log.warning('Malformed PEP event received: %s', stanza)
raise nbxmpp.NodeProcessed raise nbxmpp.NodeProcessed
for handler in handlers: for handler in handlers:
handler[PEPHandlerType.NOTIFY](jid, items_[0]) handler(jid, items_[0])
raise nbxmpp.NodeProcessed raise nbxmpp.NodeProcessed
def send_stored_publish(self) -> None: def send_stored_publish(self) -> None:
@ -165,9 +162,7 @@ class AbstractPEPModule:
self._stored_publish = None self._stored_publish = None
self._con.get_module('PEP').register_pep_handler( self._con.get_module('PEP').register_pep_handler(
self.namespace, self.namespace, self._pep_notify_received)
self._pep_notify_received,
self._pep_retract_received)
def _pep_notify_received(self, jid: nbxmpp.JID, item: nbxmpp.Node) -> None: def _pep_notify_received(self, jid: nbxmpp.JID, item: nbxmpp.Node) -> None:
try: try:
@ -177,11 +172,14 @@ class AbstractPEPModule:
return return
self._log.info('Received: %s %s', jid, data) self._log.info('Received: %s %s', jid, data)
self._push_event(jid, self.pep_class(data)) user_pep = self.pep_class(data)
self._notification_received(jid, user_pep)
def _pep_retract_received(self, jid: nbxmpp.JID, id_: str) -> None: app.nec.push_incoming_event(
self._log.info('Retract: %s %s', jid, id_) PEPReceivedEvent(None,
self._push_event(jid, self.pep_class(None)) conn=self._con,
jid=str(jid),
pep_type=self.name,
user_pep=user_pep))
def _extract_info(self, item: nbxmpp.Node) -> Any: def _extract_info(self, item: nbxmpp.Node) -> Any:
'''To be implemented by subclasses''' '''To be implemented by subclasses'''
@ -191,14 +189,6 @@ class AbstractPEPModule:
'''To be implemented by subclasses''' '''To be implemented by subclasses'''
raise NotImplementedError 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: def _notification_received(self, jid: nbxmpp.JID, user_pep: Any) -> None:
for contact in app.contacts.get_contacts(self._account, str(jid)): for contact in app.contacts.get_contacts(self._account, str(jid)):
if user_pep: if user_pep:
@ -241,13 +231,6 @@ class AbstractPEPModule:
self._con.get_module('PubSub').send_pb_publish( self._con.get_module('PubSub').send_pb_publish(
'', self.namespace, item, 'current') '', 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): class PEPReceivedEvent(NetworkIncomingEvent):
name = 'pep-received' name = 'pep-received'

View File

@ -152,10 +152,6 @@ class UserAvatar(AbstractPEPModule):
# Not implemented yet # Not implemented yet
return return
def retract(self):
# Not implemented yet
return
def get_instance(*args, **kwargs): def get_instance(*args, **kwargs):
return UserAvatar(*args, **kwargs), 'UserAvatar' return UserAvatar(*args, **kwargs), 'UserAvatar'

View File

@ -40,8 +40,7 @@ UserTuneDataT = Optional[Tuple[str, str, str, str, str]]
# PEP # PEP
PEPNotifyCallback = Callable[[nbxmpp.JID, nbxmpp.Node], None] PEPNotifyCallback = Callable[[nbxmpp.JID, nbxmpp.Node], None]
PEPRetractCallback = Callable[[nbxmpp.JID, str], None] PEPHandlersDict = Dict[str, List[PEPNotifyCallback]]
PEPHandlersDict = Dict[str, List[Tuple[PEPNotifyCallback, PEPRetractCallback]]]
# Configpaths # Configpaths
PathTuple = Tuple[Optional[PathLocation], str, Optional[PathType]] PathTuple = Tuple[Optional[PathLocation], str, Optional[PathType]]

View File

@ -331,11 +331,9 @@ class ProfileWindow(Gtk.ApplicationWindow):
transient_for=self) transient_for=self)
return return
vcard_, sha = self.make_vcard() vcard_, sha = self.make_vcard()
nick = vcard_.get('NICKNAME') nick = vcard_.get('NICKNAME') or None
if nick: app.connections[self.account].get_module('UserNickname').send(nick)
app.connections[self.account].get_module('UserNickname').send(nick) if not nick:
else:
app.connections[self.account].get_module('UserNickname').retract()
nick = app.config.get_per('accounts', self.account, 'name') nick = app.config.get_per('accounts', self.account, 'name')
app.nicks[self.account] = nick app.nicks[self.account] = nick
app.connections[self.account].get_module('VCardTemp').send_vcard( app.connections[self.account].get_module('VCardTemp').send_vcard(

View File

@ -2119,14 +2119,14 @@ class RosterWindow:
connection.get_module('UserActivity').send( connection.get_module('UserActivity').send(
(activity, subactivity, activity_text)) (activity, subactivity, activity_text))
else: else:
connection.get_module('UserActivity').retract() connection.get_module('UserActivity').send(None)
if 'mood' in pep_dict: if 'mood' in pep_dict:
mood = pep_dict['mood'] mood = pep_dict['mood']
mood_text = pep_dict.get('mood_text', None) mood_text = pep_dict.get('mood_text', None)
connection.get_module('UserMood').send((mood, mood_text)) connection.get_module('UserMood').send((mood, mood_text))
else: else:
connection.get_module('UserMood').retract() connection.get_module('UserMood').send(None)
def delete_pep(self, jid, account): def delete_pep(self, jid, account):
if jid == app.get_jid_from_account(account): if jid == app.get_jid_from_account(account):
@ -3631,7 +3631,7 @@ class RosterWindow:
if active: if active:
app.interface.enable_music_listener() app.interface.enable_music_listener()
else: 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 # disable music listener only if no other account uses it
for acc in app.connections: for acc in app.connections:
if app.config.get_per('accounts', acc, 'publish_tune'): if app.config.get_per('accounts', acc, 'publish_tune'):
@ -3647,7 +3647,7 @@ class RosterWindow:
if active: if active:
location_listener.enable() location_listener.enable()
else: else:
app.connections[account].get_module('UserLocation').retract() app.connections[account].get_module('UserLocation').send(None)
helpers.update_optional_features(account) helpers.update_optional_features(account)