From f81aa4d723577ce30518424510e45ef39ff0e29e Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sat, 7 Aug 2021 18:59:33 +0200 Subject: Drop an out-of-date comment BaseJob::loadFromJson() does just fine without QStringViews. [skip ci] --- lib/jobs/basejob.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/jobs') diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h index d33d542e..7ce4b808 100644 --- a/lib/jobs/basejob.h +++ b/lib/jobs/basejob.h @@ -180,7 +180,7 @@ public: * If there's no top-level JSON object in the response or if there's * no node with the key \p keyName, \p defaultValue is returned. */ - template // Waiting for QStringViews... + template T loadFromJson(const StrT& keyName, T&& defaultValue = {}) const { const auto& jv = jsonData().value(keyName); -- cgit v1.2.3 From 40548a7995147f3f99212928ae27047de7a79618 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 22 Aug 2021 20:10:49 +0200 Subject: Deprecate BaseJob::Data The grand plan is to get rid of `BaseJob` and turn job invocations to function calls returning `QFuture`. `RequestData` will stay though, feeding data into those calls. --- lib/jobs/basejob.cpp | 17 ++++++++++------- lib/jobs/basejob.h | 10 ++++++---- 2 files changed, 16 insertions(+), 11 deletions(-) (limited to 'lib/jobs') diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index 400a9243..239cef28 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -71,8 +71,8 @@ public: // Using an idiom from clang-tidy: // http://clang.llvm.org/extra/clang-tidy/checks/modernize-pass-by-value.html - Private(HttpVerb v, QString endpoint, const QUrlQuery& q, Data&& data, - bool nt) + Private(HttpVerb v, QString endpoint, const QUrlQuery& q, + RequestData&& data, bool nt) : verb(v) , apiEndpoint(std::move(endpoint)) , requestQuery(q) @@ -109,7 +109,7 @@ public: QString apiEndpoint; QHash requestHeaders; QUrlQuery requestQuery; - Data requestData; + RequestData requestData; bool needsToken; bool inBackground = false; @@ -168,11 +168,11 @@ public: BaseJob::BaseJob(HttpVerb verb, const QString& name, const QString& endpoint, bool needsToken) - : BaseJob(verb, name, endpoint, QUrlQuery {}, Data {}, needsToken) + : BaseJob(verb, name, endpoint, QUrlQuery {}, RequestData {}, needsToken) {} BaseJob::BaseJob(HttpVerb verb, const QString& name, const QString& endpoint, - const QUrlQuery &query, Data&& data, bool needsToken) + const QUrlQuery& query, RequestData&& data, bool needsToken) : d(new Private(verb, endpoint, query, std::move(data), needsToken)) { setObjectName(name); @@ -224,9 +224,12 @@ void BaseJob::setRequestQuery(const QUrlQuery& query) d->requestQuery = query; } -const BaseJob::Data& BaseJob::requestData() const { return d->requestData; } +const RequestData& BaseJob::requestData() const { return d->requestData; } -void BaseJob::setRequestData(Data&& data) { std::swap(d->requestData, data); } +void BaseJob::setRequestData(RequestData&& data) +{ + std::swap(d->requestData, data); +} const QByteArrayList& BaseJob::expectedContentTypes() const { diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h index 7ce4b808..7750fb8b 100644 --- a/lib/jobs/basejob.h +++ b/lib/jobs/basejob.h @@ -72,7 +72,8 @@ public: }; Q_ENUM(StatusCode) - using Data = RequestData; + using Data Q_DECL_DEPRECATED_X("Use Quotient::RequestData instead") // + = RequestData; /*! * This structure stores the status of a server call job. The status @@ -125,7 +126,8 @@ public: BaseJob(HttpVerb verb, const QString& name, const QString& endpoint, bool needsToken = true); BaseJob(HttpVerb verb, const QString& name, const QString& endpoint, - const QUrlQuery& query, Data&& data = {}, bool needsToken = true); + const QUrlQuery& query, RequestData&& data = {}, + bool needsToken = true); QUrl requestUrl() const; bool isBackground() const; @@ -330,8 +332,8 @@ protected: void setRequestHeaders(const headers_t& headers); const QUrlQuery& query() const; void setRequestQuery(const QUrlQuery& query); - const Data& requestData() const; - void setRequestData(Data&& data); + const RequestData& requestData() const; + void setRequestData(RequestData&& data); const QByteArrayList& expectedContentTypes() const; void addExpectedContentType(const QByteArray& contentType); void setExpectedContentTypes(const QByteArrayList& contentTypes); -- cgit v1.2.3 From c50420a0f2df7a7bf291312c38ac43e2c9f58141 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 22 Aug 2021 20:15:04 +0200 Subject: Drop QMatrixClient namespace alias --- lib/avatar.h | 2 -- lib/jobs/requestdata.h | 2 -- lib/logging.h | 2 -- 3 files changed, 6 deletions(-) (limited to 'lib/jobs') diff --git a/lib/avatar.h b/lib/avatar.h index 37e1eeef..d4634aea 100644 --- a/lib/avatar.h +++ b/lib/avatar.h @@ -42,5 +42,3 @@ private: std::unique_ptr d; }; } // namespace Quotient -/// \deprecated Use namespace Quotient instead -namespace QMatrixClient = Quotient; diff --git a/lib/jobs/requestdata.h b/lib/jobs/requestdata.h index 21657631..4f05e5ff 100644 --- a/lib/jobs/requestdata.h +++ b/lib/jobs/requestdata.h @@ -35,5 +35,3 @@ private: std::unique_ptr _source; }; } // namespace Quotient -/// \deprecated Use namespace Quotient instead -namespace QMatrixClient = Quotient; diff --git a/lib/logging.h b/lib/logging.h index 264215e1..1d1394e8 100644 --- a/lib/logging.h +++ b/lib/logging.h @@ -67,8 +67,6 @@ inline qint64 profilerMinNsecs() * 1000; } } // namespace Quotient -/// \deprecated Use namespace Quotient instead -namespace QMatrixClient = Quotient; inline QDebug operator<<(QDebug debug_object, const QElapsedTimer& et) { -- cgit v1.2.3 From c26015503aa0fbca37abdfc4870ac94bb7befeee Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 22 Aug 2021 20:19:15 +0200 Subject: Drop other stuff deprecated pre- or early 0.6 BaseJob: StatusCode::JsonParseError Connection: resolved() and reconnected() signals; roomMap(); postReceipt() User: bridged() and rawName() ConnectionData: setHost() and setPort() StateEventBase: prev_content() --- lib/connection.cpp | 21 --------------------- lib/connection.h | 45 --------------------------------------------- lib/connectiondata.cpp | 12 ------------ lib/connectiondata.h | 4 ---- lib/events/stateevent.h | 4 ---- lib/jobs/basejob.h | 2 -- lib/user.cpp | 4 ---- lib/user.h | 18 +----------------- 8 files changed, 1 insertion(+), 109 deletions(-) (limited to 'lib/jobs') diff --git a/lib/connection.cpp b/lib/connection.cpp index 7dd04aaa..222c3b71 100644 --- a/lib/connection.cpp +++ b/lib/connection.cpp @@ -314,8 +314,6 @@ void Connection::resolveServer(const QString& mxid) setHomeserver(maybeBaseUrl); } Q_ASSERT(d->loginFlowsJob != nullptr); // Ensured by setHomeserver() - connect(d->loginFlowsJob, &BaseJob::success, this, - &Connection::resolved); connect(d->loginFlowsJob, &BaseJob::failure, this, [this] { qCWarning(MAIN) << "Homeserver base URL sanity check failed"; emit resolveError(tr("The homeserver doesn't seem to be working")); @@ -795,11 +793,6 @@ void Connection::stopSync() QString Connection::nextBatchToken() const { return d->data->lastEvent(); } -PostReceiptJob* Connection::postReceipt(Room* room, RoomEvent* event) -{ - return callApi(room->id(), "m.read", event->id()); -} - JoinRoomJob* Connection::joinRoom(const QString& roomAlias, const QStringList& serverNames) { @@ -1239,20 +1232,6 @@ int Connection::millisToReconnect() const return d->syncJob ? d->syncJob->millisToRetry() : 0; } -QHash, Room*> Connection::roomMap() const -{ - // Copy-on-write-and-remove-elements is faster than copying elements one by - // one. - QHash, Room*> roomMap = d->roomMap; - for (auto it = roomMap.begin(); it != roomMap.end();) { - if (it.value()->joinState() == JoinState::Leave) - it = roomMap.erase(it); - else - ++it; - } - return roomMap; -} - QVector Connection::allRooms() const { QVector result; diff --git a/lib/connection.h b/lib/connection.h index a7a071f3..ecbb1a19 100644 --- a/lib/connection.h +++ b/lib/connection.h @@ -138,15 +138,6 @@ public: explicit Connection(const QUrl& server, QObject* parent = nullptr); ~Connection() override; - /// Get all Invited and Joined rooms - /*! - * \return a hashmap from a composite key - room name and whether - * it's an Invite rather than Join - to room pointers - * \sa allRooms, rooms, roomsWithTag - */ - [[deprecated("Use allRooms(), roomsWithTag() or rooms(joinStates) instead")]] - QHash, Room*> roomMap() const; - /// Get all rooms known within this Connection /*! * This includes Invite, Join and Leave rooms, in no particular order. @@ -524,30 +515,12 @@ public Q_SLOTS: */ void assumeIdentity(const QString& mxId, const QString& accessToken, const QString& deviceId); - /*! \deprecated Use loginWithPassword instead */ - void connectToServer(const QString& userId, const QString& password, - const QString& initialDeviceName, - const QString& deviceId = {}) - { - loginWithPassword(userId, password, initialDeviceName, deviceId); - } - /*! \deprecated - * Use assumeIdentity() if you have an access token or - * loginWithToken() if you have a login token. - */ - void connectWithToken(const QString& userId, const QString& accessToken, - const QString& deviceId) - { - assumeIdentity(userId, accessToken, deviceId); - } /// Explicitly request capabilities from the server void reloadCapabilities(); /// Find out if capabilites are still loading from the server bool loadingCapabilities() const; - /** @deprecated Use stopSync() instead */ - void disconnectFromServer() { stopSync(); } void logout(); void sync(int timeout = -1); @@ -662,24 +635,7 @@ public Q_SLOTS: /** \deprecated Do not use this directly, use Room::leaveRoom() instead */ virtual LeaveRoomJob* leaveRoom(Room* room); - // Old API that will be abolished any time soon. DO NOT USE. - - /** @deprecated Use callApi() or Room::postReceipt() instead - */ - virtual PostReceiptJob* postReceipt(Room* room, RoomEvent* event); - Q_SIGNALS: - /** - * @deprecated - * This was a signal resulting from a successful resolveServer(). - * Since Connection now provides setHomeserver(), the HS URL - * may change even without resolveServer() invocation. Use - * loginFLowsChanged() instead of resolved(). You can also use - * loginWith*() and assumeIdentity() without the HS URL set in - * advance (i.e. without calling resolveServer), as they trigger - * server name resolution from MXID if the server URL is not valid. - */ - void resolved(); void resolveError(QString error); void homeserverChanged(QUrl baseUrl); @@ -687,7 +643,6 @@ Q_SIGNALS: void capabilitiesLoaded(); void connected(); - void reconnected(); //< \deprecated Use connected() instead void loggedOut(); /** Login data or state have changed * diff --git a/lib/connectiondata.cpp b/lib/connectiondata.cpp index e54d909b..87ad4577 100644 --- a/lib/connectiondata.cpp +++ b/lib/connectiondata.cpp @@ -118,18 +118,6 @@ void ConnectionData::setToken(QByteArray token) d->accessToken = std::move(token); } -void ConnectionData::setHost(QString host) -{ - d->baseUrl.setHost(host); - qCDebug(MAIN) << "updated baseUrl to" << d->baseUrl; -} - -void ConnectionData::setPort(int port) -{ - d->baseUrl.setPort(port); - qCDebug(MAIN) << "updated baseUrl to" << d->baseUrl; -} - const QString& ConnectionData::deviceId() const { return d->deviceId; } const QString& ConnectionData::userId() const { return d->userId; } diff --git a/lib/connectiondata.h b/lib/connectiondata.h index 7dd96f26..e16a2dac 100644 --- a/lib/connectiondata.h +++ b/lib/connectiondata.h @@ -31,10 +31,6 @@ public: void setBaseUrl(QUrl baseUrl); void setToken(QByteArray accessToken); - [[deprecated("Use setBaseUrl() instead")]] - void setHost(QString host); - [[deprecated("Use setBaseUrl() instead")]] - void setPort(int port); void setDeviceId(const QString& deviceId); void setUserId(const QString& userId); void setNeedsToken(const QString& requestName); diff --git a/lib/events/stateevent.h b/lib/events/stateevent.h index 1415f709..bc414a5f 100644 --- a/lib/events/stateevent.h +++ b/lib/events/stateevent.h @@ -100,10 +100,6 @@ public: visitor(_content); editJson()[ContentKeyL] = _content.toJson(); } - [[deprecated("Use prevContent instead")]] const ContentT* prev_content() const - { - return prevContent(); - } const ContentT* prevContent() const { return _prev ? &_prev->content : nullptr; diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h index 7750fb8b..835cd822 100644 --- a/lib/jobs/basejob.h +++ b/lib/jobs/basejob.h @@ -52,8 +52,6 @@ public: IncorrectRequestError = IncorrectRequest, IncorrectResponse, IncorrectResponseError = IncorrectResponse, - JsonParseError //< \deprecated Use IncorrectResponse instead - = IncorrectResponse, TooManyRequests, TooManyRequestsError = TooManyRequests, RateLimited = TooManyRequests, diff --git a/lib/user.cpp b/lib/user.cpp index a4abed37..60920a59 100644 --- a/lib/user.cpp +++ b/lib/user.cpp @@ -93,8 +93,6 @@ QString User::name(const Room* room) const return room ? room->memberName(id()) : d->defaultName; } -QString User::rawName(const Room* room) const { return name(room); } - void User::rename(const QString& newName) { const auto actualNewName = sanitized(newName); @@ -185,8 +183,6 @@ QString User::fullName(const Room* room) const return displayName.isEmpty() ? id() : (displayName % " (" % id() % ')'); } -QString User::bridged() const { return {}; } - const Avatar& User::avatarObject(const Room* room) const { if (!room) diff --git a/lib/user.h b/lib/user.h index 4ff62951..78b72bf2 100644 --- a/lib/user.h +++ b/lib/user.h @@ -22,7 +22,6 @@ class User : public QObject { Q_PROPERTY(QString name READ name NOTIFY defaultNameChanged) Q_PROPERTY(QString displayName READ displayname NOTIFY defaultNameChanged STORED false) Q_PROPERTY(QString fullName READ fullName NOTIFY defaultNameChanged STORED false) - Q_PROPERTY(QString bridgeName READ bridged NOTIFY defaultNameChanged STORED false) Q_PROPERTY(QString avatarMediaId READ avatarMediaId NOTIFY defaultAvatarChanged STORED false) Q_PROPERTY(QUrl avatarUrl READ avatarUrl NOTIFY defaultAvatarChanged) public: @@ -40,18 +39,10 @@ public: * This may be empty if the user didn't choose the name or cleared * it. If the user is bridged, the bridge postfix (such as '(IRC)') * is stripped out. No disambiguation for the room is done. - * \sa displayName, rawName + * \sa displayName */ QString name(const Room* room = nullptr) const; - /** Get the user name along with the bridge postfix - * This function is similar to name() but appends the bridge postfix - * (such as '(IRC)') to the user name. No disambiguation is done. - * \sa name, displayName - */ - [[deprecated("Bridge postfixes exist no more, use name() instead")]] - QString rawName(const Room* room = nullptr) const; - /** Get the displayed user name * When \p room is null, this method returns result of name() if * the name is non-empty; otherwise it returns user id. @@ -70,13 +61,6 @@ public: */ QString fullName(const Room* room = nullptr) const; - /** - * Returns the name of bridge the user is connected from or empty. - */ - [[deprecated("Bridged status is no more supported; this always returns" - " an empty string")]] - QString bridged() const; - /** Whether the user is a guest * As of now, the function relies on the convention used in Synapse * that guests and only guests have all-numeric IDs. This may or -- cgit v1.2.3 From 93cee89f9a99e51275ac9cd304180499b0543eea Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 22 Aug 2021 20:45:53 +0200 Subject: Fix building with MSVC --- lib/jobs/basejob.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'lib/jobs') diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h index 835cd822..29443c53 100644 --- a/lib/jobs/basejob.h +++ b/lib/jobs/basejob.h @@ -70,7 +70,10 @@ public: }; Q_ENUM(StatusCode) - using Data Q_DECL_DEPRECATED_X("Use Quotient::RequestData instead") // + using Data +#ifndef Q_CC_MSVC + Q_DECL_DEPRECATED_X("Use Quotient::RequestData instead") +#endif = RequestData; /*! -- cgit v1.2.3 From c813a084b6dc8206a9c7305bbb236e23edcb49e5 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Tue, 24 Aug 2021 18:38:45 +0200 Subject: Fix building with MSVC Turned out it was broken, and I was looking the other way. --- lib/jobs/basejob.cpp | 4 ++-- lib/quotient_common.h | 31 +++++++++++++------------------ 2 files changed, 15 insertions(+), 20 deletions(-) (limited to 'lib/jobs') diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index 239cef28..6346db9d 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -156,8 +156,8 @@ public: { // FIXME: use std::array {} when Apple stdlib gets deduction guides for it static const auto verbs = - to_array({ QStringLiteral("GET"), QStringLiteral("PUT"), - QStringLiteral("POST"), QStringLiteral("DELETE") }); + make_array(QStringLiteral("GET"), QStringLiteral("PUT"), + QStringLiteral("POST"), QStringLiteral("DELETE")); const auto verbWord = verbs.at(size_t(verb)); return verbWord % ' ' % (reply ? reply->url().toString(QUrl::RemoveQuery) diff --git a/lib/quotient_common.h b/lib/quotient_common.h index d225ad63..bb2e6a6b 100644 --- a/lib/quotient_common.h +++ b/lib/quotient_common.h @@ -10,22 +10,17 @@ namespace Quotient { Q_NAMESPACE -namespace impl { - template - constexpr std::array, N> - to_array_impl(T (&&a)[N], std::index_sequence) - { - return { {std::move(a[I])...} }; - } -} // std::array {} needs explicit template parameters on macOS because -// Apple stdlib doesn't have deduction guides for std::array; to alleviate that, -// to_array() is borrowed from C++20 (thanks to cppreference for the possible -// implementation: https://en.cppreference.com/w/cpp/container/array/to_array) -template -constexpr auto to_array(T (&& items)[N]) +// Apple stdlib doesn't have deduction guides for std::array. C++20 has +// to_array() but that can't be borrowed, this time because of MSVC: +// https://developercommunity.visualstudio.com/t/vc-ice-p1-initc-line-3652-from-stdto-array/1464038 +// Therefore a simpler (but also slightly more wobbly - it resolves the element +// type using std::common_type<>) make_array facility is implemented here. +template +constexpr auto make_array(Ts&&... items) { - return impl::to_array_impl(std::move(items), std::make_index_sequence{}); + return std::array, sizeof...(items)>( + { std::forward(items)... }); } // TODO: code like this should be generated from the CS API definition @@ -49,9 +44,9 @@ enum class Membership : unsigned int { Q_DECLARE_FLAGS(MembershipMask, Membership) Q_FLAG_NS(MembershipMask) -constexpr inline auto MembershipStrings = to_array( +constexpr inline auto MembershipStrings = make_array( // The order MUST be the same as the order in the original enum - { "join", "leave", "invite", "knock", "ban" }); + "join", "leave", "invite", "knock", "ban"); //! \brief Local user join-state names //! @@ -68,10 +63,10 @@ enum class JoinState : std::underlying_type_t { Q_DECLARE_FLAGS(JoinStates, JoinState) Q_FLAG_NS(JoinStates) -constexpr inline auto JoinStateStrings = to_array({ +constexpr inline auto JoinStateStrings = make_array( MembershipStrings[0], MembershipStrings[1], MembershipStrings[2], MembershipStrings[3] /* same as MembershipStrings, sans "ban" */ -}); +); //! \brief Network job running policy flags //! -- cgit v1.2.3 From 9da6b25a26403952e5a76b043076ba302c8d3c30 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sat, 11 Sep 2021 20:35:00 +0200 Subject: BaseJob: deprecate endpoint accessors; query returns an object To provide more room for internal changes in BaseJob. --- lib/jobs/basejob.cpp | 24 ++++++++++-------------- lib/jobs/basejob.h | 4 +++- 2 files changed, 13 insertions(+), 15 deletions(-) (limited to 'lib/jobs') diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index 6346db9d..85066024 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -194,12 +194,12 @@ QUrl BaseJob::requestUrl() const { return d->reply ? d->reply->url() : QUrl(); } bool BaseJob::isBackground() const { return d->inBackground; } -const QString& BaseJob::apiEndpoint() const { return d->apiEndpoint; } +//const QString& BaseJob::apiEndpoint() const { return d->apiUrl.path(); } -void BaseJob::setApiEndpoint(const QString& apiEndpoint) -{ - d->apiEndpoint = apiEndpoint; -} +//void BaseJob::setApiEndpoint(const QString& apiEndpoint) +//{ +// d->apiEndpoint = apiEndpoint; +//} const BaseJob::headers_t& BaseJob::requestHeaders() const { @@ -217,7 +217,7 @@ void BaseJob::setRequestHeaders(const BaseJob::headers_t& headers) d->requestHeaders = headers; } -const QUrlQuery& BaseJob::query() const { return d->requestQuery; } +QUrlQuery BaseJob::query() const { return d->requestQuery; } void BaseJob::setRequestQuery(const QUrlQuery& query) { @@ -262,14 +262,10 @@ QNetworkReply* BaseJob::reply() { return d->reply.data(); } QUrl BaseJob::makeRequestUrl(QUrl baseUrl, const QString& path, const QUrlQuery& query) { - auto pathBase = baseUrl.path(); - // QUrl::adjusted(QUrl::StripTrailingSlashes) doesn't help with root '/' - while (pathBase.endsWith('/')) - pathBase.chop(1); - if (!path.startsWith('/')) // Normally API files do start with '/' - pathBase.push_back('/'); // so this shouldn't be needed these days - - baseUrl.setPath(pathBase + path, QUrl::TolerantMode); + // Make sure the added path is relative even if it's not (the official + // API definitions have the leading slash though it's not really correct). + baseUrl = baseUrl.resolved( + QUrl(path.mid(path.startsWith('/')), QUrl::TolerantMode)); baseUrl.setQuery(query); return baseUrl; } diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h index 29443c53..663c121c 100644 --- a/lib/jobs/basejob.h +++ b/lib/jobs/basejob.h @@ -325,13 +325,15 @@ Q_SIGNALS: protected: using headers_t = QHash; + Q_DECL_DEPRECATED_X("Deprecated due to being unused") const QString& apiEndpoint() const; + Q_DECL_DEPRECATED_X("Deprecated due to being unused") void setApiEndpoint(const QString& apiEndpoint); const headers_t& requestHeaders() const; void setRequestHeader(const headers_t::key_type& headerName, const headers_t::mapped_type& headerValue); void setRequestHeaders(const headers_t& headers); - const QUrlQuery& query() const; + QUrlQuery query() const; void setRequestQuery(const QUrlQuery& query); const RequestData& requestData() const; void setRequestData(RequestData&& data); -- cgit v1.2.3 From 96f31d7d8ed1c9ab905c24ac039079aea622f4dc Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Mon, 4 Oct 2021 18:39:21 +0200 Subject: BaseJob: percent-encode variable path parts This is meant to spare clients from having to percent-encode room aliases, v3 event ids etc. that happen to hit the endpoint path. It is unfair to expect clients to do that since they are not supposed to care about the shape of CS API, which parameter should be encoded in which way. The trick (together with the slightly updated GTAD configuration) is to percent-encode parts that happen to be QStrings and not `const char[]`'s while passing all constant parts as plain C character literals. This also allows to make it more certain that the path is correctly encoded by passing and storing QByteArray's wherever the path is already encoded, and only use QStrings (next to const char arrays) before that. Since the change alters the API contract (even if that contract was crappy), some crude detection of percent-encoded stuff on input is inserted; if input is already percent-encoded, a warning is put to the logs, alerting developers about the change. --- gtad/gtad.yaml | 4 ++-- gtad/operation.cpp.mustache | 2 -- lib/jobs/basejob.cpp | 51 +++++++++++++++++++++++++++++++-------------- lib/jobs/basejob.h | 21 ++++++++++++++++--- lib/jobs/syncjob.cpp | 2 +- 5 files changed, 56 insertions(+), 24 deletions(-) (limited to 'lib/jobs') diff --git a/gtad/gtad.yaml b/gtad/gtad.yaml index ee8a43fe..943ac013 100644 --- a/gtad/gtad.yaml +++ b/gtad/gtad.yaml @@ -190,8 +190,8 @@ mustache: joinedParamDef: "{{>maybeCrefType}} {{paramName}}{{>cjoin}}" passPathAndMaybeQuery: >- - QStringLiteral("{{basePathWithoutHost}}") - {{#pathParts}} % {{_}}{{/pathParts}}{{#queryParams?}}, + makePath("{{basePathWithoutHost}}"{{#pathParts}}, + {{_}}{{/pathParts}}){{#queryParams?}}, queryTo{{camelCaseOperationId}}( {{#queryParams}}{{paramName}}{{>cjoin}}{{/queryParams}}){{/queryParams?}} diff --git a/gtad/operation.cpp.mustache b/gtad/operation.cpp.mustache index 7f692e4a..3d26ec73 100644 --- a/gtad/operation.cpp.mustache +++ b/gtad/operation.cpp.mustache @@ -4,8 +4,6 @@ SPDX-License-Identifier: LGPL-2.1-or-later }}{{>preamble}} #include "{{filenameBase}}.h" -#include - using namespace Quotient; {{#operations}}{{#operation}} {{#queryParams?}} diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index 85066024..73762e4f 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -71,7 +71,7 @@ public: // Using an idiom from clang-tidy: // http://clang.llvm.org/extra/clang-tidy/checks/modernize-pass-by-value.html - Private(HttpVerb v, QString endpoint, const QUrlQuery& q, + Private(HttpVerb v, QByteArray endpoint, const QUrlQuery& q, RequestData&& data, bool nt) : verb(v) , apiEndpoint(std::move(endpoint)) @@ -106,7 +106,7 @@ public: // Contents for the network request HttpVerb verb; - QString apiEndpoint; + QByteArray apiEndpoint; QHash requestHeaders; QUrlQuery requestQuery; RequestData requestData; @@ -166,14 +166,36 @@ public: } }; -BaseJob::BaseJob(HttpVerb verb, const QString& name, const QString& endpoint, +inline bool isHex(QChar c) +{ + return c.isDigit() || (c >= u'A' && c <= u'F') || (c >= u'a' && c <= u'f'); +} + +QByteArray BaseJob::encodeIfParam(const QString& paramPart) +{ + const auto percentIndex = paramPart.indexOf('%'); + if (percentIndex != -1 && paramPart.size() > percentIndex + 2 + && isHex(paramPart[percentIndex + 1]) + && isHex(paramPart[percentIndex + 2])) { + qCWarning(JOBS) + << "Developers, upfront percent-encoding of job parameters is " + "deprecated since libQuotient 0.7; the string involved is" + << paramPart; + return QUrl(paramPart, QUrl::TolerantMode).toEncoded(); + } + return QUrl::toPercentEncoding(paramPart); +} + +BaseJob::BaseJob(HttpVerb verb, const QString& name, QByteArray endpoint, bool needsToken) - : BaseJob(verb, name, endpoint, QUrlQuery {}, RequestData {}, needsToken) + : BaseJob(verb, name, std::move(endpoint), QUrlQuery {}, RequestData {}, + needsToken) {} -BaseJob::BaseJob(HttpVerb verb, const QString& name, const QString& endpoint, +BaseJob::BaseJob(HttpVerb verb, const QString& name, QByteArray endpoint, const QUrlQuery& query, RequestData&& data, bool needsToken) - : d(new Private(verb, endpoint, query, std::move(data), needsToken)) + : d(new Private(verb, std::move(endpoint), query, std::move(data), + needsToken)) { setObjectName(name); connect(&d->timer, &QTimer::timeout, this, &BaseJob::timeout); @@ -194,13 +216,6 @@ QUrl BaseJob::requestUrl() const { return d->reply ? d->reply->url() : QUrl(); } bool BaseJob::isBackground() const { return d->inBackground; } -//const QString& BaseJob::apiEndpoint() const { return d->apiUrl.path(); } - -//void BaseJob::setApiEndpoint(const QString& apiEndpoint) -//{ -// d->apiEndpoint = apiEndpoint; -//} - const BaseJob::headers_t& BaseJob::requestHeaders() const { return d->requestHeaders; @@ -259,13 +274,17 @@ const QNetworkReply* BaseJob::reply() const { return d->reply.data(); } QNetworkReply* BaseJob::reply() { return d->reply.data(); } -QUrl BaseJob::makeRequestUrl(QUrl baseUrl, const QString& path, +QUrl BaseJob::makeRequestUrl(QUrl baseUrl, const QByteArray& encodedPath, const QUrlQuery& query) { // Make sure the added path is relative even if it's not (the official // API definitions have the leading slash though it's not really correct). - baseUrl = baseUrl.resolved( - QUrl(path.mid(path.startsWith('/')), QUrl::TolerantMode)); + const auto pathUrl = + QUrl::fromEncoded(encodedPath.mid(encodedPath.startsWith('/')), + QUrl::StrictMode); + Q_ASSERT_X(pathUrl.isValid(), __FUNCTION__, + qPrintable(pathUrl.errorString())); + baseUrl = baseUrl.resolved(pathUrl); baseUrl.setQuery(query); return baseUrl; } diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h index 663c121c..81455307 100644 --- a/lib/jobs/basejob.h +++ b/lib/jobs/basejob.h @@ -9,6 +9,7 @@ #include "../converters.h" #include +#include class QNetworkReply; class QSslError; @@ -23,6 +24,14 @@ class BaseJob : public QObject { Q_PROPERTY(QUrl requestUrl READ requestUrl CONSTANT) Q_PROPERTY(int maxRetries READ maxRetries WRITE setMaxRetries) Q_PROPERTY(int statusCode READ error NOTIFY statusChanged) + + static QByteArray encodeIfParam(const QString& paramPart); + template + static inline auto encodeIfParam(const char (&constPart)[N]) + { + return constPart; + } + public: /*! The status code of a job * @@ -70,6 +79,12 @@ public: }; Q_ENUM(StatusCode) + template + static QByteArray makePath(StrTs&&... parts) + { + return (QByteArray() % ... % encodeIfParam(parts)); + } + using Data #ifndef Q_CC_MSVC Q_DECL_DEPRECATED_X("Use Quotient::RequestData instead") @@ -124,9 +139,9 @@ public: }; public: - BaseJob(HttpVerb verb, const QString& name, const QString& endpoint, + BaseJob(HttpVerb verb, const QString& name, QByteArray endpoint, bool needsToken = true); - BaseJob(HttpVerb verb, const QString& name, const QString& endpoint, + BaseJob(HttpVerb verb, const QString& name, QByteArray endpoint, const QUrlQuery& query, RequestData&& data = {}, bool needsToken = true); @@ -352,7 +367,7 @@ protected: * The function ensures exactly one '/' between the path component of * \p baseUrl and \p path. The query component of \p baseUrl is ignored. */ - static QUrl makeRequestUrl(QUrl baseUrl, const QString& path, + static QUrl makeRequestUrl(QUrl baseUrl, const QByteArray &encodedPath, const QUrlQuery& query = {}); /*! Prepares the job for execution diff --git a/lib/jobs/syncjob.cpp b/lib/jobs/syncjob.cpp index 59a34ef3..9b1b46f0 100644 --- a/lib/jobs/syncjob.cpp +++ b/lib/jobs/syncjob.cpp @@ -10,7 +10,7 @@ static size_t jobId = 0; SyncJob::SyncJob(const QString& since, const QString& filter, int timeout, const QString& presence) : BaseJob(HttpVerb::Get, QStringLiteral("SyncJob-%1").arg(++jobId), - QStringLiteral("_matrix/client/r0/sync")) + "_matrix/client/r0/sync") { setLoggingCategory(SYNCJOB); QUrlQuery query; -- cgit v1.2.3 From 67da887e864d292608e7132388f518596374af34 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Tue, 5 Oct 2021 03:43:22 +0200 Subject: BaseJob::StatusCode: officially deprecate most *Error enumerators --- lib/connection.cpp | 8 ++++---- lib/jobs/basejob.cpp | 42 +++++++++++++++++++++--------------------- lib/jobs/basejob.h | 34 ++++++++++++++++------------------ 3 files changed, 41 insertions(+), 43 deletions(-) (limited to 'lib/jobs') diff --git a/lib/connection.cpp b/lib/connection.cpp index 4abf5097..093362ab 100644 --- a/lib/connection.cpp +++ b/lib/connection.cpp @@ -288,7 +288,7 @@ void Connection::resolveServer(const QString& mxid) if (d->resolverJob->error() == BaseJob::Abandoned) return; - if (d->resolverJob->error() != BaseJob::NotFoundError) { + if (d->resolverJob->error() != BaseJob::NotFound) { if (!d->resolverJob->status().good()) { qCWarning(MAIN) << "Fetching .well-known file failed, FAIL_PROMPT"; @@ -401,7 +401,7 @@ void Connection::reloadCapabilities() " disabling version upgrade recommendations to reduce noise"; }); connect(d->capabilitiesJob, &BaseJob::failure, this, [this] { - if (d->capabilitiesJob->error() == BaseJob::IncorrectRequestError) + if (d->capabilitiesJob->error() == BaseJob::IncorrectRequest) qCDebug(MAIN) << "Server doesn't support /capabilities;" " version upgrade recommendations won't be issued"; }); @@ -1058,7 +1058,7 @@ ForgetRoomJob* Connection::forgetRoom(const QString& id) connect(leaveJob, &BaseJob::result, this, [this, leaveJob, forgetJob, room] { if (leaveJob->error() == BaseJob::Success - || leaveJob->error() == BaseJob::NotFoundError) { + || leaveJob->error() == BaseJob::NotFound) { run(forgetJob); // If the matching /sync response hasn't arrived yet, // mark the room for explicit deletion @@ -1077,7 +1077,7 @@ ForgetRoomJob* Connection::forgetRoom(const QString& id) connect(forgetJob, &BaseJob::result, this, [this, id, forgetJob] { // Leave room in case of success, or room not known by server if (forgetJob->error() == BaseJob::Success - || forgetJob->error() == BaseJob::NotFoundError) + || forgetJob->error() == BaseJob::NotFound) d->removeRoom(id); // Delete the room from roomMap else qCWarning(MAIN).nospace() << "Error forgetting room " << id << ": " diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index 73762e4f..a7921c61 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -24,7 +24,7 @@ BaseJob::StatusCode BaseJob::Status::fromHttpCode(int httpCode) { // Based on https://en.wikipedia.org/wiki/List_of_HTTP_status_codes if (httpCode / 10 == 41) // 41x errors - return httpCode == 410 ? IncorrectRequestError : NotFoundError; + return httpCode == 410 ? IncorrectRequest : NotFound; switch (httpCode) { case 401: return Unauthorised; @@ -32,19 +32,19 @@ BaseJob::StatusCode BaseJob::Status::fromHttpCode(int httpCode) case 403: case 407: // clang-format on return ContentAccessError; case 404: - return NotFoundError; + return NotFound; // clang-format off case 400: case 405: case 406: case 426: case 428: case 505: // clang-format on case 494: // Unofficial nginx "Request header too large" case 497: // Unofficial nginx "HTTP request sent to HTTPS port" - return IncorrectRequestError; + return IncorrectRequest; case 429: - return TooManyRequestsError; + return TooManyRequests; case 501: case 510: - return RequestNotImplementedError; + return RequestNotImplemented; case 511: - return NetworkAuthRequiredError; + return NetworkAuthRequired; default: return NetworkError; } @@ -365,7 +365,7 @@ void BaseJob::initiate(ConnectionData* connData, bool inBackground) qCCritical(d->logCat) << "Developers, ensure the Connection is valid before using it"; Q_ASSERT(false); - setStatus(IncorrectRequestError, tr("Invalid server connection")); + setStatus(IncorrectRequest, tr("Invalid server connection")); } // The status is no good, finalise QTimer::singleShot(0, this, &BaseJob::finishJob); @@ -528,7 +528,7 @@ BaseJob::Status BaseJob::prepareError() // of if's below will fall through to `return NoError` at the end const auto& errorJson = jsonData(); const auto errCode = errorJson.value("errcode"_ls).toString(); - if (error() == TooManyRequestsError || errCode == "M_LIMIT_EXCEEDED") { + if (error() == TooManyRequests || errCode == "M_LIMIT_EXCEEDED") { QString msg = tr("Too many requests"); int64_t retryAfterMs = errorJson.value("retry_after_ms"_ls).toInt(-1); if (retryAfterMs >= 0) @@ -538,16 +538,16 @@ BaseJob::Status BaseJob::prepareError() d->connection->limitRate(milliseconds(retryAfterMs)); - return { TooManyRequestsError, msg }; + return { TooManyRequests, msg }; } if (errCode == "M_CONSENT_NOT_GIVEN") { d->errorUrl = QUrl(errorJson.value("consent_uri"_ls).toString()); - return { UserConsentRequiredError }; + return { UserConsentRequired }; } if (errCode == "M_UNSUPPORTED_ROOM_VERSION" || errCode == "M_INCOMPATIBLE_ROOM_VERSION") - return { UnsupportedRoomVersionError, + return { UnsupportedRoomVersion, errorJson.contains("room_version"_ls) ? tr("Requested room version: %1") .arg(errorJson.value("room_version"_ls).toString()) @@ -729,27 +729,27 @@ QString BaseJob::statusCaption() const return tr("Request was abandoned"); case NetworkError: return tr("Network problems"); - case TimeoutError: + case Timeout: return tr("Request timed out"); case Unauthorised: return tr("Unauthorised request"); case ContentAccessError: return tr("Access error"); - case NotFoundError: + case NotFound: return tr("Not found"); - case IncorrectRequestError: + case IncorrectRequest: return tr("Invalid request"); - case IncorrectResponseError: + case IncorrectResponse: return tr("Response could not be parsed"); - case TooManyRequestsError: + case TooManyRequests: return tr("Too many requests"); - case RequestNotImplementedError: + case RequestNotImplemented: return tr("Function not implemented by the server"); - case NetworkAuthRequiredError: + case NetworkAuthRequired: return tr("Network authentication required"); - case UserConsentRequiredError: + case UserConsentRequired: return tr("User consent required"); - case UnsupportedRoomVersionError: + case UnsupportedRoomVersion: return tr("The server does not support the needed room version"); default: return tr("Request failed"); @@ -811,7 +811,7 @@ void BaseJob::abandon() void BaseJob::timeout() { - setStatus(TimeoutError, "The job has timed out"); + setStatus(Timeout, "The job has timed out"); finishJob(); } diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h index 81455307..e0910a26 100644 --- a/lib/jobs/basejob.h +++ b/lib/jobs/basejob.h @@ -33,6 +33,10 @@ class BaseJob : public QObject { } public: +#define WITH_DEPRECATED_ERROR_VERSION(Recommended) \ + Recommended, Recommended##Error Q_DECL_ENUMERATOR_DEPRECATED_X( \ + "Use " #Recommended) = Recommended + /*! The status code of a job * * Every job is created in Unprepared status; upon calling prepare() @@ -43,7 +47,7 @@ public: */ enum StatusCode { Success = 0, - NoError = Success, // To be compatible with Qt conventions + NoError = Success, Pending = 1, WarningLevel = 20, //< Warnings have codes starting from this UnexpectedResponseType = 21, @@ -52,26 +56,18 @@ public: Abandoned = 50, //< A tiny period between abandoning and object deletion ErrorLevel = 100, //< Errors have codes starting from this NetworkError = 101, - Timeout, - TimeoutError = Timeout, + WITH_DEPRECATED_ERROR_VERSION(Timeout), Unauthorised, ContentAccessError, - NotFoundError, - IncorrectRequest, - IncorrectRequestError = IncorrectRequest, - IncorrectResponse, - IncorrectResponseError = IncorrectResponse, - TooManyRequests, - TooManyRequestsError = TooManyRequests, + WITH_DEPRECATED_ERROR_VERSION(NotFound), + WITH_DEPRECATED_ERROR_VERSION(IncorrectRequest), + WITH_DEPRECATED_ERROR_VERSION(IncorrectResponse), + WITH_DEPRECATED_ERROR_VERSION(TooManyRequests), RateLimited = TooManyRequests, - RequestNotImplemented, - RequestNotImplementedError = RequestNotImplemented, - UnsupportedRoomVersion, - UnsupportedRoomVersionError = UnsupportedRoomVersion, - NetworkAuthRequired, - NetworkAuthRequiredError = NetworkAuthRequired, - UserConsentRequired, - UserConsentRequiredError = UserConsentRequired, + WITH_DEPRECATED_ERROR_VERSION(RequestNotImplemented), + WITH_DEPRECATED_ERROR_VERSION(UnsupportedRoomVersion), + WITH_DEPRECATED_ERROR_VERSION(NetworkAuthRequired), + WITH_DEPRECATED_ERROR_VERSION(UserConsentRequired), CannotLeaveRoom, UserDeactivated, FileError, @@ -79,6 +75,8 @@ public: }; Q_ENUM(StatusCode) +#undef WITH_DEPRECATED_ERROR_VERSION + template static QByteArray makePath(StrTs&&... parts) { -- cgit v1.2.3 From 9cfd6ad6a4651280a12099d1a92432f07fc5aae0 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Tue, 5 Oct 2021 03:44:18 +0200 Subject: BaseJob: refresh error handling - BaseJob::prepareError() slightly updated to get the current status instead of checking the returned value outside in gotReply() - BaseJob::gotReply() no more reports on 429 Too Many Requests twice (the first time with dubious "Too Many Requests: Unknown error") --- lib/jobs/basejob.cpp | 46 +++++++++++++++++++++++----------------------- lib/jobs/basejob.h | 6 ++++-- 2 files changed, 27 insertions(+), 25 deletions(-) (limited to 'lib/jobs') diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index a7921c61..971fea7b 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -415,42 +415,42 @@ BaseJob::Status BaseJob::Private::parseJson() void BaseJob::gotReply() { - setStatus(checkReply(reply())); - - if (status().good() - && d->expectedContentTypes == QByteArrayList { "application/json" }) { + // Defer actually updating the status until it's finalised + auto statusSoFar = checkReply(reply()); + if (statusSoFar.good() + && d->expectedContentTypes == QByteArrayList { "application/json" }) // + { d->rawResponse = reply()->readAll(); - setStatus(d->parseJson()); - if (status().good() && !expectedKeys().empty()) { + statusSoFar = d->parseJson(); + if (statusSoFar.good() && !expectedKeys().empty()) { const auto& responseObject = jsonData(); QByteArrayList missingKeys; for (const auto& k: expectedKeys()) if (!responseObject.contains(k)) missingKeys.push_back(k); if (!missingKeys.empty()) - setStatus(IncorrectResponse, tr("Required JSON keys missing: ") - + missingKeys.join()); + statusSoFar = { IncorrectResponse, + tr("Required JSON keys missing: ") + + missingKeys.join() }; } + setStatus(statusSoFar); if (!status().good()) // Bad JSON in a "good" reply: bail out return; - } // else { + } // If the endpoint expects anything else than just (API-related) JSON // reply()->readAll() is not performed and the whole reply processing // is left to derived job classes: they may read it piecemeal or customise // per content type in prepareResult(), or even have read it already // (see, e.g., DownloadFileJob). - // } - - if (status().good()) + if (statusSoFar.good()) { setStatus(prepareResult()); - else { - d->rawResponse = reply()->readAll(); - qCDebug(d->logCat).noquote() - << "Error body (truncated if long):" << rawDataSample(500); - // Parse the error payload and update the status if needed - if (const auto newStatus = prepareError(); !newStatus.good()) - setStatus(newStatus); + return; } + + d->rawResponse = reply()->readAll(); + qCDebug(d->logCat).noquote() + << "Error body (truncated if long):" << rawDataSample(500); + setStatus(prepareError(statusSoFar)); } bool checkContentType(const QByteArray& type, const QByteArrayList& patterns) @@ -515,7 +515,7 @@ BaseJob::Status BaseJob::checkReply(const QNetworkReply* reply) const BaseJob::Status BaseJob::prepareResult() { return Success; } -BaseJob::Status BaseJob::prepareError() +BaseJob::Status BaseJob::prepareError(Status currentStatus) { // Try to make sense of the error payload but be prepared for all kinds // of unexpected stuff (raw HTML, plain text, foreign JSON among those) @@ -525,7 +525,7 @@ BaseJob::Status BaseJob::prepareError() // By now, if d->parseJson() above succeeded then jsonData() will return // a valid JSON object - or an empty object otherwise (in which case most - // of if's below will fall through to `return NoError` at the end + // of if's below will fall through retaining the current status) const auto& errorJson = jsonData(); const auto errCode = errorJson.value("errcode"_ls).toString(); if (error() == TooManyRequests || errCode == "M_LIMIT_EXCEEDED") { @@ -560,9 +560,9 @@ BaseJob::Status BaseJob::prepareError() // Not localisable on the client side if (errorJson.contains("error"_ls)) // Keep the code, update the message - return { d->status.code, errorJson.value("error"_ls).toString() }; + return { currentStatus.code, errorJson.value("error"_ls).toString() }; - return NoError; // Retain the status if the error payload is not recognised + return currentStatus; // The error payload is not recognised } QJsonValue BaseJob::takeValueFromJson(const QString& key) diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h index e0910a26..119d7cce 100644 --- a/lib/jobs/basejob.h +++ b/lib/jobs/basejob.h @@ -399,10 +399,12 @@ protected: * was not good (usually because of an unsuccessful HTTP code). * The base implementation assumes Matrix JSON error object in the body; * overrides are strongly recommended to call it for all stock Matrix - * responses as early as possible but in addition can process custom errors, + * responses as early as possible and only then process custom errors, * with JSON or non-JSON payload. + * + * \return updated (if necessary) job status */ - virtual Status prepareError(); + virtual Status prepareError(Status currentStatus); /*! \brief Get direct access to the JSON response object in the job * -- cgit v1.2.3