[CHORE] 초기 프로젝트 설정#2
Conversation
Walkthrough새 PR은 전역 예외/응답 DTO(성공·오류 코드 포함)와 글로벌 예외 핸들러, 테스트용 컨트롤러를 추가하고 JPA 매핑·컨버터·빌드 설정·리포지터리 메타(템플릿, .gitignore) 일부를 변경합니다. 애플리케이션 이름 설정이 제거되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Test as TestController
participant Val as BeanValidation
participant Handler as GlobalExceptionHandler
participant Resp as BaseResponse/ErrorResponse
Client->>Test: POST /test/validate-body (UserRequest)
Test->>Val: @Valid 검증
alt 검증 실패
Val-->>Handler: MethodArgumentNotValidException
Handler->>Resp: ErrorResponse.of(API_INVALID_PARAM, details)
Handler-->>Client: 400 + ErrorResponse
else 검증 성공
Test->>Resp: BaseResponse.ok(data)
Test-->>Client: 200 + BaseResponse
end
sequenceDiagram
autonumber
actor Client
participant Test as TestController
participant Handler as GlobalExceptionHandler
participant EC as ErrorCode
Client->>Test: GET /test/auth-error
Test->>Test: throw new AuthException(AUTH_UNAUTHORIZED)
Test-->>Handler: AuthException
Handler->>EC: AUTH_UNAUTHORIZED 조회
Handler-->>Client: 401 + ErrorResponse(AUTH_UNAUTHORIZED)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks (3 passed, 2 warnings)❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
Comment |
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (18)
.github/PULL_REQUEST_TEMPLATE.md (1)
1-16: 템플릿에 자동 이슈 닫기와 체크리스트 섹션을 추가하면 운영 효율이 좋아집니다.PR 머지 시 이슈 자동 Close, 리뷰/테스트 체크리스트가 있으면 누락을 줄일 수 있습니다.
## #️⃣연관된 이슈 - -> ex) #이슈번호, #이슈번호 +> ex) Closes #이슈번호, Resolves #이슈번호 @@ ## 💬리뷰 요구사항(선택) @@ > ex) 메서드 XXX의 이름을 더 잘 짓고 싶은데 혹시 좋은 명칭이 있을까요? + +## ✅ 체크리스트 +- [ ] 관련 이슈가 연결되어 있음 (Closes #) +- [ ] 변경 사항에 대한 자체 테스트/스크린샷 첨부 +- [ ] 문서/주석 업데이트 (해당 시) +- [ ] 브레이킹 체인지 명시 (해당 시) + +## ⚠️ 브레이킹 체인지(선택) +- 내용:build.gradle (1)
29-29: 보안 의존성 주석 처리 영향 검증 및 모듈화/프로필 적용 고려
spring-boot-starter-security 의존성을 주석 처리한 경우, 추후 보안 필터 체인 복원 시점에 401 응답 처리, 권한 맵핑, 테스트 유틸리티 재도입 범위를 반드시 점검하세요. 환경별 프로필(profile) 적용이나 의존성 모듈화로 관리하는 방식을 고려하면 유지보수성이 높아집니다.
참고: Spring Boot 최신 안정 버전은 3.5.5(릴리스 2025-08-21)이며, org.springframework.boot 3.5.5 + io.spring.dependency-management 1.1.7 조합은 유효합니다.src/main/java/konkuk/chacall/global/common/dto/SuccessCode.java (1)
1-18: 성공 코드 도입 LGTM.향후 HTTP Status 매핑 표준(예: 200 ↔ 20000)과 다국어 메시지 전략만 합의해 두면 됩니다.
src/main/java/konkuk/chacall/global/common/exception/EntityNotFoundException.java (1)
6-11: 원인(cause) 포함 생성자도 제공하면 디버깅이 수월합니다.public class EntityNotFoundException extends BusinessException { public EntityNotFoundException(ErrorCode errorCode) { super(errorCode); } + + public EntityNotFoundException(ErrorCode errorCode, Exception e) { + super(errorCode, e); + } }src/main/java/konkuk/chacall/global/common/dto/ResponseCode.java (1)
8-10: isSuccess의 instanceof 의존 제거 제안
default boolean isSuccess() { return this instanceof SuccessCode; }는 구현체 타입명에 결합되어 확장성/가독성이 떨어집니다. 인터페이스에서boolean isSuccess();만 선언하고 각 구현체(enum)에서 명시적으로 반환하도록 리팩터를 권장합니다.예시:
public interface ResponseCode { int getCode(); String getMessage(); boolean isSuccess(); }src/main/java/konkuk/chacall/global/common/exception/DomainRuleException.java (1)
6-7: 불필요한 @Getter 제거상위
BusinessException에서errorCode와 그에 대한@Getter를 이미 제공하므로, 현재 클래스의@Getter는 중복입니다. 제거하여 간결하게 유지하세요.-import lombok.Getter; - -@Getter public class DomainRuleException extends BusinessException {src/main/java/konkuk/chacall/global/common/dto/BaseResponse.java (1)
3-5: data null 숨김 처리로 응답 노이즈 감소성공 응답에서
data가 null인 경우 필드를 생략하도록@JsonInclude(Include.NON_NULL)를 권장합니다.import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonPropertyOrder; +import com.fasterxml.jackson.annotation.JsonInclude; import lombok.Getter; @Getter +@JsonInclude(JsonInclude.Include.NON_NULL) @JsonPropertyOrder({"isSuccess", "code", "message", "data"}) public class BaseResponse<T> {Also applies to: 7-9
src/main/java/konkuk/chacall/global/common/dto/ErrorResponse.java (1)
32-36: null/공백 message 안전 결합현재는
null전달 시"기본메시지 null"이 됩니다.null/blank일 때는 기본 메시지만 사용하도록 보완하세요.- public static ErrorResponse of(ResponseCode response, String message) { - StringBuilder sb = new StringBuilder(); - sb.append(response.getMessage()).append(" ").append(message); - return new ErrorResponse(response.isSuccess(), response.getCode(), sb.toString()); - } + public static ErrorResponse of(ResponseCode response, String message) { + String base = response.getMessage(); + String finalMsg = (message == null || message.isBlank()) ? base : base + " " + message; + return new ErrorResponse(response.isSuccess(), response.getCode(), finalMsg); + }src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java (1)
10-35: 에러 코드 체계 문서화 및 CI 중복 검증 추가
에러 코드 범위(4xxxx: API, 5xxxx: 서버, 6xxxx: 도메인)를 README.md에 명시하고, CI 파이프라인에 중복 검증 스크립트를 등록해 관리하세요. (현재 스크립트 실행 결과 중복 없음 확인됨)src/main/java/konkuk/chacall/domain/test/TestController.java (1)
79-83: @Valid 중복 표기 정리메소드 파라미터의 제약(@min)은
@Validated로 충분하며@Valid는 불필요합니다. 혼동 방지를 위해 제거 권장.- public String constraint(@RequestParam("id") @Valid @Min(1) int id) { + public String constraint(@RequestParam("id") @Min(1) int id) {src/main/java/konkuk/chacall/global/common/exception/handler/GlobalExceptionHandler.java (8)
10-10: 불필요한 @requiredargsconstructor 제거본 클래스에 주입 필드가 없어 @requiredargsconstructor 및 해당 import가 불필요합니다.
- import lombok.RequiredArgsConstructor; @@ -@RequiredArgsConstructor public class GlobalExceptionHandler {Also applies to: 31-31
45-45: 클라이언트 오류(4xx) 로깅 레벨을 warn으로 낮추기유효성·요청 오류는 서비스 정상 동작 범주입니다. error→warn으로 완화하면 알림 노이즈를 줄일 수 있습니다.
- log.error("[HttpRequestMethodNotSupportedExceptionHandler] {}", e.getMessage()); + log.warn("[HttpRequestMethodNotSupportedExceptionHandler] {}", e.getMessage()); @@ - log.error("[MethodArgumentNotValidExceptionHandler] {}", e.getMessage()); + log.warn("[MethodArgumentNotValidExceptionHandler] {}", e.getMessage()); @@ - log.error("[MethodArgumentTypeMismatchExceptionHandler] {}", e.getMessage()); + log.warn("[MethodArgumentTypeMismatchExceptionHandler] {}", e.getMessage()); @@ - log.error("[MissingServletRequestParameterExceptionHandler] {}", e.getMessage()); + log.warn("[MissingServletRequestParameterExceptionHandler] {}", e.getMessage()); @@ - log.error("[ConstraintViolationExceptionHandler] {}", e.getMessage()); + log.warn("[ConstraintViolationExceptionHandler] {}", e.getMessage()); @@ - log.error("[HandlerMethodValidationException] {}", e.getMessage()); + log.warn("[HandlerMethodValidationException] {}", e.getMessage());Also applies to: 54-54, 71-71, 81-81, 90-90, 107-107
56-66: MethodArgumentNotValidException 응답 메시지 개선(필드명+다중 오류 지원)첫 번째 FieldError만 사용하는 대신 모든 오류를 묶고 필드명을 포함하면 디버깅이 빨라집니다.
- // 첫 번째 유효성 검사 실패 메시지만 가져오기 - String errorMessage = e.getBindingResult() - .getFieldErrors() - .stream() - .findFirst() - .map(error -> error.getDefaultMessage()) - .orElse("Validation failed"); + // 모든 오류 메시지를 필드명과 함께 병합 + String errorMessage = e.getBindingResult() + .getAllErrors() + .stream() + .map(error -> { + if (error instanceof org.springframework.validation.FieldError) { + org.springframework.validation.FieldError fe = (org.springframework.validation.FieldError) error; + return fe.getField() + ": " + + java.util.Optional.ofNullable(fe.getDefaultMessage()) + .orElse("유효하지 않은 값입니다."); + } + return java.util.Optional.ofNullable(error.getDefaultMessage()) + .orElse("유효하지 않은 값입니다."); + }) + .collect(java.util.stream.Collectors.joining(" | "));
73-76: 타입 불일치 메시지 가독성 개선 및 NPE 방지getRequiredType()은 null일 수 있고, 현재 출력은 “class java.lang.Long” 형태입니다. 단순 클래스명과 입력값을 포함합시다.
- return ResponseEntity - .status(API_INVALID_TYPE.getHttpStatus()) - .body(ErrorResponse.of(API_INVALID_TYPE, e.getName() + "는 " + e.getRequiredType() + " 타입이어야 합니다.")); + Class<?> required = e.getRequiredType(); + String requiredName = required != null ? required.getSimpleName() : "요구된 타입"; + String value = String.valueOf(e.getValue()); + String detail = String.format("%s는 %s 타입이어야 합니다. (입력값: %s)", e.getName(), requiredName, value); + return ResponseEntity + .status(API_INVALID_TYPE.getHttpStatus()) + .body(ErrorResponse.of(API_INVALID_TYPE, detail));
91-103: ConstraintViolationException 다중 메시지 병합 및 경로 표시첫 번째 위반만 반환하면 진단이 어려울 수 있습니다. 모든 violation을 propertyPath: message 형태로 병합하세요.
- // 첫 번째 위반만 꺼내서 - ConstraintViolation<?> violation = e.getConstraintViolations().stream().findFirst().orElse(null); - - // 기본 메시지 또는 제약조건 메시지 사용 - String errorMessage = Optional.ofNullable(violation) - .map(v -> v.getMessage()) - .orElse("유효성 검사에 실패했습니다."); - - // API_INVALID_PARAM 코드를 공통으로 사용 - return ResponseEntity - .status(API_INVALID_PARAM.getHttpStatus()) - .body(ErrorResponse.of(API_INVALID_PARAM, errorMessage)); + String errorMessage = e.getConstraintViolations().stream() + .map(v -> { + String path = Optional.ofNullable(v.getPropertyPath()) + .map(Object::toString).orElse("parameter"); + String msg = Optional.ofNullable(v.getMessage()) + .orElse("유효하지 않은 값입니다."); + return path + ": " + msg; + }) + .collect(Collectors.joining(" | ")); + + return ResponseEntity + .status(API_INVALID_PARAM.getHttpStatus()) + .body(ErrorResponse.of(API_INVALID_PARAM, errorMessage.isBlank() + ? "유효성 검사에 실패했습니다." + : errorMessage));
182-185: IllegalStateException 로그에 스택트레이스 포함디버깅을 위해 스택을 남겨주세요. RuntimeException 핸들러와 일관성도 맞춰집니다.
- log.error("[IllegalStateExceptionHandler] {}", e.getMessage()); + log.error("[IllegalStateExceptionHandler] {}", e.getMessage(), e);
51-86: 추가 핸들러 제안: HttpMessageNotReadableException/BindExceptionJSON 파싱 오류 및 바인딩 오류(@ModelAttribute)도 400으로 통일 처리하면 UX가 좋아집니다.
아래 메서드를 추가하는 것을 제안드립니다.
// JSON 파싱/역직렬화 오류 @ExceptionHandler(org.springframework.http.converter.HttpMessageNotReadableException.class) public ResponseEntity<ErrorResponse> httpMessageNotReadableExceptionHandler(org.springframework.http.converter.HttpMessageNotReadableException e) { log.warn("[HttpMessageNotReadableException] {}", e.getMessage()); return ResponseEntity .status(API_INVALID_PARAM.getHttpStatus()) .body(ErrorResponse.of(API_INVALID_PARAM, "요청 본문이 올바른 JSON 형식이 아닙니다.")); } // 바인딩 오류(@ModelAttribute 등) @ExceptionHandler(org.springframework.validation.BindException.class) public ResponseEntity<ErrorResponse> bindExceptionHandler(org.springframework.validation.BindException e) { log.warn("[BindException] {}", e.getMessage()); String detail = e.getAllErrors().stream() .map(err -> { if (err instanceof org.springframework.validation.FieldError fe) { return fe.getField() + ": " + java.util.Optional.ofNullable(fe.getDefaultMessage()).orElse("유효하지 않은 값입니다."); } return java.util.Optional.ofNullable(err.getDefaultMessage()).orElse("유효하지 않은 값입니다."); }) .collect(java.util.stream.Collectors.joining(" | ")); return ResponseEntity .status(API_INVALID_PARAM.getHttpStatus()) .body(ErrorResponse.of(API_INVALID_PARAM, detail)); }원하시면 MockMvc 테스트 케이스도 함께 드리겠습니다.
112-114: Gradle 빌드에-parameters컴파일 옵션 추가
현재build.gradle에-parameters설정이 없어getParameterName()호출 시 파라미터명이 null이 될 수 있습니다. diagnostics 개선을 위해build.gradle에 아래 설정을 추가하세요:tasks.withType(JavaCompile) { options.compilerArgs += ['-parameters'] }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
.github/PULL_REQUEST_TEMPLATE.md(1 hunks).gitignore(1 hunks)build.gradle(1 hunks)src/main/java/konkuk/chacall/domain/chat/domain/ChatRoom.java(1 hunks)src/main/java/konkuk/chacall/domain/foodtruck/domain/FoodTruck.java(2 hunks)src/main/java/konkuk/chacall/domain/test/TestController.java(1 hunks)src/main/java/konkuk/chacall/domain/user/User.java(2 hunks)src/main/java/konkuk/chacall/global/common/dto/BaseResponse.java(1 hunks)src/main/java/konkuk/chacall/global/common/dto/ErrorResponse.java(1 hunks)src/main/java/konkuk/chacall/global/common/dto/ResponseCode.java(1 hunks)src/main/java/konkuk/chacall/global/common/dto/SuccessCode.java(1 hunks)src/main/java/konkuk/chacall/global/common/exception/AuthException.java(1 hunks)src/main/java/konkuk/chacall/global/common/exception/BusinessException.java(1 hunks)src/main/java/konkuk/chacall/global/common/exception/DomainRuleException.java(1 hunks)src/main/java/konkuk/chacall/global/common/exception/EntityNotFoundException.java(1 hunks)src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java(1 hunks)src/main/java/konkuk/chacall/global/common/exception/handler/GlobalExceptionHandler.java(1 hunks)src/main/resources/application.properties(0 hunks)
💤 Files with no reviewable changes (1)
- src/main/resources/application.properties
🧰 Additional context used
🧬 Code graph analysis (7)
src/main/java/konkuk/chacall/global/common/exception/BusinessException.java (2)
src/main/java/konkuk/chacall/global/common/exception/AuthException.java (1)
Getter(6-18)src/main/java/konkuk/chacall/global/common/exception/DomainRuleException.java (1)
Getter(6-15)
src/main/java/konkuk/chacall/global/common/exception/handler/GlobalExceptionHandler.java (1)
src/main/java/konkuk/chacall/global/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(6-11)
src/main/java/konkuk/chacall/global/common/dto/BaseResponse.java (1)
src/main/java/konkuk/chacall/global/common/dto/ErrorResponse.java (1)
Getter(7-37)
src/main/java/konkuk/chacall/global/common/exception/DomainRuleException.java (2)
src/main/java/konkuk/chacall/global/common/exception/AuthException.java (1)
Getter(6-18)src/main/java/konkuk/chacall/global/common/exception/BusinessException.java (1)
Getter(6-19)
src/main/java/konkuk/chacall/domain/test/TestController.java (1)
src/main/java/konkuk/chacall/global/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(6-11)
src/main/java/konkuk/chacall/global/common/dto/ErrorResponse.java (1)
src/main/java/konkuk/chacall/global/common/dto/BaseResponse.java (1)
Getter(7-35)
src/main/java/konkuk/chacall/global/common/exception/AuthException.java (2)
src/main/java/konkuk/chacall/global/common/exception/BusinessException.java (1)
Getter(6-19)src/main/java/konkuk/chacall/global/common/exception/DomainRuleException.java (1)
Getter(6-15)
🔇 Additional comments (6)
.gitignore (1)
7-7: 현재 저장소에.yml파일이 없어*.yml패턴이 적용되는 파일이 없습니다 — 추가 조치 불필요합니다.src/main/java/konkuk/chacall/domain/foodtruck/domain/FoodTruck.java (2)
6-7: 컨버터 import 정리는 적절합니다.의존성/패키지 구조와 일관되고 변경 의도도 명확합니다.
39-45: 컬럼 타입 텍스트 지정 및 최대 길이 검증 필요
foodTruckPhotoList와menuCategoryList는 JSON/String 직렬화 컬럼으로, 기본 VARCHAR(255)로는 잘릴 위험이 있으므로TEXT를 명시하세요. 컨버터(convertToDatabaseColumn)가 반환하는 최대 문자열 길이를 공유하고, DB 마이그레이션 스크립트에도 동일하게 반영해야 합니다.File: src/main/java/konkuk/chacall/domain/foodtruck/domain/FoodTruck.java
@@ - @Column(nullable = false) + @Column(nullable = false, columnDefinition = "TEXT") private PhotoUrlList foodTruckPhotoList; @@ - @Column(nullable = false) + @Column(nullable = false, columnDefinition = "TEXT") private MenuCategoryList menuCategoryList;컨버터 반환 문자열의 예상 최대 길이 확인 후 공유해주세요.
src/main/java/konkuk/chacall/global/common/exception/BusinessException.java (1)
1-19: 표준 에러코드 기반 예외 베이스 클래스 구현 깔끔합니다.메시지 체이닝과 cause 보존 모두 적절합니다.
src/main/java/konkuk/chacall/global/common/exception/handler/GlobalExceptionHandler.java (2)
29-33: 전역 예외 처리 구조 잘 잡혔습니다표준화된 ErrorResponse 매핑과 예외별 세분화가 명료합니다. 운영에서의 가시성과 유지보수에 도움이 되겠습니다.
109-129: HandlerMethodValidationException detail 구성 방식 좋습니다파라미터 단위로 메시지를 집계해 응답 detail을 제공하는 접근이 실용적입니다. 빈 메시지에 대한 대체 처리도 적절합니다.
| @JoinColumn(name = "member_Id", nullable = false, referencedColumnName = "user_Id") | ||
| private User member; | ||
|
|
||
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "user_id", nullable = false) | ||
| @JoinColumn(name = "owner_Id", nullable = false, referencedColumnName = "user_Id") | ||
| private User owner; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
FK 컬럼명/참조 컬럼 케이스 혼용 및 불필요한 referencedColumnName
- 컬럼명이
member_Id,owner_Id,user_Id처럼 케이스가 혼재되어 DB 설정(대소문자 민감/인용)에 따라 문제를 유발할 수 있습니다. 일반적으로 snake_case 소문자(member_id,owner_id,user_id)로 통일을 권장합니다. - PK를 참조하는
@ManyToOne에서는referencedColumnName가 불필요하며, 오타/스키마 변경 시 깨질 가능성만 높입니다. 제거를 권장합니다.
- @JoinColumn(name = "member_Id", nullable = false, referencedColumnName = "user_Id")
+ @JoinColumn(name = "member_id", nullable = false)
private User member;
- @JoinColumn(name = "owner_Id", nullable = false, referencedColumnName = "user_Id")
+ @JoinColumn(name = "owner_id", nullable = false)
private User owner;다음 스크립트로 User 엔티티의 PK 컬럼명이 실제로 무엇인지 확인해 주세요(케이스/언더스코어 일치 여부):
🏁 Script executed:
#!/bin/bash
rg -nC3 -P '(?s)class\s+User\b.*?@Id.*?(@Column\([^)]*\))?.*?\b(Long|Integer|UUID|String)\s+(\w+)\s*;' --type=javaLength of output: 0
🏁 Script executed:
#!/bin/bash
# User 엔티티 파일 경로 찾기
USER_FILE=$(find . -type f -name 'User.java' | head -n 1)
echo "User entity 경로: $USER_FILE"
# @Id가 선언된 필드 및 @Column 어노테이션 확인
rg -nC2 '@Id' "$USER_FILE"
rg -nC2 '@Column' "$USER_FILE"Length of output: 929
snake_case 소문자 컬럼명 통일 및 불필요한 referencedColumnName 제거
User 엔티티의 PK가 user_id(소문자 언더스코어)이므로, member_Id·owner_Id를 각각 member_id·owner_id로 변경하고, referencedColumnName은 기본 PK 참조에 불필요하므로 제거하세요.
- @JoinColumn(name = "member_Id", nullable = false, referencedColumnName = "user_Id")
+ @JoinColumn(name = "member_id", nullable = false)
private User member;
- @ManyToOne(fetch = FetchType.LAZY)
- @JoinColumn(name = "owner_Id", nullable = false, referencedColumnName = "user_Id")
+ @ManyToOne(fetch = FetchType.LAZY)
+ @JoinColumn(name = "owner_id", nullable = false)
private User owner;🤖 Prompt for AI Agents
In src/main/java/konkuk/chacall/domain/chat/domain/ChatRoom.java around lines 19
to 24, the @JoinColumn names use mixed/camel case and explicitly set
referencedColumnName; change the column names to snake_case "member_id" and
"owner_id" respectively and remove the referencedColumnName attribute from both
annotations so they default to the User primary key; keep fetch and nullable
settings unchanged.
| // DTO 내부 유효성 검증용 클래스 | ||
| @Getter | ||
| public static class UserRequest { | ||
| @NotBlank(message = "이름은 필수 값입니다.") | ||
| private String name; | ||
|
|
||
| @Size(min = 5, max = 20, message = "닉네임은 5~20자 사이여야 합니다.") | ||
| private String nickname; | ||
|
|
There was a problem hiding this comment.
RequestBody DTO 역직렬화 실패 가능성(@Setter 없음)
UserRequest는 @Getter만 있어 Jackson이 바인딩할 수 없습니다. @Setter(또는 record/생성자) 추가가 필요합니다.
- @Getter
+ @Getter
+ @lombok.Setter
public static class UserRequest {
@NotBlank(message = "이름은 필수 값입니다.")
private String name;
@Size(min = 5, max = 20, message = "닉네임은 5~20자 사이여야 합니다.")
private String nickname;대안: public record UserRequest(@NotBlank String name, @Size(min=5,max=20) String nickname) {} 사용.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // DTO 내부 유효성 검증용 클래스 | |
| @Getter | |
| public static class UserRequest { | |
| @NotBlank(message = "이름은 필수 값입니다.") | |
| private String name; | |
| @Size(min = 5, max = 20, message = "닉네임은 5~20자 사이여야 합니다.") | |
| private String nickname; | |
| // DTO 내부 유효성 검증용 클래스 | |
| @Getter | |
| @lombok.Setter | |
| public static class UserRequest { | |
| @NotBlank(message = "이름은 필수 값입니다.") | |
| private String name; | |
| @Size(min = 5, max = 20, message = "닉네임은 5~20자 사이여야 합니다.") | |
| private String nickname; | |
| // … remaining fields … | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/chacall/domain/test/TestController.java around lines 85
to 93, the UserRequest DTO only has @Getter so Jackson cannot deserialize
request bodies into it; add a way for Jackson to set fields—either annotate the
class with @Setter, implement an all-args constructor or convert it to a public
record (e.g., public record UserRequest(@NotBlank String name,
@Size(min=5,max=20) String nickname) {})—and keep the existing validation
annotations on the fields/record components so validation still applies.
| public AuthException(ErrorCode errorCode) { | ||
| this.errorCode = errorCode; | ||
| } | ||
|
|
||
| public AuthException(ErrorCode errorCode, Exception e) { | ||
| super(e); | ||
| this.errorCode = errorCode; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
예외 메시지 누락: super(message)로 로깅 가시성 확보
현재 RuntimeException에 메시지를 전달하지 않아 스택트레이스에서 원인 파악이 어렵습니다. errorCode.getMessage()를 super로 넘기세요.
- public AuthException(ErrorCode errorCode) {
- this.errorCode = errorCode;
- }
+ public AuthException(ErrorCode errorCode) {
+ super(errorCode.getMessage());
+ this.errorCode = errorCode;
+ }
- public AuthException(ErrorCode errorCode, Exception e) {
- super(e);
- this.errorCode = errorCode;
- }
+ public AuthException(ErrorCode errorCode, Exception e) {
+ super(errorCode.getMessage(), e);
+ this.errorCode = errorCode;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public AuthException(ErrorCode errorCode) { | |
| this.errorCode = errorCode; | |
| } | |
| public AuthException(ErrorCode errorCode, Exception e) { | |
| super(e); | |
| this.errorCode = errorCode; | |
| } | |
| public AuthException(ErrorCode errorCode) { | |
| super(errorCode.getMessage()); | |
| this.errorCode = errorCode; | |
| } | |
| public AuthException(ErrorCode errorCode, Exception e) { | |
| super(errorCode.getMessage(), e); | |
| this.errorCode = errorCode; | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/chacall/global/common/exception/AuthException.java
around lines 10 to 17, the constructors do not pass the error message to the
RuntimeException super, which reduces stacktrace/message visibility; change the
no-cause constructor to call super(errorCode.getMessage()) and the cause-taking
constructor to call super(errorCode.getMessage(), e) while still assigning
this.errorCode = errorCode.
| String detail = Optional.ofNullable(e.getCause()).map(Throwable::getMessage).orElse(""); | ||
| return ResponseEntity | ||
| .status(e.getErrorCode().getHttpStatus()) | ||
| .body(ErrorResponse.of(e.getErrorCode(), detail)); |
There was a problem hiding this comment.
내부 구현 상세(예외 cause 메시지) 노출 위험
detail에 e.getCause().getMessage()를 그대로 노출하면 내부 스택/SQL/비밀 키워드가 유출될 수 있습니다. 사용자 메시지는 e.getMessage() 또는 사전 정의된 안전한 메시지로 한정하세요. 운영 환경에서는 cause는 로그에만 남기는 것이 안전합니다.
- String detail = Optional.ofNullable(e.getCause()).map(Throwable::getMessage).orElse("");
+ String detail = Optional.ofNullable(e.getMessage()).orElse("");
@@
- String detail = Optional.ofNullable(e.getCause()).map(Throwable::getMessage).orElse("");
+ String detail = Optional.ofNullable(e.getMessage()).orElse("");
@@
- String detail = Optional.ofNullable(e.getCause()).map(Throwable::getMessage).orElse("");
+ String detail = Optional.ofNullable(e.getMessage()).orElse("");Also applies to: 154-157, 164-167
ksg1227
left a comment
There was a problem hiding this comment.
전체적으로 깔끔한 것 같습니다!
기존에 제가 진행하던 프로젝트랑은 약간 다른 부분들이 있어서, 그 부분들 잘 확인하면서 개발 해보겠슴다
|
|
||
| // DTO 내부 유효성 검증용 클래스 | ||
| @Getter | ||
| public static class UserRequest { |
There was a problem hiding this comment.
이 클래스는 외부에서도 가져다가 쓸 수 있는 클래스인가요? 아니라면 private 으로 닫아주는 게 나을 것 같습니다요
| @GetMapping("/hello") | ||
| public String hello() { | ||
| return "Hello, World!"; | ||
| } | ||
|
|
||
| @GetMapping("/ping") | ||
| public BaseResponse<String> ping() { | ||
| return BaseResponse.ok("pong"); | ||
| } | ||
|
|
||
| // === 커스텀 예외들 === | ||
| @GetMapping("/auth-error") | ||
| public String authError() { | ||
| throw new AuthException(ErrorCode.AUTH_UNAUTHORIZED); | ||
| } |
There was a problem hiding this comment.
커스텀 예외들을 확인할 수 있는 테스트용 컨트롤러들 만드신 것 좋네요!
프론트 단에서 예외 형식 확인할 때 좀 더 편할 것 같아요
| import lombok.Getter; | ||
|
|
||
| @Getter | ||
| public enum SuccessCode implements ResponseCode { |
There was a problem hiding this comment.
성공 케이스도 세분화하는 형식이군여 확인했습니다!
There was a problem hiding this comment.
음 공통적으로 HttpStatus 200일 때는 API_SUCCESS를 던져주긴 할건데 ErrorCode와 일관성을 맞추기 위해서 정의해두었습니다!
| .body(ErrorResponse.of(e.getErrorCode(), detail)); | ||
| } | ||
|
|
||
| // === 우선순위 4: 일반 비즈니스 예외(캐치올 for BusinessException) === |
There was a problem hiding this comment.
예외별 우선순위는 어떤 걸 의미하는 것인가요?
특정 상황에서 문제가 터졌을 때, 현재 어떤 문제가 터졌는지를 확인하고, 우선순위 1 -> 4까지 확인하며 먼저 매칭되는 케이스에 해당하는 예외를 던져주면 된다. 이런 의미로 이해했는데 맞을까용
There was a problem hiding this comment.
각 비즈니스 예외 클래스 내부를 보시면 모두 BusinessException을 상속하고 있습니다. 따라서 원래는 BusinessException만 핸들링해주어도 모든 예외가 핸들링이 가능하지만, 로그를 조금더 세분화하고 비즈니스 코드의 가독성을 높이기 위해서 handling의 우선순위를 정해두어 더 세분화된 예외부터 먼저 핸들링되도록 했습니다! 우선순위의 기준은 보통의 비즈니스 로직을 생각해서 구상해보았는데,
- 보통 필터 단에서 인증.인가 로직이 수행되므로 AuthException 1순위
- 로그인한 User 또는 특정 엔티티에 대한 조회가 모든 비즈니스 로직에서 선행되므로 EntityNotFound 2순위
- BusinessException과 DomainRuleException 둘중에서는 조금 고민이 되었는데 두 예외중에 더 중요도가 높은 예외가 도메인에서 발생하는 예외라고 생각해서 DomainRuleException의 우선순위를 더 먼저두었습니다
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/main/java/konkuk/chacall/global/common/dto/BaseResponse.java (1)
7-9: JsonPropertyOrder/JsonProperty 정합성 확인 — LGTM
"isSuccess"키와 정렬 설정이 일치합니다. 이전 코멘트가 반영되었습니다.src/main/java/konkuk/chacall/domain/test/TestController.java (2)
14-18: 메소드 파라미터 검증을 활성화하려면 @validated 필요
@RequestParam에 붙은 제약(@min 등)이 동작하려면 컨트롤러(또는 메서드)에@Validated가 선언되어야 합니다.import org.springframework.web.bind.annotation.*; +import org.springframework.validation.annotation.Validated; @RestController +@Validated @RequestMapping("/test") public class TestController {
85-93: @RequestBody DTO 역직렬화 실패 가능 — Setter 또는 record 필요
UserRequest는@Getter만 있어 기본 설정의 Jackson이 값을 주입하지 못합니다(필드 접근 활성화 전제 없음).// DTO 내부 유효성 검증용 클래스 - @Getter + @Getter + @lombok.Setter private static class UserRequest { @NotBlank(message = "이름은 필수 값입니다.") private String name;대안:
private static record UserRequest(@NotBlank String name, @Size(min=5, max=20) String nickname) {}로 전환.
🧹 Nitpick comments (4)
src/main/java/konkuk/chacall/global/common/dto/BaseResponse.java (2)
3-9: null 데이터 필드 숨김 권장(@JsonInclude 추가)
data가 null인 경우 응답 페이로드에서 필드를 생략하면 클라이언트 처리에 유리합니다.다음 패치를 제안합니다:
import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonPropertyOrder; +import com.fasterxml.jackson.annotation.JsonInclude; import lombok.Getter; @Getter +@JsonInclude(JsonInclude.Include.NON_NULL) @JsonPropertyOrder({"isSuccess", "code", "message", "data"}) public class BaseResponse<T> {
31-33: 데이터 없이도 성공 응답이 필요할 때를 위한 편의 메서드 추가바디 없이 200 성공을 내려야 하는 엔드포인트(예: ping)에서 유용합니다. 위의
@JsonInclude와 함께 쓰면data가 출력되지 않습니다.public static <T> BaseResponse<T> ok(T data) { return new BaseResponse<>(SuccessCode.API_SUCCESS, data); } + public static BaseResponse<Void> ok() { + return new BaseResponse<>(SuccessCode.API_SUCCESS, null); + }src/main/java/konkuk/chacall/domain/test/TestController.java (2)
81-81: @RequestParam에 붙은 @Valid는 불필요메서드 파라미터 제약은
@Validated만 있으면 동작합니다. 중복 표기는 제거해도 됩니다.- public String constraint(@RequestParam("id") @Valid @Min(1) int id) { + public String constraint(@RequestParam("id") @Min(1) int id) {
20-28: 응답 포맷 일관성 제안데모용이라도 성공 응답을
BaseResponse로 통일하면 클라이언트가 한 형태만 처리하면 됩니다(예:/hello).- public String hello() { - return "Hello, World!"; - } + public BaseResponse<String> hello() { + return BaseResponse.ok("Hello, World!"); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/konkuk/chacall/domain/chat/domain/ChatRoom.java(1 hunks)src/main/java/konkuk/chacall/domain/test/TestController.java(1 hunks)src/main/java/konkuk/chacall/global/common/dto/BaseResponse.java(1 hunks)src/main/java/konkuk/chacall/global/common/dto/ErrorResponse.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/konkuk/chacall/domain/chat/domain/ChatRoom.java
- src/main/java/konkuk/chacall/global/common/dto/ErrorResponse.java
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/java/konkuk/chacall/domain/test/TestController.java (1)
src/main/java/konkuk/chacall/global/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(6-11)
src/main/java/konkuk/chacall/global/common/dto/BaseResponse.java (1)
src/main/java/konkuk/chacall/global/common/dto/ErrorResponse.java (1)
Getter(7-37)
closes #1
@Valid어노테이션을 이용하면 ExceptionHandler에서 핸들링하도록 하였습니다.Summary by CodeRabbit