From ede29bdf1ba644a457911d43395a0c44d71e3621 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 26 Aug 2026 17:53:06 +0200 Subject: [PATCH] Remove MatrixRTC legacy mode This is the mode in which we sent membership events with the 'oldest membership' transport selection algorithm, which stopped being the default back in version 0.21.0. Users will no longer be able to select this mode in developer settings, and admins will no longer be able to select legacy mode through the config either. The app will still continue to support *receiving* membership events with the 'oldest membership' transport selection algorithm from others, however. --- docs/matrix_rtc_modes.md | 32 ++-- locales/en/app.json | 4 - playwright/spa-helpers.ts | 4 +- playwright/widget/federated-call.test.ts | 4 +- .../federation-oldest-membership-bug.spec.ts | 85 ---------- .../widget/hotswap-legacy-compat.test.ts | 91 ----------- playwright/widget/test-helpers.ts | 6 +- src/config/ConfigOptions.ts | 4 +- src/settings/DeveloperSettingsTab.test.tsx | 22 +-- src/settings/DeveloperSettingsTab.tsx | 16 -- .../DeveloperSettingsTab.test.tsx.snap | 70 ++------ src/state/CallViewModel/CallViewModel.test.ts | 8 +- src/state/CallViewModel/CallViewModel.ts | 3 +- .../localMember/LocalMember.test.ts | 26 +-- .../CallViewModel/localMember/LocalMember.ts | 1 - .../localMember/LocalTransport.test.ts | 131 +-------------- .../localMember/LocalTransport.ts | 154 +----------------- .../CallViewModel/remoteMembers/Connection.ts | 6 - src/state/CallViewModelWidget.test.ts | 6 +- src/utils/test-viewmodel.ts | 2 +- src/utils/test.ts | 5 +- 21 files changed, 51 insertions(+), 629 deletions(-) delete mode 100644 playwright/widget/federation-oldest-membership-bug.spec.ts delete mode 100644 playwright/widget/hotswap-legacy-compat.test.ts diff --git a/docs/matrix_rtc_modes.md b/docs/matrix_rtc_modes.md index 595b881ea..a30248ba0 100644 --- a/docs/matrix_rtc_modes.md +++ b/docs/matrix_rtc_modes.md @@ -1,36 +1,24 @@ # MatrixRTC modes Element Call is in the middle of a transition of how a call session is -represented and how participants pick an SFU: +represented: from room _state_ events (`org.matrix.msc3401.call.member`) to +_sticky_ events +([MSC4354](https://github.com/matrix-org/matrix-spec-proposals/pull/4354)), +which are a much better fit for the short lived, per-device nature of call +memberships. -- **Membership events**: from room _state_ events - (`org.matrix.msc3401.call.member`) to _sticky_ events - ([MSC4354](https://github.com/matrix-org/matrix-spec-proposals/pull/4354)), - which are a much better fit for the short lived, per-device nature of call - memberships. -- **SFU selection**: from "everyone connects to the SFU of the oldest member" to - **multi SFU**, where each participant uses its own homeserver's SFU and the - SFUs interconnect. - -Not every homeserver supports sticky events yet. Multi SFU is supported on all current (August 2026) -element call clients. The three MatrixRTC modes are the steps of that transition, -so a deployment can pick the newest one its homeserver and its user base can -handle. +Not every homeserver supports sticky events yet. The two MatrixRTC modes +controls whether Element Call uses them. ## The modes | Mode | Membership events | SFU selection | JWT endpoint | | --------------- | ----------------- | ------------- | ---------------------------- | -| `legacy` | state events | oldest member | legacy | | `compatibility` | state events | multi SFU | legacy | | `matrix_2_0` | sticky events | multi SFU | Matrix 2.0 (hashed identity) | -**`legacy`** — the lowest common denominator. Use it if calls need to work with -Element Call clients older than v0.17.0, which cannot handle multi SFU calls. (unused) - -**`compatibility`** — multi SFU, but still state events. Use it when all Element -Call clients are v0.17.0 or later but the homeserver does not support sticky -events. This is the default. (default) +**`compatibility`** — multi SFU, but still state events. Use it when the +homeserver does not support sticky events. This is the default. **`matrix_2_0`** — the target state. Requires a homeserver that advertises MSC4354 and all clients on v0.17.0 or later. The local membership requests its @@ -54,7 +42,7 @@ disables the Developer Settings choice: } ``` -Valid values are `legacy`, `compatibility` and `matrix_2_0`; an invalid value is +Valid values are `compatibility` and `matrix_2_0`; an invalid value is ignored (with a warning) and the user's choice applies. Pinning `matrix_2_0` on a homeserver without sticky event support makes joining fail with a "sticky events required" error. diff --git a/locales/en/app.json b/locales/en/app.json index 2b3e08358..543942e2e 100644 --- a/locales/en/app.json +++ b/locales/en/app.json @@ -80,10 +80,6 @@ "description": "Compatible with homeservers that do not support sticky events (but all other EC clients are v0.17.0 or later)", "label": "Compatibility: state events & multi SFU" }, - "Legacy": { - "description": "Compatible with old versions of EC that do not support multi SFU", - "label": "Legacy: state events & oldest membership SFU" - }, "Matrix_2_0": { "description": "Compatible only with homservers supporting sticky events and all EC clients v0.17.0 or later", "label": "Matrix 2.0: sticky events & multi SFU" diff --git a/playwright/spa-helpers.ts b/playwright/spa-helpers.ts index 24869141f..5f99e32d3 100644 --- a/playwright/spa-helpers.ts +++ b/playwright/spa-helpers.ts @@ -101,9 +101,7 @@ async function setRtcModeFromSettings( // Move to Developer tab now await page.getByRole("tab", { name: "Developer" }).click(); - if (mode == "legacy") { - await page.getByText("Legacy: state events").click(); - } else if (mode == "2_0") { + if (mode == "2_0") { await page.getByText("Matrix 2.0").click(); } else { // compat diff --git a/playwright/widget/federated-call.test.ts b/playwright/widget/federated-call.test.ts index 560636a5d..61f4750b3 100644 --- a/playwright/widget/federated-call.test.ts +++ b/playwright/widget/federated-call.test.ts @@ -12,9 +12,7 @@ import { HOST1, HOST2, type RtcMode, TestHelpers } from "./test-helpers"; const modePairs: [RtcMode, RtcMode][] = [ ["compat", "compat"], - ["legacy", "legacy"], - ["legacy", "compat"], - ["compat", "legacy"], + // TODO: Compatibility + Matrix 2.0? ]; modePairs.forEach(([rtcMode1, rtcMode2]) => { diff --git a/playwright/widget/federation-oldest-membership-bug.spec.ts b/playwright/widget/federation-oldest-membership-bug.spec.ts deleted file mode 100644 index ab5c70fc8..000000000 --- a/playwright/widget/federation-oldest-membership-bug.spec.ts +++ /dev/null @@ -1,85 +0,0 @@ -/* -Copyright 2026 Element Creations Ltd. - -SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial -Please see LICENSE in the repository root for full details. -*/ - -import { expect, test } from "@playwright/test"; - -import { widgetTest } from "../fixtures/widget-user"; -import { HOST1, HOST2, TestHelpers } from "./test-helpers"; - -widgetTest( - "Bug new joiner was not publishing on correct SFU", - async ({ addUser, browserName }) => { - test.skip( - browserName === "firefox", - "This is a bug in the old widget, not a browser problem.", - ); - - test.slow(); - - // 2 users in federation - const florian = await addUser("floriant", HOST1); - const timo = await addUser("timo", HOST2); - - // Florian creates a room and invites Timo to it - const roomName = "Call Room"; - await TestHelpers.createRoom(roomName, florian.page, [timo.mxId]); - - // Timo joins the room - await TestHelpers.acceptRoomInvite(roomName, timo.page); - - // Ensure we are in legacy mode (should be the default) - await TestHelpers.openWidgetSetEmbeddedElementCallRtcModeCloseWidget( - florian.page, - "legacy", - ); - await TestHelpers.openWidgetSetEmbeddedElementCallRtcModeCloseWidget( - timo.page, - "legacy", - ); - - // Let timo create a call - await TestHelpers.startCallInCurrentRoom(timo.page, false); - await TestHelpers.joinCallFromLobby(timo.page); - - // We want to simulate that the oldest membership authentication is way slower than - // the preffered auth. - // In this setup, timo advertised$ transport will be it's own, and the active will be the one from florian - await florian.page.route( - "**/matrix-rtc.othersite.m.localhost/livekit/jwt/**", - async (route) => { - await new Promise((resolve) => setTimeout(resolve, 2000)); // 5 second delay - await route.continue(); - }, - ); - - // Florian joins the call - await expect(florian.page.getByTestId("join-call-button")).toBeVisible(); - await florian.page.getByTestId("join-call-button").click(); - await TestHelpers.joinCallFromLobby(florian.page); - - await florian.page.waitForTimeout(3000); - await timo.page.waitForTimeout(3000); - - // We should see 2 video tiles everywhere now - for (const user of [timo, florian]) { - const frame = user.page - .locator('iframe[title="Element Call"]') - .contentFrame(); - await expect(frame.getByTestId("videoTile")).toHaveCount(2); - - // No one should be waiting for media - await expect(frame.getByText("Waiting for media...")).not.toBeVisible(); - - // There should be 2 video elements, visible and autoplaying - await expect(frame.locator("video")).toHaveCount(2, { - timeout: 10000, - }); - - await TestHelpers.expectVisibleVideoCount(frame, 2); - } - }, -); diff --git a/playwright/widget/hotswap-legacy-compat.test.ts b/playwright/widget/hotswap-legacy-compat.test.ts deleted file mode 100644 index ed6f15083..000000000 --- a/playwright/widget/hotswap-legacy-compat.test.ts +++ /dev/null @@ -1,91 +0,0 @@ -/* -Copyright 2026 Element Creations Ltd. - -SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial -Please see LICENSE in the repository root for full details. -*/ - -import { expect, test } from "@playwright/test"; - -import { widgetTest } from "../fixtures/widget-user"; -import { HOST1, HOST2, TestHelpers } from "./test-helpers"; - -// ## Issue -// This test reproduces an issue with the publisher. -// When switching local focus, we need to recreate the publisher. -// This failed because of a dead lock in the old publishers destruction. -// -// There are numerus ways to enforece this situation: -// - oldest member swap (manually set the oldest member focus and leave with the prev oldest member) -// This almost never happens in the real worls since clients will set their preferredFoci list to what the oldest member is. -// - switch from oldest member to multi sfu as the NOT the first joiner + the first joiner is on a different sfu than your preferred sfu. -// -// This test uses the "switch from oldest member to multi sfu" approach. -// -// It is a copy of federated-call.test.ts in the `["legacy", "legacy"]` setup, -// which once connected will make the second user switch to multi sfu. -widgetTest( - `Test swapping publisher from ${HOST1} to ${HOST2}`, - async ({ addUser, browserName }) => { - test.slow(); - test.skip( - browserName === "firefox", - "The is test is not working on firefox CI environment. No mic/audio device inputs so cam/mic are disabled", - ); - - const florian = await addUser("floriant", HOST1); - const timo = await addUser("timo", HOST2); - - const roomName = "Call Room"; - - await TestHelpers.createRoom(roomName, florian.page, [timo.mxId]); - - await TestHelpers.acceptRoomInvite(roomName, timo.page); - - await florian.page.pause(); - - await TestHelpers.openWidgetSetEmbeddedElementCallRtcModeCloseWidget( - florian.page, - "legacy", - ); - await TestHelpers.openWidgetSetEmbeddedElementCallRtcModeCloseWidget( - timo.page, - "legacy", - ); - - await TestHelpers.startCallInCurrentRoom(florian.page, false); - await TestHelpers.joinCallFromLobby(florian.page); - - // timo joins - await TestHelpers.joinCallInCurrentRoom(timo.page); - - // We should see 2 video tiles everywhere now - for (const user of [timo, florian]) { - const frame = user.page - .locator('iframe[title="Element Call"]') - .contentFrame(); - await expect(frame.getByTestId("videoTile")).toHaveCount(2); - - // Wait for "Waiting for media..." to disappear (with timeout) - await expect(frame.getByText("Waiting for media...")).not.toBeVisible({ - timeout: 10000, // Maximum time to wait - }); - - // There should be 2 video elements, visible and autoplaying - await expect(frame.locator("video")).toHaveCount(2, { - timeout: 10000, - }); - - await TestHelpers.expectVisibleVideoCount(frame, 2); - } - - // now we switch the mode for timo (second joiner on multi-sfu HOST2 but currently HOST1) - await TestHelpers.setEmbeddedElementCallRtcMode(timo.page, "compat"); - await timo.page.waitForTimeout(3000); - - await TestHelpers.expectVisibleVideoCount( - timo.page.locator('iframe[title="Element Call"]').contentFrame(), - 2, - ); - }, -); diff --git a/playwright/widget/test-helpers.ts b/playwright/widget/test-helpers.ts index 632b2592b..0322596aa 100644 --- a/playwright/widget/test-helpers.ts +++ b/playwright/widget/test-helpers.ts @@ -21,7 +21,7 @@ const PASSWORD = "foobarbaz1!"; export const HOST1 = "https://app.m.localhost/#/welcome"; export const HOST2 = "https://app.othersite.m.localhost/#/welcome"; -export type RtcMode = "legacy" | "compat" | "2_0"; +export type RtcMode = "compat" | "2_0"; export class TestHelpers { public static async startCallInCurrentRoom( @@ -309,9 +309,7 @@ export class TestHelpers { // Move to Developer tab now await iframe.getByRole("tab", { name: "Developer" }).click(); - if (mode == "legacy") { - await iframe.getByText("Legacy: state events").click(); - } else if (mode == "2_0") { + if (mode == "2_0") { await iframe.getByText("Matrix 2.0").click(); } else { // compat diff --git a/src/config/ConfigOptions.ts b/src/config/ConfigOptions.ts index de6500b1d..edd17e4c6 100644 --- a/src/config/ConfigOptions.ts +++ b/src/config/ConfigOptions.ts @@ -12,9 +12,7 @@ Please see LICENSE in the repository root for full details. * Settings, or pinned for a deployment via `matrix_rtc_mode` in config.json. */ export enum MatrixRTCMode { - /** Legacy single-SFU + user-keyed memberships + legacy JWT endpoint. */ - Legacy = "legacy", - /** Multi-SFU transport, legacy JWT endpoint, no sticky events. */ + /** Multi-SFU transport, legacy JWT endpoint, state events. */ Compatibility = "compatibility", /** * Multi-SFU transport with: diff --git a/src/settings/DeveloperSettingsTab.test.tsx b/src/settings/DeveloperSettingsTab.test.tsx index d4c7b8c8f..a3a19938c 100644 --- a/src/settings/DeveloperSettingsTab.test.tsx +++ b/src/settings/DeveloperSettingsTab.test.tsx @@ -317,19 +317,15 @@ describe("DeveloperSettingsTab", () => { describe("matrix rtc mode", () => { afterEach(() => { - matrixRTCModeSetting.setValue(MatrixRTCMode.Legacy); + matrixRTCModeSetting.setValue(MatrixRTCMode.Compatibility); vi.restoreAllMocks(); }); function getModeRadios(): { - legacy: HTMLInputElement; compatibility: HTMLInputElement; matrix20: HTMLInputElement; } { return { - legacy: screen.getByDisplayValue( - MatrixRTCMode.Legacy, - ) as HTMLInputElement, compatibility: screen.getByDisplayValue( MatrixRTCMode.Compatibility, ) as HTMLInputElement, @@ -359,27 +355,21 @@ describe("DeveloperSettingsTab", () => { const radios = getModeRadios(); expect(radios.compatibility).toBeChecked(); - expect(radios.legacy).not.toBeChecked(); expect(radios.matrix20).not.toBeChecked(); // None are disabled by config; only Matrix_2_0 may be disabled by sticky-events support. - expect(radios.legacy).not.toBeDisabled(); expect(radios.compatibility).not.toBeDisabled(); }); - it.each([ - MatrixRTCMode.Legacy, - MatrixRTCMode.Compatibility, - MatrixRTCMode.Matrix_2_0, - ])( + it.each([MatrixRTCMode.Compatibility, MatrixRTCMode.Matrix_2_0])( "disables all radios and shows the config value (%s) as checked when matrix_rtc_mode is set", async (configMode) => { mockConfig({ matrix_rtc_mode: configMode }); // Local setting is intentionally different from the config value to // prove config wins. matrixRTCModeSetting.setValue( - configMode === MatrixRTCMode.Legacy - ? MatrixRTCMode.Compatibility - : MatrixRTCMode.Legacy, + configMode === MatrixRTCMode.Compatibility + ? MatrixRTCMode.Matrix_2_0 + : MatrixRTCMode.Compatibility, ); const client = createMockMatrixClient(); @@ -397,13 +387,11 @@ describe("DeveloperSettingsTab", () => { ); const radios = getModeRadios(); - expect(radios.legacy).toBeDisabled(); expect(radios.compatibility).toBeDisabled(); expect(radios.matrix20).toBeDisabled(); const checkedValue = ( { - [MatrixRTCMode.Legacy]: radios.legacy, [MatrixRTCMode.Compatibility]: radios.compatibility, [MatrixRTCMode.Matrix_2_0]: radios.matrix20, } as const diff --git a/src/settings/DeveloperSettingsTab.tsx b/src/settings/DeveloperSettingsTab.tsx index 70db13db9..0b45b4d3e 100644 --- a/src/settings/DeveloperSettingsTab.tsx +++ b/src/settings/DeveloperSettingsTab.tsx @@ -520,22 +520,6 @@ export const DeveloperSettingsTab: FC = ({ {matrixRTCModeForced &&

