From 0dee880cd0d0437bb79949a9f89a387e63a0f7d8 Mon Sep 17 00:00:00 2001
From: Doug <douglase@element.io>
Date: Thu, 7 Jul 2022 17:15:12 +0100
Subject: [PATCH] Address PR comments.

---
 .../src/authentication_service.rs             | 27 ++++++-------------
 bindings/matrix-sdk-ffi/src/client.rs         |  2 +-
 crates/matrix-sdk/Cargo.toml                  |  2 +-
 crates/matrix-sdk/src/client/mod.rs           |  5 ++--
 4 files changed, 13 insertions(+), 23 deletions(-)

diff --git a/bindings/matrix-sdk-ffi/src/authentication_service.rs b/bindings/matrix-sdk-ffi/src/authentication_service.rs
index 50294b520..7ed148d82 100644
--- a/bindings/matrix-sdk-ffi/src/authentication_service.rs
+++ b/bindings/matrix-sdk-ffi/src/authentication_service.rs
@@ -6,11 +6,7 @@ use super::{client::Client, client_builder::ClientBuilder};
 
 pub struct AuthenticationService {
     base_path: String,
-    client_container: RwLock<ClientContainer>,
-}
-
-struct ClientContainer {
-    client: Option<Arc<Client>>,
+    client: RwLock<Option<Arc<Client>>>,
 }
 
 #[derive(Debug, thiserror::Error)]
@@ -30,28 +26,23 @@ impl From<anyhow::Error> for AuthenticationError {
 impl AuthenticationService {
     /// Creates a new service to authenticate a user with.
     pub fn new(base_path: String) -> Self {
-        AuthenticationService {
-            base_path,
-            client_container: RwLock::new(ClientContainer { client: None }),
-        }
+        AuthenticationService { base_path, client: RwLock::new(None) }
     }
 
     /// The currently configured homeserver.
     pub fn homeserver(&self) -> Result<String, AuthenticationError> {
-        self.client_container
+        self.client
             .read()
-            .client
             .as_ref()
             .ok_or(AuthenticationError::ClientMissing)
             .map(|client| client.homeserver())
     }
 
-    /// The OIDC Provider that is trusted by the homeserver. `nil` when
+    /// The OIDC Provider that is trusted by the homeserver. `None` when
     /// not configured.
     pub fn authentication_issuer(&self) -> Result<Option<String>, AuthenticationError> {
-        self.client_container
+        self.client
             .read()
-            .client
             .as_ref()
             .ok_or(AuthenticationError::ClientMissing)
             .map(|client| client.authentication_issuer())
@@ -59,9 +50,8 @@ impl AuthenticationService {
 
     /// Whether the current homeserver supports the password login flow.
     pub fn supports_password_login(&self) -> Result<bool, AuthenticationError> {
-        self.client_container
+        self.client
             .read()
-            .client
             .as_ref()
             .ok_or(AuthenticationError::ClientMissing)
             .and_then(|client| client.supports_password_login().map_err(AuthenticationError::from))
@@ -77,8 +67,7 @@ impl AuthenticationService {
             .build()
             .map_err(AuthenticationError::from)?;
 
-        let mut client_container = self.client_container.write();
-        client_container.client = Some(client);
+        *self.client.write() = Some(client);
         Ok(())
     }
 
@@ -88,7 +77,7 @@ impl AuthenticationService {
         username: String,
         password: String,
     ) -> Result<Arc<Client>, AuthenticationError> {
-        match self.client_container.read().client.as_ref() {
+        match self.client.read().as_ref() {
             Some(client) => {
                 let homeserver_url = client.homeserver();
 
diff --git a/bindings/matrix-sdk-ffi/src/client.rs b/bindings/matrix-sdk-ffi/src/client.rs
index 9c6728a1b..0dda08d72 100644
--- a/bindings/matrix-sdk-ffi/src/client.rs
+++ b/bindings/matrix-sdk-ffi/src/client.rs
@@ -77,7 +77,7 @@ impl Client {
         RUNTIME.block_on(async move { self.client.homeserver().await.to_string() })
     }
 
-    /// The OIDC Provider that is trusted by the homeserver. `nil` when
+    /// The OIDC Provider that is trusted by the homeserver. `None` when
     /// not configured.
     pub fn authentication_issuer(&self) -> Option<String> {
         RUNTIME.block_on(async move {
diff --git a/crates/matrix-sdk/Cargo.toml b/crates/matrix-sdk/Cargo.toml
index fe5f6ae1b..ed129ae40 100644
--- a/crates/matrix-sdk/Cargo.toml
+++ b/crates/matrix-sdk/Cargo.toml
@@ -116,7 +116,7 @@ features = ["client-api-c", "compat", "rand", "unstable-msc2448"]
 [dependencies.ruma-client-api]
 git = "https://github.com/ruma/ruma"
 rev = "96155915f"
-features = ["compat", "unstable-msc2965"]
+features = ["unstable-msc2965"]
 
 [dependencies.tokio-stream]
 version = "0.1.8"
diff --git a/crates/matrix-sdk/src/client/mod.rs b/crates/matrix-sdk/src/client/mod.rs
index 1ddb5c72f..3b0ec5204 100644
--- a/crates/matrix-sdk/src/client/mod.rs
+++ b/crates/matrix-sdk/src/client/mod.rs
@@ -297,9 +297,10 @@ impl Client {
     /// The OIDC Provider that is trusted by the homeserver.
     pub async fn authentication_issuer(&self) -> Option<Url> {
         if let Some(server) = &self.inner.authentication_issuer {
-            return Some(server.read().await.clone());
+            Some(server.read().await.clone())
+        } else {
+            None
         }
-        None
     }
 
     /// Get the user id of the current owner of the client.