-
Notifications
You must be signed in to change notification settings - Fork 1
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.
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
[DDING-82] OSIV 비활성화로 인한 지연로딩 에러 수정 #226
Conversation
워크스루이 풀 리퀘스트는 주로 스코어 히스토리 관련 클래스들의 데이터 구조와 로깅 기능을 변경합니다. 변경 사항
가능한 관련 PR
제안된 라벨
제안된 리뷰어
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/ddingdong/ddingdongBE/domain/clubmember/service/FacadeCentralClubMemberServiceImpl.java (1)
38-40
: 프록시 객체 초기화 방식 개선 제안현재 구현은 OSIV 비활성화로 인한 지연로딩 문제를 해결하지만, 다음과 같은 개선사항을 고려해보시기 바랍니다:
- size() 메소드 사용은 의도가 명확하지 않습니다
- 주석으로 의도를 설명하는 것보다 명시적인 메소드 사용이 바람직합니다
다음과 같이 개선하는 것을 제안드립니다:
- List<ClubMember> clubMembers = club.getClubMembers(); - clubMembers.size(); //프록시 객체 초기화 + List<ClubMember> clubMembers = initializeClubMembers(club.getClubMembers()); return AllClubMemberInfoQuery.of(club.getName(), clubMembers);그리고 다음 유틸리티 메소드를 추가하시면 좋을 것 같습니다:
private List<ClubMember> initializeClubMembers(List<ClubMember> members) { Hibernate.initialize(members); return members; }이렇게 하면:
- 코드의 의도가 더 명확해집니다
- 프록시 초기화 로직이 캡슐화됩니다
- 다른 메소드에서도 재사용이 가능합니다
src/main/java/ddingdong/ddingdongBE/domain/scorehistory/controller/dto/response/AdminScoreHistoryListResponse.java (1)
27-27
: 일관된 리팩토링 패턴이 잘 적용되었습니다.
ClubScoreHistoryListResponse
와 동일한 패턴으로 리팩토링되어 코드의 일관성이 잘 유지되었습니다.추가 제안사항:
- 두 응답 클래스가 매우 유사한 구조를 가지고 있으므로, 공통 부분을 추상화하여 코드 중복을 줄일 수 있을 것 같습니다.
- 예: 공통 베이스 클래스나 인터페이스를 도입하여
from
메소드의 중복을 제거
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/ddingdong/ddingdongBE/domain/club/service/GeneralClubService.java
(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/clubmember/service/FacadeCentralClubMemberServiceImpl.java
(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/scorehistory/controller/dto/response/AdminScoreHistoryListResponse.java
(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/scorehistory/controller/dto/response/ClubScoreHistoryListResponse.java
(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/scorehistory/service/FacadeAdminScoreHistoryServiceImpl.java
(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/scorehistory/service/FacadeClubScoreHistoryServiceImpl.java
(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/scorehistory/service/dto/query/AdminClubScoreHistoryListQuery.java
(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/scorehistory/service/dto/query/ClubScoreHistoryListQuery.java
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/java/ddingdong/ddingdongBE/domain/club/service/GeneralClubService.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and analyze
🔇 Additional comments (6)
src/main/java/ddingdong/ddingdongBE/domain/clubmember/service/FacadeCentralClubMemberServiceImpl.java (1)
15-15
: 로깅 기능 추가 승인Lombok의 @slf4j 어노테이션을 추가한 것은 적절한 변경사항입니다. 향후 문제 해결과 모니터링에 도움이 될 것입니다.
Also applies to: 22-22
src/main/java/ddingdong/ddingdongBE/domain/scorehistory/service/dto/query/ClubScoreHistoryListQuery.java (1)
8-13
: Club 엔티티 대신 필요한 데이터만 전달하도록 개선되었습니다.세션 범위 밖에서 Club 엔티티의 프록시 객체에 접근하는 대신 필요한 점수 데이터만 전달하도록 변경한 것이 좋습니다. 이는 OSIV 비활성화 환경에서 발생할 수 있는 LazyInitializationException을 방지하는 좋은 방법입니다.
src/main/java/ddingdong/ddingdongBE/domain/scorehistory/service/dto/query/AdminClubScoreHistoryListQuery.java (1)
8-13
: 일관된 방식으로 지연 로딩 문제가 해결되었습니다.ClubScoreHistoryListQuery와 동일한 방식으로 Club 엔티티 대신 점수 값만 전달하도록 개선되어 일관성 있게 처리되었습니다.
src/main/java/ddingdong/ddingdongBE/domain/scorehistory/service/FacadeClubScoreHistoryServiceImpl.java (1)
24-24
: 트랜잭션 범위 내에서 데이터를 초기화하도록 개선되었습니다.서비스 계층의 트랜잭션 범위 내에서 Score 값을 조회하여 전달함으로써, 세션이 닫힌 후에도 안전하게 데이터를 사용할 수 있습니다.
src/main/java/ddingdong/ddingdongBE/domain/scorehistory/service/FacadeAdminScoreHistoryServiceImpl.java (1)
37-37
: 관리자용 서비스에서도 동일한 패턴으로 개선되었습니다.Score 데이터를 트랜잭션 범위 내에서 안전하게 초기화하여 전달하도록 변경되었습니다. 이는 OSIV 비활성화 환경에서도 안정적으로 동작할 것입니다.
src/main/java/ddingdong/ddingdongBE/domain/scorehistory/controller/dto/response/ClubScoreHistoryListResponse.java (1)
26-26
: 지연 로딩 문제를 효과적으로 해결한 리팩토링입니다!
club().getScore().getValue()
와 같은 연쇄 호출을clubTotalScore()
로 단순화함으로써:
- 프록시 객체의 지연 로딩 문제를 방지
- 코드의 가독성 향상
- 성능 최적화 달성
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다!
...java/ddingdong/ddingdongBE/domain/clubmember/service/FacadeCentralClubMemberServiceImpl.java
Outdated
Show resolved
Hide resolved
고생 많으셨습니다! |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/java/ddingdong/ddingdongBE/domain/club/repository/ClubRepository.java (1)
14-15
: 구현이 잘 되었습니다만, 문서화를 추가하면 좋겠습니다!EntityGraph를 사용한 즉시 로딩 구현이 OSIV 비활성화 환경에서 발생하는 지연 로딩 문제를 효과적으로 해결했습니다. 다만, 다음과 같은 개선사항을 제안드립니다:
+ /** + * userId로 Club을 조회하며, clubMembers를 즉시 로딩합니다. + * OSIV가 비활성화된 환경에서 LazyInitializationException을 방지하기 위해 사용됩니다. + * + * @param userId 사용자 ID + * @return Club 엔티티 (clubMembers 포함) + */ @EntityGraph(attributePaths = {"clubMembers"}) Optional<Club> findEntityGraphByUserId(Long userId);src/main/java/ddingdong/ddingdongBE/domain/club/service/GeneralClubService.java (2)
8-8
: 로깅 구현이 누락되었습니다.@slf4j 어노테이션을 추가하셨지만, 실제 로그 구현이 없습니다. 주요 작업에 대한 로깅을 추가하는 것이 좋습니다.
예시:
@Override public Club getByUserIdWithFetch(Long userId) { + log.debug("Fetching club with members for userId: {}", userId); return clubRepository.findEntityGraphByUserId(userId) .orElseThrow(() -> new ResourceNotFound("Club(userId=" + userId + ")를 찾을 수 없습니다.")); }
Also applies to: 15-15
39-42
: 코드 포맷팅 이슈가 있습니다.42번 줄의 닫는 중괄호가 잘못된 위치에 있습니다. 또한 메서드의 트랜잭션 범위를 명시적으로 지정하는 것이 좋습니다.
다음과 같이 수정하는 것을 제안드립니다:
@Override + @Transactional(readOnly = true) public Club getByUserIdWithFetch(Long userId) { return clubRepository.findEntityGraphByUserId(userId) - .orElseThrow(() -> new ResourceNotFound("Club(userId=" + userId + ")를 찾을 수 없습니다.")); } + .orElseThrow(() -> new ResourceNotFound("Club(userId=" + userId + ")를 찾을 수 없습니다.")); + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/ddingdong/ddingdongBE/domain/club/repository/ClubRepository.java
(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/club/service/ClubService.java
(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/club/service/GeneralClubService.java
(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/clubmember/service/FacadeCentralClubMemberServiceImpl.java
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/ddingdong/ddingdongBE/domain/clubmember/service/FacadeCentralClubMemberServiceImpl.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and analyze
🔇 Additional comments (3)
src/main/java/ddingdong/ddingdongBE/domain/club/repository/ClubRepository.java (1)
5-5
: EntityGraph 임포트 추가가 적절합니다!필요한 기능을 위한 적절한 임포트 구문이 추가되었습니다.
src/main/java/ddingdong/ddingdongBE/domain/club/service/ClubService.java (1)
20-20
: 메서드 이름이 명확하고 적절합니다!
WithFetch
접미사를 통해 즉시 로딩 전략을 사용한다는 것을 명확히 표현하고 있습니다.src/main/java/ddingdong/ddingdongBE/domain/club/service/GeneralClubService.java (1)
41-42
: EntityGraph 사용에 대한 검증이 필요합니다.
findEntityGraphByUserId
메서드가 실제로 필요한 연관 엔티티들을 모두 페치하는지 확인이 필요합니다.✅ Verification successful
EntityGraph 설정이 적절하게 구성되어 있습니다.
현재 EntityGraph 설정은 다음과 같이 최적화되어 있습니다:
- Club의 clubMembers 컬렉션을 효율적으로 페치
- User와의 관계는 LAZY로 유지하여 성능 최적화
- ClubMember 엔티티의 추가적인 관계는 없어 현재 설정으로 충분
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Repository에서 EntityGraph 설정 확인 rg -A 5 "findEntityGraphByUserId" src/main/java/ddingdong/ddingdongBE/domain/club/repository/Length of output: 310
Script:
#!/bin/bash # Check for EntityGraph annotations and Club entity structure echo "=== Checking EntityGraph annotations ===" rg -B 3 -A 1 "@EntityGraph" src/main/java/ddingdong/ddingdongBE/domain/club/ echo -e "\n=== Checking Club entity structure ===" rg -B 1 -A 10 "class Club" src/main/java/ddingdong/ddingdongBE/domain/club/ echo -e "\n=== Checking custom repository implementations ===" fd "ClubRepository" src/main/java/ddingdong/ddingdongBE/domain/club/ --exec cat {}Length of output: 2712
Script:
#!/bin/bash # Check ClubMember entity structure and relationships echo "=== Checking ClubMember entity structure ===" rg -B 1 -A 10 "class ClubMember" src/main/java/ddingdong/ddingdongBE/domain/club/Length of output: 182
Script:
#!/bin/bash # Find ClubMember entity file echo "=== Finding ClubMember entity file ===" fd -e java "ClubMember" echo -e "\n=== If found, show content ===" fd -e java "ClubMember" --exec cat {}Length of output: 24886
(cherry picked from commit b5c3729)
🚀 작업 내용
🤔 고민했던 내용
--- Change!!!
리뷰받고 .size() 호출 방법 -> EntityGraph를 활용하여 초기화된 객체를 가져왔습니다
Summary by CodeRabbit
릴리즈 노트
개선 사항
기술적 변경
기타