mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-18 07:30:29 +00:00
7 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Deluan Quintão
|
758e64c999
|
feat(scanner): per-library PID configuration (#6252)
* feat(model): add per-library PID config columns * refactor(metadata): pass PID config to ToMediaFile and add spec validation * feat(scanner): rescan only libraries whose PID config changed * feat(server): validate library PID config and rescan on change * feat(ui): edit per-library PID config * fix(ui): label the PID mode selects * fix: tighten per-library PID rescan edge cases An interrupted PID rescan no longer upgrades every library to a full scan, a save that loses the race for the scanner logs at debug, the confirm dialog only shows when the effective PID spec changes, and it now gets translation keys. * refactor(metadata): pass the library to ToMediaFile ToMediaFile and core.Inspect took the library ID and its PID config as separate arguments, so a caller could mix values from two libraries. They now take the model.Library and resolve the effective PID config from it. * chore: tidy per-library PID comments, PropTypes and migration Trim comments that restated the code, add PropTypes to the new UI components, and recreate the migration with make migration-sql. * fix(ui): show the PID spec help under its input * feat(cmd): make inspect use the file's library PID config inspect always used the global PID config, so it showed different IDs than the scanner for files in a library with an override. It now finds the file's library in the DB and uses its effective config, falling back to the global config when there is no DB or the file is outside every library. It never creates a DB. The library path matcher moves from core/playlists to model so both can use it. * refactor: simplify per-library PID code Share the DB-file check between CLI commands, move ErrAlreadyScanning to model so core no longer imports scanner, read the libraries once for insights, and let ValidatePIDSpec accept an empty spec and look tags up directly. In the scanner, use FullScanInProgress instead of a second flag, and skip recomputing album IDs when the album spec did not change. In the UI, share the PID inputs between Create and Edit, and use docsUrl. * feat(ui): add section titles to Library Create and pre-fill Custom PID specs Custom now starts from the global spec, so admins edit a working spec instead of typing one from scratch. * fix(inspect): map files with the library-relative path the scanner uses Inspect gave metadata the file's directory as typed, so folder-based PIDs never matched the DB. It now uses the path relative to the library root, through the scanner's helper, which moves to model. * fix(scanner): say when a PID rescan only covers target folders * fix: reject tag aliases in album PID specs and match root libraries Tags are stored under canonical names, so an alias in a spec always reads as empty. In an album spec that gives every album the same ID, so album specs now require the tag name. Track specs keep accepting aliases, since the default one uses them. LibraryMatcher now matches paths under a library at the filesystem root. * refactor(model): move the tag alias lookup to tag_mappings.go * test: run the library matcher and inspect tests on Windows Build test paths with filepath instead of Unix literals, so they use the OS separator like filepath.Abs output, and drop the Windows skips. * feat(ui): add pt-BR translations for per-library PID settings |
||
|
Deluan Quintão
|
b293b96256
|
refactor(persistence): stateless repositories with per-call context (#6149)
* refactor(persistence): adopt generic deluan/rest repository API Pin deluan/rest to the refactor branch. REST-facing repository methods take a context and return typed values. Drop DataStore.Resource and ResourceRepository; the native API names typed repositories directly through a per-request adapter that later commits remove. * refactor(persistence): base repository helpers take a context * refactor(persistence): LibraryRepository takes a context per call * refactor(persistence): PropertyRepository takes a context per call * refactor(persistence): UserPropsRepository takes a context per call * refactor(persistence): TranscodingRepository takes a context per call * refactor(persistence): ShareRepository takes a context per call * refactor(persistence): PlayerRepository takes a context per call * refactor(persistence): RadioRepository takes a context per call * refactor(persistence): PlayQueueRepository takes a context per call * refactor(persistence): Tag and Genre repositories take a context per call * refactor(persistence): PluginRepository takes a context per call * refactor(persistence): Scrobble repositories take a context per call * refactor(persistence): FolderRepository takes a context per call * refactor(persistence): Artwork repositories take a context per call * refactor(persistence): UserRepository takes a context per call * refactor(persistence): ArtistRepository takes a context per call ReadAll no longer rewrites the shared sort mappings for the role filter; it works on a per-call copy. * test(persistence): assert artist role sort sanitization in ReadAll * refactor(persistence): AlbumRepository takes a context per call * test(persistence): pass the test context to album repository helpers * refactor(persistence): MediaFileRepository takes a context per call * refactor(persistence): Playlist repositories take a context per call * refactor(persistence): build all repositories once per store * refactor(core): REST repository wrappers are built once * refactor(persistence): repositories are stateless Remove the context field from the base repository and the per-request REST adapter. Enable the containedctx linter so no repository can hold a request context again. * chore(lint): skip containedctx in test files * refactor: share simplifications from the stateless repositories sweep Add deleteOwnedAll on sqlRepository and use it in player/share Delete to remove the duplicated bulk-delete loop; have Share.Repository() return model.ShareRepository so subsonic sharing.go drops its repeated type assertions. * chore(core): assert REST wrappers implement Persistable * chore: reformat imports * perf(persistence): build repositories on first use Each transaction store used to construct all 21 repositories up front, paying for filter and sort mapping setup the block never touched. Fields are now sync.OnceValue thunks, so a store only builds what it uses. * fix(persistence): clean plugin references per deleted user A bulk user delete that fails on a later id had already removed the earlier rows but skipped their plugin cleanup. Cleanup now runs right after each successful delete. * fix(core): unload disabled plugins even when a user delete fails A bulk delete can fail on a later id after earlier users were removed and their plugins auto-disabled. The wrapper returned before unloading, leaving those plugins running until the next successful delete or a restart. * chore(deps): pin deluan/rest to v1.0.1 Replaces the pseudo-version of the refactor branch with the tagged release. REST error messages now name the bare type (Artist, not model.Artist). * test: use the spec context instead of context.Background() Replace the context.Background()/context.TODO() calls this branch added to tests with the spec's ctx, GinkgoT().Context(), or t/b.Context(), so repository calls are bound to the running spec's lifetime. * test: declare the spec context once per Describe Set ctx from GinkgoT().Context() first in each top-level BeforeEach and reuse it, building user contexts on top of it instead of repeating inline calls. |
||
|
Deluan Quintão
|
2e03766a9d
|
fix(playlist): preserve smart playlist song count on re-import (#5907) (#5908)
* fix(playlist): preserve smart playlist counters on re-import (#5907) * perf(playlist): skip re-importing unchanged NSP files (#5907) * feat(playlist): also store content hash for M3U imports (unused for now) * fix(playlist): return stored record when skipping unchanged NSP import Skipping before copying the stored identity broke the ImportFile(sync=false) contract: callers received an ID-less playlist and the requested Sync change was silently dropped. * refactor(playlist): hash imports once at the caller; protect smart counters in Put Move content hashing out of both parsers into the code that owns the file (parsePlaylist and ImportFile), removing the NSP double-buffer and the duplicated hashing idiom. Put now drops song_count/duration/size for smart playlists (PostMapArgs), disarming the counter-zeroing trap for all callers. * fix(playlist): invalidate imported hash when rules are edited via API Without this, a rules edit through the REST API kept the stored file hash, so every scan skipped the unchanged file and never restored the file-backed rules while sync was on. * test(playlist): verify smart counters survive a re-import, end to end The existing Put test seeds the stored counters with a raw SQL update, so it pins the guard in PostMapArgs but not the pipeline around it. This test drives the counters through a real evaluation instead: it saves a smart playlist, reads it with GetWithTracks to populate song_count/duration/size, then saves the playlist the way the scanner rebuilds it after parsing the .nsp file, with the counters back at zero. Both routes fail without the guard, and the new one covers the exact sequence reported in #5907. Test taken from #5970, which diagnosed the same root cause independently. Co-authored-by: Junker der Provinz <133605895+junkerderprovinz@users.noreply.github.com> * test(playlist): build the service with artwork.NewUploader The artwork pipeline in #5847 replaced core.NewImageUploadService() with artwork.NewUploader(ds) and updated every call site it could see. The five call sites this branch adds were written against the old constructor, so the merge applied cleanly but left the package uncompilable. * fix(db): re-stamp the imported_hash migration after the master merge Master gained three migrations while this branch was open, the newest being 20260816180040. The original 20260808200333 stamp now sorts before them, so any database already upgraded past that point would skip this migration entirely and never get the imported_hash column. Same SQL, current timestamp. * refactor(playlist): hash imported playlists with xxh3 and the id encoding ImportedHash is a change detector, not a security boundary, so it does not need a cryptographic digest. xxh3 is already a direct dependency and is used the same way to fingerprint files in the artwork image store. Encoding the 128-bit digest with id.Encode stores it in the same 22-char base62 form as every other id in the schema, down from 64 hex chars. No migration is needed: the imported_hash column has not shipped in a release, so no database holds a value in the old format. * refactor(playlist): extract the imported-playlist fingerprint helper Both import paths encoded the hash inline, so how a playlist file is fingerprinted lived in two places. A third import path that encoded it differently would silently never match the stored value, turning the unchanged-file skip into a no-op. --------- Co-authored-by: Junker der Provinz <133605895+junkerderprovinz@users.noreply.github.com> |
||
|
Deluan Quintão
|
85132240e0
|
feat(smartplaylist): per-playlist refreshDelay for stable daily/weekly playlists (#5790)
* feat(utils): add ParseDuration/FormatDuration with day and week units * feat(criteria): add per-playlist refreshDelay to smart playlist rules * feat(smartplaylist): honor per-playlist refreshDelay in refresh gate * feat(subsonic): compute smart playlist validUntil from effective refresh delay * fix(playlists): reset smart playlist evaluation window when rules change via API * refactor(utils): flatten FormatDuration recursion, single-pass duration regex * fix(playlists): address PR review feedback - Reset EvaluatedAt to nil instead of zero-time on rules change and NSP re-import, so getPlaylist(s) never reports year-1 Changed/validUntil in the window between an edit and the next owner read - Parse negative d/w durations so they are rejected with the consistent "negative duration" error instead of "unknown unit" - Quote input in the negative-duration error, matching the parse error - Gate per-playlist RefreshDelay behind IsSmartPlaylist, matching its doc |
||
|
Deluan Quintão
|
5d1c9530ab
|
feat(cli): add pls export/import subcommands for bulk playlist management (#5412)
* refactor: rename ImportFile to ImportFromFolder in playlists service * feat: add ImportFile method with library/folder resolution * feat: allow sync flag upgrade on re-import of non-synced playlists * feat: add pls export subcommand with bulk and single export Add `navidrome pls export` command that supports: - Single playlist export to stdout (-p flag only) - Single playlist export to directory (-p and -o flags) - Bulk export of all playlists to a directory (-o flag only) - Filtering by user (-u flag) - Automatic filename sanitization and collision detection Also extracts findPlaylist helper from runExporter for reuse. * feat: add pls import subcommand with sync flag support * fix: improve error message for export without output directory * test: add tests for ImportFile sync flag and sync upgrade behavior * refactor: streamline export and import logic by removing redundant comments and improving library path matching Signed-off-by: Deluan <deluan@navidrome.org> * feat: update ImportFile method to include sync flag for playlist imports Signed-off-by: Deluan <deluan@navidrome.org> * feat: implement fetchPlaylists function to streamline playlist retrieval Signed-off-by: Deluan <deluan@navidrome.org> * feat: replace inline filename sanitization with centralized utility function Signed-off-by: Deluan <deluan@navidrome.org> * feat: refactor playlist import logic to consolidate sync handling and improve method signatures Signed-off-by: Deluan <deluan@navidrome.org> * fix: address code review feedback on playlist import/export - Fix duplicate playlist creation on non-sync re-import: only reconcile sync flag when the playlist was actually persisted (has an ID) - Distinguish "not in any library" from real errors in resolveFolder using a sentinel error, so DB/folder errors propagate instead of falling back to ImportM3U - Use bufio.Scanner in countM3UTrackLines instead of reading entire file * feat: replace bufio.Scanner with UTF8Reader and LinesFrom utility for improved file reading Signed-off-by: Deluan <deluan@navidrome.org> * fix: record path for outside-library imports to prevent duplicates Files outside all libraries now go through updatePlaylist with the absolute path recorded, so re-importing the same file updates the existing playlist instead of creating a duplicate. * refactor: name guard condition in updatePlaylist for readability Extracted the compound boolean expression into a named local variable `alreadyImportedAndNotSynced` to make the intent of the early-return guard clearer at a glance. * add godocs Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
Deluan Quintão
|
acd69f6a4f
|
feat(playlist): support #EXTALBUMARTURL directive and sidecar images (#5131)
* feat(playlist): add migration for playlist image field rename and external URL * refactor(playlist): rename ImageFile to UploadedImage and ArtworkPath to UploadedImagePath Rename playlist model fields and methods for clarity in preparation for adding external image URL and sidecar image support. Add the new ExternalImageURL field to the Playlist model. * feat(playlist): parse #EXTALBUMARTURL directive in M3U imports * feat(playlist): always sync ExternalImageURL on re-scan, preserve UploadedImage * feat(artwork): add sidecar image discovery and cache invalidation for playlists Add playlist sidecar image support to the artwork reader fallback chain. A sidecar image (e.g., MyPlaylist.jpg next to MyPlaylist.m3u) is discovered via case-insensitive base name matching using model.IsImageFile(). Cache invalidation uses max(playlist.UpdatedAt, imageFile.ModTime()) to bust stale artwork when sidecar or ExternalImageURL local files change. * feat(artwork): add external image URL source to playlist artwork reader Add fromPlaylistExternalImage source function that resolves playlist cover art from ExternalImageURL, supporting both HTTP(S) URLs (via the existing fromURL helper) and local file paths (via os.Open). Insert it in the Reader() fallback chain between sidecar and tiled cover. * refactor(artwork): simplify playlist artwork source functions Extract shared fromLocalFile helper, use url.Parse for scheme check, and collapse sidecar directory scan conditions. * test(artwork): remove redundant fromPlaylistSidecar tests These tests duplicated scenarios already covered by findPlaylistSidecarPath tests combined with fromLocalFile (tested via fromPlaylistExternalImage). After refactoring fromPlaylistSidecar to a one-liner composing those two functions, the wrapper tests add no value. * fix(playlist): address security review comments from PR #5131: - Use url.PathUnescape instead of url.QueryUnescape for file:// URLs so that '+' in filenames is preserved (not decoded as space). - Validate all local image paths (file://, absolute, relative) against known library boundaries via libraryMatcher, rejecting paths outside any configured library. - Harden #EXTALBUMARTURL against path traversal and SSRF by adding EnableM3UExternalAlbumArt config flag (default false, also disabled by EnableExternalServices=false) to gate HTTP(S) URL storage at parse time and fetching at read time (defense in depth). - Log a warning when os.ReadDir fails in findPlaylistSidecarPath for diagnosability. - Extract resolveLocalPath helper to simplify resolveImageURL. Signed-off-by: Deluan <deluan@navidrome.org> * feat(playlist): implement human-friendly filename generation for uploaded playlist cover images Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
Deluan Quintão
|
7ad2907719
|
refactor: move playlist business logic from repositories to service layer (#5027)
* refactor: move playlist business logic from repositories to core.Playlists service Move authorization, permission checks, and orchestration logic from playlist repositories to the core.Playlists service, following the existing pattern used by core.Share and core.Library. Changes: - Expand core.Playlists interface with read, mutation, track management, and REST adapter methods - Add playlistRepositoryWrapper for REST Save/Update/Delete with permission checks (follows Share/Library pattern) - Simplify persistence/playlist_repository.go: remove isWritable(), auth checks from Delete()/Put()/updatePlaylist() - Simplify persistence/playlist_track_repository.go: remove isTracksEditable() and permission checks from Add/Delete/Reorder - Update Subsonic API handlers to route through service - Update Native API handlers to accept core.Playlists instead of model.DataStore * test: add coverage for playlist service methods and REST wrapper Add 30 new tests covering the service methods added during the playlist refactoring: - Delete: owner, admin, denied, not found - Create: new playlist, replace tracks, admin bypass, denied, not found - AddTracks: owner, admin, denied, smart playlist, not found - RemoveTracks: owner, smart playlist denied, non-owner denied - ReorderTrack: owner, smart playlist denied - NewRepository wrapper: Save (owner assignment, ID clearing), Update (owner, admin, denied, ownership change, not found), Delete (delegation with permission checks) Expand mockedPlaylistRepo with Get, Delete, Tracks, GetWithTracks, and rest.Persistable methods. Add mockedPlaylistTrackRepo for track operation verification. * fix: add authorization check to playlist Update method Added ownership verification to the Subsonic Update endpoint in the playlist service layer. The authorization check was present in the old repository code but was not carried over during the refactoring to the service layer, allowing any authenticated user to modify playlists they don't own via the Subsonic API. Also added corresponding tests for the Update method's permission logic. * refactor: improve playlist permission checks and error handling, add e2e tests Signed-off-by: Deluan <deluan@navidrome.org> * refactor: rename core.Playlists to playlists package and update references Signed-off-by: Deluan <deluan@navidrome.org> * refactor: rename playlists_internal_test.go to parse_m3u_test.go and update tests; add new parse_nsp.go and rest_adapter.go files Signed-off-by: Deluan <deluan@navidrome.org> * fix: block track mutations on smart playlists in Create and Update Create now rejects replacing tracks on smart playlists (pre-existing gap). Update now uses checkTracksEditable instead of checkWritable when track changes are requested, restoring the protection that was removed from the repository layer during the refactoring. Metadata-only updates on smart playlists remain allowed. * test: add smart playlist protection tests to ensure readonly behavior and mutation restrictions * refactor: optimize track removal and renumbering in playlists Signed-off-by: Deluan <deluan@navidrome.org> * refactor: implement track reordering in playlists with SQL updates Signed-off-by: Deluan <deluan@navidrome.org> * refactor: wrap track deletion and reordering in transactions for consistency Signed-off-by: Deluan <deluan@navidrome.org> * refactor: remove unused getTracks method from playlistTrackRepository Signed-off-by: Deluan <deluan@navidrome.org> * refactor: optimize playlist track renumbering with CTE-based UPDATE Replace the DELETE + re-INSERT renumbering strategy with a two-step UPDATE approach using a materialized CTE and ROW_NUMBER() window function. The previous approach (SELECT all IDs, DELETE all tracks, re-INSERT in chunks of 200) required 13 SQL operations for a 2000-track playlist. The new approach uses just 2 UPDATEs: first negating all IDs to clear the positive space, then assigning sequential positions via UPDATE...FROM with a CTE. This avoids the UNIQUE constraint violations that affected the original correlated subquery while reducing per-delete request time from ~110ms to ~12ms on a 2000-track playlist. Signed-off-by: Deluan <deluan@navidrome.org> * refactor: rename New function to NewPlaylists for clarity Signed-off-by: Deluan <deluan@navidrome.org> * refactor: update mock playlist repository and tests for consistency Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |