[repo-assist] perf+test: replace UriBuilder+ParseQueryString with StringBuilder; add 9 tests (500→509)

src/SwaggerProvider.Runtime/RuntimeHelpers.fs

@@ -503,14 +503,16 @@ module RuntimeHelpers =
     let toFormUrlEncodedContent(keyValues: seq<string * obj>) =
         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<string * string>) =
         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))

tests/SwaggerProvider.Tests/RuntimeHelpersTests.fs

@@ -643,6 +643,29 @@ module CreateHttpRequestTests =
         uri |> shouldContainText "page=1"
         uri |> shouldContainText "size=20"
 
+    [<Fact>]
+    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"
+
+    [<Fact>]
+    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"
+
+    [<Fact>]
+    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 =
 

tests/SwaggerProvider.Tests/Schema.OperationCompilationTests.fs

@@ -1648,3 +1648,55 @@ let ``201 response in async mode resolves to Async<string> when no 200 defined``
 
     method.ReturnType.GetGenericArguments()[0]
     |> shouldEqual typeof<string>
+
+// ── 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: {}
+"""
+
+[<Fact>]
+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<Task<_>>
+
+    // 200 (string) must win over 201 (integer)
+    method.ReturnType.GetGenericArguments()[0]
+    |> shouldEqual typeof<string>
+
+[<Fact>]
+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<int32>
+    returnArg |> shouldEqual typeof<string>

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<string>
+
+// ── 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)
+
+[<Fact>]
+let ``v2 listPets generates a method with Task<Pet[]> return type``() =
+    let types = compileV2Schema minimalPetstoreV2
+    let method = (findMethod types "ListPets").Value
+    method.ReturnType.IsGenericType |> shouldEqual true
+
+    method.ReturnType.GetGenericTypeDefinition()
+    |> shouldEqual typedefof<System.Threading.Tasks.Task<_>>
+
+    let returnArg = method.ReturnType.GetGenericArguments()[0]
+    returnArg.IsArray |> shouldEqual true
+    returnArg.GetElementType().Name |> shouldEqual "Pet"
+
+[<Fact>]
+let ``v2 getPet generates a method with Task<Pet> return type``() =
+    let types = compileV2Schema minimalPetstoreV2
+    let method = (findMethod types "GetPet").Value
+    method.ReturnType.IsGenericType |> shouldEqual true
+
+    method.ReturnType.GetGenericTypeDefinition()
+    |> shouldEqual typedefof<System.Threading.Tasks.Task<_>>
+
+    let returnArg = method.ReturnType.GetGenericArguments()[0]
+    returnArg.Name |> shouldEqual "Pet"
+
+[<Fact>]
+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<int64>
+    idParam.IsOptional |> shouldEqual false
+
+[<Fact>]
+let ``v2 createPet generates a method with Task<IO.Stream> 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<IO.Stream>.
+    let types = compileV2Schema minimalPetstoreV2
+    let method = (findMethod types "CreatePet").Value
+    method.ReturnType.IsGenericType |> shouldEqual true
+
+    method.ReturnType.GetGenericTypeDefinition()
+    |> shouldEqual typedefof<System.Threading.Tasks.Task<_>>
+
+    method.ReturnType.GetGenericArguments()[0]
+    |> shouldEqual typeof<System.IO.Stream>