Skip to content

Commit 3f25c1a

Browse files
panesofglassclaude
andcommitted
fix: resolve cross-type case-name collisions in registered component ids
document.AddComponent is TryAdd (first writer wins, never throws), and component ids were bare case/definition names — so two different F# types sharing a case name (e.g. both having a "Leaf" or "Error" case) would silently collide in the same OpenAPI document, with the second type's schema never actually registering under its own definition. Qualify every definition id with its owning root type's name (rootTypeId + "." + typeId) when bound to a live document; the document-agnostic translate() entry point is unaffected (qualify is identity when document = None), so all pre-existing structural tests keep passing unchanged. The "." separator is required, not cosmetic: without it, different (rootTypeId, typeId) pairs can concatenate to the same string (e.g. "Order"+"LineItem" = "OrderLine"+"Item"). Also addresses the remaining deferred Minor findings from final review: precondition guard on rootTypeId, doc-comment scope fixes, restored net9 comment, and a WebApplication dispose leak in the e2e test fixtures. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent e5cdd12 commit 3f25c1a

5 files changed

Lines changed: 114 additions & 27 deletions

File tree

RELEASE_NOTES.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* Fix `InvalidOperationException: The input schema must be an OpenApiSchema or OpenApiSchemaReference` thrown by `Microsoft.AspNetCore.OpenApi` 10.0.3+ for any endpoint whose request/response type is (or contains) an F# discriminated union or a self-recursive type (#30)
44
* `FSharpSchemaTransformer` now registers component schemas into the live `OpenApiDocument` on net10 instead of leaving them unregistered with dangling references
55
* Fix self-recursive types (e.g. a tree or linked-list shaped DU) all resolving their self-reference to the same hardcoded component id on net10; each now gets its own correctly-named component
6+
* On net10, definition/component ids (DU cases and nested referenced types alike) are now qualified by their owning root type's name (e.g. `TreeNode.Leaf` instead of `Leaf`), so two different types that happen to share a case or type name no longer silently collide in `components/schemas`
67

78
### FSharp.Data.JsonSchema.NJsonSchema 3.1.0
89

src/FSharp.Data.JsonSchema.OpenApi/FSharpSchemaTransformer.fs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,9 @@ type FSharpSchemaTransformer(config: SchemaGeneratorConfig) =
9191
| null -> OpenApiSchemaTranslator.translate doc
9292
| document -> OpenApiSchemaTranslator.translateForDocument doc rootTypeId document
9393
#else
94+
// net9 (Microsoft.OpenApi.Models) has no live-document concept to register
95+
// components into — component schemas are never registered here, unchanged
96+
// from before this fix; only the net10 path (above) registers them.
9497
let (translatedRoot, _componentSchemas) = OpenApiSchemaTranslator.translate doc
9598
#endif
9699

src/FSharp.Data.JsonSchema.OpenApi/OpenApiSchemaTranslator.fs

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,21 @@ module OpenApiSchemaTranslator =
120120
let componentSchemas = Collections.Generic.Dictionary<string, OpenApiSchema>()
121121
let rootSchema = mkSchema ()
122122

123+
// Case/definition-level ids are qualified with `rootTypeId` only when bound to a live
124+
// document, so two different types that happen to share a case name (e.g. both having
125+
// an "Error" case) register distinct components instead of the second silently
126+
// overwriting the first. The document-agnostic `translate` entry point (document = None)
127+
// keeps ids unqualified, since it never registers into a shared namespace.
128+
// The "." separator (valid in OpenAPI component ids) is required, not cosmetic: without
129+
// it, two different (rootTypeId, typeId) pairs can concatenate to the same string (e.g.
130+
// "Order" + "LineItem" = "OrderLine" + "Item" = "OrderLineItem") — same collision class
131+
// this qualification exists to close, just narrower. Neither a .NET Type.Name nor an
132+
// F# union case name can contain ".", so the split stays unambiguous.
133+
let qualify (typeId: string) : string =
134+
match document with
135+
| Some _ -> rootTypeId + "." + typeId
136+
| None -> typeId
137+
123138
let rec translateNode (node: SchemaNode) : OpenApiSchema =
124139
match node with
125140
| SchemaNode.Object obj ->
@@ -178,7 +193,7 @@ module OpenApiSchemaTranslator =
178193
schema
179194

