From 0a82e68f86a3b4716ebada39c041a6c8673d2999 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Thu, 17 Oct 2019 09:59:37 +0900 Subject: Connection::joinRoom: make sure the room object is created early enough All direct slots connected to finished() will run before success() is even emitted; so create the room object in the earliest slot connected to finished(), rather than to success(). --- lib/connection.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/connection.cpp b/lib/connection.cpp index 2c5bf574..25f1c3f6 100644 --- a/lib/connection.cpp +++ b/lib/connection.cpp @@ -534,10 +534,14 @@ JoinRoomJob* Connection::joinRoom(const QString& roomAlias, const QStringList& serverNames) { auto job = callApi(roomAlias, serverNames); - // Upon completion, ensure a room object in Join state is created but only - // if it's not already there due to a sync completing earlier. - connect(job, &JoinRoomJob::success, this, - [this, job] { provideRoom(job->roomId()); }); + // Upon completion, ensure a room object in Join state is created + // (or it might already be there due to a sync completing earlier). + // finished() is used here instead of success() to overtake clients + // that may add their own slots to finished(). + connect(job, &BaseJob::finished, this, [this, job] { + if (job->status().good()) + provideRoom(job->roomId()); + }); return job; } -- cgit v1.2.3 From d59ca7ac194e8d57177afb1ac89603e22b61b4ec Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Thu, 3 Oct 2019 08:16:29 +0900 Subject: qmc-example -> quotest, QMCTest -> TestManager Also: some bits of refactoring in the test code to make it more extensible. Closes #352. --- .travis.yml | 14 +- .valgrind.qmc-example.supp | 68 ------ CMakeLists.txt | 18 +- examples/CMakeLists.txt | 69 ------ lib/csapi/gtad.yaml | 4 +- qmc-example.pro | 12 - quotest.pro | 13 + tests/.valgrind.supp | 68 ++++++ tests/CMakeLists.txt | 69 ++++++ tests/quotest.cpp | 577 +++++++++++++++++++++++++++++++++++++++++++++ 10 files changed, 746 insertions(+), 166 deletions(-) delete mode 100644 .valgrind.qmc-example.supp delete mode 100644 examples/CMakeLists.txt delete mode 100644 qmc-example.pro create mode 100644 quotest.pro create mode 100644 tests/.valgrind.supp create mode 100644 tests/CMakeLists.txt create mode 100644 tests/quotest.cpp (limited to 'lib') diff --git a/.travis.yml b/.travis.yml index 21b2fd64..3f2759bb 100644 --- a/.travis.yml +++ b/.travis.yml @@ -16,7 +16,7 @@ env: global: - DESTDIR="$TRAVIS_BUILD_DIR/install" - CMAKE_ARGS="-DCMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_INSTALL_PREFIX=/usr -DCMAKE_PREFIX_PATH=$DESTDIR/usr" - - VALGRIND="valgrind --tool=memcheck --leak-check=yes --gen-suppressions=all --suppressions=.valgrind.qmc-example.supp $VALGRIND_OPTIONS" + - VALGRIND="valgrind --tool=memcheck --leak-check=yes --gen-suppressions=all --suppressions=tests/.valgrind.supp $VALGRIND_OPTIONS" matrix: include: @@ -65,14 +65,14 @@ before_script: script: - _cmake_build --target install -# Build qmc-example with the installed libQuotient -- cmake $CMAKE_ARGS examples -Bbuild-example -DOlm_DIR=olm/build -- cmake --build build-example --target all +# Build quotest with the installed libQuotient +- cmake $CMAKE_ARGS tests -Bbuild-test -DOlm_DIR=olm/build +- cmake --build build-test --target all # Build with qmake -- qmake qmc-example.pro "CONFIG += debug" "CONFIG -= app_bundle" "QMAKE_CC = $CC" "QMAKE_CXX = $CXX" "INCLUDEPATH += olm/include" "LIBS += -Lbuild/lib" "LIBS += -Lolm/build" +- qmake quotest.pro "CONFIG += debug" "CONFIG -= app_bundle" "QMAKE_CC = $CC" "QMAKE_CXX = $CXX" "INCLUDEPATH += olm/include" "LIBS += -Lbuild/lib" "LIBS += -Lolm/build" - make all -# Run the qmake-compiled qmc-example under valgrind -- if [ "$QMC_TEST_USER" != "" ]; then LD_LIBRARY_PATH="olm/build" $VALGRIND ./qmc-example "$QMC_TEST_USER" "$QMC_TEST_PWD" qmc-example-travis '#qmc-test:matrix.org' "Travis CI job $TRAVIS_JOB_NUMBER"; fi +# Run the qmake-compiled quotest under valgrind +- if [ "$TEST_USER" != "" ]; then LD_LIBRARY_PATH="olm/build" $VALGRIND ./quotest "$QMC_TEST_USER" "$QMC_TEST_PWD" quotest-travis '#quotest:matrix.org' "Travis CI job $TRAVIS_JOB_NUMBER"; fi notifications: webhooks: diff --git a/.valgrind.qmc-example.supp b/.valgrind.qmc-example.supp deleted file mode 100644 index d65fb52e..00000000 --- a/.valgrind.qmc-example.supp +++ /dev/null @@ -1,68 +0,0 @@ -{ - libc_dirty_free_on_exit - Memcheck:Free - fun:free - fun:__libc_freeres - fun:_vgnU_freeres - fun:__run_exit_handlers - fun:exit -} - -{ - QAuthenticator - Memcheck:Leak - match-leak-kinds: possible - ... - fun:_ZN14QAuthenticator6detachEv -} - -{ - QTimer - Memcheck:Leak - match-leak-kinds: possible - fun:_Znwm - fun:_ZN7QObjectC1EPS_ - fun:_ZN6QTimerC1EP7QObject -} - -{ - QSslConfiguration - Memcheck:Leak - match-leak-kinds: possible - fun:_Znwm - ... - fun:_ZN17QSslConfigurationC1Ev -} - -{ - libcrypto_ASN1 - Memcheck:Leak - match-leak-kinds: definite - fun:malloc - ... - fun:ASN1_item_ex_d2i -} - -{ - malloc_from_libcrypto - Memcheck:Leak - match-leak-kinds: possible - fun:malloc - fun:CRYPTO_malloc - ... - obj:/lib/x86_64-linux-gnu/libcrypto.so.* -} - -{ - Slot_activation_from_QtNetwork - Memcheck:Leak - match-leak-kinds: definite - fun:malloc - fun:inflateInit2_ - obj:/*/*/*/libQt5Network.so.* - ... - fun:_ZN11QMetaObject8activateEP7QObjectiiPPv - ... - fun:_ZN11QMetaObject8activateEP7QObjectiiPPv - obj:/*/*/*/libQt5Network.so.* -} \ No newline at end of file diff --git a/CMakeLists.txt b/CMakeLists.txt index ce4af9a8..58509eae 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -6,7 +6,7 @@ endif() set(API_VERSION "0.6") project(Quotient VERSION "${API_VERSION}.0" LANGUAGES CXX) -option(QUOTIENT_INSTALL_EXAMPLE "install qmc-example application" ON) +option(${PROJECT_NAME}_INSTALL_TESTS "install quotest (former qmc-example) application" ON) include(CheckCXXCompilerFlag) if (NOT WIN32) @@ -52,7 +52,7 @@ else() endforeach () endif() -find_package(Qt5 5.9 REQUIRED Network Gui Multimedia) +find_package(Qt5 5.9 REQUIRED Network Gui Multimedia Test) get_filename_component(Qt5_Prefix "${Qt5_DIR}/../../../.." ABSOLUTE) if ((NOT DEFINED USE_INTREE_LIBQOLM OR USE_INTREE_LIBQOLM) @@ -218,7 +218,7 @@ if (MATRIX_DOC_PATH AND GTAD_PATH) endif() endif() -set(example_SRCS examples/qmc-example.cpp) +set(tests_SRCS tests/quotest.cpp) add_library(${PROJECT_NAME} ${lib_SRCS} ${api_SRCS}) set_target_properties(${PROJECT_NAME} PROPERTIES @@ -237,8 +237,10 @@ target_include_directories(${PROJECT_NAME} PUBLIC ) target_link_libraries(${PROJECT_NAME} QtOlm Qt5::Core Qt5::Network Qt5::Gui Qt5::Multimedia) -add_executable(qmc-example ${example_SRCS}) -target_link_libraries(qmc-example Qt5::Core Quotient) +set(TEST_BINARY quotest) +add_executable(${TEST_BINARY} ${tests_SRCS}) +target_link_libraries(${TEST_BINARY} Qt5::Core Qt5::Test Quotient) + configure_file(Quotient.pc.in ${CMAKE_CURRENT_BINARY_DIR}/Quotient.pc @ONLY NEWLINE_STYLE UNIX) # Installation @@ -281,9 +283,9 @@ if (WIN32) install(FILES mime/packages/freedesktop.org.xml DESTINATION mime/packages) endif (WIN32) -if (QUOTIENT_INSTALL_EXAMPLE) - install(TARGETS qmc-example RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}) -endif (QUOTIENT_INSTALL_EXAMPLE) +if (QUOTIENT_INSTALL_TESTS) + install(TARGETS ${TEST_BINARY} RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}) +endif () if (UNIX AND NOT APPLE) install(FILES ${CMAKE_CURRENT_BINARY_DIR}/Quotient.pc diff --git a/examples/CMakeLists.txt b/examples/CMakeLists.txt deleted file mode 100644 index 1f512958..00000000 --- a/examples/CMakeLists.txt +++ /dev/null @@ -1,69 +0,0 @@ -cmake_minimum_required(VERSION 3.1) - -# This CMakeLists file assumes that the library is installed to CMAKE_INSTALL_PREFIX -# and ignores the in-tree library code. You can use this to start work on your own client. - -project(qmc-example CXX) - -include(CheckCXXCompilerFlag) -if (NOT WIN32) - include(GNUInstallDirs) -endif(NOT WIN32) - -# Find includes in corresponding build directories -set(CMAKE_INCLUDE_CURRENT_DIR ON) -# Instruct CMake to run moc automatically when needed. -set(CMAKE_AUTOMOC ON) - -# Set a default build type if none was specified -if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES) - message(STATUS "Setting build type to 'Debug' as none was specified") - set(CMAKE_BUILD_TYPE Debug CACHE STRING "Choose the type of build" FORCE) - # Set the possible values of build type for cmake-gui - set_property(CACHE CMAKE_BUILD_TYPE PROPERTY STRINGS "Debug" "Release" - "MinSizeRel" "RelWithDebInfo") -endif() - -if (NOT CMAKE_INSTALL_LIBDIR) - set(CMAKE_INSTALL_LIBDIR ".") -endif() - -if (NOT CMAKE_INSTALL_BINDIR) - set(CMAKE_INSTALL_BINDIR ".") -endif() - -if (NOT CMAKE_INSTALL_INCLUDEDIR) - set(CMAKE_INSTALL_INCLUDEDIR "include") -endif() - -set(CMAKE_CXX_STANDARD 14) - -foreach (FLAG all "" pedantic extra error=return-type no-unused-parameter no-gnu-zero-variadic-macro-arguments) - CHECK_CXX_COMPILER_FLAG("-W${FLAG}" WARN_${FLAG}_SUPPORTED) - if ( WARN_${FLAG}_SUPPORTED AND NOT CMAKE_CXX_FLAGS MATCHES "(^| )-W?${FLAG}($| )") - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -W${FLAG}") - endif () -endforeach () - -find_package(Qt5 5.6 REQUIRED Network Gui Multimedia) -get_filename_component(Qt5_Prefix "${Qt5_DIR}/../../../.." ABSOLUTE) - -find_package(Quotient REQUIRED) -get_filename_component(QMC_Prefix "${Quotient_DIR}/../.." ABSOLUTE) - -message( STATUS "qmc-example configuration:" ) -if (CMAKE_BUILD_TYPE) - message( STATUS " Build type: ${CMAKE_BUILD_TYPE}") -endif(CMAKE_BUILD_TYPE) -message( STATUS " Compiler: ${CMAKE_CXX_COMPILER_ID} ${CMAKE_CXX_COMPILER_VERSION}" ) -message( STATUS " Qt: ${Qt5_VERSION} at ${Qt5_Prefix}" ) -message( STATUS " Quotient: ${Quotient_VERSION} at ${QMC_Prefix}" ) - -set(example_SRCS qmc-example.cpp) - -add_executable(qmc-example ${example_SRCS}) -target_link_libraries(qmc-example Qt5::Core Quotient) - -# Installation - -install (TARGETS qmc-example RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}) diff --git a/lib/csapi/gtad.yaml b/lib/csapi/gtad.yaml index 301ee0b6..6d4e080f 100644 --- a/lib/csapi/gtad.yaml +++ b/lib/csapi/gtad.yaml @@ -95,9 +95,9 @@ analyzer: - //: { type: "QVector<{{1}}>", imports: } - map: # `additionalProperties` in OpenAPI - RoomState: - type: "std::unordered_map" + type: "UnorderedMap" moveOnly: - imports: + imports: '"util.h"' - /.+/: type: "QHash" imports: diff --git a/qmc-example.pro b/qmc-example.pro deleted file mode 100644 index a9548df9..00000000 --- a/qmc-example.pro +++ /dev/null @@ -1,12 +0,0 @@ -TEMPLATE = app - -CONFIG *= c++1z warn_on object_parallel_to_source - -windows { CONFIG *= console } - -include(libquotient.pri) - -SOURCES += examples/qmc-example.cpp - -DISTFILES += \ - .valgrind.qmc-example.supp diff --git a/quotest.pro b/quotest.pro new file mode 100644 index 00000000..433a2ccc --- /dev/null +++ b/quotest.pro @@ -0,0 +1,13 @@ +TEMPLATE = app + +QT += testlib +CONFIG *= c++1z warn_on object_parallel_to_source + +windows { CONFIG *= console } + +include(libquotient.pri) + +SOURCES += tests/quotest.cpp + +DISTFILES += \ + .valgrind.supp diff --git a/tests/.valgrind.supp b/tests/.valgrind.supp new file mode 100644 index 00000000..d65fb52e --- /dev/null +++ b/tests/.valgrind.supp @@ -0,0 +1,68 @@ +{ + libc_dirty_free_on_exit + Memcheck:Free + fun:free + fun:__libc_freeres + fun:_vgnU_freeres + fun:__run_exit_handlers + fun:exit +} + +{ + QAuthenticator + Memcheck:Leak + match-leak-kinds: possible + ... + fun:_ZN14QAuthenticator6detachEv +} + +{ + QTimer + Memcheck:Leak + match-leak-kinds: possible + fun:_Znwm + fun:_ZN7QObjectC1EPS_ + fun:_ZN6QTimerC1EP7QObject +} + +{ + QSslConfiguration + Memcheck:Leak + match-leak-kinds: possible + fun:_Znwm + ... + fun:_ZN17QSslConfigurationC1Ev +} + +{ + libcrypto_ASN1 + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + ... + fun:ASN1_item_ex_d2i +} + +{ + malloc_from_libcrypto + Memcheck:Leak + match-leak-kinds: possible + fun:malloc + fun:CRYPTO_malloc + ... + obj:/lib/x86_64-linux-gnu/libcrypto.so.* +} + +{ + Slot_activation_from_QtNetwork + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + fun:inflateInit2_ + obj:/*/*/*/libQt5Network.so.* + ... + fun:_ZN11QMetaObject8activateEP7QObjectiiPPv + ... + fun:_ZN11QMetaObject8activateEP7QObjectiiPPv + obj:/*/*/*/libQt5Network.so.* +} \ No newline at end of file diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt new file mode 100644 index 00000000..b6ba0f18 --- /dev/null +++ b/tests/CMakeLists.txt @@ -0,0 +1,69 @@ +cmake_minimum_required(VERSION 3.1) + +# This CMakeLists file assumes that the library is installed to CMAKE_INSTALL_PREFIX +# and ignores the in-tree library code. You can use this to start work on your own client. + +project(quotest CXX) + +include(CheckCXXCompilerFlag) +if (NOT WIN32) + include(GNUInstallDirs) +endif(NOT WIN32) + +# Find includes in corresponding build directories +set(CMAKE_INCLUDE_CURRENT_DIR ON) +# Instruct CMake to run moc automatically when needed. +set(CMAKE_AUTOMOC ON) + +# Set a default build type if none was specified +if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES) + message(STATUS "Setting build type to 'Debug' as none was specified") + set(CMAKE_BUILD_TYPE Debug CACHE STRING "Choose the type of build" FORCE) + # Set the possible values of build type for cmake-gui + set_property(CACHE CMAKE_BUILD_TYPE PROPERTY STRINGS "Debug" "Release" + "MinSizeRel" "RelWithDebInfo") +endif() + +if (NOT CMAKE_INSTALL_LIBDIR) + set(CMAKE_INSTALL_LIBDIR ".") +endif() + +if (NOT CMAKE_INSTALL_BINDIR) + set(CMAKE_INSTALL_BINDIR ".") +endif() + +if (NOT CMAKE_INSTALL_INCLUDEDIR) + set(CMAKE_INSTALL_INCLUDEDIR "include") +endif() + +set(CMAKE_CXX_STANDARD 14) + +foreach (FLAG all "" pedantic extra error=return-type no-unused-parameter no-gnu-zero-variadic-macro-arguments) + CHECK_CXX_COMPILER_FLAG("-W${FLAG}" WARN_${FLAG}_SUPPORTED) + if ( WARN_${FLAG}_SUPPORTED AND NOT CMAKE_CXX_FLAGS MATCHES "(^| )-W?${FLAG}($| )") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -W${FLAG}") + endif () +endforeach () + +find_package(Qt5 5.9 REQUIRED Network Gui Multimedia) +get_filename_component(Qt5_Prefix "${Qt5_DIR}/../../../.." ABSOLUTE) + +find_package(Quotient REQUIRED) +get_filename_component(QMC_Prefix "${Quotient_DIR}/../.." ABSOLUTE) + +message( STATUS "${PROJECT_NAME} configuration:" ) +if (CMAKE_BUILD_TYPE) + message( STATUS " Build type: ${CMAKE_BUILD_TYPE}") +endif(CMAKE_BUILD_TYPE) +message( STATUS " Compiler: ${CMAKE_CXX_COMPILER_ID} ${CMAKE_CXX_COMPILER_VERSION}" ) +message( STATUS " Qt: ${Qt5_VERSION} at ${Qt5_Prefix}" ) +message( STATUS " Quotient: ${Quotient_VERSION} at ${QMC_Prefix}" ) + +set(example_SRCS quotest.cpp) + +add_executable(${PROJECT_NAME} ${example_SRCS}) +target_link_libraries(${PROJECT_NAME} Qt5::Core Quotient) + +# Installation + +install (TARGETS ${PROJECT_NAME} RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}) diff --git a/tests/quotest.cpp b/tests/quotest.cpp new file mode 100644 index 00000000..0a25dbc1 --- /dev/null +++ b/tests/quotest.cpp @@ -0,0 +1,577 @@ + +#include "connection.h" +#include "room.h" +#include "user.h" + +#include "csapi/joining.h" +#include "csapi/leaving.h" +#include "csapi/room_send.h" + +#include "events/reactionevent.h" +#include "events/simplestateevents.h" + +#include +#include +#include +#include +#include + +#include +#include + +using namespace Quotient; +using std::cout, std::endl; + +class TestManager : public QObject { +public: + TestManager(Connection* conn, QString testRoomName, QString source); + +private slots: + // clang-format off + void setupAndRun(); + void onNewRoom(Room* r); + void run(); + void doTests(); + void loadMembers(); + void sendMessage(); + void sendReaction(const QString& targetEvtId); + void sendFile(); + void checkFileSendingOutcome(const QString& txnId, + const QString& fileName); + void setTopic(); + void sendAndRedact(); + bool checkRedactionOutcome(const QString& evtIdToRedact); + void addAndRemoveTag(); + void markDirectChat(); + void checkDirectChatOutcome( + const Connection::DirectChatsMap& added); + void conclude(); + void finalize(); + // clang-format on + +private: + QScopedPointer c; + QStringList running; + QStringList succeeded; + QStringList failed; + QString origin; + QString targetRoomName; + Room* targetRoom = nullptr; + +using TestToken = QByteArray; // return value of QMetaMethod::name +// For now, the token itself is the test name but that may change. +const char* testName(const TestToken& token) { return token.constData(); } + bool validatePendingEvent(const QString& txnId); + void finishTest(const TestToken& token, bool condition, const char* file, + int line);}; + +#define TEST_IMPL(Name) void TestManager::Name() + +#define FINISH_TEST(description, Condition) \ + finishTest(description, Condition, __FILE__, __LINE__) + +#define FAIL_TEST(description) FINISH_TEST(description, false) + +bool TestManager::validatePendingEvent(const QString& txnId) +{ + auto it = targetRoom->findPendingEvent(txnId); + return it != targetRoom->pendingEvents().end() + && it->deliveryStatus() == EventStatus::Submitted + && (*it)->transactionId() == txnId; +} + +void TestManager::finishTest(const TestToken& token, bool condition, + const char* file, int line) +{ + const auto& item = testName(token); + Q_ASSERT_X(running.contains(item), item, + "Trying to finish an item that's not running"); + running.removeOne(item); + if (condition) { + succeeded.push_back(item); + cout << item << " successful" << endl; + if (targetRoom) + targetRoom->postMessage(origin % ": " % item % " successful", + MessageEventType::Notice); + } else { + failed.push_back(item); + cout << item << " FAILED at " << file << ":" << line << endl; + if (targetRoom) + targetRoom->postPlainText(origin % ": " % item % " FAILED at " + % file % ", line " % QString::number(line)); + } +} + +TestManager::TestManager(Connection* conn, QString testRoomName, QString source) + : c(conn), origin(std::move(source)), targetRoomName(std::move(testRoomName)) +{ + if (!origin.isEmpty()) + cout << "Origin for the test message: " << origin.toStdString() << endl; + cout << "Test room name: " << targetRoomName.toStdString() << endl; + + connect(c.data(), &Connection::connected, this, &TestManager::setupAndRun); + connect(c.data(), &Connection::loadedRoomState, this, &TestManager::onNewRoom); + // Big countdown watchdog + QTimer::singleShot(180000, this, &TestManager::conclude); +} + +void TestManager::setupAndRun() +{ + Q_ASSERT(!c->homeserver().isEmpty() && c->homeserver().isValid()); + Q_ASSERT(c->domain() == c->userId().section(':', 1)); + cout << "Connected, server: " + << c->homeserver().toDisplayString().toStdString() << endl; + cout << "Access token: " << c->accessToken().toStdString() << endl; + + cout << "Joining " << targetRoomName.toStdString() << endl; + running.push_back("Join room"); + auto joinJob = c->joinRoom(targetRoomName); + connect(joinJob, &BaseJob::failure, this, [this] { + FAIL_TEST("Join room"); + conclude(); + }); + // Connection::joinRoom() creates a Room object upon JoinRoomJob::success + // but this object is empty until the first sync is done. + connect(joinJob, &BaseJob::success, this, [this, joinJob] { + targetRoom = c->room(joinJob->roomId(), JoinState::Join); + FINISH_TEST("Join room", targetRoom != nullptr); + + run(); + }); +} + +void TestManager::onNewRoom(Room* r) +{ + cout << "New room: " << r->id().toStdString() << endl + << " Name: " << r->name().toStdString() << endl + << " Canonical alias: " << r->canonicalAlias().toStdString() << endl + << endl; + connect(r, &Room::aboutToAddNewMessages, r, [r](RoomEventsRange timeline) { + cout << timeline.size() << " new event(s) in room " + << r->canonicalAlias().toStdString() << endl; + // for (const auto& item: timeline) + // { + // cout << "From: " + // << r->roomMembername(item->senderId()).toStdString() + // << endl << "Timestamp:" + // << item->timestamp().toString().toStdString() << endl + // << "JSON:" << endl << + // item->originalJson().toStdString() << endl; + // } + }); +} + +void TestManager::run() +{ + c->setLazyLoading(true); + c->syncLoop(); + connectSingleShot(c.data(), &Connection::syncDone, this, &TestManager::doTests); + connect(c.data(), &Connection::syncDone, c.data(), [this] { + cout << "Sync complete, " << running.size() + << " test(s) in the air: " << running.join(", ").toStdString() + << endl; + if (running.isEmpty()) + conclude(); + }); +} + +void TestManager::doTests() +{ + cout << "Starting tests" << endl; + + loadMembers(); + + sendMessage(); + sendFile(); + setTopic(); + addAndRemoveTag(); + sendAndRedact(); + markDirectChat(); + // Add here tests with the test room +} + +TEST_IMPL(loadMembers) +{ + running.push_back("Loading members"); + auto* r = c->roomByAlias(QStringLiteral("#quotient:matrix.org"), + JoinState::Join); + if (!r) { + cout << "#quotient:matrix.org is not found in the test user's rooms" + << endl; + FAIL_TEST("Loading members"); + return; + } + // 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()) { + cout << "Lazy loading doesn't seem to be enabled" << endl; + FAIL_TEST("Loading members"); + return; + } + r->setDisplayed(); + connect(r, &Room::allMembersLoaded, [this, r] { + FINISH_TEST("Loading members", + r->memberNames().size() >= r->joinedCount()); + }); +} + +TEST_IMPL(sendMessage) +{ + running.push_back("Message sending"); + cout << "Sending a message" << endl; + auto txnId = targetRoom->postPlainText("Hello, " % origin % " is here"); + if (!validatePendingEvent(txnId)) { + cout << "Invalid pending event right after submitting" << endl; + FAIL_TEST("Message sending"); + return; + } + connectUntil( + targetRoom, &Room::pendingEventAboutToMerge, this, + [this, txnId](const RoomEvent* evt, int pendingIdx) { + const auto& pendingEvents = targetRoom->pendingEvents(); + Q_ASSERT(pendingIdx >= 0 && pendingIdx < int(pendingEvents.size())); + + if (evt->transactionId() != txnId) + return false; + + FINISH_TEST("Message sending", + is(*evt) && !evt->id().isEmpty() + && pendingEvents[size_t(pendingIdx)]->transactionId() + == evt->transactionId()); + sendReaction(evt->id()); + return true; + }); +} + +void TestManager::sendReaction(const QString& targetEvtId) +{ + running.push_back("Reaction sending"); + cout << "Reacting to the newest message in the room" << endl; + Q_ASSERT(targetRoom->timelineSize() > 0); + const auto key = QStringLiteral("+1"); + const auto txnId = targetRoom->postReaction(targetEvtId, key); + if (!validatePendingEvent(txnId)) { + cout << "Invalid pending event right after submitting" << endl; + FAIL_TEST("Reaction sending"); + return; + } + + // TODO: Check that it came back as a reaction event and that it attached to + // the right event + connectUntil( + targetRoom, &Room::updatedEvent, this, + [this, txnId, key, targetEvtId](const QString& actualTargetEvtId) { + if (actualTargetEvtId != targetEvtId) + return false; + const auto reactions = targetRoom->relatedEvents( + targetEvtId, EventRelation::Annotation()); + // It's a test room, assuming no interference there should + // be exactly one reaction + if (reactions.size() != 1) { + FAIL_TEST("Reaction sending"); + } else { + const auto* evt = + eventCast(reactions.back()); + FINISH_TEST("Reaction sending", + is(*evt) && !evt->id().isEmpty() + && evt->relation().key == key + && evt->transactionId() == txnId); + } + return true; + }); +} + +TEST_IMPL(sendFile) +{ + running.push_back("File sending"); + cout << "Sending a file" << endl; + auto* tf = new QTemporaryFile; + if (!tf->open()) { + cout << "Failed to create a temporary file" << endl; + FAIL_TEST("File sending"); + return; + } + tf->write("Test"); + tf->close(); + // QFileInfo::fileName brings only the file name; QFile::fileName brings + // the full path + const auto tfName = QFileInfo(*tf).fileName(); + cout << "Sending file " << tfName.toStdString() << endl; + const auto txnId = + targetRoom->postFile("Test file", QUrl::fromLocalFile(tf->fileName())); + if (!validatePendingEvent(txnId)) { + cout << "Invalid pending event right after submitting" << endl; + delete tf; + FAIL_TEST("File sending"); + return; + } + + // FIXME: Clean away connections (connectUntil doesn't help here). + connect(targetRoom, &Room::fileTransferCompleted, this, + [this, txnId, tf, tfName](const QString& id) { + auto fti = targetRoom->fileTransferInfo(id); + Q_ASSERT(fti.status == FileTransferInfo::Completed); + + if (id != txnId) + return; + + delete tf; + + checkFileSendingOutcome(txnId, tfName); + }); + connect(targetRoom, &Room::fileTransferFailed, this, + [this, txnId, tf](const QString& id, const QString& error) { + if (id != txnId) + return; + + targetRoom->postPlainText(origin % ": File upload failed: " + % error); + delete tf; + + FAIL_TEST("File sending"); + }); +} + +void TestManager::checkFileSendingOutcome(const QString& txnId, + const QString& fileName) +{ + auto it = targetRoom->findPendingEvent(txnId); + if (it == targetRoom->pendingEvents().end()) { + cout << "Pending file event dropped before upload completion" << endl; + FAIL_TEST("File sending"); + return; + } + if (it->deliveryStatus() != EventStatus::FileUploaded) { + cout << "Pending file event status upon upload completion is " + << it->deliveryStatus() << " != FileUploaded(" + << EventStatus::FileUploaded << ')' << endl; + FAIL_TEST("File sending"); + return; + } + + connectUntil( + targetRoom, &Room::pendingEventAboutToMerge, this, + [this, txnId, fileName](const RoomEvent* evt, int pendingIdx) { + const auto& pendingEvents = targetRoom->pendingEvents(); + Q_ASSERT(pendingIdx >= 0 && pendingIdx < int(pendingEvents.size())); + + if (evt->transactionId() != txnId) + return false; + + cout << "File event " << txnId.toStdString() + << " arrived in the timeline" << endl; + visit( + *evt, + [&](const RoomMessageEvent& e) { + FINISH_TEST( + "File sending", + !e.id().isEmpty() + && pendingEvents[size_t(pendingIdx)]->transactionId() + == txnId + && e.hasFileContent() + && e.content()->fileInfo()->originalName == fileName); + }, + [this](const RoomEvent&) { FAIL_TEST("File sending"); }); + return true; + }); +} + +TEST_IMPL(setTopic) +{ + running.push_back("State setting test"); + + const auto newTopic = c->generateTxnId(); // Just a way to get a unique id + targetRoom->setTopic(newTopic); + + connectUntil(targetRoom, &Room::topicChanged, this, + [this, newTopic] { + FINISH_TEST("State setting test", + targetRoom->topic() == newTopic); + return true; + }); +} + +TEST_IMPL(sendAndRedact) +{ + running.push_back("Redaction"); + cout << "Sending a message to redact" << endl; + auto txnId = targetRoom->postPlainText(origin % ": message to redact"); + if (txnId.isEmpty()) { + FAIL_TEST("Redaction"); + return; + } + connect(targetRoom, &Room::messageSent, this, + [this, txnId](const QString& tId, const QString& evtId) { + if (tId != txnId) + return; + + cout << "Redacting the message" << endl; + targetRoom->redactEvent(evtId, origin); + + connectUntil(targetRoom, &Room::addedMessages, this, + [this, evtId] { + return checkRedactionOutcome(evtId); + }); + }); +} + +bool TestManager::checkRedactionOutcome(const QString& evtIdToRedact) +{ + // There are two possible (correct) outcomes: either the event comes already + // 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()) + return false; // Waiting for the next sync + + if ((*it)->isRedacted()) { + cout << "The sync brought already redacted message" << endl; + FINISH_TEST("Redaction", true); + } else { + cout << "Message came non-redacted with the sync, waiting for redaction" + << endl; + connectUntil(targetRoom, &Room::replacedEvent, this, + [this, evtIdToRedact](const RoomEvent* newEvent, + const RoomEvent* oldEvent) { + if (oldEvent->id() != evtIdToRedact) + return false; + + FINISH_TEST("Redaction", + newEvent->isRedacted() + && newEvent->redactionReason() == origin); + return true; + }); + } + return true; +} + +TEST_IMPL(addAndRemoveTag) +{ + running.push_back("Tagging test"); + static const auto TestTag = QStringLiteral("org.quotient.test"); + // Pre-requisite + if (targetRoom->tags().contains(TestTag)) + targetRoom->removeTag(TestTag); + + // Connect first because the signal is emitted synchronously. + connect(targetRoom, &Room::tagsChanged, targetRoom, [=] { + cout << "Room " << targetRoom->id().toStdString() + << ", tag(s) changed:" << endl + << " " << targetRoom->tagNames().join(", ").toStdString() << endl; + if (targetRoom->tags().contains(TestTag)) { + cout << "Test tag set, removing it now" << endl; + targetRoom->removeTag(TestTag); + FINISH_TEST("Tagging test", !targetRoom->tags().contains(TestTag)); + disconnect(targetRoom, &Room::tagsChanged, nullptr, nullptr); + } + }); + cout << "Adding a tag" << endl; + targetRoom->addTag(TestTag); +} + +TEST_IMPL(markDirectChat) +{ + running.push_back("Direct chat test"); + if (targetRoom->directChatUsers().contains(c->user())) { + cout << "Warning: the room is already a direct chat," + " only unmarking will be tested" + << endl; + checkDirectChatOutcome({ { c->user(), targetRoom->id() } }); + return; + } + // Connect first because the signal is emitted synchronously. + connect(c.data(), &Connection::directChatsListChanged, this, + &TestManager::checkDirectChatOutcome); + cout << "Marking the room as a direct chat" << endl; + c->addToDirectChats(targetRoom, c->user()); +} + +void TestManager::checkDirectChatOutcome(const Connection::DirectChatsMap& added) +{ + disconnect(c.data(), &Connection::directChatsListChanged, nullptr, nullptr); + if (!targetRoom->isDirectChat()) { + cout << "The room has not been marked as a direct chat" << endl; + FAIL_TEST("Direct chat test"); + return; + } + if (!added.contains(c->user(), targetRoom->id())) { + cout << "The room has not been listed in new direct chats" << endl; + FAIL_TEST("Direct chat test"); + return; + } + + cout << "Unmarking the direct chat" << endl; + c->removeFromDirectChats(targetRoom->id(), c->user()); + FINISH_TEST("Direct chat test", !c->isDirectChat(targetRoom->id())); +} + +void TestManager::conclude() +{ + c->stopSync(); + auto succeededRec = QString::number(succeeded.size()) + " tests succeeded"; + if (!failed.empty() || !running.empty()) + succeededRec += + " of " + % QString::number(succeeded.size() + failed.size() + running.size()) + % " total"; + QString plainReport = origin % ": Testing complete, " % succeededRec; + QString color = failed.empty() && running.empty() ? "00AA00" : "AA0000"; + QString htmlReport = origin % ": Testing complete, " % succeededRec; + if (!failed.empty()) { + plainReport += "\nFAILED: " % failed.join(", "); + htmlReport += "
Failed: " % failed.join(", "); + } + if (!running.empty()) { + plainReport += "\nDID NOT FINISH: " % running.join(", "); + htmlReport += "
Did not finish: " + % running.join(", "); + } + cout << plainReport.toStdString() << endl; + + if (targetRoom) { + // 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 = targetRoom->postHtmlText(plainReport, htmlReport); + connect(targetRoom, &Room::messageSent, this, + [this, txnId](const QString& serverTxnId) { + if (txnId != serverTxnId) + return; + + cout << "Leaving the room" << endl; + connect(targetRoom->leaveRoom(), &BaseJob::finished, this, + &TestManager::finalize); + }); + } else + finalize(); +} + +void TestManager::finalize() +{ + cout << "Logging out" << endl; + c->logout(); + connect(c.data(), &Connection::loggedOut, qApp, [this] { + QCoreApplication::processEvents(); + QCoreApplication::exit(failed.size() + running.size()); + }); +} + +int main(int argc, char* argv[]) +{ + QCoreApplication app(argc, argv); + if (argc < 5) { + cout << "Usage: quotest [origin]" + << endl; + return -1; + } + + cout << "Connecting to the server as " << argv[1] << endl; + auto conn = new Connection; + conn->connectToServer(argv[1], argv[2], argv[3]); + TestManager test { conn, argv[4], argc >= 6 ? argv[5] : nullptr }; + return app.exec(); +} -- cgit v1.2.3 From 8b9207d5a04386957d8eab8dd251421eaaa7c0d2 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 20 Oct 2019 19:13:56 +0900 Subject: Qualify types in signals and Q_INVOKABLEs Because https://doc.qt.io/qt-5/moc.html#limitations . For direct connections that doesn't matter but it very much does for queued ones. Along with this DirectChatsMap and IgnoredUsersList have been moved from Connection:: to Quotient::. --- lib/connection.cpp | 6 ++-- lib/connection.h | 82 ++++++++++++++++++++++++++++-------------------------- lib/jobs/basejob.h | 19 +++++++------ lib/room.h | 64 ++++++++++++++++++++++-------------------- lib/user.h | 13 +++++---- 5 files changed, 97 insertions(+), 87 deletions(-) (limited to 'lib') diff --git a/lib/connection.cpp b/lib/connection.cpp index 25f1c3f6..af85d066 100644 --- a/lib/connection.cpp +++ b/lib/connection.cpp @@ -385,7 +385,7 @@ void Connection::syncLoop(int timeout) syncLoopIteration(); // initial sync to start the loop } -QJsonObject toJson(const Connection::DirectChatsMap& directChats) +QJsonObject toJson(const DirectChatsMap& directChats) { QJsonObject json; for (auto it = directChats.begin(); it != directChats.end();) { @@ -1050,7 +1050,7 @@ QVector Connection::roomsWithTag(const QString& tagName) const return rooms; } -Connection::DirectChatsMap Connection::directChats() const +DirectChatsMap Connection::directChats() const { return d->directChats; } @@ -1117,7 +1117,7 @@ bool Connection::isIgnored(const User* user) const return ignoredUsers().contains(user->id()); } -Connection::IgnoredUsersList Connection::ignoredUsers() const +IgnoredUsersList Connection::ignoredUsers() const { const auto* event = d->unpackAccountData(); return event ? event->ignored_users() : IgnoredUsersList(); diff --git a/lib/connection.h b/lib/connection.h index 1f1d4cd5..e4109fd4 100644 --- a/lib/connection.h +++ b/lib/connection.h @@ -97,6 +97,14 @@ enum RunningPolicy { ForegroundRequest = 0x0, BackgroundRequest = 0x1 }; Q_ENUM_NS(RunningPolicy) +// Room ids, rather than room pointers, are used in the direct chat +// map types because the library keeps Invite rooms separate from +// rooms in Join and Leave state; and direct chats in account data +// are stored with no regard to their state. +using DirectChatsMap = QMultiHash; +using DirectChatUsersMap = QMultiHash; +using IgnoredUsersList = IgnoredUsersEvent::content_type; + class Connection : public QObject { Q_OBJECT @@ -115,14 +123,6 @@ class Connection : public QObject { lazyLoadingChanged) public: - // Room ids, rather than room pointers, are used in the direct chat - // map types because the library keeps Invite rooms separate from - // rooms in Join and Leave state; and direct chats in account data - // are stored with no regard to their state. - using DirectChatsMap = QMultiHash; - using DirectChatUsersMap = QMultiHash; - using IgnoredUsersList = IgnoredUsersEvent::content_type; - using UsersToDevicesToEvents = UnorderedMap>; @@ -153,7 +153,7 @@ public: * from the server. * \sa rooms, room, roomsWithTag */ - Q_INVOKABLE QVector allRooms() const; + Q_INVOKABLE QVector allRooms() const; /// Get rooms that have either of the given join state(s) /*! @@ -163,10 +163,11 @@ public: * Leave rooms from the server. * \sa allRooms, room, roomsWithTag */ - Q_INVOKABLE QVector rooms(JoinStates joinStates) const; + Q_INVOKABLE QVector + rooms(Quotient::JoinStates joinStates) const; /// Get the total number of rooms in the given join state(s) - Q_INVOKABLE int roomsCount(JoinStates joinStates) const; + Q_INVOKABLE int roomsCount(Quotient::JoinStates joinStates) const; /** Check whether the account has data of the given type * Direct chats map is not supported by this method _yet_. @@ -253,10 +254,10 @@ public: QList directChatUsers(const Room* room) const; /** Check whether a particular user is in the ignore list */ - Q_INVOKABLE bool isIgnored(const User* user) const; + Q_INVOKABLE bool isIgnored(const Quotient::User* user) const; /** Get the whole list of ignored users */ - Q_INVOKABLE IgnoredUsersList ignoredUsers() const; + Q_INVOKABLE Quotient::IgnoredUsersList ignoredUsers() const; /** Add the user to the ignore list * The change signal is emitted synchronously, without waiting @@ -264,14 +265,14 @@ public: * * \sa ignoredUsersListChanged */ - Q_INVOKABLE void addToIgnoredUsers(const User* user); + Q_INVOKABLE void addToIgnoredUsers(const Quotient::User* user); /** Remove the user from the ignore list */ /** Similar to adding, the change signal is emitted synchronously. * * \sa ignoredUsersListChanged */ - Q_INVOKABLE void removeFromIgnoredUsers(const User* user); + Q_INVOKABLE void removeFromIgnoredUsers(const Quotient::User* user); /** Get the full list of users known to this account */ QMap users() const; @@ -281,13 +282,14 @@ public: /** Get the domain name used for ids/aliases on the server */ QString domain() const; /** Find a room by its id and a mask of applicable states */ - Q_INVOKABLE Room* room(const QString& roomId, - JoinStates states = JoinState::Invite - | JoinState::Join) const; + Q_INVOKABLE Quotient::Room* + room(const QString& roomId, + Quotient::JoinStates states = JoinState::Invite | JoinState::Join) const; /** Find a room by its alias and a mask of applicable states */ - Q_INVOKABLE Room* roomByAlias(const QString& roomAlias, - JoinStates states = JoinState::Invite - | JoinState::Join) const; + Q_INVOKABLE Quotient::Room* + roomByAlias(const QString& roomAlias, + Quotient::JoinStates states = JoinState::Invite + | JoinState::Join) const; /** Update the internal map of room aliases to IDs */ /// This is used to maintain the internal index of room aliases. /// It does NOT change aliases on the server, @@ -295,15 +297,15 @@ public: void updateRoomAliases(const QString& roomId, const QString& aliasServer, const QStringList& previousRoomAliases, const QStringList& roomAliases); - Q_INVOKABLE Room* invitation(const QString& roomId) const; - Q_INVOKABLE User* user(const QString& userId); + Q_INVOKABLE Quotient::Room* invitation(const QString& roomId) const; + Q_INVOKABLE Quotient::User* user(const QString& userId); const User* user() const; User* user(); QString userId() const; QString deviceId() const; QByteArray accessToken() const; QtOlm::Account* olmAccount() const; - Q_INVOKABLE SyncJob* syncJob() const; + Q_INVOKABLE Quotient::SyncJob* syncJob() const; Q_INVOKABLE int millisToReconnect() const; Q_INVOKABLE void getTurnServers(); @@ -589,6 +591,7 @@ public slots: /** @deprecated Use callApi() or Room::postReceipt() instead */ virtual PostReceiptJob* postReceipt(Room* room, RoomEvent* event) const; + signals: /** * @deprecated @@ -622,7 +625,7 @@ signals: * * @param request - the pointer to the failed job */ - void requestFailed(BaseJob* request); + void requestFailed(Quotient::BaseJob* request); /** A network request (job) failed due to network problems * @@ -640,7 +643,7 @@ signals: void syncDone(); void syncError(QString message, QString details); - void newUser(User* user); + void newUser(Quotient::User* user); /** * \group Signals emitted on room transitions @@ -672,7 +675,7 @@ signals: */ /** A new room object has been created */ - void newRoom(Room* room); + void newRoom(Quotient::Room* room); /** A room invitation is seen for the first time * @@ -680,7 +683,7 @@ signals: * that initial sync will trigger this signal for all rooms in * Invite state. */ - void invitedRoom(Room* room, Room* prev); + void invitedRoom(Quotient::Room* room, Quotient::Room* prev); /** A joined room is seen for the first time * @@ -691,7 +694,7 @@ signals: * this room was in Invite state before, the respective object is * passed in prev (and it will be deleted shortly afterwards). */ - void joinedRoom(Room* room, Room* prev); + void joinedRoom(Quotient::Room* room, Quotient::Room* prev); /** A room has just been left * @@ -702,10 +705,10 @@ signals: * Left rooms upon initial sync (not only those that were left * right before the sync). */ - void leftRoom(Room* room, Room* prev); + void leftRoom(Quotient::Room* room, Quotient::Room* prev); /** The room object is about to be deleted */ - void aboutToDeleteRoom(Room* room); + void aboutToDeleteRoom(Quotient::Room* room); /** The room has just been created by createRoom or requestDirectChat * @@ -716,7 +719,7 @@ signals: * use directChatAvailable signal if you just need to obtain * a direct chat room. */ - void createdRoom(Room* room); + void createdRoom(Quotient::Room* room); /** The first sync for the room has been completed * @@ -726,7 +729,7 @@ signals: * signals (newRoom, joinedRoom etc.) come earlier, when the room * has just been created. */ - void loadedRoomState(Room* room); + void loadedRoomState(Quotient::Room* room); /** Account data (except direct chats) have changed */ void accountDataChanged(QString type); @@ -735,18 +738,18 @@ signals: * This signal is emitted upon any successful outcome from * requestDirectChat. */ - void directChatAvailable(Room* directChat); + void directChatAvailable(Quotient::Room* directChat); /** The list of direct chats has changed * This signal is emitted every time when the mapping of users * to direct chat rooms is changed (because of either local updates * or a different list arrived from the server). */ - void directChatsListChanged(DirectChatsMap additions, - DirectChatsMap removals); + void directChatsListChanged(Quotient::DirectChatsMap additions, + Quotient::DirectChatsMap removals); - void ignoredUsersListChanged(IgnoredUsersList additions, - IgnoredUsersList removals); + void ignoredUsersListChanged(Quotient::IgnoredUsersList additions, + Quotient::IgnoredUsersList removals); void cacheStateChanged(); void lazyLoadingChanged(); @@ -812,4 +815,5 @@ private: static user_factory_t _userFactory; }; } // namespace Quotient -Q_DECLARE_METATYPE(Quotient::Connection*) +Q_DECLARE_METATYPE(Quotient::DirectChatsMap) +Q_DECLARE_METATYPE(Quotient::IgnoredUsersList) diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h index 6c1b802c..c4da40f5 100644 --- a/lib/jobs/basejob.h +++ b/lib/jobs/basejob.h @@ -184,11 +184,11 @@ public: using duration_ms_t = std::chrono::milliseconds::rep; // normally int64_t std::chrono::seconds getCurrentTimeout() const; - Q_INVOKABLE duration_ms_t getCurrentTimeoutMs() const; + Q_INVOKABLE Quotient::BaseJob::duration_ms_t getCurrentTimeoutMs() const; std::chrono::seconds getNextRetryInterval() const; - Q_INVOKABLE duration_ms_t getNextRetryMs() const; + Q_INVOKABLE Quotient::BaseJob::duration_ms_t getNextRetryMs() const; std::chrono::milliseconds timeToRetry() const; - Q_INVOKABLE duration_ms_t millisToRetry() const; + Q_INVOKABLE Quotient::BaseJob::duration_ms_t millisToRetry() const; friend QDebug operator<<(QDebug dbg, const BaseJob* j) { @@ -215,7 +215,7 @@ signals: void sentRequest(); /** The job has changed its status */ - void statusChanged(Status newStatus); + void statusChanged(Quotient::BaseJob::Status newStatus); /** * The previous network request has failed; the next attempt will @@ -225,7 +225,8 @@ signals: * @param inMilliseconds the interval after which the next attempt will be * taken */ - void retryScheduled(int nextAttempt, duration_ms_t inMilliseconds); + void retryScheduled(int nextAttempt, + Quotient::BaseJob::duration_ms_t inMilliseconds); /** * The previous network request has been rate-limited; the next attempt @@ -251,7 +252,7 @@ signals: * * @see result, success, failure */ - void finished(BaseJob* job); + void finished(Quotient::BaseJob* job); /** * Emitted when the job is finished (except when abandoned). @@ -262,14 +263,14 @@ signals: * * @see success, failure */ - void result(BaseJob* job); + void result(Quotient::BaseJob* job); /** * Emitted together with result() in case there's no error. * * @see result, failure */ - void success(BaseJob*); + void success(Quotient::BaseJob*); /** * Emitted together with result() if there's an error. @@ -277,7 +278,7 @@ signals: * * @see result, success */ - void failure(BaseJob*); + void failure(Quotient::BaseJob*); void downloadProgress(qint64 bytesReceived, qint64 bytesTotal); void uploadProgress(qint64 bytesSent, qint64 bytesTotal); diff --git a/lib/room.h b/lib/room.h index cded7eb9..80e305f0 100644 --- a/lib/room.h +++ b/lib/room.h @@ -181,11 +181,11 @@ public: QString avatarMediaId() const; QUrl avatarUrl() const; const Avatar& avatarObject() const; - Q_INVOKABLE JoinState joinState() const; - Q_INVOKABLE QList usersTyping() const; + Q_INVOKABLE Quotient::JoinState joinState() const; + Q_INVOKABLE QList usersTyping() const; QList membersLeft() const; - Q_INVOKABLE QList users() const; + Q_INVOKABLE QList users() const; QStringList memberNames() const; [[deprecated("Use joinedCount(), invitedCount(), totalMemberCount()")]] int memberCount() const; @@ -228,7 +228,7 @@ public: * \note The method will return a valid user regardless of * the membership. */ - Q_INVOKABLE User* user(const QString& userId) const; + Q_INVOKABLE Quotient::User* user(const QString& userId) const; /** * \brief Check the join state of a given user in this room @@ -236,16 +236,15 @@ public: * \note Banned and invited users are not tracked for now (Leave * will be returned for them). * - * \return either of Join, Leave, depending on the given - * user's state in the room + * \return Join if the user is a room member; Leave otherwise */ - Q_INVOKABLE JoinState memberJoinState(User* user) const; + Q_INVOKABLE Quotient::JoinState memberJoinState(Quotient::User* user) const; /** * Get a disambiguated name for a given user in * the context of the room */ - Q_INVOKABLE QString roomMembername(const User* u) const; + Q_INVOKABLE QString roomMembername(const Quotient::User* u) const; /** * Get a disambiguated name for a user with this id in * the context of the room @@ -274,9 +273,10 @@ public: Timeline::const_iterator syncEdge() const; /// \deprecated Use historyEdge instead rev_iter_t timelineEdge() const; - Q_INVOKABLE TimelineItem::index_t minTimelineIndex() const; - Q_INVOKABLE TimelineItem::index_t maxTimelineIndex() const; - Q_INVOKABLE bool isValidIndex(TimelineItem::index_t timelineIndex) const; + Q_INVOKABLE Quotient::TimelineItem::index_t minTimelineIndex() const; + Q_INVOKABLE Quotient::TimelineItem::index_t maxTimelineIndex() const; + Q_INVOKABLE bool + isValidIndex(Quotient::TimelineItem::index_t timelineIndex) const; rev_iter_t findInTimeline(TimelineItem::index_t index) const; rev_iter_t findInTimeline(const QString& evtId) const; @@ -414,7 +414,8 @@ public: * the event is even sent), while downloads are using * the normal event id for identifier. */ - Q_INVOKABLE FileTransferInfo fileTransferInfo(const QString& id) const; + Q_INVOKABLE Quotient::FileTransferInfo + fileTransferInfo(const QString& id) const; /// Get the URL to the actual file source in a unified way /*! @@ -438,7 +439,7 @@ public: /*! This method returns a (potentially empty) state event corresponding * to the pair of event type \p evtType and state key \p stateKey. */ - Q_INVOKABLE const StateEventBase* + Q_INVOKABLE const Quotient::StateEventBase* getCurrentState(const QString& evtType, const QString& stateKey = {}) const; template @@ -549,7 +550,8 @@ signals: /// The remote echo has arrived with the sync and will be merged /// with its local counterpart /** NB: Requires a sync loop to be emitted */ - void pendingEventAboutToMerge(RoomEvent* serverEvent, int pendingEventIndex); + void pendingEventAboutToMerge(Quotient::RoomEvent* serverEvent, + int pendingEventIndex); /// The remote and local copies of the event have been merged /** NB: Requires a sync loop to be emitted */ void pendingEventMerged(); @@ -577,21 +579,21 @@ signals: * upon the last sync * \sa Changes */ - void changed(Changes changes); + void changed(Quotient::Room::Changes changes); /** * \brief The room name, the canonical alias or other aliases changed * * Not triggered when display name changes. */ - void namesChanged(Room* room); - void displaynameAboutToChange(Room* room); - void displaynameChanged(Room* room, QString oldName); + void namesChanged(Quotient::Room* room); + void displaynameAboutToChange(Quotient::Room* room); + void displaynameChanged(Quotient::Room* room, QString oldName); void topicChanged(); void avatarChanged(); - void userAdded(User* user); - void userRemoved(User* user); - void memberAboutToRename(User* user, QString newName); - void memberRenamed(User* user); + void userAdded(Quotient::User* user); + void userRemoved(Quotient::User* user); + void memberAboutToRename(Quotient::User* user, QString newName); + void memberRenamed(Quotient::User* user); /// The list of members has changed /** Emitted no more than once per sync, this is a good signal to * for cases when some action should be done upon any change in @@ -605,7 +607,8 @@ signals: void allMembersLoaded(); void encryption(); - void joinStateChanged(JoinState oldState, JoinState newState); + void joinStateChanged(Quotient::JoinState oldState, + Quotient::JoinState newState); void typingChanged(); void highlightCountChanged(); @@ -614,11 +617,11 @@ signals: void displayedChanged(bool displayed); void firstDisplayedEventChanged(); void lastDisplayedEventChanged(); - void lastReadEventChanged(User* user); + void lastReadEventChanged(Quotient::User* user); void readMarkerMoved(QString fromEventId, QString toEventId); - void readMarkerForUserMoved(User* user, QString fromEventId, + void readMarkerForUserMoved(Quotient::User* user, QString fromEventId, QString toEventId); - void unreadMessagesChanged(Room* room); + void unreadMessagesChanged(Quotient::Room* room); void accountDataAboutToChange(QString type); void accountDataChanged(QString type); @@ -626,7 +629,8 @@ signals: void tagsChanged(); void updatedEvent(QString eventId); - void replacedEvent(const RoomEvent* newEvent, const RoomEvent* oldEvent); + void replacedEvent(const Quotient::RoomEvent* newEvent, + const Quotient::RoomEvent* oldEvent); void newFileTransfer(QString id, QUrl localFile); void fileTransferProgress(QString id, qint64 progress, qint64 total); @@ -634,18 +638,18 @@ signals: void fileTransferFailed(QString id, QString errorMessage = {}); void fileTransferCancelled(QString id); - void callEvent(Room* room, const RoomEvent* event); + void callEvent(Quotient::Room* room, const Quotient::RoomEvent* event); /// The room's version stability may have changed void stabilityUpdated(QString recommendedDefault, QStringList stableVersions); /// This room has been upgraded and won't receive updates any more - void upgraded(QString serverMessage, Room* successor); + void upgraded(QString serverMessage, Quotient::Room* successor); /// An attempted room upgrade has failed void upgradeFailed(QString errorMessage); /// The room is about to be deleted - void beforeDestruction(Room*); + void beforeDestruction(Quotient::Room*); protected: virtual Changes processStateEvent(const RoomEvent& e); diff --git a/lib/user.h b/lib/user.h index c9e3dbc1..28ec841b 100644 --- a/lib/user.h +++ b/lib/user.h @@ -107,9 +107,10 @@ public: qreal hueF() const; const Avatar& avatarObject(const Room* room = nullptr) const; - Q_INVOKABLE QImage avatar(int dimension, const Room* room = nullptr); + Q_INVOKABLE QImage avatar(int dimension, + const Quotient::Room* room = nullptr); Q_INVOKABLE QImage avatar(int requestedWidth, int requestedHeight, - const Room* room = nullptr); + const Quotient::Room* room = nullptr); QImage avatar(int width, int height, const Room* room, const Avatar::get_callback_t& callback); @@ -145,9 +146,10 @@ public slots: signals: void nameAboutToChange(QString newName, QString oldName, - const Room* roomContext); - void nameChanged(QString newName, QString oldName, const Room* roomContext); - void avatarChanged(User* user, const Room* roomContext); + const Quotient::Room* roomContext); + void nameChanged(QString newName, QString oldName, + const Quotient::Room* roomContext); + void avatarChanged(Quotient::User* user, const Quotient::Room* roomContext); private slots: void updateName(const QString& newName, const Room* room = nullptr); @@ -161,4 +163,3 @@ private: QScopedPointer d; }; } // namespace Quotient -Q_DECLARE_METATYPE(Quotient::User*) -- cgit v1.2.3 From c8ae34af0c4b28ef5aabf60b97661241860bc200 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 21 Oct 2019 09:41:46 +0900 Subject: ConnectionData: fix read-after-free in clearing the job queue --- lib/connectiondata.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/connectiondata.cpp b/lib/connectiondata.cpp index a3807fc4..cbc36420 100644 --- a/lib/connectiondata.cpp +++ b/lib/connectiondata.cpp @@ -67,7 +67,6 @@ ConnectionData::ConnectionData(QUrl baseUrl) for (auto& q : d->jobs) while (!q.empty()) { auto& job = q.front(); - q.pop(); if (!job || job->error() == BaseJob::Abandoned) continue; if (job->error() != BaseJob::Pending) { @@ -79,6 +78,7 @@ ConnectionData::ConnectionData(QUrl baseUrl) } job->sendRequest(); d->rateLimiter.start(); + q.pop(); return; } qCDebug(MAIN) << d->id() << "job queues are empty"; -- cgit v1.2.3 From 693beec0005bdd98732ad8b4ad760f9de4a9faee Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 15 Oct 2019 19:33:21 +0900 Subject: Connection: make syncLoop() reentrant ...in the sense that you can call it twice and expect the second invocation to be gracefully ignored rather than two loops conflicting with each other. --- lib/connection.cpp | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) (limited to 'lib') diff --git a/lib/connection.cpp b/lib/connection.cpp index af85d066..a7b1bee9 100644 --- a/lib/connection.cpp +++ b/lib/connection.cpp @@ -99,6 +99,7 @@ public: DirectChatsMap dcLocalAdditions; DirectChatsMap dcLocalRemovals; UnorderedMap accountData; + QMetaObject::Connection syncLoopConnection {}; int syncLoopTimeout = -1; GetCapabilitiesJob* capabilitiesJob = nullptr; @@ -380,9 +381,20 @@ void Connection::sync(int timeout) void Connection::syncLoop(int timeout) { - d->syncLoopTimeout = timeout; - connect(this, &Connection::syncDone, this, &Connection::syncLoopIteration); - syncLoopIteration(); // initial sync to start the loop + if (d->syncLoopConnection && d->syncLoopTimeout == timeout) { + qCInfo(MAIN) << "Attempt to run sync loop but there's one already " + "running; nothing will be done"; + return; + } + std::swap(d->syncLoopTimeout, timeout); + if (d->syncLoopConnection) { + qCWarning(MAIN) << "Overriding timeout of the running sync loop from" + << timeout << "to" << d->syncLoopTimeout; + } else { + d->syncLoopConnection = connect(this, &Connection::syncDone, + this, &Connection::syncLoopIteration); + syncLoopIteration(); // initial sync to start the loop + } } QJsonObject toJson(const DirectChatsMap& directChats) @@ -514,8 +526,7 @@ void Connection::onSyncSuccess(SyncData&& data, bool fromCache) void Connection::stopSync() { // If there's a sync loop, break it - disconnect(this, &Connection::syncDone, this, - &Connection::syncLoopIteration); + disconnect(d->syncLoopConnection); if (d->syncJob) // If there's an ongoing sync job, stop it too { d->syncJob->abandon(); -- cgit v1.2.3 From ff77965cb92e94061345e4e955f1d7289de2597a Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 21 Oct 2019 15:58:12 +0900 Subject: Connection: record sync timeout; suspend sync before logout This is mostly internal but clients may see fewer spurious sync failures upon logging out. --- lib/connection.cpp | 39 ++++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-) (limited to 'lib') diff --git a/lib/connection.cpp b/lib/connection.cpp index a7b1bee9..3d617733 100644 --- a/lib/connection.cpp +++ b/lib/connection.cpp @@ -100,7 +100,7 @@ public: DirectChatsMap dcLocalRemovals; UnorderedMap accountData; QMetaObject::Connection syncLoopConnection {}; - int syncLoopTimeout = -1; + int syncTimeout = -1; GetCapabilitiesJob* capabilitiesJob = nullptr; GetCapabilitiesJob::Capabilities capabilities; @@ -260,8 +260,6 @@ void Connection::doConnectToServer(const QString& user, const QString& password, }); } -void Connection::syncLoopIteration() { sync(d->syncLoopTimeout); } - void Connection::connectWithToken(const QString& userId, const QString& accessToken, const QString& deviceId) @@ -336,22 +334,34 @@ void Connection::checkAndConnect(const QString& userId, void Connection::logout() { - auto job = callApi(); - connect(job, &LogoutJob::finished, this, [job, this] { + // If there's an ongoing sync job, stop it but don't break the sync loop yet + const auto syncWasRunning = bool(d->syncJob); + if (syncWasRunning) + { + d->syncJob->abandon(); + d->syncJob = nullptr; + } + const auto* job = callApi(); + connect(job, &LogoutJob::finished, this, [this, job, syncWasRunning] { if (job->status().good() || job->error() == BaseJob::ContentAccessError) { - stopSync(); + if (d->syncLoopConnection) + disconnect(d->syncLoopConnection); d->data->setToken({}); emit stateChanged(); emit loggedOut(); - } + } else if (syncWasRunning) + syncLoopIteration(); // Resume sync loop (or a single sync) }); } void Connection::sync(int timeout) { - if (d->syncJob) + if (d->syncJob) { + qCInfo(MAIN) << d->syncJob << "is already running"; return; + } + d->syncTimeout = timeout; Filter filter; filter.room->timeline->limit = 100; filter.room->state->lazyLoadMembers = d->lazyLoading; @@ -381,22 +391,25 @@ void Connection::sync(int timeout) void Connection::syncLoop(int timeout) { - if (d->syncLoopConnection && d->syncLoopTimeout == timeout) { + if (d->syncLoopConnection && d->syncTimeout == timeout) { qCInfo(MAIN) << "Attempt to run sync loop but there's one already " "running; nothing will be done"; return; } - std::swap(d->syncLoopTimeout, timeout); + std::swap(d->syncTimeout, timeout); if (d->syncLoopConnection) { - qCWarning(MAIN) << "Overriding timeout of the running sync loop from" - << timeout << "to" << d->syncLoopTimeout; + qCInfo(MAIN) << "Timeout for next syncs changed from" + << timeout << "to" << d->syncTimeout; } else { d->syncLoopConnection = connect(this, &Connection::syncDone, - this, &Connection::syncLoopIteration); + this, &Connection::syncLoopIteration, + Qt::QueuedConnection); syncLoopIteration(); // initial sync to start the loop } } +void Connection::syncLoopIteration() { sync(d->syncTimeout); } + QJsonObject toJson(const DirectChatsMap& directChats) { QJsonObject json; -- cgit v1.2.3 From 60bb1cf942ad0815dcf42cbfe8acd1e076d848cf Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 29 Oct 2019 22:04:40 +0900 Subject: Derive Omittable<> from std::optional<> That breaks API all over the place but: 1. The fixes are trivial. 2. More of std:: is used instead of home-baking the same stuff. --- CMakeLists.txt | 1 + lib/connection.cpp | 30 +++--- lib/converters.h | 24 +---- lib/csapi/search.cpp | 191 ------------------------------------- lib/csapi/search.h | 203 ---------------------------------------- lib/events/accountdataevents.h | 10 +- lib/events/roommessageevent.cpp | 23 +++-- lib/events/roommessageevent.h | 2 +- lib/room.cpp | 11 +-- lib/syncdata.cpp | 15 ++- lib/util.h | 108 +++++++-------------- 11 files changed, 82 insertions(+), 536 deletions(-) delete mode 100644 lib/csapi/search.cpp delete mode 100644 lib/csapi/search.h (limited to 'lib') diff --git a/CMakeLists.txt b/CMakeLists.txt index 58509eae..11d7d194 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -193,6 +193,7 @@ if (MATRIX_DOC_PATH AND GTAD_PATH) ${ABS_GTAD_PATH} --config ${CSAPI_DIR}/gtad.yaml --out ${CSAPI_DIR} ${FULL_CSAPI_SRC_DIR} old_sync.yaml- room_initial_sync.yaml- # deprecated + search.yaml- # current GTAD is limited in handling move-only data sync.yaml- # we have a better handcrafted implementation WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}/lib SOURCES ${FULL_CSAPI_DIR}/gtad.yaml diff --git a/lib/connection.cpp b/lib/connection.cpp index 3d617733..47c643b0 100644 --- a/lib/connection.cpp +++ b/lib/connection.cpp @@ -277,16 +277,16 @@ void Connection::reloadCapabilities() else if (d->capabilitiesJob->error() == BaseJob::IncorrectRequestError) qCDebug(MAIN) << "Server doesn't support /capabilities"; - if (d->capabilities.roomVersions.omitted()) { + if (!d->capabilities.roomVersions) { qCWarning(MAIN) << "Pinning supported room version to 1"; - d->capabilities.roomVersions = { "1", { { "1", "stable" } } }; + d->capabilities.roomVersions.emplace({ "1", { { "1", "stable" } } }); } else { qCDebug(MAIN) << "Room versions:" << defaultRoomVersion() << "is default, full list:" << availableRoomVersions(); } - Q_ASSERT(!d->capabilities.roomVersions.omitted()); + Q_ASSERT(d->capabilities.roomVersions.has_value()); emit capabilitiesLoaded(); - for (auto* r : d->roomMap) + for (auto* r : qAsConst(d->roomMap)) r->checkVersion(); }); } @@ -295,7 +295,7 @@ bool Connection::loadingCapabilities() const { // (Ab)use the fact that room versions cannot be omitted after // the capabilities have been loaded (see reloadCapabilities() above). - return d->capabilities.roomVersions.omitted(); + return !d->capabilities.roomVersions; } void Connection::Private::connectWithToken(const QString& userId, @@ -363,8 +363,8 @@ void Connection::sync(int timeout) d->syncTimeout = timeout; Filter filter; - filter.room->timeline->limit = 100; - filter.room->state->lazyLoadMembers = d->lazyLoading; + filter.room.edit().timeline.edit().limit.emplace(100); + filter.room.edit().state.edit().lazyLoadMembers.emplace(d->lazyLoading); auto job = d->syncJob = callApi(BackgroundRequest, d->data->lastEvent(), filter, timeout); @@ -446,7 +446,7 @@ void Connection::onSyncSuccess(SyncData&& data, bool fromCache) r->updateData(std::move(roomData), fromCache); if (d->firstTimeRooms.removeOne(r)) { emit loadedRoomState(r); - if (!d->capabilities.roomVersions.omitted()) + if (d->capabilities.roomVersions) r->checkVersion(); // Otherwise, the version will be checked in reloadCapabilities() } @@ -1191,7 +1191,7 @@ Room* Connection::provideRoom(const QString& id, Omittable joinState) // and emit a signal. For Invite and Join, there's no such problem. if (room->joinState() == joinState && joinState != JoinState::Leave) return room; - } else if (joinState.omitted()) { + } else if (!joinState) { // No Join and Leave, maybe Invite? room = d->roomMap.value({ id, true }, nullptr); if (room) @@ -1200,9 +1200,7 @@ Room* Connection::provideRoom(const QString& id, Omittable joinState) } if (!room) { - room = roomFactory()(this, id, - joinState.omitted() ? JoinState::Join - : joinState.value()); + room = roomFactory()(this, id, joinState.value_or(JoinState::Join)); if (!room) { qCCritical(MAIN) << "Failed to create a room" << id; return nullptr; @@ -1213,7 +1211,7 @@ Room* Connection::provideRoom(const QString& id, Omittable joinState) &Connection::aboutToDeleteRoom); emit newRoom(room); } - if (joinState.omitted()) + if (!joinState) return room; if (joinState == JoinState::Invite) { @@ -1431,13 +1429,13 @@ const QString Connection::SupportedRoomVersion::StableTag = QString Connection::defaultRoomVersion() const { - Q_ASSERT(!d->capabilities.roomVersions.omitted()); + Q_ASSERT(d->capabilities.roomVersions.has_value()); return d->capabilities.roomVersions->defaultVersion; } QStringList Connection::stableRoomVersions() const { - Q_ASSERT(!d->capabilities.roomVersions.omitted()); + Q_ASSERT(d->capabilities.roomVersions.has_value()); QStringList l; const auto& allVersions = d->capabilities.roomVersions->available; for (auto it = allVersions.begin(); it != allVersions.end(); ++it) @@ -1457,7 +1455,7 @@ inline bool roomVersionLess(const Connection::SupportedRoomVersion& v1, QVector Connection::availableRoomVersions() const { - Q_ASSERT(!d->capabilities.roomVersions.omitted()); + Q_ASSERT(d->capabilities.roomVersions.has_value()); QVector result; result.reserve(d->capabilities.roomVersions->available.size()); for (auto it = d->capabilities.roomVersions->available.begin(); diff --git a/lib/converters.h b/lib/converters.h index b753a80b..157bff27 100644 --- a/lib/converters.h +++ b/lib/converters.h @@ -206,7 +206,7 @@ template struct JsonConverter> { static QJsonValue dump(const Omittable& from) { - return from.omitted() ? QJsonValue() : toJson(from.value()); + return from.has_value() ? toJson(from.value()) : QJsonValue(); } static Omittable load(const QJsonValue& jv) { @@ -378,28 +378,10 @@ namespace _impl { static void impl(ContT& container, const QString& key, const OmittableT& value) { - if (!value.omitted()) - addTo(container, key, value.value()); + if (value) + addTo(container, key, *value); } }; - -#if 0 - // This is a special one that unfolds optional<> - template - struct AddNode, Force> - { - template - static void impl(ContT& container, - const QString& key, const OptionalT& value) - { - if (value) - AddNode::impl(container, key, value.value()); - else if (Force) // Edge case, no value but must put something - AddNode::impl(container, key, QString{}); - } - }; -#endif - } // namespace _impl static constexpr bool IfNotEmpty = false; diff --git a/lib/csapi/search.cpp b/lib/csapi/search.cpp deleted file mode 100644 index 9619f340..00000000 --- a/lib/csapi/search.cpp +++ /dev/null @@ -1,191 +0,0 @@ -/****************************************************************************** - * THIS FILE IS GENERATED - ANY EDITS WILL BE OVERWRITTEN - */ - -#include "search.h" - -#include "converters.h" - -#include - -using namespace Quotient; - -static const auto basePath = QStringLiteral("/_matrix/client/r0"); - -// Converters -namespace Quotient -{ - -template <> -struct JsonObjectConverter -{ - static void dumpTo(QJsonObject& jo, - const SearchJob::IncludeEventContext& pod) - { - addParam(jo, QStringLiteral("before_limit"), - pod.beforeLimit); - addParam(jo, QStringLiteral("after_limit"), pod.afterLimit); - addParam(jo, QStringLiteral("include_profile"), - pod.includeProfile); - } -}; - -template <> -struct JsonObjectConverter -{ - static void dumpTo(QJsonObject& jo, const SearchJob::Group& pod) - { - addParam(jo, QStringLiteral("key"), pod.key); - } -}; - -template <> -struct JsonObjectConverter -{ - static void dumpTo(QJsonObject& jo, const SearchJob::Groupings& pod) - { - addParam(jo, QStringLiteral("group_by"), pod.groupBy); - } -}; - -template <> -struct JsonObjectConverter -{ - static void dumpTo(QJsonObject& jo, const SearchJob::RoomEventsCriteria& pod) - { - addParam<>(jo, QStringLiteral("search_term"), pod.searchTerm); - addParam(jo, QStringLiteral("keys"), pod.keys); - addParam(jo, QStringLiteral("filter"), pod.filter); - addParam(jo, QStringLiteral("order_by"), pod.orderBy); - addParam(jo, QStringLiteral("event_context"), - pod.eventContext); - addParam(jo, QStringLiteral("include_state"), - pod.includeState); - addParam(jo, QStringLiteral("groupings"), pod.groupings); - } -}; - -template <> -struct JsonObjectConverter -{ - static void dumpTo(QJsonObject& jo, const SearchJob::Categories& pod) - { - addParam(jo, QStringLiteral("room_events"), pod.roomEvents); - } -}; - -template <> -struct JsonObjectConverter -{ - static void fillFrom(const QJsonObject& jo, SearchJob::UserProfile& result) - { - fromJson(jo.value("displayname"_ls), result.displayname); - fromJson(jo.value("avatar_url"_ls), result.avatarUrl); - } -}; - -template <> -struct JsonObjectConverter -{ - static void fillFrom(const QJsonObject& jo, SearchJob::EventContext& result) - { - fromJson(jo.value("start"_ls), result.begin); - fromJson(jo.value("end"_ls), result.end); - fromJson(jo.value("profile_info"_ls), result.profileInfo); - fromJson(jo.value("events_before"_ls), result.eventsBefore); - fromJson(jo.value("events_after"_ls), result.eventsAfter); - } -}; - -template <> -struct JsonObjectConverter -{ - static void fillFrom(const QJsonObject& jo, SearchJob::Result& result) - { - fromJson(jo.value("rank"_ls), result.rank); - fromJson(jo.value("result"_ls), result.result); - fromJson(jo.value("context"_ls), result.context); - } -}; - -template <> -struct JsonObjectConverter -{ - static void fillFrom(const QJsonObject& jo, SearchJob::GroupValue& result) - { - fromJson(jo.value("next_batch"_ls), result.nextBatch); - fromJson(jo.value("order"_ls), result.order); - fromJson(jo.value("results"_ls), result.results); - } -}; - -template <> -struct JsonObjectConverter -{ - static void fillFrom(const QJsonObject& jo, - SearchJob::ResultRoomEvents& result) - { - fromJson(jo.value("count"_ls), result.count); - fromJson(jo.value("highlights"_ls), result.highlights); - fromJson(jo.value("results"_ls), result.results); - fromJson(jo.value("state"_ls), result.state); - fromJson(jo.value("groups"_ls), result.groups); - fromJson(jo.value("next_batch"_ls), result.nextBatch); - } -}; - -template <> -struct JsonObjectConverter -{ - static void fillFrom(const QJsonObject& jo, - SearchJob::ResultCategories& result) - { - fromJson(jo.value("room_events"_ls), result.roomEvents); - } -}; - -} // namespace Quotient - -class SearchJob::Private -{ -public: - ResultCategories searchCategories; -}; - -BaseJob::Query queryToSearch(const QString& nextBatch) -{ - BaseJob::Query _q; - addParam(_q, QStringLiteral("next_batch"), nextBatch); - return _q; -} - -static const auto SearchJobName = QStringLiteral("SearchJob"); - -SearchJob::SearchJob(const Categories& searchCategories, - const QString& nextBatch) - : BaseJob(HttpVerb::Post, SearchJobName, basePath % "/search", - queryToSearch(nextBatch)) - , d(new Private) -{ - QJsonObject _data; - addParam<>(_data, QStringLiteral("search_categories"), searchCategories); - setRequestData(_data); -} - -SearchJob::~SearchJob() = default; - -const SearchJob::ResultCategories& SearchJob::searchCategories() const -{ - return d->searchCategories; -} - -BaseJob::Status SearchJob::parseJson(const QJsonDocument& data) -{ - auto json = data.object(); - if (!json.contains("search_categories"_ls)) - return { IncorrectResponse, - "The key 'search_categories' not found in the response" }; - fromJson(json.value("search_categories"_ls), d->searchCategories); - - return Success; -} diff --git a/lib/csapi/search.h b/lib/csapi/search.h deleted file mode 100644 index 079ac8e9..00000000 --- a/lib/csapi/search.h +++ /dev/null @@ -1,203 +0,0 @@ -/****************************************************************************** - * THIS FILE IS GENERATED - ANY EDITS WILL BE OVERWRITTEN - */ - -#pragma once - -#include "converters.h" - -#include "csapi/definitions/room_event_filter.h" - -#include "events/eventloader.h" -#include "jobs/basejob.h" - -#include -#include - -#include - -namespace Quotient -{ - -// Operations - -/// Perform a server-side search. -/*! - * Performs a full text search across different categories. - */ -class SearchJob : public BaseJob -{ -public: - // Inner data structures - - /// Configures whether any context for the eventsreturned are included in - /// the response. - struct IncludeEventContext - { - /// How many events before the result arereturned. By default, this is - /// ``5``. - Omittable beforeLimit; - /// How many events after the result arereturned. By default, this is - /// ``5``. - Omittable afterLimit; - /// Requests that the server returns thehistoric profile information for - /// the usersthat sent the events that were returned.By default, this is - /// ``false``. - Omittable includeProfile; - }; - - /// Configuration for group. - struct Group - { - /// Key that defines the group. - QString key; - }; - - /// Requests that the server partitions the result setbased on the provided - /// list of keys. - struct Groupings - { - /// List of groups to request. - QVector groupBy; - }; - - /// Mapping of category name to search criteria. - struct RoomEventsCriteria - { - /// The string to search events for - QString searchTerm; - /// The keys to search. Defaults to all. - QStringList keys; - /// This takes a `filter`_. - Omittable filter; - /// The order in which to search for results.By default, this is - /// ``"rank"``. - QString orderBy; - /// Configures whether any context for the eventsreturned are included - /// in the response. - Omittable eventContext; - /// Requests the server return the current state foreach room returned. - Omittable includeState; - /// Requests that the server partitions the result setbased on the - /// provided list of keys. - Omittable groupings; - }; - - /// Describes which categories to search in and their criteria. - struct Categories - { - /// Mapping of category name to search criteria. - Omittable roomEvents; - }; - - /// Performs a full text search across different categories. - struct UserProfile - { - /// Performs a full text search across different categories. - QString displayname; - /// Performs a full text search across different categories. - QString avatarUrl; - }; - - /// Context for result, if requested. - struct EventContext - { - /// Pagination token for the start of the chunk - QString begin; - /// Pagination token for the end of the chunk - QString end; - /// The historic profile information of theusers that sent the events - /// returned.The ``string`` key is the user ID for whichthe profile - /// belongs to. - QHash profileInfo; - /// Events just before the result. - RoomEvents eventsBefore; - /// Events just after the result. - RoomEvents eventsAfter; - }; - - /// The result object. - struct Result - { - /// A number that describes how closely this result matches the search. - /// Higher is closer. - Omittable rank; - /// The event that matched. - RoomEventPtr result; - /// Context for result, if requested. - Omittable context; - }; - - /// The results for a particular group value. - struct GroupValue - { - /// Token that can be used to get the next batchof results in the group, - /// by passing as the`next_batch` parameter to the next call. Ifthis - /// field is absent, there are no moreresults in this group. - QString nextBatch; - /// Key that can be used to order differentgroups. - Omittable order; - /// Which results are in this group. - QStringList results; - }; - - /// Mapping of category name to search criteria. - struct ResultRoomEvents - { - /// An approximate count of the total number of results found. - Omittable count; - /// List of words which should be highlighted, useful for stemming which - /// may change the query terms. - QStringList highlights; - /// List of results in the requested order. - std::vector results; - /// The current state for every room in the results.This is included if - /// the request had the``include_state`` key set with a value of - /// ``true``.The ``string`` key is the room ID for which the - /// ``StateEvent`` array belongs to. - std::unordered_map state; - /// Any groups that were requested.The outer ``string`` key is the group - /// key requested (eg: ``room_id``or ``sender``). The inner ``string`` - /// key is the grouped value (eg: a room's ID or a user's ID). - QHash> groups; - /// Token that can be used to get the next batch ofresults, by passing - /// as the `next_batch` parameter tothe next call. If this field is - /// absent, there are nomore results. - QString nextBatch; - }; - - /// Describes which categories to search in and their criteria. - struct ResultCategories - { - /// Mapping of category name to search criteria. - Omittable roomEvents; - }; - - // Construction/destruction - - /*! Perform a server-side search. - * \param searchCategories - * Describes which categories to search in and their criteria. - * \param nextBatch - * The point to return events from. If given, this should be a - * ``next_batch`` result from a previous call to this endpoint. - */ - explicit SearchJob(const Categories& searchCategories, - const QString& nextBatch = {}); - - ~SearchJob() override; - - // Result properties - - /// Describes which categories to search in and their criteria. - const ResultCategories& searchCategories() const; - -protected: - Status parseJson(const QJsonDocument& data) override; - -private: - class Private; - QScopedPointer d; -}; - -} // namespace Quotient diff --git a/lib/events/accountdataevents.h b/lib/events/accountdataevents.h index 31176766..a55016d9 100644 --- a/lib/events/accountdataevents.h +++ b/lib/events/accountdataevents.h @@ -1,5 +1,3 @@ -#include - /****************************************************************************** * Copyright (C) 2018 Kitsune Ral * @@ -34,13 +32,13 @@ struct TagRecord { order_type order; - TagRecord(order_type order = none) : order(order) {} + TagRecord(order_type order = none) : order(std::move(order)) {} bool operator<(const TagRecord& other) const { - // Per The Spec, rooms with no order should be after those with order - return !order.omitted() - && (other.order.omitted() || order.value() < other.order.value()); + // Per The Spec, rooms with no order should be after those with order, + // against optional<>::operator<() convention. + return order && (!other.order || *order < *other.order); } }; diff --git a/lib/events/roommessageevent.cpp b/lib/events/roommessageevent.cpp index 09562d65..078ae70a 100644 --- a/lib/events/roommessageevent.cpp +++ b/lib/events/roommessageevent.cpp @@ -95,6 +95,11 @@ MsgType jsonToMsgType(const QString& matrixType) return MsgType::Unknown; } +inline bool isReplacement(const Omittable& rel) +{ + return rel && rel->type == RelatesTo::ReplacementTypeId(); +} + QJsonObject RoomMessageEvent::assembleContentJson(const QString& plainBody, const QString& jsonMsgType, TypedBase* content) @@ -111,6 +116,7 @@ QJsonObject RoomMessageEvent::assembleContentJson(const QString& plainBody, // After the above, we know for sure that the content is TextContent // and that its RelatesTo structure is not omitted auto* textContent = static_cast(content); + Q_ASSERT(textContent && textContent->relatesTo.has_value()); if (textContent->relatesTo->type == RelatesTo::ReplacementTypeId()) { auto newContentJson = json.take("m.new_content"_ls).toObject(); newContentJson.insert(BodyKey, plainBody); @@ -243,9 +249,7 @@ QString RoomMessageEvent::replacedEvent() const return {}; const auto& rel = static_cast(content())->relatesTo; - return !rel.omitted() && rel->type == RelatesTo::ReplacementTypeId() - ? rel->eventId - : QString(); + return isReplacement(rel) ? rel->eventId : QString(); } QString rawMsgTypeForMimeType(const QMimeType& mimeType) @@ -269,10 +273,10 @@ QString RoomMessageEvent::rawMsgTypeForFile(const QFileInfo& fi) return rawMsgTypeForMimeType(QMimeDatabase().mimeTypeForFile(fi)); } -TextContent::TextContent(const QString& text, const QString& contentType, +TextContent::TextContent(QString text, const QString& contentType, Omittable relatesTo) : mimeType(QMimeDatabase().mimeTypeForName(contentType)) - , body(text) + , body(std::move(text)) , relatesTo(std::move(relatesTo)) { if (contentType == HtmlContentTypeId) @@ -304,10 +308,9 @@ TextContent::TextContent(const QJsonObject& json) static const auto PlainTextMimeType = db.mimeTypeForName("text/plain"); static const auto HtmlMimeType = db.mimeTypeForName("text/html"); - const auto actualJson = - relatesTo.omitted() || relatesTo->type != RelatesTo::ReplacementTypeId() - ? json - : json.value("m.new_content"_ls).toObject(); + const auto actualJson = isReplacement(relatesTo) + ? json.value("m.new_content"_ls).toObject() + : json; // Special-casing the custom matrix.org's (actually, Riot's) way // of sending HTML messages. if (actualJson["format"_ls].toString() == HtmlContentTypeId) { @@ -331,7 +334,7 @@ void TextContent::fillJson(QJsonObject* json) const json->insert(FormatKey, HtmlContentTypeId); json->insert(FormattedBodyKey, body); } - if (!relatesTo.omitted()) { + if (relatesTo) { json->insert(QStringLiteral("m.relates_to"), QJsonObject { { relatesTo->type, relatesTo->eventId } }); if (relatesTo->type == RelatesTo::ReplacementTypeId()) { diff --git a/lib/events/roommessageevent.h b/lib/events/roommessageevent.h index e95aabfc..ded5e572 100644 --- a/lib/events/roommessageevent.h +++ b/lib/events/roommessageevent.h @@ -114,7 +114,7 @@ namespace EventContent { */ class TextContent : public TypedBase { public: - TextContent(const QString& text, const QString& contentType, + TextContent(QString text, const QString& contentType, Omittable relatesTo = none); explicit TextContent(const QJsonObject& json); diff --git a/lib/room.cpp b/lib/room.cpp index c4cdac04..b34c36ea 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -1201,15 +1201,13 @@ QString Room::decryptMessage(QByteArray cipher, const QString& senderKey, int Room::joinedCount() const { - return d->summary.joinedMemberCount.omitted() - ? d->membersMap.size() - : d->summary.joinedMemberCount.value(); + return d->summary.joinedMemberCount.value_or(d->membersMap.size()); } int Room::invitedCount() const { // TODO: Store invited users in Room too - Q_ASSERT(!d->summary.invitedMemberCount.omitted()); + Q_ASSERT(d->summary.invitedMemberCount.has_value()); return d->summary.invitedMemberCount.value(); } @@ -2620,9 +2618,8 @@ QString Room::Private::calculateDisplayname() const const bool emptyRoom = membersMap.isEmpty() || (membersMap.size() == 1 && isLocalUser(*membersMap.begin())); - const bool nonEmptySummary = - !summary.heroes.omitted() && !summary.heroes->empty(); - auto shortlist = nonEmptySummary ? buildShortlist(summary.heroes.value()) + const bool nonEmptySummary = summary.heroes && !summary.heroes->empty(); + auto shortlist = nonEmptySummary ? buildShortlist(*summary.heroes) : !emptyRoom ? buildShortlist(membersMap) : users_shortlist_t {}; diff --git a/lib/syncdata.cpp b/lib/syncdata.cpp index 5b47b30f..89c512a2 100644 --- a/lib/syncdata.cpp +++ b/lib/syncdata.cpp @@ -30,8 +30,7 @@ const QString SyncRoomData::UnreadCountKey = bool RoomSummary::isEmpty() const { - return joinedMemberCount.omitted() && invitedMemberCount.omitted() - && heroes.omitted(); + return !joinedMemberCount && !invitedMemberCount && !heroes; } bool RoomSummary::merge(const RoomSummary& other) @@ -46,12 +45,12 @@ QDebug Quotient::operator<<(QDebug dbg, const RoomSummary& rs) { QDebugStateSaver _(dbg); QStringList sl; - if (!rs.joinedMemberCount.omitted()) - sl << QStringLiteral("joined: %1").arg(rs.joinedMemberCount.value()); - if (!rs.invitedMemberCount.omitted()) - sl << QStringLiteral("invited: %1").arg(rs.invitedMemberCount.value()); - if (!rs.heroes.omitted()) - sl << QStringLiteral("heroes: [%1]").arg(rs.heroes.value().join(',')); + if (rs.joinedMemberCount) + sl << QStringLiteral("joined: %1").arg(*rs.joinedMemberCount); + if (rs.invitedMemberCount) + sl << QStringLiteral("invited: %1").arg(*rs.invitedMemberCount); + if (rs.heroes) + sl << QStringLiteral("heroes: [%1]").arg(rs.heroes->join(',')); dbg.nospace().noquote() << sl.join(QStringLiteral("; ")); return dbg; } diff --git a/lib/util.h b/lib/util.h index f7a81b2a..b7bb8d4c 100644 --- a/lib/util.h +++ b/lib/util.h @@ -24,6 +24,7 @@ #include #include #include +#include // Along the lines of Q_DISABLE_COPY - the upstream version comes in Qt 5.13 #define DISABLE_MOVE(_ClassName) \ @@ -43,68 +44,35 @@ struct HashQ { template using UnorderedMap = std::unordered_map>; -struct NoneTag {}; -constexpr NoneTag none {}; +inline constexpr auto none = std::nullopt; -/** A crude substitute for `optional` while we're not C++17 +/** `std::optional` with tweaks * - * Only works with default-constructible types. + * Due to tweaks, only works with default-constructible types. */ template -class Omittable { +class Omittable : public std::optional { static_assert(!std::is_reference::value, "You cannot make an Omittable<> with a reference type"); public: using value_type = std::decay_t; + static_assert(std::is_default_constructible_v, + "Omittable<> requires a default-constructible type"); - explicit Omittable() : Omittable(none) {} - Omittable(NoneTag) : _value(value_type()), _omitted(true) {} - Omittable(const value_type& val) : _value(val) {} - Omittable(value_type&& val) : _value(std::move(val)) {} - Omittable& operator=(const value_type& val) - { - _value = val; - _omitted = false; - return *this; - } - Omittable& operator=(value_type&& val) - { - // For some reason GCC complains about -Wmaybe-uninitialized - // in the context of using Omittable with converters.h; - // though the logic looks very much benign (GCC bug???) - _value = std::move(val); - _omitted = false; - return *this; - } + using std::optional::optional; - bool operator==(const value_type& rhs) const - { - return !omitted() && value() == rhs; - } - friend bool operator==(const value_type& lhs, - const Omittable& rhs) - { - return rhs == lhs; - } - bool operator!=(const value_type& rhs) const { return !operator==(rhs); } - friend bool operator!=(const value_type& lhs, - const Omittable& rhs) - { - return !(rhs == lhs); - } + // Overload emplace() to allow passing braced-init-lists (the standard + // emplace() does direct-initialisation but not direct-list-initialisation). + using std::optional::emplace; + T& emplace(const T& val) { return std::optional::emplace(val); } + T& emplace(T&& val) { return std::optional::emplace(std::move(val)); } - bool omitted() const { return _omitted; } - const value_type& value() const - { - Q_ASSERT(!_omitted); - return _value; - } - value_type& editValue() + value_type& edit() { - _omitted = false; - return _value; + return this->has_value() ? this->value() : this->emplace(); } + /// Merge the value from another Omittable /// \return true if \p other is not omitted and the value of /// the current Omittable was different (or omitted); @@ -114,26 +82,20 @@ public: auto merge(const Omittable& other) -> std::enable_if_t::value, bool> { - if (other.omitted() || (!_omitted && _value == other.value())) + if (!other || (this->has_value() && **this == *other)) return false; - _omitted = false; - _value = other.value(); + *this = other; return true; } - value_type&& release() - { - _omitted = true; - return std::move(_value); - } - const value_type* operator->() const& { return &value(); } - value_type* operator->() & { return &editValue(); } - const value_type& operator*() const& { return value(); } - value_type& operator*() & { return editValue(); } + // Hide non-const lvalue operator-> and operator* as these are + // a bit too surprising: value() & doesn't lazy-create an object; + // and it's too easy to inadvertently change the underlying value. -private: - T _value; - bool _omitted = false; + const value_type* operator->() const& { return &this->value(); } + value_type* operator->() && { return &this->value(); } + const value_type& operator*() const& { return this->value(); } + value_type& operator*() && { return this->value(); } }; namespace _impl { @@ -213,19 +175,19 @@ class Range { using size_type = typename ArrayT::size_type; public: - Range(ArrayT& arr) : from(std::begin(arr)), to(std::end(arr)) {} - Range(iterator from, iterator to) : from(from), to(to) {} + constexpr Range(ArrayT& arr) : from(std::begin(arr)), to(std::end(arr)) {} + constexpr Range(iterator from, iterator to) : from(from), to(to) {} - size_type size() const + constexpr size_type size() const { Q_ASSERT(std::distance(from, to) >= 0); return size_type(std::distance(from, to)); } - bool empty() const { return from == to; } - const_iterator begin() const { return from; } - const_iterator end() const { return to; } - iterator begin() { return from; } - iterator end() { return to; } + constexpr bool empty() const { return from == to; } + constexpr const_iterator begin() const { return from; } + constexpr const_iterator end() const { return to; } + constexpr iterator begin() { return from; } + constexpr iterator end() { return to; } private: iterator from; @@ -239,8 +201,8 @@ private: */ template inline std::pair findFirstOf(InputIt first, InputIt last, - ForwardIt sFirst, - ForwardIt sLast, Pred pred) + ForwardIt sFirst, + ForwardIt sLast, Pred pred) { for (; first != last; ++first) for (auto it = sFirst; it != sLast; ++it) -- cgit v1.2.3 From edbbc2bc77599ead0e14bc08cdddda10d1c5f305 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Thu, 31 Oct 2019 12:48:49 +0900 Subject: Omittable: get rid of value() Xcode 10 doesn't have it, and value() is not quite fitting mostly-exceptionless Quotient anyway. --- lib/connection.cpp | 10 +++++----- lib/room.cpp | 2 +- lib/util.h | 28 +++++++++++++++++++--------- 3 files changed, 25 insertions(+), 15 deletions(-) (limited to 'lib') diff --git a/lib/connection.cpp b/lib/connection.cpp index 47c643b0..998b45d1 100644 --- a/lib/connection.cpp +++ b/lib/connection.cpp @@ -1182,7 +1182,7 @@ Room* Connection::provideRoom(const QString& id, Omittable joinState) // TODO: This whole function is a strong case for a RoomManager class. Q_ASSERT_X(!id.isEmpty(), __FUNCTION__, "Empty room id"); - // If joinState.omitted(), all joinState == comparisons below are false. + // If joinState is empty, all joinState == comparisons below are false. const auto roomKey = qMakePair(id, joinState == JoinState::Invite); auto* room = d->roomMap.value(roomKey, nullptr); if (room) { @@ -1214,17 +1214,17 @@ Room* Connection::provideRoom(const QString& id, Omittable joinState) if (!joinState) return room; - if (joinState == JoinState::Invite) { + if (*joinState == JoinState::Invite) { // prev is either Leave or nullptr auto* prev = d->roomMap.value({ id, false }, nullptr); emit invitedRoom(room, prev); } else { - room->setJoinState(joinState.value()); + room->setJoinState(*joinState); // Preempt the Invite room (if any) with a room in Join/Leave state. auto* prevInvite = d->roomMap.take({ id, true }); - if (joinState == JoinState::Join) + if (*joinState == JoinState::Join) emit joinedRoom(room, prevInvite); - else if (joinState == JoinState::Leave) + else if (*joinState == JoinState::Leave) emit leftRoom(room, prevInvite); if (prevInvite) { const auto dcUsers = prevInvite->directChatUsers(); diff --git a/lib/room.cpp b/lib/room.cpp index b34c36ea..0806e30d 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -1208,7 +1208,7 @@ int Room::invitedCount() const { // TODO: Store invited users in Room too Q_ASSERT(d->summary.invitedMemberCount.has_value()); - return d->summary.invitedMemberCount.value(); + return d->summary.invitedMemberCount.value_or(0); } int Room::totalMemberCount() const { return joinedCount() + invitedCount(); } diff --git a/lib/util.h b/lib/util.h index b7bb8d4c..15c9fec8 100644 --- a/lib/util.h +++ b/lib/util.h @@ -56,6 +56,7 @@ class Omittable : public std::optional { "You cannot make an Omittable<> with a reference type"); public: + using base_type = std::optional; using value_type = std::decay_t; static_assert(std::is_default_constructible_v, "Omittable<> requires a default-constructible type"); @@ -64,13 +65,22 @@ public: // Overload emplace() to allow passing braced-init-lists (the standard // emplace() does direct-initialisation but not direct-list-initialisation). - using std::optional::emplace; - T& emplace(const T& val) { return std::optional::emplace(val); } - T& emplace(T&& val) { return std::optional::emplace(std::move(val)); } - + using base_type::emplace; + T& emplace(const T& val) { return base_type::emplace(val); } + T& emplace(T&& val) { return base_type::emplace(std::move(val)); } + + // use value_or() or check (with operator! or has_value) before accessing + // with operator-> or operator* + // The technical reason is that Xcode 10 has incomplete std::optional + // that has no value(); but using value() may also mean that you rely + // on the optional throwing an exception (which is not assumed practice + // throughout Quotient) or that you spend unnecessary CPU cycles on + // an extraneous has_value() check. + value_type& value() = delete; + const value_type& value() const = delete; value_type& edit() { - return this->has_value() ? this->value() : this->emplace(); + return this->has_value() ? base_type::operator*() : this->emplace(); } /// Merge the value from another Omittable @@ -92,10 +102,10 @@ public: // a bit too surprising: value() & doesn't lazy-create an object; // and it's too easy to inadvertently change the underlying value. - const value_type* operator->() const& { return &this->value(); } - value_type* operator->() && { return &this->value(); } - const value_type& operator*() const& { return this->value(); } - value_type& operator*() && { return this->value(); } + const value_type* operator->() const& { return base_type::operator->(); } + value_type* operator->() && { return base_type::operator->(); } + const value_type& operator*() const& { return base_type::operator*(); } + value_type& operator*() && { return base_type::operator*(); } }; namespace _impl { -- cgit v1.2.3 From ce1de2a78a947423b305289beefc5f904734b234 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 1 Nov 2019 10:25:45 +0900 Subject: Reinstate Omittable<>::omitted (deprecated) To ease on back-compatibility. --- lib/util.h | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'lib') diff --git a/lib/util.h b/lib/util.h index 15c9fec8..5be62382 100644 --- a/lib/util.h +++ b/lib/util.h @@ -83,6 +83,12 @@ public: return this->has_value() ? base_type::operator*() : this->emplace(); } + [[deprecated("Use '!o' or '!o.has_value()' instead of 'o.omitted()'")]] + bool omitted() const + { + return !this->has_value(); + } + /// Merge the value from another Omittable /// \return true if \p other is not omitted and the value of /// the current Omittable was different (or omitted); -- cgit v1.2.3 From ae5c2aa9acce500e2ad94c6781843c02310dbab3 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 1 Nov 2019 20:47:07 +0900 Subject: Omittable: Add direct-list-initialising operator=; document See the change in connection.cpp for the example of usage. Also: removed static_asserts: the first one is provided by std::optional, and the second one is only relevant to edit(). --- lib/connection.cpp | 2 +- lib/util.h | 50 +++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 42 insertions(+), 10 deletions(-) (limited to 'lib') diff --git a/lib/connection.cpp b/lib/connection.cpp index 998b45d1..5ee7e84e 100644 --- a/lib/connection.cpp +++ b/lib/connection.cpp @@ -279,7 +279,7 @@ void Connection::reloadCapabilities() if (!d->capabilities.roomVersions) { qCWarning(MAIN) << "Pinning supported room version to 1"; - d->capabilities.roomVersions.emplace({ "1", { { "1", "stable" } } }); + d->capabilities.roomVersions = { "1", { { "1", "stable" } } }; } else { qCDebug(MAIN) << "Room versions:" << defaultRoomVersion() << "is default, full list:" << availableRoomVersions(); diff --git a/lib/util.h b/lib/util.h index 5be62382..902b4bfc 100644 --- a/lib/util.h +++ b/lib/util.h @@ -44,27 +44,59 @@ struct HashQ { template using UnorderedMap = std::unordered_map>; -inline constexpr auto none = std::nullopt; +constexpr auto none = std::nullopt; /** `std::optional` with tweaks * - * Due to tweaks, only works with default-constructible types. + * The tweaks are: + * - streamlined assignment (operator=)/emplace()ment of values that can be + * used to implicitly construct the underlying type, including + * direct-list-initialisation, e.g.: + * \code + * struct S { int a; char b; } + * Omittable o; + * o = { 1, 'a' }; // std::optional would require o = S { 1, 'a' } + * \endcode + * - entirely deleted value(). The technical reason is that Xcode 10 doesn't + * have it; but besides that, value_or() or (after explicit checking) + * `operator*()`/`operator->()` are better alternatives within Quotient + * that doesn't practice throwing exceptions (as doesn't most of Qt). + * - disabled non-const lvalue operator*() and operator->(), as it's too easy + * to inadvertently cause a value change through them. + * - edit() to provide a safe and explicit lvalue accessor instead of those + * above. Requires the underlying type to be default-constructible. + * Allows chained initialisation of nested Omittables: + * \code + * struct Inner { int member = 10; Omittable innermost; }; + * struct Outer { int anotherMember = 10; Omittable inner; }; + * Omittable o; // = { 10, std::nullopt }; + * o.edit().inner.edit().innermost.emplace(42); + * \endcode + * - merge() - a soft version of operator= that only overwrites its first + * operand with the second one if the second one is not empty. */ template class Omittable : public std::optional { - static_assert(!std::is_reference::value, - "You cannot make an Omittable<> with a reference type"); - public: using base_type = std::optional; using value_type = std::decay_t; - static_assert(std::is_default_constructible_v, - "Omittable<> requires a default-constructible type"); using std::optional::optional; - // Overload emplace() to allow passing braced-init-lists (the standard - // emplace() does direct-initialisation but not direct-list-initialisation). + // Overload emplace() and operator=() to allow passing braced-init-lists + // (the standard emplace() does direct-initialisation but + // not direct-list-initialisation). + using base_type::operator=; + Omittable& operator=(const value_type& v) + { + base_type::operator=(v); + return *this; + } + Omittable& operator=(value_type&& v) + { + base_type::operator=(v); + return *this; + } using base_type::emplace; T& emplace(const T& val) { return base_type::emplace(val); } T& emplace(T&& val) { return base_type::emplace(std::move(val)); } -- cgit v1.2.3 From 2a7871cd9bc67a0bf311766a1edee2ff7a8e5355 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sat, 2 Nov 2019 16:20:57 +0900 Subject: Compatibility with Qt 5.14 --- lib/converters.h | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'lib') diff --git a/lib/converters.h b/lib/converters.h index 157bff27..075af7ef 100644 --- a/lib/converters.h +++ b/lib/converters.h @@ -30,6 +30,7 @@ #include +#if QT_VERSION < QT_VERSION_CHECK(5,14,0) // Enable std::unordered_map // REMOVEME in favor of UnorderedMap, once we regenerate API files namespace std { @@ -37,15 +38,12 @@ template <> struct hash { size_t operator()(const QString& s) const Q_DECL_NOEXCEPT { - return qHash(s -#if (QT_VERSION >= QT_VERSION_CHECK(5, 6, 0)) - , - uint(qGlobalQHashSeed()) -#endif + return qHash(s, uint(qGlobalQHashSeed()) ); } }; } // namespace std +#endif class QVariant; -- cgit v1.2.3 From 9d50c475ea60efa472ba6c298fbf23836edcad19 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 10 Nov 2019 17:42:20 +0900 Subject: makeRedacted: fix code formatting --- lib/room.cpp | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) (limited to 'lib') diff --git a/lib/room.cpp b/lib/room.cpp index 0806e30d..be07ce50 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -1934,22 +1934,16 @@ RoomEventPtr makeRedacted(const RoomEvent& target, const RedactionEvent& redaction) { auto originalJson = target.originalJsonObject(); - static const QStringList keepKeys { EventIdKey, - TypeKey, - QStringLiteral("room_id"), - QStringLiteral("sender"), - StateKeyKey, - QStringLiteral("prev_content"), - ContentKey, - QStringLiteral("hashes"), - QStringLiteral("signatures"), - QStringLiteral("depth"), - QStringLiteral("prev_events"), - QStringLiteral("prev_state"), - QStringLiteral("auth_events"), - QStringLiteral("origin"), - QStringLiteral("origin_server_ts"), - QStringLiteral("membership") }; + // clang-format off + static const QStringList keepKeys { EventIdKey, TypeKey, + QStringLiteral("room_id"), QStringLiteral("sender"), StateKeyKey, + QStringLiteral("prev_content"), ContentKey, + QStringLiteral("hashes"), QStringLiteral("signatures"), + QStringLiteral("depth"), QStringLiteral("prev_events"), + QStringLiteral("prev_state"), QStringLiteral("auth_events"), + QStringLiteral("origin"), QStringLiteral("origin_server_ts"), + QStringLiteral("membership") }; + // clang-format on std::vector> keepContentKeysMap { { RoomMemberEvent::typeId(), { QStringLiteral("membership") } }, -- cgit v1.2.3 From fdd094a5ef3dd8a2ef06cfa1282482a60837b317 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 10 Nov 2019 19:20:09 +0900 Subject: Cleanup --- lib/events/roomevent.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/events/roomevent.cpp b/lib/events/roomevent.cpp index 971d8597..4a9a8b5d 100644 --- a/lib/events/roomevent.cpp +++ b/lib/events/roomevent.cpp @@ -36,10 +36,8 @@ RoomEvent::RoomEvent(Type type, const QJsonObject& json) : Event(type, json) { const auto unsignedData = json[UnsignedKeyL].toObject(); const auto redaction = unsignedData[RedactedCauseKeyL]; - if (redaction.isObject()) { + if (redaction.isObject()) _redactedBecause = makeEvent(redaction.toObject()); - return; - } } RoomEvent::~RoomEvent() = default; // Let the smart pointer do its job -- cgit v1.2.3 From 5937127b73a82fc86f6546397373ce9dbaf4e560 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 19 Nov 2019 17:57:41 +0900 Subject: BaseJob: Don't send accessToken if not needed; send again on 401 The first part closes #358; the second part is a workaround for non-standard cases when endpoints without security by the spec turn out to be secured (in particular, the case of authenticating media servers). --- lib/connection.cpp | 9 ++++--- lib/connectiondata.cpp | 12 +++++++++ lib/connectiondata.h | 2 ++ lib/jobs/basejob.cpp | 66 ++++++++++++++++++++++++++++++-------------------- lib/jobs/basejob.h | 8 +++++- 5 files changed, 66 insertions(+), 31 deletions(-) (limited to 'lib') diff --git a/lib/connection.cpp b/lib/connection.cpp index 5ee7e84e..c830557c 100644 --- a/lib/connection.cpp +++ b/lib/connection.cpp @@ -343,7 +343,8 @@ void Connection::logout() } const auto* job = callApi(); connect(job, &LogoutJob::finished, this, [this, job, syncWasRunning] { - if (job->status().good() || job->error() == BaseJob::ContentAccessError) { + if (job->status().good() || job->error() == BaseJob::Unauthorised + || job->error() == BaseJob::ContentAccessError) { if (d->syncLoopConnection) disconnect(d->syncLoopConnection); d->data->setToken({}); @@ -380,9 +381,9 @@ void Connection::sync(int timeout) }); connect(job, &SyncJob::failure, this, [this, job] { d->syncJob = nullptr; - if (job->error() == BaseJob::ContentAccessError) { - qCWarning(SYNCJOB) << "Sync job failed with ContentAccessError - " - "login expired?"; + if (job->error() == BaseJob::Unauthorised) { + qCWarning(SYNCJOB) + << "Sync job failed with Unauthorised - login expired?"; emit loginError(job->errorString(), job->rawDataSample()); } else emit syncError(job->errorString(), job->rawDataSample()); diff --git a/lib/connectiondata.cpp b/lib/connectiondata.cpp index cbc36420..e2a0f06f 100644 --- a/lib/connectiondata.cpp +++ b/lib/connectiondata.cpp @@ -42,6 +42,7 @@ public: QString lastEvent; QString userId; QString deviceId; + std::vector needToken; mutable unsigned int txnCounter = 0; const qint64 txnBase = QDateTime::currentMSecsSinceEpoch(); @@ -143,6 +144,12 @@ const QString& ConnectionData::deviceId() const { return d->deviceId; } const QString& ConnectionData::userId() const { return d->userId; } +bool ConnectionData::needsToken(const QString& requestName) const +{ + return std::find(d->needToken.cbegin(), d->needToken.cend(), requestName) + != d->needToken.cend(); +} + void ConnectionData::setDeviceId(const QString& deviceId) { d->deviceId = deviceId; @@ -150,6 +157,11 @@ void ConnectionData::setDeviceId(const QString& deviceId) void ConnectionData::setUserId(const QString& userId) { d->userId = userId; } +void ConnectionData::setNeedsToken(const QString& requestName) +{ + d->needToken.push_back(requestName); +} + QString ConnectionData::lastEvent() const { return d->lastEvent; } void ConnectionData::setLastEvent(QString identifier) diff --git a/lib/connectiondata.h b/lib/connectiondata.h index b367c977..000099d1 100644 --- a/lib/connectiondata.h +++ b/lib/connectiondata.h @@ -40,6 +40,7 @@ public: QUrl baseUrl() const; const QString& deviceId() const; const QString& userId() const; + bool needsToken(const QString& requestName) const; QNetworkAccessManager* nam() const; void setBaseUrl(QUrl baseUrl); @@ -50,6 +51,7 @@ public: void setPort(int port); void setDeviceId(const QString& deviceId); void setUserId(const QString& userId); + void setNeedsToken(const QString& requestName); QString lastEvent() const; void setLastEvent(QString identifier); diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index 13e65188..7f558685 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -226,8 +226,9 @@ void BaseJob::Private::sendRequest() requestQuery) }; if (!requestHeaders.contains("Content-Type")) req.setHeader(QNetworkRequest::ContentTypeHeader, "application/json"); - req.setRawHeader("Authorization", - QByteArray("Bearer ") + connection->accessToken()); + if (needsToken) + req.setRawHeader("Authorization", + QByteArray("Bearer ") + connection->accessToken()); req.setAttribute(QNetworkRequest::BackgroundRequestAttribute, inBackground); req.setAttribute(QNetworkRequest::FollowRedirectsAttribute, true); req.setMaximumRedirectsAllowed(10); @@ -274,6 +275,7 @@ void BaseJob::sendRequest() return; Q_ASSERT(d->connection && status().code == Pending); qCDebug(d->logCat).noquote() << "Making" << d->dumpRequest(); + d->needsToken |= d->connection->needsToken(objectName()); emit aboutToSendRequest(); d->sendRequest(); Q_ASSERT(d->reply); @@ -341,32 +343,32 @@ bool checkContentType(const QByteArray& type, const QByteArrayList& patterns) return false; } -BaseJob::Status BaseJob::Status::fromHttpCode(int httpCode, QString msg) +BaseJob::StatusCode BaseJob::Status::fromHttpCode(int httpCode) { + if (httpCode / 10 == 41) // 41x errors + return httpCode == 410 ? IncorrectRequestError : NotFoundError; + switch (httpCode) { + case 401: + return Unauthorised; // clang-format off - return { [httpCode]() -> StatusCode { - if (httpCode / 10 == 41) // 41x errors - return httpCode == 410 ? IncorrectRequestError : NotFoundError; - switch (httpCode) { - case 401: case 403: case 407: - return ContentAccessError; - case 404: - return NotFoundError; - case 400: case 405: case 406: case 426: case 428: case 505: - case 494: // Unofficial nginx "Request header too large" - case 497: // Unofficial nginx "HTTP request sent to HTTPS port" - return IncorrectRequestError; - case 429: - return TooManyRequestsError; - case 501: case 510: - return RequestNotImplementedError; - case 511: - return NetworkAuthRequiredError; - default: - return NetworkError; - } - }(), std::move(msg) }; - // clang-format on + case 403: case 407: // clang-format on + return ContentAccessError; + case 404: + return NotFoundError; + // 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; + case 429: + return TooManyRequestsError; + case 501: case 510: + return RequestNotImplementedError; + case 511: + return NetworkAuthRequiredError; + default: + return NetworkError; + } } QDebug BaseJob::Status::dumpToLog(QDebug dbg) const @@ -496,6 +498,16 @@ void BaseJob::finishJob() d->connection->submit(this); return; } + if (error() == Unauthorised && !d->needsToken + && !d->connection->accessToken().isEmpty()) { + // Rerun with access token (extension of the spec while + // https://github.com/matrix-org/matrix-doc/issues/701 is pending) + d->connection->setNeedsToken(objectName()); + qCWarning(d->logCat) << this << "re-running with authentication"; + emit retryScheduled(d->retriesTaken, 0); + setStatus(Pending); + sendRequest(); + } if ((error() == NetworkError || error() == Timeout) && d->retriesTaken < d->maxRetries) { // TODO: The whole retrying thing should be put to Connection(Manager) @@ -596,6 +608,8 @@ QString BaseJob::statusCaption() const return tr("Network problems"); case TimeoutError: return tr("Request timed out"); + case Unauthorised: + return tr("Unauthorised request"); case ContentAccessError: return tr("Access error"); case NotFoundError: diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h index c4da40f5..ecbec74a 100644 --- a/lib/jobs/basejob.h +++ b/lib/jobs/basejob.h @@ -60,6 +60,7 @@ public: NetworkError = 100, Timeout, TimeoutError = Timeout, + Unauthorised, ContentAccessError, NotFoundError, IncorrectRequest, @@ -113,7 +114,12 @@ public: struct Status { Status(StatusCode c) : code(c) {} Status(int c, QString m) : code(c), message(std::move(m)) {} - static Status fromHttpCode(int httpCode, QString msg = {}); + + static StatusCode fromHttpCode(int httpCode); + static Status fromHttpCode(int httpCode, QString msg) + { + return { fromHttpCode(httpCode), std::move(msg) }; + } bool good() const { return code < ErrorLevel; } QDebug dumpToLog(QDebug dbg) const; -- cgit v1.2.3 From 4be8a673b4ad2f1caa0e34b53ed3b12e750cf569 Mon Sep 17 00:00:00 2001 From: Alexey Andreyev Date: Sat, 30 Nov 2019 03:13:23 +0300 Subject: Fix room highlighting for names with hashtag Fixes #359 --- lib/util.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/util.cpp b/lib/util.cpp index 041a8aba..9f4ac85f 100644 --- a/lib/util.cpp +++ b/lib/util.cpp @@ -56,7 +56,7 @@ void Quotient::linkifyUrls(QString& htmlEscapedText) // https://matrix.org/docs/spec/appendices.html#identifier-grammar static const QRegularExpression MxIdRegExp( QStringLiteral( - R"((^|[^<>/])([!#@][-a-z0-9_=/.]{1,252}:(?:\w|\.|-)+\.\w+(?::\d{1,5})?))"), + R"((^|[^<>/])([!#@][-a-z0-9_=#/.]{1,252}:(?:\w|\.|-)+\.\w+(?::\d{1,5})?))"), RegExpOptions); // NOTE: htmlEscapedText is already HTML-escaped! No literal <,>,&," -- cgit v1.2.3 From d18324f1a309bea59a0dc681de820c80bd9086e9 Mon Sep 17 00:00:00 2001 From: Alexey Andreyev Date: Fri, 6 Dec 2019 14:43:25 +0300 Subject: MSC1954: Remove prev_content from the essential keys list Fixes #318 --- lib/room.cpp | 1 - 1 file changed, 1 deletion(-) (limited to 'lib') diff --git a/lib/room.cpp b/lib/room.cpp index be07ce50..edf732ff 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -1937,7 +1937,6 @@ RoomEventPtr makeRedacted(const RoomEvent& target, // clang-format off static const QStringList keepKeys { EventIdKey, TypeKey, QStringLiteral("room_id"), QStringLiteral("sender"), StateKeyKey, - QStringLiteral("prev_content"), ContentKey, QStringLiteral("hashes"), QStringLiteral("signatures"), QStringLiteral("depth"), QStringLiteral("prev_events"), QStringLiteral("prev_state"), QStringLiteral("auth_events"), -- cgit v1.2.3 From f0d241c6bb8fb3219736ec6ead20564bcea9a991 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 25 Nov 2019 15:20:12 +0900 Subject: Remove extraneous qualification --- lib/room.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/room.h b/lib/room.h index 80e305f0..7e419823 100644 --- a/lib/room.h +++ b/lib/room.h @@ -181,7 +181,7 @@ public: QString avatarMediaId() const; QUrl avatarUrl() const; const Avatar& avatarObject() const; - Q_INVOKABLE Quotient::JoinState joinState() const; + Q_INVOKABLE JoinState joinState() const; Q_INVOKABLE QList usersTyping() const; QList membersLeft() const; -- cgit v1.2.3 From e3e94f86edc94b743af913b4b5b8b9af7c446fab Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 8 Dec 2019 21:01:05 +0300 Subject: ConnectionData: stop the timer on destruction ...to prevent from trying to send requests after closing the connection. --- lib/connectiondata.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/connectiondata.cpp b/lib/connectiondata.cpp index e2a0f06f..94cab29b 100644 --- a/lib/connectiondata.cpp +++ b/lib/connectiondata.cpp @@ -86,7 +86,11 @@ ConnectionData::ConnectionData(QUrl baseUrl) }); } -ConnectionData::~ConnectionData() = default; +ConnectionData::~ConnectionData() +{ + d->rateLimiter.disconnect(); + d->rateLimiter.stop(); +} void ConnectionData::submit(BaseJob* job) { -- cgit v1.2.3 From 77f1ca99ade62565ce30758005b911eb340953e6 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 8 Dec 2019 20:19:25 +0300 Subject: Room::addNewMessageEvents: search the whole sync batch for events to redact It seems that sometimes redactions may precede the unredacted events within the same sync batch. --- lib/room.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/room.cpp b/lib/room.cpp index be07ce50..198cef0d 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -2122,12 +2122,10 @@ Room::Changes Room::Private::addNewMessageEvents(RoomEvents&& events) // Try to find the target in the timeline, then in the batch. if (processRedaction(*r)) continue; - auto targetIt = std::find_if(events.begin(), it, - [id = r->redactedEvent()]( - const RoomEventPtr& ep) { - return ep->id() == id; - }); - if (targetIt != it) + if (auto targetIt = std::find_if(events.begin(), events.end(), + [id = r->redactedEvent()](const RoomEventPtr& ep) { + return ep->id() == id; + }); targetIt != events.end()) *targetIt = makeRedacted(**targetIt, *r); else qCDebug(EVENTS) -- cgit v1.2.3 From e829f867c14cb51e89e4d10550f2f41329c14ba8 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 9 Dec 2019 11:52:26 +0300 Subject: Initialise read marker if none is found in the whole timeline Closes #361. --- lib/room.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/room.cpp b/lib/room.cpp index 71d76f66..8253ea42 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -515,7 +515,9 @@ void Room::Private::updateUnreadCount(rev_iter_t from, rev_iter_t to) // that has just arrived. In this case we should recalculate // unreadMessages and might need to promote the read marker further // over local-origin messages. - const auto readMarker = q->readMarker(); + auto readMarker = q->readMarker(); + if (readMarker == timeline.crend() && q->allHistoryLoaded()) + --readMarker; // Read marker not found in the timeline, initialise it if (readMarker >= from && readMarker < to) { promoteReadMarker(q->localUser(), readMarker, true); return; -- cgit v1.2.3 From 105ff3874baf0792616e1b330ce97f0b725adaf5 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 9 Dec 2019 18:14:43 +0300 Subject: Room: more doc-comments --- lib/room.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'lib') diff --git a/lib/room.h b/lib/room.h index 7e419823..2c958033 100644 --- a/lib/room.h +++ b/lib/room.h @@ -442,6 +442,10 @@ public: Q_INVOKABLE const Quotient::StateEventBase* getCurrentState(const QString& evtType, const QString& stateKey = {}) const; + /// Get a state event with the given event type and state key + /*! This is a typesafe overload that accepts a C++ event type instead of + * its Matrix name. + */ template const EvT* getCurrentState(const QString& stateKey = {}) const { @@ -453,6 +457,14 @@ public: return evt; } + /// Set a state event of the given type with the given arguments + /*! This typesafe overload attempts to send a state event with the type + * \p EvT and the content defined by \p args. Specifically, the function + * creates a temporary object of type \p EvT passing \p args to + * the constructor, and sends a request to the homeserver using + * the Matrix event type defined by \p EvT and the event content produced + * via EvT::contentJson(). + */ template auto setState(ArgTs&&... args) const { -- cgit v1.2.3 From 1b38ad0ce7ca8b2a5a7e4936824697b84e6b7076 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 9 Dec 2019 18:32:18 +0300 Subject: Code cleanup --- lib/room.cpp | 46 ++++++++++++++++++++++------------------------ 1 file changed, 22 insertions(+), 24 deletions(-) (limited to 'lib') diff --git a/lib/room.cpp b/lib/room.cpp index 8253ea42..fe50aa9a 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -2134,25 +2134,23 @@ Room::Changes Room::Private::addNewMessageEvents(RoomEvents&& events) << r->redactedEvent() << "is not found"; // If the target event comes later, it comes already redacted. } - if (auto* msg = eventCast(eptr)) { - if (!msg->replacedEvent().isEmpty()) { - if (processReplacement(*msg)) - continue; - auto targetIt = std::find_if(events.begin(), it, - [id = msg->replacedEvent()]( - const RoomEventPtr& ep) { - return ep->id() == id; - }); - if (targetIt != it) - *targetIt = makeReplaced(**targetIt, *msg); - else // FIXME: don't ignore, just show it wherever it arrived - qCDebug(EVENTS) - << "Replacing event" << msg->id() - << "ignored: replaced event" << msg->replacedEvent() - << "is not found"; - // Same as with redactions above, the replaced event coming - // later will come already with the new content. - } + if (auto* msg = eventCast(eptr); + msg && !msg->replacedEvent().isEmpty()) { + if (processReplacement(*msg)) + continue; + auto targetIt = std::find_if(events.begin(), it, + [id = msg->replacedEvent()](const RoomEventPtr& ep) { + return ep->id() == id; + }); + if (targetIt != it) + *targetIt = makeReplaced(**targetIt, *msg); + else // FIXME: don't ignore, just show it wherever it arrived + qCDebug(EVENTS) + << "Replacing event" << msg->id() + << "ignored: replaced event" << msg->replacedEvent() + << "is not found"; + // Same as with redactions above, the replaced event coming + // later will come already with the new content. } } } @@ -2317,7 +2315,7 @@ Room::Changes Room::processStateEvent(const RoomEvent& e) || (oldStateEvent->matrixType() == e.matrixType() && oldStateEvent->stateKey() == e.stateKey())); if (!is(e)) // Room member events are too numerous - qCDebug(EVENTS) << "Room state event:" << e; + qCDebug(STATE) << "Room state event:" << e; // clang-format off return visit(e @@ -2327,10 +2325,10 @@ Room::Changes Room::processStateEvent(const RoomEvent& e) , [this,oldStateEvent] (const RoomAliasesEvent& ae) { // clang-format on if (ae.aliases().isEmpty()) { - qCDebug(STATE).noquote() - << ae.stateKey() << "no more has aliases for room" - << objectName(); - d->aliasServers.remove(ae.stateKey()); + if (d->aliasServers.remove(ae.stateKey())) + qCDebug(STATE).noquote() + << ae.stateKey() << "no more has aliases for room" + << objectName(); } else { d->aliasServers.insert(ae.stateKey()); qCDebug(STATE).nospace().noquote() -- cgit v1.2.3 From 608e252bae9cf8cf763e05363bfacf5e1760134f Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 9 Dec 2019 18:19:53 +0300 Subject: RoomEvent::timestamp() -> originTimestamp() The previous name is still available but deprecated. --- lib/events/roomevent.cpp | 2 +- lib/events/roomevent.h | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/events/roomevent.cpp b/lib/events/roomevent.cpp index 4a9a8b5d..a59cd6e0 100644 --- a/lib/events/roomevent.cpp +++ b/lib/events/roomevent.cpp @@ -44,7 +44,7 @@ RoomEvent::~RoomEvent() = default; // Let the smart pointer do its job QString RoomEvent::id() const { return fullJson()[EventIdKeyL].toString(); } -QDateTime RoomEvent::timestamp() const +QDateTime RoomEvent::originTimestamp() const { return Quotient::fromJson(fullJson()["origin_server_ts"_ls]); } diff --git a/lib/events/roomevent.h b/lib/events/roomevent.h index f943bce4..621652cb 100644 --- a/lib/events/roomevent.h +++ b/lib/events/roomevent.h @@ -46,7 +46,10 @@ public: ~RoomEvent() override; QString id() const; - QDateTime timestamp() const; + QDateTime originTimestamp() const; + [[deprecated("Use originTimestamp()")]] QDateTime timestamp() const { + return originTimestamp(); + } QString roomId() const; QString senderId() const; bool isReplaced() const; -- cgit v1.2.3 From 3e81ba0da47278f383ce8c329010602f84a50482 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 9 Dec 2019 18:30:36 +0300 Subject: Room::predecessor() and Room::successor() --- lib/room.cpp | 20 ++++++++++++++++++++ lib/room.h | 15 +++++++++++++++ 2 files changed, 35 insertions(+) (limited to 'lib') diff --git a/lib/room.cpp b/lib/room.cpp index fe50aa9a..41a8888c 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -387,11 +387,31 @@ QString Room::predecessorId() const return d->getCurrentState()->predecessor().roomId; } +Room* Room::predecessor(JoinStates statesFilter) const +{ + if (const auto& predId = predecessorId(); !predId.isEmpty()) + if (auto* r = connection()->room(predId, statesFilter); + r && r->successorId() == id()) + return r; + + return nullptr; +} + QString Room::successorId() const { return d->getCurrentState()->successorRoomId(); } +Room* Room::successor(JoinStates statesFilter) const +{ + if (const auto& succId = successorId(); !succId.isEmpty()) + if (auto* r = connection()->room(succId, statesFilter); + r && r->predecessorId() == id()) + return r; + + return nullptr; +} + const Room::Timeline& Room::messageEvents() const { return d->timeline; } const Room::PendingEvents& Room::pendingEvents() const diff --git a/lib/room.h b/lib/room.h index 2c958033..fa3c6ff3 100644 --- a/lib/room.h +++ b/lib/room.h @@ -167,7 +167,22 @@ public: QString version() const; bool isUnstable() const; QString predecessorId() const; + /// Room predecessor + /** This function validates that the predecessor has a tombstone and + * the tombstone refers to the current room. If that's not the case, + * or if the predecessor is in a join state not matching \p stateFilter, + * the function returns nullptr. + */ + Room* predecessor(JoinStates statesFilter = JoinState::Invite + | JoinState::Join) const; QString successorId() const; + /// Room successor + /** This function validates that the successor room's creation event + * refers to the current room. If that's not the case, or if the successor + * is in a join state not matching \p stateFilter, it returns nullptr. + */ + Room* successor(JoinStates statesFilter = JoinState::Invite + | JoinState::Join) const; QString name() const; /// Room aliases defined on the current user's server /// \sa remoteAliases, setLocalAliases -- cgit v1.2.3 From 4b56d47284500ab61f8e0c5cd7c807c58e1b53cb Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 9 Dec 2019 18:22:37 +0300 Subject: Room::creation() and Room::tombstone() --- lib/room.h | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'lib') diff --git a/lib/room.h b/lib/room.h index fa3c6ff3..10f18742 100644 --- a/lib/room.h +++ b/lib/room.h @@ -27,6 +27,8 @@ #include "events/accountdataevents.h" #include "events/encryptedevent.h" #include "events/roommessageevent.h" +#include "events/roomcreateevent.h" +#include "events/roomtombstoneevent.h" #include #include @@ -303,6 +305,11 @@ public: const RelatedEvents relatedEvents(const RoomEvent& evt, const char* relType) const; + const RoomCreateEvent* creation() const + { return getCurrentState(); } + const RoomTombstoneEvent* tombstone() const + { return getCurrentState(); } + bool displayed() const; /// Mark the room as currently displayed to the user /** -- cgit v1.2.3 From 6b2847de2325f2b818dc336c9339d50de58604ea Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 9 Dec 2019 18:31:59 +0300 Subject: Pass action scope to Room::setTags The tags can now be applied not only to the current room but also propagated to its predecessors and successors. --- lib/room.cpp | 17 ++++++++++++++++- lib/room.h | 18 +++++++++++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/room.cpp b/lib/room.cpp index 41a8888c..60b9a684 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -948,12 +948,27 @@ void Room::removeTag(const QString& name) << "not found, nothing to remove"; } -void Room::setTags(TagsMap newTags) +void Room::setTags(TagsMap newTags, ActionScope applyOn) { + bool propagate = applyOn != ActionScope::ThisRoomOnly; + auto joinStates = + applyOn == ActionScope::WithinSameState ? joinState() : + applyOn == ActionScope::OmitLeftState ? JoinState::Join|JoinState::Invite : + JoinState::Join|JoinState::Invite|JoinState::Leave; + if (propagate) { + for (auto* r = this; (r = r->predecessor(joinStates));) + r->setTags(newTags, ActionScope::ThisRoomOnly); + } + d->setTags(move(newTags)); connection()->callApi( localUser()->id(), id(), TagEvent::matrixTypeId(), TagEvent(d->tags).contentJson()); + + if (propagate) { + for (auto* r = this; (r = r->successor(joinStates));) + r->setTags(newTags, ActionScope::ThisRoomOnly); + } } void Room::Private::setTags(TagsMap newTags) diff --git a/lib/room.h b/lib/room.h index 10f18742..ad19792e 100644 --- a/lib/room.h +++ b/lib/room.h @@ -396,6 +396,19 @@ public: /// Remove a tag from the room Q_INVOKABLE void removeTag(const QString& name); + /// The scope to apply an action on + /*! This enumeration is used to pick a strategy to propagate certain + * actions on the room to its predecessors and successors. + */ + enum ActionScope { + ThisRoomOnly, //< Do not apply to predecessors and successors + WithinSameState, //< Apply to predecessors and successors in the same + //< state as the current one + OmitLeftState, //< Apply to all reachable predecessors and successors + //< except those in Leave state + WholeSequence //< Apply to all reachable predecessors and successors + }; + /** Overwrite the room's tags * This completely replaces the existing room's tags with a set * of new ones and updates the new set on the server. Unlike @@ -403,8 +416,11 @@ public: * immediately, not waiting for confirmation from the server * (because tags are saved in account data rather than in shared * room state). + * \param applyOn setting this to Room::OnAllConversations will set tags + * on this and all _known_ predecessors and successors; + * by default only the current room is changed */ - void setTags(TagsMap newTags); + void setTags(TagsMap newTags, ActionScope applyOn = ThisRoomOnly); /// Check whether the list of tags has m.favourite bool isFavourite() const; -- cgit v1.2.3 From 7b5f65ba64ae4df54919336f4beec19493d1b5ee Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Wed, 11 Dec 2019 20:26:39 +0300 Subject: BaseJob::StatusCode: offset first error from ErrorLevel Q_ENUM seems to resolve int to the first enum identifier with that value so NetworkError == ErrorLevel looks confusing in logs. --- lib/jobs/basejob.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h index ecbec74a..b4c4c136 100644 --- a/lib/jobs/basejob.h +++ b/lib/jobs/basejob.h @@ -57,7 +57,7 @@ public: Unprepared = 25, //< Initial job state is incomplete, hence warning level Abandoned = 50, //< A tiny period between abandoning and object deletion ErrorLevel = 100, //< Errors have codes starting from this - NetworkError = 100, + NetworkError = 101, Timeout, TimeoutError = Timeout, Unauthorised, -- cgit v1.2.3 From 1ffa6b96adeba8c1a00a0a32112c8b0aaeb2350e Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Thu, 12 Dec 2019 19:11:16 +0300 Subject: RequestData: optimise fromData() and preopen the buffer It was just a coincidence that QBuffer allowed reading from it without being isReadable() at the moment of starting a job. --- lib/jobs/requestdata.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/jobs/requestdata.cpp b/lib/jobs/requestdata.cpp index 0c70f085..cec15954 100644 --- a/lib/jobs/requestdata.cpp +++ b/lib/jobs/requestdata.cpp @@ -11,9 +11,8 @@ using namespace Quotient; auto fromData(const QByteArray& data) { auto source = std::make_unique(); - source->open(QIODevice::WriteOnly); - source->write(data); - source->close(); + source->setData(data); + source->open(QIODevice::ReadOnly); return source; } -- cgit v1.2.3 From eabed200f5fe09a37942a52387c0d3f25070b0c3 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Thu, 12 Dec 2019 19:34:57 +0300 Subject: Room::fileNameToDownload(): fix a typo in a check on Windows As a result of the typo, the extension was never attached to returned file names if there was none originally. --- lib/room.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/room.cpp b/lib/room.cpp index 60b9a684..eefccdb5 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -1037,7 +1037,7 @@ QString Room::Private::fileNameToDownload(const RoomMessageEvent* event) const if (QSysInfo::productType() == "windows") { if (const auto& suffixes = fileInfo->mimeType.suffixes(); - suffixes.isEmpty() + !suffixes.isEmpty() && std::none_of(suffixes.begin(), suffixes.end(), [&fileName](const QString& s) { return fileName.endsWith(s); -- cgit v1.2.3 From ec3ddd6930991b04d13cdb12720f141482f9a6eb Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Thu, 12 Dec 2019 19:37:09 +0300 Subject: Move FileError definition from DownloadFileJob to BaseJob Will use it in BaseJob in a later commit. --- lib/jobs/basejob.h | 1 + lib/jobs/downloadfilejob.h | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h index b4c4c136..e708ba8d 100644 --- a/lib/jobs/basejob.h +++ b/lib/jobs/basejob.h @@ -82,6 +82,7 @@ public: UserConsentRequiredError = UserConsentRequired, CannotLeaveRoom, UserDeactivated, + FileError, UserDefinedError = 256 }; Q_ENUM(StatusCode) diff --git a/lib/jobs/downloadfilejob.h b/lib/jobs/downloadfilejob.h index b7d2d75b..06dc145c 100644 --- a/lib/jobs/downloadfilejob.h +++ b/lib/jobs/downloadfilejob.h @@ -5,8 +5,6 @@ namespace Quotient { class DownloadFileJob : public GetContentJob { public: - enum { FileError = BaseJob::UserDefinedError + 1 }; - using GetContentJob::makeRequestUrl; static QUrl makeRequestUrl(QUrl baseUrl, const QUrl& mxcUri); -- cgit v1.2.3 From d438c08e11d17dc9c005806a6036e1aeb769c54b Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Thu, 12 Dec 2019 19:39:24 +0300 Subject: Connection::uploadFile/Content(): refactoring around QIODevice::open() No more "The file is already open" log messages. --- lib/connection.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/connection.cpp b/lib/connection.cpp index c830557c..999f4382 100644 --- a/lib/connection.cpp +++ b/lib/connection.cpp @@ -624,12 +624,17 @@ UploadContentJob* Connection::uploadContent(QIODevice* contentSource, const QString& filename, const QString& overrideContentType) const { + Q_ASSERT(contentSource != nullptr); auto contentType = overrideContentType; if (contentType.isEmpty()) { contentType = QMimeDatabase() .mimeTypeForFileNameAndData(filename, contentSource) .name(); - contentSource->open(QIODevice::ReadOnly); + if (!contentSource->open(QIODevice::ReadOnly)) { + qCWarning(MAIN) << "Couldn't open content source" << filename + << "for reading:" << contentSource->errorString(); + return nullptr; + } } return callApi(contentSource, filename, contentType); } @@ -638,11 +643,6 @@ UploadContentJob* Connection::uploadFile(const QString& fileName, const QString& overrideContentType) { auto sourceFile = new QFile(fileName); - if (!sourceFile->open(QIODevice::ReadOnly)) { - qCWarning(MAIN) << "Couldn't open" << sourceFile->fileName() - << "for reading"; - return nullptr; - } return uploadContent(sourceFile, QFileInfo(*sourceFile).fileName(), overrideContentType); } -- cgit v1.2.3 From f6505a541fc0a2e02f9c79496488121a3e46fb01 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Thu, 12 Dec 2019 19:47:07 +0300 Subject: BaseJob: prepare() -> initiate() + refactoring around it * BaseJob::initiate() now calls ConnectionData::submit() without relying on Connection to do that * ConnectionData::submit() is now the only site where a job enters Pending state * No more shortcuts to BaseJob::sendRequest(), even retries are sent through the ConnectionData submission queue * Additional validation in BaseJob::initiate() that the request data device is actually open (because QtNetwork API officially requires that, even if you can get away passing a closed QBuffer to it) --- lib/connection.cpp | 3 +-- lib/connectiondata.cpp | 2 +- lib/jobs/basejob.cpp | 28 +++++++++++++++++++--------- lib/jobs/basejob.h | 2 +- 4 files changed, 22 insertions(+), 13 deletions(-) (limited to 'lib') diff --git a/lib/connection.cpp b/lib/connection.cpp index 999f4382..c13568f1 100644 --- a/lib/connection.cpp +++ b/lib/connection.cpp @@ -1414,8 +1414,7 @@ void Connection::setLazyLoading(bool newValue) void Connection::run(BaseJob* job, RunningPolicy runningPolicy) const { connect(job, &BaseJob::failure, this, &Connection::requestFailed); - job->prepare(d->data.get(), runningPolicy & BackgroundRequest); - d->data->submit(job); + job->initiate(d->data.get(), runningPolicy & BackgroundRequest); } void Connection::getTurnServers() diff --git a/lib/connectiondata.cpp b/lib/connectiondata.cpp index 94cab29b..753fa3ad 100644 --- a/lib/connectiondata.cpp +++ b/lib/connectiondata.cpp @@ -94,7 +94,7 @@ ConnectionData::~ConnectionData() void ConnectionData::submit(BaseJob* job) { - Q_ASSERT(job->error() == BaseJob::Pending); + job->setStatus(BaseJob::Pending); if (!d->rateLimiter.isActive()) { job->sendRequest(); return; diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index 7f558685..4653e58a 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -138,8 +138,7 @@ BaseJob::BaseJob(HttpVerb verb, const QString& name, const QString& endpoint, setObjectName(name); connect(&d->timer, &QTimer::timeout, this, &BaseJob::timeout); connect(&d->retryTimer, &QTimer::timeout, this, [this] { - setStatus(Pending); - sendRequest(); + d->connection->submit(this); }); } @@ -259,14 +258,27 @@ void BaseJob::onSentRequest(QNetworkReply*) {} void BaseJob::beforeAbandon(QNetworkReply*) {} -void BaseJob::prepare(ConnectionData* connData, bool inBackground) +void BaseJob::initiate(ConnectionData* connData, bool inBackground) { + Q_ASSERT(connData != nullptr); + d->inBackground = inBackground; d->connection = connData; doPrepare(); - if (status().code != Unprepared && status().code != Pending) + + if ((d->verb == HttpVerb::Post || d->verb == HttpVerb::Put) + && !d->requestData.source()->isReadable()) { + setStatus(FileError, "Request data not ready"); + } + Q_ASSERT(status().code != Pending); // doPrepare() must NOT set this + if (status().code == Unprepared) { + d->connection->submit(this); + } else { + qDebug(d->logCat).noquote() + << "Request failed preparation and won't be sent:" + << d->dumpRequest(); QTimer::singleShot(0, this, &BaseJob::finishJob); - setStatus(Pending); + } } void BaseJob::sendRequest() @@ -292,7 +304,7 @@ void BaseJob::sendRequest() onSentRequest(d->reply.data()); emit sentRequest(); } else - qCWarning(d->logCat).noquote() + qCCritical(d->logCat).noquote() << "Request could not start:" << d->dumpRequest(); } @@ -494,7 +506,6 @@ void BaseJob::finishJob() stop(); if (error() == TooManyRequests) { emit rateLimited(); - setStatus(Pending); d->connection->submit(this); return; } @@ -505,8 +516,7 @@ void BaseJob::finishJob() d->connection->setNeedsToken(objectName()); qCWarning(d->logCat) << this << "re-running with authentication"; emit retryScheduled(d->retriesTaken, 0); - setStatus(Pending); - sendRequest(); + d->connection->submit(this); } if ((error() == NetworkError || error() == Timeout) && d->retriesTaken < d->maxRetries) { diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h index e708ba8d..c8046e9e 100644 --- a/lib/jobs/basejob.h +++ b/lib/jobs/basejob.h @@ -203,7 +203,7 @@ public: } public slots: - void prepare(ConnectionData* connData, bool inBackground); + void initiate(ConnectionData* connData, bool inBackground); /** * Abandons the result of this job, arrived or unarrived. -- cgit v1.2.3 From 7641518deea3591c6903cb5b1b6c7b551c30281b Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Thu, 12 Dec 2019 19:49:09 +0300 Subject: Room::downloadFile: minor cleanup --- lib/room.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/room.cpp b/lib/room.cpp index eefccdb5..69ebd677 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -1902,9 +1902,8 @@ void Room::downloadFile(const QString& eventId, const QUrl& localFilename) } auto job = connection()->downloadFile(fileUrl, filePath); if (isJobRunning(job)) { - // If there was a previous transfer (completed or failed), remove it. - d->fileTransfers.remove(eventId); - d->fileTransfers.insert(eventId, { job, job->targetFileName() }); + // If there was a previous transfer (completed or failed), overwrite it. + d->fileTransfers[eventId] = { job, job->targetFileName() }; connect(job, &BaseJob::downloadProgress, this, [this, eventId](qint64 received, qint64 total) { d->fileTransfers[eventId].update(received, total); -- cgit v1.2.3 From 38f45066a2304935550e2c5b6be21a9744f66bf1 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Thu, 12 Dec 2019 20:57:41 +0300 Subject: BaseJob::initiate(): fix nullptr dereferencing --- lib/jobs/basejob.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index 4653e58a..68adeaf6 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -267,7 +267,7 @@ void BaseJob::initiate(ConnectionData* connData, bool inBackground) doPrepare(); if ((d->verb == HttpVerb::Post || d->verb == HttpVerb::Put) - && !d->requestData.source()->isReadable()) { + && d->requestData.source() && !d->requestData.source()->isReadable()) { setStatus(FileError, "Request data not ready"); } Q_ASSERT(status().code != Pending); // doPrepare() must NOT set this -- cgit v1.2.3 From 646186d802cfe7213146e4a68f8f926e21931b17 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 13 Dec 2019 14:18:31 +0300 Subject: Room::postFile(): minor refactoring --- lib/room.cpp | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) (limited to 'lib') diff --git a/lib/room.cpp b/lib/room.cpp index 69ebd677..0a192203 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -1628,19 +1628,16 @@ QString Room::postFile(const QString& plainText, const QUrl& localPath, QFileInfo localFile { localPath.toLocalFile() }; Q_ASSERT(localFile.isFile()); - const auto txnId = connection()->generateTxnId(); + const auto txnId = + d->addAsPending( + makeEvent(plainText, localFile, asGenericFile)) + ->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); - { - auto&& event = - makeEvent(plainText, localFile, asGenericFile); - event->setTransactionId(txnId); - d->addAsPending(std::move(event)); - } - auto* context = new QObject(this); - connect(this, &Room::fileTransferCompleted, context, - [context, this, txnId](const QString& id, QUrl, const QUrl& mxcUri) { + // Below, the upload job is used as a context object to clean up connections + connect(this, &Room::fileTransferCompleted, d->fileTransfers[txnId].job, + [this, txnId](const QString& id, QUrl, const QUrl& mxcUri) { if (id == txnId) { auto it = findPendingEvent(txnId); if (it != d->unsyncedEvents.end()) { @@ -1656,11 +1653,10 @@ QString Room::postFile(const QString& plainText, const QUrl& localPath, << "but the event referring to it was " "cancelled"; } - context->deleteLater(); } }); - connect(this, &Room::fileTransferCancelled, this, - [context, this, txnId](const QString& id) { + connect(this, &Room::fileTransferCancelled, d->fileTransfers[txnId].job, + [this, txnId](const QString& id) { if (id == txnId) { auto it = findPendingEvent(txnId); if (it != d->unsyncedEvents.end()) { @@ -1670,7 +1666,6 @@ QString Room::postFile(const QString& plainText, const QUrl& localPath, d->unsyncedEvents.erase(d->unsyncedEvents.begin() + idx); emit pendingEventDiscarded(); } - context->deleteLater(); } }); @@ -1849,7 +1844,7 @@ void Room::uploadFile(const QString& id, const QUrl& localFilename, auto fileName = localFilename.toLocalFile(); auto job = connection()->uploadFile(fileName, overrideContentType); if (isJobRunning(job)) { - d->fileTransfers.insert(id, { job, fileName, true }); + d->fileTransfers[id] = { job, fileName, true }; connect(job, &BaseJob::uploadProgress, this, [this, id](qint64 sent, qint64 total) { d->fileTransfers[id].update(sent, total); -- cgit v1.2.3 From 04960510df6058b4f25d3c41001a519e9fc5ba4a Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 13 Dec 2019 14:47:26 +0300 Subject: Room: make downloaded file name building more robust Specifically, handle colons and long file names gracefully. Closes #366. --- lib/room.cpp | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) (limited to 'lib') diff --git a/lib/room.cpp b/lib/room.cpp index 0a192203..b6925be5 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -1005,6 +1005,11 @@ QList Room::directChatUsers() const return connection()->directChatUsers(this); } +QString safeFileName(QString rawName) +{ + return rawName.replace(QRegularExpression("[/\\<>|\"*?:]"), "_"); +} + const RoomMessageEvent* Room::Private::getEventWithFile(const QString& eventId) const { @@ -1020,20 +1025,20 @@ Room::Private::getEventWithFile(const QString& eventId) const QString Room::Private::fileNameToDownload(const RoomMessageEvent* event) const { - Q_ASSERT(event->hasFileContent()); + Q_ASSERT(event && event->hasFileContent()); const auto* fileInfo = event->content()->fileInfo(); QString fileName; if (!fileInfo->originalName.isEmpty()) - fileName = QFileInfo(fileInfo->originalName).fileName(); - else if (!event->plainBody().isEmpty()) { - // Having no better options, assume that the body has - // the original file URL or at least the file name. - if (QUrl u { event->plainBody() }; u.isValid()) - fileName = QFileInfo(u.path()).fileName(); + fileName = QFileInfo(safeFileName(fileInfo->originalName)).fileName(); + else if (QUrl u { event->plainBody() }; u.isValid()) { + qDebug(MAIN) << event->id() + << "has no file name supplied but the event body " + "looks like a URL - using the file name from it"; + fileName = u.fileName(); } - // Check the file name for sanity - if (fileName.isEmpty() || !QTemporaryFile(fileName).open()) - return "file." % fileInfo->mimeType.preferredSuffix(); + if (fileName.isEmpty()) + return safeFileName(fileInfo->mediaId()).replace('.', '-') % '.' + % fileInfo->mimeType.preferredSuffix(); if (QSysInfo::productType() == "windows") { if (const auto& suffixes = fileInfo->mimeType.suffixes(); @@ -1888,12 +1893,15 @@ void Room::downloadFile(const QString& eventId, const QUrl& localFilename) } const auto fileUrl = fileInfo->url; auto filePath = localFilename.toLocalFile(); - if (filePath.isEmpty()) { - // Build our own file path, starting with temp directory and eventId. - filePath = eventId; - filePath = QDir::tempPath() % '/' - % filePath.replace(QRegularExpression("[/\\<>|\"*?:]"), "_") - % '#' % d->fileNameToDownload(event); + if (filePath.isEmpty()) { // Setup default file path + filePath = + fileInfo->url.path().mid(1) % '_' % d->fileNameToDownload(event); + + if (filePath.size() > 200) // If too long, elide in the middle + filePath.replace(128, filePath.size() - 192, "---"); + + filePath = QDir::tempPath() % '/' % filePath; + qDebug(MAIN) << "File path:" << filePath; } auto job = connection()->downloadFile(fileUrl, filePath); if (isJobRunning(job)) { -- cgit v1.2.3 From 92264831077874511341b2dabae255649f741f54 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 23 Dec 2019 21:48:16 +0300 Subject: Connection::forgetRoom: slightly simplify code --- lib/connection.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/connection.cpp b/lib/connection.cpp index c13568f1..5bddbb83 100644 --- a/lib/connection.cpp +++ b/lib/connection.cpp @@ -804,7 +804,7 @@ ForgetRoomJob* Connection::forgetRoom(const QString& id) if (!room) room = d->roomMap.value({ id, true }); if (room && room->joinState() != JoinState::Leave) { - auto leaveJob = room->leaveRoom(); + auto leaveJob = leaveRoom(room); connect(leaveJob, &BaseJob::result, this, [this, leaveJob, forgetJob, room] { if (leaveJob->error() == BaseJob::Success -- cgit v1.2.3 From 79b9f6d928cbd516a610d845891f4054df05e0c4 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 23 Dec 2019 21:49:48 +0300 Subject: ConnectionData::submit: when not queuing, still submit asynchronously Otherwise BaseJob::messageSent has every chance to be overlooked by clients who subscribe to it after calling callApi(). --- lib/connectiondata.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/connectiondata.cpp b/lib/connectiondata.cpp index 753fa3ad..e806f952 100644 --- a/lib/connectiondata.cpp +++ b/lib/connectiondata.cpp @@ -96,7 +96,7 @@ void ConnectionData::submit(BaseJob* job) { job->setStatus(BaseJob::Pending); if (!d->rateLimiter.isActive()) { - job->sendRequest(); + QTimer::singleShot(0, job, &BaseJob::sendRequest); return; } d->jobs[size_t(job->isBackground())].emplace(job); -- cgit v1.2.3 From 0373153481e4a08f1dfb194e672188d74ce07d85 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 24 Dec 2019 11:26:11 +0300 Subject: RoomMemberEvent: introduce the reason field (MSC2367) See https://github.com/matrix-org/matrix-doc/pull/2367. Closes #370. --- lib/events/roommemberevent.cpp | 14 ++++++++++---- lib/events/roommemberevent.h | 7 +++++-- 2 files changed, 15 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/events/roommemberevent.cpp b/lib/events/roommemberevent.cpp index d0787170..d4b2be45 100644 --- a/lib/events/roommemberevent.cpp +++ b/lib/events/roommemberevent.cpp @@ -52,6 +52,7 @@ MemberEventContent::MemberEventContent(const QJsonObject& json) , isDirect(json["is_direct"_ls].toBool()) , displayName(sanitized(json["displayname"_ls].toString())) , avatarUrl(json["avatar_url"_ls].toString()) + , reason(json["reason"_ls].toString()) {} void MemberEventContent::fillJson(QJsonObject* o) const @@ -64,18 +65,23 @@ void MemberEventContent::fillJson(QJsonObject* o) const o->insert(QStringLiteral("displayname"), displayName); if (avatarUrl.isValid()) o->insert(QStringLiteral("avatar_url"), avatarUrl.toString()); + if (!reason.isEmpty()) + o->insert(QStringLiteral("reason"), reason); +} + +bool RoomMemberEvent::changesMembership() const +{ + return !prevContent() || prevContent()->membership != membership(); } bool RoomMemberEvent::isInvite() const { - return membership() == MembershipType::Invite - && (!prevContent() || prevContent()->membership != membership()); + return membership() == MembershipType::Invite && changesMembership(); } bool RoomMemberEvent::isJoin() const { - return membership() == MembershipType::Join - && (!prevContent() || prevContent()->membership != membership()); + return membership() == MembershipType::Join && changesMembership(); } bool RoomMemberEvent::isLeave() const diff --git a/lib/events/roommemberevent.h b/lib/events/roommemberevent.h index 6a34fd7f..0ca439e1 100644 --- a/lib/events/roommemberevent.h +++ b/lib/events/roommemberevent.h @@ -40,6 +40,7 @@ public: bool isDirect = false; QString displayName; QUrl avatarUrl; + QString reason; protected: void fillJson(QJsonObject* o) const override; @@ -56,8 +57,8 @@ public: explicit RoomMemberEvent(const QJsonObject& obj) : StateEvent(typeId(), obj) {} - [[deprecated("Use RoomMemberEvent(userId, contentArgs) " - "instead")]] RoomMemberEvent(MemberEventContent&& c) + [[deprecated("Use RoomMemberEvent(userId, contentArgs) instead")]] + RoomMemberEvent(MemberEventContent&& c) : StateEvent(typeId(), matrixTypeId(), QString(), c) {} template @@ -85,6 +86,8 @@ public: bool isDirect() const { return content().isDirect; } QString displayName() const { return content().displayName; } QUrl avatarUrl() const { return content().avatarUrl; } + QString reason() const { return content().reason; } + bool changesMembership() const; bool isInvite() const; bool isJoin() const; bool isLeave() const; -- cgit v1.2.3 From 16d6700950f5f0ebd71481efd5e1a24f04e3c651 Mon Sep 17 00:00:00 2001 From: Black Hat Date: Sat, 28 Dec 2019 16:12:10 +0800 Subject: isEditing(): fix a bug in replacing events --- lib/room.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/room.cpp b/lib/room.cpp index b6925be5..696a5f1b 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -2137,7 +2137,7 @@ inline bool isEditing(const RoomEventPtr& ep) if (is(*ep)) return true; if (auto* msgEvent = eventCast(ep)) - return msgEvent->replacedEvent().isEmpty(); + return !msgEvent->replacedEvent().isEmpty(); return false; } -- cgit v1.2.3