From 2eb18a735a5f75a77387a211f4311222d00c2d6c Mon Sep 17 00:00:00 2001
From: Kitsune Ral <Kitsune-Ral@users.sf.net>
Date: Thu, 5 May 2016 19:19:52 +0900
Subject: Rewritten BaseJob to not depend on KJob.
Some parts of the code were copied from the KCoreAddons sources - surprisingly few, in fact, mostly API with comments. With this commit, libqmatrixclient doesn't depend on KCoreAddons.
---
CMakeLists.txt | 48 ------------------------
connectionprivate.cpp | 64 +------------------------------
connectionprivate.h | 11 ++----
jobs/basejob.cpp | 67 ++++++++++++++++++++++++++-------
jobs/basejob.h | 102 +++++++++++++++++++++++++++++++++++++++++---------
5 files changed, 143 insertions(+), 149 deletions(-)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 3745f3c2..fa5abe56 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -10,25 +10,6 @@ set(CMAKE_INCLUDE_CURRENT_DIR ON)
# Instruct CMake to run moc automatically when needed.
set(CMAKE_AUTOMOC ON)
-# Whether to build with the bundled KCoreAddons or system KCoreAddons
-set( BUNDLE_KCOREADDONS "AUTO" CACHE STRING "Build own KCoreAddons, one of ON, OFF and AUTO" )
-set( KCOREADDONS_DIR "kcoreaddons" CACHE STRING "Local path to bundled KCoreAddons sources, if own KCoreAddons is built" )
-
-find_package(Qt5Core 5.2.0) # For JSON (de)serialization
-find_package(Qt5Network 5.2.0) # For networking
-find_package(Qt5Gui 5.2.0) # For userpics
-
-if ( (NOT BUNDLE_KCOREADDONS STREQUAL "ON")
- AND (NOT BUNDLE_KCOREADDONS STREQUAL "OFF")
- AND (NOT BUNDLE_KCOREADDONS STREQUAL "AUTO") )
- message( FATAL_ERROR "BUNDLE_KCOREADDONS must be one of ON, OFF or AUTO" )
-endif ()
-
-if ( BUNDLE_KCOREADDONS STREQUAL "AUTO" )
- find_package(KF5CoreAddons QUIET)
-elseif ( BUNDLE_KCOREADDONS STREQUAL "OFF" )
- find_package(KF5CoreAddons REQUIRED)
-endif ()
message( STATUS )
message( STATUS "================================================================================" )
@@ -37,14 +18,6 @@ message( STATUS "===============================================================
message( STATUS "Building with: ${CMAKE_CXX_COMPILER_ID} ${CMAKE_CXX_COMPILER_VERSION}" )
message( STATUS "Install Prefix: ${CMAKE_INSTALL_PREFIX}" )
message( STATUS "Path to Qt Core: ${Qt5Core_DIR}" )
-message( STATUS "Build own KCoreAddons (BUNDLE_KCOREADDONS): ${BUNDLE_KCOREADDONS}" )
-if ( NOT BUNDLE_KCOREADDONS STREQUAL "ON" )
- if ( KF5CoreAddons_FOUND )
- message( STATUS "'- Path to system KCoreAddons: ${KF5CoreAddons_DIR}" )
- else ( KF5CoreAddons_FOUND )
- message( STATUS "'- System KCoreAddons not found, using the bundled version at ${PROJECT_SOURCE_DIR}/${KCOREADDONS_DIR}" )
- endif ( KF5CoreAddons_FOUND )
-endif ( NOT BUNDLE_KCOREADDONS STREQUAL "ON" )
message( STATUS "================================================================================" )
message( STATUS )
@@ -80,16 +53,6 @@ set(libqmatrixclient_SRCS
jobs/mediathumbnailjob.cpp
jobs/logoutjob.cpp
)
-# Add bundled KCoreAddons sources if we haven't found the system sources
-# or if we ignore them
-if ( NOT KF5CoreAddons_FOUND )
- set (libqmatrixclient_SRCS ${libqmatrixclient_SRCS}
- ${KCOREADDONS_DIR}/src/lib/jobs/kjob.cpp
- ${KCOREADDONS_DIR}/src/lib/jobs/kcompositejob.cpp
- ${KCOREADDONS_DIR}/src/lib/jobs/kjobtrackerinterface.cpp
- ${KCOREADDONS_DIR}/src/lib/jobs/kjobuidelegate.cpp
- )
-endif ( NOT KF5CoreAddons_FOUND )
add_library(qmatrixclient ${libqmatrixclient_SRCS})
@@ -109,14 +72,3 @@ else ( CMAKE_VERSION VERSION_LESS "3.1" )
endif ( CMAKE_VERSION VERSION_LESS "3.1" )
target_link_libraries(qmatrixclient Qt5::Core Qt5::Network Qt5::Gui)
-if ( KF5CoreAddons_FOUND )
- # The proper way of doing things would be to make a separate config.h.in
- # file and use configure_file() command here to generate config.h with
- # needed C++ preprocessor macros. If we have more than one or two
- # dependencies like that, we should turn to that more scalable way.
- # As for now, passing a macro through -D is easier to observe and maintain.
- target_compile_definitions ( qmatrixclient PRIVATE USING_SYSTEM_KCOREADDONS )
- target_link_libraries(qmatrixclient KF5::CoreAddons)
-else ( KF5CoreAddons_FOUND )
- include_directories( ${KCOREADDONS_DIR}/src/lib/jobs )
-endif ( KF5CoreAddons_FOUND )
diff --git a/connectionprivate.cpp b/connectionprivate.cpp
index 62840473..4d49c014 100644
--- a/connectionprivate.cpp
+++ b/connectionprivate.cpp
@@ -116,69 +116,7 @@ Room* ConnectionPrivate::provideRoom(QString id)
return room;
}
-//void ConnectionPrivate::connectDone(KJob* job)
-//{
-// PasswordLogin* realJob = static_cast<PasswordLogin*>(job);
-// if( !realJob->error() )
-// {
-// isConnected = true;
-// userId = realJob->id();
-// qDebug() << "Our user ID: " << userId;
-// emit q->connected();
-// }
-// else {
-// emit q->loginError( job->errorString() );
-// }
-//}
-
-//void ConnectionPrivate::reconnectDone(KJob* job)
-//{
-// PasswordLogin* realJob = static_cast<PasswordLogin*>(job);
-// if( !realJob->error() )
-// {
-// userId = realJob->id();
-// emit q->reconnected();
-// }
-// else {
-// emit q->loginError( job->errorString() );
-// isConnected = false;
-// }
-//}
-
-//void ConnectionPrivate::syncDone(KJob* job)
-//{
-// SyncJob* syncJob = static_cast<SyncJob*>(job);
-// if( !syncJob->error() )
-// {
-// data->setLastEvent(syncJob->nextBatch());
-// processRooms(syncJob->roomData());
-// emit q->syncDone();
-// }
-// else {
-// if( syncJob->error() == BaseJob::NetworkError )
-// emit q->connectionError( syncJob->errorString() );
-// else
-// qDebug() << "syncJob failed, error:" << syncJob->error();
-// }
-//}
-
-//void ConnectionPrivate::gotJoinRoom(KJob* job)
-//{
-// qDebug() << "gotJoinRoom";
-// JoinRoomJob* joinJob = static_cast<JoinRoomJob*>(job);
-// if( !joinJob->error() )
-// {
-// if ( Room* r = provideRoom(joinJob->roomId()) )
-// emit q->joinedRoom(r);
-// }
-// else
-// {
-// if( joinJob->error() == BaseJob::NetworkError )
-// emit q->connectionError( joinJob->errorString() );
-// }
-//}
-
-void ConnectionPrivate::gotRoomMembers(KJob* job)
+void ConnectionPrivate::gotRoomMembers(BaseJob* job)
{
RoomMembersJob* membersJob = static_cast<RoomMembersJob*>(job);
if( !membersJob->error() )
diff --git a/connectionprivate.h b/connectionprivate.h
index 8e37a934..d1199081 100644
--- a/connectionprivate.h
+++ b/connectionprivate.h
@@ -19,15 +19,12 @@
#ifndef QMATRIXCLIENT_CONNECTIONPRIVATE_H
#define QMATRIXCLIENT_CONNECTIONPRIVATE_H
-class KJob;
-
#include <QtCore/QObject>
#include <QtCore/QHash>
#include <QtCore/QJsonObject>
#include "connection.h"
#include "connectiondata.h"
-#include "jobs/syncjob.h"
namespace QMatrixClient
{
@@ -35,6 +32,8 @@ namespace QMatrixClient
class Event;
class State;
class User;
+ class BaseJob;
+ class SyncRoomData;
class ConnectionPrivate : public QObject
{
@@ -60,11 +59,7 @@ namespace QMatrixClient
QString userId;
public slots:
-// void connectDone(KJob* job);
-// void reconnectDone(KJob* job);
-// void syncDone(KJob* job);
-// void gotJoinRoom(KJob* job);
- void gotRoomMembers(KJob* job);
+ void gotRoomMembers(BaseJob* job);
};
}
diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp
index 6c68ab66..2c95ef11 100644
--- a/jobs/basejob.cpp
+++ b/jobs/basejob.cpp
@@ -31,24 +31,21 @@ class BaseJob::Private
{
public:
Private(ConnectionData* c, JobHttpType t, bool nt)
- : connection(c), reply(nullptr), type(t), needsToken(nt) {}
+ : connection(c), reply(nullptr), type(t), needsToken(nt), errorCode(NoError)
+ {}
ConnectionData* connection;
QNetworkReply* reply;
JobHttpType type;
bool needsToken;
+
+ int errorCode;
+ QString errorText;
};
BaseJob::BaseJob(ConnectionData* connection, JobHttpType type, QString name, bool needsToken)
: d(new Private(connection, type, needsToken))
{
- // Work around KJob inability to separate success and failure signals
- connect(this, &BaseJob::result, [this]() {
- if (error() == NoError)
- emit success(this);
- else
- emit failure(this);
- });
setObjectName(name);
qDebug() << "Job" << objectName() << " created";
}
@@ -119,6 +116,55 @@ void BaseJob::start()
// this, &BaseJob::networkError ); // http://doc.qt.io/qt-5/qnetworkreply.html#error-1
}
+void BaseJob::finishJob(bool emitResult)
+{
+ if( d->reply->isRunning() )
+ d->reply->abort();
+
+ // Notify those that are interested in any completion of the job (including killing)
+ emit finished(this);
+
+ if (emitResult) {
+ emit result(this);
+ if (error())
+ emit failure(this);
+ else
+ emit success(this);
+ }
+
+ deleteLater();
+}
+
+int BaseJob::error() const
+{
+ return d->errorCode;
+}
+
+QString BaseJob::errorString() const
+{
+ return d->errorText;
+}
+
+void BaseJob::setError(int errorCode)
+{
+ d->errorCode = errorCode;
+}
+
+void BaseJob::setErrorText(QString errorText)
+{
+ d->errorText = errorText;
+}
+
+void BaseJob::emitResult()
+{
+ finishJob(true);
+}
+
+void BaseJob::abandon()
+{
+ finishJob(false);
+}
+
void BaseJob::fail(int errorCode, QString errorString)
{
setError( errorCode );
@@ -134,11 +180,6 @@ QNetworkReply* BaseJob::networkReply() const
return d->reply;
}
-// void BaseJob::networkError(QNetworkReply::NetworkError code)
-// {
-// fail( KJob::UserDefinedError+1, d->reply->errorString() );
-// }
-
void BaseJob::gotReply()
{
switch( d->reply->error() )
diff --git a/jobs/basejob.h b/jobs/basejob.h
index 150d39e8..9d00c0ac 100644
--- a/jobs/basejob.h
+++ b/jobs/basejob.h
@@ -19,12 +19,6 @@
#ifndef QMATRIXCLIENT_BASEJOB_H
#define QMATRIXCLIENT_BASEJOB_H
-#ifdef USING_SYSTEM_KCOREADDONS
-#include <KCoreAddons/KJob>
-#else
-#include "kjob.h"
-#endif // KCOREADDONS_FOUND
-
#include <QtCore/QJsonDocument>
#include <QtCore/QJsonObject>
#include <QtCore/QUrlQuery>
@@ -36,28 +30,75 @@ namespace QMatrixClient
enum class JobHttpType { GetJob, PutJob, PostJob };
- class BaseJob: public KJob
+ class BaseJob: public QObject
{
Q_OBJECT
public:
+ /* Just in case, the values are compatible with KJob
+ * (which BaseJob used to inherit from). */
+ enum ErrorCode { NoError = 0, NetworkError = 100,
+ JsonParseError, TimeoutError, ContentAccessError,
+ UserDefinedError = 512 };
+
BaseJob(ConnectionData* connection, JobHttpType type,
QString name, bool needsToken=true);
virtual ~BaseJob();
- void start() override;
+ void start();
- enum ErrorCode { NetworkError = KJob::UserDefinedError,
- JsonParseError, TimeoutError, ContentAccessError,
- UserDefinedError = 512 };
+ /**
+ * Abandons the result of this job, arrived or unarrived.
+ *
+ * This aborts waiting for a reply from the server (if there was
+ * any pending) and deletes the job object. It is always done quietly
+ * (as opposed to KJob::kill() that can trigger emitting the result).
+ */
+ void abandon();
+
+ int error() const;
+ virtual QString errorString() const;
signals:
/**
- * Emitted together with KJob::result() but only if there's no error.
+ * Emitted when the job is finished, in any case. It is used to notify
+ * observers that the job is terminated and that progress can be hidden.
+ *
+ * This should not be emitted directly by subclasses;
+ * use emitResult() instead.
+ *
+ * In general, to be notified of a job's completion, client code
+ * should connect to success() and failure()
+ * rather than finished(), so that kill() is indeed quiet.
+ * However if you store a list of jobs and they might get killed
+ * silently, then you must connect to this instead of result(),
+ * to avoid dangling pointers in your list.
+ *
+ * @param job the job that emitted this signal
+ * @internal
+ *
+ * @see success, failure
+ */
+ void finished(BaseJob* job);
+
+ /**
+ * Emitted when the job is finished (except when killed).
+ *
+ * Use error to know if the job was finished with error.
+ *
+ * @param job the job that emitted this signal
+ *
+ * @see success, failure
+ */
+ void result(BaseJob* job);
+
+ /**
+ * Emitted together with result() but only if there's no error.
*/
void success(BaseJob*);
+
/**
- * Emitted together with KJob::result() if there's an error.
- * Same as result(), this won't be emitted in case of kill(Quietly).
+ * Emitted together with result() if there's an error.
+ * Same as result(), this won't be emitted in case of kill().
*/
void failure(BaseJob*);
@@ -70,6 +111,34 @@ namespace QMatrixClient
virtual QJsonObject data() const;
virtual void parseJson(const QJsonDocument& data);
+ /**
+ * Sets the error code.
+ *
+ * It should be called when an error is encountered in the job,
+ * just before calling emitResult(). Normally you might want to
+ * use fail() instead - it sets error code, error text, makes sure
+ * the job has finished and invokes emitResult after that.
+ *
+ * To extend the list of error codes, define an (anonymous) enum
+ * with additional values starting at BaseJob::UserDefinedError
+ *
+ * @param errorCode the error code
+ * @see emitResult(), fail()
+ */
+ void setError(int errorCode);
+ void setErrorText(QString errorText);
+
+ /**
+ * Utility function to emit the result signal, and suicide this job.
+ * It first notifies the observers to hide the progress for this job using
+ * the finished() signal.
+ *
+ * @note: Deletes this job using deleteLater().
+ *
+ * @see result()
+ * @see finished()
+ */
+ void emitResult();
void fail( int errorCode, QString errorString );
QNetworkReply* networkReply() const;
@@ -79,10 +148,9 @@ namespace QMatrixClient
void timeout();
void sslErrors(const QList<QSslError>& errors);
- //void networkError(QNetworkReply::NetworkError code);
-
-
private:
+ void finishJob(bool emitResult);
+
class Private;
Private* d;
};
--
cgit v1.2.3
From 3254963c1ea587921d2a434839aa703b9aaff6cc Mon Sep 17 00:00:00 2001
From: Kitsune Ral <Kitsune-Ral@users.sf.net>
Date: Tue, 26 Jul 2016 16:49:16 +0900
Subject: Split BaseJob::gotReply into checkReply and parseReply + internal
tweaks to BaseJob
1. The externally (for derived classes) visible additions are checkReply() and parseReply() virtual methods, with gotReply becoming a mere dispatcher (and therefore a private method). Splitting gotReply() in that way allowed to remove boilerplate code from MediaThumbnailJob.
2. The internal tweak is using QScopedPointer<> to store pointers both to the Private object and to a QNetworkReply (with a special deleter that aborts the reply before destructing the object). This allows to remove desperate attempts to call reply->abort() wherever it's no more needed (and not aborting the in-flight replies seems to be a/the culprit of Quaternion after-exit hangs).
---
jobs/basejob.cpp | 128 +++++++++++++++++++++++++--------------------
jobs/basejob.h | 61 +++++++++++++++++----
jobs/mediathumbnailjob.cpp | 12 +----
jobs/mediathumbnailjob.h | 3 +-
4 files changed, 125 insertions(+), 79 deletions(-)
diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp
index 2c95ef11..9ab43087 100644
--- a/jobs/basejob.cpp
+++ b/jobs/basejob.cpp
@@ -19,14 +19,25 @@
#include "basejob.h"
#include <QtNetwork/QNetworkAccessManager>
-#include <QtNetwork/QNetworkReply>
#include <QtNetwork/QNetworkRequest>
+#include <QtNetwork/QNetworkReply>
+#include <QtNetwork/QSslError>
#include <QtCore/QTimer>
#include "../connectiondata.h"
using namespace QMatrixClient;
+struct NetworkReplyDeleter : public QScopedPointerDeleteLater
+{
+ static inline void cleanup(QNetworkReply* reply)
+ {
+ if (reply && reply->isRunning())
+ reply->abort();
+ QScopedPointerDeleteLater::cleanup(reply);
+ }
+};
+
class BaseJob::Private
{
public:
@@ -35,7 +46,7 @@ class BaseJob::Private
{}
ConnectionData* connection;
- QNetworkReply* reply;
+ QScopedPointer<QNetworkReply, NetworkReplyDeleter> reply;
JobHttpType type;
bool needsToken;
@@ -52,13 +63,6 @@ BaseJob::BaseJob(ConnectionData* connection, JobHttpType type, QString name, boo
BaseJob::~BaseJob()
{
- if( d->reply )
- {
- if( d->reply->isRunning() )
- d->reply->abort();
- d->reply->deleteLater();
- }
- delete d;
qDebug() << "Job" << objectName() << " destroyed";
}
@@ -77,11 +81,6 @@ QUrlQuery BaseJob::query() const
return QUrlQuery();
}
-void BaseJob::parseJson(const QJsonDocument& data)
-{
- emitResult();
-}
-
void BaseJob::start()
{
QUrl url = d->connection->baseUrl();
@@ -100,26 +99,75 @@ void BaseJob::start()
switch( d->type )
{
case JobHttpType::GetJob:
- d->reply = d->connection->nam()->get(req);
+ d->reply.reset( d->connection->nam()->get(req) );
break;
case JobHttpType::PostJob:
- d->reply = d->connection->nam()->post(req, data.toJson());
+ d->reply.reset( d->connection->nam()->post(req, data.toJson()) );
break;
case JobHttpType::PutJob:
- d->reply = d->connection->nam()->put(req, data.toJson());
+ d->reply.reset( d->connection->nam()->put(req, data.toJson()) );
break;
}
- connect( d->reply, &QNetworkReply::sslErrors, this, &BaseJob::sslErrors );
- connect( d->reply, &QNetworkReply::finished, this, &BaseJob::gotReply );
+ connect( d->reply.data(), &QNetworkReply::sslErrors, this, &BaseJob::sslErrors );
+ connect( d->reply.data(), &QNetworkReply::finished, this, &BaseJob::gotReply );
QTimer::singleShot( 120*1000, this, SLOT(timeout()) );
// 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()
+{
+ if (checkReply(d->reply.data()))
+ parseReply(d->reply->readAll());
+ // FIXME: we should not hold parseReply()/parseJson() responsible for
+ // emitting the result; it should be done here instead.
+}
+
+bool BaseJob::checkReply(QNetworkReply* reply)
+{
+ switch( reply->error() )
+ {
+ case QNetworkReply::NoError:
+ return true;
+
+ case QNetworkReply::AuthenticationRequiredError:
+ case QNetworkReply::ContentAccessDenied:
+ case QNetworkReply::ContentOperationNotPermittedError:
+ qDebug() << "Content access error, Qt error code:" << reply->error();
+ fail( ContentAccessError, reply->errorString() );
+ return false;
+
+ default:
+ qDebug() << "NetworkError, Qt error code:" << reply->error();
+ fail( NetworkError, reply->errorString() );
+ return false;
+ }
+}
+
+void BaseJob::parseReply(QByteArray data)
+{
+ QJsonParseError error;
+ QJsonDocument json = QJsonDocument::fromJson(data, &error);
+ if( error.error == QJsonParseError::NoError )
+ parseJson(json);
+ else
+ fail( JsonParseError, error.errorString() );
+}
+
+void BaseJob::parseJson(const QJsonDocument&)
+{
+ // Do nothing by default
+ emitResult();
+}
+
void BaseJob::finishJob(bool emitResult)
{
- if( d->reply->isRunning() )
- d->reply->abort();
+ if (!d->reply)
+ {
+ qWarning() << objectName()
+ << ": empty network reply (finish() called more than once?)";
+ return;
+ }
// Notify those that are interested in any completion of the job (including killing)
emit finished(this);
@@ -132,6 +180,7 @@ void BaseJob::finishJob(bool emitResult)
emit success(this);
}
+ d->reply.reset();
deleteLater();
}
@@ -169,47 +218,10 @@ void BaseJob::fail(int errorCode, QString errorString)
{
setError( errorCode );
setErrorText( errorString );
- if( d->reply && d->reply->isRunning() )
- d->reply->abort();
qWarning() << "Job" << objectName() << "failed:" << errorString;
emitResult();
}
-QNetworkReply* BaseJob::networkReply() const
-{
- return d->reply;
-}
-
-void BaseJob::gotReply()
-{
- switch( d->reply->error() )
- {
- case QNetworkReply::NoError:
- break; // All good, go to the normal flow after the switch()
-
- case QNetworkReply::AuthenticationRequiredError:
- case QNetworkReply::ContentAccessDenied:
- case QNetworkReply::ContentOperationNotPermittedError:
- qDebug() << "Content access error, Qt error code:" << d->reply->error();
- fail( ContentAccessError, d->reply->errorString() );
- return;
-
- default:
- qDebug() << "NetworkError, Qt error code:" << d->reply->error();
- fail( NetworkError, d->reply->errorString() );
- return;
- }
-
- QJsonParseError error;
- QJsonDocument data = QJsonDocument::fromJson(d->reply->readAll(), &error);
- if( error.error != QJsonParseError::NoError )
- {
- fail( JsonParseError, error.errorString() );
- return;
- }
- parseJson(data);
-}
-
void BaseJob::timeout()
{
fail( TimeoutError, "The job has timed out" );
diff --git a/jobs/basejob.h b/jobs/basejob.h
index 9d00c0ac..8e17e18e 100644
--- a/jobs/basejob.h
+++ b/jobs/basejob.h
@@ -19,10 +19,14 @@
#ifndef QMATRIXCLIENT_BASEJOB_H
#define QMATRIXCLIENT_BASEJOB_H
+#include <QtCore/QObject>
#include <QtCore/QJsonDocument>
#include <QtCore/QJsonObject>
#include <QtCore/QUrlQuery>
-#include <QtNetwork/QNetworkReply>
+#include <QtCore/QScopedPointer>
+
+class QNetworkReply;
+class QSslError;
namespace QMatrixClient
{
@@ -109,7 +113,35 @@ namespace QMatrixClient
virtual QString apiPath() const = 0;
virtual QUrlQuery query() const;
virtual QJsonObject data() const;
- virtual void parseJson(const QJsonDocument& data);
+
+ /**
+ * Checks the received reply for sanity; calls setError/setErrorText
+ * respectively. setError() with any argument except NoError prevents
+ * further parseReply()/parseJson() invocations.
+ *
+ * @param reply the reply received from the server
+ */
+ virtual bool checkReply(QNetworkReply* reply);
+
+ /**
+ * Processes the reply. By default, parses the reply into
+ * a QJsonDocument and calls parseJson() if it's a valid JSON.
+ * Overrides MUST ensure that fail() or emitResult() is called
+ * on every execution path exactly once.
+ *
+ * @param data raw contents of a HTTP reply from the server (without headers)
+ */
+ virtual void parseReply(QByteArray data);
+
+ /**
+ * Processes the JSON document received from the Matrix server.
+ * By default emits a successful result without analysing the JSON.
+ * Overrides MUST ensure that fail() or emitResult() is called
+ * on every execution path exactly once.
+ *
+ * @param json valid JSON document received from the server
+ */
+ virtual void parseJson(const QJsonDocument&);
/**
* Sets the error code.
@@ -126,12 +158,16 @@ namespace QMatrixClient
* @see emitResult(), fail()
*/
void setError(int errorCode);
+ /**
+ * Sets the error text. Usually is combined with a setError() call
+ * before it, as setErrorText() alone does not indicate the error status.
+ */
void setErrorText(QString errorText);
/**
- * Utility function to emit the result signal, and suicide this job.
- * It first notifies the observers to hide the progress for this job using
- * the finished() signal.
+ * Emits the result signal, and suicides this job.
+ * It first notifies the observers to hide the progress for this job
+ * using the finished() signal.
*
* @note: Deletes this job using deleteLater().
*
@@ -139,20 +175,27 @@ namespace QMatrixClient
* @see finished()
*/
void emitResult();
- void fail( int errorCode, QString errorString );
- QNetworkReply* networkReply() const;
+ /**
+ * Same as emitResult() but calls setError() and setErrorText()
+ * with respective arguments passed to it. Use it as a shortcut to
+ * finish the job with a failure status.
+ */
+ void fail( int errorCode, QString errorString );
+
protected slots:
- virtual void gotReply();
void timeout();
void sslErrors(const QList<QSslError>& errors);
+ private slots:
+ void gotReply();
+
private:
void finishJob(bool emitResult);
class Private;
- Private* d;
+ QScopedPointer<Private> d;
};
}
diff --git a/jobs/mediathumbnailjob.cpp b/jobs/mediathumbnailjob.cpp
index 48ed0ffb..3f5f9ebf 100644
--- a/jobs/mediathumbnailjob.cpp
+++ b/jobs/mediathumbnailjob.cpp
@@ -70,17 +70,9 @@ QUrlQuery MediaThumbnailJob::query() const
return query;
}
-void MediaThumbnailJob::gotReply()
+void MediaThumbnailJob::parseReply(QByteArray data)
{
- if( networkReply()->error() != QNetworkReply::NoError )
- {
- qDebug() << "NetworkError!!!";
- qDebug() << networkReply()->errorString();
- fail( NetworkError, networkReply()->errorString() );
- return;
- }
-
- if( !d->thumbnail.loadFromData( networkReply()->readAll() ) )
+ if( !d->thumbnail.loadFromData(data) )
{
qDebug() << "MediaThumbnailJob: could not read image data";
}
diff --git a/jobs/mediathumbnailjob.h b/jobs/mediathumbnailjob.h
index 541163ed..1af8ecd4 100644
--- a/jobs/mediathumbnailjob.h
+++ b/jobs/mediathumbnailjob.h
@@ -40,8 +40,7 @@ namespace QMatrixClient
QString apiPath() const override;
QUrlQuery query() const override;
- protected slots:
- void gotReply() override;
+ virtual void parseReply(QByteArray data) override;
private:
class Private;
--
cgit v1.2.3
From d8c3e31d34b8129dc24f52a7d726ff1206b90d48 Mon Sep 17 00:00:00 2001
From: David A Roberts <d@vidr.cc>
Date: Tue, 26 Jul 2016 19:37:34 +1000
Subject: Update libqmatrixclient.pri
---
libqmatrixclient.pri | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/libqmatrixclient.pri b/libqmatrixclient.pri
index 1290aa19..2648ac6b 100644
--- a/libqmatrixclient.pri
+++ b/libqmatrixclient.pri
@@ -32,10 +32,7 @@ HEADERS += \
$$PWD/jobs/roommessagesjob.h \
$$PWD/jobs/syncjob.h \
$$PWD/jobs/mediathumbnailjob.h \
- $$PWD/kcoreaddons/src/lib/jobs/kjob.h \
- $$PWD/kcoreaddons/src/lib/jobs/kcompositejob.h \
- $$PWD/kcoreaddons/src/lib/jobs/kjobtrackerinterface.h \
- $$PWD/kcoreaddons/src/lib/jobs/kjobuidelegate.h
+ $$PWD/jobs/logoutjob.h
SOURCES += \
$$PWD/connectiondata.cpp \
@@ -66,7 +63,4 @@ SOURCES += \
$$PWD/jobs/roommessagesjob.cpp \
$$PWD/jobs/syncjob.cpp \
$$PWD/jobs/mediathumbnailjob.cpp \
- $$PWD/kcoreaddons/src/lib/jobs/kjob.cpp \
- $$PWD/kcoreaddons/src/lib/jobs/kcompositejob.cpp \
- $$PWD/kcoreaddons/src/lib/jobs/kjobtrackerinterface.cpp \
- $$PWD/kcoreaddons/src/lib/jobs/kjobuidelegate.cpp
+ $$PWD/jobs/logoutjob.cpp
--
cgit v1.2.3
From 844d3362022d0278b07e1232460c1662e4fb61d2 Mon Sep 17 00:00:00 2001
From: Kitsune Ral <Kitsune-Ral@users.sf.net>
Date: Wed, 27 Jul 2016 14:54:43 +0900
Subject: Introduce Status class + BaseJob::{checkReply,parseReply,parseJson}
now return it
This better fixes the contract for derived job classes and simplifies error reporting. Methods error() and errorString() are kept for back-compatibility; status() returns a combination of them, conveniently packed into a Status object. For a quick status check, Status::good() is provided.
---
jobs/basejob.cpp | 76 ++++++++++++++++------------------
jobs/basejob.h | 101 +++++++++++++++++++++------------------------
jobs/checkauthmethods.cpp | 2 +-
jobs/checkauthmethods.h | 2 +-
jobs/joinroomjob.cpp | 15 +++----
jobs/joinroomjob.h | 2 +-
jobs/mediathumbnailjob.cpp | 4 +-
jobs/mediathumbnailjob.h | 2 +-
jobs/passwordlogin.cpp | 6 +--
jobs/passwordlogin.h | 2 +-
jobs/postmessagejob.cpp | 15 +++----
jobs/postmessagejob.h | 2 +-
jobs/roommembersjob.cpp | 7 ++--
jobs/roommembersjob.h | 4 +-
jobs/roommessagesjob.cpp | 4 +-
jobs/roommessagesjob.h | 2 +-
jobs/syncjob.cpp | 5 +--
jobs/syncjob.h | 2 +-
18 files changed, 118 insertions(+), 135 deletions(-)
diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp
index 9ab43087..3a0e1d3f 100644
--- a/jobs/basejob.cpp
+++ b/jobs/basejob.cpp
@@ -42,16 +42,16 @@ class BaseJob::Private
{
public:
Private(ConnectionData* c, JobHttpType t, bool nt)
- : connection(c), reply(nullptr), type(t), needsToken(nt), errorCode(NoError)
+ : connection(c), type(t), needsToken(nt)
+ , reply(nullptr), status(NoError)
{}
ConnectionData* connection;
- QScopedPointer<QNetworkReply, NetworkReplyDeleter> reply;
JobHttpType type;
bool needsToken;
- int errorCode;
- QString errorText;
+ QScopedPointer<QNetworkReply, NetworkReplyDeleter> reply;
+ Status status;
};
BaseJob::BaseJob(ConnectionData* connection, JobHttpType type, QString name, bool needsToken)
@@ -117,47 +117,45 @@ void BaseJob::start()
void BaseJob::gotReply()
{
- if (checkReply(d->reply.data()))
- parseReply(d->reply->readAll());
- // FIXME: we should not hold parseReply()/parseJson() responsible for
- // emitting the result; it should be done here instead.
+ setStatus(checkReply(d->reply.data()));
+ if (status().good())
+ setStatus(parseReply(d->reply->readAll()));
+
+ finishJob(true);
}
-bool BaseJob::checkReply(QNetworkReply* reply)
+BaseJob::Status BaseJob::checkReply(QNetworkReply* reply) const
{
switch( reply->error() )
{
case QNetworkReply::NoError:
- return true;
+ return NoError;
case QNetworkReply::AuthenticationRequiredError:
case QNetworkReply::ContentAccessDenied:
case QNetworkReply::ContentOperationNotPermittedError:
qDebug() << "Content access error, Qt error code:" << reply->error();
- fail( ContentAccessError, reply->errorString() );
- return false;
+ return { ContentAccessError, reply->errorString() };
default:
qDebug() << "NetworkError, Qt error code:" << reply->error();
- fail( NetworkError, reply->errorString() );
- return false;
+ return { NetworkError, reply->errorString() };
}
}
-void BaseJob::parseReply(QByteArray data)
+BaseJob::Status BaseJob::parseReply(QByteArray data)
{
QJsonParseError error;
QJsonDocument json = QJsonDocument::fromJson(data, &error);
if( error.error == QJsonParseError::NoError )
- parseJson(json);
+ return parseJson(json);
else
- fail( JsonParseError, error.errorString() );
+ return { JsonParseError, error.errorString() };
}
-void BaseJob::parseJson(const QJsonDocument&)
+BaseJob::Status BaseJob::parseJson(const QJsonDocument&)
{
- // Do nothing by default
- emitResult();
+ return Success;
}
void BaseJob::finishJob(bool emitResult)
@@ -165,7 +163,7 @@ void BaseJob::finishJob(bool emitResult)
if (!d->reply)
{
qWarning() << objectName()
- << ": empty network reply (finish() called more than once?)";
+ << ": empty network reply (finishJob() called more than once?)";
return;
}
@@ -184,29 +182,34 @@ void BaseJob::finishJob(bool emitResult)
deleteLater();
}
-int BaseJob::error() const
+BaseJob::Status BaseJob::status() const
{
- return d->errorCode;
+ return d->status;
}
-QString BaseJob::errorString() const
+int BaseJob::error() const
{
- return d->errorText;
+ return d->status.code;
}
-void BaseJob::setError(int errorCode)
+QString BaseJob::errorString() const
{
- d->errorCode = errorCode;
+ return d->status.message;
}
-void BaseJob::setErrorText(QString errorText)
+void BaseJob::setStatus(Status s)
{
- d->errorText = errorText;
+ d->status = s;
+ if (!s.good())
+ {
+ qWarning() << QString("Job %1 status: %2, code %3")
+ .arg(objectName()).arg(s.message).arg(s.code);
+ }
}
-void BaseJob::emitResult()
+void BaseJob::setStatus(int code, QString message)
{
- finishJob(true);
+ setStatus({ code, message });
}
void BaseJob::abandon()
@@ -214,17 +217,10 @@ void BaseJob::abandon()
finishJob(false);
}
-void BaseJob::fail(int errorCode, QString errorString)
-{
- setError( errorCode );
- setErrorText( errorString );
- qWarning() << "Job" << objectName() << "failed:" << errorString;
- emitResult();
-}
-
void BaseJob::timeout()
{
- fail( TimeoutError, "The job has timed out" );
+ setStatus( TimeoutError, "The job has timed out" );
+ finishJob(true);
}
void BaseJob::sslErrors(const QList<QSslError>& errors)
diff --git a/jobs/basejob.h b/jobs/basejob.h
index 8e17e18e..07b1f1dd 100644
--- a/jobs/basejob.h
+++ b/jobs/basejob.h
@@ -40,10 +40,38 @@ namespace QMatrixClient
public:
/* Just in case, the values are compatible with KJob
* (which BaseJob used to inherit from). */
- enum ErrorCode { NoError = 0, NetworkError = 100,
- JsonParseError, TimeoutError, ContentAccessError,
- UserDefinedError = 512 };
+ enum StatusCode { NoError = 0 // To be compatible with Qt conventions
+ , Success = 0
+ , ErrorLevel = 100 // Errors have codes starting from this
+ , NetworkError = 100
+ , JsonParseError
+ , TimeoutError
+ , ContentAccessError
+ , UserDefinedError = 200
+ };
+ /**
+ * 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,
+ * and a freeform message.
+ *
+ * To extend the list of error codes, define an (anonymous) enum
+ * along the lines of StatusCode, with additional values
+ * starting at UserDefinedError
+ */
+ class Status
+ {
+ public:
+ Status(StatusCode c) : code(c) { }
+ Status(int c, QString m) : code(c), message(m) { }
+
+ bool good() const { return code < ErrorLevel; }
+
+ int code;
+ QString message;
+ };
+
+ public:
BaseJob(ConnectionData* connection, JobHttpType type,
QString name, bool needsToken=true);
virtual ~BaseJob();
@@ -59,6 +87,7 @@ namespace QMatrixClient
*/
void abandon();
+ Status status() const;
int error() const;
virtual QString errorString() const;
@@ -115,74 +144,40 @@ namespace QMatrixClient
virtual QJsonObject data() const;
/**
- * Checks the received reply for sanity; calls setError/setErrorText
- * respectively. setError() with any argument except NoError prevents
- * further parseReply()/parseJson() invocations.
+ * Used by gotReply() slot to check the received reply for general
+ * issues such as network errors or access denial.
+ * Returning anything except NoError/Success prevents
+ * further parseReply()/parseJson() invocation.
*
* @param reply the reply received from the server
+ * @return the result of checking the reply
+ *
+ * @see gotReply
*/
- virtual bool checkReply(QNetworkReply* reply);
+ virtual Status checkReply(QNetworkReply* reply) const;
/**
* Processes the reply. By default, parses the reply into
* a QJsonDocument and calls parseJson() if it's a valid JSON.
- * Overrides MUST ensure that fail() or emitResult() is called
- * on every execution path exactly once.
*
* @param data raw contents of a HTTP reply from the server (without headers)
+ *
+ * @see gotReply, parseJson
*/
- virtual void parseReply(QByteArray data);
+ virtual Status parseReply(QByteArray data);
/**
* Processes the JSON document received from the Matrix server.
- * By default emits a successful result without analysing the JSON.
- * Overrides MUST ensure that fail() or emitResult() is called
- * on every execution path exactly once.
+ * By default returns succesful status without analysing the JSON.
*
* @param json valid JSON document received from the server
- */
- virtual void parseJson(const QJsonDocument&);
-
- /**
- * Sets the error code.
*
- * It should be called when an error is encountered in the job,
- * just before calling emitResult(). Normally you might want to
- * use fail() instead - it sets error code, error text, makes sure
- * the job has finished and invokes emitResult after that.
- *
- * To extend the list of error codes, define an (anonymous) enum
- * with additional values starting at BaseJob::UserDefinedError
- *
- * @param errorCode the error code
- * @see emitResult(), fail()
- */
- void setError(int errorCode);
- /**
- * Sets the error text. Usually is combined with a setError() call
- * before it, as setErrorText() alone does not indicate the error status.
- */
- void setErrorText(QString errorText);
-
- /**
- * Emits the result signal, and suicides this job.
- * It first notifies the observers to hide the progress for this job
- * using the finished() signal.
- *
- * @note: Deletes this job using deleteLater().
- *
- * @see result()
- * @see finished()
- */
- void emitResult();
-
- /**
- * Same as emitResult() but calls setError() and setErrorText()
- * with respective arguments passed to it. Use it as a shortcut to
- * finish the job with a failure status.
+ * @see parseReply
*/
- void fail( int errorCode, QString errorString );
+ virtual Status parseJson(const QJsonDocument&);
+ void setStatus(Status s);
+ void setStatus(int code, QString message);
protected slots:
void timeout();
diff --git a/jobs/checkauthmethods.cpp b/jobs/checkauthmethods.cpp
index 55d8632a..f471c135 100644
--- a/jobs/checkauthmethods.cpp
+++ b/jobs/checkauthmethods.cpp
@@ -57,7 +57,7 @@ QString CheckAuthMethods::apiPath() const
return "_matrix/client/r0/login";
}
-void CheckAuthMethods::parseJson(const QJsonDocument& data)
+BaseJob::Status CheckAuthMethods::parseJson(const QJsonDocument& data)
{
// TODO
}
diff --git a/jobs/checkauthmethods.h b/jobs/checkauthmethods.h
index 2951e27f..36eaa01a 100644
--- a/jobs/checkauthmethods.h
+++ b/jobs/checkauthmethods.h
@@ -35,7 +35,7 @@ namespace QMatrixClient
protected:
QString apiPath() const override;
- void parseJson(const QJsonDocument& data) override;
+ Status parseJson(const QJsonDocument& data) override;
private:
class Private;
diff --git a/jobs/joinroomjob.cpp b/jobs/joinroomjob.cpp
index 799d4926..8848aa70 100644
--- a/jobs/joinroomjob.cpp
+++ b/jobs/joinroomjob.cpp
@@ -54,18 +54,15 @@ QString JoinRoomJob::apiPath() const
return QString("_matrix/client/r0/join/%1").arg(d->roomAlias);
}
-void JoinRoomJob::parseJson(const QJsonDocument& data)
+BaseJob::Status JoinRoomJob::parseJson(const QJsonDocument& data)
{
QJsonObject json = data.object();
- if( !json.contains("room_id") )
- {
- fail( BaseJob::UserDefinedError, "Something went wrong..." );
- qDebug() << data;
- return;
- }
- else
+ if( json.contains("room_id") )
{
d->roomId = json.value("room_id").toString();
+ return Success;
}
- emitResult();
+
+ qDebug() << data;
+ return { UserDefinedError, "No room_id in the JSON response" };
}
diff --git a/jobs/joinroomjob.h b/jobs/joinroomjob.h
index 1329ca56..a6f4af21 100644
--- a/jobs/joinroomjob.h
+++ b/jobs/joinroomjob.h
@@ -35,7 +35,7 @@ namespace QMatrixClient
protected:
QString apiPath() const override;
- void parseJson(const QJsonDocument& data) override;
+ Status parseJson(const QJsonDocument& data) override;
private:
class Private;
diff --git a/jobs/mediathumbnailjob.cpp b/jobs/mediathumbnailjob.cpp
index 3f5f9ebf..1e434fbc 100644
--- a/jobs/mediathumbnailjob.cpp
+++ b/jobs/mediathumbnailjob.cpp
@@ -70,11 +70,11 @@ QUrlQuery MediaThumbnailJob::query() const
return query;
}
-void MediaThumbnailJob::parseReply(QByteArray data)
+BaseJob::Status MediaThumbnailJob::parseReply(QByteArray data)
{
if( !d->thumbnail.loadFromData(data) )
{
qDebug() << "MediaThumbnailJob: could not read image data";
}
- emitResult();
+ return Success;
}
diff --git a/jobs/mediathumbnailjob.h b/jobs/mediathumbnailjob.h
index 1af8ecd4..3babf845 100644
--- a/jobs/mediathumbnailjob.h
+++ b/jobs/mediathumbnailjob.h
@@ -40,7 +40,7 @@ namespace QMatrixClient
QString apiPath() const override;
QUrlQuery query() const override;
- virtual void parseReply(QByteArray data) override;
+ Status parseReply(QByteArray data) override;
private:
class Private;
diff --git a/jobs/passwordlogin.cpp b/jobs/passwordlogin.cpp
index 231dcce5..c85e4c13 100644
--- a/jobs/passwordlogin.cpp
+++ b/jobs/passwordlogin.cpp
@@ -80,15 +80,15 @@ QJsonObject PasswordLogin::data() const
return json;
}
-void PasswordLogin::parseJson(const QJsonDocument& data)
+BaseJob::Status PasswordLogin::parseJson(const QJsonDocument& data)
{
QJsonObject json = data.object();
if( !json.contains("access_token") || !json.contains("home_server") || !json.contains("user_id") )
{
- fail( BaseJob::UserDefinedError, "Unexpected data" );
+ return { UserDefinedError, "No expected data" };
}
d->returned_token = json.value("access_token").toString();
d->returned_server = json.value("home_server").toString();
d->returned_id = json.value("user_id").toString();
- emitResult();
+ return Success;
}
diff --git a/jobs/passwordlogin.h b/jobs/passwordlogin.h
index 75a45cae..d7e42725 100644
--- a/jobs/passwordlogin.h
+++ b/jobs/passwordlogin.h
@@ -38,7 +38,7 @@ namespace QMatrixClient
protected:
QString apiPath() const override;
QJsonObject data() const override;
- void parseJson(const QJsonDocument& data) override;
+ Status parseJson(const QJsonDocument& data) override;
private:
class Private;
diff --git a/jobs/postmessagejob.cpp b/jobs/postmessagejob.cpp
index cf9b94fd..0a38da62 100644
--- a/jobs/postmessagejob.cpp
+++ b/jobs/postmessagejob.cpp
@@ -61,14 +61,11 @@ QJsonObject PostMessageJob::data() const
return json;
}
-void PostMessageJob::parseJson(const QJsonDocument& data)
+BaseJob::Status PostMessageJob::parseJson(const QJsonDocument& data)
{
- QJsonObject json = data.object();
- if( !json.contains("event_id") )
- {
- fail( BaseJob::UserDefinedError, "Something went wrong..." );
- qDebug() << data;
- return;
- }
- emitResult();
+ if( data.object().contains("event_id") )
+ return Success;
+
+ qDebug() << data;
+ return { UserDefinedError, "No event_id in the JSON response" };
}
diff --git a/jobs/postmessagejob.h b/jobs/postmessagejob.h
index 9d354240..73d72020 100644
--- a/jobs/postmessagejob.h
+++ b/jobs/postmessagejob.h
@@ -35,7 +35,7 @@ namespace QMatrixClient
protected:
QString apiPath() const override;
QJsonObject data() const override;
- void parseJson(const QJsonDocument& data) override;
+ Status parseJson(const QJsonDocument& data) override;
private:
class Private;
diff --git a/jobs/roommembersjob.cpp b/jobs/roommembersjob.cpp
index 002be75b..7fc44c63 100644
--- a/jobs/roommembersjob.cpp
+++ b/jobs/roommembersjob.cpp
@@ -56,10 +56,9 @@ QString RoomMembersJob::apiPath() const
return QString("_matrix/client/r0/rooms/%1/members").arg(d->room->id());
}
-void RoomMembersJob::parseJson(const QJsonDocument& data)
+BaseJob::Status RoomMembersJob::parseJson(const QJsonDocument& data)
{
- QJsonObject obj = data.object();
- QJsonArray chunk = obj.value("chunk").toArray();
+ QJsonArray chunk = data.object().value("chunk").toArray();
for( const QJsonValue& val : chunk )
{
State* state = State::fromJson(val.toObject());
@@ -67,5 +66,5 @@ void RoomMembersJob::parseJson(const QJsonDocument& data)
d->states.append(state);
}
qDebug() << "States: " << d->states.count();
- emitResult();
+ return Success;
}
diff --git a/jobs/roommembersjob.h b/jobs/roommembersjob.h
index ffae4309..04803d67 100644
--- a/jobs/roommembersjob.h
+++ b/jobs/roommembersjob.h
@@ -35,8 +35,8 @@ namespace QMatrixClient
QList<State*> states();
protected:
- virtual QString apiPath() const override;
- virtual void parseJson(const QJsonDocument& data) override;
+ QString apiPath() const override;
+ Status parseJson(const QJsonDocument& data) override;
private:
class Private;
diff --git a/jobs/roommessagesjob.cpp b/jobs/roommessagesjob.cpp
index f1943d2c..ba075007 100644
--- a/jobs/roommessagesjob.cpp
+++ b/jobs/roommessagesjob.cpp
@@ -81,10 +81,10 @@ QUrlQuery RoomMessagesJob::query() const
return query;
}
-void RoomMessagesJob::parseJson(const QJsonDocument& data)
+BaseJob::Status RoomMessagesJob::parseJson(const QJsonDocument& data)
{
QJsonObject obj = data.object();
d->events = eventListFromJson(obj.value("chunk").toArray());
d->end = obj.value("end").toString();
- emitResult();
+ return Success;
}
diff --git a/jobs/roommessagesjob.h b/jobs/roommessagesjob.h
index c068f235..52a72f70 100644
--- a/jobs/roommessagesjob.h
+++ b/jobs/roommessagesjob.h
@@ -40,7 +40,7 @@ namespace QMatrixClient
protected:
QString apiPath() const override;
QUrlQuery query() const override;
- void parseJson(const QJsonDocument& data) override;
+ Status parseJson(const QJsonDocument& data) override;
private:
class Private;
diff --git a/jobs/syncjob.cpp b/jobs/syncjob.cpp
index 3a7573bc..3bd1f0c8 100644
--- a/jobs/syncjob.cpp
+++ b/jobs/syncjob.cpp
@@ -110,7 +110,7 @@ QUrlQuery SyncJob::query() const
return query;
}
-void SyncJob::parseJson(const QJsonDocument& data)
+BaseJob::Status SyncJob::parseJson(const QJsonDocument& data)
{
QJsonObject json = data.object();
d->nextBatch = json.value("next_batch").toString();
@@ -134,8 +134,7 @@ void SyncJob::parseJson(const QJsonDocument& data)
}
}
- emitResult();
- qDebug() << objectName() << ": processing complete";
+ return Success;
}
void SyncRoomData::EventList::fromJson(const QJsonObject& roomContents)
diff --git a/jobs/syncjob.h b/jobs/syncjob.h
index 95d7aacb..51d07944 100644
--- a/jobs/syncjob.h
+++ b/jobs/syncjob.h
@@ -73,7 +73,7 @@ namespace QMatrixClient
protected:
QString apiPath() const override;
QUrlQuery query() const override;
- void parseJson(const QJsonDocument& data) override;
+ Status parseJson(const QJsonDocument& data) override;
private:
class Private;
--
cgit v1.2.3
From c7f0142a3b1b92047612a5b8ada11ef231238ff6 Mon Sep 17 00:00:00 2001
From: Kitsune Ral <Kitsune-Ral@users.sf.net>
Date: Wed, 27 Jul 2016 19:22:50 +0900
Subject: Removing kcoreaddons submodule
---
.gitmodules | 3 ---
kcoreaddons | 1 -
2 files changed, 4 deletions(-)
delete mode 160000 kcoreaddons
diff --git a/.gitmodules b/.gitmodules
index 3a037c31..e69de29b 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -1,3 +0,0 @@
-[submodule "kcoreaddons"]
- path = kcoreaddons
- url = git://anongit.kde.org/kcoreaddons.git
diff --git a/kcoreaddons b/kcoreaddons
deleted file mode 160000
index 7ac7a605..00000000
--- a/kcoreaddons
+++ /dev/null
@@ -1 +0,0 @@
-Subproject commit 7ac7a605923f0bb2f0f367f6069a101a24bead9f
--
cgit v1.2.3
From f6c623a27bcb5ec2fcc83930e500afb597a32a46 Mon Sep 17 00:00:00 2001
From: Kitsune Ral <Kitsune-Ral@users.sf.net>
Date: Thu, 28 Jul 2016 18:54:45 +0900
Subject: Fixed double-emission of signal(s) on job timeout + general cleanup
---
connection.cpp | 1 -
connectionprivate.cpp | 1 -
jobs/basejob.cpp | 24 ++++++++++++++----------
3 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/connection.cpp b/connection.cpp
index 9664521c..cbc0f4a0 100644
--- a/connection.cpp
+++ b/connection.cpp
@@ -24,7 +24,6 @@
#include "room.h"
#include "jobs/passwordlogin.h"
#include "jobs/logoutjob.h"
-#include "jobs/geteventsjob.h"
#include "jobs/postmessagejob.h"
#include "jobs/postreceiptjob.h"
#include "jobs/joinroomjob.h"
diff --git a/connectionprivate.cpp b/connectionprivate.cpp
index 4d49c014..0f9f4539 100644
--- a/connectionprivate.cpp
+++ b/connectionprivate.cpp
@@ -23,7 +23,6 @@
#include "user.h"
#include "jobs/passwordlogin.h"
#include "jobs/syncjob.h"
-#include "jobs/geteventsjob.h"
#include "jobs/joinroomjob.h"
#include "jobs/roommembersjob.h"
#include "events/event.h"
diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp
index 3a0e1d3f..e0dff287 100644
--- a/jobs/basejob.cpp
+++ b/jobs/basejob.cpp
@@ -54,16 +54,21 @@ class BaseJob::Private
Status status;
};
+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))
{
setObjectName(name);
- qDebug() << "Job" << objectName() << " created";
+ qDebug() << this << "created";
}
BaseJob::~BaseJob()
{
- qDebug() << "Job" << objectName() << " destroyed";
+ qDebug() << this << "destroyed";
}
ConnectionData* BaseJob::connection() const
@@ -134,11 +139,9 @@ BaseJob::Status BaseJob::checkReply(QNetworkReply* reply) const
case QNetworkReply::AuthenticationRequiredError:
case QNetworkReply::ContentAccessDenied:
case QNetworkReply::ContentOperationNotPermittedError:
- qDebug() << "Content access error, Qt error code:" << reply->error();
return { ContentAccessError, reply->errorString() };
default:
- qDebug() << "NetworkError, Qt error code:" << reply->error();
return { NetworkError, reply->errorString() };
}
}
@@ -162,9 +165,12 @@ void BaseJob::finishJob(bool emitResult)
{
if (!d->reply)
{
- qWarning() << objectName()
- << ": empty network reply (finishJob() called more than once?)";
- return;
+ qWarning() << this << "finishes with empty network reply";
+ }
+ else if (d->reply->isRunning())
+ {
+ qWarning() << this << "finishes without ready network reply";
+ d->reply->disconnect(this); // Ignore whatever comes from the reply
}
// Notify those that are interested in any completion of the job (including killing)
@@ -178,7 +184,6 @@ void BaseJob::finishJob(bool emitResult)
emit success(this);
}
- d->reply.reset();
deleteLater();
}
@@ -202,8 +207,7 @@ void BaseJob::setStatus(Status s)
d->status = s;
if (!s.good())
{
- qWarning() << QString("Job %1 status: %2, code %3")
- .arg(objectName()).arg(s.message).arg(s.code);
+ qWarning() << this << "status" << s.code << ":" << s.message;
}
}
--
cgit v1.2.3