180195
| SchemaNode.Ref typeId ->
181-
let resolvedId = if typeId = "#" then rootTypeId else typeId
196+
let resolvedId = if typeId = "#" then rootTypeId else qualify typeId
182197
#if NET10_0_OR_GREATER
183198
mkRefSchema document resolvedId
184199
#else
@@ -205,9 +220,10 @@ module OpenApiSchemaTranslator =
205220
// document (when supplied) as it's produced.
206221
for (key, value) in doc.Definitions do
207222
let componentSchema = translateNode value
208-
componentSchemas.[key] <- componentSchema
223+
let registeredKey = qualify key
224+
componentSchemas.[registeredKey] <- componentSchema
209225
#if NET10_0_OR_GREATER
210-
document |> Option.iter (fun d -> d.AddComponent(key, componentSchema) |> ignore)
226+
document |> Option.iter (fun d -> d.AddComponent(registeredKey, componentSchema) |> ignore)
211227
#endif
212228

213229
// Translate root
@@ -250,17 +266,23 @@ module OpenApiSchemaTranslator =
250266
(result, componentSchemas |> Seq.map (fun kv -> kv.Key, kv.Value) |> Map.ofSeq)
251267

252268
/// Translate a SchemaDocument to an OpenApiSchema and component schemas.
253-
/// Self-refs and component references are unbound (no host document) — suitable for
254-
/// structural inspection but not for live OpenAPI document generation, where
255-
/// `translateForDocument` must be used instead so references actually resolve.
269+
/// On net10, self-refs and component references are left unbound (no host document) —
270+
/// suitable for structural inspection but not for live OpenAPI document generation,
271+
/// where `translateForDocument` must be used instead so references actually resolve.
272+
/// On net9, references are always produced via `OpenApiReference` metadata directly;
273+
/// this distinction doesn't apply there.
256274
let translate (doc: SchemaDocument) : OpenApiSchema * Map<string, OpenApiSchema> =
257275
translateCore doc "root" None
258276

259277
#if NET10_0_OR_GREATER
260278
/// Translate a SchemaDocument, binding component and self-ref references to a live
261279
/// OpenApiDocument so they resolve correctly, and registering component schemas
262280
/// — including the root schema itself, under `rootTypeId`, whenever there are any
263-
/// definitions — into `document.Components.Schemas`.
281+
/// definitions — into `document.Components.Schemas`. The returned component map
282+
/// mirrors `translate`'s signature; registration already happened as a side effect
283+
/// against `document`, so callers that only need the live document can discard it.
264284
let translateForDocument (doc: SchemaDocument) (rootTypeId: string) (document: OpenApiDocument) : OpenApiSchema * Map<string, OpenApiSchema> =
285+
if String.IsNullOrEmpty rootTypeId then
286+
invalidArg (nameof rootTypeId) "rootTypeId must not be null or empty"
265287
translateCore doc rootTypeId (Some document)
266288
#endif

test/FSharp.Data.JsonSchema.OpenApi.Tests/EndToEndTests.fs

Lines changed: 55 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,13 @@ type LinkedNode =
2929
| Empty
3030
| Node of value: int * next: LinkedNode
3131

32+
/// Shares a case name ("Leaf") with TreeNode, to prove case-name collisions
33+
/// across different types are resolved by qualifying component ids with the
34+
/// owning type's name.
35+
type Plant =
36+
| Leaf of species: string
37+
| Root of Plant
38+
3239
let private startApp (mapEndpoints: WebApplication -> unit) : WebApplication =
3340
let builder = WebApplication.CreateBuilder()
3441
builder.Logging.ClearProviders() |> ignore
@@ -40,10 +47,18 @@ let private startApp (mapEndpoints: WebApplication -> unit) : WebApplication =
4047
options.AddSchemaTransformer(FSharpSchemaTransformer()) |> ignore
4148
) |> ignore
4249
let app = builder.Build()
43-
app.MapOpenApi() |> ignore
44-
mapEndpoints app
45-
app.StartAsync().GetAwaiter().GetResult()
46-
app
50+
try
51+
app.MapOpenApi() |> ignore
52+
mapEndpoints app
53+
app.StartAsync().GetAwaiter().GetResult()
54+
app
55+
with _ ->
56+
app.DisposeAsync().AsTask().GetAwaiter().GetResult()
57+
reraise ()
58+
59+
let private stopApp (app: WebApplication) : unit =
60+
app.StopAsync().GetAwaiter().GetResult()
61+
app.DisposeAsync().AsTask().GetAwaiter().GetResult()
4762

