Refactor/user problem result query #186
Conversation
WalkthroughQueryDSL의 GroupBy/Projections 기반 집계를 제거하고, 동일 쿼리로 원시 결과를 fetch한 뒤 애플리케이션 코드에서 날짜별로 수기 그룹핑하여 DailyCorrectCount 목록을 생성하도록 리포지토리 구현을 변경했습니다. 외부 공개 API와 SQL의 DATE 추출 로직은 그대로 유지됩니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Svc as Service/Caller
participant Repo as UserProblemResultQueryRepositoryImpl
participant DB as Database
Svc->>Repo: countCorrectByUserGroupedByDate(userId, ...)
Repo->>DB: SELECT DATE(modifiedAt), COUNT(DISTINCT ...), problemId
DB-->>Repo: Row stream (date, countDistinct, problemId)
rect rgb(235, 245, 255)
note over Repo: 애플리케이션 레벨 그룹핑 (변경)
Repo->>Repo: 날짜별 LinkedHashMap에 problemId 집합/카운트 누적
Repo->>Repo: DailyCorrectCount 리스트 생성
end
Repo-->>Svc: List<DailyCorrectCount>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/main/java/org/ezcode/codetest/infrastructure/persistence/repository/submission/query/UserProblemResultQueryRepositoryImpl.java (3)
32-45: 중복 레코드 축소를 위해 DB 레벨 DISTINCT 추가를 고려하세요.같은 날짜·문제에 대한 다중 정답을 애플리케이션에서 Set으로 제거하는 대신, DB에서 먼저
SELECT DISTINCT로 줄이면 I/O와 메모리를 절감할 수 있습니다.- var results = queryFactory - .select( - date, - upr.problem.id - ) + var results = queryFactory + .select( + date, + upr.problem.id + ) + .distinct() .from(upr) .where( upr.user.id.eq(userId), upr.isCorrect.eq(true) ) .orderBy(date.asc()) .fetch();
39-42: 쿼리 조건에 맞춘 인덱스 제안(성능).
user_id,is_correct,modified_at,problem_id복합 인덱스를 고려하면 필터링과 정렬, 조인(문제 ID 접근)이 빨라집니다. 예:(user_id, is_correct, modified_at, problem_id).
46-46: 주석 톤/명확성 개선 제안.
// 결과를 수동으로 그룹화해야함→ “DB 집계를 제거했으므로 (날짜, 문제)별로 애플리케이션에서 그룹화/중복제거”처럼 의도를 더 구체화하면 유지보수에 유리합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/org/ezcode/codetest/infrastructure/persistence/repository/submission/query/UserProblemResultQueryRepositoryImpl.java(2 hunks)
⏰ 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 (2)
src/main/java/org/ezcode/codetest/infrastructure/persistence/repository/submission/query/UserProblemResultQueryRepositoryImpl.java (2)
5-11: 필요 컬렉션/스트림 import 적절합니다.현재 파일 내 사용과 일치하며 불필요한 import는 보이지 않습니다.
31-31: DATE 템플릿의 타임존·DB 방언 검증 필요 — 설정파일 미발견레포 검색 결과 application*.yml/.properties는 src/test/resources/application-test.properties만 존재하여 spring.jpa.database-platform / hibernate.dialect / spring.jpa.properties.hibernate.jdbc.time_zone / spring.datasource.url 등을 확인할 수 없습니다. DATE({0})는 DB 방언과 타임존 처리에 따라 결과가 달라질 수 있으므로 설정 위치(또는 환경변수/Config Server/Docker/K8s)를 확인해야 합니다.
위치: src/main/java/org/ezcode/codetest/infrastructure/persistence/repository/submission/query/UserProblemResultQueryRepositoryImpl.java (라인 31)
조치(루트에서 실행하고 출력 결과 제출):
fd -a 'application*.yml' -t f 2>/dev/null || true fd -a 'application*.properties' -t f 2>/dev/null || true rg -nC2 -S -e 'spring\.jpa\.database-platform' \ -e 'spring\.datasource\.url' \ -e 'hibernate\.dialect' \ -e 'spring\.jpa\.properties\.hibernate\.jdbc\.time_zone' \ -e 'server\.timezone' . # 환경변수 확인 (로컬/CI/컨테이너) env | rg -i 'SPRING|HIBERNATE|DATASOURCE|TIME_ZONE|TZ' || true
| var results = queryFactory | ||
| .select( | ||
| date, | ||
| upr.problem.id.countDistinct(), | ||
| upr.problem.id | ||
| ) | ||
| .from(upr) |
There was a problem hiding this comment.
집계 함수와 비집계 컬럼 동시 선택으로 JPQL/SQL 오류 위험이 큽니다.
GROUP BY 없이 countDistinct()와 date, problem.id를 함께 select하면 JPQL 규약 위반이며, 다수 DB에서 실행 오류가 납니다. 수기 그룹핑으로 갈 거라면 집계를 쿼리에서 제거해야 합니다.
아래 최소 수정으로 집계 컬럼을 제거하세요(아래 인덱스 수정은 다음 코멘트 참조).
- var results = queryFactory
- .select(
- date,
- upr.problem.id.countDistinct(),
- upr.problem.id
- )
+ var results = queryFactory
+ .select(
+ date,
+ upr.problem.id
+ )
.from(upr)
.where(
upr.user.id.eq(userId),
upr.isCorrect.eq(true)
)
.orderBy(date.asc())
.fetch();Also applies to: 44-44
| Map<LocalDate, Long> countsByDate = new LinkedHashMap<>(); | ||
|
|
||
| for (var result : results) { | ||
| LocalDate resultDate = result.get(0, LocalDate.class); | ||
| Long problemId = result.get(2, Long.class); | ||
|
|
||
| problemIdsByDate.computeIfAbsent(resultDate, k -> new HashSet<>()).add(problemId); | ||
| countsByDate.put(resultDate, (long) problemIdsByDate.get(resultDate).size()); | ||
| } |
There was a problem hiding this comment.
선택 컬럼 인덱스 불일치 및 불필요한 countsByDate 유지.
위 수정으로 select 컬럼이 2개가 되므로 result.get(2, ...)는 범위 밖 접근이 됩니다. 또한 countsByDate는 Set 크기에서 항상 유도 가능해 중복입니다.
아래처럼 인덱스와 반환 구성을 정리하세요.
- Map<LocalDate, Long> countsByDate = new LinkedHashMap<>();
-
- for (var result : results) {
- LocalDate resultDate = result.get(0, LocalDate.class);
- Long problemId = result.get(2, Long.class);
-
- problemIdsByDate.computeIfAbsent(resultDate, k -> new HashSet<>()).add(problemId);
- countsByDate.put(resultDate, (long) problemIdsByDate.get(resultDate).size());
- }
-
- return problemIdsByDate.entrySet().stream()
- .map(entry -> new DailyCorrectCount(
- entry.getKey(),
- countsByDate.get(entry.getKey()),
- entry.getValue()
- ))
- .collect(Collectors.toList());
+ for (var result : results) {
+ LocalDate resultDate = result.get(0, LocalDate.class);
+ Long problemId = result.get(1, Long.class);
+ problemIdsByDate
+ .computeIfAbsent(resultDate, k -> new HashSet<>())
+ .add(problemId);
+ }
+
+ return problemIdsByDate.entrySet().stream()
+ .map(entry -> new DailyCorrectCount(
+ entry.getKey(),
+ (long) entry.getValue().size(),
+ entry.getValue()
+ ))
+ .collect(Collectors.toList());Also applies to: 58-64
🤖 Prompt for AI Agents
In
src/main/java/org/ezcode/codetest/infrastructure/persistence/repository/submission/query/UserProblemResultQueryRepositoryImpl.java
around lines 48-56 (and similarly 58-64), the loop uses result.get(2,
Long.class) but the SELECT now returns only two columns so index 2 is out of
range and countsByDate is redundant; change to use the correct column index for
problemId (likely result.get(1, Long.class) or the second selected column),
remove the countsByDate map entirely, update the loop to only populate
problemIdsByDate (computeIfAbsent(...).add(problemId)), and when you need counts
derive them from problemIdsByDate.get(date).size() rather than maintaining a
separate map; apply the same index and removal fix to the code at lines 58-64.
작업 내용
변경 사항
UserService.createUser()메서드 추가@Email유효성 검증 적용트러블 슈팅
@Transactional이 적용되지 않음this.→AopProxyUtils.사용)해결해야 할 문제
UserController에서 비즈니스 로직 일부 처리 → 서비스로 이전 고려 필요참고 사항
코드 리뷰 전 확인 체크리스트
type :)Summary by CodeRabbit
Refactor
Chores