test: enhance GetCoverArt tests with context cancellation handling

Signed-off-by: Deluan <deluan@navidrome.org>

server/subsonic/media_retrieval_test.go

@@ -14,7 +14,6 @@ import (
 	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/conf/configtest"
 	"github.com/navidrome/navidrome/core/artwork"
-	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/server/subsonic/responses"
 	"github.com/navidrome/navidrome/tests"
@@ -25,7 +24,7 @@ import (
 var _ = Describe("MediaRetrievalController", func() {
 	var router *Router
 	var ds model.DataStore
-	mockRepo := &mockedMediaFile{}
+	mockRepo := &mockedMediaFile{MockMediaFileRepo: tests.MockMediaFileRepo{}}
 	var artwork *fakeArtwork
 	var w *httptest.ResponseRecorder
 
@@ -33,7 +32,7 @@ var _ = Describe("MediaRetrievalController", func() {
 		ds = &tests.MockDataStore{
 			MockedMediaFile: mockRepo,
 		}
-		artwork = &fakeArtwork{}
+		artwork = &fakeArtwork{data: "image data"}
 		router = New(ds, artwork, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil)
 		w = httptest.NewRecorder()
 		DeferCleanup(configtest.SetupConfig())
@@ -42,27 +41,19 @@ var _ = Describe("MediaRetrievalController", func() {
 
 	Describe("GetCoverArt", func() {
 		It("should return data for that id", func() {
-			artwork.data = "image data"
+			r := newGetRequest("id=34", "size=128", "square=true")
-			r := newGetRequest("id=34", "size=128")
 			_, err := router.GetCoverArt(w, r)
 
-			Expect(err).To(BeNil())
+			Expect(err).ToNot(HaveOccurred())
 			Expect(artwork.recvId).To(Equal("34"))
 			Expect(artwork.recvSize).To(Equal(128))
-			Expect(w.Body.String()).To(Equal(artwork.data))
+			Expect(artwork.recvSquare).To(BeTrue())
-		})
-
-		It("should return placeholder if id parameter is missing (mimicking Subsonic)", func() {
-			r := newGetRequest()
-			_, err := router.GetCoverArt(w, r)
-
-			Expect(err).To(BeNil())
 			Expect(w.Body.String()).To(Equal(artwork.data))
 		})
 
 		It("should fail when the file is not found", func() {
 			artwork.err = model.ErrNotFound
-			r := newGetRequest("id=34", "size=128")
+			r := newGetRequest("id=34", "size=128", "square=true")
 			_, err := router.GetCoverArt(w, r)
 
 			Expect(err).To(MatchError("Artwork not found"))
@@ -75,6 +66,45 @@ var _ = Describe("MediaRetrievalController", func() {
 
 			Expect(err).To(MatchError("weird error"))
 		})
+
+		When("client disconnects (context is cancelled)", func() {
+			It("should not call the service if cancelled before the call", func() {
+				// Create a request
+				ctx, cancel := context.WithCancel(context.Background())
+				r := newGetRequest("id=34", "size=128", "square=true")
+				r = r.WithContext(ctx)
+				cancel() // Cancel the context before the call
+
+				// Call the GetCoverArt method
+				_, err := router.GetCoverArt(w, r)
+
+				// Expect no error and no call to the artwork service
+				Expect(err).ToNot(HaveOccurred())
+				Expect(artwork.recvId).To(Equal(""))
+				Expect(artwork.recvSize).To(Equal(0))
+				Expect(artwork.recvSquare).To(BeFalse())
+				Expect(w.Body.String()).To(BeEmpty())
+			})
+
+			It("should not return data if cancelled during the call", func() {
+				// Create a request with a context that will be cancelled
+				ctx, cancel := context.WithCancel(context.Background())
+				defer cancel() // Ensure the context is cancelled after the test (best practices)
+				r := newGetRequest("id=34", "size=128", "square=true")
+				r = r.WithContext(ctx)
+				artwork.ctxCancelFunc = cancel // Set the cancel function to simulate cancellation in the service
+
+				// Call the GetCoverArt method
+				_, err := router.GetCoverArt(w, r)
+
+				// Expect no error and the service to have been called
+				Expect(err).ToNot(HaveOccurred())
+				Expect(artwork.recvId).To(Equal("34"))
+				Expect(artwork.recvSize).To(Equal(128))
+				Expect(artwork.recvSquare).To(BeTrue())
+				Expect(w.Body.String()).To(BeEmpty())
+			})
+		})
 	})
 
 	Describe("GetLyrics", func() {
@@ -111,10 +141,7 @@ var _ = Describe("MediaRetrievalController", func() {
 				},
 			})
 			response, err := router.GetLyrics(r)
-			if err != nil {
+			Expect(err).ToNot(HaveOccurred())
-				log.Error("You're missing something.", err)
-			}
-			Expect(err).To(BeNil())
 			Expect(response.Lyrics.Artist).To(Equal("Rick Astley"))
 			Expect(response.Lyrics.Title).To(Equal("Never Gonna Give You Up"))
 			Expect(response.Lyrics.Value).To(Equal("We're no strangers to love\nYou know the rules and so do I\n"))
@@ -123,10 +150,7 @@ var _ = Describe("MediaRetrievalController", func() {
 			r := newGetRequest("artist=Dheeraj", "title=Rinkiya+Ke+Papa")
 			mockRepo.SetData(model.MediaFiles{})
 			response, err := router.GetLyrics(r)
-			if err != nil {
+			Expect(err).ToNot(HaveOccurred())
-				log.Error("You're missing something.", err)
-			}
-			Expect(err).To(BeNil())
 			Expect(response.Lyrics.Artist).To(Equal(""))
 			Expect(response.Lyrics.Title).To(Equal(""))
 			Expect(response.Lyrics.Value).To(Equal(""))
@@ -148,14 +172,14 @@ var _ = Describe("MediaRetrievalController", func() {
 				},
 			})
 			response, err := router.GetLyrics(r)
-			Expect(err).To(BeNil())
+			Expect(err).ToNot(HaveOccurred())
 			Expect(response.Lyrics.Artist).To(Equal("Rick Astley"))
 			Expect(response.Lyrics.Title).To(Equal("Never Gonna Give You Up"))
 			Expect(response.Lyrics.Value).To(Equal("We're no strangers to love\nYou know the rules and so do I\n"))
 		})
 	})
 
-	Describe("getLyricsBySongId", func() {
+	Describe("GetLyricsBySongId", func() {
 		const syncedLyrics = "[00:18.80]We're no strangers to love\n[00:22.801]You know the rules and so do I"
 		const unsyncedLyrics = "We're no strangers to love\nYou know the rules and so do I"
 		const metadata = "[ar:Rick Astley]\n[ti:That one song]\n[offset:-100]"
@@ -295,10 +319,12 @@ var _ = Describe("MediaRetrievalController", func() {
 
 type fakeArtwork struct {
 	artwork.Artwork
-	data     string
+	data          string
-	err      error
+	err           error
-	recvId   string
+	ctxCancelFunc func()
-	recvSize int
+	recvId        string
+	recvSize      int
+	recvSquare    bool
 }
 
 func (c *fakeArtwork) GetOrPlaceholder(_ context.Context, id string, size int, square bool) (io.ReadCloser, time.Time, error) {
@@ -307,25 +333,29 @@ func (c *fakeArtwork) GetOrPlaceholder(_ context.Context, id string, size int, s
 	}
 	c.recvId = id
 	c.recvSize = size
+	c.recvSquare = square
+	if c.ctxCancelFunc != nil {
+		c.ctxCancelFunc() // Simulate context cancellation
+		return nil, time.Time{}, context.Canceled
+	}
 	return io.NopCloser(bytes.NewReader([]byte(c.data))), time.Time{}, nil
 }
 
 type mockedMediaFile struct {
-	model.MediaFileRepository
+	tests.MockMediaFileRepo
-	data model.MediaFiles
-}
-
-func (m *mockedMediaFile) SetData(mfs model.MediaFiles) {
-	m.data = mfs
 }
 
 func (m *mockedMediaFile) GetAll(opts ...model.QueryOptions) (model.MediaFiles, error) {
+	data, err := m.MockMediaFileRepo.GetAll(opts...)
+	if err != nil {
+		return nil, err
+	}
 	if len(opts) == 0 || opts[0].Sort != "lyrics, updated_at" {
-		return m.data, nil
+		return data, nil
 	}
 
 	// Hardcoded support for lyrics sorting
-	result := slices.Clone(m.data)
+	result := slices.Clone(data)
 	// Sort by presence of lyrics, then by updated_at. Respect the order specified in opts.
 	slices.SortFunc(result, func(a, b model.MediaFile) int {
 		diff := cmp.Or(
@@ -339,12 +369,3 @@ func (m *mockedMediaFile) GetAll(opts ...model.QueryOptions) (model.MediaFiles,
 	})
 	return result, nil
 }
-
-func (m *mockedMediaFile) Get(id string) (*model.MediaFile, error) {
-	for _, mf := range m.data {
-		if mf.ID == id {
-			return &mf, nil
-		}
-	}
-	return nil, model.ErrNotFound
-}