Pubsub: Unify pubsub notification parsing

PubSubEventData:

    - The data attribute is now always None if there is no data
    - The empty attribute was removed because the info is available in the data attribute
    - Pass delete, purge, retract always through to the application

11 files changed, 126 insertions, 119 deletions

diff --git a/nbxmpp/modules/activity.py b/nbxmpp/modules/activity.py
index f2143e6..7f48ba1 100644
--- a/nbxmpp/modules/activity.py
+++ b/nbxmpp/modules/activity.py
@@ -46,29 +46,32 @@ class Activity:
             return
 
         item = properties.pubsub_event.item
+        if item is None:
+            # Retract, Deleted or Purged
+            return
 
         activity_node = item.getTag('activity', namespace=NS_ACTIVITY)
         if not activity_node.getChildren():
-            pubsub_event = properties.pubsub_event._replace(empty=True)
             log.info('Received activity: %s - no activity set', properties.jid)
-        else:
-            activity, subactivity, text = None, None, None
-            for child in activity_node.getChildren():
-                name = child.getName()
-                if name == 'text':
-                    text = child.getData()
-                elif name in ACTIVITIES:
-                    activity = name
-                    subactivity = self._parse_sub_activity(child)
-
-            if activity is None and activity_node.getPayload():
-                log.warning('No valid activity value found')
-                log.warning(stanza)
-                raise NodeProcessed
-
-            data = ActivityData(activity, subactivity, text)
-            pubsub_event = properties.pubsub_event._replace(data=data)
-            log.info('Received activity: %s - %s', properties.jid, data)
+            return
+
+        activity, subactivity, text = None, None, None
+        for child in activity_node.getChildren():
+            name = child.getName()
+            if name == 'text':
+                text = child.getData()
+            elif name in ACTIVITIES:
+                activity = name
+                subactivity = self._parse_sub_activity(child)
+
+        if activity is None and activity_node.getPayload():
+            log.warning('No valid activity value found')
+            log.warning(stanza)
+            raise NodeProcessed
+
+        data = ActivityData(activity, subactivity, text)
+        pubsub_event = properties.pubsub_event._replace(data=data)
+        log.info('Received activity: %s - %s', properties.jid, data)
 
         properties.pubsub_event = pubsub_event
 
diff --git a/nbxmpp/modules/bookmarks.py b/nbxmpp/modules/bookmarks.py
index 30fb9a2..506616d 100644
--- a/nbxmpp/modules/bookmarks.py
+++ b/nbxmpp/modules/bookmarks.py
@@ -25,6 +25,7 @@ from nbxmpp.protocol import isResultNode
 from nbxmpp.protocol import Node
 from nbxmpp.protocol import Iq
 from nbxmpp.protocol import JID
+from nbxmpp.protocol import NodeProcessed
 from nbxmpp.structs import StanzaHandler
 from nbxmpp.structs import BookmarkData
 from nbxmpp.const import BookmarkStoreType
@@ -77,27 +78,28 @@ class Bookmarks:
         self._node_configuration_in_progress = False
         self._node_configuration_not_possible = False
 
-    def _process_pubsub_bookmarks(self, _con, _stanza, properties):
+    def _process_pubsub_bookmarks(self, _con, stanza, properties):
         if not properties.is_pubsub_event:
             return
 
         if properties.pubsub_event.node != NS_BOOKMARKS:
             return
 
-        if properties.pubsub_event.deleted or properties.pubsub_event.retracted:
-            return
-
         item = properties.pubsub_event.item
         if item is None:
+            # Retract, Deleted or Purged
             return
 
         storage_node = item.getTag('storage', namespace=NS_BOOKMARKS)
         if storage_node is None:
-            return
+            log.warning('No storage node found')
+            log.warning(stanza)
+            raise NodeProcessed
 
-        bookmarks = []
-        if storage_node.getChildren():
-            bookmarks = self._parse_bookmarks(storage_node)
+        bookmarks = self._parse_bookmarks(storage_node)
+        if not bookmarks:
+            log.info('Bookmarks removed')
+            return
 
         pubsub_event = properties.pubsub_event._replace(data=bookmarks)
         log.info('Received bookmarks from: %s', properties.jid)
@@ -113,16 +115,14 @@ class Bookmarks:
         if properties.pubsub_event.node != NS_BOOKMARKS_2:
             return
 
-        if properties.pubsub_event.deleted or properties.pubsub_event.retracted:
-            return
-
         item = properties.pubsub_event.item
         if item is None:
+            # Retract, Deleted or Purged
             return
 
         bookmark_item = self._parse_bookmarks2(item)
         if bookmark_item is None:
