Skip to content

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

Merged
sergey-tihon merged 3 commits into
masterfrom
repo-assist/test-perf-june-20260601-479a53668e65139e
Jun 2, 2026
Merged

[repo-assist] perf+test: replace UriBuilder+ParseQueryString with StringBuilder; add 9 tests (500→509)#457
sergey-tihon merged 3 commits into
masterfrom
repo-assist/test-perf-june-20260601-479a53668e65139e

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot commented Jun 1, 2026

🤖 This PR was created by Repo Assist, an automated AI assistant.

Summary

Three improvements across runtime performance, test coverage, and code clarity.

Task 8 — Performance: faster query string building

RuntimeHelpers.createHttpRequest previously used UriBuilder + System.Web.HttpUtility.ParseQueryString + NameValueCollection to assemble the query string. This allocates 4–5 objects per API call (UriBuilder, NameValueCollection, internal Hashtable, intermediate strings).

This PR replaces that with a single StringBuilder + Uri.EscapeDataString pass:

// Before (allocates ~5 objects)
let fakeHost = "(fakehost/redacted)
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)
builder.Query <- query.ToString()
builder.Uri.PathAndQuery.TrimStart('/')

// After (allocates 1 StringBuilder)
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 <- '&'
sb.ToString()

The encoding changes from application/x-www-form-urlencoded (+ for spaces) to RFC 3986 percent-encoding (%20 for spaces). Both are accepted by all standards-compliant HTTP servers. The System.Web reference is removed.

Task 5 (fallback for Task 4) — Coding: single-pass toFormUrlEncodedContent

The toFormUrlEncodedContent function previously did two sequential Seq passes (filter then choose). These are merged into a single Seq.choose pass, avoiding an intermediate sequence allocation.

Task 9 — Testing: 9 new tests (500 → 509)

RuntimeHelpersTests (4 tests): document the new RFC 3986 encoding behaviour:

  • Spaces in query values are encoded as %20, not +
  • Special characters (&, =) in values are percent-encoded
  • Special characters in parameter names are percent-encoded
  • Multiple query params all encode correctly

Schema.OperationCompilationTests (2 tests): verify 200 response takes priority over 201:

  • When both 200 (string) and 201 (integer) responses are defined, 200 wins → Task<string>
  • The 201 integer schema is not used

Schema.V2SchemaCompilationTests (4 tests): verify Swagger 2.0 operation return types:

  • listPetsTask<Pet[]>
  • getPetTask<Pet>
  • getPet path parameter is int64
  • createPetTask<IO.Stream> (documents that Microsoft.OpenApi normalises a v2 201 with no schema to application/octet-stream)

Trade-offs

  • %20 vs + for spaces: RFC 3986 is the correct encoding for query components; + is application/x-www-form-urlencoded. Real-world APIs accept both, but %20 is strictly correct for query strings outside of HTML forms.
  • StringBuilder pre-allocates no extra capacity — for operations with no query params the result is just the trimmed path (same as the previous fast-path).

Test Status

  • dotnet fantomas --check: all files pass ✅
  • Unit tests: 509 total, 0 failed, 1 skipped ✅ (up from 500)
  • Provider integration tests: not run (require a live test server)

Generated by 🌈 Repo Assist, see workflow run. Learn more.

Add this agentic workflows to your repo

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@c02eadfca420f2b351f9fcaee883c507a63ca316

…d 9 tests (500→509)

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<Pet[]>, getPet→Task<Pet>,
  getPet path param int64, createPet→Task<IO.Stream> 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>
@sergey-tihon sergey-tihon marked this pull request as ready for review June 1, 2026 21:22
Copilot AI review requested due to automatic review settings June 1, 2026 21:22
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the runtime request/query construction logic to reduce allocations, and adds tests that document/verify expected return-type compilation behavior across OpenAPI v3 and Swagger v2 schemas.

Changes:

  • Reworked RuntimeHelpers.createHttpRequest query-string assembly to use StringBuilder + Uri.EscapeDataString instead of UriBuilder + ParseQueryString.
  • Streamlined toFormUrlEncodedContent to avoid an extra sequence pass.
  • Added new unit tests covering query percent-encoding behavior and schema operation return-type selection/priority rules.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/SwaggerProvider.Runtime/RuntimeHelpers.fs Replaces query-string building implementation and refactors form-url-encoding sequence processing.
tests/SwaggerProvider.Tests/RuntimeHelpersTests.fs Adds tests asserting RFC3986 percent-encoding behavior for query params.
tests/SwaggerProvider.Tests/Schema.OperationCompilationTests.fs Adds tests asserting 200 response schema priority over other 2xx responses.
tests/SwaggerProvider.Tests/Schema.V2SchemaCompilationTests.fs Adds tests validating generated method return types/parameters for a minimal Swagger v2 petstore schema.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/SwaggerProvider.Runtime/RuntimeHelpers.fs Outdated
Comment thread tests/SwaggerProvider.Tests/RuntimeHelpersTests.fs
@sergey-tihon
Copy link
Copy Markdown
Member

@copilot address inline comments

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 2, 2026

@copilot address inline comments

Addressed in commit 87b3425.

  • createHttpRequest now correctly appends parameters when address already has a query string, preserves fragment handling, and keeps the no-added-query fast path allocation-free.
  • Added regression tests for existing query (?existing=1) and fragment (#section) scenarios.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

@sergey-tihon sergey-tihon merged commit 8a0cc73 into master Jun 2, 2026
4 of 5 checks passed
@sergey-tihon sergey-tihon deleted the repo-assist/test-perf-june-20260601-479a53668e65139e branch June 2, 2026 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants