Test: User 테스트 코드 / PreAuthorize 작성 #129
Conversation
kimzini
requested review from
HwangMinSeon,
dhdid,
dlwhsk0,
dongwooooooo,
jungeunyooon,
printSANO and
yuripbong
Walkthrough컨트롤러 메서드 권한 검사( Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as UserController
participant SpringSec as SpringSecurity (MethodSecurity)
participant Evaluator as DomainPermissionEvaluator
participant Repo as Repository (User / UserExperience)
participant Service as UserService
Client->>Controller: 요청 (예: DELETE /users/{id})
Controller->>SpringSec: 메서드 보안 트리거 (`@PreAuthorize`)
SpringSec->>Evaluator: hasPermission(targetId, targetType, permission)
Evaluator->>Repo: findById(targetId)
Repo-->>Evaluator: 엔티티 반환 (또는 빈)
Evaluator-->>SpringSec: 허용 or 거부 (PermissionDeniedException)
SpringSec-->>Controller: 권한 검사 결과 전달
Controller->>Service: 비즈니스 로직 실행(삭제/업데이트)
Service->>Repo: DB 변경(save/update/delete)
Repo-->>Service: 변경 완료
Service-->>Controller: 결과 반환
Controller-->>Client: HTTP 응답 (성공/오류)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45분
Possibly related PRs
Suggested reviewers
시
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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: 3
🧹 Nitpick comments (23)
techeerzip/src/test/java/backend/techeerzip/domain/user/entity/BootcampPeriodTest.java (1)
51-60: Consider using@ValueSource(ints = {...})instead of@CsvSourcefor single int parametersThe current approach with
@CsvSource({"3", "4", "5", "9", "10", "11"})works correctly, but according to JUnit 5 best practices,@ValueSourceis the recommended annotation for parameterized tests with a single simple value (primitives, strings, or classes). Using@ValueSource(ints = {3, 4, 5, 9, 10, 11})makes the intent clearer—that this is a single-argument test—while keeping the code more concise.@ParameterizedTest -@CsvSource({"3", "4", "5", "9", "10", "11"}) +@ValueSource(ints = {3, 4, 5, 9, 10, 11}) @DisplayName("유효하지 않은 월은 null 반환") void calculateGeneration_InvalidMonth_ReturnsNull(int month) { LocalDate joinDate = LocalDate.of(2025, month, 15);This is a minor refactoring that improves code clarity without changing functionality.
techeerzip/src/main/java/backend/techeerzip/domain/feedback/service/FeedbackService.java (1)
203-235: 전체 조회(getAllReceivedFeedbacks)도 구현은 적절하지만, 상태별 조회와 중복 로직이 많습니다
- 이 메서드도
validateMentorRole과 멘토 소유자 검증,startDateTime기본값 1개월 전, 공통findReceivedFeedbacks(...)사용 등 설계는 위의getReceivedFeedbacksByStatus와 일관적이라 동작 상 문제는 없어 보입니다.- 두 메서드가 “상태만 다르고 나머지 로직은 동일”하므로, 내부에서 공통 private 메서드 하나 (
getReceivedFeedbacks(mentorId, statusOpt, start, end, cursor, limit, currentUser))로 추출해 중복을 줄이는 것도 장기적으로는 유지보수에 도움이 될 것 같습니다.techeerzip/src/main/java/backend/techeerzip/domain/feedback/repository/FeedbackRepositoryImpl.java (1)
110-154: Remove unusedfindReceivedFeedbacksByStatusandfindAllReceivedFeedbacksmethodsVerification confirms both methods are indeed unused. The new unified
findReceivedFeedbacks(...)method (which accepts optional status and date range parameters) has completely replaced them in the service layer:
findReceivedFeedbacksByStatus: No call sites foundfindAllReceivedFeedbacks: No call sites foundfindReceivedFeedbacks: Called in FeedbackService at lines 185 and 228Remove both methods from:
FeedbackRepositoryCustominterface (lines 35-38)FeedbackRepositoryImplimplementation (lines 111-133 and 135-154)techeerzip/src/main/java/backend/techeerzip/domain/user/service/UserService.java (1)
240-251: signUp 내userRepository.flush()+ 재조회 사용에 대한 의견현재
@Transactional범위 안에서save이후userRepository.flush()호출 후 다시findById로managedUser를 얻어createResumeByUser에 넘기고 있는데, 일반적인 JPA 관점에서는savedUser자체도 이미 영속 상태라 재조회 없이 그대로 넘겨도 동작은 동일합니다.
createResumeByUser가REQUIRES_NEW트랜잭션이거나 별도EntityManager를 사용해서 DB에 실제로 커밋(or flush)된User를 필요로 한다면flush()호출 자체는 의미 있는 방어 코딩일 수 있으니, 그런 의도가 있다면 짧은 주석으로 남겨 두거나 “DB 동기화만 필요하다면flush()만 호출하고savedUser를 그대로 넘기는” 방향으로 단순화하는 것도 고려해 보셔도 좋겠습니다.
기존updateGithubUrlAndSync에서 더티 체킹 문제를 우회하기 위해 명시적으로 동기화·업데이트를 사용하던 패턴과 일관된 선택이라 안정성 측면에서는 긍정적으로 보입니다. Based on learnings, ...techeerzip/build.gradle (1)
229-232:test태스크 설정이 두 곳에 있어 중복될 수 있습니다위쪽
tasks.named('test', Test)블록과 여기tasks.test { ... }블록이 둘 다useJUnitPlatform()및jacocoTestReport를finalizedBy로 지정하고 있습니다. Gradle 관점에서는 문제 없지만, 이후 설정 변경 시 한쪽만 수정되는 실수를 막으려면 하나의 블록으로 합치는 것도 고려해 볼 만합니다.techeerzip/src/test/java/backend/techeerzip/config/TestContainer.java (1)
1-19: Testcontainers 기반 동적 DB 설정 패턴이 깔끔합니다 (Flyway 플래그 타입만 소소한 제안)
SharedTestContainer에서 컨테이너를 한 번만 올리고@DynamicPropertySource로 datasource를 주입하는 구조가 재사용성과 속도 면에서 좋아 보입니다.
다만registry.add("spring.flyway.enabled", () -> "false");부분은 문자열 대신() -> false로 Boolean 을 직접 넘기면 의도가 조금 더 명확해집니다.techeerzip/src/main/java/backend/techeerzip/domain/user/repository/UserRepository.java (1)
17-18:findByEmail에서 soft-delete 사용자를 제외하는 명시적 쿼리 선택이 합리적입니다
@Query로u.isDeleted = false조건을 명시해서 조회 의도가 분명해진 점이 좋습니다.
다만 바로 아래existsByEmail은 여전히 삭제 여부와 무관하게 동작할 것 같은데, 도메인 요구사항이 “삭제된 사용자도 이메일 중복 체크 대상”인지 한 번만 확인해 주세요. 만약 삭제된 사용자는 중복 체크에서 제외해야 한다면,existsByEmail도isDeleted = false조건을 포함하도록 맞춰 주면 일관성이 더 좋아질 것 같습니다.techeerzip/src/main/java/backend/techeerzip/domain/user/repository/PermissionRequestRepository.java (1)
9-22: 트랜잭션 및 벌크 업데이트 설정이 일관성 향상에 도움이 됩니다리포지토리 인터페이스에
@Transactional을 두고, 조회용인findByStatus에만readOnly = true를 명시한 점, 그리고 벌크 업데이트 메서드에clearAutomatically = true, flushAutomatically = true를 줘서 1차 캐시와 DB 상태를 맞춘 점이 좋아 보입니다.
향후 읽기 전용 메서드가 더 늘어난다면, 타입 레벨을@Transactional(readOnly = true)로 두고 쓰기 메서드에만 별도로 readOnly=false 를 지정하는 패턴도 고려해 볼 수 있겠습니다.techeerzip/src/test/java/backend/techeerzip/config/SharedTestContainer.java (1)
1-93: 싱글톤 Testcontainers 구현은 잘 작성되어 있으나 기본값 전략은 한 번 더 고민해볼 만합니다
volatile+LOCK으로 PostgreSQL 컨테이너를 한 번만 띄우고 재사용하는 패턴이 테스트 성능 관점에서 좋습니다.env.properties를 통해 로컬/CI 설정을 유연하게 가져오는 구조도 괜찮습니다.다만
DATABASE_URL/POSTGRES_DB/POSTGRES_USERNAME/POSTGRES_PASSWORD가 전혀 설정되지 않은 경우,getDatabaseName(),getUsername(),getPassword()가 모두 빈 문자열을 반환하면서 컨테이너가 다소 애매한 설정으로 뜰 수 있습니다.
테스트 편의성을 위해 예를 들어 DB 이름은"techeerzip_test"같이 명시적인 기본값을 두거나, 필수 키가 하나도 없으면 친절한 예외를 던지도록 해 두면 새 환경에서 테스트를 돌릴 때 디버깅이 더 수월할 것 같습니다.techeerzip/src/test/java/backend/techeerzip/domain/userExperience/entity/UserExperienceTest.java (1)
1-106: UserExperience 주요 동작을 잘 커버하고 있으며, 테스트 데이터 문자열만 소소한 수정 여지가 있습니다
update,delete,setIsFinished흐름을 각각 분리된 Nested 클래스로 검증해서 가독성이 좋고,updatedAt이 갱신되는지도 함께 확인하고 있어 엔티티 동작을 잘 커버하고 있습니다.
사소한 부분이지만,newPosition = "FRONTED"는"FRONTEND"정도로 오타를 정리해 두면 나중에 테스트 데이터를 읽을 때 의미 파악이 조금 더 쉬울 것 같습니다.techeerzip/src/test/java/backend/techeerzip/domain/role/repository/RoleRepositoryTest.java (2)
42-45: UUID 생성 패턴을 헬퍼 메서드로 추출하는 것을 고려하세요.동일한 UUID 기반 고유 이름 생성 패턴이 두 테스트에서 반복됩니다. 테스트 헬퍼 메서드로 추출하면 중복을 줄이고 유지보수성을 개선할 수 있습니다.
다음과 같이 헬퍼 메서드를 추가할 수 있습니다:
+ private String createUniqueRoleName(String prefix) { + return prefix + "-" + UUID.randomUUID().toString().substring(0, 8); + } + @Test @DisplayName("Role 저장 및 조회 성공") void saveAndFind_Success() { - String uniqueRoleName = - RoleTestHelper.Default.MENTOR_TEST - + "-" - + UUID.randomUUID().toString().substring(0, 8); + String uniqueRoleName = createUniqueRoleName(RoleTestHelper.Default.MENTOR_TEST); Role newRole = RoleTestHelper.createRole(uniqueRoleName);Also applies to: 57-60
35-70: 테스트 커버리지 확장을 고려하세요.현재 테스트는 기본 CRUD 작업만 다룹니다. 다음 시나리오에 대한 추가 테스트를 고려하세요:
- 존재하지 않는 ID로 조회 시 빈 Optional 반환
- 부모 Role이 있는 Role 저장 및 조회 (RoleTestHelper가 이를 지원함)
- Role 이름으로 조회 (프로덕션 코드에서 사용되는 경우)
techeerzip/src/main/java/backend/techeerzip/domain/userExperience/repository/UserExperienceRepository.java (1)
11-11: 인터페이스 레벨의 @transactional 사용을 재고하세요.Repository 인터페이스에 @transactional을 적용하는 것은 일반적이지 않습니다. 트랜잭션 관리는 일반적으로 서비스 레이어에서 처리됩니다. Repository 인터페이스에서 선언하면:
- 서비스 레이어의 트랜잭션 전략과 충돌할 수 있음
- 트랜잭션 경계가 불명확해질 수 있음
개별 메서드에 @transactional이 필요한 경우가 아니라면, 서비스 레이어에서 트랜잭션을 관리하는 것이 권장됩니다.
techeerzip/src/test/java/backend/techeerzip/domain/userExperience/repository/UserExperienceRepositoryTest.java (2)
45-75: 테스트 설정 코드를 @beforeeach 또는 헬퍼 메서드로 추출하세요.Role과 User 생성 코드가 모든 테스트에서 반복됩니다. 이는 중복을 증가시키고 유지보수를 어렵게 만듭니다.
다음과 같이 리팩토링할 수 있습니다:
+ private Role savedRole; + private User savedUser; + + private void setupUserAndRole() { + String roleName = RoleTestHelper.Default.USER_TEST + "-" + + UUID.randomUUID().toString().substring(0, 8); + savedRole = roleRepository.save(RoleTestHelper.createRole(roleName)); + + String email = "test-" + UUID.randomUUID() + "@example.com"; + String name = "테커집짱-" + UUID.randomUUID().toString().substring(0, 8); + savedUser = userRepository.save( + UserTestHelper.createUser(name, email, "Techeerzip1!", savedRole, true)); + } + @Test @DisplayName("경력 저장 및 조회 성공") void saveAndFind_Success() { - String roleName = ... - Role userRole = roleRepository.save(...); - String email = ... - User testUser = userRepository.save(...); + setupUserAndRole(); UserExperience newExperience = UserExperienceTestHelper.createOngoingExperience( - testUser.getId(), ... + savedUser.getId(), ...Also applies to: 77-108, 110-161
67-67: 빌더 패턴에 description 추가를 고려하세요.
createOngoingExperience로 생성한 후setDescription을 별도로 호출하는 것은 일관성이 떨어집니다. UserExperienceTestHelper에 description을 포함하는 오버로드 메서드를 추가하거나, 기존 메서드에 선택적 매개변수로 추가하는 것을 고려하세요.Also applies to: 144-144
techeerzip/src/test/java/backend/techeerzip/domain/user/repository/UserRepositoryTest.java (1)
41-50: 중복된 테스트 설정 코드를 헬퍼 메서드로 추출하세요.Role과 User 생성 패턴이 여러 테스트에서 반복됩니다. UserExperienceRepositoryTest와 유사하게 헬퍼 메서드나 @beforeeach 설정을 통해 중복을 줄이는 것을 고려하세요.
Also applies to: 70-79
techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.java (2)
92-97: PreAuthorize 검증이 실제로 테스트되지 않습니다.
@BeforeEach에서 모든 permission evaluator가 항상true를 반환하도록 모킹했습니다. 이는 컨트롤러의@PreAuthorize어노테이션이 실제로 올바른 권한을 확인하는지 검증하지 못합니다.PR 목표에 PreAuthorize 구현이 포함되어 있으므로, 권한 검증 로직을 실제로 테스트하는 것을 고려하세요:
- 권한이 있는 사용자는 성공
- 권한이 없는 사용자는 403 Forbidden
다음과 같이 권한 테스트를 추가할 수 있습니다:
@Test @DisplayName("권한 없는 사용자의 삭제 요청은 실패") void deleteUser_WithoutPermission_Forbidden() throws Exception { when(delegatingPermissionEvaluator.hasPermission(any(), any(), any())) .thenReturn(false); mockMvc.perform(withSecurity(delete(BASE_URL), TEST_USER_ID, RoleType.TECHEER)) .andExpect(status().isForbidden()); }
385-445: 리플렉션 사용을 피하고 빌더 또는 setter를 사용하세요.private 필드에 접근하기 위해 리플렉션을 사용하는 것은 테스트를 깨지기 쉽게 만듭니다. 필드 이름이 변경되면 컴파일 시점이 아닌 런타임에 실패합니다.
다음과 같은 대안을 고려하세요:
- Request 클래스에 public setter가 있다면 사용
- Builder 패턴이 있다면 사용
- Request 클래스에 테스트용 생성자 추가
- 테스트 전용 팩토리 메서드 추가
예시:
UpdateUserTecheerInfoRequest infoRequest = UpdateUserTecheerInfoRequest.builder() .mainPosition("BACKEND") .githubUrl("https://github.com/test") .isLft(false) .school("인천대학교") .grade("1학년") .build();techeerzip/src/test/java/backend/techeerzip/domain/user/helper/UserTestHelper.java (1)
22-28: 매직 넘버를 상수로 추출하는 것을 고려해주세요.
1L이 라인 26과 41에서 roleId로 반복 사용됩니다. 가독성을 위해 상수로 추출하면 좋겠습니다.public class UserTestHelper { + private static final Long DEFAULT_ROLE_ID = 1L; + private UserTestHelper() {}- ReflectionTestUtils.setField(user.getRole(), "id", 1L); + ReflectionTestUtils.setField(user.getRole(), "id", DEFAULT_ROLE_ID);techeerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.java (4)
44-97: 누락된 예외 클래스 import 추가 권장테스트 코드 내에서
RoleNotFoundException(line 585)과UserNotBootcampPeriodException(lines 671-673)을 fully qualified class name으로 사용하고 있습니다. 다른 예외 클래스들과의 일관성을 위해 import문을 추가하는 것이 좋습니다.import backend.techeerzip.domain.user.exception.UserAlreadyExistsException; import backend.techeerzip.domain.user.exception.UserInvalidRoleException; +import backend.techeerzip.domain.user.exception.UserNotBootcampPeriodException; import backend.techeerzip.domain.user.exception.UserNotFoundException; import backend.techeerzip.domain.user.exception.UserNotResumeException; import backend.techeerzip.domain.user.exception.UserProfileImgFailException; +import backend.techeerzip.domain.role.exception.RoleNotFoundException;
155-159: 불필요한setDeleted(false)호출
User.builder()로 생성된 User 객체는 기본적으로isDeleted = false상태입니다 (UserTest.java에서 확인 가능). 따라서 line 158의existingUser.setDeleted(false)는 불필요합니다.existingUser = UserTestHelper.createUser( "테커집짱", "[email protected]", "Techeerzip1!", defaultRole, true); - existingUser.setDeleted(false);
859-893: 사용자 삭제 테스트 커버리지 확인 필요삭제 테스트에서 관련 엔티티들(
blogRepository,bookmarkRepository등)의updateIsDeletedByUserId호출을 검증하고 있습니다. 그러나 line 892에서blogRepository만 verify하고 나머지 repository들은 검증하지 않습니다. 모든 cascade 삭제가 올바르게 수행되는지 확인하려면 다른 repository들도 verify하는 것을 권장합니다.verify(userRepository).findById(userId); verify(userRepository).save(any(User.class)); verify(blogRepository).updateIsDeletedByUserId(userId); + verify(bookmarkRepository).updateIsDeletedByUserId(userId); + verify(eventRepository).updateIsDeletedByUserId(userId); + verify(likeRepository).updateIsDeletedByUserId(userId); + verify(projectMemberRepository).updateIsDeletedByUserId(userId); + verify(resumeRepository).updateIsDeletedByUserId(userId); + verify(sessionRepository).updateIsDeletedByUserId(userId); + verify(studyMemberRepository).updateIsDeletedByUserId(userId); + verify(userExperienceRepository).updateIsDeletedByUserId(userId);
1561-1590: URL 변경 없을 시updateGithubUrlById미호출 검증 추가 권장테스트명이 "GitHub URL 변경 없을 시 동기화만 수행"이지만,
updateGithubUrlById가 호출되지 않았음을 명시적으로 검증하지 않고 있습니다. 학습된 내용에 따르면updateGithubUrlById()커스텀 업데이트 메서드를 사용하므로, URL이 변경되지 않았을 때 이 메서드가 호출되지 않음을 검증하면 테스트가 더 명확해집니다.Based on learnings,
updateGithubUrlById는 더티 체킹 대신 사용되는 커스텀 메서드입니다.verify(userRepository).findById(userId); verify(gitHubApiService).validateAndExtractUsername(githubUrl); + verify(userRepository, never()).updateGithubUrlById(any(), any()); verify(statisticDataCollectionService) .refreshGithubContributions(any(User.class), eq(githubUsername));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
techeerzip/build.gradle(3 hunks)techeerzip/src/main/java/backend/techeerzip/domain/feedback/controller/FeedbackController.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/domain/feedback/repository/FeedbackRepositoryCustom.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/domain/feedback/repository/FeedbackRepositoryImpl.java(3 hunks)techeerzip/src/main/java/backend/techeerzip/domain/feedback/service/FeedbackService.java(3 hunks)techeerzip/src/main/java/backend/techeerzip/domain/statistic/entity/WeeklyGitContributions.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/domain/user/controller/UserController.java(6 hunks)techeerzip/src/main/java/backend/techeerzip/domain/user/repository/PermissionRequestRepository.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/domain/user/repository/UserRepository.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/domain/user/service/UserService.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/domain/userExperience/repository/UserExperienceRepository.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/global/permission/UserExperiencePermissionEvaluator.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/global/permission/UserPermissionEvaluator.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/config/SharedTestContainer.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/config/TestContainer.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/domain/role/entity/RoleTest.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/domain/role/helper/RoleTestHelper.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/domain/role/repository/RoleRepositoryTest.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/domain/user/entity/BootcampPeriodTest.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/domain/user/entity/PermissionRequestTest.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/domain/user/entity/UserTest.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/domain/user/helper/UserTestHelper.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/domain/user/repository/PermissionRequestRepositoryTest.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/domain/user/repository/UserRepositoryTest.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/domain/userExperience/entity/UserExperienceTest.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/domain/userExperience/helper/UserExperienceTestHelper.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/domain/userExperience/repository/UserExperienceRepositoryTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-09-18T12:23:00.520Z
Learnt from: dlwhsk0
Repo: Techeer-Hogwarts/backend PR: 111
File: techeerzip/src/main/java/backend/techeerzip/domain/user/service/UserService.java:892-902
Timestamp: 2025-09-18T12:23:00.520Z
Learning: UserService의 updateGithubUrlAndSync 메서드에서 JPA 더티 체킹 대신 userRepository.updateGithubUrlById() 커스텀 업데이트 메서드를 사용했습니다. 더티 체킹이 예상대로 작동하지 않는 경우에 대한 실용적인 해결책입니다.
Applied to files:
techeerzip/src/main/java/backend/techeerzip/domain/userExperience/repository/UserExperienceRepository.javatecheerzip/src/main/java/backend/techeerzip/domain/user/controller/UserController.javatecheerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.javatecheerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.javatecheerzip/src/test/java/backend/techeerzip/domain/user/repository/PermissionRequestRepositoryTest.javatecheerzip/src/main/java/backend/techeerzip/domain/user/service/UserService.java
📚 Learning: 2025-09-09T10:19:12.637Z
Learnt from: kimzini
Repo: Techeer-Hogwarts/backend PR: 117
File: techeerzip/src/main/java/backend/techeerzip/global/permission/DelegatingPermissionEvaluator.java:37-43
Timestamp: 2025-09-09T10:19:12.637Z
Learning: DelegatingPermissionEvaluator에서 target이 null일 때 어떤 evaluator도 매칭되지 않는 것이 의도된 설계입니다. 어드민 권한 검증은 hasPermission이 아닌 hasRole('ADMIN')을 사용해야 합니다.
Applied to files:
techeerzip/src/main/java/backend/techeerzip/global/permission/UserExperiencePermissionEvaluator.javatecheerzip/src/main/java/backend/techeerzip/global/permission/UserPermissionEvaluator.java
📚 Learning: 2025-09-12T07:36:39.412Z
Learnt from: dlwhsk0
Repo: Techeer-Hogwarts/backend PR: 111
File: techeerzip/src/main/java/backend/techeerzip/infra/github/GitHubApiService.java:99-103
Timestamp: 2025-09-12T07:36:39.412Z
Learning: GitHub URL 검증은 서비스 레이어가 아닌 request DTO 단에서 Bean Validation 어노테이션을 사용하여 수행해야 한다.
Applied to files:
techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.java
📚 Learning: 2025-07-24T13:08:14.801Z
Learnt from: dlwhsk0
Repo: Techeer-Hogwarts/backend PR: 87
File: techeerzip/src/main/resources/db/migration/V10__statistics.sql:4-15
Timestamp: 2025-07-24T13:08:14.801Z
Learning: WeeklyGitContributions 테이블에서 주간 통계의 유니크 제약 조건을 설정할 때는 연도를 고려해야 함. 동일한 주차 번호(week)가 서로 다른 연도에 존재할 수 있으므로, UNIQUE 제약은 (userId, year, week) 또는 (userId, weekStart) 조합으로 설정해야 함.
Applied to files:
techeerzip/src/main/java/backend/techeerzip/domain/statistic/entity/WeeklyGitContributions.java
🧬 Code graph analysis (9)
techeerzip/src/test/java/backend/techeerzip/domain/userExperience/entity/UserExperienceTest.java (1)
techeerzip/src/test/java/backend/techeerzip/domain/userExperience/helper/UserExperienceTestHelper.java (1)
UserExperienceTestHelper(9-124)
techeerzip/src/test/java/backend/techeerzip/config/TestContainer.java (1)
techeerzip/src/test/java/backend/techeerzip/config/SharedTestContainer.java (1)
SharedTestContainer(9-93)
techeerzip/src/test/java/backend/techeerzip/domain/user/entity/UserTest.java (1)
techeerzip/src/test/java/backend/techeerzip/domain/user/helper/UserTestHelper.java (1)
UserTestHelper(8-104)
techeerzip/src/main/java/backend/techeerzip/global/permission/UserExperiencePermissionEvaluator.java (3)
techeerzip/src/main/java/backend/techeerzip/domain/userExperience/exception/UserExperienceNotFoundException.java (1)
UserExperienceNotFoundException(6-10)techeerzip/src/main/java/backend/techeerzip/global/exception/PermissionDeniedException.java (1)
PermissionDeniedException(3-7)techeerzip/src/main/java/backend/techeerzip/global/permission/UserPermissionEvaluator.java (1)
Slf4j(17-94)
techeerzip/src/test/java/backend/techeerzip/domain/userExperience/repository/UserExperienceRepositoryTest.java (6)
techeerzip/src/test/java/backend/techeerzip/domain/user/entity/UserTest.java (1)
DisplayName(13-228)techeerzip/src/test/java/backend/techeerzip/domain/userExperience/entity/UserExperienceTest.java (1)
DisplayName(14-106)techeerzip/src/test/java/backend/techeerzip/config/TestContainer.java (1)
TestContainer(7-20)techeerzip/src/test/java/backend/techeerzip/domain/role/helper/RoleTestHelper.java (2)
RoleTestHelper(7-65)Default(48-64)techeerzip/src/test/java/backend/techeerzip/domain/user/helper/UserTestHelper.java (1)
UserTestHelper(8-104)techeerzip/src/test/java/backend/techeerzip/domain/userExperience/helper/UserExperienceTestHelper.java (1)
UserExperienceTestHelper(9-124)
techeerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.java (10)
techeerzip/src/test/java/backend/techeerzip/domain/user/entity/PermissionRequestTest.java (4)
DisplayName(14-74)Nested(29-43)Nested(45-58)Nested(60-73)techeerzip/src/test/java/backend/techeerzip/domain/user/entity/UserTest.java (2)
DisplayName(13-228)Nested(49-101)techeerzip/src/main/java/backend/techeerzip/domain/auth/exception/AuthNotVerifiedEmailException.java (1)
AuthNotVerifiedEmailException(6-10)techeerzip/src/main/java/backend/techeerzip/domain/user/exception/UserAlreadyExistsException.java (1)
UserAlreadyExistsException(6-10)techeerzip/src/main/java/backend/techeerzip/domain/user/exception/UserInvalidRoleException.java (1)
UserInvalidRoleException(6-10)techeerzip/src/main/java/backend/techeerzip/domain/user/exception/UserNotFoundException.java (1)
UserNotFoundException(6-10)techeerzip/src/main/java/backend/techeerzip/domain/user/exception/UserNotResumeException.java (1)
UserNotResumeException(6-10)techeerzip/src/main/java/backend/techeerzip/domain/user/exception/UserProfileImgFailException.java (1)
UserProfileImgFailException(6-10)techeerzip/src/test/java/backend/techeerzip/domain/user/helper/UserTestHelper.java (1)
UserTestHelper(8-104)techeerzip/src/main/java/backend/techeerzip/domain/userExperience/exception/UserExperienceNotFoundException.java (1)
UserExperienceNotFoundException(6-10)
techeerzip/src/test/java/backend/techeerzip/domain/role/repository/RoleRepositoryTest.java (3)
techeerzip/src/test/java/backend/techeerzip/domain/role/entity/RoleTest.java (2)
DisplayName(9-36)Nested(12-35)techeerzip/src/test/java/backend/techeerzip/config/TestContainer.java (1)
TestContainer(7-20)techeerzip/src/test/java/backend/techeerzip/domain/role/helper/RoleTestHelper.java (2)
RoleTestHelper(7-65)Default(48-64)
techeerzip/src/main/java/backend/techeerzip/global/permission/UserPermissionEvaluator.java (3)
techeerzip/src/main/java/backend/techeerzip/domain/user/exception/UserNotFoundException.java (1)
UserNotFoundException(6-10)techeerzip/src/main/java/backend/techeerzip/global/exception/PermissionDeniedException.java (1)
PermissionDeniedException(3-7)techeerzip/src/main/java/backend/techeerzip/global/permission/UserExperiencePermissionEvaluator.java (1)
Slf4j(17-86)
techeerzip/src/test/java/backend/techeerzip/domain/role/entity/RoleTest.java (1)
techeerzip/src/test/java/backend/techeerzip/domain/role/repository/RoleRepositoryTest.java (1)
Nested(35-70)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (35)
techeerzip/src/main/java/backend/techeerzip/domain/statistic/entity/WeeklyGitContributions.java (2)
22-28: LGTM! 인덱스 정의 포맷 개선인덱스 컬럼 목록을 공백 구분에서 쉼표 구분 형식으로 변경한 것은 가독성과 일관성을 개선합니다. 두 형식 모두 JPA에서 유효하지만, 쉼표 구분 형식이 더 명확하고 파싱하기 쉽습니다.
1-91: PR 목표와 변경 내용 간 불일치PR 제목과 설명은 User, Role, UserExperience의 테스트 코드 및 PreAuthorize 구현에 관한 것이지만, 리뷰를 위해 제공된 파일은
WeeklyGitContributions엔티티의 인덱스 포맷팅 변경만 포함하고 있습니다. 다른 관련 파일들(테스트 파일 또는 PreAuthorize 어노테이션이 있는 파일)이 누락되었는지 확인해 주세요.techeerzip/src/test/java/backend/techeerzip/domain/user/entity/BootcampPeriodTest.java (2)
13-49: 테스트 구조와 시나리오 구성이 명확합니다
@Nested,@DisplayName를 활용해서 기수 계산 시나리오가 잘 드러납니다.- 상반기(라인 20–29), 하반기(라인 31–39), 1월 특례(라인 41–49)까지 핵심 케이스를 나눠서 검증한 점이 좋습니다.
BootcampPeriod.calculateGeneration이Integer를 반환하고, 유효 케이스에서는isEqualTo, 이후에는isNull로 검증하는 방식도 의도가 분명합니다.현재 범위 내에서는 특별히 수정이 필요해 보이지 않습니다.
41-49: 1월 입사자의 기수 매핑 도메인 규칙 확인 제안라인 44–48에서
2026-01-15를 11기로, 즉 2025년 하반기 기수로 계산하는 테스트를 두셨는데, 이는 꽤 강한 도메인 가정을 내포합니다(“1월 입사자는 직전 해 하반기 기수로 본다”).테스트 자체는 구현 의도를 잘 고정해 주지만, 도메인 정책/기획서와 이 규칙이 정확히 일치하는지 한 번만 더 확인해 보시면 좋겠습니다. 나중에 정책이 바뀌면 이 테스트가 변경 포인트가 될 수 있습니다.
techeerzip/src/main/java/backend/techeerzip/domain/feedback/repository/FeedbackRepositoryCustom.java (1)
27-33:findReceivedFeedbacks시그니처 포매팅 변경 이상 없습니다
- 파라미터 목록을 다줄로 나눈 변경이라 동작 영향은 없습니다.
FeedbackRepositoryImpl.findReceivedFeedbacks(...),FeedbackService,FeedbackController에서 사용하는 인자 순서와도 일치합니다.techeerzip/src/main/java/backend/techeerzip/domain/feedback/service/FeedbackService.java (1)
158-201: 멘토 권한/소유자 검증 + 기간/커서 필터링 로직이 잘 정리되었습니다
validateMentorRole(currentUser)호출 뒤에,mentorId와currentUser.getUserId()가 일치하거나currentUser.isAdmin()인 경우에만 허용해서 멘토 전용/자기 자신의 데이터만 보도록 잘 막고 있습니다.startDateTime == null인 경우LocalDateTime.now().minusMonths(1)로 기본 조회 범위를 최근 1개월로 제한한 것도 합리해 보입니다만, “기본 1개월”이 기획에서 명시된 요구사항인지 한 번만 더 확인해 주세요.endDateTime이startDateTime보다 이전인 잘못된 요청에 대해서는 현재 단순히 빈 결과를 반환하는 형태라, 이를 400 에러로 바꾸고 싶다면 서비스 레벨에서 선행 검증을 추가하는 것도 고려해 볼 수 있습니다.techeerzip/src/main/java/backend/techeerzip/domain/feedback/controller/FeedbackController.java (1)
238-258: 멘토 조회 API에서currentUser를 서비스로 전달하는 변경이 적절합니다
getReceivedFeedbacksByStatus/getAllReceivedFeedbacks호출 시mentorId로currentUser.getUserId()를 넘기면서, 마지막 인자로currentUser자체도 전달하고 있어 서비스 단의 멘토 권한 및 소유자 검증 로직과 정확히 매칭됩니다.- 이 변경으로 컨트롤러–서비스 사이 인자 불일치나 권한 체크 누락 없이, 멘토 본인/관리자만 데이터에 접근하도록 경계가 잘 맞춰진 상태입니다.
techeerzip/src/main/java/backend/techeerzip/domain/feedback/repository/FeedbackRepositoryImpl.java (1)
171-208:findReceivedFeedbacks에서 멘토별 상태/기간/커서 필터를 한 곳에 모은 설계가 깔끔합니다
- 기본 조건을
recipientId와isDeleted = false로 두고,status,startDateTime,endDateTime,cursor를 옵션으로.where(...)에 순차적으로 추가하는 방식이라 가독성과 확장성이 좋습니다.- 기간 조건도
createdAt >= startDateTime,createdAt < endDateTime형태의 반개구간([start, end))으로 구현돼 있어, 컨트롤러/서비스에서 전달하는 값만 일관되게 맞춘다면 원하는 범위가 정확히 표현될 것 같습니다.limit + 1로 한 건을 더 가져오는 커서 페이징 패턴도 기존 메서드들과 동일해 보이고,FeedbackCursorResponse.from(feedbacks, limit)쪽에서 잘 처리하고 있다면 성능/동작 면에서 문제 없어 보입니다.techeerzip/src/main/java/backend/techeerzip/domain/user/service/UserService.java (1)
695-700:updateNickname권한 검증 책임 위치 확인 필요현재 서비스 계층의
updateNickname은 전달된userId에 대한 권한 검증 없이 바로 닉네임을 변경하고 있어, 이 메서드를 호출할 수 있는 경로만 확보되면 임의 유저의 닉네임을 바꿀 수 있는 구조입니다. 이번 PR에서 컨트롤러 레벨@PreAuthorize나 별도 보안 컴포넌트에서 이미userId == 인증 주체또는 적절한 역할 검증을 적용했다면 괜찮지만, 다른 경로(테스트용 엔드포인트, 배치 등)에서 이 서비스를 직접 호출할 수 있다면 여기에도 최소한의 권한 체크를 두는 편이 더 안전합니다.
현재 적용한@PreAuthorize설정과 테스트들이 이 메서드 호출 경로를 충분히 커버하는지 한 번만 더 확인 부탁드립니다.techeerzip/build.gradle (1)
74-82: 테스트 의존성 구성 방향 적절합니다테스트 코드에서도 Lombok을 쓰고, H2·security-test·Testcontainers를 테스트 스코프로만 두어서 main 런타임 의존성과 분리된 구성이 잘 잡혀 있습니다. CI에서 실제로 사용하는 DB/보안 관련 테스트들이 이 조합으로 모두 커버되는지만 한 번만 확인해 주세요.
techeerzip/src/test/java/backend/techeerzip/domain/role/entity/RoleTest.java (1)
1-36: Role 생성 및 부모 관계 테스트가 간단명료합니다이름만 있는 Role, 부모 Role 을 갖는 Role 두 케이스를 분리해서 검증하고 있어서 엔티티 생성 로직과 parent 연관관계 기본 동작을 잘 커버하고 있는 것 같습니다.
techeerzip/src/test/java/backend/techeerzip/domain/user/entity/PermissionRequestTest.java (1)
1-74: PermissionRequest 생성 및 상태 전이 테스트가 핵심 흐름을 잘 검증합니다빌더로 생성 시 PENDING 상태,
approve()호출 시 APPROVED,reject()호출 시 REJECT 로 바뀌는 핵심 상태 전이가 모두 테스트되어 있어서 엔티티 수준 로직 커버리지가 좋습니다.UserTestHelper를 활용해 User 생성도 일관되게 처리한 점도 👍 입니다.techeerzip/src/test/java/backend/techeerzip/domain/userExperience/repository/UserExperienceRepositoryTest.java (1)
110-161: LGTM! 벌크 업데이트 테스트가 올바르게 구현되었습니다.
@Transactional어노테이션을 사용하여 벌크 업데이트 후 엔티티를 다시 조회하고 isDeleted 플래그를 확인하는 방식이 적절합니다. clearAutomatically와 flushAutomatically 옵션의 동작을 올바르게 검증합니다.techeerzip/src/test/java/backend/techeerzip/domain/user/repository/UserRepositoryTest.java (1)
38-84: LGTM! 테스트 커버리지가 우수합니다.findByEmail 메서드에 대한 세 가지 시나리오(존재하는 사용자, 존재하지 않는 이메일, 삭제된 사용자)를 모두 테스트합니다. 특히 소프트 삭제된 사용자가 조회되지 않는지 확인하는 테스트는 프로덕션 코드의
isDeleted = false필터링이 올바르게 작동하는지 검증합니다.techeerzip/src/test/java/backend/techeerzip/domain/user/repository/PermissionRequestRepositoryTest.java (1)
115-161: LGTM! 벌크 업데이트 테스트가 잘 구현되었습니다.테스트가 업데이트 전후의 PENDING 상태 요청 수를 올바르게 비교하고, 업데이트된 레코드 수가 0보다 큰지 확인합니다. @transactional 사용도 적절합니다.
techeerzip/src/test/java/backend/techeerzip/domain/user/entity/UserTest.java (1)
1-228: LGTM! 엔티티 테스트가 포괄적이고 잘 구성되어 있습니다.User 엔티티의 생성, 업데이트, 삭제 동작을 철저히 테스트합니다:
- 전체 필드 및 최소 필드를 사용한 생성
- 전체 및 부분 필드 업데이트
- null 처리를 포함한 피드백 노트 업데이트
- 소프트 삭제 플래그 토글
@nested를 사용한 테스트 구조와 헬퍼 메서드 사용이 가독성을 높입니다.
techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.java (1)
111-469: 컨트롤러 엔드포인트 테스트 커버리지가 우수합니다.모든 주요 엔드포인트(회원가입, 삭제, 프로필 조회/업데이트, 권한 요청, GitHub 동기화 등)를 테스트하며, MockMvc를 사용한 HTTP 요청/응답 검증이 잘 구현되어 있습니다. multipart 요청 처리도 올바르게 테스트되었습니다.
techeerzip/src/test/java/backend/techeerzip/domain/role/helper/RoleTestHelper.java (1)
7-65: LGTM! 테스트 헬퍼 클래스가 잘 설계되었습니다.
- private 생성자로 인스턴스화 방지
- 다양한 사용 사례를 위한 여러 팩토리 메서드 제공
- Default 내부 클래스로 자주 사용되는 테스트 Role 제공
- 명확한 주석으로 각 메서드의 목적을 설명
중복 메서드 문제만 해결하면 훌륭한 테스트 유틸리티 클래스입니다.
techeerzip/src/main/java/backend/techeerzip/domain/userExperience/repository/UserExperienceRepository.java (1)
13-14: Verify thatue.userIdis used correctly in the JPQL query.The query change from
ue.user.idtoue.userIdis correct and reflects UserExperience's unique entity structure. Unlike other entities (Session, Resume, ProjectMember) that use only a@ManyToOne userrelationship, UserExperience has both auserIdcolumn field and a read-onlyuserrelationship (withinsertable=false, updatable=false). Usingue.userIddirectly in the WHERE clause is more efficient as it avoids unnecessary lazy loading and accesses the foreign key column directly.@Modifying(clearAutomatically = true, flushAutomatically = true) @Query("UPDATE UserExperience ue SET ue.isDeleted = true WHERE ue.userId = :userId")techeerzip/src/test/java/backend/techeerzip/domain/user/helper/UserTestHelper.java (2)
8-10: LGTM! 테스트 헬퍼 클래스 구조가 잘 설계되었습니다.private 생성자를 통해 유틸리티 클래스의 인스턴스화를 방지하는 패턴이 적절합니다.
66-88:createUserWithAllFields메서드가 테스트 데이터 생성에 유용합니다.모든 필드를 포함한 User 객체를 생성하여 통합 테스트에 적합한 메서드입니다.
techeerzip/src/main/java/backend/techeerzip/global/permission/UserPermissionEvaluator.java (2)
79-85: 예외 처리에서UserNotFoundException이PermissionDeniedException으로 변환됩니다.Line 61에서 발생하는
UserNotFoundException이 catch 블록에서PermissionDeniedException으로 변환되어 원래 에러 컨텍스트가 손실됩니다. 사용자가 존재하지 않는 경우와 권한이 없는 경우를 구분할 수 없게 됩니다.의도된 설계인지 확인해주세요.
} catch (PermissionDeniedException e) { throw e; + } catch (UserNotFoundException e) { + throw e; } catch (Exception e) { log.error( "User 권한 검사 중 오류 - targetUserId: {}, error: {}", targetUserId, e.getMessage()); throw new PermissionDeniedException(); }
88-93:isSupportedPermission메서드가 깔끔하게 구현되었습니다.Switch expression을 사용하여 지원되는 권한을 명확하게 정의했습니다.
techeerzip/src/main/java/backend/techeerzip/global/permission/UserExperiencePermissionEvaluator.java (2)
69-77:UserExperienceNotFoundException이PermissionDeniedException으로 변환됩니다.
UserPermissionEvaluator와 동일한 패턴입니다. Line 50에서 발생하는UserExperienceNotFoundException이 catch 블록에서PermissionDeniedException으로 변환되어 원래 에러 컨텍스트가 손실됩니다.일관성을 위해 두 evaluator에서 동일한 예외 처리 정책을 적용하는 것이 좋습니다.
} catch (PermissionDeniedException e) { throw e; + } catch (UserExperienceNotFoundException e) { + throw e; } catch (Exception e) { log.error( "UserExperience 권한 검사 중 오류 - experienceId: {}, error: {}", experienceId, e.getMessage()); throw new PermissionDeniedException(); }
24-27: LGTM!supports메서드가 명확하게 구현되었습니다.
UserExperiencePermissionEvaluator가DomainPermissionEvaluator패턴을 일관성 있게 따르고 있습니다.techeerzip/src/main/java/backend/techeerzip/domain/user/controller/UserController.java (3)
83-93: LGTM!@PreAuthorize애노테이션이 올바르게 적용되었습니다.
#userIdSpEL 표현식이 메서드 파라미터와 정확히 매핑되어 있으며,UserPermissionEvaluator의 지원 권한과 일치합니다.
202-210: LGTM!deleteExperience엔드포인트의 권한 검사가 올바릅니다.
#experienceId가@PathVariable과 정확히 매핑되어UserExperiencePermissionEvaluator에서 소유권을 검증합니다.
212-230:UPDATE_NICKNAME권한이 적절하게 적용되었습니다.ADMIN/MENTOR 역할만 닉네임을 변경할 수 있도록
UserPermissionEvaluator에서 추가 검증이 이루어집니다.techeerzip/src/test/java/backend/techeerzip/domain/userExperience/helper/UserExperienceTestHelper.java (2)
9-11: LGTM! 테스트 헬퍼 클래스가 일관된 패턴으로 구현되었습니다.
UserTestHelper와 동일한 패턴을 따르며, private 생성자로 유틸리티 클래스 특성을 잘 표현했습니다.
89-98:createOngoingExperience메서드가 진행 중인 경력 테스트에 유용합니다.
endDate=null과isFinished=false조합으로 진행 중인 경력 시나리오를 테스트할 수 있습니다.techeerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.java (5)
165-221: LGTM!회원 가입 성공 테스트가 체계적으로 구성되어 있습니다. 이메일 인증, 이력서 생성, 프로필 이미지 메타데이터 등 주요 흐름이 잘 검증되고 있습니다.
635-649: MockedStatic 사용이 적절합니다
BootcampPeriod.calculateGeneration()정적 메서드를MockedStatic으로 mocking하여 부트캠프 기간 로직을 테스트하는 방식이 적절합니다. try-with-resources 패턴으로 자원 정리도 올바르게 처리되고 있습니다.
1252-1314: LGTM!
getIdAndUserMap테스트에서 정상 케이스와 존재하지 않는 사용자 ID 포함 케이스를 모두 다루고 있어 커버리지가 좋습니다.
99-131: 테스트 구조가 잘 설계되어 있습니다
@ExtendWith(MockitoExtension.class)와@Nested클래스를 활용한 테스트 구조가 체계적입니다. 많은 의존성을 mock하여 UserService를 격리 테스트하는 방식이 적절합니다.
1493-1521: Test correctly verifies bootcampYear behavior - no inconsistency foundThe UserService implementation at line 850 explicitly sets
bootcampYearto0when canceling participation. The test at line 1519 correctly asserts this value. The semantic difference betweennull(not participated) and0(cancelled after participating) is intentional and consistent with the service logic.
| Long roleId = targetUser.getRole().getId(); | ||
|
|
||
| if (roleId != 1L && roleId != 2L) { | ||
| log.warn( | ||
| "UPDATE_NICKNAME 권한 없음 - userId: {}, roleId: {})", | ||
| targetUserId, | ||
| roleId); | ||
| throw new PermissionDeniedException(); | ||
| } |
There was a problem hiding this comment.
roleId 매직 넘버 사용은 유지보수성과 안정성 문제를 야기할 수 있습니다.
roleId != 1L && roleId != 2L 조건은 데이터베이스의 role ID에 직접 의존합니다. Role ID가 변경되거나 환경마다 다를 경우 이 코드는 조용히 실패합니다.
Role 엔티티의 name 필드를 사용하거나 상수/Enum으로 관리하는 것이 더 안전합니다.
if ("UPDATE_NICKNAME".equals(permission)) {
User targetUser =
userRepository
.findById(targetUserId)
.orElseThrow(UserNotFoundException::new);
- Long roleId = targetUser.getRole().getId();
+ String roleName = targetUser.getRole().getName();
- if (roleId != 1L && roleId != 2L) {
+ if (!"ADMIN".equals(roleName) && !"MENTOR".equals(roleName)) {
log.warn(
- "UPDATE_NICKNAME 권한 없음 - userId: {}, roleId: {})",
+ "UPDATE_NICKNAME 권한 없음 - userId: {}, roleName: {})",
targetUserId,
- roleId);
+ roleName);
throw new PermissionDeniedException();
}
}
techeerzip/src/test/java/backend/techeerzip/domain/role/helper/RoleTestHelper.java
Show resolved
Hide resolved
...src/test/java/backend/techeerzip/domain/user/repository/PermissionRequestRepositoryTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
techeerzip/src/test/java/backend/techeerzip/domain/role/helper/RoleTestHelper.java (1)
22-24: 중복 메서드 문제가 여전히 존재합니다.이전 리뷰에서 지적된 대로, 이 메서드는
createRoleWithId(Long id, String name)(라인 40-44)와 기능이 동일합니다. 현재는 위임(delegation)을 사용하도록 개선되었지만, 여전히 두 메서드가 모두 존재하여 불필요한 중복이 남아있습니다.다음 중 하나를 선택하여 완전히 해결하는 것을 권장합니다:
- 한 메서드를 제거하고 모든 호출처를 다른 메서드로 통일
- 유지가 필요하다면 한 메서드에
@Deprecated추가 및 Javadoc으로 대체 메서드 명시
🧹 Nitpick comments (1)
techeerzip/src/test/java/backend/techeerzip/domain/role/helper/RoleTestHelper.java (1)
30-44: 테스트 헬퍼 메서드들이 적절하게 구현되었습니다.ReflectionTestUtils를 사용한 private 필드 설정은 테스트 코드에서 일반적이고 적절한 패턴입니다.
선택적 개선사항: 테스트 실패 시 더 명확한 디버깅을 위해
createRoleWithId메서드에 null 검증을 추가할 수 있습니다. 하지만 테스트 코드의 단순성을 고려하면 현재 구현도 충분합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
techeerzip/src/main/java/backend/techeerzip/domain/todayCs/scheduler/TodayCsScheduler.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/domain/role/helper/RoleTestHelper.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/domain/user/repository/PermissionRequestRepositoryTest.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- techeerzip/src/main/java/backend/techeerzip/domain/todayCs/scheduler/TodayCsScheduler.java
🚧 Files skipped from review as they are similar to previous changes (1)
- techeerzip/src/test/java/backend/techeerzip/domain/user/repository/PermissionRequestRepositoryTest.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
techeerzip/src/test/java/backend/techeerzip/domain/role/helper/RoleTestHelper.java (2)
7-9: LGTM! 유틸리티 클래스 패턴이 올바르게 적용되었습니다.private 생성자를 사용하여 인스턴스화를 방지하는 것은 정적 팩토리 메서드만 제공하는 유틸리티 클래스의 표준 패턴입니다.
46-62: LGTM! 기본 테스트 Role 프리셋이 잘 구성되었습니다.중첩된
Default클래스는 표준 테스트 Role 인스턴스를 제공하여 테스트 코드의 가독성과 재사용성을 향상시킵니다._TEST접미사는 테스트 데이터임을 명확히 표시합니다.
dongwooooooo
left a comment
dongwooooooo
left a comment
There was a problem hiding this comment.
수고하셨어요~ 코멘트 확인 부탁드립니다.
| return false; | ||
| } | ||
|
|
||
| CustomUserPrincipal user = (CustomUserPrincipal) auth.getPrincipal(); |
There was a problem hiding this comment.
유저 컨트롤러에서 currentUser는 아래 코드 방식으로 받아옵니다.
결국 컨트롤러에서 요청을 날린 유저를 통해 tartgetId를 가져와서 넘겨주니깐,
targetUser는 중복 조회인 것 같아요. 한번 체크해보시면 좋겠습니다.
| try (InputStream is = | ||
| SharedTestContainer.class | ||
| .getClassLoader() | ||
| .getResourceAsStream("env.properties")) { |
There was a problem hiding this comment.
테스트 컨테이너가 돌아가는 ci환경은 env.properties를 사용하지 않습니다.
| return current; | ||
| } | ||
|
|
||
| synchronized (LOCK) { |
There was a problem hiding this comment.
컨테이너를 테스트 별로 재시작 하지않고 하나의 컨테이너로 테스트들을 실행하는건 정말 좋은 것 같습니다! 다만 락 없이 구현하는 경우 어떤 문제가 발생할까요?
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
techeerzip/build.gradle (1)
117-123:test태스크 설정 중복 + coverage verification 미연결 정리 필요현재
tasks.named('test', Test)(117-122줄)과tasks.test(229-231줄)에서useJUnitPlatform()과finalizedBy jacocoTestReport를 중복 설정하고 있습니다. 향후 한쪽만 수정되면 드리프트가 발생할 수 있으므로 한 곳으로 통합하는 것이 안전합니다.또한
jacocoTestCoverageVerification태스크는 정의되어 있으나(206-226줄)check태스크에 연결되어 있지 않아, 기본 빌드 라이프사이클에서 coverage 검증이 실행되지 않습니다. CI/CD 파이프라인에서 검증을 강제하려면check태스크에 의존성을 추가해야 합니다.제안(한 곳으로 정리 + check에 연결):
tasks.named('test', Test) { systemProperty 'spring.profiles.active', 'test' systemProperty 'spring.config.location', "$projectDir/src/test/resources/application-test.properties" useJUnitPlatform() finalizedBy tasks.jacocoTestReport } -// 테스트 task 마무리 설정 -tasks.test { - useJUnitPlatform() - finalizedBy tasks.jacocoTestReport -} +tasks.named('check') { + dependsOn tasks.jacocoTestCoverageVerification +}다만 Spring Boot 3.3.1의 BOM이 testcontainers 1.19.x를 자동으로 관리하므로 현재 의존성 구성은 버전 호환성 측면에서 안정적입니다.
♻️ Duplicate comments (1)
techeerzip/src/test/java/backend/techeerzip/config/SharedTestContainer.java (1)
12-22:withReuse(true)는 CI에서 기대대로 동작하지 않을 수 있으니 동작 전제(설정) 확인 필요
withReuse(true)는 보통 로컬의~/.testcontainers.properties (testcontainers.reuse.enable=true)같은 전역 설정이 없으면 “재사용”이 비활성화될 수 있습니다(그래도 컨테이너 자체는 뜨지만 재사용 최적화가 적용 안 될 수 있음). CI 최적화 목적인지/로컬만 노린 건지 전제만 명확히 해주세요. 또한 “락 없이 구현 시 문제?”에 대해선, 같은 JVM/같은 ClassLoader 내에서는 class initialization이 동기화되므로 이 패턴 자체는 안전한 편입니다(단, Gradle fork/병렬 JVM이면 JVM별로 컨테이너가 각각 뜨는 건 정상).가능하면 이미지 선언은 Testcontainers 권장 방식으로도 고정해두는 걸 추천합니다:
+import org.testcontainers.utility.DockerImageName; ... public static final PostgreSQLContainer<?> POSTGRES = - new PostgreSQLContainer<>("postgres:14-alpine").withReuse(true); + new PostgreSQLContainer<>(DockerImageName.parse("postgres:14-alpine")) + .withReuse(true);
🧹 Nitpick comments (6)
techeerzip/src/main/java/backend/techeerzip/domain/role/entity/RoleType.java (2)
10-16:Long roleIdnullable 설계로 인한 NPE(오토 언박싱) 위험을 호출부에서 흡수하게 됩니다.
COMPANY/BOOTCAMP이null이므로, 호출부에서long으로 받거나 비교/연산 시 오토 언박싱되면 NPE가 납니다.public Optional<Long> getRoleId()로 바꾸거나, 최소한@Nullable(또는 동등한 애노테이션)로 의도를 강하게 드러내고 “ID가 필요한 코드 경로”에는requireRoleId()처럼 예외를 던지는 가드 메서드를 두는 걸 추천합니다.
22-28:getRoleId()는 오용 여지가 있어 네이밍/문서 보강을 권장합니다.
현재 Javadoc은 “마이그레이션에 정의된 고정 ID”를 반환한다고 되어 있어, 추후 권한 판정/비즈니스 로직에서roleName대신roleId를 섞어 쓰는 오용이 생길 수 있습니다. 메서드명을getSeedRoleId()/getFixedRoleId()처럼 더 구체화하거나, “권한 판단에는 roleName 사용” 같은 주의 문구를 추가하는 편이 안전합니다.techeerzip/src/test/java/backend/techeerzip/config/TestExternalServiceConfig.java (1)
28-64: Mock 범위가 넓어서 “실수로 외부 연동이 필요한 테스트”까지 가려질 수 있음(스코프 의도 명확화 추천)
@MockBean으로 Slack/S3/Redis/GoogleDrive 관련 빈을 전부 대체하는 접근은 통합 테스트 안정성엔 좋지만, 반대로 “외부 연동을 검증해야 하는 테스트”가 생기면 같은 베이스 설정을 재사용할 때 원인 파악이 어려울 수 있습니다. (예: 특정 테스트에서만 이 Config를 import 하거나, 별도 베이스 클래스로 분리)또한
@MockBean(name = "stringRedisTemplate")가 실제 애플리케이션의 빈 이름과 일치하는지(혹은 별도 qualifier로 참조되는지)만 확인해주세요.techeerzip/src/test/java/backend/techeerzip/TecheerzipApplicationTests.java (1)
3-17:@Import(TestExternalServiceConfig.class)는 베이스 클래스와 중복 +System.out.println은 테스트 노이즈
IntegrationTestSupport가 이미@Import(TestExternalServiceConfig.class)를 가지고 있어서(Line 22 in IntegrationTestSupport) 여기서는 중복입니다. 또한println대신 “컨텍스트 로드/컨테이너 연결”을 최소한의 assert로 검증하는 편이 안정적입니다.-import org.springframework.context.annotation.Import; ... -import backend.techeerzip.config.TestExternalServiceConfig; ... -@Import(TestExternalServiceConfig.class) class TecheerzipApplicationTests extends IntegrationTestSupport { @Test void contextLoads() { - System.out.println("연결된 DB URL: " + POSTGRES.getJdbcUrl()); + org.assertj.core.api.Assertions.assertThat(POSTGRES.isRunning()).isTrue(); } }techeerzip/src/test/java/backend/techeerzip/config/IntegrationTestSupport.java (1)
20-36:@ServiceConnection패턴은 좋음(다만 Spring Boot/Testcontainers 버전 호환만 재확인)
공용 베이스로@ServiceConnection를 통해 datasource를 자동 주입하는 방향은 테스트 유지보수에 유리합니다. 현재 프로젝트의 Spring Boot(3.3.1)에서는 지원되는 패턴이지만, Testcontainers/부트 BOM 정합성만 깨지면 바로 실패하니 버전 정책만 확인 부탁드립니다.(선택) 서브클래스에서 POSTGRES 접근이 필요하면
protected로 열어두는 것도 고려할 만합니다.techeerzip/src/test/java/backend/techeerzip/domain/user/repository/UserRepositoryTest.java (1)
38-84: 전반적으로 명확한 리포지토리 동작 검증(Deleted user 케이스 fixture만 정책과 일치시키면 더 안전)
테스트 구조/의도는 깔끔합니다. 다만createDeletedUser(..., "password123", ...)의 비밀번호 값이 프로젝트의 비밀번호 정책/길이 제약(컬럼 길이/도메인 규칙)과 다르면 환경에 따라 깨질 수 있으니, 다른 테스트들과 동일한 패턴(예:"Techeerzip1!")으로 맞추는 것도 고려해주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
techeerzip/build.gradle(3 hunks)techeerzip/src/main/java/backend/techeerzip/domain/role/entity/RoleType.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/global/permission/UserPermissionEvaluator.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/TecheerzipApplicationTests.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/config/IntegrationTestSupport.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/config/RepositoryTestSupport.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/config/SharedTestContainer.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/config/TestExternalServiceConfig.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/domain/role/repository/RoleRepositoryTest.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/domain/user/repository/PermissionRequestRepositoryTest.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/domain/user/repository/UserRepositoryTest.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/domain/userExperience/repository/UserExperienceRepositoryTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- techeerzip/src/test/java/backend/techeerzip/domain/role/repository/RoleRepositoryTest.java
- techeerzip/src/main/java/backend/techeerzip/global/permission/UserPermissionEvaluator.java
- techeerzip/src/test/java/backend/techeerzip/domain/user/repository/PermissionRequestRepositoryTest.java
🧰 Additional context used
🧬 Code graph analysis (4)
techeerzip/src/test/java/backend/techeerzip/config/RepositoryTestSupport.java (1)
techeerzip/src/test/java/backend/techeerzip/config/SharedTestContainer.java (1)
SharedTestContainer(5-23)
techeerzip/src/test/java/backend/techeerzip/config/IntegrationTestSupport.java (3)
techeerzip/src/test/java/backend/techeerzip/TecheerzipApplicationTests.java (1)
Import(11-18)techeerzip/src/test/java/backend/techeerzip/config/RepositoryTestSupport.java (1)
ActiveProfiles(7-17)techeerzip/src/test/java/backend/techeerzip/config/SharedTestContainer.java (1)
SharedTestContainer(5-23)
techeerzip/src/test/java/backend/techeerzip/domain/userExperience/repository/UserExperienceRepositoryTest.java (6)
techeerzip/src/test/java/backend/techeerzip/domain/user/entity/UserTest.java (1)
DisplayName(13-228)techeerzip/src/test/java/backend/techeerzip/domain/user/repository/PermissionRequestRepositoryTest.java (1)
DataJpaTest(28-150)techeerzip/src/test/java/backend/techeerzip/config/RepositoryTestSupport.java (1)
ActiveProfiles(7-17)techeerzip/src/test/java/backend/techeerzip/domain/role/helper/RoleTestHelper.java (2)
RoleTestHelper(7-63)Default(46-62)techeerzip/src/test/java/backend/techeerzip/domain/user/helper/UserTestHelper.java (1)
UserTestHelper(8-104)techeerzip/src/test/java/backend/techeerzip/domain/userExperience/helper/UserExperienceTestHelper.java (1)
UserExperienceTestHelper(9-124)
techeerzip/src/test/java/backend/techeerzip/TecheerzipApplicationTests.java (1)
techeerzip/src/test/java/backend/techeerzip/config/SharedTestContainer.java (1)
SharedTestContainer(5-23)
🔇 Additional comments (1)
techeerzip/src/test/java/backend/techeerzip/config/RepositoryTestSupport.java (1)
7-17: 레포지토리 테스트에서 공용 컨테이너 재사용 패턴 LGTM (슈퍼클래스 애노테이션 인식만 확인)
@DataJpaTest클래스가 이 베이스를 상속할 때@ServiceConnection이 기대대로 적용되는지(슈퍼클래스 정적 필드/애노테이션 인식)만 한 번 확인되면 깔끔한 구성입니다.
techeerzip/src/main/java/backend/techeerzip/domain/role/entity/RoleType.java
Outdated
Show resolved
Hide resolved
...t/java/backend/techeerzip/domain/userExperience/repository/UserExperienceRepositoryTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
techeerzip/src/main/java/backend/techeerzip/domain/role/entity/RoleType.java (1)
4-8: 이전 리뷰에서 지적된 데이터 일관성 문제가 여전히 유효합니다.하드코딩된 roleId 값들(1L~5L)과 실제 DB의 ID 값 간 불일치 가능성에 대한 우려는 이전 리뷰에서 이미 상세히 논의되었습니다. Role 엔티티가 시퀀스 기반 ID 생성을 사용하므로, 애플리케이션 부팅 시 enum의 roleId와 DB의 실제 role.id 값이 일치하는지 검증하는 통합 테스트 추가를 권장합니다.
🧹 Nitpick comments (9)
techeerzip/src/main/java/backend/techeerzip/domain/user/service/UserService.java (1)
224-239: flush 및 재조회 패턴 검토JPA 영속성 컨텍스트 동기화를 위해
flush()후 재조회하는 방식은 동작하지만,savedUser가 이미save()반환값으로 관리 상태(managed state)여야 합니다.만약 더티 체킹이 예상대로 동작하지 않는 문제를 해결하기 위한 것이라면 이 접근 방식은 합리적입니다. 다만,
flush()+findById()대신saveAndFlush()를 사용하면 한 번의 호출로 동일한 효과를 얻을 수 있습니다.- userRepository.flush(); - - User managedUser = - userRepository - .findById(savedUser.getId()) - .orElseThrow(() -> new UserNotFoundException()); + User managedUser = userRepository.saveAndFlush(savedUser);또는 현재 방식을 유지하려면, 왜 재조회가 필요한지에 대한 주석을 추가하는 것이 좋습니다.
techeerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.java (2)
117-120: 사용되지 않는 Mock 선언 제거 권장
RestTemplate과PlatformTransactionManagerMock이 선언되어 있지만, 테스트 코드에서 실제로 사용되지 않습니다. 불필요한 Mock은 테스트의 가독성을 저해할 수 있습니다.- @Mock private RestTemplate restTemplate; @Mock private StatisticDataCollectionService statisticDataCollectionService; @Mock private GitHubApiService gitHubApiService; - @Mock private PlatformTransactionManager transactionManager;
1427-1439: 불필요한 MockedStatic 사용
isParticipate메서드는BootcampGeneration.calculateBootcampGeneration()을 호출하지 않고, 단순히userBootcampYear와currentGeneration파라미터를 비교합니다. 따라서MockedStatic<BootcampGeneration>은 불필요합니다.when(userRepository.findById(userId)).thenReturn(Optional.of(user)); - try (MockedStatic<BootcampGeneration> mocked = mockStatic(BootcampGeneration.class)) { - mocked.when( - () -> - BootcampGeneration.calculateBootcampGeneration( - any(LocalDate.class))) - .thenReturn(3); - Integer currentGeneration = 3; - - boolean result = userService.isParticipate(userId, currentGeneration); + Integer currentGeneration = 3; - assertThat(result).isTrue(); - verify(userRepository).findById(userId); - } + boolean result = userService.isParticipate(userId, currentGeneration); + + assertThat(result).isTrue(); + verify(userRepository).findById(userId);techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.java (6)
119-166: Multipart 회원가입 테스트가 “빈 파일(0 bytes)”에 의존해 Validation 변경 시 깨질 수 있습니다.
컨트롤러/서비스가 “파일이 오면 non-empty”를 기대하는 순간 테스트가 불안정해집니다(파일 optional이면 아예 file part를 생략하는 케이스도 별도 테스트로 두는 편이 안전). 또한 현재는 200만 확인하므로 service 호출 검증까지 붙이면 회귀 방지에 도움이 됩니다.
173-180: 탈퇴 API는 status만 검증 중인데, 최소한 service 호출 1회 보장은 추가하는 편이 좋습니다.
HttpServletResponse가 들어가서 matcher가 어렵다면verify(userService).deleteUser(eq(TEST_USER_ID), any(HttpServletResponse.class))정도라도 추천합니다.
187-199: 비밀번호 재설정도 200만 확인합니다 — 실패 케이스(인증코드 불일치/만료 등) 4xx 테스트가 있으면 좋습니다.
PreAuthorize 범위 밖이라도, controller validation/예외 매핑 회귀를 막는 데 유용합니다.
230-243: 요청 바디를 문자열로 조립하고 있는데 DTO/ObjectMapper 사용으로 일관성 있게 유지하는 편이 좋습니다.
문자열 조립은 필드명 변경/이스케이프에서 쉽게 깨집니다.
392-425: Reflection으로 DTO private field 세팅은 매우 brittle 합니다(리팩터링에 취약).
테스트 픽스처(빌더/팩토리) 제공, 혹은 JSON을 Map으로 만들어requestPart만 구성하는 방식으로 reflection 제거를 추천합니다.
456-471: GitHub URL 검증(Bean Validation) 테스트를 추가하는 게 좋습니다.
retrieved learnings 기준으로 GitHub URL 검증은 request DTO에서 처리하는 방향이므로,invalid githubUrl→ 400(또는 프로젝트 표준 에러코드)을 보장하는 테스트가 있으면 회귀를 잘 막습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
techeerzip/src/main/java/backend/techeerzip/domain/role/entity/RoleType.java(1 hunks)techeerzip/src/main/java/backend/techeerzip/domain/user/controller/UserController.java(6 hunks)techeerzip/src/main/java/backend/techeerzip/domain/user/service/UserService.java(2 hunks)techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/domain/userExperience/repository/UserExperienceRepositoryTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- techeerzip/src/main/java/backend/techeerzip/domain/user/controller/UserController.java
- techeerzip/src/test/java/backend/techeerzip/domain/userExperience/repository/UserExperienceRepositoryTest.java
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-18T12:23:00.520Z
Learnt from: dlwhsk0
Repo: Techeer-Hogwarts/backend PR: 111
File: techeerzip/src/main/java/backend/techeerzip/domain/user/service/UserService.java:892-902
Timestamp: 2025-09-18T12:23:00.520Z
Learning: UserService의 updateGithubUrlAndSync 메서드에서 JPA 더티 체킹 대신 userRepository.updateGithubUrlById() 커스텀 업데이트 메서드를 사용했습니다. 더티 체킹이 예상대로 작동하지 않는 경우에 대한 실용적인 해결책입니다.
Applied to files:
techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.javatecheerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.javatecheerzip/src/main/java/backend/techeerzip/domain/user/service/UserService.java
📚 Learning: 2025-09-12T07:36:39.412Z
Learnt from: dlwhsk0
Repo: Techeer-Hogwarts/backend PR: 111
File: techeerzip/src/main/java/backend/techeerzip/infra/github/GitHubApiService.java:99-103
Timestamp: 2025-09-12T07:36:39.412Z
Learning: GitHub URL 검증은 서비스 레이어가 아닌 request DTO 단에서 Bean Validation 어노테이션을 사용하여 수행해야 한다.
Applied to files:
techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.java
🧬 Code graph analysis (2)
techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.java (1)
techeerzip/src/test/java/backend/techeerzip/domain/user/entity/BootcampPeriodTest.java (1)
DisplayName(13-62)
techeerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.java (7)
techeerzip/src/main/java/backend/techeerzip/domain/auth/exception/AuthNotVerifiedEmailException.java (1)
AuthNotVerifiedEmailException(6-10)techeerzip/src/main/java/backend/techeerzip/domain/user/exception/UserAlreadyExistsException.java (1)
UserAlreadyExistsException(6-10)techeerzip/src/main/java/backend/techeerzip/domain/user/exception/UserInvalidRoleException.java (1)
UserInvalidRoleException(6-10)techeerzip/src/main/java/backend/techeerzip/domain/user/exception/UserNotFoundException.java (1)
UserNotFoundException(6-10)techeerzip/src/main/java/backend/techeerzip/domain/user/exception/UserNotResumeException.java (1)
UserNotResumeException(6-10)techeerzip/src/main/java/backend/techeerzip/domain/user/exception/UserProfileImgFailException.java (1)
UserProfileImgFailException(6-10)techeerzip/src/main/java/backend/techeerzip/domain/userExperience/exception/UserExperienceNotFoundException.java (1)
UserExperienceNotFoundException(6-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (10)
techeerzip/src/main/java/backend/techeerzip/domain/role/entity/RoleType.java (2)
11-16: 구현 품질은 양호합니다.enum 필드 선언과 생성자 구현이 표준 Java 패턴을 올바르게 따르고 있습니다.
Long타입 사용은 null 처리 가능성을 고려할 때 적절한 선택입니다.
22-24: Getter 메서드 구현이 적절합니다.
getRoleId()메서드가 기존getRoleName()과 일관된 패턴을 따르며, 간결하고 명확하게 구현되어 있습니다.techeerzip/src/main/java/backend/techeerzip/domain/user/service/UserService.java (1)
675-676: 소프트 삭제 패턴 적용 확인
experience.delete()호출 후save()를 통해 변경사항을 저장하는 소프트 삭제 패턴이 적절히 적용되었습니다. 이는@Transactional어노테이션이 있어 더티 체킹으로 자동 반영될 수 있지만, 명시적인save()호출도 문제없습니다.techeerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.java (3)
161-330: 회원 가입 테스트 - 포괄적인 테스트 커버리지신규 회원 가입, 이메일 미인증, 이력서 파일 누락, 기존 회원, 삭제된 회원 재가입 등 다양한 시나리오가 잘 커버되어 있습니다.
flush()및findById()호출도 적절히 검증하고 있습니다.
472-510: 경력 삭제 테스트 - 변경된 구현과 일치
deleteExperience테스트가 새로운 소프트 삭제 패턴(entity.delete()+save())과 일치하도록 작성되었습니다.experience.isDeleted()가true인지 검증하는 것도 좋은 접근입니다.
1540-1619: GitHub URL 업데이트 테스트 - 학습 내용 반영
updateGithubUrlById()커스텀 업데이트 메서드 사용이 테스트에 잘 반영되어 있습니다. 이는 이전 학습에서 언급된 더티 체킹 대신 명시적 업데이트를 사용하는 패턴과 일치합니다.Based on learnings,
updateGithubUrlById사용은 더티 체킹이 예상대로 동작하지 않는 경우에 대한 실용적인 해결책입니다.techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.java (4)
206-223:GET /{userId}(특정 프로필 조회)가 인증 없이 200인데, 의도된 public API인지 확인 필요합니다.
원래 인증/인가 대상이면 현재 테스트가 정책과 어긋납니다(반대로 public이면 “인증 없이도 200”을 명시적으로 보장하는 테스트로 의미가 생깁니다).
302-333:@Valid가 강화되면 빈 DTO 전송 테스트가 400으로 바뀔 수 있습니다.
UpdateUserWithExperienceRequest,UpdateUserNicknameRequest를 최소 유효값으로 채워 보내면 테스트가 정책 변경에 더 탄탄해집니다.
340-352: 경력 삭제 테스트는 단순/명확해서 좋습니다.
가능하면 여기에도 service 호출 검증(verify)만 가볍게 추가하면 더 탄탄해집니다.
359-382: 프로필 목록 API는 query param/페이지네이션 계약이 있다면 현재 테스트가 충분히 대표적이지 않을 수 있습니다.
예: cursor/size 등이 필수로 바뀌는 순간 테스트가 깨질 수 있으니, 컨트롤러 시그니처에 맞춘 최소 파라미터 포함 버전을 권장합니다.
| @WebMvcTest( | ||
| controllers = UserController.class, | ||
| excludeAutoConfiguration = { | ||
| org.springframework.boot.autoconfigure.security.servlet.SecurityAutoConfiguration.class | ||
| }) |
There was a problem hiding this comment.
SecurityAutoConfiguration 제외로 @PreAuthorize 테스트가 무력화될 리스크가 큽니다.
PR 목표가 “PreAuthorize 작성”이라면, 현재 구성(excludeAutoConfiguration) + “항상 true” PermissionEvaluator 스텁은 인가 실패 케이스를 전혀 검증 못해 false positive 가능성이 큽니다. 최소한 (1) 인증 없음/권한 부족 시 401/403, (2) 올바른 role 시 200을 같이 보장하는 테스트를 추가하는 게 안전합니다.
예시 방향(프로젝트 보안 구성에 맞게 택1):
@WebMvcTest(
controllers = UserController.class,
- excludeAutoConfiguration = {
- org.springframework.boot.autoconfigure.security.servlet.SecurityAutoConfiguration.class
- })
+ excludeAutoConfiguration = {})
+// 또는 @AutoConfigureMockMvc(addFilters = true) / @Import(테스트용 SecurityConfig) 등🤖 Prompt for AI Agents
In
techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.java
around lines 63 to 67, excluding SecurityAutoConfiguration and stubbing
PermissionEvaluator to always return true removes real @PreAuthorize enforcement
and can produce false positives; update the test configuration to exercise both
denied and allowed authorization paths: either remove the
excludeAutoConfiguration so Spring Security runs in tests (or register the real
SecurityFilterChain/config used in the app), and/or adjust the
PermissionEvaluator stub to return false for unauthorized scenarios and true for
authorized scenarios, then add at least two MockMvc tests that (1) perform the
relevant request without authentication or with insufficient authorities and
assert 401/403, and (2) perform the request with proper authentication/roles and
assert 200; use SecurityMockMvcRequestPostProcessors (e.g., user().roles(...))
or authenticated tokens to simulate principals.
techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.java
Show resolved
Hide resolved
techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.java
Show resolved
Hide resolved
techeerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
techeerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.java (1)
1577-1607: “URL 변경 없음 → updateGithubUrlById 미호출” 검증 추가한 점 좋습니다.
이 케이스가 회귀 포인트라서 현재 형태가 훨씬 안전합니다.techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.java (3)
64-68:SecurityAutoConfiguration제외는@PreAuthorize/인증 주입 테스트를 불안정하게 만듭니다.
지금 설정은 “인가를 실제로 탄다”는 보장이 약해서(또는authentication(...)가 기대대로 안 먹어서) CI에서 200/403 기대가 쉽게 깨질 수 있어요. PR 목표가@PreAuthorize검증이면, 보안 필터/메서드 시큐리티가 실제로 적용되는 구성으로 테스트를 고정하는 쪽을 권장합니다.@WebMvcTest( controllers = UserController.class, - excludeAutoConfiguration = { - org.springframework.boot.autoconfigure.security.servlet.SecurityAutoConfiguration.class - }) + excludeAutoConfiguration = {})#!/bin/bash # (1) UserController의 @PreAuthorize 표현식 확인 rg -n --type=java "@PreAuthorize" -C2 # (2) UserController의 각 엔드포인트에 @Valid가 붙었는지 확인 (400 발생 원인 후보) rg -n --type=java "@Valid" techeerzip/src/main/java/backend/techeerzip/domain/user/controller/UserController.java -C2
96-101: PermissionEvaluator “항상 true” 기본 스텁은 설계 의도(특히 target=null)와 어긋나기 쉬움
DelegatingPermissionEvaluator는 target이 null이면 어떤 evaluator도 매칭되지 않는 설계(=대개 false 기대)인데, 여기선 전부 true로 깔려서 인가 회귀를 놓칠 가능성이 큽니다. 테스트별로 필요한 permission만 true로 여는 방식이 더 안전합니다. (관리자 권한은 hasPermission보다 hasRole('ADMIN') 사용 권장)
300-331: 403 테스트에 “서비스 미호출” 검증을 추가하면 인가 회귀를 더 잘 잡습니다.
지금은 status만 확인해서, 인가가 깨져도 서비스가 호출되는 회귀를 놓칠 수 있어요.void getPermissionRequests_Forbidden_WhenNotAdmin() throws Exception { List<GetPermissionResponse> response = List.of(); when(userService.getAllPendingPermissionRequests()).thenReturn(response); when(delegatingPermissionEvaluator.hasPermission(any(), any(), anyString(), any())) .thenReturn(false); mockMvc.perform( withSecurity( get(BASE_URL + "/admin/permission/request"), TEST_USER_ID, RoleType.TECHEER)) .andExpect(status().isForbidden()); + + verify(userService, never()).getAllPendingPermissionRequests(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.java(1 hunks)techeerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-09-09T10:19:12.637Z
Learnt from: kimzini
Repo: Techeer-Hogwarts/backend PR: 117
File: techeerzip/src/main/java/backend/techeerzip/global/permission/DelegatingPermissionEvaluator.java:37-43
Timestamp: 2025-09-09T10:19:12.637Z
Learning: DelegatingPermissionEvaluator에서 target이 null일 때 어떤 evaluator도 매칭되지 않는 것이 의도된 설계입니다. 어드민 권한 검증은 hasPermission이 아닌 hasRole('ADMIN')을 사용해야 합니다.
Applied to files:
techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.java
📚 Learning: 2025-09-18T12:23:00.520Z
Learnt from: dlwhsk0
Repo: Techeer-Hogwarts/backend PR: 111
File: techeerzip/src/main/java/backend/techeerzip/domain/user/service/UserService.java:892-902
Timestamp: 2025-09-18T12:23:00.520Z
Learning: UserService의 updateGithubUrlAndSync 메서드에서 JPA 더티 체킹 대신 userRepository.updateGithubUrlById() 커스텀 업데이트 메서드를 사용했습니다. 더티 체킹이 예상대로 작동하지 않는 경우에 대한 실용적인 해결책입니다.
Applied to files:
techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.javatecheerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.java
📚 Learning: 2025-09-12T07:36:39.412Z
Learnt from: dlwhsk0
Repo: Techeer-Hogwarts/backend PR: 111
File: techeerzip/src/main/java/backend/techeerzip/infra/github/GitHubApiService.java:99-103
Timestamp: 2025-09-12T07:36:39.412Z
Learning: GitHub URL 검증은 서비스 레이어가 아닌 request DTO 단에서 Bean Validation 어노테이션을 사용하여 수행해야 한다.
Applied to files:
techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.javatecheerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.java
📚 Learning: 2025-09-18T12:43:26.211Z
Learnt from: dlwhsk0
Repo: Techeer-Hogwarts/backend PR: 111
File: techeerzip/src/main/java/backend/techeerzip/domain/user/service/UserService.java:0-0
Timestamp: 2025-09-18T12:43:26.211Z
Learning: UserService의 updateGithubUrlAndSync 메서드가 단일 트랜잭션으로 수정되어 검증→저장→동기화 과정에서 동기화 실패 시 URL 저장도 함께 롤백되도록 개선되었습니다.
Applied to files:
techeerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.java
📚 Learning: 2025-09-12T07:44:45.395Z
Learnt from: dlwhsk0
Repo: Techeer-Hogwarts/backend PR: 111
File: techeerzip/src/main/java/backend/techeerzip/domain/user/service/UserService.java:0-0
Timestamp: 2025-09-12T07:44:45.395Z
Learning: 사용자가 GitHub URL 업데이트 및 동기화 기능에서 전체 과정(검증→저장→동기화)이 단일 트랜잭션으로 묶여야 한다고 명시했습니다. 동기화 실패 시 URL 저장도 롤백되어야 하는 요구사항입니다.
Applied to files:
techeerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.java
🧬 Code graph analysis (1)
techeerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.java (7)
techeerzip/src/main/java/backend/techeerzip/domain/auth/exception/AuthNotVerifiedEmailException.java (1)
AuthNotVerifiedEmailException(6-10)techeerzip/src/main/java/backend/techeerzip/domain/user/exception/UserAlreadyExistsException.java (1)
UserAlreadyExistsException(6-10)techeerzip/src/main/java/backend/techeerzip/domain/user/exception/UserInvalidRoleException.java (1)
UserInvalidRoleException(6-10)techeerzip/src/main/java/backend/techeerzip/domain/user/exception/UserNotFoundException.java (1)
UserNotFoundException(6-10)techeerzip/src/main/java/backend/techeerzip/domain/user/exception/UserNotResumeException.java (1)
UserNotResumeException(6-10)techeerzip/src/main/java/backend/techeerzip/domain/user/exception/UserProfileImgFailException.java (1)
UserProfileImgFailException(6-10)techeerzip/src/main/java/backend/techeerzip/domain/userExperience/exception/UserExperienceNotFoundException.java (1)
UserExperienceNotFoundException(6-10)
🪛 GitHub Actions: Other CI
techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.java
[error] 506-506: java.lang.AssertionError at UserControllerTest.java:506 (GitHub 동기화 테스트 → GitHub 데이터 동기화 성공 FAILED)
[error] 484-484: java.lang.AssertionError at UserControllerTest.java:484 (Techeer 변경 테스트 → Techeer 변경 성공 FAILED)
[error] 368-368: java.lang.AssertionError at UserControllerTest.java:368 (프로필 업데이트 테스트 → 닉네임 업데이트 성공 FAILED)
[error] 351-351: java.lang.AssertionError at UserControllerTest.java:351 (프로필 업데이트 테스트 → 프로필 업데이트 성공 FAILED)
[error] 330-330: java.lang.AssertionError at UserControllerTest.java:330 ([TECHEER] 권한 승인 시 권한 부족으로 실패 FAILED)
[error] 313-313: java.lang.AssertionError at UserControllerTest.java:313 (권한 요청 테스트 → 권한 요청 목록 조회 시 권한 부족으로 실패 FAILED)
[error] 267-267: java.lang.AssertionError at UserControllerTest.java:267 (권한 요청 테스트 → 권한 요청 성공 FAILED)
[error] 216-216: java.lang.AssertionError at UserControllerTest.java:216 (유저 정보 조회 → 유저 정보 조회 성공 FAILED)
[error] 182-182: java.lang.AssertionError at UserControllerTest.java:182 (회원 탈퇴 → 회원 탈퇴 성공 FAILED)
techeerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.java
[error] Process completed with exit code 1 due to failing tests in :test task.
🔇 Additional comments (2)
techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.java (2)
103-116: SecurityContext 정리 +authentication(...)적용은 좋습니다.
@AfterEach로 컨텍스트를 clear한 점은 플래키 방지에 확실히 도움 됩니다.
176-183: DTO 필드 검증 실패(400) 문제입니다. CSRF는 관계없습니다.
@WebMvcTest에서SecurityAutoConfiguration을 제외했으므로 CSRF 필터가 비활성화되어 있습니다. 따라서.with(csrf())를 추가할 필요가 없습니다.실제 문제는 DTO 검증 실패입니다:
updateProfile_Success: 빈UpdateUserWithExperienceRequest()는 중첩된@Valid필드들이 null이어서 400 에러 발생changeTecheer_Success: 리플렉션으로 필드를 설정하는 것은 DTO 검증 제약이 있음을 의미다른 테스트들과 달리 BootcampControllerTest는 SecurityAutoConfiguration을 제외하지 않아 CSRF 보호가 활성화되어 있으므로
.with(csrf())가 필요합니다.해결 방법: 각 테스트에서 DTO를 "최소 유효값"으로 채우세요.
- UpdateUserWithExperienceRequest request = new UpdateUserWithExperienceRequest(); + UpdateUserWithExperienceRequest request = new UpdateUserWithExperienceRequest(); + UpdateUserInfoRequest infoRequest = new UpdateUserInfoRequest(); + infoRequest.setYear(6); + infoRequest.setIsLft(false); + infoRequest.setSchool("인천대학교"); + infoRequest.setGrade("1학년"); + infoRequest.setMainPosition("BACKEND"); + infoRequest.setGithubUrl("https://github.com/test"); + request.setUpdateUserInfoRequest(infoRequest);동일하게
changeTecheer_Success도 리플렉션 대신 setter를 사용하도록 정리하면 코드가 더 명확해집니다.Likely an incorrect or invalid review comment.
techeerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.java (3)
68-75:SecurityAutoConfiguration제외 + “항상 true” evaluator 스텁으로@PreAuthorize가 실제로 검증되지 않을 가능성이 큽니다.
현재 구성은 인가 실패(401/403) 케이스를 거의 만들 수 없어, PR 목표(PreAuthorize 작성)에 비해 테스트가 false positive로 흘러갈 리스크가 큽니다. (과거 리뷰와 동일 이슈)권장 방향(프로젝트 보안 구성에 맞게 택1):
excludeAutoConfiguration제거(또는@AutoConfigureMockMvc(addFilters = true)+ 실제SecurityFilterChain/@EnableMethodSecurity가 로드되도록@Import)- 최소 2종 네거티브 테스트 추가: (1) 인증 없음 → 401/403, (2) 권한 부족(RoleType.TECHEER로 admin endpoint) → 403
레포 내 실제 상태 확인용(메서드 시큐리티 활성/비활성, PreAuthorize 사용 형태):
#!/bin/bash rg -n --type=java "@EnableMethodSecurity|@EnableGlobalMethodSecurity" -S rg -n --type=java "class UserController\\b" -S rg -n --type=java "@PreAuthorize\\(" techeerzip/src/main/java -S
265-315: Admin 플로우에 “권한 부족(403)” 테스트가 빠져@PreAuthorize회귀를 잡기 어렵습니다.
현재는 ADMIN으로만 200 확인 + evaluator always-true라 인가가 깨져도 통과할 수 있습니다. (과거 리뷰와 동일 이슈)추가 추천:
RoleType.TECHEER로/admin/permission/request,/admin/permission/approve호출 →isForbidden()verify(userService, never())...로 서비스 미호출까지 보장
109-114: 권한 evaluator를 전부true로 고정하면 "권한 로직이 깨져도" 테스트가 계속 통과합니다.
DelegatingPermissionEvaluator설계 상target == null이면 매칭 실패가 의도된 동작인데, 여기선 무조건 통과시켜 해당 회귀를 못 잡습니다. 현재 테스트에는 권한 검증 실패 시나리오(403 응답,never()검증 등)가 없으므로:
- evaluator mock을 케이스별로
true/false로 설정하고, 관리자/일반 사용자 교차 호출 시 403 응답 검증- 현재 admin 엔드포인트는 올바르게
hasRole('ADMIN')을 사용 중이므로, 비 admin 사용자가 admin 엔드포인트에 접근하는 실패 케이스 추가
🧹 Nitpick comments (3)
techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.java (3)
116-132:@AfterEach SecurityContextHolder.clearContext()추가는 좋습니다. 다만withSecurity()는 MockMvc post-processor 사용을 권장합니다.
전역 컨텍스트를 직접 만지는 방식은 디버깅/확장 시 불리하고, 요청 단위 인증 주입이 더 표준적입니다.예시(요지):
+import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; @@ private MockHttpServletRequestBuilder withSecurity( MockHttpServletRequestBuilder builder, Long userId, RoleType roleType) { @@ - SecurityContext securityContext = SecurityContextHolder.createEmptyContext(); - securityContext.setAuthentication(authentication); - SecurityContextHolder.setContext(securityContext); - - return builder; + return builder.with(authentication(authentication)); }
138-185: 성공 케이스만status().isOk()로 확인해 회귀 탐지력이 약합니다.
예:userService.signUp(...)/signUpExternal(...)가 “정확히 호출되는지”, 요청 바디/파트명이 맞는지까지 검증하는 편이 안전합니다.(예시)
verify(userService).signUp(...), multipart의 JSON part name(createUserWithResumeRequest) 등을 명시적으로 고정.
408-468: DTO 세팅을 리플렉션으로 하는 테스트는 매우 취약합니다.
필드명 변경/캡슐화 변경에 즉시 깨집니다. 가능하면 테스트 전용 빌더/팩토리(또는 생성자/세터/fixture)를 두는 쪽이 유지보수에 유리합니다.(대안)
ObjectMapper로 JSON을 직접 만들어 multipart JSON part로 넣거나, 테스트 fixture 유틸을 도입.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-09T10:19:12.637Z
Learnt from: kimzini
Repo: Techeer-Hogwarts/backend PR: 117
File: techeerzip/src/main/java/backend/techeerzip/global/permission/DelegatingPermissionEvaluator.java:37-43
Timestamp: 2025-09-09T10:19:12.637Z
Learning: DelegatingPermissionEvaluator에서 target이 null일 때 어떤 evaluator도 매칭되지 않는 것이 의도된 설계입니다. 어드민 권한 검증은 hasPermission이 아닌 hasRole('ADMIN')을 사용해야 합니다.
Applied to files:
techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.java
📚 Learning: 2025-09-18T12:23:00.520Z
Learnt from: dlwhsk0
Repo: Techeer-Hogwarts/backend PR: 111
File: techeerzip/src/main/java/backend/techeerzip/domain/user/service/UserService.java:892-902
Timestamp: 2025-09-18T12:23:00.520Z
Learning: UserService의 updateGithubUrlAndSync 메서드에서 JPA 더티 체킹 대신 userRepository.updateGithubUrlById() 커스텀 업데이트 메서드를 사용했습니다. 더티 체킹이 예상대로 작동하지 않는 경우에 대한 실용적인 해결책입니다.
Applied to files:
techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.java
📚 Learning: 2025-09-12T07:36:39.412Z
Learnt from: dlwhsk0
Repo: Techeer-Hogwarts/backend PR: 111
File: techeerzip/src/main/java/backend/techeerzip/infra/github/GitHubApiService.java:99-103
Timestamp: 2025-09-12T07:36:39.412Z
Learning: GitHub URL 검증은 서비스 레이어가 아닌 request DTO 단에서 Bean Validation 어노테이션을 사용하여 수행해야 한다.
Applied to files:
techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
techeerzip/src/test/java/backend/techeerzip/domain/user/controller/UserControllerTest.java (2)
78-84:UserArgumentResolver를 테스트 슬라이스에 주입한 구성은 적절합니다.
컨트롤러 시그니처가 커스텀 리졸버에 의존한다면, 이 방식이 가장 단순하게 실패를 방지합니다.
235-242: 이 엔드포인트는 의도적으로 공개 API로 설계되었으므로 현재 테스트가 올바릅니다.
GET /{userId}는 인증 없이 누구나 특정 사용자의 프로필을 조회할 수 있는 공개 엔드포인트입니다. 코드에서 @PreAuthorize 어노테이션이 없으며, 같은 패턴의 다른 GET 엔드포인트들(getAllProfiles, getBootcampMemberProfiles)도 모두 인증 없이 200을 반환하도록 테스트되어 있습니다. 따라서 현재 테스트는 보안 회귀가 아니며 설계 의도와 일치합니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
techeerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.java (2)
190-190: 이벤트 발행 검증 고려
eventPublisher에lenient()사용은 현재 acceptable하지만, 중요한 비즈니스 이벤트의 경우 명시적으로 발행 여부를 검증하는 것을 고려해보세요.- lenient().doNothing().when(eventPublisher).publishEvent(any()); + doNothing().when(eventPublisher).publishEvent(any()); + // 테스트 로직... + verify(eventPublisher).publishEvent(any()); // 필요시 검증Also applies to: 307-307, 886-886, 1133-1133, 1183-1183
873-873: Mock 생성 방식 일관성
HttpServletResponse모킹 시 전체 클래스명을 사용하고 있습니다. 다른 mock 객체들과 일관성을 위해 정적 임포트를 고려해보세요:+import static org.mockito.Mockito.mock; ... - HttpServletResponse response = org.mockito.Mockito.mock(HttpServletResponse.class); + HttpServletResponse response = mock(HttpServletResponse.class);Also applies to: 899-899
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
techeerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-09-18T12:43:26.211Z
Learnt from: dlwhsk0
Repo: Techeer-Hogwarts/backend PR: 111
File: techeerzip/src/main/java/backend/techeerzip/domain/user/service/UserService.java:0-0
Timestamp: 2025-09-18T12:43:26.211Z
Learning: UserService의 updateGithubUrlAndSync 메서드가 단일 트랜잭션으로 수정되어 검증→저장→동기화 과정에서 동기화 실패 시 URL 저장도 함께 롤백되도록 개선되었습니다.
Applied to files:
techeerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.java
📚 Learning: 2025-09-18T12:23:00.520Z
Learnt from: dlwhsk0
Repo: Techeer-Hogwarts/backend PR: 111
File: techeerzip/src/main/java/backend/techeerzip/domain/user/service/UserService.java:892-902
Timestamp: 2025-09-18T12:23:00.520Z
Learning: UserService의 updateGithubUrlAndSync 메서드에서 JPA 더티 체킹 대신 userRepository.updateGithubUrlById() 커스텀 업데이트 메서드를 사용했습니다. 더티 체킹이 예상대로 작동하지 않는 경우에 대한 실용적인 해결책입니다.
Applied to files:
techeerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.java
📚 Learning: 2025-09-12T07:44:45.395Z
Learnt from: dlwhsk0
Repo: Techeer-Hogwarts/backend PR: 111
File: techeerzip/src/main/java/backend/techeerzip/domain/user/service/UserService.java:0-0
Timestamp: 2025-09-12T07:44:45.395Z
Learning: 사용자가 GitHub URL 업데이트 및 동기화 기능에서 전체 과정(검증→저장→동기화)이 단일 트랜잭션으로 묶여야 한다고 명시했습니다. 동기화 실패 시 URL 저장도 롤백되어야 하는 요구사항입니다.
Applied to files:
techeerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.java
📚 Learning: 2025-09-12T07:36:39.412Z
Learnt from: dlwhsk0
Repo: Techeer-Hogwarts/backend PR: 111
File: techeerzip/src/main/java/backend/techeerzip/infra/github/GitHubApiService.java:99-103
Timestamp: 2025-09-12T07:36:39.412Z
Learning: GitHub URL 검증은 서비스 레이어가 아닌 request DTO 단에서 Bean Validation 어노테이션을 사용하여 수행해야 한다.
Applied to files:
techeerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
techeerzip/src/test/java/backend/techeerzip/domain/user/service/UserServiceTest.java (2)
98-159: 체계적인 테스트 구조와 설정테스트 클래스가 잘 구조화되어 있습니다:
@Nested클래스를 활용한 논리적 그룹핑- 필요한 모든 의존성에 대한 명확한 mock 선언
@BeforeEach에서 공통 테스트 픽스처 초기화
160-1537: 포괄적이고 체계적인 테스트 커버리지
UserService의 모든 주요 기능에 대한 철저한 테스트가 작성되어 있습니다:
- ✅ 회원가입 플로우 (일반, 외부, 삭제된 사용자 재가입)
- ✅ 사용자 조회 및 관리
- ✅ 권한 및 역할 관리
- ✅ 프로필 및 경력 관리
- ✅ 부트캠프 관련 기능
- ✅ GitHub 통합 (기술 스택 업데이트)
- ✅ 예외 상황 처리
각 테스트 케이스가 명확한 의도를 가지고 있으며,
@DisplayName을 통해 한글로 테스트 목적을 잘 설명하고 있습니다. Mock 검증과 상태 검증이 적절히 혼합되어 있어 행위와 결과를 모두 확인합니다.Based on learnings, UserService의 다른 메서드들(예:
updateGithubUrlAndSync)에 대한 테스트가 별도 파일이나 다른 위치에 있을 수 있습니다.
|
요약
Role, User, UserExperience 테스트 코드, PreAuthorize 작성
작업 내용
Role, User, UserExperience 테스트 코드, PreAuthorize 작성
관련 이슈
BACKEND-189
Summary by CodeRabbit
보안 강화
품질·테스트
데이터·동작
✏️ Tip: You can customize this high-level summary in your review settings.