Merge branch 'upstream' (v0.5.2)

6 files changed, 83 insertions, 172 deletions

diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp
index b21173ae..0d9b9f10 100644
--- a/lib/jobs/basejob.cpp
+++ b/lib/jobs/basejob.cpp
@@ -186,7 +186,7 @@ QUrl BaseJob::makeRequestUrl(QUrl baseUrl,
     if (!pathBase.endsWith('/') && !path.startsWith('/'))
         pathBase.push_back('/');
 
-    baseUrl.setPath( pathBase + path );
+    baseUrl.setPath(pathBase + path, QUrl::TolerantMode);
     baseUrl.setQuery(query);
     return baseUrl;
 }
@@ -325,9 +325,16 @@ void BaseJob::gotReply()
                 d->status.code = UserConsentRequiredError;
                 d->errorUrl = json.value("consent_uri"_ls).toString();
             }
-            else if (!json.isEmpty()) // Not localisable on the client side
-                setStatus(IncorrectRequestError,
-                          json.value("error"_ls).toString());
+            else if (errCode == "M_UNSUPPORTED_ROOM_VERSION" ||
+                     errCode == "M_INCOMPATIBLE_ROOM_VERSION")
+            {
+                d->status.code = UnsupportedRoomVersionError;
+                if (json.contains("room_version"))
+                    d->status.message =
+                        tr("Requested room version: %1")
+                        .arg(json.value("room_version").toString());
+            } else if (!json.isEmpty()) // Not localisable on the client side
+                setStatus(d->status.code, json.value("error"_ls).toString());
         }
     }
 
@@ -422,12 +429,12 @@ BaseJob::Status BaseJob::doCheckReply(QNetworkReply* reply) const
 BaseJob::Status BaseJob::parseReply(QNetworkReply* reply)
 {
     d->rawResponse = reply->readAll();
-    QJsonParseError error;
+    QJsonParseError error { 0, QJsonParseError::MissingObject };
     const auto& json = QJsonDocument::fromJson(d->rawResponse, &error);
     if( error.error == QJsonParseError::NoError )
         return parseJson(json);
-    else
-        return { IncorrectResponseError, error.errorString() };
+
+    return { IncorrectResponseError, error.errorString() };
 }
 
 BaseJob::Status BaseJob::parseJson(const QJsonDocument&)
@@ -519,8 +526,19 @@ BaseJob::Status BaseJob::status() const
 
 QByteArray BaseJob::rawData(int bytesAtMost) const
 {
-    return bytesAtMost > 0 && d->rawResponse.size() > bytesAtMost ?
-        d->rawResponse.left(bytesAtMost) + "...(truncated)" : d->rawResponse;
+    return bytesAtMost > 0 && d->rawResponse.size() > bytesAtMost
+            ? d->rawResponse.left(bytesAtMost) : d->rawResponse;
+}
+
+QString BaseJob::rawDataSample(int bytesAtMost) const
+{
+    auto data = rawData(bytesAtMost);
+    Q_ASSERT(data.size() <= d->rawResponse.size());
+    return data.size() == d->rawResponse.size()
+            ? data : data + tr("...(truncated, %Ln bytes in total)",
+                               "Comes after trimmed raw network response",
+                               d->rawResponse.size());
+
 }
 
 QString BaseJob::statusCaption() const
@@ -557,6 +575,8 @@ QString BaseJob::statusCaption() const
             return tr("Network authentication required");
         case UserConsentRequiredError:
             return tr("User consent required");
+        case UnsupportedRoomVersionError:
+            return tr("The server does not support the needed room version");
         default:
             return tr("Request failed");
     }
