From 25a7eb27d22ea6bf49df71e650db38b33810520f Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 24 Jan 2024 14:53:05 +0100 Subject: [PATCH] Move interaction with keychain off the main thread --- crates/ai/src/auth.rs | 14 +- crates/ai/src/providers/open_ai/completion.rs | 67 ++++--- crates/ai/src/providers/open_ai/embedding.rs | 81 ++++++--- crates/ai/src/test.rs | 38 +++- crates/assistant/src/assistant_panel.rs | 108 +++++++---- crates/client/src/client.rs | 27 ++- crates/gpui/src/app.rs | 11 +- crates/gpui/src/platform.rs | 8 +- crates/gpui/src/platform/mac/platform.rs | 171 ++++++++++-------- crates/gpui/src/platform/test/platform.rs | 15 +- crates/semantic_index/src/semantic_index.rs | 42 ++++- crates/zed/src/main.rs | 2 +- 12 files changed, 370 insertions(+), 214 deletions(-) diff --git a/crates/ai/src/auth.rs b/crates/ai/src/auth.rs index 1ea49bd615..62556d7183 100644 --- a/crates/ai/src/auth.rs +++ b/crates/ai/src/auth.rs @@ -1,3 +1,4 @@ +use futures::future::BoxFuture; use gpui::AppContext; #[derive(Clone, Debug)] @@ -9,7 +10,14 @@ pub enum ProviderCredential { pub trait CredentialProvider: Send + Sync { fn has_credentials(&self) -> bool; - fn retrieve_credentials(&self, cx: &mut AppContext) -> ProviderCredential; - fn save_credentials(&self, cx: &mut AppContext, credential: ProviderCredential); - fn delete_credentials(&self, cx: &mut AppContext); + #[must_use] + fn retrieve_credentials(&self, cx: &mut AppContext) -> BoxFuture; + #[must_use] + fn save_credentials( + &self, + cx: &mut AppContext, + credential: ProviderCredential, + ) -> BoxFuture<()>; + #[must_use] + fn delete_credentials(&self, cx: &mut AppContext) -> BoxFuture<()>; } diff --git a/crates/ai/src/providers/open_ai/completion.rs b/crates/ai/src/providers/open_ai/completion.rs index fda4d69816..aa58950113 100644 --- a/crates/ai/src/providers/open_ai/completion.rs +++ b/crates/ai/src/providers/open_ai/completion.rs @@ -222,45 +222,70 @@ impl CredentialProvider for OpenAICompletionProvider { } } - fn retrieve_credentials(&self, cx: &mut AppContext) -> ProviderCredential { + fn retrieve_credentials(&self, cx: &mut AppContext) -> BoxFuture { let existing_credential = self.credential.read().clone(); let retrieved_credential = match existing_credential { - ProviderCredential::Credentials { .. } => existing_credential.clone(), + ProviderCredential::Credentials { .. } => { + return async move { existing_credential }.boxed() + } _ => { if let Some(api_key) = env::var("OPENAI_API_KEY").log_err() { - ProviderCredential::Credentials { api_key } - } else if let Some(Some((_, api_key))) = - cx.read_credentials(OPENAI_API_URL).log_err() - { - if let Some(api_key) = String::from_utf8(api_key).log_err() { - ProviderCredential::Credentials { api_key } - } else { - ProviderCredential::NoCredentials - } + async move { ProviderCredential::Credentials { api_key } }.boxed() } else { - ProviderCredential::NoCredentials + let credentials = cx.read_credentials(OPENAI_API_URL); + async move { + if let Some(Some((_, api_key))) = credentials.await.log_err() { + if let Some(api_key) = String::from_utf8(api_key).log_err() { + ProviderCredential::Credentials { api_key } + } else { + ProviderCredential::NoCredentials + } + } else { + ProviderCredential::NoCredentials + } + } + .boxed() } } }; - *self.credential.write() = retrieved_credential.clone(); - retrieved_credential + + async move { + let retrieved_credential = retrieved_credential.await; + *self.credential.write() = retrieved_credential.clone(); + retrieved_credential + } + .boxed() } - fn save_credentials(&self, cx: &mut AppContext, credential: ProviderCredential) { + fn save_credentials( + &self, + cx: &mut AppContext, + credential: ProviderCredential, + ) -> BoxFuture<()> { *self.credential.write() = credential.clone(); let credential = credential.clone(); - match credential { + let write_credentials = match credential { ProviderCredential::Credentials { api_key } => { - cx.write_credentials(OPENAI_API_URL, "Bearer", api_key.as_bytes()) - .log_err(); + Some(cx.write_credentials(OPENAI_API_URL, "Bearer", api_key.as_bytes())) + } + _ => None, + }; + + async move { + if let Some(write_credentials) = write_credentials { + write_credentials.await.log_err(); } - _ => {} } + .boxed() } - fn delete_credentials(&self, cx: &mut AppContext) { - cx.delete_credentials(OPENAI_API_URL).log_err(); + fn delete_credentials(&self, cx: &mut AppContext) -> BoxFuture<()> { *self.credential.write() = ProviderCredential::NoCredentials; + let delete_credentials = cx.delete_credentials(OPENAI_API_URL); + async move { + delete_credentials.await.log_err(); + } + .boxed() } } diff --git a/crates/ai/src/providers/open_ai/embedding.rs b/crates/ai/src/providers/open_ai/embedding.rs index 1dca571733..89aebb1b76 100644 --- a/crates/ai/src/providers/open_ai/embedding.rs +++ b/crates/ai/src/providers/open_ai/embedding.rs @@ -1,6 +1,8 @@ use anyhow::{anyhow, Result}; use async_trait::async_trait; +use futures::future::BoxFuture; use futures::AsyncReadExt; +use futures::FutureExt; use gpui::AppContext; use gpui::BackgroundExecutor; use isahc::http::StatusCode; @@ -157,46 +159,71 @@ impl CredentialProvider for OpenAIEmbeddingProvider { _ => false, } } - fn retrieve_credentials(&self, cx: &mut AppContext) -> ProviderCredential { - let existing_credential = self.credential.read().clone(); + fn retrieve_credentials(&self, cx: &mut AppContext) -> BoxFuture { + let existing_credential = self.credential.read().clone(); let retrieved_credential = match existing_credential { - ProviderCredential::Credentials { .. } => existing_credential.clone(), + ProviderCredential::Credentials { .. } => { + return async move { existing_credential }.boxed() + } _ => { if let Some(api_key) = env::var("OPENAI_API_KEY").log_err() { - ProviderCredential::Credentials { api_key } - } else if let Some(Some((_, api_key))) = - cx.read_credentials(OPENAI_API_URL).log_err() - { - if let Some(api_key) = String::from_utf8(api_key).log_err() { - ProviderCredential::Credentials { api_key } - } else { - ProviderCredential::NoCredentials - } + async move { ProviderCredential::Credentials { api_key } }.boxed() } else { - ProviderCredential::NoCredentials + let credentials = cx.read_credentials(OPENAI_API_URL); + async move { + if let Some(Some((_, api_key))) = credentials.await.log_err() { + if let Some(api_key) = String::from_utf8(api_key).log_err() { + ProviderCredential::Credentials { api_key } + } else { + ProviderCredential::NoCredentials + } + } else { + ProviderCredential::NoCredentials + } + } + .boxed() } } }; - *self.credential.write() = retrieved_credential.clone(); - retrieved_credential - } - - fn save_credentials(&self, cx: &mut AppContext, credential: ProviderCredential) { - *self.credential.write() = credential.clone(); - match credential { - ProviderCredential::Credentials { api_key } => { - cx.write_credentials(OPENAI_API_URL, "Bearer", api_key.as_bytes()) - .log_err(); - } - _ => {} + async move { + let retrieved_credential = retrieved_credential.await; + *self.credential.write() = retrieved_credential.clone(); + retrieved_credential } + .boxed() } - fn delete_credentials(&self, cx: &mut AppContext) { - cx.delete_credentials(OPENAI_API_URL).log_err(); + fn save_credentials( + &self, + cx: &mut AppContext, + credential: ProviderCredential, + ) -> BoxFuture<()> { + *self.credential.write() = credential.clone(); + let credential = credential.clone(); + let write_credentials = match credential { + ProviderCredential::Credentials { api_key } => { + Some(cx.write_credentials(OPENAI_API_URL, "Bearer", api_key.as_bytes())) + } + _ => None, + }; + + async move { + if let Some(write_credentials) = write_credentials { + write_credentials.await.log_err(); + } + } + .boxed() + } + + fn delete_credentials(&self, cx: &mut AppContext) -> BoxFuture<()> { *self.credential.write() = ProviderCredential::NoCredentials; + let delete_credentials = cx.delete_credentials(OPENAI_API_URL); + async move { + delete_credentials.await.log_err(); + } + .boxed() } } diff --git a/crates/ai/src/test.rs b/crates/ai/src/test.rs index 3d59febbe9..89edc71b0b 100644 --- a/crates/ai/src/test.rs +++ b/crates/ai/src/test.rs @@ -104,11 +104,22 @@ impl CredentialProvider for FakeEmbeddingProvider { fn has_credentials(&self) -> bool { true } - fn retrieve_credentials(&self, _cx: &mut AppContext) -> ProviderCredential { - ProviderCredential::NotNeeded + + fn retrieve_credentials(&self, _cx: &mut AppContext) -> BoxFuture { + async { ProviderCredential::NotNeeded }.boxed() + } + + fn save_credentials( + &self, + _cx: &mut AppContext, + _credential: ProviderCredential, + ) -> BoxFuture<()> { + async {}.boxed() + } + + fn delete_credentials(&self, _cx: &mut AppContext) -> BoxFuture<()> { + async {}.boxed() } - fn save_credentials(&self, _cx: &mut AppContext, _credential: ProviderCredential) {} - fn delete_credentials(&self, _cx: &mut AppContext) {} } #[async_trait] @@ -165,11 +176,22 @@ impl CredentialProvider for FakeCompletionProvider { fn has_credentials(&self) -> bool { true } - fn retrieve_credentials(&self, _cx: &mut AppContext) -> ProviderCredential { - ProviderCredential::NotNeeded + + fn retrieve_credentials(&self, _cx: &mut AppContext) -> BoxFuture { + async { ProviderCredential::NotNeeded }.boxed() + } + + fn save_credentials( + &self, + _cx: &mut AppContext, + _credential: ProviderCredential, + ) -> BoxFuture<()> { + async {}.boxed() + } + + fn delete_credentials(&self, _cx: &mut AppContext) -> BoxFuture<()> { + async {}.boxed() } - fn save_credentials(&self, _cx: &mut AppContext, _credential: ProviderCredential) {} - fn delete_credentials(&self, _cx: &mut AppContext) {} } impl CompletionProvider for FakeCompletionProvider { diff --git a/crates/assistant/src/assistant_panel.rs b/crates/assistant/src/assistant_panel.rs index 097c6424d7..b2c539fcc2 100644 --- a/crates/assistant/src/assistant_panel.rs +++ b/crates/assistant/src/assistant_panel.rs @@ -6,14 +6,12 @@ use crate::{ NewConversation, QuoteSelection, ResetKey, Role, SavedConversation, SavedConversationMetadata, SavedMessage, Split, ToggleFocus, ToggleIncludeConversation, ToggleRetrieveContext, }; - +use ai::prompts::repository_context::PromptCodeSnippet; use ai::{ auth::ProviderCredential, completion::{CompletionProvider, CompletionRequest}, providers::open_ai::{OpenAICompletionProvider, OpenAIRequest, RequestMessage}, }; - -use ai::prompts::repository_context::PromptCodeSnippet; use anyhow::{anyhow, Result}; use chrono::{DateTime, Local}; use client::telemetry::AssistantKind; @@ -220,23 +218,9 @@ impl AssistantPanel { _: &InlineAssist, cx: &mut ViewContext, ) { - let this = if let Some(this) = workspace.panel::(cx) { - if this.update(cx, |assistant, cx| { - if !assistant.has_credentials() { - assistant.load_credentials(cx); - }; - - assistant.has_credentials() - }) { - this - } else { - workspace.focus_panel::(cx); - return; - } - } else { + let Some(assistant) = workspace.panel::(cx) else { return; }; - let active_editor = if let Some(active_editor) = workspace .active_item(cx) .and_then(|item| item.act_as::(cx)) @@ -245,12 +229,32 @@ impl AssistantPanel { } else { return; }; + let project = workspace.project().clone(); - let project = workspace.project(); + if assistant.update(cx, |assistant, _| assistant.has_credentials()) { + assistant.update(cx, |assistant, cx| { + assistant.new_inline_assist(&active_editor, cx, &project) + }); + } else { + let assistant = assistant.downgrade(); + cx.spawn(|workspace, mut cx| async move { + assistant + .update(&mut cx, |assistant, cx| assistant.load_credentials(cx))? + .await; + if assistant.update(&mut cx, |assistant, _| assistant.has_credentials())? { + assistant.update(&mut cx, |assistant, cx| { + assistant.new_inline_assist(&active_editor, cx, &project) + })?; + } else { + workspace.update(&mut cx, |workspace, cx| { + workspace.focus_panel::(cx) + })?; + } - this.update(cx, |assistant, cx| { - assistant.new_inline_assist(&active_editor, cx, project) - }); + anyhow::Ok(()) + }) + .detach_and_log_err(cx) + } } fn new_inline_assist( @@ -290,9 +294,6 @@ impl AssistantPanel { let inline_assist_id = post_inc(&mut self.next_inline_assist_id); let provider = self.completion_provider.clone(); - // Retrieve Credentials Authenticates the Provider - provider.retrieve_credentials(cx); - let codegen = cx.new_model(|cx| { Codegen::new(editor.read(cx).buffer().clone(), codegen_kind, provider, cx) }); @@ -845,11 +846,18 @@ impl AssistantPanel { api_key: api_key.clone(), }; - self.completion_provider.save_credentials(cx, credential); + let completion_provider = self.completion_provider.clone(); + cx.spawn(|this, mut cx| async move { + cx.update(|cx| completion_provider.save_credentials(cx, credential))? + .await; - self.api_key_editor.take(); - self.focus_handle.focus(cx); - cx.notify(); + this.update(&mut cx, |this, cx| { + this.api_key_editor.take(); + this.focus_handle.focus(cx); + cx.notify(); + }) + }) + .detach_and_log_err(cx); } } else { cx.propagate(); @@ -857,10 +865,17 @@ impl AssistantPanel { } fn reset_credentials(&mut self, _: &ResetKey, cx: &mut ViewContext) { - self.completion_provider.delete_credentials(cx); - self.api_key_editor = Some(build_api_key_editor(cx)); - self.focus_handle.focus(cx); - cx.notify(); + let completion_provider = self.completion_provider.clone(); + cx.spawn(|this, mut cx| async move { + cx.update(|cx| completion_provider.delete_credentials(cx))? + .await; + this.update(&mut cx, |this, cx| { + this.api_key_editor = Some(build_api_key_editor(cx)); + this.focus_handle.focus(cx); + cx.notify(); + }) + }) + .detach_and_log_err(cx); } fn toggle_zoom(&mut self, _: &workspace::ToggleZoom, cx: &mut ViewContext) { @@ -1107,8 +1122,16 @@ impl AssistantPanel { self.completion_provider.has_credentials() } - fn load_credentials(&mut self, cx: &mut ViewContext) { - self.completion_provider.retrieve_credentials(cx); + fn load_credentials(&mut self, cx: &mut ViewContext) -> Task<()> { + let completion_provider = self.completion_provider.clone(); + cx.spawn(|_, mut cx| async move { + if let Some(retrieve_credentials) = cx + .update(|cx| completion_provider.retrieve_credentials(cx)) + .log_err() + { + retrieve_credentials.await; + } + }) } } @@ -1314,11 +1337,16 @@ impl Panel for AssistantPanel { fn set_active(&mut self, active: bool, cx: &mut ViewContext) { if active { - self.load_credentials(cx); - - if self.editors.is_empty() { - self.new_conversation(cx); - } + let load_credentials = self.load_credentials(cx); + cx.spawn(|this, mut cx| async move { + load_credentials.await; + this.update(&mut cx, |this, cx| { + if this.editors.is_empty() { + this.new_conversation(cx); + } + }) + }) + .detach_and_log_err(cx); } } diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index 844c9a5eb6..370a2ba6ff 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -700,8 +700,8 @@ impl Client { } } - pub fn has_keychain_credentials(&self, cx: &AsyncAppContext) -> bool { - read_credentials_from_keychain(cx).is_some() + pub async fn has_keychain_credentials(&self, cx: &AsyncAppContext) -> bool { + read_credentials_from_keychain(cx).await.is_some() } #[async_recursion(?Send)] @@ -732,7 +732,7 @@ impl Client { let mut read_from_keychain = false; let mut credentials = self.state.read().credentials.clone(); if credentials.is_none() && try_keychain { - credentials = read_credentials_from_keychain(cx); + credentials = read_credentials_from_keychain(cx).await; read_from_keychain = credentials.is_some(); } if credentials.is_none() { @@ -770,7 +770,7 @@ impl Client { Ok(conn) => { self.state.write().credentials = Some(credentials.clone()); if !read_from_keychain && IMPERSONATE_LOGIN.is_none() { - write_credentials_to_keychain(credentials, cx).log_err(); + write_credentials_to_keychain(credentials, cx).await.log_err(); } futures::select_biased! { @@ -784,7 +784,7 @@ impl Client { Err(EstablishConnectionError::Unauthorized) => { self.state.write().credentials.take(); if read_from_keychain { - delete_credentials_from_keychain(cx).log_err(); + delete_credentials_from_keychain(cx).await.log_err(); self.set_status(Status::SignedOut, cx); self.authenticate_and_connect(false, cx).await } else { @@ -1350,14 +1350,16 @@ impl Client { } } -fn read_credentials_from_keychain(cx: &AsyncAppContext) -> Option { +async fn read_credentials_from_keychain(cx: &AsyncAppContext) -> Option { if IMPERSONATE_LOGIN.is_some() { return None; } let (user_id, access_token) = cx - .update(|cx| cx.read_credentials(&ZED_SERVER_URL).log_err().flatten()) - .ok()??; + .update(|cx| cx.read_credentials(&ZED_SERVER_URL)) + .log_err()? + .await + .log_err()??; Some(Credentials { user_id: user_id.parse().ok()?, @@ -1365,7 +1367,10 @@ fn read_credentials_from_keychain(cx: &AsyncAppContext) -> Option { }) } -fn write_credentials_to_keychain(credentials: Credentials, cx: &AsyncAppContext) -> Result<()> { +async fn write_credentials_to_keychain( + credentials: Credentials, + cx: &AsyncAppContext, +) -> Result<()> { cx.update(move |cx| { cx.write_credentials( &ZED_SERVER_URL, @@ -1373,10 +1378,12 @@ fn write_credentials_to_keychain(credentials: Credentials, cx: &AsyncAppContext) credentials.access_token.as_bytes(), ) })? + .await } -fn delete_credentials_from_keychain(cx: &AsyncAppContext) -> Result<()> { +async fn delete_credentials_from_keychain(cx: &AsyncAppContext) -> Result<()> { cx.update(move |cx| cx.delete_credentials(&ZED_SERVER_URL))? + .await } const WORKTREE_URL_PREFIX: &str = "zed://worktrees/"; diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index dfecffb80a..6f75eafaf2 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -523,17 +523,22 @@ impl AppContext { } /// Writes credentials to the platform keychain. - pub fn write_credentials(&self, url: &str, username: &str, password: &[u8]) -> Result<()> { + pub fn write_credentials( + &self, + url: &str, + username: &str, + password: &[u8], + ) -> Task> { self.platform.write_credentials(url, username, password) } /// Reads credentials from the platform keychain. - pub fn read_credentials(&self, url: &str) -> Result)>> { + pub fn read_credentials(&self, url: &str) -> Task)>>> { self.platform.read_credentials(url) } /// Deletes credentials from the platform keychain. - pub fn delete_credentials(&self, url: &str) -> Result<()> { + pub fn delete_credentials(&self, url: &str) -> Task> { self.platform.delete_credentials(url) } diff --git a/crates/gpui/src/platform.rs b/crates/gpui/src/platform.rs index 900b17bb2c..3d2679dd7e 100644 --- a/crates/gpui/src/platform.rs +++ b/crates/gpui/src/platform.rs @@ -9,7 +9,7 @@ use crate::{ Action, AnyWindowHandle, AsyncWindowContext, BackgroundExecutor, Bounds, DevicePixels, Font, FontId, FontMetrics, FontRun, ForegroundExecutor, GlobalPixels, GlyphId, Keymap, LineLayout, Pixels, PlatformInput, Point, RenderGlyphParams, RenderImageParams, RenderSvgParams, Result, - Scene, SharedString, Size, TaskLabel, WindowContext, + Scene, SharedString, Size, Task, TaskLabel, WindowContext, }; use anyhow::anyhow; use async_task::Runnable; @@ -108,9 +108,9 @@ pub(crate) trait Platform: 'static { fn write_to_clipboard(&self, item: ClipboardItem); fn read_from_clipboard(&self) -> Option; - fn write_credentials(&self, url: &str, username: &str, password: &[u8]) -> Result<()>; - fn read_credentials(&self, url: &str) -> Result)>>; - fn delete_credentials(&self, url: &str) -> Result<()>; + fn write_credentials(&self, url: &str, username: &str, password: &[u8]) -> Task>; + fn read_credentials(&self, url: &str) -> Task)>>>; + fn delete_credentials(&self, url: &str) -> Task>; } /// A handle to a platform's display, e.g. a monitor or laptop screen. diff --git a/crates/gpui/src/platform/mac/platform.rs b/crates/gpui/src/platform/mac/platform.rs index e4a688c2fd..58a759865e 100644 --- a/crates/gpui/src/platform/mac/platform.rs +++ b/crates/gpui/src/platform/mac/platform.rs @@ -3,7 +3,7 @@ use crate::{ Action, AnyWindowHandle, BackgroundExecutor, ClipboardItem, CursorStyle, DisplayId, ForegroundExecutor, Keymap, MacDispatcher, MacDisplay, MacDisplayLinker, MacTextSystem, MacWindow, Menu, MenuItem, PathPromptOptions, Platform, PlatformDisplay, PlatformInput, - PlatformTextSystem, PlatformWindow, Result, SemanticVersion, WindowOptions, + PlatformTextSystem, PlatformWindow, Result, SemanticVersion, Task, WindowOptions, }; use anyhow::anyhow; use block::ConcreteBlock; @@ -856,104 +856,115 @@ impl Platform for MacPlatform { } } - fn write_credentials(&self, url: &str, username: &str, password: &[u8]) -> Result<()> { - let url = CFString::from(url); - let username = CFString::from(username); - let password = CFData::from_buffer(password); + fn write_credentials(&self, url: &str, username: &str, password: &[u8]) -> Task> { + let url = url.to_string(); + let username = username.to_string(); + let password = password.to_vec(); + self.background_executor().spawn(async move { + unsafe { + use security::*; - unsafe { - use security::*; + let url = CFString::from(url.as_str()); + let username = CFString::from(username.as_str()); + let password = CFData::from_buffer(&password); - // First, check if there are already credentials for the given server. If so, then - // update the username and password. - let mut verb = "updating"; - let mut query_attrs = CFMutableDictionary::with_capacity(2); - query_attrs.set(kSecClass as *const _, kSecClassInternetPassword as *const _); - query_attrs.set(kSecAttrServer as *const _, url.as_CFTypeRef()); + // First, check if there are already credentials for the given server. If so, then + // update the username and password. + let mut verb = "updating"; + let mut query_attrs = CFMutableDictionary::with_capacity(2); + query_attrs.set(kSecClass as *const _, kSecClassInternetPassword as *const _); + query_attrs.set(kSecAttrServer as *const _, url.as_CFTypeRef()); - let mut attrs = CFMutableDictionary::with_capacity(4); - attrs.set(kSecClass as *const _, kSecClassInternetPassword as *const _); - attrs.set(kSecAttrServer as *const _, url.as_CFTypeRef()); - attrs.set(kSecAttrAccount as *const _, username.as_CFTypeRef()); - attrs.set(kSecValueData as *const _, password.as_CFTypeRef()); + let mut attrs = CFMutableDictionary::with_capacity(4); + attrs.set(kSecClass as *const _, kSecClassInternetPassword as *const _); + attrs.set(kSecAttrServer as *const _, url.as_CFTypeRef()); + attrs.set(kSecAttrAccount as *const _, username.as_CFTypeRef()); + attrs.set(kSecValueData as *const _, password.as_CFTypeRef()); - let mut status = SecItemUpdate( - query_attrs.as_concrete_TypeRef(), - attrs.as_concrete_TypeRef(), - ); + let mut status = SecItemUpdate( + query_attrs.as_concrete_TypeRef(), + attrs.as_concrete_TypeRef(), + ); - // If there were no existing credentials for the given server, then create them. - if status == errSecItemNotFound { - verb = "creating"; - status = SecItemAdd(attrs.as_concrete_TypeRef(), ptr::null_mut()); + // If there were no existing credentials for the given server, then create them. + if status == errSecItemNotFound { + verb = "creating"; + status = SecItemAdd(attrs.as_concrete_TypeRef(), ptr::null_mut()); + } + + if status != errSecSuccess { + return Err(anyhow!("{} password failed: {}", verb, status)); + } } - - if status != errSecSuccess { - return Err(anyhow!("{} password failed: {}", verb, status)); - } - } - Ok(()) + Ok(()) + }) } - fn read_credentials(&self, url: &str) -> Result)>> { - let url = CFString::from(url); - let cf_true = CFBoolean::true_value().as_CFTypeRef(); + fn read_credentials(&self, url: &str) -> Task)>>> { + let url = url.to_string(); + self.background_executor().spawn(async move { + let url = CFString::from(url.as_str()); + let cf_true = CFBoolean::true_value().as_CFTypeRef(); - unsafe { - use security::*; + unsafe { + use security::*; - // Find any credentials for the given server URL. - let mut attrs = CFMutableDictionary::with_capacity(5); - attrs.set(kSecClass as *const _, kSecClassInternetPassword as *const _); - attrs.set(kSecAttrServer as *const _, url.as_CFTypeRef()); - attrs.set(kSecReturnAttributes as *const _, cf_true); - attrs.set(kSecReturnData as *const _, cf_true); + // Find any credentials for the given server URL. + let mut attrs = CFMutableDictionary::with_capacity(5); + attrs.set(kSecClass as *const _, kSecClassInternetPassword as *const _); + attrs.set(kSecAttrServer as *const _, url.as_CFTypeRef()); + attrs.set(kSecReturnAttributes as *const _, cf_true); + attrs.set(kSecReturnData as *const _, cf_true); - let mut result = CFTypeRef::from(ptr::null()); - let status = SecItemCopyMatching(attrs.as_concrete_TypeRef(), &mut result); - match status { - security::errSecSuccess => {} - security::errSecItemNotFound | security::errSecUserCanceled => return Ok(None), - _ => return Err(anyhow!("reading password failed: {}", status)), + let mut result = CFTypeRef::from(ptr::null()); + let status = SecItemCopyMatching(attrs.as_concrete_TypeRef(), &mut result); + match status { + security::errSecSuccess => {} + security::errSecItemNotFound | security::errSecUserCanceled => return Ok(None), + _ => return Err(anyhow!("reading password failed: {}", status)), + } + + let result = CFType::wrap_under_create_rule(result) + .downcast::() + .ok_or_else(|| anyhow!("keychain item was not a dictionary"))?; + let username = result + .find(kSecAttrAccount as *const _) + .ok_or_else(|| anyhow!("account was missing from keychain item"))?; + let username = CFType::wrap_under_get_rule(*username) + .downcast::() + .ok_or_else(|| anyhow!("account was not a string"))?; + let password = result + .find(kSecValueData as *const _) + .ok_or_else(|| anyhow!("password was missing from keychain item"))?; + let password = CFType::wrap_under_get_rule(*password) + .downcast::() + .ok_or_else(|| anyhow!("password was not a string"))?; + + Ok(Some((username.to_string(), password.bytes().to_vec()))) } - - let result = CFType::wrap_under_create_rule(result) - .downcast::() - .ok_or_else(|| anyhow!("keychain item was not a dictionary"))?; - let username = result - .find(kSecAttrAccount as *const _) - .ok_or_else(|| anyhow!("account was missing from keychain item"))?; - let username = CFType::wrap_under_get_rule(*username) - .downcast::() - .ok_or_else(|| anyhow!("account was not a string"))?; - let password = result - .find(kSecValueData as *const _) - .ok_or_else(|| anyhow!("password was missing from keychain item"))?; - let password = CFType::wrap_under_get_rule(*password) - .downcast::() - .ok_or_else(|| anyhow!("password was not a string"))?; - - Ok(Some((username.to_string(), password.bytes().to_vec()))) - } + }) } - fn delete_credentials(&self, url: &str) -> Result<()> { - let url = CFString::from(url); + fn delete_credentials(&self, url: &str) -> Task> { + let url = url.to_string(); - unsafe { - use security::*; + self.background_executor().spawn(async move { + unsafe { + use security::*; - let mut query_attrs = CFMutableDictionary::with_capacity(2); - query_attrs.set(kSecClass as *const _, kSecClassInternetPassword as *const _); - query_attrs.set(kSecAttrServer as *const _, url.as_CFTypeRef()); + let url = CFString::from(url.as_str()); + let mut query_attrs = CFMutableDictionary::with_capacity(2); + query_attrs.set(kSecClass as *const _, kSecClassInternetPassword as *const _); + query_attrs.set(kSecAttrServer as *const _, url.as_CFTypeRef()); - let status = SecItemDelete(query_attrs.as_concrete_TypeRef()); + let status = SecItemDelete(query_attrs.as_concrete_TypeRef()); - if status != errSecSuccess { - return Err(anyhow!("delete password failed: {}", status)); + if status != errSecSuccess { + return Err(anyhow!("delete password failed: {}", status)); + } } - } - Ok(()) + Ok(()) + }) } } diff --git a/crates/gpui/src/platform/test/platform.rs b/crates/gpui/src/platform/test/platform.rs index 9a33b4b3b5..5aadc4b760 100644 --- a/crates/gpui/src/platform/test/platform.rs +++ b/crates/gpui/src/platform/test/platform.rs @@ -1,6 +1,7 @@ use crate::{ AnyWindowHandle, BackgroundExecutor, ClipboardItem, CursorStyle, DisplayId, ForegroundExecutor, - Keymap, Platform, PlatformDisplay, PlatformTextSystem, TestDisplay, TestWindow, WindowOptions, + Keymap, Platform, PlatformDisplay, PlatformTextSystem, Task, TestDisplay, TestWindow, + WindowOptions, }; use anyhow::{anyhow, Result}; use collections::VecDeque; @@ -280,16 +281,16 @@ impl Platform for TestPlatform { self.current_clipboard_item.lock().clone() } - fn write_credentials(&self, _url: &str, _username: &str, _password: &[u8]) -> Result<()> { - Ok(()) + fn write_credentials(&self, _url: &str, _username: &str, _password: &[u8]) -> Task> { + Task::ready(Ok(())) } - fn read_credentials(&self, _url: &str) -> Result)>> { - Ok(None) + fn read_credentials(&self, _url: &str) -> Task)>>> { + Task::ready(Ok(None)) } - fn delete_credentials(&self, _url: &str) -> Result<()> { - Ok(()) + fn delete_credentials(&self, _url: &str) -> Task> { + Task::ready(Ok(())) } fn double_click_interval(&self) -> std::time::Duration { diff --git a/crates/semantic_index/src/semantic_index.rs b/crates/semantic_index/src/semantic_index.rs index 475ab079dc..62773cced8 100644 --- a/crates/semantic_index/src/semantic_index.rs +++ b/crates/semantic_index/src/semantic_index.rs @@ -278,14 +278,22 @@ impl SemanticIndex { .map(|semantic_index| semantic_index.clone()) } - pub fn authenticate(&mut self, cx: &mut AppContext) -> bool { + pub fn authenticate(&mut self, cx: &mut AppContext) -> Task { if !self.embedding_provider.has_credentials() { - self.embedding_provider.retrieve_credentials(cx); - } else { - return true; - } + let embedding_provider = self.embedding_provider.clone(); + cx.spawn(|cx| async move { + if let Some(retrieve_credentials) = cx + .update(|cx| embedding_provider.retrieve_credentials(cx)) + .log_err() + { + retrieve_credentials.await; + } - self.embedding_provider.has_credentials() + embedding_provider.has_credentials() + }) + } else { + Task::ready(true) + } } pub fn is_authenticated(&self) -> bool { @@ -1005,12 +1013,26 @@ impl SemanticIndex { project: Model, cx: &mut ModelContext, ) -> Task> { - if !self.is_authenticated() { - if !self.authenticate(cx) { - return Task::ready(Err(anyhow!("user is not authenticated"))); - } + if self.is_authenticated() { + self.index_project_internal(project, cx) + } else { + let authenticate = self.authenticate(cx); + cx.spawn(|this, mut cx| async move { + if authenticate.await { + this.update(&mut cx, |this, cx| this.index_project_internal(project, cx))? + .await + } else { + Err(anyhow!("user is not authenticated")) + } + }) } + } + fn index_project_internal( + &mut self, + project: Model, + cx: &mut ModelContext, + ) -> Task> { if !self.projects.contains_key(&project.downgrade()) { let subscription = cx.subscribe(&project, |this, project, event, cx| match event { project::Event::WorktreeAdded | project::Event::WorktreeRemoved(_) => { diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 6486fc5db4..5abb046165 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -376,7 +376,7 @@ async fn authenticate(client: Arc, cx: &AsyncAppContext) -> Result<()> { if client::IMPERSONATE_LOGIN.is_some() { client.authenticate_and_connect(false, &cx).await?; } - } else if client.has_keychain_credentials(&cx) { + } else if client.has_keychain_credentials(&cx).await { client.authenticate_and_connect(true, &cx).await?; } Ok::<_, anyhow::Error>(())