Skip to content

Commit ab0daf3

Browse files
author
bnasslahsen
committed
Wrong ApiResponse Schema picked up in ExceptionHandlers returning void. Fixes #711
1 parent d5260f6 commit ab0daf3

File tree

15 files changed

+1261
-1064
lines changed

15 files changed

+1261
-1064
lines changed

springdoc-openapi-common/src/main/java/org/springdoc/api/AbstractOpenApiResource.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,8 @@ protected synchronized OpenAPI getOpenApi() {
170170
Map<String, Object> findControllerAdvice = openAPIBuilder.getControllerAdviceMap();
171171
// calculate generic responses
172172
openApi = openAPIBuilder.getCalculatedOpenAPI();
173-
responseBuilder.buildGenericResponse(openApi.getComponents(), findControllerAdvice);
173+
if (springDocConfigProperties.isOverrideWithGenericResponse())
174+
responseBuilder.buildGenericResponse(openApi.getComponents(), findControllerAdvice);
174175
getPaths(mappingsMap);
175176
// run the optional customisers
176177
openApiCustomisers.ifPresent(apiCustomisers -> apiCustomisers.forEach(openApiCustomiser -> openApiCustomiser.customise(openApi)));

springdoc-openapi-common/src/main/java/org/springdoc/core/GenericResponseBuilder.java

Lines changed: 48 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,8 @@ public ApiResponses build(Components components, HandlerMethod handlerMethod, Op
9494
apiResponsesFromDoc.forEach(apiResponses::addApiResponse);
9595
// for each one build ApiResponse and add it to existing responses
9696
// Fill api Responses
97-
computeResponse(components, handlerMethod.getReturnType(), apiResponses, methodAttributes, false);
97+
computeResponseFromDoc(components, handlerMethod.getReturnType(), apiResponses, methodAttributes);
98+
buildApiResponses(components, handlerMethod.getReturnType(), apiResponses, methodAttributes);
9899
return apiResponses;
99100
}
100101

@@ -117,26 +118,27 @@ public void buildGenericResponse(Components components, Map<String, Object> find
117118
if (reqMappringMethod != null)
118119
methodProduces = reqMappringMethod.produces();
119120
Map<String, ApiResponse> controllerAdviceInfoApiResponseMap = controllerAdviceInfo.getApiResponseMap();
120-
Map<String, ApiResponse> apiResponses = computeResponse(components, new MethodParameter(method, -1), new ApiResponses(),
121-
new MethodAttributes(methodProduces, springDocConfigProperties.getDefaultConsumesMediaType(),
122-
springDocConfigProperties.getDefaultProducesMediaType(), controllerAdviceInfoApiResponseMap), true);
121+
MethodParameter methodParameter = new MethodParameter(method, -1);
122+
ApiResponses apiResponsesOp = new ApiResponses();
123+
MethodAttributes methodAttributes = new MethodAttributes(methodProduces, springDocConfigProperties.getDefaultConsumesMediaType(),
124+
springDocConfigProperties.getDefaultProducesMediaType(), controllerAdviceInfoApiResponseMap);
125+
Map<String, ApiResponse> apiResponses = computeResponseFromDoc(components, methodParameter, apiResponsesOp, methodAttributes);
126+
buildGenericApiResponses(components, methodParameter, apiResponsesOp, methodAttributes);
123127
apiResponses.forEach(controllerAdviceInfoApiResponseMap::put);
124128
}
125129
}
126130
controllerAdviceInfos.add(controllerAdviceInfo);
127131
}
128132
}
129133

