[2단계 - 주문 기능 구현] 도치(김동흠) 미션 제출합니다. #38
Conversation
| @@ -0,0 +1,4 @@ | |||
| package cart.exception; | |||
|
|
|||
| public class CannotApplyCouponException extends RuntimeException { | |||
There was a problem hiding this comment.
- 예외 메시지가 없는 이유가 있을까요?
- 예외가 발생했을 때 어떤 예외가 일어났는지 정확하게 파악하려면 예외 메세지가 필요하지 않을까요?
There was a problem hiding this comment.
저는 @ControllerAdvice가 붙은 ControllerExceptionHandler에서 api response에 들어간 메시지를 지정하는 게
한곳에서 메시지를 관리하기에 좋다 는 장점이 있는 듯해 controllerAdVice에서 예외 메시지를 정합니다.
다만 더 생각해볼 측면은 API를 위한 메시지 뿐만 아니라 개발자를 위한 예외 메시지가 있으면 좋다는 측면에서,
예외를 던질 때와 controllerAdvice 모두에서 예외메시지를 집어넣는 게 좋을 수도 있겠다는 생각이 드네요.
우선 이렇게 수정하겠습니다.
빙봉은 controllerAdvice에서 제 코드처럼 예외메시지를 지정하는 거에 대해서 어떻게 생각하시나요?
혹은
public enum ErrorCode {
STATION_NOT_EXIST(HttpStatus.BAD_REQUEST.value(), "존재하지 않는 역입니다."),
STATION_ALREADY_EXIST(HttpStatus.BAD_REQUEST.value(), "이미 존재하는 역입니다."),
INVALID_SECTION_LENGTH(HttpStatus.BAD_REQUEST.value(), "등록될 수 없는 길이의 구간입니다. 1이상의 정수가 아니거나, 너무 큰 길이입니다."),
...
}
이처럼 Enum에서 예외에 대한 메시지를 관리하고 controllerAdvice에서 이를 가져다 쓰는 방식은 어떤가요?
빙봉은 예외메시지에 대한 어떤 관리방식을 선호하는 지 궁금합니다!
There was a problem hiding this comment.
빙봉은 controllerAdvice에서 제 코드처럼 예외메시지를 지정하는 거에 대해서 어떻게 생각하시나요?
좋은 방식이라고 생각합니다! 어떻게 보면 특정 에러 코드에 따른 메시지만을 매핑하고 있는 형태라 Code보다는 ErrorCodeMessage가 더 직관적으로 보이긴 하네요ㅎㅎ
예외 메시지는 예외가 발생했을 때 예외 자체의 메시지와 API Response로 주는 메시지 정도로 나눌 수 있을 것 같은데요. 어떤 경우에는 예외 자체의 메시지를 API Response로 그대로 주기도 하고, 어떤 경우에는 예외 자체의 메시지가 아닌 기획 or 제휴사쪽에서 정의하는 메시지를 나타내줘야하는 경우가 있습니다. 상황에 따라 다르기 때문에 관리방식도 달라집니다. 실무에서도 위에 작성해주신 방식처럼 작성합니다 (다만 HttpStatus가 아닌 ErrorCode를 따로 정의해둡니다)
이와 별개로 예외 자체의 메시지는 필수적인데요. 클래스명만으로 예외를 파악하기 어렵기 때문에 에러 메시지 없이 예외를 정의하는 경우는 거의 없습니다.
| return orderDao.save(payed, discounting); | ||
| } | ||
|
|
||
| private boolean isCouponSelected(OrderRequest orderRequest) { |
There was a problem hiding this comment.
이렇게 주문에 쿠폰,포인트 처럼 선택적으로 적용하는 기능의 경우 API 설계를 어떻게 하는 게 좋을까요?
- requestBody에 couponId를 특정 숫자를 아무 할인을 하지 않는 쿠폰이라 정해서 그 숫자로 요청 보내기 ex) couponId = 1로 주문 요청하면 할인이 없는 쿠폰을 적용
- couponId를 List로 받아 적용하지 않으려는 경우 빈 리스트로 요청
- requestBody내에서가 아닌 api url의 parameter로 받기
위에서 잘 설명해주셨다시피 현재 요구사항은 간단하기 때문에 위 3가지 방법 모두 가능합니다. 다만 여러 쿠폰이 적용될 수 있다는 가정 하에 확장성과 안정성 측면에서 2번이 가장 안정적이라고 이야기할 수 있을 것 같아요. (기본값을 emptyList로 받고 for문을 돌면서 적용하는 형태)
1번처럼 숫자를 특정하는 건 하드 코딩이 들어가는 것이기 때문에 추천하지 않고,
3번의 경우 현재 주문을 하는 행위를 하고 있기 때문에 coupon은 주문과 지엽적인 부분이라고 생각되어 request Body에 들어가는 것이 더 적절하다고 생각됩니다.
정책이 어디까지 확장될 지 모르는 상황에서의 API설계는 앞을 어디까지 내다보고 설계하는 게 좋을까요?
이건 참 어려운 부분이라고 생각해요. 실무에서 정책이 정해져도 다 개발해놨더니 엎어지는 경우도 있고 (혹은 내년에 적용해야된다던가..) 자주 사용한다고 했는데 한 번만 사용되고 쓰이지 않는 경우도 있거든요.
많은 경험을 쌓지 않은 이상 판단하기 어려울 것 같습니다. 제 약간의 경험을 보태자면 일단은 현재 요구사항에 맞게 코드를 작성하고 추후에 확장성을 고려해서 리팩토링하는 것으로도 충분하다고 생각해요 🙂
There was a problem hiding this comment.
답변 감사합니다! 빙봉의 답변으로 그래도 어떤 측면들을 생각해보면 좋을지 알게 된 것 같아요 👍
|
|
||
| public void delete(Long couponId) { | ||
| String sql = "DELETE FROM coupon WHERE id = ?"; | ||
| jdbcTemplate.update(sql, couponId); |
There was a problem hiding this comment.
- update 메서드가 어떤 걸 리턴할까요?
- 예외 처리가 필요할 것 같습니다!
There was a problem hiding this comment.
해당 update는 바뀐 행들의 수를 반환합니다!
생각지 못했는데 이를 고려하면 해당하는 쿠폰이 없다면 예외가 발생해야 할 것 같습니다.
추가로 2개 이상의 쿠폰이 update되어도 문제가 있는 상황인지 생각해 볼 수 있겠습니다.
유일해야 하는 쿠폰의 id가 2개 이상 쿼리되었기에 서버 내의 문제로 보아 500을 반환해야하는 상황인지 고민되기도 합니다.
수정해서 반영하겠습니다!
| public Money apply(Order order, Long couponId) { | ||
| Coupon coupon = couponDao.findById(couponId); | ||
| Money discounting = order.apply(coupon); | ||
| couponDao.delete(couponId); |
There was a problem hiding this comment.
쿠폰을 삭제하지 않고 사용됐다는 컬럼을 넣는 건 어떨까요? 추후 사용자가 쿠폰을 사용했는지 여부를 파악할 수 있어야 하지 않을까요?
There was a problem hiding this comment.
저도 필요한 기능이라고 생각합니다!
쿠폰이 사용됬는지를 저장하는 used 컬럼을 추가하겠습니다
| deleteIds.add(new Object[]{id}); | ||
| } | ||
|
|
||
| jdbcTemplate.batchUpdate(sql, deleteIds); |
There was a problem hiding this comment.
일부라도 변하지 않은 게 있다면 알리는 게 맞는 동작 같습니다!
수정하겠습니다.
dao에 대한 예외처리를 하면서 궁금한게 생겼습니다.
우선 저는 dao의 예외와 비즈니스의 예외는 가급적 분리되는 게 좋다고 생각하지만, 영속성 계층까지 내려가봐야 인지할 수 있는 비즈니스의 예외 상황도 어쩔 수 없이 존재한다고 생각합니다.
때문에 커스텀한 비즈니스 예외는 직접 throw해 controllerAdvice에서 따로 응답메시지를 할당하고, 나머지 영속성 관심사의 예외들-db연결, 무결성... 은 500 internal error 로 응답하자는 생각입니다.
영속성 계층의 예외에 대한 제 이런 생각이 적절할까요? 어떤 식으로 처리되는게 좋을지가 궁금합니다!
There was a problem hiding this comment.
때문에 커스텀한 비즈니스 예외는 직접 throw해 controllerAdvice에서 따로 응답메시지를 할당하고, 나머지 영속성 관심사의 예외들-db연결, 무결성... 은 500 internal error 로 응답하자는 생각입니다. 영속성 계층의 예외에 대한 제 이런 생각이 적절할까요? 어떤 식으로 처리되는게 좋을지가 궁금합니다!
네 좋습니다! db연결, timeout, 무결성 등등.. 이런 부분들은 500 예외로 응답하면 됩니다! 500 예외는 ControllerAdvice로 에러 로그를 남기고, 응답으로는 오류가 발생했습니다. 정도로 내려보내주면 될 것 같네요! (지금 controller advice에 Exceptoin 작성하신 것처럼)
|
빙봉 이번 단계 마지막 리뷰요청이 되겠군요! 리뷰 감사했습니다. |
| //then | ||
| assertThatThrownBy(() -> couponDao.findUnusedById(1L)).isInstanceOf(CouponNotFoundException.class); | ||
| } | ||
| } |
| @@ -0,0 +1,4 @@ | |||
| spring.datasource.url = jdbc:h2:mem:test # h2 support in-memory-db | |||
There was a problem hiding this comment.
실제 어플리케이션을 실행할 때는 인메모리 DB가 아닌 상용에서 사용되고 있는 DB 기준(mysql)으로 새로 만들어서 붙는 게 좋습니다. 더미데이터가 너무 많거나 외부 의존성이 많다면 개발 DB에 붙기도 합니다.
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| @ActiveProfiles("test") |
There was a problem hiding this comment.
해당 어노테이션을 없어도 돌아갈텐데요. 왜 돌아갈까요? 고민한 번 해보시면 좋을 것 같습니다
There was a problem hiding this comment.
기본적인 예외가 핸들링되어있는 ResponseEntityExceptionHandler도 사용해보세요!
| implementation 'com.h2database:h2' | ||
|
|
||
| testImplementation 'org.springframework.boot:spring-boot-starter-test' | ||
| testImplementation 'io.rest-assured:rest-assured:4.4.0' | ||
|
|
||
| runtimeOnly 'com.h2database:h2' | ||
| testImplementation 'com.h2database:h2' |
There was a problem hiding this comment.
- h2를 implementation와 testImplementation 두 곳에 추가하신 이유가 있을까요?
- testImplementation는 없어도 되지 않을까요?
- 왜 없어도 될까요?
- runtimeOnly는 어떤건지 알아볼까요?
안녕하세요 빙봉!
협업 미션 구현해보았습니다.
이번 미션은 협업하며 정책을 직접 정하기도 하기에 자유도가 높아 오히려 고민된 부분도 있는 것 같습니다.
저희는 쿠폰을 구현하기로 했고,
정책은 간단하게 10만원 초과이면 1000원 할인 쿠폰을 부여. 배달비는 3000원 고정.
쿠폰을 적용하여 주문 가능하도록 정책을 정했습니다.
협의한 API예시 명세
이번 미션 범위는
미션 커밋 범위
입니다. 배포된 상태로 개발해본 경험이 적어 쓸모없는 트러블 슈팅용 커밋도 생긴 것 같아 아쉽습니다.
추가로 질문이 있는데 답해주시면 감사하겠습니다!
선택적 적용항목에 대한 API설계?
주문 요청 시 쿠폰을 적용할 수도 있고, 쿠폰 없이 주문을 할 수도 있습니다.
지금은 쿠폰을 적용하지 않는 요청이라면
requestBody의 couponId필드에 값을 넣지 않고 요청해 null이 할당되게 하고 있습니다.
다만 api요청 시 null값이 들어오는 건 db접근 시 에러를 발생시킬 수 있고, 실수로 들어온 null값과 쿠폰을 적용하지 않으려 넣지 않은 것을 구분하기 어렵겠다는 생각도 했습니다.
때문에 생각해본 방법들은
ex) couponId = 1로 주문 요청하면 할인이 없는 쿠폰을 적용
등의 방법이 있을 듯 한데, 사실 지금의 간단한 정책 상으로는 무엇을 택해도 문제가 없겠다는 생각이 들기도 했습니다.
질문은
이렇게 주문에 쿠폰,포인트 처럼 선택적으로 적용하는 기능의 경우 API 설계를 어떻게 하는 게 좋을까요?정책이 어디까지 확장될 지 모르는 상황에서의 API설계는 앞을 어디까지 내다보고 설계하는 게 좋을까요?입니다