This commit contains a new `assert_next_matches_with_timeout!` macro that will take a `Stream` and wait for a little while until its next item is ready, then match it to a pattern.
It was found that it's making the whole build slower because it's
hitting a pathologically slow path in type-checking. Considering that it
doesn't do much, let's get rid of it and inline it instead.
After this, compiles times are reduced from 30 seconds to 22 seconds on
my machine
This patch uses the new `RoomSubscriptionState` enum to filter
room subscriptions that have already been sent, when building a
`http::Request` with the sticky parameters.
By default, to start, a `http::request::RoomSubscription` is in the
state `Pending`, i.e. it's not sent yet. Once the sticky parameters are
committed, the state is updated to `Applied`. When the sticky parameters
are applied, only the `Pending` room subscriptions are added.
This patch contains one test to specifically assert this behaviour.
This patch adds the `commit` method on the `StickyData` trait. It is
called by `SlidingSyncStickyManager::maybe_commit` when we are sure the
data can be validated because of a valid response to the sent request.
## Changes
- Creates a separate `AllEventsCache` struct holding both the actual all
events cache map and the relationship one. This new cache wrapper also
holds a single `RwLock` to both inner caches, as they are independent
from each other.
- When a new event is saved either from `/sync` or another HS request,
it'll be saved to both the 'all events' cache and the relationship map.
- Add tests for the relationship cache.
Allows to save media in a different path than the state store.
This adds a "last_access" field to the SQLite implementation, to prepare
for future work on a media retention policy.
This removes the IndexedDB media cache implementation, because as far as
I know it is currently unused, and I have no idea how to implement
efficiently the planned media retention policy with a key-value store.
Closes#1810.
- [x] Public API changes documented in changelogs (optional)
---------
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
This patch clears all sliding sync room subscriptions when a session
expires. Indeed, we might not want to request all room subscriptions
when the session restarts. Imagine if the client has subscribed to 400
rooms and the session expires: once the session restarts, it will ask
for 400 room subscriptions, which is a lot and will result in a quite
slow response.
We're finding ourselves in the situation in which we can't interact with
invites through normal Room APIs as `full_room`s can't be build from the
RoomListItem. Full rooms require the timeline to be configured before
use and the timeline can't be configured because encryption cannot be
fetched for invited rooms on homeservers that have previews disabled
(see #3848 and #3850)
In response we now expose the room's membership directly from the
`RoomListItem` so that the final client can chose which of the 2 rooms
types (invited or full) to ask for before using aforementioned APIs.
Powers https://github.com/element-hq/element-x-ios/pull/3189
This patch updates when sliding sync requests have a `timeout`.
Prior to this patch, all requests had a `timeout` query, set to the
`poll_timeout` duration value. However it means: if there is no data
to return, wait `timeout` milliseconds for new data before returning.
This definition is correct. Problem: if the current range of a list
has no data, the server will wait! It means that, in a situation where
there is no update at all, but the client is fetching all rooms batch by
batch, it will wait `poll_timeout` for each batch!
The behaviour described above is absolutely correct. Some server
implementations are less strict though, and we didn't realise our code
was doing that, because the server had some optimisations to ignore the
timeout if the range wasn't covering all the rooms. Nonetheless, a new
server implementation (namely Synapse) is strict, and it confirms we
have a bug here.
This patch then configures a `timeout` if all lists require a timeout,
otherwise there is no `timeout`, which is equivalent to `timeout=0`.
- this prevents issues where spoofing the sender field is enough to spoof and edit and display wrong decorations in the app
- fixes matrix-org/internal-config/issues/1549
Most times we pulled an InboundGroupSession from the sqlite DB, we were
overriding whatever value for `backed_up` was stored inside the pickled
value, and using the value stored in the SQL column.
But when we pulled a single InboundGroupSession from the DB by ID, we
did not override it.
I am fairly sure this was an accidental oversight, so this change
corrects it, and unifies the code with other places we create these
objects.
It seems that the fact that matrix-rust-sdk contains `olm-rs` in its
`Cargo.lock` sparked panic, so let's attempt to fend off future concerns by
adding a comment.
This patch fixes a bug in the tracing system. It introduces one
fields formatter _per layer_ to force the fields to be recorded in
different span extensions, and thus to remove the duplicated fields in
`FormattedFields`.
The patch contains links to the bug report in `tokio-rs/tracing`. This
patch is a workaround.
This patch removes `NotificationSettings::subscribe_to_changes` because
it's not used anywhere in our code except in tests. It is indeed part of
the public API but I'm not aware of anyone using it for the moment. It
only adds complexity in the code.
This patch replaces `RoomInfo::user_defined_notification_mode` by
its cached variant: `cached_user_defined_notification_mode`, and
call `Room::cached_user_defined_notification_mode` which will boost
performance when computing a new `RoomInfo`.
This patch splits/copies the
`test_room_info_deserialization_without_optional_items` test into the
same test + `test_room_info_deserialization`.
It appears that the _without optional items_ part has been forgotten.
In the past, the test has been updated to test optional items. The
initial idea (based on my understanding of the comments) is to test
potentially old `RoomInfo` can still be deserialized today. So this test
must never be changed, except if a non-optional field is added.
For this reason, this patch removes from this test the assertions about
optional fields. A new test, named `test_room_info_deserialization` is
created, and tests all the fields, including the optional ones.
One important thing:
`test_room_info_deserialization_without_optional_items` now runs
even if the `experimental-sliding-sync` feature is absent. It was
required only because of `latest_event`, but that's an optional
field! However, the `test_room_info_deserialization` requires the
`experimental-sliding-sync` feature, as it tests `latest_event` but
also `recency_stamp`.
Finally, `test_room_info_deserialization` tests
`cached_user_defined_notification_mode`.
This patch updates `matrix_sdk::Room::user_defined_notification_mode`
to cache its result if some mode has been found. The cached result can
be retrieve with
`matrix_sdk_base::Room::cached_user_defined_notification_mode`.
Use a for loop rather than `partition_map`. We're about to add a third list, so
partition_map won't work.
(partition_map ends up using Vec::push under the hood, so this is pretty much
equivalent.)
Update Ruma dependency to expect call membership state events with state
keys that are arbitrary strings, not just pure MXIDs.
When a call membership state key does not exactly match the format of an
MXID, treat it as a valid state key if it starts with an MXID followed
by an underscore, with that MXID designating the owner of the event.
(The state key may also be optionally prefixed with an underscore, which
is permitted as a way to bypass pre-MSC3757 authorization rules against
sending state events with state keys that do not exactly match the
sender's MXID.)
---------
Signed-off-by: Andrew Ferrazzutti <andrewf@element.io>
Co-authored-by: Benjamin Bouvier <public@benj.me>
Co-authored-by: Benjamin Bouvier <benjamin@bouvier.cc>
The timeline already listens to changes to the pinned events list (via a
stream), so there's no need to fully reload all the pinned events every
time we receive a new event that's pinned. Technically it may avoid one
or a few lookups, but this is cheap and a subsequent commit/PR will
merge the pinned event cache into the event cache.
This patch restricts the call to `compute_limited` to the sliding sync
proxy implementation (aka MCS3575). It is not necessary for the sliding
sync native implementation (aka Simplified MSC3575). The proxy doesn't
implement the `limited` flag, contrary to Synapse. Let's not run
workarounds when we don't need them.
The patch https://github.com/matrix-org/matrix-rust-sdk/pull/2395 has
introduced `SlidingSyncInner::past_positions` as a mechanism to filter
duplicated responses. It was a problem because the sliding sync `ops`
could easily create corrupted states if they were applied more than
once.
Since https://github.com/matrix-org/matrix-rust-sdk/pull/3664/, `ops`
are ignored.
Now, `past_positions` create a problem with the sliding sync native
implementation inside Synapse because `pos` can stay the same between
multiple responses.
While `past_positions` was helpful to fix bugs in the past, it's no
longer necessary today. Moreover, it breaks an invariant about `pos`: we
must consider it as a blackbox. It means we must ignore if a `pos` value
has been received in the past or not. This invariant has been broken for
good reasons, but it now creates new issues.
This patch removes `past_positions`, along with the associated code
(like `Error::ResponseAlreadyReceived` for example).
This patch changes the visibility of
`SlidingSyncList::invalidate_sticky_data` from `pub` to `pub(super)`.
This is the only place where it must be accessible from.
This patch asserts that when subscribing to a new room, the old room
subscriptions are still present. Is it the behaviour we want? Probably
not, but this is the standard behaviour right now, and we need to assert
it.
We had *two* copies of `response_from_file`, and all calls to them were always
immediately followed by an operation to parse the response as a Ruma response
object.
We can save a whole lot of boilerplate with a generic function that wraps the
json into an HTTP response *and* parses it into a Ruma object.
We weren't updating the database schema version immediately after the v10 -> v11
migration. This was fine in practice, because (a) for now, there is no v12
migration so we ended up setting the schema version immediately anyway; (b) the
migration is idempotent.
However, it's inconsistent with the other migrations and confusing, and is
about to make my test fail, so let's clean it up.
It's better to have fewer public APIs, especially when there's little
annoyance to have it. We could use a request builder that converts into
a Future, too, but considering there's only a single optional parameter,
it's fine to include it in the function's signature.
We should only show the spinner if the *first* sliding sync request is
taking a while. If we have received some data and the second request
takes a while, that is OK.
For the state transition of `Init -> SettingUp` this is handled
correctly, however for `Terminated -> Recovering -> Running` we waited
until the second request returned before hiding the sync spinner. This
meant that if the first request returned quickly the app would show new
data and *then* the sync spinner would show (if the second request took
time).
This situation occurs frequently with the new SSS API, where if all the
new data was returned in the first sync then the second sync would
block waiting for new data, triggering the sync spinner.
This patch changes the `SlidingSync::subscribe_to_room` method to
`subscribe_to_rooms`. Note the plural form. It's now mandatory to
subscribe to a set of rooms. The idea is to avoid calling this method
repeatedly. Why? Because each time the method is called, it sends a
`SlidingSyncInternalMessage` of kind `SyncLoopSkipOverCurrentIteration`,
i.e. it cancels the in-flight sliding sync request, to start over with
a new one (with the new room subscription). A problem arises when the
async runtime (here, Tokio) is busy: in this case, the internal message
channel can be filled pretty easily because its size is 8. Messages
are not consumed as fast as they are inserted. By changing this API:
subscribing to multiple rooms will result in a single internal message,
instead of one per room.
Consequently, the rest of the patch moves the `subscribe` method of
`room_list_service::Room` to `room_list_service::RoomListService`
because it now concerns multiple rooms instead of a single one.
Our CI used to be quite slow and used up a lot of CI time, so as an
optimization we disabled CI for draft PRs.
This is a bit annoying since people want to open draft PRs to check if
CI passes, but without triggering any review requests.
Since our CI is nowadays a bit more efficient let's see if we can enable
it for draft PRs.
This method works the same as `Room::event` but you can provide a custom `RequestConfig` to it.
It's especially useful for the pinned events timeline, since we need a max number of retries and a max number of concurrent requests. With this we can remove some unnecessary complexity.
This way it matches the rest of timelines in the SDK, I reversed it here because I didn't realise most clients just do this reversal of ordering themselves. As they do, they need the same order for this timeline too to be able to reuse their existing logic.
This ensures the cache keeps the events even when the associated `Room` is dropped, which is what we want when using it to cache the pinned events for rooms in the client.
Add `fn Client::pinned_event_cached()` to get a reference to it.
In `matrix.rs` we add methods to interact with the matrix homeserver. And in `machine/mod.rs` we implement the widgetMachine cases for handling (checking capabilities and using the matrixDriver) delayed events.
Add `PinnedEventsLoader` to encapsulate this logic, being able to provide a max number of events to load and how many can be loaded at the same time. Also implement an event cache for it.
Add `PinnedEventsRoom` trait to use it in the same way as `PaginableRoom`, only for pinned events.
The code used to only add new targets to rooms
but never remove the ones that are not in the event anymore.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
This patch ensures that we retain the master key for a given UserIdentityData object, even when a new and different identity arrives via the `/keys/query` endpoint. This concept, called pinning, is similar to certificate pinning in web browsers.
Retaining the master key allows us to detect changes and notify the user accordingly.
This tiny change allows one to easily name the return type of
`EventTimelineItem::reactions()` such that you can use it in a function
signature. Same goes for the `ReactionInfo` type.
I think this was likely just a small oversight in recent changes to the
reactions API, so hopefully it's not controversial.
Without this, it's impossible to write a function that uses
`ReactionsByKeyBySender` or `ReactionInfo` in the signature.
Signed-off-by: Kevin Boos <kevinaboos@gmail.com>
- disable backups and recovery before requesting the reset handle
- attempt device key upload
- re-enable backups both when UIAA is required and when not
This means we have all the information inside SenderData to populate
VerificationStatus and DeviceId for EncryptionInfo, so we can share the
code between SenderDataFinder and get_verification_state.
This makes it impossible to represent states like "there's a local *and*
a remote echo for the same sender for a given reaction", or multiple
reactions from the same sender to the same event, and so on.
Remove `Result` where it is not needed, and switch to `CryptoStoreError`
instead of `OlmError` where possible. Soon, this will allow us to call
some of these methods from places that don't know about `OlmError`.
These calls are equivalent because the old code called
`self.get_user_devices` with a `timeout` of `None`, which meant the call
to `wait_if_user_pending` inside was a no-op.
As it says on the tin. Needed the functions but they were missing. They are analogous to the GlobalAccountData setters.
Signed-off-by: Benjamin Kampmann <ben@acter.global>
The sliding sync proxy has a bug: despite the presence of
`m.room.create` in `bump_event_types`, an invite with an `m.room.create`
event will not have a `timestamp`. Thus, such a room cannot be sorted
reliably.
This patch fixes this problem with a terrible hack, where it tries to
find an `origin_server_ts` value from within the `invite_state` state
events.
Please read the comment to learn more.
The `UserIdentity::is_verified()` method in the matrix-sdk-crypto crate
before version 0.7.2 doesn't take into account the verification status
of the user's own identity while performing the check and may as a result
return a value contrary to what is implied by its name and documentation.
This patch fixes this and adds a regression test.
The method itself is not used internally and as such has not a larger
impact.
Co-authored-by: Denis Kasak <dkasak@termina.org.uk>
Signed-off-by: Damir Jelić <poljar@termina.org.uk>
The `RoomList` provides a `Stream<Item = Vec<VectorDiff<Room>>>`.
This `Stream` receives updates from 2 sources: `RoomList::entries`,
and `Receiver<RoomInfoNotableUpdate>`. When a `RoomInfo` is
updated, a notable update is emitted and broadcasted. The
`RoomList` was filtering these notable updates by _reasons_ (namely
`RoomInfoNotableUpdateReasons`).
This is great and it's a good idea since we can filter which
`RoomInfoNotableUpdate` will trigger a `RoomList` update. However, too
many _reasons_ were hidden/implicit, and it creates several regressions
because (i) these _reasons_ were implicit, (ii) since the business rules
are not defined, there is no tests for that (not in this SDK, not in
apps like ElementX). It means we discover missing _reasons_ bug after
bug. It's not pleasant.
The reality is: we are in the middle of big changes, mostly with room
list client-side sorting and simplified sliding sync. We want to relax
a little bit. This patch then disable the feature _filter updates by
reasons_. The `RoomList` will update to all `RoomInfoNotableUpdate` for
the moment. We will get back to this optimisation later.
The `UserIdentity::is_verified()` method in the matrix-sdk-crypto crate
before version 0.7.2 doesn't take into account the verification status
of the user's own identity while performing the check and may as a result
return a value contrary to what is implied by its name and documentation.
This patch fixes this and adds a regression test.
The method itself is not used internally and as such has not a larger
impact.
Co-authored-by: Denis Kasak <dkasak@termina.org.uk>
Signed-off-by: Damir Jelić <poljar@termina.org.uk>
Simplified sliding sync no longer has `sort` or `bump_event_types`
because they are static values on the implementation/server side. This
patch removes them here.
This patch renames “recency timestamp” to “recency stamp”. It prepares
the fact that simplified sliding sync has a `bump_stamp` instead of a
`timestamp`. The notion of _timestamp_ must be removed.
This patch extracts most of `SlidingSync::sync_once` into a
method named `SlidingSync::send_sync_request`. The name mimics
the `SlidingSync::generate_sync_request` similar method: first we
_generate_, then we _send_.
The `SlidingSync::send_sync_request` is generic over the `Request` and
`::sync_once` passes the correct type depending of whether Simplified
MSC3575 is enabled.
This patch is the first over two patches to change the support
of Simplified MSC3575 from a compiler feature flag to a runtime
configuration. This configuration is held by the `Client`.
By default, it's turned off inside `matrix-sdk-ffi` and
`matrix-sdk-integration-testing`, otherwise it's turned on.
Currently on Element X if you receive a sticker in a room, the room list
will show the room as updated but it will show the latest event that is
not a sticker. This change fixes that.
Signed-off-by:
Marco Antonio Alvarez <surakin@gmail.com>
---------
Signed-off-by: Marco Antonio Alvarez <surakin@gmail.com>
Signed-off-by: Benjamin Bouvier <public@benj.me>
Co-authored-by: Benjamin Bouvier <public@benj.me>
New feature from Matrix 1.11.
- [x] Public API changes documented in changelogs (optional)
---------
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
There were some leftovers from the rename of the ReadOnlyDevice and
identity structs. The store still referenced them.
This gets rid of all the mentions and improves the documentation of the
store methods for devices and identities.
This reduces the work we do to calculate changed devices etc. when DEBUG
logging is not enabled, but more importantly (to me) it makes clear
that this code is only used for logging.
It is possible to remove the m.call capability because we now have
merged and released a version that does not request it anymore on
call.element.io
---------
Signed-off-by: Benjamin Bouvier <public@benj.me>
Co-authored-by: Benjamin Bouvier <public@benj.me>
This patch rewrites the `test_delayed_decryption_latest_event` test a
little bit. It does exactly the same things, but in a simpler way: it
removes multiple `sleep` and remove 2 sliding sync loops.
First off, the `SyncService` already starts the `RoomListService and the
`EncryptionSync` service. Both of them have their own sliding sync
loop. The test doesn't need other sliding sync loops in their own tasks,
this is not necessary at all: it's just pretty confusing and doesn't
reflect the reality, i.e. how these API are supposed to be used.
Second, it also tests the room for Bob is seen as encrypted.
Third, the `VectorDiff::Reset` is tested before the event from Bob
is sent. It's not only for clarity: it makes the test more robust for
future modifications.
Fourth, instead of waiting with a `sleep` for the event from Bob to be
received by Alice, we instead wait on the room list's stream of Alice to
receive an update. It's more robust this way and reflects the real usage
of this API. It also helps to remove an intermediate `assert_pending!`
that is no longer necessary because we are waiting on the stream just
after.
Finally, just like for the previous modification, this patch removes
another `sleep` for the to-device event from Bob to be received by
Alice, and instead wait on the room list's stream to receive an update.
It's again more robust and reflects the real usage of this API. Plus, it
makes the last `assert_pending!` macro to not be flaky.
This patch fixes a bug where rooms stored in the sliding sync cache
aren't restored by the `SlidingSyncBuilder`.
This patch also removes `SlidingSyncBuilder::rooms` fields which was
used but never modified. It's dead code.
Again, a transaction id received from the remote flow doesn't mean it
corresponds to a local echo sent *this particular session*, so no need
to warn about it.
This log can happen when an event is received, has a transaction id (in
the data received from sync), and doesn't have a corresponding timeline
item (be it local or remote). There's no reason to warn about this,
because this would happen in most cases, for new incoming events coming
from sync, and this pollutes the logs of rageshakes.
This patch removes the `test_room_info_notable_update_deduplication`
test. First off, it's flaky because sometimes Synapse lags, or sends
another events, which makes the test to fail. Second, the same feature
is tested inside the `matrix_sdk_ui::room_list_service` test suite,
with `test_room_sorting` and `test_room_latest_event`, and inside the
`matrix_sdk_base::sliding_sync` test suite, with a better granularity.
And lastly, this test doesn't test what it says: there is no room info
notable update deduplication whatsoever. I personally don't believe it
has ever existed. This test isn't necessary.
ReadOnlyDevice is not particularly useful as a description of why we
have two device types. This commit renames it into DeviceData, as this
struct is used to hold the device keys and additional local device data.
I'm not quite sure why it took me so long to come up with a better name.
Please forgive me past readers.
The test database was created using a slightly modified `oidc-cli`
example, to turn of the database encryption, on commit
d6dca91df86413b0cbf193a4be191835dd81862e
Since it's only used for remote events, and a local echo couldn't figure
that out anyways (since it's not sent yet, and that information makes
sense after sending).
This is needed to prevent the race condition where the invite request finished, the `/sync` one didn't fetch the new membership event yet and we send a message in the room. This message won't be encrypted for the newly invited user and will result in an UTD.
I added a new integration test and I can confirm this [complement-crypto test](https://github.com/matrix-org/complement-crypto/pull/98) now passes instead of being skipped.
Fixes#3622.
---
* fix(sdk): force room member reload after inviting a user
This is needed to prevent the race condition where the invite request finished, the `/sync` one didn't fetch the new membership event yet and we send a message in the room. This message won't be encrypted for the newly invited user and will result in an UTD.
* Use `room.mark_members_missing()` instead, add integration test
* Abort syncing before the test ends
* Resolve nit: else after a return
* Fix race condition where bob may try to join the room before the invite is received
* Remove double sync
This patch removes everything related to the computation of `ops`
from a sliding sync response. With the recent `RoomList`'s client-side
sorting project, we no longer need to handle these `ops`. Moreover, the
simplified sliding sync specification that is coming removes the `ops`.
A `SlidingSyncList` was containing a `rooms` field. It's removed by
this patch. Consequently, all the `SlidingSyncList::room_list` and
`::room_list_stream` methods are also removed.
A `FrozenSlidingSyncList` was containing the `FrozenSlidingSyncRoom`.
This patch moves the `FrozenSlidingSyncRoom`s inside
`FrozenSlidingSync`. Why is it still correct? We only want to keep the
`SlidingSyncRoom::timeline_queue` in the cache for the moment (until
the `EventCache` has a persistent storage). Since a `SlidingSyncList`
no longer holds any information about the rooms, and since `SlidingSync`
itself has all the `SlidingSyncRoom`, this move is natural and still
valid.
Bye bye all this code :'-).
The only thing is that our client builder allows setting only the server
versions, so this means the new field has to contain two `Option`al
fields. This causes a bit of churn, but isn't too bad in the end.
I've been debugging a cause of flakey complement-crypto tests for
about a month now. I was pretty convinced it was deadlocking
somewhere in the memory store `save_changes` code. With additional
logging, it's now clear that the there is an ABBA style deadlock
when `save_changes` is called at the same time as `get_state_events`.
I've also adjusted code for `get_user_ids` as it has a very similar
pattern and also acquires locks in reverse order to `save_changes`,
so is potentially vulnerable to this.
This patch avoids to emit a
`RoomInfoNotableUpdateReasons::RECENCY_TIMESTAMP` for rooms that are new.
Otherwise the entries in `matrix_sdk_ui::room_list_service::RoomList`
receive a `VectorDiff` because a new room is inserted, and then a
`VectorDiff` because the recency timestamp is updated. The second
`VectorDiff` is useless in this case.
First off, this patch removes the
`RoomInfoNotableUpdate::trigger_room_list_update` field. It is replaced
by a `reasons: RoomInfoNotableUpdateReasons` 8-bit unsigned integer. It
addresses the following issues:
1. When a subscriber receives a `RoomInfoNotableUpdate`, they have no
idea what has triggered this update.
2. In
`matrix_sdk_base::sliding_sync::BaseClient::process_sliding_sync_e2ee`,
we were triggering an update even if the latest event wasn't
modified: it is a false-positive, it was a bug and a waste of
resources. Now it's more refined, see the why below.
Second, this patch removes the second `trigger_room_list_update`
argument of `matrix_sdk_base::BaseClient::apply_changes`. This method
now knows where to find the reasons for the room info notable updates,
see next point.
Third, this patch adds a new
`matrix_sdk::StateChanges::room_info_notable_updates` field which is a
B-tree map between an `OwnedRoomId` and a
`RoomInfoNotableUpdateReasons`. The idea is that all places that receive
a `StateChanges` can also create a room info notable update with a
specific reason. This is a finer grained mechanism, and it avoids to
pass new arguments everywhere. It's cleaner.
Finally, it's easier than ever to add a new reason and to propagate it
to subscribers.
The patch renames `RoomInfoUpdate` to `RoomInfoNotableUpdate`.
The functions, methods or variables whose names start with
`roominfo_update(.*)`` are renamed `room_info_notable_update$1`.
* ffi: expose methods for SSO login
Signed-off-by: Johannes Marbach <n0-0ne+github@mailbox.org>
* Update bindings/matrix-sdk-ffi/Cargo.toml
Co-authored-by: Ivan Enderlin <ivan@mnt.io>
Signed-off-by: Johannes Marbach <n0-0ne+github@mailbox.org>
* Refactor code to use a separate object to complete the SSO login
* Remove superfluous .workspace
* Remove superfluous field name
* Fix formatting with nightly toolchain
* Fix clippy errors using nightly toolchain
* Move SSO methods over into Client as AuthenticationService will go away very soon
* Add login tests
* Assign tokio runtime and url getter
* Reformat
* Relocate parts of the code as per review comments
* Add url to debugging representation of SsoHandler
* Add example for login_with_sso_callback
* Use unwrap_or_default to avoid creating empty string
* Remove leftover dependencies
---------
Signed-off-by: Johannes Marbach <n0-0ne+github@mailbox.org>
Co-authored-by: Ivan Enderlin <ivan@mnt.io>
When a request fails because of the exponential backoff, it won't be
re-logged again. It would be useful, for the purposes of the send queue
notably, to see when a request is re-attempted.
Synapse returns a bare `{ "membership": "leave" }` as the content of a
room membership event (for leave membership changes and likely others).
In this case, it'd still be nice to have some kind of display
name/avatar URL to show in UIs; it's possible to reuse information from
the previous member event, if available.
Add a new `max_concurrent_requests` parameter in the `RequestConfig` limits the number of http(s) requests the internal sdk client issues concurrently (if > 0). The default behavior is the same as before: there is no limit on concurrent requests issued.
This is especially useful for resource constrained platforms (e.g. mobile platforms), and if your pattern might lead to issuing many requests at the same time (like downloading and caching all avatars at startup).
- [x] Public API changes documented in changelogs (optional)
Signed-off-by: Benjamin Kampmann <ben.kampmann@gmail.com>
It turns out this will cause a network request if the encryption info hasn't been loaded before, which is the case for opening a client in offline mode. It will slow down displaying the room list or loading the room info in general.
Turns out legacy unencrypted objects can take many forms, and we need to be
tolerant of them.
Also expose `deserialize_legacy_value` as a separate function, because we're
going to need to special-case it.
This patch revisits the need to trigger a room list update for
all changes of `RoomInfo`. For the moment, it reduces the scope to
`recency_timestamp` update.
This patch comes with a test to ensure things work as expected.
... and make `deserialize_value` handle both the old and new formats.
`maybe_encrypt_value` uses a much more efficient representation, so let's
migrate to that.
It's currently possible for integ test results to leak from one test run to the
next (for example, the indexeddb stores hang around in the browser), causing
bad test results.
Extend the test setup routine to clear out the store before the test starts.
This patch removes the `LatestEvent::cached_event_origin_ts`.
It's no longer necessary to cache this value as the
`matrix_sdk_ui::room_list_service::sorter::recency` sorter no longer
uses it.
This patch adds a new field in `RoomInfo`: `recency_timestamp:
Option<MilliSecondsSinceUnixEpoch>>`. Its value comes from a Sliding
Sync Room response, that's why all this API is behind `cfg(feature =
"experimental-sliding-sync")`.
This allows seeing the events directly from the room event cache. I'm
hoping this will give us some interesting insights about duplicates,
among other things.
Most tests would randomize the username when creating a
`TestClientBuilder`; make it the default, since it's a sensible choice,
and avoids interference between different tests / test runs.
A single test required an actual non-randomized username, so a specific
way to opt out from this new default behavior has been introduced.
* Use `IndexeddbSerializer` more widely in test code
reuse `IndexeddbSerializer::maybe_encrypt_value` instead of re-inventing it.
* Rewrite `StoreCipher::decrypt_value_base64_typed`
Instead of un-base64-ing and calling `decrypt_value_typed` (which deserializes
the result`), call `decrypt_value_base64_data` (which un-base64s before
decrypting but does not deserialize the result), then deserialize.
This makes it more symmetrical with `encrypt_value_base64_typed`, and helps me
get rid of `decrypt_value_typed` (which is barely used.)
* Fix docs on `StoreCipher::encrypt_value_base64_typed`
looks like they got C&Ped from `encrypt_value_typed`.
* Inline `StoreCipher::{encrypt,decrypt}_value_typed`
Each of these are quite simple, are only used in two places, and their
existence melts my brain.
* Rewrite `IndexeddbSerializer::maybe_{encrypt,decrypt}_value`
... to use `en/decrypt_value_base64_data` instead of
`en/decrypt_value_base64_typed`.
We have to have the de/serialization code for the unencrypted case anyway, so
using the higher-level method isn't helping us much.
* Remove unused `StoreCipher::{en,de}crypt_value_base64_typed`
Outside of tests, these things are totally unused.
This patch removes the `RoomListService::rooms` cache, since now a
`Room` is pretty cheap to build.
This cache was also used to keep the `Timeline` alive, but it's now
recommended that the consumer of the `Room` keeps its own clone of the
`Timeline` somewhere. We may introduce a cache inside `RoomListService`
for the `Timeline` later.
This patch rewrites `merge_stream_and_receiver` to switch the order
of `roominfo_update_recv` and `raw_stream`. The idea is to give the
priority to `raw_stream` since it will necessarily trigger the room
items recomputation.
This patch also remove the `for` loop with `Iterator::enumerate`, to
simply use `Iterator::position`: it's more compact and it removes a
`break` (it makes the code simpler to understand).
Finally, this patch renames `merged_stream` into `merged_streams`.
Complement uses the FFI `RoomList` API. Since the patch set modifies
this API, Complement is broken. We disable it and will re-enable it once
we have updated Complement.
This patch adapts the `RoomList` FFI API to the recent changes to
suport a `Stream<Item = RoomListItem>` instead of a `Stream<Item =
RoomListEntry>`. Behind the scene, it supports client side sorting for
the rooms but this is transparent for this API.
This patch also removes the `RoomListInput` enum as no input is
supporter anymore.
The `entries` method no long returns a `RoomListEntriesResult` but
directly a `TaskHandle`. The given listener will receive the initial
entries as a `VectorDiff::Append`, which first is simpler but also fixe
a potential race condition bug.
This patch adds a new `cached_event_origin_server_ts` field on
`LatestEvent`, which is a copy of the `origin_server_ts` of the inner
`SyncTimelineEvent`.
This patch removes the `visible_rooms` sliding sync list from
`RoomListService`. As we are taking the path of doing client-side
sorting, the ordering of the server-side will most likely always
mismatch the ordering of the client-side, thus using `visible_rooms`
with room indices make no sense (indices from server-side won't map
indices on the client-side, so room ranges from client-side won't map
what the server knows).
We used to use `visible_rooms` to “preload” the timeline of rooms in
the user app viewport, with a `timeline_limit` of 20. This should be
replaced by room subscriptions starting from now. For the moment, the
user of `RoomListService` is responsible to do that manually. Maybe
`RoomListService` will handle that automatically in the future.
There were two disconnected sources of truth for the state of event to
be sent:
- it can or cannot be in the in-memory `being_sent` map
- it can or cannot be in the database
Unfortunately, this led to subtle race conditions when it comes to
editing/aborting. The following sequence of operations was possible:
- try to send an event: a local echo is added to storage, but it's not
marked as being sent yet
- the task wakes up, finds the local echo in the storage,...
- try to edit/abort the event: the event is not marked as being sent
yet, so we think we can edit/abort it
- ... having found the local echo, it is marked as being sent.
This would result in the event misleadlingly not being aborted/edited,
while it should have been.
Now, there's already a lock on the `being_sent` map, so we can hold onto
it while we're touching storage, making sure that there aren't two
callers trying to manipulate storage *and* `being_sent` at the same
time.
This is pretty tricky to test properly, since this requires super
precise timing control over the state store, so there's no test for
this. I can confirm this avoids some weirdness I observed with
`multiverse` though.
Using `SendHandle::abort()` after the event has been sent would look
like a successful abort of the event, while it's not the case; this
fixes this by having the state store backends return whether they've
touched an entry in the database.
It could be that the last event in a room's send queue has been marked
as wedged. In that case, the task will sleep until it's notified again.
If the event is being edited, then nothing would wake up the task; a
manual wakeup might be required in that case.
The new integration test shows the issue; the last `assert_update` would
fail with a timeout before this patch.
Commit d41af396c implemented MSC4147, which puts the device keys into a
Olm message. It failed to adhere to the unstable prefix proposed in the
MSC:
> Until this MSC is accepted, the new property should be named
> org.matrix.msc4147.device_keys.
This patch fixes this.
This patch switches the way we update the recovery state upon changes in
the backup state. Previously two places updated the recovery state after
the backup state changed:
1. A method living in the recovery subsystem that the backup
subsystem itself calls.
2. An event handler which is called when we receive a m.secret.send
event.
The first method is a hack because it introduces a circular dependency
between the recovery and backup subsystems.
More importantly, the second method can miss updates, because the backup
subsystem has a similar event handler which then processes the secret we
received and if the secret was a backup recovery key, enables backups.
Depending on the order these event handlers are called, the recovery
subsystem might update the recovery state before the secret has been
handled.
The backup subsystem provides an async stream which broadcasts updates
to the backup state, letting the recovery subsystem listen to this
stream and update its state if we notice such updates fixes both
problems we listed above. The method in the first bullet point was
completely removed, the event handler is kept for other secret types but
we don't rely on it for the backup state anymore.
Where a string is clearly documented as a room ID, event ID or mxc URI,
use OwnedRoomId, OwnedEventId and OwnedMxcURI, respectively.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
We tend to use fixup commits quite a bit, and in the heat of the moment
we sometimes forget to squash them before the final merge.
This should prevent us from doing so.
This will still disable the room's send queue, but the embedder may then
decide whether to re-enable the queue or not, based on network
connectivity. Ideally, we'd implement some retry mechanism here too, but
since we're at a different layer than the HTTP one, we can't get that
"for free", so let the embedder decide what to do here.
This patch is quite big… `RoomList::entries*` now returns `Room`s
instead of `RoomListEntry`s. This patch consequently updates all the
filters to manipulate `Room` instead of `RoomListEntry`. No more
`Client` is needed in the filters.
This patch also disables the `RoomList` integration test suite in order
to keep this patch “small”.
For each recipient device, we keep an "Olm wedging index" that indicates (approximately) how many times they tried to unwedge the Olm session. When we share a Megolm session, we store the Olm wedging index. This allows us to tell whether the Olm session was unwedged since we shared the Megolm session, so that we know if we should re-share it.
We detect an attempt to unwedge the Olm session by checking if the Olm message that we decrypted is from a brand new Olm session. This is not entirely accurate, since this could happen, for example, if Alice and Bob create Olm sessions at the same time. This may result in some Megolm sessions being re-shared unnecessarily, but should not make a huge difference.
It also resets the wedged state, so that the queue will retry to send
this event later. This will show useful in the following case: when an
event is too big, we can now retry to send it, even if it was blocked,
by splitting the event instead of copy/abort/recreate it.
It turns out that so as to be able to read the room ids, they need to be
*values*, not only *keys* (since keys are one-way hashed). This means we
need to duplicate the room_id field in indexeddb/sqlite, so each entry
contains both the room_id as a key (for queries) and as a value (to
return it).
Since there's no meaningful migration we can apply, the way to go is to
drop the pending events table and recreate it from the ground up. It is
assumed that no one has used the store on indexeddb; otherwise the
workaround would be to drop and recreate it.
A few were missing in the public API like SendEventError and RedactEventError and their dependencies.
This uses a wildcard because it should be rare to not want to expose an error type.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
A user of the library needs to add this crate as a dependency just to be able to match on `VectorDiff`.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
The issue here seems to be that
the `panic!` and `unreachable!()` macros used in the tests return `!`.
In the future, `!` will not fallback to `()`, which is what `dependency_on_unit_never_type_fallback` checks.
`add_event_handler` expects a function that returns an `EventHandlerResult`, but it is only implemented for `()`, not for `!`.
A solution could be to implement that trait for `!` but it is an unstable feature right now.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
It is only needed with the experimental-oidc and e2e-encryption features.
The former is less likely to be enabled so use it to enable the dependency.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
This patch set does two things:
1. Extracted the logic to collect the devices that should receive a room key.
2. Introduce a new CollectStrategy enum which defines which rules are
used to collect recipient devices for a room key. Currently only the
existing rules have beenmoved under this enum.
This can happen if there's a load-balancer or any modification of the
response by a reverse proxy (e.g. rewrite 5XX errors into 429, to not
let a reverse proxy mark the upstream server as being down, as
Cloudflare seems to do).
As a result, such requests will be retried in multiple places, including
when sending something with the send queue. Also, the send queue will
mark these errors as recoverable instead of unrecoverable.
No test, because the change really is trivial and a regression test
didn't seem worth it, for once.
RoomInfo::handle_state_event(...) will check this condition so we don't have to later ask the HS whether the room is encrypted or not through room::is_encrypted().
Also, as we need some way to get this info in the room list in our clients, two ways of doing this were added the FFI crate, one through RoomInfo and another one through RoomListItem.
`RoomInfo::handle_state_event` will check this condition so we don't have to later ask the HS whether the room is encrypted or not through `room::is_encrypted()`.
This adds support to input your recovery key to the OIDC example which
will allow the OIDC example client to be verified and have access to all
the secrets (cross-signing keys and the backup recovery key).
Not particularly useful right now, but once the OIDC example is able to
log in other devices via a QR code it becomes necessary to have access
to all the secrets.
The workspace root enables some features which are required to compile
the benchmarks, but if you decide to just compile the benchmarks these
features won't be enabled since they aren't specified in the Cargo.toml
file of the benchmarks.
Let's define all the required features so compilation works in both
cases.
Fixes#3538
The current implementation for send_reply and edit only work with timeline items that have already been paginated.
However given the fact that by restoring drafts, we may restore a reply to an event for timeline where such event has not been paginated, sending such reply would fail (same for the edit event).
So I reworked a bit the code here to use. only the event id, and reuse the existing timeline if available, otherwise we can fetch the event and synthethise the content and still be able to successfully send the event.
This is the third part of the breakdown of the following PR: https://github.com/matrix-org/matrix-rust-sdk/pull/3439
Change this so that it fires off a task for each UTD event, rather than each
megolm session. This is a step towards considering the message index when
deciding whether to carry on with the download.
Not doing this leads to an invalid ordering of events, as shown in the
test: if we increase the timeline limit of a room using sliding sync,
we'll receive a duplicate event, and if we ignore it, it'll be in an
invalid position. The solution is to keep the most recent event (and
remove the old one from event_meta).
This PR makes 2 changes:
- Updates the storage of room heroes to be a single array containing the user's complete profile.
- Exposes these to the FFI so that client apps can use these for avatar colours/clusters.
Closes#2702 (again, now with avatars 🖼️)
---
* rooms: Store heroes as a complete user profile.
* ffi: Expose room heroes on Room and RoomInfo.
* chore: Remove TODO comment.
* Update crates/matrix-sdk-base/src/rooms/normal.rs
Signed-off-by: Benjamin Bouvier <public@benj.me>
---------
Signed-off-by: Benjamin Bouvier <public@benj.me>
Co-authored-by: Benjamin Bouvier <public@benj.me>
retry kind: make `characterize_retry_kind` a method of `HttpError`
---
retry kind: mark all network errors as transient
This should cause more backoff in general, if the client is disconnected
of the grid, or the remote server is, which is good behavior to have in
general.
---
http error: introduce another rustfmt-formated impl block for HttpError
The previous `impl` block was marked as skipping rustfmt, which led to
weird formatting behavior.
Changes are formatting only.
* sdk: Integ tests: factor out some helper methods
A few common methods for logic shared between the tests. This serves two
purposes:
* It reduces duplication, this making it easier to maintain the tests by
reducing the number of places that we have to change things.
* It makes the tests easier to read. By factoring out discrete steps, it's
much easier to get an overview of what a test is doing than when it's all in
one big function.
* sdk: Integ tests: timeout if nothing happens
If the test is going to fail, let's have it do so properly rather than
mysteriously getting stuck.
This patch creates an `ObservableMap` for `wasm32-unknown-unknown` which
simply wraps a `BTreeMap`, and without the `stream` method. Indeed, the
first implementation uses `eyeball_im::ObservableVector` which requires
`Send` and `Sync` on its values, which cannot compile to Wasm.
This patch implements `Client::rooms_stream` by forwarding the
result of `matrix_sdk_base::Client::rooms_stream`, and mapping the
`matrix_sdk_base::Room` to `matrix_sdk::Room`.
Running `cargo test -p matrix-sdk-base --features e2e-encryption`
makes the code to fail to compile because `crate::latest_event` isn't
defined. And indeed, it must be defined if `experimental-sliding-sync`
is enabled, but also if `e2e-encryption` is enabled.
This patch updates `Store::rooms` from a
`Arc<StdRwLock<BTreeMap<OwnedRoomId, Room>>>` to a
`Arc<StdRwLock<Rooms>>` where `Rooms` is a new type that mimics a
`BTreeMap` but that is observable. It uses an `ObservableVector`
for saving us the costs of writing a new `ObservableMap` type in
`eyeball-im`. It would have too much implications that are clearly
not necessary. The major one being that an `ObservableMap` must emit
`MapDiff`, but we expect `VectorDiff` everywhere in the SDK where rooms
are observable. It would break too many API and too many projects.
The benchmark is coming, but here are the results (for 10'000 rooms,
extreme case):
* `get_rooms` and `get_rooms_filtered` are twice faster (in practise, it
means 650µs is saved per call),
* `get_room` and `get_or_create_room` are 10% slower (in practise, it
means 8-10ns is lost per call).
Overall, I believe these results are acceptable, and even an improvement
for the first one.
This patch removes the useless `SlidingSyncList::get_room_id` method as
I don't see how it can be useful for anybody. It's mostly dead public
code, let's clean that up.
This might be controversial, but I do strongly prefer explicit over
implicit, in this case: it helps looking at the use cases, when using
the LSP's goto_references().
The HttpError variant was misplaced, as it was always used when the
`user_id()` is missing, which is causing `Error::AuthenticationRequired`
errors everywhere else in the code base. This patch streamlines usage of
`Error::AuthenticationRequired`, and gets rid of the `HttpError`
variant.
This patch renames `Client::get_rooms`, `::get_rooms_filtered` and
`::get_room` to respectively `::rooms`, `::rooms_filtered` and `::room`.
This `get_` prefix isn't really Rust idiomatic.
This patch renames `Store::get_rooms`, `::get_rooms_filtered` and
`::get_room` to respectively `::rooms`, `::rooms_filtered` and `::room`.
This `get_` prefix isn't really Rust idiomatic.
`Store::get_rooms` worked as follows: For each room ID, call
* Take the read lock for `rooms`,
* For each entry in `rooms`, take the room ID (the key), then call `Store::get_room`, which…
* Take the read lock for `rooms`,
* Based on the room ID (the key), read the room (the value) from `rooms`.
So calling `get_rooms` calls `get_room` for each room, which takes the lock every time.
This patch modifies that by fetching the values (the rooms) directly
from `rooms`, without calling `get_room`.
In my benchmark, it took 1.2ms to read 10'000 rooms. Now it takes 542µs.
Performance time has improved by -55.8%.
This patch replaces the `RwLock` by a `Mutex` in
`RoomListService::rooms` because:
1. it removes a race condition,
2. `RwLock` was used because the lock could have been taken for a long
time due to the previous `.await` point. It's no longer the case, so we
can blindly use a `Mutex` here.
This patch makes `RoomListService::room` synchronous. It no longer reads
a `SlidingSyncRoom` from `SlidingSync`, then it not needs to be async
anymore. This patch replaces the `RwLock` of `RoomListService::rooms`
from `tokio::sync` to `std::sync`.
The patch updates all calls to `RoomListService::room` to remove the
`.await` point.
This patch updates `room_list_service::Room::new` to take a `&Client`
and a `&RoomId` instead of a `SlidingSyncRoom`. The `SlidingSyncRoom` is
only used in `Room::default_room_timeline_builder` and is fetched there
from the `SlidingSync` instance lazily. It confines the
`SlidingSyncRoom` to one single method for `Room` now.
Using the resolved homeserver URL causes problems if we need to inspect
the well-known configuration of the homeserver, for example, if the
server name is matrix.org, but the homeserver URL is server.matrix.org,
the well-known might be only available for the former.
This is why we also need to receive the former, i.e. the server name in
the QR code data.
This patch removes `RoomInfo::latest_event`. To get the latest event,
one has to use `RoomListItem::latest_event` because it supports
local events and remote events. It was supposed to be the case of
`Room::room_info` too, but only when the method was called: Once the
`RoomInfo` was created, the latest event inside `RoomInfo`
was static, not dynamic. Also, this code wasn't tested
contrary to `RoomListItem::latest_event` which uses
`matrix_sdk_ui::room_list_service::Room::latest_event` which is itself
tested.
This patch changes the behaviour of the `Timeline` when a duplicated
event is received. Prior to this patch, the old event was removed, and
the one was added. With this patch, the old event is kept, and the new
one is skipped.
This is important for https://github.com/matrix-org/matrix-rust-sdk/pull/3512
where events are mapped to the timeline item indices. When an event is
removed, it becomes difficult to keep the mapping correct.
This patch also adds duplication detection for pagination (with
`TimelineItemPosition::Start`).
This patch removes the `matrix_sdk_ui::timeline::SlidingSyncRoomExt`
trait as it's no longer necessary since `room_list::Room::latest_event`
no longer uses `SlidingSyncRoom` to fetch the latest event.
This patch updates `matrix_sdk_ui::room_list::Room::latest_event`
to fetch the latest event from `matrix_sdk::Room` instead of
`matrix_sdk::sliding_sync::SlidingSyncRoom`. It removes one dependency
to `SlidingSyncRoom` and it simplifies the code.
This avoids a race condition where the caller hasn't set up the initial
items or the listener, and the listener is called *before* the initial
items have been used.
second part of the #3439 breakdown
The context for this API, is for the composer preview an reply without the need of it being actively in the timeline.
This PR is the first piece of the breakdown of the following PR: https://github.com/matrix-org/matrix-rust-sdk/pull/3439 where only the core functionalities of the feature are implemented, without addressing the issue of editing and replying to events that have not yet been paginated.
The test looks at updates of `RoomInfo`s, but these depends on external
factors like the server sending them in one part or multiple ones.
Instead of trying to figure out which partial updates the server sent,
wait for the stream of `RoomInfo`s to stabilize by trying to get the
latest item from the stream, then wait up to N seconds for another item
to show up, and continue as long as items come in.
This should allow us to get rid of some code that was required to
prevent flakey updates.
The previous code would use `ACTIVE` which means "either joined or
invited" members, but the code thereafter considered this to be the
number of joined members.
Also replaced a call to `self.members()` (which does a lot of work under
the hood) with a call to `self.joined_user_ids()`, since we only
retrieve the `.len()` of the resulting vector.
- Change incorrect "key sharing" references to "key forwarding".
- Explain that this is a feature that needs to be enabled.
- Regenerate the key forwarding algorithm diagram.
- Provide a spec link and mention alternative names.
Use a Bloom filter to keep track of which events we have already reported to the parent UTD hook.
We load the data from database on startup, and flush it out immediately on every update.
This patch renames `TimelineEventHandler::add` to `add_item`.
This patch also removes the `should_add` argument. The `add_item`
is called conditionally everytime. I believe this is much cleaner.
Otherwise the method should have been called `maybe_add_item` with a
return type or something that indicates whether the item is added. This
patch makes it clear and remove one possible state in `add_item`.
This patch updates `TimelineEventHandler` to re-use the public API and
avoid using `TimelineInnerMeta::next_internal_id`. The consequence is
that `next_internal_id` can now be private instead of public. It's
better to have isolated API in this code.
This patch removes a useless clone of `TimelineInner::items`. This clone
was technically cheap because it's a `Vector` behind the scene, which is
cheap to clone, but still, it's not necessary at all to clone it.
Because of the task cancellation that can happen at any place in the
code base, it's really hard to predict in which situation a
day-divider-adjuster should have run or not, so just demote the assert
to an error. The intent is that, if we see errors with day dividers in
the future, they'll be reported along rageshakes and we'll notice this
in the logs.
Using async when not required will increase compile times, and propagate async-ness to the callers, transitively.
See also the [lint description](https://rust-lang.github.io/rust-clippy/master/#/unused_async).
Since we only had a few false positives, I've enabled it by default.
Also reunify two methods that did the same thing, with slightly
different semantics, and test the one that wasn't tested at all.
Note that `is_editable()` was already exposed to the FFI and used in EX
apps.
It was used after a previous local echo couldn't be sent (i.e. the
sending queue failed to send it). However, it was slightly incorrect to
mark those as cancelled, since those local echoes would still have
corresponding items in the sending queue, and would be retried as soon
as the sending queue was reenabled and could send events.
Instead, this is removed: it means that previously cancelled events
would be in the NotSentYet state, which is correct. (At the limit, we
could even get rid of the SendingFailed variant, since the sending queue
will automatically retry as soon as possible.)
The test relied on the fact that sending an event from a given timeline
is not observable from another timeline. Indeed, it sent a message using
a first timeline object, then constructed a second timeline object and
expected only the remote event to be in there.
Now, the sending queue is shared across all instances of a Room, thus
all instances of a Timeline, and the second timeline can see the local
echo for the message sent by the first timeline.
The "fix" is thus in the test structure itself: when waiting for the
remote echo to be there, check that the timeline item doesn't pertain to
a local echo, i.e. is a remote echo.
Technically, the test already passed before the change in the builder,
because `TimelineEventHandler::handle_event` already filters out local
events on non-live timelines, but it's a waste of resources to even
spawn the local echo listener task in that case, so let's not do it.
By keeping a reference to the FFI Client, we make sure that the SDK
Client is dropped while in a tokio runtime context. This makes it
possible for an `Encryption` (FFI) object to outlive the `Client` (FFI)
object without crashing (because deadpool requires to be in a tokio
runtime context when dropping).
Following https://github.com/matrix-org/matrix-rust-sdk/pull/3473, we
made it so that if there's a base-path but no username, then we'll
create a sqlite database.
Unfortunately, this introduced a bad side-effect: for the temporary
client used during homeserver resolution, this would create a temporary
sqlite database.
The "fix" is to not set the base path for the temporary client, and only
for the other caller of `new_client_builder()`, manually. The long term
fix is to get rid of the `AuthenticationService` so we can test it
properly.
This patch updates Ruma to 75e8829 so that `RoomSummary::heroes` is now
a `Vec<OwnedUserId>` instead of a `Vec<String>`. This patch updates our
code accordingly by removing the parsing of heroes as `OwnedUserId`.
Without this, the configs from `.cargo/config.toml` were not read in CI
tasks, causing false positives when running Clippy on CI (i.e. there
were issues observed when compiling locally that were not found when
compiling remotely).
Not entirely sure why it's needed, because I'm seeing the issues when
I'm using `cargo xtask ci clippy` locally, with nothing changed.
It turns out that the failure came when using the known room path in the
room preview: Alice knows about the room, but for some reason the client
didn't retrieve all the state events from the sliding sync proxy yet.
Before, the sync would be considered stable after 2 seconds. This is too
little, considering that events come from the proxy that listens to
events from synapse. Raising this threshold to 15 seconds should help
getting all the room information from the proxy, and thus get all the
information we expected in the client.
While this mutex is taken, in `oldest_token()` we can get a hold of the
RoomEvent mutex, making it so that the `waited_...` token covers the
critical region of room events.
Unfortunately, in `clear()`, we're taking these two locks in the
opposite order, implying that the room events critical region now
overlaps that of the `waited_...` lock.
The way to break the cycle is to keep the `waited_` token for as short
as possible.
So as to not use sync mutexes across await points, we have to use an
async mutex, BUT it can't be immediately called in a wiremock responder,
so we need to shoe-horn a bit: create a new tokio runtime, which can
only be called from another running thread, etc. It's a bit ugly, but I
couldn't find another mechanism to block the responder from returning a
Response immediately; happy to change that anytime if there's a simpler
way.
A client would require this to know that the sending queue has been
globally disabled, across all the rooms, otherwise they couldn't know
that it needs to be re-enabled later on.
This implements the following features:
- enabling/disabling the sending queue at a client-wide granularity,
- one sending queue per room, that lives until the client is shutting
down,
- retrying sending an event three times, if it failed in the past, with
some exponential backoff to attempt to re-send,
- allowing to cancel sending an event we haven't sent yet.
fixup sdk
This can be equivalently retrieved using:
`get_timeline_event_by_event_id(event_id).content().as_message()?.content()`
As per the commit date, this is used in one place in both EXA and EXI,
so it seems fine to change the caller's call sites.
And this avoids one explicit call site of
`Timeline::item_by_event_id()`.
If an event cannot be decrypted after a grace period, it is reported to the application (and thence Posthog) as a UTD event. Currently, if it is then successfully decrypted, the event is then re-reported as a "late decryption".
This does not match the expected behaviour in Posthog - an event is *either* a UTD, or a late decryption; it makes no sense for it to be both. This PR fixes the problem.
I've attempted to make the commits sensible, but I'm not entirely sure I've succeeded. The tests do pass after each commit, though. The interesting change itself is somewhere in the middle; there is non-functional groundwork before and cleanup afterwards.
---
* ui: Factor out UTD report code to a closure
For now, this doesn't help much, but in future there will be more logic here,
and it helps reduce the repetition between the delay and no-delay cases.
* ui: Convert UtdHookManager::pending_delayed to a HashMap
* ui: Store decryption time in `UtdHookManager::pending_delayed`
This is a step on getting rid of `known_utds`
* ui: Fix re-reporting of late decryptions
This fixes the problem where a message that was previously reported as a UTD,
and was then subsequently successfully decrypted, is then re-reported as a late
decryption. This artificially inflated the UTD metrics.
We do this by checking the `pending_delayed` list in `on_late_decrypt`, instead
of the `known_utds` list. There is some associated reordering of code to get
the locking right.
* ui: Remove unused "utd report time" from `UtdHookManager::known_utds`
* ui: Replace `UtdHookManager::known_utds` with `reported_utds`
Keep a list of the UTDs we've actually reported, rather than the union of those
we've reported together with those we might report in a while.
I find this much easier to reason about.
* Address minor review comments
* Reinstate assertion in UTD hook tests
* Reinstate `known_utds`
To differentiate the SAS state between the party
that sent the verification start and the party that received it.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
This patch adds a new argument to the `until` argument closure of
`RoomPagination::run_backwards`: `timeline_has_been_reset`, which
designates when the timeline has been reset.
A test has been updated accordingly.
* chore(sdk): Rename `RoomEventCacheUpdate::UpdateReadMarker`.
This patch renames `RoomEventCacheUpdate::UpdateReadMarker` to
`ReadMarker`. The `Update` prefix is already part of the enum name.
* feat(sdk): Rename `RoomEventCacheUpdate::ReadMarker::event_id` to `move_to`.
This patch renames `RoomEventCacheUpdate::ReadMarker::event_id` to
`ReadMarker::move_to` as I feel like it conveys a better semantics.
* feat(sdk): Extract `RoomEventCacheUpdate::Append::ambiguity_changes`.
This patch extracts `RoomEventCacheUpdate::Append::ambiguity_changes`
into a new variant `RoomEventCacheUpdate::Members { ambiguity_changes }
`.
This patch also creates a new private
`RoomEventCacheInner::send_grouped_updates_for_events` method to ensure
the updates are sent in a particular order.
* feat(sdk): Rename `RoomEventCacheUpdate::Append`.
This patch renames `RoomEventCacheUpdate::Append` to `SyncEvents`. The
field `events` is renamed `timeline`.
This patch also renames some variables to clarify the code and to match
the renamings in `RoomEventCacheUpdate`.
* feat(sdk): Rename `RoomEventCacheUpdate::ReadMarker` and `Members`.
This patch renames `ReadMarker { move_to: … }` to `MoveReaderMarkerTo
{ event_id: … }`.
This patch also renames `Members` to `UpdateMembers`.
Finally, this patch renames some other variables to avoid clashes with
terminology in `matrix_sdk_ui`.
* feat(sdk): Split `RoomEventCacheUpdate::SyncEvents`.
This patch splits `RoomEventCacheUpdate::SyncEvents` into 2 new variants:
`AddTimelineEvents` and `AddEphemeralEvents`.
This patch takes this opportunity to update `matrix_sdk_ui::timeline`
a little bit too. `handle_sync_events` is renamed
`handle_ephemeral_events`, and the `SyncTimelineEvent` argument is
removed: it's possible to use `add_events_at` directly to handle the
`SyncTimelineEvent`s.
* fix(sdk): Do not send `RoomEventCacheUpdate` if values are empty.
This patch prevents sending useless `RoomEventCacheUdpate` if their
respective values are empty.
* chore(ui): Update a log message.
* Apply suggestions from code review
Signed-off-by: Benjamin Bouvier <public@benj.me>
---------
Signed-off-by: Benjamin Bouvier <public@benj.me>
Co-authored-by: Benjamin Bouvier <public@benj.me>
This patch adds a new argument to `RoomPagination::run_backwards`:
`until`. It becomes:
pub async fn run_backwards<F, B, Fut>(&self,
batch_size: u16,
mut until: F
) -> Result<B>
where
F: FnMut(BackPaginationOutcome) -> Fut,
Fut: Future<Output = ControlFlow<B, ()>>,
The idea behind `until` is to run pagination _until_ `until` returns
`ControlFlow::Break`, otherwise it continues paginating.
This is useful is many scenearii (cf. the documentation). This is
also and primarily the first step to stop adding events directly from
the pagination, and starts adding events only and strictly only from
`event_cache::RoomEventCacheUpdate` (again, see the `TODO` in the
documentation). This is not done in this patch for the sake of ease
of review.
This is the right goal, and Ruma will be updated to reflect that the
`heroes` field should contain `OwnedUserId` too in [1].
This simplifies the code a bit, and avoids a round-trip encoding
user-ids into a string then decoding them from a string later, in the
case of sliding sync and room name computation.
[1] https://github.com/ruma/ruma/pull/1822
Since rendering is sync, and I want the computed display name (which
retrieval is async), I have to fetch the display name early, just after
getting the room. Oh well :)
This patch does 3 things:
1. It updates Ruma to the latest revision at the time of writing,
2. It updates `matrix-sdk-ffi` to provide the
`RoomSubscription::include_heroes` field,
3. It updates `matrix-sdk-base` so that SlidingSync consumes `heroes`
from the response and update the `RoomSummary` accordingly.
A test has been added to ensure the `RoomSummary` is updated as expected.
It doesn't make sense to expose it for a focused timeline.
Note there's no way for timeline to change focus, at the moment; this
would require special handling here, because one could subscribe to the
back pagination status on a live timeline first, then switch the
timeline to the focus mode. The stream wrapper could then observe other
states that aren't expected by the converter method.
This is actually required for one use case: if the event cache triggers
back-pagination in the background *before* a timeline is opened, it's
possible the timeline observes pagination is running at the beginning,
and doesn't know if more back-pagination is required or not.
This wraps the `Paginator::status` stream by reading the
hit_timeline_start() from the room pagination and adding it to the
`Idle` state.
This whole method is a bit broken. It assumes that, when we did an import from
backup, the backup that they came from is the same as the "current" version.
We *could* add another argument, but to be honest I find the whole method a bit
pointless. It's not particularly tied to the `BackupMachine` abstraction. Let's
instead expose `Store::import_room_keys` and
`ExportedRooomKey::from_backed_up_room_key` publicly, and tell callers to use
that directly.
When we are importing a batch of room keys, use the newly-added
`CryptoStore::save_inbound_group_sessions` method instead of
`CryptoStore::save_changes`.
To do this, we need to pass the backup version into `Store::import_room_keys`
instead of just `from_backup` flag.
When we add a batch of inbound group sessions to the store, if they came from a
backup, we need to record which backup version they came from.
`CryptoStore::save_changes` doesn't give us an easy way to set the backup
version (we could add it to the `Changes` struct, but ugh).
So, we add a new method `save_inbound_group_sessions` which takes the backup
version as a separate param. This works fine, because whenever we import
sessions there are no other changes to store.
The new method isn't used outside of tests yet -- that comes in a follow-up
commit.
This moves code around, and tweaks the behavior:
- `WeakRoom::get()` returns an `Option`, that will be None if the client
is missing or the room is missing.
- `PaginableRoom` for `WeakRoom` will now return a default response if
the room could not be reconstructed from the weak room. It's fine to do
so because we're in a shutdown context.
It's possible the timeline starts with a read marker, in which case the
first item won't be a day divider; in that case, check for the next
item, if present.
test_start_with_read_marker
This patch adds another check to ensure `AsVector` generates
`VectorDiff`s that, once combined, produce an expected `Vector`. It
avoids errors when unit testing `VectorDiff` alone.
This patch ensures that an underflow is not possible when the length
of `chunks` is 0. In practise, it's not possible because there is
_always_ one chunk inside `LinkedChunk`, but it's better to have good
code habits.
This patch removes the `unsafe` part of `as_vector`. The idea is to
pass `Iter` (the forward iterator of `Chunk`) to `AsVector` so that it
internally computes `initial_chunk_lengths`. The shape of this data must
no longer be guaranteed by the caller.
This patch goes a bit further: `UpdateToVectorDiff` has
a new constructor which consumes this `Iter` and builds
`initial_chunk_lengths` itself. Even better!
Finally, `Updates::as_vector` is removed. It's clearly no longer
necessary and it was creating borrowing issues anyway with the new code
structure.
Allow applications to skip the PBKDF2 operation if they already have a cryptographically secure key,
instead using a simple HKDF to derive a key.
In order to maintain compatibility for existing element-web sessions, if we discover that we have an
existing store that was encrypted with a key derived from PBKDF2, then we reconstruct what
element-web used to do: specifically, we base64-encode the key to obtain the "passphrase" that
was previously passed in. If that matches, we know we've got the right key, and can update the
meta store accordingly.
Part of a resolution to element-hq/element-web#26821.
Signed-off-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Co-authored-by: Damir Jelić <poljar@termina.org.uk>
This patch renames `ReattachItems` to `StartReattachItems` and
`ReattachItemsDone` to `EndReattachItems`. This naming conveys better
the idea of a _state transition_.
Add a CI step running complement crypto, automatically matching the complement-crypto branch name based on the current branch name, if needs be.
Signed-off-by: Kegan Dougal <7190048+kegsay@users.noreply.github.com>
We could end up, like in the regression test, with a sequence of
operations like that:
- remove day divider @ i+1 (because it's redundant with one @ i)
- remove day divider @ i (because it's useless, since the event before
the day divider and after the day divider use the same date).
In that case, it would break the non-decreasing invariant: we'd apply an
operation on the array @ i+1, then @ i, which troubles the offset
computation.
Instead, when doing an operation based on the "prev_item" (now with a
small helper struct, to facilitate understanding of each field), we also
record the insertion order for the operation itself: it's always "at the
end of the operation order, at the time we're looking at it", so
equivalent to a "push_back" if there's no operation in between; but that
ensures that we'll do the operation in a non-decreasing order. For
instance in the above test case, the Remove(i) is now inserted before
the Remove(i+1), instead of after.
* sdk: Return a Thumbnail from generate_image_thumbnail
We have already all the data for it.
Also fixes an error where the thumbnail format was assumed to always be
JPEG.
* sdk: Allow to select the format of the generated thumbnail
Sending an attachment could often fail if the image crate
cannot encode the thumbnail to the same format as the original.
This allows to select a known supported format to always
be able to generate a thumbnail.
* sdk: Do not return error of thumbnail generation for SendAttachment
Since the thumbnail is optional, failing to generate it should not
stop us from sending the attachment.
* Apply code review fixes
* sdk: Split attachment tests in separate file
* sdk: Add integration tests for generating thumbnails
* Revert wiremock debug log level
---------
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
It's always provided in the `avatar` response in the field, which
sliding sync's using by default, so there's no need to request the
`m.room.avatar` event too.
If we want to be able to note the absence of a room name, we shouldn't
take the name returned by the server (which may be computed, and thus
always be non-null). This way, we can expose both the name contained in
the m.room.name event as well as the computed name everywhere.
A consequence of this is that the room list service must now ask for the
m.room.name event as part of the required state for every room.
With a regression test that didn't pass on main, and would incorrectly
remove the read marker, because it tried to replace the day divider at
(4), that was just scheduled for deletion in the previous loop
iteration.
This patch removes the possibility to have `LinkedChunk` as an
`ObservableVector` via a `Stream`, which was initially the goal of
`AsVectorSubscriber`. Having a `Stream` was more complex than required
for the integration inside `EventCache`. This patch keeps the same code
but renames `AsVectorSubscriber` into `AsVector`. A new internal type,
`UpdateToVectorDiff`, applies the algorithm itself and maintains its
own state, making it possible to re-introduce a `Stream` later if we
want to.
* event cache: reuse the paginator internally
Fixes#3355.
* event cache: move the `pagination_token_notifier` into the `RoomPaginationData` as well
* event cache: introduce a `RoomPagination` API object and move code around
Only code motion. No changes in functionality.
* event cache: remove "paginate" (et al.) in `RoomPagination` method names
No changes in functionality, just renamings.
* event_cache/timeline: have the event cache handle restarting a back-pagination that failed under our feet
When a timeline reset happens while we're back-paginating, the event
cache method to run back pagination would return an success result
indicating that the pagination token disappeared. After thinking about
it, it's not the best API in the world; ideally, the backpagination
mechanism would restart automatically.
Now, this was handled in the timeline before, and the reason it was
handled there was because it was possible to back-paginate and ask for a
certain number of events. I've removed that feature, so that
back-pagination on a live timeline matches the capabilities of a
focused-timeline back-pagination: one can only ask for a given number of
*events*, not timeline items.
As a matter of fact, this simplifies the code a lot by removing many
data structures, that were also exposed (and unused, since recent
changes) in the FFI layer.
* Address review comments
This patch adds 2 new updates: `ReattachItems` and `ReattachItemsDone`.
It's useful to optimise insertion, esp. in `AsVectorSubscriber` but
almost maybe in database.
This patch renames `TruncateItems` to `DetachLastItems`. It's
technically the same things, but the fields are different: `chunk:
ChunkIdentifier` and `length: usize` become a unique `at: Position`. The
spirit is the same but the semantics is a bit different.
This patch, with this renaming, also prepares the next commits.
First off, this patch renames `Update::InsertItems` to
`Update::PushItems` because all items insertions happen at the end of
a chunk. Let's reflect that. `InsertItems` would have meant that it's
possible to insert at any position, but it's not what happens by design.
Second, this patch renames `Update::PushItems::at` to
`Update::PushItems::position_hint`. Indeed, the `at` field would have
meant that the position is specific whilst it's not. It's just a hint to
make the life of update readers simpler. It's also a good way to improve
performance in some cases: since items are pushed, to know the new
position of the items, one has to read the position of the previous last
item and compute the new position from there. Having `position_hint`
help to prevent this dance and save the cost of a reading. That's also
why the field has been renamed to `position_hint`, it's a hint.
This patch updates the constructor of `AsVectorSubscriber` to receive
the initial chunk lengths so that this type doesn't have to guess what
to do when receiving an unknown chunk identifier. `AsVectorSubscriber`
is de facto synchronized with its source `LinkedChunk` when created.
I found the previous API confusing. Firstly, the parameter name was "filename" when we want a file path.
Secondly, we take a `String` when we want a `Path`.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Cargo nightly now checks whether a cfg option exists.
We need to declare the custom
options we use
and fix those that are wrong.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
This makes the public key fields private to ensure that we don't
accidentally swap them out. It also moves the construction of the
subkeys into the master key type.
Since the master/self-signing/user-signing public key types are used for
public user identities as well as for the private key type we have, and
we'd like to sign the public key types it makes sense that the types
itself aren't using an Arc.
Let's instead put the Arc inside the user identity structs.
This will allow us later on to more easily sign the public key types.
This patch removes the notion of `take` vs. `peek` from
`LinkedChunkUpdatesInner` and widens the problem to a more general
approach: `LinkedChunkUpdatesInner` must support multiple readers, not
only two (`take` was the first reader, `peek` was the second reader,
kind of).
Why do we need multiple readers? `LinkedChunkUpdates::take` is
clearly the first reader, it's part of the public API. But the private
API needs to read the updates too, without consuming them, like
`LinkedChunkUpdatesSubscriber`. `peek` was nice for that, but it's
possible to have multiple `LinkedChunkUpdatesSubscriber` at the same
time! Hence the need to widen the approach from 2 readers to many
readers.
This patch introduces a `ReaderToken` to identify readers. The last
indexes are now all stored in a `HashMap<ReaderToken, usize>`. The rest
of the modifications are the consequence of that.
The test `test_updates_take_and_peek` has been entirely rewritten to be
`test_updates_take_and_garbage_collector` where it tests 2 readers and
see how the garbage collector reacts to that.
This patch implements `LinkedChunkUpdates::subscribe` and
`LinkedChunkUpdateSubscriber`, which itself implements `Stream`.
This patch splits `LinkedChunkUpdates` into `LinkedChunkUpdatesInner`,
so that the latter can be shared with `LinkedChunkUpdatesSubscriber`.
This patch adds the `LinkedChunkUpdates::peek` method. This is
a new channel to read the updates without consuming them, like
`LinkedChunkUpdates::take` does.
The complexity is: when do we clear/drop the updates then? We don't
want to keep them in memory forever. Initially `take` was clearing
the updates, but now that we can read them with `peek` too, who's
responsible to clear them? Enter `garbage_collect`. First off,
we already need to maintain 2 index, resp. `last_taken_index` and
`last_peeked_index` so that `take` and `peek` don't return already
returned updates. They respectively know the index of the last update
that has been read. We can use this information to know which updates
must be garbage collected: that's all updates below the two index.
Tadaa. Simple. The only _drawback_ (if it can be considered as such)
is that the garbage collection happens on the next call to `take` or
`peek` (because of the borrow checker). That's not really a big deal in
practise. We could make it happens immediately when calling `take` or
`peek` but it needs more pointer arithmetic and a less straighforward
code.
This patch updates the `LinkedChunk::update_history` field from
a simple `Vec<LinkedChunkUpdate<Item, Gap>>` type to the new
`LinkedChunkUpdates<Item, Gap>` type (note the plural).
This is going to be helpul for the next patches.
This patch removes support for OpenTelemetry because it's not used
anymore by anybody. It also adds multiple duplicated dependencies (like
`reqwest`). Anyway. Farewell.
Preferring to use "reject" wording rather than "failed to
create/update". The latter can be easily misinterpreted as a failure of
the local client to create an entirely new device from scratch, rather
than refusal to instantiate a new local device representation of an
(invalid) device definition received from the server.
This patch makes the `LinkedChunk::update_history` field optional,
so that it doesn't require the user to drain it to avoid eating the
universe.
The `new` constructor disabled the update history, the
`new_with_update_history` enables it.
This patch is a turn around about the `LinkedChunkListener`. Many
patches have been removed because `LinkedChunkListener` needed to
support I/O, so errors and async code. The whole code was affected by
that, resulting in a complex API. The idea of this patch is to decoupled
this. Here is how.
First off, `LinkedChunkListener` is removed. So it's one less generic
parameter on `LinkedChunk`. It's also one less trait, so less
implementations.
Second, now `LinkedChunk` accumulates/collects all “updates” under the
form of a new enum `LinkedChunkUpdate`. These updates can be read with
`LinkedChunk::updates(&mut self) -> &Vec<LinkedChunkUpdate>`. The reader
can simply read them, or even drain them. The reader is responsible
to handle these updates and to dispatch them in a storage or whatever.
`LinkedChunk` is no longer responsible to do that, removing the need to
support errors and to be async.
Third, the simplification has led to an optimisation by introducing a
new type `LinkedChunkLinks`. The documentation explains what it does
and why it was needed. The benefit of this type is: it doesn't increase
the size of `LinkedChunk`, but it simplifies the code: no more `Arc`,
no more `Mutex` (it was required because with I/O and async), no more
borrow checker trick, and the code stays as safe as before.
This patch removes the `displaydoc` dependency. Why?
1. It creates a warning in rustc nightly:
```
warning: non-local `impl` definition, they should be avoided as they go against expectation
--> crates/matrix-sdk-store-encryption/src/lib.rs:49:17
|
49 | #[derive(Debug, Display, thiserror::Error)]
| ^^^^^^^
|
= help: move this `impl` block outside the of the current constant `_DERIVE_Display_FOR_Error`
= note: an `impl` definition is non-local if it is nested inside an item and may impact type checking outside of that item. This can be the case if neither the trait or the self type are at the same nesting level as the `impl`
= note: one exception to the rule are anon-const (`const _: () = { ... }`) at top-level module and anon-const at the same nesting as the trait or type
= note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
= note: the derive macro `Display` may come from an old version of the `displaydoc` crate, try updating your dependency with `cargo update -p displaydoc`
= note: `#[warn(non_local_definitions)]` on by default
= note: this warning originates in the derive macro `Display` (in Nightly builds, run with -Z macro-backtrace for more info)
```
2. `thiserror` is already used, which seems to provide a similar issue.
3. That's less dependency, and less proc-macro, which will improve the
compilation time in general.
Also:
- rename `display_name` to `computed_display_name` in several places,
and reflect that change into a few callers
- simplify slightly the `computed_display_name()` method
This is in line with what the other method using sliding sync does. This
wasn't tested before, because this required `filter_by_push_rules()` to
be enabled in the notification client; now that it's the default, the
test revealed the bug, and so it could be fixed.
The builder had only one meaningful method, `filter_by_push_rules`,
which was always called by the applications — and in fact should always
be true. It was designed as an extra method because it was experimental
at the time, but it's stabilized sufficiently that we can enable this
behavior by default now, considering that a notification that is not
wanted by the user shouldn't be kept, to respect their intent. (This is
in the UI crate, which is opinionated, so it's fine to assume such
intents by design.)
They need to be updated together
because the latters depend on the former.
matrix-authentication-service is still using http 0.2
so we need to add a conversion layer between both major versions
for OIDC requests.
We need to update vodozemac too because of a dependency resolution issue.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
This makes all the requests (/context and /messages) cancellation-safe
by making two changes:
- first, use sync locks instead of async locks for the prev/next batch
tokens. This ensures that we can't get cancelled after receiving a
response and managing internal state, i.e. we keep run-to-completion
semantics until the end of the function, once we got a response.
- second, introduce a RAII guard that will reset the state to a given
value when dropped. Then use this to reset the state to Initial (resp.
Idle) in /context (resp. /messages) when the async call is aborted. We
use `mem::forget` once the response has been returned, so as to not call
the `Drop` implementation of the guard later on.
A regression test has also been introduced.
This patch changes the signature of `LinkedChunk<Item, Gap, const
CHUNK_CAPACITY = usize>` to `LinkedChunk<const CHUNK_CAPACITY = usize,
Item, Gap>`. It allows to add more generic parameters if needed, without
conflicting with generic constants.
The clippy website explains that `Box::new(Default::default())` can be
shortened to `Box::default()` and that it's more readable. That's true…
when you don't have a generic parameter in the mix (which may be
required if rustc can't infer the type of the boxee), in which case it
just looks bad, e.g. `Box::<MyType>::default()` instead of
`Box::new(MyType::default())`.
I do strongly prefer the latter, and propose to get rid of the lint, as
a result.
Version 0.7.5 triggers a warning in
our version of rust nightly in CI,
but we can't update it to a recent
version because it triggers a warning
caused by displaydoc
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
This introduces the `TimelineFocus`, a new enum to declare if the
timeline is "live" aka looking at events from sync and displaying them
as they come in, or focused on an event (e.g. after clicking a
permalink).
When in the second mode, the timeline can paginate forwards and
backwards, without interacting with the event cache (as this would
require some complicated reconciliation of known events with events
received from pagination, with no guarantee that those events are event
connected in whatever way).
An event-focused timeline will also show edits/reactions/redactions in
real-time (as the events are received from the sync), but will not show
new timeline items, be they for local echoes or events received from the
sync.
`Room::invite_details()` can return an error if the room member event
(the invite) is missing from the store. Usually that would be a sign
that the state is semi-broken, since there should always be such an
event for an invited room. But if it's missing, it shouldn't break the
creation of a `RoomInfo`, and just be missing from the struct.
In https://github.com/matrix-org/matrix-rust-sdk/pull/2691, I suppose
the way `add_mentions` is computed is… wrong. `AddMentions` is used to
automatically infer the `m.mentions` of the reply event based on the
replied event. The way it was computed was based on the reply event
`mentions`, which seems wrong: if the reply contains mentions, then the
sender should be part of it? Nah. That's a bug. We want the reply event
to automatically mention the sender of the replied event if and only
if it's not the same as the current user, i.e. the sender of the reply
event.
This patch fixes the `add_mentions` calculation. This patch also updates
a test and adds another test to ensure that `m.mentions` is correctly
defined when replying to an event.
The `state` lock was taken at the top level of this function, and
indirectly implicitly in the `set_fully_read_event` function. This fixes
it, and adds a regression test.
Fixes#3311.
When the timeline is lagging behind the event cache, it should not only
clear what it contains (because it may be lagging some information
coming from the event cache), it should also retrieve the events the
cache knows about, and adds them as if they were "initial" events.
This makes sure that the event cache's events and the timeline's events
are always the same, and that, in the case of a lag, there won't be any
missing chunks (caused by the event cache back-pagination being further
away in the past, than what's displayed in the timeline).
The lag behind a bit too hard to reproduce, I've not included a test
here; but I think this should be the right move.
Redaction was requiring the `RoomRedactionEventContent`, which was
eventually unused in all the code paths; this removes it.
Also adds lots of documentation for the different types of reaction that
can happen in the timeline.
This introduces a new helper object to run arbitrary pagination requests, backwards- or forward-. At the moment they're disconnected from the event cache, although I've put the files there for future convenience, since at some point we'll want to merge the retrieved events with the cache (? maybe).
This little state machine makes it possible to retrieve the initial data, given an initial event id, using the /context endpoint; then allow stateful pagination using a paginator kind of API. Paginating in the timeline indicates whether we've reached the start/end of the timeline.
The test for the state subscription is quite extensive and makes sure the basic functionality works as intended.
Some testing helpers have been (re)introduced in the SDK crate, simplifying the code, and introducing a better `EventFactory` / `EventBuilder` pattern than the existing one in the `matrix-sdk-test` crate. In particular, this can make use of some types in `matrix-sdk`, notably `SyncTimelineEvent` and `TimelineEvent`, and I've found the API to be simpler to use as well.
Part of #3234.
This protects against the sliding sync proxy (or synapse) sending lots
of duplicate fully read marker events for the same room in case of
reset. This would lead to forwarding lots and lots of
`RoomEventCacheUpdate`s to the timeline, cluttering the channel, and
resulting in a lag. This lag would then clear the timeline, seemingly
cause missing chunks from history.
https://github.com/matrix-org/matrix-rust-sdk/pull/3312 is likely the
long term fix (after a clear(), the timeline should retrieve all the
memoized events from the event cache), but this should prevent the root
cause of the spamming in the first place, getting us back to a safe
state.
We do see lots of "broken channel" log lines for these two log
statements, and since I'm unclear why they happened, I'd like to add a
bit more logging to those.
Also makes the log level consistent, both are set to "warn" instead of
one warn and one error. Usually it's not a big deal because the only
error that may happen is that the channel is broken, indicating the task
died before, so there's no need to stop it manually.
Those two issues really aren't errors we should be too scared of:
- on the one hand, they don't prevent correct usage of the timeline, but
slightly decrease the UX's quality
- on the other hand, they may indicate that read receipts haven't been
received yet, OR some events are unknown (can be happening if the
previous read receipt refers to an event the timeline doesn't know
about).
So I lowered their severity to debug instead of error, since only
outstanding issues should errors or warnings in my opinion.
`force_update_sender_profiles` goes through all the timeline items, even
though the `ambiguity_changes` map should be empty, leading to wasted
work. It might not be a big deal, but it's nice to avoid awaiting locks
when we don't have to.
First, add a log line, since this is a pretty big deal if we start
lagging behind, and we should understand this clearly in rageshakes.
Second: don't remove existing `RoomEventCache`s, but instead clear them:
we shouldn't re-create new `RoomEventCache`s for the same room id,
otherwise a previous subscriber to updates to `RoomEventCache` would not
get newer updates coming later on.
Third: let consumers know that we cleared the events from the
`RoomEventCaches`.
* ffi: Use async functions in AuthenticationService
* Fix swift tests
* Set async_runtime for uniffi::export attribute
* Rename RwLocks
* Get rid of unnecessary map_err
---------
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Before this patch, only local echoes that were messages not sent or
messages succesfully sent (but not ack'd yet by sync) would be pinned to
the bottom.
This fixes it by also pinning events that failed to send.
Fixes#3287.
The algorithm works on the basis that we remove a previous day divider
if it was spurious. But we'd never do this, for a final item that would
be a day divider! So chase these explicitly, also ignore read markers
that would be in the way.
The `MemberInfo` struct only existed to regroup parameters, so let's
pass individual parameters instead. One less data structure is good for
the cognitive load.
This patch fixes a bug when inserting items at the end of the
current items of a chunk. The condition was: `if item_index >=
current_items_length`, which is too restrictive. The bug was “hidden”
so far because it's not possible to get the position of “after an item”
with the current API. However, the bug arises if the items are empty and
new items are inserted the position index 0 (index = 0, length = 0).
The fix consists of relaxing the condition, and introducing an
optimisation. If we insert at the end, we do a simple push (like an
append). If we insert at another position, we split, then push the new
items, and finally push the detached items (as it was the case before).
The tests have been updated accordingly.
Prior to this patch, in `RoomEventCacheInner::backpaginate`, when the
`token` validity was checked, and it was invalid:
* before calling `/messages`, `Err(EventCacheError::UnknownBackpaginationToken)` was returned,
* after calling `/messages`, `Ok(BackPaginationOutput::UnknownBackpaginationToken)` was returned.
This patch tries to uniformize this by only returning
`Ok(BackPaginationOutput::UnknownBackpaginationToken)`.
That's a tradeoff. It will probably be refactor later.
The idea is also to call `/messages` **before** taking the write-lock
of `RoomEvents`, otherwise it can keep the lock for up to 30secs in
this case. Also, checking the validity of the `token` **before** and
**after** `/messages` is not necessary: it can be done only after.
This patch ensures that operations on `RoomEvents` happen in one block,
by sharing the same lock.
2 new methods are created: `replace_all_events_by` and
`append_new_events`.
The test `test_reset_while_backpaginating` was expecting a
race-condition, which no longer exists. It first initially tried to
assert a workaround about this race-condition. It doesn't hold anymore.
Rewrite the test to assert the (correct) new behaviour.
This patch implements the following wrapper methods (over
`LinkedChunk`): `push_gap`, `replace_gap_at` and `events`. This patch
also implements the `reset` method that clears/drops all chunks in the
`LinkedChunk`.
There was a message sent, *then* an attempt to wait for the remote echo later. It's not ideal, because if the time setting up the waiting is high enough, and the server is fast
enough, the remote echo could come *before* we started waiting for it, resulting in timeouts. This fixes it by spawning the waiting task first, and then only sending the message.
Let's see how this helps with this test.
This adds additional checks for each room updates, and works around a few race conditions, notably one where the server would send a remote echo for a message, but not update the
computed unread_notification_counts immediately. This tends to make the test more stable, in that each response is well known and now properly tested.
It's not worth panic'ing the whole timeline because we removed the wrong item; worst case, users will complain and can send a rageshake that contains all the information that's
needed to debug what went wrong.
There could be situations where we have a day divider, then a read marker, then an event. In that case, when looking at the event, if the previous day divider is wrong and needs
to be removed, we would assume the "previous item" (= the day divider) was at position i-1, which could be that of the read marker, and we'd remove the read marker instead of the
day divider.
Closes#3265.
There is currently no way to get the URL of a homeserver's sliding sync proxy before logging in on the homeserver.
I suggest exposing the URL via the `HomeserverLoginDetail` struct after configuring the homeserver (through `configure_homeserver`).
Since the homeserver may not declare a corresponding sliding sync proxy, this value is an `Optional`. It is used later in `configure_homeserver` to check if a sliding sync proxy exists and throws an error otherwise. Previously, this check was done against the client's `discovered_sliding_sync_proxy` function.
- [ ] Public API changes documented in changelogs (optional)
Signed-off-by: Thomas Völkl <thomas@vollkorntomate.de>
Signed-off-by: Benjamin Bouvier <public@benj.me>
Co-authored-by: Benjamin Bouvier <public@benj.me>
Local echoes (which haven't received a remote echo yet) can have no event id, so when computing the report, don't unwrap the event id but use a sensible
default instead.
Also tweaks comments from a previous version of another PR. And rename `DayDividerAdjuster::maybe_adjust_day_dividers` to `run`.
The previous API relied on the callers not forgetting about adjusting day dividers after handling an event.
This makes it statically impossible, by requiring that `TimelineEventHandler` takes a `&mut DayDividerAdjuster` when operating, that it marks as not "consumed". Later, the caller must call `DayDividerAdjuster::maybe_adjust_day_dividers()`, to mark it as consumed. When dropping, we check that it's been consumed, otherwise we panic — as it's a developer error to not call `maybe_adjust_day_dividers()`.
Notably, split the code into smaller functions, and revamp the high-level signatures so the individual handle_ functions don't take a bazillion arguments.
The reports now include the final state as well as the set of operations to run, so we can really debug all the steps just from looking at a rageshake.
When we're removing or inserting any day divider, we're also updating the offset, so that subsequent operations happen at the right positions.
The previous code made the error to clamp the offset when assigning it, instead of letting it be "out of bounds" and clamping the uses, which is
the correct way to implement this.
The test has been failing with a timeout recently, several time. Let's see if it was a fluke caused by the low threshold (because the server might be
busy handling other requests from other tests), or an actual issue.
It appears that tarpaulin complains if the symbol information is stripped from
the binary, and as of Rust 1.77, `debug=0` causes Cargo to strip all debug
info.
To fix this, set `debug=1`.
This patch renames `ChunkIdentifierGenerator::generate_next` to `next.
This patch also simplifies a `.saturating_add(1)` to a simple `+ 1`,
which is fine because we have checked for overflow just before.
Up until uniffi 0.26 it was not possible to send objects
across the boundary unless they were wrapped in an `Arc<>`,
see https://github.com/mozilla/uniffi-rs/pull/1672
The bindings generator used in complement-crypto only supports
up to uniffi 0.25, meaning having a function which returns objects
ends up erroring with:
```
error[E0277]: the trait bound `TaskHandle: LowerReturn<UniFfiTag>` is not satisfied
--> bindings/matrix-sdk-ffi/src/room_directory_search.rs:109:10
|
109 | ) -> TaskHandle {
| ^^^^^^^^^^ the trait `LowerReturn<UniFfiTag>` is not implemented for `TaskHandle`
|
= help: the following other types implement trait `LowerReturn<UT>`:
<bool as LowerReturn<UT>>
<i8 as LowerReturn<UT>>
<i16 as LowerReturn<UT>>
<i32 as LowerReturn<UT>>
<i64 as LowerReturn<UT>>
<u8 as LowerReturn<UT>>
<u16 as LowerReturn<UT>>
<u32 as LowerReturn<UT>>
and 133 others
error[E0277]: the trait bound `TaskHandle: LowerReturn<_>` is not satisfied
--> bindings/matrix-sdk-ffi/src/room_directory_search.rs:82:1
|
82 | #[uniffi::export(async_runtime = "tokio")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `LowerReturn<_>` is not implemented for `TaskHandle`
|
= help: the following other types implement trait `LowerReturn<UT>`:
<bool as LowerReturn<UT>>
<i8 as LowerReturn<UT>>
<i16 as LowerReturn<UT>>
<i32 as LowerReturn<UT>>
<i64 as LowerReturn<UT>>
<u8 as LowerReturn<UT>>
<u16 as LowerReturn<UT>>
<u32 as LowerReturn<UT>>
and 133 others
```
This PR wraps the offending function in an `Arc<>` to make it uniffi 0.25 compatible,
which unbreaks complement crypto.
`TryFrom` and `TryInto` are imported redundantly. They are already
defined in `core::prelude::rust_2021` which is automatically imported.
This is generating warnings on my side. This patch fixes that.
This patch makes `ChunkIdentifierGenerator::generate_next` to panic
if there is no more identifiers available. It was previously returning
a `Result` but we were doing nothing with this `Result` except
`unwrap`ping it. To simplify the API: let's panic.
As suggested in https://github.com/matrix-org/matrix-rust-sdk/
pull/3251#discussion_r1532103818 by Poljar, it is possible that the
value of the atomic changes between the `fetch_add` and the `load` (if
and only if it is used in a concurrency model, which is not the case
right now, but anyway… better being correct now!). The idea is not
`load` but repeat the addition manually to compute the “current” value.
Imagine we have this linked chunk:
```rust
assert_items_eq!(linked_chunk, ['a'] [-] ['b', 'c'] ['d', 'e', 'f']);
```
Before the patch, when we were running:
```rust
let position_of_d = linked_chunk.item_position(|item| *item == 'd').unwrap();
linked_chunk.insert_gap_at((), position_of_d)?;
```
it was taking `['d', 'e', 'f']` to split it at index 0, so `[]` + `['d',
'e', 'f']`, and then was inserting a gap in between, thus resulting
into:
```rust
assert_items_eq!(linked_chunk, ['a'] [-] ['b', 'c'] [] [-] ['d', 'e', 'f']);
```
The problem is that it creates a useless empty chunk. It's a waste of
space, and rapidly, of computation. When used with `EventCache`, this
problem occurs every time a backpagination occurs (because it executes
a `replace_gap_at` to insert the new item, and then executes a
`insert_gap_at` if the backpagination contains another `prev_batch`
token).
With this patch, the result of the operation is now:
```rust
assert_items_eq!(linked_chunk, ['a'] [-] ['b', 'c'] [-] ['d', 'e', 'f']);
```
The `assert_items_eq!` macro has been updated to be able to assert
empty chunks. The `test_insert_gap_at` has been updated to test all
edge cases.
A couple of small changes that allow us to drop locks that would
otherwise persist when running the tests over the MemoryStore, and some
documentation comments for assertions to make it easier to spot which
assertion failed, even though we are inside a macro.
Backing up room keys isn't part of the outgoing requests processing
loop, instead the user is supposed to have a separate loop calling
`BackupMachine::backup()`.
We don't need the `hyper` feature as we use our own HTTP client,
and the `keystore` will not be used in most cases.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Fallback keys until now have been rotated on the basis that the
homeserver tells us that a fallback key has been used.
Now this leads to various problems if the server tells us too often that
it has been used, i.e. if we receive the same sync response too often.
It leaves us also open to the homeserver being dishonest and never
telling us that the fallback key has been used.
Let's avoid all these problems by just rotating the fallback key in a
time-based manner.
Signed-off-by: Damir Jelić <poljar@termina.org.uk>
Co-authored-by: Andy Balaam <andy.balaam@matrix.org>
Now that there is some support for [MSC2530](https://github.com/matrix-org/matrix-spec-proposals/pull/2530), I gave adding sending captions a try. ( This is my first time with Rust 😄 )
I tried it on Element X with a hardcoded caption and it seems to work well
(It even got forwarded through mautrix-whatsapp and the caption was visible on the Whatsapp side)
---
* ffi: Expose filename and formatted body fields for media captions
In relevance to MSC2530
* MSC2530: added the ability to send media with captions
Signed-off-by: Marco Antonio Alvarez <surakin@gmail.com>
* signoff
Signed-off-by: Marco Antonio Alvarez <surakin@gmail.com>
* fixing the import messup
* fix missing parameters in documentation
* fix formatting
* move optional parameters to the end
* more formatting fixes
* more formatting fixes
* rename url parameter to filename in send_attachment and helpers
* fix send_attachment documentation example
* move caption and formatted_caption into attachmentconfig
* fix formatting
* fix formatting
* fix formatting (hopefully the last one)
* updated stale comments
* simplify attachment message comments
---------
Signed-off-by: Marco Antonio Alvarez <surakin@gmail.com>
Co-authored-by: SpiritCroc <dev@spiritcroc.de>
Previously, in a chunk with items `a`, `b` and `c`, their indices were
2, 1 and 0. It creates a problem when we push new items: all indices
must be shifted to the left inside the same chunk. That's not optimised.
This patch reverses the order, thus now `a` has index 0, `b` has index 1
and `c` has index 2.
It also removes a possible bug where we use `item_index` without
“reversing” it. This is now obvious that we don't need to re-compute the
`item_index`, we can use it directly.
This patch optimises `LinkedChunk::rchunks` and `chunks` by _not_ using
`rchunks_from` and `chunks_from`. Indeed, it's faster to not call the
inner `chunk` method + `unwrap`ping the result and so on. Just a tiny
optimisation.
This patch also uses the new `Chunk::last_item_position` method for
`LinkedChunk::items`. Abstraction for the win!
This patch implements `Chunk::first_item_position` and
`Chunk::last_item_position` as a way to _position_ the
“cursor” (`ItemPosition`) in the correct way when we want to do some
particular insertion (at the beginning or at the end of a chunk).
This patch rewrites `LinkedChunk::replace_gap_at`. Instead of inserting
new items on the _previous_ chunk of the gap and doing all the stuff
here, a new items chunk is created _after_ the gap (where items are
pushed), to finally unlink and remove the gap.
First off, it removes an `unwrap`. Second, if the previous chunk was
an items chunk, and had free space, the items would have been added in
there, which is not the intended behaviour. The tests have been updated
accordingly.
This patch implements `From<ChunkIdentifier>` for `ItemPosition`.
It's useful when we get a `ChunkIdentifier` and we need to `insert_…
_at(item_position)`.
This patch updates `ChunkContent::Gap` to hold a content `U`. Thus,
`Chunk` and LinkedChunk` both get a new generic parameter `U`. Some
methods like `new_gap` or `insert_gap_at` take a new `content: U`
parameter.
This type `RoomEvents` (that uses `LinkedChunk`) is also updated
accordingly.
The small first commit reintroduces `sanitize_server_name` in the public API. In Fractal, we use it to validate the string in the input before allowing the user to trigger the discovery.
The main commit changes a bit the discovery process: before, server names like `example.org` would only be checked for the presence of a well-known, and only URLs like `https://example.org` would be checked as a homeserver. Now, providing `example.org` will also check if `https://example.org` is the URL of a homeserver.
Sadly I don't think it's possible to add tests for it as it would require to have a homeserver accessible via HTTPS.
---
* sdk: Restore sanitize_server_name in the public API
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
* sdk: Check if a provided server name points to a homeserver during discovery
Before, only URLs like `https://example.org` would be checked as a homeserver.
Providing `example.org` will check if `https://example.org` is the URL of a homeserver.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
* Refactor to avoid duplication
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
---------
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Before this patch, the meta field would be mutated, even when the transaction would be aborted. This changes the update scheme to meta
with the following:
- when creating the transaction, clone the meta (but keep the pointer location to the previous one)
- all the transaction's methods operate on the WIP meta
- when committing, replace the previous meta with the current one
This patch is a work-in-progress. It explores an experimental data
structure to store events in an efficient way.
Note: in this comment, I will use the term _store_ to mean _database_
or _storage_.
The biggest constraint is the following: events can be ordered in
multiple ways, either topological order, or sync order. The problem is
that, when syncing events (with `/sync`), or when fetching events (with
`/messages`), we **don't know** how to order the newly received events
compared to the already downloaded events. A reconciliation algorithm
must be written (see #3058). However, from the “storage” point of view,
events must be read, written and re-ordered efficiently.
The simplest approach would be to use an `order_index` for example.
Every time a new event is inserted, it uses the position of the last
event, increments it by one, and done.
However, inserting a new event in _the middle_ of existing events would
shift all events on one side of the insertion point: given `a`, `b`,
`c`, `d`, `e`, `f` with `f` being the most recent event, if `g` needs
to be inserted between `b` and `c`, then `c`, `d`, `e`, `f`'s ordering
positions need to be shifted. That's not optimal at all as it would
imply a lot of updates in the store.
Example of a relational database:
| ordering_index | event |
|----------------|-------|
| 0 | `a` |
| 1 | `b` |
| 2 | `g` |
| 3 | `c` |
| … | … |
An insertion can be O(n), and it can happen more frequently than one
can think of. Let's imagine a permalink to an old message: the user
opens it, a couple of events are fetched (with `/messages`), and these
events must be inserted in the store, thus potentially shifting a lot of
existing events. Another example: Imagine the SDK has a search API for
events; as long as no search result is found, the SDK will back-paginate
until reaching the beginning of the room; every time there is a
back-pagination, a block of events will be inserted: there is more and
more events to shift at each back-pagination.
OK, let's forget the `order_index`. Let's use a linked list then? Each
event has a _link_ to the _previous_ and to the _next_ event.
Inserting an event would be at worst O(3) in this case: if the previous
event exists, it must be updated, if the next event exists, it must be
updated, finally, insert the new event.
Example with a relational database:
| previous | id | event | next |
|----------|---------|-------|---------|
| null | `id(a)` | `a` | `id(b)` |
| `id(a)` | `id(b)` | `b` | `id(c)` |
| `id(b)` | `id(c)` | `c` | null |
This approach ensures a fast _writing_, but a terribly slow _reading_.
Indeed, reading N events require N queries in the store. Events aren't
contiguous in the store, and cannot be ordered by the database engine
(e.g. with `ORDER BY` for SQL-based database). So it really requires one
query per event. That's a no-go.
In the two scenarios above, another problem arises. How to represent
a gap? Indeed, when new events are synced (via `/sync`), sometimes the
response contains a `limited` flag, which means that the results are
_partial_.
Let's take the following example: the store contains `a`, `b`, `c`.
After a long offline period (during which the room has been pretty
active), a sync is started, which provides the following events: `x`,
`y`, `z` + the _limited_ flag. The app is killed and reopened later.
The event cache store will contain `a`, `b`, `c`, `x`, `y`, `z`. How
do we know that there is a hole/a gap between `c` and `x`? This is an
important information! When `z`, `y` and `x` are displayed, and the user
would like to scroll up, the SDK must know that it must back-paginate
before providing `c`, `b` and `a`.
So the data structure we use must also represent gaps. This information
is also crucial for the events reconciliation algorithm.
What about a mix between the two? Here is _Linked Chunk_.
A _linked chunk_ is like a linked list, except that each node is either
a _Gap_ or an _Items_. A _Gap_ contains nothing, it's just a gap. An
_Items_ contains _several_ events. A node is called a _Chunk_. A _chunk_
has a maximum size, which is called a _capacity_. When a chunk is full,
a new chunk is created and linked appropriately. Inside a chunk, an
ordering index is used to order events. At this point, it becomes a
trade-off the find the appropriate chunk size to balance the performance
between reading and writing. Nonetheless, if the chunk size is 50, then
reading events is 50 times more efficient with a linked chunk than with
a linked list, and writing events is at worst O(49), compare to the O(n
- 1) of the ordering index.
Example with a relational database. First table is `events`, second
table is `chunks`.
| chunk id | index | event |
|----------|-------|-------|
| `$0` | 0 | `a` |
| `$0` | 1 | `b` |
| `$0` | 2 | `c` |
| `$0` | 3 | `d` |
| `$2` | 0 | `e` |
| `$2` | 1 | `f` |
| `$2` | 2 | `g` |
| `$2` | 3 | `h` |
| chunk id | type | previous | next |
|----------|-------|----------|------|
| `$0` | items | null | `$1` |
| `$1` | gap | `$0` | `$2` |
| `$2` | items | `$1` | null |
Reading the last chunk consists of reading all events where the
`chunk_id` is `$2` for example, and contains events `e`, `f`, `g` and
`h`. We can sort them easily by using the `event_index` column. The
previous chunk is a gap. The previous chunk contains events `a`, `b`,
`c` and `d`.
Being able to read events by chunk clearly limit the amount of reading
and writing in the store. It is also close to what will be really done
in real life with this store. It also allows to represent gaps. We can
replace a gap by new chunk pretty easily with few writings.
A summary:
| Data structure | Reading | Writing |
|----------------|-------------------|-----------------|
| Ordering index | “O(1)”[^1] (fast) | O(n - 1) (slow) |
| Linked list | O(n) (slow) | O(3) (fast) |
| Linked chunk | O(n / capacity) | O(capacity - 1) |
This patch contains a draft implementation of a linked chunk. It will
strictly only contain the required API for the `EventCache`, understand
it _is not_ designed as a generic data structure type.
[^1]: O(1) because it's simply one query to run; the database engine
does the sorting for us in a very efficient way, particularly if the
`ordering_index` is an unsigned integer.
This adds a new mechanism in the UI crate (since re-attempts to decrypt happen in the timeline, as of today — later that'll happen in the event cache) to notify whenever we run into a UTD (an event couldn't be decrypted) or a late-decryption event (after some time, a UTD could be decrypted).
This new hook will deduplicate pings for the same event (identifying events on their event id), and also implements an optional grace period. If an event was a UTD:
- if it's still a UTD after the grace period, then it's reported with a `None` `time_to_decrypt`,
- if it's not a UTD anymore (i.e. it's been decrypted in the meanwhile), then it's reported with a `time_to_decrypt` set to the time it took to decrypt the event (approximate, since it starts counting from the time the timeline receives it, not the time the SDK fails to decrypt it at first).
It's configurable as an optional hook on timeline builders. For the FFI, it's configurable at the sync service's level with a "delegate", and then the sync service will forward the hook to the timelines it creates, and the hook will forward the UTD info to the delegate.
Part of https://github.com/element-hq/element-meta/issues/2300.
---
* ui: add a new module and trait for subscribing to unable-to-decrypt notifications
and late decryptions (i.e. the key came in after the event that required it for decryption).
* timeline: hook in (!) the unable-to-decrypt hook
* timeline: prefix some test names with test_
* utd hook: delay reporting a UTDs
* ffi: add support for configuring the utd hook
* utd hook: switch strategy, have a single hook
And have the data structure contain extra information about late-decryption events.
* utd hook: rename `SmartUtdHook` to `UtdHookManager`
* ffi: configure the UTD hook with a grace period of 60 seconds
And ignore UTDs that have been late-decrypted in less than 4 seconds.
* utd hook: update documentation and satisfy the clippy gods
* ffi: introduce another UnableToDecryptInfo FFI struct that exposes simplified fields from the SDK's version
* review: introduce type alias for pending utd reports
* review: address other review comments
This adds support for back-pagination into the event cache, supporting enough features for integrating with the timeline (which is going to happen in a separate PR).
The idea is to provide two new primitives:
- one to get (or wait, if we don't have any handy) the latest pagination token received by the sync,
- one to run a single back-pagination, given a token (or not — which will backpaginate from the end of the room's timeline)
The timeline code can then use those two primitives in a loop to replicate the current behavior it has (next PR to be open Soon™).
The representation of events in the store is changed, so that a timeline can have *entries*, which are one of two things:
- either an event, as before
- or a gap, identified by a backpagination token (at the moment)
This allows us to avoid a lot of complexity from the back-pagination code in the timeline, where we'd attach the backpagination token to an event that only had an event_id. We don't have to do this here, and I suppose we could even attach the backpagination token to the next event itself.
This doesn't do reconciliation yet; the plan is to add it as a next step.
It's a bit unclear whether the crypto-store generation counter is doing the right thing
in terms of causing us to reload the OlmMachine. There is a suspicion that things
might be keeping hold of references to the old OlmMachine.
This PR attempts to add the generation number to the logging for any operations that
hold the cross-process lock. It's obviously not bulletproof: for example, it is possible
for the OlmMachine to be replaced without holding the lock; but hopefully this will
at least help us understand what's going on.
Wiremock doesn't allow overwriting of a mock, so if we want to mock
different sync response bodies for the same path we'll have to mount the
mock in a subscobe.
This also removes the need to access some internal OlmMachine state to
get us notified about changed devices.
When all room members are loaded, we do not need an incremental member update. We know that parsing the /members response will only lead to more ambiguous names, not less. And because /members returns the complete list, we can directly use that list as the disambiguation map.
This improves the performance in my emulator from 56s to 9s and on a less performant device from 11mins to 11s (Tested experimentally on Matrix HQ using log statements in element android. If I have time, I will write a proper benchmark tomorrow.
See also https://github.com/matrix-org/matrix-rust-sdk/pull/3184#issuecomment-1986170631 for a more detailed benchmark run.
---
* members: Simplify disambiguation logic
* members: Prevent api misuse for receive_members
* members: Benchmark receive_all_members performance
* sdk: remove unused import
* sdk-base: rename `ApiMisuse` error to `InvalidReceiveMembersParameters`
* benchmarks: extract the member loading benchmark to `room_bench.rs`
* benchmarks: remove wiremock
* sdk-base: fix format
* sdk-base: try fixing tests
* benchmark: Provide some data to the store so the search and disambiguation happen
* benchmark: fix clippy
* benchmark: use a constant for `MEMBERS_IN_ROOM`
* sdk(style): reduce indent in `receive_all_members`
---------
Co-authored-by: Jorge Martín <jorgem@element.io>
Co-authored-by: Benjamin Bouvier <public@benj.me>
The idea of this patch is to explore the possibility to unify the
`all_rooms` list with the `invites` list in `RoomListService`.
Since this is entirely experimental, it's behind a new feature
flag. The feature itself can be configured at runtime by using the
new `SyncServiceBuilder::with_unified_invites_in_room_list` builder
method, or directly with `RoomListService::new_with_unified_invites`
constructor.
This is needed to be able to diff between increases and decreases of power levels ("user Alice was promoted Admin", etc.).
---
* ffi: add `previous` power levels to `OtherState::RoomPowerLevels`
This is needed to be able to diff between increases and decreases of power levels.
* ffi: please clippy
* ffi: inline initialization of `previous` and `users`
Fixes https://github.com/matrix-org/matrix-rust-sdk/issues/3191
Allows support for fetching the unstable_features from `/_matrix/clients/versions`.
Specifically, to be used for checking the state of org.matrix.msc4028 through ffi to the clients.
---
* sdk: fetch unstable_features supported by homeserver
Signed-off-by: hanadi92 <hanadi.tamimi@gmail.com>
* ffi: add can_homeserver_push_encrypted_event_to_device method
Signed-off-by: hanadi92 <hanadi.tamimi@gmail.com>
* fix: use copied instead of dereferencing
Co-authored-by: Benjamin Bouvier <public@benj.me>
Signed-off-by: Hanadi <hanadi.tamimi@gmail.com>
* fix: move can_homeserver_push_encrypted_event_to_device logic to sdk
Signed-off-by: hanadi92 <hanadi.tamimi@gmail.com>
* fix: remove unused unstable features param in client builder
Signed-off-by: hanadi92 <hanadi.tamimi@gmail.com>
* fix: use assert instead of asserteq for bool check
Signed-off-by: hanadi92 <hanadi.tamimi@gmail.com>
* fix: documentation
Signed-off-by: hanadi92 <hanadi.tamimi@gmail.com>
* Apply suggestions from code review
Signed-off-by: Benjamin Bouvier <public@benj.me>
---------
Signed-off-by: hanadi92 <hanadi.tamimi@gmail.com>
Signed-off-by: Hanadi <hanadi.tamimi@gmail.com>
Signed-off-by: Benjamin Bouvier <public@benj.me>
Co-authored-by: Benjamin Bouvier <public@benj.me>
- `Room::build_power_level_changes_from_current()` was replaced by `Room::get_power_levels()`, which now returns an SDK/Ruma `RoomPowerLevels` value containing all the data we need to display these values in UI and not only the customised values.
- `Room::reset_power_levels()` was added to the FFI layer.
It maps user ids to users' power levels.
Also, make sure it just returns an empty map if this info is not available, instead of crashing.
Then use it in the FFI side to output updated data for the `RoomInfo`.
---
* sdk: create new `users_with_power_levels` fn which maps user ids to users' power levels
Also, make sure it just returns an empty map if this info is not available, instead of crashing.
* Update crates/matrix-sdk/src/room/mod.rs
Co-authored-by: Benjamin Bouvier <public@benj.me>
Signed-off-by: Jorge Martin Espinosa <angel.arasthel@gmail.com>
* Improve tests
---------
Signed-off-by: Jorge Martin Espinosa <angel.arasthel@gmail.com>
Co-authored-by: Benjamin Bouvier <public@benj.me>
Having `EventCache::for_room` return an `Option` avoids the need for the error. One caller can
safely unwrap it, and others can log and ignore rooms that have disappeared (like we do in `call_sync_response_handlers`).
This does two things:
- raise the timeout value for the `/members` queries, because they can super slow in Synapse because of nasal demons,
- and aggregate all the fields of `RoomMember` early, so users don't have to call getters to read each field.
---
* room_members: Export plain old data for room members to avoid FFI calls
Signed-off-by: Timo Kösters <timo@koesters.xyz>
* room_member: Respond to review feedback
* room_member: Remove previous RoomMember struct
* ffi: rename ExportedRoomMember to RoomMember
---------
Signed-off-by: Timo Kösters <timo@koesters.xyz>
Co-authored-by: Benjamin Bouvier <public@benj.me>
Changes:
- sdk: Add `get_user_power_level` and `get_suggested_user_role` functions so we don't need to load the whole room member list to know if a user has some power level/role.
- ffi: add an FFI fn for `get_suggested_user_role`.
- ffi: add `user_power_levels` to `RoomInfo`.
The goal of this PR is being able to fetch a user's power level or role almost immediately and avoid having to load and find the user in the room member list, which can be very slow to load (especially in EX Android).
---
* sdk: Add `get_user_power_level` and `get_suggested_user_role` functions to get the power level for a user without loading the room member list.
* ffi: add `suggested_role_for_user` fn, which calls the new `get_suggested_user_role` fn in Room
* sdk: add test
* ffi: add user power level info to `RoomInfo`
* Add changes to changelog
* sdk: Fix docs formatting
* sdk & ffi: use `&UserId` instead of `OwnedUserId`
Also, simplify error mapping.
* sdk: add extra test
* ffi: fix `OwnedUserId` -> `&UserId` conversion
* sdk: Replace `UserId::parse` with `user_id!` macro for literals in tests
* Update crates/matrix-sdk/tests/integration/room/joined.rs
Signed-off-by: Benjamin Bouvier <public@benj.me>
---------
Signed-off-by: Benjamin Bouvier <public@benj.me>
Co-authored-by: Benjamin Bouvier <public@benj.me>
Mostly an optimization that was also revealed by the previous version of the test failing, because it would receive an initial update with empty
events.
This adds a few basic tests for the event cache, notably one for the `add_initial_events`, for something I identified while working on the code, but
that could've caused bad issues:
The `test_add_initial_events` checks that even if we received updates with meaningful events for a room, the room's events are cleared before we add initial events (since
we have no ways to know where to insert the events, in this case). In the future, we can keep this test as a "smoke test" for basic functionality of
the event cache.
We're subscribing to settings updates *after* sending a request to change a setting. In an unlucky scenario, the following sequence of events could happen:
- sending request to change the settings
- response is received
- we set up the receiver to settings updates, but it's too late
The fix would then be to subscribe to the changes *before* we even send the request to update settings.
It means the `wiremock` dependency is not a `dev-dependency` anymore, but an optional `dependency` enabled only if `testing` is enabled.
It seems like a fine tradeoff to me.
To do so, we need to put the weak reference `EventCache -> Client` back into the `EventCache`.
Putting the `sdk::Room` in the `RoomEventCache` will be useful to run room queries like backpagination (aka call `Room::messages()`).
The ClientBuilder already exposes setting the proxy, but sadly the
AuthService doesn't let you configure the ClientBuilder directly (yet).
So logging in with a proxy wasn't supported until now.
There is a race condition in `EventCacheInner::for_room`. Before this
patch, it was possible for 2 concurrent execution to do the following:
* Execution 1 takes the read lock; there is no room in `by_room`, so it
creates the `RoomEventCache`, code is suspended while trying to get
the write lock.
* Execution 2 takes the read lock; there is no room in `by_room`, so it
creates the `RoomEventCache`, code is _not_ suspended because _insert
reason_, the write lock is acquired, and the `RoomEventCache` is saved
and a shallow clone is returned.
* Execution 1 resumes, the write lock is acquired, the `RoomEventCache`
overwrites the one from Execution 2, and a shallow clone is returned.
Now Execution 2 holds a `RoomEventCache` that isn't saved in `by_room`.
It's a ghost! Worst, because the store is cloned in both
`RoomEventCache`, 2 versions will overwrite themselves in the store
constantly.
This patch uses a single write lock, and uses the `BTreeMap::entry`
API instead. Thus, the race condition disappears.
This patch also changes the return type of `for_room` to make it
infallible. It was already the case: only the `Ok` value was returned.
This patch imports the `spawn` function from
`matrix_sdk_common::executor` instead of `tokio`.
`matrix_sdk_common::executor` adds support for WebAssembly.
* event cache: move it to the main SDK crate
* event cache: add requested Debug impl to `RoomEventCacheUpdate`
Somehow the compiler asked for it now...
* event cache: add missing copyright notice to store file
* event cache: use a weak reference to the client internally
This will make it possible to have the `Client` own an `EventCache` without a reference
cycle.
* event cache: move the spawned task to its own function
* event cache: move RwLock from EventCache::inner to the only mutable field inside EventCacheInner
* event cache: have the Client own *the* event cache
The goal is to have a unique EventCache instance overall, that's available from everywhere in
the SDK, notably when creating timelines for rooms.
Because the event cache only owns a weak reference to the client, it means the Client still
can be dropped, In turn, this will close its sender of `RoomUpdates`, which will gracefully
close the task spawned in `EventCache::new` after it's done handling the latest updates.
* event cache: process room updates one at a time
* timeline: use the client-wide event cache instead of spawning one per timeline
This now means that we're passing the "initial events" to the event cache just before initializing
the timeline. As a result, there might be previous events that the event cache saw (coming from
sync), but now we can't decide where to put them; drop previously known events in that case.
* event cache: hey, turns out we don't even need the weak back-link
Keeping it as a separate commit, to make it easier to revert later.
* event cache: remove unused errors
Keeping the error type and results, though, because we might have store errors soonish.
* fixup! event cache: move the spawned task to its own function
* event cache: manually subscribe to the event cache
It was a bad idea to have it enabled by default, since some users may not be interested in all
updates for all rooms (e.g. bots). Instead, we make it so that the event cache must be
explicitly subscribed to, and we do it in two cases:
- in the UI `TimelineBuilder::build` method, because we're interested in updates to the current
room,
- in the `RoomListService`, because we *will* be interested in updates to room derived data (e.g.
unread counts, read receipts, and so on).
This avoids a bit of fiddling when creating the event cache in the client.
This is resilient when a parent Client is forked into a child Client, because the child
`EventCache` share the same subscription as the parent's.
It doesn't make sense to set a thread identifier for the receipt of the latest event: the event might not belong to that thread, and the SDK would need to check that,
since the latest event isn't reachable from the outside world (the reason why `mark_as_read` had been introduced). So let's remove it for now, and
add comments related to threaded receipts.
This slightly changes the API when interacting from the FFI layer:
- instead of `mark_as_unread` and `mark_as_read`, there's now a single method `set_unread_flag(bool)`, which callers may call with true (i.e. unread) or false (i.e. not unread).
- there's a new method `mark_as_read` which sends a read receipt to the latest event in the timeline, using other commits from the same PR,
- forcing a room as read requires calling first `set_unread_flag(false)` then `mark_as_read()`
It's wrong that the first client, used only to determine how to log in and find the user id, try to run the encryption initialization tasks.
In particular, it should not even try to bootstrap the account, as this may send OTKs to the server, which the client will forget about as soon
as it's respawned as a database-backed client.
I have also changed the priorities so that manual updates are preferred.
This means that duplicate updates do not happen if the room was previously unknown.
Signed-off-by: Timo Kösters <timo@koesters.xyz>
Vectors grow in powers of two while reading, which is doubly wasteful:
* causes every byte to be copied in average once
* leaves the vector in average 25% larger than it needs to be, wasting memory.
For example, adding this test to
`crates/matrix-sdk-crypto/src/file_encryption/attachments.rs`:
```
fn encrypt_decrypt_minimize_memory() {
let data = std::iter::repeat("abcdefg").take(10000).collect::<String>();
let mut cursor = Cursor::new(data.clone());
let mut encryptor = AttachmentEncryptor::new(&mut cursor);
let mut encrypted = Vec::new();
encryptor.read_to_end(&mut encrypted).unwrap();
let key = encryptor.finish();
assert_ne!(encrypted.as_slice(), data.as_bytes());
let mut cursor = Cursor::new(encrypted);
let mut decryptor = AttachmentDecryptor::new(&mut cursor, key).unwrap();
let mut decrypted_data = Vec::new();
decryptor.read_to_end(&mut decrypted_data).unwrap();
assert_eq!(
decrypted_data.len(),
decrypted_data.capacity(),
"{} bytes wasted by decrypted_data",
decrypted_data.capacity() - decrypted_data.len()
);
}
```
errors with:
```
assertion `left == right` failed: 61072 bytes wasted by decrypted_data
left: 70000
right: 131072
```
By initially setting this capacity, the vector should be slightly larger
than needed from the start, avoiding both copying and leftover capacity
after decryption is done.
`matrix_sdk_ui::timeline::Message::msgtype` returns `&MessageType`,
and variants of `MessageType` implement `MediaEventContent`, so it is
allowing references here avoids cloning message content.
This didn't show up, and it's quite useful to know for which room we're trying to send a receipt, without having to look up the room based on the
target event id.
This patch rewrites all the integration tests for the notable tags. The
tests were good, but we could merge some together. The names were too
long, they have been shortened.
The workflow inside `Client::receive_sync_response` is a little
bit fragile. When `Client::handle_room_account_data` is called, it
modifies `RoomInfo`. The problem is that a current `RoomInfo` is being
computed before `handle_room_account_data` is called, and saved a
couple lines after, resulting in overwriting the modification made by
`handle_room_account_data`.
This patch ensures that the new `RoomInfo` is saved before
`handle_room_account_data` is called.
Now that `NotableTags` has replaced `RoomNotableTags` entirely, this
patch can rename `NotableTags` to `RoomNotableTags` as it's a better
name for it.
Note that this type is private to `matrix_sdk_base`, so it's fine to
rename it.
The Matrix specification uses the `m.favourite` orthography. Let's use
the same in our code. So `set_is_favorite` becomes `set_is_favourite`.
This patch updates this in various places for the sake of consistency.
The previous patch has introduced the new `NotableTags` type to replace
the `RoomNotableTags`. This patch removes the latter.
This patch keeps the `Room::set_is_favorite` and `::set_is_low_priority`
methods. However, this patch adds the `Room::is_favourite` and
`::is_low_priority` methods, with the consequence of entirely hiding the
notable tags type from the public API.
This patch is the first step to remove the [`RoomNotableTags`] API. The
idea is to compute the notable tags as a single 8-bit unsigned integer
(`u8`) and to store it inside `RoomInfo`. The benefits are numerous:
1. The type is smaller that `RoomNotableTags`,
2. It's part of `RoomInfo` so:
- We don't need an async API to fetch the tags from the store and to
compute the notable tags,
- It can be put in the store,
- The observable/subscribe mechanism is supported by `RoomInfo` behind
observable instead of introducing a new subscribe mechanism.
This patch exposes the 1-to-1 encryption method that is usually used to share a room key with a device. Users might want to send encrypted custom to-device events to a device directly, so let's expose this functionality.
Co-authored-by: Damir Jelić <poljar@termina.org.uk>
We don't want the rest of the method to abort executing because
automatic backup creation failed.
We also move the infallible event handler registration at the top of the
method.
The logic to automatically create a backup uses the following logic:
1. We check if we need to create a backup, this includes a check if a
backup exists on the server.
2. We conclude that we should create a backup, no backup exists on the
server.
3. The method to create the backup checks again if a backup exists, this
was an API change because the method is public and users misused the
method.
4. Finally, we create the backup.
A race exists between the first time we check if a backup exists and the
second time, i.e. step 1 and 3.
It seems that some users create two Client objects which then make this
race a common occurrence. Clients should not do that, but at least we
don't error out too soon anymore if the automatic creation of the backup
fails.
Quick performance improvement on `save_change` for indexeddb.
All the serialization/encryption is now done outside the db transaction
---
* do serialization/encryption before db transaction
* clippy
* add changelog for indexeddb crate
* clean comments
* fix typo
* Review: Fix typo in changelog
* Review: refactor, rename DbOperation to PendingOperation
* Review: rename variant PutKeyVal to Put
* Review: fix doc typo
* Review: rename perfrom_operation to apply
* Review: remove unneeded isEmpty checks
* Review: refactor better API for PendingStoreChanges
* Review: Prefer BTreeMap to HashMap
* Refactor: rename IndexeddbChangesKeyValue
* Refactoring: get the list of affected store from PendingIndexeddbChanges
* cleaning
* Review: Better names and comments
* Review: use filter_map instead of filter then map
fix: Preserve the event of any settings that are changed back to the default level.
sdk: Rename get_room_power_levels (drop the get).
chore: Refactor with more sensible naming.
sdk: Clean up the RoomPowerLevelChanges API.
The IndexMap::remove method has been deprecated, the documentation[1] on
the method tells us that we can replace the usage of it with
IndexMap::swap_remove:
> NOTE: This is equivalent to .swap_remove(key), replacing this entry’s
> position with the last element, and it is deprecated in favor of
> calling that explicitly.
[1]: https://docs.rs/indexmap/2.2.2/indexmap/map/struct.IndexMap.html#method.remove
This patch implements the `category` room list filter. It introduces a
new type: `RoomCategory`, to ensure that “group” and “people” are
mutually exclusives.
This patch implements `Room::direct_targets_length`. It avoids to call
`Room::is_direct` if and only if we don't care about the room's state
and we don't want an async call, and if we don't want to pay the cost of
`Room::direct_targets` which clones the `HashSet` as an alternative way
to get a similar information than `Room::is_direct`.
This patch implements the `all` and `any` filters in `matrix-sdk-ffi`.
The `not` filter cannot be implemented because recursive enum isn't
supported by UniFFI (see https://github.com/mozilla/uniffi-rs/issues/396).
This patch introduces the
`matrix_sdk_ui::room_list_service::filters::Filter` trait alias.
This patch also cleans up a little bit the filters by renaming some
methods for the sake of consistency across all the existing filters.
A decryption failure would have prevented the recording of the session
otherwise. We are mostly interested in the state of the session if a
decryption failure happens.
This patch inlines `SlidingSyncRoom::latest_event` into its unique call
site: `SlidingSyncRoomExt::latest_timeline_item`.
`SlidingSyncRoom::latest_event` is not using any data from
`SlidingSyncRoom`, except the `Client`. So it can easily live somewhere
else. Our goal is to clean up `SlidingSyncRoom` as much as possible, and
to remove any kind of logic from it.
Since my investigation found that it significantly speeds up deletion of
a store on both Firefox and Chromium if you clear() it first, do that in
our migration code.
This patch removes the need to store and to update the
`SlidingSyncRoom::inner` field (of type `v4::SlidingSyncRoom`). Now,
we just need to handle the `prev_batch`, which is the only data we care
about.
Bonus: it reduces the size of the frozen sliding sync room quite much.
This patch removes the `SlidingSyncRoom::is_dm` method. It's not used
anywhere in the SDK, neither by the FFI bindings. Since this code is
still experimental, it's fine to remove public API.
This patch removes the `SlidingSyncRoom::is_initial_response` method.
It's not used anywhere in the SDK, neither by the FFI bindings. Since
this code is still experimental, it's fine to remove public API.
I think the reasoning behind using a RwLock for sync, and notably allowing multiple concurrent `.read()` lock taking was flawed: since there's no way
to identify which subpiece of the store we're reading and writing, there could be two concurrent requests to the write to the same thing at the same
time. To get rid of this possibility, this commit changes the lock to a single access only Mutex lock.
This patch removes the `room_list::Room::has_unread_notifications` and
the `::unread_notifications` methods. Since the previous commits,
the respective `SlidingSyncRoom` methods have been removed too. It's
better to use the `Deref` implementation of `room_list::Room` to
`matrix_sdk::Room` to use the correct methods.
This patch removes the `SlidingSyncRoom::has_unread_notifications`
and the `::unread_notifications` methods. It doesn't hold any relevant
information that `matrix_sdk::Room` can provide.
`SlidingSyncRoom` no longer has an `avatar_url` method.
`room_list::Room` no longer needs to check first in sliding sync then in
`Room` as a fallback.
This patch removes the `room_list::Room::avatar_url` method.
This patch also implements `Deref` for `room_list::Room` to
`matrix_sdk::Room`, to make our lifes easier.
With the previous commit, the avatar is properly synchronized with the
`Room`. The result is that `SlidingSyncRoom` no longer needs to hold
the `avatar_url`.
To update the avatar of a room, one has to look up in the state event.
That's the canonical way to do. For Sliding Sync, it means looking
inside the `required_state` field of the `v4::SlidingSyncRoom`. This
case already works and was tested.
However, a `v4::SlidingSyncRoom` comes with a direct `avatar` field.
It's another way to know the avatar URL of the room. This case was
handled and tested in `matrix_sdk::sliding_sync::SlidingSyncRoom`, but
it was never propagated into the proper sync mechanism. So when the
`avatar` field was set up, `matrix_sdk::sliding_sync::SlidingSyncRoom`
was holding this information, and the `avatar` wasn't defined in the
proper `Room`: `SlidingSyncRoom` has to look up inside the `Room` as
a fallback.
This patch is the first one to fix this “fallback” mechanism.
The `avatar` field of a `v4::SlidingSyncRoom` now triggers an update to
the new `RoomInfo::update_avatar` method (à la `update_name`), via
`process_room_properties`.
It's only used in test code, so it's not worth exposing to the SlidingSyncRoom object (and the Room already has such a timeline function, if needs be).
Makes AmbiguityChange easier to use, especially outside of SyncResponse,
where we might not have the m.room.member event.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
The room sync could be over, and then the timeline items would not contain the updated item yet, if the timeline didn't get a chance to finish its
internal update. Fix this by:
1. limiting the sync time to 10 seconds
2. giving up to one second for the timeline to update internally, by watching its stream of events (we're ignoring stream updates just after this loop)
Adds a `TimelineEventTypeFilter` enum that either returns only events whose event_type is included in a set of allowed event types, or all events but those whose event types are in a list of excluded event types.
Also adds `TimelineEventTypeFilter` so the clients can use it to define those lists of event types, which are then converted to ruma `TimelineEventType` and used for filtering.
---
* matrix-sdk-ui: add `TimelineEventTypeFilter` to filter timeline events by either including only those of some event types or all but the ones that match those event types.
* ffi: add bindings to `TimelineEventTypeFilter` and `FilterTimelineEventType` so we can provide these event types from the FFI clients
* Fix format
* Fix tests
* Fix format again (using nightly toolchain)
* Remove `all_filter_...` functions as there is no right way to support it at the moment and they're just helpers
* Improve tests
* Make `TimelineEventFilterFn` public so it can be used in several layers.
* Make `TimelineEventTypeFilter` a struct in the FFI layer
* Add fns for creating a timeline with cache and event type filters
* Remove dead code
* Fix some review comments
* ffi: create new timeline initialization APIs, modify existing ones.
ui: make `Room::timeline()` return `None` if no timeline exists instead of lazily creating one.
More details:
- Added `init_timeline_with_builder` to `matrix_sdk_ui::room_list_service::Room` so a timeline can be initialized at will given a `TimelineBuilder`.
- Create `is_timeline_initialized()` fns in both the ui and ffi layers to check the status of the timeline.
- Make `matrix_sdk_ui::room_list_service::Room::timeline()` only return a timeline if it's already been initialized.
- Create FFI functions to expose these UI ones.
* Fix tests
* Fix some review comments
* Update bindings/matrix-sdk-ffi/src/room_list.rs
Signed-off-by: Benjamin Bouvier <public@benj.me>
---------
Signed-off-by: Benjamin Bouvier <public@benj.me>
Co-authored-by: Benjamin Bouvier <public@benj.me>
It's started failing with timeouts in multiple PRs, so this PR should help spot where the issues specifically are, since codecov test failures are so hard to reproduce locally.
Podman running unprivileged containers by default, using system folders
can result in permission issues inside the containers.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
This fixes an issue with the sliding sync processing where room account data are not processed when the associated room has no other changes.
---
* sliding_sync : fix room account data not being processed when there is no other changes in the room
* sliding_sync : properly handle room_account_data from extension
* sliding_sync : add test for processing rooms_account_data
* sliding_sync : fix formatting
* Apply suggestions from code review
Co-authored-by: Ivan Enderlin <ivan@mnt.io>
Signed-off-by: ganfra <francois.ganard@gmail.com>
* sliding_sync : avoid cloning room account data events
Co-authored-by: Benjamin Bouvier <public@benj.me>
Signed-off-by: ganfra <francois.ganard@gmail.com>
---------
Signed-off-by: ganfra <francois.ganard@gmail.com>
Co-authored-by: Ivan Enderlin <ivan@mnt.io>
Co-authored-by: Benjamin Bouvier <public@benj.me>
A quick performance improvement to not do the deserialization/decryption inside the transaction.
---
* quick perf inbound_group_sessions_for_backup
* log on deserialize error
* Review: Unneeded use of :? for an object with Display trait
Co-authored-by: Benjamin Bouvier <public@benj.me>
Signed-off-by: Valere <bill.carson@valrsoft.com>
* Doc: fix capitalization
Co-authored-by: Benjamin Bouvier <public@benj.me>
Signed-off-by: Valere <bill.carson@valrsoft.com>
* Review: better naming
---------
Signed-off-by: Valere <bill.carson@valrsoft.com>
Co-authored-by: Benjamin Bouvier <public@benj.me>
This allows us to use the native `SystemCleaner` in Android 13+ which is a bit safer regarding deadlocks than the JNA alternative we'd use otherwise.
Note this breaks compatibility with pure JVM environments, meaning the library can only be used in Android.
Before, to avoid saving a room info if the read receipts `RoomInfo` field hadn't changed, we used explicit reporting to the caller that a field
changed value. This way was error-prone, and there's been bugs twice in this area: one fixed in a previous PR,
and there was another one in the existing implementation (when the pending list shrinked, it wouldn't tell the caller).
This commit changes the implementation so that it we snapshot the previous read receipts, and after processing unread counts, we compare the
new version with the snapshot, relying on `PartialEq` to do the leg work here. This is more future proof and less error prone.
Also `compute_unread_counts` isn't fallible, so doesn't need to return a Result.
This is renamed to `has_up_to_date_read_marker_item`, and inverted in meaning, compared to its previous meaning.
It's simpler to think of it as the presence of the read marker item: if it's not there, then that's the only case where we'd need to worry about adding one.
This patch updates `SecretStorageKey::check_zero_message` to assume
that a missing `iv` and/or `mac` is valid, instead of an error, as the
spec suggests.
The real reason why the test wasn't passing was the same root cause as #3031. The client's room member information was missing, meaning we couldn't compute a push context, so we couldn't compute notifications/mentions either. The fix is in the testing code itself, the sliding sync should request the `$ME` room member state event to work correctly and non-racily.
This was quite handy during development of the client-side computation of read-receipts, to analyze some bugs.
It might be not useful to have it checked in for long, but I would love to make sure it keeps on compiling
until we have a more stable handling of read receipts in general.
The default implementation of `RingBuffer` was setting a capacity of 0.
This was incorrect as it wasn't possible to insert any items. This patch
updates it to take a `NonZeroUsize` so that it's impossible to set a
negative or zero capacity.
We need to make sure we guard against servers downgrading the encryption
settings, or breaking them with unsupported algorithms. It's not *entirely*
clear what the expectation is here, but legacy crypto in matrix-js-sdk blocks
any changes to the settings at all, so we'll follow suit for now.
Without this, we wouldn't ever have the member information for the logged-in user, the first time they log in, if they don't happen to be the
last user sending a message in a room (a case covered by `$LAZY`).
Necessary for the `.m.rule.invite_for_me` rule that should only happen in invited rooms.
Requires to create a `Notification` type that accepts stripped state events. It is simpler than Ruma's type because it lacks fields with data that is made up or redundant.
Tested locally with Fractal.
Fixes#1912.
---
* Upgrade Ruma
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
* base-client: Support notifications in invited rooms
Necessary for the .m.rule.invite_for_me rule that should
only happen in invited rooms.
Requires to create a Notification type that accepts stripped state
events.
It also removes fields with data that is made up or redundant.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
* matrix-sdk-test: Add macros to construct raw sync or stripped state events
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
* client: Add tests for register_notification_handler
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
* Upgrade Ruma
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
* Fix base changelog
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
* Fix methods on Room
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
* Fix FFI
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
* Fix FFI RoomMember
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
* Simplify and_then chain
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
---------
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Before, restoring a timeline from the cache, for which we had a receipt for the last element, would not mark that receipt as "interesting", thus
considering that all the events are new. This is incorrect, and another way to fix that would be to set `best_receipt` to the initial read receipt;
but having the information that we had a change is more useful in general, so the comparison is changed from strict to lax here.
When restoring a timeline from the disk cache, or when we get a timeline from a limited sync, we can have events in common to the two sets, messing
up with the count. In that case, forget about previous events, since we don't try to stitch timelines yet.
This is a workaround because the proxy may send the same event multiple times in a sync timeline. When that happens and we had the read receipt
on the duplicated event, it may cause some events to be counted twice, resulting in invalid counts. This patch fixes it by resetting the count
*every* time we see the matching event.
In existing code the bootstraping is done in different places:
- in `bootstrap_cross_signing_if_needed`
- or in `run_intialization_tasks`
And both are based on the same EncryptionSettings.
`bootstrap_cross_signing_if_needed` was done by `LoginBuilder` but `run_intialization_tasks` by `MatrixAuth`.
I propose to move all that under `run_intialization_tasks` that has already support to do it in background, and to call it in one place only: `MatrixAuth`
Also Fixes https://github.com/matrix-org/matrix-rust-sdk/issues/2763
## Notes
- Some tests have been updated to properly `wait_for_e2ee_initialization_tasks`
- `bootstrap_cross_signing_if_needed` might require re-authentication. Which I suppose was the original reason to have that in a seperate place. But given that the need to re-auth will soon be deprecated for bootstrap (when this [MSC](https://github.com/matrix-org/matrix-spec-proposals/pull/3967) will land); so for now we pass the authentication info to `run_intialization_tasks` if any.
---
* refactor(bootstrap): init cross-signing with other e2e initial task
* fix compilation warning for NoEncryption feature set
* Doc and Formatting: Improve doc and formatting
* Fix missing import with encryption feature flag
Replace `make_store_config` with a pair of funtions `open_stores_with_name` and
`open_state_store`. This allows us to remove a dependency on
`matrix-sdk-base/e2e-encryption`, and generally simplifies things.
Instead of needing three methods, we now only need one: since we use the
crypto crate enum directly, we no longer need to convert between the
crypto crate and main crate enums.
Signed-off-by: Denis Kasak <dkasak@termina.org.uk>
Specifically, this removes the following redundant types in the main
crate:
- enum UserIdentities
- struct OwnUserIdentity
- struct OtherUserIdentity
This is possible because `OwnUserIdentity` and `OtherUserIdentity` are
exactly the same as their crypto crate counterparts, except they also
wrap a `Client`.
As a consequence, the main crate enum `UserIdentities` is isomorphic to
a struct containing:
1. the crypto crate `UserIdentities`
2. a `Client`
So this commit performs that transformation.
Signed-off-by: Denis Kasak <dkasak@termina.org.uk>
Some sliding sync responses may include extension data that will cause an update to the room, but the room itself may not be in the set of
rooms as returned in the top-level `rooms` fields of the response. This may cause missed updates for rooms, since notifiers won't be notified
about those.
This fixes that, by making sure that a `RoomInfo`-only update will cause the room (and the lists that contain it) to be marked as updated.
When using the same pattern of `client.register(); client.login_*` that is used by tests in [our Acter app, strangely showed that more than one device/session had been opened](https://github.com/acterglobal/a3/issues/938).
Turns out, if the server is set up for it, according [to the spec](https://spec.matrix.org/v1.8/client-server-api/#post_matrixclientv3register) registration _is_ already a login and both device id and access token will be returned ... which are then straight-up ignored by the matrix-sdk Client and without any way of setting it from the outside, one _must_ login again, creating a second unnecessary device/session.
This PR adds an integration test case that checks that upon the minimal registration-login-flow only one device/session is found (as is to be expected from the outside), surfacing the error. It also contains a "somewhat fix" (as we need this in the app right now), but as it a) changes the behavior of the current API and b) isn't fully implementing the encryption-bootstrapping-pattern (and thus fails a different test), I'd leave it open to discussion whether that was appropriate way to go.
---
* Test showing registration-login-pattern leads to too many devices
* Fix too-many-devices-bug
* Do not panic on failure to set session, refactor and add docs
* refactor bootstrap crosssinging
* Fixup
* Use new post_login_cross_signing feature from Oidc, too
* unwrap at construction for less verbose code
* remove comment from test
* make new fn pub(crate) to fix build
This uses [`uniffi`](https://mozilla.github.io/uniffi-rs/Overview.html) to build the matrix bindings for native support and wasm-bindgen for web-browser assembly support. Please refer to the specific section to figure out how to build and use the bindings for your platform.
# OpenTelemetry support
The bindings have support for OpenTelemetry, allowing for the upload of traces to
any OpenTelemetry collector. This support is provided through the
allow_verified [label="Share the entire session from the earliest known index.\n\nOk(None)", color=4, fillcolor=3]
allow_limited [label="Share a limited session starting from index i, which is the index we previously shared at.\n\nOk(Some(i))", color=4, fillcolor=3]
refuse_device_key_changed [label="Sender key changed, refuse to share.\n\nErr(KeyForwardDecision::ChangedSenderKey)", color=6, fillcolor=5]
refuse_not_shared [label="Session was never shared with this device, refuse to share.\n\nErr(KeyForwardDecision::OutboundSessionNotShared)", color=6, fillcolor=5]
refuse_untrusted_own_device [label="Our own device, but it is untrusted and we haven't previously shared with it. Refuse to share.\n\nErr(KeyForwardDecision::UntrustedDevice)", color=6, fillcolor=5]
refuse_missing_outbound_session [label="Not our device and haven't previously shared with it. Refuse to share.\n\nErr(KeyForwardDecision::MissingOutboundSession)", color=6, fillcolor=5]
refuse_device_key_changed [label="Sender key changed, refuse to forward.\n\nErr(KeyForwardDecision::ChangedSenderKey)", color=6, fillcolor=5]
refuse_not_shared [label="Session was never shared with this device, refuse to forward.\n\nErr(KeyForwardDecision::OutboundSessionNotShared)", color=6, fillcolor=5]
refuse_untrusted_own_device [label="Our own device, but it is untrusted and we haven't previously shared with it. Refuse to forward.\n\nErr(KeyForwardDecision::UntrustedDevice)", color=6, fillcolor=5]
refuse_missing_outbound_session [label="Not our device and haven't previously shared with it. Refuse to forward.\n\nErr(KeyForwardDecision::MissingOutboundSession)", color=6, fillcolor=5]
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Doug
