From 5d1aa8aaa36ce11b54fcfb4dfe3cf895565d4edf Mon Sep 17 00:00:00 2001 From: Jamie Pine Date: Tue, 18 Nov 2025 01:52:33 -0800 Subject: [PATCH] fix: update volume tracking and visibility logic - Marked volumes as user-visible based on specific criteria to prevent redundant or non-useful system volumes from being displayed. - Enhanced the volume tracking actions to ensure only user-visible volumes are included in tracking and untracking operations. - Updated the storage overview component to filter and display only user-visible volumes, improving user experience and clarity. - Refactored related code for better maintainability and readability, ensuring consistent handling of volume visibility across the application. --- core/src/domain/volume.rs | 14 +++ core/src/ops/volumes/list/query.rs | 15 ++- core/src/ops/volumes/track/action.rs | 104 ++++++++---------- core/src/ops/volumes/track/input.rs | 15 +++ core/src/ops/volumes/track/mod.rs | 5 + core/src/ops/volumes/track/output.rs | 27 ++--- core/src/ops/volumes/untrack/action.rs | 70 ++++++------ core/src/ops/volumes/untrack/input.rs | 12 ++ core/src/ops/volumes/untrack/mod.rs | 5 + core/src/ops/volumes/untrack/output.rs | 17 +-- core/src/volume/fs/apfs.rs | 69 ++++++++++-- .../src/routes/overview/StorageOverview.tsx | 25 +++-- 12 files changed, 234 insertions(+), 144 deletions(-) create mode 100644 core/src/ops/volumes/track/input.rs create mode 100644 core/src/ops/volumes/untrack/input.rs diff --git a/core/src/domain/volume.rs b/core/src/domain/volume.rs index 7be2b9a2d..90ff54bc7 100644 --- a/core/src/domain/volume.rs +++ b/core/src/domain/volume.rs @@ -3,6 +3,7 @@ //! This represents volumes in Spacedrive, combining runtime detection capabilities //! with database tracking and user preferences. Supports local, network, and cloud volumes. +use crate::domain::resource::Identifiable; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; use specta::Type; @@ -526,6 +527,19 @@ pub enum FileSystem { Other(String), } +impl Identifiable for Volume { + fn id(&self) -> Uuid { + self.id + } + + fn resource_type() -> &'static str + where + Self: Sized, + { + "volume" + } +} + impl Volume { /// Create a new tracked volume pub fn new( diff --git a/core/src/ops/volumes/list/query.rs b/core/src/ops/volumes/list/query.rs index 451f9ee2f..e8fbb1c30 100644 --- a/core/src/ops/volumes/list/query.rs +++ b/core/src/ops/volumes/list/query.rs @@ -59,7 +59,7 @@ impl VolumeListQuery { // Query to calculate unique bytes on this volume: // 1. Join entries with directory_paths to get full paths - // 2. Filter entries whose paths start with this volume's mount point + // 2. Filter entries whose paths start with this volume's mount point // 3. Join with content_identity to get content hashes // 4. Group by content_hash to deduplicate, then sum total_size let query = r#" @@ -158,7 +158,11 @@ impl LibraryQuery for VolumeListQuery { let db = library.db().conn(); // Get tracked volumes from database (includes volumes from ALL devices) - let tracked_volumes = entities::volume::Entity::find().all(db).await?; + // Only include user-visible volumes + let tracked_volumes = entities::volume::Entity::find() + .filter(entities::volume::Column::IsUserVisible.eq(true)) + .all(db) + .await?; // Create a map of tracked volumes by fingerprint let mut tracked_map: HashMap = tracked_volumes @@ -209,7 +213,8 @@ impl LibraryQuery for VolumeListQuery { if matches!(self.filter, VolumeFilter::All) { let all_volumes = volume_manager.get_all_volumes().await; for vol in all_volumes { - if !tracked_map.contains_key(&vol.fingerprint.0) { + // Only show user-visible volumes + if !tracked_map.contains_key(&vol.fingerprint.0) && vol.is_user_visible { volume_items.push(super::output::VolumeItem { id: vol.id, name: vol.name.clone(), @@ -235,9 +240,9 @@ impl LibraryQuery for VolumeListQuery { // Get all detected volumes from volume manager (current device only) let all_volumes = volume_manager.get_all_volumes().await; - // Only return volumes that are NOT tracked + // Only return volumes that are NOT tracked and are user-visible for vol in all_volumes { - if !tracked_map.contains_key(&vol.fingerprint.0) { + if !tracked_map.contains_key(&vol.fingerprint.0) && vol.is_user_visible { volume_items.push(super::output::VolumeItem { id: vol.id, name: vol.name.clone(), diff --git a/core/src/ops/volumes/track/action.rs b/core/src/ops/volumes/track/action.rs index 04b6306e9..13097ce12 100644 --- a/core/src/ops/volumes/track/action.rs +++ b/core/src/ops/volumes/track/action.rs @@ -1,23 +1,14 @@ -//! Track volume action -//! -//! This action tracks a volume within a library, allowing Spacedrive to monitor -//! and index files on the volume. +//! Volume track action -use super::output::VolumeTrackOutput; +use super::{VolumeTrackInput, VolumeTrackOutput}; use crate::{ context::CoreContext, - infra::action::{error::ActionError, LibraryAction}, + domain::{resource::Identifiable, volume::Volume}, + infra::{action::error::ActionError, event::Event}, volume::VolumeFingerprint, }; use serde::{Deserialize, Serialize}; -use specta::Type; -use uuid::Uuid; - -#[derive(Debug, Clone, Serialize, Deserialize, Type)] -pub struct VolumeTrackInput { - pub fingerprint: VolumeFingerprint, - pub name: Option, -} +use std::sync::Arc; #[derive(Debug, Clone, Serialize, Deserialize)] pub struct VolumeTrackAction { @@ -28,67 +19,64 @@ impl VolumeTrackAction { pub fn new(input: VolumeTrackInput) -> Self { Self { input } } - - /// Create a volume track action with a name - pub fn with_name(fingerprint: VolumeFingerprint, name: String) -> Self { - Self::new(VolumeTrackInput { - fingerprint, - name: Some(name), - }) - } - - /// Create a volume track action without a name - pub fn without_name(fingerprint: VolumeFingerprint) -> Self { - Self::new(VolumeTrackInput { - fingerprint, - name: None, - }) - } } -impl LibraryAction for VolumeTrackAction { +crate::register_library_action!(VolumeTrackAction, "volumes.track"); + +impl crate::infra::action::LibraryAction for VolumeTrackAction { type Input = VolumeTrackInput; type Output = VolumeTrackOutput; - fn from_input(input: VolumeTrackInput) -> Result { + fn from_input(input: Self::Input) -> Result { Ok(VolumeTrackAction::new(input)) } async fn execute( self, - library: std::sync::Arc, - context: std::sync::Arc, + library: Arc, + context: Arc, ) -> Result { - // Check if volume exists - let volume = context - .volume_manager - .get_volume(&self.input.fingerprint) - .await - .ok_or_else(|| ActionError::InvalidInput("Volume not found".to_string()))?; + let fingerprint = VolumeFingerprint::from_string(&self.input.fingerprint) + .map_err(|e| ActionError::Internal(format!("Invalid fingerprint: {}", e)))?; - if !volume.is_mounted { - return Err(ActionError::InvalidInput( - "Cannot track unmounted volume".to_string(), - )); + // Track the volume + let tracked_volume = context + .volume_manager + .track_volume(&library, &fingerprint, self.input.display_name.clone()) + .await + .map_err(|e| ActionError::Internal(e.to_string()))?; + + // Get the volume from volume manager to emit full Volume resource + let volumes = context.volume_manager.get_all_volumes().await; + let volume = volumes + .iter() + .find(|v| v.fingerprint == fingerprint) + .cloned(); + + // Emit ResourceChanged event + if let Some(mut vol) = volume { + vol.is_tracked = true; + vol.library_id = Some(library.id()); + + context.events.emit(Event::ResourceChanged { + resource_type: Volume::resource_type().to_string(), + resource: serde_json::to_value(&vol) + .map_err(|e| ActionError::Internal(e.to_string()))?, + metadata: None, + }); } - // Track the volume in the database - let tracked = context - .volume_manager - .track_volume(&library, &self.input.fingerprint, self.input.name.clone()) - .await - .map_err(|e| ActionError::InvalidInput(format!("Volume tracking failed: {}", e)))?; - - Ok(VolumeTrackOutput::new( - self.input.fingerprint, - tracked.display_name.unwrap_or(volume.name), - )) + Ok(VolumeTrackOutput { + volume_id: tracked_volume.uuid, + fingerprint: tracked_volume.fingerprint, + name: tracked_volume + .display_name + .unwrap_or_else(|| "Unnamed".to_string()), + is_online: tracked_volume.is_online, + }) } fn action_kind(&self) -> &'static str { "volumes.track" } } - -// Register action -crate::register_library_action!(VolumeTrackAction, "volumes.track"); diff --git a/core/src/ops/volumes/track/input.rs b/core/src/ops/volumes/track/input.rs new file mode 100644 index 000000000..61f56f30e --- /dev/null +++ b/core/src/ops/volumes/track/input.rs @@ -0,0 +1,15 @@ +//! Volume track input + +use crate::volume::VolumeFingerprint; +use serde::{Deserialize, Serialize}; +use specta::Type; + +#[derive(Debug, Clone, Serialize, Deserialize, Type)] +pub struct VolumeTrackInput { + /// Fingerprint of the volume to track + pub fingerprint: String, + + /// Optional custom display name + pub display_name: Option, +} + diff --git a/core/src/ops/volumes/track/mod.rs b/core/src/ops/volumes/track/mod.rs index 58b1d7d7a..b4abe6a4e 100644 --- a/core/src/ops/volumes/track/mod.rs +++ b/core/src/ops/volumes/track/mod.rs @@ -1,4 +1,9 @@ +//! Track volume operation + pub mod action; +pub mod input; pub mod output; +pub use action::VolumeTrackAction; +pub use input::VolumeTrackInput; pub use output::VolumeTrackOutput; diff --git a/core/src/ops/volumes/track/output.rs b/core/src/ops/volumes/track/output.rs index 0a3dc87fb..01df8f7bb 100644 --- a/core/src/ops/volumes/track/output.rs +++ b/core/src/ops/volumes/track/output.rs @@ -1,25 +1,20 @@ -//! Volume track operation output types +//! Volume track output -use crate::volume::VolumeFingerprint; use serde::{Deserialize, Serialize}; use specta::Type; +use uuid::Uuid; -/// Output from volume track operation #[derive(Debug, Clone, Serialize, Deserialize, Type)] pub struct VolumeTrackOutput { - /// The fingerprint of the tracked volume - pub fingerprint: VolumeFingerprint, + /// UUID of the tracked volume + pub volume_id: Uuid, - /// The display name of the tracked volume - pub volume_name: String, -} + /// Fingerprint of the volume + pub fingerprint: String, -impl VolumeTrackOutput { - /// Create new volume track output - pub fn new(fingerprint: VolumeFingerprint, volume_name: String) -> Self { - Self { - fingerprint, - volume_name, - } - } + /// Display name + pub name: String, + + /// Whether the volume is currently online + pub is_online: bool, } diff --git a/core/src/ops/volumes/untrack/action.rs b/core/src/ops/volumes/untrack/action.rs index 72cd293c1..9f57736a1 100644 --- a/core/src/ops/volumes/untrack/action.rs +++ b/core/src/ops/volumes/untrack/action.rs @@ -1,64 +1,70 @@ -//! Untrack volume action -//! -//! This action removes volume tracking from a library. +//! Volume untrack action -use super::output::VolumeUntrackOutput; +use super::{VolumeUntrackInput, VolumeUntrackOutput}; use crate::{ context::CoreContext, - infra::action::{error::ActionError, LibraryAction}, - volume::VolumeFingerprint, + domain::{resource::Identifiable, volume::Volume}, + infra::{action::error::ActionError, db::entities, event::Event}, }; +use sea_orm::{ColumnTrait, EntityTrait, QueryFilter}; use serde::{Deserialize, Serialize}; -use specta::Type; +use std::sync::Arc; -#[derive(Debug, Clone, Serialize, Deserialize, Type)] -pub struct VolumeUntrackInput { - pub fingerprint: VolumeFingerprint, -} - -/// Input for untracking a volume -#[derive(Debug, Clone, Serialize, Deserialize, Type)] +#[derive(Debug, Clone, Serialize, Deserialize)] pub struct VolumeUntrackAction { - /// The fingerprint of the volume to untrack input: VolumeUntrackInput, } impl VolumeUntrackAction { - /// Create a new volume untrack action pub fn new(input: VolumeUntrackInput) -> Self { Self { input } } } -// Implement the unified ActionTrait (following VolumeTrackAction model) -impl LibraryAction for VolumeUntrackAction { +crate::register_library_action!(VolumeUntrackAction, "volumes.untrack"); + +impl crate::infra::action::LibraryAction for VolumeUntrackAction { type Input = VolumeUntrackInput; type Output = VolumeUntrackOutput; - fn from_input(input: VolumeUntrackInput) -> Result { + fn from_input(input: Self::Input) -> Result { Ok(VolumeUntrackAction::new(input)) } async fn execute( self, - library: std::sync::Arc, - context: std::sync::Arc, + library: Arc, + context: Arc, ) -> Result { - // Untrack the volume from the database - context - .volume_manager - .untrack_volume(&library, &self.input.fingerprint) - .await - .map_err(|e| ActionError::InvalidInput(format!("Volume untracking failed: {}", e)))?; + let db = library.db().conn(); - // Return native output directly - Ok(VolumeUntrackOutput::new(self.input.fingerprint)) + // Find the volume in the database + let volume = entities::volume::Entity::find() + .filter(entities::volume::Column::Uuid.eq(self.input.volume_id)) + .one(db) + .await + .map_err(|e| ActionError::Internal(e.to_string()))? + .ok_or_else(|| ActionError::Internal("Volume not found".to_string()))?; + + // Delete the volume from database + entities::volume::Entity::delete_by_id(volume.id) + .exec(db) + .await + .map_err(|e| ActionError::Internal(e.to_string()))?; + + // Emit ResourceDeleted event + context.events.emit(Event::ResourceDeleted { + resource_type: Volume::resource_type().to_string(), + resource_id: self.input.volume_id, + }); + + Ok(VolumeUntrackOutput { + volume_id: self.input.volume_id, + success: true, + }) } fn action_kind(&self) -> &'static str { "volumes.untrack" } } - -// Register action -crate::register_library_action!(VolumeUntrackAction, "volumes.untrack"); diff --git a/core/src/ops/volumes/untrack/input.rs b/core/src/ops/volumes/untrack/input.rs new file mode 100644 index 000000000..6637cf200 --- /dev/null +++ b/core/src/ops/volumes/untrack/input.rs @@ -0,0 +1,12 @@ +//! Volume untrack input + +use serde::{Deserialize, Serialize}; +use specta::Type; +use uuid::Uuid; + +#[derive(Debug, Clone, Serialize, Deserialize, Type)] +pub struct VolumeUntrackInput { + /// UUID of the volume to untrack + pub volume_id: Uuid, +} + diff --git a/core/src/ops/volumes/untrack/mod.rs b/core/src/ops/volumes/untrack/mod.rs index c8d600455..99345089e 100644 --- a/core/src/ops/volumes/untrack/mod.rs +++ b/core/src/ops/volumes/untrack/mod.rs @@ -1,4 +1,9 @@ +//! Untrack volume operation + pub mod action; +pub mod input; pub mod output; +pub use action::VolumeUntrackAction; +pub use input::VolumeUntrackInput; pub use output::VolumeUntrackOutput; diff --git a/core/src/ops/volumes/untrack/output.rs b/core/src/ops/volumes/untrack/output.rs index f8415d8f8..eef6027d4 100644 --- a/core/src/ops/volumes/untrack/output.rs +++ b/core/src/ops/volumes/untrack/output.rs @@ -1,19 +1,14 @@ -//! Volume untrack operation output types +//! Volume untrack output -use crate::volume::VolumeFingerprint; use serde::{Deserialize, Serialize}; use specta::Type; +use uuid::Uuid; -/// Output from volume untrack operation #[derive(Debug, Clone, Serialize, Deserialize, Type)] pub struct VolumeUntrackOutput { - /// The fingerprint of the untracked volume - pub fingerprint: VolumeFingerprint, -} + /// UUID of the untracked volume + pub volume_id: Uuid, -impl VolumeUntrackOutput { - /// Create new volume untrack output - pub fn new(fingerprint: VolumeFingerprint) -> Self { - Self { fingerprint } - } + /// Whether the operation was successful + pub success: bool, } diff --git a/core/src/volume/fs/apfs.rs b/core/src/volume/fs/apfs.rs index 2f50347f1..361a6a357 100644 --- a/core/src/volume/fs/apfs.rs +++ b/core/src/volume/fs/apfs.rs @@ -356,20 +356,19 @@ pub fn containers_to_volumes( let mount_type = determine_mount_type(&volume_info.role, mount_point); let volume_type = classify_volume_type(&volume_info.role, mount_point); - // Auto-track eligibility: Primary, UserData, System volumes - // Also track Secondary volumes if they're system mounts (e.g., developer tools) + // Determine if volume should be user-visible + let is_user_visible = should_be_user_visible(mount_point, &volume_info.role, &volume_info.name); + + // Auto-track eligibility: Only UserData volume + // Previously we auto-tracked System, Primary, etc., but that created too many overlapping volumes let auto_track_eligible = matches!( volume_type, crate::volume::types::VolumeType::UserData - | crate::volume::types::VolumeType::Primary - | crate::volume::types::VolumeType::System - ) || (volume_type - == crate::volume::types::VolumeType::Secondary - && mount_type == crate::volume::types::MountType::System); + ) && is_user_visible; debug!( - "APFS_CONVERT: Volume '{}' classified as Type={:?}, auto_track_eligible={}", - volume_info.name, volume_type, auto_track_eligible + "APFS_CONVERT: Volume '{}' classified as Type={:?}, user_visible={}, auto_track_eligible={}", + volume_info.name, volume_type, is_user_visible, auto_track_eligible ); // Get space information (total capacity and available space) @@ -409,7 +408,7 @@ pub fn containers_to_volumes( apfs_container: Some(container.clone()), container_volume_id: Some(volume_info.disk_id.clone()), path_mappings, - is_user_visible: true, + is_user_visible, auto_track_eligible, read_speed_mbps: None, write_speed_mbps: None, @@ -494,6 +493,56 @@ fn classify_volume_type( } } +/// Determine if a volume should be visible to the user +/// Filters out system volumes that are redundant or not useful for user interaction +fn should_be_user_visible( + mount_point: &PathBuf, + role: &ApfsVolumeRole, + name: &str, +) -> bool { + let mount_str = mount_point.to_string_lossy(); + + // Hide system utility volumes + match role { + ApfsVolumeRole::Preboot | ApfsVolumeRole::Recovery | ApfsVolumeRole::VM => return false, + _ => {} + } + + // Hide specific mount points + if mount_str.starts_with("/System/Volumes/Preboot") + || mount_str.starts_with("/System/Volumes/VM") + || mount_str.starts_with("/System/Volumes/Hardware") + || mount_str.starts_with("/System/Volumes/Update") + || mount_str.starts_with("/System/Volumes/xarts") + || mount_str.starts_with("/System/Volumes/iSCPreboot") + { + return false; + } + + // Hide iOS Simulator volumes + if mount_str.starts_with("/Library/Developer/CoreSimulator") { + return false; + } + + // Hide home autofs mounts + if mount_str.contains("/home") && name.to_lowercase() == "home" { + return false; + } + + // Hide snapshot mounts (usually contain @ symbol) + if mount_str.contains("@") { + return false; + } + + // Hide the root "/" volume if it's a system volume (prefer showing Data volume instead) + // The Data volume is where actual user files live in modern macOS + if mount_str.as_ref() == "/" && matches!(role, ApfsVolumeRole::System) { + return false; + } + + true +} + /// Get volume space information (platform-specific implementation needed) fn get_volume_space_info(mount_point: &PathBuf) -> VolumeResult<(u64, u64)> { // This would need platform-specific implementation diff --git a/packages/interface/src/routes/overview/StorageOverview.tsx b/packages/interface/src/routes/overview/StorageOverview.tsx index 86d40e9d8..5fcda99e4 100644 --- a/packages/interface/src/routes/overview/StorageOverview.tsx +++ b/packages/interface/src/routes/overview/StorageOverview.tsx @@ -53,11 +53,7 @@ function getVolumeIcon(volumeType: string, name?: string): string { } function getDiskTypeLabel(diskType: string): string { - return diskType === "SSD" - ? "SSD" - : diskType === "HDD" - ? "HDD" - : diskType; + return diskType === "SSD" ? "SSD" : diskType === "HDD" ? "HDD" : diskType; } export function StorageOverview() { @@ -101,14 +97,19 @@ export function StorageOverview() { const volumes = volumesData?.volumes || []; const devices = devicesData || []; + // Filter to only show user-visible volumes + const userVisibleVolumes = volumes.filter( + (volume) => volume.is_user_visible !== false, + ); + // Group volumes by device - note: VolumeItem doesn't have device_id yet // So we'll just show all volumes ungrouped for now // TODO: Backend needs to add device_id to VolumeItem - const volumesByDevice: Record = {}; + const volumesByDevice: Record = {}; // For now, create a single "All Devices" group - if (volumes.length > 0) { - volumesByDevice["all"] = volumes; + if (userVisibleVolumes.length > 0) { + volumesByDevice["all"] = userVisibleVolumes; } return ( @@ -118,9 +119,9 @@ export function StorageOverview() { Storage Volumes

- {volumes.length}{" "} - {volumes.length === 1 ? "volume" : "volumes"} across{" "} - {devices.length}{" "} + {userVisibleVolumes.length}{" "} + {userVisibleVolumes.length === 1 ? "volume" : "volumes"}{" "} + across {devices.length}{" "} {devices.length === 1 ? "device" : "devices"}

@@ -145,7 +146,7 @@ export function StorageOverview() { }, )} - {volumes.length === 0 && ( + {userVisibleVolumes.length === 0 && (

No volumes detected