-
Notifications
You must be signed in to change notification settings - Fork 3
refactor : 푼 문제 리스트도 반환 #187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Walkthrough이 변경은 DailyCorrectCount DTO의 count 타입을 Long에서 int로 수정하고, 사용자 문제 결과 조회 구현에서 QueryDSL의 그룹핑/프로젝션 기반 집계를 제거하여, 페치한 (date, problemId) 쌍을 자바 컬렉션으로 그룹핑해 DailyCorrectCount 리스트를 구성하도록 흐름을 변경합니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller as Caller
participant Repo as UserProblemResultQueryRepositoryImpl
participant QDSL as QueryDSL/JPA
participant DB as Database
Caller->>Repo: findDailyCorrectCounts(...)
Repo->>QDSL: select (date, problemId) with filters
QDSL->>DB: Execute SQL
DB-->>QDSL: Rows: (date, problem_id)*
QDSL-->>Repo: List<Tuple>
Note over Repo: In-memory processing<br/>- Convert Date -> LocalDate<br/>- Group by date to Set<Long><br/>- Map to DailyCorrectCount(date, int count, Set)
Repo-->>Caller: List<DailyCorrectCount>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ 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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (6)
src/main/java/org/ezcode/codetest/domain/submission/dto/DailyCorrectCount.java (1)
6-11: DTO 불변성·일관성 보강(중복 상태 제거: count vs problemIds.size())동일 정보를 중복 보유하면 쉽게 불일치가 납니다. 방어적 복사와 검증(또는 자동 보정)으로 일관성을 강제하는 것을 권장합니다.
다음과 같이 compact constructor를 추가해
problemIds를 불변화하고count를 세트 크기와 일치시키세요:public record DailyCorrectCount( LocalDate date, int count, Set<Long> problemIds ) { -} + public DailyCorrectCount { + if (date == null) throw new IllegalArgumentException("date must not be null"); + if (problemIds == null) throw new IllegalArgumentException("problemIds must not be null"); + problemIds = Set.copyOf(problemIds); + // count 불일치 자동 보정(원하면 예외로 강제해도 됩니다) + if (count != problemIds.size()) { + count = problemIds.size(); + } + } +}추가 import가 필요합니다(파일 상단에 추가):
import java.util.Set; // 이미 존재 import java.util.Collections; // 사용하지 않으면 생략src/main/java/org/ezcode/codetest/infrastructure/persistence/repository/submission/query/UserProblemResultQueryRepositoryImpl.java (5)
20-23: 미사용 로거 제거 제안
@Slf4j를 추가했지만 사용하지 않습니다. 불필요한 의존/어노테이션은 줄이는 편이 깔끔합니다.-import lombok.extern.slf4j.Slf4j; @@ -@Slf4j @Repository @RequiredArgsConstructor public class UserProblemResultQueryRepositoryImpl implements UserProblemResultQueryRepository{
46-55: Tuple 인덱싱보다 타입드 프로젝션 고려
tuple.get(0, ...)인덱스 접근은 유지보수에 취약합니다. 필요 시 작은 내부 DTO/레코드 프로젝션으로 가독성과 안정성을 높일 수 있습니다.간단 예:
record DayProblem(LocalDate day, Long problemId) {} var results = queryFactory .select(Projections.constructor(DayProblem.class, day, upr.problem.id)) .from(upr) ... .fetch();
46-55: 문제 ID 집합의 순서 안정성응답 일관성이 필요하면
toSet()대신toCollection(LinkedHashSet::new)또는 정렬된 컬렉션을 사용하세요.- Collectors.mapping( - tuple -> tuple.get(1, Long.class), - Collectors.toSet() - ) + Collectors.mapping( + tuple -> tuple.get(1, Long.class), + Collectors.toCollection(java.util.LinkedHashSet::new) + )
56-63: DTO 불변성 보장 및 count 단일 소스화외부에
Set을 그대로 노출하면 이후 변형될 수 있습니다. 방어적 복사로 불변을 보장하세요.- return problemIdsByDate.entrySet().stream() - .map(entry -> new DailyCorrectCount( - entry.getKey(), - entry.getValue().size(), - entry.getValue() - )) + return problemIdsByDate.entrySet().stream() + .map(e -> new DailyCorrectCount( + e.getKey(), + e.getValue().size(), + java.util.Set.copyOf(e.getValue()) + )) .collect(Collectors.toList());(상단 DTO에 compact constructor를 추가하면 여기서는 단순히
new DailyCorrectCount(e.getKey(), e.getValue().size(), e.getValue())만으로도 안전해집니다.)
39-43: 쿼리 성능을 위한 인덱스 점검where 절 컬럼
(user_id, is_correct, modified_at)에 대해 합성 인덱스가 없으면 정렬/필터 비용이 큽니다.(user_id, is_correct, modified_at)또는 최소한(user_id, is_correct)인덱스를 고려하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/org/ezcode/codetest/domain/submission/dto/DailyCorrectCount.java(1 hunks)src/main/java/org/ezcode/codetest/infrastructure/persistence/repository/submission/query/UserProblemResultQueryRepositoryImpl.java(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/org/ezcode/codetest/infrastructure/persistence/repository/submission/query/UserProblemResultQueryRepositoryImpl.java (2)
src/main/java/org/ezcode/codetest/infrastructure/persistence/repository/submission/impl/UserProblemResultRepositoryImpl.java (1)
Repository(16-53)src/main/java/org/ezcode/codetest/domain/submission/service/SubmissionDomainService.java (1)
Slf4j(26-125)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
src/main/java/org/ezcode/codetest/domain/submission/dto/DailyCorrectCount.java (1)
8-8: 확인 완료 — DailyCorrectCount.count를 int로 변경해도 문제 없음검사 결과 count는 쿼리 구현에서 entry.getValue().size()로(int) 생성되어 서비스→응답 DTO로 그대로 전달되며, 오픈API/문서상 Long을 기대한다는 참조는 발견되지 않았습니다.
참고 위치: src/main/java/org/ezcode/codetest/infrastructure/persistence/repository/submission/query/UserProblemResultQueryRepositoryImpl.java (new DailyCorrectCount(... entry.getValue().size() ...)), src/main/java/org/ezcode/codetest/application/usermanagement/user/dto/response/UserDailySolvedHistoryResponse.java.src/main/java/org/ezcode/codetest/infrastructure/persistence/repository/submission/query/UserProblemResultQueryRepositoryImpl.java (2)
36-45: 중복 가능성 방지(.distinct) 여부 점검이론상 한 행당 하나의 문제지만, 동일 문제에 대한 다중 갱신이 쿼리 조건을 충족할 수 있다면 결과 중복이 생길 수 있습니다. 안전하게
.distinct()를 고려하세요.var results = queryFactory - .select(date, upr.problem.id) + .select(day, upr.problem.id) .from(upr) .where( upr.user.id.eq(userId), upr.isCorrect.eq(true) ) + .distinct() .orderBy(day.asc()) .fetch();중복이 구조적으로 불가능하다면 현 상태 유지해도 됩니다.
34-34: DB 이식성·타입 개선 — java.sql.Date/DATE() 대신 LocalDate로 캐스팅 권장레포에서 application*.yml/.properties를 찾을 수 없어 DB 방언을 확인하지 못했습니다. 아래 제안은 DB가
cast({0} as date)를 지원할 경우 적용하세요. 미지원 DB면 방언 전용 템플릿으로 조정 필요.- var date = Expressions.dateTemplate(Date.class, "DATE({0})", upr.modifiedAt); + var day = Expressions.dateTemplate(LocalDate.class, "cast({0} as date)", upr.modifiedAt); @@ - .orderBy(date.asc()) + .orderBy(day.asc()) @@ - tuple -> Objects.requireNonNull(tuple.get(0, Date.class)).toLocalDate(), + tuple -> Objects.requireNonNull(tuple.get(0, LocalDate.class)),application*.yml/.properties 위치와 사용 DB 방언(hibernate.dialect 또는 spring.datasource.url)을 확인.
작업 내용
변경 사항
UserService.createUser()메서드 추가@Email유효성 검증 적용트러블 슈팅
@Transactional이 적용되지 않음this.→AopProxyUtils.사용)해결해야 할 문제
UserController에서 비즈니스 로직 일부 처리 → 서비스로 이전 고려 필요참고 사항
코드 리뷰 전 확인 체크리스트
type :)Summary by CodeRabbit