Conversation
|
""" Walkthrough투표 기능을 위한 투표자 API가 새롭게 도입되었습니다. 주요 변경사항으로는 투표 및 투표 정보 조회를 위한 REST 컨트롤러, 서비스, DTO, 도메인 및 리포지토리 계층의 확장, 관련 예외 및 에러코드 추가, DB 유니크 제약조건 및 컬럼명 변경, 환경별 에러 디코더 설정, 그리고 이에 대한 테스트 코드 및 문서화가 포함됩니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Voter
participant VoteController
participant VoteService
participant PollDomainRepository
participant VoteDomainRepository
participant PollRepository
participant VoteRepository
Voter->>VoteController: POST /api/polls/{pollId}/votes (VoteRequest)
VoteController->>VoteService: vote(pollId, voteRequest)
VoteService->>PollDomainRepository: getById(pollId)
PollDomainRepository->>PollRepository: getById(pollId)
PollRepository-->>PollDomainRepository: PollEntity
VoteService->>VoteDomainRepository: isExists(pollId, participateCode)
VoteDomainRepository->>VoteRepository: existsByPollIdAndParticipateCode(pollId, participateCode)
VoteRepository-->>VoteDomainRepository: Boolean
alt Not exists
VoteService->>VoteDomainRepository: save(Vote)
VoteDomainRepository->>PollRepository: getById(pollId)
PollRepository-->>VoteDomainRepository: PollEntity
VoteDomainRepository->>VoteRepository: save(VoteEntity)
VoteRepository-->>VoteDomainRepository: VoteEntity
VoteDomainRepository-->>VoteService: Vote
VoteService-->>VoteController: VoteCreateResponse
VoteController-->>Voter: 201 Created + VoteCreateResponse
else Already exists or poll ended
VoteService-->>VoteController: Error (400)
VoteController-->>Voter: 400 Bad Request
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes 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. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (3)
🚧 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. 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 Results118 files 118 suites 13s ⏱️ Results for commit 8094397. ♻️ This comment has been updated with latest results. |
📝 Test Coverage Report
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/main/java/com/debatetimer/service/poll/VoteService.java (2)
28-29: 검증 순서 최적화 고려현재 진행 중인 투표인지 먼저 확인하고, 그 다음에 중복 투표를 확인합니다. 성능 관점에서 데이터베이스 조회가 필요한 중복 투표 검증을 먼저 수행하는 것을 고려해볼 수 있습니다. 하지만 현재 구조도 논리적 흐름이 명확하므로 큰 문제는 없습니다.
53-53: 참여 코드 생성 방식 검토UUID를 사용한 참여 코드 생성은 고유성을 보장하지만, 실제 비즈니스 요구사항에 따라 더 사용자 친화적인 형태(예: 짧은 알파벳 코드)를 고려해볼 수 있습니다.
Do you want me to suggest alternative participation code generation strategies that might be more user-friendly?
src/test/java/com/debatetimer/controller/poll/VoteDocumentTest.java (1)
105-105: 테스트 메서드명이 내용과 일치하지 않습니다.메서드명이
투표자_선거_정보_조회로 되어 있지만, 실제로는 투표 생성을 테스트하고 있습니다.메서드명을 다음과 같이 수정하는 것을 제안합니다:
-void 투표자_선거_정보_조회() { +void 투표자_투표_생성() {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
src/main/java/com/debatetimer/controller/poll/VoteController.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/domainrepository/table/CustomizeTableDomainRepository.java(1 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/poll/VoteEntity.java(4 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/repository/util/RepositoryErrorDecoder.java(1 hunks)src/main/java/com/debatetimer/service/poll/PollService.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/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(2 hunks)src/test/java/com/debatetimer/fixture/VoteGenerator.java(1 hunks)src/test/java/com/debatetimer/repository/util/RepositoryErrorDecoderTest.java(1 hunks)src/test/java/com/debatetimer/service/poll/VoteServiceTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
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/main/resources/db/migration/V12__add_participate_code_unique_constraint.sql (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/table/CustomizeTableDomainRepository.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 컬럼 추가 시 기존 데이터 백필 전략을 고려할 필요가 없음.
src/main/java/com/debatetimer/exception/errorcode/ClientErrorCode.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/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/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/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 컬럼 추가 시 기존 데이터 백필 전략을 고려할 필요가 없음.
🧬 Code Graph Analysis (3)
src/main/java/com/debatetimer/repository/poll/PollRepository.java (1)
src/main/java/com/debatetimer/exception/custom/DTClientErrorException.java (1)
DTClientErrorException(5-10)
src/test/java/com/debatetimer/repository/util/RepositoryErrorDecoderTest.java (1)
src/main/java/com/debatetimer/repository/util/RepositoryErrorDecoder.java (1)
RepositoryErrorDecoder(6-21)
src/main/java/com/debatetimer/service/poll/VoteService.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 (37)
src/main/java/com/debatetimer/domainrepository/table/CustomizeTableDomainRepository.java (1)
1-1: 패키지 구조 개선이 적절합니다.CustomizeTable 도메인을 다루는 repository를
poll패키지에서table패키지로 이동한 것은 논리적으로 타당한 리팩토링입니다. 도메인 책임에 따른 패키지 구조가 더 명확해졌습니다.src/main/java/com/debatetimer/service/poll/PollService.java (1)
9-9: Import 경로 업데이트가 올바르게 적용되었습니다.패키지 구조 변경에 따른 import 경로 업데이트가 정확히 반영되었습니다.
src/main/java/com/debatetimer/domain/poll/Poll.java (1)
30-32: 도메인 메서드 추가가 적절합니다.
isProgress()메서드는 투표 상태 확인을 위한 도메인 레벨의 편의 메서드로, enum의 메서드에 위임하는 올바른 설계 패턴을 따르고 있습니다.src/main/java/com/debatetimer/domain/poll/PollStatus.java (1)
9-11: Enum 유틸리티 메서드가 올바르게 구현되었습니다.
isProgress()메서드는 명확하고 간단한 구현으로, 투표 진행 상태 확인을 위한 적절한 유틸리티 메서드입니다.src/main/resources/db/migration/V12__add_participate_code_unique_constraint.sql (2)
4-5: 유니크 제약조건 추가가 적절합니다.
(poll_id, participate_code)조합에 대한 유니크 제약조건은 투표 중복을 데이터베이스 레벨에서 방지하는 효과적인 방법입니다. 투표자 API의 핵심 요구사항을 잘 반영하고 있습니다.
1-2: SQL 마이그레이션 검토 완료
VoteEntity의@Column(name = "participate_code")및@UniqueConstraint(columnNames = {"poll_id", "participate_code"})매핑과 일치하도록 컬럼명을participant_code에서participate_code로 변경하는 것은 적절합니다.- 코드베이스(Java) 내에서 SQL 칼럼명으로
participant_code를 참조하는 부분이 없으며, 모든 매핑이 이미participate_code로 정상 동작함을 확인했습니다.추가 조치 불필요합니다.
src/test/java/com/debatetimer/controller/BaseDocumentTest.java (1)
70-71: LGTM! VoteService mock bean 추가가 적절합니다.새로운 투표 기능 테스트를 위해 VoteService mock bean을 추가한 것이 기존 패턴과 일치하며 테스트 격리에 적합합니다.
src/main/java/com/debatetimer/repository/poll/VoteRepository.java (1)
11-11: LGTM! 중복 투표 확인을 위한 적절한 repository 메서드입니다.Spring Data JPA 네이밍 규칙을 따르며, pollId와 participateCode 조합으로 중복 투표를 효과적으로 검증할 수 있습니다.
src/main/java/com/debatetimer/exception/errorcode/ClientErrorCode.java (1)
53-54: LGTM! 투표 기능을 위한 적절한 에러 코드 추가입니다.
ALREADY_DONE_POLL과ALREADY_VOTED_PARTICIPANT에러 코드가 명확하게 정의되었고, BAD_REQUEST 상태 코드와 한국어 메시지가 적절합니다.src/main/java/com/debatetimer/domain/poll/Vote.java (1)
16-18: LGTM! 편의를 위한 생성자 오버로딩이 잘 구현되었습니다.id 없이 Vote 인스턴스를 생성할 수 있는 편의 생성자가 적절하게 추가되었고, 기존 생성자로의 위임 패턴이 올바릅니다.
src/main/java/com/debatetimer/dto/poll/request/VoteRequest.java (1)
1-13: LGTM! 투표 요청을 위한 DTO가 잘 설계되었습니다.Record 클래스 사용과 Jakarta Bean Validation 어노테이션 적용이 적절하며, 필드명이 명확하고 비즈니스 요구사항을 잘 반영합니다.
src/test/java/com/debatetimer/fixture/VoteGenerator.java (1)
19-26: 메서드 오버로딩을 통한 테스트 유연성 향상이 잘 구현되었습니다.새로운
code매개변수를 받는 오버로드된 메서드 추가로 테스트에서 특정 참여 코드로 투표 엔티티를 생성할 수 있게 되었습니다. 기존 메서드는 UUID를 사용하여 하위 호환성을 유지하고 있어 좋습니다.src/main/java/com/debatetimer/domainrepository/poll/PollDomainRepository.java (1)
24-33: 예외 처리 중앙화를 통한 코드 간소화가 효과적입니다.기존
findPoll헬퍼 메서드를 제거하고PollRepository의 새로운 기본 메서드를 직접 사용하여 예외 처리를 리포지토리 계층으로 중앙화했습니다. 새로운getById메서드 추가로 투표 기능에서 필요한 poll 조회 기능도 지원합니다.src/main/java/com/debatetimer/dto/poll/response/VoteCreateResponse.java (1)
8-18: 투표 생성 응답 DTO가 잘 설계되었습니다.Record 클래스를 사용하여 불변성을 보장하고, 적절한 검증 어노테이션(
@NotBlank,@NotNull)을 적용했습니다.Vote도메인 객체를 받는 편의 생성자를 통해 도메인-DTO 변환이 명확하게 구현되어 있습니다.src/main/java/com/debatetimer/repository/poll/PollRepository.java (1)
13-21: 예외 기반 오류 처리를 위한 기본 메서드 추가가 적절합니다.
Optional을 반환하는 기존 메서드를 래핑하여 엔티티가 없을 때DTClientErrorException을 던지는 기본 메서드들을 추가했습니다. 이로써 호출자는 Optional 처리 없이 명시적인 예외 처리를 할 수 있어 코드가 더 명확해집니다.src/main/java/com/debatetimer/controller/poll/VoteController.java (1)
17-37: REST API 설계가 잘 구현되었습니다컨트롤러 구현이 Spring Boot 모범 사례를 잘 따르고 있습니다:
- RESTful URL 설계 (
/api/polls/{pollId}/votes)- 적절한 HTTP 메서드와 상태 코드 사용
- 입력 검증 (
@Valid) 적용- 서비스 계층으로의 깔끔한 위임
src/test/java/com/debatetimer/repository/util/RepositoryErrorDecoderTest.java (3)
16-27: 유니크 제약조건 테스트가 정확하게 구현되었습니다PostgreSQL의 유니크 제약조건 위반 SQL 상태 코드("23505")를 정확히 사용하고, 실제 데이터베이스 예외 체인을 올바르게 모방하고 있습니다. 테스트 구조도 명확합니다.
29-40: 네거티브 테스트 케이스가 적절합니다다른 SQL 에러("23000")에 대해 false를 반환하는지 확인하는 테스트가 잘 작성되어 있습니다. 이는 유니크 제약조건이 아닌 다른 데이터베이스 에러와의 구분을 보장합니다.
18-20: 제약조건 이름이 의미있게 작성되었습니다테스트에서 사용된 제약조건 이름
"vote_poll_id_participate_code"이 실제 VoteEntity의 유니크 제약조건과 일치하여 테스트의 현실성을 높입니다.src/main/java/com/debatetimer/entity/poll/VoteEntity.java (3)
26-28: 유니크 제약조건이 적절히 구현되었습니다
(poll_id, participate_code)조합에 대한 유니크 제약조건이 데이터베이스 레벨에서 중복 투표를 방지하는 강력한 보장을 제공합니다. 이는 애플리케이션 레벨 검증과 함께 이중 보안을 제공합니다.
50-51: 필드명 변경과 컬럼 매핑이 올바르게 처리되었습니다
participantCode에서participateCode로 필드명을 변경하면서@Column(name = "participate_code")를 통해 데이터베이스 컬럼과의 매핑을 명시적으로 관리한 것이 좋습니다.
53-55: 도메인 중심 생성자가 잘 구현되었습니다도메인 객체
Vote와PollEntity를 받는 생성자를 통해 도메인 주도 설계 원칙을 잘 따르고 있습니다. 도메인 객체의 값 객체들(vote.getName().getValue(),vote.getCode().getValue())을 적절히 추출하고 있습니다.src/main/java/com/debatetimer/service/poll/VoteService.java (2)
27-33: 투표 로직이 잘 구현되었습니다트랜잭션 경계가 적절하고, 검증 로직이 명확히 분리되어 있습니다. 도메인 객체를 사용한 비즈니스 로직 처리도 깔끔합니다.
42-47: 중복 투표 검증 로직이 견고합니다도메인 값 객체
ParticipateCode를 사용하여 타입 안전성을 보장하고, 적절한 예외 처리를 통해 중복 투표를 방지합니다. 리포지토리와 서비스 계층 모두에서 이중 검증하는 것도 좋습니다.src/main/java/com/debatetimer/dto/poll/response/VoterPollInfoResponse.java (2)
8-17: 투표자용 응답 DTO가 잘 설계되었습니다Java record를 사용하여 불변 데이터 전송 객체를 깔끔하게 구현했습니다. 투표자에게 필요한 모든 정보(투표 상태, 팀명, 투표 수, 참여 코드)를 포함하고 있어 완전합니다.
19-30: 도메인 객체 생성자가 우아하게 구현되었습니다도메인 객체(
Poll,VoteInfo,ParticipateCode)에서 DTO로의 매핑이 명확하고 깔끔합니다. 값 객체의 실제 값을 적절히 추출하여(getValue()) 프레젠테이션 계층에 맞는 형태로 변환했습니다.src/main/java/com/debatetimer/domainrepository/poll/VoteDomainRepository.java (3)
25-25: 새로운 의존성 주입이 적절합니다.투표 저장을 위해 PollRepository가 필요한 상황이므로 의존성 추가가 타당합니다.
41-43: 중복 투표 확인 로직이 올바르게 구현되었습니다.ParticipateCode를 사용하여 중복 투표를 확인하는 로직이 명확하고 간결합니다.
45-57: 예외 처리 로직을 개선할 수 있습니다.vote 메서드의 구현은 전반적으로 양호하나, 예외 처리 부분에서 개선 가능한 점들이 있습니다:
pollRepository.getById()에서 발생할 수 있는 예외가 적절히 처리되지 않음- DataIntegrityViolationException 외의 다른 데이터베이스 예외도 발생할 수 있음
다음과 같이 개선을 제안합니다:
public Vote vote(Vote vote) { try { PollEntity pollEntity = pollRepository.getById(vote.getPollId()); VoteEntity voteEntity = new VoteEntity(vote, pollEntity); return voteRepository.save(voteEntity) .toDomain(); - } catch (DataIntegrityViolationException exception) { + } catch (DataIntegrityViolationException exception) { if (RepositoryErrorDecoder.isUniqueConstraintViolation(exception)) { throw new DTClientErrorException(ClientErrorCode.ALREADY_VOTED_PARTICIPANT); } throw exception; + } catch (Exception exception) { + // 기타 예외에 대한 로깅 또는 처리 로직 추가 고려 + throw exception; } }⛔ Skipped due to learnings
Learnt from: coli-geonwoo PR: debate-timer/debate-timer-be#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/domainrepository/poll/VoteDomainRepositoryTest.java (2)
51-72: 중복 투표 확인 테스트가 잘 구현되었습니다.두 개의 서로 다른 poll을 생성하여 동일한 참여 코드로 참여 여부를 정확히 검증하는 테스트입니다. 테스트 시나리오와 assertion이 명확합니다.
74-106: 투표 기능 테스트가 포괄적으로 작성되었습니다.정상적인 투표 저장과 중복 투표 방지 두 가지 핵심 시나리오를 모두 커버하고 있으며, 예외 메시지까지 정확히 검증하고 있습니다.
src/test/java/com/debatetimer/service/poll/VoteServiceTest.java (2)
32-93: 투표 기능 테스트가 매우 포괄적입니다.서비스 레이어에서 발생할 수 있는 다양한 시나리오를 모두 커버하고 있습니다:
- 정상적인 첫 투표
- 중복 투표 방지
- 동시성 이슈 처리
- 종료된 poll에 대한 투표 방지
특히 동시성 테스트(lines 67-79)가 포함되어 있어 실제 운영 환경에서 발생할 수 있는 문제를 사전에 검증할 수 있습니다.
95-120: 투표자 정보 조회 테스트가 상세하게 작성되었습니다.응답 객체의 모든 필드를 검증하고 있으며, 특히 participateCode가 비어있지 않은지 확인하는 부분이 좋습니다.
src/test/java/com/debatetimer/controller/poll/VoteControllerTest.java (2)
23-53: 투표자 정보 조회 API 테스트가 잘 구현되었습니다.REST API 엔드포인트를 정확히 테스트하고 있으며, 응답 데이터의 모든 필드를 검증합니다.
55-114: 투표 API 테스트가 포괄적입니다.정상적인 투표, 중복 투표 방지, 종료된 poll 투표 방지 등 모든 주요 시나리오를 커버하고 있으며, HTTP 상태 코드도 적절히 검증합니다.
src/test/java/com/debatetimer/controller/poll/VoteDocumentTest.java (2)
32-79: 투표자 정보 조회 API 문서화가 상세합니다.API 문서화에 필요한 모든 요소가 잘 정의되어 있습니다:
- 요청 파라미터 설명
- 응답 필드별 상세 설명
- 한국어 설명이 명확함
81-155: 투표 API 문서화가 매우 포괄적입니다.정상 케이스뿐만 아니라 다양한 오류 시나리오에 대한 문서화도 포함되어 있어, API 사용자가 모든 경우를 이해할 수 있습니다. 파라미터화된 테스트를 통해 여러 오류 코드를 효율적으로 문서화한 것이 좋습니다.
src/main/java/com/debatetimer/controller/poll/VoteController.java
Outdated
Show resolved
Hide resolved
src/main/java/com/debatetimer/repository/util/RepositoryErrorDecoder.java
Outdated
Show resolved
Hide resolved
unifolio0
left a comment
There was a problem hiding this comment.
/noti
@coli-geonwoo
리뷰 조금 남겼어요
There was a problem hiding this comment.
제가 힘들게 만들었던 NotBlank에 대한 테스트를 안 만든 이유가 있나요?
There was a problem hiding this comment.
비토가 힘들게 만들었던 NotBlank에 대한 테스트를 안 만든 이유가 있나요?
| class VoteTest { | ||
|
|
||
| @Test | ||
| void 투표할_수_있다() { |
There was a problem hiding this comment.
테스트 이름이 거슬리긴 한데... 왤까요?
다른 성공케이스 테스트처럼 기능에 대한 설명으로 수정하는 건 어떤가요?
There was a problem hiding this comment.
투표를_저장할_수_있다로 변경했습니다~
|
|
||
| @Test | ||
| void 유니크_제약조건_에러를_판단할_수_있다() { | ||
| SQLException uniqueError = new SQLException("유니크 에러", "23505"); |
There was a problem hiding this comment.
이렇게 하드코딩할꺼면 RepositoryErrorDecoder의 상수를 protected로 하는건 어떨까 싶어요.
| void 끝난_선거에_투표_할_수_없다() { | ||
| Member member = memberGenerator.generate("email@email.com"); | ||
| CustomizeTableEntity table = customizeTableGenerator.generate(member); | ||
| PollEntity alreadyDonePoll = pollGenerator.generate(table, PollStatus.DONE); |
There was a problem hiding this comment.
이런 느낌이라면 전 pollGenerator에다가 완료한 poll과 진행 중인 poll을 만드는 메소드를 각각 생성할 것 같아요
There was a problem hiding this comment.
아마 휴먼 에러 가능성때문에 안정적인 코드 작성 관점에서는 비토가 말한 부분이 좋은 것 같은데, 저는 fixtureGenerator를 이전 프로젝트에서 비토가 제안한대로 사용한 입장에서 망설여지는 부분이 있어요.
조건이 하나 변할 때마다 generate 함수가 무한대로 생성됩니다. 예를 들어 지금은 상태에 대해 generateDonePoll인데, 만약 찬성측 팀이름에 따른 규칙이 추가되어 이를 선거에 적용하려면 generateByProsTeamName, 등등의 메서드가 계속 생겨났어요.
그래서 1차적으로 generator를 도입하자고 했을 때 변경가능한 것들은 파라미터로 열어두자는 입장이었어요.
src/main/java/com/debatetimer/controller/poll/VoteController.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
아하... 계층화하면서 static 제거했습니다.
| @@ -1,4 +1,4 @@ | |||
| package com.debatetimer.domainrepository.poll; | |||
| package com.debatetimer.domainrepository.table; | |||
| public record VoteCreateResponse( | ||
| long id, | ||
| @NotBlank String name, | ||
| @NotBlank String participateCode, | ||
| @NotNull VoteTeam team | ||
| ) { |
There was a problem hiding this comment.
다른 response와 달리 여기서 검증해주는 이유는 뭔가요? 그보다 동작은 하나요?
| public boolean alreadyVoted(long pollId, ParticipateCode code) { | ||
| return voteRepository.existsByPollIdAndParticipateCode(pollId, code.getValue()); | ||
| } |
There was a problem hiding this comment.
개인의 생각 차이일 수 있겠지만 저번 회의때 domainRepository는 기존 Repository의 역할을 하는데, 도메인 객체를 서로 주고 받도록 한다.라고 정했던 것 같아요. 그런데 지금 보면 메소드 명은 이미 투표했는지 -> 구현은 pollId와 code를 가진 vote가 이미 존재하는지로 이 구현이 어떤 서비스 규칙인지 알고 있는듯 보이네요. 이는 domain repository의 역할을 넘어선 정보라고 생각됩니다.
There was a problem hiding this comment.
- 비토 말에 일부 동의합니다.
- 아마 메서드의 이름을 줄이고 싶었을 것이라고 추측되는데, 그렇다면
isExist(long pollId, ParticipateCode code)정도면 괜찮지 않을까요?
There was a problem hiding this comment.
✅ 반영 완료
회의 내용에 따라서
alreadyVoted > isExists
vote > save
로 변경했습니다.
leegwichan
left a comment
There was a problem hiding this comment.
/noti @coli-geonwoo
리뷰 누락했네요;; 죄송합니다;; 다음부터는 리뷰 빨리 안하면 리마인드 주십쇼.
언급할 점이 많아서 코멘트 많이 남겼습니다. 천천히 해결해나가 보시죠.
| public boolean alreadyVoted(long pollId, ParticipateCode code) { | ||
| return voteRepository.existsByPollIdAndParticipateCode(pollId, code.getValue()); | ||
| } |
There was a problem hiding this comment.
- 비토 말에 일부 동의합니다.
- 아마 메서드의 이름을 줄이고 싶었을 것이라고 추측되는데, 그렇다면
isExist(long pollId, ParticipateCode code)정도면 괜찮지 않을까요?
| public Vote vote(Vote vote) { | ||
| try { | ||
| PollEntity pollEntity = pollRepository.getById(vote.getPollId()); | ||
| VoteEntity voteEntity = new VoteEntity(vote, pollEntity); | ||
| return voteRepository.save(voteEntity) | ||
| .toDomain(); | ||
| } catch (DataIntegrityViolationException exception) { | ||
| if (RepositoryErrorDecoder.isUniqueConstraintViolation(exception)) { | ||
| throw new DTClientErrorException(ClientErrorCode.ALREADY_VOTED_PARTICIPANT); | ||
| } | ||
| throw exception; | ||
| } | ||
| } |
There was a problem hiding this comment.
https://github.com/debate-timer/debate-timer-be/pull/205/files#r2232402210
여기에서 말한 비토의 컨텍스트와 마찬가지라고 생각합니다.
vote를 동사로 쓴 것으로 추측되는데, 차라리 save()가 더 좋다고 생각합니다.
| public record VoteCreateResponse( | ||
| long id, | ||
| @NotBlank String name, | ||
| @NotBlank String participateCode, | ||
| @NotNull VoteTeam team | ||
| ) { |
| package com.debatetimer.repository.util; | ||
|
|
||
| import org.hibernate.exception.ConstraintViolationException; | ||
| import org.springframework.dao.DataIntegrityViolationException; | ||
|
|
||
| public class RepositoryErrorDecoder { | ||
|
|
||
| private static final String UNIQUE_CONSTRAINT_VIOLATION_SQL_STATE = "23505"; |
There was a problem hiding this comment.
꼭 굳이 util 패키지여야 합니까? exception 패키지에 있어도 충분하다고 생각합니다.
There was a problem hiding this comment.
exception.decoder 로 옮겼습니다.
| @Transactional | ||
| public VoteCreateResponse vote(long pollId, VoteRequest voteRequest) { | ||
| validateProgressPoll(pollId); | ||
| validateAlreadyVoted(pollId, voteRequest.participateCode()); | ||
| Vote vote = new Vote(pollId, voteRequest.team(), voteRequest.name(), voteRequest.participateCode()); | ||
| Vote savedVote = voteDomainRepository.vote(vote); | ||
| return new VoteCreateResponse(savedVote); | ||
| } |
There was a problem hiding this comment.
한 번 투표했는지 검증하는데, RepositoryErrorDecoder 까지 이용해야 하나요? 차라리 아래 방안이 더 좋을 것 같은데요?
- 1안.
validateAlreadyVoted()를 하지 말고,RepositoryErrorDecoder를 더 발전시킨다. - 2안.
validateAlreadyVoted()할 때 X-Lock 까지 가져온다.
There was a problem hiding this comment.
음.. 목적이 다릅니다.
서비스 로직에서 중복 투표를 허용하는가? 허용하지 않는가? 를 읽을 수 있어야 된다고 생각해요. 그래서 exists를 통해 검증하는 걸 선호합니다.
같은 관점에서 일전에 있었던 테이블의 소유권 검증을getByIdAndMemberId로 하지 않고 exsits로 검증하려 했던 겁니다.
RepositoryErrorDecoder는 동시성 문제에 대해 유니크 제약조건 오류가 터졌을 때 이를 커스텀 에러로 전환하여 500이 아닌 400으로 전환하고 서비스 단에서 영속성 계층의 특수 에러로부터 관심을 기울이지 않게 하기 위함이에요.
반대로 Domain Repository에 에러 전환 코드가 있으니까, Service에서는 빼도 된다 혹은 X락은 가져온다는 관점은 영속성 계층의 구현을 너무 잘 알고 있는 관점에서 서비스 코드를 작성하는 것이라 생각합니다.
그럼 현업에서는 어떻게 하는가?
이것과 관련하여 마켓컬리와 카카오페이에서 어떻게 하는지 여쭈어 봤었는데 service 코드에서 exists로 1차 검증하고 따닥 문제 방지를 위해 2차적으로 unique 제약조건을 활용한 검증을 해둔다고 하더라고요.
종합적으로 RepositoryErrorDecoder를 interface로 말아올리고 다양한 벤더에 대응가능하도록 변경하는 게 좋다고 생각합니다.
There was a problem hiding this comment.
비토가 힘들게 만들었던 NotBlank에 대한 테스트를 안 만든 이유가 있나요?
| public static boolean isUniqueConstraintViolation(DataIntegrityViolationException e) { | ||
| Throwable cause = e.getCause(); | ||
| while (cause != null) { | ||
| if (cause instanceof ConstraintViolationException cve) { | ||
| String sqlState = cve.getSQLException().getSQLState(); | ||
| return UNIQUE_CONSTRAINT_VIOLATION_SQL_STATE.equals(sqlState); | ||
| } | ||
| cause = cause.getCause(); | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
토끼가 한 번 언급했지만, H2일 때와 MySQL일 때 모두를 고려할 수 있는 방안이 필요할 것 같아요.
객체를 따로 만들어서 각 환경에서 Bean을 갈아끼워주는 방법이 있을 수도 있구요. 그 외에 다른 방법도 있겠죠.
조금 더 고민해서 만들어보시죠. 회의 때도 이야기 해보구요.
There was a problem hiding this comment.
interface화 시켜서 local, test 일 때와 prod, dev일 때 구현체를 각각 다르게 분리하였습니다.
There was a problem hiding this comment.
[선택/반영안해도 됨]
조금 더 나아간다면, MySQL DB가 연결되었을 때와 H2 DB 연결되었을 때 를 분기점으로 다르게 빈을 등록하는 방법도 있을 것 같아요! 이미 충분하다고 생각해서 넘어가셔도 무방합니다! 👍
There was a problem hiding this comment.
추후 반영해볼게요! DriverClass 가져와서 할수 있을 것 같아요.
|
/noti @debate-timer/backend 리뷰 한번 반영했으니 검토부탁드립니다~ |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/test/java/com/debatetimer/controller/BaseControllerTest.java (1)
96-101: 테스트 데이터 빌더가 적절히 구현되었습니다.
getVoteRequestBuilder()메서드가 기존 패턴에 맞게 잘 구현되었습니다. 다만 테스트 커버리지 향상을 위해 고정값 사용을 고려해보세요.현재 구현:
name이 "콜리"로 고정team이 항상VoteTeam.PROS로 설정특정 테스트에서 다양한 시나리오를 검증해야 할 경우, 해당 테스트에서 이 값들을 오버라이드할 수 있으므로 현재 구현도 적절합니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
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/domainrepository/poll/VoteDomainRepository.java(2 hunks)src/main/java/com/debatetimer/dto/poll/response/VoteCreateResponse.java(1 hunks)src/main/java/com/debatetimer/entity/customize/CustomizeTableEntity.java(1 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/service/poll/VoteService.java(1 hunks)src/test/java/com/debatetimer/controller/BaseControllerTest.java(3 hunks)src/test/java/com/debatetimer/controller/poll/VoteControllerTest.java(1 hunks)src/test/java/com/debatetimer/domainrepository/poll/VoteDomainRepositoryTest.java(2 hunks)src/test/java/com/debatetimer/exception/decoder/H2ErrorDecoderTest.java(1 hunks)src/test/java/com/debatetimer/exception/decoder/MySqlErrorDecoderTest.java(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- src/main/java/com/debatetimer/entity/customize/CustomizeTableEntity.java
- src/main/java/com/debatetimer/controller/poll/VoteController.java
- src/main/java/com/debatetimer/config/ErrorDecoderConfig.java
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/debatetimer/dto/poll/response/VoteCreateResponse.java
- src/main/java/com/debatetimer/service/poll/VoteService.java
- src/test/java/com/debatetimer/controller/poll/VoteControllerTest.java
🧰 Additional context used
🧠 Learnings (4)
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/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/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/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도 함께 확인해야 한다.
🧬 Code Graph Analysis (1)
src/test/java/com/debatetimer/exception/decoder/MySqlErrorDecoderTest.java (2)
src/test/java/com/debatetimer/exception/decoder/H2ErrorDecoderTest.java (1)
Nested(22-50)src/main/java/com/debatetimer/exception/decoder/MySqlErrorDecoder.java (1)
MySqlErrorDecoder(7-27)
⏰ 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 (12)
src/test/java/com/debatetimer/controller/BaseControllerTest.java (1)
7-7: 새로운 투표 기능 관련 import가 적절히 추가되었습니다.투표 API 구현을 위해 필요한
VoteTeam,VoteRequest,UUIDimport가 올바르게 추가되었습니다.Also applies to: 12-12, 29-29
src/main/java/com/debatetimer/exception/decoder/RepositoryErrorDecoder.java (1)
5-8: 인터페이스 설계가 잘 되어 있습니다.데이터베이스별 유니크 제약조건 위반 감지 로직을 추상화한 깔끔한 인터페이스입니다. 단일 책임 원칙을 잘 따르고 있으며, 메서드명과 파라미터 타입이 적절합니다.
src/test/java/com/debatetimer/exception/decoder/H2ErrorDecoderTest.java (1)
12-51: 테스트 커버리지와 구조가 훌륭합니다.H2 데이터베이스의 유니크 제약조건 위반 감지 로직을 올바르게 테스트하고 있습니다. 긍정적/부정적 케이스를 모두 다루고 있으며, H2의 SQL state "23505"를 정확히 사용하고 있습니다.
src/main/java/com/debatetimer/exception/decoder/H2ErrorDecoder.java (1)
6-22: H2 데이터베이스 에러 디코더 구현이 정확합니다.H2의 유니크 제약조건 위반을 나타내는 SQL state "23505"를 올바르게 사용하고 있으며, 예외 체인을 순회하여 원본 ConstraintViolationException을 찾는 로직이 적절합니다. 최신 Java의 패턴 매칭 문법도 잘 활용되었습니다.
src/main/java/com/debatetimer/exception/decoder/MySqlErrorDecoder.java (1)
7-27: MySQL 특화 에러 디코더 구현이 정확합니다.MySQL의 유니크 제약조건 위반을 올바르게 감지하기 위해 SQL state "23000"과 error code 1062를 모두 확인하고 있습니다. MySQL에서 "23000"은 일반적인 무결성 제약조건 위반이므로 error code를 함께 검사하는 것이 필수적입니다.
src/test/java/com/debatetimer/exception/decoder/MySqlErrorDecoderTest.java (1)
12-54: MySQL 에러 디코더 테스트가 잘 구현되었습니다.MySQL 특화 유니크 제약조건 위반 감지 로직을 올바르게 테스트하고 있습니다. 긍정적 케이스에서는 MySQL의 정확한 SQL state와 error code를 사용하고, 부정적 케이스에서는 다른 값들을 사용하여 적절한 테스트 커버리지를 제공합니다.
src/test/java/com/debatetimer/domainrepository/poll/VoteDomainRepositoryTest.java (3)
4-4: 새로운 기능 테스트를 위한 import 추가가 적절합니다.투표 존재 여부 확인과 중복 투표 방지 기능을 테스트하기 위해 필요한 import들이 올바르게 추가되었습니다.
Also applies to: 8-8, 10-10, 16-18
51-72: 투표 존재 여부 확인 테스트가 잘 구현되었습니다.양성/음성 케이스를 모두 포함하여
isExists메서드의 동작을 올바르게 검증하고 있습니다. 기존 테스트 명명 규칙을 따라 한국어로 작성된 것도 일관성 있습니다.
74-106: 투표 저장 기능 테스트가 완전하게 구현되었습니다.성공 케이스와 중복 투표 방지 케이스를 모두 포함하여
save메서드를 철저히 검증하고 있습니다. 특히 중복 투표 시 올바른 예외가 발생하는지 확인하는 부분이 잘 작성되었습니다.src/main/java/com/debatetimer/domainrepository/poll/VoteDomainRepository.java (3)
3-4: 새로운 기능을 위한 의존성 추가가 적절합니다.투표 저장과 에러 처리를 위해 필요한 PollRepository와 RepositoryErrorDecoder 의존성이 올바르게 추가되었습니다.
Also applies to: 7-7, 9-12, 18-18, 25-25, 27-27
42-44: 이전 리뷰 피드백을 반영한 메서드명 개선이 좋습니다.
isExists메서드명이 이전 리뷰에서 제안된 대로 도메인 저장소의 역할에 더 적합하게 변경되었습니다. 구현도 간결하고 명확합니다.
46-58: 투표 저장 메서드가 견고하게 구현되었습니다.이전 리뷰 피드백을 반영하여
save메서드명을 사용했고, 유니크 제약조건 위반에 대한 예외 처리가 적절히 구현되어 있습니다. RepositoryErrorDecoder를 통한 에러 분류와 적절한 클라이언트 에러 코드 반환이 잘 되어 있습니다.
leegwichan
left a comment
There was a problem hiding this comment.
/noti @coli-geonwoo
고생했어요! Approve 했습니다!
| public Vote save(Vote vote) { | ||
| try { | ||
| PollEntity pollEntity = pollRepository.getById(vote.getPollId()); | ||
| VoteEntity voteEntity = new VoteEntity(vote, pollEntity); | ||
| return voteRepository.save(voteEntity) | ||
| .toDomain(); | ||
| } catch (DataIntegrityViolationException exception) { | ||
| if (errorDecoder.isUniqueConstraintViolation(exception)) { | ||
| throw new DTClientErrorException(ClientErrorCode.ALREADY_VOTED_PARTICIPANT); | ||
| } | ||
| throw exception; | ||
| } | ||
| } |
| public static boolean isUniqueConstraintViolation(DataIntegrityViolationException e) { | ||
| Throwable cause = e.getCause(); | ||
| while (cause != null) { | ||
| if (cause instanceof ConstraintViolationException cve) { | ||
| String sqlState = cve.getSQLException().getSQLState(); | ||
| return UNIQUE_CONSTRAINT_VIOLATION_SQL_STATE.equals(sqlState); | ||
| } | ||
| cause = cause.getCause(); | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
[선택/반영안해도 됨]
조금 더 나아간다면, MySQL DB가 연결되었을 때와 H2 DB 연결되었을 때 를 분기점으로 다르게 빈을 등록하는 방법도 있을 것 같아요! 이미 충분하다고 생각해서 넘어가셔도 무방합니다! 👍
| @Table(name = "customize_table") | ||
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| public class CustomizeTableEntity { | ||
| public class CustomizeTableEntity extends BaseTimeEntity { |
There was a problem hiding this comment.
BaseTimeEntity가 PollEntity에서도 쓰이던데 패키지 이동해주세요.
|
/noti 비토 리뷰 반영했고 generator 때문에 제꺼 먼저 머지할게요. 참고로 비토 flyway V12로 되어 있는데 제 PR이 먼저 머지되면서 V13으로 수정될 필요가 있어보입니다. |
🚩 연관 이슈
closed #204
🗣️ 리뷰 요구사항 (선택)
Summary by CodeRabbit
신규 기능
버그 수정
데이터베이스
문서화 및 테스트