diff --git a/src/main/java/me/bobert/netneutrality/http/server/MusicRoutes.java b/src/main/java/me/bobert/netneutrality/http/server/MusicRoutes.java index 781102d..c5d6c7d 100644 --- a/src/main/java/me/bobert/netneutrality/http/server/MusicRoutes.java +++ b/src/main/java/me/bobert/netneutrality/http/server/MusicRoutes.java @@ -24,8 +24,8 @@ import me.bobert.netneutrality.http.server.dto.MusicSourceResponse; import me.bobert.netneutrality.http.server.dto.MusicStatusResponse; import me.bobert.netneutrality.http.server.dto.PlayRequest; import me.bobert.netneutrality.http.server.dto.PlayResponse; -import me.bobert.netneutrality.http.server.dto.PlaySelectRequest; import me.bobert.netneutrality.http.server.dto.QueuedTrackResponse; +import me.bobert.netneutrality.http.server.dto.SearchRequest; import me.bobert.netneutrality.http.server.dto.VolumeRequest; import me.bobert.netneutrality.http.server.dto.VolumeResponse; import me.bobert.netneutrality.music.MusicManager; @@ -68,8 +68,8 @@ public final class MusicRoutes { routes.post("/music/{guildId}/resume", ctx -> resumePlayback(jdaSupplier, musicService, ctx)); routes.put("/music/{guildId}/volume", ctx -> volume(jdaSupplier, musicService, ctx)); routes.post("/music/{guildId}/vote-skip/{userId}", ctx -> voteSkip(jdaSupplier, musicService, ctx)); - routes.post("/music/{guildId}/play", ctx -> play(jdaSupplier, musicService, ctx)); - routes.post("/music/{guildId}/play/select/{searchId}", ctx -> playSelect(jdaSupplier, musicService, ctx)); + routes.post("/music/{guildId}/queue", ctx -> play(jdaSupplier, musicService, ctx)); + routes.post("/music/{guildId}/search", ctx -> search(jdaSupplier, musicService, ctx)); } @OpenApi( @@ -273,9 +273,9 @@ public final class MusicRoutes { } @OpenApi( - path = "/music/{guildId}/play", + path = "/music/{guildId}/queue", methods = HttpMethod.POST, - summary = "Resolve a query and queue it for a guild member; the bot must already be connected to a voice channel in this guild", + summary = "Queue a previously returned search candidate by passing the searchId/index returned by /music/{guildId}/search; the bot must already be connected to a voice channel in this guild", tags = {"Music"}, pathParams = @OpenApiParam(name = "guildId", description = "The Discord guild id"), requestBody = @OpenApiRequestBody(content = @OpenApiContent(from = PlayRequest.class)), @@ -290,45 +290,71 @@ public final class MusicRoutes { MusicManager manager = musicService.get(guild); PlayRequest body = ctx.bodyAsClass(PlayRequest.class); String userId = requireNonBlank(body.userId(), "userId"); - String query = requireNonBlank(body.query(), "query"); + requireNonBlank(body.searchId(), "searchId"); - ctx.future(() -> resolveMember(guild, userId) - .thenCompose(member -> manager.playForMember(member, query)) - .thenApply(outcome -> { - ctx.json(playOutcomeBody(outcome)); - return null; - })); + playSelect(guild, manager, body, userId, ctx); } @OpenApi( - path = "/music/{guildId}/play/select/{searchId}", + path = "/music/{guildId}/search", methods = HttpMethod.POST, - summary = "Queue a previously returned search candidate for a guild member", + summary = "Resolve a query into candidates without queuing anything, optionally selecting a searchEngine instead of prefixing the query; every result, including a single direct match, must be confirmed via /queue with searchId/index", tags = {"Music"}, - pathParams = { - @OpenApiParam(name = "guildId", description = "The Discord guild id"), - @OpenApiParam(name = "searchId", description = "The search id returned by a previous /play call") - }, - requestBody = @OpenApiRequestBody(content = @OpenApiContent(from = PlaySelectRequest.class)), + pathParams = @OpenApiParam(name = "guildId", description = "The Discord guild id"), + requestBody = @OpenApiRequestBody(content = @OpenApiContent(from = SearchRequest.class)), responses = { @OpenApiResponse(status = "200", content = @OpenApiContent(from = PlayResponse.class)), @OpenApiResponse(status = "400", content = @OpenApiContent(from = ErrorResponse.class)), @OpenApiResponse(status = "404", content = @OpenApiContent(from = ErrorResponse.class)) } ) - private static void playSelect(Supplier jdaSupplier, MusicService musicService, Context ctx) { + private static void search(Supplier jdaSupplier, MusicService musicService, Context ctx) { Guild guild = GuildRoutes.guild(jdaSupplier, ctx); MusicManager manager = musicService.get(guild); - String searchId = ctx.pathParam("searchId"); - PlaySelectRequest body = ctx.bodyAsClass(PlaySelectRequest.class); - String userId = requireNonBlank(body.userId(), "userId"); + SearchRequest body = ctx.bodyAsClass(SearchRequest.class); + String query = requireNonBlank(body.query(), "query"); + SourceFinder.SearchEngine searchEngine = parseSearchEngine(body.searchEngine()); + + ctx.future(() -> manager.search(query, searchEngine).thenApply(outcome -> { + ctx.json(playOutcomeBody(outcome)); + return null; + })); + } + + /** + * Parses the optional {@code searchEngine} field of a {@link SearchRequest}, failing with + * 400 when it doesn't match a known {@link SourceFinder.SearchEngine}. + */ + private static SourceFinder.SearchEngine parseSearchEngine(String searchEngine) { + if (searchEngine == null || searchEngine.isBlank()) { + return null; + } + + try { + return SourceFinder.SearchEngine.valueOf(searchEngine.toUpperCase(java.util.Locale.ROOT)); + } catch (IllegalArgumentException e) { + throw new BadRequestResponse("searchEngine must be one of " + Arrays.toString(SourceFinder.SearchEngine.values())); + } + } + + /** + * Queues a previously returned search candidate for a guild member, invoked from {@link + * #play(Supplier, MusicService, Context)} when the request body carries a {@code searchId}. + * Exposed via the {@code /music/{guildId}/queue} POST endpoint. + */ + private static void playSelect(Guild guild, MusicManager manager, PlayRequest body, String userId, Context ctx) { + String searchId = body.searchId(); + Integer index = body.index(); + if (index == null) { + throw new BadRequestResponse("index is required when searchId is provided"); + } List candidates = PendingSearchResults.consume(searchId, guild.getId()); if (candidates == null) { throw new NotFoundResponse("Unknown or expired searchId"); } - if (body.index() < 0 || body.index() >= candidates.size()) { + if (index < 0 || index >= candidates.size()) { throw new BadRequestResponse("index is out of range"); } @@ -336,7 +362,7 @@ public final class MusicRoutes { throw new BadRequestResponse("The bot isn't connected to a voice channel in this guild"); } - Track track = candidates.get(body.index()); + Track track = candidates.get(index); ctx.future(() -> resolveMember(guild, userId) .thenApply(member -> { @@ -375,19 +401,26 @@ public final class MusicRoutes { */ private static Map playOutcomeBody(PlayOutcome outcome) { return switch (outcome) { - case PlayOutcome.Queued queued -> queuedBody(queued.track(), queued.queuePosition()); - case PlayOutcome.PlaylistQueued playlistQueued -> { + case PlayOutcome.PendingTrack pendingTrack -> { Map body = new LinkedHashMap<>(); - body.put("status", "playlist_queued"); - body.put("queued_count", playlistQueued.queuedCount()); - body.put("truncated", playlistQueued.truncated()); + body.put("status", "pending_track"); + body.put("search_id", pendingTrack.searchId()); + body.put("track", trackSummary(pendingTrack.track())); yield body; } - case PlayOutcome.SearchCandidates searchCandidates -> { + case PlayOutcome.PendingPlaylist pendingPlaylist -> { Map body = new LinkedHashMap<>(); - body.put("status", "search_results"); - body.put("search_id", searchCandidates.searchId()); - body.put("candidates", searchCandidates.candidates().stream().map(MusicRoutes::trackSummary).toList()); + body.put("status", "pending_playlist"); + body.put("search_id", pendingPlaylist.searchId()); + body.put("tracks", pendingPlaylist.tracks().stream().map(MusicRoutes::trackSummary).toList()); + body.put("truncated", pendingPlaylist.truncated()); + yield body; + } + case PlayOutcome.PendingSearch pendingSearch -> { + Map body = new LinkedHashMap<>(); + body.put("status", "pending_search"); + body.put("search_id", pendingSearch.searchId()); + body.put("candidates", pendingSearch.candidates().stream().map(MusicRoutes::trackSummary).toList()); yield body; } case PlayOutcome.NotConnected ignored -> throw new BadRequestResponse( diff --git a/src/main/java/me/bobert/netneutrality/http/server/dto/PlayRequest.java b/src/main/java/me/bobert/netneutrality/http/server/dto/PlayRequest.java index 692cd1a..0927984 100644 --- a/src/main/java/me/bobert/netneutrality/http/server/dto/PlayRequest.java +++ b/src/main/java/me/bobert/netneutrality/http/server/dto/PlayRequest.java @@ -7,10 +7,15 @@ package me.bobert.netneutrality.http.server.dto; */ /** - * OpenAPI schema for the {@code /music/{guildId}/play} request body. + * OpenAPI schema for the {@code /music/{guildId}/queue} request body. + * + * @param userId the Discord user id to credit as the requester + * @param searchId a search id previously returned by {@code /music/{guildId}/search} + * @param index the index of the candidate to queue within the {@code searchId}'s results */ public record PlayRequest( String userId, - String query + String searchId, + Integer index ) { } diff --git a/src/main/java/me/bobert/netneutrality/http/server/dto/PlayResponse.java b/src/main/java/me/bobert/netneutrality/http/server/dto/PlayResponse.java index 6812416..2fed900 100644 --- a/src/main/java/me/bobert/netneutrality/http/server/dto/PlayResponse.java +++ b/src/main/java/me/bobert/netneutrality/http/server/dto/PlayResponse.java @@ -9,21 +9,24 @@ package me.bobert.netneutrality.http.server.dto; import java.util.List; /** - * OpenAPI schema for the {@code /music/{guildId}/play} and - * {@code /music/{guildId}/play/select/{searchId}} responses. + * OpenAPI schema for the {@code /music/{guildId}/queue} and {@code /music/{guildId}/search} + * responses. * - *

Fields are nullable since only a subset is populated depending on {@code status}: - * {@code queued} fills {@code track}/{@code queue_position}, {@code playlist_queued} fills - * {@code queued_count}/{@code truncated}, and {@code search_results} fills - * {@code search_id}/{@code candidates}. + *

Nothing is ever queued directly by {@code /music/{guildId}/search}: every resolved query is + * stashed as a pending candidate, to be confirmed via a follow-up {@code /music/{guildId}/queue} + * call carrying {@code searchId}/{@code index}. Fields are nullable since only a subset is populated depending + * on {@code status}: {@code queued} fills {@code track}/{@code queue_position} (confirmed + * selection), {@code pending_track} fills {@code search_id}/{@code track}, {@code + * pending_playlist} fills {@code search_id}/{@code tracks}/{@code truncated}, and {@code + * pending_search} fills {@code search_id}/{@code candidates}. */ public record PlayResponse( String status, TrackSummaryResponse track, Integer queue_position, - Integer queued_count, - Boolean truncated, String search_id, + List tracks, + Boolean truncated, List candidates ) { } diff --git a/src/main/java/me/bobert/netneutrality/http/server/dto/PlaySelectRequest.java b/src/main/java/me/bobert/netneutrality/http/server/dto/PlaySelectRequest.java deleted file mode 100644 index 4f69fca..0000000 --- a/src/main/java/me/bobert/netneutrality/http/server/dto/PlaySelectRequest.java +++ /dev/null @@ -1,16 +0,0 @@ -package me.bobert.netneutrality.http.server.dto; - -/* - * Copyright (c) anthorob - * - * https://discord.gg/qUa6zNFZsd - */ - -/** - * OpenAPI schema for the {@code /music/{guildId}/play/select/{searchId}} request body. - */ -public record PlaySelectRequest( - String userId, - int index -) { -} diff --git a/src/main/java/me/bobert/netneutrality/http/server/dto/SearchRequest.java b/src/main/java/me/bobert/netneutrality/http/server/dto/SearchRequest.java new file mode 100644 index 0000000..ff94f03 --- /dev/null +++ b/src/main/java/me/bobert/netneutrality/http/server/dto/SearchRequest.java @@ -0,0 +1,22 @@ +package me.bobert.netneutrality.http.server.dto; + +/* + * Copyright (c) anthorob + * + * https://discord.gg/qUa6zNFZsd + */ + +/** + * OpenAPI schema for the {@code /music/{guildId}/search} request body. + * + * @param query the identifier or search query to resolve + * @param searchEngine the {@link me.bobert.netneutrality.music.SourceFinder.SearchEngine} name + * to use for non-url queries (e.g. {@code "YTMUSIC"}), or {@code null} to + * use the guild's configured default; lets callers select an engine + * instead of manually prefixing the query + */ +public record SearchRequest( + String query, + String searchEngine +) { +} diff --git a/src/main/java/me/bobert/netneutrality/music/MusicManager.java b/src/main/java/me/bobert/netneutrality/music/MusicManager.java index 509138e..839be57 100644 --- a/src/main/java/me/bobert/netneutrality/music/MusicManager.java +++ b/src/main/java/me/bobert/netneutrality/music/MusicManager.java @@ -226,28 +226,38 @@ public class MusicManager implements MusicManagerCallback { } /** - * Member-driven counterpart of {@link #play(String, CommandContext, boolean)}, usable by - * non-command callers such as the REST API. + * Resolves a query without queuing anything, always returning a {@link PlayOutcome} that + * surfaces candidates to be confirmed via the {@code /music/{guildId}/queue} endpoint's + * {@code searchId}/{@code index} selection. * - *

Only a Discord command can make the bot join a voice channel in the first place, so this - * method never attempts to connect: it fails fast with a {@link PlayOutcome.NotConnected} - * outcome when the bot isn't already connected for this guild. + *

Nothing is ever queued directly by this method: every resolved query, whether a single + * track, a playlist, or an ambiguous search, requires a separate call carrying the returned + * {@code searchId}/{@code index} to actually add it to the queue. * - * @param member the Discord member the track is queued on behalf of - * @param query the URL or search query to resolve - * @return a future completed with the outcome of the load/queue attempt + * @param query the URL or search query to resolve + * @return a future completed with the search outcome */ - public synchronized CompletableFuture playForMember(Member member, String query) { - logger.debug("Guild {}: REST request to play '{}' by user {}", guild.getId(), query, member.getId()); + public synchronized CompletableFuture search(String query) { + return search(query, null); + } + + /** + * Same as {@link #search(String)}, but allows explicitly selecting which + * {@link SourceFinder.SearchEngine} is used to resolve a non-url {@code query}, instead of + * relying on the guild's configured default or a manually prefixed identifier + * (e.g. {@code ytmsearch:}). + * + * @param query the identifier or search query to resolve + * @param searchEngine the search engine to use for non-url queries, or {@code null} to use + * the guild's configured default + * @return a future completed with the search outcome + */ + public synchronized CompletableFuture search(String query, SourceFinder.SearchEngine searchEngine) { + logger.debug("Guild {}: REST request to search '{}' using engine {}", guild.getId(), query, searchEngine); CompletableFuture future = new CompletableFuture<>(); - if (!isConnected()) { - future.complete(new PlayOutcome.NotConnected()); - return future; - } - - ResolvedSource source = resolveSource(query); + ResolvedSource source = resolveSource(query, searchEngine); if (source.type() == SourceFinder.AudioSourceType.Udio) { future.complete(new PlayOutcome.Failed("Udio playback is temporarily unsupported since the migration to Lavalink")); return future; @@ -265,7 +275,7 @@ public class MusicManager implements MusicManagerCallback { } link.loadItem(source.identifier()).subscribe( - new RestTrackLoadingHandler(member, this, source.identifier(), future), + new SearchResultHandler(this, source.identifier(), future), error -> { LogHandler.log(new RuntimeException("Failed to load track: " + error.getMessage(), error)); future.completeExceptionally(error); @@ -283,9 +293,17 @@ public class MusicManager implements MusicManagerCallback { } private ResolvedSource resolveSource(String query) { - SourceFinder.SearchEngine searchEngine = parseSearchEngine(guildService); - SourceFinder.AudioSourceType type = SourceFinder.getSource(query, searchEngine); - String identifier = type == SourceFinder.AudioSourceType.Udio ? null : SourceFinder.getIdentifier(query, searchEngine); + return resolveSource(query, null); + } + + /** + * Resolves a query's source, using {@code searchEngine} when explicitly provided instead of + * the guild's configured default. + */ + private ResolvedSource resolveSource(String query, SourceFinder.SearchEngine searchEngine) { + SourceFinder.SearchEngine effectiveSearchEngine = searchEngine != null ? searchEngine : parseSearchEngine(guildService); + SourceFinder.AudioSourceType type = SourceFinder.getSource(query, effectiveSearchEngine); + String identifier = type == SourceFinder.AudioSourceType.Udio ? null : SourceFinder.getIdentifier(query, effectiveSearchEngine); return new ResolvedSource(type, identifier); } diff --git a/src/main/java/me/bobert/netneutrality/music/PlayOutcome.java b/src/main/java/me/bobert/netneutrality/music/PlayOutcome.java index f17d8f1..d0e0bf3 100644 --- a/src/main/java/me/bobert/netneutrality/music/PlayOutcome.java +++ b/src/main/java/me/bobert/netneutrality/music/PlayOutcome.java @@ -11,19 +11,22 @@ import dev.arbjerg.lavalink.client.player.Track; import java.util.List; /** - * Outcome of a {@link MusicManager#playForMember(net.dv8tion.jda.api.entities.Member, String)} - * call, used to report the result of a REST-triggered play request without any Discord message - * context. + * Outcome of a {@link MusicManager#search(String)} call, used to report the result of a + * REST-triggered load request without any Discord message context. + * + *

Nothing is ever queued directly: every resolved query, whether a single track, a playlist, + * or an ambiguous search, is stashed in {@link PendingSearchResults} and must be confirmed via a + * separate {@code /play} call carrying the returned {@code searchId}/{@code index}. */ public sealed interface PlayOutcome { - record Queued(Track track, int queuePosition) implements PlayOutcome { + record PendingTrack(String searchId, Track track) implements PlayOutcome { } - record PlaylistQueued(int queuedCount, boolean truncated) implements PlayOutcome { + record PendingPlaylist(String searchId, List tracks, boolean truncated) implements PlayOutcome { } - record SearchCandidates(String searchId, List candidates) implements PlayOutcome { + record PendingSearch(String searchId, List candidates) implements PlayOutcome { } record NotConnected() implements PlayOutcome { diff --git a/src/main/java/me/bobert/netneutrality/music/RestTrackLoadingHandler.java b/src/main/java/me/bobert/netneutrality/music/SearchResultHandler.java similarity index 61% rename from src/main/java/me/bobert/netneutrality/music/RestTrackLoadingHandler.java rename to src/main/java/me/bobert/netneutrality/music/SearchResultHandler.java index 35cc049..afe51d7 100644 --- a/src/main/java/me/bobert/netneutrality/music/RestTrackLoadingHandler.java +++ b/src/main/java/me/bobert/netneutrality/music/SearchResultHandler.java @@ -15,7 +15,6 @@ import dev.arbjerg.lavalink.client.player.TrackLoaded; import me.bobert.netneutrality.core.LogHandler; import me.bobert.netneutrality.db.GSettings; import net.dv8tion.jda.api.entities.Guild; -import net.dv8tion.jda.api.entities.Member; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -23,61 +22,53 @@ import java.util.List; import java.util.concurrent.CompletableFuture; /** - * Translates a Lavalink load result into queue operations and a {@link PlayOutcome}, without any - * Discord message context. + * Translates a Lavalink load result into a {@link PlayOutcome} without ever queuing anything, so + * that every REST load request, not only ambiguous text searches, requires a separate + * {@code /play} call with {@code searchId}/{@code index} to actually queue a track. * - *

Mirrors {@link TrackLoadingHandler}'s branching but completes a future instead of replying - * on a {@link me.bobert.netneutrality.core.context.CommandContext}: direct tracks and playlists - * are queued immediately, while ambiguous search results are stashed in - * {@link PendingSearchResults} for a later {@code /play/select} call instead of being queued. + *

A single resolved track, the tracks of a playlist, and ambiguous search results are each + * stashed as candidates in {@link PendingSearchResults} and reported back with their own + * {@link PlayOutcome.PendingTrack}, {@link PlayOutcome.PendingPlaylist}, or + * {@link PlayOutcome.PendingSearch} outcome, respectively. */ -public class RestTrackLoadingHandler extends AbstractAudioLoadResultHandler { - private static final Logger logger = LogManager.getLogger(RestTrackLoadingHandler.class); +public class SearchResultHandler extends AbstractAudioLoadResultHandler { + private static final Logger logger = LogManager.getLogger(SearchResultHandler.class); private static final int SEARCH_RESULTS_LIMIT = 5; private final String identifier; private final Guild guild; - private final QueueManager queueManager; - private final Member member; private final MusicManager manager; private final CompletableFuture future; - public RestTrackLoadingHandler(Member member, MusicManager manager, String identifier, CompletableFuture future) { - this.member = member; - this.manager = manager; + public SearchResultHandler(MusicManager manager, String identifier, CompletableFuture future) { this.identifier = identifier; this.guild = manager.getGuild(); - this.queueManager = manager.getQueueManager(); + this.manager = manager; this.future = future; } @Override public void ontrackLoaded(TrackLoaded result) { Track track = result.getTrack(); - logger.debug("Guild {}: track loaded: {}", guild.getId(), track.getInfo().getIdentifier()); + logger.debug("Guild {}: track resolved: {}", guild.getId(), track.getInfo().getIdentifier()); - queueManager.queue(track, member); - future.complete(new PlayOutcome.Queued(track, queueManager.getQueueSize())); + String searchId = PendingSearchResults.put(guild.getId(), List.of(track)); + future.complete(new PlayOutcome.PendingTrack(searchId, track)); } @Override public void onPlaylistLoaded(PlaylistLoaded result) { - logger.debug("Guild {}: playlist loaded: {}", guild.getId(), identifier); + logger.debug("Guild {}: playlist resolved: {}", guild.getId(), identifier); List tracks = result.getTracks(); int maxPlaylistSize = parseMaxPlaylistSize(); boolean truncated = tracks.size() > maxPlaylistSize; - List toQueue = truncated ? tracks.subList(0, maxPlaylistSize) : tracks; + List candidates = truncated ? tracks.subList(0, maxPlaylistSize) : tracks; - queueManager.queue(toQueue, member); - future.complete(new PlayOutcome.PlaylistQueued(toQueue.size(), truncated)); + String searchId = PendingSearchResults.put(guild.getId(), candidates); + future.complete(new PlayOutcome.PendingPlaylist(searchId, candidates, truncated)); } - /** - * Search results are not queued: the candidates are stashed under a generated search id and - * left for a REST caller to confirm via {@code /play/select}, since there is no interactive - * UI to pick one over HTTP. - */ @Override public void onSearchResultLoaded(SearchResult result) { List tracks = result.getTracks(); @@ -90,8 +81,7 @@ public class RestTrackLoadingHandler extends AbstractAudioLoadResultHandler { List candidates = tracks.stream().limit(SEARCH_RESULTS_LIMIT).toList(); String searchId = PendingSearchResults.put(guild.getId(), candidates); - - future.complete(new PlayOutcome.SearchCandidates(searchId, candidates)); + future.complete(new PlayOutcome.PendingSearch(searchId, candidates)); } @Override