feat(event cache): enable storage by default \o/

Author: bnjbvr

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

@@ -272,10 +272,6 @@ pub struct ClientBuilder {
     room_key_recipient_strategy: CollectStrategy,
     decryption_trust_requirement: TrustRequirement,
     request_config: Option<RequestConfig>,
-
-    /// Whether to enable use of the event cache store, for reloading events
-    /// when building timelines et al.
-    use_event_cache_persistent_storage: bool,
 }
 
 #[matrix_sdk_ffi_macros::export]
@@ -306,27 +302,9 @@ impl ClientBuilder {
             room_key_recipient_strategy: Default::default(),
             decryption_trust_requirement: TrustRequirement::Untrusted,
             request_config: Default::default(),
-            use_event_cache_persistent_storage: false,
         })
     }
 
-    /// Whether to use the event cache persistent storage or not.
-    ///
-    /// This is a temporary feature flag, for testing the event cache's
-    /// persistent storage. Follow new developments in https://github.com/matrix-org/matrix-rust-sdk/issues/3280.
-    ///
-    /// This is disabled by default. When disabled, a one-time cleanup is
-    /// performed when creating the client, and it will clear all the events
-    /// previously stored in the event cache.
-    ///
-    /// When enabled, it will attempt to store events in the event cache as
-    /// they're received, and reuse them when reconstructing timelines.
-    pub fn use_event_cache_persistent_storage(self: Arc<Self>, value: bool) -> Arc<Self> {
-        let mut builder = unwrap_or_clone_arc(self);
-        builder.use_event_cache_persistent_storage = value;
-        Arc::new(builder)
-    }
-
     pub fn cross_process_store_locks_holder_name(
         self: Arc<Self>,
         holder_name: String,
@@ -649,19 +627,6 @@ impl ClientBuilder {
 
         let sdk_client = inner_builder.build().await?;
 
-        if builder.use_event_cache_persistent_storage {
-            // Enable the persistent storage \o/
-            sdk_client.event_cache().enable_storage()?;
-        } else {
-            // Get rid of all the previous events, if any.
-            let store = sdk_client
-                .event_cache_store()
-                .lock()
-                .await
-                .map_err(EventCacheError::LockingStorage)?;
-            store.clear_all_rooms_chunks().await.map_err(EventCacheError::Storage)?;
-        }
-
         Ok(Arc::new(
             Client::new(sdk_client, builder.enable_oidc_refresh_lock, builder.session_delegate)
                 .await?,

crates/matrix-sdk-ui/src/timeline/builder.rs

@@ -157,7 +157,6 @@ impl TimelineBuilder {
         let event_cache = client.event_cache();
 
         // Subscribe the event cache to sync responses, in case we hadn't done it yet.
-        event_cache.enable_storage()?;
         event_cache.subscribe()?;
 
         let (room_event_cache, event_cache_drop) = room.event_cache().await?;

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

@@ -168,23 +168,6 @@ impl EventCache {
         }
     }
 
-    /// Enable storing updates to storage, and reload events from storage.
-    ///
-    /// Has an effect only the first time it's called. It's safe to call it
-    /// multiple times.
-    pub fn enable_storage(&self) -> Result<()> {
-        let _ = self.inner.store.get_or_try_init::<_, EventCacheError>(|| {
-            let client = self.inner.client()?;
-            Ok(client.event_cache_store().clone())
-        })?;
-        Ok(())
-    }
-
-    /// Check whether the storage is enabled or not.
-    pub fn has_storage(&self) -> bool {
-        self.inner.has_storage()
-    }
-
     /// Starts subscribing the [`EventCache`] to sync responses, if not done
     /// before.
     ///
@@ -193,6 +176,13 @@ impl EventCache {
     pub fn subscribe(&self) -> Result<()> {
         let client = self.inner.client()?;
 
+        // Initialize storage.
+        let _ = self.inner.store.get_or_try_init::<_, EventCacheError>(|| {
+            let client = self.inner.client()?;
+            Ok(client.event_cache_store().clone())
+        })?;
+
+        // Initialize the drop handles.
         let _ = self.inner.drop_handles.get_or_init(|| {
             // Spawn the task that will listen to all the room updates at once.
             let listen_updates_task = spawn(Self::listen_task(
@@ -487,11 +477,6 @@ impl EventCacheInner {
         self.client.get().ok_or(EventCacheError::ClientDropped)
     }
 
-    /// Has persistent storage been enabled for the event cache?
-    fn has_storage(&self) -> bool {
-        self.store.get().is_some()
-    }
-
     /// Clears all the room's data.
     async fn clear_all_rooms(&self) -> Result<()> {
         // Note: one must NOT clear the `by_room` map, because if something subscribed
@@ -524,9 +509,7 @@ impl EventCacheInner {
         for (room_id, left_room_update) in updates.leave {
             let room = self.for_room(&room_id).await?;
 
-            if let Err(err) =
-                room.inner.handle_left_room_update(self.has_storage(), left_room_update).await
-            {
+            if let Err(err) = room.inner.handle_left_room_update(left_room_update).await {
                 // Non-fatal error, try to continue to the next room.
                 error!("handling left room update: {err}");
             }
@@ -536,9 +519,7 @@ impl EventCacheInner {
         for (room_id, joined_room_update) in updates.join {
             let room = self.for_room(&room_id).await?;
 
-            if let Err(err) =
-                room.inner.handle_joined_room_update(self.has_storage(), joined_room_update).await
-            {
+            if let Err(err) = room.inner.handle_joined_room_update(joined_room_update).await {
                 // Non-fatal error, try to continue to the next room.
                 error!("handling joined room update: {err}");
             }
@@ -716,10 +697,7 @@ mod tests {
 
         room_event_cache
             .inner
-            .handle_joined_room_update(
-                event_cache.inner.has_storage(),
-                JoinedRoomUpdate { account_data, ..Default::default() },
-            )
+            .handle_joined_room_update(JoinedRoomUpdate { account_data, ..Default::default() })
             .await
             .unwrap();
 

crates/matrix-sdk/src/event_cache/room/mod.rs

@@ -278,13 +278,8 @@ impl RoomEventCacheInner {
         }
     }
 
-    pub(super) async fn handle_joined_room_update(
-        &self,
-        has_storage: bool,
-        updates: JoinedRoomUpdate,
-    ) -> Result<()> {
+    pub(super) async fn handle_joined_room_update(&self, updates: JoinedRoomUpdate) -> Result<()> {
         self.handle_timeline(
-            has_storage,
             updates.timeline,
             updates.ephemeral.clone(),
             updates.ambiguity_changes,
@@ -298,88 +293,33 @@ impl RoomEventCacheInner {
 
     async fn handle_timeline(
         &self,
-        has_storage: bool,
         timeline: Timeline,
         ephemeral_events: Vec<Raw<AnySyncEphemeralRoomEvent>>,
         ambiguity_changes: BTreeMap<OwnedEventId, AmbiguityChange>,
     ) -> Result<()> {
-        if !has_storage && timeline.limited {
-            // Ideally we'd try to reconcile existing events against those received in the
-            // timeline, but we're not there yet. In the meanwhile, clear the
-            // items from the room. TODO: implement Smart Matching™.
-            trace!("limited timeline, clearing all previous events and pushing new events");
-
-            self.replace_all_events_by(
-                timeline.events,
-                timeline.prev_batch,
-                ephemeral_events,
-                ambiguity_changes,
-            )
-            .await?;
-        } else {
-            // Add all the events to the backend.
-            trace!("adding new events");
-
-            // Only keep the previous-batch token if we have a limited timeline; otherwise,
-            // we know about all the events, and we don't need to back-paginate,
-            // so we wouldn't make use of the given previous-batch token.
-            let prev_batch = if timeline.limited { timeline.prev_batch } else { None };
-
-            let mut state = self.state.write().await;
-            self.append_events_locked(
-                &mut state,
-                timeline.events,
-                prev_batch,
-                ephemeral_events,
-                ambiguity_changes,
-            )
-            .await?;
-        }
-
-        Ok(())
-    }
+        // Add all the events to the backend.
+        trace!("adding new events");
 
-    pub(super) async fn handle_left_room_update(
-        &self,
-        has_storage: bool,
-        updates: LeftRoomUpdate,
-    ) -> Result<()> {
-        self.handle_timeline(has_storage, updates.timeline, Vec::new(), updates.ambiguity_changes)
-            .await?;
-        Ok(())
-    }
+        // Only keep the previous-batch token if we have a limited timeline; otherwise,
+        // we know about all the events, and we don't need to back-paginate,
+        // so we wouldn't make use of the given previous-batch token.
+        let prev_batch = if timeline.limited { timeline.prev_batch } else { None };
 
-    /// Remove existing events, and append a set of events to the room cache and
-    /// storage, notifying observers.
-    pub(super) async fn replace_all_events_by(
-        &self,
-        sync_timeline_events: Vec<SyncTimelineEvent>,
-        prev_batch: Option<String>,
-        ephemeral_events: Vec<Raw<AnySyncEphemeralRoomEvent>>,
-        ambiguity_changes: BTreeMap<OwnedEventId, AmbiguityChange>,
-    ) -> Result<()> {
-        // Acquire the lock.
         let mut state = self.state.write().await;
-
-        // Reset the room's state.
-        state.reset().await?;
-
-        // Propagate to observers.
-        let _ = self.sender.send(RoomEventCacheUpdate::Clear);
-
-        // Push the new events.
         self.append_events_locked(
             &mut state,
-            sync_timeline_events,
-            prev_batch.clone(),
+            timeline.events,
+            prev_batch,
             ephemeral_events,
             ambiguity_changes,
         )
         .await?;
 
-        // Reset the paginator status to initial.
-        self.paginator.set_idle_state(PaginatorState::Initial, prev_batch, None)?;
+        Ok(())
+    }
 
+    pub(super) async fn handle_left_room_update(&self, updates: LeftRoomUpdate) -> Result<()> {
+        self.handle_timeline(updates.timeline, Vec::new(), updates.ambiguity_changes).await?;
         Ok(())
     }
 
@@ -1092,7 +1032,6 @@ mod tests {
 
         // Don't forget to subscribe and like^W enable storage!
         event_cache.subscribe().unwrap();
-        event_cache.enable_storage().unwrap();
 
         client.base_client().get_or_create_room(room_id, matrix_sdk_base::RoomState::Joined);
         let room = client.get_room(room_id).unwrap();
@@ -1108,7 +1047,7 @@ mod tests {
 
         room_event_cache
             .inner
-            .handle_joined_room_update(true, JoinedRoomUpdate { timeline, ..Default::default() })
+            .handle_joined_room_update(JoinedRoomUpdate { timeline, ..Default::default() })
             .await
             .unwrap();
 
@@ -1164,7 +1103,6 @@ mod tests {
 
         // Don't forget to subscribe and like^W enable storage!
         event_cache.subscribe().unwrap();
-        event_cache.enable_storage().unwrap();
 
         client.base_client().get_or_create_room(room_id, matrix_sdk_base::RoomState::Joined);
         let room = client.get_room(room_id).unwrap();
@@ -1181,7 +1119,7 @@ mod tests {
 
         room_event_cache
             .inner
-            .handle_joined_room_update(true, JoinedRoomUpdate { timeline, ..Default::default() })
+            .handle_joined_room_update(JoinedRoomUpdate { timeline, ..Default::default() })
             .await
             .unwrap();
 
@@ -1297,7 +1235,6 @@ mod tests {
 
         // Don't forget to subscribe and like^W enable storage!
         event_cache.subscribe().unwrap();
-        event_cache.enable_storage().unwrap();
 
         client.base_client().get_or_create_room(room_id, matrix_sdk_base::RoomState::Joined);
         let room = client.get_room(room_id).unwrap();
@@ -1408,7 +1345,6 @@ mod tests {
 
         // Don't forget to subscribe and like^W enable storage!
         event_cache.subscribe().unwrap();
-        event_cache.enable_storage().unwrap();
 
         client.base_client().get_or_create_room(room_id, matrix_sdk_base::RoomState::Joined);
         let room = client.get_room(room_id).unwrap();
@@ -1426,7 +1362,7 @@ mod tests {
         let timeline = Timeline { limited: false, prev_batch: None, events: vec![ev2] };
         room_event_cache
             .inner
-            .handle_joined_room_update(true, JoinedRoomUpdate { timeline, ..Default::default() })
+            .handle_joined_room_update(JoinedRoomUpdate { timeline, ..Default::default() })
             .await
             .unwrap();
 
@@ -1477,7 +1413,6 @@ mod tests {
 
         // Don't forget to subscribe and like^W enable storage!
         event_cache.subscribe().unwrap();
-        event_cache.enable_storage().unwrap();
 
         client.base_client().get_or_create_room(room_id, matrix_sdk_base::RoomState::Joined);
         let room = client.get_room(room_id).unwrap();
@@ -1506,9 +1441,6 @@ mod tests {
         let event_cache = client.event_cache();
         event_cache.subscribe().unwrap();
 
-        let has_storage = true; // for testing purposes only
-        event_cache.enable_storage().unwrap();
-
         client.base_client().get_or_create_room(room_id, matrix_sdk_base::RoomState::Joined);
         let room = client.get_room(room_id).unwrap();
         let (room_event_cache, _drop_handles) = room.event_cache().await.unwrap();
@@ -1519,17 +1451,14 @@ mod tests {
         // prev-batch token.
         room_event_cache
             .inner
-            .handle_joined_room_update(
-                has_storage,
-                JoinedRoomUpdate {
-                    timeline: Timeline {
-                        limited: true,
-                        prev_batch: Some("raclette".to_owned()),
-                        events: vec![f.text_msg("hey yo").into_sync()],
-                    },
-                    ..Default::default()
+            .handle_joined_room_update(JoinedRoomUpdate {
+                timeline: Timeline {
+                    limited: true,
+                    prev_batch: Some("raclette".to_owned()),
+                    events: vec![f.text_msg("hey yo").into_sync()],
                 },
-            )
+                ..Default::default()
+            })
             .await
             .unwrap();
 
@@ -1555,17 +1484,14 @@ mod tests {
         // this time.
         room_event_cache
             .inner
-            .handle_joined_room_update(
-                has_storage,
-                JoinedRoomUpdate {
-                    timeline: Timeline {
-                        limited: false,
-                        prev_batch: Some("fondue".to_owned()),
-                        events: vec![f.text_msg("sup").into_sync()],
-                    },
-                    ..Default::default()
+            .handle_joined_room_update(JoinedRoomUpdate {
+                timeline: Timeline {
+                    limited: false,
+                    prev_batch: Some("fondue".to_owned()),
+                    events: vec![f.text_msg("sup").into_sync()],
                 },
-            )
+                ..Default::default()
+            })
             .await
             .unwrap();