-
Notifications
You must be signed in to change notification settings - Fork 2
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
컬럼 제약조건 및 DB 동기화를 위한 엔티티 클래스 수정 #806
Conversation
…to feat/801-social-normal-login
Walkthrough이 풀 리퀘스트는 여러 엔티티 클래스에 대한 JPA 및 유효성 검증 어노테이션을 개선하고 구조를 최적화하는 작업을 포함합니다. 주요 변경 사항은 Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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
CodeRabbit Configuration File (
|
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: 3
🔭 Outside diff range comments (3)
src/main/java/balancetalk/vote/domain/GameVote.java (1)
Line range hint
31-39
: 필수 관계에 대한 유효성 검증 추가 필요Member와 GameOption은 GameVote에서 필수적인 관계로 보입니다. 데이터 무결성을 보장하기 위해
@NotNull
제약조건을 추가하는 것이 좋습니다.다음과 같이 수정을 제안합니다:
@ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "member_id") + @NotNull private Member member; @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "game_option_id") + @NotNull private GameOption gameOption;src/main/java/balancetalk/bookmark/domain/TalkPickBookmark.java (1)
Line range hint
31-39
: 필수 관계에 대한 유효성 검증 추가 필요Member와 TalkPick 관계에
@NotNull
제약조건이 누락되어 있습니다.다음과 같이 수정을 제안합니다:
@ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "member_id") + @NotNull private Member member; @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "talk_pick_id") + @NotNull private TalkPick talkPick;src/main/java/balancetalk/talkpick/domain/TempTalkPick.java (1)
Line range hint
29-48
: 필수 필드에 대한 유효성 검증 추가 필요title, optionA, optionB와 같은 필수 필드에 대한
@NotNull
제약조건이 누락되어 있습니다. 또한 sourceUrl에 대한 URL 형식 검증이 필요해 보입니다.다음과 같이 수정을 제안합니다:
@OneToOne @JoinColumn(name = "member_id") + @NotNull private Member member; @Size(max = 50) + @NotNull private String title; @Column(columnDefinition = "LONGTEXT") + @NotNull private String content; @Size(max = 10) + @NotNull @Column(name = "option_a") private String optionA; @Size(max = 10) + @NotNull @Column(name = "option_b") private String optionB; + @Pattern(regexp = "^(https?|ftp)://[^\\s/$.?#].[^\\s]*$") private String sourceUrl;
🧹 Nitpick comments (7)
src/main/java/balancetalk/like/domain/Like.java (3)
34-37
: @column 어노테이션 추가를 고려해보세요.
likeType
필드에@NotNull
제약조건이 잘 추가되었습니다. 하지만 데이터베이스 컬럼 속성을 더 명확하게 지정하기 위해 다음과 같은 개선을 제안드립니다:@Enumerated(value = EnumType.STRING) @NotNull + @Column(length = 20) private LikeType likeType;
45-46
: primitive boolean 타입 사용을 고려해보세요.
@NotNull
이 지정된Boolean
래퍼 타입 대신 primitiveboolean
타입을 사용하면 다음과 같은 장점이 있습니다:
- null 가능성 제거
- 메모리 사용량 감소
- 박싱/언박싱 오버헤드 제거
@NotNull - private Boolean active = true; + private boolean active = true;
Line range hint
48-54
: 메서드 이름의 일관성을 개선해주세요.현재 메서드 이름이 일관성이 없습니다:
activate()
deActive()
(대문자 'A' 사용)다음과 같이 수정하는 것을 추천드립니다:
public void activate() { this.active = true; } - public void deActive() { + public void deactivate() { this.active = false; }src/main/java/balancetalk/bookmark/domain/TalkPickBookmark.java (1)
Line range hint
44-57
: 상태 변경 메서드에 대한 문서화 필요
activate()
,deactivate()
메서드의 용도와 영향을 명확히 하기 위해 JavaDoc 문서화를 추가하는 것이 좋습니다.다음과 같이 수정을 제안합니다:
+ /** + * 북마크를 활성화 상태로 변경합니다. + */ public void activate() { this.active = true; } + /** + * 북마크를 비활성화 상태로 변경합니다. + */ public void deactivate() { this.active = false; }src/main/java/balancetalk/talkpick/domain/TempTalkPick.java (1)
Line range hint
50-58
: update 메서드 개선 필요update 메서드에서 반환값의 의도가 명확하지 않습니다. 또한 유효성 검증이 누락되어 있습니다.
다음과 같이 수정을 제안합니다:
+ /** + * 임시 토크픽의 내용을 업데이트합니다. + * @param newTempTalkPick 새로운 임시 토크픽 정보 + * @throws IllegalArgumentException 필수 필드가 null인 경우 + */ public Long update(TempTalkPick newTempTalkPick) { + Objects.requireNonNull(newTempTalkPick, "newTempTalkPick must not be null"); + validateRequiredFields(newTempTalkPick); this.title = newTempTalkPick.getTitle(); this.content = newTempTalkPick.getContent(); this.optionA = newTempTalkPick.getOptionA(); this.optionB = newTempTalkPick.getOptionB(); this.sourceUrl = newTempTalkPick.getSourceUrl(); return id; } + private void validateRequiredFields(TempTalkPick tempTalkPick) { + if (tempTalkPick.getTitle() == null || tempTalkPick.getContent() == null || + tempTalkPick.getOptionA() == null || tempTalkPick.getOptionB() == null) { + throw new IllegalArgumentException("Required fields must not be null"); + } + }src/main/java/balancetalk/game/domain/GameOption.java (2)
Line range hint
77-84
: 투표 수 관리 메서드의 동시성 이슈를 고려해주세요.
increaseVotesCount
와decreaseVotesCount
메서드에서 동시성 문제가 발생할 수 있습니다. JPA의 낙관적 락이나 비관적 락 도입을 고려해보세요.다음과 같은 개선을 제안합니다:
+ @Version + private Long version; - public void increaseVotesCount() { + @Lock(LockModeType.OPTIMISTIC) + public synchronized void increaseVotesCount() { this.votesCount++; } - public void decreaseVotesCount() { + @Lock(LockModeType.OPTIMISTIC) + public synchronized void decreaseVotesCount() { this.votesCount--; }
Line range hint
86-88
: 이미지 ID 검증 로직을 개선해주세요.
hasImage
메서드가 단순히 null 체크만 수행하고 있습니다. 유효한 이미지 ID 범위 검증도 추가하는 것이 좋을 것 같습니다.- public boolean hasImage() { - return imgId != null; + public boolean hasImage() { + return imgId != null && imgId > 0; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/java/balancetalk/bookmark/domain/GameBookmark.java
(0 hunks)src/main/java/balancetalk/bookmark/domain/TalkPickBookmark.java
(1 hunks)src/main/java/balancetalk/comment/domain/Comment.java
(3 hunks)src/main/java/balancetalk/file/domain/File.java
(2 hunks)src/main/java/balancetalk/game/domain/GameOption.java
(2 hunks)src/main/java/balancetalk/like/domain/Like.java
(3 hunks)src/main/java/balancetalk/talkpick/domain/TalkPick.java
(1 hunks)src/main/java/balancetalk/talkpick/domain/TempTalkPick.java
(1 hunks)src/main/java/balancetalk/vote/domain/GameVote.java
(1 hunks)src/main/java/balancetalk/vote/domain/TalkPickVote.java
(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/balancetalk/bookmark/domain/GameBookmark.java
🔇 Additional comments (10)
src/main/java/balancetalk/like/domain/Like.java (1)
5-20
: 임포트 구문이 깔끔하게 정리되어 있습니다!
Jakarta Persistence, 유효성 검증, Lombok 관련 임포트가 잘 구성되어 있습니다.
src/main/java/balancetalk/file/domain/File.java (2)
15-15
: 코드가 적절해 보입니다!
새로운 필드 제약조건을 위한 Size 어노테이션 import가 올바르게 추가되었습니다.
36-45
: 데이터베이스 스키마와의 정합성 검증이 필요합니다.
필드에 추가된 제약조건들이 논리적으로는 적절해 보입니다:
- fileType: @NotNull 제약조건 추가
- uploadName: 최대 50자 제한
- storedName: 최대 100자 제한
하지만 이러한 제약조건들이 실제 데이터베이스 스키마와 일치하는지 확인이 필요합니다.
추가로 고려해볼 사항:
- 파일 이름 길이 제한이 운영체제나 파일 시스템의 제약과도 호환되는지 확인
- 향후 확장성을 고려했을 때 현재 설정된 길이 제한이 충분한지 검토
src/main/java/balancetalk/vote/domain/TalkPickVote.java (3)
6-14
: JPA 어노테이션 임포트 구성이 적절합니다.
필요한 모든 JPA 어노테이션들이 명시적으로 임포트되어 있으며, 불필요한 임포트는 없습니다.
Line range hint 33-40
: 연관관계의 Fetch 전략이 적절하게 설정되었습니다.
Member
와 TalkPick
에 대한 @ManyToOne
관계에서 FetchType.LAZY
를 사용한 것이 성능 최적화 측면에서 좋은 선택입니다.
Line range hint 42-44
: VoteOption 필드의 제약조건을 검토해주세요.
@NotNull
제약조건이 추가되었지만, 이 필드가 업데이트되는 updateVoteOption
메서드에서는 null 체크가 없습니다.
다음 스크립트로 updateVoteOption
메서드의 호출 부분을 확인해보세요:
src/main/java/balancetalk/game/domain/GameOption.java (2)
34-34
: BaseTimeEntity 상속 추가는 적절합니다.
생성일시와 수정일시를 자동으로 관리할 수 있게 되어 감사 추적성이 향상되었습니다.
Line range hint 41-44
: name 필드의 유효성 검증이 개선되었습니다.
@NotBlank
와 @Size(max = 30)
조합으로 적절한 제약조건이 설정되었습니다.
src/main/java/balancetalk/talkpick/domain/TalkPick.java (2)
Line range hint 79-81
: comments
필드의 CascadeType.ALL
사용 주의
comments
필드에 CascadeType.ALL
을 사용하면 TalkPick
엔티티 삭제 시 연관된 Comment
엔티티도 모두 삭제됩니다. 댓글 데이터를 보존해야 하는 비즈니스 로직이라면 cascade
속성을 재검토하는 것이 좋습니다.
Line range hint 75-77
: votes
필드의 CascadeType.REMOVE
사용 확인
votes
필드에 CascadeType.REMOVE
를 사용하여 TalkPick
엔티티 삭제 시 연관된 TalkPickVote
엔티티가 삭제됩니다. 투표 기록의 보존이 필요한지 확인하고, 필요하다면 cascade
설정을 조정해주세요.
Quality Gate passedIssues Measures |
💡 작업 내용
✅ 셀프 체크리스트
closes #805
Summary by CodeRabbit
신규 기능
GameOption
및TempTalkPick
클래스가BaseTimeEntity
를 상속받아 기능이 확장되었습니다.TalkPick
클래스에@Embedded
필드가 추가되어 복합 타입으로 저장됩니다.Like
클래스에active
필드의 기본값이true
로 설정되었습니다.변경 사항
Comment
클래스에서reportedCount
필드와 관련된 로직이 제거되었습니다.TalkPickVote
클래스의voteOption
필드에 대한 주석이 명확히 정의되었습니다.ReportCommentService
클래스에서 오류 처리 로직이 개선되었습니다.CustomSuccessHandler
클래스에서 변수 선언 위치가 변경되었습니다.