Skip to content

Commit dea46ec

Browse files
committed
SMB: Route file ops through smb2 via SmbVolume
Very exciting! And it works! - Add `SmbVolume` implementing `Volume` trait, backed by direct smb2 protocol I/O - "Sneaky mount": OS mount stays for Finder/Terminal compat, Cmdr ops use smb2 (~4x faster) - `Mutex<Option<(SmbClient, Tree)>>` with `Handle::block_on` bridging (MtpVolume pattern) - `ConnectionState` (Direct/OsMount/Disconnected) with atomic lock-free reads - `local_path()` returns `None` so copy strategy uses smb2 streaming, not OS mount - Add `on_unmount()` to `Volume` trait for cleanup on external eject - Add `register_if_absent()` to `VolumeManager` to prevent watcher overwriting SmbVolume - Watcher calls `on_unmount()` before unregistering, uses `register_if_absent` for mounts - Mount command establishes parallel smb2 session, registers `SmbVolume` (best-effort) - 25 unit tests for type mappings, error classification, state machine, path conversion
1 parent 945093b commit dea46ec

11 files changed

Lines changed: 1229 additions & 5 deletions

File tree

apps/desktop/src-tauri/src/commands/network.rs

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,23 +264,100 @@ use crate::network::mount::{self, MountError, MountResult};
264264
/// provided, they are used for authentication. If the share is already mounted,
265265
/// returns the existing mount path without re-mounting.
266266
///
267+
/// After a successful OS mount, also establishes a direct smb2 connection and
268+
/// registers the share as an `SmbVolume` in the `VolumeManager`. This means
269+
/// Cmdr's own file operations go through smb2 (fast), while Finder/Terminal
270+
/// use the OS mount (compatible). If smb2 connection fails, the volume falls
271+
/// through to a regular `LocalPosixVolume` (registered by the watcher).
272+
///
267273
/// # Arguments
268274
/// * `server` - Server hostname or IP address
269275
/// * `share` - Name of the share to mount
270276
/// * `username` - Optional username for authentication
271277
/// * `password` - Optional password for authentication
278+
/// * `port` - SMB port (default 445)
272279
/// * `timeout_ms` - Optional timeout in milliseconds (default: 20000)
273280
///
274281
/// # Returns
275282
/// * `Ok(MountResult)` - Mount successful, with path to mount point
276283
/// * `Err(MountError)` - Mount failed with specific error type
277284
#[tauri::command]
285+
#[allow(
286+
clippy::too_many_arguments,
287+
reason = "Tauri command requires all parameters to be top-level"
288+
)]
278289
pub async fn mount_network_share(
279290
server: String,
280291
share: String,
281292
username: Option<String>,
282293
password: Option<String>,
294+
port: Option<u16>,
283295
timeout_ms: Option<u64>,
284296
) -> Result<MountResult, MountError> {
285-
mount::mount_share(server, share, username, password, timeout_ms).await
297+
let result = mount::mount_share(
298+
server.clone(),
299+
share.clone(),
300+
username.clone(),
301+
password.clone(),
302+
timeout_ms,
303+
)
304+
.await?;
305+
306+
// Try to establish a direct smb2 connection and register as SmbVolume.
307+
// If this fails, the FSEvents watcher will register a LocalPosixVolume
308+
// as fallback (slower but still functional).
309+
register_smb_volume(
310+
&server,
311+
&share,
312+
&result.mount_path,
313+
username.as_deref(),
314+
password.as_deref(),
315+
port.unwrap_or(445),
316+
)
317+
.await;
318+
319+
Ok(result)
320+
}
321+
322+
/// Tries to establish a direct smb2 connection and register as `SmbVolume`.
323+
///
324+
/// Best-effort: logs a warning and returns quietly on failure. The FSEvents
325+
/// watcher will register a `LocalPosixVolume` as fallback.
326+
async fn register_smb_volume(
327+
server: &str,
328+
share: &str,
329+
mount_path: &str,
330+
username: Option<&str>,
331+
password: Option<&str>,
332+
port: u16,
333+
) {
334+
use crate::file_system::get_volume_manager;
335+
use crate::file_system::volume::smb::connect_smb_volume;
336+
use std::sync::Arc;
337+
338+
log::debug!(
339+
"Establishing smb2 connection for SmbVolume: {}:{}/{}",
340+
server,
341+
port,
342+
share
343+
);
344+
345+
match connect_smb_volume(share, mount_path, server, share, username, password, port).await {
346+
Ok(volume) => {
347+
let volume_id = crate::volumes::path_to_id(mount_path);
348+
// Use register (overwrite) so SmbVolume always wins over any
349+
// LocalPosixVolume the watcher may have registered in the race window.
350+
get_volume_manager().register(&volume_id, Arc::new(volume));
351+
log::info!("Registered SmbVolume for {} (id={})", mount_path, volume_id);
352+
}
353+
Err(e) => {
354+
log::warn!(
355+
"Failed to establish smb2 connection for {}/{}: {}. \
356+
Falling back to LocalPosixVolume via OS mount.",
357+
server,
358+
share,
359+
e
360+
);
361+
}
362+
}
286363
}

