18 files changed, 278 insertions, 135 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)
diff --git a/jobs/basejob.h b/jobs/basejob.h
index 150d39e8..07b1f1dd 100644
--- a/jobs/basejob.h
+++ b/jobs/basejob.h
@@ -19,16 +19,14 @@
 #ifndef QMATRIXCLIENT_BASEJOB_H
 #define QMATRIXCLIENT_BASEJOB_H
 
-#ifdef USING_SYSTEM_KCOREADDONS
-#include <KCoreAddons/KJob>
-#else
-#include "kjob.h"
-#endif // KCOREADDONS_FOUND
-
+#include <QtCore/QObject>
 #include <QtCore/QJsonDocument>
 #include <QtCore/QJsonObject>
 #include <QtCore/QUrlQuery>
-#include <QtNetwork/QNetworkReply>
+#include <QtCore/QScopedPointer>
+
+class QNetworkReply;
+class QSslError;
 
 namespace QMatrixClient
 {
@@ -36,28 +34,104 @@ namespace QMatrixClient
 
     enum class JobHttpType { GetJob, PutJob, PostJob };
     
-    class BaseJob: public KJob
+    class BaseJob: public QObject
     {
             Q_OBJECT
         public:
+            /* Just in case, the values are compatible with KJob
+             * (which BaseJob used to inherit from). */
+            enum StatusCode { NoError = 0 // To be compatible with Qt conventions
+                , Success = 0
+                , ErrorLevel = 100 // Errors have codes starting from this
+                , NetworkError = 100
+                , JsonParseError
+                , TimeoutError
+                , ContentAccessError
+                , UserDefinedError = 200
+            };
+
+            /**
+             * This structure stores the status of a server call job. The status consists
+             * of a code, that is described (but not delimited) by the respective enum,
+             * and a freeform message.
+             *
+             * To extend the list of error codes, define an (anonymous) enum
+             * along the lines of StatusCode, with additional values
+             * starting at UserDefinedError
+             */
+            class Status
+            {
+                public:
+                    Status(StatusCode c) : code(c) { }
+                    Status(int c, QString m) : code(c), message(m) { }
+
+                    bool good() const { return code < ErrorLevel; }
+
+                    int code;
+                    QString message;
+            };
+
+        public:
             BaseJob(ConnectionData* connection, JobHttpType type,
                     QString name, bool needsToken=true);
             virtual ~BaseJob();
 
-            void start() override;
+            void start();
+
+            /**
+             * Abandons the result of this job, arrived or unarrived.
+             *
+             * This aborts waiting for a reply from the server (if there was
+             * any pending) and deletes the job object. It is always done quietly
+             * (as opposed to KJob::kill() that can trigger emitting the result).
+             */
+            void abandon();
 
-            enum ErrorCode { NetworkError = KJob::UserDefinedError,
-                             JsonParseError, TimeoutError, ContentAccessError,
-                             UserDefinedError = 512 };
+            Status status() const;
+            int error() const;
+            virtual QString errorString() const;
 
         signals:
             /**
-             * Emitted together with KJob::result() but only if there's no error.
+             * Emitted when the job is finished, in any case. It is used to notify
+             * observers that the job is terminated and that progress can be hidden.
+             *
+             * This should not be emitted directly by subclasses;
+             * use emitResult() instead.
+             *
+             * In general, to be notified of a job's completion, client code
+             * should connect to success() and failure()
+             * rather than finished(), so that kill() is indeed quiet.
+             * However if you store a list of jobs and they might get killed
+             * silently, then you must connect to this instead of result(),
+             * to avoid dangling pointers in your list.
+             *
+             * @param job the job that emitted this signal
+             * @internal
+             *
+             * @see success, failure
+             */
+            void finished(BaseJob* job);
+
+            /**
+             * Emitted when the job is finished (except when killed).
+             *
+             * Use error to know if the job was finished with error.
+             *
+             * @param job the job that emitted this signal
+             *
+             * @see success, failure
+             */
+            void result(BaseJob* job);
+
+            /**
+             * Emitted together with result() but only if there's no error.
              */
             void success(BaseJob*);
+
             /**
-             * Emitted together with KJob::result() if there's an error.
-             * Same as result(), this won't be emitted in case of kill(Quietly).
+             * Emitted together with result() if there's an error.
+             * Same as result(), this won't be emitted in case of kill().
              */
             void failure(BaseJob*);
 
@@ -68,23 +142,55 @@ namespace QMatrixClient
             virtual QString apiPath() const = 0;
             virtual QUrlQuery query() const;
             virtual QJsonObject data() const;
-            virtual void parseJson(const QJsonDocument& data);
-            
-            void fail( int errorCode, QString errorString );
-            QNetworkReply* networkReply() const;
 
+            /**
+             * Used by gotReply() slot to check the received reply for general
+             * issues such as network errors or access denial.
+             * Returning anything except NoError/Success prevents
+             * further parseReply()/parseJson() invocation.
+             *
+             * @param reply the reply received from the server
+             * @return the result of checking the reply
+             *
+             * @see gotReply
+             */
+            virtual Status checkReply(QNetworkReply* reply) const;
+
+            /**
+             * Processes the reply. By default, parses the reply into
+             * a QJsonDocument and calls parseJson() if it's a valid JSON.
+             *
+             * @param data raw contents of a HTTP reply from the server (without headers)
+             *
+             * @see gotReply, parseJson
+             */
+            virtual Status parseReply(QByteArray data);
+
+            /**
+             * Processes the JSON document received from the Matrix server.
+             * By default returns succesful status without analysing the JSON.
+             *
+             * @param json valid JSON document received from the server
+             *
+             * @see parseReply
+             */
+            virtual Status parseJson(const QJsonDocument&);
             
+            void setStatus(Status s);
+            void setStatus(int code, QString message);
+
         protected slots:
-            virtual void gotReply();
             void timeout();
             void sslErrors(const QList<QSslError>& errors);
 
-            //void networkError(QNetworkReply::NetworkError code);
-
+        private slots:
+            void gotReply();
 
         private:
+            void finishJob(bool emitResult);
+
             class Private;
-            Private* d;
+            QScopedPointer<Private> d;
     };
 }
 
