This patch updates `EventCacheInner::all_caches_for_rooms` by returning
an `OwnedRwLockReadGuard` instead of `Caches`. `Caches` no longer
implement `Clone`, which prevents cloning all the event caches per room
when we need only one for example.
`Caches` has two new methods: `handle_joined_room_update` and
`handle_left_room_update`. This type acts more like a dispatcher now,
which was the initial idea. That way, we don't need to dispatch manually
on all event caches: `Caches` is responsible for that.
We need to be a bit careful now since `all_caches_for_rooms` returns an
owned read-lock over `EventCacheInner::by_room`.
This patch introduces the `Caches::prepare_to_reset` method, which
returns a `ResetCaches` type. This new type is responsible to lock all
the event caches related to a room, and to reset their in-memory state
with `ResetCaches::reset_all`. This patch fixes 2 bugs (see below).
This type is acquiring exclusive locks over all the event caches
managed by `Caches`. Once dropped, all the locks are released. Note that
`ResetCaches::reset_all` takes `self`, not `&self` or `&mut self`, i.e.
it consumes `ResetCaches`, ensuring the locks are _always_ released.
This patch also fixes a possible bug when acquiring the exclusive
locks could have failed, but the error wasn't stopping the execution;
the database would have been reset anyway. Now, `try_join_all` is used
before and after resetting the database (prior to this patch, it was
using `join_all` before resetting the database).
This patch also ensures that only one reset per `Caches` can
happen at a time, by making `Caches::prepare_to_reset` taking a
`&mut self`. This was partially supported by the write lock over
`EventCacheInner::by_room`, but it would have still been possible to
reset the same `Caches` concurrently.
This patch is the first step for the `Caches` type. It creates it, along
with the `new` constructor. More changes are required, but this is a
first step.
The pattern `Caches { room }` is used everywhere where a
`RoomEventCache` was read previously, so that we are sure the type
system will complain when we will add more fields to `Caches` (like for
threads and pinned events).
- https://github.com/matrix-org/matrix-spec-proposals/blob/matthew/location/proposals/3488-location.md
- `If m.asset is missing from the location's content the client should render it as m.self as that will be the most common use case. Otherwise, if it's not missing but the type is invalid or unknown the client should attempt to render it as a generic location. Clients should be able to distinguish between m.self and explicit assets for this feature to be correctly implemented as interpreting everything as m.self is unwanted.`
- this aligns the behavior with the newly introduced live location asset type handling
# Overview
There are scenarios in which it is sensible to have an event exist in
the same room more than once. Notably, this is true in the context of a
thread, where an event exists in the main timeline of a room, as well as
in a thread of that same room.
Support for this behavior has been implemented in the
`SQLiteEventCacheStore` in #6065; however, this was never implemented
for the `IndexeddbEventCacheStore` or the `MemoryStore`. This pull
request extends this behavior to both of those stores.
# Changes
## Integration Tests
First, `test_event_chunks_allows_same_event_in_room_and_thread` was
moved from `matrix_sdk_sqlite::event_cache_store` to
`matrix_sdk_base::event_cache::store::integration_tests`. Then, a few
additional integration tests were added to ensure that behavior is
consistent across implementations of `EventCacheStore`.
## `IndexeddbEventCacheStore`
In order to accommodate the behavioral changes specified by the
integration tests, it was necessary to modify the schema in the
IndexedDB implementation of `EventCacheStore`. Namely, the events object
store was cleared and removed and then replaced with a nearly identical
one, the only difference being the removal of a uniqueness constraint on
one of the indices.
The remaining changes mostly involved updating the behavior of top-level
`EventCacheStore` functions - e.g., filtering out events where they were
duplicated or removing positioning information where it was not
relevant.
## `MemoryStore`
The changes to `MemoryStore` mostly involved updating the behavior of
top-level `EventCacheStore` function - e.g., filtering out events where
they were duplicated or removing positioning information where it was
not relevant.
That being said, it also involved some breaking changes to
`RelationalLinkedChunk`.
1. `RelationalLinkedChunk::items` - this function returned an `Iterator`
that did not contain information about the `LinkedChunkId`, so this
information was added to the items in the `Iterator`.
2. `RelationalLinkedChunk::save_item` - this function did not update the
item in all linked chunks of the provided `Room`. It now does this, but
requires that the provided `Item` be `Clone`.
(1) could probably have been a new function, but I thought a nicer
interface was worth the breaking change. (2) could probably be prevented
by re-organizing `RelationalLinkedChunk`'s internal data structures to
remove the `Clone` requirement, but that seemed like it could turn into
a large refactoring project, so I opted for something simpler albeit
somewhat crude.
In both cases, I'm open to suggestions and would be happy to revisit if
something else is preferred.
---
Closes#6094.
- [x] I've documented the public API Changes in the appropriate
`CHANGELOG.md` files.
- [x] I've read [the `CONTRIBUTING.md`
file](https://github.com/matrix-org/matrix-rust-sdk/blob/main/CONTRIBUTING.md),
notably the sections about Pull requests, Commit message format, and AI
policy.
- [ ] This PR was made with the help of AI.
Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
---------
Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
**Note:** _this pull request has a companion pull request in the
[`complement-crypto`](https://github.com/matrix-org/complement-crypto/pull/229)
repository, which must be merged in conjunction with this one._
_Before merging, this should be tested in conjunction with the Element X
iOS client to ensure that TLS v1.3 is working properly._ @stefanceriu
has agreed to work on this.
## Overview
The primary change in this pull request upgrades the `reqwest`
dependency to its latest version, which defaults to using `rustls` with
support for `rustls-platform-verifier` instead of `native-tls` (see
[`reqwest@0.13.0`](https://github.com/seanmonstar/reqwest/releases/tag/v0.13.0)).
The benefit here is that `rustls` supports TLS v1.3 on all platforms,
whereas [`native-tls` does
not](https://github.com/sfackler/rust-native-tls/pull/278).
Additionally, this pull request makes `rustls` the default TLS
implementation in all the crates in this repository.
This will be particularly helpful with element-hq/element-x-ios#786.
## Changes
- `reqwest` bumped to `0.13.1`
- The API for adding/replacing certificates has changed a bit, so this
required some updating in `HttpSettings::make_client`
- `oauth2-reqwest` added in favor of `oauth2/reqwest`
- This is required in order to be compatible with `reqwest^0.13`
- _**`oauth2-reqwest` is currently in alpha release, so it probably
makes sense to let this stabilize a bit.**_ For details, see
https://github.com/ramosbugs/oauth2-rs/issues/333#issuecomment-3906712203.
- `getrandom` bumped to `0.3.4`
- This is required in order to be compatible with `oauth2@5.1.0`
- `proptest` bumped to `1.9.0`
- This is required in order to be compatible with `getrandom@0.3.4`
- Make `rustls` the default TLS implementation
## Questions
### Mirror feature flag names?
A number of feature flags have been replaced in the dependencies above.
1. _**`reqwest/rustls-tls` => `reqwest/rustls`**_ - this is simply a
name change, but is semantically identical (see
[`reqwest@0.13.0`](https://github.com/seanmonstar/reqwest/releases/tag/v0.13.0)).
2. _**`getrandom/js` => `getrandom/wasm_js`**_ - the semantics here have
changed slightly, but it seems to just make it easier to enable the
`wasm_js` backend (see
[`getrandom@0.3.4`](https://github.com/rust-random/getrandom/blob/master/CHANGELOG.md#major-change-to-wasm_js-backend)).
At any rate, I have updated references to these flags in each of the
various `Cargo.toml` files, but have not changed the names of our
exposed features to mimic those in the dependencies.
Any thoughts or preferences on whether to mirror those names? That
would, of course, result in a breaking change.
### Default to using `rustls`? Deprecate `native-tls`?
Now that the dependencies have all been bumped, we can use `rustls` on
all platforms. Should this be the new default given that `native-tls`
will very likely never support TLS v1.3 on Apple devices? And should
`native-tls` be deprecated as a result?
**UPDATE:** _The consensus here seems to be that we should default to
using `rustls`, but that `native-tls` should still be available._
---
Fixes#5800.
- [ ] Public API changes documented in changelogs (optional)
Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
---------
Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
Turns out, the `m.read` event was invalid, because it should've been
part of the ephemeral events in the sync response (and not part of the
room's timeline response), so the event was dismissed. The test already
passed while behaving this way, so let's make it reflect what it did
indeed.
The test was aborted just a bit too early, in that if you introduce
arbitrary sleep statements, it would fail because the stream of notable
reasons updates wouldn't be empty, and include two `LATEST_EVENT`
updates instead.
It's not clear why we get the second update, but this isn't critical to
fix at the moment, so I'll leave this as an exercise to the reader.
This will allow to reuse it outside of the `oauth` module.
It can now also be converted from a `QueryString`, for improved
compatibility with `LocalServerRedirectHandle`.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
The log message can contain HTML data. It happens notably when the
homeserver is broken, and an HTML document is returned in some errors.
We don't want to parse the fields in this case, because HTML breaks
everything.
We rely on the EventFactory to generate a hash of the events for the
event IDs, and we use the appropriate length for room IDs.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
We rely on the EventFactory to generate a hash of the events for the
event IDs, and we use the appropriate length for room IDs.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
We rely on the EventFactory to generate a hash of the events for the
event IDs, and we use the appropriate length for room IDs.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
We rely on the EventFactory to generate a hash of the events for the
event IDs, and we use the appropriate length for room IDs.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
As a fallback when the ID is not provided when constructing the event.
It allows to work with data that looks like what we would get in
production, which is important for benchmarks.
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
This allows passing the `Transaction` by mutable reference, instead of
passing it by ownership and requiring the callback to pass it back,
which is slightly better in terms of ergonomics. This was hard to
achieve without async closures, but now that we have them, this is
trivial.
Many of our tests make use of `assert_let` for checking that some value
comes out of a stream, while they could use `assert_let_timeout`, which
provides better ergonomics when the expected value doesn't arrive
immediately, by failing quickly.
This converts a few instances, making those tests easier to debug in the
future, would they fail again.
