Replace returned tuples with named structs in `Store`

Antonio Scandurra created 4 years ago

Change summary

server/src/rpc.rs       | 21 ++++++++++++---------
server/src/rpc/store.rs | 35 ++++++++++++++++++++++++++---------
2 files changed, 38 insertions(+), 18 deletions(-)

Detailed changes

server/src/rpc.rs

@@ -19,7 +19,7 @@ use std::{
     sync::Arc,
     time::Instant,
 };
-use store::{Store, Worktree};
+use store::{JoinedWorktree, Store, Worktree};
 use surf::StatusCode;
 use tide::log;
 use tide::{
@@ -324,18 +324,18 @@ impl Server {
         request: TypedEnvelope<proto::UnshareWorktree>,
     ) -> tide::Result<()> {
         let worktree_id = request.payload.worktree_id;
-        let (connection_ids, collaborator_user_ids) = self
+        let worktree = self
             .store
             .write()
             .await
             .unshare_worktree(worktree_id, request.sender_id)?;
 
-        broadcast(request.sender_id, connection_ids, |conn_id| {
+        broadcast(request.sender_id, worktree.connection_ids, |conn_id| {
             self.peer
                 .send(conn_id, proto::UnshareWorktree { worktree_id })
         })
         .await?;
-        self.update_collaborators_for_users(&collaborator_user_ids)
+        self.update_collaborators_for_users(&worktree.collaborator_ids)
             .await?;
 
         Ok(())
@@ -357,7 +357,10 @@ impl Server {
         let collaborator_user_ids;
         let mut state = self.store.write().await;
         match state.join_worktree(request.sender_id, user_id, worktree_id) {
-            Ok((peer_replica_id, worktree)) => {
+            Ok(JoinedWorktree {
+                replica_id,
+                worktree,
+            }) => {
                 let share = worktree.share()?;
                 let peer_count = share.guest_connection_ids.len();
                 let mut peers = Vec::with_capacity(peer_count);
@@ -379,7 +382,7 @@ impl Server {
                         root_name: worktree.root_name.clone(),
                         entries: share.entries.values().cloned().collect(),
                     }),
-                    replica_id: peer_replica_id as u32,
+                    replica_id: replica_id as u32,
                     peers,
                 };
                 connection_ids = worktree.connection_ids();
@@ -426,13 +429,13 @@ impl Server {
         let sender_id = request.sender_id;
         let worktree_id = request.payload.worktree_id;
 
-        if let Some((connection_ids, collaborator_ids)) = self
+        if let Some(worktree) = self
             .store
             .write()
             .await
             .leave_worktree(sender_id, worktree_id)
         {
-            broadcast(sender_id, connection_ids, |conn_id| {
+            broadcast(sender_id, worktree.connection_ids, |conn_id| {
                 self.peer.send(
                     conn_id,
                     proto::RemovePeer {
@@ -442,7 +445,7 @@ impl Server {
                 )
             })
             .await?;
-            self.update_collaborators_for_users(&collaborator_ids)
+            self.update_collaborators_for_users(&worktree.collaborator_ids)
                 .await?;
         }
         Ok(())

server/src/rpc/store.rs

@@ -47,6 +47,16 @@ pub struct RemovedConnectionState {
     pub collaborator_ids: HashSet<UserId>,
 }
 
+pub struct JoinedWorktree<'a> {
+    pub replica_id: ReplicaId,
+    pub worktree: &'a Worktree,
+}
+
+pub struct WorktreeMetadata {
+    pub connection_ids: Vec<ConnectionId>,
+    pub collaborator_ids: Vec<UserId>,
+}
+
 impl Store {
     pub fn add_connection(&mut self, connection_id: ConnectionId, user_id: UserId) {
         self.connections.insert(
@@ -110,6 +120,7 @@ impl Store {
         Ok(result)
     }
 
+    #[cfg(test)]
     pub fn channel(&self, id: ChannelId) -> Option<&Channel> {
         self.channels.get(&id)
     }
@@ -280,7 +291,7 @@ impl Store {
         &mut self,
         worktree_id: u64,
         acting_connection_id: ConnectionId,
-    ) -> tide::Result<(Vec<ConnectionId>, Vec<UserId>)> {
+    ) -> tide::Result<WorktreeMetadata> {
         let worktree = if let Some(worktree) = self.worktrees.get_mut(&worktree_id) {
             worktree
         } else {
@@ -299,7 +310,10 @@ impl Store {
                     connection.worktrees.remove(&worktree_id);
                 }
             }
-            Ok((connection_ids, worktree.collaborator_user_ids.clone()))
+            Ok(WorktreeMetadata {
+                connection_ids,
+                collaborator_ids: worktree.collaborator_user_ids.clone(),
+            })
         } else {
             Err(anyhow!("worktree is not shared"))?
         }
@@ -310,7 +324,7 @@ impl Store {
         connection_id: ConnectionId,
         user_id: UserId,
         worktree_id: u64,
-    ) -> tide::Result<(ReplicaId, &Worktree)> {
+    ) -> tide::Result<JoinedWorktree> {
         let connection = self
             .connections
             .get_mut(&connection_id)
@@ -336,22 +350,25 @@ impl Store {
         }
         share.active_replica_ids.insert(replica_id);
         share.guest_connection_ids.insert(connection_id, replica_id);
-        return Ok((replica_id, worktree));
+        Ok(JoinedWorktree {
+            replica_id,
+            worktree,
+        })
     }
 
     pub fn leave_worktree(
         &mut self,
         connection_id: ConnectionId,
         worktree_id: u64,
-    ) -> Option<(Vec<ConnectionId>, Vec<UserId>)> {
+    ) -> Option<WorktreeMetadata> {
         let worktree = self.worktrees.get_mut(&worktree_id)?;
         let share = worktree.share.as_mut()?;
         let replica_id = share.guest_connection_ids.remove(&connection_id)?;
         share.active_replica_ids.remove(&replica_id);
-        Some((
-            worktree.connection_ids(),
-            worktree.collaborator_user_ids.clone(),
-        ))
+        Some(WorktreeMetadata {
+            connection_ids: worktree.connection_ids(),
+            collaborator_ids: worktree.collaborator_user_ids.clone(),
+        })
     }
 
     pub fn update_worktree(