전수빈 seminar1 과제#56
Conversation
| @GetMapping("/api/v1/playlist-groups") | ||
| fun getPlaylistGroup(): PlaylistGroupsResponse { | ||
| TODO() | ||
| //TODO() |
| @ManyToOne // default FetchType.EAGER | ||
| @JoinColumn(name = "playlist_id") | ||
| val playlist: PlaylistEntity, | ||
|
|
||
| @ManyToOne // default FetchType.EAGER | ||
| @JoinColumn(name = "song_id") | ||
| val song: SongEntity, |
There was a problem hiding this comment.
어떠한 경우의 연관관계에도 Lazy 를 지향하는 것이 좋습니다. ManyToOne 의 default 가 EAGER 라 하더라도 이것을 변경해두는 것이 좋아요. wafflestudio/seminar-2023#169 를 참고하시면 좋습니다.
이걸 Lazy 로 해두었다면, 불필요한 N+1 쿼리가 덜 날아가서 곳곳에서 JOIN FETCH 를 덜 써도 되었을 수도 있어요.
| @ManyToOne // default FetchType.EAGER | ||
| @JoinColumn(name = "song_id") | ||
| val song: SongEntity, | ||
| ) No newline at end of file |
There was a problem hiding this comment.
git 에서 파일의 마지막 newline 을 넣는 것이 좋은 습관입니다.
| @Query("SELECT p FROM playlists p JOIN FETCH p.playlistGroup pg JOIN FETCH p.playlistSong ps JOIN FETCH ps.song s JOIN FETCH s.album a JOIN FETCH a.artist WHERE p.id = :id") | ||
| fun findPlaylistEntityById(id: Long): PlaylistEntity? |
There was a problem hiding this comment.
예를 들어 service get() 에서는 쿼리를 playlistSongRepository.findSongsInPlaylist() 과 함께 나눠서 실행하는데, 여기서 불필요하게 너무 많이 join 해서 가져오는 듯한 느낌이 있네요. 이거 혹시 연관관계 FetchType 을 EAGER 로 해둬서 발생하는 N+1 때문이었다면, 다른 코멘트와 조합해서 생각해보시면 좋을 듯해요.
| import org.springframework.data.jpa.repository.Query | ||
|
|
||
| interface PlaylistLikeRepository : JpaRepository<PlaylistLikeEntity, Long> { | ||
| //@Query("") |
| //TODO() | ||
| var i = 400 | ||
| if (e is SignUpUsernameConflictException) { | ||
| i = 409 | ||
| } else if (e is SignInUserNotFoundException || e is SignInInvalidPasswordException) { | ||
| i = 404 | ||
| } else if (e is AuthenticateException) { | ||
| i = 401 | ||
| } | ||
| return ResponseEntity.status(i).build() |
There was a problem hiding this comment.
가변인 var 사용을 가급적 지양하면 좋습니다. when 을 사용하여 더욱 깔끔한 코드를 만들 수 있을 거 같습니다.
| private val songRepository: SongRepository, | ||
| private val albumRepository: AlbumRepository |
There was a problem hiding this comment.
ktlint 를 참고하여 일관된 indent 를 하면 좋겠네요
| return playlistGroupList.map { PlaylistGroup( | ||
| id = it.id, | ||
| title = it.title, | ||
| playlists = it.playlist.map { PlaylistBrief(it.id, it.title, it.subtitle, it.image) } |
There was a problem hiding this comment.
IDE 도 알려줬을 거 같은데, map { } 바깥과 내부에서 it 이 가리키는 것이 달라서 혼동을 줄 수 있습니다. 이런 경우 map { playlist -> ... }처럼 한 쪽의 이름을 명시적으로 주는 게 좋습니다. IDE 의 경고는 무시하지 않는 게 좋아요.
| TODO() | ||
| //TODO() | ||
| val playlistLike = playlistLikeRepository.findPlaylistLikeEntityByPlaylistId(playlistId).filter { it.user.id == userId } | ||
| return !playlistLike.isEmpty() |
| TODO() | ||
| //TODO() | ||
| val playlist = playlistRepository.findPlaylistEntityById(playlistId) ?: throw PlaylistNotFoundException() | ||
| val playlistLike = playlistLikeRepository.findPlaylistLikeEntityByPlaylistId(playlistId).filter { it.user.id == userId } |
There was a problem hiding this comment.
이거 그냥 쿼리로 userId 같은 걸 찾도록, findByPlaylistIdAndUserId()를 만들면 될 거 같은데요? 만약에 이 playlist 를 like 한 유저가 너무 많다면, 서버 내부로 다 가져와서 filtering 하는 것에 성능 이슈가 있을 수 있습니다.
No description provided.