diff --git a/cockatrice/src/client/ui/picture_loader/picture_loader_worker.cpp b/cockatrice/src/client/ui/picture_loader/picture_loader_worker.cpp index a8b5dd6a0..b0ab0b4c0 100644 --- a/cockatrice/src/client/ui/picture_loader/picture_loader_worker.cpp +++ b/cockatrice/src/client/ui/picture_loader/picture_loader_worker.cpp @@ -12,9 +12,6 @@ #include #include -// Card back returned by gatherer when card is not found -QStringList PictureLoaderWorker::md5Blacklist = QStringList() << "db0c48db407a907c16ade38de048a441"; - PictureLoaderWorker::PictureLoaderWorker() : QObject(nullptr), picDownload(SettingsCache::instance().getPicDownload()) { networkManager = new QNetworkAccessManager(this); @@ -68,6 +65,7 @@ void PictureLoaderWorker::queueRequest(const QUrl &url, PictureLoaderWorkerWork if (!cachedRedirect.isEmpty()) { queueRequest(cachedRedirect, worker); } else if (cache->metaData(url).isValid()) { + // If we hit a cached url, we get to make the request for free, since it won't contribute towards the rate-limit makeRequest(url, worker); } else { requestLoadQueue.append(qMakePair(url, worker)); @@ -92,34 +90,7 @@ QNetworkReply *PictureLoaderWorker::makeRequest(const QUrl &url, PictureLoaderWo QNetworkReply *reply = networkManager->get(req); // Connect reply handling - connect(reply, &QNetworkReply::finished, this, [this, reply, url, worker]() { - QVariant redirectTarget = reply->attribute(QNetworkRequest::RedirectionTargetAttribute); - if (redirectTarget.isValid()) { - QUrl redirectUrl = redirectTarget.toUrl(); - if (redirectUrl.isRelative()) { - redirectUrl = url.resolved(redirectUrl); - } - cacheRedirect(url, redirectUrl); - } - - if (reply->error() == QNetworkReply::NoError) { - worker->picDownloadFinished(reply); - emit imageLoadSuccessful(url, worker); - - // If we hit a cached image, we get to make another request for free. - if (reply->attribute(QNetworkRequest::SourceIsFromCacheAttribute).toBool()) { - if (!requestLoadQueue.isEmpty()) { - auto request = requestLoadQueue.takeFirst(); - makeRequest(request.first, request.second); - } - } - } else if (reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt() == 429) { - qInfo() << "Too many requests."; - } else { - worker->picDownloadFinished(reply); - } - reply->deleteLater(); - }); + connect(reply, &QNetworkReply::finished, worker, [reply, worker] { worker->handleNetworkReply(reply); }); return reply; } @@ -127,10 +98,18 @@ QNetworkReply *PictureLoaderWorker::makeRequest(const QUrl &url, PictureLoaderWo void PictureLoaderWorker::processQueuedRequests() { for (int i = 0; i < 10; i++) { - if (!requestLoadQueue.isEmpty()) { - auto request = requestLoadQueue.takeFirst(); - makeRequest(request.first, request.second); - } + processSingleRequest(); + } +} + +/** + * Immediately processes a single queued request + */ +void PictureLoaderWorker::processSingleRequest() +{ + if (!requestLoadQueue.isEmpty()) { + auto request = requestLoadQueue.takeFirst(); + makeRequest(request.first, request.second); } } @@ -176,6 +155,11 @@ void PictureLoaderWorker::cacheRedirect(const QUrl &originalUrl, const QUrl &red // saveRedirectCache(); } +void PictureLoaderWorker::removedCachedUrl(const QUrl &url) +{ + networkManager->cache()->remove(url); +} + QUrl PictureLoaderWorker::getCachedRedirect(const QUrl &originalUrl) const { if (redirectCache.contains(originalUrl)) { diff --git a/cockatrice/src/client/ui/picture_loader/picture_loader_worker.h b/cockatrice/src/client/ui/picture_loader/picture_loader_worker.h index a4ddb7197..2f0a737ad 100644 --- a/cockatrice/src/client/ui/picture_loader/picture_loader_worker.h +++ b/cockatrice/src/client/ui/picture_loader/picture_loader_worker.h @@ -38,11 +38,12 @@ public: public slots: QNetworkReply *makeRequest(const QUrl &url, PictureLoaderWorkerWork *workThread); void processQueuedRequests(); + void processSingleRequest(); void imageLoadedSuccessfully(const CardInfoPtr &card, const QImage &image); + void cacheRedirect(const QUrl &originalUrl, const QUrl &redirectUrl); + void removedCachedUrl(const QUrl &url); private: - static QStringList md5Blacklist; - QThread *pictureLoaderThread; QNetworkAccessManager *networkManager; QNetworkDiskCache *cache; @@ -56,7 +57,6 @@ private: PictureLoaderLocal *localLoader; QSet currentlyLoading; // for deduplication purposes - void cacheRedirect(const QUrl &originalUrl, const QUrl &redirectUrl); QUrl getCachedRedirect(const QUrl &originalUrl) const; void loadRedirectCache(); void saveRedirectCache() const; diff --git a/cockatrice/src/client/ui/picture_loader/picture_loader_worker_work.cpp b/cockatrice/src/client/ui/picture_loader/picture_loader_worker_work.cpp index 533f4c116..712f68dc1 100644 --- a/cockatrice/src/client/ui/picture_loader/picture_loader_worker_work.cpp +++ b/cockatrice/src/client/ui/picture_loader/picture_loader_worker_work.cpp @@ -19,10 +19,10 @@ PictureLoaderWorkerWork::PictureLoaderWorkerWork(const PictureLoaderWorker *work : QObject(nullptr), cardToDownload(toLoad), picDownload(SettingsCache::instance().getPicDownload()) { // Hook up signals to the orchestrator - connect(this, &PictureLoaderWorkerWork::requestImageDownload, worker, &PictureLoaderWorker::queueRequest, - Qt::QueuedConnection); - connect(this, &PictureLoaderWorkerWork::imageLoaded, worker, &PictureLoaderWorker::imageLoadedSuccessfully, - Qt::QueuedConnection); + connect(this, &PictureLoaderWorkerWork::requestImageDownload, worker, &PictureLoaderWorker::queueRequest); + connect(this, &PictureLoaderWorkerWork::urlRedirected, worker, &PictureLoaderWorker::cacheRedirect); + connect(this, &PictureLoaderWorkerWork::cachedUrlInvalidated, worker, &PictureLoaderWorker::removedCachedUrl); + connect(this, &PictureLoaderWorkerWork::imageLoaded, worker, &PictureLoaderWorker::imageLoadedSuccessfully); // Hook up signals to settings connect(&SettingsCache::instance(), SIGNAL(picDownloadChanged()), this, SLOT(picDownloadChanged())); @@ -44,7 +44,6 @@ void PictureLoaderWorkerWork::startNextPicDownload() QString picUrl = cardToDownload.getCurrentUrl(); if (picUrl.isEmpty()) { - downloadRunning = false; picDownloadFailed(); } else { QUrl url(picUrl); @@ -78,24 +77,51 @@ void PictureLoaderWorkerWork::picDownloadFailed() } } +/** + * + * @param reply The reply. Takes ownership of the object + */ +void PictureLoaderWorkerWork::handleNetworkReply(QNetworkReply *reply) +{ + QVariant redirectTarget = reply->attribute(QNetworkRequest::RedirectionTargetAttribute); + if (redirectTarget.isValid()) { + QUrl url = reply->request().url(); + QUrl redirectUrl = redirectTarget.toUrl(); + if (redirectUrl.isRelative()) { + redirectUrl = url.resolved(redirectUrl); + } + emit urlRedirected(url, redirectUrl); + } + + if (reply->error()) { + handleFailedReply(reply); + } else { + handleSuccessfulReply(reply); + } + + reply->deleteLater(); +} + static bool imageIsBlackListed(const QByteArray &picData) { QString md5sum = QCryptographicHash::hash(picData, QCryptographicHash::Md5).toHex(); return MD5_BLACKLIST.contains(md5sum); } -void PictureLoaderWorkerWork::picDownloadFinished(QNetworkReply *reply) +void PictureLoaderWorkerWork::handleFailedReply(const QNetworkReply *reply) { - bool isFromCache = reply->attribute(QNetworkRequest::SourceIsFromCacheAttribute).toBool(); + if (reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt() == 429) { + qCWarning(PictureLoaderWorkerWorkLog) << "Too many requests."; + } else { + bool isFromCache = reply->attribute(QNetworkRequest::SourceIsFromCacheAttribute).toBool(); - if (reply->error()) { if (isFromCache) { qCDebug(PictureLoaderWorkerWorkLog).nospace() << "PictureLoader: [card: " << cardToDownload.getCard()->getName() << " set: " << cardToDownload.getSetName() << "]: Removing corrupted cache file for url " << reply->url().toDisplayString() << " and retrying (" << reply->errorString() << ")"; - networkManager->cache()->remove(reply->url()); + emit cachedUrlInvalidated(reply->url()); emit requestImageDownload(reply->url(), this); } else { @@ -106,10 +132,12 @@ void PictureLoaderWorkerWork::picDownloadFinished(QNetworkReply *reply) picDownloadFailed(); } - - reply->deleteLater(); - return; } +} + +void PictureLoaderWorkerWork::handleSuccessfulReply(QNetworkReply *reply) +{ + bool isFromCache = reply->attribute(QNetworkRequest::SourceIsFromCacheAttribute).toBool(); // List of status codes from https://doc.qt.io/qt-6/qnetworkreply.html#redirected int statusCode = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(); @@ -120,7 +148,6 @@ void PictureLoaderWorkerWork::picDownloadFinished(QNetworkReply *reply) << "PictureLoader: [card: " << cardToDownload.getCard()->getName() << " set: " << cardToDownload.getSetName() << "]: following " << (isFromCache ? "cached redirect" : "redirect") << " to " << redirectUrl.toDisplayString(); - reply->deleteLater(); emit requestImageDownload(redirectUrl, this); return; } @@ -134,7 +161,6 @@ void PictureLoaderWorkerWork::picDownloadFinished(QNetworkReply *reply) << " set: " << cardToDownload.getSetName() << "]: Picture found, but blacklisted, will consider it as not found"; - reply->deleteLater(); picDownloadFailed(); return; } @@ -147,7 +173,6 @@ void PictureLoaderWorkerWork::picDownloadFinished(QNetworkReply *reply) << " set: " << cardToDownload.getSetName() << "]: Possible " << (isFromCache ? "cached" : "downloaded") << " picture at " << reply->url().toDisplayString() << " could not be loaded: " << reply->errorString(); - reply->deleteLater(); picDownloadFailed(); } else { qCDebug(PictureLoaderWorkerWorkLog).nospace() @@ -155,7 +180,6 @@ void PictureLoaderWorkerWork::picDownloadFinished(QNetworkReply *reply) << " set: " << cardToDownload.getSetName() << "]: Image successfully " << (isFromCache ? "loaded from cached" : "downloaded from") << " url " << reply->url().toDisplayString(); - reply->deleteLater(); concludeImageLoad(image); } } diff --git a/cockatrice/src/client/ui/picture_loader/picture_loader_worker_work.h b/cockatrice/src/client/ui/picture_loader/picture_loader_worker_work.h index 8bccecd6d..9e79a72d5 100644 --- a/cockatrice/src/client/ui/picture_loader/picture_loader_worker_work.h +++ b/cockatrice/src/client/ui/picture_loader/picture_loader_worker_work.h @@ -30,15 +30,16 @@ public: PictureToLoad cardToDownload; public slots: - void picDownloadFinished(QNetworkReply *reply); + void handleNetworkReply(QNetworkReply *reply); private: QThread *pictureLoaderThread; - QNetworkAccessManager *networkManager; - bool picDownload, downloadRunning, loadQueueRunning; + bool picDownload; void startNextPicDownload(); void picDownloadFailed(); + void handleFailedReply(const QNetworkReply *reply); + void handleSuccessfulReply(QNetworkReply *reply); QImage tryLoadImageFromReply(QNetworkReply *reply); void concludeImageLoad(const QImage &image); @@ -53,6 +54,9 @@ signals: */ void imageLoaded(CardInfoPtr card, const QImage &image); void requestImageDownload(const QUrl &url, PictureLoaderWorkerWork *instance); + + void urlRedirected(const QUrl &originalUrl, const QUrl &redirectUrl); + void cachedUrlInvalidated(const QUrl &url); }; #endif // PICTURE_LOADER_WORKER_WORK_H