Refactor and add tests for game manager.

The manager is now wrapper around an Engine and a Game. The former
represents the 'pure' business data whilst the latter represents the
persisted version of that.

The manager server is the GenServer component which wraps the manager
as a process that can be referenced by name.

lib/twenty_forty_eight/game/manager.ex

@@ -1,111 +1,97 @@
 defmodule TwentyFortyEight.Game.Manager do
   @moduledoc """
-  Store game state and process business logic events.
+  Manage an `Engine` and its persistence to the database.
+
+  An `Engine` is the primary interface to a game instance, whereas a `Game` is
+  the primary mechanism for persisting that instance. A `Manager` provides a
+  layer above these two, loading a `Game` to create an `Engine` and saving an
+  `Engine` to the DB as a `Game`.
   """
-  use GenServer, restart: :transient
+  @enforce_keys [:name, :engine]
+  defstruct [:name, :engine]
 
   alias TwentyFortyEight.Game.{Board, Engine, Game}
 
-  @registry TwentyFortyEight.Game.Registry
-  @supervisor TwentyFortyEight.Game.Supervisor
-  # Shutdown the server after 10 minutes to avoid dangling processes.
-  @timeout 10 * 60 * 1_000
-
   @doc """
-  Ensure a manager is running for the game named `name`.
+  Return a manager for `Game` named `name` that exists in the DB.
 
-  If no game is currently running a new one is started with state loaded from
-  `state`.
+  If no such game exists return `{:error, :not_found}`.
   """
-  def start(name) when is_binary(name) do
-    case Registry.lookup(@registry, name) do
-      [{pid, _value}] -> {:ok, pid}
-      [] -> DynamicSupervisor.start_child(@supervisor, {__MODULE__, name})
+  def get(name) do
+    with {:ok, game} <- get_game(name) do
+      {:ok, create_manager(game)}
     end
   end
 
   @doc """
-  Increment the game by one move.
+  Increment the `Engine` by one move.
   """
-  def tick(name, move) do
-    GenServer.call(via_tuple(name), {:tick, move})
+  def tick(%__MODULE__{} = manager, move) do
+    engine = Engine.tick(manager.engine, move)
+    %__MODULE__{manager | engine: engine}
   end
 
   @doc """
-  Return state data suitable for updating an external store.
-
-  This state is not sufficient for restarting a game, but is intended by
-  updating pre-existing state stored elsewhere (e.g. in a `%Game{}`).
+  Persist the `Engine` as a `Game` to the DB.
   """
-  def state(name) do
-    GenServer.call(via_tuple(name), :state)
+  def save(%__MODULE__{} = manager) do
+    with {:ok, %Game{} = game} <- get_game(manager.name),
+         {:ok, %Game{}} <- Game.update(game, dump_engine(manager.engine)) do
+      :ok
+    end
   end
 
-  def start_link(name) do
-    GenServer.start_link(__MODULE__, name, name: via_tuple(name))
+  defp get_game(name) do
+    case Game.get_by_slug(name) do
+      nil ->
+        {:error, :not_found}
+
+      game ->
+        {:ok, game}
+    end
   end
 
-  @impl true
-  def init(name) do
-    Process.flag(:trap_exit, true)
-    game_state = Game.get_by_slug(name)
-    engine = create_or_restore_engine(game_state)
+  defp create_manager(%Game{} = game) do
+    engine = load_engine(game)
 
