refactor scrobblers (#383)

- no need to explicitly pass api key
- move packages up a level
- catch more errors by extended scrobbler interface with IsUserAuthenticated
- move interface to server
- delete scrobbber package, clients implicitly satisfy Scrobble

this also helps with gonic-lastfm-sync
This commit is contained in:
Senan Kelly
2023-09-27 01:13:00 +01:00
committed by GitHub
parent 32064d0279
commit f119659acf
27 changed files with 1100 additions and 1144 deletions

View File

@@ -22,8 +22,8 @@ import (
"go.senan.xyz/gonic"
"go.senan.xyz/gonic/db"
"go.senan.xyz/gonic/lastfm"
"go.senan.xyz/gonic/podcasts"
"go.senan.xyz/gonic/scrobble/lastfm"
"go.senan.xyz/gonic/server/ctrladmin/adminui"
"go.senan.xyz/gonic/server/ctrlbase"
)

View File

@@ -19,8 +19,8 @@ import (
"github.com/nfnt/resize"
"go.senan.xyz/gonic/db"
"go.senan.xyz/gonic/listenbrainz"
"go.senan.xyz/gonic/scanner"
"go.senan.xyz/gonic/scrobble/listenbrainz"
"go.senan.xyz/gonic/transcode"
)
@@ -97,15 +97,7 @@ func (c *Controller) ServeLinkLastFMDo(r *http.Request) *Response {
if token == "" {
return &Response{code: 400, err: "please provide a token"}
}
apiKey, err := c.DB.GetSetting(db.LastFMAPIKey)
if err != nil {
return &Response{redirect: r.Referer(), flashW: []string{fmt.Sprintf("couldn't get api key: %v", err)}}
}
secret, err := c.DB.GetSetting(db.LastFMSecret)
if err != nil {
return &Response{redirect: r.Referer(), flashW: []string{fmt.Sprintf("couldn't get secret: %v", err)}}
}
sessionKey, err := c.lastfmClient.GetSession(apiKey, secret, token)
sessionKey, err := c.lastfmClient.GetSession(token)
if err != nil {
return &Response{
redirect: "/admin/home",

View File

@@ -10,7 +10,7 @@ import (
"github.com/jinzhu/gorm"
"go.senan.xyz/gonic/db"
"go.senan.xyz/gonic/scrobble/lastfm"
"go.senan.xyz/gonic/lastfm"
)
const keepFor = 30 * time.Hour * 24
@@ -24,7 +24,7 @@ func New(db *db.DB, lastfmClient *lastfm.Client) *ArtistInfoCache {
return &ArtistInfoCache{db: db, lastfmClient: lastfmClient}
}
func (a *ArtistInfoCache) GetOrLookup(ctx context.Context, apiKey string, artistID int) (*db.ArtistInfo, error) {
func (a *ArtistInfoCache) GetOrLookup(ctx context.Context, artistID int) (*db.ArtistInfo, error) {
var artist db.Artist
if err := a.db.Find(&artist, "id=?", artistID).Error; err != nil {
return nil, fmt.Errorf("find artist in db: %w", err)
@@ -36,7 +36,7 @@ func (a *ArtistInfoCache) GetOrLookup(ctx context.Context, apiKey string, artist
}
if artistInfo.ID == 0 || artistInfo.Biography == "" /* prev not found maybe */ || time.Since(artistInfo.UpdatedAt) > keepFor {
return a.Lookup(ctx, apiKey, &artist)
return a.Lookup(ctx, &artist)
}
return &artistInfo, nil
@@ -50,7 +50,7 @@ func (a *ArtistInfoCache) Get(ctx context.Context, artistID int) (*db.ArtistInfo
return &artistInfo, nil
}
func (a *ArtistInfoCache) Lookup(ctx context.Context, apiKey string, artist *db.Artist) (*db.ArtistInfo, error) {
func (a *ArtistInfoCache) Lookup(ctx context.Context, artist *db.Artist) (*db.ArtistInfo, error) {
var artistInfo db.ArtistInfo
artistInfo.ID = artist.ID
@@ -58,7 +58,7 @@ func (a *ArtistInfoCache) Lookup(ctx context.Context, apiKey string, artist *db.
return nil, fmt.Errorf("first or create artist info: %w", err)
}
info, err := a.lastfmClient.ArtistGetInfo(apiKey, artist.Name)
info, err := a.lastfmClient.ArtistGetInfo(artist.Name)
if err != nil {
return nil, fmt.Errorf("get upstream info: %w", err)
}
@@ -77,7 +77,7 @@ func (a *ArtistInfoCache) Lookup(ctx context.Context, apiKey string, artist *db.
url, _ := a.lastfmClient.StealArtistImage(info.URL)
artistInfo.ImageURL = url
topTracksResponse, err := a.lastfmClient.ArtistGetTopTracks(apiKey, artist.Name)
topTracksResponse, err := a.lastfmClient.ArtistGetTopTracks(artist.Name)
if err != nil {
return nil, fmt.Errorf("get top tracks: %w", err)
}
@@ -94,7 +94,7 @@ func (a *ArtistInfoCache) Lookup(ctx context.Context, apiKey string, artist *db.
return &artistInfo, nil
}
func (a *ArtistInfoCache) Refresh(apiKey string, interval time.Duration) error {
func (a *ArtistInfoCache) Refresh(interval time.Duration) error {
ticker := time.NewTicker(interval)
for range ticker.C {
q := a.db.
@@ -110,7 +110,7 @@ func (a *ArtistInfoCache) Refresh(apiKey string, interval time.Duration) error {
continue
}
if _, err := a.Lookup(context.Background(), apiKey, &artist); err != nil {
if _, err := a.Lookup(context.Background(), &artist); err != nil {
log.Printf("error looking up non cached artist %s: %v", artist.Name, err)
continue
}

View File

@@ -9,9 +9,9 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.senan.xyz/gonic/db"
"go.senan.xyz/gonic/lastfm"
"go.senan.xyz/gonic/lastfm/mockclient"
"go.senan.xyz/gonic/mockfs"
"go.senan.xyz/gonic/scrobble/lastfm"
"go.senan.xyz/gonic/scrobble/lastfm/mockclient"
)
func TestInfoCache(t *testing.T) {
@@ -29,20 +29,25 @@ func TestInfoCache(t *testing.T) {
assert.Nil(artist.Info)
var count atomic.Int32
lastfmClient := lastfm.NewClientCustom(mockclient.New(t, func(w http.ResponseWriter, r *http.Request) {
switch method := r.URL.Query().Get("method"); method {
case "artist.getInfo":
count.Add(1)
w.Write(mockclient.ArtistGetInfoResponse)
case "artist.getTopTracks":
w.Write(mockclient.ArtistGetTopTracksResponse)
}
}))
lastfmClient := lastfm.NewClientCustom(
mockclient.New(t, func(w http.ResponseWriter, r *http.Request) {
switch method := r.URL.Query().Get("method"); method {
case "artist.getInfo":
count.Add(1)
w.Write(mockclient.ArtistGetInfoResponse)
case "artist.getTopTracks":
w.Write(mockclient.ArtistGetTopTracksResponse)
}
}),
func() (apiKey string, secret string, err error) {
return "", "", nil
},
)
cache := New(m.DB(), lastfmClient)
_, err := cache.GetOrLookup(context.Background(), "", artist.ID)
_, err := cache.GetOrLookup(context.Background(), artist.ID)
require.NoError(t, err)
_, err = cache.GetOrLookup(context.Background(), "", artist.ID)
_, err = cache.GetOrLookup(context.Background(), artist.ID)
require.NoError(t, err)
require.Equal(t, int32(1), count.Load())

View File

@@ -7,11 +7,12 @@ import (
"io"
"log"
"net/http"
"time"
"go.senan.xyz/gonic/db"
"go.senan.xyz/gonic/jukebox"
"go.senan.xyz/gonic/lastfm"
"go.senan.xyz/gonic/podcasts"
"go.senan.xyz/gonic/scrobble"
"go.senan.xyz/gonic/scrobble/lastfm"
"go.senan.xyz/gonic/server/ctrlbase"
"go.senan.xyz/gonic/server/ctrlsubsonic/artistinfocache"
"go.senan.xyz/gonic/server/ctrlsubsonic/params"
@@ -39,6 +40,11 @@ func PathsOf(paths []MusicPath) []string {
return r
}
type Scrobbler interface {
IsUserAuthenticated(user *db.User) bool
Scrobble(user *db.User, track *db.Track, stamp time.Time, submission bool) error
}
type Controller struct {
*ctrlbase.Controller
MusicPaths []MusicPath
@@ -46,7 +52,7 @@ type Controller struct {
CacheAudioPath string
CacheCoverPath string
Jukebox *jukebox.Jukebox
Scrobblers []scrobble.Scrobbler
Scrobblers []Scrobbler
Podcasts *podcasts.Podcasts
Transcoder transcode.Transcoder
LastFMClient *lastfm.Client

View File

@@ -3,6 +3,7 @@ package ctrlsubsonic
import (
"errors"
"fmt"
"log"
"math"
"net/http"
"net/url"
@@ -318,14 +319,10 @@ func (c *Controller) ServeGetArtistInfoTwo(r *http.Request) *spec.Response {
sub := spec.NewResponse()
sub.ArtistInfoTwo = &spec.ArtistInfo{}
apiKey, _ := c.DB.GetSetting(db.LastFMAPIKey)
if apiKey == "" {
return sub
}
info, err := c.ArtistInfoCache.GetOrLookup(r.Context(), apiKey, artist.ID)
info, err := c.ArtistInfoCache.GetOrLookup(r.Context(), artist.ID)
if err != nil {
return spec.NewError(0, "fetching artist info: %v", err)
log.Printf("error fetching artist info from lastfm: %v", err)
return sub
}
sub.ArtistInfoTwo.Biography = info.Biography
@@ -541,13 +538,10 @@ func (c *Controller) ServeGetTopSongs(r *http.Request) *spec.Response {
return spec.NewError(0, "finding artist by name: %v", err)
}
apiKey, _ := c.DB.GetSetting(db.LastFMAPIKey)
if apiKey == "" {
return spec.NewResponse()
}
info, err := c.ArtistInfoCache.GetOrLookup(r.Context(), apiKey, artist.ID)
info, err := c.ArtistInfoCache.GetOrLookup(r.Context(), artist.ID)
if err != nil {
return spec.NewError(0, "fetching artist top tracks: %v", err)
log.Printf("error fetching artist info from lastfm: %v", err)
return spec.NewResponse()
}
sub := spec.NewResponse()
@@ -597,10 +591,6 @@ func (c *Controller) ServeGetSimilarSongs(r *http.Request) *spec.Response {
if err != nil || id.Type != specid.Track {
return spec.NewError(10, "please provide an track `id` parameter")
}
apiKey, _ := c.DB.GetSetting(db.LastFMAPIKey)
if apiKey == "" {
return spec.NewResponse()
}
var track db.Track
err = c.DB.
@@ -612,10 +602,12 @@ func (c *Controller) ServeGetSimilarSongs(r *http.Request) *spec.Response {
return spec.NewError(10, "couldn't find a track with that id")
}
similarTracks, err := c.LastFMClient.TrackGetSimilarTracks(apiKey, track.TagTrackArtist, track.TagTitle)
similarTracks, err := c.LastFMClient.TrackGetSimilarTracks(track.TagTrackArtist, track.TagTitle)
if err != nil {
return spec.NewError(0, "fetching track similar tracks: %v", err)
log.Printf("error fetching similar songs from lastfm: %v", err)
return spec.NewResponse()
}
if len(similarTracks.Tracks) == 0 {
return spec.NewError(70, "no similar songs found for track: %v", track.TagTitle)
}
@@ -666,11 +658,6 @@ func (c *Controller) ServeGetSimilarSongsTwo(r *http.Request) *spec.Response {
return spec.NewError(10, "please provide an artist `id` parameter")
}
apiKey, _ := c.DB.GetSetting(db.LastFMAPIKey)
if apiKey == "" {
return spec.NewResponse()
}
var artist db.Artist
err = c.DB.
Where("id=?", id.Value).
@@ -680,9 +667,10 @@ func (c *Controller) ServeGetSimilarSongsTwo(r *http.Request) *spec.Response {
return spec.NewError(0, "artist with id `%s` not found", id)
}
similarArtists, err := c.LastFMClient.ArtistGetSimilar(apiKey, artist.Name)
similarArtists, err := c.LastFMClient.ArtistGetSimilar(artist.Name)
if err != nil {
return spec.NewError(0, "fetching artist similar artists: %v", err)
log.Printf("error fetching artist info from lastfm: %v", err)
return spec.NewResponse()
}
if len(similarArtists.Artists) == 0 {
return spec.NewError(0, "no similar artist found for: %v", artist.Name)

View File

@@ -74,6 +74,9 @@ func (c *Controller) ServeScrobble(r *http.Request) *spec.Response {
var scrobbleErrs []error
for _, scrobbler := range c.Scrobblers {
if !scrobbler.IsUserAuthenticated(user) {
continue
}
if err := scrobbler.Scrobble(user, track, optStamp, optSubmission); err != nil {
scrobbleErrs = append(scrobbleErrs, err)
}