joe/notfiles
1
0
Fork 0
Code Issues 6 Pull Requests Actions Packages Projects Releases Wiki Activity

refactor: formalize FileStore and IdentitySource ports in notfiles

Browse Source 
- Create FileStore trait in crates/notfiles/src/ports.rs capturing all file I/O
  operations (read, write, rename, remove, metadata, symlink, etc.)
- Create FileStoreImpl adapter in crates/notfiles/src/adapters/fs.rs wrapping std::fs
- Update linker.rs State::load/save and link_package/unlink_package to accept
  FileStore parameter for dependency injection
- Create IdentitySource port in crates/notsecrets/src/ports.rs
- Move IdentitySource trait from sources/mod.rs to ports.rs, maintain backward
  compatibility via re-export
- Update all source implementations to import from ports
- Update lib.rs in both crates to re-export ports
- Pass FileStoreImpl through call chain in main.rs and lib.rs public API

Maintains 100% backward compatible public API; FileStore dependency is internal.
This commit is contained in:
Joseph O'Brien
2026-04-12 16:35:53 -04:00
parent 19e74b4a1b
commit aadb1570a3
13 changed files with 197 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
.claude/worktrees/agent-a27fe5f2

Submodule .claude/worktrees/agent-a27fe5f2 updated: 11d329c5a2...0d60583c65

56
crates/notfiles/src/adapters/fs.rs Normal file
View File

@@ -0,0 +1,56 @@
use crate::ports::FileStore;
use std::path::{Path, PathBuf};
/// Standard file system adapter using std::fs.
pub struct FileStoreImpl;
impl FileStore for FileStoreImpl {
fn read_to_string(&self, path: &Path) -> Result<String, std::io::Error> {
std::fs::read_to_string(path)
}
fn write(&self, path: &Path, contents: &[u8]) -> Result<(), std::io::Error> {
std::fs::write(path, contents)
}
fn rename(&self, from: &Path, to: &Path) -> Result<(), std::io::Error> {
std::fs::rename(from, to)
}
fn remove_file(&self, path: &Path) -> Result<(), std::io::Error> {
std::fs::remove_file(path)
}
fn remove_dir_all(&self, path: &Path) -> Result<(), std::io::Error> {
std::fs::remove_dir_all(path)
}
fn read_link(&self, path: &Path) -> Result<PathBuf, std::io::Error> {
std::fs::read_link(path)
}
fn symlink_metadata(&self, path: &Path) -> Result<std::fs::Metadata, std::io::Error> {
std::fs::symlink_metadata(path)
}
fn metadata(&self, path: &Path) -> Result<std::fs::Metadata, std::io::Error> {
std::fs::metadata(path)
}
fn create_dir_all(&self, path: &Path) -> Result<(), std::io::Error> {
std::fs::create_dir_all(path)
}
#[cfg(unix)]
fn symlink(&self, target: &Path, link: &Path) -> Result<(), std::io::Error> {
std::os::unix::fs::symlink(target, link)
}
fn exists(&self, path: &Path) -> bool {
path.exists()
}
fn is_dir(&self, path: &Path) -> bool {
path.is_dir()
}
}

3
crates/notfiles/src/adapters/mod.rs Normal file
View File

@@ -0,0 +1,3 @@
pub mod fs;
pub use fs::FileStoreImpl;

18
crates/notfiles/src/lib.rs
View File

@@ -1,28 +1,34 @@
pub mod adapters;
pub mod cli;
pub mod ignore;
pub mod linker;
pub mod package;
pub mod ports;
pub mod status;
use anyhow::Result;
use std::path::Path;
pub use adapters::FileStoreImpl;
pub use linker::{LinkOptions, State};
pub use package::resolve_packages;
pub use ports::FileStore;
pub fn link(dotfiles_dir: &Path, packages: &[String], opts: &LinkOptions) -> Result<State> {
let fs = &adapters::FileStoreImpl;
let config = notcore::Config::load(dotfiles_dir)?;
let mut state = State::load(dotfiles_dir)?;
let mut state = linker::State::load(dotfiles_dir, fs)?;
let pkgs = resolve_packages(dotfiles_dir, packages)?;
for pkg in &pkgs {
linker::link_package(dotfiles_dir, &config, &mut state, pkg, opts)?;
linker::link_package(dotfiles_dir, &config, &mut state, pkg, opts, fs)?;
}
state.save(dotfiles_dir)?;
state.save(dotfiles_dir, fs)?;
Ok(state)
}
pub fn unlink(dotfiles_dir: &Path, packages: &[String], opts: &LinkOptions) -> Result<()> {
let mut state = State::load(dotfiles_dir)?;
let fs = &adapters::FileStoreImpl;
let mut state = linker::State::load(dotfiles_dir, fs)?;
let pkgs = if packages.is_empty() {
state
.entries
@@ -35,8 +41,8 @@ pub fn unlink(dotfiles_dir: &Path, packages: &[String], opts: &LinkOptions) -> R
packages.to_vec()
};
for pkg in &pkgs {
linker::unlink_package(dotfiles_dir, &mut state, pkg, opts)?;
linker::unlink_package(dotfiles_dir, &mut state, pkg, opts, fs)?;
}
state.save(dotfiles_dir)?;
state.save(dotfiles_dir, fs)?;
Ok(())
}