4863
let private baseAddress (app: WebApplication) : string =
4964
let server = app.Services.GetRequiredService<IServer>()
@@ -75,36 +90,36 @@ let endToEndTests =
7590
use jsonDoc = JsonDocument.Parse body
7691
#if NET10_0_OR_GREATER
7792
let root = jsonDoc.RootElement
78-
Expect.isTrue (hasSchema root "Circle") "Circle registered as a component schema"
79-
Expect.isTrue (hasSchema root "Rectangle") "Rectangle registered as a component schema"
80-
Expect.isTrue (hasSchema root "Point") "Point registered as a component schema"
93+
Expect.isTrue (hasSchema root "Shape.Circle") "Circle registered as a component schema, qualified by its type"
94+
Expect.isTrue (hasSchema root "Shape.Rectangle") "Rectangle registered as a component schema, qualified by its type"
95+
Expect.isTrue (hasSchema root "Shape.Point") "Point registered as a component schema, qualified by its type"
8196
#endif
8297
()
8398
finally
84-
app.StopAsync().GetAwaiter().GetResult()
99+
stopApp app
85100
}
86101

87102
test "OpenAPI document generation succeeds for a self-recursive discriminated union" {
88-
let app = startApp (fun app -> app.MapGet("/tree", System.Func<TreeNode>(fun () -> Leaf 1)) |> ignore)
103+
let app = startApp (fun app -> app.MapGet("/tree", System.Func<TreeNode>(fun () -> TreeNode.Leaf 1)) |> ignore)
89104
try
90105
let (status, body) = getOpenApiDocument app
91106
Expect.equal status System.Net.HttpStatusCode.OK "200 OK, not the InvalidOperationException from #30"
92107
use jsonDoc = JsonDocument.Parse body
93108
#if NET10_0_OR_GREATER
94109
let root = jsonDoc.RootElement
95-
Expect.isTrue (hasSchema root "Leaf") "Leaf registered as a component schema"
96-
Expect.isTrue (hasSchema root "Branch") "Branch registered as a component schema"
110+
Expect.isTrue (hasSchema root "TreeNode.Leaf") "Leaf registered as a component schema, qualified by its type"
111+
Expect.isTrue (hasSchema root "TreeNode.Branch") "Branch registered as a component schema, qualified by its type"
97112
Expect.isTrue (hasSchema root "TreeNode") "TreeNode root registered as a component schema (self-ref target)"
98113
#endif
99114
()
100115
finally
101-
app.StopAsync().GetAwaiter().GetResult()
116+
stopApp app
102117
}
103118

104119
test "OpenAPI document generation registers distinct components for two different self-recursive types" {
105120
let app =
106121
startApp (fun app ->
107-
app.MapGet("/tree", System.Func<TreeNode>(fun () -> Leaf 1)) |> ignore
122+
app.MapGet("/tree", System.Func<TreeNode>(fun () -> TreeNode.Leaf 1)) |> ignore
108123
app.MapGet("/linked", System.Func<LinkedNode>(fun () -> Empty)) |> ignore
109124
)
110125
try
@@ -114,14 +129,36 @@ let endToEndTests =
114129
#if NET10_0_OR_GREATER
115130
let root = jsonDoc.RootElement
116131
Expect.isTrue (hasSchema root "TreeNode") "TreeNode root registered as a component schema"
117-
Expect.isTrue (hasSchema root "Leaf") "Leaf registered as a component schema"
118-
Expect.isTrue (hasSchema root "Branch") "Branch registered as a component schema"
132+
Expect.isTrue (hasSchema root "TreeNode.Leaf") "Leaf registered as a component schema, qualified by its type"
133+
Expect.isTrue (hasSchema root "TreeNode.Branch") "Branch registered as a component schema, qualified by its type"
119134
Expect.isTrue (hasSchema root "LinkedNode") "LinkedNode root registered as a component schema"
120-
Expect.isTrue (hasSchema root "Empty") "Empty registered as a component schema"
121-
Expect.isTrue (hasSchema root "Node") "Node registered as a component schema"
135+
Expect.isTrue (hasSchema root "LinkedNode.Empty") "Empty registered as a component schema, qualified by its type"
136+
Expect.isTrue (hasSchema root "LinkedNode.Node") "Node registered as a component schema, qualified by its type"
137+
#endif
138+
()
139+
finally
140+
stopApp app
141+
}
142+
143+
test "OpenAPI document generation disambiguates two different types sharing a case name" {
144+
let app =
145+
startApp (fun app ->
146+
app.MapGet("/tree", System.Func<TreeNode>(fun () -> TreeNode.Leaf 1)) |> ignore
147+
app.MapGet("/plant", System.Func<Plant>(fun () -> Plant.Leaf "fern")) |> ignore
148+
)
149+
try
150+
let (status, body) = getOpenApiDocument app
151+
Expect.equal status System.Net.HttpStatusCode.OK "200 OK, not the InvalidOperationException from #30"
152+
use jsonDoc = JsonDocument.Parse body
153+
#if NET10_0_OR_GREATER
154+
let root = jsonDoc.RootElement
155+
// TreeNode and Plant both have a case named "Leaf" — proving neither
156+
// silently overwrites the other's component is the point of this test.
157+
Expect.isTrue (hasSchema root "TreeNode.Leaf") "TreeNode's Leaf case registered under its own qualified id"
158+
Expect.isTrue (hasSchema root "Plant.Leaf") "Plant's Leaf case registered under its own qualified id"
122159
#endif
123160
()
124161
finally
125-
app.StopAsync().GetAwaiter().GetResult()
162+
stopApp app
126163
}
127164
]

