-
Notifications
You must be signed in to change notification settings - Fork 0
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
[3주차] 심화 과제 #13
base: main
Are you sure you want to change the base?
[3주차] 심화 과제 #13
Conversation
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.
세미나 과제라는 점을 보면서 세미나 이외로 그 이상 추가적으로 더 공부해보시거나 혹은 보다 자세히 알아보면 좋을 것 같은 부분에 리뷰 남겨두었어요 :)
내용이 너무 어렵거나 이게 뭔소리지 싶으시면 언제든 질문해주세요~
@AllArgsConstructor | ||
@JsonInclude(JsonInclude.Include.NON_NULL) | ||
@Getter | ||
public class ApiResponse { |
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.
ApiResponse 라는 객체는 정적 팩토리 메서드로도 생성이 가능하지만 동시에 AllArgsConstructor 를 통해서도 생성이 가능하다 생각이 드네요
객체가 여러 방법으로 생성될 수 있기 때문에 함께 작업하는 협업자에게 단순 ApiResponse 를 사용하라 하면 예를 들어
success : "false"
message: "성공"
data: null
과 같은 형태가 만들어지는 휴먼에러 케이스가 발생할 수 있지 않을까요?
객체 자체의 생성을 제한하여 성공/실패 두 가지 경우만 나올 수 있도록 해보는 것은 어떨까요?
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.
물론 생성자와 팩토리 메서드는 각각의 장단점이 있기 때문에 이를 파악해보시면 좋을 것 같습니다.
Effective Java에서는 "생성자 대신 정적 팩토리 메서드를 고려하라" 라는 부분이 있는 만큼 단순히 제공하는 객체 생성 방식이 아니라 보다 의미있는 객체 생성 방식을 제공하기 위해 고려해보면 좋을 것 같습니다
public static ApiResponse fail(String message) { | ||
return new ApiResponse(false, message, null); | ||
} |
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.
정적 팩토리 메서드를 사용하는 경우 javadoc 에서 확인이 어렵기 때문에 네이밍 규칙이 존재하는 것으로 알고 있어요
그 중 정적 팩토리 메서드를 사용하는 경우에 하위 타입으로 쉽게 분류한 객체를 생성하는 방식이 있는데 현재의 경우가 "성공 응답"/ "실패 응답" 이라는 하위 타입으로도 만들 수 있는 상황이라 생각됩니다.
실제 타입으로 구분하여 구분하지 않지만 이 경우 사용되는 get[타입 이름]/new[타입이름] 의 형식 혹은 타입 이름을 간단하게 명시하곤 합니다.
success 에서 사용한 것과 같이 fail 또한 동사가 아닌 하위 상태 객체를 만든다는 입장에서 명사의 타입이름을 지어주면 어떨까요?
@RestControllerAdvice | ||
@Slf4j | ||
public class ExceptionController { | ||
|
||
@ExceptionHandler(NotFoundException.class) | ||
public ResponseEntity<ApiResponse> handleNotFoundException(NotFoundException e) { | ||
log.error("handleNotFoundException", e); | ||
return ResponseEntity.status(HttpStatus.NOT_FOUND).body(ApiResponse.fail(e.getMessage())); | ||
} | ||
|
||
@ExceptionHandler(MethodArgumentNotValidException.class) | ||
public ResponseEntity<ApiResponse> handleMethodArgumentNotValidException(MethodArgumentNotValidException e) { | ||
log.error("handleMethodArgumentNotValidException", e); | ||
return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(ApiResponse.fail(e.getMessage())); | ||
} | ||
} |
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.
현재 처리된 NotFoundException, MethodArgumentNotValidException이외 예외가 발생하면 2 예외와 다른 형태를 가지지 않을까요?
스프링에서는 기본적으로 예외를 ResponseEntityExceptionHandler에서 미리 처리해두기 때문에 해당 ExceptionController에서 처리되지 않은 Exception 은 모두 DefaultHandlerExceptionResolver로 처리하여 서로 다른 처리를 가져가 스프링에서 기본적으로 제공하는 처리로 인해 응답이 날아간다면 클라이언트에서 일괄되지 않은 형태의 에러 응답을 받아 처리가 힘들어지는 경우가 발생할 수 있어요
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.
ResponseEntityExceptionHandler에서는 아래와 같은 ServletException을 제공합니다.
HttpRequestMethodNotSupportedException.class,
HttpMediaTypeNotSupportedException.class,
HttpMediaTypeNotAcceptableException.class,
MissingPathVariableException.class,
MissingServletRequestParameterException.class,
ServletRequestBindingException.class,
ConversionNotSupportedException.class,
TypeMismatchException.class,
HttpMessageNotReadableException.class,
HttpMessageNotWritableException.class,
MethodArgumentNotValidException.class,
MissingServletRequestPartException.class,
BindException.class,
NoHandlerFoundException.class,
AsyncRequestTimeoutException.class
특히 NotFound의 경우 NoHandlerFoundException 으로 지원하기 때문에 이를 별도로 구현하는 것이 아닌 기본 제공하는 친구를 이용하거나
ResponseEntityExceptionHandler 자체를 상속하여 handleExceptionInternal를 이용하여 모든 예외 응답이 동일하게 처리 될 수 있도록 해보는 것은 어떨까요?
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.
해당 파일은 과제에서는 큰 문제 없지만 합동 세미나 및 실제 DB를 운영하는 경우 주의가 필요한 파일이에요 :)
해당 글을 읽어보면 좋을 것 같아요
참고로 깃허브에 올라오는 모든 commit은 지워지지 않는다고 보셔도 무방합니다. 따라서 db password가 올라간 경우 파일을 삭제한다 해도 커밋 hash 를 찾으면 조회가 가능하기 때문에 add, commit 전에 불필요한 파일이 snapshot에 추가 되진 않는지 확인해보면 좋습니다
📝 관련 이슈
closed #11
👩🏻💻 내용
메이커스 레포지토리를 참고했습니다!
유저 등록 시 / 게시글 등록 시
게시글 조회 시
게시글이 존재하지 않는 경우
유저가 존재하지 않는 경우
😸 중점을 둔 부분
💡 배운 점
에러 핸들링 오류 발생
😹 궁금한 점