Trim query part from URL where it's necessary. Fixes thoov#274 (thoov#275)

Author: pablobirukov

src/network-bridge.js

@@ -1,5 +1,10 @@
 import { reject } from './helpers/array-helpers';
 
+function trimQueryPartFromURL(url) {
+  const queryIndex = url.indexOf('?');
+  return queryIndex >= 0 ? url.slice(0, queryIndex) : url;
+}
+
 /*
  * The network bridge is a way for the mock websocket object to 'communicate' with
  * all available servers. This is a singleton object so it is important that you
@@ -18,8 +23,7 @@ class NetworkBridge {
    * @param {string} url
    */
   attachWebSocket(websocket, url) {
-    const queryIndex = url.indexOf('?');
-    const serverURL = queryIndex >= 0 ? url.slice(0, queryIndex) : url;
+    const serverURL = trimQueryPartFromURL(url);
     const connectionLookup = this.urlMap[serverURL];
 
     if (connectionLookup && connectionLookup.server && connectionLookup.websockets.indexOf(websocket) === -1) {
@@ -32,7 +36,7 @@ class NetworkBridge {
    * Attaches a websocket to a room
    */
   addMembershipToRoom(websocket, room) {
-    const connectionLookup = this.urlMap[websocket.url];
+    const connectionLookup = this.urlMap[trimQueryPartFromURL(websocket.url)];
 
     if (connectionLookup && connectionLookup.server && connectionLookup.websockets.indexOf(websocket) !== -1) {
       if (!connectionLookup.roomMemberships[room]) {
@@ -70,7 +74,8 @@ class NetworkBridge {
    * @param {string} url - the url to use to find which server is running on it
    */
   serverLookup(url) {
-    const connectionLookup = this.urlMap[url];
+    const serverURL = trimQueryPartFromURL(url);
+    const connectionLookup = this.urlMap[serverURL];
 
     if (connectionLookup) {
       return connectionLookup.server;
@@ -85,8 +90,9 @@ class NetworkBridge {
    * @param {class} broadcaster - socket that is broadcasting and is to be excluded from the lookup
    */
   websocketsLookup(url, room, broadcaster) {
+    const serverURL = trimQueryPartFromURL(url);
     let websockets;
-    const connectionLookup = this.urlMap[url];
+    const connectionLookup = this.urlMap[serverURL];
 
     websockets = connectionLookup ? connectionLookup.websockets : [];
 
@@ -104,7 +110,7 @@ class NetworkBridge {
    * @param {string} url
    */
   removeServer(url) {
-    delete this.urlMap[url];
+    delete this.urlMap[trimQueryPartFromURL(url)];
   }
 
   /*
@@ -114,7 +120,8 @@ class NetworkBridge {
    * @param {string} url
    */
   removeWebSocket(websocket, url) {
-    const connectionLookup = this.urlMap[url];
+    const serverURL = trimQueryPartFromURL(url);
+    const connectionLookup = this.urlMap[serverURL];
 
     if (connectionLookup) {
       connectionLookup.websockets = reject(connectionLookup.websockets, socket => socket === websocket);
@@ -125,7 +132,7 @@ class NetworkBridge {
    * Removes a websocket from a room
    */
   removeMembershipFromRoom(websocket, room) {
-    const connectionLookup = this.urlMap[websocket.url];
+    const connectionLookup = this.urlMap[trimQueryPartFromURL(websocket.url)];
     const memberships = connectionLookup.roomMemberships[room];
 
     if (connectionLookup && memberships !== null) {

tests/functional/websockets.test.js

@@ -114,6 +114,23 @@ test.cb('that the server gets called when the client sends a message', t => {
   };
 });
 
+test.cb('that the server gets called when the client sends a message using URL with query parameters', t => {
+  const testServer = new Server('ws://localhost:8080');
+
+  testServer.on('connection', socket => {
+    socket.on('message', data => {
+      t.is(data, 'Testing', 'on message fires as expected');
+      t.end();
+    });
+  });
+
+  const mockSocket = new WebSocket('ws://localhost:8080?foo=bar');
+
+  mockSocket.onopen = function open() {
+    this.send('Testing');
+  };
+});
+
 test.cb('that the onopen function will only be called once for each client', t => {
   const socketUrl = 'ws://localhost:8080';
   const mockServer = new Server(socketUrl);

tests/unit/network-bridge.test.js

@@ -58,6 +58,17 @@ test('that attachWebSocket will add a websocket to the url map', t => {
   t.is(connection.websockets.length, 1, 'websocket property contains only the websocket object');
 });
 
+test('that attachWebSocket will add a websocket with query params to the url map', t => {
+  const resultServer = networkBridge.attachServer(fakeObject, 'ws://localhost:8080');
+  const resultWebSocket = networkBridge.attachWebSocket(fakeObject, 'ws://localhost:8080?foo=bar');
+  const connection = networkBridge.urlMap['ws://localhost:8080'];
+
+  t.deepEqual(resultServer, fakeObject, 'server returned because it was successfully added to the url map');
+  t.deepEqual(resultWebSocket, fakeObject, 'server returned as the websocket was successfully added to the map');
+  t.deepEqual(connection.websockets[0], fakeObject, 'fakeObject was added to the websockets array');
+  t.is(connection.websockets.length, 1, 'websocket property contains only the websocket object');
+});
+
 test('that attachWebSocket will add the same websocket only once', t => {
   const resultServer = networkBridge.attachServer(fakeObject, 'ws://localhost:8080');
   const resultWebSocket = networkBridge.attachWebSocket(fakeObject, 'ws://localhost:8080');
@@ -83,6 +94,18 @@ test('that server and websocket lookups return the correct objects', t => {
   t.deepEqual(websocketLookup.length, 1, 'the correct number of websockets are returned');
 });
 
+test('that server and websocket lookups ignore query params', t => {
+  networkBridge.attachServer(fakeObject, 'ws://localhost:8080');
+  networkBridge.attachWebSocket(fakeObject, 'ws://localhost:8080?foo=bar');
+
+  const serverLookup = networkBridge.serverLookup('ws://localhost:8080?foo1=1');
+  const websocketLookup = networkBridge.websocketsLookup('ws://localhost:8080?foo2=2');
+
+  t.deepEqual(serverLookup, fakeObject, 'server correctly returned');
+  t.deepEqual(websocketLookup, [fakeObject], 'websockets correctly returned');
+  t.deepEqual(websocketLookup.length, 1, 'the correct number of websockets are returned');
+});
+
 test('that removing server and websockets works correctly', t => {
   networkBridge.attachServer(fakeObject, 'ws://localhost:8080');
   networkBridge.attachWebSocket(fakeObject, 'ws://localhost:8080');
@@ -99,6 +122,22 @@ test('that removing server and websockets works correctly', t => {
   t.deepEqual(networkBridge.urlMap, {}, 'Url map is back in its default state');
 });
 
+test('that removing server and websockets works correctly with query params', t => {
+  networkBridge.attachServer(fakeObject, 'ws://localhost:8080');
+  networkBridge.attachWebSocket(fakeObject, 'ws://localhost:8080?foo=bar');
+
+  let websocketLookup = networkBridge.websocketsLookup('ws://localhost:8080?anything=else');
+  t.deepEqual(websocketLookup.length, 1, 'the correct number of websockets are returned');
+
+  networkBridge.removeWebSocket(fakeObject, 'ws://localhost:8080?arbitraryParameter');
+
+  websocketLookup = networkBridge.websocketsLookup('ws://localhost:8080?one=more');
+  t.deepEqual(websocketLookup.length, 0, 'the correct number of websockets are returned');
+
+  networkBridge.removeServer('ws://localhost:8080?please');
+  t.deepEqual(networkBridge.urlMap, {}, 'Url map is back in its default state');
+});
+
 test('a socket can join and leave a room', t => {
   const fakeSocket = { url: 'ws://roomy' };
 
@@ -120,3 +159,25 @@ test('a socket can join and leave a room', t => {
   inRoom = networkBridge.websocketsLookup('ws://roomy', 'room');
   t.is(inRoom.length, 0, 'there are no sockets in the room after leaving');
 });
+
+test('a socket with query params can join and leave a room', t => {
+  const fakeSocket = { url: 'ws://roomy?foo=bar' };
+
+  networkBridge.attachServer(fakeObject, 'ws://roomy');
+  networkBridge.attachWebSocket(fakeSocket, 'ws://roomy');
+
+  let inRoom;
+  inRoom = networkBridge.websocketsLookup('ws://roomy', 'room');
+  t.is(inRoom.length, 0, 'there are no sockets in the room to start with');
+
+  networkBridge.addMembershipToRoom(fakeSocket, 'room');
+
+  inRoom = networkBridge.websocketsLookup('ws://roomy', 'room');
+  t.is(inRoom.length, 1, 'there is 1 socket in the room after joining');
+  t.deepEqual(inRoom[0], fakeSocket);
+
+  networkBridge.removeMembershipFromRoom(fakeSocket, 'room');
+
+  inRoom = networkBridge.websocketsLookup('ws://roomy', 'room');
+  t.is(inRoom.length, 0, 'there are no sockets in the room after leaving');
+});