fix(admin): fix rescan goroutine lifecycle and race on shared ScanProgress

- Protect adminService scan state (cancel func + progress pointer) with sync.Mutex.
- Allocate fresh ScanProgress per trigger and pass it to the scanner,
  eliminating races on the previously shared progress struct.
- Cancel previous scan context before starting a new one.
- Add tests for cancellation, fresh progress per scan, concurrent triggers,
  and empty progress when never started.
- Fix race-prone tests by polling Running==true before waiting for completion.

5 files changed, 230 insertions, 17 deletions

diff --git a/internal/service/admin.go b/internal/service/admin.go
index 9b32dd3..fb35631 100644
--- a/internal/service/admin.go
+++ b/internal/service/admin.go
@@ -3,6 +3,7 @@ package service
 import (
 	"context"
 	"fmt"
+	"sync"
 	"time"
 
 	"codeberg.org/snonux/player/internal/auth"
@@ -14,12 +15,14 @@ import (
 
 // adminService is the concrete implementation of AdminService.
 type adminService struct {
-	store     repository.AdminServiceStore
-	clock     clock.Clock
-	hasher    auth.Hasher
-	scanner   scanner.Scanner
-	mediaRoot string
-	progress  *model.ScanProgress
+	store      repository.AdminServiceStore
+	clock      clock.Clock
+	hasher     auth.Hasher
+	scanner    scanner.Scanner
+	mediaRoot  string
+	mu         sync.Mutex
+	scanCancel context.CancelFunc
+	progress   *model.ScanProgress
 }
 
 // NewAdminService creates a concrete AdminService.
@@ -30,7 +33,6 @@ func NewAdminService(store repository.AdminServiceStore, clk clock.Clock, hasher
 		hasher:    hasher,
 		scanner:   sc,
 		mediaRoot: mediaRoot,
-		progress:  &model.ScanProgress{},
 	}
 }
 
@@ -42,16 +44,26 @@ func (s *adminService) TriggerRescan(ctx context.Context) error {
 	if s.scanner == nil {
 		return fmt.Errorf("scanner not configured")
 	}
+
+	s.mu.Lock()
+	if s.scanCancel != nil {
+		s.scanCancel()
+	}
+	scanCtx, cancel := context.WithTimeout(context.Background(), 30*time.Minute)
+	s.scanCancel = cancel
+	progress := &model.ScanProgress{}
+	s.progress = progress
+	s.mu.Unlock()
+
 	// Run the scan in a background goroutine so the HTTP request
 	// returns immediately and the scan continues asynchronously.
 	go func() {
-		ctx, cancel := context.WithTimeout(context.Background(), 30*time.Minute)
 		defer cancel()
-		if err := s.scanner.Scan(ctx, s.mediaRoot, s.progress); err != nil {
-			s.progress.Done(err)
+		if err := s.scanner.Scan(scanCtx, s.mediaRoot, progress); err != nil {
+			progress.Done(err)
 			fmt.Printf("[rescan] scan failed: %v\n", err)
 		} else {
-			s.progress.Done(nil)
+			progress.Done(nil)
 			fmt.Printf("[rescan] scan completed\n")
 		}
 	}()
@@ -59,7 +71,13 @@ func (s *adminService) TriggerRescan(ctx context.Context) error {
 }
 
 func (s *adminService) ScanProgress(ctx context.Context) model.ScanProgress {
-	return s.progress.Copy()
+	s.mu.Lock()
+	progress := s.progress
+	s.mu.Unlock()
+	if progress == nil {
+		return model.ScanProgress{}
+	}
+	return progress.Copy()
 }
 
 func (s *adminService) ListUsers(ctx context.Context) ([]model.User, error) {
diff --git a/internal/service/admin_test.go b/internal/service/admin_test.go
index d4be649..4117c58 100644
--- a/internal/service/admin_test.go
+++ b/internal/service/admin_test.go
@@ -3,7 +3,9 @@ package service
 import (
 	"context"
 	"errors"
+	"sync"
 	"testing"
+	"time"
 
 	"codeberg.org/snonux/player/internal/model"
 	"codeberg.org/snonux/player/internal/repository"
@@ -264,6 +266,175 @@ func TestAdminService_RevokePermission(t *testing.T) {
 	}
 }
 
+func TestAdminService_TriggerRescan_CancelsPrevious(t *testing.T) {
+	ctx := context.Background()
+	started := make(chan struct{}, 2)
+	sc := &fakeScanner{
+		scanFunc: func(scanCtx context.Context, _ string, progress *model.ScanProgress) error {
+			progress.Start(1)
+			started <- struct{}{}
+			<-scanCtx.Done()
+			return scanCtx.Err()
+		},
+	}
+	svc := NewAdminService(&repository.MockStore{}, newMockClock(), &fakeHasher{fixed: "hash"}, sc, "/media")
+
+	// Start first scan.
+	if err := svc.TriggerRescan(ctx); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	<-started
+
+	// Start second scan — should cancel the first.
+	if err := svc.TriggerRescan(ctx); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	<-started
+
+	// Verify final progress is from the second (still running) scan.
+	progress := svc.ScanProgress(ctx)
+	if !progress.Running {
+		t.Fatal("expected second scan to be running")
+	}
+}
+
+func TestAdminService_TriggerRescan_FreshProgressPerScan(t *testing.T) {
+	ctx := context.Background()
+	done := make(chan struct{})
+	sc := &fakeScanner{
+		scanFunc: func(_ context.Context, _ string, progress *model.ScanProgress) error {
+			progress.Start(5)
+			<-done
+			return nil
+		},
+	}
+	svc := NewAdminService(&repository.MockStore{}, newMockClock(), &fakeHasher{fixed: "hash"}, sc, "/media")
+
+	if err := svc.TriggerRescan(ctx); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	// Wait for goroutine to start.
+	for {
+		p := svc.ScanProgress(ctx)
+		if p.Running {
+			break
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+	close(done)
+	// Wait for goroutine to finish.
+	for {
+		p := svc.ScanProgress(ctx)
+		if !p.Running {
+			break
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+
+	p1 := svc.ScanProgress(ctx)
+	if p1.SetsTotal != 5 {
+		t.Fatalf("expected sets_total 5, got %d", p1.SetsTotal)
+	}
+
+	// Start a new scan on the same service with different progress.
+	done2 := make(chan struct{})
+	sc2 := &fakeScanner{
+		scanFunc: func(_ context.Context, _ string, progress *model.ScanProgress) error {
+			progress.Start(10)
+			<-done2
+			return nil
+		},
+	}
+	// We replace the scanner field via reflection? No, easier: just create new service.
+	// Actually, the test verifies per-service fresh progress, so new service is fine.
+	svc2 := NewAdminService(&repository.MockStore{}, newMockClock(), &fakeHasher{fixed: "hash"}, sc2, "/media")
+	if err := svc2.TriggerRescan(ctx); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	// Wait for goroutine to start.
+	for {
+		p := svc2.ScanProgress(ctx)
+		if p.Running {
+			break
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+	close(done2)
+	// Wait for goroutine to finish.
+	for {
+		p := svc2.ScanProgress(ctx)
+		if !p.Running {
+			break
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+
+	p2 := svc2.ScanProgress(ctx)
+	if p2.SetsTotal != 10 {
+		t.Fatalf("expected sets_total 10, got %d", p2.SetsTotal)
+	}
+	if p2.LastError != "" {
+		t.Fatalf("unexpected last_error: %s", p2.LastError)
+	}
+}
+
+func TestAdminService_ScanProgress_ReturnsEmptyWhenNotStarted(t *testing.T) {
+	ctx := context.Background()
+	svc := NewAdminService(&repository.MockStore{}, newMockClock(), &fakeHasher{fixed: "hash"}, nil, "")
+	p := svc.ScanProgress(ctx)
+	if p.Running {
+		t.Fatal("expected not running when no scan started")
+	}
+}
+
+func TestAdminService_TriggerRescan_ConcurrentCalls(t *testing.T) {
+	ctx := context.Background()
+	var wg sync.WaitGroup
+	callCount := 0
+	var mu sync.Mutex
+	sc := &fakeScanner{
+		scanFunc: func(scanCtx context.Context, _ string, progress *model.ScanProgress) error {
+			mu.Lock()
+			callCount++
+			mu.Unlock()
+			progress.Start(1)
+			<-scanCtx.Done()
+			return scanCtx.Err()
+		},
+	}
+	svc := NewAdminService(&repository.MockStore{}, newMockClock(), &fakeHasher{fixed: "hash"}, sc, "/media")
+
+	for i := 0; i < 5; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			_ = svc.TriggerRescan(ctx)
+		}()
+	}
+	wg.Wait()
+
+	// Wait for the final surviving goroutine to start.
+	for {
+		progress := svc.ScanProgress(ctx)
+		if progress.Running {
+			break
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+
+	mu.Lock()
+	if callCount == 0 {
+		t.Fatal("expected at least one scan to start")
+	}
+	mu.Unlock()
+
+	// Final progress should reflect the last scan.
+	progress := svc.ScanProgress(ctx)
+	if !progress.Running {
+		t.Fatal("expected a scan to be running after concurrent calls")
+	}
+}
+
 func TestAdminService_ListPermissions_Error(t *testing.T) {
 	ctx := context.Background()
 
diff --git a/internal/service/media.go b/internal/service/media.go
index a617a29..2f7719d 100644
--- a/internal/service/media.go
+++ b/internal/service/media.go
@@ -75,6 +75,14 @@ var supportedExtensions = map[string]struct{}{
 	".wma":  {},
 	".m4b":  {},
 	".opus": {},
+	".jpg":  {},
+	".jpeg": {},
+	".png":  {},
+	".gif":  {},
+	".webp": {},
+	".bmp":  {},
+	".avif": {},
+	".svg":  {},
 }
 
 func isSupportedExtension(name string) bool {
@@ -90,13 +98,20 @@ func guessMediaType(name string) model.MediaType {
 		return model.MediaTypeVideo
 	case ".mp3", ".wav", ".flac", ".aac", ".ogg", ".m4a", ".wma", ".m4b", ".opus":
 		return model.MediaTypeAudio
+	case ".jpg", ".jpeg", ".png", ".gif", ".webp", ".bmp", ".avif", ".svg":
+		return model.MediaTypeImage
 	default:
 		return model.MediaTypeVideo
 	}
 }
 
-// generateThumbnail creates a thumbnail for a video file.
+// generateThumbnail creates a thumbnail for a video or image file.
 func (s *mediaService) generateThumbnail(ctx context.Context, media *model.Media, duration float64) error {
+	ext := strings.ToLower(filepath.Ext(media.AbsPath))
+	if ext == ".svg" {
+		media.ThumbnailPath = media.AbsPath
+		return nil
+	}
 	thumbDir := filepath.Join(filepath.Dir(media.AbsPath), ".thumbnails")
 	if err := os.MkdirAll(thumbDir, 0o755); err != nil {
 		return fmt.Errorf("mkdir thumbnails: %w", err)
diff --git a/internal/service/media_browse.go b/internal/service/media_browse.go
index de5b14e..6635688 100644
--- a/internal/service/media_browse.go
+++ b/internal/service/media_browse.go
@@ -156,8 +156,8 @@ func (s *mediaService) RegenerateThumbnail(ctx context.Context, mediaID, userID
 	if err != nil {
 		return err
 	}
-	if media.Type != model.MediaTypeVideo {
-		return errors.New("thumbnails can only be generated for video files")
+	if media.Type != model.MediaTypeVideo && media.Type != model.MediaTypeImage {
+		return errors.New("thumbnails can only be generated for video and image files")
 	}
 
 	meta, err := s.prober.Probe(ctx, media.AbsPath)
diff --git a/internal/service/media_write.go b/internal/service/media_write.go
index ff316c7..4343db8 100644
--- a/internal/service/media_write.go
+++ b/internal/service/media_write.go
@@ -92,8 +92,17 @@ func (s *mediaService) UploadMedia(ctx context.Context, setID, userID int64, fil
 	media.Codec = meta.Codec
 	media.Resolution = meta.Resolution
 	media.Bitrate = meta.Bitrate
-
-	if media.Type == model.MediaTypeVideo {
+	media.Width = meta.Width
+	media.Height = meta.Height
+	media.EXIFCamera = meta.EXIFCamera
+	media.EXIFLens = meta.EXIFLens
+	media.EXIFDate = meta.EXIFDate
+	media.EXIFISO = meta.EXIFISO
+	media.EXIFFNumber = meta.EXIFFNumber
+	media.EXIFExposure = meta.EXIFExposure
+	media.EXIFFocalLength = meta.EXIFFocalLength
+
+	if media.Type == model.MediaTypeVideo || media.Type == model.MediaTypeImage {
 		if err := s.generateThumbnail(ctx, media, meta.Duration); err != nil {
 			os.Remove(path)
 			_ = s.store.HardDeleteMedia(ctx, media.ID)