Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"type": "patch",
"area": "fix",
"workstream": "",
"comment": "Enable noUncheckedIndexedAccess in statefulcallclient and calling bindings",
"packageName": "@azure/communication-react",
"email": "2684369+JamesBurnside@users.noreply.github.com",
"dependentChangeType": "patch"
}
3 changes: 2 additions & 1 deletion common/config/tsc/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"forceConsistentCasingInFileNames": true,
"resolveJsonModule": true,
"isolatedModules": true,
"moduleResolution": "node"
"moduleResolution": "node",
"noUncheckedIndexedAccess": true
}
}
7 changes: 6 additions & 1 deletion packages/acs-ui-common/src/dataConversion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@
* @internal
*/
export const _base64ToBlob = (dataURI: string): Blob => {
const byteString = atob(dataURI.split(',')[1]);
const str = dataURI.split(',')[1];
if (!str) {
throw new Error('Invalid base64 string');
}

const byteString = atob(str);
const arrayBuffer = new ArrayBuffer(byteString.length);
const uint8Array = new Uint8Array(arrayBuffer);

Expand Down
6 changes: 3 additions & 3 deletions packages/calling-component-bindings/src/baseSelectors.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
import { DominantSpeakersInfo } from '@azure/communication-calling';
import { CallState, DominantSpeakersInfo } from '@azure/communication-calling';
/* @conditional-compile-remove(breakout-rooms) */
import { BreakoutRoom } from '@azure/communication-calling';
import { ParticipantCapabilities } from '@azure/communication-calling';
Expand Down Expand Up @@ -189,7 +189,7 @@ export const getDiagnostics = (
/**
* @private
*/
export const getCallState = (state: CallClientState, props: CallingBaseSelectorProps): string =>
export const getCallState = (state: CallClientState, props: CallingBaseSelectorProps): CallState | undefined =>
state.calls[props.callId]?.state;

/**
Expand All @@ -212,7 +212,7 @@ export const getParticipantCount = (state: CallClientState, props: CallingBaseSe

/* @conditional-compile-remove(acs-close-captions) */
/** @private */
export const getCaptionsKind = (state: CallClientState, props: CallingBaseSelectorProps): CaptionsKind => {
export const getCaptionsKind = (state: CallClientState, props: CallingBaseSelectorProps): CaptionsKind | undefined => {
return state.calls[props.callId]?.captionsFeature.captionsKind;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ export const screenShareButtonSelector: ScreenShareButtonSelector = reselect.cre
(capabilities?.shareScreen.isPresent === false && capabilities?.shareScreen.reason !== 'NotInitialized') ||
role === 'Consumer' ||
role === 'Attendee';
disabled = disabled || ['InLobby', 'Connecting', 'LocalHold'].includes(callState);
disabled = disabled || ['InLobby', 'Connecting', 'LocalHold'].includes(callState ?? 'None');
return {
checked: isScreenSharingOn,
disabled
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const getRemoteParticipantsExcludingConsumers = createSelector(
{
const newRemoteParticipants = { ...remoteParticipants };
Object.keys(newRemoteParticipants).forEach((k) => {
if (newRemoteParticipants[k].role === 'Consumer') {
if (newRemoteParticipants[k]?.role === 'Consumer') {
delete newRemoteParticipants[k];
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export const _isInLobbyOrConnecting = (callStatus: CallStatus | undefined): bool
export const _isPreviewOn = (deviceManager: DeviceManagerState): boolean => {
// TODO: we should take in a LocalVideoStream that developer wants to use as their 'Preview' view. We should also
// handle cases where 'Preview' view is in progress and not necessary completed.
return deviceManager.unparentedViews.length > 0 && deviceManager.unparentedViews[0].view !== undefined;
return deviceManager.unparentedViews[0]?.view !== undefined;
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ export class BreakoutRoomsSubscriber {

// If assigned breakout room has a display name, set the display name for its call state.
const assignedBreakoutRoomDisplayName =
this._context.getState().calls[this._callIdRef.callId].breakoutRooms?.assignedBreakoutRoom?.displayName;
this._context.getState().calls[this._callIdRef.callId]?.breakoutRooms?.assignedBreakoutRoom?.displayName;
if (assignedBreakoutRoomDisplayName) {
this._context.setBreakoutRoomDisplayName(call.id, assignedBreakoutRoomDisplayName);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,8 +259,8 @@ describe('declarative call agent', () => {

expect(Object.keys(context.getState().calls).length).toBe(0);
expect(Object.keys(context.getState().callsEnded).length).toBe(1);
expect(context.getState().callsEnded[mockCallId].callEndReason?.code).toBe(1);
expect(context.getState().callsEnded[mockCallId].endTime).toBeTruthy();
expect(context.getState().callsEnded[mockCallId]?.callEndReason?.code).toBe(1);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this because the return on these callEndReasons are strings?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ? is added because context.getState().callsEnded[mockCallId] now correctly returns Call | undefined (importantly the | undefined is now added to typescripts interpretation of the return type because the call is accessed via object[key] where key isn't guaranteed to exist)

expect(context.getState().callsEnded[mockCallId]?.endTime).toBeTruthy();
});

test('should move incoming call to incomingCallEnded when incoming call is ended and add endTime', async () => {
Expand All @@ -283,8 +283,8 @@ describe('declarative call agent', () => {

expect(Object.keys(context.getState().incomingCalls).length).toBe(0);
expect(Object.keys(context.getState().incomingCallsEnded).length).toBe(1);
expect(context.getState().incomingCallsEnded[mockCallId].callEndReason?.code).toBe(1);
expect(context.getState().incomingCallsEnded[mockCallId].endTime).toBeTruthy();
expect(context.getState().incomingCallsEnded[mockCallId]?.callEndReason?.code).toBe(1);
expect(context.getState().incomingCallsEnded[mockCallId]?.endTime).toBeTruthy();
});

test('should make sure that callsEnded not exceed max length', async () => {
Expand Down
52 changes: 37 additions & 15 deletions packages/calling-stateful-client/src/CallContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,10 @@ export class CallContext {
// Performance note: This loop should run only once because the number of entries
// is never allowed to exceed MAX_CALL_HISTORY_LENGTH. A loop is used for correctness.
while (Object.keys(draft.callsEnded).length >= MAX_CALL_HISTORY_LENGTH) {
delete draft.callsEnded[findOldestCallEnded(draft.callsEnded)];
const oldestCall = findOldestCallEnded(draft.callsEnded);
if (oldestCall) {
delete draft.callsEnded[oldestCall];
}
}
draft.callsEnded[latestCallId] = call;
}
Expand Down Expand Up @@ -531,6 +534,11 @@ export class CallContext {

if (participantKey === toFlatCommunicationIdentifier(this._state.userId)) {
call.localParticipantReaction = newReactionState;
} else if (!participant) {
// Warn if we can't find the participant in the call but we are trying to set their reaction state to a new reaction.
if (reactionMessage !== null) {
console.warn(`Participant ${participantKey} not found in call ${callId}. Cannot set reaction state.`);
}
} else {
participant.reactionState = newReactionState;
}
Expand Down Expand Up @@ -577,7 +585,7 @@ export class CallContext {
viewAttendeeNames.reason === 'MeetingRestricted'
) {
call.hideAttendeeNames = true;
} else {
} else if (call) {
call.hideAttendeeNames = false;
}
}
Expand Down Expand Up @@ -961,7 +969,10 @@ export class CallContext {
// Performance note: This loop should run only once because the number of entries
// is never allowed to exceed MAX_CALL_HISTORY_LENGTH. A loop is used for correctness.
while (Object.keys(draft.incomingCallsEnded).length >= MAX_CALL_HISTORY_LENGTH) {
delete draft.incomingCallsEnded[findOldestCallEnded(draft.incomingCallsEnded)];
const oldestCall = findOldestCallEnded(draft.incomingCallsEnded);
if (oldestCall) {
delete draft.incomingCallsEnded[oldestCall];
}
}
draft.incomingCallsEnded[callId] = call;
}
Expand Down Expand Up @@ -1053,11 +1064,11 @@ export class CallContext {
videoEffects: LocalVideoStreamVideoEffectsState
): void {
this.modifyState((draft: CallClientState) => {
const foundIndex = draft.deviceManager.unparentedViews.findIndex(
const view = draft.deviceManager.unparentedViews.find(
(stream) => stream.mediaStreamType === localVideoStream.mediaStreamType
);
if (foundIndex !== -1) {
draft.deviceManager.unparentedViews[foundIndex].videoEffects = videoEffects;
if (view) {
view.videoEffects = videoEffects;
}
});
}
Expand All @@ -1076,23 +1087,27 @@ export class CallContext {
captions.push(newCaption);
}
// if the last caption is final, then push the new one in
else if (captions[captions.length - 1].resultType === 'Final') {
else if (captions[captions.length - 1]?.resultType === 'Final') {
captions.push(newCaption);
}
// if the last caption is Partial, then check if the speaker is the same as the new caption, if so, update the last caption
else {
const lastCaption = captions[captions.length - 1];

if (
toFlatCommunicationIdentifier(
captions[captions.length - 1].speaker.identifier as CommunicationIdentifierKind
) === toFlatCommunicationIdentifier(newCaption.speaker.identifier as CommunicationIdentifierKind)
lastCaption &&
lastCaption.speaker.identifier &&
newCaption.speaker.identifier &&
toFlatCommunicationIdentifier(lastCaption.speaker.identifier) ===
toFlatCommunicationIdentifier(newCaption.speaker.identifier)
) {
captions[captions.length - 1] = newCaption;
}
// if different speaker, ignore the interjector until the current speaker finishes
// edge case: if we dont receive the final caption from the current speaker for 5 secs, we turn the current speaker caption to final and push in the new interjector
else {
if (Date.now() - captions[captions.length - 1].timestamp.getTime() > 5000) {
captions[captions.length - 1].resultType = 'Final';
else if (lastCaption) {
if (Date.now() - lastCaption.timestamp.getTime() > 5000) {
lastCaption.resultType = 'Final';
captions.push(newCaption);
}
}
Expand Down Expand Up @@ -1313,12 +1328,19 @@ const toCallError = (target: CallErrorTarget, error: unknown): CallError => {
return new CallError(target, new Error(error as string));
};

const findOldestCallEnded = (calls: { [key: string]: { endTime?: Date } }): string => {
const findOldestCallEnded = (calls: { [key: string]: { endTime?: Date } }): string | undefined => {
const callEntries = Object.entries(calls);
let [oldestCallId, oldestCall] = callEntries[0];
const firstCallEntry = callEntries[0];

if (!firstCallEntry) {
return undefined; // no calls exist
}

let [oldestCallId, oldestCall] = firstCallEntry;
if (oldestCall.endTime === undefined) {
return oldestCallId;
}

for (const [callId, call] of callEntries.slice(1)) {
if (call.endTime === undefined) {
return callId;
Expand Down
34 changes: 17 additions & 17 deletions packages/calling-stateful-client/src/Converter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ describe('Verify convert of TeamsMeetingAudioConferencingDetails to ConferencePh
]
});
expect(phoneInfo.length).toBe(1);
expect(phoneInfo[0].phoneNumber).toBe('1234567890');
expect(phoneInfo[0].conferenceId).toBe('123');
expect(phoneInfo[0].isTollFree).toBe(false);
expect(phoneInfo[0]?.phoneNumber).toBe('1234567890');
expect(phoneInfo[0]?.conferenceId).toBe('123');
expect(phoneInfo[0]?.isTollFree).toBe(false);
});

test('Phone number with country code and city has to contains all fields', async () => {
Expand All @@ -45,11 +45,11 @@ describe('Verify convert of TeamsMeetingAudioConferencingDetails to ConferencePh
]
});
expect(phoneInfo.length).toBe(1);
expect(phoneInfo[0].phoneNumber).toBe('1234567890');
expect(phoneInfo[0].conferenceId).toBe('123');
expect(phoneInfo[0].isTollFree).toBe(false);
expect(phoneInfo[0].country).toBe('US');
expect(phoneInfo[0].city).toBe('Redmond');
expect(phoneInfo[0]?.phoneNumber).toBe('1234567890');
expect(phoneInfo[0]?.conferenceId).toBe('123');
expect(phoneInfo[0]?.isTollFree).toBe(false);
expect(phoneInfo[0]?.country).toBe('US');
expect(phoneInfo[0]?.city).toBe('Redmond');
});

test('Toll free phone number has to be marked as toll free', async () => {
Expand All @@ -64,7 +64,7 @@ describe('Verify convert of TeamsMeetingAudioConferencingDetails to ConferencePh
]
});
expect(phoneInfo.length).toBe(1);
expect(phoneInfo[0].isTollFree).toBe(true);
expect(phoneInfo[0]?.isTollFree).toBe(true);
});

test('Toll free phone number info with multiple numbers', async () => {
Expand All @@ -84,10 +84,10 @@ describe('Verify convert of TeamsMeetingAudioConferencingDetails to ConferencePh
]
});
expect(phoneInfo.length).toBe(2);
expect(phoneInfo[0].isTollFree).toBe(true);
expect(phoneInfo[0].phoneNumber).toBe('1234567890');
expect(phoneInfo[1].isTollFree).toBe(true);
expect(phoneInfo[1].phoneNumber).toBe('0987654321');
expect(phoneInfo[0]?.isTollFree).toBe(true);
expect(phoneInfo[0]?.phoneNumber).toBe('1234567890');
expect(phoneInfo[1]?.isTollFree).toBe(true);
expect(phoneInfo[1]?.phoneNumber).toBe('0987654321');
});

test('Details with two phone numbers has to export both', async () => {
Expand All @@ -105,9 +105,9 @@ describe('Verify convert of TeamsMeetingAudioConferencingDetails to ConferencePh
]
});
expect(phoneInfo.length).toBe(2);
expect(phoneInfo[0].isTollFree).toBe(false);
expect(phoneInfo[0].phoneNumber).toBe('0987654321');
expect(phoneInfo[1].isTollFree).toBe(true);
expect(phoneInfo[1].phoneNumber).toBe('1234567890');
expect(phoneInfo[0]?.isTollFree).toBe(false);
expect(phoneInfo[0]?.phoneNumber).toBe('0987654321');
expect(phoneInfo[1]?.isTollFree).toBe(true);
expect(phoneInfo[1]?.phoneNumber).toBe('1234567890');
});
});
14 changes: 7 additions & 7 deletions packages/calling-stateful-client/src/DeviceManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ describe('device manager', () => {

const cameras = await (await client.getDeviceManager()).getCameras();
expect(cameras.length).toBe(1);
expect(cameras[0].name).toBe('camera');
expect(cameras[0]?.name).toBe('camera');

expect(client.getState().deviceManager.cameras.length).toBe(1);
expect(client.getState().deviceManager.cameras[0].name).toBe('camera');
expect(client.getState().deviceManager.cameras[0]?.name).toBe('camera');
});

test('should detect videoDevicesUpdated and update cameras in state using deviceManager', async () => {
Expand All @@ -47,7 +47,7 @@ describe('device manager', () => {

manager.emit('videoDevicesUpdated', {});
expect(await waitWithBreakCondition(() => client.getState().deviceManager.cameras.length === 1)).toBe(true);
expect(client.getState().deviceManager.cameras[0].name).toBe('camera');
expect(client.getState().deviceManager.cameras[0]?.name).toBe('camera');
});

test('should proxy getMicrophones and update microphones in state', async () => {
Expand All @@ -57,10 +57,10 @@ describe('device manager', () => {

const microphones = await (await client.getDeviceManager()).getMicrophones();
expect(microphones.length).toBe(1);
expect(microphones[0].name).toBe('microphone');
expect(microphones[0]?.name).toBe('microphone');

expect(client.getState().deviceManager.microphones.length).toBe(1);
expect(client.getState().deviceManager.microphones[0].name).toBe('microphone');
expect(client.getState().deviceManager.microphones[0]?.name).toBe('microphone');
});

test('should proxy selectMicrophone and update selectedMicrophone in state', async () => {
Expand Down Expand Up @@ -95,10 +95,10 @@ describe('device manager', () => {

const speakers = await (await client.getDeviceManager()).getSpeakers();
expect(speakers.length).toBe(1);
expect(speakers[0].name).toBe('speaker');
expect(speakers[0]?.name).toBe('speaker');

expect(client.getState().deviceManager.speakers.length).toBe(1);
expect(client.getState().deviceManager.speakers[0].name).toBe('speaker');
expect(client.getState().deviceManager.speakers[0]?.name).toBe('speaker');
});

test('should proxy selectSpeaker and update selectedSpeaker in state', async () => {
Expand Down
Loading