From dcbb2cfd0e238f788105d7d249f8aac6ad0823e4 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Fri, 11 Jun 2021 17:46:58 +0200 Subject: CMakeLists: require at least Qt 5.12; add Qt 6 support --- quotest/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'quotest') diff --git a/quotest/CMakeLists.txt b/quotest/CMakeLists.txt index 29c53fae..bf9af796 100644 --- a/quotest/CMakeLists.txt +++ b/quotest/CMakeLists.txt @@ -5,7 +5,7 @@ set(quotest_SRCS quotest.cpp) add_executable(quotest ${quotest_SRCS}) -target_link_libraries(quotest PRIVATE Qt5::Core Qt5::Test ${PROJECT_NAME}) +target_link_libraries(quotest PRIVATE ${Qt}::Core ${Qt}::Test ${PROJECT_NAME}) option(${PROJECT_NAME}_INSTALL_TESTS "install quotest (former qmc-example) application" ON) add_feature_info(InstallQuotest ${PROJECT_NAME}_INSTALL_TESTS -- cgit v1.2.3 From 004ebf8d5ba095ca1b11e30d86cedc2ff8c0cfe7 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 18 Jul 2021 18:22:28 +0200 Subject: Room::postFile(): adjust to the changed RoomMessageEvent API 9a5fa623 dropped one of RoomMessageEvent constructors for Qt 6 in order to address #483 - breaking the build with Qt 6 along the way, as Room::postFile() relied on that constructor. This commit changes Room::postFile() in turn, deprecating the current signature and adding a new one that accepts an EventContent object rather than a path to a file. In order to achieve that, FileInfo and ImageInfo classes have gained new constructors that accept QFileInfo instead of the legacy series of parameters, streamlining usage of EventContent structures. --- lib/events/eventcontent.cpp | 51 +++++++++++++++------ lib/events/eventcontent.h | 23 ++++++---- lib/events/roomavatarevent.h | 4 +- lib/room.cpp | 105 ++++++++++++++++++++++++++----------------- lib/room.h | 5 +++ quotest/quotest.cpp | 9 ++-- 6 files changed, 128 insertions(+), 69 deletions(-) (limited to 'quotest') diff --git a/lib/events/eventcontent.cpp b/lib/events/eventcontent.cpp index b249b160..1f28f195 100644 --- a/lib/events/eventcontent.cpp +++ b/lib/events/eventcontent.cpp @@ -5,10 +5,13 @@ #include "converters.h" #include "util.h" +#include "logging.h" #include +#include using namespace Quotient::EventContent; +using std::move; QJsonObject Base::toJson() const { @@ -17,22 +20,37 @@ QJsonObject Base::toJson() const return o; } -FileInfo::FileInfo(const QUrl& u, qint64 payloadSize, const QMimeType& mimeType, - const QString& originalFilename) +FileInfo::FileInfo(const QFileInfo &fi) + : mimeType(QMimeDatabase().mimeTypeForFile(fi)) + , url(QUrl::fromLocalFile(fi.filePath())) + , payloadSize(fi.size()) + , originalName(fi.fileName()) +{ + Q_ASSERT(fi.isFile()); +} + +FileInfo::FileInfo(QUrl u, qint64 payloadSize, const QMimeType& mimeType, + QString originalFilename) : mimeType(mimeType) - , url(u) + , url(move(u)) , payloadSize(payloadSize) - , originalName(originalFilename) -{} + , originalName(move(originalFilename)) +{ + if (!isValid()) + qCWarning(MESSAGES) + << "To client developers: using FileInfo(QUrl, qint64, ...) " + "constructor for non-mxc resources is deprecated since Quotient " + "0.7; for local resources, use FileInfo(QFileInfo) instead"; +} -FileInfo::FileInfo(const QUrl& u, const QJsonObject& infoJson, - const QString& originalFilename) +FileInfo::FileInfo(QUrl mxcUrl, const QJsonObject& infoJson, + QString originalFilename) : originalInfoJson(infoJson) , mimeType( QMimeDatabase().mimeTypeForName(infoJson["mimetype"_ls].toString())) - , url(u) + , url(move(mxcUrl)) , payloadSize(fromJson(infoJson["size"_ls])) - , originalName(originalFilename) + , originalName(move(originalFilename)) { if (!mimeType.isValid()) mimeType = QMimeDatabase().mimeTypeForData(QByteArray()); @@ -53,14 +71,19 @@ void FileInfo::fillInfoJson(QJsonObject* infoJson) const infoJson->insert(QStringLiteral("mimetype"), mimeType.name()); } -ImageInfo::ImageInfo(const QUrl& u, qint64 fileSize, QMimeType mimeType, - const QSize& imageSize, const QString& originalFilename) - : FileInfo(u, fileSize, mimeType, originalFilename), imageSize(imageSize) +ImageInfo::ImageInfo(const QFileInfo& fi, QSize imageSize) + : FileInfo(fi), imageSize(imageSize) +{} + +ImageInfo::ImageInfo(const QUrl& mxcUrl, qint64 fileSize, const QMimeType& type, + QSize imageSize, const QString& originalFilename) + : FileInfo(mxcUrl, fileSize, type, originalFilename) + , imageSize(imageSize) {} -ImageInfo::ImageInfo(const QUrl& u, const QJsonObject& infoJson, +ImageInfo::ImageInfo(const QUrl& mxcUrl, const QJsonObject& infoJson, const QString& originalFilename) - : FileInfo(u, infoJson, originalFilename) + : FileInfo(mxcUrl, infoJson, originalFilename) , imageSize(infoJson["w"_ls].toInt(), infoJson["h"_ls].toInt()) {} diff --git a/lib/events/eventcontent.h b/lib/events/eventcontent.h index 60d1f7b7..78c5b287 100644 --- a/lib/events/eventcontent.h +++ b/lib/events/eventcontent.h @@ -10,7 +10,8 @@ #include #include #include -#include + +class QFileInfo; namespace Quotient { namespace EventContent { @@ -73,11 +74,13 @@ namespace EventContent { */ class FileInfo { public: - explicit FileInfo(const QUrl& u, qint64 payloadSize = -1, + FileInfo() = default; + explicit FileInfo(const QFileInfo& fi); + explicit FileInfo(QUrl mxcUrl, qint64 payloadSize = -1, const QMimeType& mimeType = {}, - const QString& originalFilename = {}); - FileInfo(const QUrl& u, const QJsonObject& infoJson, - const QString& originalFilename = {}); + QString originalFilename = {}); + FileInfo(QUrl mxcUrl, const QJsonObject& infoJson, + QString originalFilename = {}); bool isValid() const; @@ -113,10 +116,12 @@ namespace EventContent { */ class ImageInfo : public FileInfo { public: - explicit ImageInfo(const QUrl& u, qint64 fileSize = -1, - QMimeType mimeType = {}, const QSize& imageSize = {}, + ImageInfo() = default; + explicit ImageInfo(const QFileInfo& fi, QSize imageSize = {}); + explicit ImageInfo(const QUrl& mxcUrl, qint64 fileSize = -1, + const QMimeType& type = {}, QSize imageSize = {}, const QString& originalFilename = {}); - ImageInfo(const QUrl& u, const QJsonObject& infoJson, + ImageInfo(const QUrl& mxcUrl, const QJsonObject& infoJson, const QString& originalFilename = {}); void fillInfoJson(QJsonObject* infoJson) const; @@ -134,7 +139,7 @@ namespace EventContent { */ class Thumbnail : public ImageInfo { public: - Thumbnail() : ImageInfo(QUrl()) {} // To allow empty thumbnails + Thumbnail() = default; // Allow empty thumbnails Thumbnail(const QJsonObject& infoJson); Thumbnail(const ImageInfo& info) : ImageInfo(info) {} using ImageInfo::ImageInfo; diff --git a/lib/events/roomavatarevent.h b/lib/events/roomavatarevent.h index a4257895..3fa11a0f 100644 --- a/lib/events/roomavatarevent.h +++ b/lib/events/roomavatarevent.h @@ -20,12 +20,12 @@ public: : StateEvent(typeId(), matrixTypeId(), QString(), avatar) {} // A replica of EventContent::ImageInfo constructor - explicit RoomAvatarEvent(const QUrl& u, qint64 fileSize = -1, + explicit RoomAvatarEvent(const QUrl& mxcUrl, qint64 fileSize = -1, QMimeType mimeType = {}, const QSize& imageSize = {}, const QString& originalFilename = {}) : RoomAvatarEvent(EventContent::ImageContent { - u, fileSize, mimeType, imageSize, originalFilename }) + mxcUrl, fileSize, mimeType, imageSize, originalFilename }) {} QUrl url() const { return content().url; } diff --git a/lib/room.cpp b/lib/room.cpp index 0984bd96..6b729b8f 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -306,6 +306,8 @@ public: return sendEvent(makeEvent(std::forward(eventArgs)...)); } + QString doPostFile(RoomEventPtr &&msgEvent, const QUrl &localUrl); + RoomEvent* addAsPending(RoomEventPtr&& event); QString doSendEvent(const RoomEvent* pEvent); @@ -1756,57 +1758,80 @@ QString Room::postReaction(const QString& eventId, const QString& key) return d->sendEvent(EventRelation::annotate(eventId, key)); } -QString Room::postFile(const QString& plainText, const QUrl& localPath, - bool asGenericFile) +QString Room::Private::doPostFile(RoomEventPtr&& msgEvent, const QUrl& localUrl) { - QFileInfo localFile { localPath.toLocalFile() }; - Q_ASSERT(localFile.isFile()); - - const auto txnId = - d->addAsPending( - makeEvent(plainText, localFile, asGenericFile)) - ->transactionId(); + const auto txnId = addAsPending(move(msgEvent))->transactionId(); // Remote URL will only be known after upload; fill in the local path // to enable the preview while the event is pending. - uploadFile(txnId, localPath); + q->uploadFile(txnId, localUrl); // Below, the upload job is used as a context object to clean up connections - 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); - if (it != d->unsyncedEvents.end()) { - it->setFileUploaded(mxcUri); - emit pendingEventChanged( - int(it - d->unsyncedEvents.begin())); - d->doSendEvent(it->get()); - } else { - // Normally in this situation we should instruct - // the media server to delete the file; alas, there's no - // API specced for that. - qCWarning(MAIN) << "File uploaded to" << mxcUri - << "but the event referring to it was " - "cancelled"; - } + const auto& transferJob = fileTransfers.value(txnId).job; + connect(q, &Room::fileTransferCompleted, transferJob, + [this, txnId](const QString& tId, const QUrl&, const QUrl& mxcUri) { + if (tId != txnId) + return; + + const auto it = q->findPendingEvent(txnId); + if (it != unsyncedEvents.end()) { + it->setFileUploaded(mxcUri); + emit q->pendingEventChanged( + int(it - unsyncedEvents.begin())); + doSendEvent(it->get()); + } else { + // Normally in this situation we should instruct + // the media server to delete the file; alas, there's no + // API specced for that. + qCWarning(MAIN) << "File uploaded to" << mxcUri + << "but the event referring to it was " + "cancelled"; } }); - connect(this, &Room::fileTransferCancelled, transferJob, - [this, txnId](const QString& id) { - if (id == txnId) { - auto it = findPendingEvent(txnId); - if (it != d->unsyncedEvents.end()) { - const auto idx = int(it - d->unsyncedEvents.begin()); - emit pendingEventAboutToDiscard(idx); - // See #286 on why iterator may not be valid here. - d->unsyncedEvents.erase(d->unsyncedEvents.begin() + idx); - emit pendingEventDiscarded(); - } - } + connect(q, &Room::fileTransferCancelled, transferJob, + [this, txnId](const QString& tId) { + if (tId != txnId) + return; + + const auto it = q->findPendingEvent(txnId); + if (it == unsyncedEvents.end()) + return; + + const auto idx = int(it - unsyncedEvents.begin()); + emit q->pendingEventAboutToDiscard(idx); + // See #286 on why `it` may not be valid here. + unsyncedEvents.erase(unsyncedEvents.begin() + idx); + emit q->pendingEventDiscarded(); }); return txnId; } +QString Room::postFile(const QString& plainText, + EventContent::TypedBase* content) +{ + Q_ASSERT(content != nullptr && content->fileInfo() != nullptr); + const auto* const fileInfo = content->fileInfo(); + Q_ASSERT(fileInfo != nullptr); + QFileInfo localFile { fileInfo->url.toLocalFile() }; + Q_ASSERT(localFile.isFile()); + + return d->doPostFile( + makeEvent( + plainText, RoomMessageEvent::rawMsgTypeForFile(localFile), content), + fileInfo->url); +} + +#if QT_VERSION_MAJOR < 6 +QString Room::postFile(const QString& plainText, const QUrl& localPath, + bool asGenericFile) +{ + QFileInfo localFile { localPath.toLocalFile() }; + Q_ASSERT(localFile.isFile()); + return d->doPostFile(makeEvent(plainText, localFile, + asGenericFile), + localPath); +} +#endif + QString Room::postEvent(RoomEvent* event) { return d->sendEvent(RoomEventPtr(event)); diff --git a/lib/room.h b/lib/room.h index 26d0121e..d71bff9c 100644 --- a/lib/room.h +++ b/lib/room.h @@ -549,8 +549,13 @@ public Q_SLOTS: QString postHtmlText(const QString& plainText, const QString& html); /// Send a reaction on a given event with a given key QString postReaction(const QString& eventId, const QString& key); + + QString postFile(const QString& plainText, EventContent::TypedBase* content); +#if QT_VERSION_MAJOR < 6 + /// \deprecated Use postFile(QString, MessageEventType, EventContent) instead QString postFile(const QString& plainText, const QUrl& localPath, bool asGenericFile = false); +#endif /** Post a pre-created room message event * * Takes ownership of the event, deleting it once the matching one diff --git a/quotest/quotest.cpp b/quotest/quotest.cpp index a0914c72..5646d54d 100644 --- a/quotest/quotest.cpp +++ b/quotest/quotest.cpp @@ -397,15 +397,16 @@ TEST_IMPL(sendFile) } tf->write("Test"); tf->close(); + QFileInfo tfi { *tf }; // QFileInfo::fileName brings only the file name; QFile::fileName brings // the full path - const auto tfName = QFileInfo(*tf).fileName(); + const auto tfName = tfi.fileName(); clog << "Sending file " << tfName.toStdString() << endl; - const auto txnId = - targetRoom->postFile("Test file", QUrl::fromLocalFile(tf->fileName())); + const auto txnId = targetRoom->postFile( + "Test file", new EventContent::FileContent(tfi)); if (!validatePendingEvent(txnId)) { clog << "Invalid pending event right after submitting" << endl; - delete tf; + tf->deleteLater(); FAIL_TEST(); } -- cgit v1.2.3 From 7ee1681d7640b7e7683f7bb40bf768704a48832c Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Fri, 30 Jul 2021 08:22:13 +0200 Subject: Clean up after the previous commit RoomAliasesEvent is no more even registered (meaning that the library will load m.room.aliases as unknown state events); quotest code updated to use historyEdge() instead of timelineEdge(). --- lib/events/simplestateevents.h | 1 - quotest/quotest.cpp | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) (limited to 'quotest') diff --git a/lib/events/simplestateevents.h b/lib/events/simplestateevents.h index 3bac54e6..cf1bfbba 100644 --- a/lib/events/simplestateevents.h +++ b/lib/events/simplestateevents.h @@ -72,5 +72,4 @@ public: QString server() const { return stateKey(); } QStringList aliases() const { return content().value; } }; -REGISTER_EVENT_TYPE(RoomAliasesEvent) } // namespace Quotient diff --git a/quotest/quotest.cpp b/quotest/quotest.cpp index 5646d54d..ec7d4dcb 100644 --- a/quotest/quotest.cpp +++ b/quotest/quotest.cpp @@ -557,7 +557,7 @@ bool TestSuite::checkRedactionOutcome(const QByteArray& thisTest, // redacted at the next sync, or the nearest sync completes with // the unredacted event but the next one brings redaction. auto it = targetRoom->findInTimeline(evtIdToRedact); - if (it == targetRoom->timelineEdge()) + if (it == targetRoom->historyEdge()) return false; // Waiting for the next sync if ((*it)->isRedacted()) { -- cgit v1.2.3 From 693b5da2920f173a9e3f723b845d35a7b4aa9823 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 12 Sep 2021 04:35:43 +0200 Subject: Add a download test to quotest --- quotest/CMakeLists.txt | 3 ++- quotest/quotest.cpp | 37 +++++++++++++++++++++++++++++++------ 2 files changed, 33 insertions(+), 7 deletions(-) (limited to 'quotest') diff --git a/quotest/CMakeLists.txt b/quotest/CMakeLists.txt index bf9af796..59334e30 100644 --- a/quotest/CMakeLists.txt +++ b/quotest/CMakeLists.txt @@ -4,8 +4,9 @@ set(quotest_SRCS quotest.cpp) +find_package(${Qt} COMPONENTS Concurrent) add_executable(quotest ${quotest_SRCS}) -target_link_libraries(quotest PRIVATE ${Qt}::Core ${Qt}::Test ${PROJECT_NAME}) +target_link_libraries(quotest PRIVATE ${Qt}::Core ${Qt}::Test ${Qt}::Concurrent ${PROJECT_NAME}) option(${PROJECT_NAME}_INSTALL_TESTS "install quotest (former qmc-example) application" ON) add_feature_info(InstallQuotest ${PROJECT_NAME}_INSTALL_TESTS diff --git a/quotest/quotest.cpp b/quotest/quotest.cpp index ec7d4dcb..3f886676 100644 --- a/quotest/quotest.cpp +++ b/quotest/quotest.cpp @@ -5,6 +5,7 @@ #include "room.h" #include "user.h" #include "uriresolver.h" +#include "networkaccessmanager.h" #include "csapi/joining.h" #include "csapi/leaving.h" @@ -20,6 +21,8 @@ #include #include #include +#include +#include #include #include @@ -435,6 +438,27 @@ TEST_IMPL(sendFile) return false; } +bool testDownload(const QUrl& url) +{ + // Move out actual test from the multithreaded code + // to help debugging + auto results = + QtConcurrent::blockingMapped(QVector { 1, 2, 3 }, [url](int) { + QEventLoop el; + auto reply = + NetworkAccessManager::instance()->get(QNetworkRequest(url)); + QObject::connect( + reply, &QNetworkReply::finished, &el, [&el] { el.exit(); }, + Qt::QueuedConnection); + el.exec(); + return reply->error(); + }); + return std::all_of(results.cbegin(), results.cend(), + [](QNetworkReply::NetworkError ne) { + return ne == QNetworkReply::NoError; + }); +} + bool TestSuite::checkFileSendingOutcome(const TestToken& thisTest, const QString& txnId, const QString& fileName) @@ -465,14 +489,15 @@ bool TestSuite::checkFileSendingOutcome(const TestToken& thisTest, return visit( *evt, [&](const RoomMessageEvent& e) { - // TODO: actually try to download it to check, e.g., #366 - // (and #368 would help to test against bad file names). + // TODO: check #366 once #368 is implemented FINISH_TEST( !e.id().isEmpty() - && pendingEvents[size_t(pendingIdx)]->transactionId() - == txnId - && e.hasFileContent() - && e.content()->fileInfo()->originalName == fileName); + && pendingEvents[size_t(pendingIdx)]->transactionId() + == txnId + && e.hasFileContent() + && e.content()->fileInfo()->originalName == fileName + && testDownload(targetRoom->connection()->makeMediaUrl( + e.content()->fileInfo()->url))); }, [this, thisTest](const RoomEvent&) { FAIL_TEST(); }); }); -- cgit v1.2.3 From 167509514587aa22d837b42a8d30d7c1128e0a45 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 12 Sep 2021 04:52:41 +0200 Subject: Fix building with older Qt --- quotest/quotest.cpp | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) (limited to 'quotest') diff --git a/quotest/quotest.cpp b/quotest/quotest.cpp index 3f886676..31a0b6d6 100644 --- a/quotest/quotest.cpp +++ b/quotest/quotest.cpp @@ -438,21 +438,30 @@ TEST_IMPL(sendFile) return false; } +// Can be replaced with a lambda once QtConcurrent is able to resolve return +// types from lambda invocations (Qt 6 can, not sure about earlier) +struct DownloadRunner { + QUrl url; + + using result_type = QNetworkReply::NetworkError; + + QNetworkReply::NetworkError operator()(int) const { + QEventLoop el; + auto reply = NetworkAccessManager::instance()->get(QNetworkRequest(url)); + QObject::connect( + reply, &QNetworkReply::finished, &el, [&el] { el.exit(); }, + Qt::QueuedConnection); + el.exec(); + return reply->error(); + } +}; + bool testDownload(const QUrl& url) { // Move out actual test from the multithreaded code // to help debugging - auto results = - QtConcurrent::blockingMapped(QVector { 1, 2, 3 }, [url](int) { - QEventLoop el; - auto reply = - NetworkAccessManager::instance()->get(QNetworkRequest(url)); - QObject::connect( - reply, &QNetworkReply::finished, &el, [&el] { el.exit(); }, - Qt::QueuedConnection); - el.exec(); - return reply->error(); - }); + auto results = QtConcurrent::blockingMapped(QVector { 1, 2, 3 }, + DownloadRunner { url }); return std::all_of(results.cbegin(), results.cend(), [](QNetworkReply::NetworkError ne) { return ne == QNetworkReply::NoError; -- cgit v1.2.3 From bcaab611840a0a2ad284e6f1e7c2f0b4de10222d Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 12 Sep 2021 05:22:53 +0200 Subject: Fix a memory leak in DownloadRunner --- quotest/quotest.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'quotest') diff --git a/quotest/quotest.cpp b/quotest/quotest.cpp index 31a0b6d6..4142c718 100644 --- a/quotest/quotest.cpp +++ b/quotest/quotest.cpp @@ -445,11 +445,14 @@ struct DownloadRunner { using result_type = QNetworkReply::NetworkError; - QNetworkReply::NetworkError operator()(int) const { + QNetworkReply::NetworkError operator()(int) const + { QEventLoop el; - auto reply = NetworkAccessManager::instance()->get(QNetworkRequest(url)); + QScopedPointer reply { + NetworkAccessManager::instance()->get(QNetworkRequest(url)) + }; QObject::connect( - reply, &QNetworkReply::finished, &el, [&el] { el.exit(); }, + reply.data(), &QNetworkReply::finished, &el, [&el] { el.exit(); }, Qt::QueuedConnection); el.exec(); return reply->error(); -- cgit v1.2.3 From 9f43d34c590a825504b72be7f6b238d0ff2c915a Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 12 Sep 2021 04:39:44 +0200 Subject: Use C++ instead of commenting --- quotest/quotest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'quotest') diff --git a/quotest/quotest.cpp b/quotest/quotest.cpp index 4142c718..3a77eb01 100644 --- a/quotest/quotest.cpp +++ b/quotest/quotest.cpp @@ -51,7 +51,7 @@ private: QByteArrayList running {}, succeeded {}, failed {}; }; -using TestToken = QByteArray; // return value of QMetaMethod::name +using TestToken = decltype(std::declval().name()); Q_DECLARE_METATYPE(TestToken) // For now, the token itself is the test name but that may change. -- cgit v1.2.3 From 062d534a0024959117e310f8c2a964434acb9fa0 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Mon, 4 Oct 2021 09:53:33 +0200 Subject: Add tests for prettyPrint() --- quotest/quotest.cpp | 49 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) (limited to 'quotest') diff --git a/quotest/quotest.cpp b/quotest/quotest.cpp index 3a77eb01..44d82adf 100644 --- a/quotest/quotest.cpp +++ b/quotest/quotest.cpp @@ -108,6 +108,7 @@ private slots: TEST_DECL(addAndRemoveTag) TEST_DECL(markDirectChat) TEST_DECL(visitResources) + TEST_DECL(prettyPrintTests) // Add more tests above here public: @@ -140,7 +141,7 @@ private: // connectUntil() to break the QMetaObject::Connection upon finishing the test // item. #define FINISH_TEST(Condition) \ - return (finishTest(thisTest, Condition, __FILE__, __LINE__), true) + return (finishTest(thisTest, (Condition), __FILE__, __LINE__), true) #define FAIL_TEST() FINISH_TEST(false) @@ -824,6 +825,52 @@ TEST_IMPL(visitResources) FINISH_TEST(true); } +bool checkPrettyPrint( + std::initializer_list> tests) +{ + bool result = true; + for (const auto& [test, etalon] : tests) { + const auto is = prettyPrint(test).toStdString(); + const auto shouldBe = std::string("") + + etalon + ""; + if (is == shouldBe) + continue; + clog << is << " != " << shouldBe << endl; + result = false; + } + return result; +} + +TEST_IMPL(prettyPrintTests) +{ + const bool prettyPrintTestResult = checkPrettyPrint( + { { "https://www.matrix.org", + R"(https://www.matrix.org)" }, +// { "www.matrix.org", // Doesn't work yet +// R"(www.matrix.org)" }, + { "smb://somewhere/file", "smb://somewhere/file" }, // Disallowed scheme + { "https:/something", "https:/something" }, // Malformed URL + { "https://matrix.to/#/!roomid:example.org", + R"(https://matrix.to/#/!roomid:example.org)" }, + { "https://matrix.to/#/@user_id:example.org", + R"(https://matrix.to/#/@user_id:example.org)" }, + { "https://matrix.to/#/#roomalias:example.org", + R"(https://matrix.to/#/#roomalias:example.org)" }, + { "https://matrix.to/#/##ircroomalias:example.org", + R"(https://matrix.to/#/##ircroomalias:example.org)" }, + { "me@example.org", + R"(me@example.org)" }, + { "mailto:me@example.org", + R"(mailto:me@example.org)" }, + { "!room_id:example.org", + R"(!room_id:example.org)" }, + { "@user_id:example.org", + R"(@user_id:example.org)" }, + { "#room_alias:example.org", + R"(#room_alias:example.org)" } }); + FINISH_TEST(prettyPrintTestResult); +} + void TestManager::conclude() { // Clean up the room (best effort) -- cgit v1.2.3 From 7b516cdf0b987e542b1e4cd4556ecb2bfbde3ff9 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Tue, 5 Oct 2021 03:52:19 +0200 Subject: Quotest: return non-zero when things go really wrong ...such as stuck login or failure to join the room. Closes #496. --- quotest/quotest.cpp | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) (limited to 'quotest') diff --git a/quotest/quotest.cpp b/quotest/quotest.cpp index 44d82adf..d006c7fb 100644 --- a/quotest/quotest.cpp +++ b/quotest/quotest.cpp @@ -245,7 +245,7 @@ void TestManager::setupAndRun() clog << "Sync " << ++i << " complete" << endl; if (auto* r = testSuite->room()) { clog << "Test room timeline size = " << r->timelineSize(); - if (r->pendingEvents().empty()) + if (!r->pendingEvents().empty()) clog << ", pending size = " << r->pendingEvents().size(); clog << endl; } @@ -939,10 +939,22 @@ void TestManager::conclude() void TestManager::finalize() { + if (!c->isUsable() || !c->isLoggedIn()) { + clog << "No usable connection reached" << endl; + QCoreApplication::exit(-2); + return; // NB: QCoreApplication::exit() does return to the caller + } clog << "Logging out" << endl; c->logout(); - connect(c, &Connection::loggedOut, this, - [this] { QCoreApplication::exit(failed.size() + running.size()); }, + connect( + c, &Connection::loggedOut, this, + [this] { + QCoreApplication::exit(!testSuite ? -3 + : succeeded.empty() && failed.empty() + && running.empty() + ? -4 + : failed.size() + running.size()); + }, Qt::QueuedConnection); } -- cgit v1.2.3 From ae0ad49f36e8ba5983839581302ed16ddbd75d5f Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Thu, 2 Dec 2021 14:04:49 +0100 Subject: visit(Event, ...) -> switchOnType() It has not much to do with the Visitor design pattern; also, std::visit() has different conventions on the order of parameters. --- lib/connection.cpp | 4 ++-- lib/events/event.h | 58 ++++++++++++++++++++++++++++++----------------------- lib/room.cpp | 9 +++++---- quotest/quotest.cpp | 4 ++-- 4 files changed, 42 insertions(+), 33 deletions(-) (limited to 'quotest') diff --git a/lib/connection.cpp b/lib/connection.cpp index e65fdac4..25219def 100644 --- a/lib/connection.cpp +++ b/lib/connection.cpp @@ -659,7 +659,7 @@ void Connection::Private::consumeAccountData(Events&& accountDataEvents) // After running this loop, the account data events not saved in // accountData (see the end of the loop body) are auto-cleaned away for (auto&& eventPtr: accountDataEvents) { - visit(*eventPtr, + switchOnType(*eventPtr, [this](const DirectChatEvent& dce) { // https://github.com/quotient-im/libQuotient/wiki/Handling-direct-chat-events const auto& usersToDCs = dce.usersToDirectChats(); @@ -760,7 +760,7 @@ void Connection::Private::consumeToDeviceEvents(Events&& toDeviceEvents) << ee.senderId(); // encryptionManager->updateDeviceKeys(); - visit(*sessionDecryptMessage(ee), + switchOnType(*sessionDecryptMessage(ee), [this, senderKey = ee.senderKey()](const RoomKeyEvent& roomKeyEvent) { if (auto* detectedRoom = q->room(roomKeyEvent.roomId())) detectedRoom->handleRoomKeyEvent(roomKeyEvent, senderKey); diff --git a/lib/events/event.h b/lib/events/event.h index 998a386c..ce737280 100644 --- a/lib/events/event.h +++ b/lib/events/event.h @@ -290,7 +290,7 @@ using Events = EventsArray; } \ // End of macro -// === is<>(), eventCast<>() and visit<>() === +// === is<>(), eventCast<>() and switchOnType<>() === template inline bool is(const Event& e) @@ -312,12 +312,12 @@ inline auto eventCast(const BasePtrT& eptr) : nullptr; } -// A single generic catch-all visitor +// A trivial generic catch-all "switch" template -inline auto visit(const BaseEventT& event, FnT&& visitor) - -> decltype(visitor(event)) +inline auto switchOnType(const BaseEventT& event, FnT&& fn) + -> decltype(fn(event)) { - return visitor(event); + return fn(event); } namespace _impl { @@ -328,52 +328,60 @@ namespace _impl { && !std::is_same_v>>; } -// A single type-specific void visitor +// A trivial type-specific "switch" for a void function template -inline auto visit(const BaseT& event, FnT&& visitor) +inline auto switchOnType(const BaseT& event, FnT&& fn) -> std::enable_if_t<_impl::needs_downcast && std::is_void_v>> { using event_type = fn_arg_t; if (is>(event)) - visitor(static_cast(event)); + fn(static_cast(event)); } -// A single type-specific non-void visitor with an optional default value -// non-voidness is guarded by defaultValue type +// A trivial type-specific "switch" for non-void functions with an optional +// default value; non-voidness is guarded by defaultValue type template -inline auto visit(const BaseT& event, FnT&& visitor, - fn_return_t&& defaultValue = {}) +inline auto switchOnType(const BaseT& event, FnT&& fn, + fn_return_t&& defaultValue = {}) -> std::enable_if_t<_impl::needs_downcast, fn_return_t> { using event_type = fn_arg_t; if (is>(event)) - return visitor(static_cast(event)); - return std::forward>(defaultValue); + return fn(static_cast(event)); + return std::move(defaultValue); } -// A chain of 2 or more visitors +// A switch for a chain of 2 or more functions template -inline std::common_type_t, fn_return_t> visit( - const BaseT& event, FnT1&& visitor1, FnT2&& visitor2, - FnTs&&... visitors) +inline std::common_type_t, fn_return_t> +switchOnType(const BaseT& event, FnT1&& fn1, FnT2&& fn2, FnTs&&... fns) { using event_type1 = fn_arg_t; if (is>(event)) - return visitor1(static_cast(event)); - return visit(event, std::forward(visitor2), - std::forward(visitors)...); + return fn1(static_cast(event)); + return switchOnType(event, std::forward(fn2), + std::forward(fns)...); } -// A facility overload that calls void-returning visit() on each event +template +[[deprecated("The new name for visit() is switchOnType()")]] // +inline std::common_type_t...> +visit(const BaseT& event, FnTs&&... fns) +{ + return switchOnType(event, std::forward(fns)...); +} + + // A facility overload that calls void-returning switchOnType() on each event // over a range of event pointers +// TODO: replace with ranges::for_each once all standard libraries have it template -inline auto visitEach(RangeT&& events, FnTs&&... visitors) +inline auto visitEach(RangeT&& events, FnTs&&... fns) -> std::enable_if_t(visitors)...))>> + decltype(switchOnType(**begin(events), std::forward(fns)...))>> { for (auto&& evtPtr: events) - visit(*evtPtr, std::forward(visitors)...); + switchOnType(*evtPtr, std::forward(fns)...); } } // namespace Quotient Q_DECLARE_METATYPE(Quotient::Event*) diff --git a/lib/room.cpp b/lib/room.cpp index 4d60570d..b3438e08 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -2779,7 +2779,7 @@ Room::Changes Room::processStateEvent(const RoomEvent& e) auto& curStateEvent = d->currentState[{ e.matrixType(), e.stateKey() }]; // Prepare for the state change // clang-format off - const bool proceed = visit(e + const bool proceed = switchOnType(e , [this, curStateEvent](const RoomMemberEvent& rme) { // clang-format on auto* oldRme = static_cast(curStateEvent); @@ -2877,7 +2877,7 @@ Room::Changes Room::processStateEvent(const RoomEvent& e) // Update internal structures as per the change and work out the return value // clang-format off - const auto result = visit(e + const auto result = switchOnType(e , [] (const RoomNameEvent&) { return Change::Name; } @@ -2885,7 +2885,7 @@ Room::Changes Room::processStateEvent(const RoomEvent& e) // clang-format on setObjectName(cae.alias().isEmpty() ? d->id : cae.alias()); const auto* oldCae = - static_cast(oldStateEvent); + static_cast(oldStateEvent); QStringList previousAltAliases {}; if (oldCae) { previousAltAliases = oldCae->altAliases(); @@ -2897,7 +2897,8 @@ Room::Changes Room::processStateEvent(const RoomEvent& e) if (!cae.alias().isEmpty()) newAliases.push_front(cae.alias()); - connection()->updateRoomAliases(id(), previousAltAliases, newAliases); + connection()->updateRoomAliases(id(), previousAltAliases, + newAliases); return Change::Aliases; // clang-format off } diff --git a/quotest/quotest.cpp b/quotest/quotest.cpp index d006c7fb..764d5dfd 100644 --- a/quotest/quotest.cpp +++ b/quotest/quotest.cpp @@ -498,8 +498,8 @@ bool TestSuite::checkFileSendingOutcome(const TestToken& thisTest, clog << "File event " << txnId.toStdString() << " arrived in the timeline" << endl; - // This part tests visit() - return visit( + // This part tests switchOnType() + return switchOnType( *evt, [&](const RoomMessageEvent& e) { // TODO: check #366 once #368 is implemented -- cgit v1.2.3 From 7981c654c0cd53f6e100ecae25882246a9de5d6e Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 5 Dec 2021 21:47:25 +0100 Subject: Drop 'qmc-example' from one last(?) place --- quotest/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'quotest') diff --git a/quotest/CMakeLists.txt b/quotest/CMakeLists.txt index 59334e30..8d67b6f1 100644 --- a/quotest/CMakeLists.txt +++ b/quotest/CMakeLists.txt @@ -8,7 +8,7 @@ find_package(${Qt} COMPONENTS Concurrent) add_executable(quotest ${quotest_SRCS}) target_link_libraries(quotest PRIVATE ${Qt}::Core ${Qt}::Test ${Qt}::Concurrent ${PROJECT_NAME}) -option(${PROJECT_NAME}_INSTALL_TESTS "install quotest (former qmc-example) application" ON) +option(${PROJECT_NAME}_INSTALL_TESTS "install quotest application" ON) add_feature_info(InstallQuotest ${PROJECT_NAME}_INSTALL_TESTS "the library functional test suite") -- cgit v1.2.3 From e5256e0b1e4c43ce96d99d1b82ca5d98a1baded6 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Fri, 17 Dec 2021 08:07:07 +0100 Subject: RoomMemberEvent: fix an off-by-one error Also: extended quotest to cover member renames, not just user profile renames. --- lib/events/roommemberevent.cpp | 8 +++----- quotest/quotest.cpp | 35 +++++++++++++++++++++++++++-------- 2 files changed, 30 insertions(+), 13 deletions(-) (limited to 'quotest') diff --git a/lib/events/roommemberevent.cpp b/lib/events/roommemberevent.cpp index b0bc7bcb..3141f6b5 100644 --- a/lib/events/roommemberevent.cpp +++ b/lib/events/roommemberevent.cpp @@ -48,11 +48,9 @@ void MemberEventContent::fillJson(QJsonObject* o) const { Q_ASSERT(o); if (membership != Membership::Invalid) - o->insert( - QStringLiteral("membership"), - MembershipStrings[qCountTrailingZeroBits( - std::underlying_type_t(membership)) - + 1]); + o->insert(QStringLiteral("membership"), + MembershipStrings[qCountTrailingZeroBits( + std::underlying_type_t(membership))]); if (displayName) o->insert(QStringLiteral("displayname"), *displayName); if (avatarUrl && avatarUrl->isValid()) diff --git a/quotest/quotest.cpp b/quotest/quotest.cpp index 764d5dfd..8703efb2 100644 --- a/quotest/quotest.cpp +++ b/quotest/quotest.cpp @@ -214,6 +214,7 @@ TestManager::TestManager(int& argc, char** argv) // Big countdown watchdog QTimer::singleShot(180000, this, [this] { + clog << "Time is up, stopping the session"; if (testSuite) conclude(); else @@ -537,14 +538,32 @@ TEST_IMPL(setTopic) TEST_IMPL(changeName) { - auto* const localUser = connection()->user(); - const auto& newName = connection()->generateTxnId(); // See setTopic() - clog << "Renaming the user to " << newName.toStdString() << endl; - localUser->rename(newName); - connectUntil(localUser, &User::defaultNameChanged, this, - [this, thisTest, localUser, newName] { - FINISH_TEST(localUser->name() == newName); - }); + connectSingleShot(targetRoom, &Room::allMembersLoaded, this, [this, thisTest] { + auto* const localUser = connection()->user(); + const auto& newName = connection()->generateTxnId(); // See setTopic() + clog << "Renaming the user to " << newName.toStdString() + << " in the target room" << endl; + localUser->rename(newName, targetRoom); + connectUntil(targetRoom, &Room::memberRenamed, this, + [this, thisTest, localUser, newName](const User* u) { + if (localUser != u) + return false; + if (localUser->name(targetRoom) != newName) + FAIL_TEST(); + + clog + << "Member rename successful, renaming the account" + << endl; + const auto newN = newName.mid(0, 5); + localUser->rename(newN); + connectUntil(localUser, &User::defaultNameChanged, + this, [this, thisTest, localUser, newN] { + targetRoom->localUser()->rename({}); + FINISH_TEST(localUser->name() == newN); + }); + return true; + }); + }); return false; } -- cgit v1.2.3 From 24a562cc64e8fac6c55108ae2b7b6997ecdd2010 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 19 Dec 2021 10:43:33 +0100 Subject: Quotest: add a missing \n in the output --- quotest/quotest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'quotest') diff --git a/quotest/quotest.cpp b/quotest/quotest.cpp index 8703efb2..7bd9c5c3 100644 --- a/quotest/quotest.cpp +++ b/quotest/quotest.cpp @@ -214,7 +214,7 @@ TestManager::TestManager(int& argc, char** argv) // Big countdown watchdog QTimer::singleShot(180000, this, [this] { - clog << "Time is up, stopping the session"; + clog << "Time is up, stopping the session\n"; if (testSuite) conclude(); else -- cgit v1.2.3 From 53c494f1b9f273395caade35c93e3fb520d083ec Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Mon, 27 Dec 2021 20:15:47 +0100 Subject: Quotest: add compile warnings from libQuotient --- quotest/CMakeLists.txt | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'quotest') diff --git a/quotest/CMakeLists.txt b/quotest/CMakeLists.txt index 8d67b6f1..cb41141d 100644 --- a/quotest/CMakeLists.txt +++ b/quotest/CMakeLists.txt @@ -8,6 +8,21 @@ find_package(${Qt} COMPONENTS Concurrent) add_executable(quotest ${quotest_SRCS}) target_link_libraries(quotest PRIVATE ${Qt}::Core ${Qt}::Test ${Qt}::Concurrent ${PROJECT_NAME}) +if (MSVC) + target_compile_options(quotest PUBLIC /EHsc /W4 + /wd4100 /wd4127 /wd4242 /wd4244 /wd4245 /wd4267 /wd4365 /wd4456 /wd4459 + /wd4464 /wd4505 /wd4514 /wd4571 /wd4619 /wd4623 /wd4625 /wd4626 /wd4706 + /wd4710 /wd4774 /wd4820 /wd4946 /wd5026 /wd5027) +else() + foreach (FLAG W Wall Wpedantic Wextra Wno-unused-parameter Werror=return-type) + CHECK_CXX_COMPILER_FLAG("-${FLAG}" COMPILER_${FLAG}_SUPPORTED) + if (COMPILER_${FLAG}_SUPPORTED AND + NOT CMAKE_CXX_FLAGS MATCHES "(^| )-?${FLAG}($| )") + target_compile_options(quotest PUBLIC -${FLAG}) + endif () + endforeach () +endif() + option(${PROJECT_NAME}_INSTALL_TESTS "install quotest application" ON) add_feature_info(InstallQuotest ${PROJECT_NAME}_INSTALL_TESTS "the library functional test suite") -- cgit v1.2.3