diff --git a/.github/workflows/pipeline.yml b/.github/workflows/pipeline.yml index c1ba8714c..3bc539132 100644 --- a/.github/workflows/pipeline.yml +++ b/.github/workflows/pipeline.yml @@ -519,13 +519,18 @@ jobs: docker buildx imagetools inspect ${{ env.REGISTRY_IMAGE }}:${{ steps.docker.outputs.version }} push-manifest-dockerhub: - name: Push to Docker Hub + name: Push to Docker Hub (${{ matrix.repository }}) runs-on: ubuntu-latest permissions: contents: read needs: [build, check-push-enabled] if: needs.check-push-enabled.outputs.is_enabled == 'true' && vars.DOCKER_HUB_REPO != '' continue-on-error: true + strategy: + fail-fast: false + matrix: + # Always push to DOCKER_HUB_REPO. The upstream repository also pushes to the navidrome/navidrome org repository. + repository: ${{ fromJSON(github.repository == 'navidrome/navidrome' && format('["{0}","navidrome/navidrome"]', vars.DOCKER_HUB_REPO) || format('["{0}"]', vars.DOCKER_HUB_REPO)) }} steps: - uses: actions/checkout@v7 @@ -541,7 +546,7 @@ jobs: id: docker with: github_token: ${{ secrets.GITHUB_TOKEN }} - hub_repository: ${{ vars.DOCKER_HUB_REPO }} + hub_repository: ${{ matrix.repository }} hub_username: ${{ secrets.DOCKER_HUB_USERNAME }} hub_password: ${{ secrets.DOCKER_HUB_PASSWORD }} @@ -558,7 +563,7 @@ jobs: - name: Inspect image in Docker Hub run: | - docker buildx imagetools inspect ${{ vars.DOCKER_HUB_REPO }}:${{ steps.docker.outputs.version }} + docker buildx imagetools inspect ${{ matrix.repository }}:${{ steps.docker.outputs.version }} cleanup-digests: name: Cleanup digest artifacts diff --git a/README.md b/README.md index 4bc85e6a6..e1a2955a1 100644 --- a/README.md +++ b/README.md @@ -35,11 +35,18 @@ See instructions on the [project's website](https://www.navidrome.org/docs/insta ## Cloud Hosting -[PikaPods](https://www.pikapods.com) has partnered with us to offer you an -[officially supported, cloud-hosted solution](https://www.navidrome.org/docs/installation/managed/#pikapods). -A share of the revenue helps fund the development of Navidrome at no additional cost for you. +Several cloud hosting providers partner with us to offer [officially supported, cloud-hosted solutions](https://www.navidrome.org/docs/installation/managed). If you sign up with any of these providers, a share of the revenue funds the development of Navidrome at no additional cost for you. + +Run on PikaPods +
+Deploy with Zenith +
+Deploy on ElfHosted + + + + -[![PikaPods](https://www.pikapods.com/static/run-button.svg)](https://www.pikapods.com/pods?run=navidrome) ## Features diff --git a/core/artwork/resolve.go b/core/artwork/resolve.go index 40baa2495..fb07332fe 100644 --- a/core/artwork/resolve.go +++ b/core/artwork/resolve.go @@ -374,8 +374,11 @@ func (r *resolver) resolvePlaylist(ctx context.Context, playlistID string) (reso } } - albumIDs, err := r.ds.Playlist().Tracks(ctx, pl.ID, false). - GetAlbumIDs(ctx, model.QueryOptions{Max: PlaylistGridSamples, Sort: "random()"}) + tracks := r.ds.Playlist().Tracks(ctx, pl.ID, false) + if tracks == nil { + return resolution{}, fmt.Errorf("resolvePlaylist: could not load tracks for playlist %s", pl.ID) + } + albumIDs, err := tracks.GetAlbumIDs(ctx, model.QueryOptions{Max: PlaylistGridSamples, Sort: "random()"}) if err != nil { return resolution{}, err } diff --git a/core/artwork/resolve_test.go b/core/artwork/resolve_test.go index da144d8e2..2a36531bb 100644 --- a/core/artwork/resolve_test.go +++ b/core/artwork/resolve_test.go @@ -707,6 +707,16 @@ var _ = Describe("resolveItem", func() { Expect(err).To(HaveOccurred()) Expect(res).To(Equal(resolution{})) }) + + It("returns an error when the playlist tracks cannot be loaded", func() { + plRepo := tests.CreateMockPlaylistRepo() + plRepo.SetData(model.Playlists{{ID: "pl4", Name: "Playlist"}}) + ds.MockedPlaylist = plRepo + + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "pl4"}) + Expect(err).To(HaveOccurred()) + Expect(res).To(Equal(resolution{})) + }) }) }) diff --git a/core/artwork/worker.go b/core/artwork/worker.go index 28e51958c..4be99f92e 100644 --- a/core/artwork/worker.go +++ b/core/artwork/worker.go @@ -4,9 +4,11 @@ import ( "bytes" "cmp" "context" + "fmt" "io" "math" "math/rand/v2" + "runtime/debug" "sync" "time" @@ -244,7 +246,7 @@ func (w *Worker) process(ctx context.Context, item model.ArtworkQueueItem) (outc item.ImageType = cmp.Or(item.ImageType, model.ImageTypePrimary) trace := &ChainTrace{} ctx = withTrace(ctx, trace) - out, got, retryIn := w.proc.acquire(ctx, item) + out, got, retryIn := w.safeAcquire(ctx, item) queue := w.proc.ds.ArtworkQueue() switch out { @@ -286,6 +288,20 @@ func (w *Worker) process(ctx context.Context, item model.ArtworkQueueItem) (outc return out, got } +// safeAcquire turns a panic into a failed attempt: the drain runs on a bare goroutine, so an +// unrecovered panic would crash the server, and the still-queued row would crash it again on restart. +func (w *Worker) safeAcquire(ctx context.Context, item model.ArtworkQueueItem) (out outcome, got *acquired, retryIn time.Duration) { + defer func() { + if r := recover(); r != nil { + log.Error(ctx, "Artwork: Panic while processing item", "kind", item.ItemKind, "id", item.ItemID, + "imageType", item.ImageType, "attempts", item.Attempts, "panic", r, "stack", string(debug.Stack())) + traceStage(ctx, "panic", fmt.Errorf("%v", r)) + out, got, retryIn = outcomeFailed, nil, 0 + } + }() + return w.proc.acquire(ctx, item) +} + // recordGiveUp keeps the last failure on the state row after the queue row is deleted. An item // that never resolved has no row to update, and creating one would settle it absent. func (w *Worker) recordGiveUp(ctx context.Context, item model.ArtworkQueueItem, trace string) { diff --git a/core/artwork/worker_test.go b/core/artwork/worker_test.go index a6c07b763..80ca68bc3 100644 --- a/core/artwork/worker_test.go +++ b/core/artwork/worker_test.go @@ -142,6 +142,18 @@ func (v *visibilityPlaylistRepo) Get(ctx context.Context, id string) (*model.Pla return v.MockPlaylistRepo.Get(ctx, id) } +type panickingAlbumRepo struct { + *tests.MockAlbumRepo + panicID string +} + +func (r *panickingAlbumRepo) Get(ctx context.Context, id string) (*model.Album, error) { + if id == r.panicID { + panic("boom") + } + return r.MockAlbumRepo.Get(ctx, id) +} + func adminUserRepo() *tests.MockedUserRepo { repo := tests.CreateMockUserRepo() Expect(repo.Put(GinkgoT().Context(), &model.User{ID: "admin", UserName: "admin", IsAdmin: true})).To(Succeed()) @@ -278,6 +290,36 @@ var _ = Describe("Worker", func() { Expect(err).To(MatchError(model.ErrNotFound), "a timeout must never settle on absent") }) + It("fails an item that panics, without stopping the rest of the batch", func() { + folderRepo.result = []model.Folder{{ + Path: "tests/fixtures/artist/an-album", + ImageFiles: []string{"cover.jpg"}, + }} + albums := tests.CreateMockAlbumRepo() + albums.SetData(model.Albums{ + {ID: "alboom", Name: "Album", FolderIDs: []string{"f1"}}, + {ID: "alok", Name: "Album", FolderIDs: []string{"f1"}}, + }) + ds.MockedAlbum = &panickingAlbumRepo{MockAlbumRepo: albums, panicID: "alboom"} + Expect(queueRepo.Enqueue(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alboom"})).To(Succeed()) + Expect(queueRepo.Enqueue(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alok"})).To(Succeed()) + + n, err := w.drain(ctx, 1) + Expect(err).ToNot(HaveOccurred()) + Expect(n).To(Equal(2)) + + it := findQueued(queueRepo, "al", "alboom") + Expect(it).ToNot(BeNil(), "a panicking item must be rescheduled, not dropped") + Expect(it.Attempts).To(Equal(1)) + Expect(it.RetryAt).To(BeTemporally(">", time.Now())) + Expect(it.Trace).To(ContainSubstring("boom")) + + Expect(findQueued(queueRepo, "al", "alok")).To(BeNil()) + ia, err := artRepo.GetItemArtwork(ctx, model.KindAlbumArtwork, "alok", model.ImageTypePrimary) + Expect(err).ToNot(HaveOccurred()) + Expect(ia.Source).To(Equal("folder")) + }) + It("reschedules past the provider's requested delay when it exceeds the backoff", func() { conf.Server.CoverArtPriority = "external" ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "al9", Name: "Album"}}) diff --git a/core/storage/local/local.go b/core/storage/local/local.go index 686838565..e2ce00a1b 100644 --- a/core/storage/local/local.go +++ b/core/storage/local/local.go @@ -76,6 +76,17 @@ func (lfs *localFS) ResolveSymlink(name string) (string, error) { return filepath.EvalSymlinks(filepath.Join(lfs.root, filepath.FromSlash(name))) } +// ReadLink and Lstat implement fs.ReadLinkFS, so callers can detect symlinks without following them. +var _ fs.ReadLinkFS = (*localFS)(nil) + +func (lfs *localFS) ReadLink(name string) (string, error) { + return fs.ReadLink(lfs.FS, name) +} + +func (lfs *localFS) Lstat(name string) (fs.FileInfo, error) { + return fs.Lstat(lfs.FS, name) +} + func (lfs *localFS) ReadTags(path ...string) (map[string]metadata.Info, error) { res, err := lfs.extractor.Parse(path...) if err != nil { diff --git a/persistence/persistence_suite_test.go b/persistence/persistence_suite_test.go index ee2794454..84fdffa4d 100644 --- a/persistence/persistence_suite_test.go +++ b/persistence/persistence_suite_test.go @@ -352,6 +352,15 @@ func GetDBXBuilder() *dbx.DB { return dbx.NewFromDB(db.Db(), db.Dialect) } +// storedRows reads playlist_tracks without joining media_file, so it also shows rows pointing to no track. +func storedRows(playlistID string) []string { + GinkgoHelper() + var rows []string + Expect(GetDBXBuilder().NewQuery("SELECT id || ':' || media_file_id FROM playlist_tracks WHERE playlist_id = {:id} ORDER BY id"). + Bind(dbx.Params{"id": playlistID}).Column(&rows)).To(Succeed()) + return rows +} + // collectCursor takes the cursor's underlying func type so the named cursor types // (model.AlbumCursor, ...) infer T. func collectCursor[T any](cursor func(func(T, error) bool), err error) []T { diff --git a/persistence/playlist_repository.go b/persistence/playlist_repository.go index 41b75266c..1a1b05a5c 100644 --- a/persistence/playlist_repository.go +++ b/persistence/playlist_repository.go @@ -281,7 +281,7 @@ func (r *playlistRepository) updatePlaylist(ctx context.Context, playlistId stri } // addTracks is the only path that writes playlist_tracks rows (smart playlists aside), so it owns -// the library check: every caller, including a full replace through Put, goes through it. +// the library and existence check: every caller, including a full replace through Put, goes through it. func (r *playlistRepository) addTracks(ctx context.Context, playlistId string, startingPos int, mediaFileIds []string) (int, error) { mediaFileIds, err := r.keepAccessible(ctx, mediaFileIds) if err != nil { @@ -305,12 +305,9 @@ func (r *playlistRepository) addTracks(ctx context.Context, playlistId string, s return len(mediaFileIds), r.refreshCounters(ctx, &model.Playlist{ID: playlistId}) } -// keepAccessible drops ids the caller cannot read, preserving order and duplicates. Chunked -// because callers pass unbounded id lists (M3U import), well past SQLITE_MAX_VARIABLE_NUMBER. +// keepAccessible drops ids the caller cannot read or that match no track, preserving order and duplicates. +// Chunked because callers pass unbounded id lists (M3U import), well past SQLITE_MAX_VARIABLE_NUMBER. func (r *playlistRepository) keepAccessible(ctx context.Context, mediaFileIds []string) ([]string, error) { - if visible, err := r.visibleLibraryIDs(ctx); err == nil && r.userSeesAllLibraries(ctx, visible) { - return mediaFileIds, nil - } accessible := make(map[string]struct{}, len(mediaFileIds)) for chunk := range slices.Chunk(slice.Unique(mediaFileIds), 200) { sq := r.applyLibraryFilter(ctx, Select("id").From("media_file").Where(Eq{"id": chunk}), "media_file") diff --git a/persistence/playlist_repository_test.go b/persistence/playlist_repository_test.go index 93d37c928..b7be0588b 100644 --- a/persistence/playlist_repository_test.go +++ b/persistence/playlist_repository_test.go @@ -469,6 +469,26 @@ var _ = Describe("PlaylistRepository", func() { }) }) + Describe("removeOrphans", func() { + It("deletes rows pointing to no track and renumbers the rest", func() { + pls := model.Playlist{Name: "Orphans", OwnerID: "userid"} + Expect(repo.Put(ctx, &pls)).To(Succeed()) + DeferCleanup(func() { Expect(repo.Delete(ctx, pls.ID)).To(Succeed()) }) + // Rows as an older version stored them, with an unknown id between real tracks + _, err := GetDBXBuilder().NewQuery(`INSERT INTO playlist_tracks (playlist_id, media_file_id, id) + VALUES ({:id}, '1001', 1), ({:id}, 'no-such-track', 2), ({:id}, '1002', 3)`). + Bind(dbx.Params{"id": pls.ID}).Execute() + Expect(err).ToNot(HaveOccurred()) + + Expect(repo.(*playlistRepository).removeOrphans(ctx)).To(Succeed()) + + Expect(storedRows(pls.ID)).To(Equal([]string{"1:1001", "2:1002"})) + got, err := repo.Get(ctx, pls.ID) + Expect(err).ToNot(HaveOccurred()) + Expect(got.SongCount).To(Equal(2)) + }) + }) + // Exists is ctx-sensitive through userFilter, so callers that only want "does it still exist" // -- the public image route serving a share -- must elevate, or a private playlist looks gone. Describe("Exists visibility", func() { diff --git a/persistence/playlist_track_repository.go b/persistence/playlist_track_repository.go index 392446cef..587410305 100644 --- a/persistence/playlist_track_repository.go +++ b/persistence/playlist_track_repository.go @@ -213,7 +213,7 @@ func (r *playlistTrackRepository) Insert(ctx context.Context, mediaFileIds []str if err != nil || inserted == n { return inserted, err } - // The shift above reserved a slot per requested id, so ids dropped by the library filter + // The shift above reserved a slot per requested id, so ids dropped by keepAccessible // leave a hole. Close it. return inserted, r.playlistRepo.renumber(ctx, r.playlistId) } diff --git a/persistence/playlist_track_repository_test.go b/persistence/playlist_track_repository_test.go index 119c57116..29911ebd4 100644 --- a/persistence/playlist_track_repository_test.go +++ b/persistence/playlist_track_repository_test.go @@ -83,12 +83,14 @@ var _ = Describe("PlaylistTrackRepository", func() { Describe("Insert", func() { var tracks model.PlaylistTrackRepository + var plsID string BeforeEach(func() { plsRepo := NewPlaylistRepository(GetDBXBuilder()) pls := model.Playlist{Name: "Insert", OwnerID: "userid", OwnerName: "userid"} Expect(plsRepo.Put(ctx, &pls)).To(Succeed()) DeferCleanup(func() { Expect(plsRepo.Delete(ctx, pls.ID)).To(Succeed()) }) + plsID = pls.ID tracks = plsRepo.Tracks(ctx, pls.ID, false) Expect(tracks.Add(ctx, []string{songDayInALife.ID, songRadioactivity.ID})).To(Equal(2)) @@ -123,6 +125,40 @@ var _ = Describe("PlaylistTrackRepository", func() { Expect(err).ToNot(HaveOccurred()) Expect([]string{all[0].ID, all[1].ID, all[2].ID}).To(Equal([]string{"1", "2", "3"})) }) + + It("drops ids that match no track, keeping positions contiguous", func() { + Expect(tracks.Insert(ctx, []string{"no-such-track", songComeTogether.ID}, 1)).To(Equal(1)) + + Expect(storedRows(plsID)).To(Equal([]string{ + "1:" + songComeTogether.ID, "2:" + songDayInALife.ID, "3:" + songRadioactivity.ID, + })) + }) + }) + + Describe("Add", func() { + var tracks model.PlaylistTrackRepository + var plsID string + + BeforeEach(func() { + plsRepo := NewPlaylistRepository(GetDBXBuilder()) + pls := model.Playlist{Name: "Add", OwnerID: "userid", OwnerName: "userid"} + Expect(plsRepo.Put(ctx, &pls)).To(Succeed()) + DeferCleanup(func() { Expect(plsRepo.Delete(ctx, pls.ID)).To(Succeed()) }) + plsID = pls.ID + tracks = plsRepo.Tracks(ctx, pls.ID, false) + }) + + It("drops ids that match no track and counts only the tracks added", func() { + Expect(tracks.Add(ctx, []string{songDayInALife.ID, "no-such-track", songComeTogether.ID, "!!not-an-id!!"})).To(Equal(2)) + + Expect(storedRows(plsID)).To(Equal([]string{"1:" + songDayInALife.ID, "2:" + songComeTogether.ID})) + }) + + It("adds nothing when no id matches a track", func() { + Expect(tracks.Add(ctx, []string{"no-such-track", "!!not-an-id!!"})).To(BeZero()) + + Expect(storedRows(plsID)).To(BeEmpty()) + }) }) Describe("Reorder", func() { diff --git a/resources/hosting/elfhosted.svg b/resources/hosting/elfhosted.svg new file mode 100644 index 000000000..86bc86df9 --- /dev/null +++ b/resources/hosting/elfhosted.svg @@ -0,0 +1,77 @@ +Deploy on ElfHosted + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +Deploy on ElfHosted diff --git a/resources/hosting/pikapods.svg b/resources/hosting/pikapods.svg new file mode 100644 index 000000000..a3865feea --- /dev/null +++ b/resources/hosting/pikapods.svg @@ -0,0 +1,25 @@ +Run on PikaPods + +Run on PikaPods Button + + + + + + + + + + + + + + + + + + diff --git a/resources/hosting/zenith.svg b/resources/hosting/zenith.svg new file mode 100644 index 000000000..0b5b11539 --- /dev/null +++ b/resources/hosting/zenith.svg @@ -0,0 +1,16 @@ +Deploy with Zenith + + + + + + + + + + + + + + + diff --git a/scanner/walk_dir_tree.go b/scanner/walk_dir_tree.go index 1864a6a44..d39cad5f6 100644 --- a/scanner/walk_dir_tree.go +++ b/scanner/walk_dir_tree.go @@ -43,6 +43,13 @@ func walkDirTree(ctx context.Context, job *scanJob, targetFolders ...string) (<- continue } + // A full walk never descends into symlinked folders when following is disabled, so a + // target reached through one (e.g. a watcher event for a new link) is skipped too. + if !conf.Server.Scanner.FollowSymlinks && isSymlinkedPath(job.fs, folderPath) { + log.Debug(ctx, "Scanner: Skipping symlinked target folder, following is disabled", "path", folderPath) + continue + } + // Create checker and push patterns from root to this folder checker := newIgnoreChecker(job.fs) err = checker.PushAllParents(ctx, folderPath) @@ -225,6 +232,18 @@ func isDirOrSymlinkToDir(fsys fs.FS, baseDir string, dirEnt fs.DirEntry) (bool, return fileInfo.IsDir(), nil } +// isSymlinkedPath returns true if folderPath, or any of its parent folders, is a symbolic link. +// It needs fsys to implement fs.ReadLinkFS, otherwise links are followed and never detected. +func isSymlinkedPath(fsys fs.FS, folderPath string) bool { + for p := path.Clean(folderPath); p != "." && p != "/"; p = path.Dir(p) { + info, err := fs.Lstat(fsys, p) + if err == nil && info.Mode()&fs.ModeSymlink != 0 { + return true + } + } + return false +} + const maxSymlinkHops = 40 // resolveEntryName returns the name to classify the entry by, and whether to diff --git a/scanner/walk_dir_tree_test.go b/scanner/walk_dir_tree_test.go index 43939e5c2..0c2aba3e0 100644 --- a/scanner/walk_dir_tree_test.go +++ b/scanner/walk_dir_tree_test.go @@ -4,8 +4,10 @@ import ( "context" "fmt" "io/fs" + "maps" "os" "path/filepath" + "slices" "testing/fstest" "github.com/navidrome/navidrome/conf" @@ -260,6 +262,44 @@ var _ = Describe("walk_dir_tree", func() { // Folders not in targets should remain in lastUpdates Expect(job.lastUpdates).To(HaveKey(model.FolderID(job.lib, "OtherArtist/Album3"))) }) + + // #6292: a watcher event for a new folder symlink makes the link itself a scan target + Context("symlinked target folders (production local storage FS)", func() { + BeforeEach(func() { + libRoot := GinkgoT().TempDir() + Expect(os.MkdirAll(filepath.Join(libRoot, "Mozart", "Album1"), 0755)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(libRoot, "Mozart", "Album1", "track.mp3"), []byte("AUDIO"), 0600)).To(Succeed()) + Expect(os.Symlink("Mozart", filepath.Join(libRoot, "Wolfgang Amadeus Mozart"))).To(Succeed()) + job = &scanJob{fs: newLocalMusicFS(libRoot), lib: model.Library{Path: libRoot}} + }) + + walkTargets := func(targets ...string) map[string]*folderEntry { + results, err := walkDirTree(ctx, job, targets...) + Expect(err).ToNot(HaveOccurred()) + folders := map[string]*folderEntry{} + for folder := range results { + folders[folder.path] = folder + } + return folders + } + + DescribeTable("with FollowSymlinks disabled", + func(target string, expected ...string) { + conf.Server.Scanner.FollowSymlinks = false + Expect(slices.Collect(maps.Keys(walkTargets(target)))).To(ConsistOf(expected)) + }, + Entry("skips a target that is a symlink", "Wolfgang Amadeus Mozart"), + Entry("skips a target under a symlinked folder", "Wolfgang Amadeus Mozart/Album1"), + Entry("walks a regular target", "Mozart", "Mozart", "Mozart/Album1"), + ) + + It("walks a symlinked target when FollowSymlinks is enabled", func() { + conf.Server.Scanner.FollowSymlinks = true + folders := walkTargets("Wolfgang Amadeus Mozart") + Expect(folders).To(HaveKey("Wolfgang Amadeus Mozart/Album1")) + Expect(folders["Wolfgang Amadeus Mozart/Album1"].audioFiles).To(HaveKey("track.mp3")) + }) + }) }) }) @@ -433,8 +473,8 @@ var _ = Describe("walk_dir_tree", func() { }) // Regression for #5752: the production localFS must resolve file symlinks. - // It wraps os.DirFS behind the fs.FS interface, so fs.ReadLink-based - // resolution is not available and full OS-level resolution is required. + // fs.ReadLink-based resolution can't follow targets outside the library + // root, so full OS-level resolution is required. Context("production local storage FS", func() { var libRoot string var musicFS storage.MusicFS @@ -460,12 +500,7 @@ var _ = Describe("walk_dir_tree", func() { Expect(os.Symlink(filepath.Join(pool, "mid.wav"), filepath.Join(libRoot, "evil.wav"))).To(Succeed()) Expect(os.Symlink(filepath.Join(pool, "missing.mp3"), filepath.Join(libRoot, "broken.mp3"))).To(Succeed()) - u, err := storage.LocalPathToURL(libRoot) - Expect(err).ToNot(HaveOccurred()) - s, err := storage.For(u.String()) - Expect(err).ToNot(HaveOccurred()) - musicFS, err = s.FS() - Expect(err).ToNot(HaveOccurred()) + musicFS = newLocalMusicFS(libRoot) }) walkRoot := func() *folderEntry { @@ -700,6 +735,17 @@ func getDirEntry(baseDir, name string) os.DirEntry { panic(fmt.Sprintf("Could not find %s in %s", name, baseDir)) } +// newLocalMusicFS returns the production local storage MusicFS rooted at libRoot +func newLocalMusicFS(libRoot string) storage.MusicFS { + u, err := storage.LocalPathToURL(libRoot) + Expect(err).ToNot(HaveOccurred()) + s, err := storage.For(u.String()) + Expect(err).ToNot(HaveOccurred()) + musicFS, err := s.FS() + Expect(err).ToNot(HaveOccurred()) + return musicFS +} + // mockMusicFS is a mock implementation of the MusicFS interface that supports symlinks type mockMusicFS struct { storage.MusicFS diff --git a/server/jellyfin/README.md b/server/jellyfin/README.md index 9315ac4f9..48e69cb05 100644 --- a/server/jellyfin/README.md +++ b/server/jellyfin/README.md @@ -158,8 +158,8 @@ and `MusicArtist` queries accept it too, matching artists credited on an album o `Years`; `StudioIds` (record labels, as listed by `GET Studios`); `SearchTerm`; `Filters` (`IsFavorite`, `IsFavoriteOrLikes`, `IsPlayed`, `IsUnplayed`) and the standalone `isFavorite`/`isPlayed` booleans it can also be expressed as — `Filters` wins when both are sent, as -in Jellyfin; `Likes`, `Dislikes`, `IsFolder`, `IsNotFolder` and `IsResumable` have no Navidrome -equivalent and are ignored; `SortBy`/`SortOrder` (every recognized key is applied in order, so secondary keys break ties; +in Jellyfin; `Likes` and `Dislikes` are not supported, and `IsFolder`, `IsNotFolder` and +`IsResumable` have no Navidrome equivalent, so all five are ignored; `SortBy`/`SortOrder` (every recognized key is applied in order, so secondary keys break ties; unrecognized keys are skipped, and `Random` always sorts alone); `StartIndex`/`Limit`; and `Ids` (batch fetch by id). `Recursive=false` with a library `ParentId` returns direct children only (no tracks — no track is a library's direct child). @@ -176,7 +176,7 @@ returns direct children only (no tracks — no track is a library's direct child | Artists / genres / labels | `GET Artists`, `GET Artists/AlbumArtists`, `GET Genres`, `GET MusicGenres`, `GET Studios`, `GET Items/Filters` | | Similar / mixes | `GET Artists/{itemId}/Similar`, `GET Items/{itemId}/Similar`, `GET Albums/{itemId}/Similar`, `GET {Items,Songs,Albums,Artists,Playlists}/{itemId}/InstantMix`, `GET Artists/InstantMix?id=`, `GET MusicGenres/InstantMix?id=` | | Images | `GET Items/{itemId}/Images/{type}[/{index}]` (public), `POST`/`DELETE Items/{itemId}/Images/{type}` (playlist cover, authenticated) | -| Favorites / ratings for songs, albums, artists, and playlists | `POST`/`DELETE UserFavoriteItems/{itemId}`, `POST`/`DELETE Users/{userId}/FavoriteItems/{itemId}`, `POST`/`DELETE Users/{userId}/Items/{itemId}/Rating`, `GET UserItems/{itemId}/UserData`, `GET Users/{userId}/Items/{itemId}/UserData` | +| Favorites / ratings for songs, albums, artists, and playlists | `POST`/`DELETE UserFavoriteItems/{itemId}`, `POST`/`DELETE Users/{userId}/FavoriteItems/{itemId}`, `POST`/`DELETE UserItems/{itemId}/Rating`, `POST`/`DELETE Users/{userId}/Items/{itemId}/Rating`, `GET`/`POST UserItems/{itemId}/UserData`, `GET`/`POST Users/{userId}/Items/{itemId}/UserData` | | Streaming | `GET Audio/{itemId}/stream[.{container}]`, `GET Audio/{itemId}/universal`, `GET Audio/{itemId}/main.m3u8`, `GET Items/{itemId}/File`, `GET Items/{itemId}/Download`, `GET`/`POST Items/{itemId}/PlaybackInfo` (`HEAD` too on stream, universal, File, Download and images; a transcode HEAD answers without starting it) | | Lyrics | `GET Audio/{itemId}/Lyrics` | | Playback reporting | `POST Sessions/Playing`, `POST Sessions/Playing/Progress`, `POST Sessions/Playing/Stopped`, `POST Sessions/Playing/Ping` (no-op), `POST Sessions/Capabilities[/Full]` | @@ -402,12 +402,20 @@ make test PKG=./server/jellyfin/... in the playlist, so ids change when entries are inserted or removed; re-read the list after an edit. Real Jellyfin uses the song id. Clients that echo `PlaylistItemId` back (Finamp) work; clients that send the song id as `EntryIds` or to `Move` (Jellify) get `404`. -- **Rating uses the `rating` param.** `POST Users/{userId}/Items/{itemId}/Rating` reads a `rating` - (0-10) and stores it as 0-5 stars. Jellyfin's `likes` boolean is not read, so a client that sends - only `likes` clears the rating. -- **Missing endpoints.** `UserPlayedItems` (mark played/unplayed), `Search/Hints` and the non-legacy - `UserItems/{itemId}/Rating` are not implemented. Play counts only change through playback - reporting. +- **Ratings are stored as 0-5 stars.** Jellyfin's 0-10 `Rating` is rounded to the nearest star + (any nonzero value keeps at least one), so it reads back as an even number. `likes=true` stores + 5 stars and `likes=false` 1 star, like Jellyfin's 10 and 1. A rating of 0 clears the rating. +- **`POST .../Rating` needs a value.** Jellyfin clears the rating when `likes` is missing; Navidrome + answers `400` and keeps it, as it does for a malformed or out-of-range value. Use + `DELETE .../Rating` to clear. A numeric `rating` param (0-10) is also accepted when `likes` is + absent. +- **`POST .../UserData` writes only `Rating`, `Likes` and `IsFavorite`.** `Played`, `PlayCount`, + `LastPlayedDate` and `PlaybackPositionTicks` are ignored. A body that is not a single JSON + object, or a `Rating` outside 0-10, is a `400` and nothing is written. +- **Annotation writes can't target another user.** Rating and UserData writes whose `userId` names + a different user get `403`, even for admins; Jellyfin lets admins edit other users' data. +- **Missing endpoints.** `UserPlayedItems` (mark played/unplayed) and `Search/Hints` are not + implemented. Play counts only change through playback reporting. - **Some `Fields` are never emitted.** `ProviderIds`, `People`, `Etag` and `DateLastMediaAdded` are accepted but never returned. - **Blurhashes can be missing.** `ImageBlurHashes` carries the real blurhash that the artwork diff --git a/server/jellyfin/annotations.go b/server/jellyfin/annotations.go index a589798be..1bb0b7846 100644 --- a/server/jellyfin/annotations.go +++ b/server/jellyfin/annotations.go @@ -1,47 +1,52 @@ package jellyfin import ( + "encoding/json" "errors" - "math" + "io" "net/http" + "strconv" + "github.com/go-chi/chi/v5" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/server/events" "github.com/navidrome/navidrome/server/jellyfin/dto" - "github.com/navidrome/navidrome/utils/req" ) -// resolveAnnotated finds which annotated repo owns id, returning the resource name used in -// refreshResource events. Albums and songs 404 when the user can't access their library; artists -// span libraries (library_artist), so have no single LibraryID to gate on and rely on list-time -// scoping. PlaylistRepository.Get enforces playlist visibility. When repo is nil the response has -// already been written, so callers must return without writing the annotation. -func (api *Router) resolveAnnotated(w http.ResponseWriter, r *http.Request, id string) (repo model.AnnotatedRepository, resource string) { +type annotatedItem struct { + repo model.AnnotatedRepository + resource string // name used in refreshResource events + ann model.Annotations +} + +// resolveAnnotated finds id's repo and the caller's annotations. Only albums and songs are library-gated +// (artists span libraries; playlists gate in Get). When !ok, the response has already been written. +func (api *Router) resolveAnnotated(w http.ResponseWriter, r *http.Request, id string) (item annotatedItem, ok bool) { ctx := r.Context() entity, err := model.GetEntityByID(ctx, api.ds, id) if err != nil && !errors.Is(err, model.ErrNotFound) { api.internalError(w, r, err) - return nil, "" + return annotatedItem{}, false } u, _ := request.UserFrom(ctx) switch e := entity.(type) { case *model.Album: if u.HasLibraryAccess(e.LibraryID) { - return api.ds.Album(), "album" + return annotatedItem{api.ds.Album(), "album", e.Annotations}, true } case *model.Artist: - return api.ds.Artist(), "artist" + return annotatedItem{api.ds.Artist(), "artist", e.Annotations}, true case *model.MediaFile: if u.HasLibraryAccess(e.LibraryID) { - return api.ds.MediaFile(), "song" + return annotatedItem{api.ds.MediaFile(), "song", e.Annotations}, true } case *model.Playlist: - return api.ds.Playlist(), "playlist" + return annotatedItem{api.ds.Playlist(), "playlist", e.Annotations}, true } // Unknown ids, inaccessible-library items and non-annotatable entities (radios) all read as absent. http.Error(w, "Not Found", http.StatusNotFound) - return nil, "" + return annotatedItem{}, false } // getUserItemData returns the caller's play/favorite/rating state for a single item. Jellify @@ -65,65 +70,135 @@ func (api *Router) getUserItemData(w http.ResponseWriter, r *http.Request) { api.ok(w, r, data) } -func (api *Router) setFavorite(w http.ResponseWriter, r *http.Request, starred bool) { +// userDataChange holds already-validated writes; a nil field leaves that annotation untouched. +type userDataChange struct { + rating *int // Navidrome stars; 0 clears + favorite *bool +} + +// applyUserData writes change for the itemId path param and answers with the item's resulting +// UserData, as Jellyfin does. +func (api *Router) applyUserData(w http.ResponseWriter, r *http.Request, change userDataChange) { id, ok := itemIDParam(w, r, "itemId") if !ok { return } - repo, resource := api.resolveAnnotated(w, r, id) - if repo == nil { + item, ok := api.resolveAnnotated(w, r, id) + if !ok { return } - if err := repo.SetStar(r.Context(), starred, id); err != nil { - api.internalError(w, r, err) - return + ctx := r.Context() + if change.rating != nil { + if err := item.repo.SetRating(ctx, *change.rating, id); err != nil { + api.internalError(w, r, err) + return + } + item.ann.Rating = *change.rating } - api.broker.SendMessage(r.Context(), (&events.RefreshResource{}).With(resource, id)) - encodedID := dto.EncodeID(id) - api.ok(w, r, &dto.UserItemDataDto{IsFavorite: starred, Key: encodedID, ItemId: encodedID}) + if change.favorite != nil { + if err := item.repo.SetStar(ctx, *change.favorite, id); err != nil { + api.internalError(w, r, err) + return + } + item.ann.Starred = *change.favorite + } + if change.rating != nil || change.favorite != nil { + api.broker.SendMessage(ctx, (&events.RefreshResource{}).With(item.resource, id)) + } + api.ok(w, r, dto.UserData(item.ann, id)) +} + +// applyCallerUserData is applyUserData for routes that may name a userId: only the caller's own +// annotations can be written, so naming another user is refused. Undecodable ids are ignored. +func (api *Router) applyCallerUserData(w http.ResponseWriter, r *http.Request, change userDataChange) { + raw := chi.URLParam(r, "userId") + if raw == "" { + raw = r.URL.Query().Get("userid") + } + if target, ok := dto.DecodeID(raw); ok { + if u, _ := request.UserFrom(r.Context()); target != u.ID { + http.Error(w, "Forbidden", http.StatusForbidden) + return + } + } + api.applyUserData(w, r, change) +} + +func (api *Router) markFavorite(w http.ResponseWriter, r *http.Request) { + api.applyUserData(w, r, userDataChange{favorite: new(true)}) } -func (api *Router) markFavorite(w http.ResponseWriter, r *http.Request) { api.setFavorite(w, r, true) } func (api *Router) unmarkFavorite(w http.ResponseWriter, r *http.Request) { - api.setFavorite(w, r, false) + api.applyUserData(w, r, userDataChange{favorite: new(false)}) } -func (api *Router) setItemRating(w http.ResponseWriter, r *http.Request, rating int) { - id, ok := itemIDParam(w, r, "itemId") - if !ok { - return - } - repo, resource := api.resolveAnnotated(w, r, id) - if repo == nil { - return - } - if err := repo.SetRating(r.Context(), rating, id); err != nil { - api.internalError(w, r, err) - return - } - api.broker.SendMessage(r.Context(), (&events.RefreshResource{}).With(resource, id)) - encodedID := dto.EncodeID(id) - d := &dto.UserItemDataDto{Key: encodedID, ItemId: encodedID} - if rating > 0 { - jfRating := float64(rating) * 2 // Navidrome 0-5 -> Jellyfin 0-10, mirrors dto.UserData - d.Rating = &jfRating - } - api.ok(w, r, d) -} - -// setRating maps Jellyfin's 0-10 rating (a nullable double, so fractional values are valid) to -// Navidrome's 0-5 stars. A nonzero rating floors at one star: rounding to 0 would clear it, since -// SetRating(0) is the delete path. +// setRating reads Jellyfin's likes flag, or the numeric rating extension older clients send. Unlike +// Jellyfin, a missing or malformed value is a 400 rather than a clear: clearing is DELETE's job. func (api *Router) setRating(w http.ResponseWriter, r *http.Request) { - jfRating := req.Params(r).Float64Or("rating", 0) - jfRating = min(max(jfRating, 0), 10) // clamp: a client sending e.g. Rating=100 must not write an out-of-domain rating - rating := int(math.Round(jfRating / 2)) - if jfRating > 0 { - rating = max(rating, 1) + q := r.URL.Query() + var stars int + switch { + case q.Has("likes"): + likes, err := strconv.ParseBool(q.Get("likes")) + if err != nil { + http.Error(w, "likes must be true or false", http.StatusBadRequest) + return + } + stars = dto.StarsFromLikes(likes) + case q.Has("rating"): + rating, err := strconv.ParseFloat(q.Get("rating"), 64) + s, ok := dto.StarsFromRating(rating) + if err != nil || !ok { + http.Error(w, "rating must be a number from 0 to 10", http.StatusBadRequest) + return + } + stars = s + default: + http.Error(w, "likes is required", http.StatusBadRequest) + return } - api.setItemRating(w, r, rating) + api.applyCallerUserData(w, r, userDataChange{rating: &stars}) } func (api *Router) removeRating(w http.ResponseWriter, r *http.Request) { - api.setItemRating(w, r, 0) + api.applyCallerUserData(w, r, userDataChange{rating: new(0)}) +} + +// updateUserItemData writes Rating, Likes and IsFavorite; omitted or null fields are left untouched. +// Play state is ignored because it only changes through playback reporting. +func (api *Router) updateUserItemData(w http.ResponseWriter, r *http.Request) { + body, ok := decodeUserDataBody(w, r) + if !ok { + return + } + change := userDataChange{favorite: body.IsFavorite} + // Jellyfin applies Likes before Rating, so Rating wins when a body carries both. + switch { + case body.Rating != nil: + stars, ok := dto.StarsFromRating(*body.Rating) + if !ok { + http.Error(w, "Rating must be from 0 to 10", http.StatusBadRequest) + return + } + change.rating = &stars + case body.Likes != nil: + change.rating = new(dto.StarsFromLikes(*body.Likes)) + } + api.applyCallerUserData(w, r, change) +} + +const maxUserDataBody = 64 << 10 + +// decodeUserDataBody accepts exactly one JSON object; Decode alone would ignore anything after it. +func decodeUserDataBody(w http.ResponseWriter, r *http.Request) (*dto.UpdateUserItemDataDto, bool) { + dec := json.NewDecoder(http.MaxBytesReader(w, r.Body, maxUserDataBody)) + var body *dto.UpdateUserItemDataDto + err := dec.Decode(&body) + if err == nil && body != nil { + if _, err = dec.Token(); errors.Is(err, io.EOF) { + return body, true + } + } + http.Error(w, "Bad Request", http.StatusBadRequest) + return nil, false } diff --git a/server/jellyfin/annotations_test.go b/server/jellyfin/annotations_test.go index a436503ba..69d73ed05 100644 --- a/server/jellyfin/annotations_test.go +++ b/server/jellyfin/annotations_test.go @@ -5,6 +5,8 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "strings" + "time" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" @@ -21,7 +23,7 @@ var _ = Describe("Annotations", func() { var broker *fakeEventBroker // alice has access to library 1 only. ctxUser := func() context.Context { - return request.WithUser(context.Background(), model.User{ID: testID("u1"), UserName: "alice", Libraries: model.Libraries{{ID: 1, Name: "Music"}}}) + return request.WithUser(GinkgoT().Context(), model.User{ID: testID("u1"), UserName: "alice", Libraries: model.Libraries{{ID: 1, Name: "Music"}}}) } BeforeEach(func() { @@ -30,6 +32,16 @@ var _ = Describe("Annotations", func() { api = &Router{ds: ds, broker: broker} }) + songWith := func(ann model.Annotations) *tests.MockMediaFileRepo { + mfRepo := ds.MediaFile().(*tests.MockMediaFileRepo) + mfRepo.SetData(model.MediaFiles{{ID: testID("s1"), Title: "Song", LibraryID: 1, Annotations: ann}}) + return mfRepo + } + songReq := func(method, target, body string, params ...string) *http.Request { + r := httptest.NewRequest(method, target, strings.NewReader(body)).WithContext(ctxUser()) + return withChiURLParam(r, append([]string{"itemId", dto.EncodeID(testID("s1"))}, params...)...) + } + Describe("markFavorite / unmarkFavorite", func() { It("stars a song and returns IsFavorite=true", func() { mfRepo := ds.MediaFile().(*tests.MockMediaFileRepo) @@ -268,41 +280,275 @@ var _ = Describe("Annotations", func() { Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(4)) }) - It("clamps a Rating above 10 to Navidrome's max (5)", func() { - mfRepo := ds.MediaFile().(*tests.MockMediaFileRepo) - mfRepo.SetData(model.MediaFiles{{ID: testID("s1"), Title: "Song", LibraryID: 1}}) - w := httptest.NewRecorder() - r := httptest.NewRequest("POST", "/Users/u1/Items/s1/Rating?Rating=100", nil).WithContext(ctxUser()) - r = withChiURLParam(r, "itemId", dto.EncodeID(testID("s1"))) - invoke(api.setRating, w, r) - Expect(w.Code).To(Equal(http.StatusOK)) - Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(5)) - }) - - It("clamps a negative Rating to Navidrome's min (0)", func() { - mfRepo := ds.MediaFile().(*tests.MockMediaFileRepo) - mfRepo.SetData(model.MediaFiles{{ID: testID("s1"), Title: "Song", LibraryID: 1}}) - w := httptest.NewRecorder() - r := httptest.NewRequest("POST", "/Users/u1/Items/s1/Rating?Rating=-5", nil).WithContext(ctxUser()) - r = withChiURLParam(r, "itemId", dto.EncodeID(testID("s1"))) - invoke(api.setRating, w, r) - Expect(w.Code).To(Equal(http.StatusOK)) - Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(0)) - }) - It("emits a refreshResource event when rating a song", func() { - mfRepo := ds.MediaFile().(*tests.MockMediaFileRepo) - mfRepo.SetData(model.MediaFiles{{ID: testID("s1"), Title: "Song", LibraryID: 1}}) - w := httptest.NewRecorder() - r := httptest.NewRequest("POST", "/Users/u1/Items/s1/Rating?Rating=8", nil).WithContext(ctxUser()) - r = withChiURLParam(r, "itemId", dto.EncodeID(testID("s1"))) - invoke(api.setRating, w, r) + songWith(model.Annotations{}) + invoke(api.setRating, httptest.NewRecorder(), songReq("POST", "/Users/u1/Items/s1/Rating?Rating=8", "")) Expect(broker.Events).To(HaveLen(1)) Expect(broker.Events[0].Data(broker.Events[0])).To(Equal(`{"song":["` + testID("s1") + `"]}`)) }) + + Context("Jellyfin likes semantics", func() { + It("stores likes=true as 5 stars and reports Rating 10 / Likes true", func() { + mfRepo := songWith(model.Annotations{}) + w := httptest.NewRecorder() + invoke(api.setRating, w, songReq("POST", "/UserItems/s1/Rating?likes=true", "")) + Expect(w.Code).To(Equal(http.StatusOK)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(5)) + d := parseUserData(w) + Expect(*d.Rating).To(Equal(10.0)) + Expect(*d.Likes).To(BeTrue()) + }) + + It("stores likes=false as 1 star (Jellyfin's dislike is Rating 1), not a clear", func() { + mfRepo := songWith(model.Annotations{Rating: 4}) + w := httptest.NewRecorder() + invoke(api.setRating, w, songReq("POST", "/UserItems/s1/Rating?Likes=False", "")) + Expect(w.Code).To(Equal(http.StatusOK)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(1)) + d := parseUserData(w) + Expect(*d.Rating).To(Equal(2.0)) + Expect(*d.Likes).To(BeFalse()) + }) + + It("prefers likes over the numeric rating extension when both are sent", func() { + mfRepo := songWith(model.Annotations{}) + w := httptest.NewRecorder() + invoke(api.setRating, w, songReq("POST", "/UserItems/s1/Rating?likes=false&rating=10", "")) + Expect(w.Code).To(Equal(http.StatusOK)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(1)) + }) + + It("still clears on an explicit rating=0 from numeric clients", func() { + mfRepo := songWith(model.Annotations{Rating: 4}) + w := httptest.NewRecorder() + invoke(api.setRating, w, songReq("POST", "/UserItems/s1/Rating?rating=0", "")) + Expect(w.Code).To(Equal(http.StatusOK)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(0)) + Expect(parseUserData(w).Rating).To(BeNil()) + }) + + It("keeps the favorite and play state and reports them in the response", func() { + playDate := time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) + mfRepo := songWith(model.Annotations{Starred: true, PlayCount: 3, PlayDate: &playDate}) + w := httptest.NewRecorder() + invoke(api.setRating, w, songReq("POST", "/UserItems/s1/Rating?likes=true", "")) + Expect(w.Code).To(Equal(http.StatusOK)) + song := mfRepo.Data[testID("s1")] + Expect(song.Starred).To(BeTrue()) + Expect(song.PlayCount).To(Equal(int64(3))) + Expect(song.PlayDate).To(Equal(&playDate)) + d := parseUserData(w) + Expect(d.IsFavorite).To(BeTrue()) + Expect(d.PlayCount).To(Equal(3)) + Expect(d.Played).To(BeTrue()) + }) + + DescribeTable("rejects a missing or malformed value without touching the stored rating", + func(query string) { + mfRepo := songWith(model.Annotations{Rating: 4}) + w := httptest.NewRecorder() + invoke(api.setRating, w, songReq("POST", "/UserItems/s1/Rating"+query, "")) + Expect(w.Code).To(Equal(http.StatusBadRequest)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(4)) + Expect(broker.Events).To(BeEmpty()) + }, + Entry("no params (Jellyfin would clear)", ""), + Entry("unrelated params only", "?userId=x&foo=1"), + Entry("empty likes", "?likes="), + Entry("non-boolean likes", "?likes=maybe"), + Entry("non-boolean likes even with a valid rating", "?likes=maybe&rating=8"), + Entry("empty rating", "?rating="), + Entry("non-numeric rating", "?rating=abc"), + Entry("NaN rating", "?rating=NaN"), + Entry("infinite rating", "?rating=Inf"), + Entry("rating above 10", "?Rating=100"), + Entry("negative rating", "?Rating=-5"), + ) + }) + + Context("authorization", func() { + DescribeTable("refuses a userId naming another user and leaves the rating", + func(method, target string, params ...string) { + handler := api.setRating + if method == "DELETE" { + handler = api.removeRating + } + mfRepo := songWith(model.Annotations{Rating: 4}) + w := httptest.NewRecorder() + invoke(handler, w, songReq(method, target, "", params...)) + Expect(w.Code).To(Equal(http.StatusForbidden)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(4)) + Expect(broker.Events).To(BeEmpty()) + }, + Entry("path userId", "POST", "/Users/u2/Items/s1/Rating?likes=true", "userId", dto.EncodeID(testID("u2"))), + Entry("query userId on the /UserItems form", "POST", "/UserItems/s1/Rating?likes=true&userId="+dto.EncodeID(testID("u2"))), + Entry("clearing", "DELETE", "/Users/u2/Items/s1/Rating", "userId", dto.EncodeID(testID("u2"))), + ) + + It("accepts the caller's own userId", func() { + mfRepo := songWith(model.Annotations{}) + w := httptest.NewRecorder() + invoke(api.setRating, w, songReq("POST", "/Users/u1/Items/s1/Rating?likes=true", "", "userId", dto.EncodeID(testID("u1")))) + Expect(w.Code).To(Equal(http.StatusOK)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(5)) + }) + }) + }) + + Describe("updateUserItemData", func() { + post := func(body string, params ...string) *httptest.ResponseRecorder { + w := httptest.NewRecorder() + invoke(api.updateUserItemData, w, songReq("POST", "/UserItems/s1/UserData", body, params...)) + return w + } + + It("maps a 0-10 Rating to stars and answers with the full UserData", func() { + mfRepo := songWith(model.Annotations{Starred: true, PlayCount: 2}) + w := post(`{"Rating":8}`) + Expect(w.Code).To(Equal(http.StatusOK)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(4)) + d := parseUserData(w) + Expect(*d.Rating).To(Equal(8.0)) + Expect(*d.Likes).To(BeTrue()) + Expect(d.IsFavorite).To(BeTrue()) + Expect(d.PlayCount).To(Equal(2)) + Expect(d.ItemId).To(Equal(dto.EncodeID(testID("s1")))) + Expect(broker.Events).To(HaveLen(1)) + }) + + DescribeTable("stores the personal rating", + func(body string, before, expected int) { + mfRepo := songWith(model.Annotations{Rating: before}) + Expect(post(body).Code).To(Equal(http.StatusOK)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(expected)) + }, + Entry("Likes true", `{"Likes":true}`, 0, 5), + Entry("Likes false (Rating 1)", `{"Likes":false}`, 4, 1), + Entry("Rating wins over Likes, as Jellyfin applies Likes first", `{"Likes":true,"Rating":2}`, 0, 1), + Entry("fractional Rating rounds to the nearest star", `{"Rating":7.5}`, 0, 4), + Entry("small nonzero Rating floors at one star", `{"Rating":0.4}`, 0, 1), + Entry("Rating 0 clears", `{"Rating":0}`, 4, 0), + ) + + DescribeTable("leaves the rating untouched when Rating and Likes are omitted or null", + func(body string, wantEvents int) { + mfRepo := songWith(model.Annotations{Rating: 4}) + w := post(body) + Expect(w.Code).To(Equal(http.StatusOK)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(4)) + Expect(*parseUserData(w).Rating).To(Equal(8.0)) + Expect(broker.Events).To(HaveLen(wantEvents)) + }, + Entry("empty object writes nothing", `{}`, 0), + Entry("explicit nulls write nothing", `{"Rating":null,"Likes":null,"IsFavorite":null}`, 0), + Entry("favorite only", `{"IsFavorite":true}`, 1), + ) + + It("sets and unsets the favorite without touching the rating", func() { + mfRepo := songWith(model.Annotations{Rating: 4}) + Expect(post(`{"IsFavorite":true}`).Code).To(Equal(http.StatusOK)) + Expect(mfRepo.Data[testID("s1")].Starred).To(BeTrue()) + Expect(post(`{"IsFavorite":false}`).Code).To(Equal(http.StatusOK)) + Expect(mfRepo.Data[testID("s1")].Starred).To(BeFalse()) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(4)) + }) + + It("ignores play-state fields, which only change through playback reporting", func() { + mfRepo := songWith(model.Annotations{PlayCount: 2}) + w := post(`{"Played":true,"PlayCount":99,"LastPlayedDate":"2026-01-01T00:00:00Z","PlaybackPositionTicks":5}`) + Expect(w.Code).To(Equal(http.StatusOK)) + song := mfRepo.Data[testID("s1")] + Expect(song.PlayCount).To(Equal(int64(2))) + Expect(song.PlayDate).To(BeNil()) + Expect(broker.Events).To(BeEmpty()) + }) + + It("leaves state unchanged when a client echoes back the UserData it read", func() { + mfRepo := songWith(model.Annotations{Rating: 3, Starred: true, PlayCount: 1}) + echo, err := json.Marshal(dto.UserData(mfRepo.Data[testID("s1")].Annotations, testID("s1"))) + Expect(err).ToNot(HaveOccurred()) + Expect(post(string(echo)).Code).To(Equal(http.StatusOK)) + song := mfRepo.Data[testID("s1")] + Expect(song.Rating).To(Equal(3)) + Expect(song.Starred).To(BeTrue()) + Expect(song.PlayCount).To(Equal(int64(1))) + }) + + DescribeTable("rejects an invalid body without writing anything", + func(body string) { + mfRepo := songWith(model.Annotations{Rating: 4}) + Expect(post(body).Code).To(Equal(http.StatusBadRequest)) + song := mfRepo.Data[testID("s1")] + Expect(song.Rating).To(Equal(4)) + Expect(song.Starred).To(BeFalse()) + Expect(broker.Events).To(BeEmpty()) + }, + Entry("empty body", ""), + Entry("malformed JSON", `{"Rating":`), + Entry("Rating above 10, even with a valid IsFavorite", `{"IsFavorite":true,"Rating":10.5}`), + Entry("negative Rating", `{"IsFavorite":true,"Rating":-1}`), + Entry("Rating as a string", `{"Rating":"8"}`), + Entry("Likes as a string", `{"Likes":"yes"}`), + Entry("IsFavorite as a number", `{"IsFavorite":1}`), + Entry("JSON null", `null`), + Entry("JSON array", `[]`), + Entry("trailing garbage after the object", `{"Rating":8} x`), + Entry("a second JSON value after the object", `{"Rating":8}{"Rating":2}`), + Entry("a second object after whitespace", "{\"Rating\":8}\n{}"), + ) + + It("accepts trailing whitespace after the object", func() { + mfRepo := songWith(model.Annotations{}) + Expect(post("{\"Rating\":8}\r\n\t ").Code).To(Equal(http.StatusOK)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(4)) + }) + + It("rejects a body larger than the limit", func() { + mfRepo := songWith(model.Annotations{Rating: 4}) + big := `{"Rating":8,"Pad":"` + strings.Repeat("x", 70<<10) + `"}` + Expect(post(big).Code).To(Equal(http.StatusBadRequest)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(4)) + }) + + Context("authorization", func() { + It("refuses a userId naming another user without writing", func() { + mfRepo := songWith(model.Annotations{Rating: 4}) + w := post(`{"Rating":10,"IsFavorite":true}`, "userId", dto.EncodeID(testID("u2"))) + Expect(w.Code).To(Equal(http.StatusForbidden)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(4)) + Expect(mfRepo.Data[testID("s1")].Starred).To(BeFalse()) + }) + + It("accepts the caller's own userId", func() { + songWith(model.Annotations{}) + Expect(post(`{"Rating":10}`, "userId", dto.EncodeID(testID("u1"))).Code).To(Equal(http.StatusOK)) + }) + + It("returns 404 and writes nothing for an album in a library the user can't access", func() { + albumRepo := ds.Album().(*tests.MockAlbumRepo) + albumRepo.SetData(model.Albums{{ID: testID("a1"), Name: "One", LibraryID: 2}}) + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", "/UserItems/a1/UserData", strings.NewReader(`{"Rating":10,"IsFavorite":true}`)).WithContext(ctxUser()) + invoke(api.updateUserItemData, w, withChiURLParam(r, "itemId", dto.EncodeID(testID("a1")))) + Expect(w.Code).To(Equal(http.StatusNotFound)) + Expect(albumRepo.Data[testID("a1")].Rating).To(Equal(0)) + Expect(albumRepo.Data[testID("a1")].Starred).To(BeFalse()) + }) + + It("returns 404 for an unknown item", func() { + w := post(`{"Rating":10}`) + Expect(w.Code).To(Equal(http.StatusNotFound)) + }) + }) }) }) +func parseUserData(w *httptest.ResponseRecorder) dto.UserItemDataDto { + var d dto.UserItemDataDto + Expect(json.Unmarshal(w.Body.Bytes(), &d)).To(Succeed()) + return d +} + type fakeEventBroker struct { http.Handler Events []events.Event diff --git a/server/jellyfin/api.go b/server/jellyfin/api.go index 4a471d1f0..b9fd32150 100644 --- a/server/jellyfin/api.go +++ b/server/jellyfin/api.go @@ -146,6 +146,8 @@ func (api *Router) routes() http.Handler { r.Delete("/userfavoriteitems/{itemId}", api.unmarkFavorite) r.Post("/users/{userId}/favoriteitems/{itemId}", api.markFavorite) r.Delete("/users/{userId}/favoriteitems/{itemId}", api.unmarkFavorite) + r.Post("/useritems/{itemId}/rating", api.setRating) + r.Delete("/useritems/{itemId}/rating", api.removeRating) r.Post("/users/{userId}/items/{itemId}/rating", api.setRating) r.Delete("/users/{userId}/items/{itemId}/rating", api.removeRating) @@ -153,6 +155,8 @@ func (api *Router) routes() http.Handler { // /Users/{userId}/Items is the legacy spelling. r.Get("/useritems/{itemId}/userdata", api.getUserItemData) r.Get("/users/{userId}/items/{itemId}/userdata", api.getUserItemData) + r.Post("/useritems/{itemId}/userdata", api.updateUserItemData) + r.Post("/users/{userId}/items/{itemId}/userdata", api.updateUserItemData) r.Get("/artists/{itemId}/similar", api.getSimilarArtists) r.Get("/items/{itemId}/similar", api.getSimilarItems) diff --git a/server/jellyfin/dto/dto.go b/server/jellyfin/dto/dto.go index 4a28eb313..318011d89 100644 --- a/server/jellyfin/dto/dto.go +++ b/server/jellyfin/dto/dto.go @@ -40,6 +40,14 @@ type UserItemDataDto struct { Key string `json:"Key"` ItemId string `json:"ItemId,omitempty"` LastPlayedDate *string `json:"LastPlayedDate,omitempty"` + Likes *bool `json:"Likes,omitempty"` +} + +// UpdateUserItemDataDto is the POST .../UserData body; other Jellyfin fields are accepted and ignored. +type UpdateUserItemDataDto struct { + Rating *float64 `json:"Rating"` + Likes *bool `json:"Likes"` + IsFavorite *bool `json:"IsFavorite"` } type BaseItemDto struct { diff --git a/server/jellyfin/dto/mappers.go b/server/jellyfin/dto/mappers.go index fc7e329de..620df783f 100644 --- a/server/jellyfin/dto/mappers.go +++ b/server/jellyfin/dto/mappers.go @@ -3,6 +3,7 @@ package dto import ( "cmp" "fmt" + "math" "time" "github.com/navidrome/navidrome/conf" @@ -121,6 +122,30 @@ func MediaSourceFromMediaFile(mf model.MediaFile) MediaSourceInfo { } } +// minLikeRating is Jellyfin's UserItemData.MinLikeValue: a rating at or above it reads as Likes. +const minLikeRating = 6.5 + +// StarsFromRating maps Jellyfin's 0-10 rating to 0-5 stars, rejecting out-of-range values and NaN. +// A nonzero rating floors at one star, since 0 stars means "not rated". +func StarsFromRating(rating float64) (int, bool) { + if !(rating >= 0 && rating <= 10) { + return 0, false + } + stars := int(math.Round(rating / 2)) + if rating > 0 { + stars = max(stars, 1) + } + return stars, true +} + +// StarsFromLikes mirrors Jellyfin, which stores likes as Rating 10 and dislikes as Rating 1. +func StarsFromLikes(likes bool) int { + if likes { + return 5 + } + return 1 +} + func UserData(a model.Annotations, itemID string) *UserItemDataDto { // Callers pass the raw model id; encode here so Key/ItemId match the encoded Id on the BaseItemDto. encodedID := EncodeID(itemID) @@ -134,6 +159,7 @@ func UserData(a model.Annotations, itemID string) *UserItemDataDto { if a.Rating > 0 { r := float64(a.Rating) * 2 // Navidrome 0-5 -> Jellyfin 0-10 d.Rating = &r + d.Likes = new(r >= minLikeRating) } if s := JellyfinDate(a.PlayDate); s != "" { d.LastPlayedDate = &s diff --git a/server/jellyfin/dto/mappers_test.go b/server/jellyfin/dto/mappers_test.go index 259904673..3e4ba941f 100644 --- a/server/jellyfin/dto/mappers_test.go +++ b/server/jellyfin/dto/mappers_test.go @@ -478,6 +478,30 @@ var _ = Describe("mappers", func() { Expect(item.ImageBlurHashes).To(BeNil()) }) + DescribeTable("UserData derives Likes from the rating like Jellyfin (rating >= 6.5)", + func(stars int, expected *bool) { + d := UserData(model.Annotations{Rating: stars}, testID("s1")) + Expect(d.Likes).To(Equal(expected)) + }, + Entry("unrated has no Likes", 0, nil), + Entry("1 star (Jellyfin 2) dislikes", 1, new(false)), + Entry("3 stars (Jellyfin 6) dislikes", 3, new(false)), + Entry("4 stars (Jellyfin 8) likes", 4, new(true)), + Entry("5 stars (Jellyfin 10) likes", 5, new(true)), + ) + + It("emits Rating as a whole number and omits Likes when unrated", func() { + rated, err := json.Marshal(UserData(model.Annotations{Rating: 4}, testID("s1"))) + Expect(err).ToNot(HaveOccurred()) + Expect(string(rated)).To(ContainSubstring(`"Rating":8,`)) + Expect(string(rated)).To(ContainSubstring(`"Likes":true`)) + + unrated, err := json.Marshal(UserData(model.Annotations{}, testID("s1"))) + Expect(err).ToNot(HaveOccurred()) + Expect(string(unrated)).ToNot(ContainSubstring(`"Rating"`)) + Expect(string(unrated)).ToNot(ContainSubstring(`"Likes"`)) + }) + It("emits DateCreated and SortName on playlists, like albums and artists", func() { p := model.Playlist{ID: testID("pl-1"), Name: "Chill", CreatedAt: time.Date(2026, 7, 1, 12, 0, 0, 0, time.UTC)} Expect(PlaylistToBaseItem(p, nil).DateCreated).To(Equal("2026-07-01T12:00:00.0000000Z")) diff --git a/server/jellyfin/e2e/annotations_test.go b/server/jellyfin/e2e/annotations_test.go index 182fd5436..014751f6e 100644 --- a/server/jellyfin/e2e/annotations_test.go +++ b/server/jellyfin/e2e/annotations_test.go @@ -130,13 +130,78 @@ var _ = Describe("Annotations", func() { Expect(*itemUserData(id).Rating).To(Equal(float64(8))) }) - It("clamps an out-of-range rating to the valid domain", func() { + It("rejects an out-of-range rating and keeps the stored one", func() { id := albumID("Help!") + Expect(post("/Users/admin-1/Items/"+enc(id)+"/Rating?Rating=8", "").Code).To(Equal(http.StatusOK)) + Expect(post("/Users/admin-1/Items/"+enc(id)+"/Rating?Rating=100", "").Code).To(Equal(http.StatusBadRequest)) + Expect(*itemUserData(id).Rating).To(Equal(float64(8))) + }) + + It("stores Jellyfin likes on the current /UserItems route instead of clearing", func() { + id := songID("So What") + var liked dto.UserItemDataDto + parseInto(post("/UserItems/"+enc(id)+"/Rating?likes=true", ""), &liked) + Expect(*liked.Rating).To(Equal(float64(10))) + Expect(*liked.Likes).To(BeTrue()) + + var disliked dto.UserItemDataDto + parseInto(post("/Users/admin-1/Items/"+enc(id)+"/Rating?likes=false", ""), &disliked) + Expect(*disliked.Rating).To(Equal(float64(2))) + Expect(*disliked.Likes).To(BeFalse()) + Expect(*itemUserData(id).Likes).To(BeFalse()) + + Expect(del("/UserItems/" + enc(id) + "/Rating").Code).To(Equal(http.StatusOK)) + Expect(itemUserData(id).Rating).To(BeNil()) + Expect(itemUserData(id).Likes).To(BeNil()) + }) + + It("does not clear a rating when the request has no value", func() { + id := songID("So What") + Expect(post("/UserItems/"+enc(id)+"/Rating?likes=true", "").Code).To(Equal(http.StatusOK)) + Expect(post("/UserItems/"+enc(id)+"/Rating", "").Code).To(Equal(http.StatusBadRequest)) + Expect(post("/UserItems/"+enc(id)+"/Rating?likes=sure", "").Code).To(Equal(http.StatusBadRequest)) + Expect(*itemUserData(id).Rating).To(Equal(float64(10))) + }) + }) + + Describe("POST /UserItems/{id}/UserData", func() { + It("sets Rating, Likes and IsFavorite on both route spellings", func() { + id := songID("Come Together") var data dto.UserItemDataDto - parseInto(post("/Users/admin-1/Items/"+enc(id)+"/Rating?Rating=100", ""), &data) - // 100 clamps to 10 (Jellyfin) -> 5 (Navidrome) -> 10 back out. - Expect(data.Rating).ToNot(BeNil()) - Expect(*data.Rating).To(Equal(float64(10))) + parseInto(post("/UserItems/"+enc(id)+"/UserData", `{"Rating":6}`), &data) + Expect(*data.Rating).To(Equal(float64(6))) + Expect(*data.Likes).To(BeFalse()) + + parseInto(post("/Users/"+enc(adminUser.ID)+"/Items/"+enc(id)+"/UserData", `{"Likes":true,"IsFavorite":true}`), &data) + stored := itemUserData(id) + Expect(*stored.Rating).To(Equal(float64(10))) + Expect(*stored.Likes).To(BeTrue()) + Expect(stored.IsFavorite).To(BeTrue()) + }) + + It("leaves omitted fields and play state untouched", func() { + id := songID("Come Together") + Expect(post("/UserItems/"+enc(id)+"/UserData", `{"Rating":8,"IsFavorite":true}`).Code).To(Equal(http.StatusOK)) + before := itemUserData(id) + + Expect(post("/UserItems/"+enc(id)+"/UserData", `{"Played":true,"PlayCount":42,"LastPlayedDate":"2026-01-01T00:00:00Z"}`).Code).To(Equal(http.StatusOK)) + Expect(post("/UserItems/"+enc(id)+"/UserData", `{"IsFavorite":false}`).Code).To(Equal(http.StatusOK)) + + after := itemUserData(id) + Expect(*after.Rating).To(Equal(float64(8))) + Expect(after.IsFavorite).To(BeFalse()) + Expect(after.PlayCount).To(Equal(before.PlayCount)) + Expect(after.Played).To(Equal(before.Played)) + Expect(after.LastPlayedDate).To(Equal(before.LastPlayedDate)) + }) + + It("rejects invalid bodies without writing", func() { + id := songID("Come Together") + Expect(post("/UserItems/"+enc(id)+"/UserData", `{"Rating":11,"IsFavorite":true}`).Code).To(Equal(http.StatusBadRequest)) + Expect(post("/UserItems/"+enc(id)+"/UserData", `{"Rating":8} {"Rating":2}`).Code).To(Equal(http.StatusBadRequest)) + stored := itemUserData(id) + Expect(stored.Rating).To(BeNil()) + Expect(stored.IsFavorite).To(BeFalse()) }) }) }) diff --git a/server/jellyfin/e2e/multiuser_test.go b/server/jellyfin/e2e/multiuser_test.go index 905d4e16f..f4f2c43d2 100644 --- a/server/jellyfin/e2e/multiuser_test.go +++ b/server/jellyfin/e2e/multiuser_test.go @@ -4,6 +4,7 @@ import ( "net/http" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/server/jellyfin/dto" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -28,6 +29,36 @@ var _ = Describe("Multi-user access control", func() { }) }) + Describe("personal ratings", func() { + userData := func(u model.User, id string) *dto.UserItemDataDto { + var data dto.UserItemDataDto + parseInto(getAs(u, "/UserItems/"+enc(id)+"/UserData"), &data) + return &data + } + + It("keeps each user's rating separate", func() { + id := songID("So What") + Expect(post("/UserItems/"+enc(id)+"/UserData", `{"Rating":10}`).Code).To(Equal(http.StatusOK)) + Expect(postAs(regularUser, "/UserItems/"+enc(id)+"/Rating?likes=false", "").Code).To(Equal(http.StatusOK)) + + Expect(*userData(adminUser, id).Rating).To(Equal(float64(10))) + Expect(*userData(regularUser, id).Rating).To(Equal(float64(2))) + }) + + It("refuses writes that name another user, even for an admin, and changes nothing", func() { + id := songID("So What") + Expect(postAs(regularUser, "/UserItems/"+enc(id)+"/UserData", `{"Rating":4}`).Code).To(Equal(http.StatusOK)) + + Expect(postAs(regularUser, "/Users/"+enc(adminUser.ID)+"/Items/"+enc(id)+"/UserData", `{"Rating":10}`).Code).To(Equal(http.StatusForbidden)) + Expect(post("/Users/"+enc(regularUser.ID)+"/Items/"+enc(id)+"/Rating?likes=true", "").Code).To(Equal(http.StatusForbidden)) + Expect(del("/Users/" + enc(regularUser.ID) + "/Items/" + enc(id) + "/Rating").Code).To(Equal(http.StatusForbidden)) + Expect(post("/UserItems/"+enc(id)+"/UserData?userId="+enc(regularUser.ID), `{"Rating":10}`).Code).To(Equal(http.StatusForbidden)) + + Expect(userData(adminUser, id).Rating).To(BeNil()) + Expect(*userData(regularUser, id).Rating).To(Equal(float64(4))) + }) + }) + Describe("private playlists", func() { It("does not expose another user's private playlist", func() { adminPl := createPlaylist("Admin Private", nil) @@ -53,6 +84,7 @@ var _ = Describe("Multi-user access control", func() { adminPl := createPlaylist("Admin Private", nil) Expect(postAs(regularUser, "/Users/user-1/FavoriteItems/"+enc(adminPl), "").Code).To(Equal(http.StatusNotFound)) Expect(postAs(regularUser, "/Users/user-1/Items/"+enc(adminPl)+"/Rating?Rating=10", "").Code).To(Equal(http.StatusNotFound)) + Expect(postAs(regularUser, "/UserItems/"+enc(adminPl)+"/UserData", `{"Rating":10}`).Code).To(Equal(http.StatusNotFound)) }) It("lets each user manage their own playlist", func() { diff --git a/server/jellyfin/items_test.go b/server/jellyfin/items_test.go index 18789a34b..bdf0be253 100644 --- a/server/jellyfin/items_test.go +++ b/server/jellyfin/items_test.go @@ -18,11 +18,13 @@ import ( . "github.com/onsi/gomega" ) -// withChiURLParam simulates chi's routing having captured a path parameter, since these -// tests call handlers directly instead of going through the full router. -func withChiURLParam(r *http.Request, key, value string) *http.Request { +// withChiURLParam simulates chi's routing having captured path parameters (key, value pairs), since +// these tests call handlers directly instead of going through the full router. +func withChiURLParam(r *http.Request, kv ...string) *http.Request { rctx := chi.NewRouteContext() - rctx.URLParams.Add(key, value) + for i := 0; i+1 < len(kv); i += 2 { + rctx.URLParams.Add(kv[i], kv[i+1]) + } return r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, rctx)) } diff --git a/ui/src/playlist/PlaylistList.jsx b/ui/src/playlist/PlaylistList.jsx index 14d819a4e..e1695e980 100644 --- a/ui/src/playlist/PlaylistList.jsx +++ b/ui/src/playlist/PlaylistList.jsx @@ -95,6 +95,7 @@ export const ToggleField = ({ resource, source }) => { return ( diff --git a/ui/src/playlist/PlaylistList.test.jsx b/ui/src/playlist/PlaylistList.test.jsx index 6c714b827..c05833166 100644 --- a/ui/src/playlist/PlaylistList.test.jsx +++ b/ui/src/playlist/PlaylistList.test.jsx @@ -2,6 +2,7 @@ import React from 'react' import { render, screen } from '@testing-library/react' import { describe, it, expect, vi } from 'vitest' import { TestContext } from 'ra-test' +import { RecordContextProvider } from 'react-admin' import { PlaylistLove, ToggleField, ToggleAutoImport } from './PlaylistList' vi.mock('../config', () => ({ @@ -14,6 +15,7 @@ vi.mock('../common', () => ({ {record?.starred ? 'starred' : 'not-starred'} ), + isWritable: (ownerId) => ownerId === 'me', })) describe('', () => { @@ -55,3 +57,28 @@ describe('playlist toggles without a record', () => { expect(container.innerHTML).toBe('') }) }) + +// Secondary is a surface color in many themes, so these toggles must use primary +describe('', () => { + const renderToggle = (record) => + render( + + + + + , + ) + + it.each([ + ['owner', 'me', false], + ['non-owner', 'someone-else', true], + ])('renders a primary-colored switch for the %s', (_, ownerId, disabled) => { + renderToggle({ id: 'pl-1', public: true, ownerId }) + const input = screen.getByRole('checkbox') + const switchBase = input.closest('.MuiSwitch-switchBase') + expect(input.checked).toBe(true) + expect(input.disabled).toBe(disabled) + expect(switchBase.classList).toContain('MuiSwitch-colorPrimary') + expect(switchBase.classList).not.toContain('MuiSwitch-colorSecondary') + }) +}) diff --git a/ui/src/themes/dracula.js b/ui/src/themes/dracula.js index 2e4ae38e5..45559c3af 100644 --- a/ui/src/themes/dracula.js +++ b/ui/src/themes/dracula.js @@ -185,16 +185,6 @@ export default { color: `${foreground} !important`, }, }, - MuiSwitch: { - colorSecondary: { - '&$checked': { - color: green, - }, - '&$checked + $track': { - backgroundColor: green, - }, - }, - }, NDAlbumGridView: { albumName: { marginTop: '0.5rem', diff --git a/ui/src/themes/gruvboxDark.js b/ui/src/themes/gruvboxDark.js index 0f4cbd7c4..3e2955dcd 100644 --- a/ui/src/themes/gruvboxDark.js +++ b/ui/src/themes/gruvboxDark.js @@ -121,16 +121,6 @@ export default { boxShadow: '3px 3px 5px #3c3836', }, }, - MuiSwitch: { - colorSecondary: { - '&$checked': { - color: '#458588', - }, - '&$checked + $track': { - backgroundColor: '#458588', - }, - }, - }, NDMobileArtistDetails: { bgContainer: { background: diff --git a/ui/src/themes/index.js b/ui/src/themes/index.js index 391a97f2c..285e579ae 100644 --- a/ui/src/themes/index.js +++ b/ui/src/themes/index.js @@ -25,6 +25,8 @@ import MoonbaseAlphaTheme from './moonbaseAlpha' import MoonbaseBravoTheme from './moonbaseBravo' import TokyoNightLightTheme from './tokyoNightLight' import TokyoNightTheme from './tokyoNight' +import SolarizedDarkTheme from './solarizedDark' +import SolarizedLightTheme from './solarizedLight' export default { // Classic default themes @@ -53,6 +55,8 @@ export default { RosePineDawnTheme, RosePineMoonTheme, RosePineTheme, + SolarizedDarkTheme, + SolarizedLightTheme, SpotifyTheme, SquiddiesGlassTheme, TokyoNightLightTheme, diff --git a/ui/src/themes/solarizedDark.css.js b/ui/src/themes/solarizedDark.css.js new file mode 100644 index 000000000..b357b8a0b --- /dev/null +++ b/ui/src/themes/solarizedDark.css.js @@ -0,0 +1,148 @@ +const stylesheet = ` + +.react-jinke-music-player-main svg:active, .react-jinke-music-player-main svg:hover { + color: #268bd2 +} + +.react-jinke-music-player-main .music-player-panel .panel-content .rc-slider-handle, .react-jinke-music-player-main .music-player-panel .panel-content .rc-slider-track { + background-color: #268bd2 +} + +.react-jinke-music-player-main ::-webkit-scrollbar-thumb { + background-color: #268bd2; +} + +.react-jinke-music-player-main .music-player-panel .panel-content .rc-slider-handle:active { + box-shadow: 0 0 2px #268bd2 +} + +.react-jinke-music-player-main .audio-item.playing svg { + color: #268bd2 +} + +.react-jinke-music-player-main .audio-item.playing .player-singer { + color: #268bd2 !important +} + +.react-jinke-music-player-main .loading svg { + color: #268bd2 !important +} + +.react-jinke-music-player-main .music-player-panel .panel-content .rc-slider-handle { + border: hidden; + box-shadow: rgba(15, 17, 21, 0.25) 0px 4px 6px, rgba(15, 17, 21, 0.1) 0px 5px 7px; +} + +.rc-slider-rail, .rc-slider-track { + height: 6px; +} + +.react-jinke-music-player-main .rc-slider-rail, +.react-jinke-music-player-mobile-progress .rc-slider-rail { + background-color: rgba(147, 161, 161, .2) !important +} + +.rc-slider { + padding: 3px 0; +} + +.sound-operation > div:nth-child(4) { + transform: translateX(-50%) translateY(5%) !important; +} + +.sound-operation { + padding: 4px 0; +} + +.react-jinke-music-player-main .music-player-panel { + background-color: #073642; + color: #93a1a1; + box-shadow: 0 0 8px rgba(0, 0, 0, 0.25); +} + +.audio-lists-panel { + background-color: #073642; + bottom: 6.25rem; + box-shadow: rgba(15, 17, 21, 0.25) 0px 4px 6px, rgba(15, 17, 21, 0.1) 0px 5px 7px; +} + +.audio-lists-panel-content .audio-item.playing { + background-color: rgba(0, 0, 0, 0); +} + +.audio-lists-panel-content .audio-item:nth-child(2n+1) { + background-color: rgba(0, 0, 0, 0); +} + +.audio-lists-panel-content .audio-item:active, +.audio-lists-panel-content .audio-item:hover { + background-color: #0d4251; +} + +.audio-lists-panel-header { + border-bottom: 1px solid rgba(0, 0, 0, 0.25); + box-shadow: none; +} + +.react-jinke-music-player-main .music-player-panel .panel-content .player-content .audio-lists-btn { + background-color: rgba(0, 0, 0, 0); + box-shadow: 0 0 0 0; +} + +.audio-lists-panel-content .audio-item { + line-height: 32px; +} + +.react-jinke-music-player-main .music-player-panel .panel-content .img-content { + box-shadow: rgba(15, 17, 21, 0.25) 0px 4px 6px, rgba(15, 17, 21, 0.1) 0px 5px 7px; +} + +.react-jinke-music-player-main .music-player-lyric { + color: #93a1a1; + -webkit-text-stroke: 0.5px #002b36; + font-weight: bolder; +} + +.react-jinke-music-player-main .lyric-btn-active, .react-jinke-music-player-main .lyric-btn-active svg { + color: #268bd2 !important; +} + +.audio-lists-panel-content .audio-item.playing, .audio-lists-panel-content .audio-item.playing svg { + color: #268bd2 +} + +.audio-lists-panel-content .audio-item:active .group:not(.player-delete) svg, .audio-lists-panel-content .audio-item:hover .group:not(.player-delete) svg { + color: #268bd2 +} + +.audio-lists-panel-content .audio-item .player-icons { + scale: 75%; +} + +/* Mobile */ + +.react-jinke-music-player-mobile-cover { + border: none; + box-shadow: rgba(15, 17, 21, 0.25) 0px 4px 6px, rgba(15, 17, 21, 0.1) 0px 5px 7px; +} + +.react-jinke-music-player .music-player-controller { + border: none; + box-shadow: rgba(15, 17, 21, 0.25) 0px 4px 6px, rgba(15, 17, 21, 0.1) 0px 5px 7px; + color: #268bd2; +} + +.react-jinke-music-player .music-player-controller .music-player-controller-setting { + color: rgba(38, 139, 210, .3); +} + +.react-jinke-music-player-mobile-progress .rc-slider-handle, .react-jinke-music-player-mobile-progress .rc-slider-track { + background-color: #268bd2; +} + +.react-jinke-music-player-mobile-progress .rc-slider-handle { + border: none; +} +` + +export default stylesheet diff --git a/ui/src/themes/solarizedDark.js b/ui/src/themes/solarizedDark.js new file mode 100644 index 000000000..1e1fcdff4 --- /dev/null +++ b/ui/src/themes/solarizedDark.js @@ -0,0 +1,380 @@ +import stylesheet from './solarizedDark.css.js' + +// Solarized palette by Ethan Schoonover: https://ethanschoonover.com/solarized/ +const background = '#002b36' // base03 +const surface = '#073642' // base02 +const currentLine = '#0d4251' // base02, slightly lighter for hover and headers +const foreground = '#93a1a1' // base1 +const comment = '#839496' // base0 +const blue = '#268bd2' +const link = '#57b5ae' // cyan, lightened to keep text links above 4.5:1 +const accent = '#2aa198' // cyan, since violet is too dark on base03 +const red = '#dc322f' + +// For Album, Playlist play button +const musicListActions = { + alignItems: 'center', + '@global': { + 'button:first-child:not(:only-child)': { + '@media screen and (max-width: 720px)': { + transform: 'scale(1.5)', + margin: '1rem', + '&:hover': { + transform: 'scale(1.6) !important', + }, + }, + transform: 'scale(2)', + margin: '1.5rem', + minWidth: 0, + padding: 5, + transition: 'transform .3s ease', + backgroundColor: `${blue} !important`, + color: background, + borderRadius: 500, + border: 0, + '&:hover': { + transform: 'scale(2.1)', + backgroundColor: `${blue} !important`, + border: 0, + }, + }, + 'button:only-child': { + margin: '1.5rem', + }, + 'button:first-child>span:first-child': { + padding: 0, + }, + 'button:first-child>span:first-child>span': { + display: 'none', + }, + 'button>span:first-child>span, button:not(:first-child)>span:first-child>svg': + { + color: foreground, + }, + }, +} + +export default { + themeName: 'Solarized Dark', + palette: { + primary: { + main: blue, + }, + secondary: { + main: accent, + contrastText: background, + }, + error: { + main: red, + }, + type: 'dark', + background: { + default: background, + paper: surface, + }, + }, + overrides: { + MuiPaper: { + root: { + color: foreground, + backgroundColor: surface, + }, + }, + MuiAppBar: { + positionFixed: { + backgroundColor: `${surface} !important`, + boxShadow: + 'rgba(15, 17, 21, 0.25) 0px 4px 6px, rgba(15, 17, 21, 0.1) 0px 5px 7px', + }, + colorSecondary: { + color: foreground, + }, + }, + MuiDrawer: { + root: { + background: background, + }, + }, + MuiButton: { + textPrimary: { + color: blue, + }, + textSecondary: { + color: foreground, + }, + }, + MuiIconButton: { + root: { + color: foreground, + }, + }, + MuiChip: { + root: { + backgroundColor: currentLine, + }, + }, + MuiFormGroup: { + root: { + color: foreground, + }, + }, + MuiFormLabel: { + root: { + color: comment, + '&$focused': { + color: blue, + }, + }, + }, + MuiFormHelperText: { + error: { + color: red, + }, + }, + MuiToolbar: { + root: { + backgroundColor: `${surface} !important`, + }, + }, + MuiOutlinedInput: { + root: { + '& $notchedOutline': { + borderColor: currentLine, + }, + '&:hover $notchedOutline': { + borderColor: comment, + }, + '&$focused $notchedOutline': { + borderColor: blue, + }, + }, + }, + MuiFilledInput: { + root: { + backgroundColor: currentLine, + '&:hover': { + backgroundColor: comment, + }, + '&$focused': { + backgroundColor: currentLine, + }, + }, + }, + MuiTableRow: { + root: { + transition: 'background-color .3s ease', + '&:hover': { + backgroundColor: `${currentLine} !important`, + }, + // Cells have an opaque background, so the hover must be applied to them too + '&:hover > .MuiTableCell-root': { + background: `${currentLine} !important`, + }, + }, + }, + MuiTableHead: { + root: { + color: foreground, + background: surface, + }, + }, + MuiTableCell: { + root: { + color: foreground, + background: `${surface} !important`, + borderBottom: `1px solid ${currentLine}`, + }, + head: { + color: `${foreground} !important`, + background: `${currentLine} !important`, + }, + body: { + color: `${foreground} !important`, + }, + }, + NDAlbumGridView: { + albumName: { + marginTop: '0.5rem', + fontWeight: 700, + color: foreground, + }, + albumSubtitle: { + color: comment, + }, + albumContainer: { + backgroundColor: surface, + borderRadius: '8px', + padding: '.75rem', + transition: 'background-color .3s ease', + '&:hover': { + backgroundColor: currentLine, + }, + }, + albumPlayButton: { + backgroundColor: blue, + borderRadius: '50%', + boxShadow: '0 8px 8px rgb(0 0 0 / 30%)', + padding: '0.35rem', + transition: 'padding .3s ease', + '&:hover': { + background: `${blue} !important`, + padding: '0.45rem', + }, + }, + }, + NDPlaylistDetails: { + container: { + background: `linear-gradient(${currentLine}, transparent)`, + borderRadius: 0, + paddingTop: '2.5rem !important', + boxShadow: 'none', + }, + title: { + fontWeight: 700, + color: foreground, + }, + details: { + fontSize: '.875rem', + color: comment, + }, + }, + NDAlbumDetails: { + root: { + background: `linear-gradient(${currentLine}, transparent)`, + borderRadius: 0, + boxShadow: 'none', + }, + cardContents: { + alignItems: 'center', + paddingTop: '1.5rem', + }, + recordName: { + fontWeight: 700, + color: foreground, + }, + recordArtist: { + fontSize: '.875rem', + fontWeight: 700, + color: accent, + }, + recordMeta: { + fontSize: '.875rem', + color: comment, + }, + }, + NDCollapsibleComment: { + commentBlock: { + fontSize: '.875rem', + color: comment, + }, + }, + NDAlbumShow: { + albumActions: musicListActions, + }, + NDPlaylistShow: { + playlistActions: musicListActions, + }, + NDAudioPlayer: { + audioTitle: { + color: foreground, + fontSize: '0.875rem', + }, + songTitle: { + fontWeight: 400, + }, + songInfo: { + fontSize: '0.675rem', + color: comment, + }, + }, + NDLogin: { + systemNameLink: { + color: link, + }, + welcome: { + color: foreground, + }, + card: { + minWidth: 300, + background: surface, + }, + button: { + boxShadow: '3px 3px 5px #00212b', + }, + }, + NDMobileArtistDetails: { + bgContainer: { + background: `linear-gradient(to bottom, rgba(0 43 54 / 72%), ${background})!important`, + }, + }, + RaLayout: { + content: { + padding: '0 !important', + background: background, + }, + root: { + backgroundColor: background, + }, + }, + RaList: { + content: { + backgroundColor: background, + }, + }, + RaListToolbar: { + toolbar: { + backgroundColor: background, + padding: '0 .55rem !important', + }, + }, + RaSidebar: { + fixed: { + backgroundColor: background, + }, + drawerPaper: { + backgroundColor: `${background} !important`, + }, + }, + RaMenuItemLink: { + root: { + color: foreground, + '&[aria-current="page"]': { + color: `${blue} !important`, + }, + '&[aria-current="page"] .MuiListItemIcon-root': { + color: `${blue} !important`, + }, + }, + active: { + color: `${blue} !important`, + '& .MuiListItemIcon-root': { + color: `${blue} !important`, + }, + }, + }, + RaLink: { + link: { + color: link, + }, + }, + RaButton: { + button: { + margin: '0 5px 0 5px', + }, + }, + RaPaginationActions: { + currentPageButton: { + border: `2px solid ${blue}`, + }, + button: { + backgroundColor: currentLine, + minWidth: 48, + margin: '0 4px', + }, + }, + }, + player: { + theme: 'dark', + stylesheet, + }, +} diff --git a/ui/src/themes/solarizedLight.css.js b/ui/src/themes/solarizedLight.css.js new file mode 100644 index 000000000..c6a599562 --- /dev/null +++ b/ui/src/themes/solarizedLight.css.js @@ -0,0 +1,123 @@ +const stylesheet = ` +.react-jinke-music-player-main.light-theme .loading svg { + color: #268bd2; + font-size: 24px +} + +.react-jinke-music-player-mobile-play-model-tip { + background-color: #268bd2; +} + +.react-jinke-music-player-main .music-player-panel .panel-content .rc-slider-handle, .react-jinke-music-player-main .music-player-panel .panel-content .rc-slider-track { + background-color: #268bd2 +} + +.react-jinke-music-player-main .music-player-panel .panel-content .rc-slider-handle:active { + box-shadow: 0 0 2px #268bd2 +} + +.react-jinke-music-player-main.light-theme .audio-item.playing svg { + color: #268bd2 +} + +.react-jinke-music-player-main.light-theme .audio-item.playing .player-singer { + color: #268bd2 !important +} + +.audio-lists-panel-content .audio-item.playing, .audio-lists-panel-content .audio-item.playing svg { + color: #268bd2 +} + +.audio-lists-panel-content .audio-item:active .group:not(.player-delete) svg, .audio-lists-panel-content .audio-item:hover .group:not(.player-delete) svg { + color: #268bd2 +} + +.react-jinke-music-player-main.light-theme ::-webkit-scrollbar-thumb { + background-color: #268bd2; +} + +.react-jinke-music-player-main.light-theme svg { + color: #586e75 +} + +.react-jinke-music-player-main.light-theme svg:active, .react-jinke-music-player-main.light-theme svg:hover { + color: #268bd2 +} + +.react-jinke-music-player-main.light-theme .rc-slider-rail { + background-color: rgba(88, 110, 117, .12) !important +} + +.react-jinke-music-player-main.light-theme .music-player-controller { + background-color: #fdf6e3; + border-color: #fdf6e3 +} + +.react-jinke-music-player-main.light-theme .music-player-panel { + background-color: #fdf6e3; + box-shadow: 0 1px 2px 0 rgba(0, 43, 54, .05); + color: #586e75 +} + +.react-jinke-music-player-main.light-theme .music-player-panel .img-content { + box-shadow: 0 0 10px #e4ddc6 +} + +.react-jinke-music-player-main.light-theme .music-player-panel .progress-load-bar { + background-color: rgba(88, 110, 117, .08) !important +} + +.react-jinke-music-player-main.light-theme .rc-switch { + color: #fdf6e3 +} + +.react-jinke-music-player-main.light-theme .rc-switch:after { + background-color: #fdf6e3 +} + +.react-jinke-music-player-main.light-theme .rc-switch-checked { + background-color: #268bd2 !important; + border: 1px solid #268bd2 +} + +.react-jinke-music-player-main.light-theme .rc-switch-inner { + color: #fdf6e3 +} + +.react-jinke-music-player-main.light-theme .audio-lists-btn { + background-color: #eee8d5 !important +} + +.react-jinke-music-player-main.light-theme .audio-lists-btn:active, .react-jinke-music-player-main.light-theme .audio-lists-btn:hover { + background-color: #e4ddc6; + color: #586e75 +} + +.react-jinke-music-player-main.light-theme .audio-lists-btn > .group:hover, .react-jinke-music-player-main.light-theme .audio-lists-btn > .group:hover > svg { + color: #268bd2 +} + +.react-jinke-music-player-main.light-theme .audio-lists-panel { + background-color: #fdf6e3; + box-shadow: 0 0 2px #e4ddc6; + color: #586e75 +} + +.react-jinke-music-player-main.light-theme .audio-lists-panel .audio-item { + background-color: #fdf6e3 +} + +.react-jinke-music-player-main.light-theme .audio-lists-panel .audio-item:nth-child(odd) { + background-color: #f5efdc !important +} + +.react-jinke-music-player-main.light-theme .audio-lists-panel .audio-item.playing { + background-color: #e4ddc6 !important +} + +.react-jinke-music-player-main.light-theme .audio-lists-panel .audio-item.playing, .react-jinke-music-player-main.light-theme .audio-lists-panel .audio-item.playing svg { + color: #268bd2 !important +} +` + +export default stylesheet diff --git a/ui/src/themes/solarizedLight.js b/ui/src/themes/solarizedLight.js new file mode 100644 index 000000000..65e0241e9 --- /dev/null +++ b/ui/src/themes/solarizedLight.js @@ -0,0 +1,380 @@ +import stylesheet from './solarizedLight.css.js' + +// Solarized palette by Ethan Schoonover: https://ethanschoonover.com/solarized/ +const background = '#eee8d5' // base2 +const surface = '#fdf6e3' // base3 +const currentLine = '#e4ddc6' // base2, slightly darker for hover and headers +const foreground = '#586e75' // base01 +const comment = '#657b83' // base00 +const blue = '#268bd2' +const link = '#1c6599' // blue, darkened to keep text links above 4.5:1 +const accent = '#5d61a9' // violet, darkened to keep text above 4.5:1 +const red = '#dc322f' + +// For Album, Playlist play button +const musicListActions = { + alignItems: 'center', + '@global': { + 'button:first-child:not(:only-child)': { + '@media screen and (max-width: 720px)': { + transform: 'scale(1.5)', + margin: '1rem', + '&:hover': { + transform: 'scale(1.6) !important', + }, + }, + transform: 'scale(2)', + margin: '1.5rem', + minWidth: 0, + padding: 5, + transition: 'transform .3s ease', + backgroundColor: `${blue} !important`, + color: background, + borderRadius: 500, + border: 0, + '&:hover': { + transform: 'scale(2.1)', + backgroundColor: `${blue} !important`, + border: 0, + }, + }, + 'button:only-child': { + margin: '1.5rem', + }, + 'button:first-child>span:first-child': { + padding: 0, + }, + 'button:first-child>span:first-child>span': { + display: 'none', + }, + 'button>span:first-child>span, button:not(:first-child)>span:first-child>svg': + { + color: foreground, + }, + }, +} + +export default { + themeName: 'Solarized Light', + palette: { + primary: { + main: blue, + }, + secondary: { + main: accent, + contrastText: surface, + }, + error: { + main: red, + }, + type: 'light', + background: { + default: background, + paper: surface, + }, + }, + overrides: { + MuiPaper: { + root: { + color: foreground, + backgroundColor: surface, + }, + }, + MuiAppBar: { + positionFixed: { + backgroundColor: `${surface} !important`, + boxShadow: + 'rgba(0, 43, 54, 0.12) 0px 4px 6px, rgba(0, 43, 54, 0.06) 0px 5px 7px', + }, + colorSecondary: { + color: foreground, + }, + }, + MuiDrawer: { + root: { + background: background, + }, + }, + MuiButton: { + textPrimary: { + color: blue, + }, + textSecondary: { + color: foreground, + }, + }, + MuiIconButton: { + root: { + color: foreground, + }, + }, + MuiChip: { + root: { + backgroundColor: currentLine, + }, + }, + MuiFormGroup: { + root: { + color: foreground, + }, + }, + MuiFormLabel: { + root: { + color: comment, + '&$focused': { + color: blue, + }, + }, + }, + MuiFormHelperText: { + error: { + color: red, + }, + }, + MuiToolbar: { + root: { + backgroundColor: `${surface} !important`, + }, + }, + MuiOutlinedInput: { + root: { + '& $notchedOutline': { + borderColor: currentLine, + }, + '&:hover $notchedOutline': { + borderColor: comment, + }, + '&$focused $notchedOutline': { + borderColor: blue, + }, + }, + }, + MuiFilledInput: { + root: { + backgroundColor: currentLine, + '&:hover': { + backgroundColor: comment, + }, + '&$focused': { + backgroundColor: currentLine, + }, + }, + }, + MuiTableRow: { + root: { + transition: 'background-color .3s ease', + '&:hover': { + backgroundColor: `${currentLine} !important`, + }, + // Cells have an opaque background, so the hover must be applied to them too + '&:hover > .MuiTableCell-root': { + background: `${currentLine} !important`, + }, + }, + }, + MuiTableHead: { + root: { + color: foreground, + background: surface, + }, + }, + MuiTableCell: { + root: { + color: foreground, + background: `${surface} !important`, + borderBottom: `1px solid ${currentLine}`, + }, + head: { + color: `${foreground} !important`, + background: `${currentLine} !important`, + }, + body: { + color: `${foreground} !important`, + }, + }, + NDAlbumGridView: { + albumName: { + marginTop: '0.5rem', + fontWeight: 700, + color: foreground, + }, + albumSubtitle: { + color: comment, + }, + albumContainer: { + backgroundColor: surface, + borderRadius: '8px', + padding: '.75rem', + transition: 'background-color .3s ease', + '&:hover': { + backgroundColor: currentLine, + }, + }, + albumPlayButton: { + backgroundColor: blue, + borderRadius: '50%', + boxShadow: '0 8px 8px rgb(0 0 0 / 20%)', + padding: '0.35rem', + transition: 'padding .3s ease', + '&:hover': { + background: `${blue} !important`, + padding: '0.45rem', + }, + }, + }, + NDPlaylistDetails: { + container: { + background: `linear-gradient(${currentLine}, transparent)`, + borderRadius: 0, + paddingTop: '2.5rem !important', + boxShadow: 'none', + }, + title: { + fontWeight: 700, + color: foreground, + }, + details: { + fontSize: '.875rem', + color: comment, + }, + }, + NDAlbumDetails: { + root: { + background: `linear-gradient(${currentLine}, transparent)`, + borderRadius: 0, + boxShadow: 'none', + }, + cardContents: { + alignItems: 'center', + paddingTop: '1.5rem', + }, + recordName: { + fontWeight: 700, + color: foreground, + }, + recordArtist: { + fontSize: '.875rem', + fontWeight: 700, + color: accent, + }, + recordMeta: { + fontSize: '.875rem', + color: comment, + }, + }, + NDCollapsibleComment: { + commentBlock: { + fontSize: '.875rem', + color: comment, + }, + }, + NDAlbumShow: { + albumActions: musicListActions, + }, + NDPlaylistShow: { + playlistActions: musicListActions, + }, + NDAudioPlayer: { + audioTitle: { + color: foreground, + fontSize: '0.875rem', + }, + songTitle: { + fontWeight: 400, + }, + songInfo: { + fontSize: '0.675rem', + color: comment, + }, + }, + NDLogin: { + systemNameLink: { + color: link, + }, + welcome: { + color: foreground, + }, + card: { + minWidth: 300, + background: surface, + }, + button: { + boxShadow: '3px 3px 5px #93a1a1', + }, + }, + NDMobileArtistDetails: { + bgContainer: { + background: `linear-gradient(to bottom, rgba(238 232 213 / 72%), ${background})!important`, + }, + }, + RaLayout: { + content: { + padding: '0 !important', + background: background, + }, + root: { + backgroundColor: background, + }, + }, + RaList: { + content: { + backgroundColor: background, + }, + }, + RaListToolbar: { + toolbar: { + backgroundColor: background, + padding: '0 .55rem !important', + }, + }, + RaSidebar: { + fixed: { + backgroundColor: background, + }, + drawerPaper: { + backgroundColor: `${background} !important`, + }, + }, + RaMenuItemLink: { + root: { + color: foreground, + '&[aria-current="page"]': { + color: `${blue} !important`, + }, + '&[aria-current="page"] .MuiListItemIcon-root': { + color: `${blue} !important`, + }, + }, + active: { + color: `${blue} !important`, + '& .MuiListItemIcon-root': { + color: `${blue} !important`, + }, + }, + }, + RaLink: { + link: { + color: link, + }, + }, + RaButton: { + button: { + margin: '0 5px 0 5px', + }, + }, + RaPaginationActions: { + currentPageButton: { + border: `2px solid ${blue}`, + }, + button: { + backgroundColor: currentLine, + minWidth: 48, + margin: '0 4px', + }, + }, + }, + player: { + theme: 'light', + stylesheet, + }, +} diff --git a/ui/src/themes/tokyoNight.js b/ui/src/themes/tokyoNight.js index 07d372a6b..9f6424b77 100644 --- a/ui/src/themes/tokyoNight.js +++ b/ui/src/themes/tokyoNight.js @@ -184,16 +184,6 @@ export default { color: `${foreground} !important`, }, }, - MuiSwitch: { - colorSecondary: { - '&$checked': { - color: blue, - }, - '&$checked + $track': { - backgroundColor: blue, - }, - }, - }, NDAlbumGridView: { albumName: { marginTop: '0.5rem', diff --git a/ui/src/themes/tokyoNightLight.js b/ui/src/themes/tokyoNightLight.js index f84cd0be9..a61c0fe87 100644 --- a/ui/src/themes/tokyoNightLight.js +++ b/ui/src/themes/tokyoNightLight.js @@ -184,16 +184,6 @@ export default { color: `${foreground} !important`, }, }, - MuiSwitch: { - colorSecondary: { - '&$checked': { - color: blue, - }, - '&$checked + $track': { - backgroundColor: blue, - }, - }, - }, NDAlbumGridView: { albumName: { marginTop: '0.5rem', diff --git a/ui/src/themes/useCurrentTheme.js b/ui/src/themes/useCurrentTheme.js index 4ccefe820..fbb5e9bc8 100644 --- a/ui/src/themes/useCurrentTheme.js +++ b/ui/src/themes/useCurrentTheme.js @@ -63,6 +63,8 @@ const useCurrentTheme = () => { ...theme.props, MuiUseMediaQuery: { noSsr: true }, MuiPopover: { disableScrollLock: true }, + // MUI defaults to secondary, which many themes use as a surface color + MuiSwitch: { color: 'primary' }, }, }), [theme], diff --git a/ui/src/themes/useCurrentTheme.test.jsx b/ui/src/themes/useCurrentTheme.test.jsx index 65c3be8c6..6553d9866 100644 --- a/ui/src/themes/useCurrentTheme.test.jsx +++ b/ui/src/themes/useCurrentTheme.test.jsx @@ -3,6 +3,10 @@ import { Provider } from 'react-redux' import { createStore } from 'redux' import mediaQuery from 'css-mediaquery' import { renderHook } from '@testing-library/react-hooks' +import { render, screen } from '@testing-library/react' +import { createMuiTheme, ThemeProvider } from '@material-ui/core/styles' +import Switch from '@material-ui/core/Switch' +import themes from './index' import useCurrentTheme from './useCurrentTheme' import { themeReducer } from '../reducers/themeReducer' import { AUTO_THEME_ID } from '../consts' @@ -161,4 +165,27 @@ describe('useCurrentTheme', () => { expect(document.body.style.backgroundColor).toBe('rgb(18, 18, 18)') }) }) + describe('switch color', () => { + it.each(Object.keys(themes))( + 'renders switches with the primary color in %s', + (theme) => { + const { result } = renderHook(() => useCurrentTheme(), { + wrapper: ({ children }) => ( + + {children} + + ), + }) + render( + + {}} /> + , + ) + const switchBase = screen + .getByRole('checkbox') + .closest('.MuiSwitch-switchBase') + expect(switchBase.classList).toContain('MuiSwitch-colorPrimary') + }, + ) + }) })