2026-04-24 23:18:20 -04:00
package persistence
import (
2026-05-22 18:00:13 -03:00
"fmt"
"strings"
2026-04-24 23:18:20 -04:00
"time"
refactor: centralize criteria sort parsing and extract smart playlist logic (#5415)
* test: add tests for recordingdate alias resolution in smart playlists
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: update FieldInfo structure and simplify fieldMap initialization
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: move sort parsing logic from persistence to criteria package
Extracted sort field parsing, validation, and direction handling from
persistence/criteria_sql.go into model/criteria/sort.go. The new
OrderByFields method on Criteria parses the Sort/Order strings into
validated SortField structs (field name + direction), resolving aliases
and handling +/- prefixes and order inversion. The persistence layer now
consumes these parsed fields and only handles SQL expression mapping.
This centralizes sort parsing to enforce consistent implementations.
* refactor: standardize field access in smartPlaylistCriteria structure
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: add ResolveLimit method to Criteria
Moved the percentage-limit resolution logic from playlist_repository
into Criteria.ResolveLimit, replacing the 3-line mutate-after-query
pattern with a single method call. The method preserves LimitPercent
rather than zeroing it, since IsPercentageLimit already returns false
once Limit is set, making the clear redundant and lossy.
* refactor: improve child playlist loading and error handling in refresh logic
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: extract smart playlist logic to dedicated files
Moved refreshSmartPlaylist, addSmartPlaylistAnnotationJoins, and
addCriteria methods from playlist_repository.go to a new
smart_playlist_repository.go file. Extracted all smart playlist tests
to smart_playlist_repository_test.go. Added DeferCleanup to the
"valid rules" test to fix ordering flakiness when Ginkgo randomizes
test execution across files.
* refactor: break refreshSmartPlaylist into smaller focused methods
Split the monolithic refreshSmartPlaylist method into discrete helpers
for readability: shouldRefreshSmartPlaylist for guard checks,
refreshChildPlaylists for recursive dependency refresh,
resolvePercentageLimit for count-based limit resolution,
buildSmartPlaylistQuery for assembling the SELECT with joins, and
addMediaFileAnnotationJoin to DRY up the repeated annotation join clause.
* refactor: deduplicate child playlist IDs in Criteria
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: simplify withSmartPlaylistOwner to accept model.User
Replaced separate ownerID string and ownerIsAdmin bool parameters with a
single model.User struct, reducing the field count in smartPlaylistCriteria
and making the option function signature clearer. Updated all call sites
and tests accordingly.
* fix: handle empty sort fields and propagate child playlist load errors
OrderByFields now falls back to [{title, asc}] when all user-supplied
sort fields are invalid, preventing empty ORDER BY clauses that would
produce invalid SQL in row_number() window functions. Also restored the
original behavior where a DB error loading child playlists aborts the
parent smart playlist refresh, by making refreshChildPlaylists return a
bool.
* refactor: log warning when no valid sort fields are found
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
2026-04-26 14:49:59 -04:00
"github.com/navidrome/navidrome/model"
2026-04-24 23:18:20 -04:00
"github.com/navidrome/navidrome/model/criteria"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
var _ = Describe ( "Smart playlist criteria SQL" , func ( ) {
BeforeEach ( func ( ) {
criteria . AddRoles ( [ ] string { "artist" , "composer" , "producer" } )
fix(smartplaylist): support isMissing/isPresent operators on ReplayGain fields (#5585)
* fix(smartplaylist): support isMissing/isPresent operators on ReplayGain fields
ReplayGain values are stored in dedicated nullable columns (rg_album_gain,
rg_album_peak, rg_track_gain, rg_track_peak) rather than in the media_file.tags
JSON blob. The isMissing/isPresent operators previously only supported tag and
role fields, causing two failure modes:
1. Using the documented alias names (replaygain_album_gain etc.) from PR #5256:
these got registered as JSON tags from mappings.yaml, so isMissing queried
json_tree(media_file.tags, '$.replaygain_album_gain') which is always empty
-> the playlist matched ALL songs.
2. Using the canonical field names (rgalbumgain etc.): not a tag/role, so SQL
generation returned an error. Because refreshSmartPlaylist deletes old tracks
before regenerating, the abort left the playlist empty.
Fix: add Nullable bool to FieldInfo and mark the four ReplayGain fields. Add
static alias entries (replaygain_album_gain -> rgalbumgain etc.) with
Numeric+Nullable set; because AddTagNames skips names already in the field map,
these static entries take precedence over the mappings.yaml tag registration.
missingExpr now emits IS NULL / IS NOT NULL for nullable column fields instead
of the json_tree lookup.
Fixes #5584
* chore(smartplaylist): address code review feedback
- Simplify the isMissing/isPresent unsupported-field error message,
removing the internal "nullable fields" jargon
- Standardize comments on mappings.yaml (the actual filename)
- Clarify the alias precedence comment in the LookupField test
2026-06-10 21:12:31 -04:00
criteria . AddTagNames ( [ ] string { "genre" , "mood" , "releasetype" , "recordingdate" , "replaygain_album_gain" } )
2026-04-24 23:18:20 -04:00
criteria . AddNumericTags ( [ ] string { "rate" } )
} )
DescribeTable ( "expressions" ,
func ( expr criteria . Expression , expectedSQL string , expectedArgs ... any ) {
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
sqlizer , err := newSmartPlaylistCriteria ( criteria . Criteria { Expression : expr } ) . where ( )
2026-04-24 23:18:20 -04:00
Expect ( err ) . ToNot ( HaveOccurred ( ) )
sql , args , err := sqlizer . ToSql ( )
Expect ( err ) . ToNot ( HaveOccurred ( ) )
Expect ( sql ) . To ( Equal ( expectedSQL ) )
Expect ( args ) . To ( HaveExactElements ( expectedArgs ... ) )
} ,
Entry ( "all group" ,
criteria . All { criteria . Contains { "title" : "love" } , criteria . Gt { "rating" : 3 } } ,
perf(smartplaylist): use annotation index for playcount/rating/loved filters (#5662)
* fix(smartplaylist): use annotation index for playcount/rating/loved filters
Annotation-field criteria wrapped the column in COALESCE(col, default) so
missing annotation rows behave as 0/false. COALESCE prevents SQLite from
using the column index, forcing a full media_file scan during smart playlist
materialization - multi-second loads on large libraries, independent of rule
complexity.
Store the raw column plus its default and drop COALESCE when the compared
value cannot match the default; fall back to 'col <op> ? OR col IS NULL' when
the default would match, so never-annotated tracks are still preserved.
Sorting keeps COALESCE to retain deterministic NULL ordering. Result set is
unchanged; the materialize query now seeks the annotation index.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for list-valued annotation comparisons
Hardening from final review: a list value (IN (...)) can't drive the index
and a default-inclusive list has per-element NULL semantics, so route slice
values through COALESCE(col, default) to stay exactly equivalent to the prior
form. Also make the bool-default branch explicit (loved only supports
equality operators) and share the COALESCE rendering via coalesceExpr.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): make coalesced() a field method
Thermo-nuclear review follow-up: promote the free coalesceExpr(f) to a
smartPlaylistField.coalesced() method that returns the bare expression when
there is no default. This lets sortExpr call field.coalesced() unconditionally
and drop its 'if coalesceDefault != nil' branch, removing the 'only annotation
fields get coalesced' special case from the sort path. Behavior unchanged.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for LIKE, bool-ordering, and tag ranges
Code review (xhigh) found the index-friendly rewrite did not cover every
operator, breaking result-set equivalence on a few reachable raw-JSON paths:
- LIKE family (contains/startsWith/endsWith/notContains) on annotation fields
used the bare column, so a NULL column never matched and missing-annotation
rows were dropped.
- Ordering comparators (gt/lt/...) on bool fields (loved) were decided as
equality, wrongly including never-annotated rows.
- InTheRange on a numeric tag split into two independent json_tree EXISTS,
letting different tag values satisfy each bound.
Centralize the decision in annotationCond via bareNullInclusion: emit the
index-friendly bare form only for scalar values under an exactly-orderable
comparator, otherwise fall back to the COALESCE form (always equivalent to the
original). Route LIKE through coalesced(); reject tag/role ranges. Replace the
local toFloat/toBool with spf13/cast (fixes unhandled numeric types and string
bool forms), and drop the redundant LookupField + double reflect.TypeOf.
A 17-case brute-force check confirms row-set equivalence to the prior
COALESCE form across all operators including the fixed LIKE/bool cases.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for list values on bool annotation fields
Second review found the bareNullInclusion bool branch missed the non-scalar
guard the numeric branch has: a list value on loved/albumloved/artistloved
(e.g. {"is":{"loved":[true]}}) coerced through toBool (which swallowed the
cast error) to false, emitting the bare/OR-IS-NULL form and wrongly including
never-annotated rows. Make toBool return (value, ok) like toFloat and bail to
COALESCE when the value isn't a scalar bool. Also add the missing test for the
tag/role range rejection. A 21-case brute-force confirms row-set equivalence to
the original COALESCE form across every operator, including the bool/numeric
list paths.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): drop spf13/cast for stdlib value coercion
The value coercion only sees the handful of types criteria produces (int,
float64, string from JSON; bool already normalized at unmarshal), so cast's
broad conversion isn't needed. Use small explicit type switches over strconv
instead, keeping the string fallback (ParseFloat/ParseBool) that closes the
'1'/'t' gap. No dependency change — cast returns to indirect.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): share bool coercion via criteria.ToBool
normalizeBoolValue (unmarshal-time) and the persistence bool guard both parsed
bool-ish values independently. Extract the shared logic into an exported
criteria.ToBool(any) (bool, ok): normalizeBoolValue delegates to it (behavior
unchanged), and the persistence layer reuses it via its existing model/criteria
import instead of a local helper. No behavior change.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): trim sqlLiteral and dedup rationale comments
/simplify cleanup: fmt %v already renders bool defaults as false/true, so drop
sqlLiteral's redundant bool branch. Consolidate the COALESCE-vs-index rationale
to the smartPlaylistField comment instead of repeating it across annotationCond
and the struct. No behavior change.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): address review feedback on multi-field maps and *any
From the PR bot reviews:
- sqlFields now uses the field's coalesced() form, so annotation fields in a
multi-field operator map (Is/Gt/Contains with >1 key) keep COALESCE and don't
silently drop never-annotated rows. Covers both the comparison and LIKE
fallback paths. (Gemini high, Copilot)
- Replace coalesceDefault *any with a plain any (0/false are non-nil
interfaces, so nil still means 'no default'); drop the coalesce() boxing
helper and the pointer indirection. (Gemini)
- Give rangeExpr clear, range-specific errors for the multi-field and malformed
-pair cases instead of an empty-field / 'in operator' message. (Copilot)
Adds tests for the multi-field COALESCE behavior and the new range errors.
Signed-off-by: Deluan <deluan@deluan.com>
* Revert multi-field COALESCE handling (YAGNI)
The multi-field operator map case the bots flagged is unreachable: marshalExpression
rejects any operator map with more than one field, so a multi-field map can never be
persisted or loaded. Revert the sqlFields change and its tests rather than harden a
code path no supported input can reach. Keep the two reachable improvements from the
review: coalesceDefault any (not *any), and the clearer malformed-range error.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(persistence): model comparator as a behavior-carrying struct
The smart-playlist comparator was a bare string alias, forcing two parallel
switches over the same six operators: squirrelCmp mapped each to its squirrel
constructor, and bareNullInclusion restated each as a float predicate. Adding
or changing an operator meant editing both in sync.
Make comparator a struct that bundles those facts per operator (the squirrel
builder, the operator as a float predicate, and whether it's an ordering op).
Both switches collapse: squirrelCmp is deleted in favor of cmp.build, and
bareNullInclusion's numeric switch becomes a single cmp.satisfy call. Generated
SQL is unchanged, as the existing table-driven tests confirm.
* docs(smartplaylist): trim comments that restate the code
Remove or tighten comments that describe what the code already says (likeCond and
comparisonExpr doc lines, redundant clauses in annotationField/coalesced/ToBool/
normalizeBoolValue). Keep the comments that explain non-obvious rationale: the
COALESCE-vs-index tradeoff, the bareNullInclusion/annotationCond contracts, and the
why-we-fall-back notes.
* docs(smartplaylist): collapse coalesceDefault comment to one line
The field's six-line block duplicated the COALESCE-vs-index rationale that already
lives on annotationCond. Reduce it to a one-line description plus a pointer there.
---------
Signed-off-by: Deluan <deluan@deluan.com>
2026-06-24 22:26:41 -04:00
"(media_file.title LIKE ? AND annotation.rating > ?)" , "%love%" , 3 ) ,
2026-04-24 23:18:20 -04:00
Entry ( "any group" ,
criteria . Any { criteria . Is { "title" : "Low Rider" } , criteria . Is { "album" : "Best Of" } } ,
"(media_file.title = ? OR media_file.album = ?)" , "Low Rider" , "Best Of" ) ,
Entry ( "is string" , criteria . Is { "title" : "Low Rider" } , "media_file.title = ?" , "Low Rider" ) ,
perf(smartplaylist): use annotation index for playcount/rating/loved filters (#5662)
* fix(smartplaylist): use annotation index for playcount/rating/loved filters
Annotation-field criteria wrapped the column in COALESCE(col, default) so
missing annotation rows behave as 0/false. COALESCE prevents SQLite from
using the column index, forcing a full media_file scan during smart playlist
materialization - multi-second loads on large libraries, independent of rule
complexity.
Store the raw column plus its default and drop COALESCE when the compared
value cannot match the default; fall back to 'col <op> ? OR col IS NULL' when
the default would match, so never-annotated tracks are still preserved.
Sorting keeps COALESCE to retain deterministic NULL ordering. Result set is
unchanged; the materialize query now seeks the annotation index.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for list-valued annotation comparisons
Hardening from final review: a list value (IN (...)) can't drive the index
and a default-inclusive list has per-element NULL semantics, so route slice
values through COALESCE(col, default) to stay exactly equivalent to the prior
form. Also make the bool-default branch explicit (loved only supports
equality operators) and share the COALESCE rendering via coalesceExpr.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): make coalesced() a field method
Thermo-nuclear review follow-up: promote the free coalesceExpr(f) to a
smartPlaylistField.coalesced() method that returns the bare expression when
there is no default. This lets sortExpr call field.coalesced() unconditionally
and drop its 'if coalesceDefault != nil' branch, removing the 'only annotation
fields get coalesced' special case from the sort path. Behavior unchanged.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for LIKE, bool-ordering, and tag ranges
Code review (xhigh) found the index-friendly rewrite did not cover every
operator, breaking result-set equivalence on a few reachable raw-JSON paths:
- LIKE family (contains/startsWith/endsWith/notContains) on annotation fields
used the bare column, so a NULL column never matched and missing-annotation
rows were dropped.
- Ordering comparators (gt/lt/...) on bool fields (loved) were decided as
equality, wrongly including never-annotated rows.
- InTheRange on a numeric tag split into two independent json_tree EXISTS,
letting different tag values satisfy each bound.
Centralize the decision in annotationCond via bareNullInclusion: emit the
index-friendly bare form only for scalar values under an exactly-orderable
comparator, otherwise fall back to the COALESCE form (always equivalent to the
original). Route LIKE through coalesced(); reject tag/role ranges. Replace the
local toFloat/toBool with spf13/cast (fixes unhandled numeric types and string
bool forms), and drop the redundant LookupField + double reflect.TypeOf.
A 17-case brute-force check confirms row-set equivalence to the prior
COALESCE form across all operators including the fixed LIKE/bool cases.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for list values on bool annotation fields
Second review found the bareNullInclusion bool branch missed the non-scalar
guard the numeric branch has: a list value on loved/albumloved/artistloved
(e.g. {"is":{"loved":[true]}}) coerced through toBool (which swallowed the
cast error) to false, emitting the bare/OR-IS-NULL form and wrongly including
never-annotated rows. Make toBool return (value, ok) like toFloat and bail to
COALESCE when the value isn't a scalar bool. Also add the missing test for the
tag/role range rejection. A 21-case brute-force confirms row-set equivalence to
the original COALESCE form across every operator, including the bool/numeric
list paths.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): drop spf13/cast for stdlib value coercion
The value coercion only sees the handful of types criteria produces (int,
float64, string from JSON; bool already normalized at unmarshal), so cast's
broad conversion isn't needed. Use small explicit type switches over strconv
instead, keeping the string fallback (ParseFloat/ParseBool) that closes the
'1'/'t' gap. No dependency change — cast returns to indirect.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): share bool coercion via criteria.ToBool
normalizeBoolValue (unmarshal-time) and the persistence bool guard both parsed
bool-ish values independently. Extract the shared logic into an exported
criteria.ToBool(any) (bool, ok): normalizeBoolValue delegates to it (behavior
unchanged), and the persistence layer reuses it via its existing model/criteria
import instead of a local helper. No behavior change.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): trim sqlLiteral and dedup rationale comments
/simplify cleanup: fmt %v already renders bool defaults as false/true, so drop
sqlLiteral's redundant bool branch. Consolidate the COALESCE-vs-index rationale
to the smartPlaylistField comment instead of repeating it across annotationCond
and the struct. No behavior change.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): address review feedback on multi-field maps and *any
From the PR bot reviews:
- sqlFields now uses the field's coalesced() form, so annotation fields in a
multi-field operator map (Is/Gt/Contains with >1 key) keep COALESCE and don't
silently drop never-annotated rows. Covers both the comparison and LIKE
fallback paths. (Gemini high, Copilot)
- Replace coalesceDefault *any with a plain any (0/false are non-nil
interfaces, so nil still means 'no default'); drop the coalesce() boxing
helper and the pointer indirection. (Gemini)
- Give rangeExpr clear, range-specific errors for the multi-field and malformed
-pair cases instead of an empty-field / 'in operator' message. (Copilot)
Adds tests for the multi-field COALESCE behavior and the new range errors.
Signed-off-by: Deluan <deluan@deluan.com>
* Revert multi-field COALESCE handling (YAGNI)
The multi-field operator map case the bots flagged is unreachable: marshalExpression
rejects any operator map with more than one field, so a multi-field map can never be
persisted or loaded. Revert the sqlFields change and its tests rather than harden a
code path no supported input can reach. Keep the two reachable improvements from the
review: coalesceDefault any (not *any), and the clearer malformed-range error.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(persistence): model comparator as a behavior-carrying struct
The smart-playlist comparator was a bare string alias, forcing two parallel
switches over the same six operators: squirrelCmp mapped each to its squirrel
constructor, and bareNullInclusion restated each as a float predicate. Adding
or changing an operator meant editing both in sync.
Make comparator a struct that bundles those facts per operator (the squirrel
builder, the operator as a float predicate, and whether it's an ordering op).
Both switches collapse: squirrelCmp is deleted in favor of cmp.build, and
bareNullInclusion's numeric switch becomes a single cmp.satisfy call. Generated
SQL is unchanged, as the existing table-driven tests confirm.
* docs(smartplaylist): trim comments that restate the code
Remove or tighten comments that describe what the code already says (likeCond and
comparisonExpr doc lines, redundant clauses in annotationField/coalesced/ToBool/
normalizeBoolValue). Keep the comments that explain non-obvious rationale: the
COALESCE-vs-index tradeoff, the bareNullInclusion/annotationCond contracts, and the
why-we-fall-back notes.
* docs(smartplaylist): collapse coalesceDefault comment to one line
The field's six-line block duplicated the COALESCE-vs-index rationale that already
lives on annotationCond. Reduce it to a one-line description plus a pointer there.
---------
Signed-off-by: Deluan <deluan@deluan.com>
2026-06-24 22:26:41 -04:00
Entry ( "is bool" , criteria . Is { "loved" : true } , "annotation.starred = ?" , true ) ,
2026-04-24 23:18:20 -04:00
Entry ( "is numeric list" , criteria . Is { "library_id" : [ ] int { 1 , 2 } } , "media_file.library_id IN (?,?)" , 1 , 2 ) ,
Entry ( "is not" , criteria . IsNot { "title" : "Low Rider" } , "media_file.title <> ?" , "Low Rider" ) ,
perf(smartplaylist): use annotation index for playcount/rating/loved filters (#5662)
* fix(smartplaylist): use annotation index for playcount/rating/loved filters
Annotation-field criteria wrapped the column in COALESCE(col, default) so
missing annotation rows behave as 0/false. COALESCE prevents SQLite from
using the column index, forcing a full media_file scan during smart playlist
materialization - multi-second loads on large libraries, independent of rule
complexity.
Store the raw column plus its default and drop COALESCE when the compared
value cannot match the default; fall back to 'col <op> ? OR col IS NULL' when
the default would match, so never-annotated tracks are still preserved.
Sorting keeps COALESCE to retain deterministic NULL ordering. Result set is
unchanged; the materialize query now seeks the annotation index.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for list-valued annotation comparisons
Hardening from final review: a list value (IN (...)) can't drive the index
and a default-inclusive list has per-element NULL semantics, so route slice
values through COALESCE(col, default) to stay exactly equivalent to the prior
form. Also make the bool-default branch explicit (loved only supports
equality operators) and share the COALESCE rendering via coalesceExpr.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): make coalesced() a field method
Thermo-nuclear review follow-up: promote the free coalesceExpr(f) to a
smartPlaylistField.coalesced() method that returns the bare expression when
there is no default. This lets sortExpr call field.coalesced() unconditionally
and drop its 'if coalesceDefault != nil' branch, removing the 'only annotation
fields get coalesced' special case from the sort path. Behavior unchanged.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for LIKE, bool-ordering, and tag ranges
Code review (xhigh) found the index-friendly rewrite did not cover every
operator, breaking result-set equivalence on a few reachable raw-JSON paths:
- LIKE family (contains/startsWith/endsWith/notContains) on annotation fields
used the bare column, so a NULL column never matched and missing-annotation
rows were dropped.
- Ordering comparators (gt/lt/...) on bool fields (loved) were decided as
equality, wrongly including never-annotated rows.
- InTheRange on a numeric tag split into two independent json_tree EXISTS,
letting different tag values satisfy each bound.
Centralize the decision in annotationCond via bareNullInclusion: emit the
index-friendly bare form only for scalar values under an exactly-orderable
comparator, otherwise fall back to the COALESCE form (always equivalent to the
original). Route LIKE through coalesced(); reject tag/role ranges. Replace the
local toFloat/toBool with spf13/cast (fixes unhandled numeric types and string
bool forms), and drop the redundant LookupField + double reflect.TypeOf.
A 17-case brute-force check confirms row-set equivalence to the prior
COALESCE form across all operators including the fixed LIKE/bool cases.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for list values on bool annotation fields
Second review found the bareNullInclusion bool branch missed the non-scalar
guard the numeric branch has: a list value on loved/albumloved/artistloved
(e.g. {"is":{"loved":[true]}}) coerced through toBool (which swallowed the
cast error) to false, emitting the bare/OR-IS-NULL form and wrongly including
never-annotated rows. Make toBool return (value, ok) like toFloat and bail to
COALESCE when the value isn't a scalar bool. Also add the missing test for the
tag/role range rejection. A 21-case brute-force confirms row-set equivalence to
the original COALESCE form across every operator, including the bool/numeric
list paths.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): drop spf13/cast for stdlib value coercion
The value coercion only sees the handful of types criteria produces (int,
float64, string from JSON; bool already normalized at unmarshal), so cast's
broad conversion isn't needed. Use small explicit type switches over strconv
instead, keeping the string fallback (ParseFloat/ParseBool) that closes the
'1'/'t' gap. No dependency change — cast returns to indirect.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): share bool coercion via criteria.ToBool
normalizeBoolValue (unmarshal-time) and the persistence bool guard both parsed
bool-ish values independently. Extract the shared logic into an exported
criteria.ToBool(any) (bool, ok): normalizeBoolValue delegates to it (behavior
unchanged), and the persistence layer reuses it via its existing model/criteria
import instead of a local helper. No behavior change.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): trim sqlLiteral and dedup rationale comments
/simplify cleanup: fmt %v already renders bool defaults as false/true, so drop
sqlLiteral's redundant bool branch. Consolidate the COALESCE-vs-index rationale
to the smartPlaylistField comment instead of repeating it across annotationCond
and the struct. No behavior change.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): address review feedback on multi-field maps and *any
From the PR bot reviews:
- sqlFields now uses the field's coalesced() form, so annotation fields in a
multi-field operator map (Is/Gt/Contains with >1 key) keep COALESCE and don't
silently drop never-annotated rows. Covers both the comparison and LIKE
fallback paths. (Gemini high, Copilot)
- Replace coalesceDefault *any with a plain any (0/false are non-nil
interfaces, so nil still means 'no default'); drop the coalesce() boxing
helper and the pointer indirection. (Gemini)
- Give rangeExpr clear, range-specific errors for the multi-field and malformed
-pair cases instead of an empty-field / 'in operator' message. (Copilot)
Adds tests for the multi-field COALESCE behavior and the new range errors.
Signed-off-by: Deluan <deluan@deluan.com>
* Revert multi-field COALESCE handling (YAGNI)
The multi-field operator map case the bots flagged is unreachable: marshalExpression
rejects any operator map with more than one field, so a multi-field map can never be
persisted or loaded. Revert the sqlFields change and its tests rather than harden a
code path no supported input can reach. Keep the two reachable improvements from the
review: coalesceDefault any (not *any), and the clearer malformed-range error.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(persistence): model comparator as a behavior-carrying struct
The smart-playlist comparator was a bare string alias, forcing two parallel
switches over the same six operators: squirrelCmp mapped each to its squirrel
constructor, and bareNullInclusion restated each as a float predicate. Adding
or changing an operator meant editing both in sync.
Make comparator a struct that bundles those facts per operator (the squirrel
builder, the operator as a float predicate, and whether it's an ordering op).
Both switches collapse: squirrelCmp is deleted in favor of cmp.build, and
bareNullInclusion's numeric switch becomes a single cmp.satisfy call. Generated
SQL is unchanged, as the existing table-driven tests confirm.
* docs(smartplaylist): trim comments that restate the code
Remove or tighten comments that describe what the code already says (likeCond and
comparisonExpr doc lines, redundant clauses in annotationField/coalesced/ToBool/
normalizeBoolValue). Keep the comments that explain non-obvious rationale: the
COALESCE-vs-index tradeoff, the bareNullInclusion/annotationCond contracts, and the
why-we-fall-back notes.
* docs(smartplaylist): collapse coalesceDefault comment to one line
The field's six-line block duplicated the COALESCE-vs-index rationale that already
lives on annotationCond. Reduce it to a one-line description plus a pointer there.
---------
Signed-off-by: Deluan <deluan@deluan.com>
2026-06-24 22:26:41 -04:00
Entry ( "gt" , criteria . Gt { "playCount" : 10 } , "annotation.play_count > ?" , 10 ) ,
Entry ( "lt" , criteria . Lt { "playCount" : 10 } , "(annotation.play_count < ? OR annotation.play_count IS NULL)" , 10 ) ,
2026-04-24 23:18:20 -04:00
Entry ( "contains" , criteria . Contains { "title" : "Low Rider" } , "media_file.title LIKE ?" , "%Low Rider%" ) ,
Entry ( "not contains" , criteria . NotContains { "title" : "Low Rider" } , "media_file.title NOT LIKE ?" , "%Low Rider%" ) ,
Entry ( "starts with" , criteria . StartsWith { "title" : "Low Rider" } , "media_file.title LIKE ?" , "Low Rider%" ) ,
Entry ( "ends with" , criteria . EndsWith { "title" : "Low Rider" } , "media_file.title LIKE ?" , "%Low Rider" ) ,
Entry ( "in range" , criteria . InTheRange { "year" : [ ] int { 1980 , 1990 } } , "(media_file.year >= ? AND media_file.year <= ?)" , 1980 , 1990 ) ,
Entry ( "before" , criteria . Before { "lastPlayed" : time . Date ( 2021 , 10 , 1 , 0 , 0 , 0 , 0 , time . Local ) } , "annotation.play_date < ?" , time . Date ( 2021 , 10 , 1 , 0 , 0 , 0 , 0 , time . Local ) ) ,
Entry ( "after" , criteria . After { "lastPlayed" : time . Date ( 2021 , 10 , 1 , 0 , 0 , 0 , 0 , time . Local ) } , "annotation.play_date > ?" , time . Date ( 2021 , 10 , 1 , 0 , 0 , 0 , 0 , time . Local ) ) ,
feat(smartplaylists): add support for referencing playlists using paths (#5187)
* feat: Add support for referencing playlists using paths
Signed-off-by: David <dvedvick@gmail.com>
* feat: Support relative playlist paths in smartlists
Signed-off-by: David <dvedvick@gmail.com>
* fix(smartplaylists): protect against nil panic
Signed-off-by: David <dvedvick@gmail.com>
* fix(smartplaylists): refreshing child playlists
Signed-off-by: David <dvedvick@gmail.com>
* chore(smartplaylists): log field parsing error
Signed-off-by: David <dvedvick@gmail.com>
* fix(smartplaylists): handle empty playlist paths
Signed-off-by: David <dvedvick@gmail.com>
* refactor(smartplaylists): make NormalizeChildPaths non-mutating
Signed-off-by: David <dvedvick@gmail.com>
* fix(smartplaylists): stop warning on every inPlaylist rule without the looked-up field
Rules that reference a playlist by id have no path field, and the reverse, so
the warning fired on every refresh. The log call also had a bad argument count.
* fix(smartplaylists): ignore empty inPlaylist id and path references
An empty path matched every playlist without a file path, including the
referencing playlist itself, so the refresh recursed until the stack overflowed.
An empty id also shadowed a valid path in the same rule.
* fix(smartplaylists): match inPlaylist paths in both NFC and NFD forms
A playlist path is stored in the Unicode form the filesystem reports, which can
differ from the form typed in the .nsp file. The exact comparison then found no
playlist for names with accents.
* fix(smartplaylists): keep all criteria fields when normalizing child paths
The field-by-field copy dropped RefreshDelay.
* fix(smartplaylists): clean absolute inPlaylist path references
Only relative references were cleaned, so an absolute reference such as
/music/./child.nsp never matched the stored /music/child.nsp.
* fix(smartplaylists): stop infinite recursion on playlists that reference each other
Two smart playlists referencing each other, by id or by path, recursed until the
stack overflowed and the server died. The refresh now tracks visited playlists.
* fix(smartplaylists): resolve inPlaylist path references with OS-native separators
Playlist.Path is OS-native, but references in a .nsp file use forward slashes.
On Windows they never matched, and a leading slash was not seen as absolute.
The specs now build OS-native paths, so they also run on Windows.
* fix(smartplaylists): warn when a relative inPlaylist path cannot be resolved
A playlist created in the UI has no file path, so a relative reference silently
matched nothing.
* refactor(smartplaylists): simplify child playlist reference handling
Share one extractor for child ids and paths, return only the normalized rules
instead of a playlist copy, and resolve each path reference in a single switch.
* test(smartplaylists): store the Unicode child path in OS-native form
Playlist.Path is OS-native, so on Windows the forward-slash fixture never matched
the normalized reference.
---------
Signed-off-by: David <dvedvick@gmail.com>
Co-authored-by: Deluan Quintão <deluan@navidrome.org>
2026-09-19 20:05:58 -05:00
Entry ( "in playlist [path]" , criteria . InPlaylist { "path" : "lacuslacus.nsp" } , "media_file.id IN (SELECT media_file_id FROM playlist_tracks pl LEFT JOIN playlist on pl.playlist_id = playlist.id WHERE (playlist.path IN (?) AND playlist.public = ?))" , "lacuslacus.nsp" , 1 ) ,
Entry ( "in playlist [id]" , criteria . InPlaylist { "id" : "deadbeef-dead-beef" } , "media_file.id IN (SELECT media_file_id FROM playlist_tracks pl LEFT JOIN playlist on pl.playlist_id = playlist.id WHERE (pl.playlist_id = ? AND playlist.public = ?))" , "deadbeef-dead-beef" , 1 ) ,
Entry ( "in playlist [empty id falls back to path]" , criteria . InPlaylist { "id" : "" , "path" : "/music/x.nsp" } , "media_file.id IN (SELECT media_file_id FROM playlist_tracks pl LEFT JOIN playlist on pl.playlist_id = playlist.id WHERE (playlist.path IN (?) AND playlist.public = ?))" , "/music/x.nsp" , 1 ) ,
Entry ( "in playlist [decomposed unicode path]" , criteria . InPlaylist { "path" : "/m\u00fasica/x.nsp" } , "media_file.id IN (SELECT media_file_id FROM playlist_tracks pl LEFT JOIN playlist on pl.playlist_id = playlist.id WHERE (playlist.path IN (?,?) AND playlist.public = ?))" , "/m\u00fasica/x.nsp" , "/mu\u0301sica/x.nsp" , 1 ) ,
2026-04-24 23:18:20 -04:00
Entry ( "not in playlist" , criteria . NotInPlaylist { "id" : "deadbeef-dead-beef" } , "media_file.id NOT IN (SELECT media_file_id FROM playlist_tracks pl LEFT JOIN playlist on pl.playlist_id = playlist.id WHERE (pl.playlist_id = ? AND playlist.public = ?))" , "deadbeef-dead-beef" , 1 ) ,
perf(smartplaylist): use annotation index for playcount/rating/loved filters (#5662)
* fix(smartplaylist): use annotation index for playcount/rating/loved filters
Annotation-field criteria wrapped the column in COALESCE(col, default) so
missing annotation rows behave as 0/false. COALESCE prevents SQLite from
using the column index, forcing a full media_file scan during smart playlist
materialization - multi-second loads on large libraries, independent of rule
complexity.
Store the raw column plus its default and drop COALESCE when the compared
value cannot match the default; fall back to 'col <op> ? OR col IS NULL' when
the default would match, so never-annotated tracks are still preserved.
Sorting keeps COALESCE to retain deterministic NULL ordering. Result set is
unchanged; the materialize query now seeks the annotation index.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for list-valued annotation comparisons
Hardening from final review: a list value (IN (...)) can't drive the index
and a default-inclusive list has per-element NULL semantics, so route slice
values through COALESCE(col, default) to stay exactly equivalent to the prior
form. Also make the bool-default branch explicit (loved only supports
equality operators) and share the COALESCE rendering via coalesceExpr.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): make coalesced() a field method
Thermo-nuclear review follow-up: promote the free coalesceExpr(f) to a
smartPlaylistField.coalesced() method that returns the bare expression when
there is no default. This lets sortExpr call field.coalesced() unconditionally
and drop its 'if coalesceDefault != nil' branch, removing the 'only annotation
fields get coalesced' special case from the sort path. Behavior unchanged.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for LIKE, bool-ordering, and tag ranges
Code review (xhigh) found the index-friendly rewrite did not cover every
operator, breaking result-set equivalence on a few reachable raw-JSON paths:
- LIKE family (contains/startsWith/endsWith/notContains) on annotation fields
used the bare column, so a NULL column never matched and missing-annotation
rows were dropped.
- Ordering comparators (gt/lt/...) on bool fields (loved) were decided as
equality, wrongly including never-annotated rows.
- InTheRange on a numeric tag split into two independent json_tree EXISTS,
letting different tag values satisfy each bound.
Centralize the decision in annotationCond via bareNullInclusion: emit the
index-friendly bare form only for scalar values under an exactly-orderable
comparator, otherwise fall back to the COALESCE form (always equivalent to the
original). Route LIKE through coalesced(); reject tag/role ranges. Replace the
local toFloat/toBool with spf13/cast (fixes unhandled numeric types and string
bool forms), and drop the redundant LookupField + double reflect.TypeOf.
A 17-case brute-force check confirms row-set equivalence to the prior
COALESCE form across all operators including the fixed LIKE/bool cases.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for list values on bool annotation fields
Second review found the bareNullInclusion bool branch missed the non-scalar
guard the numeric branch has: a list value on loved/albumloved/artistloved
(e.g. {"is":{"loved":[true]}}) coerced through toBool (which swallowed the
cast error) to false, emitting the bare/OR-IS-NULL form and wrongly including
never-annotated rows. Make toBool return (value, ok) like toFloat and bail to
COALESCE when the value isn't a scalar bool. Also add the missing test for the
tag/role range rejection. A 21-case brute-force confirms row-set equivalence to
the original COALESCE form across every operator, including the bool/numeric
list paths.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): drop spf13/cast for stdlib value coercion
The value coercion only sees the handful of types criteria produces (int,
float64, string from JSON; bool already normalized at unmarshal), so cast's
broad conversion isn't needed. Use small explicit type switches over strconv
instead, keeping the string fallback (ParseFloat/ParseBool) that closes the
'1'/'t' gap. No dependency change — cast returns to indirect.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): share bool coercion via criteria.ToBool
normalizeBoolValue (unmarshal-time) and the persistence bool guard both parsed
bool-ish values independently. Extract the shared logic into an exported
criteria.ToBool(any) (bool, ok): normalizeBoolValue delegates to it (behavior
unchanged), and the persistence layer reuses it via its existing model/criteria
import instead of a local helper. No behavior change.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): trim sqlLiteral and dedup rationale comments
/simplify cleanup: fmt %v already renders bool defaults as false/true, so drop
sqlLiteral's redundant bool branch. Consolidate the COALESCE-vs-index rationale
to the smartPlaylistField comment instead of repeating it across annotationCond
and the struct. No behavior change.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): address review feedback on multi-field maps and *any
From the PR bot reviews:
- sqlFields now uses the field's coalesced() form, so annotation fields in a
multi-field operator map (Is/Gt/Contains with >1 key) keep COALESCE and don't
silently drop never-annotated rows. Covers both the comparison and LIKE
fallback paths. (Gemini high, Copilot)
- Replace coalesceDefault *any with a plain any (0/false are non-nil
interfaces, so nil still means 'no default'); drop the coalesce() boxing
helper and the pointer indirection. (Gemini)
- Give rangeExpr clear, range-specific errors for the multi-field and malformed
-pair cases instead of an empty-field / 'in operator' message. (Copilot)
Adds tests for the multi-field COALESCE behavior and the new range errors.
Signed-off-by: Deluan <deluan@deluan.com>
* Revert multi-field COALESCE handling (YAGNI)
The multi-field operator map case the bots flagged is unreachable: marshalExpression
rejects any operator map with more than one field, so a multi-field map can never be
persisted or loaded. Revert the sqlFields change and its tests rather than harden a
code path no supported input can reach. Keep the two reachable improvements from the
review: coalesceDefault any (not *any), and the clearer malformed-range error.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(persistence): model comparator as a behavior-carrying struct
The smart-playlist comparator was a bare string alias, forcing two parallel
switches over the same six operators: squirrelCmp mapped each to its squirrel
constructor, and bareNullInclusion restated each as a float predicate. Adding
or changing an operator meant editing both in sync.
Make comparator a struct that bundles those facts per operator (the squirrel
builder, the operator as a float predicate, and whether it's an ordering op).
Both switches collapse: squirrelCmp is deleted in favor of cmp.build, and
bareNullInclusion's numeric switch becomes a single cmp.satisfy call. Generated
SQL is unchanged, as the existing table-driven tests confirm.
* docs(smartplaylist): trim comments that restate the code
Remove or tighten comments that describe what the code already says (likeCond and
comparisonExpr doc lines, redundant clauses in annotationField/coalesced/ToBool/
normalizeBoolValue). Keep the comments that explain non-obvious rationale: the
COALESCE-vs-index tradeoff, the bareNullInclusion/annotationCond contracts, and the
why-we-fall-back notes.
* docs(smartplaylist): collapse coalesceDefault comment to one line
The field's six-line block duplicated the COALESCE-vs-index rationale that already
lives on annotationCond. Reduce it to a one-line description plus a pointer there.
---------
Signed-off-by: Deluan <deluan@deluan.com>
2026-06-24 22:26:41 -04:00
Entry ( "album annotation" , criteria . Gt { "albumRating" : 3 } , "album_annotation.rating > ?" , 3 ) ,
Entry ( "artist annotation" , criteria . Is { "artistLoved" : true } , "artist_annotation.starred = ?" , true ) ,
feat(smartplaylist): add album-level fields for sorting and filtering (#5899)
Smart playlists could only sort by the track-level `dateadded`, which scatters
an album's tracks because every track has its own timestamp. There was no way
to express "newest albums first, tracks in album order".
Adds five fields backed by the album table: albumdateadded, albumdatemodified,
albumduration, albumsongcount and albumsize, so `"sort": "-albumdateadded,
tracknumber"` now works. They filter as well as sort, which also enables rules
like "everything from albums added this month" or "skip singles and EPs".
These need the album table rather than the album_annotation table the existing
album* fields join, so they get their own bit in the join mask. The bitmask
already unions joins from both the expression and the sort fields, so a
sort-only reference pulls in the join for the main query while correctly
staying out of the percentage-limit count query.
No COALESCE default is used. That mechanism exists because an album_annotation
row is genuinely often absent; the album row always exists and song_count,
duration and size are NOT NULL, so leaving the columns bare keeps filters
index-friendly.
albumdatemodified follows the existing track-level `datemodified` naming rather
than the `albumdateupdated` spelling used in the request.
albumreleasedate was considered and rejected: album.release_date is
allOrNothing() over the tracks, so it equals the track-level releasedate when
they agree and collapses to empty when they disagree.
Closes https://github.com/navidrome/navidrome/discussions/5347
2026-08-06 10:59:50 -04:00
Entry ( "album column" , criteria . Gt { "albumSongCount" : 5 } , "album.song_count > ?" , 5 ) ,
Entry ( "album duration column" , criteria . Lt { "albumDuration" : 600 } , "album.duration < ?" , 600 ) ,
Entry ( "album size column" , criteria . Gt { "albumSize" : 1000 } , "album.size > ?" , 1000 ) ,
Entry ( "album date column" , criteria . After { "albumDateAdded" : time . Date ( 2021 , 10 , 1 , 0 , 0 , 0 , 0 , time . Local ) } , "album.created_at > ?" , time . Date ( 2021 , 10 , 1 , 0 , 0 , 0 , 0 , time . Local ) ) ,
Entry ( "album modified column" , criteria . Before { "albumDateModified" : time . Date ( 2021 , 10 , 1 , 0 , 0 , 0 , 0 , time . Local ) } , "album.updated_at < ?" , time . Date ( 2021 , 10 , 1 , 0 , 0 , 0 , 0 , time . Local ) ) ,
perf(smartplaylist): use annotation index for playcount/rating/loved filters (#5662)
* fix(smartplaylist): use annotation index for playcount/rating/loved filters
Annotation-field criteria wrapped the column in COALESCE(col, default) so
missing annotation rows behave as 0/false. COALESCE prevents SQLite from
using the column index, forcing a full media_file scan during smart playlist
materialization - multi-second loads on large libraries, independent of rule
complexity.
Store the raw column plus its default and drop COALESCE when the compared
value cannot match the default; fall back to 'col <op> ? OR col IS NULL' when
the default would match, so never-annotated tracks are still preserved.
Sorting keeps COALESCE to retain deterministic NULL ordering. Result set is
unchanged; the materialize query now seeks the annotation index.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for list-valued annotation comparisons
Hardening from final review: a list value (IN (...)) can't drive the index
and a default-inclusive list has per-element NULL semantics, so route slice
values through COALESCE(col, default) to stay exactly equivalent to the prior
form. Also make the bool-default branch explicit (loved only supports
equality operators) and share the COALESCE rendering via coalesceExpr.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): make coalesced() a field method
Thermo-nuclear review follow-up: promote the free coalesceExpr(f) to a
smartPlaylistField.coalesced() method that returns the bare expression when
there is no default. This lets sortExpr call field.coalesced() unconditionally
and drop its 'if coalesceDefault != nil' branch, removing the 'only annotation
fields get coalesced' special case from the sort path. Behavior unchanged.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for LIKE, bool-ordering, and tag ranges
Code review (xhigh) found the index-friendly rewrite did not cover every
operator, breaking result-set equivalence on a few reachable raw-JSON paths:
- LIKE family (contains/startsWith/endsWith/notContains) on annotation fields
used the bare column, so a NULL column never matched and missing-annotation
rows were dropped.
- Ordering comparators (gt/lt/...) on bool fields (loved) were decided as
equality, wrongly including never-annotated rows.
- InTheRange on a numeric tag split into two independent json_tree EXISTS,
letting different tag values satisfy each bound.
Centralize the decision in annotationCond via bareNullInclusion: emit the
index-friendly bare form only for scalar values under an exactly-orderable
comparator, otherwise fall back to the COALESCE form (always equivalent to the
original). Route LIKE through coalesced(); reject tag/role ranges. Replace the
local toFloat/toBool with spf13/cast (fixes unhandled numeric types and string
bool forms), and drop the redundant LookupField + double reflect.TypeOf.
A 17-case brute-force check confirms row-set equivalence to the prior
COALESCE form across all operators including the fixed LIKE/bool cases.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for list values on bool annotation fields
Second review found the bareNullInclusion bool branch missed the non-scalar
guard the numeric branch has: a list value on loved/albumloved/artistloved
(e.g. {"is":{"loved":[true]}}) coerced through toBool (which swallowed the
cast error) to false, emitting the bare/OR-IS-NULL form and wrongly including
never-annotated rows. Make toBool return (value, ok) like toFloat and bail to
COALESCE when the value isn't a scalar bool. Also add the missing test for the
tag/role range rejection. A 21-case brute-force confirms row-set equivalence to
the original COALESCE form across every operator, including the bool/numeric
list paths.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): drop spf13/cast for stdlib value coercion
The value coercion only sees the handful of types criteria produces (int,
float64, string from JSON; bool already normalized at unmarshal), so cast's
broad conversion isn't needed. Use small explicit type switches over strconv
instead, keeping the string fallback (ParseFloat/ParseBool) that closes the
'1'/'t' gap. No dependency change — cast returns to indirect.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): share bool coercion via criteria.ToBool
normalizeBoolValue (unmarshal-time) and the persistence bool guard both parsed
bool-ish values independently. Extract the shared logic into an exported
criteria.ToBool(any) (bool, ok): normalizeBoolValue delegates to it (behavior
unchanged), and the persistence layer reuses it via its existing model/criteria
import instead of a local helper. No behavior change.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): trim sqlLiteral and dedup rationale comments
/simplify cleanup: fmt %v already renders bool defaults as false/true, so drop
sqlLiteral's redundant bool branch. Consolidate the COALESCE-vs-index rationale
to the smartPlaylistField comment instead of repeating it across annotationCond
and the struct. No behavior change.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): address review feedback on multi-field maps and *any
From the PR bot reviews:
- sqlFields now uses the field's coalesced() form, so annotation fields in a
multi-field operator map (Is/Gt/Contains with >1 key) keep COALESCE and don't
silently drop never-annotated rows. Covers both the comparison and LIKE
fallback paths. (Gemini high, Copilot)
- Replace coalesceDefault *any with a plain any (0/false are non-nil
interfaces, so nil still means 'no default'); drop the coalesce() boxing
helper and the pointer indirection. (Gemini)
- Give rangeExpr clear, range-specific errors for the multi-field and malformed
-pair cases instead of an empty-field / 'in operator' message. (Copilot)
Adds tests for the multi-field COALESCE behavior and the new range errors.
Signed-off-by: Deluan <deluan@deluan.com>
* Revert multi-field COALESCE handling (YAGNI)
The multi-field operator map case the bots flagged is unreachable: marshalExpression
rejects any operator map with more than one field, so a multi-field map can never be
persisted or loaded. Revert the sqlFields change and its tests rather than harden a
code path no supported input can reach. Keep the two reachable improvements from the
review: coalesceDefault any (not *any), and the clearer malformed-range error.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(persistence): model comparator as a behavior-carrying struct
The smart-playlist comparator was a bare string alias, forcing two parallel
switches over the same six operators: squirrelCmp mapped each to its squirrel
constructor, and bareNullInclusion restated each as a float predicate. Adding
or changing an operator meant editing both in sync.
Make comparator a struct that bundles those facts per operator (the squirrel
builder, the operator as a float predicate, and whether it's an ordering op).
Both switches collapse: squirrelCmp is deleted in favor of cmp.build, and
bareNullInclusion's numeric switch becomes a single cmp.satisfy call. Generated
SQL is unchanged, as the existing table-driven tests confirm.
* docs(smartplaylist): trim comments that restate the code
Remove or tighten comments that describe what the code already says (likeCond and
comparisonExpr doc lines, redundant clauses in annotationField/coalesced/ToBool/
normalizeBoolValue). Keep the comments that explain non-obvious rationale: the
COALESCE-vs-index tradeoff, the bareNullInclusion/annotationCond contracts, and the
why-we-fall-back notes.
* docs(smartplaylist): collapse coalesceDefault comment to one line
The field's six-line block duplicated the COALESCE-vs-index rationale that already
lives on annotationCond. Reduce it to a one-line description plus a pointer there.
---------
Signed-off-by: Deluan <deluan@deluan.com>
2026-06-24 22:26:41 -04:00
// Annotation fields use a COALESCE default (0 for numeric, false for bool) so that tracks
// with no annotation row behave as that default. To keep the annotation index usable, the
// COALESCE is dropped when the compared value cannot match the default (the missing-row
// case is then naturally excluded); otherwise an explicit `OR col IS NULL` preserves it.
Entry ( "is safe (value != default)" , criteria . Is { "playCount" : 3 } , "annotation.play_count = ?" , 3 ) ,
Entry ( "is unsafe (value == default)" , criteria . Is { "playCount" : 0 } ,
"(annotation.play_count = ? OR annotation.play_count IS NULL)" , 0 ) ,
Entry ( "is bool false (value == default)" , criteria . Is { "loved" : false } ,
"(annotation.starred = ? OR annotation.starred IS NULL)" , false ) ,
Entry ( "gt safe (value >= default)" , criteria . Gt { "playCount" : 0 } , "annotation.play_count > ?" , 0 ) ,
Entry ( "gt unsafe (value < default)" , criteria . Gt { "playCount" : - 1 } ,
"(annotation.play_count > ? OR annotation.play_count IS NULL)" , - 1 ) ,
Entry ( "lt safe (value <= default)" , criteria . Lt { "playCount" : 0 } , "annotation.play_count < ?" , 0 ) ,
Entry ( "lt unsafe (value > default)" , criteria . Lt { "playCount" : 5 } ,
"(annotation.play_count < ? OR annotation.play_count IS NULL)" , 5 ) ,
Entry ( "isNot annotation keeps null match" , criteria . IsNot { "playCount" : 3 } ,
"(annotation.play_count <> ? OR annotation.play_count IS NULL)" , 3 ) ,
Entry ( "isNot annotation value == default" , criteria . IsNot { "playCount" : 0 } ,
"annotation.play_count <> ?" , 0 ) ,
Entry ( "in range spanning default" , criteria . InTheRange { "playCount" : [ ] int { - 1 , 5 } } ,
"((annotation.play_count >= ? OR annotation.play_count IS NULL) AND (annotation.play_count <= ? OR annotation.play_count IS NULL))" , - 1 , 5 ) ,
Entry ( "in range above default" , criteria . InTheRange { "playCount" : [ ] int { 1 , 5 } } ,
"(annotation.play_count >= ? AND (annotation.play_count <= ? OR annotation.play_count IS NULL))" , 1 , 5 ) ,
// A list value can't drive the index and a default-inclusive list has per-element NULL
// semantics, so the COALESCE form is kept to stay equivalent to the original.
Entry ( "is list keeps coalesce" , criteria . Is { "playCount" : [ ] int { 0 , 3 } } ,
"COALESCE(annotation.play_count, 0) IN (?,?)" , 0 , 3 ) ,
// LIKE operators can't use the column index, so annotation fields keep the COALESCE form to
// match missing-annotation rows exactly as before (a NULL column never matches LIKE).
Entry ( "contains annotation keeps coalesce" , criteria . Contains { "playCount" : 0 } ,
"COALESCE(annotation.play_count, 0) LIKE ?" , "%0%" ) ,
Entry ( "starts with annotation keeps coalesce" , criteria . StartsWith { "rating" : 5 } ,
"COALESCE(annotation.rating, 0) LIKE ?" , "5%" ) ,
Entry ( "not contains annotation keeps coalesce" , criteria . NotContains { "playCount" : 0 } ,
"COALESCE(annotation.play_count, 0) NOT LIKE ?" , "%0%" ) ,
// Bool annotation fields only have a clean index-friendly form for equality; ordering
// comparators keep the COALESCE form so the missing-row default is honored exactly.
Entry ( "gt bool keeps coalesce" , criteria . Gt { "loved" : false } ,
"COALESCE(annotation.starred, false) > ?" , false ) ,
// A list value on a bool field is non-scalar, so it keeps the COALESCE form too (same as the
// numeric list case) — otherwise a NULL column would diverge from the original.
Entry ( "is bool list keeps coalesce" , criteria . Is { "loved" : [ ] any { true } } ,
"COALESCE(annotation.starred, false) IN (?)" , true ) ,
2026-04-24 23:18:20 -04:00
Entry ( "tag is" , criteria . Is { "genre" : "Rock" } , "exists (select 1 from json_tree(media_file.tags, '$.genre') where key='value' and value = ?)" , "Rock" ) ,
Entry ( "tag is not" , criteria . IsNot { "genre" : "Rock" } , "not exists (select 1 from json_tree(media_file.tags, '$.genre') where key='value' and value = ?)" , "Rock" ) ,
Entry ( "tag contains" , criteria . Contains { "genre" : "Rock" } , "exists (select 1 from json_tree(media_file.tags, '$.genre') where key='value' and value LIKE ?)" , "%Rock%" ) ,
Entry ( "tag not contains" , criteria . NotContains { "genre" : "Rock" } , "not exists (select 1 from json_tree(media_file.tags, '$.genre') where key='value' and value LIKE ?)" , "%Rock%" ) ,
Entry ( "numeric tag" , criteria . Lt { "rate" : 6 } , "exists (select 1 from json_tree(media_file.tags, '$.rate') where key='value' and CAST(value AS REAL) < ?)" , 6 ) ,
Entry ( "tag alias" , criteria . Is { "albumtype" : "album" } , "exists (select 1 from json_tree(media_file.tags, '$.releasetype') where key='value' and value = ?)" , "album" ) ,
refactor: centralize criteria sort parsing and extract smart playlist logic (#5415)
* test: add tests for recordingdate alias resolution in smart playlists
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: update FieldInfo structure and simplify fieldMap initialization
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: move sort parsing logic from persistence to criteria package
Extracted sort field parsing, validation, and direction handling from
persistence/criteria_sql.go into model/criteria/sort.go. The new
OrderByFields method on Criteria parses the Sort/Order strings into
validated SortField structs (field name + direction), resolving aliases
and handling +/- prefixes and order inversion. The persistence layer now
consumes these parsed fields and only handles SQL expression mapping.
This centralizes sort parsing to enforce consistent implementations.
* refactor: standardize field access in smartPlaylistCriteria structure
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: add ResolveLimit method to Criteria
Moved the percentage-limit resolution logic from playlist_repository
into Criteria.ResolveLimit, replacing the 3-line mutate-after-query
pattern with a single method call. The method preserves LimitPercent
rather than zeroing it, since IsPercentageLimit already returns false
once Limit is set, making the clear redundant and lossy.
* refactor: improve child playlist loading and error handling in refresh logic
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: extract smart playlist logic to dedicated files
Moved refreshSmartPlaylist, addSmartPlaylistAnnotationJoins, and
addCriteria methods from playlist_repository.go to a new
smart_playlist_repository.go file. Extracted all smart playlist tests
to smart_playlist_repository_test.go. Added DeferCleanup to the
"valid rules" test to fix ordering flakiness when Ginkgo randomizes
test execution across files.
* refactor: break refreshSmartPlaylist into smaller focused methods
Split the monolithic refreshSmartPlaylist method into discrete helpers
for readability: shouldRefreshSmartPlaylist for guard checks,
refreshChildPlaylists for recursive dependency refresh,
resolvePercentageLimit for count-based limit resolution,
buildSmartPlaylistQuery for assembling the SELECT with joins, and
addMediaFileAnnotationJoin to DRY up the repeated annotation join clause.
* refactor: deduplicate child playlist IDs in Criteria
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: simplify withSmartPlaylistOwner to accept model.User
Replaced separate ownerID string and ownerIsAdmin bool parameters with a
single model.User struct, reducing the field count in smartPlaylistCriteria
and making the option function signature clearer. Updated all call sites
and tests accordingly.
* fix: handle empty sort fields and propagate child playlist load errors
OrderByFields now falls back to [{title, asc}] when all user-supplied
sort fields are invalid, preventing empty ORDER BY clauses that would
produce invalid SQL in row_number() window functions. Also restored the
original behavior where a DB error loading child playlists aborts the
parent smart playlist refresh, by making refreshChildPlaylists return a
bool.
* refactor: log warning when no valid sort fields are found
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
2026-04-26 14:49:59 -04:00
Entry ( "field alias via tag registration" , criteria . Is { "recordingdate" : "2024-01-01" } , "media_file.date = ?" , "2024-01-01" ) ,
2026-05-22 18:00:13 -03:00
Entry ( "role is" , criteria . Is { "artist" : "u2" } , "exists (select 1 from media_file_artists mfa join artist on artist.id = mfa.artist_id where mfa.media_file_id = media_file.id and mfa.role = ? and artist.name = ?)" , "artist" , "u2" ) ,
Entry ( "role contains" , criteria . Contains { "composer" : "Lennon" } , "exists (select 1 from media_file_artists mfa join artist on artist.id = mfa.artist_id where mfa.media_file_id = media_file.id and mfa.role = ? and artist.name LIKE ?)" , "composer" , "%Lennon%" ) ,
Entry ( "role not contains" , criteria . NotContains { "artist" : "u2" } , "not exists (select 1 from media_file_artists mfa join artist on artist.id = mfa.artist_id where mfa.media_file_id = media_file.id and mfa.role = ? and artist.name LIKE ?)" , "artist" , "%u2%" ) ,
2026-04-28 19:46:12 -04:00
// ReplayGain fields
Entry ( "rgAlbumGain is" , criteria . Is { "rgAlbumGain" : 0 } , "media_file.rg_album_gain = ?" , 0 ) ,
Entry ( "rgAlbumGain gt" , criteria . Gt { "rgAlbumGain" : - 6.0 } , "media_file.rg_album_gain > ?" , - 6.0 ) ,
Entry ( "rgTrackPeak lt" , criteria . Lt { "rgTrackPeak" : 1.0 } , "media_file.rg_track_peak < ?" , 1.0 ) ,
2026-04-28 19:40:08 -04:00
// isMissing — tags
Entry ( "isMissing tag [true]" , criteria . IsMissing { "genre" : true } ,
"not exists (select 1 from json_tree(media_file.tags, '$.genre') where key='value')" ) ,
Entry ( "isMissing tag [false]" , criteria . IsMissing { "genre" : false } ,
"exists (select 1 from json_tree(media_file.tags, '$.genre') where key='value')" ) ,
// isMissing — roles
Entry ( "isMissing role [true]" , criteria . IsMissing { "artist" : true } ,
2026-05-22 18:00:13 -03:00
"not exists (select 1 from media_file_artists mfa where mfa.media_file_id = media_file.id and mfa.role = ?)" , "artist" ) ,
2026-04-28 19:40:08 -04:00
Entry ( "isMissing role [false]" , criteria . IsMissing { "artist" : false } ,
2026-05-22 18:00:13 -03:00
"exists (select 1 from media_file_artists mfa where mfa.media_file_id = media_file.id and mfa.role = ?)" , "artist" ) ,
2026-04-28 19:40:08 -04:00
// isPresent — tags
Entry ( "isPresent tag [true]" , criteria . IsPresent { "genre" : true } ,
"exists (select 1 from json_tree(media_file.tags, '$.genre') where key='value')" ) ,
Entry ( "isPresent tag [false]" , criteria . IsPresent { "genre" : false } ,
"not exists (select 1 from json_tree(media_file.tags, '$.genre') where key='value')" ) ,
// isPresent — roles
Entry ( "isPresent role [true]" , criteria . IsPresent { "composer" : true } ,
2026-05-22 18:00:13 -03:00
"exists (select 1 from media_file_artists mfa where mfa.media_file_id = media_file.id and mfa.role = ?)" , "composer" ) ,
2026-04-28 19:40:08 -04:00
Entry ( "isPresent role [false]" , criteria . IsPresent { "composer" : false } ,
2026-05-22 18:00:13 -03:00
"not exists (select 1 from media_file_artists mfa where mfa.media_file_id = media_file.id and mfa.role = ?)" , "composer" ) ,
fix(smartplaylist): support isMissing/isPresent operators on ReplayGain fields (#5585)
* fix(smartplaylist): support isMissing/isPresent operators on ReplayGain fields
ReplayGain values are stored in dedicated nullable columns (rg_album_gain,
rg_album_peak, rg_track_gain, rg_track_peak) rather than in the media_file.tags
JSON blob. The isMissing/isPresent operators previously only supported tag and
role fields, causing two failure modes:
1. Using the documented alias names (replaygain_album_gain etc.) from PR #5256:
these got registered as JSON tags from mappings.yaml, so isMissing queried
json_tree(media_file.tags, '$.replaygain_album_gain') which is always empty
-> the playlist matched ALL songs.
2. Using the canonical field names (rgalbumgain etc.): not a tag/role, so SQL
generation returned an error. Because refreshSmartPlaylist deletes old tracks
before regenerating, the abort left the playlist empty.
Fix: add Nullable bool to FieldInfo and mark the four ReplayGain fields. Add
static alias entries (replaygain_album_gain -> rgalbumgain etc.) with
Numeric+Nullable set; because AddTagNames skips names already in the field map,
these static entries take precedence over the mappings.yaml tag registration.
missingExpr now emits IS NULL / IS NOT NULL for nullable column fields instead
of the json_tree lookup.
Fixes #5584
* chore(smartplaylist): address code review feedback
- Simplify the isMissing/isPresent unsupported-field error message,
removing the internal "nullable fields" jargon
- Standardize comments on mappings.yaml (the actual filename)
- Clarify the alias precedence comment in the LookupField test
2026-06-10 21:12:31 -04:00
// isMissing/isPresent — nullable column fields (ReplayGain)
Entry ( "isMissing rgAlbumGain [true]" , criteria . IsMissing { "rgAlbumGain" : true } ,
feat(smartplaylist): extend isMissing/isPresent to bpm, bitDepth and many text fields (#5603)
* feat(smartplaylist): support isMissing/isPresent on mbz_* and lyrics fields
Mark the six mbz_* MusicBrainz ID columns and the lyrics column as Nullable
in the criteria field map, then extend missingExpr to handle string columns
where absence is encoded as NULL or empty string (plus '[]' for lyrics). The
Numeric/Boolean path (ReplayGain) is preserved via an explicit type check.
* refactor(model): make MediaFile BPM and BitDepth nullable pointers
Convert BPM and BitDepth fields in model.MediaFile from int to *int so that
'tag absent' is distinguishable from zero. The metadata mapper now uses
NullableFloat for BPM (nil when absent or zero/unparseable) and only sets
BitDepth when the audio property is non-zero (lossy codecs report 0).
All read sites use gg.V() for zero-fallback deref so Subsonic API output and
transcoding behaviour are byte-identical to before. The persistence layer
bridges the existing NOT NULL DB columns by coercing nil to 0 on write and 0
back to nil on read in PostMapArgs/PostScan; a later migration task will drop
those constraints.
Hash upgrade safety is verified by a new MediaFile.Hash describe block: nil
*int hashes identically to the old int(0) default via ZeroNil+IgnoreZeroValue,
so no files will be spuriously re-imported after this change.
Extra files touched beyond the plan's list: core/stream/legacy_client_test.go
(BitDepth in model.MediaFile literals), persistence/mediafile_repository.go
(NOT NULL bridge).
* test(model): pin pre-conversion golden hashes for BPM/BitDepth
* feat(smartplaylist): support isMissing/isPresent on bpm and bitDepth
* feat(db): make bpm and bit_depth columns nullable, backfill 0 to NULL
Drop the NOT NULL constraint on media_file.bpm and bit_depth via a
lossless migration that converts legacy 0-means-absent values to real
NULL. Remove the temporary shim in PostScan/PostMapArgs that was bridging
the old NOT NULL columns to the *int model fields. Add round-trip
persistence tests asserting NULL storage for nil pointers and correct
value round-trip for non-nil pointers.
* test(e2e): verify isMissing/isPresent partition for nullable fields
Add DescribeTable covering bpm, bitdepth, lyrics, and mbz_recording_id:
for each field, isMissing + isPresent song counts must equal the total
library count, proving the nullable-column SQL is exhaustive and correct.
* test(e2e): seed bpm tag so isMissing/isPresent partition is non-trivial
* fix(model): omit bitDepth from JSON when absent instead of emitting null
* feat(smartplaylist): support isMissing/isPresent on more string fields
Enable isMissing/isPresent operators for album, comment, catalognumber,
discsubtitle, albumcomment, sorttitle, sortalbum, sortartist,
sortalbumartist, and explicitstatus by marking them Nullable in fieldMap.
* refactor(smartplaylist): unify missingExpr column logic into one flow
Collapse the numeric/string fork in missingExpr into a single
empties-driven loop (numeric/boolean fields simply have no empties),
and replace the duplicated IsTag/IsRole guard with a three-way switch
that expresses the dispatch model once. No SQL semantics change for
string fields; numeric/boolean fields now emit a single-element Or/And
which squirrel parenthesizes (e.g. `(col IS NULL)` instead of bare
`col IS NULL`) — update the affected test expectations accordingly.
2026-06-13 13:15:20 -04:00
"(media_file.rg_album_gain IS NULL)" ) ,
fix(smartplaylist): support isMissing/isPresent operators on ReplayGain fields (#5585)
* fix(smartplaylist): support isMissing/isPresent operators on ReplayGain fields
ReplayGain values are stored in dedicated nullable columns (rg_album_gain,
rg_album_peak, rg_track_gain, rg_track_peak) rather than in the media_file.tags
JSON blob. The isMissing/isPresent operators previously only supported tag and
role fields, causing two failure modes:
1. Using the documented alias names (replaygain_album_gain etc.) from PR #5256:
these got registered as JSON tags from mappings.yaml, so isMissing queried
json_tree(media_file.tags, '$.replaygain_album_gain') which is always empty
-> the playlist matched ALL songs.
2. Using the canonical field names (rgalbumgain etc.): not a tag/role, so SQL
generation returned an error. Because refreshSmartPlaylist deletes old tracks
before regenerating, the abort left the playlist empty.
Fix: add Nullable bool to FieldInfo and mark the four ReplayGain fields. Add
static alias entries (replaygain_album_gain -> rgalbumgain etc.) with
Numeric+Nullable set; because AddTagNames skips names already in the field map,
these static entries take precedence over the mappings.yaml tag registration.
missingExpr now emits IS NULL / IS NOT NULL for nullable column fields instead
of the json_tree lookup.
Fixes #5584
* chore(smartplaylist): address code review feedback
- Simplify the isMissing/isPresent unsupported-field error message,
removing the internal "nullable fields" jargon
- Standardize comments on mappings.yaml (the actual filename)
- Clarify the alias precedence comment in the LookupField test
2026-06-10 21:12:31 -04:00
Entry ( "isMissing rgAlbumGain [false]" , criteria . IsMissing { "rgAlbumGain" : false } ,
feat(smartplaylist): extend isMissing/isPresent to bpm, bitDepth and many text fields (#5603)
* feat(smartplaylist): support isMissing/isPresent on mbz_* and lyrics fields
Mark the six mbz_* MusicBrainz ID columns and the lyrics column as Nullable
in the criteria field map, then extend missingExpr to handle string columns
where absence is encoded as NULL or empty string (plus '[]' for lyrics). The
Numeric/Boolean path (ReplayGain) is preserved via an explicit type check.
* refactor(model): make MediaFile BPM and BitDepth nullable pointers
Convert BPM and BitDepth fields in model.MediaFile from int to *int so that
'tag absent' is distinguishable from zero. The metadata mapper now uses
NullableFloat for BPM (nil when absent or zero/unparseable) and only sets
BitDepth when the audio property is non-zero (lossy codecs report 0).
All read sites use gg.V() for zero-fallback deref so Subsonic API output and
transcoding behaviour are byte-identical to before. The persistence layer
bridges the existing NOT NULL DB columns by coercing nil to 0 on write and 0
back to nil on read in PostMapArgs/PostScan; a later migration task will drop
those constraints.
Hash upgrade safety is verified by a new MediaFile.Hash describe block: nil
*int hashes identically to the old int(0) default via ZeroNil+IgnoreZeroValue,
so no files will be spuriously re-imported after this change.
Extra files touched beyond the plan's list: core/stream/legacy_client_test.go
(BitDepth in model.MediaFile literals), persistence/mediafile_repository.go
(NOT NULL bridge).
* test(model): pin pre-conversion golden hashes for BPM/BitDepth
* feat(smartplaylist): support isMissing/isPresent on bpm and bitDepth
* feat(db): make bpm and bit_depth columns nullable, backfill 0 to NULL
Drop the NOT NULL constraint on media_file.bpm and bit_depth via a
lossless migration that converts legacy 0-means-absent values to real
NULL. Remove the temporary shim in PostScan/PostMapArgs that was bridging
the old NOT NULL columns to the *int model fields. Add round-trip
persistence tests asserting NULL storage for nil pointers and correct
value round-trip for non-nil pointers.
* test(e2e): verify isMissing/isPresent partition for nullable fields
Add DescribeTable covering bpm, bitdepth, lyrics, and mbz_recording_id:
for each field, isMissing + isPresent song counts must equal the total
library count, proving the nullable-column SQL is exhaustive and correct.
* test(e2e): seed bpm tag so isMissing/isPresent partition is non-trivial
* fix(model): omit bitDepth from JSON when absent instead of emitting null
* feat(smartplaylist): support isMissing/isPresent on more string fields
Enable isMissing/isPresent operators for album, comment, catalognumber,
discsubtitle, albumcomment, sorttitle, sortalbum, sortartist,
sortalbumartist, and explicitstatus by marking them Nullable in fieldMap.
* refactor(smartplaylist): unify missingExpr column logic into one flow
Collapse the numeric/string fork in missingExpr into a single
empties-driven loop (numeric/boolean fields simply have no empties),
and replace the duplicated IsTag/IsRole guard with a three-way switch
that expresses the dispatch model once. No SQL semantics change for
string fields; numeric/boolean fields now emit a single-element Or/And
which squirrel parenthesizes (e.g. `(col IS NULL)` instead of bare
`col IS NULL`) — update the affected test expectations accordingly.
2026-06-13 13:15:20 -04:00
"(media_file.rg_album_gain IS NOT NULL)" ) ,
fix(smartplaylist): support isMissing/isPresent operators on ReplayGain fields (#5585)
* fix(smartplaylist): support isMissing/isPresent operators on ReplayGain fields
ReplayGain values are stored in dedicated nullable columns (rg_album_gain,
rg_album_peak, rg_track_gain, rg_track_peak) rather than in the media_file.tags
JSON blob. The isMissing/isPresent operators previously only supported tag and
role fields, causing two failure modes:
1. Using the documented alias names (replaygain_album_gain etc.) from PR #5256:
these got registered as JSON tags from mappings.yaml, so isMissing queried
json_tree(media_file.tags, '$.replaygain_album_gain') which is always empty
-> the playlist matched ALL songs.
2. Using the canonical field names (rgalbumgain etc.): not a tag/role, so SQL
generation returned an error. Because refreshSmartPlaylist deletes old tracks
before regenerating, the abort left the playlist empty.
Fix: add Nullable bool to FieldInfo and mark the four ReplayGain fields. Add
static alias entries (replaygain_album_gain -> rgalbumgain etc.) with
Numeric+Nullable set; because AddTagNames skips names already in the field map,
these static entries take precedence over the mappings.yaml tag registration.
missingExpr now emits IS NULL / IS NOT NULL for nullable column fields instead
of the json_tree lookup.
Fixes #5584
* chore(smartplaylist): address code review feedback
- Simplify the isMissing/isPresent unsupported-field error message,
removing the internal "nullable fields" jargon
- Standardize comments on mappings.yaml (the actual filename)
- Clarify the alias precedence comment in the LookupField test
2026-06-10 21:12:31 -04:00
Entry ( "isPresent rgTrackPeak [true]" , criteria . IsPresent { "rgTrackPeak" : true } ,
feat(smartplaylist): extend isMissing/isPresent to bpm, bitDepth and many text fields (#5603)
* feat(smartplaylist): support isMissing/isPresent on mbz_* and lyrics fields
Mark the six mbz_* MusicBrainz ID columns and the lyrics column as Nullable
in the criteria field map, then extend missingExpr to handle string columns
where absence is encoded as NULL or empty string (plus '[]' for lyrics). The
Numeric/Boolean path (ReplayGain) is preserved via an explicit type check.
* refactor(model): make MediaFile BPM and BitDepth nullable pointers
Convert BPM and BitDepth fields in model.MediaFile from int to *int so that
'tag absent' is distinguishable from zero. The metadata mapper now uses
NullableFloat for BPM (nil when absent or zero/unparseable) and only sets
BitDepth when the audio property is non-zero (lossy codecs report 0).
All read sites use gg.V() for zero-fallback deref so Subsonic API output and
transcoding behaviour are byte-identical to before. The persistence layer
bridges the existing NOT NULL DB columns by coercing nil to 0 on write and 0
back to nil on read in PostMapArgs/PostScan; a later migration task will drop
those constraints.
Hash upgrade safety is verified by a new MediaFile.Hash describe block: nil
*int hashes identically to the old int(0) default via ZeroNil+IgnoreZeroValue,
so no files will be spuriously re-imported after this change.
Extra files touched beyond the plan's list: core/stream/legacy_client_test.go
(BitDepth in model.MediaFile literals), persistence/mediafile_repository.go
(NOT NULL bridge).
* test(model): pin pre-conversion golden hashes for BPM/BitDepth
* feat(smartplaylist): support isMissing/isPresent on bpm and bitDepth
* feat(db): make bpm and bit_depth columns nullable, backfill 0 to NULL
Drop the NOT NULL constraint on media_file.bpm and bit_depth via a
lossless migration that converts legacy 0-means-absent values to real
NULL. Remove the temporary shim in PostScan/PostMapArgs that was bridging
the old NOT NULL columns to the *int model fields. Add round-trip
persistence tests asserting NULL storage for nil pointers and correct
value round-trip for non-nil pointers.
* test(e2e): verify isMissing/isPresent partition for nullable fields
Add DescribeTable covering bpm, bitdepth, lyrics, and mbz_recording_id:
for each field, isMissing + isPresent song counts must equal the total
library count, proving the nullable-column SQL is exhaustive and correct.
* test(e2e): seed bpm tag so isMissing/isPresent partition is non-trivial
* fix(model): omit bitDepth from JSON when absent instead of emitting null
* feat(smartplaylist): support isMissing/isPresent on more string fields
Enable isMissing/isPresent operators for album, comment, catalognumber,
discsubtitle, albumcomment, sorttitle, sortalbum, sortartist,
sortalbumartist, and explicitstatus by marking them Nullable in fieldMap.
* refactor(smartplaylist): unify missingExpr column logic into one flow
Collapse the numeric/string fork in missingExpr into a single
empties-driven loop (numeric/boolean fields simply have no empties),
and replace the duplicated IsTag/IsRole guard with a three-way switch
that expresses the dispatch model once. No SQL semantics change for
string fields; numeric/boolean fields now emit a single-element Or/And
which squirrel parenthesizes (e.g. `(col IS NULL)` instead of bare
`col IS NULL`) — update the affected test expectations accordingly.
2026-06-13 13:15:20 -04:00
"(media_file.rg_track_peak IS NOT NULL)" ) ,
fix(smartplaylist): support isMissing/isPresent operators on ReplayGain fields (#5585)
* fix(smartplaylist): support isMissing/isPresent operators on ReplayGain fields
ReplayGain values are stored in dedicated nullable columns (rg_album_gain,
rg_album_peak, rg_track_gain, rg_track_peak) rather than in the media_file.tags
JSON blob. The isMissing/isPresent operators previously only supported tag and
role fields, causing two failure modes:
1. Using the documented alias names (replaygain_album_gain etc.) from PR #5256:
these got registered as JSON tags from mappings.yaml, so isMissing queried
json_tree(media_file.tags, '$.replaygain_album_gain') which is always empty
-> the playlist matched ALL songs.
2. Using the canonical field names (rgalbumgain etc.): not a tag/role, so SQL
generation returned an error. Because refreshSmartPlaylist deletes old tracks
before regenerating, the abort left the playlist empty.
Fix: add Nullable bool to FieldInfo and mark the four ReplayGain fields. Add
static alias entries (replaygain_album_gain -> rgalbumgain etc.) with
Numeric+Nullable set; because AddTagNames skips names already in the field map,
these static entries take precedence over the mappings.yaml tag registration.
missingExpr now emits IS NULL / IS NOT NULL for nullable column fields instead
of the json_tree lookup.
Fixes #5584
* chore(smartplaylist): address code review feedback
- Simplify the isMissing/isPresent unsupported-field error message,
removing the internal "nullable fields" jargon
- Standardize comments on mappings.yaml (the actual filename)
- Clarify the alias precedence comment in the LookupField test
2026-06-10 21:12:31 -04:00
Entry ( "isPresent rgTrackPeak [false]" , criteria . IsPresent { "rgTrackPeak" : false } ,
feat(smartplaylist): extend isMissing/isPresent to bpm, bitDepth and many text fields (#5603)
* feat(smartplaylist): support isMissing/isPresent on mbz_* and lyrics fields
Mark the six mbz_* MusicBrainz ID columns and the lyrics column as Nullable
in the criteria field map, then extend missingExpr to handle string columns
where absence is encoded as NULL or empty string (plus '[]' for lyrics). The
Numeric/Boolean path (ReplayGain) is preserved via an explicit type check.
* refactor(model): make MediaFile BPM and BitDepth nullable pointers
Convert BPM and BitDepth fields in model.MediaFile from int to *int so that
'tag absent' is distinguishable from zero. The metadata mapper now uses
NullableFloat for BPM (nil when absent or zero/unparseable) and only sets
BitDepth when the audio property is non-zero (lossy codecs report 0).
All read sites use gg.V() for zero-fallback deref so Subsonic API output and
transcoding behaviour are byte-identical to before. The persistence layer
bridges the existing NOT NULL DB columns by coercing nil to 0 on write and 0
back to nil on read in PostMapArgs/PostScan; a later migration task will drop
those constraints.
Hash upgrade safety is verified by a new MediaFile.Hash describe block: nil
*int hashes identically to the old int(0) default via ZeroNil+IgnoreZeroValue,
so no files will be spuriously re-imported after this change.
Extra files touched beyond the plan's list: core/stream/legacy_client_test.go
(BitDepth in model.MediaFile literals), persistence/mediafile_repository.go
(NOT NULL bridge).
* test(model): pin pre-conversion golden hashes for BPM/BitDepth
* feat(smartplaylist): support isMissing/isPresent on bpm and bitDepth
* feat(db): make bpm and bit_depth columns nullable, backfill 0 to NULL
Drop the NOT NULL constraint on media_file.bpm and bit_depth via a
lossless migration that converts legacy 0-means-absent values to real
NULL. Remove the temporary shim in PostScan/PostMapArgs that was bridging
the old NOT NULL columns to the *int model fields. Add round-trip
persistence tests asserting NULL storage for nil pointers and correct
value round-trip for non-nil pointers.
* test(e2e): verify isMissing/isPresent partition for nullable fields
Add DescribeTable covering bpm, bitdepth, lyrics, and mbz_recording_id:
for each field, isMissing + isPresent song counts must equal the total
library count, proving the nullable-column SQL is exhaustive and correct.
* test(e2e): seed bpm tag so isMissing/isPresent partition is non-trivial
* fix(model): omit bitDepth from JSON when absent instead of emitting null
* feat(smartplaylist): support isMissing/isPresent on more string fields
Enable isMissing/isPresent operators for album, comment, catalognumber,
discsubtitle, albumcomment, sorttitle, sortalbum, sortartist,
sortalbumartist, and explicitstatus by marking them Nullable in fieldMap.
* refactor(smartplaylist): unify missingExpr column logic into one flow
Collapse the numeric/string fork in missingExpr into a single
empties-driven loop (numeric/boolean fields simply have no empties),
and replace the duplicated IsTag/IsRole guard with a three-way switch
that expresses the dispatch model once. No SQL semantics change for
string fields; numeric/boolean fields now emit a single-element Or/And
which squirrel parenthesizes (e.g. `(col IS NULL)` instead of bare
`col IS NULL`) — update the affected test expectations accordingly.
2026-06-13 13:15:20 -04:00
"(media_file.rg_track_peak IS NULL)" ) ,
fix(smartplaylist): support isMissing/isPresent operators on ReplayGain fields (#5585)
* fix(smartplaylist): support isMissing/isPresent operators on ReplayGain fields
ReplayGain values are stored in dedicated nullable columns (rg_album_gain,
rg_album_peak, rg_track_gain, rg_track_peak) rather than in the media_file.tags
JSON blob. The isMissing/isPresent operators previously only supported tag and
role fields, causing two failure modes:
1. Using the documented alias names (replaygain_album_gain etc.) from PR #5256:
these got registered as JSON tags from mappings.yaml, so isMissing queried
json_tree(media_file.tags, '$.replaygain_album_gain') which is always empty
-> the playlist matched ALL songs.
2. Using the canonical field names (rgalbumgain etc.): not a tag/role, so SQL
generation returned an error. Because refreshSmartPlaylist deletes old tracks
before regenerating, the abort left the playlist empty.
Fix: add Nullable bool to FieldInfo and mark the four ReplayGain fields. Add
static alias entries (replaygain_album_gain -> rgalbumgain etc.) with
Numeric+Nullable set; because AddTagNames skips names already in the field map,
these static entries take precedence over the mappings.yaml tag registration.
missingExpr now emits IS NULL / IS NOT NULL for nullable column fields instead
of the json_tree lookup.
Fixes #5584
* chore(smartplaylist): address code review feedback
- Simplify the isMissing/isPresent unsupported-field error message,
removing the internal "nullable fields" jargon
- Standardize comments on mappings.yaml (the actual filename)
- Clarify the alias precedence comment in the LookupField test
2026-06-10 21:12:31 -04:00
// isMissing — replaygain_* tag-name alias resolves to the nullable column (issue #5584)
Entry ( "isMissing replaygain_album_gain alias [true]" , criteria . IsMissing { "replaygain_album_gain" : true } ,
feat(smartplaylist): extend isMissing/isPresent to bpm, bitDepth and many text fields (#5603)
* feat(smartplaylist): support isMissing/isPresent on mbz_* and lyrics fields
Mark the six mbz_* MusicBrainz ID columns and the lyrics column as Nullable
in the criteria field map, then extend missingExpr to handle string columns
where absence is encoded as NULL or empty string (plus '[]' for lyrics). The
Numeric/Boolean path (ReplayGain) is preserved via an explicit type check.
* refactor(model): make MediaFile BPM and BitDepth nullable pointers
Convert BPM and BitDepth fields in model.MediaFile from int to *int so that
'tag absent' is distinguishable from zero. The metadata mapper now uses
NullableFloat for BPM (nil when absent or zero/unparseable) and only sets
BitDepth when the audio property is non-zero (lossy codecs report 0).
All read sites use gg.V() for zero-fallback deref so Subsonic API output and
transcoding behaviour are byte-identical to before. The persistence layer
bridges the existing NOT NULL DB columns by coercing nil to 0 on write and 0
back to nil on read in PostMapArgs/PostScan; a later migration task will drop
those constraints.
Hash upgrade safety is verified by a new MediaFile.Hash describe block: nil
*int hashes identically to the old int(0) default via ZeroNil+IgnoreZeroValue,
so no files will be spuriously re-imported after this change.
Extra files touched beyond the plan's list: core/stream/legacy_client_test.go
(BitDepth in model.MediaFile literals), persistence/mediafile_repository.go
(NOT NULL bridge).
* test(model): pin pre-conversion golden hashes for BPM/BitDepth
* feat(smartplaylist): support isMissing/isPresent on bpm and bitDepth
* feat(db): make bpm and bit_depth columns nullable, backfill 0 to NULL
Drop the NOT NULL constraint on media_file.bpm and bit_depth via a
lossless migration that converts legacy 0-means-absent values to real
NULL. Remove the temporary shim in PostScan/PostMapArgs that was bridging
the old NOT NULL columns to the *int model fields. Add round-trip
persistence tests asserting NULL storage for nil pointers and correct
value round-trip for non-nil pointers.
* test(e2e): verify isMissing/isPresent partition for nullable fields
Add DescribeTable covering bpm, bitdepth, lyrics, and mbz_recording_id:
for each field, isMissing + isPresent song counts must equal the total
library count, proving the nullable-column SQL is exhaustive and correct.
* test(e2e): seed bpm tag so isMissing/isPresent partition is non-trivial
* fix(model): omit bitDepth from JSON when absent instead of emitting null
* feat(smartplaylist): support isMissing/isPresent on more string fields
Enable isMissing/isPresent operators for album, comment, catalognumber,
discsubtitle, albumcomment, sorttitle, sortalbum, sortartist,
sortalbumartist, and explicitstatus by marking them Nullable in fieldMap.
* refactor(smartplaylist): unify missingExpr column logic into one flow
Collapse the numeric/string fork in missingExpr into a single
empties-driven loop (numeric/boolean fields simply have no empties),
and replace the duplicated IsTag/IsRole guard with a three-way switch
that expresses the dispatch model once. No SQL semantics change for
string fields; numeric/boolean fields now emit a single-element Or/And
which squirrel parenthesizes (e.g. `(col IS NULL)` instead of bare
`col IS NULL`) — update the affected test expectations accordingly.
2026-06-13 13:15:20 -04:00
"(media_file.rg_album_gain IS NULL)" ) ,
fix(smartplaylist): support isMissing/isPresent operators on ReplayGain fields (#5585)
* fix(smartplaylist): support isMissing/isPresent operators on ReplayGain fields
ReplayGain values are stored in dedicated nullable columns (rg_album_gain,
rg_album_peak, rg_track_gain, rg_track_peak) rather than in the media_file.tags
JSON blob. The isMissing/isPresent operators previously only supported tag and
role fields, causing two failure modes:
1. Using the documented alias names (replaygain_album_gain etc.) from PR #5256:
these got registered as JSON tags from mappings.yaml, so isMissing queried
json_tree(media_file.tags, '$.replaygain_album_gain') which is always empty
-> the playlist matched ALL songs.
2. Using the canonical field names (rgalbumgain etc.): not a tag/role, so SQL
generation returned an error. Because refreshSmartPlaylist deletes old tracks
before regenerating, the abort left the playlist empty.
Fix: add Nullable bool to FieldInfo and mark the four ReplayGain fields. Add
static alias entries (replaygain_album_gain -> rgalbumgain etc.) with
Numeric+Nullable set; because AddTagNames skips names already in the field map,
these static entries take precedence over the mappings.yaml tag registration.
missingExpr now emits IS NULL / IS NOT NULL for nullable column fields instead
of the json_tree lookup.
Fixes #5584
* chore(smartplaylist): address code review feedback
- Simplify the isMissing/isPresent unsupported-field error message,
removing the internal "nullable fields" jargon
- Standardize comments on mappings.yaml (the actual filename)
- Clarify the alias precedence comment in the LookupField test
2026-06-10 21:12:31 -04:00
Entry ( "isPresent replaygain_album_gain alias [true]" , criteria . IsPresent { "replaygain_album_gain" : true } ,
feat(smartplaylist): extend isMissing/isPresent to bpm, bitDepth and many text fields (#5603)
* feat(smartplaylist): support isMissing/isPresent on mbz_* and lyrics fields
Mark the six mbz_* MusicBrainz ID columns and the lyrics column as Nullable
in the criteria field map, then extend missingExpr to handle string columns
where absence is encoded as NULL or empty string (plus '[]' for lyrics). The
Numeric/Boolean path (ReplayGain) is preserved via an explicit type check.
* refactor(model): make MediaFile BPM and BitDepth nullable pointers
Convert BPM and BitDepth fields in model.MediaFile from int to *int so that
'tag absent' is distinguishable from zero. The metadata mapper now uses
NullableFloat for BPM (nil when absent or zero/unparseable) and only sets
BitDepth when the audio property is non-zero (lossy codecs report 0).
All read sites use gg.V() for zero-fallback deref so Subsonic API output and
transcoding behaviour are byte-identical to before. The persistence layer
bridges the existing NOT NULL DB columns by coercing nil to 0 on write and 0
back to nil on read in PostMapArgs/PostScan; a later migration task will drop
those constraints.
Hash upgrade safety is verified by a new MediaFile.Hash describe block: nil
*int hashes identically to the old int(0) default via ZeroNil+IgnoreZeroValue,
so no files will be spuriously re-imported after this change.
Extra files touched beyond the plan's list: core/stream/legacy_client_test.go
(BitDepth in model.MediaFile literals), persistence/mediafile_repository.go
(NOT NULL bridge).
* test(model): pin pre-conversion golden hashes for BPM/BitDepth
* feat(smartplaylist): support isMissing/isPresent on bpm and bitDepth
* feat(db): make bpm and bit_depth columns nullable, backfill 0 to NULL
Drop the NOT NULL constraint on media_file.bpm and bit_depth via a
lossless migration that converts legacy 0-means-absent values to real
NULL. Remove the temporary shim in PostScan/PostMapArgs that was bridging
the old NOT NULL columns to the *int model fields. Add round-trip
persistence tests asserting NULL storage for nil pointers and correct
value round-trip for non-nil pointers.
* test(e2e): verify isMissing/isPresent partition for nullable fields
Add DescribeTable covering bpm, bitdepth, lyrics, and mbz_recording_id:
for each field, isMissing + isPresent song counts must equal the total
library count, proving the nullable-column SQL is exhaustive and correct.
* test(e2e): seed bpm tag so isMissing/isPresent partition is non-trivial
* fix(model): omit bitDepth from JSON when absent instead of emitting null
* feat(smartplaylist): support isMissing/isPresent on more string fields
Enable isMissing/isPresent operators for album, comment, catalognumber,
discsubtitle, albumcomment, sorttitle, sortalbum, sortartist,
sortalbumartist, and explicitstatus by marking them Nullable in fieldMap.
* refactor(smartplaylist): unify missingExpr column logic into one flow
Collapse the numeric/string fork in missingExpr into a single
empties-driven loop (numeric/boolean fields simply have no empties),
and replace the duplicated IsTag/IsRole guard with a three-way switch
that expresses the dispatch model once. No SQL semantics change for
string fields; numeric/boolean fields now emit a single-element Or/And
which squirrel parenthesizes (e.g. `(col IS NULL)` instead of bare
`col IS NULL`) — update the affected test expectations accordingly.
2026-06-13 13:15:20 -04:00
"(media_file.rg_album_gain IS NOT NULL)" ) ,
// isMissing/isPresent — string column fields (empty string means missing)
Entry ( "isMissing mbz_recording_id [true]" , criteria . IsMissing { "mbz_recording_id" : true } ,
"(media_file.mbz_recording_id IS NULL OR media_file.mbz_recording_id = ?)" , "" ) ,
Entry ( "isMissing mbz_recording_id [false]" , criteria . IsMissing { "mbz_recording_id" : false } ,
"(media_file.mbz_recording_id IS NOT NULL AND media_file.mbz_recording_id <> ?)" , "" ) ,
Entry ( "isPresent mbz_album_id [true]" , criteria . IsPresent { "mbz_album_id" : true } ,
"(media_file.mbz_album_id IS NOT NULL AND media_file.mbz_album_id <> ?)" , "" ) ,
Entry ( "isPresent mbz_album_id [false]" , criteria . IsPresent { "mbz_album_id" : false } ,
"(media_file.mbz_album_id IS NULL OR media_file.mbz_album_id = ?)" , "" ) ,
// lyrics: absence is encoded as '' or '[]' (empty serialized LyricList)
Entry ( "isMissing lyrics [true]" , criteria . IsMissing { "lyrics" : true } ,
"(media_file.lyrics IS NULL OR media_file.lyrics = ? OR media_file.lyrics = ?)" , "" , "[]" ) ,
Entry ( "isPresent lyrics [true]" , criteria . IsPresent { "lyrics" : true } ,
"(media_file.lyrics IS NOT NULL AND media_file.lyrics <> ? AND media_file.lyrics <> ?)" , "" , "[]" ) ,
Entry ( "isMissing lyrics [false]" , criteria . IsMissing { "lyrics" : false } ,
"(media_file.lyrics IS NOT NULL AND media_file.lyrics <> ? AND media_file.lyrics <> ?)" , "" , "[]" ) ,
Entry ( "isPresent lyrics [false]" , criteria . IsPresent { "lyrics" : false } ,
"(media_file.lyrics IS NULL OR media_file.lyrics = ? OR media_file.lyrics = ?)" , "" , "[]" ) ,
// isMissing/isPresent — nullable numeric columns (BPM, BitDepth)
Entry ( "isMissing bpm [true]" , criteria . IsMissing { "bpm" : true } ,
"(media_file.bpm IS NULL)" ) ,
Entry ( "isPresent bpm [true]" , criteria . IsPresent { "bpm" : true } ,
"(media_file.bpm IS NOT NULL)" ) ,
Entry ( "isMissing bitdepth [true]" , criteria . IsMissing { "bitdepth" : true } ,
"(media_file.bit_depth IS NULL)" ) ,
Entry ( "isPresent bitdepth [false]" , criteria . IsPresent { "bitdepth" : false } ,
"(media_file.bit_depth IS NULL)" ) ,
// isMissing/isPresent — more string column fields (empty string means missing)
Entry ( "isMissing album [true]" , criteria . IsMissing { "album" : true } ,
"(media_file.album IS NULL OR media_file.album = ?)" , "" ) ,
Entry ( "isMissing comment [true]" , criteria . IsMissing { "comment" : true } ,
"(media_file.comment IS NULL OR media_file.comment = ?)" , "" ) ,
Entry ( "isMissing catalognumber [true]" , criteria . IsMissing { "catalognumber" : true } ,
"(media_file.catalog_num IS NULL OR media_file.catalog_num = ?)" , "" ) ,
Entry ( "isMissing discsubtitle [true]" , criteria . IsMissing { "discsubtitle" : true } ,
"(media_file.disc_subtitle IS NULL OR media_file.disc_subtitle = ?)" , "" ) ,
Entry ( "isMissing albumcomment [true]" , criteria . IsMissing { "albumcomment" : true } ,
"(media_file.mbz_album_comment IS NULL OR media_file.mbz_album_comment = ?)" , "" ) ,
Entry ( "isMissing sorttitle [true]" , criteria . IsMissing { "sorttitle" : true } ,
"(media_file.sort_title IS NULL OR media_file.sort_title = ?)" , "" ) ,
Entry ( "isMissing sortalbum [true]" , criteria . IsMissing { "sortalbum" : true } ,
"(media_file.sort_album_name IS NULL OR media_file.sort_album_name = ?)" , "" ) ,
Entry ( "isMissing sortartist [true]" , criteria . IsMissing { "sortartist" : true } ,
"(media_file.sort_artist_name IS NULL OR media_file.sort_artist_name = ?)" , "" ) ,
Entry ( "isMissing sortalbumartist [true]" , criteria . IsMissing { "sortalbumartist" : true } ,
"(media_file.sort_album_artist_name IS NULL OR media_file.sort_album_artist_name = ?)" , "" ) ,
Entry ( "isMissing explicitstatus [true]" , criteria . IsMissing { "explicitstatus" : true } ,
"(media_file.explicit_status IS NULL OR media_file.explicit_status = ?)" , "" ) ,
Entry ( "isPresent comment [true]" , criteria . IsPresent { "comment" : true } ,
"(media_file.comment IS NOT NULL AND media_file.comment <> ?)" , "" ) ,
2026-04-24 23:18:20 -04:00
)
2026-04-25 14:59:06 -04:00
Describe ( "playlist permissions" , func ( ) {
It ( "allows public or same-owner playlist references for regular users" , func ( ) {
sqlizer , err := newSmartPlaylistCriteria (
criteria . Criteria { Expression : criteria . InPlaylist { "id" : "deadbeef-dead-beef" } } ,
refactor: centralize criteria sort parsing and extract smart playlist logic (#5415)
* test: add tests for recordingdate alias resolution in smart playlists
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: update FieldInfo structure and simplify fieldMap initialization
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: move sort parsing logic from persistence to criteria package
Extracted sort field parsing, validation, and direction handling from
persistence/criteria_sql.go into model/criteria/sort.go. The new
OrderByFields method on Criteria parses the Sort/Order strings into
validated SortField structs (field name + direction), resolving aliases
and handling +/- prefixes and order inversion. The persistence layer now
consumes these parsed fields and only handles SQL expression mapping.
This centralizes sort parsing to enforce consistent implementations.
* refactor: standardize field access in smartPlaylistCriteria structure
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: add ResolveLimit method to Criteria
Moved the percentage-limit resolution logic from playlist_repository
into Criteria.ResolveLimit, replacing the 3-line mutate-after-query
pattern with a single method call. The method preserves LimitPercent
rather than zeroing it, since IsPercentageLimit already returns false
once Limit is set, making the clear redundant and lossy.
* refactor: improve child playlist loading and error handling in refresh logic
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: extract smart playlist logic to dedicated files
Moved refreshSmartPlaylist, addSmartPlaylistAnnotationJoins, and
addCriteria methods from playlist_repository.go to a new
smart_playlist_repository.go file. Extracted all smart playlist tests
to smart_playlist_repository_test.go. Added DeferCleanup to the
"valid rules" test to fix ordering flakiness when Ginkgo randomizes
test execution across files.
* refactor: break refreshSmartPlaylist into smaller focused methods
Split the monolithic refreshSmartPlaylist method into discrete helpers
for readability: shouldRefreshSmartPlaylist for guard checks,
refreshChildPlaylists for recursive dependency refresh,
resolvePercentageLimit for count-based limit resolution,
buildSmartPlaylistQuery for assembling the SELECT with joins, and
addMediaFileAnnotationJoin to DRY up the repeated annotation join clause.
* refactor: deduplicate child playlist IDs in Criteria
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: simplify withSmartPlaylistOwner to accept model.User
Replaced separate ownerID string and ownerIsAdmin bool parameters with a
single model.User struct, reducing the field count in smartPlaylistCriteria
and making the option function signature clearer. Updated all call sites
and tests accordingly.
* fix: handle empty sort fields and propagate child playlist load errors
OrderByFields now falls back to [{title, asc}] when all user-supplied
sort fields are invalid, preventing empty ORDER BY clauses that would
produce invalid SQL in row_number() window functions. Also restored the
original behavior where a DB error loading child playlists aborts the
parent smart playlist refresh, by making refreshChildPlaylists return a
bool.
* refactor: log warning when no valid sort fields are found
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
2026-04-26 14:49:59 -04:00
withSmartPlaylistOwner ( model . User { ID : "owner-id" , IsAdmin : false } ) ,
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
) . where ( )
2026-04-25 14:59:06 -04:00
Expect ( err ) . ToNot ( HaveOccurred ( ) )
sql , args , err := sqlizer . ToSql ( )
Expect ( err ) . ToNot ( HaveOccurred ( ) )
Expect ( sql ) . To ( Equal ( "media_file.id IN (SELECT media_file_id FROM playlist_tracks pl LEFT JOIN playlist on pl.playlist_id = playlist.id WHERE (pl.playlist_id = ? AND (playlist.public = ? OR playlist.owner_id = ?)))" ) )
Expect ( args ) . To ( HaveExactElements ( "deadbeef-dead-beef" , 1 , "owner-id" ) )
} )
It ( "allows all playlist references for admins" , func ( ) {
sqlizer , err := newSmartPlaylistCriteria (
criteria . Criteria { Expression : criteria . InPlaylist { "id" : "deadbeef-dead-beef" } } ,
refactor: centralize criteria sort parsing and extract smart playlist logic (#5415)
* test: add tests for recordingdate alias resolution in smart playlists
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: update FieldInfo structure and simplify fieldMap initialization
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: move sort parsing logic from persistence to criteria package
Extracted sort field parsing, validation, and direction handling from
persistence/criteria_sql.go into model/criteria/sort.go. The new
OrderByFields method on Criteria parses the Sort/Order strings into
validated SortField structs (field name + direction), resolving aliases
and handling +/- prefixes and order inversion. The persistence layer now
consumes these parsed fields and only handles SQL expression mapping.
This centralizes sort parsing to enforce consistent implementations.
* refactor: standardize field access in smartPlaylistCriteria structure
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: add ResolveLimit method to Criteria
Moved the percentage-limit resolution logic from playlist_repository
into Criteria.ResolveLimit, replacing the 3-line mutate-after-query
pattern with a single method call. The method preserves LimitPercent
rather than zeroing it, since IsPercentageLimit already returns false
once Limit is set, making the clear redundant and lossy.
* refactor: improve child playlist loading and error handling in refresh logic
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: extract smart playlist logic to dedicated files
Moved refreshSmartPlaylist, addSmartPlaylistAnnotationJoins, and
addCriteria methods from playlist_repository.go to a new
smart_playlist_repository.go file. Extracted all smart playlist tests
to smart_playlist_repository_test.go. Added DeferCleanup to the
"valid rules" test to fix ordering flakiness when Ginkgo randomizes
test execution across files.
* refactor: break refreshSmartPlaylist into smaller focused methods
Split the monolithic refreshSmartPlaylist method into discrete helpers
for readability: shouldRefreshSmartPlaylist for guard checks,
refreshChildPlaylists for recursive dependency refresh,
resolvePercentageLimit for count-based limit resolution,
buildSmartPlaylistQuery for assembling the SELECT with joins, and
addMediaFileAnnotationJoin to DRY up the repeated annotation join clause.
* refactor: deduplicate child playlist IDs in Criteria
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: simplify withSmartPlaylistOwner to accept model.User
Replaced separate ownerID string and ownerIsAdmin bool parameters with a
single model.User struct, reducing the field count in smartPlaylistCriteria
and making the option function signature clearer. Updated all call sites
and tests accordingly.
* fix: handle empty sort fields and propagate child playlist load errors
OrderByFields now falls back to [{title, asc}] when all user-supplied
sort fields are invalid, preventing empty ORDER BY clauses that would
produce invalid SQL in row_number() window functions. Also restored the
original behavior where a DB error loading child playlists aborts the
parent smart playlist refresh, by making refreshChildPlaylists return a
bool.
* refactor: log warning when no valid sort fields are found
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
2026-04-26 14:49:59 -04:00
withSmartPlaylistOwner ( model . User { ID : "admin-id" , IsAdmin : true } ) ,
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
) . where ( )
2026-04-25 14:59:06 -04:00
Expect ( err ) . ToNot ( HaveOccurred ( ) )
sql , args , err := sqlizer . ToSql ( )
Expect ( err ) . ToNot ( HaveOccurred ( ) )
Expect ( sql ) . To ( Equal ( "media_file.id IN (SELECT media_file_id FROM playlist_tracks pl LEFT JOIN playlist on pl.playlist_id = playlist.id WHERE (pl.playlist_id = ?))" ) )
Expect ( args ) . To ( HaveExactElements ( "deadbeef-dead-beef" ) )
} )
} )
2026-04-24 23:18:20 -04:00
It ( "builds relative date expressions" , func ( ) {
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
sqlizer , err := newSmartPlaylistCriteria ( criteria . Criteria { Expression : criteria . InTheLast { "lastPlayed" : 30 } } ) . where ( )
2026-04-24 23:18:20 -04:00
Expect ( err ) . ToNot ( HaveOccurred ( ) )
sql , args , err := sqlizer . ToSql ( )
Expect ( err ) . ToNot ( HaveOccurred ( ) )
Expect ( sql ) . To ( Equal ( "annotation.play_date > ?" ) )
Expect ( args ) . To ( HaveExactElements ( startOfPeriod ( 30 , time . Now ( ) ) ) )
} )
It ( "builds negated relative date expressions" , func ( ) {
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
sqlizer , err := newSmartPlaylistCriteria ( criteria . Criteria { Expression : criteria . NotInTheLast { "lastPlayed" : 30 } } ) . where ( )
2026-04-24 23:18:20 -04:00
Expect ( err ) . ToNot ( HaveOccurred ( ) )
sql , args , err := sqlizer . ToSql ( )
Expect ( err ) . ToNot ( HaveOccurred ( ) )
Expect ( sql ) . To ( Equal ( "(annotation.play_date < ? OR annotation.play_date IS NULL)" ) )
Expect ( args ) . To ( HaveExactElements ( startOfPeriod ( 30 , time . Now ( ) ) ) )
} )
It ( "returns an error for unknown fields" , func ( ) {
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
_ , err := newSmartPlaylistCriteria ( criteria . Criteria { Expression : criteria . EndsWith { "unknown" : "value" } } ) . where ( )
2026-04-24 23:18:20 -04:00
Expect ( err ) . To ( MatchError ( "invalid field in criteria: unknown" ) )
} )
2026-04-28 19:40:08 -04:00
It ( "returns an error when isMissing is used with a regular field" , func ( ) {
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
_ , err := newSmartPlaylistCriteria ( criteria . Criteria { Expression : criteria . IsMissing { "year" : true } } ) . where ( )
fix(smartplaylist): support isMissing/isPresent operators on ReplayGain fields (#5585)
* fix(smartplaylist): support isMissing/isPresent operators on ReplayGain fields
ReplayGain values are stored in dedicated nullable columns (rg_album_gain,
rg_album_peak, rg_track_gain, rg_track_peak) rather than in the media_file.tags
JSON blob. The isMissing/isPresent operators previously only supported tag and
role fields, causing two failure modes:
1. Using the documented alias names (replaygain_album_gain etc.) from PR #5256:
these got registered as JSON tags from mappings.yaml, so isMissing queried
json_tree(media_file.tags, '$.replaygain_album_gain') which is always empty
-> the playlist matched ALL songs.
2. Using the canonical field names (rgalbumgain etc.): not a tag/role, so SQL
generation returned an error. Because refreshSmartPlaylist deletes old tracks
before regenerating, the abort left the playlist empty.
Fix: add Nullable bool to FieldInfo and mark the four ReplayGain fields. Add
static alias entries (replaygain_album_gain -> rgalbumgain etc.) with
Numeric+Nullable set; because AddTagNames skips names already in the field map,
these static entries take precedence over the mappings.yaml tag registration.
missingExpr now emits IS NULL / IS NOT NULL for nullable column fields instead
of the json_tree lookup.
Fixes #5584
* chore(smartplaylist): address code review feedback
- Simplify the isMissing/isPresent unsupported-field error message,
removing the internal "nullable fields" jargon
- Standardize comments on mappings.yaml (the actual filename)
- Clarify the alias precedence comment in the LookupField test
2026-06-10 21:12:31 -04:00
Expect ( err ) . To ( MatchError ( ContainSubstring ( "isMissing/isPresent operator is not supported for field" ) ) )
2026-04-28 19:40:08 -04:00
} )
It ( "returns an error when isPresent is used with a regular field" , func ( ) {
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
_ , err := newSmartPlaylistCriteria ( criteria . Criteria { Expression : criteria . IsPresent { "title" : true } } ) . where ( )
fix(smartplaylist): support isMissing/isPresent operators on ReplayGain fields (#5585)
* fix(smartplaylist): support isMissing/isPresent operators on ReplayGain fields
ReplayGain values are stored in dedicated nullable columns (rg_album_gain,
rg_album_peak, rg_track_gain, rg_track_peak) rather than in the media_file.tags
JSON blob. The isMissing/isPresent operators previously only supported tag and
role fields, causing two failure modes:
1. Using the documented alias names (replaygain_album_gain etc.) from PR #5256:
these got registered as JSON tags from mappings.yaml, so isMissing queried
json_tree(media_file.tags, '$.replaygain_album_gain') which is always empty
-> the playlist matched ALL songs.
2. Using the canonical field names (rgalbumgain etc.): not a tag/role, so SQL
generation returned an error. Because refreshSmartPlaylist deletes old tracks
before regenerating, the abort left the playlist empty.
Fix: add Nullable bool to FieldInfo and mark the four ReplayGain fields. Add
static alias entries (replaygain_album_gain -> rgalbumgain etc.) with
Numeric+Nullable set; because AddTagNames skips names already in the field map,
these static entries take precedence over the mappings.yaml tag registration.
missingExpr now emits IS NULL / IS NOT NULL for nullable column fields instead
of the json_tree lookup.
Fixes #5584
* chore(smartplaylist): address code review feedback
- Simplify the isMissing/isPresent unsupported-field error message,
removing the internal "nullable fields" jargon
- Standardize comments on mappings.yaml (the actual filename)
- Clarify the alias precedence comment in the LookupField test
2026-06-10 21:12:31 -04:00
Expect ( err ) . To ( MatchError ( ContainSubstring ( "isMissing/isPresent operator is not supported for field" ) ) )
2026-04-28 19:40:08 -04:00
} )
2026-05-01 19:21:48 -04:00
It ( "returns an error when isMissing has a non-boolean value" , func ( ) {
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
_ , err := newSmartPlaylistCriteria ( criteria . Criteria { Expression : criteria . IsMissing { "genre" : "hello" } } ) . where ( )
2026-05-01 19:21:48 -04:00
Expect ( err ) . To ( MatchError ( ContainSubstring ( "invalid boolean value for 'missing' expression" ) ) )
} )
feat(smartplaylists): add support for referencing playlists using paths (#5187)
* feat: Add support for referencing playlists using paths
Signed-off-by: David <dvedvick@gmail.com>
* feat: Support relative playlist paths in smartlists
Signed-off-by: David <dvedvick@gmail.com>
* fix(smartplaylists): protect against nil panic
Signed-off-by: David <dvedvick@gmail.com>
* fix(smartplaylists): refreshing child playlists
Signed-off-by: David <dvedvick@gmail.com>
* chore(smartplaylists): log field parsing error
Signed-off-by: David <dvedvick@gmail.com>
* fix(smartplaylists): handle empty playlist paths
Signed-off-by: David <dvedvick@gmail.com>
* refactor(smartplaylists): make NormalizeChildPaths non-mutating
Signed-off-by: David <dvedvick@gmail.com>
* fix(smartplaylists): stop warning on every inPlaylist rule without the looked-up field
Rules that reference a playlist by id have no path field, and the reverse, so
the warning fired on every refresh. The log call also had a bad argument count.
* fix(smartplaylists): ignore empty inPlaylist id and path references
An empty path matched every playlist without a file path, including the
referencing playlist itself, so the refresh recursed until the stack overflowed.
An empty id also shadowed a valid path in the same rule.
* fix(smartplaylists): match inPlaylist paths in both NFC and NFD forms
A playlist path is stored in the Unicode form the filesystem reports, which can
differ from the form typed in the .nsp file. The exact comparison then found no
playlist for names with accents.
* fix(smartplaylists): keep all criteria fields when normalizing child paths
The field-by-field copy dropped RefreshDelay.
* fix(smartplaylists): clean absolute inPlaylist path references
Only relative references were cleaned, so an absolute reference such as
/music/./child.nsp never matched the stored /music/child.nsp.
* fix(smartplaylists): stop infinite recursion on playlists that reference each other
Two smart playlists referencing each other, by id or by path, recursed until the
stack overflowed and the server died. The refresh now tracks visited playlists.
* fix(smartplaylists): resolve inPlaylist path references with OS-native separators
Playlist.Path is OS-native, but references in a .nsp file use forward slashes.
On Windows they never matched, and a leading slash was not seen as absolute.
The specs now build OS-native paths, so they also run on Windows.
* fix(smartplaylists): warn when a relative inPlaylist path cannot be resolved
A playlist created in the UI has no file path, so a relative reference silently
matched nothing.
* refactor(smartplaylists): simplify child playlist reference handling
Share one extractor for child ids and paths, return only the normalized rules
instead of a playlist copy, and resolve each path reference in a single switch.
* test(smartplaylists): store the Unicode child path in OS-native form
Playlist.Path is OS-native, so on Windows the forward-slash fixture never matched
the normalized reference.
---------
Signed-off-by: David <dvedvick@gmail.com>
Co-authored-by: Deluan Quintão <deluan@navidrome.org>
2026-09-19 20:05:58 -05:00
It ( "returns an error when inPlaylist has empty path" , func ( ) {
_ , err := newSmartPlaylistCriteria (
criteria . Criteria { Expression : criteria . InPlaylist { "path" : "" } } ,
withSmartPlaylistOwner ( model . User { ID : "owner-id" , IsAdmin : false } ) ) . where ( )
Expect ( err ) . To ( MatchError ( ContainSubstring ( "playlist id or path not given" ) ) )
} )
perf(smartplaylist): use annotation index for playcount/rating/loved filters (#5662)
* fix(smartplaylist): use annotation index for playcount/rating/loved filters
Annotation-field criteria wrapped the column in COALESCE(col, default) so
missing annotation rows behave as 0/false. COALESCE prevents SQLite from
using the column index, forcing a full media_file scan during smart playlist
materialization - multi-second loads on large libraries, independent of rule
complexity.
Store the raw column plus its default and drop COALESCE when the compared
value cannot match the default; fall back to 'col <op> ? OR col IS NULL' when
the default would match, so never-annotated tracks are still preserved.
Sorting keeps COALESCE to retain deterministic NULL ordering. Result set is
unchanged; the materialize query now seeks the annotation index.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for list-valued annotation comparisons
Hardening from final review: a list value (IN (...)) can't drive the index
and a default-inclusive list has per-element NULL semantics, so route slice
values through COALESCE(col, default) to stay exactly equivalent to the prior
form. Also make the bool-default branch explicit (loved only supports
equality operators) and share the COALESCE rendering via coalesceExpr.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): make coalesced() a field method
Thermo-nuclear review follow-up: promote the free coalesceExpr(f) to a
smartPlaylistField.coalesced() method that returns the bare expression when
there is no default. This lets sortExpr call field.coalesced() unconditionally
and drop its 'if coalesceDefault != nil' branch, removing the 'only annotation
fields get coalesced' special case from the sort path. Behavior unchanged.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for LIKE, bool-ordering, and tag ranges
Code review (xhigh) found the index-friendly rewrite did not cover every
operator, breaking result-set equivalence on a few reachable raw-JSON paths:
- LIKE family (contains/startsWith/endsWith/notContains) on annotation fields
used the bare column, so a NULL column never matched and missing-annotation
rows were dropped.
- Ordering comparators (gt/lt/...) on bool fields (loved) were decided as
equality, wrongly including never-annotated rows.
- InTheRange on a numeric tag split into two independent json_tree EXISTS,
letting different tag values satisfy each bound.
Centralize the decision in annotationCond via bareNullInclusion: emit the
index-friendly bare form only for scalar values under an exactly-orderable
comparator, otherwise fall back to the COALESCE form (always equivalent to the
original). Route LIKE through coalesced(); reject tag/role ranges. Replace the
local toFloat/toBool with spf13/cast (fixes unhandled numeric types and string
bool forms), and drop the redundant LookupField + double reflect.TypeOf.
A 17-case brute-force check confirms row-set equivalence to the prior
COALESCE form across all operators including the fixed LIKE/bool cases.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for list values on bool annotation fields
Second review found the bareNullInclusion bool branch missed the non-scalar
guard the numeric branch has: a list value on loved/albumloved/artistloved
(e.g. {"is":{"loved":[true]}}) coerced through toBool (which swallowed the
cast error) to false, emitting the bare/OR-IS-NULL form and wrongly including
never-annotated rows. Make toBool return (value, ok) like toFloat and bail to
COALESCE when the value isn't a scalar bool. Also add the missing test for the
tag/role range rejection. A 21-case brute-force confirms row-set equivalence to
the original COALESCE form across every operator, including the bool/numeric
list paths.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): drop spf13/cast for stdlib value coercion
The value coercion only sees the handful of types criteria produces (int,
float64, string from JSON; bool already normalized at unmarshal), so cast's
broad conversion isn't needed. Use small explicit type switches over strconv
instead, keeping the string fallback (ParseFloat/ParseBool) that closes the
'1'/'t' gap. No dependency change — cast returns to indirect.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): share bool coercion via criteria.ToBool
normalizeBoolValue (unmarshal-time) and the persistence bool guard both parsed
bool-ish values independently. Extract the shared logic into an exported
criteria.ToBool(any) (bool, ok): normalizeBoolValue delegates to it (behavior
unchanged), and the persistence layer reuses it via its existing model/criteria
import instead of a local helper. No behavior change.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): trim sqlLiteral and dedup rationale comments
/simplify cleanup: fmt %v already renders bool defaults as false/true, so drop
sqlLiteral's redundant bool branch. Consolidate the COALESCE-vs-index rationale
to the smartPlaylistField comment instead of repeating it across annotationCond
and the struct. No behavior change.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): address review feedback on multi-field maps and *any
From the PR bot reviews:
- sqlFields now uses the field's coalesced() form, so annotation fields in a
multi-field operator map (Is/Gt/Contains with >1 key) keep COALESCE and don't
silently drop never-annotated rows. Covers both the comparison and LIKE
fallback paths. (Gemini high, Copilot)
- Replace coalesceDefault *any with a plain any (0/false are non-nil
interfaces, so nil still means 'no default'); drop the coalesce() boxing
helper and the pointer indirection. (Gemini)
- Give rangeExpr clear, range-specific errors for the multi-field and malformed
-pair cases instead of an empty-field / 'in operator' message. (Copilot)
Adds tests for the multi-field COALESCE behavior and the new range errors.
Signed-off-by: Deluan <deluan@deluan.com>
* Revert multi-field COALESCE handling (YAGNI)
The multi-field operator map case the bots flagged is unreachable: marshalExpression
rejects any operator map with more than one field, so a multi-field map can never be
persisted or loaded. Revert the sqlFields change and its tests rather than harden a
code path no supported input can reach. Keep the two reachable improvements from the
review: coalesceDefault any (not *any), and the clearer malformed-range error.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(persistence): model comparator as a behavior-carrying struct
The smart-playlist comparator was a bare string alias, forcing two parallel
switches over the same six operators: squirrelCmp mapped each to its squirrel
constructor, and bareNullInclusion restated each as a float predicate. Adding
or changing an operator meant editing both in sync.
Make comparator a struct that bundles those facts per operator (the squirrel
builder, the operator as a float predicate, and whether it's an ordering op).
Both switches collapse: squirrelCmp is deleted in favor of cmp.build, and
bareNullInclusion's numeric switch becomes a single cmp.satisfy call. Generated
SQL is unchanged, as the existing table-driven tests confirm.
* docs(smartplaylist): trim comments that restate the code
Remove or tighten comments that describe what the code already says (likeCond and
comparisonExpr doc lines, redundant clauses in annotationField/coalesced/ToBool/
normalizeBoolValue). Keep the comments that explain non-obvious rationale: the
COALESCE-vs-index tradeoff, the bareNullInclusion/annotationCond contracts, and the
why-we-fall-back notes.
* docs(smartplaylist): collapse coalesceDefault comment to one line
The field's six-line block duplicated the COALESCE-vs-index rationale that already
lives on annotationCond. Reduce it to a one-line description plus a pointer there.
---------
Signed-off-by: Deluan <deluan@deluan.com>
2026-06-24 22:26:41 -04:00
It ( "returns an error for a range over a tag/role field" , func ( ) {
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
_ , err := newSmartPlaylistCriteria ( criteria . Criteria { Expression : criteria . InTheRange { "rate" : [ ] int { 1 , 5 } } } ) . where ( )
perf(smartplaylist): use annotation index for playcount/rating/loved filters (#5662)
* fix(smartplaylist): use annotation index for playcount/rating/loved filters
Annotation-field criteria wrapped the column in COALESCE(col, default) so
missing annotation rows behave as 0/false. COALESCE prevents SQLite from
using the column index, forcing a full media_file scan during smart playlist
materialization - multi-second loads on large libraries, independent of rule
complexity.
Store the raw column plus its default and drop COALESCE when the compared
value cannot match the default; fall back to 'col <op> ? OR col IS NULL' when
the default would match, so never-annotated tracks are still preserved.
Sorting keeps COALESCE to retain deterministic NULL ordering. Result set is
unchanged; the materialize query now seeks the annotation index.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for list-valued annotation comparisons
Hardening from final review: a list value (IN (...)) can't drive the index
and a default-inclusive list has per-element NULL semantics, so route slice
values through COALESCE(col, default) to stay exactly equivalent to the prior
form. Also make the bool-default branch explicit (loved only supports
equality operators) and share the COALESCE rendering via coalesceExpr.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): make coalesced() a field method
Thermo-nuclear review follow-up: promote the free coalesceExpr(f) to a
smartPlaylistField.coalesced() method that returns the bare expression when
there is no default. This lets sortExpr call field.coalesced() unconditionally
and drop its 'if coalesceDefault != nil' branch, removing the 'only annotation
fields get coalesced' special case from the sort path. Behavior unchanged.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for LIKE, bool-ordering, and tag ranges
Code review (xhigh) found the index-friendly rewrite did not cover every
operator, breaking result-set equivalence on a few reachable raw-JSON paths:
- LIKE family (contains/startsWith/endsWith/notContains) on annotation fields
used the bare column, so a NULL column never matched and missing-annotation
rows were dropped.
- Ordering comparators (gt/lt/...) on bool fields (loved) were decided as
equality, wrongly including never-annotated rows.
- InTheRange on a numeric tag split into two independent json_tree EXISTS,
letting different tag values satisfy each bound.
Centralize the decision in annotationCond via bareNullInclusion: emit the
index-friendly bare form only for scalar values under an exactly-orderable
comparator, otherwise fall back to the COALESCE form (always equivalent to the
original). Route LIKE through coalesced(); reject tag/role ranges. Replace the
local toFloat/toBool with spf13/cast (fixes unhandled numeric types and string
bool forms), and drop the redundant LookupField + double reflect.TypeOf.
A 17-case brute-force check confirms row-set equivalence to the prior
COALESCE form across all operators including the fixed LIKE/bool cases.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for list values on bool annotation fields
Second review found the bareNullInclusion bool branch missed the non-scalar
guard the numeric branch has: a list value on loved/albumloved/artistloved
(e.g. {"is":{"loved":[true]}}) coerced through toBool (which swallowed the
cast error) to false, emitting the bare/OR-IS-NULL form and wrongly including
never-annotated rows. Make toBool return (value, ok) like toFloat and bail to
COALESCE when the value isn't a scalar bool. Also add the missing test for the
tag/role range rejection. A 21-case brute-force confirms row-set equivalence to
the original COALESCE form across every operator, including the bool/numeric
list paths.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): drop spf13/cast for stdlib value coercion
The value coercion only sees the handful of types criteria produces (int,
float64, string from JSON; bool already normalized at unmarshal), so cast's
broad conversion isn't needed. Use small explicit type switches over strconv
instead, keeping the string fallback (ParseFloat/ParseBool) that closes the
'1'/'t' gap. No dependency change — cast returns to indirect.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): share bool coercion via criteria.ToBool
normalizeBoolValue (unmarshal-time) and the persistence bool guard both parsed
bool-ish values independently. Extract the shared logic into an exported
criteria.ToBool(any) (bool, ok): normalizeBoolValue delegates to it (behavior
unchanged), and the persistence layer reuses it via its existing model/criteria
import instead of a local helper. No behavior change.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): trim sqlLiteral and dedup rationale comments
/simplify cleanup: fmt %v already renders bool defaults as false/true, so drop
sqlLiteral's redundant bool branch. Consolidate the COALESCE-vs-index rationale
to the smartPlaylistField comment instead of repeating it across annotationCond
and the struct. No behavior change.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): address review feedback on multi-field maps and *any
From the PR bot reviews:
- sqlFields now uses the field's coalesced() form, so annotation fields in a
multi-field operator map (Is/Gt/Contains with >1 key) keep COALESCE and don't
silently drop never-annotated rows. Covers both the comparison and LIKE
fallback paths. (Gemini high, Copilot)
- Replace coalesceDefault *any with a plain any (0/false are non-nil
interfaces, so nil still means 'no default'); drop the coalesce() boxing
helper and the pointer indirection. (Gemini)
- Give rangeExpr clear, range-specific errors for the multi-field and malformed
-pair cases instead of an empty-field / 'in operator' message. (Copilot)
Adds tests for the multi-field COALESCE behavior and the new range errors.
Signed-off-by: Deluan <deluan@deluan.com>
* Revert multi-field COALESCE handling (YAGNI)
The multi-field operator map case the bots flagged is unreachable: marshalExpression
rejects any operator map with more than one field, so a multi-field map can never be
persisted or loaded. Revert the sqlFields change and its tests rather than harden a
code path no supported input can reach. Keep the two reachable improvements from the
review: coalesceDefault any (not *any), and the clearer malformed-range error.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(persistence): model comparator as a behavior-carrying struct
The smart-playlist comparator was a bare string alias, forcing two parallel
switches over the same six operators: squirrelCmp mapped each to its squirrel
constructor, and bareNullInclusion restated each as a float predicate. Adding
or changing an operator meant editing both in sync.
Make comparator a struct that bundles those facts per operator (the squirrel
builder, the operator as a float predicate, and whether it's an ordering op).
Both switches collapse: squirrelCmp is deleted in favor of cmp.build, and
bareNullInclusion's numeric switch becomes a single cmp.satisfy call. Generated
SQL is unchanged, as the existing table-driven tests confirm.
* docs(smartplaylist): trim comments that restate the code
Remove or tighten comments that describe what the code already says (likeCond and
comparisonExpr doc lines, redundant clauses in annotationField/coalesced/ToBool/
normalizeBoolValue). Keep the comments that explain non-obvious rationale: the
COALESCE-vs-index tradeoff, the bareNullInclusion/annotationCond contracts, and the
why-we-fall-back notes.
* docs(smartplaylist): collapse coalesceDefault comment to one line
The field's six-line block duplicated the COALESCE-vs-index rationale that already
lives on annotationCond. Reduce it to a one-line description plus a pointer there.
---------
Signed-off-by: Deluan <deluan@deluan.com>
2026-06-24 22:26:41 -04:00
Expect ( err ) . To ( MatchError ( ContainSubstring ( "range operator not supported for tag/role field" ) ) )
} )
It ( "returns a clear error for a malformed range value" , func ( ) {
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
_ , err := newSmartPlaylistCriteria ( criteria . Criteria { Expression : criteria . InTheRange { "playCount" : [ ] int { 1 , 2 , 3 } } } ) . where ( )
perf(smartplaylist): use annotation index for playcount/rating/loved filters (#5662)
* fix(smartplaylist): use annotation index for playcount/rating/loved filters
Annotation-field criteria wrapped the column in COALESCE(col, default) so
missing annotation rows behave as 0/false. COALESCE prevents SQLite from
using the column index, forcing a full media_file scan during smart playlist
materialization - multi-second loads on large libraries, independent of rule
complexity.
Store the raw column plus its default and drop COALESCE when the compared
value cannot match the default; fall back to 'col <op> ? OR col IS NULL' when
the default would match, so never-annotated tracks are still preserved.
Sorting keeps COALESCE to retain deterministic NULL ordering. Result set is
unchanged; the materialize query now seeks the annotation index.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for list-valued annotation comparisons
Hardening from final review: a list value (IN (...)) can't drive the index
and a default-inclusive list has per-element NULL semantics, so route slice
values through COALESCE(col, default) to stay exactly equivalent to the prior
form. Also make the bool-default branch explicit (loved only supports
equality operators) and share the COALESCE rendering via coalesceExpr.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): make coalesced() a field method
Thermo-nuclear review follow-up: promote the free coalesceExpr(f) to a
smartPlaylistField.coalesced() method that returns the bare expression when
there is no default. This lets sortExpr call field.coalesced() unconditionally
and drop its 'if coalesceDefault != nil' branch, removing the 'only annotation
fields get coalesced' special case from the sort path. Behavior unchanged.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for LIKE, bool-ordering, and tag ranges
Code review (xhigh) found the index-friendly rewrite did not cover every
operator, breaking result-set equivalence on a few reachable raw-JSON paths:
- LIKE family (contains/startsWith/endsWith/notContains) on annotation fields
used the bare column, so a NULL column never matched and missing-annotation
rows were dropped.
- Ordering comparators (gt/lt/...) on bool fields (loved) were decided as
equality, wrongly including never-annotated rows.
- InTheRange on a numeric tag split into two independent json_tree EXISTS,
letting different tag values satisfy each bound.
Centralize the decision in annotationCond via bareNullInclusion: emit the
index-friendly bare form only for scalar values under an exactly-orderable
comparator, otherwise fall back to the COALESCE form (always equivalent to the
original). Route LIKE through coalesced(); reject tag/role ranges. Replace the
local toFloat/toBool with spf13/cast (fixes unhandled numeric types and string
bool forms), and drop the redundant LookupField + double reflect.TypeOf.
A 17-case brute-force check confirms row-set equivalence to the prior
COALESCE form across all operators including the fixed LIKE/bool cases.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): keep COALESCE for list values on bool annotation fields
Second review found the bareNullInclusion bool branch missed the non-scalar
guard the numeric branch has: a list value on loved/albumloved/artistloved
(e.g. {"is":{"loved":[true]}}) coerced through toBool (which swallowed the
cast error) to false, emitting the bare/OR-IS-NULL form and wrongly including
never-annotated rows. Make toBool return (value, ok) like toFloat and bail to
COALESCE when the value isn't a scalar bool. Also add the missing test for the
tag/role range rejection. A 21-case brute-force confirms row-set equivalence to
the original COALESCE form across every operator, including the bool/numeric
list paths.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): drop spf13/cast for stdlib value coercion
The value coercion only sees the handful of types criteria produces (int,
float64, string from JSON; bool already normalized at unmarshal), so cast's
broad conversion isn't needed. Use small explicit type switches over strconv
instead, keeping the string fallback (ParseFloat/ParseBool) that closes the
'1'/'t' gap. No dependency change — cast returns to indirect.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): share bool coercion via criteria.ToBool
normalizeBoolValue (unmarshal-time) and the persistence bool guard both parsed
bool-ish values independently. Extract the shared logic into an exported
criteria.ToBool(any) (bool, ok): normalizeBoolValue delegates to it (behavior
unchanged), and the persistence layer reuses it via its existing model/criteria
import instead of a local helper. No behavior change.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(smartplaylist): trim sqlLiteral and dedup rationale comments
/simplify cleanup: fmt %v already renders bool defaults as false/true, so drop
sqlLiteral's redundant bool branch. Consolidate the COALESCE-vs-index rationale
to the smartPlaylistField comment instead of repeating it across annotationCond
and the struct. No behavior change.
Signed-off-by: Deluan <deluan@deluan.com>
* fix(smartplaylist): address review feedback on multi-field maps and *any
From the PR bot reviews:
- sqlFields now uses the field's coalesced() form, so annotation fields in a
multi-field operator map (Is/Gt/Contains with >1 key) keep COALESCE and don't
silently drop never-annotated rows. Covers both the comparison and LIKE
fallback paths. (Gemini high, Copilot)
- Replace coalesceDefault *any with a plain any (0/false are non-nil
interfaces, so nil still means 'no default'); drop the coalesce() boxing
helper and the pointer indirection. (Gemini)
- Give rangeExpr clear, range-specific errors for the multi-field and malformed
-pair cases instead of an empty-field / 'in operator' message. (Copilot)
Adds tests for the multi-field COALESCE behavior and the new range errors.
Signed-off-by: Deluan <deluan@deluan.com>
* Revert multi-field COALESCE handling (YAGNI)
The multi-field operator map case the bots flagged is unreachable: marshalExpression
rejects any operator map with more than one field, so a multi-field map can never be
persisted or loaded. Revert the sqlFields change and its tests rather than harden a
code path no supported input can reach. Keep the two reachable improvements from the
review: coalesceDefault any (not *any), and the clearer malformed-range error.
Signed-off-by: Deluan <deluan@deluan.com>
* refactor(persistence): model comparator as a behavior-carrying struct
The smart-playlist comparator was a bare string alias, forcing two parallel
switches over the same six operators: squirrelCmp mapped each to its squirrel
constructor, and bareNullInclusion restated each as a float predicate. Adding
or changing an operator meant editing both in sync.
Make comparator a struct that bundles those facts per operator (the squirrel
builder, the operator as a float predicate, and whether it's an ordering op).
Both switches collapse: squirrelCmp is deleted in favor of cmp.build, and
bareNullInclusion's numeric switch becomes a single cmp.satisfy call. Generated
SQL is unchanged, as the existing table-driven tests confirm.
* docs(smartplaylist): trim comments that restate the code
Remove or tighten comments that describe what the code already says (likeCond and
comparisonExpr doc lines, redundant clauses in annotationField/coalesced/ToBool/
normalizeBoolValue). Keep the comments that explain non-obvious rationale: the
COALESCE-vs-index tradeoff, the bareNullInclusion/annotationCond contracts, and the
why-we-fall-back notes.
* docs(smartplaylist): collapse coalesceDefault comment to one line
The field's six-line block duplicated the COALESCE-vs-index rationale that already
lives on annotationCond. Reduce it to a one-line description plus a pointer there.
---------
Signed-off-by: Deluan <deluan@deluan.com>
2026-06-24 22:26:41 -04:00
Expect ( err ) . To ( MatchError ( ContainSubstring ( "must be a [min, max] pair" ) ) )
} )
2026-04-24 23:18:20 -04:00
Describe ( "sort" , func ( ) {
It ( "sorts by regular fields" , func ( ) {
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
Expect ( newSmartPlaylistCriteria ( criteria . Criteria { Sort : "title" } ) . orderBy ( ) ) . To ( Equal ( "media_file.title asc" ) )
2026-04-24 23:18:20 -04:00
} )
It ( "sorts by tag fields" , func ( ) {
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
Expect ( newSmartPlaylistCriteria ( criteria . Criteria { Sort : "genre" } ) . orderBy ( ) ) . To ( Equal ( "COALESCE(json_extract(media_file.tags, '$.genre[0].value'), '') asc" ) )
2026-04-24 23:18:20 -04:00
} )
It ( "sorts by role fields" , func ( ) {
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
Expect ( newSmartPlaylistCriteria ( criteria . Criteria { Sort : "artist" } ) . orderBy ( ) ) . To ( Equal ( "COALESCE(json_extract(media_file.participants, '$.artist[0].name'), '') asc" ) )
2026-04-24 23:18:20 -04:00
} )
It ( "casts numeric tags when sorting" , func ( ) {
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
Expect ( newSmartPlaylistCriteria ( criteria . Criteria { Sort : "rate" } ) . orderBy ( ) ) . To ( Equal ( "CAST(COALESCE(json_extract(media_file.tags, '$.rate[0].value'), '') AS REAL) asc" ) )
2026-04-24 23:18:20 -04:00
} )
It ( "sorts by albumtype alias" , func ( ) {
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
Expect ( newSmartPlaylistCriteria ( criteria . Criteria { Sort : "albumtype" } ) . orderBy ( ) ) . To ( Equal ( "COALESCE(json_extract(media_file.tags, '$.releasetype[0].value'), '') asc" ) )
2026-04-24 23:18:20 -04:00
} )
It ( "sorts by random" , func ( ) {
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
Expect ( newSmartPlaylistCriteria ( criteria . Criteria { Sort : "random" } ) . orderBy ( ) ) . To ( Equal ( "random() asc" ) )
2026-04-24 23:18:20 -04:00
} )
feat(smartplaylist): add album-level fields for sorting and filtering (#5899)
Smart playlists could only sort by the track-level `dateadded`, which scatters
an album's tracks because every track has its own timestamp. There was no way
to express "newest albums first, tracks in album order".
Adds five fields backed by the album table: albumdateadded, albumdatemodified,
albumduration, albumsongcount and albumsize, so `"sort": "-albumdateadded,
tracknumber"` now works. They filter as well as sort, which also enables rules
like "everything from albums added this month" or "skip singles and EPs".
These need the album table rather than the album_annotation table the existing
album* fields join, so they get their own bit in the join mask. The bitmask
already unions joins from both the expression and the sort fields, so a
sort-only reference pulls in the join for the main query while correctly
staying out of the percentage-limit count query.
No COALESCE default is used. That mechanism exists because an album_annotation
row is genuinely often absent; the album row always exists and song_count,
duration and size are NOT NULL, so leaving the columns bare keeps filters
index-friendly.
albumdatemodified follows the existing track-level `datemodified` naming rather
than the `albumdateupdated` spelling used in the request.
albumreleasedate was considered and rejected: album.release_date is
allOrNothing() over the tracks, so it equals the track-level releasedate when
they agree and collapses to empty when they disagree.
Closes https://github.com/navidrome/navidrome/discussions/5347
2026-08-06 10:59:50 -04:00
It ( "sorts by album columns bare, with no COALESCE default" , func ( ) {
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
Expect ( newSmartPlaylistCriteria ( criteria . Criteria { Sort : "-albumDateAdded,trackNumber" } ) . orderBy ( ) ) .
feat(smartplaylist): add album-level fields for sorting and filtering (#5899)
Smart playlists could only sort by the track-level `dateadded`, which scatters
an album's tracks because every track has its own timestamp. There was no way
to express "newest albums first, tracks in album order".
Adds five fields backed by the album table: albumdateadded, albumdatemodified,
albumduration, albumsongcount and albumsize, so `"sort": "-albumdateadded,
tracknumber"` now works. They filter as well as sort, which also enables rules
like "everything from albums added this month" or "skip singles and EPs".
These need the album table rather than the album_annotation table the existing
album* fields join, so they get their own bit in the join mask. The bitmask
already unions joins from both the expression and the sort fields, so a
sort-only reference pulls in the join for the main query while correctly
staying out of the percentage-limit count query.
No COALESCE default is used. That mechanism exists because an album_annotation
row is genuinely often absent; the album row always exists and song_count,
duration and size are NOT NULL, so leaving the columns bare keeps filters
index-friendly.
albumdatemodified follows the existing track-level `datemodified` naming rather
than the `albumdateupdated` spelling used in the request.
albumreleasedate was considered and rejected: album.release_date is
allOrNothing() over the tracks, so it equals the track-level releasedate when
they agree and collapses to empty when they disagree.
Closes https://github.com/navidrome/navidrome/discussions/5347
2026-08-06 10:59:50 -04:00
To ( Equal ( "album.created_at desc, media_file.track_number asc" ) )
} )
2026-04-24 23:18:20 -04:00
It ( "sorts by multiple fields" , func ( ) {
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
Expect ( newSmartPlaylistCriteria ( criteria . Criteria { Sort : "title,-rating" } ) . orderBy ( ) ) . To ( Equal ( "media_file.title asc, COALESCE(annotation.rating, 0) desc" ) )
2026-04-24 23:18:20 -04:00
} )
It ( "reverts order when order is desc" , func ( ) {
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
Expect ( newSmartPlaylistCriteria ( criteria . Criteria { Sort : "-date,artist" , Order : "desc" } ) . orderBy ( ) ) . To ( Equal ( "media_file.date asc, COALESCE(json_extract(media_file.participants, '$.artist[0].name'), '') desc" ) )
2026-04-24 23:18:20 -04:00
} )
It ( "ignores invalid sort fields" , func ( ) {
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
Expect ( newSmartPlaylistCriteria ( criteria . Criteria { Sort : "bogus,title" } ) . orderBy ( ) ) . To ( Equal ( "media_file.title asc" ) )
2026-04-24 23:18:20 -04:00
} )
} )
It ( "has SQL mappings for all non-tag/non-role criteria fields" , func ( ) {
for _ , name := range criteria . AllFieldNames ( ) {
info , ok := criteria . LookupField ( name )
Expect ( ok ) . To ( BeTrue ( ) , "field %q registered but LookupField fails" , name )
if info . IsTag || info . IsRole {
continue
}
2026-04-28 20:22:48 -04:00
_ , hasSQLField := smartPlaylistFields [ info . Name ( ) ]
Expect ( hasSQLField ) . To ( BeTrue ( ) , "criteria field %q (name=%q) has no entry in smartPlaylistFields" , name , info . Name ( ) )
2026-04-24 23:18:20 -04:00
}
} )
feat(smartplaylist): add album-level fields for sorting and filtering (#5899)
Smart playlists could only sort by the track-level `dateadded`, which scatters
an album's tracks because every track has its own timestamp. There was no way
to express "newest albums first, tracks in album order".
Adds five fields backed by the album table: albumdateadded, albumdatemodified,
albumduration, albumsongcount and albumsize, so `"sort": "-albumdateadded,
tracknumber"` now works. They filter as well as sort, which also enables rules
like "everything from albums added this month" or "skip singles and EPs".
These need the album table rather than the album_annotation table the existing
album* fields join, so they get their own bit in the join mask. The bitmask
already unions joins from both the expression and the sort fields, so a
sort-only reference pulls in the join for the main query while correctly
staying out of the percentage-limit count query.
No COALESCE default is used. That mechanism exists because an album_annotation
row is genuinely often absent; the album row always exists and song_count,
duration and size are NOT NULL, so leaving the columns bare keeps filters
index-friendly.
albumdatemodified follows the existing track-level `datemodified` naming rather
than the `albumdateupdated` spelling used in the request.
albumreleasedate was considered and rejected: album.release_date is
allOrNothing() over the tracks, so it equals the track-level releasedate when
they agree and collapses to empty when they disagree.
Closes https://github.com/navidrome/navidrome/discussions/5347
2026-08-06 10:59:50 -04:00
It ( "declares a joinType matching the table each field selects from" , func ( ) {
// Omitting the joinType still compiles, so without this the field would only fail at
// refresh time with "no such column".
joinByTable := map [ string ] smartPlaylistJoinType {
"media_file" : smartPlaylistJoinNone ,
"annotation" : smartPlaylistJoinNone ,
"album" : smartPlaylistJoinAlbum ,
"album_annotation" : smartPlaylistJoinAlbumAnnotation ,
"artist_annotation" : smartPlaylistJoinArtistAnnotation ,
}
for name , field := range smartPlaylistFields {
if field . expr == "" {
continue
}
table , _ , ok := strings . Cut ( field . expr , "." )
Expect ( ok ) . To ( BeTrue ( ) , "field %q has expr %q with no table prefix" , name , field . expr )
want , known := joinByTable [ table ]
Expect ( known ) . To ( BeTrue ( ) , "field %q selects from unknown table %q" , name , table )
Expect ( field . joinType ) . To ( Equal ( want ) , "field %q selects from %q but declares the wrong joinType" , name , table )
}
} )
2026-05-22 18:00:13 -03:00
Describe ( "JSON condition merging" , func ( ) {
It ( "merges multiple role conditions in an OR group into a single EXISTS" , func ( ) {
expr := criteria . Any {
criteria . Contains { "artist" : "Beatles" } ,
criteria . Contains { "artist" : "Kraftwerk" } ,
criteria . Contains { "artist" : "Pink Floyd" } ,
}
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
sqlizer , err := newSmartPlaylistCriteria ( criteria . Criteria { Expression : expr } ) . where ( )
2026-05-22 18:00:13 -03:00
Expect ( err ) . ToNot ( HaveOccurred ( ) )
sql , args , err := sqlizer . ToSql ( )
Expect ( err ) . ToNot ( HaveOccurred ( ) )
Expect ( sql ) . To ( Equal ( "(exists (select 1 from media_file_artists mfa join artist on artist.id = mfa.artist_id where mfa.media_file_id = media_file.id and mfa.role = ? and (artist.name LIKE ? OR artist.name LIKE ? OR artist.name LIKE ?)))" ) )
Expect ( args ) . To ( HaveExactElements ( "artist" , "%Beatles%" , "%Kraftwerk%" , "%Pink Floyd%" ) )
} )
It ( "does not merge role conditions from different roles" , func ( ) {
expr := criteria . Any {
criteria . Contains { "artist" : "Beatles" } ,
criteria . Contains { "composer" : "Lennon" } ,
}
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
sqlizer , err := newSmartPlaylistCriteria ( criteria . Criteria { Expression : expr } ) . where ( )
2026-05-22 18:00:13 -03:00
Expect ( err ) . ToNot ( HaveOccurred ( ) )
sql , _ , err := sqlizer . ToSql ( )
Expect ( err ) . ToNot ( HaveOccurred ( ) )
Expect ( sql ) . To ( ContainSubstring ( "mfa.role = ?" ) )
// Two separate EXISTS since roles differ
Expect ( strings . Count ( sql , "exists" ) ) . To ( Equal ( 2 ) )
} )
It ( "does not merge negated role conditions" , func ( ) {
expr := criteria . Any {
criteria . NotContains { "artist" : "Beatles" } ,
criteria . NotContains { "artist" : "Kraftwerk" } ,
}
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
sqlizer , err := newSmartPlaylistCriteria ( criteria . Criteria { Expression : expr } ) . where ( )
2026-05-22 18:00:13 -03:00
Expect ( err ) . ToNot ( HaveOccurred ( ) )
sql , _ , err := sqlizer . ToSql ( )
Expect ( err ) . ToNot ( HaveOccurred ( ) )
// Two separate "not exists" since they are negated
Expect ( strings . Count ( sql , "not exists" ) ) . To ( Equal ( 2 ) )
} )
It ( "batches large groups to avoid SQLite expression tree depth limit" , func ( ) {
// Create jsonCondBatchSize + 1 conditions to trigger batching into 2 groups
anyExprs := make ( criteria . Any , jsonCondBatchSize + 1 )
for i := range anyExprs {
anyExprs [ i ] = criteria . Contains { "artist" : fmt . Sprintf ( "Artist%d" , i ) }
}
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
sqlizer , err := newSmartPlaylistCriteria ( criteria . Criteria { Expression : anyExprs } ) . where ( )
2026-05-22 18:00:13 -03:00
Expect ( err ) . ToNot ( HaveOccurred ( ) )
sql , args , err := sqlizer . ToSql ( )
Expect ( err ) . ToNot ( HaveOccurred ( ) )
// Should produce 2 EXISTS subqueries (one batch of jsonCondBatchSize, one of 1)
Expect ( strings . Count ( sql , "exists" ) ) . To ( Equal ( 2 ) )
// First batch has jsonCondBatchSize patterns, second has 1 => total args:
// 2 roles + (jsonCondBatchSize + 1) patterns
Expect ( args ) . To ( HaveLen ( 2 + jsonCondBatchSize + 1 ) )
} )
It ( "merges role conditions while preserving non-role conditions" , func ( ) {
expr := criteria . Any {
criteria . Contains { "title" : "Love" } ,
criteria . Contains { "artist" : "Beatles" } ,
criteria . Contains { "artist" : "Kraftwerk" } ,
}
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
sqlizer , err := newSmartPlaylistCriteria ( criteria . Criteria { Expression : expr } ) . where ( )
2026-05-22 18:00:13 -03:00
Expect ( err ) . ToNot ( HaveOccurred ( ) )
sql , args , err := sqlizer . ToSql ( )
Expect ( err ) . ToNot ( HaveOccurred ( ) )
Expect ( sql ) . To ( ContainSubstring ( "media_file.title LIKE ?" ) )
Expect ( sql ) . To ( ContainSubstring ( "artist.name LIKE ? OR artist.name LIKE ?" ) )
Expect ( args ) . To ( HaveExactElements ( "%Love%" , "artist" , "%Beatles%" , "%Kraftwerk%" ) )
} )
It ( "merges multiple tag conditions in an OR group into a single EXISTS" , func ( ) {
expr := criteria . Any {
criteria . Contains { "genre" : "Rock" } ,
criteria . Contains { "genre" : "Metal" } ,
criteria . Contains { "genre" : "Punk" } ,
}
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
sqlizer , err := newSmartPlaylistCriteria ( criteria . Criteria { Expression : expr } ) . where ( )
2026-05-22 18:00:13 -03:00
Expect ( err ) . ToNot ( HaveOccurred ( ) )
sql , args , err := sqlizer . ToSql ( )
Expect ( err ) . ToNot ( HaveOccurred ( ) )
Expect ( sql ) . To ( Equal ( "(exists (select 1 from json_tree(media_file.tags, '$.genre') where key='value' and (value LIKE ? OR value LIKE ? OR value LIKE ?)))" ) )
Expect ( args ) . To ( HaveExactElements ( "%Rock%" , "%Metal%" , "%Punk%" ) )
} )
It ( "does not merge tag conditions from different tags" , func ( ) {
expr := criteria . Any {
criteria . Contains { "genre" : "Rock" } ,
criteria . Contains { "mood" : "Happy" } ,
}
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
sqlizer , err := newSmartPlaylistCriteria ( criteria . Criteria { Expression : expr } ) . where ( )
2026-05-22 18:00:13 -03:00
Expect ( err ) . ToNot ( HaveOccurred ( ) )
sql , _ , err := sqlizer . ToSql ( )
Expect ( err ) . ToNot ( HaveOccurred ( ) )
Expect ( strings . Count ( sql , "exists" ) ) . To ( Equal ( 2 ) )
} )
It ( "does not merge negated tag conditions" , func ( ) {
expr := criteria . Any {
criteria . NotContains { "genre" : "Rock" } ,
criteria . NotContains { "genre" : "Metal" } ,
}
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
sqlizer , err := newSmartPlaylistCriteria ( criteria . Criteria { Expression : expr } ) . where ( )
2026-05-22 18:00:13 -03:00
Expect ( err ) . ToNot ( HaveOccurred ( ) )
sql , _ , err := sqlizer . ToSql ( )
Expect ( err ) . ToNot ( HaveOccurred ( ) )
Expect ( strings . Count ( sql , "not exists" ) ) . To ( Equal ( 2 ) )
} )
It ( "merges role and tag conditions independently" , func ( ) {
expr := criteria . Any {
criteria . Contains { "artist" : "Beatles" } ,
criteria . Contains { "artist" : "Kraftwerk" } ,
criteria . Contains { "genre" : "Rock" } ,
criteria . Contains { "genre" : "Metal" } ,
}
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
sqlizer , err := newSmartPlaylistCriteria ( criteria . Criteria { Expression : expr } ) . where ( )
2026-05-22 18:00:13 -03:00
Expect ( err ) . ToNot ( HaveOccurred ( ) )
sql , args , err := sqlizer . ToSql ( )
Expect ( err ) . ToNot ( HaveOccurred ( ) )
// Two merged EXISTS: one for roles, one for tags
Expect ( strings . Count ( sql , "exists" ) ) . To ( Equal ( 2 ) )
Expect ( sql ) . To ( ContainSubstring ( "artist.name LIKE ? OR artist.name LIKE ?" ) )
Expect ( sql ) . To ( ContainSubstring ( "value LIKE ? OR value LIKE ?" ) )
Expect ( args ) . To ( HaveLen ( 2 + 2 + 1 ) ) // 2 tag patterns + 2 role patterns + 1 role name
} )
2026-06-14 10:47:11 -04:00
It ( "merges negated role conditions in an AND group into a single NOT EXISTS" , func ( ) {
expr := criteria . All {
criteria . IsNot { "artist" : "Beatles" } ,
criteria . IsNot { "artist" : "Kraftwerk" } ,
}
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
sqlizer , err := newSmartPlaylistCriteria ( criteria . Criteria { Expression : expr } ) . where ( )
2026-06-14 10:47:11 -04:00
Expect ( err ) . ToNot ( HaveOccurred ( ) )
sql , args , err := sqlizer . ToSql ( )
Expect ( err ) . ToNot ( HaveOccurred ( ) )
// A single NOT EXISTS with both names ORed inside (De Morgan)
Expect ( strings . Count ( sql , "not exists" ) ) . To ( Equal ( 1 ) )
Expect ( sql ) . To ( ContainSubstring ( "artist.name = ? OR artist.name = ?" ) )
Expect ( args ) . To ( HaveExactElements ( "artist" , "Beatles" , "Kraftwerk" ) )
} )
It ( "merges negated notContains role conditions in an AND group" , func ( ) {
expr := criteria . All {
criteria . NotContains { "artist" : "Beatles" } ,
criteria . NotContains { "artist" : "Kraftwerk" } ,
}
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
sqlizer , err := newSmartPlaylistCriteria ( criteria . Criteria { Expression : expr } ) . where ( )
2026-06-14 10:47:11 -04:00
Expect ( err ) . ToNot ( HaveOccurred ( ) )
sql , args , err := sqlizer . ToSql ( )
Expect ( err ) . ToNot ( HaveOccurred ( ) )
Expect ( strings . Count ( sql , "not exists" ) ) . To ( Equal ( 1 ) )
Expect ( sql ) . To ( ContainSubstring ( "artist.name LIKE ? OR artist.name LIKE ?" ) )
Expect ( args ) . To ( HaveExactElements ( "artist" , "%Beatles%" , "%Kraftwerk%" ) )
} )
It ( "merges negated tag conditions in an AND group into a single NOT EXISTS" , func ( ) {
expr := criteria . All {
criteria . NotContains { "genre" : "Rock" } ,
criteria . NotContains { "genre" : "Metal" } ,
}
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
sqlizer , err := newSmartPlaylistCriteria ( criteria . Criteria { Expression : expr } ) . where ( )
2026-06-14 10:47:11 -04:00
Expect ( err ) . ToNot ( HaveOccurred ( ) )
sql , args , err := sqlizer . ToSql ( )
Expect ( err ) . ToNot ( HaveOccurred ( ) )
Expect ( strings . Count ( sql , "not exists" ) ) . To ( Equal ( 1 ) )
Expect ( sql ) . To ( ContainSubstring ( "value LIKE ? OR value LIKE ?" ) )
Expect ( args ) . To ( HaveExactElements ( "%Rock%" , "%Metal%" ) )
} )
It ( "does not merge a single negated condition with a positive one of the same role in AND" , func ( ) {
// AND of mixed polarity must not be collapsed: NOT EXISTS(a) AND EXISTS(b)
// is not equivalent to any single merged subquery.
expr := criteria . All {
criteria . Contains { "artist" : "Beatles" } ,
criteria . IsNot { "artist" : "Kraftwerk" } ,
}
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
sqlizer , err := newSmartPlaylistCriteria ( criteria . Criteria { Expression : expr } ) . where ( )
2026-06-14 10:47:11 -04:00
Expect ( err ) . ToNot ( HaveOccurred ( ) )
sql , _ , err := sqlizer . ToSql ( )
Expect ( err ) . ToNot ( HaveOccurred ( ) )
// One positive EXISTS and one negated NOT EXISTS, kept separate
Expect ( strings . Count ( sql , "not exists" ) ) . To ( Equal ( 1 ) )
Expect ( strings . Count ( sql , "exists" ) ) . To ( Equal ( 2 ) ) // "not exists" contains "exists"
} )
It ( "does not merge negated conditions of different roles in AND" , func ( ) {
expr := criteria . All {
criteria . IsNot { "artist" : "Beatles" } ,
criteria . IsNot { "composer" : "Lennon" } ,
}
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
sqlizer , err := newSmartPlaylistCriteria ( criteria . Criteria { Expression : expr } ) . where ( )
2026-06-14 10:47:11 -04:00
Expect ( err ) . ToNot ( HaveOccurred ( ) )
sql , _ , err := sqlizer . ToSql ( )
Expect ( err ) . ToNot ( HaveOccurred ( ) )
Expect ( strings . Count ( sql , "not exists" ) ) . To ( Equal ( 2 ) )
} )
It ( "batches large negated AND groups to avoid SQLite expression tree depth limit" , func ( ) {
allExprs := make ( criteria . All , jsonCondBatchSize + 1 )
for i := range allExprs {
allExprs [ i ] = criteria . IsNot { "artist" : fmt . Sprintf ( "Artist%d" , i ) }
}
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
sqlizer , err := newSmartPlaylistCriteria ( criteria . Criteria { Expression : allExprs } ) . where ( )
2026-06-14 10:47:11 -04:00
Expect ( err ) . ToNot ( HaveOccurred ( ) )
sql , args , err := sqlizer . ToSql ( )
Expect ( err ) . ToNot ( HaveOccurred ( ) )
// Two NOT EXISTS subqueries (one batch of jsonCondBatchSize, one of 1)
Expect ( strings . Count ( sql , "not exists" ) ) . To ( Equal ( 2 ) )
Expect ( args ) . To ( HaveLen ( 2 + jsonCondBatchSize + 1 ) )
} )
2026-05-22 18:00:13 -03:00
} )
2026-04-24 23:18:20 -04:00
Describe ( "joins" , func ( ) {
It ( "excludes sort-only joins from expression joins" , func ( ) {
c := criteria . Criteria { Expression : criteria . All { criteria . Contains { "title" : "love" } } , Sort : "albumRating" }
cSQL := newSmartPlaylistCriteria ( c )
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
Expect ( cSQL . expressionJoins ( ) ) . To ( Equal ( smartPlaylistJoinNone ) )
Expect ( cSQL . requiredJoins ( ) . has ( smartPlaylistJoinAlbumAnnotation ) ) . To ( BeTrue ( ) )
2026-04-24 23:18:20 -04:00
} )
It ( "includes expression-based joins" , func ( ) {
c := criteria . Criteria { Expression : criteria . All { criteria . Gt { "albumRating" : 3 } } }
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
Expect ( newSmartPlaylistCriteria ( c ) . expressionJoins ( ) . has ( smartPlaylistJoinAlbumAnnotation ) ) . To ( BeTrue ( ) )
2026-04-24 23:18:20 -04:00
} )
It ( "detects nested album and artist joins" , func ( ) {
c := criteria . Criteria { Expression : criteria . All {
criteria . Any { criteria . All { criteria . Is { "albumLoved" : true } } } ,
criteria . Any { criteria . Gt { "artistPlayCount" : 10 } } ,
} }
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
joins := newSmartPlaylistCriteria ( c ) . requiredJoins ( )
2026-04-24 23:18:20 -04:00
Expect ( joins . has ( smartPlaylistJoinAlbumAnnotation ) ) . To ( BeTrue ( ) )
Expect ( joins . has ( smartPlaylistJoinArtistAnnotation ) ) . To ( BeTrue ( ) )
} )
It ( "detects join types from sort fields with direction prefixes" , func ( ) {
c := criteria . Criteria { Expression : criteria . All { criteria . Contains { "title" : "love" } } , Sort : "-artistRating" }
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
Expect ( newSmartPlaylistCriteria ( c ) . requiredJoins ( ) . has ( smartPlaylistJoinArtistAnnotation ) ) . To ( BeTrue ( ) )
2026-04-24 23:18:20 -04:00
} )
feat(smartplaylist): add album-level fields for sorting and filtering (#5899)
Smart playlists could only sort by the track-level `dateadded`, which scatters
an album's tracks because every track has its own timestamp. There was no way
to express "newest albums first, tracks in album order".
Adds five fields backed by the album table: albumdateadded, albumdatemodified,
albumduration, albumsongcount and albumsize, so `"sort": "-albumdateadded,
tracknumber"` now works. They filter as well as sort, which also enables rules
like "everything from albums added this month" or "skip singles and EPs".
These need the album table rather than the album_annotation table the existing
album* fields join, so they get their own bit in the join mask. The bitmask
already unions joins from both the expression and the sort fields, so a
sort-only reference pulls in the join for the main query while correctly
staying out of the percentage-limit count query.
No COALESCE default is used. That mechanism exists because an album_annotation
row is genuinely often absent; the album row always exists and song_count,
duration and size are NOT NULL, so leaving the columns bare keeps filters
index-friendly.
albumdatemodified follows the existing track-level `datemodified` naming rather
than the `albumdateupdated` spelling used in the request.
albumreleasedate was considered and rejected: album.release_date is
allOrNothing() over the tracks, so it equals the track-level releasedate when
they agree and collapses to empty when they disagree.
Closes https://github.com/navidrome/navidrome/discussions/5347
2026-08-06 10:59:50 -04:00
It ( "keeps a sort-only album join out of the expression joins" , func ( ) {
c := criteria . Criteria { Expression : criteria . All { criteria . Contains { "title" : "love" } } , Sort : "-albumDateAdded" }
cSQL := newSmartPlaylistCriteria ( c )
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
Expect ( cSQL . expressionJoins ( ) ) . To ( Equal ( smartPlaylistJoinNone ) )
Expect ( cSQL . requiredJoins ( ) . has ( smartPlaylistJoinAlbum ) ) . To ( BeTrue ( ) )
feat(smartplaylist): add album-level fields for sorting and filtering (#5899)
Smart playlists could only sort by the track-level `dateadded`, which scatters
an album's tracks because every track has its own timestamp. There was no way
to express "newest albums first, tracks in album order".
Adds five fields backed by the album table: albumdateadded, albumdatemodified,
albumduration, albumsongcount and albumsize, so `"sort": "-albumdateadded,
tracknumber"` now works. They filter as well as sort, which also enables rules
like "everything from albums added this month" or "skip singles and EPs".
These need the album table rather than the album_annotation table the existing
album* fields join, so they get their own bit in the join mask. The bitmask
already unions joins from both the expression and the sort fields, so a
sort-only reference pulls in the join for the main query while correctly
staying out of the percentage-limit count query.
No COALESCE default is used. That mechanism exists because an album_annotation
row is genuinely often absent; the album row always exists and song_count,
duration and size are NOT NULL, so leaving the columns bare keeps filters
index-friendly.
albumdatemodified follows the existing track-level `datemodified` naming rather
than the `albumdateupdated` spelling used in the request.
albumreleasedate was considered and rejected: album.release_date is
allOrNothing() over the tracks, so it equals the track-level releasedate when
they agree and collapses to empty when they disagree.
Closes https://github.com/navidrome/navidrome/discussions/5347
2026-08-06 10:59:50 -04:00
} )
It ( "distinguishes the album join from the album annotation join" , func ( ) {
c := criteria . Criteria { Expression : criteria . All { criteria . Gt { "albumRating" : 3 } } }
feat(scrobbler): add per-user scrobble filter (#5964)
* feat(scrobbler): add scrobble_filter column to user
* feat(scrobbler): validate scrobble filter criteria on user save
* refactor(persistence): make smart playlist join helpers package-level
* feat(scrobbler): add MediaFileRepository.MatchesCriteria
* feat(scrobbler): filter external scrobbles with per-user criteria
* feat(ui): add scrobble filter field to user form
* fix(scrobbler): default scrobble_filter to empty string for existing users
* refactor(scrobbler): also gate playback reports on the scrobble filter
Playback reports carry the same track metadata to plugin scrobblers, so a
filtered track leaked through that third dispatch path. Skip the filter
evaluation entirely when no scrobbler is active.
* refactor(persistence): move criteria join building into criteria_sql.go
The join set a criteria needs was decided in criteria_sql.go but built in
smart_playlist_repository.go, so both callers had to pair the two by hand.
* refactor(persistence): unexport smartPlaylistCriteria methods
The type never leaves the package, so the exported names advertised an API
that callers outside persistence could never reach. Also disambiguates
where/orderBy from squirrel's SelectBuilder methods of the same name.
* fix(ui): cap the scrobble filter field width
fullWidth stretched it across the whole page next to 256px inputs. Bounded
at 40em, with two rows and a resize handle so JSON rules stay readable.
* refactor(ui): move scrobble filter input in UserEdit component
* feat(ui): add pt-BR translations for the scrobble filter
* fix(scrobbler): take the filter verdict before incPlay
incPlay mutates play counts and dates a filter can test on, so evaluating at
dispatch time let one play decide differently on either side of the increment:
a track could be scrobbled despite matching, or lose only its stopped report
and strand presence plugins. Reject limit/offset too, rather than silently
ignoring part of a rule copied from a smart playlist.
* fix(scrobbler): filter the report from an expired session
The expiry callback runs with a stub user carrying no filter, so evaluating
there always returned false and leaked the track to plugin scrobblers. That is
the normal path for clients that never send stopped, such as legacy Subsonic
now-playing. Carry the last verdict on the session instead.
* refactor(scrobbler): skip the now-playing enqueue instead of threading the verdict
Queuing an entry only to drop it at dispatch also cancelled a pending
announcement for the previous, unfiltered track, since the queue is keyed by
player and a new entry replaces the old one.
* fix(scrobbler): evaluate the filter regardless of active scrobblers
The verdict is stored on the session and dispatched at expiry, so skipping
evaluation when no scrobbler was active let a plugin enabled mid-session
receive a filtered track. The empty-filter guard above already gives servers
without scrobbling the same free path, so the shortcut only ever applied to
users who had a filter set.
2026-08-15 16:10:53 -04:00
joins := newSmartPlaylistCriteria ( c ) . requiredJoins ( )
feat(smartplaylist): add album-level fields for sorting and filtering (#5899)
Smart playlists could only sort by the track-level `dateadded`, which scatters
an album's tracks because every track has its own timestamp. There was no way
to express "newest albums first, tracks in album order".
Adds five fields backed by the album table: albumdateadded, albumdatemodified,
albumduration, albumsongcount and albumsize, so `"sort": "-albumdateadded,
tracknumber"` now works. They filter as well as sort, which also enables rules
like "everything from albums added this month" or "skip singles and EPs".
These need the album table rather than the album_annotation table the existing
album* fields join, so they get their own bit in the join mask. The bitmask
already unions joins from both the expression and the sort fields, so a
sort-only reference pulls in the join for the main query while correctly
staying out of the percentage-limit count query.
No COALESCE default is used. That mechanism exists because an album_annotation
row is genuinely often absent; the album row always exists and song_count,
duration and size are NOT NULL, so leaving the columns bare keeps filters
index-friendly.
albumdatemodified follows the existing track-level `datemodified` naming rather
than the `albumdateupdated` spelling used in the request.
albumreleasedate was considered and rejected: album.release_date is
allOrNothing() over the tracks, so it equals the track-level releasedate when
they agree and collapses to empty when they disagree.
Closes https://github.com/navidrome/navidrome/discussions/5347
2026-08-06 10:59:50 -04:00
Expect ( joins . has ( smartPlaylistJoinAlbumAnnotation ) ) . To ( BeTrue ( ) )
Expect ( joins . has ( smartPlaylistJoinAlbum ) ) . To ( BeFalse ( ) )
} )
2026-04-24 23:18:20 -04:00
} )
} )