Skip to content

Commit 07e3390

Browse files
Smaug123claude
Smaug123
and
claude
authored
Represent "default" responses instead of crashing on them (#543)
Response keys were unconditionally Int32.Parsed, so any valid Swagger 2 spec containing a "default" response aborted generation with a FormatException. Responses are now keyed by a ResponseKey DU (Code of int | Default), and the client generator explicitly ignores Default when selecting the success response: it describes the status codes not otherwise listed, which in practice means errors. No generated output changes: Gitea declares no default responses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 08043ab commit 07e3390

5 files changed

Lines changed: 278 additions & 23 deletions

File tree

‎WoofWare.Myriad.Plugins.Test/TestSwagger/TestSuccessResponse.fs‎

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
namespace WoofWare.Myriad.Plugins.Test
2+
3+
open FsCheck
4+
open FsCheck.FSharp
5+
open NUnit.Framework
6+
open FsUnitTyped
7+
open WoofWare.Myriad.Plugins
8+
open WoofWare.Myriad.Plugins.SwaggerV2
9+
10+
[<TestFixture>]
11+
module TestSuccessResponse =
12+
13+
/// Distinct definitions, so that we can tell which one came back.
14+
let private defnGen : Gen<Definition> =
15+
[
16+
Definition.String
17+
Definition.Boolean
18+
Definition.Unspecified
19+
Definition.File
20+
Definition.Handle "#/definitions/Error"
21+
Definition.Integer (Some "int64")
22+
]
23+
|> Gen.elements
24+
25+
let private successCodeGen : Gen<ResponseKey> =
26+
Gen.choose (200, 299) |> Gen.map ResponseKey.Code
27+
28+
/// Any key which is neither an explicit 2xx code nor `default`.
29+
let private errorCodeGen : Gen<ResponseKey> =
30+
Gen.oneof [ Gen.choose (100, 199) ; Gen.choose (300, 599) ]
31+
|> Gen.map ResponseKey.Code
32+
33+
let private mapGen (keys : Gen<ResponseKey>) : Gen<Map<ResponseKey, Definition>> =
34+
gen {
35+
let! keys = Gen.listOf keys
36+
let keys = List.distinct keys
37+
let! values = Gen.listOfLength keys.Length defnGen
38+
return List.zip keys values |> Map.ofList
39+
}
40+
41+
/// An arbitrary response map: some 2xx codes, some error codes, maybe a `default`.
42+
let private responsesGen : Gen<Map<ResponseKey, Definition>> =
43+
gen {
44+
let! successes = mapGen successCodeGen
45+
let! errors = mapGen errorCodeGen
46+
let! deflt = Gen.optionOf defnGen
47+
48+
let deflt =
49+
match deflt with
50+
| None -> Map.empty
51+
| Some d -> Map.ofList [ ResponseKey.Default, d ]
52+
53+
return
54+
Seq.concat [ Map.toSeq successes ; Map.toSeq errors ; Map.toSeq deflt ]
55+
|> Map.ofSeq
56+
}
57+
58+
[<Test>]
59+
let ``an Exactly result is always one of the declared schemas`` () : unit =
60+
let property (responses : Map<ResponseKey, Definition>) : bool =
61+
match SwaggerClientGenerator.successResponse responses with
62+
| SuccessResponse.Exactly defn -> responses |> Map.exists (fun _ v -> v = defn)
63+
| SuccessResponse.Ambiguous
64+
| SuccessResponse.Missing -> true
65+
66+
Prop.forAll (Arb.fromGen responsesGen) property |> Check.QuickThrowOnFailure
67+
68+
[<Test>]
69+
let ``responses for non-2xx codes never affect the result`` () : unit =
70+
let property (responses : Map<ResponseKey, Definition>, errors : Map<ResponseKey, Definition>) : bool =
71+
let withoutErrors =
72+
responses
73+
|> Map.filter (fun k _ ->
74+
match k with
75+
| ResponseKey.Code code -> 200 <= code && code < 300
76+
| ResponseKey.Default -> true
77+
)
78+
79+
let withErrors =
80+
(withoutErrors, Map.toSeq errors)
81+
||> Seq.fold (fun acc (k, v) -> Map.add k v acc)
82+
83+
SwaggerClientGenerator.successResponse withErrors = SwaggerClientGenerator.successResponse withoutErrors
84+
85+
Prop.forAll (Arb.fromGen (Gen.zip responsesGen (mapGen errorCodeGen))) property
86+
|> Check.QuickThrowOnFailure
87+
88+
[<Test>]
89+
let ``the default response is ignored whenever an explicit 2xx response exists`` () : unit =
90+
let property (responses : Map<ResponseKey, Definition>, deflt : Definition) : bool =
91+
let hasSuccess =
92+
responses
93+
|> Map.exists (fun k _ ->
94+
match k with
95+
| ResponseKey.Code code -> 200 <= code && code < 300
96+
| ResponseKey.Default -> false
97+
)
98+
99+
if not hasSuccess then
100+
true
101+
else
102+
103+
let withoutDefault = Map.remove ResponseKey.Default responses
104+
let withDefault = Map.add ResponseKey.Default deflt responses
105+
106+
SwaggerClientGenerator.successResponse withDefault = SwaggerClientGenerator.successResponse withoutDefault
107+
108+
Prop.forAll (Arb.fromGen (Gen.zip responsesGen defnGen)) property
109+
|> Check.QuickThrowOnFailure
110+
111+
[<Test>]
112+
let ``the default response is the return type whenever no explicit 2xx response exists`` () : unit =
113+
let property (errors : Map<ResponseKey, Definition>, deflt : Definition) : bool =
114+
let responses = Map.add ResponseKey.Default deflt errors
115+
116+
SwaggerClientGenerator.successResponse responses = SuccessResponse.Exactly deflt
117+
118+
Prop.forAll (Arb.fromGen (Gen.zip (mapGen errorCodeGen) defnGen)) property
119+
|> Check.QuickThrowOnFailure
120+
121+
[<Test>]
122+
let ``exactly one 2xx response is that response`` () : unit =
123+
[
124+
ResponseKey.Code 200, Definition.String
125+
ResponseKey.Default, Definition.Handle "#/definitions/Error"
126+
]
127+
|> Map.ofList
128+
|> SwaggerClientGenerator.successResponse
129+
|> shouldEqual (SuccessResponse.Exactly Definition.String)
130+
131+
[<Test>]
132+
let ``a default-only response map falls back to the default`` () : unit =
133+
[ ResponseKey.Default, Definition.Handle "#/definitions/Error" ]
134+
|> Map.ofList
135+
|> SwaggerClientGenerator.successResponse
136+
|> shouldEqual (SuccessResponse.Exactly (Definition.Handle "#/definitions/Error"))
137+
138+
[<Test>]
139+
let ``errors plus a default falls back to the default`` () : unit =
140+
[
141+
ResponseKey.Code 404, Definition.Unspecified
142+
ResponseKey.Default, Definition.Handle "#/definitions/Error"
143+
]
144+
|> Map.ofList
145+
|> SwaggerClientGenerator.successResponse
146+
|> shouldEqual (SuccessResponse.Exactly (Definition.Handle "#/definitions/Error"))
147+
148+
[<Test>]
149+
let ``no success response and no default is Missing`` () : unit =
150+
[ ResponseKey.Code 404, Definition.Unspecified ]
151+
|> Map.ofList
152+
|> SwaggerClientGenerator.successResponse
153+
|> shouldEqual SuccessResponse.Missing
154+
155+
[<Test>]
156+
let ``an empty response map is Missing`` () : unit =
157+
Map.empty
158+
|> SwaggerClientGenerator.successResponse
159+
|> shouldEqual SuccessResponse.Missing
160+
161+
[<Test>]
162+
let ``multiple 2xx responses are Ambiguous`` () : unit =
163+
[
164+
ResponseKey.Code 200, Definition.String
165+
ResponseKey.Code 201, Definition.Boolean
166+
ResponseKey.Default, Definition.Handle "#/definitions/Error"
167+
]
168+
|> Map.ofList
169+
|> SwaggerClientGenerator.successResponse
170+
|> shouldEqual SuccessResponse.Ambiguous

‎WoofWare.Myriad.Plugins.Test/TestSwagger/TestSwaggerParse.fs‎

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,9 @@ module TestSwaggerParse =
7676
|> Some
7777
Responses =
7878
[
79-
204, Definition.Unspecified
80-
303, Definition.Unspecified
81-
404, Definition.Unspecified
79+
ResponseKey.Code 204, Definition.Unspecified
80+
ResponseKey.Code 303, Definition.Unspecified
81+
ResponseKey.Code 404, Definition.Unspecified
8282
]
8383
|> Map.ofList
8484
}
@@ -118,12 +118,50 @@ module TestSwaggerParse =
118118
endpoint.Responses
119119
|> shouldEqual (
120120
[
121-
200,
121+
ResponseKey.Code 200,
122122
Definition.Array
123123
{
124124
Items = Definition.String
125125
}
126-
403, Definition.Handle "#/responses/forbidden"
126+
ResponseKey.Code 403, Definition.Handle "#/responses/forbidden"
127+
]
128+
|> Map.ofList
129+
)
130+
131+
[<Test>]
132+
let ``Can parse a default response`` () : unit =
133+
let s =
134+
"""{
135+
"tags": [
136+
"pet"
137+
],
138+
"summary": "Returns all pets from the system that the user has access to",
139+
"operationId": "findPets",
140+
"responses": {
141+
"200": {
142+
"description": "pet response",
143+
"schema": {
144+
"type": "string"
145+
}
146+
},
147+
"default": {
148+
"description": "unexpected error",
149+
"schema": {
150+
"$ref": "#/definitions/Error"
151+
}
152+
}
153+
}
154+
}
155+
"""
156+
|> JsonNode.Parse
157+
158+
let endpoint = s.AsObject () |> SwaggerEndpoint.Parse
159+
160+
endpoint.Responses
161+
|> shouldEqual (
162+
[
163+
ResponseKey.Code 200, Definition.String
164+
ResponseKey.Default, Definition.Handle "#/definitions/Error"
127165
]
128166
|> Map.ofList
129167
)

