From dd9618bca5924ea7978061757df87644cafc8ba0 Mon Sep 17 00:00:00 2001 From: Kris Date: Wed, 15 Jul 2026 15:16:35 +0800 Subject: [PATCH] Support GITHUB_TOKEN and graceful fallback for JDTLS/Lombok downloads - Add support for GITHUB_TOKEN/GH_TOKEN env vars in get_latest_versions_from_tag to avoid GitHub API rate limiting (60/hr -> 5000/hr) - Add graceful fallback in get_or_download: when fetch_latest_version() or download() fails, fall back to locally installed version if available - Add unit tests for CheckUpdates logic - Fixes https://github.com/zed-extensions/java/issues/281 --- src/debugger.rs | 3 ++- src/downloadable.rs | 58 ++++++++++++++++++++++++++++++++++++--- src/jdk.rs | 3 ++- src/jdtls.rs | 12 +++++---- src/proxy.rs | 7 ++--- src/task.rs | 7 ++--- src/util.rs | 66 ++++++++++++++++++++++++++++++++++++++------- 7 files changed, 131 insertions(+), 25 deletions(-) diff --git a/src/debugger.rs b/src/debugger.rs index bf7bedc..dceb8fd 100644 --- a/src/debugger.rs +++ b/src/debugger.rs @@ -420,7 +420,7 @@ impl Downloadable for Debugger { self.plugin_path.is_some() } - fn fetch_latest_version(&self) -> zed::Result { + fn fetch_latest_version(&self, _worktree: &Worktree) -> zed::Result { Ok("0.53.2".to_string()) } @@ -428,6 +428,7 @@ impl Downloadable for Debugger { &mut self, _version: &str, language_server_id: &LanguageServerId, + _worktree: &Worktree, ) -> zed::Result { self.get_or_download_fork(language_server_id) } diff --git a/src/downloadable.rs b/src/downloadable.rs index da1cebd..ef1cf2d 100644 --- a/src/downloadable.rs +++ b/src/downloadable.rs @@ -11,12 +11,13 @@ pub trait Downloadable { fn loaded(&self) -> bool; - fn fetch_latest_version(&self) -> zed::Result; + fn fetch_latest_version(&self, worktree: &Worktree) -> zed::Result; fn download( &mut self, version: &str, language_server_id: &LanguageServerId, + worktree: &Worktree, ) -> zed::Result; fn get_or_download( @@ -36,8 +37,8 @@ pub trait Downloadable { } let downloaded = self - .fetch_latest_version() - .and_then(|version| self.download(&version, language_server_id)); + .fetch_latest_version(worktree) + .and_then(|version| self.download(&version, language_server_id, worktree)); match downloaded { Ok(path) => Ok(path), @@ -64,3 +65,54 @@ pub trait Downloadable { None } } + +#[cfg(test)] +mod fallback_tests { + use std::path::PathBuf; + + use serde_json::json; + + use super::*; + + #[test] + fn test_check_updates_always_allows_download() { + let result = should_use_local_or_download(&None, None, "jdtls").unwrap(); + assert!(result.is_none(), "Always mode should allow download"); + } + + #[test] + fn test_check_updates_always_with_local_still_downloads() { + let local = PathBuf::from("/mock/jdtls/1.44.0"); + let result = should_use_local_or_download(&None, Some(local), "jdtls").unwrap(); + assert!(result.is_none(), "Always mode downloads even with local"); + } + + #[test] + fn test_check_updates_never_with_local_uses_it() { + let config = Some(json!({"check_updates": "never"})); + let local = PathBuf::from("/mock/jdtls/1.44.0"); + let result = should_use_local_or_download(&config, Some(local.clone()), "jdtls").unwrap(); + assert_eq!(result, Some(local)); + } + + #[test] + fn test_check_updates_never_without_local_is_error() { + let config = Some(json!({"check_updates": "never"})); + let result = should_use_local_or_download(&config, None, "jdtls"); + assert!(result.is_err()); + } + + #[test] + fn test_check_updates_once_with_local_uses_it() { + let config = Some(json!({"check_updates": "once"})); + let local = PathBuf::from("/mock/jdtls/1.44.0"); + let result = should_use_local_or_download(&config, Some(local.clone()), "jdtls").unwrap(); + assert_eq!(result, Some(local)); + } + + #[test] + fn test_default_is_always() { + let result = should_use_local_or_download(&None, None, "test").unwrap(); + assert!(result.is_none(), "Default should be Always (None)"); + } +} diff --git a/src/jdk.rs b/src/jdk.rs index 10082f3..1e01d07 100644 --- a/src/jdk.rs +++ b/src/jdk.rs @@ -62,7 +62,7 @@ impl Downloadable for Jdk { self.cached_path.is_some() } - fn fetch_latest_version(&self) -> zed::Result { + fn fetch_latest_version(&self, _worktree: &Worktree) -> zed::Result { Ok(zed::latest_github_release( CORRETTO_REPO, zed_extension_api::GithubReleaseOptions { @@ -80,6 +80,7 @@ impl Downloadable for Jdk { &mut self, version: &str, language_server_id: &LanguageServerId, + _worktree: &Worktree, ) -> zed::Result { let jdk_path = get_curr_dir() .map_err(|err| format!("Failed to get current directory for JDK installation: {err}"))? diff --git a/src/jdtls.rs b/src/jdtls.rs index 24ada2d..c243645 100644 --- a/src/jdtls.rs +++ b/src/jdtls.rs @@ -71,8 +71,8 @@ impl Downloadable for Jdtls { self.cached_path.is_some() } - fn fetch_latest_version(&self) -> zed::Result { - let (last, _) = get_latest_versions_from_tag(JDTLS_REPO) + fn fetch_latest_version(&self, worktree: &Worktree) -> zed::Result { + let (last, _) = get_latest_versions_from_tag(JDTLS_REPO, worktree) .map_err(|err| format!("Failed to fetch JDTLS versions from {JDTLS_REPO}: {err}"))?; Ok(last) } @@ -81,13 +81,14 @@ impl Downloadable for Jdtls { &mut self, _version: &str, language_server_id: &LanguageServerId, + worktree: &Worktree, ) -> zed::Result { set_language_server_installation_status( language_server_id, &LanguageServerInstallationStatus::CheckingForUpdate, ); - let (last, second_last) = get_latest_versions_from_tag(JDTLS_REPO) + let (last, second_last) = get_latest_versions_from_tag(JDTLS_REPO, worktree) .map_err(|err| format!("Failed to fetch JDTLS versions from {JDTLS_REPO}: {err}"))?; let (latest_version, latest_version_build) = download_jdtls_milestone(last.as_ref()) @@ -181,8 +182,8 @@ impl Downloadable for Lombok { self.cached_path.is_some() } - fn fetch_latest_version(&self) -> zed::Result { - let (latest_version, _) = get_latest_versions_from_tag(LOMBOK_REPO) + fn fetch_latest_version(&self, worktree: &Worktree) -> zed::Result { + let (latest_version, _) = get_latest_versions_from_tag(LOMBOK_REPO, worktree) .map_err(|err| format!("Failed to fetch Lombok versions from {LOMBOK_REPO}: {err}"))?; Ok(latest_version) } @@ -191,6 +192,7 @@ impl Downloadable for Lombok { &mut self, version: &str, language_server_id: &LanguageServerId, + _worktree: &Worktree, ) -> zed::Result { let prefix = LOMBOK_INSTALL_PATH; let jar_name = format!("lombok-{version}.jar"); diff --git a/src/proxy.rs b/src/proxy.rs index 6a92e78..a5e4d8f 100644 --- a/src/proxy.rs +++ b/src/proxy.rs @@ -56,7 +56,7 @@ impl Downloadable for Proxy { self.cached_path.is_some() } - fn fetch_latest_version(&self) -> zed::Result { + fn fetch_latest_version(&self, _worktree: &Worktree) -> zed::Result { // The proxy is built and released together with the extension, so the // matching release is the one tagged with the extension's own version. Ok(format!("v{}", env!("CARGO_PKG_VERSION"))) @@ -66,6 +66,7 @@ impl Downloadable for Proxy { &mut self, version: &str, language_server_id: &LanguageServerId, + _worktree: &Worktree, ) -> zed::Result { let (name, file_type) = asset_name()?; let bin_path = format!("{PROXY_INSTALL_PATH}/{version}/{}", proxy_exec()); @@ -127,8 +128,8 @@ impl Downloadable for Proxy { } let downloaded = self - .fetch_latest_version() - .and_then(|version| self.download(&version, language_server_id)); + .fetch_latest_version(worktree) + .and_then(|version| self.download(&version, language_server_id, worktree)); let download_err = match downloaded { Ok(path) => return Ok(path), diff --git a/src/task.rs b/src/task.rs index c64c574..11f03ad 100644 --- a/src/task.rs +++ b/src/task.rs @@ -45,7 +45,7 @@ impl Downloadable for TaskHelper { self.cached_path.is_some() } - fn fetch_latest_version(&self) -> zed::Result { + fn fetch_latest_version(&self, _worktree: &Worktree) -> zed::Result { // The task helper is built and released together with the extension, so // the matching release is the one tagged with the extension's own version. Ok(format!("v{}", env!("CARGO_PKG_VERSION"))) @@ -55,6 +55,7 @@ impl Downloadable for TaskHelper { &mut self, version: &str, language_server_id: &LanguageServerId, + _worktree: &Worktree, ) -> zed::Result { let (name, file_type) = asset_name()?; let bin_path = format!( @@ -119,8 +120,8 @@ impl Downloadable for TaskHelper { } let downloaded = self - .fetch_latest_version() - .and_then(|version| self.download(&version, language_server_id)); + .fetch_latest_version(worktree) + .and_then(|version| self.download(&version, language_server_id, worktree)); let download_err = match downloaded { Ok(path) => return Ok(path), diff --git a/src/util.rs b/src/util.rs index 72fb41c..fbe7330 100644 --- a/src/util.rs +++ b/src/util.rs @@ -260,16 +260,28 @@ pub fn get_java_major_version(java_executable: &PathBuf) -> zed::Result { /// * Could not fetch tags from Github /// * Failed to deserialize response /// * Unexpected Github response format -pub fn get_latest_versions_from_tag(repo: &str) -> zed::Result<(String, Option)> { +pub fn get_latest_versions_from_tag( + repo: &str, + worktree: &Worktree, +) -> zed::Result<(String, Option)> { + let mut request = HttpRequest::builder() + .method(HttpMethod::Get) + .url(format!("https://api.github.com/repos/{repo}/tags")); + + // Use GITHUB_TOKEN or GH_TOKEN environment variable if available + // to avoid GitHub API rate limiting (60 req/hr unauthenticated vs 5000/hr authenticated). + if let Some(token) = github_token(&worktree.shell_env()) { + request = request.header("Authorization", format!("token {token}")); + } + + let request = request + .build() + .map_err(|err| format!("{TAG_RETRIEVAL_ERROR}: {err}"))?; + let tags_response_body = serde_json::from_slice::( - &fetch( - &HttpRequest::builder() - .method(HttpMethod::Get) - .url(format!("https://api.github.com/repos/{repo}/tags")) - .build()?, - ) - .map_err(|err| format!("{TAG_RETRIEVAL_ERROR}: {err}"))? - .body, + &fetch(&request) + .map_err(|err| format!("{TAG_RETRIEVAL_ERROR}: {err}"))? + .body, ) .map_err(|err| format!("{TAG_RESPONSE_ERROR}: {err}"))?; @@ -284,6 +296,15 @@ pub fn get_latest_versions_from_tag(repo: &str) -> zed::Result<(String, Option Option<&str> { + ["GITHUB_TOKEN", "GH_TOKEN"].into_iter().find_map(|name| { + shell_env + .iter() + .find(|(key, value)| key == name && !value.is_empty()) + .map(|(_, value)| value.as_str()) + }) +} + fn get_tag_at(github_tags: &Value, index: usize) -> Option<&str> { github_tags.as_array().and_then(|tag| { tag.get(index).and_then(|latest_tag| { @@ -491,6 +512,33 @@ mod tests { args: ArgsStringOrList, } + #[test] + fn github_token_prefers_github_token() { + let env = vec![ + ("GH_TOKEN".to_string(), "gh".to_string()), + ("GITHUB_TOKEN".to_string(), "github".to_string()), + ]; + + assert_eq!(github_token(&env), Some("github")); + } + + #[test] + fn github_token_falls_back_to_gh_token() { + let env = vec![ + ("GITHUB_TOKEN".to_string(), String::new()), + ("GH_TOKEN".to_string(), "gh".to_string()), + ]; + + assert_eq!(github_token(&env), Some("gh")); + } + + #[test] + fn github_token_ignores_missing_and_empty_tokens() { + let env = vec![("GITHUB_TOKEN".to_string(), String::new())]; + + assert_eq!(github_token(&env), None); + } + #[test] fn test_args_list_with_spaces_quotes_elements() { let json = std::fs::read_to_string("testdata/args_with_spaces.json").unwrap();