Conversation
| val member = memberRepository.findActiveByIdOrThrow(command.memberId) { | ||
| MemberException(NOT_FOUND_MEMBER) | ||
| } |
There was a problem hiding this comment.
회원 존재 여부 검증은 토큰에서 memberId를 추출할 때 이미 진행하고 있어요. 굳이 또 검증할 필요는 없다고 생각하는데 어떠신가요?
There was a problem hiding this comment.
@Combi153 @TaeyeonRoyce
맞습니다! 그런데 코드를 구현할 때는 토큰에서 member검증하는 코드는 이 서비스 로직의 트랜잭션에 포함되지 않는 코드이기 때문에 확실히 검증하려면 트랜잭션내에서 한번 더 확인해도 좋겠다고 생각했었어요! 혹시 잘못 생각하고 있는 부분이 있다면 알려주세요!😀
음 그런데 확실히 그 잠깐 사이에 멤버가 삭제되어버리는 일도 없을 것 같고... 실제로 중간에서 멤버가 삭제된다고 해도 문제가 생길 것 같지도 않고... 너무 과한 검증같긴하네여 역시 뺄까요?!
There was a problem hiding this comment.
불필요할 것 같아요.
말씀하신대로 해당 상황도 발생할 경우가 매우 적고, 트랜잭션에 포함되어 있다 하더라도 동시에 다른 요청에서 삭제하는 경우 보장이되나요?
저는 빼는게 좋을 것 같습니다!
There was a problem hiding this comment.
동시에 다른 요청에서 삭제하는 경우 보장이되나요?
아 그렇네요 ?! 빼고 오겠습니다
There was a problem hiding this comment.
빼려고 갔다왔는데... 어차피 해당 코드에서 member를 조회해와야하잖아요??...
그럼 두 분은 그냥 memberRepository.findById(command.memberId).get()... 이렇게 가져오는 걸 원하시는 건가요?
검증 코드... 그냥 둬도 좋을지도?
There was a problem hiding this comment.
아니면 body로 변경된 정보들은 변경된 상태로 넘어오고, 변경되지 않은 정보는 원본 상태로 함께 넘어오는 상태로 생각하고
memberRepository에서 바로 update쿼리 날리는 걸 말하셨던 건가요?.?
There was a problem hiding this comment.
아뇨아뇨 현재 방안으로 유지해도 좋을 것 같아요!
#131 (comment) 해당 코멘트에 대한 답변이었어요.
말씀대로 어차피 Member를 사용해야한다면 위 방식대로 써도 괜찮을 것 같습니다!
| throwExceptionWhen(memberRepository.existsMemberByNickname(Nickname.from(nickname))) { | ||
| MemberException(ALREADY_EXIST_NICKNAME) | ||
| } |
There was a problem hiding this comment.
따로 메서드를 빼면 가독성이 더 향상될 것 같아요!
There was a problem hiding this comment.
Nickname 객체를 검증때 만들지 말고, Command에서 Nickname으로 가지고 있으면 어떨까요?
Fast-fail 관점에서도 유리하고, application 영역에서도 도메인 객체로 사용할 수 있어 좋을 것 같아요
There was a problem hiding this comment.
| nickname = nickname, | ||
| ) | ||
| } | ||
| } |
| Given("프로필 수정을 할 때") { | ||
| val member = memberRepository.save(member(nickname = "닉네임")) | ||
| bannedWordRepository.save(BannedWord(word = "금지 단어")) | ||
|
|
||
| When("닉네임을 입력하면") { |
| throwExceptionWhen(memberRepository.existsMemberByNickname(Nickname.from(nickname))) { | ||
| MemberException(ALREADY_EXIST_NICKNAME) | ||
| } |
There was a problem hiding this comment.
닉네임 중복검사 좋네요!
근데, 본인이 가지고 있는 닉네임에 대한 중복검사는 이루어지지 않아야 할 것 같습니다!
| throwExceptionWhen(memberRepository.existsMemberByNickname(Nickname.from(nickname))) { | ||
| MemberException(ALREADY_EXIST_NICKNAME) | ||
| } |
There was a problem hiding this comment.
Nickname 객체를 검증때 만들지 말고, Command에서 Nickname으로 가지고 있으면 어떨까요?
Fast-fail 관점에서도 유리하고, application 영역에서도 도메인 객체로 사용할 수 있어 좋을 것 같아요
Test Results 64 files ±0 64 suites ±0 25s ⏱️ -1s Results for commit 5ded692. ± Comparison against base commit 35f447a. This pull request removes 1 and adds 2 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
📌 관련 이슈
📁 작업 설명
프로필 수정 API 구현.
하지만 프로필 수정란에 닉네임밖에 없어서 닉네임 수정이라봐도 무방한