[refactor & test] 유저 정보 컬렉션 통합 및 테스트 코드 작성(윤성원) #385
Conversation
TODO: 모집 정보 테스트코드를 시간 계산법을 LocalDateTime에서 Instant로 변경 후에 활성화할 것
Zepelown
added
📬 API
## Walkthrough
이번 변경에서는 테스트 태스크 분리(유닛/통합), 사용자 및 클럽 관련 엔티티 구조 개편, 회원가입 및 로그인 로직 개선, 테스트 픽스처 및 커스텀 테스트 어노테이션 도입, 그리고 기존 통합 테스트 삭제 및 유닛 테스트 추가가 이루어졌습니다. 비즈니스 로직과 테스트 코드 모두에서 구조적 변화가 포함되어 있습니다.
## Changes
| 파일/그룹 | 변경 요약 |
|---|---|
| `backend/build.gradle` | JUnit 태그 기반의 `unitTest`, `integrationTest` 태스크 추가 및 테스트용 Lombok 의존성 추가. 기존 `test` 태스크 명시적 선언. |
| `.../PasswordValidator.java` | 비밀번호 정규식 패턴 변경(문자 허용 범위 명확화, 패턴 문자열 오류 포함). |
| `.../user/entity/User.java` | `UserInformation` 필드 추가 및 MongoDB 필드 매핑 어노테이션 적용. |
| `.../user/entity/UserInformation.java` | `@Document` 어노테이션 및 `userId` 필드 제거, `name`과 `phoneNumber`만 받는 생성자 추가. |
| `.../user/payload/request/UserRegisterRequest.java` | 필드 검증 어노테이션을 `@NotBlank`로 변경, 유저ID와 비밀번호 동일성 검증 compact 생성자 추가, `PasswordEncoder`를 활용한 엔티티 변환 방식으로 변경. |
| `.../user/repository/UserInformationRepository.java` | `UserInformationRepository` 인터페이스 삭제. |
| `.../user/service/UserCommandService.java` | `UserInformationRepository` 의존성 및 관련 로직 제거, 회원가입 반환 타입을 `void`에서 `User`로 변경, 패스워드 인코딩 및 유저 생성 방식 변경. |
| `backend/src/test/java/moadong/fixture/ClubFixture.java`<br>`backend/src/test/java/moadong/fixture/UserFixture.java` | 클럽 및 유저 관련 테스트 픽스처 클래스 신규 추가, 테스트 데이터 생성 메서드 제공. |
| `.../unit/club/ClubProfileServiceTest.java` | 클럽 프로필 서비스 유닛 테스트 신규 추가(생성, 업데이트, 권한/존재 예외 등). |
| `.../unit/user/UserLoginTest.java` | 유저 로그인 유닛 테스트 신규 추가(성공/실패 케이스). |
| `.../unit/user/UserRegisterTest.java` | 유저 회원가입 유닛 테스트 신규 추가(정상/유효성 실패 케이스, 파라미터라이즈드 테스트 포함). |
| `.../util/annotations/IntegrationTest.java`<br>`.../util/annotations/UnitTest.java`<br>`.../util/annotations/TestTypeConstants.java` | 커스텀 테스트 어노테이션(`@UnitTest`, `@IntegrationTest`) 및 테스트 타입 상수 클래스 추가. |
| `.../fixture/ClubRequestFixture.java`<br>`.../fixture/UserRequestFixture.java` | 클럽 및 유저 요청 관련 테스트 픽스처 클래스 신규 추가, 요청 객체 생성 메서드 제공. |
| `.../user/service/UserCommandServiceTest.java` | 기존 통합 테스트 클래스 삭제. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User as 사용자
participant Controller as UserController
participant Service as UserCommandService
participant Repo as UserRepository
participant Encoder as PasswordEncoder
participant ClubRepo as ClubRepository
User->>Controller: 회원가입 요청(UserRegisterRequest)
Controller->>Service: registerUser(request)
Service->>Encoder: 비밀번호 인코딩
Service->>Repo: 유저 저장
Service->>ClubRepo: createClub(유저 ID)
Service-->>Controller: User 반환
Controller-->>User: 회원가입 응답
User->>Controller: 로그인 요청(UserLoginRequest)
Controller->>Service: loginUser(request, response)
Service->>Repo: 유저 조회
Service->>Encoder: 패스워드 검증
Service->>ClubRepo: 유저의 클럽 조회
Service-->>Controller: 토큰/클럽ID 반환
Controller-->>User: 로그인 응답Suggested reviewers
|
Test Results64 tests 64 ✅ 3s ⏱️ Results for commit 65e64b5. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🔭 Outside diff range comments (3)
backend/src/main/java/moadong/user/service/UserCommandService.java (2)
50-74:⚠️ Potential issue로그인 로직에서 비밀번호 검증 확인 필요
로그인 로직에서 예외처리가
MongoWriteException만 포착하고 있는데, 이는 주로 저장 작업에서 발생합니다. 로그인 과정에서는 인증 실패에 대한 예외 처리(예:BadCredentialsException)가 누락된 것 같습니다.인증 실패에 대한 적절한 예외 처리를 추가하는 것이 좋겠습니다:
try { Authentication authenticate = authenticationManager.authenticate( new UsernamePasswordAuthenticationToken(userLoginRequest.userId(), userLoginRequest.password())); // ... 기존 코드 ... } catch (MongoWriteException e) { throw new RestApiException(ErrorCode.USER_ALREADY_EXIST); +} catch (BadCredentialsException e) { + throw new RestApiException(ErrorCode.INVALID_CREDENTIALS); }
109-121: 🛠️ Refactor suggestion업데이트 메서드에서의 상태 관리 최적화 필요
update메서드에서 사용자 프로필을 업데이트한 후 새로운 리프레시 토큰을 생성하고 있습니다. 하지만 사용자 객체의 리프레시 토큰도 업데이트해야 일관성이 유지됩니다.아래와 같이 수정이 필요합니다:
user.updateUserProfile(userUpdateRequest.encryptPassword(passwordEncoder)); + RefreshToken newRefreshTokenObj = jwtProvider.generateRefreshToken(user.getUsername()); + user.updateRefreshToken(newRefreshTokenObj); userRepository.save(user); - String newRefreshToken = jwtProvider.generateRefreshToken(user.getUsername()).getToken(); + String newRefreshToken = newRefreshTokenObj.getToken(); ResponseCookie cookie = cookieMaker.makeRefreshTokenCookie(newRefreshToken); response.addHeader("Set-Cookie", cookie.toString());backend/src/main/java/moadong/user/payload/request/UserRegisterRequest.java (1)
43-48: 💡 Verification agent🧩 Analysis chain
UserInformation 엔티티 생성 메서드가 아직 사용되고 있습니다.
toUserInformationEntity메서드가 여전히 존재하지만, PR 설명에 따르면 user와 userinfo 컬렉션을 통합했으므로 이 메서드는 더 이상 필요하지 않을 수 있습니다. 다른 곳에서 사용되지 않는다면 제거를 고려해보세요.다음 스크립트를 실행하여 이 메서드의 사용처를 확인해 보세요:
🏁 Script executed:
#!/bin/bash # toUserInformationEntity 메서드의 사용 여부를 확인합니다. rg "toUserInformationEntity" --type javaLength of output: 184
사용되지 않는
toUserInformationEntity메서드 제거 필요확인 결과 해당 메서드는
UserRegisterRequest클래스 내에 선언만 되어 있고, 레포지토리 내 어디에서도 호출되지 않습니다. PR 설명대로 user와 userinfo 컬렉션을 통합했다면 더 이상 필요하지 않으므로 안전하게 제거를 권장합니다.• 파일:
backend/src/main/java/moadong/user/payload/request/UserRegisterRequest.java
–public UserInformation toUserInformationEntity(String userId) { … }메서드 삭제
🧹 Nitpick comments (7)
backend/src/test/java/moadong/fixture/ClubFixture.java (1)
58-84: Instant로의 시간 계산 마이그레이션에 대한 계획
//ToDo주석에서LocalDateTime에서Instant로 시간 계산법을 변경할 계획이 있음을 확인했습니다.시간 관련 코드를
Instant로 마이그레이션하는 작업은 다음을 포함해야 합니다:
ClubRecruitmentInformation클래스에서 날짜 필드 타입 업데이트- 관련 DTO 및 요청 클래스에서 날짜 필드 타입 업데이트
- 관련 서비스 로직에서 시간 계산 로직 업데이트
이 마이그레이션이 우선순위가 높다면, 해당 주석을 제거하고 이 클래스의 코드를 활성화하는 것이 좋겠습니다. 혹은 별도의 이슈를 생성하여 추적하는 것도 좋은 방법입니다.
또한 테스트용 ID 값(
"club_123","recruit_789")이 고정되어 있어, 병렬 테스트 실행 시 잠재적인 충돌이 발생할 수 있습니다. 동적으로 생성된 ID를 사용하거나 테스트 간 격리를 보장하는 방법을 고려하세요.backend/src/test/java/moadong/util/annotations/UnitTest.java (1)
3-4: 불필요한 빈 줄 제거 필요코드 가독성을 높이기 위해 3-4번 라인의 불필요한 빈 줄을 제거하는 것이 좋습니다.
package moadong.util.annotations; - - import moadong.global.util.TestTypeConstants;backend/src/test/java/moadong/fixture/UserFixture.java (1)
10-14: 사용자 테스트 데이터 상수가 명확하게 정의되었습니다.테스트에 사용할 사용자 데이터를 상수로 정의하여 일관성 있는 테스트 데이터를 보장하고 있습니다. 다만, 상수명에
collect접두사가 붙은 이유가 명확하지 않으니 이름의 의미를 더 명확히 하는 것을 고려해보세요.아래와 같이 상수명을 더 명확하게 수정하는 것을 고려해보세요:
-public static final String collectUserId = "test12345"; -public static final String collectPassword = "test12345@"; -public static final String collectName = "테스터"; -public static final String collectPhoneNumber = "010-1234-5678"; +public static final String DEFAULT_USER_ID = "test12345"; +public static final String DEFAULT_PASSWORD = "test12345@"; +public static final String DEFAULT_NAME = "테스터"; +public static final String DEFAULT_PHONE_NUMBER = "010-1234-5678";backend/src/test/java/moadong/unit/user/UserLoginTest.java (3)
58-58: MockHttpServletResponse 변수는 테스트 메서드 내에서 초기화하는 것이 좋습니다.
realHttpServletResponse는 인스턴스 변수로 선언되어 있어 모든 테스트에서 공유됩니다. 테스트가 병렬로 실행될 경우 이로 인해 문제가 발생할 수 있습니다. 각 테스트 메서드 내에서 지역 변수로 선언하는 것이 좋습니다.- private MockHttpServletResponse realHttpServletResponse = new MockHttpServletResponse();각 테스트 메서드 내부에서 다음과 같이 선언하세요:
MockHttpServletResponse realHttpServletResponse = new MockHttpServletResponse();
42-64: Validator 변수가 사용되지 않습니다.정적
validator변수가 선언되고 초기화되었지만 테스트 메서드 내에서 사용되지 않습니다. 필요하지 않다면 제거하는 것이 좋습니다.- private static Validator validator; @BeforeAll public static void setUp() { - ValidatorFactory factory = Validation.buildDefaultValidatorFactory(); - validator = factory.getValidator(); }
98-112: 테스트 메서드 이름이 테스트 내용과 일치하지 않습니다.테스트 메서드 이름은 "없는 아이디와 비밀번호를 입력 시에 실패한다"이지만, 실제 테스트는 사용자의 클럽을 찾을 수 없을 때 발생하는 예외를 검증합니다. 메서드 이름을 테스트 내용에 맞게 수정하는 것이 좋습니다.
- void 없는_아이디와_비밀번호를_입력_시에_실패한다(){ + void 사용자에_대한_클럽을_찾을_수_없을_때_예외가_발생한다(){backend/src/test/java/moadong/unit/user/UserRegisterTest.java (1)
167-167: 오류 메시지를 수정하세요.비밀번호 검증 테스트에서 실패 메시지가 여전히 "유효하지 않은 userId"를 참조하고 있습니다. 이는 아이디 검증 테스트에서 복사되었을 가능성이 있습니다. 메시지를 적절하게 수정해야 합니다.
- fail("예외나 검증 실패가 발생하지 않았습니다. 유효하지 않은 userId: " + userId); + fail("예외나 검증 실패가 발생하지 않았습니다. 유효하지 않은 password: " + password);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
backend/build.gradle(1 hunks)backend/src/main/java/moadong/global/util/TestTypeConstants.java(1 hunks)backend/src/main/java/moadong/global/validator/PasswordValidator.java(1 hunks)backend/src/main/java/moadong/user/entity/User.java(1 hunks)backend/src/main/java/moadong/user/entity/UserInformation.java(1 hunks)backend/src/main/java/moadong/user/payload/request/UserRegisterRequest.java(1 hunks)backend/src/main/java/moadong/user/repository/UserInformationRepository.java(0 hunks)backend/src/main/java/moadong/user/service/UserCommandService.java(1 hunks)backend/src/test/java/moadong/fixture/ClubFixture.java(1 hunks)backend/src/test/java/moadong/fixture/UserFixture.java(1 hunks)backend/src/test/java/moadong/unit/club/ClubProfileServiceTest.java(1 hunks)backend/src/test/java/moadong/unit/user/UserLoginTest.java(1 hunks)backend/src/test/java/moadong/unit/user/UserRegisterTest.java(1 hunks)backend/src/test/java/moadong/util/annotations/IntegrationTest.java(1 hunks)backend/src/test/java/moadong/util/annotations/UnitTest.java(1 hunks)
💤 Files with no reviewable changes (1)
- backend/src/main/java/moadong/user/repository/UserInformationRepository.java
🧰 Additional context used
🧬 Code Graph Analysis (2)
backend/src/test/java/moadong/unit/club/ClubProfileServiceTest.java (2)
backend/src/test/java/moadong/fixture/ClubFixture.java (1)
ClubFixture(15-85)backend/src/test/java/moadong/fixture/UserFixture.java (1)
UserFixture(10-35)
backend/src/test/java/moadong/unit/user/UserLoginTest.java (1)
backend/src/test/java/moadong/unit/user/UserRegisterTest.java (1)
UnitTest(36-174)
🪛 GitHub Actions: PR Test
backend/src/test/java/moadong/fixture/ClubFixture.java
[error] 40-40: Cannot find symbol: method recruitmentForm(String) in class ClubRecruitmentInformationBuilder
[error] 54-54: Incompatible types: String cannot be converted to Map<String,String>
🔇 Additional comments (21)
backend/src/main/java/moadong/global/util/TestTypeConstants.java (1)
1-10: 테스트 유형 상수 클래스 구현이 좋습니다.상수 정의 및 유틸리티 클래스 구현이 모범 사례를 잘 따르고 있습니다:
- 상수 네이밍 컨벤션(대문자와 언더스코어)
- Lombok의
@NoArgsConstructor(access = AccessLevel.PRIVATE)를 사용하여 인스턴스화 방지- 단위 테스트와 통합 테스트를 구분하는 명확한 목적 제공
이 상수들이 커스텀 테스트 어노테이션과 Gradle 빌드 설정에서 일관되게 사용되어 테스트 실행을 분리하는 것은 좋은 접근 방식입니다.
backend/src/test/java/moadong/util/annotations/IntegrationTest.java (1)
14-20: 깔끔하고 명확한 통합 테스트 어노테이션 구현입니다.이 커스텀 어노테이션은 통합 테스트를 위한 설정을 단순화하고 표준화하는 좋은 방법입니다.
@SpringBootTest와@Tag를 조합하여 통합 테스트 클래스를 쉽게 식별하고 실행할 수 있게 해줍니다.backend/src/test/java/moadong/util/annotations/UnitTest.java (1)
15-21: 단위 테스트 어노테이션 구현이 적절합니다.Mockito 확장을 포함한 단위 테스트 어노테이션 구현이 잘 되어 있습니다.
@Tag와@ExtendWith를 조합하여 단위 테스트를 쉽게 식별하고 필요한 의존성을 모의(mock)할 수 있게 해줍니다.backend/src/main/java/moadong/user/entity/UserInformation.java (2)
15-20: 엔티티 어노테이션 검토 필요
UserInformation클래스에@Entity어노테이션이 있지만, MongoDB의@Document어노테이션이 제거되었습니다. 이제 사용자 엔티티에 임베드될 것으로 보이므로,@Entity대신@Embeddable어노테이션이 더 적절할 수 있습니다.
User엔티티에서 이 클래스가 어떻게 참조되는지 확인해 주세요.
31-34: 생성자 구현이 적절합니다.필요한 필드만 초기화하는 간결한 생성자를 추가한 것은 좋은 접근법입니다. 이는
User엔티티 생성 시UserInformation을 쉽게 초기화할 수 있게 해줍니다.backend/src/main/java/moadong/user/service/UserCommandService.java (1)
40-45: 메서드 반환 타입 변경이 적절합니다.
registerUser메서드의 반환 타입을void에서User로 변경한 것은 좋은 개선입니다. 이렇게 하면 등록된 사용자 정보를 즉시 사용할 수 있어 더 유연한 API가 됩니다. 또한UserRegisterRequest에 비밀번호 인코딩 책임을 위임하여 코드가 더 깔끔해졌습니다.backend/build.gradle (3)
53-57: 테스트 태스크 설명이 명확하게 추가되었습니다.전체 테스트를 실행하는 기본 태스크에 대한 설명이 추가되어 더 명확해졌습니다. 개발자들이 이 태스크의 목적을 쉽게 이해할 수 있게 되었습니다.
59-68: 단위 테스트 태스크가 효과적으로 구성되었습니다.
UnitTest태그를 포함하고IntegrationTest태그를 제외함으로써 명확하게 단위 테스트만 실행할 수 있도록 설정되었습니다. 이는 테스트 실행 속도를 향상시키고 CI/CD 파이프라인에서 테스트 단계를 분리하는 데 유용합니다.
70-78: 통합 테스트 태스크가 적절히 구성되었습니다.통합 테스트를 위한 별도의 태스크를 만들어
IntegrationTest태그를 포함하고UnitTest태그를 제외하는 설정이 잘 되어 있습니다. 이는 더 큰 프로젝트에서 테스트 실행 전략을 관리하는 데 도움이 됩니다.backend/src/main/java/moadong/user/payload/request/UserRegisterRequest.java (3)
17-28: @notblank 어노테이션으로 검증 로직이 개선되었습니다.
@NotNull에서@NotBlank로 변경함으로써 빈 문자열이나 공백만 있는 경우도 유효성 검사에서 걸러낼 수 있게 되었습니다. 이는 사용자 입력 검증의 품질을 향상시킵니다.
29-33: 아이디와 비밀번호 동일 여부 검증 로직이 추가되었습니다.사용자 아이디와 비밀번호가 동일한 경우 보안 취약점이 될 수 있으므로, 이를 방지하는 검증 로직이 추가된 것은 적절합니다. 컴팩트 생성자를 활용한 방식도 코드 가독성을 높입니다.
35-41: 비밀번호 인코딩 책임이 명확하게 할당되었습니다.
toUserEntity메서드가PasswordEncoder를 매개변수로 받아 비밀번호 인코딩 책임을 명확히 하고,UserInformation생성 방식이 단순화되었습니다. 이는 코드의 응집도를 높이고 유지보수성을 개선합니다.backend/src/test/java/moadong/fixture/UserFixture.java (1)
15-23: 사용자 관련 객체 생성 메서드가 잘 구현되었습니다.사용자 등록 요청, 로그인 요청, 사용자 엔티티를 생성하는 메서드가 잘 구현되어 있습니다. 특히
createUser메서드에서UserRegisterRequest의toUserEntity를 활용하는 방식이 중복 코드를 방지하고 일관성을 유지하는 좋은 방법입니다.backend/src/test/java/moadong/unit/club/ClubProfileServiceTest.java (6)
28-38: 테스트 클래스 구조가 잘 설계되었습니다.
@UnitTest어노테이션을 사용하여 단위 테스트임을 명확히 하고, 필요한 종속성이 적절히 모킹되어 있습니다. 테스트에 필요한 상수도 클래스 레벨에서 정의하여 재사용성을 높였습니다.
39-50: 클럽 생성 테스트가 간결하게 작성되었습니다.Given-When-Then 구조를 따라 테스트가 명확하게 작성되었습니다. Mock 객체를 사용하여 저장소 의존성을 격리시킨 것도 좋은 방법입니다.
52-68: 클럽 약력 업데이트 성공 테스트가 잘 구현되었습니다.테스트가 Given-When-Then 패턴을 따라 명확하게 구성되어 있으며, 필요한 모든 의존성이 적절히 모킹되어 있습니다. 업데이트 후 저장 작업이 호출되는지 확인하는 부분도 잘 작성되었습니다.
70-75: 클럽 미존재 시 예외 발생 테스트가 간결하게 작성되었습니다.클럽을 찾을 수 없을 때 예외가 발생하는 상황을 테스트하는 코드가 간결하게 잘 작성되었습니다. 모킹을 사용하여 클럽을 찾을 수 없는 상황을 시뮬레이션하고, 예외가 발생하는지 검증하는 방식이 적절합니다.
77-85: 권한 검증 테스트가 명확하게 작성되었습니다.사용자가 클럽 소유자가 아닌 경우 예외가 발생하는 상황을 테스트하는 코드가 잘 작성되었습니다. 모킹을 사용하여 클럽 소유자가 다른 사용자인 상황을 시뮬레이션하고, 예외가 발생하는지 검증하는 방식이 적절합니다.
88-121: TODO 주석이 추가된 비활성화된 테스트 코드가 있습니다.시간 계산법을
LocalDateTime에서Instant로 변경 후 활성화할 테스트 코드가 있습니다. PR 설명에 언급된 것처럼 이것은 예정된 작업이지만, 이러한 주석 처리된 코드는 가능한 빨리 해결하는 것이 좋습니다.이 TODO 항목에 대한 작업 계획이나 일정이 있는지 확인해 주세요. 해당 작업을 별도의 이슈로 관리하면 추적하기 용이할 수 있습니다.
backend/src/test/java/moadong/unit/user/UserLoginTest.java (1)
66-96: 로그인 성공 테스트가 잘 작성되었습니다!로그인 성공 시나리오에 대한 테스트가 잘 작성되었습니다. 필요한 모든 모킹이 적절하게 설정되었고, 결과에 대한 검증도 올바르게 이루어지고 있습니다. 토큰 생성, 저장소 호출, 응답 헤더 확인 등 핵심 기능을 모두 테스트하고 있습니다.
backend/src/test/java/moadong/unit/user/UserRegisterTest.java (1)
60-86: 회원가입 성공 테스트가 잘 작성되었습니다!회원가입 성공 시나리오에 대한 테스트 코드가 잘 작성되었습니다. Mock 객체 설정, 사용자 등록 요청 생성, 응답 검증 등이 명확하게 구현되어 있습니다. 특히 비밀번호 인코딩 확인과 저장소 호출 검증이 적절히 포함되어 있습니다.
|
|
||
| public class PasswordValidator implements ConstraintValidator<Password, String> { | ||
| private static final Pattern PASSWORD_PATTERN = Pattern.compile("^(?=.*[a-zA-Z])(?=.*\\d)(?=.*[!@#$%^]).{8,20}$"); | ||
| private static final Pattern PASSWORD_PATTERN = Pattern.compile("\"^(?=.*[a-zA-Z])(?=.*\\\\d)(?=.*[!@#$%^])[a-zA-Z\\\\d!@#$%^]{8,20}$\"\n"); |
There was a problem hiding this comment.
정규식 패턴 정의에 오류가 있습니다.
현재 비밀번호 패턴 정의에 다음과 같은 문제가 있습니다:
- 정규식이 escaped 쌍따옴표(
\")로 둘러싸여 있음 - 백슬래시가 이중으로 사용됨 (
\\\\d가\\d여야 함) - 패턴 끝에 줄바꿈 문자가 포함됨
다음과 같이 수정하세요:
- private static final Pattern PASSWORD_PATTERN = Pattern.compile("\"^(?=.*[a-zA-Z])(?=.*\\\\d)(?=.*[!@#$%^])[a-zA-Z\\\\d!@#$%^]{8,20}$\"\n");
+ private static final Pattern PASSWORD_PATTERN = Pattern.compile("^(?=.*[a-zA-Z])(?=.*\\d)(?=.*[!@#$%^])[a-zA-Z\\d!@#$%^]{8,20}$");또한, 유효성 검사 로직이 null/빈 문자열을 유효하다고 간주합니다(항상 true 반환). 이는 @NotBlank와 같은 다른 제약 조건과 함께 사용한다고 가정하는 것 같습니다만, 명시적으로 null/빈 값을 거부하는 것이 더 명확할 수 있습니다.
| @Field("userInformation") | ||
| private UserInformation userInformation; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
UserInformation 필드 추가는 좋지만 프로필 업데이트 메서드 수정이 필요합니다.
userInformation 필드 추가는 user와 userinfo 컬렉션을 통합하는 리팩토링의 일부로 보입니다. 이 접근 방식은 데이터 일관성을 개선할 수 있습니다.
updateUserProfile 메서드가 새로 추가된 userInformation 필드를 업데이트하지 않는 문제가 있습니다. 아래와 같이 메서드를 확장하는 것이 좋겠습니다:
public void updateUserProfile(UserUpdateRequest userUpdateRequest) {
this.userId = userUpdateRequest.userId();
this.password = userUpdateRequest.password();
+ if (userUpdateRequest.userInformation() != null) {
+ this.userInformation = userUpdateRequest.userInformation();
+ }
}또한 UserInformation 필드만 업데이트하는 별도의 메서드를 추가하는 것도 고려하세요:
public void updateUserInformation(UserInformation userInformation) {
this.userInformation = userInformation;
}물론 이는 UserUpdateRequest 클래스가 userInformation 필드를 포함하도록 업데이트되어야 함을 의미합니다.
| @Id | ||
| private String id; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
ID 필드 필요성 검토
이 클래스가 이제 User 엔티티에 임베드된다면, 독립적인 @Id 필드가 필요하지 않을 수 있습니다. 임베디드 객체는 일반적으로 소유 엔티티의 ID를 공유합니다.
🏁 Script executed:
#!/bin/bash
# User 엔티티에서 UserInformation이 어떻게 사용되는지 확인
rg -A 3 -B 3 "UserInformation" backend/src/main/java/moadong/user/entity/User.javaLength of output: 232
추가 정보 확인을 위해 UserInformation.java의 상단 어노테이션과 import 구문을 살펴보겠습니다.
🏁 Script executed:
#!/bin/bash
# UserInformation 클래스 상단 어노테이션 및 import 확인
sed -n '1,50p' backend/src/main/java/moadong/user/entity/UserInformation.javaLength of output: 907
MongoDB 매핑 여부 확인을 위해 User.java 클래스 상단 어노테이션을 확인하겠습니다.
🏁 Script executed:
#!/bin/bash
# User 엔티티의 클래스 레벨 어노테이션 확인
sed -n '1,50p' backend/src/main/java/moadong/user/entity/User.javaLength of output: 1322
UserInformation: 불필요한 JPA 어노테이션 및 ID 필드 제거 검토
UserInformation은 User 도큐먼트 내부에 중첩된 값 객체로 사용되고 있으므로, 별도 식별자나 JPA 엔티티로서의 매핑이 필요하지 않습니다. 아래를 검토해주세요.
- UserInformation.java
- 클래스 레벨의
@Entity어노테이션 제거 jakarta.persistence.Idimport 및@Id private String id;필드 삭제
- 클래스 레벨의
이렇게 하면 UserInformation은 순수한 POJO로 유지되며, MongoDB 하위 문서로 올바르게 매핑됩니다.
| try { | ||
| UserRegisterRequest request = UserFixture.createUserRegisterRequest(userId, password, name, phoneNumber); | ||
| Set<ConstraintViolation<UserRegisterRequest>> violations = validator.validate(request); | ||
| // then | ||
| if (violations.isEmpty()) { | ||
| fail("예외나 검증 실패가 발생하지 않았습니다. 유효하지 않은 userId: " + userId); | ||
| } | ||
| } catch (RestApiException e) { | ||
|
|
||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
예외 처리 로직을 개선하세요.
현재 코드는 RestApiException을 catch하지만 예외에 대한 추가 검증이나 처리가 없습니다. 또한 예외가 발생했을 때와 검증 오류(violations)가 발생했을 때를 구분하여 처리하는 것이 좋습니다.
try {
UserRegisterRequest request = UserFixture.createUserRegisterRequest(userId, password, name, phoneNumber);
Set<ConstraintViolation<UserRegisterRequest>> violations = validator.validate(request);
// then
if (violations.isEmpty()) {
fail("예외나 검증 실패가 발생하지 않았습니다. 유효하지 않은 userId: " + userId);
}
- } catch (RestApiException e) {
-
+ } catch (RestApiException e) {
+ // 예외 메시지나 코드를 검증합니다
+ assertThat(e.getErrorCode()).isNotNull();
}📝 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.
| try { | |
| UserRegisterRequest request = UserFixture.createUserRegisterRequest(userId, password, name, phoneNumber); | |
| Set<ConstraintViolation<UserRegisterRequest>> violations = validator.validate(request); | |
| // then | |
| if (violations.isEmpty()) { | |
| fail("예외나 검증 실패가 발생하지 않았습니다. 유효하지 않은 userId: " + userId); | |
| } | |
| } catch (RestApiException e) { | |
| } | |
| try { | |
| UserRegisterRequest request = UserFixture.createUserRegisterRequest(userId, password, name, phoneNumber); | |
| Set<ConstraintViolation<UserRegisterRequest>> violations = validator.validate(request); | |
| // then | |
| if (violations.isEmpty()) { | |
| fail("예외나 검증 실패가 발생하지 않았습니다. 유효하지 않은 userId: " + userId); | |
| } | |
| } catch (RestApiException e) { | |
| // 예외 메시지나 코드를 검증합니다 | |
| assertThat(e.getErrorCode()).isNotNull(); | |
| } |
| // when | ||
| try { | ||
| UserRegisterRequest request = UserFixture.createUserRegisterRequest(userId, password, name, phoneNumber); | ||
| Set<ConstraintViolation<UserRegisterRequest>> violations = validator.validate(request); | ||
| // then | ||
| if (violations.isEmpty()) { | ||
| fail("예외나 검증 실패가 발생하지 않았습니다. 유효하지 않은 userId: " + userId); | ||
| } | ||
| } catch (RestApiException e) { | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
비밀번호 검증 테스트의 예외 처리 구조를 개선하세요.
회원가입시_유저_비밀번호가_조건에_맞지_않으면_실패한다 테스트에서도 예외 처리 구조를 개선하는 것이 좋습니다. 현재는 예외를 잡지만 추가적인 검증이 없습니다.
try {
UserRegisterRequest request = UserFixture.createUserRegisterRequest(userId, password, name, phoneNumber);
Set<ConstraintViolation<UserRegisterRequest>> violations = validator.validate(request);
// then
if (violations.isEmpty()) {
fail("예외나 검증 실패가 발생하지 않았습니다. 유효하지 않은 password: " + password);
}
- } catch (RestApiException e) {
-
+ } catch (RestApiException e) {
+ // 예외 메시지나 코드를 검증합니다
+ assertThat(e.getErrorCode()).isNotNull();
}📝 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.
| // when | |
| try { | |
| UserRegisterRequest request = UserFixture.createUserRegisterRequest(userId, password, name, phoneNumber); | |
| Set<ConstraintViolation<UserRegisterRequest>> violations = validator.validate(request); | |
| // then | |
| if (violations.isEmpty()) { | |
| fail("예외나 검증 실패가 발생하지 않았습니다. 유효하지 않은 userId: " + userId); | |
| } | |
| } catch (RestApiException e) { | |
| // when | |
| try { | |
| UserRegisterRequest request = UserFixture.createUserRegisterRequest(userId, password, name, phoneNumber); | |
| Set<ConstraintViolation<UserRegisterRequest>> violations = validator.validate(request); | |
| // then | |
| if (violations.isEmpty()) { | |
| fail("예외나 검증 실패가 발생하지 않았습니다. 유효하지 않은 userId: " + userId); | |
| } | |
| } catch (RestApiException e) { | |
| // 예외 메시지나 코드를 검증합니다 | |
| assertThat(e.getErrorCode()).isNotNull(); | |
| } |
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
backend/src/test/java/moadong/unit/club/ClubProfileServiceTest.java (2)
39-50: 클럽 생성 테스트가 적절히 구현되었습니다클럽 생성 테스트는 간단하고 목적에 맞게 구현되었습니다. 다만, 저장된 Club 객체의 속성이 요청과 일치하는지 추가로 검증하면 더 완벽할 것 같습니다.
테스트 강화를 위해 다음과 같이 ArgumentCaptor를 사용하여 저장된 Club 객체의 속성을 검증하는 방법을 고려해보세요:
@Test public void 정상적으로_클럽을_생성한다() { // Given ClubCreateRequest request = new ClubCreateRequest("테스트", "카테고리", "분과"); when(clubRepository.save(any())).thenReturn(mock(Club.class)); // When clubProfileService.createClub(request); // Then verify(clubRepository, times(1)).save(any(Club.class)); + ArgumentCaptor<Club> clubCaptor = ArgumentCaptor.forClass(Club.class); + verify(clubRepository).save(clubCaptor.capture()); + Club savedClub = clubCaptor.getValue(); + assertEquals("테스트", savedClub.getName()); + assertEquals("카테고리", savedClub.getCategory()); + assertEquals("분과", savedClub.getDivision()); }
87-120: TODO 주석 처리된 테스트에 대한 조언LocalDateTime에서 Instant로 변경 후 활성화할 예정인 테스트 코드에 대한 TODO 주석이 있습니다. 이러한 변경을 계획하고 미리 테스트 케이스를 작성해둔 점은 좋습니다. 하지만 주석 처리된 코드보다는 이슈 트래킹 시스템에 작업 항목으로 등록하는 것이 더 나을 수 있습니다.
완전히 주석 처리하기보다,
@Disabled어노테이션과 함께 비활성화 이유를 명시하는 방법을 고려해보세요:- // ToDo: 시간 계산법을 LocalDateTime에서 Instant로 변경 후에 활성화할 것 - // @Test - // void updateClubRecruitmentInfo_WithDates_SchedulesRecruitment() { + @Test + @Disabled("시간 계산법을 LocalDateTime에서 Instant로 변경 후에 활성화할 것") + void updateClubRecruitmentInfo_WithDates_SchedulesRecruitment() {이렇게 하면 테스트 실행 시 비활성화된 테스트가 스킵되지만, 테스트 보고서에는 포함되어 미완료 작업을 추적하기 용이합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/src/test/java/moadong/fixture/ClubFixture.java(1 hunks)backend/src/test/java/moadong/unit/club/ClubProfileServiceTest.java(1 hunks)backend/src/test/java/moadong/user/service/UserCommandServiceTest.java(0 hunks)
💤 Files with no reviewable changes (1)
- backend/src/test/java/moadong/user/service/UserCommandServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/src/test/java/moadong/fixture/ClubFixture.java
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/src/test/java/moadong/unit/club/ClubProfileServiceTest.java (2)
backend/src/test/java/moadong/fixture/ClubFixture.java (1)
ClubFixture(16-83)backend/src/test/java/moadong/fixture/UserFixture.java (1)
UserFixture(10-35)
🔇 Additional comments (4)
backend/src/test/java/moadong/unit/club/ClubProfileServiceTest.java (4)
28-38: 깔끔한 테스트 클래스 구조 👍클래스 구조가 잘 정리되어 있고, 필요한 의존성을 명확하게 모킹하셨습니다.
@UnitTest어노테이션을 사용하여 단위 테스트임을 명시하신 것이 좋습니다. 상수값도 클래스 상단에 정의하여 가독성이 향상되었습니다.
51-67: 클럽 정보 업데이트 테스트가 잘 구현되었습니다클럽 정보 업데이트 테스트가 잘 구현되었습니다. 정상 케이스에 대한 검증이 적절히 이루어지고 있습니다. ClubFixture를 활용하여 테스트 데이터를 생성하는 접근 방식이 좋습니다.
69-74: 예외 케이스 테스트가 적절히 구현되었습니다클럽이 존재하지 않는 경우에 대한 예외 처리 테스트가 잘 구현되었습니다.
assertThrows를 사용하여 예외 발생을 검증하는 방법이 적절합니다.
76-84: 권한 검증 테스트가 잘 구현되었습니다권한이 없는 사용자가 클럽 정보를 업데이트하려고 할 때 예외가 발생하는지 검증하는 테스트가 잘 구현되었습니다.
Due-IT
left a comment
Due-IT
left a comment
There was a problem hiding this comment.
고생많으셨습니다! 테스트 코드 부분에 함께 맞춰가야 할 부분이 조금 있는것 같네요 👍
| import lombok.NoArgsConstructor; | ||
|
|
||
| @NoArgsConstructor(access = AccessLevel.PRIVATE) | ||
| public class TestTypeConstants { |
There was a problem hiding this comment.
해당 어노테이션을 프로덕션 패키지내에 정의한 이유가 있나요?
There was a problem hiding this comment.
import 문제가 있어서 프로덕션에 넣어뒀는데 다시 리팩토링하였습니다.
|
|
||
| public class PasswordValidator implements ConstraintValidator<Password, String> { | ||
| private static final Pattern PASSWORD_PATTERN = Pattern.compile("^(?=.*[a-zA-Z])(?=.*\\d)(?=.*[!@#$%^]).{8,20}$"); | ||
| private static final Pattern PASSWORD_PATTERN = Pattern.compile("\"^(?=.*[a-zA-Z])(?=.*\\\\d)(?=.*[!@#$%^])[a-zA-Z\\\\d!@#$%^]{8,20}$\"\n"); |
There was a problem hiding this comment.
기존엔 123456a@ 과 같이 끝에 특수문자가 들어간 경우를 검증을 못했는데 이를 변경했습니다.
There was a problem hiding this comment.
이런 정규 표현식은 주석을 달아주는게 좋아요. 주석 + 테스트까지
어쩌다가 이런 누추한 곳에 안대장님이..
감사합니다 반영해보겠습니다
| public static UserRegisterRequest createUserRegisterRequest(String userId, String password, String name, String phoneNumber) { | ||
| return new UserRegisterRequest(userId, password, name, phoneNumber); | ||
| } | ||
| public static UserLoginRequest createUserLoginRequest(String userId, String password){ | ||
| return new UserLoginRequest(userId,password); | ||
| } | ||
| public static User createUser(PasswordEncoder passwordEncoder,String userId, String password, String name, String phoneNumber){ | ||
| return new UserRegisterRequest(userId, password, name, phoneNumber).toUserEntity(passwordEncoder); | ||
| } |
There was a problem hiding this comment.
테스트 코드에서 요청용 DTO를 명시적으로 생성하는 것이, 테스트를 명세서처럼 활용하는 데 도움이 된다고 생각합니다. 만약 이와 같이 사용한다 하더라도 테스트를 위한 User모델을 만드는 UserFixture에 위치하는건조금 이상하다고 생각되네요
There was a problem hiding this comment.
이전에 사용했던 테스트코드인데 변수를 받아서 설정하는것까지 사용하고싶다면 기본값을 넣고 builder를 리턴하여 테스트코드 내에서 수정할 수 있도록 만드는 건 어떨까요? DataBuilder패턴이라고 하더라고요
There was a problem hiding this comment.
UserFixture에 분리해서 따로 두도록 하겠습니다.
record class로 구현을 했어서 builder를 추가로 쓰는 건 과한 것 같습니다.
회의에서 record class를 아예 다 builder로 바꿀지 말지를 토론해봐야겠네요.
| public static Club createClub() { | ||
| return Club.builder() | ||
| .name("테스트 동아리") | ||
| .category("학술") | ||
| .division("프로그래밍") | ||
| .clubRecruitmentInformation(createRecruitmentInfo()) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
제안과 같이 mock을 활용하면 객체의 수정할 수 없는 부분도 테스트 가능하고, 테스트를 위해 프로덕션 코드가 수정될 가능성이 많이 줄어듭니다!
| public static Club createClub() { | |
| return Club.builder() | |
| .name("테스트 동아리") | |
| .category("학술") | |
| .division("프로그래밍") | |
| .clubRecruitmentInformation(createRecruitmentInfo()) | |
| .build(); | |
| } | |
| public static Club createClub(String clubId, String name) { | |
| Club club = mock(Club.class); | |
| when(club.getId()).thenReturn(clubId); | |
| when(club.getName()).thenReturn(name); | |
| return club; | |
| } |
| } | ||
|
|
||
|
|
||
| // ToDo: 시간 계산법을 LocalDateTime에서 Instant로 변경 후에 활성화할 것 |
There was a problem hiding this comment.
아래 주석처리된 코드부분에서 날짜/시간 생성시에만 ZonedDateTime.now(ZoneId.of("Asia/Seoul")).toLocalDateTime(); 을 통해 생성후 사용하시면 될것 같습니다! Instant를 사용하는건 글로벌 서비스로 확장될 가능성이 있을때 변경하여도 될것 같네요
There was a problem hiding this comment.
테스트에서 문제가 발생해서 Instant로 수정하는 거 아니었나요?
| @ParameterizedTest | ||
| @ValueSource(strings = { |
PororoAndFriends
left a comment
PororoAndFriends
left a comment
There was a problem hiding this comment.
고생하셨습니다! @tag를 통해 단위테스트와 통합테스트를 구분하는 부분은PR 머지 후에 제 테스트코드에도 추가해야겠네요 😊
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
backend/src/test/java/moadong/fixture/UserFixture.java (1)
10-13: 상수 이름 관례 개선 제안상수 이름에 사용된 "collect" 접두사의 의미가 명확하지 않습니다. 일반적인 Java 상수 네이밍 컨벤션에 따라
DEFAULT_USER_ID,TEST_USER_ID또는SAMPLE_USER_ID와 같이 더 직관적인 이름을 사용하는 것이 코드의 가독성을 높일 수 있습니다.- public static final String collectUserId = "test12345"; - public static final String collectPassword = "test12345@"; - public static final String collectName = "테스터"; - public static final String collectPhoneNumber = "010-1234-5678"; + public static final String DEFAULT_USER_ID = "test12345"; + public static final String DEFAULT_PASSWORD = "test12345@"; + public static final String DEFAULT_NAME = "테스터"; + public static final String DEFAULT_PHONE_NUMBER = "010-1234-5678";backend/src/test/java/moadong/fixture/ClubRequestFixture.java (1)
23-49: 주석 처리된 코드와 TODO에 대한 계획 명확화 필요현재
LocalDateTime에서Instant로 변경을 위해 코드가 주석 처리되어 있습니다. 이 변경에 대한 일정이나 이슈 추적 번호를 TODO 주석에 포함시키면 팀원들이 변경 계획을 더 명확하게 이해할 수 있을 것입니다. 또한, 주석 처리된 코드를 위한 import 문(Instant,ChronoUnit등)이 누락되어 있으므로, 코드가 활성화될 때 추가해야 합니다.- //ToDo: 시간 계산법을 LocalDateTime에서 Instant로 변경 후에 활성화할 것 + //ToDo: 시간 계산법을 LocalDateTime에서 Instant로 변경 후에 활성화할 것 (이슈 #XXX, 예상 완료일: YYYY-MM-DD)주석 처리된 코드를 위한 import 문도 추가해야 합니다:
import java.time.Instant; import java.time.temporal.ChronoUnit; import moadong.club.payload.request.ClubRecruitmentInfoUpdateRequest;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/src/test/java/moadong/fixture/ClubFixture.java(1 hunks)backend/src/test/java/moadong/fixture/ClubRequestFixture.java(1 hunks)backend/src/test/java/moadong/fixture/UserFixture.java(1 hunks)backend/src/test/java/moadong/fixture/UserRequestFixture.java(1 hunks)backend/src/test/java/moadong/unit/club/ClubProfileServiceTest.java(1 hunks)backend/src/test/java/moadong/unit/user/UserRegisterTest.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- backend/src/test/java/moadong/fixture/UserRequestFixture.java
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/src/test/java/moadong/fixture/ClubFixture.java
- backend/src/test/java/moadong/unit/club/ClubProfileServiceTest.java
- backend/src/test/java/moadong/unit/user/UserRegisterTest.java
🔇 Additional comments (2)
backend/src/test/java/moadong/fixture/UserFixture.java (1)
15-17: 메서드의 기능이 적절하게 구현되었습니다.
PasswordEncoder를 통해 비밀번호를 안전하게 인코딩하면서 사용자 엔티티를 생성하는 로직이 잘 구현되었습니다.UserRegisterRequest의toUserEntity메서드를 활용하여 변환하는 접근 방식도 적절합니다.backend/src/test/java/moadong/fixture/ClubRequestFixture.java (1)
9-22: 테스트 픽스처 메서드가 잘 구현되었습니다.
createValidClubInfoRequest메서드가 명확하고 적절한 테스트 데이터와 함께 잘 구현되었습니다. 테스트에 필요한 모든 필드가 적절한 샘플 값으로 초기화되어 있습니다.
| public static CustomUserDetails createUserDetails(String userId) { | ||
| return new CustomUserDetails( | ||
| User.builder() | ||
| .id(userId) | ||
| .userInformation(new UserInformation(collectUserId, collectPhoneNumber)) | ||
| .password(collectPassword) | ||
| .build() | ||
| ); | ||
| } |
There was a problem hiding this comment.
매개변수 활용 오류 수정 필요
createUserDetails 메서드에서 userId 매개변수를 받고 있지만, 실제로는 이 값을 제대로 활용하지 않고 있습니다. 23번 라인에서 userInformation을 생성할 때 userId 대신 고정된 상수 collectUserId를 사용하고 있습니다.
public static CustomUserDetails createUserDetails(String userId) {
return new CustomUserDetails(
User.builder()
.id(userId)
- .userInformation(new UserInformation(collectUserId, collectPhoneNumber))
+ .userInformation(new UserInformation(collectName, collectPhoneNumber))
.password(collectPassword)
.build()
);
}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
backend/src/main/java/moadong/global/validator/PasswordValidator.java (1)
13-17: 🛠️ Refactor suggestion
isValid메서드가 null/빈 문자열을 유효하게 처리합니다.현재 구현에서는 null이나 빈 문자열이 유효한 비밀번호로 간주됩니다. 그러나 비밀번호는 일반적으로 필수 입력 값이어야 합니다.
다음과 같이 수정을 고려해보세요:
@Override public boolean isValid(String s, ConstraintValidatorContext constraintValidatorContext) { if (s == null || s.isEmpty()) { - return true; + return false; } return PASSWORD_PATTERN.matcher(s).matches(); }또는
@NotBlank어노테이션을 함께 사용하여 입력값의 필수 여부를 검증하는 방법도 있습니다.
♻️ Duplicate comments (1)
backend/src/main/java/moadong/global/validator/PasswordValidator.java (1)
10-11: 정규식 패턴 정의에 오류가 있습니다.현재 비밀번호 패턴 정의에 다음과 같은 문제가 있습니다:
- 정규식이 escaped 쌍따옴표(
\")로 둘러싸여 있음- 백슬래시가 이중으로 사용됨 (
\\\\d가\\d여야 함)- 패턴 끝에 줄바꿈 문자가 포함됨
다음과 같이 수정하세요:
- // 8 ~ 20자이고, 반드시 한 개의 알파벳, 한 개의 숫자, !@#$%^ 중 한 개가 모두 들어 갔는지 확인 - private static final Pattern PASSWORD_PATTERN = Pattern.compile("\"^(?=.*[a-zA-Z])(?=.*\\\\d)(?=.*[!@#$%^])[a-zA-Z\\\\d!@#$%^]{8,20}$\"\n"); + // 8 ~ 20자이고, 반드시 한 개의 알파벳, 한 개의 숫자, !@#$%^ 중 한 개가 모두 들어 갔는지 확인 + private static final Pattern PASSWORD_PATTERN = Pattern.compile("^(?=.*[a-zA-Z])(?=.*\\d)(?=.*[!@#$%^])[a-zA-Z\\d!@#$%^]{8,20}$");
🧹 Nitpick comments (1)
backend/src/main/java/moadong/global/validator/PasswordValidator.java (1)
9-18: 정규 표현식 테스트 코드 추가를 권장합니다.복잡한 정규 표현식은 의도한 대로 동작하는지 확인하기 위해 테스트 코드를 작성하는 것이 좋습니다. 다양한 케이스(유효한 비밀번호, 너무 짧은 비밀번호, 특수문자가 없는 비밀번호 등)에 대한 테스트를 추가하여 유효성 검사 로직이 올바르게 작동하는지 확인하세요.
@Test void testPasswordValidation() { PasswordValidator validator = new PasswordValidator(); // 유효한 비밀번호 assertTrue(validator.isValid("Password1!", null)); // 너무 짧은 비밀번호 assertFalse(validator.isValid("Pas1!", null)); // 특수문자가 없는 비밀번호 assertFalse(validator.isValid("Password1", null)); // 숫자가 없는 비밀번호 assertFalse(validator.isValid("Password!", null)); // 알파벳이 없는 비밀번호 assertFalse(validator.isValid("12345678!", null)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/src/main/java/moadong/global/validator/KoreanValidator.java(1 hunks)backend/src/main/java/moadong/global/validator/PasswordValidator.java(1 hunks)backend/src/main/java/moadong/global/validator/PhoneNumberValidator.java(1 hunks)backend/src/main/java/moadong/global/validator/UserIdValidator.java(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- backend/src/main/java/moadong/global/validator/KoreanValidator.java
- backend/src/main/java/moadong/global/validator/UserIdValidator.java
- backend/src/main/java/moadong/global/validator/PhoneNumberValidator.java
🔇 Additional comments (1)
backend/src/main/java/moadong/global/validator/PasswordValidator.java (1)
10-10: 주석 추가에 감사합니다.정규식의 의도를 명확히 설명하는 주석을 추가해주셔서 코드 가독성이 향상되었습니다. 이는 좋은 개발 관행입니다.
5 participants
#️⃣연관된 이슈
#368
📝작업 내용
리팩토링
테스트 관련
차후에 해야할 작업
모집 정보 테스트코드를 시간 계산법을 LocalDateTime에서 Instant로 변경 후에 활성화할 것
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit
신규 기능
unitTest,integrationTest) 및 관련 어노테이션(@UnitTest,@IntegrationTest)을 추가했습니다.버그 수정
리팩터링
기타