diff --git a/jobs/checkauthmethods.cpp b/jobs/checkauthmethods.cpp
index 55d8632a..c93bf802 100644
--- a/jobs/checkauthmethods.cpp
+++ b/jobs/checkauthmethods.cpp
@@ -57,7 +57,8 @@ QString CheckAuthMethods::apiPath() const
     return "_matrix/client/r0/login";
 }
 
-void CheckAuthMethods::parseJson(const QJsonDocument& data)
+BaseJob::Status CheckAuthMethods::parseJson(const QJsonDocument& data)
 {
     // TODO
+    return { BaseJob::StatusCode::UserDefinedError, "Not implemented" };
 }
diff --git a/jobs/checkauthmethods.h b/jobs/checkauthmethods.h
index 2951e27f..36eaa01a 100644
--- a/jobs/checkauthmethods.h
+++ b/jobs/checkauthmethods.h
@@ -35,7 +35,7 @@ namespace QMatrixClient
             
         protected:
             QString apiPath() const override;
-            void parseJson(const QJsonDocument& data) override;
+            Status parseJson(const QJsonDocument& data) override;
             
         private:
             class Private;
diff --git a/jobs/joinroomjob.cpp b/jobs/joinroomjob.cpp
index 799d4926..8848aa70 100644
--- a/jobs/joinroomjob.cpp
+++ b/jobs/joinroomjob.cpp
@@ -54,18 +54,15 @@ QString JoinRoomJob::apiPath() const
     return QString("_matrix/client/r0/join/%1").arg(d->roomAlias);
 }
 
-void JoinRoomJob::parseJson(const QJsonDocument& data)
+BaseJob::Status JoinRoomJob::parseJson(const QJsonDocument& data)
 {
     QJsonObject json = data.object();
-    if( !json.contains("room_id") )
-    {
-        fail( BaseJob::UserDefinedError, "Something went wrong..." );
-        qDebug() << data;
-        return;
-    }
-    else
+    if( json.contains("room_id") )
     {
         d->roomId = json.value("room_id").toString();
+        return Success;
     }
-    emitResult();
+
+    qDebug() << data;
+    return { UserDefinedError, "No room_id in the JSON response" };
 }
diff --git a/jobs/joinroomjob.h b/jobs/joinroomjob.h
index 1329ca56..a6f4af21 100644
--- a/jobs/joinroomjob.h
+++ b/jobs/joinroomjob.h
@@ -35,7 +35,7 @@ namespace QMatrixClient
 
             protected:
                 QString apiPath() const override;
