fix: prevent foreign key constraint errors in album participant insertion #4373
Conversation
Prevent foreign key constraint errors when album participants contain artist IDs that don't exist in the artist table. The updateParticipants method now filters out non-existent artist IDs before attempting to insert album_artists relationships. - Add defensive filtering in updateParticipants() to query existing artist IDs - Only insert relationships for artist IDs that exist in the artist table - Add comprehensive regression test for both albums and media files - Fixes scanner errors when JSON participant data contains stale artist references Signed-off-by: Deluan <deluan@navidrome.org>
Signed-off-by: Deluan <deluan@navidrome.org>
Signed-off-by: Deluan <deluan@navidrome.org>
There was a problem hiding this comment.
Summary of Changes
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!
I've addressed a critical bug that caused foreign key constraint errors during album scanning, particularly for albums spread across multiple folders. The core issue was the attempt to insert album participant (artist) references that did not exist in the database. My solution implements a robust mechanism to automatically filter out these invalid artist IDs during the insertion process, ensuring database integrity and preventing scan failures.
Highlights
- Bug Fix: Foreign Key Errors: I've resolved the issue where album scanning would fail due to foreign key constraint errors when attempting to insert participants (artists) whose IDs did not exist in the database. This specifically addresses the problem described in issue #4002, preventing scan failures for albums spread across multiple folders.
- Participant Insertion Logic Refinement: The participant insertion logic in
sql_participations.gohas been re-engineered. Instead of iterating and inserting each participant, I now marshal the entire participant structure into JSON and use SQLite'sjson_treefunction. This allows for a single, more efficient SQL query that leverages anINNER JOINwith theartisttable to automatically filter out any non-existent artist IDs before insertion, ensuring only valid references are stored. - Enhanced Test Coverage: I've added a new comprehensive test suite in
album_repository_test.gospecifically for participant foreign key handling. These tests validate that valid participants are correctly inserted, invalid ones are gracefully filtered without causing errors, and complex JSON structures with multiple roles and sub-roles are handled as expected. This includes a direct regression test for the original foreign key constraint error.
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 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 or fill out our survey 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. ↩
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Code Review
This pull request fixes foreign key constraint errors during album participant insertion by filtering non-existent artist IDs. The tests are comprehensive, and the approach is efficient. A couple of suggestions were made to improve the test code's maintainability and robustness.
Signed-off-by: Deluan <deluan@navidrome.org>
Added individual test cleanup to 4 album repository tests that create temporary artists and albums. This ensures proper test isolation by removing test data after each test completes, preventing test interference when running with shuffle mode. Each test now cleans up its own temporary data from the artist, library_artist, album, and album_artists tables using direct SQL deletion. Signed-off-by: Deluan <deluan@navidrome.org>
…processing Signed-off-by: Deluan <deluan@navidrome.org>
Signed-off-by: Deluan <deluan@navidrome.org>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request fixes foreign key constraint errors during album participant insertion by using SQLite's json_each and an INNER JOIN. The change includes new tests covering various scenarios. The review suggests minor performance and readability improvements.
…ctly Signed-off-by: Deluan <deluan@navidrome.org>
9f5aac1 to
86d366b
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR fixes foreign key constraint errors that occur during album scanning when albums are spread across multiple folders. The issue arose when participant (artist) insertion attempted to reference artist IDs that don't exist in the database, causing "FOREIGN KEY constraint failed" errors that prevented album scanning from completing.
- Implements automatic filtering of non-existent artist IDs during participant insertion using SQLite's
json_eachfunction with anINNER JOINto the artist table - Refactors participant insertion logic to use JSON-based SQL queries instead of iterative value insertion
- Adds comprehensive test coverage for participant foreign key handling scenarios
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| persistence/sql_participations.go | Core fix implementing JSON-based participant insertion with automatic filtering of invalid artist IDs |
| persistence/album_repository_test.go | Extensive test suite for participant foreign key handling with multiple edge cases |
| Makefile | Minor documentation update for test command usage |
Comments suppressed due to low confidence (1)
persistence/album_repository_test.go:355
- Test cleanup operations use blank identifier to ignore errors, which could mask cleanup failures and potentially affect subsequent test runs. Consider checking cleanup errors or using a dedicated test cleanup mechanism.
_, _ = artistRepo.executeSQL(squirrel.Delete("artist").Where(squirrel.Eq{"id": artist.ID}))
Description
This PR fixes foreign key constraint errors during album scanning when albums are spread across multiple folders. The issue was caused by participant (artist) insertion attempting to reference artist IDs that don't exist in the database, resulting in "FOREIGN KEY constraint failed" errors that prevented album scanning from completing.
The fix implements automatic filtering of non-existent artist IDs during participant insertion using SQLite's
json_eachfunction with anINNER JOINto the artist table. This ensures only valid artist references are inserted into the album_artists and media_file_artists tables.Related Issues
Fixes #4002
Type of Change
Checklist
Please review and check all that apply:
How to Test
The issue does not occur consistently. The situation described in #4002 may not reproduce the issue. This may be caused by the asynchronous and parallelized nature of the scanner, where it can potentially try to assign artists to an album before the album is properly stored in the DB.
The fix automatically handles invalid artist references by filtering them out during insertion, so albums with missing artist metadata will scan successfully while preserving valid participant data.
Additional Notes
Technical Implementation:
json_eachfunction to parse participant JSON dataINNER JOINwith the artist table to filter out non-existent artist IDsExpr()function for securityImpact: