diff --git a/src/main/java/in/koreatech/koin/global/code/ApiResponseCodesOperationCustomizer.java b/src/main/java/in/koreatech/koin/global/code/ApiResponseCodesOperationCustomizer.java index 65bf8ea28d..95e69ba108 100644 --- a/src/main/java/in/koreatech/koin/global/code/ApiResponseCodesOperationCustomizer.java +++ b/src/main/java/in/koreatech/koin/global/code/ApiResponseCodesOperationCustomizer.java @@ -11,7 +11,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.function.Supplier; +import java.util.stream.Collectors; import java.util.stream.Stream; import org.springdoc.core.customizers.OperationCustomizer; @@ -27,6 +27,7 @@ import io.swagger.v3.core.converter.ModelConverters; import io.swagger.v3.core.converter.ResolvedSchema; import io.swagger.v3.oas.models.Operation; +import io.swagger.v3.oas.models.examples.Example; import io.swagger.v3.oas.models.media.Content; import io.swagger.v3.oas.models.media.MediaType; import io.swagger.v3.oas.models.media.Schema; @@ -52,18 +53,36 @@ public Operation customize(Operation operation, HandlerMethod handler) { } ApiResponses responses = operation.getResponses(); + if (responses == null) { + responses = new ApiResponses(); + operation.setResponses(responses); + } + ApiResponses targetResponses = responses; Type returnType = getActualResponseType(handler); - ApiResponseCode[] codes = ann.value(); - for (int i = 0; i < codes.length; i++) { - ApiResponseCode code = codes[i]; - String key = String.format("%d) %d", i + 1, code.getHttpStatus().value()); - responses.put(key, createApiResponse( - code.getMessage(), - () -> createResponseBody(code, handler, returnType) + Map> codesByStatus = Arrays.stream(ann.value()) + .collect(Collectors.groupingBy( + code -> code.getHttpStatus().value(), + LinkedHashMap::new, + Collectors.toList() )); + + ApiResponse springdocSuccessResponse = responses.get("200"); + boolean hasExplicitSuccessResponse = codesByStatus.keySet().stream() + .anyMatch(status -> status >= 200 && status < 300); + if (hasExplicitSuccessResponse && !codesByStatus.containsKey(200)) { + responses.remove("200"); } + codesByStatus.forEach((status, codes) -> { + String statusKey = String.valueOf(status); + ApiResponse existingResponse = targetResponses.get(statusKey); + targetResponses.put( + statusKey, + createApiResponse(codes, handler, returnType, existingResponse, springdocSuccessResponse) + ); + }); + return operation; } @@ -79,29 +98,78 @@ private Type getActualResponseType(HandlerMethod handler) { } private ApiResponse createApiResponse( - String description, - Supplier supplier + List codes, + HandlerMethod handler, + Type returnType, + ApiResponse existingResponse, + ApiResponse springdocSuccessResponse ) { - return new ApiResponse() - .description(description) - .content(new Content().addMediaType(APPLICATION_JSON_VALUE, supplier.get())); + ApiResponse apiResponse = existingResponse == null ? new ApiResponse() : existingResponse; + String description = codes.stream() + .map(code -> code.getCode() + ": " + code.getMessage()) + .collect(Collectors.joining("\n")); + apiResponse.setDescription(mergeDescriptions(apiResponse.getDescription(), description)); + + ApiResponseCode firstCode = codes.get(0); + if (firstCode.getHttpStatus().is2xxSuccessful()) { + mergeSuccessResponse(apiResponse, springdocSuccessResponse, returnType, firstCode); + } else { + mergeErrorResponse(apiResponse, codes, handler); + } + return apiResponse; } - private MediaType createResponseBody( - ApiResponseCode code, - HandlerMethod handler, - Type returnType + private String mergeDescriptions(String existingDescription, String generatedDescription) { + if (existingDescription == null || existingDescription.isBlank()) { + return generatedDescription; + } + if (generatedDescription == null || generatedDescription.isBlank() + || existingDescription.equals(generatedDescription) + || existingDescription.endsWith("\n" + generatedDescription)) { + return existingDescription; + } + return existingDescription + "\n" + generatedDescription; + } + + private void mergeSuccessResponse( + ApiResponse response, + ApiResponse springdocSuccessResponse, + Type returnType, + ApiResponseCode code ) { - if (code.getHttpStatus().is2xxSuccessful()) { - return new MediaType().schema(loadSchema(returnType)); + if (isNoContentResponse(returnType) || code.getHttpStatus().value() == 204) { + response.setContent(null); + return; } - if (code == ApiResponseCode.INVALID_REQUEST_BODY) { - return createErrorMediaType(createInvalidRequestBodyErrorExample(code, handler)); + if (hasSchema(response.getContent())) { + return; + } + if (springdocSuccessResponse != null && hasSchema(springdocSuccessResponse.getContent())) { + response.setContent(springdocSuccessResponse.getContent()); + return; } - return createErrorMediaType(createGenericErrorExample(code)); + response.setContent(new Content().addMediaType( + APPLICATION_JSON_VALUE, + new MediaType().schema(loadSchema(returnType)) + )); + } + + private boolean hasSchema(Content content) { + return content != null && content.values().stream() + .anyMatch(mediaType -> mediaType != null && mediaType.getSchema() != null); + } + + private boolean isNoContentResponse(Type returnType) { + return returnType.equals(Void.class) || returnType.equals(void.class); } - private Map createGenericErrorExample(ApiResponseCode code) { + private Map createErrorExample( + ApiResponseCode code, + HandlerMethod handler + ) { + if (code == ApiResponseCode.INVALID_REQUEST_BODY) { + return createInvalidRequestBodyErrorExample(code, handler); + } return Map.of( "code", code.getCode(), "message", code.getMessage(), @@ -202,9 +270,52 @@ private static Schema loadSchema(Type type) { return resolvedSchema.schema; } - private MediaType createErrorMediaType(Map example) { - MediaType mt = new MediaType().schema(errorSchema); - mt.example(example); - return mt; + private void mergeErrorResponse( + ApiResponse response, + List codes, + HandlerMethod handler + ) { + Content content = response.getContent(); + if (content == null) { + content = new Content(); + response.setContent(content); + } + + MediaType mediaType = content.get(APPLICATION_JSON_VALUE); + if (mediaType == null) { + mediaType = new MediaType(); + content.addMediaType(APPLICATION_JSON_VALUE, mediaType); + } + if (mediaType.getSchema() == null) { + mediaType.setSchema(errorSchema); + } + + Map examples = mediaType.getExamples() == null + ? new LinkedHashMap<>() + : new LinkedHashMap<>(mediaType.getExamples()); + moveSingleExampleToNamedExamples(mediaType, examples); + for (ApiResponseCode code : codes) { + examples.putIfAbsent( + code.getCode(), + new Example() + .summary(code.getCode()) + .description(code.getMessage()) + .value(createErrorExample(code, handler)) + ); + } + mediaType.setExamples(examples); + } + + private void moveSingleExampleToNamedExamples(MediaType mediaType, Map examples) { + if (mediaType.getExample() == null) { + return; + } + String exampleName = "default"; + int suffix = 2; + while (examples.containsKey(exampleName)) { + exampleName = "default-" + suffix++; + } + examples.put(exampleName, new Example().value(mediaType.getExample())); + mediaType.setExample(null); } } diff --git a/src/test/java/in/koreatech/koin/acceptance/domain/CampusOpenApiContractTest.java b/src/test/java/in/koreatech/koin/acceptance/domain/CampusOpenApiContractTest.java new file mode 100644 index 0000000000..2af0945d4a --- /dev/null +++ b/src/test/java/in/koreatech/koin/acceptance/domain/CampusOpenApiContractTest.java @@ -0,0 +1,66 @@ +package in.koreatech.koin.acceptance.domain; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import java.util.Iterator; + +import org.junit.jupiter.api.Test; +import org.springframework.test.web.servlet.MvcResult; + +import com.fasterxml.jackson.databind.JsonNode; + +import in.koreatech.koin.acceptance.AcceptanceTest; +import in.koreatech.koin.acceptance.support.JsonAssertions; + +class CampusOpenApiContractTest extends AcceptanceTest { + + private static final String CAMPUS_GROUP = "3. Campus API"; + + @Test + void root_list_success_schema_is_preserved() throws Exception { + JsonNode openApi = campusOpenApi(); + JsonNode response = openApi.at("/paths/~1callvan~1notifications/get/responses/200"); + + assertThat(response.isMissingNode()).isFalse(); + JsonNode schema = findResponseSchema(response); + assertThat(schema.path("type").asText()).isEqualTo("array"); + assertThat(schema.path("items").path("$ref").asText()).endsWith("CallvanNotificationResponse"); + } + + @Test + void explicit_created_and_no_content_responses_do_not_keep_phantom_200() throws Exception { + JsonNode openApi = campusOpenApi(); + JsonNode created = openApi.at("/paths/~1callvan/post/responses"); + JsonNode noContent = openApi.at( + "/paths/~1callvan~1notifications~1mark-all-read/post/responses"); + + assertThat(created.has("200")).isFalse(); + assertThat(created.has("201")).isTrue(); + assertThat(findResponseSchema(created.path("201")).path("$ref").asText()) + .endsWith("CallvanPostCreateResponse"); + assertThat(noContent.has("200")).isFalse(); + assertThat(noContent.has("204")).isTrue(); + assertThat(noContent.path("204").has("content")).isFalse(); + } + + private JsonNode campusOpenApi() throws Exception { + MvcResult result = mockMvc.perform(get("/v3/api-docs/{group}", CAMPUS_GROUP)) + .andExpect(status().isOk()) + .andReturn(); + return JsonAssertions.convertJsonNode(result); + } + + private JsonNode findResponseSchema(JsonNode response) { + Iterator mediaTypes = response.path("content").elements(); + while (mediaTypes.hasNext()) { + JsonNode schema = mediaTypes.next().path("schema"); + if (!schema.isMissingNode()) { + return schema; + } + } + throw new AssertionError("Response content does not contain a schema"); + } + +} diff --git a/src/test/java/in/koreatech/koin/unit/global/code/ApiResponseCodesOperationCustomizerTest.java b/src/test/java/in/koreatech/koin/unit/global/code/ApiResponseCodesOperationCustomizerTest.java new file mode 100644 index 0000000000..8f71719ace --- /dev/null +++ b/src/test/java/in/koreatech/koin/unit/global/code/ApiResponseCodesOperationCustomizerTest.java @@ -0,0 +1,220 @@ +package in.koreatech.koin.unit.global.code; + +import static in.koreatech.koin.global.code.ApiResponseCode.CREATED; +import static in.koreatech.koin.global.code.ApiResponseCode.ILLEGAL_ARGUMENT; +import static in.koreatech.koin.global.code.ApiResponseCode.INVALID_REQUEST_BODY; +import static in.koreatech.koin.global.code.ApiResponseCode.NO_CONTENT; +import static in.koreatech.koin.global.code.ApiResponseCode.OK; +import static org.assertj.core.api.Assertions.assertThat; + +import java.lang.reflect.Method; +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.Test; +import org.springframework.http.ResponseEntity; +import org.springframework.web.method.HandlerMethod; + +import in.koreatech.koin.domain.teamrecruitment.controller.TeamRecruitmentChatController; +import in.koreatech.koin.global.code.ApiResponseCodes; +import in.koreatech.koin.global.code.ApiResponseCodesOperationCustomizer; +import io.swagger.v3.oas.models.Operation; +import io.swagger.v3.oas.models.examples.Example; +import io.swagger.v3.oas.models.media.ArraySchema; +import io.swagger.v3.oas.models.media.Content; +import io.swagger.v3.oas.models.media.MediaType; +import io.swagger.v3.oas.models.media.Schema; +import io.swagger.v3.oas.models.responses.ApiResponse; +import io.swagger.v3.oas.models.responses.ApiResponses; + +class ApiResponseCodesOperationCustomizerTest { + + private final ApiResponseCodesOperationCustomizer customizer = new ApiResponseCodesOperationCustomizer(); + + @Test + void same_status_codes_are_grouped_and_no_content_has_no_media_type() throws Exception { + Operation operation = new Operation().responses(new ApiResponses()); + Method method = SampleController.class.getDeclaredMethod("update"); + + customizer.customize(operation, new HandlerMethod(new SampleController(), method)); + + assertThat(operation.getResponses()).containsOnlyKeys("204", "400"); + + ApiResponse noContent = operation.getResponses().get("204"); + assertThat(noContent.getContent()).isNull(); + + ApiResponse badRequest = operation.getResponses().get("400"); + MediaType mediaType = badRequest.getContent().get("application/json"); + assertThat(mediaType.getExamples()).containsOnlyKeys("INVALID_REQUEST_BODY", "ILLEGAL_ARGUMENT"); + assertThat(((Map) mediaType.getExamples().get("INVALID_REQUEST_BODY").getValue()).get("code")) + .isEqualTo("INVALID_REQUEST_BODY"); + assertThat(((Map) mediaType.getExamples().get("ILLEGAL_ARGUMENT").getValue()).get("code")) + .isEqualTo("ILLEGAL_ARGUMENT"); + } + + @Test + void void_success_response_has_no_media_type_even_when_status_is_not_204() throws Exception { + Operation operation = new Operation().responses(new ApiResponses()); + Method method = SampleController.class.getDeclaredMethod("acknowledge"); + + customizer.customize(operation, new HandlerMethod(new SampleController(), method)); + + assertThat(operation.getResponses().get("200").getContent()).isNull(); + } + + @Test + void team_recruitment_chat_annotation_uses_standard_status_keys() throws Exception { + Operation operation = new Operation().responses(new ApiResponses()); + Method method = TeamRecruitmentChatController.class.getMethod( + "getOrCreateDirectChatRoom", Integer.class, Integer.class, Integer.class); + + customizer.customize( + operation, + new HandlerMethod(new TeamRecruitmentChatController(null), method) + ); + + assertThat(operation.getResponses()).containsOnlyKeys("200", "201", "401", "403", "404", "409"); + } + + @Test + void existing_root_list_success_schema_and_description_are_preserved() throws Exception { + Content springdocContent = new Content().addMediaType( + "*/*", + new MediaType().schema(new ArraySchema().items( + new Schema<>().$ref("#/components/schemas/SampleResponse"))) + ); + ApiResponse springdocResponse = new ApiResponse() + .description("Springdoc success") + .content(springdocContent); + Operation operation = new Operation().responses( + new ApiResponses().addApiResponse("200", springdocResponse)); + Method method = SampleController.class.getDeclaredMethod("list"); + + customizer.customize(operation, new HandlerMethod(new SampleController(), method)); + + ApiResponse response = operation.getResponses().get("200"); + assertThat(response.getContent()).isSameAs(springdocContent); + assertThat(response.getDescription()).contains("Springdoc success", "OK:"); + Schema schema = response.getContent().get("*/*").getSchema(); + assertThat(schema.getType()).isEqualTo("array"); + assertThat(schema.getItems().get$ref()).isEqualTo("#/components/schemas/SampleResponse"); + } + + @Test + void phantom_200_is_removed_and_generated_schema_is_moved_to_created_response() throws Exception { + Content springdocContent = new Content().addMediaType( + "*/*", + new MediaType().schema(new Schema<>().$ref("#/components/schemas/SampleResponse")) + ); + Operation operation = new Operation().responses(new ApiResponses().addApiResponse( + "200", + new ApiResponse().description("Springdoc success").content(springdocContent) + )); + Method method = SampleController.class.getDeclaredMethod("create"); + + customizer.customize(operation, new HandlerMethod(new SampleController(), method)); + + assertThat(operation.getResponses()).containsOnlyKeys("201"); + assertThat(operation.getResponses().get("201").getContent()).isSameAs(springdocContent); + } + + @Test + void existing_created_schema_and_description_take_precedence_over_phantom_200() throws Exception { + Content springdocContent = new Content().addMediaType( + "*/*", + new MediaType().schema(new Schema<>().$ref("#/components/schemas/SampleResponse")) + ); + Content documentedCreatedContent = new Content().addMediaType( + "application/json", + new MediaType().schema(new Schema<>().$ref("#/components/schemas/DocumentedCreatedResponse")) + ); + Operation operation = new Operation().responses(new ApiResponses() + .addApiResponse("200", new ApiResponse().description("Springdoc success").content(springdocContent)) + .addApiResponse("201", new ApiResponse() + .description("Manually documented creation") + .content(documentedCreatedContent))); + Method method = SampleController.class.getDeclaredMethod("create"); + + customizer.customize(operation, new HandlerMethod(new SampleController(), method)); + + assertThat(operation.getResponses()).containsOnlyKeys("201"); + ApiResponse created = operation.getResponses().get("201"); + assertThat(created.getContent()).isSameAs(documentedCreatedContent); + assertThat(created.getDescription()).contains("Manually documented creation", "CREATED:"); + } + + @Test + void phantom_200_is_removed_from_no_content_response() throws Exception { + Operation operation = new Operation().responses(new ApiResponses().addApiResponse( + "200", + new ApiResponse().description("Springdoc success") + )); + Method method = SampleController.class.getDeclaredMethod("remove"); + + customizer.customize(operation, new HandlerMethod(new SampleController(), method)); + + assertThat(operation.getResponses()).containsOnlyKeys("204"); + assertThat(operation.getResponses().get("204").getContent()).isNull(); + } + + @Test + void existing_error_examples_are_preserved_when_domain_examples_are_merged() throws Exception { + Example existing = new Example().summary("manually documented example"); + Map singleExample = Map.of("source", "manual"); + Schema existingSchema = new Schema<>().type("object"); + MediaType existingMediaType = new MediaType() + .schema(existingSchema) + .example(singleExample) + .addExamples("ILLEGAL_ARGUMENT", existing); + Operation operation = new Operation().responses(new ApiResponses().addApiResponse( + "400", + new ApiResponse() + .description("Manually documented bad request") + .content(new Content().addMediaType("application/json", existingMediaType)) + )); + Method method = SampleController.class.getDeclaredMethod("update"); + + customizer.customize(operation, new HandlerMethod(new SampleController(), method)); + + ApiResponse response = operation.getResponses().get("400"); + assertThat(response.getDescription()).contains("Manually documented bad request", "INVALID_REQUEST_BODY:"); + assertThat(response.getContent().get("application/json").getSchema()).isSameAs(existingSchema); + assertThat(response.getContent().get("application/json").getExamples()) + .containsEntry("ILLEGAL_ARGUMENT", existing) + .containsKey("INVALID_REQUEST_BODY"); + assertThat(response.getContent().get("application/json").getExample()).isNull(); + assertThat(response.getContent().get("application/json").getExamples().values()) + .extracting(Example::getValue) + .contains(singleExample); + } + + private static class SampleController { + + @ApiResponseCodes({NO_CONTENT, INVALID_REQUEST_BODY, ILLEGAL_ARGUMENT}) + ResponseEntity update() { + return ResponseEntity.noContent().build(); + } + + @ApiResponseCodes(OK) + ResponseEntity acknowledge() { + return ResponseEntity.ok().build(); + } + + @ApiResponseCodes(OK) + ResponseEntity> list() { + return ResponseEntity.ok(List.of()); + } + + @ApiResponseCodes(CREATED) + ResponseEntity create() { + return ResponseEntity.status(201).body(new SampleResponse("created")); + } + + @ApiResponseCodes(NO_CONTENT) + ResponseEntity remove() { + return ResponseEntity.noContent().build(); + } + } + + private record SampleResponse(String value) {} +}