Fix avatar reactivity, simplify display names tracking

This commit is contained in:
Robin
2025-11-09 01:16:39 -05:00
parent b4c17ed26d
commit 92ddc4c797
3 changed files with 72 additions and 75 deletions

View File

@@ -13,15 +13,15 @@ import {
type LivekitTransport,
type CallMembership,
} from "matrix-js-sdk/lib/matrixrtc";
import { combineLatest, filter, map } from "rxjs";
import { combineLatest, filter, fromEvent, map, startWith } from "rxjs";
// eslint-disable-next-line rxjs/no-internal
import { type NodeStyleEventEmitter } from "rxjs/internal/observable/fromEvent";
import { type Room as MatrixRoom } from "matrix-js-sdk";
import { RoomStateEvent, type Room as MatrixRoom } from "matrix-js-sdk";
import { logger } from "matrix-js-sdk/lib/logger";
import { type Behavior } from "../../Behavior";
import { type IConnectionManager } from "./ConnectionManager";
import { Epoch, mapEpoch, type ObservableScope } from "../../ObservableScope";
import { Epoch, type ObservableScope } from "../../ObservableScope";
import { memberDisplaynames$ } from "./displayname";
import { type Connection } from "./Connection";
import { generateItemsWithEpoch } from "../../../utils/observable";
@@ -82,14 +82,17 @@ export function createMatrixLivekitMembers$({
const displaynameMap$ = memberDisplaynames$(
scope,
matrixRoom,
membershipsWithTransport$.pipe(mapEpoch((v) => v.map((v) => v.membership))),
scope.behavior(
membershipsWithTransport$.pipe(
map((ms) => ms.value.map((m) => m.membership)),
),
),
);
return scope.behavior(
combineLatest([
membershipsWithTransport$,
connectionManager.connectionManagerData$,
displaynameMap$,
]).pipe(
filter((values) =>
values.every((value) => value.epoch === values[0].epoch),
@@ -98,15 +101,11 @@ export function createMatrixLivekitMembers$({
([
{ value: membershipsWithTransports, epoch },
{ value: managerData },
{ value: displaynames },
]) =>
new Epoch(
[membershipsWithTransports, managerData, displaynames] as const,
epoch,
),
new Epoch([membershipsWithTransports, managerData] as const, epoch),
),
generateItemsWithEpoch(
function* ([membershipsWithTransports, managerData, displaynames]) {
function* ([membershipsWithTransports, managerData]) {
for (const { membership, transport } of membershipsWithTransports) {
// TODO! cannot use membership.membershipID yet, Currently its hardcoded by the jwt service to
const participantId = /*membership.membershipID*/ `${membership.userId}:${membership.deviceId}`;
@@ -116,35 +115,42 @@ export function createMatrixLivekitMembers$({
: [];
const participant =
participants.find((p) => p.identity == participantId) ?? null;
// This makes sense to add to the js-sdk callMembership (we only need the avatar so probably the call memberhsip just should aquire the avatar)
const member = matrixRoom.getMember(membership.userId);
const connection = transport
? managerData.getConnectionForTransport(transport)
: undefined;
let displayName = displaynames.get(membership.userId);
if (displayName === undefined) {
logger.warn(`No display name for user ${membership.userId}`);
displayName = "";
}
yield {
keys: [participantId, membership.userId],
data: {
membership,
participant,
connection,
displayName,
mxcAvatarUrl: member?.getMxcAvatarUrl(),
},
data: { membership, participant, connection },
};
}
},
(scope, data$, participantId, userId) => ({
participantId,
userId,
...scope.splitBehavior(data$),
}),
(scope, data$, participantId, userId) => {
const member = matrixRoom.getMember(userId);
return {
participantId,
userId,
...scope.splitBehavior(data$),
displayName$: scope.behavior(
displaynameMap$.pipe(
map((displayNames) => {
const name = displayNames.get(userId);
if (name === undefined) {
logger.warn(`No display name for user ${userId}`);
return "";
}
return name;
}),
),
),
mxcAvatarUrl$: scope.behavior(
fromEvent(matrixRoom, RoomStateEvent.Members).pipe(
startWith(undefined),
map(() => member?.getMxcAvatarUrl()),
),
),
};
},
),
),
);

View File

@@ -13,9 +13,8 @@ import {
RoomStateEvent,
} from "matrix-js-sdk";
import EventEmitter from "events";
import { map } from "rxjs";
import { ObservableScope, trackEpoch } from "../../ObservableScope.ts";
import { ObservableScope } from "../../ObservableScope.ts";
import type { Room as MatrixRoom } from "matrix-js-sdk/lib/models/room";
import { mockCallMembership, withTestScheduler } from "../../../utils/test.ts";
import { memberDisplaynames$ } from "./displayname.ts";
@@ -86,16 +85,16 @@ afterEach(() => {
// TODO this is a regression, now there the own user is not always in the map. Ask Timo if fine
test.skip("should always have our own user", () => {
withTestScheduler(({ cold, schedule, expectObservable }) => {
withTestScheduler(({ behavior, expectObservable }) => {
const dn$ = memberDisplaynames$(
testScope,
mockMatrixRoom,
cold("a", {
behavior("a", {
a: [],
}).pipe(trackEpoch()),
}),
);
expectObservable(dn$.pipe(map((e) => e.value))).toBe("a", {
expectObservable(dn$).toBe("a", {
a: new Map<string, string>([
["@local:example.com", "@local:example.com"],
]),
@@ -116,19 +115,19 @@ function setUpBasicRoom(): void {
test("should get displayName for users", () => {
setUpBasicRoom();
withTestScheduler(({ cold, schedule, expectObservable }) => {
withTestScheduler(({ behavior, expectObservable }) => {
const dn$ = memberDisplaynames$(
testScope,
mockMatrixRoom,
cold("a", {
behavior("a", {
a: [
mockCallMembership("@alice:example.com", "DEVICE1"),
mockCallMembership("@bob:example.com", "DEVICE1"),
],
}).pipe(trackEpoch()),
}),
);
expectObservable(dn$.pipe(map((e) => e.value))).toBe("a", {
expectObservable(dn$).toBe("a", {
a: new Map<string, string>([
// ["@local:example.com", "it's a me"],
["@alice:example.com", "Alice"],
@@ -139,18 +138,18 @@ test("should get displayName for users", () => {
});
test("should use userId if no display name", () => {
withTestScheduler(({ cold, schedule, expectObservable }) => {
withTestScheduler(({ behavior, expectObservable }) => {
setUpBasicRoom();
const dn$ = memberDisplaynames$(
testScope,
mockMatrixRoom,
cold("a", {
behavior("a", {
a: [mockCallMembership("@no-name:foo.bar", "D000")],
}).pipe(trackEpoch()),
}),
);
expectObservable(dn$.pipe(map((e) => e.value))).toBe("a", {
expectObservable(dn$).toBe("a", {
a: new Map<string, string>([
// ["@local:example.com", "it's a me"],
["@no-name:foo.bar", "@no-name:foo.bar"],
@@ -160,13 +159,13 @@ test("should use userId if no display name", () => {
});
test("should disambiguate users with same display name", () => {
withTestScheduler(({ cold, schedule, expectObservable }) => {
withTestScheduler(({ behavior, expectObservable }) => {
setUpBasicRoom();
const dn$ = memberDisplaynames$(
testScope,
mockMatrixRoom,
cold("a", {
behavior("a", {
a: [
mockCallMembership("@bob:example.com", "DEVICE1"),
mockCallMembership("@bob:example.com", "DEVICE2"),
@@ -174,10 +173,10 @@ test("should disambiguate users with same display name", () => {
mockCallMembership("@carl:example.com", "C000"),
mockCallMembership("@evil:example.com", "E000"),
],
}).pipe(trackEpoch()),
}),
);
expectObservable(dn$.pipe(map((e) => e.value))).toBe("a", {
expectObservable(dn$).toBe("a", {
a: new Map<string, string>([
// ["@local:example.com", "it's a me"],
["@bob:example.com", "Bob (@bob:example.com)"],
@@ -191,22 +190,22 @@ test("should disambiguate users with same display name", () => {
});
test("should disambiguate when needed", () => {
withTestScheduler(({ cold, schedule, expectObservable }) => {
withTestScheduler(({ behavior, expectObservable }) => {
setUpBasicRoom();
const dn$ = memberDisplaynames$(
testScope,
mockMatrixRoom,
cold("ab", {
behavior("ab", {
a: [mockCallMembership("@bob:example.com", "DEVICE1")],
b: [
mockCallMembership("@bob:example.com", "DEVICE1"),
mockCallMembership("@bob:foo.bar", "BOB000"),
],
}).pipe(trackEpoch()),
}),
);
expectObservable(dn$.pipe(map((e) => e.value))).toBe("ab", {
expectObservable(dn$).toBe("ab", {
a: new Map<string, string>([
// ["@local:example.com", "it's a me"],
["@bob:example.com", "Bob"],
@@ -221,22 +220,22 @@ test("should disambiguate when needed", () => {
});
test.skip("should keep disambiguated name when other leave", () => {
withTestScheduler(({ cold, schedule, expectObservable }) => {
withTestScheduler(({ behavior, expectObservable }) => {
setUpBasicRoom();
const dn$ = memberDisplaynames$(
testScope,
mockMatrixRoom,
cold("ab", {
behavior("ab", {
a: [
mockCallMembership("@bob:example.com", "DEVICE1"),
mockCallMembership("@bob:foo.bar", "BOB000"),
],
b: [mockCallMembership("@bob:example.com", "DEVICE1")],
}).pipe(trackEpoch()),
}),
);
expectObservable(dn$.pipe(map((e) => e.value))).toBe("ab", {
expectObservable(dn$).toBe("ab", {
a: new Map<string, string>([
// ["@local:example.com", "it's a me"],
["@bob:example.com", "Bob (@bob:example.com)"],
@@ -251,18 +250,18 @@ test.skip("should keep disambiguated name when other leave", () => {
});
test("should disambiguate on name change", () => {
withTestScheduler(({ cold, schedule, expectObservable }) => {
withTestScheduler(({ behavior, schedule, expectObservable }) => {
setUpBasicRoom();
const dn$ = memberDisplaynames$(
testScope,
mockMatrixRoom,
cold("a", {
behavior("a", {
a: [
mockCallMembership("@bob:example.com", "B000"),
mockCallMembership("@carl:example.com", "C000"),
],
}).pipe(trackEpoch()),
}),
);
schedule("-a", {
@@ -271,7 +270,7 @@ test("should disambiguate on name change", () => {
},
});
expectObservable(dn$.pipe(map((e) => e.value))).toBe("ab", {
expectObservable(dn$).toBe("ab", {
a: new Map<string, string>([
// ["@local:example.com", "it's a me"],
["@bob:example.com", "Bob"],

View File

@@ -6,20 +6,14 @@ Please see LICENSE in the repository root for full details.
*/
import { type RoomMember, RoomStateEvent } from "matrix-js-sdk";
import {
combineLatest,
fromEvent,
map,
type Observable,
startWith,
} from "rxjs";
import { combineLatest, fromEvent, map, startWith } from "rxjs";
import { type CallMembership } from "matrix-js-sdk/lib/matrixrtc";
import { logger } from "matrix-js-sdk/lib/logger";
import { type Room as MatrixRoom } from "matrix-js-sdk/lib/matrix";
// eslint-disable-next-line rxjs/no-internal
import { type NodeStyleEventEmitter } from "rxjs/internal/observable/fromEvent";
import { Epoch, type ObservableScope } from "../../ObservableScope";
import { type ObservableScope } from "../../ObservableScope";
import {
calculateDisplayName,
shouldDisambiguate,
@@ -49,8 +43,8 @@ export const memberDisplaynames$ = (
scope: ObservableScope,
matrixRoom: Pick<MatrixRoom, "getMember"> & NodeStyleEventEmitter,
// roomMember$: Behavior<Pick<RoomMember, "userId" | "getMxcAvatarUrl">>;
memberships$: Observable<Epoch<CallMembership[]>>,
): Behavior<Epoch<Map<string, string>>> =>
memberships$: Behavior<CallMembership[]>,
): Behavior<Map<string, string>> =>
scope.behavior(
combineLatest([
// Handle call membership changes
@@ -59,8 +53,7 @@ export const memberDisplaynames$ = (
fromEvent(matrixRoom, RoomStateEvent.Members).pipe(startWith(null)),
// TODO: do we need: pauseWhen(this.pretendToBeDisconnected$),
]).pipe(
map(([epochMemberships, _displayNames]) => {
const { epoch, value: memberships } = epochMemberships;
map(([memberships, _displayNames]) => {
const displaynameMap = new Map<string, string>();
const room = matrixRoom;
@@ -77,8 +70,7 @@ export const memberDisplaynames$ = (
calculateDisplayName(member, disambiguate),
);
}
return new Epoch(displaynameMap, epoch);
return displaynameMap;
}),
),
new Epoch(new Map<string, string>()),
);