summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRemko Tronçon <git@el-tramo.be>2011-03-11 23:04:47 (GMT)
committerRemko Tronçon <git@el-tramo.be>2011-03-11 23:04:47 (GMT)
commit3a39980d887a874e571d59ac0cebda103300c3a5 (patch)
tree9135075403ac35d39b9f638c4a1914177ff3c525 /Swiften/Avatars
parent1e63ca24edf35e5681653189fd6ca107627946f4 (diff)
downloadswift-3a39980d887a874e571d59ac0cebda103300c3a5.zip
swift-3a39980d887a874e571d59ac0cebda103300c3a5.tar.bz2
Fixed unnecessary updating and sending out VCard photo hash.
Diffstat (limited to 'Swiften/Avatars')
-rw-r--r--Swiften/Avatars/CombinedAvatarProvider.cpp45
-rw-r--r--Swiften/Avatars/CombinedAvatarProvider.h3
-rw-r--r--Swiften/Avatars/UnitTest/CombinedAvatarProviderTest.cpp13
3 files changed, 38 insertions, 23 deletions
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);