feat(crypto) Provide a method to check whether server backup exists w…

…ithout hitting the server every time

crates/matrix-sdk/src/encryption/backups/mod.rs

@@ -20,7 +20,10 @@
 //!
 //! [1]: https://spec.matrix.org/unstable/client-server-api/#server-side-key-backups
 
-use std::collections::{BTreeMap, BTreeSet};
+use std::{
+    collections::{BTreeMap, BTreeSet},
+    sync::{Arc, RwLock},
+};
 
 use futures_core::Stream;
 use futures_util::StreamExt;
@@ -62,9 +65,15 @@ use crate::{
 #[derive(Debug, Clone)]
 pub struct Backups {
     pub(super) client: Client,
+    cached_exists_on_server: Arc<RwLock<Option<bool>>>,
 }
 
 impl Backups {
+    /// Create a new instance of the [`Backups`] manager for this [`Client`].
+    pub fn new(client: Client) -> Self {
+        Backups { client, cached_exists_on_server: Arc::new(RwLock::new(None)) }
+    }
+
     /// Create a new backup version, encrypted with a new backup recovery key.
     ///
     /// The backup recovery key will be persisted locally and shared with
@@ -90,6 +99,7 @@ impl Backups {
     /// # anyhow::Ok(()) };
     /// ```
     pub async fn create(&self) -> Result<(), Error> {
+        self.clear_exists_on_server_cache();
         let _guard = self.client.locks().backup_modify_lock.lock().await;
 
         self.set_state(BackupState::Creating);
@@ -387,7 +397,38 @@ impl Backups {
     /// This method will request info about the current backup from the
     /// homeserver and if a backup exits return `true`, otherwise `false`.
     pub async fn exists_on_server(&self) -> Result<bool, Error> {
-        Ok(self.get_current_version().await?.is_some())
+        let exists_on_server = self.get_current_version().await?.is_some();
+        *self.cached_exists_on_server.write().unwrap() = Some(exists_on_server);
+        Ok(exists_on_server)
+    }
+
+    /// Does a backup exist on the server?
+    ///
+    /// This method is identical to [`Self::exists_on_server`] except that we
+    /// cache the latest answer in memory and only empty the cache if the local
+    /// device adds or deletes a backup itself.
+    ///
+    /// Do not use this method if you need an accurate answer about whether a
+    /// backup exists - instead use [`Self::exists_on_server`]. This method is
+    /// useful when performance is more important than guaranteed accuracy,
+    /// such as when classifying UTDs.
+    pub async fn fast_exists_on_server(&self) -> Result<bool, Error> {
+        // If we have an answer cached, return it immediately
+        {
+            let guard = self.cached_exists_on_server.read().unwrap();
+            if let Some(cached_exists_on_server) = *guard {
+                return Ok(cached_exists_on_server);
+            }
+        }
+
+        // Otherwise, delegate to exists_on_server. It will update the cached value for
+        // us. exists_on_server takes a write lock, which is why we take care to drop
+        // `guard` above before calling this.
+        self.exists_on_server().await
+    }
+
+    fn clear_exists_on_server_cache(&self) {
+        *self.cached_exists_on_server.write().unwrap() = None;
     }
 
     /// Subscribe to a stream that notifies when a room key for the specified
@@ -619,6 +660,8 @@ impl Backups {
     }
 
     async fn delete_backup_from_server(&self, version: String) -> Result<(), Error> {
+        self.clear_exists_on_server_cache();
+
         let request = ruma::api::client::backup::delete_backup_version::v3::Request::new(version);
 
         match self.client.send(request, Default::default()).await {
@@ -1135,6 +1178,23 @@ mod test {
         assert!(exists, "We should deduce that a backup exists on the server");
     }
 
+    #[async_test]
+    async fn test_repeated_calls_to_exists_on_server_makes_repeated_requests() {
+        let server = MatrixMockServer::new().await;
+        let client = server.client_builder().build().await;
+
+        // Expect 2 requests to the server
+        server.mock_room_keys_version().exists().expect(2).mount().await;
+
+        let backups = client.encryption().backups();
+
+        // Call exists_on_server twice
+        backups.exists_on_server().await.unwrap();
+        let exists = backups.exists_on_server().await.unwrap();
+
+        assert!(exists, "We should deduce that a backup exists on the server");
+    }
+
     #[async_test]
     async fn test_when_no_backup_exists_then_exists_on_server_returns_false() {
         let server = MatrixMockServer::new().await;
@@ -1176,21 +1236,149 @@ mod test {
         }
     }
 
+    #[async_test]
+    async fn test_when_a_backup_exists_then_fast_exists_on_server_returns_true() {
+        let server = MatrixMockServer::new().await;
+        let client = server.client_builder().build().await;
+
+        server.mock_room_keys_version().exists().expect(1).mount().await;
+
+        let exists = client
+            .encryption()
+            .backups()
+            .fast_exists_on_server()
+            .await
+            .expect("We should be able to check if backups exist on the server");
+
+        assert!(exists, "We should deduce that a backup exists on the server");
+    }
+
+    #[async_test]
+    async fn test_when_no_backup_exists_then_fast_exists_on_server_returns_false() {
+        let server = MatrixMockServer::new().await;
+        let client = server.client_builder().build().await;
+
+        server.mock_room_keys_version().none().expect(1).mount().await;
+
+        let exists = client
+            .encryption()
+            .backups()
+            .fast_exists_on_server()
+            .await
+            .expect("We should be able to check if backups exist on the server");
+
+        assert!(!exists, "We should deduce that no backup exists on the server");
+    }
+
+    #[async_test]
+    async fn test_when_server_returns_an_error_then_fast_exists_on_server_returns_an_error() {
+        let server = MatrixMockServer::new().await;
+        let client = server.client_builder().build().await;
+
+        {
+            let _scope =
+                server.mock_room_keys_version().error429().expect(1).mount_as_scoped().await;
+
+            client.encryption().backups().fast_exists_on_server().await.expect_err(
+                "If the /version endpoint returns a non 404 error we should throw an error",
+            );
+        }
+
+        {
+            let _scope =
+                server.mock_room_keys_version().error404().expect(1).mount_as_scoped().await;
+
+            client.encryption().backups().fast_exists_on_server().await.expect_err(
+                "If the /version endpoint returns a non-Matrix 404 error we should throw an error",
+            );
+        }
+    }
+
+    #[async_test]
+    async fn test_repeated_calls_to_fast_exists_on_server_do_not_make_additional_requests() {
+        let server = MatrixMockServer::new().await;
+        let client = server.client_builder().build().await;
+
+        // Create a mock stating that the request should only be made once
+        server.mock_room_keys_version().exists().expect(1).mount().await;
+
+        let backups = client.encryption().backups();
+
+        // Call fast_exists_on_server several times
+        backups.fast_exists_on_server().await.unwrap();
+        backups.fast_exists_on_server().await.unwrap();
+        backups.fast_exists_on_server().await.unwrap();
+
+        let exists = backups
+            .fast_exists_on_server()
+            .await
+            .expect("We should be able to check if backups exist on the server");
+
+        assert!(exists, "We should deduce that a backup exists on the server");
+
+        // We check expectations here, confirming that only one call was made
+    }
+
+    #[async_test]
+    async fn test_adding_a_backup_invalidates_fast_exists_on_server_cache() {
+        let server = MatrixMockServer::new().await;
+        let client = server.client_builder().build().await;
+        let backups = client.encryption().backups();
+
+        {
+            let _scope = server.mock_room_keys_version().none().expect(1).mount_as_scoped().await;
+
+            // Call fast_exists_on_server to fill the cache
+            let exists = backups.fast_exists_on_server().await.unwrap();
+            assert!(!exists, "No backup exists at this point");
+        }
+
+        // Create a new backup. Should invalidate the cache
+        server.mock_add_room_keys_version().ok().expect(1).mount().await;
+        backups.create().await.expect("Failed to create a backup");
+
+        server.mock_room_keys_version().exists().expect(1).mount().await;
+        let exists = backups
+            .fast_exists_on_server()
+            .await
+            .expect("We should be able to check if backups exist on the server");
+
+        assert!(exists, "But now a backup does exist");
+    }
+
+    #[async_test]
+    async fn test_removing_a_backup_invalidates_fast_exists_on_server_cache() {
+        let server = MatrixMockServer::new().await;
+        let client = server.client_builder().build().await;
+        let backups = client.encryption().backups();
+
+        {
+            let _scope = server.mock_room_keys_version().exists().expect(1).mount_as_scoped().await;
+
+            // Call fast_exists_on_server to fill the cache
+            let exists = backups.fast_exists_on_server().await.unwrap();
+            assert!(exists, "A backup exists at this point");
+        }
+
+        // Delete the backup. Should invalidate the cache
+        server.mock_delete_room_keys_version().ok().expect(1).mount().await;
+        backups.delete_backup_from_server("1".to_owned()).await.expect("Failed to delete a backup");
+
+        server.mock_room_keys_version().none().expect(1).mount().await;
+        let exists = backups
+            .fast_exists_on_server()
+            .await
+            .expect("We should be able to check if backups exist on the server");
+
+        assert!(!exists, "But now there is no backup");
+    }
+
     #[async_test]
     async fn test_waiting_for_steady_state_resets_the_delay() {
-        let server = MockServer::start().await;
-        let client = logged_in_client(Some(server.uri())).await;
+        let server = MatrixMockServer::new().await;
+        let client = server.client_builder().build().await;
 
-        Mock::given(method("POST"))
-            .and(path("_matrix/client/unstable/room_keys/version"))
-            .and(header("authorization", "Bearer 1234"))
-            .respond_with(ResponseTemplate::new(200).set_body_json(json!({
-              "version": "1"
-            })))
-            .expect(1)
-            .named("POST for the backup creation")
-            .mount(&server)
-            .await;
+        server.mock_add_room_keys_version().ok().expect(1).mount().await;
 
         client
             .encryption()
@@ -1247,7 +1435,5 @@ mod test {
             { client.inner.e2ee.backup_state.upload_delay.read().unwrap().to_owned() };
 
         assert_eq!(old_duration, current_duration);
-
-        server.verify().await;
     }
 }

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

@@ -1439,7 +1439,7 @@ impl Encryption {
 
     /// Get the backups manager of the client.
     pub fn backups(&self) -> Backups {
-        Backups { client: self.client.to_owned() }
+        Backups::new(self.client.to_owned())
     }
 
     /// Get the recovery manager of the client.

crates/matrix-sdk/src/test_utils/mocks.rs

@@ -590,6 +590,22 @@ impl MatrixMockServer {
             .and(header("authorization", "Bearer 1234"));
         MockEndpoint { mock, server: &self.server, endpoint: RoomKeysVersionEndpoint }
     }
+
+    /// Create a prebuilt mock for adding key storage backups via POST
+    pub fn mock_add_room_keys_version(&self) -> MockEndpoint<'_, AddRoomKeysVersionEndpoint> {
+        let mock = Mock::given(method("POST"))
+            .and(path_regex(r"_matrix/client/v3/room_keys/version"))
+            .and(header("authorization", "Bearer 1234"));
+        MockEndpoint { mock, server: &self.server, endpoint: AddRoomKeysVersionEndpoint }
+    }
+
+    /// Create a prebuilt mock for adding key storage backups via POST
+    pub fn mock_delete_room_keys_version(&self) -> MockEndpoint<'_, DeleteRoomKeysVersionEndpoint> {
+        let mock = Mock::given(method("DELETE"))
+            .and(path_regex(r"_matrix/client/v3/room_keys/version/[^/]*"))
+            .and(header("authorization", "Bearer 1234"));
+        MockEndpoint { mock, server: &self.server, endpoint: DeleteRoomKeysVersionEndpoint }
+    }
 }
 
 /// Parameter to [`MatrixMockServer::sync_room`].
@@ -1534,7 +1550,8 @@ impl<'a> MockEndpoint<'a, PublicRoomsEndpoint> {
     }
 }
 
-/// A prebuilt mock for `room_keys/version`: storage ("backup") of room keys.
+/// A prebuilt mock for `GET room_keys/version`: storage ("backup") of room
+/// keys.
 pub struct RoomKeysVersionEndpoint;
 
 impl<'a> MockEndpoint<'a, RoomKeysVersionEndpoint> {
@@ -1578,3 +1595,34 @@ impl<'a> MockEndpoint<'a, RoomKeysVersionEndpoint> {
         MatrixMock { server: self.server, mock }
     }
 }
+
+/// A prebuilt mock for `POST room_keys/version`: adding room key backups.
+pub struct AddRoomKeysVersionEndpoint;
+
+impl<'a> MockEndpoint<'a, AddRoomKeysVersionEndpoint> {
+    /// Returns an endpoint that says there is a single room keys backup
+    pub fn ok(self) -> MatrixMock<'a> {
+        let mock = self
+            .mock
+            .respond_with(ResponseTemplate::new(200).set_body_json(json!({
+              "version": "1"
+            })))
+            .named("POST for the backup creation");
+        MatrixMock { server: self.server, mock }
+    }
+}
+
+/// A prebuilt mock for `DELETE room_keys/version/xxx`: deleting room key
+/// backups.
+pub struct DeleteRoomKeysVersionEndpoint;
+
+impl<'a> MockEndpoint<'a, DeleteRoomKeysVersionEndpoint> {
+    /// Returns an endpoint that says there is a single room keys backup
+    pub fn ok(self) -> MatrixMock<'a> {
+        let mock = self
+            .mock
+            .respond_with(ResponseTemplate::new(200).set_body_json(json!({})))
+            .named("DELETE for the backup deletion");
+        MatrixMock { server: self.server, mock }
+    }
+}