oak-team/oak-gpui
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix routing of leader updates from unshared projects (#4028)

Browse Source 
Previously, leader updates in unshared projects would be sent to all
followers regardless of project, as if they were not scoped to any
project.

- Fixes a crash that could sometimes happen when following someone if
they were focused on an unshared project.
This commit is contained in:
Max Brunsfeld
2024-01-11 14:10:54 -08:00
committed by GitHub
parent 7c817642b8 258c2fdad4
commit 8478ddb6d2
5 changed files with 198 additions and 152 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/collab/src/tests/channel_buffer_tests.rs
-146
View File
@@ -599,152 +599,6 @@ async fn test_channel_buffers_and_server_restarts(
});
}
#[gpui::test(iterations = 10)]
async fn test_following_to_channel_notes_without_a_shared_project(
deterministic: BackgroundExecutor,
mut cx_a: &mut TestAppContext,
mut cx_b: &mut TestAppContext,
mut cx_c: &mut TestAppContext,
) {
let mut server = TestServer::start(deterministic.clone()).await;
let client_a = server.create_client(cx_a, "user_a").await;
let client_b = server.create_client(cx_b, "user_b").await;
let client_c = server.create_client(cx_c, "user_c").await;
cx_a.update(editor::init);
cx_b.update(editor::init);
cx_c.update(editor::init);
cx_a.update(collab_ui::channel_view::init);
cx_b.update(collab_ui::channel_view::init);
cx_c.update(collab_ui::channel_view::init);
let channel_1_id = server
.make_channel(
"channel-1",
None,
(&client_a, cx_a),
&mut [(&client_b, cx_b), (&client_c, cx_c)],
)
.await;
let channel_2_id = server
.make_channel(
"channel-2",
None,
(&client_a, cx_a),
&mut [(&client_b, cx_b), (&client_c, cx_c)],
)
.await;
// Clients A, B, and C join a channel.
let active_call_a = cx_a.read(ActiveCall::global);
let active_call_b = cx_b.read(ActiveCall::global);
let active_call_c = cx_c.read(ActiveCall::global);
for (call, cx) in [
(&active_call_a, &mut cx_a),
(&active_call_b, &mut cx_b),
(&active_call_c, &mut cx_c),
] {
call.update(*cx, |call, cx| call.join_channel(channel_1_id, cx))
.await
.unwrap();
}
deterministic.run_until_parked();
// Clients A, B, and C all open their own unshared projects.
client_a.fs().insert_tree("/a", json!({})).await;
client_b.fs().insert_tree("/b", json!({})).await;
client_c.fs().insert_tree("/c", json!({})).await;
let (project_a, _) = client_a.build_local_project("/a", cx_a).await;
let (project_b, _) = client_b.build_local_project("/b", cx_b).await;
let (project_c, _) = client_b.build_local_project("/c", cx_c).await;
let (workspace_a, cx_a) = client_a.build_workspace(&project_a, cx_a);
let (workspace_b, cx_b) = client_b.build_workspace(&project_b, cx_b);
let (_workspace_c, _cx_c) = client_c.build_workspace(&project_c, cx_c);
active_call_a
.update(cx_a, |call, cx| call.set_location(Some(&project_a), cx))
.await
.unwrap();
// Client A opens the notes for channel 1.
let channel_view_1_a = cx_a
.update(|cx| ChannelView::open(channel_1_id, workspace_a.clone(), cx))
.await
.unwrap();
channel_view_1_a.update(cx_a, |notes, cx| {
assert_eq!(notes.channel(cx).unwrap().name, "channel-1");
notes.editor.update(cx, |editor, cx| {
editor.insert("Hello from A.", cx);
editor.change_selections(None, cx, |selections| {
selections.select_ranges(vec![3..4]);
});
});
});
// Client B follows client A.
workspace_b
.update(cx_b, |workspace, cx| {
workspace
.start_following(client_a.peer_id().unwrap(), cx)
.unwrap()
})
.await
.unwrap();
// Client B is taken to the notes for channel 1, with the same
// text selected as client A.
deterministic.run_until_parked();
let channel_view_1_b = workspace_b.update(cx_b, |workspace, cx| {
assert_eq!(
workspace.leader_for_pane(workspace.active_pane()),
Some(client_a.peer_id().unwrap())
);
workspace
.active_item(cx)
.expect("no active item")
.downcast::<ChannelView>()
.expect("active item is not a channel view")
});
channel_view_1_b.update(cx_b, |notes, cx| {
assert_eq!(notes.channel(cx).unwrap().name, "channel-1");
let editor = notes.editor.read(cx);
assert_eq!(editor.text(cx), "Hello from A.");
assert_eq!(editor.selections.ranges::<usize>(cx), &[3..4]);
});
// Client A opens the notes for channel 2.
eprintln!("opening -------------------->");
let channel_view_2_a = cx_a
.update(|cx| ChannelView::open(channel_2_id, workspace_a.clone(), cx))
.await
.unwrap();
channel_view_2_a.update(cx_a, |notes, cx| {
assert_eq!(notes.channel(cx).unwrap().name, "channel-2");
});
// Client B is taken to the notes for channel 2.
deterministic.run_until_parked();
eprintln!("opening <--------------------");
let channel_view_2_b = workspace_b.update(cx_b, |workspace, cx| {
assert_eq!(
workspace.leader_for_pane(workspace.active_pane()),
Some(client_a.peer_id().unwrap())
);
workspace
.active_item(cx)
.expect("no active item")
.downcast::<ChannelView>()
.expect("active item is not a channel view")
});
channel_view_2_b.update(cx_b, |notes, cx| {
assert_eq!(notes.channel(cx).unwrap().name, "channel-2");
});
}
#[gpui::test]
async fn test_channel_buffer_changes(
deterministic: BackgroundExecutor,
crates/collab/src/tests/following_tests.rs
+169 -2
View File
@@ -1,9 +1,12 @@
use crate::{rpc::RECONNECT_TIMEOUT, tests::TestServer};
use call::{ActiveCall, ParticipantLocation};
use collab_ui::notifications::project_shared_notification::ProjectSharedNotification;
use collab_ui::{
channel_view::ChannelView,
notifications::project_shared_notification::ProjectSharedNotification,
};
use editor::{Editor, ExcerptRange, MultiBuffer};
use gpui::{
point, BackgroundExecutor, Context, SharedString, TestAppContext, View, VisualContext,
point, BackgroundExecutor, Context, Entity, SharedString, TestAppContext, View, VisualContext,
VisualTestContext,
};
use language::Capability;
@@ -1822,3 +1825,167 @@ fn pane_summaries(workspace: &View<Workspace>, cx: &mut VisualTestContext) -> Ve
.collect()
})
}
#[gpui::test(iterations = 10)]
async fn test_following_to_channel_notes_without_a_shared_project(
deterministic: BackgroundExecutor,
mut cx_a: &mut TestAppContext,
mut cx_b: &mut TestAppContext,
mut cx_c: &mut TestAppContext,
) {
let mut server = TestServer::start(deterministic.clone()).await;
let client_a = server.create_client(cx_a, "user_a").await;
let client_b = server.create_client(cx_b, "user_b").await;
let client_c = server.create_client(cx_c, "user_c").await;
cx_a.update(editor::init);
cx_b.update(editor::init);
cx_c.update(editor::init);
cx_a.update(collab_ui::channel_view::init);
cx_b.update(collab_ui::channel_view::init);
cx_c.update(collab_ui::channel_view::init);
let channel_1_id = server
.make_channel(
"channel-1",
None,
(&client_a, cx_a),
&mut [(&client_b, cx_b), (&client_c, cx_c)],
)
.await;
let channel_2_id = server
.make_channel(
"channel-2",
None,
(&client_a, cx_a),
&mut [(&client_b, cx_b), (&client_c, cx_c)],
)
.await;
// Clients A, B, and C join a channel.
let active_call_a = cx_a.read(ActiveCall::global);
let active_call_b = cx_b.read(ActiveCall::global);
let active_call_c = cx_c.read(ActiveCall::global);
for (call, cx) in [
(&active_call_a, &mut cx_a),
(&active_call_b, &mut cx_b),
(&active_call_c, &mut cx_c),
] {
call.update(*cx, |call, cx| call.join_channel(channel_1_id, cx))
.await
.unwrap();
}
deterministic.run_until_parked();
// Clients A, B, and C all open their own unshared projects.
client_a
.fs()
.insert_tree("/a", json!({ "1.txt": "" }))
.await;
client_b.fs().insert_tree("/b", json!({})).await;
client_c.fs().insert_tree("/c", json!({})).await;
let (project_a, worktree_id) = client_a.build_local_project("/a", cx_a).await;
let (project_b, _) = client_b.build_local_project("/b", cx_b).await;
let (project_c, _) = client_b.build_local_project("/c", cx_c).await;
let (workspace_a, cx_a) = client_a.build_workspace(&project_a, cx_a);
let (workspace_b, cx_b) = client_b.build_workspace(&project_b, cx_b);
let (_workspace_c, _cx_c) = client_c.build_workspace(&project_c, cx_c);
active_call_a
.update(cx_a, |call, cx| call.set_location(Some(&project_a), cx))
.await
.unwrap();
// Client A opens the notes for channel 1.
let channel_notes_1_a = cx_a
.update(|cx| ChannelView::open(channel_1_id, workspace_a.clone(), cx))
.await
.unwrap();
channel_notes_1_a.update(cx_a, |notes, cx| {
assert_eq!(notes.channel(cx).unwrap().name, "channel-1");
notes.editor.update(cx, |editor, cx| {
editor.insert("Hello from A.", cx);
editor.change_selections(None, cx, |selections| {
selections.select_ranges(vec![3..4]);
});
});
});
// Client B follows client A.
workspace_b
.update(cx_b, |workspace, cx| {
workspace
.start_following(client_a.peer_id().unwrap(), cx)
.unwrap()
})
.await
.unwrap();
// Client B is taken to the notes for channel 1, with the same
// text selected as client A.
deterministic.run_until_parked();
let channel_notes_1_b = workspace_b.update(cx_b, |workspace, cx| {
assert_eq!(
workspace.leader_for_pane(workspace.active_pane()),
Some(client_a.peer_id().unwrap())
);
workspace
.active_item(cx)
.expect("no active item")
.downcast::<ChannelView>()
.expect("active item is not a channel view")
});
channel_notes_1_b.update(cx_b, |notes, cx| {
assert_eq!(notes.channel(cx).unwrap().name, "channel-1");
let editor = notes.editor.read(cx);
assert_eq!(editor.text(cx), "Hello from A.");
assert_eq!(editor.selections.ranges::<usize>(cx), &[3..4]);
});
// Client A opens the notes for channel 2.
let channel_notes_2_a = cx_a
.update(|cx| ChannelView::open(channel_2_id, workspace_a.clone(), cx))
.await
.unwrap();
channel_notes_2_a.update(cx_a, |notes, cx| {
assert_eq!(notes.channel(cx).unwrap().name, "channel-2");
});
// Client B is taken to the notes for channel 2.
deterministic.run_until_parked();
let channel_notes_2_b = workspace_b.update(cx_b, |workspace, cx| {
assert_eq!(
workspace.leader_for_pane(workspace.active_pane()),
Some(client_a.peer_id().unwrap())
);
workspace
.active_item(cx)
.expect("no active item")
.downcast::<ChannelView>()
.expect("active item is not a channel view")
});
channel_notes_2_b.update(cx_b, |notes, cx| {
assert_eq!(notes.channel(cx).unwrap().name, "channel-2");
});
// Client A opens a local buffer in their unshared project.
let _unshared_editor_a1 = workspace_a
.update(cx_a, |workspace, cx| {
workspace.open_path((worktree_id, "1.txt"), None, true, cx)
})
.await
.unwrap()
.downcast::<Editor>()
.unwrap();
// This does not send any leader update message to client B.
// If it did, an error would occur on client B, since this buffer
// is not shared with them.
deterministic.run_until_parked();
workspace_b.update(cx_b, |workspace, cx| {
assert_eq!(
workspace.active_item(cx).expect("no active item").item_id(),
channel_notes_2_b.entity_id()
);
});
}
crates/editor/src/items.rs
+3 -1
View File
@@ -82,7 +82,9 @@ impl FollowableItem for Editor {
let pane = pane.downgrade();
Some(cx.spawn(|mut cx| async move {
let mut buffers = futures::future::try_join_all(buffers).await?;
let mut buffers = futures::future::try_join_all(buffers)
.await
.debug_assert_ok("leaders don't share views for unshared buffers")?;
let editor = pane.update(&mut cx, |pane, cx| {
let mut editors = pane.items_of_type::<Self>();
editors.find(|editor| {
crates/util/src/util.rs
+10
View File
@@ -137,6 +137,8 @@ pub trait ResultExt<E> {
type Ok;
fn log_err(self) -> Option<Self::Ok>;
/// Assert that this result should never be an error in development or tests.
fn debug_assert_ok(self, reason: &str) -> Self;
fn warn_on_err(self) -> Option<Self::Ok>;
fn inspect_error(self, func: impl FnOnce(&E)) -> Self;
}
@@ -159,6 +161,14 @@ where
}
}
#[track_caller]
fn debug_assert_ok(self, reason: &str) -> Self {
if let Err(error) = &self {
debug_panic!("{reason} - {error:?}");
}
self
}
fn warn_on_err(self) -> Option<T> {
match self {
Ok(value) => Some(value),
crates/workspace/src/workspace.rs
+16 -3
View File
@@ -2608,11 +2608,20 @@ impl Workspace {
let cx = &cx;
move |item| {
let item = item.to_followable_item_handle(cx)?;
if (project_id.is_none() || project_id != follower_project_id)
&& item.is_project_item(cx)
// If the item belongs to a particular project, then it should
// only be included if this project is shared, and the follower
// is in thie project.
//
// Some items, like channel notes, do not belong to a particular
// project, so they should be included regardless of whether the
// current project is shared, or what project the follower is in.
if item.is_project_item(cx)
&& (project_id.is_none() || project_id != follower_project_id)
{
return None;
}
let id = item.remote_id(client, cx)?.to_proto();
let variant = item.to_state_proto(cx)?;
Some(proto::View {
@@ -2790,8 +2799,12 @@ impl Workspace {
update: proto::update_followers::Variant,
cx: &mut WindowContext,
) -> Option<()> {
// If this update only applies to for followers in the current project,
// then skip it unless this project is shared. If it applies to all
// followers, regardless of project, then set `project_id` to none,
// indicating that it goes to all followers.
let project_id = if project_only {
self.project.read(cx).remote_id()
Some(self.project.read(cx).remote_id()?)
} else {
None
};