Skip to content

Commit 19bda78

Browse files
authored
Improve error response names (#29)
Currently we declare the same 403 response many times over for many endpoints. Just with a unique name for the endpoint. Same for other http status codes. We did that because for other parts of the code generator, generating unique component refs forced the generator not to be "smart" and re-use things across different endpoints. However, this is not the case with the error responses. So DeleteVirtualMachine403Response is actually being used for ~43 different api calls. Not obviously all related to deleting a virtual machine. Now we name the error response after the errors it returns (excluding the api authenticator ones applied to all responses). In addition, we don't declare the `oneOf` inside the response, that has been moved to another named schema definition and then referenced from the response. This forces the generator to use the named schema and not try to be "clever". closes: #28
1 parent d4ecf6c commit 19bda78

File tree

2 files changed

+38
-39
lines changed

2 files changed

+38
-39
lines changed

lib/apia/open_api/objects/response.rb

+13-3
Original file line numberDiff line numberDiff line change
@@ -211,13 +211,18 @@ def generate_ref(namespace, http_status_code, definitions)
211211
end
212212

213213
def generate_id_for_error_ref(http_status_code, definitions)
214-
if definitions == api_authenticator_potential_errors.map(&:definition)
214+
api_authenticator_error_defs = api_authenticator_potential_errors.map(&:definition).select do |d|
215+
d.http_status_code.to_s == http_status_code.to_s
216+
end
217+
if api_authenticator_error_defs.any? && api_authenticator_error_defs == definitions
215218
"APIAuthenticator#{http_status_code}Response"
216219
elsif definitions.length == 1
217220
"#{generate_id_from_definition(definitions.first)}Response"
218221
else
219222
[
220-
@route_spec[:operationId].sub(":", "_").gsub(":", "").split("/"),
223+
(definitions - api_authenticator_error_defs).map do |d|
224+
generate_id_from_definition(d)
225+
end.join,
221226
http_status_code,
222227
"Response"
223228
].flatten.join("_").camelize
@@ -236,7 +241,12 @@ def add_to_responses_components(http_status_code, definitions, id)
236241
component_schema[:description] = definition.description if definition.description.present?
237242
schema = generate_schema_properties_for_definition(definition)
238243
else # the same http status code is used for multiple errors
239-
schema = { oneOf: definitions.map { |d| generate_ref("schemas", http_status_code, [d]) } }
244+
one_of_id = "OneOf#{id}"
245+
@spec[:components][:schemas][one_of_id] = {
246+
oneOf: definitions.map { |d| generate_ref("schemas", http_status_code, [d]) }
247+
}
248+
249+
schema = { "$ref": "#/components/schemas/#{one_of_id}" }
240250
end
241251

242252
component_schema[:content] = {

spec/support/fixtures/openapi.json

+25-36
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,7 @@
434434
}
435435
},
436436
"400": {
437-
"$ref": "#/components/responses/GetTestObject400Response"
437+
"$ref": "#/components/responses/InvalidTestAnotherInvalidTest400Response"
438438
},
439439
"403": {
440440
"$ref": "#/components/responses/APIAuthenticator403Response"
@@ -492,7 +492,7 @@
492492
}
493493
},
494494
"400": {
495-
"$ref": "#/components/responses/PostTestObject400Response"
495+
"$ref": "#/components/responses/InvalidTestAnotherInvalidTest400Response"
496496
},
497497
"403": {
498498
"$ref": "#/components/responses/APIAuthenticator403Response"
@@ -684,6 +684,16 @@
684684
}
685685
}
686686
},
687+
"OneOfAPIAuthenticator403Response": {
688+
"oneOf": [
689+
{
690+
"$ref": "#/components/schemas/InvalidTokenResponse"
691+
},
692+
{
693+
"$ref": "#/components/schemas/UnauthorizedNetworkForAPITokenResponse"
694+
}
695+
]
696+
},
687697
"RateLimitReached": {
688698
"type": "object",
689699
"properties": {
@@ -922,6 +932,16 @@
922932
}
923933
}
924934
},
935+
"OneOfInvalidTestAnotherInvalidTest400Response": {
936+
"oneOf": [
937+
{
938+
"$ref": "#/components/schemas/InvalidTestResponse"
939+
},
940+
{
941+
"$ref": "#/components/schemas/AnotherInvalidTestResponse"
942+
}
943+
]
944+
},
925945
"ObjectLookup": {
926946
"description": "All 'object[]' params are mutually exclusive, only one can be provided.",
927947
"type": "object",
@@ -986,14 +1006,7 @@
9861006
"content": {
9871007
"application/json": {
9881008
"schema": {
989-
"oneOf": [
990-
{
991-
"$ref": "#/components/schemas/InvalidTokenResponse"
992-
},
993-
{
994-
"$ref": "#/components/schemas/UnauthorizedNetworkForAPITokenResponse"
995-
}
996-
]
1009+
"$ref": "#/components/schemas/OneOfAPIAuthenticator403Response"
9971010
}
9981011
}
9991012
}
@@ -1044,19 +1057,12 @@
10441057
}
10451058
}
10461059
},
1047-
"GetTestObject400Response": {
1060+
"InvalidTestAnotherInvalidTest400Response": {
10481061
"description": "400 error response",
10491062
"content": {
10501063
"application/json": {
10511064
"schema": {
1052-
"oneOf": [
1053-
{
1054-
"$ref": "#/components/schemas/InvalidTestResponse"
1055-
},
1056-
{
1057-
"$ref": "#/components/schemas/AnotherInvalidTestResponse"
1058-
}
1059-
]
1065+
"$ref": "#/components/schemas/OneOfInvalidTestAnotherInvalidTest400Response"
10601066
}
10611067
}
10621068
}
@@ -1083,23 +1089,6 @@
10831089
}
10841090
}
10851091
}
1086-
},
1087-
"PostTestObject400Response": {
1088-
"description": "400 error response",
1089-
"content": {
1090-
"application/json": {
1091-
"schema": {
1092-
"oneOf": [
1093-
{
1094-
"$ref": "#/components/schemas/InvalidTestResponse"
1095-
},
1096-
{
1097-
"$ref": "#/components/schemas/AnotherInvalidTestResponse"
1098-
}
1099-
]
1100-
}
1101-
}
1102-
}
11031092
}
11041093
},
11051094
"securitySchemes": {

0 commit comments

Comments
 (0)