summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--Swift/Controllers/MainController.cpp9
-rw-r--r--Swiften/Avatars/CombinedAvatarProvider.cpp45
-rw-r--r--Swiften/Avatars/CombinedAvatarProvider.h3
-rw-r--r--Swiften/Avatars/UnitTest/CombinedAvatarProviderTest.cpp13
4 files changed, 44 insertions, 26 deletions
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);