Merge pull request #15 from Fxrh/kitsune-dropped-kcoreaddons

Upon discussion with @Fxrh in #quaternion, this now comes in master,

1 files changed, 113 insertions, 60 deletions

diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp
index 6c68ab66..e0dff287 100644
--- a/jobs/basejob.cpp
+++ b/jobs/basejob.cpp
@@ -19,50 +19,56 @@
 #include "basejob.h"
 
 #include <QtNetwork/QNetworkAccessManager>
-#include <QtNetwork/QNetworkReply>
 #include <QtNetwork/QNetworkRequest>
+#include <QtNetwork/QNetworkReply>
+#include <QtNetwork/QSslError>
 #include <QtCore/QTimer>
 
 #include "../connectiondata.h"
 
 using namespace QMatrixClient;
 
+struct NetworkReplyDeleter : public QScopedPointerDeleteLater
+{
+    static inline void cleanup(QNetworkReply* reply)
+    {
+        if (reply && reply->isRunning())
+            reply->abort();
+        QScopedPointerDeleteLater::cleanup(reply);
+    }
+};
+
 class BaseJob::Private
 {
     public:
         Private(ConnectionData* c, JobHttpType t, bool nt)
-            : connection(c), reply(nullptr), type(t), needsToken(nt) {}
+            : connection(c), type(t), needsToken(nt)
+            , reply(nullptr), status(NoError)
+        {}
         
         ConnectionData* connection;
-        QNetworkReply* reply;
         JobHttpType type;
         bool needsToken;
+
+        QScopedPointer<QNetworkReply, NetworkReplyDeleter> reply;
+        Status status;
 };
 
+inline QDebug operator<<(QDebug dbg, BaseJob* j)
+{
+    return dbg << "Job" << j->objectName();
+}
+
 BaseJob::BaseJob(ConnectionData* connection, JobHttpType type, QString name, bool needsToken)
     : d(new Private(connection, type, needsToken))
 {
-    // Work around KJob inability to separate success and failure signals
-    connect(this, &BaseJob::result, [this]() {
-        if (error() == NoError)
-            emit success(this);
-        else
-            emit failure(this);
-    });
     setObjectName(name);
-    qDebug() << "Job" << objectName() << " created";
+    qDebug() << this << "created";
 }
 
 BaseJob::~BaseJob()
 {
-    if( d->reply )
-    {
-        if( d->reply->isRunning() )
-            d->reply->abort();
-        d->reply->deleteLater();
-    }
-    delete d;
-    qDebug() << "Job" << objectName() << " destroyed";
+    qDebug() << this << "destroyed";
 }
 
 ConnectionData* BaseJob::connection() const
@@ -80,11 +86,6 @@ QUrlQuery BaseJob::query() const
     return QUrlQuery();
 }
 
-void BaseJob::parseJson(const QJsonDocument& data)
-{
-    emitResult();
-}
-
 void BaseJob::start()
 {
     QUrl url = d->connection->baseUrl();
@@ -103,75 +104,127 @@ void BaseJob::start()
     switch( d->type )
     {
         case JobHttpType::GetJob:
-            d->reply = d->connection->nam()->get(req);
+            d->reply.reset( d->connection->nam()->get(req) );
             break;
         case JobHttpType::PostJob:
-            d->reply = d->connection->nam()->post(req, data.toJson());
+            d->reply.reset( d->connection->nam()->post(req, data.toJson()) );
             break;
         case JobHttpType::PutJob:
-            d->reply = d->connection->nam()->put(req, data.toJson());
+            d->reply.reset( d->connection->nam()->put(req, data.toJson()) );
             break;
     }
-    connect( d->reply, &QNetworkReply::sslErrors, this, &BaseJob::sslErrors );
-    connect( d->reply, &QNetworkReply::finished, this, &BaseJob::gotReply );
+    connect( d->reply.data(), &QNetworkReply::sslErrors, this, &BaseJob::sslErrors );
+    connect( d->reply.data(), &QNetworkReply::finished, this, &BaseJob::gotReply );
     QTimer::singleShot( 120*1000, this, SLOT(timeout()) );
 //     connect( d->reply, static_cast<void(QNetworkReply::*)(QNetworkReply::NetworkError)>(&QNetworkReply::error),
 //              this, &BaseJob::networkError ); // http://doc.qt.io/qt-5/qnetworkreply.html#error-1
 }
 
