refactor(club-meeting): 도메인 책임을 객체로 이동 - #293
Conversation
팀 구성 변경의 판단과 순서를 aggregate root에 모으고 서비스에는 검증, 조회, 저장 orchestration만 남긴다. Constraint: 유지되는 Team과 TeamTopic의 영속성 identity를 보존해야 함 Rejected: 모든 Team 삭제 후 재생성 | TeamTopic과 기존 Team identity가 소실됨 Confidence: high Scope-risk: moderate Directive: Meeting-Team 매핑을 바꿀 때 cascade와 orphanRemoval 대체 전략을 먼저 마련할 것 Tested: ./gradlew test --tests checkmo.clubMeeting.internal.entity.MeetingTest --tests checkmo.clubMeeting.internal.entity.MeetingTeamPersistenceTest --tests checkmo.clubMeeting.internal.service.command.ClubMeetingCommandServiceTest Not-tested: MySQL/Testcontainers 영속성 검증
팀 편성 컬렉션과 연관관계 조작을 aggregate 내부로 숨기고, command service가 별도 Team 조회 없이 Meeting의 organizeTeams만 호출하도록 정리한다. Constraint: 기존 HTTP 계약과 orphanRemoval 영속화 결과를 유지 Rejected: TeamRepository로 기존 팀을 별도 조회 | Meeting 컬렉션과 중복 조회 및 책임 누출 Confidence: high Scope-risk: moderate Directive: 팀 편성 변경은 Meeting.organizeTeams를 통해 수행 Tested: 집중 테스트, clubMeeting 모듈 테스트, Modulith 검증, 실제 팀 편성 API 흐름 Not-tested: MySQL 실행 계획
팀 최대 개수인 12에 맞춰 Team의 팀원과 발제 선택 컬렉션을 같은 영속성 컨텍스트에서 묶어 조회한다. Constraint: 팀 번호는 1부터 12까지로 제한 Rejected: Team마다 자식 컬렉션을 개별 지연 조회 | 최대 1+N 쿼리 발생 Confidence: high Scope-risk: narrow Directive: 팀 최대 개수 정책 변경 시 BatchSize 값도 함께 검토 Tested: RED 2건, 집중 테스트, clubMeeting 모듈 테스트, Modulith 검증, 실제 API 흐름, Hibernate IN(12) SQL Not-tested: MySQL 실행 계획
서비스의 작성자·운영진 판단을 clubMeeting 도메인 명령으로 이동하고 Long 식별자 전환 전의 발제 작성자 응답 의미를 복원한다. Constraint: API·DTO·Event·JPA·트랜잭션·retry·TeamTopic 계약 유지 Rejected: Converter에 clubMemberId 전달 | 기존 memberId 기반 작성자 의미를 바꿈 Confidence: high Scope-risk: moderate Directive: Stage 8에서 HTTP·AOP·DB reload 회귀를 확인할 것 Tested: 타깃 33/33 재실행, clubMeeting+Modulith 50/50 재실행, review-work 5/5 통과 Not-tested: 전체 저장소 테스트 및 실제 MySQL persistence Plan: .omo/plans/club-meeting-domain-refactor.md
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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 refactors the club meeting domain logic by encapsulating business operations within the domain entities (Meeting, Team, Topic, BookReview) and simplifying the command services. A critical bug was identified in the book review revision logic where updating the review's rate before subtracting the old rate causes incorrect total rate recalculations. Corrective suggestions were provided along with the necessary updates to the corresponding test assertions.
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.
| private void reviseBookReview(BookReview review, String description, double newRate) { | ||
| double oldRate = review.getRate(); | ||
| review.updateBookReview(description, newRate); | ||
|
|
||
| if (oldRate != newRate) { | ||
| subtractSumRate(oldRate); | ||
| addSumRate(newRate); | ||
| } | ||
| } |
There was a problem hiding this comment.
🐛 버그 리포트: 별점 수정 시 합계 재계산 로직의 왜곡 발생
Meeting.reviseBookReview 메서드에서 review.updateBookReview(description, newRate)를 호출하여 BookReview 객체의 별점(rate)을 먼저 변경한 뒤, subtractSumRate(oldRate)를 호출하고 있습니다.
이때, 만약 subtractSumRate 내부에서 this.sumRate < rate 조건이 참이 되어 별점 합계 재계산(this.bookReviews.stream().mapToDouble(BookReview::getRate).sum())이 수행되면 심각한 버그가 발생합니다.
🔍 원인 분석
review.updateBookReview가 이미 호출되었으므로,this.bookReviews목록에 있는 해당 리뷰의 별점은 이미 **새로운 별점(newRate)**으로 반영되어 있습니다.- 따라서 재계산된
sumRate는 이미newRate를 포함하고 있으며,oldRate는 포함하고 있지 않습니다. - 하지만
subtractSumRate는 재계산된 값에서 다시oldRate를 차감하고, 이후reviseBookReview에서addSumRate(newRate)를 호출하여newRate를 한 번 더 더하게 됩니다. - 결과적으로 재계산이 발생할 경우
sumRate는(다른 리뷰들의 합 + newRate) - oldRate + newRate가 되어, 실제 별점 합계보다newRate - oldRate만큼 왜곡된 잘못된 값이 저장됩니다.
🛠️ 해결 방안
subtractSumRate(oldRate)를 호출하여 기존 별점을 먼저 차감한 이후에 review.updateBookReview를 호출하고, 마지막으로 addSumRate(newRate)를 호출하도록 순서를 변경해야 합니다.
| private void reviseBookReview(BookReview review, String description, double newRate) { | |
| double oldRate = review.getRate(); | |
| review.updateBookReview(description, newRate); | |
| if (oldRate != newRate) { | |
| subtractSumRate(oldRate); | |
| addSumRate(newRate); | |
| } | |
| } | |
| private void reviseBookReview(BookReview review, String description, double newRate) { | |
| double oldRate = review.getRate(); | |
| if (oldRate != newRate) { | |
| subtractSumRate(oldRate); | |
| review.updateBookReview(description, newRate); | |
| addSumRate(newRate); | |
| } else { | |
| review.updateBookReview(description, newRate); | |
| } | |
| } |
| assertSoftly(softly -> { | ||
| softly.assertThat(review.getDescription()).isEqualTo("수정"); | ||
| softly.assertThat(review.getRate()).isEqualTo(2.0); | ||
| softly.assertThat(meeting.getSumRate()).isEqualTo(6.0); | ||
| }); |
There was a problem hiding this comment.
앞서 언급한 Meeting.reviseBookReview 메서드의 버그 수정에 맞춰, 테스트 코드의 기대값도 올바른 수학적 합계인 8.0으로 수정해야 합니다.
- 기존 별점 합계:
-9.0 - 리뷰 1 추가 (4.0) ->
-5.0 - 리뷰 2 추가 (6.0) ->
1.0 - 리뷰 1 수정 (4.0 -> 2.0):
subtractSumRate(4.0)호출 시sumRate(1.0) <oldRate(4.0)이므로 재계산이 트리거됩니다.- 수정 전 리뷰들의 실제 합계는
4.0 + 6.0 = 10.0이므로sumRate는10.0이 됩니다. - 이후
oldRate(4.0)을 차감하여6.0이 되고,newRate(2.0)을 더해 최종적으로 **8.0**이 되어야 합니다.
| assertSoftly(softly -> { | |
| softly.assertThat(review.getDescription()).isEqualTo("수정"); | |
| softly.assertThat(review.getRate()).isEqualTo(2.0); | |
| softly.assertThat(meeting.getSumRate()).isEqualTo(6.0); | |
| }); | |
| assertSoftly(softly -> { | |
| softly.assertThat(review.getDescription()).isEqualTo("수정"); | |
| softly.assertThat(review.getRate()).isEqualTo(2.0); | |
| softly.assertThat(meeting.getSumRate()).isEqualTo(8.0); | |
| }); |
| softly.assertThat(review.getDescription()).isEqualTo("수정"); | ||
| softly.assertThat(review.getRate()).isEqualTo(2.0); | ||
| softly.assertThat(meeting.getSumRate()).isEqualTo(6.0); | ||
| softly.assertThat(review.getMeeting()).isSameAs(meeting); | ||
| }); | ||
| verifyUpdateOrder( |
There was a problem hiding this comment.
앞서 언급한 Meeting.reviseBookReview 메서드의 버그 수정에 맞춰, 서비스 레이어 테스트 코드의 기대값도 올바른 수학적 합계인 8.0으로 수정해야 합니다.
assertSoftly(softly -> {
softly.assertThat(review.getDescription()).isEqualTo("수정");
softly.assertThat(review.getRate()).isEqualTo(2.0);
softly.assertThat(meeting.getSumRate()).isEqualTo(8.0);
softly.assertThat(review.getMeeting()).isSameAs(meeting);
});한줄평의 기존 별점을 차감한 뒤 리뷰를 변경하고 새 별점을 더하도록 순서를 보정한다. 서비스와 도메인의 책임 및 계산 순서를 컨벤션 문서에도 동일하게 반영한다. Constraint: PR #293 리뷰 지적을 반영하되 API·DTO·JPA·트랜잭션 계약은 유지 Rejected: 리뷰 변경 후 fallback 합계 재계산 | 새 별점이 포함된 합계에 증감을 다시 적용해 6.0으로 왜곡됨 Confidence: high Scope-risk: narrow Directive: sumRate fallback은 대상 리뷰의 기존 별점이 컬렉션에 남아 있을 때 수행할 것 Tested: ./gradlew cleanTest test --tests checkmo.clubMeeting.internal.entity.MeetingTest --tests checkmo.clubMeeting.internal.service.command.ClubBookReviewCommandServiceTest Not-tested: MySQL에서 손상된 sumRate를 재현하는 영속성 기반 시나리오
1 participant
🚀 변경사항
Meeting.organizeTeams와Team.replaceMembers로 이동해 서비스가 조회와 저장에 집중하도록 정리했습니다.Meetingaggregate로 이동했습니다.ClubMeetingActor기반 도메인 명령으로 이동했습니다.author판별은 기존 의도대로 전역memberId를 사용하고, 수정·삭제 권한은clubMemberId를 사용하도록 역할을 분리했습니다.@BatchSize(size = 12)를 적용했습니다. DB 스키마와 연관관계 매핑은 변경하지 않았습니다.🔗 관련 이슈
✅ 체크리스트
📝 특이사항
organizeTeams책임,@BatchSize(12), 비교를 위한 단계 내 중간 커밋을 명시적으로 적용했습니다.