-
Notifications
You must be signed in to change notification settings - Fork 3
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
음악 좋아요, 좋아요 삭제 기능 #423
음악 좋아요, 좋아요 삭제 기능 #423
Conversation
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.
+ 필수: 좋아요 추가, 삭제는 무조건 동시성 문제가 발생할 것 같은데 무조건 해결해야할 것 같아요. 여러가지 이점들을 조사해보시고 어떤 방법으로 동시성 이슈를 핸들링하도록 구현할 것인지 고민해보시고 말씀해주시면 좋겠어요 (db lock, 분산락, code level lock ex. mutex, syncronzied)
music.likeCount -= 1 | ||
musicRepository.save(music) | ||
} | ||
} |
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.
music likecount를 plus, minus하는 연산은 music 엔티티 내부 함수에서 처리하는게 좋을 것 같아요
그렇게 되면 likeCount에 대해 캡슐화도 되기 때문에 확장성의 이점과 더욱 객체지향적으로 동작할 것 같아요 plusLikeCount, minusLikeCount() 이렇게 두 개의 함수가 있으면 될 것 같네요
fun plusCount() {
this.likeCount++
}
musicRepository.save는 dirty checking
덕에 그냥 없애도 괜찮을 것 같고요
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.
val music: Music = musicRepository.findByIdOrNull(musicId) ?: throw MusicNotFoundException() | ||
val date = LocalDateTime.now() | ||
|
||
require(music.createdDate.dayOfWeek == date.dayOfWeek) {throw NotValidMusicLikeException()} |
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.
이 코드는 현재 당일날 신청된 노래들만 좋아요를 누를 수 있도록 valid하는 로직같네요. date관련한 util 클래스가 존재하는데 그 클래스에 현재 날짜인지 검증하는 로직을 넣어 util에서 함수 호출해서 예외를 throw하도록 리팩터링하면 괜찮을 것 같습니다.
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.
interface LikeRepository : JpaRepository<Like, Long> { | ||
@Lock(LockModeType.PESSIMISTIC_WRITE) | ||
@Query("select l from Like l where l.music = :music and l.member = :member") | ||
fun findByMemberAndMusic(@Param("member") member: Member, @Param("music") music: Music): Like? | ||
} |
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.
쿼리에서는 music, member순으로 비교하니까 파라미터 순서도 music member로 맞춰주면 좋을 것 같습니다
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.
Like에서 객체 대신 아이디를 가질 수 있게 수정하면서 함께 수정했습니다
src/main/kotlin/com/dotori/v2/domain/music/exception/NotValidMusicLikeException.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/com/dotori/v2/domain/music/service/impl/ToggleMusicLikeServiceImpl.kt
Outdated
Show resolved
Hide resolved
val member = userUtil.fetchCurrentUser() | ||
val music: Music = musicRepository.findByIdOrNull(musicId) ?: throw MusicNotFoundException() | ||
|
||
val like: Like? = likeRepository.findByMemberAndMusic(member, music) | ||
|
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.
변수명 뒤에 : Type 이건 빼줘도 괜찮을 것 같습니다.
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.
수정했습니다
@Id | ||
@GeneratedValue(strategy = GenerationType.IDENTITY) | ||
@Column(name = "like_id") | ||
val id: Long = 0, | ||
|
||
@ManyToOne(fetch = FetchType.LAZY) | ||
@JoinColumn(name = "music_id") | ||
val music: Music, | ||
|
||
@ManyToOne(fetch = FetchType.LAZY) | ||
@JoinColumn(name = "member_id") | ||
val member: Member | ||
) |
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.
엔티티 자체를 들고있는 것 보다는 그냥 단순하게 아이디만 들고 있는 것도 괜찮아 보이는데 어떻게 생각하세요? 자주 객체가 삭제되기도하고 생성되기도 하는 거니까
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.
수정했습니다
…MusicLikeServiceImpl.kt Co-authored-by: �Hope Kim <[email protected]>
Co-authored-by: �Hope Kim <[email protected]>
…usicLikeException.kt Co-authored-by: �Hope Kim <[email protected]>
) = Music(id, url, title, thumbnail, 0 ,member) | ||
} | ||
|
||
} |
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.
) = Music(id, url, title, thumbnail, 0 ,member) | |
} | |
} | |
) = Music(id, url, title, thumbnail, 0, member) | |
} | |
} |
…into 413-music-like-cancel-function # Conflicts: # src/main/kotlin/com/dotori/v2/domain/music/service/impl/ToggleMusicLikeServiceImpl.kt
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.
수고하셨습니다🙇♀️
💡 개요
음악 좋아요, 좋아요 삭제 기능을 구현했습니다
음악 신청한 날짜와 좋아요 누른 날짜가 다르면 에러를 던지도록 구현했습니다
📃 작업내용