Skip to content

Commit 9ef74b4

Browse files
committed
refactor(registry): inject NpmrcConfig and consolidate URL resolution
Make NpmrcConfig injectable into LiveRegistryClient for testability and future reuse. Consolidate registry URL resolution into a single resolve_url() method. Remove unused imports from update command.
1 parent 6f2ee6e commit 9ef74b4

File tree

5 files changed

+45
-69
lines changed

5 files changed

+45
-69
lines changed

src/commands/update.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
use {
2-
crate::{
3-
commands::{ui, ui::LINE_ENDING},
4-
context::Context,
5-
version_group::VersionGroupVariant,
6-
},
7-
log::{error, warn},
2+
crate::{commands::ui, context::Context, version_group::VersionGroupVariant},
3+
log::error,
84
};
95

106
pub fn run(ctx: Context) -> i32 {

src/instance.rs

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -301,13 +301,7 @@ impl Instance {
301301
Specifier::Alias(alias) => {
302302
let aliased_name = &alias.name;
303303
if !aliased_name.is_empty() {
304-
// JSR aliases are always fetched (they have their own namespace)
305-
if aliased_name.starts_with("@jsr/") {
306-
Some(UpdateUrl {
307-
internal_name: internal_name.clone(),
308-
package_name: aliased_name.clone(),
309-
})
310-
} else if aliased_name == actual_name {
304+
if aliased_name.starts_with("@jsr/") || aliased_name == actual_name {
311305
Some(UpdateUrl {
312306
internal_name: internal_name.clone(),
313307
package_name: aliased_name.clone(),

src/main.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -71,13 +71,11 @@ async fn main() {
7171
}
7272

7373
let registry_client = if is_update_command {
74-
match LiveRegistryClient::new() {
75-
Ok(client) => Some(Arc::new(client) as Arc<dyn crate::registry_client::RegistryClient>),
76-
Err(e) => {
77-
error!("Failed to initialize registry client: {e}");
78-
exit(1);
79-
}
80-
}
74+
let npmrc = npmrc_config_rs::NpmrcConfig::load().unwrap_or_else(|e| {
75+
error!("Failed to load .npmrc config: {e}");
76+
exit(1);
77+
});
78+
Some(Arc::new(LiveRegistryClient::new(npmrc)) as Arc<dyn crate::registry_client::RegistryClient>)
8179
} else {
8280
None
8381
};

src/registry_client.rs

Lines changed: 20 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,6 @@ pub enum RegistryError {
2323

2424
#[error("HTTP error for package '{url}': {status}")]
2525
HttpError { url: String, status: StatusCode },
26-
27-
#[error("Failed to load .npmrc config: {0}")]
28-
NpmrcLoadError(#[from] npmrc_config_rs::Error),
2926
}
3027

3128
/// Registry responses such as https://registry.npmjs.org/colors
@@ -60,18 +57,10 @@ pub struct LiveRegistryClient {
6057
#[async_trait::async_trait]
6158
impl RegistryClient for LiveRegistryClient {
6259
async fn fetch(&self, update_url: &UpdateUrl) -> Result<AllPackageVersions, RegistryError> {
63-
// Build full URL from package name using .npmrc config
64-
let registry_base = self.registry_url(&update_url.package_name);
65-
let full_url = registry_base
66-
.join(&update_url.package_name)
67-
.map_err(|e| RegistryError::FetchError {
68-
url: update_url.package_name.clone(),
69-
source: Box::new(e),
70-
})?;
60+
let (full_url, registry_base) = self.resolve_url(&update_url.package_name)?;
7161
let url_str = full_url.to_string();
7262

73-
// Build request with auth if credentials exist
74-
let mut req = self.client.get(full_url.clone()).header(ACCEPT, "application/json");
63+
let mut req = self.client.get(full_url).header(ACCEPT, "application/json");
7564
if let Some(creds) = self.npmrc.credentials_for(&registry_base) {
7665
req = match &creds {
7766
Credentials::Token { token, .. } => req.bearer_auth(token),
@@ -94,10 +83,7 @@ impl RegistryClient for LiveRegistryClient {
9483
let versions = package_meta
9584
.versions
9685
.into_iter()
97-
.filter(|(_, metadata)| {
98-
// Filter out deprecated versions by checking if "deprecated" field exists
99-
metadata.get("deprecated").is_none()
100-
})
86+
.filter(|(_, metadata)| metadata.get("deprecated").is_none())
10187
.map(|(version, _)| version)
10288
.collect();
10389
Ok(AllPackageVersions {
@@ -121,25 +107,31 @@ impl RegistryClient for LiveRegistryClient {
121107
}
122108

123109
impl LiveRegistryClient {
124-
pub fn new() -> Result<Self, RegistryError> {
125-
let npmrc = NpmrcConfig::load()?;
126-
Ok(LiveRegistryClient {
110+
pub fn new(npmrc: NpmrcConfig) -> Self {
111+
LiveRegistryClient {
127112
client: Client::builder()
128113
.connect_timeout(Duration::from_secs(5))
129114
.timeout(Duration::from_secs(30))
130115
.build()
131116
.expect("Failed to build reqwest client"),
132117
npmrc,
133-
})
118+
}
134119
}
135120

136-
/// Resolve registry URL for a package, with JSR fallback
137-
pub fn registry_url(&self, package_name: &str) -> Url {
138-
let url = self.npmrc.registry_for(package_name);
139-
// Fallback: @jsr/* uses npm.jsr.io if not explicitly configured
140-
if package_name.starts_with("@jsr/") && url.host_str() == Some("registry.npmjs.org") {
141-
return Url::parse("https://npm.jsr.io/").unwrap();
121+
/// Resolve the full registry URL for a package name.
122+
///
123+
/// Uses .npmrc scoped registry config, with a fallback to npm.jsr.io
124+
/// for @jsr/* packages that have no explicit registry configured.
125+
/// Returns (full_url, registry_base) so callers can look up credentials.
126+
pub fn resolve_url(&self, package_name: &str) -> Result<(Url, Url), RegistryError> {
127+
let mut registry_base = self.npmrc.registry_for(package_name);
128+
if package_name.starts_with("@jsr/") && registry_base.host_str() == Some("registry.npmjs.org") {
129+
registry_base = Url::parse("https://npm.jsr.io/").unwrap();
142130
}
143-
url
131+
let full_url = registry_base.join(package_name).map_err(|e| RegistryError::FetchError {
132+
url: package_name.to_string(),
133+
source: Box::new(e),
134+
})?;
135+
Ok((full_url, registry_base))
144136
}
145137
}

src/registry_client_test.rs

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use {
22
crate::registry_client::{LiveRegistryClient, PackageMeta},
3+
npmrc_config_rs::NpmrcConfig,
34
serde_json::json,
45
std::collections::BTreeMap,
56
};
@@ -65,36 +66,31 @@ fn includes_all_versions_when_none_deprecated() {
6566
assert!(versions.contains(&"3.0.0".to_string()));
6667
}
6768

69+
fn make_client() -> LiveRegistryClient {
70+
let npmrc = NpmrcConfig::load().expect("Failed to load .npmrc config");
71+
LiveRegistryClient::new(npmrc)
72+
}
73+
6874
#[test]
6975
fn registry_client_can_be_created() {
70-
// Verifies that LiveRegistryClient::new() succeeds
71-
// This loads .npmrc from standard locations (may be empty)
72-
let result = LiveRegistryClient::new();
73-
assert!(result.is_ok(), "Failed to create registry client: {:?}", result.err());
76+
make_client();
7477
}
7578

7679
#[test]
77-
fn registry_url_returns_default_for_regular_packages() {
78-
let client = LiveRegistryClient::new().unwrap();
79-
let url = client.registry_url("react");
80-
// Should use default npm registry (or user's configured default)
81-
// We can't assert exact URL since user might have custom config,
82-
// but we can verify it's a valid URL
80+
fn resolve_url_returns_default_for_regular_packages() {
81+
let client = make_client();
82+
let (url, _) = client.resolve_url("react").unwrap();
8383
assert!(url.scheme() == "https" || url.scheme() == "http");
8484
assert!(url.host_str().is_some());
8585
}
8686

8787
#[test]
88-
fn registry_url_uses_jsr_fallback_for_jsr_packages() {
89-
let client = LiveRegistryClient::new().unwrap();
90-
let url = client.registry_url("@jsr/luca__cases");
91-
// JSR packages should use npm.jsr.io when not explicitly configured
92-
// If user has @jsr:registry configured, this will use that instead
93-
let host = url.host_str().unwrap();
94-
// Either npm.jsr.io (fallback) or user's configured JSR registry
95-
assert!(
96-
host == "npm.jsr.io" || host != "registry.npmjs.org",
97-
"Expected JSR package to NOT use registry.npmjs.org, got: {}",
98-
url
88+
fn resolve_url_uses_jsr_fallback_for_jsr_packages() {
89+
let client = make_client();
90+
let (url, _) = client.resolve_url("@jsr/luca__cases").unwrap();
91+
assert_ne!(
92+
url.host_str().unwrap(),
93+
"registry.npmjs.org",
94+
"Expected JSR package to NOT use registry.npmjs.org, got: {url}",
9995
);
10096
}

0 commit comments

Comments
 (0)