[4기 - 김희빈] SpringBoot Part3 Weekly Mission PR 제출합니다. #834

kmebin · 2023-07-14T18:45:57Z

👩‍💻 요구 사항과 구현 내용

(기본) 바우처 서비스 관리페이지 개발하기

Spring MVC를 적용해서 thymeleaf 템플릿을 설정해보세요.
커맨드로 지원했던 기능을 thymeleaf를 이용해서 관리페이지를 만들고 다음 기능을 지원가능하게 해보세요
- 조회페이지
- 상세페이지
- 입력페이지
- 삭제기능

(기본) 바우처 서비스의 API 개발하기

Spring MVC를 적용해서 JSON과 XML을 지원하는 REST API를 개발해보세요
- 전체 조회기능
- 조건별 조회기능 (바우처 생성기간 및 특정 할인타입별)
- 바우처 추가기능
- 바우처 삭제기능
- 바우처 아이디로 조회 기능

✅ 피드백 반영사항

✅ PR 포인트 & 궁금한 점

코멘트로 추가하였습니다!

kmebin · 2023-07-14T19:03:07Z

src/main/java/com/programmers/voucher/domain/customer/controller/CustomerApiController.java

+    @PostMapping
+    @ResponseStatus(HttpStatus.CREATED)
+    public BaseResponse<CustomerResponse> createCustomer(@Validated @RequestBody CustomerCreateRequest request) {
+        CustomerResponse customer = customerService.createCustomer(request);
+        return BaseResponse.created(customer);


일관적인 Response 형태를 위해 BaseResponse 클래스를 따로 만들고, @ResponseStatus로 상태 코드를 지정하도록 했는데, 이렇게 작성하는 것보다 ResponseEntity에 BaseResponse 객체를 전달하는 방식이 더 나을까요? 🥹

아뇨, 두 방법 어떤 것으로든 사용해도 상관 없습니다. @RestController가 @responsebody를 포함하고 있어 ResponseEntity를 사용하는 것 보다 지금 하신 방법이 더 나은 것 같기도 하네요.

kmebin · 2023-07-14T19:12:21Z

src/main/java/com/programmers/voucher/exception/advice/ExceptionControllerAdvice.java

+    @ResponseStatus(HttpStatus.BAD_REQUEST)
+    @ExceptionHandler({
+            BadRequestException.class,
+            IllegalArgumentException.class,
+            HttpMessageNotReadableException.class
+    })
+    public BaseResponse<Object> handleBadRequestException(Exception exception) {
+        log.error("[BadRequestException] = ", exception);
+        return BaseResponse.error(HttpStatus.BAD_REQUEST.value(), exception.getMessage());


커스텀 exception인 BadRequestException과 다른 Exception들을 같은 handler로 처리하도록 했는데 이 방식에서 문제가 될 부분이 있을까요?!

각 익셉션들에 대해 일관된 처리라면 여러 익셉션을 한꺼번에 처리해도 상관 없습니다. try-catch 에서와 같게 생각하시면 됩니다.

kmebin · 2023-07-14T19:21:15Z

src/main/java/com/programmers/voucher/domain/voucher/dto/VoucherCreateRequest.java

 public record VoucherCreateRequest(
-        VoucherType voucherType,
-        int discountAmount
+        @NotNull VoucherType voucherType,


VoucherType과 같이 Enum 타입을 Request로 받을 때는 어떻게 검증하는 것이 좋을까요?
현재는 VoucherType에 맞지 않는 값이 들어왔을 때, VoucherType.valueOf()를 하거나, json으로 받은 요청 데이터를 역직렬화하는 과정에서 발생하는 예외를 처리하고 있습니다..!
하지만, 이 방식은 예상치 못한 곳에서 검증을 제대로 하지 못할 것 같다는 생각이 드는데 .. Enum을 validation하는 어노테이션을 커스텀해서 적용하는 것이 좋을까요?

유효성 체크는 "VoucherType 이 필수 값이고, FIXED나 PERCENT 로 들어와야한다." 면 충분해 보입니다.
그렇게 안들어 왔을때 요청이 어떻게 들어 왔는지 확인할 수 있게 로그를 잘 남기는 것이 중요합니다.

joseph-100

과제 하시느라 고생 하셨습니다.

콘솔에서 웹 확장시 컨트롤러만 추가되고 서비스는 그대로 인점에서 확장이 잘 되고 있는 느낌을 받았습니다.
TC가 돌지 않는데 확인 부탁 드리고요. 따로 설정이 필요한 부분은 README에 추가해주세요.
여유가 되시면 영명님이 공유하신 mysql docker 설정도 추가해주시면 좋을 것 같습니다.

joseph-100 · 2023-07-20T02:03:05Z

src/main/java/com/programmers/voucher/domain/customer/controller/CustomerApiController.java

+    @PostMapping("/{customerId}")
+    public BaseResponse<Object> deleteCustomer(@PathVariable UUID customerId) {


Customer를 삭제할거라 예상하기 어렵습니다. Url 경로나 HttpMethod 를 적절히 변경해주세요.
바우처쪽도 같이 챙겨주세요.

Hunkik

과제 고생하셨습니다! BaseResponse 방식은 저도 괜찮다고 생각합니다.

확실히 확장하는데 있어서 코드 변경사항이 많지 않아 리뷰할 부분이 많이 없네요. 그만큼 확장을 고려해서 개발하셨다는게 느껴져요!

현재 과제는 실제 생각하고 만든 비즈니스가 아니라 Validation이나 비즈니스 같은 부분을 생각하지 않고 코드만 짜는 훈련을 했다고 생각하시면 될 것 같습니다. 이제부터 하는 과제들에 대해선 코드적인 부분 이외에 아키텍처적인 부분, 비즈니스를 고려한 부분, 서비스에 적합한지도 같이 리뷰에 들어갈게요!

Hunkik · 2023-07-23T13:19:30Z

src/main/java/com/programmers/voucher/constant/BaseResponse.java

+    @JsonInclude(NON_NULL)
+    private String message;
+
+    @JsonInclude(NON_NULL)
+    private T data;


JsonInclude는 어떤것때문에 넣으신건가요??

Hunkik · 2023-07-24T15:25:30Z

...test/java/com/programmers/voucher/domain/customer/repository/JdbcCustomerRepositoryTest.java

    void 고객_생성() {
        // given
-        Customer customer = new Customer(UUID.randomUUID(), "test");
+        Customer customer = Customer.builder().id(UUID.randomUUID()).nickname("test").build();


똑같은게 자주 쓰인다면 메서드나 factory로 만들어서 쓰시면 더 유지보수하기 편할듯해요

Hunkik · 2023-07-24T16:17:09Z

src/main/java/com/programmers/voucher/domain/customer/controller/CustomerViewController.java

+        return "redirect:/customers";
+    }
+
+    @GetMapping
+    public String getAllCustomers(Model model) {
+        List<CustomerResponse> customers = customerService.getAllCustomers();
+        model.addAttribute("customers", customers);
+
+        return "customers/index";


여기서 redirect인 부분하고 아닌 부분 차이가 어떤건가요?

Hunkik · 2023-07-24T16:20:54Z

src/main/java/com/programmers/voucher/domain/voucher/controller/VoucherApiController.java

+    @PostMapping
+    @ResponseStatus(HttpStatus.CREATED)
+    public BaseResponse<VoucherResponse> createVoucher(@Validated @RequestBody VoucherCreateRequest request) {
+        VoucherResponse voucher = voucherService.createVoucher(request);


현재는 Voucher에 바로 request를 넣었지만, 서비스에서 request를 아는게 결국 결합도가 높아진다고 보셔도 될 것 같아여. 예를들면 Api명세가 바뀌어 request를 수정하면 여기에 맞춰 서비스로직이 영향을 받을 수 있기 때문이기도 합니다.

하지만 모든 서비스가 Dto를 사용한다면 그것대로 생산성이 떨어지기 때문에 트레이드오프를 잘 고려하시면 좋을것같아요

Hunkik · 2023-07-24T16:28:33Z

src/main/java/com/programmers/voucher/domain/voucher/dto/VoucherCreateRequest.java

 public record VoucherCreateRequest(
-        VoucherType voucherType,
-        int discountAmount
+        @NotNull VoucherType voucherType,


바로Enum을 Request로 받는 경우 문자열이 Enum에 어떤 값에도 해당하지 않거나, 다른타입인 경우는 어떻게 예외를 처리해보면 좋을것같아요.

kmebin added 23 commits July 13, 2023 14:08

Merge branch 'week2' into main

824a8b0

chore: thymeleaf, lombok 의존성 추가

99257f3

feat: 바우처 프로그램 메인 뷰 구현

563525c

feat: 바우처 전체 조회, 생성, 삭제 뷰 구현

08b1903

feat: 바우처 전체 조회, 생성, 삭제 요청 매핑

2afdbd3

feat: 바우처 상세 조회 뷰 구현

67e5b89

feat: 바우처 상세 조회 요청 매핑

9ea89d5

feat: 전체 메뉴로 돌아가기 버튼 추가

c88e54e

feat: 고객 생성, 조회, 삭제 뷰 구현 및 요청 매핑

b0fa89f

docs: 3주차 요구 사항 업데이트

e132e49

feat: ErrorCode 적용

776f45a

feat: 바우처 조회, 생성, 삭제 API 구현

b386283

feat: 고객 조회, 생성, 삭제 API 구현

14e0920

feat: 바우처 타입별 조회 구현

b1faf0b

docs: API 개발 요구 사항 업데이트

9aeea3e

fix: VoucherType 불필요한 필드 제거

0b98cae

feat: RestControllerAdivce를 통해 전역 예외 처리

cd3aeaa

refactor: lombok 적용

1c5cee0

refactor: Enum values를 맵으로 관리

7ea52b9

refactor: console 하위 패키지로 분리

706714b

chore: validation 의존성 추가

7ee0757

feat: validation을 통한 요청 검증

75221d7

test: 잘못된 검증 로직 수정

5d601a2

kmebin added 🔥 매운맛 Team-Joseph labels Jul 14, 2023

kmebin requested a review from joseph-100 July 14, 2023 18:45

kmebin self-assigned this Jul 14, 2023

kmebin commented Jul 14, 2023

View reviewed changes

joseph-100 reviewed Jul 20, 2023

View reviewed changes

Hunkik reviewed Jul 24, 2023

View reviewed changes

		@PostMapping("/{customerId}")
		public BaseResponse<Object> deleteCustomer(@PathVariable UUID customerId) {

[4기 - 김희빈] SpringBoot Part3 Weekly Mission PR 제출합니다. #834

Are you sure you want to change the base?

[4기 - 김희빈] SpringBoot Part3 Weekly Mission PR 제출합니다. #834

Uh oh!

Conversation

kmebin commented Jul 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

👩‍💻 요구 사항과 구현 내용

(기본) 바우처 서비스 관리페이지 개발하기

(기본) 바우처 서비스의 API 개발하기

✅ 피드백 반영사항

✅ PR 포인트 & 궁금한 점

Uh oh!

kmebin Jul 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kmebin Jul 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joseph-100 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hunkik left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kmebin commented Jul 14, 2023 •

edited

Loading

kmebin Jul 14, 2023 •

edited

Loading

kmebin Jul 14, 2023 •

edited

Loading