From c8a7ec1461bdbb82e262aadb8c3f2de745e8de3a Mon Sep 17 00:00:00 2001 From: Jordan Doyle Date: Thu, 09 Sep 2021 23:53:43 +0100 Subject: [PATCH] Remove unnecessary macros --- chartered-db/src/crates.rs | 67 +++++++++++++++++++++++++++++++++++-------------------------------- chartered-db/src/users.rs | 12 +++++------- chartered-web/src/main.rs | 2 +- chartered-web/src/endpoints/mod.rs | 38 +++++++++++++------------------------- chartered-web/src/endpoints/cargo_api/download.rs | 38 +++++++++++++++++++++++++++++++++----- chartered-web/src/endpoints/cargo_api/mod.rs | 24 ++++++++---------------- chartered-web/src/endpoints/cargo_api/owners.rs | 35 ++++++++++++++++++++++++++++++++--- chartered-web/src/endpoints/cargo_api/publish.rs | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------ 8 files changed, 177 insertions(+), 127 deletions(-) diff --git a/chartered-db/src/crates.rs b/chartered-db/src/crates.rs index 666f97f..9f8b368 100644 --- a/chartered-db/src/crates.rs +++ a/chartered-db/src/crates.rs @@ -1,10 +1,8 @@ use super::{ schema::{crate_versions, crates}, BitwiseExpressionMethods, ConnectionPool, Result, }; -use diesel::{ - insert_into, insert_or_ignore_into, prelude::*, Associations, Identifiable, Queryable, -}; +use diesel::{insert_into, prelude::*, Associations, Identifiable, Queryable}; use itertools::Itertools; use std::{collections::HashMap, sync::Arc}; @@ -91,6 +89,34 @@ }) .await? } + + pub async fn publish_version( + self: Arc, + conn: ConnectionPool, + version_string: String, + file_identifier: chartered_fs::FileReference, + file_checksum: String, + ) -> Result<()> { + use crate::schema::crate_versions::dsl::{ + checksum, crate_id, crate_versions, filesystem_object, version, + }; + + tokio::task::spawn_blocking(move || { + let conn = conn.get()?; + + insert_into(crate_versions) + .values(( + crate_id.eq(self.id), + version.eq(version_string), + filesystem_object.eq(file_identifier.to_string()), + checksum.eq(file_checksum), + )) + .execute(&conn)?; + + Ok(()) + }) + .await? + } } #[derive(Identifiable, Queryable, Associations, PartialEq, Debug)] @@ -102,39 +128,4 @@ pub filesystem_object: String, pub yanked: bool, pub checksum: String, -} - -pub async fn publish_crate( - conn: ConnectionPool, - crate_name: String, - version_string: String, - file_identifier: chartered_fs::FileReference, - file_checksum: String, -) -> Result<()> { - use crate::schema::{ - crate_versions::dsl::{checksum, crate_id, crate_versions, filesystem_object, version}, - crates::dsl::{crates, name}, - }; - - tokio::task::spawn_blocking(move || { - let conn = conn.get()?; - - insert_or_ignore_into(crates) - .values(name.eq(&crate_name)) - .execute(&conn)?; - - let selected_crate = crates.filter(name.eq(crate_name)).first::(&conn)?; - - insert_into(crate_versions) - .values(( - crate_id.eq(selected_crate.id), - version.eq(version_string), - filesystem_object.eq(file_identifier.to_string()), - checksum.eq(file_checksum), - )) - .execute(&conn)?; - - Ok(()) - }) - .await? } diff --git a/chartered-db/src/users.rs b/chartered-db/src/users.rs index a0c25dd..cb72a04 100644 --- a/chartered-db/src/users.rs +++ a/chartered-db/src/users.rs @@ -69,17 +69,15 @@ .await? } - pub async fn has_crate_permission( + pub async fn get_crate_permissions( self: Arc, conn: ConnectionPool, crate_id: i32, - requested_permissions: UserCratePermissionValue, - ) -> Result { - let perms = UserCratePermission::find(conn, self.id, crate_id) + ) -> Result { + Ok(UserCratePermission::find(conn, self.id, crate_id) .await? - .unwrap_or_default(); - - Ok(perms.permissions.contains(requested_permissions)) + .unwrap_or_default() + .permissions) } } diff --git a/chartered-web/src/main.rs b/chartered-web/src/main.rs index de33976..caad92c 100644 --- a/chartered-web/src/main.rs +++ a/chartered-web/src/main.rs @@ -49,7 +49,7 @@ .route("/crates/:crate/:version/unyank", put(hello_world)) .route( "/crates/:crate/:version/download", - get(endpoints::cargo_api::download), + get(endpoints::cargo_api::download) )) .layer( ServiceBuilder::new() diff --git a/chartered-web/src/endpoints/mod.rs b/chartered-web/src/endpoints/mod.rs index 700aed2..0a7a829 100644 --- a/chartered-web/src/endpoints/mod.rs +++ a/chartered-web/src/endpoints/mod.rs @@ -1,40 +1,26 @@ #[derive(serde::Serialize)] pub struct ErrorResponse { - error: &'static str, + error: String, } -macro_rules! define_error { - ($($kind:ident$(($inner_name:ident: $inner:ty))? => $status:ident / $public_text:expr,)*) => { - #[derive(thiserror::Error, Debug)] - pub enum Error { - $($kind$((#[from] $inner))?),* - } - - /// a (web-safe) explanation of the error, this shouldn't reveal internal details that - /// may be sensitive - impl std::fmt::Display for Error { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - $(Self::$kind$(($inner_name))? => f.write_str($public_text)),* - } - } - } - +macro_rules! define_error_response { + ($error:ty) => { impl axum::response::IntoResponse for Error { type Body = axum::body::Full; type BodyError = ::Error; fn into_response(self) -> axum::http::Response { - let (status, body) = match self { - $(Self::$kind$(($inner_name))? => ( - axum::http::StatusCode::$status, - serde_json::to_vec(&crate::endpoints::ErrorResponse { error: $public_text }).unwrap(), - )),* - }; + let body = serde_json::to_vec(&crate::endpoints::ErrorResponse { + error: self.to_string(), + }) + .unwrap(); let mut res = axum::http::Response::new(axum::body::Full::from(body)); - *res.status_mut() = status; - res.headers_mut().insert(axum::http::header::CONTENT_TYPE, axum::http::header::HeaderValue::from_static("application/json")); + *res.status_mut() = self.status_code(); + res.headers_mut().insert( + axum::http::header::CONTENT_TYPE, + axum::http::header::HeaderValue::from_static("application/json"), + ); res } } diff --git a/chartered-web/src/endpoints/cargo_api/download.rs b/chartered-web/src/endpoints/cargo_api/download.rs index 99e1b74..ff81cf9 100644 --- a/chartered-web/src/endpoints/cargo_api/download.rs +++ a/chartered-web/src/endpoints/cargo_api/download.rs @@ -6,21 +6,43 @@ }; use chartered_fs::FileSystem; use std::{str::FromStr, sync::Arc}; +use thiserror::Error; -define_error!( - Database(_e: chartered_db::Error) => INTERNAL_SERVER_ERROR / "Failed to query database", - File(_e: std::io::Error) => INTERNAL_SERVER_ERROR / "Failed to fetch crate file", - NoVersion => NOT_FOUND / "The requested version does not exist for the crate", - NoCrate => NOT_FOUND / "The requested crate does not exist", -); +#[derive(Error, Debug)] +pub enum Error { + #[error("Failed to query database")] + Database(#[from] chartered_db::Error), + #[error("Failed to fetch crate file")] + File(#[from] std::io::Error), + #[error("The requested crate does not exist")] + NoCrate, + #[error("The requested version does not exist for the crate")] + NoVersion, +} +impl Error { + pub fn status_code(&self) -> axum::http::StatusCode { + use axum::http::StatusCode; + + match self { + Self::Database(_) | Self::File(_) => StatusCode::INTERNAL_SERVER_ERROR, + Self::NoCrate | Self::NoVersion => StatusCode::NOT_FOUND, + } + } +} + +define_error_response!(Error); + pub async fn handle( extract::Path((_api_key, name, version)): extract::Path<(String, String, String)>, extract::Extension(db): extract::Extension, extract::Extension(user): extract::Extension>, ) -> Result, Error> { - let crate_ = get_crate!(db, name; || -> Error::NoCrate); - ensure_has_crate_perm!(db, user, crate_, Permission::VISIBLE; || -> Error::NoCrate); + let crate_ = Crate::find_by_name(db.clone(), name) + .await? + .ok_or(Error::NoCrate) + .map(std::sync::Arc::new)?; + ensure_has_crate_perm!(db, user, crate_, Permission::VISIBLE | -> Error::NoCrate); let version = crate_.version(db, version).await?.ok_or(Error::NoVersion)?; diff --git a/chartered-web/src/endpoints/cargo_api/mod.rs b/chartered-web/src/endpoints/cargo_api/mod.rs index ae07c91..d903363 100644 --- a/chartered-web/src/endpoints/cargo_api/mod.rs +++ a/chartered-web/src/endpoints/cargo_api/mod.rs @@ -1,20 +1,12 @@ -macro_rules! get_crate { - ($db:expr, $name:expr; || -> $error:expr) => { - Crate::find_by_name($db.clone(), $name) - .await? - .ok_or($error) - .map(std::sync::Arc::new)? - }; -} - macro_rules! ensure_has_crate_perm { - ($db:expr, $user:expr, $crate_expr:expr, $permissions:expr; || -> $error:expr) => {{ - if !$user - .has_crate_permission($db.clone(), $crate_expr.id, $permissions) - .await? - { - return Err($error); - } + ($db:expr, $user:expr, $crate_expr:expr, $($permission:path | -> $error:expr$(,)?),*) => {{ + let perms = $user.get_crate_permissions($db.clone(), $crate_expr.id).await?; + + $( + if !perms.contains($permission) { + return Err($error); + } + )* }}; } diff --git a/chartered-web/src/endpoints/cargo_api/owners.rs b/chartered-web/src/endpoints/cargo_api/owners.rs index 5f1191f..9edc885 100644 --- a/chartered-web/src/endpoints/cargo_api/owners.rs +++ a/chartered-web/src/endpoints/cargo_api/owners.rs @@ -6,11 +6,28 @@ }; use serde::Serialize; use std::sync::Arc; +use thiserror::Error; -define_error!( - Database(_e: chartered_db::Error) => INTERNAL_SERVER_ERROR / "Failed to query database", - NoCrate => NOT_FOUND / "The requested crate does not exist", -); +#[derive(Error, Debug)] +pub enum Error { + #[error("Failed to query database")] + Database(#[from] chartered_db::Error), + #[error("The requested crate does not exist")] + NoCrate, +} + +impl Error { + pub fn status_code(&self) -> axum::http::StatusCode { + use axum::http::StatusCode; + + match self { + Self::Database(_) => StatusCode::INTERNAL_SERVER_ERROR, + Self::NoCrate => StatusCode::NOT_FOUND, + } + } +} + +define_error_response!(Error); #[derive(Serialize)] pub struct GetResponse { @@ -29,8 +46,14 @@ extract::Extension(db): extract::Extension, extract::Extension(user): extract::Extension>, ) -> Result, Error> { - let crate_ = get_crate!(db, name; || -> Error::NoCrate); - ensure_has_crate_perm!(db, user, crate_, Permission::VISIBLE; || -> Error::NoCrate); + let crate_ = Crate::find_by_name(db.clone(), name) + .await? + .ok_or(Error::NoCrate) + .map(std::sync::Arc::new)?; + ensure_has_crate_perm!( + db, user, crate_, + Permission::VISIBLE | -> Error::NoCrate + ); let users = crate_ .owners(db) diff --git a/chartered-web/src/endpoints/cargo_api/publish.rs b/chartered-web/src/endpoints/cargo_api/publish.rs index 8c3f840..b58ec33 100644 --- a/chartered-web/src/endpoints/cargo_api/publish.rs +++ a/chartered-web/src/endpoints/cargo_api/publish.rs @@ -1,9 +1,45 @@ use axum::extract; use bytes::Bytes; -use chartered_db::ConnectionPool; +use chartered_db::{ + crates::Crate, + users::{User, UserCratePermissionValue as Permission}, + ConnectionPool, +}; +use chartered_fs::FileSystem; use serde::{Deserialize, Serialize}; -use std::convert::TryInto; +use sha2::{Digest, Sha256}; +use std::{convert::TryInto, sync::Arc}; +use thiserror::Error; + +#[derive(Error, Debug)] +pub enum Error { + #[error("Failed to query database")] + Database(#[from] chartered_db::Error), + #[error("The requested crate does not exist")] + NoCrate, + #[error("You don't have permission to publish versions for this crate")] + NoPermission, + #[error("Invalid JSON from client")] + JsonParse(#[from] serde_json::Error), + #[error("Invalid body")] + MetadataParse, +} + +impl Error { + pub fn status_code(&self) -> axum::http::StatusCode { + use axum::http::StatusCode; + + match self { + Self::Database(_) => StatusCode::INTERNAL_SERVER_ERROR, + Self::NoCrate => StatusCode::NOT_FOUND, + Self::NoPermission => StatusCode::FORBIDDEN, + Self::JsonParse(_) | Self::MetadataParse => StatusCode::BAD_REQUEST, + } + } +} +define_error_response!(Error); + #[derive(Serialize, Debug, Default)] pub struct PublishCrateResponse { warnings: PublishCrateResponseWarnings, @@ -18,33 +54,35 @@ pub async fn handle( extract::Extension(db): extract::Extension, + extract::Extension(user): extract::Extension>, body: Bytes, -) -> axum::response::Json { - use chartered_fs::FileSystem; - use sha2::{Digest, Sha256}; - - let (_, (metadata_bytes, crate_bytes)) = parse(body.as_ref()).unwrap(); +) -> Result, Error> { + let (_, (metadata_bytes, crate_bytes)) = + parse(body.as_ref()).map_err(|_| Error::MetadataParse)?; + let metadata: Metadata = serde_json::from_slice(metadata_bytes)?; + + let crate_ = Crate::find_by_name(db.clone(), metadata.name.to_string()) + .await? + .ok_or(Error::NoCrate) + .map(std::sync::Arc::new)?; + ensure_has_crate_perm!( + db, user, crate_, + Permission::VISIBLE | -> Error::NoCrate, + Permission::PUBLISH_VERSION | -> Error::NoPermission, + ); - let metadata: Metadata = serde_json::from_slice(metadata_bytes).unwrap(); - let file_ref = chartered_fs::Local.write(crate_bytes).await.unwrap(); - - let mut response = PublishCrateResponse::default(); - - if let Err(e) = chartered_db::crates::publish_crate( - db, - metadata.name.to_string(), - metadata.vers.to_string(), - file_ref, - hex::encode(Sha256::digest(crate_bytes)), - ) - .await - { - // todo: should this be a normal http error? - response.warnings.other.push(e.to_string()); - } - axum::response::Json(response) + crate_ + .publish_version( + db, + metadata.vers.to_string(), + file_ref, + hex::encode(Sha256::digest(crate_bytes)), + ) + .await?; + + Ok(axum::response::Json(PublishCrateResponse::default())) } fn parse(body: &[u8]) -> nom::IResult<&[u8], (&[u8], &[u8])> { -- rgit 0.1.3