Matrix/element-call-Github
1
0
Fork 0
mirror of https://github.com/vector-im/element-call.git synced 2026-02-08 04:19:11 +00:00
Code Issues Packages Projects Releases Wiki Activity

Remove unsound participant casts

Browse Source 
By tagging participant behaviors with a type (local vs. remote) we can now tell what kind of participant it will be in a completely type-safe manner.
This commit is contained in:
Robin
2025-12-08 23:06:19 -05:00
parent 47cd343d44
commit a7a3d4e93c
5 changed files with 100 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files

50
src/state/CallViewModel/CallViewModel.ts
View File

@@ -110,6 +110,7 @@ import { ECConnectionFactory } from "./remoteMembers/ConnectionFactory.ts";
import { createConnectionManager$ } from "./remoteMembers/ConnectionManager.ts";
import {
createMatrixLivekitMembers$,
type TaggedParticipant,
type MatrixLivekitMember,
} from "./remoteMembers/MatrixLivekitMembers.ts";
import {
@@ -504,23 +505,28 @@ export function createCallViewModel$(
),
);
const localMatrixLivekitMember$ = scope.behavior<MatrixLivekitMember | null>(
localRtcMembership$.pipe(
generateItems(
// Generate a local member when membership is non-null
function* (membership) {
if (membership !== null) yield { keys: ["local"], data: membership };
},
(_scope, membership$) => ({
membership$,
participant$: localMembership.participant$,
connection$: localMembership.connection$,
userId,
}),
const localMatrixLivekitMember$ =
scope.behavior<MatrixLivekitMember<"local"> | null>(
localRtcMembership$.pipe(
generateItems(
// Generate a local member when membership is non-null
function* (membership) {
if (membership !== null)
yield { keys: ["local"], data: membership };
},
(_scope, membership$) => ({
membership$,
participant: {
type: "local" as const,
value$: localMembership.participant$,
},
connection$: localMembership.connection$,
userId,
}),
),
map(([localMember]) => localMember ?? null),
),
map(([localMember]) => localMember ?? null),
),
);
);
// ------------------------------------------------------------------------
// callLifecycle
@@ -597,7 +603,7 @@ export function createCallViewModel$(
const members = membersWithEpoch.value;
const a$ = combineLatest(
members.map((member) =>
combineLatest([member.connection$, member.participant$]).pipe(
combineLatest([member.connection$, member.participant.value$]).pipe(
map(([connection, participant]) => {
// do not render audio for local participant
if (!connection || !participant || participant.isLocal)
@@ -675,8 +681,10 @@ export function createCallViewModel$(
let localParticipantId: string | undefined = undefined;
// add local member if available
if (localMatrixLivekitMember) {
const { userId, participant$, connection$, membership$ } =
const { userId, connection$, membership$ } =
localMatrixLivekitMember;
const participant: TaggedParticipant =
localMatrixLivekitMember.participant; // Widen the type
localParticipantId = `${userId}:${membership$.value.deviceId}`; // should be membership$.value.membershipID which is not optional
// const participantId = membership$.value.membershipID;
if (localParticipantId) {
@@ -686,7 +694,7 @@ export function createCallViewModel$(
dup,
localParticipantId,
userId,
participant$,
participant,
connection$,
],
data: undefined,
@@ -697,7 +705,7 @@ export function createCallViewModel$(
// add remote members that are available
for (const {
userId,
participant$,
participant,
connection$,
membership$,
} of matrixLivekitMembers) {
@@ -706,7 +714,7 @@ export function createCallViewModel$(
// const participantId = membership$.value?.identity;
for (let dup = 0; dup < 1 + duplicateTiles; dup++) {
yield {
keys: [dup, participantId, userId, participant$, connection$],
keys: [dup, participantId, userId, participant, connection$],
data: undefined,
};
}

20
src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.test.ts
View File

@@ -100,12 +100,12 @@ test("should signal participant not yet connected to livekit", () => {
});
expectObservable(matrixLivekitMember$.pipe(map((e) => e.value))).toBe("a", {
a: expect.toSatisfy((data: MatrixLivekitMember[]) => {
a: expect.toSatisfy((data: MatrixLivekitMember<"remote">[]) => {
expect(data.length).toEqual(1);
expectObservable(data[0].membership$).toBe("a", {
a: bobMembership,
});
expectObservable(data[0].participant$).toBe("a", {
expectObservable(data[0].participant.value$).toBe("a", {
a: null,
});
expectObservable(data[0].connection$).toBe("a", {
@@ -180,12 +180,12 @@ test("should signal participant on a connection that is publishing", () => {
});
expectObservable(matrixLivekitMember$.pipe(map((e) => e.value))).toBe("a", {
a: expect.toSatisfy((data: MatrixLivekitMember[]) => {
a: expect.toSatisfy((data: MatrixLivekitMember<"remote">[]) => {
expect(data.length).toEqual(1);
expectObservable(data[0].membership$).toBe("a", {
a: bobMembership,
});
expectObservable(data[0].participant$).toBe("a", {
expectObservable(data[0].participant.value$).toBe("a", {
a: expect.toSatisfy((participant) => {
expect(participant).toBeDefined();
expect(participant!.identity).toEqual(bobParticipantId);
@@ -231,12 +231,12 @@ test("should signal participant on a connection that is not publishing", () => {
});
expectObservable(matrixLivekitMember$.pipe(map((e) => e.value))).toBe("a", {
a: expect.toSatisfy((data: MatrixLivekitMember[]) => {
a: expect.toSatisfy((data: MatrixLivekitMember<"remote">[]) => {
expect(data.length).toEqual(1);
expectObservable(data[0].membership$).toBe("a", {
a: bobMembership,
});
expectObservable(data[0].participant$).toBe("a", {
expectObservable(data[0].participant.value$).toBe("a", {
a: null,
});
expectObservable(data[0].connection$).toBe("a", {
@@ -296,7 +296,7 @@ describe("Publication edge case", () => {
expectObservable(matrixLivekitMember$.pipe(map((e) => e.value))).toBe(
"a",
{
a: expect.toSatisfy((data: MatrixLivekitMember[]) => {
a: expect.toSatisfy((data: MatrixLivekitMember<"remote">[]) => {
expect(data.length).toEqual(2);
expectObservable(data[0].membership$).toBe("a", {
a: bobMembership,
@@ -305,7 +305,7 @@ describe("Publication edge case", () => {
// The real connection should be from transportA as per the membership
a: connectionA,
});
expectObservable(data[0].participant$).toBe("a", {
expectObservable(data[0].participant.value$).toBe("a", {
a: expect.toSatisfy((participant) => {
expect(participant).toBeDefined();
expect(participant!.identity).toEqual(bobParticipantId);
@@ -362,7 +362,7 @@ describe("Publication edge case", () => {
expectObservable(matrixLivekitMember$.pipe(map((e) => e.value))).toBe(
"a",
{
a: expect.toSatisfy((data: MatrixLivekitMember[]) => {
a: expect.toSatisfy((data: MatrixLivekitMember<"remote">[]) => {
expect(data.length).toEqual(2);
expectObservable(data[0].membership$).toBe("a", {
a: bobMembership,
@@ -371,7 +371,7 @@ describe("Publication edge case", () => {
// The real connection should be from transportA as per the membership
a: connectionA,
});
expectObservable(data[0].participant$).toBe("a", {
expectObservable(data[0].participant.value$).toBe("a", {
// No participant as Bob is not publishing on his membership transport
a: null,
});

27
src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.ts
View File

@@ -5,10 +5,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
Please see LICENSE in the repository root for full details.
*/
import {
type LocalParticipant as LocalLivekitParticipant,
type RemoteParticipant as RemoteLivekitParticipant,
} from "livekit-client";
import { type LocalParticipant, type RemoteParticipant } from "livekit-client";
import {
type LivekitTransport,
type CallMembership,
@@ -24,16 +21,24 @@ import { generateItemsWithEpoch } from "../../../utils/observable";
const logger = rootLogger.getChild("[MatrixLivekitMembers]");
/**
* A dynamic participant value with a static tag to tell what kind of
* participant it can be (local vs. remote).
*/
export type TaggedParticipant =
| { type: "local"; value$: Behavior<LocalParticipant | null> }
| { type: "remote"; value$: Behavior<RemoteParticipant | null> };
/**
* Represents a Matrix call member and their associated LiveKit participation.
* `livekitParticipant` can be undefined if the member is not yet connected to the livekit room
* or if it has no livekit transport at all.
*/
export interface MatrixLivekitMember {
export interface MatrixLivekitMember<
ParticipantType extends TaggedParticipant["type"],
> {
membership$: Behavior<CallMembership>;
participant$: Behavior<
LocalLivekitParticipant | RemoteLivekitParticipant | null
>;
participant: TaggedParticipant & { type: ParticipantType };
connection$: Behavior<Connection | null>;
// participantId: string; We do not want a participantId here since it will be generated by the jwt
// TODO decide if we can also drop the userId. Its in the matrix membership anyways.
@@ -61,7 +66,7 @@ export function createMatrixLivekitMembers$({
scope,
membershipsWithTransport$,
connectionManager,
}: Props): Behavior<Epoch<MatrixLivekitMember[]>> {
}: Props): Behavior<Epoch<MatrixLivekitMember<"remote">[]>> {
/**
* Stream of all the call members and their associated livekit data (if available).
*/
@@ -110,12 +115,14 @@ export function createMatrixLivekitMembers$({
logger.debug(
`Updating data$ for participantId: ${participantId}, userId: ${userId}`,
);
const { participant$, ...rest } = scope.splitBehavior(data$);
// will only get called once per `participantId, userId` pair.
// updates to data$ and as a result to displayName$ and mxcAvatarUrl$ are more frequent.
return {
participantId,
userId,
...scope.splitBehavior(data$),
participant: { type: "remote" as const, value$: participant$ },
...rest,
};
},
),

14
src/state/CallViewModel/remoteMembers/integration.test.ts
View File

@@ -132,7 +132,7 @@ test("bob, carl, then bob joining no tracks yet", () => {
});
expectObservable(matrixLivekitItems$).toBe(vMarble, {
a: expect.toSatisfy((e: Epoch<MatrixLivekitMember[]>) => {
a: expect.toSatisfy((e: Epoch<MatrixLivekitMember<"remote">[]>) => {
const items = e.value;
expect(items.length).toBe(1);
const item = items[0]!;
@@ -147,12 +147,12 @@ test("bob, carl, then bob joining no tracks yet", () => {
),
),
});
expectObservable(item.participant$).toBe("a", {
expectObservable(item.participant.value$).toBe("a", {
a: null,
});
return true;
}),
b: expect.toSatisfy((e: Epoch<MatrixLivekitMember[]>) => {
b: expect.toSatisfy((e: Epoch<MatrixLivekitMember<"remote">[]>) => {
const items = e.value;
expect(items.length).toBe(2);
@@ -161,7 +161,7 @@ test("bob, carl, then bob joining no tracks yet", () => {
expectObservable(item.membership$).toBe("a", {
a: bobMembership,
});
expectObservable(item.participant$).toBe("a", {
expectObservable(item.participant.value$).toBe("a", {
a: null,
});
}
@@ -172,7 +172,7 @@ test("bob, carl, then bob joining no tracks yet", () => {
expectObservable(item.membership$).toBe("a", {
a: carlMembership,
});
expectObservable(item.participant$).toBe("a", {
expectObservable(item.participant.value$).toBe("a", {
a: null,
});
expectObservable(item.connection$).toBe("a", {
@@ -189,7 +189,7 @@ test("bob, carl, then bob joining no tracks yet", () => {
}
return true;
}),
c: expect.toSatisfy((e: Epoch<MatrixLivekitMember[]>) => {
c: expect.toSatisfy((e: Epoch<MatrixLivekitMember<"remote">[]>) => {
const items = e.value;
expect(items.length).toBe(3);
@@ -216,7 +216,7 @@ test("bob, carl, then bob joining no tracks yet", () => {
return true;
}),
});
expectObservable(item.participant$).toBe("a", {
expectObservable(item.participant.value$).toBe("a", {
a: null,
});
}

69
src/state/UserMedia.ts
View File

@@ -27,6 +27,7 @@ import type { ReactionOption } from "../reactions";
import { observeSpeaker$ } from "./observeSpeaker.ts";
import { generateItems } from "../utils/observable.ts";
import { ScreenShare } from "./ScreenShare.ts";
import { type TaggedParticipant } from "./CallViewModel/remoteMembers/MatrixLivekitMembers.ts";
/**
* Sorting bins defining the order in which media tiles appear in the layout.
@@ -68,40 +69,46 @@ enum SortingBin {
* for inclusion in the call layout and tracks associated screen shares.
*/
export class UserMedia {
public readonly vm: UserMediaViewModel = this.participant$.value?.isLocal
? new LocalUserMediaViewModel(
this.scope,
this.id,
this.userId,
this.participant$ as Behavior<LocalParticipant | null>,
this.encryptionSystem,
this.livekitRoom$,
this.focusUrl$,
this.mediaDevices,
this.displayName$,
this.mxcAvatarUrl$,
this.scope.behavior(this.handRaised$),
this.scope.behavior(this.reaction$),
)
: new RemoteUserMediaViewModel(
this.scope,
this.id,
this.userId,
this.participant$ as Behavior<RemoteParticipant | null>,
this.encryptionSystem,
this.livekitRoom$,
this.focusUrl$,
this.pretendToBeDisconnected$,
this.displayName$,
this.mxcAvatarUrl$,
this.scope.behavior(this.handRaised$),
this.scope.behavior(this.reaction$),
);
public readonly vm: UserMediaViewModel =
this.participant.type === "local"
? new LocalUserMediaViewModel(
this.scope,
this.id,
this.userId,
this.participant.value$,
this.encryptionSystem,
this.livekitRoom$,
this.focusUrl$,
this.mediaDevices,
this.displayName$,
this.mxcAvatarUrl$,
this.scope.behavior(this.handRaised$),
this.scope.behavior(this.reaction$),
)
: new RemoteUserMediaViewModel(
this.scope,
this.id,
this.userId,
this.participant.value$,
this.encryptionSystem,
this.livekitRoom$,
this.focusUrl$,
this.pretendToBeDisconnected$,
this.displayName$,
this.mxcAvatarUrl$,
this.scope.behavior(this.handRaised$),
this.scope.behavior(this.reaction$),
);
private readonly speaker$ = this.scope.behavior(
observeSpeaker$(this.vm.speaking$),
);
// TypeScript needs this widening of the type to happen in a separate statement
private readonly participant$: Behavior<
LocalParticipant | RemoteParticipant | null
> = this.participant.value$;
/**
* All screen share media associated with this user media.
*/
@@ -184,9 +191,7 @@ export class UserMedia {
private readonly scope: ObservableScope,
public readonly id: string,
private readonly userId: string,
private readonly participant$: Behavior<
LocalParticipant | RemoteParticipant | null
>,
private readonly participant: TaggedParticipant,
private readonly encryptionSystem: EncryptionSystem,
private readonly livekitRoom$: Behavior<LivekitRoom | undefined>,
private readonly focusUrl$: Behavior<string | undefined>,