Merge branch 'feat/appservice-client-config'

matrix_sdk/Cargo.toml

@@ -26,7 +26,7 @@ rustls-tls = ["reqwest/rustls-tls"]
 socks = ["reqwest/socks"]
 sso_login = ["warp", "rand", "tokio-stream"]
 require_auth_for_profile_requests = []
-appservice = ["matrix-sdk-common/appservice", "serde_yaml"]
+appservice = ["matrix-sdk-common/appservice"]
 
 docs = ["encryption", "sled_cryptostore", "sled_state_store", "sso_login"]
 
@@ -41,7 +41,6 @@ url = "2.2.0"
 zeroize = "1.2.0"
 mime = "0.3.16"
 rand = { version = "0.8.2", optional = true }
-serde_yaml = { version = "0.8", optional = true }
 bytes = "1.0.1"
 
 matrix-sdk-common = { version = "0.2.0", path = "../matrix_sdk_common" }

matrix_sdk/src/client.rs

@@ -304,6 +304,11 @@ impl ClientConfig {
         self
     }
 
+    /// Get the [`RequestConfig`]
+    pub fn get_request_config(&self) -> &RequestConfig {
+        &self.request_config
+    }
+
     /// Specify a client to handle sending requests and receiving responses.
     ///
     /// Any type that implements the `HttpSend` trait can be used to

matrix_sdk/src/http_client.rs

