feat: 어드민 유저 차단 기능 추가 #604
Conversation
JAEHEE25
requested review from
Gyuhyeok99,
Hexeong,
lsy1307,
sukangpunch,
whqtker and
wibaek
as code owners
Walkthrough
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/example/solidconnection/siteuser/domain/SiteUser.java (1)
77-104: 기존 생성자들이 필수 필드userStatus를 초기화하지 않아 테스트에서 버그가 발생합니다.
userStatus필드가@Column(nullable = false)로 선언되었지만, 다음 두 생성자는 이 필드를 초기화하지 않습니다:
- Lines 77-89: 5개 파라미터 생성자 (AuthType 기본값 설정)
- Lines 91-104: 6개 파라미터 생성자 (AuthType 명시)
현재 문제점:
- 이 생성자들은 테스트 코드에서 실제로 사용되고 있습니다
SiteUserRepositoryTest.java(line 82): 6-param 생성자 호출TokenAuthenticationTest.java(line 73): 5-param 생성자 호출- 데이터베이스 NOT NULL 제약 조건 위반으로 저장 실패 위험
getUserStatus()호출 시 NullPointerException 발생 가능권장 해결 방안:
- 각 테스트에서 올바른 8-파라미터 생성자 사용 (UserStatus 포함)
- 또는 기존 생성자들에 기본값
UserStatus.ACTIVE초기화 추가🔧 테스트 코드 수정 예시
SiteUserRepositoryTest.java (line 82-87)
- return new SiteUser( - email, - nickname, - "profileImageUrl", - ExchangeStatus.CONSIDERING, - Role.MENTEE, - authType - ); + return new SiteUser( + email, + nickname, + "profileImageUrl", + ExchangeStatus.CONSIDERING, + Role.MENTEE, + authType, + null, + UserStatus.ACTIVE + );TokenAuthenticationTest.java (line 73-78)
- return new SiteUser( - "[email protected]", - "nickname", - "profileImageUrl", - ExchangeStatus.CONSIDERING, - Role.MENTEE - ); + return new SiteUser( + "[email protected]", + "nickname", + "profileImageUrl", + ExchangeStatus.CONSIDERING, + Role.MENTEE, + AuthType.KAKAO, + null, + UserStatus.ACTIVE + );
🧹 Nitpick comments (7)
src/main/resources/db/migration/V41__add_is_deleted_to_post_and_chat_message.sql (1)
1-3: soft delete 패턴이 안전하게 구현되었습니다.
- 안전한 마이그레이션:
DEFAULT FALSE로 기존 데이터에 안전하게 적용되며,NOT NULL로 데이터 무결성이 확보됩니다.- 표준 패턴 준수: soft delete의 일반적인 구현 방식을 따릅니다.
다만, 성능 최적화를 위해 인덱스 추가를 권장합니다.
@Where필터로 인해 대부분의 조회 쿼리에서is_deleted = false조건이 사용되므로, 다음을 고려해 주세요:🔎 인덱스 추가 제안
별도 마이그레이션 파일을 생성하여 다음 인덱스를 추가하는 것을 권장합니다:
-- post 테이블: is_deleted와 자주 함께 사용되는 컬럼의 복합 인덱스 CREATE INDEX idx_post_is_deleted_created_at ON post(is_deleted, created_at); -- chat_message 테이블: 채팅방별 조회 최적화 CREATE INDEX idx_chat_message_is_deleted_room ON chat_message(is_deleted, chat_room_id, created_at);또는 PostgreSQL을 사용하는 경우 부분 인덱스(partial index)로 더 효율적으로 구현할 수 있습니다:
-- is_deleted = false인 레코드만 인덱싱 (더 작은 인덱스 크기) CREATE INDEX idx_post_active ON post(created_at) WHERE is_deleted = false; CREATE INDEX idx_chat_message_active ON chat_message(chat_room_id, created_at) WHERE is_deleted = false;src/main/java/com/example/solidconnection/community/post/repository/PostRepository.java (1)
53-59: 네이티브 SQL 서브쿼리 최적화를 고려해 주세요.현재 상황:
이 메서드는 사용자 차단 시 신고된 게시글을 일괄 소프트 삭제하기 위한 관리자 작업입니다. 관리자 차단/해제 작업(AdminUserBanService)에서만 호출되는 저빈도 작업입니다.최적화 제안:
쿼리 성능 개선
- 현재
IN (SELECT ...)패턴에서report테이블의target_id,target_type컬럼에 인덱스가 없습니다.EXISTS절 사용으로 더 효율적인 쿼리 실행이 가능합니다.🔎 최적화된 쿼리
@Modifying(clearAutomatically = true, flushAutomatically = true) @Query(value = """ UPDATE post p SET p.is_deleted = :isDeleted WHERE p.site_user_id = :siteUserId - AND p.id IN (SELECT r.target_id FROM report r WHERE r.target_type = 'POST') + AND EXISTS ( + SELECT 1 FROM report r + WHERE r.target_id = p.id + AND r.target_type = 'POST' + ) """, nativeQuery = true) void updateReportedPostsIsDeleted(@Param("siteUserId") long siteUserId, @Param("isDeleted") boolean isDeleted);인덱스 추가 (선택사항)
- 복합 인덱스 추가로 쿼리 성능을 더욱 향상시킬 수 있습니다.
CREATE INDEX idx_report_target ON report(target_id, target_type);src/main/java/com/example/solidconnection/report/service/ReportService.java (1)
66-90: 대상 엔티티 중복 조회를 최적화할 수 있습니다.현재 구현에서는 다음과 같은 흐름으로 엔티티를 중복 조회합니다:
validateTargetExists(line 35)에서 존재 여부 확인을 위해 조회updateReportedUserStatus(line 37)에서 작성자/발신자 ID를 얻기 위해 다시 조회개선 방안을 고려해보세요:
- 검증과 ID 추출을 하나의 메서드로 통합하여 단일 조회로 처리
- 또는 캐시 메커니즘 활용
다만 현재 구조는 관심사의 분리가 명확하고 가독성이 좋으므로, 성능 이슈가 실제로 발생하지 않는다면 현재 구조를 유지하는 것도 합리적입니다.
src/main/java/com/example/solidconnection/siteuser/domain/SiteUser.java (1)
130-132: 상태 전환 검증 로직 추가를 권장합니다.
updateUserStatus메서드가 단순 setter로 구현되어 있어 다음과 같은 우려사항이 있습니다:
상태 전환 규칙 부재:
BANNED→ACTIVE로의 전환이 무분별하게 허용됨REPORTED→BANNED외의 비정상적인 전환도 가능권한 검증 부재:
- 어떤 코드에서든 사용자 상태를 변경할 수 있음
- 특히 차단 해제는 관리자만 가능해야 함
감사(Audit) 추적 불가:
- 누가, 언제, 왜 상태를 변경했는지 추적 어려움
개선 제안:
- 상태 전환 규칙을 정의하고 검증하는 로직 추가
- 또는 특정 상태로만 전환하는 메서드들로 분리 (예:
markAsReported(),ban(),unban())- 이전 러닝에서 언급된 것처럼, 메서드 목적을 명확히 하는 네이밍이나 문서화 고려
Based on learnings, 메서드의 의도를 명확히 하기 위해 파라미터 명명이나 문서화가 필요합니다.
src/test/java/com/example/solidconnection/common/interceptor/BannedUserInterceptorTest.java (1)
101-135: 정상 사용자 테스트가 포괄적입니다.모든 주요 엔드포인트에 대한 접근 가능성을 검증하고 있어 좋습니다.
게시글 생성 부분(lines 105-112)을 별도 헬퍼 메서드로 추출하면 테스트 가독성이 더 향상될 수 있습니다만, 현재 구조도 충분히 명확합니다.
src/main/java/com/example/solidconnection/siteuser/domain/UserBan.java (1)
43-47: 멱등성을 고려한 방어 코드를 추가할 수 있습니다.현재
manuallyUnban메서드는 이미 차단 해제된 상태에서 다시 호출되어도 검증하지 않습니다. 서비스 레이어에서 처리하는 것으로 보이지만, 도메인 레벨에서도 방어 코드를 추가하면 더 안전합니다.🔎 방어 코드 추가 예시
public void manuallyUnban(Long adminId) { + if (this.isUnbanned) { + return; // or throw exception + } this.isUnbanned = true; this.unbannedBy = adminId; this.unbannedAt = ZonedDateTime.now(UTC); }src/test/java/com/example/solidconnection/admin/service/AdminUserBanServiceTest.java (1)
126-127: 중첩 클래스에@DisplayName어노테이션이 누락되었습니다.형제 클래스인
만료된_차단_자동_해제에는@DisplayName이 있지만, 이 클래스에는 없습니다. 테스트 리포트의 일관성을 위해 추가하는 것이 좋습니다.🔎 수정 제안
@Nested +@DisplayName("사용자 차단 해제") class 사용자_차단_해제 {
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
src/main/java/com/example/solidconnection/admin/controller/AdminUserBanController.javasrc/main/java/com/example/solidconnection/admin/dto/UserBanRequest.javasrc/main/java/com/example/solidconnection/admin/service/AdminUserBanService.javasrc/main/java/com/example/solidconnection/auth/dto/SignUpRequest.javasrc/main/java/com/example/solidconnection/auth/service/signup/SignUpService.javasrc/main/java/com/example/solidconnection/chat/domain/ChatMessage.javasrc/main/java/com/example/solidconnection/chat/repository/ChatMessageRepository.javasrc/main/java/com/example/solidconnection/common/config/web/WebMvcConfig.javasrc/main/java/com/example/solidconnection/common/exception/ErrorCode.javasrc/main/java/com/example/solidconnection/common/interceptor/BannedUserInterceptor.javasrc/main/java/com/example/solidconnection/community/post/domain/Post.javasrc/main/java/com/example/solidconnection/community/post/repository/PostRepository.javasrc/main/java/com/example/solidconnection/report/repository/ReportRepository.javasrc/main/java/com/example/solidconnection/report/service/ReportService.javasrc/main/java/com/example/solidconnection/siteuser/domain/SiteUser.javasrc/main/java/com/example/solidconnection/siteuser/domain/UserBan.javasrc/main/java/com/example/solidconnection/siteuser/domain/UserBanDuration.javasrc/main/java/com/example/solidconnection/siteuser/domain/UserStatus.javasrc/main/java/com/example/solidconnection/siteuser/repository/UserBanRepository.javasrc/main/resources/db/migration/V40__create_user_ban_table.sqlsrc/main/resources/db/migration/V41__add_is_deleted_to_post_and_chat_message.sqlsrc/test/java/com/example/solidconnection/admin/service/AdminUserBanServiceTest.javasrc/test/java/com/example/solidconnection/common/interceptor/BannedUserInterceptorTest.javasrc/test/java/com/example/solidconnection/siteuser/fixture/SiteUserFixture.javasrc/test/java/com/example/solidconnection/siteuser/fixture/SiteUserFixtureBuilder.javasrc/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixture.javasrc/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixtureBuilder.java
💤 Files with no reviewable changes (1)
- src/main/java/com/example/solidconnection/auth/dto/SignUpRequest.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-12T21:43:45.760Z
Learnt from: nayonsoso
Repo: solid-connection/solid-connect-server PR: 448
File: src/main/java/com/example/solidconnection/siteuser/domain/SiteUser.java:118-121
Timestamp: 2025-08-12T21:43:45.760Z
Learning: SiteUser 도메인의 updatePassword 메서드에서는 파라미터 이름을 newEncodedPassword로 하거나 Javadoc을 추가해서 인코딩된 비밀번호가 들어와야 한다는 것을 명시해야 합니다.
Applied to files:
src/main/java/com/example/solidconnection/siteuser/domain/SiteUser.java
🧬 Code graph analysis (10)
src/test/java/com/example/solidconnection/common/interceptor/BannedUserInterceptorTest.java (2)
src/main/java/com/example/solidconnection/security/authentication/TokenAuthentication.java (1)
TokenAuthentication(7-42)src/main/java/com/example/solidconnection/security/userdetails/SiteUserDetails.java (1)
SiteUserDetails(9-56)
src/main/java/com/example/solidconnection/common/config/web/WebMvcConfig.java (1)
src/main/java/com/example/solidconnection/admin/controller/AdminUserBanController.java (1)
RequiredArgsConstructor(19-43)
src/main/java/com/example/solidconnection/admin/controller/AdminUserBanController.java (1)
src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java (1)
Slf4j(26-103)
src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java (2)
src/main/java/com/example/solidconnection/admin/controller/AdminUserBanController.java (1)
RequiredArgsConstructor(19-43)src/main/java/com/example/solidconnection/report/service/ReportService.java (1)
Service(22-97)
src/main/java/com/example/solidconnection/common/interceptor/BannedUserInterceptor.java (1)
src/main/java/com/example/solidconnection/security/userdetails/SiteUserDetails.java (1)
SiteUserDetails(9-56)
src/test/java/com/example/solidconnection/admin/service/AdminUserBanServiceTest.java (1)
src/test/java/com/example/solidconnection/common/interceptor/BannedUserInterceptorTest.java (1)
TestContainerSpringBootTest(26-153)
src/main/java/com/example/solidconnection/siteuser/domain/UserBan.java (2)
src/main/java/com/example/solidconnection/siteuser/domain/SiteUser.java (1)
Getter(21-133)src/main/java/com/example/solidconnection/application/service/NicknameCreator.java (1)
NoArgsConstructor(8-32)
src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixture.java (3)
src/test/java/com/example/solidconnection/siteuser/fixture/SiteUserFixture.java (1)
TestComponent(10-99)src/test/java/com/example/solidconnection/siteuser/fixture/SiteUserFixtureBuilder.java (1)
TestComponent(13-80)src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixtureBuilder.java (1)
TestComponent(9-36)
src/main/java/com/example/solidconnection/community/post/domain/Post.java (2)
src/main/java/com/example/solidconnection/chat/domain/ChatMessage.java (1)
Entity(20-57)src/main/java/com/example/solidconnection/community/comment/domain/Comment.java (1)
Entity(22-109)
src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixtureBuilder.java (3)
src/test/java/com/example/solidconnection/siteuser/fixture/SiteUserFixture.java (1)
TestComponent(10-99)src/test/java/com/example/solidconnection/siteuser/fixture/SiteUserFixtureBuilder.java (1)
TestComponent(13-80)src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixture.java (1)
TestComponent(8-36)
⏰ 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 (27)
src/main/java/com/example/solidconnection/siteuser/domain/UserStatus.java (1)
3-7: 사용자 상태 enum 구현이 깔끔합니다!
- 상태 정의가 명확함: ACTIVE(활성), REPORTED(신고됨), BANNED(차단) 세 가지 상태가 명확하게 구분되어 있습니다.
- 향후 확장성 확보: REPORTED 상태를 미리 포함하여, 향후 신고된 사용자에 대한 별도 처리 로직을 추가할 수 있는 여지를 남겨두었습니다.
src/main/java/com/example/solidconnection/siteuser/domain/UserBanDuration.java (1)
8-14: 차단 기간 enum이 타입 안전하게 잘 설계되었습니다!
- 명확한 기간 정의: 1일, 3일, 7일 세 가지 기간이 명확하게 정의되어 있습니다.
- 타입 안전성: enum을 사용하여 잘못된 기간 값이 입력되는 것을 컴파일 타임에 방지할 수 있습니다.
- 계산 용이성:
days필드를 통해LocalDateTime.plusDays(duration.getDays())와 같이 만료 시간을 쉽게 계산할 수 있습니다.- 확장성: 향후 기획 변경 시 새로운 기간을 추가하기 용이한 구조입니다.
src/main/java/com/example/solidconnection/common/config/web/WebMvcConfig.java (1)
29-33: 차단 사용자 인터셉터 적용 범위를 검토해 주세요.현재 설정된 경로들을 확인한 결과, 다음과 같은 상황입니다.
현재 인터셉터가 잘 적용된 경로
- 게시글(
/posts/**) ✓- 댓글(
/comments/**) ✓- 채팅(
/chats/**) ✓- 게시판(
/boards/**) ✓인터셉터가 미적용된 추가 상호작용 API
- 뉴스 좋아요(
/news/{news-id}/like) ✗- 대학 지원정보 좋아요(
/univ-apply-infos/{univ-apply-info-id}/like) ✗PR 설명에서 명시된 네 가지 영역만 차단하려는 의도라면 현재 구현이 맞습니다. 다만 사용자 상호작용 기능의 일관성을 위해, 뉴스와 대학 지원정보의 좋아요 기능도 차단 대상으로 포함해야 하는지 검토해 주세요.
src/main/java/com/example/solidconnection/auth/service/signup/SignUpService.java (1)
60-61: 신규 가입 사용자에게 ACTIVE 상태를 올바르게 설정했습니다.회원가입 시 새로운 사용자에게
UserStatus.ACTIVE상태를 부여하는 것은 적절합니다. 도메인 모델의 생성자 시그니처 변경에 맞춰 정확하게 구현되었습니다.src/test/java/com/example/solidconnection/siteuser/fixture/SiteUserFixtureBuilder.java (2)
62-65: 테스트 픽스처에 userStatus 설정 기능이 추가되었습니다.빌더 패턴을 일관되게 확장하여 테스트에서 다양한 사용자 상태를 생성할 수 있도록 했습니다. 기존 빌더 메서드들과 동일한 스타일을 따르고 있어 좋습니다.
75-76: 기본값을 ACTIVE로 설정하여 기존 테스트와의 호환성을 유지했습니다.
userStatus가 명시되지 않은 경우UserStatus.ACTIVE를 기본값으로 사용하는 것은 적절합니다. 이를 통해 기존 테스트 코드를 수정하지 않아도 정상 동작하도록 보장합니다.src/test/java/com/example/solidconnection/siteuser/fixture/SiteUserFixture.java (1)
24-24: 기존 픽스처에 ACTIVE 상태를 명시적으로 설정했습니다.모든 기존 픽스처 메서드에
.userStatus(UserStatus.ACTIVE)를 추가하여 사용자 상태를 명확히 했습니다. 일관성 있는 패턴으로 적용되어 좋습니다.Also applies to: 36-36, 48-48, 60-60, 72-72, 84-84
src/main/resources/db/migration/V40__create_user_ban_table.sql (1)
16-17: user_status 컬럼에 인덱스 추가는 불필요합니다.코드베이스를 검토한 결과, 다음과 같은 사실을 확인했습니다:
- BannedUserInterceptor는 이미 로드된 사용자 객체에서 status를 확인합니다. 데이터베이스 쿼리를 실행하지 않습니다.
- SiteUserRepository에는 user_status를 필터 조건으로 사용하는 쿼리 메서드가 없습니다.
- AdminUserBanService와 ReportService는 상태를 업데이트만 수행하며, 상태로 필터링하는 쿼리는 없습니다.
- 현재 모든 데이터베이스 쿼리는 ID, email+authType, 또는 nickname으로만 필터링합니다.
user_status는 메모리 내에서만 검사되므로 인덱스를 추가하면 write 성능만 저하되고 읽기 성능 향상이 없습니다.
Likely an incorrect or invalid review comment.
src/main/java/com/example/solidconnection/report/service/ReportService.java (2)
30-30: 새로운 의존성 추가가 적절합니다.채팅 메시지 발신자를 조회하기 위해
ChatParticipantRepository를 추가한 것은 타당합니다.
32-41: 트랜잭션 처리가 올바릅니다.신고 대상 사용자의 상태를
REPORTED로 변경하는 로직이 신고 엔티티 저장 전에 실행되지만,@Transactional어노테이션으로 인해 전체 작업이 원자적으로 처리되므로 일관성 문제가 없습니다.src/main/java/com/example/solidconnection/admin/dto/UserBanRequest.java (1)
8-11: DTO 구현이 깔끔하고 적절합니다.
UserBanRequestrecord의 구현이 다음과 같이 우수합니다:
- Java record를 활용한 불변 DTO
@NotNull검증과 명확한 한글 에러 메시지UserBanDurationenum으로 타입 안전성 보장src/main/java/com/example/solidconnection/common/interceptor/BannedUserInterceptor.java (1)
19-36: 원본 검토 의견은 부정확합니다. 실제 구현에서는 매 요청마다 DB에서 사용자 정보를 새로 조회합니다.리뷰에서 우려한 "차단된 사용자가 토큰 만료 전까지 계속 접근 가능"하다는 시나리오는 발생하지 않습니다.
매 요청마다 최신 정보 조회
- TokenAuthenticationFilter가 매 요청 시 TokenAuthenticationProvider.authenticate() 호출
- SiteUserDetailsService.loadUserByUsername()에서 DB 조회하여 최신 SiteUser 정보 획득
- 새로운 TokenAuthentication이 생성되어 SecurityContext에 저장
차단 기능의 즉시성 확보
- AdminUserBanService에서 userStatus를 BANNED로 변경
- 다음 요청부터 BannedUserInterceptor가 DB의 최신 상태를 확인하고 차단
- 토큰 만료를 기다릴 필요 없음
따라서 별도의 토큰 무효화 메커니즘이나 Redis 캐시 없이도 현재 설계로 차단 기능이 제대로 작동합니다.
src/test/java/com/example/solidconnection/common/interceptor/BannedUserInterceptorTest.java (3)
26-44: 테스트 구성이 적절합니다.인터셉터 테스트를 위해 필터를 비활성화하고, 각 테스트 전에 SecurityContext를 정리하는 구조가 깔끔합니다.
46-99: 차단된 사용자 테스트가 잘 구성되어 있습니다.
- 게시판, 게시글, 댓글, 채팅 접근 차단을 각각 검증
- 일관된 패턴으로 작성되어 유지보수가 용이
- 테스트 이름이 명확하여 의도 파악이 쉬움
148-152: 인증 설정 헬퍼 메서드가 깔끔합니다.Spring Security의 SecurityContext를 올바르게 설정하고 있으며, 테스트 코드의 중복을 효과적으로 제거했습니다.
src/main/java/com/example/solidconnection/chat/domain/ChatMessage.java (1)
18-18: Soft delete 구현이 Post 엔티티와 일관성 있게 적용되었습니다.
@Where어노테이션으로 삭제된 메시지 자동 필터링isDeleted필드에 적절한 기본값과 제약조건 설정- 차단된 사용자의 신고된 채팅 메시지 삭제/복구 처리를 위한 필수 구현
Also applies to: 23-23, 38-40
src/main/java/com/example/solidconnection/admin/controller/AdminUserBanController.java (2)
26-33: 유저 차단 엔드포인트가 적절하게 구현되었습니다.
@Valid를 통한 요청 검증 수행- 서비스 레이어로 깔끔한 위임
- RESTful 설계를 따르는 경로 구조
35-42: 차단 해제 엔드포인트 - 관리자 권한 확인 완료다음과 같이 관리자 권한이 충분히 검증되고 있으니, 안심하셔도 됩니다.
경로 수준의 보안 적용
- SecurityConfiguration에서
/admin/**경로에 대해hasRole(ADMIN.name())설정으로 관리자 권한 필수화- 해당 엔드포인트는
/admin/users/{userId}/unban경로이므로 자동으로 보호됨인증 사용자 정보 추출
@AuthorizedUser어노테이션이 SecurityContext에서 인증된 사용자의 ID를 올바르게 추출- 인증되지 않은 사용자 접근 시 AuthorizedUserResolver에서 AUTHENTICATION_FAILED 예외 발생
감시 로직으로 기록
- 서비스 메서드에서 adminId를
userBan.manuallyUnban(adminId)로 전달하여 누가 차단을 해제했는지 기록엔드포인트 보안 구현이 적절하므로 추가 수정은 불필요합니다.
src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixtureBuilder.java (1)
22-35: 빌더 메서드와 생성 로직이 적절합니다.
- 각 setter 메서드가
this를 반환하여 메서드 체이닝 지원create()메서드에서 엔티티 생성 및 저장을 명확하게 처리- 테스트 픽스처로서 단순하고 이해하기 쉬운 구조
src/test/java/com/example/solidconnection/admin/service/AdminUserBanServiceTest.java (1)
1-82: 테스트 구성이 잘 되어 있습니다.
- 테스트 픽스처 활용이 적절합니다.
@BeforeEach에서 공통 데이터를 초기화하는 방식이 깔끔합니다.- 한글 메서드명으로 테스트 의도가 명확하게 표현되어 있습니다.
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java (2)
153-157: 차단 관련 에러 코드가 적절하게 추가되었습니다.
ALREADY_BANNED_USER: 상태 충돌에 적합한CONFLICT(409)사용NOT_BANNED_USER: 잘못된 요청에 적합한BAD_REQUEST(400)사용BANNED_USER_ACCESS_DENIED: 접근 거부에 적합한FORBIDDEN(403)사용에러 메시지도 명확하고 일관성 있게 작성되었습니다.
56-56: REPORT_NOT_FOUND 에러 코드 위치가 적절합니다.
data not found섹션에 올바르게 배치되었습니다.src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixture.java (1)
14-26: 픽스처 메서드 구현이 깔끔합니다.
차단(): 일수를 파라미터로 받아 유연하게 테스트 데이터 생성만료된_차단(): 과거 날짜로 만료된 차단 상태를 간편하게 생성빌더 패턴을 활용한 구조가 잘 설계되어 있습니다.
src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java (3)
37-46: banUser 메서드 구현이 잘 되어 있습니다.
- 이미 차단된 사용자 검증
- 신고 내역 존재 검증
- 신고된 콘텐츠 soft delete
- 차단 기록 생성
- 사용자 상태 업데이트
트랜잭션 내에서 모든 작업이 원자적으로 처리됩니다.
77-82: unbanUser 메서드 구현이 적절합니다.수동 차단 해제 시
manuallyUnban()을 호출하여 해제 정보(관리자 ID, 해제 시간)를 기록하고,processUnban()으로 콘텐츠 복구 및 상태 변경을 처리합니다.
60-63: 콘텐츠 삭제/복구 로직이 깔끔합니다.
isDeleted파라미터로 삭제와 복구를 동일한 메서드에서 처리하는 방식이 효율적입니다.src/main/java/com/example/solidconnection/report/repository/ReportRepository.java (1)
13-25: 복잡한 쿼리가 정확하게 구현되었습니다.다음 사항들을 확인했으니 안심하셔도 됩니다:
Enum 매핑 검증 완료 –
TargetType열거형이@Enumerated(EnumType.STRING)으로 매핑되어 있으며, JPQL 쿼리의 문자열 리터럴'POST'와'CHAT'이 enum 상수와 정확하게 일치합니다.POST 타입 처리 –
Post엔티티와 조인하여 작성자를 올바르게 확인합니다.CHAT 타입 처리 –
ChatMessage와ChatParticipant를 조인하여 발신자를 올바르게 확인합니다.두 신고 대상 타입을 모두 안전하게 처리하는 논리가 완벽합니다.
src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java
Show resolved
Hide resolved
| @Modifying(clearAutomatically = true, flushAutomatically = true) | ||
| @Query(value = """ | ||
| UPDATE chat_message cm SET cm.is_deleted = :isDeleted | ||
| WHERE cm.id IN (SELECT r.target_id FROM report r WHERE r.target_type = 'CHAT') | ||
| AND cm.sender_id IN (SELECT cp.id FROM chat_participant cp WHERE cp.site_user_id = :siteUserId) | ||
| """, nativeQuery = true) | ||
| void updateReportedChatMessagesIsDeleted(@Param("siteUserId") long siteUserId, @Param("isDeleted") boolean isDeleted); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 마이그레이션 파일 디렉토리 위치 확인
find . -type d -name "migration" -o -name "db" | head -20Repository: solid-connection/solid-connect-server
Length of output: 138
🏁 Script executed:
# 마이그레이션 파일 목록 확인
ls -la ./src/main/resources/db/migration/
echo -e "\n=== 전체 파일 목록 ==="
find ./src/main/resources/db/migration -type f -name "*.sql" | sortRepository: solid-connection/solid-connect-server
Length of output: 6853
🏁 Script executed:
# chat 및 report 관련 테이블 생성 파일 확인
echo "=== V24__add_chat_related_tables.sql ==="
cat ./src/main/resources/db/migration/V24__add_chat_related_tables.sql
echo -e "\n=== V25__create_report_table.sql ==="
cat ./src/main/resources/db/migration/V25__create_report_table.sql
echo -e "\n=== V41__add_is_deleted_to_post_and_chat_message.sql ==="
cat ./src/main/resources/db/migration/V41__add_is_deleted_to_post_and_chat_message.sqlRepository: solid-connection/solid-connect-server
Length of output: 3030
🏁 Script executed:
# report 테이블의 target_type ENUM 변경 이력 확인
echo "=== Checking for report table modifications ==="
rg -n "ALTER TABLE report" ./src/main/resources/db/migration/ -A 5
echo -e "\n=== Checking for CHAT in target_type ==="
rg -n "CHAT" ./src/main/resources/db/migration/ -B 2 -A 2
echo -e "\n=== Checking for index creations ==="
rg -n "CREATE.*INDEX|CREATE INDEX" ./src/main/resources/db/migration/ -A 3Repository: solid-connection/solid-connect-server
Length of output: 14024
🏁 Script executed:
# ENUM 수정 쿼리 찾기
echo "=== Searching for ALTER ENUM or MODIFY for target_type ==="
rg -n "target_type|MODIFY.*enum|ALTER.*enum" ./src/main/resources/db/migration/ -i -B 1 -A 3
# V25 이후의 모든 파일에서 report 관련 수정 확인
echo -e "\n=== All ALTER for report table ==="
rg -n "report" ./src/main/resources/db/migration/ -B 2 -A 2 | head -100
# 'CHAT' 또는 'Chat' 문자열 전체 검색
echo -e "\n=== Searching for Chat keyword in migrations ==="
rg -n "'CHAT'|\"CHAT\"|Chat" ./src/main/resources/db/migration/Repository: solid-connection/solid-connect-server
Length of output: 8049
🏁 Script executed:
# Java 코드에서 실제 target_type 값 확인
echo "=== Checking Java code for target_type usage ==="
rg -n "target_type.*CHAT|'CHAT'|\"CHAT\"" src/main/java/ -B 2 -A 2 | head -50
# ReportTargetType enum 확인
echo -e "\n=== Searching for ReportTargetType enum ==="
fd -t f "ReportTargetType.java" src/
# Report entity 확인
echo -e "\n=== Searching for Report entity ==="
fd -t f "Report.java" src/ | grep -v "Repository"Repository: solid-connection/solid-connect-server
Length of output: 1730
🏁 Script executed:
# Report entity 상세 확인
echo "=== Report.java content ==="
cat src/main/java/com/example/solidconnection/report/domain/Report.java
# ChatMessageRepository 메서드 전체 확인
echo -e "\n=== ChatMessageRepository class ==="
cat src/main/java/com/example/solidconnection/chat/repository/ChatMessageRepository.java | head -80Repository: solid-connection/solid-connect-server
Length of output: 4326
🏁 Script executed:
# TargetType enum 검색
echo "=== Searching for TargetType enum ==="
fd -t f "TargetType.java" src/
# 또는 Report.java와 같은 디렉토리에서 찾기
echo -e "\n=== Files in report domain directory ==="
ls -la src/main/java/com/example/solidconnection/report/domain/
# enum 정의 검색
echo -e "\n=== Searching for enum TargetType definition ==="
rg -n "enum TargetType|class TargetType" src/main/java/ -A 5Repository: solid-connection/solid-connect-server
Length of output: 1082
🏁 Script executed:
# TargetType.java 전체 확인
echo "=== TargetType.java ==="
cat src/main/java/com/example/solidconnection/report/domain/TargetType.java
# 모든 마이그레이션 파일이 있는지 확인
echo -e "\n=== Total migration files ==="
ls -1 ./src/main/resources/db/migration/ | wc -l
# 가장 최신 마이그레이션 파일들 확인
echo -e "\n=== Latest 5 migration files ==="
ls -1 ./src/main/resources/db/migration/ | sort -V | tail -5
# application.properties에서 JPA 설정 확인
echo -e "\n=== Checking application.properties for JPA DDL settings ==="
find ./src/main/resources -name "application*.properties" -o -name "application*.yml" | xargs cat 2>/dev/null | grep -i "jpa\|hibernate\|ddl" -A 2Repository: solid-connection/solid-connect-server
Length of output: 560
데이터베이스 스키마 수정이 필수적으로 필요합니다.
신고된 채팅 메시지 일괄 처리 메서드의 구현 로직은 정확하나, 데이터베이스와 코드 간 중요한 불일치가 있습니다.
-
CHAT 타입이 report 테이블 스키마에 없음 (필수 수정)
- 마이그레이션 V25에서 report 테이블의 target_type ENUM이
('POST')만 정의되어 있습니다. - Java 코드의 TargetType enum은 POST, CHAT을 모두 포함하고 있습니다.
- 이 쿼리는 현재 데이터베이스 ENUM 제약 위반으로 실행될 수 없습니다.
- 마이그레이션 파일을 추가하여
target_typeENUM을('POST', 'CHAT')로 확장해야 합니다.
- 마이그레이션 V25에서 report 테이블의 target_type ENUM이
-
성능 최적화를 위한 인덱스 추가 권장
- report 테이블의
target_id,target_type복합 인덱스 - chat_participant 테이블의
site_user_id인덱스 - chat_message 테이블의
sender_id인덱스
중첩된 서브쿼리로 인한 성능 저하를 방지하기 위해 위의 인덱스를 추가하시기 바랍니다.
- report 테이블의
-
현재 구현의 긍정적 측면
isDeleted파라미터로 삭제와 복구를 모두 지원합니다.clearAutomatically와flushAutomatically옵션으로 벌크 연산 후 영속성 컨텍스트를 적절히 관리했습니다.
🤖 Prompt for AI Agents
In
src/main/java/com/example/solidconnection/chat/repository/ChatMessageRepository.java
around lines 53 to 59, the native UPDATE uses target_type = 'CHAT' but the DB
migration V25 defines the report.target_type ENUM only with 'POST', causing enum
constraint failures; create a new migration that alters the report.target_type
enum to include 'CHAT' (e.g., add a migration V26 that updates the enum values
to ('POST','CHAT') and migrates existing data safely), add the recommended
indexes to improve performance (composite index on report(target_type,
target_id) or (target_id, target_type) depending on query planner, index on
chat_participant(site_user_id), and index on chat_message(sender_id)), apply and
run the migration before deploying this code change, and verify the
updateReportedChatMessagesIsDeleted query executes successfully in integration
tests.
| import lombok.Getter; | ||
| import lombok.NoArgsConstructor; | ||
| import org.hibernate.annotations.BatchSize; | ||
| import org.hibernate.annotations.Where; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Comment 엔티티 파일 찾기
find . -type f -name "Comment.java" | grep -E "community|comment"Repository: solid-connection/solid-connect-server
Length of output: 162
🏁 Script executed:
# Post 엔티티에서 isDeleted 관련 메서드 확인
rg -n "isDeleted" --type java -A 3 -B 3 | grep -E "(Post\.java|public|private|protected|void|boolean)" | head -50Repository: solid-connection/solid-connect-server
Length of output: 3229
🏁 Script executed:
# Comment 엔티티의 @Where 어노테이션 확인
rg -n "@Where|@Entity|class Comment" --type java -B 2 -A 10 | grep -E "Comment|@Where"Repository: solid-connection/solid-connect-server
Length of output: 10260
🏁 Script executed:
# soft delete 관련 메서드명 패턴 검색
rg -n "delete|Delete|soft" --type java src/main/java/com/example/solidconnection/community/ | grep -i "method\|void\|class" | head -30Repository: solid-connection/solid-connect-server
Length of output: 413
🏁 Script executed:
cat -n src/main/java/com/example/solidconnection/community/comment/domain/Comment.java | head -50Repository: solid-connection/solid-connect-server
Length of output: 1797
🏁 Script executed:
# Comment.java의 @Where 어노테이션 확인
rg -n "@Where|@Entity" src/main/java/com/example/solidconnection/community/comment/domain/Comment.javaRepository: solid-connection/solid-connect-server
Length of output: 91
🏁 Script executed:
# Comment의 deprecateComment() 메서드 확인
rg -n "deprecateComment" src/main/java/com/example/solidconnection/community/comment/domain/Comment.java -A 5Repository: solid-connection/solid-connect-server
Length of output: 172
🏁 Script executed:
# CommentRepository에서 soft delete 관련 쿼리 확인
rg -n "isDeleted|deleteComment|deprecate" src/main/java/com/example/solidconnection/community/comment/repository/CommentRepository.java -B 2 -A 5Repository: solid-connection/solid-connect-server
Length of output: 63
🏁 Script executed:
# CommentRepository 파일 찾기 및 내용 확인
find . -type f -name "CommentRepository.java"Repository: solid-connection/solid-connect-server
Length of output: 176
🏁 Script executed:
# CommentRepository 전체 내용 확인
cat -n src/main/java/com/example/solidconnection/community/comment/repository/CommentRepository.java | head -80Repository: solid-connection/solid-connect-server
Length of output: 2647
🏁 Script executed:
# Comment 조회 쿼리들 확인 - isDeleted 필터링 여부
rg -n "SELECT|select|FROM|from" src/main/java/com/example/solidconnection/community/comment/repository/CommentRepository.java -B 1 -A 8Repository: solid-connection/solid-connect-server
Length of output: 1831
🏁 Script executed:
# Post 엔티티에서 isDeleted 관련 public 메서드 확인
rg -n "isDeleted|deprecated|deprecate" src/main/java/com/example/solidconnection/community/post/domain/Post.java -B 2 -A 3Repository: solid-connection/solid-connect-server
Length of output: 343
🏁 Script executed:
# Post.java 전체 메서드 확인
cat -n src/main/java/com/example/solidconnection/community/post/domain/Post.java | grep -E "public|private|void|boolean" | head -30Repository: solid-connection/solid-connect-server
Length of output: 1079
🏁 Script executed:
# Post.java 전체 내용 확인 - 특히 메서드들
cat -n src/main/java/com/example/solidconnection/community/post/domain/Post.javaRepository: solid-connection/solid-connect-server
Length of output: 3461
🏁 Script executed:
# Comment와 Post의 soft delete 메서드 비교
echo "=== Post의 soft delete 관련 메서드 ===" && \
rg -n "public.*void|public.*delete|public.*deprecate" src/main/java/com/example/solidconnection/community/post/domain/Post.java && \
echo -e "\n=== Comment의 soft delete 관련 메서드 ===" && \
rg -n "public.*void|public.*delete|public.*deprecate" src/main/java/com/example/solidconnection/community/comment/domain/Comment.javaRepository: solid-connection/solid-connect-server
Length of output: 614
🏁 Script executed:
# soft delete 호출되는 곳 확인
rg -n "deprecateComment|\.isDeleted\s*=\s*true" --type java src/main/java/com/example/solidconnection/community/comment/Repository: solid-connection/solid-connect-server
Length of output: 452
🏁 Script executed:
# CommentService의 findCommentsByPostId 메서드 전체 확인
rg -n "findCommentsByPostId" src/main/java/com/example/solidconnection/community/comment/service/CommentService.java -B 2 -A 15Repository: solid-connection/solid-connect-server
Length of output: 1041
🏁 Script executed:
# soft delete 처리 관련 전체 흐름 확인 - 어디서 soft delete가 호출되나
rg -n "deprecateComment|updateReportedPostsIsDeleted|updateReportedChatMessagesIsDeleted|updateReportedContentIsDeleted" --type java -B 3 -A 3Repository: solid-connection/solid-connect-server
Length of output: 9558
🏁 Script executed:
# Verify filterCommentsByDeletionRules implementation
rg -n "filterCommentsByDeletionRules" src/main/java/com/example/solidconnection/community/comment/service/CommentService.java -A 20Repository: solid-connection/solid-connect-server
Length of output: 1916
Comment 엔티티의 @where 어노테이션 누락과 soft delete 일관성 문제를 해결하세요.
검증 결과, Post와 ChatMessage 엔티티와의 일관성 문제가 확실합니다. 추가로 soft delete 처리에서 더 심각한 문제점도 발견되었습니다:
-
@where 어노테이션 누락으로 인한 불일치
- Post (line 27):
@Where(clause = "is_deleted = false")사용 - ChatMessage (line 23):
@Where(clause = "is_deleted = false")사용 - Comment: 어노테이션 없음 → 모든 조회가 DB 레벨에서 필터링되지 않음
- Post (line 27):
-
필터링 방식이 다름
- Post/ChatMessage: DB 레벨 자동 필터링 (신뢰성 높음)
- Comment: 애플리케이션 레벨 필터링 (filterCommentsByDeletionRules) → 모든 쿼리에서 수동으로 처리 필요
-
🚨 AdminUserBanService에서 보고된 댓글 미처리 (중대 결함)
- updateReportedContentIsDeleted() 메서드 (line 60-63)가 Post와 ChatMessage는 soft delete하지만 Comment는 처리하지 않음
- 결과: 사용자 차단 후에도 보고된 댓글이 계속 조회됨
해결 방법:
- Comment에 @where 어노테이션 추가
- Comment soft delete를 위한 bulk update 메서드를 CommentRepository에 추가
- AdminUserBanService의 updateReportedContentIsDeleted()에서 댓글도 처리하도록 수정
🤖 Prompt for AI Agents
In src/main/java/com/example/solidconnection/community/post/domain/Post.java
around line 21, the review highlights a consistency bug: the Comment entity
lacks @Where(clause = "is_deleted = false") causing DB-level filtering mismatch
and AdminUserBanService.updateReportedContentIsDeleted(...) fails to soft-delete
reported comments. Fix by adding @Where(clause = "is_deleted = false") to the
Comment entity, add a bulk soft-delete method in CommentRepository (e.g., update
isDeleted = true for matched comment IDs or reported flags), and update
AdminUserBanService.updateReportedContentIsDeleted() to call the new repository
method so comments are soft-deleted consistently with Post and ChatMessage.
| private void updateUserStatusToReported(long userId) { | ||
| SiteUser user = siteUserRepository.findById(userId) | ||
| .orElseThrow(() -> new CustomException(ErrorCode.USER_NOT_FOUND)); | ||
| user.updateUserStatus(UserStatus.REPORTED); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find UserStatus enum and SiteUser class
echo "=== Finding UserStatus enum ==="
fd -type f -name "UserStatus.java" --exec cat {}
echo -e "\n=== Finding SiteUser class ==="
fd -type f -name "SiteUser.java" --exec head -100 {}Repository: solid-connection/solid-connect-server
Length of output: 503
🏁 Script executed:
#!/bin/bash
# Search for updateUserStatus method and its implementation
echo "=== Searching for updateUserStatus method ==="
rg -n "updateUserStatus" --type java -B 2 -A 5Repository: solid-connection/solid-connect-server
Length of output: 7742
🏁 Script executed:
#!/bin/bash
# Check for transaction annotations and synchronization in ReportService
echo "=== Examining ReportService class structure ==="
fd -type f -name "ReportService.java" --exec cat {}Repository: solid-connection/solid-connect-server
Length of output: 316
🏁 Script executed:
#!/bin/bash
# Find UserStatus enum using different approach
find . -name "*.java" | xargs grep -l "enum UserStatus"
echo -e "\n=== UserStatus Enum Content ==="
find . -name "*.java" | xargs grep -l "enum UserStatus" | xargs cat
echo -e "\n=== ReportService Full Content ==="
find . -name "ReportService.java" | xargs catRepository: solid-connection/solid-connect-server
Length of output: 4737
🏁 Script executed:
#!/bin/bash
# Check if there are any status transition validations anywhere
echo "=== Searching for status validation logic ==="
rg -i "userStatus" -B 2 -A 2 | head -60
echo -e "\n=== Checking for transaction management ==="
rg "@Transactional" src/main/java/com/example/solidconnection/report/service/ -B 5 -A 3Repository: solid-connection/solid-connect-server
Length of output: 7024
상태 전환 로직에 상태 계층 검증이 필요합니다.
현재 구현에서는 사용자 상태를 검증 없이 변경하고 있습니다. 특히 다음과 같은 문제가 있습니다:
-
BANNED 상태 보호 부재: 신고된 사용자가 이미
BANNED상태인 경우, 현재 코드는 이를REPORTED로 덮어씁니다. 일반적으로 차단(BANNED)이 신고(REPORTED)보다 더 강력한 제약이므로, 이러한 상태 전환은 비즈니스 로직에 맞지 않습니다. -
상태 계층 구조 미정의: 현재
updateUserStatus()메서드는 단순 setter로만 동작하며, 상태 전환 규칙이 없습니다. 상태 간 우선순위(BANNED>REPORTED>ACTIVE)를 강제해야 합니다.
권장 수정 방향
updateUserStatus() 메서드에 다음과 같은 검증 로직을 추가하세요:
public void updateUserStatus(UserStatus newStatus) {
// BANNED 상태는 상위 상태이므로, REPORTED나 ACTIVE로 변경되지 않도록 보호
if (this.userStatus == UserStatus.BANNED && newStatus != UserStatus.BANNED) {
return; // 또는 예외 처리
}
this.userStatus = newStatus;
}또는 ReportService.updateUserStatusToReported()에서 이미 BANNED 사용자는 상태를 변경하지 않도록 처리할 수 있습니다.
🤖 Prompt for AI Agents
In src/main/java/com/example/solidconnection/report/service/ReportService.java
around lines 92-96, the method unconditionally sets a user's status to REPORTED
which can overwrite a higher-priority BANNED state; update the logic to check
the current user status before changing it (either by calling a protected
updateUserStatus that enforces hierarchy or by checking here and
skipping/throwing if user is BANNED) so that BANNED users are not downgraded to
REPORTED and the status priority BANNED > REPORTED > ACTIVE is enforced.
src/test/java/com/example/solidconnection/admin/service/AdminUserBanServiceTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/example/solidconnection/admin/service/AdminUserBanServiceTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/example/solidconnection/siteuser/fixture/SiteUserFixture.java
Show resolved
Hide resolved
| public UserBan 수동_차단_해제(long bannedUserId, long adminId) { | ||
| UserBan userBan = userBanFixtureBuilder.userBan() | ||
| .bannedUserId(bannedUserId) | ||
| .expiredAt(ZonedDateTime.now().plusDays(7)) | ||
| .create(); | ||
| userBan.manuallyUnban(adminId); | ||
| return userBan; | ||
| } |
There was a problem hiding this comment.
manuallyUnban() 호출 후 변경사항이 저장되지 않습니다.
userBanFixtureBuilder.userBan().create()로 엔티티를 저장한 후 manuallyUnban(adminId)를 호출하지만, 이 변경사항이 데이터베이스에 반영되지 않습니다. UserBanFixtureBuilder에 저장 기능을 추가하거나, 이 메서드에서 직접 저장해야 합니다.
🔎 수정 제안
UserBanFixtureBuilder에 UserBanRepository를 주입받으므로, 아래와 같이 수정할 수 있습니다:
public UserBan 수동_차단_해제(long bannedUserId, long adminId) {
UserBan userBan = userBanFixtureBuilder.userBan()
.bannedUserId(bannedUserId)
.expiredAt(ZonedDateTime.now().plusDays(7))
.create();
userBan.manuallyUnban(adminId);
- return userBan;
+ return userBanFixtureBuilder.save(userBan);
}또는 UserBanRepository를 직접 주입받아 사용하는 방법도 있습니다.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixture.java
around lines 28 to 35, the code calls userBanFixtureBuilder.userBan().create()
then userBan.manuallyUnban(adminId) but never persists the state change; update
the flow to save the updated entity after manuallyUnban. Either (A) modify
UserBanFixtureBuilder to accept and use a UserBanRepository so its create()
returns a managed entity and expose a save method to persist updates, or (B)
inject UserBanRepository into this fixture class and call
userBanRepository.save(userBan) immediately after userBan.manuallyUnban(adminId)
so the unban is written to the database.
| public UserBanFixtureBuilder userBan() { | ||
| return new UserBanFixtureBuilder(userBanRepository); | ||
| } |
There was a problem hiding this comment.
빌더 패턴 구현에 문제가 있습니다.
userBan() 메서드가 새 인스턴스를 생성하지만, 이후 체이닝되는 bannedUserId(), expiredAt() 메서드 호출이 이 새 인스턴스의 필드를 설정합니다.
하지만 UserBanFixture에서 이 빌더를 사용할 때:
userBanFixtureBuilder.userBan()
.bannedUserId(bannedUserId)
.expiredAt(...)
.create();이 패턴은 정상 작동하지만, 만약 외부에서 이미 생성된 builder 인스턴스의 필드를 재사용하려 하면 문제가 발생할 수 있습니다. SiteUserFixtureBuilder도 동일한 패턴을 사용하므로 일관성은 있으나, 필드 초기화가 명시적이지 않습니다.
🔎 더 명확한 구현 제안
필드를 명시적으로 초기화:
public UserBanFixtureBuilder userBan() {
- return new UserBanFixtureBuilder(userBanRepository);
+ UserBanFixtureBuilder builder = new UserBanFixtureBuilder(userBanRepository);
+ builder.bannedUserId = null;
+ builder.expiredAt = null;
+ return builder;
}또는 현재 인스턴스를 재설정하는 방식:
public UserBanFixtureBuilder userBan() {
- return new UserBanFixtureBuilder(userBanRepository);
+ this.bannedUserId = null;
+ this.expiredAt = null;
+ return this;
}🤖 Prompt for AI Agents
In
src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixtureBuilder.java
around lines 18 to 20, the userBan() method currently returns a new
UserBanFixtureBuilder which hides or discards any existing instance state and
makes field initialization implicit; instead, change userBan() to reuse the
current builder instance by explicitly resetting its fields (e.g., set
bannedUserId, expiredAt and any other builder fields to their default/null
values) and return this so subsequent chained calls operate on a well-defined,
freshly-initialized builder instance.
d54d03b to
b17a36b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/java/com/example/solidconnection/siteuser/domain/SiteUser.java (1)
130-132: ✅ 상태 변경 메서드가 정상적으로 추가되었습니다어드민의 차단/차단 해제 기능을 위한
updateUserStatus메서드가 명확하게 구현되었습니다. 파라미터명status도 의도를 잘 전달하고 있습니다.선택적 개선사항:
updatePassword메서드처럼 Javadoc을 추가하면 메서드의 목적과 사용 시점을 더 명확히 할 수 있습니다만, 현재 구현으로도 충분히 이해 가능합니다.📝 선택적 개선: Javadoc 추가 예시
+ /** + * 유저의 상태를 변경합니다. (예: 차단, 차단 해제) + * @param status 변경할 유저 상태 + */ public void updateUserStatus(UserStatus status) { this.userStatus = status; }src/main/java/com/example/solidconnection/admin/controller/AdminUserBanController.java (2)
19-24: 관리자 작업에 대한 감사 로깅 추가를 권장합니다.@slf4j 어노테이션은 선언되어 있지만 실제로 로그가 기록되지 않고 있습니다. 유저 차단/해제는 중요한 관리자 작업이므로 다음 정보를 로그로 남기는 것이 좋습니다:
- 작업을 수행한 관리자 ID
- 대상 유저 ID
- 작업 종류 (차단/해제)
- 작업 시각
이를 통해 추후 문제 발생 시 추적이 가능하고, 관리자 작업에 대한 감사 증적을 확보할 수 있습니다.
🔎 감사 로깅 추가 예시
각 엔드포인트에 로그를 추가할 수 있습니다:
@PostMapping("/{userId}/ban") public ResponseEntity<Void> banUser( + @AuthorizedUser long adminId, @PathVariable long userId, @Valid @RequestBody UserBanRequest request ) { + log.info("Admin ban request - adminId: {}, targetUserId: {}, duration: {}", + adminId, userId, request.duration()); adminUserBanService.banUser(userId, request); + log.info("User banned successfully - userId: {}", userId); return ResponseEntity.ok().build(); }@PatchMapping("/{userId}/unban") public ResponseEntity<Void> unbanUser( @AuthorizedUser long adminId, @PathVariable long userId ) { + log.info("Admin unban request - adminId: {}, targetUserId: {}", adminId, userId); adminUserBanService.unbanUser(userId, adminId); + log.info("User unbanned successfully - userId: {}", userId); return ResponseEntity.ok().build(); }
35-42: 구현이 올바르게 되어 있습니다!차단 해제 엔드포인트는 다음과 같이 잘 구현되어 있습니다:
@AuthorizedUser로 관리자 인증 확인 ✓- 관리자 ID를 서비스 계층에 전달하여 감사 추적 가능 ✓
- HTTP PATCH 메서드 사용이 적절함 ✓
선택적 개선사항: 차단 해제 성공 후 응답 바디에 유저의 업데이트된 상태 정보(예:
UserStatus.REPORTED)를 포함하면 클라이언트가 즉시 결과를 확인할 수 있어 더욱 유용할 수 있습니다.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/example/solidconnection/admin/controller/AdminUserBanController.javasrc/main/java/com/example/solidconnection/admin/service/AdminUserBanService.javasrc/main/java/com/example/solidconnection/siteuser/domain/SiteUser.javasrc/test/java/com/example/solidconnection/report/service/ReportServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-12T21:43:45.760Z
Learnt from: nayonsoso
Repo: solid-connection/solid-connect-server PR: 448
File: src/main/java/com/example/solidconnection/siteuser/domain/SiteUser.java:118-121
Timestamp: 2025-08-12T21:43:45.760Z
Learning: SiteUser 도메인의 updatePassword 메서드에서는 파라미터 이름을 newEncodedPassword로 하거나 Javadoc을 추가해서 인코딩된 비밀번호가 들어와야 한다는 것을 명시해야 합니다.
Applied to files:
src/main/java/com/example/solidconnection/siteuser/domain/SiteUser.java
📚 Learning: 2025-09-25T15:53:53.419Z
Learnt from: Gyuhyeok99
Repo: solid-connection/solid-connect-server PR: 513
File: src/test/java/com/example/solidconnection/community/post/service/PostQueryServiceTest.java:180-193
Timestamp: 2025-09-25T15:53:53.419Z
Learning: Gyuhyeok99 prefers using assertThatCode() for exception testing in AssertJ tests, even when testing that exceptions are thrown. This works functionally and the user has confirmed it works well in their codebase.
Applied to files:
src/test/java/com/example/solidconnection/report/service/ReportServiceTest.java
🧬 Code graph analysis (1)
src/main/java/com/example/solidconnection/admin/controller/AdminUserBanController.java (1)
src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java (1)
Slf4j(26-103)
⏰ 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 (6)
src/test/java/com/example/solidconnection/report/service/ReportServiceTest.java (3)
7-7: 변경사항 승인: Import 추가가 적절합니다.ChatParticipant와 ChatParticipantFixture import가 추가되었으며, 아래 테스트 셋업에서 올바르게 사용되고 있습니다.
Also applies to: 10-10
57-59: 변경사항 승인: Fixture 주입이 올바릅니다.ChatParticipantFixture가 다른 fixture들과 동일한 패턴으로 주입되었습니다.
73-74: 변경사항 승인: 데이터 모델 정렬이 올바르게 반영되었습니다.테스트 셋업이 개선된 아키텍처를 반영하도록 수정되었습니다.
개선 사항:
- ChatMessage 생성 시 SiteUser ID 대신 ChatParticipant ID를 사용하도록 변경
- SiteUser → ChatParticipant → ChatMessage 관계가 명확해져 도메인 모델이 더욱 정확해짐
- 채팅방 내 참여자를 통한 메시지 작성이라는 비즈니스 로직이 테스트에 올바르게 반영됨
기존 테스트 시나리오는 모두 유지되며, setUp()에서의 변경만으로 모든 테스트 케이스가 새로운 데이터 구조를 사용하게 됩니다.
src/main/java/com/example/solidconnection/siteuser/domain/SiteUser.java (2)
73-75: ✅ userStatus 필드 추가가 깔끔하게 구현되었습니다다음 사항들이 잘 구현되었습니다:
nullable = false제약 조건으로 모든 유저가 상태를 갖도록 보장EnumType.STRING사용으로 데이터베이스 가독성 확보- 기본값
UserStatus.ACTIVE설정으로 신규 유저의 초기 상태 명확화- 차단 기능 구현을 위한 필수 도메인 필드 역할 수행
107-124: ✅ 생성자 확장이 올바르게 구현되었습니다
UserStatus파라미터가 생성자에 적절하게 추가되었으며, 123번 라인에서 필드 할당도 정확히 이루어지고 있습니다. 이로써 특정 상태를 가진 유저 객체 생성이 가능해졌습니다.src/main/java/com/example/solidconnection/admin/controller/AdminUserBanController.java (1)
26-33: 보안 설정이 /admin 경로를 보호하고 있습니다. 추가 검증이 필요하지 않습니다.리뷰의 보안 취약점 주장은 정확하지 않습니다.
SecurityConfiguration.java라인 66에.requestMatchers("/admin/**").hasRole(ADMIN.name())이 설정되어 있어,/admin경로의 모든 엔드포인트는 이미 전역적으로 ADMIN 역할을 요구합니다. 따라서:
- 보안 취약점이 아님 -
banUser엔드포인트는 역할 기반 접근 제어로 보호됨- 설계 선택의 문제 - 다른 관리자 컨트롤러(
AdminMentorApplicationController)도@AuthorizedUser어노테이션을 사용하지 않으므로, 이것이 의도된 아키텍처 패턴으로 보임- 감사 추적 차이 -
unbanUser가 관리자 ID를 추적하는 이유는UserBan엔티티에unbannedBy필드가 있기 때문인데,bannedBy필드는 존재하지 않음
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (9)
src/main/resources/db/migration/V40__create_user_ban_table.sql (1)
1-15: 데이터베이스 인덱스 추가를 권장합니다.이전 리뷰에서 상세히 분석된 내용입니다.
- 현재
user_ban테이블이 인덱스 없이 생성되고 있습니다.AdminUserBanService의 쿼리 패턴 분석 결과, 다음 인덱스들이 필요합니다:
banned_user_id인덱스: 차단 여부 확인 및 차단 해제 시 사용expired_at인덱스: 만료된 차단 조회 배치 작업에서 사용- 복합 인덱스
(banned_user_id, is_unbanned): 활성 차단 확인 쿼리 최적화별도의 마이그레이션 파일로 인덱스를 추가하는 것이 좋습니다.
🔎 권장 인덱스 추가 마이그레이션
-- 새 마이그레이션 파일 (예: V42__add_indexes_to_user_ban.sql) CREATE INDEX idx_user_ban_banned_user_id ON user_ban(banned_user_id); CREATE INDEX idx_user_ban_expired_at ON user_ban(expired_at); CREATE INDEX idx_user_ban_banned_user_is_unbanned ON user_ban(banned_user_id, is_unbanned);src/main/java/com/example/solidconnection/siteuser/domain/UserBan.java (1)
48-52: 타임존을 명시적으로 지정해주세요.이전 리뷰에서 지적된 내용과 동일합니다.
- Line 51에서
ZonedDateTime.now()를 사용하고 있습니다.AdminUserBanService에서는ZonedDateTime.now(ZoneOffset.UTC)를 일관되게 사용합니다.- 시스템 기본 타임존에 의존하면 예상치 못한 동작이 발생할 수 있습니다.
🔎 수정 제안
+import static java.time.ZoneOffset.UTC; + public void manuallyUnban(Long adminId) { this.isUnbanned = true; this.unbannedBy = adminId; - this.unbannedAt = ZonedDateTime.now(); + this.unbannedAt = ZonedDateTime.now(UTC); }src/test/java/com/example/solidconnection/siteuser/fixture/SiteUserFixture.java (2)
88-98: 하드코딩된 이메일로 인한 중복 키 문제가 발생할 수 있습니다.이전 리뷰에서
차단된_사용자메서드에 대해 지적된 내용과 동일한 문제입니다.
- Line 90에서 이메일이
"[email protected]"으로 고정되어 있습니다.- 동일한 테스트에서 이 메서드를 여러 번 호출하면 중복 키 제약 조건 위반이 발생합니다.
- 다른 픽스처 메서드들은 인덱스를 받아 고유한 이메일을 생성합니다.
🔎 수정 제안
방안 1: 인덱스 파라미터 추가
- public SiteUser 신고된_사용자(String nickname) { + public SiteUser 신고된_사용자(int index, String nickname) { return siteUserFixtureBuilder.siteUser() - .email("[email protected]") + .email("reported" + index + "@example.com") .authType(AuthType.EMAIL) .nickname(nickname) .profileImageUrl("profileImageUrl") .role(Role.MENTEE) .password("reported123") .userStatus(UserStatus.REPORTED) .create(); }방안 2: 닉네임 기반 이메일 생성
public SiteUser 신고된_사용자(String nickname) { return siteUserFixtureBuilder.siteUser() - .email("[email protected]") + .email("reported_" + nickname + "@example.com") .authType(AuthType.EMAIL) .nickname(nickname) .profileImageUrl("profileImageUrl") .role(Role.MENTEE) .password("reported123") .userStatus(UserStatus.REPORTED) .create(); }
100-109: 하드코딩된 이메일로 인한 중복 키 문제가 발생할 수 있습니다.이전 리뷰에서 명시적으로 지적된 내용입니다.
- Line 102에서 이메일이
"[email protected]"으로 고정되어 있습니다.- 동일한 테스트에서 여러 차단된 사용자를 생성할 때 중복 키 제약 조건 위반이 발생합니다.
- 기존 패턴(
사용자(int index, String nickname))처럼 인덱스를 받아 고유한 이메일을 생성하는 것이 권장됩니다.🔎 수정 제안
방안 1: 인덱스 파라미터 추가 (권장)
- public SiteUser 차단된_사용자(String nickname) { + public SiteUser 차단된_사용자(int index, String nickname) { return siteUserFixtureBuilder.siteUser() - .email("[email protected]") + .email("banned" + index + "@example.com") .authType(AuthType.EMAIL) .nickname(nickname) .profileImageUrl("profileImageUrl") .role(Role.MENTEE) .password("banned123") .userStatus(UserStatus.BANNED) .create(); }방안 2: 닉네임 기반 이메일 생성
public SiteUser 차단된_사용자(String nickname) { return siteUserFixtureBuilder.siteUser() - .email("[email protected]") + .email("banned_" + nickname + "@example.com") .authType(AuthType.EMAIL) .nickname(nickname) .profileImageUrl("profileImageUrl") .role(Role.MENTEE) .password("banned123") .userStatus(UserStatus.BANNED) .create(); }src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixture.java (1)
24-32:manuallyUnban()호출 후 변경사항이 저장되지 않습니다.
create()호출로 초기 상태가 저장됩니다.- 이후
manuallyUnban(adminId)호출로 엔티티 상태가 변경됩니다.- 변경된 상태가 데이터베이스에 반영되지 않아 테스트 시 예상치 못한 결과가 발생할 수 있습니다.
🔎 수정 제안
UserBanFixtureBuilder에save메서드를 추가하거나,UserBanRepository를 직접 주입하여 변경사항을 저장해야 합니다:+ private final UserBanRepository userBanRepository; + public UserBan 수동_차단_해제(long bannedUserId, long adminId) { UserBan userBan = userBanFixtureBuilder.userBan() .bannedUserId(bannedUserId) .duration(UserBanDuration.SEVEN_DAYS) .expiredAt(ZonedDateTime.now().plusDays(7)) .create(); userBan.manuallyUnban(adminId); - return userBan; + return userBanRepository.save(userBan); }src/main/java/com/example/solidconnection/report/service/ReportService.java (1)
92-96: 상태 전환 시 기존 상태 계층 검증이 필요합니다.
- 현재 구현은 사용자 상태를 무조건
REPORTED로 변경합니다.- 이미
BANNED상태인 사용자의 상태가REPORTED로 덮어씌워질 수 있습니다.- 일반적으로
BANNED>REPORTED>ACTIVE순서의 우선순위가 적용되어야 합니다.🔎 권장 수정 방향
SiteUser.updateUserStatus()또는 이 메서드에서 상태 계층을 검증하세요:private void updateUserStatusToReported(long userId) { SiteUser user = siteUserRepository.findById(userId) .orElseThrow(() -> new CustomException(ErrorCode.USER_NOT_FOUND)); + if (user.getUserStatus() == UserStatus.BANNED) { + return; // BANNED 상태는 유지 + } user.updateUserStatus(UserStatus.REPORTED); }src/test/java/com/example/solidconnection/admin/service/AdminUserBanServiceTest.java (2)
196-219: 사용자 상태 변경 후 영속화가 필요합니다.
user1.updateUserStatus(UserStatus.BANNED)호출 후 저장되지 않습니다.expireUserBans()실행 시 DB에서 조회한 상태가 업데이트된 상태를 반영하지 못할 수 있습니다.siteUserRepository.saveAll()또는saveAndFlush()를 호출해야 합니다.🔎 수정 제안
userBanFixture.만료된_차단(user1.getId()); userBanFixture.만료된_차단(user2.getId()); user1.updateUserStatus(UserStatus.BANNED); user2.updateUserStatus(UserStatus.BANNED); +siteUserRepository.saveAll(List.of(user1, user2)); // when adminUserBanService.expireUserBans();
243-261: 수동 해제 테스트에서 사용자 상태 영속화가 누락되었습니다.
- Line 247에서
reportedUser.updateUserStatus(UserStatus.REPORTED)호출 후 저장되지 않습니다.- 트랜잭션 경계에 따라 변경사항이 DB에 반영되지 않을 수 있습니다.
🔎 수정 제안
userBanFixture.수동_차단_해제(reportedUser.getId(), admin.getId()); reportedUser.updateUserStatus(UserStatus.REPORTED); +siteUserRepository.save(reportedUser);src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java (1)
90-97: 만료된 차단 처리 시isUnbanned상태가 업데이트되지 않습니다.
expireUserBans()가 만료된 차단을 조회하여processUnban()을 호출합니다.- 하지만
UserBan.isUnbanned가true로 설정되지 않습니다.- 결과적으로 매일 자정마다 동일한 만료 차단이 반복 처리됩니다.
🔎 수정 제안
UserBan엔티티에 만료 처리 메서드를 추가하고 호출하세요:@Transactional @Scheduled(cron = "0 0 0 * * *") public void expireUserBans() { List<UserBan> expiredBans = userBanRepository.findAllByIsUnbannedFalseAndExpiredAtBefore(ZonedDateTime.now(UTC)); for (UserBan userBan : expiredBans) { + userBan.expire(); // isUnbanned = true, unbannedAt = now processUnban(userBan.getBannedUserId()); } }
UserBan엔티티:public void expire() { this.isUnbanned = true; this.unbannedAt = ZonedDateTime.now(UTC); }
🧹 Nitpick comments (3)
src/main/java/com/example/solidconnection/siteuser/domain/UserBan.java (1)
42-46: 생성자 파라미터 검증 추가를 고려해보세요.
- 현재 생성자는
expiredAt이 미래 시점인지 검증하지 않습니다.- 서비스 레이어에서 처리하고 있을 가능성이 높지만, 엔티티 레벨에서의 방어적 검증도 고려할 수 있습니다.
다음과 같은 검증을 추가하면 더욱 안전한 도메인 모델이 됩니다:
expiredAt이 null이 아닌지 확인expiredAt이 현재 시점보다 미래인지 확인🔎 제안하는 수정 방안
public UserBan(Long bannedUserId, UserBanDuration duration, ZonedDateTime expiredAt) { + if (expiredAt == null || !expiredAt.isAfter(ZonedDateTime.now(java.time.ZoneOffset.UTC))) { + throw new IllegalArgumentException("expiredAt must be a future date"); + } this.bannedUserId = bannedUserId; this.duration = duration; this.expiredAt = expiredAt; }src/main/resources/db/migration/V40__create_user_ban_table.sql (1)
20-21: 신고 대상 사용자 추적 컬럼이 추가되었습니다.
reported_id컬럼이 nullable BIGINT로 추가되었습니다.- 기존 신고 데이터와의 호환성을 위해 nullable로 설정된 것으로 보입니다.
선택적으로 참조 무결성을 위한 외래 키 제약조건을 추가할 수 있습니다:
🔎 선택적 개선사항: 외래 키 제약조건
ALTER TABLE report ADD CONSTRAINT fk_report_reported_id FOREIGN KEY (reported_id) REFERENCES site_user (id);단, 이 제약조건을 추가하려면:
- 기존 report 데이터의 reported_id가 모두 유효한 site_user.id를 참조하는지 확인 필요
- 사용자 삭제 정책(CASCADE, SET NULL 등)을 결정해야 함
src/main/java/com/example/solidconnection/report/service/ReportService.java (1)
33-41: 검증 로직의 중복 실행을 고려해 주세요.
findReportedId()가 내부적으로findPostAuthorId()또는findChatMessageSenderId()를 호출합니다.- 이 메서드들은 이미 대상이 존재하지 않으면
REPORT_TARGET_NOT_FOUND예외를 던집니다.- 이후
validateTargetExists()에서 동일한 존재 여부를 다시 확인합니다.🔎 개선 제안
validateTargetExists()호출을 제거하거나,findReportedId()와 통합하여 DB 조회 횟수를 줄일 수 있습니다:@Transactional public void createReport(long reporterId, ReportRequest request) { long reportedId = findReportedId(request.targetType(), request.targetId()); validateReporterAndReportedExists(reporterId, reportedId); - validateTargetExists(request.targetType(), request.targetId()); validateFirstReportByUser(reporterId, request.targetType(), request.targetId()); updateUserStatusToReported(reportedId); Report report = new Report(reporterId, reportedId, request.reportType(), request.targetType(), request.targetId()); reportRepository.save(report); }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.javasrc/main/java/com/example/solidconnection/report/domain/Report.javasrc/main/java/com/example/solidconnection/report/repository/ReportRepository.javasrc/main/java/com/example/solidconnection/report/service/ReportService.javasrc/main/java/com/example/solidconnection/siteuser/domain/UserBan.javasrc/main/resources/db/migration/V40__create_user_ban_table.sqlsrc/test/java/com/example/solidconnection/admin/service/AdminUserBanServiceTest.javasrc/test/java/com/example/solidconnection/report/fixture/ReportFixture.javasrc/test/java/com/example/solidconnection/report/fixture/ReportFixtureBuilder.javasrc/test/java/com/example/solidconnection/report/service/ReportServiceTest.javasrc/test/java/com/example/solidconnection/siteuser/fixture/SiteUserFixture.javasrc/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixture.javasrc/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixtureBuilder.java
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/example/solidconnection/report/repository/ReportRepository.java
- src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixtureBuilder.java
- src/test/java/com/example/solidconnection/report/service/ReportServiceTest.java
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java (2)
src/main/java/com/example/solidconnection/admin/controller/AdminUserBanController.java (1)
RequiredArgsConstructor(19-43)src/main/java/com/example/solidconnection/report/service/ReportService.java (1)
Service(22-97)
src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixture.java (1)
src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixtureBuilder.java (1)
TestComponent(10-43)
⏰ 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 (12)
src/test/java/com/example/solidconnection/report/fixture/ReportFixture.java (1)
14-21: 신고 대상 사용자 추적 기능이 잘 구현되었습니다.
- 메서드 시그니처에
reportedId파라미터가 추가되었습니다.- 빌더 체인을 통해 올바르게 전달되고 있습니다.
- Report 도메인 모델의 변경사항과 일관성 있게 구현되었습니다.
src/test/java/com/example/solidconnection/report/fixture/ReportFixtureBuilder.java (1)
17-17: 빌더 패턴이 일관성 있게 확장되었습니다.
reportedId필드가 추가되었습니다.- 플루언트 스타일의 setter가 올바르게 구현되었습니다.
- Report 생성 시 파라미터로 전달되고 있습니다.
기존 빌더 패턴과 동일한 스타일을 유지하고 있어 코드의 일관성이 좋습니다.
Also applies to: 31-34, 54-54
src/test/java/com/example/solidconnection/siteuser/fixture/SiteUserFixture.java (1)
24-24: 기존 픽스처 메서드들이 올바르게 업데이트되었습니다.
- 모든 기존 사용자 생성 메서드에
userStatus(UserStatus.ACTIVE)가 추가되었습니다.- 일관성 있게 적용되어 기본 상태가 명확합니다.
- 새로운 UserStatus 필드와 잘 통합되었습니다.
Also applies to: 36-36, 48-48, 60-60, 72-72, 84-84
src/main/resources/db/migration/V40__create_user_ban_table.sql (1)
17-18: 사용자 상태 컬럼이 안전하게 추가되었습니다.
user_status컬럼에NOT NULL제약조건과 기본값'ACTIVE'가 설정되었습니다.- 기존 데이터에 대해 안전하게 마이그레이션됩니다.
- UserStatus enum의 값과 일치합니다.
src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixture.java (1)
16-22: LGTM! 만료된 차단 픽스처가 올바르게 구현되었습니다.
- 빌더를 통해 과거 시점(
minusDays(1))으로 만료된 차단을 생성합니다.create()메서드가 내부적으로userBanRepository.save()를 호출하여 영속화가 보장됩니다.src/main/java/com/example/solidconnection/report/domain/Report.java (1)
36-38: LGTM!reportedId필드가 올바르게 추가되었습니다.
- 신고 대상 콘텐츠 작성자의 ID를 추적하기 위한
reportedId필드가 추가되었습니다.- JPA
@Column어노테이션과 생성자 파라미터가 일관성 있게 구현되었습니다.- 이 변경으로 어드민 차단 기능에서 신고된 사용자를 효율적으로 조회할 수 있습니다.
Also applies to: 50-56
src/test/java/com/example/solidconnection/admin/service/AdminUserBanServiceTest.java (3)
68-81: LGTM! 테스트 셋업이 깔끔하게 구성되었습니다.
admin,reportedUser,reporter픽스처가 명확하게 분리되어 있습니다.reportedPost생성을 통해 신고 시나리오의 전제조건을 충족합니다.- 한글 메서드명으로 테스트 의도가 직관적으로 전달됩니다.
87-124: LGTM! 차단 관련 테스트 케이스가 잘 구성되었습니다.
- 정상 차단 테스트: 신고된 사용자 차단 후
BANNED상태 확인- 중복 차단 방지: 이미 차단된 사용자 재차단 시
ALREADY_BANNED_USER예외 검증- 신고 없는 사용자 차단 방지:
REPORT_NOT_FOUND예외 검증모든 케이스에서 given-when-then 패턴이 일관되게 적용되어 있습니다.
129-189: LGTM! 차단 해제 테스트가 포괄적으로 작성되었습니다.
- 수동 해제 테스트: 상태가
REPORTED로 복원됨을 확인- 해제 정보 저장:
isUnbanned,unbannedBy,unbannedAt메타데이터 검증- 미차단 사용자 해제 방지:
NOT_BANNED_USER예외 검증- 만료된 차단 해제 방지: 이미 만료된 차단은 해제 불가 검증
Edge case까지 꼼꼼하게 커버하고 있습니다.
src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java (3)
37-46: LGTM! 차단 로직이 체계적으로 구현되었습니다.
- 검증 단계: 이미 차단 여부와 신고 존재 여부 확인
- 콘텐츠 처리: 신고된 콘텐츠 soft delete 처리
- 차단 생성: 만료 시간 계산 후
UserBan엔티티 저장- 상태 업데이트: 사용자 상태를
BANNED로 변경UTC 기준 시간 처리가 일관되게 적용되어 있습니다.
77-88: LGTM! 수동 차단 해제가 잘 구현되었습니다.
findBannedUser(): 활성 차단 조회 (미해제 + 미만료)manuallyUnban(): 해제 메타데이터 기록 (unbannedBy,unbannedAt,isUnbanned)processUnban(): 콘텐츠 복구 및 상태 변경조회 쿼리가
OrderByCreatedAtDesc로 최신 차단을 우선 처리합니다.
99-102: LGTM! 차단 해제 공통 처리 로직이 잘 분리되었습니다.
updateReportedContentIsDeleted(userId, false): soft delete된 콘텐츠 복구updateUserStatus(userId, UserStatus.REPORTED): 상태를REPORTED로 복원수동 해제와 자동 만료 모두 동일한 로직을 재사용합니다.
- Report, UserBan 컬럼 수정
JAEHEE25
force-pushed
the
feature/590-admin-user-block
branch
from
28594a7 to
adf4c9b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (9)
src/main/java/com/example/solidconnection/report/service/ReportService.java (1)
92-96: 상태 전환 시 BANNED 상태 보호가 필요합니다.이전 리뷰에서 지적된 사항입니다:
현재 문제점
- 이미
BANNED상태인 사용자가 신고되면REPORTED로 상태가 낮아집니다.- 상태 우선순위(
BANNED>REPORTED>ACTIVE)가 보장되지 않습니다.권장 수정 방향
BANNED상태인 사용자는 상태를 변경하지 않도록 조건 검사를 추가하세요.🔎 제안하는 수정 방향
private void updateUserStatusToReported(long userId) { SiteUser user = siteUserRepository.findById(userId) .orElseThrow(() -> new CustomException(ErrorCode.USER_NOT_FOUND)); + if (user.getUserStatus() == UserStatus.BANNED) { + return; + } user.updateUserStatus(UserStatus.REPORTED); }src/main/resources/db/migration/V40__create_user_ban_table.sql (1)
1-15: 인덱스 누락 문제는 이미 이전 리뷰에서 지적되었습니다.이전 리뷰 코멘트에서
user_ban테이블에 필요한 인덱스들(banned_user_id,expired_at, 복합 인덱스)이 누락되었다는 점을 상세히 분석하고 별도 마이그레이션으로 추가할 것을 권장했습니다. 해당 내용을 참고해 주세요.src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixtureBuilder.java (1)
20-22: 빌더 패턴의 명확성을 개선할 수 있습니다.
userBan()메서드가 새 인스턴스를 생성하는 방식은 동작하지만, 필드 초기화가 암시적입니다. 현재 인스턴스를 재사용하고 필드를 명시적으로 리셋하는 방식으로 변경하면 빌더 패턴의 의도가 더 명확해집니다.🔎 개선 제안
현재 인스턴스를 재사용하고 필드를 명시적으로 초기화:
public UserBanFixtureBuilder userBan() { - return new UserBanFixtureBuilder(userBanRepository); + this.bannedUserId = null; + this.duration = null; + this.expiredAt = null; + return this; }src/test/java/com/example/solidconnection/siteuser/fixture/SiteUserFixture.java (1)
100-109: 하드코딩된 이메일로 인해 동일 테스트에서 여러 차단된 사용자 생성 시 중복 키 오류가 발생합니다.Line 102의 이메일이
"[email protected]"으로 하드코딩되어 있어, 동일한 테스트 내에서 이 픽스처를 여러 번 호출하면 데이터베이스 중복 키 제약 조건 위반이 발생합니다. 인덱스 파라미터를 추가하거나 닉네임 기반으로 이메일을 생성하는 것을 권장합니다.🔎 수정 제안
방안 1: 인덱스 파라미터 추가
- public SiteUser 차단된_사용자(String nickname) { + public SiteUser 차단된_사용자(int index, String nickname) { return siteUserFixtureBuilder.siteUser() - .email("[email protected]") + .email("banned" + index + "@example.com") .authType(AuthType.EMAIL) .nickname(nickname) .profileImageUrl("profileImageUrl") .role(Role.MENTEE) .password("banned123") .userStatus(UserStatus.BANNED) .create(); }src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixture.java (1)
24-32:manuallyUnban()호출 후 변경사항이 데이터베이스에 저장되지 않습니다.Line 29에서
userBan.manuallyUnban(adminId)를 호출하지만, 이 변경사항(isUnbanned, unbannedBy, unbannedAt)이 데이터베이스에 반영되지 않습니다.UserBanFixtureBuilder또는 이 메서드에서 직접 저장 로직을 추가해야 합니다.🔎 수정 제안
UserBanRepository를 주입받아 변경사항을 저장:+ private final UserBanRepository userBanRepository; + public UserBan 수동_차단_해제(long bannedUserId, long adminId) { UserBan userBan = userBanFixtureBuilder.userBan() .bannedUserId(bannedUserId) .duration(UserBanDuration.SEVEN_DAYS) .expiredAt(ZonedDateTime.now().plusDays(7)) .create(); userBan.manuallyUnban(adminId); - return userBan; + return userBanRepository.save(userBan); }src/main/java/com/example/solidconnection/siteuser/domain/UserBan.java (1)
54-58: 타임존을 명시하여 일관성을 확보해야 합니다.Line 57에서
ZonedDateTime.now()를 사용하고 있지만,AdminUserBanService에서는ZonedDateTime.now(UTC)를 일관되게 사용합니다. 타임존을 명시하지 않으면 시스템 기본 타임존이 사용되어 예상치 못한 동작이 발생할 수 있습니다.🔎 수정 제안
UTC 타임존을 명시적으로 지정:
+import static java.time.ZoneOffset.UTC; + public void manuallyUnban(Long adminId) { this.isUnbanned = true; this.unbannedBy = adminId; - this.unbannedAt = ZonedDateTime.now(); + this.unbannedAt = ZonedDateTime.now(UTC); }src/test/java/com/example/solidconnection/admin/service/AdminUserBanServiceTest.java (2)
196-219: 사용자 상태 변경 후 영속화가 필요합니다.Lines 205-206에서
user1.updateUserStatus(UserStatus.BANNED)와user2.updateUserStatus(UserStatus.BANNED)호출 후, 변경사항이 데이터베이스에 반영되지 않을 수 있습니다.siteUserRepository.save()또는saveAndFlush()를 호출하여 상태 변경을 명시적으로 저장해주세요.🔎 수정 제안
상태 변경 후 명시적으로 저장:
user1.updateUserStatus(UserStatus.BANNED); user2.updateUserStatus(UserStatus.BANNED); +siteUserRepository.saveAll(List.of(user1, user2)); // when adminUserBanService.expireUserBans();
243-261: 수동 해제 테스트에서 사용자 상태 영속화가 누락되었습니다.Line 247에서
reportedUser.updateUserStatus(UserStatus.REPORTED)호출 후 저장하지 않으면, 트랜잭션 경계에 따라 변경사항이 반영되지 않을 수 있습니다. 상태 변경을 명시적으로 저장한 후 테스트를 진행해주세요.🔎 수정 제안
상태 업데이트 후 저장:
userBanFixture.수동_차단_해제(reportedUser.getId(), admin.getId()); reportedUser.updateUserStatus(UserStatus.REPORTED); +siteUserRepository.save(reportedUser); long beforeUnbannedCount = userBanRepository.findAll().stream()src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java (1)
90-97: 🔴 중요: 만료된 차단 기록이 업데이트되지 않아 매일 반복 처리됩니다.이 문제는 이전 리뷰에서도 지적되었으나 아직 수정되지 않았습니다.
현재
expireUserBans()메서드의 문제점:
- Line 93에서
isUnbanned=false인 만료된 차단을 조회합니다- Line 95에서
processUnban()을 호출하여 콘텐츠와 유저 상태를 업데이트합니다- 하지만
UserBan엔티티의isUnbanned플래그를 업데이트하지 않습니다- 결과: 다음날 자정에 동일한 만료 차단이 다시 조회되어 반복 처리됩니다
대조적으로, Line 80의
unbanUser()메서드는userBan.manuallyUnban(adminId)를 호출하여 차단 기록을 올바르게 업데이트합니다.🔎 수정 제안
방법 1: UserBan 엔티티에 expire() 메서드 추가 (권장)
UserBan.java에 다음 메서드를 추가하세요:public void expire() { this.isUnbanned = true; this.unbannedAt = ZonedDateTime.now(UTC); }그리고
expireUserBans()를 수정하세요:@Transactional @Scheduled(cron = "0 0 0 * * *") public void expireUserBans() { List<UserBan> expiredBans = userBanRepository.findAllByIsUnbannedFalseAndExpiredAtBefore(ZonedDateTime.now(UTC)); for (UserBan userBan : expiredBans) { + userBan.expire(); processUnban(userBan.getBannedUserId()); } }방법 2: processUnban에 UserBan 매개변수 추가
@Transactional @Scheduled(cron = "0 0 0 * * *") public void expireUserBans() { List<UserBan> expiredBans = userBanRepository.findAllByIsUnbannedFalseAndExpiredAtBefore(ZonedDateTime.now(UTC)); for (UserBan userBan : expiredBans) { - processUnban(userBan.getBannedUserId()); + userBan.expire(); + processUnban(userBan); } } -private void processUnban(long userId) { +private void processUnban(UserBan userBan) { + long userId = userBan.getBannedUserId(); updateReportedContentIsDeleted(userId, false); updateUserStatus(userId, UserStatus.REPORTED); }
🧹 Nitpick comments (2)
src/main/java/com/example/solidconnection/report/service/ReportService.java (1)
33-42: 중복 데이터베이스 쿼리를 개선할 수 있습니다.
현재 문제점
findReportedId()(Line 34)에서 Post/ChatMessage를 조회합니다.validateTargetExists()(Line 36)에서 동일한 엔티티 존재 여부를 다시 확인합니다.개선 방향
findReportedId()에서 이미 엔티티를 조회하므로, 별도의validateTargetExists()호출이 불필요합니다.- 엔티티가 없으면
findReportedId()에서 이미REPORT_TARGET_NOT_FOUND예외가 발생합니다.🔎 제안하는 수정 방향
@Transactional public void createReport(long reporterId, ReportRequest request) { long reportedId = findReportedId(request.targetType(), request.targetId()); validateReporterAndReportedExists(reporterId, reportedId); - validateTargetExists(request.targetType(), request.targetId()); validateFirstReportByUser(reporterId, request.targetType(), request.targetId()); updateUserStatusToReported(reportedId); Report report = new Report(reporterId, reportedId, request.reportType(), request.targetType(), request.targetId()); reportRepository.save(report); }
validateTargetExists()메서드는 다른 곳에서 사용되지 않는다면 제거를 고려하세요.src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java (1)
26-36: 관찰된 개선 사항: 감사 로깅 추가를 고려해보세요.@slf4j 어노테이션이 선언되어 있지만, 실제로 로그를 남기는 코드가 없습니다. 다음과 같은 중요한 관리자 작업에 대해 감사 로깅을 추가하는 것을 권장합니다:
- 유저 차단 시작 시 (userId, adminId, duration, reason)
- 유저 차단 해제 시 (userId, adminId)
- 자동 만료 처리 시 (만료된 차단 건수)
이는 문제 추적과 감사 목적으로 유용합니다.
🔎 로깅 추가 제안
@Transactional public void banUser(long userId, UserBanRequest request) { ZonedDateTime now = ZonedDateTime.now(UTC); + log.info("Starting ban process for userId={}, duration={}, reason={}", userId, request.duration(), request.reason()); validateNotAlreadyBanned(userId, now); validateReportExists(userId); updateReportedContentIsDeleted(userId, true); createUserBan(userId, request, now); updateUserStatus(userId, UserStatus.BANNED); + log.info("Successfully banned userId={}, expiredAt={}", userId, now.plusDays(request.duration().getDays())); } @Transactional public void unbanUser(long userId, long adminId) { + log.info("Starting manual unban process for userId={} by adminId={}", userId, adminId); UserBan userBan = findBannedUser(userId, ZonedDateTime.now(UTC)); userBan.manuallyUnban(adminId); processUnban(userId); + log.info("Successfully unbanned userId={} by adminId={}", userId, adminId); } @Transactional @Scheduled(cron = "0 0 0 * * *") public void expireUserBans() { List<UserBan> expiredBans = userBanRepository.findAllByIsUnbannedFalseAndExpiredAtBefore(ZonedDateTime.now(UTC)); + log.info("Processing {} expired bans", expiredBans.size()); for (UserBan userBan : expiredBans) { processUnban(userBan.getBannedUserId()); } + log.info("Completed processing {} expired bans", expiredBans.size()); }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.javasrc/main/java/com/example/solidconnection/report/domain/Report.javasrc/main/java/com/example/solidconnection/report/repository/ReportRepository.javasrc/main/java/com/example/solidconnection/report/service/ReportService.javasrc/main/java/com/example/solidconnection/siteuser/domain/UserBan.javasrc/main/resources/db/migration/V40__create_user_ban_table.sqlsrc/test/java/com/example/solidconnection/admin/service/AdminUserBanServiceTest.javasrc/test/java/com/example/solidconnection/report/fixture/ReportFixture.javasrc/test/java/com/example/solidconnection/report/fixture/ReportFixtureBuilder.javasrc/test/java/com/example/solidconnection/report/service/ReportServiceTest.javasrc/test/java/com/example/solidconnection/siteuser/fixture/SiteUserFixture.javasrc/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixture.javasrc/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixtureBuilder.java
🚧 Files skipped from review as they are similar to previous changes (3)
- src/test/java/com/example/solidconnection/report/fixture/ReportFixtureBuilder.java
- src/test/java/com/example/solidconnection/report/service/ReportServiceTest.java
- src/main/java/com/example/solidconnection/report/repository/ReportRepository.java
🧰 Additional context used
🧬 Code graph analysis (2)
src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixture.java (1)
src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixtureBuilder.java (1)
TestComponent(10-43)
src/test/java/com/example/solidconnection/admin/service/AdminUserBanServiceTest.java (1)
src/test/java/com/example/solidconnection/report/service/ReportServiceTest.java (2)
Nested(79-119)Nested(121-161)
⏰ 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 (11)
src/test/java/com/example/solidconnection/report/fixture/ReportFixture.java (1)
14-21: LGTM! 테스트 픽스처가 새로운reportedId파라미터를 잘 반영했습니다.
메서드 시그니처 업데이트
reporterId,reportedId,targetType,targetId순서로 논리적으로 배치되었습니다.빌더 체인 확장
.reportedId(reportedId)호출이 올바르게 추가되었습니다.Report 엔티티의 새로운 구조와 일관되게 업데이트되었습니다.
src/main/java/com/example/solidconnection/report/domain/Report.java (1)
36-38: LGTM!reportedId필드가 적절하게 추가되었습니다.
새 필드 추가 (Lines 36-38)
@Column(name = "reported_id")매핑이 기존 필드들과 일관된 네이밍 컨벤션을 따릅니다.- 기존
reporterId,targetId와 동일하게 primitivelong타입을 사용합니다.생성자 확장 (Lines 50-56)
- 파라미터 순서가
reporterId→reportedId순으로 논리적입니다.- 필드 할당이 올바르게 수행됩니다.
차단 기능 구현에 필요한 신고 대상자 추적이 가능해졌습니다.
Also applies to: 50-56
src/main/java/com/example/solidconnection/report/service/ReportService.java (2)
71-90: LGTM! 신고 대상자 ID 조회 로직이 잘 구현되었습니다.
findReportedId()(Lines 71-76)
TargetType에 따른 분기 처리가 명확합니다.
findPostAuthorId()(Lines 78-82)
- Post 작성자 ID를 직접 반환하는 간단한 로직입니다.
findChatMessageSenderId()(Lines 84-90)
- ChatMessage → ChatParticipant → SiteUser ID 조회 체인이 도메인 모델에 맞게 구현되었습니다.
- 각 단계에서 적절한 예외 처리(
REPORT_TARGET_NOT_FOUND,CHAT_PARTICIPANT_NOT_FOUND)가 적용되었습니다.
44-52: 검증 로직이 적절합니다.
Reporter와 Reported 존재 확인
- 두 사용자 모두 존재하는지 검증합니다.
- 동일한
USER_NOT_FOUND에러 코드를 사용하는 것은 합리적입니다.선택적 개선 사항
- 디버깅 편의를 위해 reporter/reported를 구분하는 별도 에러 코드를 고려할 수 있습니다.
- 현재 구현도 기능적으로 충분합니다.
src/main/resources/db/migration/V40__create_user_ban_table.sql (1)
17-18: 잘 구현되었습니다.
user_status컬럼이 적절한 기본값(ACTIVE)과 함께 추가되었으며, VARCHAR(10) 크기는 모든 enum 값(ACTIVE, REPORTED, BANNED)을 수용하기에 충분합니다.src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixture.java (1)
16-22: 만료된 차단 픽스처가 올바르게 구현되었습니다.과거 시간을
expiredAt으로 설정하여 만료된 차단 상태를 정확히 시뮬레이션하고 있습니다. 빌더의create()메서드를 통해 리포지토리에 저장되므로 테스트에서 안정적으로 사용할 수 있습니다.src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixtureBuilder.java (1)
39-42: 엔티티 생성 및 저장 로직이 올바르게 구현되었습니다.빌더 필드로
UserBan엔티티를 생성하고 리포지토리를 통해 저장한 후 반환하는 흐름이 정확합니다. 테스트에서 영속화된 엔티티를 안정적으로 사용할 수 있습니다.src/main/java/com/example/solidconnection/siteuser/domain/UserBan.java (1)
48-52: 차단 엔티티 생성 로직이 명확합니다.필수 필드들을 받아 엔티티를 초기화하는 생성자가 간결하고 올바르게 구현되었습니다.
src/test/java/com/example/solidconnection/admin/service/AdminUserBanServiceTest.java (1)
88-99: 차단 기능 테스트가 올바르게 구현되었습니다.신고 생성 후 차단 요청을 처리하고 사용자 상태가
BANNED로 변경되는지 검증하는 흐름이 명확합니다.src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java (2)
99-102: 문제없음: 현재 구현이 의도된 설계와 일치합니다.
API 접근 제한 확인됨:
BannedUserInterceptor는BANNED상태만 차단하며,REPORTED상태는 제약이 없습니다. 따라서 차단 해제 후REPORTED상태의 사용자는 모든 API에 접근 가능합니다.상태 복구 로직이 의도된 설계: 사용자 상태 흐름은 다음과 같이 설계되었습니다:
ACTIVE(신규) →REPORTED(신고됨) →BANNED(관리자 차단) →REPORTED(차단 해제)- 신고 이력이 남아 있으므로 차단 해제 후
REPORTED상태로 유지되는 것이 올바른 설계입니다.REPORTED→ACTIVE로의 전환은 별도의 신고 해결 프로세스에서 처리되어야 합니다.테스트 코드에서도 차단 해제 후 상태를
REPORTED로 기대하고 있어 현재 구현이 올바릅니다.
37-63: 신고된 콘텐츠 관리 로직 재검토 필요다음 구현 세부사항을 기반으로 우려사항이 확인되었습니다:
쿼리 분석
- PostRepository의
updateReportedPostsIsDeleted(): 사용자의 모든 신고된 게시글을 업데이트 (시간 기반 필터링 없음)- ChatMessageRepository의
updateReportedChatMessagesIsDeleted(): 사용자의 모든 신고된 채팅 메시지를 업데이트 (시간 기반 필터링 없음)잠재적 문제 확인
- 차단 해제 시
processUnban()에서updateReportedContentIsDeleted(userId, false)호출 → 모든 신고된 콘텐츠가 복구됨- 재차단 시 모든 신고된 콘텐츠가 다시 삭제됨
- 보고서(Report) 엔터티에 시간 정보가 없어서 어떤 신고가 어느 차단 사이클에 속하는지 추적 불가능
의도 확인 필요
기획상 의도를 명확히 하고, 필요하다면:
- 각 신고에 대한 차단 사이클 추적 메커니즘 추가
- 신고 생성 시점 기반 필터링으로 해당 차단 사이클의 콘텐츠만 관리
- 또는 차단/해제 이력과 신고를 연결하는 구조 검토
src/test/java/com/example/solidconnection/siteuser/fixture/SiteUserFixture.java
Show resolved
Hide resolved
src/main/java/com/example/solidconnection/admin/controller/AdminUserBanController.java
Outdated
Show resolved
Hide resolved
| @RequiredArgsConstructor | ||
| @RequestMapping("/admin/users") | ||
| @RestController | ||
| @Slf4j |
| import org.springframework.stereotype.Service; | ||
| import org.springframework.transaction.annotation.Transactional; | ||
|
|
||
| @Slf4j |
|
|
||
| private void updateUserStatus(long userId, UserStatus status) { | ||
| SiteUser user = siteUserRepository.findById(userId) | ||
| .orElseThrow(() -> new CustomException(ErrorCode.USER_NOT_FOUND)); |
There was a problem hiding this comment.
userId 관련 검증이 있는 건 좋은데, private 메서드 호출 전에 수행하면 좋을 거 같습니다
| import com.example.solidconnection.report.domain.TargetType; | ||
| import org.springframework.data.jpa.repository.JpaRepository; | ||
| import org.springframework.data.jpa.repository.Query; | ||
| import org.springframework.data.repository.query.Param; |
src/main/java/com/example/solidconnection/admin/controller/AdminUserBanController.java
Show resolved
Hide resolved
| import com.example.solidconnection.siteuser.domain.UserStatus; | ||
| import com.example.solidconnection.siteuser.repository.SiteUserRepository; | ||
| import com.example.solidconnection.siteuser.repository.UserBanRepository; | ||
| import static java.time.ZoneOffset.UTC; |
There was a problem hiding this comment.
일반 import, static import 구분해주세요 !
| .hasMessage(REPORT_NOT_FOUND.getMessage()); | ||
| } | ||
|
|
||
| @Nested |
| } | ||
|
|
||
| @Nested | ||
| @DisplayName("사용자 차단") |
There was a problem hiding this comment.
@Nested 클래스는 @DisplayName 어노테이션 없는 것이 컨벤션입니다 !
| @Autowired | ||
| private PostFixture postFixture; | ||
| @Autowired | ||
| private BoardFixture boardFixture; |
JAEHEE25
force-pushed
the
feature/590-admin-user-block
branch
from
1839868 to
adf4c9b
Compare
Gyuhyeok99
left a comment
Gyuhyeok99
left a comment
There was a problem hiding this comment.
고생 많으셨습니다!! 리뷰 남겨놓았습니다!
| public class AdminUserBanController { | ||
| private final AdminUserBanService adminUserBanService; | ||
|
|
||
| @PostMapping("/{userId}/ban") |
There was a problem hiding this comment.
카멜케이스가 아닌 user-id로 바꿔주세요! 아래도요!
| @@ -0,0 +1,12 @@ | |||
| package com.example.solidconnection.admin.dto; | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
여기 개행이 두 개 있는데 저희 코딩컨벤션 import를 한 번 하시면 좋을 거 같네요!
| @Entity | ||
| @Getter | ||
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| @Where(clause = "is_deleted = false") |
There was a problem hiding this comment.
이런 기능은 처음 알았네요!
is_deleted를 놓치지 않기 위해 사용하신 거 같은데 성혁님이 말씀해주신 건 반영하면 좋겠네요!
| } | ||
|
|
||
| @Test | ||
| void 이미_차단된_사용자는_다시_차단할_수_없다() { |
There was a problem hiding this comment.
예외테스트이기에 예외가 발생한다로 통일하면 좋겠습니다!
| } | ||
|
|
||
| @Test | ||
| void 신고가_없는_사용자는_차단할_수_없다() { |
| @Nested | ||
| @DisplayName("만료된 차단 자동 해제") |
| for (UserBan userBan : expiredBans) { | ||
| processUnban(userBan.getBannedUserId()); | ||
| } |
There was a problem hiding this comment.
여기 n + 1 발생할 거 같은데 추후 벌크연산이나 select 없이 바로 update쿼리 날리는 식으로 수정하는 건 어떤가요?
There was a problem hiding this comment.
추가로 혹시 이거 중간에 예외 터지면 전체롤백되는 거 같은데 try-catch로 로그만 찍는다던가 등의 예외처리를 하는 건 어떤가요?
| ZonedDateTime now = ZonedDateTime.now(UTC); | ||
| validateNotAlreadyBanned(userId, now); | ||
| validateReportExists(userId); | ||
|
|
||
| updateReportedContentIsDeleted(userId, true); | ||
| createUserBan(userId, request, now); | ||
| updateUserStatus(userId, UserStatus.BANNED); |
There was a problem hiding this comment.
여기 동시성이슈 발생할 수 있을 거 같은데 유니크키 추가하는 거 어떤가요?
There was a problem hiding this comment.
흠 bannedUserId와 isExpired=true로 만료되지 않은 차단에 대해 유니크 키를 적용하고 싶긴 한데,
mysql은 부분 인덱스 지원이 안되는 걸로 알고 있어서 적용이 쉽지 않네요.
만료될 경우 NULL 값을 가지는 flag 역할의 컬럼을 추가하는 방법도 있을 것 같긴 한데 어드민 API 특성 상 동시성 이슈가 발생할 일이 적기도 해서...어떤 방법이 좋을까요?
There was a problem hiding this comment.
으음.. 이건 애매하네요 팀에 공유한 번 해보죠!
| public void manuallyUnban(Long adminId) { | ||
| this.isUnbanned = true; | ||
| this.unbannedBy = adminId; | ||
| this.unbannedAt = ZonedDateTime.now(); |
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| private Long id; | ||
|
|
||
| @Column(name = "banned_user_id", nullable = false) | ||
| private Long bannedUserId; | ||
|
|
||
| @Column(name = "duration", nullable = false) |
There was a problem hiding this comment.
현재는 어떤 유저가 차단되었는지만 알고 어떤 어드민이 차단되었는지는 모르는 거 같은데 인지만하고 우선 이건 넘어가죠!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/main/resources/db/migration/V40__create_user_ban_table.sql:
- Line 17: The CREATE TABLE in V40__create_user_ban_table.sql has an extra
trailing comma before the closing parenthesis causing a SQL syntax error; remove
the trailing comma after the last column/constraint (the final FOREIGN KEY
constraint in the user_ban table definition) so the statement ends with ... )
followed by the semicolon.
🧹 Nitpick comments (6)
src/main/java/com/example/solidconnection/siteuser/repository/SiteUserRepository.java (1)
25-29: 벌크 업데이트 특성상clearAutomatically/flushAutomatically(또는 호출부 주의)가 있으면 더 안전합니다.
- JPQL bulk update는 1차 캐시를 스킵해서, 같은 트랜잭션에서
SiteUser를 이미 로딩했다면 상태가 “업데이트 전”으로 남을 수 있어요.userIds가 빈 리스트면IN ()류로 터질 수 있어서, 호출부에서 가드(빈 리스트면 early return)도 같이 있으면 좋습니다.제안 수정안
@@ - @Modifying + @Modifying(clearAutomatically = true, flushAutomatically = true) @Query("UPDATE SiteUser u SET u.userStatus = :status WHERE u.id IN :userIds") - void bulkUpdateUserStatus(@Param("userIds") List<Long> userIds, @Param("status") UserStatus status); + int bulkUpdateUserStatus(@Param("userIds") List<Long> userIds, @Param("status") UserStatus status);src/main/java/com/example/solidconnection/community/post/repository/PostRepository.java (1)
53-67: 현재 MySQL 환경에서는SET p.is_deleted문법이 완벽하게 유효하지만, 코드 호환성과 가독성 개선을 위해 alias 제거를 권장합니다.프로젝트가 MySQL만 사용 중이므로 현재 코드는 정상 작동하는 상태입니다. 다만 다음의 이유로 개선을 제안합니다:
SQL 표준 호환성 강화
- SET 절에서 alias를 제거하면 ANSI SQL 표준에 더 부합합니다.
- 향후 다른 데이터베이스로 마이그레이션할 경우 호환성을 미리 확보할 수 있습니다.
코드 일관성 유지
- PostRepository와 ChatMessageRepository 모두 동일한 패턴을 사용하고 있으므로, 함께 정리하면 좋습니다.
제안 수정안
@@PostRepository.java @Modifying(clearAutomatically = true, flushAutomatically = true) @Query(value = """ - UPDATE post p SET p.is_deleted = :isDeleted + UPDATE post p SET is_deleted = :isDeleted WHERE p.site_user_id = :siteUserId AND p.id IN (SELECT r.target_id FROM report r WHERE r.target_type = 'POST') """, nativeQuery = true) void updateReportedPostsIsDeleted(@Param("siteUserId") long siteUserId, @Param("isDeleted") boolean isDeleted); @@ @Modifying(clearAutomatically = true, flushAutomatically = true) @Query(value = """ - UPDATE post p SET p.is_deleted = :isDeleted + UPDATE post p SET is_deleted = :isDeleted WHERE p.site_user_id IN :siteUserIds AND p.id IN (SELECT r.target_id FROM report r WHERE r.target_type = 'POST') """, nativeQuery = true) void bulkUpdateReportedPostsIsDeleted(@Param("siteUserIds") List<Long> siteUserIds, @Param("isDeleted") boolean isDeleted);@@ChatMessageRepository.java @Modifying(clearAutomatically = true, flushAutomatically = true) @Query(value = """ - UPDATE chat_message cm SET cm.is_deleted = :isDeleted + UPDATE chat_message cm SET is_deleted = :isDeleted WHERE cm.id IN (SELECT r.target_id FROM report r WHERE r.target_type = 'CHAT') AND cm.sender_id IN (SELECT cp.id FROM chat_participant cp WHERE cp.site_user_id = :siteUserId) """, nativeQuery = true) void updateReportedChatMessagesIsDeleted(@Param("siteUserId") long siteUserId, @Param("isDeleted") boolean isDeleted); @@ @Modifying(clearAutomatically = true, flushAutomatically = true) @Query(value = """ - UPDATE chat_message cm SET cm.is_deleted = :isDeleted + UPDATE chat_message cm SET is_deleted = :isDeleted WHERE cm.id IN (SELECT r.target_id FROM report r WHERE r.target_type = 'CHAT') AND cm.sender_id IN (SELECT cp.id FROM chat_participant cp WHERE cp.site_user_id IN :siteUserIds) """, nativeQuery = true) void bulkUpdateReportedChatMessagesIsDeleted(@Param("siteUserIds") List<Long> siteUserIds, @Param("isDeleted") boolean isDeleted);src/main/resources/db/migration/V40__create_user_ban_table.sql (2)
1-17: 쿼리 성능을 위한 인덱스 추가를 권장합니다.
UserBanRepository에서banned_user_id와is_expired조합으로 조회하는 쿼리가 자주 사용됩니다. 조회 성능 향상을 위해 복합 인덱스를 추가하는 것이 좋습니다.♻️ 인덱스 추가 제안
CONSTRAINT fk_user_ban_unbanned_by_id FOREIGN KEY (unbanned_by) REFERENCES site_user (id) +); + +CREATE INDEX idx_user_ban_banned_user_expired ON user_ban (banned_user_id, is_expired, expired_at);
22-23:reported_id컬럼에 외래 키 제약 조건 추가를 검토해 주세요.
reported_id는site_user테이블을 참조하는 것으로 보입니다. 데이터 무결성을 위해 외래 키 제약 조건을 추가하는 것을 권장합니다. 다만, 기존 데이터가 있다면 NULL 값 처리에 주의가 필요합니다.♻️ 외래 키 추가 제안
ALTER TABLE report - ADD COLUMN reported_id BIGINT; + ADD COLUMN reported_id BIGINT, + ADD CONSTRAINT fk_report_reported_id FOREIGN KEY (reported_id) REFERENCES site_user (id);src/main/java/com/example/solidconnection/siteuser/repository/UserBanRepository.java (1)
21-23:@Modifying어노테이션에clearAutomatically = true옵션 추가를 권장합니다.벌크 업데이트 후 영속성 컨텍스트와 DB 상태가 불일치할 수 있습니다.
expireUserBans()메서드에서 벌크 업데이트 이후 같은 트랜잭션 내에서 해당 엔티티를 다시 조회하는 경우, 캐시된 값이 반환될 수 있습니다.♻️ 수정 제안
- @Modifying + @Modifying(clearAutomatically = true) @Query("UPDATE UserBan ub SET ub.isExpired = true WHERE ub.isExpired = false AND ub.expiredAt < :current") void bulkExpireUserBans(@Param("current") ZonedDateTime current);src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java (1)
103-105: 예외 발생 시 알림 또는 재시도 로직 추가를 검토해 주세요.현재 예외 발생 시 로그만 남기고 있습니다. 스케줄된 작업이 실패하면 다음 날 자정까지 재시도되지 않으므로, 실패 시 알림을 보내거나 재시도 메커니즘을 추가하는 것이 좋습니다.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/main/java/com/example/solidconnection/admin/controller/AdminUserBanController.javasrc/main/java/com/example/solidconnection/admin/dto/UserBanRequest.javasrc/main/java/com/example/solidconnection/admin/service/AdminUserBanService.javasrc/main/java/com/example/solidconnection/chat/repository/ChatMessageRepository.javasrc/main/java/com/example/solidconnection/common/interceptor/BannedUserInterceptor.javasrc/main/java/com/example/solidconnection/community/post/repository/PostRepository.javasrc/main/java/com/example/solidconnection/report/repository/ReportRepository.javasrc/main/java/com/example/solidconnection/siteuser/domain/UserBan.javasrc/main/java/com/example/solidconnection/siteuser/repository/SiteUserRepository.javasrc/main/java/com/example/solidconnection/siteuser/repository/UserBanRepository.javasrc/main/resources/db/migration/V40__create_user_ban_table.sqlsrc/test/java/com/example/solidconnection/admin/service/AdminUserBanServiceTest.javasrc/test/java/com/example/solidconnection/common/interceptor/BannedUserInterceptorTest.javasrc/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixture.javasrc/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixtureBuilder.java
🚧 Files skipped from review as they are similar to previous changes (5)
- src/main/java/com/example/solidconnection/admin/dto/UserBanRequest.java
- src/main/java/com/example/solidconnection/common/interceptor/BannedUserInterceptor.java
- src/test/java/com/example/solidconnection/admin/service/AdminUserBanServiceTest.java
- src/test/java/com/example/solidconnection/common/interceptor/BannedUserInterceptorTest.java
- src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixture.java
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-11T14:39:49.462Z
Learnt from: Gyuhyeok99
Repo: solid-connection/solid-connect-server PR: 443
File: src/main/resources/db/migration/V28__alter_mentor_introduction_pass_tip_not_null.sql:1-5
Timestamp: 2025-08-11T14:39:49.462Z
Learning: The mentor table currently has no data in production/staging environment, so NULL backfill operations are not needed when adding NOT NULL constraints to the introduction and pass_tip columns in the mentor table.
Applied to files:
src/main/resources/db/migration/V40__create_user_ban_table.sql
📚 Learning: 2025-08-24T16:30:16.681Z
Learnt from: nayonsoso
Repo: solid-connection/solid-connect-server PR: 479
File: src/main/java/com/example/solidconnection/auth/controller/AuthController.java:13-16
Timestamp: 2025-08-24T16:30:16.681Z
Learning: When reviewing Java import statements, always verify that both classes are not in the same package before suggesting import additions. Classes in the same package don't need explicit imports in Java.
Applied to files:
src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java
📚 Learning: 2025-07-28T03:14:33.849Z
Learnt from: whqtker
Repo: solid-connection/solid-connect-server PR: 408
File: src/main/java/com/example/solidconnection/chat/service/ChatService.java:46-54
Timestamp: 2025-07-28T03:14:33.849Z
Learning: 페치 조인을 사용한 N+1 문제 해결에 대해 사용자 whqtker는 JPQL 쿼리 접근법을 선호하며, SELECT 쿼리의 경우 영속성 컨텍스트 간 불일치 문제를 고려하지 않아도 된다고 인식하고 있습니다.
Applied to files:
src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java
📚 Learning: 2025-07-29T17:26:08.811Z
Learnt from: whqtker
Repo: solid-connection/solid-connect-server PR: 408
File: src/main/java/com/example/solidconnection/chat/repository/ChatReadStatusRepository.java:11-17
Timestamp: 2025-07-29T17:26:08.811Z
Learning: 사용자 whqtker는 JPA save() 메서드를 사용한 upsert 연산에서 레이스 컨디션 문제를 우려하고 있으며, 특히 채팅 읽음 상태 업데이트와 같이 동시성이 중요한 연산에서는 네이티브 쿼리의 원자적 연산이 더 안전하다고 인식하고 있습니다.
Applied to files:
src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java
🧬 Code graph analysis (3)
src/main/java/com/example/solidconnection/siteuser/domain/UserBan.java (2)
src/main/java/com/example/solidconnection/report/domain/Report.java (1)
Getter(17-57)src/main/java/com/example/solidconnection/siteuser/domain/SiteUser.java (1)
Getter(21-133)
src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixtureBuilder.java (6)
src/main/java/com/example/solidconnection/admin/controller/AdminUserBanController.java (1)
RequiredArgsConstructor(18-42)src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixture.java (1)
TestComponent(10-37)src/test/java/com/example/solidconnection/report/fixture/ReportFixture.java (1)
TestComponent(8-22)src/test/java/com/example/solidconnection/report/fixture/ReportFixtureBuilder.java (1)
TestComponent(10-61)src/test/java/com/example/solidconnection/siteuser/fixture/SiteUserFixtureBuilder.java (1)
TestComponent(13-80)src/test/java/com/example/solidconnection/siteuser/fixture/SiteUserFixture.java (1)
TestComponent(10-111)
src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java (2)
src/main/java/com/example/solidconnection/admin/controller/AdminUserBanController.java (1)
RequiredArgsConstructor(18-42)src/main/java/com/example/solidconnection/report/service/ReportService.java (1)
Service(22-97)
⏰ 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 (9)
src/main/java/com/example/solidconnection/report/repository/ReportRepository.java (1)
9-12:existsByReportedId추가는 깔끔합니다.
- 다만 이 쿼리는 트래픽이 쌓이면 인덱스 유무가 체감 성능을 갈라서요.
report(reported_id)혹은 실제 조회 패턴에 맞춘 복합 인덱스(예:(reported_id, target_type))가 마이그레이션에 포함돼 있는지 확인만 부탁드립니다.src/main/java/com/example/solidconnection/admin/controller/AdminUserBanController.java (1)
1-42: 엔드포인트 구성은 단정하고 좋습니다(ban/unban 흐름이 한눈에 들어옵니다).
POST /admin/users/{user-id}/ban,PATCH /admin/users/{user-id}/unban로 역할이 명확해요.- 다만 이 컨트롤러만 보면 “관리자만 접근”이 코드에 드러나지 않아서, 보안이 전역(Security config/필터/인터셉터)에서 강제되는지 확인만 부탁드립니다.
- 응답은 현재 200 OK도 충분히 합리적이고, 취향에 따라 204 No Content로 정리하는 것도 선택지입니다.
src/main/java/com/example/solidconnection/chat/repository/ChatMessageRepository.java (1)
52-67: 현재 쿼리가 올바릅니다 — 제시된 "수정안"은 오히려 더 심각한 오류입니다.스키마와 엔티티 정의를 재확인한 결과:
sender_id의미
ChatMessage.senderId는chat_participant.id를 가리킴 (엔티티 주석: "// chat_participant의 id")- DB 마이그레이션에서
CONSTRAINT FK_CHAT_MESSAGE_SENDER_ID FOREIGN KEY (sender_id) REFERENCES chat_participant (id)명시현재 코드 검증 (정확함)
- 라인 57, 65:
cm.sender_id IN (SELECT cp.id FROM chat_participant cp WHERE cp.site_user_id = :siteUserId)- 이 패턴이 올바름 (chat_participant.id 도메인으로 조인 후 필터)
제안된 "수정안"의 문제 (불정확함)
cm.sender_id = :siteUserId(또는IN :siteUserIds)- chat_participant.id와 site_user_id를 직접 비교하는 것 (FK 관계 무시)
- 쿼리가 0건 반환 또는 의도와 다른 범위 필터링 가능성 높음
결론: 현재 코드는 올바른 설계이며, 검토가 필요한 부분은 없습니다.
Likely an incorrect or invalid review comment.
src/main/java/com/example/solidconnection/siteuser/domain/UserBan.java (1)
21-61: LGTM!엔티티 설계가 잘 되어 있습니다. 주요 특징:
BaseEntity상속으로 생성/수정 시간 자동 관리- UTC 시간대 일관 사용
manuallyUnban메서드를 통한 명확한 상태 변경 로직src/main/java/com/example/solidconnection/siteuser/repository/UserBanRepository.java (1)
12-19: LGTM!리포지토리 메서드들이 적절하게 정의되어 있습니다:
- Spring Data 네이밍 컨벤션을 따르는 파생 쿼리 메서드
- JPQL을 사용한 커스텀 쿼리로 만료된 차단 사용자 ID 조회
src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixtureBuilder.java (1)
10-48: LGTM!기존 프로젝트의 픽스처 빌더 패턴(
ReportFixtureBuilder,SiteUserFixtureBuilder)과 일관성 있게 구현되어 있습니다:
- Fluent API 패턴 적용
@TestComponent어노테이션으로 테스트 컨텍스트 등록userBan()메서드로 새 빌더 인스턴스 생성src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java (3)
88-106:expireUserBans()메서드에서 조회와 업데이트 간 잠재적 불일치 가능성이 있습니다.현재 구현에서는:
- 먼저 만료된 사용자 ID 목록을 조회 (Line 93)
- 그 다음 벌크 업데이트 실행 (Line 99-101)
이 두 작업 사이에 새로운 차단이 생성되거나 수정될 경우, 조회된 목록과 실제 업데이트된 레코드가 불일치할 수 있습니다. 단, 스케줄러가 하루에 한 번 자정에만 실행되므로 실제 운영에서 문제가 될 가능성은 낮습니다.
더 엄격한 일관성이 필요하다면, 다음 방법을 고려해 주세요:
- 벌크 업데이트 쿼리가 영향받은 레코드 수를 반환하도록 수정
- 또는 단일 트랜잭션 내에서 SELECT FOR UPDATE 사용
35-45: LGTM!차단 로직이 명확하게 구현되어 있습니다:
- 사용자 존재 여부 확인
- 중복 차단 방지 검증
- 신고 내역 존재 확인
- 상태 업데이트 및 콘텐츠 soft delete 처리
71-80: 차단 해제 시 사용자 상태를REPORTED로 변경하는 로직이 의도된 것인지 확인해 주세요.현재 구현에서는 차단 해제 시 사용자 상태가
REPORTED로 설정됩니다. 이는 "차단 → 해제 후에도 여전히 신고된 상태 유지"라는 비즈니스 요구사항에 맞는 것으로 보이지만,ACTIVE로 복원해야 하는 경우도 있는지 확인이 필요합니다.
- 코드리뷰 반영
JAEHEE25
force-pushed
the
feature/590-admin-user-block
branch
from
17cd5df to
c5fe54e
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In
@src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java:
- Around line 88-106: The expireUserBans() method is swallowing all exceptions
inside a try/catch which can prevent the @Transactional rollback and cause
partial commits; remove the broad try/catch or rethrow the exception after
logging so Spring can trigger transaction rollback. Specifically, update
expireUserBans() to let exceptions propagate (or if you must log, catch
Exception e, call log.error(..., e) then throw e) so that
userBanRepository.bulkExpireUserBans(...),
siteUserRepository.bulkUpdateUserStatus(...), and
bulkUpdateReportedContentIsDeleted(...) run inside a transactional boundary that
will roll back on error.
- Around line 59-62: The current updateReportedContentIsDeleted method (calling
postRepository.updateReportedPostsIsDeleted and
chatMessageRepository.updateReportedChatMessagesIsDeleted) toggles isDeleted for
all reported content without tracking why it was deleted, risking restoring
items deleted for other reasons; modify the domain by adding a deletion-reason
field (e.g., deletedReason or deletedByBan) to Post and ChatMessage (or track
ban deletions in Report), and change the repository update queries to only
clear/delete state when deletedByBan/ deletedReason == 'BAN' (or set
deletedByBan flag on ban) so updateReportedContentIsDeleted only affects items
marked deleted due to the ban.
🧹 Nitpick comments (4)
src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixtureBuilder.java (1)
45-48: 필수 필드 검증 추가를 고려해보세요.
create()메서드가 필드 값 검증 없이 바로 엔티티를 생성하므로, 빌더를 잘못 사용하면NullPointerException이 발생할 수 있습니다. 테스트 실패 원인을 명확히 하기 위해 필수 필드 검증을 추가하는 것을 권장합니다.♻️ 필드 검증 추가 예시
public UserBan create() { + if (bannedUserId == null || bannedBy == null || duration == null || expiredAt == null) { + throw new IllegalStateException("모든 필수 필드를 설정해야 합니다."); + } UserBan userBan = new UserBan(bannedUserId, bannedBy, duration, expiredAt); return userBanRepository.save(userBan); }또는 기본값 제공 방식:
public UserBan create() { + Long actualBannedUserId = bannedUserId != null ? bannedUserId : 1L; + Long actualBannedBy = bannedBy != null ? bannedBy : 1L; + UserBanDuration actualDuration = duration != null ? duration : UserBanDuration.ONE_DAY; + ZonedDateTime actualExpiredAt = expiredAt != null ? expiredAt : ZonedDateTime.now().plusDays(1); - UserBan userBan = new UserBan(bannedUserId, bannedBy, duration, expiredAt); + UserBan userBan = new UserBan(actualBannedUserId, actualBannedBy, actualDuration, actualExpiredAt); return userBanRepository.save(userBan); }참고:
SiteUserFixtureBuilder는userStatus에 대해 기본값을 제공하는 패턴을 사용하고 있습니다.src/main/java/com/example/solidconnection/common/interceptor/BannedUserInterceptor.java (1)
27-34: LGTM! 차단된 사용자 접근 제어 로직이 잘 구현되었습니다.
- 인증되지 않은 요청이나 차단되지 않은 사용자는 정상적으로 통과됩니다.
- 차단된 사용자는 명확한 에러 코드와 함께 거부됩니다.
선택적으로 Java 16+ 패턴 매칭을 사용하면 코드가 더 간결해질 수 있습니다:
♻️ 패턴 매칭을 활용한 리팩토링 제안
- if (authentication != null && authentication.getPrincipal() instanceof SiteUserDetails) { - SiteUserDetails userDetails = (SiteUserDetails) authentication.getPrincipal(); - SiteUser siteUser = userDetails.getSiteUser(); - - if (siteUser.getUserStatus() == UserStatus.BANNED) { + if (authentication != null && authentication.getPrincipal() instanceof SiteUserDetails userDetails) { + if (userDetails.getSiteUser().getUserStatus() == UserStatus.BANNED) { throw new CustomException(BANNED_USER_ACCESS_DENIED); } }src/main/java/com/example/solidconnection/siteuser/repository/SiteUserRepository.java (1)
27-29:@Modifying어노테이션에clearAutomatically = true옵션 추가를 권장합니다.다른 Repository들(
PostRepository,ChatMessageRepository)에서는@Modifying(clearAutomatically = true, flushAutomatically = true)를 사용하고 있습니다. 동일한 트랜잭션 내에서 업데이트된 엔티티를 다시 조회할 때 영속성 컨텍스트의 stale 데이터 문제를 방지하려면 일관성 있게 옵션을 추가하는 것이 좋습니다.♻️ 제안된 수정
- @Modifying + @Modifying(clearAutomatically = true, flushAutomatically = true) @Query("UPDATE SiteUser u SET u.userStatus = :status WHERE u.id IN :userIds") void bulkUpdateUserStatus(@Param("userIds") List<Long> userIds, @Param("status") UserStatus status);src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java (1)
64-69: 만료 시각 계산과 ‘매일 자정 만료 처리’ 스케줄이 같은 기준인지 확인이 필요해요.
- 현재 동작
expiredAt = now(UTC) + days- 배치 실행 =
cron: 0 0 0 * * *(zone 미지정)- 엇갈릴 수 있는 부분
- “1일”이 ‘정확히 24시간’인지, ‘다음 자정’인지에 따라
expiredAt계산이 달라져야 해요.@Scheduled는 기본적으로 서버 타임존을 타기 쉬워서, UTC로 저장/비교하는 지금 코드와 자정 기준이 어긋날 수 있어요.- 제안
- (a) 스케줄 zone을 명시하고, (b) ‘1일/3일/7일’의 정의를 자정 기준으로 맞출지/시간 기준으로 맞출지 결정해서
expiredAt계산을 일치시키는 게 좋아요.변경 예시(스케줄 zone 명시)
- @Scheduled(cron = "0 0 0 * * *") + @Scheduled(cron = "0 0 0 * * *", zone = "UTC") public void expireUserBans() { try { ZonedDateTime now = ZonedDateTime.now(UTC); List<Long> expiredUserIds = userBanRepository.findExpiredBannedUserIds(now);Also applies to: 89-90
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/main/java/com/example/solidconnection/admin/controller/AdminUserBanController.javasrc/main/java/com/example/solidconnection/admin/dto/UserBanRequest.javasrc/main/java/com/example/solidconnection/admin/service/AdminUserBanService.javasrc/main/java/com/example/solidconnection/chat/repository/ChatMessageRepository.javasrc/main/java/com/example/solidconnection/common/interceptor/BannedUserInterceptor.javasrc/main/java/com/example/solidconnection/community/post/repository/PostRepository.javasrc/main/java/com/example/solidconnection/report/repository/ReportRepository.javasrc/main/java/com/example/solidconnection/siteuser/domain/UserBan.javasrc/main/java/com/example/solidconnection/siteuser/repository/SiteUserRepository.javasrc/main/java/com/example/solidconnection/siteuser/repository/UserBanRepository.javasrc/main/resources/db/migration/V40__create_user_ban_table.sqlsrc/test/java/com/example/solidconnection/admin/service/AdminUserBanServiceTest.javasrc/test/java/com/example/solidconnection/common/interceptor/BannedUserInterceptorTest.javasrc/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixture.javasrc/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixtureBuilder.java
🚧 Files skipped from review as they are similar to previous changes (6)
- src/test/java/com/example/solidconnection/common/interceptor/BannedUserInterceptorTest.java
- src/main/java/com/example/solidconnection/admin/dto/UserBanRequest.java
- src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixture.java
- src/main/resources/db/migration/V40__create_user_ban_table.sql
- src/main/java/com/example/solidconnection/siteuser/repository/UserBanRepository.java
- src/main/java/com/example/solidconnection/siteuser/domain/UserBan.java
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-24T16:30:16.681Z
Learnt from: nayonsoso
Repo: solid-connection/solid-connect-server PR: 479
File: src/main/java/com/example/solidconnection/auth/controller/AuthController.java:13-16
Timestamp: 2025-08-24T16:30:16.681Z
Learning: When reviewing Java import statements, always verify that both classes are not in the same package before suggesting import additions. Classes in the same package don't need explicit imports in Java.
Applied to files:
src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java
📚 Learning: 2025-07-28T03:14:33.849Z
Learnt from: whqtker
Repo: solid-connection/solid-connect-server PR: 408
File: src/main/java/com/example/solidconnection/chat/service/ChatService.java:46-54
Timestamp: 2025-07-28T03:14:33.849Z
Learning: 페치 조인을 사용한 N+1 문제 해결에 대해 사용자 whqtker는 JPQL 쿼리 접근법을 선호하며, SELECT 쿼리의 경우 영속성 컨텍스트 간 불일치 문제를 고려하지 않아도 된다고 인식하고 있습니다.
Applied to files:
src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java
📚 Learning: 2025-07-29T17:26:08.811Z
Learnt from: whqtker
Repo: solid-connection/solid-connect-server PR: 408
File: src/main/java/com/example/solidconnection/chat/repository/ChatReadStatusRepository.java:11-17
Timestamp: 2025-07-29T17:26:08.811Z
Learning: 사용자 whqtker는 JPA save() 메서드를 사용한 upsert 연산에서 레이스 컨디션 문제를 우려하고 있으며, 특히 채팅 읽음 상태 업데이트와 같이 동시성이 중요한 연산에서는 네이티브 쿼리의 원자적 연산이 더 안전하다고 인식하고 있습니다.
Applied to files:
src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java
🧬 Code graph analysis (3)
src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java (2)
src/main/java/com/example/solidconnection/admin/controller/AdminUserBanController.java (1)
RequiredArgsConstructor(18-42)src/main/java/com/example/solidconnection/report/service/ReportService.java (1)
Service(22-97)
src/main/java/com/example/solidconnection/common/interceptor/BannedUserInterceptor.java (1)
src/main/java/com/example/solidconnection/security/userdetails/SiteUserDetails.java (1)
SiteUserDetails(9-56)
src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixtureBuilder.java (2)
src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixture.java (1)
TestComponent(10-37)src/test/java/com/example/solidconnection/siteuser/fixture/SiteUserFixtureBuilder.java (1)
TestComponent(13-80)
⏰ 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 (12)
src/test/java/com/example/solidconnection/siteuser/fixture/UserBanFixtureBuilder.java (3)
10-14: 테스트 픽스처 빌더 구조가 잘 설계되었습니다.
SiteUserFixtureBuilder와 동일한 패턴을 따르고 있어 일관성이 유지됩니다.@TestComponent와@RequiredArgsConstructor를 활용한 의존성 주입 방식도 적절합니다.
21-23: 빌더 초기화 패턴이 프로젝트 컨벤션을 준수합니다.
SiteUserFixtureBuilder.siteUser()와 동일한 방식으로 새 인스턴스를 생성하여 테스트마다 깨끗한 상태를 보장합니다. 주입받은 repository를 공유하면서도 각 빌더는 독립적인 상태를 유지할 수 있는 합리적인 설계입니다.
25-43: 빌더 메서드들이 유창한 API를 제공합니다.메서드 체이닝을 지원하여 가독성 높은 테스트 데이터 구성이 가능합니다. 필드 설정이 명확하고 직관적입니다.
src/main/java/com/example/solidconnection/report/repository/ReportRepository.java (1)
10-11: LGTM! 간결하고 명확한 메서드 추가입니다.
existsByReportedId메서드가 Spring Data JPA 명명 규칙을 잘 따르고 있습니다.
- 차단 전 신고 여부를 확인하는 용도로 적절하게 활용됩니다.
src/main/java/com/example/solidconnection/chat/repository/ChatMessageRepository.java (1)
53-67: LGTM! 신고된 채팅 메시지의 soft delete 처리를 위한 벌크 업데이트가 잘 구현되었습니다.
@Modifying어노테이션의clearAutomatically와flushAutomatically설정으로 영속성 컨텍스트 일관성이 보장됩니다.- 단일 사용자용과 다중 사용자용 메서드가 일관된 패턴으로 구현되어 있습니다.
chat_participant테이블과의 조인을 통해site_user_id를 올바르게 매핑하고 있습니다.src/main/java/com/example/solidconnection/community/post/repository/PostRepository.java (1)
53-67: LGTM! 신고된 게시글의 soft delete 처리가 ChatMessageRepository와 일관된 패턴으로 구현되었습니다.
post테이블에서 직접site_user_id를 필터링하는 것이 올바릅니다.- 신고된 콘텐츠(
report테이블 조인)만 대상으로 하여 정확한 범위 제어가 됩니다.- 차단/해제 시 콘텐츠 삭제/복구 요구사항을 충족합니다.
src/test/java/com/example/solidconnection/admin/service/AdminUserBanServiceTest.java (4)
68-81: LGTM! 테스트 픽스처 설정이 잘 구성되어 있습니다.
@BeforeEach에서 각 테스트에 필요한 공통 데이터를 명확하게 준비합니다.- 관리자, 신고된 사용자, 신고자, 신고된 게시글 등 시나리오에 필요한 모든 엔티티가 초기화됩니다.
- 한글 픽스처 메서드명으로 가독성이 좋습니다.
83-124: LGTM! 차단 시나리오 테스트가 충실하게 작성되었습니다.
- 정상 차단: 신고된 사용자가 차단되면 상태가
BANNED로 변경됨을 검증- 중복 차단 예외: 이미 차단된 사용자를 다시 차단하면
ALREADY_BANNED_USER예외 발생- 신고 없는 사용자 예외: 신고 기록이 없는 사용자 차단 시
REPORT_NOT_FOUND예외 발생예외 케이스까지 꼼꼼하게 테스트하고 있어서 좋습니다.
126-190: LGTM! 차단 해제 시나리오 테스트가 잘 작성되었습니다.
- 수동 해제: 차단 해제 후 상태가
REPORTED로 복구됨을 검증- 해제 메타데이터:
unbannedBy,unbannedAt,isExpired필드가 올바르게 저장됨을 검증- 비차단 사용자 예외: 차단되지 않은 사용자 해제 시
NOT_BANNED_USER예외 발생- 만료된 차단 예외: 이미 만료된 차단은 재해제 불가
assertAll을 사용하여 여러 조건을 한번에 검증하는 것이 좋습니다.
196-218:updateUserStatus()호출 후save()를 추가할 필요는 없습니다.
expireUserBans()서비스 메서드는 Dirty Checking에 의존하지 않습니다. Line 100에서 사용되는bulkUpdateUserStatus()는@ModifyingJPA 쿼리로 데이터베이스에 직접 UPDATE를 실행합니다. 따라서 테스트에서 메모리상 객체의 상태 변경은 서비스 동작에 영향을 주지 않습니다. 서비스가 만료된 사용자 ID를 조회한 후 데이터베이스를 직접 업데이트하므로, 테스트가 재조회한 객체는 벌크 업데이트로 변경된 정확한 상태를 반영합니다.src/main/java/com/example/solidconnection/admin/controller/AdminUserBanController.java (1)
18-42: 깔끔하고 완성도 높은 관리자 차단 API 컨트롤러 구현입니다! ✓다음과 같은 점들이 잘 설계되었습니다:
RESTful 설계 원칙 준수
POST /{user-id}/ban: 차단 기록을 생성하는 새로운 리소스PATCH /{user-id}/unban: 사용자의 차단 상태를 부분적으로 수정견고한 유효성 검사 및 인증
UserBanRequest에서@NotNull로 차단 기간 필드 검증@AuthorizedUser로 관리자 인증 확보SecurityConfiguration에서/admin/**경로에ADMIN역할 요구 적용 확인 ✓책임 분리가 잘 된 아키텍처
- 비즈니스 로직을
AdminUserBanService로 위임- 서비스 레이어에서 사용자 존재 여부, 이미 차단된 사용자, 신고 기록 존재 여부 등을 검증
- 트랜잭션 처리 및 스케줄된 만료 처리가 제대로 구현됨
src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java (1)
71-80: 해제 시 유저 상태를REPORTED로 고정하는 게 의도인지 한 번만 확인해 주세요.
- 현재 동작
- 수동 해제 시
UserStatus.REPORTED로 업데이트- 확인 포인트
- 요구사항이 “차단 해제 시 정상 이용”까지면
ACTIVE로 복귀가 더 자연스러울 수도 있고,- 반대로 “신고 이력이 있으면 REPORTED 유지”가 정책이면 지금 구현이 딱 맞아요.
| @Transactional | ||
| public void banUser(long userId, long adminId, UserBanRequest request) { | ||
| SiteUser user = siteUserRepository.findById(userId) | ||
| .orElseThrow(() -> new CustomException(ErrorCode.USER_NOT_FOUND)); | ||
| validateNotAlreadyBanned(userId); | ||
| validateReportExists(userId); | ||
|
|
||
| user.updateUserStatus(UserStatus.BANNED); | ||
| updateReportedContentIsDeleted(userId, true); | ||
| createUserBan(userId, adminId, request); | ||
| } |
There was a problem hiding this comment.
차단 처리 순서/원자성(동시성) 보강이 필요해요.
- (동시성)
validateNotAlreadyBanned()→save()사이 TOCTOU- 동시에 두 번 호출되면 “둘 다 미차단”으로 통과하고 ban 레코드가 중복 생성될 수 있어요.
- (정합성)
SiteUser를 먼저 로드하는데, 이미 차단 상태면 불필요 조회validateNotAlreadyBanned(userId)를 먼저 두면 DB 호출 1회 줄일 수 있어요(성능/명확성).
| private void validateNotAlreadyBanned(long userId) { | ||
| if (userBanRepository.existsByBannedUserIdAndIsExpiredFalseAndExpiredAtAfter(userId, ZonedDateTime.now(UTC))) { | ||
| throw new CustomException(ErrorCode.ALREADY_BANNED_USER); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "AdminUserBanService.java" -type fRepository: solid-connection/solid-connect-server
Length of output: 163
🏁 Script executed:
cat -n ./src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.javaRepository: solid-connection/solid-connect-server
Length of output: 5608
🏁 Script executed:
find . -name "UserBan.java" -type f | head -5Repository: solid-connection/solid-connect-server
Length of output: 153
🏁 Script executed:
cat -n ./src/main/java/com/example/solidconnection/siteuser/domain/UserBan.javaRepository: solid-connection/solid-connect-server
Length of output: 2262
🏁 Script executed:
find . -name "UserBanRepository.java" -type fRepository: solid-connection/solid-connect-server
Length of output: 167
🏁 Script executed:
cat -n ./src/main/java/com/example/solidconnection/siteuser/repository/UserBanRepository.javaRepository: solid-connection/solid-connect-server
Length of output: 1365
시간 기준 만료와 배치 실행 사이의 시간차로 활성 차단 검증이 누락될 수 있습니다.
현재 로직의 동작 방식:
validateNotAlreadyBanned()(47-51줄)와findActiveBan()(82-86줄)은 모두isExpired=false AND expiredAt > now조건으로 활성 차단 여부를 판단합니다.- 배치 작업은 매일 자정(0 0 0 * * *)에만
isExpired를 true로 업데이트합니다.
발생 가능한 문제:
- 중복 차단 생성:
expiredAt ≤ now인데 배치가 아직 실행되지 않으면, 쿼리는 그 차단을 찾지 못하므로banUser()는 새로운 차단을 또 생성할 수 있습니다. - 수동 해제 불가: 같은 시간 구간에
unbanUser()호출 시findActiveBan()이 레코드를 찾지 못해NOT_BANNED_USER예외를 던지므로 관리자의 수동 해제가 막힙니다.
개선 방향:
- 차단 검증/조회 경로에서 시간 기준으로 만료된 차단을 즉시 마킹하거나,
- DB 레벨에서 사용자당 활성 차단 1개 제약 조건과 원자적 업데이트를 결합하여 중복을 구조적으로 방지하는 것이 안전합니다.
| private void updateReportedContentIsDeleted(long userId, boolean isDeleted) { | ||
| postRepository.updateReportedPostsIsDeleted(userId, isDeleted); | ||
| chatMessageRepository.updateReportedChatMessagesIsDeleted(userId, isDeleted); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# AdminUserBanService 파일 내용 확인
wc -l src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.javaRepository: solid-connection/solid-connect-server
Length of output: 218
🏁 Script executed:
# 업데이트 쿼리 메서드 위치 찾기
rg -n "updateReportedPostsIsDeleted|updateReportedChatMessagesIsDeleted" --type javaRepository: solid-connection/solid-connect-server
Length of output: 824
🏁 Script executed:
# Post 및 ChatMessage 저장소의 실제 구현 확인
find src -name "*Repository.java" -type f | grep -E "(Post|ChatMessage)" | head -5Repository: solid-connection/solid-connect-server
Length of output: 438
🏁 Script executed:
# MyBatis mapper XML 파일 찾기
find src -name "*.xml" -type f | head -20Repository: solid-connection/solid-connect-server
Length of output: 118
🏁 Script executed:
# PostRepository 및 ChatMessageRepository의 SQL 매퍼 찾기
find src/main/resources -name "*Mapper.xml" -o -name "*mapper.xml" | grep -i -E "(post|chat)"Repository: solid-connection/solid-connect-server
Length of output: 63
🏁 Script executed:
# 전체 resources 디렉토리 구조 확인
find src/main/resources -type f | head -30Repository: solid-connection/solid-connect-server
Length of output: 2117
🏁 Script executed:
# PostRepository 전체 내용 확인
cat -n src/main/java/com/example/solidconnection/community/post/repository/PostRepository.javaRepository: solid-connection/solid-connect-server
Length of output: 3956
🏁 Script executed:
# ChatMessageRepository 전체 내용 확인
cat -n src/main/java/com/example/solidconnection/chat/repository/ChatMessageRepository.javaRepository: solid-connection/solid-connect-server
Length of output: 3701
🏁 Script executed:
# Post 엔티티 확인 (deletion reason 관련 필드 확인)
find src -name "Post.java" -type f | grep -v TestRepository: solid-connection/solid-connect-server
Length of output: 154
🏁 Script executed:
# Post 엔티티 확인 - 삭제 이유 필드 존재 여부
cat -n src/main/java/com/example/solidconnection/community/post/domain/Post.javaRepository: solid-connection/solid-connect-server
Length of output: 3461
🏁 Script executed:
# ChatMessage 엔티티 확인
find src -name "ChatMessage.java" -path "*/domain/*" | grep -v TestRepository: solid-connection/solid-connect-server
Length of output: 151
🏁 Script executed:
# Report 테이블/엔티티 확인
find src -name "Report.java" -path "*/domain/*" | grep -v TestRepository: solid-connection/solid-connect-server
Length of output: 148
🏁 Script executed:
# AdminUserBanService의 ban/unban 로직 확인
cat -n src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java | head -150Repository: solid-connection/solid-connect-server
Length of output: 5608
🏁 Script executed:
# Report 엔티티 확인
cat -n src/main/java/com/example/solidconnection/report/domain/Report.javaRepository: solid-connection/solid-connect-server
Length of output: 2162
🏁 Script executed:
# ChatMessage 엔티티 확인
cat -n src/main/java/com/example/solidconnection/chat/domain/ChatMessage.javaRepository: solid-connection/solid-connect-server
Length of output: 2262
🏁 Script executed:
# 사용자가 자신의 콘텐츠를 삭제하는 다른 경로 확인
rg -n "isDeleted.*true" src/main/java/com/example/solidconnection/community/post/ --type java -A2Repository: solid-connection/solid-connect-server
Length of output: 63
🏁 Script executed:
# 사용자가 자신의 채팅을 삭제하는 다른 경로 확인
rg -n "isDeleted.*true" src/main/java/com/example/solidconnection/chat/ --type java -A2Repository: solid-connection/solid-connect-server
Length of output: 63
🏁 Script executed:
# Post 삭제 메서드 찾기
rg -n "setDeleted|delete\(" src/main/java/com/example/solidconnection/community/post/ --type java -A3Repository: solid-connection/solid-connect-server
Length of output: 63
🏁 Script executed:
# ChatMessage 삭제 메서드 찾기
rg -n "setDeleted|delete\(" src/main/java/com/example/solidconnection/chat/ --type java -A3Repository: solid-connection/solid-connect-server
Length of output: 63
🏁 Script executed:
# Post를 삭제하는 모든 서비스 로직 확인
find src/main/java/com/example/solidconnection -name "*Service.java" -exec grep -l "PostRepository\|updateReportedPostsIsDeleted" {} \;Repository: solid-connection/solid-connect-server
Length of output: 680
🏁 Script executed:
# 삭제 관련 메서드와 호출 흐름 확인
rg -n "updateReportedPostsIsDeleted|updateReportedChatMessagesIsDeleted|bulkUpdateReportedPostsIsDeleted|bulkUpdateReportedChatMessagesIsDeleted" src/main/java --type java -B5 -A5Repository: solid-connection/solid-connect-server
Length of output: 7857
신고 콘텐츠의 삭제 이유를 추적하지 않아, 차단 해제 시 다른 사유로 삭제된 콘텐츠까지 복구될 위험이 있습니다.
현재 코드 분석 결과:
-
엔티티 레벨에서의 문제
- Post, ChatMessage 엔티티가
isDeleted불린 플래그만 보유 - 삭제 사유(차단, 사용자 삭제, 정책 삭제 등)를 구분하는 필드 없음
- Post, ChatMessage 엔티티가
-
리스크 시나리오
- 사용자가 게시한 콘텐츠가 신고 당함 → Report 생성
- 관리자가 해당 사용자 차단 →
updateReportedPostsIsDeleted(userId, true)실행 - 나중에 차단 해제 →
updateReportedPostsIsDeleted(userId, false)실행 - 만약 해당 콘텐츠가 차단 전에 정책 위반으로 관리자가 삭제했다면, 차단 해제 시 정책 삭제된 콘텐츠까지 복구됨
-
현재 쿼리의 한계
- 저장소 메서드는
p.id IN (SELECT r.target_id FROM report WHERE target_type='POST')조건으로 필터링 - Report 존재 여부는 확인하나, 삭제된 "이유"는 확인하지 않음
- 저장소 메서드는
-
개선 방향
- 최소한
deletedReason,deletedByBan같은 필드 추가하여 차단으로 삭제된 콘텐츠만 복구 - 또는 업데이트 쿼리에
WHERE is_deleted = true AND deleted_reason = 'BAN'같은 조건 추가 - Report 테이블에 ban 상태 추적하는 방식도 검토 가능
- 최소한
🤖 Prompt for AI Agents
In
@src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java
around lines 59 - 62, The current updateReportedContentIsDeleted method (calling
postRepository.updateReportedPostsIsDeleted and
chatMessageRepository.updateReportedChatMessagesIsDeleted) toggles isDeleted for
all reported content without tracking why it was deleted, risking restoring
items deleted for other reasons; modify the domain by adding a deletion-reason
field (e.g., deletedReason or deletedByBan) to Post and ChatMessage (or track
ban deletions in Report), and change the repository update queries to only
clear/delete state when deletedByBan/ deletedReason == 'BAN' (or set
deletedByBan flag on ban) so updateReportedContentIsDeleted only affects items
marked deleted due to the ban.
| @Transactional | ||
| @Scheduled(cron = "0 0 0 * * *") | ||
| public void expireUserBans() { | ||
| try { | ||
| ZonedDateTime now = ZonedDateTime.now(UTC); | ||
| List<Long> expiredUserIds = userBanRepository.findExpiredBannedUserIds(now); | ||
|
|
||
| if (expiredUserIds.isEmpty()) { | ||
| return; | ||
| } | ||
|
|
||
| userBanRepository.bulkExpireUserBans(now); | ||
| siteUserRepository.bulkUpdateUserStatus(expiredUserIds, UserStatus.REPORTED); | ||
| bulkUpdateReportedContentIsDeleted(expiredUserIds); | ||
| log.info("Finished processing expired blocks:: userIds={}", expiredUserIds); | ||
| } catch (Exception e) { | ||
| log.error("Failed to process expired blocks", e); | ||
| } | ||
| } |
There was a problem hiding this comment.
@Transactional 메서드에서 예외를 삼키면 ‘부분 커밋’이 나올 수 있어요.
- 현재 구조
@Transactional+try/catch(Exception)로 전체를 감싸고 로깅만 함
- 문제
- 중간에 예외가 나도 catch로 먹어버리면 트랜잭션이 롤백되지 않고 “어디까지는 반영, 어디까지는 미반영”이 될 수 있어요(특히 bulk update 조합에서 치명적).
- 추천
- 예외는 밖으로 던져서 스프링이 롤백하게 두는 편이 안전해요(스케줄러는 다음 주기에 다시 돕니다).
- 로깅이 필요하면 catch 후
throw하거나, rollback-only 설정을 명시하세요.
변경 예시(예외 삼키지 않기)
@Transactional
@Scheduled(cron = "0 0 0 * * *")
public void expireUserBans() {
- try {
- ZonedDateTime now = ZonedDateTime.now(UTC);
- List<Long> expiredUserIds = userBanRepository.findExpiredBannedUserIds(now);
+ ZonedDateTime now = ZonedDateTime.now(UTC);
+ List<Long> expiredUserIds = userBanRepository.findExpiredBannedUserIds(now);
- if (expiredUserIds.isEmpty()) {
- return;
- }
+ if (expiredUserIds.isEmpty()) {
+ return;
+ }
- userBanRepository.bulkExpireUserBans(now);
- siteUserRepository.bulkUpdateUserStatus(expiredUserIds, UserStatus.REPORTED);
- bulkUpdateReportedContentIsDeleted(expiredUserIds);
- log.info("Finished processing expired blocks:: userIds={}", expiredUserIds);
- } catch (Exception e) {
- log.error("Failed to process expired blocks", e);
- }
+ userBanRepository.bulkExpireUserBans(now);
+ siteUserRepository.bulkUpdateUserStatus(expiredUserIds, UserStatus.REPORTED);
+ bulkUpdateReportedContentIsDeleted(expiredUserIds);
+ log.info("Finished processing expired bans:: userIds={}", expiredUserIds);
}🤖 Prompt for AI Agents
In
@src/main/java/com/example/solidconnection/admin/service/AdminUserBanService.java
around lines 88 - 106, The expireUserBans() method is swallowing all exceptions
inside a try/catch which can prevent the @Transactional rollback and cause
partial commits; remove the broad try/catch or rethrow the exception after
logging so Spring can trigger transaction rollback. Specifically, update
expireUserBans() to let exceptions propagate (or if you must log, catch
Exception e, call log.error(..., e) then throw e) so that
userBanRepository.bulkExpireUserBans(...),
siteUserRepository.bulkUpdateUserStatus(...), and
bulkUpdateReportedContentIsDeleted(...) run inside a transactional boundary that
will roll back on error.
4 participants
관련 이슈
작업 내용
어드민 유저 관리 페이지에서 사용할 차단/차단 해제/차단 만료 기능을 구현했습니다.
차단
차단 해제
차단 만료
특이 사항
리뷰 요구사항 (선택)