From e0ccf3ab33744321ae5c5ff700c7dceb0aff699f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hermann?= Date: Wed, 18 Mar 2026 10:23:56 +0100 Subject: [PATCH] refactor: remove legacy SourceInfo field usage --- cmd/cloudstic/cmd_backup_test.go | 4 +- cmd/cloudstic/cmd_list_test.go | 2 +- cmd/cloudstic/format.go | 9 --- cmd/cloudstic/format_test.go | 26 ++++---- internal/core/models.go | 4 -- internal/engine/backup.go | 19 +----- internal/engine/backup_test.go | 89 ++++++++++++++------------- internal/engine/list.go | 2 - internal/engine/policy.go | 5 +- internal/engine/policy_test.go | 102 ++++++++++++++++--------------- pkg/source/local_source.go | 2 - 11 files changed, 120 insertions(+), 144 deletions(-) diff --git a/cmd/cloudstic/cmd_backup_test.go b/cmd/cloudstic/cmd_backup_test.go index f3165f0..5b7aa0b 100644 --- a/cmd/cloudstic/cmd_backup_test.go +++ b/cmd/cloudstic/cmd_backup_test.go @@ -55,8 +55,8 @@ func TestInitSource_Local_VolumeUUID(t *testing.T) { t.Fatalf("initSource failed: %v", err) } info := src.Info() - if info.VolumeUUID != "test-uuid-123" { - t.Errorf("expected VolumeUUID 'test-uuid-123', got %q", info.VolumeUUID) + if info.Identity != "test-uuid-123" { + t.Errorf("expected Identity 'test-uuid-123', got %q", info.Identity) } } diff --git a/cmd/cloudstic/cmd_list_test.go b/cmd/cloudstic/cmd_list_test.go index 6917ce4..d4da150 100644 --- a/cmd/cloudstic/cmd_list_test.go +++ b/cmd/cloudstic/cmd_list_test.go @@ -54,7 +54,7 @@ func TestRunList_Group(t *testing.T) { Ref: "snapshot/abc", Snap: core.Snapshot{ Seq: 1, Created: "2024-01-01", - Source: &core.SourceInfo{Type: "gdrive", Account: "a@b.com", Path: "/", VolumeLabel: "My Drive"}, + Source: &core.SourceInfo{Type: "gdrive", Account: "a@b.com", Path: "/", DriveName: "My Drive"}, }, }, { diff --git a/cmd/cloudstic/format.go b/cmd/cloudstic/format.go index 1f8ccfa..6d7201d 100644 --- a/cmd/cloudstic/format.go +++ b/cmd/cloudstic/format.go @@ -48,9 +48,6 @@ func (r *runner) renderSnapshotTable(entries []engine.SnapshotEntry, reasons map if e.Snap.Source != nil { source = e.Snap.Source.Type driveName := e.Snap.Source.DriveName - if driveName == "" { - driveName = e.Snap.Source.VolumeLabel - } if driveName != "" { source += " (" + driveName + ")" } @@ -85,9 +82,6 @@ func sourceGroupKey(s *core.SourceInfo) string { if s.Identity != "" { return s.Type + "\x00" + s.Identity + "\x00" + pathToken } - if s.VolumeUUID != "" { - return s.Type + "\x00" + s.VolumeUUID + "\x00" + pathToken - } return s.Type + "\x00" + s.Account + "\x00" + pathToken } @@ -99,9 +93,6 @@ func sourceGroupLabel(s *core.SourceInfo) string { var parts []string label := s.Type driveName := s.DriveName - if driveName == "" { - driveName = s.VolumeLabel - } if driveName != "" { label += " (" + driveName + ")" } diff --git a/cmd/cloudstic/format_test.go b/cmd/cloudstic/format_test.go index cccb8a1..9410a24 100644 --- a/cmd/cloudstic/format_test.go +++ b/cmd/cloudstic/format_test.go @@ -86,7 +86,7 @@ func TestRenderSnapshotTable_WithReasons(t *testing.T) { } } -func TestRenderSnapshotTable_VolumeLabel(t *testing.T) { +func TestRenderSnapshotTable_DriveName(t *testing.T) { var out strings.Builder r := &runner{out: &out, errOut: &strings.Builder{}} @@ -97,10 +97,10 @@ func TestRenderSnapshotTable_VolumeLabel(t *testing.T) { Seq: 1, Created: "2024-01-01T00:00:00Z", Source: &core.SourceInfo{ - Type: "gdrive", - Account: "user@gmail.com", - Path: "/", - VolumeLabel: "My Drive", + Type: "gdrive", + Account: "user@gmail.com", + Path: "/", + DriveName: "My Drive", }, }, }, @@ -124,18 +124,18 @@ func TestSourceGroupKey(t *testing.T) { }{ {"nil source", nil, ""}, { - "local with UUID", - &core.SourceInfo{Type: "local", Account: "host", Path: ".", VolumeUUID: "UUID-1"}, + "local with identity", + &core.SourceInfo{Type: "local", Account: "host", Path: ".", Identity: "UUID-1"}, "local\x00UUID-1\x00.", }, { - "gdrive no UUID", + "gdrive no identity", &core.SourceInfo{Type: "gdrive", Account: "user@gmail.com", Path: "/"}, "gdrive\x00user@gmail.com\x00/", }, { - "shared drive with UUID", - &core.SourceInfo{Type: "gdrive", Account: "user@gmail.com", Path: "/", VolumeUUID: "drive-123"}, + "shared drive with identity", + &core.SourceInfo{Type: "gdrive", Account: "user@gmail.com", Path: "/", Identity: "drive-123"}, "gdrive\x00drive-123\x00/", }, } @@ -163,7 +163,7 @@ func TestSourceGroupLabel(t *testing.T) { }, { "gdrive with label", - &core.SourceInfo{Type: "gdrive", Account: "user@gmail.com", Path: "/", VolumeLabel: "My Drive"}, + &core.SourceInfo{Type: "gdrive", Account: "user@gmail.com", Path: "/", DriveName: "My Drive"}, "gdrive (My Drive) · user@gmail.com · /", }, } @@ -186,7 +186,7 @@ func TestRenderGroupedSnapshotTables(t *testing.T) { Ref: "snapshot/aaa", Snap: core.Snapshot{ Seq: 1, Created: "2024-01-01T00:00:00Z", - Source: &core.SourceInfo{Type: "gdrive", Account: "user@gmail.com", Path: "/", VolumeLabel: "My Drive"}, + Source: &core.SourceInfo{Type: "gdrive", Account: "user@gmail.com", Path: "/", DriveName: "My Drive"}, }, }, { @@ -200,7 +200,7 @@ func TestRenderGroupedSnapshotTables(t *testing.T) { Ref: "snapshot/ccc", Snap: core.Snapshot{ Seq: 3, Created: "2024-01-03T00:00:00Z", - Source: &core.SourceInfo{Type: "gdrive", Account: "user@gmail.com", Path: "/", VolumeLabel: "My Drive"}, + Source: &core.SourceInfo{Type: "gdrive", Account: "user@gmail.com", Path: "/", DriveName: "My Drive"}, }, }, } diff --git a/internal/core/models.go b/internal/core/models.go index ad85bd6..9368290 100644 --- a/internal/core/models.go +++ b/internal/core/models.go @@ -84,10 +84,6 @@ type SourceInfo struct { PathID string `json:"path_id,omitempty"` // stable selected-root identity within container DriveName string `json:"drive_name,omitempty"` // human-readable container label (e.g. "My Drive") FsType string `json:"fs_type,omitempty"` // source filesystem type (e.g. "apfs", "ext4", "sftp") - - // Legacy fields (read-only compatibility path; slated for future removal). - VolumeUUID string `json:"volume_uuid,omitempty"` - VolumeLabel string `json:"volume_label,omitempty"` } // Snapshot represents a backup checkpoint diff --git a/internal/engine/backup.go b/internal/engine/backup.go index 8395a4b..953be94 100644 --- a/internal/engine/backup.go +++ b/internal/engine/backup.go @@ -322,24 +322,7 @@ func (bm *BackupManager) findPreviousSnapshot(info core.SourceInfo) *core.Snapsh } } - // Pass 3: legacy UUID + path match. - if info.VolumeUUID != "" { - legacyPath := info.PathID - if legacyPath == "" { - legacyPath = info.Path - } - for _, e := range entries { - if e.Snap.Source != nil && - e.Snap.Source.Type == info.Type && - e.Snap.Source.VolumeUUID == info.VolumeUUID && - (e.Snap.Source.Path == legacyPath || e.Snap.Source.Path == info.Path) { - snap := e.Snap - return &snap - } - } - } - - // Pass 4: legacy match (type + account + path) + // Pass 3: legacy match (type + account + path) for _, e := range entries { if e.Snap.Source != nil && e.Snap.Source.Type == info.Type && diff --git a/internal/engine/backup_test.go b/internal/engine/backup_test.go index 9306015..cc6c566 100644 --- a/internal/engine/backup_test.go +++ b/internal/engine/backup_test.go @@ -226,10 +226,10 @@ func TestBackupManager_Run(t *testing.T) { } } -// TestFindPreviousSnapshot_VolumeUUID verifies that findPreviousSnapshot -// uses VolumeUUID for matching when present, enabling cross-machine +// TestFindPreviousSnapshot_Identity verifies that findPreviousSnapshot +// uses Identity for matching when present, enabling cross-machine // incremental backup for portable drives. -func TestFindPreviousSnapshot_VolumeUUID(t *testing.T) { +func TestFindPreviousSnapshot_Identity(t *testing.T) { s := NewMockStore() // Create snapshots from two different machines backing up the same drive. @@ -238,10 +238,11 @@ func TestFindPreviousSnapshot_VolumeUUID(t *testing.T) { Created: "2026-03-01T10:00:00Z", Root: "node/mac", Source: &core.SourceInfo{ - Type: "local", - Account: "mac-studio.local", - Path: ".", - VolumeUUID: "A1B2C3D4-1234-5678-ABCD-EF0123456789", + Type: "local", + Account: "mac-studio.local", + Path: ".", + Identity: "A1B2C3D4-1234-5678-ABCD-EF0123456789", + PathID: ".", }, } linuxSnap := &core.Snapshot{ @@ -249,10 +250,11 @@ func TestFindPreviousSnapshot_VolumeUUID(t *testing.T) { Created: "2026-03-02T10:00:00Z", Root: "node/linux", Source: &core.SourceInfo{ - Type: "local", - Account: "linux-workstation", - Path: ".", - VolumeUUID: "A1B2C3D4-1234-5678-ABCD-EF0123456789", + Type: "local", + Account: "linux-workstation", + Path: ".", + Identity: "A1B2C3D4-1234-5678-ABCD-EF0123456789", + PathID: ".", }, } @@ -268,16 +270,17 @@ func TestFindPreviousSnapshot_VolumeUUID(t *testing.T) { src := NewMockSource() bm := NewBackupManager(src, s, ui.NewNoOpReporter(), nil) - // Search from the Mac with same UUID and same volume-relative path. + // Search from the Mac with same identity and same selected-root path. info := core.SourceInfo{ - Type: "local", - Account: "mac-studio.local", - Path: ".", - VolumeUUID: "A1B2C3D4-1234-5678-ABCD-EF0123456789", + Type: "local", + Account: "mac-studio.local", + Path: ".", + Identity: "A1B2C3D4-1234-5678-ABCD-EF0123456789", + PathID: ".", } prev := bm.findPreviousSnapshot(info) if prev == nil { - t.Fatal("expected to find previous snapshot via UUID match") + t.Fatal("expected to find previous snapshot via identity match") } // Should return the most recent (Linux) snapshot since catalog is newest-first. if prev.Root != "node/linux" { @@ -286,7 +289,7 @@ func TestFindPreviousSnapshot_VolumeUUID(t *testing.T) { } // TestFindPreviousSnapshot_LegacyFallback verifies that snapshots without -// VolumeUUID are still found by the traditional account+path match. +// Identity are still found by the traditional account+path match. func TestFindPreviousSnapshot_LegacyFallback(t *testing.T) { s := NewMockStore() @@ -324,10 +327,10 @@ func TestFindPreviousSnapshot_LegacyFallback(t *testing.T) { } } -// TestFindPreviousSnapshot_UUIDPreferredOverLegacy verifies that the UUID +// TestFindPreviousSnapshot_IdentityPreferredOverLegacy verifies that Identity // match takes precedence when both UUID and account+path could match // different snapshots. -func TestFindPreviousSnapshot_UUIDPreferredOverLegacy(t *testing.T) { +func TestFindPreviousSnapshot_IdentityPreferredOverLegacy(t *testing.T) { s := NewMockStore() // Old snapshot from same machine, same path, no UUID. @@ -341,16 +344,17 @@ func TestFindPreviousSnapshot_UUIDPreferredOverLegacy(t *testing.T) { Path: "/Volumes/MyDrive", }, } - // Newer snapshot from different machine with UUID (volume-relative path). + // Newer snapshot from different machine with identity (portable path). newSnap := &core.Snapshot{ Seq: 2, Created: "2026-03-02T10:00:00Z", Root: "node/new", Source: &core.SourceInfo{ - Type: "local", - Account: "linux-workstation", - Path: ".", - VolumeUUID: "UUID-1234", + Type: "local", + Account: "linux-workstation", + Path: ".", + Identity: "UUID-1234", + PathID: ".", }, } @@ -365,25 +369,26 @@ func TestFindPreviousSnapshot_UUIDPreferredOverLegacy(t *testing.T) { src := NewMockSource() bm := NewBackupManager(src, s, ui.NewNoOpReporter(), nil) - // Search with UUID — should find the UUID-matched snapshot first. + // Search with identity — should find the identity-matched snapshot first. info := core.SourceInfo{ - Type: "local", - Account: "mac-studio.local", - Path: ".", - VolumeUUID: "UUID-1234", + Type: "local", + Account: "mac-studio.local", + Path: ".", + Identity: "UUID-1234", + PathID: ".", } prev := bm.findPreviousSnapshot(info) if prev == nil { t.Fatal("expected to find previous snapshot") } if prev.Root != "node/new" { - t.Errorf("expected UUID-matched snapshot (node/new), got root=%s", prev.Root) + t.Errorf("expected identity-matched snapshot (node/new), got root=%s", prev.Root) } } -// TestFindPreviousSnapshot_UUIDDifferentSubdirs verifies that backups of +// TestFindPreviousSnapshot_IdentityDifferentSubdirs verifies that backups of // different sub-directories on the same drive do not match each other. -func TestFindPreviousSnapshot_UUIDDifferentSubdirs(t *testing.T) { +func TestFindPreviousSnapshot_IdentityDifferentSubdirs(t *testing.T) { s := NewMockStore() photosSnap := &core.Snapshot{ @@ -391,10 +396,11 @@ func TestFindPreviousSnapshot_UUIDDifferentSubdirs(t *testing.T) { Created: "2026-03-01T10:00:00Z", Root: "node/photos", Source: &core.SourceInfo{ - Type: "local", - Account: "mac-studio.local", - Path: "Photos", - VolumeUUID: "UUID-SAME-DRIVE", + Type: "local", + Account: "mac-studio.local", + Path: "Photos", + Identity: "UUID-SAME-DRIVE", + PathID: "Photos", }, } @@ -408,10 +414,11 @@ func TestFindPreviousSnapshot_UUIDDifferentSubdirs(t *testing.T) { // Search for Documents on the same drive — should NOT match Photos. info := core.SourceInfo{ - Type: "local", - Account: "mac-studio.local", - Path: "Documents", - VolumeUUID: "UUID-SAME-DRIVE", + Type: "local", + Account: "mac-studio.local", + Path: "Documents", + Identity: "UUID-SAME-DRIVE", + PathID: "Documents", } prev := bm.findPreviousSnapshot(info) if prev != nil { diff --git a/internal/engine/list.go b/internal/engine/list.go index 2922c6a..2771788 100644 --- a/internal/engine/list.go +++ b/internal/engine/list.go @@ -56,8 +56,6 @@ func (lm *ListManager) Run(ctx context.Context, opts ...ListOption) (*ListResult source = fmt.Sprintf(" source=%s account=%s path=%s", e.Snap.Source.Type, e.Snap.Source.Account, e.Snap.Source.Path) if e.Snap.Source.DriveName != "" { source += fmt.Sprintf(" drive=%s", e.Snap.Source.DriveName) - } else if e.Snap.Source.VolumeLabel != "" { - source += fmt.Sprintf(" drive=%s", e.Snap.Source.VolumeLabel) } if e.Snap.Source.Identity != "" { source += fmt.Sprintf(" identity=%s", e.Snap.Source.Identity) diff --git a/internal/engine/policy.go b/internal/engine/policy.go index 2ac9d94..cb9ec8b 100644 --- a/internal/engine/policy.go +++ b/internal/engine/policy.go @@ -137,8 +137,6 @@ func makeGroupKey(snap *core.Snapshot, gf groupFields) GroupKey { switch { case snap.Source.Identity != "": k.Account = snap.Source.Identity - case snap.Source.VolumeUUID != "": - k.Account = snap.Source.VolumeUUID default: k.Account = snap.Source.Account } @@ -194,8 +192,7 @@ func matchesFilter(snap *core.Snapshot, f snapshotFilter) bool { } // Accept display account and identity fields for compatibility. if snap.Source.Account != f.account && - snap.Source.Identity != f.account && - snap.Source.VolumeUUID != f.account { + snap.Source.Identity != f.account { return false } } diff --git a/internal/engine/policy_test.go b/internal/engine/policy_test.go index b63af0f..d35142a 100644 --- a/internal/engine/policy_test.go +++ b/internal/engine/policy_test.go @@ -159,23 +159,23 @@ func TestMatchesFilter(t *testing.T) { } } -// TestMatchesFilter_VolumeUUID verifies that filtering by -account accepts -// VolumeUUID as an alternative to the hostname, so that portable-drive +// TestMatchesFilter_Identity verifies that filtering by -account accepts +// Identity as an alternative to the hostname, so that portable-drive // snapshots can be targeted by their stable UUID. -func TestMatchesFilter_VolumeUUID(t *testing.T) { +func TestMatchesFilter_Identity(t *testing.T) { const uuid = "A1B2C3D4-1234-5678-ABCD-EF0123456789" portable := &core.SourceInfo{ - Type: "local", - Account: "macbook-pro", // hostname on machine A - Path: "Documents", - VolumeUUID: uuid, + Type: "local", + Account: "macbook-pro", // hostname on machine A + Path: "Documents", + Identity: uuid, } snap := core.Snapshot{Source: portable} - // Filtering by VolumeUUID should match. + // Filtering by Identity should match. if !matchesFilter(&snap, snapshotFilter{account: uuid}) { - t.Error("should match when account filter equals VolumeUUID") + t.Error("should match when account filter equals Identity") } // Filtering by hostname still works. if !matchesFilter(&snap, snapshotFilter{account: "macbook-pro"}) { @@ -187,26 +187,29 @@ func TestMatchesFilter_VolumeUUID(t *testing.T) { } } -// TestGroupSnapshots_VolumeUUID verifies that snapshots from different -// machines but the same VolumeUUID are grouped together. -func TestGroupSnapshots_VolumeUUID(t *testing.T) { +// TestGroupSnapshots_Identity verifies that snapshots from different machines +// but the same Identity are grouped together. +func TestGroupSnapshots_Identity(t *testing.T) { macSource := &core.SourceInfo{ - Type: "local", - Account: "mac-studio.local", - Path: ".", - VolumeUUID: "UUID-SHARED", + Type: "local", + Account: "mac-studio.local", + Path: ".", + Identity: "UUID-SHARED", + PathID: ".", } linuxSource := &core.SourceInfo{ - Type: "local", - Account: "linux-workstation", - Path: ".", - VolumeUUID: "UUID-SHARED", + Type: "local", + Account: "linux-workstation", + Path: ".", + Identity: "UUID-SHARED", + PathID: ".", } otherSource := &core.SourceInfo{ - Type: "local", - Account: "mac-studio.local", - Path: ".", - VolumeUUID: "UUID-OTHER", + Type: "local", + Account: "mac-studio.local", + Path: ".", + Identity: "UUID-OTHER", + PathID: ".", } entries := []SnapshotEntry{ @@ -217,8 +220,8 @@ func TestGroupSnapshots_VolumeUUID(t *testing.T) { groups := groupSnapshots(entries, defaultGroupFields()) - // macSource and linuxSource should be in the same group (same UUID + path). - // otherSource should be separate (different UUID). + // macSource and linuxSource should be in the same group (same Identity + path). + // otherSource should be separate (different Identity). if len(groups) != 2 { t.Errorf("expected 2 groups (same UUID grouped together), got %d", len(groups)) for k, v := range groups { @@ -226,7 +229,7 @@ func TestGroupSnapshots_VolumeUUID(t *testing.T) { } } - // Find the group with 2 entries (the shared UUID group). + // Find the group with 2 entries (the shared Identity group). found := false for _, v := range groups { if len(v) == 2 { @@ -234,24 +237,26 @@ func TestGroupSnapshots_VolumeUUID(t *testing.T) { } } if !found { - t.Error("expected one group with 2 entries (same VolumeUUID)") + t.Error("expected one group with 2 entries (same Identity)") } } -// TestGroupSnapshots_VolumeUUID_DifferentSubdirs verifies that backups of +// TestGroupSnapshots_Identity_DifferentSubdirs verifies that backups of // different sub-directories on the same drive are grouped independently. -func TestGroupSnapshots_VolumeUUID_DifferentSubdirs(t *testing.T) { +func TestGroupSnapshots_Identity_DifferentSubdirs(t *testing.T) { photosSource := &core.SourceInfo{ - Type: "local", - Account: "mac-studio.local", - Path: "Photos", - VolumeUUID: "UUID-SHARED", + Type: "local", + Account: "mac-studio.local", + Path: "Photos", + Identity: "UUID-SHARED", + PathID: "Photos", } docsSource := &core.SourceInfo{ - Type: "local", - Account: "mac-studio.local", - Path: "Documents", - VolumeUUID: "UUID-SHARED", + Type: "local", + Account: "mac-studio.local", + Path: "Documents", + Identity: "UUID-SHARED", + PathID: "Documents", } entries := []SnapshotEntry{ @@ -265,14 +270,15 @@ func TestGroupSnapshots_VolumeUUID_DifferentSubdirs(t *testing.T) { } } -// TestGroupSnapshots_MixedUUIDAndLegacy verifies that snapshots with -// VolumeUUID group by UUID+path while those without group by account+path. -func TestGroupSnapshots_MixedUUIDAndLegacy(t *testing.T) { - withUUID := &core.SourceInfo{ - Type: "local", - Account: "mac-studio.local", - Path: ".", - VolumeUUID: "UUID-1234", +// TestGroupSnapshots_MixedIdentityAndLegacy verifies that snapshots with +// Identity group by identity+path while those without group by account+path. +func TestGroupSnapshots_MixedIdentityAndLegacy(t *testing.T) { + withIdentity := &core.SourceInfo{ + Type: "local", + Account: "mac-studio.local", + Path: ".", + Identity: "UUID-1234", + PathID: ".", } withoutUUID := &core.SourceInfo{ Type: "local", @@ -281,12 +287,12 @@ func TestGroupSnapshots_MixedUUIDAndLegacy(t *testing.T) { } entries := []SnapshotEntry{ - makeEntry("a", "2026-03-02T12:00:00Z", withUUID, nil), + makeEntry("a", "2026-03-02T12:00:00Z", withIdentity, nil), makeEntry("b", "2026-03-01T12:00:00Z", withoutUUID, nil), } groups := groupSnapshots(entries, defaultGroupFields()) if len(groups) != 2 { - t.Errorf("expected 2 groups (UUID vs legacy), got %d", len(groups)) + t.Errorf("expected 2 groups (identity vs legacy), got %d", len(groups)) } } diff --git a/pkg/source/local_source.go b/pkg/source/local_source.go index dd32ea3..03da6af 100644 --- a/pkg/source/local_source.go +++ b/pkg/source/local_source.go @@ -63,8 +63,6 @@ func (s *LocalSource) Info() core.SourceInfo { DriveName: s.volumeLabel, FsType: s.fsType, - VolumeUUID: s.volumeUUID, - VolumeLabel: s.volumeLabel, Identity: func() string { if s.volumeUUID != "" { return s.volumeUUID