-void BaseJob::fail(int errorCode, QString errorString)
+void BaseJob::gotReply()
 {
-    setError( errorCode );
-    setErrorText( errorString );
-    if( d->reply && d->reply->isRunning() )
-        d->reply->abort();
-    qWarning() << "Job" << objectName() << "failed:" << errorString;
-    emitResult();
-}
+    setStatus(checkReply(d->reply.data()));
+    if (status().good())
+        setStatus(parseReply(d->reply->readAll()));
 
-QNetworkReply* BaseJob::networkReply() const
-{
-    return d->reply;
+    finishJob(true);
 }
 
-// void BaseJob::networkError(QNetworkReply::NetworkError code)
-// {
-//     fail( KJob::UserDefinedError+1, d->reply->errorString() );
-// }
-
-void BaseJob::gotReply()
+BaseJob::Status BaseJob::checkReply(QNetworkReply* reply) const
 {
-    switch( d->reply->error() )
+    switch( reply->error() )
     {
     case QNetworkReply::NoError:
-        break; // All good, go to the normal flow after the switch()
+        return NoError;
 
     case QNetworkReply::AuthenticationRequiredError:
     case QNetworkReply::ContentAccessDenied:
     case QNetworkReply::ContentOperationNotPermittedError:
-        qDebug() << "Content access error, Qt error code:" << d->reply->error();
-        fail( ContentAccessError, d->reply->errorString() );
-        return;
+        return { ContentAccessError, reply->errorString() };
 
     default:
-        qDebug() << "NetworkError, Qt error code:" << d->reply->error();
-        fail( NetworkError, d->reply->errorString() );
-        return;
+        return { NetworkError, reply->errorString() };
     }
+}
 
+BaseJob::Status BaseJob::parseReply(QByteArray data)
+{
     QJsonParseError error;
-    QJsonDocument data = QJsonDocument::fromJson(d->reply->readAll(), &error);
-    if( error.error != QJsonParseError::NoError )
+    QJsonDocument json = QJsonDocument::fromJson(data, &error);
+    if( error.error == QJsonParseError::NoError )
+        return parseJson(json);
+    else
+        return { JsonParseError, error.errorString() };
+}
+
+BaseJob::Status BaseJob::parseJson(const QJsonDocument&)
+{
+    return Success;
+}
+
+void BaseJob::finishJob(bool emitResult)
+{
+    if (!d->reply)
+    {
+        qWarning() << this << "finishes with empty network reply";
+    }
+    else if (d->reply->isRunning())
+    {
+        qWarning() << this << "finishes without ready network reply";
+        d->reply->disconnect(this); // Ignore whatever comes from the reply
+    }
+
+    // Notify those that are interested in any completion of the job (including killing)
+    emit finished(this);
+
+    if (emitResult) {
+        emit result(this);
+        if (error())
+            emit failure(this);
+        else
+            emit success(this);
+    }
+
+    deleteLater();
+}
+
+BaseJob::Status BaseJob::status() const
+{
+    return d->status;
+}
+
+int BaseJob::error() const
+{
+    return d->status.code;
+}
+
+QString BaseJob::errorString() const
+{
+    return d->status.message;
+}
+
+void BaseJob::setStatus(Status s)
+{
+    d->status = s;
+    if (!s.good())
     {
-        fail( JsonParseError, error.errorString() );
-        return;
+        qWarning() << this << "status" << s.code << ":" << s.message;
     }
-    parseJson(data);
+}
+
+void BaseJob::setStatus(int code, QString message)
+{
+    setStatus({ code, message });
+}
+
+void BaseJob::abandon()
+{
+    finishJob(false);
 }
 
 void BaseJob::timeout()
 {
-    fail( TimeoutError, "The job has timed out" );
+    setStatus( TimeoutError, "The job has timed out" );
+    finishJob(true);
 }
 
 void BaseJob::sslErrors(const QList<QSslError>& errors)