diff --git a/.github/FUNDING.yml b/.github/FUNDING.yml index 534cfbd11..e5ed36e70 100644 --- a/.github/FUNDING.yml +++ b/.github/FUNDING.yml @@ -1,10 +1,10 @@ # These are supported funding model platforms -github: deluan -patreon: # Replace with a single Patreon username -open_collective: # Replace with a single Open Collective username ko_fi: deluan +github: deluan +open_collective: navidrome liberapay: deluan +patreon: # Replace with a single Patreon username tidelift: # Replace with a single Tidelift platform-name/package-name e.g., npm/babel community_bridge: # Replace with a single Community Bridge project-name e.g., cloud-foundry issuehunt: # Replace with a single IssueHunt username diff --git a/core/ffmpeg/ffmpeg.go b/core/ffmpeg/ffmpeg.go index 3d4cd0e72..af2dab647 100644 --- a/core/ffmpeg/ffmpeg.go +++ b/core/ffmpeg/ffmpeg.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "io" + "io/fs" "os" "os/exec" "path/filepath" @@ -67,7 +68,7 @@ var ErrAnimatedWebPUnsupported = errors.New("ffmpeg lacks libwebp_anim encoder const ( extractImageCmd = "ffmpeg -i %s -map 0:v -map -0:V -vcodec copy -f image2pipe -" probeCmd = "ffmpeg %s -f ffmetadata" - probeAudioStreamCmd = "ffprobe -v quiet -select_streams a:0 -print_format json -show_streams -show_format %s" + probeAudioStreamCmd = "ffprobe -v error -select_streams a:0 -print_format json -show_streams -show_format %s" ) type ffmpeg struct{} @@ -159,16 +160,80 @@ func (e *ffmpeg) ProbeAudioStream(ctx context.Context, filePath string) (*AudioP return nil, err } if err := fileExists(filePath); err != nil { - return nil, err + return nil, &ProbeError{Path: filePath, Reason: fileAccessReason(err), + NotFound: errors.Is(err, fs.ErrNotExist), err: err} } args := createFFmpegCommand(probeAudioStreamCmd, filePath, 0, 0) log.Trace(ctx, "Executing ffprobe command", "args", args) cmd := exec.CommandContext(ctx, args[0], args[1:]...) // #nosec output, err := cmd.Output() if err != nil { - return nil, fmt.Errorf("running ffprobe on %q: %w", filePath, err) + return nil, &ProbeError{Path: filePath, Reason: probeClientReason(err, filePath), err: err} } - return parseProbeOutput(output) + result, err := parseProbeOutput(output) + if err != nil { + return nil, &ProbeError{Path: filePath, Reason: err.Error(), err: err} + } + return result, nil +} + +// ProbeError reports an ffprobe failure. Reason is a path-free message safe to +// expose to clients; the wrapped cause carries the full detail for logging. +// NotFound marks the media file itself as missing — a launch failure of a +// deleted ffprobe binary also wraps fs.ErrNotExist, so callers must not infer +// it from the error chain. +type ProbeError struct { + Path string + Reason string + NotFound bool + err error +} + +func (e *ProbeError) Error() string { + if e.err == nil { + return fmt.Sprintf("probe failed on %q: %s", e.Path, e.Reason) + } + return fmt.Sprintf("probe failed on %q: %s", e.Path, probeDetail(e.err)) +} + +// Unwrap exposes the underlying cause so callers can test it with errors.Is +// (e.g. fs.ErrNotExist to detect a missing file). +func (e *ProbeError) Unwrap() error { return e.err } + +// SafeReason returns the path-free reason, safe to send to clients. +func (e *ProbeError) SafeReason() string { return e.Reason } + +// fileAccessReason maps a stat failure to a clear, path-free reason, so a moved +// or unreadable file reads as "file not found" rather than a raw ffprobe message. +func fileAccessReason(err error) string { + switch { + case errors.Is(err, fs.ErrNotExist): + return "file not found" + case errors.Is(err, fs.ErrPermission): + return "permission denied" + default: + return "file not accessible" + } +} + +// probeDetail returns the full diagnostic for logging (may contain paths): +// ffprobe's stderr when present, otherwise the raw error text. +func probeDetail(err error) string { + if exitErr, ok := errors.AsType[*exec.ExitError](err); ok && len(exitErr.Stderr) > 0 { + return strings.TrimSpace(string(exitErr.Stderr)) + } + return err.Error() +} + +// probeClientReason returns a path-free reason for an ffprobe execution failure: +// ffprobe's stderr with the file path stripped, or a generic reason when ffprobe +// couldn't run at all (its launch error may embed the binary path). +func probeClientReason(err error, path string) string { + exitErr, ok := errors.AsType[*exec.ExitError](err) + if !ok || len(exitErr.Stderr) == 0 { + return "could not read file" + } + return strings.TrimSpace(strings.ReplaceAll(string(exitErr.Stderr), path, "the file")) } type probeOutput struct { diff --git a/core/ffmpeg/ffmpeg_test.go b/core/ffmpeg/ffmpeg_test.go index 9c20e6c05..0fa3de111 100644 --- a/core/ffmpeg/ffmpeg_test.go +++ b/core/ffmpeg/ffmpeg_test.go @@ -2,6 +2,7 @@ package ffmpeg import ( "context" + "errors" "os" "os/exec" "path/filepath" @@ -553,6 +554,65 @@ var _ = Describe("ffmpeg", func() { }) }) + Describe("ProbeError", func() { + It("uses the underlying cause in Error() so logs keep the full detail", func() { + e := &ProbeError{Path: "/music/foo.flac", + err: errors.New("/music/foo.flac: Invalid data found when processing input")} + Expect(e.Error()).To(ContainSubstring("/music/foo.flac")) + Expect(e.Error()).To(ContainSubstring("Invalid data found when processing input")) + }) + + It("returns the path-free reason from SafeReason()", func() { + e := &ProbeError{Path: "/music/foo.flac", Reason: "the file: Invalid data found when processing input"} + Expect(e.SafeReason()).To(Equal("the file: Invalid data found when processing input")) + Expect(e.SafeReason()).ToNot(ContainSubstring("/music/foo.flac")) + }) + + It("unwraps to the underlying cause so errors.Is detects a missing file", func() { + e := &ProbeError{Path: "/music/foo.flac", Reason: "file not found", err: os.ErrNotExist} + Expect(errors.Is(e, os.ErrNotExist)).To(BeTrue()) + }) + }) + + Describe("probeClientReason", func() { + It("strips the file path from ffprobe stderr", func() { + if runtime.GOOS == "windows" { + Skip("uses /bin/sh") + } + _, err := exec.Command("/bin/sh", "-c", "echo '/music/foo.flac: Invalid data found' >&2; exit 1").Output() + Expect(err).To(HaveOccurred()) + Expect(probeClientReason(err, "/music/foo.flac")).To(Equal("the file: Invalid data found")) + }) + + It("returns a generic reason for launch failures, without leaking the binary path", func() { + err := errors.New("fork/exec /opt/navidrome/bin/ffprobe: no such file or directory") + Expect(probeClientReason(err, "/music/foo.flac")).To(Equal("could not read file")) + }) + }) + + Describe("probeDetail", func() { + It("surfaces ffprobe stderr for logging", func() { + if runtime.GOOS == "windows" { + Skip("uses /bin/sh") + } + _, err := exec.Command("/bin/sh", "-c", "echo 'boom detail' >&2; exit 1").Output() + Expect(err).To(HaveOccurred()) + Expect(probeDetail(err)).To(Equal("boom detail")) + }) + }) + + Describe("fileAccessReason", func() { + It("reports a missing file as 'file not found', not a raw stat message", func() { + _, err := os.Stat("/no/such/dir/really-missing.flac") + Expect(err).To(HaveOccurred()) + Expect(fileAccessReason(err)).To(Equal("file not found")) + }) + + It("falls back to a generic reason for other access errors", func() { + Expect(fileAccessReason(errors.New("boom"))).To(Equal("file not accessible")) + }) + }) + Describe("FFmpeg", func() { Context("when FFmpeg is available", func() { var ff FFmpeg @@ -566,6 +626,16 @@ var _ = Describe("ffmpeg", func() { } }) + It("ProbeAudioStream returns a not-found ProbeError for a missing file", func() { + _, err := ff.ProbeAudioStream(GinkgoT().Context(), "/no/such/dir/really-missing.flac") + Expect(err).To(HaveOccurred()) + Expect(errors.Is(err, os.ErrNotExist)).To(BeTrue()) + var pe *ProbeError + Expect(errors.As(err, &pe)).To(BeTrue()) + Expect(pe.SafeReason()).To(Equal("file not found")) + Expect(pe.NotFound).To(BeTrue()) + }) + It("should interrupt transcoding when context is cancelled", func() { ctx, cancel := context.WithTimeout(GinkgoT().Context(), 5*time.Second) defer cancel() diff --git a/go.mod b/go.mod index 014a43a56..5488b41e4 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/navidrome/navidrome go 1.26 // Fork to implement raw tags support -replace go.senan.xyz/taglib => github.com/deluan/go-taglib v0.0.0-20260619222856-1975cb12f59d +replace go.senan.xyz/taglib => github.com/deluan/go-taglib v0.0.0-20260720134629-a133b9719ea3 require ( github.com/Masterminds/squirrel v1.5.4 diff --git a/go.sum b/go.sum index 064974edb..29983a27d 100644 --- a/go.sum +++ b/go.sum @@ -31,8 +31,8 @@ github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1 github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/decred/dcrd/dcrec/secp256k1/v4 v4.4.1 h1:5RVFMOWjMyRy8cARdy79nAmgYw3hK/4HUq48LQ6Wwqo= github.com/decred/dcrd/dcrec/secp256k1/v4 v4.4.1/go.mod h1:ZXNYxsqcloTdSy/rNShjYzMhyjf0LaoftYK0p+A3h40= -github.com/deluan/go-taglib v0.0.0-20260619222856-1975cb12f59d h1:/MmnVPIlGzX5kYF6sNtMaOHMkjmu0Us7WtDyJZTglMs= -github.com/deluan/go-taglib v0.0.0-20260619222856-1975cb12f59d/go.mod h1:QGxQ4Z1IWyY9w56xNEFjYAaWE8uSxA/gneQ7RPcFJrY= +github.com/deluan/go-taglib v0.0.0-20260720134629-a133b9719ea3 h1:j7eSXqgtjhlNfwnMEzRdXnJGZTEw4I7J9TeQAll83bU= +github.com/deluan/go-taglib v0.0.0-20260720134629-a133b9719ea3/go.mod h1:QGxQ4Z1IWyY9w56xNEFjYAaWE8uSxA/gneQ7RPcFJrY= github.com/deluan/rest v0.0.0-20211102003136-6260bc399cbf h1:tb246l2Zmpt/GpF9EcHCKTtwzrd0HGfEmoODFA/qnk4= github.com/deluan/rest v0.0.0-20211102003136-6260bc399cbf/go.mod h1:tSgDythFsl0QgS/PFWfIZqcJKnkADWneY80jaVRlqK8= github.com/deluan/sanitize v0.0.0-20241120162836-fdfd8fdfaa55 h1:wSCnggTs2f2ji6nFwQmfwgINcmSMj0xF0oHnoyRSPe4= diff --git a/model/album.go b/model/album.go index 0afeb2760..55df8c63c 100644 --- a/model/album.go +++ b/model/album.go @@ -1,7 +1,6 @@ package model import ( - "fmt" "iter" "math" "sync" @@ -77,7 +76,7 @@ func (a Album) CoverArtID() ArtworkID { func (a Album) FullName() string { if conf.Server.Subsonic.AppendAlbumVersion && len(a.Tags[TagAlbumVersion]) > 0 { - return fmt.Sprintf("%s (%s)", a.Name, a.Tags[TagAlbumVersion][0]) + return appendSuffix(a.Name, a.Tags[TagAlbumVersion][0]) } return a.Name } diff --git a/model/album_test.go b/model/album_test.go index 0f4c912cd..ad2ca1cb6 100644 --- a/model/album_test.go +++ b/model/album_test.go @@ -24,6 +24,8 @@ var _ = Describe("Album", func() { Entry("returns just name when disabled", false, Tags{TagAlbumVersion: []string{"Remastered"}}, "Album"), Entry("returns just name when tag is absent", true, Tags{}, "Album"), Entry("returns just name when tag is an empty slice", true, Tags{TagAlbumVersion: []string{}}, "Album"), + Entry("does not double parentheses when version is already parenthesized", true, Tags{TagAlbumVersion: []string{"(Remastered)"}}, "Album (Remastered)"), + Entry("does not add parentheses when version is wrapped in square brackets", true, Tags{TagAlbumVersion: []string{"[Remastered]"}}, "Album [Remastered]"), ) }) diff --git a/model/mediafile.go b/model/mediafile.go index 910cd998d..22ab7fbbe 100644 --- a/model/mediafile.go +++ b/model/mediafile.go @@ -100,18 +100,28 @@ type MediaFile struct { func (mf MediaFile) FullTitle() string { if conf.Server.Subsonic.AppendSubtitle && len(mf.Tags[TagSubtitle]) > 0 { - return fmt.Sprintf("%s (%s)", mf.Title, mf.Tags[TagSubtitle][0]) + return appendSuffix(mf.Title, mf.Tags[TagSubtitle][0]) } return mf.Title } func (mf MediaFile) FullAlbumName() string { if conf.Server.Subsonic.AppendAlbumVersion && len(mf.Tags[TagAlbumVersion]) > 0 { - return fmt.Sprintf("%s (%s)", mf.Album, mf.Tags[TagAlbumVersion][0]) + return appendSuffix(mf.Album, mf.Tags[TagAlbumVersion][0]) } return mf.Album } +var bracketPairs = map[byte]byte{'(': ')', '[': ']', '{': '}', '<': '>'} + +func appendSuffix(base, suffix string) string { + suffix = strings.TrimSpace(suffix) + if len(suffix) >= 2 && bracketPairs[suffix[0]] == suffix[len(suffix)-1] { + return base + " " + suffix + } + return base + " (" + suffix + ")" +} + func (mf MediaFile) ContentType() string { return mime.TypeByExtension("." + mf.Suffix) } diff --git a/model/mediafile_test.go b/model/mediafile_test.go index 6085ccf25..3f306f1a7 100644 --- a/model/mediafile_test.go +++ b/model/mediafile_test.go @@ -533,6 +533,13 @@ var _ = Describe("MediaFile", func() { Entry("returns just title when disabled", false, Tags{TagSubtitle: []string{"Live"}}, "Song"), Entry("returns just title when tag is absent", true, Tags{}, "Song"), Entry("returns just title when tag is an empty slice", true, Tags{TagSubtitle: []string{}}, "Song"), + Entry("does not double parentheses when subtitle is already parenthesized", true, Tags{TagSubtitle: []string{"(non-explicit version)"}}, "Song (non-explicit version)"), + Entry("does not add parentheses when subtitle is wrapped in square brackets", true, Tags{TagSubtitle: []string{"[Live]"}}, "Song [Live]"), + Entry("does not add parentheses when subtitle is wrapped in curly braces", true, Tags{TagSubtitle: []string{"{Remix}"}}, "Song {Remix}"), + Entry("does not add parentheses when subtitle is wrapped in angle brackets", true, Tags{TagSubtitle: []string{""}}, "Song "), + Entry("adds parentheses when brackets do not match", true, Tags{TagSubtitle: []string{"[Live)"}}, "Song ([Live))"), + Entry("trims surrounding whitespace before wrapping", true, Tags{TagSubtitle: []string{" Live "}}, "Song (Live)"), + Entry("trims whitespace around an already-bracketed subtitle", true, Tags{TagSubtitle: []string{" (Live) "}}, "Song (Live)"), ) DescribeTable("FullAlbumName", func(enabled bool, tags Tags, expected string) { @@ -544,6 +551,8 @@ var _ = Describe("MediaFile", func() { Entry("returns just album name when disabled", false, Tags{TagAlbumVersion: []string{"Deluxe Edition"}}, "Album"), Entry("returns just album name when tag is absent", true, Tags{}, "Album"), Entry("returns just album name when tag is an empty slice", true, Tags{TagAlbumVersion: []string{}}, "Album"), + Entry("does not double parentheses when version is already parenthesized", true, Tags{TagAlbumVersion: []string{"(Deluxe Edition)"}}, "Album (Deluxe Edition)"), + Entry("does not add parentheses when version is wrapped in square brackets", true, Tags{TagAlbumVersion: []string{"[Deluxe Edition]"}}, "Album [Deluxe Edition]"), ) Describe("CoverArtId", func() { It("returns its own id if it HasCoverArt", func() { diff --git a/plugins/README.md b/plugins/README.md index b9118d36f..08ab967f3 100644 --- a/plugins/README.md +++ b/plugins/README.md @@ -136,7 +136,7 @@ Every plugin must include a `manifest.json` file. Example: **Required fields:** `name`, `author`, `version` -**Optional fields:** `description`, `website`, `config`, `permissions`, `experimental` +**Optional fields:** `description`, `website`, `config`, `permissions` #### Config Definition @@ -160,24 +160,6 @@ The `config` field defines the plugin's configuration schema using [JSON Schema } ``` -#### Experimental Features - -Plugins can opt-in to experimental WebAssembly features that may change or be removed in future versions. Currently supported: - -- **`threads`** – Enables WebAssembly threads support (for plugins compiled with multi-threading) - -```json -{ - "experimental": { - "threads": { - "reason": "Required for concurrent audio processing" - } - } -} -``` - -> **Note:** Experimental features may have compatibility or performance implications. Use only when necessary. - --- ## Capabilities diff --git a/plugins/manager_loader.go b/plugins/manager_loader.go index 1969c9b2c..3ff721bd4 100644 --- a/plugins/manager_loader.go +++ b/plugins/manager_loader.go @@ -14,8 +14,6 @@ import ( "github.com/navidrome/navidrome/plugins/host" "github.com/navidrome/navidrome/scheduler" "github.com/tetratelabs/wazero" - "github.com/tetratelabs/wazero/api" - "github.com/tetratelabs/wazero/experimental" "golang.org/x/sync/errgroup" ) @@ -399,12 +397,6 @@ func (m *Manager) loadPluginWithConfig(p *model.Plugin) error { WithCompilationCache(m.cache). WithCloseOnContextDone(true) - // Enable experimental threads if requested in manifest - if pkg.Manifest.HasExperimentalThreads() { - runtimeConfig = runtimeConfig.WithCoreFeatures(api.CoreFeaturesV2 | experimental.CoreFeaturesThreads) - log.Debug(ctx, "Enabling experimental threads support") - } - extismConfig := extism.PluginConfig{ EnableWasi: true, RuntimeConfig: runtimeConfig, diff --git a/plugins/manifest-schema.json b/plugins/manifest-schema.json index b3fbe40f8..736677313 100644 --- a/plugins/manifest-schema.json +++ b/plugins/manifest-schema.json @@ -34,9 +34,6 @@ "permissions": { "$ref": "#/$defs/Permissions" }, - "experimental": { - "$ref": "#/$defs/Experimental" - }, "config": { "$ref": "#/$defs/ConfigDefinition" } @@ -58,27 +55,6 @@ } } }, - "Experimental": { - "type": "object", - "description": "Experimental features that may change or be removed in future versions", - "additionalProperties": false, - "properties": { - "threads": { - "$ref": "#/$defs/ThreadsFeature" - } - } - }, - "ThreadsFeature": { - "type": "object", - "description": "Enable experimental WebAssembly threads support", - "additionalProperties": false, - "properties": { - "reason": { - "type": "string", - "description": "Explanation for why threads support is needed" - } - } - }, "Permissions": { "type": "object", "description": "Permissions required by the plugin", diff --git a/plugins/manifest.go b/plugins/manifest.go index 17fef76a9..71e37a025 100644 --- a/plugins/manifest.go +++ b/plugins/manifest.go @@ -117,11 +117,6 @@ func ValidateWithCapabilities(m *Manifest, capabilities []Capability) error { return nil } -// HasExperimentalThreads returns true if the manifest requests experimental threads support. -func (m *Manifest) HasExperimentalThreads() bool { - return m.Experimental != nil && m.Experimental.Threads != nil -} - // HasLibraryFilesystemPermission checks if the manifest grants filesystem permission for libraries. func (m *Manifest) HasLibraryFilesystemPermission() bool { return m.Permissions != nil && diff --git a/plugins/manifest_gen.go b/plugins/manifest_gen.go index ae629c4e4..5e0035bd5 100644 --- a/plugins/manifest_gen.go +++ b/plugins/manifest_gen.go @@ -45,12 +45,6 @@ func (j *ConfigDefinition) UnmarshalJSON(value []byte) error { return nil } -// Experimental features that may change or be removed in future versions -type Experimental struct { - // Threads corresponds to the JSON schema field "threads". - Threads *ThreadsFeature `json:"threads,omitempty" yaml:"threads,omitempty" mapstructure:"threads,omitempty"` -} - // HTTP access permissions for a plugin type HTTPPermission struct { // Explanation for why HTTP access is needed @@ -109,9 +103,6 @@ type Manifest struct { // A brief description of what the plugin does Description *string `json:"description,omitempty" yaml:"description,omitempty" mapstructure:"description,omitempty"` - // Experimental corresponds to the JSON schema field "experimental". - Experimental *Experimental `json:"experimental,omitempty" yaml:"experimental,omitempty" mapstructure:"experimental,omitempty"` - // The display name of the plugin Name string `json:"name" yaml:"name" mapstructure:"name"` @@ -252,12 +243,6 @@ func (j *TaskQueuePermission) UnmarshalJSON(value []byte) error { return nil } -// Enable experimental WebAssembly threads support -type ThreadsFeature struct { - // Explanation for why threads support is needed - Reason *string `json:"reason,omitempty" yaml:"reason,omitempty" mapstructure:"reason,omitempty"` -} - // Users service permissions for accessing user information type UsersPermission struct { // Explanation for why users access is needed diff --git a/plugins/manifest_test.go b/plugins/manifest_test.go index 2a8b0dcfa..32bcbba08 100644 --- a/plugins/manifest_test.go +++ b/plugins/manifest_test.go @@ -117,76 +117,6 @@ var _ = Describe("Manifest", func() { }) }) - Describe("HasExperimentalThreads", func() { - It("returns false when no experimental section", func() { - m := &Manifest{} - Expect(m.HasExperimentalThreads()).To(BeFalse()) - }) - - It("returns false when experimental section has no threads", func() { - m := &Manifest{ - Experimental: &Experimental{}, - } - Expect(m.HasExperimentalThreads()).To(BeFalse()) - }) - - It("returns true when threads feature is present", func() { - m := &Manifest{ - Experimental: &Experimental{ - Threads: &ThreadsFeature{}, - }, - } - Expect(m.HasExperimentalThreads()).To(BeTrue()) - }) - - It("returns true when threads feature has a reason", func() { - m := &Manifest{ - Experimental: &Experimental{ - Threads: &ThreadsFeature{ - Reason: new("Required for concurrent processing"), - }, - }, - } - Expect(m.HasExperimentalThreads()).To(BeTrue()) - }) - - It("parses experimental.threads from JSON", func() { - data := []byte(`{ - "name": "Threaded Plugin", - "author": "Test Author", - "version": "1.0.0", - "experimental": { - "threads": { - "reason": "To use multi-threaded WASM module" - } - } - }`) - - var m Manifest - err := json.Unmarshal(data, &m) - Expect(err).ToNot(HaveOccurred()) - Expect(m.HasExperimentalThreads()).To(BeTrue()) - Expect(m.Experimental.Threads.Reason).ToNot(BeNil()) - Expect(*m.Experimental.Threads.Reason).To(Equal("To use multi-threaded WASM module")) - }) - - It("parses experimental.threads without reason from JSON", func() { - data := []byte(`{ - "name": "Threaded Plugin", - "author": "Test Author", - "version": "1.0.0", - "experimental": { - "threads": {} - } - }`) - - var m Manifest - err := json.Unmarshal(data, &m) - Expect(err).ToNot(HaveOccurred()) - Expect(m.HasExperimentalThreads()).To(BeTrue()) - }) - }) - Describe("ParseManifest", func() { It("parses a valid manifest with users permission", func() { data := []byte(`{ diff --git a/resources/i18n/it.json b/resources/i18n/it.json index b91c04064..656043589 100644 --- a/resources/i18n/it.json +++ b/resources/i18n/it.json @@ -17,7 +17,7 @@ "genre": "Genere", "compilation": "Compilation", "year": "Anno", - "size": "Dimensioni", + "size": "Dimensione file", "updatedAt": "Ultimo aggiornamento", "bitRate": "Bitrate", "bitDepth": "Profondità di bit", @@ -98,9 +98,9 @@ "lists": { "all": "Tutti", "random": "Casuali", - "recentlyAdded": "Aggiunti di Recente", - "recentlyPlayed": "Riprodotti di Recente", - "mostPlayed": "I Più Riprodotti", + "recentlyAdded": "Aggiunti di recente", + "recentlyPlayed": "Riprodotti di recente", + "mostPlayed": "I più riprodotti", "starred": "Preferiti", "topRated": "Più votati" } @@ -121,17 +121,17 @@ "roles": { "albumartist": "Artista Album |||| Artisti Album", "artist": "Artista |||| Artisti", - "composer": "Compositore |||| Compositori", - "conductor": "Direttore d'orchestra |||| Direttori d'orchestra", - "lyricist": "Paroliere |||| Parolieri", - "arranger": "Arrangiatore |||| Arrangiatori", - "producer": "Produttore |||| Produttori", - "director": "Direttore |||| Direttori", - "engineer": "Ingegnere del suono |||| Ingegneri del suono", + "composer": "Composizione |||| Composizione", + "conductor": "Direzione d'orchestra |||| Direzione d'orchestra", + "lyricist": "Testi |||| Testi", + "arranger": "Arrangiamento |||| Arrangiamento", + "producer": "Produzione |||| Produzione", + "director": "Direzione |||| Direzione", + "engineer": "Ingegneria del suono |||| Ingegneria del suono", "mixer": "Mixer |||| Mixer", "remixer": "Remixer |||| Remixer", "djmixer": "DJ Mixer |||| DJ Mixer", - "performer": "Esecutore |||| Esecutori", + "performer": "Esecuzione |||| Esecuzione", "maincredit": "Artista Album o Artista |||| Artisti Album o Artisti" }, "actions": { @@ -144,7 +144,7 @@ "name": "Utente |||| Utenti", "fields": { "userName": "Nome utente", - "isAdmin": "Amministratore", + "isAdmin": "Admin", "lastLoginAt": "Ultimo login", "lastAccessAt": "Ultimo accesso", "updatedAt": "Ultimo aggiornamento", @@ -152,13 +152,13 @@ "password": "Password", "createdAt": "Creato il", "changePassword": "Cambiare la password?", - "currentPassword": "Password Attuale", - "newPassword": "Nuova Password", + "currentPassword": "Password attuale", + "newPassword": "Nuova password", "token": "Token", "libraries": "Librerie" }, "helperTexts": { - "name": "Le modifiche effettuate al tuo nome verranno mostrate al prossimo accesso", + "name": "Le modifiche al tuo nome verranno mostrate solo al prossimo accesso", "libraries": "Seleziona librerie specifiche per questo utente, o lascia vuoto per usare le librerie predefinite" }, "notifications": { @@ -167,13 +167,13 @@ "deleted": "Utente eliminato" }, "validation": { - "librariesRequired": "Almeno una libreria deve essere selezionata per gli utenti non amministratori" + "librariesRequired": "Almeno una libreria deve essere selezionata per gli utenti non admin" }, "message": { - "listenBrainzToken": "Inserisci il tuo token utente ListenBrainz", + "listenBrainzToken": "Inserisci il tuo token utente ListenBrainz.", "clickHereForToken": "Clicca qui per ottenere il tuo token", "selectAllLibraries": "Seleziona tutte le librerie", - "adminAutoLibraries": "Gli utenti amministratori hanno automaticamente accesso a tutte le librerie" + "adminAutoLibraries": "Gli utenti admin hanno automaticamente accesso a tutte le librerie" } }, "player": { @@ -203,28 +203,29 @@ "fields": { "name": "Nome", "duration": "Durata", - "ownerName": "Creatore", + "ownerName": "Di", "public": "Pubblica", "updatedAt": "Ultimo aggiornamento", "createdAt": "Data creazione", "songCount": "Tracce", "comment": "Commento", "sync": "Importazione automatica", - "path": "Importa da" + "path": "Importa da", + "starred": "Preferita" }, "actions": { "selectPlaylist": "Seleziona una playlist:", "addNewPlaylist": "Crea \"%{name}\"", "export": "Esporta", "saveQueue": "Salva la coda nella playlist", - "makePublic": "Rendi Pubblica", - "makePrivate": "Rendi Privata", + "makePublic": "Rendi pubblica", + "makePrivate": "Rendi privata", "searchOrCreate": "Cerca playlist o digita per crearne una nuova...", "pressEnterToCreate": "Premi Invio per creare una nuova playlist", "removeFromSelection": "Rimuovi dalla selezione" }, "message": { - "duplicate_song": "Aggiungere i duplicati", + "duplicate_song": "Aggiungi tracce duplicate", "song_exist": "Si stanno aggiungendo dei duplicati nella playlist. Vuoi aggiungerli o saltarli?", "noPlaylistsFound": "Nessuna playlist trovata", "noPlaylists": "Nessuna playlist disponibile" @@ -331,7 +332,7 @@ "pathInvalid": "Percorso della libreria non valido" }, "messages": { - "deleteConfirm": "Sei sicuro di voler eliminare questa libreria? Verranno rimossi tutti i dati associati e gli accessi degli utenti.", + "deleteConfirm": "Vuoi eliminare questa libreria? Verranno rimossi tutti i dati associati e gli accessi degli utenti.", "scanInProgress": "Scansione in corso...", "noLibrariesAssigned": "Nessuna libreria assegnata a questo utente" } @@ -367,7 +368,7 @@ "configuration": "Configurazione", "manifest": "Manifest", "usersPermission": "Permessi utenti", - "libraryPermission": "Permesso libreria" + "libraryPermission": "Permessi librerie" }, "status": { "enabled": "Abilitato", @@ -400,10 +401,10 @@ "allUsersHelp": "Se abilitato, il plugin avrà accesso a tutti gli utenti, inclusi quelli creati in futuro.", "noUsers": "Nessun utente selezionato", "permissionReason": "Motivo", - "usersRequired": "Questo plugin richiede accesso alle informazioni degli utenti. Seleziona quali utenti il plugin può accedere, oppure abilita 'Consenti tutti gli utenti'.", + "usersRequired": "Questo plugin richiede accesso alle informazioni degli utenti. Seleziona a quali utenti il plugin può accedere, oppure abilita 'Consenti tutti gli utenti'.", "allLibrariesHelp": "Se abilitato, il plugin avrà accesso a tutte le librerie, incluse quelle create in futuro.", "noLibraries": "Nessuna libreria selezionata", - "librariesRequired": "Questo plugin richiede accesso alle informazioni delle librerie. Seleziona quali librerie il plugin può accedere, oppure abilita 'Consenti tutte le librerie'.", + "librariesRequired": "Questo plugin richiede accesso alle informazioni delle librerie. Seleziona a quali librerie il plugin può accedere, oppure abilita 'Consenti tutte le librerie'.", "allowWriteAccessHelp": "Se abilitato, il plugin può modificare i file nelle directory della libreria. Per impostazione predefinita, i plugin hanno accesso in sola lettura.", "requiredHosts": "Host richiesti" }, @@ -416,9 +417,9 @@ "ra": { "auth": { "welcome1": "Grazie per aver installato Navidrome!", - "welcome2": "Per iniziare, crea un amministratore", + "welcome2": "Per iniziare, crea un account amministratore", "confirmPassword": "Conferma la password", - "buttonCreateAdmin": "Crea amministratore", + "buttonCreateAdmin": "Crea admin", "auth_check_error": "Per favore accedi per continuare", "user_menu": "Profilo", "username": "Nome utente", @@ -488,8 +489,8 @@ "loading": "Caricamento in corso", "not_found": "Non trovato", "show": "%{name} #%{id}", - "empty": "Nessun %{name} per adesso.", - "invite": "Vuoi aggiungerne uno?" + "empty": "Ancora niente %{name}.", + "invite": "Vuoi aggiungerne?" }, "input": { "file": { @@ -512,10 +513,10 @@ }, "message": { "about": "Informazioni", - "are_you_sure": "Sei sicuro?", - "bulk_delete_content": "Sei sicuro di voler rimuovere questo %{name}? |||| Sei sicuro di voler rimuovere questi %{smart_count} elementi?", + "are_you_sure": "Vuoi procedere?", + "bulk_delete_content": "Vuoi rimuovere questo %{name}? |||| Vuoi rimuovere questi %{smart_count} elementi?", "bulk_delete_title": "Rimuovi %{name} |||| Rimuovi %{smart_count} %{name}", - "delete_content": "Sei sicuro di voler eliminare questo elemento?", + "delete_content": "Vuoi eliminare questo elemento?", "delete_title": "Rimuovi %{name} #%{id}", "details": "Dettagli", "error": "Un errore dal lato client ha impedito il completamento della tua richiesta.", @@ -524,7 +525,7 @@ "no": "No", "not_found": "Hai inserito un URL inesistente, oppure hai cliccato un link errato.", "yes": "Sì", - "unsaved_changes": "Alcune modifiche non sono state salvate. Sei sicuro di volerle ignorare?" + "unsaved_changes": "Alcune modifiche non sono state salvate. Vuoi ignorarle?" }, "navigation": { "no_results": "Nessun risultato trovato", @@ -574,11 +575,11 @@ "noTopSongsFound": "Nessun brano più ascoltato trovato", "noPlaylistsAvailable": "Nessuna disponibile", "delete_user_title": "Rimuovi utente '%{name}'", - "delete_user_content": "Sei sicuro di voler rimuovere questo utente e tutti i suoi dati (incluse playlist e impostazioni)?", + "delete_user_content": "Vuoi rimuovere questo utente e tutti i suoi dati (incluse playlist e impostazioni)?", "remove_missing_title": "Rimuovi i file mancanti", - "remove_missing_content": "Sei sicuro di voler rimuovere i file mancanti selezionati dal database? Verranno eliminati permanentemente tutti i riferimenti ad essi, inclusi i conteggi delle riproduzioni e le valutazioni.", + "remove_missing_content": "Vuoi rimuovere i file mancanti selezionati dal database? Verranno eliminati permanentemente tutti i riferimenti ad essi, inclusi i conteggi delle riproduzioni e le valutazioni.", "remove_all_missing_title": "Rimuovi tutti i file mancanti", - "remove_all_missing_content": "Sei sicuro di voler rimuovere tutti i file mancanti dal database? Verranno eliminati permanentemente tutti i riferimenti ad essi, inclusi i conteggi delle riproduzioni e le valutazioni.", + "remove_all_missing_content": "Vuoi rimuovere tutti i file mancanti dal database? Verranno eliminati permanentemente tutti i riferimenti ad essi, inclusi i conteggi delle riproduzioni e le valutazioni.", "notifications_blocked": "Hai bloccato le notifiche per questo sito nelle tue impostazioni del browser", "notifications_not_available": "Questo browser non supporta le notifiche desktop o non stai accedendo a Navidrome tramite HTTPS", "lastfmLinkSuccess": "Collegamento a Last.fm riuscito e scrobbling abilitato", @@ -619,7 +620,7 @@ "options": { "theme": "Tema", "language": "Lingua", - "defaultView": "Vista Predefinita", + "defaultView": "Vista predefinita", "desktop_notifications": "Notifiche desktop", "lastfmNotConfigured": "La chiave API di Last.fm non è configurata", "lastfmScrobbling": "Esegui lo scrobbling tramite Last.fm", @@ -635,21 +636,22 @@ }, "albumList": "Album", "playlists": "Playlist", - "sharedPlaylists": "Playlist Condivise", - "about": "Info" + "sharedPlaylists": "Playlist condivise", + "about": "Info", + "onlyFavourites": "Mostra solo i preferiti" }, "player": { "playListsText": "Coda", "openText": "Apri", "closeText": "Chiudi", - "notContentText": "Nessuna traccia", + "notContentText": "Niente musica", "clickToPlayText": "Clicca per riprodurre", "clickToPauseText": "Clicca per mettere in pausa", "nextTrackText": "Traccia successiva", "previousTrackText": "Traccia precedente", "reloadText": "Ricarica", "volumeText": "Volume", - "toggleLyricText": "Mostra testo", + "toggleLyricText": "Mostra/nascondi testo", "toggleMiniModeText": "Minimizza", "destroyText": "Distruggi", "downloadText": "Scarica", @@ -659,7 +661,7 @@ "playModeText": { "order": "In ordine", "orderLoop": "Ripeti", - "singleLoop": "Ripeti una volta", + "singleLoop": "Ripeti traccia", "shufflePlay": "Casuale" } }, @@ -667,7 +669,7 @@ "links": { "homepage": "Sito web", "source": "Codice sorgente", - "featureRequests": "Richieste", + "featureRequests": "Proponi idee", "lastInsightsCollection": "Ultima raccolta dati", "insights": { "disabled": "Disabilitato", @@ -693,7 +695,7 @@ }, "activity": { "title": "Attività", - "totalScanned": "Cartelle scansionate totali", + "totalScanned": "Totale cartelle scansionate", "quickScan": "Rapida", "fullScan": "Completa", "selectiveScan": "Selettiva", @@ -709,17 +711,17 @@ "minutesAgo": "%{smart_count} minuto fa |||| %{smart_count} minuti fa" }, "help": { - "title": "Scorciatoie da Tastiera di Navidrome", + "title": "Scorciatoie da tastiera di Navidrome", "hotkeys": { "show_help": "Mostra questa schermata", "toggle_menu": "Mostra/Nascondi la barra laterale", "toggle_play": "Riproduzione/Pausa", - "prev_song": "Traccia Precedente", - "next_song": "Traccia Successiva", + "prev_song": "Traccia precedente", + "next_song": "Traccia successiva", "current_song": "Vai alla traccia corrente", - "vol_up": "Alza il Volume", - "vol_down": "Abbassa il Volume", + "vol_up": "Alza il volume", + "vol_down": "Abbassa il volume", "toggle_love": "Aggiungi questa traccia ai preferiti" } } -} \ No newline at end of file +} diff --git a/server/subsonic/transcode.go b/server/subsonic/transcode.go index 511db2b85..9eb2af160 100644 --- a/server/subsonic/transcode.go +++ b/server/subsonic/transcode.go @@ -8,6 +8,7 @@ import ( "slices" "strconv" + "github.com/navidrome/navidrome/core/ffmpeg" "github.com/navidrome/navidrome/core/stream" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" @@ -315,7 +316,8 @@ func (api *Router) GetTranscodeDecision(w http.ResponseWriter, r *http.Request) decision, err := api.transcodeDecision.MakeDecision(ctx, mf, clientInfo, stream.TranscodeOptions{}) if err != nil { log.Error(ctx, "Failed to make transcode decision", "mediaID", mediaID, err) - return nil, newError(responses.ErrorGeneric, "failed to make transcode decision") + code, reason := transcodeFailure(err) + return nil, newError(code, "failed to make transcode decision: %s", reason) } // Only create a token when there is a valid playback path @@ -346,6 +348,19 @@ func (api *Router) GetTranscodeDecision(w http.ResponseWriter, r *http.Request) return response, nil } +// transcodeFailure maps a decision error to a Subsonic error code and a reason +// safe to send to clients, omitting server file paths. +func transcodeFailure(err error) (int32, string) { + pe, ok := errors.AsType[*ffmpeg.ProbeError](err) + if !ok { + return responses.ErrorGeneric, "internal error" + } + if pe.NotFound { + return responses.ErrorDataNotFound, pe.SafeReason() + } + return responses.ErrorGeneric, pe.SafeReason() +} + // GetTranscodeStream handles the OpenSubsonic getTranscodeStream endpoint. // It streams media using the decision encoded in the transcodeParams JWT token. // All errors are returned as proper HTTP status codes (not Subsonic error responses). diff --git a/server/subsonic/transcode_test.go b/server/subsonic/transcode_test.go index 7e36ab243..8d5cbb974 100644 --- a/server/subsonic/transcode_test.go +++ b/server/subsonic/transcode_test.go @@ -4,12 +4,16 @@ import ( "bytes" "context" "errors" + "fmt" + "io/fs" "net/http" "net/http/httptest" + "github.com/navidrome/navidrome/core/ffmpeg" "github.com/navidrome/navidrome/core/stream" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" + "github.com/navidrome/navidrome/server/subsonic/responses" "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -77,6 +81,47 @@ var _ = Describe("Transcode endpoints", func() { Expect(err.Error()).To(ContainSubstring("error retrieving media file")) }) + It("enriches the decision error with the reason, without leaking the file path", func() { + mockMFRepo.SetData(model.MediaFiles{{ID: "song-1", Suffix: "flac"}}) + mockTD.decisionErr = fmt.Errorf("probing media file song-1: %w", + &ffmpeg.ProbeError{Path: "/music/secret/foo.flac", Reason: "the file: Invalid data found when processing input"}) + r := newJSONPostRequest("mediaId=song-1&mediaType=song", "{}") + _, err := router.GetTranscodeDecision(w, r) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failed to make transcode decision")) + Expect(err.Error()).To(ContainSubstring("Invalid data found when processing input")) + Expect(err.Error()).ToNot(ContainSubstring("/music/secret")) + var subErr subError + Expect(errors.As(err, &subErr)).To(BeTrue()) + Expect(subErr.code).To(Equal(responses.ErrorGeneric)) + }) + + It("returns ErrorDataNotFound when the source file is missing on disk", func() { + mockMFRepo.SetData(model.MediaFiles{{ID: "song-1", Suffix: "flac"}}) + mockTD.decisionErr = fmt.Errorf("probing media file song-1: %w", + &ffmpeg.ProbeError{Path: "/music/gone.flac", Reason: "file not found", NotFound: true}) + r := newJSONPostRequest("mediaId=song-1&mediaType=song", "{}") + _, err := router.GetTranscodeDecision(w, r) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("file not found")) + var subErr subError + Expect(errors.As(err, &subErr)).To(BeTrue()) + Expect(subErr.code).To(Equal(responses.ErrorDataNotFound)) + }) + + It("keeps ErrorGeneric when ffprobe is missing, even though the cause wraps fs.ErrNotExist", func() { + mockMFRepo.SetData(model.MediaFiles{{ID: "song-1", Suffix: "flac"}}) + pe := &ffmpeg.ProbeError{Path: "/music/song.flac", Reason: "could not read file"} + mockTD.decisionErr = fmt.Errorf("probing media file song-1: %w (%w)", pe, fs.ErrNotExist) + Expect(errors.Is(mockTD.decisionErr, fs.ErrNotExist)).To(BeTrue()) + r := newJSONPostRequest("mediaId=song-1&mediaType=song", "{}") + _, err := router.GetTranscodeDecision(w, r) + Expect(err).To(HaveOccurred()) + var subErr subError + Expect(errors.As(err, &subErr)).To(BeTrue()) + Expect(subErr.code).To(Equal(responses.ErrorGeneric)) + }) + It("returns error when body is empty", func() { r := newJSONPostRequest("mediaId=song-1&mediaType=song", "") _, err := router.GetTranscodeDecision(w, r) @@ -516,6 +561,7 @@ func newJSONPostRequest(queryParams string, jsonBody string) *http.Request { // mockTranscodeDecision is a test double for stream.TranscodeDecider type mockTranscodeDecision struct { decision *stream.TranscodeDecision + decisionErr error token string tokenErr error resolvedReq stream.Request @@ -525,6 +571,9 @@ type mockTranscodeDecision struct { func (m *mockTranscodeDecision) MakeDecision(_ context.Context, _ *model.MediaFile, ci *stream.ClientInfo, _ stream.TranscodeOptions) (*stream.TranscodeDecision, error) { m.capturedClient = ci + if m.decisionErr != nil { + return nil, m.decisionErr + } if m.decision != nil { return m.decision, nil } diff --git a/ui/src/album/AlbumList.jsx b/ui/src/album/AlbumList.jsx index a860c85bb..5108bfaa1 100644 --- a/ui/src/album/AlbumList.jsx +++ b/ui/src/album/AlbumList.jsx @@ -6,7 +6,6 @@ import { Filter, NullableBooleanInput, NumberInput, - Pagination, ReferenceArrayInput, ReferenceInput, SearchInput, @@ -20,6 +19,7 @@ import FavoriteIcon from '@material-ui/icons/Favorite' import { withWidth } from '@material-ui/core' import { List, + Pagination, Title, useAlbumsPerPage, useResourceRefresh, diff --git a/ui/src/artist/ArtistShow.jsx b/ui/src/artist/ArtistShow.jsx index 935b0bab7..955a565d6 100644 --- a/ui/src/artist/ArtistShow.jsx +++ b/ui/src/artist/ArtistShow.jsx @@ -100,6 +100,7 @@ const ArtistShowLayout = (props) => { const rowsPerPageOptions = [1, 2, 3].map((option) => Math.trunc(option * (perPage / 3)), ) + // react-admin's Pagination on purpose: the common one would persist 30/60/90 under the album grid's key pagination = } diff --git a/ui/src/common/List.jsx b/ui/src/common/List.jsx index 72c2d9482..0ae089460 100644 --- a/ui/src/common/List.jsx +++ b/ui/src/common/List.jsx @@ -2,6 +2,7 @@ import React from 'react' import { List as RAList } from 'react-admin' import config from '../config' import { Pagination } from './Pagination' +import { defaultRowsPerPageOptions, getStoredPerPage } from './perPageStore' import { Title } from './index' export const List = (props) => { @@ -15,7 +16,7 @@ export const List = (props) => { /> } debounce={config.uiSearchDebounceMs} - perPage={15} + perPage={getStoredPerPage(resource, defaultRowsPerPageOptions)} pagination={} {...props} /> diff --git a/ui/src/common/List.test.jsx b/ui/src/common/List.test.jsx new file mode 100644 index 000000000..5bc4b910f --- /dev/null +++ b/ui/src/common/List.test.jsx @@ -0,0 +1,27 @@ +import React from 'react' +import { render, screen } from '@testing-library/react' +import { describe, it, expect, vi } from 'vitest' +import { List } from './List' + +// Only stub the heavy react-admin List controller (data fetching, router sync); +// everything else, including our own Pagination/perPageStore wiring, stays real +// so a bad import (the bug this test guards against) throws on render. +vi.mock('react-admin', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + List: ({ children }) =>
{children}
, + } +}) + +describe('List', () => { + it('renders without throwing and shows its children', () => { + render( + +
list content
+ , + ) + expect(screen.getByTestId('ra-list')).toBeInTheDocument() + expect(screen.getByText('list content')).toBeInTheDocument() + }) +}) diff --git a/ui/src/common/Pagination.jsx b/ui/src/common/Pagination.jsx index e17d9e63e..dd18961ce 100644 --- a/ui/src/common/Pagination.jsx +++ b/ui/src/common/Pagination.jsx @@ -1,6 +1,29 @@ -import React from 'react' -import { Pagination as RAPagination } from 'react-admin' +import React, { useCallback } from 'react' +import { + Pagination as RAPagination, + useListPaginationContext, +} from 'react-admin' +import { setStoredPerPage, defaultRowsPerPageOptions } from './perPageStore' -export const Pagination = (props) => ( - -) +export const Pagination = ({ + rowsPerPageOptions = defaultRowsPerPageOptions, + ...props +}) => { + const { resource, setPerPage } = useListPaginationContext() + // Persist only a selector-driven change: mount, URL params and responsive + // fallbacks never call setPerPage, so they can't overwrite the preference. + const handleSetPerPage = useCallback( + (value) => { + if (resource) setStoredPerPage(resource, value) + setPerPage(value) + }, + [resource, setPerPage], + ) + return ( + + ) +} diff --git a/ui/src/common/Pagination.test.jsx b/ui/src/common/Pagination.test.jsx new file mode 100644 index 000000000..a488aba91 --- /dev/null +++ b/ui/src/common/Pagination.test.jsx @@ -0,0 +1,62 @@ +import React from 'react' +import { render, screen, fireEvent } from '@testing-library/react' +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { Pagination } from './Pagination' + +// stub RA's Pagination so a test can invoke the injected setPerPage, i.e. +// simulate an actual rows-per-page selection +vi.mock('react-admin', async () => { + const React = await vi.importActual('react') + return { + Pagination: ({ setPerPage }) => + React.createElement( + 'button', + { onClick: () => setPerPage(50) }, + 'select 50', + ), + useListPaginationContext: vi.fn(), + } +}) + +describe('Pagination', () => { + let mockContext + let setPerPage + + beforeEach(async () => { + vi.clearAllMocks() + localStorage.clear() + setPerPage = vi.fn() + const { useListPaginationContext } = await import('react-admin') + mockContext = vi.mocked(useListPaginationContext) + }) + + const selectPerPage = () => fireEvent.click(screen.getByText('select 50')) + + it('persists the page size chosen in the selector', () => { + mockContext.mockReturnValue({ resource: 'song', perPage: 15, setPerPage }) + render() + selectPerPage() + expect(localStorage.getItem('perPage.song')).toEqual('50') + }) + + it('still applies the change to the list', () => { + mockContext.mockReturnValue({ resource: 'song', perPage: 15, setPerPage }) + render() + selectPerPage() + expect(setPerPage).toHaveBeenCalledWith(50) + }) + + it('does not persist a page size the user did not select', () => { + mockContext.mockReturnValue({ resource: 'song', perPage: 15, setPerPage }) + render() + expect(localStorage.getItem('perPage.song')).toBeNull() + }) + + it('does not persist without a resource in context', () => { + mockContext.mockReturnValue({ perPage: 15, setPerPage }) + render() + selectPerPage() + expect(localStorage.getItem('perPage.undefined')).toBeNull() + expect(setPerPage).toHaveBeenCalledWith(50) + }) +}) diff --git a/ui/src/common/index.js b/ui/src/common/index.js index 362a0ced3..ac8d7f62c 100644 --- a/ui/src/common/index.js +++ b/ui/src/common/index.js @@ -10,6 +10,7 @@ export * from './DurationField' export * from './List' export * from './MultiLineTextField' export * from './Pagination' +export * from './perPageStore' export * from './PlayButton' export * from './QuickFilter' export * from './RangeField' diff --git a/ui/src/common/perPageStore.js b/ui/src/common/perPageStore.js new file mode 100644 index 000000000..52a8a8f29 --- /dev/null +++ b/ui/src/common/perPageStore.js @@ -0,0 +1,11 @@ +export const defaultRowsPerPageOptions = [15, 25, 50] + +const key = (resource) => `perPage.${resource}` + +export const getStoredPerPage = (resource, options, fallback = options[0]) => { + const stored = parseInt(localStorage.getItem(key(resource)), 10) + return options.includes(stored) ? stored : fallback +} + +export const setStoredPerPage = (resource, perPage) => + localStorage.setItem(key(resource), String(perPage)) diff --git a/ui/src/common/perPageStore.test.js b/ui/src/common/perPageStore.test.js new file mode 100644 index 000000000..3aeba3ad8 --- /dev/null +++ b/ui/src/common/perPageStore.test.js @@ -0,0 +1,40 @@ +import { describe, it, expect, beforeEach } from 'vitest' +import { getStoredPerPage, setStoredPerPage } from './perPageStore' + +const options = [15, 25, 50] + +describe('perPageStore', () => { + beforeEach(() => { + localStorage.clear() + }) + + it('round-trips a stored value', () => { + setStoredPerPage('song', 25) + expect(getStoredPerPage('song', options, 15)).toEqual(25) + }) + + it('keys values per resource', () => { + setStoredPerPage('song', 25) + setStoredPerPage('playlist', 50) + expect(getStoredPerPage('song', options, 15)).toEqual(25) + expect(getStoredPerPage('playlist', options, 15)).toEqual(50) + }) + + it('returns the fallback when nothing is stored', () => { + expect(getStoredPerPage('song', options, 15)).toEqual(15) + }) + + it('returns the fallback for garbage values', () => { + localStorage.setItem('perPage.song', 'bogus') + expect(getStoredPerPage('song', options, 15)).toEqual(15) + }) + + it('returns the fallback when the stored value is not a valid option', () => { + setStoredPerPage('album', 90) + expect(getStoredPerPage('album', [18, 36, 72], 18)).toEqual(18) + }) + + it('defaults the fallback to the first option', () => { + expect(getStoredPerPage('song', options)).toEqual(15) + }) +}) diff --git a/ui/src/common/useAlbumsPerPage.jsx b/ui/src/common/useAlbumsPerPage.jsx index 6a02bdeb7..0fb5616c3 100644 --- a/ui/src/common/useAlbumsPerPage.jsx +++ b/ui/src/common/useAlbumsPerPage.jsx @@ -1,4 +1,5 @@ import { useSelector } from 'react-redux' +import { getStoredPerPage } from './perPageStore' const getPerPage = (width) => { if (width === 'xs') return 12 @@ -17,10 +18,15 @@ const getPerPageOptions = (width) => { } export const useAlbumsPerPage = (width) => { - const perPage = - useSelector( - (state) => state?.admin.resources?.album?.list?.params?.perPage, - ) || getPerPage(width) + const options = getPerPageOptions(width) + const sessionPerPage = useSelector( + (state) => state?.admin.resources?.album?.list?.params?.perPage, + ) + // Use the session value only when it's valid for the current width, so a + // size picked at a wider breakpoint can't leave an out-of-range selector. + const perPage = options.includes(sessionPerPage) + ? sessionPerPage + : getStoredPerPage('album', options, getPerPage(width)) - return [perPage, getPerPageOptions(width)] + return [perPage, options] } diff --git a/ui/src/common/useAlbumsPerPage.test.jsx b/ui/src/common/useAlbumsPerPage.test.jsx new file mode 100644 index 000000000..b194a6ef2 --- /dev/null +++ b/ui/src/common/useAlbumsPerPage.test.jsx @@ -0,0 +1,61 @@ +import { renderHook } from '@testing-library/react-hooks' +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { useAlbumsPerPage } from './useAlbumsPerPage' +import { setStoredPerPage } from './perPageStore' + +vi.mock('react-redux', () => ({ + useSelector: vi.fn(), +})) + +describe('useAlbumsPerPage', () => { + let mockUseSelector + + beforeEach(async () => { + vi.clearAllMocks() + localStorage.clear() + const { useSelector } = await import('react-redux') + mockUseSelector = vi.mocked(useSelector) + }) + + const setReduxPerPage = (value) => + mockUseSelector.mockImplementation((selector) => + selector({ + admin: { + resources: { album: { list: { params: { perPage: value } } } }, + }, + }), + ) + + it('prefers the redux session value over the stored one', () => { + setReduxPerPage(36) + setStoredPerPage('album', 72) + const { result } = renderHook(() => useAlbumsPerPage('lg')) + expect(result.current[0]).toEqual(36) + }) + + it('falls back to the stored value on fresh load', () => { + setReduxPerPage(undefined) + setStoredPerPage('album', 72) + const { result } = renderHook(() => useAlbumsPerPage('lg')) + expect(result.current[0]).toEqual(72) + }) + + it('ignores stored values invalid for the current width', () => { + setReduxPerPage(undefined) + setStoredPerPage('album', 72) // valid for lg, not for md + const { result } = renderHook(() => useAlbumsPerPage('md')) + expect(result.current[0]).toEqual(12) + }) + + it('returns the responsive default when nothing is stored', () => { + setReduxPerPage(undefined) + const { result } = renderHook(() => useAlbumsPerPage('xl')) + expect(result.current).toEqual([36, [18, 36, 72]]) + }) + + it('ignores a redux value invalid for the current width', () => { + setReduxPerPage(72) // valid for lg, not for md + const { result } = renderHook(() => useAlbumsPerPage('md')) + expect(result.current[0]).toEqual(12) + }) +}) diff --git a/ui/src/missing/MissingFilesList.jsx b/ui/src/missing/MissingFilesList.jsx index 87d9f629f..a09f552d9 100644 --- a/ui/src/missing/MissingFilesList.jsx +++ b/ui/src/missing/MissingFilesList.jsx @@ -1,10 +1,15 @@ -import { List, SizeField, useResourceRefresh } from '../common/index' +import { + List, + Pagination, + SizeField, + getStoredPerPage, + useResourceRefresh, +} from '../common/index' import { Datagrid, DateField, TextField, downloadCSV, - Pagination, Filter, ReferenceInput, useTranslate, @@ -49,8 +54,10 @@ const BulkActionButtons = (props) => ( ) +const missingPerPageOptions = [50, 100, 200] + const MissingPagination = (props) => ( - + ) const MissingFilesList = (props) => { @@ -63,7 +70,7 @@ const MissingFilesList = (props) => { actions={} filters={} bulkActionButtons={} - perPage={50} + perPage={getStoredPerPage('missing', missingPerPageOptions)} pagination={} > diff --git a/ui/src/playlist/PlaylistShow.jsx b/ui/src/playlist/PlaylistShow.jsx index f0cb472b1..4e269be18 100644 --- a/ui/src/playlist/PlaylistShow.jsx +++ b/ui/src/playlist/PlaylistShow.jsx @@ -4,14 +4,21 @@ import { ShowContextProvider, useShowContext, useShowController, - Pagination, Title as RaTitle, } from 'react-admin' import { makeStyles } from '@material-ui/core/styles' import PlaylistDetails from './PlaylistDetails' import PlaylistSongs from './PlaylistSongs' import PlaylistActions from './PlaylistActions' -import { Title, canChangeTracks, useResourceRefresh } from '../common' +import { + Pagination, + Title, + canChangeTracks, + getStoredPerPage, + useResourceRefresh, +} from '../common' + +const playlistTrackPerPageOptions = [100, 250, 500] const useStyles = makeStyles( (theme) => ({ @@ -41,7 +48,10 @@ const PlaylistShowLayout = (props) => { reference="playlistTrack" target="playlist_id" sort={{ field: 'id', order: 'ASC' }} - perPage={100} + perPage={getStoredPerPage( + 'playlistTrack', + playlistTrackPerPageOptions, + )} filter={{ playlist_id: props.id }} > { } resource={'playlistTrack'} exporter={false} - pagination={} + pagination={ + + } /> )} diff --git a/ui/src/radio/RadioList.jsx b/ui/src/radio/RadioList.jsx index 945bac519..ccdb9f1ef 100644 --- a/ui/src/radio/RadioList.jsx +++ b/ui/src/radio/RadioList.jsx @@ -16,6 +16,8 @@ import { } from 'react-admin' import { List, + defaultRowsPerPageOptions, + getStoredPerPage, useImageUrl, ToggleFieldsMenu, useSelectedFields, @@ -135,7 +137,11 @@ const RadioList = ({ permissions, ...props }) => { hasCreate={isAdmin} actions={} filters={} - perPage={isXsmall ? 25 : 10} + perPage={getStoredPerPage( + 'radio', + defaultRowsPerPageOptions, + isXsmall ? 25 : 10, + )} > {isXsmall ? ( { bulkActionButtons={} actions={} filters={} - perPage={isXsmall ? 50 : 15} + perPage={getStoredPerPage( + 'song', + defaultRowsPerPageOptions, + isXsmall ? 50 : 15, + )} > {isXsmall ? (