fix(master): Use createMatch for implicit match creation (#821)

* refactor(server): Factor out metadata factory function

* refactor(master): Set default numPlayers for master#onSync

* fix(master): Use `createGame` for implicit match creation

Closes #722

Uses Storage API’s `createGame` method when implicitly creating a match 
from the master’s `onSync` handler to ensure `initialState` gets stored 
as well as any other initialisation code an implementation may include 
in `createGame`.

* refactor(master): Simplify fetch logic & tidy up code style

* test(master): Fix master sync tests

* test(master): Add tests for updating a match with no metadata

* fix(master): Use `createMatch` for compatibility with #806

Author: delucis

src/master/master.test.ts

@@ -7,6 +7,7 @@
  */
 
 import * as ActionCreators from '../core/action-creators';
+import { InitializeGame } from '../core/initialize';
 import { InMemory } from '../server/db/inmemory';
 import {
   Master,
@@ -16,7 +17,7 @@ import {
   isActionFromAuthenticPlayer,
 } from './master';
 import { error } from '../core/logger';
-import { Server } from '../types';
+import { Server, State } from '../types';
 import * as StorageAPI from '../server/db/base';
 import * as dateMock from 'jest-date-mock';
 
@@ -43,7 +44,8 @@ function TransportAPI(send = jest.fn(), sendAll = jest.fn()) {
 
 describe('sync', () => {
   const send = jest.fn();
-  const master = new Master(game, new InMemory(), TransportAPI(send));
+  const db = new InMemory();
+  const master = new Master(game, db, TransportAPI(send));
 
   beforeEach(() => {
     jest.clearAllMocks();
@@ -59,19 +61,21 @@ describe('sync', () => {
   });
 
   test('sync a second time does not create a game', async () => {
+    const fetchResult = db.fetch('matchID', { metadata: true });
     await master.onSync('matchID', '0', 2);
-    expect(send).toHaveBeenCalled();
+    expect(db.fetch('matchID', { metadata: true })).toMatchObject(fetchResult);
   });
 
   test('should not have metadata', async () => {
-    await master.onSync('matchID', '0', 2);
+    db.setState('oldGameID', {} as State);
+    await master.onSync('oldGameID', '0');
     // [0][0] = first call, first argument
-    expect(send.mock.calls[0][0].args[3]).toBeUndefined();
+    expect(send.mock.calls[0][0].args[1].filteredMetadata).toBeUndefined();
   });
 
   test('should have metadata', async () => {
     const db = new InMemory();
-    const dbMetadata = {
+    const metadata = {
       gameName: 'tic-tac-toe',
       setupData: {},
       players: {
@@ -89,7 +93,7 @@ describe('sync', () => {
       createdAt: 0,
       updatedAt: 0,
     };
-    db.setMetadata('matchID', dbMetadata);
+    db.createMatch('matchID', { metadata, initialState: {} as State });
     const masterWithMetadata = new Master(game, db, TransportAPI(send));
     await masterWithMetadata.onSync('matchID', '0', 2);
 
@@ -279,6 +283,42 @@ describe('update', () => {
     const { metadata } = db.fetch(id, { metadata: true });
     expect(metadata.updatedAt).toEqual(updatedAt.getTime());
   });
+
+  test('processes update if there is no metadata', async () => {
+    const id = 'gameWithoutMetadata';
+    const db = new InMemory();
+    const masterWithoutMetadata = new Master(game, db, TransportAPI(send));
+    // Store state manually to bypass automatic metadata initialization on sync.
+    let state = InitializeGame({ game });
+    expect(state.ctx.turn).toBe(1);
+    db.setState(id, state);
+    // Dispatch update to end the turn.
+    const event = ActionCreators.gameEvent('endTurn', null, '0');
+    await masterWithoutMetadata.onUpdate(event, 0, id, '0');
+    // Confirm the turn ended.
+    let metadata: undefined | Server.MatchData;
+    ({ state, metadata } = db.fetch(id, { state: true, metadata: true }));
+    expect(state.ctx.turn).toBe(2);
+    expect(metadata).toBeUndefined();
+  });
+
+  test('processes update if there is no metadata with async DB', async () => {
+    const id = 'gameWithoutMetadata';
+    const db = new InMemoryAsync();
+    const masterWithoutMetadata = new Master(game, db, TransportAPI(send));
+    // Store state manually to bypass automatic metadata initialization on sync.
+    let state = InitializeGame({ game });
+    expect(state.ctx.turn).toBe(1);
+    db.setState(id, state);
+    // Dispatch update to end the turn.
+    const event = ActionCreators.gameEvent('endTurn', null, '0');
+    await masterWithoutMetadata.onUpdate(event, 0, id, '0');
+    // Confirm the turn ended.
+    let metadata: undefined | Server.MatchData;
+    ({ state, metadata } = db.fetch(id, { state: true, metadata: true }));
+    expect(state.ctx.turn).toBe(2);
+    expect(metadata).toBeUndefined();
+  });
 });
 
 describe('playerView', () => {

src/master/master.ts

@@ -23,6 +23,7 @@ import {
   LogEntry,
   PlayerID,
 } from '../types';
+import { createMetadata } from '../server/util';
 import * as StorageAPI from '../server/db/base';
 
 export const getPlayerMetadata = (
@@ -216,13 +217,11 @@ export class Master {
     const key = matchID;
 
     let state: State;
-    let result: StorageAPI.FetchResult<{ state: true }>;
     if (IsSynchronous(this.storageAPI)) {
-      result = this.storageAPI.fetch(key, { state: true });
+      ({ state } = this.storageAPI.fetch(key, { state: true }));
     } else {
-      result = await this.storageAPI.fetch(key, { state: true });
+      ({ state } = await this.storageAPI.fetch(key, { state: true }));
     }
-    state = result.state;
 
     if (state === undefined) {
       logging.error(`game not found, matchID=[${key}]`);
@@ -334,66 +333,53 @@ export class Master {
    * Called when the client connects / reconnects.
    * Returns the latest game state and the entire log.
    */
-  async onSync(matchID: string, playerID: string, numPlayers: number) {
+  async onSync(matchID: string, playerID: string, numPlayers = 2) {
     const key = matchID;
 
-    let state: State;
-    let initialState: State;
-    let log: LogEntry[];
-    let matchData: Server.MatchData;
-    let filteredMetadata: FilteredMetadata;
-    let result: StorageAPI.FetchResult<{
-      state: true;
-      metadata: true;
-      log: true;
-      initialState: true;
-    }>;
+    const fetchOpts = {
+      state: true,
+      metadata: true,
+      log: true,
+      initialState: true,
+    } as const;
+
+    let fetchResult: StorageAPI.FetchResult<typeof fetchOpts>;
 
     if (IsSynchronous(this.storageAPI)) {
-      result = this.storageAPI.fetch(key, {
-        state: true,
-        metadata: true,
-        log: true,
-        initialState: true,
-      });
+      fetchResult = this.storageAPI.fetch(key, fetchOpts);
     } else {
-      result = await this.storageAPI.fetch(key, {
-        state: true,
-        metadata: true,
-        log: true,
-        initialState: true,
-      });
+      fetchResult = await this.storageAPI.fetch(key, fetchOpts);
     }
 
-    state = result.state;
-    initialState = result.initialState;
-    log = result.log;
-    matchData = result.metadata;
-
-    if (matchData) {
-      filteredMetadata = Object.values(matchData.players).map(player => {
-        const { credentials, ...filteredData } = player;
-        return filteredData;
-      });
-    }
+    let { state, initialState, log, metadata } = fetchResult;
 
     // If the game doesn't exist, then create one on demand.
     // TODO: Move this out of the sync call.
     if (state === undefined) {
       initialState = state = InitializeGame({ game: this.game, numPlayers });
-
-      this.subscribeCallback({
-        state,
-        matchID,
+      metadata = createMetadata({
+        game: this.game,
+        unlisted: true,
+        numPlayers,
       });
 
+      this.subscribeCallback({ state, matchID });
+
       if (IsSynchronous(this.storageAPI)) {
-        this.storageAPI.setState(key, state);
+        this.storageAPI.createMatch(key, { initialState, metadata });
       } else {
-        await this.storageAPI.setState(key, state);
+        await this.storageAPI.createMatch(key, { initialState, metadata });
       }
     }
 
+    let filteredMetadata: FilteredMetadata;
+    if (metadata) {
+      filteredMetadata = Object.values(metadata.players).map(player => {
+        const { credentials, ...filteredData } = player;
+        return filteredData;
+      });
+    }
+
     const filteredState = {
       ...state,
       G: this.game.playerView(state.G, state.ctx, playerID),

src/server/api.ts

@@ -15,6 +15,7 @@ import cors from '@koa/cors';
 import { InitializeGame } from '../core/initialize';
 import * as StorageAPI from './db/base';
 import { Server, LobbyAPI, Game } from '../types';
+import { createMetadata } from './util';
 
 /**
  * Creates a new match.
@@ -44,18 +45,7 @@ export const CreateMatch = async ({
 }) => {
   if (!numPlayers || typeof numPlayers !== 'number') numPlayers = 2;
 
-  const metadata: Server.MatchData = {
-    gameName: game.name,
-    unlisted: !!unlisted,
-    players: {},
-    createdAt: Date.now(),
-    updatedAt: Date.now(),
-  };
-  if (setupData !== undefined) metadata.setupData = setupData;
-  for (let playerIndex = 0; playerIndex < numPlayers; playerIndex++) {
-    metadata.players[playerIndex] = { id: playerIndex };
-  }
-
+  const metadata = createMetadata({ game, numPlayers, setupData, unlisted });
   const matchID = uuid();
   const initialState = InitializeGame({ game, numPlayers, setupData });
 

src/server/util.ts

@@ -0,0 +1,32 @@
+import { Server, Game } from '../types';
+
+/**
+ * Creates a new match metadata object.
+ */
+export const createMetadata = ({
+  game,
+  unlisted,
+  setupData,
+  numPlayers,
+}: {
+  game: Game;
+  numPlayers: number;
+  setupData?: any;
+  unlisted?: boolean;
+}): Server.MatchData => {
+  const metadata: Server.MatchData = {
+    gameName: game.name,
+    unlisted: !!unlisted,
+    players: {},
+    createdAt: Date.now(),
+    updatedAt: Date.now(),
+  };
+
+  if (setupData !== undefined) metadata.setupData = setupData;
+
+  for (let playerIndex = 0; playerIndex < numPlayers; playerIndex++) {
+    metadata.players[playerIndex] = { id: playerIndex };
+  }
+
+  return metadata;
+};