Skip to content

converted arrays to objects #2239

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 22, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 14 additions & 17 deletions lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ function Client(server, conn){
this.id = conn.id;
this.request = conn.request;
this.setup();
this.sockets = [];
this.sockets = {};
this.nsps = {};
this.connectBuffer = [];
}
Expand Down Expand Up @@ -73,7 +73,7 @@ Client.prototype.connect = function(name){

var self = this;
var socket = nsp.add(this, function(){
self.sockets.push(socket);
self.sockets[socket.id] = socket;
self.nsps[nsp.name] = socket;

if ('/' == nsp.name && self.connectBuffer.length > 0) {
Expand All @@ -90,12 +90,10 @@ Client.prototype.connect = function(name){
*/

Client.prototype.disconnect = function(){
var socket;
// we don't use a for loop because the length of
// `sockets` changes upon each iteration
while (socket = this.sockets.shift()) {
socket.disconnect();
for (var id in this.sockets) {
this.sockets[id].disconnect();
}
this.sockets = {};
this.close();
};

Expand All @@ -106,10 +104,9 @@ Client.prototype.disconnect = function(){
*/

Client.prototype.remove = function(socket){
var i = this.sockets.indexOf(socket);
if (~i) {
var nsp = this.sockets[i].nsp.name;
this.sockets.splice(i, 1);
if (this.sockets[socket.id]) {
var nsp = this.sockets[socket.id].nsp.name;
delete this.sockets[socket.id];
delete this.nsps[nsp];
} else {
debug('ignoring remove for %s', socket.id);
Expand Down Expand Up @@ -206,9 +203,9 @@ Client.prototype.ondecoded = function(packet) {
*/

Client.prototype.onerror = function(err){
this.sockets.forEach(function(socket){
socket.onerror(err);
});
for (var id in this.sockets) {
this.sockets[id].onerror(err);
}
this.onclose('client error');
};

Expand All @@ -226,10 +223,10 @@ Client.prototype.onclose = function(reason){
this.destroy();

// `nsps` and `sockets` are cleaned up seamlessly
var socket;
while (socket = this.sockets.shift()) {
socket.onclose(reason);
for (var id in this.sockets) {
this.sockets[id].onclose(reason);
}
this.sockets = {};

this.decoder.destroy(); // clean up decoder
};
Expand Down
6 changes: 3 additions & 3 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -347,9 +347,9 @@ Server.prototype.of = function(name, fn){
*/

Server.prototype.close = function(){
this.nsps['/'].sockets.forEach(function(socket){
socket.onclose();
});
for (var id in this.nsps['/'].sockets) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're not checking for hasOwnProperty here. Extensions fo the natives will make socket.io break here. Test needed for that scenario as well.

this.nsps['/'].sockets[id].onclose();
}

this.engine.close();

Expand Down
9 changes: 4 additions & 5 deletions lib/namespace.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ var emit = Emitter.prototype.emit;
function Namespace(server, name){
this.name = name;
this.server = server;
this.sockets = [];
this.sockets = {};
this.connected = {};
this.fns = [];
this.ids = 0;
Expand Down Expand Up @@ -160,7 +160,7 @@ Namespace.prototype.add = function(client, fn){
if (err) return socket.error(err.data || err.message);

// track socket
self.sockets.push(socket);
self.sockets[socket.id] = socket;

// it's paramount that the internal `onconnect` logic
// fires before user-set events to prevent state order
Expand All @@ -187,9 +187,8 @@ Namespace.prototype.add = function(client, fn){
*/

Namespace.prototype.remove = function(socket){
var i = this.sockets.indexOf(socket);
if (~i) {
this.sockets.splice(i, 1);
if (this.sockets[socket.id]) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this.sockets.hasOwnProperty would be a better test.
See for inspiration: socketio/socket.io-adapter#30

delete this.sockets[socket.id];
} else {
debug('ignoring remove for %s', socket.id);
}
Expand Down
14 changes: 6 additions & 8 deletions lib/socket.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,10 @@ function Socket(nsp, client){
this.server = nsp.server;
this.adapter = this.nsp.adapter;
this.id = client.id;
this.id = nsp.name + '#' + client.id;
this.client = client;
this.conn = client.conn;
this.rooms = [];
this.rooms = {};
this.acks = {};
this.connected = true;
this.disconnected = false;
Expand Down Expand Up @@ -222,11 +223,11 @@ Socket.prototype.packet = function(packet, opts){
Socket.prototype.join = function(room, fn){
debug('joining room %s', room);
var self = this;
if (~this.rooms.indexOf(room)) return this;
if (this.rooms[room]) return this;
this.adapter.add(this.id, room, function(err){
if (err) return fn && fn(err);
debug('joined room %s', room);
self.rooms.push(room);
self.rooms[room] = room;
fn && fn(null);
});
return this;
Expand All @@ -247,10 +248,7 @@ Socket.prototype.leave = function(room, fn){
this.adapter.del(this.id, room, function(err){
if (err) return fn && fn(err);
debug('left room %s', room);
var idx = self.rooms.indexOf(room);
if (idx >= 0) {
self.rooms.splice(idx, 1);
}
delete self.rooms[room];
fn && fn(null);
});
return this;
Expand All @@ -264,7 +262,7 @@ Socket.prototype.leave = function(room, fn){

Socket.prototype.leaveAll = function(){
this.adapter.delAll(this.id);
this.rooms = [];
this.rooms = {};
};

/**
Expand Down
26 changes: 13 additions & 13 deletions test/socket.io.js
Original file line number Diff line number Diff line change
Expand Up @@ -368,12 +368,12 @@ describe('socket.io', function(){
var clientSocket = client(srv, { reconnection: false });

clientSocket.on('disconnect', function init() {
expect(sio.nsps['/'].sockets.length).to.equal(0);
expect(Object.keys(sio.nsps['/'].sockets).length).to.equal(0);
server.listen(PORT);
});

clientSocket.on('connect', function init() {
expect(sio.nsps['/'].sockets.length).to.equal(1);
expect(Object.keys(sio.nsps['/'].sockets).length).to.equal(1);
sio.close();
});

Expand All @@ -395,12 +395,12 @@ describe('socket.io', function(){
var clientSocket = ioc('ws://0.0.0.0:' + PORT);

clientSocket.on('disconnect', function init() {
expect(sio.nsps['/'].sockets.length).to.equal(0);
expect(Object.keys(sio.nsps['/'].sockets).length).to.equal(0);
server.listen(PORT);
});

clientSocket.on('connect', function init() {
expect(sio.nsps['/'].sockets.length).to.equal(1);
expect(Object.keys(sio.nsps['/'].sockets).length).to.equal(1);
sio.close();
});

Expand Down Expand Up @@ -1932,15 +1932,15 @@ describe('socket.io', function(){
var socket = client(srv);
sio.on('connection', function(s){
s.join('a', function(){
expect(s.rooms).to.eql([s.id, 'a']);
expect(Object.keys(s.rooms)).to.eql([s.id, 'a']);
s.join('b', function(){
expect(s.rooms).to.eql([s.id, 'a', 'b']);
expect(Object.keys(s.rooms)).to.eql([s.id, 'a', 'b']);
s.join( 'c', function(){
expect(s.rooms).to.eql([s.id, 'a', 'b', 'c']);
expect(Object.keys(s.rooms)).to.eql([s.id, 'a', 'b', 'c']);
s.leave('b', function(){
expect(s.rooms).to.eql([s.id, 'a', 'c']);
expect(Object.keys(s.rooms)).to.eql([s.id, 'a', 'c']);
s.leaveAll();
expect(s.rooms).to.eql([]);
expect(Object.keys(s.rooms)).to.eql([]);
done();
});
});
Expand Down Expand Up @@ -1976,13 +1976,13 @@ describe('socket.io', function(){
var socket = client(srv);
sio.on('connection', function(s){
s.join('a', function(){
expect(s.rooms).to.eql([s.id, 'a']);
expect(Object.keys(s.rooms)).to.eql([s.id, 'a']);
s.join('b', function(){
expect(s.rooms).to.eql([s.id, 'a', 'b']);
expect(Object.keys(s.rooms)).to.eql([s.id, 'a', 'b']);
s.leave('unknown', function(){
expect(s.rooms).to.eql([s.id, 'a', 'b']);
expect(Object.keys(s.rooms)).to.eql([s.id, 'a', 'b']);
s.leaveAll();
expect(s.rooms).to.eql([]);
expect(Object.keys(s.rooms)).to.eql([]);
done();
});
});
Expand Down