-
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 구현 #131
The head ref may contain hidden characters: "130-feature-\uD504\uB85C\uD544\uC218\uC815"
Changes from 4 commits
29206d9
f685b9f
44adc8b
92e555a
9cd87be
70c3577
5ded692
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 |
---|---|---|
|
@@ -2,6 +2,7 @@ package com.petqua.application.member | |
|
||
import com.petqua.application.member.dto.MemberAddProfileCommand | ||
import com.petqua.application.member.dto.MemberSignUpCommand | ||
import com.petqua.application.member.dto.UpdateProfileCommand | ||
import com.petqua.application.token.AuthTokenInfo | ||
import com.petqua.application.token.TokenService | ||
import com.petqua.common.domain.findActiveByIdOrThrow | ||
|
@@ -19,6 +20,8 @@ import com.petqua.domain.member.nickname.NicknameWordRepository | |
import com.petqua.domain.policy.bannedword.BannedWordRepository | ||
import com.petqua.domain.policy.bannedword.BannedWords | ||
import com.petqua.exception.member.MemberException | ||
import com.petqua.exception.member.MemberExceptionType | ||
import com.petqua.exception.member.MemberExceptionType.ALREADY_EXIST_NICKNAME | ||
import com.petqua.exception.member.MemberExceptionType.FAILED_NICKNAME_GENERATION | ||
import com.petqua.exception.member.MemberExceptionType.HAS_SIGNED_UP_MEMBER | ||
import com.petqua.exception.member.MemberExceptionType.NOT_FOUND_MEMBER | ||
|
@@ -88,4 +91,19 @@ class MemberService( | |
val bannedWords = BannedWords(bannedWordRepository.findAll()) | ||
bannedWords.validateContainingBannedWord(name) | ||
} | ||
|
||
fun updateProfile(command: UpdateProfileCommand) { | ||
validateNickname(command.nickname) | ||
val member = memberRepository.findActiveByIdOrThrow(command.memberId) { | ||
MemberException(NOT_FOUND_MEMBER) | ||
} | ||
member.updateNickname(Nickname.from(command.nickname)) | ||
} | ||
|
||
private fun validateNickname(nickname: String) { | ||
validateContainingBannedWord(nickname) | ||
throwExceptionWhen(memberRepository.existsMemberByNickname(Nickname.from(nickname))) { | ||
MemberException(ALREADY_EXIST_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. 따로 메서드를 빼면 가독성이 더 향상될 것 같아요! 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 객체를 검증때 만들지 말고, Command에서 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. 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. 닉네임 중복검사 좋네요! 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. 반영했습니다!! |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,3 +51,8 @@ data class PetFishAddCommand( | |
) | ||
} | ||
} | ||
|
||
data class UpdateProfileCommand( | ||
val memberId: Long, | ||
val nickname: String | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ package com.petqua.presentation.member.dto | |
import com.petqua.application.member.dto.MemberAddProfileCommand | ||
import com.petqua.application.member.dto.MemberSignUpCommand | ||
import com.petqua.application.member.dto.PetFishAddCommand | ||
import com.petqua.application.member.dto.UpdateProfileCommand | ||
import io.swagger.v3.oas.annotations.media.Schema | ||
import java.time.YearMonth | ||
|
||
|
@@ -89,3 +90,19 @@ data class PetFishAddRequest( | |
) | ||
val count: Int, | ||
) | ||
|
||
data class UpdateProfileRequest( | ||
@Schema( | ||
description = "닉네임", | ||
example = "펫쿠아" | ||
) | ||
val nickname: String, | ||
) { | ||
|
||
fun toCommand(memberId: Long): UpdateProfileCommand { | ||
return UpdateProfileCommand( | ||
memberId = memberId, | ||
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. 깃허브가 마지막 줄 추가하라고 하네요! |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ import com.ninjasquad.springmockk.SpykBean | |
import com.petqua.application.member.dto.MemberAddProfileCommand | ||
import com.petqua.application.member.dto.MemberSignUpCommand | ||
import com.petqua.application.member.dto.PetFishAddCommand | ||
import com.petqua.application.member.dto.UpdateProfileCommand | ||
import com.petqua.domain.auth.AuthCredentialsRepository | ||
import com.petqua.domain.fish.FishRepository | ||
import com.petqua.domain.member.FishTankRepository | ||
|
@@ -20,6 +21,7 @@ import com.petqua.domain.member.nickname.NicknameWordRepository | |
import com.petqua.domain.policy.bannedword.BannedWord | ||
import com.petqua.domain.policy.bannedword.BannedWordRepository | ||
import com.petqua.exception.member.MemberException | ||
import com.petqua.exception.member.MemberExceptionType.ALREADY_EXIST_NICKNAME | ||
import com.petqua.exception.member.MemberExceptionType.CONTAINING_BANNED_WORD_NAME | ||
import com.petqua.exception.member.MemberExceptionType.FAILED_NICKNAME_GENERATION | ||
import com.petqua.exception.member.MemberExceptionType.HAS_SIGNED_UP_MEMBER | ||
|
@@ -30,6 +32,7 @@ import com.petqua.exception.member.MemberExceptionType.INVALID_MEMBER_FISH_TANK_ | |
import com.petqua.exception.member.MemberExceptionType.INVALID_MEMBER_PET_FISH | ||
import com.petqua.exception.member.MemberExceptionType.INVALID_MEMBER_PET_FISH_COUNT | ||
import com.petqua.exception.member.MemberExceptionType.NOT_FOUND_MEMBER | ||
import com.petqua.test.DataCleaner | ||
import com.petqua.test.fixture.authCredentials | ||
import com.petqua.test.fixture.fish | ||
import com.petqua.test.fixture.member | ||
|
@@ -57,6 +60,7 @@ class MemberServiceTest( | |
private val fishRepository: FishRepository, | ||
private val petFishRepository: PetFishRepository, | ||
private val fishTankRepository: FishTankRepository, | ||
private val dataCleaner: DataCleaner, | ||
|
||
@SpykBean private val nicknameGenerator: NicknameGenerator, | ||
) : BehaviorSpec({ | ||
|
@@ -444,4 +448,69 @@ class MemberServiceTest( | |
} | ||
} | ||
} | ||
|
||
Given("프로필 수정을 할 때") { | ||
val member = memberRepository.save(member(nickname = "닉네임")) | ||
bannedWordRepository.save(BannedWord(word = "금지 단어")) | ||
|
||
When("닉네임을 입력하면") { | ||
Comment on lines
+452
to
+456
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. 테스트 좋아요 👍 |
||
val command = UpdateProfileCommand( | ||
memberId = member.id, | ||
nickname = "변경된 닉네임", | ||
) | ||
memberService.updateProfile(command) | ||
|
||
Then("닉네임을 수정한다") { | ||
val updatedMember = memberRepository.findById(member.id).get() | ||
|
||
assertSoftly { | ||
updatedMember.nickname.value shouldBe "변경된 닉네임" | ||
} | ||
} | ||
} | ||
|
||
When("닉네임에 부적절한 단어가 들어가면") { | ||
val command = UpdateProfileCommand( | ||
memberId = member.id, | ||
nickname = "금지 단어", | ||
) | ||
|
||
Then("예외를 던진다") { | ||
shouldThrow<MemberException> { | ||
memberService.updateProfile(command) | ||
}.exceptionType() shouldBe CONTAINING_BANNED_WORD_NAME | ||
} | ||
} | ||
|
||
When("이미 다른 회원이 사용중인 닉네임이라면") { | ||
memberRepository.save(member(nickname = "변경된 닉네임")) | ||
val command = UpdateProfileCommand( | ||
memberId = member.id, | ||
nickname = "변경된 닉네임", | ||
) | ||
|
||
Then("예외를 던진다") { | ||
shouldThrow<MemberException> { | ||
memberService.updateProfile(command) | ||
}.exceptionType() shouldBe ALREADY_EXIST_NICKNAME | ||
} | ||
} | ||
|
||
When("회원이 존재하지 않으면") { | ||
val command = UpdateProfileCommand( | ||
memberId = Long.MAX_VALUE, | ||
nickname = "변경된 닉네임", | ||
) | ||
|
||
Then("예외를 던진다") { | ||
shouldThrow<MemberException> { | ||
memberService.updateProfile(command) | ||
}.exceptionType() shouldBe NOT_FOUND_MEMBER | ||
} | ||
} | ||
} | ||
|
||
afterContainer { | ||
dataCleaner.clean() | ||
} | ||
}) |
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.
회원 존재 여부 검증은 토큰에서 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.
@Combi153 @TaeyeonRoyce
맞습니다! 그런데 코드를 구현할 때는 토큰에서 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.
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를 조회해와야하잖아요??...
그럼 두 분은 그냥
memberRepository.findById(command.memberId).get()
... 이렇게 가져오는 걸 원하시는 건가요?검증 코드... 그냥 둬도 좋을지도?
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.
아니면 body로 변경된 정보들은 변경된 상태로 넘어오고, 변경되지 않은 정보는 원본 상태로 함께 넘어오는 상태로 생각하고
memberRepository에서 바로 update쿼리 날리는 걸 말하셨던 건가요?.?
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.
아뇨아뇨 현재 방안으로 유지해도 좋을 것 같아요!
#131 (comment) 해당 코멘트에 대한 답변이었어요.
말씀대로 어차피 Member를 사용해야한다면 위 방식대로 써도 괜찮을 것 같습니다!