refactor: 232 refactor get apicourses 지연시간 줄이기 #234
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Walkthrough사용자별 코스 목록 조회 흐름에서 최신 UserCourse 상태 계산을 스트림/그루핑 방식 대신, 코스 ID 리스트 기반의 전용 레포지토리 쿼리와 서비스 메서드로 대체했습니다. 레포지토리에 최신 레코드 조회용 JPQL이 추가되고, 서비스는 이를 맵으로 변환해 파사드에서 사용합니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant F as UserFacade
participant S as UserCourseService
participant R as UserCourseRepository
participant DB as Database
C->>F: GET /api/courses (paged)
F->>F: 페이지 코스 목록에서 courseIds 수집
F->>S: getLatestUserCourseStates(user, courseIds)
S->>R: findLatestUserCoursesByUserAndCourseIds(user, courseIds)
R->>DB: SELECT latest UserCourse by (user, courseId IN list)
DB-->>R: List<UserCourse>
R-->>S: List<UserCourse>
S->>S: Map<courseId, state>로 변환
S-->>F: Map<Long, UserCourseState>
F->>F: 각 코스에 상태 할당 (기본 NOT_STARTED)
F-->>C: 응답 (코스 + 상태)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related issues
Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (1)
src/main/java/com/aloc/aloc/user/service/facade/UserFacade.java (1)
281-285: 컨텐츠 없음일 때 불필요한 레포 호출 회피페이지가 비어 있으면 레포 호출을 생략하고 즉시 빈 페이지(혹은 NOT_STARTED 매핑)로 반환하세요. 서비스에서 빈 리스트 가드를 추가하더라도, 상위에서 호출 자체를 막는 편이 안전하고 가볍습니다.
Page<Course> courses = courseService.getCoursePageByCourseType(pageable, courseTypeOrNull); -List<Long> courseIds = courses.getContent().stream().map(Course::getId).toList(); +List<Long> courseIds = courses.getContent().stream().map(Course::getId).toList(); +if (courseIds.isEmpty()) { + return courses.map(course -> CourseResponseDto.of(course, UserCourseState.NOT_STARTED)); +} Map<Long, UserCourseState> userCourseStateMap = userCourseService.getLatestUserCourseStates(user, courseIds);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/main/java/com/aloc/aloc/course/repository/UserCourseRepository.java(1 hunks)src/main/java/com/aloc/aloc/course/service/UserCourseService.java(2 hunks)src/main/java/com/aloc/aloc/user/service/facade/UserFacade.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/main/java/com/aloc/aloc/course/repository/UserCourseRepository.java (2)
src/main/java/com/aloc/aloc/problem/repository/UserCourseProblemRepository.java (1)
UserCourseProblemRepository(15-31)src/main/java/com/aloc/aloc/course/repository/CourseRepository.java (1)
CourseRepository(13-27)
src/main/java/com/aloc/aloc/user/service/facade/UserFacade.java (2)
src/main/java/com/aloc/aloc/course/service/CourseService.java (1)
getCourses(26-29)src/main/java/com/aloc/aloc/usercourse/entity/UserCourse.java (1)
updateUserCourseState(63-65)
src/main/java/com/aloc/aloc/course/service/UserCourseService.java (1)
src/main/java/com/aloc/aloc/usercourse/entity/UserCourse.java (1)
updateUserCourseState(63-65)
🔇 Additional comments (2)
src/main/java/com/aloc/aloc/course/service/UserCourseService.java (1)
17-19: 불필요한 변경 없음 – 임포트 추가 OKMap/Collectors 임포트 추가는 합당합니다.
src/main/java/com/aloc/aloc/user/service/facade/UserFacade.java (1)
289-289: 기본값 처리 LGTM
getOrDefault(..., NOT_STARTED)기본값 전략 적절합니다.
Comment on lines
+49
to
61
| @Query( | ||
| """ | ||
| SELECT uc FROM UserCourse uc | ||
| WHERE uc.user = :user AND uc.course.id IN :courseIds | ||
| AND uc.createdAt = ( | ||
| SELECT MAX(uc2.createdAt) | ||
| FROM UserCourse uc2 | ||
| WHERE uc2.user = :user AND uc2.course.id = uc.course.id | ||
| ) | ||
| """) | ||
| List<UserCourse> findLatestUserCoursesByUserAndCourseIds( | ||
| @Param("user") User user, @Param("courseIds") List<Long> courseIds); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
타이브레이커 없는 상관 서브쿼리 → 중복 행 가능성; toMap 충돌/비결정성 위험
createdAt = MAX(createdAt)만으로는 동률 시 다건 반환 가능성이 있습니다. 서비스의 toMap 병합으로 임시 방어하더라도, 근본적으로 쿼리에서 단일 행이 선택되도록 타이브레이커를 넣어주세요. 일반적으로 단조 증가 PK(id)를 사용하는 것이 간단하고 DB에 독립적입니다.
-@Query(
- """
- SELECT uc FROM UserCourse uc
- WHERE uc.user = :user AND uc.course.id IN :courseIds
- AND uc.createdAt = (
- SELECT MAX(uc2.createdAt)
- FROM UserCourse uc2
- WHERE uc2.user = :user AND uc2.course.id = uc.course.id
- )
-"""
-)
+@Query(
+ """
+ SELECT uc FROM UserCourse uc
+ WHERE uc.user = :user
+ AND uc.course.id IN :courseIds
+ AND uc.id = (
+ SELECT MAX(uc2.id)
+ FROM UserCourse uc2
+ WHERE uc2.user = :user
+ AND uc2.course.id = uc.course.id
+ )
+"""
+)
List<UserCourse> findLatestUserCoursesByUserAndCourseIds(
@Param("user") User user, @Param("courseIds") List<Long> courseIds);보완 제안:
IN :courseIds는 빈 리스트에서 예외가 나므로, 상위 서비스에서 빈 리스트 조기 리턴(본 리뷰의 Service 코멘트)으로 방지하세요.- 성능:
(user_id, course_id, id)또는(user_id, course_id, created_at)복합 인덱스를 고려하세요(아래 참조).
인덱스 제안:
- MySQL/InnoDB 기준
ALTER TABLE user_course ADD INDEX idx_user_course_latest (user_id, course_id, id);
📝 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.
Suggested change
| @Query( | |
| """ | |
| SELECT uc FROM UserCourse uc | |
| WHERE uc.user = :user AND uc.course.id IN :courseIds | |
| AND uc.createdAt = ( | |
| SELECT MAX(uc2.createdAt) | |
| FROM UserCourse uc2 | |
| WHERE uc2.user = :user AND uc2.course.id = uc.course.id | |
| ) | |
| """) | |
| List<UserCourse> findLatestUserCoursesByUserAndCourseIds( | |
| @Param("user") User user, @Param("courseIds") List<Long> courseIds); | |
| } | |
| @Query( | |
| """ | |
| SELECT uc FROM UserCourse uc | |
| WHERE uc.user = :user | |
| AND uc.course.id IN :courseIds | |
| AND uc.id = ( | |
| SELECT MAX(uc2.id) | |
| FROM UserCourse uc2 | |
| WHERE uc2.user = :user | |
| AND uc2.course.id = uc.course.id | |
| ) | |
| """ | |
| ) | |
| List<UserCourse> findLatestUserCoursesByUserAndCourseIds( | |
| @Param("user") User user, @Param("courseIds") List<Long> courseIds); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/aloc/aloc/course/repository/UserCourseRepository.java
around lines 49 to 61, the subquery only compares createdAt which can return
multiple rows on ties leading to nondeterministic duplicates downstream; change
the selection to guarantee a single row by using a tiebreaker on the
monotonically increasing PK (id) — e.g., replace the createdAt = (SELECT
MAX(...)) condition with id = (SELECT MAX(uc2.id) ... WHERE uc2.user = :user AND
uc2.course.id = uc.course.id) or add an additional id = subcondition so ties on
createdAt resolve to the latest id; also ensure the calling service
short-circuits when courseIds is empty to avoid IN () issues, and add a
composite index like (user_id, course_id, id) at the DB level for performance.
Comment on lines
+133
to
+139
| public Map<Long, UserCourseState> getLatestUserCourseStates(User user, List<Long> courseIds) { | ||
| List<UserCourse> latestUserCourses = | ||
| userCourseRepository.findLatestUserCoursesByUserAndCourseIds(user, courseIds); | ||
|
|
||
| return latestUserCourses.stream() | ||
| .collect(Collectors.toMap(uc -> uc.getCourse().getId(), UserCourse::getUserCourseState)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
빈 목록 대비 및 중복 키 안전성 보강 필요 (빈 IN 예외, toMap 충돌 가능성)
courseIds가 빈 리스트일 때 JPAIN (:courseIds)는 런타임 예외를 유발할 수 있습니다. 서비스 레벨에서 조기 리턴하세요.- 저장 시각 동률 등으로 레포지토리 쿼리가 동일 코스에 대해 2건을 반환하면
Collectors.toMap이 중복 키로 터질 수 있습니다(병합 함수 없음). 방어적으로 병합 함수를 추가하거나(임시 방편), 레포지토리 쿼리에서 타이브레이커를 보장하세요(아래 레포 코멘트 참조).
다음과 같이 조치 제안드립니다.
@Service
@AllArgsConstructor
public class UserCourseService {
@@
- public Map<Long, UserCourseState> getLatestUserCourseStates(User user, List<Long> courseIds) {
+ @Transactional(readOnly = true)
+ public Map<Long, UserCourseState> getLatestUserCourseStates(User user, List<Long> courseIds) {
+ if (courseIds == null || courseIds.isEmpty()) {
+ return java.util.Collections.emptyMap();
+ }
List<UserCourse> latestUserCourses =
userCourseRepository.findLatestUserCoursesByUserAndCourseIds(user, courseIds);
- return latestUserCourses.stream()
- .collect(Collectors.toMap(uc -> uc.getCourse().getId(), UserCourse::getUserCourseState));
+ return latestUserCourses.stream()
+ .collect(
+ Collectors.toMap(
+ uc -> uc.getCourse().getId(),
+ UserCourse::getUserCourseState,
+ (left, right) -> left // 레포 타이브레이커 적용 전 임시 병합
+ ));
}부가 제안(선택): 레포 쿼리를 코스 ID와 상태만 프로젝션으로 가져오면(예: select uc.course.id, uc.userCourseState) 엔티티 하이드레이션을 줄일 수 있습니다.
🤖 Prompt for AI Agents
In src/main/java/com/aloc/aloc/course/service/UserCourseService.java around
lines 133-139, handle the case where courseIds is empty by returning an empty
map early to avoid JPA IN(:courseIds) runtime errors, and make the stream
collection defensive against duplicate course IDs by providing a merge function
to Collectors.toMap (e.g., pick the latest/first value or resolve
deterministically) or by deduplicating/ordering latestUserCourses before
collecting; optionally consider changing the repository to return only courseId
and state projection to avoid hydrating full entities.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📌 PR 제목
✅ 작업 내용
🔍 체크리스트
📸 스크린샷 (UI 변경이 있다면)
🔗 관련 이슈
Summary by CodeRabbit