Improve the common file library to avoid duplications

Signed-off-by: Rina Fujino <18257209+rina23q@users.noreply.github.com>

3 files changed, 70 insertions, 47 deletions

diff --git a/plugins/c8y_configuration_plugin/src/config.rs b/plugins/c8y_configuration_plugin/src/config.rs
index 0c42a804..892d157a 100644
--- a/plugins/c8y_configuration_plugin/src/config.rs
+++ b/plugins/c8y_configuration_plugin/src/config.rs
@@ -7,8 +7,8 @@ use std::collections::HashSet;
 use std::fs;
 use std::hash::{Hash, Hasher};
 use std::path::Path;
-use tedge_utils::file::FilePermissions;
-use tracing::{info, warn};
+use tedge_utils::file::PermissionEntry;
+use tracing::{error, info};
 
 #[derive(Deserialize, Debug, Default)]
 #[serde(deny_unknown_fields)]
@@ -36,7 +36,7 @@ pub struct PluginConfig {
 pub struct FileEntry {
     pub path: String,
     config_type: String,
-    pub file_permissions: FilePermissions,
+    pub file_permissions: PermissionEntry,
 }
 
 impl Hash for FileEntry {
@@ -68,7 +68,7 @@ impl FileEntry {
         Self {
             path,
             config_type,
-            file_permissions: FilePermissions { user, group, mode },
+            file_permissions: PermissionEntry { user, group, mode },
         }
     }
 }
@@ -81,12 +81,12 @@ impl RawPluginConfig {
             Ok(contents) => match toml::from_str(contents.as_str()) {
                 Ok(config) => config,
                 Err(err) => {
-                    warn!("The config file {path_str} is malformed. {err}");
+                    error!("The config file {path_str} is malformed. {err}");
                     Self::default()
                 }
             },
             Err(err) => {
-                warn!("The config file {path_str} does not exist or is not readable. {err}");
+                error!("The config file {path_str} does not exist or is not readable. {err}");
                 Self::default()
             }
         }
@@ -121,7 +121,7 @@ impl PluginConfig {
                 .unwrap_or_else(|| raw_entry.path.clone());
 
             if config_type.contains(&['+', '#']) {
-                warn!(
+                error!(
                     "The config type '{}' contains the forbidden characters, '+' or '#'.",
                     config_type
                 );
@@ -136,7 +136,7 @@ impl PluginConfig {
                 raw_entry.mode,
             );
             if !self.files.insert(entry) {
-                warn!("The config file has the duplicated type '{}'.", config_type);
+                error!("The config file has the duplicated type '{}'.", config_type);
                 return original_plugin_config;
             }
         }
@@ -207,6 +207,12 @@ mod tests {
             [[files]]
                 path = "/etc/mosquitto/mosquitto.conf"
                 type = "'single quotation'"
+            [[files]]
+                path = "path"
+                type = "type"
+                user = "user"
+                group = "group"
+                mode = 0o444
              "#,
         )
         .unwrap();
@@ -233,7 +239,14 @@ mod tests {
                 RawFileEntry::new_with_path_and_type(
                     "/etc/mosquitto/mosquitto.conf".to_string(),
                     Some("'single quotation'".to_string())
-                )
+                ),
+                RawFileEntry {
+                    path: "path".to_string(),
+                    config_type: Some("type".to_string()),
+                    user: Some("user".to_string()),
+                    group: Some("group".to_string()),
+                    mode: Some(0o444)
+                }
             ]
         );
     }
@@ -248,6 +261,7 @@ mod tests {
                    { path = "/etc/tedge/mosquitto-conf/c8y-bridge.conf" },
                    { path = "/etc/tedge/mosquitto-conf/tedge-mosquitto.conf", type = '"double quotation"' },
                    { path = "/etc/mosquitto/mosquitto.conf", type = "'single quotation'" },
+                   { path = "path", type = "type", user = "user", group = "group", mode = 0o444 },
                  ]
              "#,
         )
