Proposed changes from review

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

Author: simonsan

crates/core/src/backend.rs

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

crates/core/src/backend/cache.rs

@@ -217,6 +217,10 @@ impl WriteBackend for CachedBackend {
     }
 
     fn lock(&self, tpe: FileType, id: &Id, until: Option<DateTime<Local>>) -> Result<()> {
+        if !self.can_lock() {
+            return Err(anyhow::anyhow!("No locking configured."));
+        }
+
         self.be.lock(tpe, id, until)
     }
 }

crates/core/src/backend/decrypt.rs

@@ -585,6 +585,10 @@ impl<C: CryptoKey> WriteBackend for DecryptBackend<C> {
     }
 
     fn lock(&self, tpe: FileType, id: &Id, until: Option<DateTime<Local>>) -> Result<()> {
+        if !self.can_lock() {
+            return Err(anyhow::anyhow!("No locking configured."));
+        }
+
         self.be.lock(tpe, id, until)
     }
 }

crates/core/src/backend/dry_run.rs

@@ -163,6 +163,10 @@ impl<BE: DecryptFullBackend> WriteBackend for DryRunBackend<BE> {
     }
 
     fn lock(&self, tpe: FileType, id: &Id, until: Option<DateTime<Local>>) -> Result<()> {
+        if !self.can_lock() {
+            return Err(anyhow::anyhow!("No locking configured."));
+        }
+
         self.be.lock(tpe, id, until)
     }
 }

crates/core/src/backend/hotcold.rs

@@ -105,6 +105,10 @@ impl WriteBackend for HotColdBackend {
     }
 
     fn lock(&self, tpe: FileType, id: &Id, until: Option<DateTime<Local>>) -> Result<()> {
+        if !self.can_lock() {
+            return Err(anyhow::anyhow!("No locking configured."));
+        }
+
         self.be.lock(tpe, id, until)
     }
 }

crates/core/src/backend/lock.rs

@@ -3,7 +3,7 @@ use std::{process::Command, sync::Arc};
 use anyhow::Result;
 use bytes::Bytes;
 use chrono::{DateTime, Local};
-use log::{debug, warn};
+use log::{debug, error};
 
 use crate::{
     backend::{FileType, ReadBackend, WriteBackend},
@@ -68,7 +68,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(),
@@ -95,19 +95,33 @@ impl WriteBackend for LockBackend {
     }
 
     fn lock(&self, tpe: FileType, id: &Id, until: Option<DateTime<Local>>) -> Result<()> {
+        if !self.can_lock() {
+            return Err(anyhow::anyhow!("No locking configured."));
+        }
+
         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()?;
+
         if !status.success() {
-            warn!("lock command was not successful for {tpe:?}, id: {id}. {status}");
+            error!("lock command was not successful for {tpe:?}, id: {id}. {status}");
+
+            return Err(anyhow::anyhow!(
+                "lock command was not successful for {tpe:?}, id: {id}. {status}"
+            ));
         }
+
         Ok(())
     }
 }

crates/core/src/commands/lock.rs

@@ -62,9 +62,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);
             });