From a8f3757d535aea413ea2fc2a8a6e1472cb539d5d Mon Sep 17 00:00:00 2001 From: Jerko Steiner Date: Sat, 16 Nov 2019 23:49:12 -0300 Subject: [PATCH] Do not join call automatically Present a user with a menu to join call manually --- .eslintrc.yaml | 2 + src/client/__mocks__/window.ts | 18 +++---- src/client/actions/CallActions.test.ts | 11 +--- src/client/actions/CallActions.ts | 35 ++++++------- src/client/components/App.tsx | 2 + src/client/components/Media.test.tsx | 23 +++++--- src/client/components/Media.tsx | 49 ++++++++++++------ src/client/reducers/media.test.ts | 72 +++++++++++++++++++++++--- src/client/reducers/peers.ts | 15 +++++- src/client/reducers/streams.ts | 33 +++++++++--- src/client/window.test.ts | 53 +------------------ src/client/window.ts | 17 ------ 12 files changed, 185 insertions(+), 145 deletions(-) diff --git a/.eslintrc.yaml b/.eslintrc.yaml index 3bb2c82..44b6fa8 100644 --- a/.eslintrc.yaml +++ b/.eslintrc.yaml @@ -55,6 +55,8 @@ overrides: - files: - '*.test.ts' - '*.test.tsx' + - '**/__mocks__/*.ts' + - '**/__mocks__/*.tsx' rules: '@typescript-eslint/no-explicit-any': off - files: diff --git a/src/client/__mocks__/window.ts b/src/client/__mocks__/window.ts index d303193..4eddf1c 100644 --- a/src/client/__mocks__/window.ts +++ b/src/client/__mocks__/window.ts @@ -14,18 +14,16 @@ export class MediaStream { }] } } -export function getUserMedia () { - return !getUserMedia.shouldFail - ? Promise.resolve(getUserMedia.stream) - : Promise.reject(new Error('test')) -} -getUserMedia.shouldFail = false -getUserMedia.fail = (shouldFail: boolean) => - getUserMedia.shouldFail = shouldFail -getUserMedia.stream = new MediaStream() - export const navigator = window.navigator +;(window as any).navigator.mediaDevices = {} +window.navigator.mediaDevices.enumerateDevices = async () => { + return [] +} +window.navigator.mediaDevices.getUserMedia = async () => { + return {} as any +} + export const play = jest.fn() export const valueOf = jest.fn() diff --git a/src/client/actions/CallActions.test.ts b/src/client/actions/CallActions.test.ts index 8d03a08..75174af 100644 --- a/src/client/actions/CallActions.test.ts +++ b/src/client/actions/CallActions.test.ts @@ -6,7 +6,7 @@ import * as CallActions from './CallActions' import * as SocketActions from './SocketActions' import * as constants from '../constants' import socket from '../socket' -import { callId, getUserMedia } from '../window' +import { callId } from '../window' import { bindActionCreators, createStore, AnyAction, combineReducers, applyMiddleware } from 'redux' import reducers from '../reducers' import { middlewares } from '../middlewares' @@ -33,7 +33,6 @@ describe('CallActions', () => { applyMiddleware(...middlewares), ) callActions = bindActionCreators(CallActions, store.dispatch); - (getUserMedia as any).fail(false); (SocketActions.handshake as jest.Mock).mockReturnValue(jest.fn()) }) @@ -55,19 +54,12 @@ describe('CallActions', () => { message: 'Connected to server socket', type: 'warning', }, - }, { - type: constants.STREAM_ADD, - payload: { - stream: jasmine.anything(), - userId: constants.ME, - }, }, { type: constants.INIT, }]) expect((SocketActions.handshake as jest.Mock).mock.calls).toEqual([[{ socket, roomName: callId, - stream: (getUserMedia as any).stream, }]]) }) @@ -94,7 +86,6 @@ describe('CallActions', () => { }) it('dispatches alert when failed to get media stream', async () => { - (getUserMedia as any).fail(true) const promise = callActions.init() socket.emit('connect', undefined) await promise diff --git a/src/client/actions/CallActions.ts b/src/client/actions/CallActions.ts index 8b8ba61..84fc732 100644 --- a/src/client/actions/CallActions.ts +++ b/src/client/actions/CallActions.ts @@ -1,11 +1,9 @@ -import * as constants from '../constants' import socket from '../socket' import { Dispatch, ThunkResult } from '../store' -import { callId, getUserMedia } from '../window' +import { callId } from '../window' import { ClientSocket } from '../socket' import * as NotifyActions from './NotifyActions' import * as SocketActions from './SocketActions' -import * as StreamActions from './StreamActions' export interface InitAction { type: 'INIT' @@ -23,12 +21,12 @@ const initialize = (): InitializeAction => ({ export const init = (): ThunkResult> => async (dispatch, getState) => { const socket = await dispatch(connect()) - const stream = await dispatch(getCameraStream()) + // const stream = await dispatch(getCameraStream()) dispatch(SocketActions.handshake({ socket, roomName: callId, - stream, + // stream, })) dispatch(initialize()) @@ -48,16 +46,17 @@ export const connect = () => (dispatch: Dispatch) => { }) } -export const getCameraStream = () => async (dispatch: Dispatch) => { - try { - const stream = await getUserMedia({ - video: { facingMode: 'user' }, - audio: true, - }) - dispatch(StreamActions.addStream({ stream, userId: constants.ME })) - return stream - } catch (err) { - dispatch(NotifyActions.alert('Could not get access to microphone & camera')) - return - } -} +// export const getCameraStream = () => async (dispatch: Dispatch) => { +// try { +// const stream = await getUserMedia({ +// video: { facingMode: 'user' }, +// audio: true, +// }) +// dispatch(StreamActions.addStream({ stream, userId: constants.ME })) +// return stream +// } catch (err) { +// dispatch( +// NotifyActions.alert('Could not get access to microphone & camera')) +// return +// } +// } diff --git a/src/client/components/App.tsx b/src/client/components/App.tsx index 372ff0f..97ce7b3 100644 --- a/src/client/components/App.tsx +++ b/src/client/components/App.tsx @@ -11,6 +11,7 @@ import Chat from './Chat' import Notifications from './Notifications' import Toolbar from './Toolbar' import Video from './Video' +import { Media } from './Media' export interface AppProps { active: string | null @@ -84,6 +85,7 @@ export default class App extends React.PureComponent { /> + { - const onSave = jest.fn() beforeEach(() => { - jest.resetAllMocks() store = createStore() store.dispatch({ type: MEDIA_ENUMERATE, @@ -34,7 +34,7 @@ describe('Media', () => { ReactDOM.render(
resolve(div!)}> - +
, div, @@ -44,18 +44,27 @@ describe('Media', () => { } describe('submit', () => { - it('calls onSave', async () => { + const stream = {} as MediaStream + let promise: Promise + beforeEach(() => { + navigator.mediaDevices.getUserMedia = async () => { + promise = Promise.resolve(stream) + return promise + } + }) + it('tries to retrieve audio/video media stream', async () => { const node = await render() expect(node.tagName).toBe('FORM') TestUtils.Simulate.submit(node) - expect(onSave.mock.calls.length).toBe(1) + expect(promise).toBeDefined() + await promise }) }) describe('onVideoChange', () => { it('calls onSetVideoConstraint', async () => { const node = await render() - const select = node.querySelector('select.media-video')! + const select = node.querySelector('select[name=video-input]')! TestUtils.Simulate.change(select, { target: { value: '{"deviceId":123}', @@ -68,7 +77,7 @@ describe('Media', () => { describe('onAudioChange', () => { it('calls onSetAudioConstraint', async () => { const node = await render() - const select = node.querySelector('select.media-audio')! + const select = node.querySelector('select[name="audio-input"]')! TestUtils.Simulate.change(select, { target: { value: '{"deviceId":456}', diff --git a/src/client/components/Media.tsx b/src/client/components/Media.tsx index 1967745..6bc6792 100644 --- a/src/client/components/Media.tsx +++ b/src/client/components/Media.tsx @@ -1,19 +1,14 @@ import React from 'react' import { connect } from 'react-redux' -import { AudioConstraint, MediaDevice, setAudioConstraint, setVideoConstraint, VideoConstraint } from '../actions/MediaActions' +import { AudioConstraint, MediaDevice, setAudioConstraint, setVideoConstraint, VideoConstraint, getMediaStream, enumerateDevices } from '../actions/MediaActions' import { MediaState } from '../reducers/media' import { State } from '../store' export type MediaProps = MediaState & { + enumerateDevices: typeof enumerateDevices onSetVideoConstraint: typeof setVideoConstraint onSetAudioConstraint: typeof setAudioConstraint - onSave: () => void -} - -function getId(constraint: VideoConstraint | AudioConstraint) { - return typeof constraint === 'object' && 'deviceId' in constraint - ? constraint.deviceId - : '' + getMediaStream: typeof getMediaStream } function mapStateToProps(state: State) { @@ -23,17 +18,27 @@ function mapStateToProps(state: State) { } const mapDispatchToProps = { + enumerateDevices, onSetVideoConstraint: setVideoConstraint, onSetAudioConstraint: setAudioConstraint, + getMediaStream, } const c = connect(mapStateToProps, mapDispatchToProps) export const Media = c(React.memo(function Media(props: MediaProps) { - function onSave(event: React.FormEvent) { + React.useMemo(async () => await props.enumerateDevices(), []) + + async function onSave(event: React.FormEvent) { event.preventDefault() - props.onSave() + const { audio, video } = props + try { + await props.getMediaStream({ audio, video }) + } catch (err) { + console.error(err.stack) + // TODO display a message + } } function onVideoChange(event: React.ChangeEvent) { @@ -46,12 +51,17 @@ export const Media = c(React.memo(function Media(props: MediaProps) { props.onSetAudioConstraint(constraint) } - const videoId = getId(props.video) - const audioId = getId(props.audio) + const videoId = JSON.stringify(props.video) + const audioId = JSON.stringify(props.audio) return (
- - ) @@ -91,7 +106,9 @@ function Options(props: OptionsProps) { .map(device => , ) } diff --git a/src/client/reducers/media.test.ts b/src/client/reducers/media.test.ts index b2c4cfe..105280d 100644 --- a/src/client/reducers/media.test.ts +++ b/src/client/reducers/media.test.ts @@ -1,6 +1,7 @@ import * as MediaActions from '../actions/MediaActions' -import { MEDIA_ENUMERATE, MEDIA_VIDEO_CONSTRAINT_SET, MEDIA_AUDIO_CONSTRAINT_SET, MEDIA_STREAM } from '../constants' +import { MEDIA_ENUMERATE, MEDIA_VIDEO_CONSTRAINT_SET, MEDIA_AUDIO_CONSTRAINT_SET, MEDIA_STREAM, ME, PEERS_DESTROY, PEER_ADD } from '../constants' import { createStore, Store } from '../store' +import SimplePeer from 'simple-peer' describe('media', () => { @@ -95,14 +96,73 @@ describe('media', () => { navigator.mediaDevices.getUserMedia = async () => stream }) - it('returns a promise with media stream', async () => { - const promise = MediaActions.getMediaStream({ + async function dispatch() { + const promise = store.dispatch(MediaActions.getMediaStream({ audio: true, video: true, - }) - expect(promise.type).toBe('MEDIA_STREAM') - expect(promise.status).toBe('pending') + })) expect(await promise).toBe(stream) + } + + describe('reducers/streams', () => { + it('adds the local stream to the map of videos', async () => { + expect(store.getState().streams[ME]).toBeFalsy() + await dispatch() + expect(store.getState().streams[ME]).toBeTruthy() + expect(store.getState().streams[ME].stream).toBe(stream) + }) + }) + + describe('reducers/peers', () => { + const peer1 = new SimplePeer() + peer1.addStream = jest.fn() + peer1.removeStream = jest.fn() + const peer2 = new SimplePeer() + peer2.addStream = jest.fn() + peer2.removeStream = jest.fn() + const peers = [peer1, peer2] + + beforeEach(() => { + store.dispatch({ + type: PEERS_DESTROY, + }) + store.dispatch({ + type: PEER_ADD, + payload: { + userId: '1', + peer: peer1, + }, + }) + store.dispatch({ + type: PEER_ADD, + payload: { + userId: '2', + peer: peer2, + }, + }) + }) + + afterEach(() => { + store.dispatch({ + type: PEERS_DESTROY, + }) + }) + + it('replaces local stream on all peers', async () => { + await dispatch() + peers.forEach(peer => { + expect((peer.addStream as jest.Mock).mock.calls) + .toEqual([[ stream ]]) + expect((peer.removeStream as jest.Mock).mock.calls).toEqual([]) + }) + await dispatch() + peers.forEach(peer => { + expect((peer.addStream as jest.Mock).mock.calls) + .toEqual([[ stream ], [ stream ]]) + expect((peer.removeStream as jest.Mock).mock.calls) + .toEqual([[ stream ]]) + }) + }) }) }); diff --git a/src/client/reducers/peers.ts b/src/client/reducers/peers.ts index 82c91eb..84cbacf 100644 --- a/src/client/reducers/peers.ts +++ b/src/client/reducers/peers.ts @@ -3,14 +3,17 @@ import omit from 'lodash/omit' import Peer from 'simple-peer' import { PeerAction } from '../actions/PeerActions' import * as constants from '../constants' +import { MediaStreamAction } from '../actions/MediaActions' export type PeersState = Record const defaultState: PeersState = {} +let localStream: MediaStream | undefined + export default function peers( state = defaultState, - action: PeerAction, + action: PeerAction | MediaStreamAction, ): PeersState { switch (action.type) { case constants.PEER_ADD: @@ -21,8 +24,18 @@ export default function peers( case constants.PEER_REMOVE: return omit(state, [action.payload.userId]) case constants.PEERS_DESTROY: + localStream = undefined forEach(state, peer => peer.destroy()) return defaultState + case constants.MEDIA_STREAM: + if (action.status === 'resolved') { + forEach(state, peer => { + localStream && peer.removeStream(localStream) + peer.addStream(action.payload) + }) + localStream = action.payload + } + return state default: return state } diff --git a/src/client/reducers/streams.ts b/src/client/reducers/streams.ts index e48a670..e6decdd 100644 --- a/src/client/reducers/streams.ts +++ b/src/client/reducers/streams.ts @@ -1,8 +1,9 @@ import _debug from 'debug' import omit from 'lodash/omit' import { AddStreamAction, AddStreamPayload, RemoveStreamAction, StreamAction } from '../actions/StreamActions' -import { STREAM_ADD, STREAM_REMOVE } from '../constants' +import { STREAM_ADD, STREAM_REMOVE, MEDIA_STREAM, ME } from '../constants' import { createObjectURL, revokeObjectURL } from '../window' +import { MediaStreamAction } from '../actions/MediaActions' const debug = _debug('peercalls') const defaultState = Object.freeze({}) @@ -21,9 +22,9 @@ export interface StreamsState { } function addStream ( - state: StreamsState, action: AddStreamAction, + state: StreamsState, payload: AddStreamAction['payload'], ): StreamsState { - const { userId, stream } = action.payload + const { userId, stream } = payload const userStream: AddStreamPayload = { userId, @@ -38,9 +39,9 @@ function addStream ( } function removeStream ( - state: StreamsState, action: RemoveStreamAction, + state: StreamsState, payload: RemoveStreamAction['payload'], ): StreamsState { - const { userId } = action.payload + const { userId } = payload const stream = state[userId] if (stream && stream.url) { revokeObjectURL(stream.url) @@ -48,15 +49,31 @@ function removeStream ( return omit(state, [userId]) } +function replaceStream(state: StreamsState, stream: MediaStream): StreamsState { + state = removeStream(state, { + userId: ME, + }) + return addStream(state, { + userId: ME, + stream, + }) +} + export default function streams( state = defaultState, - action: StreamAction, + action: StreamAction | MediaStreamAction, ): StreamsState { switch (action.type) { case STREAM_ADD: - return addStream(state, action) + return addStream(state, action.payload) case STREAM_REMOVE: - return removeStream(state, action) + return removeStream(state, action.payload) + case MEDIA_STREAM: + if (action.status === 'resolved') { + return replaceStream(state, action.payload) + } else { + return state + } default: return state } diff --git a/src/client/window.test.ts b/src/client/window.test.ts index daf6513..1351ace 100644 --- a/src/client/window.test.ts +++ b/src/client/window.test.ts @@ -1,58 +1,7 @@ -import { - createObjectURL, - revokeObjectURL, - getUserMedia, - navigator, - play, - valueOf, -} from './window' +import { createObjectURL, play, revokeObjectURL, valueOf } from './window' describe('window', () => { - describe('getUserMedia', () => { - - class MediaStream {} - const stream = new MediaStream() - const constraints = { video: true } - - afterEach(() => { - delete (navigator as any).mediaDevices - delete navigator.getUserMedia - delete (navigator as any).webkitGetUserMedia - }) - - it('calls navigator.mediaDevices.getUserMedia', async () => { - const promise = Promise.resolve(stream); - (navigator as any).mediaDevices = { - getUserMedia: jest.fn().mockReturnValue(promise), - } - expect(await getUserMedia(constraints)).toBe(stream) - }) - - ;['getUserMedia', 'webkitGetUserMedia'].forEach((method) => { - it(`it calls navigator.${method} as a fallback`, () => { - (navigator as any)[method] = jest.fn() - .mockImplementation( - (constraints, onSuccess, onError) => onSuccess(stream), - ) - return getUserMedia(constraints) - .then(s => expect(s).toBe(stream)) - }) - }) - - it('throws error when no supported method', async () => { - let error: Error - try { - await getUserMedia(constraints) - } catch (err) { - error = err - } - expect(error!).toBeTruthy() - expect(error!.message).toBe('Browser unsupported') - }) - - }) - describe('play', () => { let v1: HTMLVideoElement & { play: jest.Mock } diff --git a/src/client/window.ts b/src/client/window.ts index 97a4245..7f5a9a9 100644 --- a/src/client/window.ts +++ b/src/client/window.ts @@ -2,27 +2,10 @@ import _debug from 'debug' const debug = _debug('peercalls') -export async function getUserMedia (constraints: MediaStreamConstraints) { - if (navigator.mediaDevices && navigator.mediaDevices.getUserMedia) { - return navigator.mediaDevices.getUserMedia(constraints) - } - - return new Promise((resolve, reject) => { - const getMedia = navigator.getUserMedia || - (navigator as unknown as { - webkitGetUserMedia: typeof navigator.getUserMedia - }).webkitGetUserMedia - if (!getMedia) reject(new Error('Browser unsupported')) - getMedia.call(navigator, constraints, resolve, reject) - }) -} - export const createObjectURL = (object: unknown) => window.URL.createObjectURL(object) export const revokeObjectURL = (url: string) => window.URL.revokeObjectURL(url) -export const navigator = window.navigator - export function play () { const videos = window.document.querySelectorAll('video') Array.prototype.forEach.call(videos, (video, index) => {