diff options
author | Alex Solomes <alex.solomes@softwareag.com> | 2022-06-17 09:13:21 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-06-17 09:13:21 +0100 |
commit | 1cdf426d414b04cb5fdfc7bd4de351c6e60ef313 (patch) | |
tree | bcd6ddd2d06f0eb4eda2f713a7af4f47815efff0 | |
parent | b865681acc2a086b817eb98b12f6b63ae7cd7c3e (diff) | |
parent | a73df5a4722db6982458e168e05757fe01f3dccf (diff) |
Merge pull request #1191 from initard/improvement/825/fix-build-clippy-warnings
#825 fixing clippy warnings
21 files changed, 89 insertions, 99 deletions
diff --git a/crates/common/flockfile/src/unix.rs b/crates/common/flockfile/src/unix.rs index 2efaa316..4ef847d2 100644 --- a/crates/common/flockfile/src/unix.rs +++ b/crates/common/flockfile/src/unix.rs @@ -108,7 +108,7 @@ impl AsRef<Path> for Flockfile { /// Check `run_dir`/lock/ for a lock file of a given `app_name` pub fn check_another_instance_is_not_running( app_name: &str, - run_dir: &PathBuf, + run_dir: &Path, ) -> Result<Flockfile, FlockfileError> { match Flockfile::new_lock( run_dir diff --git a/crates/common/mqtt_channel/src/session.rs b/crates/common/mqtt_channel/src/session.rs index 17831758..97cd91f4 100644 --- a/crates/common/mqtt_channel/src/session.rs +++ b/crates/common/mqtt_channel/src/session.rs @@ -37,7 +37,7 @@ pub async fn init_session(config: &Config) -> Result<(), MqttError> { } Err(err) => { - eprintln!("Connection Error {}", err.to_string()); + eprintln!("Connection Error {}", err); break; } _ => (), @@ -77,7 +77,7 @@ pub async fn clear_session(config: &Config) -> Result<(), MqttError> { } Err(err) => { - eprintln!("Connection Error {}", err.to_string()); + eprintln!("Connection Error {}", err); break; } _ => (), diff --git a/crates/common/tedge_config/src/models/file_path.rs b/crates/common/tedge_config/src/models/file_path.rs index 62ef5723..7b80c972 100644 --- a/crates/common/tedge_config/src/models/file_path.rs +++ b/crates/common/tedge_config/src/models/file_path.rs @@ -45,8 +45,12 @@ impl std::fmt::Display for FilePath { } } +// If we `impl From<FilePath> for PathBuf` as suggested by clippy, +// Then we can no more have a generic implementation +// `impl<T> From<T> for FilePath where PathBuf: From<T>` +#[allow(clippy::from_over_into)] impl Into<PathBuf> for FilePath { fn into(self) -> PathBuf { - PathBuf::from(self.0) + self.0 } } diff --git a/crates/common/tedge_users/src/unix.rs b/crates/common/tedge_users/src/unix.rs index a37e8784..79a787fd 100644 --- a/crates/common/tedge_users/src/unix.rs +++ b/crates/common/tedge_users/src/unix.rs @@ -33,6 +33,9 @@ struct InnerUserManager { guard: Option<users::switch::SwitchUserGuard>, } +// this clippy warning is misleading because `UserManager` should only +// be crated once. There should be no "default" implemented for it +#[allow(clippy::new_without_default)] impl UserManager { /// Create a `UserManager`. /// diff --git a/crates/core/c8y_api/src/http_proxy.rs b/crates/core/c8y_api/src/http_proxy.rs index b9f78e32..3d1288d6 100644 --- a/crates/core/c8y_api/src/http_proxy.rs +++ b/crates/core/c8y_api/src/http_proxy.rs @@ -283,7 +283,7 @@ impl JwtAuthHttpProxy { C8yCreateEvent::new( Some(c8y_managed_object), event_type.clone(), - event_time.unwrap_or(OffsetDateTime::now_utc()), + event_time.unwrap_or_else(OffsetDateTime::now_utc), event_text.unwrap_or(event_type), HashMap::new(), ) diff --git a/crates/core/c8y_smartrest/src/error.rs b/crates/core/c8y_smartrest/src/error.rs index 10a867d3..465a6cab 100644 --- a/crates/core/c8y_smartrest/src/error.rs +++ b/crates/core/c8y_smartrest/src/error.rs @@ -1,6 +1,10 @@ use agent_interface::SoftwareUpdateResponse; use std::path::PathBuf; +// allowing large size difference between variants warning, +// because the enum `SmartRestSerializerError` is already Boxed +// in `SMCumulocityMapperError` +#[allow(clippy::large_enum_variant)] #[derive(thiserror::Error, Debug)] pub enum SmartRestSerializerError { #[error("The operation status is not supported. {response:?}")] @@ -77,7 +81,7 @@ pub enum SMCumulocityMapperError { FromReqwest(#[from] reqwest::Error), #[error(transparent)] - FromSmartRestSerializer(#[from] SmartRestSerializerError), + FromSmartRestSerializer(#[from] Box<SmartRestSerializerError>), #[error(transparent)] FromSmartRestDeserializer(#[from] SmartRestDeserializerError), diff --git a/crates/core/c8y_smartrest/src/operations.rs b/crates/core/c8y_smartrest/src/operations.rs index 5304b8b2..87e6aee9 100644 --- a/crates/core/c8y_smartrest/src/operations.rs +++ b/crates/core/c8y_smartrest/src/operations.rs @@ -42,25 +42,15 @@ impl Operation { } } -#[derive(Debug, Clone)] +#[derive(Debug, Default, Clone)] pub struct Operations { operations: Vec<Operation>, operations_by_trigger: HashMap<String, usize>, } -impl Default for Operations { - fn default() -> Self { - Self { - operations: vec![], - operations_by_trigger: HashMap::new(), - } - } -} - impl Operations { pub fn add_operation(&mut self, operation: Operation) { if self.operations.iter().any(|o| o.name.eq(&operation.name)) { - return; } else { if let Some(detail) = operation.exec() { if let Some(on_message) = &detail.on_message { diff --git a/crates/core/c8y_smartrest/src/smartrest_serializer.rs b/crates/core/c8y_smartrest/src/smartrest_serializer.rs index 503a345b..4581a2cf 100644 --- a/crates/core/c8y_smartrest/src/smartrest_serializer.rs +++ b/crates/core/c8y_smartrest/src/smartrest_serializer.rs @@ -201,11 +201,13 @@ where } fn serialize_smartrest<S: Serialize>(record: S) -> Result<String, SmartRestSerializerError> { - let mut wtr = WriterBuilder::new() - .has_headers(false) - .quote_style(QuoteStyle::Never) - .double_quote(false) - .from_writer(vec![]); + let mut wtr = Box::new( + WriterBuilder::new() + .has_headers(false) + .quote_style(QuoteStyle::Never) + .double_quote(false) + .from_writer(vec![]), + ); wtr.serialize(record)?; let csv = String::from_utf8(wtr.into_inner()?)?; Ok(csv) diff --git a/crates/core/tedge/src/main.rs b/crates/core/tedge/src/main.rs index eab007f6..e4dc777e 100644 --- a/crates/core/tedge/src/main.rs +++ b/crates/core/tedge/src/main.rs @@ -1,7 +1,7 @@ #![forbid(unsafe_code)] #![deny(clippy::mem_forget)] -use std::path::PathBuf; +use std::path::Path; use anyhow::Context; use clap::Parser; @@ -47,8 +47,8 @@ fn main() -> anyhow::Result<()> { } } -fn initialize_tedge(cfg_dir: &PathBuf) -> anyhow::Result<()> { - let config_dir = cfg_dir.as_path().display().to_string(); +fn initialize_tedge(cfg_dir: &Path) -> anyhow::Result<()> { + let config_dir = cfg_dir.display().to_string(); create_directory_with_user_group(&config_dir, "tedge", "tedge", 0o775)?; create_directory_with_user_group("/var/log/tedge", "tedge", "tedge", 0o775)?; create_directory_with_user_group( diff --git a/crates/core/tedge_agent/src/agent.rs b/crates/core/tedge_agent/src/agent.rs index 8fe3e18a..243463f1 100644 --- a/crates/core/tedge_agent/src/agent.rs +++ b/crates/core/tedge_agent/src/agent.rs @@ -144,7 +144,7 @@ impl SmAgentConfig { let tedge_download_dir = tedge_config.query_string(TmpPathSetting)?.into(); - let tedge_log_dir: String = tedge_config.query_string(LogPathSetting)?.into(); + let tedge_log_dir: String = tedge_config.query_string(LogPathSetting)?; let tedge_log_dir = PathBuf::from(&format!("{tedge_log_dir}/{AGENT_LOG_PATH}")); let tedge_run_dir = tedge_config.query_string(RunPathSetting)?.into(); diff --git a/crates/core/tedge_agent/src/restart_operation_handler.rs b/crates/core/tedge_agent/src/restart_operation_handler.rs index ed1b46ab..29342cb0 100644 --- a/crates/core/tedge_agent/src/restart_operation_handler.rs +++ b/crates/core/tedge_agent/src/restart_operation_handler.rs @@ -1,13 +1,7 @@ pub mod restart_operation { use crate::error::AgentError; - use std::{ - fs::File, - fs::OpenOptions, - io::Read, - io::Write, - path::{Path, PathBuf}, - }; + use std::{fs::File, fs::OpenOptions, io::Read, io::Write, path::Path}; use time::OffsetDateTime; const SLASH_RUN_PATH_TEDGE_AGENT_RESTART: &str = "tedge_agent/tedge_agent_restart"; @@ -20,7 +14,7 @@ pub mod restart_operation { /// ``` /// let () = RestartOperationHelper::create_slash_run_file()?; /// ``` - pub fn create_slash_run_file(run_dir: &PathBuf) -> Result<(), AgentError> { + pub fn create_slash_run_file(run_dir: &Path) -> Result<(), AgentError> { let path = &run_dir.join(SLASH_RUN_PATH_TEDGE_AGENT_RESTART); let path = Path::new(path); @@ -40,13 +34,13 @@ pub mod restart_operation { Ok(()) } - pub fn slash_run_file_exists(run_dir: &PathBuf) -> bool { + pub fn slash_run_file_exists(run_dir: &Path) -> bool { let path = &run_dir.join(SLASH_RUN_PATH_TEDGE_AGENT_RESTART); std::path::Path::new(path).exists() } /// returns the datetime of `SLASH_RUN_PATH_TEDGE_AGENT_RESTART` "modified at". - fn get_restart_file_datetime(run_dir: &PathBuf) -> Result<time::OffsetDateTime, AgentError> { + fn get_restart_file_datetime(run_dir: &Path) -> Result<time::OffsetDateTime, AgentError> { let mut file = File::open(&run_dir.join(SLASH_RUN_PATH_TEDGE_AGENT_RESTART))?; let mut contents = String::new(); file.read_to_string(&mut contents)?; @@ -100,15 +94,15 @@ pub mod restart_operation { } /// checks if system rebooted by comparing dt of tedge_agent_restart with dt of system restart. - pub fn has_rebooted(run_dir: &PathBuf) -> Result<bool, AgentError> { + pub fn has_rebooted(run_dir: &Path) -> Result<bool, AgentError> { // there is no slash run file after the reboot, so we assume success. // this is true for most of the cases as "/run/" is normally cleared after a reboot. - if !slash_run_file_exists(&run_dir) { + if !slash_run_file_exists(run_dir) { return Ok(true); } let system_reboot_dt = get_system_uptime()?; - let tedge_restart_file_dt = get_restart_file_datetime(&run_dir)?; + let tedge_restart_file_dt = get_restart_file_datetime(run_dir)?; Ok(system_reboot_dt > tedge_restart_file_dt) } diff --git a/crates/core/tedge_mapper/src/c8y/converter.rs b/crates/core/tedge_mapper/src/c8y/converter.rs index 9ec58cb0..fd9223b5 100644 --- a/crates/core/tedge_mapper/src/c8y/converter.rs +++ b/crates/core/tedge_mapper/src/c8y/converter.rs @@ -357,13 +357,13 @@ where message: &DiscoverOp, ) -> Result<Option<Message>, ConversionError> { match message.event_type { - EventType::ADD => { + EventType::Add => { let ops_dir = message.ops_dir.clone(); let op_name = message.operation_name.clone(); let op = get_operation(ops_dir.join(op_name))?; self.operations.add_operation(op); } - EventType::REMOVE => { + EventType::Remove => { self.operations.remove_operation(&message.operation_name); } } @@ -379,7 +379,7 @@ async fn parse_c8y_topics( ) -> Result<Vec<Message>, ConversionError> { match process_smartrest( message.payload_str()?, - &operations, + operations, http_proxy, operation_logs, ) @@ -721,7 +721,7 @@ async fn process_smartrest( match message_id { "528" => forward_software_request(payload, http_proxy).await, "510" => forward_restart_request(payload), - template => forward_operation_request(payload, template, &operations, operation_logs).await, + template => forward_operation_request(payload, template, operations, operation_logs).await, } } diff --git a/crates/core/tedge_mapper/src/c8y/dynamic_discovery.rs b/crates/core/tedge_mapper/src/c8y/dynamic_discovery.rs index fca9057d..4eff35c4 100644 --- a/crates/core/tedge_mapper/src/c8y/dynamic_discovery.rs +++ b/crates/core/tedge_mapper/src/c8y/dynamic_discovery.rs @@ -5,8 +5,8 @@ use serde::{Deserialize, Serialize}; #[derive(Serialize, Deserialize, Debug)] pub enum EventType { - ADD, - REMOVE, + Add, + Remove, } #[derive(Serialize, Deserialize, Debug)] @@ -54,23 +54,21 @@ pub fn process_inotify_events( if let Some(ops_name) = event.clone().name { let operation_name = ops_name .to_str() - .ok_or(DynamicDiscoverOpsError::NotAnOperationName( - ops_name.clone(), - )); + .ok_or_else(|| DynamicDiscoverOpsError::NotAnOperationName(ops_name.clone())); match operation_name { Ok(ops_name) => match event.mask { EventMask::DELETE => { return Ok(Some(DiscoverOp { ops_dir, - event_type: EventType::REMOVE, + event_type: EventType::Remove, operation_name: ops_name.to_string(), })) } EventMask::CLOSE_WRITE => { return Ok(Some(DiscoverOp { ops_dir, - event_type: EventType::ADD, + event_type: EventType::Add, operation_name: ops_name.to_string(), })) } diff --git a/crates/core/tedge_mapper/src/c8y/error.rs b/crates/core/tedge_mapper/src/c8y/error.rs index e388084d..60b9bea1 100644 --- a/crates/core/tedge_mapper/src/c8y/error.rs +++ b/crates/core/tedge_mapper/src/c8y/error.rs @@ -4,6 +4,7 @@ use c8y_smartrest::error::{ use plugin_sm::operation_logs::OperationLogsError; #[derive(thiserror::Error, Debug)] +#[allow(clippy::large_enum_variant)] #[allow(clippy::enum_variant_names)] pub enum CumulocityMapperError { #[error(transparent)] diff --git a/crates/core/tedge_mapper/src/core/error.rs b/crates/core/tedge_mapper/src/core/error.rs index 4280c738..b703161a 100644 --- a/crates/core/tedge_mapper/src/core/error.rs +++ b/crates/core/tedge_mapper/src/core/error.rs @@ -5,6 +5,9 @@ use mqtt_channel::MqttError; use tedge_config::TEdgeConfigError; use thin_edge_json::serialize::ThinEdgeJsonSerializationError; +// allowing enum_variant_names due to a False positive where it is +// detected that "all variants have the same prefix: `From`" +#[allow(clippy::enum_variant_names)] #[derive(Debug, thiserror::Error)] pub enum MapperError { #[error(transparent)] diff --git a/crates/core/tedge_mapper/src/core/mapper.rs b/crates/core/tedge_mapper/src/core/mapper.rs index d4f0da23..a16a042d 100644 --- a/crates/core/tedge_mapper/src/core/mapper.rs +++ b/crates/core/tedge_mapper/src/core/mapper.rs @@ -171,25 +171,20 @@ async fn process_inotify_and_mqtt_messages( } } } - event = inotify_events.next() => { - match event { - Some(ev) => { - match ev { - Ok(ev_string) => { - - match process_inotify_events(dir.clone(), ev_string) { - Ok(Some(discovered_ops)) => { - let _ = mapper.output.send(mapper.converter.process_operation_update_message(discovered_ops)).await; - } - Ok(None) => {} - Err(e) => {eprintln!("Processing inotify event failed due to {}", e);} - } - - - } Err(e) => {eprintln!("Failed to extract event {}", e);} + Some(event_or_error) = inotify_events.next() => { + match event_or_error { + Ok(event) => { + match process_inotify_events(dir.clone(), event) { + Ok(Some(discovered_ops)) => { + let _ = mapper.output.send(mapper.converter.process_operation_update_message(discovered_ops)).await; + } + Ok(None) => {} + Err(e) => {eprintln!("Processing inotify event failed due to {}", e);} } } - None => {} + Err(error) => { + eprintln!("Failed to extract event {}", error); + } } } } diff --git a/crates/core/tedge_mapper/src/main.rs b/crates/core/tedge_mapper/src/main.rs index 4b4a9f47..eabc82a7 100644 --- a/crates/core/tedge_mapper/src/main.rs +++ b/crates/core/tedge_mapper/src/main.rs @@ -84,10 +84,9 @@ async fn main() -> anyhow::Result<()> { tedge_config::TEdgeConfigLocation::from_custom_root(&mapper_opt.config_dir); let config = tedge_config::TEdgeConfigRepository::new(tedge_config_location.clone()).load()?; // Run only one instance of a mapper - let _flock = check_another_instance_is_not_running( - &mapper_opt.name.to_string(), - &config.query(RunPathSetting)?.into(), - )?; + + let run_dir: PathBuf = config.query(RunPathSetting)?.into(); + let _flock = check_another_instance_is_not_running(&mapper_opt.name.to_string(), &run_dir)?; if mapper_opt.init { component.init(&mapper_opt.config_dir).await diff --git a/crates/tests/tedge_test_utils/src/fs.rs b/crates/tests/tedge_test_utils/src/fs.rs index 45ed9ff0..8a1bb0e4 100644 --- a/crates/tests/tedge_test_utils/src/fs.rs +++ b/crates/tests/tedge_test_utils/src/fs.rs @@ -15,8 +15,8 @@ pub struct TempTedgeFile { file_path: PathBuf, } -impl TempTedgeDir { - pub fn new() -> Self { +impl Default for TempTedgeDir { + fn default() -> Self { let temp_dir = TempDir::new().unwrap(); let current_file_path = temp_dir.path().to_path_buf(); TempTedgeDir { @@ -24,12 +24,16 @@ impl TempTedgeDir { current_file_path, } } +} + +impl TempTedgeDir { + pub fn new() -> Self { + Self::default() + } pub fn dir(&self, directory_name: &str) -> TempTedgeDir { let root = self.temp_dir.path().to_path_buf(); - let path = root - .join(self.current_file_path.to_path_buf()) - .join(directory_name); + let path = root.join(&self.current_file_path).join(directory_name); if !path.exists() { let () = fs::create_dir(&path).unwrap(); @@ -43,9 +47,7 @@ impl TempTedgeDir { pub fn file(&self, file_name: &str) -> TempTedgeFile { let root = self.temp_dir.path().to_path_buf(); - let path = root - .join(self.current_file_path.to_path_buf()) - .join(file_name); + let path = root.join(&self.current_file_path).join(file_name); if !path.exists() { let _file = fs::File::create(&path).unwrap(); diff --git a/plugins/c8y_log_plugin/src/config.rs b/plugins/c8y_log_plugin/src/config.rs index 580fba95..8747bdf6 100644 --- a/plugins/c8y_log_plugin/src/config.rs +++ b/plugins/c8y_log_plugin/src/config.rs @@ -33,8 +33,7 @@ impl Borrow<String> for FileEntry { impl LogPluginConfig { pub fn new(config_file_path: &Path) -> Self { - let config = Self::read_config(config_file_path); - config + Self::read_config(config_file_path) } pub fn read_config(path: &Path) -> Self { diff --git a/plugins/c8y_log_plugin/src/logfile_request.rs b/plugins/c8y_log_plugin/src/logfile_request.rs index b538f714..f88afec5 100644 --- a/plugins/c8y_log_plugin/src/logfile_request.rs +++ b/plugins/c8y_log_plugin/src/logfile_request.rs @@ -115,8 +115,8 @@ pub fn new_read_logs( ) -> Result<String, anyhow::Error> { let mut output = String::new(); // first filter logs on type - let mut logfiles_to_read = filter_logs_on_type(&smartrest_obj, &plugin_config)?; - logfiles_to_read = filter_logs_path_on_metadata(&smartrest_obj, logfiles_to_read)?; + let mut logfiles_to_read = filter_logs_on_type(smartrest_obj, plugin_config)?; + logfiles_to_read = filter_logs_path_on_metadata(smartrest_obj, logfiles_to_read)?; let mut line_counter = 0usize; for logfile in logfiles_to_read { @@ -184,7 +184,7 @@ fn filter_logs_path_on_metadata( }; // if the file metadata can not be read, we set the file's metadata // to UNIX_EPOCH (Jan 1st 1970) - return OffsetDateTime::UNIX_EPOCH; + OffsetDateTime::UNIX_EPOCH }); logs_path_vec.reverse(); // to get most recent @@ -219,7 +219,7 @@ async fn execute_logfile_request_operation( let executing = LogfileRequest::executing()?; let () = mqtt_client.published.send(executing).await?; - let log_content = new_read_logs(&smartrest_request, &plugin_config)?; + let log_content = new_read_logs(smartrest_request, plugin_config)?; let upload_event_url = http_client .upload_log_binary(&smartrest_request.log_type, &log_content) diff --git a/plugins/c8y_log_plugin/src/main.rs b/plugins/c8y_log_plugin/src/main.rs index ce133bef..8a04589f 100644 --- a/plugins/c8y_log_plugin/src/main.rs +++ b/plugins/c8y_log_plugin/src/main.rs @@ -108,7 +108,7 @@ async fn run( "522" => { info!("Log request received: {payload}"); // retrieve smartrest object from payload - let smartrest_obj = SmartRestLogRequest::from_smartrest(&payload)?; + let smartrest_obj = SmartRestLogRequest::from_smartrest(payload)?; handle_logfile_request_operation( &smartrest_obj, &plugin_config, @@ -134,16 +134,10 @@ async fn run( return Ok(()); } } - Some(event_or_error) = inotify_stream.next() => { - if let Ok(event) = event_or_error { - match event.mask { - EventMask::CLOSE_WRITE => { - plugin_config = handle_dynamic_log_type_update(mqtt_client, config_file).await?; - } - _ => {} - } + Some(Ok(event)) = inotify_stream.next() => { + if event.mask == EventMask::CLOSE_WRITE { + plugin_config = handle_dynamic_log_type_update(mqtt_client, config_file).await?; } - } } } @@ -184,10 +178,10 @@ async fn main() -> Result<(), anyhow::Error> { Ok(()) } -fn init(config_dir: &PathBuf, logs_dir: &PathBuf) -> Result<(), anyhow::Error> { +fn init(config_dir: &Path, logs_dir: &Path) -> Result<(), anyhow::Error> { info!("Creating supported operation files"); - let config_dir = config_dir.as_path().display().to_string(); - let logs_dir = logs_dir.as_path().display().to_string(); + let config_dir = config_dir.display().to_string(); + let logs_dir = logs_dir.display().to_string(); let () = create_init_logs_directories_and_files(config_dir.as_str(), logs_dir.as_str())?; Ok(()) } @@ -206,7 +200,9 @@ fn create_default_log_plugin_file(path_to_toml: &str, logs_dir: &str) -> Result< .append(true) .create(false) .open(path_to_toml) - .expect(&format!("Unable to open file: {}", path_to_toml)); + .map_err(|error| { + anyhow::anyhow!("Unable to open file: {}. Error: {}", path_to_toml, error) + })?; toml_file.write_all(data.to_string().as_bytes())?; Ok(()) } |