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

Feat/#462 닉네임 변경 API 구현 #470

wants to merge 22 commits into from

Conversation

Cyma-s
Copy link
Collaborator

@Cyma-s Cyma-s commented Sep 26, 2023

📝작업 내용

닉네임 변경 API를 구현한다.

💬리뷰 참고사항

  • 닉네임이 토큰 발행 시 필요하기 때문에, 기존에 존재하던 토큰을 없애고 토큰을 추가하는 것이 맞지만 현재는 기존 토큰을 없애지 않고 새로 발행된 토큰과 accessToken 을 추가하는 것으로 했습니다.
  • 동일한 닉네임으로 변경하려 하는 경우, 토큰을 전달하면 안 되기 때문에 TokenPair 를 null 로 반환했습니다. TokenPair 가 null 인 경우 204 상태코드를 반환합니다.
  • 중복 닉네임, 공백 닉네임을 Bad Request 로 간주했습니다.
  • reissueTokenPair 가 MemberService 에 존재하는 이유는 AuthService 에 MemberService 가 이미 존재하여 MemberService 에서 AuthService 의존이 생기면 순환 참조가 발생하기 때문입니다.

2차 리뷰 참고 사항

  • 리프레시 토큰이 다시 닉네임 정보를 갖도록 수정했습니다.
  • updateNickname 은 boolean 을 리턴하며, API 에서는 응답 코드만 클라이언트로 전달됩니다. 이는 현재 닉네임 변경이 완료되면 클라이언트에서 리다이렉트하기로 결정했기 때문에, 따로 응답이 필요없을 것 같아 우코와 합의 하에 정했습니다. 즉, 액세스 토큰은 닉네임 변경 시 발급되지 않습니다.
  • 중복 코드를 줄이기 위해 TokenService 를 분리했습니다.

#️⃣연관된 이슈

closes #462

@Cyma-s Cyma-s added [ 🌙 BE ] 백엔드 크루들의 멋진 개발 이야기 하나둘셋 야! ✨ Feat 꼼꼼한 기능 구현 중요하죠 labels Sep 26, 2023
@Cyma-s Cyma-s self-assigned this Sep 26, 2023
@github-actions
Copy link

github-actions bot commented Sep 26, 2023

Unit Test Results

  87 files    87 suites   12s ⏱️
344 tests 344 ✔️ 0 💤 0
347 runs  347 ✔️ 0 💤 0

Results for commit 04af872.

♻️ This comment has been updated with latest results.

@somsom13
Copy link
Collaborator

리뷰 참고 사항 중 1번 질문은 변경된 닉네임으로 저장소에서 refreshToken을 지우는 방향으로 구현해보는 것은 어떨까요?

@@ -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

Comment on lines 23 to 24
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.

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

@@ -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
바론: 영문명은 상관없지만 "중복되는 닉네임입니다." 어떠신가요?

@@ -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.

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

Comment on lines 109 to 111
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. 무시한다. (토큰 재발급을 막거나, 기존에 있던 값을 다시 반환한다.)
    중에 결정해야 할 것 같습니다. (프론트와 논의 필요)

@@ -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을 삭제 -> 로그인이 만료된 것 처럼 취급이 되어서 프론트엔드에서 자연스럽게 재로그인을 요청하도록 하는 것은 어떤가요?

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.

개행 (*  ̄︿ ̄)

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

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 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 에게 동일한 닉네임을 가지고 있는지 물어보도록 메서드 추가했습니다.

Comment on lines 51 to +54

public void updateNickname(final Nickname newNickname) {
this.nickname = newNickname;
}
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 객체는 외부에서 생성하여 검증하는 걸로 했습니다!

