From a3aa0dee728a299fbb54a758135ac1c8dfe867b2 Mon Sep 17 00:00:00 2001 From: joffrey-bion Date: Sat, 12 Dec 2020 02:04:17 +0100 Subject: Add checks for race conditions --- .../server/controllers/LobbyController.kt | 64 ++++++++++++---------- .../org/luxons/sevenwonders/server/lobby/Player.kt | 2 + .../luxons/sevenwonders/server/lobby/LobbyTest.kt | 7 +-- .../server/repositories/LobbyRepositoryTest.kt | 10 ++-- 4 files changed, 44 insertions(+), 39 deletions(-) (limited to 'sw-server') diff --git a/sw-server/src/main/kotlin/org/luxons/sevenwonders/server/controllers/LobbyController.kt b/sw-server/src/main/kotlin/org/luxons/sevenwonders/server/controllers/LobbyController.kt index f73d5481..34dfe4e7 100644 --- a/sw-server/src/main/kotlin/org/luxons/sevenwonders/server/controllers/LobbyController.kt +++ b/sw-server/src/main/kotlin/org/luxons/sevenwonders/server/controllers/LobbyController.kt @@ -49,10 +49,12 @@ class LobbyController( logger.info("Player {} left game '{}'", player, lobby.name) template.convertAndSendToUser(player.username, "/queue/lobby/left", lobby.id) - if (lobby.getPlayers().isEmpty()) { - deleteLobby(lobby) - } else { - sendLobbyUpdateToPlayers(lobby) + synchronized(lobby) { + if (lobby.getPlayers().isEmpty()) { + deleteLobby(lobby) + } else { + sendLobbyUpdateToPlayers(lobby) + } } } @@ -66,12 +68,14 @@ class LobbyController( val player = principal.player val lobby = player.ownedLobby - lobby.getPlayers().forEach { - it.leave() - template.convertAndSendToUser(it.username, "/queue/lobby/left", lobby.id) + synchronized(lobby) { + lobby.getPlayers().forEach { + it.leave() + template.convertAndSendToUser(it.username, "/queue/lobby/left", lobby.id) + } + logger.info("Player {} disbanded game '{}'", player, lobby.name) + deleteLobby(lobby) } - logger.info("Player {} disbanded game '{}'", player, lobby.name) - deleteLobby(lobby) } private fun deleteLobby(lobby: Lobby) { @@ -89,10 +93,12 @@ class LobbyController( @MessageMapping("/lobby/reorderPlayers") fun reorderPlayers(@Validated action: ReorderPlayersAction, principal: Principal) { val lobby = principal.player.ownedLobby - lobby.reorderPlayers(action.orderedPlayers) - logger.info("Players in game '{}' reordered to {}", lobby.name, action.orderedPlayers) - sendLobbyUpdateToPlayers(lobby) + synchronized(lobby) { + lobby.reorderPlayers(action.orderedPlayers) + logger.info("Players in game '{}' reordered to {}", lobby.name, action.orderedPlayers) + sendLobbyUpdateToPlayers(lobby) + } } /** @@ -104,10 +110,12 @@ class LobbyController( @MessageMapping("/lobby/reassignWonders") fun reassignWonders(@Validated action: ReassignWondersAction, principal: Principal) { val lobby = principal.player.ownedLobby - lobby.reassignWonders(action.assignedWonders) - logger.info("Reassigned wonders in game '{}': {}", lobby.name, action.assignedWonders) - sendLobbyUpdateToPlayers(lobby) + synchronized(lobby) { + lobby.reassignWonders(action.assignedWonders) + logger.info("Reassigned wonders in game '{}': {}", lobby.name, action.assignedWonders) + sendLobbyUpdateToPlayers(lobby) + } } /** @@ -119,10 +127,20 @@ class LobbyController( @MessageMapping("/lobby/updateSettings") fun updateSettings(@Validated action: UpdateSettingsAction, principal: Principal) { val lobby = principal.player.ownedLobby - lobby.settings = action.settings - logger.info("Updated settings of game '{}'", lobby.name) - sendLobbyUpdateToPlayers(lobby) + synchronized(lobby) { + lobby.settings = action.settings + logger.info("Updated settings of game '{}'", lobby.name) + sendLobbyUpdateToPlayers(lobby) + } + } + + internal fun sendLobbyUpdateToPlayers(lobby: Lobby) { + val lobbyDto = lobby.toDTO() + lobby.getPlayers().forEach { + template.convertAndSendToUser(it.username, "/queue/lobby/updated", lobbyDto) + } + template.convertAndSend("/topic/games", GameListEvent.CreateOrUpdate(lobbyDto).wrap()) } @MessageMapping("/lobby/addBot") @@ -132,17 +150,7 @@ class LobbyController( GlobalScope.launch { bot.play("ws://localhost:$serverPort", lobby.id) } - logger.info("Added bot {} to game '{}'", action.botDisplayName, lobby.name) - sendLobbyUpdateToPlayers(lobby) - } - - internal fun sendLobbyUpdateToPlayers(lobby: Lobby) { - val lobbyDto = lobby.toDTO() - lobby.getPlayers().forEach { - template.convertAndSendToUser(it.username, "/queue/lobby/updated", lobbyDto) - } - template.convertAndSend("/topic/games", GameListEvent.CreateOrUpdate(lobbyDto).wrap()) } /** diff --git a/sw-server/src/main/kotlin/org/luxons/sevenwonders/server/lobby/Player.kt b/sw-server/src/main/kotlin/org/luxons/sevenwonders/server/lobby/Player.kt index 94ae9742..1b63a155 100644 --- a/sw-server/src/main/kotlin/org/luxons/sevenwonders/server/lobby/Player.kt +++ b/sw-server/src/main/kotlin/org/luxons/sevenwonders/server/lobby/Player.kt @@ -36,10 +36,12 @@ class Player( get() = _game ?: throw PlayerNotInGameException(username) fun join(lobby: Lobby) { + require(_lobby == null) { "The player $displayName ($username) is already in a lobby" } _lobby = lobby } fun join(game: Game, index: Int) { + require(_game == null) { "The player $displayName ($username) is already in a game" } _game = game this.index = index } diff --git a/sw-server/src/test/kotlin/org/luxons/sevenwonders/server/lobby/LobbyTest.kt b/sw-server/src/test/kotlin/org/luxons/sevenwonders/server/lobby/LobbyTest.kt index 4125810f..7e27cf7f 100644 --- a/sw-server/src/test/kotlin/org/luxons/sevenwonders/server/lobby/LobbyTest.kt +++ b/sw-server/src/test/kotlin/org/luxons/sevenwonders/server/lobby/LobbyTest.kt @@ -34,24 +34,21 @@ class LobbyTest { @Before fun setUp() { gameOwner = Player("gameowner", "Game owner") - lobby = Lobby(0, "Test Game", gameOwner, gameDefinition) + lobby = Lobby(42, "Test Game", gameOwner, gameDefinition) } @Test fun testId() { - val lobby = Lobby(5, "Test Game", gameOwner, gameDefinition) - assertEquals(5, lobby.id) + assertEquals(42, lobby.id) } @Test fun testName() { - val lobby = Lobby(5, "Test Game", gameOwner, gameDefinition) assertEquals("Test Game", lobby.name) } @Test fun testOwner() { - val lobby = Lobby(5, "Test Game", gameOwner, gameDefinition) assertSame(gameOwner, lobby.getPlayers()[0]) assertSame(lobby, gameOwner.lobby) } diff --git a/sw-server/src/test/kotlin/org/luxons/sevenwonders/server/repositories/LobbyRepositoryTest.kt b/sw-server/src/test/kotlin/org/luxons/sevenwonders/server/repositories/LobbyRepositoryTest.kt index c59dc49f..8bef7efc 100644 --- a/sw-server/src/test/kotlin/org/luxons/sevenwonders/server/repositories/LobbyRepositoryTest.kt +++ b/sw-server/src/test/kotlin/org/luxons/sevenwonders/server/repositories/LobbyRepositoryTest.kt @@ -25,9 +25,8 @@ class LobbyRepositoryTest { @Test fun list_returnsAllLobbies() { - val owner = Player("owner", "The Owner") - val lobby1 = repository.create("Test Name 1", owner) - val lobby2 = repository.create("Test Name 2", owner) + val lobby1 = repository.create("Test Name 1", Player("owner1", "The Owner")) + val lobby2 = repository.create("Test Name 2", Player("owner2", "The Owner")) assertTrue(repository.list().contains(lobby1)) assertTrue(repository.list().contains(lobby2)) } @@ -48,9 +47,8 @@ class LobbyRepositoryTest { @Test fun find_returnsTheSameObject() { - val owner = Player("owner", "The Owner") - val lobby1 = repository.create("Test Name 1", owner) - val lobby2 = repository.create("Test Name 2", owner) + val lobby1 = repository.create("Test Name 1", Player("owner1", "The Owner")) + val lobby2 = repository.create("Test Name 2", Player("owner2", "The Owner")) assertSame(lobby1, repository.find(lobby1.id)) assertSame(lobby2, repository.find(lobby2.id)) } -- cgit