2025-06-12 13:17:34 -04:00
|
|
|
package scanner
|
|
|
|
|
|
|
|
|
|
import (
|
|
|
|
|
"crypto/md5"
|
|
|
|
|
"encoding/hex"
|
|
|
|
|
"fmt"
|
|
|
|
|
"io"
|
|
|
|
|
"io/fs"
|
|
|
|
|
"maps"
|
|
|
|
|
"slices"
|
|
|
|
|
"time"
|
|
|
|
|
|
fix(artwork): re-resolve artwork when image files change on disk (#5965)
* fix(artwork): re-resolve artwork when image files change on disk
An image-only folder change (replaced, added, or deleted cover/artist
images, with no audio files touched) was detected by the scanner but never
reached the artwork queue, so clients kept seeing the old coverArt hash
until something else forced a re-resolution.
Phase 1 now diffs each changed folder's image list and imagesUpdatedAt
against the previously persisted folder row, and at the end of the phase
bulk-enqueues re-resolution for the affected entities: albums with tracks
in the folder or its direct children (covering disc subfolder layouts),
and, when an artist-pattern image is involved, artists with albums under
the folder's subtree, mirroring the artist resolver's upward search. The
artist mapping mirrors the resolver's sole-album-artist album selection.
New repository helpers keep the mapping set-based and light: folder
GetAllIDs, media_file GetAlbumIDsByFolder (distinct, indexed by
folder_id), and album GetSoleAlbumArtistIDs.
* refactor: simplify the image-change artwork enqueue after review
Load the previous folder image state through the existing GetFolderUpdateInfo
bulk pre-pass instead of a per-folder SELECT inside the persist transaction,
and skip the diff for new folders, whose artwork the scanner already enqueues
inline. Move the artist-image classification into core/artwork
(IsArtistImageFile) so the scanner shares the resolver's ArtistArtPriority
token grammar instead of re-parsing it (the copy mistreated image-folder as a
filename glob). Move the folder-subtree query into the folder repository
(GetSubtreeIDs) with LIKE escaping and expression-tree batching, share the
sole-album-artist predicate between the resolver and the album repository
(model.SoleAlbumArtistFilter), extract a chunked single-column query helper,
and deduplicate the ArtworkQueueItem literals behind scanArtworkItem.
* fix(persistence): keep slash-form paths in GetSubtreeIDs subtree predicates
The scanner hands GetSubtreeIDs io/fs slash-form paths, but filepath.Clean
rewrites them with backslashes on Windows while folder.path is stored with
forward slashes, so the descendant predicates matched nothing and nested
artist folders were never re-enqueued there. Normalize with path.Clean, like
HasAudioOutsideFolders does, and cover a nested path in the repo test.
* refactor(persistence): move the sole-album-artist rule into the album repository
SQLizer filters belong in the persistence package, not model. The rule
becomes an unexported filter shared by GetSoleAlbumArtistIDs and a new
GetBySoleAlbumArtist repository method, which the artist artwork resolver now
calls instead of building the squirrel filter itself.
* perf(scanner): resolve image-change artists in one query over album.folder_ids
The artist half of the image-change enqueue walked folder subtree IDs, then
media_file rows, then album rows, marshalling thousands of bound IDs through
the driver on each hop. Matching albums by their own folder_ids instead is one
statement, and folder_ids is the same source the artist resolver uses to
compute an artist's folders.
Benchmarked against a copy of the production DB (97k tracks, 10k folders,
7k albums): 87ms +/-196% -> 17.4ms +/-8%, 7.1MB -> 172KB, 103k -> 1.5k allocs.
The subtree predicate becomes a shared folderSubtreeFilter, so Folder
GetSubtreeIDs and Album GetSoleAlbumArtistIDs are no longer needed.
* fix(scanner): persist ancestor folders discovered by a quick scan
A quick scan skipped any new folder with no files of its own, so an artist
folder holding only album subfolders never got a row. Adding artist.jpg to it
later then produced no artwork enqueue: the entry was new, so the image diff
was skipped, and it has no tracks, so nothing was enqueued inline either.
Skip only genuinely empty new folders, matching what a full scan already
persists. This also fixes artist artwork resolving as absent for artists first
imported by a quick scan, since the resolver's folder climb needs that row.
Also normalizes the selective-scan preload paths with path.Clean, so its
descendant predicates match the stored slash-form paths on Windows.
* fix(persistence): chunk subtree paths and match artist globs by basename
Two regressions from earlier commits on this branch.
Collapsing the subtree query into a single statement dropped the chunking the
old GetSubtreeIDs had: each path expands into 3 OR terms and SQLite rejects an
expression tree deeper than 1000, measured at 166 paths. A library with more
artist-image folders than that (the prod copy has 158) would fail the whole
collect, dropping the album items with it, so the scanner now keeps them when
the artist query fails.
The artist-image classifier compared whole tokens after stripping album/, so a
directory-bearing glob like images/artist.* never matched the basenames the
scanner has. Match on path.Base, which is what album/artist.* already reduced
to; the resolver climbs parent folders, so an exact prefix is not knowable
here and a conservative match is the right failure direction.
* refactor(persistence): halve the repository surface this PR adds
Research on the four new repository methods found two were avoidable.
GetAlbumIDsByFolder now expands the changed folders to their direct children
in its own subquery, so Folder.GetAllIDs has no callers and is deleted, one
round trip per scan disappears, and the previously unchunked id/parent_id IN
lists are covered by the existing chunking.
GetBySoleAlbumArtist becomes an exported SoleAlbumArtistFilter, matching the
ParticipantIDFilter precedent for sharing a Sqlizer with core/, so the rule
still lives in persistence but AlbumRepository gains nothing and the mock shim
that ignored the artist filter is gone.
Also drops queryAllSliceChunked, now callerless, in favour of the file-local
slices.Chunk convention used by the sibling folder queries.
Rejected on measurement: matching the album path by album.folder_ids is exactly
equivalent (13975 pairs, zero difference) but has no index, so it scans every
album and runs 5-200x slower than the media_file route.
* refactor(persistence): stop reading the deprecated album_artist_id column
Both artist lookups this PR touches now go through participation, matching
the precedent in core/archiver.go and share_repository.go.
SoleAlbumArtistFilter uses ParticipantIDFilter, which is also faster: the
album_artists unique constraint is a covering index for it, while the old
column needed album_artist_album_id plus a row fetch.
GetSoleAlbumArtistIDsInSubtrees reads the sole artist out of the participants
JSON it already parses for the sole-artist check, rather than joining back to
album_artists, which measured ~1.6x slower on a prod-sized copy.
Verified equivalent on that copy: 6828 sole-artist albums and 1088 subtree
artists resolve identically via the column, the join and the JSON. The tests
now set a deliberately wrong album_artist_id so they fail if either query
starts reading it again.
* docs: trim comments that carry rationale belonging in commit messages
Five comments had grown past the budget with benchmark numbers, rejected
alternatives, and a duplicate of the constant's own explanation.
* refactor(scanner): move the image-change enqueue into phase_1_folders
The three functions were methods on phaseFolders, so they belong with the
type; phase_1_image_changes.go also read like a fifth phase, which it wasn't.
* refactor(scanner): extract the image-change collector into its own type
phaseFolders no longer owns the per-library map and the mapping methods; it
records into a collector and asks it to enqueue once. The collector keeps the
library alongside the folders, so enqueue needs only ctx and the datastore.
* refactor(scanner): simplify enqueue method by removing redundant datastore parameter
Signed-off-by: Deluan <deluan@navidrome.org>
* docs(scanner): drop the stale zero-value claim on imageChangeCollector
The collector now takes its datastore at construction, so the zero value is
no longer usable.
* fix(scanner): pin the persist stage to concurrency 1 and guard the collector
The stage relied on go-pipeline defaulting to one worker; stating it at the
stage makes the constraint visible where someone would change it. The
collector takes a mutex too, so the type is safe on its own terms rather than
by configuration.
---------
Signed-off-by: Deluan <deluan@navidrome.org>
2026-08-16 13:24:07 -04:00
|
|
|
"github.com/navidrome/navidrome/core/artwork"
|
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>
2026-02-21 19:57:13 -05:00
|
|
|
"github.com/navidrome/navidrome/core/playlists"
|
2025-06-12 13:17:34 -04:00
|
|
|
"github.com/navidrome/navidrome/model"
|
|
|
|
|
"github.com/navidrome/navidrome/utils/chrono"
|
|
|
|
|
)
|
|
|
|
|
|
fix(artwork): re-resolve artwork when image files change on disk (#5965)
* fix(artwork): re-resolve artwork when image files change on disk
An image-only folder change (replaced, added, or deleted cover/artist
images, with no audio files touched) was detected by the scanner but never
reached the artwork queue, so clients kept seeing the old coverArt hash
until something else forced a re-resolution.
Phase 1 now diffs each changed folder's image list and imagesUpdatedAt
against the previously persisted folder row, and at the end of the phase
bulk-enqueues re-resolution for the affected entities: albums with tracks
in the folder or its direct children (covering disc subfolder layouts),
and, when an artist-pattern image is involved, artists with albums under
the folder's subtree, mirroring the artist resolver's upward search. The
artist mapping mirrors the resolver's sole-album-artist album selection.
New repository helpers keep the mapping set-based and light: folder
GetAllIDs, media_file GetAlbumIDsByFolder (distinct, indexed by
folder_id), and album GetSoleAlbumArtistIDs.
* refactor: simplify the image-change artwork enqueue after review
Load the previous folder image state through the existing GetFolderUpdateInfo
bulk pre-pass instead of a per-folder SELECT inside the persist transaction,
and skip the diff for new folders, whose artwork the scanner already enqueues
inline. Move the artist-image classification into core/artwork
(IsArtistImageFile) so the scanner shares the resolver's ArtistArtPriority
token grammar instead of re-parsing it (the copy mistreated image-folder as a
filename glob). Move the folder-subtree query into the folder repository
(GetSubtreeIDs) with LIKE escaping and expression-tree batching, share the
sole-album-artist predicate between the resolver and the album repository
(model.SoleAlbumArtistFilter), extract a chunked single-column query helper,
and deduplicate the ArtworkQueueItem literals behind scanArtworkItem.
* fix(persistence): keep slash-form paths in GetSubtreeIDs subtree predicates
The scanner hands GetSubtreeIDs io/fs slash-form paths, but filepath.Clean
rewrites them with backslashes on Windows while folder.path is stored with
forward slashes, so the descendant predicates matched nothing and nested
artist folders were never re-enqueued there. Normalize with path.Clean, like
HasAudioOutsideFolders does, and cover a nested path in the repo test.
* refactor(persistence): move the sole-album-artist rule into the album repository
SQLizer filters belong in the persistence package, not model. The rule
becomes an unexported filter shared by GetSoleAlbumArtistIDs and a new
GetBySoleAlbumArtist repository method, which the artist artwork resolver now
calls instead of building the squirrel filter itself.
* perf(scanner): resolve image-change artists in one query over album.folder_ids
The artist half of the image-change enqueue walked folder subtree IDs, then
media_file rows, then album rows, marshalling thousands of bound IDs through
the driver on each hop. Matching albums by their own folder_ids instead is one
statement, and folder_ids is the same source the artist resolver uses to
compute an artist's folders.
Benchmarked against a copy of the production DB (97k tracks, 10k folders,
7k albums): 87ms +/-196% -> 17.4ms +/-8%, 7.1MB -> 172KB, 103k -> 1.5k allocs.
The subtree predicate becomes a shared folderSubtreeFilter, so Folder
GetSubtreeIDs and Album GetSoleAlbumArtistIDs are no longer needed.
* fix(scanner): persist ancestor folders discovered by a quick scan
A quick scan skipped any new folder with no files of its own, so an artist
folder holding only album subfolders never got a row. Adding artist.jpg to it
later then produced no artwork enqueue: the entry was new, so the image diff
was skipped, and it has no tracks, so nothing was enqueued inline either.
Skip only genuinely empty new folders, matching what a full scan already
persists. This also fixes artist artwork resolving as absent for artists first
imported by a quick scan, since the resolver's folder climb needs that row.
Also normalizes the selective-scan preload paths with path.Clean, so its
descendant predicates match the stored slash-form paths on Windows.
* fix(persistence): chunk subtree paths and match artist globs by basename
Two regressions from earlier commits on this branch.
Collapsing the subtree query into a single statement dropped the chunking the
old GetSubtreeIDs had: each path expands into 3 OR terms and SQLite rejects an
expression tree deeper than 1000, measured at 166 paths. A library with more
artist-image folders than that (the prod copy has 158) would fail the whole
collect, dropping the album items with it, so the scanner now keeps them when
the artist query fails.
The artist-image classifier compared whole tokens after stripping album/, so a
directory-bearing glob like images/artist.* never matched the basenames the
scanner has. Match on path.Base, which is what album/artist.* already reduced
to; the resolver climbs parent folders, so an exact prefix is not knowable
here and a conservative match is the right failure direction.
* refactor(persistence): halve the repository surface this PR adds
Research on the four new repository methods found two were avoidable.
GetAlbumIDsByFolder now expands the changed folders to their direct children
in its own subquery, so Folder.GetAllIDs has no callers and is deleted, one
round trip per scan disappears, and the previously unchunked id/parent_id IN
lists are covered by the existing chunking.
GetBySoleAlbumArtist becomes an exported SoleAlbumArtistFilter, matching the
ParticipantIDFilter precedent for sharing a Sqlizer with core/, so the rule
still lives in persistence but AlbumRepository gains nothing and the mock shim
that ignored the artist filter is gone.
Also drops queryAllSliceChunked, now callerless, in favour of the file-local
slices.Chunk convention used by the sibling folder queries.
Rejected on measurement: matching the album path by album.folder_ids is exactly
equivalent (13975 pairs, zero difference) but has no index, so it scans every
album and runs 5-200x slower than the media_file route.
* refactor(persistence): stop reading the deprecated album_artist_id column
Both artist lookups this PR touches now go through participation, matching
the precedent in core/archiver.go and share_repository.go.
SoleAlbumArtistFilter uses ParticipantIDFilter, which is also faster: the
album_artists unique constraint is a covering index for it, while the old
column needed album_artist_album_id plus a row fetch.
GetSoleAlbumArtistIDsInSubtrees reads the sole artist out of the participants
JSON it already parses for the sole-artist check, rather than joining back to
album_artists, which measured ~1.6x slower on a prod-sized copy.
Verified equivalent on that copy: 6828 sole-artist albums and 1088 subtree
artists resolve identically via the column, the join and the JSON. The tests
now set a deliberately wrong album_artist_id so they fail if either query
starts reading it again.
* docs: trim comments that carry rationale belonging in commit messages
Five comments had grown past the budget with benchmark numbers, rejected
alternatives, and a duplicate of the constant's own explanation.
* refactor(scanner): move the image-change enqueue into phase_1_folders
The three functions were methods on phaseFolders, so they belong with the
type; phase_1_image_changes.go also read like a fifth phase, which it wasn't.
* refactor(scanner): extract the image-change collector into its own type
phaseFolders no longer owns the per-library map and the mapping methods; it
records into a collector and asks it to enqueue once. The collector keeps the
library alongside the folders, so enqueue needs only ctx and the datastore.
* refactor(scanner): simplify enqueue method by removing redundant datastore parameter
Signed-off-by: Deluan <deluan@navidrome.org>
* docs(scanner): drop the stale zero-value claim on imageChangeCollector
The collector now takes its datastore at construction, so the zero value is
no longer usable.
* fix(scanner): pin the persist stage to concurrency 1 and guard the collector
The stage relied on go-pipeline defaulting to one worker; stating it at the
stage makes the constraint visible where someone would change it. The
collector takes a mutex too, so the type is safe on its own terms rather than
by configuration.
---------
Signed-off-by: Deluan <deluan@navidrome.org>
2026-08-16 13:24:07 -04:00
|
|
|
func newFolderEntry(job *scanJob, id, path string, info model.FolderUpdateInfo) *folderEntry {
|
2025-06-12 13:17:34 -04:00
|
|
|
f := &folderEntry{
|
fix(artwork): re-resolve artwork when image files change on disk (#5965)
* fix(artwork): re-resolve artwork when image files change on disk
An image-only folder change (replaced, added, or deleted cover/artist
images, with no audio files touched) was detected by the scanner but never
reached the artwork queue, so clients kept seeing the old coverArt hash
until something else forced a re-resolution.
Phase 1 now diffs each changed folder's image list and imagesUpdatedAt
against the previously persisted folder row, and at the end of the phase
bulk-enqueues re-resolution for the affected entities: albums with tracks
in the folder or its direct children (covering disc subfolder layouts),
and, when an artist-pattern image is involved, artists with albums under
the folder's subtree, mirroring the artist resolver's upward search. The
artist mapping mirrors the resolver's sole-album-artist album selection.
New repository helpers keep the mapping set-based and light: folder
GetAllIDs, media_file GetAlbumIDsByFolder (distinct, indexed by
folder_id), and album GetSoleAlbumArtistIDs.
* refactor: simplify the image-change artwork enqueue after review
Load the previous folder image state through the existing GetFolderUpdateInfo
bulk pre-pass instead of a per-folder SELECT inside the persist transaction,
and skip the diff for new folders, whose artwork the scanner already enqueues
inline. Move the artist-image classification into core/artwork
(IsArtistImageFile) so the scanner shares the resolver's ArtistArtPriority
token grammar instead of re-parsing it (the copy mistreated image-folder as a
filename glob). Move the folder-subtree query into the folder repository
(GetSubtreeIDs) with LIKE escaping and expression-tree batching, share the
sole-album-artist predicate between the resolver and the album repository
(model.SoleAlbumArtistFilter), extract a chunked single-column query helper,
and deduplicate the ArtworkQueueItem literals behind scanArtworkItem.
* fix(persistence): keep slash-form paths in GetSubtreeIDs subtree predicates
The scanner hands GetSubtreeIDs io/fs slash-form paths, but filepath.Clean
rewrites them with backslashes on Windows while folder.path is stored with
forward slashes, so the descendant predicates matched nothing and nested
artist folders were never re-enqueued there. Normalize with path.Clean, like
HasAudioOutsideFolders does, and cover a nested path in the repo test.
* refactor(persistence): move the sole-album-artist rule into the album repository
SQLizer filters belong in the persistence package, not model. The rule
becomes an unexported filter shared by GetSoleAlbumArtistIDs and a new
GetBySoleAlbumArtist repository method, which the artist artwork resolver now
calls instead of building the squirrel filter itself.
* perf(scanner): resolve image-change artists in one query over album.folder_ids
The artist half of the image-change enqueue walked folder subtree IDs, then
media_file rows, then album rows, marshalling thousands of bound IDs through
the driver on each hop. Matching albums by their own folder_ids instead is one
statement, and folder_ids is the same source the artist resolver uses to
compute an artist's folders.
Benchmarked against a copy of the production DB (97k tracks, 10k folders,
7k albums): 87ms +/-196% -> 17.4ms +/-8%, 7.1MB -> 172KB, 103k -> 1.5k allocs.
The subtree predicate becomes a shared folderSubtreeFilter, so Folder
GetSubtreeIDs and Album GetSoleAlbumArtistIDs are no longer needed.
* fix(scanner): persist ancestor folders discovered by a quick scan
A quick scan skipped any new folder with no files of its own, so an artist
folder holding only album subfolders never got a row. Adding artist.jpg to it
later then produced no artwork enqueue: the entry was new, so the image diff
was skipped, and it has no tracks, so nothing was enqueued inline either.
Skip only genuinely empty new folders, matching what a full scan already
persists. This also fixes artist artwork resolving as absent for artists first
imported by a quick scan, since the resolver's folder climb needs that row.
Also normalizes the selective-scan preload paths with path.Clean, so its
descendant predicates match the stored slash-form paths on Windows.
* fix(persistence): chunk subtree paths and match artist globs by basename
Two regressions from earlier commits on this branch.
Collapsing the subtree query into a single statement dropped the chunking the
old GetSubtreeIDs had: each path expands into 3 OR terms and SQLite rejects an
expression tree deeper than 1000, measured at 166 paths. A library with more
artist-image folders than that (the prod copy has 158) would fail the whole
collect, dropping the album items with it, so the scanner now keeps them when
the artist query fails.
The artist-image classifier compared whole tokens after stripping album/, so a
directory-bearing glob like images/artist.* never matched the basenames the
scanner has. Match on path.Base, which is what album/artist.* already reduced
to; the resolver climbs parent folders, so an exact prefix is not knowable
here and a conservative match is the right failure direction.
* refactor(persistence): halve the repository surface this PR adds
Research on the four new repository methods found two were avoidable.
GetAlbumIDsByFolder now expands the changed folders to their direct children
in its own subquery, so Folder.GetAllIDs has no callers and is deleted, one
round trip per scan disappears, and the previously unchunked id/parent_id IN
lists are covered by the existing chunking.
GetBySoleAlbumArtist becomes an exported SoleAlbumArtistFilter, matching the
ParticipantIDFilter precedent for sharing a Sqlizer with core/, so the rule
still lives in persistence but AlbumRepository gains nothing and the mock shim
that ignored the artist filter is gone.
Also drops queryAllSliceChunked, now callerless, in favour of the file-local
slices.Chunk convention used by the sibling folder queries.
Rejected on measurement: matching the album path by album.folder_ids is exactly
equivalent (13975 pairs, zero difference) but has no index, so it scans every
album and runs 5-200x slower than the media_file route.
* refactor(persistence): stop reading the deprecated album_artist_id column
Both artist lookups this PR touches now go through participation, matching
the precedent in core/archiver.go and share_repository.go.
SoleAlbumArtistFilter uses ParticipantIDFilter, which is also faster: the
album_artists unique constraint is a covering index for it, while the old
column needed album_artist_album_id plus a row fetch.
GetSoleAlbumArtistIDsInSubtrees reads the sole artist out of the participants
JSON it already parses for the sole-artist check, rather than joining back to
album_artists, which measured ~1.6x slower on a prod-sized copy.
Verified equivalent on that copy: 6828 sole-artist albums and 1088 subtree
artists resolve identically via the column, the join and the JSON. The tests
now set a deliberately wrong album_artist_id so they fail if either query
starts reading it again.
* docs: trim comments that carry rationale belonging in commit messages
Five comments had grown past the budget with benchmark numbers, rejected
alternatives, and a duplicate of the constant's own explanation.
* refactor(scanner): move the image-change enqueue into phase_1_folders
The three functions were methods on phaseFolders, so they belong with the
type; phase_1_image_changes.go also read like a fifth phase, which it wasn't.
* refactor(scanner): extract the image-change collector into its own type
phaseFolders no longer owns the per-library map and the mapping methods; it
records into a collector and asks it to enqueue once. The collector keeps the
library alongside the folders, so enqueue needs only ctx and the datastore.
* refactor(scanner): simplify enqueue method by removing redundant datastore parameter
Signed-off-by: Deluan <deluan@navidrome.org>
* docs(scanner): drop the stale zero-value claim on imageChangeCollector
The collector now takes its datastore at construction, so the zero value is
no longer usable.
* fix(scanner): pin the persist stage to concurrency 1 and guard the collector
The stage relied on go-pipeline defaulting to one worker; stating it at the
stage makes the constraint visible where someone would change it. The
collector takes a mutex too, so the type is safe on its own terms rather than
by configuration.
---------
Signed-off-by: Deluan <deluan@navidrome.org>
2026-08-16 13:24:07 -04:00
|
|
|
id: id,
|
|
|
|
|
job: job,
|
|
|
|
|
path: path,
|
|
|
|
|
audioFiles: make(map[string]fs.DirEntry),
|
|
|
|
|
imageFiles: make(map[string]fs.DirEntry),
|
2026-08-19 19:11:00 +02:00
|
|
|
playlistFiles: make(map[string]fs.DirEntry),
|
fix(artwork): re-resolve artwork when image files change on disk (#5965)
* fix(artwork): re-resolve artwork when image files change on disk
An image-only folder change (replaced, added, or deleted cover/artist
images, with no audio files touched) was detected by the scanner but never
reached the artwork queue, so clients kept seeing the old coverArt hash
until something else forced a re-resolution.
Phase 1 now diffs each changed folder's image list and imagesUpdatedAt
against the previously persisted folder row, and at the end of the phase
bulk-enqueues re-resolution for the affected entities: albums with tracks
in the folder or its direct children (covering disc subfolder layouts),
and, when an artist-pattern image is involved, artists with albums under
the folder's subtree, mirroring the artist resolver's upward search. The
artist mapping mirrors the resolver's sole-album-artist album selection.
New repository helpers keep the mapping set-based and light: folder
GetAllIDs, media_file GetAlbumIDsByFolder (distinct, indexed by
folder_id), and album GetSoleAlbumArtistIDs.
* refactor: simplify the image-change artwork enqueue after review
Load the previous folder image state through the existing GetFolderUpdateInfo
bulk pre-pass instead of a per-folder SELECT inside the persist transaction,
and skip the diff for new folders, whose artwork the scanner already enqueues
inline. Move the artist-image classification into core/artwork
(IsArtistImageFile) so the scanner shares the resolver's ArtistArtPriority
token grammar instead of re-parsing it (the copy mistreated image-folder as a
filename glob). Move the folder-subtree query into the folder repository
(GetSubtreeIDs) with LIKE escaping and expression-tree batching, share the
sole-album-artist predicate between the resolver and the album repository
(model.SoleAlbumArtistFilter), extract a chunked single-column query helper,
and deduplicate the ArtworkQueueItem literals behind scanArtworkItem.
* fix(persistence): keep slash-form paths in GetSubtreeIDs subtree predicates
The scanner hands GetSubtreeIDs io/fs slash-form paths, but filepath.Clean
rewrites them with backslashes on Windows while folder.path is stored with
forward slashes, so the descendant predicates matched nothing and nested
artist folders were never re-enqueued there. Normalize with path.Clean, like
HasAudioOutsideFolders does, and cover a nested path in the repo test.
* refactor(persistence): move the sole-album-artist rule into the album repository
SQLizer filters belong in the persistence package, not model. The rule
becomes an unexported filter shared by GetSoleAlbumArtistIDs and a new
GetBySoleAlbumArtist repository method, which the artist artwork resolver now
calls instead of building the squirrel filter itself.
* perf(scanner): resolve image-change artists in one query over album.folder_ids
The artist half of the image-change enqueue walked folder subtree IDs, then
media_file rows, then album rows, marshalling thousands of bound IDs through
the driver on each hop. Matching albums by their own folder_ids instead is one
statement, and folder_ids is the same source the artist resolver uses to
compute an artist's folders.
Benchmarked against a copy of the production DB (97k tracks, 10k folders,
7k albums): 87ms +/-196% -> 17.4ms +/-8%, 7.1MB -> 172KB, 103k -> 1.5k allocs.
The subtree predicate becomes a shared folderSubtreeFilter, so Folder
GetSubtreeIDs and Album GetSoleAlbumArtistIDs are no longer needed.
* fix(scanner): persist ancestor folders discovered by a quick scan
A quick scan skipped any new folder with no files of its own, so an artist
folder holding only album subfolders never got a row. Adding artist.jpg to it
later then produced no artwork enqueue: the entry was new, so the image diff
was skipped, and it has no tracks, so nothing was enqueued inline either.
Skip only genuinely empty new folders, matching what a full scan already
persists. This also fixes artist artwork resolving as absent for artists first
imported by a quick scan, since the resolver's folder climb needs that row.
Also normalizes the selective-scan preload paths with path.Clean, so its
descendant predicates match the stored slash-form paths on Windows.
* fix(persistence): chunk subtree paths and match artist globs by basename
Two regressions from earlier commits on this branch.
Collapsing the subtree query into a single statement dropped the chunking the
old GetSubtreeIDs had: each path expands into 3 OR terms and SQLite rejects an
expression tree deeper than 1000, measured at 166 paths. A library with more
artist-image folders than that (the prod copy has 158) would fail the whole
collect, dropping the album items with it, so the scanner now keeps them when
the artist query fails.
The artist-image classifier compared whole tokens after stripping album/, so a
directory-bearing glob like images/artist.* never matched the basenames the
scanner has. Match on path.Base, which is what album/artist.* already reduced
to; the resolver climbs parent folders, so an exact prefix is not knowable
here and a conservative match is the right failure direction.
* refactor(persistence): halve the repository surface this PR adds
Research on the four new repository methods found two were avoidable.
GetAlbumIDsByFolder now expands the changed folders to their direct children
in its own subquery, so Folder.GetAllIDs has no callers and is deleted, one
round trip per scan disappears, and the previously unchunked id/parent_id IN
lists are covered by the existing chunking.
GetBySoleAlbumArtist becomes an exported SoleAlbumArtistFilter, matching the
ParticipantIDFilter precedent for sharing a Sqlizer with core/, so the rule
still lives in persistence but AlbumRepository gains nothing and the mock shim
that ignored the artist filter is gone.
Also drops queryAllSliceChunked, now callerless, in favour of the file-local
slices.Chunk convention used by the sibling folder queries.
Rejected on measurement: matching the album path by album.folder_ids is exactly
equivalent (13975 pairs, zero difference) but has no index, so it scans every
album and runs 5-200x slower than the media_file route.
* refactor(persistence): stop reading the deprecated album_artist_id column
Both artist lookups this PR touches now go through participation, matching
the precedent in core/archiver.go and share_repository.go.
SoleAlbumArtistFilter uses ParticipantIDFilter, which is also faster: the
album_artists unique constraint is a covering index for it, while the old
column needed album_artist_album_id plus a row fetch.
GetSoleAlbumArtistIDsInSubtrees reads the sole artist out of the participants
JSON it already parses for the sole-artist check, rather than joining back to
album_artists, which measured ~1.6x slower on a prod-sized copy.
Verified equivalent on that copy: 6828 sole-artist albums and 1088 subtree
artists resolve identically via the column, the join and the JSON. The tests
now set a deliberately wrong album_artist_id so they fail if either query
starts reading it again.
* docs: trim comments that carry rationale belonging in commit messages
Five comments had grown past the budget with benchmark numbers, rejected
alternatives, and a duplicate of the constant's own explanation.
* refactor(scanner): move the image-change enqueue into phase_1_folders
The three functions were methods on phaseFolders, so they belong with the
type; phase_1_image_changes.go also read like a fifth phase, which it wasn't.
* refactor(scanner): extract the image-change collector into its own type
phaseFolders no longer owns the per-library map and the mapping methods; it
records into a collector and asks it to enqueue once. The collector keeps the
library alongside the folders, so enqueue needs only ctx and the datastore.
* refactor(scanner): simplify enqueue method by removing redundant datastore parameter
Signed-off-by: Deluan <deluan@navidrome.org>
* docs(scanner): drop the stale zero-value claim on imageChangeCollector
The collector now takes its datastore at construction, so the zero value is
no longer usable.
* fix(scanner): pin the persist stage to concurrency 1 and guard the collector
The stage relied on go-pipeline defaulting to one worker; stating it at the
stage makes the constraint visible where someone would change it. The
collector takes a mutex too, so the type is safe on its own terms rather than
by configuration.
---------
Signed-off-by: Deluan <deluan@navidrome.org>
2026-08-16 13:24:07 -04:00
|
|
|
albumIDMap: make(map[string]string),
|
|
|
|
|
updTime: info.UpdatedAt,
|
|
|
|
|
prevHash: info.Hash,
|
|
|
|
|
prevImageFiles: info.ImageFiles,
|
|
|
|
|
prevImagesUpdatedAt: info.ImagesUpdatedAt,
|
2025-06-12 13:17:34 -04:00
|
|
|
}
|
|
|
|
|
return f
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
type folderEntry struct {
|
|
|
|
|
job *scanJob
|
|
|
|
|
elapsed chrono.Meter
|
|
|
|
|
path string // Full path
|
|
|
|
|
id string // DB ID
|
|
|
|
|
modTime time.Time // From FS
|
|
|
|
|
updTime time.Time // from DB
|
|
|
|
|
audioFiles map[string]fs.DirEntry
|
|
|
|
|
imageFiles map[string]fs.DirEntry
|
2026-08-19 19:11:00 +02:00
|
|
|
playlistFiles map[string]fs.DirEntry
|
2025-06-12 13:17:34 -04:00
|
|
|
numSubFolders int
|
|
|
|
|
imagesUpdatedAt time.Time
|
|
|
|
|
prevHash string // Previous hash from DB
|
fix(artwork): re-resolve artwork when image files change on disk (#5965)
* fix(artwork): re-resolve artwork when image files change on disk
An image-only folder change (replaced, added, or deleted cover/artist
images, with no audio files touched) was detected by the scanner but never
reached the artwork queue, so clients kept seeing the old coverArt hash
until something else forced a re-resolution.
Phase 1 now diffs each changed folder's image list and imagesUpdatedAt
against the previously persisted folder row, and at the end of the phase
bulk-enqueues re-resolution for the affected entities: albums with tracks
in the folder or its direct children (covering disc subfolder layouts),
and, when an artist-pattern image is involved, artists with albums under
the folder's subtree, mirroring the artist resolver's upward search. The
artist mapping mirrors the resolver's sole-album-artist album selection.
New repository helpers keep the mapping set-based and light: folder
GetAllIDs, media_file GetAlbumIDsByFolder (distinct, indexed by
folder_id), and album GetSoleAlbumArtistIDs.
* refactor: simplify the image-change artwork enqueue after review
Load the previous folder image state through the existing GetFolderUpdateInfo
bulk pre-pass instead of a per-folder SELECT inside the persist transaction,
and skip the diff for new folders, whose artwork the scanner already enqueues
inline. Move the artist-image classification into core/artwork
(IsArtistImageFile) so the scanner shares the resolver's ArtistArtPriority
token grammar instead of re-parsing it (the copy mistreated image-folder as a
filename glob). Move the folder-subtree query into the folder repository
(GetSubtreeIDs) with LIKE escaping and expression-tree batching, share the
sole-album-artist predicate between the resolver and the album repository
(model.SoleAlbumArtistFilter), extract a chunked single-column query helper,
and deduplicate the ArtworkQueueItem literals behind scanArtworkItem.
* fix(persistence): keep slash-form paths in GetSubtreeIDs subtree predicates
The scanner hands GetSubtreeIDs io/fs slash-form paths, but filepath.Clean
rewrites them with backslashes on Windows while folder.path is stored with
forward slashes, so the descendant predicates matched nothing and nested
artist folders were never re-enqueued there. Normalize with path.Clean, like
HasAudioOutsideFolders does, and cover a nested path in the repo test.
* refactor(persistence): move the sole-album-artist rule into the album repository
SQLizer filters belong in the persistence package, not model. The rule
becomes an unexported filter shared by GetSoleAlbumArtistIDs and a new
GetBySoleAlbumArtist repository method, which the artist artwork resolver now
calls instead of building the squirrel filter itself.
* perf(scanner): resolve image-change artists in one query over album.folder_ids
The artist half of the image-change enqueue walked folder subtree IDs, then
media_file rows, then album rows, marshalling thousands of bound IDs through
the driver on each hop. Matching albums by their own folder_ids instead is one
statement, and folder_ids is the same source the artist resolver uses to
compute an artist's folders.
Benchmarked against a copy of the production DB (97k tracks, 10k folders,
7k albums): 87ms +/-196% -> 17.4ms +/-8%, 7.1MB -> 172KB, 103k -> 1.5k allocs.
The subtree predicate becomes a shared folderSubtreeFilter, so Folder
GetSubtreeIDs and Album GetSoleAlbumArtistIDs are no longer needed.
* fix(scanner): persist ancestor folders discovered by a quick scan
A quick scan skipped any new folder with no files of its own, so an artist
folder holding only album subfolders never got a row. Adding artist.jpg to it
later then produced no artwork enqueue: the entry was new, so the image diff
was skipped, and it has no tracks, so nothing was enqueued inline either.
Skip only genuinely empty new folders, matching what a full scan already
persists. This also fixes artist artwork resolving as absent for artists first
imported by a quick scan, since the resolver's folder climb needs that row.
Also normalizes the selective-scan preload paths with path.Clean, so its
descendant predicates match the stored slash-form paths on Windows.
* fix(persistence): chunk subtree paths and match artist globs by basename
Two regressions from earlier commits on this branch.
Collapsing the subtree query into a single statement dropped the chunking the
old GetSubtreeIDs had: each path expands into 3 OR terms and SQLite rejects an
expression tree deeper than 1000, measured at 166 paths. A library with more
artist-image folders than that (the prod copy has 158) would fail the whole
collect, dropping the album items with it, so the scanner now keeps them when
the artist query fails.
The artist-image classifier compared whole tokens after stripping album/, so a
directory-bearing glob like images/artist.* never matched the basenames the
scanner has. Match on path.Base, which is what album/artist.* already reduced
to; the resolver climbs parent folders, so an exact prefix is not knowable
here and a conservative match is the right failure direction.
* refactor(persistence): halve the repository surface this PR adds
Research on the four new repository methods found two were avoidable.
GetAlbumIDsByFolder now expands the changed folders to their direct children
in its own subquery, so Folder.GetAllIDs has no callers and is deleted, one
round trip per scan disappears, and the previously unchunked id/parent_id IN
lists are covered by the existing chunking.
GetBySoleAlbumArtist becomes an exported SoleAlbumArtistFilter, matching the
ParticipantIDFilter precedent for sharing a Sqlizer with core/, so the rule
still lives in persistence but AlbumRepository gains nothing and the mock shim
that ignored the artist filter is gone.
Also drops queryAllSliceChunked, now callerless, in favour of the file-local
slices.Chunk convention used by the sibling folder queries.
Rejected on measurement: matching the album path by album.folder_ids is exactly
equivalent (13975 pairs, zero difference) but has no index, so it scans every
album and runs 5-200x slower than the media_file route.
* refactor(persistence): stop reading the deprecated album_artist_id column
Both artist lookups this PR touches now go through participation, matching
the precedent in core/archiver.go and share_repository.go.
SoleAlbumArtistFilter uses ParticipantIDFilter, which is also faster: the
album_artists unique constraint is a covering index for it, while the old
column needed album_artist_album_id plus a row fetch.
GetSoleAlbumArtistIDsInSubtrees reads the sole artist out of the participants
JSON it already parses for the sole-artist check, rather than joining back to
album_artists, which measured ~1.6x slower on a prod-sized copy.
Verified equivalent on that copy: 6828 sole-artist albums and 1088 subtree
artists resolve identically via the column, the join and the JSON. The tests
now set a deliberately wrong album_artist_id so they fail if either query
starts reading it again.
* docs: trim comments that carry rationale belonging in commit messages
Five comments had grown past the budget with benchmark numbers, rejected
alternatives, and a duplicate of the constant's own explanation.
* refactor(scanner): move the image-change enqueue into phase_1_folders
The three functions were methods on phaseFolders, so they belong with the
type; phase_1_image_changes.go also read like a fifth phase, which it wasn't.
* refactor(scanner): extract the image-change collector into its own type
phaseFolders no longer owns the per-library map and the mapping methods; it
records into a collector and asks it to enqueue once. The collector keeps the
library alongside the folders, so enqueue needs only ctx and the datastore.
* refactor(scanner): simplify enqueue method by removing redundant datastore parameter
Signed-off-by: Deluan <deluan@navidrome.org>
* docs(scanner): drop the stale zero-value claim on imageChangeCollector
The collector now takes its datastore at construction, so the zero value is
no longer usable.
* fix(scanner): pin the persist stage to concurrency 1 and guard the collector
The stage relied on go-pipeline defaulting to one worker; stating it at the
stage makes the constraint visible where someone would change it. The
collector takes a mutex too, so the type is safe on its own terms rather than
by configuration.
---------
Signed-off-by: Deluan <deluan@navidrome.org>
2026-08-16 13:24:07 -04:00
|
|
|
// Previous image state from DB, to detect image-only changes
|
|
|
|
|
prevImageFiles []string
|
|
|
|
|
prevImagesUpdatedAt time.Time
|
|
|
|
|
tracks model.MediaFiles
|
|
|
|
|
albums model.Albums
|
|
|
|
|
albumIDMap map[string]string
|
|
|
|
|
artists model.Artists
|
|
|
|
|
tags model.TagList
|
|
|
|
|
missingTracks []*model.MediaFile
|
2025-06-12 13:17:34 -04:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
func (f *folderEntry) hasNoFiles() bool {
|
2026-08-19 19:11:00 +02:00
|
|
|
return len(f.audioFiles) == 0 && len(f.imageFiles) == 0 && len(f.playlistFiles) == 0
|
2025-06-14 12:35:28 -04:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
func (f *folderEntry) isEmpty() bool {
|
|
|
|
|
return f.hasNoFiles() && f.numSubFolders == 0
|
2025-06-12 13:17:34 -04:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
func (f *folderEntry) isNew() bool {
|
|
|
|
|
return f.updTime.IsZero()
|
|
|
|
|
}
|
|
|
|
|
|
2025-06-14 12:35:28 -04:00
|
|
|
func (f *folderEntry) isOutdated() bool {
|
|
|
|
|
if f.job.lib.FullScanInProgress && f.updTime.Before(f.job.lib.LastScanStartedAt) {
|
|
|
|
|
return true
|
|
|
|
|
}
|
|
|
|
|
return f.prevHash != f.hash()
|
|
|
|
|
}
|
|
|
|
|
|
fix(artwork): re-resolve artwork when image files change on disk (#5965)
* fix(artwork): re-resolve artwork when image files change on disk
An image-only folder change (replaced, added, or deleted cover/artist
images, with no audio files touched) was detected by the scanner but never
reached the artwork queue, so clients kept seeing the old coverArt hash
until something else forced a re-resolution.
Phase 1 now diffs each changed folder's image list and imagesUpdatedAt
against the previously persisted folder row, and at the end of the phase
bulk-enqueues re-resolution for the affected entities: albums with tracks
in the folder or its direct children (covering disc subfolder layouts),
and, when an artist-pattern image is involved, artists with albums under
the folder's subtree, mirroring the artist resolver's upward search. The
artist mapping mirrors the resolver's sole-album-artist album selection.
New repository helpers keep the mapping set-based and light: folder
GetAllIDs, media_file GetAlbumIDsByFolder (distinct, indexed by
folder_id), and album GetSoleAlbumArtistIDs.
* refactor: simplify the image-change artwork enqueue after review
Load the previous folder image state through the existing GetFolderUpdateInfo
bulk pre-pass instead of a per-folder SELECT inside the persist transaction,
and skip the diff for new folders, whose artwork the scanner already enqueues
inline. Move the artist-image classification into core/artwork
(IsArtistImageFile) so the scanner shares the resolver's ArtistArtPriority
token grammar instead of re-parsing it (the copy mistreated image-folder as a
filename glob). Move the folder-subtree query into the folder repository
(GetSubtreeIDs) with LIKE escaping and expression-tree batching, share the
sole-album-artist predicate between the resolver and the album repository
(model.SoleAlbumArtistFilter), extract a chunked single-column query helper,
and deduplicate the ArtworkQueueItem literals behind scanArtworkItem.
* fix(persistence): keep slash-form paths in GetSubtreeIDs subtree predicates
The scanner hands GetSubtreeIDs io/fs slash-form paths, but filepath.Clean
rewrites them with backslashes on Windows while folder.path is stored with
forward slashes, so the descendant predicates matched nothing and nested
artist folders were never re-enqueued there. Normalize with path.Clean, like
HasAudioOutsideFolders does, and cover a nested path in the repo test.
* refactor(persistence): move the sole-album-artist rule into the album repository
SQLizer filters belong in the persistence package, not model. The rule
becomes an unexported filter shared by GetSoleAlbumArtistIDs and a new
GetBySoleAlbumArtist repository method, which the artist artwork resolver now
calls instead of building the squirrel filter itself.
* perf(scanner): resolve image-change artists in one query over album.folder_ids
The artist half of the image-change enqueue walked folder subtree IDs, then
media_file rows, then album rows, marshalling thousands of bound IDs through
the driver on each hop. Matching albums by their own folder_ids instead is one
statement, and folder_ids is the same source the artist resolver uses to
compute an artist's folders.
Benchmarked against a copy of the production DB (97k tracks, 10k folders,
7k albums): 87ms +/-196% -> 17.4ms +/-8%, 7.1MB -> 172KB, 103k -> 1.5k allocs.
The subtree predicate becomes a shared folderSubtreeFilter, so Folder
GetSubtreeIDs and Album GetSoleAlbumArtistIDs are no longer needed.
* fix(scanner): persist ancestor folders discovered by a quick scan
A quick scan skipped any new folder with no files of its own, so an artist
folder holding only album subfolders never got a row. Adding artist.jpg to it
later then produced no artwork enqueue: the entry was new, so the image diff
was skipped, and it has no tracks, so nothing was enqueued inline either.
Skip only genuinely empty new folders, matching what a full scan already
persists. This also fixes artist artwork resolving as absent for artists first
imported by a quick scan, since the resolver's folder climb needs that row.
Also normalizes the selective-scan preload paths with path.Clean, so its
descendant predicates match the stored slash-form paths on Windows.
* fix(persistence): chunk subtree paths and match artist globs by basename
Two regressions from earlier commits on this branch.
Collapsing the subtree query into a single statement dropped the chunking the
old GetSubtreeIDs had: each path expands into 3 OR terms and SQLite rejects an
expression tree deeper than 1000, measured at 166 paths. A library with more
artist-image folders than that (the prod copy has 158) would fail the whole
collect, dropping the album items with it, so the scanner now keeps them when
the artist query fails.
The artist-image classifier compared whole tokens after stripping album/, so a
directory-bearing glob like images/artist.* never matched the basenames the
scanner has. Match on path.Base, which is what album/artist.* already reduced
to; the resolver climbs parent folders, so an exact prefix is not knowable
here and a conservative match is the right failure direction.
* refactor(persistence): halve the repository surface this PR adds
Research on the four new repository methods found two were avoidable.
GetAlbumIDsByFolder now expands the changed folders to their direct children
in its own subquery, so Folder.GetAllIDs has no callers and is deleted, one
round trip per scan disappears, and the previously unchunked id/parent_id IN
lists are covered by the existing chunking.
GetBySoleAlbumArtist becomes an exported SoleAlbumArtistFilter, matching the
ParticipantIDFilter precedent for sharing a Sqlizer with core/, so the rule
still lives in persistence but AlbumRepository gains nothing and the mock shim
that ignored the artist filter is gone.
Also drops queryAllSliceChunked, now callerless, in favour of the file-local
slices.Chunk convention used by the sibling folder queries.
Rejected on measurement: matching the album path by album.folder_ids is exactly
equivalent (13975 pairs, zero difference) but has no index, so it scans every
album and runs 5-200x slower than the media_file route.
* refactor(persistence): stop reading the deprecated album_artist_id column
Both artist lookups this PR touches now go through participation, matching
the precedent in core/archiver.go and share_repository.go.
SoleAlbumArtistFilter uses ParticipantIDFilter, which is also faster: the
album_artists unique constraint is a covering index for it, while the old
column needed album_artist_album_id plus a row fetch.
GetSoleAlbumArtistIDsInSubtrees reads the sole artist out of the participants
JSON it already parses for the sole-artist check, rather than joining back to
album_artists, which measured ~1.6x slower on a prod-sized copy.
Verified equivalent on that copy: 6828 sole-artist albums and 1088 subtree
artists resolve identically via the column, the join and the JSON. The tests
now set a deliberately wrong album_artist_id so they fail if either query
starts reading it again.
* docs: trim comments that carry rationale belonging in commit messages
Five comments had grown past the budget with benchmark numbers, rejected
alternatives, and a duplicate of the constant's own explanation.
* refactor(scanner): move the image-change enqueue into phase_1_folders
The three functions were methods on phaseFolders, so they belong with the
type; phase_1_image_changes.go also read like a fifth phase, which it wasn't.
* refactor(scanner): extract the image-change collector into its own type
phaseFolders no longer owns the per-library map and the mapping methods; it
records into a collector and asks it to enqueue once. The collector keeps the
library alongside the folders, so enqueue needs only ctx and the datastore.
* refactor(scanner): simplify enqueue method by removing redundant datastore parameter
Signed-off-by: Deluan <deluan@navidrome.org>
* docs(scanner): drop the stale zero-value claim on imageChangeCollector
The collector now takes its datastore at construction, so the zero value is
no longer usable.
* fix(scanner): pin the persist stage to concurrency 1 and guard the collector
The stage relied on go-pipeline defaulting to one worker; stating it at the
stage makes the constraint visible where someone would change it. The
collector takes a mutex too, so the type is safe on its own terms rather than
by configuration.
---------
Signed-off-by: Deluan <deluan@navidrome.org>
2026-08-16 13:24:07 -04:00
|
|
|
// imagesChanged reports whether the folder's image files differ from the previously persisted
|
|
|
|
|
// state, and whether an artist image is involved (present in the old or the new list).
|
|
|
|
|
func (f *folderEntry) imagesChanged() (changed, artistImage bool) {
|
|
|
|
|
newNames := slices.Sorted(maps.Keys(f.imageFiles))
|
|
|
|
|
prevNames := slices.Sorted(slices.Values(f.prevImageFiles))
|
|
|
|
|
// Both empty also skips the timestamp check, which is noise for image-less folders.
|
|
|
|
|
if len(prevNames) == 0 && len(newNames) == 0 {
|
|
|
|
|
return false, false
|
|
|
|
|
}
|
|
|
|
|
if slices.Equal(prevNames, newNames) && f.prevImagesUpdatedAt.Equal(f.imagesUpdatedAt) {
|
|
|
|
|
return false, false
|
|
|
|
|
}
|
|
|
|
|
return true, slices.ContainsFunc(slices.Concat(prevNames, newNames), artwork.IsArtistImageFile)
|
|
|
|
|
}
|
|
|
|
|
|
2025-06-12 13:17:34 -04:00
|
|
|
func (f *folderEntry) toFolder() *model.Folder {
|
|
|
|
|
folder := model.NewFolder(f.job.lib, f.path)
|
|
|
|
|
folder.NumAudioFiles = len(f.audioFiles)
|
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>
2026-02-21 19:57:13 -05:00
|
|
|
if playlists.InPath(*folder) {
|
2026-08-19 19:11:00 +02:00
|
|
|
folder.NumPlaylists = len(f.playlistFiles)
|
2025-06-12 13:17:34 -04:00
|
|
|
}
|
|
|
|
|
folder.ImageFiles = slices.Collect(maps.Keys(f.imageFiles))
|
|
|
|
|
folder.ImagesUpdatedAt = f.imagesUpdatedAt
|
|
|
|
|
folder.Hash = f.hash()
|
|
|
|
|
return folder
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
func (f *folderEntry) hash() string {
|
2025-06-14 12:35:28 -04:00
|
|
|
h := md5.New()
|
|
|
|
|
_, _ = fmt.Fprintf(
|
|
|
|
|
h,
|
|
|
|
|
"%s:%d:%d:%s",
|
|
|
|
|
f.modTime.UTC(),
|
2026-08-19 19:11:00 +02:00
|
|
|
len(f.playlistFiles), // redundant with the loop below, but dropping it re-hashes every folder
|
2025-06-14 12:35:28 -04:00
|
|
|
f.numSubFolders,
|
|
|
|
|
f.imagesUpdatedAt.UTC(),
|
|
|
|
|
)
|
|
|
|
|
|
2026-08-19 19:11:00 +02:00
|
|
|
// Sort the keys of audio, image and playlist files to ensure consistent hashing
|
2025-06-12 13:17:34 -04:00
|
|
|
audioKeys := slices.Collect(maps.Keys(f.audioFiles))
|
|
|
|
|
slices.Sort(audioKeys)
|
|
|
|
|
imageKeys := slices.Collect(maps.Keys(f.imageFiles))
|
|
|
|
|
slices.Sort(imageKeys)
|
2026-08-19 19:11:00 +02:00
|
|
|
playlistKeys := slices.Collect(maps.Keys(f.playlistFiles))
|
|
|
|
|
slices.Sort(playlistKeys)
|
2025-06-12 13:17:34 -04:00
|
|
|
|
2025-06-14 12:35:28 -04:00
|
|
|
// Include audio files with their size and modtime
|
|
|
|
|
for _, key := range audioKeys {
|
|
|
|
|
_, _ = io.WriteString(h, key)
|
|
|
|
|
if info, err := f.audioFiles[key].Info(); err == nil {
|
|
|
|
|
_, _ = fmt.Fprintf(h, ":%d:%s", info.Size(), info.ModTime().UTC().String())
|
|
|
|
|
}
|
|
|
|
|
}
|
2025-06-12 13:17:34 -04:00
|
|
|
|
2025-06-14 12:35:28 -04:00
|
|
|
// Include image files with their size and modtime
|
|
|
|
|
for _, key := range imageKeys {
|
|
|
|
|
_, _ = io.WriteString(h, key)
|
|
|
|
|
if info, err := f.imageFiles[key].Info(); err == nil {
|
|
|
|
|
_, _ = fmt.Fprintf(h, ":%d:%s", info.Size(), info.ModTime().UTC().String())
|
|
|
|
|
}
|
2025-06-12 13:17:34 -04:00
|
|
|
}
|
2025-06-14 12:35:28 -04:00
|
|
|
|
2026-08-19 19:11:00 +02:00
|
|
|
// Include playlist files, so a content edit is detected even when the folder's
|
|
|
|
|
// mtime is preserved (rsync -a) or the playlist is not the newest file.
|
|
|
|
|
for _, key := range playlistKeys {
|
|
|
|
|
_, _ = io.WriteString(h, key)
|
|
|
|
|
if info, err := f.playlistFiles[key].Info(); err == nil {
|
|
|
|
|
_, _ = fmt.Fprintf(h, ":%d:%s", info.Size(), info.ModTime().UTC().String())
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
2025-06-14 12:35:28 -04:00
|
|
|
return hex.EncodeToString(h.Sum(nil))
|
2025-06-12 13:17:34 -04:00
|
|
|
}
|