apps/desktop/src-tauri/src/file_system/mod.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ pub use listing::get_paths_at_indices;
3636
#[cfg(any(target_os = "macos", target_os = "linux"))]
3737
#[allow(unused_imports, reason = "Public API re-exports for future use")]
3838
pub use volume::MtpVolume;
39+
#[cfg(any(target_os = "macos", target_os = "linux"))]
40+
#[allow(unused_imports, reason = "Public API re-exports for future use")]
41+
pub use volume::SmbVolume;
3942
#[allow(unused_imports, reason = "Public API re-exports for future use")]
4043
pub use volume::manager::VolumeManager;
4144
#[allow(unused_imports, reason = "Public API re-exports for future use")]

apps/desktop/src-tauri/src/file_system/volume/CLAUDE.md

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ Every file system operation (listing, copy, rename, delete, indexing, watching)
1414
| `manager.rs` | `VolumeManager` — thread-safe `RwLock<HashMap>` registry; supports a default volume |
1515
| `local_posix.rs` | `LocalPosixVolume` — real filesystem; delegates listing to `file_system::listing`, indexing to `indexing::scanner`, watching to `indexing::watcher` (FSEvents), copy scanning via `walkdir`. Uses `libc::statvfs` FFI for space info. |
1616
| `mtp.rs` | `MtpVolume` — MTP device storage; synchronous `Volume` trait bridged to async MTP calls via `tokio::runtime::Handle::block_on`. Gated with `#[cfg(any(target_os = "macos", target_os = "linux"))]`. |
17+
| `smb.rs` | `SmbVolume` — SMB share storage; synchronous `Volume` trait bridged to async smb2 calls via `Handle::block_on`. Uses `Mutex<Option<(SmbClient, Tree)>>` + `AtomicU8` connection state. Gated with `#[cfg(any(target_os = "macos", target_os = "linux"))]`. |
1718
| `in_memory.rs` | `InMemoryVolume``RwLock<HashMap>` store for tests; also used for stress tests (`with_file_count`) |
1819

