diff --git a/src/covermanager/albumcoverchoicecontroller.cpp b/src/covermanager/albumcoverchoicecontroller.cpp index f4cede4b0..9825ccf64 100644 --- a/src/covermanager/albumcoverchoicecontroller.cpp +++ b/src/covermanager/albumcoverchoicecontroller.cpp @@ -242,7 +242,7 @@ void AlbumCoverChoiceController::ShowCover(const Song &song) { void AlbumCoverChoiceController::SearchCoverAutomatically(const Song &song) { - qint64 id = cover_fetcher_->FetchAlbumCover(song.effective_albumartist(), song.effective_album()); + qint64 id = cover_fetcher_->FetchAlbumCover(song.effective_albumartist(), song.effective_album(), false); cover_fetching_tasks_[id] = song; diff --git a/src/covermanager/albumcoverfetcher.cpp b/src/covermanager/albumcoverfetcher.cpp index e7e01fc7b..fdf26a81f 100644 --- a/src/covermanager/albumcoverfetcher.cpp +++ b/src/covermanager/albumcoverfetcher.cpp @@ -41,7 +41,7 @@ AlbumCoverFetcher::AlbumCoverFetcher(CoverProviders *cover_providers, QObject *p connect(request_starter_, SIGNAL(timeout()), SLOT(StartRequests())); } -quint64 AlbumCoverFetcher::FetchAlbumCover(const QString &artist, const QString &album) { +quint64 AlbumCoverFetcher::FetchAlbumCover(const QString &artist, const QString &album, bool fetchall) { QString album2(album); album2 = album2.remove(QRegExp(" ?-? ((\\(|\\[)?)(Disc|CD) ?([0-9]{1,2})((\\)|\\])?)$")); @@ -51,6 +51,7 @@ quint64 AlbumCoverFetcher::FetchAlbumCover(const QString &artist, const QString request.album = album2; request.search = false; request.id = next_id_++; + request.fetchall = fetchall; AddRequest(request); return request.id; @@ -59,8 +60,6 @@ quint64 AlbumCoverFetcher::FetchAlbumCover(const QString &artist, const QString quint64 AlbumCoverFetcher::SearchForCovers(const QString &artist, const QString &album) { - fetchall_ = false; - QString album2(album); album2 = album2.remove(QRegExp(" ?-? ((\\(|\\[)?)(Disc|CD) ?([0-9]{1,2})((\\)|\\])?)$")); @@ -69,6 +68,7 @@ quint64 AlbumCoverFetcher::SearchForCovers(const QString &artist, const QString request.album = album2; request.search = true; request.id = next_id_++; + request.fetchall = false; AddRequest(request); return request.id; @@ -110,7 +110,6 @@ void AlbumCoverFetcher::StartRequests() { // search objects are this fetcher's children so worst case scenario - they get deleted with it AlbumCoverFetcherSearch *search = new AlbumCoverFetcherSearch(request, network_, this); - search->fetchall_ = fetchall_; active_requests_.insert(request.id, search); connect(search, SIGNAL(SearchFinished(quint64, CoverSearchResults)), SLOT(SingleSearchFinished(quint64, CoverSearchResults))); diff --git a/src/covermanager/albumcoverfetcher.h b/src/covermanager/albumcoverfetcher.h index c3dbc76ee..4846badab 100644 --- a/src/covermanager/albumcoverfetcher.h +++ b/src/covermanager/albumcoverfetcher.h @@ -51,6 +51,9 @@ struct CoverSearchRequest { // is this only a search request or should we also fetch the first cover that's found? bool search; + + // is the request part of fetchall (fetching all missing covers) + bool fetchall; }; // This structure represents a single result of some album's cover search request. @@ -84,11 +87,9 @@ class AlbumCoverFetcher : public QObject { static const int kMaxConcurrentRequests; quint64 SearchForCovers(const QString &artist, const QString &album); - quint64 FetchAlbumCover(const QString &artist, const QString &album); + quint64 FetchAlbumCover(const QString &artist, const QString &album, bool fetchall); void Clear(); - - bool fetchall_ = false; signals: void AlbumCoverFetched(quint64, const QImage &cover, const CoverSearchStatistics &statistics); diff --git a/src/covermanager/albumcoverfetchersearch.cpp b/src/covermanager/albumcoverfetchersearch.cpp index cb72e8e4b..bd0e82fa5 100644 --- a/src/covermanager/albumcoverfetchersearch.cpp +++ b/src/covermanager/albumcoverfetchersearch.cpp @@ -69,7 +69,7 @@ void AlbumCoverFetcherSearch::Start(CoverProviders *cover_providers) { for (CoverProvider *provider : cover_providers->List()) { // Skip provider if it does not have fetchall set, and we are doing fetchall - "Fetch Missing Covers". - if ((provider->fetchall() == false) && (fetchall_ == true)) { + if (!provider->fetchall() && request_.fetchall) { //qLog(Debug) << "Skipping provider" << provider->name(); continue; } diff --git a/src/covermanager/albumcoverfetchersearch.h b/src/covermanager/albumcoverfetchersearch.h index e7637addb..00fc0fbc2 100644 --- a/src/covermanager/albumcoverfetchersearch.h +++ b/src/covermanager/albumcoverfetchersearch.h @@ -52,8 +52,6 @@ class AlbumCoverFetcherSearch : public QObject { CoverSearchStatistics statistics() const { return statistics_; } - bool fetchall_ = false; - signals: // It's the end of search (when there was no fetch-me-a-cover request). void SearchFinished(quint64, const CoverSearchResults& results); diff --git a/src/covermanager/albumcovermanager.cpp b/src/covermanager/albumcovermanager.cpp index 3cc9b4700..e521d4a02 100644 --- a/src/covermanager/albumcovermanager.cpp +++ b/src/covermanager/albumcovermanager.cpp @@ -416,14 +416,12 @@ bool AlbumCoverManager::ShouldHide(const QListWidgetItem &item, const QString &f void AlbumCoverManager::FetchAlbumCovers() { - cover_fetcher_->fetchall_ = true; - for (int i = 0; i < ui_->albums->count(); ++i) { QListWidgetItem *item = ui_->albums->item(i); if (item->isHidden()) continue; if (ItemHasCover(*item)) continue; - quint64 id = cover_fetcher_->FetchAlbumCover(EffectiveAlbumArtistName(*item), item->data(Role_AlbumName).toString()); + quint64 id = cover_fetcher_->FetchAlbumCover(EffectiveAlbumArtistName(*item), item->data(Role_AlbumName).toString(), true); cover_fetching_tasks_[id] = item; jobs_++; } @@ -557,10 +555,8 @@ void AlbumCoverManager::ShowCover() { void AlbumCoverManager::FetchSingleCover() { - cover_fetcher_->fetchall_ = false; - for (QListWidgetItem *item : context_menu_items_) { - quint64 id = cover_fetcher_->FetchAlbumCover(EffectiveAlbumArtistName(*item), item->data(Role_AlbumName).toString()); + quint64 id = cover_fetcher_->FetchAlbumCover(EffectiveAlbumArtistName(*item), item->data(Role_AlbumName).toString(), false); cover_fetching_tasks_[id] = item; jobs_++; } diff --git a/src/covermanager/discogscoverprovider.cpp b/src/covermanager/discogscoverprovider.cpp index ce0cf0c81..f2b0cce8a 100644 --- a/src/covermanager/discogscoverprovider.cpp +++ b/src/covermanager/discogscoverprovider.cpp @@ -1,7 +1,6 @@ /* * Strawberry Music Player * This file was part of Clementine. - * Copyright 2010, David Sansome * Copyright 2012, Martin Björklund * Copyright 2016, Jonas Kvinge * @@ -22,13 +21,17 @@ #include "config.h" +#include #include -#include -#include +#include +#include +#include #include +#include +#include #include -#include #include +#include #include "discogscoverprovider.h" @@ -41,14 +44,14 @@ const char *DiscogsCoverProvider::kUrlSearch = "https://api.discogs.com/database const char *DiscogsCoverProvider::kUrlReleases = "https://api.discogs.com/releases"; const char *DiscogsCoverProvider::kAccessKeyB64 = "dGh6ZnljUGJlZ1NEeXBuSFFxSVk="; -const char *DiscogsCoverProvider::kSecretAccessKeyB64 = "ZkFIcmlaSER4aHhRSlF2U3d0bm5ZVmdxeXFLWUl0UXI="; +const char *DiscogsCoverProvider::kSecretKeyB64 = "ZkFIcmlaSER4aHhRSlF2U3d0bm5ZVmdxeXFLWUl0UXI="; DiscogsCoverProvider::DiscogsCoverProvider(QObject *parent) : CoverProvider("Discogs", false, parent), network_(new NetworkAccessManager(this)) {} bool DiscogsCoverProvider::StartSearch(const QString &artist, const QString &album, int s_id) { DiscogsCoverSearchContext *s_ctx = new DiscogsCoverSearchContext; - if (s_ctx == nullptr) return false; + s_ctx->id = s_id; s_ctx->artist = artist; s_ctx->album = album; @@ -60,10 +63,14 @@ bool DiscogsCoverProvider::StartSearch(const QString &artist, const QString &alb } +void DiscogsCoverProvider::CancelSearch(int id) { + delete requests_search_.take(id); +} + + bool DiscogsCoverProvider::StartRelease(DiscogsCoverSearchContext *s_ctx, int r_id, QString resource_url) { DiscogsCoverReleaseContext *r_ctx = new DiscogsCoverReleaseContext; - if (r_ctx == nullptr) return false; s_ctx->r_count++; @@ -89,7 +96,7 @@ void DiscogsCoverProvider::SendSearchRequest(DiscogsCoverSearchContext *s_ctx) { ArgList args = ArgList() << Arg("key", QByteArray::fromBase64(kAccessKeyB64)) - << Arg("secret", QByteArray::fromBase64(kSecretAccessKeyB64)); + << Arg("secret", QByteArray::fromBase64(kSecretKeyB64)); args.append(Arg("type", "release")); if (!s_ctx->artist.isEmpty()) { @@ -112,7 +119,7 @@ void DiscogsCoverProvider::SendSearchRequest(DiscogsCoverSearchContext *s_ctx) { // Sign the request const QByteArray data_to_sign = QString("GET\n%1\n%2\n%3").arg(url.host(), url.path(), query_items.join("&")).toLatin1(); - const QByteArray signature(Utilities::HmacSha256(QByteArray::fromBase64(kSecretAccessKeyB64), data_to_sign)); + const QByteArray signature(Utilities::HmacSha256(QByteArray::fromBase64(kSecretKeyB64), data_to_sign)); // Add the signature to the request url_query.addQueryItem("Signature", QUrl::toPercentEncoding(signature.toBase64())); @@ -123,8 +130,6 @@ void DiscogsCoverProvider::SendSearchRequest(DiscogsCoverSearchContext *s_ctx) { NewClosure(reply, SIGNAL(error(QNetworkReply::NetworkError)), this, SLOT(SearchRequestError(QNetworkReply::NetworkError, QNetworkReply*, int)), reply, s_ctx->id); NewClosure(reply, SIGNAL(finished()), this, SLOT(HandleSearchReply(QNetworkReply*, int)), reply, s_ctx->id); - return true; - } void DiscogsCoverProvider::SendReleaseRequest(DiscogsCoverSearchContext *s_ctx, DiscogsCoverReleaseContext *r_ctx) { @@ -140,7 +145,7 @@ void DiscogsCoverProvider::SendReleaseRequest(DiscogsCoverSearchContext *s_ctx, ArgList args = ArgList() << Arg("key", QByteArray::fromBase64(kAccessKeyB64)) - << Arg("secret", QByteArray::fromBase64(kSecretAccessKeyB64)); + << Arg("secret", QByteArray::fromBase64(kSecretKeyB64)); // Encode the arguments for (const Arg &arg : args) { EncodedArg encoded_arg(QUrl::toPercentEncoding(arg.first), QUrl::toPercentEncoding(arg.second)); @@ -148,14 +153,11 @@ void DiscogsCoverProvider::SendReleaseRequest(DiscogsCoverSearchContext *s_ctx, url_query.addQueryItem(encoded_arg.first, encoded_arg.second); } - //QString urlstr = QString("%1/%2").arg(kUrlReleases).arg(r_ctx->id); QUrl url(r_ctx->resource_url); - - //qLog(Debug) << "Send: " << url; // Sign the request - const QByteArray data_to_sign = QString("GET\n%1\n%2\n%3").arg(url.host(), url.path(), query_items.join("&")).toLatin1(); - const QByteArray signature(Utilities::HmacSha256(QByteArray::fromBase64(kSecretAccessKeyB64), data_to_sign)); + const QByteArray data_to_sign = QString("GET\n%1\n%2\n%3").arg(url.host(), url.path(), query_items.join("&")).toUtf8(); + const QByteArray signature(Utilities::HmacSha256(QByteArray::fromBase64(kSecretKeyB64), data_to_sign)); // Add the signature to the request url_query.addQueryItem("Signature", QUrl::toPercentEncoding(signature.toBase64())); @@ -166,39 +168,24 @@ void DiscogsCoverProvider::SendReleaseRequest(DiscogsCoverSearchContext *s_ctx, NewClosure(reply, SIGNAL(error(QNetworkReply::NetworkError)), this, SLOT(ReleaseRequestError(QNetworkReply::NetworkError, QNetworkReply*, int, int)), reply, s_ctx->id, r_ctx->id); NewClosure(reply, SIGNAL(finished()), this, SLOT(HandleReleaseReply(QNetworkReply*, int, int)), reply, s_ctx->id, r_ctx->id); - return true; - } void DiscogsCoverProvider::HandleSearchReply(QNetworkReply *reply, int s_id) { - //QString text(reply->readAll()); - //qLog(Debug) << "text: " << text << "\n"; - reply->deleteLater(); - DiscogsCoverSearchContext *s_ctx; if (!requests_search_.contains(s_id)) { - qLog(Error) << "Discogs: Got reply for cancelled request: " << s_id; + //qLog(Error) << "Discogs: Got reply for cancelled request: " << s_id; return; } - s_ctx = requests_search_.value(s_id); - if (s_ctx == nullptr) return; + DiscogsCoverSearchContext *s_ctx = requests_search_.value(s_id); - QString json_string; - json_string = reply->readAll(); - QByteArray json_bytes = json_string.toLocal8Bit(); - auto json_doc = QJsonDocument::fromJson(json_bytes); - if (json_doc.isNull()) { + QJsonDocument json_doc = QJsonDocument::fromJson(reply->readAll()); + if ((json_doc.isNull()) || (!json_doc.isObject())) { qLog(Error) << "Discogs: Failed to create JSON doc."; EndSearch(s_ctx); return; } - if (!json_doc.isObject()) { - qLog(Error) << "Discogs: JSON is not an object."; - EndSearch(s_ctx); - return; - } QJsonObject json_obj = json_doc.object(); if (json_obj.isEmpty()) { @@ -209,7 +196,7 @@ void DiscogsCoverProvider::HandleSearchReply(QNetworkReply *reply, int s_id) { QVariantMap reply_map = json_obj.toVariantMap(); if (!reply_map.contains("results")) { - qLog(Error) << "Discogs: Search reply from server is missing JSON results."; + //qLog(Error) << "Discogs: Search reply from server is missing JSON results."; //qLog(Error) << "Discogs: Map dump:"; //qLog(Error) << reply_map; EndSearch(s_ctx); @@ -221,24 +208,14 @@ void DiscogsCoverProvider::HandleSearchReply(QNetworkReply *reply, int s_id) { for (const QVariant &result : results) { QVariantMap result_map = result.toMap(); - int r_id = 0; - QString title; - QString resource_url; - if ((!result_map.contains("id")) || (!result_map.contains("resource_url"))) continue; - - if (result_map.contains("id")) { - r_id = result_map["id"].toInt(); - //qLog(Debug) << "id: " << r_id; + if ((result_map.contains("id")) && (result_map.contains("resource_url"))) { + int r_id = result_map["id"].toInt(); + QString title = result_map["title"].toString(); + QString resource_url = result_map["resource_url"].toString(); + if (resource_url.isEmpty()) continue; + StartRelease(s_ctx, r_id, resource_url); + i++; } - if (result_map.contains("title")) { - title = result_map["title"].toString(); - } - if (result_map.contains("resource_url")) { - resource_url = result_map["resource_url"].toString(); - //qLog(Debug) << "resource_url: " << resource_url; - } - StartRelease(s_ctx, r_id, resource_url); - i++; } if (i <= 0) EndSearch(s_ctx); @@ -246,42 +223,27 @@ void DiscogsCoverProvider::HandleSearchReply(QNetworkReply *reply, int s_id) { void DiscogsCoverProvider::HandleReleaseReply(QNetworkReply *reply, int s_id, int r_id) { - //QString text(reply->readAll()); - //qLog(Debug) << "text: " << text << "\n"; - reply->deleteLater(); - DiscogsCoverReleaseContext *r_ctx; if (!requests_release_.contains(r_id)) { //qLog(Error) << "Discogs: Got reply for cancelled request: " << r_id; return; } - r_ctx = requests_release_.value(r_id); - if (r_ctx == nullptr) return; + DiscogsCoverReleaseContext *r_ctx = requests_release_.value(r_id); - DiscogsCoverSearchContext *s_ctx; if (!requests_search_.contains(s_id)) { - qLog(Error) << "Discogs: Got reply for cancelled request: " << s_id << " " << r_id; - EndSearch(nullptr, r_ctx); + //qLog(Error) << "Discogs: Got reply for cancelled request: " << s_id << " " << r_id; + EndSearch(r_ctx); return; } - s_ctx = requests_search_.value(s_id); - if (s_ctx == nullptr) return; + DiscogsCoverSearchContext *s_ctx = requests_search_.value(s_id); - QString json_string; - json_string = reply->readAll(); - QByteArray json_bytes = json_string.toLocal8Bit(); - auto json_doc = QJsonDocument::fromJson(json_bytes); - if (json_doc.isNull()) { + QJsonDocument json_doc = QJsonDocument::fromJson(reply->readAll()); + if ((json_doc.isNull()) || (!json_doc.isObject())) { qLog(Error) << "Discogs: Failed to create JSON doc."; EndSearch(s_ctx, r_ctx); return; } - if (!json_doc.isObject()) { - qLog(Error) << "Discogs: JSON is not an object."; - EndSearch(s_ctx, r_ctx); - return; - } QJsonObject json_obj = json_doc.object(); if (json_obj.isEmpty()) { @@ -310,13 +272,10 @@ void DiscogsCoverProvider::HandleReleaseReply(QNetworkReply *reply, int s_id, in QString type = result_map["type"].toString(); if (type != "primary") continue; } - if (result_map.contains("height")) { - } - if (result_map.contains("width")) { - } if (result_map.contains("resource_url")) { cover_result.image_url = QUrl(result_map["resource_url"].toString()); } + if (cover_result.image_url.isEmpty()) continue; s_ctx->results.append(cover_result); } @@ -324,43 +283,31 @@ void DiscogsCoverProvider::HandleReleaseReply(QNetworkReply *reply, int s_id, in } -void DiscogsCoverProvider::CancelSearch(int id) { - - delete requests_search_.take(id); - -} - void DiscogsCoverProvider::SearchRequestError(QNetworkReply::NetworkError error, QNetworkReply *reply, int s_id) { - DiscogsCoverSearchContext *s_ctx; if (!requests_search_.contains(s_id)) { - qLog(Error) << "Discogs: got reply for cancelled request: " << s_id; + //qLog(Error) << "Discogs: got reply for cancelled request: " << s_id; return; } - s_ctx = requests_search_.value(s_id); - if (s_ctx == nullptr) return; - + DiscogsCoverSearchContext *s_ctx = requests_search_.value(s_id); EndSearch(s_ctx); } void DiscogsCoverProvider::ReleaseRequestError(QNetworkReply::NetworkError error, QNetworkReply *reply, int s_id, int r_id) { - DiscogsCoverSearchContext *s_ctx; - if (!requests_search_.contains(s_id)) { - qLog(Error) << "Discogs: got reply for cancelled request: " << s_id << r_id; - return; - } - s_ctx = requests_search_.value(s_id); - if (s_ctx == nullptr) return; - - DiscogsCoverReleaseContext *r_ctx; if (!requests_release_.contains(r_id)) { - qLog(Error) << "Discogs: got reply for cancelled request: " << s_id << r_id; + //qLog(Error) << "Discogs: got reply for cancelled request: " << s_id << r_id; return; } - r_ctx = requests_release_.value(r_id); - if (r_ctx == nullptr) return; + DiscogsCoverReleaseContext *r_ctx = requests_release_.value(r_id); + + if (!requests_search_.contains(s_id)) { + EndSearch(r_ctx); + //qLog(Error) << "Discogs: got reply for cancelled request: " << s_id << r_id; + return; + } + DiscogsCoverSearchContext *s_ctx = requests_search_.value(s_id); EndSearch(s_ctx, r_ctx); @@ -368,10 +315,7 @@ void DiscogsCoverProvider::ReleaseRequestError(QNetworkReply::NetworkError error void DiscogsCoverProvider::EndSearch(DiscogsCoverSearchContext *s_ctx, DiscogsCoverReleaseContext *r_ctx) { - (void)requests_release_.remove(r_ctx->id); - delete r_ctx; - - if (s_ctx == nullptr) return; + delete requests_release_.take(r_ctx->id); s_ctx->r_count--; @@ -385,8 +329,12 @@ void DiscogsCoverProvider::EndSearch(DiscogsCoverSearchContext *s_ctx) { //qLog(Debug) << "Discogs: Finished." << s_ctx->id; - (void)requests_search_.remove(s_ctx->id); + requests_search_.remove(s_ctx->id); emit SearchFinished(s_ctx->id, s_ctx->results); delete s_ctx; } + +void DiscogsCoverProvider::EndSearch(DiscogsCoverReleaseContext* r_ctx) { + delete requests_release_.take(r_ctx->id); +} diff --git a/src/covermanager/discogscoverprovider.h b/src/covermanager/discogscoverprovider.h index f37f629e8..91c5fb05e 100644 --- a/src/covermanager/discogscoverprovider.h +++ b/src/covermanager/discogscoverprovider.h @@ -1,7 +1,6 @@ /* * Strawberry Music Player * This file was part of Clementine. - * Copyright 2010, David Sansome * Copyright 2012, Martin Björklund * Copyright 2016, Jonas Kvinge * @@ -34,7 +33,6 @@ class QNetworkAccessManager; // This struct represents a single search-for-cover request. It identifies and describes the request. struct DiscogsCoverSearchContext { - enum State { State_Init, State_Release }; // the unique request identifier int id; @@ -49,6 +47,8 @@ struct DiscogsCoverSearchContext { }; Q_DECLARE_METATYPE(DiscogsCoverSearchContext) +// This struct represents a single release request. It identifies and describes +// the request. struct DiscogsCoverReleaseContext { int id; // the unique request identifier @@ -64,19 +64,6 @@ class DiscogsCoverProvider : public CoverProvider { public: explicit DiscogsCoverProvider(QObject *parent = nullptr); - static const char *kUrlSearch; - static const char *kUrlReleases; - - static const char *kRequestTokenURL; - static const char *kAuthorizeURL; - static const char *kAccessTokenURL; - - static const char *kAccessKey; - static const char *kSecretAccessKey; - - static const char *kAccessKeyB64; - static const char *kSecretAccessKeyB64; - bool StartSearch(const QString &artist, const QString &album, int s_id); void CancelSearch(int id); @@ -85,9 +72,14 @@ class DiscogsCoverProvider : public CoverProvider { void SearchRequestError(QNetworkReply::NetworkError error, QNetworkReply *reply, int s_id); void ReleaseRequestError(QNetworkReply::NetworkError error, QNetworkReply *reply, int s_id, int r_id); void HandleSearchReply(QNetworkReply *reply, int s_id); - void HandleReleaseReply(QNetworkReply *reply, int sa_id, int si_id); + void HandleReleaseReply(QNetworkReply *reply, int s_id, int r_id); private: + static const char *kUrlSearch; + static const char *kUrlReleases; + static const char *kAccessKeyB64; + static const char *kSecretKeyB64; + QNetworkAccessManager *network_; QHash requests_search_; QHash requests_release_; @@ -98,6 +90,7 @@ class DiscogsCoverProvider : public CoverProvider { void SendReleaseRequest(DiscogsCoverSearchContext *s_ctx, DiscogsCoverReleaseContext *r_ctx); void EndSearch(DiscogsCoverSearchContext *s_ctx, DiscogsCoverReleaseContext *r_ctx); void EndSearch(DiscogsCoverSearchContext *s_ctx); + void EndSearch(DiscogsCoverReleaseContext *r_ctx); };