| Age | Commit message (Collapse) | Author |
|---|
|
The template version has never worked, to the point where instantiating
it would immediately lead to FTBFS. The new version returns an event
pointer as a simpler fix that would make it usable - in particular,
there's no more need to have separate
Connection::Private::unpackAccountData(). To simplify the fix,
eventCast() has been made more tolerating - passing nullptr to it is
processed in an expected (no-op) way now.
|
|
[skip ci]
|
|
[skip ci]
|
|
|
|
|
|
This is aligned with the non-moving version.
|
|
In a situation where you have an EventPtr that you want to place
somewhere as an `event_ptr_tt<SomeMoreSpecificEventType>` you have to
carefully check that the stored event is actually of
SomeMoreSpecificType and if it is, release() that event pointer,
downcast, and re-wrap it into that new event_ptr_tt - or, as can be seen
from the diff here, re-loadEvent() from JSON, which is simpler but
inefficient. To help clients, and the library, eventCast() can now
accept an rvalue smart pointer and do all the necessary things with it.
|
|
|
|
|
|
The last commit broke it.
|
|
The upcoming event type infrastructure finally helps to detect those
omissions more or less reliably (for event types only though).
|
|
...instead of just the number of them.
|
|
Event type ids don't need a C++ type to be used, and clients might
define those types on their side (NeoChat does that, e.g.).
|
|
Profiling revealed 3 inefficiencies in read receipts code - and given
there are a lot of them coming, these inefficiences quickly add up.
Fixing them allows to slash read receipt processing time by 60%, and
the total time of updating a room by more than a half.
1. Room::lastReadEventChanged() is emitted per receipt. This can be
taxing on initial syncs or in bigger rooms; this commit converts it
to an aggregate signal only emitted once per sync room batch and
carrying the list of all user ids (more on that below) with updated
read receipts.
For that, Room::P::setLastReadEvent() is split into
Room::P::setLocalLastReadEvent() that is called whenever the local
read receipt has to be updated, and setLastReadEvent() proper that is
very fast and only updates the internal data structures, nothing else.
setLocalLastEvent() calls it, as does processEphemeralEvents(); both
take responsibility to emit lastReadEventChanged() depending on the
outcome of setLastReadEvent() invocation(s).
2. Massively aggravating the above point, user id from each read receipt
is turned to a User object - and since most of the users are unknown
at early moments, this causes thousands of allocations. Therefore
the new aggregated lastReadEventChanged() only carries user ids, and
clients will have to resolve them to User objects if they need.
3. Despite fairly tight conditions (note we're talking about thousands
of receipts), Quotient still creates an intermediate C++ structure
(EventsWithReceipts), only for the sake of passing it to
processEphemeralEvent() that immediately disassembles it back again,
converting to a series of calls to set(Local)LastReadEvent(). To fix
this, processEphemeralEvent() now takes the event content JSON
directly and iterates over it instead.
Aside from that, a few extraneous conditions and logging has been
removed and the whole function rewritten with switchOnType() to reduce
cognitive complexity.
|
|
The result of factoring out duplicate code.
|
|
|
|
If userMap only holds valid ids, there's no reason to spend time
validating the sought id: if it's invalid, it won't be found. And
lookups over a hash map are cheap.
|
|
That switch between micro- and milliseconds was pure visual sugaring,
in a potentially time-sensitive context.
Also: there's no sense in using const-ref for a small parameter in
a function that is, to top it off, almost always inlined.
|
|
I was about to decommission it but got to use it myself.
[skip ci]
|
|
|
|
|
|
This introduces enumTo/FromJsonString() and flagTo/FromJsonString(),
four facility functions to simplify conversion between C++ enums and
JSON, and refactors a couple of places where it's useful.
|
|
JSON conversions are moved out of the class, obviating the need to
define the plain data constructor and gaining default-constructibility
along the way - previously the default constructor was preempted
by user-defined ones.
|
|
EncryptionEvent was marked as Q_GADGET only for the sake of defining
EncryptionType inside of it as Q_ENUM, with aliases also available under
Quotient:: and EncryptionEventContent. This is a legacy from
pre-Q_ENUM_NS times. However, event types are not really made to be
proper Q_GADGETs: Q_GADGET implies access by value or reference
but event types are uncopyable for the former and QML is ill-equipped
for the latter.
This commit moves EncryptionType definition to where other such
enumerations reside - on the namespace level in quotient_common.h; and
the other two places are now deprecated; and EncryptionEvent is no more
Q_GADGET.
With fromJson/toJson refactored in the previous commit there's no more
need to specialise JsonConverter<>: specialising fromJson() is just
enough.
Moving EncryptionType to quotient_common.h exposed the clash
of two Undefined enumerators (in RoomType and EncryptionType),
warranting both enumerations to become scoped (which they ought to be,
anyway). And while we're at it, the base type of enumerations is
specified explicitly, as MSVC apparently uses a signed base type (int?)
by default, unlike other compilers, and the upcoming enum converters
will assume an unsigned base type.
Finally, using fillFromJson() instead of fromJson() in
the EncryptionEventContent constructor allowed to make default values
explicit in the header file, rather than buried in the initialisation
code.
|
|
fromJson() is generalised to accept any JSON-like type while passing
QJsonObject to JsonConverter<>::load (instead of doLoad). This allows to
(still) rely on JsonConverter<> as a customisation point while providing
an opportunity to overload fromJson for custom types in a pointed way
(specifically, by providing the overload for
`fromJson(const QJsonObject&)`), instead of having to go with full-blown
JsonConverter<> specialisation. This will be used in a further commit
to simplify ReceiptEvent definition.
Using if constexpr in combination with constraints (`requires()`) -
the first such case in Quotient codebase - allowed to put the entire
logic in a single JsonConverter<>::load() body instead of having a
facility JsonExporter<> class for SFINAE.
Aside from that, fromJson<QJsonValue, QJsonValue> is entirely dropped
because it's not supposed to be used that way (it's no-op after all);
reflecting that, Event::unsignedPart() and Event::contentPart() no more
default to QJsonValue as the expected return type, you have to
explicitly provide the type instead (and as one can see from the changes
in the commit, it's actually better that way since it's better
to validate the value inside JSON - e.g. check QString or QJsonObject
for emptiness - than the QJsonValue envelope which may still wrap
an empty value).
toJson() is also generalised, replacing 3 functions with one that has
a constexpr if to discern between two kinds of types.
|
|
[skip ci]
|
|
Dropping yet another translation unit.
|
|
|
|
|
|
Fixing link errors at non-template RoomStateView::get() when building
with libQuotient as a shared object. There's also a test in quotest.cpp
now to cover that case.
|
|
|
|
Aside from breaking that line, the previous line - with connect*() -
is often broken up too, making smaller lambdas consume much more
vertical space.
|
|
[skip ci]
|
|
|
|
:latest stopped working for some reason.
|
|
GCC 10 ICE's[1] in qt_connection_util.h code; and ubuntu-20.04 doesn't
have GCC 11.
Also: patch a Qt 5.15 header when compiling with GCC because a
combination of Qt 5.15 and GCC 11 in turn triggers QTBUG-91909/90568...
Which in turn required moving Qt setup before the build environment
setup. Life's fun.
[1] Internal Compiler Error
|
|
Apple Clang doesn't have those yet.
|
|
...thanks to C++20 awesomeness. A notable change is that
wrap_in_function() (and respectively function_traits<>::function_type)
and fn_return_t alias are gone. The former are no more needed because
connectUntil/connectSingleShot no more use std::function. The latter
has been relatively underused and with the optimisation of switchOnType
hereby, could be completely replaced with std::invoke_result_t.
Rewriting connect* functions using constexpr and auto parameters made
the implementation 30% more compact and much easier to understand
(though still with a couple of - now thoroughly commented - tricky
places). Dropping std::function<> from it may also bring some (quite
modest, likely) performance benefits.
|
|
|
|
Too many parameters of the same type in a row.
|
|
|
|
|
|
Since this object has to be verified against a signature it also carries
there's a rather specific procedure described in The Spec for that.
That procedure basically assumes handling the signed one-time key
object as a JSON object, not as a C++ object. And originally Quotient
E2EE code was exactly like that (obtaining the right QJsonObject from
the job result and handling it as specced) but then one enthusiastic
developer (me) decided it's better to use a proper C++ structure -
breaking the verification logic along the way. After a couple attempts
to fix it, here we are again: SignedOneTimeKey is a proper QJsonObject,
and even provides a method returning its JSON in the form prepared for
verification (according to the spec).
|
|
|
|
Now there's only 1 instead of 5 lookups of the same EncryptionEvent,
and the code is shorter.
|
|
In keyverificationevent.*, this massively shortens repetitive getter
definitions; the remaining few non-trivial ones are moved to
keyverificationevent.h, dropping the respective .cpp file and therefore
the dedicated translation unit. In roomkeyevent.h, it's just shorter.
|
|
|
|
toJson(SignedOneTimeKey) incorrectly generated a "signatures" key
mapped to an empty object when no signatures were in the C++ value.
Also: fallback keys have an additional flag that also has to be taken
into account when verifying signatures.
|
|
'performance-no-automatic-move' triggers on code where copy elision
normally takes place anyway. In fact, all cases it triggered on were
also subject to named return value optimisation (NRVO).
[skip ci]
|
|
Honestly, it was quite intuitive even without that, but in reality
there are implicit conversion under the wraps. This commit makes them
explicit, for clarity.
|
