[4기 원건희 & 박상민] Springboot-board 1차 PR 입니다. #225
Conversation
src/main/java/com/jpaboard/domain/post/application/PostServiceImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/com/jpaboard/domain/post/infrastructure/PostRepository.java
Outdated
Show resolved
Hide resolved
| @Query("select p from Post p where p.title like %:title% or p.content like %:content% or p.title like %:keyword% or p.content like %:keyword%") | ||
| Page<Post> findAllByCondition(String title, String content, String keyword, Pageable pageable); |
There was a problem hiding this comment.
음,,, 지금은 어쩔 수 없지만 이런 쿼리는 굉장히 위험할 수 있습니다! 왜 일까요?
There was a problem hiding this comment.
쿼리에 의존적인 코드이기 때문인 것 같습니다! 요구사항이 변경된다면 쿼리도 수정해줘야 하고 로직도 변경해줘야 하며, 입력하다가 실수하기도 좋은 코드이기 때문이기도 한 것 같습니다.
추가로 굉장히 위험하다고 하시니 뭔가 보안적인 이유가 있을 것 같은데 혹시 Sql Injection 문제일까요?
만약 이러한 문제가 맞다면 멘토님의 설명을 들어보고 싶습니다!
| return ResponseEntity.ok(userService.findUserById(id)); | ||
| } | ||
|
|
||
| @PutMapping("/{id}") |
There was a problem hiding this comment.
PUT Method은 해당 리소스가 없을 경우 어떻게 동작하나요?
src/test/java/com/jpaboard/domain/post/application/PostServiceImplTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/jpaboard/domain/user/application/UserServiceImplTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/jpaboard/domain/post/application/PostServiceImplTest.java
Outdated
Show resolved
Hide resolved
| PostPageResponse findPosts(Pageable pageable); | ||
|
|
||
| PostPageResponse findPostsByCondition(PostSearchRequest request, Pageable pageable); |
There was a problem hiding this comment.
PostSearchRequest의 전체 속성이 비어있다면 두 메서드는 같은게 아닌가요?
There was a problem hiding this comment.
전체 속성이 비어있으면 아무런 결과를 반환하지 않기에 분리를 하게 되었습니다.. 좀 더 나은 JPQL 쿼리문을 작성해야 하는데 방법을 잘 모르겠습니다
There was a problem hiding this comment.
전체 속성이 null인 경우에 where 구문을 spring data jpa에서 어떻게 생성하는지 확인해볼 필요가 있겠네요.
다이나믹 쿼리를 작성하는 예시가 나와 있어서 적용해봐도 좋을 것 같네요.
https://tecoble.techcourse.co.kr/post/2022-10-11-jpa-dynamic-query/
src/test/java/com/jpaboard/domain/post/application/PostServiceImplTest.java
Outdated
Show resolved
Hide resolved
src/main/java/com/jpaboard/domain/post/dto/response/PostResponse.java
Outdated
Show resolved
Hide resolved
| @Query("select p from Post p where p.title like %:title% or p.content like %:content% or p.title like %:keyword% or p.content like %:keyword%") | ||
| Page<Post> findAllByCondition(String title, String content, String keyword, Pageable pageable); |
| return ResponseEntity.ok(userService.findUserById(id)); | ||
| } | ||
|
|
||
| @PutMapping("/{id}") |
| userServiceImpl.updateUser(user.getId(), request); | ||
|
|
||
| // then | ||
| verify(userRepository, atMostOnce()).findById(user.getId()); |
| @GetMapping | ||
| public ResponseEntity<PostPageResponse> postFindAll(@PageableDefault Pageable pageable) { | ||
| PostPageResponse response = postService.findPosts(pageable); | ||
| return ResponseEntity.ok(response); | ||
| } | ||
|
|
||
| @GetMapping("/search") | ||
| public ResponseEntity<PostPageResponse> findByCondition( | ||
| @ModelAttribute PostSearchRequest request, | ||
| @PageableDefault Pageable pageable | ||
| ){ | ||
| PostPageResponse response = postService.findPostsByCondition(request, pageable); | ||
| return ResponseEntity.ok(response); | ||
| } |
| import lombok.Getter; | ||
|
|
||
| @Getter | ||
| public enum ErrorCodeWithDetail { |
| BAD_REQUEST(400, "C000", "잘못된 요청입니다."), | ||
| ENTITY_NOT_FOUND(404, "C001", "해당 엔티티를 찾을 수 없습니다."), | ||
| INTERNAL_SERVER_ERROR(500, "C999", "서버 내부 에러입니다."); |
There was a problem hiding this comment.
C000같은 code를 따로 만드신 이유가 있을까요?
어떤 유형의 에러인지 구분하기 위해서라면 조금더 가독성있게 만들어주는 편이 좋을 것 같아요.
아니면 어떤 에러인지 문서화를 해주는 것이 좋겠네요.
| @Mock | ||
| private UserRepository userRepository; | ||
|
|
||
| Random random = new Random(); |
There was a problem hiding this comment.
난수를 생성할 때 Random은 잘 사용하지 않는 것 같아요. 여러가지 방법이 있긴한데 java17에 RandomGenerator라는 것이 있더라구요.
| given(postRepository.findById(postId)).willReturn(Optional.of(post)); | ||
|
|
||
| //when | ||
| postService.updatePost(postId, request); | ||
|
|
||
| //then | ||
| verify(post, atMostOnce()).update(request.title(), request.content()); |
There was a problem hiding this comment.
별건 아니지만 BDDMockito.given과 Mockito.verity를 혼용해서 사용하고 계셔서 차이를 알고 계시면 좋겠네요.
개인적으론 둘 중 하나를 선택해서 사용합니다
https://tecoble.techcourse.co.kr/post/2020-09-29-compare-mockito-bddmockito/
📌 과제 설명
Code with me를 사용하여 게시판 (원건희), 유저 (박상민) 를 구현하였습니다.✅ PR 포인트 & 궁금한 점
PostController의findByCondition부분에서 파라미터들을@RequestParam으로 받는 방법과PostFindRequest라는 요청 객체를 만들어 받는 방법을 고민 했는데 어떤 부분이 더 좋을지 궁금합니다. 또한 해당 부분에서 분기가 4가지로 나뉘고 있는데 이러한 분기를 효율적으로 줄이는 방법도 궁금합니다!