-            return
+            raise NodeProcessed
 
         pubsub_event = properties.pubsub_event._replace(data=bookmark_item)
         log.info('Received bookmark item from: %s', properties.jid)
@@ -183,7 +183,7 @@ class Bookmarks:
             log.warning(item)
             return
 
-        autojoin = conference.getAttr('autojoin') == 'true'
+        autojoin = conference.getAttr('autojoin') in ('True', 'true', '1')
         name = conference.getAttr('name')
         nick = conference.getTagData('nick')
 
diff --git a/nbxmpp/modules/location.py b/nbxmpp/modules/location.py
index 0d87775..5fe2252 100644
--- a/nbxmpp/modules/location.py
+++ b/nbxmpp/modules/location.py
@@ -45,18 +45,21 @@ class Location:
             return
 
         item = properties.pubsub_event.item
+        if item is None:
+            # Retract, Deleted or Purged
+            return
 
         location_node = item.getTag('geoloc', namespace=NS_LOCATION)
         if not location_node.getChildren():
-            pubsub_event = properties.pubsub_event._replace(empty=True)
             log.info('Received location: %s - no location set', properties.jid)
-        else:
-            location_dict = {}
-            for node in LOCATION_DATA:
-                location_dict[node] = location_node.getTagData(node)
-            data = LocationData(**location_dict)
-            pubsub_event = properties.pubsub_event._replace(data=data)
-            log.info('Received location: %s - %s', properties.jid, data)
+            return
+
+        location_dict = {}
+        for node in LOCATION_DATA:
+            location_dict[node] = location_node.getTagData(node)
+        data = LocationData(**location_dict)
+        pubsub_event = properties.pubsub_event._replace(data=data)
+        log.info('Received location: %s - %s', properties.jid, data)
 
         properties.pubsub_event = pubsub_event
 
diff --git a/nbxmpp/modules/mood.py b/nbxmpp/modules/mood.py
index e414430..00cd755 100644
--- a/nbxmpp/modules/mood.py
+++ b/nbxmpp/modules/mood.py
@@ -46,28 +46,31 @@ class Mood:
             return
 
         item = properties.pubsub_event.item
+        if item is None:
+            # Retract, Deleted or Purged
+            return
 
         mood_node = item.getTag('mood', namespace=NS_MOOD)
         if not mood_node.getChildren():
-            pubsub_event = properties.pubsub_event._replace(empty=True)
-            log.info('Received mood: %s - no mood set', properties.jid)
-        else:
-            mood, text = None, None
-            for child in mood_node.getChildren():
-                name = child.getName().strip()
-                if name == 'text':
-                    text = child.getData()
-                elif name in MOODS:
-                    mood = name
-
-            if mood is None and mood_node.getPayload():
-                log.warning('No valid mood value found')
-                log.warning(stanza)
-                raise NodeProcessed
-
-            data = MoodData(mood, text)
-            pubsub_event = properties.pubsub_event._replace(data=data)
-            log.info('Received mood: %s - %s', properties.jid, data)
+            log.info('Received mood: %s - removed mood', properties.jid)
+            return
+
+        mood, text = None, None
+        for child in mood_node.getChildren():
+            name = child.getName().strip()
+            if name == 'text':
+                text = child.getData()
+            elif name in MOODS:
+                mood = name
+
+        if mood is None and mood_node.getPayload():
+            log.warning('No valid mood value found')
+            log.warning(stanza)
+            raise NodeProcessed
+
+        data = MoodData(mood, text)
+        pubsub_event = properties.pubsub_event._replace(data=data)
+        log.info('Received mood: %s - %s', properties.jid, data)
 
         properties.pubsub_event = pubsub_event
 
diff --git a/nbxmpp/modules/nickname.py b/nbxmpp/modules/nickname.py
index e6422bc..0262a64 100644
--- a/nbxmpp/modules/nickname.py
+++ b/nbxmpp/modules/nickname.py
@@ -64,12 +64,17 @@ class Nickname:
         if properties.pubsub_event.node != NS_NICK:
             return
 
-        nick = self._parse_nickname(properties.pubsub_event.item)
+        item = properties.pubsub_event.item
+        if item is None:
+            # Retract, Deleted or Purged
+            return
+
+        nick = self._parse_nickname(item)
         if nick is None:
-            log.info('Received nickname: %s - no nickname set', properties.jid)
-        else:
-            log.info('Received nickname: %s - %s', properties.jid, nick)
+            log.info('Received nickname: %s - nickname removed', properties.jid)
+            return
 
