mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-31 07:30:32 +00:00
Merge branch 'master' into album-record-label
This commit is contained in:
commit
a501c5cd11
@ -5,7 +5,6 @@ import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"slices"
|
||||
"strings"
|
||||
|
||||
@ -15,6 +14,7 @@ import (
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/utils/cache"
|
||||
"github.com/navidrome/navidrome/utils/httpclient"
|
||||
"github.com/navidrome/navidrome/utils/slice"
|
||||
)
|
||||
|
||||
@ -36,9 +36,7 @@ func deezerConstructor(dataStore model.DataStore) agents.Interface {
|
||||
dataStore: dataStore,
|
||||
languages: conf.Server.Deezer.Languages,
|
||||
}
|
||||
httpClient := &http.Client{
|
||||
Timeout: consts.DefaultHttpClientTimeOut,
|
||||
}
|
||||
httpClient := httpclient.New(consts.DefaultHttpClientTimeOut)
|
||||
cachedHttpClient := cache.NewHTTPClient(httpClient, consts.DefaultHttpClientTimeOut)
|
||||
agent.client = newClient(cachedHttpClient)
|
||||
return agent
|
||||
|
||||
@ -18,6 +18,7 @@ import (
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/utils/cache"
|
||||
"github.com/navidrome/navidrome/utils/httpclient"
|
||||
"golang.org/x/net/html"
|
||||
)
|
||||
|
||||
@ -59,9 +60,7 @@ func lastFMConstructor(ds model.DataStore) *lastfmAgent {
|
||||
secret: conf.Server.LastFM.Secret,
|
||||
sessionKeys: &agents.SessionKeys{DataStore: ds, KeyName: sessionKeyProperty},
|
||||
}
|
||||
hc := &http.Client{
|
||||
Timeout: consts.DefaultHttpClientTimeOut,
|
||||
}
|
||||
hc := httpclient.New(consts.DefaultHttpClientTimeOut)
|
||||
chc := cache.NewHTTPClient(hc, consts.DefaultHttpClientTimeOut)
|
||||
l.httpClient = chc
|
||||
l.client = newClient(l.apiKey, l.secret, chc)
|
||||
|
||||
@ -18,6 +18,7 @@ import (
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/model/request"
|
||||
"github.com/navidrome/navidrome/server"
|
||||
"github.com/navidrome/navidrome/utils/httpclient"
|
||||
"github.com/navidrome/navidrome/utils/req"
|
||||
)
|
||||
|
||||
@ -41,9 +42,7 @@ func NewRouter(ds model.DataStore) *Router {
|
||||
sessionKeys: &agents.SessionKeys{DataStore: ds, KeyName: sessionKeyProperty},
|
||||
}
|
||||
r.Handler = r.routes()
|
||||
hc := &http.Client{
|
||||
Timeout: consts.DefaultHttpClientTimeOut,
|
||||
}
|
||||
hc := httpclient.New(consts.DefaultHttpClientTimeOut)
|
||||
r.client = newClient(r.apiKey, r.secret, hc)
|
||||
return r
|
||||
}
|
||||
|
||||
@ -3,7 +3,6 @@ package listenbrainz
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"net/http"
|
||||
|
||||
"github.com/navidrome/navidrome/conf"
|
||||
"github.com/navidrome/navidrome/consts"
|
||||
@ -12,6 +11,7 @@ import (
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/utils/cache"
|
||||
"github.com/navidrome/navidrome/utils/httpclient"
|
||||
"github.com/navidrome/navidrome/utils/slice"
|
||||
)
|
||||
|
||||
@ -33,9 +33,7 @@ func listenBrainzConstructor(ds model.DataStore) *listenBrainzAgent {
|
||||
sessionKeys: &agents.SessionKeys{DataStore: ds, KeyName: sessionKeyProperty},
|
||||
baseURL: conf.Server.ListenBrainz.BaseURL,
|
||||
}
|
||||
hc := &http.Client{
|
||||
Timeout: consts.DefaultHttpClientTimeOut,
|
||||
}
|
||||
hc := httpclient.New(consts.DefaultHttpClientTimeOut)
|
||||
chc := cache.NewHTTPClient(hc, consts.DefaultHttpClientTimeOut)
|
||||
l.client = newClient(l.baseURL, chc)
|
||||
return l
|
||||
|
||||
@ -16,6 +16,7 @@ import (
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/model/request"
|
||||
"github.com/navidrome/navidrome/server"
|
||||
"github.com/navidrome/navidrome/utils/httpclient"
|
||||
)
|
||||
|
||||
type sessionKeysRepo interface {
|
||||
@ -37,9 +38,7 @@ func NewRouter(ds model.DataStore) *Router {
|
||||
sessionKeys: &agents.SessionKeys{DataStore: ds, KeyName: sessionKeyProperty},
|
||||
}
|
||||
r.Handler = r.routes()
|
||||
hc := &http.Client{
|
||||
Timeout: consts.DefaultHttpClientTimeOut,
|
||||
}
|
||||
hc := httpclient.New(consts.DefaultHttpClientTimeOut)
|
||||
r.client = newClient(conf.Server.ListenBrainz.BaseURL, hc)
|
||||
return r
|
||||
}
|
||||
|
||||
@ -648,7 +648,7 @@ func refreshItems(ctx context.Context, ds model.DataStore, targets []model.Artwo
|
||||
for _, t := range targets {
|
||||
kind, id := t.Kind, t.ID
|
||||
// artwork.Refresh would happily queue an id that does not exist, orphaning a queue row.
|
||||
if _, err := artworkItemName(ctx, ds, kind, id); err != nil {
|
||||
if _, err := artwork.ItemName(ctx, ds, kind, id); err != nil {
|
||||
log.Error(ctx, "Item not found", "kind", kind, "id", id, err)
|
||||
failed++
|
||||
continue
|
||||
@ -963,7 +963,7 @@ func runExplain(ctx context.Context, args []string) {
|
||||
}
|
||||
kind, id := targets[0].Kind, targets[0].ID
|
||||
|
||||
name, err := artworkItemName(ctx, ds, kind, id)
|
||||
name, err := artwork.ItemName(ctx, ds, kind, id)
|
||||
if err != nil {
|
||||
log.Fatal(ctx, "Item not found", "kind", kind, "id", id, err)
|
||||
}
|
||||
@ -1005,60 +1005,3 @@ func runExplain(ctx context.Context, args []string) {
|
||||
log.Fatal(ctx, "Failed to resolve artwork", "kind", kind, "id", id, rep.resolveErr)
|
||||
}
|
||||
}
|
||||
|
||||
// artworkItemName looks the entity up under its own kind, so a mismatched kind/id pair is
|
||||
// reported as not found instead of silently explaining another entity's artwork.
|
||||
func artworkItemName(ctx context.Context, ds model.DataStore, kind model.Kind, id string) (string, error) {
|
||||
switch kind {
|
||||
case model.KindArtistArtwork:
|
||||
ar, err := ds.Artist(ctx).Get(id)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
return ar.Name, nil
|
||||
case model.KindAlbumArtwork:
|
||||
al, err := ds.Album(ctx).Get(id)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
return al.Name, nil
|
||||
case model.KindPlaylistArtwork:
|
||||
pls, err := ds.Playlist(ctx).Get(id)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
return pls.Name, nil
|
||||
case model.KindRadioArtwork:
|
||||
rd, err := ds.Radio(ctx).Get(id)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
return rd.Name, nil
|
||||
case model.KindMediaFileArtwork:
|
||||
mf, err := ds.MediaFile(ctx).Get(id)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
return mf.Title, nil
|
||||
case model.KindDiscArtwork:
|
||||
return discArtworkName(ctx, ds, id)
|
||||
}
|
||||
return "", fmt.Errorf("unsupported kind %q", kind.Prefix())
|
||||
}
|
||||
|
||||
func discArtworkName(ctx context.Context, ds model.DataStore, id string) (string, error) {
|
||||
albumID, discNumber, err := model.ParseDiscArtworkID(id)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
al, err := ds.Album(ctx).Get(albumID)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
name := fmt.Sprintf("%s (disc %d)", al.Name, discNumber)
|
||||
// The subtitle is itself a DiscArtPriority candidate, so name it where the chain can be read against it.
|
||||
if subtitle := strings.TrimSpace(al.Discs[discNumber]); subtitle != "" {
|
||||
name += ": " + subtitle
|
||||
}
|
||||
return name, nil
|
||||
}
|
||||
|
||||
@ -424,33 +424,6 @@ var _ = Describe("explainConfig", func() {
|
||||
)
|
||||
})
|
||||
|
||||
var _ = Describe("discArtworkName", func() {
|
||||
var ds *tests.MockDataStore
|
||||
|
||||
BeforeEach(func() {
|
||||
albumRepo := tests.CreateMockAlbumRepo()
|
||||
albumRepo.SetData(model.Albums{{ID: "al-1", Name: "Sandinista!", Discs: model.Discs{2: "Side Three"}}})
|
||||
ds = &tests.MockDataStore{MockedAlbum: albumRepo}
|
||||
})
|
||||
|
||||
It("names the album, the disc and its subtitle", func() {
|
||||
name, err := artworkItemName(context.Background(), ds, model.KindDiscArtwork, "al-1:2")
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(name).To(Equal("Sandinista! (disc 2): Side Three"))
|
||||
})
|
||||
|
||||
It("omits the subtitle when the disc has none", func() {
|
||||
name, err := artworkItemName(context.Background(), ds, model.KindDiscArtwork, "al-1:1")
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(name).To(Equal("Sandinista! (disc 1)"))
|
||||
})
|
||||
|
||||
It("rejects an id that is not <albumID>:<disc>", func() {
|
||||
_, err := artworkItemName(context.Background(), ds, model.KindDiscArtwork, "al-1")
|
||||
Expect(err).To(HaveOccurred())
|
||||
})
|
||||
})
|
||||
|
||||
var _ = Describe("artwork refresh command", func() {
|
||||
It("requires at least one argument", func() {
|
||||
Expect(artworkRefreshCmd.Args(artworkRefreshCmd, []string{})).To(HaveOccurred())
|
||||
|
||||
@ -76,7 +76,10 @@ func CreateNativeAPIRouter(ctx context.Context) *nativeapi.Router {
|
||||
library := core.NewLibrary(dataStore, modelScanner, watcher, broker, manager)
|
||||
user := core.NewUser(dataStore, manager)
|
||||
maintenance := core.NewMaintenance(dataStore)
|
||||
router := nativeapi.New(dataStore, share, playlistsPlaylists, insights, library, user, maintenance, manager, uploader)
|
||||
agentsAgents := agents.GetAgents(dataStore, manager)
|
||||
matcherMatcher := matcher.New(dataStore)
|
||||
provider := external.NewProvider(dataStore, agentsAgents, matcherMatcher, broker)
|
||||
router := nativeapi.New(dataStore, share, playlistsPlaylists, insights, library, user, maintenance, manager, uploader, provider)
|
||||
return router
|
||||
}
|
||||
|
||||
@ -97,7 +100,7 @@ func CreateSubsonicAPIRouter(ctx context.Context) *subsonic.Router {
|
||||
manager := plugins.GetManager(dataStore, broker, metricsMetrics)
|
||||
agentsAgents := agents.GetAgents(dataStore, manager)
|
||||
matcherMatcher := matcher.New(dataStore)
|
||||
provider := external.NewProvider(dataStore, agentsAgents, matcherMatcher)
|
||||
provider := external.NewProvider(dataStore, agentsAgents, matcherMatcher, broker)
|
||||
uploader := artwork.NewUploader(dataStore)
|
||||
playlistsPlaylists := playlists.NewPlaylists(dataStore, uploader)
|
||||
modelScanner := scanner.New(ctx, dataStore, broker, playlistsPlaylists, metricsMetrics)
|
||||
@ -129,7 +132,7 @@ func CreateJellyfinAPIRouter(ctx context.Context) *jellyfin.Router {
|
||||
playlistsPlaylists := playlists.NewPlaylists(dataStore, uploader)
|
||||
agentsAgents := agents.GetAgents(dataStore, manager)
|
||||
matcherMatcher := matcher.New(dataStore)
|
||||
provider := external.NewProvider(dataStore, agentsAgents, matcherMatcher)
|
||||
provider := external.NewProvider(dataStore, agentsAgents, matcherMatcher, broker)
|
||||
sonicSonic := sonic.New(dataStore, manager, matcherMatcher)
|
||||
lyricsLyrics := lyrics.NewLyrics(dataStore, manager)
|
||||
router := jellyfin.New(dataStore, artworkArtwork, mediaStreamer, transcodeDecider, players, playTracker, playlistsPlaylists, provider, sonicSonic, lyricsLyrics, broker)
|
||||
|
||||
@ -201,7 +201,7 @@ var (
|
||||
}
|
||||
)
|
||||
|
||||
var HTTPUserAgent = "Navidrome" + "/" + Version
|
||||
var HTTPUserAgent = "Navidrome/" + Version + " - https://github.com/navidrome"
|
||||
|
||||
var (
|
||||
VariousArtists = "Various Artists"
|
||||
|
||||
@ -166,6 +166,63 @@ func enqueueMissingAll(ctx context.Context, ds model.DataStore) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// ItemName resolves a kind+id to the entity's display name, and errors when the item
|
||||
// does not exist. Callers use it to reject ids that would otherwise orphan a queue row.
|
||||
func ItemName(ctx context.Context, ds model.DataStore, kind model.Kind, id string) (string, error) {
|
||||
switch kind {
|
||||
case model.KindArtistArtwork:
|
||||
ar, err := ds.Artist(ctx).Get(id)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
return ar.Name, nil
|
||||
case model.KindAlbumArtwork:
|
||||
al, err := ds.Album(ctx).Get(id)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
return al.Name, nil
|
||||
case model.KindPlaylistArtwork:
|
||||
pls, err := ds.Playlist(ctx).Get(id)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
return pls.Name, nil
|
||||
case model.KindRadioArtwork:
|
||||
rd, err := ds.Radio(ctx).Get(id)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
return rd.Name, nil
|
||||
case model.KindMediaFileArtwork:
|
||||
mf, err := ds.MediaFile(ctx).Get(id)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
return mf.Title, nil
|
||||
case model.KindDiscArtwork:
|
||||
return discArtworkName(ctx, ds, id)
|
||||
}
|
||||
return "", fmt.Errorf("unsupported kind %q", kind.Prefix())
|
||||
}
|
||||
|
||||
func discArtworkName(ctx context.Context, ds model.DataStore, id string) (string, error) {
|
||||
albumID, discNumber, err := model.ParseDiscArtworkID(id)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
al, err := ds.Album(ctx).Get(albumID)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
name := fmt.Sprintf("%s (disc %d)", al.Name, discNumber)
|
||||
// The subtitle is itself a DiscArtPriority candidate, so name it where the chain can be read against it.
|
||||
if subtitle := strings.TrimSpace(al.Discs[discNumber]); subtitle != "" {
|
||||
name += ": " + subtitle
|
||||
}
|
||||
return name, nil
|
||||
}
|
||||
|
||||
// Refresh drops an item's resolved artwork state and re-queues it at Bump priority.
|
||||
func Refresh(ctx context.Context, ds model.DataStore, kind model.Kind, id string) error {
|
||||
if err := ds.Artwork(ctx).DeleteForItems(kind, []string{id}); err != nil {
|
||||
|
||||
@ -332,3 +332,56 @@ var _ = Describe("Housekeeping", func() {
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
var _ = Describe("ItemName", func() {
|
||||
var ds *tests.MockDataStore
|
||||
var ctx context.Context
|
||||
|
||||
BeforeEach(func() {
|
||||
ctx = context.Background()
|
||||
albumRepo := tests.CreateMockAlbumRepo()
|
||||
albumRepo.SetData(model.Albums{
|
||||
{ID: "al-1", Name: "Kid A"},
|
||||
{ID: "al-2", Name: "Sandinista!", Discs: model.Discs{2: "Side Three"}},
|
||||
})
|
||||
ds = &tests.MockDataStore{MockedAlbum: albumRepo}
|
||||
Expect(ds.Artist(ctx).(*tests.MockArtistRepo).Put(&model.Artist{ID: "ar-1", Name: "Radiohead"})).To(Succeed())
|
||||
})
|
||||
|
||||
It("returns the album name", func() {
|
||||
Expect(ItemName(ctx, ds, model.KindAlbumArtwork, "al-1")).To(Equal("Kid A"))
|
||||
})
|
||||
|
||||
It("returns the artist name", func() {
|
||||
Expect(ItemName(ctx, ds, model.KindArtistArtwork, "ar-1")).To(Equal("Radiohead"))
|
||||
})
|
||||
|
||||
It("errors for an unknown album", func() {
|
||||
_, err := ItemName(ctx, ds, model.KindAlbumArtwork, "nope")
|
||||
Expect(err).To(MatchError(model.ErrNotFound))
|
||||
})
|
||||
|
||||
It("errors for an unsupported kind", func() {
|
||||
// model.Kind is a struct with unexported fields, so the zero value is the only
|
||||
// unsupported Kind constructible from outside package model.
|
||||
_, err := ItemName(ctx, ds, model.Kind{}, "al-1")
|
||||
Expect(err).To(HaveOccurred())
|
||||
})
|
||||
|
||||
Context("disc artwork", func() {
|
||||
It("names the album, the disc and its subtitle", func() {
|
||||
Expect(ItemName(ctx, ds, model.KindDiscArtwork, "al-2:2")).
|
||||
To(Equal("Sandinista! (disc 2): Side Three"))
|
||||
})
|
||||
|
||||
It("omits the subtitle when the disc has none", func() {
|
||||
Expect(ItemName(ctx, ds, model.KindDiscArtwork, "al-2:1")).
|
||||
To(Equal("Sandinista! (disc 1)"))
|
||||
})
|
||||
|
||||
It("rejects an id that is not <albumID>:<disc>", func() {
|
||||
_, err := ItemName(ctx, ds, model.KindDiscArtwork, "al-2")
|
||||
Expect(err).To(HaveOccurred())
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@ -16,10 +16,10 @@ import (
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/consts"
|
||||
"github.com/navidrome/navidrome/core/ffmpeg"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/utils/httpclient"
|
||||
"go.senan.xyz/taglib"
|
||||
)
|
||||
|
||||
@ -163,9 +163,8 @@ type readCloser struct {
|
||||
}
|
||||
|
||||
func fromURL(ctx context.Context, imageUrl *url.URL) (io.ReadCloser, string, error) {
|
||||
hc := http.Client{Timeout: 5 * time.Second}
|
||||
hc := httpclient.New(5 * time.Second)
|
||||
req, _ := http.NewRequestWithContext(ctx, http.MethodGet, imageUrl.String(), nil)
|
||||
req.Header.Set("User-Agent", consts.HTTPUserAgent)
|
||||
resp, err := hc.Do(req) //nolint:gosec
|
||||
if err != nil {
|
||||
return nil, "", err
|
||||
|
||||
47
core/external/provider.go
vendored
47
core/external/provider.go
vendored
@ -4,6 +4,7 @@ import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"slices"
|
||||
"sort"
|
||||
"strings"
|
||||
"time"
|
||||
@ -14,6 +15,7 @@ import (
|
||||
"github.com/navidrome/navidrome/core/matcher"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/server/events"
|
||||
"github.com/navidrome/navidrome/utils"
|
||||
. "github.com/navidrome/navidrome/utils/gg"
|
||||
"github.com/navidrome/navidrome/utils/slice"
|
||||
@ -33,12 +35,14 @@ type Provider interface {
|
||||
UpdateArtistInfo(ctx context.Context, id string, count int, includeNotPresent bool) (*model.Artist, error)
|
||||
SimilarSongs(ctx context.Context, id string, count int) (model.MediaFiles, error)
|
||||
TopSongs(ctx context.Context, artist, artistId string, count int) (model.MediaFiles, error)
|
||||
RefreshInfo(ctx context.Context, kind model.Kind, id string) error
|
||||
}
|
||||
|
||||
type provider struct {
|
||||
ds model.DataStore
|
||||
ag Agents
|
||||
matcher *matcher.Matcher
|
||||
broker events.Broker
|
||||
artistQueue refreshQueue[auxArtist]
|
||||
albumQueue refreshQueue[auxAlbum]
|
||||
}
|
||||
@ -83,13 +87,17 @@ type Agents interface {
|
||||
agents.SimilarSongsByArtistRetriever
|
||||
}
|
||||
|
||||
func NewProvider(ds model.DataStore, agents Agents, m *matcher.Matcher) Provider {
|
||||
e := &provider{ds: ds, ag: agents, matcher: m}
|
||||
func NewProvider(ds model.DataStore, agents Agents, m *matcher.Matcher, broker events.Broker) Provider {
|
||||
e := &provider{ds: ds, ag: agents, matcher: m, broker: broker}
|
||||
e.artistQueue = newRefreshQueue(context.TODO(), e.populateArtistInfo)
|
||||
e.albumQueue = newRefreshQueue(context.TODO(), e.populateAlbumInfo)
|
||||
return e
|
||||
}
|
||||
|
||||
func (e *provider) broadcastRefresh(ctx context.Context, resource, id string) {
|
||||
e.broker.SendBroadcastMessage(ctx, (&events.RefreshResource{}).With(resource, id))
|
||||
}
|
||||
|
||||
func (e *provider) getAlbum(ctx context.Context, id string) (auxAlbum, error) {
|
||||
var entity any
|
||||
entity, err := model.GetEntityByID(ctx, e.ds, id)
|
||||
@ -179,6 +187,7 @@ func (e *provider) populateAlbumInfo(ctx context.Context, album auxAlbum) (auxAl
|
||||
"elapsed", time.Since(start), err)
|
||||
} else {
|
||||
log.Trace(ctx, "AlbumInfo collected", "album", album, "elapsed", time.Since(start))
|
||||
e.broadcastRefresh(ctx, "album", album.ID)
|
||||
}
|
||||
|
||||
return album, nil
|
||||
@ -272,10 +281,44 @@ func (e *provider) populateArtistInfo(ctx context.Context, artist auxArtist) (au
|
||||
"elapsed", time.Since(start), err)
|
||||
} else {
|
||||
log.Trace(ctx, "ArtistInfo collected", "artist", artist, "elapsed", time.Since(start))
|
||||
e.broadcastRefresh(ctx, "artist", artist.ID)
|
||||
}
|
||||
return artist, nil
|
||||
}
|
||||
|
||||
// infoKinds are the kinds RefreshInfo can act on. Callers check this instead of restating
|
||||
// the set, so the switch below stays the only place that has to know how each kind loads.
|
||||
var infoKinds = []model.Kind{model.KindArtistArtwork, model.KindAlbumArtwork}
|
||||
|
||||
// HasInfo reports whether a kind has external info to refresh.
|
||||
func HasInfo(kind model.Kind) bool { return slices.Contains(infoKinds, kind) }
|
||||
|
||||
// RefreshInfo re-fetches external info for one item, ignoring the TTL. It is synchronous:
|
||||
// callers that must not block are responsible for detaching it.
|
||||
func (e *provider) RefreshInfo(ctx context.Context, kind model.Kind, id string) error {
|
||||
ctx, cancel := context.WithTimeout(ctx, refreshTimeout)
|
||||
defer cancel()
|
||||
|
||||
switch kind {
|
||||
case model.KindArtistArtwork:
|
||||
artist, err := e.getArtist(ctx, id)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
_, err = e.populateArtistInfo(ctx, artist)
|
||||
return err
|
||||
case model.KindAlbumArtwork:
|
||||
album, err := e.getAlbum(ctx, id)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
_, err = e.populateAlbumInfo(ctx, album)
|
||||
return err
|
||||
default:
|
||||
return model.ErrNotFound
|
||||
}
|
||||
}
|
||||
|
||||
func (e *provider) TopSongs(ctx context.Context, artistName, id string, count int) (model.MediaFiles, error) {
|
||||
artist, err := e.findArtist(ctx, artistName, id)
|
||||
if err != nil {
|
||||
|
||||
156
core/external/provider_refreshinfo_test.go
vendored
Normal file
156
core/external/provider_refreshinfo_test.go
vendored
Normal file
@ -0,0 +1,156 @@
|
||||
package external_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"slices"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/core/agents"
|
||||
"github.com/navidrome/navidrome/core/external"
|
||||
"github.com/navidrome/navidrome/core/matcher"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/server/events"
|
||||
"github.com/navidrome/navidrome/tests"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
"github.com/stretchr/testify/mock"
|
||||
)
|
||||
|
||||
type fakeBroker struct {
|
||||
events.Broker
|
||||
mu sync.Mutex
|
||||
events []events.Event
|
||||
}
|
||||
|
||||
func (f *fakeBroker) SendBroadcastMessage(_ context.Context, e events.Event) {
|
||||
f.mu.Lock()
|
||||
defer f.mu.Unlock()
|
||||
f.events = append(f.events, e)
|
||||
}
|
||||
|
||||
func (f *fakeBroker) sent() []events.Event {
|
||||
f.mu.Lock()
|
||||
defer f.mu.Unlock()
|
||||
return slices.Clone(f.events)
|
||||
}
|
||||
|
||||
var _ = Describe("Provider - RefreshInfo", func() {
|
||||
var (
|
||||
ctx context.Context
|
||||
p external.Provider
|
||||
ds *tests.MockDataStore
|
||||
ag *mockAgents
|
||||
broker *fakeBroker
|
||||
mockArtistRepo *tests.MockArtistRepo
|
||||
mockAlbumRepo *tests.MockAlbumRepo
|
||||
)
|
||||
|
||||
expectArtistAgents := func() {
|
||||
ag.On("GetArtistMBID", mock.Anything, mock.Anything, mock.Anything).Return("mbid-1", nil)
|
||||
ag.On("GetArtistImages", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
|
||||
Return([]agents.ExternalImage{}, nil)
|
||||
ag.On("GetArtistBiography", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
|
||||
Return("Fresh Bio", nil)
|
||||
ag.On("GetArtistURL", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
|
||||
Return("http://artist.url", nil)
|
||||
ag.On("GetSimilarArtists", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
|
||||
Return([]agents.Artist{}, nil)
|
||||
}
|
||||
|
||||
expectAlbumAgents := func() {
|
||||
ag.On("GetAlbumInfo", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
|
||||
Return(&agents.AlbumInfo{URL: "http://album.url", Description: "Fresh Notes"}, nil)
|
||||
ag.On("GetAlbumImages", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
|
||||
Return([]agents.ExternalImage{}, nil)
|
||||
}
|
||||
|
||||
BeforeEach(func() {
|
||||
ctx = GinkgoT().Context()
|
||||
ds = new(tests.MockDataStore)
|
||||
ag = new(mockAgents)
|
||||
broker = &fakeBroker{}
|
||||
p = external.NewProvider(ds, ag, matcher.New(ds), broker)
|
||||
mockArtistRepo = ds.Artist(ctx).(*tests.MockArtistRepo)
|
||||
mockAlbumRepo = ds.Album(ctx).(*tests.MockAlbumRepo)
|
||||
})
|
||||
|
||||
It("repopulates an artist even when its info is fresh", func() {
|
||||
fresh := time.Now()
|
||||
mockArtistRepo.SetData(model.Artists{{
|
||||
ID: "ar-1", Name: "Test Artist", Biography: "stale", ExternalInfoUpdatedAt: &fresh,
|
||||
}})
|
||||
expectArtistAgents()
|
||||
|
||||
Expect(p.RefreshInfo(ctx, model.KindArtistArtwork, "ar-1")).To(Succeed())
|
||||
|
||||
saved, err := mockArtistRepo.Get("ar-1")
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(saved.Biography).To(Equal("Fresh Bio"))
|
||||
})
|
||||
|
||||
It("repopulates an album even when its info is fresh", func() {
|
||||
fresh := time.Now()
|
||||
mockAlbumRepo.SetData(model.Albums{{
|
||||
ID: "al-1", Name: "Test Album", AlbumArtist: "Test Artist",
|
||||
Description: "stale", ExternalInfoUpdatedAt: &fresh,
|
||||
}})
|
||||
expectAlbumAgents()
|
||||
|
||||
Expect(p.RefreshInfo(ctx, model.KindAlbumArtwork, "al-1")).To(Succeed())
|
||||
|
||||
saved, err := mockAlbumRepo.Get("al-1")
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(saved.Description).To(Equal("Fresh Notes"))
|
||||
})
|
||||
|
||||
It("returns ErrNotFound for an unknown id", func() {
|
||||
Expect(p.RefreshInfo(ctx, model.KindArtistArtwork, "nope")).To(MatchError(model.ErrNotFound))
|
||||
})
|
||||
|
||||
It("returns ErrNotFound for a kind with no external info", func() {
|
||||
Expect(p.RefreshInfo(ctx, model.KindPlaylistArtwork, "pl-1")).To(MatchError(model.ErrNotFound))
|
||||
})
|
||||
|
||||
It("broadcasts a RefreshResource naming the artist", func() {
|
||||
mockArtistRepo.SetData(model.Artists{{ID: "ar-1", Name: "Test Artist"}})
|
||||
expectArtistAgents()
|
||||
|
||||
Expect(p.RefreshInfo(ctx, model.KindArtistArtwork, "ar-1")).To(Succeed())
|
||||
|
||||
sent := broker.sent()
|
||||
Expect(sent).To(HaveLen(1))
|
||||
rr, ok := sent[0].(*events.RefreshResource)
|
||||
Expect(ok).To(BeTrue())
|
||||
Expect(rr.Data(rr)).To(ContainSubstring("ar-1"))
|
||||
Expect(rr.Data(rr)).To(ContainSubstring("artist"))
|
||||
})
|
||||
|
||||
It("broadcasts a RefreshResource naming the album", func() {
|
||||
mockAlbumRepo.SetData(model.Albums{{ID: "al-1", Name: "Test Album", AlbumArtist: "Test Artist"}})
|
||||
expectAlbumAgents()
|
||||
|
||||
Expect(p.RefreshInfo(ctx, model.KindAlbumArtwork, "al-1")).To(Succeed())
|
||||
|
||||
sent := broker.sent()
|
||||
Expect(sent).To(HaveLen(1))
|
||||
Expect(sent[0].Data(sent[0])).To(ContainSubstring("album"))
|
||||
Expect(sent[0].Data(sent[0])).To(ContainSubstring("al-1"))
|
||||
})
|
||||
|
||||
It("does not broadcast when the artist cannot be loaded", func() {
|
||||
mockArtistRepo.SetData(model.Artists{{ID: "ar-1", Name: "Test Artist"}})
|
||||
expectArtistAgents()
|
||||
mockArtistRepo.SetError(true)
|
||||
|
||||
_ = p.RefreshInfo(ctx, model.KindArtistArtwork, "ar-1")
|
||||
|
||||
Expect(broker.sent()).To(BeEmpty())
|
||||
})
|
||||
|
||||
It("reports which kinds have external info", func() {
|
||||
Expect(external.HasInfo(model.KindArtistArtwork)).To(BeTrue())
|
||||
Expect(external.HasInfo(model.KindAlbumArtwork)).To(BeTrue())
|
||||
Expect(external.HasInfo(model.KindPlaylistArtwork)).To(BeFalse())
|
||||
})
|
||||
})
|
||||
2
core/external/provider_similarsongs_test.go
vendored
2
core/external/provider_similarsongs_test.go
vendored
@ -61,7 +61,7 @@ var _ = Describe("Provider - SimilarSongs", func() {
|
||||
similarAgent: mockSimilarAgent,
|
||||
}
|
||||
|
||||
provider = NewProvider(ds, agentsCombined, matcher.New(ds))
|
||||
provider = NewProvider(ds, agentsCombined, matcher.New(ds), &fakeBroker{})
|
||||
})
|
||||
|
||||
// Resolves track-1 through the GetEntityByID probe order and on to its artist. Left permissive:
|
||||
|
||||
2
core/external/provider_topsongs_test.go
vendored
2
core/external/provider_topsongs_test.go
vendored
@ -45,7 +45,7 @@ var _ = Describe("Provider - TopSongs", func() {
|
||||
|
||||
ag = new(mockAgents)
|
||||
|
||||
p = NewProvider(ds, ag, matcher.New(ds))
|
||||
p = NewProvider(ds, ag, matcher.New(ds), &fakeBroker{})
|
||||
})
|
||||
|
||||
It("returns top songs for a known artist", func() {
|
||||
|
||||
@ -34,7 +34,7 @@ var _ = Describe("Provider - UpdateAlbumInfo", func() {
|
||||
ctx = GinkgoT().Context()
|
||||
ds = new(tests.MockDataStore)
|
||||
ag = new(mockAgents)
|
||||
p = external.NewProvider(ds, ag, matcher.New(ds))
|
||||
p = external.NewProvider(ds, ag, matcher.New(ds), &fakeBroker{})
|
||||
mockAlbumRepo = ds.Album(ctx).(*tests.MockAlbumRepo)
|
||||
conf.Server.DevAlbumInfoTimeToLive = 1 * time.Hour
|
||||
})
|
||||
|
||||
@ -37,7 +37,7 @@ var _ = Describe("Provider - UpdateArtistInfo", func() {
|
||||
ctx = GinkgoT().Context()
|
||||
ds = new(tests.MockDataStore)
|
||||
ag = new(mockAgents)
|
||||
p = external.NewProvider(ds, ag, matcher.New(ds))
|
||||
p = external.NewProvider(ds, ag, matcher.New(ds), &fakeBroker{})
|
||||
mockArtistRepo = ds.Artist(ctx).(*tests.MockArtistRepo)
|
||||
})
|
||||
|
||||
|
||||
@ -26,6 +26,7 @@ import (
|
||||
"github.com/navidrome/navidrome/model/request"
|
||||
"github.com/navidrome/navidrome/plugins"
|
||||
"github.com/navidrome/navidrome/server/events"
|
||||
"github.com/navidrome/navidrome/utils/httpclient"
|
||||
"github.com/navidrome/navidrome/utils/singleton"
|
||||
)
|
||||
|
||||
@ -95,9 +96,7 @@ func (c *insightsCollector) sendInsights(ctx context.Context) {
|
||||
log.Trace(ctx, "No users found, skipping Insights data collection")
|
||||
return
|
||||
}
|
||||
hc := &http.Client{
|
||||
Timeout: consts.DefaultHttpClientTimeOut,
|
||||
}
|
||||
hc := httpclient.New(consts.DefaultHttpClientTimeOut)
|
||||
data := c.collect(ctx)
|
||||
if data == nil {
|
||||
return
|
||||
|
||||
@ -152,8 +152,9 @@ func (s *Stream) EstimatedContentLength() int {
|
||||
|
||||
// Serve writes the stream to the HTTP response. For seekable streams it uses http.ServeContent
|
||||
// (supporting range requests). For non-seekable streams it writes directly and logs any errors.
|
||||
// Returns the number of bytes written and an error only when io.Copy fails with 0 bytes written
|
||||
// Returns the number of bytes written and an error only when it fails with 0 bytes written
|
||||
// (meaning the HTTP 200 status has not been flushed yet and the caller can still send an error response).
|
||||
// Once bytes are on the wire it panics with http.ErrAbortHandler instead, aborting the response.
|
||||
// Empty output (0 bytes, no error) is logged but not treated as an error.
|
||||
func (s *Stream) Serve(ctx context.Context, w http.ResponseWriter, r *http.Request) (int64, error) {
|
||||
if s.Seekable() {
|
||||
@ -183,7 +184,8 @@ func (s *Stream) Serve(ctx context.Context, w http.ResponseWriter, r *http.Reque
|
||||
w.Header().Del("Content-Length")
|
||||
return 0, fmt.Errorf("sending transcoded file: %w", err)
|
||||
}
|
||||
return c, nil
|
||||
// The 200 is already sent, so dropping the connection is the only way to say "truncated".
|
||||
panic(http.ErrAbortHandler)
|
||||
}
|
||||
if c == 0 {
|
||||
log.Error(ctx, "Transcoding returned empty output, ffmpeg may have failed. "+
|
||||
|
||||
@ -1,12 +1,18 @@
|
||||
package stream_test
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"errors"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"testing/iotest"
|
||||
"time"
|
||||
|
||||
"github.com/go-chi/chi/v5/middleware"
|
||||
|
||||
"github.com/navidrome/navidrome/conf"
|
||||
"github.com/navidrome/navidrome/conf/configtest"
|
||||
"github.com/navidrome/navidrome/core/stream"
|
||||
@ -140,4 +146,49 @@ var _ = Describe("MediaStreamer", func() {
|
||||
Expect(s.Seekable()).To(BeTrue())
|
||||
})
|
||||
})
|
||||
|
||||
Context("Serve", func() {
|
||||
var mf *model.MediaFile
|
||||
BeforeEach(func() {
|
||||
var err error
|
||||
mf, err = ds.MediaFile(ctx).Get("123")
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
})
|
||||
|
||||
It("keeps empty output a non-error, so callers still reply 200 with an empty body", func() {
|
||||
s := stream.NewStream(mf, "mp3", 128, io.NopCloser(bytes.NewReader(nil)))
|
||||
w := httptest.NewRecorder()
|
||||
r := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
|
||||
n, err := s.Serve(ctx, w, r)
|
||||
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(n).To(BeZero())
|
||||
Expect(w.Code).To(Equal(http.StatusOK))
|
||||
})
|
||||
|
||||
It("aborts the response when the source fails after sending data", func() {
|
||||
src := io.NopCloser(io.MultiReader(
|
||||
bytes.NewReader(bytes.Repeat([]byte("a"), 64*1024)),
|
||||
iotest.ErrReader(errors.New("transcoder died")),
|
||||
))
|
||||
server := httptest.NewServer(serveHandler(stream.NewStream(mf, "mp3", 128, src)))
|
||||
DeferCleanup(server.Close)
|
||||
|
||||
resp, err := http.Get(server.URL)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
defer resp.Body.Close()
|
||||
|
||||
// A client-side read failure is the only observable proof the response was aborted.
|
||||
_, err = io.ReadAll(resp.Body)
|
||||
Expect(err).To(HaveOccurred())
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
// Serve runs behind the real server's Recoverer, which must let ErrAbortHandler through.
|
||||
func serveHandler(s *stream.Stream) http.Handler {
|
||||
return middleware.Recoverer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
_, _ = s.Serve(r.Context(), w, r)
|
||||
}))
|
||||
}
|
||||
|
||||
3
go.mod
3
go.mod
@ -5,6 +5,9 @@ go 1.26
|
||||
// Fork to implement raw tags support
|
||||
replace go.senan.xyz/taglib => github.com/deluan/go-taglib v0.0.0-20260720134629-a133b9719ea3
|
||||
|
||||
// Fork to implement CloseWithError, proposed upstream in https://github.com/djherbis/fscache/pull/22
|
||||
replace github.com/djherbis/fscache => github.com/deluan/fscache v0.9.1-0.20260825221051-a07d597526e2
|
||||
|
||||
require (
|
||||
github.com/Masterminds/squirrel v1.5.4
|
||||
github.com/andybalholm/cascadia v1.3.4
|
||||
|
||||
4
go.sum
4
go.sum
@ -29,6 +29,8 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs
|
||||
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
|
||||
github.com/decred/dcrd/dcrec/secp256k1/v4 v4.4.1 h1:5RVFMOWjMyRy8cARdy79nAmgYw3hK/4HUq48LQ6Wwqo=
|
||||
github.com/decred/dcrd/dcrec/secp256k1/v4 v4.4.1/go.mod h1:ZXNYxsqcloTdSy/rNShjYzMhyjf0LaoftYK0p+A3h40=
|
||||
github.com/deluan/fscache v0.9.1-0.20260825221051-a07d597526e2 h1:s254V2hsrrCJXYtAn9WPG/5p4QHenfL9E+j6Tiq5MW4=
|
||||
github.com/deluan/fscache v0.9.1-0.20260825221051-a07d597526e2/go.mod h1:eNFa48vJrse+8ysT4IJnnUeXwLZNcR0JQumU/W/QoUI=
|
||||
github.com/deluan/go-taglib v0.0.0-20260720134629-a133b9719ea3 h1:j7eSXqgtjhlNfwnMEzRdXnJGZTEw4I7J9TeQAll83bU=
|
||||
github.com/deluan/go-taglib v0.0.0-20260720134629-a133b9719ea3/go.mod h1:QGxQ4Z1IWyY9w56xNEFjYAaWE8uSxA/gneQ7RPcFJrY=
|
||||
github.com/deluan/rest v0.0.0-20211102003136-6260bc399cbf h1:tb246l2Zmpt/GpF9EcHCKTtwzrd0HGfEmoODFA/qnk4=
|
||||
@ -39,8 +41,6 @@ github.com/dexterlb/mpvipc v0.0.0-20260722094525-0cf47d745b36 h1:KtPfdSST6e0vJbM
|
||||
github.com/dexterlb/mpvipc v0.0.0-20260722094525-0cf47d745b36/go.mod h1:RkQWLNITKkXHLP7LXxZSgEq+uFWU25M5qW7qfEhL9Wc=
|
||||
github.com/djherbis/atime v1.1.0 h1:rgwVbP/5by8BvvjBNrbh64Qz33idKT3pSnMSJsxhi0g=
|
||||
github.com/djherbis/atime v1.1.0/go.mod h1:28OF6Y8s3NQWwacXc5eZTsEsiMzp7LF8MbXE+XJPdBE=
|
||||
github.com/djherbis/fscache v0.10.2-0.20231127215153-442a07e326c4 h1:wdZllsLrDJtYfHiAKogB4PNHSDeO+v+5S3eqSWHGDlc=
|
||||
github.com/djherbis/fscache v0.10.2-0.20231127215153-442a07e326c4/go.mod h1:dHWjlanKIxaHVH1xJOTb4kzP800XdcXlgJ6JYlR2DPU=
|
||||
github.com/djherbis/stream v1.4.0 h1:aVD46WZUiq5kJk55yxJAyw6Kuera6kmC3i2vEQyW/AE=
|
||||
github.com/djherbis/stream v1.4.0/go.mod h1:cqjC1ZRq3FFwkGmUtHwcldbnW8f0Q4YuVsGW1eAFtOk=
|
||||
github.com/djherbis/times v1.6.0 h1:w2ctJ92J8fBvWPxugmXIv7Nz7Q3iDMKNx9v5ocVH20c=
|
||||
|
||||
@ -218,11 +218,11 @@ var _ = Describe("MediaFiles", func() {
|
||||
{Tags: Tags{"genre": []string{"Alternative", "Rock"}}},
|
||||
}
|
||||
})
|
||||
It("sets the correct Genre, sorted by frequency, then alphabetically", func() {
|
||||
It("sets the correct Genre, sorted by frequency, then by order of appearance", func() {
|
||||
album := mfs.ToAlbum()
|
||||
Expect(album.Tags).To(HaveLen(2))
|
||||
Expect(album.Tags).To(HaveKeyWithValue(TagGenre, []string{"Rock", "Alternative", "Punk"}))
|
||||
Expect(album.Tags).To(HaveKeyWithValue(TagMood, []string{"Chill", "Happy"}))
|
||||
Expect(album.Tags).To(HaveKeyWithValue(TagGenre, []string{"Rock", "Punk", "Alternative"}))
|
||||
Expect(album.Tags).To(HaveKeyWithValue(TagMood, []string{"Happy", "Chill"}))
|
||||
})
|
||||
})
|
||||
When("we have tags with mismatching case", func() {
|
||||
|
||||
@ -24,13 +24,17 @@ type TagList []Tag
|
||||
func (l TagList) GroupByFrequency() Tags {
|
||||
grouped := map[string]map[string]int{}
|
||||
values := map[string]string{}
|
||||
for _, t := range l {
|
||||
firstSeen := map[string]int{}
|
||||
for i, t := range l {
|
||||
if m, ok := grouped[string(t.TagName)]; !ok {
|
||||
grouped[string(t.TagName)] = map[string]int{t.ID: 1}
|
||||
} else {
|
||||
m[t.ID]++
|
||||
}
|
||||
values[t.ID] = t.TagValue
|
||||
if _, ok := firstSeen[t.ID]; !ok {
|
||||
firstSeen[t.ID] = i
|
||||
}
|
||||
}
|
||||
|
||||
tags := Tags{}
|
||||
@ -42,7 +46,7 @@ func (l TagList) GroupByFrequency() Tags {
|
||||
slices.SortFunc(idList, func(a, b string) int {
|
||||
return cmp.Or(
|
||||
cmp.Compare(counts[b], counts[a]),
|
||||
cmp.Compare(values[a], values[b]),
|
||||
cmp.Compare(firstSeen[a], firstSeen[b]),
|
||||
)
|
||||
})
|
||||
tags[TagName(name)] = slice.Map(idList, func(id string) string { return values[id] })
|
||||
|
||||
@ -93,7 +93,7 @@ var _ = Describe("Tag", func() {
|
||||
Expect(groupedTags).To(HaveKeyWithValue(TagName("artist"), []string{"The Beatles", "The Rolling Stones"}))
|
||||
})
|
||||
|
||||
It("should sort tags by name when frequency is the same", func() {
|
||||
It("should keep the order the values appeared in when frequency is the same", func() {
|
||||
tagList := TagList{
|
||||
NewTag("genre", "Jazz"),
|
||||
NewTag("genre", "Rock"),
|
||||
@ -103,7 +103,7 @@ var _ = Describe("Tag", func() {
|
||||
|
||||
groupedTags := tagList.GroupByFrequency()
|
||||
|
||||
Expect(groupedTags).To(HaveKeyWithValue(TagName("genre"), []string{"Alternative", "Jazz", "Pop", "Rock"}))
|
||||
Expect(groupedTags).To(HaveKeyWithValue(TagName("genre"), []string{"Jazz", "Rock", "Alternative", "Pop"}))
|
||||
})
|
||||
It("should normalize casing", func() {
|
||||
tagList := TagList{
|
||||
|
||||
@ -14,6 +14,7 @@ import (
|
||||
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/plugins/host"
|
||||
"github.com/navidrome/navidrome/utils/httpclient"
|
||||
)
|
||||
|
||||
const (
|
||||
@ -45,24 +46,21 @@ func newHTTPService(pluginName string, permission *HTTPPermission) *httpServiceI
|
||||
pluginName: pluginName,
|
||||
requiredHosts: requiredHosts,
|
||||
}
|
||||
svc.client = &http.Client{
|
||||
Transport: http.DefaultTransport,
|
||||
// Timeout is set per-request via context deadline, not here.
|
||||
// CheckRedirect validates hosts and enforces redirect limits.
|
||||
CheckRedirect: func(req *http.Request, via []*http.Request) error {
|
||||
if req.Context().Value(noFollowRedirectsKey) != nil {
|
||||
return http.ErrUseLastResponse
|
||||
}
|
||||
if len(via) >= httpClientMaxRedirects {
|
||||
log.Warn(req.Context(), "HTTP redirect limit exceeded", "plugin", svc.pluginName, "url", req.URL.String(), "redirectCount", len(via))
|
||||
return http.ErrUseLastResponse
|
||||
}
|
||||
if err := svc.validateHost(req.Context(), req.URL.Host); err != nil {
|
||||
log.Warn(req.Context(), "HTTP redirect blocked", "plugin", svc.pluginName, "url", req.URL.String(), "err", err)
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
},
|
||||
// No client timeout: it is set per-request via context deadline.
|
||||
svc.client = httpclient.New(0)
|
||||
svc.client.CheckRedirect = func(req *http.Request, via []*http.Request) error {
|
||||
if req.Context().Value(noFollowRedirectsKey) != nil {
|
||||
return http.ErrUseLastResponse
|
||||
}
|
||||
if len(via) >= httpClientMaxRedirects {
|
||||
log.Warn(req.Context(), "HTTP redirect limit exceeded", "plugin", svc.pluginName, "url", req.URL.String(), "redirectCount", len(via))
|
||||
return http.ErrUseLastResponse
|
||||
}
|
||||
if err := svc.validateHost(req.Context(), req.URL.Host); err != nil {
|
||||
log.Warn(req.Context(), "HTTP redirect blocked", "plugin", svc.pluginName, "url", req.URL.String(), "err", err)
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
}
|
||||
return svc
|
||||
}
|
||||
|
||||
@ -93,6 +93,7 @@
|
||||
"addToPlaylist": "Adicionar à playlist",
|
||||
"download": "Baixar",
|
||||
"info": "Detalhes",
|
||||
"refresh": "Atualizar Metadados",
|
||||
"share": "Compartilhar"
|
||||
},
|
||||
"lists": {
|
||||
@ -602,7 +603,8 @@
|
||||
"coverUploaded": "Capa atualizada",
|
||||
"coverRemoved": "Capa removida",
|
||||
"coverUploadError": "Erro ao enviar capa",
|
||||
"coverRemoveError": "Erro ao remover capa"
|
||||
"coverRemoveError": "Erro ao remover capa",
|
||||
"metadataRefreshStarted": "Atualizando metadados em segundo plano"
|
||||
},
|
||||
"menu": {
|
||||
"library": "Biblioteca",
|
||||
|
||||
@ -13,6 +13,7 @@ import (
|
||||
"github.com/navidrome/navidrome/consts"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/utils/cache"
|
||||
"github.com/navidrome/navidrome/utils/httpclient"
|
||||
"github.com/navidrome/navidrome/utils/random"
|
||||
"gopkg.in/yaml.v3"
|
||||
)
|
||||
@ -35,7 +36,7 @@ type Handler struct {
|
||||
|
||||
func NewHandler() *Handler {
|
||||
h := &Handler{}
|
||||
h.httpClient = cache.NewHTTPClient(&http.Client{Timeout: 5 * time.Second}, imageListTTL)
|
||||
h.httpClient = cache.NewHTTPClient(httpclient.New(5*time.Second), imageListTTL)
|
||||
h.cache = cache.NewFileCache(imageCacheDir, imageCacheSize, imageCacheDir, imageCacheMaxItems, h.serveImage)
|
||||
go func() {
|
||||
_, _ = h.getImageList(log.NewContext(context.Background()))
|
||||
@ -78,7 +79,7 @@ func (h *Handler) serveImage(ctx context.Context, item cache.Item) (io.Reader, e
|
||||
if image == "" {
|
||||
return nil, errors.New("empty image name")
|
||||
}
|
||||
c := http.Client{Timeout: imageRequestTimeout}
|
||||
c := httpclient.New(imageRequestTimeout)
|
||||
req, _ := http.NewRequestWithContext(ctx, http.MethodGet, imageURL(image), nil)
|
||||
resp, err := c.Do(req) //nolint:bodyclose,gosec // No need to close resp.Body, it will be closed via the CachedStream wrapper
|
||||
if errors.Is(err, context.DeadlineExceeded) {
|
||||
|
||||
@ -1,34 +0,0 @@
|
||||
package nativeapi
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"slices"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
"github.com/navidrome/navidrome/core/artwork"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
)
|
||||
|
||||
func (api *Router) addArtworkRoute(r chi.Router) {
|
||||
r.Post("/artwork/{kind}/{id}/refresh", api.refreshArtwork())
|
||||
}
|
||||
|
||||
// State is deliberately cleared so a wrong pick disappears immediately (placeholder until re-resolved).
|
||||
func (api *Router) refreshArtwork() http.HandlerFunc {
|
||||
return func(w http.ResponseWriter, r *http.Request) {
|
||||
ctx := r.Context()
|
||||
kind, _ := model.ParseKind(chi.URLParam(r, "kind"))
|
||||
id := chi.URLParam(r, "id")
|
||||
if !slices.Contains(artwork.RefreshableKinds, kind) {
|
||||
http.Error(w, "invalid artwork kind", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
if err := artwork.Refresh(ctx, api.ds, kind, id); err != nil {
|
||||
log.Error(ctx, "Error refreshing artwork", "kind", kind, "id", id, err)
|
||||
http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}
|
||||
}
|
||||
@ -1,95 +0,0 @@
|
||||
package nativeapi
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
|
||||
"github.com/navidrome/navidrome/conf"
|
||||
"github.com/navidrome/navidrome/conf/configtest"
|
||||
"github.com/navidrome/navidrome/core/auth"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/server"
|
||||
"github.com/navidrome/navidrome/tests"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
)
|
||||
|
||||
var _ = Describe("Artwork API", func() {
|
||||
var ds *tests.MockDataStore
|
||||
var artRepo *tests.MockArtworkRepo
|
||||
var queueRepo *tests.MockArtworkQueueRepo
|
||||
var router http.Handler
|
||||
var adminToken, userToken string
|
||||
|
||||
BeforeEach(func() {
|
||||
DeferCleanup(configtest.SetupConfig())
|
||||
conf.Server.EnableSharing = false
|
||||
artRepo = tests.CreateMockArtworkRepo()
|
||||
queueRepo = tests.CreateMockArtworkQueueRepo()
|
||||
ds = &tests.MockDataStore{MockedArtwork: artRepo, MockedArtworkQueue: queueRepo}
|
||||
auth.Init(ds)
|
||||
nativeRouter := New(ds, nil, nil, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, nil, nil)
|
||||
router = server.JWTVerifier(nativeRouter)
|
||||
|
||||
adminUser := model.User{ID: "admin-1", UserName: "admin", IsAdmin: true, NewPassword: "adminpass"}
|
||||
regularUser := model.User{ID: "user-1", UserName: "regular", IsAdmin: false, NewPassword: "userpass"}
|
||||
Expect(ds.User(context.TODO()).Put(&adminUser)).To(Succeed())
|
||||
Expect(ds.User(context.TODO()).Put(®ularUser)).To(Succeed())
|
||||
|
||||
var err error
|
||||
adminToken, err = auth.CreateToken(&adminUser)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
userToken, err = auth.CreateToken(®ularUser)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
})
|
||||
|
||||
Describe("POST /api/artwork/{kind}/{id}/refresh", func() {
|
||||
It("clears state and enqueues a Bump for admins", func() {
|
||||
Expect(artRepo.PutItemArtwork(&model.ItemArtwork{
|
||||
ItemKind: "al", ItemID: "al-1", Hash: "oldhash", Source: "external",
|
||||
})).To(Succeed())
|
||||
|
||||
req := createAuthenticatedRequest("POST", "/artwork/al/al-1/refresh", nil, adminToken)
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
Expect(w.Code).To(Equal(http.StatusNoContent))
|
||||
|
||||
_, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al-1", model.ImageTypePrimary)
|
||||
Expect(err).To(MatchError(model.ErrNotFound))
|
||||
|
||||
queued, err := queueRepo.DequeueBatch(1000)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(queued).To(ContainElement(SatisfyAll(
|
||||
HaveField("ItemKind", "al"),
|
||||
HaveField("ItemID", "al-1"),
|
||||
HaveField("Priority", model.ArtworkPriorityBump),
|
||||
)))
|
||||
})
|
||||
|
||||
It("returns 400 for an invalid kind", func() {
|
||||
req := createAuthenticatedRequest("POST", "/artwork/xx/id-1/refresh", nil, adminToken)
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
Expect(w.Code).To(Equal(http.StatusBadRequest))
|
||||
})
|
||||
|
||||
It("denies access to regular users", func() {
|
||||
req := createAuthenticatedRequest("POST", "/artwork/al/al-1/refresh", nil, userToken)
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
Expect(w.Code).To(Equal(http.StatusForbidden))
|
||||
})
|
||||
|
||||
It("denies access without authentication", func() {
|
||||
req := createUnauthenticatedRequest("POST", "/artwork/al/al-1/refresh", nil)
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
Expect(w.Code).To(Equal(http.StatusUnauthorized))
|
||||
})
|
||||
})
|
||||
})
|
||||
@ -29,7 +29,7 @@ var _ = Describe("Config API", func() {
|
||||
conf.Server.DevUIShowConfig = true // Enable config endpoint for tests
|
||||
ds = &tests.MockDataStore{}
|
||||
auth.Init(ds)
|
||||
nativeRouter := New(ds, nil, nil, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, nil, nil)
|
||||
nativeRouter := New(ds, nil, nil, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, nil, nil, nil)
|
||||
router = server.JWTVerifier(nativeRouter)
|
||||
|
||||
// Create test users
|
||||
|
||||
@ -31,7 +31,7 @@ var _ = Describe("Library API", func() {
|
||||
conf.Server.EnableSharing = false
|
||||
ds = &tests.MockDataStore{}
|
||||
auth.Init(ds)
|
||||
nativeRouter := New(ds, nil, nil, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, nil, nil)
|
||||
nativeRouter := New(ds, nil, nil, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, nil, nil, nil)
|
||||
router = server.JWTVerifier(nativeRouter)
|
||||
|
||||
// Create test users
|
||||
|
||||
56
server/nativeapi/metadata.go
Normal file
56
server/nativeapi/metadata.go
Normal file
@ -0,0 +1,56 @@
|
||||
package nativeapi
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"net/http"
|
||||
"slices"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
"github.com/navidrome/navidrome/core/artwork"
|
||||
"github.com/navidrome/navidrome/core/external"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
)
|
||||
|
||||
func (api *Router) addMetadataRoute(r chi.Router) {
|
||||
r.Post("/metadata/{kind}/{id}/refresh", api.refreshMetadata())
|
||||
}
|
||||
|
||||
// refreshMetadata clears the artwork state deliberately, so a wrong pick cannot be served from
|
||||
// cache again; reads fall back to local resolution while the worker re-runs the chain at Bump.
|
||||
func (api *Router) refreshMetadata() http.HandlerFunc {
|
||||
return func(w http.ResponseWriter, r *http.Request) {
|
||||
ctx := r.Context()
|
||||
kind, _ := model.ParseKind(chi.URLParam(r, "kind"))
|
||||
id := chi.URLParam(r, "id")
|
||||
if !slices.Contains(artwork.RefreshableKinds, kind) {
|
||||
http.Error(w, "invalid artwork kind", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
if _, err := artwork.ItemName(ctx, api.ds, kind, id); err != nil {
|
||||
if errors.Is(err, model.ErrNotFound) {
|
||||
http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound)
|
||||
return
|
||||
}
|
||||
log.Error(ctx, "Error looking up item to refresh", "kind", kind, "id", id, err)
|
||||
http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
if err := artwork.Refresh(ctx, api.ds, kind, id); err != nil {
|
||||
log.Error(ctx, "Error refreshing artwork", "kind", kind, "id", id, err)
|
||||
http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
if external.HasInfo(kind) {
|
||||
// Detached: the request context is cancelled the moment this handler returns 204.
|
||||
bg := context.WithoutCancel(ctx)
|
||||
go func() {
|
||||
if err := api.provider.RefreshInfo(bg, kind, id); err != nil {
|
||||
log.Error(bg, "Error refreshing external info", "kind", kind, "id", id, err)
|
||||
}
|
||||
}()
|
||||
}
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}
|
||||
}
|
||||
177
server/nativeapi/metadata_test.go
Normal file
177
server/nativeapi/metadata_test.go
Normal file
@ -0,0 +1,177 @@
|
||||
package nativeapi
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"slices"
|
||||
"sync"
|
||||
|
||||
"github.com/navidrome/navidrome/conf"
|
||||
"github.com/navidrome/navidrome/conf/configtest"
|
||||
"github.com/navidrome/navidrome/core/auth"
|
||||
"github.com/navidrome/navidrome/core/external"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/server"
|
||||
"github.com/navidrome/navidrome/tests"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
)
|
||||
|
||||
type fakeProvider struct {
|
||||
external.Provider
|
||||
mu sync.Mutex
|
||||
called []string
|
||||
}
|
||||
|
||||
func (f *fakeProvider) RefreshInfo(_ context.Context, kind model.Kind, id string) error {
|
||||
f.mu.Lock()
|
||||
defer f.mu.Unlock()
|
||||
f.called = append(f.called, kind.Prefix()+"/"+id)
|
||||
return nil
|
||||
}
|
||||
|
||||
func (f *fakeProvider) calls() []string {
|
||||
f.mu.Lock()
|
||||
defer f.mu.Unlock()
|
||||
return slices.Clone(f.called)
|
||||
}
|
||||
|
||||
var _ = Describe("Metadata API", func() {
|
||||
var ds *tests.MockDataStore
|
||||
var artRepo *tests.MockArtworkRepo
|
||||
var queueRepo *tests.MockArtworkQueueRepo
|
||||
var albumRepo *tests.MockAlbumRepo
|
||||
var provider *fakeProvider
|
||||
var router http.Handler
|
||||
var adminToken, userToken string
|
||||
|
||||
BeforeEach(func() {
|
||||
DeferCleanup(configtest.SetupConfig())
|
||||
conf.Server.EnableSharing = false
|
||||
artRepo = tests.CreateMockArtworkRepo()
|
||||
queueRepo = tests.CreateMockArtworkQueueRepo()
|
||||
albumRepo = tests.CreateMockAlbumRepo()
|
||||
artistRepo := tests.CreateMockArtistRepo()
|
||||
playlistRepo := tests.CreateMockPlaylistRepo()
|
||||
Expect(albumRepo.Put(&model.Album{ID: "al-1", Name: "Kid A"})).To(Succeed())
|
||||
Expect(artistRepo.Put(&model.Artist{ID: "ar-1", Name: "Radiohead"})).To(Succeed())
|
||||
Expect(playlistRepo.Put(&model.Playlist{ID: "pl-1", Name: "My Playlist"})).To(Succeed())
|
||||
ds = &tests.MockDataStore{
|
||||
MockedArtwork: artRepo,
|
||||
MockedArtworkQueue: queueRepo,
|
||||
MockedAlbum: albumRepo,
|
||||
MockedArtist: artistRepo,
|
||||
MockedPlaylist: playlistRepo,
|
||||
}
|
||||
auth.Init(ds)
|
||||
provider = &fakeProvider{}
|
||||
nativeRouter := New(ds, nil, nil, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, nil, nil, provider)
|
||||
router = server.JWTVerifier(nativeRouter)
|
||||
|
||||
adminUser := model.User{ID: "admin-1", UserName: "admin", IsAdmin: true, NewPassword: "adminpass"}
|
||||
regularUser := model.User{ID: "user-1", UserName: "regular", IsAdmin: false, NewPassword: "userpass"}
|
||||
Expect(ds.User(context.TODO()).Put(&adminUser)).To(Succeed())
|
||||
Expect(ds.User(context.TODO()).Put(®ularUser)).To(Succeed())
|
||||
|
||||
var err error
|
||||
adminToken, err = auth.CreateToken(&adminUser)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
userToken, err = auth.CreateToken(®ularUser)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
})
|
||||
|
||||
Describe("POST /api/metadata/{kind}/{id}/refresh", func() {
|
||||
It("clears state and enqueues a Bump for admins", func() {
|
||||
Expect(artRepo.PutItemArtwork(&model.ItemArtwork{
|
||||
ItemKind: "al", ItemID: "al-1", Hash: "oldhash", Source: "external",
|
||||
})).To(Succeed())
|
||||
|
||||
req := createAuthenticatedRequest("POST", "/metadata/al/al-1/refresh", nil, adminToken)
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
Expect(w.Code).To(Equal(http.StatusNoContent))
|
||||
|
||||
_, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al-1", model.ImageTypePrimary)
|
||||
Expect(err).To(MatchError(model.ErrNotFound))
|
||||
|
||||
queued, err := queueRepo.DequeueBatch(1000)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(queued).To(ContainElement(SatisfyAll(
|
||||
HaveField("ItemKind", "al"),
|
||||
HaveField("ItemID", "al-1"),
|
||||
HaveField("Priority", model.ArtworkPriorityBump),
|
||||
)))
|
||||
})
|
||||
|
||||
It("returns 400 for an invalid kind", func() {
|
||||
req := createAuthenticatedRequest("POST", "/metadata/xx/id-1/refresh", nil, adminToken)
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
Expect(w.Code).To(Equal(http.StatusBadRequest))
|
||||
})
|
||||
|
||||
It("denies access to regular users", func() {
|
||||
req := createAuthenticatedRequest("POST", "/metadata/al/al-1/refresh", nil, userToken)
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
Expect(w.Code).To(Equal(http.StatusForbidden))
|
||||
})
|
||||
|
||||
It("denies access without authentication", func() {
|
||||
req := createUnauthenticatedRequest("POST", "/metadata/al/al-1/refresh", nil)
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
Expect(w.Code).To(Equal(http.StatusUnauthorized))
|
||||
})
|
||||
|
||||
It("triggers an external info refresh for albums", func() {
|
||||
req := createAuthenticatedRequest("POST", "/metadata/al/al-1/refresh", nil, adminToken)
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
Expect(w.Code).To(Equal(http.StatusNoContent))
|
||||
Eventually(provider.calls).Should(ContainElement("al/al-1"))
|
||||
})
|
||||
|
||||
It("triggers an external info refresh for artists", func() {
|
||||
req := createAuthenticatedRequest("POST", "/metadata/ar/ar-1/refresh", nil, adminToken)
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
Expect(w.Code).To(Equal(http.StatusNoContent))
|
||||
Eventually(provider.calls).Should(ContainElement("ar/ar-1"))
|
||||
})
|
||||
|
||||
It("skips the external info refresh for kinds without external info", func() {
|
||||
req := createAuthenticatedRequest("POST", "/metadata/pl/pl-1/refresh", nil, adminToken)
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
Expect(w.Code).To(Equal(http.StatusNoContent))
|
||||
Consistently(provider.calls).ShouldNot(ContainElement("pl/pl-1"))
|
||||
})
|
||||
|
||||
It("returns 404 for an unknown id", func() {
|
||||
req := createAuthenticatedRequest("POST", "/metadata/al/nope/refresh", nil, adminToken)
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
Expect(w.Code).To(Equal(http.StatusNotFound))
|
||||
})
|
||||
|
||||
It("returns 500 when the lookup fails for a reason other than not-found", func() {
|
||||
albumRepo.SetError(true)
|
||||
|
||||
req := createAuthenticatedRequest("POST", "/metadata/al/al-1/refresh", nil, adminToken)
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, req)
|
||||
|
||||
Expect(w.Code).To(Equal(http.StatusInternalServerError))
|
||||
})
|
||||
})
|
||||
})
|
||||
@ -14,6 +14,7 @@ import (
|
||||
"github.com/navidrome/navidrome/conf"
|
||||
"github.com/navidrome/navidrome/core"
|
||||
"github.com/navidrome/navidrome/core/artwork"
|
||||
"github.com/navidrome/navidrome/core/external"
|
||||
"github.com/navidrome/navidrome/core/metrics"
|
||||
playlistsvc "github.com/navidrome/navidrome/core/playlists"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
@ -46,10 +47,11 @@ type Router struct {
|
||||
maintenance core.Maintenance
|
||||
pluginManager PluginManager
|
||||
imgUpload artwork.Uploader
|
||||
provider external.Provider
|
||||
}
|
||||
|
||||
func New(ds model.DataStore, share core.Share, playlists playlistsvc.Playlists, insights metrics.Insights, libraryService core.Library, userService core.User, maintenance core.Maintenance, pluginManager PluginManager, imgUpload artwork.Uploader) *Router {
|
||||
r := &Router{ds: ds, share: share, playlists: playlists, insights: insights, libs: libraryService, users: userService, maintenance: maintenance, pluginManager: pluginManager, imgUpload: imgUpload}
|
||||
func New(ds model.DataStore, share core.Share, playlists playlistsvc.Playlists, insights metrics.Insights, libraryService core.Library, userService core.User, maintenance core.Maintenance, pluginManager PluginManager, imgUpload artwork.Uploader, provider external.Provider) *Router {
|
||||
r := &Router{ds: ds, share: share, playlists: playlists, insights: insights, libs: libraryService, users: userService, maintenance: maintenance, pluginManager: pluginManager, imgUpload: imgUpload, provider: provider}
|
||||
r.Handler = r.routes()
|
||||
return r
|
||||
}
|
||||
@ -92,7 +94,7 @@ func (api *Router) routes() http.Handler {
|
||||
api.addConfigRoute(r)
|
||||
api.addUserLibraryRoute(r)
|
||||
api.addPluginRoute(r)
|
||||
api.addArtworkRoute(r)
|
||||
api.addMetadataRoute(r)
|
||||
api.RX(r, "/library", api.libs.NewRepository, true)
|
||||
})
|
||||
})
|
||||
|
||||
@ -95,7 +95,7 @@ var _ = Describe("Song Endpoints", func() {
|
||||
mfRepo.SetData(testSongs)
|
||||
|
||||
// Create the native API router and wrap it with the JWTVerifier middleware
|
||||
nativeRouter := New(ds, nil, nil, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, nil, nil)
|
||||
nativeRouter := New(ds, nil, nil, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, nil, nil, nil)
|
||||
router = server.JWTVerifier(nativeRouter)
|
||||
w = httptest.NewRecorder()
|
||||
})
|
||||
|
||||
@ -99,7 +99,7 @@ var _ = Describe("Playlist Tracks Endpoint", func() {
|
||||
err := userRepo.Put(&testUser)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
nativeRouter := New(ds, nil, plsSvc, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, nil, nil)
|
||||
nativeRouter := New(ds, nil, plsSvc, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, nil, nil, nil)
|
||||
router = server.JWTVerifier(nativeRouter)
|
||||
w = httptest.NewRecorder()
|
||||
})
|
||||
|
||||
@ -34,7 +34,7 @@ var _ = Describe("Plugin API", func() {
|
||||
ds = &tests.MockDataStore{}
|
||||
mockManager = &tests.MockPluginManager{}
|
||||
auth.Init(ds)
|
||||
nativeRouter := New(ds, nil, nil, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, mockManager, nil)
|
||||
nativeRouter := New(ds, nil, nil, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, mockManager, nil, nil)
|
||||
router = server.JWTVerifier(nativeRouter)
|
||||
|
||||
// Create test users
|
||||
|
||||
@ -45,7 +45,7 @@ var _ = Describe("PUT /user/{id}: token refresh on self password change", func()
|
||||
auth.Init(ds)
|
||||
|
||||
userService := core.NewUser(ds, noopPluginUnloader{})
|
||||
nativeRouter := New(ds, nil, nil, nil, tests.NewMockLibraryService(), userService, nil, nil, nil)
|
||||
nativeRouter := New(ds, nil, nil, nil, tests.NewMockLibraryService(), userService, nil, nil, nil, nil)
|
||||
router = server.JWTVerifier(nativeRouter)
|
||||
})
|
||||
|
||||
|
||||
@ -357,6 +357,10 @@ func (n noopProvider) TopSongs(context.Context, string, string, int) (model.Medi
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
func (n noopProvider) RefreshInfo(context.Context, model.Kind, string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// Compile-time interface checks
|
||||
var (
|
||||
_ artwork.Artwork = noopArtwork{}
|
||||
|
||||
@ -65,6 +65,9 @@ func (m *MockAlbumRepo) Put(al *model.Album) error {
|
||||
if al.ID == "" {
|
||||
al.ID = id.NewRandom()
|
||||
}
|
||||
if m.Data == nil {
|
||||
m.Data = make(map[string]*model.Album)
|
||||
}
|
||||
m.Data[al.ID] = al
|
||||
return nil
|
||||
}
|
||||
@ -142,10 +145,7 @@ func (m *MockAlbumRepo) GetTouchedAlbums(libID int) (model.AlbumCursor, error) {
|
||||
}
|
||||
|
||||
func (m *MockAlbumRepo) UpdateExternalInfo(album *model.Album) error {
|
||||
if m.Err {
|
||||
return errors.New("unexpected error")
|
||||
}
|
||||
return nil
|
||||
return m.Put(album)
|
||||
}
|
||||
|
||||
func (m *MockAlbumRepo) Search(q string, options ...model.QueryOptions) (model.Albums, error) {
|
||||
|
||||
@ -58,6 +58,9 @@ func (m *MockArtistRepo) Put(ar *model.Artist, columsToUpdate ...string) error {
|
||||
if ar.ID == "" {
|
||||
ar.ID = id.NewRandom()
|
||||
}
|
||||
if m.Data == nil {
|
||||
m.Data = make(map[string]*model.Artist)
|
||||
}
|
||||
m.Data[ar.ID] = ar
|
||||
return nil
|
||||
}
|
||||
@ -137,10 +140,7 @@ func (m *MockArtistRepo) GetCursor(options ...model.QueryOptions) (model.ArtistC
|
||||
}
|
||||
|
||||
func (m *MockArtistRepo) UpdateExternalInfo(artist *model.Artist) error {
|
||||
if m.Err {
|
||||
return errors.New("mock repo error")
|
||||
}
|
||||
return nil
|
||||
return m.Put(artist)
|
||||
}
|
||||
|
||||
func (m *MockArtistRepo) RefreshStats(allArtists bool) (int64, error) {
|
||||
|
||||
@ -38,15 +38,15 @@ const ArtistExternalLinks = ({ artistInfo, record }) => {
|
||||
}
|
||||
|
||||
if (config.lastFMEnabled) {
|
||||
if (lastFMlink && isLastFmURL(lastFMlink[2])) {
|
||||
// No agent may be enabled to supply a canonical URL, so fall back to a name-based one.
|
||||
const lastFMUrl =
|
||||
(lastFMlink && isLastFmURL(lastFMlink[2]) && lastFMlink[2]) ||
|
||||
(isLastFmURL(artistInfo?.lastFmUrl) && artistInfo.lastFmUrl) ||
|
||||
(record.name &&
|
||||
`https://last.fm/music/${encodeURIComponent(record.name)}`)
|
||||
if (lastFMUrl) {
|
||||
addLink(
|
||||
lastFMlink[2],
|
||||
'message.openIn.lastfm',
|
||||
<ImLastfm2 className="lastfm-icon" />,
|
||||
)
|
||||
} else if (isLastFmURL(artistInfo?.lastFmUrl)) {
|
||||
addLink(
|
||||
artistInfo?.lastFmUrl,
|
||||
lastFMUrl,
|
||||
'message.openIn.lastfm',
|
||||
<ImLastfm2 className="lastfm-icon" />,
|
||||
)
|
||||
|
||||
60
ui/src/artist/ArtistExternalLink.test.jsx
Normal file
60
ui/src/artist/ArtistExternalLink.test.jsx
Normal file
@ -0,0 +1,60 @@
|
||||
import React from 'react'
|
||||
import { render, screen } from '@testing-library/react'
|
||||
import { describe, it, expect, beforeEach, vi } from 'vitest'
|
||||
import { TestContext } from 'ra-test'
|
||||
import ArtistExternalLinks from './ArtistExternalLink'
|
||||
|
||||
const { mockConfig } = vi.hoisted(() => ({
|
||||
mockConfig: { lastFMEnabled: true },
|
||||
}))
|
||||
vi.mock('../config', () => ({ default: mockConfig }))
|
||||
|
||||
describe('ArtistExternalLinks', () => {
|
||||
beforeEach(() => {
|
||||
mockConfig.lastFMEnabled = true
|
||||
})
|
||||
|
||||
const renderLinks = (artistInfo, record = { id: 'ar-1', name: 'Björk' }) =>
|
||||
render(
|
||||
<TestContext>
|
||||
<ArtistExternalLinks artistInfo={artistInfo} record={record} />
|
||||
</TestContext>,
|
||||
)
|
||||
|
||||
const lastFmHref = () =>
|
||||
screen.getByLabelText('message.openIn.lastfm').closest('a').href
|
||||
|
||||
it('uses the URL returned by the server', () => {
|
||||
renderLinks({ lastFmUrl: 'https://www.last.fm/music/Bjork' })
|
||||
expect(lastFmHref()).toBe('https://www.last.fm/music/Bjork')
|
||||
})
|
||||
|
||||
it('uses the URL found in the biography', () => {
|
||||
renderLinks({
|
||||
biography: 'Read more on <a href="https://www.last.fm/music/Bjork">',
|
||||
lastFmUrl: 'https://bjork.com',
|
||||
})
|
||||
expect(lastFmHref()).toBe('https://www.last.fm/music/Bjork')
|
||||
})
|
||||
|
||||
it('builds the URL from the artist name when the server has none', () => {
|
||||
renderLinks({ lastFmUrl: 'https://bjork.com' })
|
||||
expect(lastFmHref()).toBe('https://last.fm/music/Bj%C3%B6rk')
|
||||
})
|
||||
|
||||
it('builds the URL when there is no artist info at all', () => {
|
||||
renderLinks(undefined)
|
||||
expect(lastFmHref()).toBe('https://last.fm/music/Bj%C3%B6rk')
|
||||
})
|
||||
|
||||
it('shows no Last.fm link when Last.fm is disabled', () => {
|
||||
mockConfig.lastFMEnabled = false
|
||||
renderLinks({ lastFmUrl: 'https://www.last.fm/music/Bjork' })
|
||||
expect(screen.queryByLabelText('message.openIn.lastfm')).toBeNull()
|
||||
})
|
||||
|
||||
it('shows no Last.fm link when the artist has no name', () => {
|
||||
renderLinks({}, { id: 'ar-1', name: '' })
|
||||
expect(screen.queryByLabelText('message.openIn.lastfm')).toBeNull()
|
||||
})
|
||||
})
|
||||
@ -53,7 +53,7 @@ const useStyles = makeStyles(
|
||||
},
|
||||
)
|
||||
|
||||
const ArtistDetails = (props) => {
|
||||
export const ArtistDetails = (props) => {
|
||||
const record = useRecordContext(props)
|
||||
const isDesktop = useMediaQuery((theme) => theme.breakpoints.up('sm'), {
|
||||
noSsr: true,
|
||||
@ -75,7 +75,9 @@ const ArtistDetails = (props) => {
|
||||
// eslint-disable-next-line no-console
|
||||
console.error('error on artist page', e)
|
||||
})
|
||||
}, [record.id])
|
||||
// Keyed on the record, not its id: a refreshed record must re-fetch, or the stale
|
||||
// artistInfo state keeps winning the `||` above.
|
||||
}, [record])
|
||||
|
||||
const Component = isDesktop ? DesktopArtistDetails : MobileArtistDetails
|
||||
return (
|
||||
|
||||
63
ui/src/artist/ArtistShow.test.jsx
Normal file
63
ui/src/artist/ArtistShow.test.jsx
Normal file
@ -0,0 +1,63 @@
|
||||
import React from 'react'
|
||||
import { render, waitFor } from '@testing-library/react'
|
||||
import { RecordContextProvider } from 'react-admin'
|
||||
import { ThemeProvider, createTheme } from '@material-ui/core/styles'
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
import { ArtistDetails } from './ArtistShow'
|
||||
import subsonic from '../subsonic'
|
||||
|
||||
vi.mock('../subsonic', () => ({
|
||||
default: { getArtistInfo: vi.fn(), getCoverArtUrl: vi.fn() },
|
||||
}))
|
||||
|
||||
// Not under test here: isolate ArtistDetails from the leaf presentational views.
|
||||
vi.mock('./DesktopArtistDetails', () => ({ default: () => null }))
|
||||
vi.mock('./MobileArtistDetails', () => ({ default: () => null }))
|
||||
|
||||
const mockGetArtistInfo = subsonic.getArtistInfo
|
||||
|
||||
describe('ArtistDetails', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
mockGetArtistInfo.mockResolvedValue({
|
||||
json: {
|
||||
'subsonic-response': {
|
||||
status: 'ok',
|
||||
artistInfo: { biography: 'fetched' },
|
||||
},
|
||||
},
|
||||
})
|
||||
})
|
||||
|
||||
const theme = createTheme()
|
||||
|
||||
const wrap = (record) => (
|
||||
<ThemeProvider theme={theme}>
|
||||
<RecordContextProvider value={record}>
|
||||
<ArtistDetails />
|
||||
</RecordContextProvider>
|
||||
</ThemeProvider>
|
||||
)
|
||||
|
||||
const renderDetails = (record) => render(wrap(record))
|
||||
|
||||
it('re-fetches the artist info when the record object changes', async () => {
|
||||
const record = { id: 'ar1', name: 'Artist', biography: 'old' }
|
||||
const { rerender } = renderDetails(record)
|
||||
await waitFor(() => expect(mockGetArtistInfo).toHaveBeenCalledTimes(1))
|
||||
|
||||
rerender(wrap({ ...record, biography: 'new' }))
|
||||
|
||||
await waitFor(() => expect(mockGetArtistInfo).toHaveBeenCalledTimes(2))
|
||||
})
|
||||
|
||||
it('does not re-fetch when the same record object is passed again', async () => {
|
||||
const record = { id: 'ar1', name: 'Artist', biography: 'old' }
|
||||
const { rerender } = renderDetails(record)
|
||||
await waitFor(() => expect(mockGetArtistInfo).toHaveBeenCalledTimes(1))
|
||||
|
||||
rerender(wrap(record))
|
||||
|
||||
expect(mockGetArtistInfo).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
@ -7,7 +7,12 @@ import MenuItem from '@material-ui/core/MenuItem'
|
||||
import MoreVertIcon from '@material-ui/icons/MoreVert'
|
||||
import { MdQuestionMark } from 'react-icons/md'
|
||||
import { makeStyles } from '@material-ui/core/styles'
|
||||
import { useDataProvider, useNotify, useTranslate } from 'react-admin'
|
||||
import {
|
||||
useDataProvider,
|
||||
useNotify,
|
||||
usePermissions,
|
||||
useTranslate,
|
||||
} from 'react-admin'
|
||||
import clsx from 'clsx'
|
||||
import {
|
||||
playNext,
|
||||
@ -69,6 +74,7 @@ const ContextMenu = ({
|
||||
const dispatch = useDispatch()
|
||||
const translate = useTranslate()
|
||||
const notify = useNotify()
|
||||
const { permissions } = usePermissions()
|
||||
const [anchorEl, setAnchorEl] = useState(null)
|
||||
|
||||
const isArtist = resource === 'artist'
|
||||
@ -129,6 +135,16 @@ const ContextMenu = ({
|
||||
)
|
||||
},
|
||||
},
|
||||
refresh: {
|
||||
enabled: permissions === 'admin',
|
||||
needData: false,
|
||||
label: translate('resources.album.actions.refresh'),
|
||||
action: (record) =>
|
||||
dataProvider
|
||||
.refreshMetadata(resource, record.id)
|
||||
.then(() => notify('message.metadataRefreshStarted'))
|
||||
.catch(() => notify('ra.page.error', 'warning')),
|
||||
},
|
||||
...(!hideInfo && {
|
||||
info: {
|
||||
enabled: true,
|
||||
|
||||
@ -17,12 +17,21 @@ const { mockConfig } = vi.hoisted(() => ({
|
||||
}))
|
||||
vi.mock('../config', () => ({ default: mockConfig }))
|
||||
|
||||
const { mockPermissions, mockRefreshMetadata } = vi.hoisted(() => ({
|
||||
mockPermissions: { value: 'admin' },
|
||||
mockRefreshMetadata: vi.fn(),
|
||||
}))
|
||||
|
||||
vi.mock('react-admin', async (importOriginal) => {
|
||||
const actual = await importOriginal()
|
||||
return {
|
||||
...actual,
|
||||
useNotify: () => vi.fn(),
|
||||
useDataProvider: () => ({ getList: vi.fn() }),
|
||||
usePermissions: () => ({ permissions: mockPermissions.value }),
|
||||
useDataProvider: () => ({
|
||||
getList: vi.fn(),
|
||||
refreshMetadata: mockRefreshMetadata,
|
||||
}),
|
||||
useTranslate: () => (x) => x,
|
||||
}
|
||||
})
|
||||
@ -43,6 +52,7 @@ describe('ContextMenus', () => {
|
||||
vi.clearAllMocks()
|
||||
mockConfig.enableSharing = true
|
||||
mockConfig.enableDownloads = true
|
||||
mockPermissions.value = 'admin'
|
||||
})
|
||||
|
||||
describe('ArtistContextMenu', () => {
|
||||
@ -75,4 +85,49 @@ describe('ContextMenus', () => {
|
||||
expect(screen.getByText('ra.action.download (1 MB)')).toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
|
||||
describe('refresh metadata', () => {
|
||||
it('shows the item for admins on the album menu', () => {
|
||||
renderMenu(AlbumContextMenu, { id: 'al1', name: 'Album', songCount: 1 })
|
||||
expect(
|
||||
screen.getByText('resources.album.actions.refresh'),
|
||||
).toBeInTheDocument()
|
||||
})
|
||||
|
||||
// Menu order comes from key insertion order in the options object, so it is easy to
|
||||
// change by accident when adding an entry.
|
||||
it('places the item directly above Get Info', () => {
|
||||
renderMenu(AlbumContextMenu, { id: 'al1', name: 'Album', songCount: 1 })
|
||||
const labels = screen
|
||||
.getAllByRole('menuitem')
|
||||
.map((item) => item.textContent)
|
||||
const refreshAt = labels.indexOf('resources.album.actions.refresh')
|
||||
const infoAt = labels.indexOf('resources.album.actions.info')
|
||||
|
||||
expect(refreshAt).toBeGreaterThanOrEqual(0)
|
||||
expect(infoAt).toEqual(refreshAt + 1)
|
||||
})
|
||||
|
||||
it('shows the item for admins on the artist menu', () => {
|
||||
renderMenu(ArtistContextMenu, { id: 'ar1', name: 'Artist', stats: {} })
|
||||
expect(
|
||||
screen.getByText('resources.album.actions.refresh'),
|
||||
).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('hides the item for regular users', () => {
|
||||
mockPermissions.value = 'regular'
|
||||
renderMenu(AlbumContextMenu, { id: 'al1', name: 'Album', songCount: 1 })
|
||||
expect(
|
||||
screen.queryByText('resources.album.actions.refresh'),
|
||||
).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('calls refreshMetadata with the resource and id', () => {
|
||||
mockRefreshMetadata.mockResolvedValue({})
|
||||
renderMenu(AlbumContextMenu, { id: 'al1', name: 'Album', songCount: 1 })
|
||||
fireEvent.click(screen.getByText('resources.album.actions.refresh'))
|
||||
expect(mockRefreshMetadata).toHaveBeenCalledWith('album', 'al1')
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@ -4,6 +4,8 @@ import { REST_URL } from '../consts'
|
||||
|
||||
const dataProvider = jsonServerProvider(REST_URL, httpClient)
|
||||
|
||||
const REFRESH_KIND = { album: 'al', artist: 'ar' }
|
||||
|
||||
const isAdmin = () => {
|
||||
const role = localStorage.getItem('role')
|
||||
return role === 'admin'
|
||||
@ -221,6 +223,12 @@ const wrapperDataProvider = {
|
||||
data: json,
|
||||
}))
|
||||
},
|
||||
// The endpoint answers 204 with no body, but react-admin rejects any response without a
|
||||
// `data` key, so the id stands in for one.
|
||||
refreshMetadata: (resource, id) =>
|
||||
httpClient(`${REST_URL}/metadata/${REFRESH_KIND[resource]}/${id}/refresh`, {
|
||||
method: 'POST',
|
||||
}).then(() => ({ data: { id } })),
|
||||
}
|
||||
|
||||
export default wrapperDataProvider
|
||||
|
||||
@ -87,4 +87,37 @@ describe('wrapperDataProvider', () => {
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe('refreshMetadata', () => {
|
||||
it('posts to the album metadata refresh endpoint', () => {
|
||||
mockHttpClient.mockResolvedValue({ json: {} })
|
||||
wrapperDataProvider.refreshMetadata('album', 'al-1')
|
||||
expect(mockHttpClient).toHaveBeenCalledWith(
|
||||
expect.stringContaining('/metadata/al/al-1/refresh'),
|
||||
{ method: 'POST' },
|
||||
)
|
||||
})
|
||||
|
||||
it('posts to the artist metadata refresh endpoint', () => {
|
||||
mockHttpClient.mockResolvedValue({ json: {} })
|
||||
wrapperDataProvider.refreshMetadata('artist', 'ar-1')
|
||||
expect(mockHttpClient).toHaveBeenCalledWith(
|
||||
expect.stringContaining('/metadata/ar/ar-1/refresh'),
|
||||
{ method: 'POST' },
|
||||
)
|
||||
})
|
||||
|
||||
// react-admin rejects a custom method whose response has no `data` key, and the
|
||||
// endpoint answers 204 with no body.
|
||||
it('resolves to a react-admin shaped response', async () => {
|
||||
mockHttpClient.mockResolvedValue({
|
||||
status: 204,
|
||||
body: '',
|
||||
json: undefined,
|
||||
})
|
||||
await expect(
|
||||
wrapperDataProvider.refreshMetadata('album', 'al-1'),
|
||||
).resolves.toEqual({ data: { id: 'al-1' } })
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@ -93,7 +93,8 @@
|
||||
"shuffle": "Shuffle",
|
||||
"addToPlaylist": "Add to Playlist",
|
||||
"download": "Download",
|
||||
"info": "Get Info"
|
||||
"info": "Get Info",
|
||||
"refresh": "Refresh Metadata"
|
||||
},
|
||||
"lists": {
|
||||
"all": "All",
|
||||
@ -569,6 +570,7 @@
|
||||
"coverRemoved": "Cover art removed",
|
||||
"coverUploadError": "Error uploading cover art",
|
||||
"coverRemoveError": "Error removing cover art",
|
||||
"metadataRefreshStarted": "Refreshing metadata in the background",
|
||||
"note": "NOTE",
|
||||
"transcodingDisabled": "Changing the transcoding configuration through the web interface is disabled for security reasons. If you would like to change (edit or add) transcoding options, restart the server with the %{config} configuration option.",
|
||||
"transcodingEnabled": "Navidrome is currently running with %{config}, making it possible to run system commands from the transcoding settings using the web interface. We recommend to disable it for security reasons and only enable it when configuring Transcoding options.",
|
||||
|
||||
11
utils/cache/file_caches.go
vendored
11
utils/cache/file_caches.go
vendored
@ -255,6 +255,17 @@ func (fc *fileCache) copyAndClose(ctx context.Context, key string, w io.WriteClo
|
||||
}
|
||||
if err == nil {
|
||||
fc.markComplete(ctx, key)
|
||||
} else if cw, ok := w.(interface{ CloseWithError(error) error }); ok {
|
||||
// Cancel instead of close, so readers fail with the cause rather than
|
||||
// draining a truncated entry to a clean EOF.
|
||||
if cErr := cw.CloseWithError(err); cErr != nil {
|
||||
// Join, not Append: err is now shared with readers and must not be mutated.
|
||||
return errors.Join(err, fmt.Errorf("closing cache writer: %w", cErr))
|
||||
}
|
||||
return err
|
||||
} else {
|
||||
log.Warn(ctx, "Cache writer cannot report failures; readers will see a truncated entry as a clean EOF",
|
||||
"cache", fc.name, "key", key, err)
|
||||
}
|
||||
if cErr := w.Close(); cErr != nil {
|
||||
err = multierror.Append(err, fmt.Errorf("closing cache writer: %w", cErr))
|
||||
|
||||
62
utils/cache/file_caches_test.go
vendored
62
utils/cache/file_caches_test.go
vendored
@ -259,6 +259,54 @@ var _ = Describe("File Caches", func() {
|
||||
}).Should(BeTrue())
|
||||
})
|
||||
|
||||
It("gets a writer that can report failures to readers", func() {
|
||||
// Guards the fork adoption: if the fscache replace directive is ever lost,
|
||||
// this fails in CI instead of silently reviving the truncation bug.
|
||||
fc := callNewFileCache("test", "10MB", "test", 0, nil)
|
||||
_, w, err := fc.cache.Get("capability")
|
||||
Expect(err).To(BeNil())
|
||||
DeferCleanup(func() { _ = w.Close() })
|
||||
|
||||
_, ok := w.(interface{ CloseWithError(error) error })
|
||||
Expect(ok).To(BeTrue(), "fscache writer lost CloseWithError; check the go.mod replace directive")
|
||||
})
|
||||
|
||||
It("fails the reader with the cause instead of a clean EOF", func() {
|
||||
fc := callNewFileCache("test", "10MB", "test", 0, func(ctx context.Context, arg Item) (io.Reader, error) {
|
||||
return &partialThenErrReader{data: []byte("PARTIAL"), err: errors.New("transcoder died")}, nil
|
||||
})
|
||||
s, err := fc.Get(context.Background(), &testArg{"inband"})
|
||||
Expect(err).To(BeNil())
|
||||
DeferCleanup(func() { _ = s.Close() })
|
||||
|
||||
_, err = io.ReadAll(s)
|
||||
Expect(err).To(MatchError(ContainSubstring("transcoder died")))
|
||||
})
|
||||
|
||||
It("fails a reader that joined mid-write with the same cause", func() {
|
||||
pr, pw := io.Pipe()
|
||||
fc := callNewFileCache("test", "10MB", "test", 0, func(ctx context.Context, arg Item) (io.Reader, error) {
|
||||
return pr, nil
|
||||
})
|
||||
s1, err := fc.Get(context.Background(), &testArg{"joined"})
|
||||
Expect(err).To(BeNil())
|
||||
DeferCleanup(func() { _ = s1.Close() })
|
||||
|
||||
// The blocking pipe write gives a happens-before: the entry is in flight.
|
||||
_, err = pw.Write([]byte("PARTIAL"))
|
||||
Expect(err).To(BeNil())
|
||||
|
||||
s2, err := fc.Get(context.Background(), &testArg{"joined"})
|
||||
Expect(err).To(BeNil())
|
||||
DeferCleanup(func() { _ = s2.Close() })
|
||||
Expect(s2.Cached).To(BeTrue())
|
||||
|
||||
Expect(pw.CloseWithError(errors.New("transcoder died"))).To(Succeed())
|
||||
|
||||
_, err = io.ReadAll(s2)
|
||||
Expect(err).To(MatchError(ContainSubstring("transcoder died")))
|
||||
})
|
||||
|
||||
It("does not write a completion marker when the write fails after partial bytes", func() {
|
||||
// Mimics a transcode that produces real output and then dies:
|
||||
// the bytes land on disk, but the entry must NOT be marked complete.
|
||||
@ -304,9 +352,9 @@ var _ = Describe("File Caches", func() {
|
||||
Expect(calls.Load()).To(BeNumerically("==", 2))
|
||||
})
|
||||
|
||||
It("survives an invalidated entry's deferred file removal", func() {
|
||||
// invalidate() drops the map entry but defers the unlink until readers close;
|
||||
// a Get in that window re-creates the file, which the deferred unlink then eats.
|
||||
It("removes a failed entry promptly, without eating its replacement", func() {
|
||||
// Cancel closes the failed entry's readers, so its removal no longer defers
|
||||
// past the point where a new entry re-creates the same file.
|
||||
var n atomic.Int32
|
||||
fc := callNewFileCache("test", "10MB", "test", 0, func(ctx context.Context, arg Item) (io.Reader, error) {
|
||||
if n.Add(1) == 1 {
|
||||
@ -319,7 +367,6 @@ var _ = Describe("File Caches", func() {
|
||||
s1, err := fc.Get(context.Background(), &testArg{"deferred"})
|
||||
Expect(err).To(BeNil())
|
||||
|
||||
// The failed write invalidates the entry; the removal now waits on s1.
|
||||
Eventually(func() bool { return fc.cache.Exists(key) }).Should(BeFalse())
|
||||
|
||||
s2, err := fc.Get(context.Background(), &testArg{"deferred"})
|
||||
@ -330,15 +377,16 @@ var _ = Describe("File Caches", func() {
|
||||
Expect(s1.Close()).To(Succeed())
|
||||
|
||||
dataPath := fcSpreadFS(fc).KeyMapper(key)
|
||||
Eventually(func() bool {
|
||||
Consistently(func() error {
|
||||
_, e := os.Stat(dataPath)
|
||||
return os.IsNotExist(e)
|
||||
}).Should(BeTrue(), "expected the deferred removal to take the re-created file")
|
||||
return e
|
||||
}).Should(Succeed(), "the replacement entry's file must survive the failed entry's cleanup")
|
||||
|
||||
s3, err := fc.Get(context.Background(), &testArg{"deferred"})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(io.ReadAll(s3)).To(Equal([]byte("GOOD")))
|
||||
_ = s3.Close()
|
||||
Expect(n.Load()).To(Equal(int32(2)), "the third Get must be served from cache")
|
||||
})
|
||||
|
||||
It("re-fetches when an adopted entry's data file vanished", func() {
|
||||
|
||||
35
utils/httpclient/httpclient.go
Normal file
35
utils/httpclient/httpclient.go
Normal file
@ -0,0 +1,35 @@
|
||||
// Package httpclient provides a shared http.Client factory that identifies
|
||||
// Navidrome via the User-Agent header on all outgoing requests.
|
||||
package httpclient
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/consts"
|
||||
)
|
||||
|
||||
type uaTransport struct {
|
||||
base http.RoundTripper
|
||||
}
|
||||
|
||||
func (t *uaTransport) RoundTrip(req *http.Request) (*http.Response, error) {
|
||||
if _, ok := req.Header["User-Agent"]; !ok {
|
||||
req = req.Clone(req.Context())
|
||||
req.Header.Set("User-Agent", consts.HTTPUserAgent)
|
||||
}
|
||||
return t.base.RoundTrip(req)
|
||||
}
|
||||
|
||||
// NewTransport wraps base (or http.DefaultTransport if nil) to set the
|
||||
// Navidrome User-Agent on requests that don't have one.
|
||||
func NewTransport(base http.RoundTripper) http.RoundTripper {
|
||||
if base == nil {
|
||||
base = http.DefaultTransport
|
||||
}
|
||||
return &uaTransport{base: base}
|
||||
}
|
||||
|
||||
func New(timeout time.Duration) *http.Client {
|
||||
return &http.Client{Timeout: timeout, Transport: NewTransport(nil)}
|
||||
}
|
||||
17
utils/httpclient/httpclient_suite_test.go
Normal file
17
utils/httpclient/httpclient_suite_test.go
Normal file
@ -0,0 +1,17 @@
|
||||
package httpclient_test
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/tests"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
)
|
||||
|
||||
func TestHTTPClient(t *testing.T) {
|
||||
tests.Init(t, false)
|
||||
log.SetLevel(log.LevelFatal)
|
||||
RegisterFailHandler(Fail)
|
||||
RunSpecs(t, "HTTPClient Suite")
|
||||
}
|
||||
76
utils/httpclient/httpclient_test.go
Normal file
76
utils/httpclient/httpclient_test.go
Normal file
@ -0,0 +1,76 @@
|
||||
package httpclient_test
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/consts"
|
||||
"github.com/navidrome/navidrome/utils/httpclient"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
)
|
||||
|
||||
var _ = Describe("httpclient", func() {
|
||||
var server *httptest.Server
|
||||
var receivedUA string
|
||||
|
||||
BeforeEach(func() {
|
||||
server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
receivedUA = r.Header.Get("User-Agent")
|
||||
}))
|
||||
DeferCleanup(server.Close)
|
||||
})
|
||||
|
||||
Describe("New", func() {
|
||||
It("sets the Navidrome User-Agent when the request has none", func() {
|
||||
c := httpclient.New(time.Second)
|
||||
resp, err := c.Get(server.URL)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
resp.Body.Close()
|
||||
Expect(receivedUA).To(Equal(consts.HTTPUserAgent))
|
||||
})
|
||||
|
||||
It("keeps a User-Agent already set by the caller", func() {
|
||||
c := httpclient.New(time.Second)
|
||||
req, err := http.NewRequest(http.MethodGet, server.URL, nil)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
req.Header.Set("User-Agent", "CustomAgent/1.0")
|
||||
resp, err := c.Do(req)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
resp.Body.Close()
|
||||
Expect(receivedUA).To(Equal("CustomAgent/1.0"))
|
||||
})
|
||||
|
||||
It("applies the given timeout", func() {
|
||||
c := httpclient.New(5 * time.Second)
|
||||
Expect(c.Timeout).To(Equal(5 * time.Second))
|
||||
})
|
||||
})
|
||||
|
||||
Describe("NewTransport", func() {
|
||||
It("uses the default transport when base is nil", func() {
|
||||
c := &http.Client{Transport: httpclient.NewTransport(nil)}
|
||||
resp, err := c.Get(server.URL)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
resp.Body.Close()
|
||||
Expect(receivedUA).To(Equal(consts.HTTPUserAgent))
|
||||
})
|
||||
|
||||
It("does not modify the original request", func() {
|
||||
c := &http.Client{Transport: httpclient.NewTransport(nil)}
|
||||
req, err := http.NewRequest(http.MethodGet, server.URL, nil)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
resp, err := c.Do(req)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
resp.Body.Close()
|
||||
Expect(req.Header).ToNot(HaveKey("User-Agent"))
|
||||
})
|
||||
})
|
||||
|
||||
Describe("HTTPUserAgent", func() {
|
||||
It("identifies Navidrome with version and project URL", func() {
|
||||
Expect(consts.HTTPUserAgent).To(Equal("Navidrome/" + consts.Version + " - https://github.com/navidrome"))
|
||||
})
|
||||
})
|
||||
})
|
||||
Loading…
x
Reference in New Issue
Block a user