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/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/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/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') + }, + ) + }) })