From 3a39980d887a874e571d59ac0cebda103300c3a5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Remko=20Tron=C3=A7on?= <git@el-tramo.be>
Date: Sat, 12 Mar 2011 00:04:47 +0100
Subject: Fixed unnecessary updating and sending out VCard photo hash.


diff --git a/Swift/Controllers/MainController.cpp b/Swift/Controllers/MainController.cpp
index d9e177b..c4ed28e 100644
--- a/Swift/Controllers/MainController.cpp
+++ b/Swift/Controllers/MainController.cpp
@@ -561,9 +561,12 @@ void MainController::handleVCardReceived(const JID& jid, VCard::ref vCard) {
 	if (!jid.equals(jid_, JID::WithoutResource) || !vCard || vCard->getPhoto().isEmpty()) {
 		return;
 	}
-	vCardPhotoHash_ = Hexify::hexify(SHA1::getHash(vCard->getPhoto()));
-	if (client_ && client_->isAvailable()) {
-		sendPresence(statusTracker_->getNextPresence());
+	std::string hash = Hexify::hexify(SHA1::getHash(vCard->getPhoto()));
+	if (hash != vCardPhotoHash_) {
+		vCardPhotoHash_ = hash;
+		if (client_ && client_->isAvailable()) {
+			sendPresence(statusTracker_->getNextPresence());
+		}
 	}
 }
 
diff --git a/Swiften/Avatars/CombinedAvatarProvider.cpp b/Swiften/Avatars/CombinedAvatarProvider.cpp
index ae0fde3..d7e716c 100644
--- a/Swiften/Avatars/CombinedAvatarProvider.cpp
+++ b/Swiften/Avatars/CombinedAvatarProvider.cpp
@@ -14,16 +14,13 @@
 namespace Swift {
 
 std::string CombinedAvatarProvider::getAvatarHash(const JID& jid) const {
-	SWIFT_LOG(debug) << "JID: " << jid << std::endl;
-	for (size_t i = 0; i < providers.size(); ++i) {
-		std::string hash = providers[i]->getAvatarHash(jid);
-		SWIFT_LOG(debug) << "Provider " << providers[i] << ": " << hash << std::endl;
-		if (!hash.empty()) {
-			return hash;
-		}
+	std::map<JID, std::string>::const_iterator i = avatars.find(jid);
+	if (i == avatars.end()) {
+		return getCombinedAvatarAndCache(jid);
+	}
+	else {
+		return i->second;
 	}
-	SWIFT_LOG(debug) << "No hash found" << std::endl;
-	return std::string();
 }
 
 void CombinedAvatarProvider::addProvider(AvatarProvider* provider) {
@@ -40,23 +37,27 @@ void CombinedAvatarProvider::removeProvider(AvatarProvider* provider) {
 }
 
 void CombinedAvatarProvider::handleAvatarChanged(const JID& jid) {
-	std::string hash = getAvatarHash(jid);
-	std::map<JID, std::string>::iterator i = avatars.find(jid);
+	std::string oldHash;
+	std::map<JID, std::string>::const_iterator i = avatars.find(jid);
 	if (i != avatars.end()) {
-		if (i->second != hash) {
-			if (hash.empty()) {
-				avatars.erase(i);
-			}
-			else {
-				avatars.insert(std::make_pair(jid, hash));
-			}
-			onAvatarChanged(jid);
-		}
+		oldHash = i->second;
 	}
-	else if (!hash.empty()) {
-		avatars.insert(std::make_pair(jid, hash));
+	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 {
+	SWIFT_LOG(debug) << "JID: " << jid << std::endl;
+	std::string hash;
+	for (size_t i = 0; i < providers.size() && hash.empty(); ++i) {
+		hash = providers[i]->getAvatarHash(jid);
+		SWIFT_LOG(debug) << "Provider " << providers[i] << ": " << hash << std::endl;
+	}
+	avatars[jid] = hash;
+	return hash;
+}
+
 }
diff --git a/Swiften/Avatars/CombinedAvatarProvider.h b/Swiften/Avatars/CombinedAvatarProvider.h
index 9c83732..72e67cf 100644
--- a/Swiften/Avatars/CombinedAvatarProvider.h
+++ b/Swiften/Avatars/CombinedAvatarProvider.h
@@ -22,9 +22,10 @@ namespace Swift {
 
 		private:
 			void handleAvatarChanged(const JID&);
+			std::string getCombinedAvatarAndCache(const JID&) const;
 
 		private:
 			std::vector<AvatarProvider*> providers;
-			std::map<JID, std::string> avatars;
+			mutable std::map<JID, std::string> avatars;
 	};
 }
diff --git a/Swiften/Avatars/UnitTest/CombinedAvatarProviderTest.cpp b/Swiften/Avatars/UnitTest/CombinedAvatarProviderTest.cpp
index 6153d29..3e1cba3 100644
--- a/Swiften/Avatars/UnitTest/CombinedAvatarProviderTest.cpp
+++ b/Swiften/Avatars/UnitTest/CombinedAvatarProviderTest.cpp
@@ -25,6 +25,7 @@ class CombinedAvatarProviderTest : public CppUnit::TestFixture {
 		CPPUNIT_TEST(testProviderSecondUpdateTriggersChange);
 		CPPUNIT_TEST(testProviderUpdateWithAvatarDisappearingTriggersChange);
 		CPPUNIT_TEST(testProviderUpdateAfterAvatarDisappearedTriggersChange);
+		CPPUNIT_TEST(testProviderUpdateAfterGetDoesNotTriggerChange);
 		CPPUNIT_TEST(testRemoveProviderDisconnectsUpdates);
 		CPPUNIT_TEST_SUITE_END();
 
@@ -143,6 +144,18 @@ class CombinedAvatarProviderTest : public CppUnit::TestFixture {
 			CPPUNIT_ASSERT_EQUAL(user1, changes[0]);
 		}
 
+
+		void testProviderUpdateAfterGetDoesNotTriggerChange() {
+			std::auto_ptr<CombinedAvatarProvider> testling(createProvider());
+			testling->addProvider(avatarProvider1);
+			avatarProvider1->avatars[user1] = avatarHash1;
+
+			testling->getAvatarHash(user1);
+			avatarProvider1->onAvatarChanged(user1);
+
+			CPPUNIT_ASSERT_EQUAL(0, static_cast<int>(changes.size()));
+		}
+
 		void testRemoveProviderDisconnectsUpdates() {
 			std::auto_ptr<CombinedAvatarProvider> testling(createProvider());
 			testling->addProvider(avatarProvider1);
-- 
cgit v0.10.2-6-g49f6