feat: Prune hidden-from-sidebar bookmarks when the corresponding room…

… generates a permanent error

crates/prose-core-client/src/domain/rooms/models/room_error.rs

@@ -68,4 +68,21 @@ impl RoomError {
             new_location: new_location.as_ref().and_then(|l| RoomId::from_iri(l).ok()),
         })
     }
+
+    pub(crate) fn is_registration_required_err(&self) -> bool {
+        let Self::RequestError(error) = &self else {
+            return false;
+        };
+
+        let RequestError::XMPP {
+            err: StanzaError {
+                defined_condition, ..
+            },
+        } = error
+        else {
+            return false;
+        };
+
+        defined_condition == &DefinedCondition::RegistrationRequired
+    }
 }

crates/prose-core-client/src/domain/sidebar/services/impls/sidebar_domain_service.rs

@@ -122,7 +122,7 @@ impl SidebarDomainServiceTrait for SidebarDomainService {
         }
 
         // Now collect the futures to connect each room…
-        for bookmark in &bookmarks {
+        for bookmark in bookmarks {
             if let Some(room) = rooms.iter().find(|r| r.room_id == bookmark.jid) {
                 if room.sidebar_state() != bookmark.sidebar_state {
                     // We have a room for that bookmark already, let's just update its sidebar_state…
@@ -140,20 +140,24 @@ impl SidebarDomainServiceTrait for SidebarDomainService {
                     )
                     .await;
 
-                match result {
-                    Ok(Some(room)) => {
-                        // Fire an event each time a room connects…
-                        self.client_event_dispatcher
-                            .dispatch_event(ClientEvent::SidebarChanged);
-                        Ok((bookmark.jid.clone(), Some(room)))
+                match &result {
+                    Ok(Some(_)) => {
+                        if bookmark.sidebar_state.is_in_sidebar() {
+                            // Fire an event each time a room connects…
+                            self.client_event_dispatcher
+                                .dispatch_event(ClientEvent::SidebarChanged);
+                        }
                     }
-                    Ok(None) => Ok((bookmark.jid.clone(), None)),
-                    Err(err) => {
-                        self.client_event_dispatcher
-                            .dispatch_event(ClientEvent::SidebarChanged);
-                        Err(err)
+                    Ok(None) => (),
+                    Err(_) => {
+                        if bookmark.sidebar_state.is_in_sidebar() {
+                            self.client_event_dispatcher
+                                .dispatch_event(ClientEvent::SidebarChanged);
+                        }
                     }
                 }
+
+                (bookmark, result)
             });
         }
 
@@ -163,30 +167,26 @@ impl SidebarDomainServiceTrait for SidebarDomainService {
         }
 
         // …and run them in parallel.
-        let results: Vec<Result<(RoomId, Option<Room>), RoomError>> =
+        let results: Vec<(Bookmark, Result<Option<Room>, RoomError>)> =
             join_all(join_room_futures).await;
 
         // Now evaluate the results…
-        for result in results {
+        for (bookmark, result) in results {
+            let room_id = bookmark.jid;
             match result {
-                Ok((saved_room_id, Some(room))) => {
+                Ok(Some(room)) => {
                     // The room was gone and we followed the redirect…
-                    if room.room_id != saved_room_id {
+                    if room.room_id != room_id {
                         update_bookmarks_futures.push(
                             async move {
                                 self.save_bookmark_for_room(&room).await;
-                                self.delete_bookmark(&saved_room_id).await;
+                                self.delete_bookmark(&room_id).await;
                             }
                             .prose_boxed(),
                         );
                         continue;
                     }
 
-                    let bookmark = bookmarks
-                        .iter()
-                        .find(|b| b.jid == saved_room_id)
-                        .expect("Expected bookmark to be in list of bookmarks.");
-
                     let bookmark_type = BookmarkType::from(room.r#type);
 
                     // The room has different attributes than the ones saved in our bookmark…
@@ -202,10 +202,27 @@ impl SidebarDomainServiceTrait for SidebarDomainService {
                     }
                 }
                 Ok(_) => (),
+                Err(error)
+                    if (error.is_gone_err() || error.is_registration_required_err())
+                        && !bookmark.sidebar_state.is_in_sidebar() =>
+                {
+                    // If a room that is hidden from the sidebar is gone or we're not
+                    // a member (anymore), we'll delete the corresponding bookmark.
+                    info!("Deleting bookmark for hidden gone room {room_id}…");
+                    update_bookmarks_futures.push(
+                        async move {
+                            self.connected_rooms_repo.delete(&room_id);
+                            self.delete_bookmark(&room_id).await;
+                        }
+                        .prose_boxed(),
+                    );
+                }
                 Err(error) => {
                     error!(
-                        "Failed to join room from bookmark. Reason: {}",
-                        error.to_string()
+                        "Failed to join room '{}' from bookmark. Reason: {}. is_subscription_required_err? {}",
+                        room_id,
+                        error.to_string(),
+                        error.is_registration_required_err()
                     )
                 }
             }

crates/prose-core-client/tests/sidebar_domain_service.rs

@@ -22,7 +22,7 @@ use prose_core_client::{occupant_id, room_id, user_id, user_resource_id, ClientE
 use prose_xmpp::RequestError;
 
 #[tokio::test]
-async fn test_extend_items_insert_items() -> Result<()> {
+async fn test_extend_items_inserts_items() -> Result<()> {
     let mut deps = MockSidebarDomainServiceDependencies::default();
 
     let room1 = Room::public_channel(room_id!("[email protected]"))
@@ -103,7 +103,7 @@ async fn test_extend_items_insert_items() -> Result<()> {
 }
 
 #[tokio::test]
-async fn test_handles_updated_bookmark() -> Result<()> {
+async fn test_extend_items_updates_room() -> Result<()> {
     let mut deps = MockSidebarDomainServiceDependencies::default();
     let mut seq = Sequence::new();
 
@@ -138,6 +138,155 @@ async fn test_handles_updated_bookmark() -> Result<()> {
     Ok(())
 }
 
+#[tokio::test]
+async fn test_extend_items_deletes_hidden_gone_rooms() -> Result<()> {
+    let mut deps = MockSidebarDomainServiceDependencies::default();
+
+    let room1 = Room::group(room_id!("[email protected]"))
+        .with_name("Group")
+        .with_sidebar_state(RoomSidebarState::InSidebar);
+    let room2 = Room::group(room_id!("[email protected]"))
+        .with_name("Visible Gone Group")
+        .with_sidebar_state(RoomSidebarState::InSidebar);
+    let room3 = Room::group(room_id!("[email protected]"))
+        .with_name("Hidden Gone Group")
+        .with_sidebar_state(RoomSidebarState::NotInSidebar);
+    let room4 = Room::group(room_id!("[email protected]"))
+        .with_name("Hidden Group")
+        .with_sidebar_state(RoomSidebarState::NotInSidebar);
+
+    deps.connected_rooms_repo
+        .expect_get_all()
+        .once()
+        .return_once(|| vec![]);
+
+    deps.connected_rooms_repo
+        .expect_set()
+        .times(4)
+        .returning(|_| Ok(()));
+
+    deps.client_event_dispatcher
+        .expect_dispatch_event()
+        .once()
+        .with(predicate::eq(ClientEvent::SidebarChanged))
+        .return_once(|_| ());
+
+    {
+        let room1 = room1.clone();
+        deps.rooms_domain_service
+            .expect_create_or_join_room()
+            .once()
+            .with(
+                predicate::eq(CreateOrEnterRoomRequest::JoinRoom {
+                    room_id: room_id!("[email protected]"),
+                    password: None,
+                    behavior: JoinRoomBehavior::system_initiated(),
+                }),
+                predicate::eq(RoomSidebarState::InSidebar),
+            )
+            .return_once(|_, _| Box::pin(async { Ok(room1) }));
+    }
+    {
+        deps.rooms_domain_service
+            .expect_create_or_join_room()
+            .once()
+            .with(
+                predicate::eq(CreateOrEnterRoomRequest::JoinRoom {
+                    room_id: room_id!("[email protected]"),
+                    password: None,
+                    behavior: JoinRoomBehavior::system_initiated(),
+                }),
+                predicate::eq(RoomSidebarState::InSidebar),
+            )
+            .return_once(|_, _| {
+                Box::pin(async {
+                    Err(RoomError::RequestError(RequestError::XMPP {
+                        err: StanzaError::new(
+                            ErrorType::Cancel,
+                            DefinedCondition::Gone,
+                            "en",
+                            "Room is gone",
+                        ),
+                    }))
+                })
+            });
+    }
+    {
+        deps.rooms_domain_service
+            .expect_create_or_join_room()
+            .once()
+            .with(
+                predicate::eq(CreateOrEnterRoomRequest::JoinRoom {
+                    room_id: room_id!("[email protected]"),
+                    password: None,
+                    behavior: JoinRoomBehavior::system_initiated(),
+                }),
+                predicate::eq(RoomSidebarState::NotInSidebar),
+            )
+            .return_once(|_, _| {
+                Box::pin(async {
+                    Err(RoomError::RequestError(RequestError::XMPP {
+                        err: StanzaError::new(
+                            ErrorType::Cancel,
+                            DefinedCondition::Gone,
+                            "en",
+                            "Room is gone",
+                        ),
+                    }))
+                })
+            });
+    }
+    {
+        let room4 = room4.clone();
+        deps.rooms_domain_service
+            .expect_create_or_join_room()
+            .once()
+            .with(
+                predicate::eq(CreateOrEnterRoomRequest::JoinRoom {
+                    room_id: room_id!("[email protected]"),
+                    password: None,
+                    behavior: JoinRoomBehavior::system_initiated(),
+                }),
+                predicate::eq(RoomSidebarState::NotInSidebar),
+            )
+            .return_once(|_, _| Box::pin(async { Ok(room4) }));
+    }
+
+    // An event should be fired for each room that is in the sidebar.
+    deps.client_event_dispatcher
+        .expect_dispatch_event()
+        .times(2)
+        .with(predicate::eq(ClientEvent::SidebarChanged))
+        .returning(|_| ());
+
+    {
+        let room3 = room3.clone();
+        deps.connected_rooms_repo
+            .expect_delete()
+            .once()
+            .with(predicate::eq(room_id!("[email protected]")))
+            .return_once(|_| Some(room3));
+    }
+
+    deps.bookmarks_service
+        .expect_delete_bookmark()
+        .once()
+        .with(predicate::eq(room_id!("[email protected]")))
+        .return_once(|_| Box::pin(async { Ok(()) }));
+
+    let service = SidebarDomainService::from(deps.into_deps());
+    service
+        .extend_items_from_bookmarks(vec![
+            Bookmark::try_from(&room1).unwrap(),
+            Bookmark::try_from(&room2).unwrap(),
+            Bookmark::try_from(&room3).unwrap(),
+            Bookmark::try_from(&room4).unwrap(),
+        ])
+        .await?;
+
+    Ok(())
+}
+
 #[tokio::test]
 async fn test_removes_public_channel_from_sidebar() -> Result<()> {
     let mut deps = MockSidebarDomainServiceDependencies::default();