diff --git a/model/folder.go b/model/folder.go index c59e9d465..7a769735e 100644 --- a/model/folder.go +++ b/model/folder.go @@ -90,14 +90,3 @@ type FolderRepository interface { MarkMissing(missing bool, ids ...string) error GetTouchedWithPlaylists() (FolderCursor, error) } - -// ScanTarget represents a specific folder within a library to be scanned. -// NOTE: This struct is used as a map key, so it should only contain comparable types. -type ScanTarget struct { - LibraryID int - FolderPath string // Relative path within the library, or "" for entire library -} - -func (st ScanTarget) String() string { - return fmt.Sprintf("%d:%s", st.LibraryID, st.FolderPath) -} diff --git a/model/scanner.go b/model/scanner.go new file mode 100644 index 000000000..756688a23 --- /dev/null +++ b/model/scanner.go @@ -0,0 +1,28 @@ +package model + +import ( + "fmt" + "time" +) + +// ScanTarget represents a specific folder within a library to be scanned. +// NOTE: This struct is used as a map key, so it should only contain comparable types. +type ScanTarget struct { + LibraryID int + FolderPath string // Relative path within the library, or "" for entire library +} + +func (st ScanTarget) String() string { + return fmt.Sprintf("%d:%s", st.LibraryID, st.FolderPath) +} + +// ScannerStatus holds information about the current scan status +type ScannerStatus struct { + Scanning bool + LastScan time.Time + Count uint32 + FolderCount uint32 + LastError string + ScanType string + ElapsedTime time.Duration +} diff --git a/scanner/controller.go b/scanner/controller.go index caff41b0a..b5ba2ddb7 100644 --- a/scanner/controller.go +++ b/scanner/controller.go @@ -75,17 +75,7 @@ type Scanner interface { // ScanFolders scans specific library/folder pairs, recursing into subdirectories. // If targets is nil, it scans all libraries. This is a blocking operation. ScanFolders(ctx context.Context, fullScan bool, targets []model.ScanTarget) (warnings []string, err error) - Status(context.Context) (*StatusInfo, error) -} - -type StatusInfo struct { - Scanning bool - LastScan time.Time - Count uint32 - FolderCount uint32 - LastError string - ScanType string - ElapsedTime time.Duration + Status(context.Context) (*model.ScannerStatus, error) } func New(rootCtx context.Context, ds model.DataStore, cw artwork.CacheWarmer, broker events.Broker, @@ -208,7 +198,7 @@ func (s *controller) getScanInfo(ctx context.Context) (scanType string, elapsed return scanType, elapsed, lastErr } -func (s *controller) Status(ctx context.Context) (*StatusInfo, error) { +func (s *controller) Status(ctx context.Context) (*model.ScannerStatus, error) { lastScanTime, err := s.getLastScanTime(ctx) if err != nil { return nil, fmt.Errorf("getting last scan time: %w", err) @@ -217,7 +207,7 @@ func (s *controller) Status(ctx context.Context) (*StatusInfo, error) { scanType, elapsed, lastErr := s.getScanInfo(ctx) if running.Load() { - status := &StatusInfo{ + status := &model.ScannerStatus{ Scanning: true, LastScan: lastScanTime, Count: s.count.Load(), @@ -233,7 +223,7 @@ func (s *controller) Status(ctx context.Context) (*StatusInfo, error) { if err != nil { return nil, fmt.Errorf("getting library stats: %w", err) } - return &StatusInfo{ + return &model.ScannerStatus{ Scanning: false, LastScan: lastScanTime, Count: uint32(count), diff --git a/scanner/watcher_test.go b/scanner/watcher_test.go index c10ffe00e..7ae52e725 100644 --- a/scanner/watcher_test.go +++ b/scanner/watcher_test.go @@ -3,7 +3,6 @@ package scanner import ( "context" "io/fs" - "sync" "testing/fstest" "time" @@ -18,7 +17,7 @@ import ( var _ = Describe("Watcher", func() { var ctx context.Context var cancel context.CancelFunc - var mockScanner *mockScanner + var mockScanner *tests.MockScanner var mockDS *tests.MockDataStore var w *watcher var lib *model.Library @@ -37,7 +36,7 @@ var _ = Describe("Watcher", func() { } // Set up mocks - mockScanner = NewMockScanner() + mockScanner = tests.NewMockScanner() mockDS = &tests.MockDataStore{} mockLibRepo := &tests.MockLibraryRepo{} mockLibRepo.SetData(model.Libraries{*lib}) @@ -336,95 +335,3 @@ var _ = Describe("resolveFolderPath", func() { Expect(result).To(Equal("artist2")) }) }) - -// mockScanner implements scanner.Scanner for testing -type mockScanner struct { - mu sync.Mutex - scanAllCalls []ScanAllCall - scanFoldersCalls []ScanFoldersCall - scanningStatus bool -} - -type ScanAllCall struct { - FullScan bool -} - -type ScanFoldersCall struct { - FullScan bool - Targets []model.ScanTarget -} - -func NewMockScanner() *mockScanner { - return &mockScanner{ - scanAllCalls: make([]ScanAllCall, 0), - scanFoldersCalls: make([]ScanFoldersCall, 0), - } -} - -func (m *mockScanner) ScanAll(_ context.Context, fullScan bool) ([]string, error) { - m.mu.Lock() - defer m.mu.Unlock() - - m.scanAllCalls = append(m.scanAllCalls, ScanAllCall{FullScan: fullScan}) - - return nil, nil -} - -func (m *mockScanner) ScanFolders(_ context.Context, fullScan bool, targets []model.ScanTarget) ([]string, error) { - m.mu.Lock() - defer m.mu.Unlock() - - // Make a copy of targets to avoid race conditions - targetsCopy := make([]model.ScanTarget, len(targets)) - copy(targetsCopy, targets) - - m.scanFoldersCalls = append(m.scanFoldersCalls, ScanFoldersCall{ - FullScan: fullScan, - Targets: targetsCopy, - }) - - return nil, nil -} - -func (m *mockScanner) Status(_ context.Context) (*StatusInfo, error) { - m.mu.Lock() - defer m.mu.Unlock() - - return &StatusInfo{ - Scanning: m.scanningStatus, - }, nil -} - -func (m *mockScanner) GetScanAllCallCount() int { - m.mu.Lock() - defer m.mu.Unlock() - return len(m.scanAllCalls) -} - -func (m *mockScanner) GetScanFoldersCallCount() int { - m.mu.Lock() - defer m.mu.Unlock() - return len(m.scanFoldersCalls) -} - -func (m *mockScanner) GetScanFoldersCalls() []ScanFoldersCall { - m.mu.Lock() - defer m.mu.Unlock() - // Return a copy to avoid race conditions - calls := make([]ScanFoldersCall, len(m.scanFoldersCalls)) - copy(calls, m.scanFoldersCalls) - return calls -} - -func (m *mockScanner) Reset() { - m.mu.Lock() - defer m.mu.Unlock() - m.scanAllCalls = make([]ScanAllCall, 0) - m.scanFoldersCalls = make([]ScanFoldersCall, 0) -} - -func (m *mockScanner) SetScanning(scanning bool) { - m.mu.Lock() - defer m.mu.Unlock() - m.scanningStatus = scanning -} diff --git a/server/subsonic/library_scanning_test.go b/server/subsonic/library_scanning_test.go index 4eb1703e8..85fdea5ad 100644 --- a/server/subsonic/library_scanning_test.go +++ b/server/subsonic/library_scanning_test.go @@ -2,23 +2,23 @@ package subsonic import ( "context" + "errors" "net/http/httptest" - "sync" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" - "github.com/navidrome/navidrome/scanner" "github.com/navidrome/navidrome/server/subsonic/responses" + "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) var _ = Describe("LibraryScanning", func() { var api *Router - var ms *mockScanner + var ms *tests.MockScanner BeforeEach(func() { - ms = &mockScanner{} + ms = tests.NewMockScanner() api = &Router{scanner: ms} }) @@ -40,7 +40,8 @@ var _ = Describe("LibraryScanning", func() { // Should return authorization error Expect(err).To(HaveOccurred()) Expect(response).To(BeNil()) - subErr, ok := err.(subError) + var subErr subError + ok := errors.As(err, &subErr) Expect(ok).To(BeTrue()) Expect(subErr.code).To(Equal(responses.ErrorAuthorizationFail)) }) @@ -64,10 +65,12 @@ var _ = Describe("LibraryScanning", func() { Expect(response).ToNot(BeNil()) // Verify ScanAll was called (eventually, since it's in a goroutine) - Eventually(func() bool { - return ms.getScanAllCalled() - }).Should(BeTrue()) - Expect(ms.getScanAllFullScan()).To(BeFalse()) + Eventually(func() int { + return ms.GetScanAllCallCount() + }).Should(BeNumerically(">", 0)) + calls := ms.GetScanAllCalls() + Expect(calls).To(HaveLen(1)) + Expect(calls[0].FullScan).To(BeFalse()) }) It("triggers a full scan with fullScan=true", func() { @@ -89,10 +92,12 @@ var _ = Describe("LibraryScanning", func() { Expect(response).ToNot(BeNil()) // Verify ScanAll was called with fullScan=true - Eventually(func() bool { - return ms.getScanAllCalled() - }).Should(BeTrue()) - Expect(ms.getScanAllFullScan()).To(BeTrue()) + Eventually(func() int { + return ms.GetScanAllCallCount() + }).Should(BeNumerically(">", 0)) + calls := ms.GetScanAllCalls() + Expect(calls).To(HaveLen(1)) + Expect(calls[0].FullScan).To(BeTrue()) }) It("triggers a selective scan with single path parameter", func() { @@ -114,10 +119,12 @@ var _ = Describe("LibraryScanning", func() { Expect(response).ToNot(BeNil()) // Verify ScanFolders was called with correct targets - Eventually(func() bool { - return ms.getScanFoldersCalled() - }).Should(BeTrue()) - targets := ms.getScanFoldersTargets() + Eventually(func() int { + return ms.GetScanFoldersCallCount() + }).Should(BeNumerically(">", 0)) + calls := ms.GetScanFoldersCalls() + Expect(calls).To(HaveLen(1)) + targets := calls[0].Targets Expect(targets).To(HaveLen(1)) Expect(targets[0].LibraryID).To(Equal(1)) Expect(targets[0].FolderPath).To(Equal("Music/Rock")) @@ -142,10 +149,12 @@ var _ = Describe("LibraryScanning", func() { Expect(response).ToNot(BeNil()) // Verify ScanFolders was called with correct targets - Eventually(func() bool { - return ms.getScanFoldersCalled() - }).Should(BeTrue()) - targets := ms.getScanFoldersTargets() + Eventually(func() int { + return ms.GetScanFoldersCallCount() + }).Should(BeNumerically(">", 0)) + calls := ms.GetScanFoldersCalls() + Expect(calls).To(HaveLen(1)) + targets := calls[0].Targets Expect(targets).To(HaveLen(2)) Expect(targets[0].LibraryID).To(Equal(1)) Expect(targets[0].FolderPath).To(Equal("Music/Reggae")) @@ -172,11 +181,13 @@ var _ = Describe("LibraryScanning", func() { Expect(response).ToNot(BeNil()) // Verify ScanFolders was called with fullScan=true - Eventually(func() bool { - return ms.getScanFoldersCalled() - }).Should(BeTrue()) - Expect(ms.getScanFoldersFullScan()).To(BeTrue()) - targets := ms.getScanFoldersTargets() + Eventually(func() int { + return ms.GetScanFoldersCallCount() + }).Should(BeNumerically(">", 0)) + calls := ms.GetScanFoldersCalls() + Expect(calls).To(HaveLen(1)) + Expect(calls[0].FullScan).To(BeTrue()) + targets := calls[0].Targets Expect(targets).To(HaveLen(1)) }) @@ -197,7 +208,8 @@ var _ = Describe("LibraryScanning", func() { // Should return error Expect(err).To(HaveOccurred()) Expect(response).To(BeNil()) - subErr, ok := err.(subError) + var subErr subError + ok := errors.As(err, &subErr) Expect(ok).To(BeTrue()) Expect(subErr.code).To(Equal(responses.ErrorGeneric)) }) @@ -219,7 +231,8 @@ var _ = Describe("LibraryScanning", func() { // Should return error Expect(err).To(HaveOccurred()) Expect(response).To(BeNil()) - subErr, ok := err.(subError) + var subErr subError + ok := errors.As(err, &subErr) Expect(ok).To(BeTrue()) Expect(subErr.code).To(Equal(responses.ErrorGeneric)) }) @@ -243,10 +256,11 @@ var _ = Describe("LibraryScanning", func() { Expect(response).ToNot(BeNil()) // Verify path was decoded correctly - Eventually(func() bool { - return ms.getScanFoldersCalled() - }).Should(BeTrue()) - targets := ms.getScanFoldersTargets() + Eventually(func() int { + return ms.GetScanFoldersCallCount() + }).Should(BeNumerically(">", 0)) + calls := ms.GetScanFoldersCalls() + targets := calls[0].Targets Expect(targets[0].FolderPath).To(Equal("The Beatles")) }) }) @@ -254,11 +268,11 @@ var _ = Describe("LibraryScanning", func() { Describe("GetScanStatus", func() { It("returns scan status", func() { // Setup mock scanner status - ms.statusResponse = &scanner.StatusInfo{ + ms.SetStatusResponse(&model.ScannerStatus{ Scanning: false, Count: 100, FolderCount: 10, - } + }) // Create request ctx := context.Background() @@ -278,90 +292,3 @@ var _ = Describe("LibraryScanning", func() { }) }) }) - -// mockScanner is a test double for the scanner.Scanner interface with proper synchronization -type mockScanner struct { - mu sync.Mutex - - // ScanAll tracking - scanAllCalled bool - scanAllFullScan bool - scanAllError error - scanAllWarnings []string - - // ScanFolders tracking - scanFoldersCalled bool - scanFoldersFullScan bool - scanFoldersTargets []model.ScanTarget - scanFoldersError error - scanFoldersWarnings []string - - // Status tracking - statusResponse *scanner.StatusInfo - statusError error -} - -func (m *mockScanner) ScanAll(ctx context.Context, fullScan bool) ([]string, error) { - m.mu.Lock() - defer m.mu.Unlock() - - m.scanAllCalled = true - m.scanAllFullScan = fullScan - return m.scanAllWarnings, m.scanAllError -} - -func (m *mockScanner) ScanFolders(ctx context.Context, fullScan bool, targets []model.ScanTarget) ([]string, error) { - m.mu.Lock() - defer m.mu.Unlock() - - m.scanFoldersCalled = true - m.scanFoldersFullScan = fullScan - // Make a copy of targets to avoid race conditions - m.scanFoldersTargets = make([]model.ScanTarget, len(targets)) - copy(m.scanFoldersTargets, targets) - return m.scanFoldersWarnings, m.scanFoldersError -} - -func (m *mockScanner) Status(ctx context.Context) (*scanner.StatusInfo, error) { - m.mu.Lock() - defer m.mu.Unlock() - - if m.statusResponse == nil { - return &scanner.StatusInfo{}, m.statusError - } - return m.statusResponse, m.statusError -} - -// Helper methods for safe read access in tests -func (m *mockScanner) getScanAllCalled() bool { - m.mu.Lock() - defer m.mu.Unlock() - return m.scanAllCalled -} - -func (m *mockScanner) getScanAllFullScan() bool { - m.mu.Lock() - defer m.mu.Unlock() - return m.scanAllFullScan -} - -func (m *mockScanner) getScanFoldersCalled() bool { - m.mu.Lock() - defer m.mu.Unlock() - return m.scanFoldersCalled -} - -func (m *mockScanner) getScanFoldersFullScan() bool { - m.mu.Lock() - defer m.mu.Unlock() - return m.scanFoldersFullScan -} - -func (m *mockScanner) getScanFoldersTargets() []model.ScanTarget { - m.mu.Lock() - defer m.mu.Unlock() - // Return a copy to avoid race conditions - targets := make([]model.ScanTarget, len(m.scanFoldersTargets)) - copy(targets, m.scanFoldersTargets) - return targets -} diff --git a/tests/mock_scanner.go b/tests/mock_scanner.go new file mode 100644 index 000000000..52396723f --- /dev/null +++ b/tests/mock_scanner.go @@ -0,0 +1,120 @@ +package tests + +import ( + "context" + "sync" + + "github.com/navidrome/navidrome/model" +) + +// MockScanner implements scanner.Scanner for testing with proper synchronization +type MockScanner struct { + mu sync.Mutex + scanAllCalls []ScanAllCall + scanFoldersCalls []ScanFoldersCall + scanningStatus bool + statusResponse *model.ScannerStatus +} + +type ScanAllCall struct { + FullScan bool +} + +type ScanFoldersCall struct { + FullScan bool + Targets []model.ScanTarget +} + +func NewMockScanner() *MockScanner { + return &MockScanner{ + scanAllCalls: make([]ScanAllCall, 0), + scanFoldersCalls: make([]ScanFoldersCall, 0), + } +} + +func (m *MockScanner) ScanAll(_ context.Context, fullScan bool) ([]string, error) { + m.mu.Lock() + defer m.mu.Unlock() + + m.scanAllCalls = append(m.scanAllCalls, ScanAllCall{FullScan: fullScan}) + + return nil, nil +} + +func (m *MockScanner) ScanFolders(_ context.Context, fullScan bool, targets []model.ScanTarget) ([]string, error) { + m.mu.Lock() + defer m.mu.Unlock() + + // Make a copy of targets to avoid race conditions + targetsCopy := make([]model.ScanTarget, len(targets)) + copy(targetsCopy, targets) + + m.scanFoldersCalls = append(m.scanFoldersCalls, ScanFoldersCall{ + FullScan: fullScan, + Targets: targetsCopy, + }) + + return nil, nil +} + +func (m *MockScanner) Status(_ context.Context) (*model.ScannerStatus, error) { + m.mu.Lock() + defer m.mu.Unlock() + + if m.statusResponse != nil { + return m.statusResponse, nil + } + + return &model.ScannerStatus{ + Scanning: m.scanningStatus, + }, nil +} + +func (m *MockScanner) GetScanAllCallCount() int { + m.mu.Lock() + defer m.mu.Unlock() + return len(m.scanAllCalls) +} + +func (m *MockScanner) GetScanAllCalls() []ScanAllCall { + m.mu.Lock() + defer m.mu.Unlock() + // Return a copy to avoid race conditions + calls := make([]ScanAllCall, len(m.scanAllCalls)) + copy(calls, m.scanAllCalls) + return calls +} + +func (m *MockScanner) GetScanFoldersCallCount() int { + m.mu.Lock() + defer m.mu.Unlock() + return len(m.scanFoldersCalls) +} + +func (m *MockScanner) GetScanFoldersCalls() []ScanFoldersCall { + m.mu.Lock() + defer m.mu.Unlock() + // Return a copy to avoid race conditions + calls := make([]ScanFoldersCall, len(m.scanFoldersCalls)) + copy(calls, m.scanFoldersCalls) + return calls +} + +func (m *MockScanner) Reset() { + m.mu.Lock() + defer m.mu.Unlock() + m.scanAllCalls = make([]ScanAllCall, 0) + m.scanFoldersCalls = make([]ScanFoldersCall, 0) +} + +func (m *MockScanner) SetScanning(scanning bool) { + m.mu.Lock() + defer m.mu.Unlock() + m.scanningStatus = scanning +} + +func (m *MockScanner) SetStatusResponse(status *model.ScannerStatus) { + m.mu.Lock() + defer m.mu.Unlock() + m.statusResponse = status +}