feat(sdk): Welcome `sliding_sync::Version` and `sliding_sync::VersionBuilder` #3889

Hywan · 2024-08-23T13:24:50Z

OK. So this PR should be reviewed commit-by-commit. It seems large, but most of the additions are tests.

This PR does 3 things. It's not possible to split them without breaking main.

It removes requires_sliding_sync, sliding_sync_proxy, is_simplified_sliding_sync_enabled and all these messy configurations that are super error-prone (and that already contain at least one bug, e.g. it's possible to define a sliding sync proxy URL with simplified sliding sync enabled at the same time; now: which one is used? no error, but simplified sliding sync is taking precedence). Everything is replaced by a single, unique, simple:
```
enum Version {
    None,
    Proxy { url: Url },
    Native,
}
```
It feels much better after that:
- The “no sliding sync” state is represented by None.
- The “proxy” state is represented by Proxy { url }.
- The “native” state is represented by Native.
After the introduction of matrix_sdk::sliding_sync::Version, the second part of the PR introduces matrix_sdk::sliding_sync::VersionBuilder that is used only by matrix_sdk::ClientBuilder. A VersionBuilder is also an enum:
```
enum VersionBuilder {
    None,
    Proxy { url: Url },
    Native,
    DiscoverProxy,
    DiscoverNative,
}
```
It's designed to build a Version. VersionBuilder::None maps to Version::None. Proxy to Proxy. Native to Native. Then DiscoverProxy maps to Proxy by reading the .well-known file; it expects "sliding_sync": { "url": … } to contain a valid URL. And DiscoverNative maps to Native by reading the /versions API; it expects the unstable_features to contain ("org.matrix.simplified_msc3575", true) (see Add a flag to /versions about SSS support element-hq/synapse#17571).
A last small API is added: Client::available_sliding_sync_versions() to help an owner of a Client to know which Versions are available without the need to build a Client with ClientBuilder::sliding_sync_version(VersionBuilder::…).build() and see whether it builds.

Note: Complement Crypto breaks because the C FFI API has changed. This PR disables Complement Crypto. As soon as this PR is merged, I'll fix Complement Crypto, and re-enables it in this repository. Edit: PR is ready, matrix-org/complement-crypto#128.

Address [meta] SSS & PPS #3647
Address [Story] EX: Migration to SSS element-hq/element-meta#2499

This patch replaces all the API using simplified sliding sync, or sliding sync proxy, by a unified `sliding_sync::Version` type! This patch disables auto-discovery for the moment. It will be re-enable with the next patches.

codecov · 2024-08-26T07:27:59Z

Codecov Report

Attention: Patch coverage is 96.47059% with 3 lines in your changes missing coverage. Please review.

Project coverage is 84.08%. Comparing base (be13388) to head (47f2c6f).
Report is 33 commits behind head on main.

Files	Patch %	Lines
crates/matrix-sdk/src/client/builder.rs	95.83%	1 Missing ⚠️
crates/matrix-sdk/src/sliding_sync/builder.rs	85.71%	1 Missing ⚠️
crates/matrix-sdk/src/sliding_sync/client.rs	97.50%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3889   +/-   ##
=======================================
  Coverage   84.08%   84.08%           
=======================================
  Files         266      266           
  Lines       27744    27767   +23     
=======================================
+ Hits        23328    23348   +20     
- Misses       4416     4419    +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This patch changes the type of `discover_homeserver_from_server_name_or_url`. It now returns a `Url` instead of a `String` for the homeserver URL. It also returns an `Option<get_supported_versions::Response>` in addition to the other values. The change from `String` to `Url` is necessary to avoid a double-parsing. It was parsed in `build()` but previously in `discover_homeserver_from_server_name_or_url` in the last branch. The addition of `get_supported_versions::Response` is necessary for the next patch. It's going to be helpful to auto-discover sliding sync in Synapse. The change happens here because `get_supported_versions::Response` is already received in `discover_homeserver_from_server_name_or_url`. This patch makes it easy to re-use it so that the request is sent only once. This patch therefore changes `check_is_homeserver` a little bit to become `get_supported_versions`, and inlines its previous call inside `discover_homeserver_from_server_name_or_url`.

This patch adds a builder for `sliding_sync::Version`. It is a similar enum except that it has `DiscoverProxy` and `DiscoverNative` to automatically configure `Version::Proxy` or `Version::Native`.

Previous patches have unified all sliding sync versions behind a single type: `Version`. More recent previous patches have introduced `VersionBuilder` so that a `ClientBuilder` can use them to coerce or find the best `Version` possible. This patch implements a last missing piece: `Client::available_sliding_sync_versions` will report all available `Version`s at a given time. This is useful when a `Client` is already built, and a session has been opened/a user is logged, but someone has to take the decision whether it's useful to switch to another sliding sync version or not.

This patch adds bindings to `Client::available_sliding_sync_versions` to `matrix-sdk-ffi`. This patch also moves `Client::sliding_sync_version` from “private” to “public” FFI API, in thee sense that this method is now exported with UniFFI.

bnjbvr

lgtm, thanks! Mostly minor comments, I'll help landing this.

Thanks for this clean refactoring!

crates/matrix-sdk/src/sliding_sync/client.rs

crates/matrix-sdk/src/client/mod.rs

crates/matrix-sdk/src/sliding_sync/mod.rs

crates/matrix-sdk/src/sliding_sync/client.rs

crates/matrix-sdk/src/client/builder.rs

bnjbvr · 2024-08-27T15:25:49Z

For what it's worth, I've also disabled the complement-crypto required check on main, as a result of fa4ddde. Please don't forget to reenable it later!

Hywan · 2024-08-28T06:56:35Z

crates/matrix-sdk/src/client/builder.rs

-            Ok(_) => {
-                return Ok((homeserver_url, None, None));
+            Ok(response) => {
+                return Ok((homeserver_url, None, Some(response)));


🤦 of course! Thanks for the fix.

Hywan · 2024-08-28T06:58:08Z

crates/matrix-sdk/src/sliding_sync/client.rs

    }
 }

+/// An error when building a version.
+#[derive(thiserror::Error, Debug)]
+pub enum VersionBuilderError {


🎵 This is the story of a lazy man 🎵 with an awesome non-lazy co-worker 🎶

Hywan mentioned this pull request Aug 23, 2024

[meta] SSS & PPS #3647

Open

33 tasks

Hywan added 2 commits August 26, 2024 08:39

feat(sdk): Add sliding_sync::Version.

f0100f5

feat: Use sliding_sync::Version everywhere.

b917304

This patch replaces all the API using simplified sliding sync, or sliding sync proxy, by a unified `sliding_sync::Version` type! This patch disables auto-discovery for the moment. It will be re-enable with the next patches.

Hywan force-pushed the feat-sdk-sliding-sync-version-selector branch from 9440643 to b917304 Compare August 26, 2024 07:14

Hywan force-pushed the feat-sdk-sliding-sync-version-selector branch 3 times, most recently from a5f2815 to e018677 Compare August 26, 2024 15:37

Hywan changed the title ~~feat(sdk): Sliding sync version selector~~ feat(sdk): Welcome sliding_sync::Version and sliding_sync::VersionBuilder Aug 26, 2024

Hywan added 5 commits August 26, 2024 18:05

feat(sdk): Add sliding_sync::VersionBuilder.

3793c06

This patch adds a builder for `sliding_sync::Version`. It is a similar enum except that it has `DiscoverProxy` and `DiscoverNative` to automatically configure `Version::Proxy` or `Version::Native`.

feat: Use the new sliding_sync::VersionBuilder.

ea8e885

test(sdk): Restore disabled tests.

8990789

test(sdk): Add 2 more tests for auto-discovery of sliding sync.

4b2464b

test: Disable Complement Crypto for a short period.

fa4ddde

Hywan force-pushed the feat-sdk-sliding-sync-version-selector branch from e018677 to fa4ddde Compare August 26, 2024 16:06

Hywan marked this pull request as ready for review August 26, 2024 16:21

Hywan requested a review from a team as a code owner August 26, 2024 16:21

Hywan requested review from bnjbvr and removed request for a team August 26, 2024 16:21

This was referenced Aug 26, 2024

[Story] EX: Migration to SSS element-hq/element-meta#2499

Closed

fix(api): Use the new sliding sync version API from the Rust SDK matrix-org/complement-crypto#128

Merged

Hywan added 2 commits August 27, 2024 14:30

Hywan force-pushed the feat-sdk-sliding-sync-version-selector branch from a413bff to 54b066e Compare August 27, 2024 12:30

bnjbvr approved these changes Aug 27, 2024

View reviewed changes

sliding sync version(chore): address review comments

47f2c6f

bnjbvr merged commit 468ee53 into matrix-org:main Aug 27, 2024
39 checks passed

Hywan commented Aug 28, 2024

View reviewed changes

bnjbvr mentioned this pull request Aug 28, 2024

Slow/hanging HTTP responses from self-hosted Synapse homeserver. #3894

Closed

Hywan mentioned this pull request Sep 3, 2024

test: Restore Complement Crypto #3932

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(sdk): Welcome `sliding_sync::Version` and `sliding_sync::VersionBuilder` #3889

feat(sdk): Welcome `sliding_sync::Version` and `sliding_sync::VersionBuilder` #3889

Hywan commented Aug 23, 2024 •

edited

Loading

codecov bot commented Aug 26, 2024 •

edited

Loading

bnjbvr left a comment •

edited

Loading

bnjbvr commented Aug 27, 2024

Hywan Aug 28, 2024

Hywan Aug 28, 2024

feat(sdk): Welcome sliding_sync::Version and sliding_sync::VersionBuilder #3889

feat(sdk): Welcome sliding_sync::Version and sliding_sync::VersionBuilder #3889

Conversation

Hywan commented Aug 23, 2024 • edited Loading

codecov bot commented Aug 26, 2024 • edited Loading

Codecov Report

bnjbvr left a comment • edited Loading

Choose a reason for hiding this comment

bnjbvr commented Aug 27, 2024

Hywan Aug 28, 2024

Choose a reason for hiding this comment

Hywan Aug 28, 2024

Choose a reason for hiding this comment

feat(sdk): Welcome `sliding_sync::Version` and `sliding_sync::VersionBuilder` #3889

feat(sdk): Welcome `sliding_sync::Version` and `sliding_sync::VersionBuilder` #3889

Hywan commented Aug 23, 2024 •

edited

Loading

codecov bot commented Aug 26, 2024 •

edited

Loading

bnjbvr left a comment •

edited

Loading