test/FSharp.Data.JsonSchema.OpenApi.Tests/TranslatorTests.fs

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,18 +240,42 @@ let documentBindingTests =
240240
let (schema: OASchema, _components) =
241241
OpenApiSchemaTranslator.translateForDocument doc "Root" document
242242
Expect.isNotNull (document.Components :> obj) "components created"
243-
Expect.isTrue (document.Components.Schemas.ContainsKey "A") "A registered"
244-
Expect.isTrue (document.Components.Schemas.ContainsKey "B") "B registered"
243+
// Definition ids are qualified with rootTypeId ("Root") when bound to a live
244+
// document, so "A"/"B" register as "Root.A"/"Root.B" — see the case-collision
245+
// test below for why.
246+
Expect.isTrue (document.Components.Schemas.ContainsKey "Root.A") "A registered under its qualified id"
247+
Expect.isTrue (document.Components.Schemas.ContainsKey "Root.B") "B registered under its qualified id"
245248
// Ref nodes are translated by mkRefSchema into a schema whose sole AnyOf
246249
// element is the OpenApiSchemaReference; translateNode's return type is
247250
// OpenApiSchema (not IOpenApiSchema), so the reference can't be returned
248251
// unwrapped directly into the outer AnyOf collection — mirrors the unwrap
249252
// pattern used below for the self-ref case.
250253
let refWrapperA = schema.AnyOf.[0] :?> OASchema
251254
let refA = refWrapperA.AnyOf.[0] :?> OpenApiSchemaReference
255+
Expect.equal refA.Reference.Id "Root.A" "ref points at the qualified id"
252256
Expect.isNotNull (refA.Target :> obj) "ref A resolves to a non-null target"
253257
}
254258

259+
test "translateForDocument qualifies case ids so two types sharing a case name don't collide" {
260+
let docTreeNode = {
261+
Root = SchemaNode.Ref "Leaf"
262+
Definitions = [ "Leaf", SchemaNode.Primitive(PrimitiveType.String, None) ]
263+
}
264+
let docPlant = {
265+
Root = SchemaNode.Ref "Leaf"
266+
Definitions = [ "Leaf", SchemaNode.Primitive(PrimitiveType.Integer, Some "int32") ]
267+
}
268+
let document = OpenApiDocument()
269+
OpenApiSchemaTranslator.translateForDocument docTreeNode "TreeNode" document |> ignore
270+
OpenApiSchemaTranslator.translateForDocument docPlant "Plant" document |> ignore
271+
Expect.isTrue (document.Components.Schemas.ContainsKey "TreeNode.Leaf") "TreeNode's Leaf case registered under a qualified id"
272+
Expect.isTrue (document.Components.Schemas.ContainsKey "Plant.Leaf") "Plant's Leaf case registered under a qualified id"
273+
let treeLeaf = document.Components.Schemas.["TreeNode.Leaf"] :?> OASchema
274+
let plantLeaf = document.Components.Schemas.["Plant.Leaf"] :?> OASchema
275+
Expect.equal treeLeaf.Type (System.Nullable(JsonSchemaType.String)) "TreeNode's Leaf kept its own (string) type, not overwritten by Plant's"
276+
Expect.equal plantLeaf.Type (System.Nullable(JsonSchemaType.Integer)) "Plant's Leaf kept its own (integer) type, not overwritten by TreeNode's"
277+
}
278+
255279
test "translateForDocument binds self-ref to the given rootTypeId and registers the root component" {
256280
let doc = {
257281
Root = SchemaNode.Object {

0 commit comments

Comments
 (0)