From b4c3590bfe62fb099b3bf0211161cccf4fbd9e79 Mon Sep 17 00:00:00 2001 From: Deluan Date: Wed, 25 Jun 2025 10:17:46 -0400 Subject: [PATCH] refactor: move scan triggering logic into a helper method for clarity Signed-off-by: Deluan --- core/library.go | 44 +++++++++++---------------------- core/library_test.go | 58 +++++++++++++++++++++++++------------------- 2 files changed, 47 insertions(+), 55 deletions(-) diff --git a/core/library.go b/core/library.go index c96738ad0..25c701844 100644 --- a/core/library.go +++ b/core/library.go @@ -143,16 +143,7 @@ func (r *libraryRepositoryWrapper) Save(entity interface{}) (string, error) { // Trigger scan after successful library creation if r.scanner != nil { - go func() { - log.Info(r.ctx, "Triggering scan for new library", "libraryID", lib.ID, "name", lib.Name, "path", lib.Path) - start := time.Now() - warnings, err := r.scanner.ScanAll(r.ctx, false) // Quick scan for new library - if err != nil { - log.Error(r.ctx, "Error scanning new library", "libraryID", lib.ID, "name", lib.Name, err) - } else { - log.Info(r.ctx, "Scan completed for new library", "libraryID", lib.ID, "name", lib.Name, "warnings", len(warnings), "elapsed", time.Since(start)) - } - }() + go r.triggerScan(lib, "new") } return strconv.Itoa(lib.ID), nil @@ -185,16 +176,7 @@ func (r *libraryRepositoryWrapper) Update(id string, entity interface{}, cols .. // Trigger scan if path was updated if pathChanged && r.scanner != nil { - go func() { - log.Info(r.ctx, "Triggering scan for updated library", "libraryID", lib.ID, "name", lib.Name, "oldPath", originalLib.Path, "newPath", lib.Path) - start := time.Now() - warnings, err := r.scanner.ScanAll(r.ctx, false) // Quick scan for path change - if err != nil { - log.Error(r.ctx, "Error scanning updated library", "libraryID", lib.ID, "name", lib.Name, err) - } else { - log.Info(r.ctx, "Scan completed for updated library", "libraryID", lib.ID, "name", lib.Name, "warnings", len(warnings), "elapsed", time.Since(start)) - } - }() + go r.triggerScan(lib, "updated") } return nil @@ -221,16 +203,7 @@ func (r *libraryRepositoryWrapper) Delete(id string) error { // Trigger scan after successful library deletion to clean up orphaned data if r.scanner != nil { - go func() { - log.Info(r.ctx, "Triggering scan after library deletion", "libraryID", lib.ID, "name", lib.Name, "path", lib.Path) - start := time.Now() - warnings, err := r.scanner.ScanAll(r.ctx, false) // Quick scan to clean up orphaned data - if err != nil { - log.Error(r.ctx, "Error scanning after library deletion", "libraryID", lib.ID, "name", lib.Name, err) - } else { - log.Info(r.ctx, "Scan completed after library deletion", "libraryID", lib.ID, "name", lib.Name, "warnings", len(warnings), "elapsed", time.Since(start)) - } - }() + go r.triggerScan(lib, "deleted") } return nil @@ -355,3 +328,14 @@ func (s *libraryService) validateLibraryIDs(ctx context.Context, libraryIDs []in return nil } + +func (r *libraryRepositoryWrapper) triggerScan(lib *model.Library, action string) { + log.Info(r.ctx, fmt.Sprintf("Triggering scan for %s library", action), "libraryID", lib.ID, "name", lib.Name, "path", lib.Path) + start := time.Now() + warnings, err := r.scanner.ScanAll(r.ctx, false) // Quick scan for new library + if err != nil { + log.Error(r.ctx, fmt.Sprintf("Error scanning %s library", action), "libraryID", lib.ID, "name", lib.Name, err) + } else { + log.Info(r.ctx, fmt.Sprintf("Scan completed for %s library", action), "libraryID", lib.ID, "name", lib.Name, "warnings", len(warnings), "elapsed", time.Since(start)) + } +} diff --git a/core/library_test.go b/core/library_test.go index 65f0bc039..28899f077 100644 --- a/core/library_test.go +++ b/core/library_test.go @@ -5,6 +5,7 @@ import ( "errors" "os" "path/filepath" + "sync" "github.com/deluan/rest" _ "github.com/navidrome/navidrome/adapters/taglib" // Register taglib extractor @@ -18,24 +19,6 @@ import ( . "github.com/onsi/gomega" ) -// mockScanner provides a simple mock implementation of core.Scanner for testing -type mockScanner struct { - ScanCalls []ScanCall -} - -type ScanCall struct { - FullScan bool - Context context.Context -} - -func (m *mockScanner) ScanAll(ctx context.Context, fullScan bool) (warnings []string, err error) { - m.ScanCalls = append(m.ScanCalls, ScanCall{ - FullScan: fullScan, - Context: ctx, - }) - return []string{}, nil -} - // These tests require the local storage adapter and the taglib extractor to be registered. var _ = Describe("Library Service", func() { var service core.Library @@ -623,7 +606,7 @@ var _ = Describe("Library Service", func() { // Wait briefly for the goroutine to complete Eventually(func() int { - return len(scanner.ScanCalls) + return scanner.len() }, "1s", "10ms").Should(Equal(1)) // Verify scan was called with correct parameters @@ -648,7 +631,7 @@ var _ = Describe("Library Service", func() { // Wait briefly for the goroutine to complete Eventually(func() int { - return len(scanner.ScanCalls) + return scanner.len() }, "1s", "10ms").Should(Equal(1)) // Verify scan was called with correct parameters @@ -668,7 +651,7 @@ var _ = Describe("Library Service", func() { // Wait a bit to ensure no scan was triggered Consistently(func() int { - return len(scanner.ScanCalls) + return scanner.len() }, "100ms", "10ms").Should(Equal(0)) }) @@ -681,7 +664,7 @@ var _ = Describe("Library Service", func() { // Ensure no scan was triggered since creation failed Consistently(func() int { - return len(scanner.ScanCalls) + return scanner.len() }, "100ms", "10ms").Should(Equal(0)) }) @@ -698,7 +681,7 @@ var _ = Describe("Library Service", func() { // Ensure no scan was triggered since update failed Consistently(func() int { - return len(scanner.ScanCalls) + return scanner.len() }, "100ms", "10ms").Should(Equal(0)) }) @@ -714,7 +697,7 @@ var _ = Describe("Library Service", func() { // Wait briefly for the goroutine to complete Eventually(func() int { - return len(scanner.ScanCalls) + return scanner.len() }, "1s", "10ms").Should(Equal(1)) // Verify scan was called with correct parameters @@ -728,8 +711,33 @@ var _ = Describe("Library Service", func() { // Ensure no scan was triggered since deletion failed Consistently(func() int { - return len(scanner.ScanCalls) + return scanner.len() }, "100ms", "10ms").Should(Equal(0)) }) }) }) + +// mockScanner provides a simple mock implementation of core.Scanner for testing +type mockScanner struct { + ScanCalls []ScanCall + mu sync.RWMutex +} + +type ScanCall struct { + FullScan bool +} + +func (m *mockScanner) ScanAll(ctx context.Context, fullScan bool) (warnings []string, err error) { + m.mu.Lock() + defer m.mu.Unlock() + m.ScanCalls = append(m.ScanCalls, ScanCall{ + FullScan: fullScan, + }) + return []string{}, nil +} + +func (m *mockScanner) len() int { + m.mu.RLock() + defer m.mu.RUnlock() + return len(m.ScanCalls) +}