@@ -275,7 +289,14 @@ mod tests {
                 RawFileEntry::new_with_path_and_type(
                     "/etc/mosquitto/mosquitto.conf".to_string(),
                     Some("'single quotation'".to_string())
-                )
+                ),
+                RawFileEntry {
+                    path: "path".to_string(),
+                    config_type: Some("type".to_string()),
+                    user: Some("user".to_string()),
+                    group: Some("group".to_string()),
+                    mode: Some(0o444)
+                }
             ]
         );
     }
@@ -392,6 +413,31 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn get_smartrest_single_type() {
+        let plugin_config = PluginConfig {
+            files: HashSet::from([FileEntry::new_with_path_and_type(
+                "/path/to/file".to_string(),
+                "typeA".to_string(),
+            )]),
+        };
+        let output = plugin_config.to_smartrest_payload();
+        assert_eq!(output, "119,typeA");
+    }
+
+    #[test]
+    fn get_smartrest_multiple_types() {
+        let plugin_config = PluginConfig {
+            files: HashSet::from([
+                FileEntry::new_with_path_and_type("path1".to_string(), "typeA".to_string()),
+                FileEntry::new_with_path_and_type("path2".to_string(), "typeB".to_string()),
+                FileEntry::new_with_path_and_type("path3".to_string(), "typeC".to_string()),
+            ]),
+        };
+        let output = plugin_config.to_smartrest_payload();
+        assert_eq!(output, ("119,typeA,typeB,typeC"));
+    }
+
     impl RawFileEntry {
         pub fn new_with_path_and_type(path: String, config_type: Option<String>) -> Self {
             Self {
@@ -409,7 +455,7 @@ mod tests {
             Self {
                 path,
                 config_type,
-                file_permissions: FilePermissions::default(),
+                file_permissions: PermissionEntry::default(),
             }
         }
     }
@@ -432,29 +478,4 @@ mod tests {
         file.write_all(content.as_bytes())?;
         Ok((temp_dir, config_root))
     }
-
-    #[test]
-    fn get_smartrest_single_type() {
-        let plugin_config = PluginConfig {
-            files: HashSet::from([FileEntry::new_with_path_and_type(
-                "/path/to/file".to_string(),
-                "typeA".to_string(),
-            )]),
-        };
-        let output = plugin_config.to_smartrest_payload();
-        assert_eq!(output, "119,typeA");
-    }
-
-    #[test]
-    fn get_smartrest_multiple_types() {
-        let plugin_config = PluginConfig {
-            files: HashSet::from([
-                FileEntry::new_with_path_and_type("path1".to_string(), "typeA".to_string()),
-                FileEntry::new_with_path_and_type("path2".to_string(), "typeB".to_string()),
-                FileEntry::new_with_path_and_type("path3".to_string(), "typeC".to_string()),
-            ]),
-        };
-        let output = plugin_config.to_smartrest_payload();
-        assert_eq!(output, ("119,typeA,typeB,typeC"));
-    }
 }
diff --git a/plugins/c8y_configuration_plugin/src/download.rs b/plugins/c8y_configuration_plugin/src/download.rs
index 9a345385..b281ec78 100644
--- a/plugins/c8y_configuration_plugin/src/download.rs
+++ b/plugins/c8y_configuration_plugin/src/download.rs
@@ -14,7 +14,7 @@ use serde_json::json;
 use std::fs;
 use std::os::unix::fs::PermissionsExt;
 use std::path::PathBuf;
-use tedge_utils::file::{get_filename, get_metadata, FilePermissions};
+use tedge_utils::file::{get_filename, get_metadata, PermissionEntry};
 use tracing::{info, warn};
 
 pub async fn handle_config_download_request(
@@ -64,7 +64,7 @@ async fn download_config_file(
     download_url: &str,
     file_path: PathBuf,
     tmp_dir: PathBuf,
-    file_permissions: FilePermissions,
+    file_permissions: PermissionEntry,
     http_client: &mut impl C8YHttpProxy,
 ) -> Result<(), anyhow::Error> {
     // Convert smartrest request to config download request struct
@@ -97,7 +97,7 @@ pub struct ConfigDownloadRequest {
     pub download_info: DownloadInfo,
     pub file_path: PathBuf,
     pub tmp_dir: PathBuf,
-    pub file_permissions: FilePermissions,
+    pub file_permissions: PermissionEntry,
     pub file_name: String,
 }
 
@@ -106,7 +106,7 @@ impl ConfigDownloadRequest {
         download_url: &str,
         file_path: PathBuf,
         tmp_dir: PathBuf,
-        file_permissions: FilePermissions,
+        file_permissions: PermissionEntry,
     ) -> Result<Self, ConfigManagementError> {
         let file_name = get_filename(file_path.clone()).ok_or_else(|| {
             ConfigManagementError::FileNameNotFound {
@@ -127,7 +127,6 @@ impl ConfigDownloadRequest {
     }
 
     fn has_write_access(&self) -> Result<(), ConfigManagementError> {
-        // The file does not exist before downloading a file
         let metadata =
             if self.file_path.is_file() {
                 get_metadata(&self.file_path)?
@@ -175,13 +174,13 @@ impl ConfigDownloadRequest {
 
         let file_permissions = if let Some(mode) = original_permission_mode {
             // Use the same file permission as the original one
-            FilePermissions::new(None, None, Some(mode))
+            PermissionEntry::new(None, None, Some(mode))
         } else {
             // Set the user, group, and mode as given for a new file
             self.file_permissions.clone()
         };
 
-        let () = file_permissions.change_permissions(&self.file_path.display().to_string())?;
+        let () = file_permissions.apply(&self.file_path)?;
 
         Ok(())
     }
@@ -232,7 +231,7 @@ mod tests {
             "https://test.cumulocity.com/inventory/binaries/70208",
             PathBuf::from("/etc/tedge/tedge.toml"),
             PathBuf::from("/tmp"),
-            FilePermissions::default(),
+            PermissionEntry::default(),
         )?;
 
         assert_eq!(
@@ -244,7 +243,7 @@ mod tests {
                 },
                 file_path: PathBuf::from("/etc/tedge/tedge.toml"),
                 tmp_dir: PathBuf::from("/tmp"),
-                file_permissions: FilePermissions::new(None, None, None),
+                file_permissions: PermissionEntry::new(None, None, None),
                 file_name: "tedge.toml".to_string()
             }
         );
@@ -257,7 +256,7 @@ mod tests {
             "https://test.cumulocity.com/inventory/binaries/70208",
             PathBuf::from("/"),
             PathBuf::from("/tmp"),
-            FilePermissions::default(),
+            PermissionEntry::default(),
         )
         .unwrap_err();
 
diff --git a/plugins/c8y_configuration_plugin/src/main.rs b/plugins/c8y_configuration_plugin/src/main.rs
index 71b1d9ea..b8d56fa0 100644
--- a/plugins/c8y_configuration_plugin/src/main.rs
+++ b/plugins/c8y_configuration_plugin/src/main.rs
@@ -19,7 +19,9 @@ use tedge_config::{
     ConfigRepository, ConfigSettingAccessor, MqttPortSetting, TEdgeConfig, TmpPathSetting,
     DEFAULT_TEDGE_CONFIG_PATH,
 };
-use tedge_utils::file::{create_directory_with_user_group, create_file_with_user_group};
+use tedge_utils::file::{
+    create_directory_with_mode, create_directory_with_user_group, create_file_with_user_group,
+};
 use tracing::{debug, error, info};
 
 pub const DEFAULT_PLUGIN_CONFIG_FILE_PATH: &str = "/etc/tedge/c8y/c8y-configuration-plugin.toml";
@@ -203,6 +205,7 @@ fn init(cfg_dir: PathBuf) -> Result<(), anyhow::Error> {
 }
 
 fn create_operation_files(config_dir: &str) -> Result<(), anyhow::Error> {
+    create_directory_with_mode(&format!("{config_dir}/c8y"), 0o755)?;
     create_directory_with_user_group(
         &format!("{config_dir}/operations/c8y"),
         "tedge",