From 2eb18a735a5f75a77387a211f4311222d00c2d6c Mon Sep 17 00:00:00 2001 From: Kitsune Ral 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. --- jobs/basejob.h | 102 +++++++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 85 insertions(+), 17 deletions(-) (limited to 'jobs/basejob.h') 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 -#else -#include "kjob.h" -#endif // KCOREADDONS_FOUND - #include #include #include @@ -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& 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 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.h | 61 +++++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 52 insertions(+), 9 deletions(-) (limited to 'jobs/basejob.h') 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 #include #include #include -#include +#include + +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& errors); + private slots: + void gotReply(); + private: void finishJob(bool emitResult); class Private; - Private* d; + QScopedPointer d; }; } -- cgit v1.2.3 From 844d3362022d0278b07e1232460c1662e4fb61d2 Mon Sep 17 00:00:00 2001 From: Kitsune Ral 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.h | 101 +++++++++++++++++++++++++++------------------------------ 1 file changed, 48 insertions(+), 53 deletions(-) (limited to 'jobs/basejob.h') 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(); -- cgit v1.2.3