From 62e3f5060ef9cc6a31b3e496dd15548c0abd07e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Fri, 15 Dec 2023 11:27:10 +0100 Subject: [PATCH] refactor: check if scope and package exist before publish (#21575) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Bartek Iwańczuk Co-authored-by: Luca Casonato --- cli/args/mod.rs | 28 +--- cli/tests/integration/publish_tests.rs | 14 +- cli/tests/testdata/publish/successful.out | 2 +- cli/tools/registry/api.rs | 30 ++++ cli/tools/registry/mod.rs | 158 ++++++++++++++++++++-- test_util/src/servers/registry.rs | 9 +- 6 files changed, 191 insertions(+), 50 deletions(-) diff --git a/cli/args/mod.rs b/cli/args/mod.rs index 17711371a4..2cc4517fcc 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -132,31 +132,9 @@ pub fn deno_registry_url() -> &'static Url { pub fn deno_registry_api_url() -> &'static Url { static DENO_REGISTRY_API_URL: Lazy = Lazy::new(|| { - let env_var_name = "DENO_REGISTRY_API_URL"; - if let Ok(registry_url) = std::env::var(env_var_name) { - // ensure there is a trailing slash for the directory - let registry_url = format!("{}/", registry_url.trim_end_matches('/')); - match Url::parse(®istry_url) { - Ok(url) => { - return url; - } - Err(err) => { - log::debug!( - "Invalid {} environment variable: {:#}", - env_var_name, - err, - ); - } - } - } - - let host = deno_graph::source::DEFAULT_DENO_REGISTRY_URL - .host_str() - .unwrap(); - - let mut url = deno_graph::source::DEFAULT_DENO_REGISTRY_URL.clone(); - url.set_host(Some(&format!("api.{}", host))).unwrap(); - url + let mut deno_registry_api_url = deno_registry_url().clone(); + deno_registry_api_url.set_path("api/"); + deno_registry_api_url }); &DENO_REGISTRY_API_URL diff --git a/cli/tests/integration/publish_tests.rs b/cli/tests/integration/publish_tests.rs index 6a40eabf60..9ba0a00c6a 100644 --- a/cli/tests/integration/publish_tests.rs +++ b/cli/tests/integration/publish_tests.rs @@ -3,16 +3,10 @@ static TEST_REGISTRY_URL: &str = "http://127.0.0.1:4250"; pub fn env_vars_for_registry() -> Vec<(String, String)> { - vec![ - ( - "DENO_REGISTRY_URL".to_string(), - TEST_REGISTRY_URL.to_string(), - ), - ( - "DENO_REGISTRY_API_URL".to_string(), - TEST_REGISTRY_URL.to_string(), - ), - ] + vec![( + "DENO_REGISTRY_URL".to_string(), + TEST_REGISTRY_URL.to_string(), + )] } itest!(no_token { diff --git a/cli/tests/testdata/publish/successful.out b/cli/tests/testdata/publish/successful.out index 1049bbdb43..a6a6a9bd46 100644 --- a/cli/tests/testdata/publish/successful.out +++ b/cli/tests/testdata/publish/successful.out @@ -1,3 +1,3 @@ Publishing @foo/bar@1.0.0 ... Successfully published @foo/bar@1.0.0 -http://127.0.0.1:4250/@foo/bar/1.0.0_meta.json +Visit http://127.0.0.1:4250/@foo/bar@1.0.0 for details diff --git a/cli/tools/registry/api.rs b/cli/tools/registry/api.rs index b174ea367a..2c08ef52d4 100644 --- a/cli/tools/registry/api.rs +++ b/cli/tools/registry/api.rs @@ -1,5 +1,6 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. +use deno_core::error::AnyError; use deno_core::serde_json; use deno_runtime::deno_fetch::reqwest; use serde::de::DeserializeOwned; @@ -108,3 +109,32 @@ pub async fn parse_response( x_deno_ray, }) } + +pub async fn get_scope( + client: &reqwest::Client, + registry_api_url: &str, + scope: &str, +) -> Result { + let scope_url = format!("{}scope/{}", registry_api_url, scope); + let response = client.get(&scope_url).send().await?; + Ok(response) +} + +pub fn get_package_api_url( + registry_api_url: &str, + scope: &str, + package: &str, +) -> String { + format!("{}scope/{}packages/{}", registry_api_url, scope, package) +} + +pub async fn get_package( + client: &reqwest::Client, + registry_api_url: &str, + scope: &str, + package: &str, +) -> Result { + let package_url = get_package_api_url(registry_api_url, scope, package); + let response = client.get(&package_url).send().await?; + Ok(response) +} diff --git a/cli/tools/registry/mod.rs b/cli/tools/registry/mod.rs index 012294a3e8..06d5a00910 100644 --- a/cli/tools/registry/mod.rs +++ b/cli/tools/registry/mod.rs @@ -2,6 +2,7 @@ use std::collections::HashMap; use std::fmt::Write; +use std::io::IsTerminal; use std::rc::Rc; use std::sync::Arc; @@ -26,6 +27,7 @@ use serde::Serialize; use sha2::Digest; use crate::args::deno_registry_api_url; +use crate::args::deno_registry_url; use crate::args::Flags; use crate::args::PublishFlags; use crate::factory::CliFactory; @@ -41,6 +43,11 @@ use auth::get_auth_method; use auth::AuthMethod; use publish_order::PublishOrderGraph; +fn ring_bell() { + // ASCII code for the bell character. + print!("\x07"); +} + struct PreparedPublishPackage { scope: String, package: String, @@ -224,8 +231,7 @@ async fn get_auth_headers( println!(" @{}/{}", packages[0].scope, packages[0].package); } - // ASCII code for the bell character. - print!("\x07"); + ring_bell(); println!("{}", colors::gray("Waiting...")); let interval = std::time::Duration::from_secs(auth.poll_interval); @@ -332,6 +338,115 @@ async fn get_auth_headers( Ok(authorizations) } +/// Check if both `scope` and `package` already exist, if not return +/// a URL to the management panel to create them. +async fn check_if_scope_and_package_exist( + client: &reqwest::Client, + registry_api_url: &str, + registry_manage_url: &str, + scope: &str, + package: &str, +) -> Result, AnyError> { + let mut needs_scope = false; + let mut needs_package = false; + + let response = api::get_scope(client, registry_api_url, scope).await?; + if response.status() == 404 { + needs_scope = true; + } + + let response = + api::get_package(client, registry_api_url, scope, package).await?; + if response.status() == 404 { + needs_package = true; + } + + if needs_scope || needs_package { + let create_url = format!( + "{}new?scope={}&package={}&from=cli", + registry_manage_url, scope, package + ); + return Ok(Some(create_url)); + } + + Ok(None) +} + +async fn ensure_scopes_and_packages_exist( + client: &reqwest::Client, + registry_api_url: String, + registry_manage_url: String, + packages: Vec>, +) -> Result<(), AnyError> { + if !std::io::stdin().is_terminal() { + let mut missing_packages_lines = vec![]; + for package in packages { + let maybe_create_package_url = check_if_scope_and_package_exist( + client, + ®istry_api_url, + ®istry_manage_url, + &package.scope, + &package.package, + ) + .await?; + + if let Some(create_package_url) = maybe_create_package_url { + missing_packages_lines.push(format!(" - {}", create_package_url)); + } + } + + if !missing_packages_lines.is_empty() { + bail!( + "Following packages don't exist, follow the links and create them:\n{}", + missing_packages_lines.join("\n") + ); + } + return Ok(()); + } + + for package in packages { + let maybe_create_package_url = check_if_scope_and_package_exist( + client, + ®istry_api_url, + ®istry_manage_url, + &package.scope, + &package.package, + ) + .await?; + + let Some(create_package_url) = maybe_create_package_url else { + continue; + }; + + ring_bell(); + println!( + "'@{}/{}' doesn't exist yet. Visit {} to create the package", + &package.scope, + &package.package, + colors::cyan_with_underline(create_package_url) + ); + println!("{}", colors::gray("Waiting...")); + + let package_api_url = api::get_package_api_url( + ®istry_api_url, + &package.scope, + &package.package, + ); + + loop { + tokio::time::sleep(std::time::Duration::from_secs(3)).await; + let response = client.get(&package_api_url).send().await?; + if response.status() == 200 { + let name = format!("@{}/{}", package.scope, package.package); + println!("Package {} created", colors::green(name)); + break; + } + } + } + + Ok(()) +} + async fn perform_publish( http_client: &Arc, mut publish_order_graph: PublishOrderGraph, @@ -339,7 +454,8 @@ async fn perform_publish( auth_method: AuthMethod, ) -> Result<(), AnyError> { let client = http_client.client()?; - let registry_url = deno_registry_api_url().to_string(); + let registry_api_url = deno_registry_api_url().to_string(); + let registry_url = deno_registry_url().to_string(); let packages = prepared_package_by_name .values() @@ -351,8 +467,16 @@ async fn perform_publish( .collect::>(); print_diagnostics(diagnostics); + ensure_scopes_and_packages_exist( + client, + registry_api_url.clone(), + registry_url.clone(), + packages.clone(), + ) + .await?; + let mut authorizations = - get_auth_headers(client, registry_url.clone(), packages, auth_method) + get_auth_headers(client, registry_api_url.clone(), packages, auth_method) .await?; assert_eq!(prepared_package_by_name.len(), authorizations.len()); @@ -369,14 +493,21 @@ async fn perform_publish( package.version.clone(), )) .unwrap(); + let registry_api_url = registry_api_url.clone(); let registry_url = registry_url.clone(); let http_client = http_client.clone(); futures.spawn(async move { let display_name = format!("@{}/{}@{}", package.scope, package.package, package.version); - publish_package(&http_client, package, ®istry_url, &authorization) - .await - .with_context(|| format!("Failed to publish {}", display_name))?; + publish_package( + &http_client, + package, + ®istry_api_url, + ®istry_url, + &authorization, + ) + .await + .with_context(|| format!("Failed to publish {}", display_name))?; Ok(package_name) }); } @@ -397,6 +528,7 @@ async fn perform_publish( async fn publish_package( http_client: &HttpClient, package: Rc, + registry_api_url: &str, registry_url: &str, authorization: &str, ) -> Result<(), AnyError> { @@ -411,7 +543,7 @@ async fn publish_package( let url = format!( "{}scopes/{}/packages/{}/versions/{}", - registry_url, package.scope, package.package, package.version + registry_api_url, package.scope, package.package, package.version ); let response = client @@ -449,7 +581,7 @@ async fn publish_package( while task.status != "success" && task.status != "failure" { tokio::time::sleep(interval).await; let resp = client - .get(format!("{}publish_status/{}", registry_url, task.id)) + .get(format!("{}publish_status/{}", registry_api_url, task.id)) .send() .await .with_context(|| { @@ -486,10 +618,12 @@ async fn publish_package( package.package, package.version ); - // TODO(bartlomieju): return something more useful here println!( - "{}@{}/{}/{}_meta.json", - registry_url, package.scope, package.package, package.version + "{}", + colors::gray(format!( + "Visit {}@{}/{}@{} for details", + registry_url, package.scope, package.package, package.version + )) ); Ok(()) } diff --git a/test_util/src/servers/registry.rs b/test_util/src/servers/registry.rs index 6bb2cf1fcf..56a9e31b0e 100644 --- a/test_util/src/servers/registry.rs +++ b/test_util/src/servers/registry.rs @@ -28,7 +28,12 @@ async fn registry_server_handler( ) -> Result, hyper::http::Error> { let path = req.uri().path(); - if path.starts_with("/scopes/") { + // TODO(bartlomieju): add a proper router here + if path.starts_with("/api/scope/") { + let body = serde_json::to_string_pretty(&json!({})).unwrap(); + let res = Response::new(Body::from(body)); + return Ok(res); + } else if path.starts_with("/api/scopes/") { let body = serde_json::to_string_pretty(&json!({ "id": "sdfwqer-sffg-qwerasdf", "status": "success", @@ -37,7 +42,7 @@ async fn registry_server_handler( .unwrap(); let res = Response::new(Body::from(body)); return Ok(res); - } else if path.starts_with("/publish_status/") { + } else if path.starts_with("/api/publish_status/") { let body = serde_json::to_string_pretty(&json!({ "id": "sdfwqer-qwer-qwerasdf", "status": "success",