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/#462 닉네임 변경 API 구현 #470

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
9ef006e
feat: 닉네임 변경 API 구현
Cyma-s Sep 26, 2023
95bd4c7
docs: API 명세 추가
Cyma-s Sep 26, 2023
ee8bc8a
style: 개행 추가 및 불필요한 공백 삭제, static import 삭제
Cyma-s Sep 27, 2023
3d14050
refactor: 에러 코드 이름 수정
Cyma-s Sep 27, 2023
ded41a5
refactor: 변수 이름 리팩터링
Cyma-s Sep 27, 2023
d52d13c
style: 헤더 위치 변경
Cyma-s Sep 27, 2023
9bfb040
test: 엣지 케이스를 테스트하도록 변경
Cyma-s Sep 27, 2023
621a625
refactor: member 에게 닉네임이 같은지 물어 보도록 메서드 변경
Cyma-s Sep 27, 2023
4685592
fix: @Valid 어노테이션 추가
Cyma-s Sep 27, 2023
5a64990
feat: 리프레시 토큰이 member id 로만 만들어지도록 수정
Cyma-s Sep 29, 2023
2847c8d
refactor: 닉네임 최소 길이 추가
Cyma-s Sep 29, 2023
05fdcf1
fix: cookie setPath 설정 변경
Cyma-s Sep 29, 2023
514c475
fix: cookie 테스트 수정
Cyma-s Sep 29, 2023
f0e54f9
refactor: 에러코드 문구 수정
Cyma-s Sep 30, 2023
04af872
fix: 닉네임 변경 시 응답을 내려주지 않도록 수정
Cyma-s Oct 3, 2023
3d46182
refactor: 토큰 검증 책임 ReissueController 로 이동
Cyma-s Oct 5, 2023
72ea86a
refactor: 검증 순서 변경
Cyma-s Oct 5, 2023
d9115bf
refactor: delete 메서드도 memberId 를 입력 받도록 변경
Cyma-s Oct 5, 2023
21358a7
refactor: 사용하지 않는 클래스 삭제
Cyma-s Oct 5, 2023
8ca6f07
refactor: 검증 로직 간소화
Cyma-s Oct 5, 2023
9c6fc11
refactor: save 문 삭제
Cyma-s Oct 5, 2023
21f4d60
refactor: TokenProvider, InMemoryTokenPairRepository 의존성 삭제
Cyma-s Oct 6, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public TokenPair oAuthLogin(final String oauthType, final String authorizationCo
}

public ReissueAccessTokenResponse reissueAccessTokenByRefreshToken(final String refreshToken,
final String accessToken) {
final String accessToken) {
Copy link
Collaborator

@somsom13 somsom13 Sep 27, 2023

Choose a reason for hiding this comment

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

뭐죠 이 애매한 개행..? w(゚Д゚)ww(゚Д゚)w

final Claims claims = tokenProvider.parseClaims(refreshToken);
final Long memberId = claims.get("memberId", Long.class);
final String nickname = claims.get("nickname", String.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

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

(╯‵□′)╯︵┻━┻ (╯°□°)╯︵ ┻━┻

) {
this.authArgumentResolver = authArgumentResolver;
this.loginCheckerInterceptor = loginCheckerInterceptor;
Expand All @@ -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);
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍👍

}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public ResponseEntity<LoginResponse> googleLogin(
final Cookie cookie = cookieProvider.createRefreshTokenCookie(tokenPair.getRefreshToken());
response.addCookie(cookie);
final LoginResponse loginResponse = new LoginResponse(tokenPair.getAccessToken());

return ResponseEntity.ok(loginResponse);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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, "존재하는 닉네임입니다."),
Copy link
Collaborator

Choose a reason for hiding this comment

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

스플릿: NICKNAME_ALREADY_EXIST
아코: DUPLICATE_NICKNAME
바론: 영문명은 상관없지만 "중복되는 닉네임입니다." 어떠신가요?


// 2000: 킬링파트 - 좋아요, 댓글

Expand Down Expand Up @@ -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, "해당 파트는 다른 노래의 파트입니다."),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Copy link
Collaborator

