80000Coding / 80000Coding-Backend

MIT License
2 stars 0 forks source link

게시판 공통 응답 및 에러 핸들링 적용과 User Token 인증 방식의 서비스 변경 #44

Closed dnjsals45 closed 11 months ago

dnjsals45 commented 1 year ago

작업 이유

Content 및 Comment 서비스 공통 응답 및 에러 핸들링 적용. User Token 인증 방식의 서비스 연동

작업 사항

[변경 사항] 비회원유저도 사용할 수 있는 Get 방식을 제외하고, 나머지 부분에 인증된 유저만 처리할 수 있게 적용했습니다. 또한, 현재 GET/POST/PATH/DELETE 모두 같은 "" Path를 사용하고 있었는데, Get 요청 방식만 SecurityConfig 및 Jwt토큰 필터를 통과시키기 위해 분리해야할 필요성을 느꼈습니다. 이 부분은 일단 임의대로 /get, /post, /update, /delete로 구분했습니다. 프론트 측과 이야기를 나누고 수정하도록 하겠습니다.

[요청 사항] 추가로, 에러 핸들링 부분에서 GlobalErrorException에 포함하기에는 범위가 한정적인 것 같아서, Content와 Comment 별개로 작성하였는데, 파일의 위치나 다른 부분에서 합치는게 좋을 것 같은지 의견 말씀해주시면 반영하도록 하겠습니다.

이슈 연결

ChanHoHan commented 11 months ago

혹시 SuccessResponse가 Service에서 사용되고 있는데, Controller에서 사용하도록 통일하는건 어떨까요? API Response 스펙으로서, 클라이언트에게 직접적으로 노출되는 부분이라, 컨트롤러에서 사용하는게 좀 더 괜찮지 않나 생각이 들어서요

ChanHoHan commented 11 months ago

저는 지금까지 Custom Exception을 하나 만들고, RestControllerAdvice 및 ExceptionHandler 를 한 파일에 둬서 해당 Exception 및 다른 에러들(예를들면 RuntimeException, IllegalArgumentException, Exception등)을 한 곳에서 처리했었습니다. 에러코드도 추가될 때 마다 한 파일에서 추가했습니다.

어플리케이션에서 발생한 에러는 throw new 공통Exception(코드) 와 같이 예외를 던졌구요.

성민님께서 작성하신 코드를 보고, 에러 코드를 각 도메인이 관리하는 것도 좋다고 느꼈습니다. 그렇다면 에러코드는 각 도메인 패키지 아래에 두고, CommentErrorException에서 public CommentErrorException(CommentErrorCode errorCode)를 public PalcoErrorException(StatusCode errorCode) 이런식으로 바꿔서 공통으로 관리하는건 어떻게 생각하시나요? (handler도 마찬가지)

dnjsals45 commented 11 months ago

혹시 SuccessResponse가 Service에서 사용되고 있는데, Controller에서 사용하도록 통일하는건 어떨까요? API Response 스펙으로서, 클라이언트에게 직접적으로 노출되는 부분이라, 컨트롤러에서 사용하는게 좀 더 괜찮지 않나 생각이 들어서요

아 굳이 ResponseEntity를 통해서 작성될 필요가 없었네요.. 말씀해주신 부분이 더 맞는 것 같습니다 수정하도록 하겠습니다

@GetMapping("/get") public SuccessResponse GetContent(@RequestParam Long contentId) { return SuccessResponse.from(contentService.getContent(contentId)); } 이러한 형태로 생각을 했는데 제가 말씀해주신 부분 알맞게 생각한 거 맞죠?

dnjsals45 commented 11 months ago

저는 지금까지 Custom Exception을 하나 만들고, RestControllerAdvice 및 ExceptionHandler 를 한 파일에 둬서 해당 Exception 및 다른 에러들(예를들면 RuntimeException, IllegalArgumentException, Exception등)을 한 곳에서 처리했었습니다. 에러코드도 추가될 때 마다 한 파일에서 추가했습니다.

어플리케이션에서 발생한 에러는 throw new 공통Exception(코드) 와 같이 예외를 던졌구요.

성민님께서 작성하신 코드를 보고, 에러 코드를 각 도메인이 관리하는 것도 좋다고 느꼈습니다. 그렇다면 에러코드는 각 도메인 패키지 아래에 두고, CommentErrorException에서 public CommentErrorException(CommentErrorCode errorCode)를 public PalcoErrorException(StatusCode errorCode) 이런식으로 바꿔서 공통으로 관리하는건 어떻게 생각하시나요? (handler도 마찬가지)

저는 공통으로 관리하는 것 좋은 것 같습니다. ErrorCode로 어차피 구분이 되고 불필요한 파일을 줄일 수 있을 것 같습니다.(exception 폴더 3개의 파일 -> 1개의 ErroCode 파일)

ChanHoHan commented 11 months ago

혹시 SuccessResponse가 Service에서 사용되고 있는데, Controller에서 사용하도록 통일하는건 어떨까요? API Response 스펙으로서, 클라이언트에게 직접적으로 노출되는 부분이라, 컨트롤러에서 사용하는게 좀 더 괜찮지 않나 생각이 들어서요

아 굳이 ResponseEntity를 통해서 작성될 필요가 없었네요.. 말씀해주신 부분이 더 맞는 것 같습니다 수정하도록 하겠습니다

@GetMapping("/get") public SuccessResponse GetContent(@RequestParam Long contentId) { return SuccessResponse.from(contentService.getContent(contentId)); } 이러한 형태로 생각을 했는데 제가 말씀해주신 부분 알맞게 생각한 거 맞죠?

답변이 늦었네요 죄송합니다 ㅠㅠ 제가 말씀드린건 서비스에서 SuccessResponse를 생성하는것을 말씀드린거긴 합니다.

// ContentSerivce 43라인
        return SuccessResponse.from(GetContentRes.from(content));

요 부분에서는 그대로 GetContentRes.from(content)를 리턴하고, 컨트롤러에서 저 리턴값을 받아서 SuccessResponse객체를 만드는걸 생각했었습니다.