[4기 - 김민희] SpringBoot Part3 WeeklyMission 제출합니다.#867
[4기 - 김민희] SpringBoot Part3 WeeklyMission 제출합니다.#867KimMinheee wants to merge 27 commits intoprgrms-be-devcourse:KimMinheee/week01from
Conversation
- mapStruct - spring-boot-starter-seb
- ErrorCode 내에서 관리하도록 수정
- Mapper 사용 - Controller Dto @Valid로 유효성 확인
- memberUUID -> memberId로 변경
- created_at 필드 구현
- created_at 컬럼 추가
- 매개변수로 value 값 전달받도록 수정
| import java.time.LocalDateTime; | ||
| import java.time.format.DateTimeFormatter; | ||
|
|
||
| @Component |
| */ | ||
| @ExceptionHandler(MethodArgumentNotValidException.class) | ||
| protected ResponseEntity<ErrorResponse> handleMethodArgumentNotValidException(MethodArgumentNotValidException e) { | ||
| log.error("Handle MothodArgumentNotValidException", e.getMessage()); |
There was a problem hiding this comment.
TRACE, DEBUG, INFO, WARN, ERROR 등
로그의 레벨이 나뉘어져 있어요.
각각 나뉜 의도가 있습니다.
MethodArgumentNotValidExceptionr가 발생하면 error일까요?
그렇다면 이 로그는 error일까요?
| @RequiredArgsConstructor | ||
| @RestControllerAdvice | ||
| @Slf4j | ||
| public class GlobalExceptionHandler { |
There was a problem hiding this comment.
이름좋은데요!
Rest Controller에 관련된 예외를 처리하는 handler다 라고 조금 더 명확하게 주면 어떨까요?
| protected ResponseEntity<ErrorResponse> handleMethodArgumentNotValidException(MethodArgumentNotValidException e) { | ||
| log.error("Handle MothodArgumentNotValidException", e.getMessage()); | ||
| final ErrorResponse response = ErrorResponse.of(ErrorCode.INVALID_METHOD_ERROR, e.getBindingResult()); | ||
| return new ResponseEntity<>(response, HttpStatus.BAD_REQUEST); |
There was a problem hiding this comment.
ResponseEntity.badRequest().body() 라는 정적 팩토리 메소드가 존재합니다.
| protected ResponseEntity<ErrorResponse> handleJsonProcessingException(JsonProcessingException ex) { | ||
| log.error("handleJsonProcessingException", ex); | ||
| final ErrorResponse response = ErrorResponse.of(ErrorCode.REQUEST_BODY_MISSING_ERROR, ex.getMessage()); | ||
| return new ResponseEntity<>(response, HttpStatus.OK); |
| return switch (discountType) { | ||
| case FIXED -> | ||
| new FixedAmountVoucher(UUID.randomUUID(), discountType, new DiscountValue(voucherCreateRequest.discountValue())); | ||
| new FixedAmountVoucher(UlidCreator.getUlid().toString(), discountType, new DiscountValue(voucherCreateRequest.discountValue())); |
There was a problem hiding this comment.
이렇게 static 메소드에 직접 의존하면 테스트하기도 어려울뿐더러
지금처럼 UUID -> ULID로 바뀌었을때도 상당히 많은 부분이 바뀌어야 하네요.
어떻게 해결할 수 있을까요?
| * @return BaseResponse<VoucherCreateResponse> | ||
| */ | ||
| @PostMapping() | ||
| public BaseResponse<VoucherCreateResponseData> createVoucher(@Valid @RequestBody VoucherCreateRequestData data) { |
There was a problem hiding this comment.
RequestData 라는 말이 저는 중복같아요보여요
Data 또는 Request 하나만 써도 될 것 같은데 어떻게 생각하세요?
| * @param data | ||
| * @return BaseResponse<String> | ||
| */ | ||
| @PatchMapping("/{voucherId}") |
| @NotBlank | ||
| String discountType, |
| .orElseThrow(() -> new MemberException(NOT_FOUND_MEMBER)); | ||
|
|
||
| Wallet wallet = new Wallet(UUID.randomUUID(), | ||
| Wallet wallet = new Wallet(UlidCreator.getUlid().toString(), |
WooSungHwan
left a comment
There was a problem hiding this comment.
민희님 바우처 과제 하시느라 수고 많으셨습니다. 추후 다른 과제에서도 지금처럼 좋은 모습 보여주세요 ㅎㅎ
성실하게 마무리해주셔서 감사합니다.
| final ErrorResponse response = ErrorResponse.of(ErrorCode.INTERNAL_SERVER_ERROR, e.getMessage()); | ||
| return new ResponseEntity<>(response, HttpStatus.OK); |
There was a problem hiding this comment.
Exception은 internal server error가 맞는 것 같아요.
| @ExceptionHandler(MemberException.class) | ||
| public ResponseEntity<ErrorResponse> handleNotFoundException(MemberException e) { | ||
| log.error("Handle NotFoundException :", e); | ||
| final ErrorResponse response = ErrorResponse.of(e.getErrorCode(), e.getMessage()); | ||
| return new ResponseEntity<>(response, HttpStatus.OK); | ||
| } |
There was a problem hiding this comment.
404 혹은 400이 맞는 에러코드인 것 같습니다. 멤버를 못찾는 요청이 잘못된 경우(400), 찾는 멤버가 없는 경우(404)
| @ExceptionHandler(VoucherException.class) | ||
| public ResponseEntity<ErrorResponse> handlePermissionDeniedException(VoucherException e) { | ||
| log.error("Handle PermissionDeniedException :", e); | ||
| final ErrorResponse response = ErrorResponse.of(e.getErrorCode(), e.getMessage()); | ||
| return new ResponseEntity<>(response, HttpStatus.OK); | ||
| } |
There was a problem hiding this comment.
permission denied에 맞는 http status 를 알아보시면 좋을 것 같네요. 아래도 마찬가지고요.
인증과 인가 두 개념에 대해서 http status 값에 맞는 경우를 탐구해보면 좋을 것 같습니다.
There was a problem hiding this comment.
혹은 단순히 400으로 볼 수도 있고요. 순전히 요청이 잘못됐다고 서버가 판단해야하는 포괄적인 경우 http status 에 의해서 비즈니스가 노출될 수 있으니깐요.
| @ExceptionHandler(WalletException.class) | ||
| public ResponseEntity<ErrorResponse> handlePermissionDeniedException(WalletException e) { | ||
| log.error("Handle PermissionDeniedException :", e); | ||
| final ErrorResponse response = ErrorResponse.of(e.getErrorCode(), e.getMessage()); | ||
| return new ResponseEntity<>(response, HttpStatus.OK); | ||
| } |
There was a problem hiding this comment.
permission denied에 맞는 http status 를 알아보시면 좋을 것 같네요. 아래도 마찬가지고요.
인증과 인가 두 개념에 대해서 http status 값에 맞는 경우를 탐구해보면 좋을 것 같습니다.
There was a problem hiding this comment.
혹은 단순히 400으로 볼 수도 있고요. 순전히 요청이 잘못됐다고 서버가 판단해야하는 포괄적인 경우 http status 에 의해서 비즈니스가 노출될 수 있으니깐요.
| @Override | ||
| public List<Member> findAll() { | ||
| String sql = "select member_id, member_status, name from member_table"; | ||
| String sql = "SELECT member_id, member_status, name FROM member_table"; |
There was a problem hiding this comment.
비슷한 의견으로 member 테이블의 id인데 굳이 member_id 라는 명명이 되어야 하는지도요~
| @GetMapping("/voucher/{voucherId}") | ||
| public BaseResponse<WalletGetResponses> getWalletsByVoucherId(@PathVariable String voucherId) { | ||
| WalletGetResponses responses = walletService.getWalletsByVoucherId(voucherId); | ||
| return new BaseResponse<>(responses); | ||
| } | ||
|
|
||
|
|
||
| @GetMapping("/member/{memberId}") | ||
| public BaseResponse<WalletGetResponses> getWalletsByMemberId(@PathVariable String memberId) { | ||
| WalletGetResponses responses = walletService.getWalletsByMemberId(memberId); | ||
| return new BaseResponse<>(responses); | ||
| } |
There was a problem hiding this comment.
지갑 목록에서 바우처 아이디, 멤버 아이디 두 파라미터를 받아 검색하는 지갑 목록 api로도 작성해볼 수 있겠네요 ㅎㅎ
There was a problem hiding this comment.
사실 지금도 좋긴합니다. 오히려 ocp에는 굉장히 만족스러운 행위입니다. 다만 처음에 개발할 당시엔 공통되는 부분은 최대한 로직의 재사용을 할 수 있도록 해주면 좋아요.
📌 과제 설명
👩💻 요구 사항과 구현 내용
VoucherManagement Rest API 개발- ExceptionHandler 이용
- BaseResponse를 이용한 응답 데이터 형식 통일화
2주차 이후 수정한 내용- Voucher, Member, Wallet의 id값을 UUID -> ULID로 수정
- DTO를 계층별로 구분 및 Mapper 이용
- SQL 쿼리 키워드 대문자로 수정
- BaseTimeEntity를 이용하여 created_at 데이터 추가