refactor/#119 에러 코드 문서화 작업 및 리팩토링 #120
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough이 변경 사항은 여러 예외 코드 열거형(enum) 및 관련 클래스의 메시지와 구조를 수정하는 것을 포함합니다. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (10)
aics-domain/src/main/java/kgu/developers/domain/user/exception/InvalidPasswordException.java (1)
7-11: 사용자 정의 메시지를 지원하는 생성자 추가를 고려해보세요.향후 확장성을 위해 추가 컨텍스트나 상세 정보를 포함할 수 있는 생성자를 추가하는 것이 좋을 것 같습니다.
다음과 같은 생성자 추가를 제안드립니다:
public class InvalidPasswordException extends CustomException { public InvalidPasswordException() { super(INVALID_PASSWORD); } + + public InvalidPasswordException(String message) { + super(INVALID_PASSWORD, message); + } }aics-api/src/main/java/kgu/developers/api/professor/presentation/exception/ProfessorExceptionCode.java (1)
5-5: 불필요한 import 문을 제거하세요
HttpStatus는 이미 1번 줄에서 static import로 가져오고 있습니다. 코드 정리를 위해 중복된 import 문을 제거하는 것이 좋습니다.-import org.springframework.http.HttpStatus;aics-api/src/main/java/kgu/developers/api/about/presentation/Exception/AboutExceptionCode.java (1)
6-6: 불필요한 import 제거 필요
HttpStatus는 이미 static import로BAD_REQUEST와NOT_FOUND를 가져오고 있어 직접적으로 사용되지 않습니다.다음과 같이 수정하는 것을 제안합니다:
-import org.springframework.http.HttpStatus;aics-domain/src/main/java/kgu/developers/domain/user/exception/UserDomainExceptionCode.java (2)
6-6: 중복된 import 문을 제거해주세요.
HttpStatus는 이미 static import로 가져오고 있어 추가 import가 불필요합니다.-import org.springframework.http.HttpStatus;
17-18: 도메인 계층으로의 비밀번호 검증 로직 이동이 잘 되었습니다.
- 비밀번호 검증 실패에 대한 예외 코드가 도메인 계층에 적절히 위치했습니다.
- HTTP 400 Bad Request 상태 코드와 메시지가 적절합니다.
- 열거형 상수 추가를 위한 trailing comma 사용이 좋습니다.
이러한 도메인 중심적인 예외 처리 방식은 계층 간 책임을 명확히 하고 도메인 규칙을 더 잘 표현할 수 있게 해줍니다.
aics-api/src/main/java/kgu/developers/api/auth/application/AuthService.java (2)
33-34: 토큰 만료 시간을 설정 파일로 분리하는 것을 제안합니다.토큰 만료 시간이 하드코딩되어 있습니다. 환경에 따라 쉽게 설정을 변경할 수 있도록 설정 파일로 분리하는 것이 좋습니다.
다음과 같은 방식으로 리팩토링하는 것을 제안합니다:
+@Value("${jwt.access-token.expiration}") +private Duration accessTokenExpiration; + +@Value("${jwt.refresh-token.expiration}") +private Duration refreshTokenExpiration; @Transactional(readOnly = true) public TokenResponse login(LoginRequest request) { // ... - String refreshToken = tokenProvider.generateToken(user.getId(), Duration.ofDays(7)); - String accessToken = tokenProvider.generateToken(user.getId(), Duration.ofHours(2)); + String refreshToken = tokenProvider.generateToken(user.getId(), refreshTokenExpiration); + String accessToken = tokenProvider.generateToken(user.getId(), accessTokenExpiration);
Line range hint
35-35: 리프레시 토큰 저장 구현이 필요합니다.리프레시 토큰 저장이 구현되지 않은 상태입니다. 보안상 중요한 기능이므로 우선적으로 구현이 필요합니다.
리프레시 토큰 저장 구현을 위한 코드를 생성해드릴까요? 또는 이를 추적하기 위한 GitHub 이슈를 생성해드릴까요?
aics-domain/src/testFixtures/java/user/domain/UserDomainTest.java (2)
82-103: 비밀번호 검증 테스트가 잘 구현되었습니다만, 긍정 케이스도 추가하면 좋겠습니다.현재 구현된 테스트는 잘못된 비밀번호에 대한 실패 케이스만 검증하고 있습니다. 올바른 비밀번호를 입력했을 때 정상적으로 처리되는 케이스도 함께 테스트하면 좋겠습니다.
다음과 같은 테스트 메소드 추가를 제안드립니다:
@Test @DisplayName("올바른 비밀번호 입력 시 정상적으로 검증된다") public void isPasswordMatching_ValidPassword_Success() { // given String rawPassword = "correctPassword"; PasswordEncoder passwordEncoder = new BCryptPasswordEncoder(); String encodedPassword = passwordEncoder.encode(rawPassword); User user = User.create( "202411345", encodedPassword, "홍길동", "valid@kgu.ac.kr", "010-1234-5678", Major.CSE ); // when & then assertDoesNotThrow(() -> { user.isPasswordMatching(rawPassword, passwordEncoder); }); }
81-103: 도메인 계층으로의 비밀번호 검증 이동이 잘 구현되었습니다.PR의 목적대로 비밀번호 검증 로직이 서비스 계층에서 도메인 계층으로 잘 이동되었으며, 이를 검증하는 테스트도 적절히 구현되었습니다.
User엔티티가 자신의 비밀번호를 검증하는 것은 도메인 주도 설계 원칙에도 부합합니다.aics-domain/src/main/java/kgu/developers/domain/user/domain/User.java (1)
132-136: 메소드 이름 개선이 필요합니다.현재 메소드 이름
isPasswordMatching은 boolean을 반환할 것 같은 인상을 주지만, 실제로는 void를 반환하고 예외를 발생시킵니다. 다음과 같은 이름을 제안드립니다:-public void isPasswordMatching(String rawPassword, PasswordEncoder passwordEncoder) { +public void validatePassword(String rawPassword, PasswordEncoder passwordEncoder) {비밀번호 검증 로직 자체는 잘 구현되어 있습니다:
- PasswordEncoder를 사용한 안전한 비밀번호 비교
- 도메인 계층에 맞는 예외 처리
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (16)
aics-api/src/main/java/kgu/developers/api/about/presentation/Exception/AboutExceptionCode.java(1 hunks)aics-api/src/main/java/kgu/developers/api/auth/application/AuthService.java(1 hunks)aics-api/src/main/java/kgu/developers/api/auth/presentation/exception/AuthExceptionCode.java(0 hunks)aics-api/src/main/java/kgu/developers/api/auth/presentation/exception/InvalidPasswordException.java(0 hunks)aics-api/src/main/java/kgu/developers/api/comment/presentation/exception/CommentExceptionCode.java(1 hunks)aics-api/src/main/java/kgu/developers/api/file/presentation/exception/FileExceptionCode.java(1 hunks)aics-api/src/main/java/kgu/developers/api/lab/presentation/exception/LabExceptionCode.java(1 hunks)aics-api/src/main/java/kgu/developers/api/post/presentation/exception/PostExceptionCode.java(1 hunks)aics-api/src/main/java/kgu/developers/api/professor/presentation/exception/ProfessorExceptionCode.java(1 hunks)aics-api/src/main/java/kgu/developers/api/user/presentation/exception/UserExceptionCode.java(1 hunks)aics-api/src/main/java/kgu/developers/api/user/presentation/exception/UserIdDuplicateException.java(1 hunks)aics-api/src/testFixtures/java/auth/application/AuthServiceTest.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/user/domain/User.java(3 hunks)aics-domain/src/main/java/kgu/developers/domain/user/exception/InvalidPasswordException.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/user/exception/UserDomainExceptionCode.java(1 hunks)aics-domain/src/testFixtures/java/user/domain/UserDomainTest.java(2 hunks)
💤 Files with no reviewable changes (2)
- aics-api/src/main/java/kgu/developers/api/auth/presentation/exception/AuthExceptionCode.java
- aics-api/src/main/java/kgu/developers/api/auth/presentation/exception/InvalidPasswordException.java
✅ Files skipped from review due to trivial changes (3)
- aics-api/src/main/java/kgu/developers/api/comment/presentation/exception/CommentExceptionCode.java
- aics-api/src/main/java/kgu/developers/api/lab/presentation/exception/LabExceptionCode.java
- aics-api/src/main/java/kgu/developers/api/user/presentation/exception/UserIdDuplicateException.java
🔇 Additional comments (15)
aics-domain/src/main/java/kgu/developers/domain/user/exception/InvalidPasswordException.java (1)
1-11: 구현이 깔끔하고 도메인 계층으로의 이동이 적절합니다.
도메인 계층으로 비밀번호 검증 로직을 이동한 것은 좋은 결정입니다. 이는 도메인 주도 설계(DDD) 원칙에 잘 부합합니다.
aics-api/src/main/java/kgu/developers/api/post/presentation/exception/PostExceptionCode.java (2)
3-6: LGTM - 임포트 순서가 개선되었습니다!
정적 임포트를 일반 임포트 전에 배치하여 가독성이 향상되었습니다.
14-14: 에러 메시지가 더 명확해졌습니다!
기존 메시지 "해당 ID의 게시글이 없습니다."에서 "해당 게시글을 찾을 수 없습니다."로 변경된 것이 다음과 같은 이점이 있습니다:
- 기술적인 세부사항(ID)을 제거하여 사용자 친화적으로 변경
- 더 간결하고 직관적인 메시지로 개선
aics-api/src/main/java/kgu/developers/api/file/presentation/exception/FileExceptionCode.java (2)
3-6: 임포트 구문이 개선되었습니다!
와일드카드 임포트를 제거하고 명시적인 임포트를 사용한 것이 좋습니다. 코드의 가독성과 유지보수성이 향상되었습니다.
14-14: 에러 메시지가 더 명확해졌습니다!
"파일 저장에 실패하였습니다"라는 새로운 메시지가 기술적인 의미를 더 정확하게 전달합니다. 이는 사용자와 개발자 모두에게 더 명확한 피드백을 제공할 것입니다.
aics-api/src/main/java/kgu/developers/api/professor/presentation/exception/ProfessorExceptionCode.java (1)
14-14: 에러 메시지 개선이 잘 되었습니다
에러 메시지를 "해당 교수를 찾을 수 없습니다"로 변경한 것이 적절합니다. 이전 메시지에 있던 구현 세부사항(ID)을 제거하고 더 일반적인 메시지로 만든 것이 사용자 친화적입니다.
aics-api/src/main/java/kgu/developers/api/about/presentation/Exception/AboutExceptionCode.java (1)
15-16: 에러 메시지 개선이 잘 이루어졌습니다
CATEGORY_NOT_MATCH: "맞지 않습니다" 에서 "일치하지 않습니다"로 변경하여 더 자연스러운 한국어 표현이 되었습니다.ABOUT_NOT_FOUND: "해당 조건의 소개글을"에서 "해당 소개글을"로 변경하여 더 간결하고 명확해졌습니다.
aics-api/src/main/java/kgu/developers/api/user/presentation/exception/UserExceptionCode.java (2)
16-16: HTTP 상태 코드 변경이 적절합니다.
인증되지 않은 사용자에 대해 401 Unauthorized를 사용하는 것이 HTTP 표준에 더 부합합니다.
Line range hint 1-27: 삭제된 예외 코드의 영향도 확인이 필요합니다.
USER_EMAIL_ID_DUPLICATED와 USER_PHONE_NUMBER_DUPLICATED 예외 코드가 제거되었습니다. 이메일과 전화번호 중복 검사 로직이 다른 곳으로 이동되었는지 확인이 필요합니다.
다음 스크립트로 삭제된 예외 코드의 사용처를 확인해보겠습니다:
aics-api/src/main/java/kgu/developers/api/auth/application/AuthService.java (1)
31-31: 도메인 계층으로의 비밀번호 검증 로직 이동을 승인합니다!
비밀번호 검증 로직을 도메인 계층으로 이동한 것은 도메인 주도 설계(DDD) 원칙에 잘 부합합니다. 이는 User 엔티티의 캡슐화를 강화하고 책임을 더 명확하게 합니다.
aics-api/src/testFixtures/java/auth/application/AuthServiceTest.java (2)
Line range hint 47-85: 테스트 케이스가 잘 구성되어 있습니다.
성공과 실패 케이스를 모두 적절히 검증하고 있으며, 특히 비밀번호 검증 실패 시의 예외 처리를 명확하게 테스트하고 있습니다.
18-18: 도메인 계층으로의 예외 처리 이동이 적절합니다.
비밀번호 검증 로직을 도메인 계층으로 이동한 것은 도메인 주도 설계(DDD) 원칙에 더 잘 부합합니다. 이는 비즈니스 로직의 응집도를 높이고 책임을 명확히 하는 좋은 변경입니다.
다음 스크립트를 실행하여 다른 테스트 클래스들도 새로운 예외 처리 방식을 일관되게 사용하는지 확인하겠습니다:
✅ Verification successful
Let me verify the exception usage patterns to ensure consistency.
예외 처리가 도메인 계층으로 일관되게 이동되었음을 확인했습니다.
검증 결과, 비밀번호 검증 관련 예외 처리가 도메인 계층으로 완전히 이동되었으며, 다음과 같이 일관성 있게 구현되어 있습니다:
- User 도메인에서 비밀번호 검증 로직과 함께 InvalidPasswordException을 직접 발생시킴
- AuthServiceTest와 UserDomainTest 모두 동일한 도메인 예외를 일관되게 사용
- 이전 presentation 계층의 예외 사용이 완전히 제거됨
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: 모든 테스트 파일에서 InvalidPasswordException 임포트 확인
# 이전 패키지의 예외 클래스 사용 여부 확인
rg "import.*api.auth.presentation.exception.InvalidPasswordException"
# 새로운 도메인 패키지의 예외 클래스 사용 여부 확인
rg "import.*domain.user.exception.InvalidPasswordException"
Length of output: 548
Script:
#!/bin/bash
# Check the actual usage of InvalidPasswordException in test files
ast-grep --pattern 'throw $_ InvalidPasswordException($_)'
# Check test cases using InvalidPasswordException
rg "InvalidPasswordException" -B 2 -A 2
Length of output: 5018
aics-domain/src/testFixtures/java/user/domain/UserDomainTest.java (1)
9-10: 비밀번호 검증을 위한 적절한 의존성이 추가되었습니다!
Spring Security의 PasswordEncoder와 BCryptPasswordEncoder를 사용하여 안전한 비밀번호 검증을 구현한 것이 좋습니다.
Also applies to: 17-17
aics-domain/src/main/java/kgu/developers/domain/user/domain/User.java (2)
14-17: Spring Security 관련 import 추가가 적절합니다.
UserDetails 인터페이스 구현과 인증 처리를 위한 필수적인 import들이 잘 추가되었습니다.
29-29: 도메인 계층의 예외 처리 검증이 필요합니다.
비밀번호 검증 로직이 도메인 계층으로 이동됨에 따라, InvalidPasswordException의 처리가 적절한지 확인이 필요합니다.
✅ Verification successful
비밀번호 예외 처리가 적절하게 구현되어 있습니다.
InvalidPasswordException은 도메인 계층에서 다음과 같이 잘 처리되고 있습니다:
- User 도메인에서 비밀번호 불일치 시 예외를 발생시킴
- 테스트 코드를 통해 예외 발생 케이스가 검증됨
- UserDomainTest에서 도메인 계층 테스트
- AuthServiceTest에서 애플리케이션 계층 테스트
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# InvalidPasswordException 처리 검증
rg -A 5 "InvalidPasswordException"
Length of output: 4492
aics-api/src/main/java/kgu/developers/api/about/presentation/Exception/AboutExceptionCode.java
Outdated
Show resolved
Hide resolved
Summary
에러 코드 문서화 작업 및 리팩토링
Tasks
To Reviewer
기존 USER_NOT_AUTHENTICATED 에러 코드는 HTTP 403 Forbidden 상태로 설정되어 있었습니다. 그러나 HTTP 403은 사용자가 인증에 성공했으나, 요청한 리소스에 대한 접근 권한이 없을 때 사용하는 상태 코드입니다. 따라서 해당 에러 코드와 상황에는 적합하지 않다고 판단하였습니다.
이에 USER_NOT_AUTHENTICATED 에러를 유저 인증 실패를 나타내는 HTTP 401 Unauthorized로 수정하였습니다. HTTP 401은 클라이언트가 인증되지 않았거나, 올바르지 않은 인증 정보를 제공했을 때 사용하는 상태 코드로, 현재 상황에 더 적합하다고 생각되어 수정하게 되었습니다.