… and specify a version such that publishing of the ui crate becomes possible.
We still use a git dependency for FFI crate builds because there were some
likely important breaking changes that haven't been released.
This is required to publish `matrix-sdk-ui`.
In migrate_data_for_v6, we incorrectly copied the keys in inbound_group_sessions verbatim into inbound_group_sessions2. What we should have done is re-encrypt them using the new table name, so we fix that up with a new migration here.
This caused the bug because we were looking for sessions to mark as backed up by calculating their key (from room_id and session_id) but that key did not exist, because the old sessions were stored under the incorrect keys. So no sessions were marked as backed up, and we repeatedly tried to re-mark them.
Before this patch, we needed to clone the inner `timeline_queue` and turn it into a concrete `Vec<SyncTimelineEvent>`, just to iterate on the elements,
and because returning an iterator from a trait method is impractical. This now changes it to return the actual concrete type of `timeline_queue`, so
we don't need the extra allocations.
Ideally, matrix-sdk and matrix-sdk-base would be merged, so we don't need to use a trait at all here.
* Bring back original (slower) build style under the `sequentially` flag to work around new build style hanging on older machines
* 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>
The room_info variable in this function contains the latest_event, and later it's set as the Room's room info (`inner.inner`) field.
Calling `set_latest_event` manually here will cause an update of the `RoomInfo` subscriber, while we're not done processing the full
room, and a room info update will happen anyways later (when the entire room is processed), so this one is spurious and will only
show a partial update of the fields.
- We can look up the session meta only once, it's not useful to do it once per event as it's
loop-invariant.
- We can look at the events backwards and stop after the first room membership event we see
in that order.
* maybe trigger backup at end of sync
* add test for backup upload
* missing e2e cfg
* add sliding sync tests
* fix borrow
* fix indent
* fix naming from copy/paste
* Remove extracted function
* fix clippy
* style: inline a few variables only used once into their use sites
---------
Co-authored-by: Benjamin Bouvier <public@benj.me>
This patch improves `test_media_content` to ensure that, in case of
multiple medias, only the expected ones are removed. Previously, the
test wasn't testing _other_ medias that should be kept in case of
removals.
This patch continues to improve `test_media_content` to ensure that the
content of the media are the expected ones.
Finally, this patch updates the `MemoryStore` implementation to make
tests happy.
This patch rewrites `MemoryStore::add_media_content`,
`::get_media_content`, `::remove_media_content` and
`::remove_media_content_for_uri` to (i) work on `mxc://` URI instead
of “unique key”, and (ii) to handle removal correctly thanks to the new
`RingBuffer::remove` method.
If `use_cache` is true and the cache exists, let's return everything in
one go instead of declaring a `content` variable. It makes the code
easier to read and to understand.
This is a rather over simplistic media cache implementation for the
`MemoryStore`.
It's based on a `RingBuffer` of size 20.
`remove_media_content` pops all medias until the correct one is met (if
it exists). `remove_media_content_for_uri` removes all medias, it
ignores the URI.
A `From` implementation is part of the public API
and this conversion does not make sense outside of the module.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
This patch updates `Timeline::send_image` and `Timeline::send_video` so
that `thumbnail_url` is now an `Option<String>`.
The idea is to allow sending an image or a video without a thumbnail.
This patch removes `RUNTIME.block_on` inside `Client::get_media_file`,
`::upload_media`, `::get_media_content` and `::get_media_thumbnail`, and
makes those methods async.
Useful if we want to know in what room
we just sent a VerificationRequest,
with UserIdentity::request_verification
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
… and fix build swift errors.
- The XCFramework was being built with dylibs which aren't supported on iOS
- The tests build was attempting to generate uniffi from a moved file
`notifications` were stored in the `StateChanges` struct, which made me think that they're then persisted in the database. It's not the case, they
were just stored there by convenience. This commit changes it to a parameter that's passed, as it's not too invasive and clearer that this is only
transient data.
`itertools::format` is known to cause issues when its display implementation is being
used multiple times, as it consumes the iterator it was given (and that can only happen once,
unless caching it). This is bad, as our production apps may have multiple subscribers they will
run into a panic.
The fix is to reify the debug string before it's logged, so the tracing consumer will only see
a string, and not the display implementation that would panic on the second use.
Currently, querying for inbound group sessions which need backing up is very
inefficient: we have to search through the whole list.
Here, we change the way they are stored so that we can maintain an index of the
ones that need a backup.
Fixes: https://github.com/vector-im/element-web/issues/26488
Fixes: https://github.com/matrix-org/matrix-rust-sdk/issues/2877
---
* indexeddb: Update storage for inbound_group_sessions
Currently, querying for inbound group sessions which need backing up is very
inefficient: we have to search through the whole list.
Here, we change the way they are stored so that we can maintain an index of the
ones that need a backup.
* Rename functions for clarity
* Remove spurious log line
This was a bit verbose
* Rename constants for i_g_s store names
* improve log messages
* add a warning
* Rename `InboundGroupSessionIndexedDbObject.data`
* formatting
Otherwise the matrix-sdk crate can be used with an older version of
ruma-client-api which uses different types for a sliding sync type.
The sliding sync breaking change was allowed to be part of a patch
release because sliding sync is an unstable feature in Ruma.
Somehow `fmt::pretty` manages to be both overcomplicated and not flexible
enough.
The driver here is that I want to put the event "fields" on a separate line to
the message.
This PR fixes the `EventTimelineItem.is_editable()` function for polls.
Before this changes it always returned false.
Now it consider poll's preconditions for editing:
- The poll has no votes yet
- The poll hasn't an end event
Sometimes, we called this `keys query`, sometimes `/keys/query`, and sometimes,
just for variety, `keys/query`. Generally in Matrix we talk about `/keys/query`
so let's standardise on that.
Until https://github.com/matrix-org/matrix-rust-sdk/pull/2862,
`GroupSessionManager::encrypt_session_for` called `Device::encrypt` (via
`Device::maybe_encrypt_room_key`.
`Device::encrypt` is a thin wrapper for `ReadOnlyDevice::encrypt`, but it also
has an `instrument` annotation.
https://github.com/matrix-org/matrix-rust-sdk/pull/2862 short-cuts
`Device::encrypt` and calls `ReadOnlyDevice::encrypt` directly: that was
functionally fine but of course means that we no longer benefit from the
`instrument` annotation.
This PR rectifies the situation by pushing the annotation down to
`ReadOnlyDevice::encrypt`. It also adds some documentation for that function,
since we are using it in more places now.
(Longer-term, I think we should probably aim to get rid of `Device::encrypt`
altogether, but that's a refactor I don't want to take on today.)
This is useful if we ever decide to switch to X3DH for the session
establishment. It also refactors a bit the /keys/claim response handling
method.
Co-authored-by: Jonas Platte <jplatte@matrix.org>
The mocks in some notification tests mock any PUT/DELETE request without
the request to hit a certain path. These mocks then might respond to
unintended new requests the client might make.
The tests also assert a bunch of things manually instead of using
expectations when building the mock and calling server.verify().
The meaning of "null" and "undefined" were conflated by Ruma, previously. This updates to the latest Ruma, which changed the `avatar` type from
`Option` to `JsOption` and thus allowed us to distinguish both cases: `null` means the avatar has been unset, `undefined` means it's not changed
since the previous request.
Encryption doesn't usually require access to the user identity, so we can skip loading that from the store.
Unfortunately that means a fair bit of refectoring, in the form of replacing Device with ReadOnlyDevice.
This reduces the time taken to encrypt a message in a large room from about 3 seconds to 1 second.
This is a deprecated flag for element call using livekit. It was used to enable/disable matrix signalling event encryption. This is not relevant for embedded mode at all since there the hosting client is taking care of encryption.
---
* Remove E2EEenabled flag from the rust sdk.
This is a deprecated flag for element call using livekit. It is replaced
by the `password` and the `perParticipantE2EE` flag.
If `perParticipantE2EE` is set to `false`
and there is now password encryption is disabled implicitly.
Signed-off-by: Timo K <toger5@hotmail.de>
* `cargo +nightly fmt`
Signed-off-by: Timo K <toger5@hotmail.de>
* change test based on review
Signed-off-by: Timo K <toger5@hotmail.de>
* Apply suggestions from code review
---------
Signed-off-by: Timo K <toger5@hotmail.de>
Co-authored-by: Benjamin Bouvier <public@benj.me>
Somehow `fmt::pretty` manages to be both overcomplicated and not flexible
enough.
The driver here is that I want to put the event "fields" on a separate line to
the message.
Two sets of improvements to `get_missing_sessions` here:
* Currently, for any users who have an empty device list, we call `KeyQueryManager::wait_if_user_key_query_pending`, which waits up to 5 seconds for a `/keys/query` response. (Arguably, we should be waiting longer: it is not unusual for such a request to take a while.)
The problem is that that user's server may have been blacklisted, in which case we won't even be trying to do `/keys/query` resquests for that user, so we'll be waiting for something that never happens.
To fix this, let's check the failures list when deciding if we should wait for a user's devices.
* Separately, but closely related: for each user thus affected, we do the wait *in series*. This is a bit silly: there is no point waiting 50 times. We can parallelise the work.
Fixes#2793.
---
* Move `get_user_devices` to `IdentityManager`
... since it's going to need to interact with the `FailuresCache` which is
stored there.
* `get_user_devices_for_encryption`: don't wait for failed servers
* `get_missing_sessions`: wait for users in parallel
Rather than waiting for each pending user in series, do all the waits in
parallel.
* Changelog
* Update crates/matrix-sdk-crypto/src/identities/manager.rs
* Address review comments
Includes the following bugfixes:
- Fix the name of the fallback text field for extensible events in
`RoomMessageEventContentWithoutRelation::make_reply_to_raw()`
- Allow to deserialize `(New)ConditionalPushRule` with a missing `conditions`
field
- Fix deserialization of `claim_keys` responses without a `failures` field
It doesn't matter at the moment as the only test using `test_json::MEMBERS`
does not rely on the event being valid, but it shows this error
nonetheless:
```
2023-11-10T08:54:29.920782Z DEBUG receive_members{room_id="!hIMjEx205EXNyjVPCV:localhost"}: matrix_sdk_base::client: Failed to deserialize member event: missing field `room_id` at line 1 column 297 event_id="$151800140517rfvjc:localhost"
```
and https://spec.matrix.org/v1.8/client-server-api/#get_matrixclientv3roomsroomidmembers
says it is a required key.
- Log URI instead of separate homeserver, path
- Always log query parameters (not just for sliding sync)
- Only log request size for requests that could have a body based on the
HTTP verb
Otherwise, it's possible for the `NotificationClient` to be destroyed *after* the ffi `Client`, and then the hack introduced in the previous commit won't work.
A detached task was spawned to react upon session changes, and that task captured a clone of the current `Client`.
This caused a leak of the `Client`, because that task would never get aborted, and would not stop by itself.
The fix here consists in having `Client::set_delegate` return a task handle that needs to be stashed by the FFI
users, and cancelled when the Client gets out of scope. This fixes the leak, by removing the last reference onto
the Client.
Then, when dropping the Client, we have to drop the Stores in it. These stores may be sqlite-based stores, which
make use of deadpool. Deadpool has a sync wrapper that will call `block_on` in a `drop` method, and as such it
requires to be in the scope of a tokio runtime to run properly. To avoid breaking all abstractions and giving
access to the inners of the `Client`, the hack used here to properly be in a runtime when dropping the stores is
to replace the inner sdk `Client` in the FFI `Client::drop` method (and replace it with a dummy client that is
minimally configured and will use in-memory stores).
RoomInfo is often passed around by value, including in futures where
holding types with a large stack size is especially problematic¹.
It might make sense to move the actual data of (Base)RoomInfo into
an inner struct that is always held inside a Box or Arc, but this change
should have most of the benefits of that while being a bit simpler.
¹ https://github.com/rust-lang/rust/issues/69826
* crypto(fix): don't hold the cache lock while waiting on a user key query
Fixes#2802. The lock was only useful to sync the database and the in-memory cache for the users awaiting a key query request.
So it's possible to slightly tweak the API by moving the method from `SyncedKeyQueryManager` to non-synced `KeyQueryManager`, and require a
`StoreCacheGuard` (i.e. the owned lock, so we can manually drop it when we feel like so).
I've looked at all the other methods, and they do require the cache for writing into it and the store.
At the limit we could also move `SyncedKeyQueryManager::users_for_key_query`
into `KeyQueryManager`, but the lock in there is hold for a very short-time, so it shouldn't be an issue.
* Add test for the key query deadlock while waiting for the response.
* Update crates/matrix-sdk-crypto/src/machine.rs
Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
`Account::create_outbound_session` no longer takes an entire list of keys;
rather it takes a single key and it is up to the caller to pick a key out of
the list.
This in turn means that `SessionCreationError` loses one of its reason codes.
* Add `perParticipantE2EE` to element call url
* add ffi
* nightly fmt
* refactor to use enum + test
* rename to PerParticipantKeys
* cleanup (spelling + formatting)
Signed-off-by: Timo K <toger5@hotmail.de>
The event_type passed to `encrypt_room_event_raw` must be the one of the cleartext event, not `m.room.encrypted`. The returned event has the expected type.
In the previous situation, running the tests with `cargo test` would sometimes fail because despite appending the number of milliseconds since
the start of epoch to the user names, some user names would clash across different tests, leading to unexpected results. This fixes it by using
an actual RNG in there, so the names don't ever clash.
Instead make futures modules part of the API. It is extremely uncommon
to actually refer to a type that implements `Future` by its name / path,
so it's better if these don't show up in documentation pages that are
already large and somewhat cluttered.
There was a bug previously, that the cross-signing bootstrapping could include a signature for a device key that hadn't been uploaded yet.
This fixes it by returning a third (!) request in `bootstrap_cross_signing`, that must be sent as the second in order, and will upload
keys, if required.
Also tweaks documentation. Fixes#2749.
The store cache can be filled lazily when it's actually needed. It's not needed in this
function here, and may be needed in another function the caller of this function may call
later. No need to preemptively fill the cache here.
These functions are a bit confusing, so I wrote some comments. I also factored out
`get_user_signing_key_from_response`, to reduce duplication and (IMHO) improve clarity.
Hashes computed by `compute_session_hash` may be stored in the crypto store, and may or may not
be encrypted; make them slightly more secure by using a more secure hash. Also use sha2 for
the logged hashes that are useful for debugging OIDC issues.
… allows the FFI type MessageLikeEventContent to hold custom msgtypes,
which is useful for notification.
Also allows sending custom msgtypes, as long as no fields other than the
msgtype itself and body are needed.
This PR adds a ffi binding for sending voice messages in the legacy format (before extensible events). It also makes minor changes in the `matrix-sdk` crate to accomodate this change.
* Add send_voice_message api
* Update ruma-events dependency
* Fix attachment info mapping
* Remove unstable dependency in attachment.rs
* Bump ruma-events
* Fix matrix-sdk Cargo.toml
* Fix formatting issues
* Refactor Voice case
* Remove clone from AttachmentInfo
* Remove duplicate code
* Remove unused imports
* Fix formatting issue
* Rename update function
This includes the requests that are sent to the matrix client (i.e.
a request to send a matrix event) as well as requests that are sent to
the widget.
The difference between this and `Action` is that `Action` is rather
"low-level" (i.e. it has a generic `SendToWidget(String)` variant),
whereas this API is a high-level API that we will use internally to
conveniently issue and `await` for outgoing responses.
This code path could never be taken:
- the only way to set the cross_process_token_refresh_manager is from calling the same function
- this function is guarded against a lock, so it's not reentrant
- this function's only internally called, during `restore_session` or after a successful login:
both will set a session, hence those functions would fail beforehands if another session had
been set earlier.
#2587 introduced a breaking change in the serialized format of `RoomInfo`, changing the `event`
field format in particular, without a migration. As a result, this lead to users state stores
being invalidated for containing incorrect data, and thus being logged out.
This mitigates the issue by making sure that the LatestEvent type is either deserialized as its
newer format (aka `SerializedLatestEvent`) or its older format (aka `SyncTimelineEvent`). This
way, we maintain compatibility with the previous format. We always serialize to the new one, so
we'd only run this migration once.
- `content` is now of type `RoomMessageEventContentWithoutRelation`,
the relation was previously always overwritten if set
- `add_mentions` is now inferred from `content.mentions`
This adds a new `StoreTransaction` type, that wraps a `StoreCache` and a `Store`. The idea is that it will allow write access to the `Store` (and maintains the cache at the same time), while the `Store::cache` method will only give read-only access to the store's content.
Another new data type is introduced, `PendingChanges`, that reflects `Changes` but for fields that are properly maintained in the `StoreCache` and that one can write in the `StoreTransaction`. In the future, it wouldn't be possible to use `save_pending_changes` from the outside of a `StoreTransaction` context.
The layering is the following:
- `Store` wraps the `DynCryptoStore`, contains a reference to a `StoreCache`.
- When read-only access is sufficient, one can get a handle to the cache with `Store::cache()`.
- When a write happens, then one can create a `StoreTransaction` (later, only one at a time will be allowed, by putting the `StoreCache` behind a `RwLock`; this has been deferred to not make the PR grow too much).
- Any field in the `StoreCache` will get a method to get a reference to the cached thing: it will either load from the DB if not cached, or return the previously cached value.
- Any field that can be written to will get a method to get a mutable reference in the `StoreTransaction`: it will either load from the cache into a `PendingChanges` scratch pad, or return the scratchpad temporary value.
- When a `StoreTransaction::commit()` happens, fields are backpropagated into the DB *and* the cache.
Then, this `StoreTransaction` is used to update a `ReadOnlyAccount` in multiple places (and usage of `ReadOnlyAccount` is minimized so as not to require a transaction or cache function call as much as possible). With this, the read-only account only exists transiently, and it's only stored long-term in the cache.
Followup PRs include:
- making the `ReadOnlyAccount` not cloneable
- remove inner mutability from the `ReadOnlyAccount`
- add a `RwLock` on the `StoreTransaction`
Part of https://github.com/matrix-org/matrix-rust-sdk/issues/2624 + https://github.com/matrix-org/matrix-rust-sdk/issues/2000.
---
* crypto: Replace some uses of `ReadOnlyAccount` with `StaticAccountData` and identify tests
* crypto: introduce `StoreTransaction` to modify a `ReadOnlyAccount`
* crypto: introduce `save_pending_changes`, aka `save_changes` v2
* crypto: Start using `StoreTransaction`s to save the account, get rid of `Store::save_account` + `Account::save`
* crypto: use `StoreTransaction` to save an account in `keys_for_upload`
* crypto: use `StoreTransaction` and the cache in more places
* crypto: remove `Account` from the `Changes` \o/
* crypto: remove last (test-only) callers of `Store::account()`
* crypto: move `ReadOnlyAccount` inside the cache only
* crypto: use `ReadOnlyAccount` and `Account` in fewer places
whenever we can use `StaticAccountData` in place.
* crypto: make tests rely less on OlmMachine
* crypto: Don't put the `ReadOnlyAccount` behind a RwLock just yet
+ clippy
Stores may race when performing writes, since in `save_changes` some data is pickled, and live across await points (it could be stale after an
await point). To prevent multiple threads racing when calling into `save_changes`, a new lock is introduced there.
* support creating emotes
adds an emote param to message_event_content_from_markdown and
message_event_content_from_html so that clients can create
m.room.message events with msgtype emote (on the assumption
that the msgtype returned in the RoomMessageEventContentWithoutRelation
is immutable, and so can't be overridden by the client)
* lint
* switch to separate methods per review feedback
This patch adds a `sender_profile: Option<MinimalRoomMemberEvent>`
field. Thus, `cache_latest_events` now receives an
`Option<&StateChanges>` and an `Option<&Store>`. The `StateChanges`
is used to read the most recent sender profile, otherwise the sender
profile will be fetch from the storage.
This patch adds a new `LatestEvent` type. It wraps the
`SyncTimelineEvent` that was used before. The idea is to add more fields
onto this `LatestEvent` struct, but this patch starts easy by using
`LatestEvent` everywhere where it's required.
This patch updates `all_rooms` to require the `m.room.member` state set
to `$LAZY`. That way, it's more likely to get the `m.room.member` event
associated to the latest event without needing to sync all the members.
This is to enable entirely local stacks of Element X to work correctly. It's mostly seamless (no ffi changes) because it ties into `ClientBuilder.insecure_server_name_no_tls()`.
---------
Co-authored-by: Benjamin Bouvier <public@benj.me>
Before this commit, a call to get_notification would fail if decrypting failed on the first attempt, because `Room::decrypt_event` will hard-fail if the key wasn't found.
Sorry, this commit is a bit big. What happened is that I've pulled a thread: put a `StaticAccountData` there, look at caller; it seems to use only a
static account too, so keep up.
The only contender was the `OwnUserIdentity` data structure which really wants to sign things. Lucky for us, we could pass it a `ReadOnlyAccount`
from a store, in those cases, since we always had a `Store` hanging around; in the future it'll read it from the store cache, which is somewhat
identical.
We might need to rely on the data in the DB to already be corrected
before using it for another migration
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
- Bumps ruma to include https://github.com/ruma/ruma/pull/1665
- Enables ruma `compat-arbitrary-length-ids` flag instead of using the now deprecated `lax-id-validation`
Using a static lock would prevent two Client instances to call the
method at the same time.
We're going to assume that people won't have multiple Client instances
for the same user, in which case a static lock would have been helpful.
Co-authored-by: Jonas Platte <jplatte@matrix.org>
Instead of using three separate object stores, use a single one with some structured objects and indexes.
Fixes#2605 (or at least, should make whatever is going wrong much more obvious).
Consists of a series of commits which should be reviewable on their own.
Implement a `tracing_subsciber` `Layer` which sends output to the javascript
console.
Obviously, this only works on the wasm32 target.
This is lifted from `matrix-rust-sdk-crypto-wasm`, so that we can use it in
tests etc for other crates.
Fix a bug which caused `get_outgoing_secret_requests` to return no data.
This is exposed as a public function, and when called that way it does *not*
expect the key to be escaped and encrypted.
An encryption state request was failing, but before the patch, subsequent
requests would end up in success, while this wasn't the case. The new failure
introduced by this patch was a real one, because the encryption state was
not mocked as part of this test (which tries to send a message to a homeserver).
Ruma used to handle this during deserialization when the field was
an Option.
Especially meaningful when computing the room name, where
an empty string means the name is not set.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Adds a dummy fuield to `UnstableVoiceContent`, otherwise uniffi
will generate an empty Kotlin data class which breaks compilation
(Kotlin data classes must have at least 1 field).
Upstream bug: https://github.com/mozilla/uniffi-rs/issues/1760
Without this feature:
```sh
$ cargo check -p matrix-sdk --features experimental-sliding-sync
Checking matrix-sdk-base v0.6.1 (/Users/hwhost/Development/Element/matrix-rust-sdk/crates/matrix-sdk-base)
error[E0433]: failed to resolve: could not find `poll` in `events`
--> crates/matrix-sdk-base/src/latest_event.rs:7:5
|
7 | poll::unstable_start::SyncUnstablePollStartEvent, room::message::SyncRoomMessageEvent,
| ^^^^ could not find `poll` in `events`
error[E0599]: no variant or associated item named `UnstablePollStart` found for enum `AnySyncMessageLikeEvent` in the current scope
--> crates/matrix-sdk-base/src/latest_event.rs:40:68
|
40 | AnySyncTimelineEvent::MessageLike(AnySyncMessageLikeEvent::UnstablePollStart(poll)) => {
| ^^^^^^^^^^^^^^^^^ variant or associated item not found in `AnySyncMessageLikeEvent`
```
With the feature, everything goes well.
This patch removes `SlidingSyncList::room_list_filtered_stream.
Of course, the only place where it was used,
`RoomList::entries_with_dynamic_adapters` now use `.filter` from
`VectorSubscriberExt`. It's basically less code.
This patch adds `Option<ImageInfo>` as an argument of
`Room::upload_avatar`.
To achieve that, this patch implements
`TryFrom<ImageInfo> for ruma::events::room::avatar::ImageInfo`.
The `TimelineError` type only contains error variants used for
`ImageInfo`, `AudioInfo`, `VideoInfo`, `FileInfo` and so on. It's never
used for something strictly related to the `Timeline`.
This patch then renames `TimelineError` to `MediaInfoError`.
The `MissingMediaInfoField` variant becomes `MediaField`. The
`InvalidMediaInfoField` becomes `InvalidField`.
Use `SharedObservable::set_if_not_eq` instead of `::set`, so that it
doesn't update the limit to the same value, which would be unnecesary in
this case.
This patch removes the `Mutex` around `Subscriber<Option<u32>>` inside
the `RoomListDynamicEntriesController`. The `Mutex` was necessary to
get a mutable reference, so that `Subscriber::next_now` could have been
used. However, it's not necessary to use `new_now` in this particular
context. We can use `get` instead, which take an immutable reference,
thus removing the need for the `Mutex`.
A `Mutex` has a non-negligeable cost. This function can be used in a
critical hot path, and must as fast as possible.
This patch updates `chunk_large_query_over`. This function works great
when it's required to chunk some data over a query. However, if the
data don't need to be chunked, it is possible to get a quicker path that
doesn't involve 2 `Vec` allocations, nor a `split_off` of one `Vec`, nor
a `Vec::extend` (which move data in memory). The quicker path, which is
also the most likely to be hit, simply does a single `Vec` allocation,
and that's it.
* chore(oidc): put impl of OIDC server behind a trait and add tests for the OIDC flow
chore(oidc): add first tests for login/AuthorizationResponse
chore(oidc): add tests for finish_authorization/finish_login/refresh_access_token
chore(oidc): add test for logout
chore(🤷): clippy/fmt
* chore(oidc): move account management test to the new `tests` module
* chore(oidc): address review comments
Prior to this patch, we were using 2 constants to define the
sync indicator delays: `SYNC_INDICATOR_DELAY_BEFORE_SHOWING` and
`SYNC_INDICATOR_DELAY_BEFORE_HIDING`. After some discussions with
some users, it appears that it's desirable to make these values
parameterizable.
Thus, this patch updates `RoomListService::sync_indicator` to accept 2
parameters: `delay_before_showing` and `delay_before_hiding`. The patch
also updates the FFI bindings.
In `RoomList::entries_with_dynamic_adapters`, we were using the
`Limit::dynamic` constructor. It builds a `Limit` stream adapter where
the limits come from a `Stream`. The immediate impact is that this
constructor does only return a new `Stream`, but it cannot return
the initial items of the vector. Thus, it wasn't possible to reset
the final `Stream` with a `VectorDiff::Reset { values }`. Instead, we
were emitting a `Clear` (from this code) then a `Append` (from the
`Limit` stream). Sadly, for some client apps, like Element X iOS, these
2 diffs (`Clear` and `Append`) result in a “rendering blink”.
Hopefully for us, now there is new constructors for `Limit`! One of them
is `Limit::dynamic_with_initial_limit`, which returns the initial items
along with the new `Stream`. That's perfect, we can reset the final
`Stream` with `Reset { values }` again, thus removing the “UI blinking”
effect in Element X iOS.
This patch switches `Limit::dynamic` to
`Limit::dynamic_with_initial_limit`, and updates/simplifies the tests
accordingly.
`RoomListService::new_internal` were creating the `invites` sliding
sync list, with a cache. It is not a good idea. It should not be cached.
Let's remove that :-).
`RoomListLoadingState` was initialized with the `NotLoaded` state. This
is always true… except when there is a cache! If the cache contains
N rooms, the loading state should be `Loaded { … }`. Then the next
sync (if any) will update the loading state to `Loaded { … }` with
an adjusted number of rooms or something like that, but semantically
speaking, the presence of the cache should result in a `Loaded` state.
This patch fixes this behavior, and also adds the tests.
For some room lists, the number of entries can be gigantic. For example,
some accounts have 800, 2500, or even 4000 rooms! It's not necessary for
the client/app to display all the 4000 rooms. First, it can create some
performance issues, second, nobody will scroll 4000 rooms to search for
a particular room :-). Such users are more likely to use a search bar or
something equivalent. The idea is that `RoomListService` will continue
to sync all the data, but only a _limited_ version of it will be shared
to the client/app.
This patch takes `RoomList::entries_with_dynamic_filter`, and improves
it to include this (dynamic) limit.
This patch renames `RoomList::entries_with_dynamic_filter`
to `::entries_with_dynamic_adapters`. It now returns a
`RoomListDynamicEntriesController`, which is a renaming of
`DynamicRoomListFilter`. Basically, the “dynamic filter” becomes a
“dynamic controller” because `RoomList::entries_with_dynamic_adapters`
manages more than a filter. It now uses
`eyeball_im_util::vector::DynamicLimit` to dynamically limit the size
of entries. And that's the major idea behind this patch.
`RoomListDynamicEntriesController::set` is renamed `::set_filter`, and 2
new methods are introduced: `add_one_page` and `reset_to_one_page`.
A _page_ is like a chunk of room entries we want to view or add. When
doing `next_page`, the limit increases to `old_limit + page_size`. The
`reset_pages` method resets the `limit` to `page_size` only.
This adds a cross-process lock for refresh to work correctly.
We want to coordinate token refresh across multiple processes. For that, we're using a cross-process lock, and a value in the database identifying the latest session tokens that are valid (a hash of the actual tokens, for security reasons).
Whenever we run into an HTTP error indicating that the tokens have been invalidated, we try to refresh the access tokens; that's already existing prior to this PR. The novelty introduced is that we take a cross-process lock before doing so, now. Taking this lock will also load a session hash from the database, and we'll compare it against the latest "known" session hash (that the current process saved into its memory).
If there's no mismatch (i.e. the database and the currently known are the same), then we're all good and can keep going with the refresh, synchronize the hashes everywhere (in-memory and database), make sure the client is notified about it (through a new user-provided callback `SaveSessionCallback`; on iOS this will save it into the device's keychain).
Otherwise, that means another process has done a refresh under our feet. In that case, we ask an authoritative source for trusted session tokens. On iOS, they're reloaded from the device keychain; that happens through a new user-provided callback `ReloadSessionCallback`. Then, we make sure that the DB and the in-memory value recall this latest value.
An embedder who would like to make use of the cross-process locking mechanism should call `client.oidc().set_session_callbacks` and `client.oidc().enable_cross_process_refresh`. If only interested with the pings for new sessions, the client may only call `client.oidc().set_session_callbacks`.
Fixes https://github.com/matrix-org/matrix-rust-sdk/issues/2418.
Fixes https://github.com/matrix-org/matrix-rust-sdk/issues/2476
## Future improvements
- More testing of the whole flow. Not sure if mocking will be quite fit for OIDC, as this may require setting up an HTTPS server for the authentication code exchange and other OIDC-specific flows.
- Get rid of `SessionChange`, which duplicates in some way how a client can be notified about session changes.
---
* chore: replace manual StateMemoryStore::new with derived Default
* feat: add store backing for cross-process locking in state store
* chore: rename CryptoStoreLock to CrossProcessStoreLock
* chore: generalize cross-process lock
* feat: move the cross-process locking mechanism to the main crate
* feat: add support for cross-process store lock in the state store 🥳
* feat: implement a cross-process lock for OIDC token refresh
* chore: tweak comment + function name
* feat: make restore_session safe wrt cross-process lock
* feat: add FFI method + add mechanism to reload from keychain
* fix rename
* feat: return early when there was another process refreshed tokens
* fix FFI compile error + tweak some comments
* fix: put the reload_session callback and cross-process locks behind Arc to share them across clients
* feat: Add session retrieval to FFI.
* HACKY; KIDS DON'T DO THIS AT HOME
* chore: log if the hash from db isn't the same from the one from the returned session
* make it simpler to test OIDC token refresh
* some work, that includes fixes and a first test
* feat: require that the reload_session_callback be set at the same time as the cross-process lock
* chore: traces, traces everywhere
* fix: inherit session_change_sender when creating the notification client
* Some FFI improvements to help with tokio problems
* feat: resilient mode when DB/callback disagree about session (callback wins!)
* chore: move sender.send to the finish_refreshing function
* feat: add a save_session callback in the FFI and use it to save the session in keychain while holding XP lock
* fix test expectation after adding the check 🤷
* feat: split the ClientDelegate into two parts, including brand new ClientSessionDelegate
* chore: get rid of lease lock impl in the state store, as it's now unused
* a mix of fmt + clippy
* feat: add ctor for the crossprocessrefreshlockctx
* Include user ID when retrieving session.
Necessary as this isn't known when creating the AuthenticationService.
* yo dawg, you can't block while you block
* share auth data between parent and child client, add lock, AAAAAA this is messy
* tweaks
* feat: make the cross-process store locks generic
And move the implementation to the common crate.
* chore: upgrade some code comments to doc comments in `OngoingMigration`
* feat: implement `CryptoStore::remove_custom_value`
As it's going to be used for the OIDC PR, so as to remove a remembered hash of session tokens.
* remove unneeded remnants
* correctly wait for current request to finish
* feat: make it possible to setup session delegates on android too(?)
* put the cross process stuff in its own file
* typos 🤷
* fix: detach before sending token refresh request, to make sure the response tokens are always properly saved
* kleepee
* First round of review, thanks jonas!
* review round 2. FIGHT
* remove useless logs + avoid using deref explicitly
* more specialized error when cross-process lock is enabled without session callbacks
* fix: avoid cyclic reference between the session callback and client
---------
Co-authored-by: Doug <douglase@element.io>
Co-authored-by: Jonas Platte <jplatte@matrix.org>
When receiving a sliding sync room response for a room that had no local events in its timeline cache,
we'd mark the room as limited before, which is incorrect. It was made worse by the fact that later in
the code, we'd clear the local cache if a room was marked as limited, so this is a problem that would
repeat itself over time (assuming empty responses for that room).
This fixes it and unifies logs so there's only one log line per room, at most.
Fixes https://github.com/vector-im/element-x-android/issues/1281
Fixes https://github.com/matrix-org/matrix-rust-sdk/issues/2540
Currently when the AuthenticationService is given updated metadata, it is ignored if a dynamic registration has already been made for a selected issuer. This PR fixes that by storing the metadata's hash and resetting the store when there is a mis-match.
Additionally it moves OidcRegistrations out of the FFI into a new authentication module in the UI crate and adds some tests.
This patch updates `Room::members` to return
`Result<Arc<RoomMembersIterator>, ClientError>`. This
`RoomMembersIterator` type is new, and is implemented in this patch too.
The idea behind this patch is to allow the bindings to “paginate” over
the list of members for a particular room, in case the room has 17k
members for example.
This patch changes the backoff strategy from `fixed` to `exponential`
when a flaky test is retried. The `count` value is also updated to
3. Finally, we try to avoid the thundering herd problems with `jitter
= true`.
This patch adds the `RoomListService::sync_indicator` method, along
with the `RoomListServiceSyncIndicatorListener` callback interface, and
`RoomListServiceSyncIndicator` enum.
This patch implements a new method: `RoomListService::sync_indicator`.
It returns a `impl Stream<Item = SyncIndicator>` where `SyncIndicator`
is a new enum with 2 variants: `Show` and `Hide`.
`SyncIndicator` is the UI equivalent of a sync spinner/loader/toaster,
that the app might want to show to the user to indicate when a _first_
request is sent and might be slow, i.e. the first response is taking
a little bit of time to come. The term _first_ may be innapropriate as
it covers the actual first sync request, but also the recovering sync
request. It means that when a sync error happened, the sync indicator
will be shown too, which is a pretty useful information for the user.
It's not because a `SyncIndicator` should be shown that it must be send
immediately onto the `Stream`. In case of a normal network conditions,
without any delay, it can lead to a “blinking” visual effect. Some
constants configure how long it takes to consider that a request is
“slow”, and that the `SyncIndicator` is necessary to be shown (or
hidden).
Since the verification status of an event can change, we need to be able to
refetch the verification status without doing the whole decryption dance.
Hence, we expose a new `get_room_event_encryption_info` method.
This was previously accepted by the compiler but is being phased out;
it will become a hard error in a future release! See https://github.com/
rust-lang/rust/issues/115010.
Now that we can decrypt on both of {single|multi} process setups (i.e. available for both android
and ios), we can enable retrying decrypting notifications by default.
Also rejigger the parameters passed to the notification client builder, so that it's always required to pass
a process setup. With that, we're one step closer to removing the retry_decryption() function and enable it
by default.
The `Timeline` batches its updates to its subscribers (e.g. a client app,
like Element X). A batch is built every time the inner state lock of
the `Timeline` is released. On the paper, it's nice; in practise, even
a read operation on the `Timeline` leads to building a new batch. This
is inefficient and consumes resources (like CPU cycles, FFI boundary
crossings, memory allocations etc.) even if there is no update to put in
the batch: this will just batch empty updates.
To avoid that, one needs to make the difference between read or write
operations onto the inner state. Only the write operations will fire a
batch.
This patch splits the `TimelineInnerStateLock::lock` method into
`::read` and `::write`. The idea is that a read-only lock doesn't
hold a clone of the lock release observer (`lock_release_ob:
SharedObservable<()>`), it will not notify the observer. Then it's only
the write lock that holds a clone of the lock release observer, and will
notify it.
This patch updates the code accordingly as best as possible.
This patch updates `RoomListService` to install the `invites` sliding
sync list from the start, along with `all_rooms`. Prior to the patch,
`invites` was installed after the first sync.
`invites` is installed in selective sync-mode with a range of `0..=0`.
After the next sync, `invites` switched its sync-mode to growing with a
batch size of `0..=19`.
This patch takes inspiration of
https://github.com/matrix-org/matrix-rust-sdk/pull/2480.
This patch adds a new `Recovering` state, which is a “transition”
between `Error` and `Terminated` to `Running`. When moving from `Error`
or `Terminated` to `Recovering`, `all_rooms`' sync-mode is set to
`Selective` with its initial range. Then when moving from `Recovering`
to `Running`, `all_rooms`' sync-mode is set to `Growing` with its
initial range again. For the `invites` list, it is reset to its initial
range when moving to `Recovering` too.
`Error` and `Terminated` now act the same.
This patch updates the `batch_size` of `all_rooms` once in growing
sync-mode to 100. It was previously increased from 50 to 200 in
12a1f25ec3. In some situations, it
generates very large payloads that can take time to be delivered to the
user with a slow network.
This patch tries to fine-tuned the `batch_size` value.
This patch reverts b2cc279279. On the
paper, it was a good idea. In practise, it has revealed a bug on the
server side. The only solution to mitigate this bug for now is to revert
the `timeline_limit` to 1.
Prior to this patch, we have increased the
`SlidingSyncListInner::room_list` capacity, to avoid trigger
`VectorDiff::Reset` as much as possible. Now that Element X is optimised
to handle larger diff, we can discrease this number, yeeeee :-).
When a user has hundreds of rooms, `RoomListService` will fetch at worst
one event per room, which can generate large responses. Let's use a
`timeline_limit=0`.
This patch first off renames `BaseClient::ignore_user_list_changes_tx`
to `::ignore_user_list_changes`.
This patch then changes the type from `Arc<SharedObservable<_>>` to
simply `SharedObservable` as this type implement `Clone`. We basically
have a double-`Arc` here.
Finally, this patch adds documentation for this field.
… by swapping the branches around. Previously, invocations meant to
match the found + not_found branch were actually hitting the other one
prior to Rust 1.71, likely due to parser supporting type ascription
(even though it was an unstable feature).
Inside `RoomListService`, the `State` enum handles the transition from
one state to another. In case of some `State::Error`, the `all_rooms`
sliding sync was refreshed, i.e. its sync-mode was reset to its initial
value.
This patch also refreshes the `invites` sliding sync list! It adds
the `ResetInvitesListGrowingSyncMode` action, and attaches it to the
`refresh_lists` actions.
New constants are added to represent default `batch_size` values for the
growing sync-mode for various sliding sync list. It could be helpful for
further maintenance.
This patch finally adds and updates the tests accordingly.
This patch changes the `batch_size` of the sliding sync list `invites`
for `RoomListService`. Previous value was 100, new value is 20.
For accounts that have a large number of invites, it won't slow the
rendering of `visible_rooms`.
Usually, when the sliding sync session expires, it leads the state to
be `Error`, thus some actions (like refreshing the lists) are executed.
However, if the sync-loop has been stopped manually, the state is
`Terminated`, and when the session is forced to expire, the state
remains `Terminated`, thus the actions aren't executed as expected.
Consequently, this patch updates the state to `Error` manually.
- Use OIDC for logout when appropriate.
- Allow server's that support OIDC but not passwords to work.
- Only sign out users if token refresh is explicitly refused.
- Expose the OIDC account URL.
- Support for RP initiated logout.
In the previous situation, we had two locks with similar responsibilities, the `response_handling_lock`
and the `position` lock. The latter *almost* covered the former's critical zone, albeit for a single
function call, which left room for a deadlock situation (latter taken, then former, then latter).
This removes the former, and extends the critical zone of the latter up to the end of the response handling,
removing the possibility of the deadlock entirely.
This reverts commit 9b811009e1.
We have realized that the server might not handle timeouts as expected.
Thus, it's hard to know the difference between network timeout and poll
timeout (resp. the server is unreachable vs. the server has nothing to
respond with). We will come back on this later.
As a sequel of the previous commit (d4cbcd397d), this patch updates
`SlidingSync` to “ignore” timeouts, i.e. timeouts aren't reported to the
caller, and aren't stopping the sync-loop.
All errors inside `SlidingSync` are stopping the sync-loop, and errors
are returned to the caller. However, in some situation, some errors
should be ignored, i.e. they should not stop the sync-loop and they
should not be returned to the caller: the sync-loop just continues to
run. This patch does that for `Error::ResponseAlreadyReceived`. More
errors will come.
Why is it annoying? When `matrix_sdk_ui::SyncService` sees an error,
it stops all the sync-loops (`RoomListService`, `EncryptionSync`…) and
restarts them properly. In the case of `Error::ResponseAlreadyReceived`,
this is a waste of time and resources. This error is an error from the
`SlidingSync` point of view, but _not_ from the caller point of view.
This patch updates the `BaseClient::process_sliding_sync` method to put
the `LeftRoom` in the `SyncResponse::leave` list.
To achieve so, this patch updates
`BaseClient::process_sliding_sync_room` to returns `Option<JoinedRoom>`,
`Option<LeftRoom>` and `Option<InvitedRoom>` (previously, it was only
`JoinedRoom` and `Option<InvitedRoom>`).
SlidingSync emits state events inside `required_state`, but _also_
sometimes inside `timeline`! This patch looks for state events inside
both entries.
The rest of the patch is composed of tests.
Imagine the following scenario:
A request $R_1$ is sent. A response $S_1$ is received and is being
handled. In the meantime, the sync-loop is instructed to skip over any
remaining work in its iteration and to jump to the next iteration. As a
consequence, $S_1$ is detached, but continues to run. In the meantime, a
new request $R_2$ starts. Since $S_1$ has _not_ finished to be handled,
the `pos` isn't updated yet, and $R_2$ starts with the _same_ `pos`
as $R_1$.
The impacts are the following:
1. Since the `pos` is the same, even if some parameters are different,
the server will reply with the same response. It's a waste of time
and resources (incl. network).
2. Receiving the same response could have corrupt the state. It has been
fixed in https://github.com/matrix-org/matrix-rust-sdk/pull/2395
though.
Point 2 has been addressed, but point 1 remains to be addresed. This
patch fixes point 1.
How? It changes the `RwLock` around `SlidingSyncInner::position` to
a `Mutex`. An `OwnedMutexGuard` is fetched by locking the mutex when
the request is generated (i.e. when `pos` is read to be put in the new
request). This `OwnedMutexGuard` is kept during the entire lifetime
of the request extend to the response handling. It is dropped/released
when the response has been fully handled, or if any error happens along
the process.
It means that it's impossible for a new request to be generated and to
be sent if a request and response is running. It solves point 1 in case
of successful response, otherwise the `pos` isn't updated because of
an error.
This patch first off renames `Client::deserialize_events` as
`Client::deserialize_state_events`. Then, this patch initially updated
the return type of this method from `Vec<Option<_>>` to `Vec<_>` to
filter out events that failed to deserialize. The patch ultimately
updates the return type of this method to `Vec<(Raw<_>, _)>`, so that
the raw state events that map to the state event are collected too (this
is required for making `Client::handle_state` to work correctly if an
event failed to deserialized). Finally, the rest of the patch updates
the code accordingly.
`matrix_sdk_ui::room_list_service::Error` has a `SlidingSync` variant to
report errors from `matrix_sdk::sliding_sync::Error` (to be more exact,
from `matrix_sdk::Error::SlidingSync`).
This patch updates the error message from `SlidingSync failed` to
`SlidingSync failed: <explanation>`, where `<explanation>` is the
`Display` representation of the inner error.
It will help to provide more details to the end-user when looking at
logs.
The Room::request_encryption_state method employs a DashMap to uphold a
lock, facilitating the de-duplication of requests directed to the server.
The de-duplication logic involves creating a fresh Mutex and embedding
it into the DashMap through these stages:
1. Generate a new de-duplication mutex.
2. Insert a mutex copy into the DashMap.
2. Acquire the mutex.
Due to DashMap's limitation of enabling map locking solely during
insertion (step 1), a race condition emerges. Consequently, multiple
invocations of the Room::request_encryption_state method might
concurrently endeavor to insert the de-duplication mutex.
This commit removes the chance of such a race. It substitutes the
DashMap with a combination of a mutex and a BTreeMap. This adaptation
permits us to lock the map throughout all three specified operations.
This commit addresses a bug in the Room::is_encrypted functionality.
The issue stems from the Room::request_encryption_state method, which
Room::is_encrypted relies on. This method incorporates a de-duplication
mechanism to ensure that only a single request for the m.room.encryption
state event is made to the server.
Due to this de-duplication mechanism, Room::request_encryption_state
follows two code paths. One path receives the state event from the
server, while the other path merely waits for the first path to complete.
The primary goal of the Room::request_encryption_state method is to
furnish the requested state event. However, because the second code path
doesn't receive any content, it returns an Option.
The problem arises when Room::is_encrypted evaluates this Option. It
erroneously determines that if the Option is None, encryption remains
inactive for the room.
To rectify this, the commit proposes that Room::is_encrypted analyze the
information stored in the in-memory RoomInfo, which is maintained by the
Room::request_encryption_state method. This approach mirrors the existing
behavior of Room::is_encrypted when it processes the code path where the
m.room.encryption state event has already been retrieved.
## Feature
There are now three tasks running in the sync service:
- the room list sliding sync task
- the encryption sync task
- a new scheduler task, that listens to messages sent by both of the previous tasks, and will take care of cleaning up tasks, stopping the services, and setting states after it received a message.
When any of the sliding sync fails, it sends a message to the scheduler task, indicating why it stopped (error or not, was it an expired session or not). Then the scheduler task will do any necessary cleanup.
`stop()` (née `pause()`) can now make use of that scheduler task as well, by sending a report requesting to stop both tasks and services. Responsibilities are now cleanly split: in particular, I like it much better that the room list task doesn't have to stop the encryption sync itself, since it's not really its duty. (And this avoids lots of code duplication to cleanup tasks and stop services.)
## Testing
This also tests the `SyncService` states. Unfortunately it's not perfectly deterministic in two places:
- we can't predict how many requests will be sent to the server (although, with the mocking server responding in 50ms, and considering a waiting time of e.g. 300ms, it should send at least two requests).
- the test `pause()` and re`start()` the sync service at some point, and then we can't guess what the next `pos` will be, in any of the syncs: it could either be the previous value (if we aborted while processing the previous response), or the next value (meaning we could finish processing the sliding sync response).
Still, it confirms at least that pausing and resuming work as expected.
---
Fixes#2382
The timeouts were a bit too agressive, according to some user logs, resulting in failing to
load the event mentioned in the notification.
Here's the rationale for the new timeouts: we're only limited by the iOS process which has *at
most* 30 seconds to process a notification.
- We're running at most 3 requests of the notification sliding sync, so that will be (1+3)*3 = 12
seconds allocated for that.
- If we've found an event but it required decryption, we're running the encryption sync up to
2 times, (3+4) seconds each => 14 seconds.
At most we're eating up 26 seconds of the entire time, leaving some ballast for the rest of
the program.
This patch optimises the previous patch by simplifying the use of
`Timeline`. If the `Timeline` exists, let's return the `latest_event`
every time: whether it is a remote _or_ a local event.
This patch updates `Room::latest_event` to return the `Timeline` local
event if any.
First off, it starts by checking if a `Timeline` exists. It won't create
it if it doesn't exist! Second, it checks whether a latest event exists.
Finally it checks whether it's a local event.
This patch also updates the tests accordingly.
A Sliding Sync `v4::Response` contains a `pos` value. It helps to identify
progress during several requests/responses. If two requests with the
same `pos` are sent, the server **must** reply with the same response,
as defined in the specification.
The corollary is: If a response contains a `pos` that has already
been received, the client **must** ignore it, otherwise it can create
duplications. For example, let a response containing sync operations,
like `DELETE` or `INSERT`, with indexes: if this pair of operations are
repeated twice with the same index, it creates a broken state.
To avoid this behaviour, this patch stores the last 20 position markers
received from the server. If a response contains a `pos` that has
already been received, a (new) error is returned. As with all the other
errors, the sync-loop is stopped, and must restarted. The past positions
survive to this restart, as for the rest of the state.
When the server replies with a `M_UNKNOWN_POS`, the `pos` and the
`past_positions` are all cleared.
This patch removes
`RoomListEntriesDynamicFilter::set_with_fuzzy_match_pattern` and
replaces it by a single `::set` method. It takes a new enum as
parameter: `RoomListEntriesDynamicFilterKind`. This enum has a
`FuzzyMatchRoomName` variant to simulate the removed method. It also
adds the `All` variant, to represent the `all` filter.
This splits the method out so the bigger chunks doesn't persist the
changes in the store.
This will be useful if we need to hijack the changes and persist them in
a different store.
This patch tries to fix https://github.com/vector-im/element-x-
ios/issues/1204 by adding the following required states in the
`visible_rooms` sliding sync list: `m.room.member: $LAZY`.
* feat(ffi): add an optional file logger
Also makes logging to stdout/logcat optional.
* WIP: stupidly duplicate code 🤷
* ffi: Get rid of duplication in tracing initialization
* feat: add OtlpTracingConfiguration too
* feat: log either to stdout on non-android or logcat on android
---------
Co-authored-by: Jonas Platte <jplatte@matrix.org>
If we were invited and joined a room, then the computation of is_direct() was happening after
joining, and depended on the presence of a global account event of type "m.direct". This event
is added by the "set_is_direct()" call thereafter, so this couldn't ever happen.
This fixes it by computing the is_direct field *before* joining the room, and then marking the
room as a DM based on that.
The full timeline event contains both the timestamp (requested in #2361) and the sender id;
now the sender id for the invite can also be contained in there, minimizing the amount of copying we're doing in the
FFI code.
Fixes#2361.
There must be at most one sliding sync notification per connection id. While we could use multiple ids, it seems bad
to do so, in terms of proxy performance; so this makes it so that there is at most once notification being handled
at a time.
- include the conn_id at the stream level, not the sync_once level (that's a child scope of the stream)\
- also include whether e2ee is enabled at this level
- shorten most static logs
- remove duplicate "sending request" logs
Even if the two issues are likely caused by the same root cause (the user not being
authenticated), they should be used in different contexts, in my understanding:
- the authentication required error should happen only during HTTP requests
- the absence of olm machine should be signalled when we try to get one and it's not been initialized yet
This has caused a bit of confusion when looking at debug traces today, so this patches fixes it.
This patch updates `RoomListItem::avatar_url` to use
`matrix_sdk_ui::room_list_service::Room::avatar_url` instead of
`matrix_sdk::Room::avatar_url`.
This patch also moves `avatar_url` before `is_direct` (so that it's the
same order as other places in the code).
This patch implements `Room::avatar_url`. It tries to calculate
the best avatar URL as much as possible. It's either the URL from
`SlidingSyncRoom::avatar_url` or from `Room::avatar_url`.
Based on https://github.com/ruma/ruma/pull/1607, this patch adds
support for `avatar` from a sliding sync response. This patch implements
`SlidingSyncRoom::avatar_url` to get the avatar URL of a sliding sync
room.
The batch subscriber exists in `matrix_sdk_ui::RoomList` (https://
github.com/matrix-org/matrix-rust-sdk/pull/2322) instead of being added
here.
We must keep the observable capacity to 4096, but the `TODO` is no
longer relevant.
Why keeping the capacity to 4096? Because if the batch subscriber (in
`RoomList`) isn't “listened” quickly, we don't want to get a `Reset`.
This patch brings a nice code simplification.
Instead of creating a new `Stream` with `tokio` based on
`Subscriber<State>`` to drain the batch subscriber for
`RoomList::entries` and `::filtered_entries`, we can _simply_ use
`Subscriber<State>` directly! It removes one dependency: `tokio-
stream`, and remove possible issues with the broadcast channel
`tokio::sync::broadcast`. The code is much simpler and straighforward.
As discussed, we think the entire UI crate should be considered experimental. We've also
observed a proliferation of feature flags there (many of those are my wrong doings, sorry).
Since the one internal user (FFI) of that crate enabled all experimental features, it seems
fine to make them all default, while not providing any extra stability guarantee based on that
action.
* feat: run a room sliding sync upon receiving a notification, to get its full content
* test: add test for the new sliding-sync in notifications \o/
* fix: try to get the push rules *after* a possibly-successful event decryption
* feat: set `is_noisy` only if we could build a push context, and test it
* feat: expose the `legacy_get_notification` in the FFI layer
* feat: retrieve events with a `/context` query
* fix: also request the client's user id's member information to get their display name
* feat: include the list of invites in the notification sliding sync
* feat: repeat the query multiple times if the event hasn't been immediately found
* chore: simplify retrying decryption
* chore: fail the sliding sync when fetching a notification if not using a memory store
* chore: update test expectations + sort invites by recency
* chore: cargo fmt
* chore: simplify getting push actions
Either they were already available in the timeline event if we had to re-run decryption, or
we manually compute them. (Previous comment was incorrect, the `push_actions` are now optional
because of another PR of yours truly.)
* fixup! chore: fail the sliding sync when fetching a notification if not using a memory store
* feat: try to handle invites correctly
* chore: build a local client with an in-memory store instead of reusing the parent client
* fix: remove dubious annotation
* feat: allow cloning a Client and modify it on the fly, use that for the notification client
* feat: get rid of the with_memory_state_store public func on ClientBuilder
* feat: include sender_id in the notification invite event
* feat: put sender's id in the `NotificationSenderInfo`
* feat: inherit the parent session when creating a notification client
* chore: reformat comments
* TMP: add logs
* chore: regenerate the olm machine when inheriting a session too
* chore: keep the parent client around for legacy_get_notification/with_context
The proxy server can (and does) redispatch unrelated lists/rooms from other sliding sync connections,
so the encryption sync would sometimes see room events. Apparently since this is not considered a bug
in the SS proxy, so we shouldn't spam error traces every time this happens.
Since `RoomList::entries` returns a batch stream, the listener
now receives a `Vec<VectorDiff<RoomListEntry>>` instead of a
`VectorDiff<RoomListEntry>`.
Only updated the macro is required here. Instead of calling
`StreamExt::now_or_never` on the `$stream`, we call `Iterator::next` on
`$entries` which is a `Vec<VectorDiff<_>>` now.
This patch uses a newly implemented `async-rx` crate, that provides
`StreamExt`. This trait provides new features on `Stream`, like
`StreamExt::batch_with` which allows to batch values generated by a
`Stream` into a `Vec<Stream::Item>`. The batch is drained based on
another `Stream`: every time a value is produced, it drains the batch
stream.
This feature is used in `RoomList::entries` and
`RoomList::entries_filtered` to batch `Stream<Item = VectorDiff<_>>`
into `Stream<Item = Vec<VectorDiff<_>>>`.
The “drainer” is a broadcast sender, which sends an (empty) value every
time the room list service state changes, so every time something
happens during a sync. Note that it even drains when the room list
service state jumps to `Error` or `Terminated`.
In `RoomListService`, there is a cache over the `Room`s to avoid re-
computing them every time the user calls `RoomListService::room`. It
also allows async operations to have time to finish while the user
jumps back to the room list and so on.
When reading this cache, which is `matrix_sdk_common::RingBuffer`,
`RingBuffer::iter` is used with `Iter::find` to find if the
`Room` exists in the cache. The cache has a size of 128 (given by
`ROOM_OBJECT_CACHE_SIZE`), which is quite large.
`Iter::find` will iterate from the oldest to the newest room in the
cache, but realistically, I reckon we need to iterate from the newest
to the oldest room. Indeed, the app user is more likely to jump between
recently opened rooms more frequently, thus saving quite a lot of
iterations for usual cases.
One may argue than in practice, a user won't open 128 rooms anyway… :-p.
Up until now, users had to listen for to-device events to check for
secrets that were received as an `m.secret.send` event.
This has a bunch of shortcomings:
1. Once the has been given to the consumer, it's gone and can't be
retrieved anymore. Secrets may get lost if an app restart happens
before the consumer decides what to do with it.
2. The consumer can't be sure if the event was received in a
secure manner.
This commit ads a inbox for our received secrets where we will long-term
store all secrets we receive until the user decides to delete them.
It's deemed fine to store all secrets, since we only accept secrets we
have requested and if they have been received from a verified device of
ours.
It's preferrable that users make use of the `App::observe_state/current_state` methods, instead
as there's another sliding sync in the `App` and it properly unifies the current state of both.
Before this patch, the inability to compute push actions would result in an empty vector of
push actions, leading to the false assumption that there's no push actions to account for,
while we were unable to compute them properly. This changes things up so that it's now the
user's responsibility to decide what to do when those push actions are missing.
Previously we would generate one-time keys, if needed, whenever we tried
to upload them. This code-path is critically missing a `save_account()`
call and we would only persist the account once we uploaded the one-time
keys.
This patch changes things up to generate one-time keys whenever we
receive new one-time key counts from the sync response. This aligns
with the way we generate fallback keys and removes the need to introduce
a new place where we persist the `Account`.
It's still possible to re-upload the same one-time keys, in the case
where the upload process succeeds on the server side but we fail to
receive the response.
Co-authored-by: Denis Kasak <dkasak@termina.org.uk>
This will limit the memory used by the cache entries (while it was unbounded before). It's now possible to do this,
since we have the `latest_room_event` handy for all the rooms; using the unbounded cache before was papering over
the lack of that feature.
We can bikeshed on the number of entries in this cache. It has to be small enough to not blow up memory (and keep
the linear search over room id fast, but it's secondary), and high enough that we don't hit the full timeline
re-build path that often.
* chore: rename EncryptionSyncMode variants
* feat: split the encryption sync modes into two different functions
* feat: make locking optional in the `EncryptionSync`
* feat: experimental notification client that retries decryption if it failed the first time
* fix: don't iloop retrying decryption
* chore: helper to convert from bool to `WithLocking`
* feat: don't loop and just retry decryption of the notification event linearly
* feat: remove unused set_notification_delegate
Dead code is dead.
* ffi: get rid of `get_notification_item` and introduce the `NotificationClient`
* fmt
* feat: don't swallow encryption sync errors when retrying notification event decryption
* keeping a tidy commit history is NP-hard
* will i ever learn
* chore: enable experimental-notification-client in the FFI crate
* test: add basic integration test for the common path
* Address first batch of review comments, thanks Jonas!
f36a5b8cd7 introduced
deserialize_events, moving the deserialization out of handle_state.
However, the new code in deserialize_events used filter_map, which
caused a deserialization failure to lead to the indices of the
serialized and deserialized events to no longer match up.
This was not caught because iter::zip also does not require that its
arguments have matching lengths, causing the mismatch to cascade for
that batch of events, and persist in the store. An application
affected by this form of corruption can, for example, call
room.get_state_events requesting events of a certain type, and getting
back events of a different type.
Fix the bug by using Option to preserve the length of
deserialize_events' return value, and add an assertion to ensure
handle_state's contract.
Signed-off-by: Vladimir Panteleev <git@cy.md>
This file claimed to have lots of tests, but actually they are not independent
- they are just one big test.
It's not ideal to have one massive test like this, but I don't really have time
to rewrite them and we should stop pretending.
and the latest few encrypted events in Room.
Use the latest event to provide a room preview, and use the encrypted
events to replace the laest event when they are decrypted.
This patch introduces a new `chunk_large_query_over` function. Imagine
there is a _dynamic_ query that runs potentially large number of
parameters, so much that the maximum of parameters can be hit. Then,
this function is for you. It will execute the query on chunks of
parameters.
`chunk_large_query_over` uses `Vec::split_off` to avoid cloning `Key`s
as much as possible. It's difficult to use references here because of
the async nature of `SqliteObjectExt::prepare`.
This patch updates `get_kv_blobs`, `get_room_infos`,
`get_maybe_stripped_state_events_for_keys`, `get_profiles`,
`get_user_ids`, and `get_display_names` to use this new function.
This patch finally adds the `repeat_vars` function to replace in a
more efficient way the `vec!["?"; n].join(", ")` pattern. There is less
memory allocations.
The `RoomList` and `EncryptionSync` API provide a better developer
experience and address concrete needs. Nobody no longer uses the Sliding
Sync bindings, so we can remove them.
For sliding sync that starts both the actual sliding sync request along the e2ee requests,
we need to make sure that, in case of failure of sliding sync, we reset `pos` as soon as possible.
With the code before this patch, the sliding sync response might be available, but the whole
processing could be waiting for the e2ee requests to finish. In the updated version, we spawn
the e2ee requests in a background task, then run the sliding sync request immediately and fail
if it failed.
Another nice benefit is that the e2ee requests won't be interrupted in the middle of processing,
if the sliding sync changed parameters and we cancelled the whole sync a bit too early.
Fixes#2206.
`RoomListServer` defines an `all_room` sliding sync list. This list
starts in selective sync-mode, then it switches to growing sync-mode.
The previous batch size of the growing sync-mode was 50. This patch
updates it to 200, because empirically it seems a better value for
perceived performance.
This patch also rewrites how `State::next` is written. No change in the
code, just comestic.
* sdk: Allow to update notification settings
* Add an event listener for push rule events
* Fix: simplify insertion and deletion of rules
* Fix: Limit rules cloning
* Fix: Unit tests
* Fix: set_room_notification_mode
* Fix: potential race condition when updating the local ruleset
* Refactor RuleCommands
* RuleCommands Unit Tests
* Fix: limit lock usage
* nit: use expression assignment by default
* nit: pass Ruleset by ownership in RuleCommands' ctor
* a few nits: use to_owned() for &str -> String; use free function for notify actions; use clone() where it's more obvious
and use explicit variants so we know we have to consider this match if we were to add a new variant later
* nit: rename RuleCommands::set_enabled to set_enabled_internal
* nit: test modules don't need to be publicized
* tidy tests
* nit: pass an owned RuleCommands in Rules::apply
* add comments/questions in Rules tests
* nit: no need to publicize the tests module
* tweak NotificationSettings tests too
* rustfmt
---------
Co-authored-by: Benjamin Bouvier <public@benj.me>
When the sync has been terminated and restarts (e.g. an app is going from the background to the foreground), then
the sliding sync is always restarting in growing mode, independently of the previous state of the sync (error or normal
termination). This is something the current sliding sync proxy can't handle nicely, because it prioritizes a change in
parameters over live data; as a matter of fact, to alleviate the burden of the proxy, we can try to only restart in
growing mode if we ran into an error (and not if we had a normal termination).
It ensures that we know exactly what's happening here. Tests are still
valid without any changes in the expected data, but it prevents a
potential breaking.
The Sliding Sync list `all_rooms` and `visible_rooms` of the `RoomList`
must have the exact same configurations for `sort`, `filters`, and
`bump_event_types`. Instead of maintaining the same code, this patch
adds a new function: `configure_all_or_visible_rooms_list` that
configures the lists exactly the same. This function also explicitely
configures `sort` so that we do no rely on the default values from
`SlidingSyncListBuilder`.
This patch configures the same `bump_event_types` for the
`visible_rooms` list than for the `all_rooms` list, so that we may be
sure that the sorting/ordering is the same between the two lists.
* ui: Move EventSendState into timeline::event_item::local module
* [WIP] ui: Make pending local echoes stick to the bottom of the timeline
* test: update more test expectations
* chore: tweak comment to slightly better reflect reality
* nit: remove else after return
* fix: the item's insert position is insert_idx, not `items.len()` anymore
* fix: look for remote echo before local echo when processing send state
Previous code assumed that the latest timeline items would be the most recent, and that
if there was a remote echo, it would always be after the local echo, because of that.
That's not the case anymore, so we must look for possibly a remote echo first, and then
if we find it, apply the late update process.
Also, there might remain a day divider added by the local echo, if it were inserted last.
I'm not sure it covers all the cases, but I've now made it so that the day divider is removed
if it was the last element.
* feat: switch strategy; keep on pushing if there's nothing in the timeline yet
* Revert "test: update more test expectations"
This reverts commit 400cc93ba7c98042a28b5e8d5042899e854f6cff.
* test: reset test expectations
* Address review comments
* fix: don't mix up latest event with any status, with latest non-failed event index
---------
Co-authored-by: Jonas Platte <jplatte@matrix.org>
For verification-by-emoji, the spec uses the word "Aeroplane" rather than
"Airplane". Element-web expects us to use the specced words (and will otherwise
complain about the lack of i18n data).
It seems like following the spec will help maintain consistency.
Common extensions are confusing, and they've included `e2ee` and `to-device` by default. This is not a sane default anymore,
now that there's the concept of `EncryptionSync`: it's either we have the encryption sync that enables e2ee and to-device +
a room list sync that doesn't, OR we have a single room list that has both.
Room List was misconfigured to always use `e2ee` and `to-device`, which was incorrect when it's running with the `EncryptionSync`
in the background. This is now removed, and properly tested.
This implements a new time lease based lock for the `CryptoStore`, that doesn't require explicit unlocking, so that's more robust in the context of #1928, where any process may die because the device is running out of battery, or unexpected flows cause a lock to not be released properly in one or the other process.
```
//! This is a per-process lock that may be used only for very specific use
//! cases, where multiple processes might concurrently write to the same
//! database at the same time; this would invalidate crypto store caches, so
//! that should be done mindfully. Such a lock can be acquired multiple times by
//! the same process, and it remains active as long as there's at least one user
//! in a given process.
//!
//! The lock is implemented using time-based leases to values inserted in a
//! crypto store. The store maintains the lock identifier (key), who's the
//! current holder (value), and an expiration timestamp on the side; see also
//! `CryptoStore::try_take_leased_lock` for more details.
//!
//! The lock is initially acquired for a certain period of time (namely, the
//! duration of a lease, aka `LEASE_DURATION_MS`), and then a "heartbeat" task
//! renews the lease to extend its duration, every so often (namely, every
//! `EXTEND_LEASE_EVERY_MS`). Since the tokio scheduler might be busy, the
//! extension request should happen way more frequently than the duration of a
//! lease, in case a deadline is missed. The current values have been chosen to
//! reflect that, with a ratio of 1:10 as of 2023-06-23.
//!
//! Releasing the lock happens naturally, by not renewing a lease. It happens
//! automatically after the duration of the last lease, at most.
```
---
* feat: implement a time lease based lock for the crypto store
* feat: switch the crypto-store lock a time-leased based one
* chore: fix CI, don't use unixepoch in sqlite and do time math in rust
* chore: dummy implementation in indexeddb, don't run lease locks tests there
* feat: in NSE, wait the duration of a lease if first attempt to unlock failed
* feat: immediately release the lock when there are no more holders
* chore: clippy
* chore: add comment about atomic sanity
* chore: increase sleeps in timeline queue tests?
* feat: lower lease and renew durations
* feat: keep track of the extend-lease task
* fix: increment num_holders when acquiring the lock for the first time
* chore: reduce indent + abort prev renew task on non-wasm + add logs
Calling `RoomListItem::full_room` creates its associated `Timeline`.
ElementX calls `full_room` to get the `is_direct`, `avatar_url` and
`canonical_alias` values for _all_ rooms in the room list. Instead of
doing so, let's add those methods directly in `RoomListItem` so that the
`Timeline` isn't created for _all_ rooms.
So. `SlidingSyncList::update_room_list()` does the following:
1. It adjust room list entries,
2. It updates the `maximum_number_of_rooms`,
3. It applies the sync operations on the `room_list` entries,
4. It updates the `room_list` entries for rooms outside the sync
operations.
SlidingSync answers with a `lists` and `rooms`. The `room_list` entries
is updated as follows: either a sync operation from `lists` has been
applied and the entries are updated, or `rooms` contains rooms that
are not updated by `lists` but that receive new events, and thus must
trigger an update in the `room_list` entries.
This patch changes a little bit the last part of this. Initially,
we were updating rooms that are part of `rooms` but absent of
`lists` sync operations, only **for the current list's ranges**.
But that's wrong! First, we have noticed a bug here: the
correct code wasn't `skip(start).take(end.saturating_add(1))` but
`skip(start).take(end.saturating_add(1) - start)`. Note a big deal, we
were iterating on more entries like it was necessary, but everything
was filtered later, so no bug, just useless computations. Second,
this `skip` and `take` is actually… useless. A room to which we have
subscribed can be out of any range, but we still want `room_list`
entries to receive an update for that particular room.
This patch fixes that once and for all.
In practise, the bug wasn't happening because if someone subscribes
to a room, its timeline is likely to be fetched, and updates will be
received, but still, this is better now from a SlidingSync strict point
of view.
This patch does the following:
1. changes `add_timeline_listener` to be async, thus removing one
`RUNTIME.block_on`.
2. simplifies the logic by adopting a more classical pattern we use
elsewhere:
* removes one `spawn_blocking`,
* let's move the `listener` into the task as a `Box` instead of
cloning it as an `Arc`.
Imagine the room list has the viewport set to the range `0..=19`. The
user scrolls quickly to `50..=69` to see something below and immediately
scrolls back to its initial position, without stopping the scroll
at any moment in between. The app using this API might update the
viewport from `0..=19` to… `0..=19`. Updating the viewport, updates the
`visible_rooms` sliding sync list. Since a list is modified, the current
sync-loop iteration is skipped over and a new one restarts, i.e. the
current in-flight request is cancelled… for nothing.
This patch prevents this situation. The current viewport ranges is
stored in `RoomListService`. `RoomListService::apply_input` used to
return `Result<(), Error>`, now it returns `Result<InputResult, Error>`.
The `InputResult` enum is a new type. It represents whether an input has
been applied or ignored, which must be differentiate from errors.
So now, if the viewport changes and it's not different from the previous
value, `InputResult::Ignored` is returned.
According to the spec, when a device receives a verification request and wants
to refuse it, it should send the `m.key.verification.cancel` to the originating
device, rather than broadcasting it.
This opens up a possible race condition where an embedder calls the method twice, generating two new rooms,
but then it's fine as long as they don't cache them somewhere, which we expect they don't.
That avoids recreating a timeline object every time a user calls `RoomListService::room()` with the same room id, so that should speed up
operations like fetching the latest event for rooms we've already entered before.
This patch changes the capacity of the internal buffer of
`ObservableVector` for `SlidingSyncList::room_list` from 16 to 4096.
With an increased capacity, we reduce the probability to send a
`VectorDiff::Reset` to subscribers. `Reset` are happening quite often
for apps using `matrix-sdk`, and it impacts their performance. This
patch tries to improve this situation. This `room-list` contains
`RoomListEntry`, which is can cheap in terms of memory, compared to the
impact of sending `Reset`s too often. That's a tradeoff.
This patch moves `RoomListService::entries` and `::filtered_entries`
inside `RoomList`. It also implements `RoomList::new`. Finally,
it implements `RoomListService::all_rooms` and re-implement
`RoomListService::invites`.
Basically:
```rust
// 1.
room_list.entries().await?
// 2.
room_list.invites().await?
```
becomes:
```rust
// 1.
room_list.all_rooms().await?.entries()
// 2.
room_list.invites().await?.entries()
```
`all_rooms` and `invites` both return a `RoomList`.
@@ -61,7 +61,7 @@ You can stay informed about updates on the access token by listening to `client.
## Quick Troubleshooting
You find yourself focussed with any of these, here are the steps to follow to upgrade your code accordingly:
You find yourself focused with any of these, here are the steps to follow to upgrade your code accordingly:
### warning: use of deprecated associated function `matrix_sdk::Client::register_event_handler`: Use [`Client::add_event_handler`](#method.add_event_handler) instead
// base64 decoder, based on the code at https://developer.mozilla.org/en-US/docs/Glossary/Base64#solution_2_%E2%80%93_rewriting_atob_and_btoa_using_typedarrays_and_utf-8
describe("setup workflow to encrypt/decrypt events",()=>{
letm;
constuser=newUserId("@alice:example.org");
constdevice=newDeviceId("JLAFKJWSCS");
constroom=newRoomId("!test:localhost");
beforeAll(async()=>{
m=awaitmachine(user,device);
});
test("can pass keysquery and keysclaim requests directly",async()=>{
{
// derived from https://github.com/matrix-org/matrix-rust-sdk/blob/7f49618d350fab66b7e1dc4eaf64ec25ceafd658/benchmarks/benches/crypto_bench/keys_query.json
// derived from https://github.com/matrix-org/matrix-rust-sdk/blob/7f49618d350fab66b7e1dc4eaf64ec25ceafd658/benchmarks/benches/crypto_bench/keys_claim.json
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.
Benjamin Bouvier