From c779e07f6d1d23cc157ab3823a00edd95f70ab3b Mon Sep 17 00:00:00 2001 From: Richard Maudsley <richard.maudsley@isode.com> Date: Mon, 23 Jun 2014 11:52:31 +0100 Subject: Fix old avatar being displayed in QtRosterHeader when cleared. Test-Information: Added unit tests for AvatarManagerImpl and CombinedAvatarProvider. Updated test cases to distinguish between error case and empty avatar. Tested changing between blank and non-blank avatars while watching MUC and 1-to-1 chats. Change-Id: I0bea89c7a22ae9c44a0b126e672a7af94b6f5049 diff --git a/Swiften/Avatars/AvatarManagerImpl.cpp b/Swiften/Avatars/AvatarManagerImpl.cpp index 7c3baa7..3aaae33 100644 --- a/Swiften/Avatars/AvatarManagerImpl.cpp +++ b/Swiften/Avatars/AvatarManagerImpl.cpp @@ -41,23 +41,25 @@ AvatarManagerImpl::~AvatarManagerImpl() { } boost::filesystem::path AvatarManagerImpl::getAvatarPath(const JID& jid) const { - std::string hash = combinedAvatarProvider.getAvatarHash(jid); - if (!hash.empty()) { - return avatarStorage->getAvatarPath(hash); + boost::optional<std::string> hash = combinedAvatarProvider.getAvatarHash(jid); + if (hash && !hash->empty()) { + return avatarStorage->getAvatarPath(*hash); } return boost::filesystem::path(); } ByteArray AvatarManagerImpl::getAvatar(const JID& jid) const { - std::string hash = combinedAvatarProvider.getAvatarHash(jid); - if (!hash.empty()) { - return avatarStorage->getAvatar(hash); + boost::optional<std::string> hash = combinedAvatarProvider.getAvatarHash(jid); + if (hash && !hash->empty()) { + return avatarStorage->getAvatar(*hash); } return ByteArray(); } void AvatarManagerImpl::handleCombinedAvatarChanged(const JID& jid) { - offlineAvatarManager->setAvatar(jid, combinedAvatarProvider.getAvatarHash(jid)); + boost::optional<std::string> hash = combinedAvatarProvider.getAvatarHash(jid); + assert(hash); + offlineAvatarManager->setAvatar(jid, *hash); onAvatarChanged(jid); } diff --git a/Swiften/Avatars/AvatarProvider.h b/Swiften/Avatars/AvatarProvider.h index 5c9460d..3606376 100644 --- a/Swiften/Avatars/AvatarProvider.h +++ b/Swiften/Avatars/AvatarProvider.h @@ -18,7 +18,7 @@ namespace Swift { public: virtual ~AvatarProvider(); - virtual std::string getAvatarHash(const JID&) const = 0; + virtual boost::optional<std::string> getAvatarHash(const JID&) const = 0; boost::signal<void (const JID&)> onAvatarChanged; }; diff --git a/Swiften/Avatars/CombinedAvatarProvider.cpp b/Swiften/Avatars/CombinedAvatarProvider.cpp index d283664..1bd74dd 100644 --- a/Swiften/Avatars/CombinedAvatarProvider.cpp +++ b/Swiften/Avatars/CombinedAvatarProvider.cpp @@ -13,7 +13,7 @@ namespace Swift { -std::string CombinedAvatarProvider::getAvatarHash(const JID& jid) const { +boost::optional<std::string> CombinedAvatarProvider::getAvatarHash(const JID& jid) const { return getCombinedAvatarAndCache(jid); } @@ -36,21 +36,25 @@ void CombinedAvatarProvider::handleAvatarChanged(const JID& jid) { if (i != avatars.end()) { oldHash = i->second; } - std::string newHash = getCombinedAvatarAndCache(jid); + boost::optional<std::string> newHash = getCombinedAvatarAndCache(jid); if (newHash != oldHash) { SWIFT_LOG(debug) << "Avatar changed: " << jid << ": " << oldHash << " -> " << newHash << std::endl; onAvatarChanged(jid); } } -std::string CombinedAvatarProvider::getCombinedAvatarAndCache(const JID& jid) const { +boost::optional<std::string> CombinedAvatarProvider::getCombinedAvatarAndCache(const JID& jid) const { SWIFT_LOG(debug) << "JID: " << jid << std::endl; - std::string hash; - for (size_t i = 0; i < providers.size() && hash.empty(); ++i) { + boost::optional<std::string> hash; + for (size_t i = 0; i < providers.size() && !hash; ++i) { hash = providers[i]->getAvatarHash(jid); SWIFT_LOG(debug) << "Provider " << providers[i] << ": " << hash << std::endl; } - avatars[jid] = hash; + if (hash) { + avatars[jid] = *hash; + } else { + avatars[jid] = ""; + } return hash; } diff --git a/Swiften/Avatars/CombinedAvatarProvider.h b/Swiften/Avatars/CombinedAvatarProvider.h index 96989b2..ec06e72 100644 --- a/Swiften/Avatars/CombinedAvatarProvider.h +++ b/Swiften/Avatars/CombinedAvatarProvider.h @@ -16,14 +16,14 @@ namespace Swift { class SWIFTEN_API CombinedAvatarProvider : public AvatarProvider { public: - virtual std::string getAvatarHash(const JID&) const; + virtual boost::optional<std::string> getAvatarHash(const JID&) const; void addProvider(AvatarProvider*); void removeProvider(AvatarProvider*); private: void handleAvatarChanged(const JID&); - std::string getCombinedAvatarAndCache(const JID&) const; + boost::optional<std::string> getCombinedAvatarAndCache(const JID&) const; private: std::vector<AvatarProvider*> providers; diff --git a/Swiften/Avatars/OfflineAvatarManager.cpp b/Swiften/Avatars/OfflineAvatarManager.cpp index 02c6a35..8492e86 100644 --- a/Swiften/Avatars/OfflineAvatarManager.cpp +++ b/Swiften/Avatars/OfflineAvatarManager.cpp @@ -18,7 +18,7 @@ OfflineAvatarManager::OfflineAvatarManager(AvatarStorage* avatarStorage) : avata OfflineAvatarManager::~OfflineAvatarManager() { } -std::string OfflineAvatarManager::getAvatarHash(const JID& jid) const { +boost::optional<std::string> OfflineAvatarManager::getAvatarHash(const JID& jid) const { return avatarStorage->getAvatarForJID(jid); } diff --git a/Swiften/Avatars/OfflineAvatarManager.h b/Swiften/Avatars/OfflineAvatarManager.h index 2098990..baceae8 100644 --- a/Swiften/Avatars/OfflineAvatarManager.h +++ b/Swiften/Avatars/OfflineAvatarManager.h @@ -16,7 +16,7 @@ namespace Swift { OfflineAvatarManager(AvatarStorage*); ~OfflineAvatarManager(); - virtual std::string getAvatarHash(const JID&) const; + virtual boost::optional<std::string> getAvatarHash(const JID&) const; void setAvatar(const JID&, const std::string& hash); private: diff --git a/Swiften/Avatars/UnitTest/AvatarManagerImplTest.cpp b/Swiften/Avatars/UnitTest/AvatarManagerImplTest.cpp new file mode 100644 index 0000000..9b7515d --- /dev/null +++ b/Swiften/Avatars/UnitTest/AvatarManagerImplTest.cpp @@ -0,0 +1,142 @@ +/* + * Copyright (c) 2014 Kevin Smith and Remko Tronçon + * Licensed under the GNU General Public License v3. + * See Documentation/Licenses/GPLv3.txt for more information. + */ + +#include <cppunit/extensions/HelperMacros.h> +#include <cppunit/extensions/TestFactoryRegistry.h> +#include <boost/bind.hpp> + +#include <Swiften/JID/JID.h> +#include <string> +#include <Swiften/Avatars/AvatarManagerImpl.h> +#include <Swiften/Avatars/CombinedAvatarProvider.h> +#include <Swiften/Avatars/VCardAvatarManager.h> +#include <Swiften/Avatars/OfflineAvatarManager.h> +#include <Swiften/Elements/VCardUpdate.h> +#include <Swiften/MUC/MUCRegistry.h> +#include <Swiften/Client/DummyStanzaChannel.h> +#include <Swiften/Crypto/CryptoProvider.h> +#include <Swiften/Crypto/PlatformCryptoProvider.h> +#include <Swiften/Queries/IQRouter.h> +#include <Swiften/Avatars/AvatarMemoryStorage.h> +#include <Swiften/VCards/VCardMemoryStorage.h> +#include <Swiften/VCards/VCardManager.h> +#include <Swiften/Avatars/VCardUpdateAvatarManager.h> +#include <Swiften/StringCodecs/Hexify.h> + +using namespace Swift; + +class AvatarManagerImplTest : public CppUnit::TestFixture { + CPPUNIT_TEST_SUITE(AvatarManagerImplTest); + CPPUNIT_TEST(testGetSetAvatar); + CPPUNIT_TEST_SUITE_END(); + + public: + void setUp() { + ownerJID = JID("owner@domain.com/theowner"); + stanzaChannel = boost::make_shared<DummyStanzaChannel>(); + iqRouter = boost::make_shared<IQRouter>(stanzaChannel.get()); + crypto = boost::shared_ptr<CryptoProvider>(PlatformCryptoProvider::create()); + vcardStorage = boost::make_shared<VCardMemoryStorage>(crypto.get()); + vcardManager = boost::make_shared<VCardManager>(ownerJID, iqRouter.get(), vcardStorage.get()); + avatarStorage = boost::make_shared<AvatarMemoryStorage>(); + mucRegistry = boost::make_shared<DummyMUCRegistry>(); + avatarManager = boost::make_shared<AvatarManagerImpl>(vcardManager.get(), stanzaChannel.get(), avatarStorage.get(), crypto.get(), mucRegistry.get()); + } + + void testGetSetAvatar() { + /* initially we have no knowledge of the user or their avatar */ + JID personJID("person@domain.com/theperson"); + ByteArray avatar = avatarManager->getAvatar(personJID.toBare()); + CPPUNIT_ASSERT(!avatar.size()); + + /* notify the 'owner' JID that our avatar has changed */ + + ByteArray fullAvatar = createByteArray("abcdefg"); + boost::shared_ptr<VCardUpdate> vcardUpdate = boost::make_shared<VCardUpdate>(); + vcardUpdate->setPhotoHash(Hexify::hexify(crypto->getSHA1Hash(fullAvatar))); + boost::shared_ptr<Presence> presence = boost::make_shared<Presence>(); + presence->setTo(ownerJID); + presence->setFrom(personJID); + presence->setType(Presence::Available); + presence->addPayload(vcardUpdate); + stanzaChannel->onPresenceReceived(presence); + + /* reply to the avatar request with our new avatar */ + + CPPUNIT_ASSERT_EQUAL(size_t(1), stanzaChannel->sentStanzas.size()); + boost::shared_ptr<IQ> request = boost::dynamic_pointer_cast<IQ>(stanzaChannel->sentStanzas[0]); + stanzaChannel->sentStanzas.pop_back(); + CPPUNIT_ASSERT(!!request); + boost::shared_ptr<VCard> vcard = request->getPayload<VCard>(); + CPPUNIT_ASSERT(!!vcard); + + boost::shared_ptr<IQ> reply = boost::make_shared<IQ>(IQ::Result); + reply->setTo(request->getFrom()); + reply->setFrom(request->getTo()); + reply->setID(request->getID()); + vcard->setPhoto(fullAvatar); + reply->addPayload(vcard); + stanzaChannel->onIQReceived(reply); + + /* check hash through avatarManager that it received the correct photo */ + + ByteArray reportedAvatar = avatarManager->getAvatar(personJID.toBare()); + CPPUNIT_ASSERT_EQUAL(byteArrayToString(fullAvatar), byteArrayToString(reportedAvatar)); + + /* send new presence to notify of blank avatar */ + + vcardUpdate = boost::make_shared<VCardUpdate>(); + presence = boost::make_shared<Presence>(); + presence->setTo(ownerJID); + presence->setFrom(personJID); + presence->setType(Presence::Available); + presence->addPayload(vcardUpdate); + stanzaChannel->onPresenceReceived(presence); + + /* reply to the avatar request with our EMPTY avatar */ + + CPPUNIT_ASSERT_EQUAL(size_t(1), stanzaChannel->sentStanzas.size()); + request = boost::dynamic_pointer_cast<IQ>(stanzaChannel->sentStanzas[0]); + stanzaChannel->sentStanzas.pop_back(); + CPPUNIT_ASSERT(!!request); + vcard = request->getPayload<VCard>(); + CPPUNIT_ASSERT(!!vcard); + + ByteArray blankAvatar = createByteArray(""); + reply = boost::make_shared<IQ>(IQ::Result); + reply->setTo(request->getFrom()); + reply->setFrom(request->getTo()); + reply->setID(request->getID()); + vcard->setPhoto(blankAvatar); + reply->addPayload(vcard); + stanzaChannel->onIQReceived(reply); + + /* check hash through avatarManager that it received the correct photo */ + + reportedAvatar = avatarManager->getAvatar(personJID.toBare()); + CPPUNIT_ASSERT_EQUAL(byteArrayToString(blankAvatar), byteArrayToString(reportedAvatar)); + } + + struct DummyMUCRegistry : public MUCRegistry { + bool isMUC(const JID& jid) const { return std::find(mucs_.begin(), mucs_.end(), jid) != mucs_.end(); } + std::vector<JID> mucs_; + }; + + private: + + JID ownerJID; + boost::shared_ptr<DummyStanzaChannel> stanzaChannel; + boost::shared_ptr<IQRouter> iqRouter; + boost::shared_ptr<CryptoProvider> crypto; + boost::shared_ptr<VCardMemoryStorage> vcardStorage; + boost::shared_ptr<VCardManager> vcardManager; + boost::shared_ptr<AvatarMemoryStorage> avatarStorage; + boost::shared_ptr<DummyMUCRegistry> mucRegistry; + boost::shared_ptr<AvatarManagerImpl> avatarManager; + +}; + +CPPUNIT_TEST_SUITE_REGISTRATION(AvatarManagerImplTest); diff --git a/Swiften/Avatars/UnitTest/CombinedAvatarProviderTest.cpp b/Swiften/Avatars/UnitTest/CombinedAvatarProviderTest.cpp index 50b0adb..715b7fc 100644 --- a/Swiften/Avatars/UnitTest/CombinedAvatarProviderTest.cpp +++ b/Swiften/Avatars/UnitTest/CombinedAvatarProviderTest.cpp @@ -11,6 +11,18 @@ #include <Swiften/JID/JID.h> #include <string> #include <Swiften/Avatars/CombinedAvatarProvider.h> +#include <Swiften/Avatars/VCardAvatarManager.h> +#include <Swiften/Avatars/OfflineAvatarManager.h> +#include <Swiften/MUC/MUCRegistry.h> +#include <Swiften/Client/DummyStanzaChannel.h> +#include <Swiften/Crypto/CryptoProvider.h> +#include <Swiften/Crypto/PlatformCryptoProvider.h> +#include <Swiften/Queries/IQRouter.h> +#include <Swiften/Avatars/AvatarMemoryStorage.h> +#include <Swiften/VCards/VCardMemoryStorage.h> +#include <Swiften/VCards/VCardManager.h> +#include <Swiften/Avatars/VCardUpdateAvatarManager.h> +#include <Swiften/StringCodecs/Hexify.h> using namespace Swift; @@ -28,6 +40,7 @@ class CombinedAvatarProviderTest : public CppUnit::TestFixture { CPPUNIT_TEST(testProviderUpdateAfterGetDoesNotTriggerChange); CPPUNIT_TEST(testProviderUpdateBareJIDAfterGetFullJID); CPPUNIT_TEST(testRemoveProviderDisconnectsUpdates); + CPPUNIT_TEST(testAddRemoveFallthrough); CPPUNIT_TEST_SUITE_END(); public: @@ -49,7 +62,8 @@ class CombinedAvatarProviderTest : public CppUnit::TestFixture { void testGetAvatarWithNoAvatarProviderReturnsEmpty() { boost::shared_ptr<CombinedAvatarProvider> testling(createProvider()); - CPPUNIT_ASSERT(testling->getAvatarHash(user1).empty()); + boost::optional<std::string> hash = testling->getAvatarHash(user1); + CPPUNIT_ASSERT(!hash); } void testGetAvatarWithSingleAvatarProvider() { @@ -57,7 +71,9 @@ class CombinedAvatarProviderTest : public CppUnit::TestFixture { avatarProvider1->avatars[user1] = avatarHash1; testling->addProvider(avatarProvider1); - CPPUNIT_ASSERT_EQUAL(avatarHash1, testling->getAvatarHash(user1)); + boost::optional<std::string> hash = testling->getAvatarHash(user1); + CPPUNIT_ASSERT(hash); + CPPUNIT_ASSERT_EQUAL(avatarHash1, *hash); } void testGetAvatarWithMultipleAvatarProviderReturnsFirstAvatar() { @@ -67,7 +83,9 @@ class CombinedAvatarProviderTest : public CppUnit::TestFixture { testling->addProvider(avatarProvider1); testling->addProvider(avatarProvider2); - CPPUNIT_ASSERT_EQUAL(avatarHash1, testling->getAvatarHash(user1)); + boost::optional<std::string> hash = testling->getAvatarHash(user1); + CPPUNIT_ASSERT(hash); + CPPUNIT_ASSERT_EQUAL(avatarHash1, *hash); } void testGetAvatarWithMultipleAvatarProviderAndFailingFirstProviderReturnsSecondAvatar() { @@ -76,7 +94,9 @@ class CombinedAvatarProviderTest : public CppUnit::TestFixture { testling->addProvider(avatarProvider1); testling->addProvider(avatarProvider2); - CPPUNIT_ASSERT_EQUAL(avatarHash2, testling->getAvatarHash(user1)); + boost::optional<std::string> hash = testling->getAvatarHash(user1); + CPPUNIT_ASSERT(hash); + CPPUNIT_ASSERT_EQUAL(avatarHash2, *hash); } void testProviderUpdateTriggersChange() { @@ -179,7 +199,133 @@ class CombinedAvatarProviderTest : public CppUnit::TestFixture { avatarProvider1->avatars[user1.toBare()] = avatarHash2; avatarProvider1->onAvatarChanged(user1.toBare()); - CPPUNIT_ASSERT_EQUAL(avatarHash2, testling->getAvatarHash(user1)); + boost::optional<std::string> hash = testling->getAvatarHash(user1); + CPPUNIT_ASSERT(hash); + CPPUNIT_ASSERT_EQUAL(avatarHash2, *hash); + } + + void testAddRemoveFallthrough() { + JID ownJID = JID("user0@own.com/res"); + JID user1 = JID("user1@bar.com/bla"); + + boost::shared_ptr<CryptoProvider> crypto = boost::shared_ptr<CryptoProvider>(PlatformCryptoProvider::create()); + DummyStanzaChannel* stanzaChannel = new DummyStanzaChannel(); + stanzaChannel->setAvailable(true); + IQRouter* iqRouter = new IQRouter(stanzaChannel); + DummyMUCRegistry* mucRegistry = new DummyMUCRegistry(); + AvatarMemoryStorage* avatarStorage = new AvatarMemoryStorage(); + VCardMemoryStorage* vcardStorage = new VCardMemoryStorage(crypto.get()); + VCardManager* vcardManager = new VCardManager(ownJID, iqRouter, vcardStorage); + + boost::shared_ptr<VCardUpdateAvatarManager> updateManager(new VCardUpdateAvatarManager(vcardManager, stanzaChannel, avatarStorage, crypto.get(), mucRegistry)); + updateManager->onAvatarChanged.connect(boost::bind(&CombinedAvatarProviderTest::handleAvatarChanged, this, _1)); + + boost::shared_ptr<VCardAvatarManager> manager(new VCardAvatarManager(vcardManager, avatarStorage, crypto.get(), mucRegistry)); + manager->onAvatarChanged.connect(boost::bind(&CombinedAvatarProviderTest::handleAvatarChanged, this, _1)); + + boost::shared_ptr<OfflineAvatarManager> offlineManager(new OfflineAvatarManager(avatarStorage)); + offlineManager->onAvatarChanged.connect(boost::bind(&CombinedAvatarProviderTest::handleAvatarChanged, this, _1)); + + boost::shared_ptr<CombinedAvatarProvider> testling(createProvider()); + avatarProvider1->useBare = true; + testling->addProvider(updateManager.get()); + testling->addProvider(manager.get()); + testling->addProvider(offlineManager.get()); + + /* place an avatar in the cache, check that it reads back OK */ + + CPPUNIT_ASSERT_EQUAL(size_t(0), changes.size()); + + ByteArray avatar1 = createByteArray("abcdefg"); + std::string avatar1Hash = Hexify::hexify(crypto->getSHA1Hash(avatar1)); + VCard::ref vcard1(new VCard()); + vcard1->setPhoto(avatar1); + + vcardStorage->setVCard(user1.toBare(), vcard1); + boost::optional<std::string> testHash = testling->getAvatarHash(user1.toBare()); + CPPUNIT_ASSERT(testHash); + CPPUNIT_ASSERT_EQUAL(avatar1Hash, *testHash); + + VCard::ref storedVCard = vcardStorage->getVCard(user1.toBare()); + CPPUNIT_ASSERT(!!storedVCard); + testHash = Hexify::hexify(crypto->getSHA1Hash(storedVCard->getPhoto())); + CPPUNIT_ASSERT_EQUAL(avatar1Hash, *testHash); + + /* change the avatar by sending an VCard IQ */ + + vcardManager->requestVCard(user1.toBare()); + CPPUNIT_ASSERT_EQUAL(size_t(1), stanzaChannel->sentStanzas.size()); + IQ::ref request = boost::dynamic_pointer_cast<IQ>(stanzaChannel->sentStanzas.back()); + VCard::ref payload = request->getPayload<VCard>(); + CPPUNIT_ASSERT(!!payload); + stanzaChannel->sentStanzas.pop_back(); + + ByteArray avatar2 = createByteArray("1234567"); + std::string avatar2Hash = Hexify::hexify(crypto->getSHA1Hash(avatar2)); + VCard::ref vcard2(new VCard()); + vcard2->setPhoto(avatar2); + + IQ::ref reply = boost::make_shared<IQ>(); + reply->setTo(request->getFrom()); + reply->setFrom(request->getTo()); + reply->setID(request->getID()); + reply->addPayload(vcard2); + reply->setType(IQ::Result); + + stanzaChannel->onIQReceived(reply); + + /* check that we changed the avatar successfully and that we were notified about the changes */ + + testHash = testling->getAvatarHash(user1.toBare()); + CPPUNIT_ASSERT(testHash); + CPPUNIT_ASSERT_EQUAL(avatar2Hash, *testHash); + CPPUNIT_ASSERT_EQUAL(size_t(3), changes.size()); + CPPUNIT_ASSERT_EQUAL(user1.toBare(), changes[0]); + CPPUNIT_ASSERT_EQUAL(user1.toBare(), changes[1]); + CPPUNIT_ASSERT_EQUAL(user1.toBare(), changes[2]); + changes.clear(); + storedVCard = vcardStorage->getVCard(user1.toBare()); + CPPUNIT_ASSERT(!!storedVCard); + testHash = Hexify::hexify(crypto->getSHA1Hash(storedVCard->getPhoto())); + CPPUNIT_ASSERT_EQUAL(avatar2Hash, *testHash); + + /* change the avatar to the empty avatar */ + + vcardManager->requestVCard(user1.toBare()); + CPPUNIT_ASSERT_EQUAL(size_t(1), stanzaChannel->sentStanzas.size()); + request = boost::dynamic_pointer_cast<IQ>(stanzaChannel->sentStanzas.back()); + payload = request->getPayload<VCard>(); + CPPUNIT_ASSERT(!!payload); + stanzaChannel->sentStanzas.pop_back(); + + VCard::ref vcard3(new VCard()); + reply = boost::make_shared<IQ>(); + reply->setTo(request->getFrom()); + reply->setFrom(request->getTo()); + reply->setID(request->getID()); + reply->addPayload(vcard3); + reply->setType(IQ::Result); + stanzaChannel->onIQReceived(reply); + + /* check that we changed the avatar successfully */ + + testHash = testling->getAvatarHash(user1.toBare()); + CPPUNIT_ASSERT(testHash); + CPPUNIT_ASSERT_EQUAL(std::string(""), *testHash); + CPPUNIT_ASSERT_EQUAL(size_t(3), changes.size()); + CPPUNIT_ASSERT_EQUAL(user1.toBare(), changes[0]); + CPPUNIT_ASSERT_EQUAL(user1.toBare(), changes[1]); + CPPUNIT_ASSERT_EQUAL(user1.toBare(), changes[2]); + changes.clear(); + storedVCard = vcardStorage->getVCard(user1.toBare()); + CPPUNIT_ASSERT(!!storedVCard); + CPPUNIT_ASSERT(!storedVCard->getPhoto().size()); + + delete vcardManager; + delete vcardStorage; + delete mucRegistry; + delete iqRouter; + delete stanzaChannel; } private: @@ -198,14 +344,14 @@ class CombinedAvatarProviderTest : public CppUnit::TestFixture { DummyAvatarProvider() : useBare(false) { } - std::string getAvatarHash(const JID& jid) const { + boost::optional<std::string> getAvatarHash(const JID& jid) const { JID actualJID = useBare ? jid.toBare() : jid; std::map<JID, std::string>::const_iterator i = avatars.find(actualJID); if (i != avatars.end()) { return i->second; } else { - return std::string(); + return boost::optional<std::string>(); } } @@ -213,6 +359,11 @@ class CombinedAvatarProviderTest : public CppUnit::TestFixture { std::map<JID, std::string> avatars; }; + struct DummyMUCRegistry : public MUCRegistry { + bool isMUC(const JID& jid) const { return std::find(mucs_.begin(), mucs_.end(), jid) != mucs_.end(); } + std::vector<JID> mucs_; + }; + DummyAvatarProvider* avatarProvider1; DummyAvatarProvider* avatarProvider2; JID user1; diff --git a/Swiften/Avatars/UnitTest/VCardAvatarManagerTest.cpp b/Swiften/Avatars/UnitTest/VCardAvatarManagerTest.cpp index 97edc73..778b001 100644 --- a/Swiften/Avatars/UnitTest/VCardAvatarManagerTest.cpp +++ b/Swiften/Avatars/UnitTest/VCardAvatarManagerTest.cpp @@ -66,27 +66,30 @@ class VCardAvatarManagerTest : public CppUnit::TestFixture { storeVCardWithPhoto(user1.toBare(), avatar1); avatarStorage->addAvatar(avatar1Hash, avatar1); - std::string result = testling->getAvatarHash(user1); + boost::optional<std::string> result = testling->getAvatarHash(user1); - CPPUNIT_ASSERT_EQUAL(avatar1Hash, result); + CPPUNIT_ASSERT(result); + CPPUNIT_ASSERT_EQUAL(avatar1Hash, *result); } void testGetAvatarHashEmptyAvatar() { boost::shared_ptr<VCardAvatarManager> testling = createManager(); storeEmptyVCard(user1.toBare()); - std::string result = testling->getAvatarHash(user1); + boost::optional<std::string> result = testling->getAvatarHash(user1); - CPPUNIT_ASSERT_EQUAL(std::string(), result); + CPPUNIT_ASSERT(result); + CPPUNIT_ASSERT_EQUAL(std::string(), *result); } void testGetAvatarHashUnknownAvatarKnownVCardStoresAvatar() { boost::shared_ptr<VCardAvatarManager> testling = createManager(); storeVCardWithPhoto(user1.toBare(), avatar1); - std::string result = testling->getAvatarHash(user1); + boost::optional<std::string> result = testling->getAvatarHash(user1); - CPPUNIT_ASSERT_EQUAL(avatar1Hash, result); + CPPUNIT_ASSERT(result); + CPPUNIT_ASSERT_EQUAL(avatar1Hash, *result); CPPUNIT_ASSERT(avatarStorage->hasAvatar(avatar1Hash)); CPPUNIT_ASSERT_EQUAL(avatar1, avatarStorage->getAvatar(avatar1Hash)); } @@ -94,9 +97,10 @@ class VCardAvatarManagerTest : public CppUnit::TestFixture { void testGetAvatarHashUnknownAvatarUnknownVCard() { boost::shared_ptr<VCardAvatarManager> testling = createManager(); - std::string result = testling->getAvatarHash(user1); + boost::optional<std::string> result = testling->getAvatarHash(user1); - CPPUNIT_ASSERT_EQUAL(std::string(), result); + CPPUNIT_ASSERT(result); + CPPUNIT_ASSERT_EQUAL(std::string(), *result); } void testGetAvatarHashKnownAvatarUnknownVCard() { @@ -104,9 +108,10 @@ class VCardAvatarManagerTest : public CppUnit::TestFixture { avatarStorage->setAvatarForJID(user1, avatar1Hash); - std::string result = testling->getAvatarHash(user1); - - CPPUNIT_ASSERT_EQUAL(std::string(), result); + boost::optional<std::string> result = testling->getAvatarHash(user1); + + CPPUNIT_ASSERT(result); + CPPUNIT_ASSERT_EQUAL(std::string(), *result); } diff --git a/Swiften/Avatars/UnitTest/VCardUpdateAvatarManagerTest.cpp b/Swiften/Avatars/UnitTest/VCardUpdateAvatarManagerTest.cpp index 01b10a2..c29a763 100644 --- a/Swiften/Avatars/UnitTest/VCardUpdateAvatarManagerTest.cpp +++ b/Swiften/Avatars/UnitTest/VCardUpdateAvatarManagerTest.cpp @@ -78,7 +78,9 @@ class VCardUpdateAvatarManagerTest : public CppUnit::TestFixture { CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(changes.size())); CPPUNIT_ASSERT_EQUAL(user1.toBare(), changes[0]); - CPPUNIT_ASSERT_EQUAL(avatar1Hash, testling->getAvatarHash(user1.toBare())); + boost::optional<std::string> hash = testling->getAvatarHash(user1.toBare()); + CPPUNIT_ASSERT(hash); + CPPUNIT_ASSERT_EQUAL(avatar1Hash, *hash); CPPUNIT_ASSERT(avatarStorage->hasAvatar(avatar1Hash)); CPPUNIT_ASSERT_EQUAL(avatar1, avatarStorage->getAvatar(avatar1Hash)); } @@ -108,7 +110,9 @@ class VCardUpdateAvatarManagerTest : public CppUnit::TestFixture { CPPUNIT_ASSERT_EQUAL(0, static_cast<int>(stanzaChannel->sentStanzas.size())); CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(changes.size())); CPPUNIT_ASSERT_EQUAL(user2.toBare(), changes[0]); - CPPUNIT_ASSERT_EQUAL(avatar1Hash, testling->getAvatarHash(user2.toBare())); + boost::optional<std::string> hash = testling->getAvatarHash(user2.toBare()); + CPPUNIT_ASSERT(hash); + CPPUNIT_ASSERT_EQUAL(avatar1Hash, *hash); } void testVCardWithEmptyPhoto() { @@ -117,7 +121,9 @@ class VCardUpdateAvatarManagerTest : public CppUnit::TestFixture { stanzaChannel->onIQReceived(createVCardResult(ByteArray())); CPPUNIT_ASSERT(!avatarStorage->hasAvatar(Hexify::hexify(crypto->getSHA1Hash(ByteArray())))); - CPPUNIT_ASSERT_EQUAL(std::string(), testling->getAvatarHash(JID("foo@bar.com"))); + boost::optional<std::string> hash = testling->getAvatarHash(JID("foo@bar.com")); + CPPUNIT_ASSERT(hash); + CPPUNIT_ASSERT_EQUAL(std::string(), *hash); } void testStanzaChannelReset_ClearsHash() { @@ -132,7 +138,9 @@ class VCardUpdateAvatarManagerTest : public CppUnit::TestFixture { CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(changes.size())); CPPUNIT_ASSERT_EQUAL(user1.toBare(), changes[0]); - CPPUNIT_ASSERT_EQUAL(std::string(""), testling->getAvatarHash(user1.toBare())); + boost::optional<std::string> hash = testling->getAvatarHash(user1.toBare()); + CPPUNIT_ASSERT(!hash); + //CPPUNIT_ASSERT_EQUAL(std::string(""), *hash); } void testStanzaChannelReset_ReceiveHashAfterResetUpdatesHash() { @@ -148,7 +156,9 @@ class VCardUpdateAvatarManagerTest : public CppUnit::TestFixture { CPPUNIT_ASSERT_EQUAL(2, static_cast<int>(changes.size())); CPPUNIT_ASSERT_EQUAL(user1.toBare(), changes[1]); - CPPUNIT_ASSERT_EQUAL(avatar1Hash, testling->getAvatarHash(user1.toBare())); + boost::optional<std::string> hash = testling->getAvatarHash(user1.toBare()); + CPPUNIT_ASSERT(hash); + CPPUNIT_ASSERT_EQUAL(avatar1Hash, *hash); } private: diff --git a/Swiften/Avatars/VCardAvatarManager.cpp b/Swiften/Avatars/VCardAvatarManager.cpp index 8212a6e..539b7a0 100644 --- a/Swiften/Avatars/VCardAvatarManager.cpp +++ b/Swiften/Avatars/VCardAvatarManager.cpp @@ -29,7 +29,7 @@ void VCardAvatarManager::handleVCardChanged(const JID& from) { onAvatarChanged(from); } -std::string VCardAvatarManager::getAvatarHash(const JID& jid) const { +boost::optional<std::string> VCardAvatarManager::getAvatarHash(const JID& jid) const { JID avatarJID = getAvatarJID(jid); std::string hash = vcardManager_->getPhotoHash(avatarJID); if (!hash.empty()) { diff --git a/Swiften/Avatars/VCardAvatarManager.h b/Swiften/Avatars/VCardAvatarManager.h index 9c6943e..a907fa5 100644 --- a/Swiften/Avatars/VCardAvatarManager.h +++ b/Swiften/Avatars/VCardAvatarManager.h @@ -20,7 +20,7 @@ namespace Swift { public: VCardAvatarManager(VCardManager*, AvatarStorage*, CryptoProvider* crypto, MUCRegistry* = NULL); - std::string getAvatarHash(const JID&) const; + boost::optional<std::string> getAvatarHash(const JID&) const; private: void handleVCardChanged(const JID& from); diff --git a/Swiften/Avatars/VCardUpdateAvatarManager.cpp b/Swiften/Avatars/VCardUpdateAvatarManager.cpp index 3a32889..55537ff 100644 --- a/Swiften/Avatars/VCardUpdateAvatarManager.cpp +++ b/Swiften/Avatars/VCardUpdateAvatarManager.cpp @@ -76,13 +76,13 @@ void VCardUpdateAvatarManager::setAvatar(const JID& jid, const ByteArray& avatar } */ -std::string VCardUpdateAvatarManager::getAvatarHash(const JID& jid) const { +boost::optional<std::string> VCardUpdateAvatarManager::getAvatarHash(const JID& jid) const { std::map<JID, std::string>::const_iterator i = avatarHashes_.find(getAvatarJID(jid)); if (i != avatarHashes_.end()) { return i->second; } else { - return ""; + return boost::optional<std::string>(); } } diff --git a/Swiften/Avatars/VCardUpdateAvatarManager.h b/Swiften/Avatars/VCardUpdateAvatarManager.h index 3409f99..333516b 100644 --- a/Swiften/Avatars/VCardUpdateAvatarManager.h +++ b/Swiften/Avatars/VCardUpdateAvatarManager.h @@ -27,7 +27,7 @@ namespace Swift { public: VCardUpdateAvatarManager(VCardManager*, StanzaChannel*, AvatarStorage*, CryptoProvider* crypto, MUCRegistry* = NULL); - std::string getAvatarHash(const JID&) const; + boost::optional<std::string> getAvatarHash(const JID&) const; private: void handlePresenceReceived(boost::shared_ptr<Presence>); diff --git a/Swiften/SConscript b/Swiften/SConscript index 7dbb19d..0314118 100644 --- a/Swiften/SConscript +++ b/Swiften/SConscript @@ -346,6 +346,7 @@ if env["SCONS_STAGE"] == "build" : File("Avatars/UnitTest/VCardUpdateAvatarManagerTest.cpp"), File("Avatars/UnitTest/VCardAvatarManagerTest.cpp"), File("Avatars/UnitTest/CombinedAvatarProviderTest.cpp"), + File("Avatars/UnitTest/AvatarManagerImplTest.cpp"), File("Base/UnitTest/IDGeneratorTest.cpp"), File("Base/UnitTest/SimpleIDGeneratorTest.cpp"), File("Base/UnitTest/StringTest.cpp"), -- cgit v0.10.2-6-g49f6