Merge 0e93ebfc73eea68d56ddf23ff6febec820b56b91 into 7e16b6acb5c11e283fcd320a0abb82372a8ab0dd

model/mediafile.go

@@ -432,7 +432,7 @@ type MediaFileCursor iter.Seq2[MediaFile, error]
 type MediaFileRepository interface {
 	CountAll(options ...QueryOptions) (int64, error)
 	CountBySuffix(options ...QueryOptions) (map[string]int64, error)
-	Exists(id string) (bool, error)
+	Exists(ids ...string) (bool, error)
 	Put(m *MediaFile) error
 	UpdateProbeData(id string, data string) error
 	Get(id string) (*MediaFile, error)

persistence/mediafile_repository.go

@@ -147,8 +147,29 @@ func (r *mediaFileRepository) CountBySuffix(options ...model.QueryOptions) (map[
 	return counts, nil
 }
 
-func (r *mediaFileRepository) Exists(id string) (bool, error) {
-	return r.exists(Eq{"media_file.id": id})
+// Exists checks if all given media file IDs exist in the database and are accessible to the current user.
+// If no IDs are provided, it returns true. Duplicate IDs are handled correctly.
+// If any of the IDs do not exist or are not accessible, it returns false.
+func (r *mediaFileRepository) Exists(ids ...string) (bool, error) {
+	if len(ids) == 0 {
+		return true, nil
+	}
+	uniqueIds := slice.Unique(ids)
+
+	// Process in batches to avoid hitting SQLITE_MAX_VARIABLE_NUMBER limit (default 999)
+	const batchSize = 300
+	var totalCount int64
+	for batch := range slices.Chunk(uniqueIds, batchSize) {
+		existsQuery := Select("count(*) as exist").From("media_file").Where(Eq{"media_file.id": batch})
+		existsQuery = r.applyLibraryFilter(existsQuery)
+		var res struct{ Exist int64 }
+		err := r.queryOne(existsQuery, &res)
+		if err != nil {
+			return false, err
+		}
+		totalCount += res.Exist
+	}
+	return totalCount == int64(len(uniqueIds)), nil
 }
 
 func (r *mediaFileRepository) Put(m *model.MediaFile) error {

server/subsonic/media_annotation.go

@@ -169,15 +169,26 @@ func (api *Router) Scrobble(r *http.Request) (*responses.Subsonic, error) {
 	position := p.IntOr("position", 0)
 	ctx := r.Context()
 
+	// Validate all IDs exist before processing (OpenSubsonic compliance)
+	exists, err := api.ds.MediaFile(ctx).Exists(ids...)
+	if err != nil {
+		return nil, err
+	}
+	if !exists {
+		return nil, newError(responses.ErrorDataNotFound, "Media file not found")
+	}
+
 	if submission {
 		err := api.scrobblerSubmit(ctx, ids, times)
 		if err != nil {
 			log.Error(ctx, "Error registering scrobbles", "ids", ids, "times", times, err)
+			return nil, err
 		}
 	} else {
 		err := api.scrobblerNowPlaying(ctx, ids[0], position)
 		if err != nil {
 			log.Error(ctx, "Error setting NowPlaying", "id", ids[0], err)
+			return nil, err
 		}
 	}
 

server/subsonic/media_annotation_test.go

@@ -32,6 +32,12 @@ var _ = Describe("MediaAnnotationController", func() {
 	})
 
 	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() {
 			// Back-date the baseline so the assertion still passes on platforms
 			// with millisecond clock resolution (e.g. Windows).
@@ -74,10 +80,27 @@ var _ = Describe("MediaAnnotationController", func() {
 			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() {
-				_ = ds.MediaFile(ctx).Put(&model.MediaFile{ID: "12"})
 				ctx = request.WithPlayer(ctx, model.Player{ID: "player-1"})
 				req = newGetRequest("id=12", "submission=false")
 				req = req.WithContext(ctx)
@@ -99,6 +122,16 @@ var _ = Describe("MediaAnnotationController", func() {
 				Expect(playTracker.ReportedPlayback[0].State).To(Equal(scrobbler.StatePlaying))
 				Expect(playTracker.ReportedPlayback[0].ClientId).To(Equal("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())
+			})
 		})
 	})
 

tests/mock_mediafile_repo.go

@@ -44,12 +44,16 @@ func (m *MockMediaFileRepo) SetData(mfs model.MediaFiles) {
 	}
 }
 
-func (m *MockMediaFileRepo) Exists(id string) (bool, error) {
+func (m *MockMediaFileRepo) Exists(ids ...string) (bool, error) {
 	if m.Err {
 		return false, errors.New("error")
 	}
-	_, found := m.Data[id]
-	return found, nil
+	for _, id := range ids {
+		if _, found := m.Data[id]; !found {
+			return false, nil
+		}
+	}
+	return true, nil
 }
 
 func (m *MockMediaFileRepo) Get(id string) (*model.MediaFile, error) {