[5기 김주환] SpringBoot Part3 Weekly Mission 제출합니다. #951
Conversation
This reverts commit 8826194a8fdada5b705be8ef8cb5dc127a515ce6.
| } | ||
|
|
||
| @PutMapping("/{id}") | ||
| public void updateVoucher(@PathVariable UUID id, UpdateVoucherRequest request) { |
There was a problem hiding this comment.
UpdateVoucherRequest 를 Form 타입으로 받으시는 군요 Body로 받지 않는 이유가 있으신가요? 🤔
There was a problem hiding this comment.
@requestbody 를 넣어야 하는데 누락 되었습니다...ㅎ
| @PostMapping("/") | ||
| public void createVoucher(@RequestBody CreateVoucherRequest request) { | ||
| voucherService.create(request); | ||
| } |
There was a problem hiding this comment.
다음과 같이 받으면 /api/voucher/ 라는 path로 주어져야 호출이 가능할 것 같은데 의도하신 부분인지 궁금합니다. 🤔
| @GetMapping("/vouchers") | ||
| public List<VoucherResponse> getAllVouchers() { | ||
| return voucherService.findAll(); | ||
| } |
There was a problem hiding this comment.
나중에 바우처의 개수가 1000만개 이렇게 쌓였을때 전부 가져오게 된다면 애플리케이션에 큰 부담이 갈 것 같습니다.
이를 방지하기 위해 Pagination을 적용해봐도 좋을 것 같습니다 🤔
| @GetMapping("/vouchers/search") | ||
| public List<VoucherResponse> getVouchersByCriteria( | ||
| @Valid @ModelAttribute VoucherCriteria criteria | ||
| ) { | ||
| return voucherService.findByCriteria(criteria); | ||
| } |
There was a problem hiding this comment.
해당 API는 전체 조회 API와 합치면 하나의 API로 리스트 조회를 구현할 수 있을 것 같습니다 🤔
There was a problem hiding this comment.
저는 개인적으로 endpoint 를 따로 두는것을 선호하는데, 현업에서는 보통 합쳐놓나요!?
There was a problem hiding this comment.
전체 조회를 하는 경우랑 조건을 통해 조회하는 경우가 반드시 분리되어야 할 이유가 있다면 분리할 수 있지만 두 기능 다 WHERE 조건을 주는 것 말고는 크게 다를게 없는 부분인 것 같습니다.
API를 여러개 만들 수록 사용하는 입장에서 혼란을 줄 수 있다고 생각하기 때문에 합칠 수 있다면 합치는 것을 선호하는 편입니다,,!
| dependencies { | ||
| implementation 'org.springframework.boot:spring-boot-starter' | ||
| // Display name 쓰기 위해 | ||
| implementation 'org.junit.jupiter:junit-jupiter:5.8.1' |
There was a problem hiding this comment.
jUnit은 testImplementation 'org.springframework.boot:spring-boot-starter-test' 에 포함되어 있지 않나요?
| import org.springframework.transaction.annotation.Transactional; | ||
|
|
||
| @Service | ||
| @Transactional(readOnly = true) |
There was a problem hiding this comment.
@transactional(readOnly = true)를 클래스 레벨에 선언한 기준이 있으실까요?
| import java.time.LocalDateTime; | ||
| import java.time.format.DateTimeFormatter; | ||
|
|
||
| public class SqlConverter { |
There was a problem hiding this comment.
유틸 클래스는 final 키워드를 통해 상속이 불가하도록 하여 간혹 발생할 수 있는 문제를 방지하는 것이 좋을듯합니다.
추가로 private 생성자는 불필요한 생성을 방지 하기 위해 필수 입니다.!
| @SpringBootTest | ||
| @Transactional | ||
| @ActiveProfiles("test") |
There was a problem hiding this comment.
| @SpringBootTest | |
| @Transactional | |
| @ActiveProfiles("test") | |
| @JdbcTest | |
| @ActiveProfiles("test") | |
| @ContextConfiguration(classes = { | |
| JdbcUserVoucherWalletRepository.class, | |
| JdbcUserRepository.class, | |
| JdbcVoucherRepository.class | |
| }) | |
| @AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE) |
테스트 코드가 갈수록 늘어나게 되는건 불가피합니다.
따라서 테스트 코드의 실행 시간에 해해서 고민을 하게 되는데요.
가장 큰 원인이 모든 테스트에서 @SpringBootTest를 사용하는 것입니다.
전체 context가 필요한 바깥쪽 (Controller) 테스트 영역이 있고, 안으로 들어올 수록 필요한 context가 줄어들게 됩니다.
현재 repository 레이어에서는 @SpringBootTest가 꼭 필요 없어보입니다.
| @SpringBootTest | ||
| @Transactional | ||
| @ActiveProfiles("test") |
There was a problem hiding this comment.
| @SpringBootTest | |
| @Transactional | |
| @ActiveProfiles("test") | |
| @JdbcTest | |
| @ActiveProfiles("test") | |
| @ContextConfiguration(classes = {JdbcVoucherRepository.class}) | |
| @AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE) |
| @SpringBootTest | ||
| @Transactional | ||
| @ActiveProfiles("test") |
There was a problem hiding this comment.
| @SpringBootTest | |
| @Transactional | |
| @ActiveProfiles("test") | |
| @JdbcTest | |
| @ActiveProfiles("test") | |
| @ContextConfiguration(classes = {JdbcUserRepository.class}) | |
| @AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE) |
| @ExceptionHandler({ | ||
| Exception.class, | ||
| }) | ||
| public ResponseEntity<String> handleException(Exception ex) { |
There was a problem hiding this comment.
커스텀한 메시지가 아니라면 예외의 스택에 쌓인 모든 메시지가 출력되는 이슈가 있습니다.
이러한 부분은 보안적으로도 문제가 발생할 수 있어, 클라이언트에게는 정제된 예외 메시지를 제공하고 , 내부적으로 로그 처리 하는 것을 권장합니다.
클라이언트에게 제공할 예외 메시지와 로그로 쌓아 내부적으로 trace 할 수 있는 메시지의 포맷은 다르게 가져가는 것이 좋을 것 같습니다.
There was a problem hiding this comment.
안녕하세요. 주환님 멘토 알입니다.
3주차 과제도 고생하셨습니다.
학습이 필요해보이는 내용 또는 궁금한 내용 커멘트에 남겨놓았습니다.
각 레이어별 테스트 코드를 시도해보세요.
의존성에 대한 고민을 자연스레 하실 수 있을 것 같습니다.
웹 서비스의 컨트롤러는 MockMvc 테스트로 Http에 대한 응답 결과를 검증 하시는 것을 권장합니다.
또한 globalExceptionHandler 라는 aop 기능은 어떻게 테스트 할 수 있을까요?
편하게 진행하시고, 수정사항 완료되시면 슬랙부탁드립니다.!
|
|
||
| @ControllerAdvice | ||
| public class GlobalThymeExceptionHandler { | ||
|
|
There was a problem hiding this comment.
| @ResponseStatus(value = HttpStatus.BAD_REQUEST) |
GlobalThymeExceptionHandler가 처리하는 예외들은 별도의 응답 코드가 나가지만 HttpStatus는 200으로 보여지는데 의도하신 내용일까요?
📌 과제 설명
SpringBoot Part3 Weekly Mission
(기본) 바우처 서비스 관리페이지 개발하기
(기본) 바우처 서비스의 API 개발하기
(보너스) 바우처 지갑용 관리페이지를 만들어보세요.
타임리프로 만든 김에 만든 페이지를 ec2에 배포해 보았습니다!
하찮은 페이지지만... ui 로 둘러보시면 리뷰 하시기 더 편할 것 같아 페이지 링크 첨부 합니다!
타임리프로 만든 바우처 관리 페이지
환경 변수 파일은 datasource 부분 저번이랑 같은데, 혹시 몰라 다시 공유해 드리겠습니다!
👩💻 요구 사항과 구현 내용
예외 처리
GlobalThymeExceptionHandler
타임리프에서 발생한 에러를 핸들링 하는 곳
ErrorController 의 getErrorPath 메서드는 Spring Boot 2.3 이상에서는 더 이상 사용되지 않아 ErrorController 대신 @ExceptionHandler를 사용하였습니다.
GlobalRestExceptionHandler
Rest api 에서 발생하는 에러를 핸들링 하는 곳
ErrorResponse 를 body 로 돌려줍니다.
프로파일 설정
슬랙에서 말씀해주신 대로 프로파일이
tomcat일때만 웹 관련 빈들이 등록 & 실행되게 만들었습니다.기본 프로파일 또한
tomcat으로 설정되어 있습니다!✅ PR 포인트 & 궁금한 점
아직 2주차 피드백을 수정하지 못하고 3주차 먼저 올리는 점 죄송합니다 ㅠㅠ
2주차 수정 하면서 궁금한점 생기면 말씀드리겠습니다!
path parameter 유효성 검사
유효성 검사 질문은 매 pr 에 올려서... 너무 광기로 집착하는 것 같아 죄송합니다....ㅠㅠ
원래 웹 mvc 로 컨트롤러를 짤때는 path로 들어오는 값들은 딱히 제약이 있는게 아니라면 검사를 원래 하지 않았습니다.
이유 : null 이나 빈값이면 404가 알아서 뜨니까
근데 이번에 command 컨트롤러를 짜면서 null 값이 들어올 수 있는 것을 보고 해당 컨트롤러에서는 path parameter 도 유효성 검사를 진행하였습니다.
웹 mvc 로 컨트롤러를 짤때도 String 으로 값을 받으면 " " 이런 empty 한 값이 들어 올 수 있던데 (404 안뜸)
이 부분에서는 유효성 검사를 따로 진행해야 할까요??
@validated VS validator()
컨트롤러에서 스프링 mvc 를 안쓰니까 그냥 @Valid 어노테이션을 사용할 수 없어서 @validated 를 선언을 해주었습니다.
그렇게 하니까 Test 할때 이것저것 설정해서 통합테스트를 해야하더라구요....ㅠ
그래서 validator() 로 바꿀까 생각도 해보고 구현을 해놨는데, 사용하는 곳마다 validator() .validate(...); 요렇게 적어주는게 코드 중복이 심할 것 같아 우선 @validated 방식으로 하였는데 @validated 랑 @Valid 를 사용할때는 Test 는 어떤식으로 하고 뭘 좀 더 신경 써주어야 할까요..