[CMAT-63] fix: 스크랩한 컨텐츠 조회 api 수정 #45
Conversation
Walkthrough본 PR은 Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/java/UMC/career_mate/domain/contentScrap/repository/ContentScrapRepository.java (1)
23-31: 성능 최적화를 위한 JOIN FETCH 사용이 좋습니다만, 페이징 처리 시 주의가 필요합니다.JOIN FETCH를 사용하여 N+1 문제를 해결한 것은 좋은 접근입니다. 하지만 JOIN FETCH와 페이징(Pageable)을 함께 사용할 때 몇 가지 고려사항이 있습니다:
- 메모리 사용량이 증가할 수 있습니다.
- 데이터가 많은 경우 성능 저하가 발생할 수 있습니다.
다음과 같은 대안을 고려해보시는 것은 어떨까요?:
@Query(value = "SELECT DISTINCT cs FROM ContentScrap cs " + "JOIN cs.content c " + "JOIN c.job j " + "WHERE cs.member = :member AND j.id = :jobId", countQuery = "SELECT COUNT(DISTINCT cs) FROM ContentScrap cs " + "JOIN cs.content c " + "JOIN c.job j " + "WHERE cs.member = :member AND j.id = :jobId") Page<ContentScrap> findByMemberAndJobIdWithFetch( @Param("member") Member member, @Param("jobId") Long jobId, Pageable pageable );이렇게 수정하면:
- DISTINCT를 사용하여 중복을 제거
- 별도의 countQuery를 정의하여 정확한 페이징 처리
- JOIN FETCH 대신 일반 JOIN을 사용하여 메모리 사용량 최적화
src/main/java/UMC/career_mate/domain/contentScrap/service/ContentScrapService.java (2)
56-60: 에러 메시지 업데이트 필요null 체크를
jobId에서job객체로 변경한 것은 좋은 개선이지만, 에러 메시지(NOT_FOUND_BY_JOB_ID)가 현재 검사 로직과 일치하지 않습니다.에러 코드를
NOT_FOUND_JOB과 같이 더 명확한 메시지로 변경하는 것을 고려해보세요.
71-75: 페이지네이션 응답이 개선되었습니다.페이지네이션 응답 구조가 더 명확해졌으며,
hasNext추가로 클라이언트 측에서 더 나은 페이지 처리가 가능해졌습니다.추가 개선사항으로
totalElements나totalPages도 포함하는 것을 고려해보세요. 이는 클라이언트 측에서 전체 페이지 수를 파악하는데 도움이 될 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/UMC/career_mate/domain/contentScrap/dto/response/ContentScrapResponseDTO.java(1 hunks)src/main/java/UMC/career_mate/domain/contentScrap/repository/ContentScrapRepository.java(1 hunks)src/main/java/UMC/career_mate/domain/contentScrap/service/ContentScrapService.java(1 hunks)
🔇 Additional comments (2)
src/main/java/UMC/career_mate/domain/contentScrap/dto/response/ContentScrapResponseDTO.java (1)
11-11: DTO에 jobId 필드가 적절히 추가되었습니다.스크랩된 컨텐츠와 관련된 Job을 식별하기 위한 필드가 올바르게 추가되었습니다.
src/main/java/UMC/career_mate/domain/contentScrap/service/ContentScrapService.java (1)
64-65: 성능 최적화가 잘 이루어졌습니다!JOIN FETCH를 활용하여 N+1 문제를 해결한 것이 좋은 개선입니다. 또한 주석을 통해 변경 의도가 명확히 전달되었습니다.
| String title, | ||
| String url, | ||
| String photo, | ||
| Long jobId, |
There was a problem hiding this comment.
Converter에서 jobId 처리 추가해야할 거 같아요
There was a problem hiding this comment.
이 부분 잘못 들어가서 빼두었습니다..! 나머지도 모두 수정해두었습니다
| Page<ContentScrap> findByMemberAndJobId(@Param("member") Member member, @Param("jobId") Long jobId, Pageable pageable); | ||
| @Query("SELECT cs FROM ContentScrap cs " + | ||
| "JOIN FETCH cs.content c " + | ||
| "JOIN FETCH c.job j " + |
kyoooooong
force-pushed
the
feature/CMAT-63
branch
from
ce3ba71 to
e9d01c2
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/UMC/career_mate/domain/contentScrap/repository/ContentScrapRepository.java (1)
27-31: 메서드 이름에 대한 제안현재 메서드 이름
findByMemberAndJobIdWithFetch는 구현 세부사항을 포함하고 있습니다. 비즈니스 의도를 더 잘 표현하는 이름을 고려해보세요.다음과 같은 이름을 제안드립니다:
- Page<ContentScrap> findByMemberAndJobIdWithFetch( + Page<ContentScrap> findScrapsByMemberAndJobId( @Param("member") Member member, @Param("jobId") Long jobId, Pageable pageable );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/UMC/career_mate/domain/contentScrap/repository/ContentScrapRepository.java(1 hunks)src/main/java/UMC/career_mate/domain/contentScrap/service/ContentScrapService.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/UMC/career_mate/domain/contentScrap/service/ContentScrapService.java
🔇 Additional comments (1)
src/main/java/UMC/career_mate/domain/contentScrap/repository/ContentScrapRepository.java (1)
23-31: 성능 최적화를 위한 JOIN FETCH 쿼리 검토가 필요합니다.현재 구현된 쿼리에서 다음 사항들을 고려해 주시기 바랍니다:
- 이전 리뷰어의 의견처럼 job 엔티티에 대한 페치 조인이 실제로 필요한지 검토가 필요합니다.
- 페이징 처리와 함께 여러 개의 페치 조인을 사용하면 메모리 이슈가 발생할 수 있습니다.
성능 최적화를 위해 다음과 같이 수정하는 것을 고려해보세요:
@Query("SELECT cs FROM ContentScrap cs " + "JOIN FETCH cs.content c " + - "JOIN FETCH c.job j " + + "JOIN c.job j " + "WHERE cs.member = :member AND j.id = :jobId")
kyoooooong
force-pushed
the
feature/CMAT-63
branch
2 times, most recently
from
22192b4 to
97571b2
Compare
kyoooooong
force-pushed
the
feature/CMAT-63
branch
from
97571b2 to
ea75cee
Compare
#️⃣ 요약 설명
📝 작업 내용
// 핵심 코드를 붙여넣기 해주세요코드에 대한 간단한 설명 부탁드립니다.
동작 확인
💬 리뷰 요구사항(선택)
Summary by CodeRabbit
Bug Fixes
Refactor
Documentation