Conversation
|
""" Walkthrough이 변경사항은 종소리(Bell) 도메인에 새로운 속성인 Changes
Sequence Diagram(s)sequenceDiagram
participant Controller
participant Service
participant DomainRepository
participant DB
Controller->>Service: 커스텀 테이블 저장/조회/수정/삭제 요청
Service->>DomainRepository: 도메인 객체(BellType 포함)로 CRUD
DomainRepository->>DB: CustomizeTable, CustomizeTimeBox, Bell (BellType 포함) 저장/조회/수정/삭제
DB-->>DomainRepository: 결과 반환 (BellType 포함)
DomainRepository-->>Service: 도메인 객체 반환 (BellType 포함)
Service-->>Controller: 응답 DTO 반환 (BellType 포함)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40분
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Test Results120 files 120 suites 14s ⏱️ Results for commit 981cd09. ♻️ This comment has been updated with latest results. |
📝 Test Coverage Report
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/main/java/com/debatetimer/domain/customize/Bell.java (1)
1-5: BellType import가 누락되었습니다.BellType enum을 사용하고 있지만 import 구문이 누락된 것으로 보입니다. 컴파일 오류를 방지하기 위해 import를 추가해야 합니다.
다음과 같이 import를 추가하세요:
package com.debatetimer.domain.customize; +import com.debatetimer.domain.customize.BellType; import com.debatetimer.exception.custom.DTClientErrorException; import com.debatetimer.exception.errorcode.ClientErrorCode; import lombok.Getter;
🧹 Nitpick comments (9)
src/main/java/com/debatetimer/domain/customize/BellType.java (1)
8-8: 불필요한 세미콜론 제거를 고려해보세요.enum에 메서드나 필드가 없을 때는 세미콜론이 선택적입니다. 코드 일관성을 위해 제거하는 것을 권장합니다.
AFTER_START, BEFORE_END, - AFTER_END, - ; + AFTER_ENDsrc/test/java/com/debatetimer/controller/BaseControllerTest.java (2)
11-17: 엔티티 제너레이터 import 명칭 일관성 확인
MemberGenerator만Entity접미사가 없는데, 도메인-전용 제너레이터와 구분하기 위해 의도한 것인지 확인이 필요합니다. 동일한 규칙을 유지하면 추후 테스트 유지보수가 수월합니다.
45-55: Autowired 필드를final로 선언하면 불변성을 드러낼 수 있습니다스프링이 필드 주입 시
final을 허용하므로, 테스트 픽스처 제너레이터들을final로 선언하면 IDE/리팩터링 시 실수 가능성을 줄일 수 있습니다.-@Autowired -protected CustomizeTableEntityGenerator customizeTableEntityGenerator; +@Autowired +protected final CustomizeTableEntityGenerator customizeTableEntityGenerator;src/test/java/com/debatetimer/repository/customize/CustomizeTableEntityRepositoryTest.java (1)
28-31:containsExactly→containsExactlyInAnyOrder교체 고려
findAllByMember()결과의 정렬이 보장되지 않으면 테스트가 간헐적으로 실패할 수 있습니다. 순서가 중요하지 않다면containsExactlyInAnyOrder가 안전합니다.-assertThat(foundKeoChanTables).containsExactly(chanTable1, chanTable2); +assertThat(foundKeoChanTables).containsExactlyInAnyOrder(chanTable1, chanTable2);src/test/java/com/debatetimer/repository/customize/BellRepositoryTest.java (1)
28-36: 벨 생성 시 BellType 매개변수 추가 검토모든 벨 생성에서
BellType.AFTER_START를 사용하고 있습니다. 테스트 커버리지 향상을 위해 다양한BellType값을 사용하는 것을 고려해보세요.- bellEntityGenerator.generate(deleteBellTimeBox, BellType.AFTER_START, 45, 1); - bellEntityGenerator.generate(deleteBellTimeBox, BellType.AFTER_START, 60, 1); - bellEntityGenerator.generate(otherTimeBox, BellType.AFTER_START, 45, 1); + bellEntityGenerator.generate(deleteBellTimeBox, BellType.AFTER_START, 45, 1); + bellEntityGenerator.generate(deleteBellTimeBox, BellType.BEFORE_END, 60, 1); + bellEntityGenerator.generate(otherTimeBox, BellType.AFTER_START, 45, 1);src/test/java/com/debatetimer/domain/customize/BellTest.java (1)
20-20: Bell 생성자 시그니처 변경이 올바르게 반영되었습니다.새로운
BellType매개변수가 모든 테스트 케이스에 일관성 있게 추가되었습니다.다양한
BellType값들(AFTER_START,BEFORE_END,AFTER_END)에 대한 테스트 케이스를 추가하여 포괄적인 테스트 커버리지를 확보하는 것을 고려해보세요.@Test void 벨_시간이_음수면_생성되지_않는다() { assertThatThrownBy(() -> new Bell(BellType.AFTER_START, -1, 1)) .isInstanceOf(DTClientErrorException.class) .hasMessage(ClientErrorCode.INVALID_BELL_TIME.getMessage()); + assertThatThrownBy(() -> new Bell(BellType.BEFORE_END, -1, 1)) + .isInstanceOf(DTClientErrorException.class) + .hasMessage(ClientErrorCode.INVALID_BELL_TIME.getMessage()); }Also applies to: 27-27, 34-34, 42-42
src/test/java/com/debatetimer/fixture/domain/BellGenerator.java (1)
9-23: 도메인 레이어용 Bell 픽스처 생성기가 잘 구현되었습니다.엔티티와 도메인 레이어를 명확히 분리한 설계가 좋습니다.
테스트의 유연성을 위해
BellType,time,count값을 매개변수로 받을 수 있는 오버로드 메서드를 추가하는 것을 고려해보세요:@Component public class BellGenerator { public List<Bell> generate(int size) { List<Bell> bells = new ArrayList<>(); for (int i = 0; i < size; i++) { bells.add(generate()); } return bells; } + public List<Bell> generate(int size, BellType type, int time, int count) { + List<Bell> bells = new ArrayList<>(); + for (int i = 0; i < size; i++) { + bells.add(new Bell(type, time, count)); + } + return bells; + } private Bell generate() { return new Bell(BellType.AFTER_START, 10, 1); } }src/test/java/com/debatetimer/controller/customize/CustomizeDocumentTest.java (1)
115-117: 테스트 데이터에 다양한 BellType 값 사용을 고려해보세요모든 테스트에서
BellType.AFTER_START만 사용하고 있습니다. API 문서화 테스트의 완전성을 위해BEFORE_END,AFTER_END등 다른 타입도 포함하면 더 나은 문서와 테스트 커버리지를 제공할 수 있습니다.Also applies to: 131-132, 174-176, 253-255, 371-373, 386-388, 433-435, 515-517
src/main/java/com/debatetimer/domainrepository/customize/CustomizeTableDomainRepository.java (1)
62-72: 업데이트 로직이 안전하게 구현되었습니다!Bell을 먼저 삭제하고 TimeBox를 삭제하는 순서가 외래 키 제약 조건을 고려한 올바른 구현입니다. 다만, 대량의 데이터가 있을 경우 성능 최적화를 고려해볼 수 있습니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (39)
src/main/java/com/debatetimer/domain/customize/Bell.java(1 hunks)src/main/java/com/debatetimer/domain/customize/BellType.java(1 hunks)src/main/java/com/debatetimer/domainrepository/customize/CustomizeTableDomainRepository.java(2 hunks)src/main/java/com/debatetimer/domainrepository/poll/CustomizeTableDomainRepository.java(0 hunks)src/main/java/com/debatetimer/dto/customize/request/BellRequest.java(1 hunks)src/main/java/com/debatetimer/dto/customize/request/CustomizeTableCreateRequest.java(1 hunks)src/main/java/com/debatetimer/dto/customize/response/BellResponse.java(1 hunks)src/main/java/com/debatetimer/entity/customize/BellEntity.java(3 hunks)src/main/java/com/debatetimer/service/customize/CustomizeService.java(3 hunks)src/main/java/com/debatetimer/service/poll/PollService.java(1 hunks)src/main/resources/db/migration/V12__add_bell_type_into_bell.sql(1 hunks)src/test/java/com/debatetimer/controller/BaseControllerTest.java(2 hunks)src/test/java/com/debatetimer/controller/customize/CustomizeControllerTest.java(11 hunks)src/test/java/com/debatetimer/controller/customize/CustomizeDocumentTest.java(15 hunks)src/test/java/com/debatetimer/controller/poll/PollControllerTest.java(3 hunks)src/test/java/com/debatetimer/domain/customize/BellTest.java(1 hunks)src/test/java/com/debatetimer/domainrepository/BaseDomainRepositoryTest.java(2 hunks)src/test/java/com/debatetimer/domainrepository/customize/CustomizeTableDomainRepositoryTest.java(1 hunks)src/test/java/com/debatetimer/domainrepository/poll/PollDomainRepositoryTest.java(3 hunks)src/test/java/com/debatetimer/domainrepository/poll/VoteDomainRepositoryTest.java(1 hunks)src/test/java/com/debatetimer/entity/customize/BellEntityTest.java(2 hunks)src/test/java/com/debatetimer/fixture/domain/BellGenerator.java(1 hunks)src/test/java/com/debatetimer/fixture/domain/CustomizeTableGenerator.java(1 hunks)src/test/java/com/debatetimer/fixture/domain/CustomizeTimeBoxGenerator.java(1 hunks)src/test/java/com/debatetimer/fixture/entity/BellEntityGenerator.java(1 hunks)src/test/java/com/debatetimer/fixture/entity/CustomizeTableEntityGenerator.java(2 hunks)src/test/java/com/debatetimer/fixture/entity/CustomizeTimeBoxEntityGenerator.java(2 hunks)src/test/java/com/debatetimer/fixture/entity/MemberGenerator.java(1 hunks)src/test/java/com/debatetimer/fixture/entity/PollEntityGenerator.java(2 hunks)src/test/java/com/debatetimer/fixture/entity/VoteEntityGenerator.java(2 hunks)src/test/java/com/debatetimer/repository/BaseRepositoryTest.java(2 hunks)src/test/java/com/debatetimer/repository/customize/BellRepositoryTest.java(2 hunks)src/test/java/com/debatetimer/repository/customize/CustomizeTableEntityRepositoryTest.java(3 hunks)src/test/java/com/debatetimer/repository/customize/CustomizeTimeBoxRepositoryTest.java(4 hunks)src/test/java/com/debatetimer/service/BaseServiceTest.java(2 hunks)src/test/java/com/debatetimer/service/customize/CustomizeServiceTest.java(9 hunks)src/test/java/com/debatetimer/service/customize/CustomizeServiceV2Test.java(0 hunks)src/test/java/com/debatetimer/service/member/MemberServiceTest.java(2 hunks)src/test/java/com/debatetimer/service/poll/PollServiceTest.java(3 hunks)
💤 Files with no reviewable changes (2)
- src/main/java/com/debatetimer/domainrepository/poll/CustomizeTableDomainRepository.java
- src/test/java/com/debatetimer/service/customize/CustomizeServiceV2Test.java
🧰 Additional context used
🧠 Learnings (15)
📓 Common learnings
Learnt from: leegwichan
PR: debate-timer/debate-timer-be#201
File: src/main/java/com/debatetimer/entity/customize/BellEntity.java:50-54
Timestamp: 2025-07-18T01:04:20.985Z
Learning: BellEntity에서는 도메인 레벨에서 이미 유효성 검사가 수행되므로 엔티티 레벨의 유효성 검사는 중복이며, 향후 엔티티의 유효성 검사를 제거할 예정이다.
src/main/java/com/debatetimer/service/poll/PollService.java (1)
Learnt from: coli-geonwoo
PR: #203
File: src/main/resources/db/migration/V11__add_memberId_into_poll.sql:1-2
Timestamp: 2025-07-24T03:12:25.245Z
Learning: debate-timer 프로젝트에서 poll 관련 테이블들은 아직 운영 환경에 배포되지 않았으므로, NOT NULL 컬럼 추가 시 기존 데이터 백필 전략을 고려할 필요가 없음.
src/test/java/com/debatetimer/controller/poll/PollControllerTest.java (1)
Learnt from: coli-geonwoo
PR: #203
File: src/main/resources/db/migration/V11__add_memberId_into_poll.sql:1-2
Timestamp: 2025-07-24T03:12:25.245Z
Learning: debate-timer 프로젝트에서 poll 관련 테이블들은 아직 운영 환경에 배포되지 않았으므로, NOT NULL 컬럼 추가 시 기존 데이터 백필 전략을 고려할 필요가 없음.
src/test/java/com/debatetimer/repository/customize/CustomizeTimeBoxRepositoryTest.java (2)
Learnt from: leegwichan
PR: #198
File: src/test/java/com/debatetimer/domain/customize/CustomizeTimeBoxDomainTest.java:82-112
Timestamp: 2025-07-15T01:28:24.899Z
Learning: CustomizeTimeBoxDomain에서 validateStance 메서드는 null 체크를 먼저 수행한 후에 isValidStance() 추상 메서드를 호출하는 구조로, 테스트용 구현체에서 isValidStance()가 단순히 true를 반환하는 것이 적절하다.
Learnt from: leegwichan
PR: #198
File: src/test/java/com/debatetimer/domain/customize/CustomizeTimeBoxDomainTest.java:82-112
Timestamp: 2025-07-15T01:28:24.899Z
Learning: CustomizeTimeBoxDomain 클래스에서는 생성자에서 stance null 검증을 먼저 수행한 후 isValidStance() 추상 메서드를 호출하는 구조로 되어 있어, 테스트용 구현체에서 isValidStance()는 단순히 true를 반환해도 된다.
src/test/java/com/debatetimer/service/poll/PollServiceTest.java (1)
Learnt from: coli-geonwoo
PR: #203
File: src/main/resources/db/migration/V11__add_memberId_into_poll.sql:1-2
Timestamp: 2025-07-24T03:12:25.245Z
Learning: debate-timer 프로젝트에서 poll 관련 테이블들은 아직 운영 환경에 배포되지 않았으므로, NOT NULL 컬럼 추가 시 기존 데이터 백필 전략을 고려할 필요가 없음.
src/test/java/com/debatetimer/repository/customize/BellRepositoryTest.java (1)
Learnt from: leegwichan
PR: #198
File: src/test/java/com/debatetimer/domain/customize/CustomizeTimeBoxDomainTest.java:82-112
Timestamp: 2025-07-15T01:28:24.899Z
Learning: CustomizeTimeBoxDomain에서 validateStance 메서드는 null 체크를 먼저 수행한 후에 isValidStance() 추상 메서드를 호출하는 구조로, 테스트용 구현체에서 isValidStance()가 단순히 true를 반환하는 것이 적절하다.
src/test/java/com/debatetimer/fixture/entity/CustomizeTimeBoxEntityGenerator.java (2)
Learnt from: leegwichan
PR: #198
File: src/test/java/com/debatetimer/domain/customize/CustomizeTimeBoxDomainTest.java:82-112
Timestamp: 2025-07-15T01:28:24.899Z
Learning: CustomizeTimeBoxDomain 클래스에서는 생성자에서 stance null 검증을 먼저 수행한 후 isValidStance() 추상 메서드를 호출하는 구조로 되어 있어, 테스트용 구현체에서 isValidStance()는 단순히 true를 반환해도 된다.
Learnt from: leegwichan
PR: #198
File: src/test/java/com/debatetimer/domain/customize/CustomizeTimeBoxDomainTest.java:82-112
Timestamp: 2025-07-15T01:28:24.899Z
Learning: CustomizeTimeBoxDomain에서 validateStance 메서드는 null 체크를 먼저 수행한 후에 isValidStance() 추상 메서드를 호출하는 구조로, 테스트용 구현체에서 isValidStance()가 단순히 true를 반환하는 것이 적절하다.
src/test/java/com/debatetimer/domainrepository/poll/PollDomainRepositoryTest.java (1)
Learnt from: coli-geonwoo
PR: #203
File: src/main/resources/db/migration/V11__add_memberId_into_poll.sql:1-2
Timestamp: 2025-07-24T03:12:25.245Z
Learning: debate-timer 프로젝트에서 poll 관련 테이블들은 아직 운영 환경에 배포되지 않았으므로, NOT NULL 컬럼 추가 시 기존 데이터 백필 전략을 고려할 필요가 없음.
src/test/java/com/debatetimer/fixture/entity/VoteEntityGenerator.java (1)
Learnt from: coli-geonwoo
PR: #203
File: src/main/resources/db/migration/V11__add_memberId_into_poll.sql:1-2
Timestamp: 2025-07-24T03:12:25.245Z
Learning: debate-timer 프로젝트에서 poll 관련 테이블들은 아직 운영 환경에 배포되지 않았으므로, NOT NULL 컬럼 추가 시 기존 데이터 백필 전략을 고려할 필요가 없음.
src/test/java/com/debatetimer/fixture/entity/PollEntityGenerator.java (1)
Learnt from: coli-geonwoo
PR: #203
File: src/main/resources/db/migration/V11__add_memberId_into_poll.sql:1-2
Timestamp: 2025-07-24T03:12:25.245Z
Learning: debate-timer 프로젝트에서 poll 관련 테이블들은 아직 운영 환경에 배포되지 않았으므로, NOT NULL 컬럼 추가 시 기존 데이터 백필 전략을 고려할 필요가 없음.
src/test/java/com/debatetimer/fixture/domain/CustomizeTimeBoxGenerator.java (2)
Learnt from: leegwichan
PR: #198
File: src/test/java/com/debatetimer/domain/customize/CustomizeTimeBoxDomainTest.java:82-112
Timestamp: 2025-07-15T01:28:24.899Z
Learning: CustomizeTimeBoxDomain 클래스에서는 생성자에서 stance null 검증을 먼저 수행한 후 isValidStance() 추상 메서드를 호출하는 구조로 되어 있어, 테스트용 구현체에서 isValidStance()는 단순히 true를 반환해도 된다.
Learnt from: leegwichan
PR: #198
File: src/test/java/com/debatetimer/domain/customize/CustomizeTimeBoxDomainTest.java:82-112
Timestamp: 2025-07-15T01:28:24.899Z
Learning: CustomizeTimeBoxDomain에서 validateStance 메서드는 null 체크를 먼저 수행한 후에 isValidStance() 추상 메서드를 호출하는 구조로, 테스트용 구현체에서 isValidStance()가 단순히 true를 반환하는 것이 적절하다.
src/test/java/com/debatetimer/controller/customize/CustomizeDocumentTest.java (2)
Learnt from: leegwichan
PR: #198
File: src/test/java/com/debatetimer/domain/customize/CustomizeTimeBoxDomainTest.java:82-112
Timestamp: 2025-07-15T01:28:24.899Z
Learning: CustomizeTimeBoxDomain 클래스에서는 생성자에서 stance null 검증을 먼저 수행한 후 isValidStance() 추상 메서드를 호출하는 구조로 되어 있어, 테스트용 구현체에서 isValidStance()는 단순히 true를 반환해도 된다.
Learnt from: leegwichan
PR: #198
File: src/test/java/com/debatetimer/domain/customize/CustomizeTimeBoxDomainTest.java:82-112
Timestamp: 2025-07-15T01:28:24.899Z
Learning: CustomizeTimeBoxDomain에서 validateStance 메서드는 null 체크를 먼저 수행한 후에 isValidStance() 추상 메서드를 호출하는 구조로, 테스트용 구현체에서 isValidStance()가 단순히 true를 반환하는 것이 적절하다.
src/test/java/com/debatetimer/service/customize/CustomizeServiceTest.java (2)
Learnt from: leegwichan
PR: #198
File: src/test/java/com/debatetimer/domain/customize/CustomizeTimeBoxDomainTest.java:82-112
Timestamp: 2025-07-15T01:28:24.899Z
Learning: CustomizeTimeBoxDomain에서 validateStance 메서드는 null 체크를 먼저 수행한 후에 isValidStance() 추상 메서드를 호출하는 구조로, 테스트용 구현체에서 isValidStance()가 단순히 true를 반환하는 것이 적절하다.
Learnt from: leegwichan
PR: #198
File: src/test/java/com/debatetimer/domain/customize/CustomizeTimeBoxDomainTest.java:82-112
Timestamp: 2025-07-15T01:28:24.899Z
Learning: CustomizeTimeBoxDomain 클래스에서는 생성자에서 stance null 검증을 먼저 수행한 후 isValidStance() 추상 메서드를 호출하는 구조로 되어 있어, 테스트용 구현체에서 isValidStance()는 단순히 true를 반환해도 된다.
src/test/java/com/debatetimer/controller/customize/CustomizeControllerTest.java (2)
Learnt from: leegwichan
PR: #198
File: src/test/java/com/debatetimer/domain/customize/CustomizeTimeBoxDomainTest.java:82-112
Timestamp: 2025-07-15T01:28:24.899Z
Learning: CustomizeTimeBoxDomain에서 validateStance 메서드는 null 체크를 먼저 수행한 후에 isValidStance() 추상 메서드를 호출하는 구조로, 테스트용 구현체에서 isValidStance()가 단순히 true를 반환하는 것이 적절하다.
Learnt from: leegwichan
PR: #198
File: src/test/java/com/debatetimer/domain/customize/CustomizeTimeBoxDomainTest.java:82-112
Timestamp: 2025-07-15T01:28:24.899Z
Learning: CustomizeTimeBoxDomain 클래스에서는 생성자에서 stance null 검증을 먼저 수행한 후 isValidStance() 추상 메서드를 호출하는 구조로 되어 있어, 테스트용 구현체에서 isValidStance()는 단순히 true를 반환해도 된다.
src/test/java/com/debatetimer/domainrepository/customize/CustomizeTableDomainRepositoryTest.java (2)
Learnt from: leegwichan
PR: #198
File: src/test/java/com/debatetimer/domain/customize/CustomizeTimeBoxDomainTest.java:82-112
Timestamp: 2025-07-15T01:28:24.899Z
Learning: CustomizeTimeBoxDomain에서 validateStance 메서드는 null 체크를 먼저 수행한 후에 isValidStance() 추상 메서드를 호출하는 구조로, 테스트용 구현체에서 isValidStance()가 단순히 true를 반환하는 것이 적절하다.
Learnt from: leegwichan
PR: #198
File: src/test/java/com/debatetimer/domain/customize/CustomizeTimeBoxDomainTest.java:82-112
Timestamp: 2025-07-15T01:28:24.899Z
Learning: CustomizeTimeBoxDomain 클래스에서는 생성자에서 stance null 검증을 먼저 수행한 후 isValidStance() 추상 메서드를 호출하는 구조로 되어 있어, 테스트용 구현체에서 isValidStance()는 단순히 true를 반환해도 된다.
🧬 Code Graph Analysis (3)
src/main/java/com/debatetimer/service/customize/CustomizeService.java (1)
src/main/java/com/debatetimer/domain/customize/CustomizeTimeBox.java (1)
CustomizeTimeBox(8-75)
src/test/java/com/debatetimer/fixture/domain/CustomizeTableGenerator.java (2)
src/test/java/com/debatetimer/fixture/entity/CustomizeTableEntityGenerator.java (1)
Component(10-33)src/test/java/com/debatetimer/fixture/entity/MemberGenerator.java (1)
Component(7-20)
src/test/java/com/debatetimer/fixture/domain/BellGenerator.java (1)
src/test/java/com/debatetimer/fixture/domain/CustomizeTimeBoxGenerator.java (1)
Component(10-32)
⏰ 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-and-push
🔇 Additional comments (58)
src/test/java/com/debatetimer/fixture/entity/MemberGenerator.java (1)
1-1: 테스트 픽스처의 패키지 구조 개선이 적절합니다.엔티티 관련 생성기를 별도 패키지(
fixture.entity)로 분리하여 도메인 생성기와 명확히 구분한 것이 좋은 구조적 개선입니다.src/test/java/com/debatetimer/fixture/entity/PollEntityGenerator.java (3)
1-1: 패키지 구조 개선이 일관성 있게 적용되었습니다.
fixture.entity패키지로의 이동이 적절합니다.
11-11: 클래스명 변경이 명확성을 향상시킵니다.
PollGenerator에서PollEntityGenerator로 변경하여 엔티티 생성기임을 명확히 표현했습니다.
15-15: 생성자명 변경이 일관성 있게 적용되었습니다.클래스명 변경에 맞춰 생성자명도 적절히 업데이트되었습니다.
src/test/java/com/debatetimer/fixture/domain/CustomizeTimeBoxGenerator.java (3)
1-12: 도메인 생성기 클래스 구조가 적절합니다.
fixture.domain패키지에 위치하여 엔티티 생성기와 명확히 구분되며, Spring Component로 등록하여 의존성 주입이 가능합니다.
13-21: 기본 generate 메서드가 적절한 테스트 데이터를 제공합니다.고정된 테스트 값들(Stance.PROS, "입론", "콜리", 10)을 사용하여 일관성 있는 테스트 데이터 생성이 가능합니다.
23-31: speechType 커스터마이징을 위한 오버로드 메서드가 유용합니다.다양한 speechType으로 테스트할 수 있도록 하는 오버로드 메서드가 테스트 유연성을 향상시킵니다.
src/main/java/com/debatetimer/service/poll/PollService.java (1)
7-7: 도메인 관심사에 따른 적절한 패키지 이동입니다.
CustomizeTableDomainRepository를poll패키지에서customize패키지로 이동한 것이 도메인 응집도를 높이는 좋은 리팩토링입니다.src/main/java/com/debatetimer/domain/customize/BellType.java (1)
3-9: 종소리 타이밍을 명확히 분류하는 적절한 enum 설계입니다.
AFTER_START,BEFORE_END,AFTER_END로 종소리의 타이밍을 명확하게 구분하여 Bell API V2의 요구사항을 잘 반영했습니다.src/main/resources/db/migration/V12__add_bell_type_into_bell.sql (1)
1-2: NOT NULL 컬럼 추가 시 기존 데이터 마이그레이션 위험기존
bell레코드가 있을 경우NOT NULL제약 때문에 마이그레이션이 실패합니다.
- 서비스 배포 상태를 다시 확인하고,
- 필요하면 두 단계(
ADD COLUMN … NULL→ 데이터 백필 →ALTER … SET NOT NULL)로 진행하거나 기본값을 지정하세요.-ADD COLUMN bell_type enum ('AFTER_START','BEFORE_END','AFTER_END') NOT NULL +ADD COLUMN bell_type enum ('AFTER_START','BEFORE_END','AFTER_END') NULL; + +-- 데이터 백필 후 +ALTER TABLE bell + MODIFY bell_type enum ('AFTER_START','BEFORE_END','AFTER_END') NOT NULL;src/test/java/com/debatetimer/repository/customize/CustomizeTimeBoxRepositoryTest.java (1)
28-36: 엔티티 제너레이터로의 교체 확인 완료타임박스-관련 테스트들이 모두
EntityGenerator를 사용하도록 일관성 있게 수정되었습니다. 변경 의도와 부합하며 추가 피드백 없습니다.src/test/java/com/debatetimer/domainrepository/poll/PollDomainRepositoryTest.java (1)
28-29: 폴/테이블 엔티티 제너레이터 교체가 적절합니다테스트 픽스처 생성 로직이 persistence-layer용 제너레이터로 명확히 분리되어 가독성이 좋아졌습니다. 문제 없는 변경으로 보입니다.
Also applies to: 44-46, 66-68
src/test/java/com/debatetimer/domainrepository/poll/VoteDomainRepositoryTest.java (1)
28-32: 명명 규칙 개선 승인픽스처 생성기의 이름을 더 명확하게 변경한 것이 좋습니다.
tableEntityGenerator,pollEntityGenerator,voteEntityGenerator로 변경하여 엔티티를 생성한다는 의도가 더 명확해졌습니다.src/test/java/com/debatetimer/controller/poll/PollControllerTest.java (1)
27-27: 일관된 명명 규칙 적용 승인전체 테스트 클래스에서 픽스처 생성기 명명 규칙을 일관되게 적용한 것이 좋습니다. 엔티티 생성기임을 명확히 표현하는 네이밍으로 코드 가독성이 향상되었습니다.
Also applies to: 45-49, 78-79
src/test/java/com/debatetimer/service/poll/PollServiceTest.java (1)
30-30: 서비스 테스트에서의 일관된 네이밍 적용 승인서비스 레이어 테스트에서도 동일한 엔티티 생성기 명명 규칙을 적용하여 전체 테스트 스위트의 일관성을 유지한 것이 좋습니다.
Also applies to: 45-49, 71-72
src/test/java/com/debatetimer/repository/customize/BellRepositoryTest.java (1)
6-6: BellType 도메인 모델 도입 승인새로운
BellType열거형을 도입하여 종소리 API V2의 타입 구분 기능을 지원하는 것이 좋습니다.src/test/java/com/debatetimer/fixture/entity/CustomizeTimeBoxEntityGenerator.java (1)
1-1: 픽스처 생성기 아키텍처 개선 승인픽스처 생성기를
entity패키지로 분리하고 클래스명을CustomizeTimeBoxEntityGenerator로 변경하여 엔티티 생성기임을 명확히 한 것이 좋습니다. 이는 도메인 생성기와 엔티티 생성기 간의 명확한 구분을 제공합니다.Also applies to: 11-11, 15-15
src/main/java/com/debatetimer/domain/customize/Bell.java (1)
16-22: 도메인 모델 설계가 올바르게 구현되었습니다.BellType을 포함한 새로운 생성자 시그니처가 적절하게 구현되었습니다. 기존 검증 로직을 유지하면서 새로운 타입 정보를 올바르게 캡슐화했습니다.
src/test/java/com/debatetimer/fixture/entity/CustomizeTableEntityGenerator.java (1)
1-17: 테스트 픽스처 명명 규칙 개선이 적절합니다.Entity 기반 생성기임을 명확히 하는 클래스명 변경과 패키지 구조 개선이 좋습니다. 코드의 가독성과 일관성을 향상시킵니다.
src/main/java/com/debatetimer/dto/customize/response/BellResponse.java (1)
6-14: DTO 구현이 도메인 변경사항을 올바르게 반영했습니다.BellType 필드 추가와 도메인 객체로부터의 매핑이 정확하게 구현되었습니다. Record 기반 구현도 깔끔합니다.
src/test/java/com/debatetimer/controller/customize/CustomizeControllerTest.java (1)
157-159: 테스트 픽스처 변수명이 일관성 있게 업데이트되었습니다.EntityGenerator로의 명명 규칙 통일이 잘 적용되었습니다. 테스트 코드의 가독성과 일관성이 개선되었습니다.
src/test/java/com/debatetimer/fixture/entity/VoteEntityGenerator.java (1)
1-17: 테스트 픽스처 구조 개선이 일관성 있게 적용되었습니다.VoteGenerator에서 VoteEntityGenerator로의 이름 변경과 entity 패키지로의 이동이 다른 generator들과 일관성 있게 적용되었습니다.
src/test/java/com/debatetimer/service/member/MemberServiceTest.java (1)
55-56: 테스트 픽스처 생성기 이름 변경이 올바르게 적용되었습니다.도메인과 엔티티 레이어를 구분하기 위한 리팩토링의 일환으로
customizeTableGenerator를customizeTableEntityGenerator로 변경한 것이 일관성 있게 적용되었습니다.Also applies to: 66-66
src/test/java/com/debatetimer/entity/customize/BellEntityTest.java (1)
5-5: BellEntity 생성자 시그니처 변경이 올바르게 반영되었습니다.새로운
BellType매개변수가 정확히 추가되었고, 도메인 테스트와 다른BellType.AFTER_END를 사용하여 다양성을 제공하는 것이 좋습니다.Also applies to: 20-20, 28-28
src/test/java/com/debatetimer/repository/BaseRepositoryTest.java (1)
4-7: 엔티티 픽스처 생성기 패키지 및 클래스명 변경이 일관성 있게 적용되었습니다.도메인과 엔티티 레이어를 구분하기 위한 리팩토링이 베이스 테스트 클래스에서 올바르게 반영되었습니다. 모든 Import, @import 어노테이션, @Autowired 필드가 일관성 있게 업데이트되었습니다.
Also applies to: 15-17, 26-26, 29-29, 32-32
src/main/java/com/debatetimer/dto/customize/request/BellRequest.java (1)
1-17: 변경사항이 적절합니다!
BellType을 필수 필드로 추가하고@NotNull검증을 적용한 것이 V2 API 요구사항에 부합합니다. 도메인 객체 생성 시 파라미터 순서도 일관되게 유지되었습니다.src/test/java/com/debatetimer/fixture/domain/CustomizeTableGenerator.java (1)
1-36: 도메인 테스트 픽스처 생성기가 잘 구현되었습니다!도메인 계층과 엔티티 계층의 테스트 픽스처를 분리한 설계가 DDD 접근 방식에 적합합니다. 기본값과 커스텀 이름을 지원하는 두 가지 메서드 제공도 테스트 유연성을 높여줍니다.
src/main/java/com/debatetimer/dto/customize/request/CustomizeTableCreateRequest.java (1)
14-18: 리팩토링이 코드를 단순화했습니다!복잡한 인덱스 기반 변환 로직을 제거하고 도메인 객체를 직접 생성하는 방식으로 개선되었습니다. 이는 도메인 중심 설계와 일치하며 코드의 가독성과 유지보수성을 향상시킵니다.
src/main/java/com/debatetimer/service/customize/CustomizeService.java (1)
1-59: 도메인 리포지토리 패턴으로의 리팩토링이 훌륭합니다!서비스 계층이 도메인 객체를 사용하고 모든 영속성 작업을
CustomizeTableDomainRepository에 위임하는 구조로 개선되었습니다. 이는 DDD 원칙에 부합하며 관심사의 분리를 명확히 합니다.src/test/java/com/debatetimer/fixture/entity/BellEntityGenerator.java (3)
1-3: 패키지 이동과 import 추가가 적절합니다.
BellEntityGenerator가com.debatetimer.fixture.entity패키지로 이동되고 새로운BellTypeimport가 추가되어 엔티티 생성기와 도메인 생성기를 명확히 구분하고 있습니다.
10-16: 클래스명 변경과 생성자가 일관성 있게 수정되었습니다.
BellGenerator에서BellEntityGenerator로 클래스명이 변경되어 엔티티 생성기임을 명확히 나타내고 있습니다.
18-21: 새로운 BellType 매개변수가 올바르게 적용되었습니다.
generate메서드가BellType type매개변수를 받아BellEntity생성자에 전달하고 있어, 새로 도입된 종소리 타입 기능과 잘 통합되어 있습니다.src/test/java/com/debatetimer/service/BaseServiceTest.java (2)
4-9: 테스트 픽스처 생성기 import가 일관성 있게 업데이트되었습니다.모든 생성기 import가
com.debatetimer.fixture.entity패키지로 변경되어 엔티티 생성기와 도메인 생성기를 명확히 구분하고 있습니다.
44-56: 필드명이 엔티티 생성기 명명 규칙에 맞게 수정되었습니다.모든 생성기 필드명에 "Entity" 접미사가 추가되어 (
customizeTableEntityGenerator,bellEntityGenerator등) 엔티티 레이어 테스트 생성기임을 명확히 나타내고 있습니다.src/main/java/com/debatetimer/entity/customize/BellEntity.java (5)
4-4: BellType import와 JPA 어노테이션이 적절하게 추가되었습니다.새로운
BellTypeenum을 위한 import와@EnumType,@Enumerated어노테이션이 올바르게 추가되었습니다.Also applies to: 9-11
40-44: BellType 필드 매핑이 올바르게 구현되었습니다.데이터베이스의
bell_type컬럼과 매핑되는type필드가@NotNull과@Enumerated(EnumType.STRING)로 적절히 설정되어 있습니다.
49-57: 생성자가 BellType 매개변수를 포함하도록 올바르게 수정되었습니다.첫 번째 생성자에서
BellType type매개변수를 받아 필드에 할당하고 있어, 새로운 종소리 타입 기능이 제대로 통합되었습니다.
59-64: 도메인 객체 기반 생성자가 적절히 구현되었습니다.
Bell도메인 객체로부터type을 추출하여 엔티티 필드에 설정하는 로직이 올바르게 구현되어 있습니다.
78-80: 도메인 변환 메서드가 새로운 type 필드를 포함하도록 수정되었습니다.
toDomain()메서드가type매개변수를 포함하여Bell도메인 객체를 생성하고 있어, 엔티티와 도메인 간의 변환이 완전하게 이루어집니다.src/test/java/com/debatetimer/service/customize/CustomizeServiceTest.java (5)
7-7: BellType import와 BellEntity import가 적절히 추가되었습니다.새로운
BellTypeenum과BellEntity클래스의 import가 추가되어 테스트에서 종소리 타입 기능을 활용할 수 있게 되었습니다.Also applies to: 16-16
45-48: BellRequest에 BellType이 올바르게 적용되었습니다.모든
BellRequest생성자 호출에서BellType.AFTER_START를 첫 번째 매개변수로 전달하고 있어, 새로운 종소리 타입 기능이 테스트에 정확히 반영되었습니다.
54-62: 엔티티 조회 로직과 검증이 적절히 구현되었습니다.
CustomizeTimeBoxEntity와BellEntity를 조회하여 저장된 데이터를 검증하는 로직이 추가되어, 도메인 레포지토리를 통한 영속화가 올바르게 동작하는지 확인할 수 있습니다.
72-77: 엔티티 생성기를 사용한 테스트 데이터 생성이 일관성 있게 수정되었습니다.
customizeTableEntityGenerator,customizeTimeBoxEntityGenerator,bellEntityGenerator를 사용하여 테스트 데이터를 생성하고,BellType.AFTER_START매개변수를 포함하고 있어 새로운 테스트 인프라와 잘 통합되어 있습니다.
83-86: 응답 검증 로직이 종소리 데이터를 포함하도록 확장되었습니다.
foundResponse.table().get(0).bell().hasSize(2)와 같은 검증을 통해 종소리 데이터가 응답에 올바르게 포함되는지 확인하고 있습니다.src/test/java/com/debatetimer/domainrepository/customize/CustomizeTableDomainRepositoryTest.java (7)
1-23: 새로운 도메인 레포지토리 테스트 클래스가 적절히 구성되었습니다.
CustomizeTableDomainRepository를 위한 종합적인 테스트 클래스가 생성되어 필요한 import와 의존성 주입이 올바르게 설정되어 있습니다.
32-50: 저장 테스트가 도메인과 엔티티 레이어 통합을 잘 검증합니다.도메인 생성기로 생성한 객체들을 도메인 레포지토리에 저장한 후, 엔티티 레포지토리를 통해 실제 영속화를 검증하는 구조가 적절히 구현되어 있습니다. 특히 종소리 3개가 올바르게 저장되는지 확인하고 있습니다.
56-64: 회원 소유권 기반 조회 테스트가 올바르게 구현되었습니다.
getByIdAndMember메서드의 기본 동작을 검증하는 간단하고 명확한 테스트입니다.
70-81: 시간박스 조회 테스트가 적절히 구현되었습니다.테스트 데이터를 엔티티 생성기로 설정한 후 도메인 레포지토리의
getCustomizeTimeBoxes메서드를 검증하는 구조가 올바릅니다.
87-113: 업데이트 테스트가 종소리 데이터 교체를 포함하여 포괄적으로 검증합니다.기존 데이터(종소리 3개)를 새로운 데이터(종소리 3개)로 교체하는 업데이트 로직을 검증하여, 도메인 레포지토리의 업데이트 기능이 올바르게 동작함을 확인하고 있습니다.
119-131: 사용 시간 업데이트 테스트가 간결하고 명확합니다.
updateUsedAt메서드의 동작을 검증하여 사용 시간이 올바르게 갱신되는지 확인하고 있습니다.
137-157: 삭제 테스트가 연관 데이터의 CASCADE 삭제를 적절히 검증합니다.테이블 삭제 시 연관된
CustomizeTimeBoxEntity와BellEntity가 모두 삭제되는지 확인하여 데이터 정합성을 보장하는 테스트입니다.src/test/java/com/debatetimer/domainrepository/BaseDomainRepositoryTest.java (1)
4-63: 테스트 인프라 추가가 적절합니다!Bell 도메인과 관련된 새로운 픽스처 생성기 및 리포지토리 필드 추가가 일관성 있게 잘 구현되었습니다. 도메인과 엔티티 레벨의 생성기를 명확히 구분한 점도 좋습니다.
src/main/java/com/debatetimer/domainrepository/customize/CustomizeTableDomainRepository.java (6)
28-33: 저장 로직이 올바르게 구현되었습니다!테이블 엔티티를 먼저 저장한 후 연관된 타임박스들을 저장하는 순서가 적절합니다.
35-38: 시퀀스 처리가 깔끔합니다!IntStream을 사용하여 시퀀스를 1부터 시작하도록 처리한 점이 좋습니다.
40-45: Bell 엔티티 저장 로직이 적절합니다!타임박스를 저장한 후 관련된 Bell들을 저장하는 순서가 올바릅니다.
48-51: 조회 메서드가 깔끔하게 구현되었습니다!읽기 전용 트랜잭션 설정이 적절합니다.
53-60: 연관 엔티티 조회가 효율적입니다!CustomizeTimeBoxEntities를 통한 도메인 객체 변환 패턴이 좋습니다.
74-88: 삭제 로직의 순서가 올바릅니다!외래 키 제약 조건을 고려하여 Bell → TimeBox → Table 순서로 삭제하는 것이 적절합니다.
leegwichan
left a comment
leegwichan
left a comment
There was a problem hiding this comment.
/noti @unifolio0
종소리 유효성 검사 중에, 반영안된 치명적인 문제가 있어 RC 남겼어요!
그외에는 사소한 내용이니, 의견만 남겨주세요!
(그리고 술 한 캔 마시고 하는 리뷰이니, 틀린 것 있으면 너그럽게 넘어가주세요~)
| private final BellType type; | ||
| private final int time; | ||
| private final int count; | ||
|
|
||
| public Bell(int time, int count) { | ||
| public Bell(BellType type, int time, int count) { | ||
| validateTime(time); | ||
| validateCount(count); | ||
| this.type = type; | ||
| this.time = time; | ||
| this.count = count; | ||
| } |
There was a problem hiding this comment.
님, time 검증이 Bell Type마다 다릅니다. 검증 해주세요.
There was a problem hiding this comment.
이걸 못 봤었네요. 추가 했습니다~
| @Test | ||
| void 사용자_지정_테이블을_조회한다() { | ||
| Member bito = memberGenerator.generate("default@gmail.com"); | ||
| CustomizeTableEntity bitoTable = customizeTableGenerator.generate(bito); | ||
| customizeTimeBoxGenerator.generate(bitoTable, CustomizeBoxType.NORMAL, 1); | ||
| customizeTimeBoxGenerator.generateNotExistSpeaker(bitoTable, CustomizeBoxType.NORMAL, 2); | ||
| CustomizeTableEntity bitoTable = customizeTableEntityGenerator.generate(bito); | ||
| customizeTimeBoxEntityGenerator.generate(bitoTable, CustomizeBoxType.NORMAL, 1); | ||
| customizeTimeBoxEntityGenerator.generateNotExistSpeaker(bitoTable, CustomizeBoxType.NORMAL, 2); | ||
| Headers headers = headerGenerator.generateAccessTokenHeader(bito); |
There was a problem hiding this comment.
이걸 또 EntityGenerator 로 바꿔 놨음? ㄷㄷ;;
| class CustomizeTableDomainRepositoryTest extends BaseDomainRepositoryTest { | ||
|
|
||
| @Autowired | ||
| private CustomizeTableDomainRepository customizeTableDomainRepository; |
There was a problem hiding this comment.
CustomizeTableDomainRepositoryTest까지 만드느라 고생했고~
| @Component | ||
| public class CustomizeTableGenerator { | ||
|
|
||
| public CustomizeTable generate(Member member) { | ||
| return new CustomizeTable( | ||
| member, | ||
| "토론 테이블", | ||
| "주제", | ||
| "찬성", | ||
| "반대", | ||
| false, | ||
| false, | ||
| LocalDateTime.now() | ||
| ); | ||
| } |
There was a problem hiding this comment.
도메인 객체 Generator는 어떻게 할지 이야기 해봐야 할 듯? (아마 다들 동의할 것 같기는 하지만...)
|
/noti |
coli-geonwoo
left a comment
coli-geonwoo
left a comment
There was a problem hiding this comment.
/noti
@unifolio0 비토! 진짜 고생많았어!! 몇가지 리뷰 남겼으니 확인 부탁드려요~
|
|
||
| public enum BellType { | ||
|
|
||
| AFTER_START(time -> time >= 0), |
There was a problem hiding this comment.
[질문]
이거 타입 + 양수 시간으로 나타내자고 하지 않았나요? 왜 AFTER_END에서는 음수죠? 치코랑 협의가 된 부분인지 궁금해요.
AFTER_END 30 이면 끝난 후 30초 등등으로 해석하기로 했던 것 같은데요?
| AFTER_END(time -> time <= 0), | ||
| ; | ||
|
|
||
| private final Predicate<Integer> timeValidator; |
There was a problem hiding this comment.
[제안]
IntPredicate 는 Wrapping Type을 쓰지 않아서 더 빠르다는 장점이 있습니당
| private final Predicate<Integer> timeValidator; | |
| private final IntPredicate timeValidator; |
| return savedTableEntity.toDomain(); | ||
| } | ||
|
|
||
| private void saveTimeBoxes(CustomizeTableEntity tableEntity, List<CustomizeTimeBox> timeBoxes) { |
There was a problem hiding this comment.
[질문]
아마 비토가 작성한 코드는 아닌 것 같은데, 이거 sequence mapping을 왜 domain repository에서 해주나요?
도메인이 sequence 자체를 들고 있으면 더 좋을 것 같아요.
혹은 service에서의 매핑이나 CustomizeTimeBoxes라는 일급컬렉션 도메인 객체도 고려해보면 좋을 것 같아서요..
도메인 레포지토리는 '순차대로 입력받는다' 라는 웹 프론트와 백엔드간의 협의사실을 모르게하는게 맞다고 생각하는데 비토 의견은 어떤지 궁금합니다.
There was a problem hiding this comment.
CustomizeTimeBoxes를 고민 안 해봤던 건 아닌데 문제는 그 IntStream을 사용하는 로직을 CustomizeTimeBoxes안에 만드는 방법이 안보였습니다. 그래서 콜리도 이전에 해당 로직을 dto에 뒀던 거로 기억해요.
이번에 4계층을 적용하면서 해당 로직을 dto or domain repository 둘 중 어디에 둬야 될지 선택을 했어야 했는데 저는 dto보단 domain repository가 더 맘에 들어서 이곳에 위치시켰습니다.
- 말한대로 '순차대로 입력받는다' 이건 프론트와 백엔드간의 협의 사실인데 저는 이런 규칙은 도메인 규칙이라고 생각합니다. 그런데 dto가 도메인 규칙을 알고 있다는 것보단 domain repository에서 매핑하는게 더 알맞다고 생각했어요.
- 지금 코드에서 domain repository가 프론트와 백엔드간의 협의사실을 알고 있다고 생각되지 않았어요. 말한대로 프론트와 백엔드간의 협의 사실은 서비스 단까지만 알아야 된다고 생각합니다. 그런데 지금 파라미터로 넘겨주는 것은 dto가 아닌 domain입니다. 그래서 지금 domain repository는 프론트와 백엔드간의 협의 사실을 안다기보단 서비스에서 순서대로 정렬해서 보냈으니 저장할때 순서를 추가해준다 정도로 봤습니다.
There was a problem hiding this comment.
- 만약 순차대로 입력받는다가 도메인 규칙이라면 일전에 회의에서
도메인 규칙을 domain repository는 모르게 하고 domain 매핑 위주의 레포지토리 역할만 수행하게 한다에서 어긋나는 것 같은데 어느정도까지 도메인 규칙을 domain repository에게 위임가능하다고 생각하나요? 제가 궁금한건 메서드 명은 레포지토리처럼 유지하되 도메인 규칙을 넘기는 비일관성에 대한 문제입니다.
또한, 상위 계층에서 dto -> domain을 할 때에 순차적으로 매핑하여 보내주어야 함이라는 것이 도메인 레포지토리 안의 private method를 보아야지만 알 수 있는 것이 바람직하지 않다고 생각합니다.
따라서 백엔드-프론트 간의 API 규약에 대해서 매핑하는 것은 dto가 처리하는게 맞지 않는가? 가 제 입장입니다.
There was a problem hiding this comment.
만약 순차대로 입력받는다가 도메인 규칙이라면 일전에 회의에서 도메인 규칙을 domain repository는 모르게 하고 domain 매핑 위주의 레포지토리 역할만 수행하게 한다에서 어긋나는 것 같은데 어느정도까지 도메인 규칙을 domain repository에게 위임가능하다고 생각하나요? 제가 궁금한건 메서드 명은 레포지토리처럼 유지하되 도메인 규칙을 넘기는 비일관성에 대한 문제입니다.
또한, 상위 계층에서 dto -> domain을 할 때에 순차적으로 매핑하여 보내주어야 함이라는 것이 도메인 레포지토리 안의 private method를 보아야지만 알 수 있는 것이 바람직하지 않다고 생각합니다.
따라서 백엔드-프론트 간의 API 규약에 대해서 매핑하는 것은 dto가 처리하는게 맞지 않는가? 가 제 입장입니다.
제 답변의 2번에서 언급했든이 순차대로 입력받는다는 도메인 규칙을 도메인 레포지토리가 모르는 상태라고 생각하고 있습니다. 그래서 비일관성이라는 점은 제 기준에서는 아니라는 점을 먼저 언급하고 싶네요.
다만 상위 계층에서 dto -> domain을 할 때에 순차적으로 매핑하여 보내주어야 함이라는 것이 도메인 레포지토리 안의 private method를 보아야지만 알 수 있는 것이 바람직하지 않다고 생각합니다. 이 부분은 뼈맞았네요. 좀 더 고민해보겠습니다.
There was a problem hiding this comment.
제가 이것을 보고도 그냥 넘어갔던 이유는... List가 순서가 매겨져 있기 때문이라고 생각해요. 각 타임박스의 순서가 정해져있기 때문에 'List'라는 도메인을 저장한다면 순서에 맞춰 저장할 것이라고 충분히 생각할 수 있다 생각했어요. 제 관점에서 본다면 List 보다 CustomizeTimeBoxes 와 같은 Wrapper 도메인 객체를 만들어 저장했다면, 비토의 현재 로직이 합리적이지 않았을까 생각합니다.
상위 계층에서 dto -> domain을 할 때에 순차적으로 매핑하여 보내주어야 함이라는 것이 도메인 레포지토리 안의 private method를 보아야지만 알 수 있는 것이 바람직하지 않다고 생각합니다.
자료 구조가 Set 도 아니고 List라면... 이걸 매핑하는 개발자가 역순이나 섞어서 매핑하려 하지는 않을 것 같긴 해요. "순차적으로 매핑하여 보내주어야 함"을 얼마나 강조하고 싶은지의 문제일 수는 있겠네요.
제 최종 의견은... 자료 구조가 List 라는 것만으로도 충분히 '순서를 보장해야 한다'라는 이미지를 줄 수 있다 생각해요. (만약에 그렇지 않은 경우가 있을 때, 순서대로 매핑하는 것을 방지할 수 있는 장치가 있어야 한다고 생각합니다.)
콜리의 의견도 납득이 되기는 하지만, 그것에 맞춘 대안이 잘 떠오르지는 않네요. "도메인이 sequence를 가지게 한다"라는 의견을 제시했는데요. 저는 List 내부의 인덱스와 중복되는 느낌을 강하게 받긴 합니다.
| CustomizeTimeBoxEntities timeBoxes = timeBoxRepository.findTableTimeBoxes(tableEntity); | ||
| return new CustomizeTableResponse(tableEntity.toDomain(), timeBoxes); | ||
| CustomizeTable table = customizeTableDomainRepository.getByIdAndMember(tableId, member); | ||
| List<CustomizeTimeBox> timeBoxEntities = customizeTableDomainRepository.getCustomizeTimeBoxes(tableId, member); |
There was a problem hiding this comment.
[사소한 제안]
변수명이 timeBoxEntites인데 도메인 객체라 timeBoxes 어떨지 제안남겨보아요!
| List<CustomizeTimeBox> timeBoxEntities = customizeTableDomainRepository.getCustomizeTimeBoxes(tableId, member); | |
| List<CustomizeTimeBox> timeBoxes = customizeTableDomainRepository.getCustomizeTimeBoxes(tableId, member); |
| @@ -0,0 +1,2 @@ | |||
| ALTER TABLE bell | |||
There was a problem hiding this comment.
이거 내거랑 충돌나겠네 ㅋㅋ 머지 순서 주의해야겠다
| import com.debatetimer.controller.RestDocumentationRequest; | ||
| import com.debatetimer.controller.RestDocumentationResponse; | ||
| import com.debatetimer.controller.Tag; | ||
| import com.debatetimer.domain.customize.BellType; |
There was a problem hiding this comment.
문서화 코드 작성 안한줄 알았는데 변경사안이 너무 많아서 Load Diff로 뜨는 거였구나... 미안하다 ㅋㅋ
leegwichan
left a comment
leegwichan
left a comment
There was a problem hiding this comment.
/noti @unifolio0
크게 뭐라할 부분은 없어서 그런지 반영이 빠르네요 👍
Approve 드렸습니다!
| public boolean isValidTime(int time) { | ||
| return timeValidator.test(time); | ||
| } |
There was a problem hiding this comment.
[의견] 'BellType에 따른 유효성 검사가 통과하지 않았을 때, 적합한 예외를 던지는 것'은 Bell의 책임일까? BellType의 책임일까?
|
/noti |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/test/java/com/debatetimer/domain/customize/BellTypeTest.java (1)
15-35: 양성 테스트 케이스 추가를 고려해보세요.현재 테스트는 유효하지 않은 입력에 대한 예외 발생만 검증하고 있습니다. 완전한 테스트 커버리지를 위해 유효한 입력에 대해서는 예외가 발생하지 않음을 검증하는 양성 테스트 케이스도 추가하는 것을 권장합니다.
다음과 같은 양성 테스트 케이스를 추가할 수 있습니다:
+ @Test + void 벨_타입이_AFTER_START일때_0이상_시간은_유효하다() { + assertThatCode(() -> BellType.AFTER_START.validateTime(0)) + .doesNotThrowAnyException(); + assertThatCode(() -> BellType.AFTER_START.validateTime(60)) + .doesNotThrowAnyException(); + } + + @Test + void 벨_타입이_BEFORE_END일때_0이상_시간은_유효하다() { + assertThatCode(() -> BellType.BEFORE_END.validateTime(0)) + .doesNotThrowAnyException(); + assertThatCode(() -> BellType.BEFORE_END.validateTime(30)) + .doesNotThrowAnyException(); + } + + @Test + void 벨_타입이_AFTER_END일때_0이하_시간은_유효하다() { + assertThatCode(() -> BellType.AFTER_END.validateTime(0)) + .doesNotThrowAnyException(); + assertThatCode(() -> BellType.AFTER_END.validateTime(-30)) + .doesNotThrowAnyException(); + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/debatetimer/domain/customize/Bell.java(1 hunks)src/main/java/com/debatetimer/domain/customize/BellType.java(1 hunks)src/main/java/com/debatetimer/service/customize/CustomizeService.java(3 hunks)src/test/java/com/debatetimer/domain/customize/BellTest.java(1 hunks)src/test/java/com/debatetimer/domain/customize/BellTypeTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/test/java/com/debatetimer/domain/customize/BellTest.java
- src/main/java/com/debatetimer/domain/customize/Bell.java
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: leegwichan
PR: debate-timer/debate-timer-be#201
File: src/main/java/com/debatetimer/entity/customize/BellEntity.java:50-54
Timestamp: 2025-07-18T01:04:20.985Z
Learning: BellEntity에서는 도메인 레벨에서 이미 유효성 검사가 수행되므로 엔티티 레벨의 유효성 검사는 중복이며, 향후 엔티티의 유효성 검사를 제거할 예정이다.
src/main/java/com/debatetimer/service/customize/CustomizeService.java (1)
Learnt from: leegwichan
PR: #198
File: src/test/java/com/debatetimer/domain/customize/CustomizeTimeBoxDomainTest.java:82-112
Timestamp: 2025-07-15T01:28:24.899Z
Learning: CustomizeTimeBoxDomain 클래스에서는 생성자에서 stance null 검증을 먼저 수행한 후 isValidStance() 추상 메서드를 호출하는 구조로 되어 있어, 테스트용 구현체에서 isValidStance()는 단순히 true를 반환해도 된다.
src/test/java/com/debatetimer/domain/customize/BellTypeTest.java (2)
Learnt from: leegwichan
PR: #198
File: src/test/java/com/debatetimer/domain/customize/CustomizeTimeBoxDomainTest.java:82-112
Timestamp: 2025-07-15T01:28:24.899Z
Learning: CustomizeTimeBoxDomain에서 validateStance 메서드는 null 체크를 먼저 수행한 후에 isValidStance() 추상 메서드를 호출하는 구조로, 테스트용 구현체에서 isValidStance()가 단순히 true를 반환하는 것이 적절하다.
Learnt from: leegwichan
PR: #198
File: src/test/java/com/debatetimer/domain/customize/CustomizeTimeBoxDomainTest.java:82-112
Timestamp: 2025-07-15T01:28:24.899Z
Learning: CustomizeTimeBoxDomain 클래스에서는 생성자에서 stance null 검증을 먼저 수행한 후 isValidStance() 추상 메서드를 호출하는 구조로 되어 있어, 테스트용 구현체에서 isValidStance()는 단순히 true를 반환해도 된다.
🧬 Code Graph Analysis (2)
src/main/java/com/debatetimer/domain/customize/BellType.java (1)
src/main/java/com/debatetimer/exception/custom/DTClientErrorException.java (1)
DTClientErrorException(5-10)
src/test/java/com/debatetimer/domain/customize/BellTypeTest.java (2)
src/main/java/com/debatetimer/exception/custom/DTClientErrorException.java (1)
DTClientErrorException(5-10)src/test/java/com/debatetimer/domain/customize/BellTest.java (1)
Nested(14-31)
⏰ 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-and-push
🔇 Additional comments (10)
src/main/java/com/debatetimer/service/customize/CustomizeService.java (6)
4-4: 도메인 중심 설계로의 좋은 전환입니다.CustomizeTimeBox 도메인 객체 임포트와 CustomizeTableDomainRepository 의존성 추가로 엔티티 기반 접근에서 도메인 중심 접근으로 깔끔하게 전환되었습니다. 이를 통해 서비스 레이어에서 엔티티 변환 로직을 제거하고 비즈니스 로직에 집중할 수 있게 되었습니다.
Also applies to: 6-6, 18-18
23-25: 도메인 객체 중심의 깔끔한 저장 로직입니다.도메인 repository를 통해 테이블과 타임박스를 함께 저장하는 로직이 간소화되었습니다. 엔티티 변환 과정이 도메인 repository로 캡슐화되어 서비스 레이어의 복잡도가 줄어들었습니다.
30-32: 조회 로직의 일관성이 개선되었습니다.도메인 repository를 통해 테이블과 타임박스를 조회하는 로직이 통일되었습니다. 엔티티-도메인 변환 과정이 repository 레이어로 이동하여 서비스 레이어가 더 직관적이 되었습니다.
41-45: 업데이트 로직이 더욱 명확해졌습니다.도메인 repository의 update 메서드를 통해 테이블과 타임박스를 함께 업데이트하는 로직이 간소화되었습니다. 복잡한 엔티티 조작 로직이 repository 레이어로 캡슐화되어 서비스 레이어의 가독성이 향상되었습니다.
50-52: 사용 시간 업데이트 로직이 깔끔해졌습니다.도메인 repository를 통해 사용 시간을 업데이트하고 관련 데이터를 조회하는 로직이 일관성 있게 구현되었습니다. 불필요한 엔티티 변환 과정이 제거되어 코드가 더 간결해졌습니다.
57-57: 삭제 로직이 간소화되었습니다.도메인 repository의 delete 메서드를 통해 삭제 로직이 크게 간소화되었습니다. 복잡한 엔티티 조작이 repository 레이어로 위임되어 서비스 레이어의 책임이 명확해졌습니다.
src/main/java/com/debatetimer/domain/customize/BellType.java (3)
9-11: 벨 타입별 시간 검증 로직이 올바르게 구현되었습니다.각 벨 타입에 맞는 시간 검증 로직이 정확합니다:
AFTER_START: 토론 시작 후 울리는 벨 (0 이상)BEFORE_END: 토론 종료 전 울리는 벨 (0 이상)AFTER_END: 토론 종료 후 울리는 벨 (0 이하)과거 리뷰 코멘트에서 논의된 대로
AFTER_END에서 음수/0 시간을 허용하는 것이 비즈니스 로직에 맞습니다.
14-14: IntPredicate 사용으로 성능이 최적화되었습니다.과거 리뷰 피드백을 반영하여
IntPredicate를 사용함으로써 박싱 없이 원시 타입을 직접 처리할 수 있어 성능상 이점이 있습니다.
20-24: 검증 책임이 BellType으로 적절히 분리되었습니다.과거 리뷰에서 논의된 대로 시간 검증 책임이
BellType으로 이동되어 응집도가 향상되었습니다. 각 벨 타입이 자신의 검증 로직을 담당하는 것이 객체지향 설계 원칙에 부합합니다.src/test/java/com/debatetimer/domain/customize/BellTypeTest.java (1)
12-13: 테스트 구조가 일관성 있고 명확합니다.
@Nested클래스를 사용하여validateTime메서드에 대한 테스트를 논리적으로 그룹화한 것이 좋습니다. 프로젝트의 테스트 컨벤션과 일치합니다.
coli-geonwoo
left a comment
coli-geonwoo
left a comment
There was a problem hiding this comment.
/noti
어제 오후일찍부터 그대로 잠들어가지고... 한가지 리뷰 남겼으니 확인 부탁합니다! approve 할게요~
| return savedTableEntity.toDomain(); | ||
| } | ||
|
|
||
| private void saveTimeBoxes(CustomizeTableEntity tableEntity, List<CustomizeTimeBox> timeBoxes) { |
There was a problem hiding this comment.
- 만약 순차대로 입력받는다가 도메인 규칙이라면 일전에 회의에서
도메인 규칙을 domain repository는 모르게 하고 domain 매핑 위주의 레포지토리 역할만 수행하게 한다에서 어긋나는 것 같은데 어느정도까지 도메인 규칙을 domain repository에게 위임가능하다고 생각하나요? 제가 궁금한건 메서드 명은 레포지토리처럼 유지하되 도메인 규칙을 넘기는 비일관성에 대한 문제입니다.
또한, 상위 계층에서 dto -> domain을 할 때에 순차적으로 매핑하여 보내주어야 함이라는 것이 도메인 레포지토리 안의 private method를 보아야지만 알 수 있는 것이 바람직하지 않다고 생각합니다.
따라서 백엔드-프론트 간의 API 규약에 대해서 매핑하는 것은 dto가 처리하는게 맞지 않는가? 가 제 입장입니다.
|
/noti 콜리 답변 남겼습니다. 커찬도 확인해주세요. |
# Conflicts: # src/test/java/com/debatetimer/controller/BaseControllerTest.java
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/main/java/com/debatetimer/repository/poll/PollRepository.java (1)
13-21: 예외 처리 중앙화가 잘 구현되었습니다
getById와getByIdAndMemberId메소드를 통해 Optional 처리와 예외 발생을 repository 레벨에서 중앙화한 것은 좋은 접근입니다. 호출자가 매번 Optional을 처리할 필요가 없어 코드 중복을 줄여줍니다.다만 DDD 관점에서 repository는 Optional을 반환하고 도메인 레이어에서 비즈니스 로직에 따라 예외를 처리하는 것이 더 적절할 수도 있습니다. 현재 구조가 프로젝트 전반에서 일관되게 사용되고 있다면 문제없습니다.
src/main/java/com/debatetimer/entity/poll/VoteEntity.java (1)
17-18: 엔티티 레벨 검증 어노테이션 재검토 고려이전 학습 내용에 따르면 BellEntity에서 도메인 레벨 검증이 충분하므로 엔티티 레벨 검증을 제거할 예정이라고 했습니다. VoteEntity에서도
@NotBlank,@NotNull어노테이션들이 도메인 레벨 검증과 중복될 수 있으니 일관성을 위해 검토해보시기 바랍니다.Also applies to: 46-51
src/test/java/com/debatetimer/exception/decoder/H2ErrorDecoderTest.java (1)
38-49: 네거티브 테스트 케이스 개선 필요SQL 상태 "23000"을 사용한 네거티브 테스트가 있지만, 이는 MySQL의 일반적인 무결성 제약조건 위반 상태입니다. H2에서 실제로 발생할 수 있는 다른 SQL 상태를 사용하는 것이 더 적절합니다.
- SQLException notUniqueError = new SQLException("다른 에러", "23000"); + SQLException notUniqueError = new SQLException("다른 에러", "23001"); // H2의 다른 제약조건 위반src/main/java/com/debatetimer/service/poll/VoteService.java (1)
49-55: UUID 사용에 대한 보안 고려사항 검토 필요참여 코드 생성에
UUID.randomUUID()를 사용하고 있는데, 투표의 익명성과 보안이 중요한 경우 더 강력한 랜덤 생성기 사용을 고려해보세요.- ParticipateCode code = new ParticipateCode(UUID.randomUUID().toString()); + ParticipateCode code = new ParticipateCode(generateSecureParticipateCode());별도의 보안 강화된 코드 생성 메서드를 구현하는 것을 고려해보세요.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
src/main/java/com/debatetimer/config/ErrorDecoderConfig.java(1 hunks)src/main/java/com/debatetimer/controller/poll/VoteController.java(1 hunks)src/main/java/com/debatetimer/domain/member/Member.java(1 hunks)src/main/java/com/debatetimer/domain/poll/Poll.java(1 hunks)src/main/java/com/debatetimer/domain/poll/PollStatus.java(1 hunks)src/main/java/com/debatetimer/domain/poll/Vote.java(1 hunks)src/main/java/com/debatetimer/domainrepository/poll/PollDomainRepository.java(1 hunks)src/main/java/com/debatetimer/domainrepository/poll/VoteDomainRepository.java(2 hunks)src/main/java/com/debatetimer/dto/poll/request/VoteRequest.java(1 hunks)src/main/java/com/debatetimer/dto/poll/response/VoteCreateResponse.java(1 hunks)src/main/java/com/debatetimer/dto/poll/response/VoterPollInfoResponse.java(1 hunks)src/main/java/com/debatetimer/entity/BaseTimeEntity.java(1 hunks)src/main/java/com/debatetimer/entity/customize/CustomizeTableEntity.java(2 hunks)src/main/java/com/debatetimer/entity/poll/PollEntity.java(1 hunks)src/main/java/com/debatetimer/entity/poll/VoteEntity.java(4 hunks)src/main/java/com/debatetimer/exception/decoder/H2ErrorDecoder.java(1 hunks)src/main/java/com/debatetimer/exception/decoder/MySqlErrorDecoder.java(1 hunks)src/main/java/com/debatetimer/exception/decoder/RepositoryErrorDecoder.java(1 hunks)src/main/java/com/debatetimer/exception/errorcode/ClientErrorCode.java(1 hunks)src/main/java/com/debatetimer/repository/poll/PollRepository.java(1 hunks)src/main/java/com/debatetimer/repository/poll/VoteRepository.java(1 hunks)src/main/java/com/debatetimer/service/poll/VoteService.java(1 hunks)src/main/resources/db/migration/V12__add_participate_code_unique_constraint.sql(1 hunks)src/main/resources/db/migration/V13__add_bell_type_into_bell.sql(1 hunks)src/test/java/com/debatetimer/controller/BaseControllerTest.java(5 hunks)src/test/java/com/debatetimer/controller/BaseDocumentTest.java(2 hunks)src/test/java/com/debatetimer/controller/poll/VoteControllerTest.java(1 hunks)src/test/java/com/debatetimer/controller/poll/VoteDocumentTest.java(1 hunks)src/test/java/com/debatetimer/domainrepository/poll/VoteDomainRepositoryTest.java(3 hunks)src/test/java/com/debatetimer/exception/decoder/H2ErrorDecoderTest.java(1 hunks)src/test/java/com/debatetimer/exception/decoder/MySqlErrorDecoderTest.java(1 hunks)src/test/java/com/debatetimer/fixture/entity/VoteEntityGenerator.java(2 hunks)src/test/java/com/debatetimer/service/poll/VoteServiceTest.java(1 hunks)
✅ Files skipped from review due to trivial changes (11)
- src/main/java/com/debatetimer/entity/BaseTimeEntity.java
- src/main/java/com/debatetimer/entity/poll/PollEntity.java
- src/main/java/com/debatetimer/domain/member/Member.java
- src/test/java/com/debatetimer/controller/BaseDocumentTest.java
- src/main/resources/db/migration/V13__add_bell_type_into_bell.sql
- src/main/resources/db/migration/V12__add_participate_code_unique_constraint.sql
- src/main/java/com/debatetimer/exception/decoder/RepositoryErrorDecoder.java
- src/main/java/com/debatetimer/dto/poll/response/VoteCreateResponse.java
- src/main/java/com/debatetimer/exception/errorcode/ClientErrorCode.java
- src/main/java/com/debatetimer/dto/poll/response/VoterPollInfoResponse.java
- src/main/java/com/debatetimer/config/ErrorDecoderConfig.java
🚧 Files skipped from review as they are similar to previous changes (3)
- src/test/java/com/debatetimer/domainrepository/poll/VoteDomainRepositoryTest.java
- src/test/java/com/debatetimer/controller/BaseControllerTest.java
- src/test/java/com/debatetimer/fixture/entity/VoteEntityGenerator.java
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: leegwichan
PR: debate-timer/debate-timer-be#201
File: src/main/java/com/debatetimer/entity/customize/BellEntity.java:50-54
Timestamp: 2025-07-18T01:04:20.985Z
Learning: BellEntity에서는 도메인 레벨에서 이미 유효성 검사가 수행되므로 엔티티 레벨의 유효성 검사는 중복이며, 향후 엔티티의 유효성 검사를 제거할 예정이다.
src/main/java/com/debatetimer/domain/poll/Vote.java (1)
Learnt from: coli-geonwoo
PR: #203
File: src/main/resources/db/migration/V11__add_memberId_into_poll.sql:1-2
Timestamp: 2025-07-24T03:12:25.245Z
Learning: debate-timer 프로젝트에서 poll 관련 테이블들은 아직 운영 환경에 배포되지 않았으므로, NOT NULL 컬럼 추가 시 기존 데이터 백필 전략을 고려할 필요가 없음.
src/test/java/com/debatetimer/exception/decoder/MySqlErrorDecoderTest.java (1)
Learnt from: coli-geonwoo
PR: #205
File: src/main/java/com/debatetimer/repository/util/RepositoryErrorDecoder.java:6-21
Timestamp: 2025-07-26T02:29:31.651Z
Learning: MySQL에서 unique constraint violation은 SQL state "23000"과 error code 1062로 식별되며, H2/PostgreSQL은 SQL state "23505"만으로 식별된다. MySQL의 "23000"은 일반적인 integrity constraint violation이므로 unique constraint만 감지하려면 error code도 함께 확인해야 한다.
src/main/java/com/debatetimer/exception/decoder/MySqlErrorDecoder.java (1)
Learnt from: coli-geonwoo
PR: #205
File: src/main/java/com/debatetimer/repository/util/RepositoryErrorDecoder.java:6-21
Timestamp: 2025-07-26T02:29:31.651Z
Learning: MySQL에서 unique constraint violation은 SQL state "23000"과 error code 1062로 식별되며, H2/PostgreSQL은 SQL state "23505"만으로 식별된다. MySQL의 "23000"은 일반적인 integrity constraint violation이므로 unique constraint만 감지하려면 error code도 함께 확인해야 한다.
src/main/java/com/debatetimer/exception/decoder/H2ErrorDecoder.java (1)
Learnt from: coli-geonwoo
PR: #205
File: src/main/java/com/debatetimer/repository/util/RepositoryErrorDecoder.java:6-21
Timestamp: 2025-07-26T02:29:31.651Z
Learning: MySQL에서 unique constraint violation은 SQL state "23000"과 error code 1062로 식별되며, H2/PostgreSQL은 SQL state "23505"만으로 식별된다. MySQL의 "23000"은 일반적인 integrity constraint violation이므로 unique constraint만 감지하려면 error code도 함께 확인해야 한다.
src/main/java/com/debatetimer/entity/poll/VoteEntity.java (1)
Learnt from: coli-geonwoo
PR: #203
File: src/main/resources/db/migration/V11__add_memberId_into_poll.sql:1-2
Timestamp: 2025-07-24T03:12:25.245Z
Learning: debate-timer 프로젝트에서 poll 관련 테이블들은 아직 운영 환경에 배포되지 않았으므로, NOT NULL 컬럼 추가 시 기존 데이터 백필 전략을 고려할 필요가 없음.
src/test/java/com/debatetimer/exception/decoder/H2ErrorDecoderTest.java (1)
Learnt from: coli-geonwoo
PR: #205
File: src/main/java/com/debatetimer/repository/util/RepositoryErrorDecoder.java:6-21
Timestamp: 2025-07-26T02:29:31.651Z
Learning: MySQL에서 unique constraint violation은 SQL state "23000"과 error code 1062로 식별되며, H2/PostgreSQL은 SQL state "23505"만으로 식별된다. MySQL의 "23000"은 일반적인 integrity constraint violation이므로 unique constraint만 감지하려면 error code도 함께 확인해야 한다.
src/test/java/com/debatetimer/controller/poll/VoteDocumentTest.java (1)
Learnt from: coli-geonwoo
PR: #203
File: src/main/resources/db/migration/V11__add_memberId_into_poll.sql:1-2
Timestamp: 2025-07-24T03:12:25.245Z
Learning: debate-timer 프로젝트에서 poll 관련 테이블들은 아직 운영 환경에 배포되지 않았으므로, NOT NULL 컬럼 추가 시 기존 데이터 백필 전략을 고려할 필요가 없음.
src/main/java/com/debatetimer/domainrepository/poll/PollDomainRepository.java (1)
Learnt from: coli-geonwoo
PR: #203
File: src/main/resources/db/migration/V11__add_memberId_into_poll.sql:1-2
Timestamp: 2025-07-24T03:12:25.245Z
Learning: debate-timer 프로젝트에서 poll 관련 테이블들은 아직 운영 환경에 배포되지 않았으므로, NOT NULL 컬럼 추가 시 기존 데이터 백필 전략을 고려할 필요가 없음.
🧬 Code Graph Analysis (1)
src/main/java/com/debatetimer/repository/poll/PollRepository.java (1)
src/main/java/com/debatetimer/exception/custom/DTClientErrorException.java (1)
DTClientErrorException(5-10)
⏰ 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-and-push
🔇 Additional comments (31)
src/main/java/com/debatetimer/entity/customize/CustomizeTableEntity.java (2)
6-6: BaseTimeEntity 상속을 위한 적절한 import 추가BaseTimeEntity를 상속하기 위한 import가 올바르게 추가되었습니다.
26-26: BaseTimeEntity 상속으로 공통 audit 필드 관리 확인 완료
- BaseTimeEntity에는
createdAt·modifiedAt만 정의되어 있어, 도메인 로직용usedAt와 역할이 겹치지 않습니다.usedAt는 마지막 사용 시점을 의미하므로, 별도 필드로 유지하는 것이 적절합니다.src/main/java/com/debatetimer/exception/decoder/MySqlErrorDecoder.java (1)
1-27: MySQL 고유 제약 조건 위반 감지 로직이 올바르게 구현되었습니다.검색된 학습 내용에 따르면 MySQL의 고유 제약 조건 위반은 SQL state "23000"과 error code 1062로 식별되며, 이 구현이 정확히 해당 조건을 확인하고 있습니다. 예외 원인 체인을 순회하는 로직과 instanceof 패턴 매칭도 적절합니다.
src/main/java/com/debatetimer/domain/poll/PollStatus.java (1)
9-11: 깔끔하고 명확한 상태 확인 메서드입니다.간단하고 직관적인 구현으로 enum의 유틸리티 메서드로 적절합니다.
src/main/java/com/debatetimer/domain/poll/Poll.java (1)
30-32: 적절한 위임 패턴을 사용한 상태 확인 메서드입니다.Poll 도메인 객체가 상태 enum에 위임하는 깔끔한 구조로, 관심사의 분리가 잘 이루어졌습니다.
src/main/java/com/debatetimer/repository/poll/VoteRepository.java (1)
11-11: Spring Data JPA 명명 규칙을 따른 적절한 존재 확인 메서드입니다.중복 투표 방지를 위한 메서드로, 표준 JPA 레포지토리 패턴을 올바르게 따르고 있습니다.
src/main/java/com/debatetimer/domain/poll/Vote.java (2)
16-18: 새로운 투표 생성을 위한 적절한 편의 생성자입니다.ID 없이 새로운 Vote 도메인 객체를 생성할 수 있는 편의 생성자로, 생성자 체이닝 패턴을 올바르게 사용하고 있습니다.
1-23: AI 요약과 실제 코드 변경사항 간의 불일치가 있습니다.AI 요약에서는 BellType 도입에 대해 언급하고 있지만, 실제 파일들은 투표(Vote) 기능 구현과 관련된 변경사항들입니다. 요약 내용을 확인해 주세요.
Likely an incorrect or invalid review comment.
src/main/java/com/debatetimer/dto/poll/request/VoteRequest.java (1)
1-13: PR 목적과 파일 내용 불일치PR 제목이 "종소리 관련 API V2 제작"인데 실제 파일들은 투표(Poll/Vote) 관련 기능을 구현하고 있습니다. 파일 내용이 PR 목적과 일치하는지 확인이 필요합니다.
그 외에 VoteRequest DTO 구조는 적절하며, validation 어노테이션 사용도 올바릅니다.
Likely an incorrect or invalid review comment.
src/test/java/com/debatetimer/exception/decoder/MySqlErrorDecoderTest.java (2)
41-52: 음수 테스트 케이스가 적절합니다다른 에러 코드와 SQL state를 사용하여 unique constraint violation이 아닌 경우를 제대로 테스트하고 있습니다.
26-39: MySQL 에러 코드 상수 일치 확인 완료테스트 구조와 예외 체인 설정은 적절하며,
MYSQL_UNIQUE_VIOLATION("23000")와MYSQL_DUP_ERROR_CODE(1062)상수값이 학습된 예상값과 일치함을 확인했습니다.
추가 검토나 수정이 필요 없습니다.src/main/java/com/debatetimer/exception/decoder/H2ErrorDecoder.java (1)
8-21: H2 에러 디코더 구현이 올바릅니다이전 학습 내용에 따르면 H2는 SQL state "23505"만으로 unique constraint violation을 식별할 수 있다고 했는데, 구현이 정확히 이를 반영하고 있습니다. 예외 체인을 순회하면서
ConstraintViolationException을 찾고 SQL state를 확인하는 로직도 적절합니다.MySQL과 달리 H2는 error code를 추가로 확인할 필요가 없어 구현이 더 간단한 것도 맞습니다.
src/main/java/com/debatetimer/entity/poll/VoteEntity.java (3)
26-28: 유니크 제약조건 추가가 적절합니다poll_id와 participate_code 조합에 대한 유니크 제약조건 추가로 중복 투표를 데이터베이스 레벨에서 방지할 수 있습니다. 도메인 로직과 데이터베이스 제약조건이 일치하여 데이터 일관성을 보장합니다.
50-51: 필드명 변경이 일관성을 개선합니다
participantCode에서participateCode로 변경하고 컬럼 매핑을 명시적으로 지정한 것이 좋습니다. 도메인 객체와 데이터베이스 스키마 간의 일관성이 향상되었습니다.
53-55: VoteEntity 생성자 ID 매핑 검증 완료
Vote 도메인 객체의public Vote(long pollId,…)생성자가 내부적으로this(null,…)를 호출해 ID를 항상 null로 초기화하므로,vote.getId()가 새 투표 생성 시 null이 되어 JPA의@GeneratedValue전략과 충돌 없이 정상 동작합니다. 해당 로직은 변경할 필요가 없습니다.src/test/java/com/debatetimer/exception/decoder/H2ErrorDecoderTest.java (1)
26-36: 유니크 제약조건 테스트 케이스가 올바르게 구성되었습니다.H2 데이터베이스의 유니크 제약조건 위반 SQL 상태("23505")를 정확히 사용하고 있으며, 예외 체인 구조도 실제 Hibernate/Spring 환경과 일치합니다.
src/main/java/com/debatetimer/controller/poll/VoteController.java (1)
17-37: REST 컨트롤러 구현이 우수합니다.컨트롤러가 REST 규칙을 잘 따르고 있으며, 적절한 HTTP 상태 코드(GET은 200, POST는 201)와 유효성 검사(@Valid)를 사용하고 있습니다. 생성자 주입과 경로 변수 처리도 올바르게 구현되었습니다.
src/main/java/com/debatetimer/domainrepository/poll/VoteDomainRepository.java (2)
42-44: 투표 존재 여부 확인 메서드가 잘 구현되었습니다.
ParticipateCode도메인 객체를 사용하여 값 객체의 이점을 활용하고 있으며, 레포지토리의 새로운 메서드를 적절히 활용하고 있습니다.
46-58: 에러 디코딩을 통한 예외 처리가 우수합니다.데이터베이스별 예외를 도메인 레벨 예외로 변환하는 로직이 잘 구현되었습니다.
RepositoryErrorDecoder를 사용하여 유니크 제약조건 위반을 감지하고 적절한 도메인 예외를 던지는 방식이 효과적입니다.src/main/java/com/debatetimer/service/poll/VoteService.java (2)
26-33: 투표 비즈니스 로직이 체계적으로 구현되었습니다.트랜잭션 관리, 유효성 검사, 도메인 객체 생성 순서가 논리적으로 잘 구성되어 있습니다. 각 단계별 검증을 통해 데이터 무결성을 보장하고 있습니다.
35-47: 유효성 검사 메서드들이 명확하게 분리되었습니다.투표 진행 상태 검증과 중복 투표 검증을 별도 메서드로 분리하여 코드 가독성과 유지보수성을 높였습니다. 도메인별 예외 처리도 적절합니다.
src/test/java/com/debatetimer/controller/poll/VoteControllerTest.java (4)
28-54: 투표 정보 조회 테스트가 포괄적으로 구현되었습니다.응답의 모든 필드를 검증하고 있으며,
assertAll을 사용하여 여러 검증을 한 번에 수행하는 것이 효율적입니다. 테스트 데이터 설정도 명확합니다.
60-81: 성공적인 투표 테스트 케이스가 잘 작성되었습니다.HTTP 상태 코드와 응답 데이터를 모두 검증하고 있으며, 테스트 시나리오가 실제 사용 사례를 잘 반영하고 있습니다.
83-137: 유효성 검사 테스트가 체계적으로 구성되었습니다.
@ParameterizedTest와@NullAndEmptyAndBlankSource를 사용하여 다양한 잘못된 입력값에 대한 테스트를 효율적으로 수행하고 있습니다. 각 필드별 검증이 철저합니다.
139-171: 비즈니스 규칙 테스트가 적절히 구현되었습니다.중복 투표 방지와 종료된 투표에 대한 제한 등 핵심 비즈니스 규칙들이 테스트로 잘 검증되고 있습니다. 실제 운영 환경에서 발생할 수 있는 시나리오들을 다루고 있습니다.
src/test/java/com/debatetimer/service/poll/VoteServiceTest.java (2)
68-79: 동시성 테스트가 잘 구현되었습니다.
runAtSameTime메서드를 사용하여 동시성 이슈를 효과적으로 테스트하고 있으며, 최종적으로 단일 투표만 유효하게 처리되는지 검증하고 있습니다. 실제 운영 환경에서 발생할 수 있는 동시성 문제를 잘 다루고 있습니다.
114-118: 투표 수 검증 로직이 정확합니다.각 팀별 투표 수(찬성 2표, 반대 1표)와 전체 투표 수(3표)를 정확히 검증하고 있으며, 모든 응답 필드에 대한 포괄적인 검증을 수행하고 있습니다.
src/main/java/com/debatetimer/domainrepository/poll/PollDomainRepository.java (2)
25-27: 코드 단순화가 잘 이루어졌습니다.private helper 메서드
findPoll을 제거하고 repository 메서드를 직접 호출하는 방식으로 변경된 것이 좋습니다. 예외 처리를 repository 레이어에 위임함으로써 코드가 더 간결해지고 일관성이 향상되었습니다.
30-33: 새로운 getById 메서드가 적절히 추가되었습니다.member ID 없이 poll을 조회할 수 있는
getById메서드가 투표 기능에 필요한 요구사항을 잘 충족하고 있습니다. 기존getByIdAndMemberId메서드와 일관된 패턴을 따르고 있습니다.src/test/java/com/debatetimer/controller/poll/VoteDocumentTest.java (2)
128-136: 에러 코드 테스트 범위가 적절합니다.투표 관련 주요 에러 케이스들(
ALREADY_DONE_POLL,ALREADY_VOTED_PARTICIPANT,INVALID_POLL_PARTICIPANT_CODE,INVALID_POLL_PARTICIPANT_NAME)을 모두 포함하여 포괄적인 에러 시나리오 테스트를 수행하고 있습니다.
44-51: API 응답 필드 문서화가 상세하고 정확합니다.각 필드의 타입과 설명이 명확하게 작성되어 있으며, 특히
status필드에 대한 enum 값 설명(PROGRESS, DONE)이 개발자가 이해하기 쉽게 작성되었습니다.
| ); | ||
|
|
||
| @Test | ||
| void 투표자_선거_정보_조회() { |
There was a problem hiding this comment.
테스트 메서드 이름이 잘못되었습니다.
Line 105의 메서드 이름이 투표자_선거_정보_조회로 되어 있는데, 이는 실제로 투표를 수행하는 테스트입니다. 메서드 이름과 실제 동작이 일치하지 않습니다.
다음과 같이 메서드 이름을 수정하세요:
- void 투표자_선거_정보_조회() {
+ void 투표자_투표_성공() {📝 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.
| void 투표자_선거_정보_조회() { | |
| void 투표자_투표_성공() { |
🤖 Prompt for AI Agents
In src/test/java/com/debatetimer/controller/poll/VoteDocumentTest.java at line
105, the test method name 투표자_선거_정보_조회 does not match its actual behavior of
performing a vote. Rename the method to reflect that it is testing the voting
action, ensuring the name clearly indicates it is a vote execution test rather
than just querying voter election information.
| Member member = memberGenerator.generate("email@email.com"); | ||
| CustomizeTableEntity table = customizeTableEntityGenerator.generate(member); | ||
| PollEntity pollEntity = pollEntityGenerator.generate(table, PollStatus.PROGRESS); | ||
| voteEntityGenerator.generate(pollEntity, VoteTeam.PROS, "콜리"); |
There was a problem hiding this comment.
테스트 데이터 생성 시 일관성 문제가 있습니다.
Line 40에서 voteEntityGenerator.generate(pollEntity, VoteTeam.PROS, "콜리")로 기존 투표를 생성하고, Line 42에서 동일한 이름으로 새로운 투표 요청을 생성하고 있습니다. 이는 "최초로 투표"라는 테스트 의도와 맞지 않으며, 실제로는 중복 투표 상황을 만들고 있습니다.
다음과 같이 수정하여 실제 최초 투표 상황을 테스트하세요:
- voteEntityGenerator.generate(pollEntity, VoteTeam.PROS, "콜리");📝 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.
| voteEntityGenerator.generate(pollEntity, VoteTeam.PROS, "콜리"); |
🤖 Prompt for AI Agents
In src/test/java/com/debatetimer/service/poll/VoteServiceTest.java at line 40,
the test creates a vote with the same name twice, causing inconsistency with the
intended "first vote" scenario. To fix this, remove or modify the initial vote
creation at line 40 so that the test accurately represents a first-time vote
without duplicates, ensuring the test data setup matches the test's purpose.
3 participants
🚩 연관 이슈
closed #202
🗣️ 리뷰 요구사항 (선택)
Summary by CodeRabbit
신규 기능
버그 수정
문서화
리팩터링
테스트