@@ -579,10 +599,25 @@ QUrl BaseJob::errorUrl() const
 
 void BaseJob::setStatus(Status s)
 {
+    // The crash that led to this code has been reported in
+    // https://github.com/QMatrixClient/Quaternion/issues/566 - basically,
+    // when cleaning up childrent of a deleted Connection, there's a chance
+    // of pending jobs being abandoned, calling setStatus(Abandoned).
+    // There's nothing wrong with this; however, the safety check for
+    // cleartext access tokens below uses d->connection - which is a dangling
+    // pointer.
+    // To alleviate that, a stricter condition is applied, that for Abandoned
+    // and to-be-Abandoned jobs the status message will be disregarded entirely.
+    // For 0.6 we might rectify the situation by making d->connection
+    // a QPointer<> (and derive ConnectionData from QObject, respectively).
+    if (d->status.code == Abandoned || s.code == Abandoned)
+        s.message.clear();
+
     if (d->status == s)
         return;
 
-    if (!d->connection->accessToken().isEmpty())
+    if (!s.message.isEmpty()
+            && d->connection && !d->connection->accessToken().isEmpty())
         s.message.replace(d->connection->accessToken(), "(REDACTED)");
     if (!s.good())
         qCWarning(d->logCat) << this << "status" << s;
@@ -597,9 +632,8 @@ void BaseJob::setStatus(int code, QString message)
 
 void BaseJob::abandon()
 {
-    beforeAbandon(d->reply.data());
+    beforeAbandon(d->reply ? d->reply.data() : nullptr);
     setStatus(Abandoned);
-    this->disconnect();
     if (d->reply)
         d->reply->disconnect(this);
     emit finished(this);
diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h
index 4ef25ab8..4c1c7706 100644
--- a/lib/jobs/basejob.h
+++ b/lib/jobs/basejob.h
@@ -64,6 +64,7 @@ namespace QMatrixClient
                 , IncorrectResponseError
                 , TooManyRequestsError
                 , RequestNotImplementedError
+                , UnsupportedRoomVersionError
                 , NetworkAuthRequiredError
                 , UserConsentRequiredError
                 , UserDefinedError = 200
@@ -138,8 +139,20 @@ namespace QMatrixClient
             Status status() const;
             /** Short human-friendly message on the job status */
             QString statusCaption() const;
-            /** Raw response body as received from the server */
+            /** Get raw response body as received from the server
+             * \param bytesAtMost return this number of leftmost bytes, or -1
+             *                    to return the entire response
+             */
             QByteArray rawData(int bytesAtMost = -1) const;
+            /** Get UI-friendly sample of raw data
+             *
+             * This is almost the same as rawData but appends the "truncated"
+             * suffix if not all data fit in bytesAtMost. This call is
+             * recommended to present a sample of raw data as "details" next to
+             * error messages. Note that the default \p bytesAtMost value is
+             * also tailored to UI cases.
+             */
+            QString rawDataSample(int bytesAtMost = 65535) const;
 
             /** Error (more generally, status) code
              * Equivalent to status().code
@@ -203,9 +216,9 @@ namespace QMatrixClient
              *
              * In general, to be notified of a job's completion, client code
              * should connect to result(), success(), or failure()
-             * rather than finished(). However if you store a list of jobs
-             * and need to track their lifecycle, then you should connect to this
-             * instead of result(), to avoid dangling pointers in your list.
+             * rather than finished(). However if you need to track the job's
+             * lifecycle you should connect to this instead of result();
+             * in particular, only this signal will be emitted on abandoning.
              *
              * @param job the job that emitted this signal
              *
diff --git a/lib/jobs/downloadfilejob.cpp b/lib/jobs/downloadfilejob.cpp
index 2bf9dd8f..672a7b2d 100644
--- a/lib/jobs/downloadfilejob.cpp
+++ b/lib/jobs/downloadfilejob.cpp
@@ -22,7 +22,8 @@ class DownloadFileJob::Private
 
 QUrl DownloadFileJob::makeRequestUrl(QUrl baseUrl, const QUrl& mxcUri)
 {
-    return makeRequestUrl(baseUrl, mxcUri.authority(), mxcUri.path().mid(1));
+    return makeRequestUrl(
+                std::move(baseUrl), mxcUri.authority(), mxcUri.path().mid(1));
 }
 
 DownloadFileJob::DownloadFileJob(const QString& serverName,
@@ -31,7 +32,7 @@ DownloadFileJob::DownloadFileJob(const QString& serverName,
     : GetContentJob(serverName, mediaId)
     , d(localFilename.isEmpty() ? new Private : new Private(localFilename))
 {
-    setObjectName("DownloadFileJob");
+    setObjectName(QStringLiteral("DownloadFileJob"));
 }
 
 QString DownloadFileJob::targetFileName() const
diff --git a/lib/jobs/mediathumbnailjob.cpp b/lib/jobs/mediathumbnailjob.cpp
index aeb49839..edb9b156 100644
--- a/lib/jobs/mediathumbnailjob.cpp
+++ b/lib/jobs/mediathumbnailjob.cpp
@@ -59,5 +59,5 @@ BaseJob::Status MediaThumbnailJob::parseReply(QNetworkReply* reply)
     if( _thumbnail.loadFromData(data()->readAll()) )
         return Success;
 
-    return { IncorrectResponseError, "Could not read image data" };
+    return { IncorrectResponseError, QStringLiteral("Could not read image data") };
 }
diff --git a/lib/jobs/syncjob.cpp b/lib/jobs/syncjob.cpp
index 6baf388e..84385b55 100644
--- a/lib/jobs/syncjob.cpp
+++ b/lib/jobs/syncjob.cpp
@@ -18,10 +18,6 @@
 
 #include "syncjob.h"
 
-#include "events/eventloader.h"
-
-#include <QtCore/QElapsedTimer>
-
 using namespace QMatrixClient;
 
 static size_t jobId = 0;
@@ -46,111 +42,21 @@ SyncJob::SyncJob(const QString& since, const QString& filter, int timeout,
     setMaxRetries(std::numeric_limits<int>::max());
 }
 
-QString SyncData::nextBatch() const
-{
-    return nextBatch_;
-}
-
-SyncDataList&& SyncData::takeRoomData()
-{
-    return std::move(roomData);
-}
-
-Events&& SyncData::takePresenceData()
-{
-    return std::move(presenceData);
-}
-
-Events&& SyncData::takeAccountData()
-{
-    return std::move(accountData);
-}
-
-Events&&SyncData::takeToDeviceEvents()
-{
-    return std::move(toDeviceEvents);
-}
-
-template <typename EventsArrayT, typename StrT>
-inline EventsArrayT load(const QJsonObject& batches, StrT keyName)
-{
-    return fromJson<EventsArrayT>(batches[keyName].toObject().value("events"_ls));
-}
+SyncJob::SyncJob(const QString& since, const Filter& filter,
+                 int timeout, const QString& presence)
+    : SyncJob(since,
+              QJsonDocument(toJson(filter)).toJson(QJsonDocument::Compact),
+              timeout, presence)
+{ }
 
 BaseJob::Status SyncJob::parseJson(const QJsonDocument& data)
 {
-    return d.parseJson(data);
-}
-
-BaseJob::Status SyncData::parseJson(const QJsonDocument &data)
-{
-    QElapsedTimer et; et.start();
-
-    auto json = data.object();
-    nextBatch_ = json.value("next_batch"_ls).toString();
-    presenceData = load<Events>(json, "presence"_ls);
-    accountData = load<Events>(json, "account_data"_ls);
-    toDeviceEvents = load<Events>(json, "to_device"_ls);
+    d.parseJson(data.object());
+    if (d.unresolvedRooms().isEmpty())
+        return BaseJob::Success;
 
-    auto rooms = json.value("rooms"_ls).toObject();
-    JoinStates::Int ii = 1; // ii is used to make a JoinState value
-    auto totalRooms = 0;
-    auto totalEvents = 0;
-    for (size_t i = 0; i < JoinStateStrings.size(); ++i, ii <<= 1)
-    {
-        const auto rs = rooms.value(JoinStateStrings[i]).toObject();
-        // We have a Qt container on the right and an STL one on the left
-        roomData.reserve(static_cast<size_t>(rs.size()));
-        for(auto roomIt = rs.begin(); roomIt != rs.end(); ++roomIt)
-        {
-            roomData.emplace_back(roomIt.key(), JoinState(ii),
-                                  roomIt.value().toObject());
-            const auto& r = roomData.back();
-            totalEvents += r.state.size() + r.ephemeral.size() +
-                           r.accountData.size() + r.timeline.size();
-        }
-        totalRooms += rs.size();
-    }
-    if (totalRooms > 9 || et.nsecsElapsed() >= profilerMinNsecs())
-        qCDebug(PROFILER) << "*** SyncData::parseJson(): batch with"
-                          << totalRooms << "room(s),"
-                          << totalEvents << "event(s) in" << et;
-    return BaseJob::Success;
+    qCCritical(MAIN).noquote() << "Incomplete sync response, missing rooms:"
+                               << d.unresolvedRooms().join(',');
+    return BaseJob::IncorrectResponseError;
 }
 
-const QString SyncRoomData::UnreadCountKey =
-        QStringLiteral("x-qmatrixclient.unread_count");
-
-SyncRoomData::SyncRoomData(const QString& roomId_, JoinState joinState_,
-                           const QJsonObject& room_)
-    : roomId(roomId_)
-    , joinState(joinState_)
-    , state(load<StateEvents>(room_,
-                joinState == JoinState::Invite ? "invite_state"_ls : "state"_ls))
-{
-    switch (joinState) {
-        case JoinState::Join:
-            ephemeral = load<Events>(room_, "ephemeral"_ls);
-            FALLTHROUGH;
-        case JoinState::Leave:
-        {
-            accountData = load<Events>(room_, "account_data"_ls);
-            timeline = load<RoomEvents>(room_, "timeline"_ls);
-            const auto timelineJson = room_.value("timeline"_ls).toObject();
-            timelineLimited = timelineJson.value("limited"_ls).toBool();
-            timelinePrevBatch = timelineJson.value("prev_batch"_ls).toString();
-
-            break;
-        }
-        default: /* nothing on top of state */;
-    }
-
-    const auto unreadJson = room_.value("unread_notifications"_ls).toObject();
-    unreadCount = unreadJson.value(UnreadCountKey).toInt(-2);
-    highlightCount = unreadJson.value("highlight_count"_ls).toInt();
-    notificationCount = unreadJson.value("notification_count"_ls).toInt();
-    if (highlightCount > 0 || notificationCount > 0)
-        qCDebug(SYNCJOB) << "Room" << roomId_
-                         << "has highlights:" << highlightCount
-                         << "and notifications:" << notificationCount;
-}
diff --git a/lib/jobs/syncjob.h b/lib/jobs/syncjob.h
index 6b9bedfa..036b25d0 100644
--- a/lib/jobs/syncjob.h
+++ b/lib/jobs/syncjob.h
@@ -20,62 +20,19 @@
 
 #include "basejob.h"
 
