diff --git a/server/subsonic/media_annotation_test.go b/server/subsonic/media_annotation_test.go index 487335d1a..cf4e789b4 100644 --- a/server/subsonic/media_annotation_test.go +++ b/server/subsonic/media_annotation_test.go @@ -2,8 +2,10 @@ package subsonic import ( "context" + "errors" "fmt" "net/http" + "net/http/httptest" "time" "github.com/navidrome/navidrome/core/scrobbler" @@ -33,8 +35,6 @@ var _ = Describe("MediaAnnotationController", func() { Describe("Scrobble", func() { It("submit all scrobbles with only the id", func() { - // Back-date the baseline so the assertion still passes on platforms - // with millisecond clock resolution (e.g. Windows). submissionTime := time.Now().Add(-time.Second) r := newGetRequest("id=12", "id=34") @@ -102,6 +102,119 @@ var _ = Describe("MediaAnnotationController", func() { }) }) + Describe("Star/Unstar songs", func() { + // recordingMediaFileRepo is a spy: it records every SetStar call so tests + // can assert on whether and how the repository was invoked. + var mediaRepo *recordingMediaFileRepo + + BeforeEach(func() { + // Fresh spy before each test so recorded calls don't bleed between cases. + mediaRepo = &recordingMediaFileRepo{} + // alwaysMissingAlbumRepo and alwaysMissingArtistRepo are stubs: they + // always report Exists=false, steering the handler to treat every id as + // a media-file id without needing real album/artist data in the DB. + ds.(*tests.MockDataStore).MockedAlbum = &alwaysMissingAlbumRepo{} + ds.(*tests.MockDataStore).MockedArtist = &alwaysMissingArtistRepo{} + // Inject the spy into the mock data store so the router uses it. + ds.(*tests.MockDataStore).MockedMediaFile = mediaRepo + }) + + It("stars a song by id", func() { + // newGetRequest builds a pre-authenticated fake HTTP request; no real + // network or auth middleware is involved. + resp, err := router.Star(newGetRequest("id=song-1")) + + Expect(err).ToNot(HaveOccurred()) + Expect(resp.Status).To(Equal(responses.StatusOK)) + // Spy assertion: verify the repository received the correct arguments. + Expect(mediaRepo.SetStarCalls).To(Equal([]setStarCall{{Starred: true, ItemIDs: []string{"song-1"}}})) + // fakeEventBroker is a spy: it captures broadcast events so we can + // verify the success notification was fired with the right payload. + Expect(eventBroker.Events).To(HaveLen(1)) + Expect(eventBroker.Events[0].Data(eventBroker.Events[0])).To(Equal(`{"song":["song-1"]}`)) + }) + + It("unstars a song by id", func() { + resp, err := router.Unstar(newGetRequest("id=song-1")) + + Expect(err).ToNot(HaveOccurred()) + Expect(resp.Status).To(Equal(responses.StatusOK)) + Expect(mediaRepo.SetStarCalls).To(Equal([]setStarCall{{Starred: false, ItemIDs: []string{"song-1"}}})) + Expect(eventBroker.Events).To(HaveLen(1)) + Expect(eventBroker.Events[0].Data(eventBroker.Events[0])).To(Equal(`{"song":["song-1"]}`)) + }) + + // FAV-03: missing id parameter must be rejected before touching any dependency. + It("stars returns error when no id parameter is provided", func() { + _, err := router.Star(newGetRequest()) + + Expect(err).To(HaveOccurred()) + // Spy confirms the repository was never reached — validation failed first. + Expect(mediaRepo.SetStarCalls).To(BeEmpty()) + // Spy confirms no event was broadcast on failure. + Expect(eventBroker.Events).To(BeEmpty()) + }) + + It("unstars returns error when no id parameter is provided", func() { + _, err := router.Unstar(newGetRequest()) + + Expect(err).To(HaveOccurred()) + Expect(mediaRepo.SetStarCalls).To(BeEmpty()) + Expect(eventBroker.Events).To(BeEmpty()) + }) + + // FAV-05: repository failure must propagate and suppress the success event. + It("returns error and calls repository when star persistence fails", func() { + // Sabotage the spy by setting its Err field — this turns it into a stub + // that returns a controlled error, simulating a DB write failure. + mediaRepo.Err = errors.New("db failure") + _, err := router.Star(newGetRequest("id=song-1")) + + Expect(err).To(HaveOccurred()) + // Spy confirms the repository WAS called — the failure is from persistence, + // not from input validation. + Expect(mediaRepo.SetStarCalls).To(HaveLen(1)) + // No event should be broadcast when the write fails. + Expect(eventBroker.Events).To(BeEmpty()) + }) + + // FAV-06: same as FAV-05 for the Unstar path. + It("returns error and calls repository when unstar persistence fails", func() { + mediaRepo.Err = errors.New("db failure") + _, err := router.Unstar(newGetRequest("id=song-1")) + + Expect(err).To(HaveOccurred()) + Expect(mediaRepo.SetStarCalls).To(HaveLen(1)) + Expect(eventBroker.Events).To(BeEmpty()) + }) + + It("rejects unauthenticated star requests before favoriting the song", func() { + // Use httptest.ResponseRecorder to exercise the full HTTP stack including + // auth middleware, rather than calling the handler method directly. + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/star?u=missing&v=1.16.1&c=test&id=song-1", nil) + + router.ServeHTTP(w, r) + + // Auth middleware returns error code 40 before the handler runs. + Expect(w.Body.String()).To(ContainSubstring(`code="40"`)) + // Spy confirms the repository was never reached. + Expect(mediaRepo.SetStarCalls).To(BeEmpty()) + Expect(eventBroker.Events).To(BeEmpty()) + }) + + It("rejects unauthenticated unstar requests before unfavoriting the song", func() { + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/unstar?u=missing&v=1.16.1&c=test&id=song-1", nil) + + router.ServeHTTP(w, r) + + Expect(w.Body.String()).To(ContainSubstring(`code="40"`)) + Expect(mediaRepo.SetStarCalls).To(BeEmpty()) + Expect(eventBroker.Events).To(BeEmpty()) + }) + }) + Describe("ReportPlayback", func() { It("returns error when mediaId is missing", func() { r := newGetRequest("mediaType=song", "positionMs=0", "state=playing") @@ -215,6 +328,41 @@ func (f *fakePlayTracker) ReportPlayback(_ context.Context, params scrobbler.Rep var _ scrobbler.PlayTracker = (*fakePlayTracker)(nil) +type setStarCall struct { + Starred bool + ItemIDs []string +} + +type recordingMediaFileRepo struct { + model.MediaFileRepository + SetStarCalls []setStarCall + Err error +} + +func (r *recordingMediaFileRepo) SetStar(starred bool, itemIDs ...string) error { + r.SetStarCalls = append(r.SetStarCalls, setStarCall{ + Starred: starred, + ItemIDs: append([]string(nil), itemIDs...), + }) + return r.Err +} + +type alwaysMissingAlbumRepo struct { + model.AlbumRepository +} + +func (r *alwaysMissingAlbumRepo) Exists(string) (bool, error) { + return false, nil +} + +type alwaysMissingArtistRepo struct { + model.ArtistRepository +} + +func (r *alwaysMissingArtistRepo) Exists(string) (bool, error) { + return false, nil +} + type fakeEventBroker struct { http.Handler Events []events.Event