Merge branch 'voip-team/call-viewmodel-refactor' into valere/call-viewmodel-refactor-logs

src/state/CallViewModel/CallViewModel.ts

@@ -182,6 +182,21 @@ export class CallViewModel {
     this.matrixRTCSession,
   );
 
+  // Each hbar seperates a block of input variables required for the CallViewModel to function.
+  // The outputs of this block is written under the hbar.
+  //
+  // For mocking purposes it is recommended to only mock the functions creating those outputs.
+  // All other fields are just temp computations for the mentioned output.
+  // The class does not need anything except the values underneath the bar.
+  // The creation of the values under the bar are all tested independently and testing the callViewModel Should
+  // not test their cretation. Call view model only needs:
+  //  - memberships$ via createMemberships$
+  //  - localMembership via createLocalMembership$
+  //  - callLifecycle via createCallNotificationLifecycle$
+  //  - matrixMemberMetadataStore via createMatrixMemberMetadata$
+
+  // ------------------------------------------------------------------------
+  // memberships$
   private memberships$ = createMemberships$(this.scope, this.matrixRTCSession);
 
   private membershipsAndTransports = membershipsAndTransports$(
@@ -189,6 +204,9 @@ export class CallViewModel {
     this.memberships$,
   );
 
+  // ------------------------------------------------------------------------
+  // matrixLivekitMembers$ AND localMembership
+
   private localTransport$ = createLocalTransport$({
     scope: this.scope,
     memberships$: this.memberships$,
@@ -199,28 +217,13 @@ export class CallViewModel {
     ),
   });
 
-  // ------------------------------------------------------------------------
-
   private connectionFactory = new ECConnectionFactory(
     this.matrixRoom.client,
     this.mediaDevices,
     this.trackProcessorState$,
     this.livekitKeyProvider,
     getUrlParams().controlledAudioDevices,
-  );
-
-  // Can contain duplicates. The connection manager will take care of this.
-  private allTransports$ = this.scope.behavior(
-    combineLatest(
-      [this.localTransport$, this.membershipsAndTransports.transports$],
-      (localTransport, transports) => {
-        const localTransportAsArray = localTransport ? [localTransport] : [];
-        return transports.mapInner((transports) => [
-          ...localTransportAsArray,
-          ...transports,
-        ]);
-      },
-    ),
+    this.options.livekitRoomFactory,
   );
 
   private connectionManager = createConnectionManager$({
@@ -230,8 +233,6 @@ export class CallViewModel {
     logger: logger,
   });
 
-  // ------------------------------------------------------------------------
-
   private matrixLivekitMembers$ = createMatrixLivekitMembers$({
     scope: this.scope,
     membershipsWithTransport$:
@@ -275,6 +276,7 @@ export class CallViewModel {
       ),
     ),
   );
+
   private localMatrixLivekitMemberUninitialized = {
     membership$: this.localRtcMembership$,
     participant$: this.localMembership.participant$,
@@ -296,6 +298,7 @@ export class CallViewModel {
     );
 
   // ------------------------------------------------------------------------
+  // callLifecycle
 
   private callLifecycle = createCallNotificationLifecycle$({
     scope: this.scope,
@@ -311,6 +314,13 @@ export class CallViewModel {
   public autoLeave$ = this.callLifecycle.autoLeave$;
 
   // ------------------------------------------------------------------------
+  // matrixMemberMetadataStore
+
+  private matrixMemberMetadataStore = createMatrixMemberMetadata$(
+    this.scope,
+    this.scope.behavior(this.memberships$.pipe(map((mems) => mems.value))),
+    createRoomMembers$(this.scope, this.matrixRoom),
+  );
 
   /**
    * If there is a configuration error with the call (e.g. misconfigured E2EE).
@@ -404,12 +414,6 @@ export class CallViewModel {
     ),
   );
 
-  private matrixMemberMetadataStore = createMatrixMemberMetadata$(
-    this.scope,
-    this.scope.behavior(this.memberships$.pipe(map((mems) => mems.value))),
-    createRoomMembers$(this.scope, this.matrixRoom),
-  );
-
   /**
    * List of user media (camera feeds) that we want tiles for.
    */
@@ -428,20 +432,23 @@ export class CallViewModel {
           { value: matrixLivekitMembers },
           duplicateTiles,
         ]) {
+          let localParticipantId = undefined;
           // add local member if available
           if (localMatrixLivekitMember) {
-            const {
-              userId,
-              participant$,
-              connection$,
-              // membership$,
-            } = localMatrixLivekitMember;
-            const participantId = participant$.value?.identity; // should be membership$.value.membershipID which is not optional
+            const { userId, participant$, connection$, membership$ } =
+              localMatrixLivekitMember;
+            localParticipantId = `${userId}:${membership$.value.deviceId}`; // should be membership$.value.membershipID which is not optional
             // const participantId = membership$.value.membershipID;
-            if (participantId) {
+            if (localParticipantId) {
               for (let dup = 0; dup < 1 + duplicateTiles; dup++) {
                 yield {
-                  keys: [dup, participantId, userId, participant$, connection$],
+                  keys: [
+                    dup,
+                    localParticipantId,
+                    userId,
+                    participant$,
+                    connection$,
+                  ],
                   data: undefined,
                 };
               }
@@ -452,11 +459,11 @@ export class CallViewModel {
             userId,
             participant$,
             connection$,
-            // membership$
+            membership$,
           } of matrixLivekitMembers) {
-            const participantId = participant$.value?.identity;
+            const participantId = `${userId}:${membership$.value.deviceId}`;
+            if (participantId === localParticipantId) continue;
             // const participantId = membership$.value?.identity;
-            if (!participantId) continue;
             for (let dup = 0; dup < 1 + duplicateTiles; dup++) {
               yield {
                 keys: [dup, participantId, userId, participant$, connection$],
@@ -552,9 +559,8 @@ export class CallViewModel {
    *  - There can be multiple participants for one Matrix user if they join from
    *    multiple devices.
    */
-  // TODO KEEP THIS!! and adapt it to what our membershipManger returns
   public readonly participantCount$ = this.scope.behavior(
-    this.memberships$.pipe(map((ms) => ms.value.length)),
+    this.matrixLivekitMembers$.pipe(map((ms) => ms.value.length)),
   );
 
   // only public to expose to the view.

src/state/CallViewModel/localMember/LocalMembership.ts

@@ -54,7 +54,7 @@ import { PosthogAnalytics } from "../../../analytics/PosthogAnalytics.ts";
 import { MatrixRTCMode } from "../../../settings/settings.ts";
 import { Config } from "../../../config/Config.ts";
 import { type Connection } from "../remoteMembers/Connection.ts";
-
+const logger = rootLogger.getChild("[LocalMembership]");
 export enum LivekitState {
   Uninitialized = "uninitialized",
   Connecting = "connecting",
@@ -356,10 +356,19 @@ export const createLocalMembership$ = ({
         !connectRequested ||
         state.matrix$.value.state !== MatrixState.Disconnected
       ) {
-        logger.info("Waiting for transport to enter rtc session");
+        logger.info(
+          "Not yet connecting because: ",
+          "transport === null:",
+          transport === null,
+          "!connectRequested:",
+          !connectRequested,
+          "state.matrix$.value.state !== MatrixState.Disconnected:",
+          state.matrix$.value.state !== MatrixState.Disconnected,
+        );
         return;
       }
       state.matrix$.next({ state: MatrixState.Connecting });
+      logger.info("Matrix State connecting");
       enterRTCSession(matrixRTCSession, transport, options.value).catch(
         (error) => {
           logger.error(error);
@@ -409,7 +418,9 @@ export const createLocalMembership$ = ({
         for (const p of publications) {
           if (p.track?.isUpstreamPaused === true) {
             const kind = p.track.kind;
-            logger.log(`Resuming ${kind} track (MatrixRTC connection present)`);
+            logger.info(
+              `Resuming ${kind} track (MatrixRTC connection present)`,
+            );
             p.track
               .resumeUpstream()
               .catch((e) =>
@@ -424,7 +435,7 @@ export const createLocalMembership$ = ({
         for (const p of publications) {
           if (p.track?.isUpstreamPaused === false) {
             const kind = p.track.kind;
-            logger.log(
+            logger.info(
               `Pausing ${kind} track (uncertain MatrixRTC connection)`,
             );
             p.track

src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.ts

@@ -22,7 +22,7 @@ import { Epoch, type ObservableScope } from "../../ObservableScope";
 import { type Connection } from "./Connection";
 import { generateItemsWithEpoch } from "../../../utils/observable";
 
-const logger = rootLogger.getChild("MatrixLivekitMembers");
+const logger = rootLogger.getChild("[MatrixLivekitMembers]");
 
 /**
  * Represents a Matrix call member and their associated LiveKit participation.

src/state/ObservableScope.ts

@@ -74,22 +74,14 @@ export class ObservableScope {
     // they will no longer re-emit their current value upon subscription. We want
     // to support Observables that complete (for example `of({})`), so we have to
     // take care to not propagate the completion event.
-    setValue$
-      .pipe(
-        this.bind(),
-        distinctUntilChanged((a, b) => {
-          logger.log("distinctUntilChanged", a, b);
-          return a === b;
-        }),
-      )
-      .subscribe({
-        next(value) {
-          subject$.next(value);
-        },
-        error(err: unknown) {
-          subject$.error(err);
-        },
-      });
+    setValue$.pipe(this.bind(), distinctUntilChanged()).subscribe({
+      next(value) {
+        subject$.next(value);
+      },
+      error(err: unknown) {
+        subject$.error(err);
+      },
+    });
     if (subject$.value === nothing)
       throw new Error("Behavior failed to synchronously emit an initial value");
     return subject$ as Behavior<T>;

src/state/SessionBehaviors.ts

@@ -76,6 +76,6 @@ export const createMemberships$ = (
       MatrixRTCSessionEvent.MembershipsChanged,
       (_, memberships: CallMembership[]) => memberships,
     ).pipe(trackEpoch()),
-    new Epoch([]),
+    new Epoch(matrixRTCSession.memberships),
   );
 };

src/utils/test-fixtures.ts

@@ -11,9 +11,9 @@ import {
   mockRemoteParticipant,
 } from "./test";
 
-export const localRtcMember = mockRtcMembership("@carol:example.org", "1111");
+export const localRtcMember = mockRtcMembership("@local:example.org", "1111");
 export const localRtcMemberDevice2 = mockRtcMembership(
-  "@carol:example.org",
+  "@local:example.org",
   "2222",
 );
 export const local = mockMatrixRoomMember(localRtcMember);

src/utils/test.ts

@@ -6,7 +6,14 @@ Please see LICENSE in the repository root for full details.
 */
 import { map, type Observable, of, type SchedulerLike } from "rxjs";
 import { type RunHelpers, TestScheduler } from "rxjs/testing";
-import { expect, type MockedObject, onTestFinished, vi, vitest } from "vitest";
+import {
+  expect,
+  type MockedObject,
+  type MockInstance,
+  onTestFinished,
+  vi,
+  vitest,
+} from "vitest";
 import {
   MatrixEvent,
   type Room as MatrixRoom,
@@ -269,6 +276,7 @@ export function mockLivekitRoom(
   }: { remoteParticipants$?: Observable<RemoteParticipant[]> } = {},
 ): LivekitRoom {
   const livekitRoom = {
+    options: {},
     ...mockEmitter(),
     ...room,
   } as Partial<LivekitRoom> as LivekitRoom;
@@ -291,6 +299,7 @@ export function mockLocalParticipant(
   return {
     isLocal: true,
     trackPublications: new Map(),
+    unpublishTracks: async () => Promise.resolve(),
     getTrackPublication: () =>
       ({}) as Partial<LocalTrackPublication> as LocalTrackPublication,
     ...mockEmitter(),
@@ -331,6 +340,8 @@ export function mockRemoteParticipant(
     setVolume() {},
     getTrackPublication: () =>
       ({}) as Partial<RemoteTrackPublication> as RemoteTrackPublication,
+    // this will only get used for `getTrackPublications().length`
+    getTrackPublications: () => [0],
     ...mockEmitter(),
     ...participant,
   } as RemoteParticipant;
@@ -363,13 +374,16 @@ export function createRemoteMedia(
   );
 }
 
-export function mockConfig(config: Partial<ResolvedConfigOptions> = {}): void {
-  vi.spyOn(Config, "get").mockReturnValue({
+export function mockConfig(
+  config: Partial<ResolvedConfigOptions> = {},
+): MockInstance<() => ResolvedConfigOptions> {
+  const spy = vi.spyOn(Config, "get").mockReturnValue({
     ...DEFAULT_CONFIG,
     ...config,
   });
   // simulate loading the config
   vi.spyOn(Config, "init").mockResolvedValue(void 0);
+  return spy;
 }
 
 export class MockRTCSession extends TypedEventEmitter<