130-
private Map<String, ApiResponse> computeResponse(Components components, MethodParameter methodParameter, ApiResponses apiResponsesOp,
131-
MethodAttributes methodAttributes, boolean isGeneric) {
134+
private Map<String, ApiResponse> computeResponseFromDoc(Components components, MethodParameter methodParameter, ApiResponses apiResponsesOp,
135+
MethodAttributes methodAttributes) {
132136
// Parsing documentation, if present
133137
Set<io.swagger.v3.oas.annotations.responses.ApiResponse> responsesArray = getApiResponses(methodParameter.getMethod());
134138
if (!responsesArray.isEmpty()) {
135139
methodAttributes.setWithApiResponseDoc(true);
136-
if (!springDocConfigProperties.isOverrideWithGenericResponse())
137-
for (String key : methodAttributes.getGenericMapResponse().keySet())
138-
apiResponsesOp.remove(key);
139140
for (io.swagger.v3.oas.annotations.responses.ApiResponse apiResponseAnnotations : responsesArray) {
141+
String httpCode = apiResponseAnnotations.responseCode();
140142
ApiResponse apiResponse = new ApiResponse();
141143
if (StringUtils.isNotBlank(apiResponseAnnotations.ref())) {
142144
apiResponse.$ref(apiResponseAnnotations.ref());
@@ -150,10 +152,9 @@ private Map<String, ApiResponse> computeResponse(Components components, MethodPa
150152
apiResponse.extensions(extensions);
151153
AnnotationsUtils.getHeaders(apiResponseAnnotations.headers(), methodAttributes.getJsonViewAnnotation())
152154
.ifPresent(apiResponse::headers);
153-
apiResponsesOp.addApiResponse(apiResponseAnnotations.responseCode(), apiResponse);
155+
apiResponsesOp.addApiResponse(httpCode, apiResponse);
154156
}
155157
}
156-
buildApiResponses(components, methodParameter, apiResponsesOp, methodAttributes, isGeneric);
157158
return apiResponsesOp;
158159
}
159160

@@ -180,32 +181,56 @@ public static void buildContentFromDoc(Components components, ApiResponses apiRe
180181
else
181182
apiResponse.content(newContent);
182183
}
184+
else {
185+
apiResponse.content(existingContent);
186+
}
183187
}
184188
else {
185189
optionalContent.ifPresent(apiResponse::content);
186190
}
187191
}
188192

189-
private void buildApiResponses(Components components, MethodParameter methodParameter, ApiResponses apiResponsesOp,
190-
MethodAttributes methodAttributes, boolean isGeneric) {
191-
if (!CollectionUtils.isEmpty(apiResponsesOp) && (apiResponsesOp.size() != methodAttributes.getGenericMapResponse().size() || isGeneric)) {
193+
private void buildGenericApiResponses(Components components, MethodParameter methodParameter, ApiResponses apiResponsesOp,
194+
MethodAttributes methodAttributes) {
195+
if (!CollectionUtils.isEmpty(apiResponsesOp)) {
192196
// API Responses at operation and @ApiResponse annotation
193197
for (Map.Entry<String, ApiResponse> entry : apiResponsesOp.entrySet()) {
194198
String httpCode = entry.getKey();
195199
ApiResponse apiResponse = entry.getValue();
196-
buildApiResponses(components, methodParameter, apiResponsesOp, methodAttributes, httpCode, apiResponse,
197-
isGeneric);
200+
buildApiResponses(components, methodParameter, apiResponsesOp, methodAttributes, httpCode, apiResponse, true);
198201
}
199202
}
200203
else {
201204
// Use response parameters with no description filled - No documentation
202205
// available
203-
String httpCode = evaluateResponseStatus(methodParameter.getMethod(), methodParameter.getMethod().getClass(), isGeneric);
206+
String httpCode = evaluateResponseStatus(methodParameter.getMethod(), methodParameter.getMethod().getClass(), true);
204207
ApiResponse apiResponse = methodAttributes.getGenericMapResponse().containsKey(httpCode) ? methodAttributes.getGenericMapResponse().get(httpCode)
205208
: new ApiResponse();
206209
if (httpCode != null)
207-
buildApiResponses(components, methodParameter, apiResponsesOp, methodAttributes, httpCode, apiResponse,
208-
isGeneric);
210+
buildApiResponses(components, methodParameter, apiResponsesOp, methodAttributes, httpCode, apiResponse, true);
211+
}
212+
}
213+
214+
private void buildApiResponses(Components components, MethodParameter methodParameter, ApiResponses apiResponsesOp,
215+
MethodAttributes methodAttributes) {
216+
Map<String, ApiResponse> genericMapResponse = methodAttributes.getGenericMapResponse();
217+
if (!CollectionUtils.isEmpty(apiResponsesOp) && apiResponsesOp.size() > genericMapResponse.size()) {
218+
// API Responses at operation and @ApiResponse annotation
219+
for (Map.Entry<String, ApiResponse> entry : apiResponsesOp.entrySet()) {
220+
String httpCode = entry.getKey();
221+
if (!genericMapResponse.containsKey(httpCode)) {
222+
ApiResponse apiResponse = entry.getValue();
223+
buildApiResponses(components, methodParameter, apiResponsesOp, methodAttributes, httpCode, apiResponse, false);
224+
}
225+
}
226+
}
227+
else {
228+
// Use response parameters with no description filled - No documentation
229+
// available
230+
String httpCode = evaluateResponseStatus(methodParameter.getMethod(), methodParameter.getMethod().getClass(), false);
231+
ApiResponse apiResponse = new ApiResponse();
232+
if (httpCode != null)
233+
buildApiResponses(components, methodParameter, apiResponsesOp, methodAttributes, httpCode, apiResponse, false);
209234
}
210235
}
211236

@@ -269,7 +294,7 @@ private Type getReturnType(MethodParameter methodParameter) {
269294
return returnType;
270295
}
271296

272-
private Schema calculateSchema(Components components, Type returnType, JsonView jsonView, Annotation[] annotations) {
297+
private Schema<?> calculateSchema(Components components, Type returnType, JsonView jsonView, Annotation[] annotations) {
273298
return !isVoid(returnType) ? extractSchema(components, returnType, jsonView,annotations) : null;
274299
}
275300

@@ -297,7 +322,8 @@ else if (CollectionUtils.isEmpty(apiResponse.getContent()))
297322
&& ((isGeneric || methodAttributes.isMethodOverloaded()) && methodAttributes.isNoApiResponseDoc())) {
298323
// Merge with existing schema
299324
Content existingContent = apiResponse.getContent();
300-
Schema<?> schemaN = calculateSchema(components, methodParameter.getGenericParameterType(),
325+
Type type = ReturnTypeParser.getType(methodParameter);
326+
Schema<?> schemaN = calculateSchema(components, type,
301327
methodAttributes.getJsonViewAnnotation(), methodParameter.getParameterAnnotations());
302328
if (schemaN != null && ArrayUtils.isNotEmpty(methodAttributes.getMethodProduces()))
303329
Arrays.stream(methodAttributes.getMethodProduces()).forEach(mediaTypeStr -> mergeSchema(existingContent, schemaN, mediaTypeStr));
@@ -307,7 +333,7 @@ else if (CollectionUtils.isEmpty(apiResponse.getContent()))
307333

308334
public static void setDescription(String httpCode, ApiResponse apiResponse) {
309335
try {
310-
HttpStatus httpStatus = HttpStatus.valueOf(Integer.valueOf(httpCode));
336+
HttpStatus httpStatus = HttpStatus.valueOf(Integer.parseInt(httpCode));
311337
apiResponse.setDescription(httpStatus.getReasonPhrase());
312338
}
313339
catch (IllegalArgumentException e) {

springdoc-openapi-webflux-core/src/test/resources/results/app3.json

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@
128128
"content": {
129129
"*/*": {
130130
"schema": {
131-
"$ref": "#/components/schemas/TweetDTO"
131+
"type": "string"
132132
}
133133
}
134134
}
@@ -186,7 +186,7 @@
186186
"content": {
187187
"*/*": {
188188
"schema": {
189-
"$ref": "#/components/schemas/TweetDTO"
189+
"type": "string"
190190
}
191191
}
192192
}
@@ -230,7 +230,14 @@
230230
],
231231
"responses": {
232232
"404": {
233-
"description": "tweet not found"
233+
"description": "tweet not found",
234+
"content": {
235+
"*/*": {
236+
"schema": {
237+
"type": "string"
238+
}
239+
}
240+
}
234241
},
235242
"409": {
236243
"description": "Conflict",
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*
2+
*
3+
* * Copyright 2019-2020 the original author or authors.
4+
* *
5+
* * Licensed under the Apache License, Version 2.0 (the "License");
6+
* * you may not use this file except in compliance with the License.
7+
* * You may obtain a copy of the License at
8+
* *
9+
* * https://www.apache.org/licenses/LICENSE-2.0
10+
* *
11+
* * Unless required by applicable law or agreed to in writing, software
12+
* * distributed under the License is distributed on an "AS IS" BASIS,
13+
* * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* * See the License for the specific language governing permissions and
15+
* * limitations under the License.
16+
*
17+
*/
18+
19+
package test.org.springdoc.api.app123;
20+
21+
import io.swagger.v3.oas.annotations.Operation;
22+
import io.swagger.v3.oas.annotations.responses.ApiResponse;
23+
24+
import org.springframework.web.bind.annotation.GetMapping;
25+
import org.springframework.web.bind.annotation.PathVariable;
26+
import org.springframework.web.bind.annotation.RestController;
27+
28+
@RestController
29+
public class HelloController<T> {
30+
31+
@GetMapping(value = "/hello/{numTelco}")
32+
@Operation(summary = "GET Persons", responses = @ApiResponse(responseCode = "418"))
33+
public T index(@PathVariable("numTelco") String numTel, String adresse) {
34+
return null;
35+
}
36+
}
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/*
2+
*
3+
* * Copyright 2019-2020 the original author or authors.
4+
* *
5+
* * Licensed under the Apache License, Version 2.0 (the "License");
6+
* * you may not use this file except in compliance with the License.
7+
* * You may obtain a copy of the License at
8+
* *
9+
* * https://www.apache.org/licenses/LICENSE-2.0
10+
* *
11+
* * Unless required by applicable law or agreed to in writing, software
12+
* * distributed under the License is distributed on an "AS IS" BASIS,
13+
* * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* * See the License for the specific language governing permissions and
15+
* * limitations under the License.
16+
*
17+
*/
18+
19+
package test.org.springdoc.api.app123;
20+
21+
import io.swagger.v3.oas.annotations.media.Content;
22+
import io.swagger.v3.oas.annotations.responses.ApiResponse;
23+
24+
import org.springframework.http.HttpStatus;
25+
import org.springframework.web.bind.annotation.ExceptionHandler;
26+
import org.springframework.web.bind.annotation.ResponseStatus;
27+
import org.springframework.web.bind.annotation.RestControllerAdvice;
28+
29+
@RestControllerAdvice
30+
public class MyExceptionHandler {
31+
32+
@ExceptionHandler(IllegalArgumentException.class)
33+
@ApiResponse(responseCode = "404", description = "Not here", content = @Content)
34+
@ResponseStatus(HttpStatus.NOT_FOUND)
35+
public void bad(IllegalArgumentException e) {
36+
37+
}
38+
39+
@ExceptionHandler(RuntimeException.class)
40+
@ResponseStatus(HttpStatus.BAD_GATEWAY)
41+
public Object gateway(RuntimeException e) {
42+
return null;
43+
}
44+
}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/*
2+
*
3+
* *
4+
* * *
5+
* * * * Copyright 2019-2020 the original author or authors.
6+
* * * *
7+
* * * * Licensed under the Apache License, Version 2.0 (the "License");
8+
* * * * you may not use this file except in compliance with the License.
9+
* * * * You may obtain a copy of the License at
10+
* * * *
11+
* * * * https://www.apache.org/licenses/LICENSE-2.0
12+
* * * *
13+
* * * * Unless required by applicable law or agreed to in writing, software
14+
* * * * distributed under the License is distributed on an "AS IS" BASIS,
15+
* * * * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
* * * * See the License for the specific language governing permissions and
17+
* * * * limitations under the License.
18+
* * *
19+
* *
20+
*
21+
*
22+
*/
23+
package test.org.springdoc.api.app123;
24+
25+
import test.org.springdoc.api.AbstractSpringDocTest;
26+
27+
import org.springframework.boot.autoconfigure.SpringBootApplication;
28+
29+
30+
/**
31+
* Tests Spring meta-annotations as method parameters
32+
*/
33+
public class SpringDocApp123Test extends AbstractSpringDocTest {
34+
35+
@SpringBootApplication
36+
static class SpringDocTestApp {}
37+
}

0 commit comments

Comments
 (0)