-
Notifications
You must be signed in to change notification settings - Fork 0
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
feature: 회원 탈퇴 api #96
The head ref may contain hidden characters: "94-feature-\uD68C\uC6D0-\uD0C8\uD1F4-api"
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재는 봉달, 리프레시 토큰을 삭제하기 위해 Service 에서 각각의 Repository 를 직접 의존하고 있습니다. 혹시 spring event를 활용해 의존성을 끊고, 비동기 로직으로 리팩터링 하는 건 어떠신가요?
저는 좋습니다!! 👍👍
private fun anonymize() { | ||
nickname = DELETED_MEMBER_NAME | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
음 닉네임 자체를 바꿔버려도 괜찮겠죠?!...
생각해보니 db nickname 컬럼에 unique옵션이 없었네요! 이거 없어도 될까요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
닉네임도 개인정보로 간주될 수 있기 때문에 회원이 탈퇴할 경우 삭제하는 것이 마땅할 것 같습니다! 삭제한 회원에 대해 nickname을 null로 두거나 빈 문자열로 두면 어떨까 하네요.
db 에 unique 설정하는 것은 닉네임 정책이 구체적으로 나오면 하는 게 어떨까요?
@Modifying | ||
@Query("DELETE FROM CartProduct cp WHERE cp.memberId =:memberId") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deleteBy~ @Query
없어도 동작하지 않나요?? 붙이신 이유가 있으신가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deleteBy~ 를 사용하면 JPA가 내부적으로 entity를 한 개 찾고, delete 쿼리를 내보내는 것으로 알고 있습니다.
N+1 문제를 방지하기 위해 직접 쿼리를 작성했어요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
헛 그렇네요!! 몰랐어요... 배워갑니다👍
): T = findByIdOrNull(id) ?: throw e | ||
|
||
id: ID, | ||
exceptionSupplier: () -> Exception = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이렇게 바뀐거 좋네요!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다!
회원 탈퇴와 관련하여 데이터 정책이 까다로운데 잘 반영 하셨군요!
Kakao 연동과 관련하여 코멘트 남겨두었습니다.
또, 재가입 관련해서 로직이 필요할 것 같아요.
현재 oAuthId
와 oAuthServiceType
으로 [최초 로그인/회원 로그인]을 구분하고 있는 부분에서 탈퇴한 회원(isDeleted)도 기존 회원으로 조회 될 것 같아요.
AuthService.kt login()
에 수정이 필요해 보입니다!
memberService.deleteBy(loginMember.memberId) | ||
return ResponseEntity.noContent().build() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kakao 로그인 연결 끊기도 해야 할 것 같아요..!
Test Results 35 files +1 35 suites +1 13s ⏱️ -1s Results for commit b180247. ± Comparison against base commit 4ca1b76. This pull request removes 3 and adds 10 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
361f815
to
2caf80a
Compare
회원 탈퇴 재리뷰 요청 드립니다! 작업 내용카카오 연결 끊기회원 탈퇴 시 Oauth 서버에서도 연결을 끊어주도록 했습니다. 공식 문서를 참고해 서버 내에서 api 요청을 보내도록 했습니다. 카카오 공식문서에서 소개하는 연결 끊기(회원 탈퇴) 방법은 두 가지 입니다.
두 가지 방법 중 첫 번째 방법을 사용했습니다. 그 이유는 다른 OAuth 서비스들이 첫 번째 방법만 제공하고 있기 때문입니다. 대표적으로 Naver, Google 은 accessToken 을 통해서 회원 탈퇴를 할 수 있도록 안내하고 있습니다. 현재는 기획 상 Kakao login 만 사용하고 있지만, 추후 다른 OAuth 서비스를 사용하게 될 경우를 대비해 공통의 방법을 사용하는 것이 좋겠다고 판단했습니다. 카카오 토큰 갱신회원 탈퇴 시 사용하는 accessToken 은 카카오에서 발급하는 accessToken 입니다. 이 accessToken은 발급 후 21599초(약 6시간)이 지나면 만료됩니다. 만료된 accessToken 으로는 카카오의 회원 탈퇴 등 api 를 사용할 수 없습니다. 따라서 카카오의 accessToken을 적절히 갱신해주어야 합니다. 마찬가지로 공식 문서는 refreshToken 을 활용한 토큰 갱신 방법을 안내하고 있습니다. 문서를 참고해 카카오에 api 요청을 보내기 전, Member의 accessToken이 만료되었는지 확인한 후 토큰을 갱신하는 로직을 추가했습니다. 카카오 토큰 갱신은 두 가지 경우에 사용됩니다.
외부 api 트랜잭션 분리카카오 로그인에 관련한 외부 api 를 트랜잭션에서 분리하는 작업을 했습니다. 외부 api 가 트랜잭션 내에 포함되면 다음과 같은 문제를 야기할 수 있습니다.
따라서 외부 api 작업을 트랜잭션 내에서 분리했습니다. 로그인 작업을 예로 들어 설명하겠습니다. 기존에는 외부 api 요청 로직이 트랜잭션 내부에 포함되어 있었습니다.
트랜잭션을 분리하기 위해 Facade 패턴을 활용해 OauthService, AuthService 를 나누었습니다. 그리고 외부 api 관련 작업을 OauthService, 트랜잭션 관련 작업을 AuthService 에게 할당했습니다. 전체적인 로직과 트랜잭션을 아래와 같이 수정했습니다.
회원 삭제 시 개인정보 익명화회원 탈퇴 시 개인정보를 적절히 처리해야 합니다. 회원이 사용했던 닉네임, 카카오 oauthId, 프로필 사진 등은 개인을 특정할 수 있어 개인정보로 간주될 수 있습니다. 따라서 회원 탈퇴 시 이러한 정보를 익명화하도록 변경했습니다. @Entity
class Member(
@Id @GeneratedValue(strategy = GenerationType.IDENTITY)
val id: Long = 0L,
@Column(nullable = false)
var oauthId: Long,
@Column(nullable = false)
val oauthServerNumber: Int,
// (...)
@Column(nullable = false)
var isDeleted: Boolean = false,
var oauthAccessToken: String = "",
var expireAt: LocalDateTime? = null,
var oauthRefreshToken: String = "",
) : BaseEntity(), SoftDeleteEntity {
fun delete() {
isDeleted = true
anonymize()
}
private fun anonymize() {
oauthId = DELETED_OAUTH_ID
nickname = DELETED_MEMBER_NAME
profileImageUrl = null
oauthAccessToken = DELETED_AUTH_FIELD
expireAt = null
oauthRefreshToken = DELETED_AUTH_FIELD
}
// (...)
}
기타event 로 refreshToken, cartProduct 등의 삭제 로직을 분리하는 작업은 우선순위가 높다고 생각하지 않아서 미뤘습니다. 리뷰 부탁드립니다~ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다!
탈퇴 정책이랑 리뷰 반영도 잘 되어있네요!
Facade로 트랜잭션 범위 줄이는 것도 너무 좋아요.
변경 할 사항은 없는 것 같고.. 궁금한 사항들 코멘트로 남겨두었습니다.
고생하셨어요!👍👍
val tokenRequestBody = HashMap<String, String>() | ||
tokenRequestBody["grant_type"] = "authorization_code" | ||
tokenRequestBody["client_id"] = kakaoOauthProperties.clientId | ||
tokenRequestBody["redirect_uri"] = kakaoOauthProperties.redirectUri | ||
tokenRequestBody["code"] = code | ||
tokenRequestBody["client_secret"] = kakaoOauthProperties.clientSecret |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아하?!!
) | ||
) : BaseEntity() { | ||
|
||
fun validateToken(other: String) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
토큰이 같은지 다른지만 비교하는데 다른 이름은 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이름 짓기가 애매해서 validateToken 으로 설정했습니다. 다시 생각해도 잘 안 떠오르네요. 혹시 추천하시는 이름 있으실까요?
그나마 낫다고 판단되는 validateTokenValue
으로 바꿔두었어요!
|
||
@Entity | ||
class Member( | ||
@Id @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
val id: Long = 0L, | ||
|
||
@Column(nullable = false) | ||
val oauthId: String, | ||
var oauthId: Long, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Long타입으로 수정된 이유가 궁금합니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
공식문서에서 회원 번호가 Long 타입으로 반환되길래 Long으로 바꾸었습니다! Long으로 반환되는 것을 굳이 String 으로 사용할 이유가 없다고 생각했어요
https://developers.kakao.com/docs/latest/ko/kakaologin/rest-api#req-user-info
private fun anonymize() { | ||
oauthId = DELETED_OAUTH_ID | ||
nickname = DELETED_MEMBER_NAME | ||
profileImageUrl = null | ||
oauthAccessToken = DELETED_AUTH_FIELD | ||
expireAt = null | ||
oauthRefreshToken = DELETED_AUTH_FIELD | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
필드 비우는거 좋네요!
닉네임은 빈값이면 뷰에서 "탈퇴한 회원"으로 표기하는 거겠죠..?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네 저는 그렇게 생각했습니다!
변경되기 쉬운 값이라 view 에서 관리하는 게 맞다고 생각했어요. 동시에 데이터베이스에 의미없는 데이터를 넣어둘 필요가 없다고 생각했어요.
val accessTokenClaims = AccessTokenClaims.from(jwtProvider.getPayload(token)) | ||
memberRepository.existActiveByIdOrThrow(accessTokenClaims.memberId) { | ||
MemberException(NOT_FOUND_MEMBER) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋아요~!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR 설명 미슐랭이네요
외부 api랑 트랜잭션 나눈 거 너무 좋네요~~👍👍👍
어프루브합니다~~
|
||
var oauthAccessToken: String = "", | ||
|
||
var expireAt: LocalDateTime? = null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oauthAccessTokenExpiredAt
은 너무 길까요? ㅠㅠ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oauthAccessTokenExpiresAt
으로 바꿨어요!
} | ||
} | ||
|
||
fun updateToken( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updateOauthToken
너무 구질구질한가요?
아 로그인 정보를 담은 회원, 서비스 정보를 담은 회원 둘로 나누는 작업도 미뤘습니다.
-> oauthToken정보는 나중에 Member
에서 분리될 것 같아서 메서드 이름에 oauth가 없는건가요?!
일단은 이름에 oauth가 들어가길 희망해봅니다... ㅎㅎ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oauth 붙이는 게 더 명확해서 좋네요! 붙이도록 하겠습니다.
oauthToken정보는 나중에 Member에서 분리될 것 같아서 메서드 이름에 oauth가 없는건가요?!
이 부분은 딱히 고려하지 않았습니다!
oauthServerType = member.oauthServerType, | ||
oauthRefreshToken = member.oauthRefreshToken | ||
) | ||
authService.updateOauthToken(member, oauthTokenInfo) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
바로 member.updateToken() 해버리는건 어떤가요? 서비스 메서드로 분리하지 않아도 될 것 같아서요!
member.updateToken(
accessToken = oauthTokenInfo.accessToken,
expiresIn = oauthTokenInfo.expiresIn,
refreshToken = oauthTokenInfo.refreshToken
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 메서드가 트랜잭션 내에 속해있지 않아서, member.updateToken() 를 하더라도 JPA의 변경감지가 이뤄지지 않습니다! 그리고 Entity의 내부 값을 바꾸는 로직은 authService 에서 하는 것이 트랜잭션 구조를 파악하기 용이하다고 생각해요!!
혹시 제가 잘못 알고 있다면 말씀해주세요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아 그렇네요!!!?
외부 api까지 합친 facadeService라 트랜잭셔널이 없는데... 생각을 못했어요!!!
죄송합니당
📌 관련 이슈
📁 작업 설명
회원 관련 데이터
회원 탈퇴 시 고려해야할 요소를 크게 보면 다음과 같습니다.
찜
,장바구니
,후기
,후기 추천
,리프레시 토큰
각 요소들에 대한 정책을 차례로 정리하면 이렇습니다.
이러한 정책을 코드로 옮겨본다면
찜
,상품 후기 추천
에 대해서는 작업할 것이 없습니다.상품 후기
에 대해서는 닉네임을 달리 표현하도록 바꾸어 주어야 합니다.봉달
,리프레시 토큰
은 삭제해주어야 합니다.탈퇴한 회원의 상품 후기 닉네임 변경
먼저 상품 후기 닉네임을 어떻게 변경했는지 설명드리겠습니다.
ProductReview
현재 ProductReview 는 회원의 닉네임 정보를 직접 갖지 않고 Member 테이블을 참조하고 있습니다.
이때 두 가지 방법으로 회원의 닉네임을 변경할 수 있습니다.
1. 회원 탈퇴 시 Member 테이블의 nickname 을 직접 변경
Member entity 의 nickname 값을 실제로 변경하는 방법입니다.
Member.delete() 메서드를 호출했을 때 Member의 isDeleted와 nickname 이 함께 변경됩니다.
2. ProductReview 조회 시 탈퇴한 회원에 대한 nickname 응답값 변경
Member entity 의 nickname 은 그대로 두고, 응답에서만 nickname 을 다르게 반환하는 방법입니다. Repository 에서 아래와 같은 로직을 추가해서 구현할 수 있습니다.
두 가지 방법 중 전자의 방법으로 구현해두었습니다. 탈퇴한 회원에 대해 nickname 을 저장해 둘 이유가 없다고 생각했기 때문입니다.
봉달, 리프레시 토큰 삭제
CartProduct
RefreshToken
봉달과 리프레스 토큰 table 모두 memberId 를 갖습니다. 따라서 memberId를 통해 데이터를 삭제하도록 했습니다.
의견 묻기
현재는 봉달, 리프레시 토큰을 삭제하기 위해 Service 에서 각각의 Repository 를 직접 의존하고 있습니다. 혹시 spring event를 활용해 의존성을 끊고, 비동기 로직으로 리팩터링 하는 건 어떠신가요?
참고
삭제되지 않은 회원이 존재하는지
를 검증하도록 했습니다. Service 마다 중복되었던 코드를 줄이고, 빠른 예외를 던질 수 있는 방법이라 생각했습니다.