diff --git a/src/app/features/estate/EstateNavigator.tsx b/src/app/features/estate/EstateNavigator.tsx index 68575932..bf5cfc24 100644 --- a/src/app/features/estate/EstateNavigator.tsx +++ b/src/app/features/estate/EstateNavigator.tsx @@ -24,12 +24,7 @@ import { } from '../../pages/pathUtils'; import { _RoomSearchParams } from '../../pages/paths'; import { searchModalAtom } from '../../state/searchModal'; -import { isDirectInvite } from '../../utils/room'; -import { - addRoomIdToMDirect, - getCanonicalAliasOrRoomId, - guessDmRoomUserId, -} from '../../utils/matrix'; +import { getCanonicalAliasOrRoomId } from '../../utils/matrix'; import { useMatrixClient } from '../../hooks/useMatrixClient'; import { useSelectedRoom } from '../../hooks/router/useSelectedRoom'; import { useCallEmbed } from '../../hooks/useCallEmbed'; @@ -49,8 +44,6 @@ import * as css from './EstateNavigator.css'; /** How long a tap waits for the client store to hold the room it just joined. */ const JOIN_ARRIVAL_TIMEOUT_MS = 20000; -/** How long after a timed-out tap the store may still deliver the room and have its DM filed. */ -const LATE_ARRIVAL_WINDOW_MS = 10 * 60 * 1000; const estateRoomPath = (mx: ReturnType, room: EstateRoom): string => { const alias = getCanonicalAliasOrRoomId(mx, room.roomId); @@ -67,28 +60,14 @@ function JoinRow({ room }: { room: EstateRoom }) { const navigate = useNavigate(); const [state, join] = useAsyncCallback( useCallback(async () => { - const userId = mx.getSafeUserId(); - const dmRoom = mx.getRoom(room.roomId); - const dmUserId = - room.direct && dmRoom && isDirectInvite(dmRoom, userId) - ? guessDmRoomUserId(dmRoom, userId) - : undefined; const result = await activateEstateRoom(mx, room); if (result.kind === 'refused') throw new Error(result.message); // joinRoom resolves before the store holds the room; opening it now shows the invite card. if (!(await awaitMyJoin(mx, room.roomId, JOIN_ARRIVAL_TIMEOUT_MS))) { - // The join is accepted; when the room arrives the row becomes a joined row by itself, - // so a DM is filed then (a failed write leaves it under Elsewhere, where it can be found). - if (dmUserId) { - awaitMyJoin(mx, room.roomId, LATE_ARRIVAL_WINDOW_MS) - .then(async (arrived) => { - if (arrived) await addRoomIdToMDirect(mx, room.roomId, dmUserId); - }) - .catch(() => undefined); - } + // The join is accepted; when the room arrives the row becomes a joined row by itself + // and a direct message is filed by useFileDirectRooms. throw new Error('Joined. The room is still on its way and will open from this list.'); } - if (dmUserId) await addRoomIdToMDirect(mx, room.roomId, dmUserId); navigate(estateRoomPath(mx, room)); }, [mx, navigate, room]) ); @@ -265,10 +244,7 @@ function EntryPoints() { /** A short message that wraps inside the navigator; never a raw request error. */ function EstateError({ message }: { message: string }) { return ( - + {message} ); diff --git a/src/app/hooks/useCommands.ts b/src/app/hooks/useCommands.ts index c02b0e94..798423c4 100644 --- a/src/app/hooks/useCommands.ts +++ b/src/app/hooks/useCommands.ts @@ -24,7 +24,7 @@ import { } from '../utils/matrix'; import { useRoomNavigate } from './useRoomNavigate'; import { Membership, StateEvent } from '../../types/matrix/room'; -import { getStateEvent } from '../utils/room'; +import { getStateEvent, wasInvitedAsDirect } from '../utils/room'; import { splitWithSpace } from '../utils/common'; import { createRoomEncryptionState } from '../components/create-room'; @@ -411,6 +411,16 @@ export const useCommands = (mx: MatrixClient, room: Room): CommandRecord => { description: 'Convert direct message to room', exe: async () => { await removeRoomIdFromMDirect(mx, room.roomId); + // The filer reads the invite's mark; clear it so the room stays a room. + const me = mx.getSafeUserId(); + if (wasInvitedAsDirect(room, me)) { + await mx.sendStateEvent( + room.roomId, + StateEvent.RoomMember as any, + { ...room.getMember(me)?.events?.member?.getContent(), is_direct: false }, + me + ); + } }, }, [Command.Delete]: { diff --git a/src/app/hooks/useFileDirectRooms.ts b/src/app/hooks/useFileDirectRooms.ts new file mode 100644 index 00000000..023316b5 --- /dev/null +++ b/src/app/hooks/useFileDirectRooms.ts @@ -0,0 +1,20 @@ +import { useEffect } from 'react'; +import { useMatrixClient } from './useMatrixClient'; +import { fileDirectRooms } from '../utils/matrix'; + +/** Keeps direct messages filed in m.direct: on startup, on every sync, on every membership change. */ +export const useFileDirectRooms = (): void => { + const mx = useMatrixClient(); + useEffect(() => { + const file = () => { + fileDirectRooms(mx).catch(() => undefined); + }; + file(); + mx.on('sync' as never, file); + mx.on('Room.myMembership' as never, file); + return () => { + mx.off('sync' as never, file); + mx.off('Room.myMembership' as never, file); + }; + }, [mx]); +}; diff --git a/src/app/pages/client/ClientNonUIFeatures.tsx b/src/app/pages/client/ClientNonUIFeatures.tsx index ce952bfc..5af097e4 100644 --- a/src/app/pages/client/ClientNonUIFeatures.tsx +++ b/src/app/pages/client/ClientNonUIFeatures.tsx @@ -26,6 +26,7 @@ import { getMxIdLocalPart, mxcUrlToHttp } from '../../utils/matrix'; import { useSelectedRoom } from '../../hooks/router/useSelectedRoom'; import { useInboxNotificationsSelected } from '../../hooks/router/useInbox'; import { useMediaAuthentication } from '../../hooks/useMediaAuthentication'; +import { useFileDirectRooms } from '../../hooks/useFileDirectRooms'; function SystemEmojiFeature() { const [twitterEmoji] = useSetting(settingsAtom, 'twitterEmoji'); @@ -258,6 +259,7 @@ type ClientNonUIFeaturesProps = { }; export function ClientNonUIFeatures({ children }: ClientNonUIFeaturesProps) { + useFileDirectRooms(); return ( <> diff --git a/src/app/utils/mDirect.test.ts b/src/app/utils/mDirect.test.ts new file mode 100644 index 00000000..55fc8602 --- /dev/null +++ b/src/app/utils/mDirect.test.ts @@ -0,0 +1,154 @@ +import { describe, expect, it } from 'vitest'; +import { addRoomIdToMDirect, fileDirectRooms, removeRoomIdFromMDirect } from './matrix'; +import { wasInvitedAsDirect } from './room'; + +const ME = '@me:test'; +type Content = Record; + +type FakeRoomOpts = { + membership?: string; + content?: Record; + prev?: Record; + friend?: string | null; +}; + +/** A joined room as the store holds it: my member event, and the friend once synced. */ +const fakeRoom = (roomId: string, o: FakeRoomOpts = {}) => { + const memberEvent = { + getContent: () => o.content ?? { membership: 'join' }, + getPrevContent: () => o.prev ?? {}, + }; + const friend = o.friend === undefined ? '@friend:test' : o.friend; + return { + roomId, + getMyMembership: () => o.membership ?? 'join', + getMember: () => ({ events: { member: memberEvent } }), + getJoinedMembers: () => + friend ? [{ userId: friend, events: { member: { getTs: () => 1 } } }] : [], + getLiveTimeline: () => ({ getState: () => undefined }), + }; +}; +const lateDm = (roomId: string, o: FakeRoomOpts = {}) => + fakeRoom(roomId, { prev: { membership: 'invite', is_direct: true }, ...o }); + +/** A client whose account-data write takes a while and lands only when it completes. */ +const fakeClient = (rooms: ReturnType[], initial?: Content) => { + let direct: Content | undefined = initial; + const writes: Content[] = []; + const mx = { + getSafeUserId: () => ME, + getRooms: () => rooms, + getAccountData: () => (direct ? { getContent: () => direct } : undefined), + setAccountData: async (_type: string, content: Content) => { + await new Promise((r) => { + setTimeout(r, 5); + }); + writes.push(structuredClone(content)); + direct = structuredClone(content); + }, + }; + return { mx: mx as never, raw: mx, writes, current: () => direct }; +}; + +describe('wasInvitedAsDirect', () => { + it('reads the mark from the join event or from the invite it replaced', () => { + expect(wasInvitedAsDirect(fakeRoom('!a', { content: { is_direct: true } }) as never, ME)).toBe( + true + ); + expect(wasInvitedAsDirect(lateDm('!b') as never, ME)).toBe(true); + expect(wasInvitedAsDirect(fakeRoom('!c') as never, ME)).toBe(false); + }); + + it('ignores a mark on a state the member event has since left', () => { + const room = fakeRoom('!d', { + prev: { membership: 'join', is_direct: true }, + content: { is_direct: false }, + }); + expect(wasInvitedAsDirect(room as never, ME)).toBe(false); + }); +}); + +describe('fileDirectRooms', () => { + it('files two DMs that arrive in the same sync, none lost', async () => { + const c = fakeClient([lateDm('!dm2:test'), lateDm('!dm3:test')]); + await fileDirectRooms(c.mx); + expect(c.writes).toHaveLength(1); + expect(c.current()).toEqual({ '@friend:test': ['!dm2:test', '!dm3:test'] }); + }); + + it('serializes overlapping runs so a later write builds on the earlier one', async () => { + const c = fakeClient([lateDm('!dm2:test'), fakeRoom('!room:test')]); + await Promise.all([ + fileDirectRooms(c.mx), + addRoomIdToMDirect(c.mx, '!made:test', '@other:test'), + fileDirectRooms(c.mx), + ]); + expect(c.current()).toEqual({ '@friend:test': ['!dm2:test'], '@other:test': ['!made:test'] }); + expect(c.writes).toHaveLength(2); + }); + + it('files a DM found at startup, after a reload while the join was in flight', async () => { + const reloaded = fakeClient([lateDm('!dm3:test')], { '@friend:test': ['!dm1:test'] }); + await fileDirectRooms(reloaded.mx); + expect(reloaded.current()).toEqual({ '@friend:test': ['!dm1:test', '!dm3:test'] }); + }); + + it('writes nothing when every marked room is already filed', async () => { + const c = fakeClient([lateDm('!dm2:test')], { '@friend:test': ['!dm2:test'] }); + await fileDirectRooms(c.mx); + expect(c.writes).toHaveLength(0); + }); + + it('skips rooms that are not joined, not marked, or whose other party has not synced', async () => { + const c = fakeClient([ + lateDm('!invited:test', { membership: 'invite' }), + fakeRoom('!plain:test'), + lateDm('!nofriend:test', { friend: null }), + ]); + await fileDirectRooms(c.mx); + expect(c.writes).toHaveLength(0); + // the next sync delivers the friend; the same call files it + const later = fakeClient([lateDm('!nofriend:test')]); + await fileDirectRooms(later.mx); + expect(later.current()).toEqual({ '@friend:test': ['!nofriend:test'] }); + }); + + it('does not file a room whose mark was cleared when it was converted to a room', async () => { + const converted = fakeRoom('!conv:test', { + content: { membership: 'join', is_direct: false }, + prev: { membership: 'join', is_direct: true }, + }); + const c = fakeClient([converted]); + await fileDirectRooms(c.mx); + expect(c.writes).toHaveLength(0); + }); + + it('keeps going after a failed write', async () => { + const c = fakeClient([lateDm('!dm2:test')]); + const original = c.raw.setAccountData; + let fail = true; + c.raw.setAccountData = async (...a: [string, Content]) => { + if (fail) { + fail = false; + throw new Error('offline'); + } + return original(...a); + }; + await expect(fileDirectRooms(c.mx)).rejects.toThrow('offline'); + await fileDirectRooms(c.mx); + expect(c.current()).toEqual({ '@friend:test': ['!dm2:test'] }); + }); +}); + +describe('m.direct edits', () => { + it('moves a room between users and removes it, one write at a time', async () => { + const c = fakeClient([], { '@a:test': ['!r:test'] }); + await Promise.all([ + addRoomIdToMDirect(c.mx, '!r:test', '@b:test'), + removeRoomIdFromMDirect(c.mx, '!gone:test'), + ]); + expect(c.current()).toEqual({ '@a:test': [], '@b:test': ['!r:test'] }); + await removeRoomIdFromMDirect(c.mx, '!r:test'); + expect(c.current()).toEqual({ '@a:test': [], '@b:test': [] }); + }); +}); diff --git a/src/app/utils/matrix.ts b/src/app/utils/matrix.ts index 4c86c4e2..f176764a 100644 --- a/src/app/utils/matrix.ts +++ b/src/app/utils/matrix.ts @@ -16,7 +16,7 @@ import { import to from 'await-to-js'; import { IImageInfo, IThumbnailContent, IVideoInfo } from '../../types/matrix/common'; import { AccountDataEvent } from '../../types/matrix/accountData'; -import { getStateEvent } from './room'; +import { getStateEvent, wasInvitedAsDirect } from './room'; import { Membership, StateEvent } from '../../types/matrix/room'; const DOMAIN_REGEX = /\b(?:[a-zA-Z0-9-]+\.)+[a-zA-Z]{2,}\b/; @@ -225,57 +225,92 @@ export const guessDmRoomUserId = (room: Room, myUserId: string): string => { return member1?.userId ?? myUserId; }; -export const addRoomIdToMDirect = async ( +type MDirectContent = Record; +type DirectEntry = { roomId: string; userId: string }; + +const mDirectWrites = new WeakMap>(); + +/** + * The only writer of m.direct. m.direct is one account-data event that every + * edit replaces whole, so edits run one at a time, each reading the content the + * previous one wrote. `edit` returns the new content, or undefined for no write. + */ +export const updateMDirect = ( + mx: MatrixClient, + edit: (current: MDirectContent) => MDirectContent | undefined +): Promise => { + const run = (mDirectWrites.get(mx) ?? Promise.resolve()).then(async () => { + const event = mx.getAccountData(AccountDataEvent.Direct as any); + const next = edit(event ? structuredClone(event.getContent()) : {}); + if (next) await mx.setAccountData(AccountDataEvent.Direct as any, next as any); + }); + mDirectWrites.set( + mx, + run.catch(() => undefined) + ); + return run; +}; + +/** A room can be a DM for one person only: it leaves every other user's list. */ +export const withDirectEntries = ( + current: MDirectContent, + entries: DirectEntry[] +): MDirectContent => { + const next = structuredClone(current); + entries.forEach(({ roomId, userId }) => { + Object.keys(next).forEach((targetUserId) => { + if (targetUserId !== userId) { + next[targetUserId] = next[targetUserId].filter((id) => id !== roomId); + } + }); + const roomIds = next[userId] ?? []; + if (!roomIds.includes(roomId)) roomIds.push(roomId); + next[userId] = roomIds; + }); + return next; +}; + +export const addRoomIdToMDirect = ( mx: MatrixClient, roomId: string, userId: string -): Promise => { - const mDirectsEvent = mx.getAccountData(AccountDataEvent.Direct as any); - let userIdToRoomIds: Record = {}; +): Promise => + updateMDirect(mx, (current) => withDirectEntries(current, [{ roomId, userId }])); - if (typeof mDirectsEvent !== 'undefined') - userIdToRoomIds = structuredClone(mDirectsEvent.getContent()); - - // remove it from the lists of any others users - // (it can only be a DM room for one person) - Object.keys(userIdToRoomIds).forEach((targetUserId) => { - const roomIds = userIdToRoomIds[targetUserId]; - - if (targetUserId !== userId) { - const indexOfRoomId = roomIds.indexOf(roomId); - if (indexOfRoomId > -1) { - roomIds.splice(indexOfRoomId, 1); - } - } +/** + * The one filer of direct messages. Files every joined room whose invite was + * marked is_direct and which m.direct does not list, in a single write, once + * the other party is known. Run on startup and on every sync: a DM that + * arrives late, in a burst or after a reload is filed by the same check. + */ +export const fileDirectRooms = (mx: MatrixClient): Promise => { + const me = mx.getSafeUserId(); + return updateMDirect(mx, (current) => { + const filed = new Set(Object.values(current).flat()); + const entries = mx + .getRooms() + .filter( + (room: Room) => + room.getMyMembership() === 'join' && + !filed.has(room.roomId) && + wasInvitedAsDirect(room, me) + ) + .map((room: Room) => ({ roomId: room.roomId, userId: guessDmRoomUserId(room, me) })) + // The other party's membership has not synced yet; a later sync files it. + .filter((entry: DirectEntry) => entry.userId !== me); + return entries.length > 0 ? withDirectEntries(current, entries) : undefined; }); - - const roomIds = userIdToRoomIds[userId] || []; - if (roomIds.indexOf(roomId) === -1) { - roomIds.push(roomId); - } - userIdToRoomIds[userId] = roomIds; - - await mx.setAccountData(AccountDataEvent.Direct as any, userIdToRoomIds as any); }; -export const removeRoomIdFromMDirect = async (mx: MatrixClient, roomId: string): Promise => { - const mDirectsEvent = mx.getAccountData(AccountDataEvent.Direct as any); - let userIdToRoomIds: Record = {}; - - if (typeof mDirectsEvent !== 'undefined') - userIdToRoomIds = structuredClone(mDirectsEvent.getContent()); - - Object.keys(userIdToRoomIds).forEach((targetUserId) => { - const roomIds = userIdToRoomIds[targetUserId]; - const indexOfRoomId = roomIds.indexOf(roomId); - if (indexOfRoomId > -1) { - roomIds.splice(indexOfRoomId, 1); - } +export const removeRoomIdFromMDirect = (mx: MatrixClient, roomId: string): Promise => + updateMDirect(mx, (current) => { + const next: MDirectContent = {}; + Object.keys(current).forEach((userId) => { + next[userId] = current[userId].filter((id) => id !== roomId); + }); + return next; }); - await mx.setAccountData(AccountDataEvent.Direct as any, userIdToRoomIds as any); -}; - export const mxcUrlToHttp = ( mx: MatrixClient, mxcUrl: string, diff --git a/src/app/utils/room.ts b/src/app/utils/room.ts index 05dfeb29..d4cd757c 100644 --- a/src/app/utils/room.ts +++ b/src/app/utils/room.ts @@ -72,6 +72,18 @@ export const isDirectInvite = (room: Room | null, myUserId: string | null): bool return content?.is_direct === true; }; +/** + * Whether the invite that brought this user into the room was marked is_direct. + * The join event keeps the mark in its own content when the server copies it, + * and otherwise in the invite it replaced (unsigned.prev_content). + */ +export const wasInvitedAsDirect = (room: Room, myUserId: string): boolean => { + const event = room.getMember(myUserId)?.events?.member; + if (event?.getContent()?.is_direct === true) return true; + const prev = event?.getPrevContent(); + return prev?.membership === 'invite' && prev.is_direct === true; +}; + export const isSpace = (room: Room | null): boolean => { if (!room) return false; const event = getStateEvent(room, StateEvent.RoomCreate);