v5.0.0: System.Text.Json default, remove Newtonsoft, trusted publishing

.github/workflows/release.yml

@@ -8,8 +8,15 @@ name: Release
 # <Version> when no tag exists yet), so <Version> never has to be edited by hand. Each
 # successful publish creates the matching v<version> tag and a GitHub Release.
 #
-# Requires a repository secret NUGET_API_KEY (Settings > Secrets and variables > Actions)
-# with push rights for the SimpleHttpClient package.
+# Publishing uses NuGet trusted publishing (OIDC) — no long-lived API key. The job
+# requests a short-lived key at runtime via the NuGet/login action, so it needs
+# `id-token: write` permission and a repository secret NUGET_USER holding the
+# nuget.org username (profile name, NOT an email).
+#
+# Trusted publishing setup (one-time, on nuget.org > your account > Trusted Publishing):
+#   Repository Owner: Mako88   Repository: SimpleHttpClient
+#   Workflow File: release.yml   Environment: (leave empty)
+# Docs: https://learn.microsoft.com/en-us/nuget/nuget-org/trusted-publishing
 #
 # First-time setup: seed a tag matching the last published version so the first bump is
 # correct, e.g.  git tag v4.1.0 && git push origin v4.1.0
@@ -33,6 +40,7 @@ concurrency:
 
 permissions:
   contents: write
+  id-token: write  # request the OIDC token used for NuGet trusted publishing
 
 jobs:
   release:
@@ -118,9 +126,18 @@ jobs:
         if: steps.bump.outputs.proceed == 'true'
         run: dotnet build src/SimpleHttpClient/SimpleHttpClient.csproj -c Release -p:Version=${{ steps.version.outputs.version }}
 
+      # Exchange the GitHub OIDC token for a short-lived nuget.org API key (valid ~1h).
+      # Done right before push so it doesn't expire mid-build.
+      - name: NuGet login (OIDC -> short-lived API key)
+        if: steps.bump.outputs.proceed == 'true'
+        uses: NuGet/login@v1
+        id: nuget-login
+        with:
+          user: ${{ secrets.NUGET_USER }}
+
       - name: Push to NuGet
         if: steps.bump.outputs.proceed == 'true'
-        run: dotnet nuget push "src/SimpleHttpClient/bin/Release/*.nupkg" -k ${{ secrets.NUGET_API_KEY }} -s https://api.nuget.org/v3/index.json --skip-duplicate
+        run: dotnet nuget push "src/SimpleHttpClient/bin/Release/*.nupkg" --api-key ${{ steps.nuget-login.outputs.NUGET_API_KEY }} --source https://api.nuget.org/v3/index.json --skip-duplicate
 
       - name: Tag and create GitHub Release
         if: steps.bump.outputs.proceed == 'true'

README.md

