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 /crates | |
parent | b865681acc2a086b817eb98b12f6b63ae7cd7c3e (diff) | |
parent | a73df5a4722db6982458e168e05757fe01f3dccf (diff) |
Merge pull request #1191 from initard/improvement/825/fix-build-clippy-warnings
#825 fixing clippy warnings
Diffstat (limited to 'crates')
-rw-r--r-- | crates/common/flockfile/src/unix.rs | 2 | ||||
-rw-r--r-- | crates/common/mqtt_channel/src/session.rs | 4 | ||||
-rw-r--r-- | crates/common/tedge_config/src/models/file_path.rs | 6 | ||||
-rw-r--r-- | crates/common/tedge_users/src/unix.rs | 3 | ||||
-rw-r--r-- | crates/core/c8y_api/src/http_proxy.rs | 2 | ||||
-rw-r--r-- | crates/core/c8y_smartrest/src/error.rs | 6 | ||||
-rw-r--r-- | crates/core/c8y_smartrest/src/operations.rs | 12 | ||||
-rw-r--r-- | crates/core/c8y_smartrest/src/smartrest_serializer.rs | 12 | ||||
-rw-r--r-- | crates/core/tedge/src/main.rs | 6 | ||||
-rw-r--r-- | crates/core/tedge_agent/src/agent.rs | 2 | ||||
-rw-r--r-- | crates/core/tedge_agent/src/restart_operation_handler.rs | 20 | ||||
-rw-r--r-- | crates/core/tedge_mapper/src/c8y/converter.rs | 8 | ||||
-rw-r--r-- | crates/core/tedge_mapper/src/c8y/dynamic_discovery.rs | 12 | ||||
-rw-r--r-- | crates/core/tedge_mapper/src/c8y/error.rs | 1 | ||||
-rw-r--r-- | crates/core/tedge_mapper/src/core/error.rs | 3 | ||||
-rw-r--r-- | crates/core/tedge_mapper/src/core/mapper.rs | 29 | ||||
-rw-r--r-- | crates/core/tedge_mapper/src/main.rs | 7 | ||||
-rw-r--r-- | crates/tests/tedge_test_utils/src/fs.rs | 18 |
18 files changed, 74 insertions, 79 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(); |