-    state = %{
-      game: game_state,
+    %__MODULE__{
+      name: game.slug,
       engine: engine
     }
-
-    {:ok, state, @timeout}
-  end
-
-  @impl true
-  def handle_call({:tick, move}, _from, state) do
-    state = %{state | engine: Engine.tick(state.engine, move)}
-    {:reply, :ok, state, @timeout}
-  end
-
-  @impl true
-  def handle_call(:state, _from, state) do
-    {:reply, mutable_state(state), state, @timeout}
-  end
-
-  @impl true
-  def handle_info(:timeout, state) do
-    handle_exit(state)
-    {:stop, :shutdown, state}
   end
 
-  @impl true
-  def handle_info({:EXIT, _from, reason}, state) do
-    handle_exit(state)
-    {:stop, reason, state, @timeout}
-  end
-
-  defp handle_exit(%{game: game} = state) do
-    {:ok, _} = persist_state(game, mutable_state(state))
-  end
+  defp load_engine(%Game{state: :new, board: nil} = game) do
+    board = Board.init(game.num_rows, game.num_cols, game.starting_number, game.num_obstacles)
+    opts = [turn_start_number: game.turn_start_number, winning_number: game.winning_number]
 
-  defp via_tuple(name) do
-    {:via, Registry, {@registry, name}}
+    Engine.init(board, opts)
   end
 
-  defp mutable_state(%{engine: engine} = _state) do
-    Map.take(engine, [:board, :score, :turns, :state])
-  end
+  defp load_engine(game) do
+    state = %{
+      board: load_board(game),
+      score: game.score,
+      turns: game.turns,
+      state: game.state,
+      turn_start_number: game.turn_start_number,
+      winning_number: game.winning_number
+    }
 
-  defp persist_state(game, state) do
-    Game.update(game, %{state | board: encode_board(state.board)})
+    Engine.restore(state)
   end
 
-  defp encode_board(%Board{cells: cells} = board) do
-    # Encode the cells for JSON serialisation.
-    # Note that we omit the dimensions as these are already stored on the Game.
-    cell_values = for row <- 1..board.num_rows, col <- 1..board.num_cols, do: cells[{row, col}]
-    %{cells: cell_values}
+  defp dump_engine(engine) do
+    %{
+      board: dump_board(engine.board),
+      score: engine.score,
+      turns: engine.turns,
+      state: engine.state,
+      turn_start_number: engine.turn_start_number,
+      winning_number: engine.winning_number
+    }
   end
 
-  defp decode_board(%Game{board: %{"cells" => cell_values}} = game) do
+  defp load_board(%Game{board: %{"cells" => cell_values}} = game) do
     # Decode the cells from JSON serialisation as a Board.
     coordinates = for row <- 1..game.num_rows, col <- 1..game.num_cols, do: {row, col}
 
@@ -123,23 +109,10 @@ defmodule TwentyFortyEight.Game.Manager do
     }
   end
 
-  defp create_or_restore_engine(%Game{state: :new, board: nil} = game) do
-    board = Board.init(game.num_rows, game.num_cols, game.starting_number, game.num_obstacles)
-    opts = [turn_start_number: game.turn_start_number, winning_number: game.winning_number]
-
-    Engine.init(board, opts)
-  end
-
-  defp create_or_restore_engine(game) do
-    state = %{
-      board: decode_board(game),
-      score: game.score,
-      turns: game.turns,
-      state: game.state,
-      turn_start_number: game.turn_start_number,
-      winning_number: game.winning_number
-    }
-
-    Engine.restore(state)
+  defp dump_board(%Board{cells: cells} = board) do
+    # Encode the cells for JSON serialisation.
+    # Note that we omit the dimensions as these are already stored on the Game.
+    cell_values = for row <- 1..board.num_rows, col <- 1..board.num_cols, do: cells[{row, col}]
+    %{cells: cell_values}
   end
 end

lib/twenty_forty_eight/manager_server.ex

@@ -0,0 +1,82 @@
+defmodule TwentyFortyEight.Game.ManagerServer do
+  @moduledoc """
+  Process wrapper around a `Manager`.
+
+  The process times out 10 minutes after receiving the last message. The manager
+  is instructed to persist the game state if the process times out or if a
+  linked process exits.
+  """
+  use GenServer, restart: :transient
+
+  alias TwentyFortyEight.Game.Manager
+
+  @registry TwentyFortyEight.Game.Registry
+  @supervisor TwentyFortyEight.Game.Supervisor
+  # Shutdown the server after 10 minutes to avoid dangling processes.
+  @timeout 10 * 60 * 1_000
+
+  @doc """
+  Ensure a server is running for the game named `name`.
+  """
+  def start(%Manager{name: name} = manager) do
+    case Registry.lookup(@registry, name) do
+      [{pid, _value}] -> {:ok, pid}
+      [] -> DynamicSupervisor.start_child(@supervisor, {__MODULE__, manager})
+    end
+  end
+
+  def start_link(%Manager{name: name} = manager) do
+    GenServer.start_link(__MODULE__, manager, name: via_tuple(name))
+  end
+
+  @doc """
+  Increment the game by one move.
+  """
+  def tick(name, move) do
+    GenServer.call(via_tuple(name), {:tick, move})
+  end
+
+  @doc """
+  Return the `Manager` wrapped by this server.
+  """
+  def manager(name) do
+    GenServer.call(via_tuple(name), :manager)
+  end
+
+  @impl true
+  def init(manager) do
+    Process.flag(:trap_exit, true)
+    {:ok, manager, @timeout}
+  end
+
+  @impl true
+  def handle_call({:tick, move}, _from, manager) do
+    manager = Manager.tick(manager, move)
+    {:reply, :ok, manager, @timeout}
+  end
+
+  @impl true
+  def handle_call(:manager, _from, manager) do
+    {:reply, manager, manager, @timeout}
+  end
+
+  @impl true
+  def handle_info(:timeout, manager) do
+    handle_exit(manager)
+    {:stop, :shutdown, manager}
+  end
+
+  @impl true
+  def handle_info({:EXIT, _from, reason}, manager) do
+    handle_exit(manager)
+    {:stop, reason, manager, @timeout}
+  end
+
+  defp handle_exit(manager) do
+    Manager.save(manager)
+  end
+
+  defp via_tuple(name) do
+    {:via, Registry, {@registry, name}}
+  end
+end