-                void parseJson(const QJsonDocument& data) override;
+                Status parseJson(const QJsonDocument& data) override;
 
             private:
                 class Private;
diff --git a/jobs/mediathumbnailjob.cpp b/jobs/mediathumbnailjob.cpp
index 48ed0ffb..1e434fbc 100644
--- a/jobs/mediathumbnailjob.cpp
+++ b/jobs/mediathumbnailjob.cpp
@@ -70,19 +70,11 @@ QUrlQuery MediaThumbnailJob::query() const
     return query;
 }
 
-void MediaThumbnailJob::gotReply()
+BaseJob::Status MediaThumbnailJob::parseReply(QByteArray data)
 {
-    if( networkReply()->error() != QNetworkReply::NoError )
-    {
-        qDebug() << "NetworkError!!!";
-        qDebug() << networkReply()->errorString();
-        fail( NetworkError, networkReply()->errorString() );
-        return;
-    }
-
-    if( !d->thumbnail.loadFromData( networkReply()->readAll() ) )
+    if( !d->thumbnail.loadFromData(data) )
     {
         qDebug() << "MediaThumbnailJob: could not read image data";
     }
-    emitResult();
+    return Success;
 }
diff --git a/jobs/mediathumbnailjob.h b/jobs/mediathumbnailjob.h
index 541163ed..3babf845 100644
--- a/jobs/mediathumbnailjob.h
+++ b/jobs/mediathumbnailjob.h
@@ -40,8 +40,7 @@ namespace QMatrixClient
             QString apiPath() const override;
             QUrlQuery query() const override;
 
-        protected slots:
-            void gotReply() override;
+            Status parseReply(QByteArray data) override;
 
         private:
             class Private;
diff --git a/jobs/passwordlogin.cpp b/jobs/passwordlogin.cpp
index 231dcce5..c85e4c13 100644
--- a/jobs/passwordlogin.cpp
+++ b/jobs/passwordlogin.cpp
@@ -80,15 +80,15 @@ QJsonObject PasswordLogin::data() const
     return json;
 }
 
-void PasswordLogin::parseJson(const QJsonDocument& data)
+BaseJob::Status PasswordLogin::parseJson(const QJsonDocument& data)
 {
     QJsonObject json = data.object();
     if( !json.contains("access_token") || !json.contains("home_server") || !json.contains("user_id") )
     {
-        fail( BaseJob::UserDefinedError, "Unexpected data" );
+        return { UserDefinedError, "No expected data" };
     }
     d->returned_token = json.value("access_token").toString();
     d->returned_server = json.value("home_server").toString();
     d->returned_id = json.value("user_id").toString();
-    emitResult();
+    return Success;
 }
diff --git a/jobs/passwordlogin.h b/jobs/passwordlogin.h
index 75a45cae..d7e42725 100644
--- a/jobs/passwordlogin.h
+++ b/jobs/passwordlogin.h
@@ -38,7 +38,7 @@ namespace QMatrixClient
         protected:
             QString apiPath() const override;
             QJsonObject data() const override;
-            void parseJson(const QJsonDocument& data) override;
+            Status parseJson(const QJsonDocument& data) override;
 
         private:
             class Private;
diff --git a/jobs/postmessagejob.cpp b/jobs/postmessagejob.cpp
index cf9b94fd..0a38da62 100644
--- a/jobs/postmessagejob.cpp
+++ b/jobs/postmessagejob.cpp
@@ -61,14 +61,11 @@ QJsonObject PostMessageJob::data() const
     return json;
 }
 
-void PostMessageJob::parseJson(const QJsonDocument& data)
+BaseJob::Status PostMessageJob::parseJson(const QJsonDocument& data)
 {
-    QJsonObject json = data.object();
-    if( !json.contains("event_id") )
-    {
-        fail( BaseJob::UserDefinedError, "Something went wrong..." );
-        qDebug() << data;
-        return;
-    }
-    emitResult();
+    if( data.object().contains("event_id") )
+        return Success;
+
+    qDebug() << data;
+    return { UserDefinedError, "No event_id in the JSON response" };
 }
diff --git a/jobs/postmessagejob.h b/jobs/postmessagejob.h
index 9d354240..73d72020 100644
--- a/jobs/postmessagejob.h
+++ b/jobs/postmessagejob.h
@@ -35,7 +35,7 @@ namespace QMatrixClient
         protected:
             QString apiPath() const override;
             QJsonObject data() const override;
