-
Notifications
You must be signed in to change notification settings - Fork 0
[fix] 팔로잉 동시성 이슈 문제 2차 해결시도 #342
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Walkthrough팔로잉 동시성 문제 해결을 위해 비관적 잠금, 트랜잭션 아웃박스 패턴과 Spring Retry를 도입하고, 아웃박스 엔티티/포트/서비스/스케줄러/리스너 및 관련 DB 마이그레이션과 테스트·부하 테스트 스크립트를 추가했습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor Client as 클라이언트
participant UserService as UserFollowService
participant EventPub as ApplicationEventPublisher
participant TxListener as FollowingEventListener
participant Dispatch as FollowingDispatchService
participant OutboxDB as OutboxEvents(DB)
participant Scheduler as OutboxScheduler
participant Processor as FollowingOutboxProcessService
participant UserPort as UserCommandPort
participant UserDB as User(DB)
Client->>UserService: follow(userId, targetUserId)
UserService->>EventPub: publish(UserFollowedEvent)
EventPub->>TxListener: onUserFollowed (BEFORE_COMMIT)
TxListener->>Dispatch: handleUserFollow(event)
Dispatch->>OutboxDB: save(OutboxEvent{USER_FOLLOWED})
OutboxDB-->>Dispatch: stored
UserService-->>Client: 200 OK
loop 매 1초
Scheduler->>Processor: processPendingEvents()
Processor->>OutboxDB: findTop1000ByOutboxStatus(PENDING)
OutboxDB-->>Processor: [events]
alt event = USER_FOLLOWED
Processor->>UserPort: findByIdWithLock(targetUserId) note right: PESSIMISTIC_WRITE
UserPort->>UserDB: SELECT ... FOR UPDATE
UserDB-->>UserPort: locked user
Processor->>UserDB: update follower_count
Processor->>OutboxDB: markAsProcessed(event)
else 처리 실패
Processor->>OutboxDB: markAsFailed(event)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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 |
Test Results488 tests 488 ✅ 44s ⏱️ Results for commit 5d979d7. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 10
🧹 Nitpick comments (21)
loadtest/follow_change_state_load_test.js (1)
5-5: 환경 변수를 통한 BASE_URL 설정 고려로컬 환경 외에도 스테이징 등 다른 환경에서 테스트할 경우를 대비해 환경 변수로 설정하면 유연성이 높아집니다.
🔎 제안 수정
-const BASE_URL = 'http://localhost:8080'; +const BASE_URL = __ENV.BASE_URL || 'http://localhost:8080';loadtest/follow_change_toggle_load_test.js (1)
95-99: k6 VU 격리에 대한 설명 추가 권장
isFollowing변수가 모듈 레벨에 선언되어 있지만, k6에서는 각 VU가 독립된 JS 런타임에서 실행되므로 VU별로 개별 상태가 유지됩니다. 다만, k6에 익숙하지 않은 개발자를 위해 이 동작을 명시하면 유지보수성이 향상됩니다.🔎 제안 수정
/** ===== 각 VU의 토글 상태 ===== + * (k6에서 각 VU는 독립된 JS 런타임에서 실행되므로, 모듈 레벨 변수도 VU별로 격리됩니다) * false = 아직 팔로우 안 한 상태로 가정 → 다음 요청은 follow(type:true) * true = 이미 팔로우 한 상태로 가정 → 다음 요청은 unfollow(type:false) */ let isFollowing = false;src/main/java/konkuk/thip/user/application/port/out/FollowingEventCommandPort.java (1)
3-6: 이벤트 발행 실패 시 동작에 대한 명세 추가 권장인터페이스 설계는 깔끔하고 명확합니다. 다만 이벤트 발행이 실패할 경우의 동작(예외 전파, 재시도, 무시 등)에 대한 JavaDoc이나 명세가 있으면 구현체와 호출자 간의 계약이 더 명확해질 것입니다.
JavaDoc 추가 예시
+/** + * 팔로잉 이벤트 발행을 담당하는 포트 인터페이스. + * 이벤트 발행 실패 시 예외를 전파하며, 호출자는 트랜잭션 내에서 처리해야 합니다. + */ public interface FollowingEventCommandPort { + /** + * 사용자 팔로우 이벤트를 발행합니다. + * @throws RuntimeException 이벤트 발행에 실패한 경우 + */ void publishUserFollowedEvent(Long userId, Long targetUserId); + + /** + * 사용자 언팔로우 이벤트를 발행합니다. + * @throws RuntimeException 이벤트 발행에 실패한 경우 + */ void publishUserUnfollowedEvent(Long userId, Long targetUserId); }src/test/java/konkuk/thip/user/concurrency/UserFollowConcurrencyTest.java (3)
36-47: 테스트 데이터 규모 적절성 검토500명의 동시 팔로우 요청은 상당히 높은 부하입니다. 테스트 실행 시간과 안정성을 고려할 때, 이 규모가 실제로 필요한지 검토가 필요합니다. 동시성 이슈를 재현하기에는 100-200명 정도로도 충분할 수 있습니다.
-final int followerCount = 500; +final int followerCount = 200; // 동시성 이슈 재현에 충분하며 테스트 실행 시간 단축
76-81: 예외 처리 시 상세 정보 누락AssertionError가 발생했을 때 단순히 400을 반환하기만 하고, 실패 원인을 로깅하거나 추적하지 않습니다. 테스트 디버깅 시 어떤 이유로 실패했는지 파악하기 어렵습니다.
실패 원인 로깅 추가
} catch (AssertionError e) { + System.err.println("Follow request failed for follower " + followerId + ": " + e.getMessage()); return 400;
111-114: 테스트 출력에 적절한 로깅 프레임워크 사용 권장
System.out.println대신 SLF4J 로거나 테스트 프레임워크의 리포팅 기능을 사용하는 것이 더 적절합니다.로거 사용 예시
클래스 상단에 로거 추가:
import org.slf4j.Logger; import org.slf4j.LoggerFactory; private static final Logger log = LoggerFactory.getLogger(UserFollowConcurrencyTest.class);출력문 변경:
-System.out.println("=== RESULT ==="); -System.out.println("OK responses : " + okCount); -System.out.println("followings rows : " + followingRows); -System.out.println("target user's follower_count : " + storedFollowerCount); +log.info("=== RESULT ==="); +log.info("OK responses : {}", okCount); +log.info("followings rows : {}", followingRows); +log.info("target user's follower_count : {}", storedFollowerCount);src/main/java/konkuk/thip/user/application/port/out/UserCommandPort.java (1)
12-12: 잠금 메서드에 대한 명세 추가 권장
findByIdWithLock메서드가 추가되었으나, 다음 사항들에 대한 명세가 없어 구현체와 호출자 간의 계약이 불명확합니다:
- 사용하는 잠금 모드 (PESSIMISTIC_WRITE, PESSIMISTIC_READ 등)
- 트랜잭션 컨텍스트 필수 여부
- 엔티티를 찾지 못한 경우의 동작 (예외 발생 or null 반환)
- 잠금 타임아웃 정책
JavaDoc 추가 예시
+ /** + * 사용자를 ID로 조회하며, 비관적 쓰기 잠금(PESSIMISTIC_WRITE)을 획득합니다. + * 다른 트랜잭션의 읽기/쓰기를 차단하여 동시성 제어를 보장합니다. + * + * @param userId 조회할 사용자 ID + * @return 조회된 사용자 도메인 객체 + * @throws UserNotFoundException 사용자를 찾지 못한 경우 + * @throws PersistenceTransactionRequiredException 트랜잭션 컨텍스트 외부에서 호출된 경우 + */ User findByIdWithLock(Long userId);src/main/java/konkuk/thip/outbox/application/port/in/FollowingOutboxProcessUseCase.java (1)
3-5: Outbox 처리 결과에 대한 피드백 메커니즘 고려현재
processFollowingOutboxEvents()메서드는 void를 반환하여 처리 결과에 대한 정보를 제공하지 않습니다. 스케줄러에서 호출될 때 다음 정보들이 유용할 수 있습니다:
- 처리된 이벤트 수
- 실패한 이벤트 수
- 처리 소요 시간
이는 모니터링과 디버깅에 도움이 됩니다.
처리 결과 DTO 반환 예시
public interface FollowingOutboxProcessUseCase { /** * 대기 중인 팔로잉 Outbox 이벤트를 처리합니다. * * @return 처리 결과 (성공/실패 수, 처리 시간 등) */ OutboxProcessResult processFollowingOutboxEvents(); } public record OutboxProcessResult( int processedCount, int failedCount, long processingTimeMs ) {}또는 로깅만으로 충분하다면 현재 구조를 유지하되, 구현체에서 처리 결과를 로깅하도록 보장하세요.
src/main/java/konkuk/thip/user/adapter/out/event/dto/FollowingEvent.java (1)
5-9: LGTM! 이벤트 DTO 구조가 적절합니다.Outbox 패턴을 위한 이벤트 정의가 명확합니다. 다만
@Builder는 필드가 2개뿐인 record에서는 실질적 이점이 적습니다. 직접 생성자 호출이 더 간결할 수 있으나, 일관성을 위해 유지해도 무방합니다.src/main/java/konkuk/thip/config/WorkerThreadConfig.java (1)
57-68: RejectedExecutionHandler 추가를 고려하세요.
schedulerAsyncExecutor에는CallerRunsPolicy가 설정되어 있지만,outboxAsyncExecutor에는 rejection policy가 없습니다. 큐(2000)가 가득 찬 상황에서 기본AbortPolicy는 예외를 발생시키므로, 이벤트 유실 방지를 위해 rejection policy 추가를 권장합니다.🔎 제안된 수정사항
@Bean(name = "outboxAsyncExecutor") public Executor outboxAsyncExecutor() { ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor(); executor.setCorePoolSize(4); // 아웃박스 처리 기본량 executor.setMaxPoolSize(8); executor.setQueueCapacity(2000); // 적당한 큐 크기 executor.setThreadNamePrefix("outbox-"); executor.setWaitForTasksToCompleteOnShutdown(true); executor.setAwaitTerminationSeconds(60); + executor.setRejectedExecutionHandler(new ThreadPoolExecutor.CallerRunsPolicy()); executor.initialize(); return executor; }src/main/java/konkuk/thip/outbox/application/port/in/FollowingDispatchUseCase.java (1)
1-8: 패키지 의존성 방향을 확인하세요.
application.port.in인터페이스가adapter.out.event.dto를 import하고 있습니다. 헥사고날 아키텍처에서 application 레이어(port)는 adapter 레이어에 의존하지 않는 것이 원칙입니다.FollowingEvent를application.dto또는domain패키지로 이동하는 것을 고려해 보세요.Based on learnings, buzz0331님이 과도한 엔지니어링을 피하는 것을 선호한다는 점을 고려하면, 현재 구조로도 동작에는 문제가 없으므로 향후 리팩토링 시 고려해도 됩니다.
src/main/java/konkuk/thip/outbox/application/port/out/OutboxEventPersistencePort.java (1)
3-11: 포트 인터페이스에서 JPA 엔티티 직접 사용헥사고날 아키텍처 원칙에 따르면, 포트 인터페이스는 인프라 계층의 JPA 엔티티(
OutboxEventJpaEntity)가 아닌 도메인 객체를 사용하는 것이 이상적입니다. 현재 구조에서는 애플리케이션 계층이 인프라 계층에 의존하게 됩니다.다만, 학습된 내용에 따르면 실용적인 해결책을 선호하시는 것으로 보이므로, 현재 단일 서버 환경에서는 이 정도의 단순함이 적절할 수 있습니다.
src/main/java/konkuk/thip/user/application/service/following/UserFollowService.java (2)
32-43: 주석 처리된 코드 정리 필요
@Retryable관련 주석 처리된 코드가 남아있습니다. PR 설명에 따르면 이 접근 방식은 사용하지 않기로 결정한 것으로 보입니다. 코드 가독성을 위해 제거하는 것을 권장합니다.🔎 주석 처리된 코드 제거
@Override @Transactional -// @Retryable( -// notRecoverable = { -// BusinessException.class, -// InvalidStateException.class -// }, -// noRetryFor = { -// BusinessException.class, -// InvalidStateException.class -// }, -// maxAttempts = 3, -// backoff = @Backoff(delay = 100, maxDelay = 500, multiplier = 2) -// ) public Boolean changeFollowingState(UserFollowCommand followCommand) {
57-67: 주석 처리된 코드 및 미사용 메서드 정리직접 팔로워 수 변경 및 알림 관련 코드가 주석 처리되어 있고,
sendNotifications메서드(라인 77-80)는 이제 호출되는 곳이 없어 데드 코드가 되었습니다.이벤트 기반 처리로 전환이 완료되었다면 정리하는 것이 좋습니다.
🔎 데드 코드 제거
if (isFollowRequest) { // 팔로우 요청인 경우 -// targetUser.increaseFollowerCount(); followingCommandPort.save(Following.withoutId(userId, targetUserId), targetUser); - - // 팔로우 푸쉬알림 전송 -// sendNotifications(userId, targetUserId); followingEventCommandPort.publishUserFollowedEvent(userId, targetUserId); return true; } else { // 언팔로우 요청인 경우 -// targetUser.decreaseFollowerCount(); followingCommandPort.deleteFollowing(optionalFollowing.get(), targetUser); followingEventCommandPort.publishUserUnfollowedEvent(userId, targetUserId); return false; }
sendNotifications메서드도 함께 제거하거나, 이벤트 핸들러 쪽으로 이동시키세요.src/main/java/konkuk/thip/outbox/adapter/out/jpa/OutboxEventJpaEntity.java (2)
35-38: outboxStatus 필드에 @column 어노테이션 누락
eventType필드는@Column(name = "event_type", nullable = false)가 명시되어 있지만,outboxStatus필드에는@Column어노테이션이 없습니다.JPA 기본 전략으로
outboxStatus가 아닌outbox_status로 매핑될 수 있으나, SQL 마이그레이션에서는outbox_status로 정의되어 있어 일치합니다. 다만 일관성을 위해 명시적으로 지정하는 것을 권장합니다.🔎 어노테이션 추가 제안
// PENDING / PROCESSED / FAILED @Enumerated(EnumType.STRING) - + @Column(name = "outbox_status", nullable = false) private OutboxStatus outboxStatus;
58-61: 테스트 용이성을 위한 시간 처리 고려
LocalDateTime.now()를 직접 호출하면 단위 테스트에서 시간 관련 검증이 어려워집니다. 현재 구조에서 큰 문제는 아니지만, 향후 테스트 필요 시Clock을 주입받는 방식을 고려해 볼 수 있습니다.src/main/java/konkuk/thip/outbox/adapter/out/persistence/OutboxEventPersistenceAdapter.java (1)
23-26: 파라미터명 개선 제안파라미터명이
pending으로 되어 있지만, 실제로는 어떤OutboxStatus값이든 전달받을 수 있습니다.status로 변경하면 더 명확합니다.🔎 파라미터명 수정
@Override - public List<OutboxEventJpaEntity> findTop1000ByStatusOrderByIdAsc(OutboxStatus pending) { - return outboxEventJpaRepository.findTop1000ByOutboxStatusOrderByIdAsc(pending); + public List<OutboxEventJpaEntity> findTop1000ByStatusOrderByIdAsc(OutboxStatus status) { + return outboxEventJpaRepository.findTop1000ByOutboxStatusOrderByIdAsc(status); }포트 인터페이스(
OutboxEventPersistencePort)도 동일하게 수정 필요:- List<OutboxEventJpaEntity> findTop1000ByStatusOrderByIdAsc(OutboxStatus pending); + List<OutboxEventJpaEntity> findTop1000ByStatusOrderByIdAsc(OutboxStatus status);src/main/java/konkuk/thip/outbox/application/service/FollowingDispatchService.java (3)
27-32: 집합체 타입 문자열을 상수로 추출하는 것을 고려하세요.Line 28과 Line 45에서
"USER"문자열이 하드코딩되어 있습니다. 이를 클래스 상수로 추출하면 유지보수성이 향상됩니다.🔎 제안하는 리팩토링
+ private static final String USER_AGGREGATE_TYPE = "USER"; + @Override public void handleUserFollow(FollowingEvent.UserFollowedEvent event) { try { String payload = objectMapper.writeValueAsString(event); OutboxEventJpaEntity entity = OutboxEventJpaEntity.pending( - "USER", + USER_AGGREGATE_TYPE, event.targetUserId(), OutboxEventType.USER_FOLLOWED, payload );
34-36: 원본 예외 정보를 보존하는 것을 권장합니다.JsonProcessingException을 BusinessException으로 래핑할 때 원본 예외를 cause로 전달하지 않아 디버깅 시 스택 트레이스가 손실됩니다.
🔎 제안하는 개선
BusinessException의 생성자가 cause를 받을 수 있다면:
} catch (JsonProcessingException e) { - throw new BusinessException(ErrorCode.JSON_PROCESSING_ERROR); + throw new BusinessException(ErrorCode.JSON_PROCESSING_ERROR, e); }또는 로깅 추가:
+ } catch (JsonProcessingException e) { + log.error("Failed to serialize FollowingEvent: {}", event, e); + throw new BusinessException(ErrorCode.JSON_PROCESSING_ERROR); }
39-54: 메서드 간 중복 로직 제거를 고려하세요.
handleUserFollow와handleUserUnfollow메서드가 거의 동일한 구조를 가지고 있습니다. 공통 로직을 추출하면 유지보수성이 향상됩니다.🔎 제안하는 리팩토링
+ private void dispatchFollowingEvent(Object event, Long targetUserId, OutboxEventType eventType) { + try { + String payload = objectMapper.writeValueAsString(event); + OutboxEventJpaEntity entity = OutboxEventJpaEntity.pending( + "USER", + targetUserId, + eventType, + payload + ); + outboxEventPersistencePort.save(entity); + } catch (JsonProcessingException e) { + throw new BusinessException(ErrorCode.JSON_PROCESSING_ERROR); + } + } + @Override public void handleUserFollow(FollowingEvent.UserFollowedEvent event) { - try { - String payload = objectMapper.writeValueAsString(event); - OutboxEventJpaEntity entity = OutboxEventJpaEntity.pending( - "USER", - event.targetUserId(), - OutboxEventType.USER_FOLLOWED, - payload - ); - outboxEventPersistencePort.save(entity); - } catch (JsonProcessingException e) { - throw new BusinessException(ErrorCode.JSON_PROCESSING_ERROR); - } + dispatchFollowingEvent(event, event.targetUserId(), OutboxEventType.USER_FOLLOWED); } @Override public void handleUserUnfollow(FollowingEvent.UserUnfollowedEvent event) { - try { - String payload = objectMapper.writeValueAsString(event); - OutboxEventJpaEntity entity = OutboxEventJpaEntity.pending( - "USER", - event.targetUserId(), - OutboxEventType.USER_UNFOLLOWED, - payload - ); - outboxEventPersistencePort.save(entity); - } catch (JsonProcessingException e) { - throw new BusinessException(ErrorCode.JSON_PROCESSING_ERROR); - } + dispatchFollowingEvent(event, event.targetUserId(), OutboxEventType.USER_UNFOLLOWED); }src/test/java/konkuk/thip/user/application/service/UserFollowServiceTest.java (1)
34-42: 이벤트 발행 검증을 추가하는 것을 권장합니다.새로운
FollowingEventCommandPort의존성이 추가되었지만, 테스트에서 이벤트 발행이 올바르게 호출되는지 검증하지 않고 있습니다. 팔로우/언팔로우 시 이벤트가 정상적으로 발행되는지 확인하는 검증을 추가하는 것이 좋습니다.🔎 제안하는 검증 추가 예시
@Test @DisplayName("팔로우 관계가 없으면 새로 생성 + followerCount 증가") void follow_newRelation() { // given Long userId = 1L, targetUserId = 2L; when(followingCommandPort.findByUserIdAndTargetUserId(userId, targetUserId)) .thenReturn(Optional.empty()); User user = createUserWithFollowerCount(0); when(userCommandPort.findById(targetUserId)).thenReturn(user); when(userCommandPort.findById(userId)).thenReturn(user); UserFollowCommand command = new UserFollowCommand(userId, targetUserId, true); // when Boolean result = userFollowService.changeFollowingState(command); // then assertThat(result).isTrue(); assertThat(user.getFollowerCount()).isEqualTo(1); + verify(followingEventCommandPort).publishUserFollowedEvent(userId, targetUserId); ArgumentCaptor<Following> captor = ArgumentCaptor.forClass(Following.class); verify(followingCommandPort).save(captor.capture(), eq(user)); Following saved = captor.getValue(); assertThat(saved.getUserId()).isEqualTo(userId); assertThat(saved.getFollowingUserId()).isEqualTo(targetUserId); }언팔로우 테스트에도 유사하게
publishUserUnfollowedEvent검증을 추가할 수 있습니다.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (31)
build.gradleloadtest/follow_change_state_load_test.jsloadtest/follow_change_toggle_load_test.jssrc/main/java/konkuk/thip/common/exception/code/ErrorCode.javasrc/main/java/konkuk/thip/config/RetryConfig.javasrc/main/java/konkuk/thip/config/WorkerThreadConfig.javasrc/main/java/konkuk/thip/outbox/adapter/in/event/FollowingEventListener.javasrc/main/java/konkuk/thip/outbox/adapter/in/scheduler/OutboxScheduler.javasrc/main/java/konkuk/thip/outbox/adapter/out/jpa/OutboxEventJpaEntity.javasrc/main/java/konkuk/thip/outbox/adapter/out/jpa/OutboxEventType.javasrc/main/java/konkuk/thip/outbox/adapter/out/jpa/OutboxStatus.javasrc/main/java/konkuk/thip/outbox/adapter/out/persistence/OutboxEventPersistenceAdapter.javasrc/main/java/konkuk/thip/outbox/adapter/out/persistence/repository/OutboxEventJpaRepository.javasrc/main/java/konkuk/thip/outbox/application/port/in/FollowingDispatchUseCase.javasrc/main/java/konkuk/thip/outbox/application/port/in/FollowingOutboxProcessUseCase.javasrc/main/java/konkuk/thip/outbox/application/port/out/OutboxEventPersistencePort.javasrc/main/java/konkuk/thip/outbox/application/service/FollowingDispatchService.javasrc/main/java/konkuk/thip/outbox/application/service/FollowingOutboxProcessService.javasrc/main/java/konkuk/thip/user/adapter/out/event/FollowingEventPublisherAdapter.javasrc/main/java/konkuk/thip/user/adapter/out/event/dto/FollowingEvent.javasrc/main/java/konkuk/thip/user/adapter/out/jpa/FollowingJpaEntity.javasrc/main/java/konkuk/thip/user/adapter/out/persistence/FollowingCommandPersistenceAdapter.javasrc/main/java/konkuk/thip/user/adapter/out/persistence/UserCommandPersistenceAdapter.javasrc/main/java/konkuk/thip/user/adapter/out/persistence/repository/UserJpaRepository.javasrc/main/java/konkuk/thip/user/application/port/out/FollowingEventCommandPort.javasrc/main/java/konkuk/thip/user/application/port/out/UserCommandPort.javasrc/main/java/konkuk/thip/user/application/service/following/UserFollowService.javasrc/main/resources/db/migration/V251120__Add_following_unique_constraint.sqlsrc/main/resources/db/migration/V251124__Create_OutboxEvent_table.sqlsrc/test/java/konkuk/thip/user/application/service/UserFollowServiceTest.javasrc/test/java/konkuk/thip/user/concurrency/UserFollowConcurrencyTest.java
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: buzz0331
Repo: THIP-TextHip/THIP-Server PR: 309
File: src/main/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImpl.java:36-44
Timestamp: 2025-09-23T08:31:05.161Z
Learning: buzz0331은 기술적 이슈에 대해 실용적인 해결책을 제시하면서도 과도한 엔지니어링을 피하는 균형감을 선호한다. 복잡도 대비 실제 발생 가능성을 고려하여 "굳이" 불필요한 솔루션보다는 심플함을 유지하는 것을 중요하게 생각한다.
📚 Learning: 2025-07-29T08:11:23.599Z
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 109
File: src/main/java/konkuk/thip/user/adapter/in/web/UserQueryController.java:37-46
Timestamp: 2025-07-29T08:11:23.599Z
Learning: THIP 프로젝트에서 ExceptionDescription 어노테이션은 비즈니스 로직에서 발생하는 커스텀 에러 코드가 있는 API에만 사용하며, bean validation만 수행하는 API에는 사용하지 않는다.
Applied to files:
src/main/java/konkuk/thip/common/exception/code/ErrorCode.java
📚 Learning: 2025-07-03T03:05:05.031Z
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 43
File: src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java:0-0
Timestamp: 2025-07-03T03:05:05.031Z
Learning: THIP 프로젝트에서는 CQRS Port 분리 시 다음 컨벤션을 따름: CommandPort에는 findByXXX를 통해 도메인 엔티티를 찾아오는 메서드를 추가하고, QueryPort에는 조회 API의 response에 해당하는 데이터들을 DB로부터 조회하는 메서드를 추가함.
Applied to files:
src/main/java/konkuk/thip/user/application/port/out/FollowingEventCommandPort.javasrc/main/java/konkuk/thip/user/application/service/following/UserFollowService.javasrc/main/java/konkuk/thip/user/application/port/out/UserCommandPort.java
📚 Learning: 2025-09-01T13:18:13.652Z
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 287
File: src/main/java/konkuk/thip/user/adapter/out/persistence/repository/UserJpaRepository.java:8-14
Timestamp: 2025-09-01T13:18:13.652Z
Learning: seongjunnoh는 JpaRepository의 findById 메서드 재정의보다는 도메인별 명시적 메서드(findByUserId, findByRoomId 등)를 정의하여 Hibernate Filter 적용을 보장하는 방식을 선호하며, 이를 통해 더 안전하고 의도가 명확한 코드 구조를 구축한다.
Applied to files:
src/main/java/konkuk/thip/user/adapter/out/persistence/repository/UserJpaRepository.javasrc/main/java/konkuk/thip/user/adapter/out/persistence/UserCommandPersistenceAdapter.java
📚 Learning: 2025-06-29T09:47:31.299Z
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 36
File: src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java:7-7
Timestamp: 2025-06-29T09:47:31.299Z
Learning: Spring Data JPA에서 findBy{FieldName} 패턴의 메서드는 명시적 선언 없이 자동으로 생성되며, Optional<Entity> 반환 타입을 사용하는 것이 null 안전성을 위해 권장됩니다.
Applied to files:
src/main/java/konkuk/thip/user/adapter/out/persistence/repository/UserJpaRepository.java
📚 Learning: 2025-10-13T08:39:43.833Z
Learnt from: buzz0331
Repo: THIP-TextHip/THIP-Server PR: 323
File: build.gradle:102-104
Timestamp: 2025-10-13T08:39:43.833Z
Learning: Spring AI 1.0.0-M6에서 Google AI Gemini 전용 스타터가 빈 등록에 실패하는 경우, spring-ai-openai-spring-boot-starter를 사용하고 외부 설정(환경 변수 등)으로 spring.ai.openai.base-url을 Google의 OpenAI 호환 엔드포인트로, spring.ai.openai.api-key를 Google Cloud 액세스 토큰으로, spring.ai.openai.chat.options.model을 Gemini 모델명으로 지정하여 우회할 수 있습니다.
Applied to files:
build.gradle
🧬 Code graph analysis (6)
src/main/java/konkuk/thip/outbox/application/service/FollowingOutboxProcessService.java (1)
src/main/java/konkuk/thip/user/adapter/out/event/dto/FollowingEvent.java (1)
FollowingEvent(5-10)
src/main/java/konkuk/thip/outbox/application/port/in/FollowingDispatchUseCase.java (1)
src/main/java/konkuk/thip/user/adapter/out/event/dto/FollowingEvent.java (1)
FollowingEvent(5-10)
src/main/java/konkuk/thip/outbox/adapter/in/event/FollowingEventListener.java (1)
src/main/java/konkuk/thip/user/adapter/out/event/dto/FollowingEvent.java (1)
FollowingEvent(5-10)
src/main/java/konkuk/thip/user/adapter/out/event/FollowingEventPublisherAdapter.java (1)
src/main/java/konkuk/thip/user/adapter/out/event/dto/FollowingEvent.java (1)
FollowingEvent(5-10)
src/main/java/konkuk/thip/outbox/application/service/FollowingDispatchService.java (1)
src/main/java/konkuk/thip/user/adapter/out/event/dto/FollowingEvent.java (1)
FollowingEvent(5-10)
src/test/java/konkuk/thip/user/concurrency/UserFollowConcurrencyTest.java (1)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(35-417)
⏰ 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 (18)
loadtest/follow_change_state_load_test.js (4)
30-42: LGTM!에러 파싱 로직이 안정적으로 구현되어 있으며, JSON 파싱 실패 시에도 안전한 기본값을 반환합니다.
46-61: LGTM!
per-vu-iterationsexecutor를 사용한 burst 테스트 시나리오 구성이 적절하며, 5xx 에러 0건과 p95 지연시간 임계값 설정이 서버 안정성 검증에 적합합니다.
64-93: LGTM!토큰 발급 배치 처리와 실패 시 인덱스 유지를 위한 빈 문자열 삽입 패턴이 잘 구현되어 있습니다. 동기화된 시작 시간(
startAt) 계산도 적절합니다.
96-148: LGTM!VU별 토큰 매핑, 동기화된 시작, HTTP 상태 코드별 메트릭 수집, 도메인 에러 코드 분류가 체계적으로 구현되어 있습니다. 동시성 테스트 시나리오에 적합한 구조입니다.
loadtest/follow_change_toggle_load_test.js (4)
12-13: LGTM!토글 테스트를 위한 반복 횟수(
TOGGLE_ITER)와 간격(TOGGLE_PAUSE_S) 설정이 적절합니다.
50-64: LGTM!
iterations: TOGGLE_ITER설정으로 각 VU가 여러 번 팔로우/언팔로우를 반복하는 시나리오가 적절히 구성되어 있습니다.
119-133: LGTM!상태 토글 로직이 올바르게 구현되어 있습니다. 200 응답 시에만 상태를 반전시키고, 4xx/5xx 응답 시에는 동일 동작을 재시도하는 설계가 적절합니다. 팔로우/언팔로우 성공 카운터로 정확한 메트릭 추적도 가능합니다.
154-161: LGTM!응답 검증
check()와 토글 간격sleep(TOGGLE_PAUSE_S)이 적절히 구현되어 있습니다.src/main/java/konkuk/thip/outbox/adapter/out/jpa/OutboxEventType.java (1)
1-6: 이벤트 타입 enum 정의 적절함Outbox 패턴을 위한 이벤트 타입 enum이 명확하게 정의되어 있습니다. USER_FOLLOWED와 USER_UNFOLLOWED 두 가지 이벤트 타입으로 팔로잉 관련 이벤트를 구분하기에 충분합니다.
build.gradle (1)
105-106: Spring Boot 3.5.0은 spring-retry를 버전 2.0.12로 자동 관리하며, 현재 버전은 최신 안정 릴리스입니다.Spring Retry 2.0.12에 대해 알려진 직접적인 보안 취약점은 없으므로 현재 의존성 선언은 적절합니다. 다만 Spring Framework와 Spring Security 같은 전이 의존성의 CVE는 정기적으로 모니터링하여 업데이트하는 것이 권장됩니다.
src/main/java/konkuk/thip/outbox/adapter/out/jpa/OutboxStatus.java (1)
3-7: LGTM!Outbox 이벤트 상태 관리를 위한 enum이 명확하게 정의되었습니다.
src/main/java/konkuk/thip/user/adapter/out/persistence/UserCommandPersistenceAdapter.java (1)
40-46: LGTM!기존
findById메서드와 일관된 패턴으로 잠금 버전이 구현되었습니다. 동시성 제어가 필요한 상황에서 적절하게 사용될 것입니다.src/main/java/konkuk/thip/outbox/adapter/out/persistence/repository/OutboxEventJpaRepository.java (1)
9-11: 동시 처리 시나리오를 확인하세요.여러 스케줄러 인스턴스가 동시에 실행될 경우, 동일한 이벤트를 중복 처리할 수 있습니다. 단일 서버 환경이라면 문제없지만, 다중 인스턴스 환경을 고려한다면
@Lock을 추가하거나 처리 시 상태를 먼저 업데이트하는 방식을 고려하세요.PR 설명에서 단일 서버 RPS 한계를 고려하여 merge하지 않을 예정이라고 언급되어 있으므로, 현재 구현은 단일 인스턴스 환경에서 적합합니다.
src/main/java/konkuk/thip/outbox/application/service/FollowingOutboxProcessService.java (1)
28-34: @async와 @transactional 조합 시 트랜잭션 범위 검토
@Async와@Transactional이 함께 사용되면 비동기 스레드에서 새 트랜잭션이 시작됩니다. 현재 구조에서는 최대 1000개의 이벤트가 하나의 트랜잭션에서 처리되므로:
- 하나의 이벤트 처리가 오래 걸리면 전체 트랜잭션이 지연됨
- 트랜잭션 내 예외 발생 시 이미 처리된 이벤트의 상태 변경도 롤백될 수 있음
이벤트별로 트랜잭션을 분리하거나, 상태 변경을 즉시 저장하는 방식을 고려해 보세요.
src/main/java/konkuk/thip/outbox/adapter/in/event/FollowingEventListener.java (1)
16-24: LGTM!
TransactionPhase.BEFORE_COMMIT을 사용하여 Outbox 이벤트가 원본 트랜잭션과 함께 원자적으로 저장되도록 올바르게 구현되었습니다. Transactional Outbox 패턴의 핵심 요구사항을 잘 충족합니다.src/main/resources/db/migration/V251124__Create_OutboxEvent_table.sql (1)
1-29: LGTM!Outbox 패턴에 적합한 테이블 구조입니다.
(outbox_status, id)복합 인덱스는 PENDING 이벤트 폴링 성능에 효과적입니다.src/main/java/konkuk/thip/outbox/application/service/FollowingDispatchService.java (1)
15-21: 클래스 구조가 명확합니다.서비스 계층의 의존성 주입과 인터페이스 구현이 적절하게 구성되어 있습니다.
src/main/java/konkuk/thip/user/adapter/out/event/FollowingEventPublisherAdapter.java (1)
9-34: 구현이 명확하고 적절합니다.어댑터가 포트 인터페이스를 올바르게 구현하고 있으며, Spring의 이벤트 발행 메커니즘을 적절히 활용하고 있습니다. 코드가 간결하고 이해하기 쉽습니다.
|
|
||
| PERSISTENCE_TRANSACTION_REQUIRED(HttpStatus.INTERNAL_SERVER_ERROR, 50110, "@Transactional 컨텍스트가 필요합니다. 트랜잭션 범위 내에서만 사용할 수 있습니다."), | ||
|
|
||
| RESOURCE_LOCKED(HttpStatus.LOCKED, 50200, "자원이 잠겨 있어 요청을 처리할 수 없습니다."), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for RESOURCE_LOCKED usage in the codebase
rg -nC3 'RESOURCE_LOCKED' --type=javaRepository: THIP-TextHip/THIP-Server
Length of output: 192
🏁 Script executed:
#!/bin/bash
# RESOURCE_LOCKED 에러 코드 사용처 검색
rg -nC3 'RESOURCE_LOCKED' --type=javaRepository: THIP-TextHip/THIP-Server
Length of output: 1646
자원 잠금 에러 코드 추가 확인
HTTP 423 LOCKED 상태와 에러 코드 50200을 사용한 RESOURCE_LOCKED 추가는 적절합니다. 다만 현재 RESOURCE_LOCKED는 코드베이스에서 활발하게 사용되지 않고 있습니다. UserFollowService.java의 74번 줄에서만 참조되고 있으나, 해당 코드는 주석 처리된 상태입니다. 이 에러 코드가 향후 사용될 예정인지, 아니면 활성화가 필요한지 확인하세요.
| @Configuration | ||
| @EnableRetry(proxyTargetClass = true) | ||
| public class RetryConfig { | ||
| } No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for @Retryable usage
rg -n '@Retryable' --type=java -A 5Repository: THIP-TextHip/THIP-Server
Length of output: 783
🏁 Script executed:
#!/bin/bash
# Search for uncommented @Retryable annotations (not in comments)
rg '^\s*@Retryable' --type=java -A 3
# Also check all instances of @Retryable including commented ones
rg '@Retryable' --type=java | grep -v '//' | head -20
# Check the class structure where @Retryable is used
fd RetryConfig.java -x cat {}
</parameter>Repository: THIP-TextHip/THIP-Server
Length of output: 422
🏁 Script executed:
#!/bin/bash
# Search for @Retryable not preceded by // (uncommented)
rg '@Retryable' --type=java | grep -v '//' | grep -v '^\s*//'
# Count total @Retryable occurrences
echo "=== Total @Retryable occurrences ==="
rg '@Retryable' --type=java | wc -l
# Search more precisely for active annotations
rg -B 2 -A 10 '^\s*@Retryable\(' --type=javaRepository: THIP-TextHip/THIP-Server
Length of output: 106
🏁 Script executed:
#!/bin/bash
# Read the commented @Retryable annotation context
sed -n '25,50p' src/main/java/konkuk/thip/user/application/service/following/UserFollowService.javaRepository: THIP-TextHip/THIP-Server
Length of output: 998
@EnableRetry 설정이 필요 없음
현재 코드베이스에 활성화된 @Retryable 어노테이션이 없습니다. 유일하게 발견된 @Retryable은 UserFollowService.java에서 주석 처리되어 있습니다. 따라서 @EnableRetry(proxyTargetClass = true) 설정 전체를 제거하거나, @Retryable을 실제로 사용할 계획이 있다면 재검토가 필요합니다.
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/config/RetryConfig.java around lines 6 to 9, the
configuration class only enables @EnableRetry but there are no active @Retryable
usages (the only one is commented out), so either remove the
@EnableRetry(proxyTargetClass = true) annotation (and delete the class if it
becomes empty) to avoid unnecessary configuration, or if you intend to use retry
semantics, uncomment and apply @Retryable where needed and keep @EnableRetry;
update the code accordingly.
| // 1초마다 PENDING 이벤트를 최대 100개씩 처리하는 예시 | ||
| @Scheduled(fixedDelay = 1000) | ||
| public void processPendingEvents() { | ||
| followingOutboxProcessUseCase.processFollowingOutboxEvents(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
주석과 실제 구현의 불일치 확인 필요
주석에는 "최대 100개씩 처리"라고 되어 있지만, FollowingOutboxProcessService와 OutboxEventPersistencePort에서는 findTop1000ByStatusOrderByIdAsc를 사용하여 실제로는 1000개를 조회합니다.
주석을 수정하거나 배치 크기를 의도에 맞게 조정해 주세요.
🔎 주석 수정 제안
- // 1초마다 PENDING 이벤트를 최대 100개씩 처리하는 예시
+ // 1초마다 PENDING 이벤트를 최대 1000개씩 처리
@Scheduled(fixedDelay = 1000)🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/outbox/adapter/in/scheduler/OutboxScheduler.java
around lines 14-18, the comment says "최대 100개씩 처리" but downstream code actually
queries 1000 items; update the comment to reflect the real batch size (예: "최대
1000개씩 처리") or change the batch retrieval to 100 items to match the comment;
specifically either edit the comment text to state 1000 or modify the retrieval
method in FollowingOutboxProcessService/OutboxEventPersistencePort from
findTop1000ByStatusOrderByIdAsc to a 100-sized query (and adjust any method
names/implementations accordingly) so comment and implementation agree.
| } else if (OutboxEventType.USER_UNFOLLOWED.equals(event.getEventType())) { | ||
| FollowingEvent.UserFollowedEvent payload = | ||
| objectMapper.readValue(event.getPayload(), FollowingEvent.UserFollowedEvent.class); | ||
| User user = userCommandPort.findById(payload.targetUserId()); | ||
| user.decreaseFollowerCount(); | ||
| userCommandPort.update(user); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[버그] USER_UNFOLLOWED 이벤트에서 잘못된 타입으로 역직렬화
USER_UNFOLLOWED 케이스에서 UserUnfollowedEvent가 아닌 UserFollowedEvent로 역직렬화하고 있습니다. 현재 두 이벤트의 필드가 동일하여 동작할 수 있지만, 향후 필드가 달라지면 오류가 발생하거나 잘못된 동작을 할 수 있습니다.
🔎 수정 제안
} else if (OutboxEventType.USER_UNFOLLOWED.equals(event.getEventType())) {
- FollowingEvent.UserFollowedEvent payload =
- objectMapper.readValue(event.getPayload(), FollowingEvent.UserFollowedEvent.class);
+ FollowingEvent.UserUnfollowedEvent payload =
+ objectMapper.readValue(event.getPayload(), FollowingEvent.UserUnfollowedEvent.class);
User user = userCommandPort.findById(payload.targetUserId());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } else if (OutboxEventType.USER_UNFOLLOWED.equals(event.getEventType())) { | |
| FollowingEvent.UserFollowedEvent payload = | |
| objectMapper.readValue(event.getPayload(), FollowingEvent.UserFollowedEvent.class); | |
| User user = userCommandPort.findById(payload.targetUserId()); | |
| user.decreaseFollowerCount(); | |
| userCommandPort.update(user); | |
| } else if (OutboxEventType.USER_UNFOLLOWED.equals(event.getEventType())) { | |
| FollowingEvent.UserUnfollowedEvent payload = | |
| objectMapper.readValue(event.getPayload(), FollowingEvent.UserUnfollowedEvent.class); | |
| User user = userCommandPort.findById(payload.targetUserId()); | |
| user.decreaseFollowerCount(); | |
| userCommandPort.update(user); |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/outbox/application/service/FollowingOutboxProcessService.java
around lines 43 to 48, the USER_UNFOLLOWED branch deserializes the payload as
UserFollowedEvent; change the deserialization to use UserUnfollowedEvent (and
update the payload variable type/name accordingly) so the correct event class is
used when reading event.getPayload(); keep the existing logic that finds the
target user, decreases follower count, and updates the user.
| } catch (Exception e) { | ||
| // 실패 시 전략: FAILED로 마킹 + 로그, 재시도 정책 등 | ||
| event.markAsFailed(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
예외 로깅 누락
예외 발생 시 markAsFailed()만 호출하고 예외 정보를 로깅하지 않아 디버깅이 어렵습니다. 최소한 예외 메시지와 스택 트레이스를 로깅하는 것이 좋습니다.
🔎 로깅 추가 제안
클래스에 로거 추가:
import lombok.extern.slf4j.Slf4j;
@Slf4j
@Service
@RequiredArgsConstructor
public class FollowingOutboxProcessService implements FollowingOutboxProcessUseCase {catch 블록 수정:
} catch (Exception e) {
- // 실패 시 전략: FAILED로 마킹 + 로그, 재시도 정책 등
+ log.error("Outbox 이벤트 처리 실패. eventId={}, eventType={}",
+ event.getId(), event.getEventType(), e);
event.markAsFailed();
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/outbox/application/service/FollowingOutboxProcessService.java
around lines 52-55, the catch block only calls event.markAsFailed() and does not
log the exception; add a logger to the class (e.g., annotate with @Slf4j or
create a private static final Logger) and update the catch block to log the
exception message and stack trace (e.g., log.error("Failed processing following
outbox event: {}", event.getId(), e)) before or after calling
event.markAsFailed().
| @Table( | ||
| name = "followings", | ||
| uniqueConstraints = { | ||
| @UniqueConstraint( | ||
| name = "uq_followings_user_target", | ||
| columnNames = {"user_id", "following_user_id"} | ||
| ) | ||
| } | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Find the FollowingJpaEntity.java file
find . -name "FollowingJpaEntity.java" -type fRepository: THIP-TextHip/THIP-Server
Length of output: 140
🏁 Script executed:
# Search for SQL migration files related to followings
find . -name "*.sql" -o -name "*migration*" | grep -i followRepository: THIP-TextHip/THIP-Server
Length of output: 146
🏁 Script executed:
# Search for Follow-related service and controller files
find . -type f -name "*Follow*Service*.java" -o -name "*Follow*Controller*.java"Repository: THIP-TextHip/THIP-Server
Length of output: 844
🏁 Script executed:
# Read the FollowingJpaEntity.java file to verify the constraint
cat -n ./src/main/java/konkuk/thip/user/adapter/out/jpa/FollowingJpaEntity.javaRepository: THIP-TextHip/THIP-Server
Length of output: 1347
🏁 Script executed:
# Read the SQL migration to verify it matches
cat -n ./src/main/resources/db/migration/V251120__Add_following_unique_constraint.sqlRepository: THIP-TextHip/THIP-Server
Length of output: 248
🏁 Script executed:
# Search for DataIntegrityViolationException handling in Following services
rg -n -C5 "DataIntegrityViolationException|ConstraintViolationException|duplicate" \
./src/main/java/konkuk/thip/user/application/service/following/UserFollowService.java \
./src/main/java/konkuk/thip/user/application/service/ --type javaRepository: THIP-TextHip/THIP-Server
Length of output: 50
🏁 Script executed:
# Read UserFollowService.java directly to check for exception handling
cat -n ./src/main/java/konkuk/thip/user/application/service/following/UserFollowService.javaRepository: THIP-TextHip/THIP-Server
Length of output: 4234
🏁 Script executed:
# Check the controller test to see if exception handling is tested
cat -n ./src/test/java/konkuk/thip/user/adapter/in/web/UserFollowControllerTest.javaRepository: THIP-TextHip/THIP-Server
Length of output: 2417
🏁 Script executed:
# Search more broadly for exception handling patterns in the entire codebase related to follow
rg -n "try|catch|throw|Exception" ./src/main/java/konkuk/thip/user/application/service/following/ --type java -A2 -B2Repository: THIP-TextHip/THIP-Server
Length of output: 3570
🏁 Script executed:
# Search for global exception handlers or advice that might catch DataIntegrityViolationException
rg -n "DataIntegrityViolationException|ExceptionHandler|@ControllerAdvice" ./src/main/java/konkuk/thip/ --type java -A3 -B1Repository: THIP-TextHip/THIP-Server
Length of output: 11170
🏁 Script executed:
# Check the FollowingCommandPort interface to see what exceptions it declares
rg -n "FollowingCommandPort|save.*Following" ./src/main/java/konkuk/thip/user/application/port/out/ --type java -A5Repository: THIP-TextHip/THIP-Server
Length of output: 1509
🏁 Script executed:
# Search for any test files that test duplicate follow scenarios
rg -n "duplicate|already.*follow|constraint|unique" ./src/test/java/konkuk/thip/user/ --type java -iRepository: THIP-TextHip/THIP-Server
Length of output: 1214
🏁 Script executed:
# Read the full GlobalExceptionHandler to see all exception handlers
cat -n ./src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java | head -120Repository: THIP-TextHip/THIP-Server
Length of output: 6366
🏁 Script executed:
# Check if there's any specific handling for DataIntegrityViolationException anywhere
rg -n "DataIntegrityViolationException" ./src/main/java/konkuk/thip/ --type javaRepository: THIP-TextHip/THIP-Server
Length of output: 50
🏁 Script executed:
# Check the FollowingCommandPort implementation to see actual save method
find ./src -name "*FollowingCommandPort*Impl*.java" -o -name "*FollowingAdapter*.java" | head -5Repository: THIP-TextHip/THIP-Server
Length of output: 50
🏁 Script executed:
# Look for the repository/adapter implementation that does the actual save
rg -n "class.*FollowingCommandPort" ./src/main/java/konkuk/thip/user/ --type javaRepository: THIP-TextHip/THIP-Server
Length of output: 249
🏁 Script executed:
# Read the FollowingCommandPersistenceAdapter to see how save is implemented
cat -n ./src/main/java/konkuk/thip/user/adapter/out/persistence/FollowingCommandPersistenceAdapter.javaRepository: THIP-TextHip/THIP-Server
Length of output: 4325
🏁 Script executed:
# Check if there's an error code for duplicate follow
rg -n "ALREADY_FOLLOWED|DUPLICATE.*FOLLOW|FOLLOW.*DUPLICATE" ./src/main/java/konkuk/thip/common/exception/code/ --type java -iRepository: THIP-TextHip/THIP-Server
Length of output: 212
유니크 제약 조건은 올바르게 추가되었으나, 제약 조건 위반 시 예외 처리가 미구현된 상태입니다.
JPA 엔티티와 SQL 마이그레이션에서 복합 유니크 제약이 정확하게 정의되었으나, DataIntegrityViolationException 처리가 구현되지 않았습니다. 현재 중복 팔로우 시도 시 해당 예외는 제네릭 RuntimeException 핸들러에 의해 500 오류로 응답되고 있습니다.
필요한 조치:
GlobalExceptionHandler에DataIntegrityViolationException핸들러를 추가하거나,FollowingCommandPersistenceAdapter.save()(46줄)에서 제약 조건 위반 시BusinessException(USER_ALREADY_FOLLOWED)로 변환하세요.USER_ALREADY_FOLLOWED에러 코드가 이미 정의되어 있으므로(70001, BAD_REQUEST) 이를 활용하면 됩니다.
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/user/adapter/out/jpa/FollowingJpaEntity.java (lines
8-16) the DB-level unique constraint is defined but duplicate-follow attempts
are not mapped to a business error; update
src/main/java/konkuk/thip/user/adapter/out/FollowingCommandPersistenceAdapter.java
(save method around line 46) to catch
org.springframework.dao.DataIntegrityViolationException (thrown when the unique
constraint is violated) and rethrow your domain BusinessException with the
existing USER_ALREADY_FOLLOWED error code (70001, BAD_REQUEST); ensure necessary
imports and that the catch only translates constraint-violation cases (or
optionally inspect root cause message) and rethrow other exceptions unchanged.
| // UserJpaEntity targetUserJpaEntity = updateUserFollowerCount(targetUser); | ||
| UserJpaEntity targetUserJpaEntity = userJpaRepository.findByUserId(targetUser.getId()).orElseThrow( | ||
| () -> new EntityNotFoundException(USER_NOT_FOUND)); | ||
|
|
||
| followingJpaRepository.save(followingMapper.toJpaEntity(userJpaEntity, targetUserJpaEntity)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
주석 처리된 코드와 미사용 메서드를 제거하세요.
updateUserFollowerCount 호출이 주석 처리되어 있고, 해당 private 메서드(lines 76-84)도 더 이상 사용되지 않습니다. 이벤트 기반 처리로 전환된 만큼, 주석과 미사용 코드는 제거하는 것이 좋습니다.
🔎 제안된 수정사항
@Override
public void save(Following following, User targetUser) { // insert용
UserJpaEntity userJpaEntity = userJpaRepository.findByUserId(following.getUserId()).orElseThrow(
() -> new EntityNotFoundException(USER_NOT_FOUND));
-// UserJpaEntity targetUserJpaEntity = updateUserFollowerCount(targetUser);
UserJpaEntity targetUserJpaEntity = userJpaRepository.findByUserId(targetUser.getId()).orElseThrow(
() -> new EntityNotFoundException(USER_NOT_FOUND));
followingJpaRepository.save(followingMapper.toJpaEntity(userJpaEntity, targetUserJpaEntity));
}
@Override
public void deleteFollowing(Following following, User targetUser) {
-// updateUserFollowerCount(targetUser);
FollowingJpaEntity followingJpaEntity = followingJpaRepository.findByUserAndTargetUser(following.getUserId(), following.getFollowingUserId())
.orElseThrow(() -> new EntityNotFoundException(FOLLOW_NOT_FOUND));
followingJpaRepository.delete(followingJpaEntity);
}그리고 미사용 updateUserFollowerCount private 메서드(lines 76-84)도 삭제:
- private UserJpaEntity updateUserFollowerCount(User targetUser) {
- UserJpaEntity userJpaEntity = userJpaRepository.findByUserId(targetUser.getId()).orElseThrow(
- () -> new EntityNotFoundException(USER_NOT_FOUND)
- );
-
- userJpaEntity.updateFrom(targetUser);
- userJpaRepository.save(userJpaEntity);
- return userJpaEntity;
- }Also applies to: 51-51
🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/user/adapter/out/persistence/FollowingCommandPersistenceAdapter.java
around lines 42-46 (and the commented occurrence at 51), remove the
commented-out call to updateUserFollowerCount and the commented code block
itself; then delete the now-unused private method updateUserFollowerCount at
lines 76-84; finally, run a quick import/unused-member cleanup to remove any
imports or references that became unused after deleting the method.
| @Lock(LockModeType.PESSIMISTIC_WRITE) | ||
| @QueryHints({ | ||
| @QueryHint(name = "jakarta.persistence.lock.timeout", value = "5000") // 5초 | ||
| }) | ||
| @Query("select u from UserJpaEntity u where u.userId = :userId") | ||
| Optional<UserJpaEntity> findByUserIdWithLock(@Param("userId") Long userId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
jakarta.persistence.lock.timeout MySQL InnoDB support JPA Hibernate
💡 Result:
Short answer
- jakarta.persistence.lock.timeout is a JPA hint (milliseconds) — it’s only a hint and its effect depends on the JPA provider and the database. [1][2]
- With MySQL/InnoDB the JPA hint is not reliably translated into a statement-level millisecond timeout. In practice you must either use NOWAIT / SKIP LOCKED behavior or change the session’s innodb_lock_wait_timeout (seconds) before obtaining the lock. [2][3][4]
Details and practical options
- Behavior expectations
- The JPA spec describes jakarta.persistence.lock.timeout as a hint (milliseconds) and warns it’s not portable — providers/databases may ignore it. [1]
- Hibernate will recognize 0 as NOWAIT and -2 as SKIP_LOCKED for pessimistic locks; other numeric values are not universally translated to a DB-level millisecond timeout. Whether a nonzero timeout is applied depends on DB support. [3][5]
- MySQL / InnoDB specifics
- MySQL/InnoDB does not offer a per-statement millisecond lock-wait timeout that JPA’s millisecond hint can map to. InnoDB uses the session variable innodb_lock_wait_timeout (integer seconds) as the wait limit for lock acquisition. You can set it for the session: SET SESSION innodb_lock_wait_timeout = N; then run the locking query. [2][4]
- MySQL supports SELECT ... FOR UPDATE NOWAIT and SKIP LOCKED (to fail immediately or skip locked rows) in recent MySQL 8.x versions; these map to the JPA semantics for 0 (NOWAIT) and -2 (SKIP_LOCKED) when the provider emits those SQL clauses. [3][5]
- Recommended approaches
- If you need immediate-fail or skip-locked semantics, use LockModeType + jakarta.persistence.lock.timeout = 0 (NOWAIT) or = -2 (SKIP_LOCKED) and confirm Hibernate + your MySQL version emits NOWAIT / SKIP LOCKED. [3][5]
- If you need a bounded wait (N seconds), set the session innodb_lock_wait_timeout (seconds) via native SQL before the JPA query in the same session/transaction; then perform the JPA lock. Example: entityManager.unwrap(Session.class).doWork(conn -> { execute "SET SESSION innodb_lock_wait_timeout=5"; }). Then run the pessimistic lock. [2][3]
- If you need millisecond precision timeouts, MySQL/InnoDB does not provide that; you must implement app-level timeouts or a retry/interrupt approach. [2][5]
Sources
- Jakarta EE tutorial — pessimistic locking and jakarta.persistence.lock.timeout. [Jakarta EE tutorial].
- mimacom blog “Handling Pessimistic Locking … MySQL” (shows using SET SESSION innodb_lock_wait_timeout). [mimacom].
- Community tests / examples (Spring JPA + MySQL + Hibernate) showing mapping of 0/-2 to NOWAIT/SKIP LOCKED and innodb_lock_wait_timeout behavior. [GitHub test repo].
- StackOverflow threads showing users must set innodb_lock_wait_timeout for MySQL and that JPA hint may be ignored. [StackOverflow].
- Baeldung / Hibernate notes about lock-timeout being a hint and provider/database-dependent. [Baeldung / Hibernate docs].
If you want, I can: (a) show a short code snippet that sets innodb_lock_wait_timeout in the same transaction before a JPA pessimistic lock, or (b) check which exact MySQL / Hibernate versions you’re using and verify whether NOWAIT / SKIP LOCKED will be emitted. Which do you prefer?
🌐 Web query:
jakarta.persistence.lock.timeout hint behavior different databases
💡 Result:
Short answer: jakarta.persistence.lock.timeout is only a hint — behavior varies by JPA provider and database. Some providers translate the hint into DB-specific lock syntax; others ignore or emulate it. Consequences:
- Spec: The timeout hint is non‑portable and may be ignored; apps should not rely on it. [1]
- Hibernate: will try to convert the hint into the DB's locking syntax when supported (e.g. emit FOR UPDATE WAIT/NO WAIT or equivalent) but will ignore it for dialects/drivers that don't support a timeout. Hibernate documents this and notes follow‑on/secondary selects for DBs (Oracle, etc.). [2]
- PostgreSQL: SQL supports NOWAIT or SKIP LOCKED but not an arbitrary millisecond timeout — in practice JPA providers map 0 to NOWAIT; non‑zero timeouts cannot be expressed by plain SELECT FOR UPDATE and are usually ignored or treated as “wait” (i.e. no-op). [3][5]
- MySQL/InnoDB: uses innodb_lock_wait_timeout (seconds) controlled at session/global level; providers won't translate jakarta.persistence.lock.timeout directly into a per‑statement millisecond wait unless they set session vars — so the JPA hint is often ineffective unless the provider implements special handling. [4]
- Practical effect: behavior differs across DBs — Oracle can accept FOR UPDATE WAIT n (so provider can map timeouts), Postgres only supports NO WAIT/SKIP LOCKED, MySQL relies on innodb_lock_wait_timeout, SQL Server uses SET LOCK_TIMEOUT — and many providers may emulate timeouts or ignore the hint entirely.
Recommendation: treat jakarta.persistence.lock.timeout as a hint only; test lock-timeout behavior on each target database and, if needed, use vendor-specific hints or provider extensions (or set DB/session lock timeout settings) for reliable behavior. [1][2][3][4][5]
Sources:
[1] Jakarta Persistence spec (lock timeout hint).
[2] Hibernate ORM user guide (locking hints / dialect behavior).
[3] PostgreSQL docs — SELECT ... FOR UPDATE NOWAIT / SKIP LOCKED.
[4] MySQL / InnoDB innodb_lock_wait_timeout docs.
[5] Community notes / Q&A about JPA/Hibernate mapping of timeout to NO WAIT on Postgres.
🌐 Web query:
PESSIMISTIC_WRITE lock timeout MySQL Spring Data JPA
💡 Result:
Short answer
- Use JPA lock hints (javax.persistence.lock.timeout) or change the InnoDB wait setting. Which one works depends on JPA provider and MySQL version. (docs.hibernate.org)
Details & practical options
- Use JPA lock hint (Spring Data JPA)
- You can annotate repository queries with @lock(LockModeType.PESSIMISTIC_WRITE) and a QueryHint for "javax.persistence.lock.timeout" (milliseconds). Example:
@lock(LockModeType.PESSIMISTIC_WRITE)
@QueryHints(@QueryHint(name="javax.persistence.lock.timeout", value="3000"))
Optional findById(...); - Behavior depends on your JPA provider (Hibernate) and how it maps the hint to SQL (NOWAIT / SKIP LOCKED / WAIT). Hibernate documents the hint semantics. (docs.hibernate.org)
- MySQL / InnoDB behavior
- MySQL/InnoDB uses innodb_lock_wait_timeout (seconds) for row lock wait time. You can read/set it for the session:
SELECT @@innodb_lock_wait_timeout;
SET SESSION innodb_lock_wait_timeout = 3; - If Hibernate/JPA does not translate javax.persistence.lock.timeout into a MySQL-specific WAIT/NOWAIT clause for your version, setting the session innodb_lock_wait_timeout before the transaction is a reliable fallback. (blog.mimacom.com)
- Provider/version differences to be aware of
- Hibernate may translate special timeout values: 0 -> NOWAIT, -2 -> SKIP_LOCKED, positive -> normal waiting behavior (or provider-specific WAIT syntax if supported). Newer MySQL versions (8.x) and newer Hibernate releases add support for FOR UPDATE WAIT / FOR UPDATE NOWAIT / SKIP LOCKED; older MySQL/Hibernate combos may ignore the JPA timeout hint and rely on innodb_lock_wait_timeout. Test on your exact versions. (docs.hibernate.org)
Recommended approach
- If you want app-level control and portability: use javax.persistence.lock.timeout via @QueryHints and verify behavior on your Hibernate + MySQL versions. (docs.hibernate.org)
- If you need guaranteed per-transaction seconds for MySQL: run SET SESSION innodb_lock_wait_timeout = N at the start of the transaction (then run the pessimistic lock). (blog.mimacom.com)
If you tell me your Spring Boot, Hibernate and MySQL versions I’ll give the exact code/snippets and what to expect.
Citations:
- 1: https://docs.hibernate.org/orm/5.0/userguide/html_single/chapters/locking/Locking.html?utm_source=openai
- 2: https://docs.hibernate.org/orm/5.0/userguide/html_single/chapters/locking/Locking.html?utm_source=openai
- 3: https://blog.mimacom.com/handling-pessimistic-locking-jpa-oracle-mysql-postgresql-derbi-h2/?utm_source=openai
- 4: https://docs.hibernate.org/orm/5.0/userguide/html_single/chapters/locking/Locking.html?utm_source=openai
- 5: https://docs.hibernate.org/orm/5.0/userguide/html_single/chapters/locking/Locking.html?utm_source=openai
- 6: https://blog.mimacom.com/handling-pessimistic-locking-jpa-oracle-mysql-postgresql-derbi-h2/?utm_source=openai
비관적 잠금 구현은 적절하나 타임아웃 설정이 MySQL에서 작동하지 않습니다.
PESSIMISTIC_WRITE 락은 동시성 문제 방지에 적절합니다. 다만 jakarta.persistence.lock.timeout = "5000" 힌트는 MySQL/InnoDB에서 작동하지 않습니다. MySQL은 밀리초 단위의 잠금 타임아웃을 지원하지 않으며, 이 JPA 힌트는 Hibernate에서 무시됩니다. 현재 설정으로는 세션의 기본 innodb_lock_wait_timeout(기본값 약 50초)이 적용됩니다.
5초 타임아웃을 적용하려면:
- 권장 방법: 트랜잭션 시작 시
SET SESSION innodb_lock_wait_timeout = 5실행 - 또는
jakarta.persistence.lock.timeout = 0(NOWAIT) 또는-2(SKIP_LOCKED) 사용 (MySQL 8.x+)
🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/user/adapter/out/persistence/repository/UserJpaRepository.java
around lines 22-27, the QueryHint jakarta.persistence.lock.timeout="5000" is
ignored by MySQL/InnoDB; to enforce a 5s lock wait you must either set the
session innodb_lock_wait_timeout to 5 at the start of the transactional boundary
(e.g., in the service or repository method that begins the @Transactional flow
call EntityManager.createNativeQuery("SET SESSION innodb_lock_wait_timeout =
5").executeUpdate() before invoking findByUserIdWithLock), or change the locking
strategy to NOWAIT/SKIP_LOCKED by replacing the hint with
jakarta.persistence.lock.timeout="0" (NOWAIT) or "-2" (SKIP_LOCKED) if your
MySQL version supports it; also remove or update the current QueryHint since it
has no effect on MySQL.
| -- 팔로잉 테이블에 사용자와 타겟 사용자 간의 유니크 제약 조건 추가 | ||
| ALTER TABLE followings | ||
| ADD CONSTRAINT uq_followings_user_target | ||
| UNIQUE (user_id, following_user_id); No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
기존 중복 데이터 확인이 필요합니다.
마이그레이션 실행 전 followings 테이블에 중복된 (user_id, following_user_id) 조합이 존재하면 제약 조건 추가가 실패합니다. 프로덕션 배포 시 서비스 중단을 방지하기 위해 다음 조치를 권장합니다:
- 마이그레이션 실행 전 중복 데이터 확인
- 중복 데이터가 있다면 정리 로직 선행
🔎 중복 데이터 확인 쿼리 및 개선된 마이그레이션
중복 데이터 확인 쿼리:
-- 중복된 팔로우 관계 확인
SELECT user_id, following_user_id, COUNT(*) as cnt
FROM followings
GROUP BY user_id, following_user_id
HAVING COUNT(*) > 1;개선된 마이그레이션 (중복 제거 포함):
-- 팔로잉 테이블에 사용자와 타겟 사용자 간의 유니크 제약 조건 추가
-- 1단계: 중복 데이터 정리 (최신 것만 남기고 삭제)
DELETE FROM followings
WHERE following_id NOT IN (
SELECT MIN(following_id)
FROM followings
GROUP BY user_id, following_user_id
);
-- 2단계: 유니크 제약 조건 추가
ALTER TABLE followings
ADD CONSTRAINT uq_followings_user_target
UNIQUE (user_id, following_user_id);🤖 Prompt for AI Agents
In src/main/resources/db/migration/V251120__Add_following_unique_constraint.sql
lines 1-4: before adding the UNIQUE constraint, first detect and remove any
duplicate (user_id, following_user_id) rows and then apply the constraint;
specifically, run the provided aggregation query to list duplicates, delete
duplicate rows while keeping a single canonical record per (user_id,
following_user_id) (e.g., keep the row with the MIN or MAX primary key), and
only after cleanup execute the ALTER TABLE ... ADD CONSTRAINT
uq_followings_user_target UNIQUE (user_id, following_user_id); perform these
steps in a migration script or pre-migration step inside a transaction and
ensure you have a backup/snapshot before deletion.
| System.out.println("=== RESULT ==="); | ||
| System.out.println("OK responses : " + okCount); | ||
| System.out.println("followings rows : " + followingRows); | ||
| System.out.println("target user's follower_count : " + storedFollowerCount); | ||
|
|
||
| // 실제 생성된 팔로잉 행 수는 팔로우 요청 횟수보다 적을 수 있다. (데이터 정합성이 깨지는 경우) | ||
| Assertions.assertThat(followingRows).isLessThanOrEqualTo(followerCount); | ||
| // User 테이블의 팔로워 수 컬럼 역시 팔로우 요청 횟수보다 적을 수 있다. (데이터 정합성이 깨지는 경우) | ||
| Assertions.assertThat(storedFollowerCount).isLessThanOrEqualTo(followerCount); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
데이터 정합성 불일치를 허용하는 테스트 단언문
현재 테스트는 isLessThanOrEqualTo를 사용하여 데이터 불일치를 허용하고 있습니다. 이는 Outbox 패턴 도입 "전"의 문제 상황을 재현하는 테스트로 보입니다.
만약 이 PR이 Outbox 패턴으로 동시성 이슈를 해결하는 것이라면:
- 테스트는 정합성을 검증해야 합니다 (
isEqualTo(followerCount)) - 또는 Outbox 이벤트 처리를 대기한 후 eventual consistency를 검증해야 합니다
현재 테스트는 문제를 입증하지만 해결을 검증하지 않습니다.
PR 목표가 동시성 이슈 해결이라면, 다음을 고려하세요:
// Outbox 이벤트 처리 대기
Thread.sleep(5000); // 또는 polling을 통한 대기
// 최종 정합성 검증
Assertions.assertThat(followingRows).isEqualTo(followerCount);
Assertions.assertThat(storedFollowerCount).isEqualTo(followerCount);또는 이 테스트가 "문제 재현용"이라면, 테스트명과 주석을 더 명확히 하여 의도를 표현하세요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/test/java/konkuk/thip/config/TestAsyncConfig.javasrc/test/java/konkuk/thip/user/adapter/in/web/UserFollowApiTest.javasrc/test/java/konkuk/thip/user/application/service/UserFollowServiceTest.java
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: buzz0331
Repo: THIP-TextHip/THIP-Server PR: 309
File: src/main/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImpl.java:36-44
Timestamp: 2025-09-23T08:31:05.161Z
Learning: buzz0331은 기술적 이슈에 대해 실용적인 해결책을 제시하면서도 과도한 엔지니어링을 피하는 균형감을 선호한다. 복잡도 대비 실제 발생 가능성을 고려하여 "굳이" 불필요한 솔루션보다는 심플함을 유지하는 것을 중요하게 생각한다.
⏰ 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 (1)
src/test/java/konkuk/thip/config/TestAsyncConfig.java (1)
33-39: 새로운 outboxAsyncExecutor 빈 추가가 적절합니다.테스트 환경에서 outbox 비동기 처리를 동기적으로 실행하기 위한
SyncTaskExecutor사용이 올바르게 구현되었습니다. 이는 프로덕션의WorkerThreadConfig.outboxAsyncExecutor와 일치하며, outbox 패턴 테스트에 적합합니다.
| @Bean(name = "schedulerAsyncExecutor") | ||
|
|
||
| public Executor schedulerAsyncExecutor() { | ||
|
|
||
| return new SyncTaskExecutor(); | ||
|
|
||
| } | ||
|
|
||
|
|
||
|
|
||
| @Bean(name = "outboxAsyncExecutor") | ||
|
|
||
| public Executor outboxAsyncExecutor() { | ||
|
|
||
| return new SyncTaskExecutor(); | ||
|
|
||
| } | ||
|
|
||
|
|
||
|
|
||
| @Override | ||
|
|
||
| public Executor getAsyncExecutor() { | ||
|
|
||
| return new SyncTaskExecutor(); | ||
|
|
||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
들여쓰기가 일관되지 않습니다.
Lines 23-29, 33-39, 43-49의 메서드들이 비정상적으로 들여쓰기되어 있습니다. 이는 Java 코딩 규칙에 맞지 않으며 가독성을 저해합니다.
🔎 들여쓰기 수정 제안
- @Bean(name = "schedulerAsyncExecutor")
-
- public Executor schedulerAsyncExecutor() {
-
- return new SyncTaskExecutor();
-
- }
-
-
- @Bean(name = "outboxAsyncExecutor")
-
- public Executor outboxAsyncExecutor() {
-
- return new SyncTaskExecutor();
-
- }
-
-
- @Override
-
- public Executor getAsyncExecutor() {
-
- return new SyncTaskExecutor();
-
- }
+ @Bean(name = "schedulerAsyncExecutor")
+ public Executor schedulerAsyncExecutor() {
+ return new SyncTaskExecutor();
+ }
+
+ @Bean(name = "outboxAsyncExecutor")
+ public Executor outboxAsyncExecutor() {
+ return new SyncTaskExecutor();
+ }
+
+ @Override
+ public Executor getAsyncExecutor() {
+ return new SyncTaskExecutor();
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Bean(name = "schedulerAsyncExecutor") | |
| public Executor schedulerAsyncExecutor() { | |
| return new SyncTaskExecutor(); | |
| } | |
| @Bean(name = "outboxAsyncExecutor") | |
| public Executor outboxAsyncExecutor() { | |
| return new SyncTaskExecutor(); | |
| } | |
| @Override | |
| public Executor getAsyncExecutor() { | |
| return new SyncTaskExecutor(); | |
| } | |
| @Bean(name = "schedulerAsyncExecutor") | |
| public Executor schedulerAsyncExecutor() { | |
| return new SyncTaskExecutor(); | |
| } | |
| @Bean(name = "outboxAsyncExecutor") | |
| public Executor outboxAsyncExecutor() { | |
| return new SyncTaskExecutor(); | |
| } | |
| @Override | |
| public Executor getAsyncExecutor() { | |
| return new SyncTaskExecutor(); | |
| } |
🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/config/TestAsyncConfig.java around lines 23-49,
several methods and their @Bean annotations are inconsistently indented; fix by
normalizing indentation to the project's Java style (use 4 spaces per indent
level) so annotations, method signatures, and method bodies align consistently,
remove any extra leading spaces or tabs, and ensure a single blank line between
methods for readability.
| // UserJpaEntity userJpaEntity = userJpaRepository.findById(target.getUserId()).orElseThrow(); | ||
| //// assertThat(userJpaEntity.getFollowerCount()).isEqualTo(1); // 팔로워 수 증가 확인 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
통합 테스트에서 팔로워 카운트 검증이 누락되었습니다.
이벤트 기반 아키텍처로 전환하면서 동기적인 followerCount 검증을 주석 처리했지만, 비동기 처리 결과에 대한 대체 검증이 없습니다. 통합 테스트에서는 다음 중 하나를 검증해야 합니다:
- 이벤트가 발행되었는지 확인 (Outbox 테이블에 레코드 생성 확인)
- 이벤트 처리 후 최종 일관성 확인 (비동기 처리 완료 대기 후 followerCount 검증)
FollowingEventCommandPort의 호출 여부 확인
현재 상태로는 팔로우/언팔로우가 followerCount에 미치는 영향을 전혀 검증하지 않아 테스트 커버리지가 감소했습니다.
💡 제안: Outbox 엔티티 생성 검증 추가
// UserJpaEntity userJpaEntity = userJpaRepository.findById(target.getUserId()).orElseThrow();
//// assertThat(userJpaEntity.getFollowerCount()).isEqualTo(1); // 팔로워 수 증가 확인
+
+ // Outbox에 이벤트가 저장되었는지 확인
+ List<Outbox> outboxEntries = outboxRepository.findByAggregateIdAndEventType(
+ target.getUserId(), "FOLLOW_EVENT");
+ assertThat(outboxEntries).hasSize(1);Also applies to: 90-91
| private UserFollowService userFollowService; | ||
|
|
||
| private FeedNotificationOrchestratorSyncImpl feedNotificationOrchestratorSyncImpl; | ||
| private FollowingEventCommandPort followingEventCommandPort; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
모의 객체의 상호작용 검증이 누락되었습니다.
followingEventCommandPort 모의 객체를 생성하고 서비스에 주입했지만, 테스트에서 이 포트의 메서드 호출을 검증하지 않습니다. 단위 테스트에서는 모든 의존성과의 상호작용을 명시적으로 검증해야 합니다.
🔎 제안: 이벤트 발행 검증 추가
팔로우 테스트(line 70-94)와 언팔로우 테스트(line 103-123)에 다음과 같은 검증을 추가하세요:
// 팔로우 테스트의 // then 섹션에 추가
verify(followingEventCommandPort).publishFollowEvent(eq(userId), eq(targetUserId));
// 언팔로우 테스트의 // then 섹션에 추가
verify(followingEventCommandPort).publishUnfollowEvent(eq(userId), eq(targetUserId));Also applies to: 41-42
🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/user/application/service/UserFollowServiceTest.java
around lines 34 and 41-42, the FollowingEventCommandPort mock is created and
injected but its interactions are not asserted; add verification in the tests'
then sections so the follow test verifies publishFollowEvent was called with
eq(userId) and eq(targetUserId), and the unfollow test verifies
publishUnfollowEvent was called with eq(userId) and eq(targetUserId); ensure you
import and use verify (and eq) from Mockito and keep the verifications in the
respective // then blocks.
| // then | ||
| assertThat(result).isTrue(); | ||
| assertThat(user.getFollowerCount()).isEqualTo(1); // followerCount 증가 | ||
| // assertThat(user.getFollowerCount()).isEqualTo(1); // followerCount 증가 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
주석 처리된 검증을 대체 검증으로 교체해야 합니다.
followerCount 검증을 주석 처리했지만, 이벤트 기반 아키텍처에서는 동기적 상태 변경 대신 이벤트 발행을 검증해야 합니다. 현재 테스트는 팔로우/언팔로우의 핵심 부수 효과를 검증하지 않습니다.
🔎 제안: 이벤트 발행 검증으로 교체
Line 87 대신:
-// assertThat(user.getFollowerCount()).isEqualTo(1); // followerCount 증가
+ verify(followingEventCommandPort, times(1))
+ .publishFollowEvent(eq(userId), eq(targetUserId));Line 121 대신:
-// assertThat(user.getFollowerCount()).isEqualTo(0); // followerCount 감소
+ verify(followingEventCommandPort, times(1))
+ .publishUnfollowEvent(eq(userId), eq(targetUserId));Also applies to: 121-121
🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/user/application/service/UserFollowServiceTest.java
around lines 87 and 121, the test currently comments out the followerCount
assertion; replace those synchronous state assertions with verifications that
the appropriate domain events are published (e.g., FollowCreatedEvent on follow
and FollowRemovedEvent on unfollow). Mock or spy the event publisher used by the
service, perform the follow/unfollow actions in the test, and verify that
publish/publishEvent was called once with the expected event type and payload
(or use argument captor to assert event fields) instead of asserting
followerCount.
#️⃣ 연관된 이슈
📝 작업 내용
이전 PR (#337)에 이어서 팔로잉 동시성 이슈를 EDA 도입을 통해 해결하는 방법을 시도해보았습니다.
데이터 정합성을 맞추기 위해 Outbox 패턴을 도입했는데, 자세한 내용은 노션 참고 부탁드릴게요.
다만, 저번에 카톡으로 말했던 것처럼 막상 도입하다보니 데이터베이스를 업데이트하는 이벤트를 분리함으로써 얻는 이점이 클까?라는 점과 결국 하나의 서버가 버틸 수 있는 RPS(VU)에는 한계가 있다는 것을 느껴서 해당 PR은 머지하지 않고 close할 예정입니다!
📸 스크린샷
💬 리뷰 요구사항
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit
릴리스 노트
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.