mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-12 20:37:12 +02:00
4 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Deluan Quintão
|
b293b96256 |
refactor(persistence): stateless repositories with per-call context (#6149)
* refactor(persistence): adopt generic deluan/rest repository API Pin deluan/rest to the refactor branch. REST-facing repository methods take a context and return typed values. Drop DataStore.Resource and ResourceRepository; the native API names typed repositories directly through a per-request adapter that later commits remove. * refactor(persistence): base repository helpers take a context * refactor(persistence): LibraryRepository takes a context per call * refactor(persistence): PropertyRepository takes a context per call * refactor(persistence): UserPropsRepository takes a context per call * refactor(persistence): TranscodingRepository takes a context per call * refactor(persistence): ShareRepository takes a context per call * refactor(persistence): PlayerRepository takes a context per call * refactor(persistence): RadioRepository takes a context per call * refactor(persistence): PlayQueueRepository takes a context per call * refactor(persistence): Tag and Genre repositories take a context per call * refactor(persistence): PluginRepository takes a context per call * refactor(persistence): Scrobble repositories take a context per call * refactor(persistence): FolderRepository takes a context per call * refactor(persistence): Artwork repositories take a context per call * refactor(persistence): UserRepository takes a context per call * refactor(persistence): ArtistRepository takes a context per call ReadAll no longer rewrites the shared sort mappings for the role filter; it works on a per-call copy. * test(persistence): assert artist role sort sanitization in ReadAll * refactor(persistence): AlbumRepository takes a context per call * test(persistence): pass the test context to album repository helpers * refactor(persistence): MediaFileRepository takes a context per call * refactor(persistence): Playlist repositories take a context per call * refactor(persistence): build all repositories once per store * refactor(core): REST repository wrappers are built once * refactor(persistence): repositories are stateless Remove the context field from the base repository and the per-request REST adapter. Enable the containedctx linter so no repository can hold a request context again. * chore(lint): skip containedctx in test files * refactor: share simplifications from the stateless repositories sweep Add deleteOwnedAll on sqlRepository and use it in player/share Delete to remove the duplicated bulk-delete loop; have Share.Repository() return model.ShareRepository so subsonic sharing.go drops its repeated type assertions. * chore(core): assert REST wrappers implement Persistable * chore: reformat imports * perf(persistence): build repositories on first use Each transaction store used to construct all 21 repositories up front, paying for filter and sort mapping setup the block never touched. Fields are now sync.OnceValue thunks, so a store only builds what it uses. * fix(persistence): clean plugin references per deleted user A bulk user delete that fails on a later id had already removed the earlier rows but skipped their plugin cleanup. Cleanup now runs right after each successful delete. * fix(core): unload disabled plugins even when a user delete fails A bulk delete can fail on a later id after earlier users were removed and their plugins auto-disabled. The wrapper returned before unloading, leaving those plugins running until the next successful delete or a restart. * chore(deps): pin deluan/rest to v1.0.1 Replaces the pseudo-version of the refactor branch with the tagged release. REST error messages now name the bare type (Artist, not model.Artist). * test: use the spec context instead of context.Background() Replace the context.Background()/context.TODO() calls this branch added to tests with the spec's ctx, GinkgoT().Context(), or t/b.Context(), so repository calls are bound to the running spec's lifetime. * test: declare the spec context once per Describe Set ctx from GinkgoT().Context() first in each top-level BeforeEach and reuse it, building user contexts on top of it instead of repeating inline calls. |
||
|
Deluan Quintão
|
82fde00ecc |
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. |
||
|
Deluan Quintão
|
f3887df334 |
perf(smartplaylists): merge negated artist/tag rules into one NOT EXISTS
* fix(smartplaylists): merge negated artist/tag rules in AND groups Smart playlists with many negated role/tag conditions ANDed together (e.g. 100+ "isNot artist" rules, issue #5511) generated one correlated NOT EXISTS subquery per rule, scanning media_file_artists for every candidate row. On large libraries this took minutes and triggered API timeouts and SQLite lock contention. By De Morgan, "NOT EXISTS(role=X) AND NOT EXISTS(role=Y)" is equivalent to "NOT EXISTS(role=X OR role=Y)", so multiple negated conditions for the same field can be collapsed into a single batched NOT EXISTS. This mirrors the existing OR-group merge that #5515 added for positive conditions. The shared grouping/batching logic is extracted into mergeSameFieldConds, parameterized by polarity, so the OR/positive and AND/negated paths reuse one algorithm instead of duplicating it. roleCondGroup/tagCondGroup gain a 'not' flag to emit the negated subquery. Benchmark (323k tracks, 120 isNot artist rules, reporter's exact shape): merged ~54ms vs unmerged ~8.7s steady-state (~160x faster). * docs: trim redundant comments on merge helpers The De Morgan explanation was repeated across three doc comments. Keep it in one place (mergeNegatedJsonConds, where negation is introduced) and reduce the shared core and group-type comments to concise one-liners. |
||
|
Deluan Quintão
|
74185dc6d1 |
fix(smartplaylists): optimize smart playlist performance for role and tag criteria (#5515)
* fix(server): optimize smart playlist role queries for large criteria (#5511) Role-based smart playlist criteria (artist, composer, etc.) now query the indexed media_file_artists join table instead of parsing JSON via json_tree() on every row. Multiple conditions for the same role within an OR group are merged into a single EXISTS subquery (batched at 200 to stay under SQLite's expression tree depth limit). A composite index (media_file_id, role) replaces the now-redundant single-column (media_file_id) index on media_file_artists. Benchmark (40k tracks, 500 patterns, 3 artists/track): - Merged join-table: 15ms (9.3x faster) - Merged json_tree: 30ms (4.6x faster) - Unmerged baseline: 137ms * refactor: simplify role condition SQL generation and benchmark Extract shared roleCondSQL/roleExistsSQL helpers to deduplicate the EXISTS template between roleCond and roleCondGroup. Use slices.Chunk for batching per project convention. Extract runBenchQuery helper to eliminate triplicated benchmark execution loop. * chore: raise roleCondBatchSize to 350 The empirical SQLite limit is 496 conditions per merged EXISTS subquery. Raising from 200 to 350 reduces the number of batches (e.g. 500 patterns now splits into 2 batches instead of 3). * fix(server): apply OR-merge optimization to tag conditions too Generalize mergeRoleConds into mergeJsonConds to also collapse multiple tag conditions for the same tag (e.g. genre) within OR groups. This gives the same ~5x speedup for tag-heavy smart playlists as the role optimization gives for artist-heavy ones. * refactor: benchmark uses real criteria pipeline instead of hand-built SQL The "Current" sub-benchmark now builds criteria.Criteria expressions and runs them through the actual newSmartPlaylistCriteria → Where() → ToSql() pipeline, validating the real production code path. The baseline still uses hand-built SQL representing the old json_tree approach. * fix: stabilize merged group ordering and close rows before error check Sort group keys in mergeJsonConds so the merged additions have deterministic order across runs, improving SQLite statement cache reuse. Move rows.Close() before rows.Err() in benchmark helper. |