Members of the KDE Community are recommended to subscribe to the kde-community mailing list at https://mail.kde.org/mailman/listinfo/kde-community to allow them to participate in important discussions and receive other important announcements

Commit 6864141a authored by Volker Krause's avatar Volker Krause

Also remember which avatars we didn't find

Summary:
This massively reduces the amount of network requests, at very little
disk and memory cost (1Mb per 20+k of missing avatars with both backends,
60+k for just Gravatar).

It's even more important when having both backends enabled, in that case
we always did a request for the first backend even if the second one could
provide it from the local cache.

Reviewers: #kde_pim, mlaurent

Reviewed By: #kde_pim, mlaurent

Subscribers: mlaurent

Tags: #kde_pim

Differential Revision: https://phabricator.kde.org/D7438
parent a43207a9
......@@ -19,13 +19,17 @@
#include "gravatarcachetest.h"
#include "../src/misc/gravatarcache.h"
#include "../src/misc/hash.h"
#include <QCryptographicHash>
#include <QPixmap>
#include <QStandardPaths>
#include <qtest.h>
using namespace Gravatar;
Q_DECLARE_METATYPE(Gravatar::Hash)
GravatarCacheTest::GravatarCacheTest(QObject *parent)
: QObject(parent)
{
......@@ -61,11 +65,12 @@ void GravatarCacheTest::shouldChangeCacheValue()
void GravatarCacheTest::testLookup()
{
Hash hash(QCryptographicHash::hash(QByteArray("test@example.com"), QCryptographicHash::Md5), Hash::Md5);
{
GravatarCache cache;
cache.clearAllCache();
bool found = false;
const auto result = cache.loadGravatarPixmap(QStringLiteral("fa1afe1"), found);
const auto result = cache.loadGravatarPixmap(hash, found);
QVERIFY(!found);
QVERIFY(result.isNull());
}
......@@ -75,11 +80,11 @@ void GravatarCacheTest::testLookup()
{
GravatarCache cache;
cache.saveGravatarPixmap(QStringLiteral("fa1afe1"), px);
cache.saveGravatarPixmap(hash, px);
// in-memory cache lookup
bool found = false;
const auto result = cache.loadGravatarPixmap(QStringLiteral("fa1afe1"), found);
const auto result = cache.loadGravatarPixmap(hash, found);
QVERIFY(found);
QVERIFY(!result.isNull());
QCOMPARE(result.size(), QSize(42, 42));
......@@ -89,11 +94,50 @@ void GravatarCacheTest::testLookup()
// disk lookup
GravatarCache cache;
bool found = false;
const auto result = cache.loadGravatarPixmap(QStringLiteral("fa1afe1"), found);
const auto result = cache.loadGravatarPixmap(hash, found);
QVERIFY(found);
QVERIFY(!result.isNull());
QCOMPARE(result.size(), QSize(42, 42));
}
}
void GravatarCacheTest::testMissing_data()
{
QTest::addColumn<Hash>("hash");
QTest::newRow("md5") << Hash(QCryptographicHash::hash(QByteArray("testMD5@example.com"), QCryptographicHash::Md5), Hash::Md5);
QTest::newRow("Sha256") << Hash(QCryptographicHash::hash(QByteArray("testSHA256@example.com"), QCryptographicHash::Sha256), Hash::Sha256);
}
void GravatarCacheTest::testMissing()
{
QFETCH(Hash, hash);
{
GravatarCache cache;
cache.clearAllCache();
bool found = false;
const auto result = cache.loadGravatarPixmap(hash, found);
QVERIFY(!found);
QVERIFY(result.isNull());
}
{
// store miss and verify in memory
GravatarCache cache;
cache.saveMissingGravatar(hash);
bool found = false;
const auto result = cache.loadGravatarPixmap(hash, found);
QVERIFY(found);
QVERIFY(result.isNull());
}
{
// verify miss in disk storage
GravatarCache cache;
bool found = false;
const auto result = cache.loadGravatarPixmap(hash, found);
QVERIFY(found);
QVERIFY(result.isNull());
}
}
QTEST_MAIN(GravatarCacheTest)
......@@ -33,6 +33,8 @@ private Q_SLOTS:
void shouldHaveDefaultValue();
void shouldChangeCacheValue();
void testLookup();
void testMissing();
void testMissing_data();
};
#endif // GRAVATARCACHETEST_H
......@@ -19,6 +19,7 @@
#include "gravatarresolvurljobtest.h"
#include "../src/job/gravatarresolvurljob.h"
#include "../src/misc/hash.h"
#include <qtest.h>
GravatarResolvUrlJobTest::GravatarResolvUrlJobTest(QObject *parent)
......@@ -167,7 +168,7 @@ void GravatarResolvUrlJobTest::shouldGenerateGravatarUrl()
job.setEmail(input);
job.setUseLibravatar(uselibravatar);
QUrl url = job.generateGravatarUrl(job.useLibravatar());
QCOMPARE(calculedhash, job.calculatedHash());
QCOMPARE(calculedhash, job.calculatedHash().hexString());
QCOMPARE(url, output);
}
......
......@@ -4,6 +4,7 @@ add_definitions(-DTRANSLATION_DOMAIN=\"libgravatar\")
set(gravatarlib_SRCS
misc/gravatarcache.cpp
misc/hash.cpp
widgets/gravatardownloadpixmapwidget.cpp
widgets/gravatardownloadpixmapdialog.cpp
widgets/gravatarconfigwidget.cpp
......
......@@ -19,6 +19,7 @@
#include "gravatarresolvurljob.h"
#include "misc/gravatarcache.h"
#include "misc/hash.h"
#include "gravatar_debug.h"
#include <PimCommon/NetworkManager>
......@@ -36,24 +37,27 @@ public:
GravatarResolvUrlJobPrivate()
: mNetworkAccessManager(nullptr)
, mSize(80)
, mBackends(Gravatar)
, mHasGravatar(false)
, mUseDefaultPixmap(false)
, mUseLibravatar(false)
, mFallbackGravatar(true)
, mFallbackDone(false)
{
}
QPixmap mPixmap;
QString mEmail;
QString mCalculatedHash;
Hash mCalculatedHash;
QNetworkAccessManager *mNetworkAccessManager;
int mSize;
enum Backend {
None = 0x0,
Libravatar = 0x1,
Gravatar = 0x2
};
int mBackends;
bool mHasGravatar;
bool mUseDefaultPixmap;
bool mUseLibravatar;
bool mFallbackGravatar;
bool mFallbackDone;
};
GravatarResolvUrlJob::GravatarResolvUrlJob(QObject *parent)
......@@ -88,36 +92,51 @@ bool GravatarResolvUrlJob::hasGravatar() const
void GravatarResolvUrlJob::startNetworkManager(const QUrl &url)
{
if (PimCommon::NetworkManager::self()->networkConfigureManager()->isOnline()) {
if (!d->mNetworkAccessManager) {
d->mNetworkAccessManager = new QNetworkAccessManager(this);
connect(d->mNetworkAccessManager, &QNetworkAccessManager::finished, this, &GravatarResolvUrlJob::slotFinishLoadPixmap);
}
QNetworkReply *reply = d->mNetworkAccessManager->get(QNetworkRequest(url));
connect(reply, SIGNAL(error(QNetworkReply::NetworkError)), this, SLOT(slotError(QNetworkReply::NetworkError)));
} else {
qCDebug(GRAVATAR_LOG) << " network is not connected";
deleteLater();
return;
if (!d->mNetworkAccessManager) {
d->mNetworkAccessManager = new QNetworkAccessManager(this);
connect(d->mNetworkAccessManager, &QNetworkAccessManager::finished, this, &GravatarResolvUrlJob::slotFinishLoadPixmap);
}
d->mNetworkAccessManager->get(QNetworkRequest(url));
}
void GravatarResolvUrlJob::start()
{
if (d->mBackends == GravatarResolvUrlJobPrivate::None)
d->mBackends = GravatarResolvUrlJobPrivate::Gravatar; // default is Gravatar if nothing else is selected
d->mHasGravatar = false;
d->mFallbackDone = false;
if (canStart()) {
d->mCalculatedHash.clear();
const QUrl url = createUrl(d->mUseLibravatar);
Q_EMIT resolvUrl(url);
if (!cacheLookup(d->mCalculatedHash))
startNetworkManager(url);
processNextBackend();
} else {
qCDebug(GRAVATAR_LOG) << "Gravatar can not start";
deleteLater();
}
}
void GravatarResolvUrlJob::processNextBackend()
{
if (d->mHasGravatar || d->mBackends == GravatarResolvUrlJobPrivate::None) {
Q_EMIT finished(this);
deleteLater();
return;
}
QUrl url;
if (d->mBackends & GravatarResolvUrlJobPrivate::Libravatar) {
d->mBackends &= ~GravatarResolvUrlJobPrivate::Libravatar;
url = createUrl(true);
} else if (d->mBackends & GravatarResolvUrlJobPrivate::Gravatar) {
d->mBackends &= ~GravatarResolvUrlJobPrivate::Gravatar;
url = createUrl(false);
}
Q_EMIT resolvUrl(url);
if (!cacheLookup(d->mCalculatedHash))
startNetworkManager(url);
else
processNextBackend();
}
void GravatarResolvUrlJob::slotFinishLoadPixmap(QNetworkReply *reply)
{
if (reply->error() == QNetworkReply::NoError) {
......@@ -127,25 +146,14 @@ void GravatarResolvUrlJob::slotFinishLoadPixmap(QNetworkReply *reply)
if (!d->mUseDefaultPixmap) {
GravatarCache::self()->saveGravatarPixmap(d->mCalculatedHash, d->mPixmap);
}
} else if (d->mUseLibravatar && d->mFallbackGravatar && !d->mFallbackDone) {
d->mFallbackDone = true;
d->mCalculatedHash.clear();
const QUrl url = createUrl(false);
Q_EMIT resolvUrl(url);
if (!cacheLookup(d->mCalculatedHash))
startNetworkManager(url);
return;
} else {
if (reply->error() == QNetworkReply::ContentNotFoundError)
GravatarCache::self()->saveMissingGravatar(d->mCalculatedHash);
else
qCDebug(GRAVATAR_LOG) << "Network error:" << reply->request().url() << reply->errorString();
}
reply->deleteLater();
Q_EMIT finished(this);
deleteLater();
}
void GravatarResolvUrlJob::slotError(QNetworkReply::NetworkError error)
{
if (error == QNetworkReply::ContentNotFoundError) {
d->mHasGravatar = false;
}
processNextBackend();
}
QString GravatarResolvUrlJob::email() const
......@@ -158,31 +166,38 @@ void GravatarResolvUrlJob::setEmail(const QString &email)
d->mEmail = email;
}
QString GravatarResolvUrlJob::calculateHash(bool useLibravator)
Hash GravatarResolvUrlJob::calculateHash(bool useLibravator)
{
QCryptographicHash hash(useLibravator ? QCryptographicHash::Sha256 : QCryptographicHash::Md5);
hash.addData(d->mEmail.toLower().toUtf8());
return QString::fromUtf8(hash.result().toHex());
const auto email = d->mEmail.toLower().toUtf8();
if (useLibravator)
return Hash(QCryptographicHash::hash(email, QCryptographicHash::Sha256), Hash::Sha256);
return Hash(QCryptographicHash::hash(email, QCryptographicHash::Md5), Hash::Md5);
}
bool GravatarResolvUrlJob::fallbackGravatar() const
{
return d->mFallbackGravatar;
return d->mBackends & GravatarResolvUrlJobPrivate::Gravatar;
}
void GravatarResolvUrlJob::setFallbackGravatar(bool fallbackGravatar)
{
d->mFallbackGravatar = fallbackGravatar;
if (fallbackGravatar)
d->mBackends |= GravatarResolvUrlJobPrivate::Gravatar;
else
d->mBackends &= ~GravatarResolvUrlJobPrivate::Gravatar;
}
bool GravatarResolvUrlJob::useLibravatar() const
{
return d->mUseLibravatar;
return d->mBackends & GravatarResolvUrlJobPrivate::Libravatar;
}
void GravatarResolvUrlJob::setUseLibravatar(bool useLibravatar)
{
d->mUseLibravatar = useLibravatar;
if (useLibravatar)
d->mBackends |= GravatarResolvUrlJobPrivate::Libravatar;
else
d->mBackends &= ~GravatarResolvUrlJobPrivate::Libravatar;
}
bool GravatarResolvUrlJob::useDefaultPixmap() const
......@@ -215,7 +230,7 @@ void GravatarResolvUrlJob::setSize(int size)
d->mSize = size;
}
QString GravatarResolvUrlJob::calculatedHash() const
Hash GravatarResolvUrlJob::calculatedHash() const
{
return d->mCalculatedHash;
}
......@@ -223,7 +238,7 @@ QString GravatarResolvUrlJob::calculatedHash() const
QUrl GravatarResolvUrlJob::createUrl(bool useLibravatar)
{
QUrl url;
d->mCalculatedHash.clear();
d->mCalculatedHash = Hash();
if (!canStart()) {
return url;
}
......@@ -243,21 +258,21 @@ QUrl GravatarResolvUrlJob::createUrl(bool useLibravatar)
}
url.setPort(443);
d->mCalculatedHash = calculateHash(useLibravatar);
url.setPath(QLatin1String("/avatar/") + d->mCalculatedHash);
url.setPath(QLatin1String("/avatar/") + d->mCalculatedHash.hexString());
url.setQuery(query);
return url;
}
bool GravatarResolvUrlJob::cacheLookup(const QString &hash)
bool GravatarResolvUrlJob::cacheLookup(const Hash &hash)
{
bool haveStoredPixmap = false;
const QPixmap pix = GravatarCache::self()->loadGravatarPixmap(hash, haveStoredPixmap);
if (haveStoredPixmap && !pix.isNull()) {
if (haveStoredPixmap && !pix.isNull()) { // we know a Gravatar for this hash
d->mPixmap = pix;
d->mHasGravatar = true;
Q_EMIT finished(this);
deleteLater();
return true;
}
return false;
return haveStoredPixmap;
}
......@@ -25,8 +25,12 @@
#include <QUrl>
#include <QPixmap>
#include <QNetworkReply>
class GravatarResolvUrlJobTest;
namespace Gravatar {
class GravatarResolvUrlJobPrivate;
class Hash;
class GRAVATAR_EXPORT GravatarResolvUrlJob : public QObject
{
Q_OBJECT
......@@ -40,14 +44,9 @@ public:
QString email() const;
void setEmail(const QString &email);
QUrl generateGravatarUrl(bool useLibravatar);
bool hasGravatar() const;
QString calculatedHash() const;
void setSize(int size);
int size() const;
QPixmap pixmap() const;
......@@ -67,13 +66,17 @@ Q_SIGNALS:
private Q_SLOTS:
void slotFinishLoadPixmap(QNetworkReply *reply);
void slotError(QNetworkReply::NetworkError error);
private:
friend class ::GravatarResolvUrlJobTest;
QUrl generateGravatarUrl(bool useLibravatar);
Hash calculatedHash() const;
void processNextBackend();
void startNetworkManager(const QUrl &url);
QUrl createUrl(bool useLibravatar);
QString calculateHash(bool useLibravator);
bool cacheLookup(const QString &hash);
Hash calculateHash(bool useLibravator);
bool cacheLookup(const Hash &hash);
GravatarResolvUrlJobPrivate *const d;
};
}
......
......@@ -18,13 +18,19 @@
*/
#include "gravatarcache.h"
#include <QDir>
#include "gravatar_debug.h"
#include "hash.h"
#include <QCache>
#include <QDir>
#include <QFile>
#include <QFileInfo>
#include <QSaveFile>
#include <QStandardPaths>
#include <algorithm>
#include <vector>
using namespace Gravatar;
Q_GLOBAL_STATIC(GravatarCache, s_gravatarCache)
......@@ -32,12 +38,51 @@ Q_GLOBAL_STATIC(GravatarCache, s_gravatarCache)
class Gravatar::GravatarCachePrivate
{
public:
GravatarCachePrivate()
template <typename T>
inline void insertMissingHash(std::vector<T> &vec, const T &hash)
{
auto it = std::lower_bound(vec.begin(), vec.end(), hash);
if (it != vec.end() && *it == hash)
return; // already present (shouldn't happen)
vec.insert(it, hash);
}
template <typename T>
inline void saveVector(const std::vector<T> &vec, const QString &fileName)
{
QSaveFile f(mGravatarPath + fileName);
if (!f.open(QFile::WriteOnly)) {
qCWarning(GRAVATAR_LOG) << "Can't write missing hashes cache file:" << f.fileName() << f.errorString();
return;
}
f.resize(vec.size() * sizeof(T));
f.write(reinterpret_cast<const char*>(vec.data()), vec.size() * sizeof(T));
f.commit();
}
QCache<QString, QPixmap> mCachePixmap;
template <typename T>
inline void loadVector(std::vector<T> &vec, const QString &fileName)
{
if (!vec.empty()) // already loaded
return;
QFile f(mGravatarPath + fileName);
if (!f.open(QFile::ReadOnly))
return; // does not exist yet
if (f.size() % sizeof(T) != 0) {
qCWarning(GRAVATAR_LOG) << "Missing hash cache is corrupt:" << f.fileName();
return;
}
vec.resize(f.size() / sizeof(T));
f.read(reinterpret_cast<char*>(vec.data()), f.size());
}
QCache<Hash, QPixmap> mCachePixmap;
QString mGravatarPath;
std::vector<Hash128> mMd5Misses;
std::vector<Hash256> mSha256Misses;
};
GravatarCache::GravatarCache()
......@@ -59,44 +104,75 @@ GravatarCache *GravatarCache::self()
return s_gravatarCache;
}
void GravatarCache::saveGravatarPixmap(const QString &hashStr, const QPixmap &pixmap)
void GravatarCache::saveGravatarPixmap(const Hash &hash, const QPixmap &pixmap)
{
if (!hashStr.isEmpty() && !pixmap.isNull()) {
if (!d->mCachePixmap.contains(hashStr)) {
const QString path = d->mGravatarPath + hashStr + QLatin1String(".png");
qCDebug(GRAVATAR_LOG) << " path " << path;
if (pixmap.save(path)) {
qCDebug(GRAVATAR_LOG) << " saved in cache " << hashStr << path;
d->mCachePixmap.insert(hashStr, new QPixmap(pixmap));
}
}
if (!hash.isValid() || pixmap.isNull())
return;
const QString path = d->mGravatarPath + hash.hexString() + QLatin1String(".png");
qCDebug(GRAVATAR_LOG) << " path " << path;
if (pixmap.save(path)) {
qCDebug(GRAVATAR_LOG) << " saved in cache " << path;
d->mCachePixmap.insert(hash, new QPixmap(pixmap));
}
}
void GravatarCache::saveMissingGravatar(const Hash &hash)
{
switch (hash.type()) {
case Hash::Invalid:
break;
case Hash::Md5:
d->insertMissingHash(d->mMd5Misses, hash.md5());
d->saveVector(d->mMd5Misses, QStringLiteral("missing.md5"));
break;
case Hash::Sha256:
d->insertMissingHash(d->mSha256Misses, hash.sha256());
d->saveVector(d->mSha256Misses, QStringLiteral("missing.sha256"));
break;
}
}
QPixmap GravatarCache::loadGravatarPixmap(const QString &hashStr, bool &gravatarStored)
QPixmap GravatarCache::loadGravatarPixmap(const Hash &hash, bool &gravatarStored)
{
gravatarStored = false;
qCDebug(GRAVATAR_LOG) << " hashStr" << hashStr;
if (!hashStr.isEmpty()) {
if (d->mCachePixmap.contains(hashStr)) {
qCDebug(GRAVATAR_LOG) << " contains in cache " << hashStr;
qCDebug(GRAVATAR_LOG) << " hashStr" << hash.hexString();
if (!hash.isValid())
return QPixmap();
// in-memory cache
if (d->mCachePixmap.contains(hash)) {
qCDebug(GRAVATAR_LOG) << " contains in cache " << hash.hexString();
gravatarStored = true;
return *(d->mCachePixmap.object(hash));
}
// file-system cache
const QString path = d->mGravatarPath + hash.hexString() + QLatin1String(".png");
if (QFileInfo::exists(path)) {
QPixmap pix;
if (pix.load(path)) {
qCDebug(GRAVATAR_LOG) << " add to cache " << hash.hexString() << path;
d->mCachePixmap.insert(hash, new QPixmap(pix));
gravatarStored = true;
return *(d->mCachePixmap.object(hashStr));
} else {
const QString path = d->mGravatarPath + hashStr + QLatin1String(".png");
if (QFileInfo::exists(path)) {
QPixmap pix;
if (pix.load(path)) {
qCDebug(GRAVATAR_LOG) << " add to cache " << hashStr << path;
d->mCachePixmap.insert(hashStr, new QPixmap(pix));
gravatarStored = true;
return pix;
}
} else {
return QPixmap();
}
return pix;
}
}
// missing gravatar cache (ie. known to not exist one)
switch (hash.type()) {
case Hash::Invalid:
break;
case Hash::Md5:
d->loadVector(d->mMd5Misses, QStringLiteral("missing.md5"));
gravatarStored = std::binary_search(d->mMd5Misses.begin(), d->mMd5Misses.end(), hash.md5());
break;
case Hash::Sha256:
d->loadVector(d->mSha256Misses, QStringLiteral("missing.sha256"));
gravatarStored = std::binary_search(d->mSha256Misses.begin(), d->mSha256Misses.end(), hash.sha256());
break;
}
return QPixmap();
}
......@@ -130,4 +206,6 @@ void GravatarCache::clearAllCache()
}
}
clear();
d->mMd5Misses.clear();
d->mSha256Misses.clear();
}
......@@ -26,6 +26,8 @@
namespace Gravatar {
class GravatarCachePrivate;
class Hash;
class GRAVATAR_EXPORT GravatarCache
{
public:
......@@ -34,9 +36,10 @@ public:
GravatarCache();
~GravatarCache();
void saveGravatarPixmap(const QString &hashStr, const QPixmap &pixmap);
void saveGravatarPixmap(const Hash &hash, const QPixmap &pixmap);
void saveMissingGravatar(const Hash &hash);
QPixmap loadGravatarPixmap(const QString &hashStr, bool &gravatarStored);
QPixmap loadGravatarPixmap(const Hash &hash, bool &gravatarStored);
int maximumSize() const;
void setMaximumSize(int maximumSize);
......
/*
Copyright (c) 2017 Volker Krause <vkrause@kde.org>
This library is free software; you can redistribute it and/or
modify it under the terms of the GNU Library General Public
License as published by the Free Software Foundation; either
version 2 of the License, or (at your option) any later version.
This library is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
Library General Public License for more details.
You should have received a copy of the GNU Library General Public License
along with this library; see the file COPYING.LIB. If not, write to
the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
Boston, MA 02110-1301, USA.
*/
#include "hash.h"
#include <QHash>
#include <QString>
using namespace Gravatar;
Hash::Hash()
: m_type(Invalid)
{
}
Hash::Hash(const QByteArray &data, Type type)
: m_type(type)
{
switch (type) {
case Invalid:
break;
case Md5:
Q_ASSERT(sizeof(Hash128) == data.size());
m_hash.md5 = *reinterpret_cast<const Hash128*>(data.constData());
break;
case Sha256:
Q_ASSERT(sizeof(Hash256) == data.size());
m_hash.sha256 = *reinterpret_cast<const Hash256*>(data.constData());
break;
}
}
bool Hash::operator==(const Hash &other) const
{
if (m_type != other.m_type)
return false;
switch (m_type) {
case Invalid:
return true;
case Md5:
return m_hash.md5 == other.m_hash.md5;
case Sha256:
return m_hash.sha256 == other.m_hash.sha256;