From 79eb8d3c2b8e201e381a177915ab3d0c511c44fa Mon Sep 17 00:00:00 2001 From: Jamie Pine Date: Thu, 16 Oct 2025 05:07:20 -0700 Subject: [PATCH] feat: add cloud path skipping logic and cloud identifier support --- core/src/domain/volume.rs | 23 ++++++- core/src/infra/db/entities/volume.rs | 4 ++ .../m20251016_000001_add_cloud_identifier.rs | 35 ++++++++++ core/src/infra/db/migration/mod.rs | 2 + core/src/ops/volumes/add_cloud/action.rs | 36 +++++------ core/src/service/watcher/mod.rs | 8 +-- core/src/volume/fs/apfs.rs | 1 + core/src/volume/manager.rs | 64 ++++++++----------- core/src/volume/platform/macos.rs | 2 + core/src/volume/speed.rs | 1 + 10 files changed, 114 insertions(+), 62 deletions(-) create mode 100644 core/src/infra/db/migration/m20251016_000001_add_cloud_identifier.rs diff --git a/core/src/domain/volume.rs b/core/src/domain/volume.rs index 37ec2b545..474f64bea 100644 --- a/core/src/domain/volume.rs +++ b/core/src/domain/volume.rs @@ -293,6 +293,11 @@ pub struct Volume { #[specta(skip)] pub backend: Option>, + /// Cloud identifier (bucket/drive/container name) for cloud volumes + /// This is separate from mount_point to allow display names with suffixes + /// while maintaining the correct cloud resource identifier for backend operations + pub cloud_identifier: Option, + /// APFS container information (macOS only) pub apfs_container: Option, @@ -547,6 +552,7 @@ impl Volume { is_tracked: false, hardware_id: None, backend: None, + cloud_identifier: None, apfs_container: None, container_volume_id: None, path_mappings: Vec::new(), @@ -715,16 +721,30 @@ impl Volume { crate::volume::fs::contains_path(self, path) } - /// Parse cloud service and identifier from mount point + /// Parse cloud service and identifier from cloud_identifier or mount point /// Returns None for non-cloud volumes or unparseable mount points /// /// # Examples + /// - With cloud_identifier="my-bucket" and mount_point="s3://my-bucket-2" → Some((S3, "my-bucket")) /// - "s3://my-bucket" → Some((S3, "my-bucket")) /// - "gdrive://My Drive" → Some((GoogleDrive, "My Drive")) /// - "/mnt/local" → None pub fn parse_cloud_identity(&self) -> Option<(crate::volume::backend::CloudServiceType, String)> { use crate::volume::backend::CloudServiceType; + // If we have a stored cloud_identifier, use it with the service from mount_point + if let Some(ref cloud_id) = self.cloud_identifier { + let mount_str = self.mount_point.to_string_lossy(); + let parts: Vec<&str> = mount_str.splitn(2, "://").collect(); + + if parts.len() == 2 { + if let Some(service) = CloudServiceType::from_scheme(parts[0]) { + return Some((service, cloud_id.clone())); + } + } + } + + // Fallback to parsing mount_point for backwards compatibility let mount_str = self.mount_point.to_string_lossy(); let parts: Vec<&str> = mount_str.splitn(2, "://").collect(); @@ -796,6 +816,7 @@ impl TrackedVolume { is_mounted: false, hardware_id: self.device_model.clone(), backend: None, + cloud_identifier: None, apfs_container: None, container_volume_id: None, path_mappings: Vec::new(), diff --git a/core/src/infra/db/entities/volume.rs b/core/src/infra/db/entities/volume.rs index 23d14e343..80a6ec7f4 100644 --- a/core/src/infra/db/entities/volume.rs +++ b/core/src/infra/db/entities/volume.rs @@ -33,6 +33,8 @@ pub struct Model { pub is_user_visible: Option, /// Whether volume is eligible for auto-tracking pub auto_track_eligible: Option, + /// Cloud identifier (bucket/drive/container name) for cloud volumes + pub cloud_identifier: Option, } #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] @@ -201,6 +203,7 @@ impl Syncable for Model { volume_type: Set(data.get("volume_type").and_then(|v| v.as_str()).map(String::from)), is_user_visible: Set(data.get("is_user_visible").and_then(|v| v.as_bool())), auto_track_eligible: Set(data.get("auto_track_eligible").and_then(|v| v.as_bool())), + cloud_identifier: Set(data.get("cloud_identifier").and_then(|v| v.as_str()).map(String::from)), }; Entity::insert(active) @@ -222,6 +225,7 @@ impl Syncable for Model { Column::VolumeType, Column::IsUserVisible, Column::AutoTrackEligible, + Column::CloudIdentifier, Column::LastSeenAt, ]) .to_owned(), diff --git a/core/src/infra/db/migration/m20251016_000001_add_cloud_identifier.rs b/core/src/infra/db/migration/m20251016_000001_add_cloud_identifier.rs new file mode 100644 index 000000000..bdc5d6004 --- /dev/null +++ b/core/src/infra/db/migration/m20251016_000001_add_cloud_identifier.rs @@ -0,0 +1,35 @@ +use sea_orm_migration::prelude::*; + +#[derive(DeriveMigrationName)] +pub struct Migration; + +#[async_trait::async_trait] +impl MigrationTrait for Migration { + async fn up(&self, manager: &SchemaManager) -> Result<(), DbErr> { + // Add cloud_identifier column to volumes table + // This stores the actual cloud resource identifier (bucket/drive/container name) + // separately from mount_point to handle duplicate resource names + manager + .alter_table( + Table::alter() + .table(Volumes::Table) + .add_column_if_not_exists(ColumnDef::new(Volumes::CloudIdentifier).string()) + .to_owned(), + ) + .await?; + + Ok(()) + } + + async fn down(&self, manager: &SchemaManager) -> Result<(), DbErr> { + // Remove cloud_identifier column + // Note: SQLite doesn't support dropping columns easily + Ok(()) + } +} + +#[derive(DeriveIden)] +enum Volumes { + Table, + CloudIdentifier, +} diff --git a/core/src/infra/db/migration/mod.rs b/core/src/infra/db/migration/mod.rs index ae1b424d7..785a2d2e0 100644 --- a/core/src/infra/db/migration/mod.rs +++ b/core/src/infra/db/migration/mod.rs @@ -13,6 +13,7 @@ mod m20250120_000001_create_fts5_search_index; mod m20251009_000001_add_sync_to_devices; mod m20251015_000001_add_device_slug; mod m20251015_000002_create_sync_tables; +mod m20251016_000001_add_cloud_identifier; pub struct Migrator; @@ -31,6 +32,7 @@ impl MigratorTrait for Migrator { Box::new(m20251009_000001_add_sync_to_devices::Migration), Box::new(m20251015_000001_add_device_slug::Migration), Box::new(m20251015_000002_create_sync_tables::Migration), + Box::new(m20251016_000001_add_cloud_identifier::Migration), ] } } diff --git a/core/src/ops/volumes/add_cloud/action.rs b/core/src/ops/volumes/add_cloud/action.rs index ab5e20a02..79c2938f5 100644 --- a/core/src/ops/volumes/add_cloud/action.rs +++ b/core/src/ops/volumes/add_cloud/action.rs @@ -97,7 +97,7 @@ impl LibraryAction for VolumeAddCloudAction { .map_err(|e| ActionError::InvalidInput(format!("Failed to get device ID: {}", e)))?; let library_id = library.id(); - let (backend, credential, mount_point) = match &self.input.config { + let (backend, credential, cloud_identifier, mount_point) = match &self.input.config { CloudStorageConfig::S3 { bucket, region, @@ -124,12 +124,13 @@ impl LibraryAction for VolumeAddCloudAction { None, ); + let cloud_identifier = bucket.clone(); let desired_mount_point = format!("s3://{}", bucket); let mount_point = context.volume_manager .ensure_unique_mount_point(&desired_mount_point) .await; - (backend, credential, mount_point) + (backend, credential, cloud_identifier, mount_point) } CloudStorageConfig::GoogleDrive { root, @@ -157,15 +158,13 @@ impl LibraryAction for VolumeAddCloudAction { None, // Google Drive tokens typically don't have a fixed expiry in the refresh flow ); - let desired_mount_point = format!( - "gdrive://{}", - root.as_deref().unwrap_or("root") - ); + let cloud_identifier = root.as_deref().unwrap_or("root").to_string(); + let desired_mount_point = format!("gdrive://{}", cloud_identifier); let mount_point = context.volume_manager .ensure_unique_mount_point(&desired_mount_point) .await; - (backend, credential, mount_point) + (backend, credential, cloud_identifier, mount_point) } CloudStorageConfig::OneDrive { root, @@ -193,15 +192,13 @@ impl LibraryAction for VolumeAddCloudAction { None, ); - let desired_mount_point = format!( - "onedrive://{}", - root.as_deref().unwrap_or("root") - ); + let cloud_identifier = root.as_deref().unwrap_or("root").to_string(); + let desired_mount_point = format!("onedrive://{}", cloud_identifier); let mount_point = context.volume_manager .ensure_unique_mount_point(&desired_mount_point) .await; - (backend, credential, mount_point) + (backend, credential, cloud_identifier, mount_point) } CloudStorageConfig::Dropbox { root, @@ -229,15 +226,13 @@ impl LibraryAction for VolumeAddCloudAction { None, ); - let desired_mount_point = format!( - "dropbox://{}", - root.as_deref().unwrap_or("root") - ); + let cloud_identifier = root.as_deref().unwrap_or("root").to_string(); + let desired_mount_point = format!("dropbox://{}", cloud_identifier); let mount_point = context.volume_manager .ensure_unique_mount_point(&desired_mount_point) .await; - (backend, credential, mount_point) + (backend, credential, cloud_identifier, mount_point) } CloudStorageConfig::AzureBlob { container, @@ -263,12 +258,13 @@ impl LibraryAction for VolumeAddCloudAction { None, ); + let cloud_identifier = container.clone(); let desired_mount_point = format!("azblob://{}", container); let mount_point = context.volume_manager .ensure_unique_mount_point(&desired_mount_point) .await; - (backend, credential, mount_point) + (backend, credential, cloud_identifier, mount_point) } CloudStorageConfig::GoogleCloudStorage { bucket, @@ -292,12 +288,13 @@ impl LibraryAction for VolumeAddCloudAction { service_account_json.clone(), ); + let cloud_identifier = bucket.clone(); let desired_mount_point = format!("gcs://{}", bucket); let mount_point = context.volume_manager .ensure_unique_mount_point(&desired_mount_point) .await; - (backend, credential, mount_point) + (backend, credential, cloud_identifier, mount_point) } }; @@ -332,6 +329,7 @@ impl LibraryAction for VolumeAddCloudAction { is_mounted: true, hardware_id: None, backend: Some(backend_arc), + cloud_identifier: Some(cloud_identifier), apfs_container: None, container_volume_id: None, path_mappings: Vec::new(), diff --git a/core/src/service/watcher/mod.rs b/core/src/service/watcher/mod.rs index 394789dd6..94c07e89f 100644 --- a/core/src/service/watcher/mod.rs +++ b/core/src/service/watcher/mod.rs @@ -328,8 +328,9 @@ impl LocationWatcher { } // Skip cloud locations - they don't have filesystem paths to watch + // Cloud paths use service-native URIs like s3://, gdrive://, etc. let path_str = location.path.to_string_lossy(); - if path_str.starts_with("cloud://") || path_str.starts_with("sd://cloud/") { + if path_str.contains("://") && !path_str.starts_with("local://") { debug!( "Skipping cloud location {} from filesystem watcher: {}", location.id, path_str @@ -475,10 +476,9 @@ impl LocationWatcher { { Ok(path) => { // Skip cloud locations - they don't have filesystem paths to watch + // Cloud paths use service-native URIs like s3://, gdrive://, etc. let path_str = path.to_string_lossy(); - if path_str.starts_with("cloud://") - || path_str.starts_with("sd://cloud/") - { + if path_str.contains("://") && !path_str.starts_with("local://") { debug!( "Skipping cloud location {} from filesystem watcher: {}", location.uuid, path_str diff --git a/core/src/volume/fs/apfs.rs b/core/src/volume/fs/apfs.rs index f80e63771..2ae1b29b5 100644 --- a/core/src/volume/fs/apfs.rs +++ b/core/src/volume/fs/apfs.rs @@ -321,6 +321,7 @@ pub fn containers_to_volumes( is_mounted: true, hardware_id: Some(volume_info.disk_id.clone()), backend: None, + cloud_identifier: None, apfs_container: Some(container.clone()), container_volume_id: Some(volume_info.disk_id.clone()), path_mappings, diff --git a/core/src/volume/manager.rs b/core/src/volume/manager.rs index 3f326d2bd..456ed5722 100644 --- a/core/src/volume/manager.rs +++ b/core/src/volume/manager.rs @@ -170,7 +170,7 @@ impl VolumeManager { match credential_manager.get_credential(library.id(), &db_volume.fingerprint) { Ok(credential) => { - // Get mount point and service name from database + // Get mount point from database (for display and cache purposes) let mount_point_str = match &db_volume.mount_point { Some(mp) => mp, None => { @@ -179,9 +179,14 @@ impl VolumeManager { } }; - // Recreate the cloud backend from stored credentials - // Use the service's scheme() method as the single source of truth for URI parsing - let scheme_prefix = format!("{}://", credential.service.scheme()); + // Get cloud identifier from database (actual bucket/drive/container name) + let cloud_identifier = match &db_volume.cloud_identifier { + Some(id) => id, + None => { + warn!("No cloud identifier for cloud volume {}", fingerprint.0); + continue; + } + }; let backend_result = match credential.service { crate::volume::CloudServiceType::S3 => { @@ -191,12 +196,8 @@ impl VolumeManager { .. } = &credential.data { - let bucket = mount_point_str - .strip_prefix(&scheme_prefix) - .unwrap_or("unknown"); - crate::volume::CloudBackend::new_s3( - bucket, + cloud_identifier, "us-east-1", // Default region access_key_id, secret_access_key, @@ -213,17 +214,12 @@ impl VolumeManager { refresh_token, } = &credential.data { - // Extract root from mount point if available - let root = mount_point_str - .strip_prefix(&scheme_prefix) - .map(|s| s.to_string()); - crate::volume::CloudBackend::new_google_drive( access_token, refresh_token, "", // client_id not stored yet "", // client_secret not stored yet - root, + Some(cloud_identifier.clone()), ).await } else { warn!("Invalid credential type for Google Drive volume {}", fingerprint.0); @@ -236,16 +232,12 @@ impl VolumeManager { refresh_token, } = &credential.data { - let root = mount_point_str - .strip_prefix(&scheme_prefix) - .map(|s| s.to_string()); - crate::volume::CloudBackend::new_onedrive( access_token, refresh_token, "", "", - root, + Some(cloud_identifier.clone()), ).await } else { warn!("Invalid credential type for OneDrive volume {}", fingerprint.0); @@ -258,16 +250,12 @@ impl VolumeManager { refresh_token, } = &credential.data { - let root = mount_point_str - .strip_prefix(&scheme_prefix) - .map(|s| s.to_string()); - crate::volume::CloudBackend::new_dropbox( access_token, refresh_token, "", "", - root, + Some(cloud_identifier.clone()), ).await } else { warn!("Invalid credential type for Dropbox volume {}", fingerprint.0); @@ -281,12 +269,8 @@ impl VolumeManager { .. } = &credential.data { - let container = mount_point_str - .strip_prefix(&scheme_prefix) - .unwrap_or("unknown"); - crate::volume::CloudBackend::new_azure_blob( - container, + cloud_identifier, access_key_id, secret_access_key, None, @@ -298,12 +282,8 @@ impl VolumeManager { } crate::volume::CloudServiceType::GoogleCloudStorage => { if let crate::crypto::cloud_credentials::CredentialData::ApiKey(service_account_json) = &credential.data { - let bucket = mount_point_str - .strip_prefix(&scheme_prefix) - .unwrap_or("unknown"); - crate::volume::CloudBackend::new_google_cloud_storage( - bucket, + cloud_identifier, service_account_json, None, None, @@ -322,6 +302,7 @@ impl VolumeManager { match backend_result { Ok(backend) => { let now = chrono::Utc::now(); + let volume = Volume { id: db_volume.uuid, fingerprint: fingerprint.clone(), @@ -341,6 +322,7 @@ impl VolumeManager { is_mounted: true, hardware_id: None, backend: Some(Arc::new(backend)), + cloud_identifier: db_volume.cloud_identifier.clone(), apfs_container: None, container_volume_id: None, path_mappings: Vec::new(), @@ -362,11 +344,15 @@ impl VolumeManager { }; let mut volumes = self.volumes.write().await; + + // Cache by mount_point string (includes suffix if added for uniqueness) + let mount_point_str = volume.mount_point.to_string_lossy().to_string(); + volumes.insert(fingerprint.clone(), volume.clone()); - // Update mount point cache for fast cloud volume lookup + // Update mount point cache for fast cloud volume lookup using mount_point (with suffix) let mut mount_point_cache = self.mount_point_cache.write().await; - mount_point_cache.insert(mount_point_str.to_string(), fingerprint.clone()); + mount_point_cache.insert(mount_point_str, fingerprint.clone()); loaded_count += 1; info!("Loaded cloud volume {} ({:?}) from database", db_volume.display_name.as_ref().unwrap_or(&"Unknown".to_string()), credential.service); @@ -1031,9 +1017,10 @@ impl VolumeManager { "Registering cloud volume '{}' with fingerprint {}", volume.name, fingerprint ); + volumes.insert(fingerprint.clone(), volume); - // Update mount point cache for fast cloud volume lookup + // Update mount point cache for fast cloud volume lookup using mount_point (with suffix) let mut mount_point_cache = self.mount_point_cache.write().await; mount_point_cache.insert(mount_point_str, fingerprint); } @@ -1117,6 +1104,7 @@ impl VolumeManager { volume_type: Set(Some(format!("{:?}", volume.volume_type))), is_user_visible: Set(Some(volume.is_user_visible)), auto_track_eligible: Set(Some(volume.auto_track_eligible)), + cloud_identifier: Set(volume.cloud_identifier.clone()), ..Default::default() }; diff --git a/core/src/volume/platform/macos.rs b/core/src/volume/platform/macos.rs index 1a4888b7e..ebe40c56f 100644 --- a/core/src/volume/platform/macos.rs +++ b/core/src/volume/platform/macos.rs @@ -87,6 +87,7 @@ pub async fn detect_non_apfs_volumes( let volume = Volume { id: uuid::Uuid::new_v4(), fingerprint, + device_id, name: name.clone(), library_id: None, @@ -103,6 +104,7 @@ pub async fn detect_non_apfs_volumes( is_mounted: true, hardware_id: Some(filesystem.to_string()), backend: None, + cloud_identifier: None, apfs_container: None, container_volume_id: None, path_mappings: Vec::new(), diff --git a/core/src/volume/speed.rs b/core/src/volume/speed.rs index 923b1f9ea..3871a2a6c 100644 --- a/core/src/volume/speed.rs +++ b/core/src/volume/speed.rs @@ -340,6 +340,7 @@ mod tests { let volume = Volume { id: uuid::Uuid::new_v4(), fingerprint, + cloud_identifier: None, device_id: uuid::Uuid::new_v4(), name: "Test Volume".to_string(), library_id: None,