2025-11-14 22:15:43 -05:00
|
|
|
package persistence
|
|
|
|
|
|
|
|
|
|
import (
|
|
|
|
|
"context"
|
2026-03-03 07:58:14 -05:00
|
|
|
"errors"
|
2025-11-14 22:15:43 -05:00
|
|
|
"fmt"
|
|
|
|
|
|
|
|
|
|
"github.com/navidrome/navidrome/log"
|
|
|
|
|
"github.com/navidrome/navidrome/model"
|
|
|
|
|
"github.com/navidrome/navidrome/model/request"
|
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/utils/slice"
|
2025-11-14 22:15:43 -05:00
|
|
|
. "github.com/onsi/ginkgo/v2"
|
|
|
|
|
. "github.com/onsi/gomega"
|
|
|
|
|
"github.com/pocketbase/dbx"
|
|
|
|
|
)
|
|
|
|
|
|
|
|
|
|
var _ = Describe("FolderRepository", func() {
|
|
|
|
|
var repo model.FolderRepository
|
|
|
|
|
var ctx context.Context
|
|
|
|
|
var conn *dbx.DB
|
|
|
|
|
var testLib, otherLib model.Library
|
|
|
|
|
|
|
|
|
|
BeforeEach(func() {
|
|
|
|
|
ctx = request.WithUser(log.NewContext(context.TODO()), model.User{ID: "userid"})
|
|
|
|
|
conn = GetDBXBuilder()
|
2026-09-25 18:06:10 -04:00
|
|
|
repo = newFolderRepository(conn)
|
2025-11-14 22:15:43 -05:00
|
|
|
|
|
|
|
|
// Use existing library ID 1 from test fixtures
|
2026-09-25 18:06:10 -04:00
|
|
|
libRepo := NewLibraryRepository(conn)
|
|
|
|
|
lib, err := libRepo.Get(ctx, 1)
|
2025-11-14 22:15:43 -05:00
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
testLib = *lib
|
|
|
|
|
|
|
|
|
|
// Create a second library with its own folder to verify isolation
|
|
|
|
|
otherLib = model.Library{Name: "Other Library", Path: "/other/path"}
|
2026-09-25 18:06:10 -04:00
|
|
|
Expect(libRepo.Put(ctx, &otherLib)).To(Succeed())
|
2025-11-14 22:15:43 -05:00
|
|
|
})
|
|
|
|
|
|
|
|
|
|
AfterEach(func() {
|
|
|
|
|
// Clean up only test folders created by our tests (paths starting with "Test")
|
|
|
|
|
// This prevents interference with fixture data needed by other tests
|
|
|
|
|
_, _ = conn.NewQuery("DELETE FROM folder WHERE library_id = 1 AND path LIKE 'Test%'").Execute()
|
|
|
|
|
_, _ = conn.NewQuery(fmt.Sprintf("DELETE FROM library WHERE id = %d", otherLib.ID)).Execute()
|
|
|
|
|
})
|
|
|
|
|
|
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
|
|
|
Describe("folderSubtreeFilter", func() {
|
|
|
|
|
var parent, child, grandchild, other *model.Folder
|
|
|
|
|
|
|
|
|
|
matching := func(paths ...string) []string {
|
|
|
|
|
GinkgoHelper()
|
2026-09-25 18:06:10 -04:00
|
|
|
folders, err := repo.GetAll(ctx, model.QueryOptions{Filters: folderSubtreeFilter(testLib, paths)})
|
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
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
return slice.Map(folders, func(f model.Folder) string { return f.ID })
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
BeforeEach(func() {
|
|
|
|
|
parent = model.NewFolder(testLib, "TestSubtree")
|
|
|
|
|
child = model.NewFolder(testLib, "TestSubtree/Child")
|
|
|
|
|
grandchild = model.NewFolder(testLib, "TestSubtree/Child/Grandchild")
|
|
|
|
|
other = model.NewFolder(testLib, "TestSubtreeOther")
|
|
|
|
|
for _, f := range []*model.Folder{parent, child, grandchild, other} {
|
2026-09-25 18:06:10 -04:00
|
|
|
Expect(repo.Put(ctx, f)).To(Succeed())
|
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
|
|
|
}
|
|
|
|
|
DeferCleanup(func() {
|
|
|
|
|
_, _ = conn.NewQuery("DELETE FROM folder WHERE name LIKE 'TestSubtree%' OR path LIKE 'TestSubtree%'").Execute()
|
|
|
|
|
})
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("matches a folder and all its descendants", func() {
|
|
|
|
|
Expect(matching("TestSubtree")).To(ConsistOf(parent.ID, child.ID, grandchild.ID))
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("matches the descendants of a nested slash-form path", func() {
|
|
|
|
|
Expect(matching("TestSubtree/Child")).To(ConsistOf(child.ID, grandchild.ID))
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("matches the whole library for the root path", func() {
|
|
|
|
|
Expect(matching(".")).To(ContainElements(parent.ID, child.ID, grandchild.ID, other.ID))
|
|
|
|
|
})
|
|
|
|
|
})
|
|
|
|
|
|
2025-11-14 22:15:43 -05:00
|
|
|
Describe("GetFolderUpdateInfo", func() {
|
|
|
|
|
Context("with no target paths", func() {
|
|
|
|
|
It("returns all folders in the library", func() {
|
|
|
|
|
// Create test folders with unique names to avoid conflicts
|
|
|
|
|
folder1 := model.NewFolder(testLib, "TestGetLastUpdates/Folder1")
|
|
|
|
|
folder2 := model.NewFolder(testLib, "TestGetLastUpdates/Folder2")
|
|
|
|
|
|
2026-09-25 18:06:10 -04:00
|
|
|
err := repo.Put(ctx, folder1)
|
2025-11-14 22:15:43 -05:00
|
|
|
Expect(err).ToNot(HaveOccurred())
|
2026-09-25 18:06:10 -04:00
|
|
|
err = repo.Put(ctx, folder2)
|
2025-11-14 22:15:43 -05:00
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
|
|
|
|
|
otherFolder := model.NewFolder(otherLib, "TestOtherLib/Folder")
|
2026-09-25 18:06:10 -04:00
|
|
|
err = repo.Put(ctx, otherFolder)
|
2025-11-14 22:15:43 -05:00
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
|
|
|
|
|
// Query all folders (no target paths) - should only return folders from testLib
|
2026-09-25 18:06:10 -04:00
|
|
|
results, err := repo.GetFolderUpdateInfo(ctx, testLib)
|
2025-11-14 22:15:43 -05:00
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
// Should include folders from testLib
|
|
|
|
|
Expect(results).To(HaveKey(folder1.ID))
|
|
|
|
|
Expect(results).To(HaveKey(folder2.ID))
|
|
|
|
|
// Should NOT include folders from other library
|
|
|
|
|
Expect(results).ToNot(HaveKey(otherFolder.ID))
|
|
|
|
|
})
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
Context("with specific target paths", func() {
|
|
|
|
|
It("returns folder info for existing folders", func() {
|
|
|
|
|
// Create test folders with unique names
|
|
|
|
|
folder1 := model.NewFolder(testLib, "TestSpecific/Rock")
|
|
|
|
|
folder2 := model.NewFolder(testLib, "TestSpecific/Jazz")
|
|
|
|
|
folder3 := model.NewFolder(testLib, "TestSpecific/Classical")
|
|
|
|
|
|
2026-09-25 18:06:10 -04:00
|
|
|
err := repo.Put(ctx, folder1)
|
2025-11-14 22:15:43 -05:00
|
|
|
Expect(err).ToNot(HaveOccurred())
|
2026-09-25 18:06:10 -04:00
|
|
|
err = repo.Put(ctx, folder2)
|
2025-11-14 22:15:43 -05:00
|
|
|
Expect(err).ToNot(HaveOccurred())
|
2026-09-25 18:06:10 -04:00
|
|
|
err = repo.Put(ctx, folder3)
|
2025-11-14 22:15:43 -05:00
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
|
|
|
|
|
// Query specific paths
|
2026-09-25 18:06:10 -04:00
|
|
|
results, err := repo.GetFolderUpdateInfo(ctx, testLib, "TestSpecific/Rock", "TestSpecific/Classical")
|
2025-11-14 22:15:43 -05:00
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
Expect(results).To(HaveLen(2))
|
|
|
|
|
|
|
|
|
|
// Verify folder IDs are in results
|
|
|
|
|
Expect(results).To(HaveKey(folder1.ID))
|
|
|
|
|
Expect(results).To(HaveKey(folder3.ID))
|
|
|
|
|
Expect(results).ToNot(HaveKey(folder2.ID))
|
|
|
|
|
|
|
|
|
|
// Verify update info is populated
|
|
|
|
|
Expect(results[folder1.ID].UpdatedAt).ToNot(BeZero())
|
|
|
|
|
Expect(results[folder1.ID].Hash).To(Equal(folder1.Hash))
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("includes all child folders when querying parent", func() {
|
|
|
|
|
// Create a parent folder with multiple children
|
|
|
|
|
parent := model.NewFolder(testLib, "TestParent/Music")
|
|
|
|
|
child1 := model.NewFolder(testLib, "TestParent/Music/Rock/Queen")
|
|
|
|
|
child2 := model.NewFolder(testLib, "TestParent/Music/Jazz")
|
|
|
|
|
otherParent := model.NewFolder(testLib, "TestParent2/Music/Jazz")
|
|
|
|
|
|
2026-09-25 18:06:10 -04:00
|
|
|
Expect(repo.Put(ctx, parent)).To(Succeed())
|
|
|
|
|
Expect(repo.Put(ctx, child1)).To(Succeed())
|
|
|
|
|
Expect(repo.Put(ctx, child2)).To(Succeed())
|
2025-11-14 22:15:43 -05:00
|
|
|
|
|
|
|
|
// Query the parent folder - should return parent and all children
|
2026-09-25 18:06:10 -04:00
|
|
|
results, err := repo.GetFolderUpdateInfo(ctx, testLib, "TestParent/Music")
|
2025-11-14 22:15:43 -05:00
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
Expect(results).To(HaveLen(3))
|
|
|
|
|
Expect(results).To(HaveKey(parent.ID))
|
|
|
|
|
Expect(results).To(HaveKey(child1.ID))
|
|
|
|
|
Expect(results).To(HaveKey(child2.ID))
|
|
|
|
|
Expect(results).ToNot(HaveKey(otherParent.ID))
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("excludes children from other libraries", func() {
|
|
|
|
|
// Create parent in testLib
|
|
|
|
|
parent := model.NewFolder(testLib, "TestIsolation/Parent")
|
|
|
|
|
child := model.NewFolder(testLib, "TestIsolation/Parent/Child")
|
|
|
|
|
|
2026-09-25 18:06:10 -04:00
|
|
|
Expect(repo.Put(ctx, parent)).To(Succeed())
|
|
|
|
|
Expect(repo.Put(ctx, child)).To(Succeed())
|
2025-11-14 22:15:43 -05:00
|
|
|
|
|
|
|
|
// Create similar path in other library
|
|
|
|
|
otherParent := model.NewFolder(otherLib, "TestIsolation/Parent")
|
|
|
|
|
otherChild := model.NewFolder(otherLib, "TestIsolation/Parent/Child")
|
|
|
|
|
|
2026-09-25 18:06:10 -04:00
|
|
|
Expect(repo.Put(ctx, otherParent)).To(Succeed())
|
|
|
|
|
Expect(repo.Put(ctx, otherChild)).To(Succeed())
|
2025-11-14 22:15:43 -05:00
|
|
|
|
|
|
|
|
// Query should only return folders from testLib
|
2026-09-25 18:06:10 -04:00
|
|
|
results, err := repo.GetFolderUpdateInfo(ctx, testLib, "TestIsolation/Parent")
|
2025-11-14 22:15:43 -05:00
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
Expect(results).To(HaveLen(2))
|
|
|
|
|
Expect(results).To(HaveKey(parent.ID))
|
|
|
|
|
Expect(results).To(HaveKey(child.ID))
|
|
|
|
|
Expect(results).ToNot(HaveKey(otherParent.ID))
|
|
|
|
|
Expect(results).ToNot(HaveKey(otherChild.ID))
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("excludes missing children when querying parent", func() {
|
|
|
|
|
// Create parent and children, mark one as missing
|
|
|
|
|
parent := model.NewFolder(testLib, "TestMissingChild/Parent")
|
|
|
|
|
child1 := model.NewFolder(testLib, "TestMissingChild/Parent/Child1")
|
|
|
|
|
child2 := model.NewFolder(testLib, "TestMissingChild/Parent/Child2")
|
|
|
|
|
child2.Missing = true
|
|
|
|
|
|
2026-09-25 18:06:10 -04:00
|
|
|
Expect(repo.Put(ctx, parent)).To(Succeed())
|
|
|
|
|
Expect(repo.Put(ctx, child1)).To(Succeed())
|
|
|
|
|
Expect(repo.Put(ctx, child2)).To(Succeed())
|
2025-11-14 22:15:43 -05:00
|
|
|
|
|
|
|
|
// Query parent - should only return parent and non-missing child
|
2026-09-25 18:06:10 -04:00
|
|
|
results, err := repo.GetFolderUpdateInfo(ctx, testLib, "TestMissingChild/Parent")
|
2025-11-14 22:15:43 -05:00
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
Expect(results).To(HaveLen(2))
|
|
|
|
|
Expect(results).To(HaveKey(parent.ID))
|
|
|
|
|
Expect(results).To(HaveKey(child1.ID))
|
|
|
|
|
Expect(results).ToNot(HaveKey(child2.ID))
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("handles mix of existing and non-existing target paths", func() {
|
|
|
|
|
// Create folders for one path but not the other
|
|
|
|
|
existingParent := model.NewFolder(testLib, "TestMixed/Exists")
|
|
|
|
|
existingChild := model.NewFolder(testLib, "TestMixed/Exists/Child")
|
|
|
|
|
|
2026-09-25 18:06:10 -04:00
|
|
|
Expect(repo.Put(ctx, existingParent)).To(Succeed())
|
|
|
|
|
Expect(repo.Put(ctx, existingChild)).To(Succeed())
|
2025-11-14 22:15:43 -05:00
|
|
|
|
|
|
|
|
// Query both existing and non-existing paths
|
2026-09-25 18:06:10 -04:00
|
|
|
results, err := repo.GetFolderUpdateInfo(ctx, testLib, "TestMixed/Exists", "TestMixed/DoesNotExist")
|
2025-11-14 22:15:43 -05:00
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
Expect(results).To(HaveLen(2))
|
|
|
|
|
Expect(results).To(HaveKey(existingParent.ID))
|
|
|
|
|
Expect(results).To(HaveKey(existingChild.ID))
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("handles empty folder path as root", func() {
|
|
|
|
|
// Test querying for root folder without creating it (fixtures should have one)
|
|
|
|
|
rootFolderID := model.FolderID(testLib, ".")
|
|
|
|
|
|
2026-09-25 18:06:10 -04:00
|
|
|
results, err := repo.GetFolderUpdateInfo(ctx, testLib, "")
|
2025-11-14 22:15:43 -05:00
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
// Should return the root folder if it exists
|
|
|
|
|
if len(results) > 0 {
|
|
|
|
|
Expect(results).To(HaveKey(rootFolderID))
|
|
|
|
|
}
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("returns empty map for non-existent folders", func() {
|
2026-09-25 18:06:10 -04:00
|
|
|
results, err := repo.GetFolderUpdateInfo(ctx, testLib, "NonExistent/Path")
|
2025-11-14 22:15:43 -05:00
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
Expect(results).To(BeEmpty())
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("skips missing folders", func() {
|
|
|
|
|
// Create a folder and mark it as missing
|
|
|
|
|
folder := model.NewFolder(testLib, "TestMissing/Folder")
|
|
|
|
|
folder.Missing = true
|
2026-09-25 18:06:10 -04:00
|
|
|
err := repo.Put(ctx, folder)
|
2025-11-14 22:15:43 -05:00
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
|
2026-09-25 18:06:10 -04:00
|
|
|
results, err := repo.GetFolderUpdateInfo(ctx, testLib, "TestMissing/Folder")
|
2025-11-14 22:15:43 -05:00
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
Expect(results).To(BeEmpty())
|
|
|
|
|
})
|
|
|
|
|
})
|
|
|
|
|
})
|
2026-03-03 07:58:14 -05:00
|
|
|
|
fix(artwork): never serve artist folder images as album art (#5596)
* test(artwork): add failing e2e tests for artist image leaking as album art
Reproduces a v0.62.0 regression (#5451/#5457): the album cover-art
parent-folder fallback can include the artist folder, serving the artist
thumbnail (e.g. Artist/folder.jpg) as album art for any album without
image files in its own folder(s). Covers three scenarios: a plain
Artist/Album layout with no album images, a single-disc album spread
across sibling folders under the artist folder, and a spread album whose
own front.jpg is shadowed by the artist's cover.jpg via CoverArtPriority
order. Also adds an albumByName test helper for multi-album layouts.
The tests are expected to fail until the parent-folder inclusion is
gated by a structural check (skip the common parent when audio from
other albums lives under it).
* fix(artwork): never serve artist folder images as album art
The album cover-art parent-folder fallback (introduced in #5451/#5457)
could include the artist folder as a source of album images, serving the
artist thumbnail (e.g. Artist/folder.jpg) as cover art for any album
without image files in its own folder(s). This affected both plain
Artist/Album layouts and single-disc albums spread across sibling
folders under the artist folder.
Gate the common-parent inclusion with a structural check: the parent
only qualifies as an album root when no audio belonging to other albums
lives in it or anywhere beneath it. An artist folder contains other
albums' tracks, while an album root above disc subfolders contains only
this album's, so the check works for any disc folder naming scheme and
never affects the multi-disc fixes from #5376/#5456. A single-album
artist with no images anywhere remains structurally indistinguishable
from an album root and is a known residual case.
* refactor(artwork): move album-root audio check into folder repository
Replace the raw subtree SQL (LIKE/ESCAPE expression and wildcard
escaping) that lived in core/artwork with an explicit
FolderRepository.HasAudioOutsideFolders method, implemented in the
persistence layer next to the existing folder-subtree query pattern.
This also removes the test mock's brittle dispatch that sniffed the
generated SQL to recognize the query; the fake now overrides the new
method directly.
Extract the whole parent-folder resolution from loadAlbumFoldersPaths
into an albumRootParent helper, flattening four levels of nesting back
into a linear flow. Behavior is unchanged; the unit test for a parent
containing audio moved to the persistence suite, with added coverage
for subtree boundaries, missing folders, and LIKE-wildcard escaping in
folder paths.
* refactor(persistence): use exists helper in HasAudioOutsideFolders
Replace the hand-rolled count(*) query with the repository's canonical
exists helper, as suggested in PR review.
2026-06-13 13:29:29 -04:00
|
|
|
Describe("HasAudioOutsideFolders", func() {
|
|
|
|
|
var albumRoot, disc1, disc2 *model.Folder
|
|
|
|
|
|
|
|
|
|
// TestHasAudio/Album/
|
|
|
|
|
// ├── CD1/ (audio, belongs to the album)
|
|
|
|
|
// └── CD2/ (audio, belongs to the album)
|
|
|
|
|
BeforeEach(func() {
|
|
|
|
|
albumRoot = model.NewFolder(testLib, "TestHasAudio/Album")
|
|
|
|
|
disc1 = model.NewFolder(testLib, "TestHasAudio/Album/CD1")
|
|
|
|
|
disc1.NumAudioFiles = 5
|
|
|
|
|
disc2 = model.NewFolder(testLib, "TestHasAudio/Album/CD2")
|
|
|
|
|
disc2.NumAudioFiles = 5
|
|
|
|
|
for _, f := range []*model.Folder{albumRoot, disc1, disc2} {
|
2026-09-25 18:06:10 -04:00
|
|
|
Expect(repo.Put(ctx, f)).To(Succeed())
|
fix(artwork): never serve artist folder images as album art (#5596)
* test(artwork): add failing e2e tests for artist image leaking as album art
Reproduces a v0.62.0 regression (#5451/#5457): the album cover-art
parent-folder fallback can include the artist folder, serving the artist
thumbnail (e.g. Artist/folder.jpg) as album art for any album without
image files in its own folder(s). Covers three scenarios: a plain
Artist/Album layout with no album images, a single-disc album spread
across sibling folders under the artist folder, and a spread album whose
own front.jpg is shadowed by the artist's cover.jpg via CoverArtPriority
order. Also adds an albumByName test helper for multi-album layouts.
The tests are expected to fail until the parent-folder inclusion is
gated by a structural check (skip the common parent when audio from
other albums lives under it).
* fix(artwork): never serve artist folder images as album art
The album cover-art parent-folder fallback (introduced in #5451/#5457)
could include the artist folder as a source of album images, serving the
artist thumbnail (e.g. Artist/folder.jpg) as cover art for any album
without image files in its own folder(s). This affected both plain
Artist/Album layouts and single-disc albums spread across sibling
folders under the artist folder.
Gate the common-parent inclusion with a structural check: the parent
only qualifies as an album root when no audio belonging to other albums
lives in it or anywhere beneath it. An artist folder contains other
albums' tracks, while an album root above disc subfolders contains only
this album's, so the check works for any disc folder naming scheme and
never affects the multi-disc fixes from #5376/#5456. A single-album
artist with no images anywhere remains structurally indistinguishable
from an album root and is a known residual case.
* refactor(artwork): move album-root audio check into folder repository
Replace the raw subtree SQL (LIKE/ESCAPE expression and wildcard
escaping) that lived in core/artwork with an explicit
FolderRepository.HasAudioOutsideFolders method, implemented in the
persistence layer next to the existing folder-subtree query pattern.
This also removes the test mock's brittle dispatch that sniffed the
generated SQL to recognize the query; the fake now overrides the new
method directly.
Extract the whole parent-folder resolution from loadAlbumFoldersPaths
into an albumRootParent helper, flattening four levels of nesting back
into a linear flow. Behavior is unchanged; the unit test for a parent
containing audio moved to the persistence suite, with added coverage
for subtree boundaries, missing folders, and LIKE-wildcard escaping in
folder paths.
* refactor(persistence): use exists helper in HasAudioOutsideFolders
Replace the hand-rolled count(*) query with the repository's canonical
exists helper, as suggested in PR review.
2026-06-13 13:29:29 -04:00
|
|
|
}
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("returns false when all audio under the parent belongs to the given folders", func() {
|
2026-09-25 18:06:10 -04:00
|
|
|
Expect(repo.HasAudioOutsideFolders(ctx, *albumRoot, []string{disc1.ID, disc2.ID})).To(BeFalse())
|
fix(artwork): never serve artist folder images as album art (#5596)
* test(artwork): add failing e2e tests for artist image leaking as album art
Reproduces a v0.62.0 regression (#5451/#5457): the album cover-art
parent-folder fallback can include the artist folder, serving the artist
thumbnail (e.g. Artist/folder.jpg) as album art for any album without
image files in its own folder(s). Covers three scenarios: a plain
Artist/Album layout with no album images, a single-disc album spread
across sibling folders under the artist folder, and a spread album whose
own front.jpg is shadowed by the artist's cover.jpg via CoverArtPriority
order. Also adds an albumByName test helper for multi-album layouts.
The tests are expected to fail until the parent-folder inclusion is
gated by a structural check (skip the common parent when audio from
other albums lives under it).
* fix(artwork): never serve artist folder images as album art
The album cover-art parent-folder fallback (introduced in #5451/#5457)
could include the artist folder as a source of album images, serving the
artist thumbnail (e.g. Artist/folder.jpg) as cover art for any album
without image files in its own folder(s). This affected both plain
Artist/Album layouts and single-disc albums spread across sibling
folders under the artist folder.
Gate the common-parent inclusion with a structural check: the parent
only qualifies as an album root when no audio belonging to other albums
lives in it or anywhere beneath it. An artist folder contains other
albums' tracks, while an album root above disc subfolders contains only
this album's, so the check works for any disc folder naming scheme and
never affects the multi-disc fixes from #5376/#5456. A single-album
artist with no images anywhere remains structurally indistinguishable
from an album root and is a known residual case.
* refactor(artwork): move album-root audio check into folder repository
Replace the raw subtree SQL (LIKE/ESCAPE expression and wildcard
escaping) that lived in core/artwork with an explicit
FolderRepository.HasAudioOutsideFolders method, implemented in the
persistence layer next to the existing folder-subtree query pattern.
This also removes the test mock's brittle dispatch that sniffed the
generated SQL to recognize the query; the fake now overrides the new
method directly.
Extract the whole parent-folder resolution from loadAlbumFoldersPaths
into an albumRootParent helper, flattening four levels of nesting back
into a linear flow. Behavior is unchanged; the unit test for a parent
containing audio moved to the persistence suite, with added coverage
for subtree boundaries, missing folders, and LIKE-wildcard escaping in
folder paths.
* refactor(persistence): use exists helper in HasAudioOutsideFolders
Replace the hand-rolled count(*) query with the repository's canonical
exists helper, as suggested in PR review.
2026-06-13 13:29:29 -04:00
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("returns true when another folder under the parent has audio", func() {
|
|
|
|
|
bonus := model.NewFolder(testLib, "TestHasAudio/Album/Bonus")
|
|
|
|
|
bonus.NumAudioFiles = 1
|
2026-09-25 18:06:10 -04:00
|
|
|
Expect(repo.Put(ctx, bonus)).To(Succeed())
|
fix(artwork): never serve artist folder images as album art (#5596)
* test(artwork): add failing e2e tests for artist image leaking as album art
Reproduces a v0.62.0 regression (#5451/#5457): the album cover-art
parent-folder fallback can include the artist folder, serving the artist
thumbnail (e.g. Artist/folder.jpg) as album art for any album without
image files in its own folder(s). Covers three scenarios: a plain
Artist/Album layout with no album images, a single-disc album spread
across sibling folders under the artist folder, and a spread album whose
own front.jpg is shadowed by the artist's cover.jpg via CoverArtPriority
order. Also adds an albumByName test helper for multi-album layouts.
The tests are expected to fail until the parent-folder inclusion is
gated by a structural check (skip the common parent when audio from
other albums lives under it).
* fix(artwork): never serve artist folder images as album art
The album cover-art parent-folder fallback (introduced in #5451/#5457)
could include the artist folder as a source of album images, serving the
artist thumbnail (e.g. Artist/folder.jpg) as cover art for any album
without image files in its own folder(s). This affected both plain
Artist/Album layouts and single-disc albums spread across sibling
folders under the artist folder.
Gate the common-parent inclusion with a structural check: the parent
only qualifies as an album root when no audio belonging to other albums
lives in it or anywhere beneath it. An artist folder contains other
albums' tracks, while an album root above disc subfolders contains only
this album's, so the check works for any disc folder naming scheme and
never affects the multi-disc fixes from #5376/#5456. A single-album
artist with no images anywhere remains structurally indistinguishable
from an album root and is a known residual case.
* refactor(artwork): move album-root audio check into folder repository
Replace the raw subtree SQL (LIKE/ESCAPE expression and wildcard
escaping) that lived in core/artwork with an explicit
FolderRepository.HasAudioOutsideFolders method, implemented in the
persistence layer next to the existing folder-subtree query pattern.
This also removes the test mock's brittle dispatch that sniffed the
generated SQL to recognize the query; the fake now overrides the new
method directly.
Extract the whole parent-folder resolution from loadAlbumFoldersPaths
into an albumRootParent helper, flattening four levels of nesting back
into a linear flow. Behavior is unchanged; the unit test for a parent
containing audio moved to the persistence suite, with added coverage
for subtree boundaries, missing folders, and LIKE-wildcard escaping in
folder paths.
* refactor(persistence): use exists helper in HasAudioOutsideFolders
Replace the hand-rolled count(*) query with the repository's canonical
exists helper, as suggested in PR review.
2026-06-13 13:29:29 -04:00
|
|
|
|
2026-09-25 18:06:10 -04:00
|
|
|
Expect(repo.HasAudioOutsideFolders(ctx, *albumRoot, []string{disc1.ID, disc2.ID})).To(BeTrue())
|
fix(artwork): never serve artist folder images as album art (#5596)
* test(artwork): add failing e2e tests for artist image leaking as album art
Reproduces a v0.62.0 regression (#5451/#5457): the album cover-art
parent-folder fallback can include the artist folder, serving the artist
thumbnail (e.g. Artist/folder.jpg) as album art for any album without
image files in its own folder(s). Covers three scenarios: a plain
Artist/Album layout with no album images, a single-disc album spread
across sibling folders under the artist folder, and a spread album whose
own front.jpg is shadowed by the artist's cover.jpg via CoverArtPriority
order. Also adds an albumByName test helper for multi-album layouts.
The tests are expected to fail until the parent-folder inclusion is
gated by a structural check (skip the common parent when audio from
other albums lives under it).
* fix(artwork): never serve artist folder images as album art
The album cover-art parent-folder fallback (introduced in #5451/#5457)
could include the artist folder as a source of album images, serving the
artist thumbnail (e.g. Artist/folder.jpg) as cover art for any album
without image files in its own folder(s). This affected both plain
Artist/Album layouts and single-disc albums spread across sibling
folders under the artist folder.
Gate the common-parent inclusion with a structural check: the parent
only qualifies as an album root when no audio belonging to other albums
lives in it or anywhere beneath it. An artist folder contains other
albums' tracks, while an album root above disc subfolders contains only
this album's, so the check works for any disc folder naming scheme and
never affects the multi-disc fixes from #5376/#5456. A single-album
artist with no images anywhere remains structurally indistinguishable
from an album root and is a known residual case.
* refactor(artwork): move album-root audio check into folder repository
Replace the raw subtree SQL (LIKE/ESCAPE expression and wildcard
escaping) that lived in core/artwork with an explicit
FolderRepository.HasAudioOutsideFolders method, implemented in the
persistence layer next to the existing folder-subtree query pattern.
This also removes the test mock's brittle dispatch that sniffed the
generated SQL to recognize the query; the fake now overrides the new
method directly.
Extract the whole parent-folder resolution from loadAlbumFoldersPaths
into an albumRootParent helper, flattening four levels of nesting back
into a linear flow. Behavior is unchanged; the unit test for a parent
containing audio moved to the persistence suite, with added coverage
for subtree boundaries, missing folders, and LIKE-wildcard escaping in
folder paths.
* refactor(persistence): use exists helper in HasAudioOutsideFolders
Replace the hand-rolled count(*) query with the repository's canonical
exists helper, as suggested in PR review.
2026-06-13 13:29:29 -04:00
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("returns true when the parent itself contains audio files", func() {
|
|
|
|
|
albumRoot.NumAudioFiles = 2
|
|
|
|
|
|
2026-09-25 18:06:10 -04:00
|
|
|
Expect(repo.HasAudioOutsideFolders(ctx, *albumRoot, []string{disc1.ID, disc2.ID})).To(BeTrue())
|
fix(artwork): never serve artist folder images as album art (#5596)
* test(artwork): add failing e2e tests for artist image leaking as album art
Reproduces a v0.62.0 regression (#5451/#5457): the album cover-art
parent-folder fallback can include the artist folder, serving the artist
thumbnail (e.g. Artist/folder.jpg) as album art for any album without
image files in its own folder(s). Covers three scenarios: a plain
Artist/Album layout with no album images, a single-disc album spread
across sibling folders under the artist folder, and a spread album whose
own front.jpg is shadowed by the artist's cover.jpg via CoverArtPriority
order. Also adds an albumByName test helper for multi-album layouts.
The tests are expected to fail until the parent-folder inclusion is
gated by a structural check (skip the common parent when audio from
other albums lives under it).
* fix(artwork): never serve artist folder images as album art
The album cover-art parent-folder fallback (introduced in #5451/#5457)
could include the artist folder as a source of album images, serving the
artist thumbnail (e.g. Artist/folder.jpg) as cover art for any album
without image files in its own folder(s). This affected both plain
Artist/Album layouts and single-disc albums spread across sibling
folders under the artist folder.
Gate the common-parent inclusion with a structural check: the parent
only qualifies as an album root when no audio belonging to other albums
lives in it or anywhere beneath it. An artist folder contains other
albums' tracks, while an album root above disc subfolders contains only
this album's, so the check works for any disc folder naming scheme and
never affects the multi-disc fixes from #5376/#5456. A single-album
artist with no images anywhere remains structurally indistinguishable
from an album root and is a known residual case.
* refactor(artwork): move album-root audio check into folder repository
Replace the raw subtree SQL (LIKE/ESCAPE expression and wildcard
escaping) that lived in core/artwork with an explicit
FolderRepository.HasAudioOutsideFolders method, implemented in the
persistence layer next to the existing folder-subtree query pattern.
This also removes the test mock's brittle dispatch that sniffed the
generated SQL to recognize the query; the fake now overrides the new
method directly.
Extract the whole parent-folder resolution from loadAlbumFoldersPaths
into an albumRootParent helper, flattening four levels of nesting back
into a linear flow. Behavior is unchanged; the unit test for a parent
containing audio moved to the persistence suite, with added coverage
for subtree boundaries, missing folders, and LIKE-wildcard escaping in
folder paths.
* refactor(persistence): use exists helper in HasAudioOutsideFolders
Replace the hand-rolled count(*) query with the repository's canonical
exists helper, as suggested in PR review.
2026-06-13 13:29:29 -04:00
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("ignores audio outside the parent's subtree", func() {
|
|
|
|
|
other := model.NewFolder(testLib, "TestHasAudio/Other Album")
|
|
|
|
|
other.NumAudioFiles = 10
|
2026-09-25 18:06:10 -04:00
|
|
|
Expect(repo.Put(ctx, other)).To(Succeed())
|
fix(artwork): never serve artist folder images as album art (#5596)
* test(artwork): add failing e2e tests for artist image leaking as album art
Reproduces a v0.62.0 regression (#5451/#5457): the album cover-art
parent-folder fallback can include the artist folder, serving the artist
thumbnail (e.g. Artist/folder.jpg) as album art for any album without
image files in its own folder(s). Covers three scenarios: a plain
Artist/Album layout with no album images, a single-disc album spread
across sibling folders under the artist folder, and a spread album whose
own front.jpg is shadowed by the artist's cover.jpg via CoverArtPriority
order. Also adds an albumByName test helper for multi-album layouts.
The tests are expected to fail until the parent-folder inclusion is
gated by a structural check (skip the common parent when audio from
other albums lives under it).
* fix(artwork): never serve artist folder images as album art
The album cover-art parent-folder fallback (introduced in #5451/#5457)
could include the artist folder as a source of album images, serving the
artist thumbnail (e.g. Artist/folder.jpg) as cover art for any album
without image files in its own folder(s). This affected both plain
Artist/Album layouts and single-disc albums spread across sibling
folders under the artist folder.
Gate the common-parent inclusion with a structural check: the parent
only qualifies as an album root when no audio belonging to other albums
lives in it or anywhere beneath it. An artist folder contains other
albums' tracks, while an album root above disc subfolders contains only
this album's, so the check works for any disc folder naming scheme and
never affects the multi-disc fixes from #5376/#5456. A single-album
artist with no images anywhere remains structurally indistinguishable
from an album root and is a known residual case.
* refactor(artwork): move album-root audio check into folder repository
Replace the raw subtree SQL (LIKE/ESCAPE expression and wildcard
escaping) that lived in core/artwork with an explicit
FolderRepository.HasAudioOutsideFolders method, implemented in the
persistence layer next to the existing folder-subtree query pattern.
This also removes the test mock's brittle dispatch that sniffed the
generated SQL to recognize the query; the fake now overrides the new
method directly.
Extract the whole parent-folder resolution from loadAlbumFoldersPaths
into an albumRootParent helper, flattening four levels of nesting back
into a linear flow. Behavior is unchanged; the unit test for a parent
containing audio moved to the persistence suite, with added coverage
for subtree boundaries, missing folders, and LIKE-wildcard escaping in
folder paths.
* refactor(persistence): use exists helper in HasAudioOutsideFolders
Replace the hand-rolled count(*) query with the repository's canonical
exists helper, as suggested in PR review.
2026-06-13 13:29:29 -04:00
|
|
|
|
2026-09-25 18:06:10 -04:00
|
|
|
Expect(repo.HasAudioOutsideFolders(ctx, *albumRoot, []string{disc1.ID, disc2.ID})).To(BeFalse())
|
fix(artwork): never serve artist folder images as album art (#5596)
* test(artwork): add failing e2e tests for artist image leaking as album art
Reproduces a v0.62.0 regression (#5451/#5457): the album cover-art
parent-folder fallback can include the artist folder, serving the artist
thumbnail (e.g. Artist/folder.jpg) as album art for any album without
image files in its own folder(s). Covers three scenarios: a plain
Artist/Album layout with no album images, a single-disc album spread
across sibling folders under the artist folder, and a spread album whose
own front.jpg is shadowed by the artist's cover.jpg via CoverArtPriority
order. Also adds an albumByName test helper for multi-album layouts.
The tests are expected to fail until the parent-folder inclusion is
gated by a structural check (skip the common parent when audio from
other albums lives under it).
* fix(artwork): never serve artist folder images as album art
The album cover-art parent-folder fallback (introduced in #5451/#5457)
could include the artist folder as a source of album images, serving the
artist thumbnail (e.g. Artist/folder.jpg) as cover art for any album
without image files in its own folder(s). This affected both plain
Artist/Album layouts and single-disc albums spread across sibling
folders under the artist folder.
Gate the common-parent inclusion with a structural check: the parent
only qualifies as an album root when no audio belonging to other albums
lives in it or anywhere beneath it. An artist folder contains other
albums' tracks, while an album root above disc subfolders contains only
this album's, so the check works for any disc folder naming scheme and
never affects the multi-disc fixes from #5376/#5456. A single-album
artist with no images anywhere remains structurally indistinguishable
from an album root and is a known residual case.
* refactor(artwork): move album-root audio check into folder repository
Replace the raw subtree SQL (LIKE/ESCAPE expression and wildcard
escaping) that lived in core/artwork with an explicit
FolderRepository.HasAudioOutsideFolders method, implemented in the
persistence layer next to the existing folder-subtree query pattern.
This also removes the test mock's brittle dispatch that sniffed the
generated SQL to recognize the query; the fake now overrides the new
method directly.
Extract the whole parent-folder resolution from loadAlbumFoldersPaths
into an albumRootParent helper, flattening four levels of nesting back
into a linear flow. Behavior is unchanged; the unit test for a parent
containing audio moved to the persistence suite, with added coverage
for subtree boundaries, missing folders, and LIKE-wildcard escaping in
folder paths.
* refactor(persistence): use exists helper in HasAudioOutsideFolders
Replace the hand-rolled count(*) query with the repository's canonical
exists helper, as suggested in PR review.
2026-06-13 13:29:29 -04:00
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("ignores missing folders", func() {
|
|
|
|
|
gone := model.NewFolder(testLib, "TestHasAudio/Album/Gone")
|
|
|
|
|
gone.NumAudioFiles = 3
|
|
|
|
|
gone.Missing = true
|
2026-09-25 18:06:10 -04:00
|
|
|
Expect(repo.Put(ctx, gone)).To(Succeed())
|
fix(artwork): never serve artist folder images as album art (#5596)
* test(artwork): add failing e2e tests for artist image leaking as album art
Reproduces a v0.62.0 regression (#5451/#5457): the album cover-art
parent-folder fallback can include the artist folder, serving the artist
thumbnail (e.g. Artist/folder.jpg) as album art for any album without
image files in its own folder(s). Covers three scenarios: a plain
Artist/Album layout with no album images, a single-disc album spread
across sibling folders under the artist folder, and a spread album whose
own front.jpg is shadowed by the artist's cover.jpg via CoverArtPriority
order. Also adds an albumByName test helper for multi-album layouts.
The tests are expected to fail until the parent-folder inclusion is
gated by a structural check (skip the common parent when audio from
other albums lives under it).
* fix(artwork): never serve artist folder images as album art
The album cover-art parent-folder fallback (introduced in #5451/#5457)
could include the artist folder as a source of album images, serving the
artist thumbnail (e.g. Artist/folder.jpg) as cover art for any album
without image files in its own folder(s). This affected both plain
Artist/Album layouts and single-disc albums spread across sibling
folders under the artist folder.
Gate the common-parent inclusion with a structural check: the parent
only qualifies as an album root when no audio belonging to other albums
lives in it or anywhere beneath it. An artist folder contains other
albums' tracks, while an album root above disc subfolders contains only
this album's, so the check works for any disc folder naming scheme and
never affects the multi-disc fixes from #5376/#5456. A single-album
artist with no images anywhere remains structurally indistinguishable
from an album root and is a known residual case.
* refactor(artwork): move album-root audio check into folder repository
Replace the raw subtree SQL (LIKE/ESCAPE expression and wildcard
escaping) that lived in core/artwork with an explicit
FolderRepository.HasAudioOutsideFolders method, implemented in the
persistence layer next to the existing folder-subtree query pattern.
This also removes the test mock's brittle dispatch that sniffed the
generated SQL to recognize the query; the fake now overrides the new
method directly.
Extract the whole parent-folder resolution from loadAlbumFoldersPaths
into an albumRootParent helper, flattening four levels of nesting back
into a linear flow. Behavior is unchanged; the unit test for a parent
containing audio moved to the persistence suite, with added coverage
for subtree boundaries, missing folders, and LIKE-wildcard escaping in
folder paths.
* refactor(persistence): use exists helper in HasAudioOutsideFolders
Replace the hand-rolled count(*) query with the repository's canonical
exists helper, as suggested in PR review.
2026-06-13 13:29:29 -04:00
|
|
|
|
2026-09-25 18:06:10 -04:00
|
|
|
Expect(repo.HasAudioOutsideFolders(ctx, *albumRoot, []string{disc1.ID, disc2.ID})).To(BeFalse())
|
fix(artwork): never serve artist folder images as album art (#5596)
* test(artwork): add failing e2e tests for artist image leaking as album art
Reproduces a v0.62.0 regression (#5451/#5457): the album cover-art
parent-folder fallback can include the artist folder, serving the artist
thumbnail (e.g. Artist/folder.jpg) as album art for any album without
image files in its own folder(s). Covers three scenarios: a plain
Artist/Album layout with no album images, a single-disc album spread
across sibling folders under the artist folder, and a spread album whose
own front.jpg is shadowed by the artist's cover.jpg via CoverArtPriority
order. Also adds an albumByName test helper for multi-album layouts.
The tests are expected to fail until the parent-folder inclusion is
gated by a structural check (skip the common parent when audio from
other albums lives under it).
* fix(artwork): never serve artist folder images as album art
The album cover-art parent-folder fallback (introduced in #5451/#5457)
could include the artist folder as a source of album images, serving the
artist thumbnail (e.g. Artist/folder.jpg) as cover art for any album
without image files in its own folder(s). This affected both plain
Artist/Album layouts and single-disc albums spread across sibling
folders under the artist folder.
Gate the common-parent inclusion with a structural check: the parent
only qualifies as an album root when no audio belonging to other albums
lives in it or anywhere beneath it. An artist folder contains other
albums' tracks, while an album root above disc subfolders contains only
this album's, so the check works for any disc folder naming scheme and
never affects the multi-disc fixes from #5376/#5456. A single-album
artist with no images anywhere remains structurally indistinguishable
from an album root and is a known residual case.
* refactor(artwork): move album-root audio check into folder repository
Replace the raw subtree SQL (LIKE/ESCAPE expression and wildcard
escaping) that lived in core/artwork with an explicit
FolderRepository.HasAudioOutsideFolders method, implemented in the
persistence layer next to the existing folder-subtree query pattern.
This also removes the test mock's brittle dispatch that sniffed the
generated SQL to recognize the query; the fake now overrides the new
method directly.
Extract the whole parent-folder resolution from loadAlbumFoldersPaths
into an albumRootParent helper, flattening four levels of nesting back
into a linear flow. Behavior is unchanged; the unit test for a parent
containing audio moved to the persistence suite, with added coverage
for subtree boundaries, missing folders, and LIKE-wildcard escaping in
folder paths.
* refactor(persistence): use exists helper in HasAudioOutsideFolders
Replace the hand-rolled count(*) query with the repository's canonical
exists helper, as suggested in PR review.
2026-06-13 13:29:29 -04:00
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("does not treat LIKE wildcards in the parent path as patterns", func() {
|
|
|
|
|
// "TestHas_udio" would LIKE-match "TestHasAudio" if "_" were not escaped
|
|
|
|
|
wildcardRoot := model.NewFolder(testLib, "TestHas_udio/Album")
|
2026-09-25 18:06:10 -04:00
|
|
|
Expect(repo.Put(ctx, wildcardRoot)).To(Succeed())
|
fix(artwork): never serve artist folder images as album art (#5596)
* test(artwork): add failing e2e tests for artist image leaking as album art
Reproduces a v0.62.0 regression (#5451/#5457): the album cover-art
parent-folder fallback can include the artist folder, serving the artist
thumbnail (e.g. Artist/folder.jpg) as album art for any album without
image files in its own folder(s). Covers three scenarios: a plain
Artist/Album layout with no album images, a single-disc album spread
across sibling folders under the artist folder, and a spread album whose
own front.jpg is shadowed by the artist's cover.jpg via CoverArtPriority
order. Also adds an albumByName test helper for multi-album layouts.
The tests are expected to fail until the parent-folder inclusion is
gated by a structural check (skip the common parent when audio from
other albums lives under it).
* fix(artwork): never serve artist folder images as album art
The album cover-art parent-folder fallback (introduced in #5451/#5457)
could include the artist folder as a source of album images, serving the
artist thumbnail (e.g. Artist/folder.jpg) as cover art for any album
without image files in its own folder(s). This affected both plain
Artist/Album layouts and single-disc albums spread across sibling
folders under the artist folder.
Gate the common-parent inclusion with a structural check: the parent
only qualifies as an album root when no audio belonging to other albums
lives in it or anywhere beneath it. An artist folder contains other
albums' tracks, while an album root above disc subfolders contains only
this album's, so the check works for any disc folder naming scheme and
never affects the multi-disc fixes from #5376/#5456. A single-album
artist with no images anywhere remains structurally indistinguishable
from an album root and is a known residual case.
* refactor(artwork): move album-root audio check into folder repository
Replace the raw subtree SQL (LIKE/ESCAPE expression and wildcard
escaping) that lived in core/artwork with an explicit
FolderRepository.HasAudioOutsideFolders method, implemented in the
persistence layer next to the existing folder-subtree query pattern.
This also removes the test mock's brittle dispatch that sniffed the
generated SQL to recognize the query; the fake now overrides the new
method directly.
Extract the whole parent-folder resolution from loadAlbumFoldersPaths
into an albumRootParent helper, flattening four levels of nesting back
into a linear flow. Behavior is unchanged; the unit test for a parent
containing audio moved to the persistence suite, with added coverage
for subtree boundaries, missing folders, and LIKE-wildcard escaping in
folder paths.
* refactor(persistence): use exists helper in HasAudioOutsideFolders
Replace the hand-rolled count(*) query with the repository's canonical
exists helper, as suggested in PR review.
2026-06-13 13:29:29 -04:00
|
|
|
|
2026-09-25 18:06:10 -04:00
|
|
|
Expect(repo.HasAudioOutsideFolders(ctx, *wildcardRoot, []string{"none"})).To(BeFalse())
|
fix(artwork): never serve artist folder images as album art (#5596)
* test(artwork): add failing e2e tests for artist image leaking as album art
Reproduces a v0.62.0 regression (#5451/#5457): the album cover-art
parent-folder fallback can include the artist folder, serving the artist
thumbnail (e.g. Artist/folder.jpg) as album art for any album without
image files in its own folder(s). Covers three scenarios: a plain
Artist/Album layout with no album images, a single-disc album spread
across sibling folders under the artist folder, and a spread album whose
own front.jpg is shadowed by the artist's cover.jpg via CoverArtPriority
order. Also adds an albumByName test helper for multi-album layouts.
The tests are expected to fail until the parent-folder inclusion is
gated by a structural check (skip the common parent when audio from
other albums lives under it).
* fix(artwork): never serve artist folder images as album art
The album cover-art parent-folder fallback (introduced in #5451/#5457)
could include the artist folder as a source of album images, serving the
artist thumbnail (e.g. Artist/folder.jpg) as cover art for any album
without image files in its own folder(s). This affected both plain
Artist/Album layouts and single-disc albums spread across sibling
folders under the artist folder.
Gate the common-parent inclusion with a structural check: the parent
only qualifies as an album root when no audio belonging to other albums
lives in it or anywhere beneath it. An artist folder contains other
albums' tracks, while an album root above disc subfolders contains only
this album's, so the check works for any disc folder naming scheme and
never affects the multi-disc fixes from #5376/#5456. A single-album
artist with no images anywhere remains structurally indistinguishable
from an album root and is a known residual case.
* refactor(artwork): move album-root audio check into folder repository
Replace the raw subtree SQL (LIKE/ESCAPE expression and wildcard
escaping) that lived in core/artwork with an explicit
FolderRepository.HasAudioOutsideFolders method, implemented in the
persistence layer next to the existing folder-subtree query pattern.
This also removes the test mock's brittle dispatch that sniffed the
generated SQL to recognize the query; the fake now overrides the new
method directly.
Extract the whole parent-folder resolution from loadAlbumFoldersPaths
into an albumRootParent helper, flattening four levels of nesting back
into a linear flow. Behavior is unchanged; the unit test for a parent
containing audio moved to the persistence suite, with added coverage
for subtree boundaries, missing folders, and LIKE-wildcard escaping in
folder paths.
* refactor(persistence): use exists helper in HasAudioOutsideFolders
Replace the hand-rolled count(*) query with the repository's canonical
exists helper, as suggested in PR review.
2026-06-13 13:29:29 -04:00
|
|
|
})
|
|
|
|
|
})
|
|
|
|
|
|
2026-09-20 15:27:20 -04:00
|
|
|
Describe("dbFolder.String", func() {
|
|
|
|
|
It("does not dereference a nil Folder", func() {
|
|
|
|
|
Expect(fmt.Sprint(dbFolder{})).To(Equal("<nil>"))
|
|
|
|
|
Expect(fmt.Sprint(&dbFolder{})).To(Equal("<nil>"))
|
|
|
|
|
})
|
|
|
|
|
})
|
|
|
|
|
|
2026-03-03 07:58:14 -05:00
|
|
|
Describe("wrapFolderCursor", func() {
|
|
|
|
|
It("does not panic when the cursor yields a dbFolder with nil Folder", func() {
|
|
|
|
|
// Simulate what queryWithStableResults does on the rows.Err() path:
|
|
|
|
|
// it yields a zero-value dbFolder (where Folder is nil) with an error.
|
|
|
|
|
dbErr := fmt.Errorf("database is locked")
|
|
|
|
|
cursor := func(yield func(dbFolder, error) bool) {
|
|
|
|
|
var empty dbFolder // Folder pointer is nil
|
|
|
|
|
yield(empty, dbErr)
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
// wrapFolderCursor should handle the nil Folder without panicking
|
|
|
|
|
wrappedCursor := wrapFolderCursor(cursor)
|
|
|
|
|
var gotErr error
|
|
|
|
|
Expect(func() {
|
|
|
|
|
for _, err := range wrappedCursor {
|
|
|
|
|
gotErr = err
|
|
|
|
|
}
|
|
|
|
|
}).ToNot(Panic())
|
|
|
|
|
Expect(gotErr).To(HaveOccurred())
|
fix(jellyfin): stream collection responses to prevent OOM on large libraries (#5783)
* fix(jellyfin): stream /Items responses to prevent OOM on large libraries
Finamp's library sync issues an unbounded GET /Items?IncludeItemTypes=Audio
with Fields=MediaSources and no Limit. On a large library this built the whole
result set — every MediaFile and every BaseItemDto, each fat with MediaSources —
in memory, and json.Encoder then buffered the entire ~200MB response before
writing a byte. Measured on a 96k-track library, one such request peaked near
1.6GB RSS; in a memory-limited container the resulting slowness made clients
retry, stacking concurrent full-library builds until the process was OOM-killed.
Stream the song listing straight from a DB cursor (MediaFileRepository.GetCursor),
mapping and encoding one row at a time, so peak memory is bounded to about one
item regardless of library size. The cursor is opened lazily, when streaming
begins, so it doesn't hold a DB connection across the CountAll and ServerId
lookups that run first (ServerId can write on first use and would deadlock
against an open reader). Pagination still works — the cursor query carries
LIMIT/OFFSET/ORDER BY from StartIndex/Limit/SortBy — and TotalRecordCount stays
the full count. Other materialized responses stream their JSON too, avoiding the
encoder's buffer-the-whole-output cost.
Also bound artwork image concurrency with the same server.ThrottleBacklog that
Subsonic's getCoverArt uses, so a burst of image requests during sync can't
exhaust memory through concurrent decode/resize.
Verified against a copy of a 96k-track production database: byte-for-byte
identical response, peak RSS 1593MB -> 53MB, time-to-first-byte 8.1s -> 0.2s.
* fix(jellyfin): fail loudly on /Items streaming errors instead of a truncated 200
Addresses code review: a streamed /Items response commits HTTP 200 before an
error can occur, so failures were surfacing as misleadingly successful bodies.
- A cursor-open failure (e.g. a busy DB under connection pressure) is now
surfaced before the first byte is written: the cursor open is deferred into
the item source and run in writeItems after the ServerId lookup but before the
envelope, returning a clean 500 the client can retry — not a 200 with an empty
item list.
- A mid-stream (row-scan) error now aborts without closing the JSON envelope,
leaving the body malformed. A truncated-but-valid response would let a sync
client (Finamp) treat the short list as the whole library and prune local
tracks; malformed JSON forces the client's parser to fail and retry.
* refactor(jellyfin): stream every collection endpoint from a DB cursor
Streaming was applied only to the song listing, which left two write paths, two
ServerId stamping mechanisms and a listXxx return-type split ("songs is
special") that made the code hard to follow.
Add GetCursor to the album, artist, genre and playlist repositories, mirroring
MediaFileRepository.GetCursor: same select builder as GetAll, so each cursor
yields identical, fully-hydrated rows (hydration happens per-row in PostScan,
and toModels is only a deref loop). The playlist cursor keeps GetAll's
owner/public visibility filter. Each repository test asserts the cursor yields
exactly what GetAll returns, including Max/Offset.
With cursors everywhere, every listXxx returns itemsResult and every collection
streams through one writer:
- listAlbums, listArtists and listPlaylists now stream from their cursors;
search paths stay materialized (Search returns a slice).
- listGenres stays materialized: its total is the length of the full list and it
paginates in memory, so there is nothing for a cursor to page over.
- api.ok's QueryResult case delegates to writeItems, so the ~9 inline callers
funnel into the same writer; streamQueryResult and wrapResult are gone.
- /Items/Latest streams as a bare JSON array (its Jellyfin wire shape) via
writeItemsArray, which shares the item loop and stamping. That also closes the
same unbounded hole /Items had: limit=0 disabled its LIMIT.
- ServerId is now stamped in exactly one place, so api.ok only handles single,
non-collection payloads.
Verified against a copy of a 96k-track production database: every endpoint
byte-for-byte identical to master. Unbounded /Items peak RSS 1691MB -> 54MB;
time-to-first-byte 6.3s -> 0.19s, /Artists 1.14s -> 0.13s.
* refactor(jellyfin): route every response through api.ok
Handlers were split between api.ok and api.writeItems with no clear rule for
which to call. api.ok now accepts itemsResult too, so it is the single entry
point: callers hand it whatever they have and it routes collections (cursor
-backed or materialized) to the streaming writer. writeItems is reached only
through api.ok now. The one exception is /Items/Latest, which returns a bare
JSON array rather than a QueryResult envelope and so writes directly — noted in
both doc comments.
Also drop GenreRepository.GetCursor: listGenres derives its total from the
length of the full list and paginates in memory, so there is nothing for a
cursor to page over, leaving the method unused.
* fix(jellyfin): stream the unbounded multi-type /Items merge
The multi-type merge capped each per-type query at offset+limit only when a
Limit was given. Without one (Finamp's favorites screen sends multi-type), every
type ran an unbounded query and collect() drained each cursor into a slice — so
/Items?IncludeItemTypes=MusicAlbum,Audio with no Limit materialized every album
and every song, the same OOM class this branch set out to fix. The comment
claiming the set was "capped at offset+limit per type" was wrong for limit=0.
Without a limit the merged page is just each type's rows in order minus the
first offset, which is exactly what chaining the per-type cursors yields, so
stream that instead of merging in memory. The bounded path is unchanged: with a
Limit each type holds at most offset+limit rows, so merging and paginating
across the combined list is safe. Cursors are opened one at a time (each pins a
DB connection until drained), with the first opened eagerly so the usual failure
is still a clean error before any byte is written.
Measured on a 96k-track library, /Items?IncludeItemTypes=MusicAlbum,Audio with
no Limit (103,393 items): peak RSS 612MB -> 53MB, time-to-first-byte 4.9s ->
0.6s, response byte-for-byte identical.
* refactor(jellyfin): parse /Items params into a struct
The /Items dispatcher was a single 90-line block that parsed a dozen params and
then threaded them positionally through three layers — queryItemsOfType took 11
arguments, listSongs and listAlbums 9 each — so reading any one of them meant
decoding a long argument list.
Parse once into an itemsQuery and pass that instead. queryItems now reads as the
four things it actually does: parse, the id/playlists-folder/playlist-parent
special cases, single-type dispatch, multi-type merge — with the playlist-parent
and merge bodies moved to playlistTracks and mergeTypes. Every listXxx takes
(ctx, opts, q).
No behavior change: parsing, ordering and the entityParent rule are unchanged,
and /Artists still passes favOnly=false explicitly by building the subset of the
query it uses.
* docs(jellyfin): trim comments on the streaming path
Cut the comments back to the non-obvious why: the deferred cursor open (the
ServerId write would deadlock against an open reader), the deliberate malformed
JSON on a mid-stream error, why chained opens one cursor at a time, why
listGenres stays materialized, and why the cursor helpers take the underlying
func type. Dropped the rest — restatements of the code, doc comments that
repeated a test's own name, and the GetCursor interface comments that the
existing MediaFileRepository.GetCursor does without.
* refactor(persistence): fold the cursor wrappers into a generic wrapCursor
wrapAlbumCursor, wrapArtistCursor, wrapMediaFileCursor and wrapFolderCursor were
the same twelve lines four times over, differing only in the type name, and the
playlist cursor had its own inline copy of the loop.
Add one generic wrapCursor. It takes an extractor func rather than a method on
an interface: a type parameter can't reach an embedded field, and methods that
only satisfy a generic constraint are reported by the unused linter. The
extractor also lets both type parameters be inferred, so call sites need no
explicit instantiation. Each entity keeps its named wrapper as a one-liner,
since the model cursor types are defined types and need the conversion — and the
existing wrapper tests call them directly.
The nil-row error now names the model type via %T ("unexpected nil model.Album")
instead of a hand-written per-entity string; the three tests asserting that
message are updated.
* fix(jellyfin): bound concurrent collection streams
A streamed collection holds a DB cursor — and its pooled connection — for the whole client-paced
response, where the old materialize-then-write path released it as soon as the query finished. The
pool is shared with the scanner, Subsonic, the native API and the UI, so enough slow clients take
every connection and everything else blocks waiting for one. Measured against a copy of a 96k-track
production DB, 20 concurrent unbounded streams against a pool of 16 stalled a write for 16.2s (the
other 14 writes in the run took ~2ms — the signature of connection starvation, not lock contention).
Cap concurrent streams at half the pool. Excess requests queue rather than fail, so no client is
rejected: the same 20 streams now all complete and the worst write is 2.5ms.
- conf.MaxOpenConns() now owns the pool sizing (db calls it). It belongs in conf: the pool is a
tunable, db already imports conf, and putting it in db would force server/jellyfin to import db
just to size the cap against it. Expressing the cap as MaxOpenConns()/2 also keeps the two from
drifting apart.
- Uses chi's ThrottleBacklog, not server.ThrottleBacklog: the latter buffers the whole response to
release its token early, which is right for artwork but would undo the streaming. chi's panics on a
non-positive limit, so throttleStreams guards it — setting MaxConcurrentStreams=0 disables the cap
instead of crashing the server at startup.
* refactor(jellyfin): move throttleStreams to middlewares.go
It's a middleware, so it belongs beside normalizeQueryKeys, authenticate and
withPlayer rather than in api.go. Its tests move to middlewares_test.go with it,
keeping one test file per production file.
* fix(jellyfin): abandon the scan when the client stops reading
encodeItems discarded its write errors and relied on the final Flush to report
them, so once bufio's buffer filled and the flush failed, the loop still pulled
every remaining row through the cursor and serialized it for a client that was
gone. A test with a failing writer confirms it: all 20k items were drained.
That wastes CPU, and holds the cursor's pooled DB connection and a stream slot
(now a capped resource) for the length of a full scan nobody is reading. Check
the per-item writes so the first failure ends the scan; the fixed envelope
writes stay unchecked, since bufio latches for them anyway.
2026-07-15 14:07:00 -04:00
|
|
|
Expect(gotErr.Error()).To(ContainSubstring("unexpected nil model.Folder"))
|
2026-03-03 07:58:14 -05:00
|
|
|
Expect(errors.Is(gotErr, dbErr)).To(BeTrue(), "should wrap the original cursor error")
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("yields folders from a valid cursor", func() {
|
|
|
|
|
folder := &model.Folder{ID: "f1", Name: "Test"}
|
|
|
|
|
cursor := func(yield func(dbFolder, error) bool) {
|
|
|
|
|
yield(dbFolder{Folder: folder}, nil)
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
wrappedCursor := wrapFolderCursor(cursor)
|
|
|
|
|
var folders []model.Folder
|
|
|
|
|
for f, err := range wrappedCursor {
|
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
folders = append(folders, f)
|
|
|
|
|
}
|
|
|
|
|
Expect(folders).To(HaveLen(1))
|
|
|
|
|
Expect(folders[0].ID).To(Equal("f1"))
|
|
|
|
|
})
|
|
|
|
|
})
|
2026-06-14 13:39:16 -04:00
|
|
|
|
|
|
|
|
Describe("GetAllWithPlaylists", func() {
|
|
|
|
|
It("returns all non-missing folders with playlists, ignoring the scan-timestamp gate", func() {
|
|
|
|
|
withPls := model.NewFolder(testLib, "TestAllPls/WithPls")
|
|
|
|
|
withPls.NumPlaylists = 2
|
|
|
|
|
noPls := model.NewFolder(testLib, "TestAllPls/NoPls")
|
|
|
|
|
noPls.NumPlaylists = 0
|
|
|
|
|
missingWithPls := model.NewFolder(testLib, "TestAllPls/Missing")
|
|
|
|
|
missingWithPls.NumPlaylists = 1
|
|
|
|
|
missingWithPls.Missing = true
|
|
|
|
|
|
2026-09-25 18:06:10 -04:00
|
|
|
Expect(repo.Put(ctx, withPls)).To(Succeed())
|
|
|
|
|
Expect(repo.Put(ctx, noPls)).To(Succeed())
|
|
|
|
|
Expect(repo.Put(ctx, missingWithPls)).To(Succeed())
|
2026-06-14 13:39:16 -04:00
|
|
|
|
|
|
|
|
// Force the folder's updated_at to the past so GetTouchedWithPlaylists
|
|
|
|
|
// (which gates on updated_at > last_scan_at) would NOT return it.
|
|
|
|
|
_, err := conn.NewQuery("UPDATE folder SET updated_at = {:t} WHERE id = {:id}").
|
|
|
|
|
Bind(dbx.Params{"t": "2000-01-01 00:00:00", "id": withPls.ID}).Execute()
|
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
|
|
|
|
|
var ids []string
|
2026-09-25 18:06:10 -04:00
|
|
|
cursor, err := repo.GetAllWithPlaylists(ctx)
|
2026-06-14 13:39:16 -04:00
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
for f, err := range cursor {
|
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
ids = append(ids, f.ID)
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
Expect(ids).To(ConsistOf(withPls.ID)) // only the non-missing folder with playlists
|
|
|
|
|
})
|
|
|
|
|
})
|
2025-11-14 22:15:43 -05:00
|
|
|
})
|