fix: make game updates atomic (#774)

* fix: handle game updates atomically
* cleanup game after force end

src/games/services/game-event-handler.service.ts

@@ -30,16 +30,21 @@ export class GameEventHandlerService {
   }
 
   async onMatchEnded(gameId: string) {
-    const game = await this.gameModel.findOneAndUpdate({ _id: gameId, state: 'started' }, { state: 'ended' }, { new: true });
-    if (game) {
-      game.slots.forEach(slot => {
-        if (slot.status === 'waiting for substitute') {
-          slot.status = 'active';
-        }
-      });
-
-      await game.save();
+    const game = await this.gameModel.findOneAndUpdate(
+      { _id: gameId, state: 'started' },
+      {
+        state: 'ended',
+        'slots.$[element].status': 'active',
+      },
+      {
+        new: true, // return updated document
+        arrayFilters: [
+          { 'element.status': { $eq: 'waiting for substitute' } },
+        ],
+      }
+    );
 
+    if (game) {
       this.events.gameChanges.next({ game: game.toJSON() });
       this.events.substituteRequestsChange.next();
       setTimeout(() => this.gameRuntimeService.cleanupServer(game.gameServer.toString()), serverCleanupDelay);
@@ -103,16 +108,21 @@ export class GameEventHandlerService {
       return;
     }
 
-    const game = await this.gameModel.findById(gameId);
-    if (game) {
-      const slot = game.findPlayerSlot(player.id);
-      if (slot) {
-        slot.connectionStatus = connectionStatus;
-        await game.save();
-        this.events.gameChanges.next({ game: game.toJSON() });
-      } else {
-        this.logger.warn(`player ${player.name} does not belong in this game`);
+    const game = await this.gameModel.findByIdAndUpdate(
+      gameId,
+      {
+        'slots.$[element].connectionStatus': connectionStatus,
+      },
+      {
+        new: true, // return updated document
+        arrayFilters: [
+          { 'element.player': { $eq: player.id } },
+        ],
       }
+    );
+
+    if (game) {
+      this.events.gameChanges.next({ game: game.toJSON() });
     } else {
       this.logger.warn(`no such game: ${gameId}`);
     }

src/games/services/game-runtime.service.spec.ts

@@ -35,12 +35,13 @@ describe('GameRuntimeService', () => {
   let mongod: MongoMemoryServer;
   let playersService: PlayersService;
   let gamesService: GamesService;
-  let gameServersService: GameServersService;
+  let gameServersService: jest.Mocked<GameServersService>;
   let serverConfiguratorService: ServerConfiguratorService;
   let rconFactoryService: RconFactoryService;
   let mockGameServer: GameServer & { id: string };
   let mockPlayers: DocumentType<Player>[];
   let mockGame: DocumentType<Game>;
+  let events: Events;
 
   beforeAll(() => mongod = new MongoMemoryServer());
   afterAll(async () => await mongod.stop());
@@ -68,6 +69,7 @@ describe('GameRuntimeService', () => {
     gameServersService = module.get(GameServersService);
     serverConfiguratorService = module.get(ServerConfiguratorService);
     rconFactoryService = module.get(RconFactoryService);
+    events = module.get(Events);
   });
 
   beforeEach(async () => {
@@ -79,8 +81,7 @@ describe('GameRuntimeService', () => {
       rconPassword: 'FAKE_RCON_PASSWORD',
     };
 
-    // @ts-expect-error
-    gameServersService.getById = () => Promise.resolve(mockGameServer);
+    gameServersService.getById.mockResolvedValue(mockGameServer as any);
 
     // @ts-expect-error
     mockPlayers = await Promise.all([ playersService._createOne(), playersService._createOne() ]);
@@ -161,6 +162,21 @@ describe('GameRuntimeService', () => {
       expect(releaseSpy).toHaveBeenCalledWith(mockGameServer.id);
     });
 
+    it('should emit the gameChanges event', async () => new Promise<void>(resolve => {
+      events.gameChanges.subscribe(({ game }) => {
+        expect(game.id).toEqual(mockGame.id);
+        resolve();
+      });
+
+      service.forceEnd(mockGame.id);
+    }));
+
+    // eslint-disable-next-line jest/expect-expect
+    it('should emit the substituteRequestsChange event', async () => new Promise<void>(resolve => {
+      events.substituteRequestsChange.subscribe(resolve);
+      service.forceEnd(mockGame.id);
+    }));
+
     describe('when the given game does not exist', () => {
       it('should reject', async () => {
         await expect(service.forceEnd(new ObjectId().toString())).rejects.toThrowError('no such game');
@@ -176,6 +192,18 @@ describe('GameRuntimeService', () => {
         await expect(service.forceEnd(mockGame.id)).resolves.toBeTruthy();
       });
     });
+
+    describe('if one of the players is waiting to be substituted', () => {
+      beforeEach(async () => {
+        mockGame.slots[0].status = 'waiting for substitute';
+        await mockGame.save();
+      });
+
+      it('should set his status back to active', async () => {
+        const ret = await service.forceEnd(mockGame.id);
+        expect(ret.slots[0].status).toEqual('active');
+      });
+    });
   });
 
   describe('#replacePlayer()', () => {
@@ -251,7 +279,7 @@ describe('GameRuntimeService', () => {
 
     describe('when the given game server does not exist', () => {
       beforeEach(() => {
-        gameServersService.getById = () => Promise.resolve(null);
+        gameServersService.getById.mockResolvedValue(null);
       });
 
       it('should throw an error', async () => {

src/games/services/game-runtime.service.ts

@@ -63,8 +63,10 @@ export class GameRuntimeService {
 
     game.state = 'interrupted';
     game.error = 'ended by admin';
+    game.slots.filter(s => s.status === 'waiting for substitute').forEach(s => s.status = 'active');
     await game.save();
     this.events.gameChanges.next({ game });
+    this.events.substituteRequestsChange.next();
 
     if (game.gameServer) {
       await this.cleanupServer(game.gameServer.toString());