diff --git a/Cargo.lock b/Cargo.lock index f85043ef61a..fd73097b37b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -647,7 +647,6 @@ dependencies = [ "futures", "maplit", "mqtt_channel", - "regex", "reqwest", "serde", "serde_json", diff --git a/crates/core/c8y_api/Cargo.toml b/crates/core/c8y_api/Cargo.toml index 8ab6ff79ba5..589cd23e8e3 100644 --- a/crates/core/c8y_api/Cargo.toml +++ b/crates/core/c8y_api/Cargo.toml @@ -31,7 +31,6 @@ anyhow = { workspace = true } assert-json-diff = { workspace = true } assert_matches = { workspace = true } maplit = { workspace = true } -regex = { workspace = true } tempfile = { workspace = true } test-case = { workspace = true } time = { workspace = true, features = ["macros"] } diff --git a/crates/core/c8y_api/src/smartrest/alarm.rs b/crates/core/c8y_api/src/smartrest/alarm.rs index d600be8acb2..f70b84fc112 100644 --- a/crates/core/c8y_api/src/smartrest/alarm.rs +++ b/crates/core/c8y_api/src/smartrest/alarm.rs @@ -1,5 +1,6 @@ use crate::json_c8y::AlarmSeverity; use crate::json_c8y::C8yAlarm; +use crate::smartrest::csv::fields_to_csv_string; use time::format_description::well_known::Rfc3339; /// Serialize C8yAlarm to SmartREST message @@ -20,7 +21,7 @@ pub fn serialize_alarm(c8y_alarm: &C8yAlarm) -> Result format!("306,{}", alarm.alarm_type), + C8yAlarm::Clear(alarm) => fields_to_csv_string(&["306", &alarm.alarm_type]), }; Ok(smartrest) } diff --git a/crates/core/c8y_api/src/smartrest/csv.rs b/crates/core/c8y_api/src/smartrest/csv.rs new file mode 100644 index 00000000000..17e6f05d764 --- /dev/null +++ b/crates/core/c8y_api/src/smartrest/csv.rs @@ -0,0 +1,25 @@ +pub fn fields_to_csv_string(record: &[&str]) -> String { + let mut writer = csv::Writer::from_writer(vec![]); + writer + .write_record(record) + .expect("write to vec never fails"); + let mut output = writer.into_inner().expect("write to vec never fails"); + output.pop(); + String::from_utf8(output).expect("all input is utf-8") +} + +#[cfg(test)] +mod tests { + use crate::smartrest::csv::fields_to_csv_string; + + #[test] + fn normal_fields_containing_commas_are_quoted() { + assert_eq!(fields_to_csv_string(&["503", "test,me"]), "503,\"test,me\""); + } + + #[test] + fn normal_fields_containing_quotes_are_quoted() { + let rcd = fields_to_csv_string(&["503", r#"A value"with" quotes"#, "field"]); + assert_eq!(rcd, r#"503,"A value""with"" quotes",field"#); + } +} diff --git a/crates/core/c8y_api/src/smartrest/inventory.rs b/crates/core/c8y_api/src/smartrest/inventory.rs index fda1c6b77d5..e1d8257e233 100644 --- a/crates/core/c8y_api/src/smartrest/inventory.rs +++ b/crates/core/c8y_api/src/smartrest/inventory.rs @@ -9,6 +9,7 @@ // smartrest messages are sent. There should be one comprehensive API for // generating them. +use crate::smartrest::csv::fields_to_csv_string; use crate::smartrest::topic::publish_topic_from_ancestors; use mqtt_channel::Message; @@ -92,11 +93,13 @@ pub fn service_creation_message( Ok(Message::new( &publish_topic_from_ancestors(ancestors), - // XXX: if any arguments contain commas, output will be wrong - format!( - "102,{},{},{},{}", - service_id, service_type, service_name, service_status - ), + fields_to_csv_string(&[ + "102", + service_id, + service_type, + service_name, + service_status, + ]), )) } @@ -110,21 +113,16 @@ pub fn service_creation_message( /// to set the status of). /// /// https://cumulocity.com/guides/reference/smartrest-two/#104 -pub fn service_status_update_message(external_ids: &[String], service_status: &str) -> Message { +pub fn service_status_update_message( + external_ids: &[impl AsRef], + service_status: &str, +) -> Message { let topic = publish_topic_from_ancestors(external_ids); - let mut service_status = sanitize_for_smartrest( - service_status.into(), - super::message::MAX_PAYLOAD_LIMIT_IN_BYTES, - ); - - if service_status.contains(',') { - service_status = format!("\"{service_status}\""); - } - - let payload = format!("104,{service_status}"); + let service_status = + sanitize_for_smartrest(service_status, super::message::MAX_PAYLOAD_LIMIT_IN_BYTES); - Message::new(&topic, payload) + Message::new(&topic, fields_to_csv_string(&["104", &service_status])) } #[derive(thiserror::Error, Debug)] diff --git a/crates/core/c8y_api/src/smartrest/message.rs b/crates/core/c8y_api/src/smartrest/message.rs index 17146b7cf53..b466789ed25 100644 --- a/crates/core/c8y_api/src/smartrest/message.rs +++ b/crates/core/c8y_api/src/smartrest/message.rs @@ -34,17 +34,18 @@ pub fn get_smartrest_template_id(payload: &str) -> String { /// - Strip the input according to `max_size`. // TODO: make this return Result // TODO: make a variant which assumes `max_size = MAX_PAYLOAD_LIMIT_IN_BYTES` -pub fn sanitize_for_smartrest(input: Vec, max_size: usize) -> String { - String::from_utf8(input) - .unwrap_or_else(|err| { - error!("The input contains invalid UTF-8: {err}"); - String::default() - }) +pub fn sanitize_bytes_for_smartrest(input: &[u8], max_size: usize) -> String { + let input = std::str::from_utf8(input).unwrap_or_else(|err| { + error!("The input contains invalid UTF-8: {err}"); + "" + }); + sanitize_for_smartrest(input, max_size) +} + +pub fn sanitize_for_smartrest(input: &str, max_size: usize) -> String { + input .chars() .filter(|&c| c == '\r' || c == '\n' || c == '\t' || !c.is_control()) - .collect::() - .replace('"', "\"\"") - .chars() .scan(0, |bytes_count, c| { *bytes_count += c.len_utf8(); Some((*bytes_count, c)) @@ -62,17 +63,16 @@ pub fn sanitize_for_smartrest(input: Vec, max_size: usize) -> String { /// If the input has only one line, returns the line only. /// If the input is empty or contains invalid UTF-8, it returns an empty String. /// The output is ensured to be SmartREST compatible. -pub fn get_failure_reason_for_smartrest(input: Vec, max_size: usize) -> String { - let input_string = String::from_utf8(input).unwrap_or_else(|err| { +pub fn get_failure_reason_for_smartrest(input: &[u8], max_size: usize) -> String { + let input_string = std::str::from_utf8(input).unwrap_or_else(|err| { error!("The input contains invalid UTF-8: {err}"); - String::default() + "" }); - let last_line = input_string.lines().last().unwrap_or_default(); - let failure_reason = match input_string.lines().count() { - 0 | 1 => last_line.to_string(), - _ => format!("{}\n\n{}", last_line, input_string.as_str()), - }; - sanitize_for_smartrest(failure_reason.as_bytes().to_vec(), max_size) + let last_line = input_string.trim_end().lines().last().unwrap_or_default(); + match input_string.lines().count() { + 0 | 1 => sanitize_for_smartrest(last_line, max_size), + _ => sanitize_for_smartrest(&format!("{last_line}\n\n{input_string}"), max_size), + } } /// Split MQTT message payload to multiple SmartREST messages. @@ -139,7 +139,6 @@ pub fn collect_smartrest_messages(data: &str) -> Vec { #[cfg(test)] mod tests { use super::*; - use regex::Regex; use test_case::test_case; #[test_case("512,device_id", Some("device_id"); "valid")] @@ -167,54 +166,54 @@ mod tests { #[test] fn selected_control_chars_remain() { - let input = vec![0x00, 0x09, 0x0A, 0x0D]; // NULL, \t, \n, \r - let sanitized = sanitize_for_smartrest(input, MAX_PAYLOAD_LIMIT_IN_BYTES); + let input = b"\0\t\n\r"; + let sanitized = sanitize_bytes_for_smartrest(input, MAX_PAYLOAD_LIMIT_IN_BYTES); assert_eq!(sanitized, "\t\n\r".to_string()); } #[test] fn control_chars_are_removed() { - let input: Vec = (0x00..0xff).collect(); - let sanitized = sanitize_for_smartrest(input, MAX_PAYLOAD_LIMIT_IN_BYTES); - let re = Regex::new(r"[^\x20-\x7E\xA0-\xFF\t\n\r]").unwrap(); - assert!(!re.is_match(&sanitized)); + let input: Vec = (0x00..=0x7f).collect(); + let sanitized = sanitize_bytes_for_smartrest(&input, MAX_PAYLOAD_LIMIT_IN_BYTES); + assert_eq!( + sanitized, + "\t\n\r !\"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~" + ); } #[test] fn invalid_utf8_is_contained() { let invalid_sparkle_heart = vec![0, 159, 146, 150]; - let sanitized = sanitize_for_smartrest(invalid_sparkle_heart, MAX_PAYLOAD_LIMIT_IN_BYTES); - assert_eq!(sanitized, "".to_string()); + let sanitized = + sanitize_bytes_for_smartrest(&invalid_sparkle_heart, MAX_PAYLOAD_LIMIT_IN_BYTES); + assert_eq!(sanitized, ""); } #[test] fn invalid_utf8_is_contained_last_line() { let invalid_sparkle_heart = vec![0, 159, 146, 150]; let last_line = - get_failure_reason_for_smartrest(invalid_sparkle_heart, MAX_PAYLOAD_LIMIT_IN_BYTES); - assert_eq!(last_line, "".to_string()); + get_failure_reason_for_smartrest(&invalid_sparkle_heart, MAX_PAYLOAD_LIMIT_IN_BYTES); + assert_eq!(last_line, ""); } #[test_case("foo bar baz\n", "foo bar baz\n"; "standard")] - #[test_case("foo\r\nbar\tbaz\0\"", "foo\r\nbar\tbaz\"\""; "with control chars")] + #[test_case("foo\r\nbar\tbaz\0\"", "foo\r\nbar\tbaz\""; "with control chars")] #[test_case("baz", "baz"; "no new line")] #[test_case("", ""; "empty")] #[test_case("こんにちは", "こんにちは"; "no ascii")] - #[test_case("こんにちは\"\n\0こんにちは", "こんにちは\"\"\nこんにちは"; "no ascii and control chars")] + #[test_case("こんにちは\"\n\0こんにちは", "こんにちは\"\nこんにちは"; "no ascii and control chars")] fn u8_vec_is_sanitized(input: &str, expected_output: &str) { - let vec_u8 = input.as_bytes().to_vec(); - let sanitized = sanitize_for_smartrest(vec_u8, MAX_PAYLOAD_LIMIT_IN_BYTES); + let sanitized = sanitize_for_smartrest(input, MAX_PAYLOAD_LIMIT_IN_BYTES); assert_eq!(sanitized, expected_output.to_string()); } #[test_case("foo bar bye", "foo bar by"; "ascii")] #[test_case("こんにちは", "こんに"; "no ascii")] fn size_u8_vec_is_stripped(input: &str, expected_output: &str) { - let vec_u8 = input.as_bytes().to_vec(); - let stripped = sanitize_for_smartrest(vec_u8, 10); + let stripped = sanitize_for_smartrest(input, 10); assert_eq!(stripped, expected_output.to_string()); } - #[test_case("baz\n", "baz"; "one line")] #[test_case("baz", "baz"; "no new line")] #[test_case("foo\r\nbar\n\nbaz\n", "baz\n\nfoo\r\n"; "multiline")] @@ -222,8 +221,7 @@ mod tests { #[test_case("おはよう\nこんにちは\n", "こんに"; "no ascii")] #[test_case("あ\nい\nう\nえ\nお\n", "お\n\nあ\n"; "no ascii2")] fn return_formatted_text_for_failure_reason_from_vec_u8(input: &str, expected_output: &str) { - let vec_u8 = input.as_bytes().to_vec(); - let last_line = get_failure_reason_for_smartrest(vec_u8, 10); + let last_line = get_failure_reason_for_smartrest(input.as_bytes(), 10); assert_eq!(last_line.as_str(), expected_output); } diff --git a/crates/core/c8y_api/src/smartrest/mod.rs b/crates/core/c8y_api/src/smartrest/mod.rs index 5e9cf03e9fc..0deece16772 100644 --- a/crates/core/c8y_api/src/smartrest/mod.rs +++ b/crates/core/c8y_api/src/smartrest/mod.rs @@ -1,4 +1,5 @@ pub mod alarm; +pub mod csv; pub mod error; pub mod inventory; pub mod message; diff --git a/crates/core/c8y_api/src/smartrest/operations.rs b/crates/core/c8y_api/src/smartrest/operations.rs index 9afe4a37f07..f72308e8df2 100644 --- a/crates/core/c8y_api/src/smartrest/operations.rs +++ b/crates/core/c8y_api/src/smartrest/operations.rs @@ -5,12 +5,10 @@ use std::path::Path; use std::path::PathBuf; use crate::smartrest::error::OperationsError; -use crate::smartrest::smartrest_serializer::SmartRestSerializer; -use crate::smartrest::smartrest_serializer::SmartRestSetSupportedOperations; +use crate::smartrest::smartrest_serializer::declare_supported_operations; use serde::Deserialize; use serde::Deserializer; -use super::error::SmartRestSerializerError; use std::time::Duration; const DEFAULT_GRACEFUL_TIMEOUT: Duration = Duration::from_secs(3600); @@ -173,11 +171,11 @@ impl Operations { .collect::>() } - pub fn create_smartrest_ops_message(&self) -> Result { + pub fn create_smartrest_ops_message(&self) -> String { let mut ops = self.get_operations_list(); ops.sort(); - let ops = ops.iter().map(|op| op as &str).collect::>(); - SmartRestSetSupportedOperations::new(&ops).to_smartrest() + let ops = ops.iter().map(|op| op.as_str()).collect::>(); + declare_supported_operations(&ops) } } diff --git a/crates/core/c8y_api/src/smartrest/smartrest_serializer.rs b/crates/core/c8y_api/src/smartrest/smartrest_serializer.rs index bf3fd14ddf0..ef95fb110d6 100644 --- a/crates/core/c8y_api/src/smartrest/smartrest_serializer.rs +++ b/crates/core/c8y_api/src/smartrest/smartrest_serializer.rs @@ -1,15 +1,76 @@ +use crate::smartrest::csv::fields_to_csv_string; use crate::smartrest::error::SmartRestSerializerError; use crate::smartrest::topic::C8yTopic; -use csv::QuoteStyle; -use csv::WriterBuilder; +use csv::StringRecord; use mqtt_channel::Message; +use serde::ser::SerializeSeq; use serde::Deserialize; use serde::Serialize; use serde::Serializer; pub type SmartRest = String; -#[derive(Debug)] +pub fn request_pending_operations() -> &'static str { + "500" +} + +/// Generates a SmartREST message to set the provided operation to executing +pub fn set_operation_executing(operation: impl C8yOperation) -> String { + fields_to_csv_string(&["501", operation.name()]) +} + +/// Generates a SmartREST message to set the provided operation to failed with the provided reason +pub fn fail_operation(operation: impl C8yOperation, reason: &str) -> String { + fields_to_csv_string(&["502", operation.name(), reason]) +} + +/// Generates a SmartREST message to set the provided operation to successful without a payload +pub fn succeed_operation_no_payload(operation: CumulocitySupportedOperations) -> String { + succeed_static_operation(operation, None::<&str>) +} + +/// Generates a SmartREST message to set the provided operation to successful with an optional payload +pub fn succeed_static_operation( + operation: CumulocitySupportedOperations, + payload: Option>, +) -> String { + let mut wtr = csv::Writer::from_writer(vec![]); + // Serialization will never fail for text + match payload { + Some(payload) => wtr.serialize(("503", operation.name(), payload.as_ref())), + None => wtr.serialize(("503", operation.name())), + } + .unwrap(); + let mut output = wtr.into_inner().unwrap(); + output.pop(); + String::from_utf8(output).unwrap() +} + +/// Generates a SmartREST message to set the provided custom operation to successful with a text or csv payload +/// +/// - If the payload is "text", then a single payload field will be created +/// - If the payload is "csv", then the provided CSV record will be appended to the SmartREST message +/// +/// # CSV +/// If the provided CSV does not match the standard Cumulocity format, the standard CSV escaping +/// rules will be applied. For example, `a,field "with" quotes` will be converted to +/// `a,"field ""with"" quotes"` before being appended to the output of this function. +/// +/// # Errors +/// This will return an error if the payload is a CSV with multiple records, or an empty CSV. +pub fn succeed_operation( + operation: &str, + reason: impl Into, +) -> Result { + let mut wtr = csv::Writer::from_writer(vec![]); + // Serialization can fail for CSV, but not for text + wtr.serialize(("503", operation, reason.into()))?; + let mut output = wtr.into_inner().unwrap(); + output.pop(); + Ok(String::from_utf8(output)?) +} + +#[derive(Debug, Copy, Clone)] pub enum CumulocitySupportedOperations { C8ySoftwareUpdate, C8yLogFileRequest, @@ -32,53 +93,10 @@ impl From for &'static str { } } -pub trait SmartRestSerializer<'a> -where - Self: Serialize, -{ - fn to_smartrest(&self) -> Result { - serialize_smartrest(self) - } -} - -#[derive(Debug, Deserialize, Serialize, Eq, PartialEq)] -pub struct SmartRestSetSupportedLogType { - pub message_id: &'static str, - pub supported_operations: Vec, -} - -impl From> for SmartRestSetSupportedLogType { - fn from(operation_types: Vec) -> Self { - Self { - message_id: "118", - supported_operations: operation_types, - } - } -} - -impl<'a> SmartRestSerializer<'a> for SmartRestSetSupportedLogType {} - -#[derive(Debug, Deserialize, Serialize, Eq, PartialEq)] -pub struct SmartRestSetSupportedOperations<'a> { - pub message_id: &'static str, - pub supported_operations: Vec<&'a str>, -} - -impl<'a> SmartRestSetSupportedOperations<'a> { - pub fn new(supported_operations: &[&'a str]) -> Self { - Self { - message_id: "114", - supported_operations: supported_operations.into(), - } - } - - pub fn add_operation(&mut self, operation: &'a str) { - self.supported_operations.push(operation); - } +pub fn declare_supported_operations(ops: &[&str]) -> String { + format!("114,{}", fields_to_csv_string(ops)) } -impl<'a> SmartRestSerializer<'a> for SmartRestSetSupportedOperations<'a> {} - #[derive(Debug, Deserialize, Serialize, Eq, PartialEq)] pub struct SmartRestSoftwareModuleItem { pub software: String, @@ -86,121 +104,103 @@ pub struct SmartRestSoftwareModuleItem { pub url: Option, } -#[derive(Debug, Deserialize, Serialize, Eq, PartialEq)] -pub struct SmartRestGetPendingOperations { - pub id: &'static str, +/// A supported operation of the thin-edge device, used in status updates via SmartREST +/// +/// This has two implementations, `&str` for custom operations, and [CumolocitySupportedOperations] +/// for statically supported operations. +pub trait C8yOperation { + fn name(&self) -> &str; } -impl Default for SmartRestGetPendingOperations { - fn default() -> Self { - Self { id: "500" } +impl C8yOperation for CumulocitySupportedOperations { + fn name(&self) -> &str { + (*self).into() } } -impl<'a> SmartRestSerializer<'a> for SmartRestGetPendingOperations {} - -#[derive(Debug, Deserialize, Serialize, Eq, PartialEq)] -pub struct SmartRestSetOperationToExecuting { - pub message_id: &'static str, - pub operation: &'static str, -} - -impl SmartRestSetOperationToExecuting { - pub fn new(operation: CumulocitySupportedOperations) -> Self { - Self { - message_id: "501", - operation: operation.into(), - } +impl<'a> C8yOperation for &'a str { + fn name(&self) -> &str { + self } } -impl<'a> SmartRestSerializer<'a> for SmartRestSetOperationToExecuting {} - -#[derive(Debug, Deserialize, Serialize, Eq, PartialEq)] -pub struct SmartRestSetOperationToSuccessful { - pub message_id: &'static str, - pub operation: &'static str, - pub operation_parameter: Option, +#[derive(Debug, Serialize, Eq, PartialEq)] +#[serde(untagged)] +pub enum TextOrCsv { + Text(String), + Csv(EmbeddedCsv), } -impl SmartRestSetOperationToSuccessful { - pub fn new(operation: CumulocitySupportedOperations) -> Self { - Self { - message_id: "503", - operation: operation.into(), - operation_parameter: None, - } +impl From for TextOrCsv { + fn from(value: EmbeddedCsv) -> Self { + Self::Csv(value) } +} - pub fn with_response_parameter(self, response_parameter: &str) -> Self { - Self { - operation_parameter: Some(response_parameter.into()), - ..self - } +impl From for TextOrCsv { + fn from(value: String) -> Self { + Self::Text(value) } } -impl<'a> SmartRestSerializer<'a> for SmartRestSetOperationToSuccessful {} +#[derive(Debug, Eq, PartialEq)] +pub struct EmbeddedCsv(String); -#[derive(Debug, Deserialize, Serialize, Eq, PartialEq)] -pub struct SmartRestSetOperationToFailed { - pub message_id: &'static str, - pub operation: &'static str, - #[serde(serialize_with = "reason_to_string_with_quotes")] - pub reason: String, +impl EmbeddedCsv { + pub fn new(value: String) -> Self { + Self(value) + } } -impl SmartRestSetOperationToFailed { - pub fn new(operation: CumulocitySupportedOperations, reason: String) -> Self { - Self { - message_id: "502", - operation: operation.into(), - reason, +impl Serialize for EmbeddedCsv { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + S::Error: serde::ser::Error, + { + let record = parse_single_record(&self.0)?; + let mut seq = serializer.serialize_seq(Some(record.len()))?; + for field in record.iter() { + seq.serialize_element(field)?; } + seq.end() } } -impl<'a> SmartRestSerializer<'a> for SmartRestSetOperationToFailed {} - -fn reason_to_string_with_quotes(reason: &str, serializer: S) -> Result +fn parse_single_record(csv: &str) -> Result where - S: Serializer, + E: serde::ser::Error, { - let s = format!("\"{}\"", reason); - serializer.serialize_str(&s) -} - -fn serialize_smartrest(record: S) -> Result { - let mut wtr = WriterBuilder::new() + let mut rdr = csv::ReaderBuilder::new() .has_headers(false) - .quote_style(QuoteStyle::Never) - .double_quote(false) - .from_writer(vec![]); - wtr.serialize(record)?; - - // csv::IntoInnerError still contains the writer and we can use it to - // recover, but if we don't and only report the error, we should report the - // inner io::Error - let csv = wtr.into_inner().map_err(|e| e.into_error())?; - let csv = String::from_utf8(csv)?; - Ok(csv) + .from_reader(csv.as_bytes()); + + let mut record = StringRecord::new(); + match rdr.read_record(&mut record) { + Ok(true) => Ok(()), + Ok(false) => Err(E::custom("No data in embedded csv")), + Err(e) => Err(E::custom(format!( + "Failed to read record from embedded csv: {e}" + ))), + }?; + match rdr.read_record(&mut StringRecord::new()) { + Ok(false) => Ok(record), + Ok(true) | Err(_) => Err(E::custom(format!("Multiple CSV records found (did you forget to quote a field containing a newline?) in {csv:?}"))), + } } /// Helper to generate a SmartREST operation status message -pub trait TryIntoOperationStatusMessage { - fn executing() -> Result { - let status = Self::status_executing()?; - Ok(Self::create_message(status)) +pub trait OperationStatusMessage { + fn executing() -> Message { + Self::create_message(Self::status_executing()) } - fn successful(parameter: Option) -> Result { - let status = Self::status_successful(parameter)?; - Ok(Self::create_message(status)) + fn successful(parameter: Option<&str>) -> Message { + Self::create_message(Self::status_successful(parameter)) } - fn failed(failure_reason: String) -> Result { - let status = Self::status_failed(failure_reason)?; - Ok(Self::create_message(status)) + fn failed(failure_reason: &str) -> Message { + Self::create_message(Self::status_failed(failure_reason)) } fn create_message(payload: SmartRest) -> Message { @@ -208,9 +208,9 @@ pub trait TryIntoOperationStatusMessage { Message::new(&topic, payload) } - fn status_executing() -> Result; - fn status_successful(parameter: Option) -> Result; - fn status_failed(failure_reason: String) -> Result; + fn status_executing() -> SmartRest; + fn status_successful(parameter: Option<&str>) -> SmartRest; + fn status_failed(failure_reason: &str) -> SmartRest; } #[cfg(test)] @@ -219,76 +219,122 @@ mod tests { #[test] fn serialize_smartrest_supported_operations() { - let smartrest = - SmartRestSetSupportedOperations::new(&["c8y_SoftwareUpdate", "c8y_LogfileRequest"]) - .to_smartrest() - .unwrap(); - assert_eq!(smartrest, "114,c8y_SoftwareUpdate,c8y_LogfileRequest\n"); + let smartrest = declare_supported_operations(&["c8y_SoftwareUpdate", "c8y_LogfileRequest"]); + assert_eq!(smartrest, "114,c8y_SoftwareUpdate,c8y_LogfileRequest"); } #[test] fn serialize_smartrest_get_pending_operations() { - let smartrest = SmartRestGetPendingOperations::default() - .to_smartrest() - .unwrap(); - assert_eq!(smartrest, "500\n"); + let smartrest = request_pending_operations(); + assert_eq!(smartrest, "500"); } #[test] fn serialize_smartrest_set_operation_to_executing() { - let smartrest = - SmartRestSetOperationToExecuting::new(CumulocitySupportedOperations::C8ySoftwareUpdate) - .to_smartrest() - .unwrap(); - assert_eq!(smartrest, "501,c8y_SoftwareUpdate\n"); + let smartrest = set_operation_executing(CumulocitySupportedOperations::C8ySoftwareUpdate); + assert_eq!(smartrest, "501,c8y_SoftwareUpdate"); } #[test] fn serialize_smartrest_set_operation_to_successful() { - let smartrest = SmartRestSetOperationToSuccessful::new( + let smartrest = + succeed_operation_no_payload(CumulocitySupportedOperations::C8ySoftwareUpdate); + assert_eq!(smartrest, "503,c8y_SoftwareUpdate"); + } + + #[test] + fn serialize_smartrest_set_operation_to_successful_with_payload() { + let smartrest = succeed_static_operation( CumulocitySupportedOperations::C8ySoftwareUpdate, + Some("a payload"), + ); + assert_eq!(smartrest, "503,c8y_SoftwareUpdate,a payload"); + } + + #[test] + fn serialize_smartrest_set_custom_operation_to_successful_with_text_payload() { + let smartrest = succeed_operation( + "c8y_RelayArray", + TextOrCsv::Text("true,false,true".to_owned()), ) - .to_smartrest() .unwrap(); - assert_eq!(smartrest, "503,c8y_SoftwareUpdate,\n"); + assert_eq!(smartrest, "503,c8y_RelayArray,\"true,false,true\""); } #[test] - fn serialize_smartrest_set_operation_to_failed() { - let smartrest = SmartRestSetOperationToFailed::new( - CumulocitySupportedOperations::C8ySoftwareUpdate, - "Failed due to permission.".into(), + fn serialize_smartrest_set_custom_operation_to_successful_with_csv_payload() { + let smartrest = succeed_operation( + "c8y_RelayArray", + TextOrCsv::Csv(EmbeddedCsv("true,false,true".to_owned())), + ) + .unwrap(); + assert_eq!(smartrest, "503,c8y_RelayArray,true,false,true"); + } + + #[test] + fn serialize_smartrest_set_custom_operation_to_successful_with_multi_record_csv_payload() { + succeed_operation( + "c8y_RelayArray", + TextOrCsv::Csv(EmbeddedCsv("true\n1,2,3".to_owned())), + ) + .unwrap_err(); + } + + #[test] + fn serialize_smartrest_set_custom_operation_to_successful_requotes_csv_payload() { + let smartrest = succeed_operation( + "c8y_RelayArray", + TextOrCsv::Csv(EmbeddedCsv("true,random\"quote".to_owned())), ) - .to_smartrest() .unwrap(); + assert_eq!(smartrest, "503,c8y_RelayArray,true,\"random\"\"quote\""); + } + + #[test] + fn serialize_smartrest_set_operation_to_failed() { + let smartrest = fail_operation( + CumulocitySupportedOperations::C8ySoftwareUpdate, + "Failed due to permission.", + ); assert_eq!( smartrest, - "502,c8y_SoftwareUpdate,\"Failed due to permission.\"\n" + "502,c8y_SoftwareUpdate,Failed due to permission." + ); + } + + #[test] + fn serialize_smartrest_set_custom_operation_to_failed() { + let smartrest = fail_operation("c8y_Custom", "Something went wrong"); + assert_eq!(smartrest, "502,c8y_Custom,Something went wrong"); + } + + #[test] + fn serialize_smartrest_set_operation_to_failed_with_quotes() { + let smartrest = fail_operation( + CumulocitySupportedOperations::C8ySoftwareUpdate, + "Failed due to permi\"ssion.", + ); + assert_eq!( + smartrest, + "502,c8y_SoftwareUpdate,\"Failed due to permi\"\"ssion.\"" ); } #[test] fn serialize_smartrest_set_operation_to_failed_with_comma_reason() { - let smartrest = SmartRestSetOperationToFailed::new( + let smartrest = fail_operation( CumulocitySupportedOperations::C8ySoftwareUpdate, - "Failed to install collectd, modbus, and golang.".into(), - ) - .to_smartrest() - .unwrap(); + "Failed to install collectd, modbus, and golang.", + ); assert_eq!( smartrest, - "502,c8y_SoftwareUpdate,\"Failed to install collectd, modbus, and golang.\"\n" + "502,c8y_SoftwareUpdate,\"Failed to install collectd, modbus, and golang.\"" ); } #[test] fn serialize_smartrest_set_operation_to_failed_with_empty_reason() { - let smartrest = SmartRestSetOperationToFailed::new( - CumulocitySupportedOperations::C8ySoftwareUpdate, - "".into(), - ) - .to_smartrest() - .unwrap(); - assert_eq!(smartrest, "502,c8y_SoftwareUpdate,\"\"\n"); + let smartrest = fail_operation(CumulocitySupportedOperations::C8ySoftwareUpdate, ""); + assert_eq!(smartrest, "502,c8y_SoftwareUpdate,"); } } diff --git a/crates/core/c8y_api/src/smartrest/topic.rs b/crates/core/c8y_api/src/smartrest/topic.rs index f0a075123f7..8b5c79cf32c 100644 --- a/crates/core/c8y_api/src/smartrest/topic.rs +++ b/crates/core/c8y_api/src/smartrest/topic.rs @@ -93,12 +93,12 @@ impl From<&C8yAlarm> for C8yTopic { /// - `["main"]` -> `c8y/s/us` /// - `["child1", "main"]` -> `c8y/s/us/child1` /// - `["child2", "child1", "main"]` -> `c8y/s/us/child1/child2` -pub fn publish_topic_from_ancestors(ancestors: &[String]) -> Topic { +pub fn publish_topic_from_ancestors(ancestors: &[impl AsRef]) -> Topic { let mut target_topic = SMARTREST_PUBLISH_TOPIC.to_string(); for ancestor in ancestors.iter().rev().skip(1) { // Skipping the last ancestor as it is the main device represented by the root topic itself target_topic.push('/'); - target_topic.push_str(ancestor); + target_topic.push_str(ancestor.as_ref()); } Topic::new_unchecked(&target_topic) @@ -124,8 +124,7 @@ mod tests { #[test_case(&["child1", "main"], "c8y/s/us/child1")] #[test_case(&["child3", "child2", "child1", "main"], "c8y/s/us/child1/child2/child3")] fn topic_from_ancestors(ancestors: &[&str], topic: &str) { - let ancestors: Vec = ancestors.iter().map(|v| v.to_string()).collect(); - let nested_child_topic = publish_topic_from_ancestors(&ancestors); + let nested_child_topic = publish_topic_from_ancestors(ancestors); assert_eq!(nested_child_topic, Topic::new_unchecked(topic)); } } diff --git a/crates/extensions/c8y_config_manager/src/actor.rs b/crates/extensions/c8y_config_manager/src/actor.rs index 2fd15b06b3c..6b3b50175f4 100644 --- a/crates/extensions/c8y_config_manager/src/actor.rs +++ b/crates/extensions/c8y_config_manager/src/actor.rs @@ -15,7 +15,7 @@ use async_trait::async_trait; use c8y_api::smartrest::smartrest_deserializer::SmartRestConfigDownloadRequest; use c8y_api::smartrest::smartrest_deserializer::SmartRestConfigUploadRequest; use c8y_api::smartrest::smartrest_deserializer::SmartRestRequestGeneric; -use c8y_api::smartrest::smartrest_serializer::TryIntoOperationStatusMessage; +use c8y_api::smartrest::smartrest_serializer::OperationStatusMessage; use c8y_api::smartrest::topic::C8yTopic; use c8y_http_proxy::handle::C8YHttpProxy; use log::error; @@ -117,16 +117,13 @@ impl ConfigManagerActor { ConfigOperation::Update, None, ActiveOperationState::Pending, - format!("Failed due to {}", err), + &format!("Failed due to {err}"), &mut self.messages, ) .await?; } } else { - error!( - "Incorrect Download SmartREST payload: {}", - smartrest_message - ); + error!("Incorrect Download SmartREST payload: {smartrest_message}"); } Ok(()) } @@ -146,13 +143,13 @@ impl ConfigManagerActor { ConfigOperation::Snapshot, None, ActiveOperationState::Pending, - format!("Failed due to {}", err), + &format!("Failed due to {err}"), &mut self.messages, ) .await?; } } else { - error!("Incorrect Upload SmartREST payload: {}", smartrest_message); + error!("Incorrect Upload SmartREST payload: {smartrest_message}"); } Ok(()) } @@ -209,7 +206,7 @@ impl ConfigManagerActor { config_operation, Some(child_id), ActiveOperationState::Pending, - err.to_string(), + &err.to_string(), &mut self.messages, ) .await @@ -290,7 +287,7 @@ impl ConfigManagerActor { config_operation: ConfigOperation, child_id: Option, op_state: ActiveOperationState, - failure_reason: String, + failure_reason: &str, message_box: &mut ConfigManagerMessageBox, ) -> Result<(), ConfigManagementError> { // Fail the operation in the cloud by sending EXECUTING and FAILED responses back to back @@ -305,33 +302,33 @@ impl ConfigManagerActor { ConfigOperation::Snapshot => { executing_msg = MqttMessage::new( &c8y_child_topic, - UploadConfigFileStatusMessage::status_executing()?, + UploadConfigFileStatusMessage::status_executing(), ); failed_msg = MqttMessage::new( &c8y_child_topic, - UploadConfigFileStatusMessage::status_failed(failure_reason)?, + UploadConfigFileStatusMessage::status_failed(failure_reason), ); } ConfigOperation::Update => { executing_msg = MqttMessage::new( &c8y_child_topic, - DownloadConfigFileStatusMessage::status_executing()?, + DownloadConfigFileStatusMessage::status_executing(), ); failed_msg = MqttMessage::new( &c8y_child_topic, - DownloadConfigFileStatusMessage::status_failed(failure_reason)?, + DownloadConfigFileStatusMessage::status_failed(failure_reason), ); } } } else { match config_operation { ConfigOperation::Snapshot => { - executing_msg = UploadConfigFileStatusMessage::executing()?; - failed_msg = UploadConfigFileStatusMessage::failed(failure_reason)?; + executing_msg = UploadConfigFileStatusMessage::executing(); + failed_msg = UploadConfigFileStatusMessage::failed(failure_reason); } ConfigOperation::Update => { - executing_msg = DownloadConfigFileStatusMessage::executing()?; - failed_msg = UploadConfigFileStatusMessage::failed(failure_reason)?; + executing_msg = DownloadConfigFileStatusMessage::executing(); + failed_msg = UploadConfigFileStatusMessage::failed(failure_reason); } }; } diff --git a/crates/extensions/c8y_config_manager/src/download.rs b/crates/extensions/c8y_config_manager/src/download.rs index c8b688b4913..96806807c41 100644 --- a/crates/extensions/c8y_config_manager/src/download.rs +++ b/crates/extensions/c8y_config_manager/src/download.rs @@ -16,15 +16,13 @@ use super::plugin_config::FileEntry; use super::plugin_config::PluginConfig; use super::ConfigManagerConfig; use super::DEFAULT_PLUGIN_CONFIG_FILE_NAME; -use c8y_api::smartrest::error::SmartRestSerializerError; use c8y_api::smartrest::smartrest_deserializer::SmartRestConfigDownloadRequest; +use c8y_api::smartrest::smartrest_serializer::fail_operation; +use c8y_api::smartrest::smartrest_serializer::set_operation_executing; +use c8y_api::smartrest::smartrest_serializer::succeed_static_operation; use c8y_api::smartrest::smartrest_serializer::CumulocitySupportedOperations; +use c8y_api::smartrest::smartrest_serializer::OperationStatusMessage; use c8y_api::smartrest::smartrest_serializer::SmartRest; -use c8y_api::smartrest::smartrest_serializer::SmartRestSerializer; -use c8y_api::smartrest::smartrest_serializer::SmartRestSetOperationToExecuting; -use c8y_api::smartrest::smartrest_serializer::SmartRestSetOperationToFailed; -use c8y_api::smartrest::smartrest_serializer::SmartRestSetOperationToSuccessful; -use c8y_api::smartrest::smartrest_serializer::TryIntoOperationStatusMessage; use log::error; use log::info; use log::warn; @@ -79,7 +77,7 @@ impl ConfigDownloadManager { smartrest_request: SmartRestConfigDownloadRequest, message_box: &mut ConfigManagerMessageBox, ) -> Result<(), ConfigManagementError> { - let executing_message = DownloadConfigFileStatusMessage::executing()?; + let executing_message = DownloadConfigFileStatusMessage::executing(); message_box.mqtt_publisher.send(executing_message).await?; let target_config_type = smartrest_request.config_type.clone(); @@ -104,7 +102,7 @@ impl ConfigDownloadManager { Ok(_) => { info!("The configuration download for '{target_config_type}' is successful."); - let successful_message = DownloadConfigFileStatusMessage::successful(None)?; + let successful_message = DownloadConfigFileStatusMessage::successful(None); message_box.mqtt_publisher.send(successful_message).await?; let notification_message = get_file_change_notification_message( @@ -120,7 +118,7 @@ impl ConfigDownloadManager { Err(err) => { error!("The configuration download for '{target_config_type}' failed.",); - let failed_message = DownloadConfigFileStatusMessage::failed(err.to_string())?; + let failed_message = DownloadConfigFileStatusMessage::failed(&err.to_string()); message_box.mqtt_publisher.send(failed_message).await?; Err(err) } @@ -196,7 +194,7 @@ impl ConfigDownloadManager { ConfigOperation::Update, Some(child_id), ActiveOperationState::Pending, - failure_reason, + &failure_reason, message_box, ) .await?; @@ -249,7 +247,7 @@ impl ConfigDownloadManager { if let Some(operation_status) = child_device_payload.status { let current_operation_state = self.active_child_ops.get(&operation_key); if current_operation_state != Some(&ActiveOperationState::Executing) { - let executing_status_payload = DownloadConfigFileStatusMessage::status_executing()?; + let executing_status_payload = DownloadConfigFileStatusMessage::status_executing(); mapped_responses.push(MqttMessage::new(&c8y_child_topic, executing_status_payload)); } @@ -263,7 +261,7 @@ impl ConfigDownloadManager { config_response, ); let successful_status_payload = - DownloadConfigFileStatusMessage::status_successful(None)?; + DownloadConfigFileStatusMessage::status_successful(None); mapped_responses.push(MqttMessage::new( &c8y_child_topic, successful_status_payload, @@ -279,14 +277,13 @@ impl ConfigDownloadManager { ); if let Some(error_message) = &child_device_payload.reason { let failed_status_payload = - DownloadConfigFileStatusMessage::status_failed(error_message.clone())?; + DownloadConfigFileStatusMessage::status_failed(error_message); mapped_responses .push(MqttMessage::new(&c8y_child_topic, failed_status_payload)); } else { - let default_error_message = - String::from("No fail reason provided by child device."); + let default_error_message = "No fail reason provided by child device."; let failed_status_payload = - DownloadConfigFileStatusMessage::status_failed(default_error_message)?; + DownloadConfigFileStatusMessage::status_failed(default_error_message); mapped_responses .push(MqttMessage::new(&c8y_child_topic, failed_status_payload)); } @@ -326,7 +323,7 @@ impl ConfigDownloadManager { ConfigOperation::Update, Some(child_id.clone()), operation_state, - format!("Timeout due to lack of response from child device: {child_id} for config type: {config_type}"), + &format!("Timeout due to lack of response from child device: {child_id} for config type: {config_type}"), message_box, ).await } else { @@ -348,24 +345,20 @@ pub fn get_file_change_notification_message(file_path: &str, config_type: &str) pub struct DownloadConfigFileStatusMessage {} -impl TryIntoOperationStatusMessage for DownloadConfigFileStatusMessage { - fn status_executing() -> Result { - SmartRestSetOperationToExecuting::new(CumulocitySupportedOperations::C8yDownloadConfigFile) - .to_smartrest() +impl DownloadConfigFileStatusMessage { + const OP: CumulocitySupportedOperations = CumulocitySupportedOperations::C8yDownloadConfigFile; +} + +impl OperationStatusMessage for DownloadConfigFileStatusMessage { + fn status_executing() -> SmartRest { + set_operation_executing(Self::OP) } - fn status_successful( - _parameter: Option, - ) -> Result { - SmartRestSetOperationToSuccessful::new(CumulocitySupportedOperations::C8yDownloadConfigFile) - .to_smartrest() + fn status_successful(parameter: Option<&str>) -> SmartRest { + succeed_static_operation(Self::OP, parameter) } - fn status_failed(failure_reason: String) -> Result { - SmartRestSetOperationToFailed::new( - CumulocitySupportedOperations::C8yDownloadConfigFile, - failure_reason, - ) - .to_smartrest() + fn status_failed(failure_reason: &str) -> SmartRest { + fail_operation(Self::OP, failure_reason) } } diff --git a/crates/extensions/c8y_config_manager/src/tests.rs b/crates/extensions/c8y_config_manager/src/tests.rs index 7dc2e1f727a..97154145c66 100644 --- a/crates/extensions/c8y_config_manager/src/tests.rs +++ b/crates/extensions/c8y_config_manager/src/tests.rs @@ -96,7 +96,7 @@ async fn test_config_upload_tedge_device() -> Result<(), DynError> { mqtt_message_box .assert_received([MqttMessage::new( &C8yTopic::SmartRestResponse.to_topic()?, - "501,c8y_UploadConfigFile\n", + "501,c8y_UploadConfigFile", )]) .await; @@ -118,7 +118,7 @@ async fn test_config_upload_tedge_device() -> Result<(), DynError> { mqtt_message_box .assert_received([MqttMessage::new( &C8yTopic::SmartRestResponse.to_topic()?, - "503,c8y_UploadConfigFile,test-url\n", + "503,c8y_UploadConfigFile,test-url", )]) .await; @@ -160,7 +160,7 @@ async fn test_config_download_tedge_device() -> Result<(), DynError> { mqtt_message_box .assert_received([MqttMessage::new( &C8yTopic::SmartRestResponse.to_topic()?, - "501,c8y_DownloadConfigFile\n", + "501,c8y_DownloadConfigFile", )]) .await; @@ -182,7 +182,7 @@ async fn test_config_download_tedge_device() -> Result<(), DynError> { mqtt_message_box .assert_received([MqttMessage::new( &C8yTopic::SmartRestResponse.to_topic()?, - "503,c8y_DownloadConfigFile,\n", + "503,c8y_DownloadConfigFile", )]) .await; @@ -283,7 +283,7 @@ async fn test_child_device_config_upload_executing_response_mapping() -> Result< .with_timeout(TEST_TIMEOUT) .assert_received([MqttMessage::new( &C8yTopic::ChildSmartRestResponse(child_device_id.into()).to_topic()?, - "501,c8y_UploadConfigFile\n", + "501,c8y_UploadConfigFile", )]) .await; @@ -334,11 +334,11 @@ async fn test_child_device_config_upload_failed_response_mapping() -> Result<(), .assert_received([ MqttMessage::new( &C8yTopic::ChildSmartRestResponse(child_device_id.into()).to_topic()?, - "501,c8y_UploadConfigFile\n", + "501,c8y_UploadConfigFile", ), MqttMessage::new( &C8yTopic::ChildSmartRestResponse(child_device_id.into()).to_topic()?, - "502,c8y_UploadConfigFile,\"upload failed\"\n", + "502,c8y_UploadConfigFile,upload failed", ), ]) .await; @@ -386,11 +386,11 @@ async fn test_invalid_config_snapshot_response_child_device() -> Result<(), DynE [ MqttMessage::new( &C8yTopic::ChildSmartRestResponse(child_device_id.into()).to_topic()?, - "501,c8y_UploadConfigFile\n", + "501,c8y_UploadConfigFile", ), MqttMessage::new( &C8yTopic::ChildSmartRestResponse(child_device_id.into()).to_topic()?, - "502,c8y_UploadConfigFile,\"Failed to parse response from child device with: expected value at line 1 column 1\"\n", + "502,c8y_UploadConfigFile,Failed to parse response from child device with: expected value at line 1 column 1", ), ], ) @@ -451,11 +451,11 @@ async fn test_timeout_on_no_config_snapshot_response_child_device() -> Result<() .assert_received([ MqttMessage::new( &C8yTopic::ChildSmartRestResponse(child_device_id.into()).to_topic()?, - "501,c8y_UploadConfigFile\n", + "501,c8y_UploadConfigFile", ), MqttMessage::new( &C8yTopic::ChildSmartRestResponse(child_device_id.into()).to_topic()?, - "502,c8y_UploadConfigFile,\"Timeout due to lack of response from child device: child-aa for config type: file_a\"\n", + "502,c8y_UploadConfigFile,Timeout due to lack of response from child device: child-aa for config type: file_a", ), ], ) @@ -525,11 +525,11 @@ async fn test_child_device_successful_config_snapshot_response_mapping() -> Resu .assert_received([ MqttMessage::new( &C8yTopic::ChildSmartRestResponse(child_device_id.into()).to_topic()?, - "501,c8y_UploadConfigFile\n", + "501,c8y_UploadConfigFile", ), MqttMessage::new( &C8yTopic::ChildSmartRestResponse(child_device_id.into()).to_topic()?, - "503,c8y_UploadConfigFile,test-url\n", + "503,c8y_UploadConfigFile,test-url", ), ]) .await; @@ -599,11 +599,11 @@ async fn test_child_config_snapshot_successful_response_without_uploaded_file_ma .assert_received([ MqttMessage::new( &C8yTopic::ChildSmartRestResponse(child_device_id.into()).to_topic()?, - "501,c8y_UploadConfigFile\n", + "501,c8y_UploadConfigFile", ), MqttMessage::new( &C8yTopic::ChildSmartRestResponse(child_device_id.into()).to_topic()?, - "502,c8y_UploadConfigFile,\"Failed with file not found\"\n", + "502,c8y_UploadConfigFile,Failed with file not found", ), ]) .await; @@ -722,7 +722,7 @@ async fn test_child_device_config_update_executing_response_mapping() -> Result< .with_timeout(TEST_TIMEOUT) .assert_received([MqttMessage::new( &C8yTopic::ChildSmartRestResponse(child_device_id.into()).to_topic()?, - "501,c8y_DownloadConfigFile\n", + "501,c8y_DownloadConfigFile", )]) .await; @@ -772,11 +772,11 @@ async fn test_child_device_config_update_successful_response_mapping() -> Result .assert_received([ MqttMessage::new( &C8yTopic::ChildSmartRestResponse(child_device_id.into()).to_topic()?, - "501,c8y_DownloadConfigFile\n", + "501,c8y_DownloadConfigFile", ), MqttMessage::new( &C8yTopic::ChildSmartRestResponse(child_device_id.into()).to_topic()?, - "503,c8y_DownloadConfigFile,\n", + "503,c8y_DownloadConfigFile", ), ]) .await; @@ -828,11 +828,11 @@ async fn test_child_device_config_update_failed_response_mapping() -> Result<(), .assert_received([ MqttMessage::new( &C8yTopic::ChildSmartRestResponse(child_device_id.into()).to_topic()?, - "501,c8y_DownloadConfigFile\n", + "501,c8y_DownloadConfigFile", ), MqttMessage::new( &C8yTopic::ChildSmartRestResponse(child_device_id.into()).to_topic()?, - "502,c8y_DownloadConfigFile,\"download failed\"\n", + "502,c8y_DownloadConfigFile,download failed", ), ]) .await; @@ -895,11 +895,11 @@ async fn test_child_device_config_download_fail_with_broken_url() -> Result<(), .with_timeout(TEST_TIMEOUT) .assert_received([MqttMessage::new( &C8yTopic::ChildSmartRestResponse(child_device_id.into()).to_topic()?, - "501,c8y_DownloadConfigFile\n", + "501,c8y_DownloadConfigFile", ), MqttMessage::new( &C8yTopic::ChildSmartRestResponse(child_device_id.into()).to_topic()?, - "502,c8y_DownloadConfigFile,\"Downloading the config file update from bad-url failed with Failed with file not found\"\n", + "502,c8y_DownloadConfigFile,Downloading the config file update from bad-url failed with Failed with file not found", ), ], ) @@ -954,11 +954,11 @@ async fn test_multiline_smartrest_requests() -> Result<(), DynError> { .assert_received([ MqttMessage::new( &C8yTopic::SmartRestResponse.to_topic()?, - "501,c8y_UploadConfigFile\n", + "501,c8y_UploadConfigFile", ), MqttMessage::new( &C8yTopic::SmartRestResponse.to_topic()?, - "503,c8y_UploadConfigFile,test-url\n", + "503,c8y_UploadConfigFile,test-url", ), ]) .await; diff --git a/crates/extensions/c8y_config_manager/src/upload.rs b/crates/extensions/c8y_config_manager/src/upload.rs index d63b1d2a4f3..4460c9e834d 100644 --- a/crates/extensions/c8y_config_manager/src/upload.rs +++ b/crates/extensions/c8y_config_manager/src/upload.rs @@ -14,15 +14,13 @@ use super::child_device::ConfigOperationResponse; use super::error::ConfigManagementError; use super::plugin_config::PluginConfig; use super::ConfigManagerConfig; -use c8y_api::smartrest::error::SmartRestSerializerError; use c8y_api::smartrest::smartrest_deserializer::SmartRestConfigUploadRequest; +use c8y_api::smartrest::smartrest_serializer::fail_operation; +use c8y_api::smartrest::smartrest_serializer::set_operation_executing; +use c8y_api::smartrest::smartrest_serializer::succeed_static_operation; use c8y_api::smartrest::smartrest_serializer::CumulocitySupportedOperations; +use c8y_api::smartrest::smartrest_serializer::OperationStatusMessage; use c8y_api::smartrest::smartrest_serializer::SmartRest; -use c8y_api::smartrest::smartrest_serializer::SmartRestSerializer; -use c8y_api::smartrest::smartrest_serializer::SmartRestSetOperationToExecuting; -use c8y_api::smartrest::smartrest_serializer::SmartRestSetOperationToFailed; -use c8y_api::smartrest::smartrest_serializer::SmartRestSetOperationToSuccessful; -use c8y_api::smartrest::smartrest_serializer::TryIntoOperationStatusMessage; use log::error; use log::info; use log::warn; @@ -78,7 +76,7 @@ impl ConfigUploadManager { message_box: &mut ConfigManagerMessageBox, ) -> Result<(), ConfigManagementError> { // set config upload request to executing - let msg = UploadConfigFileStatusMessage::executing()?; + let msg = UploadConfigFileStatusMessage::executing(); message_box.mqtt_publisher.send(msg).await?; let plugin_config = PluginConfig::new(&self.config.plugin_config_path); @@ -105,13 +103,13 @@ impl ConfigUploadManager { info!("The configuration upload for '{target_config_type}' is successful."); let successful_message = - UploadConfigFileStatusMessage::successful(Some(upload_event_url))?; + UploadConfigFileStatusMessage::successful(Some(&upload_event_url)); message_box.mqtt_publisher.send(successful_message).await?; } Err(err) => { error!("The configuration upload for '{target_config_type}' failed.",); - let failed_message = UploadConfigFileStatusMessage::failed(err.to_string())?; + let failed_message = UploadConfigFileStatusMessage::failed(&err.to_string()); message_box.mqtt_publisher.send(failed_message).await?; } } @@ -195,7 +193,7 @@ impl ConfigUploadManager { if let Some(operation_status) = payload.status { let current_operation_state = self.active_child_ops.get(&operation_key); if current_operation_state != Some(&ActiveOperationState::Executing) { - let executing_status_payload = UploadConfigFileStatusMessage::status_executing()?; + let executing_status_payload = UploadConfigFileStatusMessage::status_executing(); mapped_responses.push(MqttMessage::new(&c8y_child_topic, executing_status_payload)); } @@ -213,7 +211,7 @@ impl ConfigUploadManager { Ok(message) => mapped_responses.push(message), Err(err) => { let failed_status_payload = - UploadConfigFileStatusMessage::status_failed(err.to_string())?; + UploadConfigFileStatusMessage::status_failed(&err.to_string()); mapped_responses .push(MqttMessage::new(&c8y_child_topic, failed_status_payload)); } @@ -223,16 +221,14 @@ impl ConfigUploadManager { self.active_child_ops.remove(&operation_key); if let Some(error_message) = &payload.reason { - let failed_status_payload = UploadConfigFileStatusMessage::status_failed( - error_message.to_string(), - )?; + let failed_status_payload = + UploadConfigFileStatusMessage::status_failed(error_message); mapped_responses .push(MqttMessage::new(&c8y_child_topic, failed_status_payload)); } else { - let default_error_message = - String::from("No failure reason provided by child device."); + let default_error_message = "No failure reason provided by child device."; let failed_status_payload = - UploadConfigFileStatusMessage::status_failed(default_error_message)?; + UploadConfigFileStatusMessage::status_failed(default_error_message); mapped_responses .push(MqttMessage::new(&c8y_child_topic, failed_status_payload)); } @@ -347,7 +343,7 @@ impl ConfigUploadManager { info!("Marking the c8y_UploadConfigFile operation as successful with the Cumulocity URL for the uploaded file: {c8y_upload_event_url}"); let successful_status_payload = - UploadConfigFileStatusMessage::status_successful(Some(c8y_upload_event_url))?; + UploadConfigFileStatusMessage::status_successful(Some(&c8y_upload_event_url)); let message = MqttMessage::new(&c8y_child_topic, successful_status_payload); Ok(message) @@ -385,7 +381,7 @@ impl ConfigUploadManager { ConfigOperation::Snapshot, Some(child_id.clone()), operation_state, - format!("Timeout due to lack of response from child device: {child_id} for config type: {config_type}"), + &format!("Timeout due to lack of response from child device: {child_id} for config type: {config_type}"), message_box, ).await } else { @@ -397,29 +393,26 @@ impl ConfigUploadManager { pub struct UploadConfigFileStatusMessage {} -impl TryIntoOperationStatusMessage for UploadConfigFileStatusMessage { +impl UploadConfigFileStatusMessage { + const OP: CumulocitySupportedOperations = CumulocitySupportedOperations::C8yUploadConfigFile; +} + +impl OperationStatusMessage for UploadConfigFileStatusMessage { // returns a c8y message specifying to set the upload config file operation status to executing. // example message: '501,c8y_UploadConfigFile' - fn status_executing() -> Result { - SmartRestSetOperationToExecuting::new(CumulocitySupportedOperations::C8yUploadConfigFile) - .to_smartrest() + fn status_executing() -> SmartRest { + set_operation_executing(Self::OP) } // returns a c8y SmartREST message indicating the success of the upload config file operation. // example message: '503,c8y_UploadConfigFile,https://{c8y.url}/etc...' - fn status_successful(parameter: Option) -> Result { - SmartRestSetOperationToSuccessful::new(CumulocitySupportedOperations::C8yUploadConfigFile) - .with_response_parameter(parameter.unwrap_or_default().as_str()) - .to_smartrest() + fn status_successful(parameter: Option<&str>) -> SmartRest { + succeed_static_operation(Self::OP, parameter) } // returns a c8y SmartREST message indicating the failure of the upload config file operation. // example message: '502,c8y_UploadConfigFile,"failure reason"' - fn status_failed(failure_reason: String) -> Result { - SmartRestSetOperationToFailed::new( - CumulocitySupportedOperations::C8yUploadConfigFile, - failure_reason, - ) - .to_smartrest() + fn status_failed(failure_reason: &str) -> SmartRest { + fail_operation(Self::OP, failure_reason) } } diff --git a/crates/extensions/c8y_firmware_manager/src/actor.rs b/crates/extensions/c8y_firmware_manager/src/actor.rs index 3f45a54b0fe..ff0f8f7e7c5 100644 --- a/crates/extensions/c8y_firmware_manager/src/actor.rs +++ b/crates/extensions/c8y_firmware_manager/src/actor.rs @@ -3,7 +3,7 @@ use c8y_api::smartrest::message::collect_smartrest_messages; use c8y_api::smartrest::message::get_smartrest_template_id; use c8y_api::smartrest::smartrest_deserializer::SmartRestFirmwareRequest; use c8y_api::smartrest::smartrest_deserializer::SmartRestRequestGeneric; -use c8y_api::smartrest::smartrest_serializer::TryIntoOperationStatusMessage; +use c8y_api::smartrest::smartrest_serializer::OperationStatusMessage; use c8y_api::smartrest::topic::C8yTopic; use c8y_http_proxy::credentials::JwtRetriever; use camino::Utf8PathBuf; @@ -641,7 +641,7 @@ impl FirmwareManagerActor { ); let executing_msg = MqttMessage::new( &c8y_child_topic, - DownloadFirmwareStatusMessage::status_executing()?, + DownloadFirmwareStatusMessage::status_executing(), ); self.message_box.mqtt_publisher.send(executing_msg).await?; Ok(()) @@ -656,7 +656,7 @@ impl FirmwareManagerActor { ); let successful_msg = MqttMessage::new( &c8y_child_topic, - DownloadFirmwareStatusMessage::status_successful(None)?, + DownloadFirmwareStatusMessage::status_successful(None), ); self.message_box.mqtt_publisher.send(successful_msg).await?; Ok(()) @@ -672,7 +672,7 @@ impl FirmwareManagerActor { ); let failed_msg = MqttMessage::new( &c8y_child_topic, - DownloadFirmwareStatusMessage::status_failed(failure_reason.to_string())?, + DownloadFirmwareStatusMessage::status_failed(failure_reason), ); self.message_box.mqtt_publisher.send(failed_msg).await?; Ok(()) diff --git a/crates/extensions/c8y_firmware_manager/src/message.rs b/crates/extensions/c8y_firmware_manager/src/message.rs index da1be363cf2..56a44d8721b 100644 --- a/crates/extensions/c8y_firmware_manager/src/message.rs +++ b/crates/extensions/c8y_firmware_manager/src/message.rs @@ -1,14 +1,13 @@ use crate::error::FirmwareManagementError; use crate::operation::FirmwareOperationEntry; -use c8y_api::smartrest::error::SmartRestSerializerError; +use c8y_api::smartrest::smartrest_serializer::fail_operation; +use c8y_api::smartrest::smartrest_serializer::set_operation_executing; +use c8y_api::smartrest::smartrest_serializer::succeed_static_operation; use c8y_api::smartrest::smartrest_serializer::CumulocitySupportedOperations; +use c8y_api::smartrest::smartrest_serializer::CumulocitySupportedOperations::C8yFirmware; +use c8y_api::smartrest::smartrest_serializer::OperationStatusMessage; use c8y_api::smartrest::smartrest_serializer::SmartRest; -use c8y_api::smartrest::smartrest_serializer::SmartRestSerializer; -use c8y_api::smartrest::smartrest_serializer::SmartRestSetOperationToExecuting; -use c8y_api::smartrest::smartrest_serializer::SmartRestSetOperationToFailed; -use c8y_api::smartrest::smartrest_serializer::SmartRestSetOperationToSuccessful; -use c8y_api::smartrest::smartrest_serializer::TryIntoOperationStatusMessage; use tedge_api::topic::get_child_id_from_child_topic; use tedge_api::OperationStatus; use tedge_mqtt_ext::MqttMessage; @@ -115,25 +114,21 @@ impl TryFrom<&MqttMessage> for FirmwareOperationResponse { pub struct DownloadFirmwareStatusMessage {} -impl TryIntoOperationStatusMessage for DownloadFirmwareStatusMessage { - fn status_executing() -> Result { - SmartRestSetOperationToExecuting::new(CumulocitySupportedOperations::C8yFirmware) - .to_smartrest() +impl DownloadFirmwareStatusMessage { + const OP: CumulocitySupportedOperations = C8yFirmware; +} + +impl OperationStatusMessage for DownloadFirmwareStatusMessage { + fn status_executing() -> SmartRest { + set_operation_executing(Self::OP) } - fn status_successful( - _parameter: Option, - ) -> Result { - SmartRestSetOperationToSuccessful::new(CumulocitySupportedOperations::C8yFirmware) - .to_smartrest() + fn status_successful(parameter: Option<&str>) -> SmartRest { + succeed_static_operation(Self::OP, parameter) } - fn status_failed(failure_reason: String) -> Result { - SmartRestSetOperationToFailed::new( - CumulocitySupportedOperations::C8yFirmware, - failure_reason, - ) - .to_smartrest() + fn status_failed(failure_reason: &str) -> SmartRest { + fail_operation(Self::OP, failure_reason) } } diff --git a/crates/extensions/c8y_firmware_manager/src/tests.rs b/crates/extensions/c8y_firmware_manager/src/tests.rs index d54ad0c32a5..303b0c65cbd 100644 --- a/crates/extensions/c8y_firmware_manager/src/tests.rs +++ b/crates/extensions/c8y_firmware_manager/src/tests.rs @@ -274,11 +274,11 @@ async fn handle_request_child_device_with_failed_download() -> Result<(), DynErr .assert_received([ MqttMessage::new( &Topic::new_unchecked(C8Y_CHILD_PUBLISH_TOPIC_NAME), - "501,c8y_Firmware\n", + "501,c8y_Firmware", ), MqttMessage::new( &Topic::new_unchecked(C8Y_CHILD_PUBLISH_TOPIC_NAME), - format!("502,c8y_Firmware,\"Download from {DOWNLOAD_URL} failed with fail\"\n"), + format!("502,c8y_Firmware,Download from {DOWNLOAD_URL} failed with fail"), ), ]) .await; @@ -360,7 +360,7 @@ async fn handle_response_successful_child_device() -> Result<(), DynError> { .assert_received([ MqttMessage::new( &Topic::new_unchecked(C8Y_CHILD_PUBLISH_TOPIC_NAME), - "501,c8y_Firmware\n", + "501,c8y_Firmware", ), MqttMessage::new( &Topic::new_unchecked(C8Y_CHILD_PUBLISH_TOPIC_NAME), @@ -368,7 +368,7 @@ async fn handle_response_successful_child_device() -> Result<(), DynError> { ), MqttMessage::new( &Topic::new_unchecked(C8Y_CHILD_PUBLISH_TOPIC_NAME), - "503,c8y_Firmware,\n", + "503,c8y_Firmware", ), ]) .await; @@ -404,7 +404,7 @@ async fn handle_response_executing_and_failed_child_device() -> Result<(), DynEr mqtt_message_box .assert_received([MqttMessage::new( &Topic::new_unchecked(C8Y_CHILD_PUBLISH_TOPIC_NAME), - "501,c8y_Firmware\n", + "501,c8y_Firmware", )]) .await; @@ -415,7 +415,7 @@ async fn handle_response_executing_and_failed_child_device() -> Result<(), DynEr mqtt_message_box .assert_received([MqttMessage::new( &Topic::new_unchecked(C8Y_CHILD_PUBLISH_TOPIC_NAME), - "502,c8y_Firmware,\"No failure reason provided by child device.\"\n", + "502,c8y_Firmware,No failure reason provided by child device.", )]) .await; @@ -524,11 +524,11 @@ async fn handle_request_timeout_child_device() -> Result<(), DynError> { .assert_received([ MqttMessage::new( &Topic::new_unchecked(C8Y_CHILD_PUBLISH_TOPIC_NAME), - "501,c8y_Firmware\n", + "501,c8y_Firmware", ), MqttMessage::new( &Topic::new_unchecked(C8Y_CHILD_PUBLISH_TOPIC_NAME), - format!("502,c8y_Firmware,\"{expected_failure_text}\"\n"), + format!("502,c8y_Firmware,{expected_failure_text}"), ), ]) .await; @@ -572,7 +572,7 @@ async fn handle_child_response_while_busy_downloading() -> Result<(), DynError> .assert_received([ MqttMessage::new( &Topic::new_unchecked(C8Y_CHILD_PUBLISH_TOPIC_NAME), - "501,c8y_Firmware\n", + "501,c8y_Firmware", ), MqttMessage::new( &Topic::new_unchecked(C8Y_CHILD_PUBLISH_TOPIC_NAME), @@ -580,7 +580,7 @@ async fn handle_child_response_while_busy_downloading() -> Result<(), DynError> ), MqttMessage::new( &Topic::new_unchecked(C8Y_CHILD_PUBLISH_TOPIC_NAME), - "503,c8y_Firmware,\n", + "503,c8y_Firmware", ), ]) .await; diff --git a/crates/extensions/c8y_log_manager/src/actor.rs b/crates/extensions/c8y_log_manager/src/actor.rs index 33f0b3f0ad8..7036545be02 100644 --- a/crates/extensions/c8y_log_manager/src/actor.rs +++ b/crates/extensions/c8y_log_manager/src/actor.rs @@ -3,12 +3,11 @@ use async_trait::async_trait; use c8y_api::smartrest::message::get_smartrest_device_id; use c8y_api::smartrest::smartrest_deserializer::SmartRestLogRequest; use c8y_api::smartrest::smartrest_deserializer::SmartRestRequestGeneric; +use c8y_api::smartrest::smartrest_serializer::fail_operation; +use c8y_api::smartrest::smartrest_serializer::set_operation_executing; +use c8y_api::smartrest::smartrest_serializer::succeed_static_operation; use c8y_api::smartrest::smartrest_serializer::CumulocitySupportedOperations; -use c8y_api::smartrest::smartrest_serializer::SmartRestSerializer; -use c8y_api::smartrest::smartrest_serializer::SmartRestSetOperationToExecuting; -use c8y_api::smartrest::smartrest_serializer::SmartRestSetOperationToFailed; -use c8y_api::smartrest::smartrest_serializer::SmartRestSetOperationToSuccessful; -use c8y_api::smartrest::smartrest_serializer::TryIntoOperationStatusMessage; +use c8y_api::smartrest::smartrest_serializer::OperationStatusMessage; use c8y_api::smartrest::topic::C8yTopic; use c8y_api::utils::bridge::is_c8y_bridge_up; use c8y_http_proxy::handle::C8YHttpProxy; @@ -113,7 +112,7 @@ impl LogManagerActor { &mut self, smartrest_request: &SmartRestLogRequest, ) -> Result<(), anyhow::Error> { - let executing = LogfileRequest::executing()?; + let executing = LogfileRequest::executing(); self.mqtt_publisher.send(executing).await?; let log_path = log_manager::new_read_logs( @@ -136,7 +135,7 @@ impl LogManagerActor { ) .await?; - let successful = LogfileRequest::successful(Some(upload_event_url))?; + let successful = LogfileRequest::successful(Some(&upload_event_url)); self.mqtt_publisher.send(successful).await?; std::fs::remove_file(log_path)?; @@ -155,7 +154,7 @@ impl LogManagerActor { Ok(()) => Ok(()), Err(error) => { let error_message = format!("Handling of operation failed with {}", error); - let failed_msg = LogfileRequest::failed(error_message)?; + let failed_msg = LogfileRequest::failed(&error_message); self.mqtt_publisher.send(failed_msg).await?; error!( "Handling of operation for log type {} failed with: {}", @@ -242,40 +241,24 @@ impl Actor for LogManagerActor { pub struct LogfileRequest {} -impl TryIntoOperationStatusMessage for LogfileRequest { +impl LogfileRequest { + const OP: CumulocitySupportedOperations = CumulocitySupportedOperations::C8yLogFileRequest; +} + +impl OperationStatusMessage for LogfileRequest { /// returns a c8y message specifying to set log status to executing. /// /// example message: '501,c8y_LogfileRequest' - fn status_executing() -> Result< - c8y_api::smartrest::smartrest_serializer::SmartRest, - c8y_api::smartrest::error::SmartRestSerializerError, - > { - SmartRestSetOperationToExecuting::new(CumulocitySupportedOperations::C8yLogFileRequest) - .to_smartrest() + fn status_executing() -> String { + set_operation_executing(Self::OP) } - fn status_successful( - parameter: Option, - ) -> Result< - c8y_api::smartrest::smartrest_serializer::SmartRest, - c8y_api::smartrest::error::SmartRestSerializerError, - > { - SmartRestSetOperationToSuccessful::new(CumulocitySupportedOperations::C8yLogFileRequest) - .with_response_parameter(¶meter.unwrap()) - .to_smartrest() + fn status_successful(parameter: Option<&str>) -> String { + succeed_static_operation(Self::OP, parameter) } - fn status_failed( - failure_reason: String, - ) -> Result< - c8y_api::smartrest::smartrest_serializer::SmartRest, - c8y_api::smartrest::error::SmartRestSerializerError, - > { - SmartRestSetOperationToFailed::new( - CumulocitySupportedOperations::C8yLogFileRequest, - failure_reason, - ) - .to_smartrest() + fn status_failed(failure_reason: &str) -> String { + fail_operation(Self::OP, failure_reason) } } @@ -467,7 +450,7 @@ mod tests { // The log manager notifies C8Y that the request has been received and is processed assert_eq!( mqtt.recv().await, - Some(MqttMessage::new(&c8y_s_us, "501,c8y_LogfileRequest\n")) + Some(MqttMessage::new(&c8y_s_us, "501,c8y_LogfileRequest")) ); // Then uploads the requested content over HTTP @@ -487,10 +470,7 @@ mod tests { // Finally, the log manager uses the event id to notify C8Y that the request has been fully processed assert_eq!( mqtt.recv().await, - Some(MqttMessage::new( - &c8y_s_us, - "503,c8y_LogfileRequest,12345\n" - )) + Some(MqttMessage::new(&c8y_s_us, "503,c8y_LogfileRequest,12345")) ); Ok(()) diff --git a/crates/extensions/c8y_mapper_ext/src/actor.rs b/crates/extensions/c8y_mapper_ext/src/actor.rs index 0c246d8b932..c49aa56e074 100644 --- a/crates/extensions/c8y_mapper_ext/src/actor.rs +++ b/crates/extensions/c8y_mapper_ext/src/actor.rs @@ -3,13 +3,11 @@ use super::converter::CumulocityConverter; use super::dynamic_discovery::process_inotify_events; use crate::converter::FtsDownloadOperationType; use async_trait::async_trait; -use c8y_api::smartrest::error::SmartRestSerializerError; use c8y_api::smartrest::smartrest_deserializer::SmartRestOperationVariant; +use c8y_api::smartrest::smartrest_serializer::fail_operation; +use c8y_api::smartrest::smartrest_serializer::succeed_static_operation; use c8y_api::smartrest::smartrest_serializer::CumulocitySupportedOperations; use c8y_api::smartrest::smartrest_serializer::SmartRest; -use c8y_api::smartrest::smartrest_serializer::SmartRestSerializer; -use c8y_api::smartrest::smartrest_serializer::SmartRestSetOperationToFailed; -use c8y_api::smartrest::smartrest_serializer::SmartRestSetOperationToSuccessful; use c8y_auth_proxy::url::ProxyUrlGenerator; use c8y_http_proxy::handle::C8YHttpProxy; use c8y_http_proxy::messages::C8YRestRequest; @@ -242,12 +240,12 @@ impl C8yMapperActor { match self.converter.pending_upload_operations.remove(&cmd_id) { None => error!("Received an upload result for the unknown command ID: {cmd_id}"), Some(queued_data) => { - let serialize_result = match queued_data.operation { + let payload = match queued_data.operation { CumulocitySupportedOperations::C8yLogFileRequest | CumulocitySupportedOperations::C8yUploadConfigFile => self .get_smartrest_response_for_upload_result( upload_result, - queued_data.c8y_binary_url, + &queued_data.c8y_binary_url, queued_data.operation, ), other_type => { @@ -256,20 +254,12 @@ impl C8yMapperActor { } }; - match serialize_result { - Ok(sr_payload) => { - let c8y_notification = - Message::new(&queued_data.smartrest_topic, sr_payload); - let clear_local_cmd = Message::new(&queued_data.clear_cmd_topic, "") - .with_retain() - .with_qos(QoS::AtLeastOnce); - for converted_message in [c8y_notification, clear_local_cmd] { - self.mqtt_publisher.send(converted_message).await? - } - } - Err(err) => { - error!("Error occurred while processing an upload result. {err}") - } + let c8y_notification = Message::new(&queued_data.smartrest_topic, payload); + let clear_local_cmd = Message::new(&queued_data.clear_cmd_topic, "") + .with_retain() + .with_qos(QoS::AtLeastOnce); + for converted_message in [c8y_notification, clear_local_cmd] { + self.mqtt_publisher.send(converted_message).await? } } }; @@ -280,17 +270,12 @@ impl C8yMapperActor { fn get_smartrest_response_for_upload_result( &self, upload_result: UploadResult, - binary_url: String, + binary_url: &str, operation: CumulocitySupportedOperations, - ) -> Result { + ) -> SmartRest { match upload_result { - Ok(_) => SmartRestSetOperationToSuccessful::new(operation) - .with_response_parameter(&binary_url) - .to_smartrest(), - Err(err) => { - SmartRestSetOperationToFailed::new(operation, format!("Upload failed with {}", err)) - .to_smartrest() - } + Ok(_) => succeed_static_operation(operation, Some(binary_url)), + Err(err) => fail_operation(operation, &format!("Upload failed with {err}")), } } diff --git a/crates/extensions/c8y_mapper_ext/src/config_operations.rs b/crates/extensions/c8y_mapper_ext/src/config_operations.rs index f24fce5b7f9..017d0752a2a 100644 --- a/crates/extensions/c8y_mapper_ext/src/config_operations.rs +++ b/crates/extensions/c8y_mapper_ext/src/config_operations.rs @@ -10,11 +10,10 @@ use c8y_api::smartrest::smartrest_deserializer::SmartRestConfigDownloadRequest; use c8y_api::smartrest::smartrest_deserializer::SmartRestConfigUploadRequest; use c8y_api::smartrest::smartrest_deserializer::SmartRestOperationVariant; use c8y_api::smartrest::smartrest_deserializer::SmartRestRequestGeneric; +use c8y_api::smartrest::smartrest_serializer::fail_operation; +use c8y_api::smartrest::smartrest_serializer::set_operation_executing; +use c8y_api::smartrest::smartrest_serializer::succeed_operation_no_payload; use c8y_api::smartrest::smartrest_serializer::CumulocitySupportedOperations; -use c8y_api::smartrest::smartrest_serializer::SmartRestSerializer; -use c8y_api::smartrest::smartrest_serializer::SmartRestSetOperationToExecuting; -use c8y_api::smartrest::smartrest_serializer::SmartRestSetOperationToFailed; -use c8y_api::smartrest::smartrest_serializer::SmartRestSetOperationToSuccessful; use c8y_http_proxy::messages::CreateEvent; use camino::Utf8PathBuf; use sha256::digest; @@ -131,12 +130,10 @@ impl CumulocityConverter { let payload = message.payload_str()?; let response = &ConfigSnapshotCmdPayload::from_json(payload)?; - let messages = match response.status { + let messages = match &response.status { CommandStatus::Executing => { - let smartrest_operation_status = SmartRestSetOperationToExecuting::new( - CumulocitySupportedOperations::C8yUploadConfigFile, - ) - .to_smartrest()?; + let smartrest_operation_status = + set_operation_executing(CumulocitySupportedOperations::C8yUploadConfigFile); vec![Message::new(&smartrest_topic, smartrest_operation_status)] } CommandStatus::Successful => { @@ -177,12 +174,9 @@ impl CumulocityConverter { vec![] // No mqtt message can be published in this state } - CommandStatus::Failed { ref reason } => { - let smartrest_operation_status = SmartRestSetOperationToFailed::new( - CumulocitySupportedOperations::C8yUploadConfigFile, - reason.clone(), - ) - .to_smartrest()?; + CommandStatus::Failed { reason } => { + let smartrest_operation_status = + fail_operation(CumulocitySupportedOperations::C8yUploadConfigFile, reason); let c8y_notification = Message::new(&smartrest_topic, smartrest_operation_status); let clear_local_cmd = Message::new(&message.topic, "") .with_retain() @@ -213,11 +207,11 @@ impl CumulocityConverter { let download = match download_result { Err(err) => { - let smartrest_error = SmartRestSetOperationToFailed::new( + let smartrest_error = + fail_operation( CumulocitySupportedOperations::C8yUploadConfigFile, - format!("tedge-mapper-c8y failed to download configuration snapshot from file-transfer service: {err}"), - ) - .to_smartrest()?; + &format!("tedge-mapper-c8y failed to download configuration snapshot from file-transfer service: {err}"), + ); let c8y_notification = Message::new(&smartrest_topic, smartrest_error); let clean_operation = Message::new(&fts_download.message.topic, "") @@ -369,18 +363,15 @@ impl CumulocityConverter { } Err(download_err) => { let sm_topic = self.smartrest_publish_topic_for_entity(&target.topic_id)?; - let smartrest_executing = SmartRestSetOperationToExecuting::new( + let smartrest_executing = + set_operation_executing(CumulocitySupportedOperations::C8yDownloadConfigFile); + let smartrest_failed = fail_operation( CumulocitySupportedOperations::C8yDownloadConfigFile, - ) - .to_smartrest()?; - let smartrest_failed = SmartRestSetOperationToFailed::new( - CumulocitySupportedOperations::C8yDownloadConfigFile, - format!( + &format!( "Download from {} failed with {}", smartrest.url, download_err ), - ) - .to_smartrest()?; + ); Ok(vec![ Message::new(&sm_topic, smartrest_executing), @@ -411,19 +402,17 @@ impl CumulocityConverter { let payload = message.payload_str()?; let response = &ConfigUpdateCmdPayload::from_json(payload)?; - let messages = match response.status { + let messages = match &response.status { CommandStatus::Executing => { - let smartrest_operation_status = SmartRestSetOperationToExecuting::new( - CumulocitySupportedOperations::C8yDownloadConfigFile, - ) - .to_smartrest()?; + let smartrest_operation_status = + set_operation_executing(CumulocitySupportedOperations::C8yDownloadConfigFile); + vec![Message::new(&sm_topic, smartrest_operation_status)] } CommandStatus::Successful => { - let smartrest_operation_status = SmartRestSetOperationToSuccessful::new( + let smartrest_operation_status = succeed_operation_no_payload( CumulocitySupportedOperations::C8yDownloadConfigFile, - ) - .to_smartrest()?; + ); let c8y_notification = Message::new(&sm_topic, smartrest_operation_status); let clear_local_cmd = Message::new(&message.topic, "") .with_retain() @@ -433,12 +422,9 @@ impl CumulocityConverter { vec![c8y_notification, clear_local_cmd] } - CommandStatus::Failed { ref reason } => { - let smartrest_operation_status = SmartRestSetOperationToFailed::new( - CumulocitySupportedOperations::C8yDownloadConfigFile, - reason.clone(), - ) - .to_smartrest()?; + CommandStatus::Failed { reason } => { + let smartrest_operation_status = + fail_operation(CumulocitySupportedOperations::C8yDownloadConfigFile, reason); let c8y_notification = Message::new(&sm_topic, smartrest_operation_status); let clear_local_cmd = Message::new(&message.topic, "") .with_retain() @@ -869,10 +855,7 @@ mod tests { // Expect `502` smartrest message on `c8y/s/us`. assert_received_contains_str( &mut mqtt, - [( - "c8y/s/us", - "502,c8y_UploadConfigFile,\"Something went wrong\"", - )], + [("c8y/s/us", "502,c8y_UploadConfigFile,Something went wrong")], ) .await; } @@ -931,7 +914,7 @@ mod tests { &mut mqtt, [( "c8y/s/us/child1", - "502,c8y_UploadConfigFile,\"Something went wrong\"", + "502,c8y_UploadConfigFile,Something went wrong", )], ) .await; @@ -1280,7 +1263,7 @@ mod tests { &mut mqtt, [( "c8y/s/us", - "502,c8y_DownloadConfigFile,\"Something went wrong\"", + "502,c8y_DownloadConfigFile,Something went wrong", )], ) .await; @@ -1351,7 +1334,7 @@ mod tests { &mut mqtt, [( "c8y/s/us/child1", - "502,c8y_DownloadConfigFile,\"Something went wrong\"", + "502,c8y_DownloadConfigFile,Something went wrong", )], ) .await; diff --git a/crates/extensions/c8y_mapper_ext/src/converter.rs b/crates/extensions/c8y_mapper_ext/src/converter.rs index 5fdad972ae4..cfbc3449b92 100644 --- a/crates/extensions/c8y_mapper_ext/src/converter.rs +++ b/crates/extensions/c8y_mapper_ext/src/converter.rs @@ -22,7 +22,7 @@ use c8y_api::smartrest::message::collect_smartrest_messages; use c8y_api::smartrest::message::get_failure_reason_for_smartrest; use c8y_api::smartrest::message::get_smartrest_device_id; use c8y_api::smartrest::message::get_smartrest_template_id; -use c8y_api::smartrest::message::sanitize_for_smartrest; +use c8y_api::smartrest::message::sanitize_bytes_for_smartrest; use c8y_api::smartrest::message::MAX_PAYLOAD_LIMIT_IN_BYTES; use c8y_api::smartrest::operations::get_child_ops; use c8y_api::smartrest::operations::get_operations; @@ -33,12 +33,14 @@ use c8y_api::smartrest::smartrest_deserializer::SmartRestOperationVariant; use c8y_api::smartrest::smartrest_deserializer::SmartRestRequestGeneric; use c8y_api::smartrest::smartrest_deserializer::SmartRestRestartRequest; use c8y_api::smartrest::smartrest_deserializer::SmartRestUpdateSoftware; +use c8y_api::smartrest::smartrest_serializer::fail_operation; +use c8y_api::smartrest::smartrest_serializer::request_pending_operations; +use c8y_api::smartrest::smartrest_serializer::set_operation_executing; +use c8y_api::smartrest::smartrest_serializer::succeed_operation; +use c8y_api::smartrest::smartrest_serializer::succeed_operation_no_payload; use c8y_api::smartrest::smartrest_serializer::CumulocitySupportedOperations; -use c8y_api::smartrest::smartrest_serializer::SmartRestGetPendingOperations; -use c8y_api::smartrest::smartrest_serializer::SmartRestSerializer; -use c8y_api::smartrest::smartrest_serializer::SmartRestSetOperationToExecuting; -use c8y_api::smartrest::smartrest_serializer::SmartRestSetOperationToFailed; -use c8y_api::smartrest::smartrest_serializer::SmartRestSetOperationToSuccessful; +use c8y_api::smartrest::smartrest_serializer::EmbeddedCsv; +use c8y_api::smartrest::smartrest_serializer::TextOrCsv; use c8y_api::smartrest::topic::publish_topic_from_ancestors; use c8y_api::smartrest::topic::C8yTopic; use c8y_api::smartrest::topic::SMARTREST_PUBLISH_TOPIC; @@ -710,10 +712,10 @@ impl CumulocityConverter { self.execute_operation( payload, command.as_str(), - &operation.name, operation.result_format(), operation.graceful_timeout(), operation.forceful_timeout(), + operation.name, ) .await?; } @@ -726,10 +728,10 @@ impl CumulocityConverter { &self, payload: &str, command: &str, - operation_name: &str, result_format: ResultFormat, graceful_timeout: Duration, forceful_timeout: Duration, + operation_name: String, ) -> Result<(), CumulocityMapperError> { let command = command.to_owned(); let payload = payload.to_string(); @@ -755,15 +757,16 @@ impl CumulocityConverter { match maybe_child_process { Ok(child_process) => { - let op_name = operation_name.to_string(); + let op_name = operation_name.to_owned(); let mut mqtt_publisher = self.mqtt_publisher.clone(); tokio::spawn(async move { + let op_name = op_name.as_str(); let logger = log_file.buffer(); // mqtt client publishes executing let topic = C8yTopic::SmartRestResponse.to_topic().unwrap(); - let executing_str = format!("501,{op_name}"); + let executing_str = set_operation_executing(op_name); mqtt_publisher .send(Message::new(&topic, executing_str.as_str())) .await @@ -779,36 +782,44 @@ impl CumulocityConverter { { match output.status.code() { Some(0) => { - let sanitized_stdout = sanitize_for_smartrest( - output.stdout, + let sanitized_stdout = sanitize_bytes_for_smartrest( + &output.stdout, MAX_PAYLOAD_LIMIT_IN_BYTES, ); - let successful_str = match result_format { - ResultFormat::Text => { - format!("503,{op_name},\"{sanitized_stdout}\"") - } - ResultFormat::Csv => { - format!("503,{op_name},{sanitized_stdout}") - } + let result = match result_format { + ResultFormat::Text => TextOrCsv::Text(sanitized_stdout), + ResultFormat::Csv => EmbeddedCsv::new(sanitized_stdout).into(), }; - mqtt_publisher.send(Message::new(&topic, successful_str.as_str())).await - .unwrap_or_else(|err| { - error!("Failed to publish a message: {successful_str}. Error: {err}") - }) + let success_message = succeed_operation(op_name, result); + match success_message { + Ok(message) => mqtt_publisher.send(Message::new(&topic, message.as_str())).await + .unwrap_or_else(|err| { + error!("Failed to publish a message: {message}. Error: {err}") + }), + Err(e) => { + let fail_message = fail_operation( + op_name, + &format!("{:?}", anyhow::Error::from(e).context("Custom operation process exited successfully, but couldn't convert output to valid SmartREST message"))); + mqtt_publisher.send(Message::new(&topic, fail_message.as_str())).await.unwrap_or_else(|err| { + error!("Failed to publish a message: {fail_message}. Error: {err}") + }) + } + } } _ => { let failure_reason = get_failure_reason_for_smartrest( - output.stderr, + &output.stderr, MAX_PAYLOAD_LIMIT_IN_BYTES, ); - let failed_str = format!("502,{op_name},\"{failure_reason}\""); + let payload = fail_operation(op_name, &failure_reason); + mqtt_publisher - .send(Message::new(&topic, failed_str.as_str())) + .send(Message::new(&topic, payload.as_bytes())) .await .unwrap_or_else(|err| { error!( - "Failed to publish a message: {failed_str}. Error: {err}" - ) + "Failed to publish a message: {payload}. Error: {err}" + ) }) } } @@ -897,7 +908,7 @@ impl CumulocityConverter { let ops = Operations::try_new(path_to_child_devices.join(&child_id))?; self.children .insert(child_external_id.clone().into(), ops.clone()); - let ops_msg = ops.create_smartrest_ops_message()?; + let ops_msg = ops.create_smartrest_ops_message(); let topic = C8yTopic::ChildSmartRestResponse(child_external_id.into()).to_topic()?; messages_vec.push(Message::new(&topic, ops_msg)); } @@ -1241,13 +1252,13 @@ impl CumulocityConverter { let topic = C8yTopic::ChildSmartRestResponse(child_external_id.into()).to_topic()?; Ok(Message::new( &topic, - Operations::try_new(path)?.create_smartrest_ops_message()?, + Operations::try_new(path)?.create_smartrest_ops_message(), )) } else { // operations for parent Ok(Message::new( &Topic::new_unchecked(SMARTREST_PUBLISH_TOPIC), - Operations::try_new(path)?.create_smartrest_ops_message()?, + Operations::try_new(path)?.create_smartrest_ops_message(), )) } } @@ -1305,10 +1316,8 @@ fn get_child_id(dir_path: &PathBuf) -> Result { } fn create_get_pending_operations_message() -> Result { - let data = SmartRestGetPendingOperations::default(); let topic = C8yTopic::SmartRestResponse.to_topic()?; - let payload = data.to_smartrest()?; - Ok(Message::new(&topic, payload)) + Ok(Message::new(&topic, request_pending_operations())) } fn is_child_operation_path(path: &Path) -> bool { @@ -1393,18 +1402,13 @@ impl CumulocityConverter { match command.status() { CommandStatus::Executing => { - let smartrest_set_operation = SmartRestSetOperationToExecuting::new( - CumulocitySupportedOperations::C8yRestartRequest, - ) - .to_smartrest()?; - + let smartrest_set_operation = + set_operation_executing(CumulocitySupportedOperations::C8yRestartRequest); Ok(vec![Message::new(&topic, smartrest_set_operation)]) } CommandStatus::Successful => { - let smartrest_set_operation = SmartRestSetOperationToSuccessful::new( - CumulocitySupportedOperations::C8yRestartRequest, - ) - .to_smartrest()?; + let smartrest_set_operation = + succeed_operation_no_payload(CumulocitySupportedOperations::C8yRestartRequest); Ok(vec![ command.clearing_message(&self.mqtt_schema), @@ -1412,11 +1416,11 @@ impl CumulocityConverter { ]) } CommandStatus::Failed { ref reason } => { - let smartrest_set_operation = SmartRestSetOperationToFailed::new( + let smartrest_set_operation = fail_operation( CumulocitySupportedOperations::C8yRestartRequest, - format!("Restart Failed: {}", reason), - ) - .to_smartrest()?; + &format!("Restart Failed: {reason}"), + ); + Ok(vec![ command.clearing_message(&self.mqtt_schema), Message::new(&topic, smartrest_set_operation), @@ -1484,17 +1488,13 @@ impl CumulocityConverter { Ok(vec![]) } CommandStatus::Executing => { - let smartrest_set_operation_status = SmartRestSetOperationToExecuting::new( - CumulocitySupportedOperations::C8ySoftwareUpdate, - ) - .to_smartrest()?; + let smartrest_set_operation_status = + set_operation_executing(CumulocitySupportedOperations::C8ySoftwareUpdate); Ok(vec![Message::new(&topic, smartrest_set_operation_status)]) } CommandStatus::Successful => { - let smartrest_set_operation = SmartRestSetOperationToSuccessful::new( - CumulocitySupportedOperations::C8ySoftwareUpdate, - ) - .to_smartrest()?; + let smartrest_set_operation = + succeed_operation_no_payload(CumulocitySupportedOperations::C8ySoftwareUpdate); Ok(vec![ Message::new(&topic, smartrest_set_operation), @@ -1503,11 +1503,9 @@ impl CumulocityConverter { ]) } CommandStatus::Failed { reason } => { - let smartrest_set_operation = SmartRestSetOperationToFailed::new( - CumulocitySupportedOperations::C8ySoftwareUpdate, - reason, - ) - .to_smartrest()?; + let smartrest_set_operation = + fail_operation(CumulocitySupportedOperations::C8ySoftwareUpdate, &reason); + Ok(vec![ Message::new(&topic, smartrest_set_operation), response.clearing_message(&self.mqtt_schema), @@ -2654,10 +2652,10 @@ pub(crate) mod tests { .execute_operation( "5", "sleep", - "sleep_ten", ResultFormat::Text, tokio::time::Duration::from_secs(10), tokio::time::Duration::from_secs(1), + "sleep_ten".to_owned(), ) .await .unwrap(); @@ -2665,10 +2663,10 @@ pub(crate) mod tests { .execute_operation( "5", "sleep", - "sleep_twenty", ResultFormat::Text, tokio::time::Duration::from_secs(20), tokio::time::Duration::from_secs(1), + "sleep_twenty".to_owned(), ) .await .unwrap(); diff --git a/crates/extensions/c8y_mapper_ext/src/firmware_update.rs b/crates/extensions/c8y_mapper_ext/src/firmware_update.rs index 7fbe4982206..165f7c6c378 100644 --- a/crates/extensions/c8y_mapper_ext/src/firmware_update.rs +++ b/crates/extensions/c8y_mapper_ext/src/firmware_update.rs @@ -3,11 +3,10 @@ use crate::error::ConversionError; use crate::error::CumulocityMapperError; use c8y_api::smartrest::smartrest_deserializer::SmartRestFirmwareRequest; use c8y_api::smartrest::smartrest_deserializer::SmartRestRequestGeneric; +use c8y_api::smartrest::smartrest_serializer::fail_operation; +use c8y_api::smartrest::smartrest_serializer::set_operation_executing; +use c8y_api::smartrest::smartrest_serializer::succeed_operation_no_payload; use c8y_api::smartrest::smartrest_serializer::CumulocitySupportedOperations; -use c8y_api::smartrest::smartrest_serializer::SmartRestSerializer; -use c8y_api::smartrest::smartrest_serializer::SmartRestSetOperationToExecuting; -use c8y_api::smartrest::smartrest_serializer::SmartRestSetOperationToFailed; -use c8y_api::smartrest::smartrest_serializer::SmartRestSetOperationToSuccessful; use tedge_api::entity_store::EntityType; use tedge_api::messages::FirmwareMetadata; use tedge_api::messages::FirmwareUpdateCmdPayload; @@ -89,20 +88,16 @@ impl CumulocityConverter { let payload = message.payload_str()?; let response = &FirmwareUpdateCmdPayload::from_json(payload)?; - let messages = match response.status { + let messages = match &response.status { CommandStatus::Executing => { - let smartrest_operation_status = SmartRestSetOperationToExecuting::new( - CumulocitySupportedOperations::C8yFirmware, - ) - .to_smartrest()?; + let smartrest_operation_status = + set_operation_executing(CumulocitySupportedOperations::C8yFirmware); vec![Message::new(&sm_topic, smartrest_operation_status)] } CommandStatus::Successful => { - let smartrest_operation_status = SmartRestSetOperationToSuccessful::new( - CumulocitySupportedOperations::C8yFirmware, - ) - .to_smartrest()?; + let smartrest_operation_status = + succeed_operation_no_payload(CumulocitySupportedOperations::C8yFirmware); let c8y_notification = Message::new(&sm_topic, smartrest_operation_status); let clear_local_cmd = Message::new(&message.topic, "") @@ -126,12 +121,9 @@ impl CumulocityConverter { vec![c8y_notification, clear_local_cmd, update_metadata] } - CommandStatus::Failed { ref reason } => { - let smartrest_operation_status = SmartRestSetOperationToFailed::new( - CumulocitySupportedOperations::C8yFirmware, - reason.clone(), - ) - .to_smartrest()?; + CommandStatus::Failed { reason } => { + let smartrest_operation_status = + fail_operation(CumulocitySupportedOperations::C8yFirmware, reason); let c8y_notification = Message::new(&sm_topic, smartrest_operation_status); let clear_local_cmd = Message::new(&message.topic, "") .with_retain() @@ -399,7 +391,7 @@ mod tests { // Expect `502` smartrest message on `c8y/s/us`. assert_received_contains_str( &mut mqtt, - [("c8y/s/us", "502,c8y_Firmware,\"Something went wrong\"")], + [("c8y/s/us", "502,c8y_Firmware,Something went wrong")], ) .await; } @@ -455,7 +447,7 @@ mod tests { &mut mqtt, [( "c8y/s/us/test-device:device:child1", - "502,c8y_Firmware,\"Something went wrong\"", + "502,c8y_Firmware,Something went wrong", )], ) .await; diff --git a/crates/extensions/c8y_mapper_ext/src/log_upload.rs b/crates/extensions/c8y_mapper_ext/src/log_upload.rs index 858572b0f30..9e0b0b86752 100644 --- a/crates/extensions/c8y_mapper_ext/src/log_upload.rs +++ b/crates/extensions/c8y_mapper_ext/src/log_upload.rs @@ -8,10 +8,9 @@ use crate::error::CumulocityMapperError; use anyhow::Context; use c8y_api::smartrest::smartrest_deserializer::SmartRestLogRequest; use c8y_api::smartrest::smartrest_deserializer::SmartRestRequestGeneric; +use c8y_api::smartrest::smartrest_serializer::fail_operation; +use c8y_api::smartrest::smartrest_serializer::set_operation_executing; use c8y_api::smartrest::smartrest_serializer::CumulocitySupportedOperations; -use c8y_api::smartrest::smartrest_serializer::SmartRestSerializer; -use c8y_api::smartrest::smartrest_serializer::SmartRestSetOperationToExecuting; -use c8y_api::smartrest::smartrest_serializer::SmartRestSetOperationToFailed; use c8y_http_proxy::messages::CreateEvent; use camino::Utf8PathBuf; use std::collections::HashMap; @@ -114,12 +113,10 @@ impl CumulocityConverter { let payload = message.payload_str()?; let response = &LogUploadCmdPayload::from_json(payload)?; - let messages = match response.status { + let messages = match &response.status { CommandStatus::Executing => { - let smartrest_operation_status = SmartRestSetOperationToExecuting::new( - CumulocitySupportedOperations::C8yLogFileRequest, - ) - .to_smartrest()?; + let smartrest_operation_status = + set_operation_executing(CumulocitySupportedOperations::C8yLogFileRequest); vec![Message::new(&smartrest_topic, smartrest_operation_status)] } CommandStatus::Successful => { @@ -159,12 +156,9 @@ impl CumulocityConverter { vec![] // No mqtt message can be published in this state } - CommandStatus::Failed { ref reason } => { - let smartrest_operation_status = SmartRestSetOperationToFailed::new( - CumulocitySupportedOperations::C8yLogFileRequest, - reason.clone(), - ) - .to_smartrest()?; + CommandStatus::Failed { reason } => { + let smartrest_operation_status = + fail_operation(CumulocitySupportedOperations::C8yLogFileRequest, reason); let c8y_notification = Message::new(&smartrest_topic, smartrest_operation_status); let clean_operation = Message::new(&message.topic, "") .with_retain() @@ -195,13 +189,12 @@ impl CumulocityConverter { let download_response = match download_result { Err(err) => { - let smartrest_error = SmartRestSetOperationToFailed::new( + let smartrest_error = fail_operation( CumulocitySupportedOperations::C8yLogFileRequest, - format!( + &format!( "tedge-mapper-c8y failed to download log from file transfer service: {err}", ), - ) - .to_smartrest()?; + ); let c8y_notification = Message::new(&smartrest_topic, smartrest_error); let clean_operation = Message::new(&fts_download.message.topic, "") @@ -568,7 +561,7 @@ mod tests { // Expect `502` smartrest message on `c8y/s/us`. assert_received_contains_str( &mut mqtt, - [("c8y/s/us", "502,c8y_LogfileRequest,\"Unknown reason\"")], + [("c8y/s/us", "502,c8y_LogfileRequest,Unknown reason")], ) .await; } @@ -650,7 +643,7 @@ mod tests { &mut mqtt, [( "c8y/s/us/test-device:device:child1", - "502,c8y_LogfileRequest,\"Something went wrong\"", + "502,c8y_LogfileRequest,Something went wrong", )], ) .await; diff --git a/crates/extensions/c8y_mapper_ext/src/service_monitor.rs b/crates/extensions/c8y_mapper_ext/src/service_monitor.rs index 58b13deedef..63ac92b3b95 100644 --- a/crates/extensions/c8y_mapper_ext/src/service_monitor.rs +++ b/crates/extensions/c8y_mapper_ext/src/service_monitor.rs @@ -68,7 +68,7 @@ mod tests { r#"{"pid":"1234","status":"up"}"#, "c8y/s/us/test_device:device:main:service:tedge-mapper-c8y", r#"104,up"#; - "service-monitoring-thin-edge-device" + "service monitoring thin-edge device" )] #[test_case( "test_device", @@ -76,7 +76,7 @@ mod tests { r#"{"pid":"1234","status":"up"}"#, "c8y/s/us/test_device:device:child/test_device:device:child:service:tedge-mapper-c8y", r#"104,up"#; - "service-monitoring-thin-edge-child-device" + "service monitoring thin-edge child device" )] #[test_case( "test_device", @@ -84,7 +84,7 @@ mod tests { r#"{"pid":"123456"}"#, "c8y/s/us/test_device:device:main:service:tedge-mapper-c8y", r#"104,unknown"#; - "service-monitoring-thin-edge-no-status" + "service monitoring thin-edge no status" )] #[test_case( "test_device", @@ -92,7 +92,7 @@ mod tests { r#"{"status":""}"#, "c8y/s/us/test_device:device:main:service:tedge-mapper-c8y", r#"104,unknown"#; - "service-monitoring-empty-status" + "service monitoring empty status" )] #[test_case( "test_device", @@ -100,7 +100,7 @@ mod tests { "{}", "c8y/s/us/test_device:device:main:service:tedge-mapper-c8y", r#"104,unknown"#; - "service-monitoring-empty-health-message" + "service monitoring empty health message" )] #[test_case( "test_device", @@ -108,15 +108,15 @@ mod tests { r#"{"status":"up,down"}"#, "c8y/s/us/test_device:device:main:service:tedge-mapper-c8y", r#"104,"up,down""#; - "service-monitoring-type-with-comma-health-message" + "service monitoring type with comma health message" )] #[test_case( "test_device", "te/device/main/service/tedge-mapper-c8y/status/health", r#"{"status":"up\"down"}"#, "c8y/s/us/test_device:device:main:service:tedge-mapper-c8y", - r#"104,up""down"#; - "service-monitoring-double-quotes-health-message" + r#"104,"up""down""#; + "service monitoring double quotes health message" )] fn translate_health_status_to_c8y_service_monitoring_message( device_name: &str, diff --git a/crates/extensions/c8y_mapper_ext/src/tests.rs b/crates/extensions/c8y_mapper_ext/src/tests.rs index ef30b7ea89b..ef659361302 100644 --- a/crates/extensions/c8y_mapper_ext/src/tests.rs +++ b/crates/extensions/c8y_mapper_ext/src/tests.rs @@ -397,7 +397,7 @@ async fn mapper_publishes_software_update_failed_status_onto_c8y_topic() { &mut mqtt, [( "c8y/s/us", - "502,c8y_SoftwareUpdate,\"Partial failure: Couldn\'t install collectd and nginx\"\n", + "502,c8y_SoftwareUpdate,Partial failure: Couldn't install collectd and nginx", )], ) .await; @@ -1459,7 +1459,7 @@ async fn mapper_publishes_supported_operations_for_child_device() { &mut mqtt, [ ("c8y/s/us", "101,child1,child1,thin-edge.io-child"), - ("c8y/s/us/child1", "114,c8y_ChildTestOp1,c8y_ChildTestOp2\n"), + ("c8y/s/us/child1", "114,c8y_ChildTestOp1,c8y_ChildTestOp2"), ], ) .await; @@ -1500,7 +1500,7 @@ async fn mapping_child_device_dirs_with_forbidden_characters() { "c8y/s/us", "101,simple_child,simple_child,thin-edge.io-child", ), - ("c8y/s/us/simple_child", "114,c8y_ChildTestOp2\n"), + ("c8y/s/us/simple_child", "114,c8y_ChildTestOp2"), ], ) .await; @@ -2060,7 +2060,7 @@ async fn custom_operation_timeout_sigterm() { &mut mqtt, [( "c8y/s/us", - "502,c8y_Command,\"operation failed due to timeout: duration=1s\"", + "502,c8y_Command,operation failed due to timeout: duration=1s", )], ) .await; @@ -2125,7 +2125,7 @@ async fn custom_operation_timeout_sigkill() { &mut mqtt, [( "c8y/s/us", - "502,c8y_Command,\"operation failed due to timeout: duration=1s\"", + "502,c8y_Command,operation failed due to timeout: duration=1s", )], ) .await; diff --git a/crates/extensions/tedge_config_manager/src/actor.rs b/crates/extensions/tedge_config_manager/src/actor.rs index 6c785e721ec..b250d0834a0 100644 --- a/crates/extensions/tedge_config_manager/src/actor.rs +++ b/crates/extensions/tedge_config_manager/src/actor.rs @@ -243,7 +243,7 @@ impl ConfigManagerActor { } Err(err) => { let error_message = format!( - "tedge-configuration-plugin failed uploading configuration snapshot: {}", + "config-manager failed uploading configuration snapshot: {}", err ); request.failed(&error_message); @@ -269,7 +269,7 @@ impl ConfigManagerActor { } Err(error) => { let error_message = format!( - "tedge-configuration-plugin failed to start downloading configuration: {}", + "config-manager failed to start downloading configuration: {}", error ); request.failed(&error_message); @@ -324,8 +324,7 @@ impl ConfigManagerActor { let response = match result { Ok(response) => response, Err(err) => { - let error_message = - format!("tedge-configuration-plugin failed downloading a file: {err}",); + let error_message = format!("config-manager failed downloading a file: {err}",); request.failed(&error_message); error!("{}", error_message); self.publish_command_status(&topic, &ConfigOperation::Update(request)) diff --git a/crates/extensions/tedge_mqtt_ext/src/test_helpers.rs b/crates/extensions/tedge_mqtt_ext/src/test_helpers.rs index 975d7ed3e4d..6d5a3d9fd9b 100644 --- a/crates/extensions/tedge_mqtt_ext/src/test_helpers.rs +++ b/crates/extensions/tedge_mqtt_ext/src/test_helpers.rs @@ -47,9 +47,7 @@ pub fn assert_message_contains_str(message: &Message, expected: (&str, &str)) { let payload = message.payload_str().expect("non UTF-8 payload"); assert!( payload.contains(expected_payload), - "Payload assertion failed.\n Actual: {} \n Expected: {}", - payload, - expected_payload + "Payload assertion failed.\n Actual: {payload:?} \n Expected: {expected_payload:?}", ) } diff --git a/tests/RobotFramework/tests/cumulocity/configuration/configuration_operation.robot b/tests/RobotFramework/tests/cumulocity/configuration/configuration_operation.robot index f6b3cbce1f1..b8081d00db7 100644 --- a/tests/RobotFramework/tests/cumulocity/configuration/configuration_operation.robot +++ b/tests/RobotFramework/tests/cumulocity/configuration/configuration_operation.robot @@ -157,7 +157,7 @@ Get Unknown Configuration Type From Device [Arguments] ${test_desc} ${external_id} ${config_type} Cumulocity.Set Device ${external_id} ${operation}= Cumulocity.Get Configuration ${config_type} - Operation Should Be FAILED ${operation} failure_reason=.*requested config_type ${config_type} is not defined in the plugin configuration file.* + Operation Should Be FAILED ${operation} failure_reason=.*requested config_type "${config_type}" is not defined in the plugin configuration file.* Get non existent configuration file From Device [Arguments] ${test_desc} ${device} ${external_id} ${config_type} ${device_file} diff --git a/tests/RobotFramework/tests/cumulocity/configuration/configuration_with_file_transfer_https.robot b/tests/RobotFramework/tests/cumulocity/configuration/configuration_with_file_transfer_https.robot index c8064077703..3fe1b76bd01 100644 --- a/tests/RobotFramework/tests/cumulocity/configuration/configuration_with_file_transfer_https.robot +++ b/tests/RobotFramework/tests/cumulocity/configuration/configuration_with_file_transfer_https.robot @@ -47,11 +47,11 @@ Configuration operation fails when configuration-plugin does not supply client c Disable HTTP Client Certificate for Child Device Get Configuration Should Fail ... device=${CHILD_SN} - ... failure_reason=tedge-configuration-plugin failed uploading configuration snapshot:.+https://${parent_ip}:8000/tedge/file-transfer/.+received fatal alert: CertificateRequired + ... failure_reason=config-manager failed uploading configuration snapshot:.+https://${parent_ip}:8000/tedge/file-transfer/.+received fatal alert: CertificateRequired ... external_id=${PARENT_SN}:device:${CHILD_SN} Update Configuration Should Fail ... device=${CHILD_SN} - ... failure_reason=tedge-configuration-plugin failed downloading a file:.+https://${parent_ip}:8000/tedge/file-transfer/.+received fatal alert: CertificateRequired + ... failure_reason=config-manager failed downloading a file:.+https://${parent_ip}:8000/tedge/file-transfer/.+received fatal alert: CertificateRequired ... external_id=${PARENT_SN}:device:${CHILD_SN} Configuration snapshot fails when mapper does not supply client certificate diff --git a/tests/RobotFramework/tests/cumulocity/log/log_operation.robot b/tests/RobotFramework/tests/cumulocity/log/log_operation.robot index f7bf755eb5c..705323ed668 100644 --- a/tests/RobotFramework/tests/cumulocity/log/log_operation.robot +++ b/tests/RobotFramework/tests/cumulocity/log/log_operation.robot @@ -30,7 +30,7 @@ Request with non-existing log type ... fragments={"c8y_LogfileRequest":{"dateFrom":"${start_timestamp}","dateTo":"${end_timestamp}","logFile":"example1","searchText":"first","maximumLines":10}} Operation Should Be FAILED ... ${operation} - ... failure_reason=.*No logs found for log type example1 + ... failure_reason=.*No logs found for log type "example1" ... timeout=120 Manual log_upload operation request