Merge pull request #38 from hypothesis/refactor-streamer

Refactor the streamer into a stand alone class

h/static/scripts/app.js

@@ -28,8 +28,6 @@ if (settings.raven) {
   angular.module('ngRaven', []);
 }
 
-var streamer = require('./streamer');
-
 // Fetch external state that the app needs before it can run. This includes the
 // authenticated user state, the API endpoint URLs and WebSocket connection.
 var resolve = {
@@ -37,7 +35,6 @@ var resolve = {
   sessionState: function (session) {
     return session.load();
   },
-  streamer: streamer.connect,
 };
 
 // @ngInject
@@ -91,7 +88,7 @@ function configureHttp($httpProvider, jwtInterceptorProvider) {
 }
 
 // @ngInject
-function setupHttp($http) {
+function setupHttp($http, streamer) {
   $http.defaults.headers.common['X-Client-Id'] = streamer.clientId;
 }
 
@@ -175,6 +172,7 @@ module.exports = angular.module('h', [
   .service('rootThread', require('./root-thread'))
   .service('searchFilter', require('./search-filter'))
   .service('session', require('./session'))
+  .service('streamer', require('./streamer'))
   .service('streamFilter', require('./stream-filter'))
   .service('tags', require('./tags'))
   .service('unicode', require('./unicode'))

h/static/scripts/streamer.js

@@ -4,13 +4,6 @@ var uuid = require('node-uuid');
 
 var Socket = require('./websocket');
 
-// the randomly generated session UUID
-var clientId = uuid.v4();
-
-// the singleton socket instance, only one may
-// be open at a time
-var socket;
-
 /**
  * Open a new WebSocket connection to the Hypothesis push notification service.
  * Only one websocket connection may exist at a time, any existing socket is
@@ -23,37 +16,19 @@ var socket;
  * @param groups - The local groups store
  * @param session - Provides access to read and update the session state
  * @param settings - Application settings
- *
- * @return The push notification service client.
  */
 // @ngInject
-function connect($rootScope, annotationMapper, groups, session, settings) {
-  // The public interface of the streamer. Returned from connect.
-  var controls = {
-    setConfig: setConfig
-  };
+function Streamer($rootScope, annotationMapper, groups, session, settings) {
+  // The randomly generated session UUID
+  var clientId = uuid.v4();
+
+  // The socket instance for this Streamer instance
+  var socket;
+
   // Client configuration messages, to be sent each time a new connection is
   // established.
   var configMessages = {};
 
-  // Close any existing socket
-  if (socket) {
-    socket.close();
-  }
-
-  // If we have no URL configured, don't do anything.
-  var url = settings.websocketUrl;
-  if (!url) {
-    return controls;
-  }
-
-  // Open the socket
-  socket = new Socket(url);
-  setConfig('client-id', {
-    messageType: 'client_id',
-    value: clientId
-  });
-
   function handleAnnotationNotification(message) {
     var action = message.options.action;
     var annotations = message.payload;
@@ -108,52 +83,73 @@ function connect($rootScope, annotationMapper, groups, session, settings) {
     }
   }
 
-  socket.on('open', function () {
-    sendClientConfig();
-  });
-
-  socket.on('error', function (event) {
-    console.warn('Error connecting to H push notification service:', event);
-
-    // In development, warn if the connection failure might be due to
-    // the app's origin not having been whitelisted in the H service's config.
-    //
-    // Unfortunately the error event does not provide a way to get at the
-    // HTTP status code for HTTP -> WS upgrade requests.
-    var websocketHost = new URL(url).hostname;
-    if (['localhost', '127.0.0.1'].indexOf(websocketHost) !== -1) {
-      console.warn('Check that your H service is configured to allow ' +
-                   'WebSocket connections from ' + window.location.origin);
+  var connect = function () {
+    // If we have no URL configured, don't do anything.
+    var url = settings.websocketUrl;
+    if (!url) {
+      return;
+    }
+
+    // Open a new socket
+    if (socket) {
+      socket.close();
     }
-  });
-
-  socket.on('message', function (event) {
-    // Wrap message dispatches in $rootScope.$apply() so that
-    // scope watches on app state affected by the received message
-    // are updated
-    //
-    // Note: The use of $apply() here will no longer be needed once session
-    // state is moved to the Redux store in `annotationUI`.
-    $rootScope.$apply(function () {
-      var message = JSON.parse(event.data);
-      if (!message) {
-        return;
-      }
 
-      if (message.type === 'annotation-notification') {
-        handleAnnotationNotification(message);
-      } else if (message.type === 'session-change') {
-        handleSessionChangeNotification(message);
-      } else {
-        console.warn('received unsupported notification', message.type);
+    socket = new Socket(url);
+    setConfig('client-id', {
+      messageType: 'client_id',
+      value: clientId,
+    });
+
+    socket.on('open', function () {
+      sendClientConfig();
+    });
+
+    socket.on('error', function (event) {
+      console.warn('Error connecting to H push notification service:', event);
+
+      // In development, warn if the connection failure might be due to
+      // the app's origin not having been whitelisted in the H service's config.
+      //
+      // Unfortunately the error event does not provide a way to get at the
+      // HTTP status code for HTTP -> WS upgrade requests.
+      var websocketHost = new URL(url).hostname;
+      if (['localhost', '127.0.0.1'].indexOf(websocketHost) !== -1) {
+        console.warn('Check that your H service is configured to allow ' +
+                     'WebSocket connections from ' + window.location.origin);
       }
     });
-  });
 
-  return controls;
+    socket.on('message', function (event) {
+      // Wrap message dispatches in $rootScope.$apply() so that
+      // scope watches on app state affected by the received message
+      // are updated
+      //
+      // Note: The use of $apply() here will no longer be needed once session
+      // state is moved to the Redux store in `annotationUI`.
+      $rootScope.$apply(function () {
+        var message = JSON.parse(event.data);
+        if (!message) {
+          return;
+        }
+
+        if (message.type === 'annotation-notification') {
+          handleAnnotationNotification(message);
+        } else if (message.type === 'session-change') {
+          handleSessionChangeNotification(message);
+        } else {
+          console.warn('received unsupported notification', message.type);
+        }
+      });
+    });
+  };
+
+  connect();
+
+  this.connect = connect;
+  this.clientId = clientId;
+  this.setConfig = setConfig;
+  this.socket = socket;
 }
 
-module.exports = {
-  connect: connect,
-  clientId: clientId
-};
+module.exports = Streamer;

h/static/scripts/test/streamer-test.js

@@ -36,10 +36,11 @@ describe('streamer', function () {
   var fakeRootScope;
   var fakeSession;
   var fakeSettings;
-  var streamer;
+  var activeStreamer;
+  var Streamer;
 
   function createDefaultStreamer() {
-    streamer.connect(
+    activeStreamer = new Streamer(
       fakeRootScope,
       fakeAnnotationMapper,
       fakeGroups,
@@ -74,7 +75,7 @@ describe('streamer', function () {
       websocketUrl: 'ws://example.com/ws',
     };
 
-    streamer = proxyquire('../streamer', {
+    Streamer = proxyquire('../streamer', {
       './websocket': FakeSocket,
     });
   });
@@ -89,19 +90,13 @@ describe('streamer', function () {
     createDefaultStreamer();
     assert.equal(fakeWebSocket.messages.length, 1);
     assert.equal(fakeWebSocket.messages[0].messageType, 'client_id');
-    assert.equal(fakeWebSocket.messages[0].value, streamer.clientId);
+    assert.equal(fakeWebSocket.messages[0].value, activeStreamer.clientId);
   });
 
   it('should close any existing socket', function () {
     createDefaultStreamer();
     var oldWebSocket = fakeWebSocket;
-    streamer.connect(
-      fakeRootScope,
-      fakeAnnotationMapper,
-      fakeGroups,
-      fakeSession,
-      fakeSettings
-    );
+    activeStreamer.connect();
     assert.ok(oldWebSocket.didClose);
     assert.ok(!fakeWebSocket.didClose);
   });