From 7aaea85a2d9ac6c6751ebf86eb13568ab2468d86 Mon Sep 17 00:00:00 2001 From: daimond113 <72147841+daimond113@users.noreply.github.com> Date: Wed, 14 Aug 2024 19:55:58 +0200 Subject: [PATCH] feat: store more package info in index --- registry/src/auth.rs | 6 +- registry/src/endpoints/package_version.rs | 42 +++--- registry/src/endpoints/package_versions.rs | 2 + registry/src/endpoints/publish_version.rs | 156 +++++++++++++-------- registry/src/endpoints/search.rs | 2 + registry/src/package.rs | 21 +-- src/cli/commands/publish.rs | 15 +- src/main.rs | 4 +- src/manifest/mod.rs | 6 +- src/resolver.rs | 3 +- src/source/pesde/mod.rs | 6 + 11 files changed, 158 insertions(+), 105 deletions(-) diff --git a/registry/src/auth.rs b/registry/src/auth.rs index b145eae..b97fad3 100644 --- a/registry/src/auth.rs +++ b/registry/src/auth.rs @@ -37,15 +37,15 @@ pub async fn authentication( }; let token = if token.to_lowercase().starts_with("bearer ") { - token[7..].to_string() - } else { token.to_string() + } else { + format!("Bearer {token}") }; let response = match app_state .reqwest_client .get("https://api.github.com/user") - .header(reqwest::header::AUTHORIZATION, format!("Bearer {token}")) + .header(reqwest::header::AUTHORIZATION, token) .send() .await .and_then(|res| res.error_for_status()) diff --git a/registry/src/endpoints/package_version.rs b/registry/src/endpoints/package_version.rs index bff807b..0c5fc53 100644 --- a/registry/src/endpoints/package_version.rs +++ b/registry/src/endpoints/package_version.rs @@ -1,19 +1,21 @@ -use actix_web::{http::header::ACCEPT, web, HttpRequest, HttpResponse, Responder}; -use rusty_s3::{actions::GetObject, S3Action}; -use semver::Version; -use serde::{Deserialize, Deserializer}; -use std::collections::BTreeSet; - use crate::{ error::Error, package::{s3_name, PackageResponse, S3_SIGN_DURATION}, AppState, }; +use actix_web::{ + http::header::{ACCEPT, LOCATION}, + web, HttpRequest, HttpResponse, Responder, +}; use pesde::{ manifest::target::TargetKind, names::PackageName, source::{git_index::GitBasedSource, pesde::IndexFile}, }; +use rusty_s3::{actions::GetObject, S3Action}; +use semver::Version; +use serde::{Deserialize, Deserializer}; +use std::collections::BTreeSet; #[derive(Debug)] pub enum VersionRequest { @@ -68,29 +70,27 @@ pub async fn get_package_version( return Ok(HttpResponse::NotFound().finish()); }; - if request + let accept = request .headers() .get(ACCEPT) .and_then(|accept| accept.to_str().ok()) - .is_some_and(|accept| accept.eq_ignore_ascii_case("application/octet-stream")) - { + .and_then(|accept| match accept.to_lowercase().as_str() { + "text/plain" => Some(true), + "application/octet-stream" => Some(false), + _ => None, + }); + + if let Some(readme) = accept { let object_url = GetObject::new( &app_state.s3_bucket, Some(&app_state.s3_credentials), - &s3_name(&name, &v_id), + &s3_name(&name, &v_id, readme), ) .sign(S3_SIGN_DURATION); - return Ok(HttpResponse::Ok().body( - app_state - .reqwest_client - .get(object_url) - .send() - .await? - .error_for_status()? - .bytes() - .await?, - )); + return Ok(HttpResponse::TemporaryRedirect() + .append_header((LOCATION, object_url.as_str())) + .finish()); } Ok(HttpResponse::Ok().json(PackageResponse { @@ -100,5 +100,7 @@ pub async fn get_package_version( description: entry.description.clone().unwrap_or_default(), published_at: entry.published_at, license: entry.license.clone().unwrap_or_default(), + authors: entry.authors.clone(), + repository: entry.repository.clone().map(|url| url.to_string()), })) } diff --git a/registry/src/endpoints/package_versions.rs b/registry/src/endpoints/package_versions.rs index 9344176..8e635bd 100644 --- a/registry/src/endpoints/package_versions.rs +++ b/registry/src/endpoints/package_versions.rs @@ -36,6 +36,8 @@ pub async fn get_package_versions( description: entry.description.unwrap_or_default(), published_at: entry.published_at, license: entry.license.unwrap_or_default(), + authors: entry.authors.clone(), + repository: entry.repository.clone().map(|url| url.to_string()), }); info.targets.insert(entry.target.into()); diff --git a/registry/src/endpoints/publish_version.rs b/registry/src/endpoints/publish_version.rs index a464166..829ec4e 100644 --- a/registry/src/endpoints/publish_version.rs +++ b/registry/src/endpoints/publish_version.rs @@ -8,19 +8,12 @@ use actix_web::{web, HttpResponse, Responder}; use flate2::read::GzDecoder; use futures::StreamExt; use git2::{Remote, Repository, Signature}; +use reqwest::header::{CONTENT_ENCODING, CONTENT_TYPE}; use rusty_s3::{actions::PutObject, S3Action}; use tar::Archive; -use crate::{ - auth::UserId, - benv, - error::{Error, ErrorResponse}, - package::{s3_name, S3_SIGN_DURATION}, - search::update_version, - AppState, -}; use pesde::{ - manifest::{DependencyType, Manifest}, + manifest::Manifest, source::{ git_index::GitBasedSource, pesde::{IndexFile, IndexFileEntry, ScopeInfo, SCOPE_INFO_FILE}, @@ -31,6 +24,15 @@ use pesde::{ MANIFEST_FILE_NAME, }; +use crate::{ + auth::UserId, + benv, + error::{Error, ErrorResponse}, + package::{s3_name, S3_SIGN_DURATION}, + search::update_version, + AppState, +}; + fn signature<'a>() -> Signature<'a> { Signature::now( &benv!(required "COMMITTER_GIT_NAME"), @@ -80,6 +82,7 @@ pub async fn publish_package( let entries = archive.entries()?; let mut manifest = None::; + let mut readme = None::>; for entry in entries { let mut entry = entry?; @@ -107,6 +110,21 @@ pub async fn publish_package( let mut content = String::new(); entry.read_to_string(&mut content)?; manifest = Some(toml::de::from_str(&content).map_err(|_| Error::InvalidArchive)?); + } else if path.to_lowercase() == "readme" + || path + .to_lowercase() + .split_once('.') + .filter(|(file, ext)| *file == "readme" && (*ext == "md" || *ext == "txt")) + .is_some() + { + if readme.is_some() { + return Err(Error::InvalidArchive); + } + + let mut gz = flate2::read::GzEncoder::new(entry, flate2::Compression::best()); + let mut bytes = vec![]; + gz.read_to_end(&mut bytes)?; + readme = Some(bytes); } } @@ -121,59 +139,52 @@ pub async fn publish_package( let dependencies = manifest .all_dependencies() - .map_err(|_| Error::InvalidArchive)? - .into_iter() - .filter_map(|(alias, (specifier, ty))| { - match &specifier { - DependencySpecifiers::Pesde(specifier) => { - if specifier - .index - .as_ref() - .filter(|index| match index.parse::() { - Ok(_) if config.other_registries_allowed => true, - Ok(url) => url == env!("CARGO_PKG_REPOSITORY").parse().unwrap(), - Err(_) => false, - }) - .is_none() - { - return Some(Err(Error::InvalidArchive)); - } + .map_err(|_| Error::InvalidArchive)?; - let (dep_scope, dep_name) = specifier.name.as_str(); - match source.read_file([dep_scope, dep_name], &app_state.project, None) { - Ok(Some(_)) => {} - Ok(None) => return Some(Err(Error::InvalidArchive)), - Err(e) => return Some(Err(e.into())), - } + for (specifier, _) in dependencies.values() { + match specifier { + DependencySpecifiers::Pesde(specifier) => { + if specifier + .index + .as_ref() + .filter(|index| match index.parse::() { + Ok(_) if config.other_registries_allowed => true, + Ok(url) => url == env!("CARGO_PKG_REPOSITORY").parse().unwrap(), + Err(_) => false, + }) + .is_none() + { + return Err(Error::InvalidArchive); } - DependencySpecifiers::Wally(specifier) => { - if !config.wally_allowed { - return Some(Err(Error::InvalidArchive)); - } - if specifier - .index - .as_ref() - .filter(|index| index.parse::().is_ok()) - .is_none() - { - return Some(Err(Error::InvalidArchive)); - } + let (dep_scope, dep_name) = specifier.name.as_str(); + match source.read_file([dep_scope, dep_name], &app_state.project, None) { + Ok(Some(_)) => {} + Ok(None) => return Err(Error::InvalidArchive), + Err(e) => return Err(e.into()), } - DependencySpecifiers::Git(_) => { - if !config.git_allowed { - return Some(Err(Error::InvalidArchive)); - } - } - }; - - if ty == DependencyType::Dev { - return None; } + DependencySpecifiers::Wally(specifier) => { + if !config.wally_allowed { + return Err(Error::InvalidArchive); + } - Some(Ok((alias, (specifier, ty)))) - }) - .collect::>()?; + if specifier + .index + .as_ref() + .filter(|index| index.parse::().is_ok()) + .is_none() + { + return Err(Error::InvalidArchive); + } + } + DependencySpecifiers::Git(_) => { + if !config.git_allowed { + return Err(Error::InvalidArchive); + } + } + } + } let repo = source.repo_git2(&app_state.project)?; @@ -209,6 +220,8 @@ pub async fn publish_package( published_at: chrono::Utc::now(), description: manifest.description.clone(), license: manifest.license.clone(), + authors: manifest.authors.clone(), + repository: manifest.repository.clone(), dependencies, }; @@ -219,10 +232,12 @@ pub async fn publish_package( if let Some(this_version) = this_version { let other_entry = entries.get(this_version).unwrap(); - // TODO: should different licenses be allowed? // description cannot be different - which one to render in the "Recently published" list? + // the others cannot be different because what to return from the versions endpoint? if other_entry.description != new_entry.description || other_entry.license != new_entry.license + || other_entry.authors != new_entry.authors + || other_entry.repository != new_entry.repository { return Ok(HttpResponse::BadRequest().json(ErrorResponse { error: "same version with different description or license already exists" @@ -297,23 +312,42 @@ pub async fn publish_package( update_version(&app_state, &manifest.name, new_entry); } + let version_id = VersionId::new(manifest.version.clone(), manifest.target.kind()); + let object_url = PutObject::new( &app_state.s3_bucket, Some(&app_state.s3_credentials), - &s3_name( - &manifest.name, - &VersionId::new(manifest.version.clone(), manifest.target.kind()), - ), + &s3_name(&manifest.name, &version_id, false), ) .sign(S3_SIGN_DURATION); app_state .reqwest_client .put(object_url) + .header(CONTENT_TYPE, "application/gzip") + .header(CONTENT_ENCODING, "gzip") .body(bytes) .send() .await?; + if let Some(readme) = readme { + let object_url = PutObject::new( + &app_state.s3_bucket, + Some(&app_state.s3_credentials), + &s3_name(&manifest.name, &version_id, true), + ) + .sign(S3_SIGN_DURATION); + + app_state + .reqwest_client + .put(object_url) + .header(CONTENT_TYPE, "text/plain") + .header(CONTENT_ENCODING, "gzip") + .body(readme) + .send() + .await?; + } + Ok(HttpResponse::Ok().body(format!( "published {}@{} {}", manifest.name, manifest.version, manifest.target diff --git a/registry/src/endpoints/search.rs b/registry/src/endpoints/search.rs index 66b588c..f9946b7 100644 --- a/registry/src/endpoints/search.rs +++ b/registry/src/endpoints/search.rs @@ -103,6 +103,8 @@ pub async fn search_packages( .unwrap() .published_at, license: entry.license.clone().unwrap_or_default(), + authors: entry.authors.clone(), + repository: entry.repository.clone().map(|url| url.to_string()), } }) .collect::>(); diff --git a/registry/src/package.rs b/registry/src/package.rs index f1e6d8b..3d18950 100644 --- a/registry/src/package.rs +++ b/registry/src/package.rs @@ -7,10 +7,15 @@ use pesde::{ use serde::Serialize; use std::{collections::BTreeSet, time::Duration}; -pub const S3_SIGN_DURATION: Duration = Duration::from_secs(60 * 60); +pub const S3_SIGN_DURATION: Duration = Duration::from_secs(60 * 3); -pub fn s3_name(name: &PackageName, version_id: &VersionId) -> String { - format!("{}+{}.tar.gz", name.escaped(), version_id.escaped()) +pub fn s3_name(name: &PackageName, version_id: &VersionId, is_readme: bool) -> String { + format!( + "{}+{}{}", + name.escaped(), + version_id.escaped(), + if is_readme { "+readme.gz" } else { ".tar.gz" } + ) } #[derive(Debug, Serialize, Eq, PartialEq)] @@ -22,11 +27,7 @@ pub struct TargetInfo { impl From for TargetInfo { fn from(target: Target) -> Self { - TargetInfo { - kind: target.kind(), - lib: target.lib_path().is_some(), - bin: target.bin_path().is_some(), - } + (&target).into() } } @@ -62,4 +63,8 @@ pub struct PackageResponse { pub published_at: DateTime, #[serde(skip_serializing_if = "String::is_empty")] pub license: String, + #[serde(skip_serializing_if = "Vec::is_empty")] + pub authors: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + pub repository: Option, } diff --git a/src/cli/commands/publish.rs b/src/cli/commands/publish.rs index 49bc81d..65c5e9f 100644 --- a/src/cli/commands/publish.rs +++ b/src/cli/commands/publish.rs @@ -291,14 +291,19 @@ impl PublishCommand { ); println!( "authors: {}", - manifest - .authors - .as_ref() - .map_or("(none)".to_string(), |a| a.join(", ")) + if manifest.authors.is_empty() { + "(none)".to_string() + } else { + manifest.authors.join(", ") + } ); println!( "repository: {}", - manifest.repository.as_deref().unwrap_or("(none)") + manifest + .repository + .as_ref() + .map(|r| r.as_str()) + .unwrap_or("(none)") ); let roblox_target = roblox_target.is_some_and(|_| true); diff --git a/src/main.rs b/src/main.rs index 1e51a2f..2042a60 100644 --- a/src/main.rs +++ b/src/main.rs @@ -151,9 +151,7 @@ fn run() -> anyhow::Result<()> { if let Some(token) = token { headers.insert( reqwest::header::AUTHORIZATION, - format!("Bearer {token}") - .parse() - .context("failed to create auth header")?, + token.parse().context("failed to create auth header")?, ); } diff --git a/src/manifest/mod.rs b/src/manifest/mod.rs index 36e3294..ffc09b7 100644 --- a/src/manifest/mod.rs +++ b/src/manifest/mod.rs @@ -29,11 +29,11 @@ pub struct Manifest { #[serde(default, skip_serializing_if = "Option::is_none")] pub license: Option, /// The authors of the package - #[serde(default, skip_serializing_if = "Option::is_none")] - pub authors: Option>, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub authors: Vec, /// The repository of the package #[serde(default, skip_serializing_if = "Option::is_none")] - pub repository: Option, + pub repository: Option, /// The target of the package pub target: Target, /// Whether the package is private diff --git a/src/resolver.rs b/src/resolver.rs index e463ea9..2a55b1b 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -280,8 +280,7 @@ impl Project { pkg_ref.dependencies().clone() { if dependency_ty == DependencyType::Dev { - // dev dependencies of dependencies are not included in the graph - // they should not even be stored in the index, so this is just a check to avoid potential issues + // dev dependencies of dependencies are to be ignored continue; } diff --git a/src/source/pesde/mod.rs b/src/source/pesde/mod.rs index 01b8c0c..4f62dbf 100644 --- a/src/source/pesde/mod.rs +++ b/src/source/pesde/mod.rs @@ -402,6 +402,12 @@ pub struct IndexFileEntry { /// The license of this package #[serde(default, skip_serializing_if = "Option::is_none")] pub license: Option, + /// The authors of this package + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub authors: Vec, + /// The repository of this package + #[serde(default, skip_serializing_if = "Option::is_none")] + pub repository: Option, /// The dependencies of this package #[serde(default, skip_serializing_if = "BTreeMap::is_empty")]