根據第一次 code review 修正

Game-as-a-Service · Jul 16, 2023 · 588ac54 · 588ac54
1 parent 3fde082
commit 588ac54
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 53 deletions.
diff --git a/application/src/main/kotlin/tw/waterballsa/gaas/application/usecases/GetRoomUsecase.kt b/application/src/main/kotlin/tw/waterballsa/gaas/application/usecases/GetRoomUsecase.kt
@@ -4,9 +4,8 @@ import tw.waterballsa.gaas.application.eventbus.EventBus
 import tw.waterballsa.gaas.application.repositories.RoomRepository
 import tw.waterballsa.gaas.domain.Room
 import tw.waterballsa.gaas.domain.Room.Player
-import tw.waterballsa.gaas.exceptions.ForbiddenException
-import tw.waterballsa.gaas.exceptions.ForbiddenException.Companion.Message
 import tw.waterballsa.gaas.exceptions.NotFoundException
+import tw.waterballsa.gaas.exceptions.PlatformException
 import javax.inject.Named
 
 @Named
@@ -18,17 +17,17 @@ class GetRoomUsecase(
         with(request) {
             val room = findRoomById(Room.Id(roomId))!!
             room.validatePlayerInRoom(Player.Id(userId))
-            room.also { presenter.present(it) }
+            presenter.present(room)
         }
     }
 
-    private fun findRoomById(roomId: Room.Id) =
+    private fun findRoomById(roomId: Room.Id): Room =
         roomRepository.findById(roomId)
             ?: throw NotFoundException.notFound(Room::class).id(roomId)
 
     private fun Room.validatePlayerInRoom(playerId: Player.Id) {
         if (!hasPlayer(playerId)) {
-            throw ForbiddenException.forbidden(Message.NOT_IN_ROOM).message()
+            throw PlatformException("Player(" + playerId!!.value + ") is not in the room(" + roomId!!.value + ").")
         }
     }
 

diff --git a/domain/src/main/kotlin/tw/waterballsa/gaas/exceptions/ForbiddenException.kt b/domain/src/main/kotlin/tw/waterballsa/gaas/exceptions/ForbiddenException.kt
diff --git a/spring/src/main/kotlin/tw/waterballsa/gaas/spring/aspects/PlatformExceptionHandler.kt b/spring/src/main/kotlin/tw/waterballsa/gaas/spring/aspects/PlatformExceptionHandler.kt
@@ -4,7 +4,6 @@ import org.springframework.http.HttpStatus.*
 import org.springframework.web.bind.annotation.ExceptionHandler
 import org.springframework.web.bind.annotation.ResponseStatus
 import org.springframework.web.bind.annotation.RestControllerAdvice
-import tw.waterballsa.gaas.exceptions.ForbiddenException
 import tw.waterballsa.gaas.exceptions.NotFoundException
 import tw.waterballsa.gaas.exceptions.PlatformException
 import tw.waterballsa.gaas.spring.controllers.viewmodel.PlatformViewModel
@@ -18,9 +17,4 @@ class PlatformExceptionHandler {
     @ResponseStatus(BAD_REQUEST)
     @ExceptionHandler(PlatformException::class)
     fun badRequest(exception: PlatformException): PlatformViewModel = PlatformViewModel(exception.message!!)
-
-    @ResponseStatus(FORBIDDEN)
-    @ExceptionHandler(ForbiddenException::class)
-    fun forbidden(exception: ForbiddenException): PlatformViewModel = PlatformViewModel(exception.message!!)
-
 }
diff --git a/spring/src/main/kotlin/tw/waterballsa/gaas/spring/controllers/presenter/GetRoomPresenter.kt b/spring/src/main/kotlin/tw/waterballsa/gaas/spring/controllers/presenter/GetRoomPresenter.kt
@@ -19,18 +19,18 @@ private fun Room.toViewModel(): GetRoomViewModel =
     GetRoomViewModel(
         id = roomId!!.value,
         name = name,
-        game = game.toGetRoomView(),
-        host = host.toGetRoomView(),
-        players = players.map { it.toGetRoomView() },
+        game = game.toViewModel(),
+        host = host.toViewModel(),
+        players = players.map { it.toViewModel() },
         maxPlayers = maxPlayers,
         minPlayers = minPlayers,
         currentPlayers = players.size,
         isLocked = isLocked,
         status = status.toString()
     )
 
-private fun GameRegistration.toGetRoomView(): GetRoomViewModel.Game =
+private fun GameRegistration.toViewModel(): GetRoomViewModel.Game =
     GetRoomViewModel.Game(id!!.value, displayName)
 
-private fun Room.Player.toGetRoomView(): GetRoomViewModel.Player =
+private fun Room.Player.toViewModel(): GetRoomViewModel.Player =
     GetRoomViewModel.Player(id.value, nickname, readiness)
diff --git a/spring/src/test/kotlin/tw/waterballsa/gaas/spring/it/controllers/RoomControllerTest.kt b/spring/src/test/kotlin/tw/waterballsa/gaas/spring/it/controllers/RoomControllerTest.kt
@@ -26,7 +26,6 @@ import tw.waterballsa.gaas.spring.models.TestGetRoomsRequest
 import tw.waterballsa.gaas.spring.models.TestJoinRoomRequest
 import java.util.UUID.*
 import kotlin.reflect.KClass
