navidrome/server/subsonic/media_annotation_test.go
Deluan 80e9921d45 fix(subsonic): return error 70 when scrobble contains invalid IDs
Implements OpenSubsonic API clarification from PR #202: the scrobble
endpoint now validates all media file IDs before processing and returns
error code 70 (data not found) if any ID is invalid. This ensures the
entire operation fails atomically - no scrobbles are submitted when any
ID in the batch is invalid, matching the OpenSubsonic specification.

Changes include updating MediaFileRepository.Exists() to accept variadic
IDs for efficient batch validation via a single COUNT query.

See: https://github.com/opensubsonic/open-subsonic-api/pull/202
Signed-off-by: Deluan <deluan@navidrome.org>
2026-02-04 18:08:02 -05:00

179 lines
5.0 KiB
Go

package subsonic
import (
"context"
"fmt"
"net/http"
"time"
"github.com/navidrome/navidrome/core/scrobbler"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
"github.com/navidrome/navidrome/server/events"
"github.com/navidrome/navidrome/tests"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
var _ = Describe("MediaAnnotationController", func() {
var router *Router
var ds model.DataStore
var playTracker *fakePlayTracker
var eventBroker *fakeEventBroker
var ctx context.Context
BeforeEach(func() {
ctx = context.Background()
ds = &tests.MockDataStore{}
playTracker = &fakePlayTracker{}
eventBroker = &fakeEventBroker{}
router = New(ds, nil, nil, nil, nil, nil, nil, eventBroker, nil, playTracker, nil, nil, nil)
})
Describe("Scrobble", func() {
BeforeEach(func() {
// Populate mock with valid media files
_ = ds.MediaFile(ctx).Put(&model.MediaFile{ID: "12"})
_ = ds.MediaFile(ctx).Put(&model.MediaFile{ID: "34"})
})
It("submit all scrobbles with only the id", func() {
submissionTime := time.Now()
r := newGetRequest("id=12", "id=34")
_, err := router.Scrobble(r)
Expect(err).ToNot(HaveOccurred())
Expect(playTracker.Submissions).To(HaveLen(2))
Expect(playTracker.Submissions[0].Timestamp).To(BeTemporally(">", submissionTime))
Expect(playTracker.Submissions[0].TrackID).To(Equal("12"))
Expect(playTracker.Submissions[1].Timestamp).To(BeTemporally(">", submissionTime))
Expect(playTracker.Submissions[1].TrackID).To(Equal("34"))
})
It("submit all scrobbles with respective times", func() {
time1 := time.Now().Add(-20 * time.Minute)
t1 := time1.UnixMilli()
time2 := time.Now().Add(-10 * time.Minute)
t2 := time2.UnixMilli()
r := newGetRequest("id=12", "id=34", fmt.Sprintf("time=%d", t1), fmt.Sprintf("time=%d", t2))
_, err := router.Scrobble(r)
Expect(err).ToNot(HaveOccurred())
Expect(playTracker.Submissions).To(HaveLen(2))
Expect(playTracker.Submissions[0].Timestamp).To(BeTemporally("~", time1))
Expect(playTracker.Submissions[0].TrackID).To(Equal("12"))
Expect(playTracker.Submissions[1].Timestamp).To(BeTemporally("~", time2))
Expect(playTracker.Submissions[1].TrackID).To(Equal("34"))
})
It("checks if number of ids match number of times", func() {
r := newGetRequest("id=12", "id=34", "time=1111")
_, err := router.Scrobble(r)
Expect(err).To(HaveOccurred())
Expect(playTracker.Submissions).To(BeEmpty())
})
It("returns error when any id is invalid", func() {
r := newGetRequest("id=invalid")
_, err := router.Scrobble(r)
Expect(err).To(MatchError(ContainSubstring("not found")))
Expect(playTracker.Submissions).To(BeEmpty())
})
It("returns error and does not scrobble when mix of valid and invalid ids", func() {
r := newGetRequest("id=12", "id=invalid")
_, err := router.Scrobble(r)
Expect(err).To(MatchError(ContainSubstring("not found")))
Expect(playTracker.Submissions).To(BeEmpty())
})
Context("submission=false", func() {
var req *http.Request
BeforeEach(func() {
ctx = request.WithPlayer(ctx, model.Player{ID: "player-1"})
req = newGetRequest("id=12", "submission=false")
req = req.WithContext(ctx)
})
It("does not scrobble", func() {
_, err := router.Scrobble(req)
Expect(err).ToNot(HaveOccurred())
Expect(playTracker.Submissions).To(BeEmpty())
})
It("registers a NowPlaying", func() {
_, err := router.Scrobble(req)
Expect(err).ToNot(HaveOccurred())
Expect(playTracker.Playing).To(HaveLen(1))
Expect(playTracker.Playing).To(HaveKey("player-1"))
})
It("returns error when id is invalid", func() {
req = newGetRequest("id=invalid", "submission=false")
req = req.WithContext(ctx)
_, err := router.Scrobble(req)
Expect(err).To(MatchError(ContainSubstring("not found")))
Expect(playTracker.Playing).To(BeEmpty())
})
})
})
})
type fakePlayTracker struct {
Submissions []scrobbler.Submission
Playing map[string]string
Error error
}
func (f *fakePlayTracker) NowPlaying(_ context.Context, playerId string, _ string, trackId string, position int) error {
if f.Error != nil {
return f.Error
}
if f.Playing == nil {
f.Playing = make(map[string]string)
}
f.Playing[playerId] = trackId
return nil
}
func (f *fakePlayTracker) GetNowPlaying(_ context.Context) ([]scrobbler.NowPlayingInfo, error) {
return nil, f.Error
}
func (f *fakePlayTracker) Submit(_ context.Context, submissions []scrobbler.Submission) error {
if f.Error != nil {
return f.Error
}
f.Submissions = append(f.Submissions, submissions...)
return nil
}
var _ scrobbler.PlayTracker = (*fakePlayTracker)(nil)
type fakeEventBroker struct {
http.Handler
Events []events.Event
}
func (f *fakeEventBroker) SendMessage(_ context.Context, event events.Event) {
f.Events = append(f.Events, event)
}
func (f *fakeEventBroker) SendBroadcastMessage(_ context.Context, event events.Event) {
f.Events = append(f.Events, event)
}
var _ events.Broker = (*fakeEventBroker)(nil)