Merge pull request #123 from AVSystem/feat/injectable-http-client

feat(api): enable injection of a preconfigured HttpClient for shared engine and plugin usage

Author: mfabisiak

README.md

@@ -338,20 +338,45 @@ justworks {
 }
 ```
 
-### Ktor Engine
+### Ktor Engine and HttpClient
 
-The generated `HttpClient` is created without an explicit engine, so Ktor uses whichever engine is on the classpath.
-Switch engines by changing your dependency:
+By default, each generated client creates its own `HttpClient` without an explicit engine, so Ktor uses whichever
+engine is on the classpath, with that engine's default settings:
 
 ```kotlin
 dependencies {
-    // Pick one:
-    implementation("io.ktor:ktor-client-cio:3.1.1")     // CIO (default, pure Kotlin)
-    implementation("io.ktor:ktor-client-okhttp:3.1.1")   // OkHttp
-    implementation("io.ktor:ktor-client-apache:3.1.1")   // Apache
+    implementation("io.ktor:ktor-client-cio:3.1.1")   // or ktor-client-apache5, ktor-client-java, ...
 }
 ```
 
+To control the engine, connection pool, timeouts or plugins, pass your own `HttpClient`. The generated client derives
+its client from yours with `HttpClient.config {}`, so every API client built from the same `HttpClient` shares one
+engine (and its connection pool) and the plugins you installed:
+
+```kotlin
+val http = HttpClient(Apache5) {
+    engine {
+        connectTimeout = 2_000
+        socketTimeout = 8_000
+        configureConnectionManager {
+            setMaxConnTotal(500)
+            setMaxConnPerRoute(500)
+        }
+    }
+    install(HttpTimeout) { requestTimeoutMillis = 10_000 }
+}
+
+val pets = PetsApi("https://api.example.com", http) { tokenStore.getAccessToken() }
+val store = StoreApi("https://api.example.com", http) { tokenStore.getAccessToken() }
+```
+
+- The generated client always installs its own `ContentNegotiation` with the generated `Json` and sets
+  `expectSuccess = false`; response mapping relies on both. Don't install `ContentNegotiation` in the client you pass
+  in: plugin configurations are merged, and your JSON converter could take over decoding without the generated
+  `serializersModule`.
+- Closing a generated client does not close the `HttpClient` you passed in. Close it yourself once every API client
+  built from it is done.
+
 ### JSON Configuration
 
 The internal `Json` instance uses default settings (`ignoreUnknownKeys = false`, `isLenient = false`). If you need a
@@ -388,6 +413,8 @@ dependencies {
 
 Each generated client extends `ApiClientBase` and creates its own pre-configured `HttpClient` internally.
 You only need to provide the base URL and authentication credentials (if the spec defines security schemes).
+To share an engine, connection pool or plugins across clients, pass your own `HttpClient` as the second argument —
+see [Ktor Engine and HttpClient](#ktor-engine-and-httpclient).
 
 Class names are derived from OpenAPI tags as `<Tag>Api` (e.g., a `pets` tag produces `PetsApi`). Untagged endpoints go
 to `DefaultApi`.

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

@@ -8,6 +8,7 @@ import com.squareup.kotlinpoet.MemberName
 // ============================================================================
 
 val HTTP_CLIENT = ClassName("io.ktor.client", "HttpClient")
+val HTTP_CLIENT_CONFIG = ClassName("io.ktor.client", "HttpClientConfig")
 val CONTENT_NEGOTIATION = ClassName("io.ktor.client.plugins.contentnegotiation", "ContentNegotiation")
 val HTTP_HEADERS = ClassName("io.ktor.http", "HttpHeaders")
 val ENCODE_URL_PATH_PART_FUN = MemberName("io.ktor.http", "encodeURLPathPart")
@@ -127,6 +128,7 @@ val UUID_SERIALIZER = ClassName("com.avsystem.justworks", "UuidSerializer")
 const val BASE_URL = "baseUrl"
 const val TOKEN = "token"
 const val CLIENT = "client"
+const val HTTP_CLIENT_PARAM = "httpClient"
 const val BODY = "body"
 const val JSON_PROPERTY = "json"
 const val APPLY_AUTH = "applyAuth"

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

@@ -11,6 +11,7 @@ import com.avsystem.justworks.core.gen.EXPERIMENTAL_UUID_API
 import com.avsystem.justworks.core.gen.GENERATED_SERIALIZERS_MODULE
 import com.avsystem.justworks.core.gen.HEADERS_FUN
 import com.avsystem.justworks.core.gen.HTTP_CLIENT
+import com.avsystem.justworks.core.gen.HTTP_CLIENT_PARAM
 import com.avsystem.justworks.core.gen.HTTP_HEADERS
 import com.avsystem.justworks.core.gen.HTTP_REQUEST_BUILDER
 import com.avsystem.justworks.core.gen.HTTP_RESULT
@@ -87,14 +88,26 @@ internal object ClientGenerator {
         val classSimpleName = if (options.generateInterfaces) "${baseName}Impl" else baseName
         val className = ClassName(apiPackage, nameRegistry.register(classSimpleName))
 
-        val clientInitializer = CodeBlock.of("${CREATE_HTTP_CLIENT}()")
+        val clientInitializer = CodeBlock.of("$CREATE_HTTP_CLIENT($HTTP_CLIENT_PARAM)")
 
         val tokenType = LambdaTypeName.get(returnType = STRING)
         val isSingleBearer = securitySchemes.singleOrNull() is SecurityScheme.Bearer
 
+        val httpClientParam = ParameterSpec
+            .builder(HTTP_CLIENT_PARAM, HTTP_CLIENT.copy(nullable = true))
+            .defaultValue("null")
+        if (options.generateKdoc) {
+            httpClientParam.addKdoc(
+                "client to derive this API client from, sharing its engine and plugins; `null` creates a " +
+                    "standalone client. Closing this API client does not close [%L].",
+                HTTP_CLIENT_PARAM,
+            )
+        }
+
         val constructorBuilder = FunSpec
             .constructorBuilder()
             .addParameter(BASE_URL, STRING)
+            .addParameter(httpClientParam.build())
 
         val classBuilder = TypeSpec
             .classBuilder(className)

core/src/main/kotlin/com/avsystem/justworks/core/gen/shared/ApiClientBaseGenerator.kt

@@ -19,6 +19,8 @@ import com.avsystem.justworks.core.gen.ENCODE_TO_JSON_ELEMENT_FUN
 import com.avsystem.justworks.core.gen.ENCODE_URL_PATH_PART_FUN
 import com.avsystem.justworks.core.gen.ENUM_CLASS
 import com.avsystem.justworks.core.gen.HTTP_CLIENT
+import com.avsystem.justworks.core.gen.HTTP_CLIENT_CONFIG
+import com.avsystem.justworks.core.gen.HTTP_CLIENT_PARAM
 import com.avsystem.justworks.core.gen.HTTP_ERROR
 import com.avsystem.justworks.core.gen.HTTP_REQUEST_BUILDER
 import com.avsystem.justworks.core.gen.HTTP_REQUEST_TIMEOUT_EXCEPTION
@@ -44,6 +46,7 @@ import com.squareup.kotlinpoet.NUMBER
 import com.squareup.kotlinpoet.ParameterSpec
 import com.squareup.kotlinpoet.ParameterizedTypeName.Companion.parameterizedBy
 import com.squareup.kotlinpoet.PropertySpec
+import com.squareup.kotlinpoet.STAR
 import com.squareup.kotlinpoet.STRING
 import com.squareup.kotlinpoet.TypeName
 import com.squareup.kotlinpoet.TypeSpec
@@ -322,15 +325,29 @@ internal object ApiClientBaseGenerator {
             .build()
     }
 
-    private fun buildCreateHttpClient(): FunSpec = FunSpec
-        .builder(CREATE_HTTP_CLIENT)
-        .addModifiers(KModifier.PROTECTED)
-        .returns(HTTP_CLIENT)
-        .beginControlFlow("return %T", HTTP_CLIENT)
-        .beginControlFlow("install(%T)", CONTENT_NEGOTIATION)
-        .addStatement("%M(%L)", JSON_FUN, JSON_PROPERTY)
-        .endControlFlow()
-        .addStatement("expectSuccess = false")
-        .endControlFlow()
-        .build()
+    // A caller-supplied client is derived via `config {}`
+    private fun buildCreateHttpClient(): FunSpec {
+        val configType = LambdaTypeName.get(
+            receiver = HTTP_CLIENT_CONFIG.parameterizedBy(STAR),
+            returnType = UNIT,
+        )
+        val baseClient = ParameterSpec
+            .builder(HTTP_CLIENT_PARAM, HTTP_CLIENT.copy(nullable = true))
+            .defaultValue("null")
+            .build()
+
+        return FunSpec
+            .builder(CREATE_HTTP_CLIENT)
+            .addModifiers(KModifier.PROTECTED)
+            .addParameter(baseClient)
+            .returns(HTTP_CLIENT)
+            .beginControlFlow("val defaults: %T =", configType)
+            .beginControlFlow("install(%T)", CONTENT_NEGOTIATION)
+            .addStatement("%M(%L)", JSON_FUN, JSON_PROPERTY)
+            .endControlFlow()
+            .addStatement("expectSuccess = false")
+            .endControlFlow()
+            .addStatement("return %L?.config(defaults) ?: %T(defaults)", HTTP_CLIENT_PARAM, HTTP_CLIENT)
+            .build()
+    }
 }

core/src/test/kotlin/com/avsystem/justworks/core/gen/ApiClientBaseGeneratorTest.kt

@@ -99,17 +99,30 @@ class ApiClientBaseGeneratorTest {
     }
 
     @Test
-    fun `ApiClientBase has createHttpClient function with no parameters`() {
+    fun `ApiClientBase has createHttpClient function taking an optional base client`() {
         val create = classSpec.funSpecs.first { it.name == "createHttpClient" }
         assertTrue(KModifier.PROTECTED in create.modifiers)
-        assertTrue(
-            create.parameters.isEmpty(),
-            "createHttpClient should take no parameters; json is a constructor property now",
-        )
+        val param = create.parameters.single()
+        assertEquals("httpClient", param.name)
+        assertEquals("io.ktor.client.HttpClient?", param.type.toString())
+        assertEquals("null", param.defaultValue.toString(), "Expected httpClient to default to null")
         val body = create.body.toString()
         assertTrue(body.contains("ContentNegotiation"), "Expected ContentNegotiation install")
         assertTrue(body.contains("json(json)"), "Expected the shared json property installed into ContentNegotiation")
-        assertTrue(body.contains("expectSuccess"), "Expected expectSuccess = false")
+        assertTrue(body.contains("expectSuccess = false"), "Expected expectSuccess = false")
+    }
+
+    @Test
+    fun `createHttpClient derives from the base client via config and falls back to a standalone client`() {
+        val body = classFun("createHttpClient").body.toString()
+        assertTrue(
+            body.contains("httpClient?.config(defaults)"),
+            "Expected a base client to be derived with config(), sharing its engine, got: $body",
+        )
+        assertTrue(
+            body.contains("?: io.ktor.client.HttpClient(defaults)"),
+            "Expected a standalone client when no base client is given, got: $body",
+        )
     }
 
     // -- json property: constructor-injected val --

core/src/test/kotlin/com/avsystem/justworks/core/gen/ClientGeneratorTest.kt

@@ -433,14 +433,72 @@ class ClientGeneratorTest {
         assertEquals("kotlin.String", baseUrl.type.toString())
     }
 
-    // -- No security: constructor has only baseUrl --
+    // -- No security: constructor has only baseUrl and httpClient --
 
     @Test
-    fun `no security schemes generates constructor with only baseUrl`() {
+    fun `no security schemes generates constructor with only baseUrl and httpClient`() {
         val cls = clientClass(endpoint())
         val constructor = assertNotNull(cls.primaryConstructor)
         val paramNames = constructor.parameters.map { it.name }
-        assertEquals(listOf("baseUrl"), paramNames)
+        assertEquals(listOf("baseUrl", "httpClient"), paramNames)
+    }
+
+    // -- Injectable HttpClient --
+
+    @Test
+    fun `constructor takes a nullable httpClient defaulting to null`() {
+        val cls = clientClass(endpoint())
+        val httpClient = assertNotNull(cls.primaryConstructor).parameters.first { it.name == "httpClient" }
+        assertEquals("io.ktor.client.HttpClient?", httpClient.type.toString())
+        assertEquals("null", httpClient.defaultValue.toString())
+    }
+
+    @Test
+    fun `httpClient precedes auth params so a trailing token lambda still binds to the token`() {
+        val bearer = clientClass(listOf(endpoint()), listOf(SecurityScheme.Bearer("BearerAuth")))
+        assertEquals(
+            listOf("baseUrl", "httpClient", "token"),
+            assertNotNull(bearer.primaryConstructor).parameters.map { it.name },
+        )
+
+        val basic = clientClass(listOf(endpoint()), listOf(SecurityScheme.Basic("BasicAuth")))
+        assertEquals(
+            listOf("baseUrl", "httpClient", "basicAuthTestUsername", "basicAuthTestPassword"),
+            assertNotNull(basic.primaryConstructor).parameters.map { it.name },
+        )
+    }
+
+    @Test
+    fun `client has a single constructor`() {
+        val cls = clientClass(listOf(endpoint()), listOf(SecurityScheme.Bearer("BearerAuth")))
+        assertTrue(cls.funSpecs.none { it.isConstructor }, "Expected no secondary constructors")
+    }
+
+    @Test
+    fun `client property is derived from the injected httpClient`() {
+        val clientInitializer = clientClass(endpoint())
+            .propertySpecs
+            .first { it.name == "client" }
+            .initializer
+            .toString()
+        assertEquals("createHttpClient(httpClient)", clientInitializer)
+    }
+
+    @Test
+    fun `httpClient constructor param has KDoc only when generateKdoc is enabled`() {
+        fun httpClientKdoc(options: OutputOptions): String = generate(spec(endpoint()), options = options)
+            .first()
+            .members
+            .filterIsInstance<TypeSpec>()
+            .first()
+            .let { assertNotNull(it.primaryConstructor) }
+            .parameters
+            .first { it.name == "httpClient" }
+            .kdoc
+            .toString()
+
+        assertTrue(httpClientKdoc(OutputOptions(generateKdoc = true)).contains("sharing its engine"))
+        assertTrue(httpClientKdoc(OutputOptions(generateKdoc = false)).isEmpty())
     }
 
     // -- Pitfall 3: Untagged endpoints go to DefaultClient --
@@ -774,7 +832,11 @@ class ClientGeneratorTest {
             .first { it.name == "client" }
             .initializer
             .toString()
-        assertEquals("createHttpClient()", clientInitializer, "createHttpClient() no longer takes serializersModule")
+        assertEquals(
+            "createHttpClient(httpClient)",
+            clientInitializer,
+            "createHttpClient() no longer takes serializersModule",
+        )
     }
 
     // -- CONT-01: Multipart form-data code generation --
@@ -1189,7 +1251,10 @@ class ClientGeneratorTest {
             .propertySpecs
             .first { it.name == "client" }
         val clientInitializer = clientProperty.initializer.toString()
-        assertTrue(clientInitializer.contains("createHttpClient()"), "Expected plain createHttpClient()")
+        assertTrue(
+            clientInitializer.contains("createHttpClient(httpClient)"),
+            "Expected plain createHttpClient(httpClient)",
+        )
         assertTrue(!clientInitializer.contains("serializersModule"), "Expected no serializersModule")
     }
 
@@ -1301,9 +1366,9 @@ class ClientGeneratorTest {
         val constructor = assertNotNull(cls.primaryConstructor)
         val paramNames = constructor.parameters.map { it.name }
         assertEquals(
-            listOf("baseUrl"),
+            listOf("baseUrl", "httpClient"),
             paramNames,
-            "Expected only baseUrl param when security is explicitly empty",
+            "Expected only baseUrl and httpClient params when security is explicitly empty",
         )
     }
 
@@ -1391,7 +1456,7 @@ class ClientGeneratorTest {
         )
         val constructor = assertNotNull(cls.primaryConstructor)
         val paramNames = constructor.parameters.map { it.name }
-        assertEquals(listOf("baseUrl", "token"), paramNames, "Single Bearer should use plain token param")
+        assertEquals(listOf("baseUrl", "httpClient", "token"), paramNames, "Single Bearer should use plain token param")
     }
 
     @Test