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.

Sign up for GitHub

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

Jump to bottom

[Refactor/#409] 멤버 가입시 이메일과 컬럼 수정/생성 트랜잭션 분리 #410

Merged
merged 5 commits into from
Oct 30, 2024
Merged

[Refactor/#409] 멤버 가입시 이메일과 컬럼 수정/생성 트랜잭션 분리 #410

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f52ab52
refactor: 컬럼 생성/수정 트랜잭션 분리
belljun3395 Sep 19, 2024
63b18fe
refactor: TxCase 분리 반영
belljun3395 Sep 19, 2024
a10bf28
fix: 변경 사항 반영
belljun3395 Sep 20, 2024
ae65381
Merge branch 'dev' into refactor/#409_belljun3395
belljun3395 Sep 29, 2024
8a893cf
Merge branch 'dev' into refactor/#409_belljun3395
belljun3395 Oct 18, 2024
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
74 changes: 17 additions & 57 deletions api/src/main/kotlin/com/few/api/domain/member/usecase/SaveMemberUseCase.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,86 +1,46 @@
package com.few.api.domain.member.usecase

import com.few.api.config.crypto.IdEncryption
import com.few.api.domain.member.usecase.dto.SaveMemberTxCaseIn
import com.few.api.domain.member.usecase.dto.SaveMemberUseCaseIn
import com.few.api.domain.member.usecase.dto.SaveMemberUseCaseOut
import com.few.api.exception.common.InsertException
import com.few.api.domain.member.usecase.transaction.SaveMemberTxCase
import com.few.api.repo.dao.member.MemberDao
import com.few.api.repo.dao.member.command.InsertMemberCommand
import com.few.api.repo.dao.member.command.UpdateDeletedMemberTypeCommand
import com.few.api.repo.dao.member.query.SelectMemberByEmailNotConsiderDeletedAtQuery
import com.few.data.common.code.MemberType
import com.few.email.service.member.SendAuthEmailService
import com.few.email.service.member.dto.Content
import com.few.email.service.member.dto.SendAuthEmailArgs
import org.springframework.stereotype.Component
import org.springframework.transaction.annotation.Transactional
import java.net.URL
import java.util.*

@Component
class SaveMemberUseCase(
private val memberDao: MemberDao,
private val sendAuthEmailService: SendAuthEmailService,
private val idEncryption: IdEncryption,
private val saveMemberTxCase: SaveMemberTxCase,
) {
companion object {
private const val AUTH_HEAD_COMMENT = "few 로그인 링크입니다."
private const val AUTH_SUB_COMMENT = "로그인하시려면 아래 버튼을 눌러주세요!"
private const val SIGNUP_HEAD_COMMENT = "few에 가입해주셔서 감사합니다."
private const val SIGNUP_SUB_COMMENT = "가입하신 이메일 주소를 확인해주세요."
}

@Transactional
fun execute(useCaseIn: SaveMemberUseCaseIn): SaveMemberUseCaseOut {
/** email을 통해 가입 이력이 있는지 확인 */
Comment on lines +22 to 26
Copy link
Member

Choose a reason for hiding this comment

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

코드 다시보니까 이거 SaveMemberUseCase에도 @transactional가 있어서 SaveMemberTxCase로 로직 빼낼 필요 없을거 같아요 종준님, 어차피 메일 보내는건 @async 아님?

val isSignUpBeforeMember = SelectMemberByEmailNotConsiderDeletedAtQuery(
email = useCaseIn.email
val (headComment, subComment, memberId) = memberDao.selectMemberByEmail(
SelectMemberByEmailNotConsiderDeletedAtQuery(
email = useCaseIn.email
)
).let {
memberDao.selectMemberByEmail(it)
}

var headComment = AUTH_HEAD_COMMENT
var subComment = AUTH_SUB_COMMENT
var email = ""

/** 가입 이력이 없다면 회원 가입 처리 */
val token = if (Objects.isNull(isSignUpBeforeMember)) {
headComment = SIGNUP_HEAD_COMMENT
subComment = SIGNUP_SUB_COMMENT
email = useCaseIn.email
memberDao.insertMember(
InsertMemberCommand(
email = useCaseIn.email,
memberType = MemberType.PREAUTH
/** 가입 이력 여부를 기준으로 가입 처리 */
saveMemberTxCase.execute(
SaveMemberTxCaseIn(
record = it,
email = useCaseIn.email
)
) ?: throw InsertException("member.insertfail.record")
} else {
/** 삭제한 회원이라면 회원 타입을 PREAUTH로 변경 */
if (isSignUpBeforeMember!!.isDeleted) {
val isUpdate = memberDao.updateMemberType(
UpdateDeletedMemberTypeCommand(
id = isSignUpBeforeMember.memberId,
memberType = MemberType.PREAUTH
)
)
if (isUpdate != 1L) {
throw InsertException("member.updatefail.record")
}

memberDao.selectMemberByEmail(
SelectMemberByEmailNotConsiderDeletedAtQuery(
email = useCaseIn.email
)
)?.memberId ?: throw InsertException("member.selectfail.record")
} else {
/** 이미 가입한 회원이라면 회원 ID를 반환 */
isSignUpBeforeMember.memberId
}
}.let {
/** 회원 ID를 암호화하여 토큰으로 사용 */
idEncryption.encrypt(it.toString())
)
}
Comment on lines -47 to 39
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
Member

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.

[이전]
트랜잭션 시작 -> 멤버 수정/생성 -> 이메일 전송 -> 트랜잭션 종료
[수정]
트랜잭션 시작 -> 멤버 수정/생성 -> 트랜잭션 종료 -> 이메일 전송

이렇게 수정했어요.!
JPA라면 쓰지기연 때문에 수정/생성이 최대한 늦게 한번에 일어나는데 jOOQ는 쓰기 지연이 없는 것으로 알고 있어서 바로 바로 쿼리가 나갈꺼에요.
그렇게 되면 커밋하기 전까지 수정/생성 할때 락을 잡고 있다 생각해서 분리 하는게 맞지 않을까? 생각했어요

Copy link
Member

Choose a reason for hiding this comment

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

디비에선 성공했는데 이메일 전송이 실패된 상황을 고려해보면, 일단 이메일 전송은 실패했으나 디비에는 멤버 정보가 남게됨.
그럼 우리는 멤버 테이블만 보고 이 멤버가 가입완료된 사람인지 아닌지 구분이 어려울 수도 있겠네요,
추가로 이 상황에서 다시 멤버가 가입하려고 할 땐 이미 테이블에 멤버 데이터 있으니 재가입으로 판단되나요?

Copy link
Member

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.

아니면 먼저 메일을 보내놓고 콜백왔을 때 디비에서 처리

요것은 현재 이메일에 들어가는 내용중에 멤버의 ID 값이 있어서 불가능할 것 같아요!

Copy link
Member

Choose a reason for hiding this comment

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

가입대기 멤버처럼 임시 상태값을 두자고 말씀드리려고 했는데,, 딱 맞췄네용ㅋㅋㅋ

Copy link
Member

Choose a reason for hiding this comment

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

그럼 가입대기 멤버에서 다음 상태로 변경되는 시점은 언젠가요?

Copy link
Member

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.

옙 그래서 토큰을 획득하는 과정에서 업데이트 합니다.!

https://github.com/YAPP-Github/24th-Web-Team-1-BE/blob/dev/api/src/main/kotlin/com/few/api/domain/member/usecase/TokenUseCase.kt


/** 회원 ID를 암호화하여 토큰으로 사용 */
val token = idEncryption.encrypt(memberId.toString())

runCatching {
sendAuthEmailService.send(
SendAuthEmailArgs(
Expand All @@ -90,7 +50,7 @@ class SaveMemberUseCase(
content = Content(
headComment = headComment,
subComment = subComment,
email = email,
email = useCaseIn.email,
confirmLink = URL("https://www.fewletter.com/auth/validation/complete?auth_token=$token")
)
)
Expand Down
8 changes: 8 additions & 0 deletions api/src/main/kotlin/com/few/api/domain/member/usecase/dto/SaveMemberTxCaseIn.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package com.few.api.domain.member.usecase.dto

import com.few.api.repo.dao.member.record.MemberIdAndIsDeletedRecord

data class SaveMemberTxCaseIn(
val record: MemberIdAndIsDeletedRecord?,
val email: String,
)
7 changes: 7 additions & 0 deletions api/src/main/kotlin/com/few/api/domain/member/usecase/dto/SaveMemberTxCaseOut.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.few.api.domain.member.usecase.dto

data class SaveMemberTxCaseOut(
val headComment: String,
val subComment: String,
val memberId: Long,
)
76 changes: 76 additions & 0 deletions api/src/main/kotlin/com/few/api/domain/member/usecase/transaction/SaveMemberTxCase.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package com.few.api.domain.member.usecase.transaction

import com.few.api.domain.member.usecase.dto.SaveMemberTxCaseIn
import com.few.api.domain.member.usecase.dto.SaveMemberTxCaseOut
import com.few.api.exception.common.InsertException
import com.few.api.repo.dao.member.MemberDao
import com.few.api.repo.dao.member.command.InsertMemberCommand
import com.few.api.repo.dao.member.command.UpdateDeletedMemberTypeCommand
import com.few.api.repo.dao.member.record.MemberIdAndIsDeletedRecord
import com.few.data.common.code.MemberType
import org.springframework.stereotype.Component
import org.springframework.transaction.annotation.Transactional
import java.util.*

@Component
class SaveMemberTxCase(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

우선 TxCase라고 네이밍하였는데 어떠신가요?

Copy link
Member

Choose a reason for hiding this comment

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

얘 자체는 그냥 멤버 저장하는 행위인건데 Tx라는 용어를 굳이 넣어야 하나 싶습니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

우리가 Service를 다른 도메인을 사용할 때 네이밍으로 쓰고 있어서 다른 적절한 네이밍이 떠오르지 않았어요.!
그래서 우선 트랜잭션을 분리한 것이니 UseCase와 유사한 느낌으로 TxCase로 해본거였슴다.!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이번 PR말고도 다른 구현을 하며 트랜잭션을 분리해야하는 경우가 필요할 수도 있을 것 같은데 어떤 네이밍이 좋을까요??

private val memberDao: MemberDao,
) {
companion object {
private const val AUTH_HEAD_COMMENT = "few 로그인 링크입니다."
private const val AUTH_SUB_COMMENT = "로그인하시려면 아래 버튼을 눌러주세요!"
private const val SIGNUP_HEAD_COMMENT = "few에 가입해주셔서 감사합니다."
private const val SIGNUP_SUB_COMMENT = "가입하신 이메일 주소를 확인해주세요."
}

@Transactional
fun execute(dto: SaveMemberTxCaseIn): SaveMemberTxCaseOut {
Comment on lines +25 to +27
Copy link
Member

Choose a reason for hiding this comment

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

여기처럼 SaveMemberTxCase에도 @transactional이 걸려있어서 분리하든 안하든 똑같겠네요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

우선 메일 전송 여부도 응답으로 내려주고 있어서 async로 하고 있지 않고

기존 UseCase의 트랜잭션은 재가 분리하지 않고 커밋을 했네요..! 🙇🏻

Copy link
Member

Choose a reason for hiding this comment

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

REQUIRES_NEW 트랜잭션 전파 레벨 함 보시면 될거 같습니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tx로 분리한다고 하면 UC에는 DB 관련된 행동이 없어서 트랜잭션을 빼도 되겠다고 생각했는데 REQUIRES_NEW가 더 좋을까요?
저도 댓글 남겨주신거 보고 생각해보니 더 좋을 것 같긴합니다.!!!

Copy link
Member

Choose a reason for hiding this comment

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

아 제가 잘못 말씀드렸네요 (REQUIRES_NEW 아님)
코드는 기존 상태로 되돌린 담에 어싱크로 이메일 전송해야하는게 맞지않나요?

Copy link
Member

Choose a reason for hiding this comment

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

정리하면 디비 업데이트 하는거랑 이메일 보내는거랑 별도로 가져가려는거니까, @async 써서 이메일 전송은 다른 쓰레드에서 해야하는게 맞을거 같은데,,?

만약 이메일 전송하는 코드에서 디비 업데이트가 있다면 @async 안쓰고 전파 레벨(REQUIRES_NEW) 만으로도 트랜잭션 분리가 될 거 같음

Copy link
Collaborator Author

@belljun3395 belljun3395 Sep 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

응답에 이메일 전송 성공 여부까지 내려주고 있어서 불가능함다.!

다른 이메일은 다 비동기 처리했는데 인증 관련된 부분이라
나는 분명 버튼 누루고 전송되었다고 하는데 왜 이메일이 없어???? 이런 경험보다
응답이 조금 느려도 이메일 전송 여부까지 내려줘서 이메일 전송이 안되었을 경우에는 재시도하게 하는게 좋지 않을까 했어요!

Copy link
Member

Choose a reason for hiding this comment

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

회의로 말씀해보죠

return dto.record?.let { signUpBeforeMember ->
signUpBeforeMember.takeIf { it.isDeleted }?.let {
ifDeletedMember(it.memberId)
} ?: ifMember(signUpBeforeMember)
} ?: ifNewMember(dto.email)
Comment on lines +28 to +32
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
Member

Choose a reason for hiding this comment

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

takeIf 처음 보네요 재밌는거 배워갑니다~

}

private fun ifDeletedMember(memberId: Long): SaveMemberTxCaseOut {
memberDao.updateMemberType(
UpdateDeletedMemberTypeCommand(
id = memberId,
memberType = MemberType.PREAUTH
)
).also {
if (it != 1L) {
throw InsertException("member.updatefail.record")
}
}

return SaveMemberTxCaseOut(
headComment = AUTH_HEAD_COMMENT,
subComment = AUTH_SUB_COMMENT,
memberId = memberId
)
}

private fun ifMember(signUpBeforeMember: MemberIdAndIsDeletedRecord): SaveMemberTxCaseOut {
return SaveMemberTxCaseOut(
headComment = AUTH_HEAD_COMMENT,
subComment = AUTH_SUB_COMMENT,
memberId = signUpBeforeMember.memberId
)
}

private fun ifNewMember(email: String): SaveMemberTxCaseOut {
return memberDao.insertMember(
InsertMemberCommand(
email = email,
memberType = MemberType.PREAUTH
)
)?.let {
SaveMemberTxCaseOut(
headComment = SIGNUP_HEAD_COMMENT,
subComment = SIGNUP_SUB_COMMENT,
memberId = it
)
} ?: throw InsertException("member.insertfail.record")
}
}
43 changes: 30 additions & 13 deletions api/src/test/kotlin/com/few/api/domain/member/usecase/SaveMemberUseCaseTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package com.few.api.domain.member.usecase

import com.few.api.config.crypto.IdEncryption
import com.few.api.domain.member.usecase.dto.SaveMemberTxCaseOut
import com.few.api.domain.member.usecase.dto.SaveMemberUseCaseIn
import com.few.api.domain.member.usecase.transaction.SaveMemberTxCase
import com.few.api.repo.dao.member.MemberDao
import com.few.api.repo.dao.member.command.UpdateDeletedMemberTypeCommand
import com.few.api.repo.dao.member.query.SelectMemberByEmailNotConsiderDeletedAtQuery
import com.few.api.repo.dao.member.record.MemberIdAndIsDeletedRecord
import com.few.email.service.member.SendAuthEmailService
Expand All @@ -19,13 +20,15 @@ class SaveMemberUseCaseTest : BehaviorSpec({
lateinit var memberDao: MemberDao
lateinit var sendAuthEmailService: SendAuthEmailService
lateinit var idEncryption: IdEncryption
lateinit var saveMemberTxCase: SaveMemberTxCase
lateinit var useCase: SaveMemberUseCase

beforeContainer {
memberDao = mockk<MemberDao>()
sendAuthEmailService = mockk<SendAuthEmailService>()
idEncryption = mockk<IdEncryption>()
useCase = SaveMemberUseCase(memberDao, sendAuthEmailService, idEncryption)
saveMemberTxCase = mockk<SaveMemberTxCase>()
useCase = SaveMemberUseCase(memberDao, sendAuthEmailService, idEncryption, saveMemberTxCase)
}

given("회원가입/로그인 요청이 온 상황에서") {
Expand All @@ -35,7 +38,11 @@ class SaveMemberUseCaseTest : BehaviorSpec({
`when`("요청의 이메일이 가입 이력이 없는 경우") {
every { memberDao.selectMemberByEmail(any(SelectMemberByEmailNotConsiderDeletedAtQuery::class)) } returns null

every { memberDao.insertMember(any()) } returns 1L
every { saveMemberTxCase.execute(any()) } returns SaveMemberTxCaseOut(
headComment = "headComment",
subComment = "subComment",
memberId = 1L
)

val token = "encryptedToken"
every { idEncryption.encrypt(any()) } returns token
Expand All @@ -47,8 +54,7 @@ class SaveMemberUseCaseTest : BehaviorSpec({
useCaseOut.isSendAuthEmail shouldBe true

verify(exactly = 1) { memberDao.selectMemberByEmail(any(SelectMemberByEmailNotConsiderDeletedAtQuery::class)) }
verify(exactly = 1) { memberDao.insertMember(any()) }
verify(exactly = 0) { memberDao.updateMemberType(any(UpdateDeletedMemberTypeCommand::class)) }
verify(exactly = 1) { saveMemberTxCase.execute(any()) }
verify(exactly = 1) { idEncryption.encrypt(any()) }
verify(exactly = 1) { sendAuthEmailService.send(any()) }
}
Expand All @@ -61,6 +67,12 @@ class SaveMemberUseCaseTest : BehaviorSpec({
isDeleted = false
)

every { saveMemberTxCase.execute(any()) } returns SaveMemberTxCaseOut(
headComment = "headComment",
subComment = "subComment",
memberId = 1L
)

val token = "encryptedToken"
every { idEncryption.encrypt(any()) } returns token

Expand All @@ -71,8 +83,7 @@ class SaveMemberUseCaseTest : BehaviorSpec({
useCaseOut.isSendAuthEmail shouldBe true

verify(exactly = 1) { memberDao.selectMemberByEmail(any(SelectMemberByEmailNotConsiderDeletedAtQuery::class)) }
verify(exactly = 0) { memberDao.insertMember(any()) }
verify(exactly = 0) { memberDao.updateMemberType(any(UpdateDeletedMemberTypeCommand::class)) }
verify(exactly = 1) { saveMemberTxCase.execute(any()) }
verify(exactly = 1) { idEncryption.encrypt(any()) }
verify(exactly = 1) { sendAuthEmailService.send(any()) }
}
Expand All @@ -85,7 +96,11 @@ class SaveMemberUseCaseTest : BehaviorSpec({
isDeleted = true
)

every { memberDao.updateMemberType(any(UpdateDeletedMemberTypeCommand::class)) } returns memberId
every { saveMemberTxCase.execute(any()) } returns SaveMemberTxCaseOut(
headComment = "headComment",
subComment = "subComment",
memberId = 1L
)

val token = "encryptedToken"
every { idEncryption.encrypt(any()) } returns token
Expand All @@ -96,9 +111,8 @@ class SaveMemberUseCaseTest : BehaviorSpec({
val useCaseOut = useCase.execute(useCaseIn)
useCaseOut.isSendAuthEmail shouldBe true

verify(exactly = 2) { memberDao.selectMemberByEmail(any(SelectMemberByEmailNotConsiderDeletedAtQuery::class)) }
verify(exactly = 0) { memberDao.insertMember(any()) }
verify(exactly = 1) { memberDao.updateMemberType(any(UpdateDeletedMemberTypeCommand::class)) }
verify(exactly = 1) { memberDao.selectMemberByEmail(any(SelectMemberByEmailNotConsiderDeletedAtQuery::class)) }
verify(exactly = 1) { saveMemberTxCase.execute(any()) }
verify(exactly = 1) { idEncryption.encrypt(any()) }
verify(exactly = 1) { sendAuthEmailService.send(any()) }
}
Expand All @@ -107,8 +121,11 @@ class SaveMemberUseCaseTest : BehaviorSpec({
`when`("인증 이메일 발송에 실패한 경우") {
every { memberDao.selectMemberByEmail(any(SelectMemberByEmailNotConsiderDeletedAtQuery::class)) } returns null

val memberId = 1L
every { memberDao.insertMember(any()) } returns memberId
every { saveMemberTxCase.execute(any()) } returns SaveMemberTxCaseOut(
headComment = "headComment",
subComment = "subComment",
memberId = 1L
)

val token = "encryptedToken"
every { idEncryption.encrypt(any()) } returns token
Expand Down
Loading