From 7038a17ef4e01f1d0fdc780cb7b4b4d2fc335382 Mon Sep 17 00:00:00 2001 From: Christopher Poile Date: Tue, 8 Nov 2022 14:45:10 -0500 Subject: [PATCH] MM-48000: Call started in another client creates a CurrentCall on mobile client (#6708) * move creating/leaving currentCall into the connection; refactoring * refactoring; two-stage connecting so existing screen stream can be saved * better separation of concerns * change warning to debug * use getServerDatabase instead of getActiveServerDatabase * fix bugs * PR comments --- app/products/calls/actions/calls.test.ts | 25 ++++++-- app/products/calls/actions/calls.ts | 11 ++-- app/products/calls/alerts.ts | 19 +++++- .../components/channel_info_start/index.ts | 6 +- app/products/calls/connection/connection.ts | 2 +- .../calls/connection/websocket_client.ts | 5 +- app/products/calls/state/actions.test.ts | 18 +++++- app/products/calls/state/actions.ts | 58 +++++++++++++------ app/products/calls/types/calls.ts | 13 ++--- app/screens/channel/index.tsx | 2 +- app/screens/thread/index.tsx | 2 +- 11 files changed, 116 insertions(+), 45 deletions(-) diff --git a/app/products/calls/actions/calls.test.ts b/app/products/calls/actions/calls.test.ts index 9352ccb75..8d1d20406 100644 --- a/app/products/calls/actions/calls.test.ts +++ b/app/products/calls/actions/calls.test.ts @@ -11,6 +11,8 @@ import {getConnectionForTesting} from '@calls/actions/calls'; import * as Permissions from '@calls/actions/permissions'; import * as State from '@calls/state'; import { + myselfLeftCall, + newCurrentCall, setCallsConfig, setCallsState, setChannelsWithCalls, @@ -139,7 +141,10 @@ describe('Actions.Calls', () => { let response: { data?: string }; await act(async () => { - response = await CallsActions.joinCall('server1', 'channel-id', true); + response = await CallsActions.joinCall('server1', 'channel-id', 'myUserId', true); + + // manually call newCurrentConnection because newConnection is mocked + newCurrentCall('server1', 'channel-id', 'myUserId'); userJoinedCall('server1', 'channel-id', 'myUserId'); }); @@ -163,7 +168,10 @@ describe('Actions.Calls', () => { let response: { data?: string }; await act(async () => { - response = await CallsActions.joinCall('server1', 'channel-id', true); + response = await CallsActions.joinCall('server1', 'channel-id', 'myUserId', true); + + // manually call newCurrentConnection because newConnection is mocked + newCurrentCall('server1', 'channel-id', 'myUserId'); userJoinedCall('server1', 'channel-id', 'myUserId'); }); assert.equal(response!.data, 'channel-id'); @@ -174,6 +182,9 @@ describe('Actions.Calls', () => { await act(async () => { CallsActions.leaveCall(); + + // because disconnect is mocked + myselfLeftCall(); }); expect(disconnectMock).toBeCalled(); @@ -191,7 +202,10 @@ describe('Actions.Calls', () => { let response: { data?: string }; await act(async () => { - response = await CallsActions.joinCall('server1', 'channel-id', true); + response = await CallsActions.joinCall('server1', 'channel-id', 'myUserId', true); + + // manually call newCurrentConnection because newConnection is mocked + newCurrentCall('server1', 'channel-id', 'myUserId'); userJoinedCall('server1', 'channel-id', 'myUserId'); }); assert.equal(response!.data, 'channel-id'); @@ -218,7 +232,10 @@ describe('Actions.Calls', () => { let response: { data?: string }; await act(async () => { - response = await CallsActions.joinCall('server1', 'channel-id', true); + response = await CallsActions.joinCall('server1', 'channel-id', 'mysUserId', true); + + // manually call newCurrentConnection because newConnection is mocked + newCurrentCall('server1', 'channel-id', 'myUserId'); userJoinedCall('server1', 'channel-id', 'myUserId'); }); assert.equal(response!.data, 'channel-id'); diff --git a/app/products/calls/actions/calls.ts b/app/products/calls/actions/calls.ts index 8f3972cb6..bc1a2dd10 100644 --- a/app/products/calls/actions/calls.ts +++ b/app/products/calls/actions/calls.ts @@ -8,7 +8,6 @@ import {fetchUsersByIds} from '@actions/remote/user'; import { getCallsConfig, getCallsState, - myselfLeftCall, setCalls, setChannelEnabled, setConfig, @@ -16,6 +15,8 @@ import { setScreenShareURL, setSpeakerPhone, setCallForChannel, + newCurrentCall, + myselfLeftCall, } from '@calls/state'; import {General, Preferences} from '@constants'; import Calls from '@constants/calls'; @@ -218,7 +219,7 @@ export const enableChannelCalls = async (serverUrl: string, channelId: string, e return {}; }; -export const joinCall = async (serverUrl: string, channelId: string, hasMicPermission: boolean): Promise<{ error?: string | Error; data?: string }> => { +export const joinCall = async (serverUrl: string, channelId: string, userId: string, hasMicPermission: boolean): Promise<{ error?: string | Error; data?: string }> => { // Edge case: calls was disabled when app loaded, and then enabled, but app hasn't // reconnected its websocket since then (i.e., hasn't called batchLoadCalls yet) const {data: enabled} = await checkIsCallsPluginEnabled(serverUrl); @@ -231,9 +232,12 @@ export const joinCall = async (serverUrl: string, channelId: string, hasMicPermi connection = null; } setSpeakerphoneOn(false); + newCurrentCall(serverUrl, channelId, userId); try { - connection = await newConnection(serverUrl, channelId, () => null, setScreenShareURL, hasMicPermission); + connection = await newConnection(serverUrl, channelId, () => { + myselfLeftCall(); + }, setScreenShareURL, hasMicPermission); } catch (error: unknown) { await forceLogoutIfNecessary(serverUrl, error as ClientError); return {error: error as Error}; @@ -255,7 +259,6 @@ export const leaveCall = () => { connection = null; } setSpeakerphoneOn(false); - myselfLeftCall(); }; export const muteMyself = () => { diff --git a/app/products/calls/alerts.ts b/app/products/calls/alerts.ts index 37b4bb10f..986fc0af3 100644 --- a/app/products/calls/alerts.ts +++ b/app/products/calls/alerts.ts @@ -6,6 +6,9 @@ import {Alert} from 'react-native'; import {hasMicrophonePermission, joinCall, unmuteMyself} from '@calls/actions'; import {setMicPermissionsGranted} from '@calls/state'; import {errorAlert} from '@calls/utils'; +import DatabaseManager from '@database/manager'; +import {getCurrentUser} from '@queries/servers/user'; +import {logError} from '@utils/log'; import type {IntlShape} from 'react-intl'; @@ -90,10 +93,24 @@ export const leaveAndJoinWithAlert = ( const doJoinCall = async (serverUrl: string, channelId: string, isDMorGM: boolean, intl: IntlShape) => { const {formatMessage} = intl; + let user; + try { + const {database} = DatabaseManager.getServerDatabaseAndOperator(serverUrl); + + user = await getCurrentUser(database); + if (!user) { + // This shouldn't happen, so don't bother localizing and displaying an alert. + return; + } + } catch (error) { + logError('failed to getServerDatabaseAndOperator in doJoinCall', error); + return; + } + const hasPermission = await hasMicrophonePermission(); setMicPermissionsGranted(hasPermission); - const res = await joinCall(serverUrl, channelId, hasPermission); + const res = await joinCall(serverUrl, channelId, user.id, hasPermission); if (res.error) { const seeLogs = formatMessage({id: 'mobile.calls_see_logs', defaultMessage: 'See server logs'}); errorAlert(res.error?.toString() || seeLogs, intl); diff --git a/app/products/calls/components/channel_info_start/index.ts b/app/products/calls/components/channel_info_start/index.ts index 77fd3a2f9..62abb3001 100644 --- a/app/products/calls/components/channel_info_start/index.ts +++ b/app/products/calls/components/channel_info_start/index.ts @@ -34,13 +34,12 @@ const enhanced = withObservables([], ({serverUrl, channelId, database}: EnhanceP switchMap((calls) => of$(Boolean(calls[channelId]))), distinctUntilChanged(), ); - const currentCall = observeCurrentCall(); - const ccDatabase = currentCall.pipe( + const ccDatabase = observeCurrentCall().pipe( switchMap((call) => of$(call?.serverUrl || '')), distinctUntilChanged(), switchMap((url) => of$(DatabaseManager.serverDatabases[url]?.database)), ); - const ccChannelId = currentCall.pipe( + const ccChannelId = observeCurrentCall().pipe( switchMap((call) => of$(call?.channelId)), distinctUntilChanged(), ); @@ -58,7 +57,6 @@ const enhanced = withObservables([], ({serverUrl, channelId, database}: EnhanceP isACallInCurrentChannel, confirmToJoin, alreadyInCall, - currentCall, currentCallChannelName, limitRestrictedInfo: observeIsCallLimitRestricted(serverUrl, channelId), }; diff --git a/app/products/calls/connection/connection.ts b/app/products/calls/connection/connection.ts index fec4a6dbc..f0a71f04a 100644 --- a/app/products/calls/connection/connection.ts +++ b/app/products/calls/connection/connection.ts @@ -60,7 +60,6 @@ export async function newConnection( // getClient can throw an error, which will be handled by the caller. const client = NetworkManager.getClient(serverUrl); - const credentials = await getServerCredentials(serverUrl); const ws = new WebSocketClient(serverUrl, client.getWebSocketUrl(), credentials?.token); @@ -198,6 +197,7 @@ export async function newConnection( InCallManager.start({media: 'audio'}); InCallManager.stopProximitySensor(); + peer = new Peer(null, iceConfigs); peer.on('signal', (data: any) => { if (data.type === 'offer' || data.type === 'answer') { diff --git a/app/products/calls/connection/websocket_client.ts b/app/products/calls/connection/websocket_client.ts index f632348f1..64dc5baa3 100644 --- a/app/products/calls/connection/websocket_client.ts +++ b/app/products/calls/connection/websocket_client.ts @@ -8,7 +8,7 @@ import {encode} from '@msgpack/msgpack/dist'; import Calls from '@constants/calls'; import DatabaseManager from '@database/manager'; import {getCommonSystemValues} from '@queries/servers/system'; -import {logError} from '@utils/log'; +import {logDebug, logError} from '@utils/log'; const wsMinReconnectRetryTimeMs = 1000; // 1 second const wsReconnectionTimeout = 30000; // 30 seconds @@ -88,9 +88,11 @@ export class WebSocketClient extends EventEmitter { if (msg.event === 'hello') { if (msg.data.connection_id !== this.connID) { + logDebug('calls: ws new conn id from server'); this.connID = msg.data.connection_id; this.serverSeqNo = 0; if (this.originalConnID === '') { + logDebug('calls: ws setting original conn id'); this.originalConnID = this.connID; } } @@ -99,6 +101,7 @@ export class WebSocketClient extends EventEmitter { } return; } else if (!this.connID) { + logDebug('calls: ws message received while waiting for hello'); return; } diff --git a/app/products/calls/state/actions.test.ts b/app/products/calls/state/actions.test.ts index 6bb7755cc..5de034c7e 100644 --- a/app/products/calls/state/actions.test.ts +++ b/app/products/calls/state/actions.test.ts @@ -6,6 +6,7 @@ import assert from 'assert'; import {act, renderHook} from '@testing-library/react-hooks'; import { + newCurrentCall, setCallsState, setChannelsWithCalls, setCurrentCall, @@ -14,7 +15,8 @@ import { useCallsConfig, useCallsState, useChannelsWithCalls, - useCurrentCall, useGlobalCallsState, + useCurrentCall, + useGlobalCallsState, } from '@calls/state'; import { setCalls, @@ -193,6 +195,7 @@ describe('useCallsState', () => { const initialCurrentCallState: CurrentCall = { ...DefaultCurrentCall, + connected: true, serverUrl: 'server1', myUserId: 'myUserId', ...call1, @@ -251,6 +254,7 @@ describe('useCallsState', () => { }; const initialCurrentCallState: CurrentCall = { ...DefaultCurrentCall, + connected: true, serverUrl: 'server1', myUserId: 'myUserId', ...call1, @@ -352,6 +356,7 @@ describe('useCallsState', () => { const initialChannelsWithCallsState = {'channel-1': true, 'channel-2': true}; const initialCurrentCallState: CurrentCall = { ...DefaultCurrentCall, + connected: true, serverUrl: 'server1', myUserId: 'myUserId', ...call1, @@ -455,6 +460,7 @@ describe('useCallsState', () => { }; const initialCurrentCallState: CurrentCall = { ...DefaultCurrentCall, + connected: true, serverUrl: 'server1', myUserId: 'myUserId', ...call1, @@ -512,6 +518,7 @@ describe('useCallsState', () => { }; const expectedCurrentCallState: CurrentCall = { ...DefaultCurrentCall, + connected: true, serverUrl: 'server1', myUserId: 'myUserId', ...newCall1, @@ -526,7 +533,10 @@ describe('useCallsState', () => { assert.deepEqual(result.current[1], null); // test - act(() => userJoinedCall('server1', 'channel-1', 'myUserId')); + act(() => { + newCurrentCall('server1', 'channel-1', 'myUserId'); + userJoinedCall('server1', 'channel-1', 'myUserId'); + }); assert.deepEqual(result.current[0], expectedCallsState); assert.deepEqual(result.current[1], expectedCurrentCallState); @@ -596,6 +606,7 @@ describe('useCallsState', () => { assert.deepEqual(result.current[1], null); // test joining a call and setting url: + act(() => newCurrentCall('server1', 'channel-1', 'myUserId')); act(() => userJoinedCall('server1', 'channel-1', 'myUserId')); assert.deepEqual((result.current[1] as CurrentCall | null)?.screenShareURL, ''); act(() => setScreenShareURL('testUrl')); @@ -640,6 +651,7 @@ describe('useCallsState', () => { assert.deepEqual(result.current[1], null); // test + act(() => newCurrentCall('server1', 'channel-1', 'myUserId')); act(() => userJoinedCall('server1', 'channel-1', 'myUserId')); assert.deepEqual((result.current[1] as CurrentCall | null)?.speakerphoneOn, false); act(() => setSpeakerPhone(true)); @@ -680,6 +692,7 @@ describe('useCallsState', () => { ...DefaultCurrentCall, serverUrl: 'server1', myUserId: 'myUserId', + connected: true, ...newCall1, }; const secondExpectedCurrentCallState: CurrentCall = { @@ -702,6 +715,7 @@ describe('useCallsState', () => { // join call act(() => { setMicPermissionsGranted(false); + newCurrentCall('server1', 'channel-1', 'myUserId'); userJoinedCall('server1', 'channel-1', 'myUserId'); }); assert.deepEqual(result.current[0], expectedCallsState); diff --git a/app/products/calls/state/actions.ts b/app/products/calls/state/actions.ts index d0b2462b3..3b58e2f4b 100644 --- a/app/products/calls/state/actions.ts +++ b/app/products/calls/state/actions.ts @@ -13,7 +13,7 @@ import { setCurrentCall, setGlobalCallsState, } from '@calls/state'; -import {Call, CallsConfig, ChannelsWithCalls} from '@calls/types/calls'; +import {Call, CallsConfig, ChannelsWithCalls, DefaultCall, DefaultCurrentCall} from '@calls/types/calls'; export const setCalls = (serverUrl: string, myUserId: string, calls: Dictionary, enabled: Dictionary) => { const channelsWithCalls = Object.keys(calls).reduce( @@ -105,21 +105,13 @@ export const userJoinedCall = (serverUrl: string, channelId: string, userId: str participants: {...currentCall.participants, [userId]: nextCall.participants[userId]}, voiceOn, }; - setCurrentCall(nextCurrentCall); - } - // Was it me that joined the call? - if (callsState.myUserId === userId) { - setCurrentCall({ - ...nextCall, - participants: {...nextCall.participants}, - serverUrl, - myUserId: userId, - screenShareURL: '', - speakerphoneOn: false, - voiceOn: {}, - micPermissionsErrorDismissed: false, - }); + // If this is the currentUser, that means we've connected to the call we created. + if (userId === nextCurrentCall.myUserId) { + nextCurrentCall.connected = true; + } + + setCurrentCall(nextCurrentCall); } }; @@ -173,6 +165,22 @@ export const userLeftCall = (serverUrl: string, channelId: string, userId: strin setCurrentCall(nextCurrentCall); }; +export const newCurrentCall = (serverUrl: string, channelId: string, myUserId: string) => { + let existingCall: Call = DefaultCall; + const callsState = getCallsState(serverUrl); + if (callsState.calls[channelId]) { + existingCall = callsState.calls[channelId]; + } + + setCurrentCall({ + ...DefaultCurrentCall, + ...existingCall, + serverUrl, + channelId, + myUserId, + }); +}; + export const myselfLeftCall = () => { setCurrentCall(null); }; @@ -185,6 +193,21 @@ export const callStarted = (serverUrl: string, call: Call) => { const nextChannelsWithCalls = {...getChannelsWithCalls(serverUrl), [call.channelId]: true}; setChannelsWithCalls(serverUrl, nextChannelsWithCalls); + + // If we started a call, we will get a callStarted event with the 'official' data from the server. + // Save that in our currentCall. + const currentCall = getCurrentCall(); + if (!currentCall || currentCall.channelId !== call.channelId) { + return; + } + + const nextCurrentCall = { + ...currentCall, + startTime: call.startTime, + threadId: call.threadId, + ownerId: call.ownerId, + }; + setCurrentCall(nextCurrentCall); }; export const callEnded = (serverUrl: string, channelId: string) => { @@ -198,10 +221,7 @@ export const callEnded = (serverUrl: string, channelId: string) => { delete nextChannelsWithCalls[channelId]; setChannelsWithCalls(serverUrl, nextChannelsWithCalls); - const currentCall = getCurrentCall(); - if (currentCall?.channelId === channelId) { - setCurrentCall(null); - } + // currentCall is set to null by the disconnect. }; export const setUserMuted = (serverUrl: string, channelId: string, userId: string, muted: boolean) => { diff --git a/app/products/calls/types/calls.ts b/app/products/calls/types/calls.ts index 8bd41ac5e..e32714490 100644 --- a/app/products/calls/types/calls.ts +++ b/app/products/calls/types/calls.ts @@ -35,22 +35,19 @@ export type Call = { ownerId: string; } -export const DefaultCall = { +export const DefaultCall: Call = { participants: {} as Dictionary, channelId: '', startTime: 0, screenOn: '', threadId: '', + ownerId: '', }; -export type CurrentCall = { +export type CurrentCall = Call & { + connected: boolean; serverUrl: string; myUserId: string; - participants: Dictionary; - channelId: string; - startTime: number; - screenOn: string; - threadId: string; screenShareURL: string; speakerphoneOn: boolean; voiceOn: Dictionary; @@ -58,6 +55,7 @@ export type CurrentCall = { } export const DefaultCurrentCall: CurrentCall = { + connected: false, serverUrl: '', myUserId: '', participants: {}, @@ -65,6 +63,7 @@ export const DefaultCurrentCall: CurrentCall = { startTime: 0, screenOn: '', threadId: '', + ownerId: '', screenShareURL: '', speakerphoneOn: false, voiceOn: {}, diff --git a/app/screens/channel/index.tsx b/app/screens/channel/index.tsx index 7ed6ba6e9..f911ca0e4 100644 --- a/app/screens/channel/index.tsx +++ b/app/screens/channel/index.tsx @@ -30,7 +30,7 @@ const enhanced = withObservables([], ({database, serverUrl}: EnhanceProps) => { distinctUntilChanged(), ); const isInACall = currentCall.pipe( - switchMap((call) => of$(Boolean(call))), + switchMap((call) => of$(Boolean(call?.connected))), distinctUntilChanged(), ); const isInCurrentChannelCall = combineLatest([channelId, ccChannelId]).pipe( diff --git a/app/screens/thread/index.tsx b/app/screens/thread/index.tsx index 5d635cfbb..1c0a367b9 100644 --- a/app/screens/thread/index.tsx +++ b/app/screens/thread/index.tsx @@ -15,7 +15,7 @@ import type {WithDatabaseArgs} from '@typings/database/database'; const enhanced = withObservables(['rootId'], ({database, rootId}: WithDatabaseArgs & {rootId: string}) => { const isInACall = observeCurrentCall().pipe( - switchMap((call) => of$(Boolean(call))), + switchMap((call) => of$(Boolean(call?.connected))), distinctUntilChanged(), );