From b630bacdc62cfd4d3854b6f95f53486788705109 Mon Sep 17 00:00:00 2001 From: Myx Date: Thu, 11 Jun 2026 10:21:28 +0200 Subject: [PATCH] test: fix most e2e tests --- .../plugins/plugin-api-two-users.spec.ts | 11 ++- .../voice/mixed-signal-config-voice.spec.ts | 23 +++--- .../plugin-client-api.service.spec.ts | 58 +++++++++++++-- .../services/plugin-client-api.service.ts | 32 +++++++-- .../signaling-transport-handler.spec.ts | 71 +++++++++++++++++++ .../signaling/signaling-transport-handler.ts | 8 +-- .../store/users/user-avatar.effects.spec.ts | 34 +++++++++ .../app/store/users/user-avatar.effects.ts | 28 +++++--- toju-app/src/app/store/users/users.reducer.ts | 1 + 9 files changed, 230 insertions(+), 36 deletions(-) create mode 100644 toju-app/src/app/infrastructure/realtime/signaling/signaling-transport-handler.spec.ts diff --git a/e2e/tests/plugins/plugin-api-two-users.spec.ts b/e2e/tests/plugins/plugin-api-two-users.spec.ts index e6fc200..dfd01c0 100644 --- a/e2e/tests/plugins/plugin-api-two-users.spec.ts +++ b/e2e/tests/plugins/plugin-api-two-users.spec.ts @@ -42,8 +42,7 @@ test.describe('Plugin API multi-user runtime', () => { }); await test.step('Activate the server plugin for Bob as the embed/soundboard receiver', async () => { - await installGrantAndActivatePlugin(scenario.bob.page, false); - await closeSettingsModal(scenario.bob.page); + await installRequiredServerPluginsViaModal(scenario.bob.page); await expect(soundboardComposerButton(scenario.bob.page)).toBeVisible({ timeout: 20_000 }); await expect(scenario.bob.page.getByText(SOUND_BOARD_TEXT, { exact: true })).toBeVisible({ timeout: 20_000 }); }); @@ -178,6 +177,14 @@ async function installGrantAndActivatePlugin(page: Page, installFromStore: boole await expect(page.getByText('all-api plugin completed')).toBeVisible({ timeout: 30_000 }); } +async function installRequiredServerPluginsViaModal(page: Page): Promise { + const installButton = page.getByRole('button', { name: 'Install plugins' }); + + await expect(installButton).toBeVisible({ timeout: 30_000 }); + await installButton.click(); + await expect(installButton).toHaveCount(0, { timeout: 30_000 }); +} + async function closeSettingsModal(page: Page): Promise { await page.keyboard.press('Escape'); await expect(page.getByTestId('plugin-manager')).toHaveCount(0); diff --git a/e2e/tests/voice/mixed-signal-config-voice.spec.ts b/e2e/tests/voice/mixed-signal-config-voice.spec.ts index 6f3cb0c..5dcde49 100644 --- a/e2e/tests/voice/mixed-signal-config-voice.spec.ts +++ b/e2e/tests/voice/mixed-signal-config-voice.spec.ts @@ -18,7 +18,8 @@ import { import { authHeaders, readAuthTokenFromPage, - registerTestUser + registerTestUser, + type AuthSession } from '../../helpers/auth-api'; import { RegisterPage } from '../../pages/register.page'; import { ServerSearchPage } from '../../pages/server-search.page'; @@ -151,6 +152,12 @@ test.describe('Mixed signal-config voice', () => { }); let secondaryRoomId = ''; + // Identity that owns the secondary room. The invite must be created with + // this same API session: client 0 also auto-provisions a *separate* + // identity on the secondary signal endpoint, which overwrites the page's + // stored token, so reading the token back from the page would yield a + // non-owner identity and the invite request would be rejected (NOT_MEMBER). + let secondaryRoomOwner: AuthSession; // ── Create rooms ──────────────────────────────────────────── await test.step('Create voice room on primary and chat room on secondary', async () => { @@ -192,6 +199,7 @@ test.describe('Mixed signal-config voice', () => { ); secondaryRoomId = secondaryRoom.id; + secondaryRoomOwner = secondarySession; }); // ── Create invite links ───────────────────────────────────── @@ -221,17 +229,14 @@ test.describe('Mixed signal-config voice', () => { primaryRoomInviteUrl = `/invite/${primaryInvite.id}?server=${encodeURIComponent(testServer.url)}`; - // Create invite for secondary room (chat) via API - const secondaryToken = await readAuthTokenFromPage(clients[0].page, secondaryServer.url); - - if (!secondaryToken) { - throw new Error('Missing session token for secondary signal invite creation'); - } - + // Create invite for secondary room (chat) via API using the API session + // that owns the room. The page-stored token for the secondary endpoint + // belongs to client 0's auto-provisioned identity, which is not the + // room owner and would be rejected with NOT_MEMBER. const secondaryInvite = await createInviteViaApi( secondaryServer.url, secondaryRoomId, - secondaryToken, + secondaryRoomOwner.token, clients[0].user.displayName ); diff --git a/toju-app/src/app/domains/plugins/application/services/plugin-client-api.service.spec.ts b/toju-app/src/app/domains/plugins/application/services/plugin-client-api.service.spec.ts index 93a2ac3..57c48e9 100644 --- a/toju-app/src/app/domains/plugins/application/services/plugin-client-api.service.spec.ts +++ b/toju-app/src/app/domains/plugins/application/services/plugin-client-api.service.spec.ts @@ -137,6 +137,39 @@ describe('PluginClientApiService', () => { expect(context.messageRevisions.broadcastRevision).toHaveBeenCalled(); }); + it('applies an edit issued immediately after sending a message', async () => { + const api = context.service.createApi(TEST_MANIFEST); + const sent = api.messages.send('Plugin API original message'); + + api.messages.edit(sent.id, 'Plugin API edited message'); + + await new Promise((resolve) => setTimeout(resolve, 0)); + + const editDispatched = context.store.dispatch.mock.calls.some( + ([action]) => action.type === MessagesActions.editMessageSuccess.type + && action.messageId === sent.id + && action.content === 'Plugin API edited message' + ); + + expect(editDispatched).toBe(true); + }); + + it('applies a delete issued immediately after sending a message', async () => { + const api = context.service.createApi(TEST_MANIFEST); + const sent = api.messages.send('Plugin API deleted message'); + + api.messages.delete(sent.id); + + await new Promise((resolve) => setTimeout(resolve, 0)); + + const deleteDispatched = context.store.dispatch.mock.calls.some( + ([action]) => action.type === MessagesActions.deleteMessageSuccess.type + && action.messageId === sent.id + ); + + expect(deleteDispatched).toBe(true); + }); + it('publishes typing state through the realtime facade', () => { const api = context.service.createApi(TEST_MANIFEST); @@ -287,6 +320,12 @@ interface ServiceTestContext { createSignedRevision: ReturnType; persistRevision: ReturnType; }; + db: { + getMessageById: ReturnType; + saveMessage: ReturnType; + updateMessage: ReturnType; + updateRoom: ReturnType; + }; } function createServiceTestContext(): ServiceTestContext { @@ -354,6 +393,15 @@ function createServiceTestContext(): ServiceTestContext { onSignalingMessage: new Subject(), sendRawMessage: vi.fn() }; + const messageDb = new Map(); + const db = { + getMessageById: vi.fn(async (id: string) => messageDb.get(id) ?? null), + saveMessage: vi.fn(async (message: Message) => { + messageDb.set(message.id, message); + }), + updateMessage: vi.fn(async () => undefined), + updateRoom: vi.fn(async () => undefined) + }; const store = { dispatch: vi.fn(), selectSignal: vi.fn((selector: unknown) => { @@ -401,12 +449,7 @@ function createServiceTestContext(): ServiceTestContext { }, { provide: DatabaseService, - useValue: { - getMessageById: vi.fn(async () => null), - saveMessage: vi.fn(async () => undefined), - updateMessage: vi.fn(async () => undefined), - updateRoom: vi.fn(async () => undefined) - } + useValue: db }, { provide: MessageRevisionService, @@ -485,7 +528,8 @@ function createServiceTestContext(): ServiceTestContext { store, uiRegistry, voice, - messageRevisions + messageRevisions, + db }; } diff --git a/toju-app/src/app/domains/plugins/application/services/plugin-client-api.service.ts b/toju-app/src/app/domains/plugins/application/services/plugin-client-api.service.ts index 3b5bab2..90304bf 100644 --- a/toju-app/src/app/domains/plugins/application/services/plugin-client-api.service.ts +++ b/toju-app/src/app/domains/plugins/application/services/plugin-client-api.service.ts @@ -68,6 +68,15 @@ export class PluginClientApiService { private readonly voice = inject(VoiceConnectionFacade); private readonly messageRevisions = inject(MessageRevisionService); + /** + * Serializes plugin-initiated message create/edit/delete operations. `send` + * returns its draft synchronously while the create persists asynchronously, so + * an edit/delete issued on the returned id must wait for that create to land in + * the database - otherwise its `getMessageById` lookup races ahead of the + * pending `saveMessage` and the mutation is silently dropped. + */ + private messageMutationChain: Promise = Promise.resolve(); + private readonly currentMessages = this.store.selectSignal(selectCurrentRoomMessages); private readonly currentRoom = this.store.selectSignal(selectCurrentRoom); private readonly currentRoomChannels = this.store.selectSignal(selectCurrentRoomChannels); @@ -630,7 +639,7 @@ export class PluginClientApiService { } private deletePluginMessage(pluginId: string, messageId: string): void { - void this.emitPluginMessageMutation(pluginId, messageId, { + this.enqueueMessageMutation(() => this.emitPluginMessageMutation(pluginId, messageId, { type: 'plugin-delete', apply: async (existing, editedAt) => this.messageRevisions.createSignedRevision({ message: existing, @@ -647,11 +656,11 @@ export class PluginClientApiService { type: 'message-deleted' }), dispatch: () => MessagesActions.deleteMessageSuccess({ messageId }) - }); + })); } private editPluginMessage(pluginId: string, messageId: string, content: string): void { - void this.emitPluginMessageMutation(pluginId, messageId, { + this.enqueueMessageMutation(() => this.emitPluginMessageMutation(pluginId, messageId, { type: 'plugin-edit', content, apply: async (existing, editedAt) => this.messageRevisions.createSignedRevision({ @@ -674,7 +683,7 @@ export class PluginClientApiService { editedAt: message.editedAt ?? Date.now(), messageId }) - }); + })); } private sendPluginMessage(pluginId: string, content: string, channelId?: string): Message { @@ -694,7 +703,7 @@ export class PluginClientApiService { revision: 0 }; - void this.emitPluginMessageRevision(pluginId, { + this.enqueueMessageMutation(() => this.emitPluginMessageRevision(pluginId, { draftMessage, type: 'create', actorId: currentUser?.id ?? pluginId, @@ -702,11 +711,22 @@ export class PluginClientApiService { pluginId, sign: !!currentUser, dispatch: (message) => MessagesActions.sendMessageSuccess({ message }) - }); + })); return draftMessage; } + /** + * Chains a plugin message operation onto the serial mutation queue so create, + * edit, and delete calls apply in the order the plugin issued them, regardless of + * how their individual async persistence steps would otherwise interleave. + */ + private enqueueMessageMutation(task: () => Promise): void { + this.messageMutationChain = this.messageMutationChain + .then(() => task()) + .catch(() => undefined); + } + private async emitPluginMessageRevision( pluginId: string, input: { diff --git a/toju-app/src/app/infrastructure/realtime/signaling/signaling-transport-handler.spec.ts b/toju-app/src/app/infrastructure/realtime/signaling/signaling-transport-handler.spec.ts new file mode 100644 index 0000000..435f8ff --- /dev/null +++ b/toju-app/src/app/infrastructure/realtime/signaling/signaling-transport-handler.spec.ts @@ -0,0 +1,71 @@ +import { SignalingTransportHandler, ResolvedSignalCredential } from './signaling-transport-handler'; +import { SIGNALING_TYPE_IDENTIFY } from '../realtime.constants'; + +describe('SignalingTransportHandler identify', () => { + const SIGNAL_URL = 'ws://signal.example.com:3001'; + + function createHandler(resolved: ResolvedSignalCredential | null) { + const sentMessages: Record[] = []; + const manager = { + isSocketOpen: () => true, + sendRawMessage: (message: Record) => { + sentMessages.push(message); + } + }; + const coordinator = { + getConnectedSignalingManagers: () => [{ signalUrl: SIGNAL_URL, manager }], + getSignalingManager: (signalUrl: string) => (signalUrl === SIGNAL_URL ? manager : null) + } as unknown as ConstructorParameters[0]['signalingCoordinator']; + const handler = new SignalingTransportHandler({ + signalingCoordinator: coordinator, + logger: { warn: () => undefined, error: () => undefined, info: () => undefined } as never, + getLocalPeerId: () => 'local-peer', + resolveCredential: () => resolved, + getHomeCredential: () => resolved, + getClientInstanceId: () => 'device-a' + }); + + return { handler, sentMessages }; + } + + it('uses the freshly supplied display name over the stale stored credential', () => { + const { handler, sentMessages } = createHandler({ + userId: 'user-1', + token: 'token-1', + displayName: 'Alice' + }); + + handler.identify('user-1', 'Alice One', undefined, { + description: 'New bio', + profileUpdatedAt: 123456 + }); + + const identifyMessages = sentMessages.filter((entry) => entry['type'] === SIGNALING_TYPE_IDENTIFY); + + expect(identifyMessages.length).toBeGreaterThan(0); + + for (const message of identifyMessages) { + expect(message['displayName']).toBe('Alice One'); + expect(message['profileUpdatedAt']).toBe(123456); + expect(message['description']).toBe('New bio'); + } + }); + + it('falls back to the stored credential display name when none is supplied', () => { + const { handler, sentMessages } = createHandler({ + userId: 'user-1', + token: 'token-1', + displayName: 'Alice' + }); + + handler.identify('user-1', ' ', undefined); + + const identifyMessages = sentMessages.filter((entry) => entry['type'] === SIGNALING_TYPE_IDENTIFY); + + expect(identifyMessages.length).toBeGreaterThan(0); + + for (const message of identifyMessages) { + expect(message['displayName']).toBe('Alice'); + } + }); +}); diff --git a/toju-app/src/app/infrastructure/realtime/signaling/signaling-transport-handler.ts b/toju-app/src/app/infrastructure/realtime/signaling/signaling-transport-handler.ts index 4037d32..be7e131 100644 --- a/toju-app/src/app/infrastructure/realtime/signaling/signaling-transport-handler.ts +++ b/toju-app/src/app/infrastructure/realtime/signaling/signaling-transport-handler.ts @@ -226,7 +226,7 @@ export class SignalingTransportHandler { signalUrl?: string, profile?: Pick ): void { - const normalizedDisplayName = displayName.trim() || DEFAULT_DISPLAY_NAME; + const trimmedDisplayName = displayName.trim(); const normalizedDescription = typeof profile?.description === 'string' ? (profile.description.trim() || undefined) : undefined; @@ -243,7 +243,7 @@ export class SignalingTransportHandler { if (signalUrl) { this.identifyOnSignalUrl(signalUrl, { fallbackOderId: oderId, - displayName: normalizedDisplayName, + displayName: trimmedDisplayName, description: normalizedDescription, profileUpdatedAt: normalizedProfileUpdatedAt, homeSignalServerUrl: normalizedHomeSignalServerUrl, @@ -262,7 +262,7 @@ export class SignalingTransportHandler { for (const { signalUrl: managerSignalUrl, manager } of connectedManagers) { const credentials = this.identifyOnSignalUrl(managerSignalUrl, { fallbackOderId: oderId, - displayName: normalizedDisplayName, + displayName: trimmedDisplayName, description: normalizedDescription, profileUpdatedAt: normalizedProfileUpdatedAt, homeSignalServerUrl: normalizedHomeSignalServerUrl, @@ -312,7 +312,7 @@ export class SignalingTransportHandler { const credentials: IdentifyCredentials = { oderId: resolved.userId, token: resolved.token, - displayName: resolved.displayName || params.displayName, + displayName: params.displayName || resolved.displayName || DEFAULT_DISPLAY_NAME, description: params.description, profileUpdatedAt: params.profileUpdatedAt, homeSignalServerUrl: params.homeSignalServerUrl ?? resolved.homeSignalServerUrl, diff --git a/toju-app/src/app/store/users/user-avatar.effects.spec.ts b/toju-app/src/app/store/users/user-avatar.effects.spec.ts index 54b4e84..b37dee6 100644 --- a/toju-app/src/app/store/users/user-avatar.effects.spec.ts +++ b/toju-app/src/app/store/users/user-avatar.effects.spec.ts @@ -79,4 +79,38 @@ describe('user avatar sync helpers', () => { updatedAt: 100 })).toBe(false); }); + + it('requests data when only the remote profile text is newer and no avatar exists', () => { + const existingUser = createUser({ + displayName: 'Alice', + profileUpdatedAt: 100 + }); + + expect(shouldRequestAvatarData(existingUser, { + avatarUpdatedAt: 0, + profileUpdatedAt: 200 + })).toBe(true); + }); + + it('does not request profile data when the local profile is the same or newer', () => { + const existingUser = createUser({ profileUpdatedAt: 200 }); + + expect(shouldRequestAvatarData(existingUser, { + avatarUpdatedAt: 0, + profileUpdatedAt: 200 + })).toBe(false); + }); + + it('applies profile-only transfers when the remote profile is newer', () => { + const existingUser = createUser({ + displayName: 'Alice', + profileUpdatedAt: 100 + }); + + expect(shouldApplyAvatarTransfer(existingUser, { + hash: undefined, + updatedAt: 0, + profileUpdatedAt: 200 + })).toBe(true); + }); }); diff --git a/toju-app/src/app/store/users/user-avatar.effects.ts b/toju-app/src/app/store/users/user-avatar.effects.ts index 80ce43b..9e8519c 100644 --- a/toju-app/src/app/store/users/user-avatar.effects.ts +++ b/toju-app/src/app/store/users/user-avatar.effects.ts @@ -45,7 +45,7 @@ interface PendingAvatarTransfer { hash?: string; } -type AvatarVersionState = Pick | undefined; +type AvatarVersionState = Pick | undefined; type RoomProfileState = Pick | undefined, + incomingProfileUpdatedAt: number | undefined +): boolean { + return (incomingProfileUpdatedAt ?? 0) > (existingUser?.profileUpdatedAt ?? 0); +} + function hasSyncableUserData(user: Pick | null | undefined): boolean { - return (user?.avatarUpdatedAt ?? 0) > 0; + return (user?.avatarUpdatedAt ?? 0) > 0 || (user?.profileUpdatedAt ?? 0) > 0; } export function shouldRequestAvatarData( existingUser: AvatarVersionState, incomingAvatar: Pick ): boolean { - return shouldAcceptAvatarPayload(existingUser, incomingAvatar.avatarUpdatedAt ?? 0, incomingAvatar.avatarHash); + return shouldAcceptAvatarPayload(existingUser, incomingAvatar.avatarUpdatedAt ?? 0, incomingAvatar.avatarHash) + || shouldAcceptProfilePayload(existingUser, incomingAvatar.profileUpdatedAt); } export function shouldApplyAvatarTransfer( existingUser: AvatarVersionState, - transfer: Pick + transfer: Pick ): boolean { - return shouldAcceptAvatarPayload(existingUser, transfer.updatedAt, transfer.hash); + return shouldAcceptAvatarPayload(existingUser, transfer.updatedAt, transfer.hash) + || shouldAcceptProfilePayload(existingUser, transfer.profileUpdatedAt); } @Injectable() @@ -258,17 +267,18 @@ export class UserAvatarEffects { ) ); - private buildAvatarSummary(user: Pick): ChatEvent { + private buildAvatarSummary(user: Pick): ChatEvent { return { type: 'user-avatar-summary', oderId: user.oderId || user.id, avatarHash: user.avatarHash, - avatarUpdatedAt: user.avatarUpdatedAt || 0 + avatarUpdatedAt: user.avatarUpdatedAt || 0, + profileUpdatedAt: user.profileUpdatedAt || 0 }; } private handleAvatarSummary(event: ChatEvent, allUsers: User[], currentUser: User | null) { - if (!event.fromPeerId || !event.oderId || !event.avatarUpdatedAt) { + if (!event.fromPeerId || !event.oderId || (!event.avatarUpdatedAt && !event.profileUpdatedAt)) { return EMPTY; } @@ -467,6 +477,8 @@ export class UserAvatarEffects { oderId: userKey, username: user.username, displayName: user.displayName, + description: user.description, + profileUpdatedAt: user.profileUpdatedAt, avatarHash: user.avatarHash, avatarMime: blob ? (user.avatarMime || blob.type || 'image/webp') : undefined, avatarUpdatedAt: user.avatarUpdatedAt || 0, diff --git a/toju-app/src/app/store/users/users.reducer.ts b/toju-app/src/app/store/users/users.reducer.ts index 9807d89..08d15a3 100644 --- a/toju-app/src/app/store/users/users.reducer.ts +++ b/toju-app/src/app/store/users/users.reducer.ts @@ -178,6 +178,7 @@ function buildInactiveCameraState(user: User): User['cameraState'] { } function buildPresenceAwareUser(existingUser: User | undefined, incomingUser: User): User { + const presenceServerIds = mergePresenceServerIds(existingUser?.presenceServerIds, incomingUser.presenceServerIds); const isOnline = (presenceServerIds?.length ?? 0) > 0 || incomingUser.isOnline === true; const status = isOnline