mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-31 07:30:32 +00:00
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>
179 lines
5.0 KiB
Go
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)
|