Merge branch 'master' into kitsune-gtad

6 files changed, 132 insertions, 37 deletions

diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp
index 486956e1..0303af39 100644
--- a/jobs/basejob.cpp
+++ b/jobs/basejob.cpp
@@ -83,18 +83,6 @@ class BaseJob::Private
         LoggingCategory logCat = JOBS;
 };
 
-inline QDebug operator<<(QDebug dbg, const BaseJob* j)
-{
-    return dbg << j->objectName();
-}
-
-QDebug QMatrixClient::operator<<(QDebug dbg, const BaseJob::Status& s)
-{
-    QRegularExpression filter { "(access_token)(=|: )[-_A-Za-z0-9]+" };
-    return dbg << s.code << ':'
-               << QString(s.message).replace(filter, "\\1 HIDDEN");
-}
-
 BaseJob::BaseJob(HttpVerb verb, const QString& name, const QString& endpoint, bool needsToken)
     : BaseJob(verb, name, endpoint, Query { }, Data { }, needsToken)
 { }
@@ -234,8 +222,12 @@ void BaseJob::start(const ConnectionData* connData)
 {
     d->connection = connData;
     beforeStart(connData);
-    sendRequest();
-    afterStart(connData, d->reply.data());
+    if (status().good())
+        sendRequest();
+    if (status().good())
+        afterStart(connData, d->reply.data());
+    if (!status().good())
+        QTimer::singleShot(0, this, &BaseJob::finishJob);
 }
 
 void BaseJob::sendRequest()
@@ -268,9 +260,38 @@ void BaseJob::gotReply()
         setStatus(parseReply(d->reply.data()));
     else
     {
-        auto json = QJsonDocument::fromJson(d->reply->readAll()).object();
-        if (!json.isEmpty())
-            setStatus(IncorrectRequestError, json.value("error").toString());
+        const auto body = d->reply->readAll();
+        if (!body.isEmpty())
+        {
+            qCDebug(d->logCat).noquote() << "Error body:" << body;
+            auto json = QJsonDocument::fromJson(body).object();
+            if (json.isEmpty())
+                setStatus(IncorrectRequestError, body);
+            else {
+                if (error() == TooManyRequestsError ||
+                        json.value("errcode").toString() == "M_LIMIT_EXCEEDED")
+                {
+                    QString msg = tr("Too many requests");
+                    auto retryInterval = json.value("retry_after_ms").toInt(-1);
+                    if (retryInterval != -1)
+                        msg += tr(", next retry advised after %1 ms")
+                                .arg(retryInterval);
+                    else // We still have to figure some reasonable interval
+                        retryInterval = getNextRetryInterval();
+
+                    setStatus(TooManyRequestsError, msg);
+
+                    // Shortcut to retry instead of executing finishJob()
+                    stop();
+                    qCWarning(d->logCat)
+                            << this << "will retry in" << retryInterval;
+                    d->retryTimer.start(retryInterval);
+                    emit retryScheduled(d->retriesTaken, retryInterval);
+                    return;
+                }
+                setStatus(IncorrectRequestError, json.value("error").toString());
+            }
+        }
     }
 
     finishJob();
@@ -281,9 +302,12 @@ bool checkContentType(const QByteArray& type, const QByteArrayList& patterns)
     if (patterns.isEmpty())
         return true;
 
+    // ignore possible appendixes of the content type
+    const auto ctype = type.split(';').front();
+
     for (const auto& pattern: patterns)
     {
-        if (pattern.startsWith('*') || type == pattern) // Fast lane
+        if (pattern.startsWith('*') || ctype == pattern) // Fast lane
             return true;
 
         auto patternParts = pattern.split('/');
@@ -291,7 +315,7 @@ bool checkContentType(const QByteArray& type, const QByteArrayList& patterns)
             "BaseJob: Expected content type should have up to two"
             " /-separated parts; violating pattern: " + pattern);
 
-        if (type.split('/').front() == patternParts.front() &&
+        if (ctype.split('/').front() == patternParts.front() &&
                 patternParts.back() == "*")
             return true; // Exact match already went on fast lane
     }
