From 3c442f31689c41fed1191c14851833524dde015f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?To=CF=80?= Date: Sat, 2 Dec 2023 13:50:14 +0100 Subject: [PATCH] Allow setting user data on tracks (#983) * implement track user data * fix unit tests and add Track#copyWithUserData * remove unnecessary exceptionally * use loadItemSync instead of loadItem in player rest handler * Update protocol/src/commonTest/kotlin/PlayerSerializerTest.kt Co-authored-by: Duncan Sterken * throw http 400 if both track and encodedTrack/identifier is set * add convenient deserializeUserData method for java --------- Co-authored-by: Duncan Sterken --- .../server/player/PlayerRestHandler.kt | 119 +++++++++++------- .../main/java/lavalink/server/util/util.kt | 2 +- .../arbjerg/lavalink/protocol/v4/player.kt | 35 +++++- .../kotlin/LoadResultSerializerTest.kt | 3 +- .../kotlin/MessageSerializerTest.kt | 12 +- .../commonTest/kotlin/PlayerSerializerTest.kt | 16 ++- 6 files changed, 129 insertions(+), 58 deletions(-) diff --git a/LavalinkServer/src/main/java/lavalink/server/player/PlayerRestHandler.kt b/LavalinkServer/src/main/java/lavalink/server/player/PlayerRestHandler.kt index 6bd043499..6a837dbf5 100644 --- a/LavalinkServer/src/main/java/lavalink/server/player/PlayerRestHandler.kt +++ b/LavalinkServer/src/main/java/lavalink/server/player/PlayerRestHandler.kt @@ -59,8 +59,28 @@ class PlayerRestHandler( ): ResponseEntity { val context = socketContext(socketServer, sessionId) - val encodedTrack = playerUpdate.encodedTrack - if (encodedTrack is Omissible.Present && playerUpdate.identifier is Omissible.Present) { + if (playerUpdate.track.isPresent() && (playerUpdate.encodedTrack is Omissible.Present || playerUpdate.identifier is Omissible.Present)) { + throw ResponseStatusException(HttpStatus.BAD_REQUEST, "Cannot specify both track and encodedTrack/identifier") + } + + val track = if (playerUpdate.track.isPresent()) { + playerUpdate.track + } else { + if (playerUpdate.encodedTrack is Omissible.Present || playerUpdate.identifier is Omissible.Present) { + PlayerUpdateTrack( + playerUpdate.encodedTrack, + playerUpdate.identifier + ).toOmissible() + } else { + Omissible.Omitted() + } + } + + val encodedTrack = track.ifPresent { it.encoded } ?: Omissible.Omitted() + val identifier = track.ifPresent { it.identifier } ?: Omissible.Omitted() + val userData = track.ifPresent { it.userData } ?: Omissible.Omitted() + + if (encodedTrack is Omissible.Present && identifier is Omissible.Present) { throw ResponseStatusException(HttpStatus.BAD_REQUEST, "Cannot specify both encodedTrack and identifier") } @@ -112,17 +132,23 @@ class PlayerRestHandler( // we handle pause differently for playing new tracks val paused = playerUpdate.paused - paused.takeIfPresent { encodedTrack is Omissible.Omitted && playerUpdate.identifier is Omissible.Omitted } + paused.takeIfPresent { encodedTrack is Omissible.Omitted && identifier is Omissible.Omitted } ?.let { player.setPause(it) } + // we handle userData differently for playing new tracks + userData.takeIfPresent { encodedTrack is Omissible.Omitted && identifier is Omissible.Omitted } + ?.let { + player.track?.userData = it + } + playerUpdate.volume.ifPresent { player.setVolume(it) } // we handle position differently for playing new tracks - playerUpdate.position.takeIfPresent { encodedTrack is Omissible.Omitted && playerUpdate.identifier is Omissible.Omitted } + playerUpdate.position.takeIfPresent { encodedTrack is Omissible.Omitted && identifier is Omissible.Omitted } ?.let { if (player.isPlaying) { player.seekTo(it) @@ -130,7 +156,7 @@ class PlayerRestHandler( } } - playerUpdate.endTime.takeIfPresent { encodedTrack is Omissible.Omitted && playerUpdate.identifier is Omissible.Omitted } + playerUpdate.endTime.takeIfPresent { encodedTrack is Omissible.Omitted && identifier is Omissible.Omitted } ?.let { endTime -> val marker = TrackMarker(endTime, TrackEndMarkerHandler(player)) player.track?.setMarker(marker) @@ -141,7 +167,7 @@ class PlayerRestHandler( SocketServer.sendPlayerUpdate(context, player) } - if (encodedTrack is Omissible.Present || playerUpdate.identifier is Omissible.Present) { + if (encodedTrack is Omissible.Present || identifier is Omissible.Present) { if (noReplace && player.track != null) { log.info("Skipping play request because of noReplace") @@ -149,64 +175,67 @@ class PlayerRestHandler( } player.setPause(if (paused is Omissible.Present) paused.value else false) - val track: AudioTrack? = if (encodedTrack is Omissible.Present) { + val newTrack: AudioTrack? = if (encodedTrack is Omissible.Present) { encodedTrack.value?.let { decodeTrack(context.audioPlayerManager, it) } } else { val trackFuture = CompletableFuture() - val identifier = playerUpdate.identifier as Omissible.Present - context.audioPlayerManager.loadItem(identifier.value, object : AudioLoadResultHandler { - override fun trackLoaded(track: AudioTrack) { - trackFuture.complete(track) - } - - override fun playlistLoaded(playlist: AudioPlaylist) { - trackFuture.completeExceptionally( - ResponseStatusException( - HttpStatus.BAD_REQUEST, - "Cannot play a playlist or search result" + context.audioPlayerManager.loadItemSync( + (identifier as Omissible.Present).value, + object : AudioLoadResultHandler { + override fun trackLoaded(track: AudioTrack) { + trackFuture.complete(track) + } + + override fun playlistLoaded(playlist: AudioPlaylist) { + trackFuture.completeExceptionally( + ResponseStatusException( + HttpStatus.BAD_REQUEST, + "Cannot play a playlist or search result" + ) ) - ) - } - - override fun noMatches() { - trackFuture.completeExceptionally( - ResponseStatusException( - HttpStatus.BAD_REQUEST, - "No matches found for identifier" + } + + override fun noMatches() { + trackFuture.completeExceptionally( + ResponseStatusException( + HttpStatus.BAD_REQUEST, + "No matches found for identifier" + ) ) - ) - } - - override fun loadFailed(exception: FriendlyException) { - trackFuture.completeExceptionally( - ResponseStatusException( - HttpStatus.INTERNAL_SERVER_ERROR, - exception.message, - getRootCause(exception) + } + + override fun loadFailed(exception: FriendlyException) { + trackFuture.completeExceptionally( + ResponseStatusException( + HttpStatus.INTERNAL_SERVER_ERROR, + exception.message, + getRootCause(exception) + ) ) - ) - } - }) + } + }) - trackFuture.exceptionally { - throw it - }.join() + trackFuture.join() } - track?.let { + newTrack?.let { playerUpdate.position.ifPresent { position -> - track.position = position + newTrack.position = position + } + + userData.ifPresent { userData -> + newTrack.userData = userData } playerUpdate.endTime.ifPresent { endTime -> if (endTime != null) { - track.setMarker(TrackMarker(endTime, TrackEndMarkerHandler(player))) + newTrack.setMarker(TrackMarker(endTime, TrackEndMarkerHandler(player))) } } - player.play(track) + player.play(newTrack) player.provideTo(context.getMediaConnection(player)) } ?: player.stop() } diff --git a/LavalinkServer/src/main/java/lavalink/server/util/util.kt b/LavalinkServer/src/main/java/lavalink/server/util/util.kt index 4a093cd27..a3b63d39e 100644 --- a/LavalinkServer/src/main/java/lavalink/server/util/util.kt +++ b/LavalinkServer/src/main/java/lavalink/server/util/util.kt @@ -54,7 +54,7 @@ fun AudioTrack.toTrack(encoded: String, pluginInfoModifiers: List deserializePluginInfo(deserializer: DeserializationStrategy): T = pluginInfo.deserialize(deserializer) + + /** + * Deserialize the user data into a specific type. + * This method is a convenience method meant to be used in Java, + * since Kotlin extension methods are painful to use in Java. + * + * @param deserializer The deserializer to use. (e.g. `T.Companion.serializer()`) + * + * @return the deserialized user data as type T + */ + fun deserializeUserData(deserializer: DeserializationStrategy): T = userData.deserialize(deserializer) + + /** + * Copy this track with a new user data json. + * + * @param userData The new user data json. + * + * @return A copy of this track with the new user data json. + */ + fun copyWithUserData(userData: JsonObject): Track { + return copy(userData = userData) + } } @Serializable @@ -84,10 +107,20 @@ data class PlayerState( val ping: Long ) +@Serializable +data class PlayerUpdateTrack( + val encoded: Omissible = Omissible.Omitted(), + val identifier: Omissible = Omissible.Omitted(), + val userData: Omissible = Omissible.Omitted() +) + @Serializable data class PlayerUpdate( + @Deprecated("Use PlayerUpdateTrack#encoded instead", ReplaceWith("encoded")) val encodedTrack: Omissible = Omissible.Omitted(), + @Deprecated("Use PlayerUpdateTrack#identifier instead") val identifier: Omissible = Omissible.Omitted(), + val track: Omissible = Omissible.Omitted(), val position: Omissible = Omissible.Omitted(), val endTime: Omissible = Omissible.Omitted(), val volume: Omissible = Omissible.Omitted(), diff --git a/protocol/src/commonTest/kotlin/LoadResultSerializerTest.kt b/protocol/src/commonTest/kotlin/LoadResultSerializerTest.kt index c16e3ee13..af921b860 100644 --- a/protocol/src/commonTest/kotlin/LoadResultSerializerTest.kt +++ b/protocol/src/commonTest/kotlin/LoadResultSerializerTest.kt @@ -29,7 +29,8 @@ class LoadResultSerializerTest { "isrc": null, "sourceName": "youtube" }, - "pluginInfo": {} + "pluginInfo": {}, + "userData": {} }, "exception": null } diff --git a/protocol/src/commonTest/kotlin/MessageSerializerTest.kt b/protocol/src/commonTest/kotlin/MessageSerializerTest.kt index b1af3b5ce..c5c841a4b 100644 --- a/protocol/src/commonTest/kotlin/MessageSerializerTest.kt +++ b/protocol/src/commonTest/kotlin/MessageSerializerTest.kt @@ -130,7 +130,8 @@ class MessageSerializerTest { "isrc": null, "sourceName": "youtube" }, - "pluginInfo": {} + "pluginInfo": {}, + "userData": {} } } """.trimIndent() @@ -183,7 +184,8 @@ class MessageSerializerTest { "isrc": null, "sourceName": "youtube" }, - "pluginInfo": {} + "pluginInfo": {}, + "userData": {} }, "reason": "finished" } @@ -238,7 +240,8 @@ class MessageSerializerTest { "isrc": null, "sourceName": "youtube" }, - "pluginInfo": {} + "pluginInfo": {}, + "userData": {} }, "exception": { "message": "...", @@ -301,7 +304,8 @@ class MessageSerializerTest { "isrc": null, "sourceName": "youtube" }, - "pluginInfo": {} + "pluginInfo": {}, + "userData": {} }, "thresholdMs": 123456789 } diff --git a/protocol/src/commonTest/kotlin/PlayerSerializerTest.kt b/protocol/src/commonTest/kotlin/PlayerSerializerTest.kt index 8d2af63b7..f895be5e7 100644 --- a/protocol/src/commonTest/kotlin/PlayerSerializerTest.kt +++ b/protocol/src/commonTest/kotlin/PlayerSerializerTest.kt @@ -27,7 +27,8 @@ private const val json = """ "artworkUrl": null, "isrc": null }, - "pluginInfo": {} + "pluginInfo": {}, + "userData": {} }, "volume": 100, "paused": false, @@ -52,7 +53,9 @@ private const val json = """ //language=json const val updateJson = """ { - "identifier": "...", + "track": { + "identifier": "..." + }, "endTime": 0, "volume": 100, "position": 32400, @@ -112,8 +115,7 @@ class PlayerSerializerTest { val json = """{}""" test(json) { - assertIs>(encodedTrack) - assertIs>(identifier) + assertIs>(track) assertIs>(position) assertIs>(endTime) assertIs>(volume) @@ -127,7 +129,7 @@ class PlayerSerializerTest { @JsName("test3") fun `test encodedTrack and identifier exclusivity`() { //language=json - val json = """{"encodedTrack": "", "identifier": ""}""" + val json = """{"track": {"encoded": "", "identifier": ""}}""" assertFailsWith { Json.decodeFromString(json) } } @@ -136,7 +138,9 @@ class PlayerSerializerTest { @JsName("test4") fun `test update player serialization`() { test(updateJson) { - identifier shouldBe "..." + track.requirePresent { + identifier shouldBe "..." + } endTime shouldBe 0 volume shouldBe 100 position shouldBe 32400