Growth ring file handling improvements (#381)

growth-ring/Cargo.toml

@@ -26,6 +26,7 @@ hex = "0.4.3"
 rand = "0.8.5"
 indexmap = "2.0.0"
 tokio = { version = "1.28.1", features = ["tokio-macros", "rt", "macros"] }
+test-case = "3.1.0"
 
 [lib]
 name = "growthring"

growth-ring/src/lib.rs

@@ -48,8 +48,6 @@
 //! // After each recovery, the /tmp/walfiles is empty.
 //! ```
 
-#[macro_use]
-extern crate scan_fmt;
 pub mod wal;
 pub mod walerror;
 
@@ -175,7 +173,7 @@ pub fn oflags() -> OFlag {
 
 #[async_trait(?Send)]
 impl WalStore<WalFileImpl> for WalStoreImpl {
-    type FileNameIter = std::vec::IntoIter<String>;
+    type FileNameIter = std::vec::IntoIter<PathBuf>;
 
     async fn open_file(&self, filename: &str, _touch: bool) -> Result<WalFileImpl, WalError> {
         let path = self.root_dir.join(filename);
@@ -193,7 +191,7 @@ impl WalStore<WalFileImpl> for WalStoreImpl {
     fn enumerate_files(&self) -> Result<Self::FileNameIter, WalError> {
         let mut filenames = Vec::new();
         for path in fs::read_dir(&self.root_dir)?.filter_map(|entry| entry.ok()) {
-            filenames.push(path.file_name().into_string().unwrap());
+            filenames.push(path.path());
         }
         Ok(filenames.into_iter())
     }

growth-ring/src/wal.rs

@@ -9,20 +9,22 @@ use futures::{
     Future,
 };
 
-use std::cell::{RefCell, UnsafeCell};
 use std::convert::{TryFrom, TryInto};
 use std::mem::MaybeUninit;
 use std::num::NonZeroUsize;
 use std::pin::Pin;
+use std::{
+    cell::{RefCell, UnsafeCell},
+    ffi::OsStr,
+    path::{Path, PathBuf},
+};
 use std::{
     collections::{hash_map, BinaryHeap, HashMap, VecDeque},
     marker::PhantomData,
 };
 
 pub use crate::walerror::WalError;
 
-const FILENAME_FMT: &str = r"[0-9a-f]+\.log";
-
 enum WalRingType {
     #[allow(dead_code)]
     Null = 0x0,
@@ -59,8 +61,23 @@ type WalFileId = u64;
 pub type WalBytes = Box<[u8]>;
 pub type WalPos = u64;
 
-fn get_fid(fname: &str) -> WalFileId {
-    scan_fmt!(fname, "{x}.log", [hex WalFileId]).unwrap()
+// convert XXXXXX.log into number from the XXXXXX (in hex)
+fn get_fid(fname: &Path) -> Result<WalFileId, WalError> {
+    let wal_err: WalError = WalError::Other("not a log file".to_string());
+
+    if fname.extension() != Some(OsStr::new("log")) {
+        return Err(wal_err);
+    }
+
+    u64::from_str_radix(
+        fname
+            .file_stem()
+            .unwrap_or(OsStr::new(""))
+            .to_str()
+            .unwrap_or(""),
+        16,
+    )
+    .map_err(|_| wal_err)
 }
 
 fn get_fname(fid: WalFileId) -> String {
@@ -199,7 +216,7 @@ pub trait WalFile {
 
 #[async_trait(?Send)]
 pub trait WalStore<F: WalFile> {
-    type FileNameIter: Iterator<Item = String>;
+    type FileNameIter: Iterator<Item = PathBuf>;
 
     /// Open a file given the filename, create the file if not exists when `touch` is `true`.
     async fn open_file(&self, filename: &str, touch: bool) -> Result<F, WalError>;
@@ -729,7 +746,6 @@ impl<F: WalFile + 'static, S: WalStore<F>> WalWriter<F, S> {
         nrecords: usize,
         recover_policy: &RecoverPolicy,
     ) -> Result<Vec<WalBytes>, WalError> {
-        let filename_fmt = regex::Regex::new(FILENAME_FMT).unwrap();
         let file_pool = &self.file_pool;
         let file_nbit = file_pool.file_nbit;
         let block_size = 1 << file_pool.block_nbit;
@@ -740,8 +756,7 @@ impl<F: WalFile + 'static, S: WalStore<F>> WalWriter<F, S> {
             file_pool
                 .store
                 .enumerate_files()?
-                .filter(|f| filename_fmt.is_match(f))
-                .map(|s| get_fid(&s))
+                .flat_map(|s| get_fid(&s))
                 .collect(),
         );
 
@@ -1179,16 +1194,14 @@ impl WalLoader {
         let msize = std::mem::size_of::<WalRingBlob>();
         assert!(self.file_nbit > self.block_nbit);
         assert!(msize < 1 << self.block_nbit);
-        let filename_fmt = regex::Regex::new(FILENAME_FMT).unwrap();
         let mut file_pool =
             WalFilePool::new(store, self.file_nbit, self.block_nbit, self.cache_size).await?;
         let logfiles = sort_fids(
             self.file_nbit,
             file_pool
                 .store
                 .enumerate_files()?
-                .filter(|f| filename_fmt.is_match(f))
-                .map(|s| get_fid(&s))
+                .flat_map(|s| get_fid(&s))
                 .collect(),
         );
 
@@ -1301,3 +1314,25 @@ impl WalLoader {
 }
 
 pub const CRC32: crc::Crc<u32> = crc::Crc::<u32>::new(&crc::CRC_32_CKSUM);
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use test_case::test_case;
+
+    #[test_case("foo", Err("not a log file"); "no log extension")]
+    #[test_case("foo.log", Err("not a log file"); "invalid digit found in string")]
+    #[test_case("0000001.log", Ok(1); "happy path")]
+    #[test_case("1.log", Ok(1); "no leading zeroes")]
+
+    fn test_get_fid(input: &str, expected: Result<u64, &str>) {
+        let got = get_fid(Path::new(input));
+        match expected {
+            Err(has) => {
+                let err = got.err().unwrap().to_string();
+                assert!(err.contains(has), "{:?}", err)
+            }
+            Ok(val) => assert_eq!(got.unwrap(), val),
+        }
+    }
+}

growth-ring/tests/common/mod.rs

@@ -11,6 +11,7 @@ use std::cell::RefCell;
 use std::collections::VecDeque;
 use std::collections::{hash_map, HashMap};
 use std::convert::TryInto;
+use std::path::PathBuf;
 use std::rc::Rc;
 
 pub trait FailGen {
@@ -126,7 +127,7 @@ impl<'a, G> WalStore<WalFileEmul<G>> for WalStoreEmul<'a, G>
 where
     G: 'static + FailGen,
 {
-    type FileNameIter = std::vec::IntoIter<String>;
+    type FileNameIter = std::vec::IntoIter<PathBuf>;
 
     async fn open_file(&self, filename: &str, touch: bool) -> Result<WalFileEmul<G>, WalError> {
         if self.fgen.next_fail() {
@@ -175,7 +176,7 @@ where
         }
         let mut logfiles = Vec::new();
         for (fname, _) in self.state.borrow().files.iter() {
-            logfiles.push(fname.clone())
+            logfiles.push(fname.into())
         }
         Ok(logfiles.into_iter())
     }