From dcd6dc00f4fbd715f45e79eb14f5ce15eef872da Mon Sep 17 00:00:00 2001 From: seavor Date: Sat, 18 Apr 2026 01:36:37 -0500 Subject: [PATCH] harden --- webclient/.env | 1 + webclient/src/__test-utils__/index.ts | 3 + webclient/src/__test-utils__/mockWebClient.ts | 56 ++++ .../__test-utils__/renderWithProviders.tsx | 71 +++++ webclient/src/__test-utils__/storeFixtures.ts | 123 +++++++++ .../src/api/response/GameResponseImpl.ts | 4 +- .../src/components/Guard/AuthGuard.spec.tsx | 33 +++ webclient/src/components/Guard/AuthGuard.tsx | 2 +- .../src/components/Guard/ModGuard.spec.tsx | 38 +++ .../components/InputField/InputField.spec.tsx | 30 ++ .../src/components/KnownHosts/KnownHosts.tsx | 8 +- .../src/components/Message/Message.spec.tsx | 19 ++ .../ThreePaneLayout/ThreePaneLayout.tsx | 3 +- .../UserDisplay/UserDisplay.spec.tsx | 46 +++ .../VirtualList/VirtualList.spec.tsx | 21 ++ .../components/VirtualList/VirtualList.tsx | 2 +- webclient/src/containers/Account/Account.tsx | 35 ++- webclient/src/containers/Login/Login.tsx | 10 +- webclient/src/containers/Logs/Logs.tsx | 3 +- .../src/containers/Room/OpenGames.spec.tsx | 35 +++ webclient/src/containers/Room/OpenGames.tsx | 15 +- webclient/src/containers/Room/Room.tsx | 8 +- webclient/src/containers/Server/Server.tsx | 6 +- webclient/src/forms/LoginForm/LoginForm.tsx | 2 - .../src/forms/RegisterForm/RegisterForm.tsx | 11 +- webclient/src/hooks/index.ts | 1 - webclient/src/hooks/useAutoConnect.spec.ts | 86 ++++++ webclient/src/hooks/useAutoConnect.ts | 32 +-- webclient/src/hooks/useDebounce.ts | 34 --- .../hooks/useFireOnce/useFireOnce.spec.tsx | 54 ++++ .../src/hooks/useFireOnce/useFireOnce.ts | 23 +- webclient/src/hooks/useLocaleSort.spec.ts | 80 ++++++ webclient/src/hooks/useLocaleSort.ts | 17 +- webclient/src/hooks/useReduxEffect.spec.tsx | 138 +++++++++ webclient/src/hooks/useWebClient.spec.tsx | 48 ++++ .../src/services/CardImporterService.spec.ts | 261 ++++++++++++++++++ webclient/src/services/CardImporterService.ts | 27 +- webclient/src/services/HostService.ts | 47 ++++ .../src/services/dexie/DexieDTOs/CardDTO.ts | 5 +- .../src/services/dexie/DexieDTOs/SetDTO.ts | 5 +- .../services/dexie/DexieDTOs/SettingDTO.ts | 4 +- .../src/services/dexie/DexieDTOs/TokenDTO.ts | 5 +- .../services/dexie/DexieSchemas/v1.schema.ts | 4 +- webclient/src/services/index.ts | 1 + webclient/src/setupTests.ts | 37 +++ .../src/store/actions/actionReducer.spec.ts | 26 +- webclient/src/store/actions/actionReducer.ts | 14 +- .../src/store/common/normalizers.spec.ts | 18 +- webclient/src/store/common/normalizers.ts | 9 +- .../src/store/game/game.dispatch.spec.ts | 4 +- webclient/src/store/game/game.dispatch.ts | 4 +- webclient/src/store/game/game.reducer.spec.ts | 126 ++++++++- webclient/src/store/game/game.reducer.ts | 38 +-- .../src/store/rooms/rooms.actions.spec.ts | 3 +- .../src/store/rooms/rooms.dispatch.spec.ts | 2 +- webclient/src/store/rooms/rooms.dispatch.tsx | 2 +- .../src/store/rooms/rooms.reducer.spec.ts | 1 + webclient/src/store/rooms/rooms.reducer.tsx | 21 +- .../store/server/__mocks__/server-fixtures.ts | 2 +- .../src/store/server/server.actions.spec.ts | 4 +- .../src/store/server/server.dispatch.spec.ts | 8 +- webclient/src/store/server/server.dispatch.ts | 4 +- .../src/store/server/server.interfaces.ts | 2 +- .../src/store/server/server.reducer.spec.ts | 44 ++- webclient/src/store/server/server.reducer.ts | 90 +++--- .../src/store/server/server.selectors.spec.ts | 18 +- .../src/store/server/server.selectors.ts | 4 +- webclient/src/types/app.ts | 2 +- webclient/src/types/countries.ts | 4 +- webclient/src/types/enriched.ts | 25 +- ...nstants.spec.ts => regex-patterns.spec.ts} | 2 +- .../types/{constants.ts => regex-patterns.ts} | 0 webclient/src/types/server.ts | 59 +--- .../session/sessionCommands-complex.spec.ts | 16 +- .../websocket/events/game/gameEvents.spec.ts | 4 +- .../src/websocket/events/game/gameSay.ts | 2 +- .../websocket/interfaces/WebClientResponse.ts | 2 +- .../services/KeepAliveService.spec.ts | 9 + .../websocket/services/KeepAliveService.ts | 1 + .../services/ProtobufService.spec.ts | 65 ++++- .../src/websocket/services/ProtobufService.ts | 49 ++-- .../services/WebSocketService.spec.ts | 6 + .../websocket/services/WebSocketService.ts | 3 + 83 files changed, 1797 insertions(+), 390 deletions(-) create mode 100644 webclient/src/__test-utils__/mockWebClient.ts create mode 100644 webclient/src/__test-utils__/renderWithProviders.tsx create mode 100644 webclient/src/__test-utils__/storeFixtures.ts create mode 100644 webclient/src/components/Guard/AuthGuard.spec.tsx create mode 100644 webclient/src/components/Guard/ModGuard.spec.tsx create mode 100644 webclient/src/components/InputField/InputField.spec.tsx create mode 100644 webclient/src/components/Message/Message.spec.tsx create mode 100644 webclient/src/components/UserDisplay/UserDisplay.spec.tsx create mode 100644 webclient/src/components/VirtualList/VirtualList.spec.tsx create mode 100644 webclient/src/containers/Room/OpenGames.spec.tsx create mode 100644 webclient/src/hooks/useAutoConnect.spec.ts delete mode 100644 webclient/src/hooks/useDebounce.ts create mode 100644 webclient/src/hooks/useLocaleSort.spec.ts create mode 100644 webclient/src/hooks/useReduxEffect.spec.tsx create mode 100644 webclient/src/hooks/useWebClient.spec.tsx create mode 100644 webclient/src/services/CardImporterService.spec.ts create mode 100644 webclient/src/services/HostService.ts rename webclient/src/types/{constants.spec.ts => regex-patterns.spec.ts} (98%) rename webclient/src/types/{constants.ts => regex-patterns.ts} (100%) diff --git a/webclient/.env b/webclient/.env index 8592b28b4..be70fd388 100644 --- a/webclient/.env +++ b/webclient/.env @@ -1 +1,2 @@ # Future template for server admin configuration +NODE_OPTIONS=--max-old-space-size=8192 \ No newline at end of file diff --git a/webclient/src/__test-utils__/index.ts b/webclient/src/__test-utils__/index.ts index 6d606210a..4deac33d2 100644 --- a/webclient/src/__test-utils__/index.ts +++ b/webclient/src/__test-utils__/index.ts @@ -1 +1,4 @@ export { withMockLocation, withEventRegistry } from './globalGuards'; +export { renderWithProviders } from './renderWithProviders'; +export { createMockWebClient } from './mockWebClient'; +export { disconnectedState, connectedState, connectedWithRoomsState, makeUser } from './storeFixtures'; diff --git a/webclient/src/__test-utils__/mockWebClient.ts b/webclient/src/__test-utils__/mockWebClient.ts new file mode 100644 index 000000000..3faafe391 --- /dev/null +++ b/webclient/src/__test-utils__/mockWebClient.ts @@ -0,0 +1,56 @@ +import type { WebClient } from '@app/websocket'; + +/** + * Creates a mock WebClient whose `request` property has vi.fn() stubs + * for every service method that containers/forms call. Inject this into + * tests via `renderWithProviders({ webClient: createMockWebClient() })`. + */ +export function createMockWebClient() { + return { + request: { + authentication: { + login: vi.fn(), + register: vi.fn(), + disconnect: vi.fn(), + activateAccount: vi.fn(), + resetPasswordRequest: vi.fn(), + resetPasswordChallenge: vi.fn(), + resetPassword: vi.fn(), + }, + session: { + addToBuddyList: vi.fn(), + removeFromBuddyList: vi.fn(), + addToIgnoreList: vi.fn(), + removeFromIgnoreList: vi.fn(), + getUserInfo: vi.fn(), + accountEdit: vi.fn(), + accountPassword: vi.fn(), + accountImage: vi.fn(), + listUsers: vi.fn(), + }, + rooms: { + joinRoom: vi.fn(), + leaveRoom: vi.fn(), + roomSay: vi.fn(), + createGame: vi.fn(), + }, + game: { + joinGame: vi.fn(), + leaveGame: vi.fn(), + }, + admin: { + adjustMod: vi.fn(), + reloadConfig: vi.fn(), + shutdownServer: vi.fn(), + updateServerMessage: vi.fn(), + }, + moderator: { + viewLogHistory: vi.fn(), + banFromServer: vi.fn(), + warnUser: vi.fn(), + warnHistory: vi.fn(), + banHistory: vi.fn(), + }, + }, + } as unknown as WebClient; +} diff --git a/webclient/src/__test-utils__/renderWithProviders.tsx b/webclient/src/__test-utils__/renderWithProviders.tsx new file mode 100644 index 000000000..c7ae2d242 --- /dev/null +++ b/webclient/src/__test-utils__/renderWithProviders.tsx @@ -0,0 +1,71 @@ +import { ReactElement } from 'react'; +import { render, RenderOptions } from '@testing-library/react'; +import { configureStore, EnhancedStore } from '@reduxjs/toolkit'; +import { Provider } from 'react-redux'; +import { MemoryRouter } from 'react-router-dom'; +import { I18nextProvider } from 'react-i18next'; +import i18n from 'i18next'; +import { initReactI18next } from 'react-i18next'; + +import { gamesReducer } from '../store/game'; +import { roomsReducer } from '../store/rooms'; +import { serverReducer } from '../store/server'; +import { actionReducer } from '../store/actions'; +import { ToastProvider } from '../components/Toast/ToastContext'; +import type { RootState } from '../store/store'; + +// Minimal i18n instance for tests — returns keys as-is. +const testI18n = i18n.createInstance(); +testI18n.use(initReactI18next).init({ + lng: 'en', + resources: {}, + fallbackLng: 'en', + interpolation: { escapeValue: false }, +}); + +function createTestStore(preloadedState?: Partial) { + return configureStore({ + reducer: { + games: gamesReducer, + rooms: roomsReducer, + server: serverReducer, + action: actionReducer, + }, + preloadedState: preloadedState as any, + }); +} + +interface ExtendedRenderOptions extends Omit { + preloadedState?: Partial; + store?: EnhancedStore; + route?: string; +} + +export function renderWithProviders( + ui: ReactElement, + { + preloadedState, + store = createTestStore(preloadedState), + route = '/', + ...renderOptions + }: ExtendedRenderOptions = {}, +) { + function Wrapper({ children }: { children: React.ReactNode }) { + return ( + + + + + {children} + + + + + ); + } + + return { + store, + ...render(ui, { wrapper: Wrapper, ...renderOptions }), + }; +} diff --git a/webclient/src/__test-utils__/storeFixtures.ts b/webclient/src/__test-utils__/storeFixtures.ts new file mode 100644 index 000000000..9733fe6ef --- /dev/null +++ b/webclient/src/__test-utils__/storeFixtures.ts @@ -0,0 +1,123 @@ +import { App, Data, Enriched } from '@app/types'; +import type { RootState } from '../store/store'; + +/** + * Create a minimal ServerInfo_User object for testing. + */ +function makeUser(overrides: Partial = {}): Data.ServerInfo_User { + return { + name: 'testUser', + realName: '', + country: 'us', + userLevel: 0, + avatarBmp: new Uint8Array(), + accountageSecs: BigInt(0), + $typeName: 'ServerInfo_User' as any, + $unknown: undefined, + gender: 0, + ...overrides, + } as Data.ServerInfo_User; +} + +/** + * A disconnected (default) store state. This is the state before any + * connection to a server has been made. + */ +export const disconnectedState: Partial = { + server: { + initialized: false, + buddyList: {}, + ignoreList: {}, + status: { + connectionAttemptMade: false, + state: Enriched.StatusEnum.DISCONNECTED, + description: null, + }, + info: { message: null, name: null, version: null }, + logs: { room: [], game: [], chat: [] }, + user: null, + users: {}, + sortUsersBy: { field: App.UserSortField.NAME, order: App.SortDirection.ASC }, + messages: {}, + userInfo: {}, + notifications: [], + serverShutdown: null, + banUser: '', + banHistory: {}, + warnHistory: {}, + warnListOptions: [], + warnUser: '', + adminNotes: {}, + replays: {}, + backendDecks: null, + downloadedDeck: null, + downloadedReplay: null, + gamesOfUser: {}, + registrationError: null, + }, + rooms: { + rooms: {}, + joinedRoomIds: {}, + joinedGameIds: {}, + messages: {}, + sortGamesBy: { field: App.GameSortField.START_TIME, order: App.SortDirection.DESC }, + sortUsersBy: { field: App.UserSortField.NAME, order: App.SortDirection.ASC }, + }, + games: { games: {} }, + action: { type: null, payload: null, meta: null, error: false, count: 0 }, +}; + +/** + * A connected (logged-in) store state with a basic user and server info. + */ +export const connectedState: Partial = { + ...disconnectedState, + server: { + ...(disconnectedState.server as any), + initialized: true, + status: { + connectionAttemptMade: true, + state: Enriched.StatusEnum.LOGGED_IN, + description: null, + }, + info: { + message: 'Welcome', + name: 'Test Server', + version: '1.0.0', + }, + user: makeUser(), + users: { + testUser: makeUser(), + }, + }, +}; + +/** + * Connected state with rooms and a joined room containing games and users. + */ +export const connectedWithRoomsState: Partial = { + ...connectedState, + server: { + ...(connectedState.server as any), + users: { + testUser: makeUser(), + otherUser: makeUser({ name: 'otherUser' }), + }, + }, + rooms: { + ...(disconnectedState.rooms as any), + rooms: { + 1: { + info: { roomId: 1, name: 'Main Room', description: 'The main room', autoJoin: true, permissionLevel: 0 }, + gameList: [], + userList: [makeUser(), makeUser({ name: 'otherUser' })], + }, + }, + joinedRoomIds: { 1: true }, + messages: { + 1: [], + }, + }, +}; + +export { makeUser }; diff --git a/webclient/src/api/response/GameResponseImpl.ts b/webclient/src/api/response/GameResponseImpl.ts index cb8b9fef7..01978e818 100644 --- a/webclient/src/api/response/GameResponseImpl.ts +++ b/webclient/src/api/response/GameResponseImpl.ts @@ -35,8 +35,8 @@ export class GameResponseImpl implements IGameResponse { GameDispatch.kicked(gameId); } - gameSay(gameId: number, playerId: number, message: string): void { - GameDispatch.gameSay(gameId, playerId, message); + gameSay(gameId: number, playerId: number, message: string, timeReceived: number): void { + GameDispatch.gameSay(gameId, playerId, message, timeReceived); } cardMoved(gameId: number, playerId: number, data: Data.Event_MoveCard): void { diff --git a/webclient/src/components/Guard/AuthGuard.spec.tsx b/webclient/src/components/Guard/AuthGuard.spec.tsx new file mode 100644 index 000000000..92368cbab --- /dev/null +++ b/webclient/src/components/Guard/AuthGuard.spec.tsx @@ -0,0 +1,33 @@ +import { screen } from '@testing-library/react'; +import { renderWithProviders, connectedState, disconnectedState } from '../../__test-utils__'; +import AuthGuard from './AuthGuard'; + +vi.mock('@app/hooks', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, useWebClient: vi.fn(() => ({})) }; +}); + +describe('AuthGuard', () => { + it('redirects to LOGIN when disconnected', () => { + renderWithProviders(, { + preloadedState: disconnectedState, + route: '/server', + }); + + // Navigate triggers a route change — AuthGuard itself renders no text. + // We verify it doesn't render any meaningful content. + expect(screen.queryByRole('button')).toBeNull(); + expect(screen.queryByRole('heading')).toBeNull(); + }); + + it('renders nothing visible when connected', () => { + const { container } = renderWithProviders(, { + preloadedState: connectedState, + route: '/server', + }); + + // AuthGuard renders an empty fragment when connected. + // The only DOM is from provider wrappers (e.g. ToastProvider's container div). + expect(container.textContent).toBe(''); + }); +}); diff --git a/webclient/src/components/Guard/AuthGuard.tsx b/webclient/src/components/Guard/AuthGuard.tsx index 4bbb8e1d5..897556613 100644 --- a/webclient/src/components/Guard/AuthGuard.tsx +++ b/webclient/src/components/Guard/AuthGuard.tsx @@ -8,7 +8,7 @@ const AuthGuard = () => { const isConnected = useAppSelector(ServerSelectors.getIsConnected); return !isConnected ? - :
; + : <>; }; export default AuthGuard; diff --git a/webclient/src/components/Guard/ModGuard.spec.tsx b/webclient/src/components/Guard/ModGuard.spec.tsx new file mode 100644 index 000000000..962e74b33 --- /dev/null +++ b/webclient/src/components/Guard/ModGuard.spec.tsx @@ -0,0 +1,38 @@ +import { renderWithProviders, connectedState, makeUser } from '../../__test-utils__'; +import { Data } from '@app/types'; +import ModGuard from './ModGuard'; + +vi.mock('@app/hooks', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, useWebClient: vi.fn(() => ({})) }; +}); + +describe('ModGuard', () => { + it('redirects when user is not a moderator', () => { + const { container } = renderWithProviders(, { + preloadedState: connectedState, + route: '/logs', + }); + + expect(container.textContent).toBe(''); + }); + + it('renders nothing visible when user is a moderator', () => { + const modUser = makeUser({ + userLevel: Data.ServerInfo_User_UserLevelFlag.IsModerator, + }); + + const { container } = renderWithProviders(, { + preloadedState: { + ...connectedState, + server: { + ...(connectedState.server as any), + user: modUser, + }, + }, + route: '/logs', + }); + + expect(container.textContent).toBe(''); + }); +}); diff --git a/webclient/src/components/InputField/InputField.spec.tsx b/webclient/src/components/InputField/InputField.spec.tsx new file mode 100644 index 000000000..1f535a175 --- /dev/null +++ b/webclient/src/components/InputField/InputField.spec.tsx @@ -0,0 +1,30 @@ +import { render, screen } from '@testing-library/react'; +import InputField from './InputField'; + +describe('InputField', () => { + const defaultProps = { + input: { name: 'test', value: '', onChange: vi.fn(), onBlur: vi.fn(), onFocus: vi.fn() }, + meta: { touched: false, error: null, warning: null }, + label: 'Test Field', + }; + + it('renders a text field with label', () => { + render(); + expect(screen.getByRole('textbox')).toBeInTheDocument(); + }); + + it('shows error when touched and has error', () => { + render(); + expect(screen.getByText('Required')).toBeInTheDocument(); + }); + + it('shows warning when touched and has warning', () => { + render(); + expect(screen.getByText('Weak password')).toBeInTheDocument(); + }); + + it('does not show validation messages when not touched', () => { + render(); + expect(screen.queryByText('Required')).not.toBeInTheDocument(); + }); +}); diff --git a/webclient/src/components/KnownHosts/KnownHosts.tsx b/webclient/src/components/KnownHosts/KnownHosts.tsx index e233bc5ae..c16c4f67a 100644 --- a/webclient/src/components/KnownHosts/KnownHosts.tsx +++ b/webclient/src/components/KnownHosts/KnownHosts.tsx @@ -16,7 +16,7 @@ import ErrorOutlinedIcon from '@mui/icons-material/ErrorOutlined'; import { useWebClient } from '@app/hooks'; import { KnownHostDialog } from '@app/dialogs'; import { useReduxEffect } from '@app/hooks'; -import { HostDTO } from '@app/services'; +import { DefaultHosts, HostDTO, getHostPort } from '@app/services'; import { ServerTypes } from '@app/store'; import { App } from '@app/types'; import Toast from '../Toast/Toast'; @@ -87,7 +87,7 @@ const KnownHosts = (props) => { if (!hosts?.length) { // @TODO: find a better pattern to seeding default data in indexedDB - await HostDTO.bulkAdd(App.DefaultHosts); + await HostDTO.bulkAdd(DefaultHosts); loadKnownHosts(); } else { const selectedHost = hosts.find(({ lastSelected }) => lastSelected) || hosts[0]; @@ -197,7 +197,7 @@ const KnownHosts = (props) => { const testConnection = () => { setTestingConnection(TestConnection.TESTING); - const options = { ...App.getHostPort(hostsState.selectedHost) }; + const options = { ...getHostPort(hostsState.selectedHost) }; webClient.request.authentication.testConnection(options); } @@ -238,7 +238,7 @@ const KnownHosts = (props) => { { hostsState.hosts.map((host, index) => { - const hostPort = App.getHostPort(hostsState.hosts[index]); + const hostPort = getHostPort(hostsState.hosts[index]); return ( diff --git a/webclient/src/components/Message/Message.spec.tsx b/webclient/src/components/Message/Message.spec.tsx new file mode 100644 index 000000000..542760127 --- /dev/null +++ b/webclient/src/components/Message/Message.spec.tsx @@ -0,0 +1,19 @@ +import { screen } from '@testing-library/react'; +import { renderWithProviders } from '../../__test-utils__'; +import Message from './Message'; + +describe('Message', () => { + it('renders a plain message', () => { + const message = { message: 'Hello world' }; + renderWithProviders(); + + expect(screen.getByText('Hello world')).toBeInTheDocument(); + }); + + it('renders the message container', () => { + const message = { message: 'Test message' }; + const { container } = renderWithProviders(); + + expect(container.querySelector('.message')).toBeInTheDocument(); + }); +}); diff --git a/webclient/src/components/ThreePaneLayout/ThreePaneLayout.tsx b/webclient/src/components/ThreePaneLayout/ThreePaneLayout.tsx index 287e58382..0e12cd8ec 100644 --- a/webclient/src/components/ThreePaneLayout/ThreePaneLayout.tsx +++ b/webclient/src/components/ThreePaneLayout/ThreePaneLayout.tsx @@ -3,8 +3,7 @@ import Grid from '@mui/material/Grid'; import './ThreePaneLayout.css'; -// @DEPRECATED -// This component sucks balls, dont use it. It will be removed sooner than later. +/** @deprecated Scheduled for replacement with a more flexible layout component. */ function ThreePaneLayout(props: ThreePaneLayoutProps) { return (
diff --git a/webclient/src/components/UserDisplay/UserDisplay.spec.tsx b/webclient/src/components/UserDisplay/UserDisplay.spec.tsx new file mode 100644 index 000000000..67c4dce8d --- /dev/null +++ b/webclient/src/components/UserDisplay/UserDisplay.spec.tsx @@ -0,0 +1,46 @@ +import { screen } from '@testing-library/react'; +import { renderWithProviders, connectedState, makeUser, createMockWebClient } from '../../__test-utils__'; +import UserDisplay from './UserDisplay'; + +const mockWebClient = createMockWebClient(); + +vi.mock('@app/hooks', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, useWebClient: vi.fn(() => mockWebClient) }; +}); + +vi.mock('@app/images', () => ({ + Images: { Countries: { us: 'us.png', de: 'de.png' } }, +})); + +describe('UserDisplay', () => { + it('renders user name', () => { + const user = makeUser({ name: 'TestPlayer', country: 'us' }); + renderWithProviders(, { + preloadedState: connectedState, + }); + + expect(screen.getByText('TestPlayer')).toBeInTheDocument(); + }); + + it('renders country flag image', () => { + const user = makeUser({ name: 'TestPlayer', country: 'us' }); + renderWithProviders(, { + preloadedState: connectedState, + }); + + const img = screen.getByAltText('us'); + expect(img).toBeInTheDocument(); + expect(img).toHaveAttribute('src', 'us.png'); + }); + + it('renders link to player profile', () => { + const user = makeUser({ name: 'TestPlayer', country: 'us' }); + renderWithProviders(, { + preloadedState: connectedState, + }); + + const link = screen.getByRole('link', { name: /TestPlayer/ }); + expect(link).toHaveAttribute('href', '/player/TestPlayer'); + }); +}); diff --git a/webclient/src/components/VirtualList/VirtualList.spec.tsx b/webclient/src/components/VirtualList/VirtualList.spec.tsx new file mode 100644 index 000000000..1e59b7a1a --- /dev/null +++ b/webclient/src/components/VirtualList/VirtualList.spec.tsx @@ -0,0 +1,21 @@ +import { render } from '@testing-library/react'; +import VirtualList from './VirtualList'; + +describe('VirtualList', () => { + it('renders without crashing with empty items', () => { + const { container } = render(); + expect(container.querySelector('.virtual-list')).toBeInTheDocument(); + }); + + it('accepts className as a string', () => { + const { container } = render(); + expect(container.querySelector('.custom-class')).toBeInTheDocument(); + }); + + it('applies empty string as default className (not object)', () => { + const { container } = render(); + const list = container.querySelector('.virtual-list__list'); + // className should not contain "[object Object]" + expect(list?.className).not.toContain('[object Object]'); + }); +}); diff --git a/webclient/src/components/VirtualList/VirtualList.tsx b/webclient/src/components/VirtualList/VirtualList.tsx index fc2868f2d..90c15436e 100644 --- a/webclient/src/components/VirtualList/VirtualList.tsx +++ b/webclient/src/components/VirtualList/VirtualList.tsx @@ -15,7 +15,7 @@ const Row = ({ index, style, items }: RowComponentProps) => (
); -const VirtualList = ({ items, className = {}, size = 30 }) => ( +const VirtualList = ({ items, className = '', size = 30 }) => (
className={`virtual-list__list ${className}`} diff --git a/webclient/src/containers/Account/Account.tsx b/webclient/src/containers/Account/Account.tsx index ee06f590a..3902b31d6 100644 --- a/webclient/src/containers/Account/Account.tsx +++ b/webclient/src/containers/Account/Account.tsx @@ -1,5 +1,4 @@ -// eslint-disable-next-line -import React, { Component } from "react"; +import { useEffect, useMemo } from 'react'; import { useTranslation } from 'react-i18next'; import Button from '@mui/material/Button'; @@ -25,7 +24,21 @@ const Account = () => { const user = useAppSelector(state => ServerSelectors.getUser(state)); const webClient = useWebClient(); const { country, realName, name, userLevel, accountageSecs, avatarBmp } = user || {}; - let url = URL.createObjectURL(new Blob([avatarBmp as BlobPart], { 'type': 'image/png' })); + + const avatarUrl = useMemo(() => { + if (!avatarBmp) { + return ''; + } + return URL.createObjectURL(new Blob([avatarBmp as BlobPart], { type: 'image/png' })); + }, [avatarBmp]); + + useEffect(() => { + return () => { + if (avatarUrl) { + URL.revokeObjectURL(avatarUrl); + } + }; + }, [avatarUrl]); const { t } = useTranslation(); @@ -42,41 +55,41 @@ const Account = () => {
-
+
Buddies Online: ?/{buddyList.length}
( - + )) } /> -
+
-
+
Ignored Users Online: ?/{ignoreList.length}
( - + )) } /> -
+
- {name} + { avatarUrl && {name} }

{name}

Location: ({country?.toUpperCase()})

User Level: {userLevel}

@@ -95,7 +108,7 @@ const Account = () => { diff --git a/webclient/src/containers/Login/Login.tsx b/webclient/src/containers/Login/Login.tsx index 70896ce77..cf0a20420 100644 --- a/webclient/src/containers/Login/Login.tsx +++ b/webclient/src/containers/Login/Login.tsx @@ -11,7 +11,7 @@ import { LanguageDropdown } from '@app/components'; import { LoginForm } from '@app/forms'; import { useReduxEffect, useFireOnce, useWebClient } from '@app/hooks'; import { Images } from '@app/images'; -import { HostDTO, serverProps } from '@app/services'; +import { HostDTO, getHostPort, serverProps } from '@app/services'; import { App, Enriched } from '@app/types'; import { ServerSelectors, ServerTypes } from '@app/store'; import Layout from '../Layout/Layout'; @@ -125,7 +125,7 @@ const Login = () => { const { userName, password, selectedHost, remember } = loginForm; const options: Omit = { - ...App.getHostPort(selectedHost), + ...getHostPort(selectedHost), userName, password, }; @@ -154,7 +154,7 @@ const Login = () => { const { userName, password, email, country, realName, selectedHost } = registerForm; webClient.request.authentication.register({ - ...App.getHostPort(selectedHost), + ...getHostPort(selectedHost), userName, password, email, @@ -177,7 +177,7 @@ const Login = () => { const handleRequestPasswordResetDialogSubmit = (form) => { const { userName, email, selectedHost } = form; - const { host, port } = App.getHostPort(selectedHost); + const { host, port } = getHostPort(selectedHost); if (email) { webClient.request.authentication.resetPasswordChallenge({ userName, email, host, port }); @@ -188,7 +188,7 @@ const Login = () => { }; const handleResetPasswordDialogSubmit = ({ userName, token, newPassword, selectedHost }) => { - const { host, port } = App.getHostPort(selectedHost); + const { host, port } = getHostPort(selectedHost); webClient.request.authentication.resetPassword({ userName, token, newPassword, host, port }); }; diff --git a/webclient/src/containers/Logs/Logs.tsx b/webclient/src/containers/Logs/Logs.tsx index 8ab9a181b..bf2fe9a6d 100644 --- a/webclient/src/containers/Logs/Logs.tsx +++ b/webclient/src/containers/Logs/Logs.tsx @@ -1,5 +1,4 @@ -// eslint-disable-next-line -import React, { useEffect } from "react"; +import { useEffect } from 'react'; import { AuthGuard, ModGuard } from '@app/components'; import { SearchForm } from '@app/forms'; diff --git a/webclient/src/containers/Room/OpenGames.spec.tsx b/webclient/src/containers/Room/OpenGames.spec.tsx new file mode 100644 index 000000000..6da3edf90 --- /dev/null +++ b/webclient/src/containers/Room/OpenGames.spec.tsx @@ -0,0 +1,35 @@ +import { screen } from '@testing-library/react'; +import { renderWithProviders, connectedWithRoomsState } from '../../__test-utils__'; +import OpenGames from './OpenGames'; + +vi.mock('@app/hooks', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, useWebClient: vi.fn(() => ({})) }; +}); + +describe('OpenGames', () => { + const roomWithGames = { + info: { roomId: 1, name: 'Main Room' }, + }; + + it('renders the games table headers', () => { + renderWithProviders(, { + preloadedState: connectedWithRoomsState, + }); + + expect(screen.getByText('Age')).toBeInTheDocument(); + expect(screen.getByText('Description')).toBeInTheDocument(); + expect(screen.getByText('Creator')).toBeInTheDocument(); + expect(screen.getByText('Type')).toBeInTheDocument(); + expect(screen.getByText('Players')).toBeInTheDocument(); + expect(screen.getByText('Spectators')).toBeInTheDocument(); + }); + + it('renders without crashing when no games exist', () => { + const { container } = renderWithProviders(, { + preloadedState: connectedWithRoomsState, + }); + + expect(container.querySelector('.games')).toBeInTheDocument(); + }); +}); diff --git a/webclient/src/containers/Room/OpenGames.tsx b/webclient/src/containers/Room/OpenGames.tsx index d5ab68f6f..59e721aa1 100644 --- a/webclient/src/containers/Room/OpenGames.tsx +++ b/webclient/src/containers/Room/OpenGames.tsx @@ -1,5 +1,4 @@ -// eslint-disable-next-line -import React from "react"; +import React from 'react'; import Table from '@mui/material/Table'; import TableBody from '@mui/material/TableBody'; @@ -42,17 +41,17 @@ const OpenGames = ({ room }: OpenGamesProps) => { RoomsDispatch.sortGames(roomId, field, order); }; - const isUnavailableGame = ({ started, maxPlayers, playerCount }) => + const isAvailable = ({ started, maxPlayers, playerCount }) => !started && playerCount < maxPlayers; - const isPasswordProtectedGame = ({ withPassword }) => !withPassword; + const isOpen = ({ withPassword }) => !withPassword; - const isBuddiesOnlyGame = ({ onlyBuddies }) => !onlyBuddies; + const isPublic = ({ onlyBuddies }) => !onlyBuddies; const games = sortedGames.filter(game => ( - isUnavailableGame(game.info) && - isPasswordProtectedGame(game.info) && - isBuddiesOnlyGame(game.info) + isAvailable(game.info) && + isOpen(game.info) && + isPublic(game.info) )); return ( diff --git a/webclient/src/containers/Room/Room.tsx b/webclient/src/containers/Room/Room.tsx index 5877c57b5..d2f8455bc 100644 --- a/webclient/src/containers/Room/Room.tsx +++ b/webclient/src/containers/Room/Room.tsx @@ -25,7 +25,7 @@ const Room = () => { const navigate = useNavigate(); const params = useParams(); - const roomId = parseInt(params.roomId, 0); + const roomId = parseInt(params.roomId, 10); const room = rooms[roomId]; const roomMessages = messages[roomId]; const users = useAppSelector(state => RoomsSelectors.getSortedRoomUsers(state, roomId)); @@ -37,6 +37,10 @@ const Room = () => { } }, [joined]); + if (!room) { + return null; + } + const handleRoomSay = ({ message }) => { if (message) { webClient.request.rooms.roomSay(roomId, message); @@ -78,7 +82,7 @@ const Room = () => { ( - + )) } diff --git a/webclient/src/containers/Server/Server.tsx b/webclient/src/containers/Server/Server.tsx index d19e829bc..1620138fc 100644 --- a/webclient/src/containers/Server/Server.tsx +++ b/webclient/src/containers/Server/Server.tsx @@ -1,5 +1,4 @@ -// eslint-disable-next-line -import React from "react"; +import React from 'react'; import { generatePath, useNavigate } from 'react-router-dom'; import ListItemButton from '@mui/material/ListItemButton'; @@ -40,6 +39,7 @@ const Server = () => { bottom={( + {/* message is sanitized via DOMPurify in websocket/events/session/serverMessage.ts */}
)} @@ -51,7 +51,7 @@ const Server = () => {
( - + )) } diff --git a/webclient/src/forms/LoginForm/LoginForm.tsx b/webclient/src/forms/LoginForm/LoginForm.tsx index 4d6ee4e37..f46764126 100644 --- a/webclient/src/forms/LoginForm/LoginForm.tsx +++ b/webclient/src/forms/LoginForm/LoginForm.tsx @@ -43,8 +43,6 @@ const LoginForm = ({ onSubmit, disableSubmitButton, onResetPassword }: LoginForm const handleOnSubmit = ({ userName, ...values }) => { userName = userName?.trim(); - console.log(userName, values); - onSubmit({ userName, ...values }); } diff --git a/webclient/src/forms/RegisterForm/RegisterForm.tsx b/webclient/src/forms/RegisterForm/RegisterForm.tsx index 66d8916be..3754c857c 100644 --- a/webclient/src/forms/RegisterForm/RegisterForm.tsx +++ b/webclient/src/forms/RegisterForm/RegisterForm.tsx @@ -1,4 +1,4 @@ -import { useState } from 'react'; +import { useEffect, useState } from 'react'; import { useSelector } from 'react-redux'; import { Form, Field } from 'react-final-form'; import { OnChange } from 'react-final-form-listeners'; @@ -99,10 +99,11 @@ const RegisterForm = ({ onSubmit }: RegisterFormProps) => {
{({ handleSubmit, form }) => { - if (emailRequired) { - // Allow form render to complete - setTimeout(() => form.mutators.setFieldTouched('email', true)) - } + useEffect(() => { + if (emailRequired) { + form.mutators.setFieldTouched('email', true); + } + }, [emailRequired]); return ( <> diff --git a/webclient/src/hooks/index.ts b/webclient/src/hooks/index.ts index a9385d50b..7c8e7c3ef 100644 --- a/webclient/src/hooks/index.ts +++ b/webclient/src/hooks/index.ts @@ -1,6 +1,5 @@ export * from './useAutoConnect'; export * from './useFireOnce'; -export * from './useDebounce'; export * from './useLocaleSort'; export * from './useReduxEffect'; export * from './useWebClient'; diff --git a/webclient/src/hooks/useAutoConnect.spec.ts b/webclient/src/hooks/useAutoConnect.spec.ts new file mode 100644 index 000000000..d6b09d0e8 --- /dev/null +++ b/webclient/src/hooks/useAutoConnect.spec.ts @@ -0,0 +1,86 @@ +import { renderHook, act, waitFor } from '@testing-library/react'; +import { useAutoConnect } from './useAutoConnect'; + +const mockSave = vi.fn(); + +let storedSetting: any = null; + +vi.mock('@app/services', () => ({ + SettingDTO: class MockSettingDTO { + user: string; + autoConnect = false; + constructor(user: string) { + this.user = user; + } + save = mockSave; + static get = vi.fn(() => Promise.resolve(storedSetting)); + }, +})); + +vi.mock('@app/types', () => ({ + App: { APP_USER: '*app' }, +})); + +describe('useAutoConnect', () => { + beforeEach(() => { + storedSetting = null; + mockSave.mockClear(); + }); + + test('returns undefined initially, then resolves to stored autoConnect value', async () => { + storedSetting = { user: '*app', autoConnect: true, save: mockSave }; + + const { result } = renderHook(() => useAutoConnect()); + + expect(result.current[0]).toBeUndefined(); + + await waitFor(() => { + expect(result.current[0]).toBe(true); + }); + }); + + test('creates and saves a new SettingDTO when none exists', async () => { + storedSetting = null; + + const { result } = renderHook(() => useAutoConnect()); + + await waitFor(() => { + expect(result.current[0]).toBe(false); + }); + + // save() called once for the newly created setting + expect(mockSave).toHaveBeenCalledTimes(1); + }); + + test('persists when setAutoConnect is called', async () => { + storedSetting = { user: '*app', autoConnect: false, save: mockSave }; + + const { result } = renderHook(() => useAutoConnect()); + + await waitFor(() => { + expect(result.current[0]).toBe(false); + }); + + mockSave.mockClear(); + + act(() => { + result.current[1](true); + }); + + await waitFor(() => { + expect(result.current[0]).toBe(true); + }); + + expect(mockSave).toHaveBeenCalled(); + }); + + test('does not save redundantly on initial mount when setting exists', async () => { + storedSetting = { user: '*app', autoConnect: true, save: mockSave }; + + renderHook(() => useAutoConnect()); + + await waitFor(() => { + expect(mockSave).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/webclient/src/hooks/useAutoConnect.ts b/webclient/src/hooks/useAutoConnect.ts index f8daa78aa..a24a0b80f 100644 --- a/webclient/src/hooks/useAutoConnect.ts +++ b/webclient/src/hooks/useAutoConnect.ts @@ -1,35 +1,33 @@ -import { useEffect, useState } from 'react'; +import { Dispatch, SetStateAction, useEffect, useRef, useState } from 'react'; import { SettingDTO } from '@app/services'; import { App } from '@app/types'; -export function useAutoConnect() { - const [setting, setSetting] = useState(undefined); - const [autoConnect, setAutoConnect] = useState(undefined); +export function useAutoConnect(): [boolean | undefined, Dispatch>] { + const [setting, setSetting] = useState(undefined); + const [autoConnect, setAutoConnect] = useState(undefined); + const prevAutoConnectRef = useRef(undefined); useEffect(() => { - SettingDTO.get(App.APP_USER).then((setting: SettingDTO) => { - if (!setting) { - setting = new SettingDTO(App.APP_USER); - setting.save(); + SettingDTO.get(App.APP_USER).then((loaded: SettingDTO) => { + if (!loaded) { + loaded = new SettingDTO(App.APP_USER); + loaded.save(); } - setSetting(setting); + setSetting(loaded); + setAutoConnect(loaded.autoConnect); + prevAutoConnectRef.current = loaded.autoConnect; }); }, []); useEffect(() => { - if (setting) { - setAutoConnect(setting.autoConnect); - } - }, [setting]); - - useEffect(() => { - if (setting) { + if (setting && autoConnect !== prevAutoConnectRef.current) { + prevAutoConnectRef.current = autoConnect; setting.autoConnect = autoConnect; setting.save(); } - }, [setting, autoConnect]); + }, [autoConnect]); return [autoConnect, setAutoConnect]; } diff --git a/webclient/src/hooks/useDebounce.ts b/webclient/src/hooks/useDebounce.ts deleted file mode 100644 index f8b62e565..000000000 --- a/webclient/src/hooks/useDebounce.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { useCallback } from 'react'; - -type UseDebounceType = (...args: any) => any; -const DEBOUNCE_DELAY = 250; - -export interface DebouncedFn { - (...args: Parameters): void; - cancel(): void; -} - -function debounce(fn: T, timeout: number): DebouncedFn { - let timer: ReturnType | undefined; - const debounced = ((...args: Parameters): void => { - if (timer !== undefined) { - clearTimeout(timer); - } - timer = setTimeout(() => fn(...args), timeout); - }) as DebouncedFn; - debounced.cancel = (): void => { - if (timer !== undefined) { - clearTimeout(timer); - timer = undefined; - } - }; - return debounced; -} - -export function useDebounce( - fn: T, - deps: any[] = [], - timeout: number = DEBOUNCE_DELAY -): DebouncedFn { - return useCallback(debounce(fn, timeout), deps); -} diff --git a/webclient/src/hooks/useFireOnce/useFireOnce.spec.tsx b/webclient/src/hooks/useFireOnce/useFireOnce.spec.tsx index 9c87ccf64..501bb9403 100644 --- a/webclient/src/hooks/useFireOnce/useFireOnce.spec.tsx +++ b/webclient/src/hooks/useFireOnce/useFireOnce.spec.tsx @@ -2,6 +2,8 @@ import { render, fireEvent, waitFor, + renderHook, + act, } from '@testing-library/react'; import { useFireOnce } from './useFireOnce'; @@ -98,4 +100,56 @@ describe('useFireOnce hook', () => { { timeout: 100 } ); }); + + test('resetInFlightStatus re-enables firing', () => { + const fn = vi.fn(); + const { result } = renderHook(() => useFireOnce(fn)); + + act(() => { + result.current[2](); + }); + + expect(result.current[0]).toBe(true); + expect(fn).toHaveBeenCalledTimes(1); + + act(() => { + result.current[1](); + }); + + expect(result.current[0]).toBe(false); + + act(() => { + result.current[2](); + }); + + expect(fn).toHaveBeenCalledTimes(2); + }); + + test('calls the latest fn when parent updates it', () => { + const fn1 = vi.fn(); + const fn2 = vi.fn(); + const { result, rerender } = renderHook(({ fn }) => useFireOnce(fn), { + initialProps: { fn: fn1 }, + }); + + rerender({ fn: fn2 }); + + act(() => { + result.current[2](); + }); + + expect(fn1).not.toHaveBeenCalled(); + expect(fn2).toHaveBeenCalledTimes(1); + }); + + test('passes all arguments through to fn', () => { + const fn = vi.fn(); + const { result } = renderHook(() => useFireOnce(fn)); + + act(() => { + result.current[2]('a', 'b', 'c'); + }); + + expect(fn).toHaveBeenCalledWith('a', 'b', 'c'); + }); }); diff --git a/webclient/src/hooks/useFireOnce/useFireOnce.ts b/webclient/src/hooks/useFireOnce/useFireOnce.ts index 54c184160..e4f5265de 100644 --- a/webclient/src/hooks/useFireOnce/useFireOnce.ts +++ b/webclient/src/hooks/useFireOnce/useFireOnce.ts @@ -1,15 +1,20 @@ -import { useCallback, useState } from 'react'; +import { useCallback, useRef, useState } from 'react'; type UseFireOnceType = (...args: any) => any; -export function useFireOnce(fn: T): [boolean, any, any] { - const [actionIsInFlight, setActionIsInFlight] = useState(false) - const handleFireOnce = useCallback((args) => { +export function useFireOnce(fn: T): [boolean, () => void, (...args: Parameters) => void] { + const [actionIsInFlight, setActionIsInFlight] = useState(false); + const fnRef = useRef(fn); + fnRef.current = fn; + + const handleFireOnce = useCallback((...args: Parameters) => { setActionIsInFlight(true); - fn(args); - }, []) - function resetInFlightStatus() { + fnRef.current(...args); + }, []); + + const resetInFlightStatus = useCallback(() => { setActionIsInFlight(false); - } - return [actionIsInFlight, resetInFlightStatus, handleFireOnce] + }, []); + + return [actionIsInFlight, resetInFlightStatus, handleFireOnce]; } diff --git a/webclient/src/hooks/useLocaleSort.spec.ts b/webclient/src/hooks/useLocaleSort.spec.ts new file mode 100644 index 000000000..f13bdea46 --- /dev/null +++ b/webclient/src/hooks/useLocaleSort.spec.ts @@ -0,0 +1,80 @@ +import { renderHook } from '@testing-library/react'; +import { useLocaleSort } from './useLocaleSort'; + +let mockLanguage = 'en'; + +vi.mock('react-i18next', () => ({ + useTranslation: () => ({ + i18n: { + get language() { + return mockLanguage; + }, + }, + }), +})); + +describe('useLocaleSort', () => { + beforeEach(() => { + mockLanguage = 'en'; + }); + + test('sorts strings by locale using the valueGetter', () => { + const arr = ['c', 'a', 'b']; + const { result } = renderHook(() => useLocaleSort(arr, (v) => v)); + + expect(result.current).toEqual(['a', 'b', 'c']); + }); + + test('sorts using valueGetter to resolve display values', () => { + const lookup: Record = { x: 'cherry', y: 'apple', z: 'banana' }; + const arr = ['x', 'y', 'z']; + const { result } = renderHook(() => useLocaleSort(arr, (v) => lookup[v])); + + expect(result.current).toEqual(['y', 'z', 'x']); + }); + + test('handles empty array', () => { + const { result } = renderHook(() => useLocaleSort([], (v) => v)); + + expect(result.current).toEqual([]); + }); + + test('does not mutate the input array', () => { + const arr = ['c', 'a', 'b']; + renderHook(() => useLocaleSort(arr, (v) => v)); + + expect(arr).toEqual(['c', 'a', 'b']); + }); + + test('updates when arr prop changes', () => { + let arr = ['c', 'a']; + const getter = (v: string) => v; + const { result, rerender } = renderHook(() => useLocaleSort(arr, getter)); + + expect(result.current).toEqual(['a', 'c']); + + arr = ['z', 'b', 'a']; + rerender(); + + expect(result.current).toEqual(['a', 'b', 'z']); + }); + + test('re-sorts when language changes', () => { + // Swedish sorts ä after z; English sorts ä near a + const arr = ['ä', 'b', 'z']; + const getter = (v: string) => v; + + mockLanguage = 'en'; + const { result, rerender } = renderHook(() => useLocaleSort(arr, getter)); + const englishOrder = [...result.current]; + + mockLanguage = 'sv'; + rerender(); + const swedishOrder = [...result.current]; + + // In Swedish, ä comes after z + expect(swedishOrder[swedishOrder.length - 1]).toBe('ä'); + // In English, ä sorts near 'a', before 'b' + expect(englishOrder.indexOf('ä')).toBeLessThan(englishOrder.indexOf('b')); + }); +}); diff --git a/webclient/src/hooks/useLocaleSort.ts b/webclient/src/hooks/useLocaleSort.ts index 219292ed7..0b463a34f 100644 --- a/webclient/src/hooks/useLocaleSort.ts +++ b/webclient/src/hooks/useLocaleSort.ts @@ -1,18 +1,11 @@ -import { useEffect, useState } from 'react'; +import { useMemo } from 'react'; import { useTranslation } from 'react-i18next'; -export function useLocaleSort(arr: string[], valueGetter: (value: string) => string) { - const [state] = useState(arr); - const [sorted, setSorted] = useState([]); - +export function useLocaleSort(arr: string[], valueGetter: (value: string) => string): string[] { const { i18n } = useTranslation(); - useEffect(() => { + return useMemo(() => { const collator = new Intl.Collator(i18n.language); - const sorter = (a, b) => collator.compare(valueGetter(a), valueGetter(b)); - - setSorted(state.sort(sorter)); - }, [state, i18n.language]); - - return sorted; + return [...arr].sort((a, b) => collator.compare(valueGetter(a), valueGetter(b))); + }, [arr, i18n.language, valueGetter]); } diff --git a/webclient/src/hooks/useReduxEffect.spec.tsx b/webclient/src/hooks/useReduxEffect.spec.tsx new file mode 100644 index 000000000..8b71d63ea --- /dev/null +++ b/webclient/src/hooks/useReduxEffect.spec.tsx @@ -0,0 +1,138 @@ +import { renderHook, act } from '@testing-library/react'; +import { configureStore, combineReducers } from '@reduxjs/toolkit'; +import { Provider } from 'react-redux'; +import { StrictMode, ReactNode } from 'react'; +import { useReduxEffect } from './useReduxEffect'; +import { actionReducer } from '../store/actions/actionReducer'; + +function makeStore() { + return configureStore({ + reducer: combineReducers({ action: actionReducer }), + }); +} + +function makeWrapper(store: ReturnType) { + return function Wrapper({ children }: { children: ReactNode }) { + return {children}; + }; +} + +describe('useReduxEffect', () => { + test('fires callback when matching action type is dispatched', () => { + const store = makeStore(); + const effect = vi.fn(); + + renderHook(() => useReduxEffect(effect, 'TEST_ACTION'), { + wrapper: makeWrapper(store), + }); + + act(() => { + store.dispatch({ type: 'TEST_ACTION', payload: 'hello' }); + }); + + expect(effect).toHaveBeenCalledTimes(1); + expect(effect).toHaveBeenCalledWith( + expect.objectContaining({ type: 'TEST_ACTION' }), + ); + }); + + test('does not fire for non-matching action types', () => { + const store = makeStore(); + const effect = vi.fn(); + + renderHook(() => useReduxEffect(effect, 'LISTEN_FOR'), { + wrapper: makeWrapper(store), + }); + + act(() => { + store.dispatch({ type: 'OTHER_ACTION' }); + }); + + expect(effect).not.toHaveBeenCalled(); + }); + + test('handles array of action types', () => { + const store = makeStore(); + const effect = vi.fn(); + + renderHook(() => useReduxEffect(effect, ['TYPE_A', 'TYPE_B']), { + wrapper: makeWrapper(store), + }); + + act(() => { + store.dispatch({ type: 'TYPE_A' }); + }); + act(() => { + store.dispatch({ type: 'TYPE_B' }); + }); + act(() => { + store.dispatch({ type: 'TYPE_C' }); + }); + + expect(effect).toHaveBeenCalledTimes(2); + }); + + test('does not double-fire in StrictMode', () => { + const store = makeStore(); + const effect = vi.fn(); + + function StrictWrapper({ children }: { children: ReactNode }) { + return ( + + {children} + + ); + } + + renderHook(() => useReduxEffect(effect, 'TEST'), { + wrapper: StrictWrapper, + }); + + act(() => { + store.dispatch({ type: 'TEST' }); + }); + + expect(effect).toHaveBeenCalledTimes(1); + }); + + test('catches action dispatched before mount via sync check', () => { + const store = makeStore(); + const effect = vi.fn(); + + // Dispatch before the hook mounts + store.dispatch({ type: 'EARLY_ACTION' }); + + renderHook(() => useReduxEffect(effect, 'EARLY_ACTION'), { + wrapper: makeWrapper(store), + }); + + expect(effect).toHaveBeenCalledTimes(1); + expect(effect).toHaveBeenCalledWith( + expect.objectContaining({ type: 'EARLY_ACTION' }), + ); + }); + + test('uses latest effect callback via ref', () => { + const store = makeStore(); + const effect1 = vi.fn(); + const effect2 = vi.fn(); + + const { rerender } = renderHook( + ({ cb }) => useReduxEffect(cb, 'TEST'), + { + wrapper: makeWrapper(store), + initialProps: { cb: effect1 }, + }, + ); + + rerender({ cb: effect2 }); + + act(() => { + store.dispatch({ type: 'TEST' }); + }); + + expect(effect1).not.toHaveBeenCalled(); + expect(effect2).toHaveBeenCalledTimes(1); + }); + +}); diff --git a/webclient/src/hooks/useWebClient.spec.tsx b/webclient/src/hooks/useWebClient.spec.tsx new file mode 100644 index 000000000..629de8714 --- /dev/null +++ b/webclient/src/hooks/useWebClient.spec.tsx @@ -0,0 +1,48 @@ +import { renderHook } from '@testing-library/react'; +import { ReactNode } from 'react'; +import { WebClientProvider, useWebClient } from './useWebClient'; + +vi.mock('@app/websocket', () => ({ + WebClient: class MockWebClient {}, +})); + +vi.mock('@app/api', () => ({ + createWebClientRequest: vi.fn(() => 'request'), + createWebClientResponse: vi.fn(() => 'response'), +})); + +function Wrapper({ children }: { children: ReactNode }) { + return {children}; +} + +describe('useWebClient', () => { + test('provides the WebClient instance to children', () => { + const { result } = renderHook(() => useWebClient(), { wrapper: Wrapper }); + + expect(result.current).toBeDefined(); + expect(result.current.constructor.name).toBe('MockWebClient'); + }); + + test('throws when used outside WebClientProvider', () => { + // Suppress React error boundary console output + const spy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + expect(() => { + renderHook(() => useWebClient()); + }).toThrow('useWebClient must be used within a WebClientProvider'); + + spy.mockRestore(); + }); + + test('returns the same instance across re-renders', () => { + const { result, rerender } = renderHook(() => useWebClient(), { + wrapper: Wrapper, + }); + + const first = result.current; + rerender(); + const second = result.current; + + expect(first).toBe(second); + }); +}); diff --git a/webclient/src/services/CardImporterService.spec.ts b/webclient/src/services/CardImporterService.spec.ts new file mode 100644 index 000000000..4880230ab --- /dev/null +++ b/webclient/src/services/CardImporterService.spec.ts @@ -0,0 +1,261 @@ +import { cardImporterService } from './CardImporterService'; + +const mockFetch = vi.fn(); +globalThis.fetch = mockFetch; + +function jsonResponse(body: unknown, contentType = 'application/json') { + return { + ok: true, + headers: new Headers({ 'Content-Type': contentType }), + json: () => Promise.resolve(body), + } as unknown as Response; +} + +function textResponse(body: string, ok = true) { + return { + ok, + headers: new Headers({ 'Content-Type': 'application/xml' }), + text: () => Promise.resolve(body), + } as unknown as Response; +} + +function failedResponse(status = 500) { + return { + ok: false, + status, + headers: new Headers({ 'Content-Type': 'application/json' }), + json: () => Promise.resolve({}), + text: () => Promise.resolve(''), + } as unknown as Response; +} + +// Minimal MTGJSON-shaped fixture +const mtgjsonFixture = { + data: { + SET_B: { + code: 'SET_B', + name: 'Set B', + releaseDate: '2020-06-01', + cards: [ + { name: 'Zebra' }, + { name: 'Alpha' }, + ], + tokens: [{ name: 'Token B' }], + }, + SET_A: { + code: 'SET_A', + name: 'Set A', + releaseDate: '2019-01-01', + cards: [ + { name: 'Alpha' }, + { name: 'Beta' }, + ], + tokens: [{ name: 'Token A' }], + }, + }, +}; + +describe('CardImporterService', () => { + describe('importCards', () => { + it('fetches and parses valid MTGJSON data into sorted cards and sets', async () => { + mockFetch.mockResolvedValue(jsonResponse(mtgjsonFixture)); + + const { cards, sets } = await cardImporterService.importCards('http://example.com/cards.json'); + + expect(cards).toHaveLength(3); + expect(cards[0].name).toBe('Alpha'); + expect(cards[1].name).toBe('Beta'); + expect(cards[2].name).toBe('Zebra'); + + expect(sets).toHaveLength(2); + expect(sets[0].name).toBe('Set A'); + expect(sets[1].name).toBe('Set B'); + }); + + it('sorts sets by releaseDate ascending', async () => { + mockFetch.mockResolvedValue(jsonResponse(mtgjsonFixture)); + + const { sets } = await cardImporterService.importCards('http://example.com/cards.json'); + + expect(sets[0].code).toBe('SET_A'); + expect(sets[1].code).toBe('SET_B'); + }); + + it('deduplicates cards by name, keeping last occurrence (later set wins)', async () => { + mockFetch.mockResolvedValue(jsonResponse(mtgjsonFixture)); + + const { cards } = await cardImporterService.importCards('http://example.com/cards.json'); + + // Alpha appears in both SET_A and SET_B; SET_B is later so its version overwrites + // 3 unique names: Alpha (deduped), Beta (SET_A only), Zebra (SET_B only) + expect(cards).toHaveLength(3); + expect(cards.map(c => c.name)).toEqual(['Alpha', 'Beta', 'Zebra']); + }); + + it('maps set cards and tokens to name arrays', async () => { + mockFetch.mockResolvedValue(jsonResponse(mtgjsonFixture)); + + const { sets } = await cardImporterService.importCards('http://example.com/cards.json'); + + expect(sets[0].cards).toEqual(['Alpha', 'Beta']); + expect(sets[0].tokens).toEqual(['Token A']); + }); + + it('rejects when response is not ok', async () => { + mockFetch.mockResolvedValue(failedResponse(404)); + + await expect(cardImporterService.importCards('http://example.com/cards.json')) + .rejects.toThrow('Card import must be in valid MTG JSON format'); + }); + + it('rejects when Content-Type does not contain application/json', async () => { + mockFetch.mockResolvedValue({ + ok: true, + headers: new Headers({ 'Content-Type': 'text/html' }), + json: () => Promise.resolve({}), + } as unknown as Response); + + await expect(cardImporterService.importCards('http://example.com/cards.json')) + .rejects.toThrow('Card import must be in valid MTG JSON format'); + }); + + it('accepts Content-Type with charset parameter', async () => { + mockFetch.mockResolvedValue(jsonResponse(mtgjsonFixture, 'application/json; charset=utf-8')); + + const { cards } = await cardImporterService.importCards('http://example.com/cards.json'); + expect(cards.length).toBeGreaterThan(0); + }); + + it('rejects when JSON structure is invalid (missing data key)', async () => { + mockFetch.mockResolvedValue(jsonResponse({ notData: {} })); + + await expect(cardImporterService.importCards('http://example.com/cards.json')) + .rejects.toThrow('Card import must be in valid MTG JSON format'); + }); + + it('preserves the original error as cause', async () => { + mockFetch.mockResolvedValue(jsonResponse({ notData: {} })); + + try { + await cardImporterService.importCards('http://example.com/cards.json'); + expect.fail('should have thrown'); + } catch (err) { + expect((err as Error).cause).toBeDefined(); + } + }); + }); + + describe('importTokens', () => { + const validXml = ` + + + + + + +`; + + it('fetches and parses valid XML into token objects', async () => { + mockFetch.mockResolvedValue(textResponse(validXml)); + + const tokens = await cardImporterService.importTokens('http://example.com/tokens.xml'); + + expect(tokens).toHaveLength(1); + expect(tokens[0]).toHaveProperty('name'); + }); + + it('parses token attributes correctly', async () => { + mockFetch.mockResolvedValue(textResponse(validXml)); + + const tokens = await cardImporterService.importTokens('http://example.com/tokens.xml'); + + const token = tokens[0] as Record; + expect(token.name.value).toBe('Soldier'); + expect(token.set.value).toBe('M21'); + expect(token.set.picURL).toBe('http://example.com/soldier.png'); + }); + + it('rejects when response is not ok', async () => { + mockFetch.mockResolvedValue({ ok: false, text: () => Promise.resolve('') } as unknown as Response); + + await expect(cardImporterService.importTokens('http://example.com/tokens.xml')) + .rejects.toThrow('Failed to fetch'); + }); + + it('rejects when XML is malformed', async () => { + mockFetch.mockResolvedValue(textResponse('')); + + await expect(cardImporterService.importTokens('http://example.com/tokens.xml')) + .rejects.toThrow('Token import must be in valid MTG XML format'); + }); + + it('returns empty array when XML has no card elements', async () => { + const emptyXml = ''; + mockFetch.mockResolvedValue(textResponse(emptyXml)); + + const tokens = await cardImporterService.importTokens('http://example.com/tokens.xml'); + expect(tokens).toEqual([]); + }); + + it('preserves the original error as cause on parse failure', async () => { + mockFetch.mockResolvedValue(textResponse('')); + + try { + await cardImporterService.importTokens('http://example.com/tokens.xml'); + expect.fail('should have thrown'); + } catch (err) { + expect((err as Error).cause).toBeDefined(); + } + }); + }); + + describe('parseXmlAttributes', () => { + function parseXml(xml: string) { + const dom = new DOMParser().parseFromString(xml, 'application/xml'); + return (cardImporterService as any).parseXmlAttributes(dom.documentElement); + } + + it('parses simple child elements into key-value pairs', () => { + const result = parseXml(''); + expect(result.name.value).toBe('Soldier'); + }); + + it('parses nested elements recursively', () => { + const result = parseXml(''); + expect(result.prop.value).toHaveProperty('cmc'); + expect(result.prop.value.cmc.value).toBe('2'); + }); + + it('includes XML attributes alongside value', () => { + const result = parseXml(''); + expect(result.set.value).toBe('M21'); + expect(result.set.picURL).toBe('http://img.png'); + }); + + it('converts duplicate tag names into an array preserving all values', () => { + const result = parseXml( + '' + ); + expect(Array.isArray(result.related)).toBe(true); + expect(result.related).toHaveLength(2); + expect(result.related[0].value).toBe('Token A'); + expect(result.related[1].value).toBe('Token B'); + }); + + it('appends to existing array for 3+ duplicate tag names', () => { + const result = parseXml( + '' + ); + expect(Array.isArray(result.set)).toBe(true); + expect(result.set).toHaveLength(3); + expect(result.set[0].value).toBe('A'); + expect(result.set[1].value).toBe('B'); + expect(result.set[2].value).toBe('C'); + }); + + it('reads innerHTML as value for leaf elements without children', () => { + const result = parseXml('Some card text'); + expect(result.text.value).toBe('Some card text'); + }); + }); +}); diff --git a/webclient/src/services/CardImporterService.ts b/webclient/src/services/CardImporterService.ts index 27014b57c..984de9005 100644 --- a/webclient/src/services/CardImporterService.ts +++ b/webclient/src/services/CardImporterService.ts @@ -1,12 +1,19 @@ // Fetch and parse card sets +import { App } from '@app/types'; + class CardImporterService { - importCards(url): Promise { + importCards(url: string): Promise<{ cards: App.Card[]; sets: App.Set[] }> { const error = 'Card import must be in valid MTG JSON format'; return fetch(url) .then(response => { - if (response.headers.get('Content-Type') !== 'application/json') { + if (!response.ok) { + throw new Error(error); + } + + const contentType = response.headers.get('Content-Type') ?? ''; + if (!contentType.includes('application/json')) { throw new Error(error); } @@ -34,13 +41,13 @@ class CardImporterService { .map(key => unsortedCards[key]); return { cards, sets }; - } catch { - throw new Error(error); + } catch (err) { + throw new Error(error, { cause: err }); } }); } - importTokens(url): Promise { + importTokens(url: string): Promise[]> { const error = 'Token import must be in valid MTG XML format'; return fetch(url) @@ -56,13 +63,17 @@ class CardImporterService { const parser = new DOMParser(); const dom = parser.parseFromString(xmlString, 'application/xml'); + if (dom.querySelector('parsererror')) { + throw new Error(error); + } + const tokens = Array.from(dom.querySelectorAll('card')).map( (tokenElement) => this.parseXmlAttributes(tokenElement) ); return tokens; - } catch { - throw new Error(error); + } catch (err) { + throw new Error(error, { cause: err }); } }) } @@ -90,7 +101,7 @@ class CardImporterService { if (Array.isArray(attributes[child.tagName])) { attributes[child.tagName].push(parsedAttributes) } else { - attributes[child.tagName] = [parsedAttributes]; + attributes[child.tagName] = [attributes[child.tagName], parsedAttributes]; } } else { attributes[child.tagName] = parsedAttributes; diff --git a/webclient/src/services/HostService.ts b/webclient/src/services/HostService.ts new file mode 100644 index 000000000..c17c2a3cf --- /dev/null +++ b/webclient/src/services/HostService.ts @@ -0,0 +1,47 @@ +import { App } from '@app/types'; + +export const DefaultHosts: App.Host[] = [ + { + name: 'Chickatrice', + host: 'mtg.chickatrice.net', + port: '443', + localPort: '4748', + editable: false, + }, + { + name: 'Rooster', + host: 'server.cockatrice.us/servatrice', + port: '4748', + localHost: 'server.cockatrice.us', + editable: false, + }, + { + name: 'Rooster Beta', + host: 'beta.cockatrice.us/servatrice', + port: '4748', + localHost: 'beta.cockatrice.us', + editable: false, + }, + { + name: 'Tetrarch', + host: 'mtg.tetrarch.co/servatrice', + port: '443', + editable: false, + }, +]; + +export const getHostPort = (host: App.Host): { host: string, port: string } => { + const isLocal = window.location.hostname === 'localhost'; + + if (!host) { + return { + host: '', + port: '' + }; + } + + return { + host: !isLocal ? host.host : host.localHost || host.host, + port: !isLocal ? host.port : host.localPort || host.port, + }; +}; diff --git a/webclient/src/services/dexie/DexieDTOs/CardDTO.ts b/webclient/src/services/dexie/DexieDTOs/CardDTO.ts index dd43681ea..863c5107a 100644 --- a/webclient/src/services/dexie/DexieDTOs/CardDTO.ts +++ b/webclient/src/services/dexie/DexieDTOs/CardDTO.ts @@ -1,3 +1,4 @@ +import { IndexableType } from 'dexie'; import { App } from '@app/types'; import { dexieService } from '../DexieService'; @@ -7,11 +8,11 @@ export class CardDTO extends App.Card { return dexieService.cards.put(this); } - static get(name) { + static get(name: string) { return dexieService.cards.where('name').equalsIgnoreCase(name).first(); } - static bulkAdd(cards: CardDTO[]): Promise { + static bulkAdd(cards: CardDTO[]): Promise { return dexieService.cards.bulkPut(cards); } }; diff --git a/webclient/src/services/dexie/DexieDTOs/SetDTO.ts b/webclient/src/services/dexie/DexieDTOs/SetDTO.ts index 3bd02903c..ac90e19b7 100644 --- a/webclient/src/services/dexie/DexieDTOs/SetDTO.ts +++ b/webclient/src/services/dexie/DexieDTOs/SetDTO.ts @@ -1,3 +1,4 @@ +import { IndexableType } from 'dexie'; import { App } from '@app/types'; import { dexieService } from '../DexieService'; @@ -7,11 +8,11 @@ export class SetDTO extends App.Set { return dexieService.sets.put(this); } - static get(name) { + static get(name: string) { return dexieService.sets.where('name').equalsIgnoreCase(name).first(); } - static bulkAdd(sets: SetDTO[]): Promise { + static bulkAdd(sets: SetDTO[]): Promise { return dexieService.sets.bulkPut(sets); } }; diff --git a/webclient/src/services/dexie/DexieDTOs/SettingDTO.ts b/webclient/src/services/dexie/DexieDTOs/SettingDTO.ts index 357ebc5a7..451d34819 100644 --- a/webclient/src/services/dexie/DexieDTOs/SettingDTO.ts +++ b/webclient/src/services/dexie/DexieDTOs/SettingDTO.ts @@ -3,7 +3,7 @@ import { App } from '@app/types'; import { dexieService } from '../DexieService'; export class SettingDTO extends App.Setting { - constructor(user) { + constructor(user: string) { super(); this.user = user; @@ -14,7 +14,7 @@ export class SettingDTO extends App.Setting { return dexieService.settings.put(this); } - static get(user) { + static get(user: string) { return dexieService.settings.where('user').equalsIgnoreCase(user).first(); } }; diff --git a/webclient/src/services/dexie/DexieDTOs/TokenDTO.ts b/webclient/src/services/dexie/DexieDTOs/TokenDTO.ts index 04199e63d..1321fc437 100644 --- a/webclient/src/services/dexie/DexieDTOs/TokenDTO.ts +++ b/webclient/src/services/dexie/DexieDTOs/TokenDTO.ts @@ -1,3 +1,4 @@ +import { IndexableType } from 'dexie'; import { App } from '@app/types'; import { dexieService } from '../DexieService'; @@ -7,11 +8,11 @@ export class TokenDTO extends App.Token { return dexieService.tokens.put(this); } - static get(name) { + static get(name: string) { return dexieService.tokens.where('name.value').equalsIgnoreCase(name).first(); } - static bulkAdd(tokens: TokenDTO[]): Promise { + static bulkAdd(tokens: TokenDTO[]): Promise { return dexieService.tokens.bulkPut(tokens); } }; diff --git a/webclient/src/services/dexie/DexieSchemas/v1.schema.ts b/webclient/src/services/dexie/DexieSchemas/v1.schema.ts index 18e7f35c4..32c29d2ed 100644 --- a/webclient/src/services/dexie/DexieSchemas/v1.schema.ts +++ b/webclient/src/services/dexie/DexieSchemas/v1.schema.ts @@ -1,3 +1,5 @@ +import Dexie from 'dexie'; + export enum Stores { SETTINGS = 'settings', CARDS = 'cards', @@ -6,7 +8,7 @@ export enum Stores { HOSTS = 'hosts', } -export const schemaV1 = (db) => { +export const schemaV1 = (db: Dexie) => { db.version(1).stores({ [Stores.CARDS]: 'name', [Stores.SETS]: 'code', diff --git a/webclient/src/services/index.ts b/webclient/src/services/index.ts index ebebeb65e..2d36a2628 100644 --- a/webclient/src/services/index.ts +++ b/webclient/src/services/index.ts @@ -1,3 +1,4 @@ export * from './CardImporterService'; +export * from './HostService'; export * from './ServerProps'; export * from './dexie'; diff --git a/webclient/src/setupTests.ts b/webclient/src/setupTests.ts index 5c422f120..219544389 100644 --- a/webclient/src/setupTests.ts +++ b/webclient/src/setupTests.ts @@ -5,6 +5,43 @@ import './polyfills'; // Ensure jest-dom matchers are available in every test file. import '@testing-library/jest-dom/vitest'; +// jsdom doesn't provide ResizeObserver; react-window needs it. +if (typeof globalThis.ResizeObserver === 'undefined') { + globalThis.ResizeObserver = class { + observe() {} + unobserve() {} + disconnect() {} + } as any; +} + +// Mock Dexie globally to prevent IndexedDB initialization in jsdom. +// Dexie eagerly opens IndexedDB on import, and jsdom's fake-indexeddb +// is memory-intensive. No UI test needs a real database. +vi.mock('dexie', () => { + const fakeTable = { + mapToClass: () => {}, + get: () => Promise.resolve(null), + put: () => Promise.resolve(), + add: () => Promise.resolve(1), + bulkAdd: () => Promise.resolve(), + delete: () => Promise.resolve(), + toArray: () => Promise.resolve([]), + where: () => ({ equals: () => ({ first: () => Promise.resolve(null) }) }), + }; + class FakeDexie { + version() { + return { stores: () => this }; + } + open() { + return Promise.resolve(this); + } + table() { + return fakeTable; + } + } + return { default: FakeDexie, __esModule: true }; +}); + // ── Global mock hygiene under `isolate: false` ──────────────────────────────── // // Vitest is configured with `test.isolate: false` for speed — every spec file diff --git a/webclient/src/store/actions/actionReducer.spec.ts b/webclient/src/store/actions/actionReducer.spec.ts index 351995dc5..5808d8b53 100644 --- a/webclient/src/store/actions/actionReducer.spec.ts +++ b/webclient/src/store/actions/actionReducer.spec.ts @@ -1,9 +1,8 @@ import { actionReducer } from './actionReducer'; describe('actionReducer', () => { - it('spreads the init action onto state and starts count at 1', () => { + it('stores the init action type and starts count at 1', () => { const result = actionReducer(undefined, { type: '@@INIT' }); - // actionReducer always spreads the action, so type reflects the dispatched action expect(result.type).toBe('@@INIT'); expect(result.payload).toBeNull(); expect(result.meta).toBeNull(); @@ -11,7 +10,7 @@ describe('actionReducer', () => { expect(result.count).toBe(1); }); - it('spreads action onto state and increments count', () => { + it('stores action type and cloned payload', () => { const result = actionReducer(undefined, { type: 'MY_ACTION', payload: { id: 1 } }); expect(result.type).toBe('MY_ACTION'); expect(result.payload).toEqual({ id: 1 }); @@ -25,16 +24,33 @@ describe('actionReducer', () => { expect(state3.count).toBe(3); }); - it('preserves existing state fields not overridden by action', () => { + it('replaces type from previous action', () => { const initial = actionReducer(undefined, { type: 'FIRST', payload: 'original' }); const result = actionReducer(initial, { type: 'SECOND' }); expect(result.type).toBe('SECOND'); + expect(result.payload).toBeNull(); expect(result.count).toBe(2); }); - it('spreads action.meta and action.error from action onto state', () => { + it('stores meta and error from action', () => { const result = actionReducer(undefined, { type: 'ERR', meta: { source: 'api' }, error: true }); expect(result.meta).toEqual({ source: 'api' }); expect(result.error).toBe(true); }); + + it('deep-clones payload to prevent shared references', () => { + const nested = { data: { counterInfo: { id: 1, count: 20 } } }; + const result = actionReducer(undefined, { type: 'TEST', payload: nested }); + expect(result.payload).toEqual(nested); + expect(result.payload).not.toBe(nested); + expect((result.payload as any).data).not.toBe(nested.data); + expect((result.payload as any).data.counterInfo).not.toBe(nested.data.counterInfo); + }); + + it('deep-clones meta to prevent shared references', () => { + const meta = { source: { nested: true } }; + const result = actionReducer(undefined, { type: 'TEST', meta }); + expect(result.meta).toEqual(meta); + expect(result.meta).not.toBe(meta); + }); }); diff --git a/webclient/src/store/actions/actionReducer.ts b/webclient/src/store/actions/actionReducer.ts index b3e883ee1..03e32d1d8 100644 --- a/webclient/src/store/actions/actionReducer.ts +++ b/webclient/src/store/actions/actionReducer.ts @@ -25,19 +25,21 @@ const initialState: InitialState = { } /** - * Calculates the application state. + * Stores the most recent action so `useReduxEffect` can react to dispatches. * - * @param state - * @param action - * @return {*} + * Payloads are deep-cloned to prevent shared object references between this + * slice and the slice that owns the action. Without the clone, Immer mutations + * in the target slice are detected as mutations of the stale payload stored here. */ export const actionReducer = ( state = initialState, action: UnknownAction, ): InitialState => { return { - ...state, - ...action, + type: action.type ?? null, + payload: 'payload' in action ? structuredClone(action.payload) : null, + meta: 'meta' in action ? structuredClone(action.meta) : null, + error: !!action.error, count: state.count + 1, } } diff --git a/webclient/src/store/common/normalizers.spec.ts b/webclient/src/store/common/normalizers.spec.ts index a6bd6d757..a4195ed2b 100644 --- a/webclient/src/store/common/normalizers.spec.ts +++ b/webclient/src/store/common/normalizers.spec.ts @@ -77,11 +77,23 @@ describe('normalizeLogs', () => { const result = normalizeLogs(logs); expect(result.room).toHaveLength(2); expect(result.game).toHaveLength(1); - expect(result.chat).toBeUndefined(); + expect(result.chat).toEqual([]); }); - it('returns empty object for empty logs', () => { - expect(normalizeLogs([])).toEqual({}); + it('returns all three keys as empty arrays for empty logs', () => { + expect(normalizeLogs([])).toEqual({ room: [], game: [], chat: [] }); + }); + + it('skips logs whose targetType is not one of the known buckets', () => { + const logs = [ + create(Data.ServerInfo_ChatMessageSchema, { targetType: 'room' }), + create(Data.ServerInfo_ChatMessageSchema, { targetType: '' }), + create(Data.ServerInfo_ChatMessageSchema, { targetType: 'unknown' }), + ]; + const result = normalizeLogs(logs); + expect(result.room).toHaveLength(1); + expect(result.game).toEqual([]); + expect(result.chat).toEqual([]); }); }); diff --git a/webclient/src/store/common/normalizers.ts b/webclient/src/store/common/normalizers.ts index 545c3d8b9..70ec4881d 100644 --- a/webclient/src/store/common/normalizers.ts +++ b/webclient/src/store/common/normalizers.ts @@ -50,12 +50,13 @@ export function normalizeGameObject(game: Data.ServerInfo_Game, gametypeMap: Enr /** Group a flat LogItem[] into { room, game, chat } buckets for the server store. */ export function normalizeLogs(logs: Data.ServerInfo_ChatMessage[]): Enriched.LogGroups { - return logs.reduce((obj, log) => { + return logs.reduce((obj, log) => { const type = log.targetType as keyof Enriched.LogGroups; - obj[type] = obj[type] || []; - obj[type]!.push(log); + if (obj[type]) { + obj[type].push(log); + } return obj; - }, {} as Enriched.LogGroups); + }, { room: [], game: [], chat: [] }); } /** diff --git a/webclient/src/store/game/game.dispatch.spec.ts b/webclient/src/store/game/game.dispatch.spec.ts index d7cab22fb..bf6d91254 100644 --- a/webclient/src/store/game/game.dispatch.spec.ts +++ b/webclient/src/store/game/game.dispatch.spec.ts @@ -197,7 +197,7 @@ describe('Dispatch', () => { }); it('gameSay dispatches Actions.gameSay()', () => { - Dispatch.gameSay(1, 2, 'gg wp'); - expect(mockDispatch).toHaveBeenCalledWith(Actions.gameSay({ gameId: 1, playerId: 2, message: 'gg wp' })); + Dispatch.gameSay(1, 2, 'gg wp', 1700000000000); + expect(mockDispatch).toHaveBeenCalledWith(Actions.gameSay({ gameId: 1, playerId: 2, message: 'gg wp', timeReceived: 1700000000000 })); }); }); diff --git a/webclient/src/store/game/game.dispatch.ts b/webclient/src/store/game/game.dispatch.ts index c84a559e8..294f4831a 100644 --- a/webclient/src/store/game/game.dispatch.ts +++ b/webclient/src/store/game/game.dispatch.ts @@ -127,7 +127,7 @@ export const Dispatch = { store.dispatch(Actions.zonePropertiesChanged({ gameId, playerId, data })); }, - gameSay: (gameId: number, playerId: number, message: string) => { - store.dispatch(Actions.gameSay({ gameId, playerId, message })); + gameSay: (gameId: number, playerId: number, message: string, timeReceived: number) => { + store.dispatch(Actions.gameSay({ gameId, playerId, message, timeReceived })); }, }; diff --git a/webclient/src/store/game/game.reducer.spec.ts b/webclient/src/store/game/game.reducer.spec.ts index 3d4b8abb8..4eadf7238 100644 --- a/webclient/src/store/game/game.reducer.spec.ts +++ b/webclient/src/store/game/game.reducer.spec.ts @@ -417,6 +417,69 @@ describe('2C: CARD_MOVED', () => { expect(cardsIn(result, 1, 1, 'hand')).toHaveLength(1); expect(result.games[1].players[1].zones['nonexistent']).toBeUndefined(); }); + + it('CARD_MOVED → no-ops when neither cardId nor position resolve and newCardId < 0', () => { + const state = makeState({ + games: { + 1: makeGameEntry({ + players: { + 1: makePlayerEntry({ + zones: { + deck: makeZoneEntry({ name: 'deck', cards: [], cardCount: 5 }), + hand: makeZoneEntry({ name: 'hand', cards: [], cardCount: 0 }), + }, + }), + }, + }), + }, + }); + + const result = gamesReducer(state, Actions.cardMoved({ + gameId: 1, + playerId: 1, + data: { + cardId: -1, cardName: '', startPlayerId: 1, startZone: 'deck', + position: -1, targetPlayerId: 1, targetZone: 'hand', + x: 0, y: 0, newCardId: -1, faceDown: false, newCardProviderId: '', + }, + })); + + expect(result.games[1].players[1].zones['deck'].cardCount).toBe(5); + expect(cardsIn(result, 1, 1, 'hand')).toHaveLength(0); + }); + + it('CARD_MOVED → deep-clones counterList so moved card is independent', () => { + const cardCounter = create(Data.ServerInfo_CardCounterSchema, { id: 1, value: 3 }); + const card = makeCard({ id: 10, counterList: [cardCounter] }); + const state = makeState({ + games: { + 1: makeGameEntry({ + players: { + 1: makePlayerEntry({ + zones: { + hand: makeZoneEntry({ name: 'hand', cards: [card], cardCount: 1 }), + table: makeZoneEntry({ name: 'table', cardCount: 0 }), + }, + }), + }, + }), + }, + }); + + const result = gamesReducer(state, Actions.cardMoved({ + gameId: 1, + playerId: 1, + data: { + cardId: 10, cardName: '', startPlayerId: 1, startZone: 'hand', + position: -1, targetPlayerId: 1, targetZone: 'table', + x: 0, y: 0, newCardId: -1, faceDown: false, newCardProviderId: '', + }, + })); + + const movedCard = cardsIn(result, 1, 1, 'table')[0]; + expect(movedCard.counterList).toHaveLength(1); + expect(movedCard.counterList).not.toBe(card.counterList); + }); }); // ── 2D: Card mutations ──────────────────────────────────────────────────────── @@ -696,6 +759,17 @@ describe('2H: Player counters', () => { expect(result.games[1].players[1].counters[5]).toEqual(counter); }); + it('COUNTER_CREATED → clones counterInfo to prevent shared references', () => { + const state = makeState(); + const counter = makeCounter({ id: 5, name: 'Life', count: 20 }); + const result = gamesReducer(state, Actions.counterCreated({ + gameId: 1, + playerId: 1, + data: { counterInfo: counter }, + })); + expect(result.games[1].players[1].counters[5]).not.toBe(counter); + }); + it('COUNTER_SET → updates counter.count to new value', () => { const counter = makeCounter({ id: 5, count: 20 }); const state = makeState({ @@ -845,6 +919,34 @@ describe('2I: Zone operations', () => { expect(cardsIn(result, 1, 1, 'deck')[1]).toEqual(newCard); }); + it('CARDS_REVEALED → clones counterList to prevent shared references', () => { + const cardCounter = create(Data.ServerInfo_CardCounterSchema, { id: 1, value: 5 }); + const revealedCard = makeCard({ id: 3, counterList: [cardCounter] }); + const state = makeState({ + games: { + 1: makeGameEntry({ + players: { + 1: makePlayerEntry({ + zones: { + deck: makeZoneEntry({ name: 'deck', cards: [], cardCount: 0 }), + }, + }), + }, + }), + }, + }); + + const result = gamesReducer(state, Actions.cardsRevealed({ + gameId: 1, + playerId: 1, + data: { zoneName: 'deck', cards: [revealedCard] }, + })); + + const stored = cardsIn(result, 1, 1, 'deck')[0]; + expect(stored.counterList).toEqual(revealedCard.counterList); + expect(stored.counterList).not.toBe(revealedCard.counterList); + }); + it('ZONE_PROPERTIES_CHANGED → sets alwaysRevealTopCard and alwaysLookAtTopCard', () => { const state = makeState(); const result = gamesReducer(state, Actions.zonePropertiesChanged({ @@ -882,21 +984,17 @@ describe('2J: Turn, phase, and chat', () => { expect(result.games[1].reversed).toBe(true); }); - it('GAME_SAY → appends message with mocked Date.now() as timeReceived', () => { + it('GAME_SAY → appends message with timeReceived from payload', () => { const state = makeState(); - const dateNowSpy = vi.spyOn(Date, 'now').mockReturnValue(123456789); - try { - const result = gamesReducer(state, Actions.gameSay({ - gameId: 1, - playerId: 2, - message: 'gg', - })); + const result = gamesReducer(state, Actions.gameSay({ + gameId: 1, + playerId: 2, + message: 'gg', + timeReceived: 123456789, + })); - expect(result.games[1].messages).toHaveLength(1); - expect(result.games[1].messages[0]).toEqual({ playerId: 2, message: 'gg', timeReceived: 123456789 }); - } finally { - dateNowSpy.mockRestore(); - } + expect(result.games[1].messages).toHaveLength(1); + expect(result.games[1].messages[0]).toEqual({ playerId: 2, message: 'gg', timeReceived: 123456789 }); }); }); @@ -1332,6 +1430,6 @@ describe('2L: Null-guard / missing entity early-returns', () => { it('GAME_SAY with unknown gameId → state unchanged', () => { const state = makeState(); - expect(gamesReducer(state, Actions.gameSay({ gameId: UNKNOWN_GAME, playerId: 1, message: 'hi' }))).toBe(state); + expect(gamesReducer(state, Actions.gameSay({ gameId: UNKNOWN_GAME, playerId: 1, message: 'hi', timeReceived: 0 }))).toBe(state); }); }); diff --git a/webclient/src/store/game/game.reducer.ts b/webclient/src/store/game/game.reducer.ts index eff362c32..ec3c1f658 100644 --- a/webclient/src/store/game/game.reducer.ts +++ b/webclient/src/store/game/game.reducer.ts @@ -183,7 +183,7 @@ export const gamesSlice = createSlice({ state, action: PayloadAction<{ gameId: number; playerId: number; data: Data.Event_MoveCard }>, ) => { - const { gameId, playerId, data } = action.payload; + const { gameId, data } = action.payload; const { cardId, cardName, startPlayerId, startZone, position, targetPlayerId, targetZone, x, y, newCardId, faceDown, newCardProviderId, @@ -194,8 +194,7 @@ export const gamesSlice = createSlice({ return; } - const effectiveStartPlayerId = startPlayerId >= 0 ? startPlayerId : playerId; - const sourcePlayer = game.players[effectiveStartPlayerId]; + const sourcePlayer = game.players[startPlayerId]; const sourceZone = sourcePlayer?.zones[startZone]; if (!sourcePlayer || !sourceZone) { return; @@ -214,10 +213,16 @@ export const gamesSlice = createSlice({ resolvedCardId = sourceZone.order[position]; } - const removedCard: Data.ServerInfo_Card | undefined = - resolvedCardId >= 0 ? sourceZone.byId[resolvedCardId] : undefined; + // If the card can't be resolved and no newCardId is provided, the event + // is malformed — bail out to avoid creating phantom cards with id -1. + if (resolvedCardId < 0 && newCardId < 0) { + return; + } + // Remove from source zone if the card was resolved to a known entry + let removedCard: Data.ServerInfo_Card | undefined; if (resolvedCardId >= 0) { + removedCard = sourceZone.byId[resolvedCardId]; const idx = sourceZone.order.indexOf(resolvedCardId); if (idx >= 0) { sourceZone.order.splice(idx, 1); @@ -226,11 +231,12 @@ export const gamesSlice = createSlice({ } sourceZone.cardCount = Math.max(0, sourceZone.cardCount - 1); - const effectiveNewId = newCardId >= 0 ? newCardId : (removedCard?.id ?? -1); + const effectiveNewId = newCardId >= 0 ? newCardId : (removedCard?.id ?? resolvedCardId); const movedCard: Data.ServerInfo_Card = removedCard ? { ...removedCard, id: effectiveNewId, name: cardName || removedCard.name, x, y, faceDown, providerId: newCardProviderId || removedCard.providerId, + counterList: [...removedCard.counterList], } : buildEmptyCard(effectiveNewId, cardName, x, y, faceDown, newCardProviderId ?? ''); @@ -342,8 +348,8 @@ export const gamesSlice = createSlice({ arrowCreated: (state, action: PayloadAction<{ gameId: number; playerId: number; data: Data.Event_CreateArrow }>) => { const { gameId, playerId, data } = action.payload; const player = state.games[gameId]?.players[playerId]; - if (player) { - player.arrows[data.arrowInfo.id] = data.arrowInfo; + if (player && data.arrowInfo) { + player.arrows[data.arrowInfo.id] = { ...data.arrowInfo }; } }, @@ -360,8 +366,8 @@ export const gamesSlice = createSlice({ counterCreated: (state, action: PayloadAction<{ gameId: number; playerId: number; data: Data.Event_CreateCounter }>) => { const { gameId, playerId, data } = action.payload; const player = state.games[gameId]?.players[playerId]; - if (player) { - player.counters[data.counterInfo.id] = data.counterInfo; + if (player && data.counterInfo) { + player.counters[data.counterInfo.id] = { ...data.counterInfo }; } }, @@ -417,12 +423,10 @@ export const gamesSlice = createSlice({ } for (const revealedCard of cards) { - if (zone.byId[revealedCard.id]) { - Object.assign(zone.byId[revealedCard.id], revealedCard); - } else { + if (!zone.byId[revealedCard.id]) { zone.order.push(revealedCard.id); - zone.byId[revealedCard.id] = revealedCard; } + zone.byId[revealedCard.id] = { ...revealedCard, counterList: [...revealedCard.counterList] }; } }, @@ -465,8 +469,8 @@ export const gamesSlice = createSlice({ // ── Chat ────────────────────────────────────────────────────────────────── - gameSay: (state, action: PayloadAction<{ gameId: number; playerId: number; message: string }>) => { - const { gameId, playerId, message } = action.payload; + gameSay: (state, action: PayloadAction<{ gameId: number; playerId: number; message: string; timeReceived: number }>) => { + const { gameId, playerId, message, timeReceived } = action.payload; const game = state.games[gameId]; if (!game) { return; @@ -474,7 +478,7 @@ export const gamesSlice = createSlice({ if (game.messages.length >= MAX_GAME_MESSAGES) { game.messages = game.messages.slice(game.messages.length - MAX_GAME_MESSAGES + 1); } - game.messages.push({ playerId, message, timeReceived: Date.now() }); + game.messages.push({ playerId, message, timeReceived }); }, // ── Log-only events ───────────────────────────────────────────────────── diff --git a/webclient/src/store/rooms/rooms.actions.spec.ts b/webclient/src/store/rooms/rooms.actions.spec.ts index ba75c5f2a..59a806160 100644 --- a/webclient/src/store/rooms/rooms.actions.spec.ts +++ b/webclient/src/store/rooms/rooms.actions.spec.ts @@ -42,9 +42,10 @@ describe('Actions', () => { }); it('sortGames', () => { - expect(Actions.sortGames({ field: App.GameSortField.START_TIME, order: App.SortDirection.ASC })).toEqual({ + expect(Actions.sortGames({ roomId: 1, field: App.GameSortField.START_TIME, order: App.SortDirection.ASC })).toEqual({ type: Types.SORT_GAMES, payload: { + roomId: 1, field: App.GameSortField.START_TIME, order: App.SortDirection.ASC, }, diff --git a/webclient/src/store/rooms/rooms.dispatch.spec.ts b/webclient/src/store/rooms/rooms.dispatch.spec.ts index 0cec338f3..bcbfa1977 100644 --- a/webclient/src/store/rooms/rooms.dispatch.spec.ts +++ b/webclient/src/store/rooms/rooms.dispatch.spec.ts @@ -74,7 +74,7 @@ describe('Dispatch', () => { it('sortGames dispatches Actions.sortGames()', () => { Dispatch.sortGames(1, App.GameSortField.START_TIME, App.SortDirection.ASC); expect(mockDispatch).toHaveBeenCalledWith( - Actions.sortGames({ field: App.GameSortField.START_TIME, order: App.SortDirection.ASC }) + Actions.sortGames({ roomId: 1, field: App.GameSortField.START_TIME, order: App.SortDirection.ASC }) ); }); diff --git a/webclient/src/store/rooms/rooms.dispatch.tsx b/webclient/src/store/rooms/rooms.dispatch.tsx index a6eb8dd03..224eff0d9 100644 --- a/webclient/src/store/rooms/rooms.dispatch.tsx +++ b/webclient/src/store/rooms/rooms.dispatch.tsx @@ -37,7 +37,7 @@ export const Dispatch = { }, sortGames: (roomId: number, field: App.GameSortField, order: App.SortDirection) => { - store.dispatch(Actions.sortGames({ field, order })); + store.dispatch(Actions.sortGames({ roomId, field, order })); }, removeMessages: (roomId: number, name: string, amount: number) => { diff --git a/webclient/src/store/rooms/rooms.reducer.spec.ts b/webclient/src/store/rooms/rooms.reducer.spec.ts index ea09fefc7..ce5b687dd 100644 --- a/webclient/src/store/rooms/rooms.reducer.spec.ts +++ b/webclient/src/store/rooms/rooms.reducer.spec.ts @@ -243,6 +243,7 @@ describe('SORT_GAMES', () => { it('updates sortGamesBy on state (sorting itself is now derived in selectors)', () => { const state = makeRoomsState({ rooms: {} }); const result = roomsReducer(state, Actions.sortGames({ + roomId: 1, field: App.GameSortField.START_TIME, order: App.SortDirection.ASC, })); diff --git a/webclient/src/store/rooms/rooms.reducer.tsx b/webclient/src/store/rooms/rooms.reducer.tsx index efd2c916d..e6157952e 100644 --- a/webclient/src/store/rooms/rooms.reducer.tsx +++ b/webclient/src/store/rooms/rooms.reducer.tsx @@ -82,14 +82,14 @@ export const roomsSlice = createSlice({ addMessage: (state, action: PayloadAction<{ roomId: number; message: Enriched.Message }>) => { const { roomId, message } = action.payload; - const existing = state.messages[roomId] ?? []; - const normalized = normalizeUserMessage(message); - const next = - existing.length >= MAX_ROOM_MESSAGES - ? [...existing.slice(existing.length - MAX_ROOM_MESSAGES + 1), normalized] - : [...existing, normalized]; - - state.messages[roomId] = next; + if (!state.messages[roomId]) { + state.messages[roomId] = []; + } + const msgs = state.messages[roomId]; + if (msgs.length >= MAX_ROOM_MESSAGES) { + state.messages[roomId] = msgs.slice(msgs.length - MAX_ROOM_MESSAGES + 1); + } + state.messages[roomId].push(normalizeUserMessage(message)); }, updateGames: (state, action: PayloadAction<{ roomId: number; games: Data.ServerInfo_Game[] }>) => { @@ -145,8 +145,9 @@ export const roomsSlice = createSlice({ delete room.users[name]; }, - sortGames: (state, action: PayloadAction<{ field: App.GameSortField; order: App.SortDirection }>) => { - // Sort is now derived in selectors; the reducer only stores the sort config. + sortGames: (state, action: PayloadAction<{ roomId: number; field: App.GameSortField; order: App.SortDirection }>) => { + // Sort is derived in selectors; the reducer stores the sort config. + // roomId is passed through for future per-room sorting support. const { field, order } = action.payload; state.sortGamesBy = { field, order }; }, diff --git a/webclient/src/store/server/__mocks__/server-fixtures.ts b/webclient/src/store/server/__mocks__/server-fixtures.ts index be47e93e2..fd805dbbb 100644 --- a/webclient/src/store/server/__mocks__/server-fixtures.ts +++ b/webclient/src/store/server/__mocks__/server-fixtures.ts @@ -143,7 +143,7 @@ export function makeServerState(overrides: Partial = {}): ServerSta ignoreList: {}, status: { connectionAttemptMade: false, - state: App.StatusEnum.DISCONNECTED, + state: Enriched.StatusEnum.DISCONNECTED, description: null, }, info: { diff --git a/webclient/src/store/server/server.actions.spec.ts b/webclient/src/store/server/server.actions.spec.ts index 3638201b7..5bc31dbd7 100644 --- a/webclient/src/store/server/server.actions.spec.ts +++ b/webclient/src/store/server/server.actions.spec.ts @@ -1,5 +1,5 @@ import { Actions } from './server.actions'; -import { App, Data } from '@app/types'; +import { Data, Enriched } from '@app/types'; import { Types } from './server.types'; import { create } from '@bufbuild/protobuf'; import { @@ -88,7 +88,7 @@ describe('Actions', () => { }); it('updateStatus', () => { - const status = { state: App.StatusEnum.CONNECTED, description: 'connected' }; + const status = { state: Enriched.StatusEnum.CONNECTED, description: 'connected' }; expect(Actions.updateStatus({ status })).toEqual({ type: Types.UPDATE_STATUS, payload: { status } }); }); diff --git a/webclient/src/store/server/server.dispatch.spec.ts b/webclient/src/store/server/server.dispatch.spec.ts index 3d89c6330..bd7804850 100644 --- a/webclient/src/store/server/server.dispatch.spec.ts +++ b/webclient/src/store/server/server.dispatch.spec.ts @@ -6,7 +6,7 @@ vi.mock('..', () => ({ store: { dispatch: mockDispatch } })); import { Actions } from './server.actions'; import { Dispatch } from './server.dispatch'; -import { App, Data } from '@app/types'; +import { Data, Enriched } from '@app/types'; import { create } from '@bufbuild/protobuf'; import { makeBanHistoryItem, @@ -106,8 +106,10 @@ describe('Dispatch', () => { }); it('updateStatus dispatches Actions.updateStatus({ status: { state, description } })', () => { - Dispatch.updateStatus(App.StatusEnum.CONNECTED, 'ok'); - expect(mockDispatch).toHaveBeenCalledWith(Actions.updateStatus({ status: { state: App.StatusEnum.CONNECTED, description: 'ok' } })); + Dispatch.updateStatus(Enriched.StatusEnum.CONNECTED, 'ok'); + expect(mockDispatch).toHaveBeenCalledWith( + Actions.updateStatus({ status: { state: Enriched.StatusEnum.CONNECTED, description: 'ok' } }) + ); }); it('updateUser dispatches Actions.updateUser()', () => { diff --git a/webclient/src/store/server/server.dispatch.ts b/webclient/src/store/server/server.dispatch.ts index 77a2845df..fad027a57 100644 --- a/webclient/src/store/server/server.dispatch.ts +++ b/webclient/src/store/server/server.dispatch.ts @@ -1,6 +1,6 @@ import { Actions } from './server.actions'; import { store } from '..'; -import { App, Data, Enriched } from '@app/types'; +import { Data, Enriched } from '@app/types'; export const Dispatch = { initialized: () => { @@ -48,7 +48,7 @@ export const Dispatch = { updateInfo: (name: string, version: string) => { store.dispatch(Actions.updateInfo({ info: { name, version } })); }, - updateStatus: (state: App.StatusEnum, description: string) => { + updateStatus: (state: Enriched.StatusEnum, description: string) => { store.dispatch(Actions.updateStatus({ status: { state, description } })); }, updateUser: (user: Data.ServerInfo_User) => { diff --git a/webclient/src/store/server/server.interfaces.ts b/webclient/src/store/server/server.interfaces.ts index 2a4e1ca5a..eb7cd4e69 100644 --- a/webclient/src/store/server/server.interfaces.ts +++ b/webclient/src/store/server/server.interfaces.ts @@ -43,7 +43,7 @@ export interface ServerState { export interface ServerStateStatus { connectionAttemptMade: boolean; description: string | null; - state: App.StatusEnum; + state: Enriched.StatusEnum; } export interface ServerStateInfo { diff --git a/webclient/src/store/server/server.reducer.spec.ts b/webclient/src/store/server/server.reducer.spec.ts index 99ac79e97..c0edac090 100644 --- a/webclient/src/store/server/server.reducer.spec.ts +++ b/webclient/src/store/server/server.reducer.spec.ts @@ -1,6 +1,6 @@ -import { App, Data } from '@app/types'; +import { Data, Enriched } from '@app/types'; import { create } from '@bufbuild/protobuf'; -import { serverReducer } from './server.reducer'; +import { serverReducer, MAX_USER_MESSAGES } from './server.reducer'; import { Actions } from './server.actions'; import { makeBanHistoryItem, @@ -25,7 +25,7 @@ describe('Initialisation', () => { const result = serverReducer(undefined, { type: '@@INIT' }); expect(result.initialized).toBe(false); expect(result.buddyList).toEqual({}); - expect(result.status.state).toBe(App.StatusEnum.DISCONNECTED); + expect(result.status.state).toBe(Enriched.StatusEnum.DISCONNECTED); }); it('INITIALIZED → resets to initialState with initialized: true', () => { @@ -37,7 +37,7 @@ describe('Initialisation', () => { }); it('CLEAR_STORE → resets to initialState but preserves status', () => { - const status = { state: App.StatusEnum.LOGGED_IN, description: 'logged in', connectionAttemptMade: true }; + const status = { state: Enriched.StatusEnum.LOGGED_IN, description: 'logged in', connectionAttemptMade: true }; const state = makeServerState({ status, banUser: 'someone' }); const result = serverReducer(state, Actions.clearStore()); expect(result.banUser).toBe(''); @@ -56,7 +56,7 @@ describe('Initialisation', () => { describe('Account & Connection', () => { it('CONNECTION_ATTEMPTED → sets connectionAttemptMade to true', () => { - const state = makeServerState({ status: { connectionAttemptMade: false, state: App.StatusEnum.DISCONNECTED, description: null } }); + const state = makeServerState({ status: { connectionAttemptMade: false, state: Enriched.StatusEnum.DISCONNECTED, description: null } }); const result = serverReducer(state, Actions.connectionAttempted()); expect(result.status.connectionAttemptMade).toBe(true); }); @@ -131,9 +131,9 @@ describe('Server Info & Status', () => { it('UPDATE_STATUS → merges state and description into status', () => { const state = makeServerState(); - const update = { state: App.StatusEnum.LOGGED_IN, description: 'ok' }; + const update = { state: Enriched.StatusEnum.LOGGED_IN, description: 'ok' }; const result = serverReducer(state, Actions.updateStatus({ status: update })); - expect(result.status.state).toBe(App.StatusEnum.LOGGED_IN); + expect(result.status.state).toBe(Enriched.StatusEnum.LOGGED_IN); expect(result.status.description).toBe('ok'); expect(result.status.connectionAttemptMade).toBe(false); }); @@ -256,6 +256,14 @@ describe('Logs', () => { expect(result.logs.room).toEqual([log]); }); + it('VIEW_LOGS with empty array → produces all three keys as empty arrays', () => { + const state = makeServerState(); + const result = serverReducer(state, Actions.viewLogs({ logs: [] })); + expect(result.logs.room).toEqual([]); + expect(result.logs.game).toEqual([]); + expect(result.logs.chat).toEqual([]); + }); + it('CLEAR_LOGS → resets logs to empty arrays', () => { const state = makeServerState({ logs: { room: [makeLogItem()], game: [], chat: [] } }); const result = serverReducer(state, Actions.clearLogs()); @@ -284,6 +292,13 @@ describe('Messaging', () => { expect(result.messages['Alice'][0]).toEqual(messageData); }); + it('USER_MESSAGE → no-ops when user is null (not yet logged in)', () => { + const state = makeServerState({ user: null, messages: {} }); + const messageData = { senderName: 'Alice', receiverName: 'Bob', message: 'hi' } as Data.Event_UserMessage; + const result = serverReducer(state, Actions.userMessage({ messageData })); + expect(result.messages).toEqual({}); + }); + it('USER_MESSAGE → appends to existing messages for that user', () => { const existingMsg = create(Data.Event_UserMessageSchema, { senderName: 'Alice', receiverName: 'Bob', message: 'first' }); const state = makeServerState({ @@ -294,6 +309,21 @@ describe('Messaging', () => { const result = serverReducer(state, Actions.userMessage({ messageData: newMsg })); expect(result.messages['Alice']).toHaveLength(2); }); + + it(`USER_MESSAGE → caps messages at MAX_USER_MESSAGES (${MAX_USER_MESSAGES})`, () => { + const messages = Array.from({ length: MAX_USER_MESSAGES }, (_, i) => + create(Data.Event_UserMessageSchema, { senderName: 'Alice', receiverName: 'Bob', message: `msg-${i}` }) + ); + const state = makeServerState({ + user: makeUser({ name: 'Bob' }), + messages: { Alice: messages }, + }); + const newMsg = create(Data.Event_UserMessageSchema, { senderName: 'Alice', receiverName: 'Bob', message: 'overflow' }); + const result = serverReducer(state, Actions.userMessage({ messageData: newMsg })); + expect(result.messages['Alice']).toHaveLength(MAX_USER_MESSAGES); + expect(result.messages['Alice'][MAX_USER_MESSAGES - 1]).toEqual(newMsg); + expect(result.messages['Alice'][0].message).not.toBe('msg-0'); + }); }); // ── User Info & Notifications ───────────────────────────────────────────────── diff --git a/webclient/src/store/server/server.reducer.ts b/webclient/src/store/server/server.reducer.ts index a7e7682fa..9e3033961 100644 --- a/webclient/src/store/server/server.reducer.ts +++ b/webclient/src/store/server/server.reducer.ts @@ -6,6 +6,8 @@ import { normalizeBannedUserError, normalizeGameObject, normalizeGametypeMap, no import { ServerState, ServerStateStatus } from './server.interfaces'; +export const MAX_USER_MESSAGES = 1000; + function splitPath(path: string): string[] { return path ? path.split('/') : []; } @@ -71,7 +73,7 @@ const initialState: ServerState = { status: { connectionAttemptMade: false, - state: App.StatusEnum.DISCONNECTED, + state: Enriched.StatusEnum.DISCONNECTED, description: null }, info: { @@ -172,17 +174,20 @@ export const serverSlice = createSlice({ updateStatus: (state, action: PayloadAction<{ status: Pick }>) => { const { status } = action.payload; - state.status = { ...state.status, ...status }; + state.status.state = status.state; + state.status.description = status.description; - if (status.state === App.StatusEnum.DISCONNECTED) { + if (status.state === Enriched.StatusEnum.DISCONNECTED) { state.status.connectionAttemptMade = false; } }, updateUser: (state, action: PayloadAction<{ user: Partial }>) => { - state.user = state.user - ? { ...state.user, ...action.payload.user } as Data.ServerInfo_User - : action.payload.user as Data.ServerInfo_User; + if (state.user) { + Object.assign(state.user, action.payload.user); + } else { + state.user = action.payload.user as Data.ServerInfo_User; + } }, updateUsers: (state, action: PayloadAction<{ users: Data.ServerInfo_User[] }>) => { @@ -203,7 +208,7 @@ export const serverSlice = createSlice({ }, viewLogs: (state, action: PayloadAction<{ logs: Data.ServerInfo_ChatMessage[] }>) => { - state.logs = { ...normalizeLogs(action.payload.logs) }; + state.logs = normalizeLogs(action.payload.logs); }, clearLogs: (state) => { @@ -211,11 +216,18 @@ export const serverSlice = createSlice({ }, userMessage: (state, action: PayloadAction<{ messageData: Data.Event_UserMessage }>) => { + if (!state.user) { + return; + } const { senderName, receiverName } = action.payload.messageData; - const userName = state.user!.name === senderName ? receiverName : senderName; + const userName = state.user.name === senderName ? receiverName : senderName; if (!state.messages[userName]) { state.messages[userName] = []; } + const msgs = state.messages[userName]; + if (msgs.length >= MAX_USER_MESSAGES) { + state.messages[userName] = msgs.slice(msgs.length - MAX_USER_MESSAGES + 1); + } state.messages[userName].push(action.payload.messageData); }, @@ -273,7 +285,7 @@ export const serverSlice = createSlice({ newLevel = shouldBeJudge ? (newLevel | Data.ServerInfo_User_UserLevelFlag.IsJudge) : (newLevel & ~Data.ServerInfo_User_UserLevelFlag.IsJudge); - state.users[userName] = { ...user, userLevel: newLevel }; + user.userLevel = newLevel; }, replayList: (state, action: PayloadAction<{ matchList: Data.ServerInfo_ReplayMatch[] }>) => { @@ -295,7 +307,7 @@ export const serverSlice = createSlice({ if (!existing) { return; } - state.replays[gameId] = { ...existing, doNotHide }; + existing.doNotHide = doNotHide; }, replayDeleteMatch: (state, action: PayloadAction<{ gameId: number }>) => { @@ -379,39 +391,43 @@ export const serverSlice = createSlice({ }, accountEditChanged: (state, action: PayloadAction<{ user: Partial }>) => { - state.user = { ...state.user, ...action.payload.user } as Data.ServerInfo_User; + if (state.user) { + Object.assign(state.user, action.payload.user); + } }, accountImageChanged: (state, action: PayloadAction<{ user: Partial }>) => { - state.user = { ...state.user, ...action.payload.user } as Data.ServerInfo_User; + if (state.user) { + Object.assign(state.user, action.payload.user); + } }, // Signal-only action types — no state mutation, defined so type strings are generated - accountAwaitingActivation: (_state, _action: PayloadAction) => {}, - accountActivationFailed: (_state, _action: PayloadAction) => {}, - accountActivationSuccess: (_state, _action: PayloadAction) => {}, - loginSuccessful: (_state, _action: PayloadAction) => {}, - loginFailed: (_state, _action: PayloadAction) => {}, - connectionFailed: (_state, _action: PayloadAction) => {}, - testConnectionSuccessful: (_state, _action: PayloadAction) => {}, - testConnectionFailed: (_state, _action: PayloadAction) => {}, - registrationRequiresEmail: (_state, _action: PayloadAction) => {}, - registrationSuccess: (_state, _action: PayloadAction) => {}, - registrationEmailError: (_state, _action: PayloadAction) => {}, - registrationPasswordError: (_state, _action: PayloadAction) => {}, - registrationUserNameError: (_state, _action: PayloadAction) => {}, - resetPassword: (_state, _action: PayloadAction) => {}, - resetPasswordFailed: (_state, _action: PayloadAction) => {}, - resetPasswordChallenge: (_state, _action: PayloadAction) => {}, - resetPasswordSuccess: (_state, _action: PayloadAction) => {}, - reloadConfig: (_state, _action: PayloadAction) => {}, - shutdownServer: (_state, _action: PayloadAction) => {}, - updateServerMessage: (_state, _action: PayloadAction) => {}, - accountPasswordChange: (_state, _action: PayloadAction) => {}, - addToList: (_state, _action: PayloadAction) => {}, - removeFromList: (_state, _action: PayloadAction) => {}, - grantReplayAccess: (_state, _action: PayloadAction) => {}, - forceActivateUser: (_state, _action: PayloadAction) => {}, + accountAwaitingActivation: (_state, _action: PayloadAction<{ options: Enriched.PendingActivationContext }>) => {}, + accountActivationFailed: (_state) => {}, + accountActivationSuccess: (_state) => {}, + loginSuccessful: (_state, _action: PayloadAction<{ options: Enriched.LoginSuccessContext }>) => {}, + loginFailed: (_state) => {}, + connectionFailed: (_state) => {}, + testConnectionSuccessful: (_state) => {}, + testConnectionFailed: (_state) => {}, + registrationRequiresEmail: (_state) => {}, + registrationSuccess: (_state) => {}, + registrationEmailError: (_state, _action: PayloadAction<{ error: string }>) => {}, + registrationPasswordError: (_state, _action: PayloadAction<{ error: string }>) => {}, + registrationUserNameError: (_state, _action: PayloadAction<{ error: string }>) => {}, + resetPassword: (_state) => {}, + resetPasswordFailed: (_state) => {}, + resetPasswordChallenge: (_state) => {}, + resetPasswordSuccess: (_state) => {}, + reloadConfig: (_state) => {}, + shutdownServer: (_state) => {}, + updateServerMessage: (_state) => {}, + accountPasswordChange: (_state) => {}, + addToList: (_state, _action: PayloadAction<{ list: string; userName: string }>) => {}, + removeFromList: (_state, _action: PayloadAction<{ list: string; userName: string }>) => {}, + grantReplayAccess: (_state, _action: PayloadAction<{ replayId: number; moderatorName: string }>) => {}, + forceActivateUser: (_state, _action: PayloadAction<{ usernameToActivate: string; moderatorName: string }>) => {}, }, }); diff --git a/webclient/src/store/server/server.selectors.spec.ts b/webclient/src/store/server/server.selectors.spec.ts index 8d9a75097..44286f950 100644 --- a/webclient/src/store/server/server.selectors.spec.ts +++ b/webclient/src/store/server/server.selectors.spec.ts @@ -6,7 +6,7 @@ import { makeServerState, makeUser, } from './__mocks__/server-fixtures'; -import { App, Data } from '@app/types'; +import { Data, Enriched } from '@app/types'; function rootState(server: ServerState) { return { server }; @@ -34,17 +34,17 @@ describe('Selectors', () => { }); it('getDescription → returns status.description', () => { - const state = makeServerState({ status: { connectionAttemptMade: false, state: App.StatusEnum.CONNECTED, description: 'ok' } }); + const state = makeServerState({ status: { connectionAttemptMade: false, state: Enriched.StatusEnum.CONNECTED, description: 'ok' } }); expect(Selectors.getDescription(rootState(state))).toBe('ok'); }); it('getState → returns status.state', () => { - const state = makeServerState({ status: { connectionAttemptMade: false, state: App.StatusEnum.LOGGED_IN, description: null } }); - expect(Selectors.getState(rootState(state))).toBe(App.StatusEnum.LOGGED_IN); + const state = makeServerState({ status: { connectionAttemptMade: false, state: Enriched.StatusEnum.LOGGED_IN, description: null } }); + expect(Selectors.getState(rootState(state))).toBe(Enriched.StatusEnum.LOGGED_IN); }); it('getConnectionAttemptMade → returns status.connectionAttemptMade', () => { - const state = makeServerState({ status: { connectionAttemptMade: true, state: App.StatusEnum.DISCONNECTED, description: null } }); + const state = makeServerState({ status: { connectionAttemptMade: true, state: Enriched.StatusEnum.DISCONNECTED, description: null } }); expect(Selectors.getConnectionAttemptMade(rootState(state))).toBe(true); }); @@ -153,17 +153,17 @@ describe('Selectors', () => { // ── derived selectors (createSelector) ────────────────────────────── it('getIsConnected → true when state is LOGGED_IN', () => { - const state = makeServerState({ status: { connectionAttemptMade: true, state: App.StatusEnum.LOGGED_IN, description: null } }); + const state = makeServerState({ status: { connectionAttemptMade: true, state: Enriched.StatusEnum.LOGGED_IN, description: null } }); expect(Selectors.getIsConnected(rootState(state))).toBe(true); }); it('getIsConnected → false when state is CONNECTED', () => { - const state = makeServerState({ status: { connectionAttemptMade: true, state: App.StatusEnum.CONNECTED, description: null } }); + const state = makeServerState({ status: { connectionAttemptMade: true, state: Enriched.StatusEnum.CONNECTED, description: null } }); expect(Selectors.getIsConnected(rootState(state))).toBe(false); }); it('getIsConnected → false when state is DISCONNECTED', () => { - const state = makeServerState({ status: { connectionAttemptMade: false, state: App.StatusEnum.DISCONNECTED, description: null } }); + const state = makeServerState({ status: { connectionAttemptMade: false, state: Enriched.StatusEnum.DISCONNECTED, description: null } }); expect(Selectors.getIsConnected(rootState(state))).toBe(false); }); @@ -189,7 +189,7 @@ describe('Selectors', () => { // ── createSelector reference stability ────────────────────────────── it('getIsConnected → returns same value reference for identical state', () => { - const state = makeServerState({ status: { connectionAttemptMade: true, state: App.StatusEnum.LOGGED_IN, description: null } }); + const state = makeServerState({ status: { connectionAttemptMade: true, state: Enriched.StatusEnum.LOGGED_IN, description: null } }); const root = rootState(state); const a = Selectors.getIsConnected(root); const b = Selectors.getIsConnected(root); diff --git a/webclient/src/store/server/server.selectors.ts b/webclient/src/store/server/server.selectors.ts index 535f3c43e..32f5a2ac6 100644 --- a/webclient/src/store/server/server.selectors.ts +++ b/webclient/src/store/server/server.selectors.ts @@ -1,5 +1,5 @@ import { createSelector } from '@reduxjs/toolkit'; -import { App, Data } from '@app/types'; +import { Data, Enriched } from '@app/types'; import { SortUtil } from '../common'; import { ServerState } from './server.interfaces'; @@ -23,7 +23,7 @@ export const Selectors = { /** True when the server status has reached LOGGED_IN. */ getIsConnected: createSelector( [({ server }: State) => server.status.state], - (state): boolean => state === App.StatusEnum.LOGGED_IN + (state): boolean => state === Enriched.StatusEnum.LOGGED_IN ), /** True when the currently logged-in user has the IsModerator level flag. */ diff --git a/webclient/src/types/app.ts b/webclient/src/types/app.ts index 2ec7c2a68..1921e8a44 100644 --- a/webclient/src/types/app.ts +++ b/webclient/src/types/app.ts @@ -1,5 +1,5 @@ export * from './cards'; -export * from './constants'; +export * from './regex-patterns'; export * from './countries'; export * from './languages'; export * from './routes'; diff --git a/webclient/src/types/countries.ts b/webclient/src/types/countries.ts index 0289b63ad..edc525877 100644 --- a/webclient/src/types/countries.ts +++ b/webclient/src/types/countries.ts @@ -250,4 +250,6 @@ export const countryCodes = [ 'XK', 'ZM', 'ZW', -]; +] as const; + +export type CountryCode = typeof countryCodes[number]; diff --git a/webclient/src/types/enriched.ts b/webclient/src/types/enriched.ts index b111c0e5e..6cf45316d 100644 --- a/webclient/src/types/enriched.ts +++ b/webclient/src/types/enriched.ts @@ -1,6 +1,5 @@ import type { Event_RoomSay, - GameEventContext, ServerInfo_Arrow, ServerInfo_Card, ServerInfo_ChatMessage, @@ -110,32 +109,20 @@ export interface GameMessage { timeReceived: number; } -/** - * Passed to every game event handler alongside the event payload. - * Contains per-container metadata from GameEventContainer. - * Not stored in Redux — transient routing metadata only. - */ -export interface GameEventMeta { - gameId: number; - playerId: number; - /** Raw protobuf GameEventContext object. Not stored in Redux. */ - context: GameEventContext | null; - secondsElapsed: number; - /** Proto type is uint32. Non-zero means the action was forced by a judge. */ - forcedByJudge: number; -} - export interface LogGroups { room: ServerInfo_ChatMessage[]; game: ServerInfo_ChatMessage[]; chat: ServerInfo_ChatMessage[]; } -// ── Connect options (re-exported from @app/websocket) ──────────────────────── -// Source of truth lives in src/websocket/connectOptions.ts. Re-exported here -// so UI code can use the Enriched.* namespace without importing @app/websocket. +// ── Websocket re-exports ───────────────────────────────────────────────────── +// Source of truth lives in @app/websocket. Re-exported here so app code can +// reach these via the Enriched.* namespace without importing @app/websocket. + +export { StatusEnum, WebSocketConnectReason } from '@app/websocket'; export type { + GameEventMeta, LoginConnectOptions, RegisterConnectOptions, ActivateConnectOptions, diff --git a/webclient/src/types/constants.spec.ts b/webclient/src/types/regex-patterns.spec.ts similarity index 98% rename from webclient/src/types/constants.spec.ts rename to webclient/src/types/regex-patterns.spec.ts index d324867dc..c34f8f3fb 100644 --- a/webclient/src/types/constants.spec.ts +++ b/webclient/src/types/regex-patterns.spec.ts @@ -2,7 +2,7 @@ import { URL_REGEX, MESSAGE_SENDER_REGEX, MENTION_REGEX, -} from './constants'; +} from './regex-patterns'; describe('RegEx', () => { describe('URL_REGEX', () => { diff --git a/webclient/src/types/constants.ts b/webclient/src/types/regex-patterns.ts similarity index 100% rename from webclient/src/types/constants.ts rename to webclient/src/types/regex-patterns.ts diff --git a/webclient/src/types/server.ts b/webclient/src/types/server.ts index 19135bc8f..59eb399c0 100644 --- a/webclient/src/types/server.ts +++ b/webclient/src/types/server.ts @@ -1,5 +1,4 @@ -export { StatusEnum, WebSocketConnectReason } from '@app/websocket'; -import type { StatusEnum } from '@app/websocket'; +import type { StatusEnum } from './enriched'; export interface ServerStatus { status: StatusEnum; @@ -19,59 +18,3 @@ export class Host { hashedPassword?: string; remember?: boolean; } - -export const DefaultHosts: Host[] = [ - { - name: 'Chickatrice', - host: 'mtg.chickatrice.net', - port: '443', - localPort: '4748', - editable: false, - }, - { - name: 'Rooster', - host: 'server.cockatrice.us/servatrice', - port: '4748', - localHost: 'server.cockatrice.us', - editable: false, - }, - { - name: 'Rooster Beta', - host: 'beta.cockatrice.us/servatrice', - port: '4748', - localHost: 'beta.cockatrice.us', - editable: false, - }, - { - name: 'Tetrarch', - host: 'mtg.tetrarch.co/servatrice', - port: '443', - editable: false, - }, -]; - -export const getHostPort = (host: Host): { host: string, port: string } => { - const isLocal = window.location.hostname === 'localhost'; - - if (!host) { - return { - host: '', - port: '' - }; - } - - return { - host: !isLocal ? host.host : host.localHost || host.host, - port: !isLocal ? host.port : host.localPort || host.port, - } -}; - -export enum KnownHost { - ROOSTER = 'Rooster', - TETRARCH = 'Tetrarch', -} - -export const KnownHosts = { - [KnownHost.ROOSTER]: { port: 4748, host: 'server.cockatrice.us', }, - [KnownHost.TETRARCH]: { port: 443, host: 'mtg.tetrarch.co/servatrice' }, -} diff --git a/webclient/src/websocket/commands/session/sessionCommands-complex.spec.ts b/webclient/src/websocket/commands/session/sessionCommands-complex.spec.ts index 7f0b9e2c6..daf90d083 100644 --- a/webclient/src/websocket/commands/session/sessionCommands-complex.spec.ts +++ b/webclient/src/websocket/commands/session/sessionCommands-complex.spec.ts @@ -18,7 +18,7 @@ import { Mock } from 'vitest'; import { makeCallbackHelpers } from '../../__mocks__/callbackHelpers'; import { WebClient } from '../../WebClient'; import * as SessionIndexMocks from './'; -import { App, Enriched } from '@app/types'; +import { Enriched } from '@app/types'; import { StatusEnum } from '../../interfaces/StatusEnum'; import { Command_Activate_ext, @@ -59,7 +59,7 @@ const baseTransport = { host: 'h', port: '1' }; const makeLoginOpts = (overrides: Partial = {}): Enriched.LoginConnectOptions => ({ ...baseTransport, userName: 'alice', - reason: App.WebSocketConnectReason.LOGIN, + reason: Enriched.WebSocketConnectReason.LOGIN, ...overrides, }); const makeRegisterOpts = ( @@ -71,7 +71,7 @@ const makeRegisterOpts = ( email: 'a@b.com', country: 'US', realName: 'Al', - reason: App.WebSocketConnectReason.REGISTER, + reason: Enriched.WebSocketConnectReason.REGISTER, ...overrides, }); const makeActivateOpts = ( @@ -80,26 +80,26 @@ const makeActivateOpts = ( ...baseTransport, userName: 'alice', token: 'tok', - reason: App.WebSocketConnectReason.ACTIVATE_ACCOUNT, + reason: Enriched.WebSocketConnectReason.ACTIVATE_ACCOUNT, ...overrides, }); const makeForgotRequestOpts = (): Enriched.PasswordResetRequestConnectOptions => ({ ...baseTransport, userName: 'alice', - reason: App.WebSocketConnectReason.PASSWORD_RESET_REQUEST, + reason: Enriched.WebSocketConnectReason.PASSWORD_RESET_REQUEST, }); const makeForgotChallengeOpts = (): Enriched.PasswordResetChallengeConnectOptions => ({ ...baseTransport, userName: 'alice', email: 'a@b.com', - reason: App.WebSocketConnectReason.PASSWORD_RESET_CHALLENGE, + reason: Enriched.WebSocketConnectReason.PASSWORD_RESET_CHALLENGE, }); const makeForgotResetOpts = (): Enriched.PasswordResetConnectOptions => ({ ...baseTransport, userName: 'alice', token: 'tok', newPassword: 'newpw', - reason: App.WebSocketConnectReason.PASSWORD_RESET, + reason: Enriched.WebSocketConnectReason.PASSWORD_RESET, }); @@ -194,7 +194,7 @@ describe('login', () => { }); it('onSuccess passes hashedPassword to loginSuccessful when salt is used', () => { - login({ host: 'h', port: '1', userName: 'alice', reason: App.WebSocketConnectReason.LOGIN }, 'pw', 'salt'); + login({ host: 'h', port: '1', userName: 'alice', reason: Enriched.WebSocketConnectReason.LOGIN }, 'pw', 'salt'); const loginResp = { buddyList: [], ignoreList: [], userInfo: { name: 'alice' } }; invokeOnSuccess(loginResp, { responseCode: 0 }); const calledWith = (WebClient.instance.response.session.loginSuccessful as Mock).mock.calls[0][0]; diff --git a/webclient/src/websocket/events/game/gameEvents.spec.ts b/webclient/src/websocket/events/game/gameEvents.spec.ts index 1276f1724..a5756bb34 100644 --- a/webclient/src/websocket/events/game/gameEvents.spec.ts +++ b/webclient/src/websocket/events/game/gameEvents.spec.ts @@ -115,10 +115,10 @@ describe('playerPropertiesChanged event', () => { }); describe('gameSay event', () => { - it('delegates to WebClient.instance.response.game.gameSay with gameId, playerId, message', () => { + it('delegates to WebClient.instance.response.game.gameSay with gameId, playerId, message, timeReceived', () => { const data = create(Event_GameSaySchema, { message: 'gg' }); gameSay(data, meta); - expect(WebClient.instance.response.game.gameSay).toHaveBeenCalledWith(5, 2, 'gg'); + expect(WebClient.instance.response.game.gameSay).toHaveBeenCalledWith(5, 2, 'gg', expect.any(Number)); }); }); diff --git a/webclient/src/websocket/events/game/gameSay.ts b/webclient/src/websocket/events/game/gameSay.ts index 72a585fef..7773720de 100644 --- a/webclient/src/websocket/events/game/gameSay.ts +++ b/webclient/src/websocket/events/game/gameSay.ts @@ -3,5 +3,5 @@ import type { GameEventMeta } from '../../interfaces/WebSocketConfig'; import { WebClient } from '../../WebClient'; export function gameSay(data: Event_GameSay, meta: GameEventMeta): void { - WebClient.instance.response.game.gameSay(meta.gameId, meta.playerId, data.message); + WebClient.instance.response.game.gameSay(meta.gameId, meta.playerId, data.message, Date.now()); } diff --git a/webclient/src/websocket/interfaces/WebClientResponse.ts b/webclient/src/websocket/interfaces/WebClientResponse.ts index cf3bd40d5..4000021ee 100644 --- a/webclient/src/websocket/interfaces/WebClientResponse.ts +++ b/webclient/src/websocket/interfaces/WebClientResponse.ts @@ -134,7 +134,7 @@ export interface IGameResponse { gameClosed(gameId: number): void; gameHostChanged(gameId: number, hostId: number): void; kicked(gameId: number): void; - gameSay(gameId: number, playerId: number, message: string): void; + gameSay(gameId: number, playerId: number, message: string, timeReceived: number): void; cardMoved(gameId: number, playerId: number, data: Event_MoveCard): void; cardFlipped(gameId: number, playerId: number, data: Event_FlipCard): void; cardDestroyed(gameId: number, playerId: number, data: Event_DestroyCard): void; diff --git a/webclient/src/websocket/services/KeepAliveService.spec.ts b/webclient/src/websocket/services/KeepAliveService.spec.ts index c14beed1b..81e023658 100644 --- a/webclient/src/websocket/services/KeepAliveService.spec.ts +++ b/webclient/src/websocket/services/KeepAliveService.spec.ts @@ -64,5 +64,14 @@ describe('KeepAliveService', () => { expect(service.endPingLoop).toHaveBeenCalled(); }); + + it('should clear previous interval when startPingLoop is called again', () => { + const clearSpy = vi.spyOn(globalThis, 'clearInterval'); + const previousCb = (service as KeepAliveInternal).keepalivecb; + + service.startPingLoop(interval, ping); + + expect(clearSpy).toHaveBeenCalledWith(previousCb); + }); }); }); diff --git a/webclient/src/websocket/services/KeepAliveService.ts b/webclient/src/websocket/services/KeepAliveService.ts index 4b275cf3c..03f42d2a1 100644 --- a/webclient/src/websocket/services/KeepAliveService.ts +++ b/webclient/src/websocket/services/KeepAliveService.ts @@ -13,6 +13,7 @@ export class KeepAliveService { } public startPingLoop(interval: number, ping: (onPong: () => void) => void): void { + this.endPingLoop(); this.keepalivecb = setInterval(() => { // check if the previous ping got no reply if (this.lastPingPending) { diff --git a/webclient/src/websocket/services/ProtobufService.spec.ts b/webclient/src/websocket/services/ProtobufService.spec.ts index 7880b9099..fe65593d3 100644 --- a/webclient/src/websocket/services/ProtobufService.spec.ts +++ b/webclient/src/websocket/services/ProtobufService.spec.ts @@ -112,6 +112,67 @@ describe('ProtobufService', () => { expect((service as ProtobufInternal).cmdId).toBe(0); expect((service as ProtobufInternal).pendingCommands.size).toBe(0); }); + + it('returns true when command is sent', () => { + const service = new ProtobufService(mockSocket); + const result = service.sendCommand(create(CommandContainerSchema), vi.fn()); + expect(result).toBe(true); + }); + + it('returns false when transport is closed', () => { + const service = new ProtobufService(mockSocket); + mockSocket.isOpen.mockReturnValue(false); + const result = service.sendCommand(create(CommandContainerSchema), vi.fn()); + expect(result).toBe(false); + }); + }); + + describe('send*Command when transport is closed', () => { + it('calls onError when sendSessionCommand is dropped', () => { + const service = new ProtobufService(mockSocket); + mockSocket.isOpen.mockReturnValue(false); + const onError = vi.fn(); + service.sendSessionCommand(sessionExt, {}, { onError }); + expect(onError).toHaveBeenCalledWith(-1, expect.any(Object)); + }); + + it('calls onError when sendRoomCommand is dropped', () => { + const service = new ProtobufService(mockSocket); + mockSocket.isOpen.mockReturnValue(false); + const onError = vi.fn(); + service.sendRoomCommand(42, roomExt, {}, { onError }); + expect(onError).toHaveBeenCalledWith(-1, expect.any(Object)); + }); + + it('calls onError when sendGameCommand is dropped', () => { + const service = new ProtobufService(mockSocket); + mockSocket.isOpen.mockReturnValue(false); + const onError = vi.fn(); + service.sendGameCommand(7, gameExt, {}, { onError }); + expect(onError).toHaveBeenCalledWith(-1, expect.any(Object)); + }); + + it('calls onError when sendModeratorCommand is dropped', () => { + const service = new ProtobufService(mockSocket); + mockSocket.isOpen.mockReturnValue(false); + const onError = vi.fn(); + service.sendModeratorCommand(moderatorExt, {}, { onError }); + expect(onError).toHaveBeenCalledWith(-1, expect.any(Object)); + }); + + it('calls onError when sendAdminCommand is dropped', () => { + const service = new ProtobufService(mockSocket); + mockSocket.isOpen.mockReturnValue(false); + const onError = vi.fn(); + service.sendAdminCommand(adminExt, {}, { onError }); + expect(onError).toHaveBeenCalledWith(-1, expect.any(Object)); + }); + + it('does not throw when command is dropped with no options', () => { + const service = new ProtobufService(mockSocket); + mockSocket.isOpen.mockReturnValue(false); + expect(() => service.sendSessionCommand(sessionExt, {})).not.toThrow(); + }); }); describe('sendSessionCommand', () => { @@ -311,9 +372,9 @@ describe('ProtobufService', () => { expect(processGameEvent).toHaveBeenCalled(); }); - it('logs unknown message types (default case)', () => { + it('warns on unknown message types (default case)', () => { const service = new ProtobufService(mockSocket); - const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); vi.mocked(fromBinary).mockReturnValue( create(ServerMessageSchema, { diff --git a/webclient/src/websocket/services/ProtobufService.ts b/webclient/src/websocket/services/ProtobufService.ts index 05437f80e..2ffabacad 100644 --- a/webclient/src/websocket/services/ProtobufService.ts +++ b/webclient/src/websocket/services/ProtobufService.ts @@ -54,11 +54,7 @@ export class ProtobufService { const gameCmd = create(GameCommandSchema); setExtension(gameCmd, ext, value); const cmd = create(CommandContainerSchema, { gameId, gameCommand: [gameCmd] }); - this.sendCommand(cmd, raw => { - if (options) { - handleResponse(ext.typeName, raw, options); - } - }); + this.dispatchCommand(ext.typeName, cmd, options); } public sendRoomCommand( @@ -70,11 +66,7 @@ export class ProtobufService { const roomCmd = create(RoomCommandSchema); setExtension(roomCmd, ext, value); const cmd = create(CommandContainerSchema, { roomId, roomCommand: [roomCmd] }); - this.sendCommand(cmd, raw => { - if (options) { - handleResponse(ext.typeName, raw, options); - } - }); + this.dispatchCommand(ext.typeName, cmd, options); } public sendSessionCommand( @@ -85,11 +77,7 @@ export class ProtobufService { const sesCmd = create(SessionCommandSchema); setExtension(sesCmd, ext, value); const cmd = create(CommandContainerSchema, { sessionCommand: [sesCmd] }); - this.sendCommand(cmd, raw => { - if (options) { - handleResponse(ext.typeName, raw, options); - } - }); + this.dispatchCommand(ext.typeName, cmd, options); } public sendModeratorCommand( @@ -100,11 +88,7 @@ export class ProtobufService { const modCmd = create(ModeratorCommandSchema); setExtension(modCmd, ext, value); const cmd = create(CommandContainerSchema, { moderatorCommand: [modCmd] }); - this.sendCommand(cmd, raw => { - if (options) { - handleResponse(ext.typeName, raw, options); - } - }); + this.dispatchCommand(ext.typeName, cmd, options); } public sendAdminCommand( @@ -115,22 +99,31 @@ export class ProtobufService { const adminCmd = create(AdminCommandSchema); setExtension(adminCmd, ext, value); const cmd = create(CommandContainerSchema, { adminCommand: [adminCmd] }); - this.sendCommand(cmd, raw => { - if (options) { - handleResponse(ext.typeName, raw, options); - } - }); + this.dispatchCommand(ext.typeName, cmd, options); } - public sendCommand(cmd: CommandContainer, callback: (raw: Response) => void) { + private dispatchCommand(typeName: string, cmd: CommandContainer, options?: CommandOptions): void { + const sent = this.sendCommand(cmd, raw => { + if (options) { + handleResponse(typeName, raw, options); + } + }); + + if (!sent) { + options?.onError?.(-1, {} as Response); + } + } + + public sendCommand(cmd: CommandContainer, callback: (raw: Response) => void): boolean { if (!this.transport.isOpen()) { - return; + return false; } this.cmdId++; cmd.cmdId = BigInt(this.cmdId); this.pendingCommands.set(this.cmdId, callback); this.transport.send(toBinary(CommandContainerSchema, cmd)); + return true; } public handleMessageEvent({ data }: MessageEvent): void { @@ -153,7 +146,7 @@ export class ProtobufService { this.processGameEvent(msg.gameEventContainer); break; default: - console.log(msg); + console.warn('Unknown message type:', msg); break; } } diff --git a/webclient/src/websocket/services/WebSocketService.spec.ts b/webclient/src/websocket/services/WebSocketService.spec.ts index c565ac74e..553adbedf 100644 --- a/webclient/src/websocket/services/WebSocketService.spec.ts +++ b/webclient/src/websocket/services/WebSocketService.spec.ts @@ -198,6 +198,12 @@ describe('WebSocketService', () => { service.send(data); expect(mockInstance.send).toHaveBeenCalledWith(data); }); + + it('does not throw when socket is undefined (before connect)', () => { + const service = new WebSocketService(mockConfig); + const data = new Uint8Array([1, 2, 3]); + expect(() => service.send(data)).not.toThrow(); + }); }); describe('checkReadyState', () => { diff --git a/webclient/src/websocket/services/WebSocketService.ts b/webclient/src/websocket/services/WebSocketService.ts index cf2321667..32ebc5c33 100644 --- a/webclient/src/websocket/services/WebSocketService.ts +++ b/webclient/src/websocket/services/WebSocketService.ts @@ -54,6 +54,9 @@ export class WebSocketService { } public send(message: Uint8Array): void { + if (!this.socket) { + return; + } this.socket.send(message as unknown as ArrayBufferView); }