[volume-1] 회원가입/내 정보 조회/비밀번호 변경 기능 작업 #28
Conversation
|
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: 6
🤖 Fix all issues with AI agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/User.java`:
- Around line 87-88: User.changePassword currently assigns newPassword without
null checks which can cause a JPA NOT NULL violation at flush; add a defensive
null check at the start of the method (in class User, method
changePassword(Password newPassword)) that throws a BAD_REQUEST-mapped exception
(e.g., IllegalArgumentException or your project's BadRequestException) when
newPassword is null instead of assigning, and add a unit test asserting that
calling User.changePassword(null) results in that BAD_REQUEST exception.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/user/BcryptPasswordEncoder.java`:
- Around line 8-20: BcryptPasswordEncoder currently hardcodes BCrypt strength
via new BCryptPasswordEncoder() so make strength configurable: add a constructor
or `@Value-injected` field to BcryptPasswordEncoder that accepts an int strength
and instantiate encoder with new BCryptPasswordEncoder(strength) (keep matches
and encode delegations unchanged), wire the config property from application
properties/env, and add/update a unit test that creates BcryptPasswordEncoder
with a known strength (e.g., 11) and asserts the produced hash contains the
corresponding "$11$" round marker to verify the setting is applied.
In `@apps/commerce-api/src/test/java/com/loopers/domain/user/BirthDateTest.java`:
- Around line 14-36: The test is flaky due to using LocalDate.now(); replace the
dynamic future date in the test method 생년월일이_미래면_BAD_REQUEST를_던진다() with a fixed
LocalDate constant (e.g., LocalDate.of(YYYY,MM,DD).plusDays(1)) so the assertion
against BirthDate.from(...) is deterministic, and update/add tests to validate
"today is allowed" by refactoring BirthDate to accept a Clock (or overloads) so
you can inject Clock.fixed in tests and assert BirthDate.from(todayClockDate)
succeeds; reference BirthDate.from and the BirthDate class when making the Clock
injection and add a new test that uses Clock.fixed to assert today's date is
accepted.
In `@apps/commerce-api/src/test/java/com/loopers/domain/user/EmailTest.java`:
- Around line 11-20: The test EmailTest::이메일_형식을_따르지_않으면_BAD_REQUEST를_던진다
currently only asserts that Email.from(value) throws CoreException but doesn't
assert the ErrorType, risking regressions; update the test to capture the thrown
CoreException and assert that ex.getErrorType() (or equivalent accessor) equals
ErrorType.BAD_REQUEST, and add at least one more test case (e.g., blank, null,
or missing '@') that also asserts the same ErrorType to lock down the failure
flow for Email.from and ErrorType handling.
In `@apps/commerce-api/src/test/java/com/loopers/domain/user/LoginIdTest.java`:
- Around line 14-24: Rename the test method 로그인ID는_한글이_포함되면_BAD_REQUSET를_던진다()
in LoginIdTest to correct the typo to 로그인ID는_한글이_포함되면_BAD_REQUEST를_던진다(); keep
the method behavior unchanged (it still calls LoginId.from(value) and asserts
CoreException with ErrorType.BAD_REQUEST) so only the test method identifier is
updated for readability and searchability.
In
`@apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceTest.java`:
- Around line 70-86: The test misses stubbing passwordEncoder.encode, causing
null/invalid password hashes to flow into Password.of/User constructor and
making CI flaky; in the test for UserService.signup (and the other case at lines
108-116), stub passwordEncoder.encode(...) to return a deterministic hash (e.g.,
"encoded-pw"), then call userService.signup as before and assert/verify that
passwordEncoder.encode was invoked (verify(passwordEncoder).encode(...)) and/or
that the persisted User/Password stored the expected encoded value; ensure
references to passwordEncoder.encode, UserService.signup, Password.of and the
User entity constructor are used to locate and fix the test.
🧹 Nitpick comments (9)
apps/commerce-api/src/main/java/com/loopers/domain/user/BirthDate.java (2)
27-29:LocalDate.now()사용으로 테스트 격리가 어렵다.운영 관점: 현재 시간에 의존하는 로직은 테스트 시 재현성을 저해한다. 특정 날짜 경계값 테스트(예: 오늘 생년월일)가 실행 시점에 따라 결과가 달라질 수 있다.
수정안:
Clock을 파라미터로 받거나, 검증 시점의 기준 날짜를 외부에서 주입받는 방식을 고려할 수 있다. 다만 현재 단순한 미래 날짜 검증이므로 즉시 변경이 필수는 아니다.추가 테스트: 생년월일이 오늘인 경우 정상 생성되는지 확인하는 테스트를
BirthDateTest에 추가하면 경계 조건을 커버할 수 있다.
16-16:value필드에final키워드 누락값 객체의 불변성을 컴파일 타임에 보장하려면
value필드를final로 선언하는 것이 좋다. 현재는 setter가 없어 런타임에는 불변이나, 향후 실수로 setter가 추가될 여지가 있다.- private LocalDate value; + private final LocalDate value;단,
@NoArgsConstructor와 함께 사용 시 JPA 프록시 생성을 위해force = true옵션이 필요하다:-@NoArgsConstructor(access = AccessLevel.PROTECTED) +@NoArgsConstructor(access = AccessLevel.PROTECTED, force = true)apps/commerce-api/src/test/java/com/loopers/domain/user/PasswordTest.java (2)
16-27: 경계값 테스트 케이스 보강 필요운영 관점: 현재 7자와 17자만 테스트하여 실패 케이스를 검증하나, 8자와 16자의 정상 경계값 테스트가 누락되어 있다. 정규표현식
{8,16}수정 시 경계에서 버그가 발생해도 감지되지 않는다.수정안: 8자, 16자 정상 케이스를 성공 테스트에 추가한다.
`@ParameterizedTest` `@ValueSource`(strings = {"12345678", "1234567890123456"}) // 8자, 16자 경계값 void 비밀번호가_8자이상_16자이하이면_생성_성공(String value) { // given PasswordEncoder encoder = mock(PasswordEncoder.class); when(encoder.encode(value)).thenReturn("encrypted"); // when Password password = Password.of(value, encoder); // then assertThat(password.asString()).isEqualTo("encrypted"); }
14-56: 추가 테스트 케이스 누락운영 관점:
Password클래스의 전체 동작을 검증하려면 다음 케이스가 필요하다.
- null/blank 입력 테스트:
Password.of(null, encoder),Password.of("", encoder),Password.of(" ", encoder)시BAD_REQUEST발생 확인- 허용 특수문자 테스트: 패턴에 명시된 특수문자(
`~!@#$%^&*|'";:\₩?) 포함 시 정상 생성 확인matches()메서드 테스트: 원본 비밀번호와 암호화된 값 비교 로직 검증`@Test` void 비밀번호가_null이면_BAD_REQUEST를_던진다() { PasswordEncoder encoder = mock(PasswordEncoder.class); CoreException result = assertThrows(CoreException.class, () -> { Password.of(null, encoder); }); assertThat(result.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST); } `@Test` void 비밀번호에_허용된_특수문자가_포함되면_생성_성공() { String value = "pass!@#$"; PasswordEncoder encoder = mock(PasswordEncoder.class); when(encoder.encode(value)).thenReturn("encrypted"); Password password = Password.of(value, encoder); assertThat(password.asString()).isEqualTo("encrypted"); } `@Test` void matches_메서드는_원본과_암호화값을_비교한다() { String raw = "loopers123"; PasswordEncoder encoder = mock(PasswordEncoder.class); when(encoder.encode(raw)).thenReturn("hashed"); when(encoder.matches(raw, "hashed")).thenReturn(true); Password password = Password.of(raw, encoder); assertThat(password.matches(raw, encoder)).isTrue(); }apps/commerce-api/src/test/java/com/loopers/infrastructure/user/UserRepositoryImplTest.java (1)
37-42: 조회된 User의 필드값 검증이 누락되어 있다.운영 관점에서 Repository가 올바른 데이터를 반환하는지 확인하려면
isPresent()만으로는 부족하다. 저장한 User와 조회된 User의 필드가 일치하는지 검증해야 데이터 매핑 오류를 조기에 발견할 수 있다.🔧 추가 검증 제안
// then assertThat(findUser).isPresent(); + assertThat(findUser.get().loginId()).isEqualTo(loginId); + assertThat(findUser.get().name()).isEqualTo(name); + assertThat(findUser.get().email()).isEqualTo(email);apps/commerce-api/src/main/java/com/loopers/domain/user/UserInfo.java (1)
7-14: User 파라미터에 대한 null 검증이 없다.
from(User user)메서드에서 null User가 전달되면 NullPointerException이 발생한다. 서비스 계층에서 null이 전달되지 않도록 보장하더라도, 도메인 객체는 방어적으로 설계하는 것이 운영 환경에서 예기치 않은 장애를 방지하는 데 유리하다.🛡️ 방어적 검증 추가
public static UserInfo from(User user) { + if (user == null) { + throw new IllegalArgumentException("User cannot be null"); + } return new UserInfo( user.loginId().asString(), user.name().masked(), user.birthDate().asLocalDate(), user.email().asString() ); }apps/commerce-api/src/test/java/com/loopers/domain/user/UserTest.java (1)
41-57: null 필드 검증 테스트가 name 하나에만 국한되어 있다.운영 환경에서 어떤 필드가 null로 전달되어도 동일하게 BAD_REQUEST를 던지는지 확인하려면, 각 필드별 null 케이스를 모두 테스트해야 한다. 현재는 name만 검증하고 있어 loginId, password, birthDate, email이 null일 때의 동작이 보장되지 않는다.
🧪 ParameterizedTest로 전체 null 케이스 검증
`@ParameterizedTest` `@MethodSource`("nullFieldCases") void 필드_중_하나라도_null이면_BAD_REQUEST를_던진다(LoginId loginId, Password password, Name name, BirthDate birthDate, Email email) { CoreException result = assertThrows(CoreException.class, () -> { User.create(loginId, password, name, birthDate, email); }); assertThat(result.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST); } static Stream<Arguments> nullFieldCases() { PasswordEncoder encoder = mock(PasswordEncoder.class); return Stream.of( Arguments.of(null, Password.of("loopers123", encoder), Name.from("루퍼스"), BirthDate.from(LocalDate.of(1996, 11, 22)), Email.from("test@loopers.im")), Arguments.of(LoginId.from("loopers"), null, Name.from("루퍼스"), BirthDate.from(LocalDate.of(1996, 11, 22)), Email.from("test@loopers.im")), // ... 나머지 케이스 ); }apps/commerce-api/src/test/java/com/loopers/domain/user/LoginIdTest.java (1)
38-46: 경계값 성공 케이스 테스트가 누락되어 있다.현재 테스트는 실패 경계값(4자, 21자)만 검증한다. 운영 환경에서 정확히 5자, 20자인 loginId가 정상 생성되는지 확인해야 경계 조건 로직의 off-by-one 오류를 방지할 수 있다.
🧪 경계값 성공 케이스 추가
`@ParameterizedTest` `@ValueSource`(strings = {"abcde", "abcdefghijklmnopqrst"}) // 5자, 20자 void 로그인ID는_5자이상_20자이하면_정상생성(String value) { LoginId loginId = LoginId.from(value); assertThat(loginId.asString()).isEqualTo(value); }apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceIntegrationTest.java (1)
29-32: 테스트 실패 시 데이터 정리가 보장되지 않을 수 있다.
@AfterEach에서 테이블을 truncate하는 방식은 테스트 실패 시에도 실행되지만, 테스트 중 예외로 인한 트랜잭션 롤백이 없어 데이터가 남을 수 있다. 병렬 테스트 실행 시 데이터 충돌로 인한 flaky test가 발생할 가능성이 있다.현재 구조에서는
@BeforeEach에서도 cleanup을 수행하면 테스트 격리가 더 확실해진다.🛡️ 테스트 격리 강화
+ `@BeforeEach` + void cleanUp() { + databaseCleanUp.truncateAllTables(); + } + `@AfterEach` void tearDown() { databaseCleanUp.truncateAllTables(); }
| public void changePassword(Password newPassword) { | ||
| this.password = newPassword; |
There was a problem hiding this comment.
changePassword에서 null 방어가 필요하다
운영 관점에서 newPassword가 null로 유입되면 JPA flush 시 NOT NULL 제약 위반으로 500이 발생해 장애로 이어질 수 있다.
수정안: changePassword에서 null을 사전 차단하고 BAD_REQUEST로 명확히 실패시키는 방어 로직을 추가한다.
추가 테스트: User.changePassword(null) 호출 시 BAD_REQUEST가 발생하는 단위 테스트를 추가한다.
🔧 수정안 예시
public void changePassword(Password newPassword) {
+ if (newPassword == null) {
+ throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 필수입니다.");
+ }
this.password = newPassword;
}As per coding guidelines, **/*.java: null 처리, 방어적 복사, 불변성, equals/hashCode/toString 구현 안정성을 점검한다.
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/User.java` around
lines 87 - 88, User.changePassword currently assigns newPassword without null
checks which can cause a JPA NOT NULL violation at flush; add a defensive null
check at the start of the method (in class User, method changePassword(Password
newPassword)) that throws a BAD_REQUEST-mapped exception (e.g.,
IllegalArgumentException or your project's BadRequestException) when newPassword
is null instead of assigning, and add a unit test asserting that calling
User.changePassword(null) results in that BAD_REQUEST exception.
| public class BcryptPasswordEncoder implements PasswordEncoder { | ||
|
|
||
| private final BCryptPasswordEncoder encoder = new BCryptPasswordEncoder(); | ||
|
|
||
| @Override | ||
| public String encode(String raw) { | ||
| return encoder.encode(raw); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean matches(String raw, String encoded) { | ||
| return encoder.matches(raw, encoded); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "BcryptPasswordEncoder.java" -o -name "*PasswordEncoder*.java" | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 256
🌐 Web query:
Spring Security BCryptPasswordEncoder default strength value performance impact
💡 Result:
- Default strength (log rounds):
BCryptPasswordEncoderdefaults to 10. [1][2] - Performance impact: the work factor grows exponentially with the strength value; increasing strength by +1 roughly doubles the CPU work/time for both
encode()andmatches()(and decreasing by 1 roughly halves it). [2] - Practical guidance: Spring Security recommends benchmarking on your hardware and tuning strength so that password verification takes ~1 second (bcrypt is intentionally slow to resist cracking). [1]
Sources:
[1] Spring Security Reference: Password Storage (BCryptPasswordEncoder)
[2] Spring Security API Javadoc: BCryptPasswordEncoder
🏁 Script executed:
cat -n ./apps/commerce-api/src/main/java/com/loopers/infrastructure/user/BcryptPasswordEncoder.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 821
🏁 Script executed:
find ./apps/commerce-api -type f \( -name "application*.yml" -o -name "application*.properties" \) | head -10Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 139
🏁 Script executed:
cat -n ./apps/commerce-api/src/main/resources/application.ymlRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1450
BCrypt 비용 인자 하드코딩으로 환경별 성능 튜닝이 불가능하다
Line 10에서 BCryptPasswordEncoder() 기본 생성자를 사용하면 strength가 10으로 고정된다. 이는 하드웨어 사양, 트래픽 패턴, 보안 정책에 따라 strength를 조정할 수 없다는 의미다. Spring Security 공식 권장사항은 비밀번호 검증 시간이 약 1초가 되도록 하드웨어에 맞춰 strength를 튜닝하는 것이다. strength가 +1 증가하면 CPU 작업량이 약 2배 증가하므로, 트래픽 증가 시 로그인 지연이 심해지거나, 빠른 응답을 위해 strength를 낮추면 보안이 약화될 수 있다.
설정값으로 비용 인자를 주입해 환경별로 독립적으로 튜닝 가능하게 하고, 테스트에서 설정된 strength가 실제 해시에 반영되는지 검증해야 한다. 예를 들어, strength 11로 설정했다면 해시 문자열의 라운드 정보($11$ 형식)로 확인할 수 있다.
🛠️ 수정안 (설정 기반 비용 인자 주입)
import com.loopers.domain.user.PasswordEncoder;
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
+import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Component;
`@Component`
public class BcryptPasswordEncoder implements PasswordEncoder {
- private final BCryptPasswordEncoder encoder = new BCryptPasswordEncoder();
+ private final BCryptPasswordEncoder encoder;
+
+ public BcryptPasswordEncoder(`@Value`("${security.bcrypt.strength:10}") int strength) {
+ this.encoder = new BCryptPasswordEncoder(strength);
+ }🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/user/BcryptPasswordEncoder.java`
around lines 8 - 20, BcryptPasswordEncoder currently hardcodes BCrypt strength
via new BCryptPasswordEncoder() so make strength configurable: add a constructor
or `@Value-injected` field to BcryptPasswordEncoder that accepts an int strength
and instantiate encoder with new BCryptPasswordEncoder(strength) (keep matches
and encode delegations unchanged), wire the config property from application
properties/env, and add/update a unit test that creates BcryptPasswordEncoder
with a known strength (e.g., 11) and asserts the produced hash contains the
corresponding "$11$" round marker to verify the setting is applied.
| @Test | ||
| void 생년월일이_미래면_BAD_REQUEST를_던진다() { | ||
| // given | ||
| LocalDate value = LocalDate.now().plusDays(1); | ||
|
|
||
| // when-then | ||
| CoreException result = assertThrows(CoreException.class, () -> { | ||
| BirthDate.from(value); | ||
| }); | ||
| assertThat(result.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST); | ||
| } | ||
|
|
||
| @Test | ||
| void 생년월일이_오늘날짜_이전이면_정상적으로_생성() { | ||
| // given | ||
| LocalDate value = LocalDate.of(1996, 11, 22); | ||
|
|
||
| // when | ||
| BirthDate birthDate = BirthDate.from(value); | ||
|
|
||
| // then | ||
| assertThat(birthDate.asLocalDate()).isEqualTo(value); | ||
| } |
There was a problem hiding this comment.
현재 날짜 의존으로 테스트 플래키 위험이다
Line 17이 실행 시점의 날짜를 사용하므로 자정/타임존 경계에서 간헐적으로 실패해 CI 파이프라인을 불안정하게 만들 수 있다. 운영 안정성을 위해 미래 날짜는 고정 상수로 변경하고, 경계값(오늘) 검증은 Clock을 고정 주입할 수 있게 도메인을 보완한 뒤 테스트를 추가하는 것이 안전하다. 추가 테스트로 Clock.fixed 기반에서 “오늘 날짜는 허용” 케이스를 검증해 규칙 회귀를 막아야 한다. As per coding guidelines, 단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다.
🛠️ 수정안 (플래키 제거)
- LocalDate value = LocalDate.now().plusDays(1);
+ LocalDate value = LocalDate.of(2099, 1, 1);📝 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.
| @Test | |
| void 생년월일이_미래면_BAD_REQUEST를_던진다() { | |
| // given | |
| LocalDate value = LocalDate.now().plusDays(1); | |
| // when-then | |
| CoreException result = assertThrows(CoreException.class, () -> { | |
| BirthDate.from(value); | |
| }); | |
| assertThat(result.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST); | |
| } | |
| @Test | |
| void 생년월일이_오늘날짜_이전이면_정상적으로_생성() { | |
| // given | |
| LocalDate value = LocalDate.of(1996, 11, 22); | |
| // when | |
| BirthDate birthDate = BirthDate.from(value); | |
| // then | |
| assertThat(birthDate.asLocalDate()).isEqualTo(value); | |
| } | |
| `@Test` | |
| void 생년월일이_미래면_BAD_REQUEST를_던진다() { | |
| // given | |
| LocalDate value = LocalDate.of(2099, 1, 1); | |
| // when-then | |
| CoreException result = assertThrows(CoreException.class, () -> { | |
| BirthDate.from(value); | |
| }); | |
| assertThat(result.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST); | |
| } | |
| `@Test` | |
| void 생년월일이_오늘날짜_이전이면_정상적으로_생성() { | |
| // given | |
| LocalDate value = LocalDate.of(1996, 11, 22); | |
| // when | |
| BirthDate birthDate = BirthDate.from(value); | |
| // then | |
| assertThat(birthDate.asLocalDate()).isEqualTo(value); | |
| } |
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/test/java/com/loopers/domain/user/BirthDateTest.java`
around lines 14 - 36, The test is flaky due to using LocalDate.now(); replace
the dynamic future date in the test method 생년월일이_미래면_BAD_REQUEST를_던진다() with a
fixed LocalDate constant (e.g., LocalDate.of(YYYY,MM,DD).plusDays(1)) so the
assertion against BirthDate.from(...) is deterministic, and update/add tests to
validate "today is allowed" by refactoring BirthDate to accept a Clock (or
overloads) so you can inject Clock.fixed in tests and assert
BirthDate.from(todayClockDate) succeeds; reference BirthDate.from and the
BirthDate class when making the Clock injection and add a new test that uses
Clock.fixed to assert today's date is accepted.
| @Test | ||
| void 이메일_형식을_따르지_않으면_BAD_REQUEST를_던진다() { | ||
| // given | ||
| String value = "testloopers.im"; | ||
|
|
||
| // when-then | ||
| CoreException result = assertThrows(CoreException.class, () -> { | ||
| Email.from(value); | ||
| }); | ||
| } |
There was a problem hiding this comment.
예외 타입 검증 누락으로 오류 응답 회귀 가능성이 있다
현재는 예외 발생만 확인해 ErrorType이 바뀌어도 테스트가 통과하므로, 운영에서 클라이언트가 잘못된 상태코드를 받는 회귀를 놓칠 수 있다. ErrorType.BAD_REQUEST까지 검증하도록 보강하고, 추가 테스트로 공백/널/‘@’ 누락 등 대표 실패 케이스를 하나 더 넣어 오류 흐름을 고정해야 한다. As per coding guidelines, 단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다.
🛠️ 수정안 (ErrorType 검증 추가)
-import com.loopers.support.error.CoreException;
+import com.loopers.support.error.CoreException;
+import com.loopers.support.error.ErrorType;
@@
CoreException result = assertThrows(CoreException.class, () -> {
Email.from(value);
});
+ assertThat(result.getErrorType()).isEqualTo(ErrorType.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.
| @Test | |
| void 이메일_형식을_따르지_않으면_BAD_REQUEST를_던진다() { | |
| // given | |
| String value = "testloopers.im"; | |
| // when-then | |
| CoreException result = assertThrows(CoreException.class, () -> { | |
| Email.from(value); | |
| }); | |
| } | |
| import com.loopers.support.error.CoreException; | |
| import com.loopers.support.error.ErrorType; | |
| `@Test` | |
| void 이메일_형식을_따르지_않으면_BAD_REQUEST를_던진다() { | |
| // given | |
| String value = "testloopers.im"; | |
| // when-then | |
| CoreException result = assertThrows(CoreException.class, () -> { | |
| Email.from(value); | |
| }); | |
| assertThat(result.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST); | |
| } |
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/test/java/com/loopers/domain/user/EmailTest.java`
around lines 11 - 20, The test EmailTest::이메일_형식을_따르지_않으면_BAD_REQUEST를_던진다
currently only asserts that Email.from(value) throws CoreException but doesn't
assert the ErrorType, risking regressions; update the test to capture the thrown
CoreException and assert that ex.getErrorType() (or equivalent accessor) equals
ErrorType.BAD_REQUEST, and add at least one more test case (e.g., blank, null,
or missing '@') that also asserts the same ErrorType to lock down the failure
flow for Email.from and ErrorType handling.
| @Test | ||
| void 로그인ID는_한글이_포함되면_BAD_REQUSET를_던진다() { | ||
| // given | ||
| String value = "루퍼스123loopers"; | ||
|
|
||
| // when-then | ||
| CoreException result = assertThrows(CoreException.class, () -> { | ||
| LoginId.from(value); | ||
| }); | ||
| assertThat(result.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST); | ||
| } |
There was a problem hiding this comment.
메서드명에 오타가 있다: BAD_REQUSET → BAD_REQUEST
테스트 메서드명의 오타는 테스트 리포트 가독성을 저해하고, 검색 시 누락될 수 있다.
✏️ 오타 수정
- void 로그인ID는_한글이_포함되면_BAD_REQUSET를_던진다() {
+ void 로그인ID는_한글이_포함되면_BAD_REQUEST를_던진다() {🤖 Prompt for AI Agents
In `@apps/commerce-api/src/test/java/com/loopers/domain/user/LoginIdTest.java`
around lines 14 - 24, Rename the test method 로그인ID는_한글이_포함되면_BAD_REQUSET를_던진다()
in LoginIdTest to correct the typo to 로그인ID는_한글이_포함되면_BAD_REQUEST를_던진다(); keep
the method behavior unchanged (it still calls LoginId.from(value) and asserts
CoreException with ErrorType.BAD_REQUEST) so only the test method identifier is
updated for readability and searchability.
| @Test | ||
| void 정상적으로_되면_회원객체를_생성해서_반환() { | ||
| // given | ||
| String loginId = "loopers123"; | ||
| String password = "loopers123!@"; | ||
| String name = "루퍼스"; | ||
| LocalDate birthDate = LocalDate.of(1996, 11, 22); | ||
| String email = "test@loopers.im"; | ||
|
|
||
| when(userRepository.findByLoginId(LoginId.from(loginId))).thenReturn(Optional.empty()); | ||
|
|
||
| // when | ||
| LoginId returnLoginId = userService.signup(loginId, password, name, birthDate, email); | ||
|
|
||
| // then | ||
| assertThat(returnLoginId).isEqualTo(LoginId.from(loginId)); | ||
| } |
There was a problem hiding this comment.
PasswordEncoder 인코딩 스텁 누락은 수정이 필요하다
운영 관점에서 mock 기본 반환값(null)이 흘러가면 Password.of 또는 signup 내부에서 예외가 발생해 CI가 불안정해지고 해시 적용 회귀를 놓칠 수 있다.
수정안: 해당 테스트에서 passwordEncoder.encode를 명시적으로 스텁해 유효한 해시가 전달되도록 한다.
추가 테스트: 성공 케이스에서 verify(passwordEncoder).encode(...) 또는 저장된 해시가 "encoded"인지 단언해 인코딩 경로를 고정한다.
🔧 수정안 예시
// 정상적으로_되면_회원객체를_생성해서_반환()
when(userRepository.findByLoginId(LoginId.from(loginId))).thenReturn(Optional.empty());
+when(passwordEncoder.encode(password)).thenReturn("encoded");
// when
LoginId returnLoginId = userService.signup(loginId, password, name, birthDate, email);
+verify(passwordEncoder).encode(password); // 비밀번호가_불일치하면_UNAUTHORIZED를_던진다()
// Arrange
+when(passwordEncoder.encode("loopers123!@")).thenReturn("encoded");
User user = User.create(
LoginId.from("loopers123"),
Password.of("loopers123!@", passwordEncoder),
Name.from("루퍼스"),
BirthDate.from(LocalDate.of(1996, 11, 22)),
Email.from("test@loopers.im")
);Based on learnings, User 엔티티 생성자의 password 파라미터는 BCrypt 해시를 받는다.
Also applies to: 108-116
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceTest.java`
around lines 70 - 86, The test misses stubbing passwordEncoder.encode, causing
null/invalid password hashes to flow into Password.of/User constructor and
making CI flaky; in the test for UserService.signup (and the other case at lines
108-116), stub passwordEncoder.encode(...) to return a deterministic hash (e.g.,
"encoded-pw"), then call userService.signup as before and assert/verify that
passwordEncoder.encode was invoked (verify(passwordEncoder).encode(...)) and/or
that the persisted User/Password stored the expected encoded value; ensure
references to passwordEncoder.encode, UserService.signup, Password.of and the
User entity constructor are used to locate and fix the test.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/Password.java`:
- Around line 23-33: The Password.of factory currently assumes PasswordEncoder
is non-null; add a fail-fast null check at the start of Password.of( String
value, PasswordEncoder encoder ) that throws a clear CoreException
(ErrorType.BAD_REQUEST or another appropriate ErrorType per project conventions)
with a descriptive message when encoder == null, then proceed with the existing
validations and encoding; also add a unit test that calls Password.of with
encoder == null and asserts the exact exception type and message to prevent NPEs
from DI/test misconfiguration.
- Around line 39-41: The Password.matches method lacks validation for the raw
input which can let null/blank reach the PasswordEncoder and cause 500s; update
Password.matches(String raw, PasswordEncoder encoder) to defensively validate
raw (null or blank) and throw a clear, documented exception (e.g.,
IllegalArgumentException with a consistent message) so controllers can translate
it to BAD_REQUEST, then add unit tests for Password.matches verifying that null
and blank raw values throw the expected exception with the expected message;
ensure this change follows the class's immutability/null-handling conventions
and does not modify Password.value or encoder usage.
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java`:
- Around line 19-38: The validatePasswordNotContainsBirthDate method can NPE
when password or birthDate is null; add defensive null checks at the start of
validatePasswordNotContainsBirthDate to throw a CoreException with
ErrorType.BAD_REQUEST and a clear message (consistent with other value object
validations) when password==null or birthDate==null, so signup and
changePassword will fail with 400 instead of 500; update or add unit tests
covering signup(null, "", "", birthDate, email) and changePassword(loginId,
currentPassword, null) to verify BAD_REQUEST behavior.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserController.java`:
- Around line 34-49: In UserController (methods getMyInfo and changePassword)
change the `@RequestHeader` annotations to required=false, then defensively check
the loginId and password/currentPassword for null or blank at the start of
getMyInfo and changePassword and throw a CoreException (so ApiControllerAdvice's
CoreException handler returns 400 with ApiResponse) when missing/empty; ensure
you reference the methods getMyInfo and changePassword and the CoreException
type when implementing the check, and add/adjust controller/integration tests to
assert that missing or blank headers produce 400 BAD_REQUEST with
ApiResponse.meta.result=FAIL.
In `@apps/commerce-api/src/test/java/com/loopers/domain/user/PasswordTest.java`:
- Around line 43-55: Add unit tests that explicitly verify Password.matches
delegates to the injected PasswordEncoder: create tests that mock
PasswordEncoder.matches to return true and false, construct a Password (via
Password.of or an equivalent factory) using the mock encoder, then assert
Password.matches(input, encoder) (or instance.matches if API is instance method)
returns the same boolean as the mocked encoder; reference Password.matches,
Password.of, PasswordEncoder, and Password.asString to locate the class and
ensure both true and false branches are covered so encoder injection/matching
logic is validated.
🧹 Nitpick comments (6)
apps/commerce-api/src/main/java/com/loopers/domain/user/BirthDate.java (1)
27-29:LocalDate.now()직접 호출은 테스트 격리와 타임존 문제를 유발할 수 있다운영 관점에서 서버 타임존 설정에 따라 자정 전후로 동작이 달라질 수 있다. 테스트에서는 시간 고정이 불가능해 경계값 테스트가 어렵다.
개선 방안으로
Clock을 주입받는 오버로드 메서드를 추가하면 테스트 격리와 타임존 명시가 가능해진다.🔧 개선안
+ public static BirthDate from(LocalDate value) { + return from(value, Clock.systemDefaultZone()); + } + + public static BirthDate from(LocalDate value, Clock clock) { if (value == null) { throw new CoreException(ErrorType.BAD_REQUEST, "생년월일은 비어있을 수 없습니다."); } - if (value.isAfter(LocalDate.now())) { + if (value.isAfter(LocalDate.now(clock))) { throw new CoreException(ErrorType.BAD_REQUEST, "생년월일은 오늘 이전 날짜만 가능합니다."); } return new BirthDate(value); }apps/commerce-api/src/test/java/com/loopers/domain/user/BirthDateTest.java (1)
26-36: "오늘 날짜" 경계값 테스트가 누락되어 있다
BirthDate.from()의 검증 조건은value.isAfter(LocalDate.now())이므로 오늘 날짜는 허용되어야 한다. 그러나 이를 검증하는 테스트가 없어 향후 조건 변경 시 회귀를 감지할 수 없다.🧪 경계값 테스트 추가안
`@Test` void 생년월일이_오늘이면_정상적으로_생성() { // given LocalDate today = LocalDate.now(); // when BirthDate birthDate = BirthDate.from(today); // then assertThat(birthDate.asLocalDate()).isEqualTo(today); }apps/commerce-api/src/test/java/com/loopers/domain/user/UserTest.java (1)
41-55: null 필드 테스트가 다른 실패 원인과 섞일 수 있다
운영 관점에서 테스트가 특정 실패 원인을 정확히 지목하지 못하면 회귀 탐지가 불안정해진다. 현재는PasswordEncoder.encode미스텁으로 인해 해시 값이 null이 될 수 있어, 향후 비밀번호 검증이 강화되면 “이름 null”이 아닌 다른 원인으로 실패할 가능성이 있다. 수정안으로는 이 테스트에서도 encode 스텁을 추가하고, “하나라도 null” 요구를 충족하도록 필드별 null 케이스를 파라미터화해 분리하는 편이 안정적이다. 추가 테스트로 loginId/password/birthDate/email 각각 null 케이스를 포함해야 한다다.As per coding guidelines `**/*Test*.java`: Java 테스트 리뷰 기준이다. 단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다.♻️ 수정안(encode 스텁 추가)
`@Test` void 로그인ID_비밀번호_이름_생년월일_이메일_중_하나라도_null이면_BAD_REQUEST를_던진다() { // given PasswordEncoder encoder = mock(PasswordEncoder.class); + when(encoder.encode("loopers123")).thenReturn("encrypted");apps/commerce-api/src/test/java/com/loopers/interfaces/api/user/UserControllerTest.java (1)
37-63: 실패 응답 포맷 검증이 없어 예외 매핑 회귀를 놓칠 수 있다
운영 관점에서는 컨트롤러에서 CoreException이 ApiResponse로 매핑되지 않아도 이 테스트가 통과해 클라이언트 파싱 오류를 배포 후에야 발견할 수 있다.
수정안: userService.signup이 CoreException(ErrorType.CONFLICT)을 던지도록 스텁하고 409와 meta.result=FAIL을 검증하는 실패 케이스 테스트를 추가하는 것이 좋다.
추가 테스트: 중복 로그인ID 요청에 대한 실패 응답 포맷(ApiResponse meta/result)을 검증하는 테스트를 추가해야 한다.As per coding guidelines, `**/*Test*.java`: 단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다.🧪 추가 테스트 예시
+import com.loopers.support.error.CoreException; +import com.loopers.support.error.ErrorType; ... class Signup { `@Test` void 성공하면_201_CREATED() throws Exception { // given UserDto.SignupRequest request = new UserDto.SignupRequest( "looper123", "password123", "루퍼스", LocalDate.of(1996, 11, 22), "test@loopers.im" ); when(userService.signup(request.loginId(), request.password(), request.name(), request.birthDate(), request.email())) .thenReturn(LoginId.from("looper123")); ... .andExpect(status().isCreated()) .andExpect(jsonPath("$.data.loginId").value("looper123")); } + + `@Test` + void 중복이면_409_CONFLICT를_반환한다() throws Exception { + UserDto.SignupRequest request = new UserDto.SignupRequest( + "looper123", + "password123", + "루퍼스", + LocalDate.of(1996, 11, 22), + "test@loopers.im" + ); + when(userService.signup(request.loginId(), request.password(), request.name(), request.birthDate(), request.email())) + .thenThrow(new CoreException(ErrorType.CONFLICT, "이미 존재하는 로그인ID입니다.")); + + String content = objectMapper.writeValueAsString(request); + + mockMvc.perform(post("/api/users") + .contentType(MediaType.APPLICATION_JSON) + .content(content)) + .andExpect(status().isConflict()) + .andExpect(jsonPath("$.meta.result").value("FAIL")); + } }apps/commerce-api/src/test/java/com/loopers/interfaces/api/user/UserApiE2ETest.java (1)
150-191: 에러 응답의 ApiResponse 포맷 검증이 부족하다
운영 관점에서는 에러 응답 포맷이 바뀌어도 404/401 테스트가 통과해 클라이언트 파싱 오류를 늦게 발견할 수 있다.
수정안: 404/401 응답에서도 meta.result=FAIL을 검증하도록 단언을 보강하는 것이 좋다.
추가 테스트: 404/401 케이스에서 ApiResponse meta/result를 검증하는 단언을 추가해야 한다.As per coding guidelines, `**/*Test*.java`: 단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다.🔧 수정 예시
`@Test` void 존재하지_않는_loginId면_404_NOT_FOUND를_반환한다() { // Arrange HttpHeaders headers = new HttpHeaders(); headers.set("X-Loopers-LoginId", "nonexist12"); headers.set("X-Loopers-LoginPw", "loopers123!@"); // Act ResponseEntity<ApiResponse<Object>> response = testRestTemplate.exchange( ENDPOINT + "/me", HttpMethod.GET, new HttpEntity<>(null, headers), new ParameterizedTypeReference<>() {} ); // Assert - assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND); + assertAll( + () -> assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND), + () -> assertThat(response.getBody().meta().result()).isEqualTo(ApiResponse.Metadata.Result.FAIL) + ); } ... `@Test` void 비밀번호가_불일치하면_401_UNAUTHORIZED를_반환한다() { // Arrange UserDto.SignupRequest signupRequest = new UserDto.SignupRequest( "loopers123", "loopers123!@", "루퍼스", LocalDate.of(1996, 11, 22), "test@loopers.im" ); testRestTemplate.exchange( ENDPOINT, HttpMethod.POST, new HttpEntity<>(signupRequest, jsonHeaders()), new ParameterizedTypeReference<ApiResponse<UserDto.SignupResponse>>() {} ); HttpHeaders headers = new HttpHeaders(); headers.set("X-Loopers-LoginId", "loopers123"); headers.set("X-Loopers-LoginPw", "wrongPass123!"); // Act ResponseEntity<ApiResponse<Object>> response = testRestTemplate.exchange( ENDPOINT + "/me", HttpMethod.GET, new HttpEntity<>(null, headers), new ParameterizedTypeReference<>() {} ); // Assert - assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED); + assertAll( + () -> assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED), + () -> assertThat(response.getBody().meta().result()).isEqualTo(ApiResponse.Metadata.Result.FAIL) + ); }apps/commerce-api/src/main/java/com/loopers/domain/user/Password.java (1)
13-43: Embeddable 값 객체의 equals/hashCode 추가 권장
운영 관점에서 컬렉션/캐시/영속성 변경 감지 시 객체 참조 기준 비교로 인해 중복 저장이나 변경 감지 누락 위험이 있다. 수정안으로value기반 equals/hashCode를 추가하라. 추가 테스트로 동일 해시 값에 대한 equals/hashCode 동작을 검증하라. As per coding guidelines: equals/hashCode/toString 구현 안정성을 점검한다.🔧 제안 수정
+import java.util.Objects; @@ public class Password { @@ public boolean matches(String raw, PasswordEncoder encoder) { return encoder.matches(raw, value); } + + `@Override` + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Password password = (Password) o; + return Objects.equals(value, password.value); + } + + `@Override` + public int hashCode() { + return Objects.hashCode(value); + } }
| public static Password of(String value, PasswordEncoder encoder) { | ||
| if (value == null || value.isBlank()) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 비어있을 수 없습니다."); | ||
| } | ||
|
|
||
| if (!PASSWORD_PATTERN.matcher(value).matches()) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 8~16자의 영문대소문자, 숫자, 특수문자만 가능합니다."); | ||
| } | ||
|
|
||
| return new Password(encoder.encode(value)); | ||
| } |
There was a problem hiding this comment.
PasswordEncoder null 방어가 필요하다
운영 관점에서 DI 설정 누락이나 테스트 스텁 미주입 시 NPE로 500이 발생해 장애 원인 파악이 어려워진다. 수정안으로 encoder에 대한 fail-fast 검사를 추가해 명확한 오류를 내도록 하라. 추가 테스트로 encoder == null일 때 예외 타입/메시지를 검증하는 단위 테스트를 추가하라. As per coding guidelines: null 처리, 방어적 복사, 불변성, equals/hashCode/toString 구현 안정성을 점검한다.
🔧 제안 수정
+import java.util.Objects;
@@
public static Password of(String value, PasswordEncoder encoder) {
+ Objects.requireNonNull(encoder, "PasswordEncoder must not be null");
if (value == null || value.isBlank()) {
throw new CoreException(ErrorType.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.
| public static Password of(String value, PasswordEncoder encoder) { | |
| if (value == null || value.isBlank()) { | |
| throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 비어있을 수 없습니다."); | |
| } | |
| if (!PASSWORD_PATTERN.matcher(value).matches()) { | |
| throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 8~16자의 영문대소문자, 숫자, 특수문자만 가능합니다."); | |
| } | |
| return new Password(encoder.encode(value)); | |
| } | |
| public static Password of(String value, PasswordEncoder encoder) { | |
| Objects.requireNonNull(encoder, "PasswordEncoder must not be null"); | |
| if (value == null || value.isBlank()) { | |
| throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 비어있을 수 없습니다."); | |
| } | |
| if (!PASSWORD_PATTERN.matcher(value).matches()) { | |
| throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 8~16자의 영문대소문자, 숫자, 특수문자만 가능합니다."); | |
| } | |
| return new Password(encoder.encode(value)); | |
| } |
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/Password.java` around
lines 23 - 33, The Password.of factory currently assumes PasswordEncoder is
non-null; add a fail-fast null check at the start of Password.of( String value,
PasswordEncoder encoder ) that throws a clear CoreException
(ErrorType.BAD_REQUEST or another appropriate ErrorType per project conventions)
with a descriptive message when encoder == null, then proceed with the existing
validations and encoding; also add a unit test that calls Password.of with
encoder == null and asserts the exact exception type and message to prevent NPEs
from DI/test misconfiguration.
| // Password는 저장 시 암호화 되므로 Password 간의 동일성/동등성은 PasswordEncoder 가 확인하는 것으로 한다. | ||
| public boolean matches(String raw, PasswordEncoder encoder) { | ||
| return encoder.matches(raw, value); |
There was a problem hiding this comment.
matches 입력값 null/blank 처리 누락
운영 관점에서 로그인/비밀번호 변경 요청에 null/blank가 유입되면 내부 인코더가 예외를 던져 500으로 전파될 수 있다. 수정안으로 raw에 대한 사전 검증을 추가해 일관된 BAD_REQUEST 응답으로 처리하라. 추가 테스트로 raw가 null/blank일 때 예외와 메시지를 검증하는 테스트를 추가하라. As per coding guidelines: null 처리, 방어적 복사, 불변성, equals/hashCode/toString 구현 안정성을 점검한다.
🔧 제안 수정
public boolean matches(String raw, PasswordEncoder encoder) {
+ if (raw == null || raw.isBlank()) {
+ throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 비어있을 수 없습니다.");
+ }
return encoder.matches(raw, value);
}📝 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.
| // Password는 저장 시 암호화 되므로 Password 간의 동일성/동등성은 PasswordEncoder 가 확인하는 것으로 한다. | |
| public boolean matches(String raw, PasswordEncoder encoder) { | |
| return encoder.matches(raw, value); | |
| // Password는 저장 시 암호화 되므로 Password 간의 동일성/동등성은 PasswordEncoder 가 확인하는 것으로 한다. | |
| public boolean matches(String raw, PasswordEncoder encoder) { | |
| if (raw == null || raw.isBlank()) { | |
| throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 비어있을 수 없습니다."); | |
| } | |
| return encoder.matches(raw, value); | |
| } |
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/Password.java` around
lines 39 - 41, The Password.matches method lacks validation for the raw input
which can let null/blank reach the PasswordEncoder and cause 500s; update
Password.matches(String raw, PasswordEncoder encoder) to defensively validate
raw (null or blank) and throw a clear, documented exception (e.g.,
IllegalArgumentException with a consistent message) so controllers can translate
it to BAD_REQUEST, then add unit tests for Password.matches verifying that null
and blank raw values throw the expected exception with the expected message;
ensure this change follows the class's immutability/null-handling conventions
and does not modify Password.value or encoder usage.
| @Transactional | ||
| public LoginId signup(String loginId, String password, String name, LocalDate birthDate, String email) { | ||
| LoginId id = LoginId.from(loginId); | ||
|
|
||
| if (userRepository.findByLoginId(id).isPresent()) { | ||
| throw new CoreException(ErrorType.CONFLICT, "이미 존재하는 로그인ID입니다."); | ||
| } | ||
|
|
||
| validatePasswordNotContainsBirthDate(password, birthDate); | ||
|
|
||
| User user = User.create(id | ||
| , Password.of(password, passwordEncoder) | ||
| , Name.from(name) | ||
| , BirthDate.from(birthDate) | ||
| , Email.from(email)); | ||
|
|
||
| userRepository.save(user); | ||
|
|
||
| return user.loginId(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "UserService.java" -path "*/commerce-api/*" -type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 159
🏁 Script executed:
find . -name "Password.java" -path "*/domain/*" -type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 156
🏁 Script executed:
find . -name "User.java" -path "*/domain/user/*" -type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 152
🏁 Script executed:
wc -l ./apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 162
🏁 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: 3401
🏁 Script executed:
cat -n ./apps/commerce-api/src/main/java/com/loopers/domain/user/Password.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1647
🏁 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: 3228
🏁 Script executed:
# User의 다른 값 객체들도 null 체크가 있는지 확인
find . -name "*.java" -path "*/domain/user/*" -type f | head -10 | xargs grep -l "public static"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 577
🏁 Script executed:
# LoginId, Name, BirthDate, Email의 from() 메서드 null 처리 확인
cat ./apps/commerce-api/src/main/java/com/loopers/domain/user/LoginId.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1412
🏁 Script executed:
cat ./apps/commerce-api/src/main/java/com/loopers/domain/user/BirthDate.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1303
🏁 Script executed:
cat ./apps/commerce-api/src/main/java/com/loopers/domain/user/Name.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1454
🏁 Script executed:
cat ./apps/commerce-api/src/main/java/com/loopers/domain/user/Email.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1403
validatePasswordNotContainsBirthDate에서 null 입력 시 NPE 발생 방지
password나 birthDate가 null일 때 validatePasswordNotContainsBirthDate 메서드 내부에서 NPE가 발생해 500 에러로 떨어진다. signup과 changePassword 메서드에서 이 검증이 Password.of()보다 먼저 실행되므로, Password.of()의 null 체크도 도움이 되지 않는다.
validatePasswordNotContainsBirthDate 메서드 시작 부분에 null 방어 로직을 추가해 BAD_REQUEST로 명시적으로 실패시켜야 한다. 다른 값 객체들(LoginId, Password, Name, BirthDate, Email)과 일관되게 처리한다.
추가 테스트: signup(null, "", "", birthDate, email), changePassword(loginId, currentPassword, null)을 포함한 null 입력 테스트를 추가해야 한다.
🔧 수정 예시
private void validatePasswordNotContainsBirthDate(String password, LocalDate birthDate) {
+ if (password == null) {
+ throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 필수입니다.");
+ }
+ if (birthDate == null) {
+ throw new CoreException(ErrorType.BAD_REQUEST, "생년월일은 필수입니다.");
+ }
String birthDateWithDash = birthDate.toString();
String birthDateWithoutDash = birthDateWithDash.replace("-", "");
if (password.contains(birthDateWithDash) || password.contains(birthDateWithoutDash)) {
throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 생년월일을 포함할 수 없습니다.");
}
}🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java`
around lines 19 - 38, The validatePasswordNotContainsBirthDate method can NPE
when password or birthDate is null; add defensive null checks at the start of
validatePasswordNotContainsBirthDate to throw a CoreException with
ErrorType.BAD_REQUEST and a clear message (consistent with other value object
validations) when password==null or birthDate==null, so signup and
changePassword will fail with 400 instead of 500; update or add unit tests
covering signup(null, "", "", birthDate, email) and changePassword(loginId,
currentPassword, null) to verify BAD_REQUEST behavior.
| @GetMapping("/me") | ||
| public ApiResponse<UserDto.MyInfoResponse> getMyInfo( | ||
| @RequestHeader("X-Loopers-LoginId") String loginId, | ||
| @RequestHeader("X-Loopers-LoginPw") String password) { | ||
| UserInfo userInfo = userService.getMyInfo(loginId, password); | ||
| return ApiResponse.success(UserDto.MyInfoResponse.from(userInfo)); | ||
| } | ||
|
|
||
| @PatchMapping("/me/password") | ||
| public ApiResponse<Object> changePassword( | ||
| @RequestHeader("X-Loopers-LoginId") String loginId, | ||
| @RequestHeader("X-Loopers-LoginPw") String currentPassword, | ||
| @RequestBody UserDto.ChangePasswordRequest request) { | ||
| userService.changePassword(loginId, currentPassword, request.newPassword()); | ||
| return ApiResponse.success(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "*ControllerAdvice*" -o -name "*ExceptionHandler*" | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 170
🏁 Script executed:
find . -type f -name "*ApiControllerAdvice*" | head -10Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 170
🏁 Script executed:
rg "ApiControllerAdvice" --type java -l | head -10Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 168
🏁 Script executed:
rg "CoreException" --type java -l | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1585
🏁 Script executed:
rg "MissingRequestHeaderException" --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/interfaces/api/ApiControllerAdvice.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 6820
🏁 Script executed:
cat -n ./apps/commerce-api/src/main/java/com/loopers/support/error/CoreException.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 741
🏁 Script executed:
rg "@RequestHeader" --type java -B 2 -A 2Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1820
🏁 Script executed:
rg "required\s*=\s*false" --type java | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
필수 헤더 누락 시 예외 처리 흐름이 깨진다
현재 코드는 @RequestHeader 기본값(required=true)을 사용하므로, 헤더 누락 시 Spring의 MissingRequestHeaderException이 발생한다. 그런데 ApiControllerAdvice에는 이 예외에 대한 전용 핸들러가 없어, 일반 Throwable 핸들러(라인 110-114)에 포괄되어 500 INTERNAL_ERROR를 반환한다. 결과적으로 클라이언트는 ApiResponse 포맷이 아닌 기본 에러 응답을 받게 되고, 이는 모니터링과 통합 테스트 검증 기준을 깨뜨린다.
수정안: 헤더를 required=false로 받고 null 또는 공백 여부를 직접 검사해 CoreException으로 throw하는 방어 로직을 추가해야 한다. 이렇게 하면 ApiControllerAdvice의 CoreException 핸들러(라인 26-30)를 거쳐 일관된 400 BAD_REQUEST와 ApiResponse 포맷이 보장된다.
수정 예시
import com.loopers.domain.user.LoginId;
import com.loopers.domain.user.UserService;
import com.loopers.interfaces.api.ApiResponse;
+import com.loopers.support.error.CoreException;
+import com.loopers.support.error.ErrorType;
import lombok.RequiredArgsConstructor;
import com.loopers.domain.user.UserInfo;
import org.springframework.http.HttpStatus;
import org.springframework.web.bind.annotation.*;
`@GetMapping`("/me")
public ApiResponse<UserDto.MyInfoResponse> getMyInfo(
- `@RequestHeader`("X-Loopers-LoginId") String loginId,
- `@RequestHeader`("X-Loopers-LoginPw") String password) {
+ `@RequestHeader`(value = "X-Loopers-LoginId", required = false) String loginId,
+ `@RequestHeader`(value = "X-Loopers-LoginPw", required = false) String password) {
+ if (loginId == null || loginId.isBlank() || password == null || password.isBlank()) {
+ throw new CoreException(ErrorType.BAD_REQUEST, "인증 헤더는 필수입니다.");
+ }
UserInfo userInfo = userService.getMyInfo(loginId, password);
return ApiResponse.success(UserDto.MyInfoResponse.from(userInfo));
}
`@PatchMapping`("/me/password")
public ApiResponse<Object> changePassword(
- `@RequestHeader`("X-Loopers-LoginId") String loginId,
- `@RequestHeader`("X-Loopers-LoginPw") String currentPassword,
+ `@RequestHeader`(value = "X-Loopers-LoginId", required = false) String loginId,
+ `@RequestHeader`(value = "X-Loopers-LoginPw", required = false) String currentPassword,
`@RequestBody` UserDto.ChangePasswordRequest request) {
+ if (loginId == null || loginId.isBlank() || currentPassword == null || currentPassword.isBlank()) {
+ throw new CoreException(ErrorType.BAD_REQUEST, "인증 헤더는 필수입니다.");
+ }
userService.changePassword(loginId, currentPassword, request.newPassword());
return ApiResponse.success();
}추가로 헤더 누락/공백 입력 시 400 응답과 ApiResponse.meta.result=FAIL 조건을 검증하는 컨트롤러/통합 테스트를 추가해야 한다.
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserController.java`
around lines 34 - 49, In UserController (methods getMyInfo and changePassword)
change the `@RequestHeader` annotations to required=false, then defensively check
the loginId and password/currentPassword for null or blank at the start of
getMyInfo and changePassword and throw a CoreException (so ApiControllerAdvice's
CoreException handler returns 400 with ApiResponse) when missing/empty; ensure
you reference the methods getMyInfo and changePassword and the CoreException
type when implementing the check, and add/adjust controller/integration tests to
assert that missing or blank headers produce 400 BAD_REQUEST with
ApiResponse.meta.result=FAIL.
| @Test | ||
| void 유효한_비밀번호이면_암호화해서_객체생성_성공() { | ||
| // given | ||
| String value = "loopers123"; | ||
| PasswordEncoder encoder = mock(PasswordEncoder.class); | ||
| when(encoder.encode(value)).thenReturn("encrypted"); | ||
|
|
||
| // when | ||
| Password password = Password.of(value, encoder); | ||
|
|
||
| // then | ||
| assertThat(password.asString()).isEqualTo("encrypted"); | ||
| } |
There was a problem hiding this comment.
Password.matches 경로를 테스트로 고정해야 한다
- 운영 관점: encode 경로만 검증되어 matches 로직이나 인코더 주입이 깨져도 로그인·비밀번호 변경에서 오탐/미탐이 발생하고 테스트가 이를 조기에 탐지하지 못한다.
- 수정안: Password.matches가 encoder.matches 결과에 따라 true/false를 반환하는지 검증하는 테스트를 추가해야 한다.
- 추가 테스트: mock encoder로 matches=true/false 두 케이스를 각각 검증하는 테스트를 1~2개 추가해야 한다.
🧪 matches 테스트 추가 예시
@@
void 유효한_비밀번호이면_암호화해서_객체생성_성공() {
@@
assertThat(password.asString()).isEqualTo("encrypted");
}
+
+ `@Test`
+ void 비밀번호가_일치하면_matches가_true를_반환한다() {
+ // given
+ String value = "loopers123";
+ PasswordEncoder encoder = mock(PasswordEncoder.class);
+ when(encoder.encode(value)).thenReturn("encrypted");
+ when(encoder.matches(value, "encrypted")).thenReturn(true);
+
+ // when
+ Password password = Password.of(value, encoder);
+
+ // then
+ assertThat(password.matches(value, encoder)).isTrue();
+ }🤖 Prompt for AI Agents
In `@apps/commerce-api/src/test/java/com/loopers/domain/user/PasswordTest.java`
around lines 43 - 55, Add unit tests that explicitly verify Password.matches
delegates to the injected PasswordEncoder: create tests that mock
PasswordEncoder.matches to return true and false, construct a Password (via
Password.of or an equivalent factory) using the mock encoder, then assert
Password.matches(input, encoder) (or instance.matches if API is instance method)
returns the same boolean as the mocked encoder; reference Password.matches,
Password.of, PasswordEncoder, and Password.asString to locate the class and
ensure both true and false branches are covered so encoder injection/matching
logic is validated.
1 participant
📌 Summary
🧭 Context & Decision
문제 정의
선택지와 결정
🏗️ Design Overview
변경 범위
주요 컴포넌트 책임
User: 도메인 엔티티 - 사용자 생성 및 비밀번호 변경UserService: 비즈니스 로직 - 회원가입(중복 검사/비밀번호 검증), 내 정보 조회(인증 후 반환), 비밀번호 변경(인증 후 변경)UserController: API 엔드포인드 제공UserRepositoryImpl: 영속화 - JPA 기반 저장/조회BCryptPasswordEncoder: Bcrypt 기반 비밀번호 암호화/검증🔁 Flow Diagram
Main Flow
Exception Flow
sequenceDiagram autonumber participant Client participant UserService participant ControllerAdvice rect rgb(255, 230, 230) Note over Client, ControllerAdvice: 회원가입 예외 alt 로그인ID 중복 UserService->>ControllerAdvice: CoreException (CONFLICT) ControllerAdvice-->>Client: 409 Conflict else 비밀번호에 생년월일 포함 UserService->>ControllerAdvice: CoreException (BAD_REQUEST) ControllerAdvice-->>Client: 400 Bad Request end end rect rgb(255, 235, 235) Note over Client, ControllerAdvice: 내 정보 조회 / 비밀번호 변경 공통 예외 alt 사용자 없음 UserService->>ControllerAdvice: CoreException (NOT_FOUND) ControllerAdvice-->>Client: 404 Not Found else 비밀번호 불일치 UserService->>ControllerAdvice: CoreException (UNAUTHORIZED) ControllerAdvice-->>Client: 401 Unauthorized end end rect rgb(255, 240, 240) Note over Client, ControllerAdvice: 비밀번호 변경 추가 예외 alt 현재 비밀번호와 동일 UserService->>ControllerAdvice: CoreException (BAD_REQUEST) ControllerAdvice-->>Client: 400 Bad Request else 비밀번호에 생년월일 포함 UserService->>ControllerAdvice: CoreException (BAD_REQUEST) ControllerAdvice-->>Client: 400 Bad Request end endPR 요약
변경 목적: User 도메인 모델을 신규 설계하고 회원가입, 내 정보 조회, 비밀번호 변경 3가지 핵심 API를 구현하여 사용자 관리 기능을 완성합니다.
핵심 변경점:
리스크/주의사항:
테스트/검증: 단위 테스트(6개 파일), 통합 테스트(UserServiceIntegrationTest, UserRepositoryImplTest), E2E 테스트(UserApiE2ETest) 등 총 16개 테스트 파일로 회원가입 정상/예외 케이스(중복, 생년월일 포함), 인증 시나리오(미존재, 불일치), 비밀번호 변경 검증(기존 비밀번호 확인, 새 비밀번호 재사용 방지, 생년월일 포함 검사)을 커버합니다.