‎WoofWare.Myriad.Plugins.Test/WoofWare.Myriad.Plugins.Test.fsproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
<Compile Include="TestArgParser\TestArgParser.fs" />
4141
<Compile Include="TestArgParser\TestArgParserNegation.fs" />
4242
<Compile Include="TestSwagger\TestSwaggerParse.fs" />
43+
<Compile Include="TestSwagger\TestSuccessResponse.fs" />
4344
<Compile Include="TestSwagger\TestOpenApi3Parse.fs" />
4445
<EmbeddedResource Include="TestSwagger\api-with-examples.json" />
4546
<EmbeddedResource Include="TestSwagger\callback-example.json" />

‎WoofWare.Myriad.Plugins/SwaggerClientGenerator.fs‎

Lines changed: 44 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,50 @@ type internal Types =
4545
ByDefinition : IReadOnlyDictionary<SwaggerV2.Definition, TypeEntry>
4646
}
4747

48+
/// What an endpoint's "responses" object tells us about the body of a successful response.
49+
[<RequireQualifiedAccess>]
50+
type internal SuccessResponse =
51+
/// Exactly one schema can describe the body of a successful response.
52+
| Exactly of SwaggerV2.Definition
53+
/// Several 2xx statuses are declared, and we don't know which one to return.
54+
| Ambiguous
55+
/// Nothing the endpoint declares could describe the body of a successful response.
56+
| Missing
57+
4858
[<RequireQualifiedAccess>]
4959
module internal SwaggerClientGenerator =
5060

