From 40a4b0d536ed28a1ed052b740adf408b1bbddb56 Mon Sep 17 00:00:00 2001 From: Diego Mello Date: Tue, 24 Mar 2026 12:01:23 -0300 Subject: [PATCH] feat: Refactor media session handling and improve disconnect logic --- app/lib/methods/logout.ts | 3 +- app/lib/services/connect.ios.test.ts | 4 + app/lib/services/connect.test.ts | 4 + app/lib/services/connect.ts | 5 +- .../voip/MediaSessionInstance.test.ts | 159 ++++++++++++++++++ app/lib/services/voip/MediaSessionInstance.ts | 26 +-- app/lib/services/voip/MediaSessionStore.ts | 10 ++ app/sagas/login.js | 8 +- app/sagas/selectServer.ts | 2 + .../NewServerView/hooks/useConnectServer.tsx | 4 +- 10 files changed, 208 insertions(+), 17 deletions(-) create mode 100644 app/lib/services/voip/MediaSessionInstance.test.ts diff --git a/app/lib/methods/logout.ts b/app/lib/methods/logout.ts index 69a9643ec2d..d27819472d4 100644 --- a/app/lib/methods/logout.ts +++ b/app/lib/methods/logout.ts @@ -6,6 +6,7 @@ import { isSsl } from './helpers'; import { BASIC_AUTH_KEY } from './helpers/fetch'; import database, { getDatabase } from '../database'; import log from './helpers/log'; +import { disconnect } from '../services/connect'; import sdk from '../services/sdk'; import { CURRENT_SERVER, E2E_PRIVATE_KEY, E2E_PUBLIC_KEY, E2E_RANDOM_PASSWORD_KEY, TOKEN_KEY } from '../constants/keys'; import UserPreferences from './userPreferences'; @@ -111,7 +112,7 @@ export async function logout({ server }: { server: string }): Promise { } if (sdk.current) { - sdk.disconnect(); + disconnect(); } await removeServerData({ server }); diff --git a/app/lib/services/connect.ios.test.ts b/app/lib/services/connect.ios.test.ts index 84e3a5f1e87..1db1ec22ed9 100644 --- a/app/lib/services/connect.ios.test.ts +++ b/app/lib/services/connect.ios.test.ts @@ -1,5 +1,9 @@ import { determineAuthType } from './connect'; +jest.mock('./voip/MediaSessionInstance', () => ({ + mediaSessionInstance: { reset: jest.fn(), init: jest.fn() } +})); + // Mock the isIOS helper to return true for iOS-specific tests jest.mock('../methods/helpers', () => ({ ...jest.requireActual('../methods/helpers'), diff --git a/app/lib/services/connect.test.ts b/app/lib/services/connect.test.ts index 8b1ed79e2bd..34e1ceea898 100644 --- a/app/lib/services/connect.test.ts +++ b/app/lib/services/connect.test.ts @@ -1,5 +1,9 @@ import { determineAuthType } from './connect'; +jest.mock('./voip/MediaSessionInstance', () => ({ + mediaSessionInstance: { reset: jest.fn(), init: jest.fn() } +})); + // Mock the isIOS helper jest.mock('../methods/helpers/deviceInfo', () => ({ ...jest.requireActual('../methods/helpers/deviceInfo'), diff --git a/app/lib/services/connect.ts b/app/lib/services/connect.ts index 2c292873522..095fb24338c 100644 --- a/app/lib/services/connect.ts +++ b/app/lib/services/connect.ts @@ -11,6 +11,7 @@ import { twoFactor } from './twoFactor'; import { store } from '../store/auxStore'; import { loginRequest, logout, setLoginServices, setUser } from '../../actions/login'; import sdk from './sdk'; +import { mediaSessionInstance } from './voip/MediaSessionInstance'; import I18n from '../../i18n'; import { type ICredentials, type ILoggedUser, STATUSES } from '../../definitions'; import { connectRequest, connectSuccess, disconnect as disconnectAction } from '../../actions/connect'; @@ -407,7 +408,9 @@ function checkAndReopen() { } function disconnect() { - return sdk.disconnect(); + const result = sdk.disconnect(); + mediaSessionInstance.reset(); + return result; } async function getWebsocketInfo({ diff --git a/app/lib/services/voip/MediaSessionInstance.test.ts b/app/lib/services/voip/MediaSessionInstance.test.ts new file mode 100644 index 00000000000..640d499b427 --- /dev/null +++ b/app/lib/services/voip/MediaSessionInstance.test.ts @@ -0,0 +1,159 @@ +import { mediaSessionStore } from './MediaSessionStore'; +import { mediaSessionInstance } from './MediaSessionInstance'; + +const mockCallStoreReset = jest.fn(); + +jest.mock('./useCallStore', () => ({ + useCallStore: { + getState: jest.fn(() => ({ + reset: mockCallStoreReset, + setCall: jest.fn(), + callId: null as string | null + })) + } +})); + +const mockOnStreamDataStop = jest.fn(); +const mockOnStreamData = jest.fn(() => ({ stop: mockOnStreamDataStop })); +const mockMethodCall = jest.fn(); + +jest.mock('../sdk', () => ({ + __esModule: true, + default: { + onStreamData: (...args: Parameters) => mockOnStreamData(...args), + methodCall: (...args: unknown[]) => mockMethodCall(...args) + } +})); + +jest.mock('../../store/auxStore', () => ({ + store: { + getState: jest.fn(() => ({ + settings: { + VoIP_TeamCollab_Ice_Servers: '', + VoIP_TeamCollab_Ice_Gathering_Timeout: 5000 + } + })), + subscribe: jest.fn(() => jest.fn()) + } +})); + +jest.mock('react-native-webrtc', () => ({ + registerGlobals: jest.fn(), + mediaDevices: { getUserMedia: jest.fn() } +})); + +jest.mock('react-native-callkeep', () => ({})); + +jest.mock('react-native-device-info', () => ({ + getUniqueId: jest.fn(() => 'test-device-id') +})); + +jest.mock('../../native/NativeVoip', () => ({ + __esModule: true, + default: { stopNativeDDPClient: jest.fn() } +})); + +jest.mock('../../navigation/appNavigation', () => ({ + __esModule: true, + default: { navigate: jest.fn() } +})); + +type SessionRecord = { userId: string; endSession: jest.Mock }; +const createdSessions: SessionRecord[] = []; + +jest.mock('@rocket.chat/media-signaling', () => ({ + MediaCallWebRTCProcessor: jest.fn().mockImplementation(function MediaCallWebRTCProcessor(this: unknown) { + return this; + }), + MediaSignalingSession: jest.fn().mockImplementation(function MockMediaSignalingSession(this: any, config: { userId: string }) { + const endSession = jest.fn(); + createdSessions.push({ userId: config.userId, endSession }); + this.userId = config.userId; + this.endSession = endSession; + this.on = jest.fn(); + this.processSignal = jest.fn().mockResolvedValue(undefined); + this.setIceGatheringTimeout = jest.fn(); + this.startCall = jest.fn().mockResolvedValue(undefined); + this.getMainCall = jest.fn(); + }) +})); + +describe('MediaSessionInstance', () => { + beforeEach(() => { + jest.clearAllMocks(); + createdSessions.length = 0; + mediaSessionInstance.reset(); + }); + + afterEach(() => { + mediaSessionInstance.reset(); + }); + + describe('init', () => { + it('should register stream-notify-user listener', () => { + mediaSessionInstance.init('user-1'); + expect(mockOnStreamData).toHaveBeenCalledWith('stream-notify-user', expect.any(Function)); + }); + + it('should create session with userId', () => { + mediaSessionInstance.init('user-abc'); + expect(createdSessions).toHaveLength(1); + expect(createdSessions[0].userId).toBe('user-abc'); + }); + + it('should route sendSignal through sdk.methodCall with user media-calls channel', () => { + const spy = jest.spyOn(mediaSessionStore, 'setSendSignalFn'); + mediaSessionInstance.init('user-xyz'); + expect(spy).toHaveBeenCalled(); + const sendFn = spy.mock.calls[spy.mock.calls.length - 1][0] as (signal: { type: string }) => void; + sendFn({ type: 'register' }); + expect(mockMethodCall).toHaveBeenCalledWith( + 'stream-notify-user', + 'user-xyz/media-calls', + expect.stringContaining('register') + ); + spy.mockRestore(); + }); + }); + + describe('teardown and user switch', () => { + it('should call endSession on previous session when init with different userId', () => { + mediaSessionInstance.init('user-1'); + const first = createdSessions[0]; + mediaSessionInstance.init('user-2'); + expect(first.endSession).toHaveBeenCalled(); + expect(createdSessions[createdSessions.length - 1].userId).toBe('user-2'); + }); + + it('should only have one active onChange handler after re-init (getInstance once per change emit)', () => { + mediaSessionInstance.init('user-1'); + mediaSessionInstance.init('user-2'); + const spy = jest.spyOn(mediaSessionStore, 'getInstance'); + mediaSessionStore.emit('change'); + expect(spy).toHaveBeenCalledTimes(1); + expect(spy).toHaveBeenCalledWith('user-2'); + spy.mockRestore(); + }); + + it('should throw existing makeInstance error when getInstance after reset without init', () => { + mediaSessionInstance.init('user-1'); + mediaSessionInstance.reset(); + expect(() => mediaSessionStore.getInstance('any')).toThrow('WebRTC processor factory and send signal function must be set'); + }); + + it('should allow init after reset', () => { + mediaSessionInstance.init('user-1'); + mediaSessionInstance.reset(); + mediaSessionInstance.init('user-2'); + expect(createdSessions[createdSessions.length - 1].userId).toBe('user-2'); + }); + + it('should not throw when reset is called twice', () => { + mediaSessionInstance.init('user-1'); + expect(() => { + mediaSessionInstance.reset(); + mediaSessionInstance.reset(); + }).not.toThrow(); + }); + }); +}); diff --git a/app/lib/services/voip/MediaSessionInstance.ts b/app/lib/services/voip/MediaSessionInstance.ts index 02e32e961a1..772086438d0 100644 --- a/app/lib/services/voip/MediaSessionInstance.ts +++ b/app/lib/services/voip/MediaSessionInstance.ts @@ -26,13 +26,13 @@ class MediaSessionInstance { private iceServers: IceServer[] = []; private iceGatheringTimeout: number = 5000; private mediaSignalListener: { stop: () => void } | null = null; - private mediaSignalsListener: { stop: () => void } | null = null; private instance: MediaSignalingSession | null = null; + private mediaSessionStoreChangeUnsubscribe: (() => void) | null = null; private storeTimeoutUnsubscribe: (() => void) | null = null; private storeIceServersUnsubscribe: (() => void) | null = null; public init(userId: string): void { - this.stop(); + this.reset(); registerGlobals(); this.configureIceServers(); // prevent JS and native DDP clients from interfering with each other @@ -50,7 +50,9 @@ class MediaSessionInstance { sdk.methodCall('stream-notify-user', `${userId}/media-calls`, JSON.stringify(signal)); }); this.instance = mediaSessionStore.getInstance(userId); - mediaSessionStore.onChange(() => (this.instance = mediaSessionStore.getInstance(userId))); + this.mediaSessionStoreChangeUnsubscribe = mediaSessionStore.onChange(() => { + this.instance = mediaSessionStore.getInstance(userId); + }); this.mediaSignalListener = sdk.onStreamData('stream-notify-user', (ddpMessage: IDDPMessage) => { if (!this.instance) { @@ -168,22 +170,26 @@ class MediaSessionInstance { }); } - private stop() { + public reset() { + if (this.mediaSessionStoreChangeUnsubscribe) { + this.mediaSessionStoreChangeUnsubscribe(); + this.mediaSessionStoreChangeUnsubscribe = null; + } if (this.mediaSignalListener?.stop) { this.mediaSignalListener.stop(); } - if (this.mediaSignalsListener?.stop) { - this.mediaSignalsListener.stop(); - } + this.mediaSignalListener = null; if (this.storeTimeoutUnsubscribe) { this.storeTimeoutUnsubscribe(); + this.storeTimeoutUnsubscribe = null; } if (this.storeIceServersUnsubscribe) { this.storeIceServersUnsubscribe(); + this.storeIceServersUnsubscribe = null; } - if (this.instance) { - this.instance.endSession(); - } + mediaSessionStore.dispose(); + this.instance = null; + useCallStore.getState().reset(); } } diff --git a/app/lib/services/voip/MediaSessionStore.ts b/app/lib/services/voip/MediaSessionStore.ts index dfb4719176a..8ccc513999c 100644 --- a/app/lib/services/voip/MediaSessionStore.ts +++ b/app/lib/services/voip/MediaSessionStore.ts @@ -93,6 +93,16 @@ class MediaSessionStore extends Emitter<{ change: void }> { public getCurrentInstance(): MediaSignalingSession | null { return this.sessionInstance; } + + public dispose(): void { + if (this.sessionInstance !== null) { + this.sessionInstance.endSession(); + this.sessionInstance = null; + } + this.sendSignalFn = null; + this._webrtcProcessorFactory = null; + this.change(); + } } // TODO: change name diff --git a/app/sagas/login.js b/app/sagas/login.js index d3c52a6b9a8..a011021fe2c 100644 --- a/app/sagas/login.js +++ b/app/sagas/login.js @@ -32,7 +32,7 @@ import { getSlashCommands } from '../lib/methods/getSlashCommands'; import { getUserPresence, subscribeUsersPresence } from '../lib/methods/getUsersPresence'; import { logout, removeServerData, removeServerDatabase } from '../lib/methods/logout'; import { subscribeSettings } from '../lib/methods/getSettings'; -import { loginWithPassword, login } from '../lib/services/connect'; +import { disconnect, loginWithPassword, login } from '../lib/services/connect'; import { saveUserProfile, registerPushToken, getUsersRoles } from '../lib/services/restApi'; import { setUsersRoles } from '../actions/usersRoles'; import { getServerById } from '../lib/database/services/Server'; @@ -234,6 +234,8 @@ const startVoipFork = function* startVoipFork() { if (isVoipModuleAvailable() && (hasPermissions[0] || hasPermissions[1])) { const userId = yield select(state => state.login.user.id); mediaSessionInstance.init(userId); + } else { + mediaSessionInstance.reset(); } } catch (e) { log(e); @@ -411,10 +413,10 @@ const handleDeleteAccount = function* handleDeleteAccount() { } } // if there's no servers, go outside - sdk.disconnect(); + disconnect(); yield put(appStart({ root: RootEnum.ROOT_OUTSIDE })); } catch (e) { - sdk.disconnect(); + disconnect(); yield put(appStart({ root: RootEnum.ROOT_OUTSIDE })); log(e); } diff --git a/app/sagas/selectServer.ts b/app/sagas/selectServer.ts index 5373f6b0fcf..ca9e76e0b33 100644 --- a/app/sagas/selectServer.ts +++ b/app/sagas/selectServer.ts @@ -39,6 +39,7 @@ import { setPermissions } from '../lib/methods/getPermissions'; import { setRoles } from '../lib/methods/getRoles'; import { connect, disconnect, getWebsocketInfo, getLoginServices } from '../lib/services/connect'; import sdk from '../lib/services/sdk'; +import { mediaSessionInstance } from '../lib/services/voip/MediaSessionInstance'; import { appSelector } from '../lib/hooks/useAppSelector'; import { getServerById } from '../lib/database/services/Server'; import { getLoggedUserById } from '../lib/database/services/LoggedUser'; @@ -150,6 +151,7 @@ const handleSelectServer = function* handleSelectServer({ server, version, fetch yield put(inquiryReset()); yield put(encryptionStop()); yield put(clearActiveUsers()); + mediaSessionInstance.reset(); const userId = UserPreferences.getString(`${TOKEN_KEY}-${server}`); let user = null; if (userId) { diff --git a/app/views/NewServerView/hooks/useConnectServer.tsx b/app/views/NewServerView/hooks/useConnectServer.tsx index fa48fa73697..04bab105efa 100644 --- a/app/views/NewServerView/hooks/useConnectServer.tsx +++ b/app/views/NewServerView/hooks/useConnectServer.tsx @@ -1,7 +1,7 @@ import { Keyboard } from 'react-native'; import { useDispatch } from 'react-redux'; -import sdk from '../../../lib/services/sdk'; +import { disconnect } from '../../../lib/services/connect'; import { events, logEvent } from '../../../lib/methods/helpers/log'; import { selectServerClear, serverRequest } from '../../../actions/server'; import completeUrl from '../utils/completeUrl'; @@ -24,7 +24,7 @@ const useConnectServer = ({ workspaceUrl, certificate, previousServer }: TUseNew // Clear the previous workspace to prevent being stuck on the previous server if (!previousServer) { - sdk.disconnect(); + disconnect(); dispatch(selectServerClear()); } if (workspaceUrl || serverUrl) {