From 236ea7b8072df68d50fc0e1760d56672b851d8b6 Mon Sep 17 00:00:00 2001 From: jbion Date: Fri, 30 Dec 2016 02:29:17 +0100 Subject: Refactor LobbyController to make the code simpler This change gets rid of session access and makes use of a player repository. This allows for encapsulated and testable Player management. --- .../sevenwonders/actions/ChooseNameAction.java | 19 +++ .../sevenwonders/actions/CreateGameAction.java | 19 +++ .../sevenwonders/actions/JoinGameAction.java | 17 +++ .../actions/JoinOrCreateGameAction.java | 31 ----- .../sevenwonders/actions/ReorderPlayersAction.java | 18 +++ .../sevenwonders/actions/UpdateSettingsAction.java | 19 +++ .../sevenwonders/controllers/LobbyController.java | 128 ++++++++++++++------- .../java/org/luxons/sevenwonders/game/Lobby.java | 31 +++-- .../java/org/luxons/sevenwonders/game/Player.java | 44 ++++++- .../org/luxons/sevenwonders/game/Settings.java | 4 + .../sevenwonders/repositories/LobbyRepository.java | 8 -- .../repositories/PlayerRepository.java | 61 ++++++++++ .../sevenwonders/session/SessionAttributes.java | 8 -- 13 files changed, 307 insertions(+), 100 deletions(-) create mode 100644 src/main/java/org/luxons/sevenwonders/actions/ChooseNameAction.java create mode 100644 src/main/java/org/luxons/sevenwonders/actions/CreateGameAction.java create mode 100644 src/main/java/org/luxons/sevenwonders/actions/JoinGameAction.java delete mode 100644 src/main/java/org/luxons/sevenwonders/actions/JoinOrCreateGameAction.java create mode 100644 src/main/java/org/luxons/sevenwonders/actions/ReorderPlayersAction.java create mode 100644 src/main/java/org/luxons/sevenwonders/actions/UpdateSettingsAction.java create mode 100644 src/main/java/org/luxons/sevenwonders/repositories/PlayerRepository.java delete mode 100644 src/main/java/org/luxons/sevenwonders/session/SessionAttributes.java (limited to 'src/main/java/org') diff --git a/src/main/java/org/luxons/sevenwonders/actions/ChooseNameAction.java b/src/main/java/org/luxons/sevenwonders/actions/ChooseNameAction.java new file mode 100644 index 00000000..42a26f37 --- /dev/null +++ b/src/main/java/org/luxons/sevenwonders/actions/ChooseNameAction.java @@ -0,0 +1,19 @@ +package org.luxons.sevenwonders.actions; + +import javax.validation.constraints.NotNull; +import javax.validation.constraints.Size; + +public class ChooseNameAction { + + @NotNull + @Size(min=2, max=20) + private String playerName; + + public String getPlayerName() { + return playerName; + } + + public void setPlayerName(String playerName) { + this.playerName = playerName; + } +} diff --git a/src/main/java/org/luxons/sevenwonders/actions/CreateGameAction.java b/src/main/java/org/luxons/sevenwonders/actions/CreateGameAction.java new file mode 100644 index 00000000..ce1783c0 --- /dev/null +++ b/src/main/java/org/luxons/sevenwonders/actions/CreateGameAction.java @@ -0,0 +1,19 @@ +package org.luxons.sevenwonders.actions; + +import javax.validation.constraints.NotNull; +import javax.validation.constraints.Size; + +public class CreateGameAction { + + @NotNull + @Size(min=2, max=30) + private String gameName; + + public String getGameName() { + return gameName; + } + + public void setGameName(String gameName) { + this.gameName = gameName; + } +} diff --git a/src/main/java/org/luxons/sevenwonders/actions/JoinGameAction.java b/src/main/java/org/luxons/sevenwonders/actions/JoinGameAction.java new file mode 100644 index 00000000..82bff168 --- /dev/null +++ b/src/main/java/org/luxons/sevenwonders/actions/JoinGameAction.java @@ -0,0 +1,17 @@ +package org.luxons.sevenwonders.actions; + +import javax.validation.constraints.NotNull; + +public class JoinGameAction { + + @NotNull + private Long gameId; + + public Long getGameId() { + return gameId; + } + + public void setGameId(Long gameId) { + this.gameId = gameId; + } +} diff --git a/src/main/java/org/luxons/sevenwonders/actions/JoinOrCreateGameAction.java b/src/main/java/org/luxons/sevenwonders/actions/JoinOrCreateGameAction.java deleted file mode 100644 index cf5596da..00000000 --- a/src/main/java/org/luxons/sevenwonders/actions/JoinOrCreateGameAction.java +++ /dev/null @@ -1,31 +0,0 @@ -package org.luxons.sevenwonders.actions; - -import javax.validation.constraints.NotNull; -import javax.validation.constraints.Size; - -public class JoinOrCreateGameAction { - - @NotNull - @Size(min=2, max=30) - private String gameName; - - @NotNull - @Size(min=2, max=20) - private String playerName; - - public String getGameName() { - return gameName; - } - - public void setGameName(String gameName) { - this.gameName = gameName; - } - - public String getPlayerName() { - return playerName; - } - - public void setPlayerName(String playerName) { - this.playerName = playerName; - } -} diff --git a/src/main/java/org/luxons/sevenwonders/actions/ReorderPlayersAction.java b/src/main/java/org/luxons/sevenwonders/actions/ReorderPlayersAction.java new file mode 100644 index 00000000..803a71d8 --- /dev/null +++ b/src/main/java/org/luxons/sevenwonders/actions/ReorderPlayersAction.java @@ -0,0 +1,18 @@ +package org.luxons.sevenwonders.actions; + +import java.util.List; +import javax.validation.constraints.NotNull; + +public class ReorderPlayersAction { + + @NotNull + private List orderedPlayers; + + public List getOrderedPlayers() { + return orderedPlayers; + } + + public void setOrderedPlayers(List orderedPlayers) { + this.orderedPlayers = orderedPlayers; + } +} diff --git a/src/main/java/org/luxons/sevenwonders/actions/UpdateSettingsAction.java b/src/main/java/org/luxons/sevenwonders/actions/UpdateSettingsAction.java new file mode 100644 index 00000000..deb5e530 --- /dev/null +++ b/src/main/java/org/luxons/sevenwonders/actions/UpdateSettingsAction.java @@ -0,0 +1,19 @@ +package org.luxons.sevenwonders.actions; + +import javax.validation.constraints.NotNull; + +import org.luxons.sevenwonders.game.Settings; + +public class UpdateSettingsAction { + + @NotNull + private Settings settings; + + public Settings getSettings() { + return settings; + } + + public void setSettings(Settings settings) { + this.settings = settings; + } +} diff --git a/src/main/java/org/luxons/sevenwonders/controllers/LobbyController.java b/src/main/java/org/luxons/sevenwonders/controllers/LobbyController.java index 285fd778..cfc540bb 100644 --- a/src/main/java/org/luxons/sevenwonders/controllers/LobbyController.java +++ b/src/main/java/org/luxons/sevenwonders/controllers/LobbyController.java @@ -4,21 +4,25 @@ import java.security.Principal; import java.util.Collection; import java.util.Collections; -import org.luxons.sevenwonders.actions.JoinOrCreateGameAction; +import org.luxons.sevenwonders.actions.ChooseNameAction; +import org.luxons.sevenwonders.actions.CreateGameAction; +import org.luxons.sevenwonders.actions.JoinGameAction; +import org.luxons.sevenwonders.actions.ReorderPlayersAction; import org.luxons.sevenwonders.actions.StartGameAction; +import org.luxons.sevenwonders.actions.UpdateSettingsAction; import org.luxons.sevenwonders.errors.ApiMisuseException; import org.luxons.sevenwonders.game.Game; import org.luxons.sevenwonders.game.Lobby; import org.luxons.sevenwonders.game.Player; import org.luxons.sevenwonders.repositories.GameRepository; import org.luxons.sevenwonders.repositories.LobbyRepository; -import org.luxons.sevenwonders.session.SessionAttributes; +import org.luxons.sevenwonders.repositories.PlayerRepository; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.messaging.handler.annotation.MessageMapping; import org.springframework.messaging.handler.annotation.SendTo; -import org.springframework.messaging.simp.SimpMessageHeaderAccessor; +import org.springframework.messaging.simp.SimpMessagingTemplate; import org.springframework.messaging.simp.annotation.SendToUser; import org.springframework.messaging.simp.annotation.SubscribeMapping; import org.springframework.stereotype.Controller; @@ -33,80 +37,126 @@ public class LobbyController { private final GameRepository gameRepository; + private final PlayerRepository playerRepository; + + private final SimpMessagingTemplate template; + @Autowired - public LobbyController(LobbyRepository lobbyRepository, GameRepository gameRepository) { + public LobbyController(LobbyRepository lobbyRepository, GameRepository gameRepository, + PlayerRepository playerRepository, SimpMessagingTemplate template) { this.lobbyRepository = lobbyRepository; this.gameRepository = gameRepository; + this.playerRepository = playerRepository; + this.template = template; + } + + @MessageMapping("/chooseName") + public void chooseName(@Validated ChooseNameAction action, Principal principal) { + String userName = principal.getName(); + Player player = playerRepository.updateOrCreatePlayer(userName, action.getPlayerName()); + + logger.info("Player '{}' chose the name '{}'", userName, player.getDisplayName()); } @SubscribeMapping("/games") // prefix /topic not shown - public Collection listGames() { - logger.info("Subscribed to /games"); + public Collection listGames(Principal principal) { + logger.info("Player '{}' subscribed to /topic/games", principal.getName()); return lobbyRepository.list(); } - @MessageMapping("/lobby/create-game") + @MessageMapping("/lobby/create") @SendTo("/topic/games") - public Collection createGame(SimpMessageHeaderAccessor headerAccessor, - @Validated JoinOrCreateGameAction action, Principal principal) { - checkThatUserIsNotInAGame(headerAccessor, "cannot create another game"); + public Collection createGame(@Validated CreateGameAction action, Principal principal) { + checkThatUserIsNotInAGame(principal, "cannot create another game"); - Player gameOwner = new Player(action.getPlayerName(), principal.getName()); + Player gameOwner = playerRepository.find(principal.getName()); Lobby lobby = lobbyRepository.create(action.getGameName(), gameOwner); - headerAccessor.getSessionAttributes().put(SessionAttributes.ATTR_LOBBY, lobby); + gameOwner.setLobby(lobby); - logger.info("Game '{}' (id={}) created by {} ({})", lobby.getName(), lobby.getId(), gameOwner.getDisplayName(), + logger.info("Game '{}' ({}) created by {} ({})", lobby.getName(), lobby.getId(), gameOwner.getDisplayName(), gameOwner.getUserName()); return Collections.singletonList(lobby); } - @MessageMapping("/lobby/join-game") - @SendToUser("/queue/join-game") - public Collection joinGame(SimpMessageHeaderAccessor headerAccessor, - @Validated JoinOrCreateGameAction action, Principal principal) { - checkThatUserIsNotInAGame(headerAccessor, "cannot join another game"); + @MessageMapping("/lobby/join") + @SendToUser("/queue/lobby") + public Collection joinGame(@Validated JoinGameAction action, Principal principal) { + checkThatUserIsNotInAGame(principal, "cannot join another game"); - Lobby lobby = lobbyRepository.find(action.getGameName()); - Player newPlayer = new Player(action.getPlayerName(), principal.getName()); + Lobby lobby = lobbyRepository.find(action.getGameId()); + Player newPlayer = playerRepository.find(principal.getName()); lobby.addPlayer(newPlayer); - headerAccessor.getSessionAttributes().put(SessionAttributes.ATTR_LOBBY, lobby); + newPlayer.setLobby(lobby); - logger.info("Player {} joined game {}", action.getPlayerName(), action.getGameName()); + logger.info("Player '{}' ({}) joined game {}", newPlayer.getDisplayName(), newPlayer.getUserName(), + lobby.getName()); return Collections.singletonList(lobby); } - private void checkThatUserIsNotInAGame(SimpMessageHeaderAccessor headerAccessor, - String impossibleActionDescription) { - Lobby lobby = (Lobby)headerAccessor.getSessionAttributes().get(SessionAttributes.ATTR_LOBBY); + private void checkThatUserIsNotInAGame(Principal principal, String impossibleActionDescription) { + Lobby lobby = playerRepository.find(principal.getName()).getLobby(); if (lobby != null) { throw new UserAlreadyInGameException(lobby.getName(), impossibleActionDescription); } } - @MessageMapping("/lobby/start-game") - public void startGame(SimpMessageHeaderAccessor headerAccessor, @Validated StartGameAction action, - Principal principal) { - Lobby lobby = getOwnedLobby(headerAccessor, principal); - Game game = lobby.startGame(action.getSettings()); + @MessageMapping("/lobby/reorderPlayers") + public void reorderPlayers(@Validated ReorderPlayersAction action, Principal principal) { + Lobby lobby = getLobby(principal); + lobby.reorderPlayers(action.getOrderedPlayers()); + + logger.info("Players in game {} reordered to {}", lobby.getName(), action.getOrderedPlayers()); + sendLobbyUpdateToPlayers(lobby); + } + + @MessageMapping("/lobby/updateSettings") + public void updateSettings(@Validated UpdateSettingsAction action, Principal principal) { + Lobby lobby = getLobby(principal); + lobby.setSettings(action.getSettings()); + + logger.info("Updated settings of game {}", lobby.getName()); + sendLobbyUpdateToPlayers(lobby); + } + + private void sendLobbyUpdateToPlayers(Lobby lobby) { + template.convertAndSend("/topic/lobby/" + lobby.getId() + "/updated", lobby); + } + + @MessageMapping("/lobby/start") + public void startGame(@Validated StartGameAction action, Principal principal) { + Lobby lobby = getOwnedLobby(principal); + Game game = lobby.startGame(); gameRepository.add(game); logger.info("Game {} successfully started", game.getId()); + template.convertAndSend("/topic/lobby/" + lobby.getId() + "/started", (Object)null); } - private Lobby getOwnedLobby(SimpMessageHeaderAccessor headerAccessor, Principal principal) { - Lobby lobby = (Lobby)headerAccessor.getSessionAttributes().get(SessionAttributes.ATTR_LOBBY); - if (lobby == null) { - throw new UserOwnsNoLobbyException("User " + principal.getName() + " is not in a lobby"); - } + private Lobby getOwnedLobby(Principal principal) { + Lobby lobby = getLobby(principal); if (!lobby.isOwner(principal.getName())) { - throw new UserOwnsNoLobbyException("User " + principal.getName() + " does not own the lobby he's in"); + throw new UserIsNotOwnerException(principal.getName()); } return lobby; } - private static class UserOwnsNoLobbyException extends ApiMisuseException { - UserOwnsNoLobbyException(String message) { - super(message); + private Lobby getLobby(Principal principal) { + Lobby lobby = playerRepository.find(principal.getName()).getLobby(); + if (lobby == null) { + throw new UserNotInLobbyException(principal.getName()); + } + return lobby; + } + + private static class UserNotInLobbyException extends ApiMisuseException { + UserNotInLobbyException(String userName) { + super("User " + userName + " is not in a lobby, create or join a game first"); + } + } + + private static class UserIsNotOwnerException extends ApiMisuseException { + UserIsNotOwnerException(String userName) { + super("User " + userName + " does not own the lobby he's in"); } } diff --git a/src/main/java/org/luxons/sevenwonders/game/Lobby.java b/src/main/java/org/luxons/sevenwonders/game/Lobby.java index b115d32e..525321b2 100644 --- a/src/main/java/org/luxons/sevenwonders/game/Lobby.java +++ b/src/main/java/org/luxons/sevenwonders/game/Lobby.java @@ -2,7 +2,6 @@ package org.luxons.sevenwonders.game; import java.util.ArrayList; import java.util.List; -import java.util.stream.Collectors; import org.luxons.sevenwonders.game.data.GameDefinition; @@ -16,7 +15,9 @@ public class Lobby { private final GameDefinition gameDefinition; - private List players; + private final List players; + + private Settings settings; private State state = State.LOBBY; @@ -26,6 +27,7 @@ public class Lobby { this.owner = owner; this.gameDefinition = gameDefinition; this.players = new ArrayList<>(gameDefinition.getMinPlayers()); + this.settings = new Settings(); players.add(owner); } @@ -41,7 +43,15 @@ public class Lobby { return players; } - public synchronized int addPlayer(Player player) throws GameAlreadyStartedException, PlayerOverflowException { + public Settings getSettings() { + return settings; + } + + public void setSettings(Settings settings) { + this.settings = settings; + } + + public synchronized void addPlayer(Player player) throws GameAlreadyStartedException, PlayerOverflowException { if (hasStarted()) { throw new GameAlreadyStartedException(); } @@ -51,9 +61,8 @@ public class Lobby { if (playerNameAlreadyUsed(player.getDisplayName())) { throw new PlayerNameAlreadyUsedException(player.getDisplayName()); } - int playerId = players.size(); + player.setIndex(players.size()); players.add(player); - return playerId; } private boolean hasStarted() { @@ -68,7 +77,7 @@ public class Lobby { return players.stream().anyMatch(p -> p.getDisplayName().equals(name)); } - public synchronized Game startGame(Settings settings) throws PlayerUnderflowException { + public synchronized Game startGame() throws PlayerUnderflowException { if (!hasEnoughPlayers()) { throw new PlayerUnderflowException(); } @@ -82,10 +91,16 @@ public class Lobby { } public void reorderPlayers(List orderedUserNames) { - players = orderedUserNames.stream().map(this::getPlayer).collect(Collectors.toList()); + List formerList = new ArrayList<>(players); + players.clear(); + for (int i = 0; i < orderedUserNames.size(); i++) { + Player player = getPlayer(formerList, orderedUserNames.get(i)); + players.add(player); + player.setIndex(i); + } } - private Player getPlayer(String userName) { + private static Player getPlayer(List players, String userName) { return players.stream() .filter(p -> p.getUserName().equals(userName)) .findAny() diff --git a/src/main/java/org/luxons/sevenwonders/game/Player.java b/src/main/java/org/luxons/sevenwonders/game/Player.java index d550cf60..3e25cb80 100644 --- a/src/main/java/org/luxons/sevenwonders/game/Player.java +++ b/src/main/java/org/luxons/sevenwonders/game/Player.java @@ -1,26 +1,40 @@ package org.luxons.sevenwonders.game; -public class Player { +import com.fasterxml.jackson.annotation.JsonIgnore; - private final String displayName; +public class Player { private final String userName; + private String displayName; + private int index; - public Player(String displayName, String userName) { - this.displayName = displayName; + private transient Lobby lobby; + + private transient Game game; + + public Player(String userName) { this.userName = userName; } - public String getDisplayName() { - return displayName; + public Player(String displayName, String userName) { + this(userName); + setDisplayName(displayName); } public String getUserName() { return userName; } + public String getDisplayName() { + return displayName; + } + + public void setDisplayName(String displayName) { + this.displayName = displayName; + } + public int getIndex() { return index; } @@ -28,4 +42,22 @@ public class Player { public void setIndex(int index) { this.index = index; } + + @JsonIgnore + public Lobby getLobby() { + return lobby; + } + + public void setLobby(Lobby lobby) { + this.lobby = lobby; + } + + @JsonIgnore + public Game getGame() { + return game; + } + + public void setGame(Game game) { + this.game = game; + } } diff --git a/src/main/java/org/luxons/sevenwonders/game/Settings.java b/src/main/java/org/luxons/sevenwonders/game/Settings.java index 7159670d..ae85fc46 100644 --- a/src/main/java/org/luxons/sevenwonders/game/Settings.java +++ b/src/main/java/org/luxons/sevenwonders/game/Settings.java @@ -6,6 +6,8 @@ import java.util.Random; import org.luxons.sevenwonders.game.data.definitions.WonderSide; +import com.fasterxml.jackson.annotation.JsonIgnore; + public class Settings { private int nbPlayers = -1; @@ -30,6 +32,7 @@ public class Settings { wonPointsPerVictoryPerAge.put(3, 5); } + @JsonIgnore public int getNbPlayers() { if (nbPlayers < 0) { throw new IllegalStateException("The number of players has not been initialized"); @@ -97,6 +100,7 @@ public class Settings { this.randomSeedForTests = randomSeedForTests; } + @JsonIgnore public Random getRandom() { return randomSeedForTests > 0 ? new Random(randomSeedForTests) : new Random(); } diff --git a/src/main/java/org/luxons/sevenwonders/repositories/LobbyRepository.java b/src/main/java/org/luxons/sevenwonders/repositories/LobbyRepository.java index 21348890..3561161b 100644 --- a/src/main/java/org/luxons/sevenwonders/repositories/LobbyRepository.java +++ b/src/main/java/org/luxons/sevenwonders/repositories/LobbyRepository.java @@ -42,14 +42,6 @@ public class LobbyRepository { return lobby; } - public Lobby find(String gameName) { - Lobby lobby = lobbies.get(gameName); - if (lobby == null) { - throw new LobbyNotFoundException(gameName); - } - return lobby; - } - public Lobby find(long lobbyId) { Lobby lobby = lobbiesById.get(lobbyId); if (lobby == null) { diff --git a/src/main/java/org/luxons/sevenwonders/repositories/PlayerRepository.java b/src/main/java/org/luxons/sevenwonders/repositories/PlayerRepository.java new file mode 100644 index 00000000..e671088f --- /dev/null +++ b/src/main/java/org/luxons/sevenwonders/repositories/PlayerRepository.java @@ -0,0 +1,61 @@ +package org.luxons.sevenwonders.repositories; + +import java.util.HashMap; +import java.util.Map; + +import org.luxons.sevenwonders.errors.ApiMisuseException; +import org.luxons.sevenwonders.game.Player; +import org.springframework.stereotype.Repository; + +@Repository +public class PlayerRepository { + + private Map players = new HashMap<>(); + + public Player updateOrCreatePlayer(String userName, String displayName) { + if (players.containsKey(userName)) { + return updatePlayerName(userName, displayName); + } else { + return createPlayer(userName, displayName); + } + } + + private Player createPlayer(String userName, String displayName) { + Player player = new Player(displayName, userName); + add(player); + return player; + } + + private void add(Player player) { + if (players.containsKey(player.getUserName())) { + throw new PlayerAlreadyExistsException(player.getUserName()); + } + players.put(player.getUserName(), player); + } + + private Player updatePlayerName(String userName, String displayName) { + Player player = find(userName); + player.setDisplayName(displayName); + return player; + } + + public Player find(String userName) { + Player player = players.get(userName); + if (player == null) { + throw new PlayerNotFoundException(userName); + } + return player; + } + + public static class PlayerNotFoundException extends ApiMisuseException { + PlayerNotFoundException(String userName) { + super("Player '" + userName + "' doesn't exist"); + } + } + + private static class PlayerAlreadyExistsException extends ApiMisuseException { + PlayerAlreadyExistsException(String userName) { + super("Player '" + userName + "' already exists"); + } + } +} diff --git a/src/main/java/org/luxons/sevenwonders/session/SessionAttributes.java b/src/main/java/org/luxons/sevenwonders/session/SessionAttributes.java deleted file mode 100644 index 8c69d8c3..00000000 --- a/src/main/java/org/luxons/sevenwonders/session/SessionAttributes.java +++ /dev/null @@ -1,8 +0,0 @@ -package org.luxons.sevenwonders.session; - -public class SessionAttributes { - - public static final String ATTR_LOBBY = "lobby"; - - public static final String ATTR_GAME = "game"; -} -- cgit