diff --git a/adapters/lastfm/agent.go b/adapters/lastfm/agent.go index dbd73c30d..60a0fcccc 100644 --- a/adapters/lastfm/agent.go +++ b/adapters/lastfm/agent.go @@ -388,7 +388,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 0f68bd46b..330d8cde9 100644 --- a/adapters/lastfm/agent_test.go +++ b/adapters/lastfm/agent_test.go @@ -481,6 +481,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/adapters/lastfm/client.go b/adapters/lastfm/client.go index 7b2601cf9..101e9a7d0 100644 --- a/adapters/lastfm/client.go +++ b/adapters/lastfm/client.go @@ -182,7 +182,11 @@ func (c *client) scrobble(ctx context.Context, sessionKey string, info ScrobbleI params.Add("album", info.album) params.Add("trackNumber", strconv.Itoa(info.trackNumber)) params.Add("mbid", info.mbid) - params.Add("duration", strconv.Itoa(info.duration)) + if info.duration > 30 { + params.Add("duration", strconv.Itoa(info.duration)) + } else { + log.Debug(ctx, "LastFM: scrobble duration is less than 30 seconds, duration will not be transmitted", "duration", info.duration) + } params.Add("albumArtist", info.albumArtist) params.Add("sk", sessionKey) resp, err := c.makeRequest(ctx, http.MethodPost, params, true) diff --git a/conf/configuration.go b/conf/configuration.go index efa9cbf9a..1cf97d0ca 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -210,6 +210,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 @@ -515,6 +516,12 @@ func Load(noConfigDump bool) { Server.UICoverArtSize = newValue } + // Check for negative values in durations and set to default (TODO: more than just LastFM.MinScrobbleduration) + if Server.LastFM.MinScrobbleDuration < 0 { + log.Warn("MinScrobbleDuration must be non-negative, setting to default", "value", Server.LastFM.MinScrobbleDuration, "newValue", consts.DefaultMinScrobbleDuration) + Server.LastFM.MinScrobbleDuration = consts.DefaultMinScrobbleDuration + } + if Server.Scanner.Extractor != consts.DefaultScannerExtractor { log.Warn("Invalid Scanner.Extractor, using default", "value", Server.Scanner.Extractor, "default", consts.DefaultScannerExtractor) Server.Scanner.Extractor = consts.DefaultScannerExtractor @@ -1087,6 +1094,7 @@ func setViperDefaults() { viper.SetDefault("lastfm.apikey", "") viper.SetDefault("lastfm.secret", "") viper.SetDefault("lastfm.scrobblefirstartistonly", false) + viper.SetDefault("lastfm.minscrobbleduration", consts.DefaultMinScrobbleDuration) 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 8c4c8ab86..043c9e64f 100644 --- a/conf/configuration_test.go +++ b/conf/configuration_test.go @@ -402,6 +402,31 @@ var _ = Describe("Configuration", func() { }) }) + Describe("MinScrobbleDuration", func() { + It("defaults to 30 seconds", func() { + conf.Load(true) + Expect(conf.Server.LastFM.MinScrobbleDuration).To(Equal(consts.DefaultMinScrobbleDuration)) + }) + + It("sets negative values to default", func() { + viper.SetDefault("lastfm.minscrobbleduration", "-10s") + conf.Load(true) + Expect(conf.Server.LastFM.MinScrobbleDuration).To(Equal(consts.DefaultMinScrobbleDuration)) + }) + + 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("Scanner.Extractor", func() { BeforeEach(func() { viper.Reset() diff --git a/consts/consts.go b/consts/consts.go index 42e9ec42f..11b783c20 100644 --- a/consts/consts.go +++ b/consts/consts.go @@ -83,6 +83,8 @@ const ( DefaultInfoLanguage = "en" + DefaultMinScrobbleDuration = 30 * time.Second + ArtistInfoTimeToLive = 24 * time.Hour AlbumInfoTimeToLive = 7 * 24 * time.Hour UpdateLastAccessFrequency = time.Minute