Proposed changes from review

Signed-off-by: simonsan <14062932+simonsan@users.noreply.github.com>

Author: simonsan

crates/core/src/backend.rs

@@ -358,7 +358,15 @@ pub trait WriteBackend: ReadBackend {
     /// If the file could not be read.
     fn lock(&self, tpe: FileType, id: &Id, until: Option<DateTime<Local>>) -> RusticResult<()> {
         debug!("no locking implemented. {tpe:?}, {id}, {until:?}");
-        Ok(())
+
+        if self.can_lock() {
+            unimplemented!("Using default implementation. No locking implemented in backend.");
+        } else {
+            Err(RusticError::new(
+                ErrorKind::Backend,
+                "No locking configured on backend.",
+            ))
+        }
     }
 }
 

crates/core/src/backend/cache.rs

@@ -233,6 +233,13 @@ impl WriteBackend for CachedBackend {
     }
 
     fn lock(&self, tpe: FileType, id: &Id, until: Option<DateTime<Local>>) -> RusticResult<()> {
+        if !self.can_lock() {
+            return Err(RusticError::new(
+                ErrorKind::Backend,
+                "No locking configured on backend.",
+            ));
+        }
+
         self.be.lock(tpe, id, until)
     }
 }

crates/core/src/backend/decrypt.rs

@@ -643,6 +643,13 @@ impl<C: CryptoKey> WriteBackend for DecryptBackend<C> {
     }
 
     fn lock(&self, tpe: FileType, id: &Id, until: Option<DateTime<Local>>) -> RusticResult<()> {
+        if !self.can_lock() {
+            return Err(RusticError::new(
+                ErrorKind::Backend,
+                "No locking configured on backend.",
+            ));
+        }
+
         self.be.lock(tpe, id, until)
     }
 }

crates/core/src/backend/dry_run.rs

@@ -171,6 +171,12 @@ impl<BE: DecryptFullBackend> WriteBackend for DryRunBackend<BE> {
     }
 
     fn lock(&self, tpe: FileType, id: &Id, until: Option<DateTime<Local>>) -> RusticResult<()> {
+        if !self.can_lock() {
+            return Err(RusticError::new(
+                ErrorKind::Backend,
+                "No locking configured on backend.",
+            ));
+        }
         self.be.lock(tpe, id, until)
     }
 }

crates/core/src/backend/hotcold.rs

@@ -4,6 +4,7 @@ use bytes::Bytes;
 use chrono::{DateTime, Local};
 
 use crate::{
+    ErrorKind, RusticError,
     backend::{FileType, ReadBackend, WriteBackend},
     error::RusticResult,
     id::Id,
@@ -105,6 +106,12 @@ impl WriteBackend for HotColdBackend {
     }
 
     fn lock(&self, tpe: FileType, id: &Id, until: Option<DateTime<Local>>) -> RusticResult<()> {
+        if !self.can_lock() {
+            return Err(RusticError::new(
+                ErrorKind::Backend,
+                "No locking configured on backend.",
+            ));
+        }
         self.be.lock(tpe, id, until)
     }
 }

crates/core/src/backend/lock.rs

@@ -2,7 +2,7 @@ use std::{process::Command, sync::Arc};
 
 use bytes::Bytes;
 use chrono::{DateTime, Local};
-use log::{debug, warn};
+use log::debug;
 
 use crate::{
     CommandInput, ErrorKind, RusticError, RusticResult,
@@ -67,7 +67,7 @@ impl ReadBackend for LockBackend {
     }
 }
 
-fn path(tpe: FileType, id: &Id) -> String {
+fn path_to_id_from_file_type(tpe: FileType, id: &Id) -> String {
     let hex_id = id.to_hex();
     match tpe {
         FileType::Config => "config".into(),
@@ -94,15 +94,26 @@ impl WriteBackend for LockBackend {
     }
 
     fn lock(&self, tpe: FileType, id: &Id, until: Option<DateTime<Local>>) -> RusticResult<()> {
+        if !self.can_lock() {
+            return Err(RusticError::new(
+                ErrorKind::Backend,
+                "No locking configured on backend.",
+            ));
+        }
+
         let until = until.map_or_else(String::new, |u| u.to_rfc3339());
-        let path = path(tpe, id);
+
+        let path = path_to_id_from_file_type(tpe, id);
+
         let args = self.command.args().iter().map(|c| {
             c.replace("%id", &id.to_hex())
                 .replace("%type", tpe.dirname())
                 .replace("%path", &path)
                 .replace("%until", &until)
         });
+
         debug!("calling {:?}...", self.command);
+
         let status = Command::new(self.command.command())
             .args(args)
             .status()
@@ -115,9 +126,17 @@ impl WriteBackend for LockBackend {
                 .attach_context("tpe", tpe.to_string())
                 .attach_context("id", id.to_string())
             })?;
+
         if !status.success() {
-            warn!("lock command was not successful for {tpe:?}, id: {id}. {status}");
+            return Err(RusticError::new(
+                ErrorKind::Backend,
+                "lock command was not successful for {tpe}, id: {id}. {status}",
+            )
+            .attach_context("tpe", tpe.to_string())
+            .attach_context("id", id.to_string())
+            .attach_context("status", status.to_string()));
         }
+
         Ok(())
     }
 }

crates/core/src/commands/lock.rs

@@ -35,8 +35,8 @@ pub fn lock_all_files<P: ProgressBars, S, ID: RepoId + std::fmt::Debug>(
 ) -> RusticResult<()> {
     if !repo.be.can_lock() {
         return Err(RusticError::new(
-            ErrorKind::Internal,
-            "Tried to call lock_all_files on a backend which isn't able to lock.",
+            ErrorKind::Backend,
+            "No locking configured on backend.",
         ));
     }
 
@@ -71,9 +71,9 @@ fn lock_files<P: ProgressBars, S, ID: RepoId + std::fmt::Debug>(
     pool.in_place_scope(|scope| {
         for id in ids {
             scope.spawn(move |_| {
-                if let Err(e) = backend.lock(ID::TYPE, id, until) {
-                    // FIXME: Use error handling
-                    error!("lock failed for {:?} {id:?}. {e}", ID::TYPE);
+                if let Err(err) = backend.lock(ID::TYPE, id, until) {
+                    // FIXME: Use error handling, e.g. use a channel to collect the errors
+                    error!("lock failed for {:?} {id:?}. {err}", ID::TYPE);
                 }
                 p.inc(1);
             });