-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Multi-library support #4181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Multi-library support #4181
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @deluan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
Hello team,
Gemini here to provide a summary of this pull request titled "Multi-library support".
Based on the title and the code changes, the primary goal of this PR is to introduce support for multiple music libraries within the application. This includes the ability to define different library locations and, crucially, to control which users have access to which libraries. This is a significant architectural change, touching the database schema, core business logic, persistence layer, and API endpoints.
The changes involve creating a new database table to manage the many-to-many relationship between users and libraries, implementing core services and repository methods for managing libraries and user access, updating the persistence layer to filter content based on user permissions, and adding new API endpoints for administrative management of libraries and user assignments. Comprehensive tests have been added to cover the new functionality.
Highlights
- Multi-Library Core Logic: Introduces a new
core.Library
service (core/library.go
) responsible for library CRUD operations (GetAll, Get, Create, Update, Delete) and managing user-library associations (GetUserLibraries, SetUserLibraries, ValidateLibraryAccess). This service includes validation logic for library creation/update and user assignments. - User-Based Access Control: Adds helper functions in
core/auth/auth.go
(HasLibraryAccess
,GetUserLibraryIDs
) to check user permissions based on library assignments. Admin users automatically have access to all libraries, while regular users' access is explicitly managed. - Database Schema Update: Adds a new database migration (
db/migrations/20250106113337_add_user_library_table.go
) to create auser_library
join table. This table links users and libraries to define access permissions. The migration also populates the table, giving all existing users access to the default library (ID 1). - Persistence Layer Filtering: Implements a generic
applyLibraryFilter
method inpersistence/sql_base_repository.go
that adds aWHERE
clause to queries for tables with alibrary_id
column. This filter restricts results based on the logged-in user's library access, ensuring users only see content from allowed libraries. This filter is applied toalbum
,folder
, andmedia_file
repositories. - User Repository Enhancements: Updates the
model.UserRepository
interface andpersistence/user_repository.go
to include methods for managing user-library associations (GetUserLibraries
,SetUserLibraries
, etc.). User retrieval methods (Get
,GetAll
,FindByUsername
,FindFirstAdmin
) are modified to join theuser_library
andlibrary
tables and populate theLibraries
field on themodel.User
struct using JSON aggregation. - Native API Endpoints: Adds new Native API endpoints in
server/nativeapi/library.go
for managing libraries (GET, POST, PUT, DELETE/api/library
) and user-library associations (GET, PUT/api/user/{id}/library
). These endpoints are protected by a newadminOnlyMiddleware
(server/nativeapi/native_api.go
), ensuring only administrators can manage libraries and user permissions. - Comprehensive Testing: Includes extensive new tests (
core/library_test.go
,persistence/user_repository_test.go
,server/nativeapi/library_test.go
) covering the new core library service, user repository library methods (including admin auto-assignment and library field population), and the new Native API endpoints, verifying functionality and access control.
Changelog
Click here to see the changelog
- cmd/wire_gen.go
- Added
core.NewLibrary
to the dependency injection setup. - Injected the new
library
service into thenativeapi.New
constructor.
- Added
- core/auth/auth.go
- Added
HasLibraryAccess
function to check if a user has access to a specific library. - Added
GetUserLibraryIDs
function to retrieve a list of library IDs a user has access to. - Both functions handle admin users having access to all libraries.
- Added
- core/library.go
- New file defining the
Library
interface andlibraryService
implementation. - Includes CRUD methods for
model.Library
. - Includes methods for user-library associations:
GetUserLibraries
,SetUserLibraries
,ValidateLibraryAccess
. - Adds validation logic for library name/path and user library assignments (non-admins need at least one, cannot assign to admins, validate library IDs).
- New file defining the
- core/library_test.go
- New file containing tests for the
core.Library
service. - Covers Library CRUD operations (GetAll, Get, Create, Update, Delete).
- Tests User-Library Association operations (GetUserLibraries, SetUserLibraries, ValidateLibraryAccess).
- Includes tests for validation rules and admin/regular user behavior.
- New file containing tests for the
- core/wire_providers.go
- Added
NewLibrary
to thewire.NewSet
for dependency injection.
- Added
- db/migrations/20250106113337_add_user_library_table.go
- New database migration file.
- Creates the
user_library
join table with foreign keys and indexes. - Populates the
user_library
table, giving all existing users access to library ID 1.
- model/errors.go
- Added a new error variable
ErrValidation
.
- Added a new error variable
- model/library.go
- Updated
LibraryRepository
interface to includeCountAll
,Delete
, andGetUsersWithLibraryAccess
methods.
- Updated
- model/user.go
- Added
Libraries
field to theUser
struct for storing associated libraries. - Updated
UserRepository
interface to include methods for user-library associations:GetUserLibraries
,SetUserLibraries
,AddUserLibrary
,RemoveUserLibrary
.
- Added
- persistence/album_repository.go
- Applied the new
applyLibraryFilter
to theselectAlbum
query builder (line 220).
- Applied the new
- persistence/folder_repository.go
- Applied the new
applyLibraryFilter
to theselectFolder
query builder (line 66).
- Applied the new
- persistence/library_repository.go
- Implemented
Delete
method for libraries, including admin check and cache invalidation. - Implemented
CountAll
method for libraries. - Implemented
GetUsersWithLibraryAccess
method. - Added logic in
Put
to automatically assign newly created/updated libraries to all admin users (lines 92-101).
- Implemented
- persistence/mediafile_repository.go
- Applied the new
applyLibraryFilter
to theselectMediaFile
query builder (line 131).
- Applied the new
- persistence/sql_base_repository.go
- Added
applyLibraryFilter
method to filter queries based on user library access (lines 198-219). - This filter uses a subquery on
user_library
for regular users and allows all for admins.
- Added
- persistence/user_repository.go
- Implemented
GetUserLibraries
,SetUserLibraries
,AddUserLibrary
, andRemoveUserLibrary
methods for user-library associations. - Added
dbUser
struct andPostScan
method to handle unmarshalling JSON library data. - Modified
Get
,GetAll
,FindFirstAdmin
, andFindByUsername
to useselectUserWithLibraries
(lines 69-85) to populate theLibraries
field on themodel.User
struct. - Added logic in
Put
to automatically assign all existing libraries to admin users upon creation or update (lines 141-150).
- Implemented
- persistence/user_repository_test.go
- Added extensive tests for the user repository's library association methods.
- Includes tests for
AddUserLibrary
,GetUserLibraries
,SetUserLibraries
,RemoveUserLibrary
. - Tests the automatic assignment of libraries to admin users.
- Tests the population of the
Libraries
field when retrieving users.
- scanner/scanner_test.go
- Added creation of an admin user in the test setup (lines 74-82), likely to ensure scanner tests run with a valid user context for the new filtering logic.
- server/nativeapi/config.go
- Removed the explicit admin check from the
getConfig
handler (lines 102-106), now handled by middleware.
- Removed the explicit admin check from the
- server/nativeapi/config_test.go
- Updated config API tests to use authenticated request helpers.
- Tests now verify access control via the router's middleware.
- server/nativeapi/inspect.go
- Removed the explicit admin check from the
inspect
handler (lines 33-36), now handled by middleware.
- Removed the explicit admin check from the
- server/nativeapi/library.go
- New file defining Native API handlers for library management and user-library associations.
- Includes middleware for parsing library and user IDs from URLs.
- Implements handlers for GET all libraries, GET library by ID, CREATE library, UPDATE library, DELETE library.
- Implements handlers for GET user libraries and SET user libraries.
- server/nativeapi/library_test.go
- New file containing tests for the Native API library and user-library association endpoints.
- Verifies access control (admin-only) for all new endpoints.
- Tests the functionality of the library CRUD handlers.
- Tests the functionality and validation of the user-library association handlers.
- server/nativeapi/native_api.go
- Modified
New
function to accept the newcore.Library
service. - Added
adminOnlyMiddleware
(lines 228-238) to restrict access to certain routes. - Grouped
/inspect
,/config
,/library
, and/user/{id}/library
routes under theadminOnlyMiddleware
.
- Modified
- server/nativeapi/native_api_song_test.go
- Removed unused mock implementations for
Share
,Playlists
, andInsights
. - Updated the
New
call to passnil
for the services and the newcore.Library
service.
- Removed unused mock implementations for
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces significant and well-architected support for multiple libraries, a major enhancement to Navidrome. The changes span the data model, persistence layer, core services, and API, enabling fine-grained control over library creation, management, and user access.
Overall, the implementation is robust, with good attention to detail in database migrations, data filtering, and API design. The new core.Library
service provides a clear abstraction for library-related business logic, and the persistence layer changes are thoughtfully implemented, including automatic library assignments for admin users and efficient data retrieval. The addition of comprehensive unit and API tests is commendable and greatly contributes to the quality of these changes.
I've identified a couple of areas related to cache updates in the library_repository.go
that could lead to inconsistencies. Addressing these will further solidify the reliability of the new features.
Style Guide:
As no specific style guide was provided, this review adheres to common Go programming idioms and best practices, such as those outlined in "Effective Go" and common Go code review guidelines.
Key Changes & Observations:
- Multi-Library Data Model:
- New
user_library
association table correctly models the many-to-many relationship between users and libraries. model.User
now includes aLibraries
field to carry associated library information.
- New
- Core Library Service:
core.Library
service introduced for managing library CRUD and user-library associations.- Includes validation for library properties and user assignments (e.g., non-admin users must have at least one library).
- Persistence Layer:
applyLibraryFilter
insql_base_repository.go
effectively restricts data visibility based on user library access.- Admin users are automatically granted access to all libraries upon user creation/promotion or library creation.
- User repository now fetches and includes associated library details using SQLite's JSON functions.
- API Endpoints:
- New API routes under
/api/library
and/api/user/{id}/library
for managing libraries and their associations. - Admin-only access for these new routes is enforced via
adminOnlyMiddleware
. For example, the/api/config
and/api/inspect
endpoints are now correctly placed behind this middleware.
- New API routes under
- Testing:
- Extensive tests for the new
core.Library
service and the new API endpoints. - Existing tests updated to reflect authentication changes and new dependencies.
- Extensive tests for the new
Excellent work on this complex feature!
Summary of Findings
- Cache Update Logic in
library_repository.go
: ThelibCache
inpersistence/library_repository.go
appears to be updated only when database operations withinPut
andStoreMusicFolder
methods fail. This could lead to cache inconsistency. The cache should ideally be updated upon successful database operations.
Merge Readiness
This pull request introduces a significant and valuable feature. The implementation is largely excellent. However, there are a couple of medium-severity issues related to cache update logic in persistence/library_repository.go
that could lead to inconsistencies. I recommend addressing these issues to ensure the cache remains consistent with the database state. Once these are resolved, the PR should be in good shape for merging. As an AI, I am not authorized to approve pull requests; please ensure further review and approval by authorized team members.
4591c9c
to
4353ab7
Compare
6ea26ff
to
1f64cc5
Compare
0c92181
to
5ea1a43
Compare
6e13de6
to
7a6abfd
Compare
Refactored the cache buffering logic to ensure thread safety when checking the buffer length Signed-off-by: Deluan <deluan@navidrome.org>
Signed-off-by: Deluan <deluan@navidrome.org>
Implemented comprehensive multi-library support for artist statistics that automatically aggregates stats from user-accessible libraries. This fundamental change moves artist statistics from global scope to per-library granularity while maintaining backward compatibility and transparent operation. Key changes include: - Migrated artist statistics from global artist.stats to per-library library_artist.stats - Added automatic library filtering and aggregation in existing Get/GetAll methods - Updated role-based filtering to work with per-library statistics storage - Enhanced statistics calculation to process and store stats per library - Implemented user permission-aware aggregation that respects library access control - Added comprehensive test coverage for library filtering and restricted user access - Created helper functions to ensure proper library associations in tests This enables users to see statistics that accurately reflect only the content from libraries they have access to, providing proper multi-tenant behavior while maintaining the existing API surface and UI functionality. Signed-off-by: Deluan <deluan@navidrome.org>
Signed-off-by: Deluan <deluan@navidrome.org>
Signed-off-by: Deluan <deluan@navidrome.org>
Implemented comprehensive library filtering for tag repositories to support the multi-library feature. This change ensures that users only see tags from libraries they have access to, while admin users can see all tags. Key changes: - Enhanced TagRepository.Add() method to accept libraryID parameter for proper library association - Updated baseTagRepository to implement library-aware queries with proper joins - Added library_tag table integration for per-library tag statistics - Implemented user permission-based filtering through user_library associations - Added comprehensive test coverage for library filtering scenarios - Updated UI data provider to include tag filtering by selected libraries - Modified scanner to pass library ID when adding tags during folder processing The implementation maintains backward compatibility while providing proper isolation between libraries for tag-based operations like genres and other metadata tags.
Removed conditional admin logic from applyLibraryFilterToArtistQuery method and unified the library filtering approach to match the tag repository pattern. The method now always uses the same SQL join structure regardless of user role, with admin access handled automatically through user_library associations. Added artistLibraryIdFilter function to properly qualify library_id column references and prevent SQL ambiguity errors when multiple tables contain library_id columns. This ensures the filter targets library_artist.library_id specifically rather than causing ambiguous column name conflicts.
Fixed validation error 'At least one library must be selected for non-admin users' that appeared even when libraries were selected. The issue was caused by a data format mismatch between backend and frontend. The backend sends user data with libraries as an array of objects, but the LibrarySelectionField component expects libraryIds as an array of IDs. Added data transformation in the data provider's getOne method to automatically convert libraries array to libraryIds format when fetching user records. Also extracted validation logic into a separate userValidation module for better code organization and added comprehensive test coverage to prevent similar issues.
Signed-off-by: Deluan <deluan@navidrome.org>
Signed-off-by: Deluan <deluan@navidrome.org>
Added user context handling to scrobbleBufferRepository.Next method to resolve SQL error 'no such column: library_artist.library_id' when processing scrobble entries in multi-library environments. The artist repository now requires user context for proper library filtering, so we fetch the user and temporarily inject it into the context before calling getParticipants. This ensures background scrobbling operations work correctly with multi-library support.
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This is a substantial and well-architected pull request that introduces multi-library support. The changes are extensive, touching the database schema, core services, persistence layer, APIs, and the UI. The approach to isolate library data seems robust, with careful consideration for database migrations, data isolation, and API compatibility.
The introduction of the core.Library
service provides a clean abstraction for library management, and the use of a repository wrapper to handle business logic in the API layer is a good pattern. The database migration is comprehensive, though the rollback path has some data loss potential, which is a critical point to be aware of.
The changes to the persistence layer to make queries library-aware are consistent and well-implemented, using a subquery-based approach for filtering that should be performant with proper indexing. The updates to the scanner and watcher to handle multiple libraries are also well-designed.
Overall, this is a high-quality contribution that significantly enhances the application's capabilities. My feedback focuses on a few critical areas, potential improvements for code clarity, and robustness.
Implemented cross-library move detection for the scanner phase 2 to properly handle files moved between libraries. This prevents users from losing play counts, ratings, and other metadata when moving files across library boundaries. Changes include: - Added MediaFileRepository methods for two-tier matching: FindRecentFilesByMBZTrackID (primary) and FindRecentFilesByProperties (fallback) - Extended scanner phase 2 pipeline with processCrossLibraryMoves stage that processes files unmatched within their library - Implemented findCrossLibraryMatch with MusicBrainz Release Track ID priority and intrinsic properties fallback - Updated producer logic to handle missing tracks without matches, ensuring cross-library processing - Updated tests to reflect new producer behavior and cross-library functionality The implementation uses existing moveMatched function for unified move operations, automatically preserving all user data through database foreign key relationships. Cross-library moves are detected using the same Equals() and IsEquivalent() matching logic as within-library moves for consistency. Signed-off-by: Deluan <deluan@navidrome.org>
Implemented album annotation reassignment functionality for the scanner's missing tracks phase. When tracks move between libraries and change album IDs, the system now properly reassigns album annotations (starred status, ratings) from the old album to the new album. This prevents loss of user annotations when tracks are moved across library boundaries. The implementation includes: - Thread-safe annotation reassignment using mutex protection - Duplicate reassignment prevention through processed album tracking - Graceful error handling that doesn't fail the entire move operation - Comprehensive test coverage for various scenarios including error conditions This enhancement ensures data integrity and user experience continuity during cross-library media file movements.
Fixed several issues identified in PR review: - Removed unnecessary artist stats initialization check since the map is already initialized in PostScan() - Improved code clarity in user repository by extracting isNewUser variable to avoid checking count == 0 twice - Fixed library selection logic to properly handle initial library state and prevent overriding user selections These changes address code quality and logic issues identified during the multi-library support PR review.
Implemented automatic playlist statistics (duration, size, song count) refreshing when tracks are modified. Added new refreshStats() method to recalculate statistics from playlist tracks, and SetTracks() method to update tracks and refresh statistics atomically. Modified all track manipulation methods (RemoveTracks, AddTracks, AddMediaFiles) to automatically refresh statistics. Updated playlist repository to use the new SetTracks method for consistent statistics handling.
Renamed the AddTracks method to AddMediaFilesByID throughout the codebase to better reflect its purpose of adding media files to a playlist by their IDs. This change improves code readability and makes the method name more descriptive of its actual functionality. Updated all references in playlist model, tests, core playlist logic, and Subsonic API handlers to use the new method name.
Removed duplicate helper functions userId() and isAdmin() from sql_base_repository.go and consolidated all user context access to use loggedUser(r.ctx).ID and loggedUser(r.ctx).IsAdmin consistently across the persistence layer. This change eliminates code duplication and provides a single, consistent pattern for accessing user context information in repository methods. All functionality remains unchanged - this is purely a code cleanup refactoring.
- Replace 235-line MockLibraryService with 40-line embedded struct pattern - Enhance MockLibraryRepo with service-layer methods (192→310 lines) - Maintain full compatibility with existing tests - All 72 nativeapi specs pass with proper error handling Signed-off-by: Deluan <deluan@navidrome.org>
Signed-off-by: Deluan <deluan@navidrome.org>
This PR includes database schema changes that are NOT reversible by simply downgrading to a previous version of Navidrome.
Before installing this version:
navidrome.db
fileMigration Details:
user_library
,library_tag
) and modifies existing tablesIf you need to rollback:
We strongly recommend testing this in a development environment first if you have a large music collection or complex setup.
Description
This PR implements comprehensive multi-library support in Navidrome, enabling users to manage multiple music collections with proper permission controls and complete UI integration. This major architectural enhancement allows organizing music into separate libraries while maintaining proper access controls and data isolation.
Related Issues
Related to multiple user requests and discussions about multi-library support functionality.
Closes #192
Type of Change
What's New
Backend Foundation
user_library
table for managing user access to librariesCore Features
User Interface
API Compatibility
Technical Implementation
Database Changes
user_library
association table with foreign key constraintslibrary
table with duration tracking and default settingslibrary_tag
table for per-library tag statisticsCore Services
core.Library
service for library business logicChecklist
How to Test
Basic Setup
Core Functionality
Migration Testing
Performance Considerations
Breaking Changes
Database Migration Required: Requires database migration that cannot be easily reversed. Users must backup before upgrading.
Additional Notes
This implementation provides solid multi-library foundation while maintaining full backward compatibility. The architecture is extensible for future enhancements and has been tested with various scenarios including single-to-multi library migration, complex permissions, and large library performance.