From 4455c20085834098f6d9aa872db3115d466e7004 Mon Sep 17 00:00:00 2001 From: Tobias Markmann Date: Sun, 13 Sep 2015 01:22:12 +0200 Subject: Fix notification logic for signals in BlockListImpl The logic for calling onItemAdded and onItemRemoved signals when setting a new list of block items using BlockListImpl::setItems used to be broken. This commit fixes and documents the correct signal notification behavior Test-Information: Added a unit test which verifies the notification behavior in case of added block list items, removed block list items and a complete change of the block list. Change-Id: I3061545e25ddfc2d9d1a3c987045a58e5c9230ac diff --git a/Swiften/Client/BlockListImpl.cpp b/Swiften/Client/BlockListImpl.cpp index 02c1c18..4abaa37 100644 --- a/Swiften/Client/BlockListImpl.cpp +++ b/Swiften/Client/BlockListImpl.cpp @@ -1,34 +1,36 @@ /* - * Copyright (c) 2011 Isode Limited. + * Copyright (c) 2011-2015 Isode Limited. * All rights reserved. * See the COPYING file for more information. */ #include -#include - #include +#include + using namespace Swift; BlockListImpl::BlockListImpl() : state(Init) { } -void BlockListImpl::setItems(const std::vector& items) { - foreach (const JID& jid, this->items) { - if (std::find(items.begin(), items.end(), jid) != items.end()) { +void BlockListImpl::setItems(const std::vector& newItems) { + // JIDs which are in the current list but not in the new list, are removed. + foreach (const JID& jid, items) { + if (std::find(newItems.begin(), newItems.end(), jid) == newItems.end()) { onItemRemoved(jid); } } - foreach (const JID& jid, items) { - if (std::find(this->items.begin(), this->items.end(), jid) != this->items.end()) { + // JIDs which are in the new list but not in the current list, are added. + foreach (const JID& jid, newItems) { + if (std::find(items.begin(), items.end(), jid) == items.end()) { onItemAdded(jid); } } - this->items = items; + items = newItems; } void BlockListImpl::addItem(const JID& item) { diff --git a/Swiften/Client/BlockListImpl.h b/Swiften/Client/BlockListImpl.h index 19359b0..e203d68 100644 --- a/Swiften/Client/BlockListImpl.h +++ b/Swiften/Client/BlockListImpl.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011 Isode Limited. + * Copyright (c) 2011-2015 Isode Limited. * All rights reserved. * See the COPYING file for more information. */ @@ -23,7 +23,7 @@ namespace Swift { return items; } - void setItems(const std::vector& items); + void setItems(const std::vector& newItems); void addItem(const JID& item); void removeItem(const JID& item); void addItems(const std::vector& items); diff --git a/Swiften/Client/UnitTest/BlockListImplTest.cpp b/Swiften/Client/UnitTest/BlockListImplTest.cpp new file mode 100644 index 0000000..9e5bc1a --- /dev/null +++ b/Swiften/Client/UnitTest/BlockListImplTest.cpp @@ -0,0 +1,91 @@ +/* + * Copyright (c) 2015 Isode Limited. + * All rights reserved. + * See the COPYING file for more information. + */ + +#include + +#include + +#include +#include + +#include +#include + +using namespace Swift; + +class BlockListImplTest : public CppUnit::TestFixture { + CPPUNIT_TEST_SUITE(BlockListImplTest); + CPPUNIT_TEST(testSetItemsToSubset); + CPPUNIT_TEST(testSetItemsToSuperset); + CPPUNIT_TEST(testSetItemsAllDifferent); + CPPUNIT_TEST_SUITE_END(); + + public: + + void testSetItemsToSubset() { + std::vector subset; + subset.push_back(JID("a@example.com")); + + blockList_->setItems(subset); + + CPPUNIT_ASSERT_EQUAL(static_cast(0), addedJIDs_.size()); + CPPUNIT_ASSERT_EQUAL(static_cast(1), removedJIDs_.size()); + } + + void testSetItemsToSuperset() { + std::vector superset; + superset.push_back(JID("a@example.com")); + superset.push_back(JID("b@example.com")); + superset.push_back(JID("c@example.com")); + + blockList_->setItems(superset); + + CPPUNIT_ASSERT_EQUAL(static_cast(1), addedJIDs_.size()); + CPPUNIT_ASSERT_EQUAL(static_cast(0), removedJIDs_.size()); + } + + void testSetItemsAllDifferent() { + std::vector newBlockList; + newBlockList.push_back(JID("x@example.com")); + newBlockList.push_back(JID("y@example.com")); + newBlockList.push_back(JID("z@example.com")); + + blockList_->setItems(newBlockList); + + CPPUNIT_ASSERT_EQUAL(static_cast(3), addedJIDs_.size()); + CPPUNIT_ASSERT_EQUAL(static_cast(2), removedJIDs_.size()); + } + + void setUp() { + blockList_ = boost::make_shared(); + addedJIDs_.clear(); + removedJIDs_.clear(); + blockList_->addItem(JID("a@example.com")); + blockList_->addItem(JID("b@example.com")); + + blockList_->onItemAdded.connect(boost::bind(&BlockListImplTest::handleBlockListItemAdded, this, _1)); + blockList_->onItemRemoved.connect(boost::bind(&BlockListImplTest::handleBlockListItemRemoved, this, _1)); + } + + void tearDown() { + blockList_.reset(); + } + + void handleBlockListItemAdded(const JID& jid) { + addedJIDs_.push_back(jid); + } + + void handleBlockListItemRemoved(const JID& jid) { + removedJIDs_.push_back(jid); + } + + private: + boost::shared_ptr blockList_; + std::vector addedJIDs_; + std::vector removedJIDs_; +}; + +CPPUNIT_TEST_SUITE_REGISTRATION(BlockListImplTest); diff --git a/Swiften/SConscript b/Swiften/SConscript index aff1478..9216b39 100644 --- a/Swiften/SConscript +++ b/Swiften/SConscript @@ -380,6 +380,7 @@ if env["SCONS_STAGE"] == "build" : File("Client/UnitTest/ClientSessionTest.cpp"), File("Client/UnitTest/NickResolverTest.cpp"), File("Client/UnitTest/ClientBlockListManagerTest.cpp"), + File("Client/UnitTest/BlockListImplTest.cpp"), File("Compress/UnitTest/ZLibCompressorTest.cpp"), File("Compress/UnitTest/ZLibDecompressorTest.cpp"), File("Component/UnitTest/ComponentHandshakeGeneratorTest.cpp"), -- cgit v0.10.2-6-g49f6