From 43f2f60bfd0d7042f83fdf835fbcb13ea35f1815 Mon Sep 17 00:00:00 2001
From: Ivan Enderlin <ivan@mnt.io>
Date: Thu, 9 Feb 2023 13:04:54 +0100
Subject: [PATCH] fix(sdk): Remove `Deref` impl for `SlidingSyncRoom`.

This implementation is wrong in the sense of its semantics is not about
dereferencing a thin pointer to something, but just to give access to one
specific field of the entire structure. That's not how `Deref` is supposed to
be used.

Moreover, it creates conflict between the `SlidingSyncRoom.timeline` field,
and `SlidingSyncRoom.inner.timeline` field, which both exist, but not for the
same purposes. It creates confusion in the code.

Finally, it's better to expose proper getters to the outside world, so that we
control _and_ test _and_ know exactly what API we provide.
---
 bindings/matrix-sdk-ffi/src/sliding_sync.rs |  9 ++---
 crates/matrix-sdk/src/sliding_sync.rs       | 38 +++++++++++++++------
 2 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/bindings/matrix-sdk-ffi/src/sliding_sync.rs b/bindings/matrix-sdk-ffi/src/sliding_sync.rs
index 350babe08..a49e6c72e 100644
--- a/bindings/matrix-sdk-ffi/src/sliding_sync.rs
+++ b/bindings/matrix-sdk-ffi/src/sliding_sync.rs
@@ -130,22 +130,23 @@ impl SlidingSyncRoom {
     }
 
     pub fn is_dm(&self) -> Option<bool> {
-        self.inner.is_dm
+        self.inner.is_dm()
     }
 
     pub fn is_initial(&self) -> Option<bool> {
-        self.inner.initial
+        self.inner.is_an_initial_response()
     }
+
     pub fn is_loading_more(&self) -> bool {
         self.inner.is_loading_more()
     }
 
     pub fn has_unread_notifications(&self) -> bool {
-        !self.inner.unread_notifications.is_empty()
+        self.inner.has_unread_notifications()
     }
 
     pub fn unread_notifications(&self) -> Arc<UnreadNotificationsCount> {
-        Arc::new(self.inner.unread_notifications.clone().into())
+        Arc::new(self.inner.unread_notifications().clone().into())
     }
 
     pub fn full_room(&self) -> Option<Arc<Room>> {
diff --git a/crates/matrix-sdk/src/sliding_sync.rs b/crates/matrix-sdk/src/sliding_sync.rs
index cefe73dce..12e2f8e3f 100644
--- a/crates/matrix-sdk/src/sliding_sync.rs
+++ b/crates/matrix-sdk/src/sliding_sync.rs
@@ -16,7 +16,7 @@
 use std::{
     collections::BTreeMap,
     fmt::Debug,
-    ops::Deref,
+    ops::{Deref, Not},
     sync::{
         atomic::{AtomicBool, AtomicU8, Ordering},
         Arc, Mutex,
@@ -34,9 +34,12 @@ use matrix_sdk_base::{deserialized_responses::SyncTimelineEvent, sync::SyncRespo
 use ruma::{
     api::client::{
         error::ErrorKind,
-        sync::sync_events::v4::{
-            self, AccountDataConfig, E2EEConfig, ExtensionsConfig, ReceiptConfig, ToDeviceConfig,
-            TypingConfig,
+        sync::sync_events::{
+            v4::{
+                self, AccountDataConfig, E2EEConfig, ExtensionsConfig, ReceiptConfig,
+                ToDeviceConfig, TypingConfig,
+            },
+            UnreadNotificationsCount,
         },
     },
     assign,
@@ -275,6 +278,26 @@ impl SlidingSyncRoom {
         self.inner.name.as_deref()
     }
 
+    /// Is this a direct message?
+    pub fn is_dm(&self) -> Option<bool> {
+        self.inner.is_dm
+    }
+
+    /// Was this an initial response.
+    pub fn is_an_initial_response(&self) -> Option<bool> {
+        self.inner.initial
+    }
+
+    /// Is there any unread notifications?
+    pub fn has_unread_notifications(&self) -> bool {
+        self.inner.unread_notifications.is_empty().not()
+    }
+
+    /// Get unread notifications.
+    pub fn unread_notifications(&self) -> &UnreadNotificationsCount {
+        &self.inner.unread_notifications
+    }
+
     fn update(&mut self, room_data: &v4::SlidingSyncRoom, timeline: Vec<SyncTimelineEvent>) {
         let v4::SlidingSyncRoom {
             name,
@@ -331,13 +354,6 @@ impl SlidingSyncRoom {
     }
 }
 
-impl Deref for SlidingSyncRoom {
-    type Target = v4::SlidingSyncRoom;
-    fn deref(&self) -> &Self::Target {
-        &self.inner
-    }
-}
-
 type ViewState = Mutable<SlidingSyncState>;
 type SyncMode = Mutable<SlidingSyncMode>;
 type StringState = Mutable<Option<String>>;