… 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.
* bindings: Ignore silently malformed userId in migration of tracked users
* Clean: collect will now never fail
Co-authored-by: Benjamin Bouvier <public@benj.me>
* crypto: implement more primitives for the MemoryStore to work in tests
* crypto: change the shape of the `CryptoStoreLock` API
In particular:
- make the lock work across multiple threads of the same process trying to acquire it,
using `num_holders`.
- add a mechanism to get the lock only once (for the NSE process, in case the main app
had acquired the lock before).
* client: add a cross-process crypto store lock, enable it with `Encryption::enable_cross_process_store_lock`
* client: make `preshare_room_key` a critical section of the cross-process lock
* sliding sync: make it possible to define different timeouts for a `SlidingSyncInstance`
This will be handy for the NSE process on iOS, which has very little time to wait for the proxy's responses.
* feat: implement the `EncryptionSync` API (renamed from `Notification` API)
* fixup! client: add a cross-process crypto store lock, enable it with `Encryption::enable_cross_process_store_lock`
* feat: allow disabling e2ee / to-device in the RoomList API
* feat: use same SS id for main/NSE process, reload to-device token from disk before each encryption sync
* fix: better error handling if restoring the to-device token failed
* feat: add logs for the locking functions
* test: add a few tests for encryption sync
* feat: add `reload_caches` method in the `EncryptionSync` + FFI bindings
* chore: clean up FFI loop
* encryption sync: Remove unused errors, specialize some errors
* feat: include termination reason in the encryption sync loop
* feat: add more logs
* chore: fmt + clippy + doc
* comment: precise only in the presence of another process
* Tweak `room_list` APIs to include `_with_encryption` variants
* chore: rustfmt
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`.
This patch introduces `State::Error`, which replaces
`State::Termianted`. And a “new” `State::Terminated` state is added.
They do the same, but their semantics is different. `Error` is when an
error happened, and it's fine to restart the sync again. `Terminated`
is when a termination has been reached (either because SS sync-loop
has ended naturally, or because `RoomList::stop_sync` has been called),
in this case, it may not be fine to restart the sync automatically for
example.
This patch renames `State::FirstRooms` to `State::SettingUp`, and
`State::Running` as requested by users.
It hides the “inner Sliding Sync logic”, the Sliding Sync lists etc.
This patch implements `RoomList::stop_sync` on the FFI bindings.
Technically, it's no more useful to store the `TaskHandle` of the
`sync`, but it's still here, in case of.
This patch implements `RoomList::stop_sync`. The goal is twofold:
1. It forces to stop the syncing, thus putting the state-machine into
the `Terminated` state, which is semantically better than “stop
polling the `sync`'s `Stream`”.
2. It literally forces to stop the syncing. It cancels pending futures,
it cancels in-flight HTTP requests etc. It's a more robust way to
stop the `RoomList` sync.
It has an account field besides the issuer field.
Also store it as immutable, the data used for authentication will be
stored in another variable.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
`SlidingSync::sync` had a legacy behaviour when a `M_UNKNOWN_POS`
error was received from the server: It was resetting `pos` and sticky
parameters before re-running a sync-loop iteration, hoping to get a
valid response from the server after that. This retrying
mechanism was running up to 3 times in row (represented by the
`MAXIMUM_SLIDING_SYNC_SESSION_EXPIRATION` constant) before stopping the
`sync` for real.
While it seemed a good idea, it actually brings numerous problems:
1. Each iteration in the sync-loop generates a new request, thus making
the `ranges` of the requests to move forwards. For `SlidingSyncList`
that are in `Selective` sync-mode, there is no problem, but for
`Growing` or `Paging` sync-modes, the `ranges` increased. Thus,
when a `SlidingSync` session expires, instead of returning to
the caller to do something clever (like what `RoomList` does: start
again with a small range so that the “first” sync after a session
expiration is guaranteed to be fast), it was running larger and
larger requests, up to 3 times.
2. `M_UNKNOWN_POS` _is_ an error. Yes, `SlidingSync` must reset `pos`
and must invalidate sticky parameters, but the `sync` must be
stopped, and the error must be returned so that the caller can do
something about it. Until now, this error was likely to be missed by
the caller.
3. This legacy mechanism was forbidding `SlidingSync::sync`'s caller to
do something with this special error. And since the caller was blind to
this error, it disallowed more smart error management.
This patch removes this legacy retry mechanism entirely. When
`M_UNKNOWN_POS` is received from the server, `pos` is reset and sticky
parameters are invalidated as before, but the error is returned like any
other error.
This patch renames and updates an existing test that was testing sticky
parameters invalidation on `M_UNKNOWN_POS`, to include some assertions
on `pos` and to ensure that the sync-loop is stopped accordingly.
This is the first PR for splitting the sync loop into two. This offers a new high-level API, `NotificationApi`, that makes use of a separate `SlidingSync` instance which sole role is to listen to to-device events and e2ee; it's pre-configured to do so. That means we're not force-enabling e2ee and to-device by default for every sliding sync instance, and as such we won't either generate Olm requests to the home server in general.
In the future, this new high-level API will hide some low-level dirty details so that its can be instantiated in multiple processes at the same time (lock across process, invalidate and refill crypto caches, etc.).
An embedder who would want to make use of this would need the following:
- a main sliding sync instance, without e2ee and to-device. Using the `matrix_sdk_ui::RoomList` would be the best bet, at this time.
- an instance of this `matrix_sdk_ui::NotificationApi`, with a different identifier.
Note that this is not ready to be used in an external process; or it will cause the same kind of issues that we're seeing as of today: invalid crypto caches resulting in UTD, etc.
Fixes https://github.com/matrix-org/matrix-rust-sdk/issues/1961.
Usually it's impossible to create a Olm session from a pre-key message
twice. The one-time key that should be used for the 3DH step will be
used up and we're going to throw a `MissingOneTimeKey` error.
This used to be true and unproblematic until we added fallback keys,
these keys will not get discarded immediately after they have been used
once.
This means that a pre-key message, for which we already have a Session,
but decryption for it fails, might create a new Session overwriting the
existing one which will essentially reset the ratchet.
This implements a value-based lock in the crypto stores. The intent is to use that for multiple processes to be able to make writes into the store concurrently, while still cooperating on who does them. In particular, we need this for #1928, since we may have up to two different processes trying to write into the crypto store at the same time.
## New methods in the `CryptoStore` trait
The idea is to introduce two new methods touching **custom values** in the crypto store:
- one to atomically insert a value, only if it was missing (so, not following the semantics of `upsert` used in the `set_custom_value`)
- one to atomically remove a custom value
Those two operations match the semantics we want:
- take the lock only if it ain't taken already == insert an entry only if it was missing
- release the lock = remove the entry
By looking at the number of lines affected by the query, we can infer whether the insert/remove happened or not, that is, if we managed to take the lock or not.
## High-level APIs
I've also added an high-level API, `CryptoStoreLock`, that helps managing such a lock, and adds some niceties on top of that:
- exponential backoff to retry attempts at acquiring the lock, when it was already taken
- attempt to gracefully recover when the lock has been taken by an app that's been killed by the environment
- full configuration of the key / value / backoff parameters
While it'd be nice to have something like a `CryptoStoreLockGuard`, it's hard to implement without being racy, because of the `async` statements that would happen in the `Drop` method (and async drop isn't stable yet).
## Test program
There's also a test program in which I shamelessly show my rudimentary unix skills; I've put it in the `labs/` directory but this could as well be a large integration test. A parent program initially fills a custom crypto store, then creates a `pipe()` for 1-way communication with a child created with `fork()`; then the parent sends commands to the child. These commands consist in reading and writing into the crypto store, using a lock. And while the child attempts to perform these operations, the parent tries hard to get the lock at the same time. This helps figuring out a few issues and making sure that cross-process locking would work as intended.
This patch renames `SlidingSyncState` to `SlidingSyncListLoadingState`
because:
1. It's about a list information,
2. It's about the loading state, not a generic state.
The MSC3575 says that if no `ranges` for a list is provided, it defaults
to `0..=99`. We don't want that! This patch sets the default value to
`0..=19` for the `visible_rooms`.
* feat: lazily generate and include the transaction id, only if it's useful
* chore: add a small `LazyTransactionId` wrapper that ensures it's only created once
---------
Signed-off-by: Benjamin Bouvier <public@benj.me>
`RoomList::state` provides a `Subscriber` to the `State`. This patch
modifies the way the state is tested, to ensure that there is always
an update broadcasted even if the state is the same (e.g. if the state
moves from `CarryOn` to `CarryOn`).
This patch updates `RoomInner::timeline` and `::sneaky_timeline` to
`AsyncOnceCell<Arc<Timeline>>`. Adding `Arc` allows to copy the timeline
if necessary more easily.
This patch updates `Room::add_timeline_listener` to return a `RoomTimelineListenerResult`, a new type defined as:
```rust
pub RoomTimelineListenerResult {
items: Vec<Arc<TimelineItem>>,
items_stream: TaskHandle,
}
```
It is not possible to cancel the `items_stream` by cancelling the
`TaskHandle`. Without this, dropping `Room` won't drop the `Timeline`,
as a clone is moved inside the spawned task. As a consequence, it will
endlessly call the listener.
What the FFI user wants is to subscribe a listener to the
timeline updates. This feature is already supported by
`room_item.full_room().add_timeline_listener()`. So we can safely
remove `RoomListItem::timeline` as we are sure it's never going to be
used for now.
It's possible to do `room.inner_room().room_id()` but it's just simpler
to call `room.id()`. This patch adds this shortcut.
It's especially useful for FFI users, where creating a “full
room” (`room_item.full_room().room_id()`) may be expensive.
This patch adds the `subscribe` and `unsubscribe` method on `Room`.
To achieve that, `Room` receives an `Arc<SlidingSync>`, as the
subscription methods are on `SlidingSync`. It's just easier to put them
on `Room` for the API consumer. It makes the API also more elegant.
Finally, the patch adds the appropriate test.
Since stripped and non-stripped room infos use the same types,
the separation is not necessary anymore.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
… and simplify wasm spawn implementation.
This reduces the differences on wasm vs. non-wasm. Of course it's still
possible to rely on details of the different error types, but at least
both implement a few common traits:
`Debug`, `Display`, `Error`.
Look through m.room.member events when processing Sliding Sync
responses, finding those that refer to our user's membership, so that we
can recognise which rooms we have left.
Instead of storing `sliding_sync_proxy` inside the `Client`, this patch
updates the code to store it inside `matrix_sdk::Client` directly.
That way, there is unique place where to store the sliding sync proxy
URL.
This patch changes `matrix_sdk::Client::sliding_sync_proxy` to be a
`std::sync::RwLock<Option<Url>>` instead of `tokio::sync::RwLock<_>`.
It means that all methods reading or writing this field are sync instead
of async, which makes the code a lot more easier. Having an async-aware
lock wasn't necessary here.
* chore: add comments explaining what the position markers are
* feat(sdk): add scaffolding for sticky parameters
* test(sdk): add tests for the sticky parameters API
* feat(sdk): include the bump_event_types in the sticky parameters
* WIP: add TODO comments for deeply nested sticky parameters
* test(sdk): add extra test for sticky parameters
* feat: add extensions in the sticky parameters + test request generation + test since token
* chore: fix merge + get rid of unused inner extensions
* feat: use sticky parameters for lists too
* chore: Introduce the StickyManager, a reusable way to manage per-data sticky parameters
* chore: move the sticky scaffolding to its own module
* chore: clippy + fmt
* chore: get rid of `update_to_device_token`
* review: append SlidingSync prefix to Stick* data structures
* address review comments
* Replace nbsp with regular space
* Get rid of a write-access to a lock (because thanks inner mutability!)
* Add an integration test when losing `pos` in sticky parameters, and avoid `block_on`
Signed-off-by: Benjamin Bouvier <public@benj.me>
This patch implements `RoomList::rooms` to fetch one room. The type
name is `RoomListRoom` to avoid any collision with an existing `Room`
type already.
`RoomListRoom` implements `name`, `timeline` and `latest_event`.
By returning a `Subscriber`, one can call `Subscriber::get` to get the
inner value. Thus, having `state` and `state_stream` is useless. It's
possible to return have `state` which returns `Subscriber`, and we get
one value for two usages.
The example in MSC3575 shows a room with invite_state property, but also
timeline, joined_count etc. properties. That may or may not be very
likely in practice, but I think it makes sense to check for all these
properties even when the room is an invite, and use them if they are
present.
Added some comments based on my understanding of MSC3575. Previously we
had a FIXME about how we were setting the room state to invite or join
based on the presence or absence of the invite_state property. I think
this behaviour is correct, so I added some comments explaining why.
The functional change here is to note that we call
store.get_or_create_stripped_room to find/create the stripped room, and
this actually deletes any room of this ID that is in the store, so our
later call to store.get_room would always fall back to getting the
stripped room, which we already have, so there was no need to do the
get_room call at all.
This patch uses a new `async-once-cell` crate to make `Room::timeline`
and `Room::sneaky_timeline` by making them lazy. Their values are
computed lazily, on-demand, when calling the `Room::timeline` method or
`Room::latest_event`.
This extension was enabled by both ElementX apps by default, along with the e2ee / to-device extensions that will be handled
by the new Notification API. To maintain the current behavior, it's important to re-enable this extension before getting
RoomList in production.
Signed-off-by: Benjamin Bouvier <public@benj.me>
By default, get_media_file will attempt to use a default directory
for temporary files and directories. Unfortunately, there might not be
such a thing on some older Android devices, which use per-application
directories.
For this specific case, an optional temporary directory parameter
is added to the `get_media_files` parameters, so one can provide their own
temporary directory path.
This new parameter is expected to be set at least on Android; other platforms
should work just fine without it.
Signed-off-by: Benjamin Bouvier <public@benj.me>
This was a temporary replacement I've made, waiting for a PR of mine to be merged, and it's been merged since then. Oh well.
Signed-off-by: Benjamin Bouvier <public@benj.me>
* sliding sync: infer the storage key from the loop id and user id
* chore: rename `sync_id` to `id`
* chore: check that the sliding sync id is less than 16 chars
* chore: rejigger the storage key creation logic
Now the prefix is visible only in the `format_storage_key_prefix` function, and other `format_storage_key` function will be based off that.
* chore: update sliding sync README with API updates and fix outdated information
* chore: clippy + fix test
Signed-off-by: Benjamin Bouvier <public@benj.me>
This patch handles errors from the Sliding Sync loop properly.
When an error is received, the `RoomList` state machine enters the
`Terminated` state. Immediately after that, the sync stream is stopped.
When the sync stream is restarted, the next state will be calculated.
When the current state is `Terminated`, the next state is the state
that led to the `Terminated` state. To avoid having a “first” huge sync
(imagine a room list of 1000 rooms, you don't want to “resume” from an
error with a first sync for 1000 rooms), lists are partially “reset”
depending of the state where the machine is in.
An important test has been added to test all possibles cases with errors
and list' states.
First off, `SlidingSync::sync_once` no longer returns
`Option<UpdateSummary>` but `UpdateSummary`. It simplifies the rest of
the patch.
Next, `SlidingSync::sync` returns either `Ok(…)` and continues to sync,
or it returns `Err(…)` and it stops the sync stream immediately. No more
error could be returned with the stream to continue syncing.
Finally, the `UnknownPos` error no longer emits an err, but silently
skip over the sync-loop until the threshold is reached; which in this
case will generate a proper error.
This patch changes `RoomInner` to have an `room:
Option<matrix_sdk::room::Room>` field to `room: matrix_sdk::room::Room`.
`room` is not longer an `Option`.
Then, it's easier to have a new `timeline: Timeline` field, along with a
`sneaky_timeline: Timeline` field. Both are used by the new
`Room::timeline()` and `Room::latest_event()` method.
This patch implements `RoomList::room`, which returns a `Result<Room>`:
the room ay or may not exist.
A new `Room` type is created. It wraps an `Arc<RoomInner>` type, so that
it's cheap to clone it.
The `RoomInner` type owns a `SlidingSyncRoom` and
`matrix_sdk::room::Room` type. If some API or data are missing on
the former, the latter acts as a backup. This is the case for the
`Room::name` method which makes it best to return a name to the caller.
This patch lands the first design of the `Input` API. An `Input` is
something external that can be understood by the `RoomList` state
machine. The first inpuput is `Viewport` to change the “viewport” of the
`RoomList`, which translates to the change of the ranges of one specific
Sliding Sync list.
This patch implements `RoomList::update_entries_stream_filter` which
allows to… change the… entries stream filter. This patch also provides a
test for that. How nice it is.
This patch adds a new method on `SlidingSyncList` named
`room_list_filtered_stream`, which uses the new `eyeball-im-util` crate
with its new `FilteredSubscriber` type.
This patch does several things.
First, `SlidingSync::on_list` is now async, and accept async closures.
Second, `SlidingSync::lists` and `::rooms` are behind an `AsyncRwLock`
instead of a `StdRwLock`. The rest of the patch updates the consequence
of this.
This patch replaces a panic (`Option::expect`) by a `tracing::error`
log.
Imagine the Sliding Sync proxy responds with the following payload:
```json
{
"pos": …,
"lists": {
…: {
"count": …,
"ops": [
{
"op": "INSERT",
"index": 0,
"room_id": !foo:bar.org",
}
]
}
},
"rooms": []
}
```
It's an invalid response, as it will update the room list entry (because
it's present in `lists.$list.ops`), but the room won't be created (it's
absent in the `rooms`).
This situation creates a panic in the patched code. We don't want to
crash if the server replies with invalid data.
Ultimately, we should check that the sync operations are valid regarding
the rooms' updates.
This patch uses `FutureExt::now_or_never` so that we don't wait on the
future to be resolved to get a result. If we have a bug in our code and
the test expects a value, the test will hang forever, which is not a
desired behavior. With `now_or_never`, this situation cannot happen.
The only reason why a sender can fail to send a message is because there
is no receiver. In the current design of `SlidingSync`, there is only
one receiver active per sync-loop. Thus, calling `SlidingSync::add_list`
may fail if `sync` has not been started. Hence the need for a new
`internal_channel_send_if_possible` method which will never fail: it
will send a message is possible, otherwise it won't do anything.
This turns more functions infallible.
`tokio::sync::broadcast` is interesting as it's possible to
generate receivers per subscribers. Being able to do that allows
us to remove the new for an `AsyncLock` around the receiver in
`SlidingSync::internal_channel`, and it can even remove the need to hold
a receiver entirely!
Another improvement is that new receivers can't receive past messages,
so we no longer need to drain the internal channel.
Another improvement is that the sender' `send` method is synchronous!
Which helps to make many functions no longer `async`.
Something weird is happening with ElementX iOS. When
`SlidingSync::stop_sync` is called, the internal message `SyncLoopStop`
has time to be sent in the internal channel, but iOS decides to suspend
the code before the sync-loop processes it. When ElementX decides to
start the sync-loop again, it immediately processes the `SyncLoopStop`
message, and… stops the sync-loop.
This patch ensures that `SlidingSync::sync` drains the internal channel
before starting the sync-loop for real. `tokio::sync::mpsc::Receiver`
type has no `drain` method, so this patch implements its
own logic by calling `try_recv` in a loop, until it returns
`Err(TryRecvError::Empty)`.
* feat: introduce SlidingSyncSelectiveModeBuilder
* feat: get rid of `CannotModifyRanges` \o/
* chore: rustfmt
* chore: remove scope in test
* chore: move request generator update to its own mod, gets rid of `set_ranges`
* chore: add comments on the request generator methods
* chore: sink one range_end definition into the match arm that matters
* ffi: remove unused `add_range` method
* test: update incorrect test expectation
* chore: make clippy happy
* address first review comments
* review: don't reuse the ranges field for two things
* chore: use a builder pattern for sliding sync selective mode
* address review comments + CI
Signed-off-by: Benjamin Bouvier <public@benj.me>
In case it's not obvious to drop the `Stream` returned by
`SlidingSync::sync` immediately to “stop” the sync-loop, one can use the
new `stop_sync` method to do achieve the same result.
The `VerificationRequest` object is used to control the flow of the
verification but only up to a certain point.
Once we start handling of different specific verification flows (i.e.
SAS or QR code verification) the `VerificationRequest` object creates a
child object of the Verification type.
This patch adds a new `VerificationRequestState` variant called
`Transitioned` which holds the child verification object as associated
data.
This makes it much simpler to go through the whole verification flow by
allowing users to just listen to the `VerificationRequest::changes()`
method.
We suspect that there might be too many internal messages being pushed, causing a deadlock on the senders' side.
This attempts to increase the value of the buffer to give it more leeway.
Signed-off-by: Benjamin Bouvier <public@benj.me>
The `SlidingSyncListInner::timeline_limit` and `::ranges` fields were
observable (behind `eyeball::Observable`). It was actually useless
as those fields were never exposed to the public API, thus it was
impossible to subscribe to them.
This patch cleans up that. `timeline_limit` and `ranges` are no longer
observable.
The comment and the check in the code were contradicting each other; the comment was wrong, and the code was right, so there's that.
Signed-off-by: Benjamin Bouvier <public@benj.me>
This patch removes our dependency to libolm completely. This should
allow WASM targets to use the backups_v1 feature of the
matrix-sdk-crypto crate as well.
First off, this patch changes `pushd(…)` to `pushd(…)?` so that errors
are propagated.
Second, instead of assuming that all crates live in `crates/`, let's
allow to precise a prefix, like `crates/` or `bindings/` directly in the
“folder” path of `args`.
Changing the sync-mode of a list on-the-fly is necessary to optimise the
new `RoomList` API. For example, we can start with a list in a selective
mode, a range of `0..=50` and a `timeline_limit=0` to fetch the
beginning of the room list, and then _change_ the sync-mode to growing
to continue to sync the room list in the background.
Today, this is done with 2 lists and a merge of the lists, but
(i) this is error-prone, (ii) this is not optimal. Thank to
`SlidingSyncList::set_sync_mode`, merging lists is no more necessary,
thus removing a class of bugs in client's code.
This patch replaces `sync_mode` on `SlidingSyncListBuilder` by
`sync_mode_selective`, `sync_mode_paging` and `sync_mode_growing`, which
removes the need to use `SlidingSyncMode` directly.
This patch also removes `batch_size`, `room_limit` and `no_room_limit`
as it's now arguments of `sync_mode_paging` and `sync_mode_growing`.
`SlidingSyncMode` was previously declared as:
```rust
enum SlidingSyncMode {
Selective,
Paging,
Growing,
}
```
Now, the new declaration is:
```rust
enum SlidingSyncMode {
Selective,
Paging {
batch_size: u32,
maximum_number_of_rooms_to_fetch: Option<u32>,
},
Growing {
batch_size: u32,
maximum_number_of_rooms_to_fetch: Option<u32>,
}
}
```
First off, it helps to remove the `full_sync_batch_size` and
`full_sync_maximum_number_of_rooms_to_fetch` methods and fields from
`SlidingSyncListBuilder`. It was containing default values in case of.
That was useless and needed to be fixed. Also, calling a `full_sync_*`
method with the `Selective` mode had no effect, which could disturb
the user. Well, now everything is clean on that front.
Second, `SlidingSyncListRequestGenerator` no longer has constructors,
but a `From<SlidingSyncMode>` implementation. However, the constructors
now live in `SlidingSyncMode` with `new_selective`, `new_paging` and
`new_growing` methods which are helpers. All in all, it makes creating a
`SlidingSyncListRequestGeneator` simpler: just call `sync_mode.into()`
and boom.
Finally, this patch removes the `default_with_fullsync` list builder
helper, which makes no sense at all.
`SlidingSync::subscribe`, `::unsubscribe` and `::add_list` are
now async. `subscribe` has been renamed to `subscribe_to_room` and
`unsubscribe` to `unsubscribe_from_room`.
`SlidingSyncRoom::subscribe_and_add_timeline_listener` has
been removed, and replaced by a new `::subscribe_to_room` and
`::unsubscribe_from_room` methods (the `::add_timeline_listener` method
already exists).
`TaskHandle::finalizer` is no longer necessary, thus this code has been
cleaned up.
This bug has been found by @bnjbvr, all the credits go to him. I've just
added some comments around his code.
Prior to this patch, the room unsubscription buffer
(`SlidingSync::room_unsubscriptions`) was reset before the request was
sent. So if something went wrong, the next request would not include the
room unsubscriptions.
This patch updates this behavior. First, it replaces `Vec` by `HashSet`
to avoid a O(n^2) look up.
Second, a copy of room unsubscriptions used by the request is kept, so
that it can be used to cherry-pick which room unsubscription to remove
from the buffer once a response from the server is received. It's
important to not clear the entire room unsubscriptions buffer as more
unsubscriptions could have been inserted meanwhile.
This has slightly drifted from the initial design I thought about in the issue.
Instead of having `build()` be fallible and mutably borrow some substate (namely `BTreeMap<OwnedRoomId, SlidingSyncRoom>`) from `SlidingSync` (that may or may not already exist), I've introduced a new `add_cached_list` method on `SlidingSync` and `SlidingSyncBuilder`. This method hides all the underlying machinery, and injects the room data read from the list cache into the sliding sync room map.
In particular, with these changes:
- any list added with `add_list` **won't** be loaded from/written to the cache storage,
- any list added with `add_cached_list` will be cached, and an attempt to reload it from the cache will be done during this call (hence `async` + `Result`).
- `SlidingSyncBuilder::build()` now only fetches the `SlidingSync` data from the cache (assuming the storage key has been defined), not that of the lists anymore.
Fixes#1737.
Signed-off-by: Benjamin Bouvier <public@benj.me>
Co-authored-by: Jonas Platte <jplatte+git@posteo.de>
Currently, the `body` of a `SignatureUploadRequest` includes a spurious
`signed_keys: {...}` property in which the actual content is wrapped. Fix that.
We were using `txn_id` as a way to identify the running stream. Now
things are cleaner so we can remove this “debug tool”. This class of
problems should not appear anymore. For the record, the biggest problem
was the following: It was possible to start multiple `stream`s at the
same time, and thus a stream could receive a response sent by another
stream. Since we no longer need to restart SlidingSync anymore (i.e. no
need to start multiple `stream`s at the same time), this problem should
not happen.
Prior to this patch, the `v4::Response`, aka the SlidingSync response,
was processed by the client and transformed into a `SyncResponse` into
`sync_once` before being passed to `handle_response`. Now it's done in
`handle_response`.
This is not mandatory _now_, but it will be helpful in a close future.
* chore: use RangeInclusive instead of raw start/end integers for ranges in sliding sync
* chore: have timeline_limit use a u32 instead of a Ruma UInt
* chore: Remove all the `Into<u32>` generics on timeline_limit and ranges APIs
* chore: introduce a `Bound` type alias for u32
Signed-off-by: Benjamin Bouvier <public@benj.me>
This patch changes `SlidingSyncRoom` to move all its fields inside an
inner type `SlidingSyncRoom` behind an `Arc`, so that cloning is cheap,
and all clones are sharing the same state.
A `SlidingSyncList` has a state, which can be either `NotLoaded`,
`Preloaded`, `PartiallyLoaded`, or `FullyLoaded`.
A `SlidingSyncList` can be reset, either manually when
`SlidingSyncList::reset` is called, or when a range is updated for
example.
Resetting a list is modifying its state. Prior to this patch, the state
was set to `NotLoaded`. However, it's not entirely true. This patch
updates this behavior to the following rules:
* When the state is `NotLoaded`, it's kept at this state as nothing has
happened yet,
* When the state is `Preloaded`, the list is restored from the cache,
but nothing has happened yet too, so it's kept at this state,
* When the state is `PartiallyLoaded` or `FullyLoaded`, it means some
(or all) updates have been done, so the new state is `PartiallyLoaded`
after the reset.
The list' state is used mostly by the client to know whether a loader
should be prompted to the users. The ranges are modified when the
users scroll inside the room list for example: scrolling in the room
list doesn't imply the state should go to `NotLoaded`. Some data
have potentially be loaded, so changing the ranges should result in a
`PartiallyLoaded` state. It seems more logical once explained like this.
`invite_state` is managed when the Sliding Sync response is processed/
handled by the `Client`. Inside `SlidingSyncRoom`, it's not necessary to
maintain it.
This patch creates a constant to represent the
maximum number of timeline event to put in the cache:
`NUMBER_OF_TIMELINE_EVENTS_TO_KEEP_FOR_THE_CACHE`. Verbose, but easily
understandable.
This patch also renames a few variables.
This patch simplifies `SlidingSyncRoom::timeline_queue` from
```rust
Arc<RwLock<ObservableVector<SyncTimelineEvent>>>
```
to
```rust
Vector<SyncTimelineEvent>
```
First, we don't need to be observable. It's never observed since
it's private, and even privately, it's never observed, there is no
subscriber.
Second, no lock is required as updates happen synchronously.
Third, `Arc` is not necessary. We want each clone of `SlidingSyncRoom`
to not share any state across them.
Finally, this patch simplifies the iterator + `.push_back` by a simple
`.extend` to update the `timeline_queue`. Behind the scene, `impl Extend
for Vector` actually does an iterate + `.push_back`; let's keep our code
simple though.
`SlidingSyncRoom::is_cold` is not well-named. This patch introduces an
enum, named `SlidingSyncRoomState` which contains more detailed state:
`NotLoaded`, `Preloaded`, and `Loaded`.
The use of `AtomicBool` is also removed. Thus, we no longer need an
`Arc` for the state, which makes `Clone` more obvious: the state is not
shared across clones anymore.
A `SlidingSyncRoom` receives an Ruma
`api::client::sync::sync_events::v4:SlidingSyncRoom`. This value is
stored in the `SlidingSyncRoom::inner` field. From here, some getters
like `name()`, `is_dm()` etc. are using the `inner` field to compute a
result. There was one exception though: `prev_batch`. This value is part
of `v4::SlidingSyncRoom` but it was copied and updated in its own field:
`SlidingSyncRoom::prev_batch`.
I was wondering why. Turns out, there is no reason. Its getter
`prev_batch()` is public, but it's not used by the FFI bindings, so
basically nobody uses it (as this project is experimental as the time of
writing, we know our users).
This patch removes the `SlidingSyncRoom::prev_batch` field.
This patch also removes the `SlidingSyncRoom::prev_batch()` getter.
This patch finally removes the `FrozenSlidingSyncRoom::prev_batch` field
too.
Prior to this patch, to check a room exists, it was removed from the
collection, then re-added if it exists. Instead of doing this `remove`
+ `insert` dance, we can simply use `get_mut`, which also returns an
`Option`. This patch does that, in addition to rewrite the code to use a
`match` instead of a `if` + `else`.
`SlidingSync::subscribe` and `SlidingSync::unsubscribe` will cancel in-
flight requests, i.e. the `SlidingSyncInternalMessage::ContinueSyncLoop`
will be sent in the internal channel, just like what `SlidingSyncList`s
already do when a parameter is changed.
If the redaction already happened server-side, it is a bug for the
server to even still have access to the non-redacted form.
We don't accept the server data in this case, and also log a warning.
`on_built` is a method that registers a closure. This closure is called
when a list is built by `SlidingSyncListBuilder::build`. It receives
a `SlidingSyncList` and returns a `SlidingSyncList`, the list can be
updated or returned as is. It allows to configure a `SlidingSyncList`
right after it's built. `SlidingSyncBuilder::build` is responsible to
finalize the configuration, and to build all the lists. Once they are
built, the state is restored from the cache. If one wants to configure
a list before the state is restored from the cache, `once_built` will
serve well.
The timestamp value of the Notification was not reliable since it was the
timestamp in which it was generated internally, not the timestamp of when the
event was sent, now we instead expose the `origin_server_ts` of the
TimelineEvent.
`is_read` is also very unreliable, so it's just removed for now.
Currently this takes a `CrossSigningKeyExport`, but that's not a class you can
construct from the JS side. Instead, let's just pass in the individual keys.
Using the #[derive(uniffi::Object)] on certain types make them not show up
in the .aar file when building for Android. Could not determine why that is,
but removing the derive macro, and adding the empty interface declarations
inside the UDL, makes them show up.
We can still use #[uniffi::export] and #[uniffi::constructor].
Signed-off-by: Simon Farre <simon.farre.cx@gmail.com>
Currently, we can't pass the response to a `SigningKeysUploadRequest` into
`mark_request_sent`, which apparently we should be able to do.
So, we need to make `SigningKeysUploadRequest` have a `type` like the other
`OutgoingRequest`s, and add support for `SigningKeysUploadResponse` to
`OwnedResponse`.
Because since SlidingSync no longer restarts, the behaviour is really
different. The current way the tests are written cannot assert the full
behaviour. We need to rewrite this test suite entirely.
The unknown filter can be difficult to match with MembershipState,
depending on the store implementation.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Storing as bitset is not future-proof.
Just fix the latest migration. We assume no-one used it yet.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
The MPSC channel that has been introduced in recent commits is now used
in this patch. The `SlidingSync::stream` method can be controlled via
the `internal_channel`.
This patch updates `SlidingSync::stream` to use the `tokio::select!
` macro to select any future that resolves first between the
`internal_channel` receiver, or `SlidingSync::sync_once`. Fairness is
biaised as the `internal_channel` has the priority.
This mechanism is already used by this patch: `SlidingSyncList::reset`
will send the `SlidingSyncInternalMessage::ContinueSyncLoop` to
“continue”… well… the sync loop, i.e. it will cancel in-flight waiting
for a response.
This entire mechanism removes the need to “stop” and “start”, i.e.
“restart” the `SlidingSync::stream` method manually, which was the
source of many bugs. Now everything is controlled internally.
Prior to this patch, `SlidingSync::add_list` was taking a
`SlidingSyncList`. However, we need to inject more data when building
the list, so let's modify `add_list` to take a `SlidingSyncListBuilder`
instead. It's even better for the user as calling `build()` isn't
necessary anymore.
It's not possible to clone a `SlidingSyncList` anymore. Why? Because
it's not correct. Prior to this patch, it was possible to add a list
to a `SlidingSync` instance, then add a clone of the same list to
another `SlidingSync` instance. Weird behaviors could happen, but more
importantly, for the next Sliding Sync design we are working on, it
could lead to gigantic bugs.
Removing `Clone` from `SlidingSyncList` makes the code simpler.
For example, `SlidingSyncList.inner` no longer needs an `Arc`, or
`SlidingSync::stream` no longer needs to clone all the lists.
The `SlidingSyncBuilder` now has a new method: `bump_event_types`, to
configure the `bump_event_types` HTTP request parameter. This value
is passed to `SlidingSync`, which is used to build the `SlidingSync`
request.
The format of the outbound group session struct has changed. We nowadays correctly rotate the group session if we can't restore it, but it's still good to avoid logging the error in this case.
An error is currently thrown if loading an outbound group session fails.
This error may only affect loading outbound group sessions, while other
store operations continue to work fine. As a result, the user is unable
to send messages, but can still use the application without any problems.
Since outbound group sessions are rotated frequently, we can simply
rotate the session if loading fails. However, if the error is related to
a more serious storage issue, persisting the newly rotated outbound group
session will also fail. If the error is specific to outbound group
sessions, the user will be able to continue using the application without
interruption.
The SQLite crypto store uses rmp_serde to serialize all the data we're
going to store. This works nicely for most things, one exception to this
is the OutboundGroupSession type.
The OutboundGroupSession type stores to-device requests to ensure that
the session doesn't get used before it is shared with the whole group
and to ensure that the to-device requests get restored if the
session gets restored after an application restart.
The to-device requests type critically contain `Raw<AnyToDeviceEvent>`,
the `Raw` type here being the serde_json::Raw type. rmp_serde seems to
serialize this just fine, but later deserialization fails.
We're avoiding the issue by using serde_json to serialize the
OutboundGroupSession.
Those methods are public, but never used by our current users.
Moreover, there is some big overlaps. For example, `storage_key`,
`cold_cache` and `no_cache` do the same thing: They update the
`storage_key` field. Or `add_fullsync_list` is just a helper that is
never used except in the tests etc.
The test does the following:
1. Create 2 (identical) lists,
2. Do a sync.
3. Assert that the 1st and 2nd lists are receiving an update,
4. Add a 3rd (identical) list,
5. Do a new sync,
6. Assert that 3rd list is receiving an update.
This last step is wrong. All lists should receive an update as they
are identical.
This patch updates a test to ensure that `room_list` receives “diff”
for rooms that are modified by a sync operations, but also by another
updates (like a new event).
The `SlidingSyncList.room_list` field is used to store the list of rooms
for a particular Sliding Sync list. It's used by the user to receive
“diff”s when a room sees its position modified. For example when a new
room receives a message, it “climbs back up” the entire room list to
be at the top place. Another example is when a new room is created,
some rooms will move around to give the new room a space. All those
moves will create “diff”.
However, the user also expects to receive a “diff” when a room has
received some updates, even if it's position doesn't change. For
example, when a room is already at the top of the list but receives a
new message: It has received an update, but its position stays the same.
This specific latter feature was implemented before, but it has been
removed by accident in https://github.com/matrix-org/matrix-rust-sdk/
pull/1699 (more specifically in https://github.com/matrix-org/matrix-
rust-sdk/pull/1699/commits/861a05be69a566d9a4ad125dc6ecb418d2b3210f).
At that time, it was not clear why the code was filtering for specific
filled room entires, to set the filled room entries, at the same
position. Zero comment, zero test. I didn't consider this as a feature
but as a bug.
So this patch re-introduces this feature. Hopefully in a more optimal
way. If a room has already triggered a first “diff” because of a
position change, it won't trigger a second “diff” because it has
received an update (which was the case before).
The `SlidingSyncList::handle_response` method has also been renamed
`update`, as it does update, whenever it comes from.
The code has been commented and documented to explain this feature.
Existing tests have been updated, especially for `apply_sync_operations`
which now ensure the `rooms_that_have_received_an_update` collections is
updated accordingly. Another test has been updated specifically to test
the “diff”s received by `room_list`.
This patch also re-uses the `format_storage_key_*` functions to use the
same key as the `restore_sliding_sync_state` function. It fixes a bug
where the keys were mismatching.
This PR is self explanatory.
Exposes said function to the FFI. Defined in the .adl file,
and defined to throw `ClientError`
Signed-off-by: Simon Farre <simon.farre.cx@gmail.com>
Co-authored-by: Damir Jelić <poljar@termina.org.uk>
In some cases, restoring client state from backups may cause Olm sessions
to become corrupted, resulting in a backward ratchet. As a consequence,
the receiving side is unable to decrypt messages. To address this issue,
the failed side initiates a new Olm session and sends a dummy encrypted
message.
However, this dummy message also creates a new Olm session on the
sender's side. To ensure that both sides use the same new session, we
must sort sessions by their creation timestamp before encrypting new
messages.
Although the session list was sorted correctly previously, we selected
the wrong side of the list, resulting in an outdated session being
selected instead of the newest one. This patch resolves the issue by
selecting the newest Olm session and adds a test to the session getter
logic to prevent reintroduction of this bug.
Prior to this patch, when `SlidingSync` was built with a storage key,
the storage was read to find a previous `SlidingSync` version that
was in a cache. To put a `SlidingSync` instance in the cache, it
was serialized to JSON. When one reads from the cache, the value is
deserialized. A problem happens when the internal representation of
`SlidingSync` and other types (e.g. `SlidingSyncList`) have changed (due
to an SDK update for example): Loading a previous state from the cache
results in an error.
After discussion with the teams, clients don't want to deal with
obsolete cache values. In such a scenario, (i) they cannot interact
with the cache to remove the obsolete entry, and (ii) their only
possibility is to log out and log in the user again. What an unfriendly
user experience!
This patch modifies this behaviour. When loading a `SlidingSync` type or
a `SlidingSyncList` type from the cache, 3 cases are now handled:
1. The cache entry exists and has been successfully deserialized: in
this case, we do our business.
2. The cache entry exists, but it wasn't possible to deserialize it: the
cache entry is declared as obsolete, and the entire `SlidingSync`
cache is cleaned up, so all entries for `SlidingSync` and
`SlidingSyncList` are removed from the storage,
3. The cache entry doesn't exist: we do nothing particular.
Note that if one cache is obsolete, all cache entries are removed.
The `SlidingSync::stream` method no longer resets the lists everytime
it is called. If one wants to reset the lists, they need to call
`SlidingSync::reset_lists`.
`SyncOp::Invalidate` means _invalidating_ a particular range. When a
room is `Filled`, it becomes `Invalidated`, when it is `Invalidated` it
stays `Invalidated`, and when it is `Empty` it stays `Empty`.
Before this patch, `Empty` was becoming `Invalidated`, which apparently
is a bug.
This patch also fixes out-of-bound accesses, and adds many tests.
Finally, this patch renames `update_state` to `update_room_lists`.
`SyncOp::Insert` means _inserting_ a new room ID. The `rooms_list`
contains all possible rooms (based on `maximum_number_of_rooms`, a value
returned by the server). Here, inserting = setting
`RoomListEntry::Filled` at a particular index of `rooms_list`, that's
it.
The previous code was doing very complex stuff, like removing things
around the `index` if something etc. It was using the requested ranges
(the range passed to the request) etc.
Applying `SyncOp` should be simple and is focused on updating
`rooms_list` only: the requested ranges have nothing to do here.
This patch also prevents against out-of-bounds acccesses, which wasn't
the case before.
This patch prevents out of bounds acceses for `SyncOp::Delete`, and adds
more tests.
This patch also removes a cast from `UInt` to `u32` to `usize`. It's now
from `UInt` to `usize` directly.
First off, this patch renames `ops` to `sync_operations` and `room_ops`
to `apply_sync_operations`.
Second, the `SlidingOp::Sync` was creating an out-of-bounds access
depending of the range present in the server's response. For example, if
the `rooms_list` contains 5 elements (because the
`maximum_number_of_rooms` is set to 5), and the server replies with:
```json
{
"op": "SYNC",
"ranges": [3, 17],
"room_ids": […]
}
```
the previous code was setting a new `RoomListEntry` at indices `3..=17`,
whilst the `rooms_list` contains only indices from `0..=4`. That's
annoying.
The previous code was also counting the number of `room_ids` for
nothing, just to execute the iterator that was applying the actual
changes in a `map`. Well, everything was fishy.
This patch updates the code to protect against an unexpected server's
reply by raising an `Err`. This patch also adds tests.
The `updated_rooms` argument was passed to `find_rooms_in_list` to
update the `room_list`: the update is setting the filtered room list
entry to `RoomListEntry::Filled`.
_But_, `find_rooms_in_list` was already filtering rooms which are
`Filled`. So it does… nothing: it filters rooms which are `Filled` to
update them to `Filled`.
So we can remove `find_rooms_in_list` because it becomes useless. And we
can remove `updated_rooms` too.
The `rooms_list` is updated by `rooms_ops` itself. Let's keep
modifications in one unique place.
The `update_state` method of `SlidingSyncListInner` has basically
2 cases:
1. For an initial response,
2. For other responses.
The code between the 2 cases were almost identical. Or, they could be
identical. The few exceptions are:
* In the first case, the `rooms_list` updates were taking the
form of a `VectorDiff::Append`, while the second case, it was a
`VectorDiff::PushBack`.
* In the first case, the `is_cold` flag was set to `false`.
It's fine for the clients to receive only a `VectorDiff::Append` event
only. So let's make it uniform.
And it appears that the `is_cold` field is now private, and never read
anywhere else. So… it's… basically useless. We can remove it! It was
previously used here to know which flow to use, but since we can make
both flows identical, its role becomes insignificant.
`create_range` and `SlidingSyncList::next_request` return a `Result`
instead of an `Option`. It was a legacy from the old `impl Iterator` of
`SlidingSyncListRequestGenerator`. But actually, when `create_range`
fails, it must return a `Err` value, not a `None` value.
And since it's a regular error, `sync_once` can propagate the
error. Thus, it is no longer necessary to “update” the list of
`SlidingSyncList`. It's not necessary to remove some items in this list
when the `next_request` method can no return a value: it always returns
a value, except when a error happens.
First off, `set_ranges`, `set_range`, `add_range` and `reset_ranges`
take a `&[(U, U)]` instead of a `Vec<(U, U)>` when a vector was needed,
or takes a `(U, U)` instead of 2 arguments when it was needed.
Second, all those methods now return a `Result<(), Error>`. The
`Error::CannotModifyRanges` is raised if the chosen `SlidingSyncMode`
doesn't allow to modify ranges. Basically, `Selective` does allow a
user to modufy the ranges, but there is no real ranges with `Growing` or
`Paging`. Let's make it an explicit error.
Finally, `SlidingSyncListInner` has 2 methods: `set_ranges` and
`add_range`, but without any “ranges check”; `SlidingSyncList` does call
the inner methods, but does the checks.
The `SlidingSyncListInner::find_rooms_in_list` method can be greatly
improved by using the `Iterator` API.
It was already using `Iterator::skip`, great! Let's continue by using
`Iterator::take` instead of using a stop index.
And let's use `Iterator::enumerate` to avoid using a mutable index.
`SlidingSyncList::find_room_in_list` and
`SlidingSyncList::find_rooms_in_list` aren't useful (and never used) by
the public consumer of this API. Let's remove it. They are only used for
internal purposes.
Before, the `update_request_generator_state` method was emitting an
`error!` log, but not an error. But that's clearly an error!
This patch updates the method to return a `Result<(), Error>`, and adds
a new variant to `Error`.
There are problems with `SlidingSyncListRequestGenerator`:
* It's part of the module API,
* It contains a clone of `SlidingSyncList`.
To create a `SlidingSyncListRequestGenerator`, one has to
call `SlidingSyncList::request_generator`. It was done in
`SlidingSync::stream`. The problem is that it clones `SlidingSyncList`.
So theoritically it is possible to create multiple request generators
for the _same_ list, and use them to send many requests and to update
the _same_ list with multiple responses. This is utterly error-prone and
can lead to really complex bugs to discover.
Moreover, it's a lot of clones. Cloning a
`SlidingSyncListRequestGenerator` isn't cheap as it means cloning a
`SlidingSyncList`. Moreover, cloning a `SlidingSyncList` isn't cheap as
it means cloning the entire struct.
Having `SlidingSyncListRequestGenerator` inside the module API also
makes the code of `SlidingSync` more complex (why having to deal with
lists and request generators at the same time? we must be very careful
to maintain both side by side? what if a list is removed but not its
request generator? and so on).
So. This patch simplifies all that.
First off, it extracts all the fields of `SlidingSyncList` into a
`SlidingSyncListInner` struct. Then, `SlidingSyncList` has only one
field: `Arc<SlidingSyncListInner>`. Boom, it's now cheap to clone it.
Second, `SlidingSyncListRequestGenerator` is only a
struct with constructors, but there is no extra methods.
`SlidingSyncListRequestGenerator` _no longer_ contains a
`SlidingSyncList`, and doesn't no need a list at all to work. It's just
values.
Third, `SlidingSyncList` holds a `SlidingSyncListRequestGenerator`, and
only one.
Fourth, `SlidingSyncList` (and `SlidingSyncListInner`) now has methods
to handle the internal request generator. The initial `impl Iterator for
SlidingSyncListRequestGenerator` becomes a simple
`SlidingSyncList::next_request` method.
Fifth, previously, the `SlidingSyncList::handle_response`
was never called directly by `SlidingSync`. It was called by
`SlidingSyncListRequestGenerator`! How confusing! `SlidingSync` called
`SlidingSyncListRequestGenerator::handle_response` which was calling
`SlidingSyncList::handle_response`. Now, the flow is more natural:
`SlidingSync` calls `SlidingSyncList::handle_response` and that's it.
Sixth, the `SlidingSyncList::handle_response` is now composed of 2
parts: updating the list itself, and updating the request generator. It
was kind of the case before, but onto two different types.
It was unclear which types were updating `SlidingSyncList`.
For example, `SlidingSyncList::state` was updated by…
`SlidingSyncListRequestGenerator`. Now `SlidingSyncList` is responsible
to update itself, and no one else.
Finally, `SlidingSync` no longer have to deal with
`SlidingSyncListRequestGenerator`. All it has is a set of
`SlidingSyncList`, and that's it!
The tests are still passing, hurray.
In `SlidingSyncListBuilder`, the `ranges`, `set_range`, and `add_range`
methods do not take a `u32` but a `U: Into<UInt>`. That's great!
However, in `SlidingSyncList`, the same methods take a `u32`. It makes
the API inconsistent.
This patch fixes that.
This patch renames `SlidingSyncList.set_ranges` to `ranges` so that
it matches the same terminology of the `SlidingSyncListBuilder` with
`ranges`, `set_range`, `add_range` and `reset_ranges`.
Consistency is important here.
The `SlidingSyncList::new_builder` method creates a new builder from a
`SlidingSyncList`. This method was missing some configurations, like
`full_sync_maximum_numbre_of_rooms_to_fetch`, `send_updates_for_items`,
`filters`, `ranges` and `timeline_limit`.
This patchs adds those missing configurations.
The `SlidingSyncListBuilder` struct has a `state` and a `rooms_list`
fields. Those fields are never updated and no setters or getters exist
to use them. There are just set with defaults, and passed to the final
`SlidingSyncList` type within the `build` method.
If a field is present in a builder type, it means they can be modified,
but it's not the case. So this patch removes them.
Event deduplication is supposed to be handled by the `Timeline`. Sliding
Sync is not responsible to de-duplicate the events, in our model. Having
two different de-duplication logics (one in Sliding Sync, and one in the
Timeline) could create weird situations, and hard to debug issues.
We give it a try by removing the deduplication from Sliding Sync.
The `m.no_olm` withheld code is supposed to be sent out only once for a
given device.
This patch prevents double sending of m.no_olm withheld code by adding
a test that ensures only one group session will send it out.
Previously, the logic for sharing group sessions was mostly correct, but
an edge case was forgotten where two group sessions attempted to share
the withheld code concurrently.
Although there's still a possibility for multiple m.no_olm withheld codes
to be sent out. This may happen during the propagation of the flag
remembering the fact that the `m.no_olm` code has been sent from the
OutboundGroupSession to Device. This scenario is likely unrealistic and
the consequences of sending things twice aren't problematic.
With https://github.com/matrix-org/matrix-rust-sdk/pull/1601, it's safer to
cancel a stream at any time, as any `Future` cancellation. This documentation
section of `SlidingSync` is no longer relevant as is, and can be rephrased.
@@ -51,7 +51,7 @@ You can stay informed about updates on the access token by listening to `client.
- [`MessageOptions`][message options] has been updated to Matrix 1.3 by making the `from` parameter optional (and function signatures have been updated, too). You can now request the server sends you messages from the first one you are allowed to have received.
-`client.user_id()` is not a `future` anymore. Remove any `.await` you had behind it.
-`verified()`, `blacklisted()` and `deleted()` on `matrix_sdk::encryption::identities::Device` have been renamed with a `is_` prefix.
-`verified()` on `matrix_sdk::encryption::identities::UserIdentity`, too has been prefixed with `is_` and thus is onw called `is_verified()`.
-`verified()` on `matrix_sdk::encryption::identities::UserIdentity`, too has been prefixed with `is_` and thus is now called `is_verified()`.
- The top-level crypto and state-store types of Indexeddb and Sled have been renamed to unique types>
-`state_store` and `crypto_store` do not need to be boxed anymore when passed to the [`StoreConfig`][store config]
- Indexeddb's `SerializationError` is now `IndexedDBStoreError`
@@ -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
This project and build script demonstrate how to create an XCFramework that can be imported into an Xcode project and run on Apple platforms. It can compile and bundle an [entire SDK](#Building-the-SDK), or only a smaller [Crypto module](#Building-only-the-Crypto-SDK) that provides end-to-end encryption for clients that already depend on an SDK (e.g. [Matrix iOS SDK](https://github.com/matrix-org/matrix-ios-sdk))
## Prerequisites for building universal frameworks
-`xcodebuild` command line tool from [Apple](https://developer.apple.com/library/archive/technotes/tn2339/_index.html)
-`lipo` for creating the fat static libs
### Building the SDK
```
cargo xtask swift build-framework
@@ -17,7 +19,7 @@ cargo xtask swift build-framework
The `build-framework` task will go through all the steps required to generate a fully usable `.xcframework`:
1. compile `matrix-sdk-ffi` libraries for iOS, the iOS simulator and macOS under `/target`. Some targets are not part of the standard library and they will be built using the nightly toolchain.
1. compile `matrix-sdk-ffi` libraries for iOS, the iOS simulator and macOS under `/target`. Some targets are not part of the standard library and they will be built using the nightly toolchain.
2.`lipo` together the libraries for the same platform under `/generated`
3. run `uniffi` and generate the C header, module map and swift files
4.`xcodebuild` an `xcframework` from the fat static libs and the original iOS one, and add the header and module map to it under `generated/MatrixSDKFFI.xcframework`
@@ -27,7 +29,7 @@ For development purposes, it will additionally generate a `Package.swift` file i
When building the SDK for release you should pass the `--release` argument to the task, which will strip away any symbols and optimise the created binary.
## Building only the Crypto SDK
### Building only the Crypto SDK
```
build_crypto_xcframework.sh
@@ -41,7 +43,7 @@ The `build_crypto_xcframework.sh` script will go through all the steps required
4.`xcodebuild` an `xcframework` from the fat static libs and the original iOS one, and add the header and module map to it under `generated/MatrixSDKCryptoFFI.xcframework`
5. cleanup and delete the generated files except the .xcframework and the swift sources (that aren't part of the framework)
## Building & testing the Swift package
### Building & testing the Swift package
The `Package.swift` file in this directory provides a simple example on how to integrate everything together but also a place to run unit and integration tests from.
@@ -52,8 +54,20 @@ It's pre-configured to link to the generated static lib and .swift files so succ
The generated framework and Swift code can be distributed and integrated directly but in order to make things simpler we bundle them together as a [Swift package](https://github.com/matrix-org/matrix-rust-components-swift/) in the case of SDK, and as a CocoaPods podspec in the case of Crypto SDK.
### Publishing MatrixSDKCrypto
1. Run `build_crypto_xcframework.sh` script which generates a .zip file with the framework
2. Increment the version in `MatrixSDKCrypto.podspec`
3. Create a new [GitHub release](https://github.com/matrix-org/matrix-rust-sdk/releases) with the same version (see [example](https://github.com/matrix-org/matrix-rust-sdk/releases/tag/matrix-sdk-crypto-ffi-0.1.0) for naming)
4. Upload the .zip file to this release
5. Push new Podspec version to Cocoapods via `pod trunk push MatrixSDKCrypto.podspec --allow-warnings`
1. Navigate into `bindings/apple` and run [`build_crypto_xcframework.sh`](#building-only-the-crypto-sdk) script which generates a .zip file with the framework in `./generated` folder
Note that whilst you can run this command from any git branch, if you want to make a public release, ensure you are building from latest `main`
2. Tag the commit you just used to build the release and push the tag to GitHub
Use `matrix-sdk-crypto-ffi-<major>.<minor>.<patch>` tag name
3. Create a new [GitHub release](https://github.com/matrix-org/matrix-rust-sdk/releases) using this tag (see [example](https://github.com/matrix-org/matrix-rust-sdk/releases/tag/matrix-sdk-crypto-ffi-0.3.4))
4. Upload the previously generated .zip file to this release
5. Increment the version in [`MatrixSDKCrypto.podspec`](./MatrixSDKCrypto.podspec)
Note that this is not automated and is a local-only change. To determine the most recent published version, run `pod repo update && pod search MatrixSDKCrypto`
or check git tags via `git tag | grep matrix-sdk-crypto-ffi`
6. Push new Podspec version to Cocoapods via `pod trunk push MatrixSDKCrypto.podspec --allow-warnings`
// 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.
Jonas Platte