[feat] 소셜 로그인 구현 #10
[feat] 소셜 로그인 구현 #10
Conversation
|
|
||
| @SpringBootApplication | ||
| @EnableFeignClients |
There was a problem hiding this comment.
@EnableFeignClients 어노테이션 여기에 붙이지 마시구 따로 config class 만들어주세요
| @Override | ||
| public boolean supportsParameter(MethodParameter parameter) { | ||
| boolean hasUserIdAnnotation = parameter.hasParameterAnnotation(UserId.class); | ||
| boolean isLongType = Long.class.isAssignableFrom(parameter.getParameterType()); |
There was a problem hiding this comment.
parameter.getParameterType().equals(Long.class);를 사용는 것은 어떨까요?
|
|
||
| @RestController | ||
| @RequiredArgsConstructor | ||
| @RequestMapping("api/v1") |
There was a problem hiding this comment.
/ 추가해주세요 /를 추가하지 않아도 정상작동하지만 가독성에 영향을 줍니다.
| import static org.hankki.hankkiserver.domain.user.model.MemberStatus.ACTIVE; | ||
| import static org.hankki.hankkiserver.domain.user.model.MemberStatus.INACTIVE; | ||
| import static org.hankki.hankkiserver.domain.user.model.Platform.*; | ||
| import static org.hankki.hankkiserver.domain.user.model.User.createUser; | ||
| import static org.hankki.hankkiserver.domain.user.model.UserInfo.createMemberInfo; | ||
| import static org.hankki.hankkiserver.auth.filter.JwtAuthenticationFilter.BEARER; | ||
|
|
There was a problem hiding this comment.
static import는 여러번 사용되는 변수, 함수에 대해서만 해주세요. 한번만 사용되는 변수 및 함수에 사용하면 오히려 코드의 가독성을 해친다 생각합니다.
| import org.springframework.web.bind.annotation.GetMapping; | ||
| import org.springframework.web.bind.annotation.RequestHeader; | ||
|
|
||
| @FeignClient(name = "${oauth.kakao.name}", url = "${oauth.kakao.url}") |
There was a problem hiding this comment.
해당 부분은 공개해도 되는 정보일 거 같은데 굳이 yml로 관리하는 이유가 있을까요?
There was a problem hiding this comment.
코드에 Url을 직접 넣는것 보다는 저렇게 작성하는 것이 심미적으로 좋다고 생각했습니다..
| private final UserRepository userRepository; | ||
| private final UserInfoRepository userInfoRepository; |
|
|
||
| @PostMapping("/auth/reissue") | ||
| public ResponseEntity<BaseResponse<?>> reissue( | ||
| @RequestHeader(HttpHeaders.AUTHORIZATION) final String refreshtoken) { |
|
|
||
| public void logOut(final Long userId) { | ||
| UserInfo findUserInfo = getUserInfo(userId); | ||
| updateRefreshToken(null, findUserInfo); |
There was a problem hiding this comment.
해당 부분은 굳이 함수로 빼야할까라는 생각이 드네용
There was a problem hiding this comment.
로그아웃을 할 경우 refreshToken 값을 null로 바꿔주는 것이 좋다고 생각하여 함수를 따로 만들어서 처리를 하였는데 재연님은 왜 그렇게 생각하셨는지 물어봐도 될까요?
| import org.springframework.data.jpa.repository.Modifying; | ||
| import org.springframework.data.jpa.repository.Query; | ||
| import org.springframework.data.repository.query.Param; | ||
| import org.springframework.transaction.annotation.Transactional; |
| userRepository.softDeleteById(userId, INACTIVE); | ||
| userInfoRepository.softDeleteByUserId(userId); | ||
| user.softDelete(INACTIVE); |
There was a problem hiding this comment.
softdeleteById에서 하는 역할을
user.softDelete(INACTIVE)에서도 하고 있는 것 같습니다.
deletetAt까지 설정해주는 user.softDelete()만 호출하면 될 것 같습니다.
위에서 이미 getuser를 호출해서 따로 save를 해주지 않아도 자동으로 디비 컬럼이 업데이트 됩니다.
그리고 softDelete가 이미 멤버의 상태를 inactive로 바꾼다는 의미를 내포하고 있으니 softDelete에서 굳이 MemberStatus를 인자로 받지 않고
this.memberStatus = INACTIVE;
this.deletedAt = LocalDateTime.now()만 호출하셔도 될 거 같앙ㅅ
Related Issue 📌
close #3
Description ✔️
To Reviewers
soft delete 다른 방법으로 구현하도록 수정해야할 것 같습니다..