Feature/#2/login #6
Conversation
| @GetMapping("/{id}") | ||
| @LoginCheck | ||
| public UserDto getUserInfoById(@PathVariable @NotBlank String id) { | ||
| @GetMapping("/myInfo") |
There was a problem hiding this comment.
URL 주소는 가급적 RESTful하게 작성하는 것이 좋습니다.
대문자는 사용하지 않고 "-"를 구분자로 사용합니다.
| public enum ErrorCode { | ||
|
|
||
| /* 401 UNAUTHORIZED */ | ||
| NOT_LOGIN(HttpStatus.UNAUTHORIZED, "로그인된 사용자 정보가 존재하지 않습니다."), |
There was a problem hiding this comment.
CustomException, ErrorCode, ExceptionAdvice, ErrorResponse가 너무 강하게 결합되어 있습니다.
CustomException을 날릴 때, ErrorCode를 지정해서 발생시키고 있는데요
CustomException이라는 예외는 단순 에러코드를 전달하는 것 외에는 에러코드 안에 내용을 보기 전에 어떤 에러인지 명확하지가 않습니다.
또한 이 예외 안에 들어가는 ErrorCdoe에는 HttpStatus 코드가 들어가 있습니다.
예외는 서비스단에서 발생을 시키고 있으며, 서비스단에서 발생하는 예외는 Controller단(Presentation Layer)과는 분리가 되어있어야 하는데 서비스단에서 발생한 예외가 Controller단에 사용자에게 어떻게 응답을 할지에 대한 로직까지가 결합이 되고 있습니다.
로그인 서비스 로직을 처리할 때 사용자 로그인이 실패했다는 예외를 통해 알려주기만 해야하는데, 사용자 응답까지 다 결합하는 것은 레이어별 로직 분리가 안되게 됩니다.
그래서 예외 발생시 사용자에게 전달한 메시지와 HttpStatus코드는 service layer와 결합이 되지 않게 해주세요.
|
|
||
| @Getter | ||
| @Builder | ||
| public class ErrorResponse { |
There was a problem hiding this comment.
ErrorResponse는 현재의 코드를 보면
에러가 발생할 때에만 ResponseEntity에 들어가는 데이터입니다.
그런데, 클라이언트 입장에서 생각해보면
응답 데이터의 body에 성공시에는 userDto데이터가, 실패시에는 ErrorResponse 객체가 들어가 있게 됩니다.
API 응답 포맷은 일관성이 있어야 하는데, 이렇게 되면 클라이언트 입장에서 개발이 난해해질 때가 많고 서버측에서도 유지보수 측면에서 좋지 않습니다.
API 요청에 대한 메시지와 코드 등에 대한 값을 전달을 하려면 성공시나 실패시에나 동일하게 전달을 해줘야 합니다.
ErrorResponse에 대한 명칭을 정상 상황에서도 처리할 수 있는 형태로 변경하고 정상시에도 이 객체를 응답하게 해주는 방법이 있고
또는 ResponseEntity대신에 별도의 응답 클래스를 만들어서 쓰는 방법도 있습니다. (이 ErrorResponse대신에)
아래와 같은 형태의 코드라고 보시면 될꺼 같은데 참고하세요
public class Response<T> {
private String code;
private String message;
pirvate T body;
}
| @ResponseStatus(HttpStatus.BAD_REQUEST) | ||
| public String illegalArgumentException(IllegalArgumentException e) { | ||
| return e.getMessage(); | ||
| @ExceptionHandler(ConstraintViolationException.class) |
There was a problem hiding this comment.
해당 예외에 대한 StatusCode를 정의하는 것은 아래 어노테이션으로 할 수 있습니다.
@ResponseStatus(HttpStatus.BAD_REQUEST)
No description provided.