@@ -301,17 +325,26 @@ bool checkContentType(const QByteArray& type, const QByteArrayList& patterns)
 
 BaseJob::Status BaseJob::checkReply(QNetworkReply* reply) const
 {
-    qCDebug(d->logCat) << this << "returned"
+    const auto httpCode =
+            reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
+    qCDebug(d->logCat).nospace().noquote() << this << " returned HTTP code "
+        << httpCode << ": "
         << (reply->error() == QNetworkReply::NoError ?
                 "Success" : reply->errorString())
-        << "from" << reply->url().toDisplayString();
+        << " (URL: " << reply->url().toDisplayString() << ")";
+
+    if (httpCode == 429) // Qt doesn't know about it yet
+        return { TooManyRequestsError, tr("Too many requests") };
+
+    // Should we check httpCode instead? Maybe even use it in BaseJob::Status?
+    // That would make codes in logs slightly more readable.
     switch( reply->error() )
     {
         case QNetworkReply::NoError:
             if (checkContentType(reply->rawHeader("Content-Type"),
                                  d->expectedContentTypes))
                 return NoError;
-            else
+            else // A warning in the logs might be more proper instead
                 return { IncorrectResponseError,
                          "Incorrect content type of the response" };
 
@@ -372,9 +405,10 @@ void BaseJob::finishJob()
         // TODO: The whole retrying thing should be put to ConnectionManager
         // otherwise independently retrying jobs make a bit of notification
         // storm towards the UI.
-        const auto retryInterval = getNextRetryInterval();
+        const auto retryInterval =
+                error() == TimeoutError ? 0 : getNextRetryInterval();
         ++d->retriesTaken;
-        qCWarning(d->logCat) << this << "will take retry" << d->retriesTaken
+        qCWarning(d->logCat) << this << "will retry" << d->retriesTaken
                    << "in" << retryInterval/1000 << "s";
         d->retryTimer.start(retryInterval);
         emit retryScheduled(d->retriesTaken, retryInterval);
@@ -447,6 +481,7 @@ void BaseJob::setStatus(Status s)
 
 void BaseJob::setStatus(int code, QString message)
 {
+    message.replace(d->connection->accessToken(), "(REDACTED)");
     setStatus({ code, message });
 }
 
diff --git a/jobs/basejob.h b/jobs/basejob.h
index a5b457c5..ed630a67 100644
--- a/jobs/basejob.h
+++ b/jobs/basejob.h
@@ -62,6 +62,7 @@ namespace QMatrixClient
                 , NotFoundError
                 , IncorrectRequestError
                 , IncorrectResponseError
+                , TooManyRequestsError
                 , UserDefinedError = 200
             };
 
@@ -98,7 +99,12 @@ namespace QMatrixClient
                     Status(int c, QString m) : code(c), message(std::move(m)) { }
 
                     bool good() const { return code < ErrorLevel; }
-                    friend QDebug operator<<(QDebug dbg, const Status& s);
+                    friend QDebug operator<<(QDebug dbg, const Status& s)
+                    {
+                        QDebug(dbg).noquote().nospace()
+                                << s.code << ": " << s.message;
+                        return dbg;
+                    }
 
                     int code;
                     QString message;
@@ -124,6 +130,11 @@ namespace QMatrixClient
             Q_INVOKABLE duration_t getNextRetryInterval() const;
             Q_INVOKABLE duration_t millisToRetry() const;
 
+            friend QDebug operator<<(QDebug dbg, const BaseJob* j)
+            {
+                return dbg << j->objectName();
+            }
+
         public slots:
             void start(const ConnectionData* connData);
 
diff --git a/jobs/downloadfilejob.cpp b/jobs/downloadfilejob.cpp
index 07d14197..6a3d8483 100644
--- a/jobs/downloadfilejob.cpp
+++ b/jobs/downloadfilejob.cpp
@@ -54,6 +54,7 @@ void DownloadFileJob::beforeStart(const ConnectionData*)
         qCWarning(JOBS) << "Couldn't open the temporary file"
                         << d->tempFile->fileName() << "for writing";
         setStatus(FileError, "Could not open the temporary download file");
+        return;
     }
     qCDebug(JOBS) << "Downloading to" << d->tempFile->fileName();
 }