Your deployment overrides the mode.

}
- - } - > - - - {t("developer_mode.matrixRTCMode.Legacy.description")} - - renders and matches snapshot 1`] = ` class="_container_1ug7n_10" > -
-
- -
- - - Compatible with old versions of EC that do not support multi SFU - -
- -
-
-
- renders and matches snapshot 1`] = ` > Compatible with homeservers that do not support sticky events (but all other EC clients are v0.17.0 or later) @@ -366,9 +326,9 @@ exports[`DeveloperSettingsTab > renders and matches snapshot 1`] = ` class="_container_1ug7n_10" > renders and matches snapshot 1`] = ` > Compatible only with homservers supporting sticky events and all EC clients v0.17.0 or later @@ -491,7 +451,7 @@ exports[`DeveloperSettingsTab > renders and matches snapshot 1`] = ` class="_field_1bd8c0 _checkboxField_1bd8c0" > @@ -521,7 +481,7 @@ exports[`DeveloperSettingsTab > renders and matches snapshot 1`] = `

Configure resolution, framerate, bitrate, and codec for camera video. Changes apply on next call join.

@@ -543,7 +503,7 @@ exports[`DeveloperSettingsTab > renders and matches snapshot 1`] = ` class="_field_1bd8c0 _checkboxField_1bd8c0" > @@ -573,7 +533,7 @@ exports[`DeveloperSettingsTab > renders and matches snapshot 1`] = `

Configure resolution, framerate, bitrate, and codec for screen sharing

@@ -598,7 +558,7 @@ exports[`DeveloperSettingsTab > renders and matches snapshot 1`] = ` class="_field_1bd8c0 _checkboxField_1bd8c0" > renders and matches snapshot 1`] = ` class="_field_1bd8c0 _checkboxField_1bd8c0" > renders and matches snapshot 1`] = ` class="_field_1bd8c0 _checkboxField_1bd8c0" > { +const modes = [[MatrixRTCMode.Compatibility], [MatrixRTCMode.Matrix_2_0]]; + +describe.each(modes)("CallViewModel (%s mode)", (mode) => { const withCallViewModel = withCallViewModelInMode(mode); test("participants are retained during a focus switch", () => { diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index 87512b446..43d42a98c 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -441,7 +441,7 @@ export function createCallViewModel$( const matrixRTCMode$ = configMatrixRTCMode !== undefined ? constant(configMatrixRTCMode) - : (options.matrixRTCMode$ ?? constant(MatrixRTCMode.Legacy)); + : (options.matrixRTCMode$ ?? constant(MatrixRTCMode.Compatibility)); // Each hbar seperates a block of input variables required for the CallViewModel to function. // The outputs of this block is written under the hbar. @@ -503,7 +503,6 @@ export function createCallViewModel$( mode === MatrixRTCMode.Matrix_2_0 ? JwtEndpointVersion.Matrix_2_0 : JwtEndpointVersion.Legacy, - useOldestMember: mode === MatrixRTCMode.Legacy, }), ), ), diff --git a/src/state/CallViewModel/localMember/LocalMember.test.ts b/src/state/CallViewModel/localMember/LocalMember.test.ts index d273d1acb..8743559da 100644 --- a/src/state/CallViewModel/localMember/LocalMember.test.ts +++ b/src/state/CallViewModel/localMember/LocalMember.test.ts @@ -59,7 +59,7 @@ import { initializeWidget(); -const MATRIX_RTC_MODE = MatrixRTCMode.Legacy; +const MATRIX_RTC_MODE = MatrixRTCMode.Compatibility; const getUrlParams = vi.hoisted(() => vi.fn(() => ({}))); vi.mock("../../../UrlParams", () => ({ getUrlParams })); vi.mock("@livekit/components-core", () => ({ @@ -71,12 +71,6 @@ vi.mock("@livekit/components-core", () => ({ describe("LocalMembership", () => { describe("enterRTCSession", () => { it("It joins the correct Session", () => { - const focusFromOlderMembership = { - type: "livekit", - livekit_service_url: "http://my-oldest-member-service-url.com", - livekit_alias: "my-oldest-member-service-alias", - }; - mockConfig({ livekit: { livekit_service_url: "http://my-default-service-url.com" }, }); @@ -95,10 +89,6 @@ describe("LocalMembership", () => { }, }, memberships: [], - getFocusInUse: vi.fn().mockReturnValue(focusFromOlderMembership), - getOldestMembership: vi.fn().mockReturnValue({ - getPreferredFoci: vi.fn().mockReturnValue([focusFromOlderMembership]), - }), joinRTCSession: vi.fn(), }) as unknown as MatrixRTCSession; @@ -122,14 +112,12 @@ describe("LocalMembership", () => { memberId: "@alice:example.org:DEVICE", userId: "@alice:example.org", }, - [ - { - livekit_alias: "roomId", - livekit_service_url: "http://my-livekit-service-url.com", - type: "livekit", - }, - ], - undefined, + [], + { + livekit_alias: "roomId", + livekit_service_url: "http://my-livekit-service-url.com", + type: "livekit", + }, expect.objectContaining({ manageMediaKeys: true }), ); }); diff --git a/src/state/CallViewModel/localMember/LocalMember.ts b/src/state/CallViewModel/localMember/LocalMember.ts index 5108b7e3b..bf4ce01cf 100644 --- a/src/state/CallViewModel/localMember/LocalMember.ts +++ b/src/state/CallViewModel/localMember/LocalMember.ts @@ -117,7 +117,6 @@ export type LocalMemberState = }; /* - * - get oldest membership * - get transport to use * - get openId + jwt token * - wait for createTrack() call diff --git a/src/state/CallViewModel/localMember/LocalTransport.test.ts b/src/state/CallViewModel/localMember/LocalTransport.test.ts index 89cb831da..09f6ecec0 100644 --- a/src/state/CallViewModel/localMember/LocalTransport.test.ts +++ b/src/state/CallViewModel/localMember/LocalTransport.test.ts @@ -13,7 +13,6 @@ import { it, type MockedObject, vi, - type MockInstance, } from "vitest"; import { type CallMembership, @@ -26,7 +25,6 @@ import { mockConfig, flushPromises, ownMemberMock, - mockRtcMembership, testScope, } from "../../../utils/test"; import { @@ -35,7 +33,7 @@ import { type LocalTransportWithSFUConfig, } from "./LocalTransport"; import { constant } from "../../Behavior"; -import { Epoch, ObservableScope, trackEpoch } from "../../ObservableScope"; +import { Epoch, ObservableScope } from "../../ObservableScope"; import { MatrixRTCTransportMissingError, FailToGetOpenIdToken, @@ -58,7 +56,6 @@ describe("LocalTransport", () => { const { advertised$, active$ } = createLocalTransport$({ scope: testScope(), roomId: "!room:example.org", - useOldestMember: false, memberships$: constant(new Epoch([])), client: { // eslint-disable-next-line @typescript-eslint/naming-convention @@ -101,7 +98,6 @@ describe("LocalTransport", () => { const { advertised$, active$ } = createLocalTransport$({ scope, roomId: "!example_room_id", - useOldestMember: false, memberships$: constant(new Epoch([])), client: { baseUrl: "https://example.org", @@ -144,7 +140,6 @@ describe("LocalTransport", () => { const { advertised$, active$ } = createLocalTransport$({ scope: testScope(), roomId: "!room:example.org", - useOldestMember: false, memberships$: constant(new Epoch([])), client: { // eslint-disable-next-line @typescript-eslint/naming-convention @@ -185,127 +180,6 @@ describe("LocalTransport", () => { }); }); - describe("oldest member mode", () => { - const aliceTransport: LivekitTransportConfig = { - type: "livekit", - livekit_service_url: "https://alice.example.org", - }; - const bobTransport: LivekitTransportConfig = { - type: "livekit", - livekit_service_url: "https://bob.example.org", - }; - const aliceMembership = mockRtcMembership("@alice:example.org", "AAA", { - fociPreferred: [aliceTransport], - }); - const bobMembership = mockRtcMembership("@bob:example.org", "BBB", { - fociPreferred: [bobTransport], - }); - - let openIdSpy: MockInstance<(typeof openIDSFU)["getSFUConfigWithOpenID"]>; - beforeEach(() => { - openIdSpy = vi - .spyOn(openIDSFU, "getSFUConfigWithOpenID") - .mockResolvedValue(openIdResponse); - }); - - it("updates active transport when oldest member changes", async () => { - // Initially, Alice is the only member - const memberships$ = new BehaviorSubject([aliceMembership]); - - const scope = testScope(); - const { advertised$, active$ } = createLocalTransport$({ - scope, - roomId: "!example_room_id", - useOldestMember: true, - memberships$: scope.behavior(memberships$.pipe(trackEpoch())), - client: { - getDomain: () => "example.org", - // eslint-disable-next-line @typescript-eslint/naming-convention - _unstable_getRTCTransports: async () => Promise.resolve([]), - getOpenIdToken: vi.fn(), - getDeviceId: vi.fn(), - baseUrl: "https://example.org", - }, - ownMembershipIdentity: ownMemberMock, - forceJwtEndpoint: JwtEndpointVersion.Legacy, - delayId$: constant("delay_id_mock"), - }); - - expect(active$.value).toBe(null); - await flushPromises(); - // SFU config should've been fetched - expect(openIdSpy).toHaveBeenCalled(); - // Alice's transport should be active and advertised - expect(active$.value?.transport).toStrictEqual(aliceTransport); - expect(advertised$.value).toStrictEqual(aliceTransport); - - // Now Bob joins the call, but Alice is still the oldest member - openIdSpy.mockClear(); - memberships$.next([aliceMembership, bobMembership]); - await flushPromises(); - // No new SFU config should've been fetched - expect(openIdSpy).not.toHaveBeenCalled(); - // Alice's transport should still be active and advertised - expect(active$.value?.transport).toStrictEqual(aliceTransport); - expect(advertised$.value).toStrictEqual(aliceTransport); - - // Now Bob takes Alice's place as the oldest member - openIdSpy.mockClear(); - memberships$.next([bobMembership, aliceMembership]); - // Active transport should reset to null until we have Bob's SFU config - expect(active$.value).toStrictEqual(null); - await flushPromises(); - // Bob's SFU config should've been fetched - expect(openIdSpy).toHaveBeenCalled(); - // Bob's transport should be active, but Alice's should remain advertised - // (since we don't want the change in oldest member to cause a wave of new - // state events) - expect(active$.value?.transport).toStrictEqual(bobTransport); - expect(advertised$.value).toStrictEqual(aliceTransport); - }); - - it("advertises preferred transport when no other member exists", async () => { - // Initially, there are no members - const memberships$ = new BehaviorSubject([]); - - const scope = testScope(); - const { advertised$, active$ } = createLocalTransport$({ - scope, - roomId: "!example_room_id", - useOldestMember: true, - memberships$: scope.behavior(memberships$.pipe(trackEpoch())), - client: { - getDomain: () => "example.org", - // eslint-disable-next-line @typescript-eslint/naming-convention - _unstable_getRTCTransports: async () => - Promise.resolve([aliceTransport]), - getOpenIdToken: vi.fn(), - getDeviceId: vi.fn(), - baseUrl: "https://example.org", - }, - ownMembershipIdentity: ownMemberMock, - forceJwtEndpoint: JwtEndpointVersion.Legacy, - delayId$: constant("delay_id_mock"), - }); - - expect(active$.value).toBe(null); - await flushPromises(); - // Our own preferred transport should be advertised - expect(advertised$.value).toStrictEqual(aliceTransport); - // No transport should be active however (there is still no oldest member) - expect(active$.value).toBe(null); - - // Now Bob joins the call and becomes the oldest member - memberships$.next([bobMembership]); - await flushPromises(); - // We should still advertise our own preferred transport (to avoid - // unnecessary state changes) - expect(advertised$.value).toStrictEqual(aliceTransport); - // Bob's transport should become active - expect(active$.value?.transport).toBe(bobTransport); - }); - }); - type LocalTransportProps = Parameters[0]; describe("transport configuration mechanisms", () => { @@ -320,7 +194,6 @@ describe("LocalTransport", () => { ownMembershipIdentity: ownMemberMock, scope: testScope(), roomId: "!example_room_id", - useOldestMember: false, forceJwtEndpoint: JwtEndpointVersion.Legacy, delayId$: constant(null), memberships$: constant(new Epoch([])), @@ -433,7 +306,6 @@ describe("LocalTransport", () => { scope: testScope(), ownMembershipIdentity: ownMemberMock, roomId: "!example_room_id", - useOldestMember: false, forceJwtEndpoint: JwtEndpointVersion.Legacy, delayId$: constant(null), memberships$: constant(new Epoch([])), @@ -473,7 +345,6 @@ describe("LocalTransport", () => { ownMembershipIdentity: ownMemberMock, roomId: "!example_room_id", // We want multi-sdu - useOldestMember: false, forceJwtEndpoint: JwtEndpointVersion.Legacy, delayId$: delayId$, memberships$: constant(new Epoch([])), diff --git a/src/state/CallViewModel/localMember/LocalTransport.ts b/src/state/CallViewModel/localMember/LocalTransport.ts index 1a6dddc1f..f98a266fd 100644 --- a/src/state/CallViewModel/localMember/LocalTransport.ts +++ b/src/state/CallViewModel/localMember/LocalTransport.ts @@ -7,23 +7,16 @@ Please see LICENSE in the repository root for full details. import { type CallMembership, - isLivekitTransportConfig, type LivekitTransportConfig, } from "matrix-js-sdk/lib/matrixrtc"; import { type MatrixClient } from "matrix-js-sdk"; import { - catchError, combineLatest, distinctUntilChanged, - first, from, map, - merge, - type Observable, of, - startWith, switchMap, - tap, } from "rxjs"; import { logger as rootLogger, type Logger } from "matrix-js-sdk/lib/logger"; import { type CallMembershipIdentityParts } from "matrix-js-sdk/lib/matrixrtc/EncryptionManager"; @@ -47,8 +40,7 @@ import { RtcTransportAutoDiscovery } from "./RtcTransportAutoDiscovery.ts"; /* * It figures out “which LiveKit focus URL/alias the local user should use,” - * optionally aligning with the oldest member, and ensures the SFU path is primed - * before advertising that choice. + * and ensures the SFU path is primed before advertising that choice. */ interface Props { scope: ObservableScope; @@ -61,7 +53,6 @@ interface Props { OpenIDClientParts; // Used by the jwt service to create the livekit room and compute the livekit alias. roomId: string; - useOldestMember: boolean; forceJwtEndpoint: JwtEndpointVersion; delayId$: Behavior; } @@ -119,8 +110,6 @@ export interface LocalTransport { /** * Connects to the JWT service and determines the transports that the local member should use. * - * @prop useOldestMember Whether to use the same transport as the oldest member. - * This will only update once the first oldest member appears. Will not recompute if the oldest member leaves. * @prop useOldJwtEndpoint Whether to set forceOldJwtEndpoint on the returned transport and to use the old JWT endpoint. * This is used when the connection manager needs to know if it has to use the legacy endpoint which implies a string concatenated rtcBackendIdentity. * (which is expected for non sticky event based rtc member events) @@ -133,18 +122,10 @@ export const createLocalTransport$ = ({ ownMembershipIdentity, client, roomId, - useOldestMember, forceJwtEndpoint, delayId$, }: Props): LocalTransport => { const logger = rootLogger.getChild("[LocalTransport]"); - // The LiveKit transport in use by the oldest RTC membership. `null` when the - // oldest member has no such transport. - const oldestMemberTransport$ = observerOldestMembership$( - scope, - memberships$, - logger, - ); const transportDiscovery = new RtcTransportAutoDiscovery({ client: client, @@ -203,19 +184,6 @@ export const createLocalTransport$ = ({ }), ); - if (useOldestMember) { - return observeLocalTransportForOldestMembership( - scope, - oldestMemberTransport$, - preferredTransport$, - client, - ownMembershipIdentity, - roomId, - logger, - ); - } - - // --- Multi-SFU mode --- // Always publish on and advertise the preferred transport. return { advertised$: scope.behavior( @@ -243,47 +211,6 @@ export const createLocalTransport$ = ({ }; }; -/** - * Observes the oldest member in the room and returns the transport that it uses if it is a livekit transport. - * @param scope - The observable scope. - * @param memberships$ - The observable of the call's memberships.' - */ -function observerOldestMembership$( - scope: ObservableScope, - memberships$: Behavior>, - logger: Logger, -): Behavior { - return scope.behavior( - memberships$.pipe( - map((memberships) => { - const oldestMember = memberships.value[0]; - if (oldestMember === undefined) { - logger.info("Oldest member: not found"); - return null; - } - const transport = oldestMember.getTransport(oldestMember); - if (transport === undefined) { - logger.warn( - `Oldest member: ${oldestMember.userId}|${oldestMember.deviceId}|${oldestMember.memberId} has no transport`, - ); - return null; - } - if (!isLivekitTransportConfig(transport)) { - logger.warn( - `Oldest member: ${oldestMember.userId}|${oldestMember.deviceId}|${oldestMember.memberId} has invalid transport`, - ); - return null; - } - logger.info( - "Oldest member: ${oldestMember.userId}|${oldestMember.deviceId}|${oldestMember.memberId} has valid transport", - ); - return transport; - }), - distinctUntilChanged(areLivekitTransportsEqual), - ), - ); -} - /** * Utility to ensure the user can authenticate with the SFU. * We will call `getSFUConfigWithOpenID` once per transport here as it's our @@ -331,85 +258,6 @@ async function doOpenIdAndJWTFromUrl( }; } -function observeLocalTransportForOldestMembership( - scope: ObservableScope, - oldestMemberTransport$: Behavior, - preferredTransport$: Observable, - client: Pick< - MatrixClient, - "getDomain" | "baseUrl" | "_unstable_getRTCTransports" - > & - OpenIDClientParts, - ownMembershipIdentity: CallMembershipIdentityParts, - roomId: string, - logger: Logger, -): LocalTransport { - // Ensure we can authenticate with the SFU. - const authenticatedOldestMemberTransport$ = oldestMemberTransport$.pipe( - switchMap((transport) => { - // Oldest member not available -we are first- (or invalid SFU config). - if (transport === null) return of(null); - - // Whenever there is transport change we want to revert - // to no transport while we do the authentication. - // So do a from(promise) here to be able to startWith(null) - return from( - doOpenIdAndJWTFromUrl( - transport, - JwtEndpointVersion.Legacy, - ownMembershipIdentity, - roomId, - client, - undefined, - logger, - ), - ).pipe( - catchError((e: unknown) => { - logger.error( - `Failed to authenticate to transport ${transport.livekit_service_url}`, - e, - ); - throw mapAuthErrorToUserFriendlyError(e); - }), - startWith(null), - ); - }), - ); - - // --- Oldest member mode --- - return { - // Never update the transport that we advertise in our membership. Just - // take the first valid oldest member or preferred transport that we learn - // about, and stick with that. This avoids unnecessary SFU hops and room - // state changes. - advertised$: scope.behavior( - merge( - authenticatedOldestMemberTransport$.pipe( - map((t) => t?.transport ?? null), - ), - preferredTransport$.pipe(map((t) => t.transport)), - ).pipe( - first((t) => t !== null), - tap((t) => - logger.info(`Advertise transport: ${t.livekit_service_url}`), - ), - ), - null, - ), - // Publish on the transport used by the oldest member. - active$: scope.behavior( - authenticatedOldestMemberTransport$.pipe( - tap((t) => - logger.info( - `Publish on transport: ${t?.transport.livekit_service_url}`, - ), - ), - ), - null, - ), - }; -} - function mapAuthErrorToUserFriendlyError(e: unknown): Error { if ( e instanceof FailToGetOpenIdToken || diff --git a/src/state/CallViewModel/remoteMembers/Connection.ts b/src/state/CallViewModel/remoteMembers/Connection.ts index 013bd96c7..f320e6303 100644 --- a/src/state/CallViewModel/remoteMembers/Connection.ts +++ b/src/state/CallViewModel/remoteMembers/Connection.ts @@ -36,7 +36,6 @@ import { SFURoomCreationRestrictedError, UnknownCallError, } from "../../../utils/errors.ts"; -import { type JwtEndpointVersion } from "../localMember/LocalTransport.ts"; export interface ConnectionOpts { /** @@ -44,11 +43,6 @@ export interface ConnectionOpts { * On top the local transport will send additional data to the jwt server to use delayed event delegation. */ existingSFUConfig?: SFUConfig; - /** - * For local connections that use the oldest member pattern. here we have not prefetched the sfuConfig - * and hence we need to let the connection do the jwt token fetching. - */ - forceJwtEndpoint?: JwtEndpointVersion; /** The identity parts to use on this connection */ ownMembershipIdentity: CallMembershipIdentityParts; /** The media transport to connect to. */ diff --git a/src/state/CallViewModelWidget.test.ts b/src/state/CallViewModelWidget.test.ts index 2e4ef39dd..2f331bd32 100644 --- a/src/state/CallViewModelWidget.test.ts +++ b/src/state/CallViewModelWidget.test.ts @@ -35,11 +35,7 @@ vi.mock("../widget", () => ({ }, })); -it.each([ - [MatrixRTCMode.Legacy], - [MatrixRTCMode.Compatibility], - [MatrixRTCMode.Matrix_2_0], -])( +it.each([[MatrixRTCMode.Compatibility], [MatrixRTCMode.Matrix_2_0]])( "expect leave when ElementWidgetActions.HangupCall is called (%s mode)", async (mode) => { const pr = Promise.withResolvers(); diff --git a/src/utils/test-viewmodel.ts b/src/utils/test-viewmodel.ts index 526fc95c9..c8282ce51 100644 --- a/src/utils/test-viewmodel.ts +++ b/src/utils/test-viewmodel.ts @@ -171,7 +171,7 @@ export function getBasicCallViewModelEnvironment( setE2EEEnabled: async () => Promise.resolve(), }), connectionState$: constant(ConnectionState.Connected), - matrixRTCMode$: constant(MatrixRTCMode.Legacy), + matrixRTCMode$: constant(MatrixRTCMode.Compatibility), ...callViewModelOptions, }, handRaisedSubject$, diff --git a/src/utils/test.ts b/src/utils/test.ts index 206db88f5..fd4ce58f1 100644 --- a/src/utils/test.ts +++ b/src/utils/test.ts @@ -237,7 +237,7 @@ export function mockRtcMembership( fociPreferred: [exampleTransport], focusActive: { type: "livekit" as const, - focus_selection: "oldest_membership" as const, + focus_selection: "multi_sfu" as const, }, callId: "", membership: {}, @@ -463,9 +463,6 @@ export class MockRTCSession extends TypedEventEmitter< session.reemitEncryptionKeys = vi .fn<() => void>() .mockReturnValue(undefined); - session.getOldestMembership = vi - .fn<() => CallMembership | undefined>() - .mockReturnValue(this.memberships[0]); return session; }