From f4fb957a38e00902b1997094e255980a1e07adb6 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Mon, 16 Oct 2023 08:58:53 -0700 Subject: [PATCH] Code cleanup in cargo-registry (#33711) --- cargo-registry/src/client.rs | 22 ++--- cargo-registry/src/main.rs | 110 +++++-------------------- cargo-registry/src/publisher.rs | 6 +- cargo-registry/src/response_builder.rs | 29 ++++++- 4 files changed, 63 insertions(+), 104 deletions(-) diff --git a/cargo-registry/src/client.rs b/cargo-registry/src/client.rs index 7edad1e0aa6599..109b1a8b1975eb 100644 --- a/cargo-registry/src/client.rs +++ b/cargo-registry/src/client.rs @@ -19,9 +19,9 @@ use { std::{error, sync::Arc, time::Duration}, }; -pub struct ClientConfig<'a>(pub ProgramV4CommandConfig<'a>); +pub(crate) struct RPCCommandConfig<'a>(pub ProgramV4CommandConfig<'a>); -impl<'a> ClientConfig<'a> { +impl<'a> RPCCommandConfig<'a> { pub fn new(client: &'a Client) -> Self { Self(ProgramV4CommandConfig { websocket_url: &client.websocket_url, @@ -34,7 +34,7 @@ impl<'a> ClientConfig<'a> { } } -pub struct Client { +pub(crate) struct Client { pub rpc_client: Arc, pub port: u16, pub server_url: String, @@ -161,7 +161,7 @@ impl Client { ) } - pub fn new() -> Result> { + pub(crate) fn new() -> Result> { let matches = Self::get_clap_app( crate_name!(), crate_description!(), @@ -169,7 +169,7 @@ impl Client { ) .get_matches(); - let config = if let Some(config_file) = matches.value_of("config_file") { + let cli_config = if let Some(config_file) = matches.value_of("config_file") { Config::load(config_file).unwrap_or_default() } else { Config::default() @@ -177,19 +177,19 @@ impl Client { let (_, json_rpc_url) = ConfigInput::compute_json_rpc_url_setting( matches.value_of("json_rpc_url").unwrap_or(""), - &config.json_rpc_url, + &cli_config.json_rpc_url, ); let (_, websocket_url) = ConfigInput::compute_websocket_url_setting( matches.value_of("websocket_url").unwrap_or(""), - &config.websocket_url, + &cli_config.websocket_url, matches.value_of("json_rpc_url").unwrap_or(""), - &config.json_rpc_url, + &cli_config.json_rpc_url, ); let (_, commitment) = ConfigInput::compute_commitment_config( matches.value_of("commitment").unwrap_or(""), - &config.commitment, + &cli_config.commitment, ); let rpc_timeout = value_t_or_exit!(matches, "rpc_timeout", u64); @@ -200,8 +200,8 @@ impl Client { let confirm_transaction_initial_timeout = Duration::from_secs(confirm_transaction_initial_timeout); - let payer_keypair = Self::get_keypair(&matches, &config.keypair_path, "keypair")?; - let authority_keypair = Self::get_keypair(&matches, &config.keypair_path, "authority")?; + let payer_keypair = Self::get_keypair(&matches, &cli_config.keypair_path, "keypair")?; + let authority_keypair = Self::get_keypair(&matches, &cli_config.keypair_path, "authority")?; let port = value_t_or_exit!(matches, "port", u16); diff --git a/cargo-registry/src/main.rs b/cargo-registry/src/main.rs index 4ba61c917969b7..419e8cf434202d 100644 --- a/cargo-registry/src/main.rs +++ b/cargo-registry/src/main.rs @@ -79,23 +79,14 @@ impl CargoRegistryService { _request: &hyper::Request, ) -> hyper::Response { let Some((path, _crate_name, _version)) = Self::get_crate_name_and_version(path) else { - return response_builder::error_response( - hyper::StatusCode::BAD_REQUEST, - "Failed to parse the request.", - ); + return response_builder::error_in_parsing(); }; if path.len() != PATH_PREFIX.len() { - return response_builder::error_response( - hyper::StatusCode::BAD_REQUEST, - "Request length is incorrect", - ); + return response_builder::error_incorrect_length(); } - response_builder::error_response( - hyper::StatusCode::NOT_IMPLEMENTED, - "This command is not implemented yet", - ) + response_builder::error_not_implemented() } fn handle_unyank_request( @@ -103,23 +94,14 @@ impl CargoRegistryService { _request: &hyper::Request, ) -> hyper::Response { let Some((path, _crate_name, _version)) = Self::get_crate_name_and_version(path) else { - return response_builder::error_response( - hyper::StatusCode::BAD_REQUEST, - "Failed to parse the request.", - ); + return response_builder::error_in_parsing(); }; if path.len() != PATH_PREFIX.len() { - return response_builder::error_response( - hyper::StatusCode::BAD_REQUEST, - "Request length is incorrect", - ); + return response_builder::error_incorrect_length(); } - response_builder::error_response( - hyper::StatusCode::NOT_IMPLEMENTED, - "This command is not implemented yet", - ) + response_builder::error_not_implemented() } fn get_crate_name(path: &str) -> Option<(&str, &str)> { @@ -131,23 +113,14 @@ impl CargoRegistryService { _request: &hyper::Request, ) -> hyper::Response { let Some((path, _crate_name)) = Self::get_crate_name(path) else { - return response_builder::error_response( - hyper::StatusCode::BAD_REQUEST, - "Failed to parse the request.", - ); + return response_builder::error_in_parsing(); }; if path.len() != PATH_PREFIX.len() { - return response_builder::error_response( - hyper::StatusCode::BAD_REQUEST, - "Request length is incorrect", - ); + return response_builder::error_incorrect_length(); } - response_builder::error_response( - hyper::StatusCode::NOT_IMPLEMENTED, - "This command is not implemented yet", - ) + response_builder::error_not_implemented() } fn handle_add_owners_request( @@ -155,23 +128,14 @@ impl CargoRegistryService { _request: &hyper::Request, ) -> hyper::Response { let Some((path, _crate_name)) = Self::get_crate_name(path) else { - return response_builder::error_response( - hyper::StatusCode::BAD_REQUEST, - "Failed to parse the request.", - ); + return response_builder::error_in_parsing(); }; if path.len() != PATH_PREFIX.len() { - return response_builder::error_response( - hyper::StatusCode::BAD_REQUEST, - "Request length is incorrect", - ); + return response_builder::error_incorrect_length(); } - response_builder::error_response( - hyper::StatusCode::NOT_IMPLEMENTED, - "This command is not implemented yet", - ) + response_builder::error_not_implemented() } fn handle_delete_owners_request( @@ -179,23 +143,14 @@ impl CargoRegistryService { _request: &hyper::Request, ) -> hyper::Response { let Some((path, _crate_name)) = Self::get_crate_name(path) else { - return response_builder::error_response( - hyper::StatusCode::BAD_REQUEST, - "Failed to parse the request.", - ); + return response_builder::error_in_parsing(); }; if path.len() != PATH_PREFIX.len() { - return response_builder::error_response( - hyper::StatusCode::BAD_REQUEST, - "Request length is incorrect", - ); + return response_builder::error_incorrect_length(); } - response_builder::error_response( - hyper::StatusCode::NOT_IMPLEMENTED, - "This command is not implemented yet", - ) + response_builder::error_not_implemented() } fn handle_get_crates_request( @@ -208,16 +163,10 @@ impl CargoRegistryService { // full path started with PATH_PREFIX. So it's sufficient to check that provided // path is smaller than PATH_PREFIX. if path.len() >= PATH_PREFIX.len() { - return response_builder::error_response( - hyper::StatusCode::BAD_REQUEST, - "Request length is incorrect", - ); + return response_builder::error_incorrect_length(); } - response_builder::error_response( - hyper::StatusCode::NOT_IMPLEMENTED, - "This command is not implemented yet", - ) + response_builder::error_not_implemented() } async fn handler( @@ -255,41 +204,26 @@ impl CargoRegistryService { Method::PUT => match endpoint { "new" => { if path.len() != PATH_PREFIX.len() { - response_builder::error_response( - hyper::StatusCode::BAD_REQUEST, - "Invalid length of the request.", - ) + response_builder::error_incorrect_length() } else { Self::handle_publish_request(request, client.clone(), index.clone()).await } } "unyank" => Self::handle_unyank_request(path, &request), "owners" => Self::handle_add_owners_request(path, &request), - _ => response_builder::error_response( - hyper::StatusCode::METHOD_NOT_ALLOWED, - "Unknown request", - ), + _ => response_builder::error_not_allowed(), }, Method::GET => match endpoint { "crates" => Self::handle_get_crates_request(path, &request), "owners" => Self::handle_get_owners_request(path, &request), - _ => response_builder::error_response( - hyper::StatusCode::METHOD_NOT_ALLOWED, - "Unknown request", - ), + _ => response_builder::error_not_allowed(), }, Method::DELETE => match endpoint { "yank" => Self::handle_yank_request(path, &request), "owners" => Self::handle_delete_owners_request(path, &request), - _ => response_builder::error_response( - hyper::StatusCode::METHOD_NOT_ALLOWED, - "Unknown request", - ), + _ => response_builder::error_not_allowed(), }, - _ => response_builder::error_response( - hyper::StatusCode::METHOD_NOT_ALLOWED, - "Unknown request", - ), + _ => response_builder::error_not_allowed(), }) } } diff --git a/cargo-registry/src/publisher.rs b/cargo-registry/src/publisher.rs index 5940191b46dcaf..ea4c74a7251b67 100644 --- a/cargo-registry/src/publisher.rs +++ b/cargo-registry/src/publisher.rs @@ -1,6 +1,6 @@ use { crate::{ - client::{Client, ClientConfig}, + client::{Client, RPCCommandConfig}, sparse_index::{IndexEntry, RegistryIndex}, }, flate2::read::GzDecoder, @@ -129,7 +129,7 @@ impl Publisher { let tempdir = tempdir()?; archive.unpack(tempdir.path())?; - let config = ClientConfig::new(client.as_ref()); + let command_config = RPCCommandConfig::new(client.as_ref()); let lib_name = Self::program_library_name(&tempdir, &meta_data)?; @@ -152,7 +152,7 @@ impl Publisher { process_deploy_program( client.rpc_client.clone(), - &config.0, + &command_config.0, &program_data, program_data.len() as u32, &program_keypair.pubkey(), diff --git a/cargo-registry/src/response_builder.rs b/cargo-registry/src/response_builder.rs index 8a56e298f713ae..a8da2d9d6fdbba 100644 --- a/cargo-registry/src/response_builder.rs +++ b/cargo-registry/src/response_builder.rs @@ -1,4 +1,4 @@ -use {crate::response_builder, log::error}; +use log::error; pub(crate) fn error_response(status: hyper::StatusCode, msg: &str) -> hyper::Response { error!("{}", msg); @@ -23,5 +23,30 @@ pub(crate) fn success_response_str(value: &str) -> hyper::Response } pub(crate) fn success_response() -> hyper::Response { - response_builder::success_response_str("") + success_response_str("") +} + +pub(crate) fn error_not_allowed() -> hyper::Response { + error_response(hyper::StatusCode::METHOD_NOT_ALLOWED, "Unknown request") +} + +pub(crate) fn error_not_implemented() -> hyper::Response { + error_response( + hyper::StatusCode::NOT_IMPLEMENTED, + "This command is not implemented yet", + ) +} + +pub(crate) fn error_in_parsing() -> hyper::Response { + error_response( + hyper::StatusCode::BAD_REQUEST, + "Failed to parse the request", + ) +} + +pub(crate) fn error_incorrect_length() -> hyper::Response { + error_response( + hyper::StatusCode::BAD_REQUEST, + "Request length is incorrect", + ) }