#57 북마크 이동하기 API구현 및 CRUD API테스트 #74
Conversation
| if(!l.equals(userId)) { | ||
| throw new Exception403((BookmarkExceptionStatus.BOOKMARK_FORBIDDEN)); | ||
| } | ||
|
|
||
| Bookmark bookmark = bookmarkJPARepository.findById(l).orElseThrow( | ||
| () -> new Exception404(BookmarkExceptionStatus.BOOKMARK_NOT_FOUND) | ||
| ); |
There was a problem hiding this comment.
변수명 l보단 Long bookmarkId가 낫지않을까요??
또한 현재 코드에서 if(!l.equals(userId)) 조건인데 북마크 Id랑 userId를 비교하고 있는 문제가 있는 것 같습니다.
추가로 하나의 for문안에 처리하는 것 보단 validateUser와 같은 메소드를 만들어 처리하는 것도 코드 가독성 상 좋아보입니다.
그외에도 bookmark를 하나하나 findById하기보단 In절로 가져오는 것이 효율상 좋아보입니다. 또한 Bookmark 작성자 (userId)를 가져오기 위해서는 현재 엔티티 구조 상, bookmark -> category -> workspace -> userId 순으로 fetch join을 해야하는 상황인 것 같습니다. (join을 많이 하는게 좋은지는 잘 모르겠습니다.)
대충 data jpa로 나타내자면
@Query("select b from Bookmark b " +
"join fetch b.category c " +
"join fetch c.workspace w " +
"where b.bookmarkId in :bookmarkIds")
List<Bookmark> findByIdsFetchJoinCategoryAndWorkspace(@Param("bookmarkIds") List<Long> bookmarkIds);가 되겠네요.
이렇게 되면 Optional이 아니니 .orElseThrow()를 사용할 수 없지만, Dto에서 받아온 BookmarkList와 디비에서 가져온 북마크 List를 각각 Set으로 치환, 차집합으로 나타내봤을 때 데이터가 존재한다면, 예외를 발생시키는 방식으로 로직을 수정하는 것도 좋아보입니다.
There was a problem hiding this comment.
위에 숫자 1은 뭔가요?.. 하드코딩이 있어야하는겁니까?
숫자 1이 아니라 변수명을 영문자 'l'로 설정한 거였습니다. 현재 진행 상태에서는 변수명은 수정된 상태입니다.
- 북마크id와 사용자id를 비교하는 버그 수정 - BookmarkRequestDto.bookmarkAddDTO 에서 카테고리id의 @notempty 어노테이션을 제거하였음
# Conflicts: # linknamu/src/main/java/com/kakao/linknamu/bookmark/repository/BookmarkJPARepository.java
수정 사항