[volume-1] 회원가입, 내 정보 조회, 비밀번호 수정 구현 #19
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java`:
- Around line 70-96: The code currently NPEs when birthDate is null because
validatePassword calls birthDate.format; update validateRegisterRequest to
explicitly check if birthDate is null and throw new
CoreException(ErrorType.BAD_REQUEST, "생년월일은 필수입니다.") before calling
validatePassword, and add a null guard at the start of validatePassword (e.g.,
if (birthDate == null) throw CoreException(ErrorType.BAD_REQUEST, ...)) to
defensively prevent formatting NPEs; finally add a test in UserServiceTest that
submits a register request with null birthDate and asserts the service
returns/throws a BAD_REQUEST CoreException.
- Around line 46-55: The updatePassword method calls
passwordEncoder.matches(newPassword, user.getPassword()) before validating
newPassword, which can throw IllegalArgumentException on null; move the call to
validatePassword(newPassword, birthDate) to occur immediately after
findUserAndValidatePassword(...) and before any passwordEncoder.matches or
encode calls so null/length checks run first, then perform the matches check and
proceed to passwordEncoder.encode(newPassword); also add a UserServiceTest case
that passes null newPassword to updatePassword and asserts a BAD_REQUEST
(CoreException with ErrorType.BAD_REQUEST) is thrown.
🧹 Nitpick comments (4)
apps/commerce-api/build.gradle.kts (1)
13-15: spring-security-crypto는 Spring Boot 버전 관리 대상이므로 수정 불필요하다.spring-security-crypto는 Spring Boot 3.4.4의 의존성 관리 범위에 포함되어 명시 버전 없이도 일관되게 해석된다. io.spring.dependency-management 플러그인이 적용되어 있어 Spring Boot BOM이 자동으로 로드되므로 환경별 버전 편차 위험은 없다.
h2는 testRuntimeOnly 스코프로 테스트 환경 전용이므로 프로덕션 보안에 미치는 영향이 없으나, 테스트 환경의 일관성을 위해 gradle.properties에서 명시 버전으로 관리하는 것을 권장한다.
apps/commerce-api/src/main/java/com/loopers/config/SecurityConfig.java (1)
10-12: BCrypt strength를 설정으로 외부화하고 운영 환경에 맞게 조정할 것을 권장한다.기본 strength 값 10은 OWASP 권장사항을 만족하나, 프로덕션 환경의 하드웨어 사양과 요구되는 응답 시간에 맞게 튜닝이 필요하다. strength는 log rounds(work factor)로 작동하며, 값이 높아질수록 password 검증에 소요되는 CPU 비용이 지수적으로 증가한다. 따라서 코드에 고정하면 트래픽 패턴이나 서버 사양 변화 시 로그인 지연이 발생하거나 CPU 과부하가 생길 수 있다.
프로퍼티로 strength를 주입하여 환경별로 조정 가능하게 하고, 각 환경에서 password 검증이 약 1초 정도 소요되도록 벤치마크하여 최적값을 설정할 것을 제안한다. 추가로 strength 값이 변경되었을 때 해시 생성이 정상 동작하고 비용이 예상 범위 내인지 검증하는 통합 테스트를 작성하는 것이 좋다.
수정안
+import org.springframework.beans.factory.annotation.Value; ... `@Bean` - public BCryptPasswordEncoder passwordEncoder() { - return new BCryptPasswordEncoder(); - } + public BCryptPasswordEncoder passwordEncoder( + `@Value`("${security.bcrypt.strength:10}") int strength + ) { + return new BCryptPasswordEncoder(strength); + }apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Controller.java (1)
59-66: 비밀번호 변경에서 사용자 조회가 중복된다운영 관점: 같은 요청에서 사용자 조회와 비밀번호 검증이 2회 수행되어 DB 부하와 지연이 누적된다.
수정안: UserService.updatePassword에서 user.getBirthDate()로 검증하도록 시그니처를 단순화하고, 컨트롤러는 getUserInfo 호출을 제거한다.
추가 테스트: UserServiceTest에서 updatePassword가 사용자 birthDate를 사용해 생년월일 포함 검증이 유지되는지 확인하는 케이스를 추가한다.🛠️ 컨트롤러 수정안 예시
`@PutMapping`("/password") public ApiResponse<Void> updatePassword( `@RequestHeader`("X-Loopers-LoginId") String loginId, `@RequestHeader`("X-Loopers-LoginPw") String currentPassword, `@RequestBody` UserV1Dto.UpdatePasswordRequest request ) { - UserInfo userInfo = userFacade.getUserInfo(loginId, currentPassword); - userFacade.updatePassword(loginId, currentPassword, request.newPassword(), userInfo.birthDate()); + userFacade.updatePassword(loginId, currentPassword, request.newPassword(), /* birthDate는 서비스에서 조회 */); return ApiResponse.success(null); }apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceTest.java (1)
43-231: null 입력에 대한 단위 테스트가 빠져 있다운영 관점: null birthDate/email에서 BAD_REQUEST가 아닌 500으로 바뀌어도 테스트가 감지하지 못한다.
수정안: Register 테스트에 birthDate=null, email=null 케이스를 추가한다.
추가 테스트: 각각 BAD_REQUEST와 ErrorType.BAD_REQUEST를 검증하는 테스트를 추가한다.
As per coding guidelines,단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다를 근거로 한다.
Comment on lines
+46
to
+55
| public void updatePassword(String loginId, String currentPassword, String newPassword, LocalDate birthDate) { | ||
| User user = findUserAndValidatePassword(loginId, currentPassword); | ||
|
|
||
| if (passwordEncoder.matches(newPassword, user.getPassword())) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "현재 비밀번호와 동일한 비밀번호는 사용할 수 없습니다."); | ||
| } | ||
|
|
||
| validatePassword(newPassword, birthDate); | ||
|
|
||
| String encryptedPassword = passwordEncoder.encode(newPassword); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find UserService.java and examine validatePassword method
find . -name "UserService.java" -type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 159
🏁 Script executed:
# Search for validatePassword method implementation
rg "validatePassword" -A 10 --type javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3092
🏁 Script executed:
# Check if there are any tests for updatePassword with null password
rg "updatePassword" -A 5 --type java | grep -A 5 "null\|Null"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 749
🌐 Web query:
Spring Security BCryptPasswordEncoder.matches null rawPassword exception behavior
💡 Result:
BCryptPasswordEncoder.matches(rawPassword, encodedPassword) behavior when rawPassword == null:
- It throws
IllegalArgumentExceptionwith message"rawPassword cannot be null"(it does not returnfalse). [2] - If
encodedPasswordisnullor empty, it returnsfalse(and typically logs “Empty encoded password”). [2] - More generally, Spring Security’s
PasswordEncoder.matches(...)contract says it is nevertruewhen either argument isnull(or empty). [1]
Sources
- Spring Security
PasswordEncoderJavadoc (matchesnull/empty contract). [1] BCryptPasswordEncoderimplementation (shows explicitrawPassword == null→IllegalArgumentException, andencodedPasswordnull/empty →false). [2]
validatePassword를 matches 호출 전에 수행하여 null 검증을 먼저 진행해야 한다
newPassword가 null인 경우 BCryptPasswordEncoder.matches에서 IllegalArgumentException이 발생하여 500 에러로 응답되고 운영 장애로 이어질 수 있다. validatePassword 메서드에서 null과 길이를 검증하고 있으므로, 이를 matches 호출 전에 수행하여 BAD_REQUEST로 처리해야 한다.
수정안 예시
`@Transactional`
public void updatePassword(String loginId, String currentPassword, String newPassword, LocalDate birthDate) {
+ validatePassword(newPassword, birthDate);
User user = findUserAndValidatePassword(loginId, currentPassword);
if (passwordEncoder.matches(newPassword, user.getPassword())) {
throw new CoreException(ErrorType.BAD_REQUEST, "현재 비밀번호와 동일한 비밀번호는 사용할 수 없습니다.");
}
-
- validatePassword(newPassword, birthDate);
String encryptedPassword = passwordEncoder.encode(newPassword);
user.updatePassword(encryptedPassword);
}추가로 UserServiceTest에 newPassword가 null인 경우 BAD_REQUEST로 반환되는 테스트 케이스를 작성해야 한다.
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java`
around lines 46 - 55, The updatePassword method calls
passwordEncoder.matches(newPassword, user.getPassword()) before validating
newPassword, which can throw IllegalArgumentException on null; move the call to
validatePassword(newPassword, birthDate) to occur immediately after
findUserAndValidatePassword(...) and before any passwordEncoder.matches or
encode calls so null/length checks run first, then perform the matches check and
proceed to passwordEncoder.encode(newPassword); also add a UserServiceTest case
that passes null newPassword to updatePassword and asserts a BAD_REQUEST
(CoreException with ErrorType.BAD_REQUEST) is thrown.
Comment on lines
70
to
96
| private void validateRegisterRequest(String loginId, String password, String name, LocalDate birthDate, String email) { | ||
| if (loginId == null || loginId.isBlank()) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "로그인 ID는 필수입니다."); | ||
| } | ||
|
|
||
| if (name == null || name.isBlank()) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "이름은 필수입니다."); | ||
| } | ||
|
|
||
| validatePassword(password, birthDate); | ||
| validateEmail(email); | ||
| } | ||
|
|
||
| private void validatePassword(String password, LocalDate birthDate) { | ||
| if (password == null || password.length() < 8 || password.length() > 16) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 8~16자여야 합니다."); | ||
| } | ||
|
|
||
| if (!PASSWORD_PATTERN.matcher(password).matches()) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 영문 대소문자, 숫자, 특수문자만 가능합니다."); | ||
| } | ||
|
|
||
| String yyyyMMdd = birthDate.format(YYYY_MM_DD_FORMATTER); | ||
| String yyMMdd = birthDate.format(YY_MM_DD_FORMATTER); | ||
|
|
||
| if (password.contains(yyyyMMdd) || password.contains(yyMMdd)) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호에 생년월일을 포함할 수 없습니다."); |
There was a problem hiding this comment.
생년월일 null 입력 시 500으로 실패한다
운영 관점: birthDate가 null이면 format 호출에서 NPE가 발생해 500으로 떨어지고 ApiResponse 포맷이 깨진다.
수정안: validateRegisterRequest에서 birthDate null을 BAD_REQUEST로 처리하고 validatePassword에서도 null 방어를 추가한다.
추가 테스트: UserServiceTest에 birthDate가 null인 회원가입 요청이 BAD_REQUEST로 반환되는 케이스를 추가한다.
🛠️ 수정안 예시
private void validateRegisterRequest(String loginId, String password, String name, LocalDate birthDate, String email) {
if (loginId == null || loginId.isBlank()) {
throw new CoreException(ErrorType.BAD_REQUEST, "로그인 ID는 필수입니다.");
}
if (name == null || name.isBlank()) {
throw new CoreException(ErrorType.BAD_REQUEST, "이름은 필수입니다.");
}
+ if (birthDate == null) {
+ throw new CoreException(ErrorType.BAD_REQUEST, "생년월일은 필수입니다.");
+ }
validatePassword(password, birthDate);
validateEmail(email);
}
private void validatePassword(String password, LocalDate birthDate) {
+ if (birthDate == null) {
+ throw new CoreException(ErrorType.BAD_REQUEST, "생년월일은 필수입니다.");
+ }
if (password == null || password.length() < 8 || password.length() > 16) {
throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 8~16자여야 합니다.");
}🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java`
around lines 70 - 96, The code currently NPEs when birthDate is null because
validatePassword calls birthDate.format; update validateRegisterRequest to
explicitly check if birthDate is null and throw new
CoreException(ErrorType.BAD_REQUEST, "생년월일은 필수입니다.") before calling
validatePassword, and add a null guard at the start of validatePassword (e.g.,
if (birthDate == null) throw CoreException(ErrorType.BAD_REQUEST, ...)) to
defensively prevent formatting NPEs; finally add a test in UserServiceTest that
submits a register request with null birthDate and asserts the service
returns/throws a BAD_REQUEST CoreException.
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/User.java`:
- Around line 45-49: The entity timestamps (createdAt/updatedAt) in User.java
are set using ZonedDateTime.now() without an explicit UTC ZoneId; update the
`@PrePersist` and `@PreUpdate` logic to use ZonedDateTime.now(ZoneId.of("UTC")) (or
Instant.now().atZone(ZoneId.of("UTC"))) when assigning createdAt/updatedAt, and
move this logic into a shared BaseEntity (e.g., methods annotated `@PrePersist`
and `@PreUpdate` in BaseEntity) so User reuses that central implementation to
avoid duplication and ensure UTC normalization.
- Around line 17-78: Add JPA identifier-based equals/hashCode to User: implement
equals(Object) to return true for same instance, false for null/different class,
and compare id fields only when id != null (i.e., id.equals(other.id));
implement hashCode() to return getClass().hashCode(). Update the User class by
adding these two methods (referencing the User class and its id field) and
include a unit test verifying equality behavior between transient (id == null)
and persisted (id != null) instances.
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java`:
- Around line 59-68: In findUserAndValidatePassword(String loginId, String
password) add a null check for the password parameter before calling
passwordEncoder.matches: if password is null (or blank, per policy) throw new
CoreException(ErrorType.BAD_REQUEST, "비밀번호가 누락되었습니다." or similar) so that
passwordEncoder.matches(password, user.getPassword()) is never called with a
null value; update the method to perform this validation and ensure tests in
UserServiceTest include a case where password is null and assert a BAD_REQUEST
CoreException is thrown.
In
`@apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceTest.java`:
- Around line 39-232: Add a test in the Register nested class to cover null
birthDate to prevent NPE from validatePassword's birthDate.format() call: add a
new test method (e.g., throwsBadRequest_whenBirthDateIsNull) that calls
userService.register("testuser","Password1!","홍길동", null, "test@example.com")
and asserts a CoreException with ErrorType.BAD_REQUEST; additionally, update
UserService.register / validatePassword to null-check birthDate (or guard before
calling birthDate.format()) so validatePassword does not call birthDate.format()
when birthDate is null.
- Around line 302-466: The tests are missing null-parameter coverage and the
service lacks null checks: ensure UserService.updatePassword validates
newPassword and birthDate (and any inputs used by validatePassword) before
calling passwordEncoder.matches or validatePassword to avoid
IllegalArgumentException/NPE; add early guard clauses in updatePassword (or
inside validatePassword) that throw CoreException with ErrorType.BAD_REQUEST
when newPassword == null or birthDate == null, and update unit tests (tests
shown reference updatePassword) to assert these BAD_REQUEST behaviors.
- Around line 234-300: Add a test in the GetUserInfo nested test class to cover
the null password path: ensure when userRepository.findByLoginId(...) returns a
User and userService.getUserInfo(loginId, null) is called it throws
CoreException with ErrorType.BAD_REQUEST; mock behavior so
passwordEncoder.matches is not invoked (verify never called) and name the test
method throwsBadRequest_whenPasswordIsNull to mirror existing tests and
reference userService.getUserInfo, userRepository.findByLoginId, and
passwordEncoder.matches.
In
`@apps/commerce-api/src/test/java/com/loopers/interfaces/api/UserV1ApiE2ETest.java`:
- Around line 55-282: Add an E2E test that submits a register request with
birthDate == null and asserts HttpStatus.BAD_REQUEST; specifically add a new
test method (e.g., returnsBadRequest_whenBirthDateIsNull) inside the Register
nested class in UserV1ApiE2ETest that constructs a UserV1Dto.RegisterRequest
with null birthDate, uses the existing REGISTER_ENDPOINT,
ParameterizedTypeReference<ApiResponse<UserV1Dto.RegisterResponse>>, and
testRestTemplate.exchange(...) to post the request, and asserts
response.getStatusCode() == HttpStatus.BAD_REQUEST; after adding the test,
ensure UserService.validatePassword (or the null-check logic around birthDate)
is updated to handle null birthDate to avoid NPE so the test can pass.
🧹 Nitpick comments (5)
apps/commerce-api/src/main/java/com/loopers/config/SecurityConfig.java (1)
10-13: 반환 타입을 PasswordEncoder 인터페이스로 변경하는 것을 권장한다.현재 구체 클래스 BCryptPasswordEncoder를 반환하고 있다. 인터페이스 타입을 반환하면 향후 알고리즘 변경(예: Argon2) 시 클라이언트 코드 수정 없이 교체가 가능하고, 테스트 시 Mock 주입이 용이해진다.
♻️ 인터페이스 타입 반환 제안
import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; +import org.springframework.security.crypto.password.PasswordEncoder; `@Configuration` public class SecurityConfig { `@Bean` - public BCryptPasswordEncoder passwordEncoder() { + public PasswordEncoder passwordEncoder() { return new BCryptPasswordEncoder(); } }apps/commerce-api/src/main/java/com/loopers/domain/user/User.java (1)
53-61: create() 팩토리 메서드에서 null 입력에 대한 방어가 없다.PR 설명에 따르면 도메인 레이어에서 검증을 수행한다고 했으나, User.create() 메서드는 null 값을 그대로 할당한다. UserService에서 검증이 수행된다 하더라도, 엔티티 생성 시점에 최소한의 불변 조건(invariant)을 보장하는 것이 방어적 프로그래밍 관점에서 안전하다.
다만 retrieved learning에 따르면 password는 BCrypt 해시만 받도록 설계되어 있어 생성자에서 검증하지 않는다. 이 설계 결정이 의도적이라면 Javadoc으로 계약을 명시하는 것을 권장한다.
📝 Javadoc으로 계약 명시 제안
+ /** + * User 엔티티를 생성한다. + * + * `@param` loginId 로그인 ID (UserService에서 중복 검증 완료) + * `@param` password BCrypt 해시 (평문 비밀번호 전달 금지, UserService에서 인코딩 완료) + * `@param` name 사용자 이름 + * `@param` birthDate 생년월일 + * `@param` email 이메일 (UserService에서 검증 완료) + * `@return` 새로운 User 인스턴스 + */ public static User create(String loginId, String password, String name, LocalDate birthDate, String email) {apps/commerce-api/src/main/java/com/loopers/application/user/UserFacade.java (1)
10-28: 사용자 관련 주요 작업에 대한 감사 로깅이 없다.회원가입, 정보 조회, 비밀번호 변경은 보안 및 컴플라이언스 관점에서 중요한 이벤트이다. 운영 환경에서 다음 상황 대응을 위해 감사 로그를 남기는 것을 권장한다:
- 비정상적인 계정 활동 탐지
- 보안 사고 발생 시 추적
- 규정 준수 증빙
단, 로깅 시 password, birthDate 등 민감 정보는 반드시 마스킹해야 한다.
📋 감사 로깅 추가 제안
+import org.slf4j.Logger; +import org.slf4j.LoggerFactory; `@Component` `@RequiredArgsConstructor` public class UserFacade { + private static final Logger log = LoggerFactory.getLogger(UserFacade.class); private final UserService userService; public UserInfo register(String loginId, String password, String name, LocalDate birthDate, String email) { User user = userService.register(loginId, password, name, birthDate, email); + log.info("User registered: loginId={}", loginId); return UserInfo.from(user); } public UserInfo getUserInfo(String loginId, String password) { User user = userService.getUserInfo(loginId, password); + log.debug("User info retrieved: loginId={}", loginId); return UserInfo.from(user); } public void updatePassword(String loginId, String currentPassword, String newPassword, LocalDate birthDate) { userService.updatePassword(loginId, currentPassword, newPassword, birthDate); + log.info("Password updated: loginId={}", loginId); } }apps/commerce-api/src/main/java/com/loopers/application/user/UserInfo.java (1)
14-22:from(User user)메서드에 null 방어 로직이 누락되었다운영 환경에서
user가 null로 전달되면user.getId()호출 시 NPE가 발생하고, 스택 트레이스만으로는 원인 파악이 어렵다. Facade 계층에서 호출 시 예기치 않은 null이 전달될 가능성이 있으므로 방어 코드를 추가하는 것이 안전하다.수정안
public static UserInfo from(User user) { + if (user == null) { + throw new IllegalArgumentException("User cannot be null"); + } return new UserInfo( user.getId(), user.getLoginId(),apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Controller.java (1)
59-69: 중복 인증으로 인한 불필요한 DB 조회와 비밀번호 검증이 발생한다운영 관점:
getUserInfo와updatePassword가 각각 사용자 조회 및 비밀번호 검증을 수행하므로, 한 번의 비밀번호 변경 요청에 DB 조회 2회 + 비밀번호 해시 비교 2회가 발생한다. 트래픽이 증가하면 불필요한 부하로 이어진다.수정안 1: UserFacade에서 birthDate를 포함한 updatePassword 오버로드 제공
- UserInfo userInfo = userFacade.getUserInfo(loginId, currentPassword); - userFacade.updatePassword(loginId, currentPassword, request.newPassword(), userInfo.birthDate()); + userFacade.updatePassword(loginId, currentPassword, request.newPassword());UserFacade.updatePassword에서 내부적으로 사용자 조회 시 birthDate를 함께 가져오도록 수정한다.
수정안 2: birthDate를 클라이언트 요청에 포함
UpdatePasswordRequest에 birthDate 필드를 추가하여 클라이언트가 전달하도록 변경한다.
Comment on lines
17
to
78
| @Entity | ||
| @Table(name = "users", indexes = { | ||
| @Index(name = "idx_login_id", columnList = "login_id"), | ||
| @Index(name = "idx_email", columnList = "email") | ||
| }) | ||
| @Getter | ||
| public class User { | ||
|
|
||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| @Column(name = "user_id") | ||
| private Long id; | ||
|
|
||
| @Column(name = "login_id", length = 50, nullable = false, unique = true) | ||
| private String loginId; | ||
|
|
||
| @Column(name = "password", length = 255, nullable = false) | ||
| private String password; | ||
|
|
||
| @Column(name = "name", length = 100, nullable = false) | ||
| private String name; | ||
|
|
||
| @Column(name = "birth_date", nullable = false) | ||
| private LocalDate birthDate; | ||
|
|
||
| @Column(name = "email", length = 255, nullable = false) | ||
| private String email; | ||
|
|
||
| @Column(name = "created_at", nullable = false, updatable = false) | ||
| private ZonedDateTime createdAt; | ||
|
|
||
| @Column(name = "updated_at", nullable = false) | ||
| private ZonedDateTime updatedAt; | ||
|
|
||
| protected User() {} | ||
|
|
||
| public static User create(String loginId, String password, String name, LocalDate birthDate, String email) { | ||
| User user = new User(); | ||
| user.loginId = loginId; | ||
| user.password = password; | ||
| user.name = name; | ||
| user.birthDate = birthDate; | ||
| user.email = email; | ||
| return user; | ||
| } | ||
|
|
||
| @PrePersist | ||
| private void prePersist() { | ||
| ZonedDateTime now = ZonedDateTime.now(); | ||
| this.createdAt = now; | ||
| this.updatedAt = now; | ||
| } | ||
|
|
||
| @PreUpdate | ||
| private void preUpdate() { | ||
| this.updatedAt = ZonedDateTime.now(); | ||
| } | ||
|
|
||
| public void updatePassword(String newPassword) { | ||
| this.password = newPassword; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/commerce-api/src/main/java/com/loopers/domain/user/User.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2752
🏁 Script executed:
# Find all `@Entity` classes in the domain package
fd -e java --path '*/domain/**' . --exec grep -l '@Entity' {} \;Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 318
🏁 Script executed:
# Check for equals/hashCode implementations in domain entities
rg -n "public boolean equals|public int hashCode" --type java -g '**/domain/**/*.java' -B2Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# Find all Java files in domain package
find apps/commerce-api/src/main/java -path '*/domain/**' -name '*.java' -type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 299
🏁 Script executed:
# Check for `@EqualsAndHashCode` Lombok annotation usage
rg -n "@EqualsAndHashCode" --type javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# Check if User entity is used in collections or as cache keys
rg -n "Set<User|Map.*User|HashSet.*User|HashMap.*User" --type javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
cat -n apps/commerce-api/src/main/java/com/loopers/domain/user/UserRepository.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 366
🏁 Script executed:
cat -n apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 5026
🏁 Script executed:
# Check for any L2 cache configuration or EntityManager.merge() usage
rg -n "merge|cache|Cache|CACHE" --type java apps/commerce-api/src/main/javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
JPA 엔티티 식별자 기반 equals/hashCode 구현 추가 권장한다.
User 엔티티는 현재 equals/hashCode가 구현되지 않았다. 현재 코드에서는 개별 엔티티 조회/저장 패턴만 사용하므로 즉각적인 운영 문제는 없으나, 향후 다음과 같은 상황에서 예상치 못한 동작이 발생할 수 있다:
- 엔티티를 Set이나 Map에 저장하거나 컬렉션에서 검색할 경우
- 캐싱 또는 second-level cache 도입 시
- EntityManager.merge() 사용 시
데이터베이스 식별자 기반으로 equals/hashCode를 구현하면 장기적 운영 안정성을 확보할 수 있다:
`@Override`
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
User user = (User) o;
return id != null && id.equals(user.id);
}
`@Override`
public int hashCode() {
return getClass().hashCode();
}추가로 저장되지 않은 엔티티(id=null)와 저장된 엔티티 간 동치성 비교를 검증하는 단위 테스트를 작성한다.
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/User.java` around
lines 17 - 78, Add JPA identifier-based equals/hashCode to User: implement
equals(Object) to return true for same instance, false for null/different class,
and compare id fields only when id != null (i.e., id.equals(other.id));
implement hashCode() to return getClass().hashCode(). Update the User class by
adding these two methods (referencing the User class and its id field) and
include a unit test verifying equality behavior between transient (id == null)
and persisted (id != null) instances.
Comment on lines
45
to
49
| @Column(name = "created_at", nullable = false, updatable = false) | ||
| private ZonedDateTime createdAt; | ||
|
|
||
| @Column(name = "updated_at", nullable = false) | ||
| private ZonedDateTime updatedAt; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for JPA/Hibernate timezone configuration
echo "=== Checking JPA/Hibernate timezone configuration ==="
find . -type f \( -name "*.yaml" -o -name "*.yml" -o -name "*.properties" \) -exec grep -l "timezone\|time_zone\|spring.jpa" {} \;
echo ""
echo "=== Searching for JPA config content ==="
rg -n "hibernate.jdbc.time_zone|spring.jpa.properties.hibernate" --type yaml --type properties -A 2 -B 2 2>/dev/null || echo "No timezone config found"
echo ""
echo "=== Searching for ZonedDateTime usage and converters ==="
rg -n "ZonedDateTime" --type java -B 2 -A 2
echo ""
echo "=== Checking for AttributeConverter implementations ==="
rg -n "AttributeConverter|@Converter" --type java -B 1 -A 3Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 5206
🏁 Script executed:
#!/bin/bash
# Search for application configuration files
echo "=== Looking for application configuration ==="
find . -type f \( -name "application*.yml" -o -name "application*.properties" \) -exec echo "File: {}" \; -exec cat {} \;
echo ""
echo "=== Checking test configuration ==="
find . -path "*/test/resources/*" -type f \( -name "application*.yml" -o -name "application*.properties" \) -exec echo "File: {}" \; -exec cat {} \;
echo ""
echo "=== Checking jpa.yml content ==="
cat ./modules/jpa/src/main/resources/jpa.yml 2>/dev/null || echo "File not found"
echo ""
echo "=== Checking for any timezone-related code or comments ==="
rg -i "utc|timezone|zone|time.zone" --type java -B 1 -A 1 | head -50
echo ""
echo "=== Verify User.java extends BaseEntity or has separate lifecycle ==="
cat ./apps/commerce-api/src/main/java/com/loopers/domain/user/User.java | head -60Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 10918
🏁 Script executed:
#!/bin/bash
# Check if test profile uses H2 or MySQL
echo "=== Checking test profile datasource ==="
rg -n "h2|H2|test-datasource" --type java --type yaml
echo ""
echo "=== Verify if User extends BaseEntity ==="
rg -n "extends.*BaseEntity|class User" apps/commerce-api/src/main/java/com/loopers/domain/user/User.java
echo ""
echo "=== Check for explicit UTC usage in ZonedDateTime calls ==="
rg -n "ZonedDateTime.now\(|ZoneId\|UTC" --type java -A 1 | head -40Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1246
ZonedDateTime.now() 호출 시 명시적 UTC 타임존 지정이 필요하다.
jpa.yml에 jdbc.time_zone: UTC와 timezone.default_storage: NORMALIZE_UTC로 UTC 저장 전략이 이미 선언되어 있음에도, 코드에서 ZonedDateTime.now()를 명시적 타임존 없이 호출하고 있다. 이는 JVM 시스템 기본 타임존(컨테이너 환경에서 변동 가능)을 사용하게 되어 설정된 UTC 전략과 일관성이 맞지 않는다.
운영 환경에서 타임존 불일치로 인한 데이터 저장 오류를 방지하려면:
ZonedDateTime.now(ZoneId.of("UTC"))또는Instant.now().atZone(ZoneId.of("UTC"))로 변경- User.java의
@PrePersist/@PreUpdate 로직을 BaseEntity로 통합하여 중복 제거
수정 예시
`@PrePersist`
private void prePersist() {
ZonedDateTime now = ZonedDateTime.now(ZoneId.of("UTC"));
this.createdAt = now;
this.updatedAt = now;
}
`@PreUpdate`
private void preUpdate() {
this.updatedAt = ZonedDateTime.now(ZoneId.of("UTC"));
}🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/User.java` around
lines 45 - 49, The entity timestamps (createdAt/updatedAt) in User.java are set
using ZonedDateTime.now() without an explicit UTC ZoneId; update the `@PrePersist`
and `@PreUpdate` logic to use ZonedDateTime.now(ZoneId.of("UTC")) (or
Instant.now().atZone(ZoneId.of("UTC"))) when assigning createdAt/updatedAt, and
move this logic into a shared BaseEntity (e.g., methods annotated `@PrePersist`
and `@PreUpdate` in BaseEntity) so User reuses that central implementation to
avoid duplication and ensure UTC normalization.
Comment on lines
+59
to
+68
| private User findUserAndValidatePassword(String loginId, String password) { | ||
| User user = userRepository.findByLoginId(loginId) | ||
| .orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, "사용자를 찾을 수 없습니다.")); | ||
|
|
||
| if (!passwordEncoder.matches(password, user.getPassword())) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호가 일치하지 않습니다."); | ||
| } | ||
|
|
||
| return user; | ||
| } |
There was a problem hiding this comment.
password 파라미터 null 검증이 누락되었다
운영 관점: password가 null인 경우 passwordEncoder.matches(password, ...) 호출 시 IllegalArgumentException이 발생하여 500 Internal Server Error로 응답된다. 헤더 X-Loopers-LoginPw가 누락되거나 null로 전달될 수 있으므로, matches 호출 전에 null 검증이 필요하다.
수정안
private User findUserAndValidatePassword(String loginId, String password) {
+ if (password == null) {
+ throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 필수입니다.");
+ }
User user = userRepository.findByLoginId(loginId)
.orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, "사용자를 찾을 수 없습니다."));추가 테스트: UserServiceTest에 password가 null인 경우 BAD_REQUEST로 반환되는 테스트 케이스를 작성해야 한다.
📝 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.
Suggested change
| private User findUserAndValidatePassword(String loginId, String password) { | |
| User user = userRepository.findByLoginId(loginId) | |
| .orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, "사용자를 찾을 수 없습니다.")); | |
| if (!passwordEncoder.matches(password, user.getPassword())) { | |
| throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호가 일치하지 않습니다."); | |
| } | |
| return user; | |
| } | |
| private User findUserAndValidatePassword(String loginId, String password) { | |
| if (password == null) { | |
| throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 필수입니다."); | |
| } | |
| User user = userRepository.findByLoginId(loginId) | |
| .orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, "사용자를 찾을 수 없습니다.")); | |
| if (!passwordEncoder.matches(password, user.getPassword())) { | |
| throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호가 일치하지 않습니다."); | |
| } | |
| return user; | |
| } |
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java`
around lines 59 - 68, In findUserAndValidatePassword(String loginId, String
password) add a null check for the password parameter before calling
passwordEncoder.matches: if password is null (or blank, per policy) throw new
CoreException(ErrorType.BAD_REQUEST, "비밀번호가 누락되었습니다." or similar) so that
passwordEncoder.matches(password, user.getPassword()) is never called with a
null value; update the method to perform this validation and ensure tests in
UserServiceTest include a case where password is null and assert a BAD_REQUEST
CoreException is thrown.
Comment on lines
39
to
232
| @Nested | ||
| @DisplayName("회원가입") | ||
| class Register { | ||
|
|
||
| @Test | ||
| @DisplayName("유효한 정보로 회원가입하면 성공한다") | ||
| void success() { | ||
| // given | ||
| String loginId = "testuser"; | ||
| String password = "Password1!"; | ||
| String name = "홍길동"; | ||
| LocalDate birthDate = LocalDate.of(1990, 1, 15); | ||
| String email = "test@example.com"; | ||
|
|
||
| when(userRepository.existsByLoginId(loginId)).thenReturn(false); | ||
| when(passwordEncoder.encode(password)).thenReturn("$2a$10$encryptedPassword"); | ||
| when(userRepository.save(any(User.class))).thenAnswer(invocation -> invocation.getArgument(0)); | ||
|
|
||
| // when | ||
| User result = userService.register(loginId, password, name, birthDate, email); | ||
|
|
||
| // then | ||
| assertThat(result).isNotNull(); | ||
| verify(userRepository).existsByLoginId(loginId); | ||
| verify(passwordEncoder).encode(password); | ||
| verify(userRepository).save(any(User.class)); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("이미 존재하는 로그인 ID로 회원가입하면 CONFLICT 예외가 발생한다") | ||
| void throwsConflict_whenLoginIdExists() { | ||
| // given | ||
| String loginId = "existinguser"; | ||
| when(userRepository.existsByLoginId(loginId)).thenReturn(true); | ||
|
|
||
| // when & then | ||
| assertThatThrownBy(() -> userService.register( | ||
| loginId, | ||
| "Password1!", | ||
| "홍길동", | ||
| LocalDate.of(1990, 1, 15), | ||
| "test@example.com" | ||
| )) | ||
| .isInstanceOf(CoreException.class) | ||
| .hasFieldOrPropertyWithValue("errorType", ErrorType.CONFLICT); | ||
|
|
||
| verify(userRepository).existsByLoginId(loginId); | ||
| verify(userRepository, never()).save(any(User.class)); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("로그인 ID가 null이면 BAD_REQUEST 예외가 발생한다") | ||
| void throwsBadRequest_whenLoginIdIsNull() { | ||
| // when & then | ||
| assertThatThrownBy(() -> userService.register( | ||
| null, | ||
| "Password1!", | ||
| "홍길동", | ||
| LocalDate.of(1990, 1, 15), | ||
| "test@example.com" | ||
| )) | ||
| .isInstanceOf(CoreException.class) | ||
| .hasFieldOrPropertyWithValue("errorType", ErrorType.BAD_REQUEST); | ||
|
|
||
| verify(userRepository, never()).existsByLoginId(anyString()); | ||
| verify(userRepository, never()).save(any(User.class)); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("로그인 ID가 빈 문자열이면 BAD_REQUEST 예외가 발생한다") | ||
| void throwsBadRequest_whenLoginIdIsEmpty() { | ||
| // when & then | ||
| assertThatThrownBy(() -> userService.register( | ||
| "", | ||
| "Password1!", | ||
| "홍길동", | ||
| LocalDate.of(1990, 1, 15), | ||
| "test@example.com" | ||
| )) | ||
| .isInstanceOf(CoreException.class) | ||
| .hasFieldOrPropertyWithValue("errorType", ErrorType.BAD_REQUEST); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("이름이 null이면 BAD_REQUEST 예외가 발생한다") | ||
| void throwsBadRequest_whenNameIsNull() { | ||
| // when & then | ||
| assertThatThrownBy(() -> userService.register( | ||
| "testuser", | ||
| "Password1!", | ||
| null, | ||
| LocalDate.of(1990, 1, 15), | ||
| "test@example.com" | ||
| )) | ||
| .isInstanceOf(CoreException.class) | ||
| .hasFieldOrPropertyWithValue("errorType", ErrorType.BAD_REQUEST); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("비밀번호가 8자 미만이면 BAD_REQUEST 예외가 발생한다") | ||
| void throwsBadRequest_whenPasswordTooShort() { | ||
| // when & then | ||
| assertThatThrownBy(() -> userService.register( | ||
| "testuser", | ||
| "Pass1!", | ||
| "홍길동", | ||
| LocalDate.of(1990, 1, 15), | ||
| "test@example.com" | ||
| )) | ||
| .isInstanceOf(CoreException.class) | ||
| .hasFieldOrPropertyWithValue("errorType", ErrorType.BAD_REQUEST) | ||
| .hasMessageContaining("8~16자"); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("비밀번호가 16자 초과이면 BAD_REQUEST 예외가 발생한다") | ||
| void throwsBadRequest_whenPasswordTooLong() { | ||
| // when & then | ||
| assertThatThrownBy(() -> userService.register( | ||
| "testuser", | ||
| "Password1!Password1!", | ||
| "홍길동", | ||
| LocalDate.of(1990, 1, 15), | ||
| "test@example.com" | ||
| )) | ||
| .isInstanceOf(CoreException.class) | ||
| .hasFieldOrPropertyWithValue("errorType", ErrorType.BAD_REQUEST) | ||
| .hasMessageContaining("8~16자"); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("비밀번호에 공백이 포함되면 BAD_REQUEST 예외가 발생한다") | ||
| void throwsBadRequest_whenPasswordContainsSpace() { | ||
| // when & then | ||
| assertThatThrownBy(() -> userService.register( | ||
| "testuser", | ||
| "Pass word1!", | ||
| "홍길동", | ||
| LocalDate.of(1990, 1, 15), | ||
| "test@example.com" | ||
| )) | ||
| .isInstanceOf(CoreException.class) | ||
| .hasFieldOrPropertyWithValue("errorType", ErrorType.BAD_REQUEST) | ||
| .hasMessageContaining("영문 대소문자, 숫자, 특수문자"); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("비밀번호에 생년월일(yyyyMMdd)이 포함되면 BAD_REQUEST 예외가 발생한다") | ||
| void throwsBadRequest_whenPasswordContainsBirthDateYYYYMMDD() { | ||
| // when & then | ||
| assertThatThrownBy(() -> userService.register( | ||
| "testuser", | ||
| "19900115Pw!", | ||
| "홍길동", | ||
| LocalDate.of(1990, 1, 15), | ||
| "test@example.com" | ||
| )) | ||
| .isInstanceOf(CoreException.class) | ||
| .hasFieldOrPropertyWithValue("errorType", ErrorType.BAD_REQUEST) | ||
| .hasMessageContaining("생년월일"); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("비밀번호에 생년월일(yyMMdd)이 포함되면 BAD_REQUEST 예외가 발생한다") | ||
| void throwsBadRequest_whenPasswordContainsBirthDateYYMMDD() { | ||
| // when & then | ||
| assertThatThrownBy(() -> userService.register( | ||
| "testuser", | ||
| "900115Pass!", | ||
| "홍길동", | ||
| LocalDate.of(1990, 1, 15), | ||
| "test@example.com" | ||
| )) | ||
| .isInstanceOf(CoreException.class) | ||
| .hasFieldOrPropertyWithValue("errorType", ErrorType.BAD_REQUEST) | ||
| .hasMessageContaining("생년월일"); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("이메일 형식이 올바르지 않으면 BAD_REQUEST 예외가 발생한다") | ||
| void throwsBadRequest_whenEmailFormatInvalid() { | ||
| // when & then | ||
| assertThatThrownBy(() -> userService.register( | ||
| "testuser", | ||
| "Password1!", | ||
| "홍길동", | ||
| LocalDate.of(1990, 1, 15), | ||
| "invalid-email" | ||
| )) | ||
| .isInstanceOf(CoreException.class) | ||
| .hasFieldOrPropertyWithValue("errorType", ErrorType.BAD_REQUEST) | ||
| .hasMessageContaining("이메일"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Register에서 birthDate null 테스트 케이스가 누락되었다
운영 관점: birthDate가 null인 경우 validatePassword 메서드 내 birthDate.format() 호출에서 NPE가 발생한다.
추가 테스트 예시
`@Test`
`@DisplayName`("생년월일이 null이면 BAD_REQUEST 예외가 발생한다")
void throwsBadRequest_whenBirthDateIsNull() {
// when & then
assertThatThrownBy(() -> userService.register(
"testuser",
"Password1!",
"홍길동",
null, // birthDate가 null
"test@example.com"
))
.isInstanceOf(CoreException.class)
.hasFieldOrPropertyWithValue("errorType", ErrorType.BAD_REQUEST);
}🤖 Prompt for AI Agents
In `@apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceTest.java`
around lines 39 - 232, Add a test in the Register nested class to cover null
birthDate to prevent NPE from validatePassword's birthDate.format() call: add a
new test method (e.g., throwsBadRequest_whenBirthDateIsNull) that calls
userService.register("testuser","Password1!","홍길동", null, "test@example.com")
and asserts a CoreException with ErrorType.BAD_REQUEST; additionally, update
UserService.register / validatePassword to null-check birthDate (or guard before
calling birthDate.format()) so validatePassword does not call birthDate.format()
when birthDate is null.
Comment on lines
+234
to
+300
| @Nested | ||
| @DisplayName("내 정보 조회") | ||
| class GetUserInfo { | ||
|
|
||
| @Test | ||
| @DisplayName("유효한 로그인 ID와 비밀번호로 사용자를 조회하면 성공한다") | ||
| void success() { | ||
| // given | ||
| String loginId = "testuser"; | ||
| String rawPassword = "Password1!"; | ||
| String encodedPassword = "$2a$10$encryptedPassword"; | ||
|
|
||
| User user = User.create(loginId, encodedPassword, "홍길동", LocalDate.of(1990, 1, 15), "test@example.com"); | ||
|
|
||
| when(userRepository.findByLoginId(loginId)).thenReturn(Optional.of(user)); | ||
| when(passwordEncoder.matches(rawPassword, encodedPassword)).thenReturn(true); | ||
|
|
||
| // when | ||
| User result = userService.getUserInfo(loginId, rawPassword); | ||
|
|
||
| // then | ||
| assertThat(result).isNotNull(); | ||
| assertThat(result.getLoginId()).isEqualTo(loginId); | ||
| verify(userRepository).findByLoginId(loginId); | ||
| verify(passwordEncoder).matches(rawPassword, encodedPassword); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("존재하지 않는 로그인 ID로 조회하면 NOT_FOUND 예외가 발생한다") | ||
| void throwsNotFound_whenUserNotExists() { | ||
| // given | ||
| String loginId = "nonexistent"; | ||
| when(userRepository.findByLoginId(loginId)).thenReturn(Optional.empty()); | ||
|
|
||
| // when & then | ||
| assertThatThrownBy(() -> userService.getUserInfo(loginId, "Password1!")) | ||
| .isInstanceOf(CoreException.class) | ||
| .hasFieldOrPropertyWithValue("errorType", ErrorType.NOT_FOUND) | ||
| .hasMessageContaining("사용자를 찾을 수 없습니다"); | ||
|
|
||
| verify(userRepository).findByLoginId(loginId); | ||
| verify(passwordEncoder, never()).matches(anyString(), anyString()); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("비밀번호가 일치하지 않으면 BAD_REQUEST 예외가 발생한다") | ||
| void throwsBadRequest_whenPasswordNotMatch() { | ||
| // given | ||
| String loginId = "testuser"; | ||
| String rawPassword = "WrongPassword1!"; | ||
| String encodedPassword = "$2a$10$encryptedPassword"; | ||
|
|
||
| User user = User.create(loginId, encodedPassword, "홍길동", LocalDate.of(1990, 1, 15), "test@example.com"); | ||
|
|
||
| when(userRepository.findByLoginId(loginId)).thenReturn(Optional.of(user)); | ||
| when(passwordEncoder.matches(rawPassword, encodedPassword)).thenReturn(false); | ||
|
|
||
| // when & then | ||
| assertThatThrownBy(() -> userService.getUserInfo(loginId, rawPassword)) | ||
| .isInstanceOf(CoreException.class) | ||
| .hasFieldOrPropertyWithValue("errorType", ErrorType.BAD_REQUEST) | ||
| .hasMessageContaining("비밀번호가 일치하지 않습니다"); | ||
|
|
||
| verify(userRepository).findByLoginId(loginId); | ||
| verify(passwordEncoder).matches(rawPassword, encodedPassword); | ||
| } | ||
| } |
There was a problem hiding this comment.
GetUserInfo에서 password null 테스트 케이스가 누락되었다
운영 관점: password가 null인 경우 passwordEncoder.matches에서 IllegalArgumentException이 발생한다. 헤더 누락 등으로 null이 전달될 수 있으므로 이 시나리오에 대한 테스트가 필요하다.
추가 테스트 예시
`@Test`
`@DisplayName`("비밀번호가 null이면 BAD_REQUEST 예외가 발생한다")
void throwsBadRequest_whenPasswordIsNull() {
// given
String loginId = "testuser";
String encodedPassword = "$2a$10$encryptedPassword";
User user = User.create(loginId, encodedPassword, "홍길동", LocalDate.of(1990, 1, 15), "test@example.com");
when(userRepository.findByLoginId(loginId)).thenReturn(Optional.of(user));
// when & then
assertThatThrownBy(() -> userService.getUserInfo(loginId, null))
.isInstanceOf(CoreException.class)
.hasFieldOrPropertyWithValue("errorType", ErrorType.BAD_REQUEST);
}🤖 Prompt for AI Agents
In `@apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceTest.java`
around lines 234 - 300, Add a test in the GetUserInfo nested test class to cover
the null password path: ensure when userRepository.findByLoginId(...) returns a
User and userService.getUserInfo(loginId, null) is called it throws
CoreException with ErrorType.BAD_REQUEST; mock behavior so
passwordEncoder.matches is not invoked (verify never called) and name the test
method throwsBadRequest_whenPasswordIsNull to mirror existing tests and
reference userService.getUserInfo, userRepository.findByLoginId, and
passwordEncoder.matches.
Comment on lines
+302
to
+466
| @Nested | ||
| @DisplayName("비밀번호 수정") | ||
| class UpdatePassword { | ||
|
|
||
| @Test | ||
| @DisplayName("유효한 정보로 비밀번호를 수정하면 성공한다") | ||
| void success() { | ||
| // given | ||
| String loginId = "testuser"; | ||
| String currentPassword = "Password1!"; | ||
| String newPassword = "NewPassword2!"; | ||
| String encodedCurrentPassword = "$2a$10$encryptedPassword"; | ||
| String encodedNewPassword = "$2a$10$newEncryptedPassword"; | ||
| LocalDate birthDate = LocalDate.of(1990, 1, 15); | ||
|
|
||
| User user = User.create(loginId, encodedCurrentPassword, "홍길동", birthDate, "test@example.com"); | ||
|
|
||
| when(userRepository.findByLoginId(loginId)).thenReturn(Optional.of(user)); | ||
| when(passwordEncoder.matches(currentPassword, encodedCurrentPassword)).thenReturn(true); | ||
| when(passwordEncoder.matches(newPassword, encodedCurrentPassword)).thenReturn(false); | ||
| when(passwordEncoder.encode(newPassword)).thenReturn(encodedNewPassword); | ||
|
|
||
| // when | ||
| userService.updatePassword(loginId, currentPassword, newPassword, birthDate); | ||
|
|
||
| // then | ||
| verify(userRepository).findByLoginId(loginId); | ||
| verify(passwordEncoder).matches(currentPassword, encodedCurrentPassword); | ||
| verify(passwordEncoder).matches(newPassword, encodedCurrentPassword); | ||
| verify(passwordEncoder).encode(newPassword); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("존재하지 않는 사용자의 비밀번호를 수정하면 NOT_FOUND 예외가 발생한다") | ||
| void throwsNotFound_whenUserNotExists() { | ||
| // given | ||
| String loginId = "nonexistent"; | ||
| when(userRepository.findByLoginId(loginId)).thenReturn(Optional.empty()); | ||
|
|
||
| // when & then | ||
| assertThatThrownBy(() -> userService.updatePassword( | ||
| loginId, | ||
| "Password1!", | ||
| "NewPassword2!", | ||
| LocalDate.of(1990, 1, 15) | ||
| )) | ||
| .isInstanceOf(CoreException.class) | ||
| .hasFieldOrPropertyWithValue("errorType", ErrorType.NOT_FOUND) | ||
| .hasMessageContaining("사용자를 찾을 수 없습니다"); | ||
|
|
||
| verify(userRepository).findByLoginId(loginId); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("기존 비밀번호가 일치하지 않으면 BAD_REQUEST 예외가 발생한다") | ||
| void throwsBadRequest_whenCurrentPasswordNotMatch() { | ||
| // given | ||
| String loginId = "testuser"; | ||
| String currentPassword = "WrongPassword1!"; | ||
| String encodedPassword = "$2a$10$encryptedPassword"; | ||
|
|
||
| User user = User.create(loginId, encodedPassword, "홍길동", LocalDate.of(1990, 1, 15), "test@example.com"); | ||
|
|
||
| when(userRepository.findByLoginId(loginId)).thenReturn(Optional.of(user)); | ||
| when(passwordEncoder.matches(currentPassword, encodedPassword)).thenReturn(false); | ||
|
|
||
| // when & then | ||
| assertThatThrownBy(() -> userService.updatePassword( | ||
| loginId, | ||
| currentPassword, | ||
| "NewPassword2!", | ||
| LocalDate.of(1990, 1, 15) | ||
| )) | ||
| .isInstanceOf(CoreException.class) | ||
| .hasFieldOrPropertyWithValue("errorType", ErrorType.BAD_REQUEST) | ||
| .hasMessageContaining("비밀번호가 일치하지 않습니다"); | ||
|
|
||
| verify(userRepository).findByLoginId(loginId); | ||
| verify(passwordEncoder).matches(currentPassword, encodedPassword); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("새 비밀번호가 기존 비밀번호와 동일하면 BAD_REQUEST 예외가 발생한다") | ||
| void throwsBadRequest_whenNewPasswordSameAsCurrent() { | ||
| // given | ||
| String loginId = "testuser"; | ||
| String currentPassword = "Password1!"; | ||
| String newPassword = "Password1!"; | ||
| String encodedPassword = "$2a$10$encryptedPassword"; | ||
|
|
||
| User user = User.create(loginId, encodedPassword, "홍길동", LocalDate.of(1990, 1, 15), "test@example.com"); | ||
|
|
||
| when(userRepository.findByLoginId(loginId)).thenReturn(Optional.of(user)); | ||
| when(passwordEncoder.matches(currentPassword, encodedPassword)).thenReturn(true); | ||
| when(passwordEncoder.matches(newPassword, encodedPassword)).thenReturn(true); | ||
|
|
||
| // when & then | ||
| assertThatThrownBy(() -> userService.updatePassword( | ||
| loginId, | ||
| currentPassword, | ||
| newPassword, | ||
| LocalDate.of(1990, 1, 15) | ||
| )) | ||
| .isInstanceOf(CoreException.class) | ||
| .hasFieldOrPropertyWithValue("errorType", ErrorType.BAD_REQUEST) | ||
| .hasMessageContaining("현재 비밀번호와 동일한 비밀번호는 사용할 수 없습니다"); | ||
|
|
||
| verify(userRepository).findByLoginId(loginId); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("새 비밀번호가 8자 미만이면 BAD_REQUEST 예외가 발생한다") | ||
| void throwsBadRequest_whenNewPasswordTooShort() { | ||
| // given | ||
| String loginId = "testuser"; | ||
| String currentPassword = "Password1!"; | ||
| String newPassword = "Pass1!"; | ||
| String encodedPassword = "$2a$10$encryptedPassword"; | ||
|
|
||
| User user = User.create(loginId, encodedPassword, "홍길동", LocalDate.of(1990, 1, 15), "test@example.com"); | ||
|
|
||
| when(userRepository.findByLoginId(loginId)).thenReturn(Optional.of(user)); | ||
| when(passwordEncoder.matches(currentPassword, encodedPassword)).thenReturn(true); | ||
| when(passwordEncoder.matches(newPassword, encodedPassword)).thenReturn(false); | ||
|
|
||
| // when & then | ||
| assertThatThrownBy(() -> userService.updatePassword( | ||
| loginId, | ||
| currentPassword, | ||
| newPassword, | ||
| LocalDate.of(1990, 1, 15) | ||
| )) | ||
| .isInstanceOf(CoreException.class) | ||
| .hasFieldOrPropertyWithValue("errorType", ErrorType.BAD_REQUEST) | ||
| .hasMessageContaining("8~16자"); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("새 비밀번호에 생년월일이 포함되면 BAD_REQUEST 예외가 발생한다") | ||
| void throwsBadRequest_whenNewPasswordContainsBirthDate() { | ||
| // given | ||
| String loginId = "testuser"; | ||
| String currentPassword = "Password1!"; | ||
| String newPassword = "19900115Pw!"; | ||
| String encodedPassword = "$2a$10$encryptedPassword"; | ||
| LocalDate birthDate = LocalDate.of(1990, 1, 15); | ||
|
|
||
| User user = User.create(loginId, encodedPassword, "홍길동", birthDate, "test@example.com"); | ||
|
|
||
| when(userRepository.findByLoginId(loginId)).thenReturn(Optional.of(user)); | ||
| when(passwordEncoder.matches(currentPassword, encodedPassword)).thenReturn(true); | ||
| when(passwordEncoder.matches(newPassword, encodedPassword)).thenReturn(false); | ||
|
|
||
| // when & then | ||
| assertThatThrownBy(() -> userService.updatePassword( | ||
| loginId, | ||
| currentPassword, | ||
| newPassword, | ||
| birthDate | ||
| )) | ||
| .isInstanceOf(CoreException.class) | ||
| .hasFieldOrPropertyWithValue("errorType", ErrorType.BAD_REQUEST) | ||
| .hasMessageContaining("생년월일"); | ||
| } | ||
| } |
There was a problem hiding this comment.
UpdatePassword에서 null 파라미터 테스트 케이스가 누락되었다
운영 관점: newPassword가 null인 경우 BCryptPasswordEncoder.matches에서 IllegalArgumentException이 발생하여 500 에러로 응답된다. 또한 birthDate가 null인 경우 validatePassword 내 format 호출에서 NPE가 발생한다. 이러한 경계 조건에 대한 테스트가 없으면 장애 시나리오를 사전에 검증할 수 없다.
추가 테스트 예시
`@Test`
`@DisplayName`("새 비밀번호가 null이면 BAD_REQUEST 예외가 발생한다")
void throwsBadRequest_whenNewPasswordIsNull() {
// given
String loginId = "testuser";
String currentPassword = "Password1!";
String encodedPassword = "$2a$10$encryptedPassword";
User user = User.create(loginId, encodedPassword, "홍길동", LocalDate.of(1990, 1, 15), "test@example.com");
when(userRepository.findByLoginId(loginId)).thenReturn(Optional.of(user));
when(passwordEncoder.matches(currentPassword, encodedPassword)).thenReturn(true);
// when & then
assertThatThrownBy(() -> userService.updatePassword(
loginId,
currentPassword,
null, // newPassword가 null
LocalDate.of(1990, 1, 15)
))
.isInstanceOf(CoreException.class)
.hasFieldOrPropertyWithValue("errorType", ErrorType.BAD_REQUEST);
}
`@Test`
`@DisplayName`("생년월일이 null이면 BAD_REQUEST 예외가 발생한다")
void throwsBadRequest_whenBirthDateIsNull() {
// given
String loginId = "testuser";
String currentPassword = "Password1!";
String newPassword = "NewPassword2!";
String encodedPassword = "$2a$10$encryptedPassword";
User user = User.create(loginId, encodedPassword, "홍길동", LocalDate.of(1990, 1, 15), "test@example.com");
when(userRepository.findByLoginId(loginId)).thenReturn(Optional.of(user));
when(passwordEncoder.matches(currentPassword, encodedPassword)).thenReturn(true);
when(passwordEncoder.matches(newPassword, encodedPassword)).thenReturn(false);
// when & then
assertThatThrownBy(() -> userService.updatePassword(
loginId,
currentPassword,
newPassword,
null // birthDate가 null
))
.isInstanceOf(CoreException.class)
.hasFieldOrPropertyWithValue("errorType", ErrorType.BAD_REQUEST);
}현재 UserService 코드에서 해당 null 검증이 누락되어 있으므로, 서비스 코드 수정 후 이 테스트들이 통과해야 한다.
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceTest.java`
around lines 302 - 466, The tests are missing null-parameter coverage and the
service lacks null checks: ensure UserService.updatePassword validates
newPassword and birthDate (and any inputs used by validatePassword) before
calling passwordEncoder.matches or validatePassword to avoid
IllegalArgumentException/NPE; add early guard clauses in updatePassword (or
inside validatePassword) that throw CoreException with ErrorType.BAD_REQUEST
when newPassword == null or birthDate == null, and update unit tests (tests
shown reference updatePassword) to assert these BAD_REQUEST behaviors.
Comment on lines
55
to
282
| @DisplayName("POST /api/v1/users/register") | ||
| @Nested | ||
| class Register { | ||
|
|
||
| @DisplayName("유효한 정보로 회원가입하면, 성공 응답을 반환한다.") | ||
| @Test | ||
| void returnsSuccess_whenValidInfoProvided() { | ||
| // given | ||
| UserV1Dto.RegisterRequest request = new UserV1Dto.RegisterRequest( | ||
| "testuser", | ||
| "Password1!", | ||
| "홍길동", | ||
| LocalDate.of(1990, 1, 15), | ||
| "test@example.com" | ||
| ); | ||
|
|
||
| // when | ||
| ParameterizedTypeReference<ApiResponse<UserV1Dto.RegisterResponse>> responseType = new ParameterizedTypeReference<>() {}; | ||
| ResponseEntity<ApiResponse<UserV1Dto.RegisterResponse>> response = | ||
| testRestTemplate.exchange(REGISTER_ENDPOINT, HttpMethod.POST, createRequestEntity(request), responseType); | ||
|
|
||
| // then | ||
| assertAll( | ||
| () -> assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK), | ||
| () -> assertThat(response.getBody().data().loginId()).isEqualTo("testuser"), | ||
| () -> assertThat(response.getBody().data().name()).isEqualTo("홍길동"), | ||
| () -> assertThat(response.getBody().data().email()).isEqualTo("test@example.com") | ||
| ); | ||
| } | ||
|
|
||
| @DisplayName("이미 존재하는 로그인 ID로 회원가입하면, 409 CONFLICT 응답을 반환한다.") | ||
| @Test | ||
| void returnsConflict_whenLoginIdAlreadyExists() { | ||
| // given | ||
| UserV1Dto.RegisterRequest firstRequest = new UserV1Dto.RegisterRequest( | ||
| "duplicateuser", | ||
| "Password1!", | ||
| "홍길동", | ||
| LocalDate.of(1990, 1, 15), | ||
| "first@example.com" | ||
| ); | ||
| ParameterizedTypeReference<ApiResponse<UserV1Dto.RegisterResponse>> responseType = new ParameterizedTypeReference<>() {}; | ||
| testRestTemplate.exchange(REGISTER_ENDPOINT, HttpMethod.POST, createRequestEntity(firstRequest), responseType); | ||
|
|
||
| UserV1Dto.RegisterRequest duplicateRequest = new UserV1Dto.RegisterRequest( | ||
| "duplicateuser", | ||
| "Password2!", | ||
| "김철수", | ||
| LocalDate.of(1995, 5, 20), | ||
| "second@example.com" | ||
| ); | ||
|
|
||
| // when | ||
| ResponseEntity<ApiResponse<UserV1Dto.RegisterResponse>> response = | ||
| testRestTemplate.exchange(REGISTER_ENDPOINT, HttpMethod.POST, createRequestEntity(duplicateRequest), responseType); | ||
|
|
||
| // then | ||
| assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CONFLICT); | ||
| } | ||
|
|
||
| @DisplayName("비밀번호가 8자 미만이면, 400 BAD_REQUEST 응답을 반환한다.") | ||
| @Test | ||
| void returnsBadRequest_whenPasswordTooShort() { | ||
| // given | ||
| UserV1Dto.RegisterRequest request = new UserV1Dto.RegisterRequest( | ||
| "testuser", | ||
| "Pass1!", | ||
| "홍길동", | ||
| LocalDate.of(1990, 1, 15), | ||
| "test@example.com" | ||
| ); | ||
|
|
||
| // when | ||
| ParameterizedTypeReference<ApiResponse<UserV1Dto.RegisterResponse>> responseType = new ParameterizedTypeReference<>() {}; | ||
| ResponseEntity<ApiResponse<UserV1Dto.RegisterResponse>> response = | ||
| testRestTemplate.exchange(REGISTER_ENDPOINT, HttpMethod.POST, createRequestEntity(request), responseType); | ||
|
|
||
| // then | ||
| assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); | ||
| } | ||
|
|
||
| @DisplayName("비밀번호가 16자 초과이면, 400 BAD_REQUEST 응답을 반환한다.") | ||
| @Test | ||
| void returnsBadRequest_whenPasswordTooLong() { | ||
| // given | ||
| UserV1Dto.RegisterRequest request = new UserV1Dto.RegisterRequest( | ||
| "testuser", | ||
| "Password1!Password1!", | ||
| "홍길동", | ||
| LocalDate.of(1990, 1, 15), | ||
| "test@example.com" | ||
| ); | ||
|
|
||
| // when | ||
| ParameterizedTypeReference<ApiResponse<UserV1Dto.RegisterResponse>> responseType = new ParameterizedTypeReference<>() {}; | ||
| ResponseEntity<ApiResponse<UserV1Dto.RegisterResponse>> response = | ||
| testRestTemplate.exchange(REGISTER_ENDPOINT, HttpMethod.POST, createRequestEntity(request), responseType); | ||
|
|
||
| // then | ||
| assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); | ||
| } | ||
|
|
||
| @DisplayName("비밀번호에 허용되지 않는 문자(공백)가 포함되면, 400 BAD_REQUEST 응답을 반환한다.") | ||
| @Test | ||
| void returnsBadRequest_whenPasswordContainsInvalidCharacter() { | ||
| // given | ||
| UserV1Dto.RegisterRequest request = new UserV1Dto.RegisterRequest( | ||
| "testuser", | ||
| "Pass word1!", | ||
| "홍길동", | ||
| LocalDate.of(1990, 1, 15), | ||
| "test@example.com" | ||
| ); | ||
|
|
||
| // when | ||
| ParameterizedTypeReference<ApiResponse<UserV1Dto.RegisterResponse>> responseType = new ParameterizedTypeReference<>() {}; | ||
| ResponseEntity<ApiResponse<UserV1Dto.RegisterResponse>> response = | ||
| testRestTemplate.exchange(REGISTER_ENDPOINT, HttpMethod.POST, createRequestEntity(request), responseType); | ||
|
|
||
| // then | ||
| assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); | ||
| } | ||
|
|
||
| @DisplayName("비밀번호에 생년월일(yyyyMMdd)이 포함되면, 400 BAD_REQUEST 응답을 반환한다.") | ||
| @Test | ||
| void returnsBadRequest_whenPasswordContainsBirthDateYYYYMMDD() { | ||
| // given | ||
| UserV1Dto.RegisterRequest request = new UserV1Dto.RegisterRequest( | ||
| "testuser", | ||
| "19900115Pw!", | ||
| "홍길동", | ||
| LocalDate.of(1990, 1, 15), | ||
| "test@example.com" | ||
| ); | ||
|
|
||
| // when | ||
| ParameterizedTypeReference<ApiResponse<UserV1Dto.RegisterResponse>> responseType = new ParameterizedTypeReference<>() {}; | ||
| ResponseEntity<ApiResponse<UserV1Dto.RegisterResponse>> response = | ||
| testRestTemplate.exchange(REGISTER_ENDPOINT, HttpMethod.POST, createRequestEntity(request), responseType); | ||
|
|
||
| // then | ||
| assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); | ||
| } | ||
|
|
||
| @DisplayName("비밀번호에 생년월일(yyMMdd)이 포함되면, 400 BAD_REQUEST 응답을 반환한다.") | ||
| @Test | ||
| void returnsBadRequest_whenPasswordContainsBirthDateYYMMDD() { | ||
| // given | ||
| UserV1Dto.RegisterRequest request = new UserV1Dto.RegisterRequest( | ||
| "testuser", | ||
| "900115Pass!", | ||
| "홍길동", | ||
| LocalDate.of(1990, 1, 15), | ||
| "test@example.com" | ||
| ); | ||
|
|
||
| // when | ||
| ParameterizedTypeReference<ApiResponse<UserV1Dto.RegisterResponse>> responseType = new ParameterizedTypeReference<>() {}; | ||
| ResponseEntity<ApiResponse<UserV1Dto.RegisterResponse>> response = | ||
| testRestTemplate.exchange(REGISTER_ENDPOINT, HttpMethod.POST, createRequestEntity(request), responseType); | ||
|
|
||
| // then | ||
| assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); | ||
| } | ||
|
|
||
| @DisplayName("이메일 형식이 올바르지 않으면, 400 BAD_REQUEST 응답을 반환한다.") | ||
| @Test | ||
| void returnsBadRequest_whenEmailFormatInvalid() { | ||
| // given | ||
| UserV1Dto.RegisterRequest request = new UserV1Dto.RegisterRequest( | ||
| "testuser", | ||
| "Password1!", | ||
| "홍길동", | ||
| LocalDate.of(1990, 1, 15), | ||
| "invalid-email" | ||
| ); | ||
|
|
||
| // when | ||
| ParameterizedTypeReference<ApiResponse<UserV1Dto.RegisterResponse>> responseType = new ParameterizedTypeReference<>() {}; | ||
| ResponseEntity<ApiResponse<UserV1Dto.RegisterResponse>> response = | ||
| testRestTemplate.exchange(REGISTER_ENDPOINT, HttpMethod.POST, createRequestEntity(request), responseType); | ||
|
|
||
| // then | ||
| assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); | ||
| } | ||
|
|
||
| @DisplayName("이름이 비어있으면, 400 BAD_REQUEST 응답을 반환한다.") | ||
| @Test | ||
| void returnsBadRequest_whenNameIsEmpty() { | ||
| // given | ||
| UserV1Dto.RegisterRequest request = new UserV1Dto.RegisterRequest( | ||
| "testuser", | ||
| "Password1!", | ||
| "", | ||
| LocalDate.of(1990, 1, 15), | ||
| "test@example.com" | ||
| ); | ||
|
|
||
| // when | ||
| ParameterizedTypeReference<ApiResponse<UserV1Dto.RegisterResponse>> responseType = new ParameterizedTypeReference<>() {}; | ||
| ResponseEntity<ApiResponse<UserV1Dto.RegisterResponse>> response = | ||
| testRestTemplate.exchange(REGISTER_ENDPOINT, HttpMethod.POST, createRequestEntity(request), responseType); | ||
|
|
||
| // then | ||
| assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); | ||
| } | ||
|
|
||
| @DisplayName("로그인 ID가 비어있으면, 400 BAD_REQUEST 응답을 반환한다.") | ||
| @Test | ||
| void returnsBadRequest_whenLoginIdIsEmpty() { | ||
| // given | ||
| UserV1Dto.RegisterRequest request = new UserV1Dto.RegisterRequest( | ||
| "", | ||
| "Password1!", | ||
| "홍길동", | ||
| LocalDate.of(1990, 1, 15), | ||
| "test@example.com" | ||
| ); | ||
|
|
||
| // when | ||
| ParameterizedTypeReference<ApiResponse<UserV1Dto.RegisterResponse>> responseType = new ParameterizedTypeReference<>() {}; | ||
| ResponseEntity<ApiResponse<UserV1Dto.RegisterResponse>> response = | ||
| testRestTemplate.exchange(REGISTER_ENDPOINT, HttpMethod.POST, createRequestEntity(request), responseType); | ||
|
|
||
| // then | ||
| assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); | ||
| } | ||
| } |
There was a problem hiding this comment.
birthDate가 null인 경우의 테스트 케이스가 누락되었다
운영 관점: UserService의 validatePassword 메서드에서 birthDate가 null인 경우 NPE가 발생하여 500 에러로 응답된다. 이 시나리오에 대한 E2E 테스트가 없으면 운영 배포 후 장애 발생 시 원인 파악이 지연될 수 있다.
추가 테스트 예시
`@DisplayName`("생년월일이 null이면, 400 BAD_REQUEST 응답을 반환한다.")
`@Test`
void returnsBadRequest_whenBirthDateIsNull() {
// given
UserV1Dto.RegisterRequest request = new UserV1Dto.RegisterRequest(
"testuser",
"Password1!",
"홍길동",
null, // birthDate가 null
"test@example.com"
);
// when
ParameterizedTypeReference<ApiResponse<UserV1Dto.RegisterResponse>> responseType = new ParameterizedTypeReference<>() {};
ResponseEntity<ApiResponse<UserV1Dto.RegisterResponse>> response =
testRestTemplate.exchange(REGISTER_ENDPOINT, HttpMethod.POST, createRequestEntity(request), responseType);
// then
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST);
}현재 UserService 코드에서 birthDate null 검증이 누락되어 있으므로, 이 테스트는 서비스 코드 수정 후 통과할 것이다.
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/test/java/com/loopers/interfaces/api/UserV1ApiE2ETest.java`
around lines 55 - 282, Add an E2E test that submits a register request with
birthDate == null and asserts HttpStatus.BAD_REQUEST; specifically add a new
test method (e.g., returnsBadRequest_whenBirthDateIsNull) inside the Register
nested class in UserV1ApiE2ETest that constructs a UserV1Dto.RegisterRequest
with null birthDate, uses the existing REGISTER_ENDPOINT,
ParameterizedTypeReference<ApiResponse<UserV1Dto.RegisterResponse>>, and
testRestTemplate.exchange(...) to post the request, and asserts
response.getStatusCode() == HttpStatus.BAD_REQUEST; after adding the test,
ensure UserService.validatePassword (or the null-check logic around birthDate)
is updated to handle null birthDate to avoid NPE so the test can pass.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📌 Summary
🧭 Context & Decision
비밀번호 검증 로직 위치 (Domain vs Service)
고려한 대안:
A: Domain Layer (User 엔티티 내부)
B: Service Layer (UserService)
최종 결정: B (Service Layer)
이유:
트레이드오프:
도메인 순수성 vs 실용성
현재는 실용성을 우선시하되, 핵심 도메인 규칙은 User 엔티티에 유지
User.updatePassword()- 비밀번호 변경 책임은 엔티티에 위임고민 : 코치님이라면 해당 검증로직은 어디에 작성하는게 좋을까요?
🏗️ Design Overview
변경 범위
영향 받는 모듈/도메인:
apps/commerce-api- 사용자 API 추가modules/jpa- User 엔티티 추가신규 추가:
User,UserService,UserRepositoryUserFacade,UserInfoUserV1Controller,UserV1DtoUserRepositoryImpl,UserJpaRepositoryUserServiceTest,UserV1ApiE2ETest제거/대체:
주요 컴포넌트 책임
Interface Layer
UserV1Controller:Application Layer
UserFacade:UserInfo:Domain Layer
User:UserService:UserRepository:Infrastructure Layer
UserRepositoryImpl:UserJpaRepository:🔁 Flow Diagram
1. 회원가입 (POST /api/v1/users/register)
sequenceDiagram autonumber participant Client participant Controller as UserV1Controller participant Facade as UserFacade participant Service as UserService participant Repository as UserRepository participant DB as MySQL Client->>Controller: POST /api/v1/users/register<br/>{loginId, password, name, birthDate, email} Controller->>Facade: register(loginId, password, name, birthDate, email) Facade->>Service: register(loginId, password, name, birthDate, email) Service->>Service: validateRegisterRequest()<br/>(loginId, password, name, email 검증) Service->>Repository: existsByLoginId(loginId) Repository->>DB: SELECT COUNT(*) FROM users WHERE login_id = ? DB-->>Repository: 0 (존재하지 않음) Repository-->>Service: false Service->>Service: passwordEncoder.encode(password) Service->>Service: User.create(...) Service->>Repository: save(user) Repository->>DB: INSERT INTO users VALUES (...) DB-->>Repository: user (with id) Repository-->>Service: user Service-->>Facade: user Facade->>Facade: UserInfo.from(user) Facade-->>Controller: userInfo Controller->>Controller: new RegisterResponse(...) Controller-->>Client: 200 OK<br/>{id, loginId, name, email}예외 흐름 (회원가입)
sequenceDiagram autonumber participant Client participant Controller participant Service participant Repository Note over Client,Repository: Case 1: 중복 로그인 ID Client->>Controller: POST /register (중복 loginId) Controller->>Service: register(...) Service->>Repository: existsByLoginId(loginId) Repository-->>Service: true Service-->>Controller: throw CoreException(CONFLICT) Controller-->>Client: 409 CONFLICT Note over Client,Repository: Case 2: 비밀번호 규칙 위반 Client->>Controller: POST /register (짧은 password) Controller->>Service: register(...) Service->>Service: validatePassword() Service-->>Controller: throw CoreException(BAD_REQUEST) Controller-->>Client: 400 BAD_REQUEST2. 내 정보 조회 (GET /api/v1/users/info)
sequenceDiagram autonumber participant Client participant Controller as UserV1Controller participant Facade as UserFacade participant Service as UserService participant Repository as UserRepository participant DB as MySQL Client->>Controller: GET /api/v1/users/info<br/>Headers: X-Loopers-LoginId, X-Loopers-LoginPw Controller->>Facade: getUserInfo(loginId, password) Facade->>Service: getUserInfo(loginId, password) Service->>Service: findUserAndValidatePassword(loginId, password) Service->>Repository: findByLoginId(loginId) Repository->>DB: SELECT * FROM users WHERE login_id = ? DB-->>Repository: user Repository-->>Service: Optional<User> Service->>Service: passwordEncoder.matches(password, user.password) Service-->>Facade: user Facade->>Facade: UserInfo.from(user) Facade-->>Controller: userInfo Controller->>Controller: userInfo.getMaskedName()<br/>("홍길동" → "홍길*") Controller->>Controller: new UserInfoResponse(...) Controller-->>Client: 200 OK<br/>{loginId, name(masked), birthDate, email}예외 흐름 (내 정보 조회)
sequenceDiagram autonumber participant Client participant Controller participant Service participant Repository Note over Client,Repository: Case 1: 사용자 없음 Client->>Controller: GET /info (존재하지 않는 loginId) Controller->>Service: getUserInfo(...) Service->>Repository: findByLoginId(loginId) Repository-->>Service: Optional.empty() Service-->>Controller: throw CoreException(NOT_FOUND) Controller-->>Client: 404 NOT_FOUND Note over Client,Repository: Case 2: 비밀번호 불일치 Client->>Controller: GET /info (잘못된 password) Controller->>Service: getUserInfo(...) Service->>Repository: findByLoginId(loginId) Repository-->>Service: user Service->>Service: passwordEncoder.matches(password, user.password) Service-->>Controller: throw CoreException(BAD_REQUEST) Controller-->>Client: 400 BAD_REQUEST3. 비밀번호 변경 (PUT /api/v1/users/password)
sequenceDiagram autonumber participant Client participant Controller as UserV1Controller participant Facade as UserFacade participant Service as UserService participant Repository as UserRepository participant DB as MySQL Client->>Controller: PUT /api/v1/users/password<br/>Headers: X-Loopers-LoginId, X-Loopers-LoginPw<br/>Body: {newPassword} Note over Controller: Step 1: 사용자 인증 Controller->>Facade: getUserInfo(loginId, currentPassword) Facade->>Service: getUserInfo(loginId, currentPassword) Service->>Service: findUserAndValidatePassword(...) Service->>Repository: findByLoginId(loginId) Repository->>DB: SELECT * FROM users WHERE login_id = ? DB-->>Repository: user Repository-->>Service: user Service->>Service: passwordEncoder.matches(currentPassword, user.password) Service-->>Facade: user Facade-->>Controller: userInfo Note over Controller: Step 2: 비밀번호 변경 Controller->>Facade: updatePassword(loginId, currentPassword, newPassword, birthDate) Facade->>Service: updatePassword(loginId, currentPassword, newPassword, birthDate) Service->>Service: findUserAndValidatePassword(loginId, currentPassword) Service->>Repository: findByLoginId(loginId) Repository->>DB: SELECT * FROM users WHERE login_id = ? DB-->>Repository: user Repository-->>Service: user Service->>Service: passwordEncoder.matches(newPassword, user.password)<br/>(신규 != 기존 확인) Service->>Service: validatePassword(newPassword, birthDate) Service->>Service: passwordEncoder.encode(newPassword) Service->>Service: user.updatePassword(encryptedPassword) Service->>Repository: save(user) (JPA dirty checking) Repository->>DB: UPDATE users SET password = ?, updated_at = ? WHERE user_id = ? DB-->>Repository: success Repository-->>Service: void Service-->>Facade: void Facade-->>Controller: void Controller-->>Client: 200 OK예외 흐름 (비밀번호 변경)
sequenceDiagram autonumber participant Client participant Controller participant Service Note over Client,Service: Case 1: 현재 비밀번호 불일치 Client->>Controller: PUT /password (잘못된 currentPassword) Controller->>Service: getUserInfo(...) Service-->>Controller: throw CoreException(BAD_REQUEST) Controller-->>Client: 400 BAD_REQUEST Note over Client,Service: Case 2: 신규 비밀번호 = 기존 비밀번호 Client->>Controller: PUT /password (동일한 newPassword) Controller->>Service: updatePassword(...) Service->>Service: passwordEncoder.matches(newPassword, user.password) Service-->>Controller: throw CoreException(BAD_REQUEST)<br/>"동일한 비밀번호 사용 불가" Controller-->>Client: 400 BAD_REQUEST Note over Client,Service: Case 3: 신규 비밀번호 규칙 위반 Client->>Controller: PUT /password (짧은 newPassword) Controller->>Service: updatePassword(...) Service->>Service: validatePassword(newPassword, birthDate) Service-->>Controller: throw CoreException(BAD_REQUEST) Controller-->>Client: 400 BAD_REQUEST변경 목적
Example 도메인 예제 코드를 완전히 제거하고, AI 기반 TDD를 통해 사용자 도메인의 회원가입, 내 정보 조회, 비밀번호 수정 기능을 구현합니다.
핵심 변경점
테스트/검증 방법
UserServiceTest(483줄)에서 회원가입 중복 검사, 비밀번호 유효성(길이, 패턴, 생년월일), 내 정보 조회(사용자 미존재, 비밀번호 불일치), 비밀번호 변경(현재 비밀번호 검증, 새 비밀번호 동일 검사) 등을 단위 테스트로 검증하고, UserV1ApiE2ETest(619줄)에서 POST /register, GET /info(이름 마스킹 포함), PUT /password의 전체 API 흐름을 E2E 테스트로 검증합니다. 코드 리뷰 후 birthDate null 체크와 timezone 수정이 추가되었습니다.
리스크/주의사항
비밀번호 검증이 서비스 레이어에서 수행되어 도메인 순수성과 실용성의 트레이드오프가 존재합니다. 또한 X-Loopers-LoginId/X-Loopers-LoginPw 헤더 기반 인증이 평문으로 전달되는데, 프로덕션 환경에서는 반드시 HTTPS 사용 확인이 필요하고, 사용자 정보 조회 시 반환되는 maskedName(조회할 때만 마스킹, 저장되지 않음)의 설계 의도를 확인하시기 바랍니다.