Skip to content

Commit dc25ecb

Browse files
e5le5l
committed
KTOR-4511 Add String to default ignored types
1 parent 344be96 commit dc25ecb

File tree

14 files changed

+284
-72
lines changed

14 files changed

+284
-72
lines changed

‎ktor-client/ktor-client-plugins/ktor-client-content-negotiation/api/ktor-client-content-negotiation.api‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ public final class io/ktor/client/plugins/contentnegotiation/ContentNegotiation
88

99
public final class io/ktor/client/plugins/contentnegotiation/ContentNegotiation$Config : io/ktor/serialization/Configuration {
1010
public fun <init> ()V
11+
public final fun clearIgnoredTypes ()V
12+
public final fun getIgnoredTypes ()Ljava/util/Set;
1113
public final fun register (Lio/ktor/http/ContentType;Lio/ktor/serialization/ContentConverter;Lio/ktor/http/ContentTypeMatcher;Lkotlin/jvm/functions/Function1;)V
1214
public fun register (Lio/ktor/http/ContentType;Lio/ktor/serialization/ContentConverter;Lkotlin/jvm/functions/Function1;)V
1315
}

‎ktor-client/ktor-client-plugins/ktor-client-content-negotiation/common/src/io/ktor/client/plugins/contentnegotiation/ContentNegotiation.kt‎

Lines changed: 99 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,24 @@ import io.ktor.client.plugins.*
99
import io.ktor.client.request.*
1010
import io.ktor.client.statement.*
1111
import io.ktor.client.utils.*
12+
import io.ktor.client.utils.EmptyContent.contentType
1213
import io.ktor.http.*
1314
import io.ktor.http.content.*
1415
import io.ktor.serialization.*
1516
import io.ktor.util.*
17+
import io.ktor.util.reflect.*
1618
import io.ktor.utils.io.*
1719
import io.ktor.utils.io.charsets.*
1820
import kotlin.reflect.*
1921

22+
internal val DefaultCommonIgnoredTypes: Set<KClass<*>> = setOf(
23+
ByteArray::class,
24+
String::class,
25+
HttpStatusCode::class,
26+
ByteReadChannel::class,
27+
OutgoingContent::class
28+
)
29+
2030
internal expect val DefaultIgnoredTypes: Set<KClass<*>>
2131

2232
/**
@@ -28,7 +38,8 @@ internal expect val DefaultIgnoredTypes: Set<KClass<*>>
2838
* You can learn more from [Content negotiation and serialization](https://ktor.io/docs/serialization-client.html).
2939
*/
3040
public class ContentNegotiation internal constructor(
31-
internal val registrations: List<Config.ConverterRegistration>
41+
internal val registrations: List<Config.ConverterRegistration>,
42+
internal val ignoredTypes: Set<KClass<*>>
3243
) {
3344

3445
/**
@@ -39,9 +50,13 @@ public class ContentNegotiation internal constructor(
3950
internal class ConverterRegistration(
4051
val converter: ContentConverter,
4152
val contentTypeToSend: ContentType,
42-
val contentTypeMatcher: ContentTypeMatcher,
53+
val contentTypeMatcher: ContentTypeMatcher
4354
)
4455

56+
@PublishedApi
57+
internal val ignoredTypes: MutableSet<KClass<*>> =
58+
(DefaultIgnoredTypes + DefaultCommonIgnoredTypes).toMutableSet()
59+
4560
internal val registrations = mutableListOf<ConverterRegistration>()
4661

4762
/**
@@ -77,11 +92,84 @@ public class ContentNegotiation internal constructor(
7792
registrations.add(registration)
7893
}
7994

95+
/**
96+
* Adds a type to the list of types that should be ignored by [ContentNegotiation].
97+
*
98+
* The list contains the [HttpStatusCode], [ByteArray], [String] and streaming types by default.
99+
*/
100+
public inline fun <reified T> ignoreType() {
101+
ignoredTypes.add(T::class)
102+
}
103+
104+
/**
105+
* Remove [T] from the list of types that should be ignored by [ContentNegotiation].
106+
*/
107+
public inline fun <reified T> removeIgnoredType() {
108+
ignoredTypes.remove(T::class)
109+
}
110+
111+
/**
112+
* Clear all configured ignored types including defaults.
113+
*/
114+
public fun clearIgnoredTypes() {
115+
ignoredTypes.clear()
116+
}
117+
80118
private fun defaultMatcher(pattern: ContentType): ContentTypeMatcher = object : ContentTypeMatcher {
81119
override fun contains(contentType: ContentType): Boolean = contentType.match(pattern)
82120
}
83121
}
84122