+        log.info('Received nickname: %s - %s', properties.jid, nick)
         properties.pubsub_event = properties.pubsub_event._replace(data=nick)
 
     @staticmethod
diff --git a/nbxmpp/modules/omemo.py b/nbxmpp/modules/omemo.py
index b184db9..596d208 100644
--- a/nbxmpp/modules/omemo.py
+++ b/nbxmpp/modules/omemo.py
@@ -133,15 +133,11 @@ class OMEMO:
         if properties.pubsub_event.node != NS_OMEMO_TEMP_DL:
             return
 
-        if properties.pubsub_event.retracted:
-            # Retracts should not happen and its unclear how we should react
-            raise NodeProcessed
-
-        if properties.pubsub_event.deleted:
-            log.info('Devicelist node deleted by %s', properties.jid)
+        item = properties.pubsub_event.item
+        if item is None:
+            # Retract, Deleted or Purged
             return
 
-        item = properties.pubsub_event.item
         try:
             devices = self._parse_devicelist(item)
         except StanzaMalformed as error:
@@ -150,13 +146,13 @@ class OMEMO:
             raise NodeProcessed
 
         if not devices:
-            pubsub_event = properties.pubsub_event._replace(empty=True)
             log.info('Received OMEMO devicelist: %s - no devices set',
                      properties.jid)
-        else:
-            pubsub_event = properties.pubsub_event._replace(data=devices)
-            log.info('Received OMEMO devicelist: %s - %s',
-                     properties.jid, devices)
+            return
+
+        pubsub_event = properties.pubsub_event._replace(data=devices)
+        log.info('Received OMEMO devicelist: %s - %s',
+                 properties.jid, devices)
 
         properties.pubsub_event = pubsub_event
 
@@ -172,15 +168,12 @@ class OMEMO:
           </item>
         </items>
         '''
-        if item is None:
-            return []
-
         list_node = item.getTag('list', namespace=NS_OMEMO_TEMP)
         if list_node is None:
             raise StanzaMalformed('No list node found')
 
         if not list_node.getChildren():
-            return []
+            return None
 
         result = []
         devices_nodes = list_node.getChildren()
diff --git a/nbxmpp/modules/openpgp.py b/nbxmpp/modules/openpgp.py
index db5c677..8079098 100644
--- a/nbxmpp/modules/openpgp.py
+++ b/nbxmpp/modules/openpgp.py
@@ -101,15 +101,11 @@ class OpenPGP:
             </public-keys-list>
           </item>
         '''
-        if properties.pubsub_event.retracted:
-            # Retracts should not happen and its unclear how we should react
-            raise NodeProcessed
-
-        if properties.pubsub_event.deleted:
-            log.info('Keylist node deleted by %s', properties.jid)
+        item = properties.pubsub_event.item
+        if item is None:
+            # Retract, Deleted or Purged
             return
 
-        item = properties.pubsub_event.item
         try:
             data = self._parse_keylist(properties.jid, item)
         except StanzaMalformed as error:
@@ -118,26 +114,23 @@ class OpenPGP:
             raise NodeProcessed
 
         if data is None:
-            pubsub_event = properties.pubsub_event._replace(empty=True)
             log.info('Received PGP keylist: %s - no keys set', properties.jid)
-        else:
-            pubsub_event = properties.pubsub_event._replace(data=data)
-            log.info('Received PGP keylist: %s - %s', properties.jid, data)
+            return
+
+        pubsub_event = properties.pubsub_event._replace(data=data)
+        log.info('Received PGP keylist: %s - %s', properties.jid, data)
 
         properties.pubsub_event = pubsub_event
 
     @staticmethod
     def _parse_keylist(jid, item):
-        if item is None:
-            return []
-
         keylist_node = item.getTag('public-keys-list', namespace=NS_OPENPGP)
         if keylist_node is None:
             raise StanzaMalformed('No public-keys-list node found')
 
         metadata = keylist_node.getTags('pubkey-metadata')
         if not metadata:
-            return []
+            return None
 
         data = []
         for key in metadata:
diff --git a/nbxmpp/modules/pubsub.py b/nbxmpp/modules/pubsub.py
index b1f9edd..2f8d1a2 100644
--- a/nbxmpp/modules/pubsub.py
+++ b/nbxmpp/modules/pubsub.py
@@ -58,13 +58,12 @@ class PubSub:
         if delete is not None:
             node = delete.getAttr('node')
             properties.pubsub_event = PubSubEventData(
-                node, empty=True, deleted=True)
+                node, deleted=True)
             return
 
         purge = event.getTag('purge')
         if purge is not None:
             node = purge.getAttr('node')
