[Spring Core] 안금서 미션 제출합니다. #386
Conversation
There was a problem hiding this comment.
골드몽키원숭이님 안녕하세요~ 미션 고생하셨습니다 🐒
이번 리뷰부터 대략적인 가이드라인이 생겨서, 앞으로는 적은 리뷰로 자주 소통할 수 있도록 진행할 것 같아요.
먼저, 본문에 남겨주신 질문에 대한 제 생각을 남겨두겠습니다.
이 부분에서 가장 헤맸던 것 같아요. 기존 코드가 수정되면서 영향이 가는 부분들을 다 캐치 못하고 방황했던 것 같아요.
각 도메인에서 의존 관계가 형성될 때의 처리를 좀 더 연습해봐야겠네요,,
도메인 간 발생하는 의존 관계를 다루는 것은 실제로 어려운 영역이에요. 도메인 연관 관계가 복잡해지면 나중에 신경 쓸 부분이 많아지는데, 이번 미션이 그 초입을 다지기에 좋은 것 같네요.
Q1
이렇게 매핑이 안 되는 것은 time에 대해 지정한 변수명이 time 이기 때문인 것이죠?
네 맞습니다.
그렇다면 실제 프로젝트나 실무에서는 이런 실수를 어떻게 방지할 수 있을까요? 프로젝트 시작 전에 프론트 측과 변수명까지도 다 정하고 시작하나요..? 아니면 프로젝트 중에 서로 끊임없이 커뮤니케이션 하면서 맞추나요..? 🤔
좋은 질문이에요. REST API 스펙은 클라이언트와 서버가 통신을 하기 위한 인터페이스예요. 즉, 서로 간의 약속입니다.
클라이언트와 서버가 어떤 API로 통신할 거고, 요청 인자에 무엇이 담길 것이고, 그에 따른 응답 인자는 무엇이 담길 것인지 등등, 서로 약속하고 그 스펙에 맞추어서 구현을 해야하는 것은 너무나 당연한 일이에요.
따라서, 프론트와 백엔드가 공통적으로 사용할 변수명을 정하는 것, 그리고 그것을 약속대로 잘 사용하는 것은 서로가 응당히 해야할 일이겠죠.
다만, 클라이언트와 서버가 API 스펙을 맞추는 일을 사람이 모두 관리해야 한다면, 당연히 실수가 발생하기 마련일 거예요. 이를 방지하기 위해선 아래와 같은 방어책을 둬 볼 수 있을 것 같아요.
- API 문서화 도구(RestDocs, Swagger 등)를 이용해서 최신의 API 스펙을 가시적으로 보여주기
- API 스펙에 대한 테스트 코드를 만들어서 검증 자동화 하기
- 클라이언트 개발자와 서버 개발자 간의 API 스펙 소통 창구 마련하기
- 서로 자주 소통하기
Q2
스프링에서 던지는 HttpMessageNotReadableException 예외의 의도가 무엇인지를 파악하는 게 중요할 것 같아요.
Spring 공식 문서 1, Spring 공식 문서 2 에 따르면, 클라이언트 측에서 서버에 잘못된 데이터를 전송했을 경우에 이 예외가 발생하도록 설계한 것 같네요.
즉, 이 예외는 클라이언트의 잘못으로 인해 발생하는 예외인 것 같아요. 따라서, 이 예외를 잡아서 400 응답코드를 반환시키는 것은 테스트에 상관없이 상당히 자연스러운 행위라고 생각합니다.
Q3
제 생각에, 현재 미션의 코드에서 생성한 객체를 직접 보내주는 이유는 예약을 추가하자마자 바로 뷰 페이지 업데이트를 하기 위해서인 것 같아요. 즉각적으로 생성된 객체의 정보를 받아서 업데이트 하는 거죠.
네 저도 그렇게 생각해요. 그게 프론트의 의도인 것 같네요.
생성한 객체를 바디에 넣어서 보내야 하는 경우와, 보내지 않아도 되는 경우를 구분하기 위한 방법이 있을까요?
클라이언트 코드를 뜯어보는 것도 방법일 수 있지만, 그보다는 API 스펙 문서를 더 꼼꼼하게 보면 된다는 사실!
미션에 명시된 응답 명세에 따르면, 프론트에서 Time 객체에 대한 정보가 필요한 것 같네요. 이는 금서님이 언급해주신대로, 프론트에서 바로 뷰 페이지를 업데이트하기 위함인 것 같아요.
리뷰 반영하시다가, 궁금한 점 있으시면 언제든지 질문 주세요~! 파이팅~
| private final JdbcReservationRepository reservationRepository; | ||
| private final JdbcTimeRepository timeRepository; |
There was a problem hiding this comment.
ReservationRepository와 TimeRepository에 대한 인터페이스를 만드셔서 추상화를 해주셨던데, ReservationService 및 TimeService에서는 추상화 한 객체들에 의존하지 않고 구체 클래스에 의존하고 있네요. 어떤 이유에서 이런 선택을 해주셨나요?
There was a problem hiding this comment.
예리한 지적이시네요 코코닭!
Spring MVC 미션에서는 메모리를 기반으로 repository를 쓰다가 Spring Jdbc로 넘어가면서 데이터베이스를 기반으로 repositroy를 쓰는 방식을 변경했기 때문에 변경에 유연하게 대처하기 위한 방법 중 하나로 인터페이스를 두는 것을 생각했습니다!
하지만,,, 그걸 service에 적용해볼 생각은 못 해봤네요.. service 같은 경우에는 현재 미션으로 봐선 따로 인터페이스를 둘 필요가 있나 싶은데, 코코닭님은 어떻게 생각하시나요? 나중에 프로젝트 규모가 커지고 비즈니스 로직이 복잡해진다면 인터페이스를 두어야 할 상황이 생기기도 하나요? 궁금합니다!
There was a problem hiding this comment.
소통의 오류가 있던 것 같아요. 제가 의미했던 바는 아래와 같아요.
AS-IS
private final JdbcReservationRepository reservationRepository;
private final JdbcTimeRepository timeRepository;TO-BE
private final ReservationRepository reservationRepository;
private final TimeRepository timeRepository;Repository를 추상화했다면, Service에서 의존하고 있는 Repository는 구체 클래스보다는 추상화한 클래스가 되는 편이 좋을 것 같습니다. 그래야 다형성을 이용해볼 수도 있을 것 같아요~!
There was a problem hiding this comment.
아하 그런 의미였군요! 이해 했습니다!!!!! 반영하도록 할게요 😃
| @ExceptionHandler(HttpMessageNotReadableException.class) | ||
| public ResponseEntity<String> handleInvalidTypeException(HttpMessageNotReadableException e) { | ||
| return ResponseEntity.badRequest().body(e.getMessage()); | ||
| } |
There was a problem hiding this comment.
PR 본문에도 남겨뒀지만, 제 생각에는 HttpMessageNotReadableException 예외를 잡아서 400 응답을 내려주는 것이 자연스러운 행위라고 생각해요.
여기서 드는 한 가지 궁금증은, 이 예외를 Reservation에 대해서만 잡아도 괜찮을까? 인 것 같네요. TimeController에서도 같은 시나리오로 이 예외가 발생할 여지가 있어보여요. 어떻게 처리하면 좋을까요?
There was a problem hiding this comment.
저도 코코닭님의 생각에 동의합니다!
Time의 경우 시간을 직접 키보드로 입력받지 않고 주어진 보기로부터 마우스로 선택하는 형태로 입력을 받다보니, 잘못된 타입의 값이 들어올 거라고는 생각을 못해서 처리를 안 해줬던 것 같네요. time과 date 같이, 주어진 보기를 마우스로 클릭하는 형태로 받는 것은 클라이언트 코드에 달려있는 것이겠죠? 그렇다면 잘못된 값이 들어올 수 있는 경우가 있을까요? 궁금해서 여쭤봅니다!
There was a problem hiding this comment.
좋은 질문을 남겨주셨었네요!
일단 제 생각은, 서버 개발자는 API 관점에서 이를 바라보는 게 좋을 것 같아요.
서버 개발자의 입장에서 클라이언트는 API를 사용하는 대상일 거예요. 이때 클라이언트는 아래와 같을 수 있어요.
- API를 사용하는 프론트 개발자
- API를 사용하는 제3자
첫 번째의 경우, 프론트 개발자의 설계대로(마우스로 클릭하는 형태)만 사용자가 움직여준다면 예외가 발생하지 않을 수 있겠네요. 다만, 프론트의 설계를 기반으로 백엔드 설계를 하게 된다면, 설계가 프론트의 구조에 의존하게 되면서 변화에 취약해질 수 있어요. 이를테면, 마우스로 클릭하는 형태에서 키보드로 입력하는 형태로 바뀐다면 이에 맞추어서 예외 처리 코드를 새로 만들어 주어야 될 거예요.
두 번째의 경우, 우리가 만든 API가 어떻게 사용될 지 예측할 수 없어요. 악의적으로 API 요청을 보내는 경우도 포함될 수 있구요.
이러한 것들을 모두 고려해봤을 때, 저는 서버 관점에서만 바라보았을 때 발생할 수 있는 모든 예외는 마땅히 처리되는 편이 좋다고 생각합니다~!
|
|
||
| @Slf4j | ||
| @ControllerAdvice(assignableTypes = TimeController.class) | ||
| public class TimeExceptionHandler { |
There was a problem hiding this comment.
TimeExceptionHandler의 내용이 ReservationExceptionHandler와 중복적으로 겹치는 부분들이 보이네요. 왜 겹칠까요?
예를 들어, MethodArgumentNotValidException 같은 예외는 Time만의 예외인가요? TimeExceptionHandler 라는 영역에서 반드시 처리되어야 하는 예외인가요?
ReservationExceptionHandler와 TimeExceptionHandler와 같이 도메인 별로 예외 처리를 나누었을 때, 내부적으로 각각 어떤 예외를 처리하는 것이 가장 효과적일지 고민해보면 좋을 것 같아요.
There was a problem hiding this comment.
엇 좋은 지적 감사합니다 코코닭님!
각각 도메인만의 예외처리에만 집중해서 코드를 추가하다보니, 위 코코닭님이 말씀하신 부분에 대해서는 고려해보지 못했네요.
MethodArgumentNotValidException예외는 Dto 객체에 달린 @NotBlank 와 같은 어노테이션을 통한 유효성 검증에 실패했을 때 발생하는 예외인데요! 현재 코드에는 ReservationController의 범위에서 발생한 예외를 처리하는 ReservationExceptionHandler와, TimeController의 범위에서 발생한 예외를 처리하는 TimeExceptionHandler 내에 중복 코드로 존재합니다. MethodArgumentNotValidException예외 뿐만 아니라, HttpMessageNotReadableException예외, Exception예외도 마찬가지죠. 코코닭의 리뷰를 보고 각 핸들러 내에 있는 코드 중복은 왜 존재할까? 에 대해 생각해보았고, 결국 "공통적으로" 처리해야 하는 예외라는 결론을 지었어요!
NotFoundReservation, NotFoundTimeReservation_ 예외 같은 경우, 커스텀 예외이므로 공통으로 처리하기 보단, 각 도메인에 대한 범위를 지정하여 예외를 처리하는 것이 좋은 방법 같아요. 만약, 커스텀 예외가 아니라 NoSuchElementException예외나, EmptyResultDataAccessException 와 같은 이미 존재하는 예외로 처리했을 경우에는 지금처럼 각 도메인 관련 예외 핸들러를 만들지 않고 공통 예외로 다 뺐을 수도 있겠네요.
앞으로 예외처리를 할 때에는 예외 범위에 대해 잘 생각해서 공통 예외와 도메인별 예외를 잘 구별해야겠네요!
| public class Time { | ||
|
|
||
| private Long id; | ||
| private String time; |
There was a problem hiding this comment.
시간을 다루는 객체에서, 가장 중요한 필드 값의 자료형이 적절하지 않다는 생각이 들어요!
도메인 객체가 만들어질 때, 그 도메인의 특징에 맞는 데이터가 들어갈 수 있도록, 데이터 무결성을 보장해야 한다고 생각해요.
현재는 time 값에 대한 검증이 없는 상태이기 때문에, Time 객체가 시간을 나타내는 객체로서 유효하지 않을 수 있겠다는 생각이 듭니다.
시간을 표현할 때 String보다 더 적절한 자료형이 있을지 고민해보면 좋겠어요.
| import lombok.Getter; | ||
|
|
||
| @Getter | ||
| public class Time { |
There was a problem hiding this comment.
데이터베이스의 스키마의 이름은 time 이지만, 도메인의 이름이 스키마의 이름을 꼭 따라가야할까요?
도메인은 비즈니스를 정의하는 영역이죠. 데이터베이스와는 별개의 영역이에요. 조금 더 비즈니스 적으로 직관적인 이름을 가지도록 해보면 어떨까요?
이를테면 ReservationTime 이라는 이름을 지어볼 수 있을 것 같아요. Time이라는 도메인 명은, 이 도메인이 어느 비즈니스를 위해 사용되는지 모호한 이름일 수 있어요.
There was a problem hiding this comment.
그러네요,,
코코닭님이 스터디 때 언급하셨던 것처럼,
시간이라는 게 예약 시간일 수도 있고, 예약 시작 시간 혹은 끝나는 시간일 수도 있고 등등 ... 시간에 관련된 객체들이 만들어질수록 구현하는 데 있어 혼란을 야기할 것 같습니다. 네임을 더 구체적이고 명확하게 표현하는 습관을 들이겠습니다! 수정하겠니다~!
안녕하세요 코코닥! 또 뵙네요.
항상 말씀드리지만 사소한 거라도 좋으니, 날카로운 리뷰 부탁드립니다!
이번에도 기대하고 있을게요 🙉
이번 미션은
이 부분에서 가장 헤맸던 것 같아요. 기존 코드가 수정되면서 영향이 가는 부분들을 다 캐치 못하고 방황했던 것 같아요.
각 도메인에서 의존 관계가 형성될 때의 처리를 좀 더 연습해봐야겠네요,,
<궁금한 점>
Q1. 이렇게 매핑이 안 되는 것은 time에 대해 지정한 변수명이 time 이기 때문인 것이죠? 그렇다면 실제 프로젝트나 실무에서는 이런 실수를 어떻게 방지할 수 있을까요? 프로젝트 시작 전에 프론트 측과 변수명까지도 다 정하고 시작하나요..? 아니면 프로젝트 중에 서로 끊임없이 커뮤니케이션 하면서 맞추나요..? 🤔
`Map<String, String> reservation = new HashMap<>();
.body(reservation) .when().post("/reservations").statusCode(400);Q2. 이걸 구현 후 테스트를 통과했을 때, 테스트를 통과하기 위해 짜맞추는 느낌(?)이 강했거든요. 이렇게 하는 것이 맞는 방식인지 궁금합니다!
Q3. 위 두 가지 방식의 차이점은 응답 바디에 저장한 시간 객체를 넣어서 보내냐 마냐인데요!
생성한 객체를 바디에 넣어서 보내야 하는 경우와, 보내지 않아도 되는 경우를 구분하기 위한 방법이 있을까요?
제 생각에, 현재 미션의 코드에서 생성한 객체를 직접 보내주는 이유는 예약을 추가하자마자 바로 뷰 페이지 업데이트를 하기 위해서인 것 같아요. 즉각적으로 생성된 객체의 정보를 받아서 업데이트 하는 거죠. 그런데 이 부분은 클라이언트 코드를 봐야 알 것 같은데, 이 부분도 1번 질문과 비슷하게 클라이언트와 커뮤니케이션을 하면서 구현해야하는 건가요...? 아니면 제가 생각한 내용에서 틀린 내용이 있나요? 코코닥의 생각을 들려주세요!
이번에도 잘 부탁드립니다 꼬꼬닭! 🐓
화이팅!