diff --git a/adapters/lastfm/agent.go b/adapters/lastfm/agent.go index f967595e3..48864d574 100644 --- a/adapters/lastfm/agent.go +++ b/adapters/lastfm/agent.go @@ -383,7 +383,7 @@ func (l *lastfmAgent) Scrobble(ctx context.Context, userId string, s scrobbler.S return errors.Join(err, scrobbler.ErrNotAuthorized) } - if s.Duration <= 30 { + if s.Duration <= float32(conf.Server.LastFM.MinScrobbleDuration.Seconds()) { log.Debug(ctx, "Skipping Last.fm scrobble for short song", "track", s.Title, "duration", s.Duration) return nil } diff --git a/adapters/lastfm/agent_test.go b/adapters/lastfm/agent_test.go index 94024b9ab..c61b9ed18 100644 --- a/adapters/lastfm/agent_test.go +++ b/adapters/lastfm/agent_test.go @@ -472,6 +472,28 @@ var _ = Describe("lastfmAgent", func() { Expect(httpClient.SavedRequest).To(BeNil()) }) + It("skips songs according to the configured minimum duration", func() { + conf.Server.LastFM.MinScrobbleDuration = 60 * time.Second + track.Duration = 59 + httpClient.Res = http.Response{Body: io.NopCloser(bytes.NewBufferString("{}")), StatusCode: 200} + + err := agent.Scrobble(ctx, "user-1", scrobbler.Scrobble{MediaFile: *track, TimeStamp: time.Now()}) + + Expect(err).ToNot(HaveOccurred()) + Expect(httpClient.SavedRequest).To(BeNil()) + }) + + It("does not skip short songs when the minimum duration is disabled", func() { + conf.Server.LastFM.MinScrobbleDuration = 0 + track.Duration = 29 + httpClient.Res = http.Response{Body: io.NopCloser(bytes.NewBufferString("{}")), StatusCode: 200} + + err := agent.Scrobble(ctx, "user-1", scrobbler.Scrobble{MediaFile: *track, TimeStamp: time.Now()}) + + Expect(err).ToNot(HaveOccurred()) + Expect(httpClient.SavedRequest).ToNot(BeNil()) + }) + It("returns ErrNotAuthorized if user is not linked", func() { err := agent.Scrobble(ctx, "user-2", scrobbler.Scrobble{MediaFile: *track, TimeStamp: time.Now()}) Expect(err).To(MatchError(scrobbler.ErrNotAuthorized)) diff --git a/conf/configuration.go b/conf/configuration.go index fbbaaf252..961741d0d 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -208,6 +208,7 @@ type lastfmOptions struct { Secret string //nolint:gosec Language string ScrobbleFirstArtistOnly bool + MinScrobbleDuration time.Duration // Computed values Languages []string `conf:"-"` // Computed from Language, split by comma @@ -484,6 +485,12 @@ func Load(noConfigDump bool) { Server.UICoverArtSize = newValue } + // Check for negative values in durations and clamp to zero (TODO: more than just LastFM.MinScrobbleduration) + if Server.LastFM.MinScrobbleDuration < 0 { + log.Warn("MinScrobbleDuration must be non-negative, clamping", "value", Server.LastFM.MinScrobbleDuration, "newValue", 0) + Server.LastFM.MinScrobbleDuration = 0 + } + // Floor MaxImageSize at MaxImageUploadSize so accepted uploads can always be read back. imgSize, _ := humanize.ParseBytes(Server.MaxImageSize) uploadSize, _ := humanize.ParseBytes(Server.MaxImageUploadSize) @@ -1050,6 +1057,7 @@ func setViperDefaults() { viper.SetDefault("lastfm.apikey", "") viper.SetDefault("lastfm.secret", "") viper.SetDefault("lastfm.scrobblefirstartistonly", false) + viper.SetDefault("lastfm.minscrobbleduration", 30*time.Second) viper.SetDefault("deezer.enabled", true) viper.SetDefault("deezer.language", consts.DefaultInfoLanguage) viper.SetDefault("listenbrainz.enabled", true) diff --git a/conf/configuration_test.go b/conf/configuration_test.go index ac6477572..849ff1257 100644 --- a/conf/configuration_test.go +++ b/conf/configuration_test.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" "testing" + "time" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/log" @@ -383,6 +384,31 @@ var _ = Describe("Configuration", func() { }) }) + Describe("MinScrobbleDuration", func() { + It("defaults to 30 seconds", func() { + conf.Load(true) + Expect(conf.Server.LastFM.MinScrobbleDuration).To(Equal(30 * time.Second)) + }) + + It("clamps negative values to zero", func() { + viper.SetDefault("lastfm.minscrobbleduration", "-10s") + conf.Load(true) + Expect(conf.Server.LastFM.MinScrobbleDuration).To(Equal(time.Duration(0))) + }) + + It("disables with zero value", func() { + viper.SetDefault("lastfm.minscrobbleduration", "0s") + conf.Load(true) + Expect(conf.Server.LastFM.MinScrobbleDuration).To(Equal(time.Duration(0))) + }) + + It("preserves positive values", func() { + viper.SetDefault("lastfm.minscrobbleduration", "45s") + conf.Load(true) + Expect(conf.Server.LastFM.MinScrobbleDuration).To(Equal(45 * time.Second)) + }) + }) + Describe("EnforceNonRootUser", func() { It("defaults to false", func() { conf.Load(true)