1.5.0-rc.3 (#186)

* chore: bumped version

* feat: added locks on evps

resolves #161

* style: reformatted

* chore: added fslock license

Author: lilopkins

Cargo.lock

@@ -1,7 +1,7 @@
 [package]
 name = "evidenceangel"
 description = "Library and executables to work with EvidenceAngel evidence packages (*.evp)."
-version = "1.5.0-rc.2"
+version = "1.5.0-rc.3"
 edition = "2024"
 license = "GPL-3.0-or-later"
 authors = [
@@ -70,6 +70,7 @@ colored = { version = "3.0.0", optional = true }
 directories = { version = "6.0.0", optional = true }
 fluent = { version = "0.16.1", optional = true }
 fluent-templates = { version = "0.13.0", optional = true }
+fslock = "0.2.1"
 getset = "0.1.2"
 html-escape = { version = "0.2.13", optional = true }
 infer = "0.19.0"

Cargo.toml

@@ -180,7 +180,9 @@ See an example <uuid>.json file in (#example-test-case).
 
 The "custom" field is used to add custom metadata that has been
 specified in the package manifest's "custom_test_case_metadata" field.
-All values **MUST** be strings.
+If a value is specified in "custom", it **MUST** be present in the
+package manifest, but all values in the package manifest do not need to
+be present here. All values **MUST** be strings.
 
 #### "evidence" Array Element {#test-case-evidence}
 
@@ -225,6 +227,24 @@ an additional media file.
 
 # Handling an Evidence Package
 
+## Locking
+
+When loading an evidence package, implemetors **MUST** use a lock file
+with the file name ".~" followed by the full name of the package it
+protects, for example for a package called "example.evp", the lock file
+**MUST** be called ".~example.evp". It **MUST** be located adjacent (in
+the same directory as) the evidence package.
+
+The lock file should be considered as locking the package if:
+
+* the lock file is present, and;
+* the lock file contains a process ID.
+
+If either of these is not the case, it should be assumed that the there
+is no current lock over the package.
+
+## Media Loading
+
 Software implementing the evidence package format **MUST NOT** load
 files from the "media" directory into memory until it is needed for
 display or for extraction. Implementors **MUST** use streams to load

schemas/draft-hopkins-evp-spec.md

@@ -38,6 +38,7 @@ impl SimpleComponent for AppAbout {
             add_legal_section: ("directories",              None, gtk::License::MitX11,    None),
             add_legal_section: ("fluent",                   None, gtk::License::MitX11,    None),
             add_legal_section: ("fluent-templates",         None, gtk::License::MitX11,    None),
+            add_legal_section: ("fslock",                   None, gtk::License::MitX11,    None),
             add_legal_section: ("getset",                   None, gtk::License::MitX11,    None),
             add_legal_section: ("html-escape",              None, gtk::License::MitX11,    None),
             add_legal_section: ("infer",                    None, gtk::License::MitX11,    None),

src/evidenceangel-ui/about.rs

@@ -83,7 +83,8 @@ fn main() {
     let cli = Args::parse();
 
     let app = RelmApp::new("uk.hpkns.EvidenceAngel");
-    relm4::main_application().set_flags(ApplicationFlags::HANDLES_OPEN);
+    relm4::main_application()
+        .set_flags(ApplicationFlags::HANDLES_OPEN | ApplicationFlags::NON_UNIQUE);
     relm4::main_application().connect_open(|_app, _files, _hint| {
         // nothing to do, this is handled by clap...
     });

src/evidenceangel-ui/main.rs

@@ -95,30 +95,30 @@ impl fmt::Debug for EvidencePackage {
 impl EvidencePackage {
     /// Create a new evidence package.
     ///
-    /// # Panics
-    ///
-    /// All the potential panics are checked statically ahead of time, so should never trigger at runtime.
-    ///
     /// # Errors
     ///
     /// - [`Error::Io`] if the evp couldn't be written at all.
     /// - [`Error::Zip`] if the evp file couldn't be written correctly.
     /// - [`Error::ManifestSchemaValidationFailed`] if the manifest is invalid.
+    #[allow(
+        clippy::missing_panics_doc,
+        reason = "panics have been statically validated to never occur"
+    )]
     pub fn new(path: PathBuf, title: String, authors: Vec<Author>) -> Result<Self> {
         Self::new_with_description(path, title, None, authors)
     }
 
     /// Create a new evidence package with a specified description.
     ///
-    /// # Panics
-    ///
-    /// All the potential panics are checked statically ahead of time, so should never trigger at runtime.
-    ///
     /// # Errors
     ///
     /// - [`Error::Io`] if the evp couldn't be written at all.
     /// - [`Error::Zip`] if the evp file couldn't be written correctly.
     /// - [`Error::ManifestSchemaValidationFailed`] if the manifest is invalid.
+    #[allow(
+        clippy::missing_panics_doc,
+        reason = "panics have been statically validated to never occur"
+    )]
     pub fn new_with_description(
         path: PathBuf,
         title: String,
@@ -127,7 +127,7 @@ impl EvidencePackage {
     ) -> Result<Self> {
         // Create manifest data.
         let mut manifest = Self {
-            zip: ZipReaderWriter::new(path),
+            zip: ZipReaderWriter::new(path)?,
             media_data: HashMap::new(),
             test_case_data: HashMap::new(),
 
@@ -307,7 +307,7 @@ impl EvidencePackage {
     /// - [`Error::TestCaseSchemaValidationFailed`] if one of the test case manifests fails schema validation.
     pub fn open(path: PathBuf) -> Result<Self> {
         // Open ZIP file
-        let mut zip_rw = ZipReaderWriter::new(path);
+        let mut zip_rw = ZipReaderWriter::new(path)?;
         let zip = zip_rw.as_reader()?;
 
         // Read manifest
@@ -382,7 +382,7 @@ impl EvidencePackage {
     /// Clone fields that will be serialized by serde
     fn clone_serde(&self) -> Self {
         Self {
-            zip: ZipReaderWriter::new(PathBuf::new()),
+            zip: ZipReaderWriter::default(),
             media_data: HashMap::new(),
             test_case_data: HashMap::new(),
 

src/package.rs

@@ -4,6 +4,14 @@ use uuid::Uuid;
 /// An error raised by `EvidenceAngel`.
 #[derive(Debug, Error)]
 pub enum Error {
+    /// A file lock couldn't be obtained over the package.
+    #[error("Failed to obtain a lock over the package: {0}")]
+    Locking(std::io::Error),
+
+    /// You are trying to perform an operation without a lock on the package.
+    #[error("The file you are working with is already open. Please close it and try again.")]
+    LockNotObtained,
+
     /// An I/O error from the system.
     #[error("I/O Error: {0}")]
     Io(#[from] std::io::Error),

src/result.rs

@@ -4,18 +4,20 @@ use std::{
     path,
 };
 
-use getset::Setters;
+use fslock::LockFile;
 use zip::{ZipArchive, ZipWriter};
 
 /// A convenient type which can read and write to a ZIP file and cleanly switch between the two modes.
 ///
 /// Whilst writing, you can also read the previous file, as it writes to a new temporary file, until
 /// [`ZipReaderWriter::conclude_write`] is called.
-#[derive(Default, Setters)]
+#[derive(Default)]
 pub(crate) struct ZipReaderWriter {
     /// The path of this zip file, if set
-    #[getset(set = "pub")]
-    file: Option<path::PathBuf>,
+    path: Option<path::PathBuf>,
+    /// The locking file for this evidence package. If this is [`Some`],
+    /// you can be assured that the lock is obtained.
+    lock_file: Option<LockFile>,
     /// The reader, if in write mode, of this reader/writer
     reader: Option<ZipArchive<BufReader<fs::File>>>,
     /// The writer, if in write mode, of this reader/writer
@@ -25,7 +27,7 @@ pub(crate) struct ZipReaderWriter {
 impl Clone for ZipReaderWriter {
     fn clone(&self) -> Self {
         Self {
-            file: self.file.clone(),
+            path: self.path.clone(),
             ..Default::default()
         }
     }
@@ -41,19 +43,46 @@ impl fmt::Debug for ZipReaderWriter {
             "idle"
         };
         f.debug_struct("ZipReadWriter")
-            .field("file", &self.file)
+            .field("file", &self.path)
             .field("current_mode", &mode)
-            .finish()
+            .finish_non_exhaustive()
     }
 }
 
 impl ZipReaderWriter {
     /// Create a new [`ZipReaderWriter`] instance.
-    pub fn new(path: path::PathBuf) -> Self {
-        Self {
-            file: Some(path),
+    pub fn new(path: path::PathBuf) -> crate::Result<Self> {
+        let mut o = Self {
+            path: Some(path),
             ..Default::default()
+        };
+        o.update_lock_file()?;
+        Ok(o)
+    }
+
+    /// Update the locking file for this [`ZipReaderWriter`]. This will
+    /// either obtain it (if a path is set), drop it (if a path isn't
+    /// set), or will return a [`crate::Error::Locking`] error.
+    fn update_lock_file(&mut self) -> crate::Result<()> {
+        if let Some(path) = &self.path {
+            let mut lock_path = path.clone();
+            // SAFETY: only a file can be specified here
+            lock_path.set_file_name(format!(
+                ".~{}",
+                lock_path.file_name().unwrap().to_str().unwrap()
+            ));
+            let mut lock_file = LockFile::open(&lock_path).map_err(crate::Error::Locking)?;
+            if !lock_file
+                .try_lock_with_pid()
+                .map_err(crate::Error::Locking)?
+            {
+                return Err(crate::Error::LockNotObtained);
+            }
+            self.lock_file = Some(lock_file);
+        } else {
+            self.lock_file = None;
         }
+        Ok(())
     }
 
     /// Get this [`ZipReaderWriter`] instance in read mode.
@@ -66,7 +95,7 @@ impl ZipReaderWriter {
             // Open reader
             tracing::debug!("Opening reader");
             self.reader = Some(ZipArchive::new(BufReader::new(fs::File::open(
-                self.file
+                self.path
                     .as_ref()
                     .expect("zipreadwriter must not be called upon until file is set."),
             )?))?);
@@ -82,11 +111,14 @@ impl ZipReaderWriter {
         Option<&mut ZipArchive<BufReader<fs::File>>>,
         &mut ZipWriter<BufWriter<fs::File>>,
     )> {
+        if self.lock_file.is_none() {
+            return Err(crate::Error::LockNotObtained);
+        }
         if self.writer.is_none() {
             tracing::debug!("Opening writer");
             // Open writer
             self.writer = Some(ZipWriter::new(BufWriter::new(fs::File::create(
-                self.file
+                self.path
                     .as_ref()
                     .map(|p| {
                         let mut p = p.clone();
@@ -104,6 +136,9 @@ impl ZipReaderWriter {
 
     /// Conclude writing to the ZIP file and reset for reading or writing again.
     pub fn conclude_write(&mut self) -> crate::Result<()> {
+        if self.lock_file.is_none() {
+            return Err(crate::Error::LockNotObtained);
+        }
         if self.writer.is_some() {
             // Close write
             tracing::debug!("Closing writer");
@@ -116,7 +151,7 @@ impl ZipReaderWriter {
             // Move temp file
             tracing::debug!("Moving temp file to overwrite package");
             let tmp_path = self
-                .file
+                .path
                 .as_ref()
                 .map(|p| {
                     let mut p = p.clone();
@@ -126,7 +161,7 @@ impl ZipReaderWriter {
                 .expect("zipreadwriter must not be called upon until file is set.");
             fs::rename(
                 tmp_path,
-                self.file
+                self.path
                     .as_ref()
                     .expect("zipreadwriter must not be called upon until file is set."),
             )?;
@@ -136,6 +171,9 @@ impl ZipReaderWriter {
 
     /// Interrupt a write early, concluding the write and removing the temporary file.
     pub fn interrupt_write(&mut self) -> crate::Result<()> {
+        if self.lock_file.is_none() {
+            return Err(crate::Error::LockNotObtained);
+        }
         if self.writer.is_some() {
             // Close write
             tracing::debug!("Closing writer");
@@ -148,7 +186,7 @@ impl ZipReaderWriter {
             // Delete temp file
             tracing::debug!("Moving temp file to overwrite package");
             let tmp_path = self
-                .file
+                .path
                 .as_ref()
                 .map(|p| {
                     let mut p = p.clone();