-            item = purge.getTag('item')
             properties.pubsub_event = PubSubEventData(node, purged=True)
             return
 
@@ -80,12 +79,14 @@ class PubSub:
                 return
 
             if len(items.getChildren()) != 1:
-                log.warning('PubSub event with more than one item')
+                log.warning('PubSub event with != 1 item')
                 log.warning(stanza)
                 return
 
             item = items.getTag('item')
             if item is None:
+                log.warning('No item node found')
+                log.warning(stanza)
                 return
             id_ = item.getAttr('id')
             properties.pubsub_event = PubSubEventData(node, id_, item)
diff --git a/nbxmpp/modules/tune.py b/nbxmpp/modules/tune.py
index 0a1abfc..6ce4b0f 100644
--- a/nbxmpp/modules/tune.py
+++ b/nbxmpp/modules/tune.py
@@ -45,19 +45,22 @@ class Tune:
             return
 
         item = properties.pubsub_event.item
+        if item is None:
+            # Retract, Deleted or Purged
+            return
 
         tune_node = item.getTag('tune', namespace=NS_TUNE)
         if not tune_node.getChildren():
-            pubsub_event = properties.pubsub_event._replace(empty=True)
             log.info('Received tune: %s - no tune set', properties.jid)
-        else:
-            tune_dict = {}
-            for attr in TUNE_DATA:
-                tune_dict[attr] = tune_node.getTagData(attr)
+            return
+
+        tune_dict = {}
+        for attr in TUNE_DATA:
+            tune_dict[attr] = tune_node.getTagData(attr)
 
-            data = TuneData(**tune_dict)
-            pubsub_event = properties.pubsub_event._replace(data=data)
-            log.info('Received tune: %s - %s', properties.jid, data)
+        data = TuneData(**tune_dict)
+        pubsub_event = properties.pubsub_event._replace(data=data)
+        log.info('Received tune: %s - %s', properties.jid, data)
 
         properties.pubsub_event = pubsub_event
 
diff --git a/nbxmpp/modules/user_avatar.py b/nbxmpp/modules/user_avatar.py
index 80b86c9..fb79c96 100644
--- a/nbxmpp/modules/user_avatar.py
+++ b/nbxmpp/modules/user_avatar.py
@@ -53,6 +53,9 @@ class UserAvatar:
             return
 
         item = properties.pubsub_event.item
+        if item is None:
+            # Retract, Deleted or Purged
+            return
 
         metadata = item.getTag('metadata', namespace=NS_AVATAR_METADATA)
         if metadata is None:
@@ -61,20 +64,20 @@ class UserAvatar:
             raise NodeProcessed
 
         if not metadata.getChildren():
-            pubsub_event = properties.pubsub_event._replace(empty=True)
             log.info('Received avatar metadata: %s - no avatar set',
                      properties.jid)
-        else:
-            info = metadata.getTags('info', one=True)
-            try:
-                data = AvatarMetaData(**info.getAttrs())
-            except Exception:
-                log.warning('Malformed user avatar data')
-                log.warning(stanza)
-                raise NodeProcessed
-
-            pubsub_event = properties.pubsub_event._replace(data=data)
-            log.info('Received avatar metadata: %s - %s', properties.jid, data)
+            return
+
+        info = metadata.getTags('info', one=True)
+        try:
+            data = AvatarMetaData(**info.getAttrs())
+        except Exception:
+            log.warning('Malformed user avatar data')
+            log.warning(stanza)
+            raise NodeProcessed
+
+        pubsub_event = properties.pubsub_event._replace(data=data)
+        log.info('Received avatar metadata: %s - %s', properties.jid, data)
 
         properties.pubsub_event = pubsub_event
 
diff --git a/nbxmpp/structs.py b/nbxmpp/structs.py
index 52691c3..347d4b1 100644
--- a/nbxmpp/structs.py
+++ b/nbxmpp/structs.py
@@ -73,8 +73,8 @@ HTTPAuthData.__new__.__defaults__ = (None, None, None, None)
 StanzaIDData = namedtuple('StanzaIDData', 'id by')
 StanzaIDData.__new__.__defaults__ = (None, None)
 
-PubSubEventData = namedtuple('PubSubEventData', 'node id item data empty deleted retracted purged')
-PubSubEventData.__new__.__defaults__ = (None, None, None, False, False, False, False)
+PubSubEventData = namedtuple('PubSubEventData', 'node id item data deleted retracted purged')
+PubSubEventData.__new__.__defaults__ = (None, None, None, False, False, False)
 
 PubSubConfigResult = namedtuple('PubSubConfigResult', 'jid node form')