[FIX] 행사 리스트 조회 오류를 수정합니다. - #246
Conversation
…into BM/fix/#243/attend-list
Walkthrough이 PR은 Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant S as AttendService
participant R as AttendRepository
C->>S: findMyAttendInfo(page, size, startDate, endDate) 호출
S->>R: findAllByMemberIdAndDateBetween(memberId, startDate, endDate) 호출
R-->>S: 전체 AttendEntity 리스트 반환
S->>S: 수동 페이징 처리 (인덱스 계산 및 서브리스트 생성)
S-->>C: PageResponse 반환
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the error in fetching the event list by switching from a member‐based pagination query to a more traditional pagination approach using in-memory sublisting. Key changes include:
- Adding a new repository method (findAllByMemberIdAndDateBetween) to fetch attend records within a date range.
- Replacing the previous pageable query with manual pagination logic in the service layer.
- Adjusting the pagination response construction in AttendService.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| eeos/src/main/java/com/blackcompany/eeos/target/persistence/AttendRepository.java | Added a new query method to fetch attend records by member ID and date range. |
| eeos/src/main/java/com/blackcompany/eeos/target/application/service/AttendService.java | Replaced the existing pageable query with logic to retrieve all attend records, then applying manual in-memory pagination. |
Comments suppressed due to low confidence (1)
eeos/src/main/java/com/blackcompany/eeos/target/application/service/AttendService.java:251
- After sublisting, 'myAttend.size()' reflects only the count of items on the current page rather than the full result count, which can mislead pagination. Consider storing the full count in a separate variable before sublisting and using it in the PageImpl constructor.
PageRequest.of(page - 1, size), myAttend.size()
| Pageable pageable); | ||
|
|
||
| @Query( | ||
| "SELECT a FROM AttendEntity a WHERE a.memberId = :memberId AND a.isDeleted = false AND a.createdDate >= :startDate AND a.createdDate <= :endDate") |
There was a problem hiding this comment.
The query does not include an ORDER BY clause. Without a consistent order, the results may be paginated inconsistently; consider adding an ORDER BY clause (e.g., 'ORDER BY a.createdDate DESC') for stable pagination.
| "SELECT a FROM AttendEntity a WHERE a.memberId = :memberId AND a.isDeleted = false AND a.createdDate >= :startDate AND a.createdDate <= :endDate") | |
| "SELECT a FROM AttendEntity a WHERE a.memberId = :memberId AND a.isDeleted = false AND a.createdDate >= :startDate AND a.createdDate <= :endDate ORDER BY a.createdDate ASC") |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
eeos/src/main/java/com/blackcompany/eeos/target/application/service/AttendService.java (1)
216-252: 성능 최적화 고려사항!현재 구현은 두 가지 성능 문제가 있을 수 있습니다:
전체 데이터를 먼저 조회한 후 메모리에서 페이징 처리하므로, 데이터가 많을 경우 메모리 사용량이 증가할 수 있습니다.
각
AttendEntity에 대해 별도의 쿼리로ProgramModel을 조회하고 있어 N+1 쿼리 문제가 발생할 수 있습니다.최적화 제안:
이미 총 개수를 알고 있다면, 쿼리 한 번으로 총 개수와 페이지 데이터를 모두 가져오는 것이 좋습니다.
N+1 쿼리 문제를 해결하기 위해 프로그램 ID 목록을 수집하여 한 번에 조회하는 방식을 고려해보세요:
// 페이지 데이터 조회 후 List<Long> programIds = myAttend.stream() .map(AttendEntity::getProgramId) .distinct() .collect(Collectors.toList()); Map<Long, ProgramModel> programMap = programRepository.findAllById(programIds).stream() .map(programEntityConverter::from) .collect(Collectors.toMap(ProgramModel::getId, program -> program)); // 이후 변환 로직에서 Map을 활용 .map(attendModel -> { ProgramModel program = programMap.get(attendModel.getProgramId()); if (program == null) return null; return attendInfoWithProgramConverter.from(attendModel, program); })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
eeos/src/main/java/com/blackcompany/eeos/target/application/service/AttendService.java(2 hunks)eeos/src/main/java/com/blackcompany/eeos/target/persistence/AttendRepository.java(1 hunks)
🔇 Additional comments (3)
eeos/src/main/java/com/blackcompany/eeos/target/persistence/AttendRepository.java (1)
69-74: 새로운 메서드 추가가 올바르게 이루어졌습니다!새로 추가된
findAllByMemberIdAndDateBetween메서드는 기존findAllByMemberIdAndCreatedDateGreaterThan메서드와 유사하지만 몇 가지 중요한 차이점이 있습니다:
Page<AttendEntity>대신List<AttendEntity>를 반환- 날짜 비교 연산자가
>및<(불포함)에서>=및<=(포함)으로 변경- 메서드 이름이 기능을 더 정확하게 반영
이러한 변경은 PR 목표에 맞게 페이징 방식을 변경하기 위한 적절한 개선입니다.
eeos/src/main/java/com/blackcompany/eeos/target/application/service/AttendService.java (2)
216-219: Repository 메서드 호출이 적절하게 수정되었습니다!페이징 방식 변경에 맞게
findAllByMemberIdAndDateBetween메서드를 사용하도록 코드가 수정되었습니다. 이 변경으로 인해 데이터베이스 레벨에서의 페이징 대신 애플리케이션 레벨에서 페이징을 처리할 수 있게 되었습니다.
222-233: 수동 페이징 구현이 효과적으로 이루어졌습니다!애플리케이션 레벨에서의 페이징 처리가 명확하고 논리적으로 구현되었습니다:
- 시작 인덱스와 종료 인덱스 계산
- 범위를 벗어나는 경우에 대한 예외 처리
- 빈 페이지 반환 처리
이 방식은 이전 구현에서 발생했던 행사 리스트 조회 오류를 해결할 것으로 예상됩니다.
| PageRequest.of(page - 1, size), | ||
| myAttend.size()); |
There was a problem hiding this comment.
총 요소 수 계산 로직 검토 필요!
PageImpl 생성자의 세 번째 매개변수는 전체 요소의 수여야 합니다. 현재 구현에서는 myAttend.size()를 사용하고 있는데, 이 값은 페이징 처리 후의 하위 목록 크기입니다.
이로 인해 애플리케이션의 페이지 탐색에 문제가 발생할 수 있습니다. 전체 데이터 셋의 크기를 전달해야 총 페이지 수가 정확하게 계산됩니다.
다음과 같이 수정하는 것을 제안합니다:
+ int totalSize = attendRepository.findAllByMemberIdAndDateBetween(
+ memberId, new Timestamp(startDate), new Timestamp(endDate)).size();
responses =
new PageImpl<>(
myAttend.stream()
.map(attendEntityConverter::from)
.map(
attendModel -> {
ProgramModel program =
programRepository
.findById(attendModel.getProgramId())
.map(programEntityConverter::from)
.orElse(null);
if (program == null) return null;
return attendInfoWithProgramConverter.from(attendModel, program);
})
.filter(Objects::nonNull)
.toList(),
PageRequest.of(page - 1, size),
- myAttend.size());
+ totalSize);📝 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.
| PageRequest.of(page - 1, size), | |
| myAttend.size()); | |
| int totalSize = attendRepository.findAllByMemberIdAndDateBetween( | |
| memberId, new Timestamp(startDate), new Timestamp(endDate)).size(); | |
| responses = | |
| new PageImpl<>( | |
| myAttend.stream() | |
| .map(attendEntityConverter::from) | |
| .map( | |
| attendModel -> { | |
| ProgramModel program = | |
| programRepository | |
| .findById(attendModel.getProgramId()) | |
| .map(programEntityConverter::from) | |
| .orElse(null); | |
| if (program == null) return null; | |
| return attendInfoWithProgramConverter.from(attendModel, program); | |
| }) | |
| .filter(Objects::nonNull) | |
| .toList(), | |
| PageRequest.of(page - 1, size), | |
| totalSize); |
- 생성일을 기준으로 내림차순
- 페이징 처리 후 size 가 아닌, 전체 size 로 수정
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
eeos/src/main/java/com/blackcompany/eeos/target/application/service/AttendService.java (1)
223-235: 수동 페이지네이션 로직이 적절히 구현되었습니다.애플리케이션 레벨에서 페이지네이션을 처리하기 위한 로직이 잘 구현되었습니다. 요청된 페이지가 데이터 범위를 벗어나는 경우와 마지막 페이지의 크기 조정을 모두 적절히 처리하고 있습니다.
다만, 대용량 데이터셋의 경우 전체 데이터를 메모리에 로드하는 방식은 성능 이슈가 발생할 수 있습니다. 현재는 행사 리스트 조회 문제 해결을 위한 변경이지만, 장기적으로는 데이터베이스 수준의 페이지네이션을 최적화하는 방안도 고려해보시기 바랍니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
eeos/src/main/java/com/blackcompany/eeos/target/application/service/AttendService.java(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (3)
eeos/src/main/java/com/blackcompany/eeos/target/application/service/AttendService.java (3)
216-218: 데이터 조회 방식이 변경되었습니다.이전에는 페이지네이션을 리포지토리 레벨에서 처리했지만, 이제는 전체 데이터를 먼저 조회한 후 애플리케이션 레벨에서 페이지네이션을 적용하는 방식으로 변경되었습니다. 이는 PR 목적에 맞게 행사 리스트 조회 오류를 해결하기 위한 변경으로 보입니다.
220-220: 총 요소 수 계산이 올바르게 수정되었습니다.이전 코드 리뷰에서 지적된 문제를 해결하기 위해 전체 요소 수를 올바르게 계산하고 있습니다. 페이징 처리 전의 전체 데이터 크기를
totalSize에 저장하고 있어 올바른 페이지 정보가 제공됩니다.
253-254: PageImpl 생성자 매개변수가 올바르게 수정되었습니다.이전 리뷰에서 지적된
PageImpl생성자의 세 번째 매개변수 문제가 수정되었습니다. 이제 페이징 처리 전의 전체 데이터 크기인totalSize를 전달하여 총 페이지 수가 정확하게 계산됩니다.
📌 관련 이슈
closes #243
✒️ 작업 내용
스크린샷 🏞️ (선택)
💬 REVIEWER에게 요구사항 💬
Summary by CodeRabbit