Add checks for race conditions

4 files changed, 44 insertions, 39 deletions

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))
     }