From abbab8d8f8c566bc2c9cdf766c6fbb11d978ca47 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 23 Jan 2022 17:11:22 +0100 Subject: Omittable: split out from util.h and refresh Improvements: - Quotient::lift() - a way to invoke a function on an optional (including Omittable) or a pointer if it's 'truthy'. Doesn't need enhanced function_traits<>, only the standard library; works on any number of arguments that can be dereferenced and casted to bool. - then() - the version of lift() as a member function. - edit() was renamed to ensure() (edit() might become a read-write counterpart of then() at some point). It's not really used across libQuotient codebase (or elsewhere) but is staying there just in case. It can also accept an initializer, removing the requirement of default-constructibility. - Quotient::merge() is simplified, with one universal implementation covering both Omittable/optional and plain values. - All that now lives in its dedicated pair of files, further decluttering util.h --- lib/connection.h | 1 + lib/converters.h | 1 + lib/omittable.cpp | 34 +++++++++ lib/omittable.h | 223 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ lib/util.h | 141 ---------------------------------- 5 files changed, 259 insertions(+), 141 deletions(-) create mode 100644 lib/omittable.cpp create mode 100644 lib/omittable.h (limited to 'lib') diff --git a/lib/connection.h b/lib/connection.h index 28688cc1..dc2eaad1 100644 --- a/lib/connection.h +++ b/lib/connection.h @@ -8,6 +8,7 @@ #include "ssosession.h" #include "qt_connection_util.h" #include "quotient_common.h" +#include "util.h" #include "csapi/login.h" #include "csapi/create_room.h" diff --git a/lib/converters.h b/lib/converters.h index 8eea1cd3..e7bc6898 100644 --- a/lib/converters.h +++ b/lib/converters.h @@ -3,6 +3,7 @@ #pragma once +#include "omittable.h" #include "util.h" #include diff --git a/lib/omittable.cpp b/lib/omittable.cpp new file mode 100644 index 00000000..245ae721 --- /dev/null +++ b/lib/omittable.cpp @@ -0,0 +1,34 @@ +// SPDX-FileCopyrightText: 2021 Kitsune Ral +// SPDX-License-Identifier: LGPL-2.1-or-later + +#include "omittable.h" + +// Omittable<> tests +using namespace Quotient; + +Omittable testFn(bool) { return 0; } +bool testFn2(int) { return false; } +static_assert( + std::is_same_v>().then(testFn)), + Omittable>); +static_assert( + std::is_same_v< + decltype(std::declval>().then_or(testFn, 0)), int>); +static_assert( + std::is_same_v>().then(testFn)), + Omittable>); +static_assert(std::is_same_v>() + .then(testFn2) + .then(testFn)), + Omittable>); +static_assert(std::is_same_v>() + .then(testFn) + .then_or(testFn2, false)), + bool>); + +constexpr auto visitTestFn(int, bool) { return false; } +static_assert( + std::is_same_v, decltype(lift(testFn2, Omittable()))>); +static_assert(std::is_same_v, + decltype(lift(visitTestFn, Omittable(), + Omittable()))>); diff --git a/lib/omittable.h b/lib/omittable.h new file mode 100644 index 00000000..b5efecf5 --- /dev/null +++ b/lib/omittable.h @@ -0,0 +1,223 @@ +// SPDX-FileCopyrightText: 2018 Kitsune Ral +// SPDX-License-Identifier: LGPL-2.1-or-later + +#pragma once + +#include +#include + +namespace Quotient { + +template +class Omittable; + +constexpr auto none = std::nullopt; + +//! \brief Lift an operation into dereferenceable types (Omittables or pointers) +//! +//! This is a more generic version of Omittable::then() that extends to +//! an arbitrary number of arguments of any type that is dereferenceable (unary +//! operator*() can be applied to it) and (explicitly or implicitly) convertible +//! to bool. This allows to streamline checking for nullptr/none before applying +//! the operation on the underlying types. \p fn is only invoked if all \p args +//! are "truthy" (i.e. (... && bool(args)) == true). +//! \param fn A callable that should accept the types stored inside +//! Omittables/pointers passed in \p args +//! \return Always an Omittable: if \p fn returns another type, lift() wraps +//! it in an Omittable; if \p fn returns an Omittable, that return value +//! (or none) is returned as is. +template +inline auto lift(FnT&& fn, MaybeTs&&... args) +{ + return (... && bool(args)) + ? Omittable(std::invoke(std::forward(fn), *args...)) + : none; +} + +/** `std::optional` with tweaks + * + * 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. + * - ensure() to provide a safe and explicit lvalue accessor instead of + * those above. 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.ensure().inner.ensure().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. + * - then() and then_or() to streamline read-only interrogation in a "monadic" + * interface. + */ +template +class Omittable : public std::optional { +public: + using base_type = std::optional; + using value_type = std::decay_t; + + using std::optional::optional; + + // 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=(std::move(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)); } + + // 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. + auto& value() = delete; + const auto& value() const = delete; + + template + value_type& ensure(U&& defaultValue = value_type {}) + { + return this->has_value() ? this->operator*() + : this->emplace(std::forward(defaultValue)); + } + value_type& ensure(const value_type& defaultValue) + { + return ensure<>(defaultValue); + } + value_type& ensure(value_type&& defaultValue) + { + return ensure<>(std::move(defaultValue)); + } + + //! 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), + //! in other words, if the current Omittable has changed; + //! false otherwise + template + auto merge(const std::optional& other) + -> std::enable_if_t, bool> + { + if (!other || (this->has_value() && **this == *other)) + return false; + this->emplace(*other); + return true; + } + + // 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. + + 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*(); } + + // The below is inspired by the proposed std::optional monadic operations + // (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p0798r6.html). + + //! \brief Lift a callable into the Omittable + //! + //! 'Lifting', as used in functional programming, means here invoking + //! a callable (e.g., a function) on the contents of the Omittable if it has + //! any and wrapping the returned value (that may be of a different type T2) + //! into a new Omittable\. If the current Omittable is empty, + //! the invocation is skipped altogether and Omittable\{none} is + //! returned instead. + //! \note if \p fn already returns an Omittable (i.e., it is a 'functor', + //! in functional programming terms), then() will not wrap another + //! Omittable around but will just return what \p fn returns. The + //! same doesn't hold for the parameter: if \p fn accepts an Omittable + //! you have to wrap it in another Omittable before calling then(). + //! \return `none` if the current Omittable has `none`; + //! otherwise, the Omittable returned from a call to \p fn + //! \tparam FnT a callable with \p T (or const T&) + //! returning Omittable, T2 is any supported type + //! \sa then_or, transform + template + auto then(FnT&& fn) const& + { + return lift(std::forward(fn), *this); + } + + //! \brief Lift a callable into the rvalue Omittable + //! + //! This is an rvalue overload for then(). + template + auto then(FnT&& fn) && + { + return lift(std::forward(fn), *this); + } + + //! \brief Lift a callable into the const lvalue Omittable, with a fallback + //! + //! This effectively does the same what then() does, except that it returns + //! a value of type returned by the callable, or the provided fallback value + //! if the current Omittable is empty. This is a typesafe version to apply + //! an operation on an Omittable without having to deal with another + //! Omittable afterwards. + template + auto then_or(FnT&& fn, FallbackT&& fallback) const& + { + return then(std::forward(fn)) + .value_or(std::forward(fallback)); + } + + //! \brief Lift a callable into the rvalue Omittable, with a fallback + //! + //! This is an overload for functions that accept rvalue + template + auto then_or(FnT&& fn, FallbackT&& fallback) && + { + return then(std::forward(fn)) + .value_or(std::forward(fallback)); + } +}; + +template +Omittable(T&&) -> Omittable; + +//! \brief Merge the value from an optional +//! This is an adaptation of Omittable::merge() to the case when the value +//! on the left hand side is not an Omittable. +//! \return true if \p rhs is not omitted and the \p lhs value was different, +//! in other words, if \p lhs has changed; +//! false otherwise +template +inline auto merge(T1& lhs, const std::optional& rhs) + -> std::enable_if_t, bool> +{ + if (!rhs || lhs == *rhs) + return false; + lhs = *rhs; + return true; +} + +} // namespace Quotient diff --git a/lib/util.h b/lib/util.h index 3505b62f..753eb1ea 100644 --- a/lib/util.h +++ b/lib/util.h @@ -9,10 +9,8 @@ #include #include -#include #include #include -#include #ifndef Q_DISABLE_MOVE // Q_DISABLE_MOVE was introduced in Q_VERSION_CHECK(5,13,0) @@ -52,145 +50,6 @@ struct HashQ { template using UnorderedMap = std::unordered_map>; -namespace _impl { - template - constexpr auto IsOmittableValue = false; - template - constexpr auto IsOmittable = IsOmittableValue>; -} - -constexpr auto none = std::nullopt; - -/** `std::optional` with tweaks - * - * 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 { -public: - using base_type = std::optional; - using value_type = std::decay_t; - - using std::optional::optional; - - // 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)); } - - // 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() ? 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), - //! in other words, if the current Omittable has changed; - //! false otherwise - template - auto merge(const Omittable& other) - -> std::enable_if_t, bool> - { - if (!other || (this->has_value() && **this == *other)) - return false; - emplace(*other); - return true; - } - - // 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. - - 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*(); } -}; -template -Omittable(T&&) -> Omittable; - -namespace _impl { - template - constexpr auto IsOmittableValue> = true; -} - -template -inline auto merge(Omittable& lhs, T2&& rhs) -{ - return lhs.merge(std::forward(rhs)); -} - -//! \brief Merge the value from an Omittable -//! This is an adaptation of Omittable::merge() to the case when the value -//! on the left hand side is not an Omittable. -//! \return true if \p rhs is not omitted and the \p lhs value was different, -//! in other words, if \p lhs has changed; -//! false otherwise -template -inline auto merge(T1& lhs, const Omittable& rhs) - -> std::enable_if_t - && std::is_convertible_v, bool> -{ - if (!rhs || lhs == *rhs) - return false; - lhs = *rhs; - return true; -} - constexpr auto operator"" _ls(const char* s, std::size_t size) { return QLatin1String(s, int(size)); -- cgit v1.2.3 From 29775218e0c8b6c176015dd3128a0d545906ae6f Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Thu, 2 Dec 2021 20:54:39 +0100 Subject: Cleanup some #includes --- lib/events/eventcontent.cpp | 1 - lib/events/receiptevent.cpp | 1 - lib/events/roomevent.cpp | 1 - lib/events/roommemberevent.cpp | 1 - lib/jobs/basejob.cpp | 2 -- lib/jobs/basejob.h | 6 +++--- 6 files changed, 3 insertions(+), 9 deletions(-) (limited to 'lib') diff --git a/lib/events/eventcontent.cpp b/lib/events/eventcontent.cpp index 22878d4c..4ce130a6 100644 --- a/lib/events/eventcontent.cpp +++ b/lib/events/eventcontent.cpp @@ -4,7 +4,6 @@ #include "eventcontent.h" #include "converters.h" -#include "util.h" #include "logging.h" #include diff --git a/lib/events/receiptevent.cpp b/lib/events/receiptevent.cpp index 72dbf2e3..7f06d99f 100644 --- a/lib/events/receiptevent.cpp +++ b/lib/events/receiptevent.cpp @@ -20,7 +20,6 @@ Example of a Receipt Event: #include "receiptevent.h" -#include "converters.h" #include "logging.h" using namespace Quotient; diff --git a/lib/events/roomevent.cpp b/lib/events/roomevent.cpp index b728e0bf..3502e3f7 100644 --- a/lib/events/roomevent.cpp +++ b/lib/events/roomevent.cpp @@ -3,7 +3,6 @@ #include "roomevent.h" -#include "converters.h" #include "logging.h" #include "redactionevent.h" diff --git a/lib/events/roommemberevent.cpp b/lib/events/roommemberevent.cpp index 3141f6b5..b4770224 100644 --- a/lib/events/roommemberevent.cpp +++ b/lib/events/roommemberevent.cpp @@ -4,7 +4,6 @@ #include "roommemberevent.h" -#include "converters.h" #include "logging.h" #include diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index f518a1b0..b6858b5a 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -5,11 +5,9 @@ #include "basejob.h" #include "connectiondata.h" -#include "quotient_common.h" #include #include -#include #include #include #include diff --git a/lib/jobs/basejob.h b/lib/jobs/basejob.h index 9ed58ba8..555c602b 100644 --- a/lib/jobs/basejob.h +++ b/lib/jobs/basejob.h @@ -5,9 +5,9 @@ #pragma once #include "requestdata.h" -#include "../logging.h" -#include "../converters.h" -#include "../quotient_common.h" +#include "logging.h" +#include "converters.h" // Common for csapi/ headers even though not used here +#include "quotient_common.h" // For DECL_DEPRECATED_ENUMERATOR #include #include -- cgit v1.2.3 From 1215888438b901ad80e8bf9882cdcc6a3f057399 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sat, 4 Dec 2021 20:12:59 +0100 Subject: converters.h: slightly clearer code in _impl::addTo --- lib/converters.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'lib') diff --git a/lib/converters.h b/lib/converters.h index e7bc6898..8ddf6e45 100644 --- a/lib/converters.h +++ b/lib/converters.h @@ -288,6 +288,8 @@ QVariantHash QUOTIENT_API fromJson(const QJsonValue& jv); // Conditional insertion into a QJsonObject +constexpr bool IfNotEmpty = false; + namespace _impl { template inline void addTo(QJsonObject& o, const QString& k, ValT&& v) @@ -333,7 +335,7 @@ namespace _impl { // This one is for types that have isEmpty() when Force is false template - struct AddNode().isEmpty())> { + struct AddNode().isEmpty())> { template static void impl(ContT& container, const QString& key, ForwardedT&& value) @@ -343,9 +345,9 @@ namespace _impl { } }; - // This one unfolds Omittable<> (also only when Force is false) + // This one unfolds Omittable<> (also only when IfNotEmpty is requested) template - struct AddNode, false> { + struct AddNode, IfNotEmpty> { template static void impl(ContT& container, const QString& key, const OmittableT& value) @@ -356,8 +358,6 @@ namespace _impl { }; } // namespace _impl -constexpr bool IfNotEmpty = false; - /*! Add a key-value pair to QJsonObject or QUrlQuery * * Adds a key-value pair(s) specified by \p key and \p value to -- cgit v1.2.3 From 8183a33c130f1284404edc61767ff6d29402d200 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 23 Jan 2022 17:11:45 +0100 Subject: RoomStateView This class is called to provide an arbitrary snapshot of a room state; as the first step, Room::currentState() returns an instance of this class that stores, well, the current state. Implelementation-wise it's the same hash map of two-part state event keys to const event pointers; however, RoomStateView provides additional operations: - get(), that deprecates Room::getCurrentState(), returns a pointer to a particular event if the current state has it. Unlike the original method, the pointer returned from this one can be nullptr; this is done to get rid of stubbed state events that have to be created everytime a "state miss" occurred (i.e., when getCurrentState() does not find an existing event in the current state). - eventsOfType() - this is a new place for Room::stateEventsOfType() introduced recently. - query() - this is a way to specify a piece of the state content that you need to retrieve by passing a member function or a function object that retrieves it. That is especially convenient with member functions of the event class; just pass the pointer to this member function, and query() will parse the event type it has to retrieve out of it and call that member function on the event object. Returns an Omittable<>; if the respective piece of state doesn't exist, you'll get `Quotient::none` (the same as `std::nullopt`). - queryOr() - the same but with the fallback value; instead of an Omittable<>, the fallback value will be returned if the needed event is not found. --- CMakeLists.txt | 1 + lib/room.cpp | 150 ++++++++++++++++++++++++-------------------------- lib/room.h | 54 ++++++++---------- lib/roomstateview.cpp | 35 ++++++++++++ lib/roomstateview.h | 127 ++++++++++++++++++++++++++++++++++++++++++ lib/user.cpp | 17 ++++-- 6 files changed, 268 insertions(+), 116 deletions(-) create mode 100644 lib/roomstateview.cpp create mode 100644 lib/roomstateview.h (limited to 'lib') diff --git a/CMakeLists.txt b/CMakeLists.txt index 13c8f591..ddf11680 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -134,6 +134,7 @@ list(APPEND lib_SRCS lib/ssosession.h lib/ssosession.cpp lib/logging.h lib/logging.cpp lib/room.h lib/room.cpp + lib/roomstateview.h lib/roomstateview.cpp lib/user.h lib/user.cpp lib/avatar.h lib/avatar.cpp lib/uri.h lib/uri.cpp diff --git a/lib/room.cpp b/lib/room.cpp index ba63f50d..1450eb3b 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -16,6 +16,7 @@ #include "syncdata.h" #include "user.h" #include "eventstats.h" +#include "roomstateview.h" // NB: since Qt 6, moc_room.cpp needs User fully defined #include "moc_room.cpp" @@ -103,7 +104,7 @@ public: static decltype(baseState) stubbedState; /// The state of the room at syncEdge() /// \sa syncEdge - QHash currentState; + RoomStateView currentState; /// Servers with aliases for this room except the one of the local user /// \sa Room::remoteAliases QSet aliasServers; @@ -227,34 +228,6 @@ public: return evt; } - QVector stateEventsOfType(const QString& evtType) const - { - auto vals = QVector(); - for (auto it = currentState.cbegin(); it != currentState.cend(); ++it) - if (it.key().first == evtType) - vals.append(it.value()); - - return vals; - } - - template - const EventT* getCurrentState(const QString& stateKey = {}) const - { - const auto* evt = getCurrentState({ EventT::matrixTypeId(), stateKey }); - Q_ASSERT(evt->type() == EventT::typeId() - && evt->matrixType() == EventT::matrixTypeId()); - return static_cast(evt); - } - -// template -// const auto& getCurrentStateContent(const QString& stateKey = {}) const -// { -// if (const auto* evt = -// currentState.value({ EventT::matrixTypeId(), stateKey }, nullptr)) -// return evt->content(); -// return EventT::content_type() -// } - template Changes updateStateFrom(EventArrayT&& events) { @@ -480,8 +453,8 @@ const QString& Room::id() const { return d->id; } QString Room::version() const { - const auto v = d->getCurrentState()->version(); - return v.isEmpty() ? QStringLiteral("1") : v; + const auto v = currentState().query(&RoomCreateEvent::version); + return v && !v->isEmpty() ? *v : QStringLiteral("1"); } bool Room::isUnstable() const @@ -492,7 +465,10 @@ bool Room::isUnstable() const QString Room::predecessorId() const { - return d->getCurrentState()->predecessor().roomId; + if (const auto* evt = currentState().get()) + return evt->predecessor().roomId; + + return {}; } Room* Room::predecessor(JoinStates statesFilter) const @@ -507,7 +483,8 @@ Room* Room::predecessor(JoinStates statesFilter) const QString Room::successorId() const { - return d->getCurrentState()->successorRoomId(); + return currentState().queryOr(&RoomTombstoneEvent::successorRoomId, + QString()); } Room* Room::successor(JoinStates statesFilter) const @@ -534,39 +511,41 @@ bool Room::allHistoryLoaded() const QString Room::name() const { - return d->getCurrentState()->name(); + return currentState().queryOr(&RoomNameEvent::name, QString()); } QStringList Room::aliases() const { - const auto* evt = d->getCurrentState(); - auto result = evt->altAliases(); - if (!evt->alias().isEmpty()) - result << evt->alias(); - return result; + if (const auto* evt = currentState().get()) { + auto result = evt->altAliases(); + if (!evt->alias().isEmpty()) + result << evt->alias(); + return result; + } + return {}; } QStringList Room::altAliases() const { - return d->getCurrentState()->altAliases(); + return currentState().queryOr(&RoomCanonicalAliasEvent::altAliases, + QStringList()); } QString Room::canonicalAlias() const { - return d->getCurrentState()->alias(); + return currentState().queryOr(&RoomCanonicalAliasEvent::alias, QString()); } QString Room::displayName() const { return d->displayname; } QStringList Room::pinnedEventIds() const { - return d->getCurrentState()->pinnedEvents(); + return currentState().queryOr(&RoomPinnedEvent::pinnedEvents, QStringList()); } QVector Quotient::Room::pinnedEvents() const { - const auto& pinnedIds = d->getCurrentState()->pinnedEvents(); QVector pinnedEvents; - for (auto&& evtId: pinnedIds) + for (const auto& evtId : pinnedEventIds()) if (const auto& it = findInTimeline(evtId); it != historyEdge()) pinnedEvents.append(it->event()); @@ -582,7 +561,7 @@ void Room::refreshDisplayName() { d->updateDisplayname(); } QString Room::topic() const { - return d->getCurrentState()->topic(); + return currentState().queryOr(&RoomTopicEvent::topic, QString()); } QString Room::avatarMediaId() const { return d->avatar.mediaId(); } @@ -621,7 +600,8 @@ JoinState Room::memberJoinState(User* user) const Membership Room::memberState(const QString& userId) const { - return d->getCurrentState(userId)->membership(); + return currentState().queryOr(userId, &RoomMemberEvent::membership, + Membership::Leave); } bool Room::isMember(const QString& userId) const @@ -899,8 +879,9 @@ bool Room::canSwitchVersions() const if (!successorId().isEmpty()) return false; // No one can upgrade a room that's already upgraded - if (const auto* plEvt = d->getCurrentState()) { - const auto currentUserLevel = plEvt->powerLevelForUser(localUser()->id()); + if (const auto* plEvt = currentState().get()) { + const auto currentUserLevel = + plEvt->powerLevelForUser(localUser()->id()); const auto tombstonePowerLevel = plEvt->powerLevelForState("m.room.tombstone"_ls); return currentUserLevel >= tombstonePowerLevel; @@ -1008,6 +989,16 @@ const Room::RelatedEvents Room::relatedEvents( return relatedEvents(evt.id(), relType); } +const RoomCreateEvent* Room::creation() const +{ + return currentState().get(); +} + +const RoomTombstoneEvent *Room::tombstone() const +{ + return currentState().get(); +} + void Room::Private::getAllMembers() { // If already loaded or already loading, there's nothing to do here. @@ -1472,7 +1463,9 @@ int Room::timelineSize() const { return int(d->timeline.size()); } bool Room::usesEncryption() const { - return !d->getCurrentState()->algorithm().isEmpty(); + return !currentState() + .queryOr(&EncryptionEvent::algorithm, QString()) + .isEmpty(); } const StateEventBase* Room::getCurrentState(const QString& evtType, @@ -1481,13 +1474,7 @@ const StateEventBase* Room::getCurrentState(const QString& evtType, return d->getCurrentState({ evtType, stateKey }); } -const QVector -Room::stateEventsOfType(const QString& evtType) const -{ - return d->stateEventsOfType(evtType); -} - -const QHash& Room::currentState() const +RoomStateView Room::currentState() const { return d->currentState; } @@ -1564,7 +1551,7 @@ Room::Changes Room::Private::setSummary(RoomSummary&& newSummary) void Room::Private::insertMemberIntoMap(User* u) { const auto maybeUserName = - getCurrentState(u->id())->newDisplayName(); + currentState.query(u->id(), &RoomMemberEvent::newDisplayName); if (!maybeUserName) qCWarning(MEMBERS) << "insertMemberIntoMap():" << u->id() << "has no name (even empty)"; @@ -1594,9 +1581,9 @@ void Room::Private::insertMemberIntoMap(User* u) void Room::Private::removeMemberFromMap(User* u) { - const auto userName = - getCurrentState( - u->id())->newDisplayName().value_or(QString()); + const auto userName = currentState.queryOr(u->id(), + &RoomMemberEvent::newDisplayName, + QString()); qCDebug(MEMBERS) << "removeMemberFromMap(), username" << userName << "for user" << u->id(); @@ -1679,10 +1666,13 @@ Room::Private::moveEventsToTimeline(RoomEventsRange events, QString Room::memberName(const QString& mxId) const { // See https://github.com/matrix-org/matrix-doc/issues/1375 - const auto rme = getCurrentState(mxId); - return rme->newDisplayName() ? *rme->newDisplayName() - : rme->prevContent() ? rme->prevContent()->displayName.value_or(QString()) - : QString(); + if (const auto rme = currentState().get(mxId)) { + if (rme->newDisplayName()) + return *rme->newDisplayName(); + if (rme->prevContent() && rme->prevContent()->displayName) + return *rme->prevContent()->displayName; + } + return {}; } QString Room::roomMembername(const User* u) const @@ -1737,10 +1727,13 @@ QString Room::htmlSafeMemberName(const QString& userId) const QUrl Room::memberAvatarUrl(const QString &mxId) const { // See https://github.com/matrix-org/matrix-doc/issues/1375 - const auto rme = getCurrentState(mxId); - return rme->newAvatarUrl() ? *rme->newAvatarUrl() - : rme->prevContent() ? rme->prevContent()->avatarUrl.value_or(QUrl()) - : QUrl(); + if (const auto rme = currentState().get(mxId)) { + if (rme->newAvatarUrl()) + return *rme->newAvatarUrl(); + if (rme->prevContent() && rme->prevContent()->avatarUrl) + return *rme->prevContent()->avatarUrl; + } + return {}; } Room::Changes Room::Private::updateStatsFromSyncData(const SyncRoomData& data, @@ -2489,12 +2482,14 @@ bool Room::Private::processRedaction(const RedactionEvent& redaction) auto oldEvent = ti.replaceEvent(makeRedacted(*ti, redaction)); qCDebug(EVENTS) << "Redacted" << oldEvent->id() << "with" << redaction.id(); if (oldEvent->isStateEvent()) { - const StateEventKey evtKey { oldEvent->matrixType(), - oldEvent->stateKey() }; - Q_ASSERT(currentState.contains(evtKey)); - if (currentState.value(evtKey) == oldEvent.get()) { - Q_ASSERT(ti.index() >= 0); // Historical states can't be in - // currentState + // Check whether the old event was a part of current state; if it was, + // update the current state to the redacted event object. + const auto currentStateEvt = + currentState.get(oldEvent->matrixType(), oldEvent->stateKey()); + Q_ASSERT(currentStateEvt); + if (currentStateEvt == oldEvent.get()) { + // Historical states can't be in currentState + Q_ASSERT(ti.index() >= 0); qCDebug(STATE).nospace() << "Redacting state " << oldEvent->matrixType() << "/" << oldEvent->stateKey(); @@ -2514,6 +2509,7 @@ bool Room::Private::processRedaction(const RedactionEvent& redaction) } q->onRedaction(*oldEvent, *ti); emit q->replacedEvent(ti.event(), rawPtr(oldEvent)); + // By now, all references to oldEvent must have been updated to ti.event() return true; } @@ -2754,7 +2750,7 @@ void Room::Private::addHistoricalMessageEvents(RoomEvents&& events) for (const auto& eptr : events) { const auto& e = *eptr; if (e.isStateEvent() - && !currentState.contains({ e.matrixType(), e.stateKey() })) { + && !currentState.contains(e.matrixType(), e.stateKey())) { changes |= q->processStateEvent(e); } } @@ -2791,9 +2787,7 @@ Room::Changes Room::processStateEvent(const RoomEvent& e) return Change::None; // Find a value (create an empty one if necessary) and get a reference - // to it. Can't use getCurrentState<>() because it (creates and) returns - // a stub if a value is not found, and what's needed here is a "real" event - // or nullptr. + // to it, anticipating a change further in the function. auto& curStateEvent = d->currentState[{ e.matrixType(), e.stateKey() }]; // Prepare for the state change // clang-format off diff --git a/lib/room.h b/lib/room.h index 15bc7648..d49cfb55 100644 --- a/lib/room.h +++ b/lib/room.h @@ -10,6 +10,7 @@ #pragma once #include "connection.h" +#include "roomstateview.h" #include "eventitem.h" #include "quotient_common.h" @@ -399,14 +400,8 @@ public: const RelatedEvents relatedEvents(const RoomEvent& evt, EventRelation::reltypeid_t relType) const; - const RoomCreateEvent* creation() const - { - return getCurrentState(); - } - const RoomTombstoneEvent* tombstone() const - { - return getCurrentState(); - } + const RoomCreateEvent* creation() const; + const RoomTombstoneEvent* tombstone() const; bool displayed() const; /// Mark the room as currently displayed to the user @@ -760,45 +755,40 @@ 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 Quotient::StateEventBase* + [[deprecated("Use currentState().get() instead; " + "make sure to check its result for nullptrs")]] // + const Quotient::StateEventBase* getCurrentState(const QString& evtType, const QString& stateKey = {}) const; - /// Get all state events in the room. - /*! This method returns all known state events that have occured in - * the room, as a mapping from the event type and state key to value. - */ - const QHash& currentState() const; - - /// Get all state events in the room of a certain type. - /*! This method returns all known state events that have occured in - * the room of the given type. - */ - Q_INVOKABLE const QVector - stateEventsOfType(const QString& evtType) 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 + [[deprecated("Use currentState().get() instead; " + "make sure to check its result for nullptrs")]] // const EvT* getCurrentState(const QString& stateKey = {}) const { - const auto* evt = - eventCast(getCurrentState(EvT::matrixTypeId(), stateKey)); + QT_IGNORE_DEPRECATIONS( + const auto* evt = eventCast( + getCurrentState(EvT::matrixTypeId(), stateKey));) Q_ASSERT(evt); Q_ASSERT(evt->matrixTypeId() == EvT::matrixTypeId() && evt->stateKey() == stateKey); 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(). - */ + /// \brief Get the current room state + RoomStateView currentState() const; + + //! \brief 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 { diff --git a/lib/roomstateview.cpp b/lib/roomstateview.cpp new file mode 100644 index 00000000..94c88eee --- /dev/null +++ b/lib/roomstateview.cpp @@ -0,0 +1,35 @@ +// SPDX-FileCopyrightText: 2021 Kitsune Ral +// SPDX-License-Identifier: LGPL-2.1-or-later + +#include "roomstateview.h" + +using namespace Quotient; + +const StateEventBase* RoomStateView::get(const QString& evtType, + const QString& stateKey) const +{ + return value({ evtType, stateKey }); +} + +bool RoomStateView::contains(const QString& evtType, + const QString& stateKey) const +{ + return contains({ evtType, stateKey }); +} + +QJsonObject RoomStateView::contentJson(const QString& evtType, + const QString& stateKey) const +{ + return queryOr(evtType, stateKey, &Event::contentJson, QJsonObject()); +} + +const QVector +RoomStateView::eventsOfType(const QString& evtType) const +{ + auto vals = QVector(); + for (auto it = cbegin(); it != cend(); ++it) + if (it.key().first == evtType) + vals.append(it.value()); + + return vals; +} diff --git a/lib/roomstateview.h b/lib/roomstateview.h new file mode 100644 index 00000000..cab69ae3 --- /dev/null +++ b/lib/roomstateview.h @@ -0,0 +1,127 @@ +// SPDX-FileCopyrightText: 2021 Kitsune Ral +// SPDX-License-Identifier: LGPL-2.1-or-later + +#pragma once + +#include "events/stateevent.h" + +#include + +namespace Quotient { + +class Room; + +class RoomStateView : private QHash { + Q_GADGET +public: + const QHash& events() const + { + return *this; + } + + //! \brief Get a state event with the given event type and state key + //! \return A state event corresponding to the pair of event type + //! \p evtType and state key \p stateKey, or nullptr if there's + //! no such \p evtType / \p stateKey combination in the current + //! state. + //! \warning In libQuotient 0.7 the return type changed to an OmittableCref + //! which is effectively a nullable const reference wrapper. You + //! have to check that it has_value() before using. Alternatively + //! you can now use queryCurrentState() to access state safely. + //! \sa getCurrentStateContentJson + const StateEventBase* get(const QString& evtType, + const QString& stateKey = {}) const; + + //! \brief 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. + //! \warning In libQuotient 0.7 the return type changed to an Omittable with + //! a reference wrapper inside - you have to check that it + //! has_value() before using. Alternatively you can now use + //! queryCurrentState() to access state safely. + template + const EvT* get(const QString& stateKey = {}) const + { + static_assert(std::is_base_of_v); + if (const auto* evt = get(EvT::matrixTypeId(), stateKey)) { + Q_ASSERT(evt->matrixType() == EvT::matrixTypeId() + && evt->stateKey() == stateKey); + return eventCast(evt); + } + return nullptr; + } + + using QHash::contains; + + bool contains(const QString& evtType, const QString& stateKey = {}) const; + + template + bool contains(const QString& stateKey = {}) const + { + return contains(EvT::matrixTypeId(), stateKey); + } + + //! \brief Get the content of the current state event with the given + //! event type and state key + //! \return An empty object if there's no event in the current state with + //! this event type and state key; the contents of the event + //! 'content' object otherwise + Q_INVOKABLE QJsonObject contentJson(const QString& evtType, + const QString& stateKey = {}) const; + + //! \brief Get all state events in the room of a certain type. + //! + //! This method returns all known state events that have occured in + //! the room of the given type. + const QVector + eventsOfType(const QString& evtType) const; + + template + auto query(const QString& evtType, const QString& stateKey, FnT&& fn) const + { + return lift(std::forward(fn), get(evtType, stateKey)); + } + + template + auto query(const QString& stateKey, FnT&& fn) const + { + using EventT = std::decay_t>; + static_assert(std::is_base_of_v); + return lift(std::forward(fn), get(stateKey)); + } + + template + auto queryOr(const QString& evtType, const QString& stateKey, FnT&& fn, + FallbackT&& fallback) const + { + return lift(std::forward(fn), get(evtType, stateKey)) + .value_or(std::forward(fallback)); + } + + template + auto query(FnT&& fn) const + { + return query({}, std::forward(fn)); + } + + template + auto queryOr(const QString& stateKey, FnT&& fn, FallbackT&& fallback) const + { + using EventT = std::decay_t>; + static_assert(std::is_base_of_v); + return lift(std::forward(fn), get(stateKey)) + .value_or(std::forward(fallback)); + } + + template + auto queryOr(FnT&& fn, FallbackT&& fallback) const + { + return queryOr({}, std::forward(fn), + std::forward(fallback)); + } + +private: + friend class Room; +}; +} // namespace Quotient diff --git a/lib/user.cpp b/lib/user.cpp index 0dbc444a..f7840c40 100644 --- a/lib/user.cpp +++ b/lib/user.cpp @@ -119,12 +119,17 @@ void User::rename(const QString& newName, const Room* r) return; } // #481: take the current state and update it with the new name - auto evtC = r->getCurrentState(id())->content(); - Q_ASSERT_X(evtC.membership == Membership::Join, __FUNCTION__, - "Attempt to rename a user that's not a room member"); - evtC.displayName = sanitized(newName); - r->setState(id(), move(evtC)); - // The state will be updated locally after it arrives with sync + if (const auto& maybeEvt = r->currentState().get(id())) { + auto content = maybeEvt->content(); + if (content.membership == Membership::Join) { + content.displayName = sanitized(newName); + r->setState(id(), move(content)); + // The state will be updated locally after it arrives with sync + return; + } + } + qCCritical(MEMBERS) + << "Attempt to rename a non-member in a room context - ignored"; } template -- cgit v1.2.3 From a1fcad591968ec717214a73a2dbe78f608207bc5 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 5 Dec 2021 10:59:44 +0100 Subject: Move away Omittable static tests to autotests/ These are not required to build libQuotient, and omittable.cpp entirely consisted of them. --- CMakeLists.txt | 2 +- autotests/CMakeLists.txt | 1 + autotests/utiltests.cpp | 45 +++++++++++++++++++++++++++++++++++++++++++++ lib/omittable.cpp | 34 ---------------------------------- 4 files changed, 47 insertions(+), 35 deletions(-) create mode 100644 autotests/utiltests.cpp delete mode 100644 lib/omittable.cpp (limited to 'lib') diff --git a/CMakeLists.txt b/CMakeLists.txt index ddf11680..89eb996a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -127,7 +127,7 @@ list(APPEND lib_SRCS lib/quotient_common.h lib/quotient_export.h lib/function_traits.h lib/function_traits.cpp - lib/omittable.h lib/omittable.cpp + lib/omittable.h lib/networkaccessmanager.h lib/networkaccessmanager.cpp lib/connectiondata.h lib/connectiondata.cpp lib/connection.h lib/connection.cpp diff --git a/autotests/CMakeLists.txt b/autotests/CMakeLists.txt index 282ab036..9efab0d1 100644 --- a/autotests/CMakeLists.txt +++ b/autotests/CMakeLists.txt @@ -12,3 +12,4 @@ function(QUOTIENT_ADD_TEST) endfunction() quotient_add_test(NAME callcandidateseventtest) +quotient_add_test(NAME utiltests) diff --git a/autotests/utiltests.cpp b/autotests/utiltests.cpp new file mode 100644 index 00000000..e3ec63d0 --- /dev/null +++ b/autotests/utiltests.cpp @@ -0,0 +1,45 @@ +// SPDX-FileCopyrightText: 2021 Kitsune Ral +// SPDX-License-Identifier: LGPL-2.1-or-later + +#include "omittable.h" + +#include + +// compile-time Omittable<> tests +using namespace Quotient; + +Omittable testFn(bool) { return 0; } +bool testFn2(int) { return false; } +static_assert( + std::is_same_v>().then(testFn)), + Omittable>); +static_assert( + std::is_same_v< + decltype(std::declval>().then_or(testFn, 0)), int>); +static_assert( + std::is_same_v>().then(testFn)), + Omittable>); +static_assert(std::is_same_v>() + .then(testFn2) + .then(testFn)), + Omittable>); +static_assert(std::is_same_v>() + .then(testFn) + .then_or(testFn2, false)), + bool>); + +constexpr auto visitTestFn(int, bool) { return false; } +static_assert( + std::is_same_v, decltype(lift(testFn2, Omittable()))>); +static_assert(std::is_same_v, + decltype(lift(visitTestFn, Omittable(), + Omittable()))>); + +class TestUtils : public QObject { + Q_OBJECT +private Q_SLOTS: + // TODO +}; + +QTEST_APPLESS_MAIN(TestUtils) +#include "utiltests.moc" diff --git a/lib/omittable.cpp b/lib/omittable.cpp deleted file mode 100644 index 245ae721..00000000 --- a/lib/omittable.cpp +++ /dev/null @@ -1,34 +0,0 @@ -// SPDX-FileCopyrightText: 2021 Kitsune Ral -// SPDX-License-Identifier: LGPL-2.1-or-later - -#include "omittable.h" - -// Omittable<> tests -using namespace Quotient; - -Omittable testFn(bool) { return 0; } -bool testFn2(int) { return false; } -static_assert( - std::is_same_v>().then(testFn)), - Omittable>); -static_assert( - std::is_same_v< - decltype(std::declval>().then_or(testFn, 0)), int>); -static_assert( - std::is_same_v>().then(testFn)), - Omittable>); -static_assert(std::is_same_v>() - .then(testFn2) - .then(testFn)), - Omittable>); -static_assert(std::is_same_v>() - .then(testFn) - .then_or(testFn2, false)), - bool>); - -constexpr auto visitTestFn(int, bool) { return false; } -static_assert( - std::is_same_v, decltype(lift(testFn2, Omittable()))>); -static_assert(std::is_same_v, - decltype(lift(visitTestFn, Omittable(), - Omittable()))>); -- cgit v1.2.3 From 9ae0d4e45befc79f621b03dc4efe869cd4277e06 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 23 Jan 2022 10:32:40 +0100 Subject: Refactor Room::setState() There are two important aspects here: - Introducing Room::setState(evtType, stateKey, contentJson). These components are ultimately what is getting sent to the homeserver, so it makes sense to expose a respective `setState()` overload. Unlike setState(event) the new overload can be Q_INVOKABLE. - Room::setState() is no more const. Although it doesn't cause any changes in Room class (and only transient changes in Room::Private), it ultimately initiates a change in the room state, so calling it const has always been a bit of hypocrisy. If you relied on that, you most likely do something wrong (see the fix to User::rename() in this very commit for a simple example of such wrongness). Also: the backend is simplified by calling the original templated Room::setState() instead of calling Room::Private::requestSetState() that does exactly the same thing. --- lib/room.cpp | 36 ++++++++++++++++++++---------------- lib/room.h | 11 ++++++++--- lib/user.cpp | 2 +- lib/user.h | 2 +- 4 files changed, 30 insertions(+), 21 deletions(-) (limited to 'lib') diff --git a/lib/room.cpp b/lib/room.cpp index 1450eb3b..abd6110c 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -297,7 +297,9 @@ public: QString doSendEvent(const RoomEvent* pEvent); void onEventSendingFailure(const QString& txnId, BaseJob* call = nullptr); - SetRoomStateWithKeyJob* requestSetState(const StateEventBase& event) + SetRoomStateWithKeyJob* requestSetState(const QString& evtType, + const QString& stateKey, + const QJsonObject& contentJson) { // if (event.roomId().isEmpty()) // event.setRoomId(id); @@ -305,14 +307,8 @@ public: // event.setSender(connection->userId()); // TODO: Queue up state events sending (see #133). // TODO: Maybe addAsPending() as well, despite having no txnId - return connection->callApi( - id, event.matrixType(), event.stateKey(), event.contentJson()); - } - - template - auto requestSetState(ArgTs&&... args) - { - return requestSetState(EvT(std::forward(args)...)); + return connection->callApi(id, evtType, stateKey, + contentJson); } /*! Apply redaction to the timeline @@ -2126,33 +2122,41 @@ QString Room::postJson(const QString& matrixType, return d->sendEvent(loadEvent(matrixType, eventContent)); } -SetRoomStateWithKeyJob* Room::setState(const StateEventBase& evt) const +SetRoomStateWithKeyJob* Room::setState(const StateEventBase& evt) +{ + return d->requestSetState(evt.matrixType(), evt.stateKey(), + evt.contentJson()); +} + +SetRoomStateWithKeyJob* Room::setState(const QString& evtType, + const QString& stateKey, + const QJsonObject& contentJson) { - return d->requestSetState(evt); + return d->requestSetState(evtType, stateKey, contentJson); } void Room::setName(const QString& newName) { - d->requestSetState(newName); + setState(newName); } void Room::setCanonicalAlias(const QString& newAlias) { - d->requestSetState(newAlias, altAliases()); + setState(newAlias, altAliases()); } void Room::setPinnedEvents(const QStringList& events) { - d->requestSetState(events); + setState(events); } void Room::setLocalAliases(const QStringList& aliases) { - d->requestSetState(canonicalAlias(), aliases); + setState(canonicalAlias(), aliases); } void Room::setTopic(const QString& newTopic) { - d->requestSetState(newTopic); + setState(newTopic); } bool isEchoEvent(const RoomEventPtr& le, const PendingEventItem& re) diff --git a/lib/room.h b/lib/room.h index d49cfb55..9f70d77a 100644 --- a/lib/room.h +++ b/lib/room.h @@ -781,6 +781,9 @@ public: /// \brief Get the current room state RoomStateView currentState() const; + //! Send a request to update the room state with the given event + SetRoomStateWithKeyJob* setState(const StateEventBase& evt); + //! \brief Set a state event of the given type with the given arguments //! //! This typesafe overload attempts to send a state event with the type @@ -790,7 +793,7 @@ public: //! the Matrix event type defined by \p EvT and the event content produced //! via EvT::contentJson(). template - auto setState(ArgTs&&... args) const + auto setState(ArgTs&&... args) { return setState(EvT(std::forward(args)...)); } @@ -824,8 +827,10 @@ public Q_SLOTS: QString retryMessage(const QString& txnId); void discardMessage(const QString& txnId); - /// Send a request to update the room state with the given event - SetRoomStateWithKeyJob* setState(const StateEventBase& evt) const; + //! Send a request to update the room state based on freeform inputs + SetRoomStateWithKeyJob* setState(const QString& evtType, + const QString& stateKey, + const QJsonObject& contentJson); void setName(const QString& newName); void setCanonicalAlias(const QString& newAlias); void setPinnedEvents(const QStringList& events); diff --git a/lib/user.cpp b/lib/user.cpp index f7840c40..4c3fc9e2 100644 --- a/lib/user.cpp +++ b/lib/user.cpp @@ -110,7 +110,7 @@ void User::rename(const QString& newName) }); } -void User::rename(const QString& newName, const Room* r) +void User::rename(const QString& newName, Room* r) { if (!r) { qCWarning(MAIN) << "Passing a null room to two-argument User::rename()" diff --git a/lib/user.h b/lib/user.h index 8412b7fd..dfbff4a0 100644 --- a/lib/user.h +++ b/lib/user.h @@ -96,7 +96,7 @@ public Q_SLOTS: /// Set a new name in the global user profile void rename(const QString& newName); /// Set a new name for the user in one room - void rename(const QString& newName, const Room* r); + void rename(const QString& newName, Room* r); /// Upload the file and use it as an avatar bool setAvatar(const QString& fileName); /// Upload contents of the QIODevice and set that as an avatar -- cgit v1.2.3 From 08612cb253417fe70ef45a1ad08663a0745d748a Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Fri, 10 Dec 2021 19:26:23 +0100 Subject: No more default construction of events Default construction was only done to support stubbed state in Room and even that did not really use those, opting to construct an event from an empty QJsonObject instead. Now that Room doesn't have stubbed state, default constructors are even less needed. --- lib/events/encryptionevent.h | 20 ++++++++++++-------- lib/events/roomcreateevent.h | 1 - lib/events/roommemberevent.h | 10 +++------- lib/events/roompowerlevelsevent.h | 4 +++- lib/events/roomtombstoneevent.h | 1 - lib/events/simplestateevents.h | 1 - 6 files changed, 18 insertions(+), 19 deletions(-) (limited to 'lib') diff --git a/lib/events/encryptionevent.h b/lib/events/encryptionevent.h index dfb28b2f..56913393 100644 --- a/lib/events/encryptionevent.h +++ b/lib/events/encryptionevent.h @@ -12,7 +12,11 @@ class QUOTIENT_API EncryptionEventContent : public EventContent::Base { public: enum EncryptionType : size_t { MegolmV1AesSha2 = 0, Undefined }; - explicit EncryptionEventContent(EncryptionType et = Undefined); + explicit(false) EncryptionEventContent(EncryptionType et); + [[deprecated("This constructor will require explicit EncryptionType soon")]] // + explicit EncryptionEventContent() + : EncryptionEventContent(Undefined) + {} explicit EncryptionEventContent(const QJsonObject& json); EncryptionType encryption; @@ -34,15 +38,15 @@ public: using EncryptionType = EncryptionEventContent::EncryptionType; Q_ENUM(EncryptionType) - explicit EncryptionEvent(const QJsonObject& obj = {}) // TODO: apropriate - // default value + explicit EncryptionEvent(const QJsonObject& obj) : StateEvent(typeId(), obj) {} - EncryptionEvent(EncryptionEvent&&) = delete; - template - EncryptionEvent(ArgTs&&... contentArgs) - : StateEvent(typeId(), matrixTypeId(), QString(), - std::forward(contentArgs)...) + [[deprecated("This constructor will require an explicit parameter soon")]] // +// explicit EncryptionEvent() +// : EncryptionEvent(QJsonObject()) +// {} + explicit EncryptionEvent(EncryptionEventContent&& content) + : StateEvent(typeId(), matrixTypeId(), QString(), std::move(content)) {} EncryptionType encryption() const { return content().encryption; } diff --git a/lib/events/roomcreateevent.h b/lib/events/roomcreateevent.h index 016855b9..989030ac 100644 --- a/lib/events/roomcreateevent.h +++ b/lib/events/roomcreateevent.h @@ -11,7 +11,6 @@ class QUOTIENT_API RoomCreateEvent : public StateEventBase { public: DEFINE_EVENT_TYPEID("m.room.create", RoomCreateEvent) - explicit RoomCreateEvent() : StateEventBase(typeId(), matrixTypeId()) {} explicit RoomCreateEvent(const QJsonObject& obj) : StateEventBase(typeId(), obj) {} diff --git a/lib/events/roommemberevent.h b/lib/events/roommemberevent.h index 5e446dbe..3296ae22 100644 --- a/lib/events/roommemberevent.h +++ b/lib/events/roommemberevent.h @@ -15,9 +15,7 @@ public: using MembershipType [[deprecated("Use Quotient::Membership instead")]] = Membership; - explicit MemberEventContent(Membership ms = Membership::Join) - : membership(ms) - {} + explicit(false) MemberEventContent(Membership ms) : membership(ms) {} explicit MemberEventContent(const QJsonObject& json); Membership membership; @@ -43,10 +41,8 @@ public: explicit RoomMemberEvent(const QJsonObject& obj) : StateEvent(typeId(), obj) {} - template - RoomMemberEvent(const QString& userId, ArgTs&&... contentArgs) - : StateEvent(typeId(), matrixTypeId(), userId, - std::forward(contentArgs)...) + RoomMemberEvent(const QString& userId, MemberEventContent&& content) + : StateEvent(typeId(), matrixTypeId(), userId, std::move(content)) {} //! \brief A special constructor to create unknown RoomMemberEvents diff --git a/lib/events/roompowerlevelsevent.h b/lib/events/roompowerlevelsevent.h index 80e27048..415cc814 100644 --- a/lib/events/roompowerlevelsevent.h +++ b/lib/events/roompowerlevelsevent.h @@ -36,10 +36,12 @@ protected: class QUOTIENT_API RoomPowerLevelsEvent : public StateEvent { - Q_GADGET public: DEFINE_EVENT_TYPEID("m.room.power_levels", RoomPowerLevelsEvent) + explicit RoomPowerLevelsEvent(PowerLevelsEventContent&& content) + : StateEvent(typeId(), matrixTypeId(), QString(), std::move(content)) + {} explicit RoomPowerLevelsEvent(const QJsonObject& obj) : StateEvent(typeId(), obj) {} diff --git a/lib/events/roomtombstoneevent.h b/lib/events/roomtombstoneevent.h index e336c448..15d26923 100644 --- a/lib/events/roomtombstoneevent.h +++ b/lib/events/roomtombstoneevent.h @@ -10,7 +10,6 @@ class QUOTIENT_API RoomTombstoneEvent : public StateEventBase { public: DEFINE_EVENT_TYPEID("m.room.tombstone", RoomTombstoneEvent) - explicit RoomTombstoneEvent() : StateEventBase(typeId(), matrixTypeId()) {} explicit RoomTombstoneEvent(const QJsonObject& obj) : StateEventBase(typeId(), obj) {} diff --git a/lib/events/simplestateevents.h b/lib/events/simplestateevents.h index e6c05880..9610574b 100644 --- a/lib/events/simplestateevents.h +++ b/lib/events/simplestateevents.h @@ -35,7 +35,6 @@ namespace EventContent { public: \ using value_type = content_type::value_type; \ DEFINE_EVENT_TYPEID(_TypeId, _Name) \ - explicit _Name() : _Name(value_type()) {} \ template \ explicit _Name(T&& value) \ : StateEvent(typeId(), matrixTypeId(), QString(), \ -- cgit v1.2.3 From 1747575321cda4fc11f90c2ffb2148a69d0d9946 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Fri, 28 Jan 2022 17:08:10 +0100 Subject: QUO_IMPLICIT Because Apple Clang choked on `explicit(false)`. --- lib/events/encryptionevent.h | 3 ++- lib/events/roommemberevent.h | 2 +- lib/quotient_common.h | 7 +++++++ 3 files changed, 10 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/events/encryptionevent.h b/lib/events/encryptionevent.h index 56913393..124ced33 100644 --- a/lib/events/encryptionevent.h +++ b/lib/events/encryptionevent.h @@ -6,13 +6,14 @@ #include "eventcontent.h" #include "stateevent.h" +#include "quotient_common.h" namespace Quotient { class QUOTIENT_API EncryptionEventContent : public EventContent::Base { public: enum EncryptionType : size_t { MegolmV1AesSha2 = 0, Undefined }; - explicit(false) EncryptionEventContent(EncryptionType et); + QUO_IMPLICIT EncryptionEventContent(EncryptionType et); [[deprecated("This constructor will require explicit EncryptionType soon")]] // explicit EncryptionEventContent() : EncryptionEventContent(Undefined) diff --git a/lib/events/roommemberevent.h b/lib/events/roommemberevent.h index 3296ae22..ceb7826b 100644 --- a/lib/events/roommemberevent.h +++ b/lib/events/roommemberevent.h @@ -15,7 +15,7 @@ public: using MembershipType [[deprecated("Use Quotient::Membership instead")]] = Membership; - explicit(false) MemberEventContent(Membership ms) : membership(ms) {} + QUO_IMPLICIT MemberEventContent(Membership ms) : membership(ms) {} explicit MemberEventContent(const QJsonObject& json); Membership membership; diff --git a/lib/quotient_common.h b/lib/quotient_common.h index 02a9f0cd..b3fb3efa 100644 --- a/lib/quotient_common.h +++ b/lib/quotient_common.h @@ -25,6 +25,13 @@ Q_ENUM_NS_IMPL(Enum) \ Q_FLAG_NS(Flags) +// Apple Clang hasn't caught up with explicit(bool) yet +#if __cpp_conditional_explicit >= 201806L +#define QUO_IMPLICIT explicit(false) +#else +#define QUO_IMPLICIT +#endif + #define DECL_DEPRECATED_ENUMERATOR(Deprecated, Recommended) \ Deprecated Q_DECL_ENUMERATOR_DEPRECATED_X("Use " #Recommended) = Recommended -- cgit v1.2.3