5161
let internal log (_ : string) = ()
5262

63+
/// Determine the schema of the body we expect back from a successful call to an endpoint.
64+
let successResponse (responses : Map<SwaggerV2.ResponseKey, SwaggerV2.Definition>) : SuccessResponse =
65+
let successes =
66+
responses
67+
|> Seq.choose (fun (KeyValue (key, defn)) ->
68+
match key with
69+
| SwaggerV2.ResponseKey.Code code when 200 <= code && code < 300 -> Some defn
70+
| SwaggerV2.ResponseKey.Code _
71+
| SwaggerV2.ResponseKey.Default -> None
72+
)
73+
|> Seq.toList
74+
75+
match successes with
76+
| [ defn ] ->
77+
// Strictly, "default" also describes any 2xx status we haven't declared, so a server
78+
// answering with an undeclared 201 here would be described by "default", not by `defn`.
79+
// We assume the server honours the single 2xx status it declares. The alternative,
80+
// calling this shape ambiguous, would discard the return type of nearly every real
81+
// spec, since {200: T, default: Error} is the idiomatic way to write an endpoint.
82+
SuccessResponse.Exactly defn
83+
| [] ->
84+
// The "default" response describes every status code not explicitly listed,
85+
// which includes any undeclared 2xx status: so with no explicit 2xx response,
86+
// it's the only description of a success body we have.
87+
match Map.tryFind SwaggerV2.ResponseKey.Default responses with
88+
| Some defn -> SuccessResponse.Exactly defn
89+
| None -> SuccessResponse.Missing
90+
| _ :: _ :: _ -> SuccessResponse.Ambiguous
91+
5392
let renderType (types : Types) (defn : SwaggerV2.Definition) : SynType option =
5493
match types.ByDefinition.TryGetValue defn with
5594
| true, v -> Some v.Signature
@@ -653,20 +692,11 @@ module internal SwaggerV2Generator =
653692
failwith $"we don't support multiple Produces right now, at %s{path} (%O{method})"
654693

