Verdict 도메인 N+1 문제 해결 및 리팩터링#79
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough이번 PR은 소비 심판 기능 리팩터링을 포함합니다. User.hashCode()가 id 기반(Objects.hash(getId()))으로 변경되어 equals와 일관되게 되었고, Verdict 엔티티가 BaseEntity를 상속하도록 선언이 변경되었습니다. VerdictRepository의 두 쿼리에 엔티티를 즉시 로드하기 위한 JOIN FETCH들이 추가되었고, VerdictController의 요청 바디 파라미터에 Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 리뷰 시 주목할 점안내: 아래 항목들을 중심으로 검토하세요 — 친절하고 구체적으로 문제를 찾는 연습을 합시다.
필요하면 위 항목 중 우려되는 부분을 지정해서 작은 수정 PR로 분리해 주세요. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이번 PR은 Verdict 도메인의 N+1 문제를 해결하고, hashCode 구현을 개선하며, 유효성 검사를 추가하는 등 여러 유용한 수정을 포함하고 있습니다. 전반적으로 코드 품질을 높이는 좋은 변경이라고 생각합니다.
다만, N+1 문제 해결을 위해 추가한 JOIN FETCH 구문이 일부 케이스에서 모든 연관 관계를 포함하지 않아 추가적인 개선이 필요해 보입니다. 자세한 내용은 VerdictRepository.java 파일에 남긴 리뷰 코멘트를 참고해주세요.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/example/demo/domain/User.java (1)
73-84:⚠️ Potential issue | 🟡 Minor
equals()와hashCode()일관성 개선 - 좋은 수정입니다!기존
getClass().hashCode()에서Objects.hash(getId())로 변경하여equals()와 일관성을 맞추셨네요.다만, JPA 엔티티에서
id기반hashCode()는 잠재적인 문제가 있습니다:
- 새 엔티티(
id == null)가HashSet에 추가된 후 영속화되면id가 할당되어hashCode()가 변경됩니다.- 이는
hashCode()계약 위반으로, 컬렉션에서 해당 객체를 찾지 못하는 버그가 발생할 수 있습니다.Hibernate 팀에서 권장하는 패턴은 상수 hashCode를 반환하는 것입니다:
💡 안정적인 hashCode 패턴 제안
`@Override` public int hashCode() { - return Objects.hash(getId()); + // JPA 엔티티의 안정적인 hashCode - id 변경에 영향받지 않음 + return getClass().hashCode(); }또는 현재 구현을 유지하되, 새 엔티티를
HashSet/HashMap에 추가하기 전에 반드시 영속화하는 규칙을 팀 내에서 합의하시면 됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/example/demo/domain/User.java` around lines 73 - 84, The current User.equals(Object) compares by id but hashCode() uses Objects.hash(getId()), which changes when a transient entity is persisted; update the User.hashCode() implementation so it returns a stable value for newly created entities (e.g., return a fixed constant) or return Objects.hash(getId()) only when getId() != null and a constant otherwise, ensuring the User.equals and User.hashCode methods remain consistent for JPA entities.
🧹 Nitpick comments (1)
src/main/java/com/example/demo/infrastructure/controller/dto/RequestJudgeWebRequest.java (1)
5-8:@NotNull에 커스텀 메시지 추가를 권장합니다.
VerdictJudgeWebRequest의verdictType에는@NotNull(message = "심판 유형(type)은 필수입니다.")처럼 명확한 메시지가 있는데, 이 DTO에는 기본 메시지만 사용됩니다.API 사용자 경험과 팀 내 일관성을 위해 커스텀 메시지 추가를 고려해 보세요.
♻️ 커스텀 메시지 추가 제안
public record RequestJudgeWebRequest( - `@NotNull` + `@NotNull`(message = "가계부 항목 ID(ledgerEntryId)는 필수입니다.") Long ledgerEntryId ) { }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/example/demo/infrastructure/controller/dto/RequestJudgeWebRequest.java` around lines 5 - 8, RequestJudgeWebRequest 레코드의 ledgerEntryId 필드에 기본 `@NotNull` 메시지 대신 일관된 커스텀 메시지를 추가하세요; RequestJudgeWebRequest 레코드 선언의 Long ledgerEntryId에 `@NotNull`(message = "원장 항목 ID(ledgerEntryId)는 필수입니다.")와 같이 명확한 한국어 메시지를 적용해 VerdictJudgeWebRequest의 verdictType과 동일한 스타일로 사용자 친화적 오류 메시지를 제공하도록 수정하세요.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/main/java/com/example/demo/domain/User.java`:
- Around line 73-84: The current User.equals(Object) compares by id but
hashCode() uses Objects.hash(getId()), which changes when a transient entity is
persisted; update the User.hashCode() implementation so it returns a stable
value for newly created entities (e.g., return a fixed constant) or return
Objects.hash(getId()) only when getId() != null and a constant otherwise,
ensuring the User.equals and User.hashCode methods remain consistent for JPA
entities.
---
Nitpick comments:
In
`@src/main/java/com/example/demo/infrastructure/controller/dto/RequestJudgeWebRequest.java`:
- Around line 5-8: RequestJudgeWebRequest 레코드의 ledgerEntryId 필드에 기본 `@NotNull` 메시지
대신 일관된 커스텀 메시지를 추가하세요; RequestJudgeWebRequest 레코드 선언의 Long ledgerEntryId에
`@NotNull`(message = "원장 항목 ID(ledgerEntryId)는 필수입니다.")와 같이 명확한 한국어 메시지를 적용해
VerdictJudgeWebRequest의 verdictType과 동일한 스타일로 사용자 친화적 오류 메시지를 제공하도록 수정하세요.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 15ad8459-e6a3-4d66-913b-d3903519792d
📒 Files selected for processing (8)
src/main/java/com/example/demo/domain/User.javasrc/main/java/com/example/demo/domain/Verdict.javasrc/main/java/com/example/demo/domain/VerdictRepository.javasrc/main/java/com/example/demo/infrastructure/controller/VerdictController.javasrc/main/java/com/example/demo/infrastructure/controller/dto/JurorVerdictWebResponse.javasrc/main/java/com/example/demo/infrastructure/controller/dto/JurorVerdictsWebResponse.javasrc/main/java/com/example/demo/infrastructure/controller/dto/RequestJudgeWebRequest.javasrc/main/java/com/example/demo/infrastructure/controller/dto/VerdictJudgeWebRequest.java
💤 Files with no reviewable changes (2)
- src/main/java/com/example/demo/infrastructure/controller/dto/JurorVerdictWebResponse.java
- src/main/java/com/example/demo/infrastructure/controller/dto/JurorVerdictsWebResponse.java
|
|
||
| Verdict save(Verdict verdict); | ||
|
|
||
| Optional<Verdict> findById(Long verdictId); |
There was a problem hiding this comment.
VerdictService.getVerdict() 에서 findById로 엔티티를 조회한 후, Verdict 내부 User, LedgerEntry에 접근하여 여전히 N+1 문제가 발생합니다. findById를 다음과 같이 수정하면 성능이 더 개선될 것 같습니다!
| @Query(""" | |
| SELECT v FROM Verdict v | |
| JOIN FETCH v.ledgerEntry le | |
| JOIN FETCH le.user | |
| WHERE v.id = :verdictId | |
| """) | |
| Optional<Verdict> findByIdWithLedgerEntryAndUser(@Param("verdictId") Long verdictId); |
| import jakarta.validation.constraints.NotNull; | ||
|
|
||
| public record RequestJudgeWebRequest( | ||
| @NotNull |
There was a problem hiding this comment.
이 어노테이션에도 오류 메시지가 있으면 좋을 것 같습니다
2 participants
이슈
요약
n+1 문제를 해결합니다.
이외에도 소소한 수정들이 있습니다.
그리고 제안해주신
saveAll()메서드의 경우 JPA의Repository인터페이스를 상속하고 있으면, saveAll을 자동으로 생성해주지 않더라고요!또한, saveAll 내부 구조가 결국 여러번 save를 호출하는 형태라 크게 다르지 않을 거 같다고 판단해서 그대로 두었습니다.
Summary by CodeRabbit
출시 노트
버그 수정
새로운 기능 / 유효성 검사
성능 개선