From e934015d87fea7d83cd8641e120bb8eec767570d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Mon, 18 May 2020 18:33:04 +0200 Subject: [PATCH] Refactoring searchClientById searchClientById was scanning through all open sockets to find the right one (which is inefficient if we have many). Instead, I created a new Map that directly maps ids to sockets. Furthermore, this solves a long-standing issue (when a socket is disconnected, we cannot find it anymore in the sockets list but it is still available in the disconnect callback from the map) As a result, we should not have any remaining circles any more. --- back/src/Controller/IoSocketController.ts | 108 +++++++--------------- back/src/Model/World.ts | 1 - 2 files changed, 33 insertions(+), 76 deletions(-) diff --git a/back/src/Controller/IoSocketController.ts b/back/src/Controller/IoSocketController.ts index 2c798797..fed04c78 100644 --- a/back/src/Controller/IoSocketController.ts +++ b/back/src/Controller/IoSocketController.ts @@ -30,6 +30,7 @@ enum SockerIoEvent { export class IoSocketController { Io: socketIO.Server; Worlds: Map = new Map(); + sockets: Map = new Map(); constructor(server: http.Server) { this.Io = socketIO(server); @@ -60,10 +61,7 @@ export class IoSocketController { // Let's get the room of the group. To do this, let's get anyone in the group and find its room. // Note: this is suboptimal let userId = group.getUsers()[0].id; - let client: ExSocketInterface|null = this.searchClientById(userId); - if (client === null) { - return; - } + let client: ExSocketInterface = this.searchClientByIdOrFail(userId); let roomId = client.roomId; this.Io.in(roomId).emit(SockerIoEvent.GROUP_CREATE_UPDATE, { position: group.getPosition(), @@ -73,19 +71,15 @@ export class IoSocketController { private sendDeleteGroupEvent(uuid: string, lastUser: UserInterface): void { // Let's get the room of the group. To do this, let's get anyone in the group and find its room. - // Note: this is suboptimal let userId = lastUser.id; - let client: ExSocketInterface|null = this.searchClientById(userId); - if (client === null) { - console.warn('Could not find client ', userId, ' in group') - return; - } + let client: ExSocketInterface = this.searchClientByIdOrFail(userId); let roomId = client.roomId; this.Io.in(roomId).emit(SockerIoEvent.GROUP_DELETE, uuid); } ioConnection() { this.Io.on(SockerIoEvent.CONNECTION, (socket: Socket) => { + this.sockets.set(socket.id, socket as ExSocketInterface); /*join-rom event permit to join one room. message : userId : user identification @@ -148,20 +142,19 @@ export class IoSocketController { socket.on(SockerIoEvent.WEBRTC_SIGNAL, (data: any) => { //send only at user - let client = this.searchClientById(data.receiverId); - if (!client) { - console.error("While exchanging a WebRTC signal: client doesn't exist for ", data.receiverId); + let client = this.sockets.get(data.receiverId); + if (client === undefined) { + console.warn("While exchanging a WebRTC signal: client with id ", data.receiverId, " does not exist. This might be a race condition."); return; } return client.emit(SockerIoEvent.WEBRTC_SIGNAL, data); }); socket.on(SockerIoEvent.WEBRTC_OFFER, (data: any) => { - //send only at user - let client = this.searchClientById(data.receiverId); - if (!client) { - console.error("client doesn't exist for ", data.receiverId); + let client = this.sockets.get(data.receiverId); + if (client === undefined) { + console.warn("While exchanging a WebRTC offer: client with id ", data.receiverId, " does not exist. This might be a race condition."); return; } client.emit(SockerIoEvent.WEBRTC_OFFER, data); @@ -170,16 +163,16 @@ export class IoSocketController { socket.on(SockerIoEvent.DISCONNECT, () => { try { let Client = (socket as ExSocketInterface); - this.sendDisconnectedEvent(Client); + //this.sendDisconnectedEvent(Client); - //refresh position of all user in all rooms in real time + //refresh position of all user in all rooms in real time (to remove the disconnected user) this.refreshUserPosition(Client); //leave room this.leaveRoom(Client); //leave webrtc room - socket.leave(Client.webRtcRoomId); + //socket.leave(Client.webRtcRoomId); //delete all socket information delete Client.webRtcRoomId; @@ -190,6 +183,7 @@ export class IoSocketController { console.error('An error occurred on "disconnect"'); console.error(e); } + this.sockets.delete(socket.id); }); // Let's send the user id to the user @@ -202,54 +196,12 @@ export class IoSocketController { }); } - /** - * TODO: each call to this method is suboptimal. It means that instead of passing an ID, we should pass a client object. - * @param userId - */ - searchClientById(userId: string): ExSocketInterface | null { - let clients: Array = Object.values(this.Io.sockets.sockets); - for (let i = 0; i < clients.length; i++) { - let client: ExSocketInterface = clients[i]; - if (client.id !== userId) { - continue; - } - return client; + searchClientByIdOrFail(userId: string): ExSocketInterface { + let client: ExSocketInterface|undefined = this.sockets.get(userId); + if (client === undefined) { + throw new Error("Could not find user with id " + userId); } - console.log("Could not find user with id ", userId); - //throw new Error("Could not find user with id " + userId); - return null; - } - - /** - * @param userId - */ - searchClientByToken(userId: string): ExSocketInterface | null { - let clients: Array = Object.values(this.Io.sockets.sockets); - for (let i = 0; i < clients.length; i++) { - let client: ExSocketInterface = clients[i]; - if (client.id !== userId) { - continue - } - return client; - } - return null; - } - - /** - * - * @param Client: ExSocketInterface - */ - sendDisconnectedEvent(Client: ExSocketInterface) { - Client.broadcast.emit(SockerIoEvent.WEBRTC_DISCONNECT, { - userId: Client.id - }); - - //disconnect webrtc room - if(!Client.webRtcRoomId){ - return; - } - Client.leave(Client.webRtcRoomId); - delete Client.webRtcRoomId; + return client; } leaveRoom(Client : ExSocketInterface){ @@ -349,12 +301,11 @@ export class IoSocketController { rooms.refreshUserPosition(rooms, this.Io); // update position in the world - let messageUserPosition = new MessageUserPosition(Client.id, Client.name, Client.character, Client.position); let world = this.Worlds.get(Client.roomId); if (!world) { return; } - world.updatePosition(Client, messageUserPosition.position); + world.updatePosition(Client, Client.position); this.Worlds.set(Client.roomId, world); } @@ -412,19 +363,26 @@ export class IoSocketController { //connected user connectedUser(userId: string, group: Group) { - let Client = this.searchClientById(userId); - if (!Client) { + /*let Client = this.sockets.get(userId); + if (Client === undefined) { return; - } + }*/ + let Client = this.searchClientByIdOrFail(userId); this.joinWebRtcRoom(Client, group.getId()); } //disconnect user disConnectedUser(userId: string, group: Group) { - let Client = this.searchClientById(userId); - if (!Client) { + let Client = this.searchClientByIdOrFail(userId); + Client.to(group.getId()).emit(SockerIoEvent.WEBRTC_DISCONNECT, { + userId: userId + }); + + //disconnect webrtc room + if(!Client.webRtcRoomId){ return; } - this.sendDisconnectedEvent(Client) + Client.leave(Client.webRtcRoomId); + delete Client.webRtcRoomId; } } diff --git a/back/src/Model/World.ts b/back/src/Model/World.ts index 39233271..fb33f02c 100644 --- a/back/src/Model/World.ts +++ b/back/src/Model/World.ts @@ -60,7 +60,6 @@ export class World { public leave(user : Identificable){ let userObj = this.users.get(user.id); if (userObj === undefined) { - // FIXME: this seems always wrong. I guess user.id is different from userPosition.userId console.warn('User ', user.id, 'does not belong to world! It should!'); } if (userObj !== undefined && typeof userObj.group !== 'undefined') {