[리팩토링] Post, Service 코드 개선 및 테스트 코드 추가 #129
Conversation
| @@ -70,25 +70,30 @@ public Post findByPostIdx(Long postIdx){ | |||
| @Transactional(readOnly = true) | |||
| public ApiPagingResultResponse<PostDto.ListResponse> getPostsListView(Long cursorId, Pageable page, Long userIdx){ | |||
| // 마지막으로 검색된 회고글의 조회수 | |||
| List<PostDto.ListResponse> result = getPostsView(cursorId, page).stream().map(post->postMapper.postToListResponse(post, userIdx)) | |||
| List<Post> posts = getPostsView(cursorId, page); | |||
| if (posts.isEmpty()) throw new EntityNullException(ErrorInfo.CONDITION_NULL); | |||
There was a problem hiding this comment.
회고글 존재하지 않을 땐 예외 날리는 것보단 빈 배열 200으로 응답하는게 낫지 않을까?
There was a problem hiding this comment.
그 두 개 방법이랑 고민해봤는데
- return 값이 현재 postDto.ListResponse
- mapstruct로 바꾸려면 배열에 값이 필요함
등등의 이유로 일단 exception 처리는 해놨는데, ApiPagingResult가 Controller 쪽으로 빼서 변환하기 어렵더라구 함 다시 시도해볼게!
There was a problem hiding this comment.
CommentController.getCommentsByPostIdx 한번 보는거 추천~
| Post post = postRepository.findById(postIdx) | ||
| .orElseThrow(() -> new EntityNullException(ErrorInfo.POST_NULL)); | ||
| if (isWriter(post.getUser().getUserIdx(), userIdx)){ | ||
| if (!isWriter(post.getUser().getUserIdx(), userIdx)) throw new EntityNullException(ErrorInfo.POST_NULL); |
There was a problem hiding this comment.
여기는 AccessDeniedException을 던지는게 더 적절할 듯
| @@ -89,4 +89,9 @@ public User findByUserEmail(String email){ | |||
| return userRepository.findByEmail(email) | |||
| .orElseThrow(() -> new EntityNullException(ErrorInfo.USER_NULL)); | |||
| } | |||
|
|
|||
|
|
|||
| public Long getUserInfoFromToken(User user){ | |||
There was a problem hiding this comment.
@currentuser는 @AuthenticationPrincipal을 기반으로 만든건데 스프링 시큐리티에서 인증된 사용자 정보는 CustomUserDetailsService.loadUserByUsername으로 DB에서 Entity를 가져오게 되어있거든.
따라서 인증에 실패하면 UnAuthorized로 응답하게 되서 User 객체는 null 체크를 해줄 필요가 없는데 이 함수의 용도를 잘 모르겠어.
잘 이해가 안된다면 https://ncucu.me/137, https://jjeda.tistory.com/7를 참고해봐!
There was a problem hiding this comment.
옹 @currentuser 보니까 @AuthenticationPrincipal(expression = "#this == 'anonymousUser' ? null : user") 이 부분에서
anonymousUser이면 nulㅣ이 반환되고 아니면 user이 반환되는건가? 싶어서 처리해놨어 어제 물어보고싶었던게 이부분ㅠ
null check가 필요없다면 수정해놓겟습니더!
There was a problem hiding this comment.
익명 사용자 접근은 어차피 권한에 따라 SecurityConfig filter에서 걸러지기 때문에 null 신경 안써도 될 꺼같아
| User user = userRepository.findById(1L) | ||
| .orElseThrow(() -> new EntityNullException(ErrorInfo.USER_NULL)); | ||
|
|
||
| assertThat(user.getUserIdx()).isEqualTo(expected.getUserIdx()); |
There was a problem hiding this comment.
import static org.junit.jupiter.api.Assertions.assertEquals;
assertEquals(user.getUserIdx(), expected.getUserIdx());
-> 요거 쓰면 훨씬 간결하고 깔끔하게 비교할 수 있음
만약 boolean 값을 비교하고 싶다면 assertTrue(예상값); 이런 식으로도 활용 가능!
There was a problem hiding this comment.
코드가 짧아지는것도 좋지만 기댓값, 예상값이 뚜렷하게 보여서 실수도 미연에 방지할 수 있고 가독성이 더 높은 Aseertj(AssertThat)를 이용함!
There was a problem hiding this comment.
오호 나는 오히려 assertEquals가 가독성이 좋다고 생각했는데 그런 차이가 있었구먼👍
There was a problem hiding this comment.
https://itcoin.tistory.com/500
대충 이러이러한 장점이 있다네여
예외 사항
EntityNullException으로 처리@CurrentUser를 통해 이미 검증된 user 객체에서 가져오도록 리팩토링