-
Notifications
You must be signed in to change notification settings - Fork 330
feat(sdk,ffi): Add server_vendor_info method to matrix-sdk with automatic logging in FFI #5515
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
31c0d8f
2929ffa
7bed5ea
80d868b
9a15775
f5669ce
c89a9cc
d81a541
7d13bd6
2e3f822
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1608,6 +1608,15 @@ impl Client { | |
Ok(self.inner.server_versions().await?.contains(&ruma::api::MatrixVersion::V1_13)) | ||
} | ||
|
||
/// Get the server version information from the federation API. | ||
/// | ||
/// This method calls the `/_matrix/federation/v1/version` endpoint to get | ||
/// both the server name and version. | ||
pub async fn server_version(&self) -> Result<ServerVersionInfo, ClientError> { | ||
let server_info = self.inner.server_version().await?; | ||
Ok(server_info.into()) | ||
} | ||
|
||
/// Checks if the server supports the LiveKit RTC focus for placing calls. | ||
pub async fn is_livekit_rtc_supported(&self) -> Result<bool, ClientError> { | ||
Ok(self | ||
|
@@ -1797,6 +1806,23 @@ impl From<search_users::v3::Response> for SearchUsersResults { | |
} | ||
} | ||
|
||
#[derive(uniffi::Record)] | ||
pub struct ServerVersionInfo { | ||
/// The server name. | ||
pub server_name: String, | ||
/// The server version. | ||
pub version: String, | ||
} | ||
|
||
impl From<matrix_sdk::ServerVersionInfo> for ServerVersionInfo { | ||
fn from(value: matrix_sdk::ServerVersionInfo) -> Self { | ||
Self { | ||
server_name: value.server_name, | ||
version: value.version, | ||
} | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With my above suggestion we can get rid of this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed the FFI |
||
|
||
#[derive(uniffi::Record)] | ||
pub struct UserProfile { | ||
pub user_id: String, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -62,6 +62,7 @@ use ruma::{ | |
uiaa, | ||
user_directory::search_users, | ||
}, | ||
federation::discovery::get_server_version, | ||
error::FromHttpResponseError, | ||
FeatureFlag, MatrixVersion, OutgoingRequest, SupportedVersions, | ||
}, | ||
|
@@ -151,6 +152,15 @@ pub enum SessionChange { | |
TokensRefreshed, | ||
} | ||
|
||
/// Information about the server version obtained from the federation API. | ||
#[derive(Debug, Clone, PartialEq, Eq)] | ||
pub struct ServerVersionInfo { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we name this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Renamed |
||
/// The server name. | ||
pub server_name: String, | ||
/// The server version. | ||
pub version: String, | ||
} | ||
|
||
/// An async/await enabled Matrix client. | ||
/// | ||
/// All of the state is held in an `Arc` so the `Client` can be cloned freely. | ||
|
@@ -521,6 +531,35 @@ impl Client { | |
Ok(res.capabilities) | ||
} | ||
|
||
/// Get the server version information from the federation API. | ||
/// | ||
/// This method calls the `/_matrix/federation/v1/version` endpoint to get | ||
/// both the server name and version. | ||
bnjbvr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/// | ||
/// # Examples | ||
/// | ||
/// ```no_run | ||
/// # use matrix_sdk::Client; | ||
/// # use url::Url; | ||
/// # async { | ||
/// # let homeserver = Url::parse("http://example.com")?; | ||
/// let client = Client::new(homeserver).await?; | ||
/// | ||
/// let server_info = client.server_version().await?; | ||
/// println!("Server: {}, Version: {}", server_info.server_name, server_info.version); | ||
/// # anyhow::Ok(()) }; | ||
/// ``` | ||
pub async fn server_version(&self) -> HttpResult<ServerVersionInfo> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we rename this method There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Renamed method from |
||
let res = self.send(get_server_version::v1::Request::new()).await?; | ||
|
||
// Extract server info, using defaults if fields are missing | ||
bnjbvr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
let server = res.server.unwrap_or_default(); | ||
let server_name_str = server.name.unwrap_or_else(|| "unknown".to_string()); | ||
let version = server.version.unwrap_or_else(|| "unknown".to_string()); | ||
|
||
Ok(ServerVersionInfo { server_name: server_name_str, version }) | ||
} | ||
|
||
/// Get a copy of the default request config. | ||
/// | ||
/// The default request config is what's used when sending requests if no | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1490,3 +1490,53 @@ async fn test_room_sync_state_after() { | |
let member = room.get_member_no_sync(user_id!("@invited:localhost")).await.unwrap().unwrap(); | ||
assert_eq!(*member.membership(), MembershipState::Leave); | ||
} | ||
|
||
#[async_test] | ||
async fn test_server_version() { | ||
use matrix_sdk::test_utils::mocks::MatrixMockServer; | ||
use serde_json::json; | ||
use wiremock::{matchers::{method, path}, Mock, ResponseTemplate}; | ||
|
||
let server = MatrixMockServer::new().await; | ||
let client = server.client_builder().build().await; | ||
|
||
// Mock the federation version endpoint | ||
Mock::given(method("GET")) | ||
.and(path("/_matrix/federation/v1/version")) | ||
.respond_with(ResponseTemplate::new(200).set_body_json(json!({ | ||
"server": { | ||
"name": "Synapse", | ||
"version": "1.70.0" | ||
} | ||
}))) | ||
.mount(server.server()) | ||
.await; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a new method in Then you can use it in this test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added the new |
||
|
||
let server_info = client.server_version().await.unwrap(); | ||
|
||
assert_eq!(server_info.server_name, "Synapse"); | ||
assert_eq!(server_info.version, "1.70.0"); | ||
} | ||
|
||
#[async_test] | ||
async fn test_server_version_with_missing_fields() { | ||
use matrix_sdk::test_utils::mocks::MatrixMockServer; | ||
use serde_json::json; | ||
use wiremock::{matchers::{method, path}, Mock, ResponseTemplate}; | ||
|
||
let server = MatrixMockServer::new().await; | ||
let client = server.client_builder().build().await; | ||
|
||
// Mock the federation version endpoint with missing fields | ||
Mock::given(method("GET")) | ||
.and(path("/_matrix/federation/v1/version")) | ||
.respond_with(ResponseTemplate::new(200).set_body_json(json!({}))) | ||
.mount(server.server()) | ||
.await; | ||
|
||
let server_info = client.server_version().await.unwrap(); | ||
|
||
// Should use defaults for missing fields | ||
assert_eq!(server_info.server_name, "unknown"); | ||
assert_eq!(server_info.version, "unknown"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather not expose the
server_version
as a public function. Instead, I'd like that just before returning theClient
in theClientBuilder::build()
method in the same crate, the server version be logged at aninfo!
tracing level.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the public
server_version
method from FFI and instead added automatic server version logging in theClientBuilder::build()
method atinfo!
tracing level as requested in commit f5669ce.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I was actually thinking about using this in the FFI so that we could label rageshakes with the server name when it isn't synapse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops sorry, I imagined that it would be sufficient to log it when building the client, but we can reinstate it if you'd like to use specifically as a rageshake tag/marker 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot Can you reinstate this method please? also don't add a new
ServerVersionInfo
, but reuse the one from the SDK, annotated with theuniffi::Record
derive macro only if theuniffi
feature has been enabled.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reinstated the
server_vendor_info
method in FFI and reused the SDK struct with conditionaluniffi::Record
derive as requested in commit c89a9cc.