feat(ui): add Refresh Metadata to the album and artist context menus (#6036)
* refactor(artwork): move artworkItemName into core/artwork as ItemName
* feat(external): add RefreshInfo to force an external info refresh
RefreshInfo re-fetches and re-saves external info for one artist or
album, bypassing the TTL check that UpdateArtistInfo/UpdateAlbumInfo
use. It is synchronous; callers that must not block detach it themselves.
Also makes MockArtistRepo/MockAlbumRepo.UpdateExternalInfo persist to
Data (previously a no-op) and adds the new method to the e2e noopProvider,
both required so the interface addition compiles and is observable in tests.
* feat(external): broadcast RefreshResource after external info is saved
populateArtistInfo and populateAlbumInfo now emit the same RefreshResource
event the artwork worker uses, so the UI learns about both foreground and
background metadata refreshes.
* feat(nativeapi): replace artwork refresh endpoint with metadata refresh
* feat(ui): add refreshMetadata to the data provider
* feat(ui): add a Refresh Metadata item to the album and artist context menus
* fix(ui): re-fetch artist info when the record is refreshed
* test: fix mislabeled spec, add kind-gate negative case, guard nil mock maps
- Rename the RefreshInfo spec that claimed to cover the save-failure/broadcast
path: SetError(true) fails Get too, so it only proves RefreshInfo bails out
early at getArtist.
- Add a spec proving playlist refreshes skip the external-info step, since
that asymmetry (al/ar only) was documented but unasserted.
- Add lazy nil-map init to MockAlbumRepo/MockArtistRepo.UpdateExternalInfo so
a composite-literal-constructed mock doesn't panic on first save.
* test: relocate discArtworkName specs from cmd to core/artwork
artworkItemName moved into core/artwork as ItemName in an earlier commit, but
its disc-name specs stayed behind in cmd/artwork_test.go, reaching across
packages. Move them to core/artwork/item_name_test.go where the code now lives.
* fix(ui): shape refreshMetadata like a react-admin response
react-admin validates custom dataProvider methods and rejects any response
without a `data` key, so the raw httpClient promise made every click surface
an error toast instead of the success message. The unit test mocked
useDataProvider, which skips that validation.
Also folds "which kinds have external info" into external.HasInfo so the
handler stops restating it, drops the nil-broker guard that only existed for
tests, and delegates the mocks' UpdateExternalInfo to Put.
* refactor(external): unexport infoKinds
Only HasInfo is used outside the package, so the slice itself does not need
to be exported.
* refactor(artwork): fold ItemName into housekeeping.go next to Refresh
ItemName exists to guard Refresh from ids that would orphan a queue row, and
both callers invoke them back to back. A separate file hid that pairing; it was
only split out to keep the move out of cmd/ legible in review.
* fix(nativeapi): return 500 when the refresh lookup fails for a non-ErrNotFound reason
A transient repository error told the admin the id did not exist, and the error
was dropped without a log line, so nothing pointed at the real cause.
Also drops the inherited claim that clearing artwork state shows a placeholder.
Reads fall back to local resolution, so that only holds when there is no local art.
* fix(ui): move Refresh Metadata above Get Info in the context menu
Menu order follows key insertion order in the options object, so the new spec
pins the position rather than leaving it to be shuffled by the next addition.
2026-08-25 23:59:40 -04:00
|
|
|
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{}
|
feat(jellyfin): add Quick Connect sign-in (#6174)
* feat(jellyfin): add Quick Connect sign-in
Jellyfin clients can now sign in without a password: the client shows a
6-digit code, a signed-in user approves it, and the client redeems a secret
for its access token.
- core/quickconnect: in-memory store shared by both routers through wire.
Codes expire after 10 minutes; a secret redeems only once (Jellyfin
allows repeats for 10 minutes); at most 1000 pending requests.
- Jellyfin API: Initiate, Connect, Authorize and AuthenticateWithQuickConnect.
Admins may approve for another user via UserId, like Swiftfin's admin page.
Initiate and redeem share the login rate limiter; Connect does not, since
Finamp and Streamyfin poll it every second.
- Web UI: a Quick Connect item in the user menu looks up the code and shows
the app and device before approving, so a user can't be tricked into
approving an unknown device blindly.
- Jellyfin.QuickConnect option, on by default like Jellyfin. It only matters
when the Jellyfin API is enabled.
* refactor(jellyfin): tidy Quick Connect naming and route guards
Group the Quick Connect routes under one requireQuickConnect guard, make the
request's device a named field so req.Device.ID can't be mistaken for a
request id, and rename the web API response type to quickConnectDevice.
* refactor(jellyfin): remove duplicated Jellyfin date formatting function
* test(jellyfin): set play count and starred in the song fixture literal
* refactor(jellyfin): inline the Quick Connect redeem body and use the shared date helper
* fix(jellyfin): bound the client fields Quick Connect keeps in memory
Initiate is unauthenticated and keeps the Client, Device, DeviceId and Version
header fields for up to ten minutes. With no header size limit, each pending
request could hold about 1 MB, and even a short field kept the whole header
alive because the parsed values are substrings of it. Reject fields over 512
bytes and copy the stored values.
Also answer 500 instead of 401 when the redeem user lookup fails for a reason
other than the user being gone.
* fix(jellyfin): rate-limit Quick Connect code approval
Any signed-in user could try codes without limit on the Jellyfin Authorize
endpoint and the web UI lookup/authorize endpoints, and so could approve
another person's pending device for their own account. Apply the same per-IP
limiter as the login (AuthRequestLimit/AuthWindowLength) to both surfaces.
2026-09-19 14:57:01 -04:00
|
|
|
nativeRouter := New(ds, nil, nil, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, nil, nil, provider, nil)
|
feat(ui): add Refresh Metadata to the album and artist context menus (#6036)
* refactor(artwork): move artworkItemName into core/artwork as ItemName
* feat(external): add RefreshInfo to force an external info refresh
RefreshInfo re-fetches and re-saves external info for one artist or
album, bypassing the TTL check that UpdateArtistInfo/UpdateAlbumInfo
use. It is synchronous; callers that must not block detach it themselves.
Also makes MockArtistRepo/MockAlbumRepo.UpdateExternalInfo persist to
Data (previously a no-op) and adds the new method to the e2e noopProvider,
both required so the interface addition compiles and is observable in tests.
* feat(external): broadcast RefreshResource after external info is saved
populateArtistInfo and populateAlbumInfo now emit the same RefreshResource
event the artwork worker uses, so the UI learns about both foreground and
background metadata refreshes.
* feat(nativeapi): replace artwork refresh endpoint with metadata refresh
* feat(ui): add refreshMetadata to the data provider
* feat(ui): add a Refresh Metadata item to the album and artist context menus
* fix(ui): re-fetch artist info when the record is refreshed
* test: fix mislabeled spec, add kind-gate negative case, guard nil mock maps
- Rename the RefreshInfo spec that claimed to cover the save-failure/broadcast
path: SetError(true) fails Get too, so it only proves RefreshInfo bails out
early at getArtist.
- Add a spec proving playlist refreshes skip the external-info step, since
that asymmetry (al/ar only) was documented but unasserted.
- Add lazy nil-map init to MockAlbumRepo/MockArtistRepo.UpdateExternalInfo so
a composite-literal-constructed mock doesn't panic on first save.
* test: relocate discArtworkName specs from cmd to core/artwork
artworkItemName moved into core/artwork as ItemName in an earlier commit, but
its disc-name specs stayed behind in cmd/artwork_test.go, reaching across
packages. Move them to core/artwork/item_name_test.go where the code now lives.
* fix(ui): shape refreshMetadata like a react-admin response
react-admin validates custom dataProvider methods and rejects any response
without a `data` key, so the raw httpClient promise made every click surface
an error toast instead of the success message. The unit test mocked
useDataProvider, which skips that validation.
Also folds "which kinds have external info" into external.HasInfo so the
handler stops restating it, drops the nil-broker guard that only existed for
tests, and delegates the mocks' UpdateExternalInfo to Put.
* refactor(external): unexport infoKinds
Only HasInfo is used outside the package, so the slice itself does not need
to be exported.
* refactor(artwork): fold ItemName into housekeeping.go next to Refresh
ItemName exists to guard Refresh from ids that would orphan a queue row, and
both callers invoke them back to back. A separate file hid that pairing; it was
only split out to keep the move out of cmd/ legible in review.
* fix(nativeapi): return 500 when the refresh lookup fails for a non-ErrNotFound reason
A transient repository error told the admin the id did not exist, and the error
was dropped without a log line, so nothing pointed at the real cause.
Also drops the inherited claim that clearing artwork state shows a placeholder.
Reads fall back to local resolution, so that only holds when there is no local art.
* fix(ui): move Refresh Metadata above Get Info in the context menu
Menu order follows key insertion order in the options object, so the new spec
pins the position rather than leaving it to be shuffled by the next addition.
2026-08-25 23:59:40 -04:00
|
|
|
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))
|
|
|
|
|
})
|
|
|
|
|
})
|
|
|
|
|
})
|