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

[FEAT] JWT 기반 소셜 로그인 기능 구현 #15

Merged
merged 20 commits into from
Mar 1, 2024
Merged

Conversation

orijoon98
Copy link
Member

@orijoon98 orijoon98 commented Feb 16, 2024

✒️ 관련 이슈번호

🔑 Key Changes

  1. 카카오, 애플 소셜 회원가입 / 로그인 API 추가
  2. 토큰 갱신 API 추가
  3. 로그아웃 API 추가
  4. 인증용 인터셉터, 리졸버 추가

📸 Screenshot

X

📢 To Reviewers

  • 생각보다 작업량이 많아요.. 커밋 단위로 보시는거 추천드립니다.
  • 작업하다 보니깐 발견한건데, 닉네임 중복 체크 API 도 추가되어야 할 것 같아요.
  • 그리고 로깅 하는거 한번 공통적으로 처리 필요해보입니다.
  • 보다보니깐 코틀린스럽지 않은 코드들을 많이 봤는데, 조금씩 개선해 나가면 좋을 것 같습니당!
  • JWT_SECRET 적용 추가되어서 노션에 환경 변수 업데이트해뒀습니당.

@orijoon98 orijoon98 added the ✨ Feat 기능 개발 label Feb 16, 2024
@orijoon98 orijoon98 self-assigned this Feb 16, 2024
Copy link
Member

@PgmJun PgmJun left a comment

Choose a reason for hiding this comment

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

수고하셨습니다! 리뷰를 몇 개 남겨보았는데 확인 부탁드려요~!

val socialId = appleTokenProvider.getSocialIdFromIdToken(request.token)
val member = MemberServiceUtils.findMemberBySocialIdAndSocialType(memberRepository, socialId, socialType)
member.updateFcmToken(request.fcmToken)
return member.id!!
Copy link
Member

Choose a reason for hiding this comment

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

!! 연산자를 통해 Not Nullable임을 명시하여 얻을 수 잇는 어떤 이점이 있을까요??

Copy link
Collaborator

Choose a reason for hiding this comment

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

@PgmJun Entity 에서 id 는 nullable 하게 구성되어야해서, 해당 응답은 저장된 Entity 라 id 값이 필수라 not null 명시한것같네요

혁준 이거 entity 에서 아래와 같은 패턴 쓰면 좀 더 가독성 좋을것같네요

@Entity
class Member (
 @Id
 var _id: String?
...
) {
  val id : String = _id ?: throw ~
..
}

Copy link
Member

Choose a reason for hiding this comment

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

@PgmJun Entity 에서 id 는 nullable 하게 구성되어야해서, 해당 응답은 저장된 Entity 라 id 값이 필수라 not null 명시한것같네요

오호 그렇겠네요!

Copy link
Member Author

Choose a reason for hiding this comment

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

getter 사용할 때 좀 더 편리할 수 있을 것 같긴하네요! 우선은 이거 빠르게 머지되어야 다른분들 작업할 수 있으니 이건 우선 스킵할게요

@orijoon98
Copy link
Member Author

최대한 리뷰 내용들 반영해봤어요 한번 확인 부탁드립니다~

@orijoon98 orijoon98 requested review from daehwan2da and PgmJun March 1, 2024 08:54
@orijoon98 orijoon98 merged commit 27d91da into develop Mar 1, 2024
2 checks passed
orijoon98 added a commit that referenced this pull request Mar 1, 2024
orijoon98 added a commit that referenced this pull request Mar 1, 2024
orijoon98 added a commit that referenced this pull request Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Feat 기능 개발
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEAT] JWT 기반 소셜 로그인 기능 구현
3 participants