-
Notifications
You must be signed in to change notification settings - Fork 2
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/#462 닉네임 변경 API 구현 #470
base: main
Are you sure you want to change the base?
Changes from 2 commits
9ef006e
95bd4c7
ee8bc8a
3d14050
ded41a5
d52d13c
9bfb040
621a625
4685592
5a64990
2847c8d
05fdcf1
514c475
f0e54f9
04af872
3d46182
72ea86a
d9115bf
21358a7
8ca6f07
9c6fc11
21f4d60
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,8 +20,8 @@ public class AuthConfig implements WebMvcConfigurer { | |
private final TokenInterceptor tokenInterceptor; | ||
|
||
public AuthConfig(final AuthArgumentResolver authArgumentResolver, | ||
final LoginCheckerInterceptor loginCheckerInterceptor, | ||
final TokenInterceptor tokenInterceptor | ||
final LoginCheckerInterceptor loginCheckerInterceptor, | ||
final TokenInterceptor tokenInterceptor | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (╯‵□′)╯︵┻━┻ (╯°□°)╯︵ ┻━┻ |
||
) { | ||
this.authArgumentResolver = authArgumentResolver; | ||
this.loginCheckerInterceptor = loginCheckerInterceptor; | ||
|
@@ -45,7 +45,8 @@ private HandlerInterceptor tokenInterceptor() { | |
.includePathPattern("/songs/*/parts/*/likes", PathMethod.PUT) | ||
.includePathPattern("/voting-songs/*/parts", PathMethod.POST) | ||
.includePathPattern("/songs/*/parts/*/comments", PathMethod.POST) | ||
.includePathPattern("/members/*", PathMethod.DELETE); | ||
.includePathPattern("/members/*", PathMethod.DELETE) | ||
.includePathPattern("/members/*/nickname", PathMethod.PATCH); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍👍 |
||
} | ||
|
||
@Override | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ public enum ErrorCode { | |
NOT_FOUND_OAUTH(1010, "현재 지원하지 않는 OAuth 요청입니다."), | ||
INVALID_REFRESH_TOKEN(1011, "존재하지 않는 refreshToken 입니다."), | ||
TOKEN_PAIR_NOT_MATCHING_EXCEPTION(1012, "올바르지 않은 TokenPair 입니다."), | ||
ALREADY_EXIST_NICKNAME(1013, "존재하는 닉네임입니다."), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 스플릿: NICKNAME_ALREADY_EXIST |
||
|
||
// 2000: 킬링파트 - 좋아요, 댓글 | ||
|
||
|
@@ -60,7 +61,7 @@ public enum ErrorCode { | |
VOTING_PART_END_OVER_SONG_LENGTH(4003, "파트의 끝 초는 노래 길이를 초과할 수 없습니다."), | ||
INVALID_VOTING_PART_LENGTH(4004, "파트의 길이는 5, 10, 15초 중 하나여야합니다."), | ||
VOTING_PART_DUPLICATE_START_AND_LENGTH_EXCEPTION(4005, | ||
"한 노래에 동일한 파트를 두 개 이상 등록할 수 없습니다."), | ||
"한 노래에 동일한 파트를 두 개 이상 등록할 수 없습니다."), | ||
VOTING_SONG_PART_NOT_EXIST(4006, "투표 대상 파트가 존재하지 않습니다."), | ||
|
||
VOTING_SONG_PART_FOR_OTHER_SONG(4007, "해당 파트는 다른 노래의 파트입니다."), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,8 +6,12 @@ | |
import lombok.RequiredArgsConstructor; | ||
import org.springframework.stereotype.Service; | ||
import org.springframework.transaction.annotation.Transactional; | ||
import shook.shook.auth.application.TokenProvider; | ||
import shook.shook.auth.application.dto.TokenPair; | ||
import shook.shook.auth.exception.AuthorizationException; | ||
import shook.shook.auth.repository.InMemoryTokenPairRepository; | ||
import shook.shook.auth.ui.argumentresolver.MemberInfo; | ||
import shook.shook.member.application.dto.NicknameUpdateRequest; | ||
import shook.shook.member.domain.Email; | ||
import shook.shook.member.domain.Member; | ||
import shook.shook.member.domain.Nickname; | ||
|
@@ -27,6 +31,8 @@ public class MemberService { | |
private final MemberRepository memberRepository; | ||
private final KillingPartCommentRepository commentRepository; | ||
private final KillingPartLikeRepository likeRepository; | ||
private final TokenProvider tokenProvider; | ||
private final InMemoryTokenPairRepository inMemoryTokenPairRepository; | ||
|
||
@Transactional | ||
public Member register(final String email) { | ||
|
@@ -55,19 +61,25 @@ public Member findByIdAndNicknameThrowIfNotExist(final Long id, final Nickname n | |
|
||
@Transactional | ||
public void deleteById(final Long id, final MemberInfo memberInfo) { | ||
final long requestMemberId = memberInfo.getMemberId(); | ||
final Member requestMember = findById(requestMemberId); | ||
final Member targetMember = findById(id); | ||
validateMemberAuthentication(requestMember, targetMember); | ||
final Member member = getMemberIfValidRequest(id, memberInfo); | ||
|
||
final List<KillingPartLike> membersExistLikes = likeRepository.findAllByMemberAndIsDeleted( | ||
targetMember, | ||
member, | ||
false | ||
); | ||
|
||
membersExistLikes.forEach(KillingPartLike::updateDeletion); | ||
commentRepository.deleteAllByMember(targetMember); | ||
memberRepository.delete(targetMember); | ||
commentRepository.deleteAllByMember(member); | ||
memberRepository.delete(member); | ||
} | ||
|
||
private Member getMemberIfValidRequest(final Long memberId, final MemberInfo memberInfo) { | ||
final long requestMemberId = memberInfo.getMemberId(); | ||
final Member requestMember = findById(requestMemberId); | ||
final Member targetMember = findById(memberId); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 부분에서 현재 request의 Path로 넘어온 id와 arguemntResolver로 넘어온 id로 member를 DB에서 불러온 뒤에 같은지 비교를 하고 있는데 현재 member의 경우 equal&hash가 id로만 재정의 되어 있습니다. 따라서 db에서 member를 불러오기에 앞서서 memberId와 requestMemberId를 비교하고 그 다음에 실제 존재하는 member인지 검증하기 위해 DB로 요청을 보내면 db로 보내는 요청횟수를 줄일 수 있을 것 같습니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 오 굉장히 합리적인 방법이네요 |
||
|
||
validateMemberAuthentication(requestMember, targetMember); | ||
return targetMember; | ||
} | ||
|
||
private Member findById(final Long id) { | ||
|
@@ -77,8 +89,7 @@ private Member findById(final Long id) { | |
)); | ||
} | ||
|
||
private void validateMemberAuthentication(final Member requestMember, | ||
final Member targetMember) { | ||
private void validateMemberAuthentication(final Member requestMember, final Member targetMember) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 메서드 순서 변경이 필요해 보입니다! 이 메서드를 위로 올리는 것은 어떨까요? |
||
if (!requestMember.equals(targetMember)) { | ||
throw new AuthorizationException.UnauthenticatedException( | ||
Map.of( | ||
|
@@ -88,4 +99,45 @@ private void validateMemberAuthentication(final Member requestMember, | |
); | ||
} | ||
} | ||
|
||
@Transactional | ||
public TokenPair updateNickname(final Long memberId, final MemberInfo memberInfo, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 닉네임 변경으로 인해 토큰 재발급이 일어나고, 응답이 반환되는 보안 문제가 걱정된다면 닉네임 같은 개인정보를 바꾼 후에 재로그인을 요청하는 것은 어떨까요? 닉네임이 변경되면 서버 내부적으로 TOKEN을 삭제 -> 로그인이 만료된 것 처럼 취급이 되어서 프론트엔드에서 자연스럽게 재로그인을 요청하도록 하는 것은 어떤가요? |
||
final NicknameUpdateRequest request) { | ||
final Member member = getMemberIfValidRequest(memberId, memberInfo); | ||
final Nickname nickname = new Nickname(request.getNickname()); | ||
|
||
if (isSameNickname(member, nickname)) { | ||
return null; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 기존에 사용했던 닉네임으로 재변경 할 때
|
||
|
||
validateDuplicateNickname(nickname); | ||
member.updateNickname(nickname.getValue()); | ||
memberRepository.save(member); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 부분 jpa 더티채킹을 통해서 값이 수정되어서 memberRespository.save()를 하지 않아도 될 것 같습니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 좋은 지적입니다 👍🏻 |
||
return reissueTokenPair(member.getId(), member.getNickname()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 여기 return 문 위에 개행이 필요합니다. ~ (아코: 왜 추가 안하셨죠?) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 개행 추가 완!!!!!입니다 |
||
} | ||
|
||
private TokenPair reissueTokenPair(final Long memberId, final String nickname) { | ||
final String reissuedAccessToken = tokenProvider.createAccessToken(memberId, nickname); | ||
final String reissuedRefreshToken = tokenProvider.createRefreshToken(memberId, nickname); | ||
inMemoryTokenPairRepository.addOrUpdateTokenPair(reissuedRefreshToken, reissuedAccessToken); | ||
return new TokenPair(reissuedAccessToken, reissuedRefreshToken); | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 개행 (*  ̄︿ ̄) |
||
|
||
private boolean isSameNickname(final Member member, final Nickname nickname) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Member한테 물어보는 방향은 어떠신가요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Member 에게 동일한 닉네임을 가지고 있는지 물어보도록 메서드 추가했습니다. |
||
final String originalNickname = member.getNickname(); | ||
final String nicknameForUpdate = nickname.getValue(); | ||
|
||
return originalNickname.equals(nicknameForUpdate); | ||
} | ||
|
||
private void validateDuplicateNickname(final Nickname nickname) { | ||
final boolean isDuplicated = memberRepository.existsMemberByNickname(nickname); | ||
if (isDuplicated) { | ||
throw new MemberException.ExistNicknameException( | ||
Map.of("Nickname", nickname.getValue()) | ||
); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
package shook.shook.member.application.dto; | ||
|
||
import io.swagger.v3.oas.annotations.media.Schema; | ||
import jakarta.validation.constraints.NotBlank; | ||
import lombok.AllArgsConstructor; | ||
import lombok.Getter; | ||
import lombok.NoArgsConstructor; | ||
|
||
@Schema(description = "닉네임 변경 요청") | ||
@NoArgsConstructor(access = lombok.AccessLevel.PRIVATE) | ||
@AllArgsConstructor | ||
@Getter | ||
public class NicknameUpdateRequest { | ||
|
||
@Schema(description = "닉네임", example = "shookshook") | ||
@NotBlank | ||
private String nickname; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,6 +49,10 @@ public void updateNickname(final String newNickName) { | |
this.nickname = new Nickname(newNickName); | ||
} | ||
|
||
public void updateNickname(final Nickname newNickname) { | ||
this.nickname = newNickname; | ||
} | ||
Comment on lines
51
to
+54
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
public String getEmail() { | ||
return email.getValue(); | ||
} | ||
|
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.
뭐죠 이 애매한 개행..? w(゚Д゚)ww(゚Д゚)w