Skip to content

Commit a208c61

Browse files
github-actions[bot]CopilotCopilotsergey-tihon
authored
[repo-assist] refactor+test: extract compileSingleRefOrNewObject helper; add text/plain body, octet-stream response, path-level param tests (+
[Content truncated due to length] (#452) * refactor+test: extract compileSingleRefOrNewObject helper; add text/plain body, octet-stream response, path-level param tests (+9 tests, 465→474) Task 5: Extract the three identical allOf/oneOf/anyOf single-$ref collapse match arm bodies into a shared compileSingleRefOrNewObject local helper in DefinitionCompiler.compileBySchema. The semantics are unchanged — each of the three guards fires under the same conditions as before, but the 4-line inner match expression is replaced by a single call. This removes ~18 lines of duplicated code while keeping the guard conditions explicit and separate. Task 10: Add 9 unit tests covering three previously untested OperationCompiler behaviours: - text/plain request body → parameter named "textPlain" - application/octet-stream response → return type Task<IO.Stream> - Path-level parameters on PathItem → inherited by all operations on that path Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: trigger checks * fix: use typed field access in generated property getter/setter Agent-Logs-Url: https://github.com/fsprojects/SwaggerProvider/sessions/b2ed13f0-6603-43f9-a415-011fb299a1b6 Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> * fix: revert FieldGet/FieldSet to FieldGetUnchecked/FieldSetUnchecked The previous commit accidentally changed Expr.FieldGetUnchecked/FieldSetUnchecked to the checked variants Expr.FieldGet/FieldSet. ProvidedField instances require the Unchecked variants; the checked variants caused 100 type-inference errors in the ProviderTests (FS0072) because the generated property accessors produced expressions with incorrect type information. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: trigger checks * fix: restore typed field expressions for generated properties * fix: revert FieldGet/FieldSet to FieldGetUnchecked/FieldSetUnchecked Expr.FieldGet performs strict type-equality checks which rejects array types where the provided-field type is System.IO.Stream[] but the expression carries System.IO.Stream[*]. FieldGetUnchecked skips this check and was the working approach before the previous fix attempt. All 474 unit tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: trigger checks * fix: use fresh Var per call in coerceString/coerceQueryString to avoid duplicate quotation variable exceptions When coerceString or coerceQueryString was called for multiple parameters in the same operation, the quotation literal `<@ let x = ... @>` reused the same Var object on every call. Composing multiple such quotations into one invokeCode expression caused the ProvidedTypes/quotation compiler to throw "An item with the same key has already been added. Key: x" (or 'o') for any operation with two or more path/header/cookie/query parameters. Fix: replace the static quotation literals with Expr.Let + a freshly constructed Var on each call, ensuring each binding has its own unique Var identity. * style: apply Fantomas formatting * fix: eliminate duplicate Var by building Expr.Call directly instead of let-binding The original coerceString used a quotation literal: <@ let x = (%obj) in RuntimeHelpers.toParam x @> F# compiles quotation literals to static data, so the Var("x") inside the literal is the same object on every call. When an operation has two or more path/header/cookie parameters, coerceString is called once per parameter and the fold combines the results. The resulting expression tree contained the same Var object bound by multiple Let nodes. ProvidedTypes' IL emitter does localsMap.Add(v, lb) for each Let(v,...) node — since all Let nodes shared the same Var object, the second Add threw "An item with the same key has already been added. Key: x". The same problem affected coerceQueryString with Var("o"). Fix: replace the quotation literals entirely with Expr.Call, extracting the MethodInfo once via a quotation pattern match and then building the call node directly. This avoids any Let binding and any shared Var, so each invocation produces a structurally independent expression. --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> Co-authored-by: Sergey Tihon <sergey.tihon@gmail.com>
1 parent 16a1865 commit a208c61

3 files changed

Lines changed: 192 additions & 21 deletions

File tree

src/SwaggerProvider.DesignTime/DefinitionCompiler.fs

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,17 @@ type DefinitionCompiler(schema: OpenApiDocument, provideNullable, useDateOnly: b
441441
else
442442
DefinitionPath.DefinitionPrefix + refId
443443

444+
// Helper for the allOf/oneOf/anyOf single-ref collapse pattern.
445+
// When `schemas` has exactly one entry that is a $ref and the outer schema has no
446+
// own properties, collapse directly to the referenced type; otherwise fall back to
447+
// compileNewObject so composite/inline schemas are handled as usual.
448+
let compileSingleRefOrNewObject(schemas: System.Collections.Generic.IList<IOpenApiSchema>) =
449+
match schemas.[0] with
450+
| :? OpenApiSchemaReference as schemaRef when not(isNull schemaRef.Reference) ->
451+
ns.ReleaseNameReservation tyName
452+
compileByPath <| getFullPath schemaRef.Reference.Id
453+
| _ -> compileNewObject()
454+
444455
let tyType =
445456
match schemaObj with
446457
| null -> failwithf $"Cannot compile object '%s{tyName}' when schema is 'null'"
@@ -472,39 +483,26 @@ type DefinitionCompiler(schema: OpenApiDocument, provideNullable, useDateOnly: b
472483
compileBySchema ns (ns.ReserveUniqueName tyName "Item") elSchema true ns.RegisterType false
473484

474485
ProvidedTypeBuilder.MakeGenericType(typedefof<Map<string, obj>>, [ typeof<string>; elTy ])
475-
// Handle allOf with single reference (e.g., nullable reference to another type)
486+
// Handle allOf/oneOf/anyOf with a single $ref and no own properties:
487+
// collapse the wrapper to the referenced type directly.
476488
| _ when
477489
not(isNull schemaObj.AllOf)
478490
&& schemaObj.AllOf.Count = 1
479491
&& (schemaObj.Properties |> isNull || schemaObj.Properties.Count = 0)
480492
->
481-
match schemaObj.AllOf.[0] with
482-
| :? OpenApiSchemaReference as schemaRef when not(isNull schemaRef.Reference) ->
483-
ns.ReleaseNameReservation tyName
484-
compileByPath <| getFullPath schemaRef.Reference.Id
485-
| _ -> compileNewObject()
486-
// Handle oneOf with single reference (resolves to the referenced type)
493+
compileSingleRefOrNewObject schemaObj.AllOf
487494
| _ when
488495
not(isNull schemaObj.OneOf)
489496
&& schemaObj.OneOf.Count = 1
490497
&& (schemaObj.Properties |> isNull || schemaObj.Properties.Count = 0)
491498
->
492-
match schemaObj.OneOf.[0] with
493-
| :? OpenApiSchemaReference as schemaRef when not(isNull schemaRef.Reference) ->
494-
ns.ReleaseNameReservation tyName
495-
compileByPath <| getFullPath schemaRef.Reference.Id
496-
| _ -> compileNewObject()
497-
// Handle anyOf with single reference (resolves to the referenced type)
499+
compileSingleRefOrNewObject schemaObj.OneOf
498500
| _ when
499501
not(isNull schemaObj.AnyOf)
500502
&& schemaObj.AnyOf.Count = 1
501503
&& (schemaObj.Properties |> isNull || schemaObj.Properties.Count = 0)
502504
->
503-
match schemaObj.AnyOf.[0] with
504-
| :? OpenApiSchemaReference as schemaRef when not(isNull schemaRef.Reference) ->
505-
ns.ReleaseNameReservation tyName
506-
compileByPath <| getFullPath schemaRef.Reference.Id
507-
| _ -> compileNewObject()
505+
compileSingleRefOrNewObject schemaObj.AnyOf
508506
| _ when
509507
resolvedType.IsNone
510508
|| resolvedType = Some JsonSchemaType.Object

src/SwaggerProvider.DesignTime/OperationCompiler.fs

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ open System.Text.Json
77

88
open Microsoft.FSharp.Quotations
99
open Microsoft.FSharp.Quotations.ExprShape
10+
open Microsoft.FSharp.Quotations.Patterns
1011
open Microsoft.OpenApi
1112
open ProviderImplementation.ProvidedTypes
1213
open FSharp.Data.Runtime.NameUtils
@@ -319,13 +320,29 @@ type OperationCompiler(schema: OpenApiDocument, defCompiler: DefinitionCompiler,
319320
| _ -> failwithf $"Function '%s{providedMethodName}' does not support functions as arguments.")
320321

321322
// Makes argument a string // TODO: Make body an exception
323+
// NOTE: avoid `let x = ...` in quotation literals — they share a single Var
324+
// object across all calls, causing "duplicate key" exceptions in ProvidedTypes
325+
// when the same helper is called for multiple parameters in one operation.
326+
// Instead, build the call expression directly without an intermediate binding.
327+
let toParamMethod =
328+
match <@@ RuntimeHelpers.toParam(null) @@> with
329+
| Call(None, m, _) -> m
330+
| _ -> failwith "Cannot extract toParam MethodInfo"
331+
322332
let coerceString exp =
323-
let obj = Expr.Coerce(exp, typeof<obj>) |> Expr.Cast<obj>
324-
<@ let x = (%obj) in RuntimeHelpers.toParam x @>
333+
let obj = Expr.Coerce(exp, typeof<obj>)
334+
Expr.Call(toParamMethod, [ obj ]) |> Expr.Cast<string>
335+
336+
let toQueryParamsMethod =
337+
match <@@ RuntimeHelpers.toQueryParams "" null (%this) @@> with
338+
| Call(None, m, _) -> m
339+
| _ -> failwith "Cannot extract toQueryParams MethodInfo"
325340

326341
let rec coerceQueryString name expr =
327342
let obj = Expr.Coerce(expr, typeof<obj>)
328-
<@ let o = (%%obj: obj) in RuntimeHelpers.toQueryParams name o (%this) @>
343+
344+
Expr.Call(toQueryParamsMethod, [ Expr.Value name; obj; this ])
345+
|> Expr.Cast<(string * string) list>
329346

330347
// Partitions arguments based on their locations
331348
let path, queryParams, headers =

tests/SwaggerProvider.Tests/Schema.OperationCompilationTests.fs

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1113,6 +1113,162 @@ let ``ignoreOperationId=true does not generate the original operationId as metho
11131113
methodNames |> shouldNotContain "ListAllPets"
11141114
methodNames |> shouldNotContain "GetPetById"
11151115

1116+
// ── text/plain request body ───────────────────────────────────────────────────
1117+
1118+
let private textPlainBodySchema =
1119+
"""openapi: "3.0.0"
1120+
info:
1121+
title: TextPlainBodyTest
1122+
version: "1.0.0"
1123+
paths:
1124+
/echo:
1125+
post:
1126+
operationId: echoText
1127+
requestBody:
1128+
required: true
1129+
content:
1130+
text/plain:
1131+
schema:
1132+
type: string
1133+
responses:
1134+
"200":
1135+
description: OK
1136+
components:
1137+
schemas: {}
1138+
"""
1139+
1140+
[<Fact>]
1141+
let ``text/plain request body generates a method``() =
1142+
let types = compileTaskSchema textPlainBodySchema
1143+
let method = findMethod types "EchoText"
1144+
method.IsSome |> shouldEqual true
1145+
1146+
[<Fact>]
1147+
let ``text/plain request body parameter is named textPlain``() =
1148+
let types = compileTaskSchema textPlainBodySchema
1149+
let method = (findMethod types "EchoText").Value
1150+
let paramNames = method.GetParameters() |> Array.map(fun p -> p.Name)
1151+
paramNames |> shouldContain "textPlain"
1152+
1153+
[<Fact>]
1154+
let ``text/plain request body has CancellationToken as last parameter``() =
1155+
let types = compileTaskSchema textPlainBodySchema
1156+
let method = (findMethod types "EchoText").Value
1157+
let lastParam = method.GetParameters() |> Array.last
1158+
lastParam.ParameterType |> shouldEqual typeof<CancellationToken>
1159+
1160+
// ── application/octet-stream response ────────────────────────────────────────
1161+
1162+
let private octetStreamResponseSchema =
1163+
"""openapi: "3.0.0"
1164+
info:
1165+
title: OctetStreamResponseTest
1166+
version: "1.0.0"
1167+
paths:
1168+
/file:
1169+
get:
1170+
operationId: downloadFile
1171+
responses:
1172+
"200":
1173+
description: File contents
1174+
content:
1175+
application/octet-stream:
1176+
schema:
1177+
type: string
1178+
format: binary
1179+
components:
1180+
schemas: {}
1181+
"""
1182+
1183+
[<Fact>]
1184+
let ``octet-stream response generates a method``() =
1185+
let types = compileTaskSchema octetStreamResponseSchema
1186+
let method = findMethod types "DownloadFile"
1187+
method.IsSome |> shouldEqual true
1188+
1189+
[<Fact>]
1190+
let ``octet-stream response produces Task<IO.Stream> return type``() =
1191+
let types = compileTaskSchema octetStreamResponseSchema
1192+
let method = (findMethod types "DownloadFile").Value
1193+
method.ReturnType.IsGenericType |> shouldEqual true
1194+
1195+
method.ReturnType.GetGenericTypeDefinition()
1196+
|> shouldEqual typedefof<Task<_>>
1197+
1198+
method.ReturnType.GetGenericArguments()[0]
1199+
|> shouldEqual typeof<IO.Stream>
1200+
1201+
[<Fact>]
1202+
let ``octet-stream response has CancellationToken as its only parameter``() =
1203+
let types = compileTaskSchema octetStreamResponseSchema
1204+
let method = (findMethod types "DownloadFile").Value
1205+
let parameters = method.GetParameters()
1206+
parameters.Length |> shouldEqual 1
1207+
parameters[0].ParameterType |> shouldEqual typeof<CancellationToken>
1208+
1209+
// ── Path-level parameters (inherited from PathItem) ───────────────────────────
1210+
1211+
let private pathLevelParamSchema =
1212+
"""openapi: "3.0.0"
1213+
info:
1214+
title: PathLevelParamTest
1215+
version: "1.0.0"
1216+
paths:
1217+
/users/{userId}/posts:
1218+
parameters:
1219+
- name: userId
1220+
in: path
1221+
required: true
1222+
schema:
1223+
type: integer
1224+
get:
1225+
operationId: getUserPosts
1226+
responses:
1227+
"200":
1228+
description: OK
1229+
content:
1230+
application/json:
1231+
schema:
1232+
type: array
1233+
items:
1234+
type: string
1235+
post:
1236+
operationId: createUserPost
1237+
requestBody:
1238+
required: true
1239+
content:
1240+
application/json:
1241+
schema:
1242+
type: string
1243+
responses:
1244+
"201":
1245+
description: Created
1246+
components:
1247+
schemas: {}
1248+
"""
1249+
1250+
[<Fact>]
1251+
let ``path-level parameter is inherited by GET operation``() =
1252+
let types = compileTaskSchema pathLevelParamSchema
1253+
let method = (findMethod types "GetUserPosts").Value
1254+
let paramNames = method.GetParameters() |> Array.map(fun p -> p.Name)
1255+
paramNames |> shouldContain "userId"
1256+
1257+
[<Fact>]
1258+
let ``path-level parameter is required with correct type``() =
1259+
let types = compileTaskSchema pathLevelParamSchema
1260+
let method = (findMethod types "GetUserPosts").Value
1261+
let userIdParam = method.GetParameters() |> Array.find(fun p -> p.Name = "userId")
1262+
userIdParam.ParameterType |> shouldEqual typeof<int32>
1263+
userIdParam.IsOptional |> shouldEqual false
1264+
1265+
[<Fact>]
1266+
let ``path-level parameter is inherited by POST operation``() =
1267+
let types = compileTaskSchema pathLevelParamSchema
1268+
let method = (findMethod types "CreateUserPost").Value
1269+
let paramNames = method.GetParameters() |> Array.map(fun p -> p.Name)
1270+
paramNames |> shouldContain "userId"
1271+
11161272
// ── asAsync=true: octet-stream response produces Async<IO.Stream> ─────────────
11171273

11181274
let private octetStreamResponseAsyncSchema =

0 commit comments

Comments
 (0)