@@ -133,7 +133,7 @@ impl HttpClient {
         let request = if !self.request_config.assert_identity {
             self.try_into_http_request(request, session, config).await?
         } else {
-            self.try_into_http_request_with_identy_assertion(request, session, config).await?
+            self.try_into_http_request_with_identity_assertion(request, session, config).await?
         };
 
         self.inner.send_request(request, config).await
@@ -180,7 +180,7 @@ impl HttpClient {
     }
 
     #[cfg(feature = "appservice")]
-    async fn try_into_http_request_with_identy_assertion<Request: OutgoingRequest>(
+    async fn try_into_http_request_with_identity_assertion<Request: OutgoingRequest>(
         &self,
         request: Request,
         session: Arc<RwLock<Option<Session>>>,

matrix_sdk/src/lib.rs

@@ -52,8 +52,7 @@
 //! synapse configuration `require_auth_for_profile_requests`. Enabled by
 //! default.
 //! * `appservice`: Enables low-level appservice functionality. For an
-//!   high-level API there's the
-//! `matrix-sdk-appservice` crate
+//!   high-level API there's the `matrix-sdk-appservice` crate
 
 #![deny(
     missing_debug_implementations,

matrix_sdk_appservice/Cargo.toml

@@ -16,6 +16,7 @@ docs = []
 [dependencies]
 actix-rt = { version = "2", optional = true }
 actix-web = { version = "4.0.0-beta.6", optional = true }
+dashmap = "4"
 futures = "0.3"
 futures-util = "0.3"
 http = "0.2"

matrix_sdk_appservice/examples/actix_autojoin.rs

@@ -34,9 +34,10 @@ impl EventHandler for AppserviceEventHandler {
         if let MembershipState::Invite = event.content.membership {
             let user_id = UserId::try_from(event.state_key.clone()).unwrap();
 
-            self.appservice.register(user_id.localpart()).await.unwrap();
+            let mut appservice = self.appservice.clone();
+            appservice.register(user_id.localpart()).await.unwrap();
 
-            let client = self.appservice.client(Some(user_id.localpart())).await.unwrap();
+            let client = appservice.client(Some(user_id.localpart())).await.unwrap();
 
             client.join_room_by_id(room.room_id()).await.unwrap();
         }
@@ -53,7 +54,7 @@ pub async fn main() -> std::io::Result<()> {
     let registration =
         AppserviceRegistration::try_from_yaml_file("./tests/registration.yaml").unwrap();
 
-    let appservice = Appservice::new(homeserver_url, server_name, registration).await.unwrap();
+    let mut appservice = Appservice::new(homeserver_url, server_name, registration).await.unwrap();
 
     let event_handler = AppserviceEventHandler::new(appservice.clone());
 

matrix_sdk_appservice/src/actix.rs

@@ -65,7 +65,7 @@ async fn push_transactions(
         return Ok(HttpResponse::Unauthorized().finish());
     }
 
-    appservice.client(None).await?.receive_transaction(request.incoming).await?;
+    appservice.get_cached_client(None)?.receive_transaction(request.incoming).await?;
 
     Ok(HttpResponse::Ok().json("{}"))
 }

matrix_sdk_appservice/src/error.rs

@@ -16,9 +16,6 @@ use thiserror::Error;
 
 #[derive(Error, Debug)]
 pub enum Error {
-    #[error("tried to run without webserver configured")]
-    RunWithoutServer,
-
     #[error("missing access token")]
     MissingAccessToken,
 
@@ -31,6 +28,9 @@ pub enum Error {
     #[error("no port found")]
     MissingRegistrationPort,
 
+    #[error("no client for localpart found")]
+    NoClientForLocalpart,
+
     #[error(transparent)]
     HttpRequest(#[from] matrix_sdk::FromHttpRequestError),
 

matrix_sdk_appservice/src/lib.rs

@@ -14,8 +14,9 @@
 
 //! Matrix [Application Service] library
 //!
-//! The appservice crate aims to provide a batteries-included experience. That
-//! means that we
+//! The appservice crate aims to provide a batteries-included experience by
+//! being a thin wrapper around the [`matrix_sdk`]. That means that we
+//!
 //! * ship with functionality to configure your webserver crate or simply run
 //!   the webserver for you
 //! * receive and validate requests from the homeserver correctly
@@ -57,7 +58,7 @@
 //!             regex: '@_appservice_.*'
 //!     ")?;
 //!
-//! let appservice = Appservice::new(homeserver_url, server_name, registration).await?;
+//! let mut appservice = Appservice::new(homeserver_url, server_name, registration).await?;
 //! appservice.set_event_handler(Box::new(AppserviceEventHandler)).await?;
 //!
 //! let (host, port) = appservice.registration().get_host_and_port()?;
@@ -81,8 +82,10 @@ use std::{
     fs::File,
     ops::Deref,
     path::PathBuf,
+    sync::Arc,
 };
 
+use dashmap::DashMap;
 use http::Uri;
 #[doc(inline)]
 pub use matrix_sdk::api_appservice as api;
@@ -98,8 +101,7 @@ use matrix_sdk::{
     assign,
     identifiers::{self, DeviceId, ServerNameBox, UserId},
     reqwest::Url,
-    Client, ClientConfig, EventHandler, FromHttpResponseError, HttpError, RequestConfig,
-    ServerError, Session,
+    Client, ClientConfig, EventHandler, FromHttpResponseError, HttpError, ServerError, Session,
 };
 use regex::Regex;
 use tracing::warn;
@@ -173,34 +175,31 @@ impl Deref for AppserviceRegistration {
     }
 }
 
-async fn client_session_with_login_restore(
-    client: &Client,
-    registration: &AppserviceRegistration,
-    localpart: impl AsRef<str> + Into<Box<str>>,
-    server_name: &ServerNameBox,
-) -> Result<()> {
-    let session = Session {
-        access_token: registration.as_token.clone(),
-        user_id: UserId::parse_with_server_name(localpart, server_name)?,
-        device_id: DeviceId::new(),
-    };
-    client.restore_login(session).await?;
+type Localpart = String;
 
-    Ok(())
-}
+/// The main appservice user is the `sender_localpart` from the given
+/// [`AppserviceRegistration`]
+///
+/// Dummy type for shared documentation
+#[allow(dead_code)]
+pub type MainAppserviceUser = ();
 
 /// Appservice
 #[derive(Debug, Clone)]
 pub struct Appservice {
     homeserver_url: Url,
     server_name: ServerNameBox,
-    registration: AppserviceRegistration,
-    client_sender_localpart: Client,
+    registration: Arc<AppserviceRegistration>,
+    clients: Arc<DashMap<Localpart, Client>>,
 }
 
 impl Appservice {
     /// Create new Appservice
     ///
+    /// Also creates and caches a [`Client`] with the [`MainAppserviceUser`].
+    /// The default [`ClientConfig`] is used, if you want to customize it
+    /// use [`Self::new_with_client_config()`] instead.
+    ///
     /// # Arguments
     ///
     /// * `homeserver_url` - The homeserver that the client should connect to.
@@ -215,28 +214,46 @@ impl Appservice {
         server_name: impl TryInto<ServerNameBox, Error = identifiers::Error>,
         registration: AppserviceRegistration,
     ) -> Result<Self> {
-        let homeserver_url = homeserver_url.try_into()?;
-        let server_name = server_name.try_into()?;
-
-        let client_sender_localpart = Client::new(homeserver_url.clone())?;
-
-        client_session_with_login_restore(
-            &client_sender_localpart,
-            &registration,
-            registration.sender_localpart.as_ref(),
-            &server_name,
+        let appservice = Self::new_with_client_config(
+            homeserver_url,
+            server_name,
+            registration,
+            ClientConfig::default(),
         )
         .await?;
 
-        Ok(Appservice { homeserver_url, server_name, registration, client_sender_localpart })
+        Ok(appservice)
     }
 
-    /// Get a [`Client`]
+    /// Same as [`Self::new()`] but lets you provide a [`ClientConfig`] for the
+    /// [`Client`]
+    pub async fn new_with_client_config(
+        homeserver_url: impl TryInto<Url, Error = url::ParseError>,
+        server_name: impl TryInto<ServerNameBox, Error = identifiers::Error>,
+        registration: AppserviceRegistration,
+        client_config: ClientConfig,
+    ) -> Result<Self> {
+        let homeserver_url = homeserver_url.try_into()?;
+        let server_name = server_name.try_into()?;
+        let registration = Arc::new(registration);
+        let clients = Arc::new(DashMap::new());
+
+        let appservice = Appservice { homeserver_url, server_name, registration, clients };
+
+        // we cache the [`MainAppserviceUser`] by default
+        appservice.client_with_config(None, client_config).await?;
+
+        Ok(appservice)
+    }
+
+    /// Create a [`Client`]
     ///
-    /// Will return a `Client` that's configured to [assert the identity] on all
-    /// outgoing homeserver requests if `localpart` is given. If not given
-    /// the `Client` will use the main user associated with this appservice,
-    /// that is the `sender_localpart` in the [`AppserviceRegistration`]
+    /// Will create and return a [`Client`] that's configured to [assert the
+    /// identity] on all outgoing homeserver requests if `localpart` is
+    /// given. If not given the [`Client`] will use the [`MainAppserviceUser`].
+    ///
+    /// This method is a singleton that saves the client internally for re-use
+    /// based on the `localpart`.
     ///
     /// # Arguments
     ///
@@ -245,25 +262,46 @@ impl Appservice {
     /// [registration]: https://matrix.org/docs/spec/application_service/r0.1.2#registration
     /// [assert the identity]: https://matrix.org/docs/spec/application_service/r0.1.2#identity-assertion
     pub async fn client(&self, localpart: Option<&str>) -> Result<Client> {
+        let client = self.client_with_config(localpart, ClientConfig::default()).await?;
+
+        Ok(client)
+    }
+
+    /// Same as [`Self::client`] but with the ability to pass in a
+    /// [`ClientConfig`]
+    ///
+    /// Since this method is a singleton follow-up calls with different
+    /// [`ClientConfig`]s will be ignored.
+    pub async fn client_with_config(
+        &self,
+        localpart: Option<&str>,
+        config: ClientConfig,
+    ) -> Result<Client> {
         let localpart = localpart.unwrap_or_else(|| self.registration.sender_localpart.as_ref());
 
-        // The `as_token` in the `Session` maps to the main appservice user
+        let client = if let Some(client) = self.clients.get(localpart) {
+            client.clone()
+        } else {
+            let user_id = UserId::parse_with_server_name(localpart, &self.server_name)?;
+
+            // The `as_token` in the `Session` maps to the [`MainAppserviceUser`]
             // (`sender_localpart`) by default, so we don't need to assert identity
             // in that case
-        let client = if localpart == self.registration.sender_localpart {
-            self.client_sender_localpart.clone()
-        } else {
-            let request_config = RequestConfig::default().assert_identity();
-            let config = ClientConfig::default().request_config(request_config);
+            if localpart != self.registration.sender_localpart {
+                config.get_request_config().assert_identity();
+            }
+
             let client = Client::new_with_config(self.homeserver_url.clone(), config)?;
 
-            client_session_with_login_restore(
-                &client,
-                &self.registration,
-                localpart,
-                &self.server_name,
-            )
-            .await?;
+            let session = Session {
+                access_token: self.registration.as_token.clone(),
+                user_id: user_id.clone(),
+                // TODO: expose & proper E2EE
+                device_id: DeviceId::new(),
+            };
+
+            client.restore_login(session).await?;
+            self.clients.insert(localpart.to_owned(), client.clone());
 
             client
         };
@@ -271,9 +309,26 @@ impl Appservice {
         Ok(client)
     }
 
+    /// Get cached [`Client`]
+    ///
+    /// Will return the client for the given `localpart` if previously
+    /// constructed with [`Self::client()`] or [`Self::client_with_config()`].
+    /// If no client for the `localpart` is found it will return an Error.
+    pub fn get_cached_client(&self, localpart: Option<&str>) -> Result<Client> {
+        let localpart = localpart.unwrap_or_else(|| self.registration.sender_localpart.as_ref());
+
+        let entry = self.clients.get(localpart).ok_or(Error::NoClientForLocalpart)?;
+
+        Ok(entry.value().clone())
+    }
+
     /// Convenience wrapper around [`Client::set_event_handler()`]
-    pub async fn set_event_handler(&self, handler: Box<dyn EventHandler>) -> Result<()> {
+    ///
+    /// Attaches the event handler to [`Self::client()`] with `None` as
+    /// `localpart`
+    pub async fn set_event_handler(&mut self, handler: Box<dyn EventHandler>) -> Result<()> {
         let client = self.client(None).await?;
+
         client.set_event_handler(handler).await;
 
         Ok(())
@@ -286,7 +341,7 @@ impl Appservice {
     ///
     /// * `localpart` - The localpart of the user to register. Must be covered
     ///   by the namespaces in the [`Registration`] in order to succeed.
-    pub async fn register(&self, localpart: impl AsRef<str>) -> Result<()> {
+    pub async fn register(&mut self, localpart: impl AsRef<str>) -> Result<()> {
         let request = assign!(RegistrationRequest::new(), {
             username: Some(localpart.as_ref()),
             login_type: Some(&LoginType::ApplicationService),

matrix_sdk_appservice/tests/actix.rs

@@ -12,7 +12,7 @@ mod actix {
         Appservice::new(
             mockito::server_url().as_ref(),
             "test.local",
-            AppserviceRegistration::try_from_yaml_file("./tests/registration.yaml").unwrap(),
+            AppserviceRegistration::try_from_yaml_str(include_str!("./registration.yaml")).unwrap(),
         )
         .await
         .unwrap()

matrix_sdk_appservice/tests/tests.rs

@@ -59,7 +59,7 @@ fn member_json() -> serde_json::Value {
 
 #[async_test]
 async fn test_event_handler() -> Result<()> {
-    let appservice = appservice(None).await?;
+    let mut appservice = appservice(None).await?;
 
     struct Example {}
 
@@ -94,7 +94,7 @@ async fn test_event_handler() -> Result<()> {
 
 #[async_test]
 async fn test_transaction() -> Result<()> {
-    let appservice = appservice(None).await?;
+    let mut appservice = appservice(None).await?;
 
     let event = serde_json::from_value::<AnyStateEvent>(member_json()).unwrap();
     let event: Raw<AnyRoomEvent> = AnyRoomEvent::State(event).into();