feature/#98 User 단위 및 통합 테스트코드 구현 #101
Conversation
LeeHanEum
added
✨feature
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough이번 변경 사항은 여러 파일에서의 수정 및 추가 작업을 포함합니다. Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Outside diff range and nitpick comments (31)
aics-domain/src/main/java/kgu/developers/domain/user/exception/DeptCodeNotValidException.java (1)
7-11: 예외 메시지 추가를 고려해보세요.현재 구현은 기본적인 예외 코드만 전달하고 있습니다. 문제가 발생한 학과 코드 값을 포함하는 상세 메시지를 추가하면 디버깅에 도움이 될 것 같습니다.
다음과 같이 수정하는 것을 제안드립니다:
public class DeptCodeNotValidException extends CustomException { + public DeptCodeNotValidException(String deptCode) { + super(DEPT_CODE_NOT_VALID, String.format("유효하지 않은 학과 코드입니다: %s", deptCode)); + } - public DeptCodeNotValidException() { - super(DEPT_CODE_NOT_VALID); - } }aics-domain/build.gradle (1)
14-16: 테스트 픽스처 구현이 적절히 설정되었습니다.테스트 픽스처 의존성이 메인 구현 의존성과 일치하도록 잘 구성되어 있습니다. 이는 PR의 목표인 FakeRepository를 활용한 테스트 구현을 위한 좋은 기반이 될 것입니다.
다음 사항들을 고려해보시면 좋을 것 같습니다:
- 테스트 픽스처의 재사용성을 높이기 위해 문서화를 추가하는 것을 고려해보세요.
- 테스트 픽스처가 실제 구현체와 동일한 동작을 보장하도록 계약 테스트(Contract Test)의 구현을 고려해보세요.
aics-api/src/main/java/kgu/developers/api/auth/presentation/request/LoginRequest.java (1)
Line range hint
11-18: 요청 DTO에 대한 유효성 검증 테스트 추가를 제안드립니다.현재
@NotNull검증이 적용되어 있는데, 이에 대한 테스트 케이스를 추가하면 좋을 것 같습니다. 특히 다음과 같은 케이스들을 고려해보시면 좋겠습니다:
- userId가 null인 경우
- password가 null인 경우
- 모든 필드가 정상적으로 입력된 경우
테스트 코드 작성에 도움이 필요하시다면 제가 예시 코드를 제공해드릴 수 있습니다.
aics-common/src/main/java/kgu/developers/common/auth/jwt/JwtProperties.java (2)
6-7: Lombok 어노테이션 사용 최적화 필요
@AllArgsConstructor를 import 했지만 사용하지 않고 있습니다. 불필요한 import는 제거하는 것이 좋습니다.-import lombok.AllArgsConstructor;
Line range hint
1-22: 보안 관련 속성 검증 로직 추가 필요JWT 관련 중요 설정을 다루는 클래스이므로, 속성값 검증 로직을 추가하는 것이 좋습니다.
다음과 같은 검증 로직 추가를 제안드립니다:
@PostConstruct public void validate() { if (issuer == null || issuer.trim().isEmpty()) { throw new IllegalStateException("JWT issuer must not be null or empty"); } if (secretKey == null || secretKey.length() < 32) { throw new IllegalStateException("JWT secret key must be at least 32 characters long"); } }aics-api/build.gradle (1)
15-21: 테스트 의존성 중복 검토 필요일부 프로젝트에 대해 직접 의존성과 testFixtures 의존성이 중복 선언되어 있습니다. 특히 ':aics-domain'과 ':aics-global-utils' 프로젝트가 해당됩니다. 이는 불필요한 의존성을 야기할 수 있습니다.
각 프로젝트의 테스트 픽스처 사용 목적을 검토하시고, 실제로 필요한 의존성만 남기는 것을 권장드립니다. 예를 들어:
testFixturesImplementation project(':aics-domain') testFixturesImplementation project(':aics-common') testFixturesImplementation project(':aics-infra') testFixturesImplementation project(':aics-global-utils') -testFixturesImplementation testFixtures(project(':aics-domain')) -testFixturesImplementation testFixtures(project(':aics-global-utils'))aics-api/src/main/java/kgu/developers/api/user/presentation/request/UserUpdateRequest.java (1)
Line range hint
15-16: 이메일 도메인 검증 강화 검토 필요현재 이메일 정규식은 모든 도메인을 허용하고 있습니다. 경기대학교 도메인(
kyonggi.ac.kr)으로 제한하는 것이 더 안전할 수 있습니다.다음과 같이 정규식을 수정하는 것을 고려해보세요:
-@Pattern(regexp = "^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\\.[A-Za-z]{2,6}$", message = "유효한 이메일 형식이 아닙니다.") +@Pattern(regexp = "^[A-Za-z0-9._%+-]+@kyonggi\\.ac\\.kr$", message = "경기대학교 이메일만 사용 가능합니다.")aics-domain/src/main/java/kgu/developers/domain/user/exception/UserDomainExceptionCode.java (1)
Line range hint
1-27: 클래스 수준의 문서화가 필요합니다.이 enum 클래스의 목적과 각 예외 코드의 사용 사례를 설명하는 JavaDoc 문서를 추가하면 좋을 것 같습니다.
다음과 같은 문서화를 제안합니다:
+/** + * 사용자 도메인과 관련된 예외 상황을 정의하는 열거형 클래스입니다. + * 각 예외 코드는 특정 HTTP 상태 코드와 에러 메시지를 포함합니다. + * + * @see kgu.developers.common.exception.ExceptionCode + */ @Getter @AllArgsConstructor public enum UserDomainExceptionCode implements ExceptionCode {aics-domain/src/main/java/kgu/developers/domain/user/domain/DeptCode.java (1)
23-29: null 반환 대신 Optional 사용을 고려해보세요.현재 구현은 일치하는 코드가 없을 경우
null을 반환하는데, 이는 잠재적인NullPointerException의 위험이 있습니다.다음과 같이 개선하는 것을 제안드립니다:
-public static DeptCode from(String code) { +public static Optional<DeptCode> from(String code) { for (DeptCode condition : values()) { if (condition.year.equals(code)) { - return condition; + return Optional.of(condition); } } - return null; + return Optional.empty(); }aics-api/src/testFixtures/java/mock/TestContainer.java (2)
1-11: 보안 관련 테스트 구성에 대한 제안테스트 환경에서도 민감한 정보(예: JWT 설정)를 안전하게 관리하는 것이 좋습니다. 테스트용 프로퍼티를 외부 설정 파일로 분리하는 것을 고려해보세요.
다음과 같은 방식으로 개선할 수 있습니다:
+ // src/test/resources/application-test.yml 파일 생성 + jwt: + issuer: testIssuer + secret-key: testSecretKey
12-17: 클래스 문서화 개선 제안테스트 컨테이너의 목적과 사용 방법을 명확히 하기 위해 JavaDoc 문서화를 추가하면 좋을 것 같습니다.
+ /** + * 사용자 인증 관련 테스트를 위한 테스트 컨테이너 + * FakeUserRepository를 사용하여 실제 데이터베이스 없이 테스트를 수행합니다. + * + * @see FakeUserRepository + * @see UserService + * @see AuthService + * @see AuthController + */ public class TestContainer {aics-api/src/main/java/kgu/developers/api/user/presentation/request/UserCreateRequest.java (1)
Line range hint
39-40: Major 열거형에 대한 추가 검증 고려가 필요합니다.현재 Major 필드는 @NotNull 검증만 수행하고 있습니다. Major 열거형에 정의된 값만 허용되도록 @EnumValidator 같은 추가 검증을 고려해보시는 것이 좋을 것 같습니다.
@Schema(description = "전공 이름", example = "CSE", requiredMode = REQUIRED) @NotNull +@EnumValidator(enumClass = Major.class, message = "유효하지 않은 전공입니다.") Major majoraics-api/src/testFixtures/java/auth/presentation/AuthControllerTest.java (2)
3-16: 테스트 검증을 위한 추가 Assertion 라이브러리 제안보다 다양한 테스트 케이스 검증을 위해 다음과 같은 AssertJ의 추가 기능을 활용하는 것이 좋습니다:
assertThatThrownBy(): 예외 처리 테스트assertThatCode(): 예외가 발생하지 않는 케이스 테스트assertSoftly(): 여러 검증을 한번에 수행
17-20: medium test 전환 계획에 대한 구체화 필요현재 주석은 medium test로의 전환 계획을 언급하고 있지만, 다음 사항들을 추가로 명시하면 좋을 것 같습니다:
- 전환 시기
- 구체적인 전환 방법
- TestContainer 변경 사항
build.gradle (1)
57-62: 테스트 의존성 최적화를 검토해주세요.현재 구성된 테스트 픽스처 의존성들이 테스트 환경 구축에 필요한 기능들을 포함하고 있습니다. 하지만 몇 가지 고려사항이 있습니다:
springdoc-openapi-starter-webmvc-ui는 테스트 픽스처에서 실제로 필요한지 검토가 필요합니다.- 테스트 실행 속도 향상을 위해 각 모듈별로 실제 필요한 의존성만 선택적으로 포함하는 것이 좋습니다.
aics-domain/src/testFixtures/java/mock/FakeUserRepository.java (3)
23-35: save 메서드 최적화 검토 필요현재 구현에서는 새로운 User 객체를 생성하여 모든 필드를 복사하고 있습니다. 테스트 환경에서는 입력된 User 객체를 직접 저장하는 것으로 충분할 수 있습니다.
다음과 같이 간소화하는 것을 고려해보세요:
@Override public User save(User user) { - User newUser = User.builder() - .id(user.getId()) - .password(user.getPassword()) - .name(user.getName()) - .email(user.getEmail()) - .phone(user.getPhone()) - .role(user.getRole()) - .major(user.getMajor()) - .build(); - data.add(newUser); - return newUser; + data.add(user); + return user; }
38-40: existsById 메서드 성능 최적화 제안현재 구현은
stream().anyMatch()를 사용하고 있습니다. 간단한 존재 여부 확인의 경우 일반 for-each나 contains를 사용하는 것이 더 효율적일 수 있습니다.@Override public boolean existsById(String userId) { - return data.stream().anyMatch(item -> item.getId().equals(userId)); + for (User user : data) { + if (user.getId().equals(userId)) { + return true; + } + } + return false; }
43-45: findById 메서드 일관성 개선 제안
existsById와 유사한 로직을 수행하지만 다른 스타일로 구현되어 있습니다. 코드 일관성을 위해 유사한 구현 스타일을 사용하는 것이 좋습니다.@Override public Optional<User> findById(String userId) { - return data.stream().filter(item -> item.getId().equals(userId)).findAny(); + for (User user : data) { + if (user.getId().equals(userId)) { + return Optional.of(user); + } + } + return Optional.empty(); }aics-domain/src/testFixtures/java/user/domain/UserDomainTest.java (2)
18-41: 테스트 케이스 보완이 필요해 보입니다.현재 구현은 기본적인 속성 검증을 잘 수행하고 있으나, 다음 사항들을 고려해보시면 좋겠습니다:
- 비밀번호 인코딩 검증
- 전화번호 형식 유효성 검증
- 이메일 형식 유효성 검증
다음과 같이 테스트 케이스를 보완해보세요:
@Test @DisplayName("USER 객체를 생성할 수 있다") public void createUser_Success() { // given String id = "202411345"; String password = "password"; String name = "홍길동"; String email = "valid@kyonggi.ac.kr"; String phone = "010-1234-5678"; Major major = Major.CSE; // when User user = User.create(id, password, name, email, phone, major); // then assertNotNull(user); assertEquals(id, user.getId()); - assertEquals(password, user.getPassword()); + assertTrue(passwordEncoder.matches(password, user.getPassword())); assertEquals(name, user.getName()); assertEquals(email, user.getEmail()); assertEquals(phone, user.getPhone()); assertEquals(Role.USER, user.getRole()); assertEquals(major, user.getMajor()); + assertTrue(user.getPhone().matches("\\d{3}-\\d{4}-\\d{4}")); + assertTrue(user.getEmail().matches("^[A-Za-z0-9+_.-]+@kyonggi\\.ac\\.kr$")); }
61-77: 학과 코드 유효성 검증에 대한 문서화가 필요합니다.테스트 코드에서 사용된 학과 코드의 유효성 규칙이 명확하지 않습니다. 다음 사항들을 보완해주세요:
- 유효한 학과 코드의 규칙 문서화
- 테스트 케이스에 사용된 "202410345"가 왜 잘못된 코드인지 설명
- 다양한 경계값 테스트 추가
코드에 다음과 같이 주석을 추가하여 문서화해보세요:
@Test @DisplayName("잘못된 학과 코드로 USER 생성 시 DeptCodeNotValidException이 발생 한다") public void createUser_InvalidDeptCode_ThrowsException() { // given - String id = "202410345"; // 잘못된 학과 코드 + // 학번 구조: YYYY(입학년도) + DD(학과코드) + XXX(순번) + // 컴퓨터공학부 학과코드: 11 + // 잘못된 학과 코드 10을 사용하여 테스트 + String id = "202410345"; String password = "password"; String name = "홍길동"; String email = "valid@kyonggi.ac.kr"; String phone = "010-1234-5678"; Major major = Major.CSE; // when & then assertThatThrownBy(() -> { User.create(id, password, name, email, phone, major); }).isInstanceOf(DeptCodeNotValidException.class); }aics-common/src/main/java/kgu/developers/common/auth/jwt/TokenProvider.java (2)
Line range hint
44-52: 토큰 검증 로직 개선 필요validateToken 메소드의 예외 처리가 너무 광범위합니다. 다음과 같은 개선이 필요합니다:
- 구체적인 예외 처리 (ExpiredJwtException, MalformedJwtException 등)
- 명시적인 만료 시간 검증
- 로깅 추가
다음과 같이 수정하는 것을 권장드립니다:
public boolean validateToken(String token) { try { - Jwts.parser() - .setSigningKey(jwtProperties.getSecretKey()) - .parseClaimsJws(token); - return true; - } catch (Exception e) { - return false; + Claims claims = Jwts.parser() + .setSigningKey(jwtProperties.getSecretKey()) + .parseClaimsJws(token) + .getBody(); + return !claims.getExpiration().before(new Date()); + } catch (ExpiredJwtException e) { + log.warn("만료된 JWT 토큰입니다: {}", e.getMessage()); + return false; + } catch (MalformedJwtException e) { + log.warn("유효하지 않은 JWT 토큰입니다: {}", e.getMessage()); + return false; + } catch (Exception e) { + log.error("JWT 토큰 검증 중 예외 발생: {}", e.getMessage()); + return false; } }
Line range hint
54-67: 인증 구현 개선 필요getAuthentication 메소드에서 다음과 같은 개선이 필요합니다:
- 하드코딩된 "ROLE_USER" 역할
- 빈 문자열 비밀번호 사용
- 사용자 역할 정보가 토큰에 포함되어 있지 않음
다음과 같은 개선사항을 제안드립니다:
- 사용자 역할을 토큰 클레임에 포함
- 역할 정보를 상수로 분리
- UserDetails 구현체를 별도 클래스로 분리
토큰 생성 시 다음과 같이 역할 정보를 포함하는 것을 고려해보세요:
private String makeToken(Date expiry, String userId, Set<String> roles) { return Jwts.builder() // ... existing code ... .claim("roles", roles) .signWith(HS256, jwtProperties.getSecretKey()) .compact(); }aics-api/src/testFixtures/java/auth/application/AuthServiceTest.java (2)
24-52: 테스트 설정이 FakeUserRepository를 효과적으로 활용하고 있습니다.Mockito 대신 FakeUserRepository를 사용한 접근 방식이 PR의 목표와 잘 부합합니다. 다만, 몇 가지 개선 사항을 제안드립니다:
- 테스트용 상수값들을 클래스 상단에 정의하면 재사용성과 가독성이 향상될 것 같습니다.
- 테스트 데이터 생성 로직을 별도의 팩토리 메서드로 분리하면 좋을 것 같습니다.
다음과 같은 리팩토링을 제안드립니다:
public class AuthServiceTest { + private static final String TEST_USER_ID = "202411345"; + private static final String TEST_PASSWORD = "password1234"; + private static final String TEST_NAME = "홍길동"; + private static final String TEST_EMAIL = "test@kyonggi.ac.kr"; + private static final String TEST_PHONE = "010-1234-5678"; private AuthService authService; + private FakeUserRepository fakeUserRepository; + private BCryptPasswordEncoder bCryptPasswordEncoder; @BeforeEach public void init() { - FakeUserRepository fakeUserRepository = new FakeUserRepository(); - BCryptPasswordEncoder bCryptPasswordEncoder = new BCryptPasswordEncoder(); + fakeUserRepository = new FakeUserRepository(); + bCryptPasswordEncoder = new BCryptPasswordEncoder(); // ... (authService 설정 코드는 동일) + saveTestUser(); + } - fakeUserRepository.save(User.builder() + private void saveTestUser() { + User testUser = User.builder() - .id("202411345") - .password(bCryptPasswordEncoder.encode("password1234")) - .name("홍길동") - .email("test@kyonggi.ac.kr") - .phone("010-1234-5678") + .id(TEST_USER_ID) + .password(bCryptPasswordEncoder.encode(TEST_PASSWORD)) + .name(TEST_NAME) + .email(TEST_EMAIL) + .phone(TEST_PHONE) .major(Major.CSE) - .build()); + .build(); + fakeUserRepository.save(testUser); }
72-88: 실패 케이스에 대한 테스트가 적절히 구현되어 있습니다.예외 검증이 잘 되어있습니다. 다만, 다음과 같은 추가 테스트 케이스들도 고려해보시면 좋을 것 같습니다:
- 존재하지 않는 사용자 ID로 로그인 시도
- 빈 문자열이나 null 값으로 로그인 시도
- 특수문자나 SQL 인젝션 문자열로 로그인 시도
테스트 케이스 추가를 원하시면 구현을 도와드리겠습니다.
aics-api/src/main/java/kgu/developers/api/user/application/UserService.java (1)
Line range hint
77-86: 보안 컨텍스트 처리 개선이 필요합니다현재 me() 메소드의 예외 처리가 모든 예외를 UserNotAuthenticatedException으로 변환하고 있습니다. 이는 다른 종류의 보안 관련 문제를 식별하기 어렵게 만듭니다.
다음과 같이 구체적인 예외 처리를 추가하는 것을 제안드립니다:
public User me() { try { Object principal = SecurityContextHolder.getContext().getAuthentication().getPrincipal(); String userId = ((UserDetails)principal).getUsername(); return getUserById(userId); - } catch (Exception e) { + } catch (NullPointerException e) { throw new UserNotAuthenticatedException(); + } catch (ClassCastException e) { + throw new InvalidAuthenticationTypeException(); } }aics-domain/src/main/java/kgu/developers/domain/user/domain/User.java (2)
70-72: 사용자 생성 메서드의 가독성이 개선되었습니다.매개변수 배치와
validateDept호출이 명확해졌습니다. 다만, 생성 메서드에 대한 문서화가 필요해 보입니다.다음과 같이 메서드 문서화를 추가하는 것을 제안드립니다:
+/** + * 새로운 사용자 엔티티를 생성합니다. + * @param id 학번 + * @param password 비밀번호 + * @param name 이름 + * @param email 경기대학교 이메일 + * @param phone 전화번호 + * @param major 전공 + * @return 생성된 사용자 엔티티 + * @throws DeptCodeNotValidException 유효하지 않은 학과 코드인 경우 + * @throws EmailDomainNotValidException 유효하지 않은 이메일 도메인인 경우 + */ public static User create(String id, String password, String name, String email, String phone, Major major) {
116-119: 이메일 도메인 검증이 상수화되어 개선되었습니다.도메인 문자열을 상수로 분리하고 null 체크를 추가한 것이 좋은 변경입니다. 다만, 이메일 형식 자체에 대한 추가적인 검증도 고려해볼 만합니다.
정규식을 사용한 이메일 형식 검증을 추가하는 것을 제안드립니다:
private static boolean isValidEmailDomain(String email) { - return email != null && email.endsWith(ACCESSIBLE_EMAIL_DOMAIN); + return email != null && + email.matches("^[A-Za-z0-9+_.-]+@kyonggi\\.ac\\.kr$"); }aics-api/src/testFixtures/java/user/application/UserServiceTest.java (4)
23-50: 테스트 데이터 설정 개선이 필요합니다다음과 같은 개선사항을 제안드립니다:
- 테스트 데이터를 별도의 테스트 픽스처 클래스로 분리하여 재사용성을 높이는 것이 좋습니다.
- 비밀번호와 같은 민감한 정보는 상수로 분리하는 것이 좋습니다.
- 각 테스트 케이스에 필요한 데이터만 설정하는 것이 테스트의 의도를 더 명확하게 할 수 있습니다.
예시 리팩토링:
private static final String TEST_PASSWORD = "password1234"; private static final String TEST_EMAIL_DOMAIN = "kyonggi.ac.kr"; private TestUserFixture testUser; @BeforeEach public void init() { BCryptPasswordEncoder bCryptPasswordEncoder = new BCryptPasswordEncoder(); FakeUserRepository fakeUserRepository = new FakeUserRepository(); this.userService = UserService.builder() .userRepository(fakeUserRepository) .bCryptPasswordEncoder(bCryptPasswordEncoder) .build(); testUser = TestUserFixture.createDefaultUser(bCryptPasswordEncoder); fakeUserRepository.save(testUser.toEntity()); }
72-90: 예외 메시지 검증을 추가해주세요예외가 발생하는 것은 확인하고 있지만, 예외 메시지가 적절한지는 확인하고 있지 않습니다. 사용자에게 더 명확한 피드백을 제공하기 위해 예외 메시지도 검증하면 좋을 것 같습니다.
assertThatThrownBy(() -> { userService.createUser(request); -}).isInstanceOf(UserIdDuplicateException.class); +}).isInstanceOf(UserIdDuplicateException.class) + .hasMessageContaining("이미 존재하는 사용자 ID입니다");
92-103: getUserById 성공 케이스의 검증을 보완해주세요현재는 이름만 검증하고 있는데, 사용자의 다른 중요한 정보들도 함께 검증하면 좋을 것 같습니다.
// then -assertEquals("홍길동", result.getName()); +assertEquals("홍길동", result.getName()); +assertEquals("test@kyonggi.ac.kr", result.getEmail()); +assertEquals("010-1234-5678", result.getPhone()); +assertEquals(Major.CSE, result.getMajor());
1-117: 테스트 아키텍처에 대한 제안FakeRepository를 사용한 접근 방식이 좋습니다. 다음과 같은 개선사항을 고려해보시면 좋을 것 같습니다:
- 테스트 격리를 위해
@Nested클래스를 사용하여 관련 테스트들을 그룹화- 테스트 데이터 생성을 위한 빌더나 팩토리 메서드 도입
- 반복되는 검증 로직을 커스텀 Assertion으로 추출
예시 구조:
@DisplayName("UserService 테스트") class UserServiceTest { @Nested @DisplayName("사용자 생성 시") class CreateUser { @Test @DisplayName("유효한 요청이면 성공한다") void success() { ... } @Test @DisplayName("중복된 ID면 실패한다") void duplicateId() { ... } } @Nested @DisplayName("사용자 조회 시") class GetUser { ... } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (25)
.coderabbit.yaml(0 hunks)aics-api/build.gradle(1 hunks)aics-api/src/main/java/kgu/developers/api/auth/application/AuthService.java(1 hunks)aics-api/src/main/java/kgu/developers/api/auth/presentation/AuthController.java(1 hunks)aics-api/src/main/java/kgu/developers/api/auth/presentation/request/LoginRequest.java(1 hunks)aics-api/src/main/java/kgu/developers/api/user/application/UserService.java(1 hunks)aics-api/src/main/java/kgu/developers/api/user/presentation/request/UserCreateRequest.java(1 hunks)aics-api/src/main/java/kgu/developers/api/user/presentation/request/UserUpdateRequest.java(1 hunks)aics-api/src/testFixtures/java/auth/application/AuthServiceTest.java(1 hunks)aics-api/src/testFixtures/java/auth/presentation/AuthControllerTest.java(1 hunks)aics-api/src/testFixtures/java/mock/TestContainer.java(1 hunks)aics-api/src/testFixtures/java/user/application/UserServiceTest.java(1 hunks)aics-api/src/testFixtures/java/user/presentation/UserControllerTest.java(1 hunks)aics-common/src/main/java/kgu/developers/common/auth/jwt/JwtProperties.java(2 hunks)aics-common/src/main/java/kgu/developers/common/auth/jwt/TokenProvider.java(1 hunks)aics-domain/build.gradle(1 hunks)aics-domain/src/main/java/kgu/developers/domain/user/domain/DeptCode.java(2 hunks)aics-domain/src/main/java/kgu/developers/domain/user/domain/User.java(7 hunks)aics-domain/src/main/java/kgu/developers/domain/user/exception/DepartmentCodeNotValidException.java(0 hunks)aics-domain/src/main/java/kgu/developers/domain/user/exception/DeptCodeNotValidException.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/user/exception/UserDomainExceptionCode.java(1 hunks)aics-domain/src/testFixtures/java/mock/FakeUserRepository.java(1 hunks)aics-domain/src/testFixtures/java/user/application/UserServiceTest.java(1 hunks)aics-domain/src/testFixtures/java/user/domain/UserDomainTest.java(1 hunks)build.gradle(3 hunks)
💤 Files with no reviewable changes (2)
- .coderabbit.yaml
- aics-domain/src/main/java/kgu/developers/domain/user/exception/DepartmentCodeNotValidException.java
✅ Files skipped from review due to trivial changes (3)
- aics-api/src/main/java/kgu/developers/api/auth/application/AuthService.java
- aics-api/src/testFixtures/java/user/presentation/UserControllerTest.java
- aics-domain/src/testFixtures/java/user/application/UserServiceTest.java
🔇 Additional comments (21)
aics-domain/src/main/java/kgu/developers/domain/user/exception/DeptCodeNotValidException.java (2)
1-1: 클래스 설계가 깔끔하고 명확합니다!
예외 클래스의 구현이 간단명료하며, CustomException을 상속받아 도메인 특화 예외를 잘 표현하고 있습니다. 상수를 static import하여 코드의 가독성도 향상되었습니다.
Also applies to: 3-3, 5-5, 7-11
1-11: 기존 코드와의 호환성을 확인해주세요.
DepartmentCodeNotValidException에서 DeptCodeNotValidException으로의 변경이 이루어졌습니다. 이 예외를 사용하는 모든 코드가 업데이트되었는지 확인이 필요합니다.
다음 스크립트로 기존 예외 클래스의 사용처를 확인할 수 있습니다:
✅ Verification successful
Based on the search results, I can generate the final response since we have clear evidence that:
- There are no occurrences of the old
DepartmentCodeNotValidExceptionin the codebase - The new
DeptCodeNotValidExceptionis properly used in all relevant places (User domain class and its tests)
예외 클래스 이름 변경이 올바르게 적용되었습니다.
기존 DepartmentCodeNotValidException의 사용처가 발견되지 않았으며, 새로운 DeptCodeNotValidException이 도메인 클래스와 테스트 코드에서 정상적으로 사용되고 있습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: 기존 DepartmentCodeNotValidException 사용 여부 확인
# 기존 예외 클래스 사용 검색
echo "Checking for old exception class usage..."
rg "DepartmentCodeNotValidException"
# 새로운 예외 클래스 사용 검색
echo "Checking for new exception class usage..."
rg "DeptCodeNotValidException"
Length of output: 1189
aics-api/src/main/java/kgu/developers/api/auth/presentation/request/LoginRequest.java (1)
7-7: @builder 어노테이션 추가가 적절합니다.
Record 클래스에 Builder 패턴을 적용함으로써 테스트 코드 작성 시 객체 생성이 더욱 유연해질 것으로 보입니다. 특히 테스트 데이터 설정이 더욱 명확하고 가독성이 좋아질 것으로 예상됩니다.
Also applies to: 9-9
aics-api/build.gradle (1)
Line range hint 1-6: JAR 구성 설정 검토 필요
bootJar와 jar 태스크가 모두 활성화되어 있습니다. 이는 빌드 시 두 개의 다른 JAR 파일이 생성될 수 있으며, 배포 시 혼란을 야기할 수 있습니다. Spring Boot 애플리케이션의 경우 일반적으로 bootJar만 활성화하고 jar는 비활성화하는 것이 권장됩니다.
다음 설정을 권장드립니다:
bootJar {
enabled = true
}
jar {
- enabled = true
+ enabled = false
}aics-api/src/main/java/kgu/developers/api/user/presentation/request/UserUpdateRequest.java (1)
9-9: Builder 패턴 적용이 잘 되었습니다!
테스트 코드 작성 시 객체 생성이 더욱 용이해질 것으로 보입니다.
aics-domain/src/main/java/kgu/developers/domain/user/exception/UserDomainExceptionCode.java (1)
16-16: DEPARTMENT_CODE_NOT_VALID에서 DEPT_CODE_NOT_VALID로의 변경이 확인되었습니다.
이름 변경이 적절하게 이루어졌습니다. 다만, 관련된 다른 코드들에서도 일관성 있게 변경되었는지 확인이 필요합니다.
다음 스크립트를 실행하여 코드베이스 전체에서 일관성을 확인하겠습니다:
✅ Verification successful
코드베이스 전체에서 'department'에서 'dept'로의 일관된 변경이 확인되었습니다.
검증 결과:
- 'department' 키워드가 포함된 파일이 전혀 발견되지 않았습니다.
- 모든 관련 코드가 'dept' 키워드를 일관되게 사용하고 있습니다:
DeptCode열거형DeptCodeNotValidException예외 클래스DEPT_CODE_NOT_VALID예외 코드- 기타 도메인의 카테고리 상수들 (
DEPT_INTRO,DEPT_INFO,DEPT_NEWS)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: 'department' 관련 네이밍이 모두 'dept'로 변경되었는지 확인
# department 키워드 검색
echo "Checking for remaining 'department' occurrences:"
rg -i "department" --type java
# dept 키워드 검색으로 변경된 부분 확인
echo -e "\nChecking for new 'dept' occurrences:"
rg -i "dept" --type java
Length of output: 3563
aics-domain/src/main/java/kgu/developers/domain/user/domain/DeptCode.java (2)
Line range hint 10-17: 코드 구조가 깔끔하고 명확합니다!
열거형의 이름을 DepartmentCodeCondition에서 DeptCode로 변경한 것이 더 간결하고 가독성이 좋습니다.
32-41: from() 메소드의 null 반환값 처리를 확인해주세요.
40번 라인에서 from() 메소드의 반환값이 null일 경우에 대한 처리가 안전하게 되어있는지 확인이 필요합니다.
다음 스크립트로 이 메소드의 사용처들을 확인해보세요:
✅ Verification successful
Let me check the from() method implementation to verify the null safety.
Let me try to find the from method using ripgrep instead.
from() 메소드의 null 처리가 안전하게 구현되어 있습니다.
isValidDeptCode() 메소드에서 from() 메소드의 null 반환값은 condition != null 체크를 통해 안전하게 처리되고 있습니다. 또한 User 클래스에서도 isValidDeptCode()의 결과를 적절히 예외 처리하고 있어 전반적인 null 안전성이 확보되어 있습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all usages of isValidDeptCode to verify null handling
rg -l "isValidDeptCode" | xargs rg -A 5 "isValidDeptCode"
Length of output: 1876
Script:
#!/bin/bash
# Find the implementation of from() method in DeptCode.java
ast-grep --pattern 'from($_) {
$$$
}'
Length of output: 41
Script:
#!/bin/bash
# Find the implementation of from method in DeptCode.java
rg -A 10 "from\(" aics-domain/src/main/java/kgu/developers/domain/user/domain/DeptCode.java
Length of output: 557
aics-api/src/testFixtures/java/mock/TestContainer.java (1)
12-37: 테스트 정리(cleanup) 메커니즘 추가 제안
테스트 실행 후 리소스를 정리하기 위한 메커니즘이 필요해 보입니다.
다음과 같은 cleanup 메서드 추가를 제안드립니다:
public class TestContainer {
+ public void cleanup() {
+ // FakeUserRepository 데이터 초기화
+ if (userRepository instanceof FakeUserRepository) {
+ ((FakeUserRepository) userRepository).clear();
+ }
+ }
}아래 스크립트로 테스트 클래스에서 cleanup 메서드 호출 여부를 확인할 수 있습니다:
aics-api/src/main/java/kgu/developers/api/user/presentation/request/UserCreateRequest.java (1)
9-9: @builder 어노테이션 추가 적절합니다.
Builder 패턴의 도입으로 테스트 코드 작성이 용이해지고, 객체 생성의 유연성이 향상되었습니다. PR의 목표인 테스트 코드 구현에 잘 부합하는 변경사항입니다.
Also applies to: 11-11
aics-api/src/main/java/kgu/developers/api/auth/presentation/AuthController.java (1)
Line range hint 28-38: 로그인 API 구현이 적절해 보입니다.
컨트롤러의 기본 구현은 잘 되어있습니다. API 문서화를 위한 Swagger 어노테이션과 validation 처리가 잘 구현되어 있습니다.
aics-api/src/testFixtures/java/auth/presentation/AuthControllerTest.java (1)
27-28: TestContainer 구현 검증 필요
TestContainer의 구현이 적절한지 확인이 필요합니다.
✅ Verification successful
Let me gather more information about the test container implementation and its dependencies.
Let me check the AuthController implementation and its tests to complete the verification.
TestContainer 구현이 적절함을 확인했습니다
TestContainer의 구현이 다음과 같은 이유로 적절합니다:
- 필요한 모든 의존성(UserRepository, UserService, AuthService, AuthController)이 올바르게 초기화되어 있습니다
- FakeUserRepository를 통해 테스트용 저장소가 적절히 구현되어 있습니다
- BCryptPasswordEncoder, TokenProvider 등 인증에 필요한 컴포넌트들이 테스트 환경에 맞게 구성되어 있습니다
- AuthControllerTest에서 실제 로그인 시나리오를 성공적으로 테스트하고 있습니다
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# TestContainer 구현 확인
rg -t java "class TestContainer" -A 10
Length of output: 1079
Script:
#!/bin/bash
# Check FakeUserRepository implementation
rg -t java "class FakeUserRepository" -A 10
# Check full TestContainer implementation
rg -t java "class TestContainer" -A 30
# Check AuthController test usage
rg -t java "TestContainer.*authController"
Length of output: 3476
Script:
#!/bin/bash
# Check AuthController implementation
rg -t java "class AuthController" -A 10
# Check AuthController tests
rg -t java "class.*AuthControllerTest" -A 20
# Check AuthService implementation
rg -t java "class AuthService" -A 10
Length of output: 6989
build.gradle (1)
3-3: 테스트 픽스처 플러그인 설정이 적절합니다.
java-test-fixtures 플러그인의 추가는 테스트 코드의 재사용성과 모듈 간 공유를 가능하게 하여, FakeRepository 구현과 테스트 코드 관리에 적합한 환경을 제공합니다.
Also applies to: 23-23
aics-domain/src/testFixtures/java/user/domain/UserDomainTest.java (1)
1-15: 패키지 구조와 임포트 선언이 잘 구성되어 있습니다!
테스트에 필요한 라이브러리와 도메인 클래스들이 적절하게 임포트되어 있습니다.
aics-api/src/testFixtures/java/auth/application/AuthServiceTest.java (2)
1-21: 클래스 구조가 잘 구성되어 있습니다!
필요한 의존성들이 잘 임포트되어 있으며, 테스트 클래스의 기본 구조가 깔끔합니다.
54-70: 로그인 성공 테스트가 잘 구현되어 있습니다.
테스트 구조와 검증 로직이 명확합니다. 하지만 토큰 발급에 대한 실제 검증이 누락되어 있습니다.
토큰 발급 검증을 위해 다음과 같은 개선을 제안드립니다:
@Test
@DisplayName("login은 토큰을 발급할 수 있다")
public void login_Success() {
// given
String userId = "202411345";
String password = "password1234";
// when
+ String token = authService.login(LoginRequest.builder()
+ .userId(userId)
+ .password(password)
+ .build());
// then
- assertThatCode(() -> {
- authService.login(LoginRequest.builder()
- .userId(userId)
- .password(password)
- .build()
- );
- }).doesNotThrowAnyException();
+ assertThat(token).isNotNull().isNotEmpty();aics-api/src/main/java/kgu/developers/api/user/application/UserService.java (1)
Line range hint 32-45: 보안 관련 검증이 필요합니다
사용자 생성 로직에서 다음 사항들을 검증해주시기 바랍니다:
- 비밀번호 복잡도 요구사항
- 이메일 형식 검증
- 전화번호 형식 검증
✅ Verification successful
입력값 검증이 이미 적절히 구현되어 있습니다
UserCreateRequest 클래스에 다음과 같은 검증이 이미 구현되어 있습니다:
- 비밀번호:
@Pattern정규식으로 복잡도 요구사항이 검증됨 - 이메일:
@Pattern(regexp = "^[A-Za-z0-9._%+-]+@(kyonggi|kgu)\\.ac\\.kr$")로 학교 이메일 형식 검증 - 전화번호:
@Pattern(regexp = "^\\d{2,4}-\\d{3,4}-\\d{4}$")로 형식 검증
추가로 도메인 계층에서도 User 클래스의 validateDept(), isValidEmailDomain() 등을 통해 이중으로 검증하고 있습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: 비밀번호 정책 및 입력값 검증 로직 확인
# 비밀번호 정책 관련 코드 검색
rg -l "password.*validation|password.*policy"
# 입력값 검증 관련 코드 검색
ast-grep --pattern 'class $_ implements Validator { $$$ }'
Length of output: 108
Script:
#!/bin/bash
# 사용자 입력 검증 관련 코드 검색을 위한 다양한 패턴 시도
# 1. UserCreateRequest 클래스 검색
rg -A 10 "class UserCreateRequest"
# 2. @Valid, @Validated 어노테이션 사용 검색
rg -l "@Valid|@Validated"
# 3. Bean Validation 제약조건 검색
rg "@Pattern|@Email|@Size|@NotBlank|@NotNull"
# 4. 정규식 패턴이나 유효성 검증 메서드 검색
rg "Pattern\.compile|\.matches\(|validate[A-Z]|isValid"
Length of output: 6812
aics-domain/src/main/java/kgu/developers/domain/user/domain/User.java (4)
6-6: 임포트 구문과 어노테이션 변경이 적절합니다.
DeptCode 관련 임포트와 보호 수준이 지정된 @AllArgsConstructor 어노테이션의 사용이 적절합니다.
Also applies to: 17-17, 23-23, 38-38
85-87: 이메일 업데이트 로직이 간결해졌습니다.
유효성 검사와 예외 처리가 명확하게 구현되었습니다.
96-96: 권한 부여 로직이 개선되었습니다.
Role.USER.name()을 사용하여 하드코딩된 문자열을 제거한 것이 좋은 변경입니다.
109-113: 학과 코드 검증 로직의 책임이 명확해졌습니다.
검증 로직이 단순화되었고 예외 처리가 명확해졌습니다. 다만, 메서드 이름이 변경된 만큼 다른 부분에서의 호출도 모두 업데이트되었는지 확인이 필요합니다.
다음 스크립트로 변경된 메서드 이름의 일관성을 확인하겠습니다:
✅ Verification successful
메서드 이름 변경이 일관되게 적용되었습니다.
코드베이스 검사 결과, validateDept, isValidDeptCode, DeptCodeNotValidException 등의 새로운 명명이 일관되게 사용되고 있으며, 이전 메서드명(validateDepartment, isValidDepartmentCode)은 더 이상 사용되지 않음을 확인했습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: 이전 메서드명이 아직 사용되고 있는지 확인
# 이전 메서드명 검색
echo "Checking for old method names..."
rg "validateDepartment|isValidDepartmentCode|DepartmentCodeNotValidException"
Length of output: 154
Script:
#!/bin/bash
# 메서드 호출과 관련된 모든 파일 검색
echo "Searching for method calls and related files..."
# 현재 메서드명 검색
echo "Current method names:"
rg "validateDept|isValidDeptCode|DeptCodeNotValidException" -A 2
# User 클래스의 전체 내용 확인
echo -e "\nChecking User class implementation:"
fd "User.java" --exec cat {}
Length of output: 7045
aics-common/src/main/java/kgu/developers/common/auth/jwt/JwtProperties.java
Outdated
Show resolved
Hide resolved
| public TestContainer() { | ||
| BCryptPasswordEncoder bCryptPasswordEncoder = new BCryptPasswordEncoder(); | ||
| this.userRepository = new FakeUserRepository(); | ||
| this.userService = UserService.builder() | ||
| .userRepository(this.userRepository) | ||
| .bCryptPasswordEncoder(bCryptPasswordEncoder) | ||
| .build(); | ||
| this.authService = AuthService.builder() | ||
| .userService(this.userService) | ||
| .passwordEncoder(bCryptPasswordEncoder) | ||
| .tokenProvider(TokenProvider.builder() | ||
| .jwtProperties(new JwtProperties("testIssuer", "testSecretKey")) | ||
| .build() | ||
| ) | ||
| .build(); | ||
| this.authController = AuthController.builder() | ||
| .authService(this.authService) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
테스트 성능 및 유지보수성 개선 제안
몇 가지 개선 사항을 제안드립니다:
- 테스트 성능 향상을 위한 BCryptPasswordEncoder 설정:
- BCryptPasswordEncoder bCryptPasswordEncoder = new BCryptPasswordEncoder();
+ // 테스트 환경에서는 낮은 강도를 사용하여 성능 향상
+ BCryptPasswordEncoder bCryptPasswordEncoder = new BCryptPasswordEncoder(4);- 컴포넌트 초기화 모듈화:
+ private TokenProvider createTokenProvider() {
+ return TokenProvider.builder()
+ .jwtProperties(new JwtProperties("testIssuer", "testSecretKey"))
+ .build();
+ }
+
+ private UserService createUserService(BCryptPasswordEncoder encoder) {
+ return UserService.builder()
+ .userRepository(this.userRepository)
+ .bCryptPasswordEncoder(encoder)
+ .build();
+ }- 테스트 설정값 상수화:
+ private static final String TEST_ISSUER = "testIssuer";
+ private static final String TEST_SECRET_KEY = "testSecretKey";Committable suggestion skipped: line range outside the PR's diff.
aics-api/src/main/java/kgu/developers/api/auth/presentation/AuthController.java
Show resolved
Hide resolved
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.http.ResponseEntity; | ||
| import org.springframework.web.bind.annotation.PostMapping; | ||
| import org.springframework.web.bind.annotation.RequestBody; | ||
| import org.springframework.web.bind.annotation.RequestMapping; | ||
| import org.springframework.web.bind.annotation.RestController; | ||
|
|
||
| @Builder |
There was a problem hiding this comment.
💡 Codebase verification
@builder 어노테이션 제거 시 테스트 코드 수정 필요
TestContainer.java 파일에서 AuthController.builder()를 사용하고 있어 수정이 필요합니다:
aics-api/src/testFixtures/java/mock/TestContainer.java에서 AuthController 빌더 패턴 사용 중
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if any test files are using AuthController's builder pattern
# Search for AuthController.builder() usage in test files
rg -t java "AuthController\.builder\(\)" src/test/
Length of output: 102
Script:
#!/bin/bash
# First, let's find all test files in the repository
fd -t f "Test.*\.java$"
# Then, let's search for any builder pattern usage related to AuthController
rg -t java "AuthController.*builder"
# Also check for any test files that might be importing or using AuthController
rg -t java "AuthController"
Length of output: 1209
aics-api/src/testFixtures/java/auth/presentation/AuthControllerTest.java
Show resolved
Hide resolved
aics-common/src/main/java/kgu/developers/common/auth/jwt/TokenProvider.java
Show resolved
Hide resolved
aics-api/src/main/java/kgu/developers/api/user/application/UserService.java
Show resolved
Hide resolved
…operties.java Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Summary
User, Auth 도메인에 대하여 단위 및 통합 테스트 코드를 작성하였습니다.
Tasks
To Reviewer
이 PR에서는 Mockito 라이브러리를 사용하지 않고, Fake 구현체와 TestContainer를 활용한 테스트 방식을 도입했습니다.
이를 통해 다음과 같은 이점을 얻을 수 있습니다:
현실성 높은 테스트 환경 제공
Fake와 TestContainer를 활용하면 실제 환경에 더 가까운 테스트가 가능해져, 과도하게 Mock된 동작으로 인한 오탐(false positive)이나 누락(false negative)을 줄일 수 있습니다.
테스트 시간 단축
Fake 구현체는 외부 리소스 의존성을 최소화하여 테스트 실행 속도를 크게 개선합니다. 이를 통해 빠른 피드백 루프가 가능해지고, 개발 생산성이 향상됩니다.
테스트 유지보수성 향상
Mockito와 같은 서드파티 라이브러리 의존도를 낮춤으로써 테스트 코드가 단순화되고, 특히 장기 프로젝트에서 발생할 수 있는 의존성 관련 문제를 최소화할 수 있습니다.
Screenshot