lib/twenty_forty_eight_web/live/game_live.ex

@@ -1,8 +1,8 @@
 defmodule TwentyFortyEightWeb.GameLive do
   use TwentyFortyEightWeb, :live_view
 
-  alias TwentyFortyEight.Game.Game
   alias TwentyFortyEight.Game.Manager, as: GameManager
+  alias TwentyFortyEight.Game.ManagerServer, as: GameManagerServer
 
   # Support arrows keys, hjkl, and wasd for movement.
   @up_keys ~w(ArrowUp w k)
@@ -26,15 +26,15 @@ defmodule TwentyFortyEightWeb.GameLive do
   end
 
   def mount(%{"name" => name} = _params, _session, socket) do
-    case Game.get_by_slug(name) do
-      nil ->
+    case GameManager.get(name) do
+      {:error, :not_found} ->
         {:ok,
          socket
          |> put_flash(:error, "Could not find game with ID #{name}.")
          |> redirect(to: ~p"/")}
 
-      _game_state ->
-        {:ok, _pid} = GameManager.start(name)
+      {:ok, manager} ->
+        {:ok, _pid} = GameManagerServer.start(manager)
 
         if connected?(socket) do
           Phoenix.PubSub.subscribe(TwentyFortyEight.PubSub, name)
@@ -49,34 +49,46 @@ defmodule TwentyFortyEightWeb.GameLive do
   """
   def handle_event("move", %{"key" => key}, %{assigns: %{name: name, state: :running}} = socket)
       when key in @known_keys do
-    :ok = GameManager.tick(name, key_to_move(key))
-    Phoenix.PubSub.broadcast(TwentyFortyEight.PubSub, name, {:update, name})
+    socket =
+      key
+      |> key_to_move()
+      |> case do
+        {:ok, move} ->
+          :ok = GameManagerServer.tick(name, move)
+          Phoenix.PubSub.broadcast(TwentyFortyEight.PubSub, name, {:update, name})
+          socket
+
+        {:error, :unknown_key} ->
+          put_flash(socket, :error, "Could not handle key press #{key}.")
+      end
+
     {:noreply, socket}
   end
 
   def handle_event("move", _params, socket), do: {:noreply, socket}
 
   def handle_info({:update, name}, socket) do
-    {:noreply, assign_game_state(socket, name)}
+    {:noreply, assign_engine(socket, name)}
   end
 
-  defp key_to_move(up) when up in @up_keys, do: :up
-  defp key_to_move(down) when down in @down_keys, do: :down
-  defp key_to_move(left) when left in @left_keys, do: :left
-  defp key_to_move(right) when right in @right_keys, do: :right
+  defp key_to_move(up) when up in @up_keys, do: {:ok, :up}
+  defp key_to_move(down) when down in @down_keys, do: {:ok, :down}
+  defp key_to_move(left) when left in @left_keys, do: {:ok, :left}
+  defp key_to_move(right) when right in @right_keys, do: {:ok, :right}
+  defp key_to_move(_key), do: {:error, :unknown_key}
 
   defp assign_game(socket, name) do
     socket
     |> assign(name: name)
-    |> assign_game_state(name)
+    |> assign_engine(name)
   end
 
-  defp assign_game_state(socket, name) do
-    game_state = GameManager.state(name)
+  defp assign_engine(socket, name) do
+    %GameManager{engine: engine} = GameManagerServer.manager(name)
 
     socket
-    |> assign(Map.from_struct(game_state.board))
-    |> assign(game_state)
+    |> assign(Map.from_struct(engine.board))
+    |> assign(engine)
   end
 
   defp status_message(:running), do: ""