Navidrome (Beta): implement Navidrome media browser service with custom playlists and overview tab - #155
Navidrome (Beta): implement Navidrome media browser service with custom playlists and overview tab #155lxBlazarxl wants to merge 34 commits into
Conversation
…and export providers
…ces, and prevent duplication
…th 7 album filter categories
…ed music icons and dynamic theme colors
… section badges, and list/grid views
…rtists and albums
… favorites toggle
…rating and star changes
…bpaths for artwork and web ui
…, and subsonic endpoints
The lock was written on a newer SDK than the 3.41.9 the workflows pin, so it asked for meta 1.18.0, test 1.31.0, test_api 0.7.11 and test_core 0.6.17, none of which 3.41.9 can supply. pub wants to downgrade all four, so flutter pub get --enforce-lockfile refused and the iOS build failed in its first step, before it compiled a line of the new service. Regenerated on 3.41.9. The only change that survives is the workspace entry for service_navidrome, which is the one this PR actually needs.
Navidrome answers 200 for everything. A healthy server, a rejected password and a request missing its credentials are all 200 with JSON; only the status inside the subsonic-response object tells them apart. The kind was registered as reachable, which treats any answer as healthy, so the dashboard dot was green whatever the credentials were. The probe was not even sending credentials. AuthInterceptor had no arm for this kind, so the user/password fell through to the catch-all that sends nothing, and the shared probe and connection tester both went out anonymous. The service module signed its own calls, which is why the screens worked while the dot and Test connection did not. So both halves: the interceptor now signs Subsonic requests the way the spec asks, with the username, a fresh salt and an MD5 of the password with that salt, and the health arm requires the envelope to actually say ok. Same shape as the Unraid arm above it, and for the same reason. Verified against Navidrome 0.64.0: a correct password is status ok, a wrong one is status failed code 40, and no credentials at all is code 10, all three at HTTP 200. The bodies in the test are those responses. The salt is Random.secure rather than Random. It travels in the clear next to the hash it salts, so a predictable one lets anyone who captures a request precompute against it.
Two things, both in the client provider. It built its Dio straight off the factory with nothing to close it, and it watches the image epoch, so it rebuilds on every hard refresh. Each pull to refresh therefore created a fresh Dio and abandoned the previous one with its connection pool still open. instanceDioProvider, which the other twenty-odd services use, closes on dispose. And every read method parses the envelope optimistically, returning an empty list when the key it wants is absent. That is right for an empty library and wrong for a rejected credential, so a wrong password rendered as an empty result on all four tabs with nothing logged and no error shown. Confirmed on a device against a real server: green dot, Connected on both URLs, and four empty tabs. SubsonicErrorInterceptor turns a failed envelope into a DioException, so the existing error handling has something to show. One interceptor rather than a check in each of the twenty-two methods, which also keeps it out of the parsing code. The guard before adding it matters because the Dio is shared. Also restores the test suite to version control. It was added to .gitignore, so CI never ran it and no reviewer could see it.
The music artwork resolution added to jellyfin_client.dart is a line-for-line copy of the one in emby_client.dart: substituting the service name makes the two diffs identical. Only the Emby path had a test, and because the logic is duplicated rather than shared, that test protects none of the Jellyfin copy. This covers the other half. The duplication itself is worth removing, but picking a home for shared Emby and Jellyfin code is a wider decision than a test belongs in.
|
Went through this properly and tested it against a real Navidrome (0.64.0) on a device. The Navidrome work itself is good: zero analyze issues in the package, correct use of I pushed four commits into the branch rather than leaving a list of comments, since three of them were blocking. CI was red before any of the review. The committed The health dot and Test connection were both meaningless for Navidrome. Subsonic answers 200 for everything, and the kind was registered as A wrong password looked like an empty library. Every read method returns an empty list when the key it wants is absent, which is right for an empty library and wrong for a rejected credential. Nothing was logged and no error was shown anywhere. Added a The client Dio was never closed. Also switched the auth salt to Three things I left for you rather than deciding inside your PR: The music artwork resolution in
Smaller: One process note: the Emby and Jellyfin changes alter the Activity tab for every existing media server user, in a PR titled "Navidrome (Beta)". Worth calling out in the release notes so it is not a surprise if something regresses. Verification on my side: analyze is at the repo's existing 60 issues with none added, 139 app tests, 66 in core_networking and 6 new in service_navidrome all pass, and I ran it behind an nginx basic-auth proxy with a global |
Summary
Note
Navidrome Beta Status: The Navidrome service is currently in active Beta. Core browsing, metadata exploration, and custom playlist management are fully functional. In-app media playback functionality is currently in development and will be added soon in an upcoming release.
This PR introduces two major feature sets:
Changes
1. Navidrome Service (Beta)
[Albums, Artists, Playlists, Overview].getTopSongs,getSimilarSongs,getSimilarSongs2, andgetNowPlaying.core_uiEasyRefreshacross all four tabs to cascade cache invalidation to deep views (albums/artists).2. Dashdot Dashboard Widget
DashdotRingMetricscomponent with animated circular progress indicators and dynamic M3 color tokens.Upcoming
Verification
flutter analyzeverified.