Conversation
|
Caution Review failedThe pull request is closed. Walkthrough공통 응답 형식으로 표준화하기 위해 ApiResponse에 정수형 코드를 받는 두 개의 오버로드 메서드를 추가하고, ImageController의 모든 이미지 업로드/삭제 엔드포인트를 ResponseEntity<ApiResponse<...>> 래퍼로 감싸는 형식으로 수정하여 일관된 응답 구조 제공. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Possibly related PRs
Poem
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR standardizes image API responses by wrapping them with ResponseEntity and ApiResponse to provide consistent HTTP status codes, headers, and response body structure across all image endpoints.
Key Changes:
- All image controller endpoints now return
ResponseEntity<ApiResponse<T>>instead of direct DTOs - Added new
ApiResponse.success()overloaded methods acceptingintparameters for status codes - Updated all POST endpoints to return HTTP 201 CREATED and DELETE endpoints to return HTTP 200 OK
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| ImageController.java | Wrapped all endpoint responses with ResponseEntity and ApiResponse, added appropriate HTTP status codes and descriptive messages |
| ApiResponse.java | Added convenience methods accepting int status codes to simplify controller usage |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public static <T> ApiResponse<T> success(int code, String message, T data) { | ||
| return new ApiResponse<>(String.valueOf(code), message, data); | ||
| } | ||
|
|
||
| public static <T> ApiResponse<T> success(int code, String message) { | ||
| return new ApiResponse<>(String.valueOf(code), message, null); | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
There's a parameter type inconsistency in the new overloaded methods. The existing success methods accept String code parameter, but the new methods accept int code and convert it to String internally. This creates an inconsistent API where callers must remember which overload accepts which type. Consider making all methods consistently accept String or consistently accept int to improve the API design.
| public static <T> ApiResponse<T> success(int code, String message, T data) { | |
| return new ApiResponse<>(String.valueOf(code), message, data); | |
| } | |
| public static <T> ApiResponse<T> success(int code, String message) { | |
| return new ApiResponse<>(String.valueOf(code), message, null); | |
| } |
|
|
||
| return ResponseEntity | ||
| .ok(ApiResponse.success( | ||
| HttpStatus.CREATED.toString(), |
There was a problem hiding this comment.
The HTTP status code is incorrect. The deleteOne operation returns 200 OK (via ResponseEntity.ok()), but the message body incorrectly contains HttpStatus.CREATED (201). This should be HttpStatus.OK.value() to match the actual response status.
| HttpStatus.CREATED.toString(), | |
| HttpStatus.OK.value(), |
| return new ApiResponse<>(String.valueOf(code), message, null); | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
[nitpick] Extra blank line should be removed for consistency with the rest of the file formatting.
📝 Pull Request
📌 PR 종류
해당하는 항목에 체크해주세요.
✨ 변경 내용
ResponseEntity로 감싸서 ApiResponse 공통 응답 클래스 형식으로 응답하도록 합니다. 정리하자면, HTTP의 의미(상태 코드, 헤더)는 ResponseEntity로, 본문의 구조(우리 서비스의 JSON 규칙)는 ApiResponse로 나눠서 맡기는 구조입니다.
둘을 분리해서 쓰기 때문에 HTTP 표준도 잘 지키고 프론트/도메인 입장에서도 일관된 응답 구조를 유지할 수 있어서 확장성과 유지보수가 훨씬 좋아집니다.나중에 Location 헤더나 Set-Cookie 같은 것도 한 번 붙여보면 ResponseEntity를 사용하고, ApiResponse 사용하는 메리트를 크게 느낄 수 있습니다.
🔍 관련 이슈
🧪 테스트
변경된 기능에 대한 테스트 범위 또는 테스트 결과를 작성해주세요.
🚨 확인해야 할 사항 (Checklist)
PR을 제출하기 전에 아래 항목들을 확인해주세요.
🙋 기타 참고 사항
리뷰어가 참고하면 좋을 만한 추가 설명이 있다면 적어주세요.
Summary by CodeRabbit
API 개선
✏️ Tip: You can customize this high-level summary in your review settings.