diff --git a/cmd/scan.go b/cmd/scan.go index 9c92de966..8f2dc8acf 100644 --- a/cmd/scan.go +++ b/cmd/scan.go @@ -3,9 +3,7 @@ package cmd import ( "context" "encoding/gob" - "fmt" "os" - "strconv" "strings" "github.com/navidrome/navidrome/core" @@ -98,44 +96,7 @@ func runScanner(ctx context.Context) { } // parseTargets parses the comma-separated targets string into ScanTarget structs -// Format: "libraryID:folderPath,libraryID:folderPath,..." -// Example: "1:Music/Rock,1:Music/Jazz,2:Classical" func parseTargets(targetsStr string) ([]scanner.ScanTarget, error) { - parts := strings.Split(targetsStr, ",") - targets := make([]scanner.ScanTarget, 0, len(parts)) - - for _, part := range parts { - part = strings.TrimSpace(part) - if part == "" { - continue - } - - // Split by the first colon - colonIdx := strings.Index(part, ":") - if colonIdx == -1 { - return nil, fmt.Errorf("invalid target format: %q (expected libraryID:folderPath)", part) - } - - libIDStr := part[:colonIdx] - folderPath := part[colonIdx+1:] - - libID, err := strconv.Atoi(libIDStr) - if err != nil { - return nil, fmt.Errorf("invalid library ID %q: %w", libIDStr, err) - } - if libID <= 0 { - return nil, fmt.Errorf("invalid library ID %q", libIDStr) - } - - targets = append(targets, scanner.ScanTarget{ - LibraryID: libID, - FolderPath: folderPath, - }) - } - - if len(targets) == 0 { - return nil, fmt.Errorf("no valid targets found in %q", targetsStr) - } - - return targets, nil + targets := strings.Split(targetsStr, ",") + return scanner.ParseTargets(targets) } diff --git a/cmd/scan_test.go b/cmd/scan_test.go index c8dce51d6..fecd79c4b 100644 --- a/cmd/scan_test.go +++ b/cmd/scan_test.go @@ -8,14 +8,6 @@ import ( var _ = Describe("parseTargets", func() { Context("Valid targets", func() { - It("parses a single target", func() { - targets, err := parseTargets("1:Music/Rock") - Expect(err).ToNot(HaveOccurred()) - Expect(targets).To(HaveLen(1)) - Expect(targets[0].LibraryID).To(Equal(1)) - Expect(targets[0].FolderPath).To(Equal("Music/Rock")) - }) - It("parses multiple targets", func() { targets, err := parseTargets("1:Music/Rock,2:Jazz,3:Classical/Beethoven") Expect(err).ToNot(HaveOccurred()) @@ -25,59 +17,12 @@ var _ = Describe("parseTargets", func() { Expect(targets[2]).To(Equal(scanner.ScanTarget{LibraryID: 3, FolderPath: "Classical/Beethoven"})) }) - It("handles targets with spaces around commas", func() { - targets, err := parseTargets("1:Music/Rock And Roll, 2:Jazz , 3:Classical") - Expect(err).ToNot(HaveOccurred()) - Expect(targets).To(HaveLen(3)) - Expect(targets[0].FolderPath).To(Equal("Music/Rock And Roll")) - }) - - It("handles paths with colons after the first colon", func() { - targets, err := parseTargets("1:C:/Music/Rock") - Expect(err).ToNot(HaveOccurred()) - Expect(targets).To(HaveLen(1)) - Expect(targets[0].LibraryID).To(Equal(1)) - Expect(targets[0].FolderPath).To(Equal("C:/Music/Rock")) - }) - - It("handles empty folder paths", func() { - targets, err := parseTargets("1:,2:") - Expect(err).ToNot(HaveOccurred()) - Expect(targets).To(HaveLen(2)) - Expect(targets[0].FolderPath).To(BeEmpty()) - Expect(targets[1].FolderPath).To(BeEmpty()) - }) - }) - - Context("Invalid targets", func() { It("returns error for empty string", func() { _, err := parseTargets("") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("no valid targets")) }) - It("returns error for missing colon", func() { - _, err := parseTargets("1Music") - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("invalid target format")) - }) - - It("returns error for invalid library ID", func() { - _, err := parseTargets("abc:Music") - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("invalid library ID")) - }) - - It("return error on negative library ID", func() { - _, err := parseTargets("-1:Music") - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("invalid library ID")) - }) - - It("handles only whitespace", func() { - _, err := parseTargets(" , , ") - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("no valid targets")) - }) + // Other test cases are covered in scanner/controller_test.go }) }) diff --git a/scanner/controller.go b/scanner/controller.go index 28dbb7287..f6a80cec7 100644 --- a/scanner/controller.go +++ b/scanner/controller.go @@ -4,6 +4,8 @@ import ( "context" "errors" "fmt" + "strconv" + "strings" "sync/atomic" "time" @@ -37,11 +39,52 @@ func (st ScanTarget) String() string { return fmt.Sprintf("%d:%s", st.LibraryID, st.FolderPath) } +// ParseTargets parses scan targets strings into ScanTarget structs. +// Example: []string{"1:Music/Rock", "2:Classical"} +func ParseTargets(libFolders []string) ([]ScanTarget, error) { + targets := make([]ScanTarget, 0, len(libFolders)) + + for _, part := range libFolders { + part = strings.TrimSpace(part) + if part == "" { + continue + } + + // Split by the first colon + colonIdx := strings.Index(part, ":") + if colonIdx == -1 { + return nil, fmt.Errorf("invalid target format: %q (expected libraryID:folderPath)", part) + } + + libIDStr := part[:colonIdx] + folderPath := part[colonIdx+1:] + + libID, err := strconv.Atoi(libIDStr) + if err != nil { + return nil, fmt.Errorf("invalid library ID %q: %w", libIDStr, err) + } + if libID <= 0 { + return nil, fmt.Errorf("invalid library ID %q", libIDStr) + } + + targets = append(targets, ScanTarget{ + LibraryID: libID, + FolderPath: folderPath, + }) + } + + if len(targets) == 0 { + return nil, fmt.Errorf("no valid targets found") + } + + return targets, nil +} + type Scanner interface { - // ScanAll starts a full scan of the music library. This is a blocking operation. + // ScanAll starts a scan of all libraries. This is a blocking operation. ScanAll(ctx context.Context, fullScan bool) (warnings []string, err error) - // ScanFolders scans specific library/folder pairs without recursing into subdirectories. - // This is a blocking operation. + // 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 []ScanTarget) (warnings []string, err error) Status(context.Context) (*StatusInfo, error) } diff --git a/scanner/controller_test.go b/scanner/controller_test.go index e551e15b1..d1109b0be 100644 --- a/scanner/controller_test.go +++ b/scanner/controller_test.go @@ -53,3 +53,85 @@ var _ = Describe("Controller", func() { }) }) }) + +var _ = Describe("ParseTargets", func() { + It("parses multiple entries in slice", func() { + targets, err := scanner.ParseTargets([]string{"1:Music/Rock", "1:Music/Jazz", "2:Classical"}) + Expect(err).ToNot(HaveOccurred()) + Expect(targets).To(HaveLen(3)) + Expect(targets[0].LibraryID).To(Equal(1)) + Expect(targets[0].FolderPath).To(Equal("Music/Rock")) + Expect(targets[1].LibraryID).To(Equal(1)) + Expect(targets[1].FolderPath).To(Equal("Music/Jazz")) + Expect(targets[2].LibraryID).To(Equal(2)) + Expect(targets[2].FolderPath).To(Equal("Classical")) + }) + + It("handles empty folder paths", func() { + targets, err := scanner.ParseTargets([]string{"1:", "2:"}) + Expect(err).ToNot(HaveOccurred()) + Expect(targets).To(HaveLen(2)) + Expect(targets[0].FolderPath).To(Equal("")) + Expect(targets[1].FolderPath).To(Equal("")) + }) + + It("trims whitespace from entries", func() { + targets, err := scanner.ParseTargets([]string{" 1:Music/Rock", " 2:Classical "}) + Expect(err).ToNot(HaveOccurred()) + Expect(targets).To(HaveLen(2)) + Expect(targets[0].LibraryID).To(Equal(1)) + Expect(targets[0].FolderPath).To(Equal("Music/Rock")) + Expect(targets[1].LibraryID).To(Equal(2)) + Expect(targets[1].FolderPath).To(Equal("Classical")) + }) + + It("skips empty strings", func() { + targets, err := scanner.ParseTargets([]string{"1:Music/Rock", "", "2:Classical"}) + Expect(err).ToNot(HaveOccurred()) + Expect(targets).To(HaveLen(2)) + }) + + It("handles paths with colons", func() { + targets, err := scanner.ParseTargets([]string{"1:C:/Music/Rock", "2:/path:with:colons"}) + Expect(err).ToNot(HaveOccurred()) + Expect(targets).To(HaveLen(2)) + Expect(targets[0].FolderPath).To(Equal("C:/Music/Rock")) + Expect(targets[1].FolderPath).To(Equal("/path:with:colons")) + }) + + It("returns error for invalid format without colon", func() { + _, err := scanner.ParseTargets([]string{"1Music/Rock"}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("invalid target format")) + }) + + It("returns error for non-numeric library ID", func() { + _, err := scanner.ParseTargets([]string{"abc:Music/Rock"}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("invalid library ID")) + }) + + It("returns error for negative library ID", func() { + _, err := scanner.ParseTargets([]string{"-1:Music/Rock"}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("invalid library ID")) + }) + + It("returns error for zero library ID", func() { + _, err := scanner.ParseTargets([]string{"0:Music/Rock"}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("invalid library ID")) + }) + + It("returns error for empty input", func() { + _, err := scanner.ParseTargets([]string{}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("no valid targets found")) + }) + + It("returns error for all empty strings", func() { + _, err := scanner.ParseTargets([]string{"", " ", ""}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("no valid targets found")) + }) +}) diff --git a/server/subsonic/library_scanning.go b/server/subsonic/library_scanning.go index b6ccb9ae6..abd388428 100644 --- a/server/subsonic/library_scanning.go +++ b/server/subsonic/library_scanning.go @@ -1,11 +1,13 @@ package subsonic import ( + "fmt" "net/http" "time" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model/request" + "github.com/navidrome/navidrome/scanner" "github.com/navidrome/navidrome/server/subsonic/responses" "github.com/navidrome/navidrome/utils/req" ) @@ -44,15 +46,32 @@ func (api *Router) StartScan(r *http.Request) (*responses.Subsonic, error) { p := req.Params(r) fullScan := p.BoolOr("fullScan", false) + // Parse optional path parameters for selective scanning + var targets []scanner.ScanTarget + if pathParams, err := p.Strings("path"); err == nil && len(pathParams) > 0 { + targets, err = scanner.ParseTargets(pathParams) + if err != nil { + return nil, newError(responses.ErrorGeneric, fmt.Sprintf("Invalid path parameter: %v", err)) + } + } + go func() { start := time.Now() - log.Info(ctx, "Triggering manual scan", "fullScan", fullScan, "user", loggedUser.UserName) - _, err := api.scanner.ScanAll(ctx, fullScan) + var err error + + if len(targets) > 0 { + log.Info(ctx, "Triggering on-demand scan", "fullScan", fullScan, "targets", len(targets), "user", loggedUser.UserName) + _, err = api.scanner.ScanFolders(ctx, fullScan, targets) + } else { + log.Info(ctx, "Triggering on-demand scan", "fullScan", fullScan, "user", loggedUser.UserName) + _, err = api.scanner.ScanAll(ctx, fullScan) + } + if err != nil { log.Error(ctx, "Error scanning", err) return } - log.Info(ctx, "Manual scan complete", "user", loggedUser.UserName, "elapsed", time.Since(start)) + log.Info(ctx, "On-demand scan complete", "user", loggedUser.UserName, "elapsed", time.Since(start)) }() return api.GetScanStatus(r) diff --git a/server/subsonic/library_scanning_test.go b/server/subsonic/library_scanning_test.go new file mode 100644 index 000000000..413cd8606 --- /dev/null +++ b/server/subsonic/library_scanning_test.go @@ -0,0 +1,315 @@ +package subsonic + +import ( + "context" + "net/http/httptest" + + "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/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("LibraryScanning", func() { + var api *Router + var ms *mockScanner + + BeforeEach(func() { + ms = &mockScanner{} + api = &Router{scanner: ms} + }) + + Describe("StartScan", func() { + It("requires admin authentication", func() { + // Create non-admin user + ctx := request.WithUser(context.Background(), model.User{ + ID: "user-id", + IsAdmin: false, + }) + + // Create request + r := httptest.NewRequest("GET", "/rest/startScan", nil) + r = r.WithContext(ctx) + + // Call endpoint + response, err := api.StartScan(r) + + // Should return authorization error + Expect(err).To(HaveOccurred()) + Expect(response).To(BeNil()) + subErr, ok := err.(subError) + Expect(ok).To(BeTrue()) + Expect(subErr.code).To(Equal(responses.ErrorAuthorizationFail)) + }) + + It("triggers a full scan with no parameters", func() { + // Create admin user + ctx := request.WithUser(context.Background(), model.User{ + ID: "admin-id", + IsAdmin: true, + }) + + // Create request with no parameters + r := httptest.NewRequest("GET", "/rest/startScan", nil) + r = r.WithContext(ctx) + + // Call endpoint + response, err := api.StartScan(r) + + // Should succeed + Expect(err).ToNot(HaveOccurred()) + Expect(response).ToNot(BeNil()) + + // Verify ScanAll was called (eventually, since it's in a goroutine) + Eventually(func() bool { + return ms.scanAllCalled + }).Should(BeTrue()) + Expect(ms.scanAllFullScan).To(BeFalse()) + }) + + It("triggers a full scan with fullScan=true", func() { + // Create admin user + ctx := request.WithUser(context.Background(), model.User{ + ID: "admin-id", + IsAdmin: true, + }) + + // Create request with fullScan parameter + r := httptest.NewRequest("GET", "/rest/startScan?fullScan=true", nil) + r = r.WithContext(ctx) + + // Call endpoint + response, err := api.StartScan(r) + + // Should succeed + Expect(err).ToNot(HaveOccurred()) + Expect(response).ToNot(BeNil()) + + // Verify ScanAll was called with fullScan=true + Eventually(func() bool { + return ms.scanAllCalled + }).Should(BeTrue()) + Expect(ms.scanAllFullScan).To(BeTrue()) + }) + + It("triggers a selective scan with single path parameter", func() { + // Create admin user + ctx := request.WithUser(context.Background(), model.User{ + ID: "admin-id", + IsAdmin: true, + }) + + // Create request with single path parameter + r := httptest.NewRequest("GET", "/rest/startScan?path=1:Music/Rock", nil) + r = r.WithContext(ctx) + + // Call endpoint + response, err := api.StartScan(r) + + // Should succeed + Expect(err).ToNot(HaveOccurred()) + Expect(response).ToNot(BeNil()) + + // Verify ScanFolders was called with correct targets + Eventually(func() bool { + return ms.scanFoldersCalled + }).Should(BeTrue()) + Expect(ms.scanFoldersTargets).To(HaveLen(1)) + Expect(ms.scanFoldersTargets[0].LibraryID).To(Equal(1)) + Expect(ms.scanFoldersTargets[0].FolderPath).To(Equal("Music/Rock")) + }) + + It("triggers a selective scan with multiple path parameters", func() { + // Create admin user + ctx := request.WithUser(context.Background(), model.User{ + ID: "admin-id", + IsAdmin: true, + }) + + // Create request with multiple path parameters + r := httptest.NewRequest("GET", "/rest/startScan?path=1:Music/Reggae&path=2:Classical/Bach", nil) + r = r.WithContext(ctx) + + // Call endpoint + response, err := api.StartScan(r) + + // Should succeed + Expect(err).ToNot(HaveOccurred()) + Expect(response).ToNot(BeNil()) + + // Verify ScanFolders was called with correct targets + Eventually(func() bool { + return ms.scanFoldersCalled + }).Should(BeTrue()) + Expect(ms.scanFoldersTargets).To(HaveLen(2)) + Expect(ms.scanFoldersTargets[0].LibraryID).To(Equal(1)) + Expect(ms.scanFoldersTargets[0].FolderPath).To(Equal("Music/Reggae")) + Expect(ms.scanFoldersTargets[1].LibraryID).To(Equal(2)) + Expect(ms.scanFoldersTargets[1].FolderPath).To(Equal("Classical/Bach")) + }) + + It("triggers a selective full scan with path and fullScan parameters", func() { + // Create admin user + ctx := request.WithUser(context.Background(), model.User{ + ID: "admin-id", + IsAdmin: true, + }) + + // Create request with path and fullScan parameters + r := httptest.NewRequest("GET", "/rest/startScan?path=1:Music/Jazz&fullScan=true", nil) + r = r.WithContext(ctx) + + // Call endpoint + response, err := api.StartScan(r) + + // Should succeed + Expect(err).ToNot(HaveOccurred()) + Expect(response).ToNot(BeNil()) + + // Verify ScanFolders was called with fullScan=true + Eventually(func() bool { + return ms.scanFoldersCalled + }).Should(BeTrue()) + Expect(ms.scanFoldersFullScan).To(BeTrue()) + Expect(ms.scanFoldersTargets).To(HaveLen(1)) + }) + + It("returns error for invalid path format", func() { + // Create admin user + ctx := request.WithUser(context.Background(), model.User{ + ID: "admin-id", + IsAdmin: true, + }) + + // Create request with invalid path format (missing colon) + r := httptest.NewRequest("GET", "/rest/startScan?path=1MusicRock", nil) + r = r.WithContext(ctx) + + // Call endpoint + response, err := api.StartScan(r) + + // Should return error + Expect(err).To(HaveOccurred()) + Expect(response).To(BeNil()) + subErr, ok := err.(subError) + Expect(ok).To(BeTrue()) + Expect(subErr.code).To(Equal(responses.ErrorGeneric)) + }) + + It("returns error for invalid library ID", func() { + // Create admin user + ctx := request.WithUser(context.Background(), model.User{ + ID: "admin-id", + IsAdmin: true, + }) + + // Create request with invalid library ID + r := httptest.NewRequest("GET", "/rest/startScan?path=0:Music/Rock", nil) + r = r.WithContext(ctx) + + // Call endpoint + response, err := api.StartScan(r) + + // Should return error + Expect(err).To(HaveOccurred()) + Expect(response).To(BeNil()) + subErr, ok := err.(subError) + Expect(ok).To(BeTrue()) + Expect(subErr.code).To(Equal(responses.ErrorGeneric)) + }) + + It("handles URL-encoded paths", func() { + // Create admin user + ctx := request.WithUser(context.Background(), model.User{ + ID: "admin-id", + IsAdmin: true, + }) + + // Create request with URL-encoded path + r := httptest.NewRequest("GET", "/rest/startScan?path=1:The%20Beatles", nil) + r = r.WithContext(ctx) + + // Call endpoint + response, err := api.StartScan(r) + + // Should succeed + Expect(err).ToNot(HaveOccurred()) + Expect(response).ToNot(BeNil()) + + // Verify path was decoded correctly + Eventually(func() bool { + return ms.scanFoldersCalled + }).Should(BeTrue()) + Expect(ms.scanFoldersTargets[0].FolderPath).To(Equal("The Beatles")) + }) + }) + + Describe("GetScanStatus", func() { + It("returns scan status", func() { + // Setup mock scanner status + ms.statusResponse = &scanner.StatusInfo{ + Scanning: false, + Count: 100, + FolderCount: 10, + } + + // Create request + ctx := context.Background() + r := httptest.NewRequest("GET", "/rest/getScanStatus", nil) + r = r.WithContext(ctx) + + // Call endpoint + response, err := api.GetScanStatus(r) + + // Should succeed + Expect(err).ToNot(HaveOccurred()) + Expect(response).ToNot(BeNil()) + Expect(response.ScanStatus).ToNot(BeNil()) + Expect(response.ScanStatus.Scanning).To(BeFalse()) + Expect(response.ScanStatus.Count).To(Equal(int64(100))) + Expect(response.ScanStatus.FolderCount).To(Equal(int64(10))) + }) + }) +}) + +// mockScanner is a test double for the scanner.Scanner interface +type mockScanner struct { + // ScanAll tracking + scanAllCalled bool + scanAllFullScan bool + scanAllError error + scanAllWarnings []string + + // ScanFolders tracking + scanFoldersCalled bool + scanFoldersFullScan bool + scanFoldersTargets []scanner.ScanTarget + scanFoldersError error + scanFoldersWarnings []string + + // Status tracking + statusResponse *scanner.StatusInfo + statusError error +} + +func (m *mockScanner) ScanAll(ctx context.Context, fullScan bool) ([]string, error) { + m.scanAllCalled = true + m.scanAllFullScan = fullScan + return m.scanAllWarnings, m.scanAllError +} + +func (m *mockScanner) ScanFolders(ctx context.Context, fullScan bool, targets []scanner.ScanTarget) ([]string, error) { + m.scanFoldersCalled = true + m.scanFoldersFullScan = fullScan + m.scanFoldersTargets = targets + return m.scanFoldersWarnings, m.scanFoldersError +} + +func (m *mockScanner) Status(ctx context.Context) (*scanner.StatusInfo, error) { + if m.statusResponse == nil { + return &scanner.StatusInfo{}, m.statusError + } + return m.statusResponse, m.statusError +}