From 6644df6b969df3f7ff0516ab7acd9b600dff0a54 Mon Sep 17 00:00:00 2001 From: Trinity Pointard Date: Fri, 23 Apr 2021 22:41:24 +0200 Subject: [PATCH] fix clippy warnings on garage --- src/garage/admin_rpc.rs | 134 +++++++++++++++++++--------------------- src/garage/cli.rs | 32 +++++----- src/garage/server.rs | 8 +-- 3 files changed, 84 insertions(+), 90 deletions(-) diff --git a/src/garage/admin_rpc.rs b/src/garage/admin_rpc.rs index d04dd7a1..dc27caae 100644 --- a/src/garage/admin_rpc.rs +++ b/src/garage/admin_rpc.rs @@ -25,7 +25,7 @@ pub const ADMIN_RPC_TIMEOUT: Duration = Duration::from_secs(30); pub const ADMIN_RPC_PATH: &str = "_admin"; #[derive(Debug, Serialize, Deserialize)] -pub enum AdminRPC { +pub enum AdminRpc { BucketOperation(BucketOperation), KeyOperation(KeyOperation), LaunchRepair(RepairOpt), @@ -39,35 +39,35 @@ pub enum AdminRPC { KeyInfo(Key), } -impl RpcMessage for AdminRPC {} +impl RpcMessage for AdminRpc {} pub struct AdminRpcHandler { garage: Arc, - rpc_client: Arc>, + rpc_client: Arc>, } impl AdminRpcHandler { pub fn new(garage: Arc) -> Arc { - let rpc_client = garage.system.clone().rpc_client::(ADMIN_RPC_PATH); + let rpc_client = garage.system.clone().rpc_client::(ADMIN_RPC_PATH); Arc::new(Self { garage, rpc_client }) } pub fn register_handler(self: Arc, rpc_server: &mut RpcServer) { - rpc_server.add_handler::(ADMIN_RPC_PATH.to_string(), move |msg, _addr| { + rpc_server.add_handler::(ADMIN_RPC_PATH.to_string(), move |msg, _addr| { let self2 = self.clone(); async move { match msg { - AdminRPC::BucketOperation(bo) => self2.handle_bucket_cmd(bo).await, - AdminRPC::KeyOperation(ko) => self2.handle_key_cmd(ko).await, - AdminRPC::LaunchRepair(opt) => self2.handle_launch_repair(opt).await, - AdminRPC::Stats(opt) => self2.handle_stats(opt).await, - _ => Err(Error::BadRPC(format!("Invalid RPC"))), + AdminRpc::BucketOperation(bo) => self2.handle_bucket_cmd(bo).await, + AdminRpc::KeyOperation(ko) => self2.handle_key_cmd(ko).await, + AdminRpc::LaunchRepair(opt) => self2.handle_launch_repair(opt).await, + AdminRpc::Stats(opt) => self2.handle_stats(opt).await, + _ => Err(Error::BadRPC("Invalid RPC".to_string())), } } }); } - async fn handle_bucket_cmd(&self, cmd: BucketOperation) -> Result { + async fn handle_bucket_cmd(&self, cmd: BucketOperation) -> Result { match cmd { BucketOperation::List => { let bucket_names = self @@ -78,11 +78,11 @@ impl AdminRpcHandler { .iter() .map(|b| b.name.to_string()) .collect::>(); - Ok(AdminRPC::BucketList(bucket_names)) + Ok(AdminRpc::BucketList(bucket_names)) } BucketOperation::Info(query) => { let bucket = self.get_existing_bucket(&query.name).await?; - Ok(AdminRPC::BucketInfo(bucket)) + Ok(AdminRpc::BucketInfo(bucket)) } BucketOperation::Create(query) => { let bucket = match self.garage.bucket_table.get(&EmptyKey, &query.name).await? { @@ -101,7 +101,7 @@ impl AdminRpcHandler { None => Bucket::new(query.name.clone()), }; self.garage.bucket_table.insert(&bucket).await?; - Ok(AdminRPC::Ok(format!("Bucket {} was created.", query.name))) + Ok(AdminRpc::Ok(format!("Bucket {} was created.", query.name))) } BucketOperation::Delete(query) => { let mut bucket = self.get_existing_bucket(&query.name).await?; @@ -114,9 +114,9 @@ impl AdminRpcHandler { return Err(Error::BadRPC(format!("Bucket {} is not empty", query.name))); } if !query.yes { - return Err(Error::BadRPC(format!( - "Add --yes flag to really perform this operation" - ))); + return Err(Error::BadRPC( + "Add --yes flag to really perform this operation".to_string(), + )); } // --- done checking, now commit --- for (key_id, _, _) in bucket.authorized_keys() { @@ -131,7 +131,7 @@ impl AdminRpcHandler { } bucket.state.update(BucketState::Deleted); self.garage.bucket_table.insert(&bucket).await?; - Ok(AdminRPC::Ok(format!("Bucket {} was deleted.", query.name))) + Ok(AdminRpc::Ok(format!("Bucket {} was deleted.", query.name))) } BucketOperation::Allow(query) => { let key = self.get_existing_key(&query.key_pattern).await?; @@ -142,7 +142,7 @@ impl AdminRpcHandler { .await?; self.update_bucket_key(bucket, &key.key_id, allow_read, allow_write) .await?; - Ok(AdminRPC::Ok(format!( + Ok(AdminRpc::Ok(format!( "New permissions for {} on {}: read {}, write {}.", &key.key_id, &query.bucket, allow_read, allow_write ))) @@ -156,7 +156,7 @@ impl AdminRpcHandler { .await?; self.update_bucket_key(bucket, &key.key_id, allow_read, allow_write) .await?; - Ok(AdminRPC::Ok(format!( + Ok(AdminRpc::Ok(format!( "New permissions for {} on {}: read {}, write {}.", &key.key_id, &query.bucket, allow_read, allow_write ))) @@ -165,9 +165,9 @@ impl AdminRpcHandler { let mut bucket = self.get_existing_bucket(&query.bucket).await?; if !(query.allow ^ query.deny) { - return Err(Error::Message(format!( - "You must specify exactly one flag, either --allow or --deny" - ))); + return Err(Error::Message( + "You must specify exactly one flag, either --allow or --deny".to_string(), + )); } if let BucketState::Present(state) = bucket.state.get_mut() { @@ -179,7 +179,7 @@ impl AdminRpcHandler { format!("Website access denied for {}", &query.bucket) }; - Ok(AdminRPC::Ok(msg.to_string())) + Ok(AdminRpc::Ok(msg)) } else { unreachable!(); } @@ -187,7 +187,7 @@ impl AdminRpcHandler { } } - async fn handle_key_cmd(&self, cmd: KeyOperation) -> Result { + async fn handle_key_cmd(&self, cmd: KeyOperation) -> Result { match cmd { KeyOperation::List => { let key_ids = self @@ -203,29 +203,29 @@ impl AdminRpcHandler { .iter() .map(|k| (k.key_id.to_string(), k.name.get().clone())) .collect::>(); - Ok(AdminRPC::KeyList(key_ids)) + Ok(AdminRpc::KeyList(key_ids)) } KeyOperation::Info(query) => { let key = self.get_existing_key(&query.key_pattern).await?; - Ok(AdminRPC::KeyInfo(key)) + Ok(AdminRpc::KeyInfo(key)) } KeyOperation::New(query) => { let key = Key::new(query.name); self.garage.key_table.insert(&key).await?; - Ok(AdminRPC::KeyInfo(key)) + Ok(AdminRpc::KeyInfo(key)) } KeyOperation::Rename(query) => { let mut key = self.get_existing_key(&query.key_pattern).await?; key.name.update(query.new_name); self.garage.key_table.insert(&key).await?; - Ok(AdminRPC::KeyInfo(key)) + Ok(AdminRpc::KeyInfo(key)) } KeyOperation::Delete(query) => { let key = self.get_existing_key(&query.key_pattern).await?; if !query.yes { - return Err(Error::BadRPC(format!( - "Add --yes flag to really perform this operation" - ))); + return Err(Error::BadRPC( + "Add --yes flag to really perform this operation".to_string(), + )); } // --- done checking, now commit --- for (ab_name, _, _) in key.authorized_buckets.items().iter() { @@ -240,7 +240,7 @@ impl AdminRpcHandler { } let del_key = Key::delete(key.key_id.to_string()); self.garage.key_table.insert(&del_key).await?; - Ok(AdminRPC::Ok(format!( + Ok(AdminRpc::Ok(format!( "Key {} was deleted successfully.", key.key_id ))) @@ -252,11 +252,12 @@ impl AdminRpcHandler { } let imported_key = Key::import(&query.key_id, &query.secret_key, &query.name); self.garage.key_table.insert(&imported_key).await?; - Ok(AdminRPC::KeyInfo(imported_key)) + Ok(AdminRpc::KeyInfo(imported_key)) } } } + #[allow(clippy::ptr_arg)] async fn get_existing_bucket(&self, bucket: &String) -> Result { self.garage .bucket_table @@ -264,10 +265,7 @@ impl AdminRpcHandler { .await? .filter(|b| !b.is_deleted()) .map(Ok) - .unwrap_or(Err(Error::BadRPC(format!( - "Bucket {} does not exist", - bucket - )))) + .unwrap_or_else(|| Err(Error::BadRPC(format!("Bucket {} does not exist", bucket)))) } async fn get_existing_key(&self, pattern: &str) -> Result { @@ -298,7 +296,7 @@ impl AdminRpcHandler { async fn update_bucket_key( &self, mut bucket: Bucket, - key_id: &String, + key_id: &str, allow_read: bool, allow_write: bool, ) -> Result<(), Error> { @@ -313,9 +311,9 @@ impl AdminRpcHandler { }, )); } else { - return Err(Error::Message(format!( - "Bucket is deleted in update_bucket_key" - ))); + return Err(Error::Message( + "Bucket is deleted in update_bucket_key".to_string(), + )); } self.garage.bucket_table.insert(&bucket).await?; Ok(()) @@ -325,14 +323,14 @@ impl AdminRpcHandler { async fn update_key_bucket( &self, key: &Key, - bucket: &String, + bucket: &str, allow_read: bool, allow_write: bool, ) -> Result<(), Error> { let mut key = key.clone(); let old_map = key.authorized_buckets.take_and_clear(); key.authorized_buckets.merge(&old_map.update_mutator( - bucket.clone(), + bucket.to_string(), PermissionSet { allow_read, allow_write, @@ -342,11 +340,11 @@ impl AdminRpcHandler { Ok(()) } - async fn handle_launch_repair(self: &Arc, opt: RepairOpt) -> Result { + async fn handle_launch_repair(self: &Arc, opt: RepairOpt) -> Result { if !opt.yes { - return Err(Error::BadRPC(format!( - "Please provide the --yes flag to initiate repair operations." - ))); + return Err(Error::BadRPC( + "Please provide the --yes flag to initiate repair operations.".to_string(), + )); } if opt.all_nodes { let mut opt_to_send = opt.clone(); @@ -359,17 +357,17 @@ impl AdminRpcHandler { .rpc_client .call( *node, - AdminRPC::LaunchRepair(opt_to_send.clone()), + AdminRpc::LaunchRepair(opt_to_send.clone()), ADMIN_RPC_TIMEOUT, ) .await .is_err() { - failures.push(node.clone()); + failures.push(*node); } } if failures.is_empty() { - Ok(AdminRPC::Ok(format!("Repair launched on all nodes"))) + Ok(AdminRpc::Ok("Repair launched on all nodes".to_string())) } else { Err(Error::Message(format!( "Could not launch repair on nodes: {:?} (launched successfully on other nodes)", @@ -386,14 +384,14 @@ impl AdminRpcHandler { .spawn_worker("Repair worker".into(), move |must_exit| async move { repair.repair_worker(opt, must_exit).await }); - Ok(AdminRPC::Ok(format!( + Ok(AdminRpc::Ok(format!( "Repair launched on {:?}", self.garage.system.id ))) } } - async fn handle_stats(&self, opt: StatsOpt) -> Result { + async fn handle_stats(&self, opt: StatsOpt) -> Result { if opt.all_nodes { let mut ret = String::new(); let ring = self.garage.system.ring.borrow().clone(); @@ -406,21 +404,21 @@ impl AdminRpcHandler { writeln!(&mut ret, "Stats for node {:?}:", node).unwrap(); match self .rpc_client - .call(*node, AdminRPC::Stats(opt), ADMIN_RPC_TIMEOUT) + .call(*node, AdminRpc::Stats(opt), ADMIN_RPC_TIMEOUT) .await { - Ok(AdminRPC::Ok(s)) => writeln!(&mut ret, "{}", s).unwrap(), + Ok(AdminRpc::Ok(s)) => writeln!(&mut ret, "{}", s).unwrap(), Ok(x) => writeln!(&mut ret, "Bad answer: {:?}", x).unwrap(), Err(e) => writeln!(&mut ret, "Error: {}", e).unwrap(), } } - Ok(AdminRPC::Ok(ret)) + Ok(AdminRpc::Ok(ret)) } else { - Ok(AdminRPC::Ok(self.gather_stats_local(opt)?)) + Ok(AdminRpc::Ok(self.gather_stats_local(opt))) } } - fn gather_stats_local(&self, opt: StatsOpt) -> Result { + fn gather_stats_local(&self, opt: StatsOpt) -> String { let mut ret = String::new(); writeln!( &mut ret, @@ -445,11 +443,11 @@ impl AdminRpcHandler { writeln!(&mut ret, " {:?} {}", n, c).unwrap(); } - self.gather_table_stats(&mut ret, &self.garage.bucket_table, &opt)?; - self.gather_table_stats(&mut ret, &self.garage.key_table, &opt)?; - self.gather_table_stats(&mut ret, &self.garage.object_table, &opt)?; - self.gather_table_stats(&mut ret, &self.garage.version_table, &opt)?; - self.gather_table_stats(&mut ret, &self.garage.block_ref_table, &opt)?; + self.gather_table_stats(&mut ret, &self.garage.bucket_table, &opt); + self.gather_table_stats(&mut ret, &self.garage.key_table, &opt); + self.gather_table_stats(&mut ret, &self.garage.object_table, &opt); + self.gather_table_stats(&mut ret, &self.garage.version_table, &opt); + self.gather_table_stats(&mut ret, &self.garage.block_ref_table, &opt); writeln!(&mut ret, "\nBlock manager stats:").unwrap(); if opt.detailed { @@ -467,15 +465,10 @@ impl AdminRpcHandler { ) .unwrap(); - Ok(ret) + ret } - fn gather_table_stats( - &self, - to: &mut String, - t: &Arc>, - opt: &StatsOpt, - ) -> Result<(), Error> + fn gather_table_stats(&self, to: &mut String, t: &Arc>, opt: &StatsOpt) where F: TableSchema + 'static, R: TableReplication + 'static, @@ -497,6 +490,5 @@ impl AdminRpcHandler { ) .unwrap(); writeln!(to, " GC todo queue length: {}", t.data.gc_todo_len()).unwrap(); - Ok(()) } } diff --git a/src/garage/cli.rs b/src/garage/cli.rs index 55cd222b..d281570b 100644 --- a/src/garage/cli.rs +++ b/src/garage/cli.rs @@ -294,7 +294,7 @@ pub struct StatsOpt { pub async fn cli_cmd( cmd: Command, membership_rpc_cli: RpcAddrClient, - admin_rpc_cli: RpcAddrClient, + admin_rpc_cli: RpcAddrClient, rpc_host: SocketAddr, ) -> Result<(), Error> { match cmd { @@ -306,11 +306,11 @@ pub async fn cli_cmd( cmd_remove(membership_rpc_cli, rpc_host, remove_opt).await } Command::Bucket(bo) => { - cmd_admin(admin_rpc_cli, rpc_host, AdminRPC::BucketOperation(bo)).await + cmd_admin(admin_rpc_cli, rpc_host, AdminRpc::BucketOperation(bo)).await } - Command::Key(ko) => cmd_admin(admin_rpc_cli, rpc_host, AdminRPC::KeyOperation(ko)).await, - Command::Repair(ro) => cmd_admin(admin_rpc_cli, rpc_host, AdminRPC::LaunchRepair(ro)).await, - Command::Stats(so) => cmd_admin(admin_rpc_cli, rpc_host, AdminRPC::Stats(so)).await, + Command::Key(ko) => cmd_admin(admin_rpc_cli, rpc_host, AdminRpc::KeyOperation(ko)).await, + Command::Repair(ro) => cmd_admin(admin_rpc_cli, rpc_host, AdminRpc::LaunchRepair(ro)).await, + Command::Stats(so) => cmd_admin(admin_rpc_cli, rpc_host, AdminRpc::Stats(so)).await, _ => unreachable!(), } } @@ -446,12 +446,14 @@ pub async fn cmd_configure( capacity: args .capacity .expect("Please specifiy a capacity with the -c flag"), - tag: args.tag.unwrap_or("".to_string()), + tag: args.tag.unwrap_or_default(), }, Some(old) => NetworkConfigEntry { - datacenter: args.datacenter.unwrap_or(old.datacenter.to_string()), + datacenter: args + .datacenter + .unwrap_or_else(|| old.datacenter.to_string()), capacity: args.capacity.unwrap_or(old.capacity), - tag: args.tag.unwrap_or(old.tag.to_string()), + tag: args.tag.unwrap_or_else(|| old.tag.to_string()), }, }; @@ -504,30 +506,30 @@ pub async fn cmd_remove( } pub async fn cmd_admin( - rpc_cli: RpcAddrClient, + rpc_cli: RpcAddrClient, rpc_host: SocketAddr, - args: AdminRPC, + args: AdminRpc, ) -> Result<(), Error> { match rpc_cli.call(&rpc_host, args, ADMIN_RPC_TIMEOUT).await?? { - AdminRPC::Ok(msg) => { + AdminRpc::Ok(msg) => { println!("{}", msg); } - AdminRPC::BucketList(bl) => { + AdminRpc::BucketList(bl) => { println!("List of buckets:"); for bucket in bl { println!("{}", bucket); } } - AdminRPC::BucketInfo(bucket) => { + AdminRpc::BucketInfo(bucket) => { print_bucket_info(&bucket); } - AdminRPC::KeyList(kl) => { + AdminRpc::KeyList(kl) => { println!("List of keys:"); for key in kl { println!("{}\t{}", key.0, key.1); } } - AdminRPC::KeyInfo(key) => { + AdminRpc::KeyInfo(key) => { print_key_info(&key); } r => { diff --git a/src/garage/server.rs b/src/garage/server.rs index 3004d3d2..36f7de5c 100644 --- a/src/garage/server.rs +++ b/src/garage/server.rs @@ -25,7 +25,7 @@ async fn shutdown_signal(send_cancel: watch::Sender) -> Result<(), Error> Ok(()) } -async fn wait_from(mut chan: watch::Receiver) -> () { +async fn wait_from(mut chan: watch::Receiver) { while !*chan.borrow() { if chan.changed().await.is_err() { return; @@ -48,7 +48,7 @@ pub async fn run_server(config_file: PathBuf) -> Result<(), Error> { .expect("Unable to open sled DB"); info!("Initialize RPC server..."); - let mut rpc_server = RpcServer::new(config.rpc_bind_addr.clone(), config.rpc_tls.clone()); + let mut rpc_server = RpcServer::new(config.rpc_bind_addr, config.rpc_tls.clone()); info!("Initializing background runner..."); let (send_cancel, watch_cancel) = watch::channel(false); @@ -71,9 +71,9 @@ pub async fn run_server(config_file: PathBuf) -> Result<(), Error> { let web_server = run_web_server(garage, wait_from(watch_cancel.clone())); futures::try_join!( - bootstrap.map(|rv| { + bootstrap.map(|()| { info!("Bootstrap done"); - Ok(rv) + Ok(()) }), run_rpc_server.map(|rv| { info!("RPC server exited");