-#include "joinstate.h"
-#include "events/stateevent.h"
-#include "util.h"
+#include "../syncdata.h"
+#include "../csapi/definitions/sync_filter.h"
 
 namespace QMatrixClient
 {
-    class SyncRoomData
-    {
-        public:
-            QString roomId;
-            JoinState joinState;
-            StateEvents state;
-            RoomEvents timeline;
-            Events ephemeral;
-            Events accountData;
-
-            bool timelineLimited;
-            QString timelinePrevBatch;
-            int unreadCount;
-            int highlightCount;
-            int notificationCount;
-
-            SyncRoomData(const QString& roomId, JoinState joinState_,
-                         const QJsonObject& room_);
-            SyncRoomData(SyncRoomData&&) = default;
-            SyncRoomData& operator=(SyncRoomData&&) = default;
-
-            static const QString UnreadCountKey;
-    };
-    // QVector cannot work with non-copiable objects, std::vector can.
-    using SyncDataList = std::vector<SyncRoomData>;
-
-    class SyncData
-    {
-        public:
-            BaseJob::Status parseJson(const QJsonDocument &data);
-            Events&& takePresenceData();
-            Events&& takeAccountData();
-            Events&& takeToDeviceEvents();
-            SyncDataList&& takeRoomData();
-            QString nextBatch() const;
-
-        private:
-            QString nextBatch_;
-            Events presenceData;
-            Events accountData;
-            Events toDeviceEvents;
-            SyncDataList roomData;
-    };
-
     class SyncJob: public BaseJob
     {
         public:
             explicit SyncJob(const QString& since = {},
                              const QString& filter = {},
                              int timeout = -1, const QString& presence = {});
+            explicit SyncJob(const QString& since, const Filter& filter,
+                             int timeout = -1, const QString& presence = {});
 
             SyncData &&takeData() { return std::move(d); }