Feature/#9 10 대회 투표 현황 조회 api 개발#74
Conversation
There was a problem hiding this comment.
Pull request overview
관리자 페이지에서 대회 투표 현황(랭킹/집계) 을 조회할 수 있도록 API/쿼리/응답 DTO 및 문서를 추가한 PR입니다.
Changes:
- 대회 투표 랭킹 조회 API (
GET /contests/{contestId}/ranking) 및 Dense Ranking 적용 - 대회 투표 집계 조회 API (
GET /contests/{contestId}/votes/statistics) 추가 (총 투표 수/투표자 수/평균) - 관련 DTO/테스트/REST Docs/Asciidoc 문서 보강
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/com/opus/opus/modules/team/application/TeamQueryService.java | 랭킹/집계 조회 서비스 로직 및 dense ranking 적용 추가 |
| src/main/java/com/opus/opus/modules/team/domain/dao/TeamRepository.java | 대회별 팀 투표수 집계 쿼리(LEFT JOIN) 추가 |
| src/main/java/com/opus/opus/modules/team/domain/dao/TeamVoteRepository.java | 대회별 총 투표수/투표자수 집계 쿼리 추가 |
| src/main/java/com/opus/opus/modules/contest/api/ContestController.java | 관리자용 랭킹/집계 조회 엔드포인트 추가 |
| src/main/java/com/opus/opus/modules/contest/application/dto/response/ContestRankingResponse.java | 랭킹 응답 DTO 신규 추가 |
| src/main/java/com/opus/opus/modules/contest/application/dto/response/ContestVoteStatisticsResponse.java | 집계 응답 DTO 신규 추가 |
| src/test/java/com/opus/opus/team/application/TeamQueryServiceTest.java | 랭킹/집계 통합 테스트 케이스 추가 |
| src/test/java/com/opus/opus/restdocs/docs/ContestApiDocsTest.java | 랭킹/집계 API REST Docs 스니펫 생성 테스트 추가 |
| src/main/java/com/opus/opus/docs/asciidoc/contest.adoc | 랭킹/집계 API 문서 섹션 추가 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @Query("SELECT COUNT(vote) " + | ||
| "FROM TeamVote vote " + | ||
| "JOIN vote.team team " + | ||
| "WHERE team.contestId = :contestId AND vote.isVoted = true") | ||
| long countTotalVotesByContest(Long contestId); |
There was a problem hiding this comment.
집계 쿼리가 isVoted=true인 모든 레코드를 카운트하고 있어, (대회 투표기간이 변경될 수 있는 구조라면) 현재 설정된 투표기간 밖에서 생성된 투표까지 포함될 수 있습니다. 이 API가 '투표 기간에 행해진 투표만 집계' 요구사항을 만족해야 한다면 vote.createdAt을 contest.voteStartAt~voteEndAt 범위로 제한하도록 Contest 조인/기간 조건을 추가하는 방향을 검토해주세요.
There was a problem hiding this comment.
투표 생성 시점에 이미 기간 검증을 하니 집계 시 기간 필터는 불필요하다고 느꼈는데 (쿼리가 불필요하게 복잡해지므로), 방어적으로 기간 조건을 추가해야할지가 고민이네요...
There was a problem hiding this comment.
그런데 투표 기간이 아니라면 투표가 불가능 할 텐데(UI적으로 불가로 알고 있어요), 이 부분을 고려해야 하나 생각이 드네요
집계 쿼리에 내용이 추가되면 그 만큼 성능적인 부분에서 문제가 있을테니까요
src/main/java/com/opus/opus/modules/team/domain/dao/TeamRepository.java
Outdated
Show resolved
Hide resolved
src/main/java/com/opus/opus/modules/team/domain/dao/TeamVoteRepository.java
Outdated
Show resolved
Hide resolved
src/test/java/com/opus/opus/team/application/TeamQueryServiceTest.java
Outdated
Show resolved
Hide resolved
| @Query("SELECT new com.opus.opus.modules.contest.application.dto.response.ContestRankingResponse(" + | ||
| "team.id, team.teamName, team.projectName, track.trackName, COUNT(vote.id)) " + | ||
| "FROM Team team " + | ||
| "LEFT JOIN TeamVote vote ON vote.team = team AND vote.isVoted = true " + | ||
| "LEFT JOIN ContestTrack track ON team.trackId = track.id " + |
There was a problem hiding this comment.
Repository 레이어(dao)가 ...application.dto.response.ContestRankingResponse(컨트롤러 응답 DTO)에 직접 의존하고 있어 계층/모듈 결합도가 커집니다. 이 쿼리 전용 Projection 인터페이스/DTO를 dao 패키지에 두거나, repository는 필요한 원시 값만 반환하고 service에서 응답 DTO로 매핑하는 방식으로 분리하는 것이 유지보수에 더 안전합니다.
There was a problem hiding this comment.
dto가 상위 계층인 application에 의존하고 있어서 문제가 될 수 있을 거 같습니다!
Projection을 dao 패키지에 둔다면 계층 분리 측면에서 이득일 수 있겠네요!
Repository(dao 계층) → TeamRankingProjection(dao 계층) ← Service(application 계층)
1. 기존 방식 (Repository에서 응답 DTO 직접 의존)
- TeamRepository.java
@Query("SELECT new com.opus.opus.modules.contest.application.dto.response.ContestRankingResponse(" +
"team.id, team.teamName, team.projectName, track.trackName, COUNT(vote.id)) " +
"FROM Team team ...")
List<ContestRankingResponse> findTeamRankingByContestId(Long contestId);2. 변경 후 방식 (Projection 인터페이스로 계층 분리)
- TeamRankingProjection.java (dao 패키지 내에 둠)
public interface TeamRankingProjection {
Long getTeamId();
String getTeamName();
String getProjectName();
String getTrackName();
Long getVoteCount();
}- TeamRepository.java
@Query("SELECT team.id AS teamId, team.teamName AS teamName, " +
"team.projectName AS projectName, track.trackName AS trackName, " +
"COUNT(vote.id) AS voteCount " +
"FROM Team team " +
"LEFT JOIN TeamVote vote ON vote.team = team AND vote.isVoted = true " +
"LEFT JOIN ContestTrack track ON team.trackId = track.id " +
"WHERE team.contestId = :contestId " +
"GROUP BY team.id, team.teamName, team.projectName, track.trackName " +
"ORDER BY COUNT(vote.id) DESC")
List<TeamRankingProjection> findTeamRankingByContestId(Long contestId);- TeamQueryService.java
public List<ContestRankingResponse> getTeamRanking(Long contestId) {
contestConvenience.getValidateExistContest(contestId);
List<TeamRankingProjection> votesPerTeam = teamRepository.findTeamRankingByContestId(contestId);
return applyDenseRanking(votesPerTeam);
}- 위 방식에 대해 다른 분들의 의견이 궁금합니다!
There was a problem hiding this comment.
말씀하신대로 dto매핑을 사용하면 상위 패키지를 의존하는것 같습니다.
- projection을 사용거나
- queryservice에서는 상위 패키지 의존을 허용하거나
- 레포지토리용 dto를 새로 정의하고 ContestRankingResponse로 매핑하는 방법이 있을것 같습니다.
현재는 랭킹을 계산하면서 응답 객체를 한번 더 만들기 때문에 3번을 사용해도 괜찮지 않을까 생각합니다.
There was a problem hiding this comment.
저도 성재님 의견에서는 3번이 더 좋을 것 같습니다
그런데 저는 DAO에서 Response를 반환하게 된 이유, 고려했던 다른 방안이 궁금합니다!
Repository용 DTO가 생긴다면 저희 프로젝트의 하나의 고려사항이 더 생긴다는 의미인데, 해당 결론이 날 때까지의 과정이나 해당 DTO가 위치할 곳 등 최적화를 위해 고민했던 부분들이 알고 싶어요!
There was a problem hiding this comment.
제가 고려했던 방안은 다음과 같습니다.
- Projection 인터페이스 사용
- DTO를 정의하여 응답값을 한 번에 생성
- QueryDSL 사용
통계 API와 같이 다소 복잡한 쿼리를 사용하는 부분에서 가독성과 효율을 챙기는 방법이 무엇인지 고민하였습니다. QueryDSL은 작업 사이즈가 커지고, 최근 보안 이슈로 관리 중단되어 장기적 의존이 부담스러워 제외하였습니다. 1번과 2번 중에서는, Repository에서 application 계층의 응답 DTO를 직접 반환하는 것이 계층 간 결합을 만든다는 점이 걸렸지만, Projection 인터페이스도 파일이 하나 더 느는 것 대비 이점이 크지 않다고 판단하여 2번을 선택했었습니다.
하지만 성재님 의견처럼 Repository용 DTO를 dao 패키지에 별도 정의하고, 서비스에서 응답 DTO로 매핑하는 방식이 계층 분리도 지키면서 가장 깔끔할 것 같습니다. 해당 방향으로 수정해보겠습니다!
# Conflicts: # src/main/java/com/opus/opus/modules/contest/api/ContestController.java # src/main/java/com/opus/opus/modules/team/domain/dao/TeamRepository.java # src/test/java/com/opus/opus/restdocs/docs/ContestApiDocsTest.java
sjmoon00
left a comment
There was a problem hiding this comment.
수고하셨습니다~
코드를 이해하기 쉽게 작성하신것 같아요!
src/main/java/com/opus/opus/modules/team/domain/dao/TeamRepository.java
Outdated
Show resolved
Hide resolved
src/main/java/com/opus/opus/modules/team/application/TeamQueryService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/opus/opus/modules/team/application/TeamQueryService.java
Outdated
Show resolved
Hide resolved
JJimini
left a comment
There was a problem hiding this comment.
수고하셨습니다! comment 확인부탁드려요~!
src/main/java/com/opus/opus/modules/team/application/TeamQueryService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/opus/opus/modules/team/application/TeamQueryService.java
Outdated
Show resolved
Hide resolved
|
|
||
| private List<ContestRankingResponse> applyDenseRanking(List<ContestRankingResponse> votesPerTeam) { |
There was a problem hiding this comment.
Dense Ranking을 사용했다고 하셨는데, 기획적으로 논의가 완료된 부분인지 궁금합니다!
개인적으로는 Ranking이 좀 더 일반적으로 사용되지 않나 생각이 들어서요~!
There was a problem hiding this comment.
저희 MVP 단계부터 Dense Ranking을 사용하기로 기획 논의가 완료된 상태입니다!
다만 말씀하신 것처럼 일반적인 대회 순위에서는 Standard Ranking(1224 방식)이 더 보편적일 수 있을 것 같아요.
이 부분은 회의 때 한번 논의해보면 좋을 것 같습니다!
🔥 연관된 이슈
close: #9
close: #10
📜 작업 내용
💬 리뷰 요구사항
TeamRepository의 랭킹 쿼리가 다소 복잡합니다. Team 기준 시작 +LEFT JOIN TeamVote+LEFT JOIN ContestTrack으로 투표 0개 팀과 trackName까지 한방에 가져오도록 했습니다.ContestTrackRepository주입 없이 dense ranking 로직만 담당하고자 하였습니다.✨ 기타