Skip to content

이한나 seminar1 과제#59

Open
onemeee wants to merge 2 commits intowafflestudio:master-week1from
onemeee:master-week1
Open

이한나 seminar1 과제#59
onemeee wants to merge 2 commits intowafflestudio:master-week1from
onemeee:master-week1

Conversation

@onemeee
Copy link

@onemeee onemeee commented Oct 2, 2023

No description provided.

Copy link
Member

@davin111 davin111 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc. @PFCJeong

@onemeee 테스트 하나가 실패하고 있습니다. 확인 부탁드립니다.

Comment on lines +19 to +21
@ManyToOne
@JoinColumn(name = "album_id")
val album: AlbumEntity,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

어떠한 경우의 연관관계에도 LAZY 를 지향하는 것이 좋습니다. ManyToOne 의 default 가 EAGER 라 하더라도 이것을 변경해두는 것이 좋아요. wafflestudio/seminar-2023#169 를 참고하시면 좋습니다.

Comment on lines +10 to +11
@Query("SELECT s FROM songs s LEFT JOIN FETCH s.album a LEFT JOIN FETCH s.songArtists sa LEFT JOIN FETCH sa.artist aa WHERE s.title LIKE %:keyword% ORDER BY LENGTH(s.title)")
fun searchWithJoinFetch(keyword: String) : List<SongEntity>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

쿼리로 DB 에서 length() 로 order 하는 것보단, 일단 application (서버) 에 가져와서 코드로 sort 하는 게 성능상 더 나은 선택일 것 같습니다.

Comment on lines +41 to +60
val playlist = playlistRepository.findByIdWithJoinFetch(id)
?: throw PlaylistNotFoundException()
val songEntities = songRepository.findBySongIdWithJoinFetch(
playlist.playlistSongs.map { it.song.id }
)
val songs = songEntities.map { songEntity ->
Song(
id = songEntity.id,
title = songEntity.title,
album = songEntity.album.title,
image = songEntity.album.image,
duration = songEntity.duration.toString(),
artists = songEntity.songArtists.map {
Artist(
id = it.artist.id,
name = it.artist.name
)
}
)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 메서드와 관련해, 테스트 플레이리스트 조회, 쿼리 횟수는 2회로 제한가 실패하네요. N+1 문제가 발생하고 있는 것 같습니다.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davin111 해결했습니다.
새로운 commit 추가하였습니다.
감사합니다!!

fun existsByPlaylistIdAndUserId(playlistId: Long, userId: Long) : Boolean

// 플레이 리스트 좋아요 취소
@Transactional
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Transactional은 불필요한 것 같네요.

import org.springframework.data.jpa.repository.Query

interface PlaylistGroupRepository: JpaRepository<PlaylistGroupEntity, Long> {
@Query("SELECT g FROM playlist_groups g LEFT JOIN FETCH g.playlists p WHERE SIZE(p) > 0 and g.open = TRUE")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JPQL 로 SIZE(p) > 0를 하면 실제 쿼리가 복잡해질 수도 있어서, 일단 가져와서 서버 코드로 playlists 가 isNotEmpty()인 것만 filter 하는 것도 좋을 것 같습니다.

Comment on lines +34 to +39
val isLiked = when (user) {
// 미로그인 상태
null -> false
// 로그인 상태
else -> playlistLikeService.exists(id, user.id)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요기는 굳이 when 을 써서 얻는 이점이 있을까? 싶기도 하네요. 그냥 if 문으로 써도 어쩌면 더 간결할지도?

@onemeee
Copy link
Author

onemeee commented Oct 22, 2023

@davin111 @PFCJeong
이제 제출 approve가 된 걸까요?

@PFCJeong
Copy link
Member

@onemeee 안녕하세요~ 예 확인되었습니다 수고하셨습니다

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants