Skip to content

Commit 38d12af

Browse files
mfabisiakmfabisiak
authored
Merge pull request #109 from AVSystem/fix/108-path-parameters-encoding
fix(core): use URL encoding for path parameters
2 parents 1725282 + 43b04f8 commit 38d12af

10 files changed

Lines changed: 1291 additions & 115 deletions

File tree

‎core/src/main/kotlin/com/avsystem/justworks/core/gen/Names.kt‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import com.squareup.kotlinpoet.MemberName
1010
val HTTP_CLIENT = ClassName("io.ktor.client", "HttpClient")
1111
val CONTENT_NEGOTIATION = ClassName("io.ktor.client.plugins.contentnegotiation", "ContentNegotiation")
1212
val HTTP_HEADERS = ClassName("io.ktor.http", "HttpHeaders")
13+
val ENCODE_URL_PATH_PART_FUN = MemberName("io.ktor.http", "encodeURLPathPart")
1314

1415
val JSON_FUN = MemberName("io.ktor.serialization.kotlinx.json", "json")
1516
val BODY_FUN = MemberName("io.ktor.client.call", "body")
@@ -59,6 +60,8 @@ val JSON_ENCODER = ClassName("kotlinx.serialization.json", "JsonEncoder")
5960
val SERIALIZERS_MODULE = ClassName("kotlinx.serialization.modules", "SerializersModule")
6061

6162
val JSON_OBJECT_EXT = MemberName("kotlinx.serialization.json", "jsonObject")
63+
val JSON_PRIMITIVE_EXT = MemberName("kotlinx.serialization.json", "jsonPrimitive")
64+
val ENCODE_TO_JSON_ELEMENT_FUN = MemberName("kotlinx.serialization.json", "encodeToJsonElement")
6265

