|
| 1 | +# Volume filesystem abstraction plan |
| 2 | + |
| 3 | +**Date**: 2025-12-31 |
| 4 | +**Status**: ✅ **FULLY IMPLEMENTED** (completed 2025-12-31, 80 tests passing) |
| 5 | +**Goal**: Abstract file system access to enable multiple volume types and testability |
| 6 | + |
| 7 | +## Summary |
| 8 | + |
| 9 | +Introduce a `Volume` trait that abstracts file system operations. This enables: |
| 10 | + |
| 11 | +1. **LocalPosixVolume** - Real file system access (with configurable root path) |
| 12 | +2. **InMemoryVolume** - In-memory file system for testing |
| 13 | + |
| 14 | +Future implementations (out of scope): NetworkShareVolume (SMB/AFP), S3Volume, SftpVolume. |
| 15 | + |
| 16 | +## Design decisions |
| 17 | + |
| 18 | +| # | Decision | Choice | |
| 19 | +| --- | ----------- | ------------------------------------------------------------------- | |
| 20 | +| 1 | Terminology | **Volume** (macOS-native) | |
| 21 | +| 2 | Trait scope | **Read required + optional write** (defaults return `NotSupported`) | |
| 22 | +| 3 | Root path | **In the instance** (each Volume has a root) | |
| 23 | +| 4 | Path type | **`&Path`** (standard Rust, converted internally) | |
| 24 | +| 5 | TestFS | **In-memory** (supports create/delete for realistic tests) | |
| 25 | +| 6 | Watcher | **Default no-op** in trait, LocalPosix overrides | |
| 26 | +| 7 | API pattern | **Provider owns the API** (methods on Volume) | |
| 27 | +| 8 | FileEntry | **Keep as-is** (use `None` for unsupported fields) | |
| 28 | + |
| 29 | +## Architecture |
| 30 | + |
| 31 | +### Before (current) |
| 32 | + |
| 33 | +``` |
| 34 | +commands/file_system.rs |
| 35 | + └─> file_system/operations.rs |
| 36 | + └─> std::fs (direct calls) |
| 37 | + └─> LISTING_CACHE (static) |
| 38 | +``` |
| 39 | + |
| 40 | +### After |
| 41 | + |
| 42 | +``` |
| 43 | +commands/file_system.rs |
| 44 | + └─> VolumeManager (holds active volumes) |
| 45 | + └─> dyn Volume (trait object) |
| 46 | + ├── LocalPosixVolume (wraps std::fs) |
| 47 | + └── InMemoryVolume (for tests) |
| 48 | +``` |
| 49 | + |
| 50 | +## New files and modules |
| 51 | + |
| 52 | +``` |
| 53 | +src-tauri/src/file_system/ |
| 54 | +├── mod.rs # Updated: exports new types |
| 55 | +├── volume/ |
| 56 | +│ ├── mod.rs # Volume trait + error types |
| 57 | +│ ├── local_posix.rs # LocalPosixVolume implementation |
| 58 | +│ └── in_memory.rs # InMemoryVolume for testing |
| 59 | +├── operations.rs # Refactored: uses Volume trait |
| 60 | +├── watcher.rs # Minor updates for Volume integration |
| 61 | +└── (other existing files) |
| 62 | +``` |
| 63 | + |
| 64 | +## Trait design |
| 65 | + |
| 66 | +```rust |
| 67 | +/// Error type for volume operations |
| 68 | +#[derive(Debug, Clone)] |
| 69 | +pub enum VolumeError { |
| 70 | + NotFound(String), |
| 71 | + PermissionDenied(String), |
| 72 | + NotSupported, |
| 73 | + IoError(String), |
| 74 | +} |
| 75 | + |
| 76 | +/// Core trait for volume file system operations |
| 77 | +pub trait Volume: Send + Sync { |
| 78 | + /// Returns the display name for this volume (e.g., "Macintosh HD", "Dropbox") |
| 79 | + fn name(&self) -> &str; |
| 80 | + |
| 81 | + /// Returns the root path of this volume |
| 82 | + fn root(&self) -> &Path; |
| 83 | + |
| 84 | + // ======================================== |
| 85 | + // Required: All volumes must implement |
| 86 | + // ======================================== |
| 87 | + |
| 88 | + /// Lists directory contents at the given path (relative to volume root) |
| 89 | + fn list_directory(&self, path: &Path) -> Result<Vec<FileEntry>, VolumeError>; |
| 90 | + |
| 91 | + /// Gets metadata for a single path (relative to volume root) |
| 92 | + fn get_metadata(&self, path: &Path) -> Result<FileEntry, VolumeError>; |
| 93 | + |
| 94 | + /// Checks if a path exists (relative to volume root) |
| 95 | + fn exists(&self, path: &Path) -> bool; |
| 96 | + |
| 97 | + // ======================================== |
| 98 | + // Optional: Default to NotSupported |
| 99 | + // ======================================== |
| 100 | + |
| 101 | + /// Creates a file with the given content |
| 102 | + fn create_file(&self, path: &Path, content: &[u8]) -> Result<(), VolumeError> { |
| 103 | + let _ = (path, content); |
| 104 | + Err(VolumeError::NotSupported) |
| 105 | + } |
| 106 | + |
| 107 | + /// Creates a directory |
| 108 | + fn create_directory(&self, path: &Path) -> Result<(), VolumeError> { |
| 109 | + let _ = path; |
| 110 | + Err(VolumeError::NotSupported) |
| 111 | + } |
| 112 | + |
| 113 | + /// Deletes a file or empty directory |
| 114 | + fn delete(&self, path: &Path) -> Result<(), VolumeError> { |
| 115 | + let _ = path; |
| 116 | + Err(VolumeError::NotSupported) |
| 117 | + } |
| 118 | + |
| 119 | + // ======================================== |
| 120 | + // Watching: Optional, default no-op |
| 121 | + // ======================================== |
| 122 | + |
| 123 | + /// Returns true if this volume supports file watching |
| 124 | + fn supports_watching(&self) -> bool { |
| 125 | + false |
| 126 | + } |
| 127 | + |
| 128 | + /// Starts watching a directory (no-op by default) |
| 129 | + fn start_watching( |
| 130 | + &self, |
| 131 | + _path: &Path, |
| 132 | + _callback: Box<dyn Fn() + Send + Sync>, |
| 133 | + ) -> Result<(), VolumeError> { |
| 134 | + Ok(()) // No-op default |
| 135 | + } |
| 136 | + |
| 137 | + /// Stops watching (no-op by default) |
| 138 | + fn stop_watching(&self, _path: &Path) {} |
| 139 | +} |
| 140 | +``` |
| 141 | + |
| 142 | +## Implementation details |
| 143 | + |
| 144 | +### LocalPosixVolume |
| 145 | + |
| 146 | +- Wraps real `std::fs` operations |
| 147 | +- Constructor takes `name` and `root_path` |
| 148 | +- Implements all required methods by calling existing `operations.rs` code |
| 149 | +- Overrides `supports_watching()` → `true` |
| 150 | +- Integrates with existing `notify-debouncer-full` watcher |
| 151 | + |
| 152 | +**Key**: The existing `list_directory_core()` and related functions will be refactored to take paths relative to a root, |
| 153 | +or the Volume will convert paths before calling them. |
| 154 | + |
| 155 | +### InMemoryVolume |
| 156 | + |
| 157 | +- Stores entries in a `HashMap<PathBuf, InMemoryEntry>` |
| 158 | +- Supports all optional methods (create, delete) |
| 159 | +- Useful for testing file operations without disk I/O |
| 160 | +- Can simulate large directories (50k+ files) for stress tests |
| 161 | + |
| 162 | +```rust |
| 163 | +struct InMemoryEntry { |
| 164 | + metadata: FileEntry, |
| 165 | + content: Option<Vec<u8>>, // None for directories |
| 166 | + children: Vec<String>, // For directories |
| 167 | +} |
| 168 | + |
| 169 | +pub struct InMemoryVolume { |
| 170 | + name: String, |
| 171 | + root: PathBuf, |
| 172 | + entries: RwLock<HashMap<PathBuf, InMemoryEntry>>, |
| 173 | +} |
| 174 | +``` |
| 175 | + |
| 176 | +### VolumeManager |
| 177 | + |
| 178 | +A registry of available volumes, used by the command layer: |
| 179 | + |
| 180 | +```rust |
| 181 | +pub struct VolumeManager { |
| 182 | + volumes: RwLock<HashMap<String, Arc<dyn Volume>>>, |
| 183 | + default_volume_id: RwLock<Option<String>>, |
| 184 | +} |
| 185 | + |
| 186 | +impl VolumeManager { |
| 187 | + pub fn register(&self, id: &str, volume: Arc<dyn Volume>); |
| 188 | + pub fn get(&self, id: &str) -> Option<Arc<dyn Volume>>; |
| 189 | + pub fn default(&self) -> Option<Arc<dyn Volume>>; |
| 190 | +} |
| 191 | +``` |
| 192 | + |
| 193 | +### Migration strategy |
| 194 | + |
| 195 | +1. **Phase 1**: Create new `volume/` module with trait and implementations |
| 196 | +2. **Phase 2**: Refactor `operations.rs` to call through Volume trait internally |
| 197 | +3. **Phase 3**: Update `commands/file_system.rs` to use VolumeManager |
| 198 | +4. **Phase 4**: Migrate existing tests to use InMemoryVolume |
| 199 | +5. **Phase 5**: Remove old test-only `#[cfg(test)]` provider code |
| 200 | + |
| 201 | +## Tasks |
| 202 | + |
| 203 | +### Phase 1: Core abstraction (foundation) |
| 204 | + |
| 205 | +- [ ] **1.1** Create `file_system/volume/mod.rs` with `Volume` trait and `VolumeError` |
| 206 | +- [ ] **1.2** Create `file_system/volume/local_posix.rs` with `LocalPosixVolume` |
| 207 | + - Implement required methods: `list_directory`, `get_metadata`, `exists` |
| 208 | + - Call into existing `operations.rs` code initially |
| 209 | +- [ ] **1.3** Create `file_system/volume/in_memory.rs` with `InMemoryVolume` |
| 210 | + - Implement all methods including `create_file`, `create_directory`, `delete` |
| 211 | + - Add helper methods: `with_entries()`, `with_file_count()` for test setup |
| 212 | +- [ ] **1.4** Add unit tests for both implementations |
| 213 | + |
| 214 | +### Phase 2: Refactor operations |
| 215 | + |
| 216 | +- [ ] **2.1** Extract path resolution logic (joining root + relative path) |
| 217 | +- [ ] **2.2** Create `VolumeManager` struct |
| 218 | +- [ ] **2.3** Refactor `list_directory_start` to accept a Volume reference |
| 219 | +- [ ] **2.4** Update `LISTING_CACHE` to store which volume each listing belongs to |
| 220 | +- [ ] **2.5** Refactor `get_file_range`, `get_total_count`, etc. to work with volumes |
| 221 | + |
| 222 | +### Phase 3: Integrate watcher |
| 223 | + |
| 224 | +- [ ] **3.1** Add watcher methods to LocalPosixVolume |
| 225 | +- [ ] **3.2** Update `handle_directory_change` to use Volume for re-reading |
| 226 | +- [ ] **3.3** Test that file watching still works end-to-end |
| 227 | + |
| 228 | +### Phase 4: Update command layer |
| 229 | + |
| 230 | +- [ ] **4.1** Initialize VolumeManager in app setup (create "root" LocalPosixVolume) |
| 231 | +- [ ] **4.2** Update `list_directory_start` command to use default volume |
| 232 | +- [ ] **4.3** Add volume ID parameter to commands (optional, for future multi-volume) |
| 233 | + |
| 234 | +### Phase 5: Testing and cleanup |
| 235 | + |
| 236 | +- [ ] **5.1** Create integration tests using InMemoryVolume |
| 237 | +- [ ] **5.2** Migrate existing `mock_provider` tests to use InMemoryVolume |
| 238 | +- [ ] **5.3** Remove deprecated `#[cfg(test)]` provider code |
| 239 | +- [ ] **5.4** Run full check suite, fix any issues |
| 240 | +- [ ] **5.5** Manual testing: verify app still works normally |
| 241 | + |
| 242 | +## Out of scope (future) |
| 243 | + |
| 244 | +- Volume chooser UI |
| 245 | +- NetworkShareVolume, S3Volume, SftpVolume implementations |
| 246 | +- Multi-volume support in frontend |
| 247 | +- Volume configuration/settings UI |
| 248 | + |
| 249 | +## Risks and mitigations |
| 250 | + |
| 251 | +| Risk | Mitigation | |
| 252 | +| ------------------------------- | -------------------------------------------------------------- | |
| 253 | +| Breaking existing functionality | Each phase ends with working code, run checks after each | |
| 254 | +| Performance regression | LocalPosixVolume adds minimal overhead (one trait dispatch) | |
| 255 | +| Watcher complexity | Keep watcher logic mostly unchanged, just route through volume | |
| 256 | + |
| 257 | +## Success criteria |
| 258 | + |
| 259 | +1. All existing tests pass |
| 260 | +2. `./scripts/check.sh` passes |
| 261 | +3. App works exactly as before (manual test) |
| 262 | +4. New tests for InMemoryVolume demonstrate testability |
| 263 | +5. Code is ready for future volume types |
0 commit comments