cIonecoder / expedia

Expedia Clone Coding - Portfolio
0 stars 1 forks source link

[회원/인증] 로그인, 토큰 발급 기능 구현 #39

Closed BAEKJungHo closed 2 years ago

BAEKJungHo commented 2 years ago

코드 리뷰 이후 -> 로그인 API 리팩토링 -> 테스트 코드 작성 순서로 진행 예정

shbada commented 2 years ago

클라이언트에서는 어떤 에러가 내려올지 모르는 상황에서 errorFields의 존재 여부를 판단하는데에 오류 가능성이 있어보여요! 이 부분 이해를 못했는데 구체적으로 설명해주실 수 있을까요?

예를들어서 400 에러가 발생했을때 어떤 경우에는

return CommonResponse().apply {
result = Result.FAIL
this.message = errorCode.message
this.errorCode = errorCode.code
this.errorFields = errorFields
}

가 수행되고,

return CommonResponse().apply {
result = Result.FAIL
this.message = message
this.errorCode = ErrorCode.BAD_REQUEST.code
}

가 수행됬을때

클라이언트에서는 errorFields가 존재하는지를 체크해서 값을 사용하던지 해야해서 이렇게 result 가 같은 값일때 어떤 객체에는 errorFields가 null이고 어떤 객체는 아닐때 판별하기 어려워서 오류가 발생할 수 있을거같아서요!

보통 Response의 key는 모두 동일하게 가져가고 그 안에서 값이 달라지지 않을까해서요!

BAEKJungHo commented 2 years ago

errorFields

해당 필드는 빈 List 로 초기화 돼서 들어가 있기 때문에, @JsonInclude 같은 어노테이션으로 제외시키지 않는 이상 응답에 포함되지 않나요?

errorFields: []
shbada commented 2 years ago

해당 필드는 빈 List 로 초기화 돼서 들어가 있기 때문에, @JsonInclude 같은 어노테이션으로 제외시키지 않는 이상 응답에 포함되지 않나요?

class CommonResponse<T> {

    private var result: Result? = null
    private var data: T? = null
    private var message: String? = null
    private var errorCode: String? = null
    private var errorFields: List<ErrorField>? = emptyList()

아 여기서 emptyList()를 해주고 있네여! 그럼 제가 말했던 오류 상황은 발생하지 않을거같아요~ 근데 클라이언트에서는 errorFields 존재 여부를 체크해서 있는지를 봐야하지 않나요?? errorCode로 분별 가능할까여? data에 넣지않고 errorFields 가 별도로 있을 이유가 있는지 궁금합니다~ validation 을 List로 담아서 data에 넣어보내도 되지 않나?라는 생각이 들어서요!

BAEKJungHo commented 2 years ago

validation 을 List로 담아서 data에 넣어보내도 되지 않나?라는 생각이 들어서요!

음 그래도 상관은 없을 것 같은데, errorFields 가 있다고하면 명시적으로 ValidationError 구나라는 것을 알 것 같아서 넣어뒀어요,

어차피 클라이언트에서 관심 깊게 봐야하는 거는 errorCode 이기 때문에 data 에 넣어서 처리할 지, 명시적으로 errorFields 를 따로 뺄지는, 응답 스펙을 어떻게 정하냐에 따라서 다를 것 같아요.