diff --git a/jobs/postreadmarkersjob.h b/jobs/postreadmarkersjob.h
new file mode 100644
index 00000000..d0198821
--- /dev/null
+++ b/jobs/postreadmarkersjob.h
@@ -0,0 +1,37 @@
+/******************************************************************************
+ * Copyright (C) 2017 Kitsune Ral <kitsune-ral@users.sf.net>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#pragma once
+
+#include "basejob.h"
+
+using namespace QMatrixClient;
+
+class PostReadMarkersJob : public BaseJob
+{
+    public:
+        explicit PostReadMarkersJob(const QString& roomId,
+                                    const QString& readUpToEventId)
+            : BaseJob(HttpVerb::Post, "PostReadMarkersJob",
+                      QStringLiteral("_matrix/client/r0/rooms/%1/read_markers")
+                      .arg(roomId))
+        {
+            setRequestData(QJsonObject {{
+                QStringLiteral("m.fully_read"), readUpToEventId }});
+        }
+};
diff --git a/jobs/syncjob.cpp b/jobs/syncjob.cpp
index 7b066f4f..435dfd0e 100644
--- a/jobs/syncjob.cpp
+++ b/jobs/syncjob.cpp
@@ -68,25 +68,30 @@ BaseJob::Status SyncData::parseJson(const QJsonDocument &data)
 {
     QElapsedTimer et; et.start();
 
-    auto json { data.object() };
+    auto json = data.object();
     nextBatch_ = json.value("next_batch").toString();
     // TODO: presence
     accountData.fromJson(json);
 
     QJsonObject rooms = json.value("rooms").toObject();
-    for (size_t i = 0; i < JoinStateStrings.size(); ++i)
+    JoinStates::Int ii = 1; // ii is used to make a JoinState value
+    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(i),
+            roomData.emplace_back(roomIt.key(), JoinState(ii),
                                   roomIt.value().toObject());
     }
-    qCDebug(PROFILER) << "*** SyncData::parseJson():" << et.elapsed() << "ms";
+    qCDebug(PROFILER) << "*** SyncData::parseJson(): batch with"
+                      << rooms.size() << "room(s) in" << et;
     return BaseJob::Success;
 }
 
+const QString SyncRoomData::UnreadCountKey =
+        QStringLiteral("x-qmatrixclient.unread_count");
+
 SyncRoomData::SyncRoomData(const QString& roomId_, JoinState joinState_,
                            const QJsonObject& room_)
     : roomId(roomId_)
@@ -114,12 +119,15 @@ SyncRoomData::SyncRoomData(const QString& roomId_, JoinState joinState_,
         qCWarning(SYNCJOB) << "SyncRoomData: Unknown JoinState value, ignoring:" << int(joinState);
     }
 
-    QJsonObject timeline = room_.value("timeline").toObject();
-    timelineLimited = timeline.value("limited").toBool();
-    timelinePrevBatch = timeline.value("prev_batch").toString();
-
-    QJsonObject unread = room_.value("unread_notifications").toObject();
-    highlightCount = unread.value("highlight_count").toInt();
-    notificationCount = unread.value("notification_count").toInt();
-    qCDebug(SYNCJOB) << "Highlights: " << highlightCount << " Notifications:" << notificationCount;
+    auto timelineJson = room_.value("timeline").toObject();
+    timelineLimited = timelineJson.value("limited").toBool();
+    timelinePrevBatch = timelineJson.value("prev_batch").toString();
+
+    auto unreadJson = room_.value("unread_notifications").toObject();
+    unreadCount = unreadJson.value(UnreadCountKey).toInt(-2);
+    highlightCount = unreadJson.value("highlight_count").toInt();
+    notificationCount = unreadJson.value("notification_count").toInt();
+    if (highlightCount > 0 || notificationCount > 0)
+        qCDebug(SYNCJOB) << "Highlights: " << highlightCount
+                         << " Notifications:" << notificationCount;
 }
diff --git a/jobs/syncjob.h b/jobs/syncjob.h
index 5956e73b..919060be 100644
--- a/jobs/syncjob.h
+++ b/jobs/syncjob.h
@@ -53,6 +53,7 @@ namespace QMatrixClient
 
             bool timelineLimited;
             QString timelinePrevBatch;
+            int unreadCount;
             int highlightCount;
             int notificationCount;
 
@@ -60,6 +61,8 @@ namespace QMatrixClient
                          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>;