Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -17,8 +17,8 @@ import org.springframework.web.bind.annotation.*
@RequestMapping("/api/v1")
class ChatController(
private val chatService: ChatService,
val authClient: AuthClient,
val serverClient: ServerClient,
private val authClient: AuthClient,
private val serverClient: ServerClient,
) {
@PostMapping("/chat")
fun createChat(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package kpring.chat.chatroom.api.v1
import kpring.chat.chatroom.service.ChatRoomService
import kpring.core.auth.client.AuthClient
import kpring.core.chat.chatroom.dto.request.CreateChatRoomRequest
import kpring.core.global.dto.response.ApiResponse
import org.springframework.http.ResponseEntity
import org.springframework.validation.annotation.Validated
import org.springframework.web.bind.annotation.*
Expand Down Expand Up @@ -34,4 +35,15 @@ class ChatRoomController(
val result = chatRoomService.exitChatRoom(chatRoomId, userId)
return ResponseEntity.ok().body(result)
}

@PatchMapping("/chatroom/{chatRoomId}/invite/{userId}")
fun inviteToChatRoomByUserId(
@PathVariable("userId") userId: String,
@PathVariable("chatRoomId") chatRoomId: String,
@RequestHeader("Authorization") token: String,
): ResponseEntity<*> {
val inviterId = authClient.getTokenInfo(token).data!!.userId
chatRoomService.inviteToChatRoomByUserIdWithLock(userId, inviterId, chatRoomId)
return ResponseEntity.ok().body(ApiResponse<Nothing>(status = 201))
}
}
7 changes: 7 additions & 0 deletions chat/src/main/kotlin/kpring/chat/chatroom/dto/Lock.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package kpring.chat.chatroom.dto

data class Lock(
val lockId: String,
val owner: String,
val acquired: Boolean,
)
8 changes: 6 additions & 2 deletions chat/src/main/kotlin/kpring/chat/chatroom/model/ChatRoom.kt
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,20 @@ class ChatRoom : BaseTime() {
@Id
var id: String? = null

var members: MutableList<String> = mutableListOf()
var members: MutableSet<String> = mutableSetOf()
Comment on lines 10 to +12
Copy link
Contributor

Choose a reason for hiding this comment

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

id와 member를 public이면서 var이면 위험해보여요


fun getUsers(): List<String> {
return members
return members.stream().toList()
}

fun addUsers(list: List<String>) {
members.addAll(list)
}

fun addUser(userId: String) {
members.add(userId)
}

fun removeUser(userId: String) {
members.remove(userId)
}
Expand Down
12 changes: 12 additions & 0 deletions chat/src/main/kotlin/kpring/chat/chatroom/model/DistributedLock.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package kpring.chat.chatroom.model

import org.springframework.data.annotation.Id
import org.springframework.data.mongodb.core.mapping.Document

@Document(collection = "distributedLocks")
data class DistributedLock(
@Id
val id: String,
val owner: String,
val expiresAt: Long,
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package kpring.chat.chatroom.repository

import kpring.chat.chatroom.model.DistributedLock
import org.springframework.data.mongodb.repository.MongoRepository
import org.springframework.stereotype.Repository

@Repository
interface DistributedLockRepository : MongoRepository<DistributedLock, String>
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ import kpring.chat.global.exception.ErrorCode
import kpring.chat.global.exception.GlobalException
import kpring.core.chat.chatroom.dto.request.CreateChatRoomRequest
import org.springframework.stereotype.Service
import org.springframework.transaction.annotation.Transactional

@Service
class ChatRoomService(
private val chatRoomRepository: ChatRoomRepository,
private val lockService: DistributedLockService,
) {
fun createChatRoom(
request: CreateChatRoomRequest,
Expand All @@ -30,6 +32,31 @@ class ChatRoomService(
chatRoomRepository.save(chatRoom)
}

@Transactional
fun inviteToChatRoomByUserIdWithLock(
userId: String,
inviterId: String,
chatRoomId: String,
): Boolean {
val lock = lockService.getLock(chatRoomId)

inviteToChatRoomByUserId(userId, inviterId, chatRoomId)

lockService.releaseLock(lock.lockId, lock.owner)
Comment on lines +41 to +45
Copy link
Contributor

Choose a reason for hiding this comment

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

inviteToChatRoomByUserId에서 예외가 발생하면 락을 회수하지 못하는 상황이 발생할 것 같아요.

return true
}

fun inviteToChatRoomByUserId(
userId: String,
inviterId: String,
chatRoomId: String,
) {
verifyChatRoomAccess(chatRoomId, inviterId)
val chatRoom: ChatRoom = getChatRoom(chatRoomId)
chatRoom.addUser(userId)
chatRoomRepository.save(chatRoom)
}

fun verifyChatRoomAccess(
chatRoomId: String,
userId: String,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package kpring.chat.chatroom.service

import kpring.chat.chatroom.dto.Lock
import kpring.chat.chatroom.model.DistributedLock
import kpring.chat.chatroom.repository.DistributedLockRepository
import kpring.chat.global.exception.ErrorCode
import kpring.chat.global.exception.GlobalException
import org.springframework.stereotype.Service
import java.util.*

@Service
class DistributedLockService(
private val lockRepository: DistributedLockRepository,
) {
fun acquireLock(
lockId: String,
owner: String,
expireInMillis: Long,
): Boolean {
val now = System.currentTimeMillis()
val expiresAt = now + expireInMillis

val optionalLock = lockRepository.findById(lockId)
return if (optionalLock.isPresent) {
val lock = optionalLock.get()
if (lock.expiresAt < now) {
lockRepository.save(DistributedLock(lockId, owner, expiresAt))
true
} else {
false
}
} else {
lockRepository.save(DistributedLock(lockId, owner, expiresAt))
true
}
}

fun releaseLock(
lockId: String,
owner: String,
) {
val optionalLock = lockRepository.findById(lockId)
if (optionalLock.isPresent && optionalLock.get().owner == owner) {
lockRepository.deleteById(lockId)
}
}

fun getLock(chatRoomId: String): Lock {
val lockId = "chatRoom:$chatRoomId:lock"
Copy link
Contributor

Choose a reason for hiding this comment

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

lock service인데 chatRoom의 도메인이 쓰였기 때문에 결합도가 높아지는 원인으로 보이네요.

val owner = UUID.randomUUID().toString()

val lockAcquired = acquireLock(lockId, owner, 10000) // 10초 동안 잠금 유지
Copy link
Contributor

Choose a reason for hiding this comment

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

트랜잭션의 평균 실행시간이 10초 보다는 짧을 것 같아서 1초로 시작해서 튜닝하는 것이 좋아보여요. 설정 값은 하드 코딩하기 보다는 설정을 따로 빼는 것이 좋아보여요.

if (!lockAcquired) {
throw GlobalException(ErrorCode.CONCURRENCY_CONFLICTION)
}
return Lock(lockId = lockId, owner = owner, acquired = lockAcquired)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,7 @@ enum class ErrorCode(val httpStatus: Int, val message: String) {

// 404
CHATROOM_NOT_FOUND(HttpStatus.NOT_FOUND.value(), "해당 id로 chatroom을 찾을 수 없습니다"),

// 500
CONCURRENCY_CONFLICTION(HttpStatus.CONFLICT.value(), "동시성 충돌이 발생했습니다."),
Copy link
Contributor

Choose a reason for hiding this comment

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

외부에 동시성 충돌을 알려주는 것은 구현을 알려주는 것이라서 지양하는 것이 좋다고 생각합니다.

}
72 changes: 71 additions & 1 deletion chat/src/test/kotlin/kpring/chat/chatroom/ChatRoomServiceTest.kt
Original file line number Diff line number Diff line change
@@ -1,22 +1,30 @@
package kpring.chat.chatroom

import io.kotest.assertions.throwables.shouldThrow
import io.kotest.core.spec.style.FunSpec
import io.kotest.matchers.collections.shouldContain
import io.kotest.matchers.collections.shouldNotContain
import io.kotest.matchers.shouldBe
import io.mockk.every
import io.mockk.mockk
import io.mockk.verify
import kpring.chat.chatroom.dto.Lock
import kpring.chat.chatroom.model.ChatRoom
import kpring.chat.chatroom.repository.ChatRoomRepository
import kpring.chat.chatroom.service.ChatRoomService
import kpring.chat.chatroom.service.DistributedLockService
import kpring.chat.global.ChatRoomTest
import kpring.chat.global.CommonTest
import kpring.chat.global.exception.ErrorCode
import kpring.chat.global.exception.GlobalException
import kpring.core.chat.chatroom.dto.request.CreateChatRoomRequest
import java.util.*

class ChatRoomServiceTest : FunSpec({

val chatRoomRepository = mockk<ChatRoomRepository>()
val chatRoomService = ChatRoomService(chatRoomRepository)
val lockService = mockk<DistributedLockService>()
val chatRoomService = ChatRoomService(chatRoomRepository, lockService)

test("createChatRoom 는 새 ChatRoom을 저장해야 한다") {
// Given
Expand Down Expand Up @@ -49,4 +57,66 @@ class ChatRoomServiceTest : FunSpec({
verify { chatRoomRepository.save(chatRoom) }
chatRoom.members.shouldNotContain(CommonTest.TEST_USER_ID)
}

test("inviteToChatRoomByUserIdWithLock 는 동시성 잠금을 획득한 후 사용자를 초대해야 한다") {
// Given
val chatRoom =
ChatRoom().apply {
id = ChatRoomTest.TEST_ROOM_ID
addUsers(ChatRoomTest.TEST_MEMBERS)
}
val lock = Lock("chatRoom:${chatRoom.id}:lock", UUID.randomUUID().toString(), true)

every { lockService.getLock(any()) } returns lock
every { lockService.releaseLock(any(), any()) } returns Unit
every { chatRoomRepository.findById(chatRoom.id!!) } returns Optional.of(chatRoom)
every { chatRoomRepository.save(any()) } returns chatRoom
every { chatRoomRepository.existsByIdAndMembersContaining(any(), CommonTest.TEST_USER_ID) } returns true

// When
chatRoomService.inviteToChatRoomByUserIdWithLock(CommonTest.TEST_USER_ID, CommonTest.TEST_USER_ID, chatRoom.id!!)

// Then
verify { lockService.getLock(any()) }
verify { chatRoomRepository.save(chatRoom) }
verify { lockService.releaseLock(any(), any()) }
chatRoom.members.shouldContain(CommonTest.TEST_USER_ID)
}

test("inviteToChatRoomByUserId 는 사용자를 초대해야 한다") {
// Given
val chatRoom =
ChatRoom().apply {
id = ChatRoomTest.TEST_ROOM_ID
addUsers(ChatRoomTest.TEST_MEMBERS)
}

every { chatRoomRepository.findById(chatRoom.id!!) } returns Optional.of(chatRoom)
every { chatRoomRepository.save(any()) } returns chatRoom
every { chatRoomRepository.existsByIdAndMembersContaining(any(), CommonTest.TEST_USER_ID) } returns true

// When
chatRoomService.inviteToChatRoomByUserId(CommonTest.TEST_USER_ID, CommonTest.TEST_USER_ID, chatRoom.id!!)

// Then
verify { chatRoomRepository.save(chatRoom) }
chatRoom.members.shouldContain(CommonTest.TEST_USER_ID)
}

test("inviteToChatRoomByUserIdWithLock 는 잠금 획득 실패 시 예외를 발생해야 한다") {
// Given
val chatRoomId = ChatRoomTest.TEST_ROOM_ID
val userId = CommonTest.TEST_USER_ID

every { lockService.getLock(any()) } throws GlobalException(ErrorCode.CONCURRENCY_CONFLICTION)

// When
val exception =
shouldThrow<GlobalException> {
chatRoomService.inviteToChatRoomByUserIdWithLock(userId, userId, chatRoomId)
}

// Then
exception.getErrorCode() shouldBe ErrorCode.CONCURRENCY_CONFLICTION
}
})
Loading