diff --git a/buildSrc/src/main/kotlin/Dependencies.kt b/buildSrc/src/main/kotlin/Dependencies.kt index 96c440f0cbf..ec2f0019902 100644 --- a/buildSrc/src/main/kotlin/Dependencies.kt +++ b/buildSrc/src/main/kotlin/Dependencies.kt @@ -1,6 +1,6 @@ object Versions { const val kotlin = "1.5.0" - const val kotlinxSerialization = "1.0.0" + const val kotlinxSerialization = "1.2.0" const val ktor = "1.5.3" const val kotlinxCoroutines = "1.5.0-RC" const val kotlinLogging = "2.0.4" diff --git a/common/src/test/kotlin/entity/optional/OptionalBooleanTest.kt b/common/src/test/kotlin/entity/optional/OptionalBooleanTest.kt index 0d004196e76..14b32bf1d2b 100644 --- a/common/src/test/kotlin/entity/optional/OptionalBooleanTest.kt +++ b/common/src/test/kotlin/entity/optional/OptionalBooleanTest.kt @@ -10,41 +10,42 @@ import org.junit.jupiter.api.Test internal class OptionalBooleanTest { + @Serializable + private class EmptyOptionalEntity(val value: OptionalBoolean = OptionalBoolean.Missing) + @Test fun `deserializing nothing in optional assigns Missing`(){ @Language("json") val json = """{}""" - @Serializable - class Entity(val value: OptionalBoolean = OptionalBoolean.Missing) - - val entity = Json.decodeFromString(json) + val entity = Json.decodeFromString(json) assert(entity.value is OptionalBoolean.Missing) } + @Serializable + private class NullOptionalEntity(@Suppress("unused") val value: OptionalBoolean = OptionalBoolean.Missing) + @Test fun `deserializing null in optional throws SerializationException`(){ @Language("json") val json = """{ "value":null }""" - @Serializable - class Entity(@Suppress("unused") val value: OptionalBoolean = OptionalBoolean.Missing) - org.junit.jupiter.api.assertThrows { - Json.decodeFromString(json) + Json.decodeFromString(json) } } + @Serializable + private class ValueOptionalEntity(@Suppress("unused") val value: OptionalBoolean = OptionalBoolean.Missing) + @Test fun `deserializing value in optional assigns Value`(){ @Language("json") val json = """{ "value":true }""" - @Serializable - class Entity(@Suppress("unused") val value: OptionalBoolean = OptionalBoolean.Missing) - val entity = Json.decodeFromString(json) + val entity = Json.decodeFromString(json) require(entity.value is OptionalBoolean.Value) Assertions.assertEquals(true, entity.value.value) diff --git a/common/src/test/kotlin/entity/optional/OptionalIntTest.kt b/common/src/test/kotlin/entity/optional/OptionalIntTest.kt index c9d3990ab04..a6f497e26be 100644 --- a/common/src/test/kotlin/entity/optional/OptionalIntTest.kt +++ b/common/src/test/kotlin/entity/optional/OptionalIntTest.kt @@ -10,41 +10,43 @@ import org.junit.jupiter.api.Test internal class OptionalIntTest { + @Serializable + private class EmptyOptionalEntity(val value: OptionalInt = OptionalInt.Missing) + @Test fun `deserializing nothing in optional assigns Missing`(){ @Language("json") val json = """{}""" - @Serializable - class Entity(val value: OptionalInt = OptionalInt.Missing) - - val entity = Json.decodeFromString(json) + val entity = Json.decodeFromString(json) assert(entity.value is OptionalInt.Missing) } + + @Serializable + private class NullOptionalEntity(@Suppress("unused") val value: OptionalInt = OptionalInt.Missing) + @Test fun `deserializing null in optional throws SerializationException`(){ @Language("json") val json = """{ "value":null }""" - @Serializable - class Entity(@Suppress("unused") val value: OptionalInt = OptionalInt.Missing) org.junit.jupiter.api.assertThrows { - Json.decodeFromString(json) + Json.decodeFromString(json) } } + @Serializable + class ValueOptionalEntity(@Suppress("unused") val value: OptionalInt = OptionalInt.Missing) + @Test fun `deserializing value in optional assigns Value`(){ @Language("json") val json = """{ "value":5 }""" - @Serializable - class Entity(@Suppress("unused") val value: OptionalInt = OptionalInt.Missing) - - val entity = Json.decodeFromString(json) + val entity = Json.decodeFromString(json) require(entity.value is OptionalInt.Value) assertEquals(5, entity.value.value) diff --git a/common/src/test/kotlin/entity/optional/OptionalLongTest.kt b/common/src/test/kotlin/entity/optional/OptionalLongTest.kt index f5222c6525b..9ec0618be62 100644 --- a/common/src/test/kotlin/entity/optional/OptionalLongTest.kt +++ b/common/src/test/kotlin/entity/optional/OptionalLongTest.kt @@ -10,41 +10,45 @@ import org.junit.jupiter.api.Test internal class OptionalLongTest { + @Serializable + class EmptyOptionalEntity(val value: OptionalLong = OptionalLong.Missing) + @Test - fun `deserializing nothing in optional assigns Missing`(){ + fun `deserializing nothing in optional assigns Missing`() { @Language("json") val json = """{}""" - @Serializable - class Entity(val value: OptionalLong = OptionalLong.Missing) - val entity = Json.decodeFromString(json) + val entity = Json.decodeFromString(json) assert(entity.value is OptionalLong.Missing) } + + @Serializable + class NullOptionalEntity(@Suppress("unused") val value: OptionalLong = OptionalLong.Missing) + @Test - fun `deserializing null in optional throws SerializationException`(){ + fun `deserializing null in optional throws SerializationException`() { @Language("json") val json = """{ "value":null }""" - @Serializable - class Entity(@Suppress("unused") val value: OptionalLong = OptionalLong.Missing) org.junit.jupiter.api.assertThrows { - Json.decodeFromString(json) + Json.decodeFromString(json) } } + + @Serializable + class ValueOptionalEntity(@Suppress("unused") val value: OptionalLong = OptionalLong.Missing) + @Test - fun `deserializing value in optional assigns Value`(){ + fun `deserializing value in optional assigns Value`() { @Language("json") val json = """{ "value":5 }""" - @Serializable - class Entity(@Suppress("unused") val value: OptionalLong = OptionalLong.Missing) - - val entity = Json.decodeFromString(json) + val entity = Json.decodeFromString(json) require(entity.value is OptionalLong.Value) Assertions.assertEquals(5, entity.value.value) diff --git a/common/src/test/kotlin/entity/optional/OptionalSnowflakeTest.kt b/common/src/test/kotlin/entity/optional/OptionalSnowflakeTest.kt index d28cd31dfbb..948b8c3ce72 100644 --- a/common/src/test/kotlin/entity/optional/OptionalSnowflakeTest.kt +++ b/common/src/test/kotlin/entity/optional/OptionalSnowflakeTest.kt @@ -11,41 +11,45 @@ import org.junit.jupiter.api.Test internal class OptionalSnowflakeTest { + + @Serializable + class EmptyOptionalEntity(val value: OptionalSnowflake = OptionalSnowflake.Missing) + @Test fun `deserializing nothing in optional assigns Missing`(){ @Language("json") val json = """{}""" - @Serializable - class Entity(val value: OptionalSnowflake = OptionalSnowflake.Missing) - val entity = Json.decodeFromString(json) + val entity = Json.decodeFromString(json) assert(entity.value is OptionalSnowflake.Missing) } + + @Serializable + class NullOptionalEntity(@Suppress("unused") val value: OptionalSnowflake = OptionalSnowflake.Missing) + @Test fun `deserializing null in optional throws SerializationException`(){ @Language("json") val json = """{ "value":null }""" - @Serializable - class Entity(@Suppress("unused") val value: OptionalSnowflake = OptionalSnowflake.Missing) - org.junit.jupiter.api.assertThrows { - Json.decodeFromString(json) + Json.decodeFromString(json) } } + + @Serializable + class ValueOptionalEntity(@Suppress("unused") val value: OptionalSnowflake = OptionalSnowflake.Missing) + @Test fun `deserializing value in optional assigns Value`(){ @Language("json") val json = """{ "value":5 }""" - @Serializable - class Entity(@Suppress("unused") val value: OptionalSnowflake = OptionalSnowflake.Missing) - - val entity = Json.decodeFromString(json) + val entity = Json.decodeFromString(json) require(entity.value is OptionalSnowflake.Value) Assertions.assertEquals(Snowflake(5), entity.value.value) diff --git a/common/src/test/kotlin/entity/optional/OptionalTest.kt b/common/src/test/kotlin/entity/optional/OptionalTest.kt index b66e0d6f255..144a0387288 100644 --- a/common/src/test/kotlin/entity/optional/OptionalTest.kt +++ b/common/src/test/kotlin/entity/optional/OptionalTest.kt @@ -10,7 +10,7 @@ import org.junit.jupiter.api.Test internal class OptionalTest { @Test - fun `creating optional from nullable value returns Value on non-null value`(){ + fun `creating optional from nullable value returns Value on non-null value`() { val value: Int? = 5 val optional = Optional(value) @@ -19,62 +19,66 @@ internal class OptionalTest { } @Test - fun `creating optional from nullable value returns Null on null value`(){ + fun `creating optional from nullable value returns Null on null value`() { val value: Int? = null val optional = Optional(value) assert(optional is Optional.Null) } + + @Serializable + private class NullOptionalEntity(val value: Optional) + @Test - fun `deserializing null in nullable optional assigns Null`(){ + fun `deserializing null in nullable optional assigns Null`() { @Language("json") val json = """{ "value":null }""" - @Serializable - class Entity(val value: Optional) - - val entity = Json.decodeFromString(json) + val entity = Json.decodeFromString(json) assert(entity.value is Optional.Null) } + + @Serializable + class EmptyOptionalEntity(val value: Optional = Optional.Missing()) + @Test - fun `deserializing nothing in nullable optional assigns Missing`(){ + fun `deserializing nothing in nullable optional assigns Missing`() { @Language("json") val json = """{}""" - @Serializable - class Entity(val value: Optional = Optional.Missing()) - - val entity = Json.decodeFromString(json) + val entity = Json.decodeFromString(json) assert(entity.value is Optional.Missing) } + + @Serializable + class UnexpectedEmptyOptionalEntity(val value: Optional = Optional.Missing()) + @Test - fun `deserializing nothing in non-nullable optional assigns Missing`(){ + fun `deserializing nothing in non-nullable optional assigns Missing`() { @Language("json") val json = """{}""" - @Serializable - class Entity(val value: Optional = Optional.Missing()) - - val entity = Json.decodeFromString(json) + val entity = Json.decodeFromString(json) assert(entity.value is Optional.Missing) } + + @Serializable + private class UnexpectedNullOptionalEntity(@Suppress("unused") val value: Optional = Optional.Missing()) + @Test - fun `deserializing null in non-nullable optional throws SerializationException`(){ + fun `deserializing null in non-nullable optional throws SerializationException`() { @Language("json") val json = """{ "value":null }""" - @Serializable - class Entity(@Suppress("unused") val value: Optional = Optional.Missing()) - org.junit.jupiter.api.assertThrows { - Json.decodeFromString(json) + Json.decodeFromString(json) } } diff --git a/core/src/test/kotlin/regression/CacheMissRegression.kt b/core/src/test/kotlin/regression/CacheMissRegression.kt index 01a8bd89f40..e8678085b36 100644 --- a/core/src/test/kotlin/regression/CacheMissRegression.kt +++ b/core/src/test/kotlin/regression/CacheMissRegression.kt @@ -108,9 +108,7 @@ class CrashingHandler(val client: HttpClient) : RequestHandler { }.execute() - return parser.decodeFromString(request.route.strategy, response.readText()) - - + return request.route.mapper.deserialize(parser, response.readText()) } } diff --git a/gateway/src/test/kotlin/json/CommandTest.kt b/gateway/src/test/kotlin/json/CommandTest.kt index 167bc76d80b..25b9f8e7f87 100644 --- a/gateway/src/test/kotlin/json/CommandTest.kt +++ b/gateway/src/test/kotlin/json/CommandTest.kt @@ -2,6 +2,7 @@ package json +import dev.kord.common.entity.DiscordBotActivity import dev.kord.common.entity.DiscordShard import dev.kord.common.entity.PresenceStatus import dev.kord.common.entity.Snowflake @@ -61,7 +62,10 @@ class CommandTest { val query = "test" val limit = 1337 - val request = json.encodeToString(Command.Companion, RequestGuildMembers(Snowflake(guildId), query.optional(), OptionalInt.Value(limit))) + val request = json.encodeToString( + Command.Companion, + RequestGuildMembers(Snowflake(guildId), query.optional(), OptionalInt.Value(limit)) + ) val json = json.encodeToString(JsonObject.serializer(), buildJsonObject { put("op", OpCode.RequestGuildMembers.code) @@ -83,7 +87,10 @@ class CommandTest { val selfMute = true val selfDeaf = false - val status = json.encodeToString(Command.Companion, UpdateVoiceStatus(Snowflake(guildId), Snowflake(channelId), selfMute, selfDeaf)) + val status = json.encodeToString( + Command.Companion, + UpdateVoiceStatus(Snowflake(guildId), Snowflake(channelId), selfMute, selfDeaf) + ) val json = json.encodeToString(JsonObject.serializer(), buildJsonObject { put("op", OpCode.VoiceStateUpdate.code) @@ -102,7 +109,7 @@ class CommandTest { @Test fun `UpdateState command serialization`() { val since = 1242518400L - val game = null + val game = emptyList() val status = PresenceStatus.Online val afk = false @@ -112,7 +119,7 @@ class CommandTest { put("op", OpCode.StatusUpdate.code) put("d", buildJsonObject { put("since", since) - put("activities", null as String?) + put("activities", JsonArray(emptyList())) put("status", status.value.lowercase(Locale.getDefault())) put("afk", afk) }) @@ -133,8 +140,16 @@ class CommandTest { val presence: DiscordPresence? = null val identify = json.encodeToString( - Command.Companion, - Identify(token, properties, compress.optional(), largeThreshold.optionalInt(), shard.optional(), presence.optional().coerceToMissing(), Intents.all) + Command.Companion, + Identify( + token, + properties, + compress.optional(), + largeThreshold.optionalInt(), + shard.optional(), + presence.optional().coerceToMissing(), + Intents.all + ) ) val json = json.encodeToString(JsonObject.serializer(), buildJsonObject { diff --git a/rest/src/main/kotlin/json/OptionalSerializer.kt b/rest/src/main/kotlin/json/OptionalSerializer.kt deleted file mode 100644 index 207dd74b69c..00000000000 --- a/rest/src/main/kotlin/json/OptionalSerializer.kt +++ /dev/null @@ -1,35 +0,0 @@ -package dev.kord.rest.json - -import kotlinx.serialization.* -import kotlinx.serialization.descriptors.SerialDescriptor -import kotlinx.serialization.encoding.Decoder -import kotlinx.serialization.encoding.Encoder - -/** - * This is a very stupid serializer and you should feel ashamed for calling this. - * Essentially, there's a use case where a [dev.kord.rest.route.Route] may *sometimes* return - * a value, and sometimes nothing. - * - * `nullable` doesn't save you here since it'll expect at least something, and thus throws on an empty input. - * Thus this crime against control flow was born. This will try to serialize the type as if it wasn't null, and - * swallow any exceptions while doing so, returning null instead. This is incredibly bad because we won't propagate any - * actual bugs. - */ -@OptIn(ExperimentalSerializationApi::class) -internal val KSerializer.optional: KSerializer - get() = object : KSerializer { - - override val descriptor: SerialDescriptor - get() = this@optional.descriptor - - override fun deserialize(decoder: Decoder): T? = try { - decoder.decodeSerializableValue(this@optional) - } catch (e: Exception) { - null - } - - override fun serialize(encoder: Encoder, value: T?) { - if (value == null) return encoder.encodeNull() - else encoder.encodeSerializableValue(this@optional, value) - } - } \ No newline at end of file diff --git a/rest/src/main/kotlin/request/KtorRequestHandler.kt b/rest/src/main/kotlin/request/KtorRequestHandler.kt index a3d9643bd37..6ca19fcb799 100644 --- a/rest/src/main/kotlin/request/KtorRequestHandler.kt +++ b/rest/src/main/kotlin/request/KtorRequestHandler.kt @@ -1,8 +1,8 @@ package dev.kord.rest.request -import dev.kord.rest.json.optional import dev.kord.rest.json.response.DiscordErrorResponse import dev.kord.rest.ratelimit.* +import dev.kord.rest.route.optional import io.ktor.client.* import io.ktor.client.engine.cio.* import io.ktor.client.features.* @@ -61,13 +61,13 @@ class KtorRequestHandler( if (response.contentType() == ContentType.Application.Json) throw KtorRequestException( response, - parser.decodeFromString(DiscordErrorResponse.serializer().optional, body) + DiscordErrorResponse.serializer().optional.deserialize(parser, body) ) else throw KtorRequestException(response, null) } else -> { logger.debug { response.logString(body) } - parser.decodeFromString(request.route.strategy, body) + request.route.mapper.deserialize(parser, body) } } } diff --git a/rest/src/main/kotlin/route/Route.kt b/rest/src/main/kotlin/route/Route.kt index 704953ff6f5..ed4891dfa3d 100644 --- a/rest/src/main/kotlin/route/Route.kt +++ b/rest/src/main/kotlin/route/Route.kt @@ -4,7 +4,6 @@ import dev.kord.common.annotation.DeprecatedSinceKord import dev.kord.common.annotation.KordExperimental import dev.kord.common.annotation.KordPreview import dev.kord.common.entity.* -import dev.kord.rest.json.optional import dev.kord.rest.json.response.* import io.ktor.http.* import kotlinx.serialization.DeserializationStrategy @@ -15,21 +14,48 @@ import kotlinx.serialization.descriptors.SerialDescriptor import kotlinx.serialization.descriptors.StructureKind import kotlinx.serialization.descriptors.buildSerialDescriptor import kotlinx.serialization.encoding.Decoder +import kotlinx.serialization.json.Json import kotlinx.serialization.serializer import dev.kord.common.entity.DiscordEmoji as EmojiEntity internal const val REST_VERSION_PROPERTY_NAME = "dev.kord.rest.version" internal val restVersion get() = System.getenv(REST_VERSION_PROPERTY_NAME) ?: "v8" +sealed interface ResponseMapper { + fun deserialize(json: Json, body: String): T +} + +internal class ValueJsonMapper(val strategy: DeserializationStrategy) : ResponseMapper { + override fun deserialize(json: Json, body: String): T = json.decodeFromString(strategy, body) + override fun toString(): String = "ValueJsonMapper(strategy=$strategy)" +} + +internal class NullAwareMapper(val strategy: DeserializationStrategy) : ResponseMapper { + override fun deserialize(json: Json, body: String): T? { + if (body.isBlank()) return null + return json.decodeFromString(strategy, body) + } + + override fun toString(): String = "NullAwareMapper(strategy=$strategy)" +} + +internal val DeserializationStrategy.optional: ResponseMapper + get() = NullAwareMapper(this) + sealed class Route( val method: HttpMethod, val path: String, - val strategy: DeserializationStrategy + val mapper: ResponseMapper ) { + constructor( + method: HttpMethod, + path: String, + strategy: DeserializationStrategy + ) : this(method, path, ValueJsonMapper(strategy)) @OptIn(ExperimentalSerializationApi::class) override fun toString(): String = - "Route(method:${method.value},path:$path,strategy:${strategy.descriptor.serialName})" + "Route(method:${method.value},path:$path,mapper:$mapper)" object GatewayGet : Route(HttpMethod.Get, "/gateway", GatewayResponse.serializer()) @@ -425,7 +451,7 @@ sealed class Route( : Route(HttpMethod.Post, "/webhooks/$WebhookId/$WebhookToken/slack", NoStrategy) object ExecuteGithubWebhookPost - : Route(HttpMethod.Post, "/webhooks/$WebhookId/$WebhookToken", NoStrategy) + : Route(HttpMethod.Post, "/webhooks/$WebhookId/$WebhookToken/github", NoStrategy) object EditWebhookMessage : Route( HttpMethod.Patch, @@ -591,33 +617,33 @@ sealed class Route( @KordPreview object GuildApplicationCommandPermissionsGet : Route( - HttpMethod.Get, - "/applications/${ApplicationId}/guilds/$GuildId/commands/permissions", - DiscordGuildApplicationCommandPermissions.serializer() + HttpMethod.Get, + "/applications/${ApplicationId}/guilds/$GuildId/commands/permissions", + DiscordGuildApplicationCommandPermissions.serializer() ) @KordPreview object ApplicationCommandPermissionsGet : Route( - HttpMethod.Get, - "/applications/${ApplicationId}/guilds/$GuildId/commands/$CommandId/permissions", - DiscordGuildApplicationCommandPermissions.serializer() + HttpMethod.Get, + "/applications/${ApplicationId}/guilds/$GuildId/commands/$CommandId/permissions", + DiscordGuildApplicationCommandPermissions.serializer() ) @KordPreview object ApplicationCommandPermissionsPut : Route( - HttpMethod.Put, - "/applications/$ApplicationId/guilds/$GuildId/commands/$CommandId/permissions", - DiscordGuildApplicationCommandPermissions.serializer() + HttpMethod.Put, + "/applications/$ApplicationId/guilds/$GuildId/commands/$CommandId/permissions", + DiscordGuildApplicationCommandPermissions.serializer() ) @KordPreview object ApplicationCommandPermissionsBatchPut : Route>( - HttpMethod.Put, - "/applications/$ApplicationId/guilds/$GuildId/commands/permissions", - serializer() + HttpMethod.Put, + "/applications/$ApplicationId/guilds/$GuildId/commands/permissions", + serializer() ) object FollowupMessageCreate : Route(