Add queue endpoints and index-based playqueue #4210
Conversation
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!
This pull request introduces native API support for managing the user's play queue. It refactors the internal PlayQueue model and persistence to use an index for the 'current' item instead of its ID, includes a database migration for this change, and ensures backward compatibility with the Subsonic API by handling the ID/index conversion.
Highlights
- Native API Endpoints: I've added new
/api/queueendpoints (GETandPOST) to the native API for managing the user's play queue. - PlayQueue Current Item Representation: The internal representation of the current item in the
PlayQueuemodel and persistence layer has been changed from storing the item's ID (string) to storing its index (integer) within the queue's item list. - Database Migration: A new database migration is included to convert existing
playqueue.currentvalues from item IDs to their corresponding indices when upgrading. - Subsonic API Compatibility: The Subsonic API handlers for play queue (
getPlayQueue,savePlayQueue) have been updated to convert between the internal index-basedcurrentand the external ID-basedcurrentrequired by the Subsonic API specification. - Testing and Mocks: New unit tests for the native queue endpoints and a mock implementation for the
PlayQueueRepositoryhave been added.
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 configureGemini 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.
Code Review
This pull request introduces new native API endpoints for managing play queues and refactors the play queue's current item tracking from ID-based to index-based, including a database migration. The Subsonic API is updated for compatibility, and tests have been added.
The review identified a critical issue regarding potential data loss in the database migration. Additionally, there are concerns about missing input validation in the new saveQueue API endpoint and the behavior of the Subsonic savePlayQueue endpoint when a current ID is not found. The irreversibility of the down migration and error handling in HTTP responses were also noted. Addressing these points will enhance the robustness and data integrity of the changes.
Signed-off-by: Deluan <deluan@navidrome.org>
There was a problem hiding this comment.
Pull Request Overview
This PR adds native queue management endpoints and shifts PlayQueue’s Current field from a string ID to a numeric index throughout the API, persistence layer, and Subsonic compatibility layer.
- Introduce GET/POST
/api/queuehandlers in the native API with full test coverage - Change
PlayQueue.Currenttype tointand add a migration to convert existing ID values to indices - Update Subsonic endpoint to map between index and item ID, and adjust mocks, tests, and repository code accordingly
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/mock_playqueue_repo.go | Add mock repository implementation for PlayQueue |
| tests/mock_data_store.go | Wire MockedPlayQueue into the mock data store |
| server/subsonic/bookmarks.go | Map PlayQueue index back to item ID for Subsonic responses |
| server/nativeapi/queue.go | Implement native GET/POST queue endpoints |
| server/nativeapi/queue_test.go | Add unit tests for native queue endpoints |
| server/nativeapi/native_api.go | Register the new queue routes |
| persistence/playqueue_repository.go | Change stored Current column type from string to int |
| persistence/playqueue_repository_test.go | Update repository tests to use integer Current values |
| model/playqueue.go | Change PlayQueue.Current field type and JSON tag |
| db/migrations/20250611010101_playqueue_current_to_index.go | Add migration converting old ID-based current column to index |
Comments suppressed due to low confidence (3)
tests/mock_playqueue_repo.go:12
- [nitpick] The field name
Erris ambiguous—it's a boolean but reads like anerror. Consider renaming it to something likeShouldFailor splitting intoErrOnStoreandErrOnRetrieveto clarify intent.
Err bool
server/nativeapi/queue.go:65
- The call to
max(payload.Position, 0)references an undefinedmaxfunction. Consider replacing it with an inline check (e.g.,pos := payload.Position; if pos < 0 { pos = 0 }) or importing/defining a suitablemaxhelper.
Position: max(payload.Position, 0),
db/migrations/20250611010101_playqueue_current_to_index.go:78
- The down migration is left as a no-op, which prevents rolling back this schema change. If rolling back is required, implement the inverse transformation or document why it’s intentionally omitted.
func downPlayQueueCurrentToIndex(ctx context.Context, tx *sql.Tx) error {
Summary
/api/queueendpoints in native APIcurrentvalues to numeric indexTesting
make testhttps://chatgpt.com/codex/tasks/task_b_6848caf1beb4832e8e8bd843fac41bed