Refactor/#102 hexagonal architecture#103
Conversation
common/domain/application/infrastructure/presentation/bootstrap 6개 모듈로 분리. 패키지명 유지하며 파일만 이동, testFixtures로 Fake 클래스 공유.
bootstrap 모듈로 파일 이동 완료 후 남은 루트 src 잔재 정리
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
제공해주신 Git Diff를 분석한 결과, 모놀리식 아키텍처에서 모듈화된(헥사고날/클린 아키텍처) 구조로의 대규모 리팩토링이 성공적으로 이루어졌음을 확인했습니다. 이는 코드 품질, 유지보수성, 확장성 및 테스트 용이성 측면에서 매우 긍정적인 변화입니다. 특히 도메인 계층의 풍부한 모델링(값 객체 도입), 애플리케이션 계층의 유스케이스 분리, 인프라 계층의 어댑터 패턴 적용, 그리고 읽기 전용 쿼리 최적화를 위한 JOOQ 도입은 매우 인상적입니다. 전반적으로 높은 수준의 코드 품질과 아키텍처 설계가 적용되었습니다. 몇 가지 사소한 개선점이나 확인 사항을 코멘트로 남겼습니다.
|
|
||
| @Override | ||
| @Transactional | ||
| public ApplyResponse execute(UUID userId, UUID applyId) { |
There was a problem hiding this comment.
스터디 게시글(StudyPost)에 먼저 락을 걸고, 그 다음 지원(Apply)에 락을 거는 순서는 데드락 방지에 매우 중요합니다. 다른 트랜잭션이 Apply에 먼저 락을 걸고 StudyPost에 락을 걸려고 할 때 발생할 수 있는 데드락을 효과적으로 방지합니다. 좋은 구현입니다.
|
|
||
| StudyPost post = studyPostRepository.findByIdWithAuthorForUpdate(applyInfo.getPost().getId()) | ||
| .orElseThrow(() -> new CustomException(ErrorCode.POST_NOT_FOUND)); | ||
| post.validateAuthor(userId); |
There was a problem hiding this comment.
post.markFullIfNeeded(approvedCount);와 같이 도메인 로직을 엔티티 내부로 이동시킨 것은 매우 좋은 변화입니다. 도메인 주도 설계 원칙에 부합하며, 도메인 객체의 응집도를 높이고 비즈니스 로직이 여러 서비스에 분산되는 것을 방지합니다.
| @Service | ||
| @RequiredArgsConstructor | ||
| public class PostSearchService { | ||
|
|
There was a problem hiding this comment.
Elasticsearch 검색 서비스에 CircuitBreaker를 적용한 것은 시스템의 복원력을 높이는 좋은 방법입니다. 검색 엔진 장애 시 전체 서비스에 영향을 미치지 않도록 방지합니다.
|
|
||
| @CircuitBreaker(name = "elasticsearch", fallbackMethod = "searchFallback") | ||
| public Page<PostSearchResult> search(String keyword, String techStack, StudyPostStatus status, | ||
| int maxMembers, Pageable pageable) { |
There was a problem hiding this comment.
index(StudyPost post) 메서드가 PostDocument 대신 StudyPost 도메인 객체를 직접 받도록 변경된 점이 좋습니다. 이는 애플리케이션 서비스가 인프라 계층의 특정 문서 모델에 의존하지 않도록 하여 계층 간 결합도를 낮춥니다. StudyPost를 PostDocument로 변환하는 책임은 이제 인프라 계층의 어댑터(PostSearchAdapter)로 이동했습니다.
| private final StudyTeamRepository studyTeamRepository; | ||
| private final TeamMemberRepository teamMemberRepository; | ||
| private final StudyPostRepository studyPostRepository; | ||
| private final UserRepository userRepository; |
There was a problem hiding this comment.
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)과 @Transactional(propagation = Propagation.REQUIRES_NEW)를 함께 사용하여 도메인 이벤트 핸들러가 별도의 트랜잭션으로 실행되도록 한 것은 좋은 패턴입니다. 이는 이벤트 발행 트랜잭션과 이벤트 처리 트랜잭션을 분리하여 시스템의 견고성을 높이고 느슨한 결합을 유지하는 데 도움이 됩니다.
| @Repository | ||
| @RequiredArgsConstructor | ||
| public class StudyPostRepositoryAdapter implements StudyPostRepository { | ||
|
|
There was a problem hiding this comment.
saveAndFlush를 사용하여 즉시 DB에 반영하는 것은 특정 시점에 데이터가 DB에 존재해야 하는 경우(예: 락 획득 전)에 유용합니다. 하지만 불필요한 경우 성능 저하를 일으킬 수 있으므로, 이 위치에서 saveAndFlush가 반드시 필요한지 한 번 더 검토해볼 가치가 있습니다. (예: PostSyncEvent 발행 전 ID가 필요한 경우 등)
| import java.util.UUID; | ||
|
|
||
| public class JooqUtils { | ||
|
|
There was a problem hiding this comment.
UUID를 BINARY로 변환하여 저장하고 쿼리하는 유틸리티 메서드는 MySQL에서 UUID를 효율적으로 다루는 데 필수적입니다. 인덱싱 및 저장 공간 효율성 측면에서 좋은 접근입니다.
|
|
||
| import java.util.List; | ||
|
|
||
| @Component |
There was a problem hiding this comment.
Redis Lua 스크립트를 활용한 슬라이딩 윈도우 방식의 Rate Limit 구현은 매우 효율적이고 정확합니다. Redis 서버에서 모든 로직을 처리하여 네트워크 왕복 횟수를 줄이고, 경쟁 조건 문제를 방지합니다. 성능 병목을 효과적으로 해결하는 좋은 예시입니다.
|
|
||
| public class SseEmitterAdapter implements SseConnection { | ||
|
|
||
| private final SseEmitter emitter; |
There was a problem hiding this comment.
SseConnection 인터페이스를 구현하는 SseEmitterAdapter를 도입하여 SseEmitter와 같은 Spring WebFlux/MVC 특정 클래스를 도메인 계층에서 분리한 것은 좋은 추상화입니다. 이는 프레젠테이션 계층의 기술적 세부 사항이 도메인에 침투하는 것을 방지합니다.
| @@ -19,7 +18,7 @@ public class IdempotencyFilter extends OncePerRequestFilter { | |||
|
|
|||
| private static final Duration TTL = Duration.ofHours(24); | |||
|
|
|||
| private final StringRedisTemplate stringRedisTemplate; | |||
| private final IdempotencyStoragePort idempotencyStoragePort; | |||
There was a problem hiding this comment.
IdempotencyStoragePort 인터페이스를 사용하여 Redis 의존성을 추상화한 것은 좋은 설계입니다. 이를 통해 IdempotencyFilter는 특정 Redis 구현체에 종속되지 않고, 다른 저장소로 쉽게 교체할 수 있습니다.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request restructures the application into a multi-module Gradle project, adopting a clean/hexagonal architecture by separating use cases, domain entities, ports, and adapters. Key improvements include the introduction of transactional outbox patterns, sliding window rate limiting, and event-driven communication. The code review highlights several critical areas for improvement: adding missing post-open validation and removing redundant queries in ApproveApplyService, resolving a race condition in CreateStudyTeamService via pessimistic locking, ensuring Kafka messages are published only after transaction commit in CreateTeamScheduleService, fixing a Sorted Set member collision bug in the Redis rate limiter, and preserving domain model purity by decoupling the Apply entity from the DomainEventPublisher port.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| Apply applyInfo = applyRepository.findByIdWithPostAndApplicant(applyId) | ||
| .orElseThrow(() -> new CustomException(ErrorCode.APPLICATION_NOT_FOUND)); | ||
|
|
||
| StudyPost post = studyPostRepository.findByIdWithAuthorForUpdate(applyInfo.getPost().getId()) | ||
| .orElseThrow(() -> new CustomException(ErrorCode.POST_NOT_FOUND)); | ||
| post.validateAuthor(userId); | ||
|
|
||
| Apply apply = applyRepository.findByIdWithPostAndApplicantForUpdate(applyId) | ||
| .orElseThrow(() -> new CustomException(ErrorCode.APPLICATION_NOT_FOUND)); | ||
| apply.approve(eventPublisher); |
There was a problem hiding this comment.
지원서 승인 시(ApproveApplyService), 두 가지 중요한 개선 사항이 있습니다.
- 비즈니스 검증 누락 (정원 초과 및 마감 여부): 모집글이 이미 마감되었거나 정원이 찬 상태(
FULL또는CLOSED)인지 검증하는 로직이 누락되어 있습니다. 이로 인해 정원을 초과하여 지원서를 승인할 수 있는 비즈니스 오류가 발생할 수 있으므로, 승인 전에post.validateOpen();을 호출해야 합니다. - 불필요한 중복 쿼리 제거: 동일한
applyId로applyRepository를 두 번 조회(findByIdWithPostAndApplicant및findByIdWithPostAndApplicantForUpdate)하고 있습니다. 비관적 락을 사용하는 조회 한 번으로 최적화하여 데이터베이스 라운드트립을 줄일 수 있습니다.
| Apply applyInfo = applyRepository.findByIdWithPostAndApplicant(applyId) | |
| .orElseThrow(() -> new CustomException(ErrorCode.APPLICATION_NOT_FOUND)); | |
| StudyPost post = studyPostRepository.findByIdWithAuthorForUpdate(applyInfo.getPost().getId()) | |
| .orElseThrow(() -> new CustomException(ErrorCode.POST_NOT_FOUND)); | |
| post.validateAuthor(userId); | |
| Apply apply = applyRepository.findByIdWithPostAndApplicantForUpdate(applyId) | |
| .orElseThrow(() -> new CustomException(ErrorCode.APPLICATION_NOT_FOUND)); | |
| apply.approve(eventPublisher); | |
| Apply apply = applyRepository.findByIdWithPostAndApplicantForUpdate(applyId) | |
| .orElseThrow(() -> new CustomException(ErrorCode.APPLICATION_NOT_FOUND)); | |
| StudyPost post = studyPostRepository.findByIdWithAuthorForUpdate(apply.getPost().getId()) | |
| .orElseThrow(() -> new CustomException(ErrorCode.POST_NOT_FOUND)); | |
| post.validateAuthor(userId); | |
| post.validateOpen(); | |
| apply.approve(eventPublisher); |
| public void execute(ApplyApprovedEvent event) { | ||
| if (studyTeamRepository.findByPostId(event.postId()).isPresent()) { | ||
| addMember(event); | ||
| return; | ||
| } | ||
|
|
||
| StudyPost post = studyPostRepository.findByIdWithAuthor(event.postId()) | ||
| .orElseThrow(() -> new CustomException(ErrorCode.POST_NOT_FOUND)); | ||
| StudyTeam team = studyTeamRepository.save(StudyTeam.create(post)); |
There was a problem hiding this comment.
동시성 환경에서 동일한 postId에 대해 여러 지원서가 동시에 승인될 경우, CreateStudyTeamService.execute가 동시에 실행되면서 여러 개의 StudyTeam이 중복 생성되거나 DB 유니크 제약 조건 예외로 인해 일부 멤버가 팀에 추가되지 못하는 레이스 컨디션 버그가 존재합니다.
이를 방지하기 위해, 메서드 진입 시 studyPostRepository.findByIdWithAuthorForUpdate를 호출하여 해당 StudyPost에 대해 비관적 락(Pessimistic Lock)을 먼저 획득함으로써 팀 생성 및 멤버 추가 로직을 직렬화(Serialization)해야 합니다.
| public void execute(ApplyApprovedEvent event) { | |
| if (studyTeamRepository.findByPostId(event.postId()).isPresent()) { | |
| addMember(event); | |
| return; | |
| } | |
| StudyPost post = studyPostRepository.findByIdWithAuthor(event.postId()) | |
| .orElseThrow(() -> new CustomException(ErrorCode.POST_NOT_FOUND)); | |
| StudyTeam team = studyTeamRepository.save(StudyTeam.create(post)); | |
| public void execute(ApplyApprovedEvent event) { | |
| StudyPost post = studyPostRepository.findByIdWithAuthorForUpdate(event.postId()) | |
| .orElseThrow(() -> new CustomException(ErrorCode.POST_NOT_FOUND)); | |
| if (studyTeamRepository.findByPostId(event.postId()).isPresent()) { | |
| addMember(event); | |
| return; | |
| } | |
| StudyTeam team = studyTeamRepository.save(StudyTeam.create(post)); |
| private void notifyAllMembers(UUID teamId, String teamName, UUID scheduledId) { | ||
| String message = teamName + " " + NotificationType.SCHEDULE_CREATED.getDescription(); | ||
| teamMemberRepository.findAllByTeamId(teamId).forEach(member -> { | ||
| String payload = objectMapper.writeValueAsString( | ||
| new NotificationEvent(member.getUser().getId(), NotificationType.SCHEDULE_CREATED, message, scheduledId, 0)); | ||
| Long outboxEventId = outboxEventService.save(KafkaConstants.NOTIFICATION_TOPIC, member.getUser().getId().toString(), payload); | ||
| kafkaProducer.send(outboxEventId, member.getUser().getId(), NotificationType.SCHEDULE_CREATED, message, scheduledId); | ||
| notificationPublisher.send(outboxEventId, member.getUser().getId(), NotificationType.SCHEDULE_CREATED, message, scheduledId); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Transactional Outbox 패턴을 적용했음에도 불구하고, 비즈니스 트랜잭션 범위 내에서 notificationPublisher.send를 통해 Kafka로 메시지를 즉시 발행하고 있습니다. 만약 비즈니스 트랜잭션이 롤백되면 아웃박스 테이블의 데이터는 롤백되지만, Kafka 메시지는 이미 전송되어 '유령 알림(Ghost Notification)'이 발송되는 일관성 문제가 발생합니다.
알림 발송 또한 PostSyncKafkaProducer처럼 @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)을 사용하거나 트랜잭션 커밋 이후에 비동기로 발행하도록 개선해야 합니다.
| public boolean isAllowed(String key, long windowSeconds, long limit) { | ||
| Long result = stringRedisTemplate.execute( | ||
| SLIDING_WINDOW_SCRIPT, | ||
| List.of(key), | ||
| String.valueOf(System.currentTimeMillis()), | ||
| String.valueOf(windowSeconds), | ||
| String.valueOf(limit) | ||
| ); | ||
| return Long.valueOf(1L).equals(result); | ||
| } |
There was a problem hiding this comment.
Redis Sliding Window Rate Limiter 구현에서 ZADD 등록 시 score와 member 모두에 동일한 밀리초 단위 타임스탬프(now)를 사용하고 있습니다. Redis Sorted Set은 member의 중복을 허용하지 않으므로, 동일한 밀리초에 여러 요청이 들어올 경우 하나의 요청으로 취급되어 실제 설정한 limit보다 더 많은 요청이 허용될 수 있습니다.
이를 해결하기 위해 Java 계층에서 요청마다 고유한 식별자(예: UUID.randomUUID().toString())를 생성하여 Redis Lua 스크립트의 member 인자로 전달해 주시기 바랍니다.
| public static Apply create(StudyPost post, User applicant, String message, DomainEventPublisher publisher) { | ||
| Apply apply = Apply.builder() | ||
| .post(post) | ||
| .applicant(applicant) | ||
| .message(message) | ||
| .build(); | ||
| publisher.publish(new ApplyReceivedEvent(post.getId(), post.getAuthor().getId(), post.getTitle())); | ||
| return apply; | ||
| } |
There was a problem hiding this comment.
도메인 엔티티(Apply)의 메서드에 DomainEventPublisher 포트를 직접 전달하여 이벤트를 발행하고 있습니다. 이는 DDD 및 헥사고날 아키텍처 관점에서 도메인 모델이 외부 서비스/포트에 의존하게 만들어 순수성을 해치고 테스트를 어렵게 만듭니다.
대신, Spring Data JPA의 @DomainEvents를 활용하여 엔티티 내부에 이벤트를 축적하고 저장 시 자동으로 발행되게 하거나, 서비스 계층에서 도메인 로직 호출 후 이벤트를 발행하는 방식을 권장합니다.
1 participant
관련 이슈
closes #102
작업 내용
아키텍처 전면 개선
도메인 로직 개선
CQRS 적용
테스트 개선
테스트
참고 사항