From 572fa99e67b16d4133b9c01cd6798af997425a0c Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 21 Feb 2022 12:17:09 +0000 Subject: [PATCH] Fix virtual / native room mapping on call transfers (#7848) * Fix virtual / native room mapping on call transfers By just sending them through the same code path as regular calls. Also re-do the tests & add a test for transfer specifically. * Optional arg * Types --- src/CallHandler.tsx | 11 ++- test/CallHandler-test.ts | 187 +++++++++++++++++++++++++++++---------- 2 files changed, 151 insertions(+), 47 deletions(-) diff --git a/src/CallHandler.tsx b/src/CallHandler.tsx index 8fd9db1ef1..d1458ef39c 100644 --- a/src/CallHandler.tsx +++ b/src/CallHandler.tsx @@ -901,7 +901,7 @@ export default class CallHandler extends EventEmitter { this.pause(AudioID.Ring); } - public async dialNumber(number: string): Promise { + public async dialNumber(number: string, transferee?: MatrixCall): Promise { const results = await this.pstnLookup(number); if (!results || results.length === 0 || !results[0].userid) { Modal.createTrackedDialog('', '', ErrorDialog, { @@ -932,12 +932,19 @@ export default class CallHandler extends EventEmitter { metricsTrigger: "WebDialPad", }); - await this.placeMatrixCall(roomId, CallType.Voice, null); + await this.placeMatrixCall(roomId, CallType.Voice, transferee); } public async startTransferToPhoneNumber( call: MatrixCall, destination: string, consultFirst: boolean, ): Promise { + if (consultFirst) { + // if we're consulting, we just start by placing a call to the transfer + // target (passing the transferee so the actual tranfer can happen later) + this.dialNumber(destination, call); + return; + } + const results = await this.pstnLookup(destination); if (!results || results.length === 0 || !results[0].userid) { Modal.createTrackedDialog('', '', ErrorDialog, { diff --git a/test/CallHandler-test.ts b/test/CallHandler-test.ts index 6aaf8cbe1b..7affcb9d82 100644 --- a/test/CallHandler-test.ts +++ b/test/CallHandler-test.ts @@ -16,10 +16,13 @@ limitations under the License. import './skinned-sdk'; +import { IProtocol } from 'matrix-js-sdk/src'; import { CallEvent, CallState, CallType } from 'matrix-js-sdk/src/webrtc/call'; import EventEmitter from 'events'; -import CallHandler, { CallHandlerEvent } from '../src/CallHandler'; +import CallHandler, { + CallHandlerEvent, PROTOCOL_PSTN, PROTOCOL_PSTN_PREFIXED, PROTOCOL_SIP_NATIVE, PROTOCOL_SIP_VIRTUAL, +} from '../src/CallHandler'; import { stubClient, mkStubRoom } from './test-utils'; import { MatrixClientPeg } from '../src/MatrixClientPeg'; import dis from '../src/dispatcher/dispatcher'; @@ -28,9 +31,26 @@ import SdkConfig from '../src/SdkConfig'; import { ActionPayload } from '../src/dispatcher/payloads'; import { Action } from "../src/dispatcher/actions"; -const REAL_ROOM_ID = '$room1:example.org'; -const MAPPED_ROOM_ID = '$room2:example.org'; -const MAPPED_ROOM_ID_2 = '$room3:example.org'; +// The Matrix IDs that the user sees when talking to Alice & Bob +const NATIVE_ALICE = "@alice:example.org"; +const NATIVE_BOB = "@bob:example.org"; +const NATIVE_CHARLIE = "@charlie:example.org"; + +// Virtual user for Bob +const VIRTUAL_BOB = "@virtual_bob:example.org"; + +//const REAL_ROOM_ID = "$room1:example.org"; +// The rooms the user sees when they're communicating with these users +const NATIVE_ROOM_ALICE = "$alice_room:example.org"; +const NATIVE_ROOM_BOB = "$bob_room:example.org"; +const NATIVE_ROOM_CHARLIE = "$charlie_room:example.org"; + +// The room we use to talk to virtual Bob (but that the user does not see) +// Bob has a virtual room, but Alice doesn't +const VIRTUAL_ROOM_BOB = "$virtual_bob_room:example.org"; + +// Bob's phone number +const BOB_PHONE_NUMBER = "01818118181"; function mkStubDM(roomId, userId) { const room = mkStubRoom(roomId); @@ -103,6 +123,10 @@ describe('CallHandler', () => { let audioElement; let fakeCall; + // what addresses the app has looked up via pstn and native lookup + let pstnLookup: string; + let nativeLookup: string; + beforeEach(() => { stubClient(); MatrixClientPeg.get().createCall = roomId => { @@ -113,41 +137,58 @@ describe('CallHandler', () => { return fakeCall; }; + MatrixClientPeg.get().getThirdpartyProtocols = () => { + return Promise.resolve({ + "m.id.phone": {} as IProtocol, + "im.vector.protocol.sip_native": {} as IProtocol, + "im.vector.protocol.sip_virtual": {} as IProtocol, + }); + }; + callHandler = new CallHandler(); callHandler.start(); - const realRoom = mkStubDM(REAL_ROOM_ID, '@user1:example.org'); - const mappedRoom = mkStubDM(MAPPED_ROOM_ID, '@user2:example.org'); - const mappedRoom2 = mkStubDM(MAPPED_ROOM_ID_2, '@user3:example.org'); + const nativeRoomAlice = mkStubDM(NATIVE_ROOM_ALICE, NATIVE_ALICE); + const nativeRoomBob = mkStubDM(NATIVE_ROOM_BOB, NATIVE_BOB); + const nativeRoomCharie = mkStubDM(NATIVE_ROOM_CHARLIE, NATIVE_CHARLIE); + const virtualBobRoom = mkStubDM(VIRTUAL_ROOM_BOB, VIRTUAL_BOB); MatrixClientPeg.get().getRoom = roomId => { switch (roomId) { - case REAL_ROOM_ID: - return realRoom; - case MAPPED_ROOM_ID: - return mappedRoom; - case MAPPED_ROOM_ID_2: - return mappedRoom2; + case NATIVE_ROOM_ALICE: + return nativeRoomAlice; + case NATIVE_ROOM_BOB: + return nativeRoomBob; + case NATIVE_ROOM_CHARLIE: + return nativeRoomCharie; + case VIRTUAL_ROOM_BOB: + return virtualBobRoom; } }; dmRoomMap = { getUserIdForRoomId: roomId => { - if (roomId === REAL_ROOM_ID) { - return '@user1:example.org'; - } else if (roomId === MAPPED_ROOM_ID) { - return '@user2:example.org'; - } else if (roomId === MAPPED_ROOM_ID_2) { - return '@user3:example.org'; + if (roomId === NATIVE_ROOM_ALICE) { + return NATIVE_ALICE; + } else if (roomId === NATIVE_ROOM_BOB) { + return NATIVE_BOB; + } else if (roomId === NATIVE_ROOM_CHARLIE) { + return NATIVE_CHARLIE; + } else if (roomId === VIRTUAL_ROOM_BOB) { + return VIRTUAL_BOB; } else { return null; } }, getDMRoomsForUserId: userId => { - if (userId === '@user2:example.org') { - return [MAPPED_ROOM_ID]; - } else if (userId === '@user3:example.org') { - return [MAPPED_ROOM_ID_2]; + if (userId === NATIVE_ALICE) { + return [NATIVE_ROOM_ALICE]; + } else if (userId === NATIVE_BOB) { + return [NATIVE_ROOM_BOB]; + } else if (userId === NATIVE_CHARLIE) { + return [NATIVE_ROOM_CHARLIE]; + } else if (userId === VIRTUAL_BOB) { + return [VIRTUAL_ROOM_BOB]; } else { return []; } @@ -155,6 +196,48 @@ describe('CallHandler', () => { }; DMRoomMap.setShared(dmRoomMap); + pstnLookup = null; + nativeLookup = null; + + MatrixClientPeg.get().getThirdpartyUser = (proto, params) => { + if ([PROTOCOL_PSTN, PROTOCOL_PSTN_PREFIXED].includes(proto)) { + pstnLookup = params['m.id.phone']; + return Promise.resolve([{ + userid: VIRTUAL_BOB, + protocol: "m.id.phone", + fields: { + is_native: true, + lookup_success: true, + }, + }]); + } else if (proto === PROTOCOL_SIP_NATIVE) { + nativeLookup = params['virtual_mxid']; + if (params['virtual_mxid'] === VIRTUAL_BOB) { + return Promise.resolve([{ + userid: NATIVE_BOB, + protocol: "im.vector.protocol.sip_native", + fields: { + is_native: true, + lookup_success: true, + }, + }]); + } + return Promise.resolve([]); + } else if (proto === PROTOCOL_SIP_VIRTUAL) { + if (params['native_mxid'] === NATIVE_BOB) { + return Promise.resolve([{ + userid: VIRTUAL_BOB, + protocol: "im.vector.protocol.sip_virtual", + fields: { + is_virtual: true, + lookup_success: true, + }, + }]); + } + return Promise.resolve([]); + } + }; + audioElement = document.createElement('audio'); audioElement.id = "remoteAudio"; document.body.appendChild(audioElement); @@ -173,31 +256,45 @@ describe('CallHandler', () => { }); it('should look up the correct user and start a call in the room when a phone number is dialled', async () => { - MatrixClientPeg.get().getThirdpartyUser = jest.fn().mockResolvedValue([{ - userid: '@user2:example.org', - protocol: "im.vector.protocol.sip_native", - fields: { - is_native: true, - lookup_success: true, - }, - }]); + await callHandler.dialNumber(BOB_PHONE_NUMBER); - await callHandler.dialNumber('01818118181'); + expect(pstnLookup).toEqual(BOB_PHONE_NUMBER); + expect(nativeLookup).toEqual(VIRTUAL_BOB); + // we should have switched to the native room for Bob const viewRoomPayload = await untilDispatch(Action.ViewRoom); - expect(viewRoomPayload.room_id).toEqual(MAPPED_ROOM_ID); + expect(viewRoomPayload.room_id).toEqual(NATIVE_ROOM_BOB); - // Check that a call was started - expect(fakeCall.roomId).toEqual(MAPPED_ROOM_ID); + // Check that a call was started: its room on the protocol level + // should be the virtual room + expect(fakeCall.roomId).toEqual(VIRTUAL_ROOM_BOB); + + // but it should appear to the user to be in thw native room for Bob + expect(callHandler.roomIdForCall(fakeCall)).toEqual(NATIVE_ROOM_BOB); + }); + + it('should look up the correct user and start a call in the room when a call is transferred', async () => { + // we can pass a very minimal object as as the call since we pass consultFirst=true: + // we don't need to actually do any transferring + const mockTransferreeCall = { type: CallType.Voice }; + await callHandler.startTransferToPhoneNumber(mockTransferreeCall, BOB_PHONE_NUMBER, true); + + // same checks as above + const viewRoomPayload = await untilDispatch(Action.ViewRoom); + expect(viewRoomPayload.room_id).toEqual(NATIVE_ROOM_BOB); + + expect(fakeCall.roomId).toEqual(VIRTUAL_ROOM_BOB); + + expect(callHandler.roomIdForCall(fakeCall)).toEqual(NATIVE_ROOM_BOB); }); it('should move calls between rooms when remote asserted identity changes', async () => { - callHandler.placeCall(REAL_ROOM_ID, CallType.Voice); + callHandler.placeCall(NATIVE_ROOM_ALICE, CallType.Voice); await untilCallHandlerEvent(callHandler, CallHandlerEvent.CallState); - // should start off in the actual room ID it's in at the protocol level - expect(callHandler.getCallForRoom(REAL_ROOM_ID)).toBe(fakeCall); + // We placed the call in Alice's room so it should start off there + expect(callHandler.getCallForRoom(NATIVE_ROOM_ALICE)).toBe(fakeCall); let callRoomChangeEventCount = 0; const roomChangePromise = new Promise(resolve => { @@ -207,10 +304,10 @@ describe('CallHandler', () => { }); }); - // Now emit an asserted identity for user2: this should be ignored + // Now emit an asserted identity for Bob: this should be ignored // because we haven't set the config option to obey asserted identity fakeCall.getRemoteAssertedIdentity = jest.fn().mockReturnValue({ - id: "@user2:example.org", + id: NATIVE_BOB, }); fakeCall.emit(CallEvent.AssertedIdentityChanged); @@ -223,7 +320,7 @@ describe('CallHandler', () => { // ...and send another asserted identity event for a different user fakeCall.getRemoteAssertedIdentity = jest.fn().mockReturnValue({ - id: "@user3:example.org", + id: NATIVE_CHARLIE, }); fakeCall.emit(CallEvent.AssertedIdentityChanged); @@ -231,13 +328,13 @@ describe('CallHandler', () => { callHandler.removeAllListeners(); // If everything's gone well, we should have seen only one room change - // event and the call should now be in user 3's room. - // If it's not obeying any, the call will still be in REAL_ROOM_ID. + // event and the call should now be in Charlie's room. + // If it's not obeying any, the call will still be in NATIVE_ROOM_ALICE. // If it incorrectly obeyed both asserted identity changes, either it will // have just processed one and the call will be in the wrong room, or we'll // have seen two room change dispatches. expect(callRoomChangeEventCount).toEqual(1); - expect(callHandler.getCallForRoom(REAL_ROOM_ID)).toBeNull(); - expect(callHandler.getCallForRoom(MAPPED_ROOM_ID_2)).toBe(fakeCall); + expect(callHandler.getCallForRoom(NATIVE_ROOM_BOB)).toBeNull(); + expect(callHandler.getCallForRoom(NATIVE_ROOM_CHARLIE)).toBe(fakeCall); }); });