From f64e838773f14c5a3f535a50566ad9ae334b087d Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 1 Jun 2026 21:03:01 +0000 Subject: [PATCH 1/3] =?UTF-8?q?perf+test:=20replace=20UriBuilder+ParseQuer?= =?UTF-8?q?yString=20with=20StringBuilder,=20add=209=20tests=20(500?= =?UTF-8?q?=E2=86=92509)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tasks: 8 (perf), 9 (tests), 5 (coding improvement) perf: RuntimeHelpers.createHttpRequest - Replace UriBuilder + System.Web.HttpUtility.ParseQueryString + NameValueCollection with a single StringBuilder + Uri.EscapeDataString pass. - Avoids 4-5 allocations per API call (UriBuilder, NameValueCollection, Hashtable, intermediate strings). Now allocates only one StringBuilder. - Encoding changes from form-encoding (spaces as '+') to RFC 3986 percent-encoding (spaces as '%20'); both are accepted by HTTP servers. - Removes the Seq.isEmpty early-exit fast path (no longer needed — the StringBuilder path is already allocation-efficient with zero params). - Removes the now-unused System.Web.HttpUtility reference. coding: RuntimeHelpers.toFormUrlEncodedContent - Merge the Seq.filter + Seq.choose passes into a single Seq.choose, avoiding an intermediate sequence allocation for the common case. tests (9 new, 500→509): - RuntimeHelpersTests: 4 tests documenting RFC 3986 percent-encoding in createHttpRequest (spaces, & and = in values, spaces in names). - Schema.OperationCompilationTests: 2 tests asserting that the '200' response takes priority over '201' when both are present in an operation. - Schema.V2SchemaCompilationTests: 4 tests verifying compiled operation return types from a Swagger 2.0 petstore (listPets→Task, getPet→Task, getPet path param int64, createPet→Task per OpenApi normalisation). Build/test: - dotnet fantomas --check: no issues - Unit tests: 509 total, 0 failed, 1 skipped Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/SwaggerProvider.Runtime/RuntimeHelpers.fs | 47 ++++++++------- .../RuntimeHelpersTests.fs | 23 ++++++++ .../Schema.OperationCompilationTests.fs | 52 ++++++++++++++++ .../Schema.V2SchemaCompilationTests.fs | 59 +++++++++++++++++++ 4 files changed, 159 insertions(+), 22 deletions(-) diff --git a/src/SwaggerProvider.Runtime/RuntimeHelpers.fs b/src/SwaggerProvider.Runtime/RuntimeHelpers.fs index c6f427af..1d141473 100644 --- a/src/SwaggerProvider.Runtime/RuntimeHelpers.fs +++ b/src/SwaggerProvider.Runtime/RuntimeHelpers.fs @@ -503,14 +503,16 @@ module RuntimeHelpers = let toFormUrlEncodedContent(keyValues: seq) = let keyValues = keyValues - |> Seq.filter(snd >> isNull >> not) |> Seq.choose(fun (k, v) -> - let param = toParam v - - if isNull param then + if isNull v then None else - Some(Collections.Generic.KeyValuePair(k, param))) + let param = toParam v + + if isNull param then + None + else + Some(Collections.Generic.KeyValuePair(k, param))) new FormUrlEncodedContent(keyValues) @@ -556,24 +558,25 @@ module RuntimeHelpers = let createHttpRequest (httpMethod: string) (address: string) (queryParams: seq) = let requestUrl = - // Fast path: avoid UriBuilder + ParseQueryString allocation when there are no query params. - // TrimStart('/') mirrors the UriBuilder path's PathAndQuery.TrimStart('/') normalisation, - // which strips the leading slash from schema paths such as "/pets" → "pets". A leading- - // slash relative URI resolves from the host root and silently drops any base path, so - // normalisation must be applied on both branches. - if Seq.isEmpty queryParams then - address.TrimStart('/') - else - let fakeHost = "http://fake-host/" - let builder = UriBuilder(combineUrl fakeHost address) - let query = System.Web.HttpUtility.ParseQueryString(builder.Query) - - for name, value in queryParams do - if not <| isNull value then - query.Add(name, value) + // Build the request URL using a StringBuilder to avoid UriBuilder + ParseQueryString + // allocations (NameValueCollection, internal Hashtable, multiple string copies). + // TrimStart('/') strips the leading slash from schema paths such as "/pets" → "pets" + // so that the relative URI resolves from the HttpClient.BaseAddress path rather than + // the host root. + // Values are RFC 3986 percent-encoded via Uri.EscapeDataString (spaces → %20), which + // is accepted by all standards-compliant HTTP servers. + let sb = Text.StringBuilder(address.TrimStart('/')) + let mutable sep = '?' + + for name, value in queryParams do + if not(isNull value) then + sb.Append(sep) |> ignore + sb.Append(Uri.EscapeDataString(name)) |> ignore + sb.Append('=') |> ignore + sb.Append(Uri.EscapeDataString(value)) |> ignore + sep <- '&' - builder.Query <- query.ToString() - builder.Uri.PathAndQuery.TrimStart('/') + sb.ToString() let method = resolveHttpMethod httpMethod new HttpRequestMessage(method, Uri(requestUrl, UriKind.Relative)) diff --git a/tests/SwaggerProvider.Tests/RuntimeHelpersTests.fs b/tests/SwaggerProvider.Tests/RuntimeHelpersTests.fs index 75e5afa0..784cb9e3 100644 --- a/tests/SwaggerProvider.Tests/RuntimeHelpersTests.fs +++ b/tests/SwaggerProvider.Tests/RuntimeHelpersTests.fs @@ -643,6 +643,29 @@ module CreateHttpRequestTests = uri |> shouldContainText "page=1" uri |> shouldContainText "size=20" + [] + let ``createHttpRequest percent-encodes spaces in query parameter values``() = + // The StringBuilder+Uri.EscapeDataString implementation encodes spaces as %20 + // (RFC 3986 percent-encoding) rather than + (application/x-www-form-urlencoded). + use req = createHttpRequest "GET" "v1/search" [ ("q", "hello world") ] + let uri = req.RequestUri.ToString() + uri |> shouldContainText "q=hello%20world" + uri |> shouldNotContainText "q=hello+world" + uri |> shouldNotContainText "q=hello world" + + [] + let ``createHttpRequest percent-encodes special characters in query parameter values``() = + use req = createHttpRequest "GET" "v1/items" [ ("filter", "a=1&b=2") ] + let uri = req.RequestUri.ToString() + // & and = in values must be encoded so they are not confused with separators + uri |> shouldContainText "filter=a%3D1%26b%3D2" + + [] + let ``createHttpRequest percent-encodes special characters in parameter names``() = + use req = createHttpRequest "GET" "v1/items" [ ("my param", "value") ] + let uri = req.RequestUri.ToString() + uri |> shouldContainText "my%20param=value" + module FillHeadersTests = diff --git a/tests/SwaggerProvider.Tests/Schema.OperationCompilationTests.fs b/tests/SwaggerProvider.Tests/Schema.OperationCompilationTests.fs index 49c74654..0bd9095a 100644 --- a/tests/SwaggerProvider.Tests/Schema.OperationCompilationTests.fs +++ b/tests/SwaggerProvider.Tests/Schema.OperationCompilationTests.fs @@ -1648,3 +1648,55 @@ let ``201 response in async mode resolves to Async when no 200 defined`` method.ReturnType.GetGenericArguments()[0] |> shouldEqual typeof + +// ── 200 response takes priority over other 2xx when both are defined ───────── + +/// OpenAPI 3.0 schema where an operation defines both a 200 (string) and a 201 +/// (integer) response. The 200 response must win and determine the return type. +let private twoHundredAndCreatedSchema = + """openapi: "3.0.0" +info: + title: PriorityTest + version: "1.0.0" +paths: + /items: + post: + operationId: createItem + responses: + "200": + description: OK + content: + application/json: + schema: + type: string + "201": + description: Created + content: + application/json: + schema: + type: integer +components: + schemas: {} +""" + +[] +let ``200 response takes priority over 201 when both are defined``() = + let types = compileTaskSchema twoHundredAndCreatedSchema + let method = (findMethod types "CreateItem").Value + method.ReturnType.IsGenericType |> shouldEqual true + + method.ReturnType.GetGenericTypeDefinition() + |> shouldEqual typedefof> + + // 200 (string) must win over 201 (integer) + method.ReturnType.GetGenericArguments()[0] + |> shouldEqual typeof + +[] +let ``200 response schema is used not 201 when both are present``() = + // Verify that the 201 integer schema is not used when a 200 string schema is present. + let types = compileTaskSchema twoHundredAndCreatedSchema + let method = (findMethod types "CreateItem").Value + let returnArg = method.ReturnType.GetGenericArguments()[0] + returnArg |> shouldNotEqual typeof + returnArg |> shouldEqual typeof diff --git a/tests/SwaggerProvider.Tests/Schema.V2SchemaCompilationTests.fs b/tests/SwaggerProvider.Tests/Schema.V2SchemaCompilationTests.fs index 3b798e42..97b46f67 100644 --- a/tests/SwaggerProvider.Tests/Schema.V2SchemaCompilationTests.fs +++ b/tests/SwaggerProvider.Tests/Schema.V2SchemaCompilationTests.fs @@ -405,3 +405,62 @@ let ``v2 compiled object type ToString invokeCode does not throw for concrete pr let body = invokeCode [ thisExpr ] // Expr is a value type; just verifying invokeCode did not throw is sufficient body.Type |> shouldEqual typeof + +// ── V2 operation return types ───────────────────────────────────────────────── + +/// Finds a method on any type in the compiled result list. +let private findMethod (types: ProviderImplementation.ProvidedTypes.ProvidedTypeDefinition list) (name: string) = + types + |> List.collect(fun t -> t.GetMethods() |> Array.toList) + |> List.tryFind(fun m -> m.Name = name) + +[] +let ``v2 listPets generates a method with Task return type``() = + let types = compileV2Schema minimalPetstoreV2 + let method = (findMethod types "ListPets").Value + method.ReturnType.IsGenericType |> shouldEqual true + + method.ReturnType.GetGenericTypeDefinition() + |> shouldEqual typedefof> + + let returnArg = method.ReturnType.GetGenericArguments()[0] + returnArg.IsArray |> shouldEqual true + returnArg.GetElementType().Name |> shouldEqual "Pet" + +[] +let ``v2 getPet generates a method with Task return type``() = + let types = compileV2Schema minimalPetstoreV2 + let method = (findMethod types "GetPet").Value + method.ReturnType.IsGenericType |> shouldEqual true + + method.ReturnType.GetGenericTypeDefinition() + |> shouldEqual typedefof> + + let returnArg = method.ReturnType.GetGenericArguments()[0] + returnArg.Name |> shouldEqual "Pet" + +[] +let ``v2 getPet has an integer path parameter``() = + let types = compileV2Schema minimalPetstoreV2 + let method = (findMethod types "GetPet").Value + let parameters = method.GetParameters() + // id path param (int64) + CancellationToken + parameters.Length |> shouldEqual 2 + let idParam = parameters |> Array.find(fun p -> p.Name = "id") + idParam.ParameterType |> shouldEqual typeof + idParam.IsOptional |> shouldEqual false + +[] +let ``v2 createPet generates a method with Task return type``() = + // In Swagger 2.0, when the 201 response has no explicit schema and no 'produces' key, + // Microsoft.OpenApi normalises the response to application/octet-stream with a null + // schema, which the OperationCompiler maps to Task. + let types = compileV2Schema minimalPetstoreV2 + let method = (findMethod types "CreatePet").Value + method.ReturnType.IsGenericType |> shouldEqual true + + method.ReturnType.GetGenericTypeDefinition() + |> shouldEqual typedefof> + + method.ReturnType.GetGenericArguments()[0] + |> shouldEqual typeof From f7349eaaf5ecf79f22339a8a07bd559b6a5ab5b5 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Mon, 1 Jun 2026 21:03:05 +0000 Subject: [PATCH 2/3] ci: trigger checks From 87b3425ccf9580a8f21ba34fa1139a7b04bb5835 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 2 Jun 2026 06:16:32 +0000 Subject: [PATCH 3/3] fix: handle existing query and fragment in createHttpRequest --- src/SwaggerProvider.Runtime/RuntimeHelpers.fs | 36 ++++++++++++++++--- .../RuntimeHelpersTests.fs | 15 ++++++++ 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/src/SwaggerProvider.Runtime/RuntimeHelpers.fs b/src/SwaggerProvider.Runtime/RuntimeHelpers.fs index 1d141473..aaa94854 100644 --- a/src/SwaggerProvider.Runtime/RuntimeHelpers.fs +++ b/src/SwaggerProvider.Runtime/RuntimeHelpers.fs @@ -565,18 +565,44 @@ module RuntimeHelpers = // the host root. // Values are RFC 3986 percent-encoded via Uri.EscapeDataString (spaces → %20), which // is accepted by all standards-compliant HTTP servers. - let sb = Text.StringBuilder(address.TrimStart('/')) - let mutable sep = '?' + let address = address.TrimStart('/') + let fragmentStart = address.IndexOf('#') + + let baseAddress, fragment = + if fragmentStart >= 0 then + address.Substring(0, fragmentStart), address.Substring(fragmentStart) + else + address, null + + let mutable sb = null for name, value in queryParams do if not(isNull value) then - sb.Append(sep) |> ignore + if isNull sb then + sb <- Text.StringBuilder(baseAddress) + + if baseAddress.Contains("?") then + if sb.Length > 0 then + let last = sb[sb.Length - 1] + + if last <> '?' && last <> '&' then + sb.Append('&') |> ignore + else + sb.Append('?') |> ignore + else + sb.Append('&') |> ignore + sb.Append(Uri.EscapeDataString(name)) |> ignore sb.Append('=') |> ignore sb.Append(Uri.EscapeDataString(value)) |> ignore - sep <- '&' - sb.ToString() + if isNull sb then + address + else + if not(isNull fragment) then + sb.Append(fragment) |> ignore + + sb.ToString() let method = resolveHttpMethod httpMethod new HttpRequestMessage(method, Uri(requestUrl, UriKind.Relative)) diff --git a/tests/SwaggerProvider.Tests/RuntimeHelpersTests.fs b/tests/SwaggerProvider.Tests/RuntimeHelpersTests.fs index 784cb9e3..72ae7acb 100644 --- a/tests/SwaggerProvider.Tests/RuntimeHelpersTests.fs +++ b/tests/SwaggerProvider.Tests/RuntimeHelpersTests.fs @@ -666,6 +666,21 @@ module CreateHttpRequestTests = let uri = req.RequestUri.ToString() uri |> shouldContainText "my%20param=value" + [] + let ``createHttpRequest appends query params to address with existing query``() = + use req = + createHttpRequest "GET" "v1/items?existing=1" [ ("page", "2"); ("size", "10") ] + + req.RequestUri.OriginalString + |> shouldEqual "v1/items?existing=1&page=2&size=10" + + [] + let ``createHttpRequest inserts query params before fragment``() = + use req = createHttpRequest "GET" "v1/items?existing=1#section" [ ("page", "2") ] + + req.RequestUri.OriginalString + |> shouldEqual "v1/items?existing=1&page=2#section" + module FillHeadersTests =