Skip to content

Commit

Permalink
FCE-562 Restore participant metadata (#109)
Browse files Browse the repository at this point in the history
## Description

`metadata` field now holds `peer` and `server` metadata. 

SDK doesn't handle server metadata at all. There is a separate task for
that:

https://linear.app/swmansion/issue/FCE-563/handle-server-metadata-in-web-client-sdk

This is a basic fix. I didn't want to refactor or redesign anything.

## Motivation and Context

Peer / Participant metadata didn't work.

## Types of changes

Bug fix (non-breaking change which fixes an issue)

---------

Co-authored-by: Adrian Czerwiec <[email protected]>
  • Loading branch information
kamil-stasiak and czerwiukk authored Sep 27, 2024
1 parent 6a51596 commit 901b989
Show file tree
Hide file tree
Showing 15 changed files with 79 additions and 41 deletions.
9 changes: 9 additions & 0 deletions .github/workflows/playwright.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,15 @@ jobs:
- name: Install Playwright Browsers 🧭
run: npx playwright install --with-deps

- name: Login to GitHub Container Registry
run: echo ${{ secrets.GITHUB_TOKEN }} | docker login ghcr.io -u ${{ github.actor }} --password-stdin

- name: Pull Fishjam for ts-client tests
run: docker compose -f e2e-tests/ts-client/docker-compose-test.yaml pull fishjam

- name: Pull Fishjam for react-client tests
run: docker compose -f e2e-tests/react-client/docker-compose-test.yaml pull fishjam

- name: Run Playwright tests 🧪
run: yarn test:e2e

Expand Down
2 changes: 1 addition & 1 deletion e2e-tests/react-client/docker-compose-test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ version: "3"

services:
fishjam:
image: "ghcr.io/fishjam-dev/fishjam:${FISHJAM_VERSION:-0.6.3}"
image: "ghcr.io/fishjam-cloud/fishjam:0.8.0"
container_name: fishjam
restart: on-failure
healthcheck:
Expand Down
6 changes: 4 additions & 2 deletions e2e-tests/ts-client/app/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -347,11 +347,13 @@ export function App() {
<details key={id} open>
<summary>{id}</summary>
metadata:{" "}
<code id={`metadata-${id}`}>{JSON.stringify(metadata)}</code>
<code id={`metadata-${id}`}>
{JSON.stringify(metadata.peer)}
</code>
<br />
raw metadata:{" "}
<code id={`raw-metadata-${id}`}>
{JSON.stringify(rawMetadata)}
{JSON.stringify(rawMetadata.peer)}
</code>
<br />
metadata parsing error:{" "}
Expand Down
2 changes: 1 addition & 1 deletion e2e-tests/ts-client/docker-compose-test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ version: '3'

services:
fishjam:
image: "ghcr.io/fishjam-dev/fishjam:${FISHJAM_VERSION:-0.6.3}"
image: "ghcr.io/fishjam-cloud/fishjam:0.8.0"
container_name: fishjam
restart: on-failure
healthcheck:
Expand Down
2 changes: 1 addition & 1 deletion examples/react-client/fishjam-chat/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ function App() {
screenShareAudioTrack,
metadata,
}) => {
const label = metadata?.displayName ?? id;
const label = metadata?.peer?.displayName ?? id;

return (
<Fragment key={id}>
Expand Down
2 changes: 1 addition & 1 deletion packages/react-client/src/hooks/useScreenShare.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export const useScreenShare = (): ScreenshareApi => {
const stream = state.stream ?? null;
const [videoTrack, audioTrack] = stream ? getTracks(stream) : [null, null];

const getDisplayName = () => tsClient.getLocalPeer()?.metadata?.displayName;
const getDisplayName = () => tsClient.getLocalPeer()?.metadata?.peer?.displayName;

const startStreaming: ScreenshareApi["startStreaming"] = async (props) => {
const stream = await navigator.mediaDevices.getDisplayMedia({
Expand Down
2 changes: 1 addition & 1 deletion packages/react-client/src/hooks/useTrackManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export const useTrackManager = ({
// see `getRemoteOrLocalTrackContext()` explanation
setCurrentTrackId(media.track.id);

const displayName = tsClient.getLocalPeer()?.metadata?.displayName;
const displayName = tsClient.getLocalPeer()?.metadata?.peer?.displayName;

const trackMetadata: TrackMetadata = { ...metadata, displayName, paused: false };

Expand Down
5 changes: 4 additions & 1 deletion packages/react-client/src/state.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ export type TrackWithOrigin = Track & {

export type PeerState = {
id: PeerId;
metadata?: PeerMetadata;
metadata: {
peer?: PeerMetadata;
server: Record<string, unknown>;
};
rawMetadata: Record<string, unknown>;
metadataParsingError?: any; // eslint-disable-line @typescript-eslint/no-explicit-any
tracks: Record<TrackId, Track>;
Expand Down
2 changes: 1 addition & 1 deletion packages/ts-client/src/reconnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export class ReconnectManager<PeerMetadata, TrackMetadata> {
}

private getLastPeerMetadata(): PeerMetadata | undefined {
return this.lastLocalEndpoint?.metadata;
return this.lastLocalEndpoint?.metadata?.peer;
}

private reconnect() {
Expand Down
20 changes: 13 additions & 7 deletions packages/ts-client/src/webrtc/tracks/Local.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,14 @@ export class Local<EndpointMetadata, TrackMetadata> {
private readonly localEndpoint: EndpointWithTrackContext<EndpointMetadata, TrackMetadata> = {
id: '',
type: 'webrtc',
metadata: undefined,
rawMetadata: undefined,
metadata: {
peer: undefined,
server: undefined,
},
rawMetadata: {
peer: undefined,
server: undefined,
},
tracks: new Map(),
};

Expand Down Expand Up @@ -170,14 +176,14 @@ export class Local<EndpointMetadata, TrackMetadata> {

public setEndpointMetadata = (metadata: EndpointMetadata) => {
try {
this.localEndpoint.metadata = this.endpointMetadataParser(metadata);
this.localEndpoint.metadata.peer = this.endpointMetadataParser(metadata);
this.localEndpoint.metadataParsingError = undefined;
} catch (error) {
this.localEndpoint.metadata = undefined;
this.localEndpoint.metadata.peer = undefined;
this.localEndpoint.metadataParsingError = error;
throw error;
}
this.localEndpoint.rawMetadata = metadata;
this.localEndpoint.rawMetadata.peer = metadata;
};

public getEndpoint = (): EndpointWithTrackContext<EndpointMetadata, TrackMetadata> => {
Expand Down Expand Up @@ -234,12 +240,12 @@ export class Local<EndpointMetadata, TrackMetadata> {
};

public updateEndpointMetadata = (metadata: unknown) => {
this.localEndpoint.metadata = this.endpointMetadataParser(metadata);
this.localEndpoint.metadata.peer = this.endpointMetadataParser(metadata);
this.localEndpoint.rawMetadata = this.localEndpoint.metadata;
this.localEndpoint.metadataParsingError = undefined;

const mediaEvent = generateMediaEvent('updateEndpointMetadata', {
metadata: this.localEndpoint.metadata,
metadata: this.localEndpoint.metadata.peer,
});
this.sendMediaEvent(mediaEvent);
this.emit('localEndpointMetadataChanged', {
Expand Down
14 changes: 7 additions & 7 deletions packages/ts-client/src/webrtc/tracks/Remote.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,14 +105,14 @@ export class Remote<EndpointMetadata, TrackMetadata> {
const newEndpoint: EndpointWithTrackContext<EndpointMetadata, TrackMetadata> = {
id: endpoint.id,
type: endpoint.type,
metadata: undefined,
rawMetadata: undefined,
metadata: { peer: undefined, server: undefined },
rawMetadata: { peer: undefined, server: undefined },
metadataParsingError: undefined,
tracks: new Map(),
};

// mutation in place
this.updateEndpointMetadata(newEndpoint, endpoint.metadata);
this.updateEndpointMetadata(newEndpoint, endpoint?.metadata?.peer);

this.addEndpoint(newEndpoint);
this.addTracks(newEndpoint.id, endpoint.tracks, endpoint.trackIdToMetadata);
Expand All @@ -132,7 +132,7 @@ export class Remote<EndpointMetadata, TrackMetadata> {
if (!endpoint) throw new Error(`Endpoint ${data.id} not found`);

// mutation in place
this.updateEndpointMetadata(endpoint, data.metadata);
this.updateEndpointMetadata(endpoint, data.metadata.peer);

this.emit('endpointUpdated', endpoint);
};
Expand All @@ -142,13 +142,13 @@ export class Remote<EndpointMetadata, TrackMetadata> {
metadata: unknown,
) => {
try {
endpoint.metadata = this.endpointMetadataParser(metadata);
endpoint.metadata.peer = this.endpointMetadataParser(metadata);
endpoint.metadataParsingError = undefined;
} catch (error) {
endpoint.metadata = undefined;
endpoint.metadata.peer = undefined;
endpoint.metadataParsingError = error;
}
endpoint.rawMetadata = metadata;
endpoint.rawMetadata.peer = metadata;
};

public removeRemoteEndpoint = (endpointId: EndpointId) => {
Expand Down
12 changes: 9 additions & 3 deletions packages/ts-client/src/webrtc/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ export interface WebRTCEndpointEvents<EndpointMetadata, TrackMetadata> {

localTrackEncodingDisabled: (event: { trackId: string; encoding: Encoding }) => void;

localEndpointMetadataChanged: (event: { metadata: EndpointMetadata }) => void;
localEndpointMetadataChanged: (event: { metadata: { peer?: EndpointMetadata; server?: unknown } }) => void;

localTrackMetadataChanged: (event: { trackId: string; metadata: TrackMetadata }) => void;
}
Expand All @@ -321,8 +321,14 @@ export interface Endpoint<EndpointMetadata, TrackMetadata> {
/**
* Any information that was provided in {@link WebRTCEndpoint.connect}.
*/
metadata?: EndpointMetadata;
rawMetadata: any;
metadata: {
peer?: EndpointMetadata;
server: any;
};
rawMetadata: {
peer?: any;
server: any;
};
metadataParsingError?: any;
/**
* List of tracks that are sent by the endpoint.
Expand Down
24 changes: 14 additions & 10 deletions packages/ts-client/tests/events/endpointUpdatedEvent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { WebRTCEndpoint } from '../../src';
import {
createConnectedEvent,
createConnectedEventWithOneEndpoint,
createEndpointUpdated,
createEndpointUpdatedPeerMetadata,
endpointId,
notExistingEndpointId,
} from '../fixtures';
Expand All @@ -22,11 +22,11 @@ it('Update existing endpoint metadata', () => {
newField: 'new field value',
};

webRTCEndpoint.receiveMediaEvent(JSON.stringify(createEndpointUpdated(endpointId, metadata)));
webRTCEndpoint.receiveMediaEvent(JSON.stringify(createEndpointUpdatedPeerMetadata(endpointId, metadata)));

// Then
const endpoint = webRTCEndpoint.getRemoteEndpoints()[endpointId]!;
expect(endpoint.metadata).toMatchObject(metadata);
expect(endpoint.metadata.peer).toMatchObject(metadata);
});

it('Update existing endpoint produce event', () =>
Expand All @@ -44,12 +44,12 @@ it('Update existing endpoint produce event', () =>

webRTCEndpoint.on('endpointUpdated', (endpoint) => {
// Then
expect(endpoint.metadata).toMatchObject(metadata);
expect(endpoint.metadata.peer).toMatchObject(metadata);
done('');
});

// When
webRTCEndpoint.receiveMediaEvent(JSON.stringify(createEndpointUpdated(endpointId, metadata)));
webRTCEndpoint.receiveMediaEvent(JSON.stringify(createEndpointUpdatedPeerMetadata(endpointId, metadata)));
}));

it('Update existing endpoint with undefined metadata', () => {
Expand All @@ -62,11 +62,11 @@ it('Update existing endpoint with undefined metadata', () => {

// When
const metadata = undefined;
webRTCEndpoint.receiveMediaEvent(JSON.stringify(createEndpointUpdated(endpointId, metadata)));
webRTCEndpoint.receiveMediaEvent(JSON.stringify(createEndpointUpdatedPeerMetadata(endpointId, metadata)));

// Then
const endpoint = webRTCEndpoint.getRemoteEndpoints()[endpointId]!;
expect(endpoint.metadata).toBe(undefined);
expect(endpoint.metadata.peer).toBe(undefined);
});

it('Update endpoint that not exist', () => {
Expand All @@ -81,7 +81,11 @@ it('Update endpoint that not exist', () => {
newField: 'new field value',
};

expect(() => webRTCEndpoint.receiveMediaEvent(JSON.stringify(createEndpointUpdated(notExistingEndpointId, metadata))))
expect(() =>
webRTCEndpoint.receiveMediaEvent(
JSON.stringify(createEndpointUpdatedPeerMetadata(notExistingEndpointId, metadata)),
),
)
// todo change this error in production code
.toThrow("Cannot set properties of undefined (setting 'metadata')");
});
Expand All @@ -106,7 +110,7 @@ it('Parse metadata on endpoint update', () => {
extraFluff: 'nah',
};

webRTCEndpoint.receiveMediaEvent(JSON.stringify(createEndpointUpdated(endpointId, metadata)));
webRTCEndpoint.receiveMediaEvent(JSON.stringify(createEndpointUpdatedPeerMetadata(endpointId, metadata)));

// Then
const endpoints = webRTCEndpoint.getRemoteEndpoints();
Expand Down Expand Up @@ -139,7 +143,7 @@ it('Correctly handle incorrect metadata on endpoint update', () => {
trash: 'metadata',
};

webRTCEndpoint.receiveMediaEvent(JSON.stringify(createEndpointUpdated(endpointId, metadata)));
webRTCEndpoint.receiveMediaEvent(JSON.stringify(createEndpointUpdatedPeerMetadata(endpointId, metadata)));

// Then
const endpoints = webRTCEndpoint.getRemoteEndpoints();
Expand Down
8 changes: 5 additions & 3 deletions packages/ts-client/tests/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export const createSimulcastTrack = (metadata: any = undefined): Track => ({
export const createEmptyEndpoint = (endpointId?: string): Endpoint =>
EndpointSchema.parse({
id: endpointId ?? '210fdb82-80d2-4868-8c31-a45f54f6e3c9',
metadata: null,
metadata: { peer: undefined, server: undefined },
trackIdToMetadata: {},
tracks: {},
type: 'webrtc',
Expand Down Expand Up @@ -332,11 +332,13 @@ a=rtcp-rsize\r
type: 'custom',
});

export const createEndpointUpdated = (endpointId: string, metadata: any): EndpointUpdatedWebrtcEvent =>
export const createEndpointUpdatedPeerMetadata = (endpointId: string, metadata: any): EndpointUpdatedWebrtcEvent =>
EndpointUpdatedWebrtcEventSchema.parse({
data: {
id: endpointId,
metadata: metadata,
metadata: {
peer: metadata,
},
},
type: 'endpointUpdated',
});
10 changes: 8 additions & 2 deletions packages/ts-client/tests/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,10 @@ export type CustomSdpAnswerDataEvent = z.infer<typeof CustomSdpAnswerDataEventSc
export const EndpointAddedWebrtcEventSchema = z.object({
data: z.object({
id: z.string().min(1),
metadata: z.any(), // undefined is possible, null is possible
metadata: z.object({
peer: z.any(),
server: z.any(),
}),
type: z.union([z.literal('webrtc'), z.literal('hls'), z.literal('rtsp')]),
}),
type: z.literal('endpointAdded'),
Expand All @@ -116,7 +119,10 @@ export type EndpointRemovedEvent = z.infer<typeof EndpointRemovedEventSchema>;
export const EndpointUpdatedWebrtcEventSchema = z.object({
data: z.object({
id: z.string().min(1),
metadata: z.any(), // undefined is possible
metadata: z.object({
peer: z.any(),
server: z.any(),
}),
}),
type: z.literal('endpointUpdated'),
});
Expand Down

0 comments on commit 901b989

Please sign in to comment.