From 7f3cc5fc2db6283e05a9fec600b943442279be18 Mon Sep 17 00:00:00 2001 From: Diego Mello Date: Thu, 27 Aug 2026 16:01:56 -0300 Subject: [PATCH 1/3] fix: settle every 2FA request and treat cancellation as neutral The 2FA service now owns the single active request and exposes one idempotent cancelActiveRequest, used by explicit Cancel, by request replacement, and by presenter cleanup. A request whose presenter is removed no longer leaves the initiating action loading forever. Presenter absence gets its own TwoFactorUnavailableError so a systemic failure stays a real failure, while user cancellation keeps rejecting with TwoFactorCancelledError. Cancellation policy moves to the user-action boundary: runCancellableAction returns completed | cancelled and rethrows every other error unchanged. SDK and service operations still reject, so existing error cleanup and composition are untouched. Canonical logging and presentation suppress typed cancellation, so tapping Cancel produces no failure telemetry and no operational error. --- app/containers/TwoFactor/index.test.tsx | 32 ++++++ app/containers/TwoFactor/index.tsx | 44 ++------ .../helpers/handleSaveUserProfileError.ts | 37 +++---- app/lib/methods/helpers/info.ts | 25 +++-- app/lib/services/twoFactor/twoFactor.ts | 46 +++++++- .../twoFactor/twoFactorCancellation.test.ts | 6 ++ .../twoFactor/twoFactorOutcome.test.ts | 20 ++++ .../services/twoFactor/twoFactorOutcome.ts | 14 +++ .../twoFactor/twoFactorRequest.test.ts | 83 ++++++++++++++ .../twoFactor/twoFactorUnavailable.ts | 9 ++ app/sagas/login.js | 12 +-- app/views/ChangeAvatarView/index.tsx | 28 ++--- app/views/ChangePasswordView/index.tsx | 15 +-- .../E2EEToggleRoomView/resetRoomKey.test.ts | 102 ++++++++++++++++++ app/views/E2EEToggleRoomView/resetRoomKey.ts | 33 +++--- .../ChangePassword.tsx | 10 +- app/views/E2EEncryptionSecurityView/index.tsx | 12 +-- .../ConfirmDeleteAccountContent.tsx | 12 +-- .../DeleteAccountActionSheetContent/index.tsx | 10 +- app/views/ProfileView/index.test.tsx | 18 ++++ app/views/ProfileView/index.tsx | 18 ++-- .../methods/logoutOtherLocations.test.ts | 68 ++++++++++++ .../methods/logoutOtherLocations.ts | 10 +- app/views/SetUsernameView.tsx | 12 +-- 24 files changed, 525 insertions(+), 151 deletions(-) create mode 100644 app/lib/services/twoFactor/twoFactorOutcome.test.ts create mode 100644 app/lib/services/twoFactor/twoFactorOutcome.ts create mode 100644 app/lib/services/twoFactor/twoFactorRequest.test.ts create mode 100644 app/lib/services/twoFactor/twoFactorUnavailable.ts create mode 100644 app/views/E2EEToggleRoomView/resetRoomKey.test.ts create mode 100644 app/views/ProfileView/methods/logoutOtherLocations.test.ts diff --git a/app/containers/TwoFactor/index.test.tsx b/app/containers/TwoFactor/index.test.tsx index 21826e6a0b..45a235ed87 100644 --- a/app/containers/TwoFactor/index.test.tsx +++ b/app/containers/TwoFactor/index.test.tsx @@ -36,4 +36,36 @@ describe('TwoFactor', () => { await expect(newest!).resolves.toEqual({ twoFactorCode: '123456', twoFactorMethod: 'totp' }); }); + + it('rejects the pending request when the presenter is removed', async () => { + const { getByTestId, unmount } = render(); + + let pending: Promise | undefined; + await act(() => { + pending = requestTwoFactor().catch(error => error); + }); + + await waitFor(() => expect(getByTestId('two-factor-input')).toBeTruthy()); + + unmount(); + + expect(isTwoFactorCancelled(await pending!)).toBe(true); + }); + + it('rejects the pending request when Cancel is pressed', async () => { + const { getByTestId, getByText } = render(); + + let pending: Promise | undefined; + await act(() => { + pending = requestTwoFactor().catch(error => error); + }); + + await waitFor(() => expect(getByTestId('two-factor-input')).toBeTruthy()); + + await act(() => { + fireEvent.press(getByText('Cancel')); + }); + + expect(isTwoFactorCancelled(await pending!)).toBe(true); + }); }); diff --git a/app/containers/TwoFactor/index.tsx b/app/containers/TwoFactor/index.tsx index 21b80e26cf..23b49b725a 100644 --- a/app/containers/TwoFactor/index.tsx +++ b/app/containers/TwoFactor/index.tsx @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState, memo } from 'react'; +import { useEffect, useState, memo } from 'react'; import { AccessibilityInfo, Text, View } from 'react-native'; import isEmpty from 'lodash/isEmpty'; import { sha256 } from 'js-sha256'; @@ -11,18 +11,16 @@ import { GestureHandlerRootView } from 'react-native-gesture-handler'; import { ControlledFormTextInput } from '../TextInput'; import I18n from '../../i18n'; -import EventEmitter from '../../lib/methods/helpers/events'; import { useTheme } from '../../theme'; import Button from '../Button'; import sharedStyles from '../../views/Styles'; import styles from './styles'; -import { type ILoginCredentials } from '../../definitions'; import { sendEmailCode } from '../../lib/services/restApi'; import { useMasterDetail } from '../../lib/hooks/useMasterDetail'; import Toast from '../Toast'; import { showToast } from '../../lib/methods/helpers/showToast'; import log from '../../lib/methods/helpers/log'; -import { TWO_FACTOR } from '../../lib/constants/twoFactor'; +import { cancelActiveRequest, type ITwoFactorPrompt, subscribeToTwoFactorPrompts } from '../../lib/services/twoFactor/twoFactor'; interface IMethodsProp { text: string; @@ -30,21 +28,7 @@ interface IMethodsProp { title?: string; secureTextEntry?: boolean; } -interface IMethods { - totp: IMethodsProp; - email: IMethodsProp; - password: IMethodsProp; -} - -interface EventListenerMethod { - params?: ILoginCredentials; - method?: keyof IMethods; - submit?: (param: string) => void; - cancel?: () => void; - invalid?: boolean; -} - -const methods: IMethods = { +const methods: Record = { totp: { text: 'Open_your_authentication_app_and_enter_the_code', keyboardType: 'numeric' @@ -68,8 +52,7 @@ const TwoFactor = memo(() => { const { colors } = useTheme(); const isMasterDetail = useMasterDetail(); const [visible, setVisible] = useState(false); - const [data, setData] = useState({}); - const pendingCancel = useRef(undefined); + const [data, setData] = useState>({}); const { control, setValue, @@ -113,9 +96,7 @@ const TwoFactor = memo(() => { } }, [data]); - const showTwoFactor = (args: EventListenerMethod) => { - pendingCancel.current?.(); - pendingCancel.current = args.cancel; + const showTwoFactor = (args: ITwoFactorPrompt) => { setData(args); if (args.invalid) { setError('code', { message: I18n.t('Invalid_code'), type: 'validate' }); @@ -123,24 +104,15 @@ const TwoFactor = memo(() => { } }; - useEffect(() => { - const listener = EventEmitter.addEventListener(TWO_FACTOR, showTwoFactor); - - return () => EventEmitter.removeListener(TWO_FACTOR, listener); - }, []); + useEffect(() => subscribeToTwoFactorPrompts(showTwoFactor), []); const onCancel = () => { - const { cancel } = data; - pendingCancel.current = undefined; - if (cancel) { - cancel(); - } + cancelActiveRequest(); setData({}); }; const onSubmit = () => { const { submit } = data; - pendingCancel.current = undefined; if (submit) { const { code } = getValues(); if (data.method === 'password') { @@ -148,6 +120,8 @@ const TwoFactor = memo(() => { } else { submit(code); } + } else { + cancelActiveRequest(); } clearErrors(); setData({}); diff --git a/app/lib/methods/helpers/handleSaveUserProfileError.ts b/app/lib/methods/helpers/handleSaveUserProfileError.ts index cc80e73502..b09f2e9b7c 100644 --- a/app/lib/methods/helpers/handleSaveUserProfileError.ts +++ b/app/lib/methods/helpers/handleSaveUserProfileError.ts @@ -1,24 +1,21 @@ import I18n from '../../../i18n'; -import { showErrorAlert } from '.'; -import { isTwoFactorCancelled } from '../../services/twoFactor/twoFactorCancelled'; +import { presentUnlessCancelled, showErrorAlert } from '.'; -const handleSaveUserProfileError = (e: any, action: string) => { - if (isTwoFactorCancelled(e)) { - return; - } - if (e.data && e.data.error.includes('[error-too-many-requests]')) { - return showErrorAlert(e.data.error); - } - if (I18n.isTranslated(e.error)) { - return showErrorAlert(I18n.t(e.error)); - } - let msg = I18n.t('There_was_an_error_while_action', { action: I18n.t(action) }); - let title = ''; - if (typeof e.reason === 'string') { - title = msg; - msg = e.reason; - } - showErrorAlert(msg, title); -}; +const handleSaveUserProfileError = (e: any, action: string) => + presentUnlessCancelled(e, () => { + if (e.data && e.data.error.includes('[error-too-many-requests]')) { + return showErrorAlert(e.data.error); + } + if (I18n.isTranslated(e.error)) { + return showErrorAlert(I18n.t(e.error)); + } + let msg = I18n.t('There_was_an_error_while_action', { action: I18n.t(action) }); + let title = ''; + if (typeof e.reason === 'string') { + title = msg; + msg = e.reason; + } + showErrorAlert(msg, title); + }); export default handleSaveUserProfileError; diff --git a/app/lib/methods/helpers/info.ts b/app/lib/methods/helpers/info.ts index 12f7fc9ece..d0a7496c20 100644 --- a/app/lib/methods/helpers/info.ts +++ b/app/lib/methods/helpers/info.ts @@ -6,21 +6,26 @@ import { isTwoFactorCancelled } from '../../services/twoFactor/twoFactorCancelle export const showErrorAlert = (message: string, title?: string, onPress = () => {}): void => Alert.alert(title || '', message, [{ text: 'OK', onPress }], { cancelable: true }); -export const showErrorAlertWithEMessage = (e: any, title?: string): void => { +export const presentUnlessCancelled = (e: unknown, present: () => void): void => { if (isTwoFactorCancelled(e)) { return; } - let errorMessage: string = e?.data?.error; + present(); +}; - if (errorMessage?.includes('[error-too-many-requests]')) { - const seconds = errorMessage.replace(/\D/g, ''); - errorMessage = I18n.t('error-too-many-requests', { seconds }); - } else { - errorMessage = I18n.isTranslated(errorMessage) ? I18n.t(errorMessage) : errorMessage; - } +export const showErrorAlertWithEMessage = (e: any, title?: string): void => + presentUnlessCancelled(e, () => { + let errorMessage: string = e?.data?.error; - showErrorAlert(errorMessage, title); -}; + if (errorMessage?.includes('[error-too-many-requests]')) { + const seconds = errorMessage.replace(/\D/g, ''); + errorMessage = I18n.t('error-too-many-requests', { seconds }); + } else { + errorMessage = I18n.isTranslated(errorMessage) ? I18n.t(errorMessage) : errorMessage; + } + + showErrorAlert(errorMessage, title); + }); interface IShowConfirmationAlert { title?: string; diff --git a/app/lib/services/twoFactor/twoFactor.ts b/app/lib/services/twoFactor/twoFactor.ts index d382f591d5..9dea8efbae 100644 --- a/app/lib/services/twoFactor/twoFactor.ts +++ b/app/lib/services/twoFactor/twoFactor.ts @@ -4,6 +4,7 @@ import { TWO_FACTOR } from '../../constants/twoFactor'; import EventEmitter from '../../methods/helpers/events'; import { type ILoginCredentials } from '../../../definitions'; import { TwoFactorCancelledError } from './twoFactorCancelled'; +import { TwoFactorUnavailableError } from './twoFactorUnavailable'; interface ITwoFactor { method: string; @@ -11,14 +12,57 @@ interface ITwoFactor { params?: ILoginCredentials; } +export interface ITwoFactorPrompt { + method: string; + invalid: boolean; + params?: ILoginCredentials; + submit: (code: string) => void; +} + +let activeRequest: { reject: (error: Error) => void } | null = null; +let presenters = 0; + +export const cancelActiveRequest = () => { + const request = activeRequest; + activeRequest = null; + request?.reject(new TwoFactorCancelledError()); +}; + +export const subscribeToTwoFactorPrompts = (present: (prompt: ITwoFactorPrompt) => void) => { + const listener = EventEmitter.addEventListener(TWO_FACTOR, present); + presenters += 1; + + return () => { + EventEmitter.removeListener(TWO_FACTOR, listener); + presenters -= 1; + queueMicrotask(() => { + if (presenters === 0) { + cancelActiveRequest(); + } + }); + }; +}; + export const twoFactor = ({ method, invalid, params }: ITwoFactor): Promise<{ twoFactorCode: string; twoFactorMethod: string }> => new Promise((resolve, reject) => { + cancelActiveRequest(); + + if (presenters === 0) { + reject(new TwoFactorUnavailableError()); + return; + } + + const request = { reject }; + activeRequest = request; + EventEmitter.emit(TWO_FACTOR, { method, invalid, params, - cancel: () => reject(new TwoFactorCancelledError()), submit: (code: string) => { + if (activeRequest === request) { + activeRequest = null; + } settings.customHeaders = { ...settings.customHeaders, 'x-2fa-code': code, diff --git a/app/lib/services/twoFactor/twoFactorCancellation.test.ts b/app/lib/services/twoFactor/twoFactorCancellation.test.ts index 0151025e9b..4b9d667645 100644 --- a/app/lib/services/twoFactor/twoFactorCancellation.test.ts +++ b/app/lib/services/twoFactor/twoFactorCancellation.test.ts @@ -4,6 +4,7 @@ import bugsnag from '@bugsnag/react-native'; import log from '../../methods/helpers/log'; import { showErrorAlertWithEMessage } from '../../methods/helpers/info'; import handleSaveUserProfileError from '../../methods/helpers/handleSaveUserProfileError'; +import { handleError } from '../../../views/ChangeAvatarView/submitHelpers'; import { handleLoginErrors } from '../../../views/LoginView/handleLoginErrors'; import { TwoFactorCancelledError } from './twoFactorCancelled'; @@ -58,6 +59,11 @@ describe('two-factor cancellation', () => { expect(Alert.alert).toHaveBeenCalled(); }); + it('preserves the typed cancellation through non-presenting error translation', () => { + expect(() => handleError(cancelled, 'changing_avatar')).toThrow(cancelled); + expect(Alert.alert).not.toHaveBeenCalled(); + }); + it('surfaces a generic login error when the login path reports a cancellation', () => { expect(handleLoginErrors((cancelled as any).error)).toBe('Login_error'); }); diff --git a/app/lib/services/twoFactor/twoFactorOutcome.test.ts b/app/lib/services/twoFactor/twoFactorOutcome.test.ts new file mode 100644 index 0000000000..8d664b6f1c --- /dev/null +++ b/app/lib/services/twoFactor/twoFactorOutcome.test.ts @@ -0,0 +1,20 @@ +import { TwoFactorCancelledError } from './twoFactorCancelled'; +import { runCancellableAction } from './twoFactorOutcome'; + +describe('runCancellableAction', () => { + it('returns the successful value as completed', async () => { + await expect(runCancellableAction(() => Promise.resolve('code'))).resolves.toEqual({ status: 'completed', value: 'code' }); + }); + + it('returns cancelled for a typed cancellation', async () => { + await expect(runCancellableAction(() => Promise.reject(new TwoFactorCancelledError()))).resolves.toEqual({ + status: 'cancelled' + }); + }); + + it('rethrows every other error unchanged', async () => { + const failure = { data: { error: 'error-invalid-password' } }; + + await expect(runCancellableAction(() => Promise.reject(failure))).rejects.toBe(failure); + }); +}); diff --git a/app/lib/services/twoFactor/twoFactorOutcome.ts b/app/lib/services/twoFactor/twoFactorOutcome.ts new file mode 100644 index 0000000000..50b82c9cb2 --- /dev/null +++ b/app/lib/services/twoFactor/twoFactorOutcome.ts @@ -0,0 +1,14 @@ +import { isTwoFactorCancelled } from './twoFactorCancelled'; + +export type TwoFactorOutcome = { status: 'completed'; value: T } | { status: 'cancelled' }; + +export const runCancellableAction = async (action: () => Promise): Promise> => { + try { + return { status: 'completed', value: await action() }; + } catch (e) { + if (isTwoFactorCancelled(e)) { + return { status: 'cancelled' }; + } + throw e; + } +}; diff --git a/app/lib/services/twoFactor/twoFactorRequest.test.ts b/app/lib/services/twoFactor/twoFactorRequest.test.ts new file mode 100644 index 0000000000..2708d305c1 --- /dev/null +++ b/app/lib/services/twoFactor/twoFactorRequest.test.ts @@ -0,0 +1,83 @@ +import { cancelActiveRequest, type ITwoFactorPrompt, subscribeToTwoFactorPrompts, twoFactor } from './twoFactor'; +import { isTwoFactorCancelled } from './twoFactorCancelled'; +import { isTwoFactorUnavailable } from './twoFactorUnavailable'; + +describe('two-factor request lifecycle', () => { + let prompts: ITwoFactorPrompt[]; + let unsubscribe: () => void; + + const request = () => twoFactor({ method: 'totp', invalid: false }); + + beforeEach(() => { + prompts = []; + unsubscribe = subscribeToTwoFactorPrompts(prompt => prompts.push(prompt)); + }); + + afterEach(() => { + unsubscribe(); + }); + + it('cancels the displaced request and keeps the newest one active', async () => { + const displaced = request(); + const newest = request(); + + await expect(displaced.catch(isTwoFactorCancelled)).resolves.toBe(true); + + prompts[1].submit('123456'); + await expect(newest).resolves.toEqual({ twoFactorCode: '123456', twoFactorMethod: 'totp' }); + }); + + it('rejects the pending request with a typed cancellation', async () => { + const pending = request(); + cancelActiveRequest(); + + await expect(pending.catch(isTwoFactorCancelled)).resolves.toBe(true); + }); + + it('is harmless when cancelled repeatedly', async () => { + const pending = request(); + cancelActiveRequest(); + + await expect(pending.catch(isTwoFactorCancelled)).resolves.toBe(true); + expect(() => { + cancelActiveRequest(); + cancelActiveRequest(); + }).not.toThrow(); + }); + + it('is harmless when cancelled after a successful submission', async () => { + const pending = request(); + prompts[0].submit('123456'); + + await expect(pending).resolves.toEqual({ twoFactorCode: '123456', twoFactorMethod: 'totp' }); + + cancelActiveRequest(); + await expect(pending).resolves.toEqual({ twoFactorCode: '123456', twoFactorMethod: 'totp' }); + }); + + it('rejects as unavailable, not cancelled, when no presenter is subscribed', async () => { + unsubscribe(); + + const pending = request(); + const error = await pending.catch(e => e); + + expect(isTwoFactorUnavailable(error)).toBe(true); + expect(isTwoFactorCancelled(error)).toBe(false); + expect(prompts).toHaveLength(0); + + unsubscribe = subscribeToTwoFactorPrompts(prompt => prompts.push(prompt)); + expect(prompts).toHaveLength(0); + }); + + it('keeps the pending request alive across a presenter remount', async () => { + const pending = request(); + + unsubscribe(); + unsubscribe = subscribeToTwoFactorPrompts(prompt => prompts.push(prompt)); + + await Promise.resolve(); + + prompts[0].submit('123456'); + await expect(pending).resolves.toEqual({ twoFactorCode: '123456', twoFactorMethod: 'totp' }); + }); +}); diff --git a/app/lib/services/twoFactor/twoFactorUnavailable.ts b/app/lib/services/twoFactor/twoFactorUnavailable.ts new file mode 100644 index 0000000000..8a7d007503 --- /dev/null +++ b/app/lib/services/twoFactor/twoFactorUnavailable.ts @@ -0,0 +1,9 @@ +export class TwoFactorUnavailableError extends Error { + constructor() { + super('Two-factor authentication prompt is unavailable'); + this.name = 'TwoFactorUnavailableError'; + } +} + +export const isTwoFactorUnavailable = (e: unknown): e is TwoFactorUnavailableError => + e instanceof TwoFactorUnavailableError || (e instanceof Error && e.name === 'TwoFactorUnavailableError'); diff --git a/app/sagas/login.js b/app/sagas/login.js index 3c7160cf63..933ac69ac3 100644 --- a/app/sagas/login.js +++ b/app/sagas/login.js @@ -29,7 +29,7 @@ import { getIsMasterDetail } from '../lib/hooks/useMasterDetail'; import { getEnterpriseModules, isOmnichannelModuleAvailable, isVoipModuleAvailable } from '../lib/methods/enterpriseModules'; import { getPermissions } from '../lib/methods/getPermissions'; import { getRoles } from '../lib/methods/getRoles'; -import { isTwoFactorCancelled } from '../lib/services/twoFactor/twoFactorCancelled'; +import { runCancellableAction } from '../lib/services/twoFactor/twoFactorOutcome'; import { getSlashCommands } from '../lib/methods/getSlashCommands'; import { getUserPresence, refreshDmUsersPresence, subscribeUsersPresence } from '../lib/methods/getUsersPresence'; import { logout, removeServerData, removeServerDatabase } from '../lib/methods/logout'; @@ -125,13 +125,9 @@ const handleLoginRequest = function* handleLoginRequest({ credentials, logoutOnE }); yield put(loginSuccess(result)); if (registerCustomFields) { - try { - const updatedUser = yield call(saveUserProfile, {}, { ...registerCustomFields }); - yield put(setUser({ ...result, ...updatedUser.user })); - } catch (e) { - if (!isTwoFactorCancelled(e)) { - throw e; - } + const outcome = yield call(runCancellableAction, () => saveUserProfile({}, { ...registerCustomFields })); + if (outcome.status === 'completed') { + yield put(setUser({ ...result, ...outcome.value.user })); } } } diff --git a/app/views/ChangeAvatarView/index.tsx b/app/views/ChangeAvatarView/index.tsx index d3a9dc57bd..4cd6a38400 100644 --- a/app/views/ChangeAvatarView/index.tsx +++ b/app/views/ChangeAvatarView/index.tsx @@ -29,7 +29,7 @@ import ImagePicker, { type Image } from '../../lib/methods/helpers/ImagePicker/I import { compareServerVersion, isImageURL, useDebounce } from '../../lib/methods/helpers'; import { ControlledFormTextInput } from '../../containers/TextInput'; import { HeaderBackButton } from '../../containers/Header/components/HeaderBackButton'; -import { isTwoFactorCancelled } from '../../lib/services/twoFactor/twoFactorCancelled'; +import { runCancellableAction } from '../../lib/services/twoFactor/twoFactorOutcome'; enum AvatarStateActions { CHANGE_AVATAR = 'CHANGE_AVATAR', @@ -161,21 +161,23 @@ const ChangeAvatarView = () => { const submit = async () => { try { setSaving(true); - if (context === 'room' && room?.rid) { - // Change Rooms Avatar - await changeRoomsAvatar(room.rid, state?.data); - } else if (state?.url) { - // Change User's Avatar - await changeUserAvatar(state); - } else if (state.resetUserAvatar) { - // Change User's Avatar - await resetUserAvatar(userId); + const outcome = await runCancellableAction(async () => { + if (context === 'room' && room?.rid) { + // Change Rooms Avatar + await changeRoomsAvatar(room.rid, state?.data); + } else if (state?.url) { + // Change User's Avatar + await changeUserAvatar(state); + } else if (state.resetUserAvatar) { + // Change User's Avatar + await resetUserAvatar(userId); + } + }); + if (outcome.status === 'cancelled') { + return; } isDirty.current = false; } catch (e: any) { - if (isTwoFactorCancelled(e)) { - return; - } log(e); return showErrorAlert(e.message, I18n.t('Oops')); } finally { diff --git a/app/views/ChangePasswordView/index.tsx b/app/views/ChangePasswordView/index.tsx index 4a679417fb..cdf4bff870 100644 --- a/app/views/ChangePasswordView/index.tsx +++ b/app/views/ChangePasswordView/index.tsx @@ -8,7 +8,7 @@ import { useDispatch } from 'react-redux'; import { sha256 } from 'js-sha256'; import { twoFactor } from '../../lib/services/twoFactor/twoFactor'; -import { isTwoFactorCancelled } from '../../lib/services/twoFactor/twoFactorCancelled'; +import { runCancellableAction } from '../../lib/services/twoFactor/twoFactorOutcome'; import { type ProfileStackParamList } from '../../stacks/types'; import { ControlledFormTextInput } from '../../containers/TextInput'; import { useAppSelector } from '../../lib/hooks/useAppSelector'; @@ -149,14 +149,17 @@ const ChangePasswordView = ({ navigation }: IChangePasswordViewProps) => { } catch (e: any) { if (e?.error === 'totp-invalid' && e?.details.method !== TwoFactorMethods.PASSWORD) { try { - const code = await twoFactor({ method: e.details.method, invalid: e?.error === 'totp-invalid' && !!twoFactorCode }); - setTwoFactorCode(code as any); + const outcome = await runCancellableAction(() => + twoFactor({ method: e.details.method, invalid: e?.error === 'totp-invalid' && !!twoFactorCode }) + ); + if (outcome.status === 'cancelled') { + resetTwoFactorState(); + return; + } + setTwoFactorCode(outcome.value as any); return handleSetNewPassword(); } catch (twoFactorError) { resetTwoFactorState(); - if (isTwoFactorCancelled(twoFactorError)) { - return; - } return handleSaveUserProfileError(twoFactorError, 'saving_profile'); } } diff --git a/app/views/E2EEToggleRoomView/resetRoomKey.test.ts b/app/views/E2EEToggleRoomView/resetRoomKey.test.ts new file mode 100644 index 0000000000..308dd0c2a4 --- /dev/null +++ b/app/views/E2EEToggleRoomView/resetRoomKey.test.ts @@ -0,0 +1,102 @@ +import { Alert } from 'react-native'; + +import { Encryption } from '../../lib/encryption'; +import log from '../../lib/methods/helpers/log'; +import { showToast } from '../../lib/methods/helpers/showToast'; +import { e2eResetRoomKey } from '../../lib/services/restApi'; +import { TwoFactorCancelledError } from '../../lib/services/twoFactor/twoFactorCancelled'; +import { resetRoomKey } from './resetRoomKey'; + +jest.mock('../../i18n', () => ({ + t: (key: string) => key, + isTranslated: () => true +})); + +jest.mock('../../lib/encryption', () => ({ + Encryption: { getRoomInstance: jest.fn() } +})); + +jest.mock('../../lib/methods/helpers/log', () => jest.fn()); + +jest.mock('../../lib/methods/helpers/showToast', () => ({ + showToast: jest.fn() +})); + +jest.mock('../../lib/services/restApi', () => ({ + e2eResetRoomKey: jest.fn() +})); + +const roomKey = { e2eKey: 'key', e2eKeyId: 'key-id' }; + +const mockRoom = (resetRoomKeyImplementation: jest.Mock) => { + (Encryption.getRoomInstance as jest.Mock).mockResolvedValue({ resetRoomKey: resetRoomKeyImplementation }); +}; + +const confirm = () => { + const [, , buttons] = (Alert.alert as jest.Mock).mock.calls[0]; + return buttons[1].onPress(); +}; + +describe('resetRoomKey', () => { + beforeEach(() => { + jest.clearAllMocks(); + jest.spyOn(Alert, 'alert').mockImplementation(() => {}); + jest.spyOn(console, 'log').mockImplementation(() => {}); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('confirms the reset when every step completes', async () => { + mockRoom(jest.fn().mockResolvedValue(roomKey)); + (e2eResetRoomKey as jest.Mock).mockResolvedValue({ success: true }); + + resetRoomKey('rid'); + await confirm(); + + expect(showToast).toHaveBeenCalledWith('Encryption_keys_reset'); + }); + + it('shows no error when the local key reset is cancelled', async () => { + mockRoom(jest.fn().mockRejectedValue(new TwoFactorCancelledError())); + + resetRoomKey('rid'); + await confirm(); + + expect(showToast).not.toHaveBeenCalled(); + expect(log).not.toHaveBeenCalled(); + }); + + it('shows no error when looking the room up is cancelled', async () => { + (Encryption.getRoomInstance as jest.Mock).mockRejectedValue(new TwoFactorCancelledError()); + + resetRoomKey('rid'); + await confirm(); + + expect(showToast).not.toHaveBeenCalled(); + expect(log).not.toHaveBeenCalled(); + }); + + it('shows no error when the server reset is cancelled', async () => { + mockRoom(jest.fn().mockResolvedValue(roomKey)); + (e2eResetRoomKey as jest.Mock).mockRejectedValue(new TwoFactorCancelledError()); + + resetRoomKey('rid'); + await confirm(); + + expect(showToast).not.toHaveBeenCalled(); + expect(log).not.toHaveBeenCalled(); + }); + + it('still reports a genuine failure', async () => { + mockRoom(jest.fn().mockResolvedValue(roomKey)); + (e2eResetRoomKey as jest.Mock).mockRejectedValue(new Error('boom')); + + resetRoomKey('rid'); + await confirm(); + + expect(log).toHaveBeenCalled(); + expect(showToast).toHaveBeenCalledWith('Encryption_keys_failed'); + }); +}); diff --git a/app/views/E2EEToggleRoomView/resetRoomKey.ts b/app/views/E2EEToggleRoomView/resetRoomKey.ts index 767260227b..dd597f7445 100644 --- a/app/views/E2EEToggleRoomView/resetRoomKey.ts +++ b/app/views/E2EEToggleRoomView/resetRoomKey.ts @@ -5,7 +5,7 @@ import { Encryption } from '../../lib/encryption'; import log from '../../lib/methods/helpers/log'; import { showToast } from '../../lib/methods/helpers/showToast'; import { e2eResetRoomKey } from '../../lib/services/restApi'; -import { isTwoFactorCancelled } from '../../lib/services/twoFactor/twoFactorCancelled'; +import { runCancellableAction } from '../../lib/services/twoFactor/twoFactorOutcome'; export const resetRoomKey = (rid: string) => { Alert.alert( @@ -21,24 +21,27 @@ export const resetRoomKey = (rid: string) => { style: 'destructive', onPress: async () => { try { - const e2eRoom = await Encryption.getRoomInstance(rid); - if (!e2eRoom) { - console.log('Encryption room instance not found'); - return; - } + const outcome = await runCancellableAction(async () => { + const e2eRoom = await Encryption.getRoomInstance(rid); + if (!e2eRoom) { + console.log('Encryption room instance not found'); + return false; + } - const { e2eKey, e2eKeyId } = (await e2eRoom.resetRoomKey()) ?? {}; + const { e2eKey, e2eKeyId } = (await e2eRoom.resetRoomKey()) ?? {}; - if (!e2eKey || !e2eKeyId) { - return; - } + if (!e2eKey || !e2eKeyId) { + return false; + } - await e2eResetRoomKey(rid, e2eKey, e2eKeyId); - showToast(I18n.t('Encryption_keys_reset')); - } catch (e) { - if (isTwoFactorCancelled(e)) { - return; + await e2eResetRoomKey(rid, e2eKey, e2eKeyId); + return true; + }); + + if (outcome.status === 'completed' && outcome.value) { + showToast(I18n.t('Encryption_keys_reset')); } + } catch (e) { log(e); showToast(I18n.t('Encryption_keys_failed')); } diff --git a/app/views/E2EEncryptionSecurityView/ChangePassword.tsx b/app/views/E2EEncryptionSecurityView/ChangePassword.tsx index 27086b6c0b..85d0b5f3c7 100644 --- a/app/views/E2EEncryptionSecurityView/ChangePassword.tsx +++ b/app/views/E2EEncryptionSecurityView/ChangePassword.tsx @@ -8,7 +8,7 @@ import log, { events, logEvent } from '../../lib/methods/helpers/log'; import { FormTextInput } from '../../containers/TextInput'; import Button from '../../containers/Button'; import { Encryption } from '../../lib/encryption'; -import { isTwoFactorCancelled } from '../../lib/services/twoFactor/twoFactorCancelled'; +import { runCancellableAction } from '../../lib/services/twoFactor/twoFactorOutcome'; import { showConfirmationAlert, showErrorAlert } from '../../lib/methods/helpers/info'; import EventEmitter from '../../lib/methods/helpers/events'; import { LISTENER } from '../../containers/Toast'; @@ -44,14 +44,14 @@ const ChangePassword = () => { onPress: async () => { logEvent(events.E2E_SEC_CHANGE_PASSWORD); try { - await Encryption.changePassword(server, newPassword); + const outcome = await runCancellableAction(() => Encryption.changePassword(server, newPassword)); + if (outcome.status === 'cancelled') { + return; + } EventEmitter.emit(LISTENER, { message: I18n.t('E2E_encryption_change_password_success') }); newPasswordInputRef?.current?.clear(); newPasswordInputRef?.current?.blur(); } catch (e) { - if (isTwoFactorCancelled(e)) { - return; - } log(e); showErrorAlert(I18n.t('E2E_encryption_change_password_error')); } diff --git a/app/views/E2EEncryptionSecurityView/index.tsx b/app/views/E2EEncryptionSecurityView/index.tsx index 5af6ade1dc..a6c6f67e61 100644 --- a/app/views/E2EEncryptionSecurityView/index.tsx +++ b/app/views/E2EEncryptionSecurityView/index.tsx @@ -13,7 +13,7 @@ import Button from '../../containers/Button'; import { logout } from '../../actions/login'; import { showConfirmationAlert, showErrorAlert } from '../../lib/methods/helpers/info'; import { e2eResetOwnKey } from '../../lib/services/restApi'; -import { isTwoFactorCancelled } from '../../lib/services/twoFactor/twoFactorCancelled'; +import { runCancellableAction } from '../../lib/services/twoFactor/twoFactorOutcome'; import { type SettingsStackParamList } from '../../stacks/types'; import ChangePassword from './ChangePassword'; import { styles } from './styles'; @@ -37,15 +37,15 @@ const E2EEncryptionSecurityView = () => { onPress: async () => { logEvent(events.E2E_SEC_RESET_OWN_KEY); try { - const res = await e2eResetOwnKey(); + const outcome = await runCancellableAction(e2eResetOwnKey); + if (outcome.status === 'cancelled') { + return; + } - if (res?.success === true) { + if (outcome.value?.success === true) { dispatch(logout()); } } catch (e) { - if (isTwoFactorCancelled(e)) { - return; - } log(e); showErrorAlert(I18n.t('E2E_encryption_reset_error')); } diff --git a/app/views/ProfileView/components/DeleteAccountActionSheetContent/ConfirmDeleteAccountContent.tsx b/app/views/ProfileView/components/DeleteAccountActionSheetContent/ConfirmDeleteAccountContent.tsx index 817022d8ef..062771a042 100644 --- a/app/views/ProfileView/components/DeleteAccountActionSheetContent/ConfirmDeleteAccountContent.tsx +++ b/app/views/ProfileView/components/DeleteAccountActionSheetContent/ConfirmDeleteAccountContent.tsx @@ -7,7 +7,7 @@ import sharedStyles from '../../../Styles'; import FooterButtons from './FooterButtons'; import AlertText from './AlertText'; import { deleteOwnAccount } from '../../../../lib/services/restApi'; -import { isTwoFactorCancelled } from '../../../../lib/services/twoFactor/twoFactorCancelled'; +import { runCancellableAction } from '../../../../lib/services/twoFactor/twoFactorOutcome'; import { deleteAccount } from '../../../../actions/login'; import { CustomIcon } from '../../../../containers/CustomIcon'; import { useTheme } from '../../../../theme'; @@ -56,13 +56,9 @@ const ConfirmDeleteAccountContent = ({ const handleDeleteAccount = async () => { hideActionSheet(); - try { - await deleteOwnAccount(password, true); - } catch (e) { - if (isTwoFactorCancelled(e)) { - return; - } - throw e; + const outcome = await runCancellableAction(() => deleteOwnAccount(password, true)); + if (outcome.status === 'cancelled') { + return; } dispatch(deleteAccount()); }; diff --git a/app/views/ProfileView/components/DeleteAccountActionSheetContent/index.tsx b/app/views/ProfileView/components/DeleteAccountActionSheetContent/index.tsx index cd0caeffbf..ea8e85a0fa 100644 --- a/app/views/ProfileView/components/DeleteAccountActionSheetContent/index.tsx +++ b/app/views/ProfileView/components/DeleteAccountActionSheetContent/index.tsx @@ -9,7 +9,7 @@ import sharedStyles from '../../../Styles'; import FooterButtons from './FooterButtons'; import ConfirmDeleteAccountContent from './ConfirmDeleteAccountContent'; import { deleteOwnAccount } from '../../../../lib/services/restApi'; -import { isTwoFactorCancelled } from '../../../../lib/services/twoFactor/twoFactorCancelled'; +import { runCancellableAction } from '../../../../lib/services/twoFactor/twoFactorOutcome'; import { deleteAccount } from '../../../../actions/login'; import { CustomIcon } from '../../../../containers/CustomIcon'; import { useTheme } from '../../../../theme'; @@ -61,12 +61,12 @@ const DeleteAccountActionSheetContent = (): ReactElement => { const { password } = getValues(); Keyboard.dismiss(); try { - await deleteOwnAccount(sha256(password)); - hideActionSheet(); - } catch (error: any) { - if (isTwoFactorCancelled(error)) { + const outcome = await runCancellableAction(() => deleteOwnAccount(sha256(password))); + if (outcome.status === 'cancelled') { return; } + hideActionSheet(); + } catch (error: any) { if (error.data.errorType === 'user-last-owner') { const { shouldChangeOwner, shouldBeRemoved } = error.data.details; const { changeOwnerRooms, removedRooms } = getTranslations({ shouldChangeOwner, shouldBeRemoved }); diff --git a/app/views/ProfileView/index.test.tsx b/app/views/ProfileView/index.test.tsx index 088fa83cd2..59fc800874 100644 --- a/app/views/ProfileView/index.test.tsx +++ b/app/views/ProfileView/index.test.tsx @@ -8,6 +8,7 @@ import { saveUserProfile } from '../../lib/services/restApi'; import { twoFactor } from '../../lib/services/twoFactor/twoFactor'; import { TwoFactorCancelledError } from '../../lib/services/twoFactor/twoFactorCancelled'; import handleSaveUserProfileError from '../../lib/methods/helpers/handleSaveUserProfileError'; +import { events, logEvent } from '../../lib/methods/helpers/log'; import EventEmitter from '../../lib/methods/helpers/events'; import { setUser } from '../../actions/login'; @@ -34,6 +35,11 @@ jest.mock('../../lib/services/twoFactor/twoFactor', () => ({ jest.mock('../../lib/methods/helpers/handleSaveUserProfileError', () => jest.fn()); +jest.mock('../../lib/methods/helpers/log', () => ({ + events: jest.requireActual('../../lib/methods/helpers/log/events').default, + logEvent: jest.fn() +})); + const mockShowActionSheet = jest.fn(); const mockHideActionSheet = jest.fn(); jest.mock('../../containers/ActionSheet', () => ({ @@ -146,6 +152,18 @@ describe('ProfileView submit', () => { expect(handleSaveUserProfileError).not.toHaveBeenCalled(); }); + it('reports no failure when the save itself is cancelled by the 2FA prompt', async () => { + (saveUserProfile as jest.Mock).mockRejectedValue(new TwoFactorCancelledError()); + + const { getByTestId } = renderProfile(); + changeNameAndSubmit(getByTestId); + + await waitFor(() => expect(saveUserProfile).toHaveBeenCalled()); + expect(logEvent).not.toHaveBeenCalledWith(events.PROFILE_SAVE_CHANGES_F); + expect(handleSaveUserProfileError).not.toHaveBeenCalled(); + expect(dispatch).not.toHaveBeenCalledWith(setUser(expect.anything())); + }); + it('reports the 2FA error itself when the challenge fails for a reason other than cancelling', async () => { const twoFactorError = { error: 'totp-required' }; (saveUserProfile as jest.Mock).mockRejectedValue({ error: 'totp-invalid', details: { method: 'totp' } }); diff --git a/app/views/ProfileView/index.tsx b/app/views/ProfileView/index.tsx index 6429d52bba..d124f7ff7e 100644 --- a/app/views/ProfileView/index.tsx +++ b/app/views/ProfileView/index.tsx @@ -27,7 +27,7 @@ import { events, logEvent } from '../../lib/methods/helpers/log'; import scrollPersistTaps from '../../lib/methods/helpers/scrollPersistTaps'; import { saveUserProfile } from '../../lib/services/restApi'; import { twoFactor } from '../../lib/services/twoFactor/twoFactor'; -import { isTwoFactorCancelled } from '../../lib/services/twoFactor/twoFactorCancelled'; +import { runCancellableAction } from '../../lib/services/twoFactor/twoFactorOutcome'; import { getUserSelector } from '../../selectors/login'; import { type ProfileStackParamList } from '../../stacks/types'; import { useTheme } from '../../theme'; @@ -213,14 +213,16 @@ const ProfileView = ({ navigation }: IProfileViewProps): ReactElement => { return { status: 'failed', error: e }; } try { - const code = await twoFactor({ method: e.details.method, invalid: e?.error === 'totp-invalid' && !!twoFactorCode }); - setTwoFactorCode(code as any); + const outcome = await runCancellableAction(() => + twoFactor({ method: e.details.method, invalid: e?.error === 'totp-invalid' && !!twoFactorCode }) + ); + if (outcome.status === 'cancelled') { + return { status: 'cancelled' }; + } + setTwoFactorCode(outcome.value as any); await submit(); return { status: 'retried' }; } catch (twoFactorError) { - if (isTwoFactorCancelled(twoFactorError)) { - return { status: 'cancelled' }; - } return { status: 'failed', error: twoFactorError }; } }; @@ -244,8 +246,8 @@ const ProfileView = ({ navigation }: IProfileViewProps): ReactElement => { } try { - const result = await saveUserProfile(params, customFields); - if (result) { + const outcome = await runCancellableAction(() => saveUserProfile(params, customFields)); + if (outcome.status === 'completed' && outcome.value) { applySaveSuccess(params); } resetSavingState(); diff --git a/app/views/ProfileView/methods/logoutOtherLocations.test.ts b/app/views/ProfileView/methods/logoutOtherLocations.test.ts new file mode 100644 index 0000000000..b3e73f4dd8 --- /dev/null +++ b/app/views/ProfileView/methods/logoutOtherLocations.test.ts @@ -0,0 +1,68 @@ +import { Alert } from 'react-native'; + +import EventEmitter from '../../../lib/methods/helpers/events'; +import { events, logEvent } from '../../../lib/methods/helpers/log'; +import { logoutOtherLocations as logoutOtherLocationsService } from '../../../lib/services/restApi'; +import { TwoFactorCancelledError } from '../../../lib/services/twoFactor/twoFactorCancelled'; +import logoutOtherLocations from './logoutOtherLocations'; + +jest.mock('../../../i18n', () => ({ + t: (key: string) => key, + isTranslated: () => true +})); + +jest.mock('../../../lib/services/restApi', () => ({ + logoutOtherLocations: jest.fn() +})); + +jest.mock('../../../lib/methods/helpers/log', () => ({ + events: { PL_OTHER_LOCATIONS: 'PL_OTHER_LOCATIONS', PL_OTHER_LOCATIONS_F: 'PL_OTHER_LOCATIONS_F' }, + logEvent: jest.fn() +})); + +const confirm = () => { + const [, , buttons] = (Alert.alert as jest.Mock).mock.calls[0]; + return buttons[1].onPress(); +}; + +describe('logoutOtherLocations', () => { + beforeEach(() => { + jest.clearAllMocks(); + jest.spyOn(Alert, 'alert').mockImplementation(() => {}); + jest.spyOn(EventEmitter, 'emit').mockImplementation(() => {}); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('confirms success when the action completes', async () => { + (logoutOtherLocationsService as jest.Mock).mockResolvedValue({ success: true }); + + logoutOtherLocations(); + await confirm(); + + expect(EventEmitter.emit).toHaveBeenCalledWith(expect.anything(), { message: 'Logged_out_of_other_clients_successfully' }); + expect(logEvent).not.toHaveBeenCalledWith(events.PL_OTHER_LOCATIONS_F); + }); + + it('skips success behavior and reports no failure after a cancellation', async () => { + (logoutOtherLocationsService as jest.Mock).mockRejectedValue(new TwoFactorCancelledError()); + + logoutOtherLocations(); + await confirm(); + + expect(EventEmitter.emit).not.toHaveBeenCalled(); + expect(logEvent).not.toHaveBeenCalledWith(events.PL_OTHER_LOCATIONS_F); + }); + + it('still reports a genuine failure', async () => { + (logoutOtherLocationsService as jest.Mock).mockRejectedValue(new Error('boom')); + + logoutOtherLocations(); + await confirm(); + + expect(logEvent).toHaveBeenCalledWith(events.PL_OTHER_LOCATIONS_F); + expect(EventEmitter.emit).toHaveBeenCalledWith(expect.anything(), { message: 'Logout_failed' }); + }); +}); diff --git a/app/views/ProfileView/methods/logoutOtherLocations.ts b/app/views/ProfileView/methods/logoutOtherLocations.ts index 82f718caf3..f970f05ca9 100644 --- a/app/views/ProfileView/methods/logoutOtherLocations.ts +++ b/app/views/ProfileView/methods/logoutOtherLocations.ts @@ -4,7 +4,7 @@ import EventEmitter from '../../../lib/methods/helpers/events'; import { showConfirmationAlert } from '../../../lib/methods/helpers'; import { events, logEvent } from '../../../lib/methods/helpers/log'; import { logoutOtherLocations as logoutOtherLocationsService } from '../../../lib/services/restApi'; -import { isTwoFactorCancelled } from '../../../lib/services/twoFactor/twoFactorCancelled'; +import { runCancellableAction } from '../../../lib/services/twoFactor/twoFactorOutcome'; const logoutOtherLocations = () => { logEvent(events.PL_OTHER_LOCATIONS); @@ -13,12 +13,12 @@ const logoutOtherLocations = () => { confirmationText: I18n.t('Logout'), onPress: async () => { try { - await logoutOtherLocationsService(); - EventEmitter.emit(LISTENER, { message: I18n.t('Logged_out_of_other_clients_successfully') }); - } catch (e) { - if (isTwoFactorCancelled(e)) { + const outcome = await runCancellableAction(logoutOtherLocationsService); + if (outcome.status === 'cancelled') { return; } + EventEmitter.emit(LISTENER, { message: I18n.t('Logged_out_of_other_clients_successfully') }); + } catch { logEvent(events.PL_OTHER_LOCATIONS_F); EventEmitter.emit(LISTENER, { message: I18n.t('Logout_failed') }); } diff --git a/app/views/SetUsernameView.tsx b/app/views/SetUsernameView.tsx index 59efecbfb8..4980329468 100644 --- a/app/views/SetUsernameView.tsx +++ b/app/views/SetUsernameView.tsx @@ -20,7 +20,7 @@ import { showErrorAlert } from '../lib/methods/helpers'; import scrollPersistTaps from '../lib/methods/helpers/scrollPersistTaps'; import sharedStyles from './Styles'; import { getUsernameSuggestion, saveUserProfile } from '../lib/services/restApi'; -import { isTwoFactorCancelled } from '../lib/services/twoFactor/twoFactorCancelled'; +import { runCancellableAction } from '../lib/services/twoFactor/twoFactorOutcome'; import { useAppSelector } from '../lib/hooks/useAppSelector'; const styles = StyleSheet.create({ @@ -83,12 +83,12 @@ const SetUsernameView = () => { } setLoading(true); try { - await saveUserProfile({ username, name }); - dispatch(loginRequest({ resume: user.token })); - } catch (e: any) { - if (!isTwoFactorCancelled(e)) { - showErrorAlert(e.message, I18n.t('Oops')); + const outcome = await runCancellableAction(() => saveUserProfile({ username, name })); + if (outcome.status === 'completed') { + dispatch(loginRequest({ resume: user.token })); } + } catch (e: any) { + showErrorAlert(e.message, I18n.t('Oops')); } setLoading(false); }; From 93a0c9042d2d30fc5855eb4bc15374975fae8c7c Mon Sep 17 00:00:00 2001 From: Diego Mello Date: Thu, 27 Aug 2026 16:02:08 -0300 Subject: [PATCH 2/3] refactor: give per-room synchronization a single owner Room view initialization and reconnect now call one syncRoom operation that reads the subscription once and routes to initial room history or cursor-based catch-up from the captured last open. The two remain independent operations and no longer route into each other. Overlapping requests for the same room serialize into one active run plus at most one coalesced trailing rerun, which re-reads the latest subscription. Each caller settles on the run that serves it, so a reconnect arriving during a failing run still gets its catch-up. Last open advances only from raw server timestamps in updated message payloads and only after every page completes; deleted messages never contribute, an empty history leaves it absent, and an empty catch-up preserves it. --- app/lib/methods/loadMissedMessages.test.ts | 176 +++++++-------- app/lib/methods/loadMissedMessages.ts | 80 +++---- app/lib/methods/subscriptions/room.ts | 4 +- app/lib/methods/syncRoom.test.ts | 245 +++++++++++++++++++++ app/lib/methods/syncRoom.ts | 72 ++++++ app/views/RoomView/index.test.tsx | 25 +-- app/views/RoomView/index.tsx | 7 +- app/views/RoomView/services/getMessages.ts | 17 -- app/views/RoomView/services/index.ts | 2 - 9 files changed, 451 insertions(+), 177 deletions(-) create mode 100644 app/lib/methods/syncRoom.test.ts create mode 100644 app/lib/methods/syncRoom.ts delete mode 100644 app/views/RoomView/services/getMessages.ts diff --git a/app/lib/methods/loadMissedMessages.test.ts b/app/lib/methods/loadMissedMessages.test.ts index d80abede22..a60eb6183d 100644 --- a/app/lib/methods/loadMissedMessages.test.ts +++ b/app/lib/methods/loadMissedMessages.test.ts @@ -1,10 +1,9 @@ import { loadMissedMessages } from './loadMissedMessages'; import sdk from '../services/sdk'; import updateMessages from './updateMessages'; -import { getSubscriptionByRoomId } from '../database/services/Subscription'; import { updateLastOpen } from './updateLastOpen'; import { store } from '../store/auxStore'; -import { loadMessagesForRoom } from './loadMessagesForRoom'; +import log from './helpers/log'; jest.mock('../services/sdk', () => ({ __esModule: true, @@ -13,10 +12,6 @@ jest.mock('../services/sdk', () => ({ } })); -jest.mock('../database/services/Subscription', () => ({ - getSubscriptionByRoomId: jest.fn() -})); - jest.mock('../store/auxStore', () => ({ store: { getState: jest.fn(() => ({ server: { version: '7.4.0' } })), @@ -30,86 +25,27 @@ jest.mock('./updateLastOpen', () => ({ updateLastOpen: jest.fn() })); jest.mock('./helpers/log', () => ({ __esModule: true, default: jest.fn() })); -jest.mock('./loadMessagesForRoom', () => ({ loadMessagesForRoom: jest.fn() })); const mockedSdkGet = sdk.get as jest.MockedFunction; const mockedUpdateMessages = updateMessages as jest.MockedFunction; -const mockedGetSubscriptionByRoomId = getSubscriptionByRoomId as jest.MockedFunction; const mockedUpdateLastOpen = updateLastOpen as jest.MockedFunction; -const mockedLoadMessagesForRoom = loadMessagesForRoom as jest.MockedFunction; +const mockedLog = log as jest.MockedFunction; const RID = 'ROOM_ID'; +const CURSOR = new Date(Date.UTC(2024, 0, 1, 11, 0, 0)); describe('loadMissedMessages', () => { beforeEach(() => { jest.clearAllMocks(); mockedUpdateMessages.mockResolvedValue(0); - mockedLoadMessagesForRoom.mockResolvedValue(undefined as never); - mockedGetSubscriptionByRoomId.mockResolvedValue(null as never); (store.getState as jest.Mock).mockReturnValue({ server: { version: '7.4.0' } }); }); - it('routes a deleted-only recursion payload to remove, not update', async () => { - mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: null, t: 'p' } as never); - const deletedMessage = { _id: 'deleted-1', rid: RID, _updatedAt: new Date(Date.UTC(2024, 0, 1, 12, 0, 0)) }; - mockedSdkGet.mockResolvedValue({ - result: { updated: [], deleted: [deletedMessage], cursor: { next: null } } - } as never); - - await loadMissedMessages({ rid: RID, deletedNext: 1704110400000 }); - - expect(mockedLoadMessagesForRoom).not.toHaveBeenCalled(); - expect(mockedSdkGet).toHaveBeenCalledTimes(1); - expect(mockedSdkGet).toHaveBeenCalledWith('chat.syncMessages', expect.objectContaining({ roomId: RID, type: 'DELETED' })); - expect(mockedUpdateMessages).toHaveBeenCalledWith( - expect.objectContaining({ - rid: RID, - update: [], - remove: [deletedMessage] - }) - ); - }); - - it('loads the room history instead of syncing when the subscription has no cursor', async () => { - mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: null, t: 'p' } as never); - - await loadMissedMessages({ rid: RID }); - - expect(mockedLoadMessagesForRoom).toHaveBeenCalledTimes(1); - expect(mockedLoadMessagesForRoom).toHaveBeenCalledWith({ rid: RID, t: 'p' }); - expect(mockedSdkGet).not.toHaveBeenCalled(); - expect(mockedUpdateMessages).not.toHaveBeenCalled(); - }); - - it('falls through to the sync path when the subscription type is not a room type', async () => { - mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: null, t: 'not-a-room-type' } as never); - mockedSdkGet.mockResolvedValue({ result: { updated: [], deleted: [], cursor: { next: null } } } as never); - - await loadMissedMessages({ rid: RID }); - - expect(mockedLoadMessagesForRoom).not.toHaveBeenCalled(); - expect(mockedUpdateMessages).toHaveBeenCalledWith(expect.objectContaining({ rid: RID, update: [], remove: [] })); - }); - - it('does nothing when there is no subscription', async () => { - mockedGetSubscriptionByRoomId.mockResolvedValue(null as never); - - await expect(loadMissedMessages({ rid: RID })).resolves.toBeUndefined(); - - expect(mockedLoadMessagesForRoom).not.toHaveBeenCalled(); - expect(mockedSdkGet).not.toHaveBeenCalled(); - expect(mockedUpdateMessages).not.toHaveBeenCalled(); - expect(mockedUpdateLastOpen).not.toHaveBeenCalled(); - }); - - it('syncs from the cursor when the subscription has one', async () => { - const CURSOR = new Date(Date.UTC(2024, 0, 1, 11, 0, 0)); - mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: CURSOR, t: 'c' } as never); + it('syncs both cursors from the captured Last Open', async () => { mockedSdkGet.mockResolvedValue({ result: { updated: [], deleted: [], cursor: { next: null } } } as never); - await loadMissedMessages({ rid: RID }); + await loadMissedMessages({ rid: RID, cursor: CURSOR }); - expect(mockedLoadMessagesForRoom).not.toHaveBeenCalled(); expect(mockedSdkGet).toHaveBeenCalledTimes(2); expect(mockedSdkGet).toHaveBeenCalledWith('chat.syncMessages', { roomId: RID, @@ -125,14 +61,31 @@ describe('loadMissedMessages', () => { }); }); - it('never delegates on an updated continuation page, even without a cursor', async () => { - mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: null, t: 'p' } as never); + it('routes a deleted-only recursion payload to remove, not update', async () => { + const deletedMessage = { _id: 'deleted-1', rid: RID, _updatedAt: new Date(Date.UTC(2024, 0, 1, 12, 0, 0)) }; + mockedSdkGet.mockResolvedValue({ + result: { updated: [], deleted: [deletedMessage], cursor: { next: null } } + } as never); + + await loadMissedMessages({ rid: RID, cursor: CURSOR, deletedNext: 1704110400000 }); + + expect(mockedSdkGet).toHaveBeenCalledTimes(1); + expect(mockedSdkGet).toHaveBeenCalledWith('chat.syncMessages', expect.objectContaining({ roomId: RID, type: 'DELETED' })); + expect(mockedUpdateMessages).toHaveBeenCalledWith( + expect.objectContaining({ + rid: RID, + update: [], + remove: [deletedMessage] + }) + ); + }); + + it('fetches only the updated page on an updated continuation', async () => { const UPDATED_NEXT = Date.UTC(2024, 0, 1, 11, 30, 0); mockedSdkGet.mockResolvedValue({ result: { updated: [], deleted: [], cursor: { next: null } } } as never); - await loadMissedMessages({ rid: RID, updatedNext: UPDATED_NEXT }); + await loadMissedMessages({ rid: RID, cursor: CURSOR, updatedNext: UPDATED_NEXT }); - expect(mockedLoadMessagesForRoom).not.toHaveBeenCalled(); expect(mockedSdkGet).toHaveBeenCalledTimes(1); expect(mockedSdkGet).toHaveBeenCalledWith( 'chat.syncMessages', @@ -141,32 +94,29 @@ describe('loadMissedMessages', () => { }); describe('last open', () => { - const CURSOR = new Date(Date.UTC(2024, 0, 1, 11, 0, 0)); - const flush = () => new Promise(resolve => setImmediate(resolve)); - const message = (id: string, updatedAt: string) => ({ _id: id, rid: RID, _updatedAt: updatedAt }); - beforeEach(() => { - mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: CURSOR, t: 'c' } as never); - }); - it('writes the Last Open from the updated payload once the cursor has drained', async () => { mockedSdkGet.mockResolvedValue({ result: { updated: [message('a', '2024-01-01T11:30:00.000Z')], deleted: [], cursor: { next: null } } } as never); - await loadMissedMessages({ rid: RID }); + await loadMissedMessages({ rid: RID, cursor: CURSOR }); expect(mockedUpdateLastOpen).toHaveBeenCalledTimes(1); expect(mockedUpdateLastOpen).toHaveBeenCalledWith(RID, [{ _updatedAt: '2024-01-01T11:30:00.000Z' }]); }); - it('does not write mid-pagination, only after the final page of a paginated run', async () => { + it('writes once, after the whole pagination chain, from the stamps of every page', async () => { const PAGE_2 = Date.UTC(2024, 0, 1, 11, 30, 0); + const updatedPagesFetched: (number | undefined)[] = []; + const lastOpenWritesBeforeEachUpdatedPage: number[] = []; mockedSdkGet.mockImplementation(((_endpoint: string, params: { type?: string; next?: number }) => { if (params.type === 'DELETED') { return Promise.resolve({ result: { deleted: [], cursor: { next: null } } }); } + updatedPagesFetched.push(params.next); + lastOpenWritesBeforeEachUpdatedPage.push(mockedUpdateLastOpen.mock.calls.length); if (params.next === PAGE_2) { return Promise.resolve({ result: { updated: [message('b', '2024-01-01T11:45:00.000Z')], deleted: [], cursor: { next: null } } @@ -177,15 +127,11 @@ describe('loadMissedMessages', () => { }); }) as never); - await loadMissedMessages({ rid: RID }); - - // First page still has a next cursor, so nothing may be persisted yet. - expect(mockedUpdateLastOpen).not.toHaveBeenCalled(); - - await flush(); + await loadMissedMessages({ rid: RID, cursor: CURSOR }); + expect(updatedPagesFetched).toEqual([CURSOR.getTime(), PAGE_2]); + expect(lastOpenWritesBeforeEachUpdatedPage).toEqual([0, 0]); expect(mockedUpdateLastOpen).toHaveBeenCalledTimes(1); - // Every page walked contributes its stamps, not only the last one. expect(mockedUpdateLastOpen).toHaveBeenCalledWith(RID, [ { _updatedAt: '2024-01-01T11:30:00.000Z' }, { _updatedAt: '2024-01-01T11:45:00.000Z' } @@ -208,8 +154,7 @@ describe('loadMissedMessages', () => { }); }) as never); - await loadMissedMessages({ rid: RID }); - await flush(); + await loadMissedMessages({ rid: RID, cursor: CURSOR }); const received = mockedUpdateLastOpen.mock.calls[0][1]; const timestamps = received.map(m => new Date(m._updatedAt as string | Date).getTime()).filter(t => !Number.isNaN(t)); @@ -221,7 +166,7 @@ describe('loadMissedMessages', () => { result: { updated: [], deleted: [message('gone', '2024-01-01T11:30:00.000Z')], cursor: { next: null } } } as never); - await loadMissedMessages({ rid: RID, deletedNext: Date.UTC(2024, 0, 1, 11, 30, 0) }); + await loadMissedMessages({ rid: RID, cursor: CURSOR, deletedNext: Date.UTC(2024, 0, 1, 11, 30, 0) }); expect(mockedUpdateLastOpen).not.toHaveBeenCalled(); }); @@ -231,18 +176,61 @@ describe('loadMissedMessages', () => { result: { updated: [], deleted: [message('gone', '2024-01-01T11:30:00.000Z')], cursor: { next: null } } } as never); - await loadMissedMessages({ rid: RID }); + await loadMissedMessages({ rid: RID, cursor: CURSOR }); expect(mockedUpdateLastOpen).toHaveBeenCalledWith(RID, []); }); + it('waits for a deleted continuation before writing the Last Open', async () => { + const DELETED_PAGE_2 = Date.UTC(2024, 0, 1, 11, 30, 0); + let deletedPagesFetched = 0; + mockedSdkGet.mockImplementation(((_endpoint: string, params: { type?: string }) => { + if (params.type === 'DELETED') { + deletedPagesFetched += 1; + return Promise.resolve({ + result: { deleted: [], cursor: { next: deletedPagesFetched === 1 ? DELETED_PAGE_2 : null } } + }); + } + return Promise.resolve({ + result: { updated: [message('a', '2024-01-01T11:30:00.000Z')], deleted: [], cursor: { next: null } } + }); + }) as never); + + await loadMissedMessages({ rid: RID, cursor: CURSOR }); + + expect(deletedPagesFetched).toBe(2); + expect(mockedUpdateLastOpen).toHaveBeenCalledTimes(1); + expect(mockedUpdateLastOpen).toHaveBeenCalledWith(RID, [{ _updatedAt: '2024-01-01T11:30:00.000Z' }]); + }); + + it('logs a failed continuation and leaves the Last Open where it was', async () => { + const PAGE_2 = Date.UTC(2024, 0, 1, 11, 30, 0); + const failure = new Error('network down'); + mockedSdkGet.mockImplementation(((_endpoint: string, params: { type?: string; next?: number }) => { + if (params.type === 'DELETED') { + return Promise.resolve({ result: { deleted: [], cursor: { next: null } } }); + } + if (params.next === PAGE_2) { + return Promise.reject(failure); + } + return Promise.resolve({ + result: { updated: [message('a', '2024-01-01T11:30:00.000Z')], deleted: [], cursor: { next: PAGE_2 } } + }); + }) as never); + + await expect(loadMissedMessages({ rid: RID, cursor: CURSOR })).resolves.toBeUndefined(); + + expect(mockedLog).toHaveBeenCalledWith(failure); + expect(mockedUpdateLastOpen).not.toHaveBeenCalled(); + }); + it('writes once on the legacy unpaginated server branch', async () => { (store.getState as jest.Mock).mockReturnValue({ server: { version: '7.0.0' } }); mockedSdkGet.mockResolvedValue({ result: { updated: [message('a', '2024-01-01T11:30:00.000Z')], deleted: [] } } as never); - await loadMissedMessages({ rid: RID }); + await loadMissedMessages({ rid: RID, cursor: CURSOR }); expect(mockedSdkGet).toHaveBeenCalledWith( 'chat.syncMessages', diff --git a/app/lib/methods/loadMissedMessages.ts b/app/lib/methods/loadMissedMessages.ts index d210c37766..c360e26e64 100644 --- a/app/lib/methods/loadMissedMessages.ts +++ b/app/lib/methods/loadMissedMessages.ts @@ -3,11 +3,8 @@ import { compareServerVersion } from './helpers'; import updateMessages from './updateMessages'; import sdk from '../services/sdk'; import { store } from '../store/auxStore'; -import { getSubscriptionByRoomId } from '../database/services/Subscription'; import log from './helpers/log'; import { snapshotServerTimestamps, type TServerTimestamps, updateLastOpen } from './updateLastOpen'; -import { loadMessagesForRoom } from './loadMessagesForRoom'; -import { isRoomType } from './roomTypeToApiType'; const count = 50; @@ -19,14 +16,14 @@ const syncMessages = async ({ roomId, next, type }: { roomId: string; next: numb const getSyncMessagesFromCursor = async ( roomId: string, - cursor?: number, + cursor: number, updatedNext?: number | null, deletedNext?: number | null ) => { let updatedPromise; let deletedPromise; - if (cursor && !updatedNext && !deletedNext) { + if (!updatedNext && !deletedNext) { updatedPromise = syncMessages({ roomId, next: cursor, type: 'UPDATED' }); deletedPromise = syncMessages({ roomId, next: cursor, type: 'DELETED' }); } @@ -48,82 +45,77 @@ const getSyncMessagesFromCursor = async ( async function load({ rid: roomId, + cursor, updatedNext, deletedNext }: { rid: string; + cursor: Date; updatedNext?: number | null; deletedNext?: number | null; }) { - const sub = await getSubscriptionByRoomId(roomId); - if (!sub) { - return; - } - const cursor = sub.lastOpen; - const { version: serverVersion } = store.getState().server; if (compareServerVersion(serverVersion, 'greaterThanOrEqualTo', '7.1.0')) { - const result = await getSyncMessagesFromCursor(roomId, cursor?.getTime(), updatedNext, deletedNext); - return result; + return getSyncMessagesFromCursor(roomId, cursor.getTime(), updatedNext, deletedNext); } // RC 0.60.0 // @ts-ignore // this method dont have type - const { result } = await sdk.get('chat.syncMessages', { roomId, lastUpdate: cursor?.toISOString() }); + const { result } = await sdk.get('chat.syncMessages', { roomId, lastUpdate: cursor.toISOString() }); return result; } export async function loadMissedMessages(args: { rid: string; + cursor: Date; updatedNext?: number | null; deletedNext?: number | null; serverTimestamps?: TServerTimestamps; }): Promise { - const isFirstPage = !args.updatedNext && !args.deletedNext; - if (isFirstPage) { - // A room whose history load fetched no messages has no cursor to sync from, and syncing is what - // would write one. Load its history instead: it fetches the same gap and seeds the cursor. - const sub = await getSubscriptionByRoomId(args.rid); - if (sub && !sub.lastOpen && isRoomType(sub.t)) { - return loadMessagesForRoom({ rid: args.rid, t: sub.t }); - } - } - // A DELETED-only continuation fetches no UPDATED page, so it must not write the cursor again. const fetchedUpdatedPage = !!args.updatedNext || !args.deletedNext; const data = await load({ rid: args.rid, + cursor: args.cursor, updatedNext: args.updatedNext, deletedNext: args.deletedNext }); - if (data) { - const { - updated, - updatedNext, - deleted, - deletedNext - }: { updated: ILastMessage[]; deleted: ILastMessage[]; updatedNext: number | null; deletedNext: number | null } = data; + if (!data) { + return; + } + + const { + updated, + updatedNext, + deleted, + deletedNext + }: { updated: ILastMessage[]; deleted: ILastMessage[]; updatedNext: number | null; deletedNext: number | null } = data; - const serverTimestamps = [...(args.serverTimestamps ?? []), ...snapshotServerTimestamps(updated)]; + const serverTimestamps = [...(args.serverTimestamps ?? []), ...snapshotServerTimestamps(updated)]; - // @ts-ignore // TODO: remove loaderItem obligatoriness - await updateMessages({ rid: args.rid, update: updated, remove: deleted }); + // @ts-ignore // TODO: remove loaderItem obligatoriness + await updateMessages({ rid: args.rid, update: updated, remove: deleted }); - if (deletedNext || updatedNext) { - loadMissedMessages({ + if (deletedNext || updatedNext) { + try { + await loadMissedMessages({ rid: args.rid, + cursor: args.cursor, updatedNext, deletedNext, serverTimestamps - }).catch(log); + }); + } catch (e) { + log(e); + return; } + } - // Only once the UPDATED cursor has drained, from the stamps of every page walked: the - // pages descend from the newest, so the last one alone would lower the cursor. Advancing - // mid-pagination would skip pages not yet fetched; `deleted` is never a source, its rows - // carry no new history. - if (fetchedUpdatedPage && !updatedNext) { - await updateLastOpen(args.rid, serverTimestamps); - } + // Only once the UPDATED cursor has drained, from the stamps of every page walked: the + // pages descend from the newest, so the last one alone would lower the cursor. Advancing + // mid-pagination would skip pages not yet fetched; `deleted` is never a source, its rows + // carry no new history. + if (fetchedUpdatedPage && !updatedNext) { + await updateLastOpen(args.rid, serverTimestamps); } } diff --git a/app/lib/methods/subscriptions/room.ts b/app/lib/methods/subscriptions/room.ts index 93f67bdeb5..895c885d8c 100644 --- a/app/lib/methods/subscriptions/room.ts +++ b/app/lib/methods/subscriptions/room.ts @@ -26,7 +26,7 @@ import { import { type IDDPMessage } from '../../../definitions/IDDPMessage'; import sdk from '../../services/sdk'; import { readMessages } from '../readMessages'; -import { loadMissedMessages } from '../loadMissedMessages'; +import { syncRoom } from '../syncRoom'; import markMessagesRead from '../helpers/markMessagesRead'; export default class RoomSubscription { @@ -94,7 +94,7 @@ export default class RoomSubscription { handleConnection = async () => { try { reduxStore.dispatch(clearUserTyping()); - await loadMissedMessages({ rid: this.rid }); + await syncRoom({ rid: this.rid }); this.read(); } catch (e) { log(e); diff --git a/app/lib/methods/syncRoom.test.ts b/app/lib/methods/syncRoom.test.ts new file mode 100644 index 0000000000..b31a215607 --- /dev/null +++ b/app/lib/methods/syncRoom.test.ts @@ -0,0 +1,245 @@ +import { syncRoom } from './syncRoom'; +import { getSubscriptionByRoomId } from '../database/services/Subscription'; +import { loadMessagesForRoom } from './loadMessagesForRoom'; +import { loadMissedMessages } from './loadMissedMessages'; + +jest.mock('../database/services/Subscription', () => ({ + getSubscriptionByRoomId: jest.fn() +})); +jest.mock('./loadMessagesForRoom', () => ({ loadMessagesForRoom: jest.fn() })); +jest.mock('./loadMissedMessages', () => ({ loadMissedMessages: jest.fn() })); + +const mockedGetSubscriptionByRoomId = getSubscriptionByRoomId as jest.MockedFunction; +const mockedLoadMessagesForRoom = loadMessagesForRoom as jest.MockedFunction; +const mockedLoadMissedMessages = loadMissedMessages as jest.MockedFunction; + +const RID = 'ROOM_ID'; +const CURSOR = new Date(Date.UTC(2024, 0, 1, 11, 0, 0)); + +const deferred = () => { + let resolve!: (value: T) => void; + let reject!: (reason?: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +}; + +const flush = () => new Promise(resolve => setImmediate(resolve)); + +describe('syncRoom', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockedLoadMessagesForRoom.mockResolvedValue(undefined); + mockedLoadMissedMessages.mockResolvedValue(undefined); + mockedGetSubscriptionByRoomId.mockResolvedValue(null as never); + }); + + describe('routing', () => { + it('catches up from the captured Last Open', async () => { + mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: CURSOR, t: 'c' } as never); + + await syncRoom({ rid: RID }); + + expect(mockedLoadMissedMessages).toHaveBeenCalledWith({ rid: RID, cursor: CURSOR }); + expect(mockedLoadMessagesForRoom).not.toHaveBeenCalled(); + }); + + it('loads the room history from the subscription type when there is no Last Open', async () => { + mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: null, t: 'p' } as never); + + await syncRoom({ rid: RID, fallbackRoomType: 'c' }); + + expect(mockedLoadMessagesForRoom).toHaveBeenCalledWith({ rid: RID, t: 'p' }); + expect(mockedLoadMissedMessages).not.toHaveBeenCalled(); + }); + + it('loads the room history from the fallback type when there is no subscription', async () => { + await syncRoom({ rid: RID, fallbackRoomType: 'd' }); + + expect(mockedLoadMessagesForRoom).toHaveBeenCalledWith({ rid: RID, t: 'd' }); + }); + + it('loads the room history from the fallback type when the subscription type is not a room type', async () => { + mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: null, t: 'not-a-room-type' } as never); + + await syncRoom({ rid: RID, fallbackRoomType: 'c' }); + + expect(mockedLoadMessagesForRoom).toHaveBeenCalledWith({ rid: RID, t: 'c' }); + }); + + it('does nothing without a subscription and without a fallback type', async () => { + await expect(syncRoom({ rid: RID })).resolves.toBeUndefined(); + + expect(mockedLoadMessagesForRoom).not.toHaveBeenCalled(); + expect(mockedLoadMissedMessages).not.toHaveBeenCalled(); + }); + + it('does nothing when neither the subscription nor the caller offers a valid room type', async () => { + mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: null, t: 'not-a-room-type' } as never); + + await expect(syncRoom({ rid: RID })).resolves.toBeUndefined(); + + expect(mockedLoadMessagesForRoom).not.toHaveBeenCalled(); + expect(mockedLoadMissedMessages).not.toHaveBeenCalled(); + }); + }); + + describe('identical routing from room initialization and reconnect', () => { + it('catches up for a room with a Last Open, whichever trigger asked', async () => { + mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: CURSOR, t: 'c' } as never); + + await syncRoom({ rid: RID, fallbackRoomType: 'c' }); + await syncRoom({ rid: RID }); + + expect(mockedLoadMessagesForRoom).not.toHaveBeenCalled(); + expect(mockedLoadMissedMessages.mock.calls).toEqual([[{ rid: RID, cursor: CURSOR }], [{ rid: RID, cursor: CURSOR }]]); + }); + + it('loads the room history for a cursor-less room, whichever trigger asked', async () => { + mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: null, t: 'c' } as never); + + await syncRoom({ rid: RID, fallbackRoomType: 'c' }); + await syncRoom({ rid: RID }); + + expect(mockedLoadMissedMessages).not.toHaveBeenCalled(); + expect(mockedLoadMessagesForRoom.mock.calls).toEqual([[{ rid: RID, t: 'c' }], [{ rid: RID, t: 'c' }]]); + }); + }); + + describe('concurrency', () => { + it('runs one active run and coalesces overlapping requests into a single trailing rerun', async () => { + mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: CURSOR, t: 'c' } as never); + const firstRun = deferred(); + const trailingRun = deferred(); + mockedLoadMissedMessages + .mockReturnValueOnce(firstRun.promise) + .mockReturnValueOnce(trailingRun.promise) + .mockResolvedValue(undefined); + + const first = syncRoom({ rid: RID }); + await flush(); + expect(mockedLoadMissedMessages).toHaveBeenCalledTimes(1); + + const second = syncRoom({ rid: RID }); + const third = syncRoom({ rid: RID }); + const fourth = syncRoom({ rid: RID }); + await flush(); + expect(mockedLoadMissedMessages).toHaveBeenCalledTimes(1); + + firstRun.resolve(); + await flush(); + expect(mockedLoadMissedMessages).toHaveBeenCalledTimes(2); + + trailingRun.resolve(); + await Promise.all([first, second, third, fourth]); + + expect(mockedLoadMissedMessages).toHaveBeenCalledTimes(2); + }); + + it('re-reads the latest subscription for the trailing rerun', async () => { + const firstRun = deferred(); + mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: null, t: 'c' } as never); + mockedLoadMessagesForRoom.mockReturnValueOnce(firstRun.promise).mockResolvedValue(undefined); + + const first = syncRoom({ rid: RID, fallbackRoomType: 'c' }); + await flush(); + + const second = syncRoom({ rid: RID }); + mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: CURSOR, t: 'c' } as never); + + firstRun.resolve(); + await Promise.all([first, second]); + + expect(mockedGetSubscriptionByRoomId).toHaveBeenCalledTimes(2); + expect(mockedLoadMessagesForRoom).toHaveBeenCalledTimes(1); + expect(mockedLoadMissedMessages).toHaveBeenCalledWith({ rid: RID, cursor: CURSOR }); + }); + + it('keeps rooms independent', async () => { + const otherRid = 'OTHER_ROOM_ID'; + const firstRun = deferred(); + mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: CURSOR, t: 'c' } as never); + mockedLoadMissedMessages.mockReturnValueOnce(firstRun.promise).mockResolvedValue(undefined); + + const first = syncRoom({ rid: RID }); + const other = syncRoom({ rid: otherRid }); + await flush(); + + expect(mockedLoadMissedMessages).toHaveBeenCalledTimes(2); + + firstRun.resolve(); + await Promise.all([first, other]); + }); + + it('settles a coalescing caller on its own rerun, not on the active run', async () => { + mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: CURSOR, t: 'c' } as never); + const firstRun = deferred(); + const trailingRun = deferred(); + mockedLoadMissedMessages.mockReturnValueOnce(firstRun.promise).mockReturnValueOnce(trailingRun.promise); + + const settled: string[] = []; + const first = syncRoom({ rid: RID }).then(() => settled.push('first')); + await flush(); + const second = syncRoom({ rid: RID }).then(() => settled.push('second')); + + firstRun.resolve(); + await first; + await flush(); + + expect(settled).toEqual(['first']); + expect(mockedLoadMissedMessages).toHaveBeenCalledTimes(2); + + trailingRun.resolve(); + await second; + + expect(settled).toEqual(['first', 'second']); + }); + + it("runs a caller's pending rerun even when the active run failed, without reusing its error", async () => { + mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: CURSOR, t: 'c' } as never); + const firstRun = deferred(); + mockedLoadMissedMessages.mockReturnValueOnce(firstRun.promise).mockResolvedValue(undefined); + + const first = syncRoom({ rid: RID }); + await flush(); + const second = syncRoom({ rid: RID }); + + firstRun.reject(new Error('network down')); + + await expect(first).rejects.toThrow('network down'); + await expect(second).resolves.toBeUndefined(); + expect(mockedLoadMissedMessages).toHaveBeenCalledTimes(2); + }); + + it('does not rerun after an empty response', async () => { + mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: CURSOR, t: 'c' } as never); + + await syncRoom({ rid: RID }); + + expect(mockedLoadMissedMessages).toHaveBeenCalledTimes(1); + expect(mockedGetSubscriptionByRoomId).toHaveBeenCalledTimes(1); + }); + + it('does not rerun after a failure', async () => { + mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: CURSOR, t: 'c' } as never); + mockedLoadMissedMessages.mockRejectedValue(new Error('network down')); + + await expect(syncRoom({ rid: RID })).rejects.toThrow('network down'); + await flush(); + + expect(mockedLoadMissedMessages).toHaveBeenCalledTimes(1); + }); + + it('accepts a new run once the previous one failed', async () => { + mockedGetSubscriptionByRoomId.mockResolvedValue({ lastOpen: CURSOR, t: 'c' } as never); + mockedLoadMissedMessages.mockRejectedValueOnce(new Error('network down')).mockResolvedValue(undefined); + + await expect(syncRoom({ rid: RID })).rejects.toThrow('network down'); + await syncRoom({ rid: RID }); + + expect(mockedLoadMissedMessages).toHaveBeenCalledTimes(2); + }); + }); +}); diff --git a/app/lib/methods/syncRoom.ts b/app/lib/methods/syncRoom.ts new file mode 100644 index 0000000000..124999ca0e --- /dev/null +++ b/app/lib/methods/syncRoom.ts @@ -0,0 +1,72 @@ +import { getSubscriptionByRoomId } from '../database/services/Subscription'; +import { loadMessagesForRoom } from './loadMessagesForRoom'; +import { loadMissedMessages } from './loadMissedMessages'; +import { isRoomType, type RoomTypes } from './roomTypeToApiType'; + +export interface ISyncRoomParams { + rid: string; + fallbackRoomType?: RoomTypes; +} + +interface IRequest { + params: ISyncRoomParams; + resolve: () => void; + reject: (reason: unknown) => void; +} + +const activeRooms = new Set(); +const requestedReruns = new Map(); + +async function routeFromLatestSubscription({ rid, fallbackRoomType }: ISyncRoomParams): Promise { + const subscription = await getSubscriptionByRoomId(rid); + + if (subscription?.lastOpen) { + return loadMissedMessages({ rid, cursor: subscription.lastOpen }); + } + + const roomType = isRoomType(subscription?.t) ? subscription.t : fallbackRoomType; + if (!isRoomType(roomType)) { + return; + } + + return loadMessagesForRoom({ rid, t: roomType }); +} + +async function drain(rid: string, firstRequests: IRequest[]): Promise { + let requests: IRequest[] | undefined = firstRequests; + try { + while (requests) { + try { + await routeFromLatestSubscription(requests[requests.length - 1].params); + requests.forEach(({ resolve }) => resolve()); + } catch (error) { + requests.forEach(({ reject }) => reject(error)); + } + requests = requestedReruns.get(rid); + requestedReruns.delete(rid); + } + } finally { + requestedReruns.delete(rid); + activeRooms.delete(rid); + } +} + +export function syncRoom(params: ISyncRoomParams): Promise { + const { rid } = params; + return new Promise((resolve, reject) => { + const request = { params, resolve, reject }; + + if (activeRooms.has(rid)) { + const rerun = requestedReruns.get(rid); + if (rerun) { + rerun.push(request); + } else { + requestedReruns.set(rid, [request]); + } + return; + } + + activeRooms.add(rid); + drain(rid, [request]); + }); +} diff --git a/app/views/RoomView/index.test.tsx b/app/views/RoomView/index.test.tsx index 14eb5d2bdf..c923a0cfb3 100644 --- a/app/views/RoomView/index.test.tsx +++ b/app/views/RoomView/index.test.tsx @@ -4,7 +4,7 @@ import { BehaviorSubject, Subject } from 'rxjs'; import { RoomView } from './index'; import { type IRoomViewProps } from './definitions'; -import RoomServices from './services'; +import { syncRoom } from '../../lib/methods/syncRoom'; import { readMessages } from '../../lib/methods/readMessages'; import { mockedStore } from '../../reducers/mockedStore'; import { initStore } from '../../lib/store/auxStore'; @@ -50,10 +50,7 @@ jest.mock('../../lib/services/voip/isInActiveVoipCall', () => ({ useIsInActiveVoipCall: () => false })); -jest.mock('./services', () => ({ - __esModule: true, - default: { getMessages: jest.fn().mockResolvedValue(undefined) } -})); +jest.mock('../../lib/methods/syncRoom', () => ({ syncRoom: jest.fn().mockResolvedValue(undefined) })); jest.mock('../../lib/methods/readMessages', () => ({ readMessages: jest.fn().mockResolvedValue(undefined) })); jest.mock('../../lib/methods/loadThreadMessages', () => ({ loadThreadMessages: jest.fn().mockResolvedValue(undefined) })); jest.mock('../../lib/methods/helpers/isReadOnly', () => ({ isReadOnly: jest.fn().mockResolvedValue(false) })); @@ -62,7 +59,7 @@ jest.mock('../../lib/methods/AudioManager', () => ({ default: { pauseAudio: jest.fn(), unloadRoomAudios: jest.fn().mockResolvedValue(undefined) } })); -const mockedGetMessages = RoomServices.getMessages as jest.Mock; +const mockedSyncRoom = syncRoom as jest.Mock; const mockedReadMessages = readMessages as jest.Mock; const mockSubscriptionsQuery = { observe: jest.fn(), observeWithColumns: jest.fn(() => new Subject()) }; @@ -144,7 +141,7 @@ describe('RoomView init cursor predicate', () => { beforeEach(() => { jest.clearAllMocks(); - mockedGetMessages.mockResolvedValue(undefined); + mockedSyncRoom.mockResolvedValue(undefined); mockedReadMessages.mockResolvedValue(undefined); mockSubscriptionsCollection.find.mockRejectedValue(new Error('not found')); mockSubscriptionsQuery.observe.mockReturnValue(new Subject()); @@ -154,19 +151,19 @@ describe('RoomView init cursor predicate', () => { renderRoomView({ rid: 'rid-1', t: 'c' }); await flush(); - expect(mockedGetMessages).toHaveBeenCalledWith({ rid: 'rid-1', t: 'c' }); + expect(mockedSyncRoom).toHaveBeenCalledWith({ rid: 'rid-1', fallbackRoomType: 'c' }); expect(mockedReadMessages).not.toHaveBeenCalled(); }); - it('routes a cursor-less subscribed room to the room-history loader directly', async () => { + it('synchronizes a cursor-less subscribed room once', async () => { renderRoomView({ rid: 'rid-1', t: 'c', room: buildRow() }); await flush(); - expect(mockedGetMessages).toHaveBeenCalledTimes(1); - expect(mockedGetMessages).toHaveBeenCalledWith({ rid: 'rid-1', t: 'c' }); + expect(mockedSyncRoom).toHaveBeenCalledTimes(1); + expect(mockedSyncRoom).toHaveBeenCalledWith({ rid: 'rid-1', fallbackRoomType: 'c' }); }); - it('routes a subscribed room with a cursor to the missed-messages loader', async () => { + it('synchronizes a subscribed room with a cursor once', async () => { renderRoomView({ rid: 'rid-1', t: 'c', @@ -174,7 +171,7 @@ describe('RoomView init cursor predicate', () => { }); await flush(); - expect(mockedGetMessages).toHaveBeenCalledTimes(1); - expect(mockedGetMessages).toHaveBeenCalledWith({ rid: 'rid-1' }); + expect(mockedSyncRoom).toHaveBeenCalledTimes(1); + expect(mockedSyncRoom).toHaveBeenCalledWith({ rid: 'rid-1', fallbackRoomType: 'c' }); }); }); diff --git a/app/views/RoomView/index.tsx b/app/views/RoomView/index.tsx index aea43ebad4..14aef497cd 100644 --- a/app/views/RoomView/index.tsx +++ b/app/views/RoomView/index.tsx @@ -49,6 +49,8 @@ import getThreadName from '../../lib/methods/getThreadName'; import getRoomInfo from '../../lib/methods/getRoomInfo'; import { ContainerTypes } from '../../containers/UIKit/interfaces'; import RoomServices from './services'; +import { syncRoom } from '../../lib/methods/syncRoom'; +import { type RoomTypes } from '../../lib/methods/roomTypeToApiType'; import LoadMore from './LoadMore'; import Banner from './Banner'; import RightButtons from './RightButtons'; @@ -678,10 +680,7 @@ export class RoomView extends Component { this.consumeJumpParam(messageId); } } else { - await RoomServices.getMessages({ - rid: room.rid, - ...('lastOpen' in room && room.lastOpen ? {} : { t: room.t as RoomType }) - }); + await syncRoom({ rid: room.rid, fallbackRoomType: room.t as RoomTypes }); // if room is joined if (joined && 'id' in room) { diff --git a/app/views/RoomView/services/getMessages.ts b/app/views/RoomView/services/getMessages.ts deleted file mode 100644 index 7d4165120d..0000000000 --- a/app/views/RoomView/services/getMessages.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { loadMessagesForRoom } from '../../../lib/methods/loadMessagesForRoom'; -import { loadMissedMessages } from '../../../lib/methods/loadMissedMessages'; -import { type RoomTypes } from '../../../lib/methods/roomTypeToApiType'; - -interface IGetMessagesParams { - rid: string; - t?: RoomTypes; -} - -const getMessages = ({ rid, t }: IGetMessagesParams): Promise => { - if (!t) { - return loadMissedMessages({ rid }); - } - return loadMessagesForRoom({ rid, t }); -}; - -export default getMessages; diff --git a/app/views/RoomView/services/index.ts b/app/views/RoomView/services/index.ts index d1cff8ff7e..a74c512234 100644 --- a/app/views/RoomView/services/index.ts +++ b/app/views/RoomView/services/index.ts @@ -1,9 +1,7 @@ -import getMessages from './getMessages'; import getMessageInfo from './getMessageInfo'; import getLocalAnchorTs from './getLocalAnchor'; export default { - getMessages, getMessageInfo, getLocalAnchorTs }; From 30eb20c1d14e1c3fdd87902393c53608d7f92d55 Mon Sep 17 00:00:00 2001 From: Diego Mello Date: Thu, 27 Aug 2026 16:02:17 -0300 Subject: [PATCH 3/3] test: exercise the SDK integration harness through public behavior SDK integration tests now drive the real SDK through public client and driver operations over one app-owned WebSocket/DDP transport fake, which controls connection creation, open and close, incoming frames and withheld responses, and records outgoing frames. Subscription identifiers, replay and method calls are asserted from the wire rather than from private driver state. Socket health is classified through public connection state and a round trip instead of mutating last ping. The private driver shape, its double casts, and the arbitrary-round flush and settleUntil helpers are gone. Tests wait on named frames, actions, lifecycle events, database effects and operation promises. The one genuinely opaque callback uses a single labelled bounded wait that re-checks its condition after each scheduler advance and always rejects on expiry, naming the awaited condition; a direct test proves an unmet wait cannot return successfully. --- app/lib/methods/logout.test.ts | 8 +- .../roomSubscription.integration.test.ts | 63 ++--- .../__tests__/rooms.hostGuard.test.ts | 8 +- .../__tests__/connect.integration.test.ts | 221 ++++++++------- .../sdkTransport.integration.test.ts | 90 ++++++ .../socketHealth.integration.test.ts | 267 +++++++----------- .../services/__tests__/socketHealth.test.ts | 55 ++-- app/lib/services/restApi.test.ts | 8 +- .../voip/MediaSessionInstance.test.ts | 8 +- .../voip/acceptNativeCall.integration.test.ts | 50 ++-- .../acceptNativeCall.sdk.integration.test.ts | 97 +++---- .../services/voip/acceptNativeCall.test.ts | 47 ++- .../__tests__/observedEffects.test.ts | 21 ++ app/lib/testUtils/appMocks.ts | 58 ++++ app/lib/testUtils/observedEffects.ts | 106 +++++++ app/lib/testUtils/sdkIntegration.ts | 206 -------------- app/lib/testUtils/sdkModuleFake.ts | 40 +++ app/lib/testUtils/sdkTransport.ts | 191 +++++++++++++ .../foregroundResume.integration.test.ts | 237 +++++++--------- .../__tests__/selectServer.sdkHost.test.ts | 22 +- 20 files changed, 1001 insertions(+), 802 deletions(-) create mode 100644 app/lib/services/__tests__/sdkTransport.integration.test.ts create mode 100644 app/lib/testUtils/__tests__/observedEffects.test.ts create mode 100644 app/lib/testUtils/appMocks.ts create mode 100644 app/lib/testUtils/observedEffects.ts delete mode 100644 app/lib/testUtils/sdkIntegration.ts create mode 100644 app/lib/testUtils/sdkModuleFake.ts create mode 100644 app/lib/testUtils/sdkTransport.ts diff --git a/app/lib/methods/logout.test.ts b/app/lib/methods/logout.test.ts index 54ea9b56e4..bb61e365d0 100644 --- a/app/lib/methods/logout.test.ts +++ b/app/lib/methods/logout.test.ts @@ -1,4 +1,4 @@ -import type * as SdkIntegration from '../testUtils/sdkIntegration'; +import type * as SdkModuleFake from '../testUtils/sdkModuleFake'; jest.mock('../database', () => ({ __esModule: true, @@ -33,8 +33,8 @@ jest.mock('../services/restApi', () => ({ const mockSdkLogout = jest.fn(); jest.mock('../services/sdk', () => { - const { makeSdkMock } = jest.requireActual('../testUtils/sdkIntegration'); - return { __esModule: true, default: makeSdkMock({ logout: () => mockSdkLogout() }) }; + const { createSdkModuleFake } = jest.requireActual('../testUtils/sdkModuleFake'); + return { __esModule: true, default: createSdkModuleFake({ logout: () => mockSdkLogout() }) }; }); import { logout, removeServerData } from './logout'; @@ -52,7 +52,7 @@ import { TOKEN_KEY } from '../constants/keys'; -const mockSdk = sdk as unknown as SdkIntegration.IMockSdk; +const mockSdk = sdk as unknown as SdkModuleFake.ISdkModuleFake; const SERVER = 'https://a.rocket.chat'; const OTHER_SERVER = 'https://b.rocket.chat'; diff --git a/app/lib/methods/subscriptions/__tests__/roomSubscription.integration.test.ts b/app/lib/methods/subscriptions/__tests__/roomSubscription.integration.test.ts index 2cd653af94..7eb2ee8c14 100644 --- a/app/lib/methods/subscriptions/__tests__/roomSubscription.integration.test.ts +++ b/app/lib/methods/subscriptions/__tests__/roomSubscription.integration.test.ts @@ -1,11 +1,8 @@ jest.unmock('@rocket.chat/sdk'); -jest.mock('universal-websocket-client', () => - jest.fn().mockImplementation(() => { - const sdkIntegration = jest.requireActual('../../../testUtils/sdkIntegration'); - return new sdkIntegration.MockConnection(mockConnections); - }) -); +const mockTransport = createTransportFake(); + +jest.mock('universal-websocket-client', () => jest.fn().mockImplementation(() => mockTransport.createConnection())); jest.mock('../../../encryption', () => ({ Encryption: { decryptMessage: jest.fn(async (message: unknown) => message) } @@ -73,22 +70,14 @@ import { getMessageById } from '../../../database/services/Message'; import buildMessage from '../../helpers/buildMessage'; import { subscribeRoom, unsubscribeRoom } from '../../../../actions/room'; import { clearUserTyping } from '../../../../actions/usersTyping'; -import { - flush, - framesOn, - makeCollection as makeBaseCollection, - makeReduxStore, - receiveFrame -} from '../../../testUtils/sdkIntegration'; -import type { IMockCollection, MockConnection } from '../../../testUtils/sdkIntegration'; -import type * as SdkIntegration from '../../../testUtils/sdkIntegration'; +import { makeCollection as makeBaseCollection, makeReduxStore } from '../../../testUtils/appMocks'; +import type { IMockCollection } from '../../../testUtils/appMocks'; +import { createTransportFake } from '../../../testUtils/sdkTransport'; const database = require('../../../database').default as { active: { get: jest.Mock; write: jest.Mock; batch: jest.Mock }; }; -const mockConnections: MockConnection[] = []; - function makeCollection(name: string): IMockCollection { const collection = makeBaseCollection(name); collection.prepareCreate.mockImplementation((fn: (record: Record) => void) => { @@ -114,7 +103,7 @@ let collections: Record>; beforeEach(() => { jest.clearAllMocks(); jest.useFakeTimers(); - mockConnections.length = 0; + mockTransport.reset(); collections = {}; redux = makeReduxStore(); initStore(redux.store); @@ -125,34 +114,39 @@ beforeEach(() => { }); afterEach(() => { + sdk.disconnect(); jest.useRealTimers(); }); async function connectDriver() { sdk.initialize('https://example.com'); - const connectPromise = sdk.connect(); - await flush(); - mockConnections[0].onopen(); - await flush(); - await connectPromise; + const connecting = sdk.connect(); + mockTransport.open(await mockTransport.awaitConnection()); + await connecting; } async function subscribeToRoom(rid: string) { const room = new RoomSubscription(rid); - const subscribing = room.subscribe(); - await flush(); - await subscribing; - await flush(); + await room.subscribe(); return room; } +function nextBatchedRecords(): Promise { + return new Promise(resolve => { + database.active.batch.mockImplementation((...records: unknown[]) => { + resolve(records); + return Promise.resolve(records); + }); + }); +} + describe('RoomSubscription over the real SDK', () => { it('subscribes to the room streams and registers the store subscription', async () => { await connectDriver(); await subscribeToRoom('room-rid'); - expect(framesOn(mockConnections[0], 'sub')).toHaveLength(5); + expect(mockTransport.frames({ msg: 'sub' })).toHaveLength(5); expect(redux.store.dispatch).toHaveBeenCalledWith(subscribeRoom('room-rid')); }); @@ -160,18 +154,17 @@ describe('RoomSubscription over the real SDK', () => { await connectDriver(); await subscribeToRoom('room-rid'); - receiveFrame(mockConnections[0], { + const batched = nextBatchedRecords(); + mockTransport.deliver({ msg: 'changed', collection: 'stream-room-messages', fields: { eventName: 'room-rid', args: [MESSAGE] } }); - await flush(); + expect(await batched).toMatchObject([{ _id: 'msg-1', rid: 'room-rid', msg: 'hello' }]); expect(buildMessage).toHaveBeenCalledTimes(1); expect(getMessageById).toHaveBeenCalledWith('msg-1'); expect(database.active.write).toHaveBeenCalled(); - const record = database.active.batch.mock.calls[0][0]; - expect(record).toMatchObject({ _id: 'msg-1', rid: 'room-rid', msg: 'hello' }); }); it('stops its listeners and unsubscribes all five subscriptions', async () => { @@ -179,18 +172,16 @@ describe('RoomSubscription over the real SDK', () => { const room = await subscribeToRoom('room-rid'); await room.unsubscribe(); - await flush(); - expect(framesOn(mockConnections[0], 'unsub')).toHaveLength(5); + expect(mockTransport.frames({ msg: 'unsub' })).toHaveLength(5); expect(redux.store.dispatch).toHaveBeenCalledWith(unsubscribeRoom('room-rid')); expect(redux.store.dispatch).toHaveBeenCalledWith(clearUserTyping()); - receiveFrame(mockConnections[0], { + mockTransport.deliver({ msg: 'changed', collection: 'stream-room-messages', fields: { eventName: 'room-rid', args: [MESSAGE] } }); - await flush(); expect(buildMessage).not.toHaveBeenCalled(); }); diff --git a/app/lib/methods/subscriptions/__tests__/rooms.hostGuard.test.ts b/app/lib/methods/subscriptions/__tests__/rooms.hostGuard.test.ts index 2ef4e50488..cf30736dc7 100644 --- a/app/lib/methods/subscriptions/__tests__/rooms.hostGuard.test.ts +++ b/app/lib/methods/subscriptions/__tests__/rooms.hostGuard.test.ts @@ -2,10 +2,10 @@ const mockOnStreamData = jest.fn(async (_event: string, _callback: (message: IDD const mockSubscribeNotifyUser = jest.fn(async () => undefined); jest.mock('../../../services/sdk', () => { - const { makeSdkMock } = jest.requireActual('../../../testUtils/sdkIntegration'); + const { createSdkModuleFake } = jest.requireActual('../../../testUtils/sdkModuleFake'); return { __esModule: true, - default: makeSdkMock({ + default: createSdkModuleFake({ onStreamData: (...args: Parameters) => mockOnStreamData(...args), subscribeNotifyUser: () => mockSubscribeNotifyUser() }) @@ -27,9 +27,9 @@ import subscribeRooms, { roomsSubscription } from '../rooms'; import sdk from '../../../services/sdk'; import database from '../../../database'; import type { IDDPMessage } from '../../../../definitions/IDDPMessage'; -import type * as SdkIntegration from '../../../testUtils/sdkIntegration'; +import type * as SdkModuleFake from '../../../testUtils/sdkModuleFake'; -const mockedSdk = sdk as unknown as SdkIntegration.IMockSdk; +const mockedSdk = sdk as unknown as SdkModuleFake.ISdkModuleFake; const mockedDatabase = database as unknown as { active: { get: jest.Mock } }; const HOST = 'https://open.rocket.chat'; diff --git a/app/lib/services/__tests__/connect.integration.test.ts b/app/lib/services/__tests__/connect.integration.test.ts index bdb75990c6..e392819198 100644 --- a/app/lib/services/__tests__/connect.integration.test.ts +++ b/app/lib/services/__tests__/connect.integration.test.ts @@ -1,5 +1,7 @@ jest.unmock('@rocket.chat/sdk'); +import type { AnyAction, Store } from 'redux'; + import { connect, login, loginWithPassword } from '../connect'; import sdk from '../sdk'; import { initStore } from '../../store/auxStore'; @@ -9,18 +11,16 @@ import { setActiveUsers } from '../../../actions/activeUsers'; import { updateSettings } from '../../../actions/settings'; import { updatePermission } from '../../../actions/permissions'; import { _activeUsers, _setUserTimer } from '../../methods/setUser'; -import { flush, framesOn, makeCollection, makeReduxStore, receiveFrame } from '../../testUtils/sdkIntegration'; -import type { MockConnection } from '../../testUtils/sdkIntegration'; -import type * as SdkIntegration from '../../testUtils/sdkIntegration'; +import { makeCollection } from '../../testUtils/appMocks'; +import { createActionRecorder, trackCalls } from '../../testUtils/observedEffects'; +import { createTransportFake } from '../../testUtils/sdkTransport'; +import type { IApplicationState } from '../../../definitions'; + +const ROUND_TRIP_BUDGET = 2000; -const mockConnections: MockConnection[] = []; +const mockTransport = createTransportFake(); -jest.mock('universal-websocket-client', () => - jest.fn().mockImplementation(() => { - const sdkIntegration = jest.requireActual('../../testUtils/sdkIntegration'); - return new sdkIntegration.MockConnection(mockConnections); - }) -); +jest.mock('universal-websocket-client', () => jest.fn().mockImplementation(() => mockTransport.createConnection())); jest.mock('../voip/MediaSessionInstance', () => ({ mediaSessionInstance: { @@ -83,16 +83,45 @@ const REST_LOGIN_ME = { requirePasswordChange: false }; -let redux: ReturnType; +interface IHarnessState { + meteor: { connected: boolean }; + login: { user: Record | null; isAuthenticated: boolean }; + server: { version: string }; + settings: Record; + room: { subscribedRoom: string | null }; +} + +const recorder = createActionRecorder(); + +let state: IHarnessState; +let store: Store & { dispatch: jest.Mock }; let collections: Record>; +function makeStore(): void { + state = { + meteor: { connected: false }, + login: { user: null, isAuthenticated: false }, + server: { version: '5.0.0' }, + settings: {}, + room: { subscribedRoom: null } + }; + store = { + getState: () => state, + dispatch: jest.fn((action: AnyAction) => { + recorder.record(action); + return action; + }), + subscribe: () => () => undefined + } as unknown as Store & { dispatch: jest.Mock }; +} + beforeEach(() => { jest.clearAllMocks(); - jest.useFakeTimers(); - mockConnections.length = 0; + mockTransport.reset(); + recorder.reset(); collections = {}; - redux = makeReduxStore(); - initStore(redux.store); + makeStore(); + initStore(store); database.setActiveDB.mockReset(); database.active.get.mockReset().mockImplementation((name: string) => (collections[name] ??= makeCollection(name))); database.active.write.mockReset().mockImplementation((fn: () => unknown) => fn()); @@ -114,82 +143,79 @@ beforeEach(() => { }); afterEach(() => { - jest.useRealTimers(); + sdk.disconnect(); + if (_setUserTimer.setUserTimer) clearTimeout(_setUserTimer.setUserTimer); + _setUserTimer.setUserTimer = null; }); async function connectAndDriveHandshake(server = 'https://example.com') { - await connect({ server }); - await flush(); - expect(mockConnections.length).toBeGreaterThan(0); - mockConnections[0].onopen(); - await flush(); + const index = mockTransport.connections.length; + const connecting = connect({ server }); + mockTransport.open(await mockTransport.awaitConnection(index)); + await connecting; + await recorder.awaitAction(connectSuccess().type); + state.meteor.connected = true; } describe('connect() over the real SDK', () => { it('dispatches connectRequest when connecting and connectSuccess once on the handshake', async () => { await connectAndDriveHandshake(); - expect(redux.store.dispatch).toHaveBeenCalledWith(connectRequest()); - expect(redux.store.dispatch).toHaveBeenCalledWith(connectSuccess()); - expect(redux.store.dispatch.mock.calls.filter(([action]) => action.type === connectSuccess().type)).toHaveLength(1); + expect(store.dispatch).toHaveBeenCalledWith(connectRequest()); + expect(store.dispatch).toHaveBeenCalledWith(connectSuccess()); + expect(recorder.actionsOfType(connectSuccess().type)).toHaveLength(1); }); it('ignores a repeated connected frame after the first', async () => { await connectAndDriveHandshake(); - redux.state.meteor.connected = true; - receiveFrame(mockConnections[0], { msg: 'connected', session: 'again' }); - await flush(); + mockTransport.deliver({ msg: 'connected', session: 'again' }); + await expect(sdk.driver?.probe(ROUND_TRIP_BUDGET)).resolves.toBe(true); - expect(redux.store.dispatch.mock.calls.filter(([action]) => action.type === connectSuccess().type)).toHaveLength(1); + expect(recorder.actionsOfType(connectSuccess().type)).toHaveLength(1); }); it('dispatches disconnect when the socket closes', async () => { await connectAndDriveHandshake(); - mockConnections[0].onclose({ code: 1006 }); - await flush(); + mockTransport.closeTransport(); - expect(redux.store.dispatch).toHaveBeenCalledWith(disconnectAction()); + await expect(recorder.awaitAction(disconnectAction().type)).resolves.toEqual(disconnectAction()); }); it('resumes login with the stored token once connected', async () => { - redux.state.login.user = { token: 'stored-token' }; + state.login.user = { token: 'stored-token' }; await connectAndDriveHandshake(); - expect(redux.store.dispatch).toHaveBeenCalledWith(loginRequest({ resume: 'stored-token' }, false)); + expect(recorder.requireAction(loginRequest({}, false).type)).toEqual(loginRequest({ resume: 'stored-token' }, false)); }); it('tears down the prior connection and stops its listeners when connect() is re-run', async () => { await connectAndDriveHandshake('https://a.example.com'); - const firstConnection = mockConnections[0]; - const successCount = () => redux.store.dispatch.mock.calls.filter(([action]) => action.type === connectSuccess().type).length; - const before = successCount(); + const firstConnection = mockTransport.connections[0]; + const successBefore = recorder.actionsOfType(connectSuccess().type).length; + state.meteor.connected = false; - await connect({ server: 'https://b.example.com' }); - await flush(); + const reconnecting = connect({ server: 'https://b.example.com' }); + await mockTransport.awaitConnection(1); + await reconnecting; - expect(firstConnection.close).toHaveBeenCalled(); + expect(firstConnection.readyState).toBe(3); - firstConnection.onmessage({ data: JSON.stringify({ msg: 'connected', session: 'x' }) }); - await flush(); + mockTransport.deliver({ msg: 'connected', session: 'x' }, firstConnection); + mockTransport.open(mockTransport.connections[1]); + await recorder.awaitAction(connectSuccess().type); - expect(successCount()).toBe(before); + expect(recorder.actionsOfType(connectSuccess().type)).toHaveLength(successBefore + 1); }); }); describe('login() over the real SDK', () => { - async function connectLoggedIn() { - await connectAndDriveHandshake(); - } - it('maps the server login result to the logged user', async () => { - await connectLoggedIn(); + await connectAndDriveHandshake(); - const loginPromise = login({ user: 'the-user', password: 'secret' }); - await flush(); - const user = await loginPromise; + const user = await login({ user: 'the-user', password: 'secret' }); expect(user).toEqual( expect.objectContaining({ @@ -208,11 +234,9 @@ describe('login() over the real SDK', () => { }); it('defaults the parser/main-thread preferences on servers >= 5.0.0', async () => { - await connectLoggedIn(); + await connectAndDriveHandshake(); - const loginPromise = login({ user: 'the-user', password: 'secret' }); - await flush(); - const user = await loginPromise; + const user = await login({ user: 'the-user', password: 'secret' }); expect(user).toEqual( expect.objectContaining({ @@ -223,15 +247,13 @@ describe('login() over the real SDK', () => { }); it('reads the parser/main-thread preferences from the server below 5.0.0', async () => { - redux.state.server.version = '4.9.0'; + state.server.version = '4.9.0'; (REST_LOGIN_ME.settings.preferences as Record).enableMessageParserEarlyAdoption = false; (REST_LOGIN_ME.settings.preferences as Record).showMessageInMainThread = true; - await connectLoggedIn(); + await connectAndDriveHandshake(); - const loginPromise = login({ user: 'the-user', password: 'secret' }); - await flush(); - const user = await loginPromise; + const user = await login({ user: 'the-user', password: 'secret' }); expect(user).toEqual( expect.objectContaining({ @@ -242,12 +264,10 @@ describe('login() over the real SDK', () => { }); it('sends LDAP params on the wire when LDAP is enabled', async () => { - redux.state.settings.LDAP_Enable = true; - await connectLoggedIn(); + state.settings.LDAP_Enable = true; + await connectAndDriveHandshake(); - const loginPromise = loginWithPassword({ user: 'the-user', password: 'secret' }); - await flush(); - await loginPromise; + await loginWithPassword({ user: 'the-user', password: 'secret' }); const loginCall = (global.fetch as jest.Mock).mock.calls.find(([url]) => String(url).includes('/api/v1/login')); const body = JSON.parse(loginCall[1].body); @@ -255,12 +275,10 @@ describe('login() over the real SDK', () => { }); it('sends CROWD params on the wire when CROWD is enabled', async () => { - redux.state.settings.CROWD_Enable = true; - await connectLoggedIn(); + state.settings.CROWD_Enable = true; + await connectAndDriveHandshake(); - const loginPromise = loginWithPassword({ user: 'the-user', password: 'secret' }); - await flush(); - await loginPromise; + await loginWithPassword({ user: 'the-user', password: 'secret' }); const loginCall = (global.fetch as jest.Mock).mock.calls.find(([url]) => String(url).includes('/api/v1/login')); const body = JSON.parse(loginCall[1].body); @@ -273,97 +291,95 @@ describe('onStreamData handlers over real frames', () => { await connectAndDriveHandshake(); database.active.get('settings').find.mockResolvedValue({ update: jest.fn(async (fn: (u: unknown) => void) => fn({})) }); - receiveFrame(mockConnections[0], { + mockTransport.deliver({ msg: 'changed', collection: 'stream-notify-all', fields: { eventName: 'public-settings-changed', args: [null, { _id: 'Site_Name', value: 'New Name' }] } }); - await flush(); - expect(redux.store.dispatch).toHaveBeenCalledWith(updateSettings('Site_Name', 'New Name')); + await expect(recorder.awaitAction(updateSettings('Site_Name', 'New Name').type)).resolves.toEqual( + updateSettings('Site_Name', 'New Name') + ); }); it('stream-user-presence sets the active user and the logged user', async () => { - redux.state.login.user = { id: 'user-id' }; + state.login.user = { id: 'user-id' }; await connectAndDriveHandshake(); - receiveFrame(mockConnections[0], { + mockTransport.deliver({ msg: 'changed', collection: 'stream-user-presence', fields: { uid: 'user-id', args: [['user-id', 1, '', '', undefined]] } }); - await flush(); + await recorder.awaitAction(setUser({}).type); - expect(redux.store.dispatch).toHaveBeenCalledWith( - setActiveUsers({ 'user-id': expect.objectContaining({ status: 'online' }) }) - ); - expect(redux.store.dispatch).toHaveBeenCalledWith(setUser(expect.objectContaining({ status: 'online' }))); + expect(store.dispatch).toHaveBeenCalledWith(setActiveUsers({ 'user-id': expect.objectContaining({ status: 'online' }) })); + expect(store.dispatch).toHaveBeenCalledWith(setUser(expect.objectContaining({ status: 'online' }))); }); it('user-status batches into _activeUsers and sets the logged user', async () => { - redux.state.login.user = { id: 'user-id' }; + state.login.user = { id: 'user-id' }; await connectAndDriveHandshake(); - receiveFrame(mockConnections[0], { + mockTransport.deliver({ msg: 'changed', collection: 'stream-notify-logged', fields: { eventName: 'user-status', args: [['user-id', 'online', 1, '', '', undefined]] } }); - await flush(); + await recorder.awaitAction(setUser({}).type); expect(_activeUsers.activeUsers['user-id']).toEqual(expect.objectContaining({ status: 'online' })); - expect(redux.store.dispatch).toHaveBeenCalledWith(setUser(expect.objectContaining({ status: 'online' }))); + expect(store.dispatch).toHaveBeenCalledWith(setUser(expect.objectContaining({ status: 'online' }))); }); it('permissions-changed dispatches updatePermission', async () => { await connectAndDriveHandshake(); database.active.get('permissions').find.mockResolvedValue({ update: jest.fn(async (fn: (u: unknown) => void) => fn({})) }); - receiveFrame(mockConnections[0], { + mockTransport.deliver({ msg: 'changed', collection: 'stream-notify-logged', fields: { eventName: 'permissions-changed', args: [null, { _id: 'create-c', roles: ['admin'] }] } }); - await flush(); - expect(redux.store.dispatch).toHaveBeenCalledWith(updatePermission('create-c', ['admin'])); + await expect(recorder.awaitAction(updatePermission('create-c', ['admin']).type)).resolves.toEqual( + updatePermission('create-c', ['admin']) + ); }); it('Users:NameChanged upserts the user in the database', async () => { await connectAndDriveHandshake(); const collection = database.active.get('users'); collection.find.mockResolvedValue({ update: jest.fn(async (fn: (u: unknown) => void) => fn({})) }); + const writes = trackCalls(database.active.write); - receiveFrame(mockConnections[0], { + mockTransport.deliver({ msg: 'changed', collection: 'stream-notify-logged', fields: { eventName: 'Users:NameChanged', args: [{ _id: 'user-id', username: 'renamed' }] } }); - await flush(); + await writes.awaitCall(); expect(collection.find).toHaveBeenCalledWith('user-id'); - expect(database.active.write).toHaveBeenCalled(); }); it('stream-force_logout dispatches logout(true)', async () => { await connectAndDriveHandshake(); - receiveFrame(mockConnections[0], { msg: 'changed', collection: 'stream-force_logout', fields: {} }); - await flush(); + mockTransport.deliver({ msg: 'changed', collection: 'stream-force_logout', fields: {} }); - expect(redux.store.dispatch).toHaveBeenCalledWith(logout(true)); + await expect(recorder.awaitAction(logout(true).type)).resolves.toEqual(logout(true)); }); it('users frame feeds _setUser', async () => { await connectAndDriveHandshake(); - receiveFrame(mockConnections[0], { + mockTransport.deliver({ msg: 'added', collection: 'users', id: 'user-id', fields: { username: 'the-user', status: 'online' } }); - await flush(); expect(_activeUsers.activeUsers['user-id']).toEqual(expect.objectContaining({ status: 'online' })); }); @@ -371,14 +387,12 @@ describe('onStreamData handlers over real frames', () => { describe('sdk.subscribeRoom() over the real SDK', () => { it('subscribes to the room streams for servers >= 4.0.0', async () => { - redux.state.server.version = '5.0.0'; + state.server.version = '5.0.0'; await connectAndDriveHandshake(); - const subscribing = sdk.subscribeRoom('room-rid'); - await flush(); - await subscribing; + await sdk.subscribeRoom('room-rid'); - const subs = framesOn(mockConnections[0], 'sub'); + const subs = mockTransport.frames({ msg: 'sub' }); expect(subs.map(sub => sub.name)).toEqual([ 'stream-notify-room', 'stream-room-messages', @@ -396,14 +410,11 @@ describe('sdk.subscribeRoom() over the real SDK', () => { }); it('subscribes to the typing event on servers below 4.0.0', async () => { - redux.state.server.version = '3.9.0'; + state.server.version = '3.9.0'; await connectAndDriveHandshake(); - const subscribing = sdk.subscribeRoom('room-rid'); - await flush(); - await subscribing; + await sdk.subscribeRoom('room-rid'); - const subs = framesOn(mockConnections[0], 'sub'); - expect(subs[0].params?.[0]).toBe('room-rid/typing'); + expect(mockTransport.frames({ msg: 'sub' })[0].params?.[0]).toBe('room-rid/typing'); }); }); diff --git a/app/lib/services/__tests__/sdkTransport.integration.test.ts b/app/lib/services/__tests__/sdkTransport.integration.test.ts new file mode 100644 index 0000000000..fbe9240bb8 --- /dev/null +++ b/app/lib/services/__tests__/sdkTransport.integration.test.ts @@ -0,0 +1,90 @@ +import { connectAuthenticatedSdk, createTransportFake } from '../../testUtils/sdkTransport'; +import type { RealSdkClient } from '../../testUtils/sdkTransport'; + +const mockTransport = createTransportFake(); + +jest.mock('universal-websocket-client', () => jest.fn().mockImplementation(() => mockTransport.createConnection())); + +let client: RealSdkClient; + +beforeEach(() => { + jest.clearAllMocks(); + mockTransport.reset(); +}); + +afterEach(async () => { + await client?.disconnect(); +}); + +describe('SDK transport fake', () => { + it('reaches connected and authenticated state through public connect and login', async () => { + client = await connectAuthenticatedSdk(mockTransport, { token: 'resume-token' }); + + expect(client.driver.connected).toBe(true); + expect(client.userId).toBe('user-id'); + expect(mockTransport.requireFrame({ msg: 'method', method: 'login' }).params).toEqual([{ resume: 'resume-token' }]); + }); + + it('completes a public method call when its server frame is delivered', async () => { + client = await connectAuthenticatedSdk(mockTransport); + + const pending = client.methodCall('getRoomIdByNameOrId', 'general'); + const request = await mockTransport.awaitFrame({ msg: 'method', method: 'getRoomIdByNameOrId' }); + mockTransport.respond(request, 'room-id'); + + await expect(pending).resolves.toBe('room-id'); + }); + + it('records subscription frames sent through a public subscribe operation', async () => { + client = await connectAuthenticatedSdk(mockTransport); + + const subscription = await client.subscribeRaw('stream-notify-user', ['user-id/media-signal', false]); + const request = await mockTransport.awaitFrame({ msg: 'sub', name: 'stream-notify-user' }); + + expect(subscription?.id).toBe(request.id); + expect(request.params).toEqual(['user-id/media-signal', false]); + }); + + it('keeps an operation pending while its response is withheld', async () => { + client = await connectAuthenticatedSdk(mockTransport); + mockTransport.withhold({ msg: 'sub' }); + + let settled = false; + const pending = client.subscribeRaw('stream-notify-user', ['user-id/media-calls', false]).then(subscription => { + settled = true; + return subscription; + }); + const request = await mockTransport.awaitFrame({ msg: 'sub' }); + await Promise.resolve(); + + expect(settled).toBe(false); + + mockTransport.deliver({ msg: 'ready', subs: [request.id] }); + + await expect(pending).resolves.toEqual(expect.objectContaining({ id: request.id })); + }); + + it('rejects an in-flight public method call when recovery replaces the transport', async () => { + client = await connectAuthenticatedSdk(mockTransport); + + const pending = client.methodCall('getUsersOfRoom', 'room-id'); + await mockTransport.awaitFrame({ msg: 'method', method: 'getUsersOfRoom' }); + + const recovering = client.driver.reopenNow(); + + await expect(pending).rejects.toThrow('[ddp] connection reopened before the response arrived'); + + mockTransport.open(await mockTransport.awaitConnection(1)); + await recovering; + }); + + it('closes the transport through a public disconnect operation', async () => { + client = await connectAuthenticatedSdk(mockTransport); + const connection = mockTransport.latestConnection; + + await client.disconnect(); + + expect(connection.readyState).toBe(3); + expect(client.driver.connected).toBe(false); + }); +}); diff --git a/app/lib/services/__tests__/socketHealth.integration.test.ts b/app/lib/services/__tests__/socketHealth.integration.test.ts index aaae464b76..2c6359a209 100644 --- a/app/lib/services/__tests__/socketHealth.integration.test.ts +++ b/app/lib/services/__tests__/socketHealth.integration.test.ts @@ -1,239 +1,184 @@ -import sdk from '../sdk'; +import sdk, { type ISocketDriver } from '../sdk'; import { recoverSocket } from '../socketHealth'; -import { - addMediaSubs, - backdateLastPing, - buildConnectedDriver, - framesOn, - stopAnsweringFrames -} from '../../testUtils/sdkIntegration'; -import type { IMockSdk, MockConnection, IMockSdkDriver } from '../../testUtils/sdkIntegration'; -import type * as SdkIntegration from '../../testUtils/sdkIntegration'; - -const mockConnections: MockConnection[] = []; - -jest.mock('universal-websocket-client', () => - jest.fn().mockImplementation(() => { - const sdkIntegration = jest.requireActual('../../testUtils/sdkIntegration'); - return new sdkIntegration.MockConnection(mockConnections); - }) -); +import { connectAuthenticatedSdk, createTransportFake } from '../../testUtils/sdkTransport'; +import type { FakeConnection, RealSdkClient } from '../../testUtils/sdkTransport'; +import type * as SdkModuleFake from '../../testUtils/sdkModuleFake'; + +const mockTransport = createTransportFake(); + +jest.mock('universal-websocket-client', () => jest.fn().mockImplementation(() => mockTransport.createConnection())); jest.mock('../sdk', () => { - const sdkIntegration = jest.requireActual('../../testUtils/sdkIntegration'); - return { __esModule: true, default: sdkIntegration.makeSdkMock() }; + const { createSdkModuleFake } = jest.requireActual('../../testUtils/sdkModuleFake'); + return { __esModule: true, default: createSdkModuleFake() }; }); const USER_ID = 'user-id'; -const PING_INTERVAL = 10000; +const ROUND_TRIP_BUDGET = 2000; +const RESUBSCRIBE_POLL = 100; const CLOSED = 3; -describe('recoverSocket against the real SDK socket', () => { - let driver: IMockSdkDriver; +const MEDIA_SUBS = [ + { id: expect.any(String), name: 'stream-notify-user', params: [`${USER_ID}/media-signal`, false] }, + { id: expect.any(String), name: 'stream-notify-user', params: [`${USER_ID}/media-calls`, false] } +]; + +describe('recoverSocket over the public SDK driver and an app-owned transport', () => { + let client: RealSdkClient; + let driver: ISocketDriver; + let frozen: FakeConnection; beforeEach(async () => { jest.clearAllMocks(); jest.useFakeTimers(); - mockConnections.length = 0; - driver = await buildConnectedDriver(mockConnections, USER_ID); - (sdk as unknown as IMockSdk).setClient({ driver }); + mockTransport.reset(); + client = await connectAuthenticatedSdk(mockTransport); + driver = client.driver; + frozen = mockTransport.latestConnection; + (sdk as unknown as SdkModuleFake.ISdkModuleFake).setClient({ driver }); }); - afterEach(() => { - if (driver.socket.pingTimeout) clearTimeout(driver.socket.pingTimeout); - if (driver.socket.openTimeout) clearTimeout(driver.socket.openTimeout); + afterEach(async () => { + await client.disconnect(); jest.useRealTimers(); }); - it('exposes the ping interval the health classification depends on', () => { - expect(driver.pingInterval).toBe(PING_INTERVAL); - }); - - it('keeps a doubtful socket when the round trip gets a pong', async () => { - backdateLastPing(driver, PING_INTERVAL + 5000); - - const recovery = recoverSocket(); - await jest.advanceTimersByTimeAsync(0); - - await expect(recovery).resolves.toBe('confirmed-alive'); - expect(framesOn(mockConnections[0], 'ping').length).toBeGreaterThan(0); - expect(mockConnections).toHaveLength(1); - }); - - it('reopens a doubtful socket when the round trip gets no pong', async () => { - backdateLastPing(driver, PING_INTERVAL + 5000); - stopAnsweringFrames(mockConnections[0]); + async function subscribeToMediaStreams(): Promise { + await Promise.all([ + client.subscribeRaw('stream-notify-user', [`${USER_ID}/media-signal`, false]), + client.subscribeRaw('stream-notify-user', [`${USER_ID}/media-calls`, false]) + ]); + } + async function reopenAfterUnansweredRoundTrip(): Promise<{ recovery: Promise; reopened: FakeConnection }> { + mockTransport.withhold({ msg: 'ping' }); const recovery = recoverSocket(); - await jest.advanceTimersByTimeAsync(2000); - - expect(framesOn(mockConnections[0], 'ping').length).toBeGreaterThan(0); - expect(mockConnections).toHaveLength(2); - mockConnections[1].onopen(); - await jest.advanceTimersByTimeAsync(0); - - await expect(recovery).resolves.toBe('reopened'); + await mockTransport.awaitFrame({ msg: 'ping' }, frozen); + await jest.advanceTimersByTimeAsync(ROUND_TRIP_BUDGET); + const reopened = await mockTransport.awaitConnection(1); + mockTransport.open(reopened); + return { recovery, reopened }; + } + + it('keeps a live socket when the round trip gets a pong', async () => { + await expect(recoverSocket()).resolves.toBe('confirmed-alive'); + + expect(mockTransport.frames({ msg: 'ping' }, frozen).length).toBeGreaterThan(0); + expect(mockTransport.connections).toHaveLength(1); }); - it('reopens a frozen socket whose young lastPing sits on an unusable session', async () => { - stopAnsweringFrames(mockConnections[0]); - - const recovery = recoverSocket(); - await jest.advanceTimersByTimeAsync(2000); - - expect(framesOn(mockConnections[0], 'ping').length).toBeGreaterThan(0); - expect(mockConnections).toHaveLength(2); - mockConnections[1].onopen(); - await jest.advanceTimersByTimeAsync(0); + it('reopens a frozen socket when the round trip gets no pong', async () => { + const { recovery } = await reopenAfterUnansweredRoundTrip(); + expect(mockTransport.frames({ msg: 'ping' }, frozen).length).toBeGreaterThan(0); + expect(mockTransport.connections).toHaveLength(2); await expect(recovery).resolves.toBe('reopened'); }); - it('reopens a known-dead socket without a round trip', async () => { - backdateLastPing(driver, PING_INTERVAL * 2 + 1000); + it('reopens a closed transport without a round trip', async () => { + mockTransport.closeTransport(frozen); + const pingsBefore = mockTransport.frames({ msg: 'ping' }, frozen).length; const recovery = recoverSocket(); - await jest.advanceTimersByTimeAsync(0); - - expect(mockConnections).toHaveLength(2); - expect(framesOn(mockConnections[0], 'ping')).toHaveLength(0); - - mockConnections[1].onopen(); - await jest.advanceTimersByTimeAsync(0); + const reopened = await mockTransport.awaitConnection(1); + mockTransport.open(reopened); await expect(recovery).resolves.toBe('reopened'); + expect(mockTransport.frames({ msg: 'ping' }, frozen)).toHaveLength(pingsBefore); + expect(frozen.readyState).toBe(CLOSED); }); it('shares one reopen with a concurrent direct reopenNow', async () => { - backdateLastPing(driver, PING_INTERVAL * 3); + mockTransport.closeTransport(frozen); const directReopen = driver.reopenNow(); const recovery = recoverSocket(); - await jest.advanceTimersByTimeAsync(0); - expect(mockConnections).toHaveLength(2); - mockConnections[1].onopen(); - await jest.advanceTimersByTimeAsync(0); + mockTransport.open(await mockTransport.awaitConnection(1)); await directReopen; await expect(recovery).resolves.toBe('reopened'); - expect(mockConnections).toHaveLength(2); + expect(mockTransport.connections).toHaveLength(2); await jest.advanceTimersByTimeAsync(60000); - expect(mockConnections).toHaveLength(2); + expect(mockTransport.connections).toHaveLength(2); }); - it('rejects an in-flight DDP method call when recovery reopens the socket', async () => { - let rejected = false; - const inFlight = driver.socket.send({ msg: 'method', method: 'getRoomByTypeAndName', params: [] }).catch(() => { - rejected = true; - }); - await jest.advanceTimersByTimeAsync(0); - expect(rejected).toBe(false); + it('shares one reopen between two concurrent recoverSocket calls', async () => { + mockTransport.closeTransport(frozen); - backdateLastPing(driver, PING_INTERVAL * 3); + const first = recoverSocket(); + const second = recoverSocket(); - const recovery = recoverSocket(); - await jest.advanceTimersByTimeAsync(0); - await inFlight; - expect(rejected).toBe(true); + mockTransport.open(await mockTransport.awaitConnection(1)); - mockConnections[1].onopen(); - await jest.advanceTimersByTimeAsync(0); + await expect(first).resolves.toBe('reopened'); + await expect(second).resolves.toBe('reopened'); + expect(mockTransport.connections).toHaveLength(2); + }); + it('rejects an in-flight method call when recovery reopens the socket', async () => { + mockTransport.withhold({ msg: 'method', method: 'getRoomByTypeAndName' }); + let rejection: Error | undefined; + const inFlight = client.methodCall('getRoomByTypeAndName', 'general').catch((error: Error) => { + rejection = error; + }); + await mockTransport.awaitFrame({ msg: 'method', method: 'getRoomByTypeAndName' }, frozen); + + const { recovery } = await reopenAfterUnansweredRoundTrip(); + + await inFlight; + expect(rejection?.message).toBe('[ddp] connection reopened before the response arrived'); await expect(recovery).resolves.toBe('reopened'); }); - it('re-sends the media subscriptions on the new socket reusing their ids', async () => { - backdateLastPing(driver, PING_INTERVAL * 3); - addMediaSubs(driver, USER_ID); + it('re-sends the media subscriptions on the reopened socket reusing their ids', async () => { + await subscribeToMediaStreams(); + const establishedSubs = mockTransport.frames({ msg: 'sub' }, frozen); - const recovery = recoverSocket(); - await jest.advanceTimersByTimeAsync(0); - mockConnections[1].onopen(); - await jest.advanceTimersByTimeAsync(0); + const { recovery, reopened } = await reopenAfterUnansweredRoundTrip(); await expect(recovery).resolves.toBe('reopened'); const resubscribed = driver.waitForNotifyUserMediaSubs(); - await jest.advanceTimersByTimeAsync(200); + await jest.advanceTimersByTimeAsync(RESUBSCRIBE_POLL); await expect(resubscribed).resolves.toBe(true); - expect(framesOn(mockConnections[0], 'sub')).toHaveLength(0); - expect(framesOn(mockConnections[1], 'sub')).toEqual([ - expect.objectContaining({ id: 'sub-0', name: 'stream-notify-user', params: [`${USER_ID}/media-signal`] }), - expect.objectContaining({ id: 'sub-1', name: 'stream-notify-user', params: [`${USER_ID}/media-calls`] }) - ]); + expect(mockTransport.frames({ msg: 'sub' }, reopened)).toEqual( + establishedSubs.map(sub => expect.objectContaining({ id: sub.id, name: sub.name, params: sub.params })) + ); }); - it('reopens a closed transport without a round trip even when lastPing is fresh', async () => { - mockConnections[0].readyState = CLOSED; - - const recovery = recoverSocket(); - await jest.advanceTimersByTimeAsync(0); - - expect(mockConnections).toHaveLength(2); - expect(framesOn(mockConnections[0], 'ping')).toHaveLength(0); - - mockConnections[1].onopen(); - await jest.advanceTimersByTimeAsync(0); - - await expect(recovery).resolves.toBe('reopened'); - }); - - it('waits for media subs to appear after reopen, then re-acks them', async () => { - backdateLastPing(driver, PING_INTERVAL * 3); - - const recovery = recoverSocket(); - await jest.advanceTimersByTimeAsync(0); - mockConnections[1].onopen(); - await jest.advanceTimersByTimeAsync(0); + it('waits for media subs to appear after the reopen, then re-acks them', async () => { + const { recovery, reopened } = await reopenAfterUnansweredRoundTrip(); await expect(recovery).resolves.toBe('reopened'); const resubscribed = driver.waitForNotifyUserMediaSubs(1000); - await jest.advanceTimersByTimeAsync(100); - expect(framesOn(mockConnections[1], 'sub')).toHaveLength(0); + await jest.advanceTimersByTimeAsync(RESUBSCRIBE_POLL); + expect(mockTransport.frames({ msg: 'sub' }, reopened)).toHaveLength(0); - addMediaSubs(driver, USER_ID); - await jest.advanceTimersByTimeAsync(200); + await subscribeToMediaStreams(); + const establishedSubs = mockTransport.frames({ msg: 'sub' }, reopened); + expect(establishedSubs).toEqual(MEDIA_SUBS.map(sub => expect.objectContaining(sub))); + await jest.advanceTimersByTimeAsync(RESUBSCRIBE_POLL); await expect(resubscribed).resolves.toBe(true); - expect(framesOn(mockConnections[1], 'sub')).toEqual([ - expect.objectContaining({ id: 'sub-0', name: 'stream-notify-user', params: [`${USER_ID}/media-signal`] }), - expect.objectContaining({ id: 'sub-1', name: 'stream-notify-user', params: [`${USER_ID}/media-calls`] }) + expect(mockTransport.frames({ msg: 'sub' }, reopened)).toEqual([ + ...establishedSubs, + ...establishedSubs.map(sub => expect.objectContaining({ id: sub.id, params: sub.params })) ]); }); it('resolves false when the reopened socket never acks the re-sub', async () => { - backdateLastPing(driver, PING_INTERVAL * 3); - addMediaSubs(driver, USER_ID); + await subscribeToMediaStreams(); - const recovery = recoverSocket(); - await jest.advanceTimersByTimeAsync(0); - mockConnections[1].onopen(); - await jest.advanceTimersByTimeAsync(0); + const { recovery } = await reopenAfterUnansweredRoundTrip(); await expect(recovery).resolves.toBe('reopened'); - stopAnsweringFrames(mockConnections[1]); + mockTransport.withhold({ msg: 'sub' }); const resubscribed = driver.waitForNotifyUserMediaSubs(500); await jest.advanceTimersByTimeAsync(500); await expect(resubscribed).resolves.toBe(false); }); - - it('shares one reopen between two concurrent recoverSocket calls', async () => { - backdateLastPing(driver, PING_INTERVAL * 3); - - const first = recoverSocket(); - const second = recoverSocket(); - await jest.advanceTimersByTimeAsync(0); - - expect(mockConnections).toHaveLength(2); - mockConnections[1].onopen(); - await jest.advanceTimersByTimeAsync(0); - - await expect(first).resolves.toBe('reopened'); - await expect(second).resolves.toBe('reopened'); - expect(mockConnections).toHaveLength(2); - }); }); diff --git a/app/lib/services/__tests__/socketHealth.test.ts b/app/lib/services/__tests__/socketHealth.test.ts index ef51619afb..f26a0e3faa 100644 --- a/app/lib/services/__tests__/socketHealth.test.ts +++ b/app/lib/services/__tests__/socketHealth.test.ts @@ -1,57 +1,50 @@ import sdk, { type ISocketDriver } from '../sdk'; import { classifySocketHealth, recoverSocket } from '../socketHealth'; -import { buildConnectedDriver } from '../../testUtils/sdkIntegration'; -import type { IMockSdk, IMockSdkDriver, MockConnection } from '../../testUtils/sdkIntegration'; -import type * as SdkIntegration from '../../testUtils/sdkIntegration'; +import { connectAuthenticatedSdk, createTransportFake } from '../../testUtils/sdkTransport'; +import type { RealSdkClient } from '../../testUtils/sdkTransport'; +import type * as SdkModuleFake from '../../testUtils/sdkModuleFake'; -const mockConnections: MockConnection[] = []; +const mockTransport = createTransportFake(); -jest.mock('universal-websocket-client', () => - jest.fn().mockImplementation(() => { - const sdkIntegration = jest.requireActual('../../testUtils/sdkIntegration'); - return new sdkIntegration.MockConnection(mockConnections); - }) -); +jest.mock('universal-websocket-client', () => jest.fn().mockImplementation(() => mockTransport.createConnection())); jest.mock('../sdk', () => { - const sdkIntegration = jest.requireActual('../../testUtils/sdkIntegration'); - return { __esModule: true, default: sdkIntegration.makeSdkMock() }; + const { createSdkModuleFake } = jest.requireActual('../../testUtils/sdkModuleFake'); + return { __esModule: true, default: createSdkModuleFake() }; }); -const sdkMock = sdk as unknown as IMockSdk; +const sdkMock = sdk as unknown as SdkModuleFake.ISdkModuleFake; -const USER_ID = 'user-id'; -const CLOSED = 3; - -describe('socket health against a driver from the shared harness', () => { - let driver: IMockSdkDriver; +describe('socket health over the public SDK driver', () => { + let client: RealSdkClient; + let driver: ISocketDriver; let probe: jest.SpyInstance, [number?]>; let reopenNow: jest.SpyInstance, []>; beforeEach(async () => { jest.clearAllMocks(); - jest.useFakeTimers(); - mockConnections.length = 0; - driver = await buildConnectedDriver(mockConnections, USER_ID); + mockTransport.reset(); + client = await connectAuthenticatedSdk(mockTransport); + driver = client.driver; probe = jest.spyOn(driver, 'probe').mockResolvedValue(true); reopenNow = jest.spyOn(driver, 'reopenNow').mockResolvedValue(); sdkMock.setClient({ driver }); }); - afterEach(() => { - if (driver.socket.pingTimeout) clearTimeout(driver.socket.pingTimeout); - if (driver.socket.openTimeout) clearTimeout(driver.socket.openTimeout); - jest.useRealTimers(); + afterEach(async () => { + reopenNow.mockRestore(); + probe.mockRestore(); + await client.disconnect(); }); describe('classifySocketHealth', () => { it('returns round-trip-check for a connected socket rather than trusting it outright', () => { - expect(classifySocketHealth(driver as unknown as ISocketDriver)).toBe('round-trip-check'); + expect(classifySocketHealth(driver)).toBe('round-trip-check'); }); - it('returns reopen for a closed socket even when lastPing is fresh', () => { - mockConnections[0].readyState = CLOSED; - expect(classifySocketHealth(driver as unknown as ISocketDriver)).toBe('reopen'); + it('returns reopen once the transport is closed', () => { + mockTransport.closeTransport(); + expect(classifySocketHealth(driver)).toBe('reopen'); }); }); @@ -73,7 +66,7 @@ describe('socket health against a driver from the shared harness', () => { }); it('reopens a known-dead socket without a round trip', async () => { - mockConnections[0].readyState = CLOSED; + mockTransport.closeTransport(); await expect(recoverSocket()).resolves.toBe('reopened'); expect(probe).not.toHaveBeenCalled(); expect(reopenNow).toHaveBeenCalledTimes(1); @@ -99,7 +92,7 @@ describe('socket health against a driver from the shared harness', () => { }); it('rejects when reopening throws', async () => { - mockConnections[0].readyState = CLOSED; + mockTransport.closeTransport(); reopenNow.mockRejectedValue(new Error('reopen failed')); await expect(recoverSocket()).rejects.toThrow('reopen failed'); }); diff --git a/app/lib/services/restApi.test.ts b/app/lib/services/restApi.test.ts index 7405c6d633..661a561574 100644 --- a/app/lib/services/restApi.test.ts +++ b/app/lib/services/restApi.test.ts @@ -1,19 +1,19 @@ import type { ServerMediaSignal } from '@rocket.chat/media-signaling'; import { Platform } from 'react-native'; -import type * as SdkIntegration from '../testUtils/sdkIntegration'; +import type * as SdkModuleFake from '../testUtils/sdkModuleFake'; import { mediaCallsStateSignals } from './restApi'; const mockSdkGet = jest.fn(); const mockSdkPost = jest.fn(); const mockSdkDel = jest.fn(); -let mockSdk!: SdkIntegration.IMockSdk; +let mockSdk!: SdkModuleFake.ISdkModuleFake; jest.mock('./sdk', () => { - const { makeSdkMock } = jest.requireActual('../testUtils/sdkIntegration'); + const { createSdkModuleFake } = jest.requireActual('../testUtils/sdkModuleFake'); mockSdk = mockSdk ?? - makeSdkMock({ + createSdkModuleFake({ get: (...args: unknown[]) => mockSdkGet(...args), post: (...args: unknown[]) => mockSdkPost(...args), del: (...args: unknown[]) => mockSdkDel(...args) diff --git a/app/lib/services/voip/MediaSessionInstance.test.ts b/app/lib/services/voip/MediaSessionInstance.test.ts index 1a35d57edc..5b57601926 100644 --- a/app/lib/services/voip/MediaSessionInstance.test.ts +++ b/app/lib/services/voip/MediaSessionInstance.test.ts @@ -3,7 +3,7 @@ import RNCallKeep from 'react-native-callkeep'; import { waitFor } from '@testing-library/react-native'; import type { IDDPMessage } from '../../../definitions/IDDPMessage'; -import type * as SdkIntegration from '../../testUtils/sdkIntegration'; +import type * as SdkModuleFake from '../../testUtils/sdkModuleFake'; import sdk from '../sdk'; import Navigation from '../../navigation/appNavigation'; import { getDMSubscriptionByUsername } from '../../database/services/Subscription'; @@ -58,7 +58,7 @@ jest.mock('./useCallStore', () => ({ } })); -const mockSdk = sdk as unknown as SdkIntegration.IMockSdk; +const mockSdk = sdk as unknown as SdkModuleFake.ISdkModuleFake; const SDK_HOST = 'https://open.rocket.chat'; const mockOnStreamDataStop = jest.fn(); @@ -67,10 +67,10 @@ const mockOnStreamData = jest.fn((_event: string, _callback: (message: IDDPMessa ); const mockMethodCall = jest.fn(); jest.mock('../sdk', () => { - const { makeSdkMock } = jest.requireActual('../../testUtils/sdkIntegration'); + const { createSdkModuleFake } = jest.requireActual('../../testUtils/sdkModuleFake'); return { __esModule: true, - default: makeSdkMock({ + default: createSdkModuleFake({ onStreamData: (...args: Parameters) => mockOnStreamData(...args), methodCall: (...args: unknown[]) => { mockMethodCall(...args); diff --git a/app/lib/services/voip/acceptNativeCall.integration.test.ts b/app/lib/services/voip/acceptNativeCall.integration.test.ts index 02f2753d22..aa97737d64 100644 --- a/app/lib/services/voip/acceptNativeCall.integration.test.ts +++ b/app/lib/services/voip/acceptNativeCall.integration.test.ts @@ -6,9 +6,9 @@ import { useCallStore } from './useCallStore'; import { initStore } from '../../store/auxStore'; import { recoverSocket } from '../socketHealth'; import sdk from '../sdk'; -import { addMediaSubs, buildConnectedDriver } from '../../testUtils/sdkIntegration'; -import type { IMockSdk, IMockSdkDriver, MockConnection } from '../../testUtils/sdkIntegration'; -import type * as SdkIntegration from '../../testUtils/sdkIntegration'; +import { connectAuthenticatedSdk, createTransportFake, subscribeMediaStreams } from '../../testUtils/sdkTransport'; +import type { RealSdkClient } from '../../testUtils/sdkTransport'; +import type { ISdkModuleFake } from '../../testUtils/sdkModuleFake'; import type { IApplicationState } from '../../../definitions'; jest.mock('./terminateNativeCall', () => ({ @@ -26,18 +26,13 @@ jest.mock('../socketHealth', () => ({ })); jest.mock('../sdk', () => { - const sdkIntegration = jest.requireActual('../../testUtils/sdkIntegration'); - return { __esModule: true, default: sdkIntegration.makeSdkMock() }; + const { createSdkModuleFake } = jest.requireActual('../../testUtils/sdkModuleFake'); + return { __esModule: true, default: createSdkModuleFake() }; }); -const mockConnections: MockConnection[] = []; +const mockTransport = createTransportFake(); -jest.mock('universal-websocket-client', () => - jest.fn().mockImplementation(() => { - const sdkIntegration = jest.requireActual('../../testUtils/sdkIntegration'); - return new sdkIntegration.MockConnection(mockConnections); - }) -); +jest.mock('universal-websocket-client', () => jest.fn().mockImplementation(() => mockTransport.createConnection())); jest.mock('../../methods/helpers/log', () => ({ __esModule: true, @@ -45,12 +40,12 @@ jest.mock('../../methods/helpers/log', () => ({ })); const CALL_ID = 'call-uuid'; -const USER_ID = 'user-id'; const READINESS_TIMEOUT = 8000; const mockTerminateNativeCall = terminateNativeCall as jest.Mock; const mockGetCallState = useCallStore.getState as jest.Mock; const mockRecoverSocket = recoverSocket as jest.MockedFunction; +const sdkModule = sdk as unknown as ISdkModuleFake; interface IMediaSession { applyRestStateSignals: jest.Mock, []>; @@ -68,10 +63,6 @@ function makeMediaSession(): IMediaSession { }; } -/** - * Minimal redux surface so `waitForLoginReady` runs for real: it reads - * `login.isAuthenticated` / `meteor.connected` and subscribes for changes. - */ function makeReduxStore() { const listeners = new Set<() => void>(); const state = { login: { isAuthenticated: false }, meteor: { connected: false } }; @@ -94,33 +85,32 @@ function makeReduxStore() { describe('acceptNativeCallWithReadiness against real login readiness', () => { let redux: ReturnType; - let driver: IMockSdkDriver; + let client: RealSdkClient; beforeEach(async () => { jest.clearAllMocks(); - jest.useFakeTimers(); - mockConnections.length = 0; + mockTransport.reset(); redux = makeReduxStore(); initStore(redux.store); mockGetCallState.mockReturnValue({ call: null, resetNativeCallId: jest.fn() }); mockRecoverSocket.mockResolvedValue('reopened'); - driver = await buildConnectedDriver(mockConnections, USER_ID); - addMediaSubs(driver, USER_ID); - (sdk as unknown as IMockSdk).setClient({ driver }); + client = await connectAuthenticatedSdk(mockTransport); + sdkModule.setClient(client); }); - afterEach(() => { - if (driver.socket.pingTimeout) clearTimeout(driver.socket.pingTimeout); - if (driver.socket.openTimeout) clearTimeout(driver.socket.openTimeout); + afterEach(async () => { jest.useRealTimers(); + await client.disconnect(); + sdkModule.setClient(null); }); it('recovers the socket, waits for readiness, then answers the call', async () => { + await subscribeMediaStreams(client); + jest.useFakeTimers(); const mediaSession = makeMediaSession(); const gate = acceptNativeCallWithReadiness(CALL_ID, mediaSession); - // Readiness only lands after the gate is already waiting on it. await jest.advanceTimersByTimeAsync(0); expect(mediaSession.answerCall).not.toHaveBeenCalled(); redux.setLoginReady(); @@ -135,6 +125,8 @@ describe('acceptNativeCallWithReadiness against real login readiness', () => { }); it('releases its store listener and readiness polling as soon as readiness lands', async () => { + await subscribeMediaStreams(client); + jest.useFakeTimers(); redux.setLoginReady(); const mediaSession = makeMediaSession(); @@ -144,21 +136,19 @@ describe('acceptNativeCallWithReadiness against real login readiness', () => { expect(redux.listenerCount()).toBe(0); - // Nothing is left scheduled: no late failure ladder. await jest.advanceTimersByTimeAsync(60000); expect(mockTerminateNativeCall).not.toHaveBeenCalled(); expect(mediaSession.endCall).not.toHaveBeenCalled(); }); it('runs the failure ladder once and leaves nothing behind when readiness never lands', async () => { - driver.socket.subscriptions = {}; + jest.useFakeTimers(); const resetNativeCallId = jest.fn(); mockGetCallState.mockReturnValue({ call: null, resetNativeCallId }); const mediaSession = makeMediaSession(); const gate = acceptNativeCallWithReadiness(CALL_ID, mediaSession); - // Login never authenticates and the media subs never ack. await jest.advanceTimersByTimeAsync(READINESS_TIMEOUT); await gate; diff --git a/app/lib/services/voip/acceptNativeCall.sdk.integration.test.ts b/app/lib/services/voip/acceptNativeCall.sdk.integration.test.ts index b51086e41e..80e8978d14 100644 --- a/app/lib/services/voip/acceptNativeCall.sdk.integration.test.ts +++ b/app/lib/services/voip/acceptNativeCall.sdk.integration.test.ts @@ -3,13 +3,15 @@ import { acceptNativeCallWithReadiness } from './acceptNativeCall'; import { useCallStore } from './useCallStore'; import { terminateNativeCall } from './terminateNativeCall'; import { waitForLoginReady } from '../waitForLoginReady'; -import { addMediaSubs, backdateLastPing, buildConnectedDriver, stopAnsweringFrames } from '../../testUtils/sdkIntegration'; -import type { IMockSdk, MockConnection, IMockSdkDriver } from '../../testUtils/sdkIntegration'; -import type * as SdkIntegration from '../../testUtils/sdkIntegration'; +import { connectAuthenticatedSdk, createTransportFake, subscribeMediaStreams } from '../../testUtils/sdkTransport'; +import type { FakeConnection, RealSdkClient } from '../../testUtils/sdkTransport'; +import type { ISdkModuleFake } from '../../testUtils/sdkModuleFake'; + +const mockTransport = createTransportFake(); jest.mock('../sdk', () => { - const sdkIntegration = jest.requireActual('../../testUtils/sdkIntegration'); - return { __esModule: true, default: sdkIntegration.makeSdkMock() }; + const { createSdkModuleFake } = jest.requireActual('../../testUtils/sdkModuleFake'); + return { __esModule: true, default: createSdkModuleFake() }; }); jest.mock('./useCallStore', () => ({ @@ -29,22 +31,17 @@ jest.mock('../../methods/helpers/log', () => ({ default: jest.fn() })); -const mockConnections: MockConnection[] = []; - -jest.mock('universal-websocket-client', () => - jest.fn().mockImplementation(() => { - const sdkIntegration = jest.requireActual('../../testUtils/sdkIntegration'); - return new sdkIntegration.MockConnection(mockConnections); - }) -); +jest.mock('universal-websocket-client', () => jest.fn().mockImplementation(() => mockTransport.createConnection())); const mockWaitForLoginReady = waitForLoginReady as jest.MockedFunction; const mockGetState = useCallStore.getState as jest.Mock; const mockTerminateNativeCall = terminateNativeCall as jest.Mock; +const sdkModule = sdk as unknown as ISdkModuleFake; const CALL_ID = 'call-uuid'; const USER_ID = 'user-id'; -const PING_INTERVAL = 10000; +const MEDIA_STREAMS = ['media-signal', 'media-calls']; +const READINESS_TIMEOUT = 8000; interface IMediaSession { applyRestStateSignals: jest.Mock>; @@ -63,38 +60,42 @@ function makeMediaSession(overrides: Partial = {}): IMediaSession }; } -let driver: IMockSdkDriver; +function mediaSubIdsOn(connection: FakeConnection): (string | undefined)[] { + const sent = mockTransport.frames({ msg: 'sub', name: 'stream-notify-user' }, connection); + return MEDIA_STREAMS.map(stream => sent.find(frame => frame.params?.[0] === `${USER_ID}/${stream}`)?.id); +} + +let client: RealSdkClient; beforeEach(async () => { jest.clearAllMocks(); - jest.useFakeTimers(); - mockConnections.length = 0; - driver = await buildConnectedDriver(mockConnections, USER_ID); - (sdk as unknown as IMockSdk).setClient({ driver }); + mockTransport.reset(); + client = await connectAuthenticatedSdk(mockTransport); + sdkModule.setClient(client); mockWaitForLoginReady.mockResolvedValue(true); mockGetState.mockReturnValue({ call: null, resetNativeCallId: jest.fn() }); }); -afterEach(() => { - if (driver.socket.pingTimeout) clearTimeout(driver.socket.pingTimeout); - if (driver.socket.openTimeout) clearTimeout(driver.socket.openTimeout); +afterEach(async () => { + await client.disconnect(); + sdkModule.setClient(null); jest.useRealTimers(); }); describe('acceptNativeCallWithReadiness against the real SDK socket', () => { - it('answers the call once media subs re-ack on the reopened socket', async () => { + it('replays the media subscription ids onto the reopened socket and answers the call', async () => { + await subscribeMediaStreams(client); + const originalIds = mediaSubIdsOn(mockTransport.connections[0]); + expect(originalIds).toEqual([expect.any(String), expect.any(String)]); const mediaSession = makeMediaSession(); - backdateLastPing(driver, PING_INTERVAL * 3); - addMediaSubs(driver, USER_ID); - + const reopened = mockTransport.awaitConnection(1); + mockTransport.closeTransport(mockTransport.connections[0]); const accept = acceptNativeCallWithReadiness(CALL_ID, mediaSession); - await jest.advanceTimersByTimeAsync(0); - mockConnections[1].onopen(); - await jest.advanceTimersByTimeAsync(0); - await jest.advanceTimersByTimeAsync(200); + mockTransport.open(await reopened); await accept; + expect(mediaSubIdsOn(mockTransport.connections[1])).toEqual(originalIds); expect(mockWaitForLoginReady).toHaveBeenCalledTimes(1); expect(mediaSession.applyRestStateSignals).toHaveBeenCalledTimes(1); expect(mediaSession.answerCall).toHaveBeenCalledWith(CALL_ID); @@ -102,21 +103,21 @@ describe('acceptNativeCallWithReadiness against the real SDK socket', () => { expect(mediaSession.endCall).not.toHaveBeenCalled(); }); - it('fails the call without answering when the reopened socket never acks the re-sub', async () => { - const mediaSession = makeMediaSession(); + it('fails the call without answering when the reopened socket never acks the replayed subscriptions', async () => { + await subscribeMediaStreams(client); const resetNativeCallId = jest.fn(); mockGetState.mockReturnValue({ call: null, resetNativeCallId }); + const mediaSession = makeMediaSession(); - backdateLastPing(driver, PING_INTERVAL * 3); - addMediaSubs(driver, USER_ID); + jest.useFakeTimers(); + mockTransport.withhold({ msg: 'sub' }); + const reopened = mockTransport.awaitConnection(1); + mockTransport.closeTransport(mockTransport.connections[0]); const accept = acceptNativeCallWithReadiness(CALL_ID, mediaSession); - await jest.advanceTimersByTimeAsync(0); - mockConnections[1].onopen(); - - stopAnsweringFrames(mockConnections[1]); - await jest.advanceTimersByTimeAsync(0); - await jest.advanceTimersByTimeAsync(8000); + mockTransport.open(await reopened); + await mockTransport.awaitFrame({ msg: 'sub' }, mockTransport.connections[1]); + await jest.advanceTimersByTimeAsync(READINESS_TIMEOUT); await accept; expect(mockTerminateNativeCall).toHaveBeenCalledWith(CALL_ID); @@ -126,22 +127,22 @@ describe('acceptNativeCallWithReadiness against the real SDK socket', () => { expect(mediaSession.applyRestStateSignals).not.toHaveBeenCalled(); }); - it('answers when the media subs only appear after the reopen', async () => { + it('answers when the media subscriptions are only created after the reopen', async () => { const mediaSession = makeMediaSession(); - backdateLastPing(driver, PING_INTERVAL * 3); + jest.useFakeTimers(); + const reopened = mockTransport.awaitConnection(1); + mockTransport.closeTransport(mockTransport.connections[0]); const accept = acceptNativeCallWithReadiness(CALL_ID, mediaSession); - await jest.advanceTimersByTimeAsync(0); - mockConnections[1].onopen(); - await jest.advanceTimersByTimeAsync(0); - - await jest.advanceTimersByTimeAsync(100); + const connection = await reopened; + mockTransport.open(connection); + await subscribeMediaStreams(client); - addMediaSubs(driver, USER_ID); await jest.advanceTimersByTimeAsync(200); await accept; + expect(mediaSubIdsOn(connection)).toEqual([expect.any(String), expect.any(String)]); expect(mediaSession.applyRestStateSignals).toHaveBeenCalledTimes(1); expect(mediaSession.answerCall).toHaveBeenCalledWith(CALL_ID); expect(mockTerminateNativeCall).not.toHaveBeenCalled(); diff --git a/app/lib/services/voip/acceptNativeCall.test.ts b/app/lib/services/voip/acceptNativeCall.test.ts index 13eb383497..ccdc80edc0 100644 --- a/app/lib/services/voip/acceptNativeCall.test.ts +++ b/app/lib/services/voip/acceptNativeCall.test.ts @@ -4,15 +4,17 @@ import { terminateNativeCall } from './terminateNativeCall'; import { waitForLoginReady } from '../waitForLoginReady'; import { recoverSocket } from '../socketHealth'; import sdk from '../sdk'; -import { buildConnectedDriver } from '../../testUtils/sdkIntegration'; -import type { IMockSdk, IMockSdkDriver, MockConnection } from '../../testUtils/sdkIntegration'; -import type * as SdkIntegration from '../../testUtils/sdkIntegration'; +import { connectAuthenticatedSdk, createTransportFake } from '../../testUtils/sdkTransport'; +import type { RealSdkClient } from '../../testUtils/sdkTransport'; +import type { ISdkModuleFake } from '../../testUtils/sdkModuleFake'; const mockWaitForLoginReady = waitForLoginReady as jest.MockedFunction; const mockRecoverSocket = recoverSocket as jest.MockedFunction; const mockGetState = useCallStore.getState as jest.Mock; const mockTerminateNativeCall = terminateNativeCall as jest.Mock; +const mockTransport = createTransportFake(); + jest.mock('./useCallStore', () => ({ useCallStore: { getState: jest.fn() @@ -24,18 +26,11 @@ jest.mock('./terminateNativeCall', () => ({ })); jest.mock('../sdk', () => { - const sdkIntegration = jest.requireActual('../../testUtils/sdkIntegration'); - return { __esModule: true, default: sdkIntegration.makeSdkMock() }; + const { createSdkModuleFake: create } = jest.requireActual('../../testUtils/sdkModuleFake'); + return { __esModule: true, default: create() }; }); -const mockConnections: MockConnection[] = []; - -jest.mock('universal-websocket-client', () => - jest.fn().mockImplementation(() => { - const sdkIntegration = jest.requireActual('../../testUtils/sdkIntegration'); - return new sdkIntegration.MockConnection(mockConnections); - }) -); +jest.mock('universal-websocket-client', () => jest.fn().mockImplementation(() => mockTransport.createConnection())); jest.mock('../socketHealth', () => ({ recoverSocket: jest.fn() @@ -51,6 +46,8 @@ jest.mock('../../methods/helpers/log', () => ({ default: jest.fn() })); +const sdkModule = sdk as unknown as ISdkModuleFake; + interface IMediaSession { applyRestStateSignals: jest.Mock>; answerCall: jest.Mock, [string]>; @@ -78,27 +75,24 @@ function makeStoreState(overrides: Record = {}) { describe('acceptNativeCallWithReadiness', () => { const CALL_ID = 'call-uuid'; - const USER_ID = 'user-id'; - let driver: IMockSdkDriver; + let client: RealSdkClient; let waitForMediaSubs: jest.SpyInstance, [number?]>; beforeEach(async () => { jest.clearAllMocks(); - jest.useFakeTimers(); - mockConnections.length = 0; - driver = await buildConnectedDriver(mockConnections, USER_ID); - waitForMediaSubs = jest.spyOn(driver, 'waitForNotifyUserMediaSubs').mockResolvedValue(true); - (sdk as unknown as IMockSdk).setClient({ driver }); + mockTransport.reset(); + client = await connectAuthenticatedSdk(mockTransport); + waitForMediaSubs = jest.spyOn(client.driver, 'waitForNotifyUserMediaSubs').mockResolvedValue(true); + sdkModule.setClient(client); mockRecoverSocket.mockResolvedValue('confirmed-alive'); mockWaitForLoginReady.mockResolvedValue(true); mockGetState.mockReturnValue(makeStoreState()); }); - afterEach(() => { - if (driver.socket.pingTimeout) clearTimeout(driver.socket.pingTimeout); - if (driver.socket.openTimeout) clearTimeout(driver.socket.openTimeout); - jest.useRealTimers(); + afterEach(async () => { + await client.disconnect(); + sdkModule.setClient(null); }); it.each(['confirmed-alive', 'reopened'] as const)( @@ -201,7 +195,7 @@ describe('acceptNativeCallWithReadiness', () => { }); it('terminates and ends the call when the SDK socket is unavailable for media subscriptions', async () => { - (sdk as unknown as IMockSdk).setClient({}); + sdkModule.setClient(null); const mediaSession = makeMediaSession(); const resetNativeCallId = jest.fn(); mockGetState.mockReturnValue(makeStoreState({ resetNativeCallId })); @@ -258,6 +252,7 @@ describe('acceptNativeCallWithReadiness', () => { }); it('keeps the newer gate entry when an older gate cleans up, so a third gate aborts the newer one', async () => { + jest.useFakeTimers(); let gateIndex = 0; mockWaitForLoginReady.mockImplementation((_timeoutMs, signal) => { const myIndex = ++gateIndex; @@ -288,5 +283,7 @@ describe('acceptNativeCallWithReadiness', () => { expect(firstSession.endCall).not.toHaveBeenCalled(); expect(secondSession.answerCall).not.toHaveBeenCalled(); expect(thirdSession.answerCall).toHaveBeenCalledWith(CALL_ID); + + jest.useRealTimers(); }); }); diff --git a/app/lib/testUtils/__tests__/observedEffects.test.ts b/app/lib/testUtils/__tests__/observedEffects.test.ts new file mode 100644 index 0000000000..34e960d80d --- /dev/null +++ b/app/lib/testUtils/__tests__/observedEffects.test.ts @@ -0,0 +1,21 @@ +import { waitUntil } from '../observedEffects'; + +describe('waitUntil', () => { + it('resolves once the awaited condition holds', async () => { + let ready = false; + setImmediate(() => { + ready = true; + }); + + await expect(waitUntil(() => ready, { label: 'ready flips', observed: () => ready })).resolves.toBeUndefined(); + }); + + it('rejects instead of returning successfully when the condition is never met', async () => { + const advance = jest.fn(() => Promise.resolve()); + + await expect( + waitUntil(() => false, { label: 'never met', observed: () => ['first-action'], attempts: 3, advance }) + ).rejects.toThrow('[waitUntil] "never met" was still false after 3 scheduler advances. Observed: ["first-action"]'); + expect(advance).toHaveBeenCalledTimes(3); + }); +}); diff --git a/app/lib/testUtils/appMocks.ts b/app/lib/testUtils/appMocks.ts new file mode 100644 index 0000000000..25a78c0b99 --- /dev/null +++ b/app/lib/testUtils/appMocks.ts @@ -0,0 +1,58 @@ +import type { Store } from 'redux'; + +import type { IApplicationState } from '../../definitions'; + +export interface IMockCollection { + name: string; + find: jest.Mock; + query: jest.Mock; + create: jest.Mock; + prepareCreate: jest.Mock; + schema: Record; +} + +export function makeCollection(name: string): IMockCollection { + return { + name, + find: jest.fn(), + query: jest.fn(() => ({ fetch: jest.fn(() => Promise.resolve([])) })), + create: jest.fn(), + prepareCreate: jest.fn(), + schema: {} + }; +} + +export interface IMockReduxState { + meteor: { connected: boolean }; + login: { user: Record | null; isAuthenticated: boolean }; + server: { version: string }; + settings: Record; + room: { subscribedRoom: string | null }; +} + +export interface IMockReduxStore { + state: IMockReduxState; + store: Store & { dispatch: jest.Mock }; +} + +export function makeReduxStore(): IMockReduxStore { + const listeners = new Set<() => void>(); + const state: IMockReduxState = { + meteor: { connected: false }, + login: { user: null, isAuthenticated: false }, + server: { version: '5.0.0' }, + settings: {}, + room: { subscribedRoom: null } + }; + return { + state, + store: { + getState: () => state, + dispatch: jest.fn(), + subscribe: (listener: () => void) => { + listeners.add(listener); + return () => listeners.delete(listener); + } + } as unknown as Store & { dispatch: jest.Mock } + }; +} diff --git a/app/lib/testUtils/observedEffects.ts b/app/lib/testUtils/observedEffects.ts new file mode 100644 index 0000000000..1343d41996 --- /dev/null +++ b/app/lib/testUtils/observedEffects.ts @@ -0,0 +1,106 @@ +import type { AnyAction } from 'redux'; + +interface IActionWaiter { + type: string; + resolve(action: AnyAction): void; +} + +export interface IActionRecorder { + actions: AnyAction[]; + record(action: AnyAction): void; + reset(): void; + types(): string[]; + actionsOfType(type: string): AnyAction[]; + requireAction(type: string): AnyAction; + awaitAction(type: string): Promise; +} + +export function createActionRecorder(): IActionRecorder { + const actions: AnyAction[] = []; + let waiters: IActionWaiter[] = []; + const recorder: IActionRecorder = { + actions, + record(action: AnyAction) { + actions.push(action); + waiters = waiters.filter(waiter => { + if (waiter.type !== action.type) return true; + waiter.resolve(action); + return false; + }); + }, + reset() { + actions.length = 0; + waiters = []; + }, + types() { + return actions.map(action => action.type); + }, + actionsOfType(type: string) { + return actions.filter(action => action.type === type); + }, + requireAction(type: string) { + const found = recorder.actionsOfType(type); + if (!found.length) throw new Error(`[action recorder] no "${type}" action was dispatched`); + return found[found.length - 1]; + }, + awaitAction(type: string) { + const existing = recorder.actionsOfType(type); + if (existing.length) return Promise.resolve(existing[existing.length - 1]); + return new Promise(resolve => waiters.push({ type, resolve })); + } + }; + return recorder; +} + +interface ICallWaiter { + count: number; + resolve(args: unknown[]): void; +} + +export interface ICallTracker { + awaitCall(count?: number): Promise; +} + +export function trackCalls(mock: jest.Mock): ICallTracker { + const implementation = mock.getMockImplementation(); + let waiters: ICallWaiter[] = []; + mock.mockImplementation((...args: unknown[]) => { + const result = implementation?.(...args); + waiters = waiters.filter(waiter => { + if (mock.mock.calls.length < waiter.count) return true; + waiter.resolve(args); + return false; + }); + return result; + }); + return { + awaitCall(count = 1) { + const calls = mock.mock.calls as unknown[][]; + if (calls.length >= count) return Promise.resolve(calls[count - 1]); + return new Promise(resolve => waiters.push({ count, resolve })); + } + }; +} + +export interface IWaitUntilOptions { + label: string; + observed(): unknown; + attempts?: number; + advance?: () => Promise; +} + +const advanceScheduler = (): Promise => new Promise(resolve => setImmediate(resolve)); + +export async function waitUntil( + isMet: () => boolean, + { label, observed, attempts = 50, advance = advanceScheduler }: IWaitUntilOptions +): Promise { + for (let attempt = 0; attempt < attempts; attempt += 1) { + if (isMet()) return; + await advance(); + } + if (isMet()) return; + throw new Error( + `[waitUntil] "${label}" was still false after ${attempts} scheduler advances. Observed: ${JSON.stringify(observed())}` + ); +} diff --git a/app/lib/testUtils/sdkIntegration.ts b/app/lib/testUtils/sdkIntegration.ts deleted file mode 100644 index f56f47d873..0000000000 --- a/app/lib/testUtils/sdkIntegration.ts +++ /dev/null @@ -1,206 +0,0 @@ -import type * as RocketChatSdk from '@rocket.chat/sdk'; -import type { Store } from 'redux'; - -import type { IApplicationState } from '../../definitions'; -import type sdk from '../services/sdk'; -import type { ISocketDriver } from '../services/sdk'; - -export interface IDdpMessage { - msg: string; - id?: string; - name?: string; - method?: string; - params?: unknown[]; -} - -export class MockConnection { - send = jest.fn((frame: string) => { - const message = JSON.parse(frame) as IDdpMessage; - if (message.msg === 'connect') { - setImmediate(() => this.onmessage({ data: JSON.stringify({ msg: 'connected', session: 'session-id' }) })); - } else if (message.msg === 'ping') { - setImmediate(() => this.onmessage({ data: JSON.stringify({ msg: 'pong' }) })); - } else if (message.msg === 'sub') { - setImmediate(() => this.onmessage({ data: JSON.stringify({ msg: 'ready', subs: [message.id] }) })); - } else if (message.msg === 'unsub') { - setImmediate(() => this.onmessage({ data: JSON.stringify({ msg: 'nosub', id: message.id }) })); - } else if (message.msg === 'method' && message.method === 'login') { - setImmediate(() => - this.onmessage({ - data: JSON.stringify({ msg: 'result', id: message.id, result: { id: 'user-id', token: 'auth-token' } }) - }) - ); - } - }); - - close = jest.fn(); - readyState = 1; - onopen = () => {}; - onmessage = (_event: { data: string }) => {}; - onerror = () => {}; - onclose = (_event?: { code?: number }) => {}; - - constructor(registry: MockConnection[]) { - registry.push(this); - } -} - -export interface IMockSdkDriver extends ISocketDriver { - userId: string; - pingInterval: number; - socket: { - lastPing: number; - pingTimeout?: ReturnType; - openTimeout?: ReturnType; - open(): Promise; - send(message: Record): Promise; - subscriptions: Record; - }; -} - -export interface IMockSdkClient { - host?: string; - driver?: ISocketDriver; -} - -export type IMockSdk = Pick & { - setClient(client: IMockSdkClient | null): void; -}; - -export function makeSdkMock = Record>( - members?: TMembers -): IMockSdk & TMembers { - let client: IMockSdkClient | null = null; - const mock: IMockSdk = { - setClient(next: IMockSdkClient | null) { - client = next; - }, - get host() { - return client?.host ?? null; - }, - get driver() { - return client?.driver ?? null; - }, - get isInitialized() { - return client !== null; - } - }; - return Object.assign(mock, members ?? ({} as TMembers)); -} - -export function latestConnection(connections: MockConnection[]): MockConnection { - return connections[connections.length - 1]; -} - -export function framesOn(connection: MockConnection, msg: string): IDdpMessage[] { - return connection.send.mock.calls - .map(([frame]: [string]) => JSON.parse(frame) as IDdpMessage) - .filter(message => message.msg === msg); -} - -export function receiveFrame(connection: MockConnection, frame: Record): void { - connection.onmessage({ data: JSON.stringify(frame) }); -} - -const { Rocketchat } = jest.requireActual('@rocket.chat/sdk'); - -const driverLogger = { debug: jest.fn(), info: jest.fn(), error: jest.fn(), warn: jest.fn() }; - -export async function buildConnectedDriver(connections: MockConnection[], userId: string): Promise { - const driver = new Rocketchat({ host: 'localhost:3000', logger: driverLogger }).driver as unknown as IMockSdkDriver; - driver.userId = userId; - const openPromise = driver.socket.open(); - connections[0].onopen(); - await jest.advanceTimersByTimeAsync(0); - await openPromise; - return driver; -} - -export function addMediaSubs(driver: IMockSdkDriver, userId: string): void { - ['media-signal', 'media-calls'].forEach((name, index) => { - const id = `sub-${index}`; - driver.socket.subscriptions[id] = { - id, - name: 'stream-notify-user', - params: [`${userId}/${name}`], - unsubscribe: jest.fn() - }; - }); -} - -export function backdateLastPing(driver: IMockSdkDriver, ageMs: number): void { - driver.socket.lastPing = Date.now() - ageMs; -} - -export function stopAnsweringFrames(connection: MockConnection): void { - connection.send.mockImplementation(() => undefined); -} - -export interface IMockCollection { - name: string; - find: jest.Mock; - query: jest.Mock; - create: jest.Mock; - prepareCreate: jest.Mock; - schema: Record; -} - -export function makeCollection(name: string): IMockCollection { - return { - name, - find: jest.fn(), - query: jest.fn(() => ({ fetch: jest.fn(() => Promise.resolve([])) })), - create: jest.fn(), - prepareCreate: jest.fn(), - schema: {} - }; -} - -export async function flush(turns = 10): Promise { - for (let i = 0; i < turns; i++) { - await Promise.resolve(); - await jest.advanceTimersByTimeAsync(0); - } -} - -export async function settleUntil(isSettled: () => boolean, maxRounds = 20): Promise { - for (let round = 0; round < maxRounds && !isSettled(); round++) { - await jest.runOnlyPendingTimersAsync(); - await flush(); - } -} - -export interface IMockReduxState { - meteor: { connected: boolean }; - login: { user: Record | null; isAuthenticated: boolean }; - server: { version: string }; - settings: Record; - room: { subscribedRoom: string | null }; -} - -export interface IMockReduxStore { - state: IMockReduxState; - store: Store & { dispatch: jest.Mock }; -} - -export function makeReduxStore(): IMockReduxStore { - const listeners = new Set<() => void>(); - const state: IMockReduxState = { - meteor: { connected: false }, - login: { user: null, isAuthenticated: false }, - server: { version: '5.0.0' }, - settings: {}, - room: { subscribedRoom: null } - }; - return { - state, - store: { - getState: () => state, - dispatch: jest.fn(), - subscribe: (listener: () => void) => { - listeners.add(listener); - return () => listeners.delete(listener); - } - } as unknown as Store & { dispatch: jest.Mock } - }; -} diff --git a/app/lib/testUtils/sdkModuleFake.ts b/app/lib/testUtils/sdkModuleFake.ts new file mode 100644 index 0000000000..8cdba2386c --- /dev/null +++ b/app/lib/testUtils/sdkModuleFake.ts @@ -0,0 +1,40 @@ +import type sdk from '../services/sdk'; +import type { ISocketDriver } from '../services/sdk'; +import type { RealSdkClient } from './sdkTransport'; + +export interface ISdkClientStub { + host?: string; + driver?: ISocketDriver; +} + +export type SdkClientLike = ISdkClientStub | RealSdkClient; + +export type ISdkModuleFake = Pick & { + setClient(client: SdkClientLike | null): void; +}; + +function hostOf(client: SdkClientLike): string | null { + if ('client' in client) return client.client.host ?? null; + return client.host ?? null; +} + +export function createSdkModuleFake = Record>( + members?: TMembers +): ISdkModuleFake & TMembers { + let current: SdkClientLike | null = null; + const fake: ISdkModuleFake = { + setClient(client: SdkClientLike | null) { + current = client; + }, + get host() { + return current ? hostOf(current) : null; + }, + get driver() { + return (current?.driver as unknown as ISocketDriver) ?? null; + }, + get isInitialized() { + return current !== null; + } + }; + return Object.assign(fake, members ?? ({} as TMembers)); +} diff --git a/app/lib/testUtils/sdkTransport.ts b/app/lib/testUtils/sdkTransport.ts new file mode 100644 index 0000000000..0e081ba979 --- /dev/null +++ b/app/lib/testUtils/sdkTransport.ts @@ -0,0 +1,191 @@ +import type * as RocketChatSdk from '@rocket.chat/sdk'; + +export interface IDdpFrame { + msg: string; + id?: string; + name?: string; + method?: string; + params?: unknown[]; +} + +export interface IFrameMatcher { + msg?: string; + method?: string; + name?: string; + id?: string; +} + +const connectingState = 0; +const openState = 1; +const closedState = 3; + +function matchesFrame(frame: IDdpFrame, matcher: IFrameMatcher): boolean { + return Object.entries(matcher).every(([key, value]) => frame[key as keyof IDdpFrame] === value); +} + +function describeMatcher(matcher: IFrameMatcher): string { + return JSON.stringify(matcher); +} + +export class FakeConnection { + readyState = connectingState; + frames: IDdpFrame[] = []; + onopen: ((event?: unknown) => void) | null = null; + onmessage: ((event: { data: string }) => void) | null = null; + onerror: ((event?: unknown) => void) | null = null; + onclose: ((event?: { code?: number; reason?: string }) => void) | null = null; + + constructor(private readonly transport: TransportFake) {} + + send(data: string): void { + const frame = JSON.parse(data) as IDdpFrame; + this.frames.push(frame); + this.transport.recordFrame(this, frame); + } + + close(code?: number): void { + if (this.readyState === closedState) return; + this.readyState = closedState; + this.onclose?.({ code }); + } +} + +interface IFrameWaiter { + matcher: IFrameMatcher; + connection?: FakeConnection; + resolve(frame: IDdpFrame): void; +} + +interface IConnectionWaiter { + index: number; + resolve(connection: FakeConnection): void; +} + +export class TransportFake { + connections: FakeConnection[] = []; + loginResult: { id: string; token: string } = { id: 'user-id', token: 'auth-token' }; + private withheld: IFrameMatcher[] = []; + private frameWaiters: IFrameWaiter[] = []; + private connectionWaiters: IConnectionWaiter[] = []; + + createConnection = (): FakeConnection => { + const connection = new FakeConnection(this); + this.connections.push(connection); + const index = this.connections.length - 1; + this.connectionWaiters = this.connectionWaiters.filter(waiter => { + if (waiter.index !== index) return true; + waiter.resolve(connection); + return false; + }); + return connection; + }; + + reset(): void { + this.connections = []; + this.withheld = []; + this.frameWaiters = []; + this.connectionWaiters = []; + this.loginResult = { id: 'user-id', token: 'auth-token' }; + } + + get latestConnection(): FakeConnection { + return this.connections[this.connections.length - 1]; + } + + awaitConnection(index = 0): Promise { + const existing = this.connections[index]; + if (existing) return Promise.resolve(existing); + return new Promise(resolve => this.connectionWaiters.push({ index, resolve })); + } + + awaitFrame(matcher: IFrameMatcher, connection?: FakeConnection): Promise { + const existing = this.frames(matcher, connection); + if (existing.length) return Promise.resolve(existing[existing.length - 1]); + return new Promise(resolve => this.frameWaiters.push({ matcher, connection, resolve })); + } + + frames(matcher: IFrameMatcher = {}, connection?: FakeConnection): IDdpFrame[] { + const sources = connection ? [connection] : this.connections; + return sources.flatMap(source => source.frames).filter(frame => matchesFrame(frame, matcher)); + } + + requireFrame(matcher: IFrameMatcher, connection?: FakeConnection): IDdpFrame { + const found = this.frames(matcher, connection); + if (!found.length) throw new Error(`[transport fake] no frame matching ${describeMatcher(matcher)} was sent`); + return found[found.length - 1]; + } + + open(connection: FakeConnection = this.latestConnection): void { + connection.readyState = openState; + connection.onopen?.(); + } + + closeTransport(connection: FakeConnection = this.latestConnection, code = 1006): void { + connection.readyState = closedState; + connection.onclose?.({ code }); + } + + deliver(frame: Record, connection: FakeConnection = this.latestConnection): void { + connection.onmessage?.({ data: JSON.stringify(frame) }); + } + + respond(request: IDdpFrame, result: unknown, connection: FakeConnection = this.latestConnection): void { + this.deliver({ msg: 'result', id: request.id, result }, connection); + } + + withhold(matcher: IFrameMatcher): void { + this.withheld.push(matcher); + } + + recordFrame(connection: FakeConnection, frame: IDdpFrame): void { + this.frameWaiters = this.frameWaiters.filter(waiter => { + if (waiter.connection && waiter.connection !== connection) return true; + if (!matchesFrame(frame, waiter.matcher)) return true; + waiter.resolve(frame); + return false; + }); + if (this.withheld.some(matcher => matchesFrame(frame, matcher))) return; + const response = this.automaticResponse(frame); + if (response) Promise.resolve().then(() => this.deliver(response, connection)); + } + + private automaticResponse(frame: IDdpFrame): Record | undefined { + if (frame.msg === 'connect') return { msg: 'connected', session: 'session-id' }; + if (frame.msg === 'ping') return { msg: 'pong' }; + if (frame.msg === 'sub') return { msg: 'ready', subs: [frame.id] }; + if (frame.msg === 'unsub') return { msg: 'nosub', id: frame.id }; + if (frame.msg === 'method' && frame.method === 'login') { + return { msg: 'result', id: frame.id, result: this.loginResult }; + } + return undefined; + } +} + +export function createTransportFake(): TransportFake { + return new TransportFake(); +} + +export const silentLogger = { debug: jest.fn(), info: jest.fn(), error: jest.fn(), warn: jest.fn() }; + +export type RealSdkClient = InstanceType; + +export function createSdkClient(host = 'localhost:3000'): RealSdkClient { + const { Rocketchat } = jest.requireActual('@rocket.chat/sdk'); + return new Rocketchat({ host, logger: silentLogger }); +} + +export async function connectAuthenticatedSdk( + transport: TransportFake, + { host, token }: { host?: string; token?: string } = {} +): Promise { + const client = createSdkClient(host); + const connecting = client.connect(); + transport.open(await transport.awaitConnection()); + await connecting; + await client.resume({ token: token ?? 'resume-token' }); + return client; +} + +export async function subscribeMediaStreams(client: RealSdkClient): Promise { + await client.subscribeNotifyUser(); +} diff --git a/app/sagas/__tests__/foregroundResume.integration.test.ts b/app/sagas/__tests__/foregroundResume.integration.test.ts index 307a06838d..b5633dfb93 100644 --- a/app/sagas/__tests__/foregroundResume.integration.test.ts +++ b/app/sagas/__tests__/foregroundResume.integration.test.ts @@ -3,20 +3,17 @@ jest.unmock('@rocket.chat/sdk'); import { applyMiddleware, createStore, type AnyAction, type Store } from 'redux'; import createSagaMiddleware from 'redux-saga'; -import type * as SdkIntegration from '../../lib/testUtils/sdkIntegration'; -import type { MockConnection } from '../../lib/testUtils/sdkIntegration'; +import { createActionRecorder, trackCalls } from '../../lib/testUtils/observedEffects'; +import { createTransportFake } from '../../lib/testUtils/sdkTransport'; +import type { FakeConnection } from '../../lib/testUtils/sdkTransport'; const USER_ID = 'user-id'; const RESUME_TOKEN = 'auth-token'; const CLOSED = 3; -const mockConnections: MockConnection[] = []; -jest.mock('universal-websocket-client', () => - jest.fn().mockImplementation(() => { - const sdkIntegration = jest.requireActual('../../lib/testUtils/sdkIntegration'); - return new sdkIntegration.MockConnection(mockConnections); - }) -); +const mockTransport = createTransportFake(); + +jest.mock('universal-websocket-client', () => jest.fn().mockImplementation(() => mockTransport.createConnection())); jest.mock('../../lib/methods/helpers/localAuthentication', () => ({ localAuthenticate: jest.fn(), @@ -52,8 +49,8 @@ jest.mock('../../lib/methods/subscribeRooms', () => ({ unsubscribeRooms: jest.fn() })); -jest.mock('../../lib/methods/loadMissedMessages', () => ({ - loadMissedMessages: jest.fn(() => Promise.resolve()) +jest.mock('../../lib/methods/syncRoom', () => ({ + syncRoom: jest.fn(() => Promise.resolve()) })); jest.mock('../../lib/methods/readMessages', () => ({ @@ -97,7 +94,8 @@ import RoomSubscription from '../../lib/methods/subscriptions/room'; import databaseModule from '../../lib/database'; import { connect } from '../../lib/services/connect'; import sdk from '../../lib/services/sdk'; -import { loadMissedMessages } from '../../lib/methods/loadMissedMessages'; +import { syncRoom } from '../../lib/methods/syncRoom'; +import { makeCollection } from '../../lib/testUtils/appMocks'; import { initStore } from '../../lib/store/auxStore'; import { APP_STATE } from '../../actions/actionsTypes'; import { appStart } from '../../actions/app'; @@ -108,20 +106,12 @@ import { RootEnum } from '../../definitions'; import reducers from '../../reducers'; import loginRoot from '../login'; import stateRoot from '../state'; -import { - flush, - framesOn, - latestConnection, - makeCollection, - settleUntil, - stopAnsweringFrames -} from '../../lib/testUtils/sdkIntegration'; import { saveLastLocalAuthenticationSession } from '../../lib/methods/helpers/localAuthentication'; -import { setUserPresenceAway } from '../../lib/services/restApi'; +import { setUserPresenceAway, setUserPresenceOnline } from '../../lib/services/restApi'; const SERVER = 'https://open.rocket.chat'; const ROOM_ID = 'room-rid'; -const RECOVERY_WINDOW = 5000; +const ROUND_TRIP_BUDGET = 2000; const database = databaseModule as unknown as { active: { get: jest.Mock; write: jest.Mock; batch: jest.Mock }; @@ -135,31 +125,30 @@ const ROOM_TOPICS = [ `stream-notify-room:${ROOM_ID}/messagesRead` ]; -function typeOf(action: AnyAction): string { - return action.type; -} +const recorder = createActionRecorder(); -function topicsOn(connection: MockConnection): string[] { - return framesOn(connection, 'sub').map(frame => `${frame.name}:${frame.params?.[0]}`); -} +let store: Store; +let collections: Record>; -function roomTopicsOn(connection: MockConnection): string[] { - return topicsOn(connection).filter(topic => topic.includes(ROOM_ID)); +function roomTopicsOn(connection: FakeConnection): string[] { + return mockTransport + .frames({ msg: 'sub' }, connection) + .map(frame => `${frame.name}:${frame.params?.[0]}`) + .filter(topic => topic.includes(ROOM_ID)); } -let dispatched: AnyAction[]; -let store: Store; -let collections: Record>; +function pingsOn(connection: FakeConnection): number { + return mockTransport.frames({ msg: 'ping' }, connection).length; +} function recordDispatched() { return () => (next: (action: AnyAction) => AnyAction) => (action: AnyAction) => { - dispatched.push(action); + recorder.record(action); return next(action); }; } function bootApp(): void { - dispatched = []; const sagaMiddleware = createSagaMiddleware(); store = createStore(reducers, applyMiddleware(recordDispatched(), sagaMiddleware)); sagaMiddleware.run(stateRoot); @@ -169,39 +158,39 @@ function bootApp(): void { store.dispatch(selectServerSuccess({ server: SERVER, name: 'open.rocket.chat', version: '6.0.0' })); } -async function openSocket(): Promise { - await connect({ server: SERVER }); - await flush(); - mockConnections[0].onopen(); - await flush(); - store.dispatch(connectSuccess()); - await flush(); +async function openSocket(): Promise { + const index = mockTransport.connections.length; + const connecting = connect({ server: SERVER }); + const connection = await mockTransport.awaitConnection(index); + mockTransport.open(connection); + await connecting; + await recorder.awaitAction(connectSuccess().type); + return connection; } -async function openSignedInSocket(): Promise { - await openSocket(); +async function openSignedInSocket(): Promise { + const connection = await openSocket(); store.dispatch(loginSuccess({ id: USER_ID, token: RESUME_TOKEN } as never)); - await flush(); + await recorder.awaitAction(loginSuccess({} as never).type); + return connection; } function resumedUser(): unknown { - const resumed = dispatched.find(action => typeOf(action) === typeOf(loginSuccess({} as never))); + const resumed = recorder.actionsOfType(loginSuccess({} as never).type).pop(); return resumed?.user; } async function subscribeToRoom(rid: string): Promise { const room = new RoomSubscription(rid); - const subscribing = room.subscribe(); - await flush(); - await subscribing; - await flush(); + await room.subscribe(); return room; } beforeEach(() => { jest.clearAllMocks(); jest.useFakeTimers(); - mockConnections.length = 0; + mockTransport.reset(); + recorder.reset(); collections = {}; database.active.get.mockReset().mockImplementation((name: string) => (collections[name] ??= makeCollection(name))); database.active.write.mockReset().mockImplementation((fn: () => unknown) => fn()); @@ -218,143 +207,127 @@ beforeEach(() => { ) as unknown as typeof fetch; }); -afterEach(async () => { +afterEach(() => { sdk.disconnect(); - await flush(); jest.useRealTimers(); }); describe('foreground resume over the real SDK socket', () => { it('gets messages flowing again when the socket died silently while away', async () => { bootApp(); - await openSignedInSocket(); + const frozen = await openSignedInSocket(); await subscribeToRoom(ROOM_ID); - const frozen = mockConnections[0]; expect(roomTopicsOn(frozen)).toEqual(expect.arrayContaining(ROOM_TOPICS)); - stopAnsweringFrames(frozen); - const pingsBefore = framesOn(frozen, 'ping').length; - dispatched.length = 0; - jest.mocked(loadMissedMessages).mockClear(); + mockTransport.withhold({ msg: 'ping' }); + const pingsBefore = pingsOn(frozen); + recorder.reset(); + const syncs = trackCalls(jest.mocked(syncRoom)); store.dispatch({ type: APP_STATE.FOREGROUND }); - await flush(); - await jest.advanceTimersByTimeAsync(RECOVERY_WINDOW); + await mockTransport.awaitFrame({ msg: 'ping' }, frozen); - expect(framesOn(frozen, 'ping').length).toBeGreaterThan(pingsBefore); - expect(mockConnections).toHaveLength(2); - const reopened = latestConnection(mockConnections); + expect(pingsOn(frozen)).toBeGreaterThan(pingsBefore); + expect(syncRoom).not.toHaveBeenCalled(); - expect(loadMissedMessages).not.toHaveBeenCalled(); + await jest.advanceTimersByTimeAsync(ROUND_TRIP_BUDGET); + const reopened = await mockTransport.awaitConnection(1); + mockTransport.open(reopened); - reopened.onopen(); - await settleUntil(() => resumedUser() !== undefined); + await recorder.awaitAction(loginSuccess({} as never).type); + await syncs.awaitCall(); - expect(dispatched).toContainEqual(connectSuccess()); - expect(dispatched).toContainEqual(loginRequest({ resume: RESUME_TOKEN }, false)); - expect(loadMissedMessages).toHaveBeenCalledWith({ rid: ROOM_ID }); + expect(recorder.actions).toContainEqual(connectSuccess()); + expect(recorder.actions).toContainEqual(loginRequest({ resume: RESUME_TOKEN }, false)); + expect(syncRoom).toHaveBeenCalledWith({ rid: ROOM_ID }); expect(roomTopicsOn(reopened)).toEqual(expect.arrayContaining(ROOM_TOPICS)); expect(resumedUser()).toEqual(expect.objectContaining({ id: USER_ID, token: RESUME_TOKEN, username: 'the-user' })); }); it('lands on a reconnected, still-signed-in app instead of forcing a relaunch after the network dropped while away', async () => { bootApp(); - await openSignedInSocket(); - const dropped = mockConnections[0]; + const dropped = await openSignedInSocket(); - dropped.readyState = CLOSED; - dropped.onclose({ code: 1006 }); - await flush(); - expect(dispatched).toContainEqual(disconnect()); - dispatched.length = 0; + mockTransport.closeTransport(dropped); + await recorder.awaitAction(disconnect().type); + recorder.reset(); - await jest.advanceTimersByTimeAsync(RECOVERY_WINDOW); - expect(mockConnections).toHaveLength(1); + expect(mockTransport.connections).toHaveLength(1); store.dispatch({ type: APP_STATE.FOREGROUND }); - await flush(); - await jest.advanceTimersByTimeAsync(RECOVERY_WINDOW); + const reopened = await mockTransport.awaitConnection(1); + mockTransport.open(reopened); - expect(mockConnections.length).toBeGreaterThan(1); - const reopened = latestConnection(mockConnections); + await recorder.awaitAction(loginSuccess({} as never).type); - reopened.onopen(); - await settleUntil(() => resumedUser() !== undefined); - - const connectSuccessAt = dispatched.findIndex(action => typeOf(action) === typeOf(connectSuccess())); - const loginRequestAt = dispatched.findIndex( - action => typeOf(action) === typeOf(loginRequest({ resume: RESUME_TOKEN }, false)) - ); + const connectSuccessAt = recorder.types().indexOf(connectSuccess().type); + const loginRequestAt = recorder.types().indexOf(loginRequest({ resume: RESUME_TOKEN }, false).type); expect(connectSuccessAt).toBeGreaterThanOrEqual(0); expect(loginRequestAt).toBeGreaterThan(connectSuccessAt); - expect(dispatched[loginRequestAt]).toEqual(loginRequest({ resume: RESUME_TOKEN }, false)); + expect(recorder.actions[loginRequestAt]).toEqual(loginRequest({ resume: RESUME_TOKEN }, false)); expect(resumedUser()).toEqual(expect.objectContaining({ id: USER_ID, token: RESUME_TOKEN, username: 'the-user' })); - expect(framesOn(reopened, 'connect').length).toBeGreaterThan(0); + expect(dropped.readyState).toBe(CLOSED); + expect(mockTransport.frames({ msg: 'connect' }, reopened).length).toBeGreaterThan(0); }); it('keeps the live connection instead of paying for an avoidable reconnect when switching straight back', async () => { bootApp(); - await openSignedInSocket(); + const alive = await openSignedInSocket(); await subscribeToRoom(ROOM_ID); - const alive = mockConnections[0]; - const pingsBefore = framesOn(alive, 'ping').length; - const connectFramesBefore = framesOn(alive, 'connect').length; - const connectionsBefore = mockConnections.length; - dispatched.length = 0; + const pingsBefore = pingsOn(alive); + const connectFramesBefore = mockTransport.frames({ msg: 'connect' }, alive).length; + recorder.reset(); + const presence = trackCalls(jest.mocked(setUserPresenceOnline)); store.dispatch({ type: APP_STATE.FOREGROUND }); - await flush(); - await jest.advanceTimersByTimeAsync(RECOVERY_WINDOW); - - expect(framesOn(alive, 'ping').length).toBeGreaterThan(pingsBefore); - expect(mockConnections).toHaveLength(connectionsBefore); - expect(framesOn(alive, 'connect')).toHaveLength(connectFramesBefore); - expect(dispatched.map(typeOf)).not.toContain(typeOf(connectSuccess())); - expect(dispatched.map(typeOf)).not.toContain(typeOf(loginRequest({ resume: RESUME_TOKEN }, false))); + await presence.awaitCall(); + + expect(pingsOn(alive)).toBeGreaterThan(pingsBefore); + expect(mockTransport.connections).toHaveLength(1); + expect(mockTransport.frames({ msg: 'connect' }, alive)).toHaveLength(connectFramesBefore); + expect(recorder.types()).not.toContain(connectSuccess().type); + expect(recorder.types()).not.toContain(loginRequest({ resume: RESUME_TOKEN }, false).type); }); it('leaves the socket alone when the app returns to the foreground before anyone is signed in', async () => { bootApp(); - await openSocket(); - const frozen = mockConnections[0]; - stopAnsweringFrames(frozen); - dispatched.length = 0; + const frozen = await openSocket(); + mockTransport.withhold({ msg: 'ping' }); + recorder.reset(); store.dispatch({ type: APP_STATE.FOREGROUND }); - await flush(); - await jest.advanceTimersByTimeAsync(RECOVERY_WINDOW); + await jest.advanceTimersByTimeAsync(ROUND_TRIP_BUDGET); - expect(framesOn(frozen, 'ping')).toHaveLength(0); - expect(mockConnections).toHaveLength(1); - expect(dispatched.map(typeOf)).not.toContain(typeOf(loginRequest({ resume: RESUME_TOKEN }, false))); + expect(pingsOn(frozen)).toBe(0); + expect(mockTransport.connections).toHaveLength(1); + expect(recorder.types()).not.toContain(loginRequest({ resume: RESUME_TOKEN }, false).type); }); it('leaves the socket alone when the app returns to the foreground on the Outside Stack', async () => { bootApp(); - await openSignedInSocket(); - const frozen = mockConnections[0]; - stopAnsweringFrames(frozen); + const frozen = await openSignedInSocket(); + mockTransport.withhold({ msg: 'ping' }); store.dispatch(appStart({ root: RootEnum.ROOT_OUTSIDE })); - await flush(); - const pingsBefore = framesOn(frozen, 'ping').length; - dispatched.length = 0; + await recorder.awaitAction(appStart({ root: RootEnum.ROOT_OUTSIDE }).type); + const pingsBefore = pingsOn(frozen); + recorder.reset(); store.dispatch({ type: APP_STATE.FOREGROUND }); - await flush(); - await jest.advanceTimersByTimeAsync(RECOVERY_WINDOW); + await jest.advanceTimersByTimeAsync(ROUND_TRIP_BUDGET); - expect(framesOn(frozen, 'ping')).toHaveLength(pingsBefore); - expect(mockConnections).toHaveLength(1); - expect(dispatched.map(typeOf)).not.toContain(typeOf(loginRequest({ resume: RESUME_TOKEN }, false))); + expect(pingsOn(frozen)).toBe(pingsBefore); + expect(mockTransport.connections).toHaveLength(1); + expect(recorder.types()).not.toContain(loginRequest({ resume: RESUME_TOKEN }, false).type); }); it('saves the local authentication session and goes away when the app leaves for the background', async () => { bootApp(); await openSignedInSocket(); + const savedSessions = trackCalls(jest.mocked(saveLastLocalAuthenticationSession)); store.dispatch({ type: APP_STATE.BACKGROUND }); - await flush(); + await savedSessions.awaitCall(); expect(saveLastLocalAuthenticationSession).toHaveBeenCalledWith(SERVER); expect(setUserPresenceAway).toHaveBeenCalled(); @@ -365,7 +338,7 @@ describe('foreground resume over the real SDK socket', () => { await openSocket(); store.dispatch({ type: APP_STATE.BACKGROUND }); - await flush(); + await jest.advanceTimersByTimeAsync(0); expect(saveLastLocalAuthenticationSession).not.toHaveBeenCalled(); expect(setUserPresenceAway).not.toHaveBeenCalled(); @@ -375,10 +348,10 @@ describe('foreground resume over the real SDK socket', () => { bootApp(); await openSignedInSocket(); store.dispatch(disconnect()); - await flush(); + await recorder.awaitAction(disconnect().type); store.dispatch({ type: APP_STATE.BACKGROUND }); - await flush(); + await jest.advanceTimersByTimeAsync(0); expect(saveLastLocalAuthenticationSession).not.toHaveBeenCalled(); expect(setUserPresenceAway).not.toHaveBeenCalled(); @@ -388,10 +361,10 @@ describe('foreground resume over the real SDK socket', () => { bootApp(); await openSignedInSocket(); store.dispatch(appStart({ root: RootEnum.ROOT_OUTSIDE })); - await flush(); + await recorder.awaitAction(appStart({ root: RootEnum.ROOT_OUTSIDE }).type); store.dispatch({ type: APP_STATE.BACKGROUND }); - await flush(); + await jest.advanceTimersByTimeAsync(0); expect(saveLastLocalAuthenticationSession).not.toHaveBeenCalled(); expect(setUserPresenceAway).not.toHaveBeenCalled(); diff --git a/app/sagas/__tests__/selectServer.sdkHost.test.ts b/app/sagas/__tests__/selectServer.sdkHost.test.ts index f45a410538..fe24e345f5 100644 --- a/app/sagas/__tests__/selectServer.sdkHost.test.ts +++ b/app/sagas/__tests__/selectServer.sdkHost.test.ts @@ -1,13 +1,8 @@ jest.unmock('@rocket.chat/sdk'); -const mockConnections: MockConnection[] = []; +const mockTransport = createTransportFake(); -jest.mock('universal-websocket-client', () => - jest.fn().mockImplementation(() => { - const sdkIntegration = jest.requireActual('../../lib/testUtils/sdkIntegration'); - return new sdkIntegration.MockConnection(mockConnections); - }) -); +jest.mock('universal-websocket-client', () => jest.fn().mockImplementation(() => mockTransport.createConnection())); jest.mock('../../lib/methods/helpers/sslPinning', () => ({ __esModule: true, @@ -38,15 +33,15 @@ import { APP, SERVER } from '../../actions/actionsTypes'; import { RootEnum } from '../../definitions'; import sdk from '../../lib/services/sdk'; import { connect } from '../../lib/services/connect'; -import type { MockConnection } from '../../lib/testUtils/sdkIntegration'; -import type * as SdkIntegration from '../../lib/testUtils/sdkIntegration'; -import { cancelSagaTasks, createRecordingStore, flushSagaMicrotasks } from '../../lib/testUtils/sagaStore'; +import { cancelSagaTasks, createRecordingStore } from '../../lib/testUtils/sagaStore'; +import { waitUntil } from '../../lib/testUtils/observedEffects'; +import { createTransportFake } from '../../lib/testUtils/sdkTransport'; const HOST = 'https://open.rocket.chat'; describe('selectServer saga — redundant select for the live SDK host', () => { beforeEach(() => { - mockConnections.length = 0; + mockTransport.reset(); }); afterEach(() => { @@ -61,7 +56,10 @@ describe('selectServer saga — redundant select for the live SDK host', () => { const { store, dispatchedActions } = createRecordingStore(selectServerRoot); store.dispatch(selectServerRequest(HOST, '7.0.0', false)); - await flushSagaMicrotasks(); + await waitUntil(() => dispatchedActions.some(action => action.type === SERVER.SELECT_CANCEL), { + label: 'selectServer cancels the redundant select', + observed: () => dispatchedActions.map(action => action.type) + }); const insideIndex = dispatchedActions.findIndex(action => action.type === APP.START && action.root === RootEnum.ROOT_INSIDE); const cancelIndex = dispatchedActions.findIndex(action => action.type === SERVER.SELECT_CANCEL);