6366
val K_SERIALIZER = ClassName("kotlinx.serialization", "KSerializer")
6467
val SERIAL_DESCRIPTOR = ClassName("kotlinx.serialization.descriptors", "SerialDescriptor")
@@ -68,7 +71,6 @@ val PRIMITIVE_KIND = ClassName("kotlinx.serialization.descriptors", "PrimitiveKi
6871
val DECODER = ClassName("kotlinx.serialization.encoding", "Decoder")
6972
val ENCODER = ClassName("kotlinx.serialization.encoding", "Encoder")
7073

71-
val ENCODE_TO_STRING_FUN = MemberName("kotlinx.serialization", "encodeToString")
7274
val POLYMORPHIC_FUN = MemberName("kotlinx.serialization.modules", "polymorphic")
7375
val SUBCLASS_FUN = MemberName("kotlinx.serialization.modules", "subclass")
7476

@@ -93,7 +95,6 @@ val EXPERIMENTAL_UUID_API = ClassName("kotlin.uuid", "ExperimentalUuidApi")
9395
val HTTP_ERROR = ClassName("com.avsystem.justworks", "HttpError")
9496
val HTTP_SUCCESS = ClassName("com.avsystem.justworks", "HttpSuccess")
9597
val HTTP_RESULT = ClassName("com.avsystem.justworks", "HttpResult")
96-
val DESERIALIZE_ERROR_BODY_FUN = MemberName("com.avsystem.justworks", "deserializeErrorBody")
9798

9899
// ============================================================================
99100
// Kotlin stdlib
@@ -104,6 +105,7 @@ val CLOSEABLE = ClassName("java.io", "Closeable")
104105
val IO_EXCEPTION = ClassName("java.io", "IOException")
105106
val HTTP_REQUEST_TIMEOUT_EXCEPTION = ClassName("io.ktor.client.plugins", "HttpRequestTimeoutException")
106107
val OPT_IN = ClassName("kotlin", "OptIn")
108+
val ENUM_CLASS = ClassName("kotlin", "Enum")
107109

108110
// ============================================================================
109111
// Shared client base (generated)
@@ -112,9 +114,10 @@ val OPT_IN = ClassName("kotlin", "OptIn")
112114
val API_CLIENT_BASE = ClassName("com.avsystem.justworks", "ApiClientBase")
113115
val HTTP_RESPONSE = ClassName("io.ktor.client.statement", "HttpResponse")
114116
val HTTP_REQUEST_BUILDER = ClassName("io.ktor.client.request", "HttpRequestBuilder")
115-
val TO_RESULT_FUN = MemberName("com.avsystem.justworks", "toResult")
116-
val TO_EMPTY_RESULT_FUN = MemberName("com.avsystem.justworks", "toEmptyResult")
117+
val BODY_AS_TEXT_FUN = MemberName("io.ktor.client.statement", "bodyAsText")
118+
val DECODE_FROM_STRING_FUN = MemberName("kotlinx.serialization", "decodeFromString")
117119
val ENCODE_PARAM_FUN = MemberName("com.avsystem.justworks", "encodeParam")
120+
val ENCODE_PATH_PARAM_FUN = MemberName("com.avsystem.justworks", "encodePathParam")
118121
val UUID_SERIALIZER = ClassName("com.avsystem.justworks", "UuidSerializer")
119122

120123
// ============================================================================
@@ -125,7 +128,15 @@ const val BASE_URL = "baseUrl"
125128
const val TOKEN = "token"
126129
const val CLIENT = "client"
127130
const val BODY = "body"
131+
const val JSON_PROPERTY = "json"
128132
const val APPLY_AUTH = "applyAuth"
129133
const val SAFE_CALL = "safeCall"
130134
const val CREATE_HTTP_CLIENT = "createHttpClient"
131135
const val GENERATED_SERIALIZERS_MODULE = "generatedSerializersModule"
136+
137+
// toResult/toRawResult/toEmptyResult/deserializeErrorBody are members of ApiClientBase (they need
138+
// access to its `json` property), so call sites reference them by plain name — no import required.
139+
const val TO_RESULT_FUN = "toResult"
140+
const val TO_RAW_RESULT_FUN = "toRawResult"
141+
const val TO_EMPTY_RESULT_FUN = "toEmptyResult"
142+
const val DESERIALIZE_ERROR_BODY_FUN = "deserializeErrorBody"

‎core/src/main/kotlin/com/avsystem/justworks/core/gen/Utils.kt‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,14 @@ internal fun TypeRef.toTypeName(): TypeName = when (this) {
8080

8181
internal fun TypeRef.isBinaryUpload(): Boolean = this is TypeRef.Primitive && this.type == PrimitiveType.BYTE_ARRAY
8282

83+
internal fun TypeRef.containsUuid(): Boolean = when (this) {
84+
is TypeRef.Primitive -> type == PrimitiveType.UUID
85+
is TypeRef.Array -> items.containsUuid()
86+
is TypeRef.Map -> valueType.containsUuid()
87+
is TypeRef.Inline -> properties.any { it.type.containsUuid() }
88+
is TypeRef.Reference, is TypeRef.InlineEnum, TypeRef.Unknown -> false
89+
}
90+
8391
/**
8492
* Resolves the @SerialName value for a variant within a oneOf schema.
8593
*/

‎core/src/main/kotlin/com/avsystem/justworks/core/gen/client/BodyGenerator.kt‎

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import com.avsystem.justworks.core.gen.CONTENT_TYPE_APPLICATION
88
import com.avsystem.justworks.core.gen.CONTENT_TYPE_FUN
99
import com.avsystem.justworks.core.gen.DELETE_FUN
1010
import com.avsystem.justworks.core.gen.ENCODE_PARAM_FUN
11+
import com.avsystem.justworks.core.gen.ENCODE_PATH_PARAM_FUN
12+
import com.avsystem.justworks.core.gen.ENCODE_URL_PATH_PART_FUN
1113
import com.avsystem.justworks.core.gen.FORM_DATA_FUN
1214
import com.avsystem.justworks.core.gen.GET_FUN
1315
import com.avsystem.justworks.core.gen.HEADERS_CLASS
@@ -26,6 +28,7 @@ import com.avsystem.justworks.core.gen.SET_BODY_FUN
2628
import com.avsystem.justworks.core.gen.SUBMIT_FORM_FUN
2729
import com.avsystem.justworks.core.gen.SUBMIT_FORM_WITH_BINARY_DATA_FUN
2830
import com.avsystem.justworks.core.gen.TO_EMPTY_RESULT_FUN
31+
import com.avsystem.justworks.core.gen.TO_RAW_RESULT_FUN
2932
import com.avsystem.justworks.core.gen.TO_RESULT_FUN
3033
import com.avsystem.justworks.core.gen.isBinaryUpload
3134
import com.avsystem.justworks.core.gen.properties
@@ -39,7 +42,9 @@ import com.avsystem.justworks.core.model.Parameter
3942
import com.avsystem.justworks.core.model.ParameterLocation
4043
import com.avsystem.justworks.core.model.PrimitiveType
4144
import com.avsystem.justworks.core.model.TypeRef
45+
import com.squareup.kotlinpoet.BYTE_ARRAY
4246
import com.squareup.kotlinpoet.CodeBlock
47+
import com.squareup.kotlinpoet.STRING
4348
import com.squareup.kotlinpoet.TypeName
4449
import com.squareup.kotlinpoet.UNIT
4550

@@ -48,8 +53,9 @@ internal object BodyGenerator {
4853
endpoint: Endpoint,
4954
params: Map<ParameterLocation, List<Parameter>>,
5055
returnBodyType: TypeName,
56+
responseContentType: ContentType?,
5157
): CodeBlock {
52-
val resultFun = if (returnBodyType == UNIT) TO_EMPTY_RESULT_FUN else TO_RESULT_FUN
58+
val resultFun = resolveResultFun(returnBodyType, responseContentType)
5359
val code = CodeBlock.builder()
5460

5561
code.beginControlFlow("return $SAFE_CALL")
@@ -73,14 +79,21 @@ internal object BodyGenerator {
7379
}
7480
}
7581

76-
// Close the HTTP call block and chain .toResult() / .toEmptyResult()
82+
// Close the HTTP call block and chain .toResult() / .toRawResult() / .toEmptyResult()
7783
code.unindent()
78-
code.add("}.%M()\n", resultFun)
84+
code.add("}.$resultFun()\n")
7985
code.endControlFlow() // safeCall
8086

8187
return code.build()
8288
}
8389

90+
private fun resolveResultFun(returnBodyType: TypeName, responseContentType: ContentType?): String = when {
91+
returnBodyType == UNIT -> TO_EMPTY_RESULT_FUN
92+
returnBodyType == BYTE_ARRAY -> TO_RAW_RESULT_FUN
93+
returnBodyType == STRING && responseContentType != ContentType.JSON_CONTENT_TYPE -> TO_RAW_RESULT_FUN
94+
else -> TO_RESULT_FUN
95+
}
96+
8497
private fun CodeBlock.Builder.buildJsonBody(
8598
endpoint: Endpoint,
8699
params: Map<ParameterLocation, List<Parameter>>,
@@ -222,11 +235,27 @@ internal object BodyGenerator {
222235
}
223236
}
224237

238+
// These types are always JSON-primitive-safe but live outside the shared ApiClientBase.kt
239+
// overload set (String/Number/Boolean/Enum<T>) so that referencing them doesn't force every
240+
// generated client to depend on kotlinx-datetime or opt into ExperimentalUuidApi. Their values
241+
// are stringified directly at the call site instead of going through encodeParam/encodePathParam.
242+
private val CALLSITE_TO_STRING_TYPES = setOf(PrimitiveType.UUID, PrimitiveType.DATE_TIME, PrimitiveType.DATE)
243+
244+
private fun Parameter.needsCallsiteToString(): Boolean =
245+
(schema as? TypeRef.Primitive)?.type in CALLSITE_TO_STRING_TYPES
246+
225247
private fun buildUrlString(endpoint: Endpoint, params: Map<ParameterLocation, List<Parameter>>): CodeBlock {
226248
val (format, args) = params[ParameterLocation.PATH]
227249
.orEmpty()
228250
.fold($$"${%L}" + endpoint.path to listOf<Any>(BASE_URL)) { (format, args), param ->
229-
format.replace("{${param.name}}", $$"${%M(%L)}") to args + ENCODE_PARAM_FUN + param.name.toCamelCase()
251+
val paramName = param.name.toCamelCase()
252+
val (placeholder, newArgs) = if (param.needsCallsiteToString()) {
253+
$$"${%L.toString().%M()}" to listOf(paramName, ENCODE_URL_PATH_PART_FUN)
254+
} else {
255+
$$"${%M(%L)}" to listOf(ENCODE_PATH_PARAM_FUN, paramName)
256+
}
257+
val newFormat = format.replace("{${param.name}}", placeholder)
258+
newFormat to args + newArgs
230259
}
231260
return CodeBlock.of("%P", CodeBlock.of(format, *args.toTypedArray<Any>()))
232261
}
@@ -238,7 +267,11 @@ internal object BodyGenerator {
238267
for (param in headerParams) {
239268
val paramName = param.name.toCamelCase()
240269
optionalGuard(param.required, paramName) {
241-
addStatement("append(%S, %M(%L))", param.name, ENCODE_PARAM_FUN, paramName)
270+
if (param.needsCallsiteToString()) {
271+
addStatement("append(%S, %L.toString())", param.name, paramName)
272+
} else {
273+
addStatement("append(%S, %M(%L))", param.name, ENCODE_PARAM_FUN, paramName)
274+
}
242275
}
243276
}
244277
endControlFlow()
@@ -252,7 +285,11 @@ internal object BodyGenerator {
252285
for (param in queryParams) {
253286
val paramName = param.name.toCamelCase()
254287
optionalGuard(param.required, paramName) {
255-
addStatement("this.parameters.append(%S, %M(%L))", param.name, ENCODE_PARAM_FUN, paramName)
288+
if (param.needsCallsiteToString()) {
289+
addStatement("this.parameters.append(%S, %L.toString())", param.name, paramName)
290+
} else {
291+
addStatement("this.parameters.append(%S, %M(%L))", param.name, ENCODE_PARAM_FUN, paramName)
292+
}
256293
}
257294
}
258295
endControlFlow()

‎core/src/main/kotlin/com/avsystem/justworks/core/gen/client/ClientGenerator.kt‎

Lines changed: 74 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import com.avsystem.justworks.core.gen.BASE64_CLASS
77
import com.avsystem.justworks.core.gen.BASE_URL
88
import com.avsystem.justworks.core.gen.CLIENT
99
import com.avsystem.justworks.core.gen.CREATE_HTTP_CLIENT
10+
import com.avsystem.justworks.core.gen.EXPERIMENTAL_UUID_API
1011
import com.avsystem.justworks.core.gen.GENERATED_SERIALIZERS_MODULE
1112
import com.avsystem.justworks.core.gen.HEADERS_FUN
1213
import com.avsystem.justworks.core.gen.HTTP_CLIENT
@@ -15,23 +16,31 @@ import com.avsystem.justworks.core.gen.HTTP_REQUEST_BUILDER
1516
import com.avsystem.justworks.core.gen.HTTP_RESULT
1617
import com.avsystem.justworks.core.gen.HTTP_SUCCESS
1718
import com.avsystem.justworks.core.gen.Hierarchy
19+
import com.avsystem.justworks.core.gen.JSON_CLASS
1820
import com.avsystem.justworks.core.gen.JSON_ELEMENT
21+
import com.avsystem.justworks.core.gen.JSON_PROPERTY
1922
import com.avsystem.justworks.core.gen.NameRegistry
23+
import com.avsystem.justworks.core.gen.OPT_IN
2024
import com.avsystem.justworks.core.gen.OutputOptions
2125
import com.avsystem.justworks.core.gen.TOKEN
2226
import com.avsystem.justworks.core.gen.client.BodyGenerator.buildFunctionBody
2327
import com.avsystem.justworks.core.gen.client.ParametersGenerator.buildBodyParams
2428
import com.avsystem.justworks.core.gen.client.ParametersGenerator.buildNullableParameter
29+
import com.avsystem.justworks.core.gen.containsUuid
2530
import com.avsystem.justworks.core.gen.invoke
2631
import com.avsystem.justworks.core.gen.shared.toAuthParam
2732
import com.avsystem.justworks.core.gen.toCamelCase
2833
import com.avsystem.justworks.core.gen.toPascalCase
2934
import com.avsystem.justworks.core.gen.toTypeName
3035
import com.avsystem.justworks.core.model.ApiKeyLocation
3136
import com.avsystem.justworks.core.model.ApiSpec
37+
import com.avsystem.justworks.core.model.ContentType
3238
import com.avsystem.justworks.core.model.Endpoint
3339
import com.avsystem.justworks.core.model.ParameterLocation
40+
import com.avsystem.justworks.core.model.Response
3441
import com.avsystem.justworks.core.model.SecurityScheme
42+
import com.squareup.kotlinpoet.AnnotationSpec
43+
import com.squareup.kotlinpoet.BYTE_ARRAY
3544
import com.squareup.kotlinpoet.ClassName
3645
import com.squareup.kotlinpoet.CodeBlock
3746
import com.squareup.kotlinpoet.FileSpec
@@ -74,12 +83,7 @@ internal object ClientGenerator {
7483
val simpleName = "${options.apiClassPrefix}${tag.toPascalCase()}${options.apiClassSuffix}"
7584
val className = ClassName(apiPackage, nameRegistry.register(simpleName))
7685

77-
val clientInitializer = if (hasPolymorphicTypes) {
78-
val generatedSerializersModule = MemberName(hierarchy.modelPackage, GENERATED_SERIALIZERS_MODULE)
79-
CodeBlock.of("${CREATE_HTTP_CLIENT}(%M)", generatedSerializersModule)
80-
} else {
81-
CodeBlock.of("${CREATE_HTTP_CLIENT}()")
82-
}
86+
val clientInitializer = CodeBlock.of("${CREATE_HTTP_CLIENT}()")
8387

8488
val tokenType = LambdaTypeName.get(returnType = STRING)
8589
val isSingleBearer = securitySchemes.singleOrNull() is SecurityScheme.Bearer
@@ -93,6 +97,15 @@ internal object ClientGenerator {
9397
.superclass(API_CLIENT_BASE)
9498
.addSuperclassConstructorParameter(BASE_URL)
9599

100+
if (hasPolymorphicTypes) {
101+
val generatedSerializersModule = MemberName(hierarchy.modelPackage, GENERATED_SERIALIZERS_MODULE)
102+
classBuilder.addSuperclassConstructorParameter(
103+
"$JSON_PROPERTY = %T { serializersModule = %M }",
104+
JSON_CLASS,
105+
generatedSerializersModule,
106+
)
107+
}
108+
96109
if (isSingleBearer) {
97110
// Single Bearer: use plain "token" param name for ergonomics
98111
constructorBuilder.addParameter(TOKEN, tokenType)
@@ -143,10 +156,28 @@ internal object ClientGenerator {
143156
classBuilder.addFunctions(endpoints.map { generateEndpointFunction(it) })
144157
}
145158

146-
return FileSpec
147-
.builder(className)
148-
.addType(classBuilder.build())
149-
.build()
159+
val fileBuilder = FileSpec.builder(className).addType(classBuilder.build())
160+
if (endpoints.usesUuid()) {
161+
fileBuilder.addAnnotation(
162+
AnnotationSpec
163+
.builder(OPT_IN)
164+
.addMember("%T::class", EXPERIMENTAL_UUID_API)
165+
.build(),
166+
)
167+
}
168+
return fileBuilder.build()
169+
}
170+
171+
// A Uuid-typed path/query/header param, request body, or response schema anywhere in this tag
172+
// group means the generated function signatures reference kotlin.uuid.Uuid directly, which
173+
// requires this file to opt into ExperimentalUuidApi (mirrors ModelGenerator's per-model check).
174+
private fun List<Endpoint>.usesUuid(): Boolean = any { endpoint ->
175+
val responseRefs = endpoint.responses.values
176+
.asSequence()
177+
.mapNotNull { it.schema }
178+
val requestRef = endpoint.requestBody?.schema
179+
val parameterRefs = endpoint.parameters.asSequence().map { it.schema }
180+
(responseRefs + listOfNotNull(requestRef) + parameterRefs).any { it.containsUuid() }
150181
}
151182

152183
private fun buildApplyAuth(
@@ -224,6 +255,7 @@ internal object ClientGenerator {
224255
private fun generateEndpointFunction(endpoint: Endpoint): FunSpec {
225256
val functionName = methodRegistry.register(endpoint.operationId.toCamelCase())
226257
val returnBodyType = resolveReturnType(endpoint)
258+
val responseContentType = resolveSuccessResponse(endpoint)?.contentType
227259
val errorType = resolveErrorType(endpoint)
228260
val returnType = HTTP_RESULT.parameterizedBy(errorType, returnBodyType)
229261
@@ -273,7 +305,7 @@ internal object ClientGenerator {
273305
}
274306
}
275307
276-
funBuilder.addCode(buildFunctionBody(endpoint, params, returnBodyType))
308+
funBuilder.addCode(buildFunctionBody(endpoint, params, returnBodyType, responseContentType))
277309
278310
return funBuilder.build()
279311
}
@@ -296,15 +328,41 @@ internal object ClientGenerator {
296328
297329
context(_: Hierarchy)
298330
private fun resolveReturnType(endpoint: Endpoint): TypeName {
299-
val twoXxSchema = endpoint.responses
331+
val response = resolveSuccessResponse(endpoint) ?: return UNIT
332+
val schemaType = response.schema?.toTypeName() ?: return UNIT
333+
334+
// The declared schema type isn't always the type that can actually be decoded off the
335+
// wire for a given content type, so it's overridden with whatever IS a faithful, safely
336+
// decodable representation of that content type — rather than either forcing a decode
337+
// that throws on every call, or failing generation over a spec inconsistency:
338+
// - application/json: a `{type: string, format: byte}` (ByteArray) schema is a base64
339+
// *string* on the wire, not a JSON byte array — kotlinx.serialization's built-in
340+
// ByteArraySerializer can't decode it. Surface the (still base64-encoded) String as-is.
341+
// - text/plain is always raw text, so String is always a faithful representation of it,
342+
// regardless of what the schema claims (e.g. `type: integer`) — body<String>() always
343+
// works, and a caller wanting the parsed type can convert it themselves.
344+
// - application/octet-stream is arbitrary binary, not necessarily valid UTF-8 text, so
345+
// ByteArray is the only safe universal representation — never downgrade this one to
346+
// String, unlike text/plain, since that risks throwing or corrupting non-UTF8 bytes.
347+
return when {
348+
schemaType == BYTE_ARRAY && response.contentType == ContentType.JSON_CONTENT_TYPE -> STRING
349+
response.contentType == ContentType.TEXT_PLAIN -> STRING
350+
response.contentType == ContentType.OCTET_STREAM -> BYTE_ARRAY
351+
else -> schemaType
352+
}
353+
}
354+
355+
// The response whose schema/contentType determine the endpoint's return type: the first 2xx
356+
// response with a schema, or (only when there's no 2xx response at all) the default response.
357+
private fun resolveSuccessResponse(endpoint: Endpoint): Response? {
358+
val twoXxResponse = endpoint.responses.entries
300359
.asSequence()
301360
.filter { it.key.startsWith("2") }
302-
.firstNotNullOfOrNull { it.value.schema }
361+
.map { it.value }
362+
.firstOrNull { it.schema != null }
303363
304-
val schema = twoXxSchema ?: endpoint.responses["default"]?.schema.takeIf {
364+
return twoXxResponse ?: endpoint.responses["default"]?.takeIf {
305365
endpoint.responses.none { it.key.startsWith("2") }
306366
}
307-
308-
return schema?.toTypeName() ?: UNIT
309367
}
310368
}

0 commit comments

Comments
 (0)