Skip to content

refactor: 232 refactor get apicourses 지연시간 줄이기 #234

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.

Sign up for GitHub

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

Open
jigun058 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

refactor: 232 refactor get apicourses 지연시간 줄이기 #234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a399244
refactor: getCourses api 지연시간 줄이기 리팩토링
jigun058 Aug 31, 2025
90d8c6e
refactor: getCourses api 지연시간 줄이기 리팩토링
jigun058 Aug 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/main/java/com/aloc/aloc/course/repository/UserCourseRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,17 @@ SELECT COUNT(uc) + 1
int findClearRank(@Param("course") Course course, @Param("updatedAt") LocalDateTime updatedAt);

long countByUserCourseState(UserCourseState userCourseState);

@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);
}
Comment on lines +49 to 61
Copy link

@coderabbitai coderabbitai bot Aug 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

10 changes: 10 additions & 0 deletions src/main/java/com/aloc/aloc/course/service/UserCourseService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
import java.time.LocalDateTime;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.stream.Collectors;
import lombok.AllArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
Expand Down Expand Up @@ -127,4 +129,12 @@ public int getClearRank(UserCourse userCourse) {
public long getUserCourseCountByUserCourseState(UserCourseState userCourseState) {
return userCourseRepository.countByUserCourseState(userCourseState);
}

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));
}
Comment on lines +133 to +139
Copy link

@coderabbitai coderabbitai bot Aug 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

빈 목록 대비 및 중복 키 안전성 보강 필요 (빈 IN 예외, toMap 충돌 가능성)

  • courseIds가 빈 리스트일 때 JPA IN (: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.

}
18 changes: 5 additions & 13 deletions src/main/java/com/aloc/aloc/user/service/facade/UserFacade.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,25 +276,17 @@ public CourseUserResponseDto closeUserCourse(Long courseId, String oauthId) {
public Page<CourseResponseDto> getCoursesByUser(
Pageable pageable, String oauthId, CourseType courseTypeOrNull) {
User user = userService.getUser(oauthId);
List<UserCourse> userCourses = userCourseService.getUserCoursesByUser(user);
Page<Course> courses = courseService.getCoursePageByCourseType(pageable, courseTypeOrNull);

Map<Long, UserCourse> latestUserCourseMap =
userCourses.stream()
.collect(
Collectors.groupingBy(
uc -> uc.getCourse().getId(),
Collectors.collectingAndThen(
Collectors.maxBy(Comparator.comparing(UserCourse::getCreatedAt)),
optional -> optional.orElse(null))));
List<Long> courseIds = courses.getContent().stream().map(Course::getId).toList();

Map<Long, UserCourseState> userCourseStateMap =
userCourseService.getLatestUserCourseStates(user, courseIds);

return courses.map(
course -> {
UserCourse latestUserCourse = latestUserCourseMap.get(course.getId());
UserCourseState state =
(latestUserCourse != null)
? latestUserCourse.getUserCourseState()
: UserCourseState.NOT_STARTED;
userCourseStateMap.getOrDefault(course.getId(), UserCourseState.NOT_STARTED);
return CourseResponseDto.of(course, state);
});
}
Expand Down