[5기 김용상] SpringBoot Part3 Weekly Mission 제출합니다. #953
[5기 김용상] SpringBoot Part3 Weekly Mission 제출합니다. #953YongNyeo wants to merge 110 commits intoprgrms-be-devcourse:yongnyeo/3from
Conversation
…nto yongnyeo/1
zxcv9203
left a comment
zxcv9203
left a comment
There was a problem hiding this comment.
안녕하세요 용상님, 스프링 3주차 과제 고생하셨습니다. 🙇
궁금한 부분 코멘트 남겨드렸으니 확인부탁드립니다. 🙇
| @GetMapping("/date") | ||
| ResponseEntity<List<Voucher>> findByDate(@RequestParam String startTime, @RequestParam String endTime) { | ||
| LocalDateTime startDate = LocalDateTime.parse(startTime); | ||
| LocalDateTime endDate = LocalDateTime.parse(endTime); | ||
| return ResponseEntity.ok(voucherService.findByDate(startDate, endDate)); | ||
| } | ||
|
|
||
| @GetMapping("/type") | ||
| ResponseEntity<List<Voucher>> findByType(@RequestParam String type) { | ||
| VoucherType voucherType = VoucherType.valueOf(type); | ||
| return ResponseEntity.ok(voucherService.findByType(voucherType)); | ||
| } | ||
|
|
||
| @GetMapping("") | ||
| ResponseEntity<List<Voucher>> findAll() { | ||
| return ResponseEntity.ok(voucherService.findAll()); | ||
| } |
There was a problem hiding this comment.
해당 API 들은 findAll()로 하나로 합칠 수 있을 것 같습니다.
추가로 Voucher의 데이터의 개수가 매우 많을수도 있으므로 Pagination 적용을 고려해보셔도 좋을 것 같습니다 🤔
| @PostMapping("/create") | ||
| ResponseEntity<Voucher> create(@RequestBody CreateVoucherDto dto) { | ||
| if (dto.getVoucherType() == 2 && dto.getValue() > 100) { | ||
| throw new InputMismatchException(INVALID_VOUCHER_PERCENT.getMessage()); | ||
| } | ||
| return ResponseEntity | ||
| .status(HttpStatus.CREATED) | ||
| .body(voucherService.create(dto)); | ||
| } |
There was a problem hiding this comment.
CREATED는 HTTP 스펙상 URI Location 정보를 추가하는 것이 좋을 것 같습니다.
추가로 설계한 API에 동작이 들어가 있는데 Http Method로 동작을 어느정도 표현가능할 것 같습니다 🤔
Rest API 디자인 가이드를 한번 읽어보셔도 좋을 것 같습니다.
https://learn.microsoft.com/ko-kr/azure/architecture/best-practices/api-design
|
|
||
|
|
||
| @PutMapping("/{id}/edit") | ||
| ResponseEntity<?> update(@PathVariable String id, @RequestBody UpdateVoucherDto dto) { |
There was a problem hiding this comment.
아무것도 반환하지 않는다면 와일드카드를 반환하는 것보다는 ResponseEntity<Void>를 반환하는게 적절할 것 같습니다 🤔
| @ExceptionHandler | ||
| String ArgumentTypeMistMatch(HttpMessageNotReadableException e) { | ||
| return INVALID_VOUCHER_INFO.getMessage(); | ||
| } | ||
|
|
||
| @ExceptionHandler | ||
| String DuplicatedId(DuplicateKeyException e) { | ||
| return e.getMessage(); | ||
| } | ||
|
|
||
| @ExceptionHandler | ||
| String ExceedVoucherPercent(InputMismatchException e) { | ||
| return e.getMessage(); | ||
| } | ||
|
|
||
| @ExceptionHandler | ||
| String EmptyResult(EmptyResultDataAccessException e) { | ||
| return e.getMessage(); | ||
| } |
There was a problem hiding this comment.
예외에 대한 핸들링은 @RestControllerAdvice를 통해 한곳에서 관리하면 컨트롤러 클래스의 코드가 간결해질 것 같습니다.
| // @Test | ||
| // @DisplayName("findById()로 존재하지 않는 바우처 읽기에 실패한다.") | ||
| // void findByIdFail() throws Exception { | ||
| // mockMvc.perform(get("/api/vouchers/{id}", NotExistId) | ||
| // .contentType(MediaType.APPLICATION_JSON)) | ||
| // .andExpect(status().isNotFound()) | ||
| // .andDo(print()); | ||
| // } --> 웹이나 postman에서는 notFound 처리되지만, mockMvc 테스트만 항상 200처리됩니다. |
There was a problem hiding this comment.
실제로 VoucherService에 대한 처리가 진행되지 않아 결과로 null이 반환되어 200이 발생한 상황입니다.
이미지를 보면 200임에도 아무것도 반환되지 않는 것을 볼 수 있는데 원래 Service는 Exception이 발생하는데 @MockBean 으로 선언된 VoucherService의 findById(...)는 어떤 동작을 기대하는지 지정하지 않아 null이 반환된 것이고 이로인해 테스트에서는 200을 반환하는 상황입니다.
이를 위해 어떤 동작을 기대하는지 명시하거나, 직접 서비스를 등록하여 주입받는 방법이 있을 것 같습니다 🤔
2 participants
📌 과제 설명
Api Voucher Controller : api 를 통해 서비스 동작을 돕는 Controller입니다.
Web Voucher Controller : web thymeleaf를 통해 서비스 동작을 돕는 Controller입니다.
API컨트롤러에서는 @ExceptionHandler를 통해 예외처리를 진행했고,
Web 컨트롤러에서는 BindingResult로 예외처리를 도왔습니다.
👩💻 요구 사항과 구현 내용
SpringBoot Part3 Weekly Mission
(기본) 바우처 서비스 관리페이지 개발하기
(기본) 바우처 서비스의 API 개발하기 (뷰는 따로 X)
(보너스) 바우처 지갑용 관리페이지를 만들어보세요.
✅ PR 포인트 & 궁금한 점
처음 예외처리할때 @ExceptionHandler에서 status변경 +메시지 변경을 했습니다.
그런데 mockMvc 테스트를 짜다보니, 실패했어도 무조건 200 (ok) 처리가 돼서 찾아보니(웹과 postman은 정상작동하지만 mockMvc만 )
MockMvc Test always return status code 200 라는 키워드가 있었고, 해당 문제를 처리하려면 컨트롤러 코드를 다 바꿔야하는데,
과연 테스트를 위해 도메인쪽 코드를 바꾸는게 옳은가? 라는 생각이 들었습니다. 그래서 일단은 테스트코드를 보류했는데..
@ExceptionHandler에서 모두 status를 ok 로 관리하면 문제없지만.. 실무에선 상태코드를 어떻게 관리하는지? 궁금합니다.
Api controller에서 현재로서는 response dto가 따로 없이 ResponseEntity<>로 반환을 하고있습니다. response dto는 request dto와 다르게 검증이 따로 필요없기에.. 복잡도 올라간다 생각해서 구현을 안했는데, 엔티티의 변경 가능성 때문이라도 반드시 구현해야 하는 것일까요?
어떤 예외는 controller-service-repository레이어에서 처리하고, 어떤 예외는 디스패쳐서블릿 밖으로 던져서 ExceptionHandler로 처리하는지... 궁금합니다! 예외를 특정시점에서 정상흐름으로 처리해야할지/말지 정도로 이해하면될까요?
고민할거리 많이 던져주셔서 항상 감사합니다 😄 👍