123+
internal suspend fun convertRequest(request: HttpRequestBuilder, body: Any): Any? {
124+
registrations.forEach { request.accept(it.contentTypeToSend) }
125+
126+
if (body is OutgoingContent || ignoredTypes.any { it.isInstance(body) }) return null
127+
val contentType = request.contentType() ?: return null
128+
129+
if (body is Unit) {
130+
request.headers.remove(HttpHeaders.ContentType)
131+
return EmptyContent
132+
}
133+
134+
val matchingRegistrations = registrations.filter { it.contentTypeMatcher.contains(contentType) }
135+
.takeIf { it.isNotEmpty() } ?: return null
136+
if (request.bodyType == null) return null
137+
request.headers.remove(HttpHeaders.ContentType)
138+
139+
// Pick the first one that can convert the subject successfully
140+
val serializedContent = matchingRegistrations.firstNotNullOfOrNull { registration ->
141+
registration.converter.serializeNullable(
142+
contentType,
143+
contentType.charset() ?: Charsets.UTF_8,
144+
request.bodyType!!,
145+
body.takeIf { it != NullBody }
146+
)
147+
} ?: throw ContentConverterException(
148+
"Can't convert $body with contentType $contentType using converters " +
149+
matchingRegistrations.joinToString { it.converter.toString() }
150+
)
151+
152+
return serializedContent
153+
}
154+
155+
@OptIn(InternalAPI::class)
156+
internal suspend fun convertResponse(
157+
info: TypeInfo,
158+
body: Any,
159+
responseContentType: ContentType,
160+
charset: Charset = Charsets.UTF_8
161+
): Any? {
162+
if (body !is ByteReadChannel) return null
163+
if (info.type in ignoredTypes) return null
164+
165+
val suitableConverters = registrations
166+
.filter { it.contentTypeMatcher.contains(responseContentType) }
167+
.map { it.converter }
168+
.takeIf { it.isNotEmpty() } ?: return null
169+
170+
return suitableConverters.deserialize(body, info, charset)
171+
}
172+
85173
/**
86174
* A companion object used to install a plugin.
87175
*/
@@ -91,61 +179,22 @@ public class ContentNegotiation internal constructor(
91179

92180
override fun prepare(block: Config.() -> Unit): ContentNegotiation {
93181
val config = Config().apply(block)
94-
return ContentNegotiation(config.registrations)
182+
return ContentNegotiation(config.registrations, config.ignoredTypes)
95183
}
96184

97185
override fun install(plugin: ContentNegotiation, scope: HttpClient) {
98-
scope.requestPipeline.intercept(HttpRequestPipeline.Transform) { payload ->
99-
val registrations = plugin.registrations
100-
registrations.forEach { context.accept(it.contentTypeToSend) }
101-
102-
if (subject is OutgoingContent || DefaultIgnoredTypes.any { it.isInstance(payload) }) {
103-
return@intercept
104-
}
105-
val contentType = context.contentType() ?: return@intercept
106-
107-
if (payload is Unit) {
108-
context.headers.remove(HttpHeaders.ContentType)
109-
proceedWith(EmptyContent)
110-
return@intercept
111-
}
112-
113-
val matchingRegistrations = registrations.filter { it.contentTypeMatcher.contains(contentType) }
114-
.takeIf { it.isNotEmpty() } ?: return@intercept
115-
if (context.bodyType == null) return@intercept
116-
context.headers.remove(HttpHeaders.ContentType)
117-
118-
// Pick the first one that can convert the subject successfully
119-
val serializedContent = matchingRegistrations.firstNotNullOfOrNull { registration ->
120-
registration.converter.serializeNullable(
121-
contentType,
122-
contentType.charset() ?: Charsets.UTF_8,
123-
context.bodyType!!,
124-
payload.takeIf { it != NullBody }
125-
)
126-
} ?: throw ContentConverterException(
127-
"Can't convert $payload with contentType $contentType using converters " +
128-
matchingRegistrations.joinToString { it.converter.toString() }
129-
)
130-
131-
proceedWith(serializedContent)
186+
scope.requestPipeline.intercept(HttpRequestPipeline.Transform) {
187+
val result = plugin.convertRequest(context, subject) ?: return@intercept
188+
proceedWith(result)
132189
}
133190

134191
scope.responsePipeline.intercept(HttpResponsePipeline.Transform) { (info, body) ->
135-
if (body !is ByteReadChannel) return@intercept
136-
if (info.type == ByteReadChannel::class) return@intercept
137-
138192
val contentType = context.response.contentType() ?: return@intercept
139-
val registrations = plugin.registrations
140-
val suitableConverters = registrations
141-
.filter { it.contentTypeMatcher.contains(contentType) }
142-
.map { it.converter }
143-
.takeIf { it.isNotEmpty() } ?: return@intercept
144-
145-
@OptIn(InternalAPI::class)
146-
val parsedBody = suitableConverters.deserialize(body, info, context.request.headers.suitableCharset())
147-
val response = HttpResponseContainer(info, parsedBody)
148-
proceedWith(response)
193+
val charset = context.request.headers.suitableCharset()
194+
195+
val deserializedBody = plugin.convertResponse(info, body, contentType, charset) ?: return@intercept
196+
val result = HttpResponseContainer(info, deserializedBody)
197+
proceedWith(result)
149198
}
150199
}
151200
}

‎ktor-client/ktor-client-plugins/ktor-client-content-negotiation/common/test/io/ktor/client/plugins/ContentNegotiationTests.kt‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,11 @@ import io.ktor.client.statement.*
1212
import io.ktor.client.tests.utils.*
1313
import io.ktor.http.*
1414
import io.ktor.http.content.*
15+
import io.ktor.serialization.*
1516
import io.ktor.util.*
17+
import io.ktor.util.reflect.*
1618
import io.ktor.utils.io.*
19+
import io.ktor.utils.io.charsets.*
1720
import io.ktor.utils.io.core.*
1821
import kotlin.test.*
1922

@@ -47,7 +50,7 @@ class ContentNegotiationTests {
4750
val registeredTypesToSend = listOf(
4851
ContentType("testing", "a"),
4952
ContentType("testing", "b"),
50-
ContentType("testing", "c"),
53+
ContentType("testing", "c")
5154
)
5255

5356
setupWithContentNegotiation {
@@ -161,7 +164,7 @@ class ContentNegotiationTests {
161164
fun selectMatchingConverterInRequestPipeline(): Unit = testWithEngine(MockEngine) {
162165
val types = listOf(
163166
ContentType("testing", "a"),
164-
ContentType("testing", "b"),
167+
ContentType("testing", "b")
165168
)
166169

167170
val outgoingContentPerType = types.map { contentType ->
@@ -226,7 +229,7 @@ class ContentNegotiationTests {
226229
val contentTypeToSend = ContentType("testing", "client-to-send")
227230
val outgoingContents = listOf(
228231
TextContent("FIRST CONVERTER", contentTypeToSend),
229-
TextContent("SECOND CONVERTER", contentTypeToSend),
232+
TextContent("SECOND CONVERTER", contentTypeToSend)
230233
)
231234

232235
assertTrue(outgoingContents.size > 1, "Must have more than 1 content converter registered for this test")
@@ -257,7 +260,7 @@ class ContentNegotiationTests {
257260
val contentTypeToReceive = ContentType("testing", "client-to-receive")
258261
val deserializedValues = listOf(
259262
StringWrapper("FIRST CONVERTER"),
260-
StringWrapper("SECOND CONVERTER"),
263+
StringWrapper("SECOND CONVERTER")
261264
)
262265

263266
assertTrue(deserializedValues.size > 1, "Must have more than 1 converter registered for this test")
@@ -399,6 +402,8 @@ class ContentNegotiationTests {
399402
}
400403
}
401404

405+
406+
402407
object Thing
403408

404409
data class StringWrapper(val value: String)

‎ktor-client/ktor-client-plugins/ktor-client-content-negotiation/common/test/io/ktor/client/plugins/IgnoreTypesTest.kt‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
/*
2+
* Copyright 2014-2022 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
3+
*/
4+
5+
package io.ktor.client.plugins
6+
7+
import io.ktor.client.*
8+
import io.ktor.client.engine.mock.*
9+
import io.ktor.client.plugins.contentnegotiation.*
10+
import io.ktor.client.request.*
11+
import io.ktor.client.statement.*
12+
import io.ktor.client.utils.*
13+
import io.ktor.http.*
14+
import io.ktor.http.content.*
15+
import io.ktor.serialization.*
16+
import io.ktor.test.dispatcher.*
17+
import io.ktor.util.*
18+
import io.ktor.util.reflect.*
19+
import io.ktor.utils.io.*
20+
import io.ktor.utils.io.charsets.*
21+
import kotlin.test.*
22+
23+
class IgnoreTypesTest {
24+
val converter = object : ContentConverter {
25+
override suspend fun deserialize(charset: Charset, typeInfo: TypeInfo, content: ByteReadChannel): Any? {
26+
error("This should not be called")
27+
}
28+
29+
override suspend fun serializeNullable(
30+
contentType: ContentType,
31+
charset: Charset,
32+
typeInfo: TypeInfo,
33+
value: Any?
34+
): OutgoingContent? {
35+
error("This should not be called")
36+
}
37+
}
38+
39+
@OptIn(InternalAPI::class)
40+
val request = HttpRequestBuilder().apply {
41+
bodyType = typeInfo<String>()
42+
contentType(ContentType.Application.Json)
43+
}
44+
45+
@Test
46+
fun testTriesToConvertString() = testSuspend {
47+
val plugin = ContentNegotiation(
48+
registrations = listOf(
49+
ContentNegotiation.Config.ConverterRegistration(
50+
converter,
51+
ContentType.Any,
52+
object : ContentTypeMatcher {
53+
override fun contains(contentType: ContentType): Boolean = true
54+
}
55+
)
56+
),
57+
ignoredTypes = setOf()
58+
)
59+
60+
var message = assertFails {
61+
plugin.convertRequest(request, "Hello")
62+
}.message
63+
64+
assertEquals("This should not be called", message)
65+
66+
message = assertFails {
67+
plugin.convertResponse(typeInfo<String>(), ByteReadChannel("FOO"), ContentType.Application.Json)
68+
}.message
69+
70+
assertEquals("This should not be called", message)
71+
}
72+
73+
@Test
74+
fun testIgnoresString() = testSuspend {
75+
val plugin = ContentNegotiation(
76+
registrations = listOf(
77+
ContentNegotiation.Config.ConverterRegistration(
78+
converter,
79+
ContentType.Any,
80+
object : ContentTypeMatcher {
81+
override fun contains(contentType: ContentType): Boolean = true
82+
}
83+
)
84+
),
85+
ignoredTypes = setOf(String::class)
86+
)
87+
88+
assertNull(plugin.convertRequest(request, "Hello"))
89+
assertNull(plugin.convertResponse(typeInfo<String>(), ByteReadChannel.Empty, ContentType.Application.Json))
90+
}
91+
92+
@Test
93+
fun testRequestWithIgnoredString() = testSuspend {
94+
val jsonBody = "{\"foo\":\"bar\"}"
95+
96+
val client = HttpClient(MockEngine) {
97+
engine {
98+
addHandler { request ->
99+
assertEquals(jsonBody, request.body.toByteReadPacket().readText())
100+
respond(
101+
jsonBody,
102+
HttpStatusCode.OK,
103+
buildHeaders {
104+
append(HttpHeaders.ContentType, ContentType.Application.Json.toString())
105+
}
106+
)
107+
}
108+
}
109+
}
110+
111+
val response = client.get("/") {
112+
contentType(ContentType.Application.Json)
113+
setBody(jsonBody)
114+
}
115+
116+
assertEquals(jsonBody, response.bodyAsText())
117+
}
118+
}

‎ktor-client/ktor-client-plugins/ktor-client-content-negotiation/js/src/DefaultIgnoredTypesJs.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,4 @@ import io.ktor.utils.io.*
99
import kotlin.reflect.*
1010

1111
internal actual val DefaultIgnoredTypes: Set<KClass<*>> =
12-
mutableSetOf(OutgoingContent::class, ByteReadChannel::class, ByteArray::class)
12+
mutableSetOf()

0 commit comments

Comments
 (0)