From 903e309c51cdbe9b29131a1ce1a24facfd51de9e Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Thu, 10 Dec 2020 17:34:10 +0100 Subject: Uri: fix a few cases of insufficient escaping --- tests/quotest.cpp | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) (limited to 'tests') diff --git a/tests/quotest.cpp b/tests/quotest.cpp index 3b7f2f48..9930a3ab 100644 --- a/tests/quotest.cpp +++ b/tests/quotest.cpp @@ -727,14 +727,19 @@ TEST_IMPL(visitResources) "matrix:room/" + roomAlias.mid(1) + "/event/" + eventId.mid(1), "https://matrix.to/#/" + roomId + '/' + eventId }; - // The following URIs are not supposed to be actually joined (and even - // exist, as yet) - only to be syntactically correct + // Check that reserved characters are correctly processed. static const auto& joinRoomAlias = - QStringLiteral("unjoined:example.org"); // # will be added below + QStringLiteral("##/?.@\"unjoined:example.org"); + static const auto& encodedRoomAliasNoSigil = + QUrl::toPercentEncoding(joinRoomAlias.mid(1), ":"); static const QString joinQuery { "?action=join" }; + // These URIs are not supposed to be actually joined (and even exist, + // as yet) - only to be syntactically correct static const QStringList joinByAliasUris { - "matrix:room/" + joinRoomAlias + joinQuery, - "https://matrix.to/#/%23"/*`#`*/ + joinRoomAlias + joinQuery + Uri(joinRoomAlias.toUtf8(), {}, joinQuery.mid(1)).toDisplayString(), + "matrix:room/" + encodedRoomAliasNoSigil + joinQuery, + "https://matrix.to/#/%23"/*`#`*/ + encodedRoomAliasNoSigil + joinQuery, + "https://matrix.to/#/%23" + joinRoomAlias.mid(1) /* unencoded */ + joinQuery }; static const auto& joinRoomId = QStringLiteral("!anyid:example.org"); static const QStringList viaServers { "matrix.org", "example.org" }; @@ -755,7 +760,7 @@ TEST_IMPL(visitResources) || testResourceResolver(eventUris, &UriDispatcher::roomAction, room(), { eventId }) || testResourceResolver(joinByAliasUris, &UriDispatcher::joinAction, - connection(), { '#' + joinRoomAlias }) + connection(), { joinRoomAlias }) || testResourceResolver(joinByIdUris, &UriDispatcher::joinAction, connection(), { joinRoomId, viaServers })) return true; -- cgit v1.2.3 From e617f0151df9a5edbefeb2c36d306a2989a278af Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Thu, 24 Dec 2020 18:20:04 +0100 Subject: Fix clang-tidy/clazy warnings (cherry picked from commit 0a2acd750a4155969092be674ed3dd9a71b2354f) --- lib/connection.cpp | 4 ++-- lib/converters.cpp | 4 ++-- lib/jobs/downloadfilejob.cpp | 2 +- lib/networkaccessmanager.cpp | 7 ++++--- lib/room.cpp | 49 ++++++++++++++++++++++---------------------- lib/uri.cpp | 8 ++++---- lib/uriresolver.cpp | 2 ++ lib/user.cpp | 4 ++-- lib/util.cpp | 5 +---- tests/quotest.cpp | 38 +++++++++++++++++++--------------- 10 files changed, 64 insertions(+), 59 deletions(-) (limited to 'tests') diff --git a/lib/connection.cpp b/lib/connection.cpp index 386c6564..b76ca691 100644 --- a/lib/connection.cpp +++ b/lib/connection.cpp @@ -930,8 +930,8 @@ void Connection::doInDirectChat(User* u, // There can be more than one DC; find the first valid (existing and // not left), and delete inexistent (forgotten?) ones along the way. DirectChatsMap removals; - for (auto it = std::as_const(d->directChats).find(u); - it != d->directChats.end() && it.key() == u; ++it) { + for (auto it = d->directChats.constFind(u); + it != d->directChats.cend() && it.key() == u; ++it) { const auto& roomId = *it; if (auto r = room(roomId, JoinState::Join)) { Q_ASSERT(r->id() == roomId); diff --git a/lib/converters.cpp b/lib/converters.cpp index e5236bb9..9f570087 100644 --- a/lib/converters.cpp +++ b/lib/converters.cpp @@ -32,9 +32,9 @@ QVariant JsonConverter::load(const QJsonValue& jv) return jv.toVariant(); } -QJsonObject JsonConverter::dump(const QVariantHash& map) +QJsonObject JsonConverter::dump(const QVariantHash& vh) { - return QJsonObject::fromVariantHash(map); + return QJsonObject::fromVariantHash(vh); } QVariantHash JsonConverter::load(const QJsonValue& jv) diff --git a/lib/jobs/downloadfilejob.cpp b/lib/jobs/downloadfilejob.cpp index 7b4cf690..0011a97c 100644 --- a/lib/jobs/downloadfilejob.cpp +++ b/lib/jobs/downloadfilejob.cpp @@ -64,7 +64,7 @@ void DownloadFileJob::onSentRequest(QNetworkReply* reply) return; auto sizeHeader = reply->header(QNetworkRequest::ContentLengthHeader); if (sizeHeader.isValid()) { - auto targetSize = sizeHeader.value(); + auto targetSize = sizeHeader.toLongLong(); if (targetSize != -1) if (!d->tempFile->resize(targetSize)) { qCWarning(JOBS) << "Failed to allocate" << targetSize diff --git a/lib/networkaccessmanager.cpp b/lib/networkaccessmanager.cpp index b9037bcc..e8aa85df 100644 --- a/lib/networkaccessmanager.cpp +++ b/lib/networkaccessmanager.cpp @@ -52,9 +52,10 @@ static NetworkAccessManager* createNam() auto nam = new NetworkAccessManager(QCoreApplication::instance()); #if (QT_VERSION < QT_VERSION_CHECK(5, 15, 0)) // See #109; in newer Qt, bearer management is deprecated altogether - nam->connect(nam, &QNetworkAccessManager::networkAccessibleChanged, [nam] { - nam->setNetworkAccessible(QNetworkAccessManager::Accessible); - }); + NetworkAccessManager::connect(nam, + &QNetworkAccessManager::networkAccessibleChanged, [nam] { + nam->setNetworkAccessible(QNetworkAccessManager::Accessible); + }); #endif return nam; } diff --git a/lib/room.cpp b/lib/room.cpp index 5992a3ae..55e2931e 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -350,7 +350,7 @@ public: */ bool processReplacement(const RoomMessageEvent& newEvent); - void setTags(TagsMap newTags); + void setTags(TagsMap&& newTags); QJsonObject toJson() const; @@ -1073,11 +1073,11 @@ void Room::setTags(TagsMap newTags, ActionScope applyOn) if (propagate) { for (auto* r = this; (r = r->successor(joinStates));) - r->setTags(newTags, ActionScope::ThisRoomOnly); + r->setTags(d->tags, ActionScope::ThisRoomOnly); } } -void Room::Private::setTags(TagsMap newTags) +void Room::Private::setTags(TagsMap&& newTags) { emit q->tagsAboutToChange(); const auto keys = newTags.keys(); @@ -1192,8 +1192,8 @@ QString Room::fileNameToDownload(const QString& eventId) const FileTransferInfo Room::fileTransferInfo(const QString& id) const { - auto infoIt = d->fileTransfers.find(id); - if (infoIt == d->fileTransfers.end()) + const auto infoIt = d->fileTransfers.constFind(id); + if (infoIt == d->fileTransfers.cend()) return {}; // FIXME: Add lib tests to make sure FileTransferInfo::status stays @@ -1222,8 +1222,8 @@ QUrl Room::fileSource(const QString& id) const return url; // No urlToDownload means it's a pending or completed upload. - auto infoIt = d->fileTransfers.find(id); - if (infoIt != d->fileTransfers.end()) + auto infoIt = d->fileTransfers.constFind(id); + if (infoIt != d->fileTransfers.cend()) return QUrl::fromLocalFile(infoIt->localFileInfo.absoluteFilePath()); qCWarning(MAIN) << "File source for identifier" << id << "not found"; @@ -1312,7 +1312,6 @@ void Room::handleRoomKeyEvent(const RoomKeyEvent& roomKeyEvent, Q_UNUSED(roomKeyEvent) Q_UNUSED(senderKey) qCWarning(E2EE) << "End-to-end encryption (E2EE) support is turned off."; - return; #else // Quotient_E2EE_ENABLED if (roomKeyEvent.algorithm() != MegolmV1AesSha2AlgoKey) { qCWarning(E2EE) << "Ignoring unsupported algorithm" @@ -1438,7 +1437,7 @@ Room::Private::moveEventsToTimeline(RoomEventsRange events, } const auto insertedSize = (index - baseIndex) * placement; Q_ASSERT(insertedSize == int(events.size())); - return insertedSize; + return Timeline::size_type(insertedSize); } QString Room::memberName(const QString& mxId) const @@ -1654,8 +1653,8 @@ QString Room::retryMessage(const QString& txnId) const auto it = findPendingEvent(txnId); Q_ASSERT(it != d->unsyncedEvents.end()); qCDebug(EVENTS) << "Retrying transaction" << txnId; - const auto& transferIt = d->fileTransfers.find(txnId); - if (transferIt != d->fileTransfers.end()) { + const auto& transferIt = d->fileTransfers.constFind(txnId); + if (transferIt != d->fileTransfers.cend()) { Q_ASSERT(transferIt->isUpload); if (transferIt->status == FileTransferInfo::Completed) { qCDebug(MESSAGES) @@ -1752,7 +1751,8 @@ QString Room::postFile(const QString& plainText, const QUrl& localPath, // to enable the preview while the event is pending. uploadFile(txnId, localPath); // Below, the upload job is used as a context object to clean up connections - connect(this, &Room::fileTransferCompleted, d->fileTransfers[txnId].job, + const auto& transferJob = d->fileTransfers.value(txnId).job; + connect(this, &Room::fileTransferCompleted, transferJob, [this, txnId](const QString& id, const QUrl&, const QUrl& mxcUri) { if (id == txnId) { auto it = findPendingEvent(txnId); @@ -1771,7 +1771,7 @@ QString Room::postFile(const QString& plainText, const QUrl& localPath, } } }); - connect(this, &Room::fileTransferCancelled, d->fileTransfers[txnId].job, + connect(this, &Room::fileTransferCancelled, transferJob, [this, txnId](const QString& id) { if (id == txnId) { auto it = findPendingEvent(txnId); @@ -1973,8 +1973,8 @@ void Room::uploadFile(const QString& id, const QUrl& localFilename, void Room::downloadFile(const QString& eventId, const QUrl& localFilename) { - auto ongoingTransfer = d->fileTransfers.find(eventId); - if (ongoingTransfer != d->fileTransfers.end() + if (auto ongoingTransfer = d->fileTransfers.constFind(eventId); + ongoingTransfer != d->fileTransfers.cend() && ongoingTransfer->status == FileTransferInfo::Started) { qCWarning(MAIN) << "Transfer for" << eventId << "is ongoing; download won't start"; @@ -2031,8 +2031,8 @@ void Room::downloadFile(const QString& eventId, const QUrl& localFilename) void Room::cancelFileTransfer(const QString& id) { - auto it = d->fileTransfers.find(id); - if (it == d->fileTransfers.end()) { + const auto it = d->fileTransfers.constFind(id); + if (it == d->fileTransfers.cend()) { qCWarning(MAIN) << "No information on file transfer" << id << "in room" << d->id; return; @@ -2134,8 +2134,8 @@ bool Room::Private::processRedaction(const RedactionEvent& redaction) { // Can't use findInTimeline because it returns a const iterator, and // we need to change the underlying TimelineItem. - const auto pIdx = eventsIndex.find(redaction.redactedEvent()); - if (pIdx == eventsIndex.end()) + const auto pIdx = eventsIndex.constFind(redaction.redactedEvent()); + if (pIdx == eventsIndex.cend()) return false; Q_ASSERT(q->isValidIndex(*pIdx)); @@ -2205,8 +2205,8 @@ bool Room::Private::processReplacement(const RoomMessageEvent& newEvent) { // Can't use findInTimeline because it returns a const iterator, and // we need to change the underlying TimelineItem. - const auto pIdx = eventsIndex.find(newEvent.replacedEvent()); - if (pIdx == eventsIndex.end()) + const auto pIdx = eventsIndex.constFind(newEvent.replacedEvent()); + if (pIdx == eventsIndex.cend()) return false; Q_ASSERT(q->isValidIndex(*pIdx)); @@ -2451,12 +2451,11 @@ Room::Changes Room::processStateEvent(const RoomEvent& e) if (!e.isStateEvent()) return Change::NoChange; - // Find a value (create empty if necessary) and get a reference to it - // getCurrentState<> is not used here because it (creates and) returns + // Find a value (create an empty one if necessary) and get a reference + // to it. Can't use getCurrentState<>() because it (creates and) returns // a stub if a value is not found, and what's needed here is a "real" event // or nullptr. - const auto*& curStateEvent = - d->currentState[{ e.matrixType(), e.stateKey() }]; + auto& curStateEvent = d->currentState[{ e.matrixType(), e.stateKey() }]; // Prepare for the state change visit(e, [this, oldRme = static_cast(curStateEvent)]( const RoomMemberEvent& rme) { diff --git a/lib/uri.cpp b/lib/uri.cpp index 0e2bcb87..e0912eb6 100644 --- a/lib/uri.cpp +++ b/lib/uri.cpp @@ -32,7 +32,7 @@ Uri::Uri(QByteArray primaryId, QByteArray secondaryId, QString query) primaryType_ = Type(p.sigil); auto safePrimaryId = primaryId.mid(1); safePrimaryId.replace('/', "%2F"); - pathToBe = p.uriString + std::move(safePrimaryId); + pathToBe = p.uriString + safePrimaryId; break; } if (!secondaryId.isEmpty()) { @@ -42,12 +42,12 @@ Uri::Uri(QByteArray primaryId, QByteArray secondaryId, QString query) } auto safeSecondaryId = secondaryId.mid(1); safeSecondaryId.replace('/', "%2F"); - pathToBe += "/event/" + std::move(safeSecondaryId); + pathToBe += "/event/" + safeSecondaryId; } setPath(pathToBe, QUrl::TolerantMode); } if (!query.isEmpty()) - setQuery(std::move(query)); + setQuery(query); } static inline auto encodedPath(const QUrl& url) @@ -156,7 +156,7 @@ QUrl Uri::toUrl(UriForm form) const return {}; if (form == CanonicalUri || type() == NonMatrix) - return *this; + return *this; // NOLINT(cppcoreguidelines-slicing): It's intentional QUrl url; url.setScheme("https"); diff --git a/lib/uriresolver.cpp b/lib/uriresolver.cpp index ec30512c..27360bcc 100644 --- a/lib/uriresolver.cpp +++ b/lib/uriresolver.cpp @@ -75,6 +75,8 @@ private: std::tuple fns_; }; +template +StaticUriDispatcher(FnTs&&... fns) -> StaticUriDispatcher; UriResolveResult Quotient::visitResource( Connection* account, const Uri& uri, diff --git a/lib/user.cpp b/lib/user.cpp index 45a9c121..c399ce88 100644 --- a/lib/user.cpp +++ b/lib/user.cpp @@ -78,7 +78,7 @@ QString User::id() const { return d->id; } bool User::isGuest() const { Q_ASSERT(!d->id.isEmpty() && d->id.startsWith('@')); - auto it = std::find_if_not(d->id.begin() + 1, d->id.end(), + auto it = std::find_if_not(d->id.cbegin() + 1, d->id.cend(), [](QChar c) { return c.isDigit(); }); Q_ASSERT(it != d->id.end()); return *it == ':'; @@ -138,7 +138,7 @@ inline bool User::doSetAvatar(SourceT&& source) connect(j, &BaseJob::success, this, [this, newUrl = QUrl(contentUri)] { if (newUrl == d->defaultAvatar.url()) { - d->defaultAvatar.updateUrl(move(newUrl)); + d->defaultAvatar.updateUrl(newUrl); emit defaultAvatarChanged(); } else qCWarning(MAIN) << "User" << id() diff --git a/lib/util.cpp b/lib/util.cpp index 0c1c54ff..36015d7a 100644 --- a/lib/util.cpp +++ b/lib/util.cpp @@ -161,9 +161,6 @@ static_assert(std::is_same, int>(), "Test fn_arg_t defaulting to first argument"); template -static QString ft(T&&) -{ - return {}; -} +static QString ft(T&&); static_assert(std::is_same)>, QString&&>(), "Test function templates"); diff --git a/tests/quotest.cpp b/tests/quotest.cpp index 9930a3ab..d4bfa786 100644 --- a/tests/quotest.cpp +++ b/tests/quotest.cpp @@ -104,17 +104,21 @@ private slots: // Add more tests above here public: - Room* room() const { return targetRoom; } - Connection* connection() const { return targetRoom->connection(); } + [[nodiscard]] Room* room() const { return targetRoom; } + [[nodiscard]] Connection* connection() const + { + return targetRoom->connection(); + } private: - bool checkFileSendingOutcome(const TestToken& thisTest, - const QString& txnId, const QString& fileName); - bool checkRedactionOutcome(const QByteArray& thisTest, - const QString& evtIdToRedact); - - bool validatePendingEvent(const QString& txnId); - bool checkDirectChat() const; + [[nodiscard]] bool checkFileSendingOutcome(const TestToken& thisTest, + const QString& txnId, + const QString& fileName); + [[nodiscard]] bool checkRedactionOutcome(const QByteArray& thisTest, + const QString& evtIdToRedact); + + [[nodiscard]] bool validatePendingEvent(const QString& txnId); + [[nodiscard]] bool checkDirectChat() const; void finishTest(const TestToken& token, bool condition, const char* file, int line); @@ -185,7 +189,7 @@ TestManager::TestManager(int& argc, char** argv) [this](const QString& error) { clog << "Failed to resolve the server: " << error.toStdString() << endl; - this->exit(-2); + QCoreApplication::exit(-2); }, Qt::QueuedConnection); connect(c, &Connection::loginError, this, @@ -195,7 +199,7 @@ TestManager::TestManager(int& argc, char** argv) << message.toStdString() << endl << "Details:" << endl << details.toStdString() << endl; - this->exit(-2); + QCoreApplication::exit(-2); }, Qt::QueuedConnection); connect(c, &Connection::loadedRoomState, this, &TestManager::onNewRoom); @@ -320,7 +324,7 @@ TEST_IMPL(loadMembers) FAIL_TEST(); } r->setDisplayed(); - connect(r, &Room::allMembersLoaded, [this, thisTest, r] { + connect(r, &Room::allMembersLoaded, this, [this, thisTest, r] { FINISH_TEST(r->memberNames().size() >= r->joinedCount()); }); return false; @@ -745,7 +749,7 @@ TEST_IMPL(visitResources) static const QStringList viaServers { "matrix.org", "example.org" }; static const auto viaQuery = std::accumulate(viaServers.cbegin(), viaServers.cend(), joinQuery, - [](QString q, const QString& s) { + [](const QString& q, const QString& s) { return q + "&via=" + s; }); static const QStringList joinByIdUris { @@ -807,7 +811,8 @@ void TestManager::conclude() // Now just wait until all the pending events reach the server connectUntil(room, &Room::messageSent, this, [this, room, plainReport] { const auto& pendingEvents = room->pendingEvents(); - if (auto c = std::count_if(pendingEvents.begin(), pendingEvents.end(), + if (auto c = std::count_if(pendingEvents.cbegin(), + pendingEvents.cend(), [](const PendingEventItem& pe) { return pe.deliveryStatus() < EventStatus::ReachedServer; @@ -834,8 +839,8 @@ void TestManager::finalize() { clog << "Logging out" << endl; c->logout(); - connect(c, &Connection::loggedOut, - this, [this] { this->exit(failed.size() + running.size()); }, + connect(c, &Connection::loggedOut, this, + [this] { QCoreApplication::exit(failed.size() + running.size()); }, Qt::QueuedConnection); } @@ -847,6 +852,7 @@ int main(int argc, char* argv[]) << endl; return -1; } + // NOLINTNEXTLINE(readability-static-accessed-through-instance) return TestManager(argc, argv).exec(); } -- cgit v1.2.3 From 34660bed9e90374a1ace0913331ca2965513d19b Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Thu, 24 Dec 2020 18:23:00 +0100 Subject: quotest: wait until the final report is actually sent Previously the code was waiting until an arbitrary event is sent. (cherry picked from commit 424d5c33542c4c38baefb773163e9ebed1accfb6) --- tests/quotest.cpp | 55 +++++++++++++++++++++++++++++-------------------------- 1 file changed, 29 insertions(+), 26 deletions(-) (limited to 'tests') diff --git a/tests/quotest.cpp b/tests/quotest.cpp index d4bfa786..2b1f0229 100644 --- a/tests/quotest.cpp +++ b/tests/quotest.cpp @@ -803,36 +803,39 @@ void TestManager::conclude() htmlReport += "
Did not finish:" + dnfList; } - // TODO: Waiting for proper futures to come so that it could be: - // targetRoom->postHtmlText(...) - // .then(this, &TestManager::finalize); // Qt-style or - // .then([this] { finalize(); }); // STL-style auto txnId = room->postHtmlText(plainReport, htmlReport); // Now just wait until all the pending events reach the server - connectUntil(room, &Room::messageSent, this, [this, room, plainReport] { - const auto& pendingEvents = room->pendingEvents(); - if (auto c = std::count_if(pendingEvents.cbegin(), - pendingEvents.cend(), - [](const PendingEventItem& pe) { - return pe.deliveryStatus() - < EventStatus::ReachedServer; - }); - c > 0) { - clog << "Events to reach the server: " << c << ", not leaving yet" - << endl; - return false; - } + connectUntil(room, &Room::messageSent, this, + [this, txnId, room, plainReport] (const QString& sentTxnId) { + if (sentTxnId != txnId) + return false; + const auto& pendingEvents = room->pendingEvents(); + if (auto c = std::count_if(pendingEvents.cbegin(), + pendingEvents.cend(), + [](const PendingEventItem& pe) { + return pe.deliveryStatus() + < EventStatus::ReachedServer; + }); + c > 0) { + clog << "Events to reach the server: " << c + << ", not leaving yet" << endl; + return false; + } - clog << "Leaving the room" << endl; - auto* job = room->leaveRoom(); - connect(job, &BaseJob::finished, this, [this, job,plainReport] { - Q_ASSERT(job->status().good()); - finalize(); - // Still flying, as the exit() connection in finalize() is queued - clog << plainReport.toStdString() << endl; + clog << "Leaving the room" << endl; + // TODO: Waiting for proper futures to come so that it could be: +// room->leaveRoom() +// .then(this, &TestManager::finalize); // Qt-style or +// .then([this] { finalize(); }); // STL-style + auto* job = room->leaveRoom(); + connect(job, &BaseJob::finished, this, [this, job,plainReport] { + Q_ASSERT(job->status().good()); + finalize(); + // Still flying, as the exit() connection in finalize() is queued + clog << plainReport.toStdString() << endl; + }); + return true; }); - return true; - }); } void TestManager::finalize() -- cgit v1.2.3 From 56c1db077b5da653c230432abc6c746318a77bed Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 27 Dec 2020 23:31:20 +0100 Subject: Cleanup and clang-tidy/clazy fixes --- lib/jobs/basejob.cpp | 2 +- lib/jobs/basejob.h | 2 +- lib/room.cpp | 33 ++++++++++++--------------------- tests/quotest.cpp | 8 ++++---- 4 files changed, 18 insertions(+), 27 deletions(-) (limited to 'tests') diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index 2ac942f5..1a41f795 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -263,7 +263,7 @@ void BaseJob::setExpectedContentTypes(const QByteArrayList& contentTypes) d->expectedContentTypes = contentTypes; } -const QByteArrayList BaseJob::expectedKeys() const { return d->expectedKeys; } +QByteArrayList BaseJob::expectedKeys() const { return d->expectedKeys; } void BaseJob::addExpectedKey(const QByteArray& key) { d->expectedKeys << key; } diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h index a0b89ef7..a72f6120 100644 --- a/lib/jobs/basejob.h +++ b/lib/jobs/basejob.h @@ -363,7 +363,7 @@ protected: const QByteArrayList& expectedContentTypes() const; void addExpectedContentType(const QByteArray& contentType); void setExpectedContentTypes(const QByteArrayList& contentTypes); - const QByteArrayList expectedKeys() const; + QByteArrayList expectedKeys() const; void addExpectedKey(const QByteArray &key); void setExpectedKeys(const QByteArrayList &keys); diff --git a/lib/room.cpp b/lib/room.cpp index a19624d8..602653f3 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -2522,31 +2522,20 @@ Room::Changes Room::processStateEvent(const RoomEvent& e) , [this, oldStateEvent] (const RoomCanonicalAliasEvent& cae) { // clang-format on setObjectName(cae.alias().isEmpty() ? d->id : cae.alias()); - QString previousCanonicalAlias = - oldStateEvent - ? static_cast(oldStateEvent) - ->alias() - : QString(); - - auto previousAltAliases = - oldStateEvent - ? static_cast(oldStateEvent) - ->altAliases() - : QStringList(); - - if (!previousCanonicalAlias.isEmpty()) { - previousAltAliases.push_back(previousCanonicalAlias); + const auto* oldCae = + static_cast(oldStateEvent); + QStringList previousAltAliases {}; + if (oldCae) { + previousAltAliases = oldCae->altAliases(); + if (!oldCae->alias().isEmpty()) + previousAltAliases.push_back(oldCae->alias()); } - const auto previousAliases = std::move(previousAltAliases); - auto newAliases = cae.altAliases(); - - if (!cae.alias().isEmpty()) { + if (!cae.alias().isEmpty()) newAliases.push_front(cae.alias()); - } - connection()->updateRoomAliases(id(), previousAliases, newAliases); + connection()->updateRoomAliases(id(), previousAltAliases, newAliases); return AliasesChange; // clang-format off } @@ -2588,7 +2577,9 @@ Room::Changes Room::processStateEvent(const RoomEvent& e) if (u == localUser() && evt.isDirect()) connection()->addToDirectChats(this, user(evt.senderId())); break; - default: + case MembershipType::Knock: + case MembershipType::Ban: + case MembershipType::Leave: if (!d->membersLeft.contains(u)) d->membersLeft.append(u); } diff --git a/tests/quotest.cpp b/tests/quotest.cpp index 2b1f0229..a0bad753 100644 --- a/tests/quotest.cpp +++ b/tests/quotest.cpp @@ -186,7 +186,7 @@ TestManager::TestManager(int& argc, char** argv) connect(c, &Connection::connected, this, &TestManager::setupAndRun); connect(c, &Connection::resolveError, this, - [this](const QString& error) { + [](const QString& error) { clog << "Failed to resolve the server: " << error.toStdString() << endl; QCoreApplication::exit(-2); @@ -268,7 +268,7 @@ void TestManager::onNewRoom(Room* r) << endl; connect(r, &Room::aboutToAddNewMessages, r, [r](RoomEventsRange timeline) { clog << timeline.size() << " new event(s) in room " - << r->canonicalAlias().toStdString() << endl; + << r->objectName().toStdString() << endl; }); } @@ -319,13 +319,13 @@ TEST_IMPL(loadMembers) // It's not exactly correct because an arbitrary server might not support // lazy loading; but in the absence of capabilities framework we assume // it does. - if (r->memberNames().size() >= r->joinedCount()) { + if (r->users().size() >= r->joinedCount()) { clog << "Lazy loading doesn't seem to be enabled" << endl; FAIL_TEST(); } r->setDisplayed(); connect(r, &Room::allMembersLoaded, this, [this, thisTest, r] { - FINISH_TEST(r->memberNames().size() >= r->joinedCount()); + FINISH_TEST(r->users().size() >= r->joinedCount()); }); return false; } -- cgit v1.2.3 From 8fc5de0529458851a4cd5c042b2b2f2543068c22 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Thu, 7 Jan 2021 20:16:59 +0100 Subject: Prefer connecting to BaseJob::result(), not finished() ...because finished() includes abandoning and should only be relevant when lifecycle issues are involved. (cherry picked from commit 90d41b697af39253483d9bcca4e57b11d2197112) --- lib/connection.cpp | 2 +- tests/quotest.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'tests') diff --git a/lib/connection.cpp b/lib/connection.cpp index 42b17570..9afdfe7e 100644 --- a/lib/connection.cpp +++ b/lib/connection.cpp @@ -1555,7 +1555,7 @@ void Connection::setHomeserver(const QUrl& url) // Whenever a homeserver is updated, retrieve available login flows from it d->loginFlowsJob = callApi(BackgroundRequest); - connect(d->loginFlowsJob, &BaseJob::finished, this, [this] { + connect(d->loginFlowsJob, &BaseJob::result, this, [this] { if (d->loginFlowsJob->status().good()) d->loginFlows = d->loginFlowsJob->flows(); else diff --git a/tests/quotest.cpp b/tests/quotest.cpp index a0bad753..7ab7365f 100644 --- a/tests/quotest.cpp +++ b/tests/quotest.cpp @@ -828,7 +828,7 @@ void TestManager::conclude() // .then(this, &TestManager::finalize); // Qt-style or // .then([this] { finalize(); }); // STL-style auto* job = room->leaveRoom(); - connect(job, &BaseJob::finished, this, [this, job,plainReport] { + connect(job, &BaseJob::result, this, [this, job,plainReport] { Q_ASSERT(job->status().good()); finalize(); // Still flying, as the exit() connection in finalize() is queued -- cgit v1.2.3 From 70846cf880c2b2e6dc9aa225aa3fb0e86ca04568 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 8 Jan 2021 07:41:37 +0100 Subject: quotest: use the target room for loadMembers test Now that we've crowded it with a few synthetic users, lazy-loading of members doesn't some other room to get tested. Bonus: Connection::roomByAlias() has its own very simple test now. (cherry picked from commit d09383d5dc7379c534860b5a66467a32d6adc932) --- tests/quotest.cpp | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'tests') diff --git a/tests/quotest.cpp b/tests/quotest.cpp index 7ab7365f..98c01cfc 100644 --- a/tests/quotest.cpp +++ b/tests/quotest.cpp @@ -91,6 +91,7 @@ public slots: void doTest(const QByteArray& testName); private slots: + TEST_DECL(findRoomByAlias) TEST_DECL(loadMembers) TEST_DECL(sendMessage) TEST_DECL(sendReaction) @@ -306,26 +307,25 @@ void TestManager::doTests() }); } +TEST_IMPL(findRoomByAlias) +{ + auto* roomByAlias = connection()->roomByAlias(targetRoom->canonicalAlias(), + JoinState::Join); + FINISH_TEST(roomByAlias == targetRoom); +} + TEST_IMPL(loadMembers) { - // Trying to load members from another (larger) room - const auto& testRoomAlias = QStringLiteral("#test:matrix.org"); - auto* r = connection()->roomByAlias(testRoomAlias, JoinState::Join); - if (!r) { - clog << testRoomAlias.toStdString() - << " is not found in the test user's rooms" << endl; - FAIL_TEST(); - } // It's not exactly correct because an arbitrary server might not support // lazy loading; but in the absence of capabilities framework we assume // it does. - if (r->users().size() >= r->joinedCount()) { + if (targetRoom->users().size() >= targetRoom->joinedCount()) { clog << "Lazy loading doesn't seem to be enabled" << endl; FAIL_TEST(); } - r->setDisplayed(); - connect(r, &Room::allMembersLoaded, this, [this, thisTest, r] { - FINISH_TEST(r->users().size() >= r->joinedCount()); + targetRoom->setDisplayed(); + connect(targetRoom, &Room::allMembersLoaded, this, [this, thisTest] { + FINISH_TEST(targetRoom->users().size() >= targetRoom->joinedCount()); }); return false; } -- cgit v1.2.3