PubSubEvent: Split up Items type into Items and virtual Retract type

This makes the handling much easier because you don't have to manually
check whether the set 'Items' type means items have been published or
retracted. There is no retract type in the XML representation, but that
doesn't mean that we cannot distinguish both cases.

Closes #521.

4 files changed, 44 insertions, 24 deletions

diff --git a/src/base/QXmppPubSubEvent.cpp b/src/base/QXmppPubSubEvent.cpp
index 97c5cf61..61eb8ce8 100644
--- a/src/base/QXmppPubSubEvent.cpp
+++ b/src/base/QXmppPubSubEvent.cpp
@@ -50,6 +50,7 @@ static const QStringList PUBSUB_EVENTS = {
     QStringLiteral("configuration"),
     QStringLiteral("delete"),
     QStringLiteral("items"),
+    QStringLiteral("items"),  // virtual retract type
     QStringLiteral("purge"),
     QStringLiteral("subscription"),
 };
@@ -248,6 +249,7 @@ bool QXmppPubSubEventBase::isPubSubEvent(const QDomElement &stanza, std::functio
     switch (eventType) {
     case Delete:
     case Items:
+    case Retract:
     case Purge:
         if (!eventTypeElement.hasAttribute(QStringLiteral("node"))) {
             return false;
@@ -267,7 +269,8 @@ bool QXmppPubSubEventBase::isPubSubEvent(const QDomElement &stanza, std::functio
         }
         break;
     }
-    case Items: {
+    case Items:
+    case Retract: {
         // check validity of the items using isItemValid()
         for (auto itemElement = eventTypeElement.firstChildElement(QStringLiteral("item"));
              !itemElement.isNull();
@@ -305,11 +308,23 @@ bool QXmppPubSubEventBase::parseExtension(const QDomElement &eventElement, QXmpp
             return false;
         }
 
+        // Detect our virtual retract event type
+        if (d->eventType == Items) {
+            auto child = eventTypeElement.firstChildElement();
+            if (!child.isNull()) {
+                if (child.tagName() == QStringLiteral("retract")) {
+                    d->eventType = Retract;
+                }
+            }
+            // Don't support mixed retract/item events.
+        }
+
         // parse "node" attribute
         switch (d->eventType) {
         case Configuration:
         case Delete:
         case Items:
+        case Retract:
         case Purge:
             d->node = eventTypeElement.attribute(QStringLiteral("node"));
             break;
@@ -328,7 +343,8 @@ bool QXmppPubSubEventBase::parseExtension(const QDomElement &eventElement, QXmpp
         case Items:
             // parse items
             parseItems(eventTypeElement);
-
+            break;
+        case Retract:
             // parse retract ids
             for (auto retract = eventTypeElement.firstChildElement(QStringLiteral("retract"));
                  !retract.isNull();
@@ -381,6 +397,7 @@ void QXmppPubSubEventBase::serializeExtensions(QXmlStreamWriter *writer, QXmpp::
         switch (d->eventType) {
         case Delete:
         case Items:
+        case Retract:
         case Purge:
             // node attribute is required
             writer->writeAttribute(QStringLiteral("node"), d->node);
@@ -409,6 +426,8 @@ void QXmppPubSubEventBase::serializeExtensions(QXmlStreamWriter *writer, QXmpp::
             // serialize items
             serializeItems(writer);
 
+            break;
+        case Retract:
             // serialize retract ids
             for (const auto &id : std::as_const(d->retractIds)) {
                 writer->writeStartElement(QStringLiteral("retract"));
diff --git a/src/base/QXmppPubSubEvent.h b/src/base/QXmppPubSubEvent.h
index eca43fb6..0faf62b6 100644
--- a/src/base/QXmppPubSubEvent.h
+++ b/src/base/QXmppPubSubEvent.h
@@ -27,6 +27,7 @@ public:
         Configuration,
         Delete,
         Items,
+        Retract,
         Purge,
         Subscription,
     };
diff --git a/src/client/QXmppPep_p.h b/src/client/QXmppPep_p.h
index c2891979..6ffac236 100644
--- a/src/client/QXmppPep_p.h
+++ b/src/client/QXmppPep_p.h
@@ -46,6 +46,9 @@ inline bool handlePubSubEvent(const QDomElement &element, const QString &pubSubS
                 (manager->*itemReceived)(pubSubService, {});
             }
             return true;
+        } else if (event.eventType() == QXmppPubSubEventBase::Retract) {
+            (manager->*itemReceived)(pubSubService, {});
+            return true;
         }
     }
     return false;
diff --git a/src/omemo/QXmppOmemoManager.cpp b/src/omemo/QXmppOmemoManager.cpp
index 1f26ee16..856e00a5 100644
--- a/src/omemo/QXmppOmemoManager.cpp
+++ b/src/omemo/QXmppOmemoManager.cpp
@@ -1259,35 +1259,32 @@ bool Manager::handlePubSubEvent(const QDomElement &element, const QString &pubSu
         event.parse(element);
 
         switch (event.eventType()) {
-        // Items are published or deleted.
+        // Items have been published.
         case QXmppPubSubEventBase::Items: {
-            // If there are IDs of deleted items, check for an inconsistency.
-            // Otherwise, check for published items.
-            if (const auto retractIds = event.retractIds(); !retractIds.isEmpty()) {
-                // Specific items are deleted.
-                const auto &retractedItem = event.retractIds().constFirst();
-                if (retractedItem == QXmppPubSubManager::standardItemIdToString(QXmppPubSubManager::Current)) {
+            const auto items = event.items();
+
+            // Only process items if the event notification contains one.
+            // That is necessary because PubSub allows publishing without
+            // items leading to notification-only events.
+            if (!items.isEmpty()) {
+                const auto &deviceListItem = items.constFirst();
+                if (deviceListItem.id() == QXmppPubSubManager::standardItemIdToString(QXmppPubSubManager::Current)) {
+                    d->updateDevices(pubSubService, event.items().constFirst());
+                } else {
                     d->handleIrregularDeviceListChanges(pubSubService);
                 }
-            } else {
-                const auto items = event.items();
-
-                // Only process items if the event notification contains one.
-                // That is necessary because PubSub allows publishing without
-                // items leading to notification-only events.
-                if (!items.isEmpty()) {
-                    const auto &deviceListItem = items.constFirst();
-                    if (deviceListItem.id() == QXmppPubSubManager::standardItemIdToString(QXmppPubSubManager::Current)) {
-                        d->updateDevices(pubSubService, event.items().constFirst());
-                    } else {
-                        d->handleIrregularDeviceListChanges(pubSubService);
-                    }
-                }
             }
 
             break;
         }
-
+        // Items have been retracted.
+        case QXmppPubSubEventBase::Retract: {
+            // Specific items are deleted.
+            const auto &retractedItem = event.retractIds().constFirst();
+            if (retractedItem == QXmppPubSubManager::standardItemIdToString(QXmppPubSubManager::Current)) {
+                d->handleIrregularDeviceListChanges(pubSubService);
+            }
+        }
         // All items are deleted.
         case QXmppPubSubEventBase::Purge:
         // The whole node is deleted.