From 4e291205d5fa944d9cd53b4eab48e238bfe2a3e8 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 14 Aug 2024 17:02:01 +0200 Subject: [PATCH] feat(sdk): Remove `NotificationSettings::subscribe_to_changes`. This patch removes `NotificationSettings::subscribe_to_changes` because it's not used anywhere in our code except in tests. It is indeed part of the public API but I'm not aware of anyone using it for the moment. It only adds complexity in the code. --- .../src/notification_settings/mod.rs | 57 ++----------------- .../src/tests/sliding_sync/room.rs | 4 +- 2 files changed, 5 insertions(+), 56 deletions(-) diff --git a/crates/matrix-sdk/src/notification_settings/mod.rs b/crates/matrix-sdk/src/notification_settings/mod.rs index e0c89a9cc..3e3e901cb 100644 --- a/crates/matrix-sdk/src/notification_settings/mod.rs +++ b/crates/matrix-sdk/src/notification_settings/mod.rs @@ -25,10 +25,7 @@ use ruma::{ push::{Action, PredefinedUnderrideRuleId, RuleKind, Ruleset, Tweak}, RoomId, }; -use tokio::sync::{ - broadcast::{self, Receiver}, - RwLock, -}; +use tokio::sync::RwLock; use tracing::{debug, error}; use self::{command::Command, rule_commands::RuleCommands, rules::Rules}; @@ -91,40 +88,29 @@ pub struct NotificationSettings { rules: Arc>, /// Drop guard of event handler for push rules event. _push_rules_event_handler_guard: Arc, - changes_sender: broadcast::Sender<()>, } impl NotificationSettings { - /// Build a new `NotificationSettings`` + /// Build a new `NotificationSettings`. /// /// # Arguments /// /// * `client` - A `Client` used to perform API calls /// * `ruleset` - A `Ruleset` containing account's owner push rules pub(crate) fn new(client: Client, ruleset: Ruleset) -> Self { - let changes_sender = broadcast::Sender::new(100); let rules = Arc::new(RwLock::new(Rules::new(ruleset))); // Listen for PushRulesEvent let push_rules_event_handler_handle = client.add_event_handler({ - let changes_sender = changes_sender.clone(); let rules = Arc::clone(&rules); move |ev: PushRulesEvent| async move { *rules.write().await = Rules::new(ev.content.global); - let _ = changes_sender.send(()); } }); let _push_rules_event_handler_guard = client.event_handler_drop_guard(push_rules_event_handler_handle).into(); - Self { client, rules, _push_rules_event_handler_guard, changes_sender } - } - - /// Subscribe to changes in the `NotificationSettings`. - /// - /// Changes can happen due to local changes or changes in another session. - pub fn subscribe_to_changes(&self) -> Receiver<()> { - self.changes_sender.subscribe() + Self { client, rules, _push_rules_event_handler_guard } } /// Get the user defined notification mode for a room. @@ -528,7 +514,6 @@ mod tests { use matrix_sdk_test::{ async_test, notification_settings::{build_ruleset, get_server_default_ruleset}, - test_json, }; use ruma::{ push::{ @@ -537,16 +522,12 @@ mod tests { }, OwnedRoomId, RoomId, }; - use serde_json::json; - use stream_assert::{assert_next_eq, assert_pending}; - use tokio_stream::wrappers::BroadcastStream; use wiremock::{ - matchers::{header, method, path, path_regex}, + matchers::{method, path, path_regex}, Mock, MockServer, ResponseTemplate, }; use crate::{ - config::SyncSettings, error::NotificationSettingsError, notification_settings::{ IsEncrypted, IsOneToOne, NotificationSettings, RoomNotificationMode, @@ -574,36 +555,6 @@ mod tests { settings.rules.read().await.get_custom_rules_for_room(room_id) } - #[async_test] - async fn subscribe_to_changes() { - let server = MockServer::start().await; - let client = logged_in_client(Some(server.uri())).await; - let settings = client.notification_settings().await; - - Mock::given(method("GET")) - .and(path("/_matrix/client/r0/sync")) - .and(header("authorization", "Bearer 1234")) - .respond_with(ResponseTemplate::new(200).set_body_json(json!({ - "next_batch": "1234", - "account_data": { - "events": [*test_json::PUSH_RULES] - } - }))) - .expect(1) - .mount(&server) - .await; - - let subscriber = settings.subscribe_to_changes(); - let mut stream = BroadcastStream::new(subscriber); - - assert_pending!(stream); - - client.sync_once(SyncSettings::default()).await.unwrap(); - - assert_next_eq!(stream, Ok(())); - assert_pending!(stream); - } - #[async_test] async fn test_get_custom_rules_for_room() { let server = MockServer::start().await; diff --git a/testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs b/testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs index 12f600609..8cf82bbdd 100644 --- a/testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs +++ b/testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs @@ -582,8 +582,6 @@ async fn test_room_notification_count() -> Result<()> { // Now Alice is only interesting in mentions of their name. let settings = alice.notification_settings().await; - let mut settings_changes = settings.subscribe_to_changes(); - tracing::warn!("Updating room notification mode to mentions and keywords only..."); settings .set_room_notification_mode( @@ -594,7 +592,7 @@ async fn test_room_notification_count() -> Result<()> { tracing::warn!("Done!"); // Wait for remote echo. - timeout(Duration::from_secs(3), settings_changes.recv()) + tokio::time::sleep(Duration::from_secs(3)) .await .expect("timeout when waiting for settings update") .expect("should've received echo after updating settings");