655694
let returnType =
656-
endpoint.Responses
657-
|> Seq.choose (fun (KeyValue (response, defn)) ->
658-
if 200 <= response && response < 300 then
659-
Some defn
660-
else
661-
None
662-
)
663-
|> Seq.toList
664-
665-
let returnType =
666-
match returnType with
667-
| [ t ] -> Some t
668-
| [] -> failwith $"got no successful response results, %s{path} %O{method}"
669-
| _ ->
695+
match SwaggerClientGenerator.successResponse endpoint.Responses with
696+
| SuccessResponse.Exactly t -> Some t
697+
| SuccessResponse.Missing ->
698+
failwith $"got no successful response results, %s{path} %O{method}"
699+
| SuccessResponse.Ambiguous ->
670700
SwaggerClientGenerator.log
671701
$"Ignoring %s{path} %O{method} due to multiple success responses"
672702
// can't be bothered to work out how to deal with multiple success

‎WoofWare.Myriad.Plugins/SwaggerV2.fs‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,22 @@ type Response =
330330
Schema = schema
331331
}
332332

333+
/// Key into an endpoint's "responses" map: either a specific HTTP status code,
334+
/// or the catch-all "default", which describes all responses whose status codes
335+
/// are not otherwise listed.
336+
type ResponseKey =
337+
/// A specific HTTP status code, e.g. 200.
338+
| Code of int
339+
/// The catch-all "default" response.
340+
| Default
341+
342+
/// Parse a key of the "responses" object, e.g. "200" or "default".
343+
static member Parse (s : string) : ResponseKey =
344+
if s = "default" then
345+
ResponseKey.Default
346+
else
347+
ResponseKey.Code (Int32.Parse s)
348+
333349
/// An "endpoint" is basically a single HTTP verb, applied to some path.
334350
type SwaggerEndpoint =
335351
{
@@ -350,9 +366,9 @@ type SwaggerEndpoint =
350366
/// (Each parameter knows how it needs to be supplied: e.g. if it's a query parameter or
351367
/// if it's interpolated into the path.)
352368
Parameters : SwaggerParameter list option
353-
/// Map of HTTP response code to the type that we expect to receive in the body if we
354-
/// get that response code back.
355-
Responses : Map<int, Definition>
369+
/// Map of HTTP response code (or the catch-all "default") to the type that we
370+
/// expect to receive in the body if we get that response code back.
371+
Responses : Map<ResponseKey, Definition>
356372
}
357373

358374
/// Render a JsonObject into this strongly-typed specification.
@@ -375,7 +391,7 @@ type SwaggerEndpoint =
375391
| Some _ -> Definition.Parse value
376392
| None -> (Response.Parse value).Schema
377393

378-
Int32.Parse key, defn
394+
ResponseKey.Parse key, defn
379395
)
380396
|> Map.ofSeq
381397

0 commit comments

Comments
 (0)