diff options
author | Remko Tronçon <git@el-tramo.be> | 2011-03-11 23:04:47 (GMT) |
---|---|---|
committer | Remko Tronçon <git@el-tramo.be> | 2011-03-11 23:04:47 (GMT) |
commit | 3a39980d887a874e571d59ac0cebda103300c3a5 (patch) | |
tree | 9135075403ac35d39b9f638c4a1914177ff3c525 | |
parent | 1e63ca24edf35e5681653189fd6ca107627946f4 (diff) | |
download | swift-contrib-3a39980d887a874e571d59ac0cebda103300c3a5.zip swift-contrib-3a39980d887a874e571d59ac0cebda103300c3a5.tar.bz2 |
Fixed unnecessary updating and sending out VCard photo hash.
-rw-r--r-- | Swift/Controllers/MainController.cpp | 9 | ||||
-rw-r--r-- | Swiften/Avatars/CombinedAvatarProvider.cpp | 45 | ||||
-rw-r--r-- | Swiften/Avatars/CombinedAvatarProvider.h | 3 | ||||
-rw-r--r-- | Swiften/Avatars/UnitTest/CombinedAvatarProviderTest.cpp | 13 |
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); |