diff --git a/src/covermanager/discogscoverprovider.cpp b/src/covermanager/discogscoverprovider.cpp index e157e2ee6..2f06620f9 100644 --- a/src/covermanager/discogscoverprovider.cpp +++ b/src/covermanager/discogscoverprovider.cpp @@ -25,8 +25,6 @@ #include #include #include -#include -#include #include #include #include @@ -38,7 +36,8 @@ #include #include #include -#include +#include +#include #include "core/closure.h" #include "core/logging.h" @@ -115,7 +114,6 @@ void DiscogsCoverProvider::SendSearchRequest(DiscogsCoverSearchContext *s_ctx) { } QUrlQuery url_query; - QUrl url(kUrlSearch); QStringList query_items; // Encode the arguments @@ -125,6 +123,9 @@ void DiscogsCoverProvider::SendSearchRequest(DiscogsCoverSearchContext *s_ctx) { url_query.addQueryItem(encoded_arg.first, encoded_arg.second); } + QUrl url(kUrlSearch); + url.setQuery(url_query); + // Sign the request 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)); @@ -132,10 +133,7 @@ void DiscogsCoverProvider::SendSearchRequest(DiscogsCoverSearchContext *s_ctx) { // Add the signature to the request url_query.addQueryItem("Signature", QUrl::toPercentEncoding(signature.toBase64())); - url.setQuery(url_query); QNetworkReply *reply = network_->get(QNetworkRequest(url)); - - 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); } @@ -171,61 +169,147 @@ void DiscogsCoverProvider::SendReleaseRequest(DiscogsCoverSearchContext *s_ctx, url_query.addQueryItem("Signature", QUrl::toPercentEncoding(signature.toBase64())); url.setQuery(url_query); - QNetworkReply *reply = network_->get(QNetworkRequest(url)); - NewClosure(reply, SIGNAL(error(QNetworkReply::NetworkError)), this, SLOT(ReleaseRequestError(QNetworkReply::NetworkError, QNetworkReply*, int, int)), reply, s_ctx->id, r_ctx->id); + QNetworkReply *reply = network_->get(QNetworkRequest(url)); NewClosure(reply, SIGNAL(finished()), this, SLOT(HandleReleaseReply(QNetworkReply*, int, int)), reply, s_ctx->id, r_ctx->id); } +QByteArray DiscogsCoverProvider::GetReplyData(QNetworkReply *reply) { + + QByteArray data; + + if (reply->error() == QNetworkReply::NoError && reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt() == 200) { + data = reply->readAll(); + } + else { + if (reply->error() < 200) { + // This is a network error, there is nothing more to do. + QString failure_reason = QString("%1 (%2)").arg(reply->errorString()).arg(reply->error()); + Error(failure_reason); + } + else { + // See if there is Json data containing "message" - then use that instead. + data = reply->readAll(); + QJsonParseError error; + QJsonDocument json_doc = QJsonDocument::fromJson(data, &error); + QString failure_reason; + if (error.error == QJsonParseError::NoError && !json_doc.isNull() && !json_doc.isEmpty() && json_doc.isObject()) { + QJsonObject json_obj = json_doc.object(); + if (json_obj.contains("message")) { + failure_reason = json_obj["message"].toString(); + } + else { + failure_reason = QString("%1 (%2)").arg(reply->errorString()).arg(reply->error()); + } + } + else { + failure_reason = QString("%1 (%2)").arg(reply->errorString()).arg(reply->error()); + } + Error(failure_reason); + } + return QByteArray(); + } + + return data; + +} + +QJsonObject DiscogsCoverProvider::ExtractJsonObj(const QByteArray &data) { + + QJsonParseError error; + QJsonDocument json_doc = QJsonDocument::fromJson(data, &error); + + if (error.error != QJsonParseError::NoError) { + Error("Reply from server missing Json data.", data); + return QJsonObject(); + } + if (json_doc.isNull() || json_doc.isEmpty()) { + Error("Received empty Json document.", json_doc); + return QJsonObject(); + } + if (!json_doc.isObject()) { + Error("Json document is not an object.", json_doc); + return QJsonObject(); + } + QJsonObject json_obj = json_doc.object(); + if (json_obj.isEmpty()) { + Error("Received empty Json object.", json_doc); + return QJsonObject(); + } + + return json_obj; + +} + +QJsonValue DiscogsCoverProvider::ExtractData(const QByteArray &data, const QString name, const bool silent) { + + QJsonObject json_obj = ExtractJsonObj(data); + if (json_obj.isEmpty()) return QJsonObject(); + + if (json_obj.contains(name)) { + QJsonValue json_results = json_obj[name]; + return json_results; + } + else if (json_obj.contains("message")) { + QString message = json_obj["message"].toString(); + Error(QString("%1").arg(message)); + } + else { + if (!silent) Error(QString("Json reply is missing \"%1\".").arg(name), json_obj); + } + return QJsonValue(); + +} + void DiscogsCoverProvider::HandleSearchReply(QNetworkReply *reply, int s_id) { reply->deleteLater(); if (!requests_search_.contains(s_id)) { - //qLog(Error) << "Discogs: Got reply for cancelled request: " << s_id; + Error(QString("Got reply for cancelled request: %1").arg(s_id)); return; } DiscogsCoverSearchContext *s_ctx = requests_search_.value(s_id); - QJsonDocument json_doc = QJsonDocument::fromJson(reply->readAll()); - if ((json_doc.isNull()) || (!json_doc.isObject())) { - qLog(Error) << "Discogs: Failed to create JSON doc."; + QByteArray data = GetReplyData(reply); + + QJsonValue json_value = ExtractData(data, "results"); + if (!json_value.isArray()) { EndSearch(s_ctx); return; } - QJsonObject json_obj = json_doc.object(); - if (json_obj.isEmpty()) { - qLog(Error) << "Discogs: JSON object is empty."; + QJsonArray json_results = json_value.toArray(); + if (json_results.isEmpty()) { + Error("Json array is empty."); EndSearch(s_ctx); return; } - 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: Map dump:"; - //qLog(Error) << reply_map; - EndSearch(s_ctx); - return; - } - - QVariantList results = reply_map["results"].toList(); int i = 0; - - for (const QVariant &result : results) { - QVariantMap result_map = result.toMap(); - 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++; + for (const QJsonValue &value : json_results) { + if (!value.isObject()) { + Error("Invalid Json reply, data is not an object.", value); + continue; } + QJsonObject json_obj = value.toObject(); + if (!json_obj.contains("id") || !json_obj.contains("title") || !json_obj.contains("resource_url")) { + Error("Invalid Json reply, value is missing ID, title or resource_url.", json_obj); + continue; + } + int r_id = json_obj["id"].toInt(); + QString title = json_obj["title"].toString(); + QString resource_url = json_obj["resource_url"].toString(); + if (resource_url.isEmpty()) continue; + StartRelease(s_ctx, r_id, resource_url); + i++; + } + + if (i <= 0) { + Error("Ending search with no results."); + EndSearch(s_ctx); } - if (i <= 0) EndSearch(s_ctx); } @@ -234,88 +318,58 @@ void DiscogsCoverProvider::HandleReleaseReply(QNetworkReply *reply, int s_id, in reply->deleteLater(); if (!requests_release_.contains(r_id)) { - //qLog(Error) << "Discogs: Got reply for cancelled request: " << r_id; + Error(QString("Got reply for cancelled request: %1 %2").arg(s_id).arg(r_id)); return; } DiscogsCoverReleaseContext *r_ctx = requests_release_.value(r_id); if (!requests_search_.contains(s_id)) { - //qLog(Error) << "Discogs: Got reply for cancelled request: " << s_id << " " << r_id; + Error(QString("Got reply for cancelled request: %1 %2").arg(s_id).arg(r_id)); EndSearch(r_ctx); return; } DiscogsCoverSearchContext *s_ctx = requests_search_.value(s_id); - QJsonDocument json_doc = QJsonDocument::fromJson(reply->readAll()); - if ((json_doc.isNull()) || (!json_doc.isObject())) { - qLog(Error) << "Discogs: Failed to create JSON doc."; + QByteArray data = GetReplyData(reply); + + QJsonValue json_value = ExtractData(data, "images", true); + if (!json_value.isArray()) { EndSearch(s_ctx, r_ctx); return; } - QJsonObject json_obj = json_doc.object(); - if (json_obj.isEmpty()) { - qLog(Error) << "Discogs: JSON object is empty."; + QJsonArray json_images = json_value.toArray(); + if (json_images.isEmpty()) { + Error("Json array is empty."); EndSearch(s_ctx, r_ctx); return; } - QVariantMap reply_map = json_obj.toVariantMap(); - if (!reply_map.contains("images")) { - //qLog(Error) << "Discogs: Search reply from server is missing JSON images."; - //qLog(Error) << "Discogs: Map dump:"; - //qLog(Error) << reply_map; - EndSearch(s_ctx, r_ctx); - return; - } - - QVariantList results = reply_map["images"].toList(); - - for (const QVariant &result : results) { - QVariantMap result_map = result.toMap(); + int i = 0; + for (const QJsonValue &value : json_images) { + if (!value.isObject()) { + Error("Invalid Json reply, value is not an object.", value); + continue; + } + QJsonObject json_obj = value.toObject(); + if (!json_obj.contains("type") || !json_obj.contains("resource_url")) { + Error("Invalid Json reply, value is missing ID or resource_url.", json_obj); + continue; + } CoverSearchResult cover_result; cover_result.description = s_ctx->title; - - if (result_map.contains("type")) { - QString type = result_map["type"].toString(); - if (type != "primary") continue; - } - if (result_map.contains("resource_url")) { - cover_result.image_url = QUrl(result_map["resource_url"].toString()); + QString type = json_obj["type"].toString(); + i++; + if (type != "primary") { + continue; } + cover_result.image_url = QUrl(json_obj["resource_url"].toString()); if (cover_result.image_url.isEmpty()) continue; s_ctx->results.append(cover_result); } - - EndSearch(s_ctx, r_ctx); - -} - -void DiscogsCoverProvider::SearchRequestError(QNetworkReply::NetworkError error, QNetworkReply *reply, int s_id) { - - if (!requests_search_.contains(s_id)) { - //qLog(Error) << "Discogs: got reply for cancelled request: " << s_id; - return; + if (i <= 0) { + Error("Ending search with no results."); } - 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) { - - if (!requests_release_.contains(r_id)) { - //qLog(Error) << "Discogs: got reply for cancelled request: " << s_id << r_id; - 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); @@ -324,19 +378,13 @@ void DiscogsCoverProvider::ReleaseRequestError(QNetworkReply::NetworkError error void DiscogsCoverProvider::EndSearch(DiscogsCoverSearchContext *s_ctx, DiscogsCoverReleaseContext *r_ctx) { delete requests_release_.take(r_ctx->id); - s_ctx->r_count--; - - //qLog(Debug) << "r_count: " << s_ctx->r_count; - if (s_ctx->r_count <= 0) EndSearch(s_ctx); } void DiscogsCoverProvider::EndSearch(DiscogsCoverSearchContext *s_ctx) { - //qLog(Debug) << "Discogs: Finished." << s_ctx->id; - requests_search_.remove(s_ctx->id); emit SearchFinished(s_ctx->id, s_ctx->results); delete s_ctx; @@ -346,3 +394,8 @@ void DiscogsCoverProvider::EndSearch(DiscogsCoverSearchContext *s_ctx) { void DiscogsCoverProvider::EndSearch(DiscogsCoverReleaseContext* r_ctx) { delete requests_release_.take(r_ctx->id); } + +void DiscogsCoverProvider::Error(QString error, QVariant debug) { + qLog(Error) << "Discogs:" << error; + if (debug.isValid()) qLog(Debug) << debug; +} diff --git a/src/covermanager/discogscoverprovider.h b/src/covermanager/discogscoverprovider.h index 18cbdb5bd..891a75b7a 100644 --- a/src/covermanager/discogscoverprovider.h +++ b/src/covermanager/discogscoverprovider.h @@ -73,8 +73,6 @@ class DiscogsCoverProvider : public CoverProvider { void CancelSearch(int id); private slots: - 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 s_id, int r_id); @@ -92,9 +90,13 @@ class DiscogsCoverProvider : public CoverProvider { void SendSearchRequest(DiscogsCoverSearchContext *s_ctx); void SendReleaseRequest(DiscogsCoverSearchContext *s_ctx, DiscogsCoverReleaseContext *r_ctx); + QByteArray GetReplyData(QNetworkReply *reply); + QJsonObject ExtractJsonObj(const QByteArray &data); + QJsonValue ExtractData(const QByteArray &data, const QString name, const bool silent = false); void EndSearch(DiscogsCoverSearchContext *s_ctx, DiscogsCoverReleaseContext *r_ctx); void EndSearch(DiscogsCoverSearchContext *s_ctx); void EndSearch(DiscogsCoverReleaseContext *r_ctx); + void Error(QString error, QVariant debug = QVariant()); };