[5기 - 홍지인] SpringBoot Part3 Weekly Mission 제출합니다. #963
Conversation
…ponseDto 추가, csv 파일 간의 데이터 전달을 위한 Dto 추가
…rController 추가, console로 입력받은 discount 값의 유효성을 검증하는 validateHandler 인터페이스와 타입별 구현체 추가
… 동시성 제어를 위한 concurrentHashMap 타입의 저장소 사용
… 입력받은 정보의 유효성이 검증되었을 때만 repository에게 요청을 전달하도록 하는 로직 추가
… command-line-application 구현, 명령어에 따라 적절한 controller에게 요청을 전달하는 로직 추가
| } | ||
|
|
||
| dependencies { | ||
| implementation 'org.springframework.boot:spring-boot-starter' |
There was a problem hiding this comment.
gradle 문법에서 dependencies라는 의존성을 추가하는 문법의 속성에는 어떤 것들이 있고, 어떤 역할들을 하는지 알고 계신지요?
| @Slf4j | ||
| @Repository | ||
| @RequiredArgsConstructor | ||
| public class JdbcVoucherRepository implements VoucherRepository { |
There was a problem hiding this comment.
테스트 코드가 없어서 간단하게 가이드 한번 해볼까 합니다.
@JdbcTest
@ContextConfiguration(classes = {JdbcVoucherRepository.class})
@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE)
class JdbcVoucherRepositoryTest {
@Autowired
private JdbcVoucherRepository jdbcVoucherRepository;
@DisplayName("Voucher 저장 테스트")
@Test
void save() {
// Setup
Voucher voucher = Voucher.builder()
.voucherId(UUID.randomUUID())
.voucherType(VoucherType.FIXED)
.discount(1000L)
.createdAt(LocalDateTime.now())
.build();
// Execution
jdbcVoucherRepository.save(voucher);
// Verification
Optional<Voucher> foundVoucher = jdbcVoucherRepository.findById(voucher.getVoucherId());
assertThat(foundVoucher).isPresent();
assertThat(foundVoucher.get()).isEqualTo(voucher);
}
@DisplayName("Voucher 조회 테스트")
@Test
void findById() {
// Setup - assuming a voucher has been saved already
Voucher voucher = Voucher.builder()
.voucherId(UUID.randomUUID())
.voucherType(VoucherType.FIXED)
.discount(1000L)
.createdAt(LocalDateTime.now())
.build();
jdbcVoucherRepository.save(voucher);
// Execution
Optional<Voucher> foundVoucher = jdbcVoucherRepository.findById(voucher.getVoucherId());
// Verification
assertThat(foundVoucher).isPresent();
assertThat(foundVoucher.get()).isEqualTo(voucher);
}
@DisplayName("모든 Voucher 조회 테스트")
@Test
void findAll() {
// Execution
List<Voucher> vouchers = jdbcVoucherRepository.findAll();
// Verification
assertThat(vouchers).isNotNull();
}
@DisplayName("Voucher 업데이트 테스트")
@Test
void update() {
// Setup - assuming a voucher has been saved already
Voucher originalVoucher = Voucher.builder()
.voucherId(UUID.randomUUID())
.voucherType(VoucherType.FIXED)
.discount(1000L)
.createdAt(LocalDateTime.now())
.build();
jdbcVoucherRepository.save(originalVoucher);
// Change some data
Voucher updatedVoucher = Voucher.builder()
.voucherId(originalVoucher.getVoucherId())
.voucherType(originalVoucher.getVoucherType())
.discount(1500L)
.createdAt(originalVoucher.getCreatedAt())
.build();
// Execution
jdbcVoucherRepository.update(updatedVoucher);
// Verification
Optional<Voucher> foundVoucher = jdbcVoucherRepository.findById(originalVoucher.getVoucherId());
assertThat(foundVoucher).isPresent();
assertThat(foundVoucher.get().getDiscount()).isEqualTo(1500L);
}
@DisplayName("Voucher 삭제 테스트")
@Test
void delete() {
// Setup - assuming a voucher has been saved already
Voucher voucher = Voucher.builder()
.voucherId(UUID.randomUUID())
.voucherType(VoucherType.FIXED)
.discount(1000L)
.createdAt(LocalDateTime.now())
.build();
jdbcVoucherRepository.save(voucher);
// Execution
jdbcVoucherRepository.delete(voucher.getVoucherId());
// Verification
Optional<Voucher> foundVoucher = jdbcVoucherRepository.findById(voucher.getVoucherId());
assertThat(foundVoucher).isNotPresent();
}
}
| import org.springframework.context.annotation.Profile; | ||
| import org.springframework.web.filter.HiddenHttpMethodFilter; | ||
|
|
||
| @Profile("default") |
| VoucherType voucherType = VoucherType.valueOf(rs.getInt("voucher_type")); | ||
| Long discount = rs.getLong("discount"); | ||
| DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss"); | ||
| LocalDateTime createdAt = LocalDateTime.parse(rs.getString("createdAt"), formatter); |
There was a problem hiding this comment.
| LocalDateTime createdAt = LocalDateTime.parse(rs.getString("createdAt"), formatter); | |
| LocalDateTime createdAt = LocalDateTime.parse(rs.getString("created_at"), formatter); |
RDBMS에서 정의한 스키마의 컨벤션과 자바에서의 컨벤션이 왔다갔다 하는 것 같습니다.
DB에서는 snake_case, Java에서는 camelCase로 통일 시키는 것을 권장합니다.
| uuidGenerator, | ||
| dateTimeGenerator, | ||
| validationStrategyMap); | ||
| voucher.validate(); |
There was a problem hiding this comment.
서비스 레이어의 통합 테스트 코드 작성하다보니 create에서 아래와 같은 예외가 발생하네요.
java.lang.NullPointerException: Cannot invoke "org.programmers.springboot.basic.domain.voucher.service.validate.VoucherValidationStrategy.validate(java.lang.Long)" because "this.validationStrategy" is null
| private byte[] getNullableBytes(UUID value) { | ||
| return value != null ? UUIDConverter.toBytes(value) : null; | ||
| } | ||
|
|
||
| private Integer getNullableValue(VoucherType value) { | ||
| return value != null ? value.getValue() : null; | ||
| } | ||
|
|
||
| private Timestamp getNullableTimestamp(LocalDateTime value) { | ||
| return value != null ? Timestamp.valueOf(value) : null; | ||
| } |
| Voucher voucher = VoucherMapper.INSTANCE.mapToEntityWithGenerator(voucherRequestDto, | ||
| uuidGenerator, | ||
| dateTimeGenerator, | ||
| validationStrategyMap); | ||
| voucher.validate(); |
There was a problem hiding this comment.
| Voucher voucher = VoucherMapper.INSTANCE.mapToEntityWithGenerator(voucherRequestDto, | |
| uuidGenerator, | |
| dateTimeGenerator, | |
| validationStrategyMap); | |
| voucher.validate(); | |
| VoucherValidationStrategy validationStrategy = validationStrategyMap.get(voucherRequestDto.getVoucherType()); | |
| if (validationStrategy == null) { | |
| throw new IllegalStateException("바우처 타입 전략 찾을 수 없는 예외: " + voucherRequestDto.getVoucherType()); | |
| } | |
| Voucher voucher = VoucherMapper.INSTANCE.mapToEntityWithGenerator( | |
| voucherRequestDto, | |
| uuidGenerator, | |
| dateTimeGenerator, | |
| validationStrategyMap | |
| ); | |
| voucher.setValidationStrategy(validationStrategy); | |
| voucher.validate(); |
작성하신 코드에서 어떤게 필요한거였을까요?
| } catch (IllegalArgumentException e) { | ||
| log.warn("Your input is Invalid UUID string: '{}'", voucherId); | ||
| model.addAttribute("err", e.getMessage()); | ||
| } catch (IllegalEmailException e) { | ||
| log.warn("Your input is Invalid Email Type: '{}'", email); | ||
| model.addAttribute("err", e.getMessage()); | ||
| } catch (DuplicateWalletException e) { | ||
| log.warn("email '{}' and voucherId '{}' is already exists in Wallet", email, voucherId); | ||
| model.addAttribute("err", e.getMessage()); | ||
| } catch (CustomerNotFoundException e) { | ||
| log.warn("No customer of email '{}' found", email); | ||
| model.addAttribute("err", e.getMessage()); | ||
| } catch (VoucherNotFoundException e) { | ||
| log.warn("No voucher of voucherId '{}' found", voucherId); | ||
| model.addAttribute("err", e.getMessage()); |
There was a problem hiding this comment.
예외마다 계속 catch 가 늘어날 것 같습니다.
이런 부분은 AOP로 예외 처리에 대한 관심사를 분리할 수 있을 것 같습니다.
GlobalExceptionHandler에서도
- Exception
- BusinessException
- 표준 예외
등등 처리 방법과 HttpStatus를 분리하여 처리할 수 있습니다.
| import org.programmers.springboot.basic.util.response.ErrResponse; | ||
|
|
||
| @RestControllerAdvice | ||
| public class GlobalExceptionHandler { |
There was a problem hiding this comment.
HttpStatus 가 겹치는 예외들은 그루핑해서 처리가 가능해보이네요.
There was a problem hiding this comment.
안녕하세요. 지인님 멘토 알입니다.
3주차 과제까지 고생하셨습니다.
고민해볼 내용과 피드백 커멘트로 남겨 놓았습니다.
✅ PR 포인트 & 궁금한 점
- 도메인 기반 개발
제가 권장하는 방법은 테스트 코드를 작성해보는 것 입니다.
로직을 작성할 때 자연스럽게 논리적인 흐름으로 생각하고 개발하게 되는데, 이러한 방식에서 벗어나기 위해서는 관점을 바꿔서 top down(하향식) / bottom up(상향식)으로 접근하여 생각해보는 것입니다.
예를 들어 하향식은 큰 로직의 흐름을 그려보고, 상향식은 최종적인 그림을 그리기 위해 하나씩 구축해보는 것입니다.
이러한 활동을 위해서는 테스트 코드를 통한 검증이 필수 단계가 되어 도메인 기반이라고 하는 개념에 조금이나마 접근할 수 있습니다.
-
조회 기능에 대한 전략
조회 기능을 구현하기 위한 전략은 엄청 많습니다. 현재 구현하신 방법은 queryString에 파라미터 key=value&... 방식으로 개발하신 것처럼 보여지네요.
검색 API를 어떤 기준으로 사용자에게 제공하기 위해 개발된 내용인지 들어봐야 알 것 같습니다.!
- ControllerAdvice를 이용한 예외처리
공통적인 부분을 처리 하는 기능은 어느정도의 규격과 프로젝트마다의 정책이 필요합니다.
- 표준 예외 코드(HttpStatus)에 따른 예외 분류하기
- 예외 메시지 규격화 시키기(e.getMessage() 사용 X)
- 개발자가 확인할 수 있는 로깅하기(로그레벨)
- 예외 타입(클래스)에 대한 전략 만들기
- 예외 테스트를 통해 메시지 확인하기
- RESTful API
RESTful API라는 개념적인 측면에서도 접근해보고, 실제 회사들이 고객들에게 제공하는 방식에 대해서도 한번 학습해보시는 것을 추천드립니다.
지금까지 학습하셨던 내용에 대해서 정리하는 시간을 우선 갖는 것이 필요해보입니다.
어떤 개념들이 정립되었고, 모호한 것들은 어떤 것들이 있는지 경계를 두는 활동을 해보는 겁니다.
정립된 것과 모호한 것에 대한 구분은 최소 기준은 "만약에 과제 또는 학습 과정에서 특정 키워드가 나왔을 때 이를 찾아가기 위해서 어떤 것들을 찾으면 되는 지?" 입니다.
예를 들면, 현재 우리는 아래와 같은 활동을 진행해보았습니다.
- 순수 자바 프로그래밍
- 프레임워크 기반의 CLI 프로그래밍
- 프레임워크 기반 프로젝트를 CLI 프로그래밍에서 Web 서비스로의 전환
이러한 활동을 강의를 토대로 하여 진행되어왔습니다.
각 활동에서 학습한 내용과 어떤 것들을 적용하셨는지 정리가 될까요?
어느정도는 정리 하면서 골격을 세우지 않으면 각 지식들의 연관성이 떨어져 모래성 마냥 무너질 수 있습니다.
| private final VoucherService voucherService; | ||
|
|
||
| @PostMapping("/create") | ||
| @ResponseStatus(HttpStatus.CREATED) |
There was a problem hiding this comment.
ResponseEntity.ok() 에 이미 HttpStatus OK 가 포함되어 있습니다.!
created라는 HttpStatus created를 사용하고 싶다면 created() 메서드가 존재합니다.
| VoucherType voucherType = null; | ||
| Long discount = null; |
There was a problem hiding this comment.
아래 로직 수행에 따라 값이 결정되기 위해 임시 선언하는 변수들은 지양합니다.
어떻게 하면 이러한 로직을 개선할 수 있을까요?.
| private final VoucherType voucherType; | ||
| private final Long discount; | ||
| private final LocalDateTime createdAt; | ||
| private VoucherValidationStrategy validationStrategy; |
There was a problem hiding this comment.
바우처 타입과 전략은 강결합되어 있는 비즈니스 정책이지 않을까 싶습니다.
타입과 전략이 다를 수도 있을까요 ?
| @ResponseStatus(HttpStatus.CREATED) | ||
| public ResponseEntity<String> create(@RequestBody VoucherControllerRequestDto requestDto) { | ||
| VoucherType voucherType = VoucherType.valueOf(requestDto.getVoucherType()); | ||
| Long discount = Long.parseLong(requestDto.getDiscount()); |
There was a problem hiding this comment.
discount를 처음부터 Long으로 받지 않고 String으로 받으시는 이유가 궁금합니다 🤔
| @RequestParam(name = "voucherId", required = false) String voucherId, | ||
| @RequestParam(name = "createdAt", required = false) String createdAt, | ||
| @RequestParam(name = "discount", required = false) String discount, | ||
| @RequestParam(name = "voucherType", required = false) String voucherType |
There was a problem hiding this comment.
쿼리 파라미터로 받을 값이 많다면 객체로 만들어서 받아도 괜찮을 것 같습니다 🤔
| } catch (NumberFormatException e) { | ||
| log.error("your input is Invalid Discount Type '{}'", discount); | ||
| model.addAttribute("err", e.getMessage()); | ||
| } catch (IllegalDiscountException e) { | ||
| log.error("your input is Invalid Discount Range '{}'", discount); | ||
| model.addAttribute("err", e.getMessage()); | ||
| } catch (DuplicateVoucherException e) { | ||
| log.warn("voucher of voucherType '{}' and discount '{} is already exists", | ||
| voucherType, discount); | ||
| model.addAttribute("err", e.getMessage()); | ||
| } catch (VoucherNotFoundException e) { | ||
| log.warn("No matching voucher of voucherType '{}' found", voucherType); | ||
| model.addAttribute("err", e.getMessage()); | ||
| } |
There was a problem hiding this comment.
콘솔 애플리케이션의 경우 별도의 ExceptionHandler를 만들지 않는 이상 이와 같이 작성해야 했지만 웹 애플리케이션의 경우 @ControllerAdvice를 통해 예외를 한 곳에서 통합 관리해도 좋을 것 같습니다.
| @PatchMapping("/{discount}") | ||
| public ResponseEntity<String> updateVoucher(@PathVariable Long discount, | ||
| @RequestBody VoucherControllerRequestDto requestDto) { |
There was a problem hiding this comment.
discount를 PathVariable로 받으시는 이유가 궁금합니다.
PathVariable로 받는 것과 Body로 받는 것의 차이는 무엇인가요? 🤔
|
|
||
| private final VoucherService voucherService; | ||
|
|
||
| @PostMapping("/create") |
There was a problem hiding this comment.
API Path에 동작이 포함되어 있군요 🤔
불가피하다면 어쩔 수 없지만 현재 상황은 HttpMethod로도 동작을 표현할 수 있을 것 같습니다.
과제가 REST API를 만드는 과제인만큼 한번 REST API에 맞게 짜보는 것도 좋을 것 같습니다..!
https://learn.microsoft.com/ko-kr/azure/architecture/best-practices/api-design
📌 과제 설명
(기본) 바우처 서비스 관리페이지 개발하기
(기본) 바우처 서비스의 API 개발하기
Spring MVC를 적용해서 JSON과 XML을 지원하는 REST API를 개발해보세요
(보너스) 바우처 지갑용 관리페이지를 만들어보세요.
👩💻 요구 사항과 구현 내용
handler
📍controller 레이어에서 발생하는 예외에 대해서 한 곳에서 몰아서 처리하는 RestControllerAdvice 클래스
📍에러가 던져졌을 경우, 해당 클래스에서 예외에 대한 ErrResponse와 HttpStatus를 담은 ResponseEntity 반환
generator
📍Voucher, Customer 도메인에 대해서 인스턴스를 생성할 때 외부에서 주입할 수 있도록 하는 Generator 클래스
converter
📍UUID와 byte[] 간의 데이터 변환을 담당하는 유틸 클래스
web
📍메인 페이지를 매핑하는, 애플리케이션을 아우르는 Controller
Response
📍각각의 예외에 대한 ErrCode와 에러가 던져졌을 때 ErrCode와 Status를 반환할 ErrResponse 클래스
Dto
📍web에서 전달받은 데이터를 담아놓은 RequestDto 클래스
Controller
📍Voucher 관리용 서비스를 위한 관리 페이지에 대한 Controller 클래스
API
📍Voucher 관리용 서비스를 위한 REST API 클래스
✅ PR 포인트 & 궁금한 점
2주차 과제에 대한 피드백에서 받았던, 단순한 기능의 분리인 Trasaction Script 패턴에서 도메인 클래스 간의 협력 관계를 구성하는 Domain Model 패턴으로 리팩토링하는 과정을 고려하고자 했습니다. Domain Model 패턴을 이용한 설계 방식이 처음인지라 제가 리팩토링하고 있는 방향성이 잘 맞는 것인지 확신이 부족한 것 같습니다. Voucher 도메인에 대해 리팩토링하는 과정에서, VoucherType에 대한 Validator를 통해 검증하는 메서드를 설계하는 과정에서, if문을 통해 validator를 가져오는 것보다 생성하는 과정에서 voucherType에 대한 적절한 validator를 주입받는 것이 더욱 관리하기에 편할 것 같다는 생각이 들었고, 그 과정에서 validator를 빈으로 등록하려고 하다 보니, Voucher 도메인의 생성자에 ValidationStrategyMap이 들어가게 되었습니다. 그 과정에서 Voucher 엔티티와 Dto 간의 필드가 맞지 않아 VoucherMapper를 통해 매핑하는 과정에서 어쩔 수 없이 set 메서드를 사용해서 validator를 주입하도록 설계가 되었습니다. 이러한 경우, 그냥 if문을 통해서 적절한 validator 클래스를 통해 validate를 검증하도록 하는 방식이 더 나은 방식인지, 어떤 방식이 최선의 방식일지 고민이 됩니다. 멘토님의 의견이 듣고 싶습니다!
트랜잭션 스크립트 패턴에서 비즈니스 로직을 도메인으로 모으는 도메인 모델 패턴으로 리팩토링하는 과정에서, 도메인 클래스로 비즈니스 로직이모여아한다라고 이해를 했습니다. 도메인 클래스에 모여야하는 비즈니스 로직의 기준(?), 어떠한 비즈니스 로직이 도메인으로 모여야하는가, 그 과정에서 도메인 클래스에서 저장소, Repository를 주입받아서 사용해도 괜찮은가, 등등 궁금한 점이 많습니다. 혹시 관련해서 참고하거나 읽으면 좋을 내용이 있을지, 멘토님의 생각은 어떠하신지 궁금합니다.
REST API를 개발하는 과정에서, 쿼리 파라미터를 통해 값이 들어오고, 이를 통해 조회의 조건을 바꿔가며 하나의 API로 조회를 할 수 있도록 조회 메서드를 설계하게 되었습니다. 해당 방식에 대해서 어떻게 생각하시는지, 혹시 옳지 않은 점이나 추가로 고려해야 할 점이 있는지 멘토님의 의견을 듣고 싶습니다.
@RestControllerAdvice를 통해 예외를 한 곳에서 처리하도록 하였는데, 이 과정에서 ResponseEntity를 반환하면서 ErrCode와 ErrStatus가 담긴 ErrResponse와 HttpStatus를 반환하도록 하였습니다. 혹시 추가적으로 더 고려해야 하거나 신경쓰면 좋을 내용들이 있을까요?
REST API를 설계하는 과정에서 제가 설계한 url 매핑 방식이 적절한 방식이었는지 확신이 부족한 것 같습니다. REST API 설계에 관련해서 참고하거나 추가로 고려하면 좋을 부분들이 있을 것 같은데, 이에 대해 멘토님의 의견이 궁금합니다. 같이 보면 좋을 내용이 있을까요?