@@ -233,14 +233,14 @@ request.SerializerOverride = new SimpleHttpDefaultJsonSerializer();
 ```
 You can supply your own serializer by implementing `ISimpleHttpSerializer`.
 
-#### System.Text.Json
-The default JSON serializer uses `Newtonsoft.Json`. A `System.Text.Json`-based serializer is also included and can be opted into the same way:
-```csharp
-client.Serializer = new SimpleHttpSystemTextJsonSerializer();
-```
-Its settings mirror the default (camelCase names, null values omitted, indented output, case-insensitive deserialization), so it's a drop-in for most payloads. Note that `System.Text.Json` is stricter than `Newtonsoft.Json` — most notably it can't use a non-public parameterless constructor when deserializing, so such types need a public constructor or a `[JsonConstructor]`.
-
-> **Heads up:** `SimpleHttpSystemTextJsonSerializer` is slated to become the default in the next major version (v5), at which point the `Newtonsoft.Json` dependency will be removed.
+#### JSON serialization
+The default JSON serializer (`SimpleHttpDefaultJsonSerializer`) is backed by `System.Text.Json`. It serializes with camelCase names, omits null values, writes indented output, and deserializes case-insensitively. For smoother interop it also reads numbers from JSON strings (e.g. `"123"`) and tolerates trailing commas and comments while reading. The equivalent `SimpleHttpSystemTextJsonSerializer` is also available for callers who reference it explicitly.
+
+> **Upgrading from v4?** As of **v5.0.0** the default serializer moved from `Newtonsoft.Json` to `System.Text.Json` and the `Newtonsoft.Json` dependency was removed. The defaults above cover the most common differences, but `System.Text.Json` is stricter in two ways it won't soften:
+> - **Non-public parameterless constructors** aren't used — add a public constructor or a `[JsonConstructor]`.
+> - **Wrong-shape values aren't coerced** — a field that's sometimes a string and sometimes an object (and similar) threw nothing under Newtonsoft but throws here. For such fields, attach a custom `JsonConverter` to the property.
+>
+> If you'd rather keep the old behavior wholesale, implement `ISimpleHttpSerializer` with your own `Newtonsoft.Json` serializer and set it on the client.
 
 ### Logging
 You can log requests and responses by setting the `LogRequest` and `LogResponse` delegates (called immediately before a request is sent and immediately after a response is received), or by providing an `ISimpleHttpLogger`:

src/SimpleHttpClient.Tests.Integration/HttpClientProviderTests.cs

@@ -0,0 +1,67 @@
+using Moq;
+using System;
+using System.Net.Http;
+using System.Threading.Tasks;
+
+namespace SimpleHttpClient.Tests
+{
+    public class HttpClientProviderTests
+    {
+        [Fact]
+        public void Factory_Create_ReturnsProviderForTargetFramework()
+        {
+            using var provider = HttpClientProviderFactory.Create(null);
+
+#if NETFRAMEWORK
+            // The netstandard2.0 asset (used by .NET Framework) rotates a cached client.
+            Assert.IsType<RotatingHttpClientProvider>(provider);
+#else
+            // The net8.0 asset relies on SocketsHttpHandler's pooled connection lifetime.
+            Assert.IsType<PooledHttpClientProvider>(provider);
+#endif
+        }
+
+        [Fact]
+        public void GetClient_WithoutFactory_ReturnsConfiguredClient_CachedAcrossCalls()
+        {
+            using var provider = HttpClientProviderFactory.Create(null);
+
+            var first = provider.GetClient();
+            var second = provider.GetClient();
+
+            Assert.NotNull(first);
+            // With no factory the provider owns and caches a single client.
+            Assert.Same(first, second);
+            // Configured with an infinite timeout - SimpleClient applies its own per-request timeout.
+            Assert.Equal(System.Threading.Timeout.InfiniteTimeSpan, first.Timeout);
+        }
+
+        [Fact]
+        public void GetClient_WithFactory_DelegatesToFactory()
+        {
+            using var factoryClient = new HttpClient();
+            var factory = new Mock<IHttpClientFactory>();
+            factory.Setup(f => f.CreateClient(Constants.HttpClientNameString)).Returns(factoryClient);
+
+            using var provider = HttpClientProviderFactory.Create(factory.Object);
+
+            var client = provider.GetClient();
+
+            Assert.Same(factoryClient, client);
+            factory.Verify(f => f.CreateClient(Constants.HttpClientNameString), Times.AtLeastOnce);
+        }
+
+        [Fact]
+        public async Task Dispose_DisposesOwnedClient_AndIsIdempotent()
+        {
+            var provider = HttpClientProviderFactory.Create(null);
+            var client = provider.GetClient();
+
+            provider.Dispose();
+            provider.Dispose(); // idempotent: a second dispose must not throw
+
+            // The client the provider owns is disposed along with it.
+            await Assert.ThrowsAsync<ObjectDisposedException>(() => client.GetAsync("http://localhost"));
+        }
+    }
+}

src/SimpleHttpClient.Tests.Integration/Serialization/SimpleHttpDefaultJsonSerializerTests.cs

@@ -11,6 +11,14 @@ public class SimpleHttpDefaultJsonSerializerTests
   ""property3"": true
 }";
 
+        [Fact]
+        public void DefaultSerializer_IsBackedBy_SystemTextJson()
+        {
+            // As of v5.0.0 the default JSON serializer is System.Text.Json-based
+            // (it derives from SimpleHttpSystemTextJsonSerializer).
+            Assert.IsAssignableFrom<SimpleHttpSystemTextJsonSerializer>(new SimpleHttpDefaultJsonSerializer());
+        }
+
         [Fact]
         public void RoundTrip_Succeeds()
         {
@@ -37,7 +45,9 @@ public void Serialization_Succeeds()
 
             var serialized = testObject.Serialize(objectToSerialize);
 
-            Assert.Equal(TestSerializationString, serialized);
+            // Normalize line endings so the comparison doesn't depend on the
+            // platform's newline or the checkout's git autocrlf setting.
+            Assert.Equal(TestSerializationString.Replace("\r\n", "\n"), serialized.Replace("\r\n", "\n"));
         }
 
         [Fact]

src/SimpleHttpClient.Tests.Integration/Serialization/SimpleHttpSystemTextJsonSerializerTests.cs

@@ -4,8 +4,7 @@
 {
     public class SimpleHttpSystemTextJsonSerializerTests
     {
-        // Same shape and formatting the Newtonsoft-based default produces, so the
-        // System.Text.Json serializer is verified to be a drop-in for typical payloads.
+        // The camelCase, indented shape the serializer is expected to produce.
         private const string TestSerializationString =
 @"{
   ""property1"": ""property1 value"",
@@ -78,6 +77,85 @@
             Assert.Contains("\"value\": 1", serialized);
         }
 
+        [Fact]
+        public void RoundTrip_WithNestedObjectCollectionAndEnum_PreservesValues()
+        {
+            var serializer = new SimpleHttpSystemTextJsonSerializer();
+
+            var original = new ComplexObject
+            {
+                Name = "outer",
+                Status = SampleStatus.Active,
+                Tags = new List<string> { "a", "b" },
+                Child = new TestSerializationObject(),
+            };
+
+            var roundTripped = serializer.Deserialize<ComplexObject>(serializer.Serialize(original));
+
+            Assert.NotNull(roundTripped);
+            Assert.Equal("outer", roundTripped.Name);
+            Assert.Equal(SampleStatus.Active, roundTripped.Status);
+            Assert.Equal(new[] { "a", "b" }, roundTripped.Tags);
+            Assert.Equal("property1 value", roundTripped.Child?.Property1);
+        }
+
+        [Fact]
+        public void Serialization_WritesEnumsAsNumbers()
+        {
+            // Matches Newtonsoft's default (numeric enums), so migrating to System.Text.Json
+            // doesn't silently change the enum wire format.
+            var serializer = new SimpleHttpSystemTextJsonSerializer();
+
+            var serialized = serializer.Serialize(new ComplexObject { Status = SampleStatus.Active });
+
+            Assert.Contains("\"status\": 1", serialized);
+            Assert.DoesNotContain("Active", serialized);
+        }
+
+        [Fact]
+        public void Deserialization_IntoTypeWithOnlyNonPublicConstructor_Throws()
+        {
+            // The headline strictness difference from Newtonsoft: System.Text.Json will not
+            // invoke a non-public parameterless constructor. Documented in the README migration
+            // notes; pinned here so the behavior (and the docs) stay honest.
+            var serializer = new SimpleHttpSystemTextJsonSerializer();
+
+            Assert.ThrowsAny<NotSupportedException>(
+                () => serializer.Deserialize<NonPublicCtorObject>("{\"value\":1}"));
+        }
+
+        [Fact]
+        public void Deserialization_ReadsNumbersFromStrings()
+        {
+            // A quoted number must bind to a numeric property (common API interop case,
+            // and Newtonsoft's default behavior).
+            var serializer = new SimpleHttpSystemTextJsonSerializer();
+
+            var result = serializer.Deserialize<TestSerializationObject>("{\"property2\": \"42\"}");
+
+            Assert.NotNull(result);
+            Assert.Equal(42, result.Property2);
+        }
+
+        [Fact]
+        public void Deserialization_ToleratesTrailingCommasAndComments()
+        {
+            var serializer = new SimpleHttpSystemTextJsonSerializer();
+
+            const string json =
+@"{
+  // leading comment
+  ""property1"": ""value"",
+  ""property2"": 7,
+}";
+
+            var result = serializer.Deserialize<TestSerializationObject>(json);
+
+            Assert.NotNull(result);
+            Assert.Equal("value", result.Property1);
+            Assert.Equal(7, result.Property2);
+        }
+
         private static string Normalize(string value) => value.Replace("\r\n", "\n");
 
         private class NullableSerializationObject
@@ -86,5 +164,31 @@
 
             public int Value { get; set; } = 1;
         }
+
+        private enum SampleStatus
+        {
+            None = 0,
+            Active = 1,
+        }
+
+        private class ComplexObject
+        {
+            public string? Name { get; set; }
+
+            public SampleStatus Status { get; set; }
+
+            public List<string>? Tags { get; set; }
+
+            public TestSerializationObject? Child { get; set; }
+        }
+
+        private class NonPublicCtorObject
+        {
+            private NonPublicCtorObject()
+            {
+            }
+
+            public int Value { get; set; }
+        }
     }
 }

src/SimpleHttpClient.Tests.Integration/SimpleClientTests.cs

@@ -1,5 +1,7 @@
 using Moq;
-using Newtonsoft.Json.Linq;
+using System.Text.Json;
+using System.Text.Json.Nodes;
+using System.Text.Json.Serialization;
 using SimpleHttpClient.Logging;
 using SimpleHttpClient.Models;
 using SimpleHttpClient.Serialization;
@@ -113,7 +115,7 @@
 
             var response = await client.MakeRequest(request);
 
-            var responseJson = JObject.Parse(response.StringBody);
+            var responseJson = JsonNode.Parse(response.StringBody);
 
             Assert.Equal(HttpStatusCode.OK, response.StatusCode);
             Assert.Equal("value1", responseJson?["args"]?["param1"]?.ToString());
@@ -349,7 +351,7 @@
 
             var response = await client.MakeRequest(request);
 
-            var body = JToken.Parse(response.StringBody);
+            var body = JsonNode.Parse(response.StringBody)!;
 
             Assert.Equal(HttpStatusCode.OK, response.StatusCode);
             Assert.Equal("text/json", request.ContentType);
@@ -368,7 +370,7 @@
 
             var response = await client.MakeRequest(request);
 
-            var body = JToken.Parse(response.StringBody);
+            var body = JsonNode.Parse(response.StringBody)!;
 
             Assert.Equal(HttpStatusCode.OK, response.StatusCode);
             Assert.Equal("text/json", request.ContentType);
@@ -416,7 +418,7 @@
 
             await client.MakeRequest<PostmanEchoResponse>(request);
 
-            var body = JToken.Parse(request.StringBody);
+            var body = JsonNode.Parse(request.StringBody)!;
 
             Assert.Equal("value1", body["param1"]?.ToString());
             Assert.Equal("value2", body["param2"]?.ToString());
@@ -590,7 +592,7 @@
         public async Task LogResponse_IsCalled_WhenNotNull()
         {
             var test = 0;
             ISimpleResponse loggedResponse = null;
 
             var request = new SimpleRequest("/get");
 
@@ -623,14 +625,42 @@
 
         public Args? Form { get; set; }
 
+        // postman-echo returns "data" as the posted object for JSON bodies, but as an
+        // empty string for form posts. System.Text.Json (unlike Newtonsoft) won't coerce
+        // a string into a complex type, so the converter maps the non-object case to null
+        // while keeping Data strongly typed (so the JSON-body tests still verify that
+        // properties deserialize onto the typed object).
+        [JsonConverter(typeof(EmptyStringTolerantConverter<Args>))]
         public Args? Data { get; set; }
 
-        public JToken? Headers { get; set; }
+        public JsonNode? Headers { get; set; }
     }
 
     public class Args
     {
         public string? Param1 { get; set; }
         public string? Param2 { get; set; }
     }
+
+    // Deserializes an object normally, but yields null for any non-object JSON value
+    // (e.g. the empty string postman-echo returns for the "data" field on form posts),
+    // rather than letting System.Text.Json throw and fail the whole response.
+    public class EmptyStringTolerantConverter<T> : JsonConverter<T> where T : class
+    {
+        public override T? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
+        {
+            if (reader.TokenType != JsonTokenType.StartObject)
+            {
+                reader.Skip();
+                return null;
+            }
+
+            // No converter is registered for T in options (this one is applied per-property),
+            // so this deserializes with the default object handling rather than recursing.
+            return JsonSerializer.Deserialize<T>(ref reader, options);
+        }
+
+        public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions options) =>
+            JsonSerializer.Serialize(writer, value, options);
+    }
 }