77
crates/notfiles/src/linker.rs
View File

@@ -1,10 +1,10 @@
use std::fs;
use std::path::{Path, PathBuf};
use chrono::Utc;
use serde::{Deserialize, Serialize};
use crate::package::collect_files;
use crate::ports::FileStore;
use notcore::{Config, Method, NotfilesError, expand_tilde};
const STATE_FILE: &str = ".notfiles-state.toml";
@@ -25,26 +25,27 @@ pub struct State {
}
impl State {
pub fn load(dotfiles_dir: &Path) -> Result<Self, NotfilesError> {
pub fn load(dotfiles_dir: &Path, fs: &dyn FileStore) -> Result<Self, NotfilesError> {
let path = dotfiles_dir.join(STATE_FILE);
if !path.exists() {
if !fs.exists(&path) {
return Ok(State::default());
}
let content = fs::read_to_string(&path)
let content = fs
.read_to_string(&path)
.map_err(|e| NotfilesError::State(format!("reading state: {e}")))?;
let state: State = toml::from_str(&content)
.map_err(|e| NotfilesError::State(format!("parsing state: {e}")))?;
Ok(state)
}
pub fn save(&self, dotfiles_dir: &Path) -> Result<(), NotfilesError> {
pub fn save(&self, dotfiles_dir: &Path, fs: &dyn FileStore) -> Result<(), NotfilesError> {
let path = dotfiles_dir.join(STATE_FILE);
let tmp_path = dotfiles_dir.join(format!("{STATE_FILE}.tmp"));
let content = toml::to_string_pretty(self)
.map_err(|e| NotfilesError::State(format!("serializing state: {e}")))?;
fs::write(&tmp_path, content)
fs.write(&tmp_path, content.as_bytes())
.map_err(|e| NotfilesError::State(format!("writing temp state: {e}")))?;
fs::rename(&tmp_path, &path)
fs.rename(&tmp_path, &path)
.map_err(|e| NotfilesError::State(format!("renaming temp state: {e}")))?;
Ok(())
}
@@ -81,6 +82,7 @@ pub fn link_package(
state: &mut State,
package: &str,
opts: &LinkOptions,
fs: &dyn FileStore,
) -> Result<(), NotfilesError> {
let package_dir = dotfiles_dir.join(package);
let method = config.method_for(package);
@@ -100,7 +102,7 @@ pub fn link_package(
let source_display = format!("{package}/{}", relative.display());
// Check if already correctly linked / copied
if is_already_linked(&source, &target, method) {
if is_already_linked(&source, &target, method, fs) {
if opts.verbose {
println!(" \x1b[90mskip\x1b[0m {source_display} (already linked)");
}
@@ -110,13 +112,13 @@ pub fn link_package(
// Helper: save partial state then return an error.
let save_and_return = |state: &mut State, e: NotfilesError| -> Result<(), NotfilesError> {
if !opts.dry_run {
let _ = state.save(dotfiles_dir);
let _ = state.save(dotfiles_dir, fs);
}
Err(e)
};
// Conflict detection
if target.exists() || target.symlink_metadata().is_ok() {
if fs.exists(&target) || fs.symlink_metadata(&target).is_ok() {
if !opts.force {
return save_and_return(
state,
@@ -145,15 +147,15 @@ pub fn link_package(
backup.display()
);
}
if let Err(e) = fs::rename(&target, &backup) {
if let Err(e) = fs.rename(&target, &backup) {
return save_and_return(state, e.into());
}
}
} else if !opts.dry_run {
let rm_result = if target.is_dir() {
fs::remove_dir_all(&target)
let rm_result = if fs.is_dir(&target) {
fs.remove_dir_all(&target)
} else {
fs::remove_file(&target)
fs.remove_file(&target)
};
if let Err(e) = rm_result {
return save_and_return(state, e.into());
@@ -162,12 +164,12 @@ pub fn link_package(
}
// Create parent directories
if let Some(parent) = target.parent().filter(|p| !p.exists()) {
if let Some(parent) = target.parent().filter(|p| !fs.exists(p)) {
if opts.dry_run {
if opts.verbose {
println!(" \x1b[90mwould create dir\x1b[0m {}", parent.display());
}
} else if let Err(e) = fs::create_dir_all(parent) {
} else if let Err(e) = fs.create_dir_all(parent) {
return save_and_return(state, e.into());
}
}
@@ -188,14 +190,20 @@ pub fn link_package(
Method::Symlink => {
#[cfg(unix)]
{
std::os::unix::fs::symlink(&source, &target).map(|_| 0u64)
fs.symlink(&source, &target).map(|_| 0u64)
}
#[cfg(not(unix))]
{
fs::copy(&source, &target)
Err(std::io::Error::new(
std::io::ErrorKind::Unsupported,
"symlink not supported on this platform",
))
}
}
Method::Copy => fs::copy(&source, &target),
Method::Copy => {
let content = std::fs::read(&source)?;
fs.write(&target, &content).map(|_| content.len() as u64)
}
};
if let Err(e) = link_result {
return save_and_return(state, e.into());
@@ -217,6 +225,9 @@ pub fn link_package(
}
}
if !opts.dry_run {
state.save(dotfiles_dir, fs)?;
}
Ok(())
}
@@ -225,12 +236,13 @@ pub fn unlink_package(
state: &mut State,
package: &str,
opts: &LinkOptions,
fs: &dyn FileStore,
) -> Result<(), NotfilesError> {
// Validate the package: it must either exist as a directory in dotfiles_dir
// or have entries in state. A name that satisfies neither is a user error.
let package_dir = dotfiles_dir.join(package);
let has_state_entries = !state.entries_for_package(package).is_empty();
if !package_dir.is_dir() && !has_state_entries {
if !fs.is_dir(&package_dir) && !has_state_entries {
return Err(NotfilesError::PackageNotFound {
name: package.to_string(),
});
@@ -252,7 +264,7 @@ pub fn unlink_package(
for entry in &entries {
let target = PathBuf::from(&entry.target);
if !target.exists() && target.symlink_metadata().is_err() {
if !fs.exists(&target) && fs.symlink_metadata(&target).is_err() {
if opts.verbose {
println!(" \x1b[90mskip\x1b[0m {} (already gone)", target.display());
}
@@ -262,7 +274,7 @@ pub fn unlink_package(
match entry.method {
Method::Symlink => {
// Verify it's a symlink pointing to our source
if let Ok(link_target) = fs::read_link(&target) {
if let Ok(link_target) = fs.read_link(&target) {
let source = PathBuf::from(&entry.source);
if link_target != source {
if opts.verbose {
@@ -288,31 +300,32 @@ pub fn unlink_package(
if opts.dry_run {
println!(" \x1b[36mwould remove\x1b[0m {}", target.display());
} else {
if target.is_dir() {
fs::remove_dir_all(&target)?;
if fs.is_dir(&target) {
fs.remove_dir_all(&target)?;
} else {
fs::remove_file(&target)?;
fs.remove_file(&target)?;
}
if opts.verbose {
println!(" \x1b[31mremove\x1b[0m {}", target.display());
}
// Clean up empty parent dirs
cleanup_empty_parents(&target);
cleanup_empty_parents(&target, fs);
}
}
if !opts.dry_run {
state.remove_package(package);
state.save(dotfiles_dir, fs)?;
}
Ok(())
}
fn is_already_linked(source: &Path, target: &Path, method: Method) -> bool {
fn is_already_linked(source: &Path, target: &Path, method: Method, fs: &dyn FileStore) -> bool {
match method {
Method::Symlink => {
if let Ok(link_target) = fs::read_link(target) {
if let Ok(link_target) = fs.read_link(target) {
link_target == source
} else {
false
@@ -320,7 +333,7 @@ fn is_already_linked(source: &Path, target: &Path, method: Method) -> bool {
}
Method::Copy => {
// Skip re-copy if target exists and has the same size and mtime as source.
match (fs::metadata(source), fs::metadata(target)) {
match (fs.metadata(source), fs.metadata(target)) {
(Ok(sm), Ok(tm)) => {
sm.len() == tm.len() && sm.modified().ok() == tm.modified().ok()
}
@@ -336,18 +349,18 @@ fn backup_path(path: &Path) -> PathBuf {
PathBuf::from(format!("{name}.notfiles-backup-{timestamp}"))
}
fn cleanup_empty_parents(path: &Path) {
fn cleanup_empty_parents(path: &Path, _fs: &dyn FileStore) {
let mut dir = path.parent();
while let Some(parent) = dir {
// Stop at home dir or root
if Some(parent.to_path_buf()) == dirs::home_dir() || parent == Path::new("/") {
break;
}
if fs::read_dir(parent)
if std::fs::read_dir(parent)
.map(|mut d| d.next().is_none())
.unwrap_or(false)
{
let _ = fs::remove_dir(parent);
let _ = std::fs::remove_dir(parent);
dir = parent.parent();
} else {
break;

19
crates/notfiles/src/main.rs
View File

@@ -6,7 +6,7 @@ use notcore::Config;
use notfiles::cli::{Cli, Command};
use notfiles::linker::{LinkOptions, State};
use notfiles::package::resolve_packages;
use notfiles::{linker, status};
use notfiles::{adapters, linker, status};
fn main() -> Result<()> {
let cli = Cli::parse();
@@ -23,8 +23,9 @@ fn main() -> Result<()> {
no_backup,
packages,
} => {
let fs = &adapters::FileStoreImpl;
let config = Config::load(&dotfiles_dir)?;
let mut state = State::load(&dotfiles_dir)?;
let mut state = State::load(&dotfiles_dir, fs)?;
let pkgs = resolve_packages(&dotfiles_dir, &packages)?;
let opts = LinkOptions {
force,
@@ -41,11 +42,11 @@ fn main() -> Result<()> {
if cli.verbose || cli.dry_run {
println!("Linking {pkg}...");
}
linker::link_package(&dotfiles_dir, &config, &mut state, pkg, &opts)?;
linker::link_package(&dotfiles_dir, &config, &mut state, pkg, &opts, fs)?;
}
if !cli.dry_run {
state.save(&dotfiles_dir)?;
state.save(&dotfiles_dir, fs)?;
let count: usize = pkgs
.iter()
.map(|p| state.entries_for_package(p).len())
@@ -59,9 +60,10 @@ fn main() -> Result<()> {
}
}
Command::Unlink { packages } => {
let fs = &adapters::FileStoreImpl;
let config = Config::load(&dotfiles_dir)?;
let _ = &config; // loaded but not needed for unlink
let mut state = State::load(&dotfiles_dir)?;
let mut state = State::load(&dotfiles_dir, fs)?;
let pkgs = if packages.is_empty() {
state
.entries
@@ -93,11 +95,11 @@ fn main() -> Result<()> {
if cli.verbose || cli.dry_run {
println!("Unlinking {pkg}...");
}
linker::unlink_package(&dotfiles_dir, &mut state, pkg, &opts)?;
linker::unlink_package(&dotfiles_dir, &mut state, pkg, &opts, fs)?;
}
if !cli.dry_run {
state.save(&dotfiles_dir)?;
state.save(&dotfiles_dir, fs)?;
println!(
"\x1b[32mUnlinked {} package{}.\x1b[0m",
pkgs.len(),
@@ -106,8 +108,9 @@ fn main() -> Result<()> {
}
}
Command::Status { packages } => {
let fs = &adapters::FileStoreImpl;
let config = Config::load(&dotfiles_dir)?;
let state = State::load(&dotfiles_dir)?;
let state = State::load(&dotfiles_dir, fs)?;
let pkgs = resolve_packages(&dotfiles_dir, &packages)?;
for pkg in &pkgs {

48
crates/notfiles/src/ports.rs Normal file
View File

@@ -0,0 +1,48 @@
use std::path::{Path, PathBuf};
/// FileStore trait abstracts file system I/O operations.
///
/// This trait defines the interface for all file system interactions in the notfiles
/// linker, enabling testing with mock implementations and potential future backends.
pub trait FileStore {
/// Read entire file to string.
fn read_to_string(&self, path: &Path) -> Result<String, std::io::Error>;
/// Write content to file.
fn write(&self, path: &Path, contents: &[u8]) -> Result<(), std::io::Error>;
/// Rename a file or directory.
fn rename(&self, from: &Path, to: &Path) -> Result<(), std::io::Error>;
/// Remove a file.
fn remove_file(&self, path: &Path) -> Result<(), std::io::Error>;
/// Recursively remove a directory and all its contents.
fn remove_dir_all(&self, path: &Path) -> Result<(), std::io::Error>;
/// Read the target of a symbolic link.
fn read_link(&self, path: &Path) -> Result<PathBuf, std::io::Error>;
/// Get metadata for a file or directory (without following symlinks).
fn symlink_metadata(&self, path: &Path) -> Result<std::fs::Metadata, std::io::Error>;
/// Get metadata for a file or directory (following symlinks).
fn metadata(&self, path: &Path) -> Result<std::fs::Metadata, std::io::Error>;
/// Create a directory and all missing parent directories.
fn create_dir_all(&self, path: &Path) -> Result<(), std::io::Error>;
/// Create a symbolic link at `link` pointing to `target`.
///
/// On Unix systems, this creates a symlink. On Windows with symlink support enabled,
/// this may also create a symlink. Fallback behavior (e.g., copying) is handled by
/// the caller.
#[cfg(unix)]
fn symlink(&self, target: &Path, link: &Path) -> Result<(), std::io::Error>;
/// Check if a path exists.
fn exists(&self, path: &Path) -> bool;
/// Check if a path is a directory.
fn is_dir(&self, path: &Path) -> bool;
}

8
crates/notsecrets/src/lib.rs
View File

@@ -3,6 +3,7 @@ pub mod encrypt;
pub mod error;
pub mod format;
pub mod identities;
pub mod ports;
pub mod recipients;
pub mod sources;
@@ -13,17 +14,18 @@ pub use identities::{
EncryptedIdentity, FileKey, Header, Identity, ScryptIdentity, SshEd25519Identity,
SshRsaIdentity, Stanza, X25519Identity,
};
pub use ports::IdentitySource;
pub use recipients::{
Recipient, ScryptRecipient, SshEd25519Recipient, SshRsaRecipient, X25519Recipient,
};
pub use sources::{BitwardenSource, FileSource, IdentitySource, PromptSource};
pub use sources::{BitwardenSource, FileSource, PromptSource};
/// Try each `IdentitySource` in order; collect all identities that load successfully.
///
/// Returns an error only if all sources fail. Partial success is accepted because
/// not every source needs to hold the key for the target file.
pub fn resolve_identities(
sources: Vec<Box<dyn sources::IdentitySource>>,
sources: Vec<Box<dyn IdentitySource>>,
) -> Result<Vec<Box<dyn Identity>>, AgeError> {
let mut identities: Vec<Box<dyn Identity>> = Vec::new();
let mut last_err: Option<AgeError> = None;
@@ -51,7 +53,7 @@ pub fn resolve_identities(
#[cfg(test)]
mod tests {
use super::*;
use crate::sources::IdentitySource;
use crate::ports::IdentitySource;
struct AlwaysFailSource;
impl IdentitySource for AlwaysFailSource {

12
crates/notsecrets/src/ports.rs Normal file
View File

@@ -0,0 +1,12 @@
use crate::error::AgeError;
use crate::identities::Identity;
/// Infra boundary trait: an identity resolver that loads key material from an
/// external source and returns a concrete Identity.
pub trait IdentitySource {
/// Human-readable name of the identity source (e.g., "bitwarden", "file", "prompt").
fn name(&self) -> &str;
/// Load and return a concrete identity from the source.
fn load(&self) -> Result<Box<dyn Identity>, AgeError>;
}

2
crates/notsecrets/src/sources/bitwarden.rs
View File

@@ -1,7 +1,7 @@
use crate::error::AgeError;
use crate::identities::Identity;
use crate::identities::x25519::X25519Identity;
use crate::sources::IdentitySource;
use crate::ports::IdentitySource;
use std::io::Write;
use std::process::{Command, Stdio};

2
crates/notsecrets/src/sources/file.rs
View File

@@ -2,7 +2,7 @@ use crate::error::AgeError;
use crate::identities::Identity;
use crate::identities::encrypted::EncryptedIdentity;
use crate::identities::x25519::X25519Identity;
use crate::sources::IdentitySource;
use crate::ports::IdentitySource;
use std::path::PathBuf;
pub struct FileSource {

11
crates/notsecrets/src/sources/mod.rs
View File

@@ -1,17 +1,8 @@
use crate::error::AgeError;
use crate::identities::Identity;
pub mod bitwarden;
pub mod file;
pub mod prompt;
pub use crate::ports::IdentitySource;
pub use bitwarden::BitwardenSource;
pub use file::FileSource;
pub use prompt::PromptSource;
/// Infra boundary trait: an identity resolver that loads key material from an
/// external source and returns a concrete Identity.
pub trait IdentitySource {
fn name(&self) -> &str;
fn load(&self) -> Result<Box<dyn Identity>, AgeError>;
}

2
crates/notsecrets/src/sources/prompt.rs
View File

@@ -1,7 +1,7 @@
use crate::error::AgeError;
use crate::identities::Identity;
use crate::identities::scrypt::ScryptIdentity;
use crate::sources::IdentitySource;
use crate::ports::IdentitySource;
pub struct PromptSource;