@@ -40,11 +50,17 @@ class MemberServiceTest extends UsingJpaTest {
@Autowired
private KillingPartLikeRepository likeRepository;

private TokenProvider tokenProvider;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Autowired 로 변경할 수 있을 것 같아요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아쉽게도 UsingJpaTest 라서 @Autowired 로는 변경할 수 없었습니다 😭

@@ -152,7 +168,7 @@ void success_delete() {

saveAndClearEntityManager();
// when
memberService.deleteById(targetId, new MemberInfo(targetId, Authority.MEMBER));
memberService.deleteById(targetId, new MemberInfo(targetId, MEMBER));
Copy link
Collaborator

Choose a reason for hiding this comment

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

static import가 되어버렸네요~

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

static import 제거완입니다 👍

@@ -169,7 +185,7 @@ void fail_delete() {

// when, then
assertThatThrownBy(() ->
memberService.deleteById(targetId, new MemberInfo(unsavedMemberId, Authority.MEMBER))
memberService.deleteById(targetId, new MemberInfo(unsavedMemberId, MEMBER))
Copy link
Collaborator

Choose a reason for hiding this comment

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

ditto

memberService.deleteById(targetMember.getId(),
new MemberInfo(requestMember.getId(), Authority.MEMBER))
memberService.deleteById(targetMember.getId(),
new MemberInfo(requestMember.getId(), MEMBER))
Copy link
Collaborator

Choose a reason for hiding this comment

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

di


// when
// then
assertThat(memberService.updateNickname(savedMember.getId(), new MemberInfo(savedMember.getId(), MEMBER),
Copy link
Collaborator

Choose a reason for hiding this comment

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

기왕 개행된거, , 로 된 거 전부 개행하는 것은 어떨까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

개행 완료했습니다!!!!

void fail_updateNickname_duplicate_nickname() {
// given
final Member newMember = memberRepository.save(new Member("temp@email", "shook2"));
final String newNickname = "shook"; // 중복된 닉네임
Copy link
Collaborator

Choose a reason for hiding this comment

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

변수명을 duplicateNickname 으로 하는건 어떠신가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

더 좋은 것 같네요 바로 변경했습니다 👍🏻

}

@DisplayName("닉네임이 빈 문자열이면 예외를 던진다.")
@ValueSource(strings = {"", " ", " ", "\r", "\n", "\t"})
Copy link
Collaborator

Choose a reason for hiding this comment

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

ヾ(⌐■_■)ノ♪

아코: Empty, Blank 어노테이션이 있다고 하네요.

@Test
void create_fail_lengthOver100() {
void create_fail_lengthOver20() {
//given
final String nickname = ".".repeat(101);
Copy link
Collaborator

Choose a reason for hiding this comment

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

엣지케이스로 21 어떠신가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

21로 변경했습니다!

.contentType(ContentType.JSON)
.body(request)
.when().log().all()
.header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken)
Copy link
Collaborator

Choose a reason for hiding this comment

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

token이 담긴 header는 given 으로 둬요 ^__^

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

모든 header 는 given 으로 이동하는 것으로 변경되었습니다 👍🏻

.contentType(ContentType.JSON)
.body(request)
.when().log().all()
.header(HttpHeaders.AUTHORIZATION, "Bearer " + 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.

ditto

중복된 닉네임 수정 시 예외 처리 여부, 상태 코드 반환 여부, 재발급 여부는 추후 논의가 필요합니다.

.contentType(ContentType.JSON)
.body(request)
.when().log().all()
.header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken)
Copy link
Collaborator

Choose a reason for hiding this comment

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

(▀̿Ĺ̯▀̿ ̿) ditto

Copy link
Collaborator

@somsom13 somsom13 left a comment

Choose a reason for hiding this comment

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

백엔드 4인이 모인 몹 코드리뷰 결과입니다.
고생하셨어요 베로 ✍(◔◡◔)

somsom13

This comment was marked as duplicate.

@Cyma-s Cyma-s requested a review from somsom13 October 3, 2023 05:38
Copy link
Collaborator

@somsom13 somsom13 left a comment

Choose a reason for hiding this comment

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

베로~~ 너무 고생 많으셨어요!!
닉네임 변경이라는게 생각보다 고민해야 할 요소들이 정말 많은 기능이었네요... 🥲 😓

하지만 역시 베로답게 야무지게 구현, 수정해주셨군요 👍

로직 상에 문제가 생기지 않을까~? 싶은 부분들이 조금 있어서 코멘트 남겼습니다! 베로의 의견도 알려주시고 같이 이야기 해보면 좋을 것 같아요. (੭ˊᵕˋ)੭*ଘ

@@ -42,7 +43,7 @@ public ResponseEntity<Void> deleteMember(
public ResponseEntity<ReissueAccessTokenResponse> updateNickname(
@PathVariable(name = "member_id") final Long memberId,
@Authenticated final MemberInfo memberInfo,
@RequestBody final NicknameUpdateRequest request,
@Valid @RequestBody final NicknameUpdateRequest request,
Copy link
Collaborator

Choose a reason for hiding this comment

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

저도 매의 눈을 뜨기 위해 노력할게요 🦅 👀

Comment on lines 28 to 31
tokenService.validateRefreshToken(refreshToken);
final String accessToken = tokenService.extractAccessToken(authorization);
final ReissueAccessTokenResponse response = tokenService.reissueAccessTokenByRefreshToken(refreshToken,
accessToken);
Copy link
Collaborator

@somsom13 somsom13 Oct 3, 2023

Choose a reason for hiding this comment

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

TokenServicevalidateRefreshToken, extractAccessToken 모두 토큰 재발급 용도로 이 controller 메서드 내에서만 사용되는 것 같아요.

reissue 메서드 내에서 validate, extract 를 함께 하도록 하지 않고 validate와 extract를 public으로 두고 controller 에서 따로 호출하도록 하신 이유가 있을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

지금은 사라졌지만 닉네임 변경 시 리프레시 토큰을 받도록 하면 다른 곳에서도 검증이 필요해서 public 메서드로 만들어두었습니다!
그런데 생각해보면 현재 리프레시 토큰은 path 가 /reissue 로 고정되어 있기 때문에 private 메서드로 둬도 될 것 같네요 좋은 리뷰 감사합니다 👍🏻

}

public String extractAccessToken(final String authorization) {
return authorization.substring(TOKEN_PREFIX.length());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Bearer {accessToken} 구조의 문자열에서 엑세스 토큰만 추출하는 로직인 것 같은데, 엑세스 토큰이 Bearer로 시작하지 않는 상황에 대해서는 따로 검증하지 않아도 괜찮을까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

그리고 만약 authorization 문자열이 Bearer 보다 짧은, 잘못된 값이 들어오면 substring 에서 IndexOutOfBoundException이 발생할 수도 있을 것 같아요!

Comment on lines 34 to 38
final Claims claims = tokenProvider.parseClaims(refreshToken);
final Long memberId = claims.get("memberId", Long.class);
final String nickname = claims.get("nickname", String.class);

inMemoryTokenPairRepository.validateTokenPair(refreshToken, accessToken);
Copy link
Collaborator

Choose a reason for hiding this comment

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

tokenProvider.parseClaims 보다 InMemoryTokenPairRepository.validateTokenPair 가 선행되어야 하지 않을까요?

현재 TokenScheduler에 의해 1초마다 스케줄러를 돌려서 만료된 refreshToken을 삭제하고, Repository에 없는 refreshToken으로 재발급 요청이 오면 RefreshTokenNotFoundException 을 발생시키는 것으로 알고있어요.

만약 refreshToken이 만료되었는데, 이를 캐치하지 못하고 먼저 tokenProvider.parseClaimsrefreshToken을 추출한다면 아래와 같은 문제가 생길 것 같아요.


parseClaims가 파라미터로 받은 token의 유효기간이 만료되었다면 TokenException.ExpiredTokenException 을 발생시키고 프론트엔드에게 EXPIRED_TOKEN(1001, "유효기간이 만료된 토큰입니다.") 에러 메세지가 반환됩니다.

그러면 프론트엔드에서는 accessToken이 만료되었으니, refreshToken을 가지고 reissue 요청을 보내야 하는 상황 으로 인식이 될 것 같아요. (저희가 만료된 토큰이 accessToken인지, refreshToken인지에 대해 식별을 안하고 있는 걸로 알고 있습니다)
그런데 만료된 것이 refreshToken이라면 어떻게 될까요? 이 상황에서도 동일하게 유효기간이 만료된 토큰 이라는 에러 메세지가 프론트엔드에게 전달될 것 같아요.
그리고 다음 단계인 InMemoryTokenPairRepository.validateTokenPair까지 흐름이 넘어가지 않고 예외가 발생한 상태로 끝나버리겠죠?

이렇게 되면 결과적으로 "accessToken이 만료되어서 재발급 받으려 하는데, refreshToken도 이미 만료가 된 상황"에 대한 처리가 제대로 이루어지지 않을 것 같다는 생각이 들어요.

베로는 어떻게 생각하시나요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

날카로운 지적입니다. validateTokenPair 가 먼저 실행되어야 한다는 것에 100% 동의합니다
추가로 parseClaim 을 할 때, 어떤 토큰이 만료되었는지 로깅하는 건 어떤가요?


private final AuthService authService;
private final TokenService tokenService;
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

public TokenPair updateNickname(final Long memberId, final MemberInfo memberInfo,
final NicknameUpdateRequest request) {
final Member member = getMemberIfValidRequest(memberId, memberInfo);
public boolean updateNickname(final Long memberId, final Long requestMemberId,
Copy link
Collaborator

Choose a reason for hiding this comment

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

💬 위의 deleteByIdmemberInfo 를 그대로 파라미터로 받아오고 있는데, 두 메서드가 파라미터를 받는 방식을 통일해보면 어떨까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

현재 memberInfo 의 id 만 쓰이고 있는 상태라 id 만 넘겨주는 걸로 통일했습니다 😄

public TokenPair updateNickname(final Long memberId, final MemberInfo memberInfo,
final NicknameUpdateRequest request) {
final Member member = getMemberIfValidRequest(memberId, memberInfo);
public boolean updateNickname(final Long memberId, final Long requestMemberId,
Copy link
Collaborator

Choose a reason for hiding this comment

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

MemberService에서 Token을 재발급 해주는 로직이 사라지니 확실히 더 책임 분리가 잘 된 느낌이 드네용 ㅎㅎ 👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

이제 사용되지 않는 Response네요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

매의 눈... 이거 왜 남아 있었을까요 🤦🏻‍♀️

Comment on lines +63 to +66
@Parameter(
name = "memberInfo",
hidden = true
),
Copy link
Collaborator

Choose a reason for hiding this comment

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

오홍 👍 👍

@SpringBootTest
class TokenServiceTest {


Copy link
Collaborator

Choose a reason for hiding this comment

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

개행이 한 줄 더 들어갔네용~

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

매의 눈 🦅👁️

Copy link
Collaborator

@seokhwan-an seokhwan-an left a comment

Choose a reason for hiding this comment

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

배로! 역시 코드를 잘짜시네요!
리뷰할 때마다 배워나가는 것 같아요! 👍👍
리뷰 남긴 것 확인 부탁드립니다!

Comment on lines 74 to 75
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 를 찾아오는 로직으로 변경했습니다! 로직이 정말 간결해졌네요 👍🏻👍🏻 👍🏻 👍🏻 👍🏻 👍🏻

@@ -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.

👍👍

Comment on lines 110 to 112
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.

좋은 지적입니다 👍🏻

Comment on lines 14 to 21
public class TokenService {

public static final String EMPTY_REFRESH_TOKEN = "none";
public static final String TOKEN_PREFIX = "Bearer ";

private final TokenProvider tokenProvider;
private final InMemoryTokenPairRepository inMemoryTokenPairRepository;

Copy link
Collaborator

Choose a reason for hiding this comment

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

현재 TokenService 내부에 TokenProviderInMemoryTokenPairRepository를 가지고 구성이 되어 있는데 MemberService와 AuthService는 TokenService를 의존하는 것이 아닌 TokenProviderInMemoryTokenPairRepository의존하고 있는데 이 부분이 전에 말했던 순환참조 때문이었나요? 그렇지 않으면 TokenService를 의존하는 것이 좋을 것 같은데 어떻게 생각하시나요?

@ParameterizedTest
void updateNickname_badRequest(final String invalidNickname) {
// given
final Member member = memberRepository.save(new Member("[email protected]", "hi"));
Copy link
Collaborator

Choose a reason for hiding this comment

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

현재 이 부분은 이용되지 않는 것 같습니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Member 변수를 의미하는 것입니다!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[ 🌙 BE ] 백엔드 크루들의 멋진 개발 이야기 하나둘셋 야! ✨ Feat 꼼꼼한 기능 구현 중요하죠
Projects
Status: In Code Review
Development

Successfully merging this pull request may close these issues.

[FEAT] 닉네임 변경 API 추가
3 participants