Skip to content

Commit 468ee53

Browse files
bnjbvrbnjbvr
committed
sliding sync version(chore): address review comments
1 parent 067b1e0 commit 468ee53

File tree

6 files changed

+63
-61
lines changed

6 files changed

+63
-61
lines changed

‎bindings/matrix-sdk-ffi/src/client_builder.rs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use matrix_sdk::{
1212
ruma::{ServerName, UserId},
1313
sliding_sync::{
1414
Error as MatrixSlidingSyncError, VersionBuilder as MatrixSlidingSyncVersionBuilder,
15+
VersionBuilderError,
1516
},
1617
Client as MatrixClient, ClientBuildError as MatrixClientBuildError, HttpError, IdParseError,
1718
RumaApiError,
@@ -195,8 +196,11 @@ pub enum ClientBuildError {
195196
#[error(transparent)]
196197
WellKnownDeserializationError(DeserializationError),
197198
#[error(transparent)]
199+
#[allow(dead_code)] // rustc's drunk, this is used
198200
SlidingSync(MatrixSlidingSyncError),
199201
#[error(transparent)]
202+
SlidingSyncVersion(VersionBuilderError),
203+
#[error(transparent)]
200204
Sdk(MatrixClientBuildError),
201205
#[error("Failed to build the client: {message}")]
202206
Generic { message: String },
@@ -213,7 +217,9 @@ impl From<MatrixClientBuildError> for ClientBuildError {
213217
MatrixClientBuildError::AutoDiscovery(FromHttpResponseError::Deserialization(e)) => {
214218
ClientBuildError::WellKnownDeserializationError(e)
215219
}
216-
MatrixClientBuildError::SlidingSync(e) => ClientBuildError::SlidingSync(e),
220+
MatrixClientBuildError::SlidingSyncVersion(e) => {
221+
ClientBuildError::SlidingSyncVersion(e)
222+
}
217223
_ => ClientBuildError::Sdk(e),
218224
}
219225
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -582,8 +582,8 @@ async fn discover_homeserver_from_server_name_or_url(
582582
if let Ok(homeserver_url) = Url::parse(&server_name_or_url) {
583583
// Make sure the URL is definitely for a homeserver.
584584
match get_supported_versions(&homeserver_url, http_client).await {
585-
Ok(_) => {
586-
return Ok((homeserver_url, None, None));
585+
Ok(response) => {
586+
return Ok((homeserver_url, None, Some(response)));
587587
}
588588
Err(e) => {
589589
debug!(error = %e, "Checking supported versions failed.");
@@ -826,10 +826,10 @@ pub enum ClientBuildError {
826826
#[error("Error looking up the .well-known endpoint on auto-discovery")]
827827
AutoDiscovery(FromHttpResponseError<RumaApiError>),
828828

829-
/// Error from sliding sync.
829+
/// Error when building the sliding sync version.
830830
#[cfg(feature = "experimental-sliding-sync")]
831831
#[error(transparent)]
832-
SlidingSync(#[from] crate::sliding_sync::Error),
832+
SlidingSyncVersion(#[from] crate::sliding_sync::VersionBuilderError),
833833

834834
/// An error encountered when trying to parse the homeserver url.
835835
#[error(transparent)]

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -459,13 +459,13 @@ impl Client {
459459
self.inner.homeserver.read().unwrap().clone()
460460
}
461461

462-
/// Get the sliding version.
462+
/// Get the sliding sync version.
463463
#[cfg(feature = "experimental-sliding-sync")]
464464
pub fn sliding_sync_version(&self) -> SlidingSyncVersion {
465465
self.inner.sliding_sync_version.read().unwrap().clone()
466466
}
467467

468-
/// Override the sliding version.
468+
/// Override the sliding sync version.
469469
#[cfg(feature = "experimental-sliding-sync")]
470470
pub fn set_sliding_sync_version(&self, version: SlidingSyncVersion) {
471471
let mut lock = self.inner.sliding_sync_version.write().unwrap();

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

Lines changed: 46 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use std::collections::BTreeMap;
22

3+
use as_variant::as_variant;
34
use imbl::Vector;
45
use matrix_sdk_base::{sliding_sync::http, sync::SyncResponse, PreviousEventsProvider};
56
use ruma::{
@@ -14,13 +15,13 @@ use ruma::{
1415
use tracing::error;
1516
use url::Url;
1617

17-
use super::{Error, SlidingSync, SlidingSyncBuilder};
18+
use super::{SlidingSync, SlidingSyncBuilder};
1819
use crate::{config::RequestConfig, Client, Result, SlidingSyncRoom};
1920

2021
/// A sliding sync version.
2122
#[derive(Clone, Debug)]
2223
pub enum Version {
23-
/// No version. Useful to represent that sliding sync is disable for
24+
/// No version. Useful to represent that sliding sync is disabled for
2425
/// example, and that the version is unknown.
2526
None,
2627

@@ -41,13 +42,35 @@ impl Version {
4142
}
4243

4344
pub(crate) fn overriding_url(&self) -> Option<&Url> {
44-
match self {
45-
Self::Proxy { url } => Some(url),
46-
_ => None,
47-
}
45+
as_variant!(self, Self::Proxy { url } => url)
4846
}
4947
}
5048

49+
/// An error when building a version.
50+
#[derive(thiserror::Error, Debug)]
51+
pub enum VersionBuilderError {
52+
/// The `.well-known` response is not set.
53+
#[error("`.well-known` is not set")]
54+
WellKnownNotSet,
55+
56+
/// `.well-known` does not contain a `sliding_sync_proxy` entry.
57+
#[error("`.well-known` does not contain a `sliding_sync_proxy` entry")]
58+
NoSlidingSyncInWellKnown,
59+
60+
/// The `sliding_sync_proxy` URL in .well-known` is not valid ({0}).
61+
#[error("the `sliding_sync_proxy` URL in .well-known` is not valid ({0})")]
62+
UnparsableSlidingSyncUrl(url::ParseError),
63+
64+
/// The `/versions` response is not set.
65+
#[error("The `/versions` response is not set")]
66+
MissingVersionsResponse,
67+
68+
/// `/versions` does not contain `org.matrix.simplified_msc3575` in its
69+
/// `unstable_features`, or it's not set to true.
70+
#[error("`/versions` does not contain `org.matrix.simplified_msc3575` in its `unstable_features`, or it's not set to true.")]
71+
NativeVersionIsUnset,
72+
}
73+
5174
/// A builder for [`Version`].
5275
#[derive(Clone, Debug)]
5376
pub enum VersionBuilder {
@@ -87,7 +110,7 @@ impl VersionBuilder {
87110
self,
88111
well_known: Option<&discover_homeserver::Response>,
89112
versions: Option<&get_supported_versions::Response>,
90-
) -> Result<Version, Error> {
113+
) -> Result<Version, VersionBuilderError> {
91114
Ok(match self {
92115
Self::None => Version::None,
93116

@@ -97,42 +120,27 @@ impl VersionBuilder {
97120

98121
Self::DiscoverProxy => {
99122
let Some(well_known) = well_known else {
100-
return Err(Error::VersionCannotBeDiscovered(
101-
"`.well-known` is `None`".to_owned(),
102-
));
123+
return Err(VersionBuilderError::WellKnownNotSet);
103124
};
104125

105126
let Some(info) = &well_known.sliding_sync_proxy else {
106-
return Err(Error::VersionCannotBeDiscovered(
107-
"`.well-known` does not contain a `sliding_sync_proxy` entry".to_owned(),
108-
));
127+
return Err(VersionBuilderError::NoSlidingSyncInWellKnown);
109128
};
110129

111-
let url = Url::parse(&info.url).map_err(|e| {
112-
Error::VersionCannotBeDiscovered(format!(
113-
"`.well-known` contains an invalid `sliding_sync_proxy` entry ({e})"
114-
))
115-
})?;
130+
let url =
131+
Url::parse(&info.url).map_err(VersionBuilderError::UnparsableSlidingSyncUrl)?;
116132

117133
Version::Proxy { url }
118134
}
119135

120136
Self::DiscoverNative => {
121137
let Some(versions) = versions else {
122-
return Err(Error::VersionCannotBeDiscovered(
123-
"`/versions` is `None`".to_owned(),
124-
));
138+
return Err(VersionBuilderError::MissingVersionsResponse);
125139
};
126140

127141
match versions.unstable_features.get("org.matrix.simplified_msc3575") {
128142
Some(value) if *value => Version::Native,
129-
_ => {
130-
return Err(Error::VersionCannotBeDiscovered(
131-
"`/versions` does not contain `org.matrix.simplified_msc3575` in its \
132-
`unstable_features`"
133-
.to_owned(),
134-
))
135-
}
143+
_ => return Err(VersionBuilderError::NativeVersionIsUnset),
136144
}
137145
}
138146
})
@@ -324,9 +332,11 @@ mod tests {
324332
Mock, ResponseTemplate,
325333
};
326334

327-
use super::{discover_homeserver, get_supported_versions, Error, Version, VersionBuilder};
335+
use super::{discover_homeserver, get_supported_versions, Version, VersionBuilder};
328336
use crate::{
329-
error::Result, sliding_sync::http, test_utils::logged_in_client_with_server,
337+
error::Result,
338+
sliding_sync::{http, VersionBuilderError},
339+
test_utils::logged_in_client_with_server,
330340
SlidingSyncList, SlidingSyncMode,
331341
};
332342

@@ -373,9 +383,7 @@ mod tests {
373383
fn test_version_builder_discover_proxy_no_well_known() {
374384
assert_matches!(
375385
VersionBuilder::DiscoverProxy.build(None, None),
376-
Err(Error::VersionCannotBeDiscovered(msg)) => {
377-
assert_eq!(msg, "`.well-known` is `None`");
378-
}
386+
Err(VersionBuilderError::WellKnownNotSet)
379387
);
380388
}
381389

@@ -388,9 +396,7 @@ mod tests {
388396

389397
assert_matches!(
390398
VersionBuilder::DiscoverProxy.build(Some(&response), None),
391-
Err(Error::VersionCannotBeDiscovered(msg)) => {
392-
assert_eq!(msg, "`.well-known` does not contain a `sliding_sync_proxy` entry");
393-
}
399+
Err(VersionBuilderError::NoSlidingSyncInWellKnown)
394400
);
395401
}
396402

@@ -404,8 +410,8 @@ mod tests {
404410

405411
assert_matches!(
406412
VersionBuilder::DiscoverProxy.build(Some(&response), None),
407-
Err(Error::VersionCannotBeDiscovered(msg)) => {
408-
assert_eq!(msg, "`.well-known` contains an invalid `sliding_sync_proxy` entry (relative URL without a base)");
413+
Err(VersionBuilderError::UnparsableSlidingSyncUrl(err)) => {
414+
assert_eq!(err.to_string(), "relative URL without a base");
409415
}
410416
);
411417
}
@@ -425,9 +431,7 @@ mod tests {
425431
fn test_version_builder_discover_native_no_supported_versions() {
426432
assert_matches!(
427433
VersionBuilder::DiscoverNative.build(None, None),
428-
Err(Error::VersionCannotBeDiscovered(msg)) => {
429-
assert_eq!(msg, "`/versions` is `None`".to_owned());
430-
}
434+
Err(VersionBuilderError::MissingVersionsResponse)
431435
);
432436
}
433437

@@ -438,12 +442,7 @@ mod tests {
438442

439443
assert_matches!(
440444
VersionBuilder::DiscoverNative.build(None, Some(&response)),
441-
Err(Error::VersionCannotBeDiscovered(msg)) => {
442-
assert_eq!(
443-
msg,
444-
"`/versions` does not contain `org.matrix.simplified_msc3575` in its `unstable_features`".to_owned()
445-
);
446-
}
445+
Err(VersionBuilderError::NativeVersionIsUnset)
447446
);
448447
}
449448

‎crates/matrix-sdk/src/sliding_sync/error.rs‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,6 @@ pub enum Error {
1212
#[error("Sliding sync version is missing")]
1313
VersionIsMissing,
1414

15-
/// Sliding sync version has been configured to be discovered,
16-
/// but it has failed. See [`crate::sliding_sync::Version`].
17-
#[error("Sliding sync version cannot be discovered: {0}")]
18-
VersionCannotBeDiscovered(String),
19-
2015
/// The response we've received from the server can't be parsed or doesn't
2116
/// match up with the current expectations on the client side. A
2217
/// `sync`-restart might be required.

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ use tracing::{debug, error, info, instrument, trace, warn, Instrument, Span};
5050

5151
#[cfg(feature = "e2e-encryption")]
5252
use self::utils::JoinHandleExt as _;
53-
pub use self::{builder::*, error::*, list::*, room::*};
53+
pub use self::{builder::*, client::VersionBuilderError, error::*, list::*, room::*};
5454
use self::{
5555
cache::restore_sliding_sync_state,
5656
client::SlidingSyncResponseProcessor,
@@ -74,6 +74,8 @@ pub(super) struct SlidingSyncInner {
7474
/// Used to distinguish different connections to the sliding sync proxy.
7575
id: String,
7676

77+
/// Either an overridden sliding sync [`Version`], or one inherited from the
78+
/// client.
7779
version: Version,
7880

7981
/// The HTTP Matrix client.
@@ -2003,7 +2005,7 @@ mod tests {
20032005
assert_matches!(sync.version(), Version::Native);
20042006
}
20052007

2006-
// Sliding can override the configuration from the client.
2008+
// Sliding sync can override the configuration from the client.
20072009
{
20082010
let url = Url::parse("https://bar.matrix/").unwrap();
20092011
let sync = client

0 commit comments

Comments
 (0)