14 files changed, 273 insertions, 21 deletions
diff --git a/Swiften/Avatars/AvatarManager.cpp b/Swiften/Avatars/AvatarManager.cpp
index 6811a8e..6ad39fb 100644
--- a/Swiften/Avatars/AvatarManager.cpp
+++ b/Swiften/Avatars/AvatarManager.cpp
@@ -9,6 +9,7 @@
 #include <boost/bind.hpp>
 
 #include "Swiften/Avatars/VCardUpdateAvatarManager.h"
+#include "Swiften/Avatars/VCardAvatarManager.h"
 #include "Swiften/Avatars/AvatarStorage.h"
 
 namespace Swift {
@@ -16,10 +17,16 @@ namespace Swift {
 AvatarManager::AvatarManager(VCardManager* vcardManager, StanzaChannel* stanzaChannel, AvatarStorage* avatarStorage, MUCRegistry* mucRegistry) : avatarStorage(avatarStorage) {
 	vcardUpdateAvatarManager = new VCardUpdateAvatarManager(vcardManager, stanzaChannel, avatarStorage, mucRegistry);
 	combinedAvatarProvider.addProvider(vcardUpdateAvatarManager);
+
+	vcardAvatarManager = new VCardAvatarManager(vcardManager, avatarStorage, mucRegistry);
+	combinedAvatarProvider.addProvider(vcardAvatarManager);
+
 	combinedAvatarProvider.onAvatarChanged.connect(boost::ref(onAvatarChanged));
 }
 
 AvatarManager::~AvatarManager() {
+	combinedAvatarProvider.removeProvider(vcardAvatarManager);
+	delete vcardAvatarManager;
 	combinedAvatarProvider.removeProvider(vcardUpdateAvatarManager);
 	delete vcardUpdateAvatarManager;
 }
@@ -27,6 +34,7 @@ AvatarManager::~AvatarManager() {
 boost::filesystem::path AvatarManager::getAvatarPath(const JID& jid) const {
 	String hash = combinedAvatarProvider.getAvatarHash(jid);
 	if (!hash.isEmpty()) {
+		std::cout << "getAvatar " << jid << " " << avatarStorage->getAvatarPath(hash) << std::endl;
 		return avatarStorage->getAvatarPath(hash);
 	}
 	return boost::filesystem::path();
diff --git a/Swiften/Avatars/AvatarManager.h b/Swiften/Avatars/AvatarManager.h
index 04ce496..0309b20 100644
--- a/Swiften/Avatars/AvatarManager.h
+++ b/Swiften/Avatars/AvatarManager.h
@@ -24,6 +24,7 @@ namespace Swift {
 	class StanzaChannel;
 	class VCardManager;
 	class VCardUpdateAvatarManager;
+	class VCardAvatarManager;
 
 	class AvatarManager {
 		public:
@@ -37,7 +38,8 @@ namespace Swift {
 
 		private:
 			CombinedAvatarProvider combinedAvatarProvider;
-			VCardUpdateAvatarManager* vcardUpdateAvatarManager;
 			AvatarStorage* avatarStorage;
+			VCardUpdateAvatarManager* vcardUpdateAvatarManager;
+			VCardAvatarManager* vcardAvatarManager;
 	};
 }
diff --git a/Swiften/Avatars/SConscript b/Swiften/Avatars/SConscript
index c5181e8..60bee08 100644
--- a/Swiften/Avatars/SConscript
+++ b/Swiften/Avatars/SConscript
@@ -3,6 +3,7 @@ Import("swiften_env")
 objects = swiften_env.StaticObject([
 			"AvatarFileStorage.cpp",
 			"VCardUpdateAvatarManager.cpp",
+			"VCardAvatarManager.cpp",
 			"AvatarManager.cpp",
 			"AvatarStorage.cpp",
 			"AvatarProvider.cpp",
diff --git a/Swiften/Avatars/UnitTest/VCardAvatarManagerTest.cpp b/Swiften/Avatars/UnitTest/VCardAvatarManagerTest.cpp
new file mode 100644
index 0000000..8a0658c
--- /dev/null
+++ b/Swiften/Avatars/UnitTest/VCardAvatarManagerTest.cpp
@@ -0,0 +1,152 @@
+/*
+ * Copyright (c) 2010 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/Elements/VCard.h"
+#include "Swiften/Avatars/VCardAvatarManager.h"
+#include "Swiften/Avatars/AvatarMemoryStorage.h"
+#include "Swiften/VCards/VCardMemoryStorage.h"
+#include "Swiften/VCards/VCardManager.h"
+#include "Swiften/MUC/MUCRegistry.h"
+#include "Swiften/Queries/IQRouter.h"
+#include "Swiften/Client/DummyStanzaChannel.h"
+#include "Swiften/StringCodecs/SHA1.h"
+#include "Swiften/StringCodecs/Hexify.h"
+
+using namespace Swift;
+
+class VCardAvatarManagerTest : public CppUnit::TestFixture {
+		CPPUNIT_TEST_SUITE(VCardAvatarManagerTest);
+		CPPUNIT_TEST(testGetAvatarHashKnownAvatar);
+		CPPUNIT_TEST(testGetAvatarHashEmptyAvatar);
+		CPPUNIT_TEST(testGetAvatarHashUnknownAvatarKnownVCardStoresAvatar);
+		CPPUNIT_TEST(testGetAvatarHashUnknownAvatarUnknownVCard);
+		CPPUNIT_TEST(testVCardUpdateTriggersUpdate);
+		CPPUNIT_TEST_SUITE_END();
+
+	public:
+		void setUp() {
+			ownJID = JID("foo@fum.com/bum");
+			stanzaChannel = new DummyStanzaChannel();
+			stanzaChannel->setAvailable(true);
+			iqRouter = new IQRouter(stanzaChannel);
+			mucRegistry = new DummyMUCRegistry();
+			avatarStorage = new AvatarMemoryStorage();
+			vcardStorage = new VCardMemoryStorage();
+			vcardManager = new VCardManager(ownJID, iqRouter, vcardStorage);
+			avatar1 = ByteArray("abcdefg");
+			avatar1Hash = Hexify::hexify(SHA1::getHash(avatar1));
+			user1 = JID("user1@bar.com/bla");
+			user2 = JID("user2@foo.com/baz");
+		}
+
+		void tearDown() {
+			delete vcardManager;
+			delete vcardStorage;
+			delete avatarStorage;
+			delete mucRegistry;
+			delete iqRouter;
+			delete stanzaChannel;
+		}
+
+		void testGetAvatarHashKnownAvatar() {
+			std::auto_ptr<VCardAvatarManager> testling = createManager();
+			storeVCardWithPhoto(user1.toBare(), avatar1);
+			avatarStorage->addAvatar(avatar1Hash, avatar1);
+
+			String result = testling->getAvatarHash(user1);
+
+			CPPUNIT_ASSERT_EQUAL(avatar1Hash, result);
+		}
+
+		void testGetAvatarHashEmptyAvatar() {
+			std::auto_ptr<VCardAvatarManager> testling = createManager();
+			storeEmptyVCard(user1.toBare());
+
+			String result = testling->getAvatarHash(user1);
+
+			CPPUNIT_ASSERT_EQUAL(String(), result);
+		}
+
+		void testGetAvatarHashUnknownAvatarKnownVCardStoresAvatar() {
+			std::auto_ptr<VCardAvatarManager> testling = createManager();
+			storeVCardWithPhoto(user1.toBare(), avatar1);
+
+			String result = testling->getAvatarHash(user1);
+
+			CPPUNIT_ASSERT_EQUAL(avatar1Hash, result);
+			CPPUNIT_ASSERT(avatarStorage->hasAvatar(avatar1Hash));
+			CPPUNIT_ASSERT_EQUAL(avatar1, avatarStorage->getAvatar(avatar1Hash));
+		}
+
+		void testGetAvatarHashUnknownAvatarUnknownVCard() {
+			std::auto_ptr<VCardAvatarManager> testling = createManager();
+
+			String result = testling->getAvatarHash(user1);
+
+			CPPUNIT_ASSERT_EQUAL(String(), result);
+		}
+
+		void testVCardUpdateTriggersUpdate() {
+			std::auto_ptr<VCardAvatarManager> testling = createManager();
+			vcardManager->requestVCard(user1);
+			sendVCardResult();
+
+			CPPUNIT_ASSERT_EQUAL(1, static_cast<int>(changes.size()));
+		}
+
+	private:
+		std::auto_ptr<VCardAvatarManager> createManager() {
+			std::auto_ptr<VCardAvatarManager> result(new VCardAvatarManager(vcardManager, avatarStorage, mucRegistry));
+			result->onAvatarChanged.connect(boost::bind(&VCardAvatarManagerTest::handleAvatarChanged, this, _1));
+			return result;
+		}
+
+		void storeVCardWithPhoto(const JID& jid, const ByteArray& avatar) {
+			VCard::ref vcard(new VCard());
+			vcard->setPhoto(avatar);
+			vcardStorage->setVCard(jid, vcard);
+		}
+
+		void storeEmptyVCard(const JID& jid) {
+			VCard::ref vcard(new VCard());
+			vcardStorage->setVCard(jid, vcard);
+		}
+
+		void handleAvatarChanged(const JID& jid) {
+			changes.push_back(jid);
+		}
+
+		void sendVCardResult() {
+			VCard::ref vcard(new VCard());
+			vcard->setFullName("Foo Bar");
+			stanzaChannel->onIQReceived(IQ::createResult(JID("baz@fum.com/dum"), stanzaChannel->sentStanzas[0]->getID(), vcard));
+		}
+
+	private:
+		struct DummyMUCRegistry : public MUCRegistry {
+			bool isMUC(const JID& jid) const { return std::find(mucs_.begin(), mucs_.end(), jid) != mucs_.end(); }
+			std::vector<JID> mucs_;
+		};
+
+		JID ownJID;
+		DummyStanzaChannel* stanzaChannel;
+		IQRouter* iqRouter;
+		DummyMUCRegistry* mucRegistry;
+		AvatarMemoryStorage* avatarStorage;
+		VCardManager* vcardManager;
+		VCardMemoryStorage* vcardStorage;
+		ByteArray avatar1;
+		String avatar1Hash;
+		std::vector<JID> changes;
+		JID user1;
+		JID user2;
+};
+
+CPPUNIT_TEST_SUITE_REGISTRATION(VCardAvatarManagerTest);
diff --git a/Swiften/Avatars/VCardAvatarManager.cpp b/Swiften/Avatars/VCardAvatarManager.cpp
new file mode 100644
index 0000000..244a73e
--- /dev/null
+++ b/Swiften/Avatars/VCardAvatarManager.cpp
@@ -0,0 +1,50 @@
+/*
+ * Copyright (c) 2010 Remko Tronçon
+ * Licensed under the GNU General Public License v3.
+ * See Documentation/Licenses/GPLv3.txt for more information.
+ */
+
+#include "Swiften/Avatars/VCardAvatarManager.h"
+
+#include <boost/bind.hpp>
+
+#include "Swiften/Elements/VCard.h"
+#include "Swiften/StringCodecs/SHA1.h"
+#include "Swiften/StringCodecs/Hexify.h"
+#include "Swiften/Avatars/AvatarStorage.h"
+#include "Swiften/MUC/MUCRegistry.h"
+#include "Swiften/VCards/VCardManager.h"
+
+namespace Swift {
+
+VCardAvatarManager::VCardAvatarManager(VCardManager* vcardManager, AvatarStorage* avatarStorage, MUCRegistry* mucRegistry) : vcardManager_(vcardManager), avatarStorage_(avatarStorage), mucRegistry_(mucRegistry) {
+	vcardManager_->onVCardChanged.connect(boost::bind(&VCardAvatarManager::handleVCardChanged, this, _1));
+}
+
+void VCardAvatarManager::handleVCardChanged(const JID& from) {
+	// We don't check whether the avatar actually changed. Direct use of this
+	// manager could cause unnecessary updates, but in practice, this will be
+	// caught by the wrapping CombinedAvatarManager anyway.
+	onAvatarChanged(from);
+}
+
+String VCardAvatarManager::getAvatarHash(const JID& jid) const {
+	VCard::ref vCard = vcardManager_->getVCard(getAvatarJID(jid));
+	if (vCard && !vCard->getPhoto().isEmpty()) {
+		String hash = Hexify::hexify(SHA1::getHash(vCard->getPhoto()));
+		if (!avatarStorage_->hasAvatar(hash)) {
+			avatarStorage_->addAvatar(hash, vCard->getPhoto());
+		}
+		return hash;
+	}
+	else {
+		return "";
+	}
+}
+
+JID VCardAvatarManager::getAvatarJID(const JID& jid) const {
+	JID bareFrom = jid.toBare();
+	return (mucRegistry_ && mucRegistry_->isMUC(bareFrom)) ? jid : bareFrom;
+}
+
+}
diff --git a/Swiften/Avatars/VCardAvatarManager.h b/Swiften/Avatars/VCardAvatarManager.h
new file mode 100644
index 0000000..069f938
--- /dev/null
+++ b/Swiften/Avatars/VCardAvatarManager.h
@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2010 Remko Tronçon
+ * Licensed under the GNU General Public License v3.
+ * See Documentation/Licenses/GPLv3.txt for more information.
+ */
+
+#pragma once
+
+#include <map>
+
+#include "Swiften/Avatars/AvatarProvider.h"
+#include "Swiften/JID/JID.h"
+#include "Swiften/Elements/VCard.h"
+
+namespace Swift {
+	class MUCRegistry;
+	class AvatarStorage;
+	class VCardManager;
+
+	class VCardAvatarManager : public AvatarProvider {
+		public:
+			VCardAvatarManager(VCardManager*, AvatarStorage*, MUCRegistry* = NULL);
+
+			String getAvatarHash(const JID&) const;
+
+		private:
+			void handleVCardChanged(const JID& from);
+			JID getAvatarJID(const JID& o) const;
+
+		private:
+			VCardManager* vcardManager_;
+			AvatarStorage* avatarStorage_;
+			MUCRegistry* mucRegistry_;
+	};
+}
diff --git a/Swiften/Avatars/VCardUpdateAvatarManager.cpp b/Swiften/Avatars/VCardUpdateAvatarManager.cpp
index 9d0ae2d..c830b54 100644
--- a/Swiften/Avatars/VCardUpdateAvatarManager.cpp
+++ b/Swiften/Avatars/VCardUpdateAvatarManager.cpp
@@ -25,10 +25,6 @@ VCardUpdateAvatarManager::VCardUpdateAvatarManager(VCardManager* vcardManager, S
 	vcardManager_->onVCardChanged.connect(boost::bind(&VCardUpdateAvatarManager::handleVCardChanged, this, _1, _2));
 }
 
-VCardUpdateAvatarManager::~VCardUpdateAvatarManager() {
-
-}
-
 void VCardUpdateAvatarManager::handlePresenceReceived(boost::shared_ptr<Presence> presence) {
 	boost::shared_ptr<VCardUpdate> update = presence->getPayload<VCardUpdate>();
 	if (!update || presence->getPayload<ErrorPayload>()) {
@@ -57,7 +53,9 @@ void VCardUpdateAvatarManager::handleVCardChanged(const JID& from, VCard::ref vC
 	}
 	else {
 		String hash = Hexify::hexify(SHA1::getHash(vCard->getPhoto()));
-		avatarStorage_->addAvatar(hash, vCard->getPhoto());
+		if (!avatarStorage_->hasAvatar(hash)) {
+			avatarStorage_->addAvatar(hash, vCard->getPhoto());
+		}
 		setAvatarHash(from, hash);
 	}
 }
diff --git a/Swiften/Avatars/VCardUpdateAvatarManager.h b/Swiften/Avatars/VCardUpdateAvatarManager.h
index 8e11223..8827ab7 100644
--- a/Swiften/Avatars/VCardUpdateAvatarManager.h
+++ b/Swiften/Avatars/VCardUpdateAvatarManager.h
@@ -24,7 +24,6 @@ namespace Swift {
 	class VCardUpdateAvatarManager : public AvatarProvider, public boost::bsignals::trackable {
 		public:
 			VCardUpdateAvatarManager(VCardManager*, StanzaChannel*, AvatarStorage*, MUCRegistry* = NULL);
-			virtual ~VCardUpdateAvatarManager();
 
 			String getAvatarHash(const JID&) const;
 
diff --git a/Swiften/Roster/Roster.cpp b/Swiften/Roster/Roster.cpp
index 64a7241..f9f0dbb 100644
--- a/Swiften/Roster/Roster.cpp
+++ b/Swiften/Roster/Roster.cpp
@@ -68,9 +68,10 @@ void Roster::handleChildrenChanged(GroupRosterItem* item) {
 	onChildrenChanged(item);
 }
 
-void Roster::addContact(const JID& jid, const JID& displayJID, const String& name, const String& groupName) {
+void Roster::addContact(const JID& jid, const JID& displayJID, const String& name, const String& groupName, const String& avatarPath) {
 	GroupRosterItem* group(getGroup(groupName));
 	ContactRosterItem *item = new ContactRosterItem(jid, displayJID, name, group);
+	item->setAvatarPath(avatarPath);
 	group->addChild(item);
 	itemMap_[fullJIDMapping_ ? jid : jid.toBare()].push_back(item);
 	item->onDataChanged.connect(boost::bind(&Roster::handleDataChanged, this, item));
diff --git a/Swiften/Roster/Roster.h b/Swiften/Roster/Roster.h
index aa117d7..2f12b57 100644
--- a/Swiften/Roster/Roster.h
+++ b/Swiften/Roster/Roster.h
@@ -28,7 +28,7 @@ class Roster {
 		Roster(bool sortByStatus = true, bool fullJIDMapping = false);
 		~Roster();
 
-		void addContact(const JID& jid, const JID& displayJID, const String& name, const String& group);
+		void addContact(const JID& jid, const JID& displayJID, const String& name, const String& group, const String& avatarPath);
 		void removeContact(const JID& jid);
 		void removeContactFromGroup(const JID& jid, const String& group);
 		void removeAll();
diff --git a/Swiften/Roster/UnitTest/RosterTest.cpp b/Swiften/Roster/UnitTest/RosterTest.cpp
index 5dbe5e1..9c8e65b 100644
--- a/Swiften/Roster/UnitTest/RosterTest.cpp
+++ b/Swiften/Roster/UnitTest/RosterTest.cpp
@@ -43,9 +43,9 @@ class RosterTest : public CppUnit::TestFixture
 		}
 
 		void testGetGroup() {
-			roster_->addContact(jid1_, JID(), "Bert", "group1");
-			roster_->addContact(jid2_, JID(), "Ernie", "group2");
-			roster_->addContact(jid3_, JID(), "Cookie", "group1");
+			roster_->addContact(jid1_, JID(), "Bert", "group1", "");
+			roster_->addContact(jid2_, JID(), "Ernie", "group2", "");
+			roster_->addContact(jid3_, JID(), "Cookie", "group1", "");
 
 			CPPUNIT_ASSERT_EQUAL(2, static_cast<int>(roster_->getRoot()->getChildren().size()));
 			CPPUNIT_ASSERT_EQUAL(String("group1"), roster_->getRoot()->getChildren()[0]->getDisplayName());
@@ -57,7 +57,7 @@ class RosterTest : public CppUnit::TestFixture
 		}
 
 		void testRemoveContact() {
-			roster_->addContact(jid1_, jid1_, "Bert", "group1");
+			roster_->addContact(jid1_, jid1_, "Bert", "group1", "");
 			CPPUNIT_ASSERT_EQUAL(String("Bert"), static_cast<GroupRosterItem*>(roster_->getRoot()->getChildren()[0])->getChildren()[0]->getDisplayName());
 
 			roster_->removeContact(jid1_);
@@ -65,8 +65,8 @@ class RosterTest : public CppUnit::TestFixture
 		}
 
 		void testRemoveSecondContact() {
-			roster_->addContact(jid1_, jid1_, "Bert", "group1");
-			roster_->addContact(jid2_, jid2_, "Cookie", "group1");
+			roster_->addContact(jid1_, jid1_, "Bert", "group1", "");
+			roster_->addContact(jid2_, jid2_, "Cookie", "group1", "");
 			CPPUNIT_ASSERT_EQUAL(String("Cookie"), static_cast<GroupRosterItem*>(roster_->getRoot()->getChildren()[0])->getChildren()[1]->getDisplayName());
 
 			roster_->removeContact(jid2_);
@@ -77,8 +77,8 @@ class RosterTest : public CppUnit::TestFixture
 		void testRemoveSecondContactSameBare() {
 			JID jid4a("a@b/c");
 			JID jid4b("a@b/d");
-			roster_->addContact(jid4a, JID(), "Bert", "group1");
-			roster_->addContact(jid4b, JID(), "Cookie", "group1");
+			roster_->addContact(jid4a, JID(), "Bert", "group1", "");
+			roster_->addContact(jid4b, JID(), "Cookie", "group1", "");
 			CPPUNIT_ASSERT_EQUAL(String("Cookie"), static_cast<GroupRosterItem*>(roster_->getRoot()->getChildren()[0])->getChildren()[1]->getDisplayName());
 
 			roster_->removeContact(jid4b);
@@ -90,11 +90,11 @@ class RosterTest : public CppUnit::TestFixture
 			JID jid4a("a@b/c");
 			JID jid4b("a@b/d");
 			JID jid4c("a@b/e");
-			roster_->addContact(jid4a, JID(), "Bird", "group1");
-			roster_->addContact(jid4b, JID(), "Cookie", "group1");
+			roster_->addContact(jid4a, JID(), "Bird", "group1", "");
+			roster_->addContact(jid4b, JID(), "Cookie", "group1", "");
 			roster_->removeContact(jid4b);
-			roster_->addContact(jid4c, JID(), "Bert", "group1");
-			roster_->addContact(jid4b, JID(), "Ernie", "group1");
+			roster_->addContact(jid4c, JID(), "Bert", "group1", "");
+			roster_->addContact(jid4b, JID(), "Ernie", "group1", "");
 			boost::shared_ptr<Presence> presence(new Presence());
 			presence->setShow(StatusShow::DND);
 			presence->setFrom(jid4a);
diff --git a/Swiften/SConscript b/Swiften/SConscript
index 588bd47..7689ffc 100644
--- a/Swiften/SConscript
+++ b/Swiften/SConscript
@@ -141,6 +141,7 @@ if env["SCONS_STAGE"] == "build" :
 	env.Append(UNITTEST_SOURCES = [
 			File("Application/UnitTest/ApplicationPathProviderTest.cpp"),
 			File("Avatars/UnitTest/VCardUpdateAvatarManagerTest.cpp"),
+			File("Avatars/UnitTest/VCardAvatarManagerTest.cpp"),
 			File("Avatars/UnitTest/CombinedAvatarProviderTest.cpp"),
 			File("Base/UnitTest/IDGeneratorTest.cpp"),
 			File("Base/UnitTest/StringTest.cpp"),
diff --git a/Swiften/VCards/VCardManager.cpp b/Swiften/VCards/VCardManager.cpp
index 628f4c8..673b937 100644
--- a/Swiften/VCards/VCardManager.cpp
+++ b/Swiften/VCards/VCardManager.cpp
@@ -17,6 +17,10 @@ namespace Swift {
 VCardManager::VCardManager(const JID& ownJID, IQRouter* iqRouter, VCardStorage* vcardStorage) : ownJID(ownJID), iqRouter(iqRouter), storage(vcardStorage) {
 }
 
+VCard::ref VCardManager::getVCard(const JID& jid) const {
+	return storage->getVCard(jid);
+}
+
 VCard::ref VCardManager::getVCardAndRequestWhenNeeded(const JID& jid) {
 	VCard::ref vcard = storage->getVCard(jid);
 	if (!vcard) {
diff --git a/Swiften/VCards/VCardManager.h b/Swiften/VCards/VCardManager.h
index 61fc50d..67a2ef8 100644
--- a/Swiften/VCards/VCardManager.h
+++ b/Swiften/VCards/VCardManager.h
@@ -22,6 +22,7 @@ namespace Swift {
 		public:
 			VCardManager(const JID& ownJID, IQRouter* iqRouter, VCardStorage* vcardStorage);
 
+			VCard::ref getVCard(const JID& jid) const;
 			VCard::ref getVCardAndRequestWhenNeeded(const JID& jid);
 			void requestVCard(const JID& jid);
 			void requestOwnVCard();