-import tw.waterballsa.gaas.exceptions.ForbiddenException.Companion.Message
 
 
 class RoomControllerTest @Autowired constructor(
@@ -277,7 +276,7 @@ class RoomControllerTest @Autowired constructor(
         val playerB = createUser("2", "[email protected]", "winner1122").toRoomPlayer()
         val playerC = createUser("3", "[email protected]", "winner0033").toRoomPlayer()
 
-        givenHostAndPlayersAreInTheRoom(host, playerB, playerC)
+        givenHostAndPlayersJoinedTheRoom(host, playerB, playerC)
             .whenUserLeaveTheRoom(userA)
             .thenPlayerShouldBeNotInRoomAndHostIsChanged(host)
     }
@@ -305,22 +304,22 @@ class RoomControllerTest @Autowired constructor(
     }
 
     @Test
-    fun giveHostAndPlayerBAreInRoomC_WhenHostGetRoomCInfo_ThenShouldReturnRoomCInfoSuccessfully() {
+    fun giveHostAndPlayerBJoinedRoomC_WhenHostGetRoomC_ThenShouldGetRoomCSuccessfully() {
         val userA = testUser
         val host = userA.toRoomPlayer()
         val playerB = createUser("2", "[email protected]", "winner1122").toRoomPlayer()
-        givenHostAndPlayersAreInTheRoom(host, playerB)
+        givenHostAndPlayersJoinedTheRoom(host, playerB)
             .whenUserGetTheRoom(userA)
-            .thenReturnRoomInfo()
+            .thenGetRoomSuccessfully()
     }
 
     @Test
-    fun giveUserANotInRoomB_WhenUserAGetRoomBInfo_ThenShouldNotReceiveRoomBInfo(){
+    fun giveUserANotJoinedRoomB_WhenUserAGetRoomB_ThenShouldFail() {
         val userA = testUser
         val host = createUser("2", "[email protected]", "winner1122").toRoomPlayer()
-        givenHostAndPlayersAreInTheRoom(host)
+        givenHostAndPlayersJoinedTheRoom(host)
             .whenUserGetTheRoom(userA)
-            .thenShouldNotReceiveRoomInfo(Message.NOT_IN_ROOM.toString())
+            .thenShouldFail("Player(" + userA.id!!.value + ") is not in the room(" + testRoom.roomId!!.value + ").")
     }
 
     private fun TestGetRoomsRequest.whenUserAVisitLobby(joinUser: User): ResultActions =
@@ -382,10 +381,10 @@ class RoomControllerTest @Autowired constructor(
                 .withJwt(leaveUser)
         )
 
-    private fun getRoom(user: Jwt): ResultActions =
+    private fun getRoom(user: User): ResultActions =
         mockMvc.perform(
             get("/rooms/${testRoom.roomId!!.value}")
-                .withJwt(user)
+                .withJwt(mockUserJwt(user))
         )
 
     private fun givenTheHostCreatePublicRoom(host: User): Room {
@@ -398,7 +397,7 @@ class RoomControllerTest @Autowired constructor(
         return testRoom
     }
 
-    private fun givenHostAndPlayersAreInTheRoom(host: Player, vararg players: Player): Room {
+    private fun givenHostAndPlayersJoinedTheRoom(host: Player, vararg players: Player): Room {
         val combinedPlayers = (listOf(host) + players).toMutableList()
         testRoom = createRoom(host, combinedPlayers)
         return testRoom
@@ -420,7 +419,7 @@ class RoomControllerTest @Autowired constructor(
         return leaveRoom(leaveUser)
     }
 
-    private fun Room.whenUserGetTheRoom(user: User) = getRoom(user.id!!.value.toJwt())
+    private fun Room.whenUserGetTheRoom(user: User) = getRoom(user)
 
     private fun ResultActions.thenCreateRoomSuccessfully(request: TestCreateRoomRequest) {
         request.let {
@@ -460,7 +459,7 @@ class RoomControllerTest @Autowired constructor(
         assertFalse(room.isHost(player.id))
     }
 
-    private fun ResultActions.thenReturnRoomInfo() {
+    private fun ResultActions.thenGetRoomSuccessfully() {
         val room = roomRepository.findById(testRoom.roomId!!)!!
         room.let {
             andExpect(status().isOk)
@@ -487,11 +486,6 @@ class RoomControllerTest @Autowired constructor(
         }
     }
 
-    private fun ResultActions.thenShouldNotReceiveRoomInfo(message: String): ResultActions {
-        return andExpect(status().isForbidden)
-            .andExpect(jsonPath("$.message").value(message))
-    }
-
     private fun createUser(id: String, email: String, nickname: String): User =
         userRepository.createUser(User(User.Id(id), email, nickname))
 
@@ -588,9 +582,6 @@ class RoomControllerTest @Autowired constructor(
     private fun User.toRoomPlayer(): Player =
         Player(Player.Id(id!!.value), nickname)
 
-    private fun Room.hasPlayer(playerId: Player.Id): Boolean =
-        players.any { it.id == playerId }
-
     private fun Room.isHost(playerId: Player.Id): Boolean =
         host.id == playerId
 }