Merge pull request #3369 from matrix-org/bnjbvr/get-rid-of-notificationclientbuilder

notification client: get rid of builder
This commit is contained in:
Ivan Enderlin
2024-05-02 09:34:32 +02:00
committed by GitHub
5 changed files with 38 additions and 117 deletions

View File

@@ -37,7 +37,10 @@ use matrix_sdk::{
},
AuthApi, AuthSession, Client as MatrixClient, SessionChange, SessionTokens,
};
use matrix_sdk_ui::notification_client::NotificationProcessSetup as MatrixNotificationProcessSetup;
use matrix_sdk_ui::notification_client::{
NotificationClient as MatrixNotificationClient,
NotificationProcessSetup as MatrixNotificationProcessSetup,
};
use mime::Mime;
use ruma::{
api::client::discovery::discover_homeserver::AuthenticationServerInfo,
@@ -58,7 +61,7 @@ use super::{room::Room, session_verification::SessionVerificationController, RUN
use crate::{
client,
encryption::Encryption,
notification::NotificationClientBuilder,
notification::NotificationClient,
notification_settings::NotificationSettings,
room_directory_search::RoomDirectorySearch,
room_preview::RoomPreview,
@@ -645,8 +648,12 @@ impl Client {
pub async fn notification_client(
self: Arc<Self>,
process_setup: NotificationProcessSetup,
) -> Result<Arc<NotificationClientBuilder>, ClientError> {
NotificationClientBuilder::new(self.clone(), process_setup.into()).await
) -> Result<Arc<NotificationClient>, ClientError> {
Ok(Arc::new(NotificationClient {
inner: MatrixNotificationClient::new((*self.inner).clone(), process_setup.into())
.await?,
_client: self.clone(),
}))
}
pub fn sync_service(&self) -> Arc<SyncServiceBuilder> {

View File

@@ -1,15 +1,11 @@
use std::sync::Arc;
use matrix_sdk_ui::notification_client::{
NotificationClient as MatrixNotificationClient,
NotificationClientBuilder as MatrixNotificationClientBuilder,
NotificationItem as MatrixNotificationItem, NotificationProcessSetup,
NotificationClient as MatrixNotificationClient, NotificationItem as MatrixNotificationItem,
};
use ruma::{EventId, RoomId};
use crate::{
client::Client, error::ClientError, event::TimelineEvent, helpers::unwrap_or_clone_arc,
};
use crate::{client::Client, error::ClientError, event::TimelineEvent};
#[derive(uniffi::Enum)]
pub enum NotificationEvent {
@@ -80,49 +76,16 @@ impl NotificationItem {
}
}
#[derive(Clone, uniffi::Object)]
pub struct NotificationClientBuilder {
client: Arc<Client>,
builder: MatrixNotificationClientBuilder,
}
impl NotificationClientBuilder {
pub(crate) async fn new(
client: Arc<Client>,
process_setup: NotificationProcessSetup,
) -> Result<Arc<Self>, ClientError> {
let builder =
MatrixNotificationClient::builder((*client.inner).clone(), process_setup).await?;
Ok(Arc::new(Self { builder, client }))
}
}
#[uniffi::export]
impl NotificationClientBuilder {
/// Filter out the notification event according to the push rules present in
/// the event.
pub fn filter_by_push_rules(self: Arc<Self>) -> Arc<Self> {
let this = unwrap_or_clone_arc(self);
let builder = this.builder.filter_by_push_rules();
Arc::new(Self { builder, client: this.client })
}
pub fn finish(self: Arc<Self>) -> Arc<NotificationClient> {
let this = unwrap_or_clone_arc(self);
Arc::new(NotificationClient { inner: this.builder.build(), _client: this.client })
}
}
#[derive(uniffi::Object)]
pub struct NotificationClient {
inner: MatrixNotificationClient,
pub(crate) inner: MatrixNotificationClient,
/// A reference to the FFI client.
///
/// Note: we do this to make it so that the FFI `NotificationClient` keeps
/// the FFI `Client` and thus the SDK `Client` alive. Otherwise, we
/// would need to repeat the hack done in the FFI `Client::drop` method.
_client: Arc<Client>,
pub(crate) _client: Arc<Client>,
}
#[uniffi::export(async_runtime = "tokio")]

View File

@@ -87,10 +87,6 @@ pub struct NotificationClient {
/// Is the notification client running on its own process or not?
process_setup: NotificationProcessSetup,
/// Should we try to filter out the notification event according to the push
/// rules?
filter_by_push_rules: bool,
/// A mutex to serialize requests to the notifications sliding sync.
///
/// If several notifications come in at the same time (e.g. network was
@@ -111,12 +107,19 @@ impl NotificationClient {
const CONNECTION_ID: &'static str = "notifications";
const LOCK_ID: &'static str = "notifications";
/// Create a new builder for a notification client.
pub async fn builder(
client: Client,
/// Create a new notification client.
pub async fn new(
parent_client: Client,
process_setup: NotificationProcessSetup,
) -> Result<NotificationClientBuilder, Error> {
NotificationClientBuilder::new(client, process_setup).await
) -> Result<Self, Error> {
let client = parent_client.notification_client().await?;
Ok(NotificationClient {
client,
parent_client,
notification_sync_mutex: AsyncMutex::new(()),
encryption_sync_mutex: AsyncMutex::new(()),
process_setup,
})
}
/// Fetches the content of a notification.
@@ -446,7 +449,7 @@ impl NotificationClient {
};
if let Some(push_actions) = &push_actions {
if self.filter_by_push_rules && !push_actions.iter().any(|a| a.should_notify()) {
if !push_actions.iter().any(|a| a.should_notify()) {
return Ok(NotificationStatus::EventFilteredOut);
}
}
@@ -491,13 +494,10 @@ impl NotificationClient {
timeline_event = decrypted_event;
}
if self.filter_by_push_rules
&& !timeline_event
.push_actions
.as_ref()
.is_some_and(|actions| actions.iter().any(|a| a.should_notify()))
{
return Ok(None);
if let Some(actions) = timeline_event.push_actions.as_ref() {
if !actions.iter().any(|a| a.should_notify()) {
return Ok(None);
}
}
Ok(Some(
@@ -529,52 +529,6 @@ pub enum NotificationStatus {
EventFilteredOut,
}
/// Builder for a `NotificationClient`.
///
/// Fields have the same meaning as in `NotificationClient`.
#[derive(Clone)]
pub struct NotificationClientBuilder {
/// SDK client that uses an in-memory state store, to be used with the
/// sliding sync method.
client: Client,
/// SDK client that uses the same state store as the caller's context.
parent_client: Client,
filter_by_push_rules: bool,
/// Is the notification client running on its own process or not?
process_setup: NotificationProcessSetup,
}
impl NotificationClientBuilder {
async fn new(
parent_client: Client,
process_setup: NotificationProcessSetup,
) -> Result<Self, Error> {
let client = parent_client.notification_client().await?;
Ok(Self { client, parent_client, filter_by_push_rules: false, process_setup })
}
/// Filter out the notification event according to the push rules present in
/// the event.
pub fn filter_by_push_rules(mut self) -> Self {
self.filter_by_push_rules = true;
self
}
/// Finishes configuring the `NotificationClient`.
pub fn build(self) -> NotificationClient {
NotificationClient {
client: self.client,
parent_client: self.parent_client,
filter_by_push_rules: self.filter_by_push_rules,
notification_sync_mutex: AsyncMutex::new(()),
encryption_sync_mutex: AsyncMutex::new(()),
process_setup: self.process_setup,
}
}
}
/// The Notification event as it was fetched from remote for the
/// given `event_id`, represented as Raw but decrypted, thus only
/// whether it is an invite or regular Timeline event has been

View File

@@ -59,8 +59,7 @@ async fn test_notification_client_with_context() {
let dummy_sync_service = Arc::new(SyncService::builder(client.clone()).build().await.unwrap());
let process_setup =
NotificationProcessSetup::SingleProcess { sync_service: dummy_sync_service };
let notification_client =
NotificationClient::builder(client, process_setup).await.unwrap().build();
let notification_client = NotificationClient::new(client, process_setup).await.unwrap();
{
// The notification client retrieves the event via `/rooms/*/context/`.
@@ -242,8 +241,7 @@ async fn test_notification_client_sliding_sync() {
let dummy_sync_service = Arc::new(SyncService::builder(client.clone()).build().await.unwrap());
let process_setup =
NotificationProcessSetup::SingleProcess { sync_service: dummy_sync_service };
let notification_client =
NotificationClient::builder(client, process_setup).await.unwrap().build();
let notification_client = NotificationClient::new(client, process_setup).await.unwrap();
let item =
notification_client.get_notification_with_sliding_sync(room_id, event_id).await.unwrap();

View File

@@ -85,7 +85,7 @@ async fn test_notification() -> Result<()> {
// Try with sliding sync first.
let notification_client =
NotificationClient::builder(bob.clone(), process_setup.clone()).await.unwrap().build();
NotificationClient::new(bob.clone(), process_setup.clone()).await.unwrap();
assert_let!(
NotificationStatus::Event(notification) =
notification_client.get_notification_with_sliding_sync(&room_id, &event_id).await?
@@ -112,7 +112,7 @@ async fn test_notification() -> Result<()> {
// Then with /context.
let notification_client =
NotificationClient::builder(bob.clone(), process_setup.clone()).await.unwrap().build();
NotificationClient::new(bob.clone(), process_setup.clone()).await.unwrap();
let notification =
notification_client.get_notification_with_context(&room_id, &event_id).await;
// We aren't authorized to inspect events from rooms we were not invited to.
@@ -198,15 +198,14 @@ async fn test_notification() -> Result<()> {
};
let notification_client =
NotificationClient::builder(bob.clone(), process_setup.clone()).await.unwrap().build();
NotificationClient::new(bob.clone(), process_setup.clone()).await.unwrap();
assert_let!(
NotificationStatus::Event(notification) =
notification_client.get_notification_with_sliding_sync(&room_id, &event_id).await?
);
check_notification(true, notification);
let notification_client =
NotificationClient::builder(bob.clone(), process_setup).await.unwrap().build();
let notification_client = NotificationClient::new(bob.clone(), process_setup).await.unwrap();
let notification = notification_client
.get_notification_with_context(&room_id, &event_id)
.await?