Age | Commit message (Collapse) | Author |
|
On Windows QUOTIENT_API expands to different things depending on whether
the library is built or used. This results in confusing statements (and
MSVC erroring out on them, in some cases - see below - quite
legitimately) not only when the application includes Quotient headers
but also when the application defines custom events and uses
REGISTER_EVENT_TYPE to make them creatable from /sync responses.
To avoid repeated registration when dynamic linking is involved,
EventFactory<>::addMethod() now bluntly looks up the method for this
type in the vector of already registered methods. It would surely be
quicker to use a static variable instead; but since the refreshed API
for addMethod returns a reference to the factory method it's necessary
to do this lookup anyway. Once the primary goal of this branch is
achieved across platforms I might experiment with lighter ways to
register factory methods; for now here's a minimal change to make
the code build on Windows.
|
|
Thanks to Sonar for reminding that constexpr implies inline.
|
|
Once visibility kicks in, MSVC changes its ways and tries to instantiate
Private classes wrapped in smart pointers upon their occurence in the
header file - which leads to build breakage because of a missing
destructor. Usually making the outer class destructor out-of-line helps
to fix this (see RoomEvent, for one example).
|
|
MSVC is quite picky to redeclaration with __declspec(dllexport), judging
it as changing the class of storage. This commit tries to reorder
declarations so that MSVC is made aware of dllexport attribute on the
first encounter rather than the second one.
|
|
Before all, this fixes the problem with double-initialising of type ids;
it could have been fixed with a smaller change but EventTypeRegistry
is fairly superfluous now when inline variables are a thing and it's
possible to have an extensible registry system using literally pointers
to the memory that are guaranteed to be unique. That being said,
event_type_t is still QLatin1String and not a bare const char* (or
void*), mostly to stay on the safe side when it comes to type
identities: unlike const char*, QLatin1String's are deep-compared,
meaning that matching for switchOnType (former visit) occurs a bit
slower now. This may change in the future; but this is the first step
in getting rid of EventTypeRegistry.
This change means that initializeTypeId is no more needed; also, two
static member functions, typeId() and matrixTypeId(), are being replaced
with a single inline static member variable, TypeId. This commit doesn't
apply that transition across the event types, meaning that you'll get
a pile of warnings when compiling the library. These warnings will be
tackled in further commits within this branch.
|
|
Strictly speaking, EventFactory can be further instantiated if any
client application figures they need a whole new base class for events
and respectively a separate EventFactory specialisation for it.
Where this whole commit started though was a linkage error because I
did not plan to expose Quotient-specific logging categories for linkage
(effectively, usage) from the client code - meanwhile the inline code
of EventFactory uses qDebug(EVENTS), meaning I had to either add
QUOTIENT_API to EVENTS or hide those invocations. This in turn led
to trimming the EventFactory constructor back to trivial implementation
and dropping the guard variable that was supposed to trace duplicate
EventFactory<BaseEventT> objects for the same BaseEventT - with the
reasoning that such situation is not really dangerous (unlike
EventTypeRegistry double-initialisation fiasco, see #413), and at the
same time it can be easily detected in the logs by duplicated factory
method registration messages. And while I was at it, I replaced the
meaningless bool in the return type of EventFactory<>::addMethod with
the slightly more (but still barely) useful reference to the inserted
factory method. One can (in theory) use it now if they need to turn
some event JSON into an object of some specific event type or nullptr
if the event type in the JSON payload doesn't match - but at the same
rate (for now at least) one can call makeIfMatches<EventT>() directly.
With this commit, both Quotest and Quaternion build and link using
either Clang or GCC even under -fvisibility=hidden. However, running
quotest now reproduces #413, which is a matter of event typeId
infrastructure refactoring, coming in further commits.
|
|
|
|
This include all (hopefully) classes/structures and functions that have
non-inline definitions, as well as namespaces with Q_NAMESPACE since
those have non-inline (as of Qt 5.15) QMetaObject - for that a new
macro, QUO_NAMESPACE, has been devised to accommodate the lack of
Q_NAMESPACE_EXPORT in Qt before 5.14.
|
|
Instead of using CMake's generate_export_header macro, it's a bit easier
to maintain a static file (that is not supposed to ever change) with
necessary export/import/hidden visibility macros.
|
|
|
|
There's no need to return lambdas where pointers to specialised function
templates would work just fine.
|
|
|
|
|
|
|
|
|
|
The former code assumed that EventFactory<> is just a class-level shell
for a bunch of functions and a static data member that only exists to
allow specialisations to occur for the whole group together. On top of
that, setupFactory() and registerEventType() strived to protect this
group from double registration coming from static variables in an
anonymous namespace produced by REGISTER_EVENT_TYPE.
The whole thing is now de-static-ed: resolving the factory now relies
on class-static Event/RoomEvent/StateEventBase::factory variables
instead of factory_t type aliases; and REGISTER_EVENT_TYPE produces
non-static inline variables instead, obviating the need of
registerEventType/setupFactory kludge.
|
|
Objects derived from Event are not room events (in the spec sense) and
never occur in the same arrays as room events; therefore this chaining
has always been superfluous.
|
|
Also: extended quotest to cover member renames, not just user profile renames.
|
|
Also, RoomAliasesEvent is to be completely gone after 0.7.
|
|
|
|
Quotient::function_traits<> did not support member functions in a proper
way (i.e. the way std::invoke_result<> treats them, with the function's
owning class represented as the first parameter). Now that I gained
the skill and understanding in function_traits<> somewhat wicked
machinery, I could properly support member functions. Overloads and
generic lambdas are not supported but maybe we'll get to those one day.
|
|
It has not much to do with the Visitor design pattern; also,
std::visit() has different conventions on the order of parameters.
|
|
|
|
There was a lot of excess redirection in fromJson() and toJson() with
most of JsonConverter<> specialisations being unnecessary boilerplate.
These have been replaced by overloads for toJson() and explicit
specialisations for fromJson() wherever possible without breaking
the conversion logic.
|
|
Proper linters recognise that the returned types are not primitive,
while people might still be confused a bit.
|
|
In both fixed cases the callee accepts a const reference, which makes
std::move() useless. Static analyzers apparently missed them because
the cases are inside a macro.
|
|
It's might look weird; but without making fromJson() a specialisation
it becomes an overload next to an implicit specialisation of
the template function defined just above, and then loses to that
specialisation because it (also) has the perfect match.
(would be great if the compiler shaded the implicit specialisation in
such cases - alas it's not how the standard works.)
|
|
|
|
Similar to contentPart() - apparently there are enough places across
the code that would benefit from it.
|
|
Given that QJsonObject only accepts QStrings in the list constructor,
the template is useless cruft.
|
|
|
|
There's a clash between Event::content() (a template function) and
RoomMessageEvent::content() (plain member). Out of these two, the name
more fits to the RME's member function - strictly speaking,
Event::content() retrieves a part of content, and so is renamed.
In addition, contentPart() defaults to QJsonValue now, which is pretty
intuitive (the function returns values from a JSON object) and allows
to implement more elaborate logic such as
if (const auto v = contentPart<>("key"_ls); v.isObject()) {
// foo
} else if (v.isString()) {
// bar
} else {
// boo
}
|
|
|
|
This is useful for implementing Spaces support, where all events of
type `m.space.child` are needed, and we don't know their state keys in
advance.
|
|
The problem is in Room::processStateEvent(): after
potentially-inserting-nullptr into currentState, pre-check failure
(that may occur on member and trigger events for now) leaves that
nullptr in the hash map. Basically anything that uses currentState
(e.g., Room::toJson) assumes that currentState has no nullptrs - which
leads to either an assertion failure, or nullptr dereferencing. The fix
removes the nullptr placeholder if the pre-checks failed.
|
|
Trying to test bits with Changes::testFlag(Change::Any) was a bad idea.
Along the way: made logging in setLastReadReceipt() refer to the actual
timeline item when possible.
|
|
Doesn't really help build times, instead breaking the build on older Qt.
|
|
The "original JSON" wording is misleading: the returned JSON can be and
is routinely edited as a part of event construction, redaction, editing.
Also, originalJson() name is misleading in that it returns a stringified
(in a very specific way) JSON and not an object. You have to call
fullJson() to get the object, and originalJsonObject(), confusingly,
returns exactly the same thing but as a value rather than as
a reference. The original intention of keeping originalJsonObject() was
to make it Q_INVOKABLE or use it as an accessor for a Q_PROPERTY.
unfortunately, this was never really practical as discussed in
the previous commit.
All that implies that clients have to handle passing event JSON to QML
themselves, in the form they prefer (as an object or a string). The
added complexity is negligible though; on the other hand, there's added
flexibility in, e.g., choosing a compact instead of default JSON layout
or even generate a highlighted JSON representation.
|
|
Makes compilation a tad lighter.
|
|
Q_GADGET is generally used to enable two things outside of QObject:
Q_PROPERTY/Q_INVOKABLE and Q_ENUM/Q_FLAG. While the latter can be used
in its own right in QML, the former requires Q_GADGET instances to be
passed to QML by value, which is not really possible with
uncopyable/unassignable classes. Bottom line is that Q_PROPERTY in
anything derived from Quotient::Event is not viable, making Q_GADGET
macro useless unless there's a Q_ENUM/Q_FLAG (as is the case with
RoomMessageEvent, e.g.).
|
|
|
|
|
|
|
|
|
|
|
|
This introduces a new API to count unread events that would allow to
obtain those unread and highlight counts since either the fully read
marker (Room::partiallyReadStats) or the last read receipt
(Room::unreadStats). Element uses the read receipt as the anchor
to count unread numbers, while Quaternion historically used the fully
read marker for that (with the pre-0.7 library sticking the two markers
to each other). From now on the meaning of "unread" in Quotient is
aligned with that of the spec and Element, and "partially read" means
events between the fully read marker and the local read receipt;
the design allows client authors to use either or both counting
strategies as they see fit. Respectively, Room::P::setFullyReadMarker()
updates partially-read statistics, while Room::P::setLastReadReceipt(),
when called on a local user, updates unread statistics.
Room::notificationCount() and Room::highlightCount() maintain their
previous meaning as the counters since the last read receipt;
Room::notificationCount() counts unread events locally, falling back
to the value from the above-mentioned key defined by MSC2654, and if
that is not there, further to `unread_notifications/notification_count`
defined in the current spec. Room::highlightCount(), however, is still
taken from the homeserver, not from Room::unreadStats().highlightCount.
|
|
This makes updating display name and emission of necessary signals
including Room::changed() more systematic when it has to occur outside
of updateData() flow - e.g. when loading all members.
|
|
Since MSC2654's unread count is counted from the m.read receipt, and
the course is to follow the spec's terminology and use "unread count"
for the number of notable events since m.read, this required to move
the existing number of notable events since m.fully_read to another
field, henceforth called partiallyReadCount. At the same time,
SyncData::notificationCount is dropped completely since MSC2654 claims
to supersede it.
Also: Room::resetNotificationCount() and Room::resetHighlightCount() are
deprecated, as these never worked properly overwriting values that can
be calculated or sourced from the server, only for these values to be
set back again the next time the room is updated from /sync.
|
|
Room::isEventNotable has been moved out from Room::Private and made
compliant with MSC2654.
The concept of Room::checkForNotifications is taken from Quaternion
where a method with the same name has been in QuaternionRoom for a long
time - however, actual body is a stub for now, always returning
{ Notification::None } (Quaternion's implementation is too crude to be
taken to the library). Now we really need a pushrules processor to fill
this method with something reasonably good. Internally the library now
calls checkForNotifications() on every event added to the timeline,
filling up the events-to-notifications map because it is anticipated
that calculation of notifications can be rather resource-intensive and
should only be done once for a given event.
Finally, Room::notificationsFor is an accessor into the mentioned map,
standing next to isEventNotable (but unlike isEventNotable, it's not
virtual; checkForNotifications is).
|
|
[skip ci]
|