diff options
| author | Richard Maudsley <richard.maudsley@isode.com> | 2014-06-23 10:52:31 (GMT) | 
|---|---|---|
| committer | Richard Maudsley <richard.maudsley@isode.com> | 2014-06-24 14:32:55 (GMT) | 
| commit | c779e07f6d1d23cc157ab3823a00edd95f70ab3b (patch) | |
| tree | 5de546a38677ce19be1b34932144460f11197f96 | |
| parent | bd7f30aec53fc776be678577dbe4f9afec5898a6 (diff) | |
| download | swift-contrib-c779e07f6d1d23cc157ab3823a00edd95f70ab3b.zip swift-contrib-c779e07f6d1d23cc157ab3823a00edd95f70ab3b.tar.bz2 | |
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
| -rw-r--r-- | Swiften/Avatars/AvatarManagerImpl.cpp | 16 | ||||
| -rw-r--r-- | Swiften/Avatars/AvatarProvider.h | 2 | ||||
| -rw-r--r-- | Swiften/Avatars/CombinedAvatarProvider.cpp | 16 | ||||
| -rw-r--r-- | Swiften/Avatars/CombinedAvatarProvider.h | 4 | ||||
| -rw-r--r-- | Swiften/Avatars/OfflineAvatarManager.cpp | 2 | ||||
| -rw-r--r-- | Swiften/Avatars/OfflineAvatarManager.h | 2 | ||||
| -rw-r--r-- | Swiften/Avatars/UnitTest/AvatarManagerImplTest.cpp | 142 | ||||
| -rw-r--r-- | Swiften/Avatars/UnitTest/CombinedAvatarProviderTest.cpp | 165 | ||||
| -rw-r--r-- | Swiften/Avatars/UnitTest/VCardAvatarManagerTest.cpp | 27 | ||||
| -rw-r--r-- | Swiften/Avatars/UnitTest/VCardUpdateAvatarManagerTest.cpp | 20 | ||||
| -rw-r--r-- | Swiften/Avatars/VCardAvatarManager.cpp | 2 | ||||
| -rw-r--r-- | Swiften/Avatars/VCardAvatarManager.h | 2 | ||||
| -rw-r--r-- | Swiften/Avatars/VCardUpdateAvatarManager.cpp | 4 | ||||
| -rw-r--r-- | Swiften/Avatars/VCardUpdateAvatarManager.h | 2 | ||||
| -rw-r--r-- | Swiften/SConscript | 1 | 
15 files changed, 361 insertions, 46 deletions
| 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 @@ -42,7 +42,7 @@ 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(); @@ -50,7 +50,7 @@ boost::filesystem::path AvatarManagerImpl::getAvatarPath(const JID& jid) const {  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(); @@ -58,5 +58,7 @@ ByteArray AvatarManagerImpl::getAvatar(const JID& jid) const {  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 @@ -19,5 +19,5 @@ namespace Swift {  			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 @@ -14,5 +14,5 @@  namespace Swift { -std::string CombinedAvatarProvider::getAvatarHash(const JID& jid) const { +boost::optional<std::string> CombinedAvatarProvider::getAvatarHash(const JID& jid) const {  	return getCombinedAvatarAndCache(jid);  } @@ -37,5 +37,5 @@ void CombinedAvatarProvider::handleAvatarChanged(const JID& jid) {  		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; @@ -44,12 +44,16 @@ void CombinedAvatarProvider::handleAvatarChanged(const JID& 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 @@ -17,5 +17,5 @@ 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*); @@ -24,5 +24,5 @@ namespace Swift {  		private:  			void handleAvatarChanged(const JID&); -			std::string getCombinedAvatarAndCache(const JID&) const; +			boost::optional<std::string> getCombinedAvatarAndCache(const JID&) const;  		private: 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 @@ -19,5 +19,5 @@ 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 @@ -17,5 +17,5 @@ namespace Swift {  			~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); 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 @@ -12,4 +12,16 @@  #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; @@ -29,4 +41,5 @@ class CombinedAvatarProviderTest : public CppUnit::TestFixture {  		CPPUNIT_TEST(testProviderUpdateBareJIDAfterGetFullJID);  		CPPUNIT_TEST(testRemoveProviderDisconnectsUpdates); +		CPPUNIT_TEST(testAddRemoveFallthrough);  		CPPUNIT_TEST_SUITE_END(); @@ -50,5 +63,6 @@ class CombinedAvatarProviderTest : public CppUnit::TestFixture {  			boost::shared_ptr<CombinedAvatarProvider> testling(createProvider()); -			CPPUNIT_ASSERT(testling->getAvatarHash(user1).empty()); +			boost::optional<std::string> hash = testling->getAvatarHash(user1); +			CPPUNIT_ASSERT(!hash);  		} @@ -58,5 +72,7 @@ class CombinedAvatarProviderTest : public CppUnit::TestFixture {  			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);  		} @@ -68,5 +84,7 @@ class CombinedAvatarProviderTest : public CppUnit::TestFixture {  			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);  		} @@ -77,5 +95,7 @@ class CombinedAvatarProviderTest : public CppUnit::TestFixture {  			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);  		} @@ -180,5 +200,131 @@ class CombinedAvatarProviderTest : public CppUnit::TestFixture {  			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;  		} @@ -199,5 +345,5 @@ class CombinedAvatarProviderTest : public CppUnit::TestFixture {  			} -			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); @@ -206,5 +352,5 @@ class CombinedAvatarProviderTest : public CppUnit::TestFixture {  				}  				else { -					return std::string(); +					return boost::optional<std::string>();  				}  			} @@ -214,4 +360,9 @@ class CombinedAvatarProviderTest : public CppUnit::TestFixture {  		}; +		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; 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 @@ -67,7 +67,8 @@ class VCardAvatarManagerTest : public CppUnit::TestFixture {  			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);  		} @@ -76,7 +77,8 @@ class VCardAvatarManagerTest : public CppUnit::TestFixture {  			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);  		} @@ -85,7 +87,8 @@ class VCardAvatarManagerTest : public CppUnit::TestFixture {  			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)); @@ -95,7 +98,8 @@ class VCardAvatarManagerTest : public CppUnit::TestFixture {  			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);  		} @@ -105,7 +109,8 @@ 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 @@ -79,5 +79,7 @@ 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)); @@ -109,5 +111,7 @@ class VCardUpdateAvatarManagerTest : public CppUnit::TestFixture {  			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);  		} @@ -118,5 +122,7 @@ class VCardUpdateAvatarManagerTest : public CppUnit::TestFixture {  			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);  		} @@ -133,5 +139,7 @@ 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);  		} @@ -149,5 +157,7 @@ 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);  		} 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 @@ -30,5 +30,5 @@ void VCardAvatarManager::handleVCardChanged(const JID& 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); 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 @@ -21,5 +21,5 @@ namespace Swift {  			VCardAvatarManager(VCardManager*, AvatarStorage*, CryptoProvider* crypto, MUCRegistry* = NULL); -			std::string getAvatarHash(const JID&) const; +			boost::optional<std::string> getAvatarHash(const JID&) const;  		private: 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 @@ -77,5 +77,5 @@ 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()) { @@ -83,5 +83,5 @@ std::string VCardUpdateAvatarManager::getAvatarHash(const JID& jid) const {  	}  	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 @@ -28,5 +28,5 @@ namespace Swift {  			VCardUpdateAvatarManager(VCardManager*, StanzaChannel*, AvatarStorage*, CryptoProvider* crypto, MUCRegistry* = NULL); -			std::string getAvatarHash(const JID&) const; +			boost::optional<std::string> getAvatarHash(const JID&) const;  		private: diff --git a/Swiften/SConscript b/Swiften/SConscript index 7dbb19d..0314118 100644 --- a/Swiften/SConscript +++ b/Swiften/SConscript @@ -347,4 +347,5 @@ if env["SCONS_STAGE"] == "build" :  			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"), | 
 Swift
 Swift