Merged
Conversation
- 방장에 해당하는 RoomParticipant create & DB save 하는 코드 추가
- RoomParticipantRepository 의존성 추가 - RoomParticipant DB save 되는지 확인하는 테스트 코드 추가
- 투표 생성한 사람의 userPercentage 값 수정 - 해당 방의 roomPercentage 값 수정 (기록 생성 서비스 코드 참고하였습니다)
…비스 통합 테스트 코드 작성 (#181)
2 tasks
WalkthroughRoom 생성 시 호스트 RoomParticipant를 영속화하도록 변경되었고, Vote/Record 생성 흐름에 방 멤버 검증 및 사용자·방 진행률 업데이트 로직(RoomProgressHelper) 도입 및 관련 테스트 추가·수정이 이루어졌습니다. (46 단어) Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RoomCreateService
participant RoomCommandPort
participant RoomParticipantCommandPort
User->>RoomCreateService: createRoom(command, userId)
RoomCreateService->>RoomCommandPort: save(room)
RoomCommandPort-->>RoomCreateService: savedRoom(with id)
RoomCreateService->>RoomParticipantCommandPort: save(hostWithoutId(userId, savedRoomId))
RoomParticipantCommandPort-->>RoomCreateService: savedParticipant
RoomCreateService-->>User: return savedRoomId
sequenceDiagram
participant User
participant VoteCreateService
participant RoomParticipantValidator
participant RoomQueryPort
participant BookQueryPort
participant VoteCommandPort
participant VoteItemCommandPort
participant RoomProgressHelper
User->>VoteCreateService: createVote(command, userId)
VoteCreateService->>RoomParticipantValidator: validateMember(userId, roomId)
VoteCreateService->>RoomQueryPort: get(roomId)
VoteCreateService->>BookQueryPort: get(bookId)
VoteCreateService->>VoteCommandPort: save(vote)
VoteCreateService->>VoteItemCommandPort: saveAll(voteItems)
VoteCreateService->>RoomProgressHelper: updateUserAndRoomProgress(userId, roomId, page)
VoteCreateService-->>User: return voteId
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these settings in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Test Results392 tests 392 ✅ 30s ⏱️ Results for commit 3b7cbc5. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🔭 Outside diff range comments (1)
src/main/java/konkuk/thip/vote/application/service/VoteCreateService.java (1)
56-63: 투표 항목 유효성 보강 필요 (서비스 레벨)
VoteCreateService#create메서드에서voteItemCreateCommands()로 바로VoteItem을 생성하기 전에 아래 조건을 검증해주세요.
컨트롤러에서 검증하더라도, 서비스 계층에서 한 번 더 보장하면 외부 어댑터가 변경되더라도 도메인 일관성을 유지할 수 있습니다.
- 최소 2개 이상의 항목 존재
- 대/소문자 및 앞뒤 공백 제거 후 중복 금지
예시 (56~63행 사이,
voteItems생성 직전 위치):List<String> names = command.voteItemCreateCommands().stream() .map(VoteCreateCommand.VoteItemCreateCommand::itemName) .map(String::trim) .map(String::toLowerCase) .toList(); if (names.size() < 2) { throw new IllegalArgumentException("투표 항목은 최소 2개 이상이어야 합니다."); } if (names.stream().distinct().count() != names.size()) { throw new IllegalArgumentException("중복된 투표 항목이 존재합니다."); } // 기존 voteItems 생성 로직 List<VoteItem> voteItems = command.voteItemCreateCommands().stream() .map(itemCmd -> VoteItem.withoutId( itemCmd.itemName(), 0, savedVoteId )) .toList(); voteCommandPort.saveAllVoteItems(voteItems);
🧹 Nitpick comments (8)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
106-121: 주석 표현 및 중복 개선 제안
- 주석의 “custom 생성자”는 오해 소지가 있습니다. 팩토리 메서드 표현이 더 적절합니다.
createBook()와 필드가 중복됩니다. 공통 빌더를 분리하면 유지보수성이 좋아집니다.주석 수정:
- /** - * page 값을 지정할 수 있는 custom 생성자 - */ + /** + * page 값을 지정할 수 있는 Book 팩토리 메서드 + */중복 제거(참고용 예시):
// 파일 상단 어딘가에 추가 private static BookJpaEntity.BookJpaEntityBuilder baseBookBuilder() { return BookJpaEntity.builder() .title("책제목") .authorName("저자") .bestSeller(false) .publisher("출판사") .imageUrl("img") .description("설명"); } // 기존 메서드들에서 사용 public static BookJpaEntity createBook() { return baseBookBuilder() .isbn(UUID.randomUUID().toString().replace("-", "").substring(0, 13)) .pageCount(100) .build(); } public static BookJpaEntity createBook(int page) { return baseBookBuilder() .isbn(UUID.randomUUID().toString().replace("-", "").substring(0, 13)) .pageCount(page) .build(); }src/test/java/konkuk/thip/vote/adapter/in/web/VoteCreateControllerTest.java (1)
22-26: 컨트롤러 단위 테스트는 @WebMvcTest가 더 가볍고 빠릅니다현재
@SpringBootTest로 전체 컨텍스트를 올리고 있습니다. 입력 검증만 목적이라면@WebMvcTest(VoteCreateController.class)로 슬라이스 테스트를 권장합니다. 필요 빈은@Import/@MockBean으로 주입하세요.예시:
-@SpringBootTest -@ActiveProfiles("test") -@AutoConfigureMockMvc(addFilters = false) -@DisplayName("[단위] 투표 생성 api controller 단위 테스트") +@WebMvcTest(controllers = VoteCreateController.class) +@ActiveProfiles("test") +@DisplayName("[단위] 투표 생성 api controller 단위 테스트")src/test/java/konkuk/thip/vote/application/service/VoteCreateServiceTest.java (2)
24-26: Service 테스트에 MockMvc 자동설정은 불필요합니다Service 단위 테스트에서
@AutoConfigureMockMvc(addFilters = false)는 컨텍스트 무겁게 할 뿐 사용되지 않습니다. 제거하여 테스트 시간을 단축하세요.
77-113: 음수/비회원 시나리오도 커버하면 검증 완결성이 높아집니다이 PR의 목표(“방 소속 유저만 투표 가능” 검증)와 일치하도록, 방에 속하지 않은 유저가 투표할 때 예외 발생을 검증하는 테스트를 추가하는 것을 권장합니다. THIP 프로젝트 선호에 따라 해당 검증 로직이
RoomParticipantPolicy같은 도메인 서비스로 캡슐화되어 있다면 해당 경로도 확인해 주세요.src/test/java/konkuk/thip/vote/adapter/in/web/VoteCreateApiTest.java (3)
30-30: 불필요한 JdbcTemplate 제거 또는 실제 사용으로 전환JdbcTemplate을 주입하고 있지만 실제로 사용하지 않습니다. 유지할 계획이 없다면 제거해 주세요. 만약 TRUNCATE 기반의 빠른 정리를 도입할 예정이라면 아래 tearDown 최적화 제안을 참고해 실제로 사용해 주세요.
적용 예시:
-import org.springframework.jdbc.core.JdbcTemplate;- @Autowired - private JdbcTemplate jdbcTemplate;Also applies to: 77-79
80-90: tearDown 정리 방식 최적화 제안 (성능/안정성)여러 Repository의 deleteAllInBatch 호출은 테이블 수만큼 쿼리가 발생합니다. 테스트가 늘어날수록 성능에 영향이 큽니다. 선택지:
- 옵션 A(권장): JdbcTemplate로 TRUNCATE + IDENTITY 리셋을 한 번에 수행(테스트 DB가 H2/PG라면 효과적).
- 옵션 B: 공용 DatabaseCleaner 컴포넌트를 만들어 재사용.
- 옵션 C: 통합테스트가 적다면 현 상태 유지.
예시(포스트그레스/H2 유사 환경):
@AfterEach void tearDown() { jdbcTemplate.execute("TRUNCATE TABLE vote_item RESTART IDENTITY CASCADE"); jdbcTemplate.execute("TRUNCATE TABLE vote RESTART IDENTITY CASCADE"); jdbcTemplate.execute("TRUNCATE TABLE room_participant RESTART IDENTITY CASCADE"); jdbcTemplate.execute("TRUNCATE TABLE room RESTART IDENTITY CASCADE"); jdbcTemplate.execute("TRUNCATE TABLE book RESTART IDENTITY CASCADE"); jdbcTemplate.execute("TRUNCATE TABLE \"user\" RESTART IDENTITY CASCADE"); jdbcTemplate.execute("TRUNCATE TABLE category RESTART IDENTITY CASCADE"); jdbcTemplate.execute("TRUNCATE TABLE alias RESTART IDENTITY CASCADE"); }DB마다 문법이 다르므로 환경에 맞게 조정해 주세요.
103-151: 실패 케이스 통합 테스트 추가 제안성공 경로만 검증 중입니다. “방에 속하지 않은 사용자가 투표 생성 시 403/400” 같은 부정 경로를 추가하면 밸리데이션 회귀를 방지할 수 있습니다.
원하시면 컨트롤러/예외 규격에 맞춰 테스트 메서드 스켈레톤을 제공하겠습니다.
src/main/java/konkuk/thip/vote/application/service/VoteCreateService.java (1)
7-8: 팀 선호도 반영: 멤버십 검증을 도메인 정책 서비스로 캡슐화러닝 메모에 따르면 “사용자가 방에 속하는지 검증”은 RoomParticipantPolicy 같은 도메인 서비스로 캡슐화하는 것을 선호합니다. 현재
RoomParticipantValidator가 application 계층에 위치해 있는데, 도메인 정책 서비스로 승격/이관을 고려해 주세요. 동일 정책의 재사용성과 표현력이 좋아집니다.질문:
- 기존에 RoomParticipantPolicy(또는 유사 클래스)가 있는지요? 있다면 해당 정책을 재사용하도록 의존성만 교체하는 방안을 제안합니다.
Also applies to: 28-29, 37-37
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/java/konkuk/thip/room/application/service/RoomCreateService.java(3 hunks)src/main/java/konkuk/thip/vote/application/service/VoteCreateService.java(4 hunks)src/test/java/konkuk/thip/common/util/TestEntityFactory.java(1 hunks)src/test/java/konkuk/thip/room/adapter/in/web/RoomCreateAPITest.java(4 hunks)src/test/java/konkuk/thip/vote/adapter/in/web/VoteCreateApiTest.java(1 hunks)src/test/java/konkuk/thip/vote/adapter/in/web/VoteCreateControllerTest.java(2 hunks)src/test/java/konkuk/thip/vote/application/service/VoteCreateServiceTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#101
File: src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java:36-39
Timestamp: 2025-07-26T06:09:00.850Z
Learning: THIP 프로젝트에서는 "사용자가 방에 속하는지 검증" 로직을 RoomParticipantPolicy 도메인 서비스로 캡슐화하여 재사용성을 높이고 비즈니스 로직의 중복을 방지하는 방식을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#113
File: src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java:38-44
Timestamp: 2025-07-30T14:05:04.945Z
Learning: seongjunnoh는 코드 최적화 제안에 대해 구체적인 기술적 근거와 효율성 차이를 이해하고 싶어하며, 성능 개선 방식에 대한 상세한 설명을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#93
File: src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java:49-114
Timestamp: 2025-07-28T16:44:31.224Z
Learning: seongjunnoh는 코드 중복 문제에 대한 리팩토링 제안을 적극적으로 수용하고 함수형 인터페이스를 활용한 해결책을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#112
File: src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java:272-272
Timestamp: 2025-07-30T10:44:34.115Z
Learning: seongjunnoh는 피드 커서 페이지네이션에서 LocalDateTime 단일 커서 방식을 선호하며, 복합 키 기반 커서보다 구현 단순성과 성능을 우선시한다.
🔇 Additional comments (3)
src/main/java/konkuk/thip/room/application/service/RoomCreateService.java (1)
48-56: 저장된 roomId 반환 및 방장 참가자 저장 흐름은 적절합니다
roomCommandPort.save(room)의 반환값을 재사용해RoomParticipant저장 후 동일 ID를 반환하는 흐름이 명확해졌습니다. 트랜잭션 경계도 적절합니다.src/test/java/konkuk/thip/room/adapter/in/web/RoomCreateAPITest.java (2)
69-71: RoomParticipant 리포지토리 주입 추가 적절합니다방 생성 시 참가자 저장 검증을 위한 주입 추가가 타당합니다.
72-80: 정리 순서 적절외래키 제약을 고려해 RoomParticipant → Room → Book → User → Category → Alias 순으로
deleteAllInBatch()수행하는 것은 안전합니다.
src/main/java/konkuk/thip/room/application/service/RoomCreateService.java
Show resolved
Hide resolved
Comment on lines
49
to
50
| Book book = bookCommandPort.findById(room.getBookId()); | ||
| validateVote(vote, room, book); |
There was a problem hiding this comment.
NPE 가능성: Book 조회 실패 시 명시적 예외 처리 필요
bookCommandPort.findById(room.getBookId())가 null을 반환할 경우 이후 book.getPageCount()에서 NPE가 발생합니다. getByIdOrThrow 형태로 명시적으로 예외를 던지거나 null 체크가 필요합니다.
선호하는 방식 2가지:
- 권장: Port에
getByIdOrThrow(Long id)추가 후 사용
- Book book = bookCommandPort.findById(room.getBookId());
+ Book book = bookCommandPort.getByIdOrThrow(room.getBookId());- 또는 명시적 검증/예외
Book book = bookCommandPort.findById(room.getBookId());
if (book == null) {
throw new IllegalStateException("Book not found: id=" + room.getBookId());
}🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/vote/application/service/VoteCreateService.java
around lines 49 to 50, the call to bookCommandPort.findById(room.getBookId())
may return null, causing a NullPointerException later when accessing book
properties. To fix this, either add and use a getByIdOrThrow(Long id) method in
the port that throws an exception if the book is not found, or explicitly check
if the returned book is null and throw an IllegalStateException with a clear
message before proceeding.
Comment on lines
71
to
87
| private void updateRoomProgress(Vote vote, Room room, Book book) { | ||
| RoomParticipant roomParticipant = roomParticipantCommandPort.getByUserIdAndRoomIdOrThrow(vote.getCreatorId(), vote.getRoomId()); | ||
|
|
||
| if(roomParticipant.updateUserProgress(vote.getPage(), book.getPageCount())) { | ||
| // userPercentage가 업데이트되었으면 Room의 roomPercentage 업데이트 | ||
| List<RoomParticipant> roomParticipantList = roomParticipantCommandPort.findAllByRoomId(vote.getRoomId()); | ||
| Double totalUserPercentage = roomParticipantList.stream() | ||
| .filter(participant -> !roomParticipant.getId().equals(participant.getId())) // 현재 업데이트 중인 사용자 제외 | ||
| .map(RoomParticipant::getUserPercentage) | ||
| .reduce(0.0, Double::sum); | ||
| totalUserPercentage += roomParticipant.getUserPercentage(); | ||
| room.updateRoomPercentage(totalUserPercentage / roomParticipantList.size()); | ||
| } | ||
|
|
||
| roomCommandPort.update(room); | ||
| roomParticipantCommandPort.update(roomParticipant); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
불필요한 DB 쓰기 최소화 및 평균 계산/동시성 개선
- update 조건:
updateUserProgress(...)가 false면 실제 변경이 없으므로 DB update 호출을 생략하는 것이 낫습니다(쓰기 부하 감소). - 평균 계산: 매번 참여자 전부를 로드+합산(O(n)) 대신 DB 집계 쿼리(AVG)로 대체하면 효율/일관성이 좋아집니다. 동시에 여러 투표가 생성되는 상황에서 “마지막 기록 우선” 문제가 발생할 수 있어, DB 레벨 집계 + Room에 낙관적 락(version) 적용을 권합니다.
권장 리팩토링(Port에 집계 쿼리 추가하는 안):
- private void updateRoomProgress(Vote vote, Room room, Book book) {
- RoomParticipant roomParticipant = roomParticipantCommandPort.getByUserIdAndRoomIdOrThrow(vote.getCreatorId(), vote.getRoomId());
-
- if(roomParticipant.updateUserProgress(vote.getPage(), book.getPageCount())) {
- // userPercentage가 업데이트되었으면 Room의 roomPercentage 업데이트
- List<RoomParticipant> roomParticipantList = roomParticipantCommandPort.findAllByRoomId(vote.getRoomId());
- Double totalUserPercentage = roomParticipantList.stream()
- .filter(participant -> !roomParticipant.getId().equals(participant.getId())) // 현재 업데이트 중인 사용자 제외
- .map(RoomParticipant::getUserPercentage)
- .reduce(0.0, Double::sum);
- totalUserPercentage += roomParticipant.getUserPercentage();
- room.updateRoomPercentage(totalUserPercentage / roomParticipantList.size());
- }
-
- roomCommandPort.update(room);
- roomParticipantCommandPort.update(roomParticipant);
- }
+ private void updateRoomProgress(Vote vote, Room room, Book book) {
+ RoomParticipant participant =
+ roomParticipantCommandPort.getByUserIdAndRoomIdOrThrow(vote.getCreatorId(), vote.getRoomId());
+
+ boolean changed = participant.updateUserProgress(vote.getPage(), book.getPageCount());
+ if (!changed) {
+ return; // 변경 없으면 불필요한 update 생략
+ }
+
+ // DB 집계 쿼리로 평균 계산(Port에 새 메서드 필요)
+ double avg = roomParticipantCommandPort.getAverageUserPercentageByRoomId(vote.getRoomId());
+ room.updateRoomPercentage(avg);
+
+ // 실제 변경이 있는 경우에만 update
+ roomParticipantCommandPort.update(participant);
+ roomCommandPort.update(room);
+ }만약 Port 확장이 어렵다면 최소한 스트림 합계를 원시형으로 바꾸세요(박싱 비용 감소):
- Double totalUserPercentage = roomParticipantList.stream()
- .filter(p -> !participant.getId().equals(p.getId()))
- .map(RoomParticipant::getUserPercentage)
- .reduce(0.0, Double::sum);
+ double totalUserPercentage = roomParticipantList.stream()
+ .filter(p -> !participant.getId().equals(p.getId()))
+ .mapToDouble(RoomParticipant::getUserPercentage)
+ .sum();Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/vote/application/service/VoteCreateService.java
around lines 71 to 87, avoid unnecessary database writes by only calling update
on Room and RoomParticipant if updateUserProgress returns true, indicating
actual changes. Replace the current in-memory average calculation of user
percentages by loading all participants with a database-level aggregation query
(e.g., AVG) in the port layer to improve efficiency and consistency.
Additionally, apply optimistic locking (versioning) on the Room entity to handle
concurrent updates safely. If adding aggregation queries to the port is not
feasible, at least optimize the stream sum by using primitive streams to reduce
boxing overhead.
Comment on lines
+283
to
314
| @Test | ||
| @DisplayName("방 생성에 성공하면, 방장의 정보가 DB에 저장된다.") | ||
| @Transactional // RoomParticipant -> Room, User 의 manyToOne 지연로딩을 위해 추가 | ||
| void room_create_room_participant_save_success() throws Exception { | ||
| //given : user, category, pageCount값이 있는 book 생성, request 생성 | ||
| saveUserAndCategory(); | ||
| saveBookWithPageCount(); | ||
|
|
||
| Long userId = userJpaRepository.findAll().get(0).getUserId(); | ||
| Long bookId = bookJpaRepository.findAll().get(0).getBookId(); | ||
| Long categoryId = categoryJpaRepository.findAll().get(0).getCategoryId(); | ||
|
|
||
| Map<String, Object> request = buildRoomCreateRequest(); | ||
|
|
||
| //when | ||
| ResultActions result = mockMvc.perform(post("/rooms") | ||
| .requestAttr("userId", userId) | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .content(objectMapper.writeValueAsString(request) | ||
| )); | ||
|
|
||
| //then | ||
| result.andExpect(status().isOk()) | ||
| .andExpect(jsonPath("$.data.roomId").exists()); | ||
|
|
||
| RoomParticipantJpaEntity roomParticipantJpaEntity = roomParticipantJpaRepository.findAll().get(0); | ||
| assertThat(roomParticipantJpaEntity.getUserJpaEntity().getUserId()).isEqualTo(userId); | ||
|
|
||
| RoomJpaEntity savedRoomJpaEntity = roomJpaRepository.findAll().get(0); | ||
| assertThat(roomParticipantJpaEntity.getRoomJpaEntity().getRoomId()).isEqualTo(savedRoomJpaEntity.getRoomId()); | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
검증 보강 제안: 역할/초기값까지 확인하고 조회도 더 견고하게
현재는 참가자-유저/방 매핑만 검증합니다. 다음을 추가하면 회귀를 더 잘 잡을 수 있습니다.
- 역할이 HOST인지 검증
- 초기값(예: currentPage=0, userPercentage=0.0) 검증
findAll().get(0)대신 응답에서 roomId를 파싱해(userId, roomId)로 조회하면 테스트가 견고해집니다
예시 수정:
result.andExpect(status().isOk())
.andExpect(jsonPath("$.data.roomId").exists());
- RoomParticipantJpaEntity roomParticipantJpaEntity = roomParticipantJpaRepository.findAll().get(0);
- assertThat(roomParticipantJpaEntity.getUserJpaEntity().getUserId()).isEqualTo(userId);
-
- RoomJpaEntity savedRoomJpaEntity = roomJpaRepository.findAll().get(0);
- assertThat(roomParticipantJpaEntity.getRoomJpaEntity().getRoomId()).isEqualTo(savedRoomJpaEntity.getRoomId());
+ String json = result.andReturn().getResponse().getContentAsString();
+ Long roomId = objectMapper.readTree(json).path("data").path("roomId").asLong();
+
+ RoomParticipantJpaEntity roomParticipant = roomParticipantJpaRepository
+ .findByUserIdAndRoomId(userId, roomId)
+ .orElseThrow();
+ assertThat(roomParticipant.getUserJpaEntity().getUserId()).isEqualTo(userId);
+ assertThat(roomParticipant.getRoomJpaEntity().getRoomId()).isEqualTo(roomId);
+ // 초기 상태 검증(도메인 기본값에 맞게 조정)
+ assertThat(roomParticipant.getRoomParticipantRole()).isNotNull(); // .isEqualTo(RoomParticipantRole.HOST) 권장
+ assertThat(roomParticipant.getCurrentPage()).isZero();
+ assertThat(roomParticipant.getUserPercentage()).isEqualTo(0.0);🏁 Script executed:
#!/bin/bash
set -e
# RoomParticipantJpaEntity 클래스 위치 확인
rg "class RoomParticipantJpaEntity" -n
# RoomParticipantJpaRepository 인터페이스 및 findByUserIdAndRoomId 메서드 존재 여부 확인
rg "interface RoomParticipantJpaRepository" -n
rg "findByUserIdAndRoomId" -n
# 엔티티 필드 및 기본값 설정 확인
rg "currentPage" -n src/main/java
rg "userPercentage" -n src/main/javaLength of output: 8152
아래 스크립트로 RoomParticipantJpaEntity의 역할 필드 기본값을 확인해보겠습니다.
🏁 Script executed:
#!/bin/bash
set -e
# 역할(role) 필드 선언 및 기본값 확인
rg "roomParticipantRole" -n src/main/java/konkuk/thip/room/adapter/out/jpa/RoomParticipantJpaEntity.java
# 엔티티 파일 상단 일부 출력
sed -n '1,200p' src/main/java/konkuk/thip/room/adapter/out/jpa/RoomParticipantJpaEntity.javaLength of output: 2311
아래 스크립트로 도메인 RoomParticipant의 기본 역할 설정과 방 생성 서비스(RoomCreateService)에서 역할이 어떻게 초기화되는지 확인해 보겠습니다.
🏁 Script executed:
#!/bin/bash
set -e
# 1. 도메인 RoomParticipant 클래스 내용 확인
rg "class RoomParticipant" -n src/main/java
sed -n '1,200p' src/main/java/konkuk/thip/room/domain/RoomParticipant.java
# 2. 방 생성 서비스에서 RoomParticipant를 생성하는 로직 확인
rg "class RoomCreateService" -n src/main/java
sed -n '1,200p' src/main/java/konkuk/thip/room/application/service/RoomCreateService.javaLength of output: 6197
검증 보강 제안: 역할/초기값 검증 및 조회 견고화
현재 테스트는 참가자–유저/방 매핑만 검증하고 있습니다. 아래 항목을 추가해 회귀를 더욱 잘 잡아주세요.
- 방장 역할이
HOST인지 검증 - 초기 상태인
currentPage = 0,userPercentage = 0.0검증 findAll().get(0)대신, 응답에서roomId를 파싱한 뒤
findByUserIdAndRoomId(userId, roomId)로 조회
예시 수정:
result.andExpect(status().isOk())
.andExpect(jsonPath("$.data.roomId").exists());
- RoomParticipantJpaEntity roomParticipantJpaEntity = roomParticipantJpaRepository.findAll().get(0);
- assertThat(roomParticipantJpaEntity.getUserJpaEntity().getUserId()).isEqualTo(userId);
-
- RoomJpaEntity savedRoomJpaEntity = roomJpaRepository.findAll().get(0);
- assertThat(roomParticipantJpaEntity.getRoomJpaEntity().getRoomId()).isEqualTo(savedRoomJpaEntity.getRoomId());
+ String json = result.andReturn().getResponse().getContentAsString();
+ Long roomId = objectMapper.readTree(json).path("data").path("roomId").asLong();
+
+ RoomParticipantJpaEntity roomParticipant = roomParticipantJpaRepository
+ .findByUserIdAndRoomId(userId, roomId)
+ .orElseThrow();
+ assertThat(roomParticipant.getUserJpaEntity().getUserId()).isEqualTo(userId);
+ assertThat(roomParticipant.getRoomJpaEntity().getRoomId()).isEqualTo(roomId);
+ assertThat(roomParticipant.getRoomParticipantRole()).isEqualTo(RoomParticipantRole.HOST);
+ assertThat(roomParticipant.getCurrentPage()).isZero();
+ assertThat(roomParticipant.getUserPercentage()).isEqualTo(0.0);📝 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.
Suggested change
| @Test | |
| @DisplayName("방 생성에 성공하면, 방장의 정보가 DB에 저장된다.") | |
| @Transactional // RoomParticipant -> Room, User 의 manyToOne 지연로딩을 위해 추가 | |
| void room_create_room_participant_save_success() throws Exception { | |
| //given : user, category, pageCount값이 있는 book 생성, request 생성 | |
| saveUserAndCategory(); | |
| saveBookWithPageCount(); | |
| Long userId = userJpaRepository.findAll().get(0).getUserId(); | |
| Long bookId = bookJpaRepository.findAll().get(0).getBookId(); | |
| Long categoryId = categoryJpaRepository.findAll().get(0).getCategoryId(); | |
| Map<String, Object> request = buildRoomCreateRequest(); | |
| //when | |
| ResultActions result = mockMvc.perform(post("/rooms") | |
| .requestAttr("userId", userId) | |
| .contentType(MediaType.APPLICATION_JSON) | |
| .content(objectMapper.writeValueAsString(request) | |
| )); | |
| //then | |
| result.andExpect(status().isOk()) | |
| .andExpect(jsonPath("$.data.roomId").exists()); | |
| RoomParticipantJpaEntity roomParticipantJpaEntity = roomParticipantJpaRepository.findAll().get(0); | |
| assertThat(roomParticipantJpaEntity.getUserJpaEntity().getUserId()).isEqualTo(userId); | |
| RoomJpaEntity savedRoomJpaEntity = roomJpaRepository.findAll().get(0); | |
| assertThat(roomParticipantJpaEntity.getRoomJpaEntity().getRoomId()).isEqualTo(savedRoomJpaEntity.getRoomId()); | |
| } | |
| } | |
| @Test | |
| @DisplayName("방 생성에 성공하면, 방장의 정보가 DB에 저장된다.") | |
| @Transactional // RoomParticipant -> Room, User 의 manyToOne 지연로딩을 위해 추가 | |
| void room_create_room_participant_save_success() throws Exception { | |
| //given : user, category, pageCount값이 있는 book 생성, request 생성 | |
| saveUserAndCategory(); | |
| saveBookWithPageCount(); | |
| Long userId = userJpaRepository.findAll().get(0).getUserId(); | |
| Long bookId = bookJpaRepository.findAll().get(0).getBookId(); | |
| Long categoryId = categoryJpaRepository.findAll().get(0).getCategoryId(); | |
| Map<String, Object> request = buildRoomCreateRequest(); | |
| //when | |
| ResultActions result = mockMvc.perform(post("/rooms") | |
| .requestAttr("userId", userId) | |
| .contentType(MediaType.APPLICATION_JSON) | |
| .content(objectMapper.writeValueAsString(request) | |
| )); | |
| //then | |
| result.andExpect(status().isOk()) | |
| .andExpect(jsonPath("$.data.roomId").exists()); | |
| String json = result.andReturn().getResponse().getContentAsString(); | |
| Long roomId = objectMapper.readTree(json).path("data").path("roomId").asLong(); | |
| RoomParticipantJpaEntity roomParticipant = roomParticipantJpaRepository | |
| .findByUserIdAndRoomId(userId, roomId) | |
| .orElseThrow(); | |
| assertThat(roomParticipant.getUserJpaEntity().getUserId()).isEqualTo(userId); | |
| assertThat(roomParticipant.getRoomJpaEntity().getRoomId()).isEqualTo(roomId); | |
| assertThat(roomParticipant.getRoomParticipantRole()).isEqualTo(RoomParticipantRole.HOST); | |
| assertThat(roomParticipant.getCurrentPage()).isZero(); | |
| assertThat(roomParticipant.getUserPercentage()).isEqualTo(0.0); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/room/adapter/in/web/RoomCreateAPITest.java around
lines 283 to 314, enhance the test by verifying that the participant's role is
HOST and that currentPage and userPercentage are initialized to 0 and 0.0
respectively. Also, replace the use of findAll().get(0) with extracting the
roomId from the response JSON and then querying the repository using
findByUserIdAndRoomId(userId, roomId) to make the retrieval more robust and
precise.
Comment on lines
+92
to
+101
| private void saveUserAndRoom() { | ||
| AliasJpaEntity alias = aliasJpaRepository.save(TestEntityFactory.createScienceAlias()); | ||
| UserJpaEntity user = userJpaRepository.save(TestEntityFactory.createUser(alias, "user")); | ||
|
|
||
| BookJpaEntity book = bookJpaRepository.save(TestEntityFactory.createBook()); | ||
| CategoryJpaEntity category = categoryJpaRepository.save(TestEntityFactory.createScienceCategory(alias)); | ||
| RoomJpaEntity room = roomJpaRepository.save(TestEntityFactory.createRoom(book, category)); | ||
|
|
||
| roomParticipantJpaRepository.save(TestEntityFactory.createRoomParticipant(room, user, RoomParticipantRole.MEMBER, 0.0)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
findAll().get(0) 의존 제거 — 저장 시점의 식별자 반환하도록 리팩토링
엔티티 ID를 findAll().get(0)으로 얻는 것은 비결정적입니다(JPA 구현/정렬에 의존). 저장 시점에 식별자를 반환받아 사용하세요.
예시 리팩토링:
- private void saveUserAndRoom() {
+ private record Ids(Long userId, Long roomId) {}
+ private Ids saveUserAndRoom() {
AliasJpaEntity alias = aliasJpaRepository.save(TestEntityFactory.createScienceAlias());
UserJpaEntity user = userJpaRepository.save(TestEntityFactory.createUser(alias, "user"));
BookJpaEntity book = bookJpaRepository.save(TestEntityFactory.createBook());
CategoryJpaEntity category = categoryJpaRepository.save(TestEntityFactory.createScienceCategory(alias));
RoomJpaEntity room = roomJpaRepository.save(TestEntityFactory.createRoom(book, category));
roomParticipantJpaRepository.save(TestEntityFactory.createRoomParticipant(room, user, RoomParticipantRole.MEMBER, 0.0));
- }
+ return new Ids(user.getUserId(), room.getRoomId());
+ }- saveUserAndRoom();
+ Ids ids = saveUserAndRoom();- Long userId = userJpaRepository.findAll().get(0).getUserId();
- Long roomId = roomJpaRepository.findAll().get(0).getRoomId();
+ Long userId = ids.userId();
+ Long roomId = ids.roomId();Also applies to: 107-107, 121-123
🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/vote/adapter/in/web/VoteCreateApiTest.java around
lines 92 to 101, the method saveUserAndRoom currently does not return any entity
references, leading to reliance on findAll().get(0) to obtain entity IDs later,
which is non-deterministic. Refactor this method to return the saved entities or
their IDs directly at the time of saving, so subsequent code can use these
returned identifiers instead of querying with findAll().get(0). Apply the same
approach to the code at lines 107 and 121-123.
Comment on lines
+139
to
+141
| VoteJpaEntity voteJpaEntity = voteJpaRepository.findById(voteId).orElse(null); | ||
| List<VoteItemJpaEntity> voteItemJpaEntityList = voteItemJpaRepository.findAllByVoteJpaEntity_PostId(voteJpaEntity.getPostId()); | ||
|
|
There was a problem hiding this comment.
NPE 위험 — 조회 실패 시 즉시 실패하도록 처리
orElse(null) 후 즉시 사용하면 NPE 가능성이 있습니다. 조회 실패 시 테스트가 명확히 실패하도록 orElseThrow를 사용하세요.
- VoteJpaEntity voteJpaEntity = voteJpaRepository.findById(voteId).orElse(null);
- List<VoteItemJpaEntity> voteItemJpaEntityList = voteItemJpaRepository.findAllByVoteJpaEntity_PostId(voteJpaEntity.getPostId());
+ VoteJpaEntity voteJpaEntity = voteJpaRepository.findById(voteId).orElseThrow();
+ List<VoteItemJpaEntity> voteItemJpaEntityList =
+ voteItemJpaRepository.findAllByVoteJpaEntity_PostId(voteJpaEntity.getPostId());📝 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.
Suggested change
| VoteJpaEntity voteJpaEntity = voteJpaRepository.findById(voteId).orElse(null); | |
| List<VoteItemJpaEntity> voteItemJpaEntityList = voteItemJpaRepository.findAllByVoteJpaEntity_PostId(voteJpaEntity.getPostId()); | |
| VoteJpaEntity voteJpaEntity = voteJpaRepository.findById(voteId).orElseThrow(); | |
| List<VoteItemJpaEntity> voteItemJpaEntityList = | |
| voteItemJpaRepository.findAllByVoteJpaEntity_PostId(voteJpaEntity.getPostId()); |
🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/vote/adapter/in/web/VoteCreateApiTest.java around
lines 139 to 141, the code uses orElse(null) which can cause a
NullPointerException if the entity is not found. Replace orElse(null) with
orElseThrow to immediately throw an exception when the voteJpaEntity is not
found, ensuring the test fails clearly and avoids NPE.
Comment on lines
+147
to
+149
| assertThat(voteItemJpaEntityList).hasSize(2) | ||
| .extracting(VoteItemJpaEntity::getItemName) | ||
| .containsExactlyInAnyOrder("찬성", "반대"); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
도메인 목표 검증 보강: 참여자/방 진행률(userPercentage/roomPercentage) 갱신도 검증 필요
PR 목적에 “투표 생성 이후 사용자 userPercentage 및 방 roomPercentage 갱신”이 포함되어 있습니다. API 통합 테스트에서도 이를 검증하면 회귀를 막을 수 있습니다.
예시(스케일 가정: 0~100 퍼센트, 소수 오차 허용). 해당 엔티티/필드명이 다르면 맞게 조정해 주세요.
// 추가 import
import static org.assertj.core.api.Assertions.within;
// 추가 검증 코드 (기존 항목명 검증 뒤에 이어서)
RoomJpaEntity roomEntity = roomJpaRepository.findById(roomId).orElseThrow();
int pageCount = bookJpaRepository.findAll().get(0).getPageCount();
double expectedUserPct = (page * 100.0) / pageCount;
// userPercentage
var participant = roomParticipantJpaRepository.findAll().stream()
.filter(p -> p.getRoomJpaEntity().getRoomId().equals(roomId)
&& p.getUserJpaEntity().getUserId().equals(userId))
.findFirst()
.orElseThrow();
assertThat(participant.getUserPercentage()).isCloseTo(expectedUserPct, within(0.01));
// roomPercentage (단일 참여자일 경우 userPercentage와 동일)
assertThat(roomEntity.getRoomPercentage()).isCloseTo(expectedUserPct, within(0.01));주의:
- 프로젝트가 0~1 스케일을 사용한다면 기대값을 100으로 나누세요.
- 참여자가 여러 명인 경우 roomPercentage는 평균이어야 합니다(서비스 로직과 일치하는지 확인).
다음 스크립트를 통해 userPercentage 및 roomPercentage 필드 존재 여부와 테스트 코드에서 관련 리포지토리가 사용되는지를 확인해 주세요.
🏁 Script executed:
#!/bin/bash
# 도메인 엔티티에 퍼센트 필드가 정의되어 있는지 확인
rg "class RoomParticipantJpaEntity" -A5
rg "userPercentage" -A5
rg "class RoomJpaEntity" -A5
rg "roomPercentage" -A5
# 테스트 코드에서 리포지토리 주입 및 사용 여부 확인
rg "roomJpaRepository" -A3 src/test/java/konkuk/thip/vote/adapter/in/web/VoteCreateApiTest.java
rg "roomParticipantJpaRepository" -A3 src/test/java/konkuk/thip/vote/adapter/in/web/VoteCreateApiTest.javaLength of output: 54107
VoteCreateApiTest에 userPercentage/roomPercentage 검증 추가 필요
VoteCreateApiTest.java의 Vote 항목 검증(assertThat(voteItemJpaEntityList)…) 직후에 API 호출 후 도메인 목표인 사용자 진행률(userPercentage)과 방 진행률(roomPercentage) 갱신을 검증하도록 아래 코드를 추가해 주세요.
파일: src/test/java/konkuk/thip/vote/adapter/in/web/VoteCreateApiTest.java
추가 위치: 라인 147–149 이후
+ // 추가 import
+ import static org.assertj.core.api.Assertions.within;
+
assertThat(voteItemJpaEntityList).hasSize(2)
.extracting(VoteItemJpaEntity::getItemName)
.containsExactlyInAnyOrder("찬성", "반대");
+
+ // --- userPercentage 및 roomPercentage 검증 ---
+ // 방·책 정보 조회
+ RoomJpaEntity roomEntity = roomJpaRepository.findById(roomId).orElseThrow();
+ BookJpaEntity bookEntity = bookJpaRepository.findById(roomEntity.getBookJpaEntity().getBookId())
+ .orElseThrow();
+ int totalPageCount = bookEntity.getPageCount();
+ // 투표 요청 페이지(page)는 테스트 내 변수명을 맞춰 조정하세요.
+ double expectedUserPct = (page * 100.0) / totalPageCount;
+
+ // 참여자(userId) 조회
+ var participant = roomParticipantJpaRepository.findAll().stream()
+ .filter(p -> p.getRoomJpaEntity().getRoomId().equals(roomId)
+ && p.getUserJpaEntity().getUserId().equals(userId))
+ .findFirst()
+ .orElseThrow();
+ // userPercentage 갱신 확인 (오차 허용)
+ assertThat(participant.getUserPercentage())
+ .isCloseTo(expectedUserPct, within(0.01));
+ // 단일 참여자일 땐 roomPercentage == userPercentage
+ assertThat(roomEntity.getRoomPercentage())
+ .isCloseTo(expectedUserPct, within(0.01));주의사항:
- 프로젝트가 0~1 스케일인 경우
expectedUserPct/100으로 조정하세요. - 다수 참여자 로직인 경우 roomPercentage는 평균이 되도록 테스트를 확장해 주세요.
📝 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.
Suggested change
| assertThat(voteItemJpaEntityList).hasSize(2) | |
| .extracting(VoteItemJpaEntity::getItemName) | |
| .containsExactlyInAnyOrder("찬성", "반대"); | |
| // at the top of the file, add: | |
| import static org.assertj.core.api.Assertions.within; | |
| assertThat(voteItemJpaEntityList).hasSize(2) | |
| .extracting(VoteItemJpaEntity::getItemName) | |
| .containsExactlyInAnyOrder("찬성", "반대"); | |
| // --- userPercentage 및 roomPercentage 검증 --- | |
| // 방·책 정보 조회 | |
| RoomJpaEntity roomEntity = roomJpaRepository.findById(roomId).orElseThrow(); | |
| BookJpaEntity bookEntity = | |
| bookJpaRepository.findById(roomEntity.getBookJpaEntity().getBookId()) | |
| .orElseThrow(); | |
| int totalPageCount = bookEntity.getPageCount(); | |
| // 투표 요청 페이지(page)는 테스트 내 변수명을 맞춰 조정하세요. | |
| double expectedUserPct = (page * 100.0) / totalPageCount; | |
| // 참여자(userId) 조회 | |
| var participant = roomParticipantJpaRepository.findAll().stream() | |
| .filter(p -> p.getRoomJpaEntity().getRoomId().equals(roomId) | |
| && p.getUserJpaEntity().getUserId().equals(userId)) | |
| .findFirst() | |
| .orElseThrow(); | |
| // userPercentage 갱신 확인 (오차 허용) | |
| assertThat(participant.getUserPercentage()) | |
| .isCloseTo(expectedUserPct, within(0.01)); | |
| // 단일 참여자일 땐 roomPercentage == userPercentage | |
| assertThat(roomEntity.getRoomPercentage()) | |
| .isCloseTo(expectedUserPct, within(0.01)); |
🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/vote/adapter/in/web/VoteCreateApiTest.java after
lines 147 to 149, add assertions to verify that userPercentage and
roomPercentage are correctly updated after the API call. Retrieve the updated
domain object or relevant fields and assert that userPercentage and
roomPercentage reflect the expected progress values. If the project uses a 0 to
1 scale, adjust expected values by dividing by 100. For multiple participants,
ensure roomPercentage is tested as the average progress. Insert these checks
immediately following the existing voteItemJpaEntityList assertions.
Comment on lines
77
to
113
| @Test | ||
| @DisplayName("유저가 투표를 생성하면, 해당 유저의 [RoomParticipant의 currentPage, userPercentage]와 해당 방의 [Room의 roomPercentage] 값이 변경된다.") | ||
| void vote_create_room_participant_and_room_percentage_update() throws Exception { | ||
| //given | ||
| AliasJpaEntity alias = aliasJpaRepository.save(TestEntityFactory.createScienceAlias()); | ||
| UserJpaEntity me = userJpaRepository.save(TestEntityFactory.createUser(alias, "me")); | ||
|
|
||
| BookJpaEntity book = bookJpaRepository.save(TestEntityFactory.createBook(369)); | ||
| CategoryJpaEntity category = categoryJpaRepository.save(TestEntityFactory.createScienceCategory(alias)); | ||
| RoomJpaEntity room = roomJpaRepository.save(TestEntityFactory.createRoom(book, category)); | ||
|
|
||
| roomParticipantJpaRepository.save(TestEntityFactory.createRoomParticipant(room, me, RoomParticipantRole.MEMBER, 0.0)); | ||
|
|
||
| UserJpaEntity user = userJpaRepository.save(TestEntityFactory.createUser(alias, "user")); | ||
| roomParticipantJpaRepository.save(TestEntityFactory.createRoomParticipant(room, user, RoomParticipantRole.MEMBER, 7.7)); // userPercentage = 7.7 인 RoomParticipant 생성 | ||
|
|
||
| //when | ||
| List<VoteCreateCommand.VoteItemCreateCommand> voteItems = List.of( | ||
| new VoteCreateCommand.VoteItemCreateCommand("투표 항목1"), | ||
| new VoteCreateCommand.VoteItemCreateCommand("투표 항목2") | ||
| ); | ||
|
|
||
| VoteCreateCommand command = new VoteCreateCommand( | ||
| me.getUserId(), room.getRoomId(), 89, false, "투표 내용", voteItems | ||
| ); | ||
|
|
||
| //then | ||
| voteCreateService.createVote(command); | ||
|
|
||
| //then | ||
| RoomParticipantJpaEntity roomParticipant = roomParticipantJpaRepository.findByUserIdAndRoomId(me.getUserId(), room.getRoomId()).get(); | ||
| RoomJpaEntity refreshRoom = roomJpaRepository.findAll().get(0); | ||
|
|
||
| // userPercentage, roomPercentage 값 update 확인 | ||
| assertThat(roomParticipant.getUserPercentage()).isEqualTo((double) 89 / 369 * 100); | ||
| assertThat(refreshRoom.getRoomPercentage()).isEqualTo((7.7 + (double) 89 / 369 * 100) / 2); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
실수 비교 정확도와 검증 범위 보완 필요
- Double을
isEqualTo로 비교하면 정밀도 문제로 플레키해질 수 있습니다.isCloseTo(..., within(…))로 여유값을 두세요. - 테스트 이름에
currentPage갱신이 포함되어 있으나 검증이 없습니다.currentPage == 89도 함께 검증하세요. - 메서드 시그니처의
throws Exception불필요합니다.
수정 예시:
- void vote_create_room_participant_and_room_percentage_update() throws Exception {
+ void vote_create_room_participant_and_room_percentage_update() {
@@
- assertThat(roomParticipant.getUserPercentage()).isEqualTo((double) 89 / 369 * 100);
- assertThat(refreshRoom.getRoomPercentage()).isEqualTo((7.7 + (double) 89 / 369 * 100) / 2);
+ assertThat(roomParticipant.getCurrentPage()).isEqualTo(89);
+ assertThat(roomParticipant.getUserPercentage())
+ .isCloseTo((double) 89 / 369 * 100, within(1e-6));
+ assertThat(refreshRoom.getRoomPercentage())
+ .isCloseTo((7.7 + (double) 89 / 369 * 100) / 2, within(1e-6));추가로 필요한 정적 임포트:
import static org.assertj.core.api.Assertions.within;🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/vote/application/service/VoteCreateServiceTest.java
around lines 77 to 113, the test uses isEqualTo for double comparisons which can
cause flaky results due to precision issues; replace these with isCloseTo and
within to allow a tolerance margin. Also, the test name mentions verifying
currentPage update but does not assert it; add an assertion to check currentPage
equals 89. Finally, remove the unnecessary throws Exception declaration from the
test method signature. Add the static import for within from AssertJ to support
the tolerance check.
hd0rable
reviewed
Aug 11, 2025
Member
hd0rable
left a comment
hd0rable
left a comment
There was a problem hiding this comment.
수고하셨습니다!! 간단히 리뷰남겼는데 확인해주시면 감사하겠습니당~!👍🏻👍🏻
| @Override | ||
| //todo UserRoom 업데이트 로직 추가 필요!! | ||
| public VoteCreateResult createVote(VoteCreateCommand command) { | ||
| roomParticipantValidator.validateUserIsRoomMember(command.roomId(), command.userId()); |
| roomParticipantCommandPort.update(roomParticipant); | ||
| } | ||
|
|
||
| private void validateVote(Vote vote, Room room, Book book) { |
Member
There was a problem hiding this comment.
혹시 이거 room,book은 안쓰는데 파라미터로 받고있는 이유가 따로 있나요??
Contributor
There was a problem hiding this comment.
book은 책 전체 페이지를 가져올때 사용중인데 room은 사용하지 않는 것 같네요!
Collaborator
Author
There was a problem hiding this comment.
room 파라미터로 넘기는 부분 수정했습니다!
| private void validateVote(Vote vote) { | ||
| Room room = roomCommandPort.getByIdOrThrow(vote.getRoomId()); | ||
| Book book = bookCommandPort.findById(room.getBookId()); | ||
| private void updateRoomProgress(Vote vote, Room room, Book book) { |
Member
There was a problem hiding this comment.
p2: 이거 기록생성,투표 생성시에 둘다 로직 같은 걸루 알고있는데 헬퍼서비스 도입해서 중복코드 삭제하는 것은 어떨까요??
Contributor
There was a problem hiding this comment.
두 로직을 공통으로 묶어서 리팩토링하고 가면 나중에 유지보수하기 편할 것 같긴하네요!
| } | ||
|
|
||
| @Test | ||
| @DisplayName("유저가 투표를 생성하면, 해당 유저의 [RoomParticipant의 currentPage, userPercentage]와 해당 방의 [Room의 roomPercentage] 값이 변경된다.") |
buzz0331
previously approved these changes
Aug 13, 2025
Contributor
buzz0331
left a comment
buzz0331
left a comment
There was a problem hiding this comment.
수고하셨습니다~ 희진님이 리뷰 대부분 남겨주셔서 그거 참고하시면 될 것 같아욥!
src/main/java/konkuk/thip/room/application/service/RoomCreateService.java
Show resolved
Hide resolved
| private void validateVote(Vote vote) { | ||
| Room room = roomCommandPort.getByIdOrThrow(vote.getRoomId()); | ||
| Book book = bookCommandPort.findById(room.getBookId()); | ||
| private void updateRoomProgress(Vote vote, Room room, Book book) { |
Contributor
There was a problem hiding this comment.
두 로직을 공통으로 묶어서 리팩토링하고 가면 나중에 유지보수하기 편할 것 같긴하네요!
| roomParticipantCommandPort.update(roomParticipant); | ||
| } | ||
|
|
||
| private void validateVote(Vote vote, Room room, Book book) { |
Contributor
There was a problem hiding this comment.
book은 책 전체 페이지를 가져올때 사용중인데 room은 사용하지 않는 것 같네요!
Comment on lines
+283
to
+313
| @Test | ||
| @DisplayName("방 생성에 성공하면, 방장의 정보가 DB에 저장된다.") | ||
| @Transactional // RoomParticipant -> Room, User 의 manyToOne 지연로딩을 위해 추가 | ||
| void room_create_room_participant_save_success() throws Exception { | ||
| //given : user, category, pageCount값이 있는 book 생성, request 생성 | ||
| saveUserAndCategory(); | ||
| saveBookWithPageCount(); | ||
|
|
||
| Long userId = userJpaRepository.findAll().get(0).getUserId(); | ||
| Long bookId = bookJpaRepository.findAll().get(0).getBookId(); | ||
| Long categoryId = categoryJpaRepository.findAll().get(0).getCategoryId(); | ||
|
|
||
| Map<String, Object> request = buildRoomCreateRequest(); | ||
|
|
||
| //when | ||
| ResultActions result = mockMvc.perform(post("/rooms") | ||
| .requestAttr("userId", userId) | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .content(objectMapper.writeValueAsString(request) | ||
| )); | ||
|
|
||
| //then | ||
| result.andExpect(status().isOk()) | ||
| .andExpect(jsonPath("$.data.roomId").exists()); | ||
|
|
||
| RoomParticipantJpaEntity roomParticipantJpaEntity = roomParticipantJpaRepository.findAll().get(0); | ||
| assertThat(roomParticipantJpaEntity.getUserJpaEntity().getUserId()).isEqualTo(userId); | ||
|
|
||
| RoomJpaEntity savedRoomJpaEntity = roomJpaRepository.findAll().get(0); | ||
| assertThat(roomParticipantJpaEntity.getRoomJpaEntity().getRoomId()).isEqualTo(savedRoomJpaEntity.getRoomId()); | ||
| } |
- double 값의 비교이므로 isCloseTo 메서드로 검증하도록 수정
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (3)
src/main/java/konkuk/thip/room/application/service/RoomJoinService.java (1)
57-70: 불필요한 파라미터 제거 (메서드 시그니처 축소)
handleCancel의 두 번째 Optional 인자(roomParticipantOptional)는 사용되지 않습니다. 시그니처에서 제거해 주십시오.- private void handleCancel(RoomJoinCommand roomJoinCommand, Optional<RoomParticipant> participantOptional, Optional<RoomParticipant> roomParticipantOptional, Room room) { + private void handleCancel(RoomJoinCommand roomJoinCommand, Optional<RoomParticipant> participantOptional, Room room) { // 참여하지 않은 상태 RoomParticipant participant = participantOptional.orElseThrow(() -> new BusinessException(ErrorCode.USER_NOT_PARTICIPATED_CANNOT_CANCEL) );src/main/java/konkuk/thip/room/domain/RoomParticipant.java (1)
49-57: 0페이지/음수 페이지 방어 로직 부재로 인한 0으로 나누기 위험
totalPageCount가 0 이하인 데이터가 유입되면((double) currentPage / totalPageCount)에서 예외가 발생합니다. 입력값 사전 검증으로 방어해 주세요. 음수requestPage도 방어하는 편이 안전합니다.public boolean updateUserProgress(int requestPage, int totalPageCount) { + // 유효하지 않은 입력 방어 + if (totalPageCount <= 0 || requestPage < 0) { + return false; + } if (currentPage < requestPage) { currentPage = requestPage; userPercentage = Math.min(((double) currentPage / totalPageCount) * 100, 100.0); return true; }src/main/java/konkuk/thip/record/application/service/RecordCreateService.java (1)
50-54: NPE 위험: Book 조회 시 명시적 예외로 전환 필요여기서도
bookCommandPort.findById(...)가 null이면 Line 87/90의book.getPageCount()접근에서 NPE가 발생합니다. 예외 보장형 조회로 교체해 주세요.권장 변경:
- Book book = bookCommandPort.findById(room.getBookId()); + Book book = bookCommandPort.getByIdOrThrow(room.getBookId());대안(포트 확장이 어려운 경우):
Book book = bookCommandPort.findById(room.getBookId()); if (book == null) { throw new IllegalStateException("Book not found: id=" + room.getBookId()); }
♻️ Duplicate comments (2)
src/main/java/konkuk/thip/vote/application/service/VoteCreateService.java (2)
66-68: 진행률 업데이트: 불필요한 쓰기 최소화·동시성 안전성 보장 필요
roomProgressHelper.updateUserAndRoomProgress(...)내부에서
- 변경이 없는 경우 DB update 생략(쓰기 부하 감소),
- Room 평균 진행률 계산을 DB 집계(AVG)로 대체(일관성·성능),
- Room에 낙관적 락(version) 적용하여 동시 투표 생성 시 경합 방지
를 보장하는지 확인 부탁드립니다. 기존 제안과 동일한 맥락입니다.
50-53: NPE 위험: Book 조회 시 명시적 예외로 전환 필요
bookCommandPort.findById(...)가 null을 반환하면 Line 74/77에서book.getPageCount()접근 시 NPE가 발생합니다. 예외가 보장되는 쿼리 메서드로 전환하거나 명시적 null 체크가 필요합니다.권장 변경:
- Book book = bookCommandPort.findById(room.getBookId()); + Book book = bookCommandPort.getByIdOrThrow(room.getBookId());만약
getByIdOrThrow추가가 어려우면 최소한 다음과 같이 방어 코드를 추가해주세요(대안):Book book = bookCommandPort.findById(room.getBookId()); if (book == null) { throw new IllegalStateException("Book not found: id=" + room.getBookId()); }
🧹 Nitpick comments (8)
src/main/java/konkuk/thip/vote/domain/VoteItem.java (1)
40-47: 음수 카운트에 대한 사전 검증 추가 제안입력 리스트에 음수 카운트가 들어오는 경우를 방어하지 않으면 잘못된 비율이 산출될 수 있습니다. 동일한 에러 코드를 재사용해 방어 코드를 추가하는 것을 권장합니다.
아래 변경을 적용해 주세요:
public static List<Integer> calculatePercentages(List<Integer> counts) { + // 음수 카운트 방지 + for (Integer c : counts) { + if (c < 0) { + throw new InvalidStateException(ErrorCode.VOTE_ITEM_COUNT_CANNOT_BE_NEGATIVE); + } + } int total = counts.stream().mapToInt(Integer::intValue).sum();테스트 보강 제안:
- 모든 항목 0일 때 [0,0,...] 반환
- 합이 100이 되도록 잔여 분배(라운딩) 검증: [1,1,1] → [34,33,33], [2,1] → [67,33] 등
- 매우 큰 합계에서도 오동작 없음을 확인
src/main/java/konkuk/thip/room/application/service/RoomJoinService.java (1)
46-49: 조건 분기 단순화 및 중복 인자 제거동일 조건의 부정을 다시 검사하지 말고 else로 단순화하는 편이 가독성·의도 전달 면에서 낫습니다. 또한
handleCancel로 동일 Optional을 두 번 전달하는 중복을 제거하세요. (아래 시그니처 변경과 함께 적용)- // 취소하기 요청 - if(!type.isJoinType()) { - handleCancel(roomJoinCommand, roomParticipantOptional, roomParticipantOptional, room); - } + // 취소하기 요청 + else { + handleCancel(roomJoinCommand, roomParticipantOptional, room); + }src/test/java/konkuk/thip/room/application/service/RoomJoinServiceTest.java (1)
66-78: 상태 변화에 대한 단언 추가 제안 (memberCount 증가 검증)현재 테스트는 저장/업데이트 호출 여부만 검증합니다. 도메인 상태(memberCount) 변화까지 검증하면 회귀 방지에 도움이 됩니다.
예시:
void successJoin() { RoomJoinCommand command = new RoomJoinCommand(USER_ID, ROOM_ID, "join"); @@ given(roomParticipantCommandPort.findByUserIdAndRoomIdOptional(USER_ID, ROOM_ID)) .willReturn(Optional.empty()); + int before = room.getMemberCount(); roomJoinService.changeJoinState(command); then(roomParticipantCommandPort).should().save(any(RoomParticipant.class)); then(roomCommandPort).should().update(any(Room.class)); + assertThat(room.getMemberCount()).isEqualTo(before + 1); }추가로 필요한 정적 임포트:
import static org.assertj.core.api.Assertions.assertThat;src/main/java/konkuk/thip/roompost/application/service/helper/RoomProgressHelper.java (1)
23-45: 진행률 업데이트 검증을 정책 도메인으로 캡슐화 고려학습 맥락에 따르면 이 프로젝트는 "사용자 방 소속 검증"을 RoomParticipantPolicy로 캡슐화하는 것을 선호합니다. 본 헬퍼는
getByUserIdAndRoomIdOrThrow로 간접 검증을 하고 있으나, 동일 정책을 투표/기록 생성 서비스 전반에서 재사용할 수 있도록 정책 서비스를 주입·호출하는 방식으로 일관성을 높이는 것을 권장합니다.src/main/java/konkuk/thip/vote/application/service/VoteCreateService.java (1)
58-64: Stream.toList() 사용 호환성 확인
.toList()는 JDK 16+ 전용입니다. 프로젝트의 최소 JDK 버전이 16 미만이면 컴파일 실패합니다. 호환성을 확인하시거나Collectors.toList()로 변경을 고려해주세요.대체안:
- List<VoteItem> voteItems = command.voteItemCreateCommands().stream() - .map(itemCmd -> VoteItem.withoutId( - itemCmd.itemName(), - savedVoteId - )) - .toList(); + List<VoteItem> voteItems = command.voteItemCreateCommands().stream() + .map(itemCmd -> VoteItem.withoutId(itemCmd.itemName(), savedVoteId)) + .collect(java.util.stream.Collectors.toList());src/main/java/konkuk/thip/record/application/service/RecordCreateService.java (3)
16-16: Helper 패키지 경계 확인
RoomProgressHelper가 roompost 하위 패키지에 위치하지만 Record/Vote/Room 전역에서 사용하는 공통 도메인 정책에 가깝습니다. 모듈 경계 명확화를 위해 room(or roomprogress) 쪽 공통 위치로의 이동을 검토해주세요.
60-65: 레코드 저장 후 진행률 갱신 순서 적절하나, 내부 정책 확인 필요트랜잭션 내에서 저장 → 진행률 갱신 순서는 일관성 측면에서 타당합니다. 다만 Helper 내부에서
- 변경 없을 시 update 생략,
- 평균은 DB 집계(AVG) 사용,
- Room 낙관적 락으로 동시 요청 경합 처리
를 보장하는지 확인 부탁드립니다. 스케줄러로 분리하는 TODO(라인 37)와도 설계상 충돌 없는지 검토하면 좋겠습니다.
37-37: TODO 처리 계획 필요: 스케줄러 분리 여부
updateRoomPercentage의 스케줄러 책임 분리 TODO가 남아 있습니다. 동시성/부하 관점에서 의미 있는 선택지이므로 이슈로 분리해 논의/추적을 제안합니다. 원하시면 이슈 템플릿에 맞춰 초안을 만들어드리겠습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/konkuk/thip/record/application/service/RecordCreateService.java(3 hunks)src/main/java/konkuk/thip/room/application/service/RoomCreateService.java(3 hunks)src/main/java/konkuk/thip/room/application/service/RoomJoinService.java(1 hunks)src/main/java/konkuk/thip/room/domain/RoomParticipant.java(1 hunks)src/main/java/konkuk/thip/roompost/application/service/helper/RoomProgressHelper.java(1 hunks)src/main/java/konkuk/thip/vote/application/service/VoteCreateService.java(4 hunks)src/main/java/konkuk/thip/vote/domain/VoteItem.java(1 hunks)src/test/java/konkuk/thip/room/application/service/RoomJoinServiceTest.java(2 hunks)src/test/java/konkuk/thip/vote/application/service/VoteCreateServiceTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/test/java/konkuk/thip/vote/application/service/VoteCreateServiceTest.java
- src/main/java/konkuk/thip/room/application/service/RoomCreateService.java
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#101
File: src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java:36-39
Timestamp: 2025-07-26T06:09:00.850Z
Learning: THIP 프로젝트에서는 "사용자가 방에 속하는지 검증" 로직을 RoomParticipantPolicy 도메인 서비스로 캡슐화하여 재사용성을 높이고 비즈니스 로직의 중복을 방지하는 방식을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#113
File: src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java:38-44
Timestamp: 2025-07-30T14:05:04.945Z
Learning: seongjunnoh는 코드 최적화 제안에 대해 구체적인 기술적 근거와 효율성 차이를 이해하고 싶어하며, 성능 개선 방식에 대한 상세한 설명을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#93
File: src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java:49-114
Timestamp: 2025-07-28T16:44:31.224Z
Learning: seongjunnoh는 코드 중복 문제에 대한 리팩토링 제안을 적극적으로 수용하고 함수형 인터페이스를 활용한 해결책을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#195
File: src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java:0-0
Timestamp: 2025-08-13T05:22:32.287Z
Learning: seongjunnoh는 데이터 무결성과 중복 방지에 대한 고민이 깊으며, LinkedHashSet을 활용한 중복 제거와 순서 보장을 동시에 달성하는 솔루션을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#112
File: src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java:272-272
Timestamp: 2025-07-30T10:44:34.115Z
Learning: seongjunnoh는 피드 커서 페이지네이션에서 LocalDateTime 단일 커서 방식을 선호하며, 복합 키 기반 커서보다 구현 단순성과 성능을 우선시한다.
⏰ 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 (14)
src/main/java/konkuk/thip/vote/domain/VoteItem.java (1)
25-31: 팩토리 시그니처 단순화와 초기 카운트 0 고정, 적절합니다도메인 상 새 VoteItem의 count가 항상 0에서 시작한다는 불변식을 보장해 주며, 서비스 레이어에서 불필요한 파라미터를 제거합니다. 연쇄 변경(VoteCreateService 등)과도 일관됩니다.
src/main/java/konkuk/thip/room/application/service/RoomJoinService.java (1)
78-83: 멤버 참여 생성 팩토리 변경, 도메인 의도에 부합합니다
RoomParticipant.memberWithoutId사용으로 역할을 암시적으로 부여하도록 개선된 점 좋습니다. 호스트 생성 흐름(RoomCreateService)과도 일관됩니다.src/test/java/konkuk/thip/room/application/service/RoomJoinServiceTest.java (2)
58-59: 테스트 픽스처 팩토리 교체 적절도메인 팩토리 변경(
memberWithoutId)에 맞춘 수정이 정확합니다.
103-108: 취소 시 멤버 생성자 사용으로 픽스처 준비, 적절합니다취소 경로에서의 참여자 픽스처도 새 팩토리에 맞게 일관되게 구성된 점 좋습니다.
src/main/java/konkuk/thip/room/domain/RoomParticipant.java (3)
24-32: 멤버 전용 팩토리 도입, 역할 불변식 명확화
memberWithoutId가 진행률 초기값과 역할을 일관되게 초기화하여 도메인 무결성에 기여합니다.
34-41: 호스트 전용 팩토리 도입, RoomCreate 플로우와 일치호스트 역할 부여를 팩토리로 캡슐화한 설계, 훌륭합니다.
24-41: 레거시 팩토리 호출(RoomParticipant.withoutId) 잔존 여부 수동 확인 필요샌드박스에서 실행한 검색이 출력 없이 종료되어 호출 잔존 여부를 확정할 수 없습니다. 로컬 저장소 루트에서 아래 명령들을 실행해 결과(매치된 파일:라인 또는 매치 없음)를 알려 주세요.
# 정확한 호출 검색 rg -n "RoomParticipant\.withoutId\(" -S # 일반 검색(정적 import 또는 단축 호출 가능성 대비) rg -n "withoutId\(" -S # static import 확인 rg -n "import static .*RoomParticipant" -S # 숨김/무시 파일 포함(필요 시) rg -n --hidden --no-ignore "RoomParticipant\.withoutId\(" -S # ripgrep 실패 시 대체 git grep -n "RoomParticipant.withoutId(" || truesrc/main/java/konkuk/thip/roompost/application/service/helper/RoomProgressHelper.java (1)
23-31: 검증 완료 — Book 조회 널 안전성 문제 없음BookCommandPort.findById는 null을 반환하지 않습니다. BookCommandPersistenceAdapter.findById 구현이 bookJpaRepository.findById(id).orElseThrow(() -> new EntityNotFoundException(BOOK_NOT_FOUND))로 되어 있어, 값이 없을 경우 예외를 던지므로 NPE 위험이 없습니다.
검증 위치:
- src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java — 선언: Book findById(Long id);
- src/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.java — findById 구현: orElseThrow(BOOK_NOT_FOUND)
- src/main/java/konkuk/thip/roompost/application/service/helper/RoomProgressHelper.java — 호출 위치: bookCommandPort.findById(room.getBookId())
결론: getByIdOrThrow로 변경할 필요 없이 현재 코드 상태는 안전합니다.
src/main/java/konkuk/thip/vote/application/service/VoteCreateService.java (2)
39-40: 방 멤버십 검증 추가: 매우 좋습니다투표 생성 전 방 참여자 여부를 선검증함으로써 후속 로직의 불필요한 DB 접근을 줄이고 책임 경계를 명확히 했습니다.
72-78: 검증 로직 위임 적절Book 페이지 수를 이용한 페이지/총평 검증을 도메인 메서드로 위임한 점 좋습니다. 단일 책임과 테스트 용이성이 확보됩니다.
src/main/java/konkuk/thip/record/application/service/RecordCreateService.java (4)
33-34: 진행률 갱신을 Helper로 위임: 좋습니다서비스별 중복을 제거하고 정책 변경점을 단일화할 수 있어 유지보수성이 높아집니다.
39-40: 방 멤버십 검증 선행: 좋습니다레코드 생성 전 멤버십을 강제해 도메인 불변식 위반을 초기에 차단합니다.
85-91: 검증 위임 적절페이지/총평 검증을 도메인 메서드로 위임해 중복을 줄였고 테스트 용이성이 좋아졌습니다.
69-78: 검증 결과 — Null 안전성 보강 불필요, 총평 허용은 ‘사전 진행률’ 기준입니다.간단 요약:
- RoomParticipant.getUserPercentage()는 primitive double(널 불가)이라 String.format("%.2f", ...)로 NPE/형변환 예외가 발생하지 않습니다.
- Record 생성 흐름에서 총평 허용 여부는 레코드 저장·진행률 갱신 이전에 검사되므로(사전 진행률 기준) “이번 기록으로 80%를 넘기는 경우”는 허용되지 않습니다.
참고 코드 위치:
- src/main/java/konkuk/thip/room/domain/RoomParticipant.java
- 필드: private double userPercentage;
- 검증: public boolean canWriteOverview() { return userPercentage >= 80; }
- src/main/java/konkuk/thip/record/application/service/RecordCreateService.java
- roomParticipant 조회 후 validateRoomParticipant(...) 호출 → 그 이후에 레코드 저장(recordCommandPort.saveRecord) 및 roomProgressHelper.updateUserAndRoomProgress(...) 실행
- src/main/java/konkuk/thip/roompost/application/service/helper/RoomProgressHelper.java
- updateUserAndRoomProgress(...)가 userPercentage를 갱신(사후 반영)
결론: 제안하신 null-체크(diff)는 현재 코드상 불필요합니다. (만약 userPercentage를 nullable로 모델링하려면 엔티티/DB/매퍼 전반에 영향을 주는 설계 변경이 필요합니다.)
Likely an incorrect or invalid review comment.
Comment on lines
+33
to
+45
| // 2. 방 평균 진행률 update | ||
| List<RoomParticipant> all = roomParticipantCommandPort.findAllByRoomId(roomId); | ||
| double total = all.stream() | ||
| .filter(p -> !roomParticipant.getId().equals(p.getId())) // 현재 유저 제외 | ||
| .mapToDouble(RoomParticipant::getUserPercentage) | ||
| .sum(); | ||
| total += roomParticipant.getUserPercentage(); | ||
| room.updateRoomPercentage(total / all.size()); | ||
|
|
||
| // 3. 영속화 | ||
| roomCommandPort.update(room); | ||
| roomParticipantCommandPort.update(roomParticipant); | ||
| } |
There was a problem hiding this comment.
참여자 목록 공집합 및 평균 계산 시 예외 가능성 방어
all.size()가 0이면 0으로 나누기 예외가 발생합니다. 정상 플로우에서는 최소 1명이 존재하겠지만, 데이터 불일치나 경합 상황에서 방어가 필요합니다.- 동일 트랜잭션에서 평균만 갱신되고 영속화 순서가 반대로 되더라도 큰 문제는 없으나, 조기 종료 경로(
!updated) 뒤에 불필요한 조회/계산을 하지 않는 현재 구조는 적절합니다.
간단한 방어 로직을 추가해 주세요.
// 2. 방 평균 진행률 update
List<RoomParticipant> all = roomParticipantCommandPort.findAllByRoomId(roomId);
- double total = all.stream()
+ if (all.isEmpty()) {
+ return; // 참여자가 없으면 평균 계산 불가
+ }
+ double total = all.stream()
.filter(p -> !roomParticipant.getId().equals(p.getId())) // 현재 유저 제외
.mapToDouble(RoomParticipant::getUserPercentage)
.sum();
total += roomParticipant.getUserPercentage();
room.updateRoomPercentage(total / all.size());
// 3. 영속화
roomCommandPort.update(room);
roomParticipantCommandPort.update(roomParticipant);📝 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.
Suggested change
| // 2. 방 평균 진행률 update | |
| List<RoomParticipant> all = roomParticipantCommandPort.findAllByRoomId(roomId); | |
| double total = all.stream() | |
| .filter(p -> !roomParticipant.getId().equals(p.getId())) // 현재 유저 제외 | |
| .mapToDouble(RoomParticipant::getUserPercentage) | |
| .sum(); | |
| total += roomParticipant.getUserPercentage(); | |
| room.updateRoomPercentage(total / all.size()); | |
| // 3. 영속화 | |
| roomCommandPort.update(room); | |
| roomParticipantCommandPort.update(roomParticipant); | |
| } | |
| // 2. 방 평균 진행률 update | |
| List<RoomParticipant> all = roomParticipantCommandPort.findAllByRoomId(roomId); | |
| if (all.isEmpty()) { | |
| return; // 참여자가 없으면 평균 계산 불가 | |
| } | |
| double total = all.stream() | |
| .filter(p -> !roomParticipant.getId().equals(p.getId())) // 현재 유저 제외 | |
| .mapToDouble(RoomParticipant::getUserPercentage) | |
| .sum(); | |
| total += roomParticipant.getUserPercentage(); | |
| room.updateRoomPercentage(total / all.size()); | |
| // 3. 영속화 | |
| roomCommandPort.update(room); | |
| roomParticipantCommandPort.update(roomParticipant); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/roompost/application/service/helper/RoomProgressHelper.java
around lines 33 to 45, protect against division-by-zero when computing the
average: check if the participant list `all` is empty (or size == 0) before
dividing; if empty, set the room percentage to a safe default (e.g., 0.0) and
skip the average calculation, otherwise perform the existing sum/average logic
and persist updates as before.
Contributor
There was a problem hiding this comment.
방을 생성할때 무조건 방의 HOST를 위해 최소 하나의 RoomParticipant가 만들어지는데 참여자가 없을리가 있나?
There was a problem hiding this comment.
@buzz0331 맞는 말씀입니다! 비즈니스 로직상 방 생성 시 항상 HOST 역할의 RoomParticipant가 생성되고, 이 헬퍼 메서드가 호출되는 경로(투표 생성, 기록 생성)에서는 모두 사용자가 해당 방의 참여자인지 사전 검증을 거치기 때문에 논리적으로 참여자 목록이 비어있을 수 없습니다.
해당 방어 로직은 불필요한 over-engineering일 가능성이 높습니다. 비즈니스 규칙이 명확하게 보장하는 상황에서는 과도한 방어 코드가 오히려 코드의 가독성을 해칠 수 있죠.
기존 코드 그대로 유지하시는 것이 적절할 것 같습니다.
✏️ Learnings added
Learnt from: buzz0331
PR: THIP-TextHip/THIP-Server#183
File: src/main/java/konkuk/thip/roompost/application/service/helper/RoomProgressHelper.java:33-45
Timestamp: 2025-08-14T12:29:00.102Z
Learning: 방 생성 시 항상 HOST 역할의 RoomParticipant가 생성되며, RoomProgressHelper.updateUserAndRoomProgress() 메서드는 사용자가 해당 방의 참여자임이 사전 검증된 상황에서만 호출되므로 참여자 목록이 비어있을 수 없다.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
buzz0331
previously approved these changes
Aug 14, 2025
Comment on lines
+63
to
+64
| // 4. RoomParticipant의 currentPage, userPercentage 업데이트 | ||
| updateRoomProgress(roomParticipant, record, book, room); |
Contributor
There was a problem hiding this comment.
와우 공통 로직을 분리하니까 확실히 응용서비스 로직이 간단해졌네요! 굿굿
Comment on lines
+24
to
42
| public static RoomParticipant memberWithoutId(Long userId, Long roomId) { | ||
| return RoomParticipant.builder() | ||
| .currentPage(0) | ||
| .userPercentage(0.0) | ||
| .userId(userId) | ||
| .roomId(roomId) | ||
| .roomParticipantRole(roomParticipantRole) | ||
| .roomParticipantRole(RoomParticipantRole.MEMBER.getType()) | ||
| .build(); | ||
| } | ||
|
|
||
| public static RoomParticipant hostWithoutId(Long userId, Long roomId) { | ||
| return RoomParticipant.builder() | ||
| .currentPage(0) | ||
| .userPercentage(0.0) | ||
| .userId(userId) | ||
| .roomId(roomId) | ||
| .roomParticipantRole(RoomParticipantRole.HOST.getType()) | ||
| .build(); | ||
| } |
|
|
||
| import java.util.List; | ||
|
|
||
| @Service |
Contributor
There was a problem hiding this comment.
헬퍼서비스인 것 같으니 어노테이션을 바꾸는 것 어떨까욥
Collaborator
Author
There was a problem hiding this comment.
아아 이것도 수정하겠습니다
src/main/java/konkuk/thip/roompost/application/service/helper/RoomProgressHelper.java
Outdated
Show resolved
Hide resolved
Comment on lines
+33
to
+45
| // 2. 방 평균 진행률 update | ||
| List<RoomParticipant> all = roomParticipantCommandPort.findAllByRoomId(roomId); | ||
| double total = all.stream() | ||
| .filter(p -> !roomParticipant.getId().equals(p.getId())) // 현재 유저 제외 | ||
| .mapToDouble(RoomParticipant::getUserPercentage) | ||
| .sum(); | ||
| total += roomParticipant.getUserPercentage(); | ||
| room.updateRoomPercentage(total / all.size()); | ||
|
|
||
| // 3. 영속화 | ||
| roomCommandPort.update(room); | ||
| roomParticipantCommandPort.update(roomParticipant); | ||
| } |
Contributor
There was a problem hiding this comment.
방을 생성할때 무조건 방의 HOST를 위해 최소 하나의 RoomParticipant가 만들어지는데 참여자가 없을리가 있나?
Comment on lines
+112
to
+114
| // 허용 오차범위를 10의 -6제곱(= 0.000001) 로 설정 | ||
| assertThat(roomParticipant.getUserPercentage()).isCloseTo((double) 89 / 369 * 100, within(1e-6)); | ||
| assertThat(refreshRoom.getRoomPercentage()).isCloseTo((7.7 + (double) 89 / 369 * 100) / 2, within(1e-6)); |
seongjunnoh
deleted the
refactor/#181-modifiy-pre-apis-by-changing-jpa-entity
branch
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#️⃣ 연관된 이슈
📝 작업 내용
바뀐 jpa entity 구조에 따라 방 생성, 투표 생성 api 의 코드를 수정하였습니다
📸 스크린샷
💬 리뷰 요구사항
방 검색 api 수정도 같이 하려고 했으나, 의논하고 싶은 부분이 있어서 이건 빼고 나머지 api 들만 수정하였습니다
방 검색 api 의논사항은 디스코드 정리 참고해주시면 감사하겠습니다!
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit
버그 수정
리팩터
테스트
기타