lp1848887: Fix parsing of AcoustID response

src/musicbrainz/acoustidclient.cpp

@@ -7,51 +7,58 @@
  *  See http://www.wtfpl.net/ for more details.                              *
  *****************************************************************************/
 
-#include <QCoreApplication>
 #include <QNetworkReply>
-#include <QXmlStreamReader>
-#include <QTextStream>
+#include <QJsonDocument>
 #include <QUrl>
+#include <QUrlQuery>
 #include <QtDebug>
 
 #include "musicbrainz/acoustidclient.h"
 #include "musicbrainz/gzip.h"
 #include "musicbrainz/network.h"
 
+namespace {
+
 // see API-KEY site here http://acoustid.org/application/496
 // I registered the KEY for version 1.12 -- kain88 (may 2013)
+// See also: https://acoustid.org/webservice
 const QString CLIENT_APIKEY = "czKxnkyO";
-const QString ACOUSTID_URL = "http://api.acoustid.org/v2/lookup";
-const int AcoustidClient::m_DefaultTimeout = 5000; // msec
+const QString ACOUSTID_URL = "https://api.acoustid.org/v2/lookup";
+constexpr int kDefaultTimeout = 5000; // msec
+
+} // anonymous namespace
 
 AcoustidClient::AcoustidClient(QObject* parent)
               : QObject(parent),
                 m_network(this),
-                m_timeouts(m_DefaultTimeout, this) {
+                m_timeouts(kDefaultTimeout, this) {
 }
 
 void AcoustidClient::setTimeout(int msec) {
     m_timeouts.setTimeout(msec);
 }
 
 void AcoustidClient::start(int id, const QString& fingerprint, int duration) {
-    QUrl url;
-    url.addQueryItem("format", "xml");
-    url.addQueryItem("client", CLIENT_APIKEY);
-    url.addQueryItem("duration", QString::number(duration));
-    url.addQueryItem("meta", "recordingids");
-    url.addQueryItem("fingerprint", fingerprint);
-    QByteArray body = url.encodedQuery();
-
-    QNetworkRequest req(QUrl::fromEncoded(ACOUSTID_URL.toAscii()));
+    QUrlQuery urlQuery;
+    // The default format is JSON
+    //urlQuery.addQueryItem("format", "json");
+    urlQuery.addQueryItem("client", CLIENT_APIKEY);
+    urlQuery.addQueryItem("duration", QString::number(duration));
+    urlQuery.addQueryItem("meta", "recordingids");
+    urlQuery.addQueryItem("fingerprint", fingerprint);
+    // application/x-www-form-urlencoded request bodies must be percent encoded.
+    QByteArray body = urlQuery.query(QUrl::FullyEncoded).toLatin1();
+
+    QUrl url(ACOUSTID_URL);
+    QNetworkRequest req(url);
     req.setHeader(QNetworkRequest::ContentTypeHeader, "application/x-www-form-urlencoded");
     req.setRawHeader("Content-Encoding", "gzip");
 
     qDebug() << "AcoustIdClient POST request:" << ACOUSTID_URL
              << "body:" << body;
 
     QNetworkReply* reply = m_network.post(req, gzipCompress(body));
-    connect(reply, SIGNAL(finished()), SLOT(requestFinished()));
+    connect(reply, &QNetworkReply::finished, this, &AcoustidClient::requestFinished);
     m_requests[reply] = id;
 
     m_timeouts.addReply(reply);
@@ -64,8 +71,13 @@ void AcoustidClient::cancel(int id) {
 }
 
 void AcoustidClient::cancelAll() {
-    qDeleteAll(m_requests.keys());
+    auto requests = m_requests;
     m_requests.clear();
+
+    for (auto it = requests.constBegin();
+         it != requests.constEnd(); ++it) {
+        delete it.key();
+    }
 }
 
 void AcoustidClient::requestFinished() {
@@ -79,45 +91,44 @@ void AcoustidClient::requestFinished() {
 
     int id = m_requests.take(reply);
 
-    int status = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
-    if (status != 200) {
-        QTextStream body(reply);
-        qDebug() << "AcoustIdClient POST reply status:" << status << "body:" << body.readAll();
-        emit(networkError(
-             reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(),
-             "AcoustID"));
-        return;
-    }
-
+    const int statusCode = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
+    const QByteArray body = reply->readAll();
+    qDebug() << "AcoustIdClient POST reply status:" << statusCode << "body:" << body;
 
-    QTextStream textReader(reply);
-    QString body = textReader.readAll();
-    qDebug() << "AcoustIdClient POST reply status:" << status << "body:" << body;
-
-    QXmlStreamReader reader(body);
-    QString ID;
-    while (!reader.atEnd()) {
-        if (reader.readNext() == QXmlStreamReader::StartElement
-            && reader.name()== "results") {
-                ID = parseResult(reader);
-            }
+    const auto jsonResponse = QJsonDocument::fromJson(body);
+    QString statusText;
+    if (jsonResponse.isObject()) {
+        statusText = jsonResponse.object().value("status").toString();
     }
 
-    emit(finished(id, ID));
-}
-
-QString AcoustidClient::parseResult(QXmlStreamReader& reader) {
-    while (!reader.atEnd()) {
-        QXmlStreamReader::TokenType type = reader.readNext();
-        if (type== QXmlStreamReader::StartElement) {
-            QStringRef name = reader.name();
-            if (name == "id") {
-                return reader.readElementText();
-            }
+    if (statusCode != 200 || statusText != "ok") {
+        QString errorMessage;
+        int errorCode = 0;
+        if (jsonResponse.isObject() && statusText == "error") {
+            const auto error = jsonResponse.object().value("error").toObject();
+            errorMessage = error.value("message").toString();
+            errorCode = error.value("message").toInt(errorCode);
         }
-        if (type == QXmlStreamReader::EndElement && reader.name()=="result") {
-            break;
+        emit networkError(
+             statusCode,
+             "AcoustID",
+             errorMessage,
+             errorCode);
+        return;
+    }
+
+    QString recordingId;
+    DEBUG_ASSERT(jsonResponse.isObject());
+    DEBUG_ASSERT(jsonResponse.object().value("results").isArray());
+    const QJsonArray results = jsonResponse.object().value("results").toArray();
+    if (!results.isEmpty()) {
+        // Only take the first result with the maximum(?) score
+        DEBUG_ASSERT(results.at(0).toObject().value("recordings").isArray());
+        const QJsonArray recordings = results.at(0).toObject().value("recordings").toArray();
+        if (!recordings.isEmpty()) {
+            // Only take the first recording
+            recordingId = recordings.at(0).toObject().value("id").toString();
         }
     }
-    return QString();
+    emit finished(id, recordingId);
 }

src/musicbrainz/acoustidclient.h

@@ -17,8 +17,6 @@
 #include "musicbrainz/network.h"
 #include "track/track.h"
 
-class QXmlStreamReader;
-
 class AcoustidClient : public QObject {
   Q_OBJECT
 
@@ -48,18 +46,14 @@ class AcoustidClient : public QObject {
     // requests.
     void cancelAll();
 
-    QString parseResult(QXmlStreamReader& reader);
-
   signals:
     void finished(int id, const QString& mbid);
-    void networkError(int, QString);
+    void networkError(int httpStatus, QString app, QString message, int code);
 
   private slots:
     void requestFinished();
 
   private:
-    static const int m_DefaultTimeout;
-
     QNetworkAccessManager m_network;
     NetworkTimeouts m_timeouts;
     QMap<QNetworkReply*, int> m_requests;