BaseJob: Use saved parameters instead of overriding apiPath(), query() and data() in each job class

CMakeLists.txt

@@ -59,7 +59,6 @@ set(libqmatrixclient_SRCS
    jobs/postreceiptjob.cpp
    jobs/joinroomjob.cpp
    jobs/leaveroomjob.cpp
-   jobs/roommembersjob.cpp
    jobs/roommessagesjob.cpp
    jobs/syncjob.cpp
    jobs/mediathumbnailjob.cpp

connection.cpp

@@ -189,9 +189,7 @@ void Connection::sync(int timeout)
 
 SyncJob* Connection::Private::startSyncJob(const QString& filter, int timeout)
 {
-    syncJob = new SyncJob(data, data->lastEvent());
-    syncJob->setFilter(filter);
-    syncJob->setTimeout(timeout);
+    syncJob = new SyncJob(data, data->lastEvent(), filter, timeout);
     syncJob->start();
     return syncJob;
 

jobs/basejob.cpp

@@ -41,13 +41,22 @@ struct NetworkReplyDeleter : public QScopedPointerDeleteLater
 class BaseJob::Private
 {
     public:
-        Private(ConnectionData* c, JobHttpType t, bool nt)
-            : connection(c), type(t), needsToken(nt)
+        Private(ConnectionData* c, JobHttpType t, QString endpoint,
+                const QUrlQuery& q, const Data& data, bool nt)
+            : connection(c), type(t), apiEndpoint(endpoint), requestQuery(q)
+            , requestData(data), needsToken(nt)
             , reply(nullptr), status(NoError)
-        {}
+        { }
 
+        inline void sendRequest();
+
         ConnectionData* connection;
+
+        // Contents for the network request
         JobHttpType type;
+        QString apiEndpoint;
+        QUrlQuery requestQuery;
+        Data requestData;
         bool needsToken;
 
         QScopedPointer<QNetworkReply, NetworkReplyDeleter> reply;
@@ -61,8 +70,10 @@ 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))
+BaseJob::BaseJob(ConnectionData* connection, JobHttpType type, QString name,
+                 QString endpoint, BaseJob::Query query, BaseJob::Data data,
+                 bool needsToken)
+    : d(new Private(connection, type, endpoint, query, data, needsToken))
 {
     setObjectName(name);
     connect (&d->timer, &QTimer::timeout, this, &BaseJob::timeout);
@@ -79,48 +90,63 @@ ConnectionData* BaseJob::connection() const
     return d->connection;
 }
 
-QJsonObject BaseJob::data() const
+const QUrlQuery&BaseJob::query() const
 {
-    return QJsonObject();
+    return d->requestQuery;
 }
 
-QUrlQuery BaseJob::query() const
+void BaseJob::setRequestQuery(const QUrlQuery& query)
 {
-    return QUrlQuery();
+    d->requestQuery = query;
 }
 
-void BaseJob::start()
+const BaseJob::Data&BaseJob::requestData() const
+{
+    return d->requestData;
+}
+
+void BaseJob::setRequestData(const BaseJob::Data& data)
 {
-    QUrl url = d->connection->baseUrl();
-    url.setPath( url.path() + "/" + apiPath() );
-    QUrlQuery query = this->query();
-    if( d->needsToken )
-        query.addQueryItem("access_token", connection()->accessToken());
-    url.setQuery(query);
-    QNetworkRequest req = QNetworkRequest(url);
+    d->requestData = data;
+}
+
+void BaseJob::Private::sendRequest()
+{
+    QUrl url = connection->baseUrl();
+    url.setPath( url.path() + "/" + apiEndpoint );
+    if (needsToken)
+        requestQuery.addQueryItem("access_token", connection->accessToken());
+    url.setQuery(requestQuery);
+
+    QNetworkRequest req {url};
     req.setHeader(QNetworkRequest::ContentTypeHeader, "application/json");
 #if (QT_VERSION >= QT_VERSION_CHECK(5, 6, 0))
     req.setAttribute(QNetworkRequest::FollowRedirectsAttribute, true);
     req.setMaximumRedirectsAllowed(10);
 #endif
-    QJsonDocument data = QJsonDocument(this->data());
-    switch( d->type )
+    switch( type )
     {
         case JobHttpType::GetJob:
-            d->reply.reset( d->connection->nam()->get(req) );
+            reply.reset( connection->nam()->get(req) );
             break;
         case JobHttpType::PostJob:
-            d->reply.reset( d->connection->nam()->post(req, data.toJson()) );
+            reply.reset( connection->nam()->post(req, requestData.serialize()) );
             break;
         case JobHttpType::PutJob:
-            d->reply.reset( d->connection->nam()->put(req, data.toJson()) );
+            reply.reset( connection->nam()->put(req, requestData.serialize()) );
+            break;
+        case JobHttpType::DeleteJob:
+            reply.reset( connection->nam()->deleteResource(req) );
             break;
     }
+}
+
+void BaseJob::start()
+{
+    d->sendRequest();
     connect( d->reply.data(), &QNetworkReply::sslErrors, this, &BaseJob::sslErrors );
     connect( d->reply.data(), &QNetworkReply::finished, this, &BaseJob::gotReply );
     d->timer.start( 120*1000 );
-//     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::gotReply()

jobs/basejob.h

@@ -32,8 +32,8 @@ namespace QMatrixClient
 {
     class ConnectionData;
 
-    enum class JobHttpType { GetJob, PutJob, PostJob };
-    
+    enum class JobHttpType { GetJob, PutJob, PostJob, DeleteJob };
+
     class BaseJob: public QObject
     {
             Q_OBJECT
@@ -50,6 +50,42 @@ namespace QMatrixClient
                 , UserDefinedError = 200
             };
 
+            /**
+             * A simple wrapper around QUrlQuery that allows its creation from
+             * a list of string pairs
+             */
+            class Query : public QUrlQuery
+            {
+                public:
+                    using QUrlQuery::QUrlQuery;
+                    Query() = default;
+                    Query(const QList< QPair<QString, QString> >& l)
+                    {
+                        setQueryItems(l);
+                    }
+            };
+            /**
+             * A simple wrapper around QJsonObject that represents a JSON data
+             * section of an HTTP request to a Matrix server. Facilitates
+             * creation from a list of key-value string pairs and dumping of
+             * a resulting JSON to a QByteArray.
+             */
+            class Data : public QJsonObject
+            {
+                public:
+                    using QJsonObject::QJsonObject;
+                    Data() = default;
+                    Data(const QList< QPair<QString, QString> >& l)
+                    {
+                        for (auto i: l)
+                            insert(i.first, i.second);
+                    }
+                    QByteArray serialize() const
+                    {
+                        return QJsonDocument(*this).toJson();
+                    }
+            };
+
             /**
              * 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,
@@ -72,8 +108,9 @@ namespace QMatrixClient
             };
 
         public:
-            BaseJob(ConnectionData* connection, JobHttpType type,
-                    QString name, bool needsToken=true);
+            BaseJob(ConnectionData* connection, JobHttpType type, QString name,
+                    QString endpoint, Query query = Query(), Data data = Data(),
+                    bool needsToken = true);
             virtual ~BaseJob();
 
             void start();
@@ -138,10 +175,10 @@ namespace QMatrixClient
         protected:
             ConnectionData* connection() const;
 
-            // to implement
-            virtual QString apiPath() const = 0;
-            virtual QUrlQuery query() const;
-            virtual QJsonObject data() const;
+            const QUrlQuery& query() const;
+            void setRequestQuery(const QUrlQuery& query);
+            const Data& requestData() const;
+            void setRequestData(const Data& data);
 
             /**
              * Used by gotReply() slot to check the received reply for general

jobs/checkauthmethods.cpp

@@ -37,7 +37,8 @@ class CheckAuthMethods::Private
 };
 
 CheckAuthMethods::CheckAuthMethods(ConnectionData* connection)
-    : BaseJob(connection, JobHttpType::GetJob, "CheckAuthMethods", false)
+    : BaseJob(connection, JobHttpType::GetJob, "CheckAuthMethods",
+              "_matrix/client/r0/login", Query(), Data(), false)
     , d(new Private)
 {
 }
@@ -52,11 +53,6 @@ QString CheckAuthMethods::session()
     return d->session;
 }
 
-QString CheckAuthMethods::apiPath() const
-{
-    return "_matrix/client/r0/login";
-}
-
 BaseJob::Status CheckAuthMethods::parseJson(const QJsonDocument& data)
 {
     // TODO

jobs/checkauthmethods.h

@@ -34,7 +34,6 @@ namespace QMatrixClient
             QString session();
 
         protected:
-            QString apiPath() const override;
             Status parseJson(const QJsonDocument& data) override;
 
         private:

jobs/joinroomjob.cpp

@@ -29,14 +29,13 @@ class JoinRoomJob::Private
 {
     public:
         QString roomId;
-        QString roomAlias;
 };
 
 JoinRoomJob::JoinRoomJob(ConnectionData* data, QString roomAlias)
-    : BaseJob(data, JobHttpType::PostJob, "JoinRoomJob")
+    : BaseJob(data, JobHttpType::PostJob, "JoinRoomJob",
+              QString("_matrix/client/r0/join/%1").arg(roomAlias))
     , d(new Private)
 {
-    d->roomAlias = roomAlias;
 }
 
 JoinRoomJob::~JoinRoomJob()
@@ -49,11 +48,6 @@ QString JoinRoomJob::roomId()
     return d->roomId;
 }
 
-QString JoinRoomJob::apiPath() const
-{
-    return QString("_matrix/client/r0/join/%1").arg(d->roomAlias);
-}
-
 BaseJob::Status JoinRoomJob::parseJson(const QJsonDocument& data)
 {
     QJsonObject json = data.object();

jobs/joinroomjob.h

@@ -34,7 +34,6 @@ namespace QMatrixClient
             QString roomId();
 
             protected:
-                QString apiPath() const override;
                 Status parseJson(const QJsonDocument& data) override;
 
             private:

jobs/leaveroomjob.cpp

@@ -18,33 +18,14 @@
 
 #include "leaveroomjob.h"
 
-#include <QtNetwork/QNetworkReply>
-
 #include "../room.h"
-#include "../connectiondata.h"
 
 using namespace QMatrixClient;
 
-class LeaveRoomJob::Private
-{
-    public:
-        Private(Room* r) : room(r) {}
-
-        Room* room;
-};
-
 LeaveRoomJob::LeaveRoomJob(ConnectionData* data, Room* room)
-    : BaseJob(data, JobHttpType::PostJob, "LeaveRoomJob")
-    , d(new Private(room))
-{
-}
+    : BaseJob(data, JobHttpType::PostJob, "LeaveRoomJob",
+              QString("_matrix/client/r0/rooms/%1/leave").arg(room->id()))
+{ }
 
 LeaveRoomJob::~LeaveRoomJob()
-{
-    delete d;
-}
-
-QString LeaveRoomJob::apiPath() const
-{
-    return QString("_matrix/client/r0/rooms/%1/leave").arg(d->room->id());
-}
+{ }

jobs/leaveroomjob.h

@@ -31,13 +31,6 @@ namespace QMatrixClient
         public:
             LeaveRoomJob(ConnectionData* data, Room* room);
             virtual ~LeaveRoomJob();
-
-        protected:
-            QString apiPath() const override;
-
-        private:
-            class Private;
-            Private* d;
     };
 }
 

jobs/logoutjob.cpp

@@ -21,15 +21,10 @@
 using namespace QMatrixClient;
 
 LogoutJob::LogoutJob(ConnectionData* connection)
-    : BaseJob(connection, JobHttpType::PostJob, "LogoutJob")
+    : BaseJob(connection, JobHttpType::PostJob, "LogoutJob", "/_matrix/client/r0/logout")
 {
 }
 
 LogoutJob::~LogoutJob()
 {
 }
-
-QString LogoutJob::apiPath() const
-{
-    return "/_matrix/client/r0/logout";
-}

jobs/logoutjob.h

@@ -27,8 +27,5 @@ namespace QMatrixClient
         public:
             LogoutJob(ConnectionData* connection);
             virtual ~LogoutJob();
-
-        protected:
-            QString apiPath() const override;
     };
 }