Skip to content

Commit 4f232d7

Browse files
Don't make oneOf errors tagged unions
OpenAPI only lets you have one error per HTTP response code. This is a problem when converting things because Smithy has no such limitation. To address that, we added a toggle that lets you disambiguate by making the body contents a `oneOf`, which is an untagged union. The problem is that we made this a tagged union by leaning on how Smithy converts Smithy's union shape, which is tagged. To illustrate the problem, the OpenAPI modeled body technically now declares the following body valid: ```json { "Error1" { "foo": "bar" } } ``` But the model for the structure is: ``` @error("client") @HttpError(400) structure Error1 { foo: String } ``` And an actual valid body would be: ```json { "foo": "bar" } ``` This commit turns the OpenAPI body into an untagged union instead, which is how it should be.
1 parent 328c7d3 commit 4f232d7

File tree

2 files changed

+13
-85
lines changed

2 files changed

+13
-85
lines changed

smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/protocols/AwsRestJson1Protocol.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import static software.amazon.smithy.openapi.OpenApiConfig.ErrorStatusConflictHandlingStrategy.ONE_OF;
88

9+
import java.util.ArrayList;
910
import java.util.List;
1011
import java.util.Set;
1112
import java.util.TreeSet;
@@ -130,16 +131,15 @@ Schema createDocumentSchema(
130131
if (context.getConfig().getOnErrorStatusConflict() != null
131132
&& context.getConfig().getOnErrorStatusConflict().equals(ONE_OF)
132133
&& targetsSyntheticError(cleanedShape, context)) {
133-
UnionShape.Builder asUnion = UnionShape.builder().id(cleanedShape.getId());
134134
UnionShape targetUnion = context.getModel()
135135
.expectShape(
136136
cleanedShape.getAllMembers().values().stream().findFirst().get().getTarget(),
137137
UnionShape.class);
138+
List<Schema> oneOfMembers = new ArrayList<>();
138139
for (MemberShape member : targetUnion.getAllMembers().values()) {
139-
String name = member.getMemberName();
140-
asUnion.addMember(member.toBuilder().id(cleanedShape.getId().withMember(name)).build());
140+
oneOfMembers.add(context.createRef(member));
141141
}
142-
return context.getJsonSchemaConverter().convertShape(asUnion.build()).getRootSchema();
142+
return Schema.builder().type(null).oneOf(oneOfMembers).build();
143143
}
144144
return context.getJsonSchemaConverter().convertShape(cleanedShape).getRootSchema();
145145
}

smithy-openapi/src/test/resources/software/amazon/smithy/openapi/fromsmithy/protocols/error-code-collision-test-use-oneof.openapi.json

Lines changed: 9 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -154,104 +154,32 @@
154154
"GetCurrentTime404ErrorResponseContent": {
155155
"oneOf": [
156156
{
157-
"type": "object",
158-
"title": "Error7",
159-
"properties": {
160-
"Error7": {
161-
"$ref": "#/components/schemas/Error7"
162-
}
163-
},
164-
"required": [
165-
"Error7"
166-
]
157+
"$ref": "#/components/schemas/Error7"
167158
},
168159
{
169-
"type": "object",
170-
"title": "Error8",
171-
"properties": {
172-
"Error8": {
173-
"$ref": "#/components/schemas/Error8"
174-
}
175-
},
176-
"required": [
177-
"Error8"
178-
]
160+
"$ref": "#/components/schemas/Error8"
179161
}
180162
]
181163
},
182164
"GetCurrentTime429ErrorResponseContent": {
183165
"oneOf": [
184166
{
185-
"type": "object",
186-
"title": "Error1",
187-
"properties": {
188-
"Error1": {
189-
"$ref": "#/components/schemas/Error1"
190-
}
191-
},
192-
"required": [
193-
"Error1"
194-
]
167+
"$ref": "#/components/schemas/Error1"
195168
},
196169
{
197-
"type": "object",
198-
"title": "Error2",
199-
"properties": {
200-
"Error2": {
201-
"$ref": "#/components/schemas/Error2"
202-
}
203-
},
204-
"required": [
205-
"Error2"
206-
]
170+
"$ref": "#/components/schemas/Error2"
207171
},
208172
{
209-
"type": "object",
210-
"title": "Error3",
211-
"properties": {
212-
"Error3": {
213-
"$ref": "#/components/schemas/Error3"
214-
}
215-
},
216-
"required": [
217-
"Error3"
218-
]
173+
"$ref": "#/components/schemas/Error3"
219174
},
220175
{
221-
"type": "object",
222-
"title": "Error4",
223-
"properties": {
224-
"Error4": {
225-
"$ref": "#/components/schemas/Error4"
226-
}
227-
},
228-
"required": [
229-
"Error4"
230-
]
176+
"$ref": "#/components/schemas/Error4"
231177
},
232178
{
233-
"type": "object",
234-
"title": "Error5",
235-
"properties": {
236-
"Error5": {
237-
"$ref": "#/components/schemas/Error5"
238-
}
239-
},
240-
"required": [
241-
"Error5"
242-
]
179+
"$ref": "#/components/schemas/Error5"
243180
},
244181
{
245-
"type": "object",
246-
"title": "Error6",
247-
"properties": {
248-
"Error6": {
249-
"$ref": "#/components/schemas/Error6"
250-
}
251-
},
252-
"required": [
253-
"Error6"
254-
]
182+
"$ref": "#/components/schemas/Error6"
255183
}
256184
]
257185
},
@@ -269,4 +197,4 @@
269197
}
270198
}
271199
}
272-
}
200+
}

0 commit comments

Comments
 (0)