Skip to content
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

[refac] BaseResponse, ExceptionHandler 수정 #32

Merged
merged 9 commits into from
Jul 9, 2024
Merged

[refac] BaseResponse, ExceptionHandler 수정 #32

merged 9 commits into from
Jul 9, 2024

Conversation

Parkjyun
Copy link
Contributor

@Parkjyun Parkjyun commented Jul 9, 2024

Related Issue 📌

close #8

Description ✔️

  • BaseResponse, ExceptionHandler 수정

To Reviewers

전체적인 구조 코드리뷰 부탁드립니다.

@Parkjyun Parkjyun added the enhancement New feature or request label Jul 9, 2024
@Parkjyun Parkjyun self-assigned this Jul 9, 2024
Copy link
Member

@kgy1008 kgy1008 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

filter 단에서 발생하는 에러도 hankkiResponse 응답구조로 메세지를 전송하게끔 잘 수정하신 것 같아요. 수고하셨습니다~

response.setContentType(MediaType.APPLICATION_JSON_VALUE);
response.setCharacterEncoding("utf-8");
response.setStatus(httpStatus.value());
PrintWriter writer = response.getWriter();
writer.write(objectMapper.writeValueAsString(BaseResponse.of(errorCode)));
writer.write(objectMapper.writeValueAsString(HankkiResponse.fail(authErrorCode)));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

굳굳

response.setContentType(MediaType.APPLICATION_JSON_VALUE);
response.setCharacterEncoding("utf-8");
response.setStatus(httpStatus.value());
PrintWriter writer = response.getWriter();
writer.write(objectMapper.writeValueAsString(BaseResponse.of(errorCode)));
writer.write(objectMapper.writeValueAsString(HankkiResponse.fail(authErrorCode)));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

굳!


@Getter
@RequiredArgsConstructor
public enum UserErrorCode implements ErrorCode {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

에러 상황별로 errorCode 클래스 나눠서 관리하는거 너무 좋습니당

@RequiredArgsConstructor(access = AccessLevel.PRIVATE)
public class HankkiResponse<T> {

private final int status;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1
저희 혹시 status는 code로 주기로 하지 않았나요??

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

반영했습니다.

@@ -52,6 +52,6 @@ private String getToken(final String refreshToken) {
if (refreshToken.startsWith(BEARER)) {
return refreshToken.substring(BEARER.length());
}
throw new UnauthorizedException(ErrorCode.INVALID_ACCESS_TOKEN);
throw new UnauthorizedException(AuthErrorCode.INVALID_ACCESS_TOKEN);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1
리프래시 토큰 검증 과정이라 INVALID_ACCESS_TOKEN은 적절하지 않은 것 같습다

@Parkjyun Parkjyun merged commit 7edf277 into develop Jul 9, 2024
1 check passed
kgy1008 pushed a commit that referenced this pull request Jul 10, 2024
* [refac] create interface for code

* [refac] refactor exception and common api response

* [feat] create response advice

* [refac] fix typo in UserErrorCode.java

* [refac] change Hankki response creation logic

* [refac] delete duplicated BaseTimeEntity.java

* [refac] fix HankkiResponse.java as api spec

* [refac] fix unappropriate error message while validating refreshtoken

* [refac] change typo in ResponseAdvice.java
@kgy1008 kgy1008 deleted the refac/8 branch December 2, 2024 05:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[refac] BaseResponse 변경 및 ExceptionHandler 수정
3 participants