From 85685c6f7c7e26e921ac9f237998727b85a41168 Mon Sep 17 00:00:00 2001
From: Damian Momot <damianmomot@google.com>
Date: Mon, 6 Jul 2026 00:10:49 -0700
Subject: [PATCH] feat: ChatCompletionsHttpClient - support a custom
 OkHttpClient and lazily create the shared one

Memoize the shared OkHttpClient so its pool/threads are created only after first use,
and add a public ChatCompletionsHttpClient(HttpOptions, OkHttpClient) so callers
can pass their own client.

PiperOrigin-RevId: 943089648
---
 .../chat/ChatCompletionsHttpClient.java       | 38 ++++++++++---------
 .../chat/ChatCompletionsHttpClientTest.java   |  4 +-
 2 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/core/src/main/java/com/google/adk/models/chat/ChatCompletionsHttpClient.java b/core/src/main/java/com/google/adk/models/chat/ChatCompletionsHttpClient.java
index f83287096..283dae671 100644
--- a/core/src/main/java/com/google/adk/models/chat/ChatCompletionsHttpClient.java
+++ b/core/src/main/java/com/google/adk/models/chat/ChatCompletionsHttpClient.java
@@ -21,7 +21,8 @@
 import com.google.adk.JsonBaseModel;
 import com.google.adk.models.LlmRequest;
 import com.google.adk.models.LlmResponse;
-import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.genai.types.HttpOptions;
@@ -68,11 +69,12 @@ public final class ChatCompletionsHttpClient implements ChatCompletionsClient {
   private static final Duration DEFAULT_CALL_TIMEOUT = Duration.ofMinutes(5);
 
   /**
-   * Shared OkHttpClient instance whose connection pool and thread dispatcher are reused across all
-   * {@link ChatCompletionsHttpClient} instances. Each instance forks this client via {@link
-   * OkHttpClient#newBuilder()} to apply per-instance timeouts without leaking pools.
+   * Shared OkHttpClient whose connection pool and dispatcher are reused across all instances (each
+   * forks it via {@link OkHttpClient#newBuilder()} for per-instance timeouts). Memoized so the pool
+   * and its threads are created on first use, not at class load.
    */
-  private static final OkHttpClient SHARED_POOL_CLIENT = new OkHttpClient();
+  private static final Supplier<OkHttpClient> SHARED_POOL_CLIENT =
+      Suppliers.memoize(OkHttpClient::new);
 
   private final OkHttpClient client;
   private final HttpUrl completionsUrl;
@@ -119,7 +121,18 @@ public ChatCompletionsHttpClient(HttpOptions httpOptions) {
     this(httpOptions, buildClient(httpOptions));
   }
 
-  private ChatCompletionsHttpClient(HttpOptions httpOptions, OkHttpClient client) {
+  /**
+   * Constructs a client backed by a caller-supplied {@link OkHttpClient} instead of the shared,
+   * lazily-created one -- e.g. to add interceptors, share a pool, use a proxy, or inject a mock.
+   * {@code baseUrl} and {@code headers} still come from {@code httpOptions}; timeouts and other
+   * transport settings are owned by {@code client} ({@code httpOptions.timeout()} is ignored).
+   *
+   * @param httpOptions HTTP configuration; {@link HttpOptions#baseUrl()} must be a valid HTTP(S)
+   *     URL
+   * @param client the {@link OkHttpClient} used to issue requests; must not be {@code null}
+   * @throws IllegalArgumentException if {@code httpOptions.baseUrl()} is missing or invalid
+   */
+  public ChatCompletionsHttpClient(HttpOptions httpOptions, OkHttpClient client) {
     Objects.requireNonNull(httpOptions, "httpOptions cannot be null");
     String baseUrl =
         httpOptions
@@ -140,16 +153,7 @@ private ChatCompletionsHttpClient(HttpOptions httpOptions, OkHttpClient client)
             .headers()
             .<ImmutableMap<String, String>>map(ImmutableMap::copyOf)
             .orElse(ImmutableMap.of());
-    this.client = client;
-  }
-
-  /**
-   * Test-only factory that injects a custom {@link OkHttpClient} (typically a mock) without
-   * touching production wiring. Production callers should use the public constructor.
-   */
-  @VisibleForTesting
-  static ChatCompletionsHttpClient forTesting(HttpOptions httpOptions, OkHttpClient client) {
-    return new ChatCompletionsHttpClient(httpOptions, client);
+    this.client = Objects.requireNonNull(client, "client cannot be null");
   }
 
   /**
@@ -158,7 +162,7 @@ static ChatCompletionsHttpClient forTesting(HttpOptions httpOptions, OkHttpClien
    */
   private static OkHttpClient buildClient(HttpOptions httpOptions) {
     Objects.requireNonNull(httpOptions, "httpOptions cannot be null");
-    OkHttpClient.Builder builder = SHARED_POOL_CLIENT.newBuilder();
+    OkHttpClient.Builder builder = SHARED_POOL_CLIENT.get().newBuilder();
     builder.connectTimeout(Duration.ZERO);
     builder.readTimeout(Duration.ZERO);
     builder.writeTimeout(Duration.ZERO);
diff --git a/core/src/test/java/com/google/adk/models/chat/ChatCompletionsHttpClientTest.java b/core/src/test/java/com/google/adk/models/chat/ChatCompletionsHttpClientTest.java
index cdb83393e..b32fa6017 100644
--- a/core/src/test/java/com/google/adk/models/chat/ChatCompletionsHttpClientTest.java
+++ b/core/src/test/java/com/google/adk/models/chat/ChatCompletionsHttpClientTest.java
@@ -85,7 +85,7 @@ public final class ChatCompletionsHttpClientTest {
   public void setUp() {
     when(mockHttpClient.newCall(any())).thenReturn(mockCall);
     client =
-        ChatCompletionsHttpClient.forTesting(
+        new ChatCompletionsHttpClient(
             HttpOptions.builder().baseUrl("https://example.com/").build(), mockHttpClient);
   }
 
@@ -96,7 +96,7 @@ public void setUp() {
    */
   private ChatCompletionsHttpClient newClientWithMock(HttpOptions options) {
     when(mockHttpClient.newCall(any())).thenReturn(mockCall);
-    return ChatCompletionsHttpClient.forTesting(options, mockHttpClient);
+    return new ChatCompletionsHttpClient(options, mockHttpClient);
   }
 
   private Response createMockResponse(String body, MediaType mediaType) {