From 1285d74245125ce5e1effabebf15cfe9f8962004 Mon Sep 17 00:00:00 2001 From: Karen Yao Date: Tue, 18 Aug 2026 18:00:51 -0700 Subject: [PATCH 1/6] feat: reregister users when server restarts and also clears cookie if connecting fails 3 times --- frontend/src/app/core/credentials.service.ts | 9 +++ .../src/app/core/player-name.service.spec.ts | 79 +++++++++++++++++++ frontend/src/app/core/player-name.service.ts | 26 ++++++ .../app/core/sockets/game-socket.service.ts | 29 ++++++- .../game-page/game-page.component.spec.ts | 8 +- .../pages/game-page/game-page.component.ts | 47 +++++++++++ .../register-page/register-page.component.ts | 3 + 7 files changed, 197 insertions(+), 4 deletions(-) create mode 100644 frontend/src/app/core/player-name.service.spec.ts create mode 100644 frontend/src/app/core/player-name.service.ts diff --git a/frontend/src/app/core/credentials.service.ts b/frontend/src/app/core/credentials.service.ts index 1c0b881..8c40233 100644 --- a/frontend/src/app/core/credentials.service.ts +++ b/frontend/src/app/core/credentials.service.ts @@ -47,4 +47,13 @@ export class CredentialsService { const attributes = `; Path=/; SameSite=Lax${secure}`; this.document.cookie = `id=${encodeURIComponent(credentials.id)}${attributes}`; } + + clear(): void { + if (!this.browserWindow) { + return; + } + + const secure = this.browserWindow.location.protocol === 'https:' ? '; Secure' : ''; + this.document.cookie = `id=; Path=/; SameSite=Lax${secure}; Max-Age=0`; + } } diff --git a/frontend/src/app/core/player-name.service.spec.ts b/frontend/src/app/core/player-name.service.spec.ts new file mode 100644 index 0000000..6c70f4e --- /dev/null +++ b/frontend/src/app/core/player-name.service.spec.ts @@ -0,0 +1,79 @@ +import { TestBed } from '@angular/core/testing'; + +import { PAC_WINDOW } from './browser-window.token'; +import { PlayerNameService } from './player-name.service'; + +describe('PlayerNameService', () => { + let service: PlayerNameService; + let mockStorage: Record; + + beforeEach(() => { + mockStorage = {}; + const mockWindow = { + localStorage: { + getItem: (key: string) => mockStorage[key] ?? null, + setItem: (key: string, value: string) => { + mockStorage[key] = value; + }, + }, + }; + + TestBed.configureTestingModule({ + providers: [PlayerNameService, { provide: PAC_WINDOW, useValue: mockWindow }], + }); + service = TestBed.inject(PlayerNameService); + }); + + it('returns an empty string when no name has been saved', () => { + expect(service.get()).toBe(''); + }); + + it('saves and retrieves a player name', () => { + service.save('Odin'); + expect(service.get()).toBe('Odin'); + }); + + it('returns an empty string when localStorage throws on get', () => { + TestBed.resetTestingModule(); + const throwingWindow = { + localStorage: { + getItem: () => { + throw new Error('unavailable'); + }, + setItem: () => {}, + }, + }; + TestBed.configureTestingModule({ + providers: [PlayerNameService, { provide: PAC_WINDOW, useValue: throwingWindow }], + }); + service = TestBed.inject(PlayerNameService); + expect(service.get()).toBe(''); + }); + + it('silently ignores localStorage errors on save', () => { + TestBed.resetTestingModule(); + const throwingWindow = { + localStorage: { + getItem: () => null, + setItem: () => { + throw new Error('quota exceeded'); + }, + }, + }; + TestBed.configureTestingModule({ + providers: [PlayerNameService, { provide: PAC_WINDOW, useValue: throwingWindow }], + }); + service = TestBed.inject(PlayerNameService); + expect(() => service.save('Odin')).not.toThrow(); + }); + + it('returns an empty string when PAC_WINDOW is null', () => { + TestBed.resetTestingModule(); + TestBed.configureTestingModule({ + providers: [PlayerNameService, { provide: PAC_WINDOW, useValue: null }], + }); + service = TestBed.inject(PlayerNameService); + expect(service.get()).toBe(''); + expect(() => service.save('Odin')).not.toThrow(); + }); +}); diff --git a/frontend/src/app/core/player-name.service.ts b/frontend/src/app/core/player-name.service.ts new file mode 100644 index 0000000..4bde22a --- /dev/null +++ b/frontend/src/app/core/player-name.service.ts @@ -0,0 +1,26 @@ +import { inject, Service, signal } from '@angular/core'; +import { PAC_WINDOW } from './browser-window.token'; + +const PLAYER_NAME_KEY = 'playerName'; + +@Service() +export class PlayerNameService { + private readonly browserWindow = inject(PAC_WINDOW); + private readonly statusMessage = signal(null); + + get(): string { + try { + return this.browserWindow?.localStorage.getItem(PLAYER_NAME_KEY) ?? ''; + } catch { + return ''; + } + } + + save(name: string): void { + try { + this.browserWindow?.localStorage.setItem(PLAYER_NAME_KEY, name); + } catch { + this.statusMessage.set('Could not save your name for auto-re-registration.'); + } + } +} diff --git a/frontend/src/app/core/sockets/game-socket.service.ts b/frontend/src/app/core/sockets/game-socket.service.ts index 15a74f8..b969854 100644 --- a/frontend/src/app/core/sockets/game-socket.service.ts +++ b/frontend/src/app/core/sockets/game-socket.service.ts @@ -32,15 +32,19 @@ export class GameSocketService extends WebSocketService { private reconnecting = false; private suspendedReason = 'Paused while the browser is offline.'; private readonly statusMessage = signal(null); - + private consecutiveFailures = 0; readonly players = signal>({}); readonly isFlagFound = signal(false); + readonly sessionExpired = signal(false); + readonly MAX_FAILED_ATTEMPTS = 3; start(id: string, onConnected: () => void): void { this.stop(); this.mode = 'player'; this.playerId = id; this.onConnected = onConnected; + this.consecutiveFailures = 0; + this.sessionExpired.set(false); this.resume(); } @@ -74,6 +78,8 @@ export class GameSocketService extends WebSocketService { this.onConnected = null; this.reconnecting = false; this.statusMessage.set(null); + this.consecutiveFailures = 0; + this.sessionExpired.set(false); this.disconnect(); } @@ -94,8 +100,9 @@ export class GameSocketService extends WebSocketService { // Every connection receives a fresh list of active players. Clearing the // cache removes disconnects that may have been missed while unavailable. this.players.set({}); - this.reconnecting = false; this.statusMessage.set(null); + this.reconnecting = false; + this.consecutiveFailures = 0; this.onConnected?.(); } @@ -103,6 +110,24 @@ export class GameSocketService extends WebSocketService { this.reconnecting = true; } + protected override shouldReconnect(closeEvent: CloseEvent): boolean { + if (this.mode !== 'player') { + return true; + } + + this.consecutiveFailures++; + + if (this.consecutiveFailures >= this.MAX_FAILED_ATTEMPTS) { + this.sessionExpired.set(true); + this.statusMessage.set( + 'Session has expired as game server restarted.', + ); + return false; + } + + return true; + } + protected override onSocketError(): void { this.statusMessage.set( this.mode === 'viewer' diff --git a/frontend/src/app/pages/game-page/game-page.component.spec.ts b/frontend/src/app/pages/game-page/game-page.component.spec.ts index 5d040de..5f7a145 100644 --- a/frontend/src/app/pages/game-page/game-page.component.spec.ts +++ b/frontend/src/app/pages/game-page/game-page.component.spec.ts @@ -10,6 +10,7 @@ import { GeolocationService } from '../../core/geolocation.service'; import { MapInfo, PlayerStatus, PlayerType } from '../../core/game.models'; import { WakeLockService } from '../../core/wake-lock.service'; import { GamePageComponent } from './game-page.component'; +import { PlayerNameService } from '../../core/player-name.service'; describe('GamePageComponent leader link', () => { let fixture: ComponentFixture; @@ -34,6 +35,7 @@ describe('GamePageComponent leader link', () => { }), status: signal('Connected.'), isFlagFound: signal(false), + sessionExpired: signal(false), start: vi.fn(), stop: vi.fn(), resume: vi.fn(), @@ -69,7 +71,8 @@ describe('GamePageComponent leader link', () => { imports: [GamePageComponent], providers: [ { provide: ApiService, useValue: { getMap: vi.fn(() => of(map)) } }, - { provide: CredentialsService, useValue: { get: () => ({ id: 'SELF' }) } }, + { provide: CredentialsService, useValue: { get: () => ({ id: 'SELF' }), save: vi.fn(), clear: vi.fn() } }, + { provide: PlayerNameService, useValue: { get: vi.fn(() => ''), save: vi.fn() } }, { provide: Router, useValue: { navigateByUrl: vi.fn() } }, ], }) @@ -110,6 +113,7 @@ describe('GamePageComponent leader link', () => { it('does not show the leader link to a non-leader', async () => { const page = await render(PlayerType.Ghost); - expect(page.querySelector('.game-page__leader-link')).toBeNull(); + const link = page.querySelector('.game-page__leader-link a'); + expect(link?.style.visibility).toBe('hidden'); }); }); diff --git a/frontend/src/app/pages/game-page/game-page.component.ts b/frontend/src/app/pages/game-page/game-page.component.ts index b435cb3..918184b 100644 --- a/frontend/src/app/pages/game-page/game-page.component.ts +++ b/frontend/src/app/pages/game-page/game-page.component.ts @@ -4,6 +4,7 @@ import { Component, computed, DestroyRef, + effect, inject, signal, } from '@angular/core'; @@ -19,6 +20,7 @@ import { isLeaderType, MapInfo, typeLabel } from '../../core/game.models'; import { WakeLockService } from '../../core/wake-lock.service'; import { GameCanvasComponent } from '../../game/game-canvas/game-canvas.component'; import { BrandHeaderComponent } from '../../shared/brand-header/brand-header.component'; +import { PlayerNameService } from '../../core/player-name.service'; @Component({ selector: 'pac-game-page', @@ -34,6 +36,7 @@ export class GamePageComponent { private readonly credentials = inject(CredentialsService); private readonly destroyRef = inject(DestroyRef); private readonly router = inject(Router); + private readonly playerName = inject(PlayerNameService); protected readonly socket = inject(GameSocketService); protected readonly geolocation = inject(GeolocationService); @@ -41,6 +44,8 @@ export class GamePageComponent { protected readonly map = signal(null); protected readonly selfId = signal(''); protected readonly pageStatus = signal('Loading the game map…'); + + private readonly reregistering = signal(false); protected readonly selfSummary = computed(() => { const player = this.socket.players()[this.selfId()]?.player; return player ? `${player.name} (${player.id}) is ${typeLabel(player.type)}` : ''; @@ -62,12 +67,54 @@ export class GamePageComponent { constructor() { afterNextRender(() => void this.initialize()); this.destroyRef.onDestroy(() => this.cleanup()); + + effect(() => { + if (this.socket.sessionExpired() && !this.reregistering()) { + void this.autoReregister(); + } + }); } protected async toggleWakeLock(event: Event): Promise { await this.wakeLock.setEnabled((event.target as HTMLInputElement).checked); } + private async autoReregister(): Promise { + const name = this.playerName.get(); + if (!name) { + this.credentials.clear(); + this.socket.stop(); + await this.router.navigateByUrl('/register'); + return; + } + + this.reregistering.set(true); + this.pageStatus.set('Re-registering…'); + + try { + const response = await firstValueFrom(this.api.registerPlayer(name)); + const id = response.id.trim(); + if (!id) { + throw new Error('The API returned an empty player ID.'); + } + + this.credentials.save({ id }); + this.selfId.set(id); + this.pageStatus.set('Re-registered. Reconnecting…'); + this.socket.start(id, () => { + this.pageStatus.set('Connected to PacMacro.'); + this.geolocation.start((coordinate) => this.socket.sendCoordinate(coordinate)); + }); + } catch { + this.credentials.clear(); + this.socket.stop(); + this.pageStatus.set('Could not re-register. Redirecting…'); + await this.router.navigateByUrl('/register'); + } finally { + this.reregistering.set(false); + } + } + private async initialize(): Promise { if (!this.browserWindow) { return; diff --git a/frontend/src/app/pages/register-page/register-page.component.ts b/frontend/src/app/pages/register-page/register-page.component.ts index c04ca41..6b251ca 100644 --- a/frontend/src/app/pages/register-page/register-page.component.ts +++ b/frontend/src/app/pages/register-page/register-page.component.ts @@ -12,6 +12,7 @@ import { firstValueFrom } from 'rxjs'; import { ApiService } from '../../core/api.service'; import { CredentialsService } from '../../core/credentials.service'; import { BrandHeaderComponent } from '../../shared/brand-header/brand-header.component'; +import { PlayerNameService } from '../../core/player-name.service'; interface RegistrationModel { name: string; @@ -28,6 +29,7 @@ export class RegisterPageComponent { private readonly api = inject(ApiService); private readonly credentials = inject(CredentialsService); private readonly router = inject(Router); + private readonly playerName = inject(PlayerNameService); protected readonly registrationModel = signal({ name: '', @@ -62,6 +64,7 @@ export class RegisterPageComponent { throw new Error('The API returned an empty player ID.'); } this.credentials.save({ id }); + this.playerName.save(trimmedName); await this.router.navigateByUrl('/'); } catch (error) { this.status.set('Registration failed. Check your details and the API connection.'); From 7a8c616331c2621e2b962a7ed42871cca6dc82d6 Mon Sep 17 00:00:00 2001 From: Karen Yao Date: Tue, 18 Aug 2026 18:04:44 -0700 Subject: [PATCH 2/6] style: code ordering --- frontend/src/app/core/sockets/game-socket.service.ts | 5 +++-- frontend/src/app/pages/game-page/game-page.component.ts | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/frontend/src/app/core/sockets/game-socket.service.ts b/frontend/src/app/core/sockets/game-socket.service.ts index b969854..cd80361 100644 --- a/frontend/src/app/core/sockets/game-socket.service.ts +++ b/frontend/src/app/core/sockets/game-socket.service.ts @@ -31,8 +31,9 @@ export class GameSocketService extends WebSocketService { private onConnected: (() => void) | null = null; private reconnecting = false; private suspendedReason = 'Paused while the browser is offline.'; - private readonly statusMessage = signal(null); private consecutiveFailures = 0; + private readonly statusMessage = signal(null); + readonly players = signal>({}); readonly isFlagFound = signal(false); readonly sessionExpired = signal(false); @@ -100,8 +101,8 @@ export class GameSocketService extends WebSocketService { // Every connection receives a fresh list of active players. Clearing the // cache removes disconnects that may have been missed while unavailable. this.players.set({}); - this.statusMessage.set(null); this.reconnecting = false; + this.statusMessage.set(null); this.consecutiveFailures = 0; this.onConnected?.(); } diff --git a/frontend/src/app/pages/game-page/game-page.component.ts b/frontend/src/app/pages/game-page/game-page.component.ts index 918184b..7f0b548 100644 --- a/frontend/src/app/pages/game-page/game-page.component.ts +++ b/frontend/src/app/pages/game-page/game-page.component.ts @@ -37,6 +37,7 @@ export class GamePageComponent { private readonly destroyRef = inject(DestroyRef); private readonly router = inject(Router); private readonly playerName = inject(PlayerNameService); + private readonly reregistering = signal(false); protected readonly socket = inject(GameSocketService); protected readonly geolocation = inject(GeolocationService); @@ -45,7 +46,6 @@ export class GamePageComponent { protected readonly selfId = signal(''); protected readonly pageStatus = signal('Loading the game map…'); - private readonly reregistering = signal(false); protected readonly selfSummary = computed(() => { const player = this.socket.players()[this.selfId()]?.player; return player ? `${player.name} (${player.id}) is ${typeLabel(player.type)}` : ''; From fe5a0368dab387a342986811370d7db1b1d63007 Mon Sep 17 00:00:00 2001 From: Karen Yao Date: Wed, 19 Aug 2026 01:33:41 -0700 Subject: [PATCH 3/6] test: add unit tests for re-registration after server restart --- .../src/app/core/credentials.service.spec.ts | 53 +++- .../core/sockets/game-socket.service.spec.ts | 96 ++++++++ .../game-page/game-page.component.spec.ts | 232 +++++++++++++----- .../register-page.component.spec.ts | 8 +- 4 files changed, 324 insertions(+), 65 deletions(-) diff --git a/frontend/src/app/core/credentials.service.spec.ts b/frontend/src/app/core/credentials.service.spec.ts index 3b3ddae..90edebb 100644 --- a/frontend/src/app/core/credentials.service.spec.ts +++ b/frontend/src/app/core/credentials.service.spec.ts @@ -1,4 +1,8 @@ -import { readCookie } from './credentials.service'; +import { DOCUMENT } from '@angular/common'; +import { TestBed } from '@angular/core/testing'; + +import { PAC_WINDOW } from './browser-window.token'; +import { CredentialsService, readCookie } from './credentials.service'; describe('readCookie', () => { it('reads and decodes an exact cookie name', () => { @@ -10,3 +14,50 @@ describe('readCookie', () => { expect(readCookie('theme=dark', 'id')).toBe(''); }); }); + +describe('CredentialsService', () => { + let service: CredentialsService; + let mockDocument: { cookie: string }; + let mockWindow: { location: { protocol: string } }; + + beforeEach(() => { + mockDocument = { cookie: '' }; + mockWindow = { location: { protocol: 'http:' } }; + + TestBed.configureTestingModule({ + providers: [ + CredentialsService, + { provide: DOCUMENT, useValue: mockDocument }, + { provide: PAC_WINDOW, useValue: mockWindow }, + ], + }); + service = TestBed.inject(CredentialsService); + }); + + it.each(['http:', 'https:'] as const)('expires the id cookie over %s', (protocol) => { + mockWindow.location.protocol = protocol; + mockDocument.cookie = 'id=ABC; theme=dark'; + + service.clear(); + + const secure = protocol === 'https:' ? '; Secure' : ''; + expect(mockDocument.cookie).toBe(`id=; Path=/; SameSite=Lax${secure}; Max-Age=0`); + }); + + it('leaves the cookie untouched when PAC_WINDOW is null', () => { + TestBed.resetTestingModule(); + TestBed.configureTestingModule({ + providers: [ + CredentialsService, + { provide: DOCUMENT, useValue: mockDocument }, + { provide: PAC_WINDOW, useValue: null }, + ], + }); + service = TestBed.inject(CredentialsService); + mockDocument.cookie = 'id=ABC'; + + service.clear(); + + expect(mockDocument.cookie).toBe('id=ABC'); + }); +}); diff --git a/frontend/src/app/core/sockets/game-socket.service.spec.ts b/frontend/src/app/core/sockets/game-socket.service.spec.ts index e0b6040..d8f3b20 100644 --- a/frontend/src/app/core/sockets/game-socket.service.spec.ts +++ b/frontend/src/app/core/sockets/game-socket.service.spec.ts @@ -297,4 +297,100 @@ describe('GameSocketService', () => { expect(service.state()).toBe('error'); expect(service.status()).toContain('Register as admin again in this browser'); }); + + it('expires the session after three consecutive failed player connections', () => { + vi.useFakeTimers(); + vi.spyOn(console, 'log').mockImplementation(() => undefined); + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + service.start('ABCD', () => undefined); + + MockGameWebSocket.instances[0].serverClose(false); + vi.advanceTimersByTime(1000); + MockGameWebSocket.instances[1].serverClose(false); + vi.advanceTimersByTime(2000); + MockGameWebSocket.instances[2].serverClose(false); + vi.runAllTimers(); + + expect(service.sessionExpired()).toBe(true); + expect(service.state()).toBe('error'); + expect(service.status()).toContain('Session has expired as game server restarted.'); + expect(MockGameWebSocket.instances).toHaveLength(3); + }); + + it('keeps reconnecting a player socket after fewer than three failures', () => { + vi.useFakeTimers(); + vi.spyOn(console, 'log').mockImplementation(() => undefined); + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + service.start('ABCD', () => undefined); + + MockGameWebSocket.instances[0].serverClose(false); + vi.advanceTimersByTime(1000); + MockGameWebSocket.instances[1].serverClose(false); + vi.advanceTimersByTime(2000); + + expect(service.sessionExpired()).toBe(false); + expect(service.state()).toBe('connecting'); + expect(MockGameWebSocket.instances).toHaveLength(3); + }); + + it('resets the failure counter when a player connection succeeds', () => { + vi.useFakeTimers(); + vi.spyOn(console, 'log').mockImplementation(() => undefined); + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + service.start('ABCD', () => undefined); + + MockGameWebSocket.instances[0].serverClose(false); + vi.advanceTimersByTime(1000); + MockGameWebSocket.instances[1].serverClose(false); + vi.advanceTimersByTime(2000); + + MockGameWebSocket.instances[2].open(); + MockGameWebSocket.instances[2].serverClose(false); + vi.advanceTimersByTime(4000); + MockGameWebSocket.instances[3].serverClose(false); + vi.runAllTimers(); + + expect(service.sessionExpired()).toBe(false); + expect(MockGameWebSocket.instances).toHaveLength(5); + }); + + it('never expires the session for a viewer socket', () => { + vi.useFakeTimers(); + vi.spyOn(console, 'log').mockImplementation(() => undefined); + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + service.startViewer(); + + MockGameWebSocket.instances[0].serverClose(false); + vi.advanceTimersByTime(1000); + MockGameWebSocket.instances[1].serverClose(false); + vi.advanceTimersByTime(2000); + MockGameWebSocket.instances[2].serverClose(false); + vi.runAllTimers(); + + expect(service.sessionExpired()).toBe(false); + expect(MockGameWebSocket.instances.length).toBeGreaterThan(3); + }); + + it('start and stop clear an expired session', () => { + vi.useFakeTimers(); + vi.spyOn(console, 'log').mockImplementation(() => undefined); + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + service.start('ABCD', () => undefined); + + MockGameWebSocket.instances[0].serverClose(false); + vi.advanceTimersByTime(1000); + MockGameWebSocket.instances[1].serverClose(false); + vi.advanceTimersByTime(2000); + MockGameWebSocket.instances[2].serverClose(false); + vi.runAllTimers(); + + expect(service.sessionExpired()).toBe(true); + + service.stop(); + expect(service.sessionExpired()).toBe(false); + + service.start('ABCD', () => undefined); + expect(service.sessionExpired()).toBe(false); + expect(MockGameWebSocket.instances).toHaveLength(4); + }); }); diff --git a/frontend/src/app/pages/game-page/game-page.component.spec.ts b/frontend/src/app/pages/game-page/game-page.component.spec.ts index 5f7a145..3d69963 100644 --- a/frontend/src/app/pages/game-page/game-page.component.spec.ts +++ b/frontend/src/app/pages/game-page/game-page.component.spec.ts @@ -1,7 +1,7 @@ import { signal } from '@angular/core'; import { ComponentFixture, TestBed } from '@angular/core/testing'; import { Router } from '@angular/router'; -import { of } from 'rxjs'; +import { of, throwError } from 'rxjs'; import { ApiService } from '../../core/api.service'; import { CredentialsService } from '../../core/credentials.service'; @@ -12,51 +12,88 @@ import { WakeLockService } from '../../core/wake-lock.service'; import { GamePageComponent } from './game-page.component'; import { PlayerNameService } from '../../core/player-name.service'; +const map: MapInfo = { + min: { latitude: 49.27, longitude: -122.92 }, + max: { latitude: 49.28, longitude: -122.9 }, + width: 32, + height: 32, + isFlagFound: false, +}; +const api = { + getMap: vi.fn(() => of(map)), + registerPlayer: vi.fn(() => of({ id: 'NEWID' })), +}; +const credentials = { + get: vi.fn(() => ({ id: 'SELF' })), + save: vi.fn(), + clear: vi.fn(), +}; +const playerName = { + get: vi.fn(() => ''), + save: vi.fn(), +}; +const router = { navigateByUrl: vi.fn() }; +const gameSocket = { + players: signal({ + SELF: { + coordinate: { latitude: 49.275, longitude: -122.91 }, + player: { + id: 'SELF', + name: 'Leader', + type: PlayerType.Ghost, + status: PlayerStatus.Connected, + }, + }, + }), + status: signal('Connected.'), + isFlagFound: signal(false), + sessionExpired: signal(false), + start: vi.fn((_id: string, _onConnected: () => void) => gameSocket.sessionExpired.set(false)), + stop: vi.fn(() => gameSocket.sessionExpired.set(false)), + resume: vi.fn(), + suspend: vi.fn(), + sendCoordinate: vi.fn(), + setInitialState: vi.fn(), +}; +const geolocation = { + status: signal('Ready.'), + start: vi.fn(), + stop: vi.fn(), +}; +const wakeLock = { + supported: signal(true), + enabled: signal(false), + status: signal('Screen wake lock is off.'), + initialize: vi.fn(), + setEnabled: vi.fn(async () => undefined), + handleVisibilityChange: vi.fn(async () => undefined), + release: vi.fn(async () => undefined), +}; + +async function configureTestBed(): Promise { + await TestBed.configureTestingModule({ + imports: [GamePageComponent], + providers: [ + { provide: ApiService, useValue: api }, + { provide: CredentialsService, useValue: credentials }, + { provide: PlayerNameService, useValue: playerName }, + { provide: Router, useValue: router }, + ], + }) + .overrideComponent(GamePageComponent, { + set: { + providers: [ + { provide: GameSocketService, useValue: gameSocket }, + { provide: GeolocationService, useValue: geolocation }, + { provide: WakeLockService, useValue: wakeLock }, + ], + }, + }) + .compileComponents(); +} + describe('GamePageComponent leader link', () => { let fixture: ComponentFixture; - const map: MapInfo = { - min: { latitude: 49.27, longitude: -122.92 }, - max: { latitude: 49.28, longitude: -122.9 }, - width: 32, - height: 32, - isFlagFound: false, - }; - const gameSocket = { - players: signal({ - SELF: { - coordinate: { latitude: 49.275, longitude: -122.91 }, - player: { - id: 'SELF', - name: 'Leader', - type: PlayerType.Ghost, - status: PlayerStatus.Connected, - }, - }, - }), - status: signal('Connected.'), - isFlagFound: signal(false), - sessionExpired: signal(false), - start: vi.fn(), - stop: vi.fn(), - resume: vi.fn(), - suspend: vi.fn(), - sendCoordinate: vi.fn(), - setInitialState: vi.fn(), - }; - const geolocation = { - status: signal('Ready.'), - start: vi.fn(), - stop: vi.fn(), - }; - const wakeLock = { - supported: signal(true), - enabled: signal(false), - status: signal('Screen wake lock is off.'), - initialize: vi.fn(), - setEnabled: vi.fn(async () => undefined), - handleVisibilityChange: vi.fn(async () => undefined), - release: vi.fn(async () => undefined), - }; beforeEach(async () => { gameSocket.players.update((players) => ({ @@ -67,25 +104,7 @@ describe('GamePageComponent leader link', () => { }, })); - await TestBed.configureTestingModule({ - imports: [GamePageComponent], - providers: [ - { provide: ApiService, useValue: { getMap: vi.fn(() => of(map)) } }, - { provide: CredentialsService, useValue: { get: () => ({ id: 'SELF' }), save: vi.fn(), clear: vi.fn() } }, - { provide: PlayerNameService, useValue: { get: vi.fn(() => ''), save: vi.fn() } }, - { provide: Router, useValue: { navigateByUrl: vi.fn() } }, - ], - }) - .overrideComponent(GamePageComponent, { - set: { - providers: [ - { provide: GameSocketService, useValue: gameSocket }, - { provide: GeolocationService, useValue: geolocation }, - { provide: WakeLockService, useValue: wakeLock }, - ], - }, - }) - .compileComponents(); + await configureTestBed(); }); async function render(playerType: PlayerType): Promise { @@ -117,3 +136,90 @@ describe('GamePageComponent leader link', () => { expect(link?.style.visibility).toBe('hidden'); }); }); + +describe('GamePageComponent re-registration', () => { + let fixture: ComponentFixture; + + beforeEach(async () => { + vi.clearAllMocks(); + gameSocket.sessionExpired.set(false); + await configureTestBed(); + }); + + async function render(): Promise { + fixture = TestBed.createComponent(GamePageComponent); + fixture.detectChanges(); + await new Promise((resolve) => setTimeout(resolve, 0)); + fixture.detectChanges(); + return fixture.nativeElement as HTMLElement; + } + + async function flushReRegistration(): Promise { + fixture.detectChanges(); + await new Promise((resolve) => setTimeout(resolve, 0)); + fixture.detectChanges(); + } + + it('clears credentials and redirects to /register when no name is saved', async () => { + playerName.get.mockReturnValue(''); + await render(); + + gameSocket.sessionExpired.set(true); + await flushReRegistration(); + + expect(credentials.clear).toHaveBeenCalled(); + expect(gameSocket.stop).toHaveBeenCalled(); + expect(router.navigateByUrl).toHaveBeenCalledWith('/register'); + }); + + it('re-registers with the saved name and reconnects', async () => { + playerName.get.mockReturnValue('Odin'); + api.registerPlayer.mockReturnValue(of({ id: 'NEWID' })); + const page = await render(); + + gameSocket.sessionExpired.set(true); + await flushReRegistration(); + + expect(api.registerPlayer).toHaveBeenCalledWith('Odin'); + expect(credentials.save).toHaveBeenCalledWith({ id: 'NEWID' }); + expect(credentials.clear).not.toHaveBeenCalled(); + expect(page.textContent).toContain('Re-registered. Reconnecting…'); + + const startCalls = gameSocket.start.mock.calls; + const onConnected = startCalls[startCalls.length - 1][1] as () => void; + expect(startCalls[startCalls.length - 1][0]).toBe('NEWID'); + onConnected(); + fixture.detectChanges(); + + expect(geolocation.start).toHaveBeenCalledWith(expect.any(Function)); + expect(page.textContent).toContain('Connected to PacMacro.'); + }); + + it('clears credentials and redirects when re-registration fails', async () => { + playerName.get.mockReturnValue('Odin'); + api.registerPlayer.mockReturnValue(throwError(() => new Error('API is down'))); + const page = await render(); + + gameSocket.sessionExpired.set(true); + await flushReRegistration(); + + expect(credentials.clear).toHaveBeenCalled(); + expect(gameSocket.stop).toHaveBeenCalled(); + expect(router.navigateByUrl).toHaveBeenCalledWith('/register'); + expect(page.textContent).toContain('Could not re-register. Redirecting…'); + }); + + it('treats an empty player ID from the API as a failure', async () => { + playerName.get.mockReturnValue('Odin'); + api.registerPlayer.mockReturnValue(of({ id: ' ' })); + const page = await render(); + + gameSocket.sessionExpired.set(true); + await flushReRegistration(); + + expect(credentials.clear).toHaveBeenCalled(); + expect(gameSocket.stop).toHaveBeenCalled(); + expect(router.navigateByUrl).toHaveBeenCalledWith('/register'); + expect(page.textContent).toContain('Could not re-register. Redirecting…'); + }); +}); diff --git a/frontend/src/app/pages/register-page/register-page.component.spec.ts b/frontend/src/app/pages/register-page/register-page.component.spec.ts index e560a89..263c45b 100644 --- a/frontend/src/app/pages/register-page/register-page.component.spec.ts +++ b/frontend/src/app/pages/register-page/register-page.component.spec.ts @@ -5,6 +5,7 @@ import { of } from 'rxjs'; import { ApiService } from '../../core/api.service'; import { CredentialsService } from '../../core/credentials.service'; +import { PlayerNameService } from '../../core/player-name.service'; import { RegisterPageComponent } from './register-page.component'; describe('RegisterPageComponent', () => { @@ -13,6 +14,7 @@ describe('RegisterPageComponent', () => { registerPlayer: vi.fn(() => of({ id: 'ABCD' })), }; const credentials = { save: vi.fn() }; + const playerName = { save: vi.fn() }; const router = { navigateByUrl: vi.fn(() => Promise.resolve(true)) }; beforeEach(() => { @@ -22,6 +24,7 @@ describe('RegisterPageComponent', () => { providers: [ { provide: ApiService, useValue: api }, { provide: CredentialsService, useValue: credentials }, + { provide: PlayerNameService, useValue: playerName }, { provide: Router, useValue: router }, ], }); @@ -31,7 +34,7 @@ describe('RegisterPageComponent', () => { const component = TestBed.createComponent(RegisterPageComponent) .componentInstance as unknown as RegisterPageHarness; component.registrationModel.set({ - name: 'Test2', + name: ' Test2 ', }); await component.submit(submitEvent()); @@ -39,6 +42,7 @@ describe('RegisterPageComponent', () => { expect(api.registerPlayer).toHaveBeenCalledWith('Test2'); expect(api.registerAdmin).not.toHaveBeenCalled(); expect(credentials.save).toHaveBeenCalledWith({ id: 'ABCD' }); + expect(playerName.save).toHaveBeenCalledWith('Test2'); expect(router.navigateByUrl).toHaveBeenCalledWith('/'); }); @@ -52,6 +56,8 @@ describe('RegisterPageComponent', () => { await component.submit(submitEvent()); expect(api.registerPlayer).not.toHaveBeenCalled(); + expect(credentials.save).not.toHaveBeenCalled(); + expect(playerName.save).not.toHaveBeenCalled(); }); }); From 9e60e143d0e7a9ef74128bc507e48d1b5edd2585 Mon Sep 17 00:00:00 2001 From: Karen Yao Date: Wed, 19 Aug 2026 01:53:27 -0700 Subject: [PATCH 4/6] refactor: extract socket connect to deduplicate socket start callback --- .../src/app/pages/game-page/game-page.component.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/frontend/src/app/pages/game-page/game-page.component.ts b/frontend/src/app/pages/game-page/game-page.component.ts index 7f0b548..e88f8f8 100644 --- a/frontend/src/app/pages/game-page/game-page.component.ts +++ b/frontend/src/app/pages/game-page/game-page.component.ts @@ -101,10 +101,7 @@ export class GamePageComponent { this.credentials.save({ id }); this.selfId.set(id); this.pageStatus.set('Re-registered. Reconnecting…'); - this.socket.start(id, () => { - this.pageStatus.set('Connected to PacMacro.'); - this.geolocation.start((coordinate) => this.socket.sendCoordinate(coordinate)); - }); + this.connectAs(id); } catch { this.credentials.clear(); this.socket.stop(); @@ -143,7 +140,11 @@ export class GamePageComponent { this.browserWindow.document.addEventListener('visibilitychange', this.onVisibilityChange); this.browserWindow.addEventListener('online', this.onOnline); this.browserWindow.addEventListener('offline', this.onOffline); - this.socket.start(credentials.id, () => { + this.connectAs(credentials.id); + } + + private connectAs(id: string): void { + this.socket.start(id, () => { this.pageStatus.set('Connected to PacMacro.'); this.geolocation.start((coordinate) => this.socket.sendCoordinate(coordinate)); }); From 20cc73bc5a550772c90832d030b47b4ec84bce53 Mon Sep 17 00:00:00 2001 From: Karen Yao Date: Wed, 19 Aug 2026 02:18:34 -0700 Subject: [PATCH 5/6] refactor: simplify session-expiry handling so it can't double-fire --- frontend/angular.json | 3 ++- .../core/sockets/game-socket.service.spec.ts | 20 +++++++++++++++- .../app/core/sockets/game-socket.service.ts | 6 ++++- .../game-page/game-page.component.spec.ts | 15 ++++++++---- .../pages/game-page/game-page.component.ts | 23 +++++++------------ 5 files changed, 44 insertions(+), 23 deletions(-) diff --git a/frontend/angular.json b/frontend/angular.json index 1d74f66..21e6be7 100644 --- a/frontend/angular.json +++ b/frontend/angular.json @@ -2,7 +2,8 @@ "$schema": "./node_modules/@angular/cli/lib/config/schema.json", "version": 1, "cli": { - "packageManager": "npm" + "packageManager": "npm", + "analytics": "132fecc1-3206-4f36-bf3c-2dcfd0f12560" }, "newProjectRoot": "projects", "projects": { diff --git a/frontend/src/app/core/sockets/game-socket.service.spec.ts b/frontend/src/app/core/sockets/game-socket.service.spec.ts index d8f3b20..6138836 100644 --- a/frontend/src/app/core/sockets/game-socket.service.spec.ts +++ b/frontend/src/app/core/sockets/game-socket.service.spec.ts @@ -302,7 +302,8 @@ describe('GameSocketService', () => { vi.useFakeTimers(); vi.spyOn(console, 'log').mockImplementation(() => undefined); vi.spyOn(console, 'warn').mockImplementation(() => undefined); - service.start('ABCD', () => undefined); + const onSessionExpired = vi.fn(); + service.start('ABCD', () => undefined, onSessionExpired); MockGameWebSocket.instances[0].serverClose(false); vi.advanceTimersByTime(1000); @@ -315,6 +316,23 @@ describe('GameSocketService', () => { expect(service.state()).toBe('error'); expect(service.status()).toContain('Session has expired as game server restarted.'); expect(MockGameWebSocket.instances).toHaveLength(3); + expect(onSessionExpired).toHaveBeenCalledOnce(); + }); + + it('does not invoke the session-expired callback after fewer than three failures', () => { + vi.useFakeTimers(); + vi.spyOn(console, 'log').mockImplementation(() => undefined); + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + const onSessionExpired = vi.fn(); + service.start('ABCD', () => undefined, onSessionExpired); + + MockGameWebSocket.instances[0].serverClose(false); + vi.advanceTimersByTime(1000); + MockGameWebSocket.instances[1].serverClose(false); + vi.advanceTimersByTime(2000); + + expect(service.sessionExpired()).toBe(false); + expect(onSessionExpired).not.toHaveBeenCalled(); }); it('keeps reconnecting a player socket after fewer than three failures', () => { diff --git a/frontend/src/app/core/sockets/game-socket.service.ts b/frontend/src/app/core/sockets/game-socket.service.ts index cd80361..8f835b6 100644 --- a/frontend/src/app/core/sockets/game-socket.service.ts +++ b/frontend/src/app/core/sockets/game-socket.service.ts @@ -29,6 +29,7 @@ export class GameSocketService extends WebSocketService { private playerId: string | null = null; private mode: SocketMode | null = null; private onConnected: (() => void) | null = null; + private onSessionExpired: (() => void) | null = null; private reconnecting = false; private suspendedReason = 'Paused while the browser is offline.'; private consecutiveFailures = 0; @@ -39,11 +40,12 @@ export class GameSocketService extends WebSocketService { readonly sessionExpired = signal(false); readonly MAX_FAILED_ATTEMPTS = 3; - start(id: string, onConnected: () => void): void { + start(id: string, onConnected: () => void, onSessionExpired: () => void = () => undefined): void { this.stop(); this.mode = 'player'; this.playerId = id; this.onConnected = onConnected; + this.onSessionExpired = onSessionExpired; this.consecutiveFailures = 0; this.sessionExpired.set(false); this.resume(); @@ -77,6 +79,7 @@ export class GameSocketService extends WebSocketService { this.mode = null; this.playerId = null; this.onConnected = null; + this.onSessionExpired = null; this.reconnecting = false; this.statusMessage.set(null); this.consecutiveFailures = 0; @@ -123,6 +126,7 @@ export class GameSocketService extends WebSocketService { this.statusMessage.set( 'Session has expired as game server restarted.', ); + this.onSessionExpired?.(); return false; } diff --git a/frontend/src/app/pages/game-page/game-page.component.spec.ts b/frontend/src/app/pages/game-page/game-page.component.spec.ts index 3d69963..5b7dd2e 100644 --- a/frontend/src/app/pages/game-page/game-page.component.spec.ts +++ b/frontend/src/app/pages/game-page/game-page.component.spec.ts @@ -33,6 +33,7 @@ const playerName = { save: vi.fn(), }; const router = { navigateByUrl: vi.fn() }; +let triggerSessionExpired: (() => void) | null = null; const gameSocket = { players: signal({ SELF: { @@ -48,7 +49,10 @@ const gameSocket = { status: signal('Connected.'), isFlagFound: signal(false), sessionExpired: signal(false), - start: vi.fn((_id: string, _onConnected: () => void) => gameSocket.sessionExpired.set(false)), + start: vi.fn((_id: string, _onConnected: () => void, onSessionExpired: () => void) => { + triggerSessionExpired = onSessionExpired; + gameSocket.sessionExpired.set(false); + }), stop: vi.fn(() => gameSocket.sessionExpired.set(false)), resume: vi.fn(), suspend: vi.fn(), @@ -143,6 +147,7 @@ describe('GamePageComponent re-registration', () => { beforeEach(async () => { vi.clearAllMocks(); gameSocket.sessionExpired.set(false); + triggerSessionExpired = null; await configureTestBed(); }); @@ -164,7 +169,7 @@ describe('GamePageComponent re-registration', () => { playerName.get.mockReturnValue(''); await render(); - gameSocket.sessionExpired.set(true); + triggerSessionExpired?.(); await flushReRegistration(); expect(credentials.clear).toHaveBeenCalled(); @@ -177,7 +182,7 @@ describe('GamePageComponent re-registration', () => { api.registerPlayer.mockReturnValue(of({ id: 'NEWID' })); const page = await render(); - gameSocket.sessionExpired.set(true); + triggerSessionExpired?.(); await flushReRegistration(); expect(api.registerPlayer).toHaveBeenCalledWith('Odin'); @@ -200,7 +205,7 @@ describe('GamePageComponent re-registration', () => { api.registerPlayer.mockReturnValue(throwError(() => new Error('API is down'))); const page = await render(); - gameSocket.sessionExpired.set(true); + triggerSessionExpired?.(); await flushReRegistration(); expect(credentials.clear).toHaveBeenCalled(); @@ -214,7 +219,7 @@ describe('GamePageComponent re-registration', () => { api.registerPlayer.mockReturnValue(of({ id: ' ' })); const page = await render(); - gameSocket.sessionExpired.set(true); + triggerSessionExpired?.(); await flushReRegistration(); expect(credentials.clear).toHaveBeenCalled(); diff --git a/frontend/src/app/pages/game-page/game-page.component.ts b/frontend/src/app/pages/game-page/game-page.component.ts index e88f8f8..3574ae2 100644 --- a/frontend/src/app/pages/game-page/game-page.component.ts +++ b/frontend/src/app/pages/game-page/game-page.component.ts @@ -4,7 +4,6 @@ import { Component, computed, DestroyRef, - effect, inject, signal, } from '@angular/core'; @@ -37,7 +36,6 @@ export class GamePageComponent { private readonly destroyRef = inject(DestroyRef); private readonly router = inject(Router); private readonly playerName = inject(PlayerNameService); - private readonly reregistering = signal(false); protected readonly socket = inject(GameSocketService); protected readonly geolocation = inject(GeolocationService); @@ -67,12 +65,6 @@ export class GamePageComponent { constructor() { afterNextRender(() => void this.initialize()); this.destroyRef.onDestroy(() => this.cleanup()); - - effect(() => { - if (this.socket.sessionExpired() && !this.reregistering()) { - void this.autoReregister(); - } - }); } protected async toggleWakeLock(event: Event): Promise { @@ -88,7 +80,6 @@ export class GamePageComponent { return; } - this.reregistering.set(true); this.pageStatus.set('Re-registering…'); try { @@ -107,8 +98,6 @@ export class GamePageComponent { this.socket.stop(); this.pageStatus.set('Could not re-register. Redirecting…'); await this.router.navigateByUrl('/register'); - } finally { - this.reregistering.set(false); } } @@ -144,10 +133,14 @@ export class GamePageComponent { } private connectAs(id: string): void { - this.socket.start(id, () => { - this.pageStatus.set('Connected to PacMacro.'); - this.geolocation.start((coordinate) => this.socket.sendCoordinate(coordinate)); - }); + this.socket.start( + id, + () => { + this.pageStatus.set('Connected to PacMacro.'); + this.geolocation.start((coordinate) => this.socket.sendCoordinate(coordinate)); + }, + () => void this.autoReregister(), + ); } private cleanup(): void { From f72fbeacd1ff284565dd5cfa93c16e6889e9f498 Mon Sep 17 00:00:00 2001 From: Karen Yao Date: Wed, 19 Aug 2026 02:19:21 -0700 Subject: [PATCH 6/6] fix: restore frontend/angular.json file --- frontend/angular.json | 3 +-- .../app/pages/register-page/register-page.component.spec.ts | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/frontend/angular.json b/frontend/angular.json index 21e6be7..1d74f66 100644 --- a/frontend/angular.json +++ b/frontend/angular.json @@ -2,8 +2,7 @@ "$schema": "./node_modules/@angular/cli/lib/config/schema.json", "version": 1, "cli": { - "packageManager": "npm", - "analytics": "132fecc1-3206-4f36-bf3c-2dcfd0f12560" + "packageManager": "npm" }, "newProjectRoot": "projects", "projects": { diff --git a/frontend/src/app/pages/register-page/register-page.component.spec.ts b/frontend/src/app/pages/register-page/register-page.component.spec.ts index 263c45b..9ff5f38 100644 --- a/frontend/src/app/pages/register-page/register-page.component.spec.ts +++ b/frontend/src/app/pages/register-page/register-page.component.spec.ts @@ -34,7 +34,7 @@ describe('RegisterPageComponent', () => { const component = TestBed.createComponent(RegisterPageComponent) .componentInstance as unknown as RegisterPageHarness; component.registrationModel.set({ - name: ' Test2 ', + name: 'Test2', }); await component.submit(submitEvent());