Choose a reason for hiding this comment

The 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로 보내는 요청횟수를 줄일 수 있을 것 같습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오 굉장히 합리적인 방법이네요
아코 말씀대로 id가 같은지 먼저 비교하고, member 를 찾아오는 로직으로 변경했습니다! 로직이 정말 간결해졌네요 👍🏻👍🏻 👍🏻 👍🏻 👍🏻 👍🏻


validateMemberAuthentication(requestMember, targetMember);
return targetMember;
}

private Member findById(final Long id) {
Expand All @@ -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) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

메서드 순서 변경이 필요해 보입니다! 이 메서드를 위로 올리는 것은 어떨까요?

if (!requestMember.equals(targetMember)) {
throw new AuthorizationException.UnauthenticatedException(
Map.of(
Expand All @@ -88,4 +99,45 @@ private void validateMemberAuthentication(final Member requestMember,
);
}
}

@Transactional
public TokenPair updateNickname(final Long memberId, final MemberInfo memberInfo,
Copy link
Collaborator

@somsom13 somsom13 Sep 27, 2023

Choose a reason for hiding this comment

The 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;
}
Copy link
Collaborator

@somsom13 somsom13 Sep 27, 2023

Choose a reason for hiding this comment

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

기존에 사용했던 닉네임으로 재변경 할 때

  1. 예외를 던진다
  2. 무시한다. (토큰 재발급을 막거나, 기존에 있던 값을 다시 반환한다.)
    중에 결정해야 할 것 같습니다. (프론트와 논의 필요)


validateDuplicateNickname(nickname);
member.updateNickname(nickname.getValue());
memberRepository.save(member);

Copy link
Collaborator

@seokhwan-an seokhwan-an Oct 3, 2023

Choose a reason for hiding this comment

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

이 부분 jpa 더티채킹을 통해서 값이 수정되어서 memberRespository.save()를 하지 않아도 될 것 같습니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

좋은 지적입니다 👍🏻

return reissueTokenPair(member.getId(), member.getNickname());
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기 return 문 위에 개행이 필요합니다. ~ (아코: 왜 추가 안하셨죠?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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);
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

개행 (*  ̄︿ ̄)


private boolean isSameNickname(final Member member, final Nickname nickname) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Member한테 물어보는 방향은 어떠신가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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;
}
4 changes: 4 additions & 0 deletions backend/src/main/java/shook/shook/member/domain/Member.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

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

의견)

  1. isSameNickname의 책임을 Member한테 넘기면서, 내부적으로 new Nickname() 을 통해 닉네임 형식 검증 + 중복 검증을 해보면 어떨까요?
  2. 그렇게 되면 이 Nickname 객체를 받는 메서드는 사라질 것 같습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

isSameNickname 의 검증 책임을 멤버에게 넘기려고 했는데, memberRepository 에서 중복을 확인할 때 Nickname 객체가 필요하더라고요. 그래서 해당 메서드는 그대로 두고, Nickname 객체는 외부에서 생성하여 검증하는 걸로 했습니다!


public String getEmail() {
return email.getValue();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
@Embeddable
public class Nickname {

private static final int NICKNAME_MAXIMUM_LENGTH = 100;
private static final int NICKNAME_MAXIMUM_LENGTH = 20;

@Column(name = "nickname", length = NICKNAME_MAXIMUM_LENGTH, nullable = false)
private String value;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,6 @@ public interface MemberRepository extends JpaRepository<Member, Long> {
Optional<Member> findByEmail(final Email email);

Optional<Member> findByIdAndNickname(final Long id, final Nickname nickname);

boolean existsMemberByNickname(final Nickname nickname);
}
Original file line number Diff line number Diff line change
Expand Up @@ -93,4 +93,15 @@ public MemberNotExistException(final Map<String, String> inputValuesByProperty)
super(ErrorCode.MEMBER_NOT_EXIST, inputValuesByProperty);
}
}

