From 87ddff98d9c9ca6524d8b47633dd5c39ab48c476 Mon Sep 17 00:00:00 2001 From: "vdf.dev" Date: Sun, 25 Jul 2021 18:13:18 -0700 Subject: [PATCH 1/2] Decouple player view calculation from Server's Master --- src/client/transport/local.ts | 6 +- src/master/filter-player-view.test.ts | 208 ++++++++++++++++++ src/master/filter-player-view.ts | 82 +++++++ src/master/master.test.ts | 299 ++------------------------ src/master/master.ts | 115 ++++------ src/server/transport/socketio.test.ts | 16 +- src/server/transport/socketio.ts | 32 ++- 7 files changed, 387 insertions(+), 371 deletions(-) create mode 100644 src/master/filter-player-view.test.ts create mode 100644 src/master/filter-player-view.ts diff --git a/src/client/transport/local.ts b/src/client/transport/local.ts index 44a804979..b30c8f06c 100644 --- a/src/client/transport/local.ts +++ b/src/client/transport/local.ts @@ -22,6 +22,7 @@ import type { State, SyncInfo, } from '../../types'; +import { getFilterPlayerView } from '../../master/filter-player-view'; /** * Returns null if it is not a bot's turn. @@ -88,11 +89,12 @@ export class LocalMaster extends Master { } }; + const filterPlayerView = getFilterPlayerView(game); const transportAPI: TransportAPI = { send, - sendAll: (makePlayerData) => { + sendAll: (payload) => { for (const playerID in clientCallbacks) { - const data = makePlayerData(playerID); + const data = filterPlayerView(playerID, payload); send({ playerID, ...data }); } }, diff --git a/src/master/filter-player-view.test.ts b/src/master/filter-player-view.test.ts new file mode 100644 index 000000000..6adb390b9 --- /dev/null +++ b/src/master/filter-player-view.test.ts @@ -0,0 +1,208 @@ +import { getFilterPlayerView, redactLog } from './filter-player-view'; +import * as ActionCreators from '../core/action-creators'; +import { Master } from './master'; +import { InMemory } from '../server/db/inmemory'; + +function TransportAPI(send = jest.fn(), sendAll = jest.fn()) { + return { send, sendAll }; +} + +describe('playerView', () => { + const send = jest.fn(); + const sendAll = jest.fn(); + const game = { + playerView: (G, ctx, player) => { + return { ...G, player }; + }, + }; + const master = new Master(game, new InMemory(), TransportAPI(send, sendAll)); + + beforeAll(async () => { + await master.onSync('matchID', '0', undefined, 2); + }); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + test('sync', async () => { + await master.onSync('matchID', '0', undefined, 2); + expect(send.mock.calls[0][0].args[1].state).toMatchObject({ + G: { player: '0' }, + }); + }); + + test('update', async () => { + const action = ActionCreators.gameEvent('endTurn'); + await master.onSync('matchID', '0', undefined, 2); + await master.onUpdate(action, 0, 'matchID', '0'); + const payload = sendAll.mock.calls[sendAll.mock.calls.length - 1][0]; + const filterPlayerView = getFilterPlayerView(game); + + const transportData0 = filterPlayerView('0', payload); + const G_player0 = (transportData0.args[1] as any).G; + const transportData1 = filterPlayerView('1', payload); + const G_player1 = (transportData1.args[1] as any).G; + + expect(G_player0.player).toBe('0'); + expect(G_player1.player).toBe('1'); + }); +}); + +describe('redactLog', () => { + test('no-op with undefined log', () => { + const result = redactLog(undefined, '0'); + expect(result).toBeUndefined(); + }); + + test('no redactedMoves', () => { + const logEvents = [ + { + _stateID: 0, + turn: 0, + phase: '', + action: ActionCreators.gameEvent('endTurn'), + }, + ]; + const result = redactLog(logEvents, '0'); + expect(result).toMatchObject(logEvents); + }); + + test('redacted move is only shown with args to the player that made the move', () => { + const logEvents = [ + { + _stateID: 0, + turn: 0, + phase: '', + action: ActionCreators.makeMove('clickCell', [1, 2, 3], '0'), + redact: true, + }, + ]; + + // player that made the move + let result = redactLog(logEvents, '0'); + expect(result).toMatchObject(logEvents); + + // other player + result = redactLog(logEvents, '1'); + expect(result).toMatchObject([ + { + _stateID: 0, + turn: 0, + phase: '', + action: { + type: 'MAKE_MOVE', + payload: { + credentials: undefined, + playerID: '0', + type: 'clickCell', + }, + }, + }, + ]); + }); + + test('not redacted move is shown to all', () => { + const logEvents = [ + { + _stateID: 0, + turn: 0, + phase: '', + action: ActionCreators.makeMove('unclickCell', [1, 2, 3], '0'), + }, + ]; + + // player that made the move + let result = redactLog(logEvents, '0'); + expect(result).toMatchObject(logEvents); + // other player + result = redactLog(logEvents, '1'); + expect(result).toMatchObject(logEvents); + }); + + test('can explicitly set showing args to true', () => { + const logEvents = [ + { + _stateID: 0, + turn: 0, + phase: '', + action: ActionCreators.makeMove('unclickCell', [1, 2, 3], '0'), + }, + ]; + + // player that made the move + let result = redactLog(logEvents, '0'); + expect(result).toMatchObject(logEvents); + // other player + result = redactLog(logEvents, '1'); + expect(result).toMatchObject(logEvents); + }); + + test('events are not redacted', () => { + const logEvents = [ + { + _stateID: 0, + turn: 0, + phase: '', + action: ActionCreators.gameEvent('endTurn'), + }, + ]; + + // player that made the move + let result = redactLog(logEvents, '0'); + expect(result).toMatchObject(logEvents); + // other player + result = redactLog(logEvents, '1'); + expect(result).toMatchObject(logEvents); + }); + + test('make sure sync redacts the log', async () => { + const game = { + moves: { + A: (G) => G, + B: { + move: (G) => G, + redact: true, + }, + }, + }; + + const send = jest.fn(); + const master = new Master(game, new InMemory(), TransportAPI(send)); + + const actionA = ActionCreators.makeMove('A', ['not redacted'], '0'); + const actionB = ActionCreators.makeMove('B', ['redacted'], '0'); + + // test: ping-pong two moves, then sync and check the log + await master.onSync('matchID', '0', undefined, 2); + await master.onUpdate(actionA, 0, 'matchID', '0'); + await master.onUpdate(actionB, 1, 'matchID', '0'); + await master.onSync('matchID', '1', undefined, 2); + + const { log } = send.mock.calls[send.mock.calls.length - 1][0].args[1]; + expect(log).toMatchObject([ + { + action: { + type: 'MAKE_MOVE', + payload: { + type: 'A', + args: ['not redacted'], + playerID: '0', + }, + }, + _stateID: 0, + }, + { + action: { + type: 'MAKE_MOVE', + payload: { + type: 'B', + args: null, + playerID: '0', + }, + }, + _stateID: 1, + }, + ]); + }); +}); diff --git a/src/master/filter-player-view.ts b/src/master/filter-player-view.ts new file mode 100644 index 000000000..fdff3d4d8 --- /dev/null +++ b/src/master/filter-player-view.ts @@ -0,0 +1,82 @@ +import { PlayerView } from '../plugins/main'; +import { createPatch } from 'rfc6902'; +import type { Game, State, LogEntry, PlayerID } from '../types'; +import type { TransportData, IntermediateTransportData } from './master'; + +const applyPlayerView = ( + game: Game, + playerID: string, + state: State +): State => ({ + ...state, + G: game.playerView(state.G, state.ctx, playerID), + plugins: PlayerView(state, { playerID, game }), + deltalog: undefined, + _undo: [], + _redo: [], +}); + +/** Gets a function that filters the TransportData for a given player and game. */ +export const getFilterPlayerView = (game: Game) => ( + playerID: string, + payload: IntermediateTransportData +): TransportData => { + if (payload.type === 'patch') { + const [matchID, stateID, prevState, state] = payload.args; + const log = redactLog(state.deltalog, playerID); + const filteredState = applyPlayerView(game, playerID, state); + const newStateID = state._stateID; + const prevFilteredState = applyPlayerView(game, playerID, prevState); + const patch = createPatch(prevFilteredState, filteredState); + return { + type: 'patch', + args: [matchID, stateID, newStateID, patch, log], + }; + } else if (payload.type === 'update') { + const [matchID, state] = payload.args; + const log = redactLog(state.deltalog, playerID); + const filteredState = applyPlayerView(game, playerID, state); + return { + type: 'update', + args: [matchID, filteredState, log], + }; + } else { + return payload; + } +}; + +/** + * Redact the log. + * + * @param {Array} log - The game log (or deltalog). + * @param {String} playerID - The playerID that this log is + * to be sent to. + */ +export function redactLog(log: LogEntry[], playerID: PlayerID) { + if (log === undefined) { + return log; + } + + return log.map((logEvent) => { + // filter for all other players and spectators. + if (playerID !== null && +playerID === +logEvent.action.payload.playerID) { + return logEvent; + } + + if (logEvent.redact !== true) { + return logEvent; + } + + const payload = { + ...logEvent.action.payload, + args: null, + }; + const filteredEvent = { + ...logEvent, + action: { ...logEvent.action, payload }, + }; + + const { redact, ...remaining } = filteredEvent; + return remaining; + }); +} diff --git a/src/master/master.test.ts b/src/master/master.test.ts index fb2021bdf..97dcbec48 100644 --- a/src/master/master.test.ts +++ b/src/master/master.test.ts @@ -9,7 +9,7 @@ import * as ActionCreators from '../core/action-creators'; import { InitializeGame } from '../core/initialize'; import { InMemory } from '../server/db/inmemory'; -import { Master, redactLog } from './master'; +import { Master } from './master'; import { error } from '../core/logger'; import type { Game, Server, State, Ctx, LogEntry } from '../types'; import { Auth } from '../server/auth'; @@ -171,12 +171,8 @@ describe('sync', () => { }); describe('update', () => { - let sendAllReturn; - const send = jest.fn(); - const sendAll = jest.fn((arg) => { - sendAllReturn = arg; - }); + const sendAll = jest.fn(); const game = { moves: { A: (G) => G, @@ -190,24 +186,18 @@ describe('update', () => { db = new InMemory(); master = new Master(game, db, TransportAPI(send, sendAll)); await master.onSync('matchID', '0', undefined, 2); - sendAllReturn = undefined; jest.clearAllMocks(); }); test('basic', async () => { await master.onUpdate(action, 0, 'matchID', '0'); expect(sendAll).toBeCalled(); - expect(sendAllReturn).not.toBeUndefined(); - - const value = sendAllReturn('0'); + const value = sendAll.mock.calls[0][0]; expect(value.type).toBe('update'); expect(value.args[0]).toBe('matchID'); expect(value.args[1]).toMatchObject({ G: {}, - deltalog: undefined, - _redo: [], _stateID: 1, - _undo: [], ctx: { currentPlayer: '1', numPlayers: 2, @@ -217,20 +207,6 @@ describe('update', () => { turn: 2, }, }); - - expect(value.args[2]).toMatchObject([ - { - action: { - payload: { - args: undefined, - credentials: undefined, - playerID: undefined, - type: 'endTurn', - }, - type: 'GAME_EVENT', - }, - }, - ]); }); test('invalid matchID', async () => { @@ -340,10 +316,9 @@ describe('update', () => { master.onSync('matchID', '1', undefined, 2), ]); - const G_player0 = sendAllReturn('0').args[1].G; - const G_player1 = sendAllReturn('1').args[1].G; + const G = sendAll.mock.calls[sendAll.mock.calls.length - 1][0].args[1].G; - expect(G_player0).toMatchObject({ + expect(G).toMatchObject({ players: { '0': { cards: ['card1'], @@ -352,15 +327,6 @@ describe('update', () => { cards: ['card0'], discardedCards: ['card3'], }); - expect(G_player1).toMatchObject({ - players: { - '1': { - cards: ['card2'], - }, - }, - cards: ['card0'], - discardedCards: ['card3'], - }); }); describe('undo / redo', () => { @@ -524,12 +490,8 @@ describe('update', () => { }); describe('patch', () => { - let sendAllReturn; - const send = jest.fn(); - const sendAll = jest.fn((arg) => { - sendAllReturn = arg; - }); + const sendAll = jest.fn(); const db = new InMemory(); const master = new Master( { @@ -592,39 +554,21 @@ describe('patch', () => { }); beforeEach(() => { - sendAllReturn = undefined; jest.clearAllMocks(); }); test('basic', async () => { await master.onUpdate(move, 0, 'matchID', '0'); expect(sendAll).toBeCalled(); - expect(sendAllReturn).not.toBeUndefined(); - const value = sendAllReturn('0'); + const value = sendAll.mock.calls[0][0]; expect(value.type).toBe('patch'); expect(value.args[0]).toBe('matchID'); expect(value.args[1]).toBe(0); - expect(value.args[2]).toBe(1); - expect(value.args[3]).toMatchObject([ - { op: 'remove', path: '/G/players/0/cards/0' }, - { op: 'add', path: '/G/discardedCards/-', value: 'card3' }, - { op: 'replace', path: '/ctx/numMoves', value: 1 }, - { op: 'replace', path: '/_stateID', value: 1 }, - ]); - - expect(value.args[4]).toMatchObject([ - { - action: { - payload: { - args: null, - playerID: '0', - type: 'A', - }, - type: 'MAKE_MOVE', - }, - }, - ]); + // prevState -- had a card + expect(value.args[2].G.players[0].cards).toEqual(['card3']); + // state -- doesnt have a card anymore + expect(value.args[3].G.players[0].cards).toEqual([]); }); test('invalid matchID', async () => { @@ -728,12 +672,8 @@ describe('patch', () => { }); describe('connectionChange', () => { - let sendAllReturn; - const send = jest.fn(); - const sendAll = jest.fn((arg) => { - sendAllReturn = arg; - }); + const sendAll = jest.fn(); const db = new InMemory(); const master = new Master(game, db, TransportAPI(send, sendAll)); @@ -760,7 +700,6 @@ describe('connectionChange', () => { db.createMatch('matchID', { metadata, initialState: {} as State }); beforeEach(() => { - sendAllReturn = undefined; master.subscribe(({ state }) => { validateNotTransientState(state); }); @@ -783,7 +722,7 @@ describe('connectionChange', () => { { id: 0, name: 'Alice', isConnected: true }, { id: 1, name: 'Bob', isConnected: false }, ]; - const sentMessage = sendAllReturn('0'); + const sentMessage = sendAll.mock.calls[0][0]; expect(sentMessage.type).toEqual('matchData'); expect(sentMessage.args[1]).toMatchObject(expectedMetadata); }); @@ -832,53 +771,6 @@ describe('connectionChange', () => { }); }); -describe('playerView', () => { - let sendAllReturn; - let sendReturn; - - const send = jest.fn((arg) => { - sendReturn = arg; - }); - const sendAll = jest.fn((arg) => { - sendAllReturn = arg; - }); - const game = { - playerView: (G, ctx, player) => { - return { ...G, player }; - }, - }; - const master = new Master(game, new InMemory(), TransportAPI(send, sendAll)); - - beforeAll(async () => { - await master.onSync('matchID', '0', undefined, 2); - }); - - beforeEach(() => { - sendReturn = undefined; - sendAllReturn = undefined; - jest.clearAllMocks(); - }); - - test('sync', async () => { - await master.onSync('matchID', '0', undefined, 2); - expect(sendReturn.args[1].state).toMatchObject({ - G: { player: '0' }, - }); - }); - - test('update', async () => { - const action = ActionCreators.gameEvent('endTurn'); - await master.onSync('matchID', '0', undefined, 2); - await master.onUpdate(action, 0, 'matchID', '0'); - - const G_player0 = sendAllReturn('0').args[1].G; - const G_player1 = sendAllReturn('1').args[1].G; - - expect(G_player0.player).toBe('0'); - expect(G_player1.player).toBe('1'); - }); -}); - describe('subscribe', () => { const callback = jest.fn(); @@ -1102,170 +994,9 @@ describe('authentication', () => { }); }); -describe('redactLog', () => { - test('no-op with undefined log', () => { - const result = redactLog(undefined, '0'); - expect(result).toBeUndefined(); - }); - - test('no redactedMoves', () => { - const logEvents = [ - { - _stateID: 0, - turn: 0, - phase: '', - action: ActionCreators.gameEvent('endTurn'), - }, - ]; - const result = redactLog(logEvents, '0'); - expect(result).toMatchObject(logEvents); - }); - - test('redacted move is only shown with args to the player that made the move', () => { - const logEvents = [ - { - _stateID: 0, - turn: 0, - phase: '', - action: ActionCreators.makeMove('clickCell', [1, 2, 3], '0'), - redact: true, - }, - ]; - - // player that made the move - let result = redactLog(logEvents, '0'); - expect(result).toMatchObject(logEvents); - - // other player - result = redactLog(logEvents, '1'); - expect(result).toMatchObject([ - { - _stateID: 0, - turn: 0, - phase: '', - action: { - type: 'MAKE_MOVE', - payload: { - credentials: undefined, - playerID: '0', - type: 'clickCell', - }, - }, - }, - ]); - }); - - test('not redacted move is shown to all', () => { - const logEvents = [ - { - _stateID: 0, - turn: 0, - phase: '', - action: ActionCreators.makeMove('unclickCell', [1, 2, 3], '0'), - }, - ]; - - // player that made the move - let result = redactLog(logEvents, '0'); - expect(result).toMatchObject(logEvents); - // other player - result = redactLog(logEvents, '1'); - expect(result).toMatchObject(logEvents); - }); - - test('can explicitly set showing args to true', () => { - const logEvents = [ - { - _stateID: 0, - turn: 0, - phase: '', - action: ActionCreators.makeMove('unclickCell', [1, 2, 3], '0'), - }, - ]; - - // player that made the move - let result = redactLog(logEvents, '0'); - expect(result).toMatchObject(logEvents); - // other player - result = redactLog(logEvents, '1'); - expect(result).toMatchObject(logEvents); - }); - - test('events are not redacted', () => { - const logEvents = [ - { - _stateID: 0, - turn: 0, - phase: '', - action: ActionCreators.gameEvent('endTurn'), - }, - ]; - - // player that made the move - let result = redactLog(logEvents, '0'); - expect(result).toMatchObject(logEvents); - // other player - result = redactLog(logEvents, '1'); - expect(result).toMatchObject(logEvents); - }); - - test('make sure sync redacts the log', async () => { - const game = { - moves: { - A: (G) => G, - B: { - move: (G) => G, - redact: true, - }, - }, - }; - - const send = jest.fn(); - const master = new Master(game, new InMemory(), TransportAPI(send)); - - const actionA = ActionCreators.makeMove('A', ['not redacted'], '0'); - const actionB = ActionCreators.makeMove('B', ['redacted'], '0'); - - // test: ping-pong two moves, then sync and check the log - await master.onSync('matchID', '0', undefined, 2); - await master.onUpdate(actionA, 0, 'matchID', '0'); - await master.onUpdate(actionB, 1, 'matchID', '0'); - await master.onSync('matchID', '1', undefined, 2); - - const { log } = send.mock.calls[send.mock.calls.length - 1][0].args[1]; - expect(log).toMatchObject([ - { - action: { - type: 'MAKE_MOVE', - payload: { - type: 'A', - args: ['not redacted'], - playerID: '0', - }, - }, - _stateID: 0, - }, - { - action: { - type: 'MAKE_MOVE', - payload: { - type: 'B', - args: null, - playerID: '0', - }, - }, - _stateID: 1, - }, - ]); - }); -}); - describe('chat', () => { - let sendAllReturn; const send = jest.fn(); - const sendAll = jest.fn((arg) => { - sendAllReturn = arg; - }); + const sendAll = jest.fn(); const db = new InMemory(); const master = new Master(game, db, TransportAPI(send, sendAll)); @@ -1279,7 +1010,7 @@ describe('chat', () => { { id: 'uuid', sender: '0', payload: { message: 'foo' } }, undefined ); - expect(sendAllReturn('0')).toEqual({ + expect(sendAll.mock.calls[0][0]).toEqual({ type: 'chat', args: [ 'matchID', diff --git a/src/master/master.ts b/src/master/master.ts index ddb5960f2..93091f3c1 100644 --- a/src/master/master.ts +++ b/src/master/master.ts @@ -31,7 +31,7 @@ import { createMatch } from '../server/util'; import type { Auth } from '../server/auth'; import * as StorageAPI from '../server/db/base'; import type { Operation } from 'rfc6902'; -import { createPatch } from 'rfc6902'; +import { redactLog } from './filter-player-view'; /** * Filter match data to get a player metadata object with credentials stripped. @@ -42,42 +42,6 @@ const filterMatchData = (matchData: Server.MatchData): FilteredMetadata => return filteredData; }); -/** - * Redact the log. - * - * @param {Array} log - The game log (or deltalog). - * @param {String} playerID - The playerID that this log is - * to be sent to. - */ -export function redactLog(log: LogEntry[], playerID: PlayerID) { - if (log === undefined) { - return log; - } - - return log.map((logEvent) => { - // filter for all other players and spectators. - if (playerID !== null && +playerID === +logEvent.action.payload.playerID) { - return logEvent; - } - - if (logEvent.redact !== true) { - return logEvent; - } - - const payload = { - ...logEvent.action.payload, - args: null, - }; - const filteredEvent = { - ...logEvent, - action: { ...logEvent.action, payload }, - }; - - const { redact, ...remaining } = filteredEvent; - return remaining; - }); -} - /** * Remove player credentials from action payload */ @@ -114,9 +78,31 @@ export type TransportData = args: [string, ChatMessage]; }; +export type IntermediateTransportData = + | { + type: 'update'; + args: [string, State]; + } + | { + type: 'patch'; + args: [string, number, State, State]; + } + | { + type: 'sync'; + args: [string, SyncInfo]; + } + | { + type: 'matchData'; + args: [string, FilteredMetadata]; + } + | { + type: 'chat'; + args: [string, ChatMessage]; + }; + export interface TransportAPI { send: (playerData: { playerID: PlayerID } & TransportData) => void; - sendAll: (makePlayerData: (playerID: PlayerID) => TransportData) => void; + sendAll: (payload: IntermediateTransportData) => void; } /** @@ -277,40 +263,17 @@ export class Master { matchID, }); - this.transportAPI.sendAll((playerID: string) => { - const log = redactLog(state.deltalog, playerID); - const filteredState = { - ...state, - G: this.game.playerView(state.G, state.ctx, playerID), - plugins: PlayerView(state, { playerID, game: this.game }), - deltalog: undefined, - _undo: [], - _redo: [], - }; - - if (this.game.deltaState) { - const newStateID = state._stateID; - const prevFilteredState = { - ...prevState, - G: this.game.playerView(prevState.G, prevState.ctx, playerID), - plugins: PlayerView(prevState, { playerID, game: this.game }), - deltalog: undefined, - _undo: [], - _redo: [], - }; - const patch = createPatch(prevFilteredState, filteredState); - - return { - type: 'patch', - args: [matchID, stateID, newStateID, patch, log], - }; - } else { - return { - type: 'update', - args: [matchID, filteredState, log], - }; - } - }); + if (this.game.deltaState) { + this.transportAPI.sendAll({ + type: 'patch', + args: [matchID, stateID, prevState, state], + }); + } else { + this.transportAPI.sendAll({ + type: 'update', + args: [matchID, state], + }); + } const { deltalog, ...stateWithoutDeltalog } = state; @@ -482,10 +445,10 @@ export class Master { const filteredMetadata = filterMatchData(metadata); - this.transportAPI.sendAll(() => ({ + this.transportAPI.sendAll({ type: 'matchData', args: [matchID, filteredMetadata], - })); + }); if (StorageAPI.isSynchronous(this.storageAPI)) { this.storageAPI.setMetadata(key, metadata); @@ -518,9 +481,9 @@ export class Master { } } - this.transportAPI.sendAll(() => ({ + this.transportAPI.sendAll({ type: 'chat', args: [matchID, chatMessage], - })); + }); } } diff --git a/src/server/transport/socketio.test.ts b/src/server/transport/socketio.test.ts index 86ae136d1..c97415578 100644 --- a/src/server/transport/socketio.test.ts +++ b/src/server/transport/socketio.test.ts @@ -12,6 +12,7 @@ import { Auth } from '../auth'; import { ProcessGameConfig } from '../../core/game'; import type { Master } from '../../master/master'; import { error } from '../../core/logger'; +import { getFilterPlayerView } from '../../master/filter-player-view'; jest.mock('../../core/logger', () => ({ info: jest.fn(), @@ -165,7 +166,14 @@ describe('TransportAPI', () => { const transport = new SocketIOTestAdapter({ clientInfo, roomInfo }); transport.init(app, games); io = app.context.io; - api = TransportAPI('matchID', io.socket, clientInfo, roomInfo); + const filterPlayerView = getFilterPlayerView(games[0]); + api = TransportAPI( + 'matchID', + io.socket, + clientInfo, + roomInfo, + filterPlayerView + ); }); beforeEach(async () => { @@ -191,9 +199,9 @@ describe('TransportAPI', () => { }); test('sendAll - function', () => { - api.sendAll((playerID) => ({ type: 'A', args: [playerID] })); - expect(io.socket.emit).toHaveBeenCalledWith('A', '0'); - expect(io.socket.emit).toHaveBeenCalledWith('A', '1'); + api.sendAll({ type: 'A', args: [] }); + expect(io.socket.emit).toHaveBeenCalledTimes(2); + expect(io.socket.emit).toHaveBeenCalledWith('A'); }); }); diff --git a/src/server/transport/socketio.ts b/src/server/transport/socketio.ts index 26e973765..5f3c71e58 100644 --- a/src/server/transport/socketio.ts +++ b/src/server/transport/socketio.ts @@ -15,6 +15,7 @@ import type { TransportAPI as MasterTransport, TransportData, } from '../../master/master'; +import { getFilterPlayerView } from '../../master/filter-player-view'; import type { Game, Server } from '../../types'; const PING_TIMEOUT = 20 * 1e3; @@ -29,6 +30,7 @@ export function TransportAPI( socket: IOTypes.Socket, clientInfo: SocketIO['clientInfo'], roomInfo: SocketIO['roomInfo'], + filterPlayerView: (string, IntermediateTransportData) => TransportData, provisionalClient?: Client ): MasterTransport { /** @@ -70,9 +72,9 @@ export function TransportAPI( /** * Send a message to all clients. */ - const sendAll: MasterTransport['sendAll'] = (makePlayerData) => { + const sendAll: MasterTransport['sendAll'] = (payload) => { forEachClient((client) => { - const data = makePlayerData(client.playerID); + const data = filterPlayerView(client.playerID, payload); emit(client.socket.id, data); }); }; @@ -174,6 +176,7 @@ export class SocketIO { for (const game of games) { const nsp = app._io.of(game.name); + const filterPlayerView = getFilterPlayerView(game); nsp.on('connection', (socket: IOTypes.Socket) => { socket.on('update', async (...args: Parameters) => { @@ -181,7 +184,13 @@ export class SocketIO { const master = new Master( game, app.context.db, - TransportAPI(matchID, socket, this.clientInfo, this.roomInfo), + TransportAPI( + matchID, + socket, + this.clientInfo, + this.roomInfo, + filterPlayerView + ), app.context.auth ); @@ -202,6 +211,7 @@ export class SocketIO { socket, this.clientInfo, this.roomInfo, + filterPlayerView, client ); const master = new Master( @@ -227,7 +237,13 @@ export class SocketIO { const master = new Master( game, app.context.db, - TransportAPI(matchID, socket, this.clientInfo, this.roomInfo), + TransportAPI( + matchID, + socket, + this.clientInfo, + this.roomInfo, + filterPlayerView + ), app.context.auth ); await master.onConnectionChange( @@ -246,7 +262,13 @@ export class SocketIO { const master = new Master( game, app.context.db, - TransportAPI(matchID, socket, this.clientInfo, this.roomInfo), + TransportAPI( + matchID, + socket, + this.clientInfo, + this.roomInfo, + filterPlayerView + ), app.context.auth ); master.onChatMessage(...args); From dac8dc1466066644c2a6e289151e92dbb49e4de6 Mon Sep 17 00:00:00 2001 From: "vdf.dev" Date: Sun, 25 Jul 2021 18:43:28 -0700 Subject: [PATCH 2/2] Increase coverage --- src/master/filter-player-view.test.ts | 97 ++++++++++++++++++++++++++- 1 file changed, 96 insertions(+), 1 deletion(-) diff --git a/src/master/filter-player-view.test.ts b/src/master/filter-player-view.test.ts index 6adb390b9..b0f7a0d9d 100644 --- a/src/master/filter-player-view.test.ts +++ b/src/master/filter-player-view.test.ts @@ -2,12 +2,21 @@ import { getFilterPlayerView, redactLog } from './filter-player-view'; import * as ActionCreators from '../core/action-creators'; import { Master } from './master'; import { InMemory } from '../server/db/inmemory'; +import { PlayerView } from '../core/player-view'; +import { INVALID_MOVE } from '../core/constants'; +import type { Ctx } from '../types'; function TransportAPI(send = jest.fn(), sendAll = jest.fn()) { return { send, sendAll }; } -describe('playerView', () => { +function validateNotTransientState(state: any) { + expect(state).toEqual( + expect.not.objectContaining({ transients: expect.anything() }) + ); +} + +describe('playerView - update', () => { const send = jest.fn(); const sendAll = jest.fn(); const game = { @@ -49,6 +58,92 @@ describe('playerView', () => { }); }); +describe('playerView - patch', () => { + const send = jest.fn(); + const sendAll = jest.fn(); + const db = new InMemory(); + const game = { + seed: 0, + deltaState: true, + setup: () => { + return { + players: { + '0': { + cards: ['card3'], + }, + '1': { + cards: [], + }, + }, + cards: ['card0', 'card1', 'card2'], + discardedCards: [], + }; + }, + playerView: PlayerView.STRIP_SECRETS, + turn: { + activePlayers: { currentPlayer: { stage: 'A' } }, + stages: { + A: { + moves: { + Invalid: () => { + return INVALID_MOVE; + }, + A: { + client: false, + move: (G, ctx: Ctx) => { + const card = G.players[ctx.playerID].cards.shift(); + G.discardedCards.push(card); + }, + }, + B: { + client: false, + ignoreStaleStateID: true, + move: (G, ctx: Ctx) => { + const card = G.cards.pop(); + G.players[ctx.playerID].cards.push(card); + }, + }, + }, + }, + }, + }, + }; + const master = new Master(game, db, TransportAPI(send, sendAll)); + const move = ActionCreators.makeMove('A', null, '0'); + + beforeAll(async () => { + master.subscribe(({ state }) => { + validateNotTransientState(state); + }); + await master.onSync('matchID', '0', undefined, 2); + }); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + test('patch', async () => { + await master.onUpdate(move, 0, 'matchID', '0'); + expect(sendAll).toBeCalled(); + + const payload = sendAll.mock.calls[sendAll.mock.calls.length - 1][0]; + expect(payload.type).toBe('patch'); + + const filterPlayerView = getFilterPlayerView(game); + const value = filterPlayerView('0', payload); + expect(value.type).toBe('patch'); + expect(value.args[0]).toBe('matchID'); + expect(value.args[1]).toBe(0); + expect(value.args[2]).toBe(1); + expect(value.args[3]).toMatchObject([ + { op: 'remove', path: '/G/players/0/cards/0' }, + { op: 'add', path: '/G/discardedCards/-', value: 'card3' }, + { op: 'replace', path: '/ctx/numMoves', value: 1 }, + { op: 'replace', path: '/_stateID', value: 1 }, + ]); + }); +}); + describe('redactLog', () => { test('no-op with undefined log', () => { const result = redactLog(undefined, '0');