From 9b1c1ec6922047fa30c19c0744619dc458a12b78 Mon Sep 17 00:00:00 2001
From: Tobias Markmann <tm@ayena.de>
Date: Wed, 11 May 2016 17:38:57 +0200
Subject: Fix handling of notify attribute in PubSub retract elements

Test-Information:

Added serializer and parser unit tests. All tests pass on
Mac OS X 10.11.4.

Change-Id: I8550c76ba182a67613d55634c72c0f2979f8b80a

diff --git a/Sluift/ElementConvertors/PubSubRetractConvertor.cpp b/Sluift/ElementConvertors/PubSubRetractConvertor.cpp
index 6597f0b..38e15a1 100644
--- a/Sluift/ElementConvertors/PubSubRetractConvertor.cpp
+++ b/Sluift/ElementConvertors/PubSubRetractConvertor.cpp
@@ -75,8 +75,10 @@ void PubSubRetractConvertor::doConvertToLua(lua_State* L, std::shared_ptr<PubSub
         }
         lua_setfield(L, -2, "items");
     }
-    lua_pushboolean(L, payload->isNotify());
-    lua_setfield(L, -2, "notify");
+    if (payload->isNotify().is_initialized()) {
+        lua_pushboolean(L, payload->isNotify().get_value_or(false));
+        lua_setfield(L, -2, "notify");
+    }
 }
 
 boost::optional<LuaElementConvertor::Documentation> PubSubRetractConvertor::getDocumentation() const {
diff --git a/Swiften/Elements/PubSubRetract.cpp b/Swiften/Elements/PubSubRetract.cpp
index 90a63e6..1b188ee 100644
--- a/Swiften/Elements/PubSubRetract.cpp
+++ b/Swiften/Elements/PubSubRetract.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013 Isode Limited.
+ * Copyright (c) 2013-2016 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
@@ -8,7 +8,7 @@
 
 using namespace Swift;
 
-PubSubRetract::PubSubRetract() : notify(false) {
+PubSubRetract::PubSubRetract() {
 }
 
 PubSubRetract::~PubSubRetract() {
diff --git a/Swiften/Elements/PubSubRetract.h b/Swiften/Elements/PubSubRetract.h
index 0d30c31..2629c16 100644
--- a/Swiften/Elements/PubSubRetract.h
+++ b/Swiften/Elements/PubSubRetract.h
@@ -10,8 +10,9 @@
 #include <string>
 #include <vector>
 
+#include <boost/optional.hpp>
+
 #include <Swiften/Base/API.h>
-#include <Swiften/Base/Override.h>
 #include <Swiften/Elements/Payload.h>
 #include <Swiften/Elements/PubSubItem.h>
 #include <Swiften/Elements/PubSubPayload.h>
@@ -44,11 +45,11 @@ namespace Swift {
                 this->items.push_back(value);
             }
 
-            bool isNotify() const {
+            boost::optional<bool> isNotify() const {
                 return notify;
             }
 
-            void setNotify(bool value) {
+            void setNotify(const boost::optional<bool>& value) {
                 this->notify = value ;
             }
 
@@ -56,6 +57,6 @@ namespace Swift {
         private:
             std::string node;
             std::vector< std::shared_ptr<PubSubItem> > items;
-            bool notify;
+            boost::optional<bool> notify;
     };
 }
diff --git a/Swiften/Parser/PayloadParsers/PubSubRetractParser.cpp b/Swiften/Parser/PayloadParsers/PubSubRetractParser.cpp
index b011b76..d5d5c0a 100644
--- a/Swiften/Parser/PayloadParsers/PubSubRetractParser.cpp
+++ b/Swiften/Parser/PayloadParsers/PubSubRetractParser.cpp
@@ -4,15 +4,12 @@
  * See the COPYING file for more information.
  */
 
-#pragma clang diagnostic ignored "-Wunused-private-field"
-
 #include <Swiften/Parser/PayloadParsers/PubSubRetractParser.h>
 
 #include <boost/optional.hpp>
 
-
-#include <Swiften/Parser/PayloadParserFactoryCollection.h>
 #include <Swiften/Parser/PayloadParserFactory.h>
+#include <Swiften/Parser/PayloadParserFactoryCollection.h>
 #include <Swiften/Parser/PayloadParsers/PubSubItemParser.h>
 
 using namespace Swift;
@@ -29,7 +26,11 @@ void PubSubRetractParser::handleStartElement(const std::string& element, const s
             getPayloadInternal()->setNode(*attributeValue);
         }
         if (boost::optional<std::string> attributeValue = attributes.getAttributeValue("notify")) {
-            getPayloadInternal()->setNotify(*attributeValue == "true" ? true : false);
+            boost::optional<bool> notify;
+            if (attributeValue.is_initialized()) {
+                notify = (attributeValue.get() == "true" || attributeValue.get() == "1") ? true : false;
+            }
+            getPayloadInternal()->setNotify(notify);
         }
     }
 
diff --git a/Swiften/Parser/PayloadParsers/UnitTest/PubSubRetractParserTest.cpp b/Swiften/Parser/PayloadParsers/UnitTest/PubSubRetractParserTest.cpp
new file mode 100644
index 0000000..91bc123
--- /dev/null
+++ b/Swiften/Parser/PayloadParsers/UnitTest/PubSubRetractParserTest.cpp
@@ -0,0 +1,73 @@
+/*
+ * Copyright (c) 2016 Isode Limited.
+ * All rights reserved.
+ * See the COPYING file for more information.
+ */
+
+#include <cppunit/extensions/HelperMacros.h>
+#include <cppunit/extensions/TestFactoryRegistry.h>
+
+#include <Swiften/Elements/ContainerPayload.h>
+#include <Swiften/Parser/PayloadParsers/PubSubRetractParser.h>
+#include <Swiften/Parser/PayloadParsers/UnitTest/PayloadsParserTester.h>
+
+using namespace Swift;
+
+class PubSubRetractParserTest : public CppUnit::TestFixture {
+        CPPUNIT_TEST_SUITE(PubSubRetractParserTest);
+        CPPUNIT_TEST(testParse);
+        CPPUNIT_TEST(testParseNotify);
+        CPPUNIT_TEST_SUITE_END();
+
+    public:
+        void testParse() {
+            PayloadsParserTester parser;
+
+            CPPUNIT_ASSERT(parser.parse("<pubsub xmlns='http://jabber.org/protocol/pubsub'>"
+                                            "<retract node='princely_musings' xmlns='http://jabber.org/protocol/pubsub'>"
+                                                "<item id='ae890ac52d0df67ed7cfdf51b644e901' xmlns='http://jabber.org/protocol/pubsub'/>"
+                                            "</retract>"
+                                        "</pubsub>"));
+
+            auto payload = parser.getPayload<ContainerPayload<PubSubPayload>>();
+            std::shared_ptr<PubSubRetract> retract = std::dynamic_pointer_cast<PubSubRetract>(payload->getPayload());
+            CPPUNIT_ASSERT(retract);
+            CPPUNIT_ASSERT_EQUAL(std::string("princely_musings"), retract->getNode());
+            CPPUNIT_ASSERT_EQUAL(false, retract->isNotify().is_initialized());
+        }
+
+        void testParseNotify() {
+            {
+                PayloadsParserTester parser;
+
+                CPPUNIT_ASSERT(parser.parse("<pubsub xmlns='http://jabber.org/protocol/pubsub'>"
+                                                "<retract node='princely_musings' notify='true' xmlns='http://jabber.org/protocol/pubsub'>"
+                                                    "<item id='ae890ac52d0df67ed7cfdf51b644e901' xmlns='http://jabber.org/protocol/pubsub'/>"
+                                                "</retract>"
+                                            "</pubsub>"));
+
+                auto payload = parser.getPayload<ContainerPayload<PubSubPayload>>();
+                std::shared_ptr<PubSubRetract> retract = std::dynamic_pointer_cast<PubSubRetract>(payload->getPayload());
+                CPPUNIT_ASSERT(retract);
+                CPPUNIT_ASSERT_EQUAL(std::string("princely_musings"), retract->getNode());
+                CPPUNIT_ASSERT_EQUAL(true, retract->isNotify().get());
+            }
+
+            {
+                PayloadsParserTester parser;
+                CPPUNIT_ASSERT(parser.parse("<pubsub xmlns='http://jabber.org/protocol/pubsub'>"
+                                                "<retract node='princely_musings' notify='0' xmlns='http://jabber.org/protocol/pubsub'>"
+                                                    "<item id='ae890ac52d0df67ed7cfdf51b644e901' xmlns='http://jabber.org/protocol/pubsub'/>"
+                                                "</retract>"
+                                            "</pubsub>"));
+
+                auto payload = parser.getPayload<ContainerPayload<PubSubPayload>>();
+                auto retract = std::dynamic_pointer_cast<PubSubRetract>(payload->getPayload());
+                CPPUNIT_ASSERT(retract);
+                CPPUNIT_ASSERT_EQUAL(std::string("princely_musings"), retract->getNode());
+                CPPUNIT_ASSERT_EQUAL(false, retract->isNotify().get());
+            }
+        }
+};
+
+CPPUNIT_TEST_SUITE_REGISTRATION(PubSubRetractParserTest);
diff --git a/Swiften/SConscript b/Swiften/SConscript
index 5270ac5..eb7ae19 100644
--- a/Swiften/SConscript
+++ b/Swiften/SConscript
@@ -449,6 +449,7 @@ if env["SCONS_STAGE"] == "build" :
             File("Parser/PayloadParsers/UnitTest/CarbonsParserTest.cpp"),
             File("Parser/PayloadParsers/UnitTest/UserTuneParserTest.cpp"),
             File("Parser/PayloadParsers/UnitTest/UserLocationParserTest.cpp"),
+            File("Parser/PayloadParsers/UnitTest/PubSubRetractParserTest.cpp"),
             File("Parser/UnitTest/BOSHBodyExtractorTest.cpp"),
             File("Parser/UnitTest/AttributeMapTest.cpp"),
             File("Parser/UnitTest/EnumParserTest.cpp"),
@@ -512,6 +513,7 @@ if env["SCONS_STAGE"] == "build" :
             File("Serializer/PayloadSerializers/UnitTest/MAMQuerySerializerTest.cpp"),
             File("Serializer/PayloadSerializers/UnitTest/PubSubItemSerializerTest.cpp"),
             File("Serializer/PayloadSerializers/UnitTest/PubSubItemsSerializerTest.cpp"),
+            File("Serializer/PayloadSerializers/UnitTest/PubSubRetractSerializerTest.cpp"),
             File("Serializer/PayloadSerializers/UnitTest/UserTuneSerializerTest.cpp"),
             File("Serializer/PayloadSerializers/UnitTest/UserLocationSerializerTest.cpp"),
             File("Serializer/UnitTest/StreamFeaturesSerializerTest.cpp"),
diff --git a/Swiften/Serializer/PayloadSerializers/PubSubRetractSerializer.cpp b/Swiften/Serializer/PayloadSerializers/PubSubRetractSerializer.cpp
index 752c234..99b425e 100644
--- a/Swiften/Serializer/PayloadSerializers/PubSubRetractSerializer.cpp
+++ b/Swiften/Serializer/PayloadSerializers/PubSubRetractSerializer.cpp
@@ -4,15 +4,14 @@
  * See the COPYING file for more information.
  */
 
-#pragma clang diagnostic ignored "-Wunused-private-field"
-
 #include <Swiften/Serializer/PayloadSerializers/PubSubRetractSerializer.h>
-#include <Swiften/Serializer/XML/XMLElement.h>
+
 #include <memory>
 
-#include <Swiften/Serializer/PayloadSerializerCollection.h>
 #include <Swiften/Base/foreach.h>
+#include <Swiften/Serializer/PayloadSerializerCollection.h>
 #include <Swiften/Serializer/PayloadSerializers/PubSubItemSerializer.h>
+#include <Swiften/Serializer/XML/XMLElement.h>
 #include <Swiften/Serializer/XML/XMLRawTextNode.h>
 
 using namespace Swift;
@@ -32,7 +31,9 @@ std::string PubSubRetractSerializer::serializePayload(std::shared_ptr<PubSubRetr
     foreach(std::shared_ptr<PubSubItem> item, payload->getItems()) {
         element.addNode(std::make_shared<XMLRawTextNode>(PubSubItemSerializer(serializers).serialize(item)));
     }
-    element.setAttribute("notify", payload->isNotify() ? "true" : "false");
+    if (payload->isNotify().is_initialized()) {
+        element.setAttribute("notify", payload->isNotify().get() ? "true" : "false");
+    }
     return element.serialize();
 }
 
diff --git a/Swiften/Serializer/PayloadSerializers/PubSubRetractSerializer.h b/Swiften/Serializer/PayloadSerializers/PubSubRetractSerializer.h
index 0cb12e4..64737df 100644
--- a/Swiften/Serializer/PayloadSerializers/PubSubRetractSerializer.h
+++ b/Swiften/Serializer/PayloadSerializers/PubSubRetractSerializer.h
@@ -24,9 +24,6 @@ namespace Swift {
             virtual std::string serializePayload(std::shared_ptr<PubSubRetract>) const SWIFTEN_OVERRIDE;
 
         private:
-
-
-        private:
             PayloadSerializerCollection* serializers;
     };
 }
diff --git a/Swiften/Serializer/PayloadSerializers/UnitTest/PubSubRetractSerializerTest.cpp b/Swiften/Serializer/PayloadSerializers/UnitTest/PubSubRetractSerializerTest.cpp
new file mode 100644
index 0000000..f00eddc
--- /dev/null
+++ b/Swiften/Serializer/PayloadSerializers/UnitTest/PubSubRetractSerializerTest.cpp
@@ -0,0 +1,57 @@
+/*
+ * Copyright (c) 2016 Isode Limited.
+ * All rights reserved.
+ * See the COPYING file for more information.
+ */
+
+#include <cppunit/extensions/HelperMacros.h>
+#include <cppunit/extensions/TestFactoryRegistry.h>
+
+#include <Swiften/Serializer/PayloadSerializers/FullPayloadSerializerCollection.h>
+#include <Swiften/Serializer/PayloadSerializers/PubSubRetractSerializer.h>
+
+using namespace Swift;
+
+class PubSubRetractSerializerTest: public CppUnit::TestFixture {
+    CPPUNIT_TEST_SUITE(PubSubRetractSerializerTest);
+    CPPUNIT_TEST(testSerializeDefault);
+    CPPUNIT_TEST(testSerializeNotify);
+    CPPUNIT_TEST_SUITE_END();
+
+    public:
+        PubSubRetractSerializerTest() {}
+
+        std::shared_ptr<PubSubItem> somePubSubItem() {
+            auto item = std::make_shared<PubSubItem>();
+            item->setID("ae890ac52d0df67ed7cfdf51b644e901");
+            return item;
+        }
+
+        void testSerializeDefault() {
+            PubSubRetractSerializer testling(&serializer);
+            auto retract = std::make_shared<PubSubRetract>();
+            retract->setNode("princely_musings");
+            retract->setItems({somePubSubItem()});
+            CPPUNIT_ASSERT_EQUAL(std::string("<retract node=\"princely_musings\" xmlns=\"http://jabber.org/protocol/pubsub\"><item id=\"ae890ac52d0df67ed7cfdf51b644e901\" xmlns=\"http://jabber.org/protocol/pubsub\"/></retract>"), testling.serialize(retract));
+        }
+
+        void testSerializeNotify() {
+            PubSubRetractSerializer testling(&serializer);
+            auto retract = std::make_shared<PubSubRetract>();
+            retract->setNode("princely_musings");
+            retract->setItems({somePubSubItem()});
+            retract->setNotify(true);
+            CPPUNIT_ASSERT_EQUAL(std::string("<retract node=\"princely_musings\" notify=\"true\" xmlns=\"http://jabber.org/protocol/pubsub\"><item id=\"ae890ac52d0df67ed7cfdf51b644e901\" xmlns=\"http://jabber.org/protocol/pubsub\"/></retract>"), testling.serialize(retract));
+
+            retract = std::make_shared<PubSubRetract>();
+            retract->setNode("princely_musings");
+            retract->setItems({somePubSubItem()});
+            retract->setNotify(false);
+            CPPUNIT_ASSERT_EQUAL(std::string("<retract node=\"princely_musings\" notify=\"false\" xmlns=\"http://jabber.org/protocol/pubsub\"><item id=\"ae890ac52d0df67ed7cfdf51b644e901\" xmlns=\"http://jabber.org/protocol/pubsub\"/></retract>"), testling.serialize(retract));
+        }
+
+    private:
+        FullPayloadSerializerCollection serializer;
+};
+
+CPPUNIT_TEST_SUITE_REGISTRATION(PubSubRetractSerializerTest);
-- 
cgit v0.10.2-6-g49f6