-            void parseJson(const QJsonDocument& data) override;
+            Status parseJson(const QJsonDocument& data) override;
 
         private:
             class Private;
diff --git a/jobs/roommembersjob.cpp b/jobs/roommembersjob.cpp
index 002be75b..7fc44c63 100644
--- a/jobs/roommembersjob.cpp
+++ b/jobs/roommembersjob.cpp
@@ -56,10 +56,9 @@ QString RoomMembersJob::apiPath() const
     return QString("_matrix/client/r0/rooms/%1/members").arg(d->room->id());
 }
 
-void RoomMembersJob::parseJson(const QJsonDocument& data)
+BaseJob::Status RoomMembersJob::parseJson(const QJsonDocument& data)
 {
-    QJsonObject obj = data.object();
-    QJsonArray chunk = obj.value("chunk").toArray();
+    QJsonArray chunk = data.object().value("chunk").toArray();
     for( const QJsonValue& val : chunk )
     {
         State* state = State::fromJson(val.toObject());
@@ -67,5 +66,5 @@ void RoomMembersJob::parseJson(const QJsonDocument& data)
             d->states.append(state);
     }
     qDebug() << "States: " << d->states.count();
-    emitResult();
+    return Success;
 }
diff --git a/jobs/roommembersjob.h b/jobs/roommembersjob.h
index ffae4309..04803d67 100644
--- a/jobs/roommembersjob.h
+++ b/jobs/roommembersjob.h
@@ -35,8 +35,8 @@ namespace QMatrixClient
             QList<State*> states();
 
         protected:
-            virtual QString apiPath() const override;
-            virtual void parseJson(const QJsonDocument& data) override;
+            QString apiPath() const override;
+            Status parseJson(const QJsonDocument& data) override;
 
         private:
             class Private;
diff --git a/jobs/roommessagesjob.cpp b/jobs/roommessagesjob.cpp
index f1943d2c..ba075007 100644
--- a/jobs/roommessagesjob.cpp
+++ b/jobs/roommessagesjob.cpp
@@ -81,10 +81,10 @@ QUrlQuery RoomMessagesJob::query() const
     return query;
 }
 
-void RoomMessagesJob::parseJson(const QJsonDocument& data)
+BaseJob::Status RoomMessagesJob::parseJson(const QJsonDocument& data)
 {
     QJsonObject obj = data.object();
     d->events = eventListFromJson(obj.value("chunk").toArray());
     d->end = obj.value("end").toString();
-    emitResult();
+    return Success;
 }
diff --git a/jobs/roommessagesjob.h b/jobs/roommessagesjob.h
index c068f235..52a72f70 100644
--- a/jobs/roommessagesjob.h
+++ b/jobs/roommessagesjob.h
@@ -40,7 +40,7 @@ namespace QMatrixClient
         protected:
             QString apiPath() const override;
             QUrlQuery query() const override;
-            void parseJson(const QJsonDocument& data) override;
+            Status parseJson(const QJsonDocument& data) override;
 
         private:
             class Private;
diff --git a/jobs/syncjob.cpp b/jobs/syncjob.cpp
index c04a74d7..a9865b55 100644
--- a/jobs/syncjob.cpp
+++ b/jobs/syncjob.cpp
@@ -110,7 +110,7 @@ QUrlQuery SyncJob::query() const
     return query;
 }
 
-void SyncJob::parseJson(const QJsonDocument& data)
+BaseJob::Status SyncJob::parseJson(const QJsonDocument& data)
 {
     QJsonObject json = data.object();
     d->nextBatch = json.value("next_batch").toString();
@@ -134,8 +134,7 @@ void SyncJob::parseJson(const QJsonDocument& data)
         }
     }
 
-    emitResult();
-    qDebug() << objectName() << ": processing complete";
+    return Success;
 }
 
 void SyncRoomData::EventList::fromJson(const QJsonObject& roomContents)
diff --git a/jobs/syncjob.h b/jobs/syncjob.h
index 79185dc9..f08a6b56 100644
--- a/jobs/syncjob.h
+++ b/jobs/syncjob.h
@@ -76,7 +76,7 @@ namespace QMatrixClient
         protected:
             QString apiPath() const override;
             QUrlQuery query() const override;
-            void parseJson(const QJsonDocument& data) override;
+            Status parseJson(const QJsonDocument& data) override;
 
         private:
             class Private;