Merge branch 'master' into kitsune-invite-kick

3 files changed, 54 insertions, 41 deletions

diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp
index 26ceb268..6b2ebc58 100644
--- a/jobs/basejob.cpp
+++ b/jobs/basejob.cpp
@@ -24,6 +24,7 @@
 #include <QtNetwork/QNetworkRequest>
 #include <QtNetwork/QNetworkReply>
 #include <QtCore/QTimer>
+#include <QtCore/QStringBuilder>
 
 #include <array>
 
@@ -76,7 +77,7 @@ class BaseJob::Private
 
 inline QDebug operator<<(QDebug dbg, const BaseJob* j)
 {
-    return dbg << "Job" << j->objectName();
+    return dbg << j->objectName();
 }
 
 BaseJob::BaseJob(const ConnectionData* connection, HttpVerb verb,
@@ -89,7 +90,6 @@ BaseJob::BaseJob(const ConnectionData* connection, HttpVerb verb,
     connect (&d->timer, &QTimer::timeout, this, &BaseJob::timeout);
     d->retryTimer.setSingleShot(true);
     connect (&d->retryTimer, &QTimer::timeout, this, &BaseJob::start);
-    qCDebug(d->logCat) << this << "created";
 }
 
 BaseJob::~BaseJob()
@@ -159,11 +159,19 @@ void BaseJob::start()
 {
     emit aboutToStart();
     d->retryTimer.stop(); // In case we were counting down at the moment
+    qCDebug(d->logCat) << this << "sending request to"
+                       << d->apiEndpoint % '?' % d->requestQuery.toString();
     d->sendRequest();
     connect( d->reply.data(), &QNetworkReply::sslErrors, this, &BaseJob::sslErrors );
     connect( d->reply.data(), &QNetworkReply::finished, this, &BaseJob::gotReply );
-    d->timer.start(getCurrentTimeout());
-    emit started();
+    if (d->reply->isRunning())
+    {
+        d->timer.start(getCurrentTimeout());
+        qCDebug(d->logCat) << this << "request has been sent";
+        emit started();
+    }
+    else
+        qCWarning(d->logCat) << this << "request could not start";
 }
 
 void BaseJob::gotReply()
@@ -219,16 +227,17 @@ BaseJob::Status BaseJob::parseJson(const QJsonDocument&)
 void BaseJob::stop()
 {
     d->timer.stop();
-    if (!d->reply)
-    {
-        qCWarning(d->logCat) << this << "stopped with empty network reply";
-    }
-    else if (d->reply->isRunning())
+    if (d->reply)
     {
-        qCWarning(d->logCat) << this << "stopped without ready network reply";
         d->reply->disconnect(this); // Ignore whatever comes from the reply
-        d->reply->abort();
+        if (d->reply->isRunning())
+        {
+            qCWarning(d->logCat) << this << "stopped without ready network reply";
+            d->reply->abort();
+        }
     }
+    else
+        qCWarning(d->logCat) << this << "stopped with empty network reply";
 }
 
 void BaseJob::finishJob()
@@ -320,6 +329,9 @@ void BaseJob::setStatus(int code, QString message)
 
 void BaseJob::abandon()
 {
+    this->disconnect();
+    if (d->reply)
+        d->reply->disconnect(this);
     deleteLater();
 }
 
diff --git a/jobs/syncjob.cpp b/jobs/syncjob.cpp
index 38cfcb2a..f679e6f4 100644
--- a/jobs/syncjob.cpp
+++ b/jobs/syncjob.cpp
@@ -22,20 +22,12 @@
 
 using namespace QMatrixClient;
 
-class SyncJob::Private
-{
-    public:
-        QString nextBatch;
-        SyncData roomData;
-};
-
 static size_t jobId = 0;
 
 SyncJob::SyncJob(const ConnectionData* connection, const QString& since,
                  const QString& filter, int timeout, const QString& presence)
     : BaseJob(connection, HttpVerb::Get, QString("SyncJob-%1").arg(++jobId),
               "_matrix/client/r0/sync")
-    , d(new Private)
 {
     setLoggingCategory(SYNCJOB);
     QUrlQuery query;
@@ -52,47 +44,48 @@ SyncJob::SyncJob(const ConnectionData* connection, const QString& since,
     setMaxRetries(std::numeric_limits<int>::max());
 }
 
-SyncJob::~SyncJob()
+QString SyncData::nextBatch() const
 {
-    delete d;
+    return nextBatch_;
 }
 
-QString SyncJob::nextBatch() const
+SyncDataList&& SyncData::takeRoomData()
 {
-    return d->nextBatch;
+    return std::move(roomData);
 }
 
-SyncData&& SyncJob::takeRoomData()
+BaseJob::Status SyncJob::parseJson(const QJsonDocument& data)
 {
-    return std::move(d->roomData);
+    return d.parseJson(data);
 }
 
-BaseJob::Status SyncJob::parseJson(const QJsonDocument& data)
+BaseJob::Status SyncData::parseJson(const QJsonDocument &data)
 {
     QElapsedTimer et; et.start();
+
     QJsonObject json = data.object();
-    d->nextBatch = json.value("next_batch").toString();
+    nextBatch_ = json.value("next_batch").toString();
     // TODO: presence
     // TODO: account_data
     QJsonObject rooms = json.value("rooms").toObject();
 
-    const struct { QString jsonKey; JoinState enumVal; } roomStates[]
+    static const struct { QString jsonKey; JoinState enumVal; } roomStates[]
     {
         { "join", JoinState::Join },
         { "invite", JoinState::Invite },
         { "leave", JoinState::Leave }
     };
-    for (auto roomState: roomStates)
+    for (const auto& roomState: roomStates)
     {
         const QJsonObject rs = rooms.value(roomState.jsonKey).toObject();
         // We have a Qt container on the right and an STL one on the left
-        d->roomData.reserve(static_cast<size_t>(rs.size()));
-        for( auto rkey: rs.keys() )
-            d->roomData.emplace_back(rkey, roomState.enumVal, rs[rkey].toObject());
+        roomData.reserve(static_cast<size_t>(rs.size()));
+        for(auto roomIt = rs.begin(); roomIt != rs.end(); ++roomIt)
+            roomData.emplace_back(roomIt.key(), roomState.enumVal,
+                                  roomIt.value().toObject());
     }
-    qCDebug(PROFILER) << "*** SyncJob::parseJson():" << et.elapsed() << "ms";
-
-    return Success;
+    qCDebug(PROFILER) << "*** SyncData::parseJson():" << et.elapsed() << "ms";
+    return BaseJob::Success;
 }
 
 SyncRoomData::SyncRoomData(const QString& roomId_, JoinState joinState_,
diff --git a/jobs/syncjob.h b/jobs/syncjob.h
index 57a87c9f..6697a265 100644
--- a/jobs/syncjob.h
+++ b/jobs/syncjob.h
@@ -66,7 +66,18 @@ Q_DECLARE_TYPEINFO(QMatrixClient::SyncRoomData, Q_MOVABLE_TYPE);
 namespace QMatrixClient
 {
     // QVector cannot work with non-copiable objects, std::vector can.
-    using SyncData = std::vector<SyncRoomData>;
+    using SyncDataList = std::vector<SyncRoomData>;
+
+    class SyncData {
+    public:
+        BaseJob::Status parseJson(const QJsonDocument &data);
+        SyncDataList&& takeRoomData();
+        QString nextBatch() const;
+
+    private:
+        QString nextBatch_;
+        SyncDataList roomData;
+    };
 
     class SyncJob: public BaseJob
     {
@@ -74,16 +85,13 @@ namespace QMatrixClient
             explicit SyncJob(const ConnectionData* connection, const QString& since = {},
                              const QString& filter = {},
                              int timeout = -1, const QString& presence = {});
-            virtual ~SyncJob();
 
-            SyncData&& takeRoomData();
-            QString nextBatch() const;
+            SyncData &&takeData() { return std::move(d); }
 
         protected:
             Status parseJson(const QJsonDocument& data) override;
 
         private:
-            class Private;
-            Private* d;
+            SyncData d;
     };
 }  // namespace QMatrixClient