From 6ae8e3d78b5c4a75ca7d5ca88af730071047d148 Mon Sep 17 00:00:00 2001 From: Roman Plášil Date: Wed, 16 Aug 2017 13:56:13 +0800 Subject: Implement saving save to enable incremental sync even after shutdown --- jobs/syncjob.cpp | 32 ++++++++++++++------------------ jobs/syncjob.h | 19 ++++++++++++++----- 2 files changed, 28 insertions(+), 23 deletions(-) (limited to 'jobs') diff --git a/jobs/syncjob.cpp b/jobs/syncjob.cpp index 29ddc2e6..3adf6b0c 100644 --- a/jobs/syncjob.cpp +++ b/jobs/syncjob.cpp @@ -22,20 +22,13 @@ 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) + , d(new SyncData) { setLoggingCategory(SYNCJOB); QUrlQuery query; @@ -57,21 +50,26 @@ SyncJob::~SyncJob() delete d; } -QString SyncJob::nextBatch() const +QString SyncData::nextBatch() const { - return d->nextBatch; + return nextBatch_; } -SyncData&& SyncJob::takeRoomData() +SyncDataList&& SyncData::takeRoomData() { - return std::move(d->roomData); + return std::move(roomData); } BaseJob::Status SyncJob::parseJson(const QJsonDocument& data) { + d->parseJson(data); + return Success; +} + +void 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(); @@ -86,13 +84,11 @@ BaseJob::Status SyncJob::parseJson(const QJsonDocument& data) { 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(rs.size())); + roomData.reserve(static_cast(rs.size())); for( auto rkey: rs.keys() ) - d->roomData.emplace_back(rkey, roomState.enumVal, rs[rkey].toObject()); + roomData.emplace_back(rkey, roomState.enumVal, rs[rkey].toObject()); } - qCDebug(PROFILER) << "*** SyncJob::parseJson():" << et.elapsed() << "ms"; - - return Success; + qCDebug(PROFILER) << "*** SyncData::parseJson():" << et.elapsed() << "ms"; } SyncRoomData::SyncRoomData(const QString& roomId_, JoinState joinState_, diff --git a/jobs/syncjob.h b/jobs/syncjob.h index 07824e23..9dc221b5 100644 --- a/jobs/syncjob.h +++ b/jobs/syncjob.h @@ -67,7 +67,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; + using SyncDataList = std::vector; + + class SyncData { + public: + void parseJson(const QJsonDocument &data); + SyncDataList&& takeRoomData(); + QString nextBatch() const; + + private: + QString nextBatch_; + SyncDataList roomData; + }; class SyncJob: public BaseJob { @@ -77,14 +88,12 @@ namespace QMatrixClient int timeout = -1, const QString& presence = {}); virtual ~SyncJob(); - SyncData&& takeRoomData(); - QString nextBatch() const; + SyncData *data() const { return d; } protected: Status parseJson(const QJsonDocument& data) override; private: - class Private; - Private* d; + SyncData* d; }; } // namespace QMatrixClient -- cgit v1.2.3 From 9a26c52f8c3e71f803fd38128ecddfd6ce9748f6 Mon Sep 17 00:00:00 2001 From: Roman Plášil Date: Sat, 2 Sep 2017 00:58:58 +0800 Subject: Use status return type for parseJson --- connection.cpp | 7 +++---- jobs/syncjob.cpp | 3 ++- jobs/syncjob.h | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) (limited to 'jobs') diff --git a/connection.cpp b/connection.cpp index 97594e7d..fe0cb251 100644 --- a/connection.cpp +++ b/connection.cpp @@ -32,10 +32,9 @@ #include "jobs/mediathumbnailjob.h" #include -#include -#include -#include -#include +#include +#include +#include using namespace QMatrixClient; diff --git a/jobs/syncjob.cpp b/jobs/syncjob.cpp index 3adf6b0c..1e71e215 100644 --- a/jobs/syncjob.cpp +++ b/jobs/syncjob.cpp @@ -66,7 +66,7 @@ BaseJob::Status SyncJob::parseJson(const QJsonDocument& data) return Success; } -void SyncData::parseJson(const QJsonDocument &data) { +BaseJob::Status SyncData::parseJson(const QJsonDocument &data) { QElapsedTimer et; et.start(); QJsonObject json = data.object(); nextBatch_ = json.value("next_batch").toString(); @@ -89,6 +89,7 @@ void SyncData::parseJson(const QJsonDocument &data) { roomData.emplace_back(rkey, roomState.enumVal, rs[rkey].toObject()); } qCDebug(PROFILER) << "*** SyncData::parseJson():" << et.elapsed() << "ms"; + return Success; } SyncRoomData::SyncRoomData(const QString& roomId_, JoinState joinState_, diff --git a/jobs/syncjob.h b/jobs/syncjob.h index 9dc221b5..16ac5895 100644 --- a/jobs/syncjob.h +++ b/jobs/syncjob.h @@ -71,7 +71,7 @@ namespace QMatrixClient class SyncData { public: - void parseJson(const QJsonDocument &data); + BaseJob::Status parseJson(const QJsonDocument &data); SyncDataList&& takeRoomData(); QString nextBatch() const; -- cgit v1.2.3 From a7ee0dfacc2c571572240191b3cf0846a9e32998 Mon Sep 17 00:00:00 2001 From: Roman Plášil Date: Sun, 3 Sep 2017 14:43:05 +0800 Subject: More fixes --- connection.cpp | 3 +-- jobs/syncjob.cpp | 5 ++--- room.cpp | 17 ++++++++++------- room.h | 2 +- 4 files changed, 14 insertions(+), 13 deletions(-) (limited to 'jobs') diff --git a/connection.cpp b/connection.cpp index fe0cb251..d2acf928 100644 --- a/connection.cpp +++ b/connection.cpp @@ -333,8 +333,7 @@ void Connection::saveState(const QUrl &toFile) { QJsonObject rooms; for (auto i : this->roomMap()) { - QJsonObject roomObj; - i->toJson(roomObj); + QJsonObject roomObj = i->toJson(); rooms[i->id()] = roomObj; } diff --git a/jobs/syncjob.cpp b/jobs/syncjob.cpp index 1e71e215..bbec968e 100644 --- a/jobs/syncjob.cpp +++ b/jobs/syncjob.cpp @@ -62,8 +62,7 @@ SyncDataList&& SyncData::takeRoomData() BaseJob::Status SyncJob::parseJson(const QJsonDocument& data) { - d->parseJson(data); - return Success; + return d->parseJson(data); } BaseJob::Status SyncData::parseJson(const QJsonDocument &data) { @@ -89,7 +88,7 @@ BaseJob::Status SyncData::parseJson(const QJsonDocument &data) { roomData.emplace_back(rkey, roomState.enumVal, rs[rkey].toObject()); } qCDebug(PROFILER) << "*** SyncData::parseJson():" << et.elapsed() << "ms"; - return Success; + return BaseJob::Success; } SyncRoomData::SyncRoomData(const QString& roomId_, JoinState joinState_, diff --git a/room.cpp b/room.cpp index 3841eab8..212c8acd 100644 --- a/room.cpp +++ b/room.cpp @@ -119,7 +119,7 @@ class Room::Private void setLastReadEvent(User* u, const QString& eventId); rev_iter_pair_t promoteReadMarker(User* u, rev_iter_t newMarker); - void toJson(QJsonObject &out); + QJsonObject toJson() const; private: QString calculateDisplayname() const; @@ -877,7 +877,7 @@ void Room::Private::updateDisplayname() emit q->displaynameChanged(q); } -void Room::Private::toJson(QJsonObject &out) { +QJsonObject Room::Private::toJson() const { QJsonValue nowTimestamp { QDateTime::currentMSecsSinceEpoch() }; QJsonArray stateEvents; @@ -890,7 +890,7 @@ void Room::Private::toJson(QJsonObject &out) { nameEvent.insert("content", nameEventContent); stateEvents.append(nameEvent); - for (auto i : this->membersMap) { + for (const auto &i : this->membersMap) { QJsonObject content; content.insert("membership", QStringLiteral("join")); content.insert("displayname", i->displayname()); @@ -908,7 +908,7 @@ void Room::Private::toJson(QJsonObject &out) { { QJsonArray aliases; - for (auto i : this->aliases) { + for (const auto &i : this->aliases) { aliases.append(QJsonValue(i)); } @@ -935,11 +935,14 @@ void Room::Private::toJson(QJsonObject &out) { QJsonObject roomStateObj; roomStateObj.insert("events", stateEvents); - out.insert("state", roomStateObj); + + QJsonObject result; + result.insert("state", roomStateObj); + return result; } -void Room::toJson(QJsonObject &out) const { - d->toJson(out); +QJsonObject Room::toJson() const { + return d->toJson(); } MemberSorter Room::memberSorter() const diff --git a/room.h b/room.h index 9e363556..12de0f31 100644 --- a/room.h +++ b/room.h @@ -142,7 +142,7 @@ namespace QMatrixClient MemberSorter memberSorter() const; - void toJson(QJsonObject &out) const; + QJsonObject toJson() const; public slots: void postMessage(const QString& plainText, -- cgit v1.2.3 From 56d34ecab5eb35c426a6e64b034bf2507761dd09 Mon Sep 17 00:00:00 2001 From: Roman Plášil Date: Mon, 4 Sep 2017 10:35:45 +0800 Subject: Use SyncJob::SyncData as a plain member --- connection.cpp | 2 +- jobs/syncjob.cpp | 8 +------- jobs/syncjob.h | 5 ++--- 3 files changed, 4 insertions(+), 11 deletions(-) (limited to 'jobs') diff --git a/connection.cpp b/connection.cpp index d2acf928..9fc2f85b 100644 --- a/connection.cpp +++ b/connection.cpp @@ -160,7 +160,7 @@ void Connection::sync(int timeout) auto job = d->syncJob = callApi(d->data->lastEvent(), filter, timeout); connect( job, &SyncJob::success, [=] () { - onSyncSuccess(*job->data()); + onSyncSuccess(job->data()); d->syncJob = nullptr; emit syncDone(); }); diff --git a/jobs/syncjob.cpp b/jobs/syncjob.cpp index bbec968e..062f1b15 100644 --- a/jobs/syncjob.cpp +++ b/jobs/syncjob.cpp @@ -28,7 +28,6 @@ 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 SyncData) { setLoggingCategory(SYNCJOB); QUrlQuery query; @@ -45,11 +44,6 @@ SyncJob::SyncJob(const ConnectionData* connection, const QString& since, setMaxRetries(std::numeric_limits::max()); } -SyncJob::~SyncJob() -{ - delete d; -} - QString SyncData::nextBatch() const { return nextBatch_; @@ -62,7 +56,7 @@ SyncDataList&& SyncData::takeRoomData() BaseJob::Status SyncJob::parseJson(const QJsonDocument& data) { - return d->parseJson(data); + return d.parseJson(data); } BaseJob::Status SyncData::parseJson(const QJsonDocument &data) { diff --git a/jobs/syncjob.h b/jobs/syncjob.h index 16ac5895..80cc6735 100644 --- a/jobs/syncjob.h +++ b/jobs/syncjob.h @@ -86,14 +86,13 @@ namespace QMatrixClient explicit SyncJob(const ConnectionData* connection, const QString& since = {}, const QString& filter = {}, int timeout = -1, const QString& presence = {}); - virtual ~SyncJob(); - SyncData *data() const { return d; } + SyncData &data() { return d; } protected: Status parseJson(const QJsonDocument& data) override; private: - SyncData* d; + SyncData d; }; } // namespace QMatrixClient -- cgit v1.2.3 From 071856d31995f665ee9219b3e05510ab83f9f4d8 Mon Sep 17 00:00:00 2001 From: Roman Plášil Date: Mon, 4 Sep 2017 19:23:50 +0800 Subject: Use move on SyncData --- connection.cpp | 9 ++++----- connection.h | 2 +- jobs/syncjob.h | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) (limited to 'jobs') diff --git a/connection.cpp b/connection.cpp index 9fc2f85b..27f0a86f 100644 --- a/connection.cpp +++ b/connection.cpp @@ -160,7 +160,7 @@ void Connection::sync(int timeout) auto job = d->syncJob = callApi(d->data->lastEvent(), filter, timeout); connect( job, &SyncJob::success, [=] () { - onSyncSuccess(job->data()); + onSyncSuccess(job->takeData()); d->syncJob = nullptr; emit syncDone(); }); @@ -174,7 +174,7 @@ void Connection::sync(int timeout) }); } -void Connection::onSyncSuccess(SyncData &data) { +void Connection::onSyncSuccess(SyncData &&data) { d->data->setLastEvent(data.nextBatch()); for( auto&& roomData: data.takeRoomData() ) { @@ -333,8 +333,7 @@ void Connection::saveState(const QUrl &toFile) { QJsonObject rooms; for (auto i : this->roomMap()) { - QJsonObject roomObj = i->toJson(); - rooms[i->id()] = roomObj; + rooms[i->id()] = i->toJson(); } QJsonObject roomObj; @@ -372,5 +371,5 @@ void Connection::loadState(const QUrl &fromFile) { QJsonDocument doc { QJsonDocument::fromJson(data) }; SyncData sync; sync.parseJson(doc); - onSyncSuccess(sync); + onSyncSuccess(std::move(sync)); } diff --git a/connection.h b/connection.h index 0265d92f..ad161d7c 100644 --- a/connection.h +++ b/connection.h @@ -151,7 +151,7 @@ namespace QMatrixClient /** * Completes loading sync data. */ - void onSyncSuccess(SyncData &data); + void onSyncSuccess(SyncData &&data); private: class Private; diff --git a/jobs/syncjob.h b/jobs/syncjob.h index 80cc6735..2ded0df3 100644 --- a/jobs/syncjob.h +++ b/jobs/syncjob.h @@ -87,7 +87,7 @@ namespace QMatrixClient const QString& filter = {}, int timeout = -1, const QString& presence = {}); - SyncData &data() { return d; } + SyncData &&takeData() { return std::move(d); } protected: Status parseJson(const QJsonDocument& data) override; -- cgit v1.2.3 From d4cb62523585137dee7879f2143ae1b4dd62513d Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 19 Sep 2017 11:39:30 +0900 Subject: Fix a race condition leading to a crash on close It seems that some reply processing still might have happened after BaseJob::abandon() (caused in turn by destroying a Connection object), probably because the event from QNetworkReply landed in the event queue after BaseJob::abandon() but before actual deletion of a job object. Now countered by disconnecting from QNetworkReply signals in abandon() and stop(). --- jobs/basejob.cpp | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) (limited to 'jobs') diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index 26ceb268..8c9ef222 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -219,16 +219,17 @@ BaseJob::Status BaseJob::parseJson(const QJsonDocument&) void BaseJob::stop() { d->timer.stop(); - if (!d->reply) + if (d->reply) { - qCWarning(d->logCat) << this << "stopped with empty network reply"; - } - else if (d->reply->isRunning()) - { - 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 +321,9 @@ void BaseJob::setStatus(int code, QString message) void BaseJob::abandon() { + this->disconnect(); + if (d->reply) + d->reply->disconnect(this); deleteLater(); } -- cgit v1.2.3 From 0b11b06379fe668063ea5658a261f53f1dcf117a Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 19 Sep 2017 12:56:29 +0900 Subject: BaseJob: improved logging Your QT_LOGGING_RULES (especially useful with Qt 5.6 and newer) should work a bit better now: * "Job" prefix is no more needed because the Qt logging prefix (libqmatrixclient.jobs) says it already; * The "created" record didn't follow the logging category if overridden from the concrete job class (see SyncJob); so instead of "created" there's now much more useful "sending request" record. --- jobs/basejob.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'jobs') diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index 8c9ef222..3057ed75 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include @@ -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,6 +159,8 @@ 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 ); -- cgit v1.2.3 From 7e8c2ee1d00e43aab90030493c31aef0b4467f71 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Wed, 20 Sep 2017 13:00:21 +0900 Subject: Minor optimisations in sync data parsing --- jobs/syncjob.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'jobs') diff --git a/jobs/syncjob.cpp b/jobs/syncjob.cpp index 062f1b15..78a9e93f 100644 --- a/jobs/syncjob.cpp +++ b/jobs/syncjob.cpp @@ -59,27 +59,30 @@ BaseJob::Status SyncJob::parseJson(const QJsonDocument& data) return d.parseJson(data); } -BaseJob::Status SyncData::parseJson(const QJsonDocument &data) { +BaseJob::Status SyncData::parseJson(const QJsonDocument &data) +{ QElapsedTimer et; et.start(); + QJsonObject json = data.object(); 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 roomData.reserve(static_cast(rs.size())); - for( auto rkey: rs.keys() ) - roomData.emplace_back(rkey, roomState.enumVal, rs[rkey].toObject()); + for(auto roomIt = rs.begin(); roomIt != rs.end(); ++roomIt) + roomData.emplace_back(roomIt.key(), roomState.enumVal, + roomIt.value().toObject()); } qCDebug(PROFILER) << "*** SyncData::parseJson():" << et.elapsed() << "ms"; return BaseJob::Success; -- cgit v1.2.3 From b01591bddbcc4bcf3957feeb6b4b2875a9a2d978 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Wed, 20 Sep 2017 14:05:16 +0900 Subject: BaseJob: track the outcome of sendRequest() in the logs Also: no reason to start the job timer if the request is not running, so don't even bother. --- jobs/basejob.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'jobs') diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index 3057ed75..6b2ebc58 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -164,8 +164,14 @@ 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(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() -- cgit v1.2.3