1920
## Architecture
@@ -23,6 +24,7 @@ VolumeManager (registry)
2324
└─ Arc<dyn Volume>
2425
├─ LocalPosixVolume → real FS, FSEvents watcher, jwalk scanner
2526
├─ MtpVolume → async MTP ops via block_on (spawn_blocking context)
27+
├─ SmbVolume → async smb2 ops via block_on (direct protocol, not OS mount)
2628
└─ InMemoryVolume → HashMap, test/stress use only
2729
```
2830

@@ -35,7 +37,8 @@ Optional methods default to `Err(VolumeError::NotSupported)` or `false`, so new
3537
- `supports_watching()` — enables the `notify`-based *listing* file watcher in `operations.rs` (separate from the `VolumeWatcher` trait used for drive indexing). `MtpVolume` returns `false` (it has its own USB event loop).
3638
- `supports_export()` — enables copy/move UI. Both local and MTP return `true`.
3739
- `supports_streaming()` — enables chunked MTP-to-MTP transfers. Only `MtpVolume` returns `true`.
38-
- `local_path()` — returns `Some` only for local volumes; allows `copyfile(2)` fast-path in copy operations.
40+
- `local_path()` — returns `Some` only for local volumes; allows `copyfile(2)` fast-path in copy operations. `SmbVolume` returns `None` so copies go through smb2 instead of the slow OS mount.
41+
- `on_unmount()` — lifecycle hook called before unregistration. `SmbVolume` uses it to disconnect its smb2 session. Default is no-op.
3942
- `scanner()` / `watcher()` — drive indexing hooks; `None` by default.
4043

4144
## Path handling gotchas
@@ -63,6 +66,9 @@ Optional methods default to `Err(VolumeError::NotSupported)` or `false`, so new
6366
**Decision**: `VolumeManager` uses `RwLock<HashMap>` (not `DashMap` or `Mutex`)
6467
**Why**: Volume registration/unregistration is rare (mount/unmount events); reads are frequent (every file operation resolves a volume). `RwLock` gives concurrent read access without pulling in an extra dependency. `DashMap` would work but is heavier than needed for a registry that rarely exceeds ~10 entries.
6568

69+
**Decision**: `VolumeManager::register_if_absent` for watcher registrations
70+
**Why**: When the mount flow pre-registers an `SmbVolume`, the FSEvents watcher would overwrite it with a `LocalPosixVolume` via `register`. `register_if_absent` is a no-op if a volume is already registered, preserving the `SmbVolume`. The existing `register` (overwrite) is kept for explicit replacement (like SmbVolume replacing itself on reconnect).
71+
6672
**Decision**: `MtpVolume` bridges sync `Volume` trait to async MTP calls via `Handle::block_on`
6773
**Why**: The `Volume` trait is synchronous because local filesystem ops are blocking and shouldn't touch the async executor. MTP operations are inherently async (USB bulk transfers), so `block_on` bridges the gap. This is safe because MTP methods are always called from `spawn_blocking` contexts (separate OS thread pool), avoiding nested-runtime panics.
6874

@@ -83,10 +89,20 @@ Optional methods default to `Err(VolumeError::NotSupported)` or `false`, so new
8389
**Gotcha**: `MtpVolume::get_metadata` returns `NotSupported`
8490
**Why**: MTP has no single-file stat call — you must list the parent directory and search for the entry by name. The listing pipeline doesn't use `get_metadata` during normal browsing (it gets metadata from `list_directory` results), so implementing it would add an expensive round-trip for a code path that's currently unused.
8591

92+
**Decision**: `SmbVolume` uses `Mutex<Option<(SmbClient, Tree)>>`, not `RwLock`
93+
**Why**: Every `SmbClient` method takes `&mut self` — there is no read-only access path. An `RwLock` where you only ever take write locks is strictly worse than a `Mutex` (higher overhead). The `Option` allows graceful cleanup on disconnect (set to `None`).
94+
95+
**Decision**: `SmbVolume::local_path()` returns `None`
96+
**Why**: `local_path()` is checked in `volume_copy.rs` to decide whether to use native OS copy APIs. If SmbVolume returned `Some(mount_path)`, copies would go through the slow OS mount — exactly what we're trying to avoid. `root()` still returns the mount path for frontend path resolution.
97+
98+
**Decision**: `on_unmount()` trait method instead of `Any` downcasting
99+
**Why**: Avoids runtime type checking, extensible for future volume types (S3, FTP might also need cleanup), consistent with the trait's design of optional methods with default no-ops.
100+
86101
## Testing
87102

88103
- `in_memory_test.rs` — unit tests for `InMemoryVolume` (CRUD, sorting, concurrency, stress 50k entries)
89104
- `inmemory_test.rs` — integration tests combining `InMemoryVolume` + `VolumeManager`, streaming state, sort helpers
90105
- `local_posix_test.rs` — real-FS tests (write ops, symlinks, copy, space info) using `std::env::temp_dir()`
91106
- `manager.rs` inline tests — concurrent registration/read/write-mix scenarios
92107
- `mtp.rs` inline tests — path conversion and capability flags (no device needed)
108+
- `smb.rs` inline tests — type mapping (DirectoryEntry→FileEntry, FsInfo→SpaceInfo, Error→VolumeError), connection state transitions, path conversion, capability flags (no server needed)

apps/desktop/src-tauri/src/file_system/volume/manager.rs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,25 @@ impl VolumeManager {
3333
}
3434
}
3535

36+
/// Registers a volume only if no volume with this ID exists yet.
37+
///
38+
/// Returns `true` if the volume was registered, `false` if a volume
39+
/// with this ID already exists (the existing volume is kept).
40+
pub fn register_if_absent(&self, id: &str, volume: Arc<dyn Volume>) -> bool {
41+
if let Ok(mut volumes) = self.volumes.write() {
42+
use std::collections::hash_map::Entry;
43+
match volumes.entry(id.to_string()) {
44+
Entry::Occupied(_) => false,
45+
Entry::Vacant(e) => {
46+
e.insert(volume);
47+
true
48+
}
49+
}
50+
} else {
51+
false
52+
}
53+
}
54+
3655
/// Unregisters a volume by ID.
3756
///
3857
/// If this was the default volume, the default is cleared.
@@ -185,6 +204,29 @@ mod tests {
185204
assert!(list.iter().any(|(id, name)| id == "vol2" && name == "Volume Two"));
186205
}
187206

207+
#[test]
208+
fn test_register_if_absent_new_volume() {
209+
let manager = VolumeManager::new();
210+
let volume = Arc::new(InMemoryVolume::new("Test Volume"));
211+
212+
assert!(manager.register_if_absent("test", volume));
213+
assert_eq!(manager.count(), 1);
214+
assert_eq!(manager.get("test").unwrap().name(), "Test Volume");
215+
}
216+
217+
#[test]
218+
fn test_register_if_absent_existing_volume_keeps_original() {
219+
let manager = VolumeManager::new();
220+
let original = Arc::new(InMemoryVolume::new("Original"));
221+
let replacement = Arc::new(InMemoryVolume::new("Replacement"));
222+
223+
manager.register("test", original);
224+
assert!(!manager.register_if_absent("test", replacement));
225+
226+
// Original should be kept
227+
assert_eq!(manager.get("test").unwrap().name(), "Original");
228+
}
229+
188230
#[test]
189231
fn test_multiple_volumes() {
190232
let manager = VolumeManager::new();

apps/desktop/src-tauri/src/file_system/volume/mod.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,16 @@ pub trait Volume: Send + Sync {
250250
Err(VolumeError::NotSupported)
251251
}
252252

253+
// ========================================
254+
// Lifecycle: Optional, default no-op
255+
// ========================================
256+
257+
/// Called when the volume is about to be unmounted/unregistered.
258+
///
259+
/// Implementations can use this to clean up resources (disconnect network
260+
/// sessions, cancel background tasks, etc.). Default is a no-op.
261+
fn on_unmount(&self) {}
262+
253263
// ========================================
254264
// Watching: Optional, default no-op
255265
// ========================================
@@ -370,11 +380,15 @@ mod local_posix;
370380
pub(crate) mod manager;
371381
#[cfg(any(target_os = "macos", target_os = "linux"))]
372382
mod mtp;
383+
#[cfg(any(target_os = "macos", target_os = "linux"))]
384+
pub(crate) mod smb;
373385

374386
pub use in_memory::InMemoryVolume;
375387
pub use local_posix::LocalPosixVolume;
376388
#[cfg(any(target_os = "macos", target_os = "linux"))]
377389
pub use mtp::MtpVolume;
390+
#[cfg(any(target_os = "macos", target_os = "linux"))]
391+
pub use smb::SmbVolume;
378392

379393
// Re-export types defined in this module for convenience
380394
// (they're already public since defined in mod.rs)

0 commit comments

Comments
 (0)