Skip to content

Commit f0a8566

Browse files
committed
Simplify tedge_utils file api usage
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
1 parent cf4ba9e commit f0a8566

File tree

6 files changed

+60
-70
lines changed

6 files changed

+60
-70
lines changed

crates/common/tedge_config/src/tedge_toml/tedge_config_location.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -432,13 +432,11 @@ impl TEdgeConfigLocation {
432432

433433
atomically_write_file_async(toml_path, toml.as_bytes()).await?;
434434

435-
if let Err(err) =
436-
change_user_and_group(toml_path.into(), "tedge".into(), "tedge".into()).await
437-
{
435+
if let Err(err) = change_user_and_group(toml_path, "tedge", "tedge").await {
438436
warn!("failed to set file ownership for '{toml_path}': {err}");
439437
}
440438

441-
if let Err(err) = change_mode(toml_path.as_ref(), 0o644).await {
439+
if let Err(err) = change_mode(toml_path, 0o644).await {
442440
warn!("failed to set file permissions for '{toml_path}': {err}");
443441
}
444442

crates/common/tedge_utils/src/file.rs

Lines changed: 50 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -73,15 +73,15 @@ pub enum FileError {
7373
},
7474
}
7575

76-
pub async fn create_directory<P: AsRef<Path>>(
77-
dir: P,
76+
pub async fn create_directory(
77+
dir: impl AsRef<Path>,
7878
permissions: &PermissionEntry,
7979
) -> Result<(), FileError> {
8080
permissions.create_directory(dir.as_ref()).await
8181
}
8282

8383
/// Create the directory owned by the user running this API with default directory permissions
84-
pub async fn create_directory_with_defaults<P: AsRef<Path>>(dir: P) -> Result<(), FileError> {
84+
pub async fn create_directory_with_defaults(dir: impl AsRef<Path>) -> Result<(), FileError> {
8585
create_directory(dir, &PermissionEntry::default()).await
8686
}
8787

@@ -95,17 +95,17 @@ pub async fn create_directory_with_user_group(
9595
perm_entry.create_directory(dir.as_ref()).await
9696
}
9797

98-
pub async fn create_file<P: AsRef<Path>>(
99-
file: P,
98+
pub async fn create_file(
99+
file: impl AsRef<Path>,
100100
content: Option<&str>,
101101
permissions: PermissionEntry,
102102
) -> Result<(), FileError> {
103103
permissions.create_file(file.as_ref(), content).await
104104
}
105105

106106
/// Create the directory owned by the user running this API with default file permissions
107-
pub async fn create_file_with_defaults<P: AsRef<Path>>(
108-
file: P,
107+
pub async fn create_file_with_defaults(
108+
file: impl AsRef<Path>,
109109
content: Option<&str>,
110110
) -> Result<(), FileError> {
111111
create_file(file, content, PermissionEntry::default()).await
@@ -247,13 +247,13 @@ impl PermissionEntry {
247247
pub async fn apply(self, path: &Path) -> Result<(), FileError> {
248248
match (self.user, self.group) {
249249
(Some(user), Some(group)) => {
250-
change_user_and_group(path.to_owned(), user, group).await?;
250+
change_user_and_group(path, user, group).await?;
251251
}
252252
(Some(user), None) => {
253-
change_user(path.to_owned(), user).await?;
253+
change_user(path, user).await?;
254254
}
255255
(None, Some(group)) => {
256-
change_group(path.to_owned(), group).await?;
256+
change_group(path, group).await?;
257257
}
258258
(None, None) => {}
259259
}
@@ -329,10 +329,11 @@ impl PermissionEntry {
329329
/// Err(_) When it can not create the file with the appropriate owner and access permissions.
330330
async fn create_file(
331331
&self,
332-
file: &Path,
332+
file: impl AsRef<Path>,
333333
default_content: Option<&str>,
334334
) -> Result<(), FileError> {
335335
let mut options = fs::OpenOptions::new();
336+
let file = file.as_ref();
336337
match options.create_new(true).write(true).open(file).await {
337338
Ok(mut f) => {
338339
self.clone().apply(file).await?;
@@ -362,7 +363,8 @@ impl PermissionEntry {
362363
}
363364

364365
/// Overwrite the content of existing file. The file permissions will be kept.
365-
pub async fn overwrite_file(file: &Path, content: &str) -> Result<(), FileError> {
366+
pub async fn overwrite_file(file: impl AsRef<Path>, content: &str) -> Result<(), FileError> {
367+
let file = file.as_ref();
366368
match fs::OpenOptions::new()
367369
.write(true)
368370
.truncate(true)
@@ -393,13 +395,13 @@ pub async fn overwrite_file(file: &Path, content: &str) -> Result<(), FileError>
393395
}
394396

395397
pub async fn change_user_and_group(
396-
file: PathBuf,
397-
user: String,
398-
group: String,
398+
file: impl AsRef<Path>,
399+
user: impl Into<String>,
400+
group: impl Into<String>,
399401
) -> Result<(), FileError> {
400-
let file = file.to_owned();
401-
let user = user.to_owned();
402-
let group = group.to_owned();
402+
let file = file.as_ref().to_owned();
403+
let user = user.into();
404+
let group = group.into();
403405
tokio::task::spawn_blocking(move || change_user_and_group_sync(&file, &user, &group))
404406
.await
405407
.unwrap()
@@ -438,15 +440,18 @@ pub fn change_user_and_group_sync(path: &Path, user: &str, group: &str) -> Resul
438440
Ok(())
439441
}
440442

441-
async fn change_user(file: PathBuf, user: String) -> Result<(), FileError> {
443+
async fn change_user(file: impl Into<PathBuf>, user: impl Into<String>) -> Result<(), FileError> {
444+
let file = file.into();
445+
let user = user.into();
442446
tokio::task::spawn_blocking(move || change_user_sync(&file, &user))
443447
.await
444448
.unwrap()
445449
}
446450

447-
fn change_user_sync(file: &Path, user: &str) -> Result<(), FileError> {
451+
fn change_user_sync(file: impl AsRef<Path>, user: &str) -> Result<(), FileError> {
452+
let file = file.as_ref();
448453
let metadata = get_metadata_sync(file)?;
449-
let ud = get_user_by_name(&user)
454+
let ud = get_user_by_name(user)
450455
.map(|u| u.uid())
451456
.ok_or_else(|| FileError::UserNotFound { user: user.into() })?;
452457

@@ -466,24 +471,25 @@ fn change_user_sync(file: &Path, user: &str) -> Result<(), FileError> {
466471
async fn change_group(file: impl Into<PathBuf>, group: impl Into<String>) -> Result<(), FileError> {
467472
let file = file.into();
468473
let group = group.into();
469-
tokio::task::spawn_blocking(move || change_group_sync(file, group))
474+
tokio::task::spawn_blocking(move || change_group_sync(&file, &group))
470475
.await
471476
.unwrap()
472477
}
473478

474-
fn change_group_sync(file: impl Into<PathBuf>, group: impl Into<String>) -> Result<(), FileError> {
475-
let file = file.into();
476-
let group = group.into();
477-
let metadata = get_metadata_sync(&file)?;
478-
let gd = get_group_by_name(&group)
479+
fn change_group_sync(file: impl AsRef<Path>, group: &str) -> Result<(), FileError> {
480+
let file = file.as_ref();
481+
let metadata = get_metadata_sync(file)?;
482+
let gd = get_group_by_name(group)
479483
.map(|g| g.gid())
480-
.ok_or_else(|| FileError::GroupNotFound { group })?;
484+
.ok_or_else(|| FileError::GroupNotFound {
485+
group: group.to_owned(),
486+
})?;
481487

482488
let gid = metadata.gid();
483489

484490
// if group is same as existing, then do not change
485491
if gd != gid {
486-
chown(&file, None, Some(Gid::from_raw(gd))).map_err(|e| FileError::MetaDataError {
492+
chown(file, None, Some(Gid::from_raw(gd))).map_err(|e| FileError::MetaDataError {
487493
name: file.display().to_string(),
488494
from: e.into(),
489495
})?;
@@ -492,15 +498,16 @@ fn change_group_sync(file: impl Into<PathBuf>, group: impl Into<String>) -> Resu
492498
Ok(())
493499
}
494500

495-
pub async fn change_mode(file: &Path, mode: u32) -> Result<(), FileError> {
496-
let file = file.to_owned();
501+
pub async fn change_mode(file: impl AsRef<Path>, mode: u32) -> Result<(), FileError> {
502+
let file = file.as_ref().to_owned();
497503
tokio::task::spawn_blocking(move || change_mode_sync(&file, mode))
498504
.await
499505
.unwrap()
500506
}
501507

502-
pub fn change_mode_sync(file: &Path, mode: u32) -> Result<(), FileError> {
503-
let mut permissions = get_metadata_sync(Path::new(file))?.permissions();
508+
pub fn change_mode_sync(file: impl AsRef<Path>, mode: u32) -> Result<(), FileError> {
509+
let file = file.as_ref();
510+
let mut permissions = get_metadata_sync(file)?.permissions();
504511

505512
if permissions.mode() & 0o777 != mode {
506513
permissions.set_mode(mode);
@@ -519,23 +526,25 @@ pub fn change_mode_sync(file: &Path, mode: u32) -> Result<(), FileError> {
519526
}
520527

521528
/// Return metadata when the given path exists and accessible by user
522-
pub async fn get_metadata(path: &Path) -> Result<std::fs::Metadata, FileError> {
529+
pub async fn get_metadata(path: impl AsRef<Path>) -> Result<std::fs::Metadata, FileError> {
530+
let path = path.as_ref();
523531
fs::metadata(path)
524532
.await
525533
.map_err(|_| FileError::PathNotAccessible {
526534
path: path.to_path_buf(),
527535
})
528536
}
529537

530-
fn get_metadata_sync(path: &Path) -> Result<std::fs::Metadata, FileError> {
538+
fn get_metadata_sync(path: impl AsRef<Path>) -> Result<std::fs::Metadata, FileError> {
539+
let path = path.as_ref();
531540
std::fs::metadata(path).map_err(|_| FileError::PathNotAccessible {
532541
path: path.to_path_buf(),
533542
})
534543
}
535544

536545
/// Return filename if the given path contains a filename
537-
pub fn get_filename(path: PathBuf) -> Option<String> {
538-
let filename = path.file_name()?.to_str()?.to_string();
546+
pub fn get_filename(path: impl AsRef<Path>) -> Option<String> {
547+
let filename = path.as_ref().file_name()?.to_str()?.to_string();
539548
Some(filename)
540549
}
541550

@@ -679,9 +688,9 @@ mod tests {
679688
#[tokio::test]
680689
async fn create_directory_with_wrong_user() {
681690
let temp_dir = TempDir::new().unwrap();
682-
let dir_path = temp_dir.path().join("dir").display().to_string();
691+
let dir_path = temp_dir.path().join("dir");
683692

684-
let err = create_directory_with_user_group(dir_path, "nonexistent_user", &GROUP, 0o775)
693+
let err = create_directory_with_user_group(&dir_path, "nonexistent_user", &GROUP, 0o775)
685694
.await
686695
.unwrap_err();
687696

@@ -691,9 +700,9 @@ mod tests {
691700
#[tokio::test]
692701
async fn create_directory_with_wrong_group() {
693702
let temp_dir = TempDir::new().unwrap();
694-
let dir_path = temp_dir.path().join("dir").display().to_string();
703+
let dir_path = temp_dir.path().join("dir");
695704

696-
let err = create_directory_with_user_group(dir_path, &USER, "nonexistent_group", 0o775)
705+
let err = create_directory_with_user_group(&dir_path, &USER, "nonexistent_group", 0o775)
697706
.await
698707
.unwrap_err();
699708

crates/core/tedge/src/cli/certificate/create.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,9 +177,7 @@ async fn create_new_file(
177177
// Ignore errors - This was the behavior with the now deprecated user manager.
178178
// - When `tedge cert create` is not run as root, a certificate is created but owned by the user running the command.
179179
// - A better approach could be to remove this `chown` and run the command as mosquitto.
180-
let _ =
181-
tedge_utils::file::change_user_and_group(path.into(), user.to_string(), group.to_string())
182-
.await;
180+
let _ = tedge_utils::file::change_user_and_group(path, user, group).await;
183181

184182
Ok(file)
185183
}

crates/core/tedge/src/cli/connect/command.rs

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -924,10 +924,7 @@ pub async fn chown_certificate_and_key(bridge_config: &BridgeConfig) {
924924
// - When `tedge cert create` is not run as root, a certificate is created but owned by the user running the command.
925925
// - A better approach could be to remove this `chown` and run the command as mosquitto.
926926
let path = &bridge_config.bridge_certfile;
927-
if let Err(err) =
928-
tedge_utils::file::change_user_and_group(path.into(), user.to_owned(), group.to_owned())
929-
.await
930-
{
927+
if let Err(err) = tedge_utils::file::change_user_and_group(path, user, group).await {
931928
warn!("Failed to change ownership of {path} to {user}:{group}: {err}");
932929
}
933930

@@ -936,10 +933,7 @@ pub async fn chown_certificate_and_key(bridge_config: &BridgeConfig) {
936933
if !path.exists() {
937934
return;
938935
}
939-
if let Err(err) =
940-
tedge_utils::file::change_user_and_group(path.into(), user.to_owned(), group.to_owned())
941-
.await
942-
{
936+
if let Err(err) = tedge_utils::file::change_user_and_group(path, user, group).await {
943937
warn!("Failed to change ownership of {path} to {user}:{group}: {err}");
944938
}
945939
}

crates/core/tedge/src/cli/init.rs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -99,12 +99,7 @@ impl TEdgeInitCmd {
9999
let entity_store_file = config_dir.join(".agent").join("entity_store.jsonl");
100100

101101
if entity_store_file.exists() {
102-
change_user_and_group(
103-
entity_store_file.into(),
104-
self.user.to_string(),
105-
self.group.to_string(),
106-
)
107-
.await?;
102+
change_user_and_group(entity_store_file, &self.user, &self.group).await?;
108103
}
109104

110105
Ok(())

plugins/c8y_remote_access_plugin/src/lib.rs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -104,14 +104,10 @@ async fn declare_supported_operation(
104104
.context("Creating supported operations directory")?;
105105

106106
if supported_operation_path.exists() {
107-
change_user_and_group(
108-
supported_operation_path.into(),
109-
user.to_string(),
110-
group.to_string(),
111-
)
112-
.await
113-
.into_diagnostic()
114-
.context("Changing permissions of supported operations")
107+
change_user_and_group(&supported_operation_path, user, group)
108+
.await
109+
.into_diagnostic()
110+
.context("Changing permissions of supported operations")
115111
} else {
116112
create_file_with_user_group(
117113
supported_operation_path,

0 commit comments

Comments
 (0)