diff --git a/src/app/hooks/usePerMessageProfile.proxy.test.ts b/src/app/hooks/usePerMessageProfile.proxy.test.ts index 2f5fca6e2..ac910a9ea 100644 --- a/src/app/hooks/usePerMessageProfile.proxy.test.ts +++ b/src/app/hooks/usePerMessageProfile.proxy.test.ts @@ -1,4 +1,5 @@ -import { describe, expect, it } from 'vitest'; +import type { MatrixClient } from '$types/matrix-sdk'; +import { describe, expect, it, vi } from 'vitest'; import { extractCircumfixProxyTagsFromKey, @@ -8,6 +9,8 @@ import { type PerMessageProfileProxyAssociationV1, proxyNeedsMigration, createProxyKey, + setCurrentlyUsedPerMessageProfileIdForAccount, + setCurrentlyUsedPerMessageProfileIdForRoom, } from './usePerMessageProfile'; describe('migratePerMessageProfileProxyAssociation', () => { @@ -132,3 +135,60 @@ describe('parsePerMessageProfileProxyAssociation', () => { expect(parsed.regex.test('[no] trailing')).toBe(false); }); }); + +describe('per-message profile persistence', () => { + it('serializes room writes and reads the latest account data snapshot', async () => { + const associations: Record = {}; + const writes: unknown[] = []; + let releaseFirstWrite!: () => void; + const firstWrite = new Promise((resolve) => { + releaseFirstWrite = resolve; + }); + + const mx = { + getAccountData: vi.fn<() => { getContent: () => { associations: typeof associations } }>( + () => ({ getContent: () => ({ associations }) }) + ), + setAccountData: vi.fn< + (_event: unknown, content: { associations: typeof associations }) => Promise + >(async (_event, content) => { + writes.push(content); + if (writes.length === 1) await firstWrite; + Object.assign(associations, content.associations); + }), + } as unknown as MatrixClient; + + const first = setCurrentlyUsedPerMessageProfileIdForRoom(mx, '!room:example.org', 'first'); + const second = setCurrentlyUsedPerMessageProfileIdForRoom(mx, '!room:example.org', 'second'); + + await vi.waitFor(() => expect(writes).toHaveLength(1)); + + releaseFirstWrite(); + await Promise.all([first, second]); + + expect(writes).toHaveLength(2); + expect((writes[1] as { associations: typeof associations }).associations).toEqual({ + '!room:example.org': { profileId: 'second' }, + }); + }); + + it('continues queued account writes after a rejected write', async () => { + const writes: string[] = []; + const mx = { + setAccountData: vi.fn< + (_event: unknown, content: { association: { profileId: string } }) => Promise + >(async (_event, content) => { + writes.push(content.association.profileId); + if (writes.length === 1) throw new Error('write failed'); + }), + deleteAccountData: vi.fn<(...args: unknown[]) => void>(), + } as unknown as MatrixClient; + + const first = setCurrentlyUsedPerMessageProfileIdForAccount(mx, 'first'); + const second = setCurrentlyUsedPerMessageProfileIdForAccount(mx, 'second'); + + await expect(first).rejects.toThrow('write failed'); + await expect(second).resolves.toBeUndefined(); + expect(writes).toEqual(['first', 'second']); + }); +}); diff --git a/src/app/hooks/usePerMessageProfile.ts b/src/app/hooks/usePerMessageProfile.ts index 3b18de237..bfc540f37 100644 --- a/src/app/hooks/usePerMessageProfile.ts +++ b/src/app/hooks/usePerMessageProfile.ts @@ -6,9 +6,13 @@ import { CustomAccountDataEvent } from '$types/matrix/accountData'; import { MATRIX_UNSTABLE_COLORS } from '$unstable/prefixes'; import { MATRIX_UNSTABLE_PROFILE_PRONOUNS_PROPERTY_NAME } from '$unstable/prefixes'; import type { ColorSet } from './useUserProfile'; +import { createKeyedQueue } from '$utils/keyedQueue'; const ACCOUNT_DATA_PREFIX = CustomAccountDataEvent.SablePerProfileMessageProfiles; +/** Account data is read-modify-written, so writes to the same key must not interleave. */ +const enqueueProfilePersistence = createKeyedQueue(); + /** * a per message profile */ @@ -453,32 +457,34 @@ export async function setCurrentlyUsedPerMessageProfileIdForRoom( validUntil?: number, reset?: boolean ) { - const accountData = mx.getAccountData( - `${ACCOUNT_DATA_PREFIX}.roomassociation` as Parameters[0] - ); - const content: PerMessageProfileRoomAssociationWrapper | undefined = accountData?.getContent(); - const associations = getAssociationsMap(content); - - if (reset) { - associations.delete(roomId); - mx.setAccountData( + return enqueueProfilePersistence('roomassociation', async () => { + const accountData = mx.getAccountData( + `${ACCOUNT_DATA_PREFIX}.roomassociation` as Parameters[0] + ); + const content: PerMessageProfileRoomAssociationWrapper | undefined = accountData?.getContent(); + const associations = getAssociationsMap(content); + + if (reset) { + associations.delete(roomId); + await mx.setAccountData( + `${ACCOUNT_DATA_PREFIX}.roomassociation` as Parameters[0], + { associations: associationsMapToObject(associations) } as Parameters< + typeof mx.setAccountData + >[1] + ); + return; + } + if (!profileId) { + throw new Error("profile Id is empty, yet it isn't a reset"); + } + associations.set(roomId, { profileId, validUntil }); + await mx.setAccountData( `${ACCOUNT_DATA_PREFIX}.roomassociation` as Parameters[0], { associations: associationsMapToObject(associations) } as Parameters< typeof mx.setAccountData >[1] ); - return; - } - if (!profileId) { - throw new Error("profile Id is empty, yet it isn't a reset"); - } - associations.set(roomId, { profileId, validUntil }); - mx.setAccountData( - `${ACCOUNT_DATA_PREFIX}.roomassociation` as Parameters[0], - { associations: associationsMapToObject(associations) } as Parameters< - typeof mx.setAccountData - >[1] - ); + }); } /** @@ -490,22 +496,24 @@ export async function setCurrentlyUsedPerMessageProfileIdForAccount( validUntil?: number, reset?: boolean ) { - if (reset) { - mx.deleteAccountData( - `${ACCOUNT_DATA_PREFIX}.globalassociation` as Parameters[0] - ); - return; - } - if (!profileId) { - throw new Error("profile Id is empty, yet it isn't a reset"); - } + return enqueueProfilePersistence('globalassociation', async () => { + if (reset) { + await mx.deleteAccountData( + `${ACCOUNT_DATA_PREFIX}.globalassociation` as Parameters[0] + ); + return; + } + if (!profileId) { + throw new Error("profile Id is empty, yet it isn't a reset"); + } - const association: PerMessageProfileRoomAssociation = { profileId, validUntil }; + const association: PerMessageProfileRoomAssociation = { profileId, validUntil }; - mx.setAccountData( - `${ACCOUNT_DATA_PREFIX}.globalassociation` as Parameters[0], - { association: association } as Parameters[1] - ); + await mx.setAccountData( + `${ACCOUNT_DATA_PREFIX}.globalassociation` as Parameters[0], + { association: association } as Parameters[1] + ); + }); } /** diff --git a/src/app/utils/keyedQueue.ts b/src/app/utils/keyedQueue.ts new file mode 100644 index 000000000..1ce68997a --- /dev/null +++ b/src/app/utils/keyedQueue.ts @@ -0,0 +1,20 @@ +/** Serializes operations per key so read-modify-write sequences cannot interleave. */ +export function createKeyedQueue() { + const tails = new Map>(); + + return function run(key: string, operation: () => T | PromiseLike): Promise { + const previous = tails.get(key) ?? Promise.resolve(); + const current = previous.catch(() => undefined).then(operation); + const tail = current.then( + () => undefined, + () => undefined + ); + + tails.set(key, tail); + void tail.then(() => { + if (tails.get(key) === tail) tails.delete(key); + }); + + return current; + }; +}