From 93a859d88df1f3de322d732c639eded3f8514a8b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E6=B2=88=E6=98=9F=E7=B9=81?= <ikesnowy@outlook.com>
Date: Wed, 5 Jun 2024 15:24:29 +0800
Subject: [PATCH] fix: allow configuring timeout with DashScopeClient

---
 sample/Cnblogs.DashScope.Sample/Program.cs    |  2 +-
 src/Cnblogs.DashScope.Core/DashScopeClient.cs | 45 +++++++-----
 .../DashScopeClientTests.cs                   | 71 ++++++++++++++++++-
 3 files changed, 98 insertions(+), 20 deletions(-)

diff --git a/sample/Cnblogs.DashScope.Sample/Program.cs b/sample/Cnblogs.DashScope.Sample/Program.cs
index 54f21f5..b770ec9 100644
--- a/sample/Cnblogs.DashScope.Sample/Program.cs
+++ b/sample/Cnblogs.DashScope.Sample/Program.cs
@@ -7,7 +7,7 @@
 using Json.Schema;
 using Json.Schema.Generation;
 
-const string apiKey = "sk-eeff76d62cc946e5af8d1444f079a34e";
+const string apiKey = "sk-**";
 var dashScopeClient = new DashScopeClient(apiKey);
 
 Console.WriteLine("Choose the sample you want to run:");
diff --git a/src/Cnblogs.DashScope.Core/DashScopeClient.cs b/src/Cnblogs.DashScope.Core/DashScopeClient.cs
index 639128e..0c970b3 100644
--- a/src/Cnblogs.DashScope.Core/DashScopeClient.cs
+++ b/src/Cnblogs.DashScope.Core/DashScopeClient.cs
@@ -1,5 +1,4 @@
-using System.Net.Http.Headers;
-using Cnblogs.DashScope.Core.Internals;
+using Cnblogs.DashScope.Core.Internals;
 
 namespace Cnblogs.DashScope.Core;
 
@@ -8,27 +7,37 @@ namespace Cnblogs.DashScope.Core;
 /// </summary>
 public class DashScopeClient : DashScopeClientCore
 {
-    private static HttpClient? singletonClient;
-    private static HttpClient SingletonClient
-    {
-        get
-        {
-            singletonClient ??= new HttpClient(
-                new SocketsHttpHandler { PooledConnectionLifetime = TimeSpan.FromMinutes(2) })
-            {
-                BaseAddress = new Uri(DashScopeDefaults.DashScopeApiBaseAddress)
-            };
-            return singletonClient;
-        }
-    }
+    private static readonly Dictionary<string, HttpClient> ClientPools = new();
 
     /// <summary>
     /// Creates a DashScopeClient for further api call.
     /// </summary>
     /// <param name="apiKey">The DashScope api key.</param>
-    public DashScopeClient(string apiKey)
-        : base(SingletonClient)
+    /// <param name="timeout">The timeout for internal http client, defaults to 2 minute.</param>
+    /// <remarks>
+    ///     The underlying httpclient is cached by apiKey and timeout.
+    ///     Client created with same apiKey and timeout value will share same underlying <see cref="HttpClient"/> instance.
+    /// </remarks>
+    public DashScopeClient(string apiKey, TimeSpan? timeout = null)
+        : base(GetConfiguredClient(apiKey, timeout))
     {
-        SingletonClient.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", apiKey);
+    }
+
+    private static HttpClient GetConfiguredClient(string apiKey, TimeSpan? timeout)
+    {
+        var client = ClientPools.GetValueOrDefault(GetCacheKey());
+        if (client is null)
+        {
+            client = new HttpClient
+            {
+                BaseAddress = new Uri(DashScopeDefaults.DashScopeApiBaseAddress),
+                Timeout = timeout ?? TimeSpan.FromMinutes(2)
+            };
+            ClientPools.Add(GetCacheKey(), client);
+        }
+
+        return client;
+
+        string GetCacheKey() => $"{apiKey}-{timeout?.TotalMilliseconds}";
     }
 }
diff --git a/test/Cnblogs.DashScope.Sdk.UnitTests/DashScopeClientTests.cs b/test/Cnblogs.DashScope.Sdk.UnitTests/DashScopeClientTests.cs
index 8741448..54d05be 100644
--- a/test/Cnblogs.DashScope.Sdk.UnitTests/DashScopeClientTests.cs
+++ b/test/Cnblogs.DashScope.Sdk.UnitTests/DashScopeClientTests.cs
@@ -1,4 +1,5 @@
-using Cnblogs.DashScope.Core;
+using System.Reflection;
+using Cnblogs.DashScope.Core;
 using FluentAssertions;
 
 namespace Cnblogs.DashScope.Sdk.UnitTests;
@@ -17,4 +18,72 @@ public void DashScopeClient_Constructor_New()
         // Assert
         act.Should().NotThrow();
     }
+
+    [Theory]
+    [MemberData(nameof(ParamsShouldNotCache))]
+    public void DashScopeClient_Constructor_NotCacheableParams(
+        string apiKey,
+        string apiKey2,
+        TimeSpan? timeout,
+        TimeSpan? timeout2)
+    {
+        // Arrange
+        var client = new DashScopeClient(apiKey, timeout);
+        var client2 = new DashScopeClient(apiKey2, timeout2);
+
+        // Act
+        var value = HttpClientAccessor.GetValue(client);
+        var value2 = HttpClientAccessor.GetValue(client2);
+
+        // Assert
+        value.Should().NotBe(value2);
+    }
+
+    [Theory]
+    [MemberData(nameof(ParamsShouldCache))]
+    public void DashScopeClient_Constructor_CacheableParams(
+        string apiKey,
+        string apiKey2,
+        TimeSpan? timeout,
+        TimeSpan? timeout2)
+    {
+        // Arrange
+        var client = new DashScopeClient(apiKey, timeout);
+        var client2 = new DashScopeClient(apiKey2, timeout2);
+
+        // Act
+        var value = HttpClientAccessor.GetValue(client);
+        var value2 = HttpClientAccessor.GetValue(client2);
+
+        // Assert
+        value.Should().Be(value2);
+    }
+
+    public static TheoryData<string, string, TimeSpan?, TimeSpan?> ParamsShouldNotCache
+        => new()
+        {
+            { "apiKey", "apiKey2", null, null }, // same null timespan with different apikey
+            {
+                // same timespan with different apiKey
+                "apikey", "apiKey2", TimeSpan.FromMinutes(2), TimeSpan.FromMinutes(2)
+            },
+            {
+                // same apikey with different timeout
+                "apiKey", "apiKey", TimeSpan.FromMinutes(2), TimeSpan.FromMinutes(1)
+            }
+        };
+
+    public static TheoryData<string, string, TimeSpan?, TimeSpan?> ParamsShouldCache
+        => new()
+        {
+            { "apiKey", "apiKey", null, null }, // same null timespan with same apikey
+            {
+                // same timespan with same apiKey
+                "apiKey", "apiKey", TimeSpan.FromMinutes(2), TimeSpan.FromMinutes(2)
+            }
+        };
+
+    private static readonly FieldInfo HttpClientAccessor = typeof(DashScopeClientCore).GetField(
+        "_httpClient",
+        BindingFlags.Instance | BindingFlags.NonPublic)!;
 }