From c8202f0d9e0ba1a94bdfeab9c5033de811e11c14 Mon Sep 17 00:00:00 2001 From: mickelr <121160648+mickelr@users.noreply.github.com> Date: Wed, 3 Apr 2024 09:08:44 +0800 Subject: [PATCH 01/20] Feat/si: cancel named media groups after remove interpreter role (#3508) --- packages/@webex/plugin-meetings/src/interpretation/index.ts | 4 ++-- .../plugin-meetings/test/unit/spec/interpretation/index.ts | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/@webex/plugin-meetings/src/interpretation/index.ts b/packages/@webex/plugin-meetings/src/interpretation/index.ts index a543ce6c622..9544a7ceafb 100644 --- a/packages/@webex/plugin-meetings/src/interpretation/index.ts +++ b/packages/@webex/plugin-meetings/src/interpretation/index.ts @@ -124,14 +124,14 @@ const SimultaneousInterpretation = WebexPlugin.extend({ * @returns {bool} is target language changed */ updateSelfInterpretation({interpretation, selfParticipantId}) { - this.set('selfIsInterpreter', true); const preTargetLanguage = this.targetLanguage; const {originalLanguage, sourceLanguage, order, isActive, targetLanguage, receiveLanguage} = interpretation || {}; this.set({originalLanguage, sourceLanguage, order, isActive, targetLanguage, receiveLanguage}); this.set('selfParticipantId', selfParticipantId); + this.set('selfIsInterpreter', !!targetLanguage); - return !!(targetLanguage && preTargetLanguage !== targetLanguage); + return !!(preTargetLanguage !== targetLanguage); }, /** diff --git a/packages/@webex/plugin-meetings/test/unit/spec/interpretation/index.ts b/packages/@webex/plugin-meetings/test/unit/spec/interpretation/index.ts index 7aa427ee9f2..58c397d82c1 100644 --- a/packages/@webex/plugin-meetings/test/unit/spec/interpretation/index.ts +++ b/packages/@webex/plugin-meetings/test/unit/spec/interpretation/index.ts @@ -131,6 +131,7 @@ describe('plugin-meetings', () => { assert.equal(interpretation.receiveLanguage, 'en'); assert.equal(interpretation.isActive, true); assert.equal(interpretation.order, 0); + assert.equal(interpretation.selfIsInterpreter, true); sampleData.interpretation = { originalLanguage: 'en', @@ -141,13 +142,15 @@ describe('plugin-meetings', () => { assert.equal(interpretation.sourceLanguage, undefined); assert.equal(interpretation.targetLanguage, 'zh'); assert.equal(interpretation.receiveLanguage, undefined); + assert.equal(interpretation.selfIsInterpreter, true); sampleData.interpretation = { order: 0, }; - assert.equal(interpretation.updateSelfInterpretation(sampleData), false); + assert.equal(interpretation.updateSelfInterpretation(sampleData), true); assert.equal(interpretation.originalLanguage, undefined); assert.equal(interpretation.targetLanguage, undefined); + assert.equal(interpretation.selfIsInterpreter, false); }); }); From ca99162f60eacb931f017bc68518728e9d22bdfe Mon Sep 17 00:00:00 2001 From: JudyZhu <120536178+JudyZhuHz@users.noreply.github.com> Date: Wed, 3 Apr 2024 17:26:59 +0800 Subject: [PATCH 02/20] feat(plugin-meetings): Return resourceType when get locus annotation info (#3504) --- .../src/locus-info/mediaSharesUtils.ts | 16 +++++++++ .../plugin-meetings/src/meeting/index.ts | 3 ++ .../unit/spec/locus-info/mediaSharesUtils.ts | 9 +++++ .../test/unit/spec/meeting/index.js | 35 ++++++++++++------- 4 files changed, 50 insertions(+), 13 deletions(-) diff --git a/packages/@webex/plugin-meetings/src/locus-info/mediaSharesUtils.ts b/packages/@webex/plugin-meetings/src/locus-info/mediaSharesUtils.ts index dde515069d9..f46758a2c71 100644 --- a/packages/@webex/plugin-meetings/src/locus-info/mediaSharesUtils.ts +++ b/packages/@webex/plugin-meetings/src/locus-info/mediaSharesUtils.ts @@ -17,6 +17,7 @@ MediaSharesUtils.parse = (mediaShares: object) => { url: MediaSharesUtils.getContentUrl(mediaShares), shareInstanceId: MediaSharesUtils.getShareInstanceId(mediaShares), deviceUrlSharing: MediaSharesUtils.getContentBeneficiaryDeviceUrl(mediaShares), + resourceType: MediaSharesUtils.getContentResourceType(mediaShares), }, whiteboard: { beneficiaryId: MediaSharesUtils.getWhiteboardBeneficiaryId(mediaShares), @@ -159,6 +160,21 @@ MediaSharesUtils.getContentAnnotation = (mediaShares: object) => { return extractContent.annotation; }; +/** + * get live resourceType is sharing from media shares (content) + * @param {Object} mediaShares + * @returns {Object} + */ +MediaSharesUtils.getContentResourceType = (mediaShares: object) => { + const extractContent = MediaSharesUtils.extractContent(mediaShares); + + if (!extractContent || !extractContent.resourceType) { + return undefined; + } + + return extractContent.resourceType; +}; + /** * get url is sharing from media shares (content) * @param {Object} mediaShares diff --git a/packages/@webex/plugin-meetings/src/meeting/index.ts b/packages/@webex/plugin-meetings/src/meeting/index.ts index 2765ba7d1fd..ec9fe7e6a14 100644 --- a/packages/@webex/plugin-meetings/src/meeting/index.ts +++ b/packages/@webex/plugin-meetings/src/meeting/index.ts @@ -2519,6 +2519,7 @@ export default class Meeting extends StatelessWebexPlugin { { annotationInfo: contentShare?.annotation, meetingId: this.id, + resourceType: contentShare?.resourceType, } ); } @@ -2669,6 +2670,7 @@ export default class Meeting extends StatelessWebexPlugin { url: contentShare.url, shareInstanceId: this.remoteShareInstanceId, annotationInfo: contentShare.annotation, + resourceType: contentShare.resourceType, } ); }; @@ -2761,6 +2763,7 @@ export default class Meeting extends StatelessWebexPlugin { url: contentShare.url, shareInstanceId: this.remoteShareInstanceId, annotationInfo: contentShare.annotation, + resourceType: contentShare.resourceType, } ); this.members.locusMediaSharesUpdate(payload); diff --git a/packages/@webex/plugin-meetings/test/unit/spec/locus-info/mediaSharesUtils.ts b/packages/@webex/plugin-meetings/test/unit/spec/locus-info/mediaSharesUtils.ts index 352c92698b5..0eed0839921 100644 --- a/packages/@webex/plugin-meetings/test/unit/spec/locus-info/mediaSharesUtils.ts +++ b/packages/@webex/plugin-meetings/test/unit/spec/locus-info/mediaSharesUtils.ts @@ -20,6 +20,15 @@ describe('getContentUrl', () => { }); }); +describe('getContentResourceType', () => { + it('getContentResourceType return correct resourceType value', () => { + const stub = Sinon.stub(MediaSharesUtils, 'extractContent').returns({resourceType:'resourceType'}); + const resourceType = MediaSharesUtils.getContentResourceType(); + assert.equal(resourceType,'resourceType'); + stub.restore(); + }); +}); + describe('getContentBeneficiaryDeviceUrl', () => { it('getContentBeneficiaryDeviceUrl return correct deviceUrl value', () => { const mockContentBeneficiaryDeviceUrl = "https://wdm-a.wbx2.com/wdm/api/v1/devices/e9ffd8a1-1fae-42d1-afbe-013e951f93ab" diff --git a/packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js b/packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js index d18714433e9..85bf17a56e6 100644 --- a/packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js +++ b/packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js @@ -9427,9 +9427,9 @@ describe('plugin-meetings', () => { it('check triggerAnnotationInfoEvent event', () => { TriggerProxy.trigger.reset(); const annotationInfo = {version: '1', policy: 'Approval'}; - const expectAnnotationInfo = {annotationInfo, meetingId: meeting.id}; + const expectAnnotationInfo = {annotationInfo, meetingId: meeting.id, resourceType: 'FILE'}; meeting.webex.meetings = {}; - meeting.triggerAnnotationInfoEvent({annotation: annotationInfo}, {}); + meeting.triggerAnnotationInfoEvent({annotation: annotationInfo, resourceType: 'FILE'}, {}); assert.calledWith( TriggerProxy.trigger, {}, @@ -9443,8 +9443,8 @@ describe('plugin-meetings', () => { TriggerProxy.trigger.reset(); meeting.triggerAnnotationInfoEvent( - {annotation: annotationInfo}, - {annotation: annotationInfo} + {annotation: annotationInfo, resourceType: 'FILE'}, + {annotation: annotationInfo, resourceType: 'FILE'} ); assert.notCalled(TriggerProxy.trigger); @@ -9453,10 +9453,11 @@ describe('plugin-meetings', () => { const expectAnnotationInfoUpdated = { annotationInfo: annotationInfoUpdate, meetingId: meeting.id, + resourceType: 'FILE', }; meeting.triggerAnnotationInfoEvent( - {annotation: annotationInfoUpdate}, - {annotation: annotationInfo} + {annotation: annotationInfoUpdate, resourceType: 'FILE'}, + {annotation: annotationInfo, resourceType: 'FILE'} ); assert.calledWith( TriggerProxy.trigger, @@ -9470,7 +9471,7 @@ describe('plugin-meetings', () => { ); TriggerProxy.trigger.reset(); - meeting.triggerAnnotationInfoEvent(null, {annotation: annotationInfoUpdate}); + meeting.triggerAnnotationInfoEvent(null, {annotation: annotationInfoUpdate, resourceType: 'FILE'}); assert.notCalled(TriggerProxy.trigger); }); }); @@ -9505,11 +9506,14 @@ describe('plugin-meetings', () => { beneficiaryId = null, disposition = null, deviceUrlSharing = null, - annotation = undefined + annotation = undefined, + resourceType = undefined, ) => ({ beneficiaryId, disposition, deviceUrlSharing, + annotation, + resourceType, }); const generateWhiteboard = ( beneficiaryId = null, @@ -9528,7 +9532,8 @@ describe('plugin-meetings', () => { annotation, url, shareInstanceId, - deviceUrlSharing + deviceUrlSharing, + resourceType ) => { const newPayload = cloneDeep(payload); @@ -9562,7 +9567,8 @@ describe('plugin-meetings', () => { beneficiaryId, FLOOR_ACTION.GRANTED, deviceUrlSharing, - annotation + annotation, + resourceType ); if (isEqual(newPayload.current, newPayload.previous)) { @@ -9623,6 +9629,7 @@ describe('plugin-meetings', () => { url, shareInstanceId, annotationInfo: undefined, + resourceType: undefined }, }); } @@ -10464,7 +10471,8 @@ describe('plugin-meetings', () => { undefined, undefined, undefined, - DEVICE_URL.REMOTE_A + DEVICE_URL.REMOTE_A, + undefined ); const data2 = generateData( data1.payload, @@ -10477,9 +10485,10 @@ describe('plugin-meetings', () => { undefined, undefined, undefined, - DEVICE_URL.REMOTE_B + DEVICE_URL.REMOTE_B, + undefined ); - const data3 = generateData(data2.payload, false, true, USER_IDS.REMOTE_B); + const data3 = generateData(data2.payload, false, true, USER_IDS.REMOTE_B, undefined); payloadTestHelper([data1, data2, data3]); }); From d8759668706b880824dddb6f62c979feb61f7e8a Mon Sep 17 00:00:00 2001 From: shnaaz <82164376+shnaaz@users.noreply.github.com> Date: Wed, 3 Apr 2024 16:33:10 +0100 Subject: [PATCH 03/20] fix(internal-plugin-metrics): define latency before ready (#3514) --- .../src/new-metrics.ts | 4 +- .../test/unit/spec/new-metrics.ts | 60 +++++++++---------- 2 files changed, 31 insertions(+), 33 deletions(-) diff --git a/packages/@webex/internal-plugin-metrics/src/new-metrics.ts b/packages/@webex/internal-plugin-metrics/src/new-metrics.ts index 411fa29a896..b9749dc3efa 100644 --- a/packages/@webex/internal-plugin-metrics/src/new-metrics.ts +++ b/packages/@webex/internal-plugin-metrics/src/new-metrics.ts @@ -43,6 +43,8 @@ class Metrics extends WebexPlugin { constructor(...args) { super(...args); + // @ts-ignore + this.callDiagnosticLatencies = new CallDiagnosticLatencies({}, {parent: this.webex}); this.onReady(); } @@ -54,8 +56,6 @@ class Metrics extends WebexPlugin { this.webex.once('ready', () => { // @ts-ignore this.callDiagnosticMetrics = new CallDiagnosticMetrics({}, {parent: this.webex}); - // @ts-ignore - this.callDiagnosticLatencies = new CallDiagnosticLatencies({}, {parent: this.webex}); }); } diff --git a/packages/@webex/internal-plugin-metrics/test/unit/spec/new-metrics.ts b/packages/@webex/internal-plugin-metrics/test/unit/spec/new-metrics.ts index 5bb1a674e39..d300c91a39b 100644 --- a/packages/@webex/internal-plugin-metrics/test/unit/spec/new-metrics.ts +++ b/packages/@webex/internal-plugin-metrics/test/unit/spec/new-metrics.ts @@ -1,29 +1,31 @@ import {assert} from '@webex/test-helper-chai'; -import {NewMetrics} from '@webex/internal-plugin-metrics'; +import {NewMetrics, CallDiagnosticLatencies} from '@webex/internal-plugin-metrics'; import MockWebex from '@webex/test-helper-mock-webex'; import sinon from 'sinon'; import {Utils} from '@webex/internal-plugin-metrics'; describe('internal-plugin-metrics', () => { + const mockWebex = () => new MockWebex({ + children: { + newMetrics: NewMetrics, + }, + meetings: { + meetingCollection: { + get: sinon.stub(), + }, + }, + request: sinon.stub().resolves({}), + logger: { + log: sinon.stub(), + error: sinon.stub(), + } + }); + describe('check submitClientEvent when webex is not ready', () => { let webex; //@ts-ignore - webex = new MockWebex({ - children: { - newMetrics: NewMetrics, - }, - meetings: { - meetingCollection: { - get: sinon.stub(), - }, - }, - request: sinon.stub().resolves({}), - logger: { - log: sinon.stub(), - error: sinon.stub(), - } - }); + webex = mockWebex(); it('checks the log', () => { webex.internal.newMetrics.submitClientEvent({ @@ -38,26 +40,22 @@ describe('internal-plugin-metrics', () => { ); }); }); + + describe('new-metrics contstructor', () => { + it('checks callDiagnosticLatencies is defined before ready emit', () => { + + const webex = mockWebex(); + + assert.instanceOf(webex.internal.newMetrics.callDiagnosticLatencies, CallDiagnosticLatencies); + }); + }); + describe('new-metrics', () => { let webex; beforeEach(() => { //@ts-ignore - webex = new MockWebex({ - children: { - newMetrics: NewMetrics, - }, - meetings: { - meetingCollection: { - get: sinon.stub(), - }, - }, - request: sinon.stub().resolves({}), - logger: { - log: sinon.stub(), - error: sinon.stub(), - } - }); + webex = mockWebex(); webex.emit('ready'); From 2b596e977b04c2260ef425c776978e293d6974cb Mon Sep 17 00:00:00 2001 From: Marcin Date: Wed, 3 Apr 2024 17:31:54 +0100 Subject: [PATCH 04/20] fix: handle empty TURN url (#3516) --- .../@webex/plugin-meetings/src/media/index.ts | 4 +- .../src/reconnection-manager/index.ts | 2 +- .../test/unit/spec/media/index.ts | 167 ++++++++++-------- .../test/unit/spec/meeting/index.js | 4 +- .../unit/spec/reconnection-manager/index.js | 28 +++ 5 files changed, 123 insertions(+), 82 deletions(-) diff --git a/packages/@webex/plugin-meetings/src/media/index.ts b/packages/@webex/plugin-meetings/src/media/index.ts index eec862652a7..15d2376106e 100644 --- a/packages/@webex/plugin-meetings/src/media/index.ts +++ b/packages/@webex/plugin-meetings/src/media/index.ts @@ -160,7 +160,9 @@ Media.createMediaConnection = ( const iceServers = []; - if (turnServerInfo) { + // we might not have any TURN server if TURN discovery failed or wasn't done or + // we might get an empty TURN url if we land on a video mesh node + if (turnServerInfo?.url) { iceServers.push({ urls: turnServerInfo.url, username: turnServerInfo.username || '', diff --git a/packages/@webex/plugin-meetings/src/reconnection-manager/index.ts b/packages/@webex/plugin-meetings/src/reconnection-manager/index.ts index acd548d24f1..3eb149dfbfc 100644 --- a/packages/@webex/plugin-meetings/src/reconnection-manager/index.ts +++ b/packages/@webex/plugin-meetings/src/reconnection-manager/index.ts @@ -568,7 +568,7 @@ export default class ReconnectionManager { const iceServers = []; - if (turnServerResult.turnServerInfo) { + if (turnServerResult.turnServerInfo?.url) { iceServers.push({ urls: turnServerResult.turnServerInfo.url, username: turnServerResult.turnServerInfo.username || '', diff --git a/packages/@webex/plugin-meetings/test/unit/spec/media/index.ts b/packages/@webex/plugin-meetings/test/unit/spec/media/index.ts index 161a6815ccd..52f62cc9c30 100644 --- a/packages/@webex/plugin-meetings/test/unit/spec/media/index.ts +++ b/packages/@webex/plugin-meetings/test/unit/spec/media/index.ts @@ -170,37 +170,15 @@ describe('createMediaConnection', () => { ); }); - it('passes empty ICE servers array to MultistreamRoapMediaConnection if turnServerInfo is undefined (multistream enabled)', () => { - const multistreamRoapMediaConnectionConstructorStub = sinon - .stub(internalMediaModule, 'MultistreamRoapMediaConnection') - .returns(fakeRoapMediaConnection); - - Media.createMediaConnection(true, 'debug string', webex, 'meeting id', 'correlationId', { - mediaProperties: { - mediaDirection: { - sendAudio: true, - sendVideo: true, - sendShare: false, - receiveAudio: true, - receiveVideo: true, - receiveShare: true, - }, - }, - }); - assert.calledOnce(multistreamRoapMediaConnectionConstructorStub); - assert.calledWith( - multistreamRoapMediaConnectionConstructorStub, - { - iceServers: [], - }, - 'meeting id' - ); - - it('does not pass bundlePolicy to MultistreamRoapMediaConnection if bundlePolicy is undefined', () => { + [ + {testCase: 'turnServerInfo is undefined', turnServerInfo: undefined}, + {testCase: 'turnServerInfo.url is empty string', turnServerInfo: {url: '', username: 'turn username', password: 'turn password'}}, + ].forEach(({testCase, turnServerInfo}) => { + it(`passes empty ICE servers array to MultistreamRoapMediaConnection if ${testCase} (multistream enabled)`, () => { const multistreamRoapMediaConnectionConstructorStub = sinon .stub(internalMediaModule, 'MultistreamRoapMediaConnection') .returns(fakeRoapMediaConnection); - + Media.createMediaConnection(true, 'debug string', webex, 'meeting id', 'correlationId', { mediaProperties: { mediaDirection: { @@ -212,7 +190,7 @@ describe('createMediaConnection', () => { receiveShare: true, }, }, - bundlePolicy: undefined, + turnServerInfo, }); assert.calledOnce(multistreamRoapMediaConnectionConstructorStub); assert.calledWith( @@ -221,75 +199,108 @@ describe('createMediaConnection', () => { iceServers: [], }, 'meeting id' - ); + ); }); }); - - it('passes empty ICE servers array to RoapMediaConnection if turnServerInfo is undefined (multistream disabled)', () => { - const roapMediaConnectionConstructorStub = sinon - .stub(internalMediaModule, 'RoapMediaConnection') + + it('does not pass bundlePolicy to MultistreamRoapMediaConnection if bundlePolicy is undefined', () => { + const multistreamRoapMediaConnectionConstructorStub = sinon + .stub(internalMediaModule, 'MultistreamRoapMediaConnection') .returns(fakeRoapMediaConnection); - StaticConfig.set({bandwidth: {audio: 123, video: 456, startBitrate: 999}}); - - const ENABLE_EXTMAP = false; - const ENABLE_RTX = true; - - Media.createMediaConnection(false, 'some debug id', webex, 'meeting id', 'correlationId', { + Media.createMediaConnection(true, 'debug string', webex, 'meeting id', 'correlationId', { mediaProperties: { mediaDirection: { sendAudio: true, sendVideo: true, - sendShare: true, + sendShare: false, receiveAudio: true, receiveVideo: true, receiveShare: true, }, - audioStream: fakeAudioStream, - videoStream: null, - shareVideoStream: fakeShareVideoStream, - shareAudioStream: fakeShareAudioStream, }, - remoteQualityLevel: 'HIGH', - enableRtx: ENABLE_RTX, - enableExtmap: ENABLE_EXTMAP, - turnServerInfo: undefined, + bundlePolicy: undefined, }); - assert.calledOnce(roapMediaConnectionConstructorStub); + assert.calledOnce(multistreamRoapMediaConnectionConstructorStub); assert.calledWith( - roapMediaConnectionConstructorStub, + multistreamRoapMediaConnectionConstructorStub, { iceServers: [], - skipInactiveTransceivers: false, - requireH264: true, - sdpMunging: { - convertPort9to0: false, - addContentSlides: true, - bandwidthLimits: { - audio: 123, - video: 456, + }, + 'meeting id' + ); + }); + + [ + {testCase: 'turnServerInfo is undefined', turnServerInfo: undefined}, + {testCase: 'turnServerInfo.url is empty string', turnServerInfo: {url: '', username: 'turn username', password: 'turn password'}}, + ].forEach(({testCase, turnServerInfo}) => { + it(`passes empty ICE servers array to RoapMediaConnection if ${testCase} (multistream disabled)`, () => { + const roapMediaConnectionConstructorStub = sinon + .stub(internalMediaModule, 'RoapMediaConnection') + .returns(fakeRoapMediaConnection); + + StaticConfig.set({bandwidth: {audio: 123, video: 456, startBitrate: 999}}); + + const ENABLE_EXTMAP = false; + const ENABLE_RTX = true; + + Media.createMediaConnection(false, 'some debug id', webex, 'meeting id', 'correlationId', { + mediaProperties: { + mediaDirection: { + sendAudio: true, + sendVideo: true, + sendShare: true, + receiveAudio: true, + receiveVideo: true, + receiveShare: true, }, - startBitrate: 999, - periodicKeyframes: 20, - disableExtmap: !ENABLE_EXTMAP, - disableRtx: !ENABLE_RTX, + audioStream: fakeAudioStream, + videoStream: null, + shareVideoStream: fakeShareVideoStream, + shareAudioStream: fakeShareAudioStream, }, - }, - { - localTracks: { - audio: fakeTrack, - video: undefined, - screenShareVideo: fakeTrack, - screenShareAudio: fakeTrack, + remoteQualityLevel: 'HIGH', + enableRtx: ENABLE_RTX, + enableExtmap: ENABLE_EXTMAP, + turnServerInfo, + }); + assert.calledOnce(roapMediaConnectionConstructorStub); + assert.calledWith( + roapMediaConnectionConstructorStub, + { + iceServers: [], + skipInactiveTransceivers: false, + requireH264: true, + sdpMunging: { + convertPort9to0: false, + addContentSlides: true, + bandwidthLimits: { + audio: 123, + video: 456, + }, + startBitrate: 999, + periodicKeyframes: 20, + disableExtmap: !ENABLE_EXTMAP, + disableRtx: !ENABLE_RTX, + }, }, - direction: { - audio: 'sendrecv', - video: 'sendrecv', - screenShareVideo: 'sendrecv', + { + localTracks: { + audio: fakeTrack, + video: undefined, + screenShareVideo: fakeTrack, + screenShareAudio: fakeTrack, + }, + direction: { + audio: 'sendrecv', + video: 'sendrecv', + screenShareVideo: 'sendrecv', + }, + remoteQualityLevel: 'HIGH', }, - remoteQualityLevel: 'HIGH', - }, - 'some debug id' - ); + 'some debug id' + ); + }); }); }); diff --git a/packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js b/packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js index 85bf17a56e6..7bbeddfe05f 100644 --- a/packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js +++ b/packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js @@ -2857,7 +2857,7 @@ describe('plugin-meetings', () => { meeting.webex.meetings.geoHintInfo = {regionCode: 'EU', countryCode: 'UK'}; meeting.roap.doTurnDiscovery = sinon .stub() - .resolves({turnServerInfo: {}, turnDiscoverySkippedReason: 'reachability'}); + .resolves({turnServerInfo: { url: 'turn-url', username: 'turn user', password: 'turn password'}, turnDiscoverySkippedReason: 'reachability'}); meeting.deferSDPAnswer = new Defer(); meeting.deferSDPAnswer.resolve(); meeting.webex.meetings.meetingCollection = new MeetingCollection(); @@ -2868,7 +2868,7 @@ describe('plugin-meetings', () => { // setup things that are expected to be the same across all the tests and are actually irrelevant for these tests expectedDebugId = `MC-${meeting.id.substring(0, 4)}`; expectedMediaConnectionConfig = { - iceServers: [{urls: undefined, username: '', credential: ''}], + iceServers: [{urls: 'turn-url', username: 'turn user', credential: 'turn password'}], skipInactiveTransceivers: false, requireH264: true, sdpMunging: { diff --git a/packages/@webex/plugin-meetings/test/unit/spec/reconnection-manager/index.js b/packages/@webex/plugin-meetings/test/unit/spec/reconnection-manager/index.js index 0c24f54e2d2..3a1e050c9c9 100644 --- a/packages/@webex/plugin-meetings/test/unit/spec/reconnection-manager/index.js +++ b/packages/@webex/plugin-meetings/test/unit/spec/reconnection-manager/index.js @@ -144,6 +144,34 @@ describe('plugin-meetings', () => { }); }); + // this can happen when we land on a video mesh node + it('does not use TURN server if TURN url is an empty string', async () => { + const rm = new ReconnectionManager(fakeMeeting); + + fakeMeeting.roap.doTurnDiscovery.resolves({ + turnServerInfo: { + url: '', + username: 'whatever', + password: 'whatever', + }, + turnDiscoverySkippedReason: undefined, + }); + + await rm.reconnect(); + + assert.calledOnce(fakeMeeting.roap.doTurnDiscovery); + assert.calledWith(fakeMeeting.roap.doTurnDiscovery, fakeMeeting, true, true); + assert.calledOnce(fakeMediaConnection.reconnect); + assert.calledWith(fakeMediaConnection.reconnect, []); + + assert.calledWith(fakeMeeting.webex.internal.newMetrics.submitClientEvent, { + name: 'client.media.reconnecting', + options: { + meetingId: rm.meeting.id, + }, + }); + }); + it('does not clear previous requests and re-request media for non-multistream meetings', async () => { fakeMeeting.isMultistream = false; const rm = new ReconnectionManager(fakeMeeting); From f2a612d072c59d97492b61c55c71071086126c11 Mon Sep 17 00:00:00 2001 From: Peter Cole <55573154+peter7cole@users.noreply.github.com> Date: Wed, 3 Apr 2024 11:15:26 -0700 Subject: [PATCH 05/20] fix(meetings-and-device): installationid-and-machineid (#3454) --- packages/@webex/internal-plugin-device/src/config.js | 9 +++++++++ .../src/call-diagnostic/call-diagnostic-metrics.ts | 6 ++++++ .../unit/spec/call-diagnostic/call-diagnostic-metrics.ts | 6 ++++++ 3 files changed, 21 insertions(+) diff --git a/packages/@webex/internal-plugin-device/src/config.js b/packages/@webex/internal-plugin-device/src/config.js index 6d54b13bbe8..8f37c6dc6f8 100644 --- a/packages/@webex/internal-plugin-device/src/config.js +++ b/packages/@webex/internal-plugin-device/src/config.js @@ -57,4 +57,13 @@ export default { */ ephemeralDeviceTTL: 30 * 60, }, + + /** + * installationId is used exclusively as web client for fraud prevention, + * and is aliased to as machineId by CA. + * + * @alias device.machineId + * @type {string} + */ + installationId: undefined, }; diff --git a/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics.ts b/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics.ts index 0a23402a7bd..81bf12c7b7c 100644 --- a/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics.ts +++ b/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics.ts @@ -293,11 +293,17 @@ export default class CallDiagnosticMetrics extends StatelessWebexPlugin { if (this.webex.internal) { // @ts-ignore const {device} = this.webex.internal; + const {installationId} = device.config || {}; + identifiers.userId = device.userId || preLoginId; identifiers.deviceId = device.url; identifiers.orgId = device.orgId; // @ts-ignore identifiers.locusUrl = this.webex.internal.services.get('locus'); + + if (installationId) { + identifiers.machineId = installationId; + } } if (meeting?.locusInfo?.fullState) { diff --git a/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics.ts b/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics.ts index 7a8d3a8fd3f..43a76986d60 100644 --- a/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics.ts +++ b/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics.ts @@ -302,6 +302,11 @@ describe('internal-plugin-metrics', () => { describe('#getIdentifiers', () => { it('should build identifiers correctly', () => { + webex.internal.device = { + ...webex.internal.device, + config: {installationId: 'installationId'}, + }; + const res = cd.getIdentifiers({ mediaConnections: [ {mediaAgentAlias: 'mediaAgentAlias', mediaAgentGroupId: 'mediaAgentGroupId'}, @@ -315,6 +320,7 @@ describe('internal-plugin-metrics', () => { locusId: 'url', locusStartTime: 'lastActive', locusUrl: 'locus/url', + machineId: 'installationId', mediaAgentAlias: 'mediaAgentAlias', mediaAgentGroupId: 'mediaAgentGroupId', orgId: 'orgId', From 738ed3812e356a407e48164739790a95417aa13f Mon Sep 17 00:00:00 2001 From: shnaaz <82164376+shnaaz@users.noreply.github.com> Date: Thu, 4 Apr 2024 14:26:19 +0100 Subject: [PATCH 06/20] feat(internal-plugin-metrics): added helpers (#3515) Co-authored-by: Catalin Torge --- .../call-diagnostic-metrics-latencies.ts | 27 +++++- .../call-diagnostic-metrics-latencies.ts | 97 ++++++++++++++++++- 2 files changed, 120 insertions(+), 4 deletions(-) diff --git a/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics-latencies.ts b/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics-latencies.ts index fbf766d8fba..47219da0ce4 100644 --- a/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics-latencies.ts +++ b/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics-latencies.ts @@ -92,12 +92,33 @@ export default class CallDiagnosticLatencies extends WebexPlugin { /** * Store precomputed latency value * @param key - key - * @param value -value + * @param value - value + * @param overwrite - overwrite existing value or add it * @throws * @returns */ - public saveLatency(key: PreComputedLatencies, value: number) { - this.precomputedLatencies.set(key, value); + public saveLatency(key: PreComputedLatencies, value: number, overwrite = true) { + const existingValue = overwrite ? 0 : this.precomputedLatencies.get(key) || 0; + this.precomputedLatencies.set(key, value + existingValue); + } + + /** + * Measure latency for a request + * @param key - key + * @param callback - callback for which you would like to measure latency + * @param overwrite - overwite existing value or add to it + * @returns + */ + public measureLatency( + callback: () => Promise, + key: PreComputedLatencies, + overwrite = false + ) { + const start = performance.now(); + + return callback().finally(() => { + this.saveLatency(key, performance.now() - start, overwrite); + }); } /** diff --git a/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics-latencies.ts b/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics-latencies.ts index 7139c852681..0afb8a4f45c 100644 --- a/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics-latencies.ts +++ b/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics-latencies.ts @@ -41,11 +41,41 @@ describe('internal-plugin-metrics', () => { assert.deepEqual(cdl.latencyTimestamps.get('client.alert.displayed'), now.getTime()); }); - it('should save latency correctly', () => { + it('should save latency correctly by default and overwrites', () => { assert.deepEqual(cdl.precomputedLatencies.size, 0); cdl.saveLatency('internal.client.pageJMT', 10); assert.deepEqual(cdl.precomputedLatencies.size, 1); assert.deepEqual(cdl.precomputedLatencies.get('internal.client.pageJMT'), 10); + cdl.saveLatency('internal.client.pageJMT', 20); + assert.deepEqual(cdl.precomputedLatencies.size, 1); + assert.deepEqual(cdl.precomputedLatencies.get('internal.client.pageJMT'), 20); + }); + + it('should overwrite latency when overwrite is true', () => { + assert.deepEqual(cdl.precomputedLatencies.size, 0); + cdl.saveLatency('internal.client.pageJMT', 10, true); + assert.deepEqual(cdl.precomputedLatencies.size, 1); + assert.deepEqual(cdl.precomputedLatencies.get('internal.client.pageJMT'), 10); + cdl.saveLatency('internal.client.pageJMT', 20, true); + assert.deepEqual(cdl.precomputedLatencies.size, 1); + assert.deepEqual(cdl.precomputedLatencies.get('internal.client.pageJMT'), 20); + }); + + it('should save latency correctly when overwrite is false', () => { + assert.deepEqual(cdl.precomputedLatencies.size, 0); + cdl.saveLatency('internal.client.pageJMT', 10, false); + assert.deepEqual(cdl.precomputedLatencies.size, 1); + assert.deepEqual(cdl.precomputedLatencies.get('internal.client.pageJMT'), 10); + }); + + it('should save latency correctly when overwrite is false and there is existing value', () => { + assert.deepEqual(cdl.precomputedLatencies.size, 0); + cdl.saveLatency('internal.client.pageJMT', 10); + assert.deepEqual(cdl.precomputedLatencies.size, 1); + assert.deepEqual(cdl.precomputedLatencies.get('internal.client.pageJMT'), 10); + cdl.saveLatency('internal.client.pageJMT', 10, false); + assert.deepEqual(cdl.precomputedLatencies.size, 1); + assert.deepEqual(cdl.precomputedLatencies.get('internal.client.pageJMT'), 20); }); it('should save only first timestamp correctly', () => { @@ -111,6 +141,71 @@ describe('internal-plugin-metrics', () => { assert.deepEqual(cdl.getMeetingInfoReqResp(), 10); }); + describe('measureLatency', () => { + let clock; + let saveLatencySpy; + + beforeEach(() => { + clock = sinon.useFakeTimers(); + + saveLatencySpy = sinon.stub(cdl, 'saveLatency'); + }); + + afterEach(() => { + clock.restore(); + sinon.restore(); + }); + + it('checks measureLatency with overwrite false', async () => { + const key = 'internal.client.pageJMT'; + const overwrite = false; + const callbackStub = sinon.stub().callsFake(() => { + clock.tick(50); + return Promise.resolve('test'); + }); + + const promise = cdl.measureLatency(callbackStub, 'internal.client.pageJMT', overwrite); + + const resolvedValue = await promise; + assert.deepEqual(resolvedValue, 'test'); + assert.calledOnceWithExactly(callbackStub); + assert.calledOnceWithExactly(saveLatencySpy, key, 50, overwrite) + }); + + it('checks measureLatency with overwrite true', async () => { + const key = 'internal.download.time'; + const overwrite = true; + const callbackStub = sinon.stub().callsFake(() => { + clock.tick(20); + return Promise.resolve('test123'); + }); + + const promise = cdl.measureLatency(callbackStub, 'internal.download.time', overwrite); + + const resolvedValue = await promise; + assert.deepEqual(resolvedValue, 'test123'); + assert.calledOnceWithExactly(callbackStub); + assert.calledOnceWithExactly(saveLatencySpy, key, 20, overwrite) + }); + + it('checks measureLatency when callBack rejects', async () => { + const key = 'internal.client.pageJMT'; + const overwrite = true; + const error = new Error('some error'); + const callbackStub = sinon.stub().callsFake(() => { + clock.tick(50); + return Promise.reject(error); + }); + + const promise = cdl.measureLatency(callbackStub, 'internal.client.pageJMT', overwrite); + + const rejectedValue = await assert.isRejected(promise); + assert.deepEqual(rejectedValue, error); + assert.calledOnceWithExactly(callbackStub); + assert.calledOnceWithExactly(saveLatencySpy, key, 50, overwrite) + }); + }); + describe('saveTimestamp', () => { afterEach(() => { sinon.restore(); From 86e7308e3a3f7dd8d05d9d5410a72942437f3b75 Mon Sep 17 00:00:00 2001 From: Catalin Torge Date: Thu, 4 Apr 2024 15:07:17 +0100 Subject: [PATCH 07/20] feat(jmt): remove server side JMT (#3511) Co-authored-by: Catalin --- .../call-diagnostic/call-diagnostic-metrics-latencies.ts | 9 --------- .../src/call-diagnostic/call-diagnostic-metrics.util.ts | 1 - .../call-diagnostic/call-diagnostic-metrics-batcher.ts | 4 ---- .../spec/call-diagnostic/call-diagnostic-metrics.util.ts | 1 - 4 files changed, 15 deletions(-) diff --git a/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics-latencies.ts b/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics-latencies.ts index 47219da0ce4..e17180c9c1d 100644 --- a/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics-latencies.ts +++ b/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics-latencies.ts @@ -209,15 +209,6 @@ export default class CallDiagnosticLatencies extends WebexPlugin { return this.getDiffBetweenTimestamps('client.locus.join.request', 'client.locus.join.response'); } - /** - * Locus Join Response Sent Received - * @returns - latency - */ - public getJoinRespSentReceived() { - // TODO: not clear SPARK-440554 - return undefined; - } - /** * Time taken to do turn discovery * @returns - latency diff --git a/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics.util.ts b/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics.util.ts index 9ec2f100729..dcec777ac54 100644 --- a/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics.util.ts +++ b/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics.util.ts @@ -259,7 +259,6 @@ export const prepareDiagnosticMetricItem = (webex: any, item: any) => { joinTimes.meetingInfoReqResp = cdl.getMeetingInfoReqResp(); joinTimes.callInitJoinReq = cdl.getCallInitJoinReq(); joinTimes.joinReqResp = cdl.getJoinReqResp(); - joinTimes.joinReqSentReceived = cdl.getJoinRespSentReceived(); joinTimes.pageJmt = cdl.getPageJMT(); joinTimes.clickToInterstitial = cdl.getClickToInterstitial(); joinTimes.interstitialToJoinOK = cdl.getInterstitialToJoinOK(); diff --git a/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics-batcher.ts b/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics-batcher.ts index b152f586666..0a9e3c68b9b 100644 --- a/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics-batcher.ts +++ b/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics-batcher.ts @@ -159,9 +159,6 @@ describe('plugin-metrics', () => { webex.internal.newMetrics.callDiagnosticLatencies.getDiffBetweenTimestamps = sinon .stub() .returns(10); - webex.internal.newMetrics.callDiagnosticLatencies.getJoinRespSentReceived = sinon - .stub() - .returns(20); webex.internal.newMetrics.callDiagnosticLatencies.getPageJMT = sinon.stub().returns(30); webex.internal.newMetrics.callDiagnosticLatencies.getClientJMT = sinon.stub().returns(5); webex.internal.newMetrics.callDiagnosticLatencies.getClickToInterstitial = sinon @@ -191,7 +188,6 @@ describe('plugin-metrics', () => { clickToInterstitial: 10, interstitialToJoinOK: 10, joinReqResp: 10, - joinReqSentReceived: 20, meetingInfoReqResp: 10, pageJmt: 30, totalJmt: 20, diff --git a/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics.util.ts b/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics.util.ts index 896d41a2abe..d74a7f8da1b 100644 --- a/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics.util.ts +++ b/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics.util.ts @@ -332,7 +332,6 @@ describe('internal-plugin-metrics', () => { meetingInfoReqResp: undefined, callInitJoinReq: undefined, joinReqResp: undefined, - joinReqSentReceived: undefined, pageJmt: undefined, clickToInterstitial: undefined, interstitialToJoinOK: undefined, From 24593d4b03514987d643693f0307ea707f12789e Mon Sep 17 00:00:00 2001 From: Shreyas Sharma <72344404+Shreyas281299@users.noreply.github.com> Date: Fri, 5 Apr 2024 15:01:29 +0530 Subject: [PATCH 08/20] fix: add-next-in-beta-plugin-meetings (#3493) Co-authored-by: Shreyas Sharma Co-authored-by: Sreekanth Narayanan <131740035+sreenara@users.noreply.github.com> --- packages/@webex/plugin-meetings/src/meeting/index.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/@webex/plugin-meetings/src/meeting/index.ts b/packages/@webex/plugin-meetings/src/meeting/index.ts index ec9fe7e6a14..74b6f690be3 100644 --- a/packages/@webex/plugin-meetings/src/meeting/index.ts +++ b/packages/@webex/plugin-meetings/src/meeting/index.ts @@ -120,7 +120,6 @@ import { MeetingInfoV2CaptchaError, MeetingInfoV2PolicyError, } from '../meeting-info/meeting-info-v2'; -import BrowserDetection from '../common/browser-detection'; import {CSI, ReceiveSlotManager} from '../multistream/receiveSlotManager'; import SendSlotManager from '../multistream/sendSlotManager'; import {MediaRequestManager} from '../multistream/mediaRequestManager'; @@ -148,8 +147,6 @@ import ControlsOptionsManager from '../controls-options-manager'; import PermissionError from '../common/errors/permission'; import {LocusMediaRequest} from './locusMediaRequest'; -const {isBrowser} = BrowserDetection(); - const logRequest = (request: any, {logText = ''}) => { LoggerProxy.logger.info(`${logText} - sending request`); From 367a1d51ebb4fa0a8891d4f6594eaceabc6f57e6 Mon Sep 17 00:00:00 2001 From: Catalin Torge Date: Fri, 5 Apr 2024 15:04:21 +0100 Subject: [PATCH 09/20] feat(metrics): add otherAppApi jmt (#3496) Co-authored-by: Catalin --- .../internal-plugin-metrics/package.json | 2 +- .../call-diagnostic-metrics-latencies.ts | 10 ++++++++ .../call-diagnostic-metrics.util.ts | 3 +++ .../internal-plugin-metrics/src/index.ts | 2 ++ .../src/metrics.types.ts | 3 ++- .../call-diagnostic-metrics-latencies.ts | 24 +++++++++++++++++++ .../call-diagnostic-metrics.util.ts | 5 ++++ yarn.lock | 10 ++++---- 8 files changed, 52 insertions(+), 7 deletions(-) diff --git a/packages/@webex/internal-plugin-metrics/package.json b/packages/@webex/internal-plugin-metrics/package.json index 0aa94e8dca3..12e4f02f1a1 100644 --- a/packages/@webex/internal-plugin-metrics/package.json +++ b/packages/@webex/internal-plugin-metrics/package.json @@ -37,7 +37,7 @@ "dependencies": { "@webex/common": "workspace:*", "@webex/common-timers": "workspace:*", - "@webex/event-dictionary-ts": "^1.0.1329", + "@webex/event-dictionary-ts": "^1.0.1387", "@webex/internal-plugin-device": "workspace:*", "@webex/internal-plugin-metrics": "workspace:*", "@webex/test-helper-chai": "workspace:*", diff --git a/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics-latencies.ts b/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics-latencies.ts index e17180c9c1d..489b4009d82 100644 --- a/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics-latencies.ts +++ b/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics-latencies.ts @@ -444,4 +444,14 @@ export default class CallDiagnosticLatencies extends WebexPlugin { public getVideoJoinRespTxStart() { return this.getDiffBetweenTimestamps('client.locus.join.response', 'client.media.tx.start'); } + + /** + * Total latency for all other app api requests. + * Excludes meeting info, because it's measured separately. + */ + public getOtherAppApiReqResp() { + const otherAppApiJMT = this.precomputedLatencies.get('internal.other.app.api.time'); + + return otherAppApiJMT > 0 ? Math.floor(otherAppApiJMT) : undefined; + } } diff --git a/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics.util.ts b/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics.util.ts index dcec777ac54..cb63378e69a 100644 --- a/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics.util.ts +++ b/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics.util.ts @@ -244,6 +244,9 @@ export const prepareDiagnosticMetricItem = (webex: any, item: any) => { case 'client.webexapp.launched': joinTimes.downloadTime = cdl.getDownloadTimeJMT(); break; + case 'client.login.end': + joinTimes.otherAppApiReqResp = cdl.getOtherAppApiReqResp(); + break; case 'client.interstitial-window.launched': joinTimes.meetingInfoReqResp = cdl.getMeetingInfoReqResp(); joinTimes.clickToInterstitial = cdl.getClickToInterstitial(); diff --git a/packages/@webex/internal-plugin-metrics/src/index.ts b/packages/@webex/internal-plugin-metrics/src/index.ts index 2aa10b26e03..f02c67b109d 100644 --- a/packages/@webex/internal-plugin-metrics/src/index.ts +++ b/packages/@webex/internal-plugin-metrics/src/index.ts @@ -18,6 +18,7 @@ import { SubmitInternalEvent, SubmitOperationalEvent, SubmitMQE, + PreComputedLatencies, } from './metrics.types'; import * as CALL_DIAGNOSTIC_CONFIG from './call-diagnostic/config'; import * as CallDiagnosticUtils from './call-diagnostic/call-diagnostic-metrics.util'; @@ -51,4 +52,5 @@ export type { SubmitInternalEvent, SubmitMQE, SubmitOperationalEvent, + PreComputedLatencies, }; diff --git a/packages/@webex/internal-plugin-metrics/src/metrics.types.ts b/packages/@webex/internal-plugin-metrics/src/metrics.types.ts index 68cee8bf594..35270029378 100644 --- a/packages/@webex/internal-plugin-metrics/src/metrics.types.ts +++ b/packages/@webex/internal-plugin-metrics/src/metrics.types.ts @@ -165,4 +165,5 @@ export type PreComputedLatencies = | 'internal.client.pageJMT' | 'internal.download.time' | 'internal.click.to.interstitial' - | 'internal.call.init.join.req'; + | 'internal.call.init.join.req' + | 'internal.other.app.api.time'; diff --git a/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics-latencies.ts b/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics-latencies.ts index 0afb8a4f45c..b6e6090626f 100644 --- a/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics-latencies.ts +++ b/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics-latencies.ts @@ -611,5 +611,29 @@ describe('internal-plugin-metrics', () => { cdl.saveLatency('internal.download.time', 1000); assert.deepEqual(cdl.getDownloadTimeJMT(), 1000); }); + + describe('getOtherAppApiReqResp', () => { + it('returns undefined when no precomputed value available', () => { + assert.deepEqual(cdl.getOtherAppApiReqResp(), undefined); + }); + + it('returns undefined if it is less than 0', () => { + cdl.saveLatency('internal.other.app.api.time', 0); + + assert.deepEqual(cdl.getOtherAppApiReqResp(), undefined); + }); + + it('returns the correct value', () => { + cdl.saveLatency('internal.other.app.api.time', 123); + + assert.deepEqual(cdl.getOtherAppApiReqResp(), 123); + }); + + it('returns the correct whole number', () => { + cdl.saveLatency('internal.other.app.api.time', 321.44); + + assert.deepEqual(cdl.getOtherAppApiReqResp(), 321); + }); + }); }); }); diff --git a/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics.util.ts b/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics.util.ts index d74a7f8da1b..6dcce40a710 100644 --- a/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics.util.ts +++ b/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics.util.ts @@ -301,6 +301,11 @@ describe('internal-plugin-metrics', () => { [ ['client.exit.app', {}], + ['client.login.end', { + joinTimes: { + otherAppApiReqResp: undefined, + } + }], ['client.webexapp.launched', { joinTimes: { downloadTime: undefined, diff --git a/yarn.lock b/yarn.lock index 1a8bfc2c9d6..496840ee044 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6805,9 +6805,9 @@ __metadata: languageName: unknown linkType: soft -"@webex/event-dictionary-ts@npm:^1.0.1329": - version: 1.0.1339 - resolution: "@webex/event-dictionary-ts@npm:1.0.1339" +"@webex/event-dictionary-ts@npm:^1.0.1387": + version: 1.0.1387 + resolution: "@webex/event-dictionary-ts@npm:1.0.1387" dependencies: amf-client-js: "npm:^5.2.6" json-schema-to-typescript: "npm:^12.0.0" @@ -6815,7 +6815,7 @@ __metadata: ramldt2jsonschema: "npm:^1.2.3" shelljs: "npm:^0.8.5" webapi-parser: "npm:^0.5.0" - checksum: a3925693382294ee23b749e7591aea9e44de7dd655df90202ea4eae3468199d7b45d24d7264f0b65b60b2f6e6e101eb7537e4cca36cde9f6e2b0e5f721a0590c + checksum: 71caab44d800fb50e7b094b07a10a3a923dde23a1910e794902f954022c789cf9bc54a00b0adcd32a76e9e60e670beecf2d385277cf9a1927115e3631b572592 languageName: node linkType: hard @@ -7328,7 +7328,7 @@ __metadata: "@webex/common": "workspace:*" "@webex/common-timers": "workspace:*" "@webex/eslint-config-legacy": "workspace:*" - "@webex/event-dictionary-ts": ^1.0.1329 + "@webex/event-dictionary-ts": ^1.0.1387 "@webex/internal-plugin-device": "workspace:*" "@webex/internal-plugin-metrics": "workspace:*" "@webex/jest-config-legacy": "workspace:*" From b0d33a6e399038a4dd9373241e0317aaacdab75b Mon Sep 17 00:00:00 2001 From: shnaaz <82164376+shnaaz@users.noreply.github.com> Date: Mon, 8 Apr 2024 10:02:06 +0100 Subject: [PATCH 10/20] feat(internal-plugin-meetings): u2c latency (#3519) Co-authored-by: Catalin Torge --- .../call-diagnostic-metrics-latencies.ts | 10 ++++ .../call-diagnostic-metrics.util.ts | 1 + .../src/metrics.types.ts | 1 + .../call-diagnostic-metrics-batcher.ts | 4 ++ .../call-diagnostic-metrics-latencies.ts | 18 ++++++ .../call-diagnostic-metrics.util.ts | 1 + .../webex-core/src/lib/services/services.js | 4 +- .../test/unit/spec/services/services.js | 56 +++++++++++++++++++ 8 files changed, 94 insertions(+), 1 deletion(-) diff --git a/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics-latencies.ts b/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics-latencies.ts index 489b4009d82..c7a71228cd2 100644 --- a/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics-latencies.ts +++ b/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics-latencies.ts @@ -179,6 +179,16 @@ export default class CallDiagnosticLatencies extends WebexPlugin { ); } + /** + * getU2CTime + * @returns - latency + */ + public getU2CTime() { + const u2cLatency = this.precomputedLatencies.get('internal.get.u2c.time'); + + return u2cLatency ? Math.floor(u2cLatency) : undefined; + } + /** * Device Register Time * @returns - latency diff --git a/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics.util.ts b/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics.util.ts index cb63378e69a..3c673580f1f 100644 --- a/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics.util.ts +++ b/packages/@webex/internal-plugin-metrics/src/call-diagnostic/call-diagnostic-metrics.util.ts @@ -256,6 +256,7 @@ export const prepareDiagnosticMetricItem = (webex: any, item: any) => { joinTimes.meetingInfoReqResp = cdl.getMeetingInfoReqResp(); joinTimes.showInterstitialTime = cdl.getShowInterstitialTime(); joinTimes.registerWDMDeviceJMT = cdl.getRegisterWDMDeviceJMT(); + joinTimes.getU2CTime = cdl.getU2CTime(); break; case 'client.locus.join.response': diff --git a/packages/@webex/internal-plugin-metrics/src/metrics.types.ts b/packages/@webex/internal-plugin-metrics/src/metrics.types.ts index 35270029378..b38703888a0 100644 --- a/packages/@webex/internal-plugin-metrics/src/metrics.types.ts +++ b/packages/@webex/internal-plugin-metrics/src/metrics.types.ts @@ -165,5 +165,6 @@ export type PreComputedLatencies = | 'internal.client.pageJMT' | 'internal.download.time' | 'internal.click.to.interstitial' + | 'internal.get.u2c.time' | 'internal.call.init.join.req' | 'internal.other.app.api.time'; diff --git a/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics-batcher.ts b/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics-batcher.ts index 0a9e3c68b9b..2ed66992f46 100644 --- a/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics-batcher.ts +++ b/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics-batcher.ts @@ -131,6 +131,9 @@ describe('plugin-metrics', () => { webex.internal.newMetrics.callDiagnosticLatencies.getDiffBetweenTimestamps = sinon .stub() .returns(10); + webex.internal.newMetrics.callDiagnosticLatencies.getU2CTime = sinon + .stub() + .returns(20); const promise = webex.internal.newMetrics.callDiagnosticMetrics.submitToCallDiagnostics( //@ts-ignore {event: {name: 'client.call.initiated'}} @@ -147,6 +150,7 @@ describe('plugin-metrics', () => { meetingInfoReqResp: 10, registerWDMDeviceJMT: 10, showInterstitialTime: 10, + getU2CTime: 20 }, }); assert.lengthOf( diff --git a/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics-latencies.ts b/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics-latencies.ts index b6e6090626f..9db71dcde4b 100644 --- a/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics-latencies.ts +++ b/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics-latencies.ts @@ -607,6 +607,24 @@ describe('internal-plugin-metrics', () => { assert.deepEqual(cdl.getInterstitialToMediaOKJMT(), 10); }); + it('calculates getU2CTime correctly', () => { + it('returns undefined when no precomputed value available', () => { + assert.deepEqual(cdl.getU2CTime(), undefined); + }); + + it('returns the correct value', () => { + cdl.saveLatency('internal.get.u2c.time', 123); + + assert.deepEqual(cdl.getU2CTime(), 123); + }); + + it('returns the correct whole number', () => { + cdl.saveLatency('internal.get.u2c.time', 321.44); + + assert.deepEqual(cdl.getU2CTime(), 321); + }); + }); + it('calculates getDownloadTimeJMT correctly', () => { cdl.saveLatency('internal.download.time', 1000); assert.deepEqual(cdl.getDownloadTimeJMT(), 1000); diff --git a/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics.util.ts b/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics.util.ts index 6dcce40a710..7e79bb20834 100644 --- a/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics.util.ts +++ b/packages/@webex/internal-plugin-metrics/test/unit/spec/call-diagnostic/call-diagnostic-metrics.util.ts @@ -327,6 +327,7 @@ describe('internal-plugin-metrics', () => { showInterstitialTime: undefined, meetingInfoReqResp: undefined, registerWDMDeviceJMT: undefined, + getU2CTime: undefined }, }, ], diff --git a/packages/@webex/webex-core/src/lib/services/services.js b/packages/@webex/webex-core/src/lib/services/services.js index 1e722f89b72..43900e96212 100644 --- a/packages/@webex/webex-core/src/lib/services/services.js +++ b/packages/@webex/webex-core/src/lib/services/services.js @@ -897,7 +897,9 @@ const Services = WebexPlugin.extend({ requestObject.headers = {authorization: token}; } - return this.request(requestObject).then(({body}) => this._formatReceivedHostmap(body)); + return this.webex.internal.newMetrics.callDiagnosticLatencies + .measureLatency(() => this.request(requestObject), 'internal.get.u2c.time') + .then(({body}) => this._formatReceivedHostmap(body)); }, /** diff --git a/packages/@webex/webex-core/test/unit/spec/services/services.js b/packages/@webex/webex-core/test/unit/spec/services/services.js index 65aba3ee9ea..13bebd493e1 100644 --- a/packages/@webex/webex-core/test/unit/spec/services/services.js +++ b/packages/@webex/webex-core/test/unit/spec/services/services.js @@ -6,6 +6,7 @@ import {assert} from '@webex/test-helper-chai'; import MockWebex from '@webex/test-helper-mock-webex'; import sinon from 'sinon'; import {Services, ServiceRegistry, ServiceState} from '@webex/webex-core'; +import {NewMetrics} from '@webex/internal-plugin-metrics'; /* eslint-disable no-underscore-dangle */ describe('webex-core', () => { @@ -18,6 +19,7 @@ describe('webex-core', () => { webex = new MockWebex({ children: { services: Services, + newMetrics: NewMetrics, }, }); services = webex.internal.services; @@ -130,6 +132,60 @@ describe('webex-core', () => { }); }); + describe('#_fetchNewServiceHostmap()', () => { + + beforeEach(() => { + sinon.spy(webex.internal.newMetrics.callDiagnosticLatencies, 'measureLatency'); + }); + + afterEach(() => { + sinon.restore(); + }); + + it('checks service request resolves', async () => { + const mapResponse = 'map response'; + + sinon.stub(services, '_formatReceivedHostmap').resolves(mapResponse); + sinon.stub(services, 'request').resolves({}); + + const mapResult = await services._fetchNewServiceHostmap({from: 'limited'}); + + assert.deepEqual(mapResult, mapResponse); + + assert.calledOnceWithExactly(services.request, { + method: 'GET', + service: 'u2c', + resource: '/limited/catalog', + qs: {format: 'hostmap'} + } + ); + assert.calledOnceWithExactly(webex.internal.newMetrics.callDiagnosticLatencies.measureLatency, sinon.match.func, 'internal.get.u2c.time'); + }); + + it('checks service request rejects', async () => { + const error = new Error('some error'); + + sinon.spy(services, '_formatReceivedHostmap'); + sinon.stub(services, 'request').rejects(error); + + const promise = services._fetchNewServiceHostmap({from: 'limited'}); + const rejectedValue = await assert.isRejected(promise); + + assert.deepEqual(rejectedValue, error); + + assert.notCalled(services._formatReceivedHostmap); + + assert.calledOnceWithExactly(services.request, { + method: 'GET', + service: 'u2c', + resource: '/limited/catalog', + qs: {format: 'hostmap'} + } + ); + assert.calledOnceWithExactly(webex.internal.newMetrics.callDiagnosticLatencies.measureLatency, sinon.match.func, 'internal.get.u2c.time'); + }); + }); + describe('#_formatReceivedHostmap()', () => { let serviceHostmap; let formattedHM; From d72597add58397245910392dee5181716d6a8677 Mon Sep 17 00:00:00 2001 From: Marcin Date: Mon, 8 Apr 2024 15:38:18 +0100 Subject: [PATCH 11/20] fix: Roap optimizations phase 2 (#3449) --- .../@webex/plugin-meetings/src/constants.ts | 2 +- .../plugin-meetings/src/meeting/index.ts | 191 +++++++---- .../plugin-meetings/src/meeting/request.ts | 13 +- .../plugin-meetings/src/meeting/util.ts | 1 + .../@webex/plugin-meetings/src/roap/index.ts | 28 +- .../plugin-meetings/src/roap/turnDiscovery.ts | 322 +++++++++++++----- .../test/unit/spec/meeting/index.js | 178 ++++++++-- .../test/unit/spec/meeting/request.js | 3 + .../test/unit/spec/meeting/utils.js | 7 +- .../test/unit/spec/roap/index.ts | 67 +++- .../test/unit/spec/roap/turnDiscovery.ts | 314 ++++++++++++++++- 11 files changed, 939 insertions(+), 187 deletions(-) diff --git a/packages/@webex/plugin-meetings/src/constants.ts b/packages/@webex/plugin-meetings/src/constants.ts index f407928605f..898104a542b 100644 --- a/packages/@webex/plugin-meetings/src/constants.ts +++ b/packages/@webex/plugin-meetings/src/constants.ts @@ -1,7 +1,7 @@ // @ts-ignore import {hydraTypes} from '@webex/common'; -type Enum> = T[keyof T]; +export type Enum> = T[keyof T]; // *********** LOWERCASE / CAMELCASE STRINGS ************ diff --git a/packages/@webex/plugin-meetings/src/meeting/index.ts b/packages/@webex/plugin-meetings/src/meeting/index.ts index 74b6f690be3..a95880a1d21 100644 --- a/packages/@webex/plugin-meetings/src/meeting/index.ts +++ b/packages/@webex/plugin-meetings/src/meeting/index.ts @@ -51,7 +51,11 @@ import {StatsAnalyzer, EVENTS as StatsAnalyzerEvents} from '../statsAnalyzer'; import NetworkQualityMonitor from '../networkQualityMonitor'; import LoggerProxy from '../common/logs/logger-proxy'; import Trigger from '../common/events/trigger-proxy'; -import Roap from '../roap/index'; +import Roap, { + type TurnDiscoveryResult, + type TurnServerInfo, + type TurnDiscoverySkipReason, +} from '../roap/index'; import Media, {type BundlePolicy} from '../media'; import MediaProperties from '../media/properties'; import MeetingStateMachine from './state'; @@ -621,7 +625,7 @@ export default class Meeting extends StatelessWebexPlugin { allowMediaInLobby: boolean; localShareInstanceId: string; remoteShareInstanceId: string; - turnDiscoverySkippedReason: string; + turnDiscoverySkippedReason: TurnDiscoverySkipReason; turnServerUsed: boolean; areVoiceaEventsSetup = false; voiceaListenerCallbacks: object = { @@ -4445,47 +4449,90 @@ export default class Meeting extends StatelessWebexPlugin { * } * }) */ - public joinWithMedia( + public async joinWithMedia( options: { joinOptions?: any; mediaOptions?: AddMediaOptions; } = {} ) { - const {mediaOptions, joinOptions} = options; + const {mediaOptions, joinOptions = {}} = options; if (!mediaOptions?.allowMediaInLobby) { return Promise.reject( new ParameterError('joinWithMedia() can only be used with allowMediaInLobby set to true') ); } + this.allowMediaInLobby = true; LoggerProxy.logger.info('Meeting:index#joinWithMedia called'); - return this.join(joinOptions) - .then((joinResponse) => - this.addMedia(mediaOptions).then((mediaResponse) => ({ - join: joinResponse, - media: mediaResponse, - })) - ) - .catch((error) => { - LoggerProxy.logger.error('Meeting:index#joinWithMedia --> ', error); + let joined = false; - Metrics.sendBehavioralMetric( - BEHAVIORAL_METRICS.JOIN_WITH_MEDIA_FAILURE, - { - correlation_id: this.correlationId, - locus_id: this.locusUrl.split('/').pop(), - reason: error.message, - stack: error.stack, - }, - { - type: error.name, - } - ); + try { + let turnServerInfo; + let turnDiscoverySkippedReason; - return Promise.reject(error); - }); + // @ts-ignore + joinOptions.reachability = await this.webex.meetings.reachability.getReachabilityResults(); + const turnDiscoveryRequest = await this.roap.generateTurnDiscoveryRequestMessage(this, true); + + ({turnDiscoverySkippedReason} = turnDiscoveryRequest); + joinOptions.roapMessage = turnDiscoveryRequest.roapMessage; + + const joinResponse = await this.join(joinOptions); + + joined = true; + + if (joinOptions.roapMessage) { + ({turnServerInfo, turnDiscoverySkippedReason} = + await this.roap.handleTurnDiscoveryHttpResponse(this, joinResponse)); + + this.turnDiscoverySkippedReason = turnDiscoverySkippedReason; + this.turnServerUsed = !!turnServerInfo; + + if (turnServerInfo === undefined) { + this.roap.abortTurnDiscovery(); + } + } + + const mediaResponse = await this.addMedia(mediaOptions, turnServerInfo); + + return { + join: joinResponse, + media: mediaResponse, + }; + } catch (error) { + LoggerProxy.logger.error('Meeting:index#joinWithMedia --> ', error); + + let leaveError; + + this.roap.abortTurnDiscovery(); + + if (joined) { + try { + await this.leave({resourceId: joinOptions?.resourceId, reason: 'joinWithMedia failure'}); + } catch (e) { + LoggerProxy.logger.error('Meeting:index#joinWithMedia --> leave error', e); + leaveError = e; + } + } + + Metrics.sendBehavioralMetric( + BEHAVIORAL_METRICS.JOIN_WITH_MEDIA_FAILURE, + { + correlation_id: this.correlationId, + locus_id: this.locusUrl?.split('/').pop(), // if join fails, we may end up with no locusUrl + reason: error.message, + stack: error.stack, + leaveErrorReason: leaveError?.message, + }, + { + type: error.name, + } + ); + + throw error; + } } /** @@ -6328,6 +6375,44 @@ export default class Meeting extends StatelessWebexPlugin { } } + /** + * Performs TURN discovery as a separate call to the Locus /media API + * + * @param {boolean} isRetry + * @param {boolean} isForced + * @returns {Promise} + */ + private async doTurnDiscovery(isRetry: boolean, isForced: boolean): Promise { + // @ts-ignore + const cdl = this.webex.internal.newMetrics.callDiagnosticLatencies; + + // @ts-ignore + this.webex.internal.newMetrics.submitInternalEvent({ + name: 'internal.client.add-media.turn-discovery.start', + }); + + const turnDiscoveryResult = await this.roap.doTurnDiscovery(this, isRetry, isForced); + + this.turnDiscoverySkippedReason = turnDiscoveryResult?.turnDiscoverySkippedReason; + this.turnServerUsed = !this.turnDiscoverySkippedReason; + + // @ts-ignore + this.webex.internal.newMetrics.submitInternalEvent({ + name: 'internal.client.add-media.turn-discovery.end', + }); + + if (this.turnServerUsed && turnDiscoveryResult.turnServerInfo) { + Metrics.sendBehavioralMetric(BEHAVIORAL_METRICS.TURN_DISCOVERY_LATENCY, { + correlation_id: this.correlationId, + latency: cdl.getTurnDiscoveryTime(), + turnServerUsed: this.turnServerUsed, + retriedWithTurnServer: this.retriedWithTurnServer, + }); + } + + return turnDiscoveryResult; + } + /** * Does TURN discovery, SDP offer/answer exhange, establishes ICE connection and DTLS handshake. * @@ -6335,43 +6420,21 @@ export default class Meeting extends StatelessWebexPlugin { * @param {RemoteMediaManagerConfiguration} [remoteMediaManagerConfig] * @param {BundlePolicy} [bundlePolicy] * @param {boolean} [isForced] - let isForced be true to do turn discovery regardless of reachability results + * @param {TurnServerInfo} [turnServerInfo] * @returns {Promise} */ private async establishMediaConnection( remoteMediaManagerConfig?: RemoteMediaManagerConfiguration, bundlePolicy?: BundlePolicy, - isForced?: boolean + isForced?: boolean, + turnServerInfo?: TurnServerInfo ): Promise { const LOG_HEADER = 'Meeting:index#addMedia():establishMediaConnection -->'; - // @ts-ignore - const cdl = this.webex.internal.newMetrics.callDiagnosticLatencies; const isRetry = this.retriedWithTurnServer; try { - // @ts-ignore - this.webex.internal.newMetrics.submitInternalEvent({ - name: 'internal.client.add-media.turn-discovery.start', - }); - - const turnDiscoveryObject = await this.roap.doTurnDiscovery(this, isRetry, isForced); - - this.turnDiscoverySkippedReason = turnDiscoveryObject?.turnDiscoverySkippedReason; - this.turnServerUsed = !this.turnDiscoverySkippedReason; - - // @ts-ignore - this.webex.internal.newMetrics.submitInternalEvent({ - name: 'internal.client.add-media.turn-discovery.end', - }); - - const {turnServerInfo} = turnDiscoveryObject; - - if (this.turnServerUsed && turnServerInfo) { - Metrics.sendBehavioralMetric(BEHAVIORAL_METRICS.TURN_DISCOVERY_LATENCY, { - correlation_id: this.correlationId, - latency: cdl.getTurnDiscoveryTime(), - turnServerUsed: this.turnServerUsed, - retriedWithTurnServer: this.retriedWithTurnServer, - }); + if (!turnServerInfo) { + ({turnServerInfo} = await this.doTurnDiscovery(isRetry, isForced)); } const mc = await this.createMediaConnection(turnServerInfo, bundlePolicy); @@ -6488,15 +6551,21 @@ export default class Meeting extends StatelessWebexPlugin { * Creates a media connection to the server. Media connection is required for sending or receiving any audio/video. * * @param {AddMediaOptions} options + * @param {TurnServerInfo} turnServerInfo - TURN server information (used only internally by the SDK) * @returns {Promise} * @public * @memberof Meeting */ - async addMedia(options: AddMediaOptions = {}): Promise { + async addMedia( + options: AddMediaOptions = {}, + turnServerInfo: TurnServerInfo = undefined + ): Promise { this.retriedWithTurnServer = false; this.hasMediaConnectionConnectedAtLeastOnce = false; const LOG_HEADER = 'Meeting:index#addMedia -->'; - LoggerProxy.logger.info(`${LOG_HEADER} called with: ${JSON.stringify(options)}`); + LoggerProxy.logger.info( + `${LOG_HEADER} called with: ${JSON.stringify(options)}, ${JSON.stringify(turnServerInfo)}` + ); if (options.allowMediaInLobby !== true && this.meetingState !== FULL_STATE.ACTIVE) { throw new MeetingNotActiveError(); @@ -6514,14 +6583,13 @@ export default class Meeting extends StatelessWebexPlugin { shareVideoEnabled = true, remoteMediaManagerConfig, bundlePolicy, - allowMediaInLobby, } = options; this.allowMediaInLobby = options?.allowMediaInLobby; // If the user is unjoined or guest waiting in lobby dont allow the user to addMedia // @ts-ignore - isUserUnadmitted coming from SelfUtil - if (this.isUserUnadmitted && !this.wirelessShare && !allowMediaInLobby) { + if (this.isUserUnadmitted && !this.wirelessShare && !this.allowMediaInLobby) { throw new UserInLobbyError(); } @@ -6590,7 +6658,12 @@ export default class Meeting extends StatelessWebexPlugin { this.createStatsAnalyzer(); - await this.establishMediaConnection(remoteMediaManagerConfig, bundlePolicy, false); + await this.establishMediaConnection( + remoteMediaManagerConfig, + bundlePolicy, + false, + turnServerInfo + ); await Meeting.handleDeviceLogging(); diff --git a/packages/@webex/plugin-meetings/src/meeting/request.ts b/packages/@webex/plugin-meetings/src/meeting/request.ts index 3dce1e5321e..6c085e7eec6 100644 --- a/packages/@webex/plugin-meetings/src/meeting/request.ts +++ b/packages/@webex/plugin-meetings/src/meeting/request.ts @@ -122,6 +122,7 @@ export default class MeetingRequest extends StatelessWebexPlugin { meetingNumber: any; permissionToken: any; preferTranscoding: any; + reachability: any; breakoutsSupported: boolean; locale?: string; deviceCapabilities?: Array; @@ -143,6 +144,7 @@ export default class MeetingRequest extends StatelessWebexPlugin { pin, moveToResource, roapMessage, + reachability, preferTranscoding, breakoutsSupported, locale, @@ -260,8 +262,15 @@ export default class MeetingRequest extends StatelessWebexPlugin { }; } - if (roapMessage) { - body.localMedias = roapMessage.localMedias; + if (roapMessage || reachability) { + body.localMedias = [ + { + localSdp: JSON.stringify({ + roapMessage, + reachability, + }), + }, + ]; } /// @ts-ignore diff --git a/packages/@webex/plugin-meetings/src/meeting/util.ts b/packages/@webex/plugin-meetings/src/meeting/util.ts index e09146bf04f..3ac685a80e2 100644 --- a/packages/@webex/plugin-meetings/src/meeting/util.ts +++ b/packages/@webex/plugin-meetings/src/meeting/util.ts @@ -149,6 +149,7 @@ const MeetingUtil = { locusUrl: meeting.locusUrl, locusClusterUrl: meeting.meetingInfo?.locusClusterUrl, correlationId: meeting.correlationId, + reachability: options.reachability, roapMessage: options.roapMessage, permissionToken: meeting.permissionToken, resourceId: options.resourceId || null, diff --git a/packages/@webex/plugin-meetings/src/roap/index.ts b/packages/@webex/plugin-meetings/src/roap/index.ts index 3462a9cf0b7..5761db15d5f 100644 --- a/packages/@webex/plugin-meetings/src/roap/index.ts +++ b/packages/@webex/plugin-meetings/src/roap/index.ts @@ -5,12 +5,18 @@ import {ROAP} from '../constants'; import LoggerProxy from '../common/logs/logger-proxy'; import RoapRequest from './request'; -import TurnDiscovery from './turnDiscovery'; +import TurnDiscovery, {TurnDiscoveryResult} from './turnDiscovery'; import Meeting from '../meeting'; import MeetingUtil from '../meeting/util'; import Metrics from '../metrics'; import BEHAVIORAL_METRICS from '../metrics/constants'; +export { + type TurnDiscoveryResult, + type TurnServerInfo, + type TurnDiscoverySkipReason, +} from './turnDiscovery'; + /** * Roap options * @typedef {Object} RoapOptions @@ -39,7 +45,7 @@ export default class Roap extends StatelessWebexPlugin { options: any; roapHandler: any; roapRequest: any; - turnDiscovery: any; + turnDiscovery: TurnDiscovery; /** * @@ -260,7 +266,23 @@ export default class Roap extends StatelessWebexPlugin { * @param {Boolean} [isForced] * @returns {Promise} */ - doTurnDiscovery(meeting: Meeting, isReconnecting: boolean, isForced?: boolean) { + doTurnDiscovery( + meeting: Meeting, + isReconnecting: boolean, + isForced?: boolean + ): Promise { return this.turnDiscovery.doTurnDiscovery(meeting, isReconnecting, isForced); } + + generateTurnDiscoveryRequestMessage(meeting: Meeting, isForced: boolean) { + return this.turnDiscovery.generateTurnDiscoveryRequestMessage(meeting, isForced); + } + + handleTurnDiscoveryHttpResponse(meeting: Meeting, httpResponse: object) { + return this.turnDiscovery.handleTurnDiscoveryHttpResponse(meeting, httpResponse); + } + + abortTurnDiscovery() { + return this.turnDiscovery.abort(); + } } diff --git a/packages/@webex/plugin-meetings/src/roap/turnDiscovery.ts b/packages/@webex/plugin-meetings/src/roap/turnDiscovery.ts index a0c5809e5f4..1c732e93052 100644 --- a/packages/@webex/plugin-meetings/src/roap/turnDiscovery.ts +++ b/packages/@webex/plugin-meetings/src/roap/turnDiscovery.ts @@ -4,7 +4,7 @@ import {Defer} from '@webex/common'; import Metrics from '../metrics'; import BEHAVIORAL_METRICS from '../metrics/constants'; import LoggerProxy from '../common/logs/logger-proxy'; -import {ROAP} from '../constants'; +import {ROAP, Enum} from '../constants'; import RoapRequest from './request'; import Meeting from '../meeting'; @@ -18,6 +18,28 @@ const TURN_DISCOVERY_TIMEOUT = 10; // in seconds // and do the SDP offer with seq=1 const TURN_DISCOVERY_SEQ = 0; +const TurnDiscoverySkipReason = { + missingHttpResponse: 'missing http response', // when we asked for the TURN discovery response to be in the http response, but it wasn't there + reachability: 'reachability', // when udp reachability to public clusters is ok, so we don't need TURN (this doens't apply when joinWithMedia() is used) + alreadyInProgress: 'already in progress', // when we try to start TURN discovery while it's already in progress +} as const; + +export type TurnDiscoverySkipReason = + | Enum // this is a kind of FYI, because in practice typescript will infer the type of TurnDiscoverySkipReason as a string + | string // used in case of errors, contains the error message + | undefined; // used when TURN discovery is not skipped + +export type TurnServerInfo = { + url: string; + username: string; + password: string; +}; + +export type TurnDiscoveryResult = { + turnServerInfo?: TurnServerInfo; + turnDiscoverySkippedReason: TurnDiscoverySkipReason; +}; + /** * Handles the process of finding out TURN server information from Linus. * This is achieved by sending a TURN_DISCOVERY_REQUEST. @@ -27,11 +49,7 @@ export default class TurnDiscovery { private defer?: Defer; // used for waiting for the response - private turnInfo: { - url: string; - username: string; - password: string; - }; + private turnInfo: TurnServerInfo; private responseTimer?: ReturnType; @@ -85,7 +103,8 @@ export default class TurnDiscovery { } /** - * handles TURN_DISCOVERY_RESPONSE roap message + * Handles TURN_DISCOVERY_RESPONSE roap message. Use it if the roap message comes over the websocket, + * otherwise use handleTurnDiscoveryHttpResponse() if it comes in the http response. * * @param {Object} roapMessage * @param {string} from string to indicate how we got the response (used just for logging) @@ -158,18 +177,191 @@ export default class TurnDiscovery { } /** - * handles TURN_DISCOVERY_RESPONSE roap message that came in http response + * Generates TURN_DISCOVERY_REQUEST roap message. When this method returns a roapMessage, it means that a TURN discovery process has started. + * It needs be ended by calling handleTurnDiscoveryHttpResponse() once you get a response from the backend. If you don't get any response + * or want to abort, you need to call abort(). * - * @param {Object} roapMessage - * @returns {Promise} + * @param {Meeting} meeting + * @param {boolean} isForced + * @returns {Object} + */ + public async generateTurnDiscoveryRequestMessage( + meeting: Meeting, + isForced: boolean + ): Promise<{roapMessage?: object; turnDiscoverySkippedReason: TurnDiscoverySkipReason}> { + if (this.defer) { + LoggerProxy.logger.warn( + 'Roap:turnDiscovery#generateTurnDiscoveryRequestMessage --> TURN discovery already in progress' + ); + + return { + roapMessage: undefined, + turnDiscoverySkippedReason: TurnDiscoverySkipReason.alreadyInProgress, + }; + } + + let turnDiscoverySkippedReason: TurnDiscoverySkipReason; + + if (!isForced) { + turnDiscoverySkippedReason = await this.getSkipReason(meeting); + } + + if (turnDiscoverySkippedReason) { + return {roapMessage: undefined, turnDiscoverySkippedReason}; + } + + this.defer = new Defer(); + + const roapMessage = { + messageType: ROAP.ROAP_TYPES.TURN_DISCOVERY_REQUEST, + version: ROAP.ROAP_VERSION, + seq: TURN_DISCOVERY_SEQ, + headers: ['includeAnswerInHttpResponse', 'noOkInTransaction'], + }; + + LoggerProxy.logger.info( + 'Roap:turnDiscovery#generateTurnDiscoveryRequestMessage --> generated TURN_DISCOVERY_REQUEST message' + ); + + return {roapMessage, turnDiscoverySkippedReason: undefined}; + } + + /** + * Handles any errors that occur during TURN discovery without re-throwing them. + * + * @param {Meeting} meeting + * @param {Error} error + * @returns {TurnDiscoveryResult} + */ + private handleTurnDiscoveryFailure(meeting: Meeting, error: Error): TurnDiscoveryResult { + // we catch any errors and resolve with no turn information so that the normal call join flow can continue without TURN + LoggerProxy.logger.info( + `Roap:turnDiscovery#doTurnDiscovery --> TURN discovery failed, continuing without TURN: ${error}` + ); + + Metrics.sendBehavioralMetric(BEHAVIORAL_METRICS.TURN_DISCOVERY_FAILURE, { + correlation_id: meeting.correlationId, + locus_id: meeting.locusUrl.split('/').pop(), + reason: error.message, + stack: error.stack, + }); + + return {turnServerInfo: undefined, turnDiscoverySkippedReason: `failure: ${error.message}`}; + } + + /** + * Handles TURN_DISCOVERY_RESPONSE roap message that came in http response. If the response is not valid, + * it returns an object with turnServerInfo set to undefined. In that case you need to call abort() + * to end the TURN discovery process. + * + * @param {Meeting} meeting + * @param {Object|undefined} httpResponse can be undefined to indicate that we didn't get the response + * @returns {Promise} * @memberof Roap */ - private async handleTurnDiscoveryResponseInHttpResponse( - roapMessage: object - ): Promise<{isOkRequired: boolean}> { - this.handleTurnDiscoveryResponse(roapMessage, 'in http response'); + public async handleTurnDiscoveryHttpResponse( + meeting: Meeting, + httpResponse?: object + ): Promise { + if (!this.defer) { + LoggerProxy.logger.warn( + 'Roap:turnDiscovery#handleTurnDiscoveryHttpResponse --> unexpected http response, TURN discovery is not in progress' + ); - return this.defer.promise; + throw new Error( + 'handleTurnDiscoveryHttpResponse() called before generateTurnDiscoveryRequestMessage()' + ); + } + + if (httpResponse === undefined) { + return { + turnServerInfo: undefined, + turnDiscoverySkippedReason: TurnDiscoverySkipReason.missingHttpResponse, + }; + } + + try { + const roapMessage = this.parseHttpTurnDiscoveryResponse(meeting, httpResponse); + + if (!roapMessage) { + return { + turnServerInfo: undefined, + turnDiscoverySkippedReason: TurnDiscoverySkipReason.missingHttpResponse, + }; + } + + this.handleTurnDiscoveryResponse(roapMessage, 'in http response'); + + const {isOkRequired} = await this.defer.promise; + + if (isOkRequired) { + await this.sendRoapOK(meeting); + } + + this.defer = undefined; + + LoggerProxy.logger.info('Roap:turnDiscovery#doTurnDiscovery --> TURN discovery completed'); + + return {turnServerInfo: this.turnInfo, turnDiscoverySkippedReason: undefined}; + } catch (error) { + this.abort(); + + return this.handleTurnDiscoveryFailure(meeting, error); + } + } + + /** + * Aborts current TURN discovery. This method needs to be called if you called generateTurnDiscoveryRequestMessage(), + * but then never got any response from the server. + * @returns {void} + */ + public abort() { + if (this.defer) { + this.defer.reject(new Error('TURN discovery aborted')); + this.defer = undefined; + } + } + + /** + * Parses the TURN_DISCOVERY_RESPONSE roap message out of the http response + * and returns it. + * + * @param {Meeting} meeting + * @param {any} httpResponse + * @returns {any} + */ + private parseHttpTurnDiscoveryResponse( + meeting: Meeting, + httpResponse: {mediaConnections?: Array<{remoteSdp?: string}>} + ) { + let turnDiscoveryResponse; + + if (httpResponse.mediaConnections?.[0]?.remoteSdp) { + const remoteSdp = JSON.parse(httpResponse.mediaConnections[0].remoteSdp); + + if (remoteSdp.roapMessage) { + // yes, it's misleading that remoteSdp actually contains a TURN discovery response, but that's how the backend works... + const {seq, messageType, errorType, errorCause, headers} = remoteSdp.roapMessage; + + turnDiscoveryResponse = { + seq, + messageType, + errorType, + errorCause, + headers, + }; + } + } + + if (!turnDiscoveryResponse) { + Metrics.sendBehavioralMetric(BEHAVIORAL_METRICS.ROAP_HTTP_RESPONSE_MISSING, { + correlationId: meeting.correlationId, + messageType: 'TURN_DISCOVERY_RESPONSE', + isMultistream: meeting.isMultistream, + }); + } + + return turnDiscoveryResponse; } /** @@ -181,13 +373,19 @@ export default class TurnDiscovery { * @private * @memberof Roap */ - sendRoapTurnDiscoveryRequest(meeting: Meeting, isReconnecting: boolean) { + private sendRoapTurnDiscoveryRequest( + meeting: Meeting, + isReconnecting: boolean + ): Promise { if (this.defer) { LoggerProxy.logger.warn( 'Roap:turnDiscovery#sendRoapTurnDiscoveryRequest --> already in progress' ); - return Promise.resolve(); + return Promise.resolve({ + turnServerInfo: undefined, + turnDiscoverySkippedReason: TurnDiscoverySkipReason.alreadyInProgress, + }); } this.defer = new Defer(); @@ -215,41 +413,14 @@ export default class TurnDiscovery { // @ts-ignore - because of meeting.webex ipVersion: MeetingUtil.getIpVersion(meeting.webex), }) - .then((response) => { + .then(async (response) => { const {mediaConnections} = response; - let turnDiscoveryResponse; - if (mediaConnections) { meeting.updateMediaConnections(mediaConnections); - - if (mediaConnections[0]?.remoteSdp) { - const remoteSdp = JSON.parse(mediaConnections[0].remoteSdp); - - if (remoteSdp.roapMessage) { - // yes, it's misleading that remoteSdp actually contains a TURN discovery response, but that's how the backend works... - const {seq, messageType, errorType, errorCause, headers} = remoteSdp.roapMessage; - - turnDiscoveryResponse = { - seq, - messageType, - errorType, - errorCause, - headers, - }; - } - } } - if (!turnDiscoveryResponse) { - Metrics.sendBehavioralMetric(BEHAVIORAL_METRICS.ROAP_HTTP_RESPONSE_MISSING, { - correlationId: meeting.correlationId, - messageType: 'TURN_DISCOVERY_RESPONSE', - isMultistream: meeting.isMultistream, - }); - } - - return turnDiscoveryResponse; + return this.handleTurnDiscoveryHttpResponse(meeting, response); }); } @@ -261,7 +432,14 @@ export default class TurnDiscovery { * @returns {Promise} */ sendRoapOK(meeting: Meeting) { - LoggerProxy.logger.info('Roap:turnDiscovery#sendRoapOK --> sending OK'); + LoggerProxy.logger.info( + 'Roap:turnDiscovery#sendRoapOK --> TURN discovery response requires OK, sending it...' + ); + + Metrics.sendBehavioralMetric(BEHAVIORAL_METRICS.TURN_DISCOVERY_REQUIRES_OK, { + correlation_id: meeting.correlationId, + locus_id: meeting.locusUrl.split('/').pop(), + }); return this.roapRequest.sendRoap({ roapMessage: { @@ -284,7 +462,7 @@ export default class TurnDiscovery { * @param {Meeting} meeting * @returns {Promise} Promise with empty string if reachability is not skipped or a reason if it is skipped */ - private async getSkipReason(meeting: Meeting): Promise { + private async getSkipReason(meeting: Meeting): Promise { const isAnyPublicClusterReachable = // @ts-ignore - fix type await meeting.webex.meetings.reachability.isAnyPublicClusterReachable(); @@ -294,10 +472,10 @@ export default class TurnDiscovery { 'Roap:turnDiscovery#getSkipReason --> reachability has not failed, skipping TURN discovery' ); - return 'reachability'; + return TurnDiscoverySkipReason.reachability; } - return ''; + return undefined; } /** @@ -330,8 +508,12 @@ export default class TurnDiscovery { * @param {Boolean} [isForced] * @returns {Promise} */ - async doTurnDiscovery(meeting: Meeting, isReconnecting?: boolean, isForced?: boolean) { - let turnDiscoverySkippedReason: string; + async doTurnDiscovery( + meeting: Meeting, + isReconnecting?: boolean, + isForced?: boolean + ): Promise { + let turnDiscoverySkippedReason: TurnDiscoverySkipReason; if (!isForced) { turnDiscoverySkippedReason = await this.getSkipReason(meeting); @@ -345,24 +527,20 @@ export default class TurnDiscovery { } try { - const httpResponse = await this.sendRoapTurnDiscoveryRequest(meeting, isReconnecting); + const turnDiscoveryResult = await this.sendRoapTurnDiscoveryRequest(meeting, isReconnecting); + + if ( + turnDiscoveryResult.turnDiscoverySkippedReason !== + TurnDiscoverySkipReason.missingHttpResponse + ) { + return turnDiscoveryResult; + } // if we haven't got the response over http, we need to wait for it to come over the websocket via Mercury - const {isOkRequired} = httpResponse - ? await this.handleTurnDiscoveryResponseInHttpResponse(httpResponse) - : await this.waitForTurnDiscoveryResponse(); + const {isOkRequired} = await this.waitForTurnDiscoveryResponse(); if (isOkRequired) { await this.sendRoapOK(meeting); - - LoggerProxy.logger.info( - 'Roap:turnDiscovery#doTurnDiscovery --> TURN discovery response requires OK' - ); - - Metrics.sendBehavioralMetric(BEHAVIORAL_METRICS.TURN_DISCOVERY_REQUIRES_OK, { - correlation_id: meeting.correlationId, - locus_id: meeting.locusUrl.split('/').pop(), - }); } this.defer = undefined; @@ -371,19 +549,7 @@ export default class TurnDiscovery { return {turnServerInfo: this.turnInfo, turnDiscoverySkippedReason: undefined}; } catch (e) { - // we catch any errors and resolve with no turn information so that the normal call join flow can continue without TURN - LoggerProxy.logger.info( - `Roap:turnDiscovery#doTurnDiscovery --> TURN discovery failed, continuing without TURN: ${e}` - ); - - Metrics.sendBehavioralMetric(BEHAVIORAL_METRICS.TURN_DISCOVERY_FAILURE, { - correlation_id: meeting.correlationId, - locus_id: meeting.locusUrl.split('/').pop(), - reason: e.message, - stack: e.stack, - }); - - return {turnServerInfo: undefined, turnDiscoverySkippedReason: undefined}; + return this.handleTurnDiscoveryFailure(meeting, e); } } } diff --git a/packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js b/packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js index 7bbeddfe05f..dc7f59f1485 100644 --- a/packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js +++ b/packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js @@ -613,36 +613,172 @@ describe('plugin-meetings', () => { assert.exists(meeting.joinWithMedia); }); - describe('resolution', () => { - it('should success and return a promise', async () => { - meeting.join = sinon.stub().returns(Promise.resolve(test1)); - meeting.addMedia = sinon.stub().returns(Promise.resolve(test4)); + const fakeRoapMessage = {id: 'fake TURN discovery message'}; + const fakeReachabilityResults = {id: 'fake reachability'}; + const fakeTurnServerInfo = {id: 'fake turn info'}; + const fakeJoinResult = {id: 'join result'}; - const joinOptions = {correlationId: '12345'}; - const mediaOptions = {audioEnabled: test1, allowMediaInLobby: true}; + const joinOptions = {correlationId: '12345'}; + const mediaOptions = {audioEnabled: true, allowMediaInLobby: true}; - const result = await meeting.joinWithMedia({ - joinOptions, - mediaOptions, - }); - assert.calledOnceWithExactly(meeting.join, joinOptions); - assert.calledOnceWithExactly(meeting.addMedia, mediaOptions); - assert.deepEqual(result, {join: test1, media: test4}); + let generateTurnDiscoveryRequestMessageStub; + let handleTurnDiscoveryHttpResponseStub; + let abortTurnDiscoveryStub; + + beforeEach(() => { + meeting.join = sinon.stub().returns(Promise.resolve(fakeJoinResult)); + meeting.addMedia = sinon.stub().returns(Promise.resolve(test4)); + + webex.meetings.reachability.getReachabilityResults.resolves(fakeReachabilityResults); + + generateTurnDiscoveryRequestMessageStub = sinon + .stub(meeting.roap, 'generateTurnDiscoveryRequestMessage') + .resolves({roapMessage: fakeRoapMessage}); + handleTurnDiscoveryHttpResponseStub = sinon + .stub(meeting.roap, 'handleTurnDiscoveryHttpResponse') + .resolves({turnServerInfo: fakeTurnServerInfo, turnDiscoverySkippedReason: undefined}); + abortTurnDiscoveryStub = sinon.stub(meeting.roap, 'abortTurnDiscovery'); + }); + + it('should work as expected', async () => { + const result = await meeting.joinWithMedia({ + joinOptions, + mediaOptions, }); + + // check that TURN discovery is done with join and addMedia called + assert.calledOnceWithExactly(meeting.join, { + ...joinOptions, + roapMessage: fakeRoapMessage, + reachability: fakeReachabilityResults, + }); + assert.calledOnceWithExactly(generateTurnDiscoveryRequestMessageStub, meeting, true); + assert.calledOnceWithExactly( + handleTurnDiscoveryHttpResponseStub, + meeting, + fakeJoinResult + ); + assert.calledOnceWithExactly(meeting.addMedia, mediaOptions, fakeTurnServerInfo); + + assert.deepEqual(result, {join: fakeJoinResult, media: test4}); }); - describe('rejection', () => { - it('should error out and return a promise', async () => { - meeting.join = sinon.stub().returns(Promise.reject()); - assert.isRejected(meeting.joinWithMedia({mediaOptions: {allowMediaInLobby: true}})); + it("should not call handleTurnDiscoveryHttpResponse if we don't send a TURN discovery request with join", async () => { + generateTurnDiscoveryRequestMessageStub.resolves({roapMessage: undefined}); + + const result = await meeting.joinWithMedia({ + joinOptions, + mediaOptions, + }); + + // check that TURN discovery is done with join and addMedia called + assert.calledOnceWithExactly(meeting.join, { + ...joinOptions, + roapMessage: undefined, + reachability: fakeReachabilityResults, }); + assert.calledOnceWithExactly(generateTurnDiscoveryRequestMessageStub, meeting, true); + assert.notCalled(handleTurnDiscoveryHttpResponseStub); + assert.notCalled(abortTurnDiscoveryStub); + assert.calledOnceWithExactly(meeting.addMedia, mediaOptions, undefined); + + assert.deepEqual(result, {join: fakeJoinResult, media: test4}); + assert.equal(meeting.turnServerUsed, false); + }); + + it('should call abortTurnDiscovery() if we do not get a TURN server info', async () => { + handleTurnDiscoveryHttpResponseStub.resolves({turnServerInfo: undefined, turnDiscoverySkippedReason: 'missing http response'}); - it('should fail if called with allowMediaInLobby:false', async () => { - meeting.join = sinon.stub().returns(Promise.resolve(test1)); - meeting.addMedia = sinon.stub().returns(Promise.resolve(test4)); + const result = await meeting.joinWithMedia({ + joinOptions, + mediaOptions, + }); - assert.isRejected(meeting.joinWithMedia({mediaOptions: {allowMediaInLobby: false}})); + // check that TURN discovery is done with join and addMedia called + assert.calledOnceWithExactly(meeting.join, { + ...joinOptions, + roapMessage: fakeRoapMessage, + reachability: fakeReachabilityResults, }); + assert.calledOnceWithExactly(generateTurnDiscoveryRequestMessageStub, meeting, true); + assert.calledOnceWithExactly( + handleTurnDiscoveryHttpResponseStub, + meeting, + fakeJoinResult + ); + assert.calledOnceWithExactly(abortTurnDiscoveryStub); + assert.calledOnceWithExactly(meeting.addMedia, mediaOptions, undefined); + + assert.deepEqual(result, {join: fakeJoinResult, media: test4}); + }); + + it('should reject if join() fails', async () => { + const error = new Error('fake'); + meeting.join = sinon.stub().returns(Promise.reject(error)); + meeting.locusUrl = null; // when join fails, we end up with null locusUrl + + await assert.isRejected(meeting.joinWithMedia({mediaOptions: {allowMediaInLobby: true}})); + + assert.calledOnceWithExactly(abortTurnDiscoveryStub); + + assert.calledWith(Metrics.sendBehavioralMetric, + BEHAVIORAL_METRICS.JOIN_WITH_MEDIA_FAILURE, + { + correlation_id: meeting.correlationId, + locus_id: undefined, + reason: error.message, + stack: error.stack, + leaveErrorReason: undefined, + }, + { + type: error.name, + } + ); + }); + + it('should fail if called with allowMediaInLobby:false', async () => { + meeting.join = sinon.stub().returns(Promise.resolve(test1)); + meeting.addMedia = sinon.stub().returns(Promise.resolve(test4)); + + await assert.isRejected( + meeting.joinWithMedia({mediaOptions: {allowMediaInLobby: false}}) + ); + }); + + it('should call leave() if addMedia fails and ignore leave() failure', async () => { + const leaveError = new Error('leave error'); + const addMediaError = new Error('fake addMedia error'); + + const leaveStub = sinon.stub(meeting, 'leave').rejects(leaveError); + meeting.addMedia = sinon.stub().rejects(addMediaError); + + await assert.isRejected( + meeting.joinWithMedia({ + joinOptions: {resourceId: 'some resource'}, + mediaOptions: {allowMediaInLobby: true}, + }), + addMediaError + ); + + assert.calledOnce(leaveStub); + assert.calledOnceWithExactly(leaveStub, { + resourceId: 'some resource', + reason: 'joinWithMedia failure', + }); + + assert.calledWith(Metrics.sendBehavioralMetric, + BEHAVIORAL_METRICS.JOIN_WITH_MEDIA_FAILURE, + { + correlation_id: meeting.correlationId, + locus_id: meeting.locusUrl.split('/').pop(), + reason: addMediaError.message, + stack: addMediaError.stack, + leaveErrorReason: leaveError.message, + }, + { + type: addMediaError.name, + } + ); }); }); diff --git a/packages/@webex/plugin-meetings/test/unit/spec/meeting/request.js b/packages/@webex/plugin-meetings/test/unit/spec/meeting/request.js index 49532299b57..f8a0874a29b 100644 --- a/packages/@webex/plugin-meetings/test/unit/spec/meeting/request.js +++ b/packages/@webex/plugin-meetings/test/unit/spec/meeting/request.js @@ -194,12 +194,14 @@ describe('plugin-meetings', () => { const roapMessage = 'roap-message'; const permissionToken = 'permission-token'; const installationId = 'installationId'; + const reachability = 'reachability'; await meetingsRequest.joinMeeting({ locusUrl, deviceUrl, correlationId, roapMessage, + reachability, permissionToken, }); const requestParams = meetingsRequest.request.getCall(0).args[0]; @@ -212,6 +214,7 @@ describe('plugin-meetings', () => { assert.equal(requestParams.body.permissionToken, 'permission-token'); assert.equal(requestParams.body.device.regionCode, 'WEST-COAST'); assert.include(requestParams.body.device.localIp, '127.0.0'); + assert.deepEqual(requestParams.body.localMedias, [{localSdp: '{"roapMessage":"roap-message","reachability":"reachability"}'}]); assert.calledOnceWithExactly(anonymizeIpSpy, '127.0.0.1'); }); diff --git a/packages/@webex/plugin-meetings/test/unit/spec/meeting/utils.js b/packages/@webex/plugin-meetings/test/unit/spec/meeting/utils.js index 7ad1b06c99f..937c7d922de 100644 --- a/packages/@webex/plugin-meetings/test/unit/spec/meeting/utils.js +++ b/packages/@webex/plugin-meetings/test/unit/spec/meeting/utils.js @@ -402,13 +402,18 @@ describe('plugin-meetings', () => { }; const parseLocusJoinSpy = sinon.stub(MeetingUtil, 'parseLocusJoin'); - await MeetingUtil.joinMeeting(meeting, {}); + await MeetingUtil.joinMeeting(meeting, { + reachability: 'reachability', + roapMessage: 'roapMessage', + }); assert.calledOnce(meeting.meetingRequest.joinMeeting); const parameter = meeting.meetingRequest.joinMeeting.getCall(0).args[0]; assert.equal(parameter.inviteeAddress, 'meetingJoinUrl'); assert.equal(parameter.preferTranscoding, true); + assert.equal(parameter.reachability, 'reachability'); + assert.equal(parameter.roapMessage, 'roapMessage'); assert.calledWith(webex.internal.newMetrics.submitClientEvent, { name: 'client.locus.join.request', diff --git a/packages/@webex/plugin-meetings/test/unit/spec/roap/index.ts b/packages/@webex/plugin-meetings/test/unit/spec/roap/index.ts index 11a3b8614ea..4b8bcc621d5 100644 --- a/packages/@webex/plugin-meetings/test/unit/spec/roap/index.ts +++ b/packages/@webex/plugin-meetings/test/unit/spec/roap/index.ts @@ -13,15 +13,23 @@ import BEHAVIORAL_METRICS from '@webex/plugin-meetings/src/metrics/constants'; import { IP_VERSION } from '../../../../src/constants'; describe('Roap', () => { + let webex; + + const RESULT = {something: 'some value'}; + const meeting = {id: 'some meeting id'} as Meeting; + + beforeEach(() => { + webex = new MockWebex({}); + }); + + afterEach(() => { + sinon.restore(); + }); + describe('doTurnDiscovery', () => { [false, true].forEach(function (isReconnecting) { [false, true, undefined].forEach(function (isForced) { it(`calls this.turnDiscovery.doTurnDiscovery() and forwards all the arguments when isReconnecting = ${isReconnecting} and isForced = ${isForced}`, async () => { - const webex = new MockWebex({}); - - const RESULT = {something: 'some value'}; - const meeting = {id: 'some meeting id'} as Meeting; - const doTurnDiscoveryStub = sinon .stub(TurnDiscovery.prototype, 'doTurnDiscovery') .resolves(RESULT); @@ -32,11 +40,58 @@ describe('Roap', () => { assert.calledOnceWithExactly(doTurnDiscoveryStub, meeting, isReconnecting, isForced); assert.deepEqual(result, RESULT); + }); + }); + }); + + describe('generateTurnDiscoveryRequestMessage', () => { + [false, true].forEach(function (isForced) { + it(`calls this.turnDiscovery.generateTurnDiscoveryRequestMessage with isForced=${isForced}`, async () => { + const generateTurnDiscoveryRequestMessageStub = sinon + .stub(TurnDiscovery.prototype, 'generateTurnDiscoveryRequestMessage') + .resolves(RESULT); + + const roap = new Roap({}, {parent: webex}); - sinon.restore(); + const result = await roap.generateTurnDiscoveryRequestMessage(meeting, isForced); + + assert.calledOnceWithExactly(generateTurnDiscoveryRequestMessageStub, meeting, isForced); + assert.deepEqual(result, RESULT); }); }); }); + + describe('handleTurnDiscoveryHttpResponse', () => { + it('calls this.turnDiscovery.handleTurnDiscoveryHttpResponse', async () => { + const handleTurnDiscoveryHttpResponseStub = sinon + .stub(TurnDiscovery.prototype, 'handleTurnDiscoveryHttpResponse') + .resolves(RESULT); + + const httpReponse = {some: 'http response'}; + + const roap = new Roap({}, {parent: webex}); + + const result = await roap.handleTurnDiscoveryHttpResponse(meeting, httpReponse); + + assert.calledOnceWithExactly(handleTurnDiscoveryHttpResponseStub, meeting, httpReponse); + assert.deepEqual(result, RESULT); + }); + }); + + describe('abortTurnDiscovery', () => { + it('calls this.turnDiscovery.abort', async () => { + const abortStub = sinon + .stub(TurnDiscovery.prototype, 'abort') + .returns(RESULT); + + const roap = new Roap({}, {parent: webex}); + + const result = await roap.abortTurnDiscovery(); + + assert.calledOnceWithExactly(abortStub); + assert.deepEqual(result, RESULT); + }); + }); }); describe('sendRoapMediaRequest', () => { diff --git a/packages/@webex/plugin-meetings/test/unit/spec/roap/turnDiscovery.ts b/packages/@webex/plugin-meetings/test/unit/spec/roap/turnDiscovery.ts index 787776c0533..c8ce7ce9699 100644 --- a/packages/@webex/plugin-meetings/test/unit/spec/roap/turnDiscovery.ts +++ b/packages/@webex/plugin-meetings/test/unit/spec/roap/turnDiscovery.ts @@ -121,7 +121,7 @@ describe('TurnDiscovery', () => { }); // checks that OK roap message was sent or not sent and that the result is as expected - const checkResult = async (resultPromise, expectedRoapMessageSent, expectedResult) => { + const checkResult = async (resultPromise, expectedRoapMessageSent, expectedResult, expectedSkipReason?: string) => { let turnServerInfo, turnDiscoverySkippedReason; if (expectedRoapMessageSent === 'OK') { @@ -150,7 +150,7 @@ describe('TurnDiscovery', () => { } assert.deepEqual(turnServerInfo, expectedResult); - assert.isUndefined(turnDiscoverySkippedReason); + assert.equal(turnDiscoverySkippedReason, expectedSkipReason); }; it('sends TURN_DISCOVERY_REQUEST, waits for response and sends OK', async () => { @@ -302,7 +302,7 @@ describe('TurnDiscovery', () => { // @ts-ignore mockRoapRequest.sendRoap.resetHistory(); - await checkResult(result, undefined, undefined); + await checkResult(result, undefined, undefined, 'failure: Unexpected token o in JSON at position 1'); checkFailureMetricsSent(); }); @@ -366,7 +366,7 @@ describe('TurnDiscovery', () => { // @ts-ignore mockRoapRequest.sendRoap.resetHistory(); - await checkResult(result, undefined, undefined); + await checkResult(result, undefined, undefined, 'failure: TURN_DISCOVERY_RESPONSE in http response has unexpected messageType: {"seq":"0","messageType":"ERROR"}'); }); }); }); @@ -494,7 +494,7 @@ describe('TurnDiscovery', () => { assert.isUndefined(turnDiscoverySkippedReason); }); - it('resolves with undefined if sending the request fails', async () => { + it('resolves with undefined turnServerInfo if sending the request fails', async () => { const td = new TurnDiscovery(mockRoapRequest); mockRoapRequest.sendRoap = sinon.fake.rejects(new Error('fake error')); @@ -504,11 +504,11 @@ describe('TurnDiscovery', () => { const {turnServerInfo, turnDiscoverySkippedReason} = result; assert.isUndefined(turnServerInfo); - assert.isUndefined(turnDiscoverySkippedReason); + assert.equal(turnDiscoverySkippedReason, 'failure: fake error'); checkFailureMetricsSent(); }); - it('resolves with undefined when cluster is reachable', async () => { + it('resolves with undefined turnServerInfo when cluster is reachable', async () => { const prev = testMeeting.webex.meetings.reachability.isAnyPublicClusterReachable; testMeeting.webex.meetings.reachability.isAnyPublicClusterReachable = () => Promise.resolve(true); @@ -523,7 +523,7 @@ describe('TurnDiscovery', () => { testMeeting.webex.meetings.reachability.isAnyPublicClusterReachable = prev; }); - it("resolves with undefined if we don't get a response within 10s", async () => { + it("resolves with undefined turnServerInfo if we don't get a response within 10s", async () => { const td = new TurnDiscovery(mockRoapRequest); const promise = td.doTurnDiscovery(testMeeting, false); @@ -534,11 +534,11 @@ describe('TurnDiscovery', () => { const {turnServerInfo, turnDiscoverySkippedReason} = await promise; assert.isUndefined(turnServerInfo); - assert.isUndefined(turnDiscoverySkippedReason); + assert.equal(turnDiscoverySkippedReason, 'failure: Timed out waiting for TURN_DISCOVERY_RESPONSE'); checkFailureMetricsSent(); }); - it('resolves with undefined if the response does not have all the headers we expect', async () => { + it('resolves with undefined turnServerInfo if the response does not have all the headers we expect', async () => { const td = new TurnDiscovery(mockRoapRequest); const turnDiscoveryPromise = td.doTurnDiscovery(testMeeting, false); @@ -559,11 +559,11 @@ describe('TurnDiscovery', () => { const {turnServerInfo, turnDiscoverySkippedReason} = await turnDiscoveryPromise; assert.isUndefined(turnServerInfo); - assert.isUndefined(turnDiscoverySkippedReason); + assert.equal(turnDiscoverySkippedReason, `failure: TURN_DISCOVERY_RESPONSE from test missing some headers: ["x-cisco-turn-url=${FAKE_TURN_URL}","x-cisco-turn-username=${FAKE_TURN_USERNAME}"]`); checkFailureMetricsSent(); }); - it('resolves with undefined if the response does not have any headers', async () => { + it('resolves with undefined turnServerInfo if the response does not have any headers', async () => { const td = new TurnDiscovery(mockRoapRequest); const turnDiscoveryPromise = td.doTurnDiscovery(testMeeting, false); @@ -576,11 +576,11 @@ describe('TurnDiscovery', () => { const {turnServerInfo, turnDiscoverySkippedReason} = await turnDiscoveryPromise; assert.isUndefined(turnServerInfo); - assert.isUndefined(turnDiscoverySkippedReason); + assert.equal(turnDiscoverySkippedReason, 'failure: TURN_DISCOVERY_RESPONSE from test missing some headers: undefined'); checkFailureMetricsSent(); }); - it('resolves with undefined if the response has empty headers array', async () => { + it('resolves with undefined turnServerInfo if the response has empty headers array', async () => { const td = new TurnDiscovery(mockRoapRequest); const turnDiscoveryPromise = td.doTurnDiscovery(testMeeting, false); @@ -596,7 +596,7 @@ describe('TurnDiscovery', () => { const {turnServerInfo, turnDiscoverySkippedReason} = await turnDiscoveryPromise; assert.isUndefined(turnServerInfo); - assert.isUndefined(turnDiscoverySkippedReason); + assert.equal(turnDiscoverySkippedReason, 'failure: TURN_DISCOVERY_RESPONSE from test missing some headers: []'); checkFailureMetricsSent(); }); @@ -636,7 +636,7 @@ describe('TurnDiscovery', () => { const {turnServerInfo, turnDiscoverySkippedReason} = await turnDiscoveryPromise; assert.isUndefined(turnServerInfo); - assert.isUndefined(turnDiscoverySkippedReason); + assert.equal(turnDiscoverySkippedReason, 'failure: fake error'); checkFailureMetricsSent(); }); }); @@ -676,4 +676,286 @@ describe('TurnDiscovery', () => { assert.notCalled(mockRoapRequest.sendRoap); }); }); + + describe('generateTurnDiscoveryRequestMessage', () => { + let td; + + beforeEach(() => { + td = new TurnDiscovery(mockRoapRequest); + sinon.stub(td, 'getSkipReason').resolves(undefined); + }); + + it('generates TURN_DISCOVERY_REQUEST message irrespective of skip reason when called with isForced=true', async () => { + td.getSkipReason.resolves('reachability'); + + const result = await td.generateTurnDiscoveryRequestMessage(testMeeting, true); + + assert.deepEqual(result, { + roapMessage: { + messageType: 'TURN_DISCOVERY_REQUEST', + version: '2', + seq: 0, + headers: ['includeAnswerInHttpResponse', 'noOkInTransaction'], + }, + turnDiscoverySkippedReason: undefined, + }); + }); + + it('takes into account skip reason when called with isForced=false', async () => { + td.getSkipReason.resolves('reachability'); + + const result = await td.generateTurnDiscoveryRequestMessage(testMeeting, false); + + assert.deepEqual(result, { + roapMessage: undefined, + turnDiscoverySkippedReason: 'reachability', + }); + }); + + it('generates TURN_DISCOVERY_REQUEST message if there is no skip reason when called with isForced=false', async () => { + const result = await td.generateTurnDiscoveryRequestMessage(testMeeting, false); + + assert.deepEqual(result, { + roapMessage: { + messageType: 'TURN_DISCOVERY_REQUEST', + version: '2', + seq: 0, + headers: ['includeAnswerInHttpResponse', 'noOkInTransaction'], + }, + turnDiscoverySkippedReason: undefined, + }); + }); + + it('returns "already in progress" if TURN_DISCOVERY_REQUEST was already generated', async () => { + // 1st call + await td.generateTurnDiscoveryRequestMessage(testMeeting, true); + + // 2nd call + const result = await td.generateTurnDiscoveryRequestMessage(testMeeting, true); + + assert.deepEqual(result, { + roapMessage: undefined, + turnDiscoverySkippedReason: 'already in progress', + }); + }); + + it('returns "already in progress" if doTurnDiscovery was called and not completed', async () => { + let promiseResolve; + + // set it up so that doTurnDiscovery doesn't complete + mockRoapRequest.sendRoap = sinon.fake.returns(new Promise((resolve) => { + promiseResolve = resolve; + })); + td.doTurnDiscovery(testMeeting, false, true); + + // now call generateTurnDiscoveryRequestMessage + const result = await td.generateTurnDiscoveryRequestMessage(testMeeting, true); + + assert.deepEqual(result, { + roapMessage: undefined, + turnDiscoverySkippedReason: 'already in progress', + }); + + // resolve the promise, just so that we don't leave it hanging + promiseResolve(); + }); + }); + + describe('handleTurnDiscoveryHttpResponse', () => { + let td; + let roapMessage; + + beforeEach(() => { + roapMessage = { + seq: 1, + messageType: 'TURN_DISCOVERY_RESPONSE', + errorType: undefined, + errorCause: undefined, + headers: [ + `x-cisco-turn-url=${FAKE_TURN_URL}`, + `x-cisco-turn-username=${FAKE_TURN_USERNAME}`, + `x-cisco-turn-password=${FAKE_TURN_PASSWORD}`, + 'noOkInTransaction' + ], + } + + td = new TurnDiscovery(mockRoapRequest); + }); + + // checks if another TURN discovery can be started without any problem + const checkNextTurnDiscovery = async () => { + // after each test check that another TURN discovery can be started without any problems + const secondMessage = await td.generateTurnDiscoveryRequestMessage(testMeeting, true); + + assert.isDefined(secondMessage.roapMessage); + }; + + it('works as expected when called with undefined httpResponse', async () => { + await td.generateTurnDiscoveryRequestMessage(testMeeting, true); + + const result = await td.handleTurnDiscoveryHttpResponse(testMeeting, undefined); + + assert.deepEqual(result, { + turnServerInfo: undefined, + turnDiscoverySkippedReason: 'missing http response', + }); + }); + + [ + {testCase: 'is missing mediaConnections', httpResponse: {}}, + {testCase: 'is missing mediaConnections[0]', httpResponse: {mediaConnections: []}}, + {testCase: 'is missing mediaConnections[0].remoteSdp', httpResponse: {mediaConnections: [{}]}}, + {testCase: 'is missing roapMesssage in mediaConnections[0].remoteSdp', httpResponse: {mediaConnections: [{remoteSdp: JSON.stringify({something: "whatever"})}]}}, + ].forEach(({testCase, httpResponse}) => { + it(`handles httpResponse that ${testCase}`, async () => { + await td.generateTurnDiscoveryRequestMessage(testMeeting, true); + + const result = await td.handleTurnDiscoveryHttpResponse(testMeeting, httpResponse); + + assert.deepEqual(result, { + turnServerInfo: undefined, + turnDiscoverySkippedReason: 'missing http response', + }); + }); + }); + + it('handles httpResponse with invalid JSON in mediaConnections[0].remoteSdp', async () => { + await td.generateTurnDiscoveryRequestMessage(testMeeting, true); + + const result = await td.handleTurnDiscoveryHttpResponse(testMeeting, {mediaConnections: [{remoteSdp: 'not a json'}]}); + + assert.deepEqual(result, { + turnServerInfo: undefined, + turnDiscoverySkippedReason: 'failure: Unexpected token o in JSON at position 1', + }); + }); + + it('fails when called before generateTurnDiscoveryRequestMessage() was called', async () => { + const httpResponse = {mediaConnections: [{remoteSdp: JSON.stringify({roapMessage})}]}; + await assert.isRejected(td.handleTurnDiscoveryHttpResponse(testMeeting, httpResponse), + 'handleTurnDiscoveryHttpResponse() called before generateTurnDiscoveryRequestMessage()'); + }); + + it('works as expected when called with valid httpResponse', async () => { + const httpResponse = {mediaConnections: [{remoteSdp: JSON.stringify({roapMessage})}]}; + + // we spy on handleTurnDiscoveryResponse and check that it's called so that we don't have to repeat + // all the edge case tests here, they're already covered in other tests that call handleTurnDiscoveryResponse + const handleTurnDiscoveryResponseSpy = sinon.spy(td, 'handleTurnDiscoveryResponse'); + + await td.generateTurnDiscoveryRequestMessage(testMeeting, true); + const result = await td.handleTurnDiscoveryHttpResponse(testMeeting, httpResponse); + + assert.deepEqual(result, { + turnServerInfo: { + url: FAKE_TURN_URL, + username: FAKE_TURN_USERNAME, + password: FAKE_TURN_PASSWORD, + }, + turnDiscoverySkippedReason: undefined, + }); + + assert.calledOnceWithExactly(handleTurnDiscoveryResponseSpy, roapMessage, 'in http response'); + }); + + it('works as expected when httpResponse is missing some headers', async () => { + roapMessage.headers = [ + `x-cisco-turn-url=${FAKE_TURN_URL}`, // missing headers for username and password + ]; + + const httpResponse = {mediaConnections: [{remoteSdp: JSON.stringify({roapMessage})}]}; + + // we spy on handleTurnDiscoveryResponse and check that it's called so that we don't have to repeat + // all the edge case tests here, they're already covered in other tests that call handleTurnDiscoveryResponse + // we test just this 1 edge case here to confirm that when handleTurnDiscoveryResponse rejects, we get the correct result + const handleTurnDiscoveryResponseSpy = sinon.spy(td, 'handleTurnDiscoveryResponse'); + + await td.generateTurnDiscoveryRequestMessage(testMeeting, true); + const result = await td.handleTurnDiscoveryHttpResponse(testMeeting, httpResponse); + + assert.deepEqual(result, { + turnServerInfo: undefined, + turnDiscoverySkippedReason: 'failure: TURN_DISCOVERY_RESPONSE in http response missing some headers: ["x-cisco-turn-url=turns:fakeTurnServer.com:443?transport=tcp"]', + }); + assert.calledOnceWithExactly(handleTurnDiscoveryResponseSpy, roapMessage, 'in http response'); + + checkNextTurnDiscovery(); + }); + + it('sends OK when required', async () => { + roapMessage.headers = [ + `x-cisco-turn-url=${FAKE_TURN_URL}`, + `x-cisco-turn-username=${FAKE_TURN_USERNAME}`, + `x-cisco-turn-password=${FAKE_TURN_PASSWORD}`, + // noOkInTransaction is missing + ]; + const httpResponse = {mediaConnections: [{remoteSdp: JSON.stringify({roapMessage})}]}; + + await td.generateTurnDiscoveryRequestMessage(testMeeting, true); + const result = await td.handleTurnDiscoveryHttpResponse(testMeeting, httpResponse); + + assert.deepEqual(result, { + turnServerInfo: { + url: FAKE_TURN_URL, + username: FAKE_TURN_USERNAME, + password: FAKE_TURN_PASSWORD, + }, + turnDiscoverySkippedReason: undefined, + }); + + // check that OK was sent along with the metric for it + await checkRoapMessageSent('OK', 0); + + assert.calledWith( + Metrics.sendBehavioralMetric, + BEHAVIORAL_METRICS.TURN_DISCOVERY_REQUIRES_OK, + sinon.match({ + correlation_id: testMeeting.correlationId, + locus_id: FAKE_LOCUS_ID, + }) + ); + + checkNextTurnDiscovery(); + }); + + describe('abort', () => { + it('allows starting a new TURN discovery', async () => { + let result; + + // this mock is required for doTurnDiscovery() to work + mockRoapRequest.sendRoap = sinon.fake.resolves({ + mediaConnections: [ + { + mediaId: '464ff97f-4bda-466a-ad06-3a22184a2274', + remoteSdp: `{"roapMessage": {"messageType":"TURN_DISCOVERY_RESPONSE","seq":"0","headers": ["x-cisco-turn-url=${FAKE_TURN_URL}","x-cisco-turn-username=${FAKE_TURN_USERNAME}","x-cisco-turn-password=${FAKE_TURN_PASSWORD}", "noOkInTransaction"]}}`, + }, + ], + }); + + result = await td.generateTurnDiscoveryRequestMessage(testMeeting, true); + assert.isDefined(result.roapMessage); + + td.abort(); + + result = await td.generateTurnDiscoveryRequestMessage(testMeeting, true); + assert.isDefined(result.roapMessage); + + td.abort(); + + // check also that doTurnDiscovery() works after abort() + result = await td.doTurnDiscovery(testMeeting, false); + }); + + it('does nothing when called outside of a TURN discovery', async () => { + let result; + + // call abort() without any other calls before it - it should do nothing + // there is not much we can check, so afterwards we just check that we can start a new TURN discovery + td.abort(); + + result = await td.generateTurnDiscoveryRequestMessage(testMeeting, true); + assert.isDefined(result.roapMessage); + }); + }); + }); }); From dfcbe9de71c12e66d1b07480ea356b399e32dc19 Mon Sep 17 00:00:00 2001 From: JudyZhu <120536178+JudyZhuHz@users.noreply.github.com> Date: Tue, 9 Apr 2024 09:00:34 +0800 Subject: [PATCH 12/20] feat(plugin-meetings): Return resourceType when get locus annotation info (#3521) --- .../plugin-meetings/src/meeting/index.ts | 3 +- .../test/unit/spec/meeting/index.js | 37 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/packages/@webex/plugin-meetings/src/meeting/index.ts b/packages/@webex/plugin-meetings/src/meeting/index.ts index a95880a1d21..74aea8ae659 100644 --- a/packages/@webex/plugin-meetings/src/meeting/index.ts +++ b/packages/@webex/plugin-meetings/src/meeting/index.ts @@ -2549,7 +2549,8 @@ export default class Meeting extends StatelessWebexPlugin { contentShare.deviceUrlSharing === previousContentShare.deviceUrlSharing && whiteboardShare.beneficiaryId === previousWhiteboardShare?.beneficiaryId && whiteboardShare.disposition === previousWhiteboardShare?.disposition && - whiteboardShare.resourceUrl === previousWhiteboardShare?.resourceUrl + whiteboardShare.resourceUrl === previousWhiteboardShare?.resourceUrl && + contentShare.resourceType === previousContentShare?.resourceType ) { // nothing changed, so ignore // (this happens when we steal presentation from remote) diff --git a/packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js b/packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js index dc7f59f1485..ab28285812f 100644 --- a/packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js +++ b/packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js @@ -9631,6 +9631,13 @@ describe('plugin-meetings', () => { 'https://board-a.wbx2.com/board/api/v1/channels/977a7330-54f4-11eb-b1ef-91f5eefc7bf3', }; + const SHARE_TYPE = { + FILE: + 'FILE', + DESKTOP: + 'DESKTOP', + }; + const DEVICE_URL = { LOCAL_WEB: 'my-web-url', LOCAL_MAC: 'my-mac-url', @@ -10629,6 +10636,36 @@ describe('plugin-meetings', () => { payloadTestHelper([data1, data2, data3]); }); }); + + describe('File Share --> Desktop Share', () => { + it('Scenario #1: remote person A shares file then share desktop', () => { + const data1 = generateData( + blankPayload, + true, + true, + USER_IDS.ME, + undefined, + false, + undefined, + undefined, + undefined, + undefined, + DEVICE_URL.LOCAL_WEB, + SHARE_TYPE.FILE + ); + const data2 = generateData( + data1.payload, + true, + false, + USER_IDS.ME, + SHARE_TYPE.DESKTOP + ); + const data3 = generateData(data2.payload, true, true, USER_IDS.ME); + + payloadTestHelper([data1, data2, data3]); + }); + + }); }); }); From ec6bf6ed1aa77eba006e33194ae948b4c56d3599 Mon Sep 17 00:00:00 2001 From: Ashwin Kumar Date: Thu, 11 Apr 2024 11:31:36 -0700 Subject: [PATCH 13/20] Fix: Added updateLLMConnection when guest is admitted from the lobby (#3529) Co-authored-by: asmuthuk --- packages/@webex/plugin-meetings/src/meeting/index.ts | 1 + packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js | 2 ++ 2 files changed, 3 insertions(+) diff --git a/packages/@webex/plugin-meetings/src/meeting/index.ts b/packages/@webex/plugin-meetings/src/meeting/index.ts index 74aea8ae659..cca816ba062 100644 --- a/packages/@webex/plugin-meetings/src/meeting/index.ts +++ b/packages/@webex/plugin-meetings/src/meeting/index.ts @@ -3075,6 +3075,7 @@ export default class Meeting extends StatelessWebexPlugin { options: {meetingId: this.id}, }); } + this.updateLLMConnection(); }); // @ts-ignore - check if MEDIA_INACTIVITY exists diff --git a/packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js b/packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js index ab28285812f..f3749c66d80 100644 --- a/packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js +++ b/packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js @@ -7496,6 +7496,7 @@ describe('plugin-meetings', () => { }); it('listens to the self admitted guest event', (done) => { meeting.stopKeepAlive = sinon.stub(); + meeting.updateLLMConnection = sinon.stub(); meeting.locusInfo.emit({function: 'test', file: 'test'}, 'SELF_ADMITTED_GUEST', test1); assert.calledOnceWithExactly(meeting.stopKeepAlive); assert.calledThrice(TriggerProxy.trigger); @@ -7506,6 +7507,7 @@ describe('plugin-meetings', () => { 'meeting:self:guestAdmitted', {payload: test1} ); + assert.calledOnce(meeting.updateLLMConnection); done(); }); From 0cfa952d0614555e6b43e596548a77529c00a0d1 Mon Sep 17 00:00:00 2001 From: Bryce Tham Date: Mon, 15 Apr 2024 04:25:34 -0400 Subject: [PATCH 14/20] feat(plugin-meetings): separate API for enabled and muted states (#3443) Co-authored-by: Bryce Tham --- docs/samples/browser-plugin-meetings/app.js | 51 +++- .../browser-plugin-meetings/index.html | 2 + .../samples/browser-plugin-meetings/style.css | 4 + packages/@webex/media-helpers/package.json | 3 +- packages/@webex/media-helpers/src/index.ts | 1 + .../@webex/media-helpers/src/webrtc-core.ts | 19 +- .../test/unit/spec/webrtc-core.js | 21 +- packages/@webex/plugin-meetings/package.json | 2 +- packages/@webex/plugin-meetings/src/index.ts | 1 + .../plugin-meetings/src/meeting/index.ts | 76 ++++- .../plugin-meetings/src/meeting/muteState.ts | 54 ++-- .../test/integration/spec/journey.js | 26 +- .../test/unit/spec/meeting/index.js | 131 ++++++-- .../test/unit/spec/meeting/muteState.js | 286 ++++++++++++++---- yarn.lock | 70 +++-- 15 files changed, 551 insertions(+), 196 deletions(-) diff --git a/docs/samples/browser-plugin-meetings/app.js b/docs/samples/browser-plugin-meetings/app.js index c88774d42df..231662e7fb1 100644 --- a/docs/samples/browser-plugin-meetings/app.js +++ b/docs/samples/browser-plugin-meetings/app.js @@ -831,6 +831,8 @@ const stopShareBtn = document.querySelector('#ts-stop-screenshare'); const toggleAudioButton = document.querySelector('#ts-toggle-audio'); const stopVideoButton = document.querySelector('#ts-stop-video'); const stopAudioButton = document.querySelector('#ts-stop-audio'); +const muteVideoMessage = document.querySelector('#ts-mute-video-message'); +const muteAudioMessage = document.querySelector('#ts-mute-audio-message'); const modeBtn = document.getElementById('mode-type'); /** @@ -1320,6 +1322,15 @@ async function loadCamera(constraints) { meetingStreamsLocalVideo.srcObject = localMedia.cameraStream.outputStream; + localMedia.cameraStream.on('user-mute-state-change', (muted) => { + console.log('MeetingControls#loadCamera() :: local camera stream user mute state changed to', muted); + }); + + localMedia.cameraStream.on('system-mute-state-change', (muted) => { + console.log('MeetingControls#loadCamera() :: local camera stream system mute state changed to', muted); + handleMuteVideoMessage(); + }); + localMedia.cameraStream.on('stream-ended', () => { console.log('MeetingControls#loadCamera() :: local camera stream ended'); @@ -1332,6 +1343,7 @@ async function loadCamera(constraints) { clearVideoResolutionCheckInterval(localVideoResElm, localVideoResolutionInterval); }); + handleMuteVideoMessage(); handleEffectsButton(toggleVbgBtn, VBG); loadCameraBtn.disabled = true; stopVideoButton.disabled = false; @@ -1392,6 +1404,16 @@ async function loadMicrophone(constraints) { meetingStreamsLocalAudio.srcObject = localMedia.microphoneStream.outputStream; + localMedia.microphoneStream.on('user-mute-state-change', (muted) => { + console.log('MeetingControls#loadMicrophone() :: local microphone stream user mute state changed to', muted); + handleAudioButton(); + }); + + localMedia.microphoneStream.on('system-mute-state-change', (muted) => { + console.log('MeetingControls#loadMicrophone() :: local microphone stream system mute state changed to', muted); + handleMuteAudioMessage(); + }); + localMedia.microphoneStream.on('stream-ended', () => { console.log('MeetingControls#loadMicrophone() :: local microphone stream ended'); @@ -1402,6 +1424,7 @@ async function loadMicrophone(constraints) { loadMicrophoneBtn.disabled = false; }); + handleMuteAudioMessage(); handleEffectsButton(toggleBNRBtn, BNR); loadMicrophoneBtn.disabled = true; stopAudioButton.disabled = false; @@ -1642,12 +1665,12 @@ function setVideoInputDevice() { const {video} = getAudioVideoInput(); if (meeting) { - const isMuted = localMedia.cameraStream?.muted; + const isMuted = localMedia.cameraStream?.userMuted; localMedia.cameraStream?.stop(); return getUserMedia({video}) .then(() => { - localMedia.cameraStream.setMuted(!!isMuted); + localMedia.cameraStream.setUserMuted(!!isMuted); localVideoResolutionCheckInterval(); meeting.publishStreams({camera: localMedia.cameraStream}); }); @@ -1662,12 +1685,12 @@ function setAudioInputDevice() { const {audio} = getAudioVideoInput(); if (meeting) { - const isMuted = localMedia.microphoneStream?.muted; + const isMuted = localMedia.microphoneStream?.userMuted; localMedia.microphoneStream?.stop(); return getUserMedia({audio}) .then(() => { - localMedia.microphoneStream.setMuted(!!isMuted); + localMedia.microphoneStream.setUserMuted(!!isMuted); meeting.publishStreams({microphone: localMedia.microphoneStream}); }); } @@ -1690,31 +1713,38 @@ function setAudioOutputDevice() { } function handleAudioButton() { - const audioButtonTitle = localMedia.microphoneStream.muted ? 'Unmute' : 'Mute'; + const audioButtonTitle = localMedia.microphoneStream.userMuted ? 'Unmute' : 'Mute'; toggleAudioButton.innerHTML = `${audioButtonTitle} Audio`; } +function handleMuteAudioMessage() { + muteAudioMessage.innerHTML = localMedia.microphoneStream.systemMuted ? "Warning: microphone may be muted by the system" : ""; +} + function toggleSendAudio() { console.log('MeetingControls#toggleSendAudio()'); if (localMedia.microphoneStream) { - const newMuteValue = !localMedia.microphoneStream.muted; + const newMuteValue = !localMedia.microphoneStream.userMuted; - localMedia.microphoneStream.setMuted(newMuteValue); - handleAudioButton(); + localMedia.microphoneStream.setUserMuted(newMuteValue); console.log(`MeetingControls#toggleSendAudio() :: Successfully ${newMuteValue ? 'muted': 'unmuted'} audio!`); return; } } +function handleMuteVideoMessage() { + muteVideoMessage.innerHTML = localMedia.cameraStream.systemMuted ? "Warning: camera may be muted by the system" : ""; +} + function toggleSendVideo() { console.log('MeetingControls#toggleSendVideo()'); if (localMedia.cameraStream) { - const newMuteValue = !localMedia.cameraStream.muted; + const newMuteValue = !localMedia.cameraStream.userMuted; - localMedia.cameraStream.setMuted(newMuteValue); + localMedia.cameraStream.setUserMuted(newMuteValue); console.log(`MeetingControls#toggleSendVideo() :: Successfully ${newMuteValue ? 'muted': 'unmuted'} video!`); @@ -3083,7 +3113,6 @@ function muteMember(muteButton) { if (meeting) { meeting.mute(participantID, newMuteStatus).then((res) => { console.log(res, `participant is ${newMuteStatus ? 'muted' : 'unmuted'}`); - handleAudioButton(); }).catch((err) => { console.log('error', err); }); diff --git a/docs/samples/browser-plugin-meetings/index.html b/docs/samples/browser-plugin-meetings/index.html index 5873b65b6a0..aae1f09e1dd 100644 --- a/docs/samples/browser-plugin-meetings/index.html +++ b/docs/samples/browser-plugin-meetings/index.html @@ -189,6 +189,7 @@

+ @@ -209,6 +210,7 @@

+ diff --git a/docs/samples/browser-plugin-meetings/style.css b/docs/samples/browser-plugin-meetings/style.css index 0477faba931..adb172f8cb3 100644 --- a/docs/samples/browser-plugin-meetings/style.css +++ b/docs/samples/browser-plugin-meetings/style.css @@ -386,6 +386,10 @@ legend { padding: 0.3rem; } +.webex-warning { + color: #7d4705; +} + .webex-error { color: #de3434; } diff --git a/packages/@webex/media-helpers/package.json b/packages/@webex/media-helpers/package.json index 78d66be8311..d998b31ee62 100644 --- a/packages/@webex/media-helpers/package.json +++ b/packages/@webex/media-helpers/package.json @@ -22,7 +22,7 @@ "deploy:npm": "yarn npm publish" }, "dependencies": { - "@webex/internal-media-core": "2.2.9", + "@webex/internal-media-core": "2.3.0", "@webex/ts-events": "^1.1.0", "@webex/web-media-effects": "^2.15.6" }, @@ -41,6 +41,7 @@ "@webex/test-helper-chai": "workspace:*", "@webex/test-helper-mock-webex": "workspace:*", "eslint": "^8.24.0", + "jsdom-global": "3.0.2", "sinon": "^9.2.4" } } diff --git a/packages/@webex/media-helpers/src/index.ts b/packages/@webex/media-helpers/src/index.ts index ca01bd60d0a..b22e2794515 100644 --- a/packages/@webex/media-helpers/src/index.ts +++ b/packages/@webex/media-helpers/src/index.ts @@ -6,6 +6,7 @@ export { LocalStreamEventNames, StreamEventNames, RemoteStream, + RemoteStreamEventNames, type ServerMuteReason, LocalMicrophoneStreamEventNames, LocalCameraStreamEventNames, diff --git a/packages/@webex/media-helpers/src/webrtc-core.ts b/packages/@webex/media-helpers/src/webrtc-core.ts index 672413dde35..023e69f81dd 100644 --- a/packages/@webex/media-helpers/src/webrtc-core.ts +++ b/packages/@webex/media-helpers/src/webrtc-core.ts @@ -23,12 +23,13 @@ export { LocalStreamEventNames, StreamEventNames, RemoteStream, + RemoteStreamEventNames, type VideoContentHint, } from '@webex/internal-media-core'; export type ServerMuteReason = | 'remotelyMuted' // other user has remotely muted us - | 'clientRequestFailed' // client called setMuted() but server request failed + | 'clientRequestFailed' // client called setUserMuted() but server request failed | 'localUnmuteRequired'; // server forced the client to be unmuted // these events are in addition to WCME events. This will be properly typed once webrtc-core event types inheritance is fixed @@ -74,22 +75,22 @@ class _LocalMicrophoneStream extends WcmeLocalMicrophoneStream { return this.unmuteAllowed; } - setMuted(muted: boolean): void { + setUserMuted(muted: boolean): void { if (!muted) { if (!this.isUnmuteAllowed()) { throw new Error('Unmute is not allowed'); } } - return super.setMuted(muted); + return super.setUserMuted(muted); } /** * @internal */ setServerMuted(muted: boolean, reason: ServerMuteReason) { - if (muted !== this.muted) { - this.setMuted(muted); + if (muted !== this.userMuted) { + this.setUserMuted(muted); this[LocalMicrophoneStreamEventNames.ServerMuted].emit(muted, reason); } } @@ -116,22 +117,22 @@ class _LocalCameraStream extends WcmeLocalCameraStream { return this.unmuteAllowed; } - setMuted(muted: boolean): void { + setUserMuted(muted: boolean): void { if (!muted) { if (!this.isUnmuteAllowed()) { throw new Error('Unmute is not allowed'); } } - return super.setMuted(muted); + return super.setUserMuted(muted); } /** * @internal */ setServerMuted(muted: boolean, reason: ServerMuteReason) { - if (muted !== this.muted) { - this.setMuted(muted); + if (muted !== this.userMuted) { + this.setUserMuted(muted); this[LocalCameraStreamEventNames.ServerMuted].emit(muted, reason); } } diff --git a/packages/@webex/media-helpers/test/unit/spec/webrtc-core.js b/packages/@webex/media-helpers/test/unit/spec/webrtc-core.js index 6f9646fe6dd..abbcf0a5002 100644 --- a/packages/@webex/media-helpers/test/unit/spec/webrtc-core.js +++ b/packages/@webex/media-helpers/test/unit/spec/webrtc-core.js @@ -1,3 +1,4 @@ +import 'jsdom-global/register'; import {assert, expect} from '@webex/test-helper-chai'; import sinon from 'sinon'; import { @@ -53,22 +54,22 @@ describe('media-helpers', () => { it('by default allows unmuting', async () => { assert.equal(stream.isUnmuteAllowed(), true); - await stream.setMuted(false); + await stream.setUserMuted(false); }); - it('rejects setMute(false) if unmute is not allowed', async () => { - await stream.setUnmuteAllowed(false); + it('rejects setUserMuted(false) if unmute is not allowed', async () => { + stream.setUnmuteAllowed(false); assert.equal(stream.isUnmuteAllowed(), false); - const fn = () => stream.setMuted(false); + const fn = () => stream.setUserMuted(false); expect(fn).to.throw(/Unmute is not allowed/); }); - it('resolves setMute(false) if unmute is allowed', async () => { - await stream.setUnmuteAllowed(true); + it('resolves setUserMuted(false) if unmute is allowed', async () => { + stream.setUnmuteAllowed(true); assert.equal(stream.isUnmuteAllowed(), true); - await stream.setMuted(false); + await stream.setUserMuted(false); }); it('returns a reasonable length string from JSON.stringify()', () => { @@ -81,16 +82,16 @@ describe('media-helpers', () => { }); const checkSetServerMuted = async (startMute, setMute, expectedCalled) => { - await stream.setMuted(startMute); + await stream.setUserMuted(startMute); - assert.equal(stream.muted, startMute); + assert.equal(stream.userMuted, startMute); const handler = sinon.fake(); stream.on(event.ServerMuted, handler); await stream.setServerMuted(setMute, 'remotelyMuted'); - assert.equal(stream.muted, setMute); + assert.equal(stream.userMuted, setMute); if (expectedCalled) { assert.calledOnceWithExactly(handler, setMute, 'remotelyMuted'); } else { diff --git a/packages/@webex/plugin-meetings/package.json b/packages/@webex/plugin-meetings/package.json index ac85bb83b65..45745472869 100644 --- a/packages/@webex/plugin-meetings/package.json +++ b/packages/@webex/plugin-meetings/package.json @@ -62,7 +62,7 @@ }, "dependencies": { "@webex/common": "workspace:*", - "@webex/internal-media-core": "2.2.9", + "@webex/internal-media-core": "2.3.0", "@webex/internal-plugin-conversation": "workspace:*", "@webex/internal-plugin-device": "workspace:*", "@webex/internal-plugin-llm": "workspace:*", diff --git a/packages/@webex/plugin-meetings/src/index.ts b/packages/@webex/plugin-meetings/src/index.ts index 54fdfafbc91..0d04c5ea429 100644 --- a/packages/@webex/plugin-meetings/src/index.ts +++ b/packages/@webex/plugin-meetings/src/index.ts @@ -19,6 +19,7 @@ export { LocalSystemAudioStream, LocalStreamEventNames, StreamEventNames, + RemoteStreamEventNames, type ServerMuteReason, LocalMicrophoneStreamEventNames, LocalCameraStreamEventNames, diff --git a/packages/@webex/plugin-meetings/src/meeting/index.ts b/packages/@webex/plugin-meetings/src/meeting/index.ts index cca816ba062..e845037efe8 100644 --- a/packages/@webex/plugin-meetings/src/meeting/index.ts +++ b/packages/@webex/plugin-meetings/src/meeting/index.ts @@ -616,8 +616,8 @@ export default class Meeting extends StatelessWebexPlugin { resourceUrl: string; selfId: string; state: any; - localAudioStreamMuteStateHandler: (muted: boolean) => void; - localVideoStreamMuteStateHandler: (muted: boolean) => void; + localAudioStreamMuteStateHandler: () => void; + localVideoStreamMuteStateHandler: () => void; localOutputTrackChangeHandler: () => void; roles: any[]; environment: string; @@ -1382,12 +1382,12 @@ export default class Meeting extends StatelessWebexPlugin { */ this.remoteMediaManager = null; - this.localAudioStreamMuteStateHandler = (muted: boolean) => { - this.audio.handleLocalStreamMuteStateChange(this, muted); + this.localAudioStreamMuteStateHandler = () => { + this.audio.handleLocalStreamMuteStateChange(this); }; - this.localVideoStreamMuteStateHandler = (muted: boolean) => { - this.video.handleLocalStreamMuteStateChange(this, muted); + this.localVideoStreamMuteStateHandler = () => { + this.video.handleLocalStreamMuteStateChange(this); }; // The handling of output track changes should be done inside @@ -3897,7 +3897,14 @@ export default class Meeting extends StatelessWebexPlugin { private async setLocalAudioStream(localStream?: LocalMicrophoneStream) { const oldStream = this.mediaProperties.audioStream; - oldStream?.off(StreamEventNames.MuteStateChange, this.localAudioStreamMuteStateHandler); + oldStream?.off( + LocalStreamEventNames.UserMuteStateChange, + this.localAudioStreamMuteStateHandler + ); + oldStream?.off( + LocalStreamEventNames.SystemMuteStateChange, + this.localAudioStreamMuteStateHandler + ); oldStream?.off(LocalStreamEventNames.OutputTrackChange, this.localOutputTrackChangeHandler); // we don't update this.mediaProperties.mediaDirection.sendAudio, because we always keep it as true to avoid extra SDP exchanges @@ -3905,7 +3912,14 @@ export default class Meeting extends StatelessWebexPlugin { this.audio.handleLocalStreamChange(this); - localStream?.on(StreamEventNames.MuteStateChange, this.localAudioStreamMuteStateHandler); + localStream?.on( + LocalStreamEventNames.UserMuteStateChange, + this.localAudioStreamMuteStateHandler + ); + localStream?.on( + LocalStreamEventNames.SystemMuteStateChange, + this.localAudioStreamMuteStateHandler + ); localStream?.on(LocalStreamEventNames.OutputTrackChange, this.localOutputTrackChangeHandler); if (!this.isMultistream || !localStream) { @@ -3925,7 +3939,14 @@ export default class Meeting extends StatelessWebexPlugin { private async setLocalVideoStream(localStream?: LocalCameraStream) { const oldStream = this.mediaProperties.videoStream; - oldStream?.off(StreamEventNames.MuteStateChange, this.localVideoStreamMuteStateHandler); + oldStream?.off( + LocalStreamEventNames.UserMuteStateChange, + this.localVideoStreamMuteStateHandler + ); + oldStream?.off( + LocalStreamEventNames.SystemMuteStateChange, + this.localVideoStreamMuteStateHandler + ); oldStream?.off(LocalStreamEventNames.OutputTrackChange, this.localOutputTrackChangeHandler); // we don't update this.mediaProperties.mediaDirection.sendVideo, because we always keep it as true to avoid extra SDP exchanges @@ -3933,7 +3954,14 @@ export default class Meeting extends StatelessWebexPlugin { this.video.handleLocalStreamChange(this); - localStream?.on(StreamEventNames.MuteStateChange, this.localVideoStreamMuteStateHandler); + localStream?.on( + LocalStreamEventNames.UserMuteStateChange, + this.localVideoStreamMuteStateHandler + ); + localStream?.on( + LocalStreamEventNames.SystemMuteStateChange, + this.localVideoStreamMuteStateHandler + ); localStream?.on(LocalStreamEventNames.OutputTrackChange, this.localOutputTrackChangeHandler); if (!this.isMultistream || !localStream) { @@ -3954,14 +3982,17 @@ export default class Meeting extends StatelessWebexPlugin { private async setLocalShareVideoStream(localDisplayStream?: LocalDisplayStream) { const oldStream = this.mediaProperties.shareVideoStream; - oldStream?.off(StreamEventNames.MuteStateChange, this.handleShareVideoStreamMuteStateChange); + oldStream?.off( + LocalStreamEventNames.SystemMuteStateChange, + this.handleShareVideoStreamMuteStateChange + ); oldStream?.off(StreamEventNames.Ended, this.handleShareVideoStreamEnded); oldStream?.off(LocalStreamEventNames.OutputTrackChange, this.localOutputTrackChangeHandler); this.mediaProperties.setLocalShareVideoStream(localDisplayStream); localDisplayStream?.on( - StreamEventNames.MuteStateChange, + LocalStreamEventNames.SystemMuteStateChange, this.handleShareVideoStreamMuteStateChange ); localDisplayStream?.on(StreamEventNames.Ended, this.handleShareVideoStreamEnded); @@ -4047,10 +4078,24 @@ export default class Meeting extends StatelessWebexPlugin { public cleanupLocalStreams() { const {audioStream, videoStream, shareAudioStream, shareVideoStream} = this.mediaProperties; - audioStream?.off(StreamEventNames.MuteStateChange, this.localAudioStreamMuteStateHandler); + audioStream?.off( + LocalStreamEventNames.UserMuteStateChange, + this.localAudioStreamMuteStateHandler + ); + audioStream?.off( + LocalStreamEventNames.SystemMuteStateChange, + this.localAudioStreamMuteStateHandler + ); audioStream?.off(LocalStreamEventNames.OutputTrackChange, this.localOutputTrackChangeHandler); - videoStream?.off(StreamEventNames.MuteStateChange, this.localVideoStreamMuteStateHandler); + videoStream?.off( + LocalStreamEventNames.UserMuteStateChange, + this.localVideoStreamMuteStateHandler + ); + videoStream?.off( + LocalStreamEventNames.SystemMuteStateChange, + this.localVideoStreamMuteStateHandler + ); videoStream?.off(LocalStreamEventNames.OutputTrackChange, this.localOutputTrackChangeHandler); shareAudioStream?.off(StreamEventNames.Ended, this.handleShareAudioStreamEnded); @@ -4058,8 +4103,9 @@ export default class Meeting extends StatelessWebexPlugin { LocalStreamEventNames.OutputTrackChange, this.localOutputTrackChangeHandler ); + shareVideoStream?.off( - StreamEventNames.MuteStateChange, + LocalStreamEventNames.SystemMuteStateChange, this.handleShareVideoStreamMuteStateChange ); shareVideoStream?.off(StreamEventNames.Ended, this.handleShareVideoStreamEnded); diff --git a/packages/@webex/plugin-meetings/src/meeting/muteState.ts b/packages/@webex/plugin-meetings/src/meeting/muteState.ts index 394f4cc7186..1c3ed6ccbc2 100644 --- a/packages/@webex/plugin-meetings/src/meeting/muteState.ts +++ b/packages/@webex/plugin-meetings/src/meeting/muteState.ts @@ -150,15 +150,30 @@ export class MuteState { * @param {Boolean} [mute] true for muting, false for unmuting request * @returns {void} */ - public handleLocalStreamMuteStateChange(meeting?: object, mute?: boolean) { + public handleLocalStreamMuteStateChange(meeting?: any) { if (this.ignoreMuteStateChange) { return; } + + // either user or system may have triggered a mute state change, but localMute should reflect both + let newMuteState: boolean; + let userMuteState: boolean; + let systemMuteState: boolean; + if (this.type === AUDIO) { + newMuteState = meeting.mediaProperties.audioStream?.muted; + userMuteState = meeting.mediaProperties.audioStream?.userMuted; + systemMuteState = meeting.mediaProperties.audioStream?.systemMuted; + } else { + newMuteState = meeting.mediaProperties.videoStream?.muted; + userMuteState = meeting.mediaProperties.videoStream?.userMuted; + systemMuteState = meeting.mediaProperties.videoStream?.systemMuted; + } + LoggerProxy.logger.info( - `Meeting:muteState#handleLocalStreamMuteStateChange --> ${this.type}: local stream new mute state: ${mute}` + `Meeting:muteState#handleLocalStreamMuteStateChange --> ${this.type}: local stream new mute state: ${newMuteState} (user mute: ${userMuteState}, system mute: ${systemMuteState})` ); - this.state.client.localMute = mute; + this.state.client.localMute = newMuteState; this.applyClientStateToServer(meeting); } @@ -249,7 +264,12 @@ export class MuteState { `Meeting:muteState#applyClientStateToServer --> ${this.type}: error: ${e}` ); - this.applyServerMuteToLocalStream(meeting, 'clientRequestFailed'); + // failed to apply client state to server, so revert stream mute state to server state + this.muteLocalStream( + meeting, + this.state.server.localMute || this.state.server.remoteMute, + 'clientRequestFailed' + ); }); } @@ -325,18 +345,6 @@ export class MuteState { }); } - /** Sets the mute state of the local stream according to what server thinks is our state - * @param {Object} meeting - the meeting object - * @param {ServerMuteReason} serverMuteReason - reason why we're applying server mute to the local stream - * @returns {void} - */ - private applyServerMuteToLocalStream(meeting: any, serverMuteReason: ServerMuteReason) { - const muted = this.state.server.localMute || this.state.server.remoteMute; - - // update the local stream mute state, but not this.state.client.localMute - this.muteLocalStream(meeting, muted, serverMuteReason); - } - /** Applies the current value for unmute allowed to the underlying stream * * @param {Meeting} meeting @@ -371,7 +379,7 @@ export class MuteState { } if (muted !== undefined) { this.state.server.remoteMute = muted; - this.applyServerMuteToLocalStream(meeting, 'remotelyMuted'); + this.muteLocalStream(meeting, muted, 'remotelyMuted'); } } @@ -383,7 +391,7 @@ export class MuteState { * @param {Object} [meeting] the meeting object * @returns {undefined} */ - public handleServerLocalUnmuteRequired(meeting?: object) { + public handleServerLocalUnmuteRequired(meeting?: any) { if (!this.state.client.enabled) { LoggerProxy.logger.warn( `Meeting:muteState#handleServerLocalUnmuteRequired --> ${this.type}: localAudioUnmuteRequired received while ${this.type} is disabled -> local unmute will not result in ${this.type} being sent` @@ -396,9 +404,15 @@ export class MuteState { // todo: I'm seeing "you can now unmute yourself " popup when this happens - but same thing happens on web.w.c so we can ignore for now this.state.server.remoteMute = false; - this.state.client.localMute = false; - this.applyClientStateLocally(meeting, 'localUnmuteRequired'); + // change user mute state to false, but keep localMute true if overall mute state is still true + this.muteLocalStream(meeting, false, 'localUnmuteRequired'); + if (this.type === AUDIO) { + this.state.client.localMute = meeting.mediaProperties.audioStream?.muted; + } else { + this.state.client.localMute = meeting.mediaProperties.videoStream?.muted; + } + this.applyClientStateToServer(meeting); } diff --git a/packages/@webex/plugin-meetings/test/integration/spec/journey.js b/packages/@webex/plugin-meetings/test/integration/spec/journey.js index 36938703f2d..57355d68a9d 100644 --- a/packages/@webex/plugin-meetings/test/integration/spec/journey.js +++ b/packages/@webex/plugin-meetings/test/integration/spec/journey.js @@ -432,11 +432,11 @@ skipInNode(describe)('plugin-meetings', () => { {scope: bob.meeting.members, event: 'members:update', match: checkEvent}, ]); - localStreams.alice.microphone.setMuted(true); + localStreams.alice.microphone.setUserMuted(true); await membersUpdate; - assert.equal(localStreams.alice.microphone.muted, true); + assert.equal(localStreams.alice.microphone.userMuted, true); }); it('alice Audio unMute ', async () => { @@ -451,11 +451,11 @@ skipInNode(describe)('plugin-meetings', () => { {scope: bob.meeting.members, event: 'members:update', match: checkEvent}, ]); - localStreams.alice.microphone.setMuted(false); + localStreams.alice.microphone.setUserMuted(false); await membersUpdate; - assert.equal(localStreams.alice.microphone.muted, false); + assert.equal(localStreams.alice.microphone.userMuted, false); }); it('alice video mute', async () => { @@ -470,11 +470,11 @@ skipInNode(describe)('plugin-meetings', () => { {scope: bob.meeting.members, event: 'members:update', match: checkEvent}, ]); - localStreams.alice.camera.setMuted(true); + localStreams.alice.camera.setUserMuted(true); await membersUpdate; - assert.equal(localStreams.alice.camera.muted, true); + assert.equal(localStreams.alice.camera.userMuted, true); }); it('alice video unmute', async () => { @@ -489,11 +489,11 @@ skipInNode(describe)('plugin-meetings', () => { {scope: bob.meeting.members, event: 'members:update', match: checkEvent}, ]); - localStreams.alice.camera.setMuted(false); + localStreams.alice.camera.setUserMuted(false); await membersUpdate; - assert.equal(localStreams.alice.camera.muted, false); + assert.equal(localStreams.alice.camera.userMuted, false); }); it('alice update Audio', async () => { @@ -574,11 +574,11 @@ skipInNode(describe)('plugin-meetings', () => { ]); // first bob mutes himself - localStreams.bob.microphone.setMuted(true); + localStreams.bob.microphone.setUserMuted(true); await membersUpdate; - assert.equal(localStreams.bob.microphone.muted, true); + assert.equal(localStreams.bob.microphone.userMuted, true); // now alice tries to unmmute bob await testUtils.delayedPromise(alice.meeting.mute(bob.meeting.members.selfId, false)) @@ -593,7 +593,7 @@ skipInNode(describe)('plugin-meetings', () => { assert.fail('bob received unexpected meeting:self:unmutedByOthers event'); }) .catch(() => { - assert.equal(localStreams.bob.microphone.muted, true); + assert.equal(localStreams.bob.microphone.userMuted, true); }); }); @@ -607,11 +607,11 @@ skipInNode(describe)('plugin-meetings', () => { {scope: alice.meeting.members, event: 'members:update', match: checkEvent}, ]); - localStreams.bob.microphone.setMuted(false); + localStreams.bob.microphone.setUserMuted(false); await membersUpdate; - assert.equal(localStreams.bob.microphone.muted, false); + assert.equal(localStreams.bob.microphone.userMuted, false); }); it('alice shares the screen with highFrameRate', async () => { diff --git a/packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js b/packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js index f3749c66d80..267cf61fe47 100644 --- a/packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js +++ b/packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js @@ -43,7 +43,7 @@ import { RemoteTrackType, MediaType, } from '@webex/internal-media-core'; -import {StreamEventNames} from '@webex/media-helpers'; +import {LocalStreamEventNames} from '@webex/media-helpers'; import * as StatsAnalyzerModule from '@webex/plugin-meetings/src/statsAnalyzer'; import EventsScope from '@webex/plugin-meetings/src/common/events/events-scope'; import Meetings, {CONSTANTS} from '@webex/plugin-meetings'; @@ -1548,7 +1548,6 @@ describe('plugin-meetings', () => { describe('#addMedia', () => { const muteStateStub = { handleClientRequest: sinon.stub().returns(Promise.resolve(true)), - applyClientStateLocally: sinon.stub().returns(Promise.resolve(true)), }; let fakeMediaConnection; @@ -3028,9 +3027,13 @@ describe('plugin-meetings', () => { getSettings: sinon.stub().returns({ deviceId: 'some device id', }), - muted: false, + userMuted: false, + systemMuted: false, + get muted() { + return this.userMuted || this.systemMuted; + }, setUnmuteAllowed: sinon.stub(), - setMuted: sinon.stub(), + setUserMuted: sinon.stub(), setServerMuted: sinon.stub(), outputStream: { getTracks: () => { @@ -3314,7 +3317,7 @@ describe('plugin-meetings', () => { } }; - it('addMedia() works correctly when media is enabled without tracks to publish', async () => { + it('addMedia() works correctly when media is enabled without streams to publish', async () => { await meeting.addMedia(); await simulateRoapOffer(); await simulateRoapOk(); @@ -3380,8 +3383,42 @@ describe('plugin-meetings', () => { assert.calledTwice(locusMediaRequestStub); }); - it('addMedia() works correctly when media is enabled with tracks to publish and track is muted', async () => { - fakeMicrophoneStream.muted = true; + it('addMedia() works correctly when media is enabled with streams to publish and stream is user muted', async () => { + fakeMicrophoneStream.userMuted = true; + + await meeting.addMedia({localStreams: {microphone: fakeMicrophoneStream}}); + await simulateRoapOffer(); + await simulateRoapOk(); + + // check RoapMediaConnection was created correctly + checkMediaConnectionCreated({ + mediaConnectionConfig: expectedMediaConnectionConfig, + localStreams: { + audio: fakeMicrophoneStream, + video: undefined, + screenShareVideo: undefined, + screenShareAudio: undefined, + }, + direction: { + audio: 'sendrecv', + video: 'sendrecv', + screenShare: 'recvonly', + }, + remoteQualityLevel: 'HIGH', + expectedDebugId, + meetingId: meeting.id, + }); + // and SDP offer was sent with the right audioMuted/videoMuted values + checkSdpOfferSent({audioMuted: true, videoMuted: true}); + // check OK was sent with the right audioMuted/videoMuted values + checkOkSent({audioMuted: true, videoMuted: true}); + + // and that these were the only /media requests that were sent + assert.calledTwice(locusMediaRequestStub); + }); + + it('addMedia() works correctly when media is enabled with streams to publish and stream is system muted', async () => { + fakeMicrophoneStream.systemMuted = true; await meeting.addMedia({localStreams: {microphone: fakeMicrophoneStream}}); await simulateRoapOffer(); @@ -3414,7 +3451,7 @@ describe('plugin-meetings', () => { assert.calledTwice(locusMediaRequestStub); }); - it('addMedia() works correctly when media is disabled with tracks to publish', async () => { + it('addMedia() works correctly when media is disabled with streams to publish', async () => { await meeting.addMedia({ localStreams: {microphone: fakeMicrophoneStream}, audioEnabled: false, @@ -3450,7 +3487,7 @@ describe('plugin-meetings', () => { assert.calledTwice(locusMediaRequestStub); }); - it('addMedia() works correctly when media is disabled with no tracks to publish', async () => { + it('addMedia() works correctly when media is disabled with no streams to publish', async () => { await meeting.addMedia({audioEnabled: false}); await simulateRoapOffer(); await simulateRoapOk(); @@ -3483,7 +3520,7 @@ describe('plugin-meetings', () => { assert.calledTwice(locusMediaRequestStub); }); - it('addMedia() works correctly when video is disabled with no tracks to publish', async () => { + it('addMedia() works correctly when video is disabled with no streams to publish', async () => { await meeting.addMedia({videoEnabled: false}); await simulateRoapOffer(); await simulateRoapOk(); @@ -3516,7 +3553,7 @@ describe('plugin-meetings', () => { assert.calledTwice(locusMediaRequestStub); }); - it('addMedia() works correctly when screen share is disabled with no tracks to publish', async () => { + it('addMedia() works correctly when screen share is disabled with no streams to publish', async () => { await meeting.addMedia({shareAudioEnabled: false, shareVideoEnabled: false}); await simulateRoapOffer(); await simulateRoapOk(); @@ -3617,9 +3654,13 @@ describe('plugin-meetings', () => { const fakeMicrophoneStream2 = { on: sinon.stub(), off: sinon.stub(), - muted: false, + userMuted: false, + systemMuted: false, + get muted() { + return this.userMuted || this.systemMuted; + }, setUnmuteAllowed: sinon.stub(), - setMuted: sinon.stub(), + setUserMuted: sinon.stub(), outputStream: { getTracks: () => { return [ @@ -3856,12 +3897,55 @@ describe('plugin-meetings', () => { }); [ - {mute: true, title: 'muting a track before confluence is created'}, - {mute: false, title: 'unmuting a track before confluence is created'}, + {mute: true, title: 'user muting a track before confluence is created'}, + {mute: false, title: 'user unmuting a track before confluence is created'}, + ].forEach(({mute, title}) => + it(title, async () => { + // initialize the microphone mute state to opposite of what we do in the test + fakeMicrophoneStream.userMuted = !mute; + + await meeting.addMedia({localStreams: {microphone: fakeMicrophoneStream}}); + await stableState(); + + resetHistory(); + + assert.equal( + fakeMicrophoneStream.on.getCall(0).args[0], + LocalStreamEventNames.UserMuteStateChange + ); + const mutedListener = fakeMicrophoneStream.on.getCall(0).args[1]; + // simulate track being muted + fakeMicrophoneStream.userMuted = mute; + mutedListener(mute); + + await stableState(); + + // nothing should happen + assert.notCalled(locusMediaRequestStub); + assert.notCalled(fakeRoapMediaConnection.update); + + // now simulate roap offer and ok + await simulateRoapOffer(); + await simulateRoapOk(); + + // it should be sent with the right mute status + checkSdpOfferSent({audioMuted: mute, videoMuted: true}); + // check OK was sent with the right audioMuted/videoMuted values + checkOkSent({audioMuted: mute, videoMuted: true}); + + // nothing else should happen + assert.calledTwice(locusMediaRequestStub); + assert.notCalled(fakeRoapMediaConnection.update); + }) + ); + + [ + {mute: true, title: 'system muting a track before confluence is created'}, + {mute: false, title: 'system unmuting a track before confluence is created'}, ].forEach(({mute, title}) => it(title, async () => { // initialize the microphone mute state to opposite of what we do in the test - fakeMicrophoneStream.muted = !mute; + fakeMicrophoneStream.systemMuted = !mute; await meeting.addMedia({localStreams: {microphone: fakeMicrophoneStream}}); await stableState(); @@ -3870,10 +3954,11 @@ describe('plugin-meetings', () => { assert.equal( fakeMicrophoneStream.on.getCall(0).args[0], - StreamEventNames.MuteStateChange + LocalStreamEventNames.UserMuteStateChange ); const mutedListener = fakeMicrophoneStream.on.getCall(0).args[1]; // simulate track being muted + fakeMicrophoneStream.systemMuted = mute; mutedListener(mute); await stableState(); @@ -6408,11 +6493,11 @@ describe('plugin-meetings', () => { meeting.sendSlotManager.setNamedMediaGroups = sinon.stub().returns(undefined); }); it('should throw error if not audio type', () => { - expect(() => meeting.setSendNamedMediaGroup(MediaType.VideoMain, 20)).to.throw(`cannot set send named media group which media type is ${MediaType.VideoMain}`) - + expect(() => meeting.setSendNamedMediaGroup(MediaType.VideoMain, 20)).to.throw( + `cannot set send named media group which media type is ${MediaType.VideoMain}` + ); }); it('fails if there is no media connection', () => { - meeting.mediaProperties.webrtcMediaConnection = undefined; meeting.setSendNamedMediaGroup('AUDIO-MAIN', 20); assert.notCalled(meeting.sendSlotManager.setNamedMediaGroups); @@ -6421,8 +6506,10 @@ describe('plugin-meetings', () => { it('success if there is media connection', () => { meeting.isMultistream = true; meeting.mediaProperties.webrtcMediaConnection = true; - meeting.setSendNamedMediaGroup("AUDIO-MAIN", 20); - assert.calledOnceWithExactly(meeting.sendSlotManager.setNamedMediaGroups, "AUDIO-MAIN", [{type: 1, value: 20}]); + meeting.setSendNamedMediaGroup('AUDIO-MAIN', 20); + assert.calledOnceWithExactly(meeting.sendSlotManager.setNamedMediaGroups, 'AUDIO-MAIN', [ + {type: 1, value: 20}, + ]); }); }); diff --git a/packages/@webex/plugin-meetings/test/unit/spec/meeting/muteState.js b/packages/@webex/plugin-meetings/test/unit/spec/meeting/muteState.js index 403ce7b723b..217a265a708 100644 --- a/packages/@webex/plugin-meetings/test/unit/spec/meeting/muteState.js +++ b/packages/@webex/plugin-meetings/test/unit/spec/meeting/muteState.js @@ -15,21 +15,25 @@ describe('plugin-meetings', () => { const fakeLocus = {info: 'this is a fake locus'}; - const createFakeLocalStream = (id, muted) => { + const createFakeLocalStream = (id, userMuted, systemMuted) => { return { id, setServerMuted: sinon.stub(), setUnmuteAllowed: sinon.stub(), - setMuted: sinon.stub(), - muted, + setUserMuted: sinon.stub(), + userMuted, + systemMuted, + get muted() { + return this.userMuted || this.systemMuted; + }, }; }; beforeEach(async () => { meeting = { mediaProperties: { - audioStream: createFakeLocalStream('fake audio stream', false), - videoStream: createFakeLocalStream('fake video stream', false), + audioStream: createFakeLocalStream('fake audio stream', false, false), + videoStream: createFakeLocalStream('fake video stream', false, false), }, remoteMuted: false, unmuteAllowed: true, @@ -109,9 +113,9 @@ describe('plugin-meetings', () => { assert.isTrue(audio.isRemotelyMuted()); }); - it('does local unmute if localAudioUnmuteRequired is received', async () => { - // first we need to mute have the local stream muted - meeting.mediaProperties.audioStream.muted = true; + it('does local audio unmute if localAudioUnmuteRequired is received', async () => { + // first we need to have the local stream user muted + meeting.mediaProperties.audioStream.userMuted = true; audio.handleLocalStreamChange(meeting); assert.isTrue(audio.isMuted()); @@ -119,8 +123,12 @@ describe('plugin-meetings', () => { MeetingUtil.remoteUpdateAudioVideo.resetHistory(); // now simulate server requiring us to locally unmute + // assuming setServerMuted succeeds at updating userMuted + meeting.mediaProperties.audioStream.setServerMuted = sinon.stub().callsFake((muted) => { + meeting.mediaProperties.audioStream.userMuted = muted; + }); audio.handleServerLocalUnmuteRequired(meeting); - + await testUtils.flushPromises(); // check that local stream was unmuted @@ -137,9 +145,41 @@ describe('plugin-meetings', () => { assert.isFalse(audio.isMuted()); }); + it('handles localAudioUnmuteRequired but does not send unmute to server if system is muted', async () => { + // first we need to have the local stream user muted and system muted + meeting.mediaProperties.audioStream.userMuted = true; + meeting.mediaProperties.audioStream.systemMuted = true; + audio.handleLocalStreamChange(meeting); + + assert.isTrue(audio.isMuted()); + + MeetingUtil.remoteUpdateAudioVideo.resetHistory(); + + // now simulate server requiring us to locally unmute + // assuming setServerMuted succeeds at updating userMuted + meeting.mediaProperties.audioStream.setServerMuted = sinon.stub().callsFake((muted) => { + meeting.mediaProperties.audioStream.userMuted = muted; + }); + audio.handleServerLocalUnmuteRequired(meeting); + + await testUtils.flushPromises(); + + // check that local stream was unmuted + assert.calledWith( + meeting.mediaProperties.audioStream.setServerMuted, + false, + 'localUnmuteRequired' + ); + + // system was muted so local unmute was not sent to server + assert.notCalled(MeetingUtil.remoteUpdateAudioVideo); + + assert.isTrue(audio.isMuted()); + }); + it('does local video unmute if localVideoUnmuteRequired is received', async () => { - // first we need to mute - meeting.mediaProperties.videoStream.muted = true; + // first we need to have the local stream user muted + meeting.mediaProperties.videoStream.userMuted = true; video.handleLocalStreamChange(meeting); assert.isTrue(video.isMuted()); @@ -147,7 +187,12 @@ describe('plugin-meetings', () => { MeetingUtil.remoteUpdateAudioVideo.resetHistory(); // now simulate server requiring us to locally unmute + // assuming setServerMuted succeeds at updating userMuted + meeting.mediaProperties.videoStream.setServerMuted = sinon.stub().callsFake((muted) => { + meeting.mediaProperties.videoStream.userMuted = muted; + }); video.handleServerLocalUnmuteRequired(meeting); + await testUtils.flushPromises(); // check that local stream was unmuted @@ -164,11 +209,43 @@ describe('plugin-meetings', () => { assert.isFalse(video.isMuted()); }); + it('handles localVideoUnmuteRequired but does not send unmute to server if system is muted', async () => { + // first we need to have the local stream user muted and system muted + meeting.mediaProperties.videoStream.userMuted = true; + meeting.mediaProperties.videoStream.systemMuted = true; + audio.handleLocalStreamChange(meeting); + + assert.isTrue(video.isMuted()); + + MeetingUtil.remoteUpdateAudioVideo.resetHistory(); + + // now simulate server requiring us to locally unmute + // assuming setServerMuted succeeds at updating userMuted + meeting.mediaProperties.videoStream.setServerMuted = sinon.stub().callsFake((muted) => { + meeting.mediaProperties.videoStream.userMuted = muted; + }); + video.handleServerLocalUnmuteRequired(meeting); + + await testUtils.flushPromises(); + + // check that local stream was unmuted + assert.calledWith( + meeting.mediaProperties.videoStream.setServerMuted, + false, + 'localUnmuteRequired' + ); + + // system was muted so local unmute was not sent to server + assert.notCalled(MeetingUtil.remoteUpdateAudioVideo); + + assert.isTrue(video.isMuted()); + }); + describe('#isLocallyMuted()', () => { it('does not consider remote mute status for audio', async () => { // simulate being already remote muted and locally unmuted meeting.remoteMuted = true; - meeting.mediaProperties.audioStream.muted = false; + meeting.mediaProperties.audioStream.userMuted = false; // create a new MuteState instance audio = createMuteState(AUDIO, meeting, true); @@ -182,7 +259,7 @@ describe('plugin-meetings', () => { it('does not consider remote mute status for video', async () => { // simulate being already remote muted meeting.remoteVideoMuted = true; - meeting.mediaProperties.videoStream.muted = false; + meeting.mediaProperties.videoStream.userMuted = false; // create a new MuteState instance video = createMuteState(VIDEO, meeting, true); @@ -202,27 +279,44 @@ describe('plugin-meetings', () => { await testUtils.flushPromises(); }); - const simulateAudioMuteChange = async (muteValue) => { - meeting.mediaProperties.audioStream.muted = muteValue; - audio.handleLocalStreamMuteStateChange(meeting, muteValue); + const simulateAudioUserMuteChange = async (muteValue) => { + meeting.mediaProperties.audioStream.userMuted = muteValue; + audio.handleLocalStreamMuteStateChange(meeting); + + await testUtils.flushPromises(); + }; + + const simulateAudioSystemMuteChange = async (muteValue) => { + meeting.mediaProperties.audioStream.systemMuted = muteValue; + audio.handleLocalStreamMuteStateChange(meeting); await testUtils.flushPromises(); }; - const simulateVideoMuteChange = async (muteValue) => { - meeting.mediaProperties.videoStream.muted = muteValue; - video.handleLocalStreamMuteStateChange(meeting, muteValue); + const simulateVideoUserMuteChange = async (muteValue) => { + meeting.mediaProperties.videoStream.userMuted = muteValue; + video.handleLocalStreamMuteStateChange(meeting); await testUtils.flushPromises(); }; - it('returns correct value in isMuted() methods after local stream is muted/unmuted', async () => { + it('returns correct value in isMuted() methods after local stream is user muted/unmuted', async () => { // mute - await simulateAudioMuteChange(true); + await simulateAudioUserMuteChange(true); assert.isTrue(audio.isMuted()); // unmute - await simulateAudioMuteChange(false); + await simulateAudioUserMuteChange(false); + assert.isFalse(audio.isMuted()); + }); + + it('returns correct value in isMuted() methods after local stream is system muted/unmuted', async () => { + // mute + await simulateAudioSystemMuteChange(true); + assert.isTrue(audio.isMuted()); + + // unmute + await simulateAudioSystemMuteChange(false); assert.isFalse(audio.isMuted()); }); @@ -231,7 +325,7 @@ describe('plugin-meetings', () => { audio.handleServerRemoteMuteUpdate(meeting, true, true); // unmute - await simulateAudioMuteChange(false); + await simulateAudioUserMuteChange(false); // check that remote unmute was sent to server assert.calledOnce(meeting.members.muteMember); @@ -245,7 +339,7 @@ describe('plugin-meetings', () => { video.handleServerRemoteMuteUpdate(meeting, true, true); // unmute - await simulateVideoMuteChange(false); + await simulateVideoUserMuteChange(false); // check that remote unmute was sent to server assert.calledOnce(meeting.members.muteMember); @@ -259,7 +353,7 @@ describe('plugin-meetings', () => { video.handleServerRemoteMuteUpdate(meeting, false, true); // unmute - await simulateVideoMuteChange(false); + await simulateVideoUserMuteChange(false); // check that remote unmute was not sent to server assert.notCalled(meeting.members.muteMember); @@ -270,7 +364,7 @@ describe('plugin-meetings', () => { it('calls setServerMuted with "clientRequestFailed" when server request for local mute fails', async () => { MeetingUtil.remoteUpdateAudioVideo = sinon.stub().rejects(new Error('fake error')); - await simulateAudioMuteChange(true); + await simulateAudioUserMuteChange(true); assert.calledOnceWithExactly( meeting.mediaProperties.audioStream.setServerMuted, @@ -289,7 +383,7 @@ describe('plugin-meetings', () => { meeting.members.muteMember = sinon.stub().rejects(); meeting.mediaProperties.audioStream.setServerMuted.resetHistory(); - await simulateAudioMuteChange(false); + await simulateAudioUserMuteChange(false); assert.calledOnceWithExactly( meeting.mediaProperties.audioStream.setServerMuted, @@ -313,11 +407,11 @@ describe('plugin-meetings', () => { // the stream is initially unmuted // simulate many mute changes with the last one matching the first one - await simulateAudioMuteChange(true); - await simulateAudioMuteChange(false); - await simulateAudioMuteChange(true); - await simulateAudioMuteChange(false); - await simulateAudioMuteChange(true); + await simulateAudioUserMuteChange(true); + await simulateAudioUserMuteChange(false); + await simulateAudioUserMuteChange(true); + await simulateAudioUserMuteChange(false); + await simulateAudioUserMuteChange(true); // so far there should have been only 1 request to server (because our stub hasn't resolved yet // and MuteState sends only 1 server request at a time) @@ -342,8 +436,8 @@ describe('plugin-meetings', () => { ); // 2 client requests, one after another without waiting for first one to resolve - await simulateAudioMuteChange(true); - await simulateAudioMuteChange(false); + await simulateAudioUserMuteChange(true); + await simulateAudioUserMuteChange(false); assert.calledOnce(MeetingUtil.remoteUpdateAudioVideo); assert.calledWith(MeetingUtil.remoteUpdateAudioVideo, meeting, true, undefined); @@ -364,7 +458,7 @@ describe('plugin-meetings', () => { it('does not send remote mute for video', async () => { // mute - await simulateVideoMuteChange(true); + await simulateVideoUserMuteChange(true); assert.isTrue(video.isMuted()); @@ -376,7 +470,7 @@ describe('plugin-meetings', () => { meeting.members.muteMember.resetHistory(); // unmute - await simulateVideoMuteChange(false); + await simulateVideoUserMuteChange(false); assert.isFalse(video.isMuted()); @@ -390,22 +484,22 @@ describe('plugin-meetings', () => { meeting.video = video; // mute audio -> the call to remoteUpdateAudioVideo should have video undefined - await simulateAudioMuteChange(true); + await simulateAudioUserMuteChange(true); assert.calledWith(MeetingUtil.remoteUpdateAudioVideo, meeting, true, undefined); MeetingUtil.remoteUpdateAudioVideo.resetHistory(); // now mute video -> the call to remoteUpdateAudioVideo should have unmute for video and undefined for audio - await simulateVideoMuteChange(true); + await simulateVideoUserMuteChange(true); assert.calledWith(MeetingUtil.remoteUpdateAudioVideo, meeting, undefined, true); MeetingUtil.remoteUpdateAudioVideo.resetHistory(); // now unmute the audio -> the call to remoteUpdateAudioVideo should have video undefined - await simulateAudioMuteChange(false); + await simulateAudioUserMuteChange(false); assert.calledWith(MeetingUtil.remoteUpdateAudioVideo, meeting, false, undefined); MeetingUtil.remoteUpdateAudioVideo.resetHistory(); // unmute video -> the call to remoteUpdateAudioVideo should have audio undefined - await simulateVideoMuteChange(false); + await simulateVideoUserMuteChange(false); assert.calledWith(MeetingUtil.remoteUpdateAudioVideo, meeting, undefined, false); }); }); @@ -414,12 +508,13 @@ describe('plugin-meetings', () => { let meeting; let muteState; let setServerMutedSpy; - let setMutedSpy, setUnmuteAllowedSpy; + let setUserMutedSpy, setUnmuteAllowedSpy; const setupMeeting = ( mediaType, remoteMuted = false, - muted = false, + userMuted = false, + systemMuted = false, defineStreams = true ) => { const remoteMuteField = mediaType === AUDIO ? 'remoteMuted' : 'remoteVideoMuted'; @@ -427,10 +522,10 @@ describe('plugin-meetings', () => { meeting = { mediaProperties: { audioStream: defineStreams - ? createFakeLocalStream('fake audio stream', muted) + ? createFakeLocalStream('fake audio stream', userMuted, systemMuted) : undefined, videoStream: defineStreams - ? createFakeLocalStream('fake video stream', muted) + ? createFakeLocalStream('fake video stream', userMuted, systemMuted) : undefined, }, [remoteMuteField]: remoteMuted, @@ -447,8 +542,14 @@ describe('plugin-meetings', () => { }; }; - const setup = async (mediaType, remoteMuted = false, muted = false, defineStreams = true) => { - setupMeeting(mediaType, remoteMuted, muted, defineStreams); + const setup = async ( + mediaType, + remoteMuted = false, + userMuted = false, + systemMuted = false, + defineStreams = true + ) => { + setupMeeting(mediaType, remoteMuted, userMuted, systemMuted, defineStreams); muteState = createMuteState(mediaType, meeting, true); muteState.handleLocalStreamChange(meeting); @@ -467,10 +568,10 @@ describe('plugin-meetings', () => { mediaType === AUDIO ? meeting.mediaProperties.audioStream?.setServerMuted : meeting.mediaProperties.videoStream?.setServerMuted; - setMutedSpy = + setUserMutedSpy = mediaType === AUDIO - ? meeting.mediaProperties.audioStream?.setMuted - : meeting.mediaProperties.videoStream?.setMuted; + ? meeting.mediaProperties.audioStream?.setUserMuted + : meeting.mediaProperties.videoStream?.setUserMuted; clearSpies(); }; @@ -478,8 +579,25 @@ describe('plugin-meetings', () => { const clearSpies = () => { setUnmuteAllowedSpy?.resetHistory(); setServerMutedSpy?.resetHistory(); - setMutedSpy?.resetHistory(); + setUserMutedSpy?.resetHistory(); }; + + const simulateUserMute = (mediaType, mute) => { + if (mediaType === AUDIO) { + meeting.mediaProperties.audioStream.userMuted = mute; + } else { + meeting.mediaProperties.videoStream.userMuted = mute; + } + }; + + const simulateSystemMute = (mediaType, mute) => { + if (mediaType === AUDIO) { + meeting.mediaProperties.audioStream.systemMuted = mute; + } else { + meeting.mediaProperties.videoStream.systemMuted = mute; + } + }; + const tests = [ {mediaType: AUDIO, title: 'audio'}, {mediaType: VIDEO, title: 'video'}, @@ -513,16 +631,17 @@ describe('plugin-meetings', () => { const setupWithoutInit = async ( mediaType, remoteMuted = false, - muted = false, + userMuted = false, + systemMuted = false, defineStreams = true ) => { - setupMeeting(mediaType, remoteMuted, muted, defineStreams); + setupMeeting(mediaType, remoteMuted, userMuted, systemMuted, defineStreams); muteState = new MuteState(mediaType, meeting, true); }; it('nothing goes bad when stream is undefined', async () => { - await setupWithoutInit(mediaType, false, false, false); + await setupWithoutInit(mediaType, false, false, false, false); setupSpies(mediaType); muteState.init(meeting); @@ -530,8 +649,20 @@ describe('plugin-meetings', () => { assert.isTrue(muteState.state.client.localMute); }); - it('tests when stream muted is true and remoteMuted is false', async () => { - await setupWithoutInit(mediaType, false, true); + it('tests when stream user muted is true and remoteMuted is false', async () => { + await setupWithoutInit(mediaType, false, true, false, true); + setupSpies(mediaType); + + muteState.init(meeting); + + assert.calledWith(setUnmuteAllowedSpy, muteState.state.server.unmuteAllowed); + assert.notCalled(setServerMutedSpy); + assert.notCalled(MeetingUtil.remoteUpdateAudioVideo); + assert.isTrue(muteState.state.client.localMute); + }); + + it('tests when stream system muted is true and remoteMuted is false', async () => { + await setupWithoutInit(mediaType, false, false, true, true); setupSpies(mediaType); muteState.init(meeting); @@ -542,8 +673,8 @@ describe('plugin-meetings', () => { assert.isTrue(muteState.state.client.localMute); }); - it('tests when stream muted is false and remoteMuted is false', async () => { - await setupWithoutInit(mediaType, false, false); + it('tests when both stream user muted and system muted are false and remoteMuted is false', async () => { + await setupWithoutInit(mediaType, false, false, false, true); setupSpies(mediaType); muteState.init(meeting); @@ -556,7 +687,7 @@ describe('plugin-meetings', () => { it('tests when remoteMuted is true', async () => { // testing that muteLocalStream is called - await setupWithoutInit(mediaType, true); + await setupWithoutInit(mediaType, true, false, false, true); setupSpies(mediaType); muteState.init(meeting); @@ -568,27 +699,48 @@ describe('plugin-meetings', () => { describe('#handleLocalStreamMuteStateChange', () => { it('checks when ignoreMuteStateChange is true nothing changes', async () => { - await setup(mediaType, false, false); + await setup(mediaType, false, false, false, true); muteState.ignoreMuteStateChange = true; - muteState.handleLocalStreamMuteStateChange(meeting, true); + simulateUserMute(mediaType, true); + muteState.handleLocalStreamMuteStateChange(meeting); assert.notCalled(MeetingUtil.remoteUpdateAudioVideo); assert.isFalse(muteState.state.client.localMute); }); - it('tests localMute - true to false', async () => { - await setup(mediaType, false, true); + it('tests localMute - user mute from true to false', async () => { + await setup(mediaType, false, true, false, true); + + simulateUserMute(mediaType, false); + muteState.handleLocalStreamMuteStateChange(meeting); + assert.equal(muteState.state.client.localMute, false); + assert.called(MeetingUtil.remoteUpdateAudioVideo); + }); + + it('tests localMute - user mute from false to true', async () => { + await setup(mediaType, false, false, false, true); + + simulateUserMute(mediaType, true); + muteState.handleLocalStreamMuteStateChange(meeting); + assert.equal(muteState.state.client.localMute, true); + assert.called(MeetingUtil.remoteUpdateAudioVideo); + }); + + it('tests localMute - system mute from true to false', async () => { + await setup(mediaType, false, false, true, true); - muteState.handleLocalStreamMuteStateChange(meeting, false); + simulateSystemMute(mediaType, false); + muteState.handleLocalStreamMuteStateChange(meeting); assert.equal(muteState.state.client.localMute, false); assert.called(MeetingUtil.remoteUpdateAudioVideo); }); - it('tests localMute - false to true', async () => { - await setup(mediaType, false, false); + it('tests localMute - system mute from false to true', async () => { + await setup(mediaType, false, false, false, true); - muteState.handleLocalStreamMuteStateChange(meeting, true); + simulateSystemMute(mediaType, true); + muteState.handleLocalStreamMuteStateChange(meeting); assert.equal(muteState.state.client.localMute, true); assert.called(MeetingUtil.remoteUpdateAudioVideo); }); @@ -609,7 +761,7 @@ describe('plugin-meetings', () => { muteState.state.client.localMute, 'somereason' ); - assert.notCalled(setMutedSpy); + assert.notCalled(setUserMutedSpy); }); it('nothing explodes when streams are undefined', async () => { diff --git a/yarn.lock b/yarn.lock index 496840ee044..6f54598857f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1621,11 +1621,11 @@ __metadata: linkType: hard "@babel/runtime@npm:^7.18.9": - version: 7.23.8 - resolution: "@babel/runtime@npm:7.23.8" + version: 7.24.1 + resolution: "@babel/runtime@npm:7.24.1" dependencies: regenerator-runtime: "npm:^0.14.0" - checksum: 0bd5543c26811153822a9f382fd39886f66825ff2a397a19008011376533747cd05c33a91f6248c0b8b0edf0448d7c167ebfba34786088f1b7eb11c65be7dfc3 + checksum: 5c8f3b912ba949865f03b3cf8395c60e1f4ebd1033fbd835bdfe81b6cac8a87d85bc3c7aded5fcdf07be044c9ab8c818f467abe0deca50020c72496782639572 languageName: node linkType: hard @@ -5264,11 +5264,11 @@ __metadata: linkType: hard "@types/node@npm:^18.7.6": - version: 18.19.7 - resolution: "@types/node@npm:18.19.7" + version: 18.19.28 + resolution: "@types/node@npm:18.19.28" dependencies: undici-types: "npm:~5.26.4" - checksum: f1241173a66ec7761a0c1e30deb3ba74c507ba06539e2ebd64d37bc62f4b8af420eef8e0db9487d900afeb9a4bdef72764729fed9742e184f2c743b2bc70bfab + checksum: b74f2b46c7732ed9eccd70548522700474384fc8bf4fafa0373c25483a4930a7e7ebdba7f481b339483857a673e1f89f5cb0ecfc2a36551993cdf1846fe26dc7 languageName: node linkType: hard @@ -6914,19 +6914,19 @@ __metadata: languageName: node linkType: hard -"@webex/internal-media-core@npm:2.2.9": - version: 2.2.9 - resolution: "@webex/internal-media-core@npm:2.2.9" +"@webex/internal-media-core@npm:2.3.0": + version: 2.3.0 + resolution: "@webex/internal-media-core@npm:2.3.0" dependencies: "@babel/runtime": "npm:^7.18.9" "@webex/ts-sdp": "npm:1.6.0" - "@webex/web-client-media-engine": "npm:v3.11.8-hf-duplicateMsid.1" + "@webex/web-client-media-engine": "npm:3.14.1" events: "npm:^3.3.0" typed-emitter: "npm:^2.1.0" uuid: "npm:^8.3.2" webrtc-adapter: "npm:^8.1.2" xstate: "npm:^4.30.6" - checksum: d73597c7a9a492a6b71dc7bc429edd83ecb1493ebd956bad8b673a8613287c2e2b350a072e7657dc9824e3e1e5812b048eb15c607850b5332623e72f2e403127 + checksum: c29f611569ce1af447506677ce7fec1af47663f2dc9978d84a3cb53a4884f74972026fbd274d16e7797c418fd212a535cad911d31c2587c1383bb3919db90782 languageName: node linkType: hard @@ -7687,7 +7687,7 @@ __metadata: "@babel/preset-typescript": 7.22.11 "@webex/babel-config-legacy": "workspace:*" "@webex/eslint-config-legacy": "workspace:*" - "@webex/internal-media-core": 2.2.9 + "@webex/internal-media-core": 2.3.0 "@webex/jest-config-legacy": "workspace:*" "@webex/legacy-tools": "workspace:*" "@webex/test-helper-chai": "workspace:*" @@ -7695,6 +7695,7 @@ __metadata: "@webex/ts-events": ^1.1.0 "@webex/web-media-effects": ^2.15.6 eslint: ^8.24.0 + jsdom-global: 3.0.2 sinon: ^9.2.4 languageName: unknown linkType: soft @@ -7922,7 +7923,7 @@ __metadata: "@webex/babel-config-legacy": "workspace:*" "@webex/common": "workspace:*" "@webex/eslint-config-legacy": "workspace:*" - "@webex/internal-media-core": 2.2.9 + "@webex/internal-media-core": 2.3.0 "@webex/internal-plugin-conversation": "workspace:*" "@webex/internal-plugin-device": "workspace:*" "@webex/internal-plugin-llm": "workspace:*" @@ -8642,21 +8643,21 @@ __metadata: languageName: node linkType: hard -"@webex/web-client-media-engine@npm:v3.11.8-hf-duplicateMsid.1": - version: 3.11.8-hf-duplicateMsid.1 - resolution: "@webex/web-client-media-engine@npm:3.11.8-hf-duplicateMsid.1" +"@webex/web-client-media-engine@npm:3.14.1": + version: 3.14.1 + resolution: "@webex/web-client-media-engine@npm:3.14.1" dependencies: - "@webex/json-multistream": 2.1.3 - "@webex/rtcstats": ^1.1.2 - "@webex/ts-events": ^1.0.1 - "@webex/ts-sdp": 1.6.0 - "@webex/web-capabilities": ^1.1.1 - "@webex/webrtc-core": 2.4.0 - async: ^3.2.4 - js-logger: ^1.6.1 - typed-emitter: ^2.1.0 - uuid: ^8.3.2 - checksum: aa83f5214bcf3c3d0889d1b7aa40151dd81a3b894cb48445753a2a3f125793ac16daf3f64441e69f18e7b01de383c51823fe709a3c6b01863bf1e74848707dee + "@webex/json-multistream": "npm:2.1.3" + "@webex/rtcstats": "npm:^1.1.2" + "@webex/ts-events": "npm:^1.0.1" + "@webex/ts-sdp": "npm:1.6.0" + "@webex/web-capabilities": "npm:^1.1.1" + "@webex/webrtc-core": "npm:2.6.0" + async: "npm:^3.2.4" + js-logger: "npm:^1.6.1" + typed-emitter: "npm:^2.1.0" + uuid: "npm:^8.3.2" + checksum: d73a81e55b4a56351da848ccd3ffd0704d7bde1f6b5bb46aa07203fa4eaff0696fae22f97e2b0f730a5bdf5be8a8db999918e8a9ed2e0332bc002fe74ae8725b languageName: node linkType: hard @@ -8778,6 +8779,21 @@ __metadata: languageName: node linkType: hard +"@webex/webrtc-core@npm:2.6.0": + version: 2.6.0 + resolution: "@webex/webrtc-core@npm:2.6.0" + dependencies: + "@webex/ts-events": "npm:^1.1.0" + "@webex/web-capabilities": "npm:^1.1.0" + "@webex/web-media-effects": "npm:^2.15.6" + events: "npm:^3.3.0" + js-logger: "npm:^1.6.1" + typed-emitter: "npm:^2.1.0" + webrtc-adapter: "npm:^8.1.2" + checksum: d219ac9aaaf92ea6d2c3e60598af9862dc111a541eefff6554862e07f00f1f4887c1201908ac0ed76c4656afa60bb54c824869f457c2dbfb55739d5a27524528 + languageName: node + linkType: hard + "@webex/webrtc@workspace:packages/@webex/webrtc": version: 0.0.0-use.local resolution: "@webex/webrtc@workspace:packages/@webex/webrtc" From 4c0217c525a716569002f4a37be825a81e8d69b2 Mon Sep 17 00:00:00 2001 From: Catalin Date: Mon, 15 Apr 2024 16:09:05 +0100 Subject: [PATCH 15/20] feat(metrics): fix test failure due to circular dep --- packages/@webex/test-helper-mock-webex/src/index.js | 1 + .../@webex/webex-core/test/unit/spec/webex-internal-core.js | 2 ++ 2 files changed, 3 insertions(+) diff --git a/packages/@webex/test-helper-mock-webex/src/index.js b/packages/@webex/test-helper-mock-webex/src/index.js index 22a4c0c683c..6ab27becd19 100644 --- a/packages/@webex/test-helper-mock-webex/src/index.js +++ b/packages/@webex/test-helper-mock-webex/src/index.js @@ -298,6 +298,7 @@ function makeWebex(options) { submitClientEvent: sinon.stub(), callDiagnosticLatencies: { saveTimestamp: sinon.stub(), + measureLatency: (callback) => callback(), }, callDiagnosticMetrics: { submitClientEvent: sinon.stub(), diff --git a/packages/@webex/webex-core/test/unit/spec/webex-internal-core.js b/packages/@webex/webex-core/test/unit/spec/webex-internal-core.js index 5e3d5945b53..aa7638ac19d 100644 --- a/packages/@webex/webex-core/test/unit/spec/webex-internal-core.js +++ b/packages/@webex/webex-core/test/unit/spec/webex-internal-core.js @@ -5,6 +5,8 @@ import {assert} from '@webex/test-helper-chai'; import sinon from 'sinon'; import WebexCore, {WebexPlugin, registerInternalPlugin} from '@webex/webex-core'; +// TODO: fix circular dependency core->metrics->core https://jira-eng-gpk2.cisco.com/jira/browse/SPARK-515520 +require('@webex/internal-plugin-metrics'); describe('Webex', () => { describe('#internal', () => { From 3b2808d405435e33fa056983140d0a94f2238595 Mon Sep 17 00:00:00 2001 From: Kesava Krishnan Madavan Date: Mon, 15 Apr 2024 21:31:50 +0530 Subject: [PATCH 16/20] test(media-helpers): conflict due to media-helper importing jsdom/register --- packages/@webex/media-helpers/jest.config.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/@webex/media-helpers/jest.config.js b/packages/@webex/media-helpers/jest.config.js index ec4fc2e99b6..0e9d38b401c 100644 --- a/packages/@webex/media-helpers/jest.config.js +++ b/packages/@webex/media-helpers/jest.config.js @@ -1,6 +1,3 @@ const config = require('@webex/jest-config-legacy'); -module.exports = { - ...config, - testEnvironment: 'jsdom', -}; +module.exports = config; From a84876ab4be775279e325a01f54766e5c766879c Mon Sep 17 00:00:00 2001 From: Catalin Date: Tue, 16 Apr 2024 11:26:37 +0100 Subject: [PATCH 17/20] feat(metrics): fix unit test --- packages/@webex/webex-core/test/unit/spec/webex-core.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/@webex/webex-core/test/unit/spec/webex-core.js b/packages/@webex/webex-core/test/unit/spec/webex-core.js index 3a66b8dac1e..a83bb4422d5 100644 --- a/packages/@webex/webex-core/test/unit/spec/webex-core.js +++ b/packages/@webex/webex-core/test/unit/spec/webex-core.js @@ -7,6 +7,8 @@ import sinon from 'sinon'; import WebexCore, {MemoryStoreAdapter, registerPlugin, WebexPlugin} from '@webex/webex-core'; import {set} from 'lodash'; import {version} from '@webex/webex-core/package'; +// TODO: fix circular dependency core->metrics->core https://jira-eng-gpk2.cisco.com/jira/browse/SPARK-515520 +require('@webex/internal-plugin-metrics'); describe('Webex', () => { let webex; From 31e8fc00244739fcbba4bcfc4957864cae48241e Mon Sep 17 00:00:00 2001 From: Catalin Date: Tue, 16 Apr 2024 11:56:03 +0100 Subject: [PATCH 18/20] feat(metrics): fix other unit test in wb core --- .../test/unit/spec/interceptors/payload-transformer.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/@webex/webex-core/test/unit/spec/interceptors/payload-transformer.js b/packages/@webex/webex-core/test/unit/spec/interceptors/payload-transformer.js index 04008ced85f..2d0b853e6e8 100644 --- a/packages/@webex/webex-core/test/unit/spec/interceptors/payload-transformer.js +++ b/packages/@webex/webex-core/test/unit/spec/interceptors/payload-transformer.js @@ -5,6 +5,8 @@ import {assert} from '@webex/test-helper-chai'; import {capitalize} from 'lodash'; import WebexCore from '@webex/webex-core'; +// TODO: fix circular dependency core->metrics->core https://jira-eng-gpk2.cisco.com/jira/browse/SPARK-515520 +require('@webex/internal-plugin-metrics'); describe('webex-core', () => { describe('Interceptors', () => { From e6c5a41a61f11ff90b4b6e012fc8b5815201d100 Mon Sep 17 00:00:00 2001 From: Kesava Krishnan Madavan Date: Wed, 17 Apr 2024 18:18:28 +0530 Subject: [PATCH 19/20] fix: webex-core integration tests --- .../integration/spec/services/services.js | 61 +++++++++---------- .../test/integration/spec/webex-core.js | 6 +- 2 files changed, 32 insertions(+), 35 deletions(-) diff --git a/packages/@webex/webex-core/test/integration/spec/services/services.js b/packages/@webex/webex-core/test/integration/spec/services/services.js index 1e653d8fe52..dda3e7ced70 100644 --- a/packages/@webex/webex-core/test/integration/spec/services/services.js +++ b/packages/@webex/webex-core/test/integration/spec/services/services.js @@ -42,22 +42,21 @@ describe('webex-core', () => { }) ); - beforeEach('create webex instance', () => { + beforeEach('create webex instance', async () => { webex = new WebexCore({credentials: {supertoken: webexUser.token}}); webexEU = new WebexCore({credentials: {supertoken: webexUserEU.token}}); services = webex.internal.services; servicesEU = webexEU.internal.services; catalog = services._getCatalog(); - return Promise.all([ + await Promise.all([ services.waitForCatalog('postauth', 10), servicesEU.waitForCatalog('postauth', 10), - ]).then(() => - services.updateServices({ - from: 'limited', - query: {userId: webexUser.id}, - }) - ); + ]); + await services.updateServices({ + from: 'limited', + query: {userId: webexUser.id}, + }) }); describe('#_getCatalog()', () => { @@ -87,14 +86,14 @@ describe('webex-core', () => { let testUrlTemplate; let testUrl; - beforeEach('load test url', () => { + beforeEach('load test url', async () => { testUrlTemplate = { defaultUrl: 'https://www.example.com/api/v1', hosts: [], name: 'exampleValid', }; testUrl = new ServiceUrl(testUrlTemplate); - catalog._loadServiceUrls('preauth', [testUrl]); + await catalog._loadServiceUrls('preauth', [testUrl]); }); afterEach('unload test url', () => { @@ -532,7 +531,7 @@ describe('webex-core', () => { let testUrl; let testUrlTemplate; - beforeEach('load test url', () => { + beforeEach('load test url', async () => { testUrlTemplate = { defaultUrl: 'https://www.example.com/api/v1', hosts: [ @@ -553,7 +552,7 @@ describe('webex-core', () => { name: 'exampleValid', }; testUrl = new ServiceUrl(testUrlTemplate); - catalog._loadServiceUrls('preauth', [testUrl]); + await catalog._loadServiceUrls('preauth', [testUrl]); }); it('converts the url to a priority host url', () => { @@ -734,18 +733,15 @@ describe('webex-core', () => { done(); }); }); - it('updates the limited catalog when query param mode is provided', (done) => { + it('updates the limited catalog when query param mode is provided', async () => { catalog.serviceGroups.preauth = []; - services + await services .updateServices({ from: 'limited', query: {mode: 'DEFAULT_BY_PROXIMITY'}, - }) - .then(() => { - assert.isAbove(catalog.serviceGroups.preauth.length, 0); - done(); }); + assert.isAbove(catalog.serviceGroups.preauth.length, 0); }); it('does not update the limited catalog when nothing is provided', () => { catalog.serviceGroups.preauth = []; @@ -793,11 +789,12 @@ describe('webex-core', () => { }); describe('#fetchClientRegionInfo()', () => { - it('returns client region info', () => + it('returns client region info', async () => { services.fetchClientRegionInfo().then((r) => { assert.isDefined(r.countryCode); assert.isDefined(r.timezone); - })); + }) + }); }); describe('#validateUser()', () => { @@ -870,21 +867,21 @@ describe('webex-core', () => { assert.equal(Object.keys(unauthServices.list(false, 'postauth')).length, 0); })); - it('validates new user with activationOptions suppressEmail false', () => - unauthServices + it('validates new user with activationOptions suppressEmail false', async () => { + const r = await unauthServices .validateUser({ email: `Collabctg+webex-js-sdk-${uuid.v4()}@gmail.com`, activationOptions: {suppressEmail: false}, - }) - .then((r) => { - assert.hasAllKeys(r, ['activated', 'exists', 'user', 'details']); - assert.equal(r.activated, false); - assert.equal(r.exists, false); - assert.equal(r.user.verificationEmailTriggered, true); - assert.isAbove(Object.keys(unauthServices.list(false, 'preauth')).length, 0); - assert.equal(Object.keys(unauthServices.list(false, 'signin')).length, 0); - assert.equal(Object.keys(unauthServices.list(false, 'postauth')).length, 0); - })); + }); + + assert.hasAllKeys(r, ['activated', 'exists', 'user', 'details']); + assert.equal(r.activated, false); + assert.equal(r.exists, false); + assert.equal(r.user.verificationEmailTriggered, true); + assert.isAbove(Object.keys(unauthServices.list(false, 'preauth')).length, 0); + assert.equal(Object.keys(unauthServices.list(false, 'signin')).length, 0); + assert.equal(Object.keys(unauthServices.list(false, 'postauth')).length, 0); + }); it('validates new user with activationOptions suppressEmail true', () => unauthServices diff --git a/packages/@webex/webex-core/test/integration/spec/webex-core.js b/packages/@webex/webex-core/test/integration/spec/webex-core.js index fe53651b10b..ce8f5350874 100644 --- a/packages/@webex/webex-core/test/integration/spec/webex-core.js +++ b/packages/@webex/webex-core/test/integration/spec/webex-core.js @@ -17,7 +17,7 @@ describe('webex-core', function () { webex = new WebexCore(); }); - it('adds a tracking id to each request', () => + it.skip('adds a tracking id to each request', () => webex .request({ uri: makeLocalUrl('/'), @@ -35,7 +35,7 @@ describe('webex-core', function () { ); })); - it('adds a spark-user-agent id to each request', () => + it.skip('adds a spark-user-agent id to each request', () => webex .request({ uri: makeLocalUrl('/'), @@ -47,7 +47,7 @@ describe('webex-core', function () { assert.property(res.options.headers, 'spark-user-agent'); })); - it('fails with a WebexHttpError', () => + it.skip('fails with a WebexHttpError', () => assert .isRejected( webex.request({ From 0836d09e3ce29339701bec61b77088b3a5630419 Mon Sep 17 00:00:00 2001 From: Arun Ganeshan Date: Wed, 17 Apr 2024 12:59:05 -0400 Subject: [PATCH 20/20] fix: add server for mocha test (#3) --- .../integration/spec/services/services.js | 61 ++++++++++--------- .../test/integration/spec/webex-core.js | 6 +- packages/legacy/tools/src/utils/index.ts | 2 +- .../legacy/tools/src/utils/karma/karma.ts | 2 +- .../legacy/tools/src/utils/mocha/mocha.ts | 5 +- .../tools/src/utils/{karma => }/server.ts | 0 6 files changed, 40 insertions(+), 36 deletions(-) rename packages/legacy/tools/src/utils/{karma => }/server.ts (100%) diff --git a/packages/@webex/webex-core/test/integration/spec/services/services.js b/packages/@webex/webex-core/test/integration/spec/services/services.js index dda3e7ced70..1e653d8fe52 100644 --- a/packages/@webex/webex-core/test/integration/spec/services/services.js +++ b/packages/@webex/webex-core/test/integration/spec/services/services.js @@ -42,21 +42,22 @@ describe('webex-core', () => { }) ); - beforeEach('create webex instance', async () => { + beforeEach('create webex instance', () => { webex = new WebexCore({credentials: {supertoken: webexUser.token}}); webexEU = new WebexCore({credentials: {supertoken: webexUserEU.token}}); services = webex.internal.services; servicesEU = webexEU.internal.services; catalog = services._getCatalog(); - await Promise.all([ + return Promise.all([ services.waitForCatalog('postauth', 10), servicesEU.waitForCatalog('postauth', 10), - ]); - await services.updateServices({ - from: 'limited', - query: {userId: webexUser.id}, - }) + ]).then(() => + services.updateServices({ + from: 'limited', + query: {userId: webexUser.id}, + }) + ); }); describe('#_getCatalog()', () => { @@ -86,14 +87,14 @@ describe('webex-core', () => { let testUrlTemplate; let testUrl; - beforeEach('load test url', async () => { + beforeEach('load test url', () => { testUrlTemplate = { defaultUrl: 'https://www.example.com/api/v1', hosts: [], name: 'exampleValid', }; testUrl = new ServiceUrl(testUrlTemplate); - await catalog._loadServiceUrls('preauth', [testUrl]); + catalog._loadServiceUrls('preauth', [testUrl]); }); afterEach('unload test url', () => { @@ -531,7 +532,7 @@ describe('webex-core', () => { let testUrl; let testUrlTemplate; - beforeEach('load test url', async () => { + beforeEach('load test url', () => { testUrlTemplate = { defaultUrl: 'https://www.example.com/api/v1', hosts: [ @@ -552,7 +553,7 @@ describe('webex-core', () => { name: 'exampleValid', }; testUrl = new ServiceUrl(testUrlTemplate); - await catalog._loadServiceUrls('preauth', [testUrl]); + catalog._loadServiceUrls('preauth', [testUrl]); }); it('converts the url to a priority host url', () => { @@ -733,15 +734,18 @@ describe('webex-core', () => { done(); }); }); - it('updates the limited catalog when query param mode is provided', async () => { + it('updates the limited catalog when query param mode is provided', (done) => { catalog.serviceGroups.preauth = []; - await services + services .updateServices({ from: 'limited', query: {mode: 'DEFAULT_BY_PROXIMITY'}, + }) + .then(() => { + assert.isAbove(catalog.serviceGroups.preauth.length, 0); + done(); }); - assert.isAbove(catalog.serviceGroups.preauth.length, 0); }); it('does not update the limited catalog when nothing is provided', () => { catalog.serviceGroups.preauth = []; @@ -789,12 +793,11 @@ describe('webex-core', () => { }); describe('#fetchClientRegionInfo()', () => { - it('returns client region info', async () => { + it('returns client region info', () => services.fetchClientRegionInfo().then((r) => { assert.isDefined(r.countryCode); assert.isDefined(r.timezone); - }) - }); + })); }); describe('#validateUser()', () => { @@ -867,21 +870,21 @@ describe('webex-core', () => { assert.equal(Object.keys(unauthServices.list(false, 'postauth')).length, 0); })); - it('validates new user with activationOptions suppressEmail false', async () => { - const r = await unauthServices + it('validates new user with activationOptions suppressEmail false', () => + unauthServices .validateUser({ email: `Collabctg+webex-js-sdk-${uuid.v4()}@gmail.com`, activationOptions: {suppressEmail: false}, - }); - - assert.hasAllKeys(r, ['activated', 'exists', 'user', 'details']); - assert.equal(r.activated, false); - assert.equal(r.exists, false); - assert.equal(r.user.verificationEmailTriggered, true); - assert.isAbove(Object.keys(unauthServices.list(false, 'preauth')).length, 0); - assert.equal(Object.keys(unauthServices.list(false, 'signin')).length, 0); - assert.equal(Object.keys(unauthServices.list(false, 'postauth')).length, 0); - }); + }) + .then((r) => { + assert.hasAllKeys(r, ['activated', 'exists', 'user', 'details']); + assert.equal(r.activated, false); + assert.equal(r.exists, false); + assert.equal(r.user.verificationEmailTriggered, true); + assert.isAbove(Object.keys(unauthServices.list(false, 'preauth')).length, 0); + assert.equal(Object.keys(unauthServices.list(false, 'signin')).length, 0); + assert.equal(Object.keys(unauthServices.list(false, 'postauth')).length, 0); + })); it('validates new user with activationOptions suppressEmail true', () => unauthServices diff --git a/packages/@webex/webex-core/test/integration/spec/webex-core.js b/packages/@webex/webex-core/test/integration/spec/webex-core.js index ce8f5350874..fe53651b10b 100644 --- a/packages/@webex/webex-core/test/integration/spec/webex-core.js +++ b/packages/@webex/webex-core/test/integration/spec/webex-core.js @@ -17,7 +17,7 @@ describe('webex-core', function () { webex = new WebexCore(); }); - it.skip('adds a tracking id to each request', () => + it('adds a tracking id to each request', () => webex .request({ uri: makeLocalUrl('/'), @@ -35,7 +35,7 @@ describe('webex-core', function () { ); })); - it.skip('adds a spark-user-agent id to each request', () => + it('adds a spark-user-agent id to each request', () => webex .request({ uri: makeLocalUrl('/'), @@ -47,7 +47,7 @@ describe('webex-core', function () { assert.property(res.options.headers, 'spark-user-agent'); })); - it.skip('fails with a WebexHttpError', () => + it('fails with a WebexHttpError', () => assert .isRejected( webex.request({ diff --git a/packages/legacy/tools/src/utils/index.ts b/packages/legacy/tools/src/utils/index.ts index d6935459381..50af41f511d 100644 --- a/packages/legacy/tools/src/utils/index.ts +++ b/packages/legacy/tools/src/utils/index.ts @@ -3,7 +3,7 @@ import Karma from './karma'; import { startServer, stopServer, -} from './karma/server'; +} from './server'; import Mocha from './mocha'; export { diff --git a/packages/legacy/tools/src/utils/karma/karma.ts b/packages/legacy/tools/src/utils/karma/karma.ts index 7411db851ca..e703a19d6a7 100644 --- a/packages/legacy/tools/src/utils/karma/karma.ts +++ b/packages/legacy/tools/src/utils/karma/karma.ts @@ -2,7 +2,7 @@ import '@babel/register'; import '@webex/env-config-legacy'; import KarmaRunner from 'karma'; -import { startServer, stopServer } from './server'; +import { startServer, stopServer } from '../server'; import Browsers from './browsers'; diff --git a/packages/legacy/tools/src/utils/mocha/mocha.ts b/packages/legacy/tools/src/utils/mocha/mocha.ts index c17c2bc7732..b09e79bc130 100644 --- a/packages/legacy/tools/src/utils/mocha/mocha.ts +++ b/packages/legacy/tools/src/utils/mocha/mocha.ts @@ -1,7 +1,7 @@ import Babel from '@babel/register'; import '@webex/env-config-legacy'; - import MochaRunner from 'mocha'; +import { startServer, stopServer } from '../server'; import CONSTANTS from './mocha.constants'; @@ -31,11 +31,12 @@ class Mocha { files.forEach((file) => mochaRunner.addFile(file)); return new Promise((resolve) => { + startServer(); mochaRunner.run((failures) => { if (failures !== 0) { process.exit(1); } - + stopServer(); resolve(undefined); }); }); diff --git a/packages/legacy/tools/src/utils/karma/server.ts b/packages/legacy/tools/src/utils/server.ts similarity index 100% rename from packages/legacy/tools/src/utils/karma/server.ts rename to packages/legacy/tools/src/utils/server.ts