diff options
author | Lars Wirzenius <liw@liw.fi> | 2022-05-10 05:51:39 +0000 |
---|---|---|
committer | Lars Wirzenius <liw@liw.fi> | 2022-05-10 05:51:39 +0000 |
commit | 9d93e77ed08b086b705f3c6d52e6f936dc337ade (patch) | |
tree | b04294a85b4bcfb540107e39490018e8f42d5a6f | |
parent | 0b6adba069cd1300079c822b14832d690595cfb4 (diff) | |
parent | 4913347201f4d00ccaf959c53357241d5bc3f9e0 (diff) | |
download | obnam2-9d93e77ed08b086b705f3c6d52e6f936dc337ade.tar.gz |
Merge branch 'liw/integer-test' into 'main'
add tests for storing max integers (change FileId to signed)
Closes #188
See merge request obnam/obnam!231
-rw-r--r-- | src/cmd/restore.rs | 2 | ||||
-rw-r--r-- | src/cmd/show_gen.rs | 5 | ||||
-rw-r--r-- | src/db.rs | 39 | ||||
-rw-r--r-- | src/dbgen.rs | 6 | ||||
-rw-r--r-- | src/fsentry.rs | 174 | ||||
-rw-r--r-- | src/generation.rs | 29 |
6 files changed, 199 insertions, 56 deletions
diff --git a/src/cmd/restore.rs b/src/cmd/restore.rs index 4a637da..223d481 100644 --- a/src/cmd/restore.rs +++ b/src/cmd/restore.rs @@ -297,7 +297,7 @@ fn path_to_cstring(path: &Path) -> CString { fn create_progress_bar(file_count: FileId, verbose: bool) -> ProgressBar { let progress = if verbose { - ProgressBar::new(file_count) + ProgressBar::new(file_count as u64) } else { ProgressBar::hidden() }; diff --git a/src/cmd/show_gen.rs b/src/cmd/show_gen.rs index 98c57fc..f47a07b 100644 --- a/src/cmd/show_gen.rs +++ b/src/cmd/show_gen.rs @@ -3,6 +3,7 @@ use crate::chunk::ClientTrust; use crate::client::BackupClient; use crate::config::ClientConfig; +use crate::db::DbInt; use crate::error::ObnamError; use crate::fsentry::FilesystemKind; use crate::generation::GenId; @@ -66,7 +67,7 @@ impl ShowGeneration { #[derive(Debug, Default, Serialize)] struct Output { generation_id: String, - file_count: u64, + file_count: DbInt, file_bytes: String, file_bytes_raw: u64, db_bytes: String, @@ -81,7 +82,7 @@ impl Output { } } - fn file_count(mut self, n: u64) -> Self { + fn file_count(mut self, n: DbInt) -> Self { self.file_count = n; self } @@ -353,13 +353,16 @@ impl Column { } } +/// Type of plain integers that can be stored. +pub type DbInt = i64; + /// A value in a named column. #[derive(Debug)] pub enum Value<'a> { /// An integer primary key. - PrimaryKey(&'a str, u64), + PrimaryKey(&'a str, DbInt), /// An integer. - Int(&'a str, u64), + Int(&'a str, DbInt), /// A text string. Text(&'a str, &'a str), /// A binary string. @@ -381,12 +384,12 @@ impl<'a> Value<'a> { } /// Create an integer primary key value. - pub fn primary_key(name: &'a str, value: u64) -> Self { + pub fn primary_key(name: &'a str, value: DbInt) -> Self { Self::PrimaryKey(name, value) } /// Create an integer value. - pub fn int(name: &'a str, value: u64) -> Self { + pub fn int(name: &'a str, value: DbInt) -> Self { Self::Int(name, value) } @@ -406,6 +409,7 @@ impl<'a> Value<'a> { } } +#[allow(clippy::useless_conversion)] impl<'a> ToSql for Value<'a> { // The trait defines to_sql to return a Result. However, for our // particular case, to_sql can't ever fail. We only store values @@ -438,9 +442,9 @@ impl<'a> ToSql for Value<'a> { /// Like a Value, but owns the data. pub enum OwnedValue { /// An integer primary key. - PrimaryKey(String, u64), + PrimaryKey(String, DbInt), /// An integer. - Int(String, u64), + Int(String, DbInt), /// A text string. Text(String, String), /// A binary string. @@ -462,6 +466,7 @@ impl From<&Value<'_>> for OwnedValue { } impl ToSql for OwnedValue { + #[allow(clippy::useless_conversion)] fn to_sql(&self) -> rusqlite::Result<rusqlite::types::ToSqlOutput> { use rusqlite::types::Value; let v = match self { @@ -547,7 +552,7 @@ mod test { use std::path::Path; use tempfile::tempdir; - fn get_bar(row: &rusqlite::Row) -> Result<u64, rusqlite::Error> { + fn get_bar(row: &rusqlite::Row) -> Result<DbInt, rusqlite::Error> { row.get("bar") } @@ -566,12 +571,12 @@ mod test { Database::open(file).unwrap() } - fn insert(db: &mut Database, value: u64) { + fn insert(db: &mut Database, value: DbInt) { let table = table(); db.insert(&table, &[Value::int("bar", value)]).unwrap(); } - fn values(db: Database) -> Vec<u64> { + fn values(db: Database) -> Vec<DbInt> { let table = table(); let mut rows = db.all_rows(&table, &get_bar).unwrap(); let iter = rows.iter().unwrap(); @@ -606,7 +611,7 @@ mod test { #[test] fn inserts_many_rows() { - const N: u64 = 1000; + const N: DbInt = 1000; let tmp = tempdir().unwrap(); let filename = tmp.path().join("test.db"); @@ -618,7 +623,7 @@ mod test { let db = open_db(&filename); let values = values(db); - assert_eq!(values.len() as u64, N); + assert_eq!(values.len() as DbInt, N); let mut expected = vec![]; for i in 0..N { @@ -626,4 +631,16 @@ mod test { } assert_eq!(values, expected); } + #[test] + fn round_trips_int_max() { + let tmp = tempdir().unwrap(); + let filename = tmp.path().join("test.db"); + let mut db = create_db(&filename); + insert(&mut db, DbInt::MAX); + db.close().unwrap(); + + let db = open_db(&filename); + let values = values(db); + assert_eq!(values, vec![DbInt::MAX]); + } } diff --git a/src/dbgen.rs b/src/dbgen.rs index 8e5ece5..0053d4a 100644 --- a/src/dbgen.rs +++ b/src/dbgen.rs @@ -2,7 +2,7 @@ use crate::backup_reason::Reason; use crate::chunkid::ChunkId; -use crate::db::{Column, Database, DatabaseError, SqlResults, Table, Value}; +use crate::db::{Column, Database, DatabaseError, DbInt, SqlResults, Table, Value}; use crate::fsentry::FilesystemEntry; use crate::genmeta::{GenerationMeta, GenerationMetaError}; use crate::label::LabelChecksumKind; @@ -28,8 +28,8 @@ pub const DEFAULT_SCHEMA_MAJOR: VersionComponent = V0_0::MAJOR; /// Major schema versions supported by this version of Obnam. pub const SCHEMA_MAJORS: &[VersionComponent] = &[0, 1]; -/// An identifier for a file in a generation. -pub type FileId = u64; +/// An integer identifier for a file in a generation. +pub type FileId = DbInt; /// Possible errors from using generation databases. #[derive(Debug, thiserror::Error)] diff --git a/src/fsentry.rs b/src/fsentry.rs index 90afd70..276e3f9 100644 --- a/src/fsentry.rs +++ b/src/fsentry.rs @@ -76,43 +76,16 @@ impl FilesystemEntry { cache: &mut UsersCache, ) -> Result<Self, FsEntryError> { let kind = FilesystemKind::from_file_type(meta.file_type()); - let symlink_target = if kind == FilesystemKind::Symlink { - debug!("reading symlink target for {:?}", path); - let target = - read_link(path).map_err(|err| FsEntryError::ReadLink(path.to_path_buf(), err))?; - Some(target) - } else { - None - }; - - let uid = meta.st_uid(); - let gid = meta.st_gid(); - let user: String = if let Some(user) = cache.get_user_by_uid(uid) { - user.name().to_string_lossy().to_string() - } else { - "".to_string() - }; - let group = if let Some(group) = cache.get_group_by_gid(gid) { - group.name().to_string_lossy().to_string() - } else { - "".to_string() - }; - - Ok(Self { - path: path.to_path_buf().into_os_string().into_vec(), - kind: FilesystemKind::from_file_type(meta.file_type()), - len: meta.len(), - mode: meta.st_mode(), - mtime: meta.st_mtime(), - mtime_ns: meta.st_mtime_nsec(), - atime: meta.st_atime(), - atime_ns: meta.st_atime_nsec(), - symlink_target, - uid, - gid, - user, - group, - }) + Ok(EntryBuilder::new(kind) + .path(path.to_path_buf()) + .len(meta.len()) + .mode(meta.st_mode()) + .mtime(meta.st_mtime(), meta.st_mtime_nsec()) + .atime(meta.st_atime(), meta.st_atime_nsec()) + .user(meta.st_uid(), cache)? + .group(meta.st_uid(), cache)? + .symlink_target()? + .build()) } /// Return the kind of file the entry refers to. @@ -167,6 +140,133 @@ impl FilesystemEntry { } } +#[derive(Debug)] +pub(crate) struct EntryBuilder { + kind: FilesystemKind, + path: PathBuf, + len: u64, + + // 16 bits should be enough for a Unix mode_t. + // https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_stat.h.html + // However, it's 32 bits on Linux, so that's what we store. + mode: u32, + + // Linux can store file system time stamps in nanosecond + // resolution. We store them as two 64-bit integers. + mtime: i64, + mtime_ns: i64, + atime: i64, + atime_ns: i64, + + // The target of a symbolic link, if any. + symlink_target: Option<PathBuf>, + + // User and group owning the file. We store them as both the + // numeric id and the textual name corresponding to the numeric id + // at the time of the backup. + uid: u32, + gid: u32, + user: String, + group: String, +} + +impl EntryBuilder { + pub(crate) fn new(kind: FilesystemKind) -> Self { + Self { + kind, + path: PathBuf::new(), + len: 0, + mode: 0, + mtime: 0, + mtime_ns: 0, + atime: 0, + atime_ns: 0, + symlink_target: None, + uid: 0, + user: "".to_string(), + gid: 0, + group: "".to_string(), + } + } + + pub(crate) fn build(self) -> FilesystemEntry { + FilesystemEntry { + kind: self.kind, + path: self.path.into_os_string().into_vec(), + len: self.len, + mode: self.mode, + mtime: self.mtime, + mtime_ns: self.mtime_ns, + atime: self.atime, + atime_ns: self.atime_ns, + symlink_target: self.symlink_target, + uid: self.uid, + user: self.user, + gid: self.gid, + group: self.group, + } + } + + pub(crate) fn path(mut self, path: PathBuf) -> Self { + self.path = path; + self + } + + pub(crate) fn len(mut self, len: u64) -> Self { + self.len = len; + self + } + + pub(crate) fn mode(mut self, mode: u32) -> Self { + self.mode = mode; + self + } + + pub(crate) fn mtime(mut self, secs: i64, nsec: i64) -> Self { + self.mtime = secs; + self.mtime_ns = nsec; + self + } + + pub(crate) fn atime(mut self, secs: i64, nsec: i64) -> Self { + self.atime = secs; + self.atime_ns = nsec; + self + } + + pub(crate) fn symlink_target(mut self) -> Result<Self, FsEntryError> { + self.symlink_target = if self.kind == FilesystemKind::Symlink { + debug!("reading symlink target for {:?}", self.path); + let target = read_link(&self.path) + .map_err(|err| FsEntryError::ReadLink(self.path.clone(), err))?; + Some(target) + } else { + None + }; + Ok(self) + } + + pub(crate) fn user(mut self, uid: u32, cache: &mut UsersCache) -> Result<Self, FsEntryError> { + self.uid = uid; + self.user = if let Some(user) = cache.get_user_by_uid(uid) { + user.name().to_string_lossy().to_string() + } else { + "".to_string() + }; + Ok(self) + } + + pub(crate) fn group(mut self, gid: u32, cache: &mut UsersCache) -> Result<Self, FsEntryError> { + self.gid = gid; + self.group = if let Some(group) = cache.get_group_by_gid(gid) { + group.name().to_string_lossy().to_string() + } else { + "".to_string() + }; + Ok(self) + } +} + /// Different types of file system entries. #[derive(Debug, Copy, Clone, PartialEq, Serialize, Deserialize)] pub enum FilesystemKind { diff --git a/src/generation.rs b/src/generation.rs index 180efbe..477edc0 100644 --- a/src/generation.rs +++ b/src/generation.rs @@ -296,8 +296,33 @@ impl LocalGeneration { #[cfg(test)] mod test { - use super::{LabelChecksumKind, LocalGeneration, NascentGeneration, SchemaVersion}; - use tempfile::NamedTempFile; + use super::{LabelChecksumKind, LocalGeneration, NascentGeneration, Reason, SchemaVersion}; + use crate::fsentry::EntryBuilder; + use crate::fsentry::FilesystemKind; + use std::path::PathBuf; + use tempfile::{tempdir, NamedTempFile}; + + #[test] + fn round_trips_u64_max() { + let tmp = tempdir().unwrap(); + let filename = tmp.path().join("test.db"); + let path = PathBuf::from("/"); + let schema = SchemaVersion::new(0, 0); + { + let e = EntryBuilder::new(FilesystemKind::Directory) + .path(path.clone()) + .len(u64::MAX) + .build(); + let mut gen = + NascentGeneration::create(&filename, schema, LabelChecksumKind::Sha256).unwrap(); + gen.insert(e, &[], Reason::IsNew, false).unwrap(); + gen.close().unwrap(); + } + + let db = LocalGeneration::open(&filename).unwrap(); + let e = db.get_file(&path).unwrap().unwrap(); + assert_eq!(e.len(), u64::MAX); + } #[test] fn empty() { |