From 511cbaa2bdedade147724fce0fc0c5661f721f83 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 23 Sep 2021 15:35:35 +0200 Subject: [PATCH 1/4] Version the zrpc protocol using a `X-ZRPC-VERSION` header --- server/src/rpc.rs | 5 ++++- zed/src/rpc.rs | 49 +++++++++++++++++++++++++++++------------------ zrpc/src/lib.rs | 2 ++ 3 files changed, 36 insertions(+), 20 deletions(-) diff --git a/server/src/rpc.rs b/server/src/rpc.rs index fec6182fcc..52ebd51396 100644 --- a/server/src/rpc.rs +++ b/server/src/rpc.rs @@ -894,8 +894,11 @@ pub fn add_routes(app: &mut tide::Server>, rpc: &Arc) { let connection_upgrade = header_contains_ignore_case(&request, CONNECTION, "upgrade"); let upgrade_to_websocket = header_contains_ignore_case(&request, UPGRADE, "websocket"); let upgrade_requested = connection_upgrade && upgrade_to_websocket; + let client_protocol_version: Option = request + .header("X-ZRPC-VERSION") + .and_then(|v| v.as_str().parse().ok()); - if !upgrade_requested { + if !upgrade_requested || client_protocol_version != Some(zrpc::VERSION) { return Ok(Response::new(StatusCode::UpgradeRequired)); } diff --git a/zed/src/rpc.rs b/zed/src/rpc.rs index 5dc2b49b76..5b18ff310b 100644 --- a/zed/src/rpc.rs +++ b/zed/src/rpc.rs @@ -55,6 +55,8 @@ pub struct Client { #[derive(Error, Debug)] pub enum EstablishConnectionError { + #[error("upgrade required")] + UpgradeRequired, #[error("unauthorized")] Unauthorized, #[error("{0}")] @@ -68,8 +70,10 @@ pub enum EstablishConnectionError { impl From for EstablishConnectionError { fn from(error: WebsocketError) -> Self { if let WebsocketError::Http(response) = &error { - if response.status() == StatusCode::UNAUTHORIZED { - return EstablishConnectionError::Unauthorized; + match response.status() { + StatusCode::UNAUTHORIZED => return EstablishConnectionError::Unauthorized, + StatusCode::UPGRADE_REQUIRED => return EstablishConnectionError::UpgradeRequired, + _ => {} } } EstablishConnectionError::Other(error.into()) @@ -85,6 +89,7 @@ impl EstablishConnectionError { #[derive(Copy, Clone, Debug)] pub enum Status { SignedOut, + UpgradeRequired, Authenticating, Connecting, ConnectionError, @@ -227,7 +232,7 @@ impl Client { } })); } - Status::SignedOut => { + Status::SignedOut | Status::UpgradeRequired => { state._maintain_connection.take(); } _ => {} @@ -346,6 +351,7 @@ impl Client { | Status::Reconnecting { .. } | Status::Authenticating | Status::Reauthenticating => return Ok(()), + Status::UpgradeRequired => return Err(EstablishConnectionError::UpgradeRequired)?, }; if was_disconnected { @@ -388,22 +394,25 @@ impl Client { self.set_connection(conn, cx).await; Ok(()) } - Err(err) => { - if matches!(err, EstablishConnectionError::Unauthorized) { - self.state.write().credentials.take(); - if used_keychain { - cx.platform().delete_credentials(&ZED_SERVER_URL).log_err(); - self.set_status(Status::SignedOut, cx); - self.authenticate_and_connect(cx).await - } else { - self.set_status(Status::ConnectionError, cx); - Err(err)? - } + Err(EstablishConnectionError::Unauthorized) => { + self.state.write().credentials.take(); + if used_keychain { + cx.platform().delete_credentials(&ZED_SERVER_URL).log_err(); + self.set_status(Status::SignedOut, cx); + self.authenticate_and_connect(cx).await } else { self.set_status(Status::ConnectionError, cx); - Err(err)? + Err(EstablishConnectionError::Unauthorized)? } } + Err(EstablishConnectionError::UpgradeRequired) => { + self.set_status(Status::UpgradeRequired, cx); + Err(EstablishConnectionError::UpgradeRequired)? + } + Err(error) => { + self.set_status(Status::ConnectionError, cx); + Err(error)? + } } } @@ -489,10 +498,12 @@ impl Client { credentials: &Credentials, cx: &AsyncAppContext, ) -> Task> { - let request = Request::builder().header( - "Authorization", - format!("{} {}", credentials.user_id, credentials.access_token), - ); + let request = Request::builder() + .header( + "Authorization", + format!("{} {}", credentials.user_id, credentials.access_token), + ) + .header("X-ZRPC-VERSION", zrpc::VERSION); cx.background().spawn(async move { if let Some(host) = ZED_SERVER_URL.strip_prefix("https://") { let stream = smol::net::TcpStream::connect(host).await?; diff --git a/zrpc/src/lib.rs b/zrpc/src/lib.rs index a7bb44774b..607cd252f0 100644 --- a/zrpc/src/lib.rs +++ b/zrpc/src/lib.rs @@ -4,3 +4,5 @@ mod peer; pub mod proto; pub use conn::Connection; pub use peer::*; + +pub const VERSION: u32 = 0; From 374b05a379e1aeb67c0f8d741ab69fc77aab1e81 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 23 Sep 2021 15:36:32 +0200 Subject: [PATCH 2/4] Display warning in titlebar when Zed is out-of-date --- zed/assets/themes/_base.toml | 1 + zed/src/theme.rs | 3 ++- zed/src/workspace.rs | 10 ++++++++++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/zed/assets/themes/_base.toml b/zed/assets/themes/_base.toml index 5938032c2c..3e326ab62c 100644 --- a/zed/assets/themes/_base.toml +++ b/zed/assets/themes/_base.toml @@ -11,6 +11,7 @@ title = "$text.0" avatar_width = 20 icon_color = "$text.2.color" avatar = { corner_radius = 10, border = { width = 1, color = "#00000088" } } +outdated_warning = { extends = "$text.2", size = 13 } [workspace.titlebar.offline_icon] padding = { right = 4 } diff --git a/zed/src/theme.rs b/zed/src/theme.rs index 8b43b09f13..a5378fe033 100644 --- a/zed/src/theme.rs +++ b/zed/src/theme.rs @@ -54,6 +54,7 @@ pub struct Titlebar { pub offline_icon: OfflineIcon, pub icon_color: Color, pub avatar: ImageStyle, + pub outdated_warning: ContainedText, } #[derive(Clone, Deserialize)] @@ -169,7 +170,7 @@ pub struct Selector { pub active_item: ContainedLabel, } -#[derive(Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize)] pub struct ContainedText { #[serde(flatten)] pub container: ContainerStyle, diff --git a/zed/src/workspace.rs b/zed/src/workspace.rs index df4afb020e..c148b72916 100644 --- a/zed/src/workspace.rs +++ b/zed/src/workspace.rs @@ -1041,6 +1041,16 @@ impl Workspace { .with_style(theme.workspace.titlebar.offline_icon.container) .boxed(), ), + rpc::Status::UpgradeRequired => Some( + Label::new( + "Please update Zed to collaborate".to_string(), + theme.workspace.titlebar.outdated_warning.text.clone(), + ) + .contained() + .with_style(theme.workspace.titlebar.outdated_warning.container) + .aligned() + .boxed(), + ), _ => None, } } From 2352725c58da42b82ace12a1f768c033b349ab84 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 23 Sep 2021 17:32:37 +0200 Subject: [PATCH 3/4] :lipstick: Co-Authored-By: Nathan Sobo --- server/src/rpc.rs | 4 ++-- zed/src/rpc.rs | 2 +- zrpc/src/lib.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server/src/rpc.rs b/server/src/rpc.rs index 52ebd51396..1d8de2d9a3 100644 --- a/server/src/rpc.rs +++ b/server/src/rpc.rs @@ -895,10 +895,10 @@ pub fn add_routes(app: &mut tide::Server>, rpc: &Arc) { let upgrade_to_websocket = header_contains_ignore_case(&request, UPGRADE, "websocket"); let upgrade_requested = connection_upgrade && upgrade_to_websocket; let client_protocol_version: Option = request - .header("X-ZRPC-VERSION") + .header("X-Zed-Protocol-Version") .and_then(|v| v.as_str().parse().ok()); - if !upgrade_requested || client_protocol_version != Some(zrpc::VERSION) { + if !upgrade_requested || client_protocol_version != Some(zrpc::PROTOCOL_VERSION) { return Ok(Response::new(StatusCode::UpgradeRequired)); } diff --git a/zed/src/rpc.rs b/zed/src/rpc.rs index 5b18ff310b..86f484607d 100644 --- a/zed/src/rpc.rs +++ b/zed/src/rpc.rs @@ -503,7 +503,7 @@ impl Client { "Authorization", format!("{} {}", credentials.user_id, credentials.access_token), ) - .header("X-ZRPC-VERSION", zrpc::VERSION); + .header("X-Zed-Protocol-Version", zrpc::PROTOCOL_VERSION); cx.background().spawn(async move { if let Some(host) = ZED_SERVER_URL.strip_prefix("https://") { let stream = smol::net::TcpStream::connect(host).await?; diff --git a/zrpc/src/lib.rs b/zrpc/src/lib.rs index 607cd252f0..7e32da83c5 100644 --- a/zrpc/src/lib.rs +++ b/zrpc/src/lib.rs @@ -5,4 +5,4 @@ pub mod proto; pub use conn::Connection; pub use peer::*; -pub const VERSION: u32 = 0; +pub const PROTOCOL_VERSION: u32 = 0; From e32f1f8b8034234a523ab450d0d1fd7814d834bf Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 23 Sep 2021 18:17:49 +0200 Subject: [PATCH 4/4] Create `test_app_state` with `FakeFs` instead of `RealFs` Co-Authored-By: Nathan Sobo --- zed/src/file_finder.rs | 59 ++++++++++++++--------- zed/src/fs.rs | 12 +++++ zed/src/test.rs | 4 +- zed/src/workspace.rs | 104 +++++++++++++++++++++++------------------ 4 files changed, 110 insertions(+), 69 deletions(-) diff --git a/zed/src/file_finder.rs b/zed/src/file_finder.rs index 8f3217b2e5..2749bbe59d 100644 --- a/zed/src/file_finder.rs +++ b/zed/src/file_finder.rs @@ -438,29 +438,37 @@ mod tests { use crate::{ editor::{self, Insert}, fs::FakeFs, - test::{temp_tree, test_app_state}, + test::test_app_state, workspace::Workspace, }; use serde_json::json; - use std::fs; - use tempdir::TempDir; + use std::path::PathBuf; #[gpui::test] async fn test_matching_paths(mut cx: gpui::TestAppContext) { - let tmp_dir = TempDir::new("example").unwrap(); - fs::create_dir(tmp_dir.path().join("a")).unwrap(); - fs::write(tmp_dir.path().join("a/banana"), "banana").unwrap(); - fs::write(tmp_dir.path().join("a/bandana"), "bandana").unwrap(); + let app_state = cx.update(test_app_state); + app_state + .fs + .as_fake() + .insert_tree( + "/root", + json!({ + "a": { + "banana": "", + "bandana": "", + } + }), + ) + .await; cx.update(|cx| { super::init(cx); editor::init(cx); }); - let app_state = cx.update(test_app_state); let (window_id, workspace) = cx.add_window(|cx| Workspace::new(&app_state, cx)); workspace .update(&mut cx, |workspace, cx| { - workspace.add_worktree(tmp_dir.path(), cx) + workspace.add_worktree(Path::new("/root"), cx) }) .await .unwrap(); @@ -572,17 +580,17 @@ mod tests { #[gpui::test] async fn test_single_file_worktrees(mut cx: gpui::TestAppContext) { - let temp_dir = TempDir::new("test-single-file-worktrees").unwrap(); - let dir_path = temp_dir.path().join("the-parent-dir"); - let file_path = dir_path.join("the-file"); - fs::create_dir(&dir_path).unwrap(); - fs::write(&file_path, "").unwrap(); - let app_state = cx.update(test_app_state); + app_state + .fs + .as_fake() + .insert_tree("/root", json!({ "the-parent-dir": { "the-file": "" } })) + .await; + let (_, workspace) = cx.add_window(|cx| Workspace::new(&app_state, cx)); workspace .update(&mut cx, |workspace, cx| { - workspace.add_worktree(&file_path, cx) + workspace.add_worktree(Path::new("/root/the-parent-dir/the-file"), cx) }) .await .unwrap(); @@ -620,18 +628,25 @@ mod tests { #[gpui::test(retries = 5)] async fn test_multiple_matches_with_same_relative_path(mut cx: gpui::TestAppContext) { - let tmp_dir = temp_tree(json!({ - "dir1": { "a.txt": "" }, - "dir2": { "a.txt": "" } - })); - let app_state = cx.update(test_app_state); + app_state + .fs + .as_fake() + .insert_tree( + "/root", + json!({ + "dir1": { "a.txt": "" }, + "dir2": { "a.txt": "" } + }), + ) + .await; + let (_, workspace) = cx.add_window(|cx| Workspace::new(&app_state, cx)); workspace .update(&mut cx, |workspace, cx| { workspace.open_paths( - &[tmp_dir.path().join("dir1"), tmp_dir.path().join("dir2")], + &[PathBuf::from("/root/dir1"), PathBuf::from("/root/dir2")], cx, ) }) diff --git a/zed/src/fs.rs b/zed/src/fs.rs index e9d6d9230b..7f67d6ef02 100644 --- a/zed/src/fs.rs +++ b/zed/src/fs.rs @@ -29,6 +29,8 @@ pub trait Fs: Send + Sync { latency: Duration, ) -> Pin>>>; fn is_fake(&self) -> bool; + #[cfg(any(test, feature = "test-support"))] + fn as_fake(&self) -> &FakeFs; } #[derive(Clone, Debug)] @@ -125,6 +127,11 @@ impl Fs for RealFs { fn is_fake(&self) -> bool { false } + + #[cfg(any(test, feature = "test-support"))] + fn as_fake(&self) -> &FakeFs { + panic!("called `RealFs::as_fake`") + } } #[derive(Clone, Debug)] @@ -413,4 +420,9 @@ impl Fs for FakeFs { fn is_fake(&self) -> bool { true } + + #[cfg(any(test, feature = "test-support"))] + fn as_fake(&self) -> &FakeFs { + self + } } diff --git a/zed/src/test.rs b/zed/src/test.rs index eb6bf20f45..59edd380d5 100644 --- a/zed/src/test.rs +++ b/zed/src/test.rs @@ -1,7 +1,7 @@ use crate::{ assets::Assets, channel::ChannelList, - fs::RealFs, + fs::FakeFs, http::{HttpClient, Request, Response, ServerResponse}, language::LanguageRegistry, rpc::{self, Client, Credentials, EstablishConnectionError}, @@ -177,7 +177,7 @@ pub fn test_app_state(cx: &mut MutableAppContext) -> Arc { channel_list: cx.add_model(|cx| ChannelList::new(user_store.clone(), rpc.clone(), cx)), rpc, user_store, - fs: Arc::new(RealFs), + fs: Arc::new(FakeFs::new()), }) } diff --git a/zed/src/workspace.rs b/zed/src/workspace.rs index c148b72916..456b9e7042 100644 --- a/zed/src/workspace.rs +++ b/zed/src/workspace.rs @@ -1205,11 +1205,9 @@ mod tests { editor::{Editor, Insert}, fs::FakeFs, test::{temp_tree, test_app_state}, - worktree::WorktreeHandle, }; use serde_json::json; - use std::{collections::HashSet, fs}; - use tempdir::TempDir; + use std::collections::HashSet; #[gpui::test] async fn test_open_paths_action(mut cx: gpui::TestAppContext) { @@ -1278,20 +1276,26 @@ mod tests { #[gpui::test] async fn test_open_entry(mut cx: gpui::TestAppContext) { - let dir = temp_tree(json!({ - "a": { - "file1": "contents 1", - "file2": "contents 2", - "file3": "contents 3", - }, - })); - let app_state = cx.update(test_app_state); + app_state + .fs + .as_fake() + .insert_tree( + "/root", + json!({ + "a": { + "file1": "contents 1", + "file2": "contents 2", + "file3": "contents 3", + }, + }), + ) + .await; let (_, workspace) = cx.add_window(|cx| Workspace::new(&app_state, cx)); workspace .update(&mut cx, |workspace, cx| { - workspace.add_worktree(dir.path(), cx) + workspace.add_worktree(Path::new("/root"), cx) }) .await .unwrap(); @@ -1455,28 +1459,30 @@ mod tests { #[gpui::test] async fn test_save_conflicting_item(mut cx: gpui::TestAppContext) { - let dir = temp_tree(json!({ - "a.txt": "", - })); - let app_state = cx.update(test_app_state); + app_state + .fs + .as_fake() + .insert_tree( + "/root", + json!({ + "a.txt": "", + }), + ) + .await; + let (window_id, workspace) = cx.add_window(|cx| Workspace::new(&app_state, cx)); workspace .update(&mut cx, |workspace, cx| { - workspace.add_worktree(dir.path(), cx) + workspace.add_worktree(Path::new("/root"), cx) }) .await .unwrap(); - let tree = cx.read(|cx| { - let mut trees = workspace.read(cx).worktrees().iter(); - trees.next().unwrap().clone() - }); - tree.flush_fs_events(&cx).await; // Open a file within an existing worktree. cx.update(|cx| { workspace.update(cx, |view, cx| { - view.open_paths(&[dir.path().join("a.txt")], cx) + view.open_paths(&[PathBuf::from("/root/a.txt")], cx) }) }) .await; @@ -1487,7 +1493,12 @@ mod tests { }); cx.update(|cx| editor.update(cx, |editor, cx| editor.insert(&Insert("x".into()), cx))); - fs::write(dir.path().join("a.txt"), "changed").unwrap(); + app_state + .fs + .as_fake() + .insert_file("/root/a.txt", "changed".to_string()) + .await + .unwrap(); editor .condition(&cx, |editor, cx| editor.has_conflict(cx)) .await; @@ -1503,12 +1514,12 @@ mod tests { #[gpui::test] async fn test_open_and_save_new_file(mut cx: gpui::TestAppContext) { - let dir = TempDir::new("test-new-file").unwrap(); let app_state = cx.update(test_app_state); + app_state.fs.as_fake().insert_dir("/root").await.unwrap(); let (_, workspace) = cx.add_window(|cx| Workspace::new(&app_state, cx)); workspace .update(&mut cx, |workspace, cx| { - workspace.add_worktree(dir.path(), cx) + workspace.add_worktree(Path::new("/root"), cx) }) .await .unwrap(); @@ -1521,7 +1532,6 @@ mod tests { .unwrap() .clone() }); - tree.flush_fs_events(&cx).await; // Create a new untitled buffer let editor = workspace.update(&mut cx, |workspace, cx| { @@ -1547,7 +1557,7 @@ mod tests { workspace.save_active_item(&Save, cx) }); cx.simulate_new_path_selection(|parent_dir| { - assert_eq!(parent_dir, dir.path()); + assert_eq!(parent_dir, Path::new("/root")); Some(parent_dir.join("the-new-name.rs")) }); cx.read(|cx| { @@ -1608,8 +1618,8 @@ mod tests { async fn test_setting_language_when_saving_as_single_file_worktree( mut cx: gpui::TestAppContext, ) { - let dir = TempDir::new("test-new-file").unwrap(); let app_state = cx.update(test_app_state); + app_state.fs.as_fake().insert_dir("/root").await.unwrap(); let (_, workspace) = cx.add_window(|cx| Workspace::new(&app_state, cx)); // Create a new untitled buffer @@ -1633,7 +1643,7 @@ mod tests { workspace.update(&mut cx, |workspace, cx| { workspace.save_active_item(&Save, cx) }); - cx.simulate_new_path_selection(|_| Some(dir.path().join("the-new-name.rs"))); + cx.simulate_new_path_selection(|_| Some(PathBuf::from("/root/the-new-name.rs"))); editor .condition(&cx, |editor, cx| !editor.is_dirty(cx)) @@ -1650,7 +1660,7 @@ mod tests { cx.update(init); let app_state = cx.update(test_app_state); - cx.dispatch_global_action(OpenNew(app_state)); + cx.dispatch_global_action(OpenNew(app_state.clone())); let window_id = *cx.window_ids().first().unwrap(); let workspace = cx.root_view::(window_id).unwrap(); let editor = workspace.update(&mut cx, |workspace, cx| { @@ -1670,10 +1680,8 @@ mod tests { workspace.save_active_item(&Save, cx) }); - let dir = TempDir::new("test-new-empty-workspace").unwrap(); - cx.simulate_new_path_selection(|_| { - Some(dir.path().canonicalize().unwrap().join("the-new-name")) - }); + app_state.fs.as_fake().insert_dir("/root").await.unwrap(); + cx.simulate_new_path_selection(|_| Some(PathBuf::from("/root/the-new-name"))); editor .condition(&cx, |editor, cx| editor.title(cx) == "the-new-name") @@ -1686,20 +1694,26 @@ mod tests { #[gpui::test] async fn test_pane_actions(mut cx: gpui::TestAppContext) { cx.update(|cx| pane::init(cx)); - - let dir = temp_tree(json!({ - "a": { - "file1": "contents 1", - "file2": "contents 2", - "file3": "contents 3", - }, - })); - let app_state = cx.update(test_app_state); + app_state + .fs + .as_fake() + .insert_tree( + "/root", + json!({ + "a": { + "file1": "contents 1", + "file2": "contents 2", + "file3": "contents 3", + }, + }), + ) + .await; + let (window_id, workspace) = cx.add_window(|cx| Workspace::new(&app_state, cx)); workspace .update(&mut cx, |workspace, cx| { - workspace.add_worktree(dir.path(), cx) + workspace.add_worktree(Path::new("/root"), cx) }) .await .unwrap();