Remove hard-coded video tile count from end-to-end tests

By keying the connections by the transport's resolved auth flow. This is important because connecting to the same SFU twice with the same participant ID will cause a 'duplicate participant ID' error that stops one of the connections.

playwright/spa-helpers.ts

@@ -133,7 +133,7 @@ async function disableLeaveDelegation(page: Page): Promise<void> {
  * Expect a certain number of video tiles to be present and visible.
  */
 async function expectVideoTilesCount(page: Page, count: number): Promise<void> {
-  await expect(page.getByTestId("videoTile")).toHaveCount(2);
+  await expect(page.getByTestId("videoTile")).toHaveCount(count);
 
   // No one should be waiting for media
   await expect(page.getByText("Waiting for media...")).not.toBeVisible({

playwright/url-transport.spec.ts

@@ -160,6 +160,37 @@ test("Publisher shows an error when the homeserver lacks MSC4195", async ({
   ).toBeVisible();
 });
 
+test("Group call with all possible transport shapes at once", async ({
+  browser,
+  page,
+  browserName,
+}) => {
+  skipOnFirefox(browserName);
+
+  // The host starts the call with a url transport.
+  await advertiseTransport(page, shapes.url);
+  await page.goto("/");
+  await SpaHelpers.createCall(page, "Alice", "HelloCall", true);
+  const inviteLink = await SpaHelpers.getCallInviteLink(page);
+
+  // The guest with a legacy transport joins.
+  const bobContext = await browser.newContext({ reducedMotion: "reduce" });
+  const bobPage = await bobContext.newPage();
+  await advertiseTransport(bobPage, shapes.legacy);
+  await SpaHelpers.joinCallFromInviteLink(bobPage, inviteLink, "Bob");
+
+  // The guest with a url+legacy transport joins.
+  const carolContext = await browser.newContext({ reducedMotion: "reduce" });
+  const carolPage = await carolContext.newPage();
+  await advertiseTransport(carolPage, shapes["url+legacy"]);
+  await SpaHelpers.joinCallFromInviteLink(carolPage, inviteLink, "Carol");
+
+  // Wait for the call to connect and render.
+  await SpaHelpers.expectVideoTilesCount(page, 3);
+  await SpaHelpers.expectVideoTilesCount(bobPage, 3);
+  await SpaHelpers.expectVideoTilesCount(carolPage, 3);
+});
+
 /**
  * Delegation follows the flow that authenticated the publisher: the homeserver
  * for a `url`-only transport, the JWT service wherever there is one.

sdk/src/media-backend/livekit/ConnectionMap.test.ts

@@ -24,7 +24,7 @@ import {
 import { type ConnectionFactory } from "./ConnectionFactory.ts";
 import { type Connection } from "./Connection.ts";
 import { ownMemberMock, withTestScheduler } from "../../utils/test";
-import { type Behavior } from "../../reactive/Behavior";
+import { constant, type Behavior } from "../../reactive/Behavior";
 import { type TransportLocator } from "./auth";
 
 const TRANSPORT_1: TransportLocator = {
@@ -207,7 +207,7 @@ describe("connections$ stream", () => {
     });
   });
 
-  test("Keeps one connection per homeserver for transports with a url", () => {
+  test("Keeps one connection per homeserver for url transports", () => {
     withTestScheduler(({ behavior, expectObservable }) => {
       const urlTransport: TransportLocator = {
         transport: { type: "livekit", url: "wss://sfu.example.org" },
@@ -243,6 +243,94 @@ describe("connections$ stream", () => {
     });
   });
 
+  test("Shares connection between local livekit_service_url transport and remote url + livekit_service_url transport", () => {
+    withTestScheduler(({ behavior, expectObservable, scope }) => {
+      const serviceUrlTransport: TransportLocator = {
+        transport: {
+          type: "livekit",
+          livekit_service_url: "https://jwt.example.org",
+        },
+        serverName: "example.org",
+      };
+      const urlAndServiceUrlTransport: TransportLocator = {
+        transport: {
+          type: "livekit",
+          livekit_service_url: "https://jwt.example.org",
+          url: "wss://sfu.elsewhere.org",
+        },
+        // Should pass even if local server != remote server
+        serverName: "elsewhere.org",
+      };
+      const connectionMap$ = createConnectionMap$({
+        scope,
+        connectionFactory: fakeConnectionFactory,
+        localTransport: serviceUrlTransport,
+        remoteTransports$: constant(new Epoch([urlAndServiceUrlTransport])),
+        logger,
+        ownMembershipIdentity: ownMemberMock,
+      });
+
+      expectObservable(
+        connectionMap$.pipe(mapEpoch((d) => d.getConnections())),
+      ).toBe("a", {
+        a: expect.toSatisfy(({ value: cs }: Epoch<Connection[]>) => {
+          // One connection total, since local and remote connections are shared
+          expect(cs.length).toBe(1);
+          expect(findConnection(serviceUrlTransport, cs)).toBeDefined();
+          expect(findConnection(serviceUrlTransport, cs)).toBe(
+            findConnection(urlAndServiceUrlTransport, cs),
+          );
+          return true;
+        }),
+      });
+    });
+  });
+
+  test("Shares connection between remote livekit_service_url transport and remote url + livekit_service_url transport", () => {
+    withTestScheduler(({ behavior, expectObservable, scope }) => {
+      const serviceUrlTransport: TransportLocator = {
+        transport: {
+          type: "livekit",
+          livekit_service_url: "https://jwt.example.org",
+        },
+        serverName: "example.org",
+      };
+      const urlAndServiceUrlTransport: TransportLocator = {
+        transport: {
+          type: "livekit",
+          livekit_service_url: "https://jwt.example.org",
+          url: "wss://sfu.elsewhere.org",
+        },
+        // Should pass even if local server != remote server
+        serverName: "elsewhere.org",
+      };
+      const connectionMap$ = createConnectionMap$({
+        scope,
+        connectionFactory: fakeConnectionFactory,
+        localTransport: TRANSPORT_2,
+        remoteTransports$: constant(
+          new Epoch([serviceUrlTransport, urlAndServiceUrlTransport]),
+        ),
+        logger,
+        ownMembershipIdentity: ownMemberMock,
+      });
+
+      expectObservable(
+        connectionMap$.pipe(mapEpoch((d) => d.getConnections())),
+      ).toBe("a", {
+        a: expect.toSatisfy(({ value: cs }: Epoch<Connection[]>) => {
+          // Two connections total: a local connection and a shared remote connection
+          expect(cs.length).toBe(2);
+          expect(findConnection(serviceUrlTransport, cs)).toBeDefined();
+          expect(findConnection(serviceUrlTransport, cs)).toBe(
+            findConnection(urlAndServiceUrlTransport, cs),
+          );
+          return true;
+        }),
+      });
+    });
+  });
+
   test("Should start connection only once", () => {
     withTestScheduler(({ behavior, expectObservable }) => {
       const connectionMap$ = createConnectionMap$({
@@ -344,7 +432,7 @@ describe("ConnectionMap", () => {
     data.add({ ...connection } as Connection, [p("b")]);
     expect(warn).toHaveBeenCalledWith(
       expect.stringContaining(
-        'second connection to [null,"https://lk.example.org",null]: existing [a], adding [b]',
+        'second connection to ["openID","https://lk.example.org"]: existing [a], adding [b]',
       ),
     );
     expect(data.getParticipantsForTransport(TRANSPORT_1)).toHaveLength(2);

sdk/src/media-backend/livekit/ConnectionMap.ts

@@ -18,7 +18,11 @@ import {
   type ObservableScope,
 } from "../../reactive/ObservableScope";
 import { generateItemsWithEpoch } from "../../reactive/observable";
-import { type ConnectionRole, type TransportLocator } from "./auth";
+import {
+  chooseAuthFlow,
+  type ConnectionRole,
+  type TransportLocator,
+} from "./auth";
 import { type Connection } from "./Connection";
 import { type ConnectionFactory } from "./ConnectionFactory";
 
@@ -26,22 +30,25 @@ import { type ConnectionFactory } from "./ConnectionFactory";
 export type TransportKey = string & { __transportKey: true };
 
 /**
- * The key a locator is identified by in a map or set. Locators with the same
- * key share a connection: the server name only takes part in authentication
- * for a transport with a `url`, where our homeserver asks that server for the
- * token. A JWT service is reached directly, so members of several homeservers
- * advertising the same one share a connection rather than each get their own.
+ * Converts a {@link TransportLocator} to a key suitable for identifying it in a
+ * map of connections.
  */
 export function keyFor({
   transport,
   serverName,
 }: TransportLocator): TransportKey {
-  const hasUrl = "url" in transport;
+  // Base the key on the auth flow we would use, so that we never create two
+  // connections that share the same auth details. (That would be bad, as we
+  // might then try to connect to the same SFU twice with the same participant
+  // ID, resulting in only one working connection.)
+  // TODO: This strategy will not work once `chooseAuthFlow` begins allowing for
+  // falling back from the CS API flow to the OpenID flow.
+  const flow = chooseAuthFlow(transport);
   // Serialize the fields in a consistent order
   return JSON.stringify([
-    hasUrl ? transport.url : null,
-    "livekit_service_url" in transport ? transport.livekit_service_url : null,
-    hasUrl ? serverName : null,
+    flow.kind,
+    // With the CS API flow, we additionally need to distinguish server names
+    ...(flow.kind === "csApi" ? [flow.url, serverName] : [flow.serviceUrl]),
   ]) as TransportKey;
 }