Split scanSet into smaller helpers and fix mockFS SkipDir (task y0)

2 files changed, 182 insertions, 59 deletions

diff --git a/internal/scanner/scanner.go b/internal/scanner/scanner.go
index ac62144..92bc6c3 100644
--- a/internal/scanner/scanner.go
+++ b/internal/scanner/scanner.go
@@ -330,27 +330,9 @@ func (s *FSScanner) scanSet(ctx context.Context, root, setPath string, progress
 
 	coverImages := s.gatherCoverImages(setPath)
 
-	// First pass: collect supported media files so we know the total and can
-	// safely close the path channel without blocking the walk.
-	var files []string
-	walkErr := s.fs.WalkDir(setPath, func(path string, d fs.DirEntry, err error) error {
-		if err != nil {
-			return fmt.Errorf("walk %q: %w", path, err)
-		}
-		if d.IsDir() {
-			if strings.HasPrefix(d.Name(), ".") && path != setPath {
-				return filepath.SkipDir
-			}
-			return nil
-		}
-		if !mediatype.IsSupportedExt(path) {
-			return nil
-		}
-		files = append(files, path)
-		return nil
-	})
-	if walkErr != nil {
-		return fmt.Errorf("scan set %q: %w", setName, walkErr)
+	files, err := s.collectFiles(setPath)
+	if err != nil {
+		return fmt.Errorf("scan set %q: %w", setName, err)
 	}
 
 	if progress != nil {
@@ -370,54 +352,20 @@ func (s *FSScanner) scanSet(ctx context.Context, root, setPath string, progress
 	}
 
 	var workerWg sync.WaitGroup
-
-	// Spawn worker goroutines that consume paths, probe files, and send results.
 	for i := 0; i < workers; i++ {
 		workerWg.Add(1)
 		go func() {
 			defer workerWg.Done()
-			for path := range pathChan {
-				if scanCtx.Err() != nil {
-					continue
-				}
-				result, err := s.probeFile(ctx, path, setPath, setID, setName, existing, coverImages, progress)
-				if err != nil {
-					sendErr(err)
-					return
-				}
-				if result == nil {
-					continue
-				}
-				select {
-				case resultChan <- *result:
-				case <-scanCtx.Done():
-					return
-				}
-			}
+			s.probeWorkerLoop(ctx, scanCtx, pathChan, resultChan, setPath, setID, setName, existing, coverImages, progress, sendErr)
 		}()
 	}
 
 	var newFiles int32
-
-	// Single writer goroutine serialises all SQLite inserts.
 	var writerWg sync.WaitGroup
 	writerWg.Add(1)
 	go func() {
 		defer writerWg.Done()
-		for result := range resultChan {
-			if scanCtx.Err() != nil {
-				continue
-			}
-			if _, err := s.store.CreateMedia(ctx, result.media); err != nil {
-				sendErr(fmt.Errorf("create media %q: %w", result.path, err))
-				continue
-			}
-			nf := atomic.AddInt32(&newFiles, 1)
-			if nf == 1 || nf%25 == 0 {
-				relPath, _ := filepath.Rel(setPath, result.path)
-				s.log().Info("scanner set progress", "name", setName, "new_media", nf, "latest", filepath.ToSlash(relPath))
-			}
-		}
+		s.writerLoop(ctx, scanCtx, resultChan, setName, setPath, &newFiles, sendErr)
 	}()
 
 	// Feed the worker pool.
@@ -457,6 +405,93 @@ func (s *FSScanner) scanSet(ctx context.Context, root, setPath string, progress
 	return nil
 }
 
+// collectFiles walks the set and returns the absolute paths of all supported media files.
+func (s *FSScanner) collectFiles(setPath string) ([]string, error) {
+	var files []string
+	walkErr := s.fs.WalkDir(setPath, func(path string, d fs.DirEntry, err error) error {
+		if err != nil {
+			return fmt.Errorf("walk %q: %w", path, err)
+		}
+		if d.IsDir() {
+			if strings.HasPrefix(d.Name(), ".") && path != setPath {
+				return filepath.SkipDir
+			}
+			return nil
+		}
+		if !mediatype.IsSupportedExt(path) {
+			return nil
+		}
+		files = append(files, path)
+		return nil
+	})
+	if walkErr != nil {
+		return nil, walkErr
+	}
+	return files, nil
+}
+
+// probeWorkerLoop consumes file paths, probes each one, and sends the result to resultChan.
+// It stops early if scanCtx is cancelled or if sendErr reports a fatal error.
+func (s *FSScanner) probeWorkerLoop(
+	ctx context.Context,
+	scanCtx context.Context,
+	pathChan <-chan string,
+	resultChan chan<- fileResult,
+	setPath string,
+	setID int64,
+	setName string,
+	existing map[string]model.Media,
+	coverImages map[string]string,
+	progress *model.ScanProgress,
+	sendErr func(error),
+) {
+	for path := range pathChan {
+		if scanCtx.Err() != nil {
+			continue
+		}
+		result, err := s.probeFile(ctx, path, setPath, setID, setName, existing, coverImages, progress)
+		if err != nil {
+			sendErr(err)
+			return
+		}
+		if result == nil {
+			continue
+		}
+		select {
+		case resultChan <- *result:
+		case <-scanCtx.Done():
+			return
+		}
+	}
+}
+
+// writerLoop reads probed results and inserts them into the store.
+// It logs progress every 25 files and tracks the total newFiles count.
+func (s *FSScanner) writerLoop(
+	ctx context.Context,
+	scanCtx context.Context,
+	resultChan <-chan fileResult,
+	setName string,
+	setPath string,
+	newFiles *int32,
+	sendErr func(error),
+) {
+	for result := range resultChan {
+		if scanCtx.Err() != nil {
+			continue
+		}
+		if _, err := s.store.CreateMedia(ctx, result.media); err != nil {
+			sendErr(fmt.Errorf("create media %q: %w", result.path, err))
+			continue
+		}
+		nf := atomic.AddInt32(newFiles, 1)
+		if nf == 1 || nf%25 == 0 {
+			relPath, _ := filepath.Rel(setPath, result.path)
+			s.log().Info("scanner set progress", "name", setName, "new_media", nf, "latest", filepath.ToSlash(relPath))
+		}
+	}
+}
+
 func findCoverImage(filePath string, coverImages map[string]string, setPath string) string {
 	for dir := filepath.Dir(filePath); len(dir) >= len(setPath); dir = filepath.Dir(dir) {
 		if coverRel, ok := coverImages[dir]; ok {
diff --git a/internal/scanner/scanner_test.go b/internal/scanner/scanner_test.go
index ec837bd..3a77f72 100644
--- a/internal/scanner/scanner_test.go
+++ b/internal/scanner/scanner_test.go
@@ -81,12 +81,30 @@ func (m *mockFS) WalkDir(root string, walkFn fs.WalkDirFunc) error {
 	if m.walkErr != nil {
 		return m.walkErr
 	}
+	var skipDirs []string
 	for _, e := range m.walkList {
-		if !strings.HasPrefix(filepath.Clean(e.path), filepath.Clean(root)) {
+		cleanRoot := filepath.Clean(root)
+		cleanPath := filepath.Clean(e.path)
+		if !strings.HasPrefix(cleanPath, cleanRoot) {
+			continue
+		}
+		skipped := false
+		for _, sd := range skipDirs {
+			if strings.HasPrefix(cleanPath, sd) {
+				skipped = true
+				break
+			}
+		}
+		if skipped {
 			continue
 		}
 		de := mockDirEntry{name: filepath.Base(e.path), isDir: e.isDir}
-		if err := walkFn(e.path, de, nil); err != nil {
+		err := walkFn(e.path, de, nil)
+		if err == filepath.SkipDir {
+			skipDirs = append(skipDirs, cleanPath)
+			continue
+		}
+		if err != nil {
 			return err
 		}
 	}
@@ -413,4 +431,74 @@ func TestFSScanner_Scan(t *testing.T) {
 	})
 }
 
+func TestFSScanner_collectFiles(t *testing.T) {
+	t.Run("collects supported media files excluding hidden directories", func(t *testing.T) {
+		mfs := &mockFS{
+			walkList: []walkEntry{
+				{path: "/music", isDir: true},
+				{path: "/music/track.mp3", isDir: false},
+				{path: "/music/cover.jpg", isDir: false},
+				{path: "/music/.hidden", isDir: true},
+				{path: "/music/.hidden/secret.mp3", isDir: false},
+			},
+		}
+		s := newTestScanner(nil, nil, nil, nil, mfs)
+		files, err := s.collectFiles("/music")
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		if len(files) != 2 {
+			t.Fatalf("expected 2 supported files, got %v", files)
+		}
+		if files[0] != "/music/track.mp3" || files[1] != "/music/cover.jpg" {
+			t.Fatalf("unexpected files: %v", files)
+		}
+	})
+
+	t.Run("skips dot directories", func(t *testing.T) {
+		mfs := &mockFS{
+			walkList: []walkEntry{
+				{path: "/music", isDir: true},
+				{path: "/music/.hidden", isDir: true},
+				{path: "/music/.hidden/secret.mp3", isDir: false},
+			},
+		}
+		s := newTestScanner(nil, nil, nil, nil, mfs)
+		files, err := s.collectFiles("/music")
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		if len(files) != 0 {
+			t.Fatalf("expected 0 files from dot dir, got %v", files)
+		}
+	})
+
+	t.Run("walk error", func(t *testing.T) {
+		mfs := &mockFS{walkErr: errors.New("walk failed")}
+		s := newTestScanner(nil, nil, nil, nil, mfs)
+		_, err := s.collectFiles("/bad")
+		if err == nil {
+			t.Fatal("expected error for walk failure")
+		}
+	})
+
+	t.Run("ignores unsupported extensions", func(t *testing.T) {
+		mfs := &mockFS{
+			walkList: []walkEntry{
+				{path: "/stuff", isDir: true},
+				{path: "/stuff/file.txt", isDir: false},
+				{path: "/stuff/notes.md", isDir: false},
+				{path: "/stuff/track.mp3", isDir: false},
+			},
+		}
+		s := newTestScanner(nil, nil, nil, nil, mfs)
+		files, err := s.collectFiles("/stuff")
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		if len(files) != 1 || files[0] != "/stuff/track.mp3" {
+			t.Fatalf("expected only mp3, got %v", files)
+		}
+	})
+}