From aa2f3f47d8c0c040b5e8dc9e0b914868a2e0a796 Mon Sep 17 00:00:00 2001 From: Jerko Steiner Date: Tue, 22 Nov 2016 11:10:08 -0500 Subject: [PATCH] Fix a bug introduced by SocketIO not having consistent ids During the time of the initial release of Peer Calls, the server and client sockets used to have different namespaces - one side had `/#` prepended to the name, whereas the other did not, so I had to check for this in the code. This was fixed since the release of [SocketIO v1.5.0][1], thus breaking the compatibility with PeerCalls. Any new `npm install` would break this because of the way it works - it tries to install the latest available "compatible" release. As of this commit, the SocketIO version is locked to v1.6.0. [1]: https://github.com/socketio/socket.io/releases/tag/1.5.0 --- package.json | 4 +-- src/client/peer/__tests__/handshake-test.js | 31 ++++++++++----------- src/client/peer/__tests__/peers-test.js | 22 +++++++-------- src/client/peer/handshake.js | 2 +- src/client/peer/peers.js | 2 +- src/server/__tests__/socket-test.js | 22 +++++++-------- 6 files changed, 41 insertions(+), 42 deletions(-) diff --git a/package.json b/package.json index 8bb6995..38c3bc7 100644 --- a/package.json +++ b/package.json @@ -21,8 +21,8 @@ "react-addons-css-transition-group": "^0.14.8", "react-dom": "^0.14.8", "simple-peer": "^6.0.3", - "socket.io": "^1.3.7", - "socket.io-client": "^1.3.7", + "socket.io": "1.6.0", + "socket.io-client": "1.6.0", "underscore": "^1.8.3", "uuid": "^2.0.1" }, diff --git a/src/client/peer/__tests__/handshake-test.js b/src/client/peer/__tests__/handshake-test.js index 2ee7247..3dbd551 100644 --- a/src/client/peer/__tests__/handshake-test.js +++ b/src/client/peer/__tests__/handshake-test.js @@ -40,23 +40,22 @@ describe('handshake', () => { // given let payload = { users: [{ id: 'a'}, { id: 'b' }], - initiator: '/#a', + initiator: 'a', }; socket.emit('users', payload); - expect(peerInstances.length).toBe(2); + expect(peerInstances.length).toBe(1); // when payload = { users: [{ id: 'a'}, { id: 'c' }], - initiator: '/#c', + initiator: 'c', }; socket.emit('users', payload); // then - expect(peerInstances.length).toBe(3); - expect(peerInstances[0].destroy.mock.calls.length).toBe(0); - expect(peerInstances[1].destroy.mock.calls.length).toBe(1); - expect(peerInstances[2].destroy.mock.calls.length).toBe(0); + expect(peerInstances.length).toBe(2); + expect(peerInstances[0].destroy.mock.calls.length).toBe(1); + expect(peerInstances[1].destroy.mock.calls.length).toBe(0); }); }); @@ -67,14 +66,14 @@ describe('handshake', () => { data = {}; handshake.init(socket, 'bla'); socket.emit('users', { - initiator: '#/a', - users: [{ id: 'a' }] + initiator: 'a', + users: [{ id: 'a' }, { id: 'b' }] }); }); it('should forward signal to peer', () => { socket.emit('signal', { - userId: 'a', + userId: 'b', data }); @@ -84,7 +83,7 @@ describe('handshake', () => { it('does nothing if no peer', () => { socket.emit('signal', { - userId: 'b', + userId: 'a', data }); @@ -106,8 +105,8 @@ describe('handshake', () => { handshake.init(socket, 'bla'); socket.emit('users', { - users: [{ id: 'a' }], - initiator: '/#a' + initiator: 'a', + users: [{ id: 'a' }, { id: 'b'}] }); expect(peerInstances.length).toBe(1); peer = peerInstances[0]; @@ -130,7 +129,7 @@ describe('handshake', () => { let signal = { bla: 'bla' }; socket.once('signal', payload => { - expect(payload.userId).toEqual('a'); + expect(payload.userId).toEqual('b'); expect(payload.signal).toBe(signal); done(); }); @@ -151,7 +150,7 @@ describe('handshake', () => { expect(dispatcher.dispatch.mock.calls.length).toBe(1); expect(dispatcher.dispatch.mock.calls).toEqual([[{ type: 'add-stream', - userId: 'a', + userId: 'b', stream }]]); }); @@ -166,7 +165,7 @@ describe('handshake', () => { expect(dispatcher.dispatch.mock.calls.length).toBe(1); expect(dispatcher.dispatch.mock.calls).toEqual([[{ type: 'remove-stream', - userId: 'a' + userId: 'b' }]]); }); diff --git a/src/client/peer/__tests__/peers-test.js b/src/client/peer/__tests__/peers-test.js index a46d85f..247a87d 100644 --- a/src/client/peer/__tests__/peers-test.js +++ b/src/client/peer/__tests__/peers-test.js @@ -19,7 +19,7 @@ describe('peers', () => { dispatcher.dispatch.mockClear(); notify.warn.mockClear(); - user = { id: '/#user2' }; + user = { id: 'user2' }; socket = createSocket(); peerInstances = []; stream = { stream: true }; @@ -38,7 +38,7 @@ describe('peers', () => { describe('create', () => { it('creates a new peer', () => { - peers.create({ socket, user, initiator: '/#user2', stream }); + peers.create({ socket, user, initiator: 'user2', stream }); expect(notify.warn.mock.calls).toEqual([[ 'Connecting to peer...' ]]); @@ -49,7 +49,7 @@ describe('peers', () => { }); it('sets initiator correctly', () => { - peers.create({ socket, user, initiator: '/#user1', stream }); + peers.create({ socket, user, initiator: 'user1', stream }); expect(peerInstances.length).toBe(1); expect(Peer.init.mock.calls.length).toBe(1); @@ -58,8 +58,8 @@ describe('peers', () => { }); it('destroys old peer before creating new one', () => { - peers.create({ socket, user, initiator: '/#user2', stream }); - peers.create({ socket, user, initiator: '/#user2', stream }); + peers.create({ socket, user, initiator: 'user2', stream }); + peers.create({ socket, user, initiator: 'user2', stream }); expect(peerInstances.length).toBe(2); expect(Peer.init.mock.calls.length).toBe(2); @@ -74,7 +74,7 @@ describe('peers', () => { let peer; beforeEach(() => { - peers.create({ socket, user, initiator: '/#user1', stream }); + peers.create({ socket, user, initiator: 'user1', stream }); notify.warn.mockClear(); peer = peerInstances[0]; }); @@ -115,13 +115,13 @@ describe('peers', () => { it('returns ids of all peers', () => { peers.create({ - socket, user: {id: '/#user2' }, initiator: '/#user2', stream + socket, user: {id: 'user2' }, initiator: 'user2', stream }); peers.create({ - socket, user: {id: '/#user3' }, initiator: '/#user3', stream + socket, user: {id: 'user3' }, initiator: 'user3', stream }); - expect(peers.getIds()).toEqual([ '/#user2', '/#user3' ]); + expect(peers.getIds()).toEqual([ 'user2', 'user3' ]); }); }); @@ -146,10 +146,10 @@ describe('peers', () => { it('destroys all peers and removes them', () => { peers.create({ - socket, user: {id: '/#user2' }, initiator: '/#user2', stream + socket, user: {id: 'user2' }, initiator: 'user2', stream }); peers.create({ - socket, user: {id: '/#user3' }, initiator: '/#user3', stream + socket, user: {id: 'user3' }, initiator: 'user3', stream }); peers.clear(); diff --git a/src/client/peer/handshake.js b/src/client/peer/handshake.js index 5a370a5..343d08d 100644 --- a/src/client/peer/handshake.js +++ b/src/client/peer/handshake.js @@ -25,7 +25,7 @@ function init(socket, roomName, stream) { notify.info('Connected users: {0}', users.length); users - .filter(user => !peers.get(user.id) && user.id !== '/#' + socket.id) + .filter(user => !peers.get(user.id) && user.id !== socket.id) .forEach(user => createPeer(user, initiator)); let newUsersMap = _.indexBy(users, 'id'); diff --git a/src/client/peer/peers.js b/src/client/peer/peers.js index 227bca5..073e9ad 100644 --- a/src/client/peer/peers.js +++ b/src/client/peer/peers.js @@ -23,7 +23,7 @@ function create({ socket, user, initiator, stream }) { } let peer = peers[user.id] = Peer.init({ - initiator: '/#' + socket.id === initiator, + initiator: socket.id === initiator, stream, config: { iceServers: [{ diff --git a/src/server/__tests__/socket-test.js b/src/server/__tests__/socket-test.js index 58d0cf6..9445eb3 100644 --- a/src/server/__tests__/socket-test.js +++ b/src/server/__tests__/socket-test.js @@ -11,7 +11,7 @@ describe('socket', () => { let socket, io, rooms; beforeEach(() => { socket = new EventEmitter(); - socket.id = '/#socket0'; + socket.id = 'socket0'; socket.join = jest.genMockFunction(); socket.leave = jest.genMockFunction(); rooms = {}; @@ -27,16 +27,16 @@ describe('socket', () => { adapter: { rooms: { room1: { - '/#socket0': true + 'socket0': true }, room2: { - '/#socket0': true + 'socket0': true }, room3: { sockets: { - '/#socket0': true, - '/#socket1': true, - '/#socket2': true + 'socket0': true, + 'socket1': true, + 'socket2': true } } } @@ -65,7 +65,7 @@ describe('socket', () => { expect(io.to.mock.calls).toEqual([[ 'a' ]]); expect(io.to('a').emit.mock.calls).toEqual([[ 'signal', { - userId: '/#socket0', + userId: 'socket0', signal } ]]); @@ -94,13 +94,13 @@ describe('socket', () => { expect(io.to.mock.calls).toEqual([[ 'room3' ]]); expect(io.to('room3').emit.mock.calls).toEqual([[ 'users', { - initiator: '/#socket0', + initiator: 'socket0', users: [{ - id: '/#socket0', + id: 'socket0', }, { - id: '/#socket1', + id: 'socket1', }, { - id: '/#socket2' + id: 'socket2' }] } ]]);