From 99ba74ce838daa7497623dc4d204b99c55e4d274 Mon Sep 17 00:00:00 2001 From: Rina Fujino Date: Wed, 3 Apr 2024 19:32:55 +0200 Subject: [PATCH 1/2] Uploader supports multipart/form-data and attaches filename - Use async client to support multipart/form-data - Expand Uploader to support both PUT and POST method - Add MIME auto-detection based on file extension Signed-off-by: Rina Fujino --- Cargo.lock | 22 +++ Cargo.toml | 2 + crates/common/upload/Cargo.toml | 8 +- crates/common/upload/src/lib.rs | 3 + crates/common/upload/src/upload.rs | 125 +++++++++++++++--- .../src/operations/config_snapshot.rs | 17 ++- .../src/operations/log_upload.rs | 20 ++- .../tedge_uploader_ext/src/actor.rs | 23 +++- .../extensions/tedge_uploader_ext/src/lib.rs | 2 + 9 files changed, 196 insertions(+), 26 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bdf319010dc..6e73c5ef6fd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2012,6 +2012,16 @@ version = "0.3.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6877bb514081ee2a7ff5ef9de3281f14a4dd4bceac4c09388074a6b5df8a139a" +[[package]] +name = "mime_guess" +version = "2.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4192263c238a5f0d0c6bfd21f336a313a4ce1c450542449ca191bb657b4642ef" +dependencies = [ + "mime", + "unicase", +] + [[package]] name = "minimal-lexical" version = "0.2.1" @@ -2794,6 +2804,7 @@ dependencies = [ "js-sys", "log", "mime", + "mime_guess", "once_cell", "percent-encoding", "pin-project-lite", @@ -4429,6 +4440,15 @@ dependencies = [ "version_check", ] +[[package]] +name = "unicase" +version = "2.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f7d2d4dafb69621809a81864c9c1b864479e1235c0dd4e199924b9742439ed89" +dependencies = [ + "version_check", +] + [[package]] name = "unicode-bidi" version = "0.3.15" @@ -4485,6 +4505,8 @@ dependencies = [ "camino", "futures", "log", + "mime", + "mime_guess", "mockito", "reqwest", "tedge_test_utils", diff --git a/Cargo.toml b/Cargo.toml index 5988ea489c8..f5f7a7c057c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -88,6 +88,8 @@ log_manager = { path = "crates/common/log_manager" } logged_command = { path = "crates/common/logged_command" } maplit = "1.0" miette = { version = "5.5.0", features = ["fancy"] } +mime = "0.3.17" +mime_guess = "2.0.4" mockall = "0.11" mockito = "1.1.0" mqtt_channel = { path = "crates/common/mqtt_channel" } diff --git a/crates/common/upload/Cargo.toml b/crates/common/upload/Cargo.toml index d5411e1b1f0..e9638434c6b 100644 --- a/crates/common/upload/Cargo.toml +++ b/crates/common/upload/Cargo.toml @@ -14,7 +14,13 @@ axum_tls = { workspace = true, features = ["error-matching"] } backoff = { workspace = true } camino = { workspace = true } log = { workspace = true } -reqwest = { workspace = true, features = ["stream", "rustls-tls-native-roots"] } +mime = { workspace = true } +mime_guess = { workspace = true } +reqwest = { workspace = true, features = [ + "stream", + "rustls-tls-native-roots", + "multipart", +] } thiserror = { workspace = true } tokio = { workspace = true, features = ["fs"] } tokio-util = { workspace = true, features = ["codec"] } diff --git a/crates/common/upload/src/lib.rs b/crates/common/upload/src/lib.rs index 9758ca37adf..c8f543e5f36 100644 --- a/crates/common/upload/src/lib.rs +++ b/crates/common/upload/src/lib.rs @@ -41,5 +41,8 @@ mod upload; pub use crate::error::UploadError; pub use crate::upload::Auth; pub use crate::upload::ContentType; +pub use crate::upload::FormData; pub use crate::upload::UploadInfo; +pub use crate::upload::UploadMethod; pub use crate::upload::Uploader; +pub use mime::Mime; diff --git a/crates/common/upload/src/upload.rs b/crates/common/upload/src/upload.rs index 2eb22d1eb7c..441e17c438c 100644 --- a/crates/common/upload/src/upload.rs +++ b/crates/common/upload/src/upload.rs @@ -5,12 +5,13 @@ use camino::Utf8Path; use camino::Utf8PathBuf; use log::info; use log::warn; +use mime::Mime; +use mime_guess::MimeGuess; use reqwest::header::CONTENT_LENGTH; use reqwest::header::CONTENT_TYPE; +use reqwest::multipart; use reqwest::Body; use reqwest::Identity; -use std::fmt::Display; -use std::fmt::Formatter; use std::time::Duration; use tokio::fs::File; use tokio_util::codec::BytesCodec; @@ -27,19 +28,51 @@ fn default_backoff() -> ExponentialBackoff { } } +/// Auto tries to detect the mime from the given file extension without direct file access. +/// Custom sets a custom Content-Type. +/// If multipart request is needed, choose FormData. #[derive(Debug, Clone, Eq, PartialEq)] pub enum ContentType { - TextPlain, - ApplicationOctetStream, + Auto, + Custom(Mime), + FormData(FormData), } -impl Display for ContentType { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - match self { - ContentType::TextPlain => write!(f, "text/plain"), - ContentType::ApplicationOctetStream => write!(f, "application/octet-stream"), +/// Dataset to construct reqwest::multipart::Form. +/// To avoid using reqwest::multipart::Form inside the ContentType enum +/// since reqwest::multipart::Form doesn't support Copy or Clone. +/// If mime is None, the mime will be guessed while uploading a file. +#[derive(Debug, Eq, Clone, PartialEq)] +pub struct FormData { + filename: String, + mime: Option, +} + +impl FormData { + pub fn new(filename: String) -> Self { + Self { + filename, + mime: None, } } + + pub fn set_mime(self, mime: Mime) -> Self { + Self { + mime: Some(mime), + ..self + } + } + + pub fn text_plain(self) -> Self { + self.set_mime(mime::TEXT_PLAIN) + } +} + +/// Switch upload method +#[derive(Debug, Clone, Eq, PartialEq)] +pub enum UploadMethod { + PUT, + POST, } #[derive(Debug, Clone, Eq, PartialEq)] @@ -47,6 +80,7 @@ pub struct UploadInfo { pub url: String, pub auth: Option, pub content_type: ContentType, + pub method: UploadMethod, } impl From<&str> for UploadInfo { @@ -60,7 +94,8 @@ impl UploadInfo { Self { url: url.into(), auth: None, - content_type: ContentType::ApplicationOctetStream, + content_type: ContentType::Auto, + method: UploadMethod::PUT, } } @@ -71,13 +106,17 @@ impl UploadInfo { } } - pub fn with_content_type(self, content_type: ContentType) -> Self { + pub fn set_content_type(self, content_type: ContentType) -> Self { Self { content_type, ..self } } + pub fn set_method(self, method: UploadMethod) -> Self { + Self { method, ..self } + } + pub fn url(&self) -> &str { self.url.as_str() } @@ -166,18 +205,42 @@ impl Uploader { info!("Redirecting request from {} to {target_url}", url.url()) } - // Todo: Ideally it detects the appropriate content-type automatically, e.g. UTF-8 => text/plain - let mut client = client - .put(target_url) - .header(CONTENT_TYPE, url.content_type.to_string()) - .header(CONTENT_LENGTH, file_length); + let mut client = match url.method { + UploadMethod::PUT => client.put(target_url), + UploadMethod::POST => client.post(target_url), + }; if let Some(Auth::Bearer(token)) = &url.auth { client = client.bearer_auth(token) } + client = match url.content_type.clone() { + ContentType::Auto => { + let mime = MimeGuess::from_path(&self.source_filename).first_or_octet_stream(); + client + .header(CONTENT_TYPE, mime.as_ref()) + .header(CONTENT_LENGTH, file_length) + .body(file_body) + } + ContentType::Custom(mime) => client + .header(CONTENT_TYPE, mime.as_ref()) + .header(CONTENT_LENGTH, file_length) + .body(file_body), + ContentType::FormData(data) => { + let mime = match data.mime { + None => MimeGuess::from_path(&self.source_filename).first_or_octet_stream(), + Some(mime) => mime, + }; + let file_part = multipart::Part::stream_with_length(file_body, file_length) + .file_name(data.filename) + .mime_str(mime.as_ref()) + .unwrap(); // safe, ensured that mime is valid + let form = multipart::Form::new().part("file", file_part); + client.multipart(form) + } + }; + client - .body(file_body) .send() .await .map_err(|err| { @@ -255,6 +318,34 @@ mod tests { assert!(uploader.upload(&url).await.is_ok()) } + #[tokio::test] + async fn upload_content_no_auth_post() { + let mut server = mockito::Server::new(); + let _mock1 = server + .mock("POST", "/some_file.txt") + .with_status(201) + .create(); + + let mut target_url = server.url(); + target_url.push_str("/some_file.txt"); + + let url = UploadInfo::new(&target_url) + .set_content_type(ContentType::FormData(FormData::new("filename".into()))) + .set_method(UploadMethod::POST); + + let ttd = TempTedgeDir::new(); + ttd.file("file_upload.txt") + .with_raw_content("Hello, world!"); + + let mut uploader = Uploader::new(ttd.utf8_path().join("file_upload.txt"), None); + uploader.set_backoff(ExponentialBackoff { + current_interval: Duration::ZERO, + ..Default::default() + }); + + assert!(uploader.upload(&url).await.is_ok()) + } + #[tokio::test] async fn upload_content_with_auth() { let mut server = mockito::Server::new(); diff --git a/crates/extensions/c8y_mapper_ext/src/operations/config_snapshot.rs b/crates/extensions/c8y_mapper_ext/src/operations/config_snapshot.rs index 3fd1fa18ba4..fceb449ead9 100644 --- a/crates/extensions/c8y_mapper_ext/src/operations/config_snapshot.rs +++ b/crates/extensions/c8y_mapper_ext/src/operations/config_snapshot.rs @@ -28,6 +28,8 @@ use tedge_downloader_ext::DownloadResult; use tedge_mqtt_ext::MqttMessage; use tedge_mqtt_ext::QoS; use tedge_mqtt_ext::TopicFilter; +use tedge_uploader_ext::ContentType; +use tedge_uploader_ext::FormData; use tedge_uploader_ext::UploadRequest; use time::OffsetDateTime; use tracing::log::warn; @@ -183,6 +185,7 @@ impl CumulocityConverter { fts_download: FtsDownloadOperationData, ) -> Result, ConversionError> { let target = self.entity_store.try_get(&fts_download.entity_topic_id)?; + let xid = target.external_id.as_ref(); let smartrest_topic = self.smartrest_publish_topic_for_entity(&fts_download.entity_topic_id)?; let payload = fts_download.message.payload_str()?; @@ -212,7 +215,7 @@ impl CumulocityConverter { time: OffsetDateTime::now_utc(), text: response.config_type.clone(), extras: HashMap::new(), - device_id: target.external_id.as_ref().to_string(), + device_id: xid.to_string(), }; let event_response_id = self.http_proxy.send_event(create_event).await?; @@ -220,12 +223,20 @@ impl CumulocityConverter { .c8y_endpoint .get_url_for_event_binary_upload_unchecked(&event_response_id); + let file_path = Utf8PathBuf::try_from(download.file_path).map_err(|e| e.into_io_error())?; + + // The method must be POST, otherwise file name won't be supported. let upload_request = UploadRequest::new( self.auth_proxy .proxy_url(binary_upload_event_url.clone()) .as_str(), - &Utf8PathBuf::try_from(download.file_path).map_err(|e| e.into_io_error())?, - ); + &file_path, + ) + .post() + .with_content_type(ContentType::FormData(FormData::new(format!( + "{xid}_{filename}", + filename = file_path.file_name().unwrap_or("filename") + )))); self.pending_upload_operations.insert( cmd_id.clone(), diff --git a/crates/extensions/c8y_mapper_ext/src/operations/log_upload.rs b/crates/extensions/c8y_mapper_ext/src/operations/log_upload.rs index 034ef4644ac..1282c712822 100644 --- a/crates/extensions/c8y_mapper_ext/src/operations/log_upload.rs +++ b/crates/extensions/c8y_mapper_ext/src/operations/log_upload.rs @@ -31,6 +31,7 @@ use tedge_mqtt_ext::MqttMessage; use tedge_mqtt_ext::QoS; use tedge_mqtt_ext::TopicFilter; use tedge_uploader_ext::ContentType; +use tedge_uploader_ext::FormData; use tedge_uploader_ext::UploadRequest; use time::OffsetDateTime; use tracing::debug; @@ -187,6 +188,7 @@ impl CumulocityConverter { let smartrest_topic = self.smartrest_publish_topic_for_entity(&topic_id)?; let payload = fts_download.message.payload_str()?; let response = &LogUploadCmdPayload::from_json(payload)?; + let xid = target.external_id.as_ref(); let download_response = match download_result { Err(err) => { @@ -212,7 +214,7 @@ impl CumulocityConverter { time: OffsetDateTime::now_utc(), text: response.log_type.clone(), extras: HashMap::new(), - device_id: target.external_id.as_ref().to_string(), + device_id: xid.to_string(), }; let event_response_id = self.http_proxy.send_event(create_event).await?; @@ -220,13 +222,25 @@ impl CumulocityConverter { .c8y_endpoint .get_url_for_event_binary_upload_unchecked(&event_response_id); + let file_path = + Utf8PathBuf::try_from(download_response.file_path).map_err(|e| e.into_io_error())?; + + // The method must be POST, otherwise file name won't be supported. + // Mime must be text/*, otherwise c8y UI doesn't give a preview of the content. let upload_request = UploadRequest::new( self.auth_proxy .proxy_url(binary_upload_event_url.clone()) .as_str(), - &Utf8PathBuf::try_from(download_response.file_path).map_err(|e| e.into_io_error())?, + &file_path, ) - .with_content_type(ContentType::TextPlain); + .post() + .with_content_type(ContentType::FormData( + FormData::new(format!( + "{xid}_{filename}", + filename = file_path.file_name().unwrap_or("filename") + )) + .text_plain(), + )); self.uploader_sender .send((cmd_id.clone(), upload_request)) diff --git a/crates/extensions/tedge_uploader_ext/src/actor.rs b/crates/extensions/tedge_uploader_ext/src/actor.rs index 8e9e70d1509..2b15e316ee5 100644 --- a/crates/extensions/tedge_uploader_ext/src/actor.rs +++ b/crates/extensions/tedge_uploader_ext/src/actor.rs @@ -11,6 +11,7 @@ use upload::Auth; use upload::ContentType; use upload::UploadError; use upload::UploadInfo; +use upload::UploadMethod; use upload::Uploader; #[derive(Debug, Clone, Eq, PartialEq)] @@ -19,6 +20,7 @@ pub struct UploadRequest { pub file_path: Utf8PathBuf, pub auth: Option, pub content_type: ContentType, + pub method: UploadMethod, } impl UploadRequest { @@ -27,7 +29,8 @@ impl UploadRequest { url: url.into(), file_path: file_path.to_owned(), auth: None, - content_type: ContentType::ApplicationOctetStream, + content_type: ContentType::Auto, + method: UploadMethod::PUT, } } @@ -44,6 +47,20 @@ impl UploadRequest { ..self } } + + pub fn put(self) -> Self { + Self { + method: UploadMethod::PUT, + ..self + } + } + + pub fn post(self) -> Self { + Self { + method: UploadMethod::POST, + ..self + } + } } #[derive(Debug)] @@ -101,7 +118,9 @@ impl Server for UploaderActor { async fn handle(&mut self, id_request: Self::Request) -> Self::Response { let (id, request) = id_request; - let mut upload_info = UploadInfo::new(&request.url).with_content_type(request.content_type); + let mut upload_info = UploadInfo::new(&request.url) + .set_content_type(request.content_type) + .set_method(request.method); if let Some(auth) = request.auth { upload_info = upload_info.with_auth(auth); } diff --git a/crates/extensions/tedge_uploader_ext/src/lib.rs b/crates/extensions/tedge_uploader_ext/src/lib.rs index 7d083cdb167..2d62b30c9e7 100644 --- a/crates/extensions/tedge_uploader_ext/src/lib.rs +++ b/crates/extensions/tedge_uploader_ext/src/lib.rs @@ -5,3 +5,5 @@ mod tests; pub use actor::*; pub use upload::ContentType; +pub use upload::FormData; +pub use upload::Mime; From d342cd52962ae17fa5bd97e359a6abf1d9f54351 Mon Sep 17 00:00:00 2001 From: Rina Fujino Date: Thu, 4 Apr 2024 14:49:35 +0000 Subject: [PATCH 2/2] tests: add system test to validate log file meta info Co-authored-by: reubenmiller Signed-off-by: Rina Fujino --- tests/RobotFramework/requirements/requirements.txt | 2 +- .../configuration/configuration_operation.robot | 3 +++ .../tests/cumulocity/log/log_operation.robot | 3 ++- .../tests/cumulocity/log/log_operation_child.robot | 13 +++++++++++++ 4 files changed, 19 insertions(+), 2 deletions(-) diff --git a/tests/RobotFramework/requirements/requirements.txt b/tests/RobotFramework/requirements/requirements.txt index 8b336175e5d..a61d75be0de 100644 --- a/tests/RobotFramework/requirements/requirements.txt +++ b/tests/RobotFramework/requirements/requirements.txt @@ -2,7 +2,7 @@ dateparser~=1.2.0 paho-mqtt~=1.6.1 python-dotenv~=1.0.0 robotframework~=6.1.1 -robotframework-c8y @ git+https://github.com/reubenmiller/robotframework-c8y.git@0.31.3 +robotframework-c8y @ git+https://github.com/reubenmiller/robotframework-c8y.git@0.32.1 robotframework-debuglibrary~=2.3.0 robotframework-jsonlibrary~=0.5 robotframework-pabot~=2.17.0 diff --git a/tests/RobotFramework/tests/cumulocity/configuration/configuration_operation.robot b/tests/RobotFramework/tests/cumulocity/configuration/configuration_operation.robot index cad92800234..8b11dcf82a3 100644 --- a/tests/RobotFramework/tests/cumulocity/configuration/configuration_operation.robot +++ b/tests/RobotFramework/tests/cumulocity/configuration/configuration_operation.robot @@ -232,6 +232,9 @@ Get Configuration from Device ${contents}= Cumulocity.Event Should Have An Attachment ... ${events[0]["id"]} ... expected_md5=${expected_checksum} + + ${event}= Cumulocity.Event Attachment Should Have File Info ${events[0]["id"]} name=^${external_id}_[\\w\\W]+-c8y-mapper-\\d+$ + RETURN ${contents} # diff --git a/tests/RobotFramework/tests/cumulocity/log/log_operation.robot b/tests/RobotFramework/tests/cumulocity/log/log_operation.robot index c9a7ebeb3c6..c1366e5c705 100644 --- a/tests/RobotFramework/tests/cumulocity/log/log_operation.robot +++ b/tests/RobotFramework/tests/cumulocity/log/log_operation.robot @@ -152,13 +152,14 @@ Publish and Verify Local Command [Teardown] Execute Command tedge mqtt pub --retain '${topic}' '' Log File Contents Should Be Equal - [Arguments] ${operation} ${expected_contents} ${encoding}=utf-8 + [Arguments] ${operation} ${expected_contents} ${encoding}=utf-8 ${expected_filename}=^${DEVICE_SN}_[\\w\\W]+-c8y-mapper-\\d+$ ${expected_mime_type}=text/plain ${event_url_parts}= Split String ${operation["c8y_LogfileRequest"]["file"]} separator=/ ${event_id}= Set Variable ${event_url_parts}[-2] ${contents}= Cumulocity.Event Should Have An Attachment ... ${event_id} ... expected_contents=${expected_contents} ... encoding=${encoding} + ${event}= Cumulocity.Event Attachment Should Have File Info ${event_id} name=${expected_filename} mime_type=${expected_mime_type} RETURN ${contents} Disable log upload capability of tedge-agent diff --git a/tests/RobotFramework/tests/cumulocity/log/log_operation_child.robot b/tests/RobotFramework/tests/cumulocity/log/log_operation_child.robot index 22a26c1e8b2..68075f807ec 100644 --- a/tests/RobotFramework/tests/cumulocity/log/log_operation_child.robot +++ b/tests/RobotFramework/tests/cumulocity/log/log_operation_child.robot @@ -3,6 +3,7 @@ Resource ../../../resources/common.resource Library Cumulocity Library DateTime Library ThinEdgeIO +Library String Test Setup Custom Setup Test Teardown Custom Teardown @@ -19,6 +20,7 @@ Successful log operation ... description=Log file request ... fragments={"c8y_LogfileRequest":{"dateFrom":"${start_timestamp}","dateTo":"${end_timestamp}","logFile":"example","searchText":"first","maximumLines":10}} ${operation}= Operation Should Be SUCCESSFUL ${operation} timeout=120 + Log File Contents Should Be Equal ${operation} filename: example.log\n1 first line\n *** Keywords *** @@ -66,3 +68,14 @@ Custom Setup Custom Teardown Get Logs ${PARENT_SN} Get Logs ${CHILD_SN} + +Log File Contents Should Be Equal + [Arguments] ${operation} ${expected_contents} ${encoding}=utf-8 ${expected_filename}=^${CHILD_XID}_[\\w\\W]+-c8y-mapper-\\d+$ ${expected_mime_type}=text/plain + ${event_url_parts}= Split String ${operation["c8y_LogfileRequest"]["file"]} separator=/ + ${event_id}= Set Variable ${event_url_parts}[-2] + ${contents}= Cumulocity.Event Should Have An Attachment + ... ${event_id} + ... expected_contents=${expected_contents} + ... encoding=${encoding} + ${event}= Cumulocity.Event Attachment Should Have File Info ${event_id} name=${expected_filename} mime_type=${expected_mime_type} + RETURN ${contents}