Automatically fetch remote participant users in `Room`

Antonio Scandurra created

Change summary

crates/call/src/participant.rs         |  9 +--
crates/call/src/room.rs                | 69 +++++++++++++++++----------
crates/collab/src/integration_tests.rs | 65 +++++++++----------------
3 files changed, 72 insertions(+), 71 deletions(-)

Detailed changes

crates/call/src/participant.rs 🔗

@@ -1,7 +1,6 @@
 use anyhow::{anyhow, Result};
-use client::proto;
-use gpui::ModelHandle;
-use project::Project;
+use client::{proto, User};
+use std::sync::Arc;
 
 pub enum ParticipantLocation {
     Project { project_id: u64 },
@@ -21,7 +20,7 @@ impl ParticipantLocation {
 }
 
 pub struct RemoteParticipant {
-    pub user_id: u64,
-    pub projects: Vec<ModelHandle<Project>>,
+    pub user: Arc<User>,
+    pub project_ids: Vec<u64>,
     pub location: ParticipantLocation,
 }

crates/call/src/room.rs 🔗

@@ -19,7 +19,7 @@ pub struct Room {
     client: Arc<Client>,
     user_store: ModelHandle<UserStore>,
     _subscriptions: Vec<client::Subscription>,
-    _load_pending_users: Option<Task<()>>,
+    _pending_room_update: Option<Task<()>>,
 }
 
 impl Entity for Room {
@@ -58,7 +58,7 @@ impl Room {
             remote_participants: Default::default(),
             pending_users: Default::default(),
             _subscriptions: vec![client.add_message_handler(cx.handle(), Self::handle_room_updated)],
-            _load_pending_users: None,
+            _pending_room_update: None,
             client,
             user_store,
         }
@@ -133,32 +133,51 @@ impl Room {
         Ok(())
     }
 
-    fn apply_room_update(&mut self, room: proto::Room, cx: &mut ModelContext<Self>) -> Result<()> {
-        // TODO: compute diff instead of clearing participants
-        self.remote_participants.clear();
-        for participant in room.participants {
-            if Some(participant.user_id) != self.client.user_id() {
-                self.remote_participants.insert(
-                    PeerId(participant.peer_id),
-                    RemoteParticipant {
-                        user_id: participant.user_id,
-                        projects: Default::default(), // TODO: populate projects
-                        location: ParticipantLocation::from_proto(participant.location)?,
-                    },
-                );
-            }
-        }
-
-        let pending_users = self.user_store.update(cx, move |user_store, cx| {
-            user_store.get_users(room.pending_user_ids, cx)
+    fn apply_room_update(
+        &mut self,
+        mut room: proto::Room,
+        cx: &mut ModelContext<Self>,
+    ) -> Result<()> {
+        // Filter ourselves out from the room's participants.
+        room.participants
+            .retain(|participant| Some(participant.user_id) != self.client.user_id());
+
+        let participant_user_ids = room
+            .participants
+            .iter()
+            .map(|p| p.user_id)
+            .collect::<Vec<_>>();
+        let (participants, pending_users) = self.user_store.update(cx, move |user_store, cx| {
+            (
+                user_store.get_users(participant_user_ids, cx),
+                user_store.get_users(room.pending_user_ids, cx),
+            )
         });
-        self._load_pending_users = Some(cx.spawn(|this, mut cx| async move {
-            if let Some(pending_users) = pending_users.await.log_err() {
-                this.update(&mut cx, |this, cx| {
+        self._pending_room_update = Some(cx.spawn(|this, mut cx| async move {
+            let (participants, pending_users) = futures::join!(participants, pending_users);
+
+            this.update(&mut cx, |this, cx| {
+                if let Some(participants) = participants.log_err() {
+                    // TODO: compute diff instead of clearing participants
+                    this.remote_participants.clear();
+                    for (participant, user) in room.participants.into_iter().zip(participants) {
+                        this.remote_participants.insert(
+                            PeerId(participant.peer_id),
+                            RemoteParticipant {
+                                user,
+                                project_ids: participant.project_ids,
+                                location: ParticipantLocation::from_proto(participant.location)
+                                    .unwrap_or(ParticipantLocation::External),
+                            },
+                        );
+                    }
+                }
+
+                if let Some(pending_users) = pending_users.log_err() {
                     this.pending_users = pending_users;
                     cx.notify();
-                });
-            }
+                }
+            });
         }));
 
         cx.notify();

crates/collab/src/integration_tests.rs 🔗

@@ -82,7 +82,7 @@ async fn test_basic_calls(
         .await
         .unwrap();
     assert_eq!(
-        room_participants(&room_a, &client_a, cx_a).await,
+        room_participants(&room_a, cx_a),
         RoomParticipants {
             remote: Default::default(),
             pending: Default::default()
@@ -100,7 +100,7 @@ async fn test_basic_calls(
 
     deterministic.run_until_parked();
     assert_eq!(
-        room_participants(&room_a, &client_a, cx_a).await,
+        room_participants(&room_a, cx_a),
         RoomParticipants {
             remote: Default::default(),
             pending: vec!["user_b".to_string()]
@@ -127,14 +127,14 @@ async fn test_basic_calls(
 
     deterministic.run_until_parked();
     assert_eq!(
-        room_participants(&room_a, &client_a, cx_a).await,
+        room_participants(&room_a, cx_a),
         RoomParticipants {
             remote: vec!["user_b".to_string()],
             pending: Default::default()
         }
     );
     assert_eq!(
-        room_participants(&room_b, &client_b, cx_b).await,
+        room_participants(&room_b, cx_b),
         RoomParticipants {
             remote: vec!["user_a".to_string()],
             pending: Default::default()
@@ -152,14 +152,14 @@ async fn test_basic_calls(
 
     deterministic.run_until_parked();
     assert_eq!(
-        room_participants(&room_a, &client_a, cx_a).await,
+        room_participants(&room_a, cx_a),
         RoomParticipants {
             remote: vec!["user_b".to_string()],
             pending: vec!["user_c".to_string()]
         }
     );
     assert_eq!(
-        room_participants(&room_b, &client_b, cx_b).await,
+        room_participants(&room_b, cx_b),
         RoomParticipants {
             remote: vec!["user_a".to_string()],
             pending: vec!["user_c".to_string()]
@@ -176,14 +176,14 @@ async fn test_basic_calls(
 
     deterministic.run_until_parked();
     assert_eq!(
-        room_participants(&room_a, &client_a, cx_a).await,
+        room_participants(&room_a, cx_a),
         RoomParticipants {
             remote: vec!["user_b".to_string()],
             pending: Default::default()
         }
     );
     assert_eq!(
-        room_participants(&room_b, &client_b, cx_b).await,
+        room_participants(&room_b, cx_b),
         RoomParticipants {
             remote: vec!["user_a".to_string()],
             pending: Default::default()
@@ -194,14 +194,14 @@ async fn test_basic_calls(
     room_a.update(cx_a, |room, cx| room.leave(cx)).unwrap();
     deterministic.run_until_parked();
     assert_eq!(
-        room_participants(&room_a, &client_a, cx_a).await,
+        room_participants(&room_a, cx_a),
         RoomParticipants {
             remote: Default::default(),
             pending: Default::default()
         }
     );
     assert_eq!(
-        room_participants(&room_b, &client_b, cx_b).await,
+        room_participants(&room_b, cx_b),
         RoomParticipants {
             remote: Default::default(),
             pending: Default::default()
@@ -245,14 +245,14 @@ async fn test_leaving_room_on_disconnection(
         .unwrap();
     deterministic.run_until_parked();
     assert_eq!(
-        room_participants(&room_a, &client_a, cx_a).await,
+        room_participants(&room_a, cx_a),
         RoomParticipants {
             remote: vec!["user_b".to_string()],
             pending: Default::default()
         }
     );
     assert_eq!(
-        room_participants(&room_b, &client_b, cx_b).await,
+        room_participants(&room_b, cx_b),
         RoomParticipants {
             remote: vec!["user_a".to_string()],
             pending: Default::default()
@@ -262,14 +262,14 @@ async fn test_leaving_room_on_disconnection(
     server.disconnect_client(client_a.current_user_id(cx_a));
     cx_a.foreground().advance_clock(rpc::RECEIVE_TIMEOUT);
     assert_eq!(
-        room_participants(&room_a, &client_a, cx_a).await,
+        room_participants(&room_a, cx_a),
         RoomParticipants {
             remote: Default::default(),
             pending: Default::default()
         }
     );
     assert_eq!(
-        room_participants(&room_b, &client_b, cx_b).await,
+        room_participants(&room_b, cx_b),
         RoomParticipants {
             remote: Default::default(),
             pending: Default::default()
@@ -5822,34 +5822,17 @@ struct RoomParticipants {
     pending: Vec<String>,
 }
 
-async fn room_participants(
-    room: &ModelHandle<Room>,
-    client: &TestClient,
-    cx: &mut TestAppContext,
-) -> RoomParticipants {
-    let remote_users = room.update(cx, |room, cx| {
-        room.remote_participants()
-            .values()
-            .map(|participant| {
-                client
-                    .user_store
-                    .update(cx, |users, cx| users.get_user(participant.user_id, cx))
-            })
-            .collect::<Vec<_>>()
-    });
-    let remote_users = futures::future::try_join_all(remote_users).await.unwrap();
-    let pending_users = room.read_with(cx, |room, _| {
-        room.pending_users().iter().cloned().collect::<Vec<_>>()
-    });
-
-    RoomParticipants {
-        remote: remote_users
-            .into_iter()
-            .map(|user| user.github_login.clone())
+fn room_participants(room: &ModelHandle<Room>, cx: &mut TestAppContext) -> RoomParticipants {
+    room.read_with(cx, |room, _| RoomParticipants {
+        remote: room
+            .remote_participants()
+            .iter()
+            .map(|(_, participant)| participant.user.github_login.clone())
             .collect(),
-        pending: pending_users
-            .into_iter()
+        pending: room
+            .pending_users()
+            .iter()
             .map(|user| user.github_login.clone())
             .collect(),
-    }
+    })
 }