[volume-1] 회원가입, 내 정보 조회, 비밀번호 변경 구현#29
[volume-1] 회원가입, 내 정보 조회, 비밀번호 변경 구현#29iohyeon wants to merge 42 commits intoLoopers-dev-lab:iohyeonfrom
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: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/ApiControllerAdvice.java (1)
33-115:⚠️ Potential issue | 🟠 Major필수 헤더 누락이 500으로 반환되고 있다
클라이언트 오류(헤더 누락)가 500 상태 코드로 반환되면 운영 지표와 알람이 왜곡되어 장애 판단과 대응이 지연될 수 있다. 현재 MissingRequestHeaderException이 핸들러에 없어 라인 111-115의 전역 Throwable 핸들러에서 INTERNAL_ERROR로 처리된다. 다른 요청 관련 예외들(MissingServletRequestParameterException, HttpMessageNotReadableException, ServerWebInputException)은 BAD_REQUEST로 처리하고 있으므로, 헤더 누락도 동일하게 처리하도록 MissingRequestHeaderException 전용 핸들러를 추가하길 권장한다.
🛠️ 제안 수정안
import org.springframework.web.bind.MissingServletRequestParameterException; +import org.springframework.web.bind.MissingRequestHeaderException; @@ `@ExceptionHandler` public ResponseEntity<ApiResponse<?>> handleBadRequest(MissingServletRequestParameterException e) { String name = e.getParameterName(); String type = e.getParameterType(); String message = String.format("필수 요청 파라미터 '%s' (타입: %s)가 누락되었습니다.", name, type); return failureResponse(CommonErrorType.BAD_REQUEST, message); } + + `@ExceptionHandler` + public ResponseEntity<ApiResponse<?>> handleBadRequest(MissingRequestHeaderException e) { + String name = e.getHeaderName(); + String message = String.format("필수 요청 헤더 '%s'가 누락되었습니다.", name); + return failureResponse(CommonErrorType.BAD_REQUEST, message); + }E2E 테스트 또는 컨트롤러 어드바이스 테스트로 X-Loopers-LoginId, X-Loopers-LoginPw 등 필수 헤더 누락 시 400 상태 코드와 CommonErrorType.BAD_REQUEST가 반환되는지 확인하길 권장한다.
🤖 Fix all issues with AI agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/BirthDate.java`:
- Around line 48-54: The CoreException currently drops the original cause so
BirthDate.parseDate loses the DateTimeParseException; update CoreException to
add a constructor that accepts a Throwable cause and stores it (so getCause()
returns the original), then update BirthDate.parseDate to pass the caught
DateTimeParseException as the cause when throwing CoreException (i.e., throw new
CoreException(UserErrorType.INVALID_BIRTH_DATE, "생년월일은 YYYY-MM-DD 형식이어야 합니다.",
e)); finally add the BirthDateTest case named preservesCauseWhenInvalidFormat to
assert that constructing new BirthDate with an invalid string results in a
CoreException whose getCause() is a DateTimeParseException.
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/PasswordPolicy.java`:
- Around line 19-29: The current validate()/validateBirthDateNotContained
methods can NPE when rawPassword or birthDate is null; add pre-validation in
validate(String rawPassword, LocalDate birthDate) to check if rawPassword is
null/blank or birthDate is null and throw a CoreException with an appropriate
UserErrorType (consistent with existing error handling) before calling
validateBirthDateNotContained; keep validateBirthDateNotContained as-is but
assume non-null inputs. Also add unit tests in PasswordPolicyTest to assert that
null/blank rawPassword and null birthDate produce the expected CoreException.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/user/BCryptPasswordEncryptor.java`:
- Around line 17-20: Add defensive null handling to
BCryptPasswordEncryptor.matches: check if the incoming raw/current password is
null before delegating to BCryptPasswordEncoder.matches and throw a clear
IllegalArgumentException (or a domain-specific exception) with a descriptive
message (e.g., "currentPassword must not be null") so callers get a predictable
error instead of BCrypt's IllegalArgumentException; update or add unit tests for
BCryptPasswordEncryptor.matches to cover null input; alternatively, if you
prefer validation at the service layer, add an explicit null check in
UserService before calling BCryptPasswordEncryptor.matches (but keep the
encryptor-level check to be defensive).
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserJpaRepository.java`:
- Around line 8-11: The repository methods findByLoginIdValue and
existsByLoginIdValue will cause full table scans unless the underlying login_id
column is indexed/unique; update the LoginId value field's `@Column` (the value
property inside the LoginId value object) to include unique=true and
nullable=false or add an index on the User entity via `@Table`(indexes =
`@Index`(name="idx_login_id", columnList="login_id", unique=true)) on the User
class; ensure the actual DB schema migration is created/updated so the login_id
column has the index/unique constraint and then run load tests for login
throughput to validate the improvement.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/auth/AuthV1Dto.java`:
- Around line 28-31: The ChangePasswordRequest record exposes sensitive data
because its implicit toString() will include currentPassword and newPassword;
override the record's toString() in ChangePasswordRequest to avoid printing raw
passwords (e.g., mask or omit currentPassword and newPassword) so
logging/exception paths cannot leak secrets, and implement the override inside
the ChangePasswordRequest record body (reference: ChangePasswordRequest,
currentPassword, newPassword).
- Around line 9-15: The SignupRequest record's default toString() will include
the password field and can leak credentials; override toString inside the
SignupRequest record to exclude or mask the password (e.g., show "*****" or omit
the password field) so logs never contain the plain password. Add a toString()
method in the SignupRequest record body that returns a string with loginId,
name, birthDate, email and a masked or absent password, leaving the record
components and constructor unchanged.
In
`@apps/commerce-api/src/test/java/com/loopers/application/auth/AuthFacadeIntegrationTest.java`:
- Around line 129-196: Add a test in the ChangePassword nested class that covers
the missing rule: when the new password contains the user’s birth date the
changePassword call should throw a CoreException with error type
UserErrorType.PASSWORD_CONTAINS_BIRTH_DATE; implement it by signing up the user
via authFacade.signup (use variables like loginId, currentPassword, birthDate)
and then calling authFacade.changePassword(loginId, currentPassword,
currentPassword, "<passwordContainingBirthDate>") inside assertThrows, and
assert that exception.getErrorType() equals
UserErrorType.PASSWORD_CONTAINS_BIRTH_DATE.
In `@apps/commerce-api/src/test/java/com/loopers/domain/user/BirthDateTest.java`:
- Around line 71-82: The tests createsBirthDate_whenExactlyMinAge and
throwsException_whenUnderMinAge are flakey because they call LocalDate.now()
multiple times; fix both tests by capturing LocalDate now once into a local
variable (e.g. LocalDate today = LocalDate.now()), compute the input string with
today.minusYears(14).format(...) and compute the expected LocalDate with the
same today.minusYears(14) (or today.minusYears(14).minusDays(1) for the
under-min test) and use those captured values when constructing new BirthDate
and asserting, ensuring all references to LocalDate.now() in each test are
replaced with the single captured variable.
In `@apps/commerce-api/src/test/java/com/loopers/domain/user/LoginIdTest.java`:
- Around line 54-65: The test createsLoginId_whenMinLength currently uses a
7-char string ("nahyeon") so it doesn't actually verify the minimum length
boundary; change the test's arranged value variable to a 4-character string
(e.g., "abcd") so the test for LoginId (constructor and getValue) validates the
minimum-length constraint, keeping the test method name
createsLoginId_whenMinLength and class LoginId unchanged; optionally split into
a separate test explicitly for length 4 if you prefer clearer coverage.
In
`@apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceTest.java`:
- Around line 152-170: The test currently asserts only the in-memory password
change on User in changesPassword_whenValidRequest(); ensure persistence is
verified by adding a mock verification or re-fetch: if UserService is
responsible for saving, add a Mockito
verify(userRepository).save(ArgumentMatchers.any(User.class)) after calling
userService.changePassword(user, ...); otherwise convert this to an
integration-style test that calls changePassword, then reloads the user via
userRepository.findById(...) and asserts the persisted password equals
"$2a$10$newHash"; also add an extra scenario to cover a detached-entity case
where the entity is detached before changePassword and assert the DB reflects
the update after reloading.
- Around line 42-58: The signup_whenValidInput test currently asserts returned
fields but doesn't verify critical interactions; update
UserServiceTest.signup_whenValidInput to verify that
passwordEncryptor.encode(...) and userRepository.save(...) are each invoked (use
verify(passwordEncryptor, times(1)).encode(anyString()) and
verify(userRepository, times(1)).save(any(User.class))). Additionally capture
the User passed to save using an ArgumentCaptor<User> and assert that the
captured User.getPassword() equals the encoded value returned by
passwordEncryptor.encode to ensure the persisted password is the encoded hash.
In
`@apps/commerce-api/src/test/java/com/loopers/interfaces/api/AuthV1ApiE2ETest.java`:
- Around line 114-175: Add an E2E test that asserts 401 is returned when header
authentication fails: inside the existing ChangePassword test class add a new
test method (e.g., returns401_whenHeaderAuthFails) that signs up using
validSignupRequest(), builds headers using authHeaders("nahyeon", "wrongPw1!")
with ContentType set to MediaType.APPLICATION_JSON, creates an
AuthV1Dto.ChangePasswordRequest with the current and new password, calls
testRestTemplate.exchange(CHANGE_PW_URL, HttpMethod.PUT, new HttpEntity<>(body,
headers), ApiResponse.class) and asserts the response status is
HttpStatus.UNAUTHORIZED; reference ChangePassword, authHeaders, CHANGE_PW_URL,
AuthV1Dto.ChangePasswordRequest, and testRestTemplate.exchange to locate where
to add the test.
- Around line 112-114: The `@DisplayName` on the ChangePassword nested test class
is incorrect: it says "PATCH /api/v1/auth/password" but the test uses
HttpMethod.PUT in the ChangePassword tests; update the `@DisplayName` value on the
ChangePassword class to "PUT /api/v1/auth/password" (or change the test to use
PATCH if the API is actually PATCH) so the DisplayName matches the HttpMethod
used in the tests (refer to the ChangePassword nested class and the
HttpMethod.PUT usage).
In `@http/http-client.env.json`:
- Around line 4-5: 현재 평문 자격증명이 http-client.env.json에 포함되어 git으로 추적되고 있으므로 이를
분리해야 합니다; 수정 방법은 http-client.env.json에서 민감정보(loginId, loginPw)를 제거하고 대신 새 파일
http-client.private.env.json을 생성하여 해당 민감값을 옮긴 뒤 http-client.private.env.json을
.gitignore에 추가하여 추적 대상에서 제외하세요; 또한 기존 커밋 이력에 남아있는 자격증명은 git filter-branch 또는 BFG
Repo-Cleaner 등으로 제거하고(또는 리포지터리 클린업 절차 수행) 운영팀과 자격증명 재발급 여부를 협의하십시오.
In
`@modules/redis/src/testFixtures/java/com/loopers/testcontainers/RedisTestContainersConfig.java`:
- Around line 7-13: The Docker image tag used in RedisTestContainersConfig is a
floating tag; update the RedisContainer instantiation to use a fixed, validated
patch version (e.g., "redis:7.4.7-alpine") by extracting the image string into a
private static final constant (e.g., REDIS_IMAGE) and replacing
DockerImageName.parse("redis:7-alpine") with DockerImageName.parse(REDIS_IMAGE).
Ensure the constant is used by the redisContainer field and re-run the
integration tests to verify container startup and Redis connectivity.
🧹 Nitpick comments (16)
apps/commerce-api/src/main/java/com/loopers/domain/user/BirthDate.java (1)
21-40: Embeddable 값 객체에 equals/hashCode가 없으면 JPA 변경 감지가 불안정해질 수 있다
운영 환경에서 컬렉션 중복 제거, 캐시 키, 더티 체킹이 값 기반으로 동작하지 않아 예기치 않은 업데이트 누락이나 중복 저장이 발생할 수 있다.
value 기반 equals/hashCode를 추가하여 값 객체로서의 동등성을 보장하는 수정이 필요하다.
추가 테스트로 동일 날짜 두 객체의 equals/hashCode 일치를 검증하는 단위 테스트를 추가하는 것이 좋다.🛠️ 수정안
+import java.util.Objects; @@ public class BirthDate { @@ public LocalDate getValue() { return value; } + + `@Override` + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + BirthDate birthDate = (BirthDate) o; + return Objects.equals(value, birthDate.value); + } + + `@Override` + public int hashCode() { + return Objects.hash(value); + }As per coding guidelines, null 처리, 방어적 복사, 불변성, equals/hashCode/toString 구현 안정성을 점검한다.
apps/commerce-api/src/main/java/com/loopers/domain/user/Email.java (1)
20-66: Embeddable 값 객체에 equals/hashCode가 없어 값 비교가 불안정하다
운영에서 Set/Map 키 또는 JPA 더티 체킹이 값 기반으로 동작하지 않아 중복 저장이나 변경 미감지가 발생할 수 있다.
value 기반 equals/hashCode를 추가하여 값 객체 동등성 보장을 권장한다.
추가 테스트로 동일 이메일 두 객체의 equals/hashCode 일치를 검증하는 단위 테스트를 추가하는 것이 좋다.🛠️ 수정안
+import java.util.Objects; @@ public class Email { @@ public String getValue() { return value; } + + `@Override` + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Email email = (Email) o; + return Objects.equals(value, email.value); + } + + `@Override` + public int hashCode() { + return Objects.hash(value); + }As per coding guidelines, null 처리, 방어적 복사, 불변성, equals/hashCode/toString 구현 안정성을 점검한다.
apps/commerce-api/src/main/java/com/loopers/domain/user/UserName.java (1)
21-60: 값 객체 동등성 보장이 없어 운영 비교 로직이 흔들릴 수 있다
값 객체가 equals/hashCode를 제공하지 않으면 캐시 키, 컬렉션 비교, JPA 변경 감지에서 예기치 않은 누락이나 중복이 발생할 수 있다.
value 기반 equals/hashCode를 추가하여 값 객체로서의 일관성을 확보하는 수정이 필요하다.
추가 테스트로 동일 이름 두 객체의 equals/hashCode 일치를 검증하는 케이스를 추가하는 것이 좋다.🛠️ 수정안
+import java.util.Objects; @@ public class UserName { @@ public String getMaskedValue() { return value.substring(0, value.length() - 1) + "*"; } + + `@Override` + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + UserName userName = (UserName) o; + return Objects.equals(value, userName.value); + } + + `@Override` + public int hashCode() { + return Objects.hash(value); + }As per coding guidelines, null 처리, 방어적 복사, 불변성, equals/hashCode/toString 구현 안정성을 점검한다.
apps/commerce-api/src/main/java/com/loopers/domain/user/Password.java (1)
22-120: 값 객체 비교가 필요해질 경우 동등성 정의가 없어 운영 로직이 불안정해질 수 있다
운영 중 캐시 키, 중복 검사, 테스트 비교에서 값 객체 동등성이 보장되지 않아 예기치 않은 중복이나 비교 실패가 발생할 수 있다.
value 기반 equals/hashCode를 추가해 값 객체 일관성을 확보하는 수정이 필요하다.
추가 테스트로 동일 비밀번호 두 객체의 equals/hashCode 일치를 검증하는 케이스를 추가하는 것이 좋다.🛠️ 수정안
+import java.util.Objects; @@ public class Password { @@ public String getValue() { return 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.hash(value); + }As per coding guidelines, null 처리, 방어적 복사, 불변성, equals/hashCode/toString 구현 안정성을 점검한다.
apps/commerce-api/src/main/java/com/loopers/domain/user/LoginId.java (1)
19-54: 값 객체 equals/hashCode 부재로 비교/캐싱 오류 가능하다.운영 관점에서 값 객체가 equals/hashCode를 제공하지 않으면 컬렉션 조회, 캐시 키, 테스트 비교에서 동일 값이라도 다르게 취급되어 미묘한 버그가 발생할 수 있다.
수정안으로 value 기반 equals/hashCode(toString 포함)를 구현하는 것을 권장한다.
추가 테스트로 LoginIdTest에 동일 value 비교 시 equals/hashCode가 일치함을 검증하는 케이스를 추가해야 한다.🔧 수정 예시
`@Embeddable` public class LoginId { @@ public String getValue() { return value; } + + `@Override` + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof LoginId that)) return false; + return java.util.Objects.equals(value, that.value); + } + + `@Override` + public int hashCode() { + return java.util.Objects.hash(value); + } + + `@Override` + public String toString() { + return value; + } }As per coding guidelines, equals/hashCode/toString 구현 안정성을 점검한다.
apps/commerce-api/src/main/java/com/loopers/domain/user/User.java (1)
32-45: 비밀번호가 해시값임을 명시하지 않으면 평문 저장 위험이 있다.운영 관점에서 User.create/changePassword가 해시 문자열만 받는다는 계약이 명시되지 않으면 상위 계층에서 실수로 평문을 전달해 DB에 저장될 수 있어 보안 사고로 이어질 수 있다.
수정안으로 create/changePassword에 “BCrypt 등 인코딩된 비밀번호만 전달”이라는 Javadoc을 명시하고, 필요하면 메서드명을 encoded 전용으로 더 명확히 하는 방식을 권장한다.
추가 테스트로 UserServiceTest에서 signup/changePassword 시 PasswordEncryptor.encode가 호출되고 저장된 값이 raw와 다름을 검증하는 케이스를 추가해야 한다.Based on learnings, User 엔티티 생성자의 password 파라미터는 BCrypt 해시를 받는다.
apps/commerce-api/src/test/java/com/loopers/domain/user/UserRepositoryIntegrationTest.java (2)
47-55: 테스트 헬퍼의 BCrypt 해시 값이 유효하지 않다.
"$2a$10$encodedPasswordHash"는 실제 BCrypt 해시 형식(60자)이 아니다. 현재 테스트는 통과하지만, 향후 Password VO나 User 엔티티에서 BCrypt 형식 검증이 추가되면 테스트가 깨질 수 있다.운영 관점에서 테스트 데이터는 실제 형식을 따르는 것이 안전하다.
♻️ 유효한 BCrypt 해시 사용 제안
private User createTestUser(String loginIdValue) { return User.create( new LoginId(loginIdValue), - "$2a$10$encodedPasswordHash", + "$2a$10$N9qo8uLOickgx2ZMRZoMye/IjqG4UdRYK4xKJwEqF5p0f1KJ5zJeW", new UserName("홍길동"), new BirthDate("1994-11-15"), new Email(loginIdValue + "@example.com") ); }
57-98: 비밀번호 변경 시 영속성 검증 테스트가 누락되었다.PR 목표에서 "Detached Entity issue for password change" 버그를 수정했다고 언급했다. 해당 버그의 재발 방지를 위한 회귀 테스트가 Repository 통합 테스트에 포함되어야 한다.
운영 관점에서 수정된 버그에 대한 테스트가 없으면 동일 문제가 재발할 위험이 있다.
🧪 업데이트 영속성 테스트 추가 제안
`@Nested` `@DisplayName`("save 메서드는") class Save { // 기존 테스트들... `@Test` `@DisplayName`("기존 사용자의 비밀번호를 변경하면, 변경된 비밀번호가 저장된다") void updatesPasswordCorrectly() { // arrange User user = createTestUser("testuser"); User savedUser = userRepository.save(user); String newEncodedPassword = "$2a$10$newEncodedPasswordHashHere1234567890123456"; // act savedUser.changePassword(newEncodedPassword); userRepository.save(savedUser); // assert Optional<User> reloadedUser = userRepository.findByLoginId("testuser"); assertThat(reloadedUser).isPresent(); assertThat(reloadedUser.get().getPassword()).isEqualTo(newEncodedPassword); } }apps/commerce-api/src/test/java/com/loopers/interfaces/api/UserV1ApiE2ETest.java (1)
53-91: 인증 관련 경계 케이스 테스트가 부족하다.현재 테스트는 "유효한 인증"과 "잘못된 비밀번호"만 검증한다. 보안 관점에서 다음 케이스들이 누락되었다:
- 인증 헤더가 아예 없는 경우
- 존재하지 않는 loginId로 요청하는 경우
- 빈 문자열 헤더로 요청하는 경우
운영 환경에서 악의적 요청 패턴을 테스트하지 않으면 예상치 못한 동작이 발생할 수 있다.
🧪 추가 테스트 케이스 제안
`@DisplayName`("인증 헤더가 없으면, 401 Unauthorized 응답을 받는다.") `@Test` void returns401_whenNoAuthHeaders() { // arrange signup(validSignupRequest()); // act ResponseEntity<ApiResponse> response = testRestTemplate.exchange(ME_URL, HttpMethod.GET, new HttpEntity<>(new HttpHeaders()), ApiResponse.class); // assert assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED); } `@DisplayName`("존재하지 않는 로그인ID로 조회하면, 401 Unauthorized 응답을 받는다.") `@Test` void returns401_whenLoginIdNotExists() { // arrange HttpHeaders headers = authHeaders("nonexistent", "Hx7!mK2@"); // act ResponseEntity<ApiResponse> response = testRestTemplate.exchange(ME_URL, HttpMethod.GET, new HttpEntity<>(headers), ApiResponse.class); // assert assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED); }apps/commerce-api/src/test/java/com/loopers/support/error/CoreExceptionTest.java (1)
22-33: null ErrorType 전달 시 동작 테스트가 없다.
CoreException(null)호출 시errorType.getMessage()에서 NPE가 발생한다. 의도된 동작인지 명시적인 예외를 던질지 결정하고, 해당 케이스를 테스트로 문서화하는 것이 좋다.운영 환경에서 예상치 못한 NPE는 디버깅을 어렵게 한다.
`@Test` `@DisplayName`("ErrorType이 null이면, NullPointerException이 발생한다.") void throwsNPE_whenErrorTypeIsNull() { assertThatThrownBy(() -> new CoreException(null)) .isInstanceOf(NullPointerException.class); }apps/commerce-api/src/test/java/com/loopers/domain/user/LoginIdTest.java (1)
12-36: 과도한 블록 주석은 테스트 의도를 흐린다운영 관점에서 불필요한 장문 주석은 유지보수와 리뷰 속도를 저하시켜 장애 대응 시 맥락 파악을 늦춘다. 수정안으로 Line 12-36의 장문 주석을 삭제하고 간결한 테스트 목적 주석만 유지하길 권장한다. 추가 테스트는 문서 정리만 수행되는 변경이므로 기존 테스트를 그대로 유지하면 충분하다.
🧹 제안 수정안
-/** - - * Red Phase - - * 1. 먼저 유효한 ID 생성 성공 테스트 작성 Happy Case - - * 2. 테스트 실행 → 컴파일 에러 확인 (LoginId 클래스 없음) - - * 3. Red 상태를 진행해봄! - 컴파일 에러 발생 근데 일단 이 컴파일 에러는 TDD에서의 시작이라 - - * 시작이 중요한게 아니라 테스트 케이스를 작성한 다음에 메인 코드가 작성이 잘되는게 테스트 코드 작성의 꽃이다. - - * 테스트 케이스를 작성하다보면 너무 많아지는데 이 상황에서는 AB CD DE ... 무수히 많은 테스트 코드 - - * 그래서 깔끔하고 응집도 높게 집중할 수 있는 테스트 케이스를 작성하고 검증할 수 있는 테스트 케이스를 작성하면 객체 지향을 준수한 - - * 메인 테스트 코드를 만들 수 있다고 생각한다. 테스트 코드의 장점이다. - - * 테스트 커버리지ㄴ를 올리기보단 .. - - * 객체지향적 프로그래밍을 고려해서 테스트 코드를 진행하고싶음 - +/** - * - - * 스순환참조가 발생될 수 있는데 서비스간 함수를 두자 상위 레이어로 파사드란 레이어를 위치한다. - - * A 파사드는 A 서비스를 호출하면서도 의존하면서도 서비스를 의존받을 수 있다. - - * 파사드 패턴이란 무엇일가 - - * - - * 테스트 코드를 작성하기 이전에 설계부터 진행하는게 맞다고 생각해서 일다 나는 설계부터 진행 - - * - - * 검증 로직은 아직 넣지 않음 - - * - - * 껍데기 단계에서는 단순 할당만 수행 - - * → 테스트 작성 후 Red 상태 확인 - - * → 그 다음 검증 로직 추가 (Green) - - - -apps/commerce-api/src/main/java/com/loopers/application/auth/AuthFacade.java (1)
19-22: changePassword의 트랜잭션 경계 검토 필요
authenticate와changePassword가 별도 트랜잭션으로 실행된다.authenticate성공 후changePassword호출 전 사용자가 삭제되거나 비밀번호가 변경되면 예상치 못한 동작이 발생할 수 있다.현재 구조에서는
UserService.changePassword가 내부에서 다시 비밀번호를 검증하므로 큰 문제는 아니나, 두 작업을 단일 트랜잭션으로 묶는 것이 더 안전하다. Facade 레벨에@Transactional을 추가하거나 UserService에 통합 메서드를 고려할 수 있다.apps/commerce-api/src/main/java/com/loopers/infrastructure/user/BCryptPasswordEncryptor.java (1)
10-10: BCrypt strength 설정값 외부화 고려현재 기본 strength(10)를 사용한다. 운영 환경에서 성능과 보안 균형을 조절할 필요가 있을 때 설정값을 외부화(application.yml)하면 재배포 없이 조정할 수 있다.
설정 외부화 예시
+import org.springframework.beans.factory.annotation.Value; + `@Component` public class BCryptPasswordEncryptor implements PasswordEncryptor { - private final BCryptPasswordEncoder encoder = new BCryptPasswordEncoder(); + private final BCryptPasswordEncoder encoder; + + public BCryptPasswordEncryptor(`@Value`("${security.bcrypt.strength:10}") int strength) { + this.encoder = new BCryptPasswordEncoder(strength); + }apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java (1)
9-9: 미사용 import 제거
java.time.LocalDate가 import되어 있으나 이 클래스에서 직접 사용되지 않는다.수정안
-import java.time.LocalDate;apps/commerce-api/src/test/java/com/loopers/application/auth/AuthFacadeIntegrationTest.java (1)
4-4: 와일드카드 import 사용
com.loopers.domain.user.*와일드카드 import가 사용되었다. 명시적 import를 사용하면 의존성이 명확해진다.apps/commerce-api/src/test/java/com/loopers/interfaces/api/AuthV1ApiE2ETest.java (1)
56-64: signup 성공 시 응답 본문 검증 부재201 Created 상태 코드만 검증하고, 응답 본문의
loginId,name,birthDate,응답 본문 검증 추가 예시
`@DisplayName`("유효한 정보로 가입하면, 201 Created와 사용자 정보를 반환한다.") `@Test` void returns201_whenValidRequest() { // act ParameterizedTypeReference<ApiResponse<AuthV1Dto.SignupResponse>> type = new ParameterizedTypeReference<>() {}; ResponseEntity<ApiResponse<AuthV1Dto.SignupResponse>> response = testRestTemplate.exchange(SIGNUP_URL, HttpMethod.POST, new HttpEntity<>(validSignupRequest()), type); // assert assertAll( () -> assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CREATED), () -> assertThat(response.getBody().data().loginId()).isEqualTo("nahyeon"), () -> assertThat(response.getBody().data().name()).isEqualTo("홍길동") ); }
| private static LocalDate parseDate(String rawValue) { | ||
| try { | ||
| return LocalDate.parse(rawValue); | ||
| } catch (DateTimeParseException e) { | ||
| throw new CoreException(UserErrorType.INVALID_BIRTH_DATE, | ||
| "생년월일은 YYYY-MM-DD 형식이어야 합니다."); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "CoreException.java" -type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 163
🏁 Script executed:
find . -name "BirthDate.java" -type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 157
🏁 Script executed:
cd apps/commerce-api && find . -name "*.java" -path "*domain/user*" -type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1182
🏁 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:
cat -n ./apps/commerce-api/src/main/java/com/loopers/domain/user/BirthDate.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2643
🏁 Script executed:
rg "new CoreException" --type java -A 2 -B 2 | head -50Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 5047
🏁 Script executed:
cat -n ./apps/commerce-api/src/test/java/com/loopers/domain/user/BirthDateTest.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 6816
CoreException이 원인(cause)을 지원하도록 수정해야 예외 추적이 가능하다
현재 BirthDate.parseDate에서 DateTimeParseException이 발생하면 원인 정보가 완전히 유실되어, 운영 환경에서 장애 원인 파악 및 대응 시간이 증가한다.
그러나 현재 CoreException 생성자는 cause 매개변수를 받지 않으므로, 단순히 throw new CoreException(..., e)로 수정할 수 없다. CoreException을 다음과 같이 수정하여 cause를 지원해야 한다:
public class CoreException extends RuntimeException {
private final ErrorType errorType;
private final String customMessage;
public CoreException(ErrorType errorType) {
this(errorType, null, null);
}
public CoreException(ErrorType errorType, String customMessage) {
this(errorType, customMessage, null);
}
public CoreException(ErrorType errorType, String customMessage, Throwable cause) {
super(customMessage != null ? customMessage : errorType.getMessage(), cause);
this.errorType = errorType;
this.customMessage = customMessage;
}
}그 후 BirthDate.parseDate에서 다음과 같이 수정한다:
private static LocalDate parseDate(String rawValue) {
try {
return LocalDate.parse(rawValue);
} catch (DateTimeParseException e) {
throw new CoreException(UserErrorType.INVALID_BIRTH_DATE,
- "생년월일은 YYYY-MM-DD 형식이어야 합니다.");
+ "생년월일은 YYYY-MM-DD 형식이어야 합니다.", e);
}
}추가로 BirthDateTest에서 다음 테스트 케이스를 추가하여 원인 정보가 올바르게 보존되는지 검증한다:
`@DisplayName`("잘못된 형식이면, 예외의 원인으로 DateTimeParseException을 포함한다.")
`@Test`
void preservesCauseWhenInvalidFormat() {
CoreException exception = assertThrows(CoreException.class, () -> {
new BirthDate("1994/11/15");
});
assertThat(exception.getCause()).isInstanceOf(DateTimeParseException.class);
}🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/BirthDate.java`
around lines 48 - 54, The CoreException currently drops the original cause so
BirthDate.parseDate loses the DateTimeParseException; update CoreException to
add a constructor that accepts a Throwable cause and stores it (so getCause()
returns the original), then update BirthDate.parseDate to pass the caught
DateTimeParseException as the cause when throwing CoreException (i.e., throw new
CoreException(UserErrorType.INVALID_BIRTH_DATE, "생년월일은 YYYY-MM-DD 형식이어야 합니다.",
e)); finally add the BirthDateTest case named preservesCauseWhenInvalidFormat to
assert that constructing new BirthDate with an invalid string results in a
CoreException whose getCause() is a DateTimeParseException.
| public static void validate(String rawPassword, LocalDate birthDate) { | ||
| validateBirthDateNotContained(rawPassword, birthDate); | ||
| } | ||
|
|
||
| private static void validateBirthDateNotContained(String rawPassword, LocalDate birthDate) { | ||
| String yyyymmdd = birthDate.format(DateTimeFormatter.BASIC_ISO_DATE); | ||
| String yymmdd = yyyymmdd.substring(2); | ||
| String mmdd = yyyymmdd.substring(4); | ||
|
|
||
| if (rawPassword.contains(yyyymmdd) || rawPassword.contains(yymmdd) || rawPassword.contains(mmdd)) { | ||
| throw new CoreException(UserErrorType.PASSWORD_CONTAINS_BIRTH_DATE); |
There was a problem hiding this comment.
null 입력 시 NPE로 500 발생 가능하다.
운영 관점에서 rawPassword 또는 birthDate가 null이면 Line 24~28에서 NPE가 발생해 CoreException 기반의 에러 포맷이 깨질 수 있다.
수정안으로 validate()에서 null/blank를 선검증하고 CoreException으로 일관 처리하는 방식을 권장한다.
추가 테스트로 PasswordPolicyTest에 rawPassword가 null/blank이거나 birthDate가 null인 경우 CoreException 발생을 검증하는 케이스를 추가해야 한다.
🔧 수정 예시
public static void validate(String rawPassword, LocalDate birthDate) {
+ if (rawPassword == null || rawPassword.isBlank()) {
+ throw new CoreException(UserErrorType.INVALID_PASSWORD, "비밀번호는 필수입니다.");
+ }
+ if (birthDate == null) {
+ throw new CoreException(UserErrorType.INVALID_BIRTH_DATE, "생년월일은 필수입니다.");
+ }
validateBirthDateNotContained(rawPassword, birthDate);
}As per coding guidelines, 예외 흐름이 명확한지 점검한다.
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/PasswordPolicy.java`
around lines 19 - 29, The current validate()/validateBirthDateNotContained
methods can NPE when rawPassword or birthDate is null; add pre-validation in
validate(String rawPassword, LocalDate birthDate) to check if rawPassword is
null/blank or birthDate is null and throw a CoreException with an appropriate
UserErrorType (consistent with existing error handling) before calling
validateBirthDateNotContained; keep validateBirthDateNotContained as-is but
assume non-null inputs. Also add unit tests in PasswordPolicyTest to assert that
null/blank rawPassword and null birthDate produce the expected CoreException.
| @Override | ||
| public boolean matches(String rawPassword, String encodedPassword) { | ||
| return encoder.matches(rawPassword, encodedPassword); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# BCryptPasswordEncryptor 관련 테스트 파일 검색
find . -type f -name '*Test*.java' -o -name '*Tests.java' | xargs grep -l "BCryptPasswordEncryptor" 2>/dev/nullRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 69
🏁 Script executed:
#!/bin/bash
# BCryptPasswordEncryptor 파일 위치 및 전체 코드 확인
find . -type f -name "BCryptPasswordEncryptor.java" -exec cat -n {} +Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 875
🏁 Script executed:
#!/bin/bash
# BCryptPasswordEncryptor 호출하는 상위 레이어 확인
rg -B5 -A5 "BCryptPasswordEncryptor|\.matches\(" --glob '*.java' | head -100Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 11415
🏁 Script executed:
#!/bin/bash
# PasswordEncryptor 인터페이스 확인
find . -type f -name "PasswordEncryptor.java" -exec cat -n {} +Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 849
🏁 Script executed:
#!/bin/bash
# UserService를 호출하는 컨트롤러/API 레이어 확인
rg -B10 "\.authenticate\(|\.changePassword\(" --glob '*.java' -A3 | grep -E "(authenticate|changePassword|null|@|Controller)" | head -50Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 7259
🏁 Script executed:
#!/bin/bash
# Password.of() 메서드 확인 - null 체크 포함 여부
rg -A10 "class Password|Password\.of\(" --glob '*.java' | head -60Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 5822
🏁 Script executed:
#!/bin/bash
# Password 클래스 구현 확인
find . -type f -name "Password.java" -path "*/domain/user/*" -exec cat -n {} +Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 5018
🏁 Script executed:
#!/bin/bash
# AuthV1Controller의 changePassword 메서드 전체 확인
rg -B5 -A30 "public ApiResponse.*changePassword" --glob '*.java' -t javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1810
🏁 Script executed:
#!/bin/bash
# changePassword 요청 DTO 확인
rg -B5 -A10 "class.*ChangePasswordRequest|record.*ChangePasswordRequest" --glob '*.java' -t javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1169
BCryptPasswordEncryptor의 null 입력에 대한 방어적 처리 추가 필요
currentPassword(또는 loginPw) 파라미터가 null일 경우 BCryptPasswordEncoder.matches()에서 IllegalArgumentException을 발생시킨다. newPassword는 UserService에서 Password.of()로 검증되지만, currentPassword는 상위 레이어에서 명시적 검증이 이루어지지 않는다. 다음 방식 중 하나로 처리하면 된다:
- BCryptPasswordEncryptor의
matches()메서드에 null 체크 추가 후 적절한 예외 발생 - 또는 UserService에서
matches()호출 전 null 체크 추가 - 추가로 null 입력 시나리오에 대한 단위 테스트 작성
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/user/BCryptPasswordEncryptor.java`
around lines 17 - 20, Add defensive null handling to
BCryptPasswordEncryptor.matches: check if the incoming raw/current password is
null before delegating to BCryptPasswordEncoder.matches and throw a clear
IllegalArgumentException (or a domain-specific exception) with a descriptive
message (e.g., "currentPassword must not be null") so callers get a predictable
error instead of BCrypt's IllegalArgumentException; update or add unit tests for
BCryptPasswordEncryptor.matches to cover null input; alternatively, if you
prefer validation at the service layer, add an explicit null check in
UserService before calling BCryptPasswordEncryptor.matches (but keep the
encryptor-level check to be defensive).
| public interface UserJpaRepository extends JpaRepository<User, Long> { | ||
| Optional<User> findByLoginIdValue(String loginId); | ||
| boolean existsByLoginIdValue(String loginId); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# UserJpaRepository 파일 위치 확인
fd -t f "UserJpaRepository.java"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 171
🏁 Script executed:
# User 엔티티 파일 검색 및 내용 확인
fd -t f "User.java" --path "*/infrastructure/user/*" --exec cat {}Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 318
🏁 Script executed:
# LoginId 값 객체 파일 검색 및 내용 확인
fd -t f "LoginId.java" --exec cat {}Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1507
🏁 Script executed:
# Domain 또는 Entity 패키지에서 User 관련 파일 검색
fd -t f "User.java" --exec head -100 {}Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1272
loginIdValue 컬럼에 대한 인덱스 또는 unique 제약 설정이 필요하다.
findByLoginIdValue와 existsByLoginIdValue는 로그인 및 회원가입 시 매번 호출되는 쿼리이다. 현재 LoginId의 @Column 정의에 unique=true 또는 인덱스 설정이 없으면, 대량 사용자 환경에서 Full Table Scan이 발생하여 성능 병목이 된다.
LoginId 값 객체의 @Column 정의를 다음과 같이 수정하거나, User 엔티티의 @Table에 인덱스를 정의해야 한다:
`@Column`(name = "login_id", unique = true, nullable = false)
private String value;또는 User 엔티티에서:
`@Table`(name = "users", indexes = `@Index`(name = "idx_login_id", columnList = "login_id", unique = true))대량 사용자 환경에서 로그인 성능을 측정하여 인덱스 효과를 검증해야 한다.
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserJpaRepository.java`
around lines 8 - 11, The repository methods findByLoginIdValue and
existsByLoginIdValue will cause full table scans unless the underlying login_id
column is indexed/unique; update the LoginId value field's `@Column` (the value
property inside the LoginId value object) to include unique=true and
nullable=false or add an index on the User entity via `@Table`(indexes =
`@Index`(name="idx_login_id", columnList="login_id", unique=true)) on the User
class; ensure the actual DB schema migration is created/updated so the login_id
column has the index/unique constraint and then run load tests for login
throughput to validate the improvement.
| public record SignupRequest( | ||
| String loginId, | ||
| String password, | ||
| String name, | ||
| String birthDate, | ||
| String email | ||
| ) {} |
There was a problem hiding this comment.
SignupRequest 레코드의 toString() 메서드가 비밀번호를 노출할 수 있다.
Java record의 기본 toString() 구현은 모든 필드를 포함한다. 디버깅이나 에러 로깅 시 SignupRequest 객체가 로그에 출력되면 비밀번호가 평문으로 노출될 수 있다.
운영 환경에서 로그를 통한 민감정보 유출은 보안 감사 대상이다.
🛡️ toString() 오버라이드 제안
public record SignupRequest(
String loginId,
String password,
String name,
String birthDate,
String email
- ) {}
+ ) {
+ `@Override`
+ public String toString() {
+ return "SignupRequest[loginId=" + loginId + ", name=" + name +
+ ", birthDate=" + birthDate + ", email=" + email + "]";
+ }
+ }🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/auth/AuthV1Dto.java`
around lines 9 - 15, The SignupRequest record's default toString() will include
the password field and can leak credentials; override toString inside the
SignupRequest record to exclude or mask the password (e.g., show "*****" or omit
the password field) so logs never contain the plain password. Add a toString()
method in the SignupRequest record body that returns a string with loginId,
name, birthDate, email and a masked or absent password, leaving the record
components and constructor unchanged.
| @DisplayName("유효한 요청이면, 비밀번호가 변경된다.") | ||
| @Test | ||
| void changesPassword_whenValidRequest() { | ||
| // arrange | ||
| User user = User.create( | ||
| new LoginId("nahyeon"), "$2a$10$oldHash", | ||
| new UserName("홍길동"), new BirthDate("1994-11-15"), | ||
| new Email("nahyeon@example.com") | ||
| ); | ||
| when(passwordEncryptor.matches("Hx7!mK2@", "$2a$10$oldHash")).thenReturn(true); | ||
| when(passwordEncryptor.matches("Nw8@pL3#", "$2a$10$oldHash")).thenReturn(false); | ||
| when(passwordEncryptor.encode("Nw8@pL3#")).thenReturn("$2a$10$newHash"); | ||
|
|
||
| // act | ||
| userService.changePassword(user, "Hx7!mK2@", "Nw8@pL3#"); | ||
|
|
||
| // assert | ||
| assertThat(user.getPassword()).isEqualTo("$2a$10$newHash"); | ||
| } |
There was a problem hiding this comment.
비밀번호 변경 성공 테스트가 영속화 여부를 검증하지 않아 재발 위험이 있다.
운영 관점에서 비밀번호 변경이 메모리에서만 반영되면 다음 로그인에서 실패해 즉시 장애로 이어진다. 수정안으로 changePassword 성공 케이스에서 저장 책임이 UserService에 있다면 userRepository.save 호출을 verify로 검증하고, 그렇지 않다면 통합 테스트에서 변경 후 재조회로 영속화 여부를 확인하라. 추가 테스트로 “Detached Entity 상태에서도 변경이 DB에 반영된다”는 시나리오를 포함하라.
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceTest.java`
around lines 152 - 170, The test currently asserts only the in-memory password
change on User in changesPassword_whenValidRequest(); ensure persistence is
verified by adding a mock verification or re-fetch: if UserService is
responsible for saving, add a Mockito
verify(userRepository).save(ArgumentMatchers.any(User.class)) after calling
userService.changePassword(user, ...); otherwise convert this to an
integration-style test that calls changePassword, then reloads the user via
userRepository.findById(...) and asserts the persisted password equals
"$2a$10$newHash"; also add an extra scenario to cover a detached-entity case
where the entity is detached before changePassword and assert the DB reflects
the update after reloading.
| @DisplayName("PATCH /api/v1/auth/password") | ||
| @Nested | ||
| class ChangePassword { |
There was a problem hiding this comment.
@DisplayName HTTP 메서드 오류
@DisplayName("PATCH /api/v1/auth/password")로 표기되어 있으나, 실제 API는 PUT 메서드를 사용한다(Line 127의 HttpMethod.PUT). 테스트 문서와 실제 동작이 불일치한다.
수정안
- `@DisplayName`("PATCH /api/v1/auth/password")
+ `@DisplayName`("PUT /api/v1/auth/password")📝 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.
| @DisplayName("PATCH /api/v1/auth/password") | |
| @Nested | |
| class ChangePassword { | |
| `@DisplayName`("PUT /api/v1/auth/password") | |
| `@Nested` | |
| class ChangePassword { |
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/test/java/com/loopers/interfaces/api/AuthV1ApiE2ETest.java`
around lines 112 - 114, The `@DisplayName` on the ChangePassword nested test class
is incorrect: it says "PATCH /api/v1/auth/password" but the test uses
HttpMethod.PUT in the ChangePassword tests; update the `@DisplayName` value on the
ChangePassword class to "PUT /api/v1/auth/password" (or change the test to use
PATCH if the API is actually PATCH) so the DisplayName matches the HttpMethod
used in the tests (refer to the ChangePassword nested class and the
HttpMethod.PUT usage).
| class ChangePassword { | ||
|
|
||
| @DisplayName("유효한 요청이면, 200 OK 응답을 받는다.") | ||
| @Test | ||
| void returns200_whenValidRequest() { | ||
| // arrange | ||
| signup(validSignupRequest()); | ||
| HttpHeaders headers = authHeaders("nahyeon", "Hx7!mK2@"); | ||
| headers.setContentType(MediaType.APPLICATION_JSON); | ||
| AuthV1Dto.ChangePasswordRequest body = new AuthV1Dto.ChangePasswordRequest("Hx7!mK2@", "Nw8@pL3#"); | ||
|
|
||
| // act | ||
| ResponseEntity<ApiResponse> response = testRestTemplate.exchange( | ||
| CHANGE_PW_URL, HttpMethod.PUT, new HttpEntity<>(body, headers), ApiResponse.class); | ||
|
|
||
| // assert | ||
| assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK); | ||
| } | ||
|
|
||
| @DisplayName("변경 후 새 비밀번호로 인증되고, 이전 비밀번호로는 실패한다.") | ||
| @Test | ||
| void authenticatesWithNewPassword_afterChange() { | ||
| // arrange - 회원가입 + 비밀번호 변경 | ||
| signup(validSignupRequest()); | ||
| HttpHeaders headers = authHeaders("nahyeon", "Hx7!mK2@"); | ||
| headers.setContentType(MediaType.APPLICATION_JSON); | ||
| AuthV1Dto.ChangePasswordRequest body = new AuthV1Dto.ChangePasswordRequest("Hx7!mK2@", "Nw8@pL3#"); | ||
| testRestTemplate.exchange(CHANGE_PW_URL, HttpMethod.PUT, new HttpEntity<>(body, headers), ApiResponse.class); | ||
|
|
||
| // act - 새 비밀번호로 조회 | ||
| HttpHeaders newHeaders = authHeaders("nahyeon", "Nw8@pL3#"); | ||
| ResponseEntity<ApiResponse> newPwResponse = | ||
| testRestTemplate.exchange(ME_URL, HttpMethod.GET, new HttpEntity<>(newHeaders), ApiResponse.class); | ||
|
|
||
| // act - 이전 비밀번호로 조회 | ||
| HttpHeaders oldHeaders = authHeaders("nahyeon", "Hx7!mK2@"); | ||
| ResponseEntity<ApiResponse> oldPwResponse = | ||
| testRestTemplate.exchange(ME_URL, HttpMethod.GET, new HttpEntity<>(oldHeaders), ApiResponse.class); | ||
|
|
||
| // assert | ||
| assertAll( | ||
| () -> assertThat(newPwResponse.getStatusCode()).isEqualTo(HttpStatus.OK), | ||
| () -> assertThat(oldPwResponse.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED) | ||
| ); | ||
| } | ||
|
|
||
| @DisplayName("현재 비밀번호와 동일한 새 비밀번호면, 400 Bad Request 응답을 받는다.") | ||
| @Test | ||
| void returns400_whenSamePassword() { | ||
| // arrange | ||
| signup(validSignupRequest()); | ||
| HttpHeaders headers = authHeaders("nahyeon", "Hx7!mK2@"); | ||
| headers.setContentType(MediaType.APPLICATION_JSON); | ||
| AuthV1Dto.ChangePasswordRequest body = new AuthV1Dto.ChangePasswordRequest("Hx7!mK2@", "Hx7!mK2@"); | ||
|
|
||
| // act | ||
| ResponseEntity<ApiResponse> response = testRestTemplate.exchange( | ||
| CHANGE_PW_URL, HttpMethod.PUT, new HttpEntity<>(body, headers), ApiResponse.class); | ||
|
|
||
| // assert | ||
| assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); | ||
| } |
There was a problem hiding this comment.
changePassword 401 Unauthorized 테스트 누락
헤더 인증(X-Loopers-LoginId, X-Loopers-LoginPw) 실패 시 401 응답을 검증하는 테스트가 없다. AuthFacadeIntegrationTest에는 해당 케이스가 있으나 E2E 레벨에서도 HTTP 상태 코드 검증이 필요하다.
테스트 추가 예시
`@DisplayName`("잘못된 헤더 인증 정보면, 401 Unauthorized 응답을 받는다.")
`@Test`
void returns401_whenHeaderAuthFails() {
// arrange
signup(validSignupRequest());
HttpHeaders headers = authHeaders("nahyeon", "wrongPw1!");
headers.setContentType(MediaType.APPLICATION_JSON);
AuthV1Dto.ChangePasswordRequest body = new AuthV1Dto.ChangePasswordRequest("Hx7!mK2@", "Nw8@pL3#");
// act
ResponseEntity<ApiResponse> response = testRestTemplate.exchange(
CHANGE_PW_URL, HttpMethod.PUT, new HttpEntity<>(body, headers), ApiResponse.class);
// assert
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED);
}🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/test/java/com/loopers/interfaces/api/AuthV1ApiE2ETest.java`
around lines 114 - 175, Add an E2E test that asserts 401 is returned when header
authentication fails: inside the existing ChangePassword test class add a new
test method (e.g., returns401_whenHeaderAuthFails) that signs up using
validSignupRequest(), builds headers using authHeaders("nahyeon", "wrongPw1!")
with ContentType set to MediaType.APPLICATION_JSON, creates an
AuthV1Dto.ChangePasswordRequest with the current and new password, calls
testRestTemplate.exchange(CHANGE_PW_URL, HttpMethod.PUT, new HttpEntity<>(body,
headers), ApiResponse.class) and asserts the response status is
HttpStatus.UNAUTHORIZED; reference ChangePassword, authHeaders, CHANGE_PW_URL,
AuthV1Dto.ChangePasswordRequest, and testRestTemplate.exchange to locate where
to add the test.
http/http-client.env.json
Outdated
| "loginId": "testuser01", | ||
| "loginPw": "Test10395!@" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== 1. http-client.env.json 파일 확인 ==="
if [ -f "http/http-client.env.json" ]; then
echo "파일 존재함:"
cat -n http/http-client.env.json
else
echo "파일을 찾을 수 없음"
fi
echo ""
echo "=== 2. .gitignore 확인 ==="
if [ -f ".gitignore" ]; then
echo ".gitignore 내용:"
cat -n .gitignore
else
echo ".gitignore 파일 없음"
fi
echo ""
echo "=== 3. http-client.private.env.json 확인 ==="
if [ -f "http/http-client.private.env.json" ]; then
echo "private env 파일 존재함"
else
echo "private env 파일 없음"
fi
echo ""
echo "=== 4. Git tracked 상태 확인 ==="
git ls-files | grep -E "http-client.*\.json" || echo "git tracked 파일 없음"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1209
보안 위험: 자격 증명이 버전 관리 시스템에 평문으로 노출된다.
자격 증명이 http-client.env.json에 평문으로 저장되어 있고, 현재 git으로 추적 중이다. 이는 다음과 같은 운영 문제를 야기한다:
- git history에 자격 증명이 영구적으로 저장되어 삭제가 매우 어려움
- 추후 레포지토리 공개 시 자격 증명이 즉시 노출됨
- 팀원이 실수로 프로덕션 환경에서 이 자격 증명을 재사용할 가능성
수정 방법: http-client.private.env.json 파일을 생성하여 민감 정보를 분리하고, .gitignore에 해당 파일을 추가한다.
http-client.private.env.json생성 (git 추적 제외):
{
"local": {
"loginId": "testuser01",
"loginPw": "Test10395!@"
}
}http-client.env.json수정 (git 추적):
{
"local": {
"commerce-api": "http://localhost:8080"
}
}.gitignore에 다음 패턴 추가:
http-client.private.env.json
추가 확인 사항: git history에서 기존 자격 증명 제거 필요. git filter-branch 또는 BFG Repo-Cleaner 사용을 검토하고, 해당 자격 증명의 재설정 여부를 운영팀과 협의한다.
🤖 Prompt for AI Agents
In `@http/http-client.env.json` around lines 4 - 5, 현재 평문 자격증명이
http-client.env.json에 포함되어 git으로 추적되고 있으므로 이를 분리해야 합니다; 수정 방법은
http-client.env.json에서 민감정보(loginId, loginPw)를 제거하고 대신 새 파일
http-client.private.env.json을 생성하여 해당 민감값을 옮긴 뒤 http-client.private.env.json을
.gitignore에 추가하여 추적 대상에서 제외하세요; 또한 기존 커밋 이력에 남아있는 자격증명은 git filter-branch 또는 BFG
Repo-Cleaner 등으로 제거하고(또는 리포지터리 클린업 절차 수행) 운영팀과 자격증명 재발급 여부를 협의하십시오.
| import java.time.Duration; | ||
|
|
||
| @Configuration | ||
| public class RedisTestContainersConfig { | ||
| private static final RedisContainer redisContainer = new RedisContainer(DockerImageName.parse("redis:latest")); | ||
| private static final RedisContainer redisContainer = new RedisContainer( | ||
| DockerImageName.parse("redis:7-alpine").asCompatibleSubstituteFor("redis")) | ||
| .withStartupTimeout(Duration.ofMinutes(2)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "RedisTestContainersConfig.java" modules/redis/Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 178
🏁 Script executed:
cat -n modules/redis/src/testFixtures/java/com/loopers/testcontainers/RedisTestContainersConfig.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1341
🌐 Web query:
redis 7-alpine latest patch version docker official image
💡 Result:
The latest Redis 7.x Alpine tag in the official redis Docker image is:
redis:7.4.7-alpine(and equivalentlyredis:7-alpine/redis:7.4-alpine) [1]- Alpine-pinned variant:
redis:7.4.7-alpine3.21(andredis:7-alpine3.21/redis:7.4-alpine3.21) [1]
Source: Docker Hub “redis” official image tags list [1].
Redis 이미지 태그를 검증된 패치 버전으로 고정해 테스트 재현성을 보장해야 한다.
redis:7-alpine은 변동 태그로서 레지스트리 업데이트 시 CI와 로컬 테스트의 동작이 변할 수 있으며, 장애 재현과 테스트 안정성이 낮아진다. 현재 최신 안정 버전인 redis:7.4.7-alpine으로 고정하고 상수로 분리해 관리하면 재현성을 높일 수 있다. 변경 후에는 통합 테스트를 재실행해 컨테이너 기동과 Redis 연결이 정상임을 확인해야 한다.
수정 예시
+ private static final String REDIS_IMAGE_VERSION = "redis:7.4.7-alpine";
+
private static final RedisContainer redisContainer = new RedisContainer(
- DockerImageName.parse("redis:7-alpine").asCompatibleSubstituteFor("redis"))
+ DockerImageName.parse(REDIS_IMAGE_VERSION).asCompatibleSubstituteFor("redis"))
.withStartupTimeout(Duration.ofMinutes(2));📝 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.
| import java.time.Duration; | |
| @Configuration | |
| public class RedisTestContainersConfig { | |
| private static final RedisContainer redisContainer = new RedisContainer(DockerImageName.parse("redis:latest")); | |
| private static final RedisContainer redisContainer = new RedisContainer( | |
| DockerImageName.parse("redis:7-alpine").asCompatibleSubstituteFor("redis")) | |
| .withStartupTimeout(Duration.ofMinutes(2)); | |
| import java.time.Duration; | |
| `@Configuration` | |
| public class RedisTestContainersConfig { | |
| private static final String REDIS_IMAGE_VERSION = "redis:7.4.7-alpine"; | |
| private static final RedisContainer redisContainer = new RedisContainer( | |
| DockerImageName.parse(REDIS_IMAGE_VERSION).asCompatibleSubstituteFor("redis")) | |
| .withStartupTimeout(Duration.ofMinutes(2)); |
🤖 Prompt for AI Agents
In
`@modules/redis/src/testFixtures/java/com/loopers/testcontainers/RedisTestContainersConfig.java`
around lines 7 - 13, The Docker image tag used in RedisTestContainersConfig is a
floating tag; update the RedisContainer instantiation to use a fixed, validated
patch version (e.g., "redis:7.4.7-alpine") by extracting the image string into a
private static final constant (e.g., REDIS_IMAGE) and replacing
DockerImageName.parse("redis:7-alpine") with DockerImageName.parse(REDIS_IMAGE).
Ensure the constant is used by the redisContainer field and re-run the
integration tests to verify container startup and Redis connectivity.
- 에러코드 단순화 및 @repository 어노테이션 수정
- 패스스루만 수행하는 UserFacade 삭제 - Controller에서 UserInfo.from() 직접 호출로 변경
- Email 정규식을 기본 이메일 형식으로 간소화 - UserName validate()에 길이 검증 로직 추가 (상수만 정의되어 있던 버그 수정)
1 participant
📌 Summary
배경: Round-1 TDD 과제로 User 도메인 기반 회원가입, 비밀번호 변경, 내 정보 조회 3가지 API 구현이 필요했습니다.
기존 레이어드 아키텍처 대신 헥사고날 아키텍처를 직접 경험하고, TDD로 견고한 도메인 모델을 설계하고자 했습니다.
목표:
(1) 헥사고날 아키텍처 적용으로 도메인 계층의 독립성 확보,
(2) Value Object와 Domain Service로 도메인 규칙 표현,
(3) TDD로 각 계층별 책임 검증,
(4) DIP를 통한 인프라 의존성 제거
결과:
Auth/User 패키지 분리로 관심사 분리 달성, PasswordEncryptor 포트로 Spring Security 의존성 제거,
PasswordPolicy 도메인 서비스로 교차 검증 구현, 15개 테스트 파일(단위/통합/E2E)로 각 계층 검증 완료.
통합 테스트에서 Detached Entity 버그를 발견하여 운영 배포 전 수정.
구현 기능
🧭 Context & Decision
문제 정의
현재 동작/제약:
문제(또는 리스크):
성공 기준(완료 정의):
선택지와 결정
1. PasswordEncryptor 위치 선택
고려한 대안:
최종 결정: B 선택 -
PasswordEncryptor인터페이스를 Domain에,BCryptPasswordEncryptorAdapter를 Infrastructure에 배치트레이드오프:
2. PasswordPolicy 책임 분리
고려한 대안:
최종 결정: B 선택 -
PasswordPolicy도메인 서비스로 분리트레이드오프:
3. Detached Entity 문제 해결
발견한 문제:
changePassword()메서드에서user.changePassword()후save()호출 없이 반환 시 DB 미반영최종 결정:
userRepository.save(user)명시적 호출로 변경 내용 영속화트레이드오프:
추후 개선 여지
🏗️ Design Overview
변경 범위
영향 받는 모듈/도메인:
commerce-api모듈 전체 (Interfaces, Application, Domain, Infrastructure 계층)신규 추가:
제거/대체:
주요 컴포넌트 책임
Interfaces Layer (외부 세계 → 내부)
AuthV1Controller/UserV1Controller: HTTP 요청 수신, DTO 검증, Facade 호출, 응답 반환@Tag,@Operation) 문서화Application Layer (오케스트레이션)
AuthFacade: 회원가입, 비밀번호 변경 유스케이스 조율signup(): UserService 호출 → UserInfo DTO 변환changePassword(): 인증 → 비밀번호 변경 트랜잭션 관리UserFacade: 사용자 정보 조회 유스케이스getMyInfo(): loginId로 사용자 조회 → DTO 변환Domain Layer (비즈니스 규칙)
User(Entity): 사용자 집합근, 도메인 로직 캡슐화signup(): 정적 팩토리 메서드, PasswordPolicy 검증 포함changePassword(): 현재 비밀번호 검증 → 새 비밀번호 변경authenticate(): 비밀번호 일치 검증PasswordPolicy(Domain Service): 비밀번호-생년월일 교차 검증UserService(Domain Service): 사용자 생성/조회/변경 조율LoginId,Password,UserName,BirthDate,Email): 불변성, 자가 검증, 도메인 규칙 표현LoginId: 4~20자, 영문+숫자+특수문자(-_)Password: 8~20자, 영문+숫자+특수문자 조합 (암호화된 상태로 저장)UserName: 2~50자BirthDate: 1900-01-01 ~ 오늘, LocalDate 래핑Email: RFC 5322 형식 검증Infrastructure Layer (인프라 어댑터)
UserJpaRepository: JPA 기반 영속성 구현 (Spring Data JPA)BCryptPasswordEncryptorAdapter: BCryptPasswordEncoder 래핑, PasswordEncryptor 포트 구현CustomUserDetailsService: Spring Security 인증을 위한 UserDetailsService 구현🔁 Flow Diagram
Main Flow: 회원가입 (POST /api/v1/auth/signup)
sequenceDiagram autonumber actor Client participant AuthV1Controller participant AuthFacade participant UserService participant PasswordPolicy participant PasswordEncryptor participant UserRepository participant DB Client->>AuthV1Controller: POST /api/v1/auth/signup<br/>{loginId, password, name, birthDate, email} AuthV1Controller->>AuthV1Controller: DTO 검증 (@Valid) AuthV1Controller->>AuthFacade: signup(loginId, password, ...) AuthFacade->>UserService: signup(loginId, password, ...) UserService->>UserService: 중복 체크 (findByLoginId) UserService->>PasswordPolicy: validate(password, birthDate) PasswordPolicy-->>UserService: 검증 완료 (또는 예외) UserService->>PasswordEncryptor: encode(password) PasswordEncryptor-->>UserService: encodedPassword UserService->>UserService: User.signup(..., encodedPassword) Note over UserService: VO들 생성 및 자가 검증<br/>(LoginId, Password, UserName, etc.) UserService->>UserRepository: save(user) UserRepository->>DB: INSERT user DB-->>UserRepository: saved user UserRepository-->>UserService: user UserService-->>AuthFacade: user AuthFacade->>AuthFacade: UserInfo.from(user) AuthFacade-->>AuthV1Controller: UserInfo AuthV1Controller-->>Client: 201 Created<br/>{id, loginId, name, email, ...}Main Flow: 비밀번호 변경 (PUT /api/v1/auth/password)
sequenceDiagram autonumber actor Client participant AuthV1Controller participant AuthFacade participant UserService participant PasswordPolicy participant PasswordEncryptor participant UserRepository participant DB Client->>AuthV1Controller: PUT /api/v1/auth/password<br/>Authorization: Basic {loginId:headerPassword}<br/>{currentPassword, newPassword} AuthV1Controller->>AuthV1Controller: DTO 검증 AuthV1Controller->>AuthFacade: changePassword(loginId, headerPassword, currentPassword, newPassword) AuthFacade->>UserService: authenticate(loginId, headerPassword) UserService->>UserRepository: findByLoginId(loginId) UserRepository->>DB: SELECT user DB-->>UserRepository: user UserRepository-->>UserService: user UserService->>User: authenticate(headerPassword, encryptor) User->>PasswordEncryptor: matches(headerPassword, encodedPassword) PasswordEncryptor-->>User: true/false User-->>UserService: (인증 성공 또는 예외) UserService-->>AuthFacade: user AuthFacade->>UserService: changePassword(user, currentPassword, newPassword) UserService->>User: changePassword(currentPassword, newPassword, encryptor) User->>PasswordEncryptor: matches(currentPassword, encodedPassword) PasswordEncryptor-->>User: true/false User->>PasswordPolicy: validate(newPassword, birthDate) PasswordPolicy-->>User: 검증 완료 User->>PasswordEncryptor: encode(newPassword) PasswordEncryptor-->>User: newEncodedPassword User->>User: password = new Password(newEncodedPassword) UserService->>UserRepository: save(user) Note over UserRepository: Detached Entity 방지를 위해 명시적 save() UserRepository->>DB: UPDATE user SET password=... DB-->>UserRepository: updated UserRepository-->>UserService: user UserService-->>AuthFacade: void AuthFacade-->>AuthV1Controller: void AuthV1Controller-->>Client: 200 OKMain Flow: 내 정보 조회 (GET /api/v1/users/me)
sequenceDiagram autonumber actor Client participant UserV1Controller participant UserFacade participant UserService participant UserRepository participant DB Client->>UserV1Controller: GET /api/v1/users/me<br/>Authorization: Basic {loginId:password} Note over UserV1Controller: Spring Security가 인증 처리<br/>SecurityContext에서 loginId 추출 UserV1Controller->>UserFacade: getMyInfo(loginId) UserFacade->>UserService: findByLoginId(loginId) UserService->>UserRepository: findByLoginId(loginId) UserRepository->>DB: SELECT user WHERE login_id=? DB-->>UserRepository: user UserRepository-->>UserService: Optional<User> UserService-->>UserFacade: user (또는 예외) UserFacade->>UserFacade: UserInfo.from(user) UserFacade-->>UserV1Controller: UserInfo UserV1Controller-->>Client: 200 OK<br/>{id, loginId, name, email, birthDate, ...}Exception Flow: 비밀번호에 생년월일 포함 시
sequenceDiagram autonumber actor Client participant Controller participant PasswordPolicy participant ApiControllerAdvice Client->>Controller: 비밀번호에 "19900101" 포함 Controller->>PasswordPolicy: validate("Pass19900101!", birthDate) PasswordPolicy->>PasswordPolicy: 생년월일 패턴 검사<br/>(YYYYMMDD, YYMMDD, MMDD) PasswordPolicy->>PasswordPolicy: "19900101" 발견 PasswordPolicy-->>Controller: CoreException(PASSWORD_CONTAINS_BIRTH_DATE) Controller-->>ApiControllerAdvice: 예외 전파 ApiControllerAdvice-->>Client: 400 Bad Request<br/>{"code": "USER_004", "message": "비밀번호에 생년월일이 포함되어 있습니다"}🎯 이번 과제에서 도전한 것
헥사고날 아키텍처 첫 도전
기존에 익숙한 Controller → Service → Repository 레이어드 아키텍처를 따라갈 수 있었지만, 이번 기회에 궁금했던 헥사고날 아키텍처를 직접 경험해보고 싶었습니다. 시간이 더 걸리더라도 "왜 이렇게 구조를 나누는가"를 이해하며 진행했습니다.
TDD와 테스트 코드에 진심
테스트로 발견한 버그
통합 테스트 과정에서 비밀번호 변경 시 Detached Entity로 인해 DB에 반영되지 않는 버그를 발견하고 수정했습니다.
테스트 코드가 없었다면 운영에서 발견됐을 문제였습니다.
💬 리뷰 포인트
PasswordEncryptor의 Infrastructure 분리
PasswordPolicy 도메인 서비스 분리
Facade 테스트 범위
✅ Checklist
http/commerce-api/auth-v1.http,user-v1.http)📎 기타 참고 사항
테스트 구성
LoginIdTest,PasswordTest,UserNameTest,EmailTest,BirthDateTestUserServiceTest,PasswordPolicyTestAuthFacadeIntegrationTest,UserFacadeIntegrationTestUserRepositoryIntegrationTestAuthV1ApiE2ETest,UserV1ApiE2ETest학습 포인트
주요 기술적 결정 사항
com.loopers.{interfaces, application, domain, infrastructure}