Conversation
- ApiResponse, PageResponse 클래스 추가 - 기존 ProblemDetail 기반 예외 응답 제거 - 모든 예외 응답을 ApiResponse.fail 형태로 일관화
- ApiResponse 도입으로 응답이 data 필드로 래핑됨에 따라 $.guestId → $.data.guestId 로 수정
|
No description provided. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Walkthrough공통 API 응답 래퍼 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Out-of-scope changes
💡 리뷰 의견잘된 점깔끔하게 진행된 표준화입니다. 팩토리 메서드 패턴으로 응답 생성 로직을 일관되게 제어하는 접근이 좋습니다. 개선 제안1. ErrorCategory와 detail 처리 명확화 fun <T> fail(category: ErrorCategory, status: HttpStatus, detail: String? = null): ApiResponse<T>
2. PageResponse 활용 지점 검토
3. 예외 핸들러 category 매핑 검토 handleBaseException(e: BaseException):
→ ApiResponse.fail(e.errorCategory, e.httpStatus, ...)
4. Swagger 문서 일관성
학습 포인트Generic 팩토리 메서드 패턴을 효과적으로 사용했습니다. 이 패턴은 유연성과 타입 안전성을 동시에 제공하므로, 향후 응답 처리가 더 복잡해지면 이 구조를 확장하는 방식을 생각해보세요. 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/test/kotlin/com/depromeet/team3/guest/controller/GuestControllerTest.kt (1)
34-57:⚠️ Potential issue | 🟡 Minor래퍼 전체를 한 번은 검증해 주세요.
지금은
data.guestId만 확인해서status/detail/code가 깨져도 테스트가 통과합니다. 공통 응답 포맷 통일이 이번 PR의 핵심이니, 최소 한 케이스는 wrapper 필드까지 검증해야 회귀를 더 잘 잡을 수 있습니다.수정 예시
mockMvc.perform(post("/api/v1/guests")) .andExpect(status().isOk) .andExpect(jsonPath("$.data.guestId").value(expectedUuid.toString())) + .andExpect(jsonPath("$.detail").value("요청이 정상적으로 처리되었습니다.")) + .andExpect(jsonPath("$.code").value("COMMON_SUCCESS"))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/kotlin/com/depromeet/team3/guest/controller/GuestControllerTest.kt` around lines 34 - 57, Update the GuestControllerTest to assert the common response wrapper fields in at least one test (e.g., inside the test functions `POST api v1 guests 는 발급된 UUID 를 guestId 필드로 반환한다` or `POST api v1 guests 는 매 요청마다 GuestService 를 호출해 새 UUID 를 받아온다` in class GuestControllerTest): in addition to verifying jsonPath("$.data.guestId"), also assert the wrapper fields like jsonPath("$.status"), jsonPath("$.code") and jsonPath("$.detail") (or their actual names used by your API) to ensure the response format is validated; keep the existing guestId assertions and add these extra jsonPath checks to fail if wrapper fields are incorrect.src/main/kotlin/com/depromeet/team3/guest/controller/GuestController.kt (1)
29-52:⚠️ Potential issue | 🟡 MinorSwagger 문서와 실제 응답 구조가 불일치합니다.
schema = Schema(implementation = GuestResponse::class)로 설정하면 OpenAPI 문서는GuestResponse만 보여주지만, 실제 응답은ApiResponse<GuestResponse>래퍼입니다. 결과적으로status,detail,code필드가 생성된 API 문서에서 빠지게 됩니다.또한 예제의
detail값이"성공"이지만,ApiResponse.ok()의 실제 기본값은"요청이 정상적으로 처리되었습니다."입니다. 클라이언트가 예제를 보고 구현했을 때 실제 응답과 달라 혼동할 수 있습니다.수정 방안:
`@SwaggerApiResponse`( responseCode = "200", description = "게스트 ID 발급 성공", content = [ Content( schema = Schema(implementation = ApiResponse::class), examples = [ ExampleObject( name = "발급 성공", value = """ { "status": 200, "data": { "guestId": "8f1a3c2b-9d44-4e2a-9b12-1a2b3c4d5e6f" }, "detail": "요청이 정상적으로 처리되었습니다.", "code": "COMMON_SUCCESS" } """, ), ], ), ], )
schema를ApiResponse::class로 수정하고, 예제의detail을 실제 기본값으로 맞춰주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/depromeet/team3/guest/controller/GuestController.kt` around lines 29 - 52, The Swagger annotation on GuestController.issueGuestId currently declares Schema(implementation = GuestResponse::class) but the endpoint actually returns ApiResponse<GuestResponse> via ApiResponse.ok(GuestResponse(...)), so update the `@SwaggerApiResponse` to use Schema(implementation = ApiResponse::class) and adjust the example JSON to match ApiResponse.ok()'s real default detail ("요청이 정상적으로 처리되었습니다.") and shape (including status, detail, code, and data with guestId) so docs match the runtime response.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/kotlin/com/depromeet/team3/common/exception/GlobalExceptionHandler.kt`:
- Around line 25-30: In GlobalExceptionHandler.handleIllegalArgument, include
the exception message (e.message) in the 400 response body so clients get the
validation detail; update the ApiResponse produced in handleIllegalArgument
(currently ApiResponse.fail(ErrorCategory.INVALID_INPUT,
HttpStatus.BAD_REQUEST)) to pass the exception detail (e.message) into the
ApiResponse payload (use the existing overload or add a detail field) so
ErrorCategory.INVALID_INPUT and the HTTP 400 status are preserved while the
response.detail contains e.message.
In `@src/main/kotlin/com/depromeet/team3/common/response/ApiResponse.kt`:
- Around line 17-43: The ApiResponse creators currently bind the response "code"
to HTTP status names, preventing domain-specific codes like
WISH_CREATED/INVALID_INPUT; update the factory methods (created(), fail(), and
ok() as needed) to accept an explicit code:String parameter (or provide separate
successCode:String / errorCode:String parameters) and set ApiResponse.code to
that value instead of using status.name; ensure created() defaults remain (e.g.,
default code "CREATED" or require caller-specified code) and adjust fail() to
accept either an error code parameter or derive it from ErrorCategory so
controllers can pass WISH_DUPLICATED/INVALID_INPUT.
---
Outside diff comments:
In `@src/main/kotlin/com/depromeet/team3/guest/controller/GuestController.kt`:
- Around line 29-52: The Swagger annotation on GuestController.issueGuestId
currently declares Schema(implementation = GuestResponse::class) but the
endpoint actually returns ApiResponse<GuestResponse> via
ApiResponse.ok(GuestResponse(...)), so update the `@SwaggerApiResponse` to use
Schema(implementation = ApiResponse::class) and adjust the example JSON to match
ApiResponse.ok()'s real default detail ("요청이 정상적으로 처리되었습니다.") and shape
(including status, detail, code, and data with guestId) so docs match the
runtime response.
In `@src/test/kotlin/com/depromeet/team3/guest/controller/GuestControllerTest.kt`:
- Around line 34-57: Update the GuestControllerTest to assert the common
response wrapper fields in at least one test (e.g., inside the test functions
`POST api v1 guests 는 발급된 UUID 를 guestId 필드로 반환한다` or `POST api v1 guests 는 매
요청마다 GuestService 를 호출해 새 UUID 를 받아온다` in class GuestControllerTest): in
addition to verifying jsonPath("$.data.guestId"), also assert the wrapper fields
like jsonPath("$.status"), jsonPath("$.code") and jsonPath("$.detail") (or their
actual names used by your API) to ensure the response format is validated; keep
the existing guestId assertions and add these extra jsonPath checks to fail if
wrapper fields are incorrect.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 5ba88699-aad0-4a58-a28c-f8d6e806aa7d
📒 Files selected for processing (6)
src/main/kotlin/com/depromeet/team3/common/exception/GlobalExceptionHandler.ktsrc/main/kotlin/com/depromeet/team3/common/response/ApiResponse.ktsrc/main/kotlin/com/depromeet/team3/common/response/PageResponse.ktsrc/main/kotlin/com/depromeet/team3/guest/controller/GuestController.ktsrc/main/kotlin/com/depromeet/team3/wishlist/controller/WishlistController.ktsrc/test/kotlin/com/depromeet/team3/guest/controller/GuestControllerTest.kt
| @ExceptionHandler(IllegalArgumentException::class) | ||
| fun handleIllegalArgument(e: IllegalArgumentException): ProblemDetail { | ||
| fun handleIllegalArgument(e: IllegalArgumentException): ResponseEntity<ApiResponse<Nothing>> { | ||
| log.warn("[IllegalArgumentException] {}", e.message) | ||
| return ProblemDetail.forStatusAndDetail(HttpStatus.BAD_REQUEST, e.message ?: "잘못된 요청입니다.").apply { | ||
| setProperty("category", ErrorCategory.INVALID_INPUT) | ||
| } | ||
| return ResponseEntity | ||
| .status(HttpStatus.BAD_REQUEST) | ||
| .body(ApiResponse.fail(ErrorCategory.INVALID_INPUT, HttpStatus.BAD_REQUEST)) |
There was a problem hiding this comment.
400 응답에 예외 메시지를 전달하세요.
지금은 IllegalArgumentException의 실제 사유를 버리고 항상 같은 입력 오류만 내려서, 어떤 필드/값이 잘못됐는지 클라이언트가 알 수 없습니다. 공통 포맷을 쓰더라도 detail에는 e.message를 실어야 디버깅과 UI 메시지 매핑이 가능합니다.
수정 예시
- .body(ApiResponse.fail(ErrorCategory.INVALID_INPUT, HttpStatus.BAD_REQUEST))
+ .body(ApiResponse.fail(ErrorCategory.INVALID_INPUT, HttpStatus.BAD_REQUEST, e.message))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/kotlin/com/depromeet/team3/common/exception/GlobalExceptionHandler.kt`
around lines 25 - 30, In GlobalExceptionHandler.handleIllegalArgument, include
the exception message (e.message) in the 400 response body so clients get the
validation detail; update the ApiResponse produced in handleIllegalArgument
(currently ApiResponse.fail(ErrorCategory.INVALID_INPUT,
HttpStatus.BAD_REQUEST)) to pass the exception detail (e.message) into the
ApiResponse payload (use the existing overload or add a detail field) so
ErrorCategory.INVALID_INPUT and the HTTP 400 status are preserved while the
response.detail contains e.message.
| fun <T> ok(data: T? = null): ApiResponse<T> = ApiResponse( | ||
| status = HttpStatus.OK.value(), | ||
| data = data, | ||
| detail = "요청이 정상적으로 처리되었습니다.", | ||
| code = "COMMON_SUCCESS", | ||
| ) | ||
|
|
||
| fun <T> created( | ||
| data: T, | ||
| detail: String?, | ||
| ): ApiResponse<T> = ApiResponse( | ||
| status = HttpStatus.CREATED.value(), | ||
| data = data, | ||
| detail = detail ?: "생성되었습니다.", | ||
| code = "CREATED", | ||
| ) | ||
|
|
||
| fun <T> fail( | ||
| category: ErrorCategory, | ||
| status: HttpStatus, | ||
| detail: String? = null, | ||
| ): ApiResponse<T> = ApiResponse( | ||
| status = status.value(), | ||
| data = null, | ||
| detail = detail ?: category.description, | ||
| code = status.name, | ||
| ) |
There was a problem hiding this comment.
응답 code를 HTTP 상태명에 묶지 마세요.
지금 created()/fail()이 CREATED, BAD_REQUEST, CONFLICT 같은 상태명만 내려서, PR 설명과 컨트롤러 예제에서 기대하는 WISH_CREATED, INVALID_INPUT, WISH_DUPLICATED를 표현할 수 없습니다. 이 구조로는 클라이언트가 code만 보고 성공/실패를 세분화할 수 없으니, code를 매개변수로 받거나 성공/오류용 코드 체계를 분리해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/kotlin/com/depromeet/team3/common/response/ApiResponse.kt` around
lines 17 - 43, The ApiResponse creators currently bind the response "code" to
HTTP status names, preventing domain-specific codes like
WISH_CREATED/INVALID_INPUT; update the factory methods (created(), fail(), and
ok() as needed) to accept an explicit code:String parameter (or provide separate
successCode:String / errorCode:String parameters) and set ApiResponse.code to
that value instead of using status.name; ensure created() defaults remain (e.g.,
default code "CREATED" or require caller-specified code) and adjust fail() to
accept either an error code parameter or derive it from ErrorCategory so
controllers can pass WISH_DUPLICATED/INVALID_INPUT.
|
|
||
| @ExceptionHandler(BaseException::class) | ||
| fun handleBaseException(e: BaseException): ProblemDetail { | ||
| fun handleBaseException(e: BaseException): ResponseEntity<ApiResponse<Nothing>> { |
| fun <T> ok(data: T? = null): ApiResponse<T> = ApiResponse( | ||
| status = HttpStatus.OK.value(), | ||
| data = data, | ||
| detail = "요청이 정상적으로 처리되었습니다.", | ||
| code = "COMMON_SUCCESS", | ||
| ) | ||
|
|
||
| fun <T> created( | ||
| data: T, | ||
| detail: String?, | ||
| ): ApiResponse<T> = ApiResponse( | ||
| status = HttpStatus.CREATED.value(), | ||
| data = data, | ||
| detail = detail ?: "생성되었습니다.", | ||
| code = "CREATED", | ||
| ) |
There was a problem hiding this comment.
common success 랑 created 를 일관적으로 가져가면 좋겠당!
| import io.swagger.v3.oas.annotations.media.ExampleObject | ||
| import io.swagger.v3.oas.annotations.media.Schema | ||
| import io.swagger.v3.oas.annotations.responses.ApiResponse | ||
| import io.swagger.v3.oas.annotations.responses.ApiResponse as SwaggerApiResponse |
There was a problem hiding this comment.
요거 스웨거의 ApiResponse랑 우리꺼 응답래퍼 ApiResponse랑 이름이 똑같아가지고
스웨거꺼는 SwaggerApiResponse로 쓰도록 수정했어
Situation
Task
ApiResponse<T>단일 포맷으로 통일ApiResponse.fail()형태로 일관화Action
ApiResponse<T>클래스 도입:status,data,detail,code필드 구성,ok()/created()/fail()팩토리 메서드 제공PageResponse클래스 추가 (페이지네이션 응답용)GlobalExceptionHandler를 ProblemDetail 방식에서ApiResponse.fail()방식으로 전환GuestController,WishlistController에 ApiResponse 적용GuestControllerTest의 JSONPath를$.guestId→$.data.guestId로 수정 (ApiResponse 래핑 반영)Result
{ status, data, detail, code }포맷으로 통일되어 클라이언트 파싱 일관성 확보ApiResponse.ok(data)/ApiResponse.created(data, detail)만 사용하면 포맷이 자동으로 맞춰짐연관 이슈
Summary by CodeRabbit
릴리스 노트
Refactor
New Features
Tests