public static class ExistNicknameException extends MemberException {

public ExistNicknameException() {
super(ErrorCode.ALREADY_EXIST_NICKNAME);
}

public ExistNicknameException(final Map<String, String> inputValuesByProperty) {
super(ErrorCode.ALREADY_EXIST_NICKNAME, inputValuesByProperty);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,15 +1,23 @@
package shook.shook.member.ui;

import jakarta.servlet.http.Cookie;
import jakarta.servlet.http.HttpServletResponse;
import lombok.RequiredArgsConstructor;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.PatchMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;
import shook.shook.auth.application.dto.ReissueAccessTokenResponse;
import shook.shook.auth.application.dto.TokenPair;
import shook.shook.auth.ui.CookieProvider;
import shook.shook.auth.ui.argumentresolver.Authenticated;
import shook.shook.auth.ui.argumentresolver.MemberInfo;
import shook.shook.member.application.MemberService;
import shook.shook.member.application.dto.NicknameUpdateRequest;
import shook.shook.member.ui.openapi.MemberApi;

@RequiredArgsConstructor
Expand All @@ -18,6 +26,7 @@
public class MemberController implements MemberApi {

private final MemberService memberService;
private final CookieProvider cookieProvider;

@DeleteMapping
public ResponseEntity<Void> deleteMember(
Expand All @@ -28,4 +37,25 @@ public ResponseEntity<Void> deleteMember(

return ResponseEntity.status(HttpStatus.NO_CONTENT).build();
}

@PatchMapping("/nickname")
public ResponseEntity<ReissueAccessTokenResponse> updateNickname(
@PathVariable(name = "member_id") final Long memberId,
@Authenticated final MemberInfo memberInfo,
@RequestBody final NicknameUpdateRequest request,
final HttpServletResponse response
) {
final TokenPair tokenPair = memberService.updateNickname(memberId, memberInfo, request);

if (tokenPair == null) {
return ResponseEntity.noContent().build();
}

final Cookie cookie = cookieProvider.createRefreshTokenCookie(tokenPair.getRefreshToken());
response.addCookie(cookie);
final ReissueAccessTokenResponse reissueResponse = new ReissueAccessTokenResponse(
tokenPair.getAccessToken());

return ResponseEntity.ok(reissueResponse);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,20 @@

import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.Parameter;
import io.swagger.v3.oas.annotations.Parameters;
import io.swagger.v3.oas.annotations.responses.ApiResponse;
import io.swagger.v3.oas.annotations.responses.ApiResponses;
import io.swagger.v3.oas.annotations.tags.Tag;
import jakarta.servlet.http.HttpServletResponse;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.PatchMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RequestBody;
import shook.shook.auth.application.dto.ReissueAccessTokenResponse;
import shook.shook.auth.ui.argumentresolver.Authenticated;
import shook.shook.auth.ui.argumentresolver.MemberInfo;
import shook.shook.member.application.dto.NicknameUpdateRequest;

@Tag(name = "Member", description = "회원 관리 API")
public interface MemberApi {
Expand All @@ -31,4 +38,46 @@ ResponseEntity<Void> deleteMember(
@PathVariable(name = "member_id") final Long memberId,
@Authenticated final MemberInfo memberInfo
);

@Operation(
summary = "닉네임 변경",
description = "닉네임을 변경한다."
)
@ApiResponses(
value = {
@ApiResponse(
responseCode = "200",
description = "닉네임 변경 성공"
),
@ApiResponse(
responseCode = "204",
description = "동일한 닉네임으로 변경하여 변경된 닉네임이 없음"
),
@ApiResponse(
responseCode = "400",
description = "중복된 닉네임, 20자가 넘는 닉네임, 공백 닉네임의 이유로 닉네임 변경 실패"
)
}
)
@Parameters(
value = {
@Parameter(
name = "member_id",
description = "닉네임을 변경할 회원 id",
required = true
),
@Parameter(
name = "nickname",
description = "변경할 닉네임",
required = true
)
}
)
@PatchMapping("/nickname")
ResponseEntity<ReissueAccessTokenResponse> updateNickname(
@PathVariable(name = "member_id") final Long memberId,
@Authenticated final MemberInfo memberInfo,
@RequestBody final NicknameUpdateRequest request,
final HttpServletResponse response
);
}
Loading
Loading