From 4739c683af45d10cddbfcc9e10515aa8967a9446 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 11 May 2022 16:49:56 -0700 Subject: [PATCH] Fix bug where Contacts included projects for which the use was a guest --- crates/collab/src/rpc.rs | 74 +++++++++++++------------------ crates/collab/src/rpc/store.rs | 79 ++++++++-------------------------- 2 files changed, 49 insertions(+), 104 deletions(-) diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 33d1d52677..c48ac3b83a 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -4995,13 +4995,11 @@ mod tests { cx_c: &mut TestAppContext, ) { cx_a.foreground().forbid_parking(); - let lang_registry = Arc::new(LanguageRegistry::test()); - let fs = FakeFs::new(cx_a.background()); // Connect to a server as 3 clients. let mut server = TestServer::start(cx_a.foreground(), cx_a.background()).await; - let client_a = server.create_client(cx_a, "user_a").await; - let client_b = server.create_client(cx_b, "user_b").await; + let mut client_a = server.create_client(cx_a, "user_a").await; + let mut client_b = server.create_client(cx_b, "user_b").await; let client_c = server.create_client(cx_c, "user_c").await; server .make_contacts(vec![ @@ -5025,27 +5023,10 @@ mod tests { }); } - // Share a worktree as client A. + // Share a project as client A. + let fs = FakeFs::new(cx_a.background()); fs.create_dir(Path::new("/a")).await.unwrap(); - - let project_a = cx_a.update(|cx| { - Project::local( - client_a.clone(), - client_a.user_store.clone(), - lang_registry.clone(), - fs.clone(), - cx, - ) - }); - let (worktree_a, _) = project_a - .update(cx_a, |p, cx| { - p.find_or_create_local_worktree("/a", true, cx) - }) - .await - .unwrap(); - worktree_a - .read_with(cx_a, |tree, _| tree.as_local().unwrap().scan_complete()) - .await; + let (project_a, _) = client_a.build_local_project(fs, "/a", cx_a).await; deterministic.run_until_parked(); for (client, cx) in [(&client_a, &cx_a), (&client_b, &cx_b), (&client_c, &cx_c)] { @@ -5083,16 +5064,7 @@ mod tests { }); } - let _project_b = Project::remote( - project_id, - client_b.clone(), - client_b.user_store.clone(), - lang_registry.clone(), - fs.clone(), - &mut cx_b.to_async(), - ) - .await - .unwrap(); + let _project_b = client_b.build_remote_project(project_id, cx_b).await; deterministic.run_until_parked(); for (client, cx) in [(&client_a, &cx_a), (&client_b, &cx_b), (&client_c, &cx_c)] { @@ -5108,12 +5080,32 @@ mod tests { }); } + // Add a local project as client B + let fs = FakeFs::new(cx_b.background()); + fs.create_dir(Path::new("/b")).await.unwrap(); + let (_project_b, _) = client_b.build_local_project(fs, "/b", cx_a).await; + + deterministic.run_until_parked(); + for (client, cx) in [(&client_a, &cx_a), (&client_b, &cx_b), (&client_c, &cx_c)] { + client.user_store.read_with(*cx, |store, _| { + assert_eq!( + contacts(store), + [ + ("user_a", true, vec![("a", true, vec!["user_b"])]), + ("user_b", true, vec![("b", false, vec![])]), + ("user_c", true, vec![]) + ] + ) + }); + } + project_a .condition(&cx_a, |project, _| { project.collaborators().contains_key(&client_b.peer_id) }) .await; + client_a.project.take(); cx_a.update(move |_| drop(project_a)); deterministic.run_until_parked(); for (client, cx) in [(&client_a, &cx_a), (&client_b, &cx_b), (&client_c, &cx_c)] { @@ -5122,7 +5114,7 @@ mod tests { contacts(store), [ ("user_a", true, vec![]), - ("user_b", true, vec![]), + ("user_b", true, vec![("b", false, vec![])]), ("user_c", true, vec![]) ] ) @@ -5138,7 +5130,7 @@ mod tests { contacts(store), [ ("user_a", true, vec![]), - ("user_b", true, vec![]), + ("user_b", true, vec![("b", false, vec![])]), ("user_c", false, vec![]) ] ) @@ -5161,7 +5153,7 @@ mod tests { contacts(store), [ ("user_a", true, vec![]), - ("user_b", true, vec![]), + ("user_b", true, vec![("b", false, vec![])]), ("user_c", true, vec![]) ] ) @@ -5173,7 +5165,7 @@ mod tests { .contacts() .iter() .map(|contact| { - let worktrees = contact + let projects = contact .projects .iter() .map(|p| { @@ -5184,11 +5176,7 @@ mod tests { ) }) .collect(); - ( - contact.user.github_login.as_str(), - contact.online, - worktrees, - ) + (contact.user.github_login.as_str(), contact.online, projects) }) .collect() } diff --git a/crates/collab/src/rpc/store.rs b/crates/collab/src/rpc/store.rs index 8ca2706228..52cf2b2628 100644 --- a/crates/collab/src/rpc/store.rs +++ b/crates/collab/src/rpc/store.rs @@ -259,73 +259,30 @@ impl Store { let mut metadata = Vec::new(); for project_id in project_ids { if let Some(project) = self.projects.get(&project_id) { - metadata.push(proto::ProjectMetadata { - id: project_id, - is_shared: project.share.is_some(), - worktree_root_names: project - .worktrees - .values() - .map(|worktree| worktree.root_name.clone()) - .collect(), - guests: project - .share - .iter() - .flat_map(|share| { - share.guests.values().map(|(_, user_id)| user_id.to_proto()) - }) - .collect(), - }); + if project.host_user_id == user_id { + metadata.push(proto::ProjectMetadata { + id: project_id, + is_shared: project.share.is_some(), + worktree_root_names: project + .worktrees + .values() + .map(|worktree| worktree.root_name.clone()) + .collect(), + guests: project + .share + .iter() + .flat_map(|share| { + share.guests.values().map(|(_, user_id)| user_id.to_proto()) + }) + .collect(), + }); + } } } metadata } - // pub fn contacts_for_user(&self, user_id: UserId) -> Vec { - // let mut contacts = HashMap::default(); - // for project_id in self - // .visible_projects_by_user_id - // .get(&user_id) - // .unwrap_or(&HashSet::default()) - // { - // let project = &self.projects[project_id]; - - // let mut guests = HashSet::default(); - // if let Ok(share) = project.share() { - // for guest_connection_id in share.guests.keys() { - // if let Ok(user_id) = self.user_id_for_connection(*guest_connection_id) { - // guests.insert(user_id.to_proto()); - // } - // } - // } - - // if let Ok(host_user_id) = self.user_id_for_connection(project.host_connection_id) { - // let mut worktree_root_names = project - // .worktrees - // .values() - // .filter(|worktree| worktree.visible) - // .map(|worktree| worktree.root_name.clone()) - // .collect::>(); - // worktree_root_names.sort_unstable(); - // contacts - // .entry(host_user_id) - // .or_insert_with(|| proto::Contact { - // user_id: host_user_id.to_proto(), - // projects: Vec::new(), - // }) - // .projects - // .push(proto::ProjectMetadata { - // id: *project_id, - // worktree_root_names, - // is_shared: project.share.is_some(), - // guests: guests.into_iter().collect(), - // }); - // } - // } - - // contacts.into_values().collect() - // } - pub fn register_project( &mut self, host_connection_id: ConnectionId,