-
Notifications
You must be signed in to change notification settings - Fork 171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[4기 - 안재영] SpringBoot Part3 Weekly Mission PR 제출합니다. #862
base: JaeyoungAhn
Are you sure you want to change the base?
Conversation
@@ -0,0 +1,26 @@ | |||
GET http://localhost:8080/api/json/v1/vouchers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good
이렇게 다양한 방식으로 테스트해보는것도 좋다고 생각합니다
import org.springframework.web.bind.annotation.*; | ||
|
||
@RestController | ||
@RequestMapping("/api/json/v1") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rest api 아닌가요?
rest의 url 규칙에 대해 알아보세요
헤더와 mapping 어노테이션을 이용할수 있습니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 내용에 대해서 공부를 해보았는데 URL에는 어떠한 데이터 형식으로 주고받는지에 대한 내용이 있으면 안되는 것을 배웠습니다. 대신 mapping의 옵션에서 어떤 데이터를 받아들일지 명확히 나타내도록 하겠습니다.
@@ -0,0 +1,26 @@ | |||
package com.prgmrs.voucher.controller.mvc.dto; | |||
|
|||
public class VoucherMvcCreateRequest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setter를 안써도 됩니다.
ModelAttribute에 대해 알아보세요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import org.springframework.web.bind.annotation.*; | ||
|
||
@RestController | ||
@RequestMapping("/api/xml/v1") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
VoucherJsonController와
마찬가지에요
@Override | ||
public List<Voucher> findByCreationTimeAndDiscountType(LocalDateTime startDate, LocalDateTime endDate, int discountType) { | ||
String sql = """ | ||
SELECT | ||
v.voucher_id, v.discount_type, v.discount_value | ||
FROM `voucher` v | ||
WHERE (v.discount_type = :discountType) | ||
AND (v.created_at BETWEEN :startDate AND :endDate) | ||
"""; | ||
|
||
Map<String, Object> paramMap = new HashMap<>(); | ||
|
||
paramMap.put("startDate", startDate); | ||
paramMap.put("endDate", endDate); | ||
paramMap.put("discountType", discountType); | ||
|
||
return jdbcTemplate.query(sql, paramMap, toRowMapper()); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
메소드랑 쿼리랑 순서가 일치하지 않네요.
쿼리를 봐야 조건절 순서를 알 수 있겠네요
순서를 맞출수도 있고
메서드 명이 길어지므로 파라미터로만 파악할수도 있습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
순서를 일치시킬 수 있도록 하겠습니다!
@@ -26,7 +26,7 @@ | |||
import static org.mockito.BDDMockito.given; | |||
|
|||
@DisplayName("바우처 컨트롤러 레이어를 테스트한다.") | |||
class VoucherControllerTest { | |||
class VoucherJsonControllerTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
json controller인데 json에 대한 테스트는 진행되지 않고있네요.
컨트롤러 레이어를 테스트 하는 방법은 무엇이 있을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
컨트롤러 레이어를 테스트하기 위해 실제 서블릿 컨테이너의 구동 없이 MVC 환경에서 HTTP를 서블릿 요청을 할 수 있는 MockMVC가 있는 것을 배웠습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@WebMvcTest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@WebMvcTest
WebMvcTest에 대해서 공부해보겠습니다..!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
재영님 바우처 과제하시느라 수고 많으셨습니다. 성실하게 마무리해주셔서 감사드립니다. 다음과제에서는 바우처에서 아쉬웠던 부분 좀더 보완해서 성장하신 모습 볼 수 있었으면 좋겠습니다.
@@ -0,0 +1,26 @@ | |||
package com.prgmrs.voucher.controller.mvc.dto; | |||
|
|||
public class VoucherMvcCreateRequest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GetMapping(value = "/vouchers", produces = "application/xml") | ||
public ResponseEntity<VoucherListResponse> findAll() { | ||
return ResponseEntity.ok(voucherService.findAll()); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 한가지 언급안했던 부분이 있긴한데 ㅠ
produces 속성 안에 들어가보시면 알겠지만 배열입니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
produces = {MediaType.APPLICATION_JSON_VALUE, MediaType.APPLICATION_XML_VALUE}
이런 형상이 가능해져요 ㅎㅎ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
형식별로 두개의 컨트롤러를 만들어야 하나 생각했는데, 이렇게 배열로 처리 할 수 있군요..!
private UUIDConverter() { | ||
throw new IllegalStateException("Util class"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
클래스도 final이면 더 좋을듯 합니다 ㅎㅎ 의미도 맞고~
📌 과제 설명 및 요구사항
(기본) 바우처 서비스 관리페이지 개발하기
(기본) 바우처 서비스의 API 개발하기
구조 및 흐름
👩💻 구현 내용
2주차 피드백 반영
refactor : 콘솔 실행 관련 클래스 위치 view 밖으로 이동
refactor : GlobalExceptionHandler 패키지 변경
rename : 가독성 향상을 위해 enum class 내 변수명 변경
refactor : 가독성을 위해 short이 아닌 int 타입으로 데이터를 다루도록 변경
refactor : null 관련 검증 코드 추가
refactor : 추후 테스팅 용이성을 위해 추상화 된 UUID 생성 클래스를 주입하는 것으로 변경
refactor : NumberFormatException을 Global Exception Handler에서 에러 처리하도록 변경
fix : UUID Generator 수정에 따라 제어 요구사항이 필요하지 않은 테스트 코드 수정
refactor : 일부 익셉션을 @ControllerAdvice를 통해 처리하도록 변경
3주차
feat : @ControllerAdvice를 이용한 글로벌 익셉션 핸들링
feat : 바우처 도메인 관련 json, xml, thymeleaf 지원
✅ PR 포인트 & 궁금한 점
프로젝트 전체적으로 문제는 없는지 궁금합니다!
validation은 NullPointerException을 내지 않게끔 하는 것에서 충분한지 아니면 더 많은 검증을 해야하는지 궁금합니다. 예를 들어서 DTO에 시작 시간, 끝 시간 두 가지가 LocalDateTime이라는 타입으로 들어오는데, 이것이 null이 아닌 것을 조사하는 것에서 끝나지 않고 더 나아가서, 끝 시간이 시작 시간보다 뒤에 있다는 것을 검증해주는 것이 맞는지 고민이 되는데, 여기서 더 나아가서 그렇다면 애초에 년도 데이터 형식이 올바른지도 검증을 해야할 것 같고, 더 나아가서 이제 2023년인데 2024년 2022년 같은 원치않는 데이터가 들어오면 어떻게 해야하는지와 같은 끝이 없는 고민을 하게 되는 것 같습니다.