Refactor out HTTP layer from WebSocket + add specs

- HTTP content-type possibly fixed (livereload.js file)
- WebSocket layer should now be easier to replace
- missing socket/http/streaming specs added

lib/guard/livereload/websocket.rb

@@ -1,74 +1,34 @@
 require 'eventmachine'
 require 'em-websocket'
-require 'http/parser'
-require 'uri'
+require 'guard/livereload/websocket/dispatcher'
 
 module Guard
   class LiveReload
     class WebSocket < EventMachine::WebSocket::Connection
-      HTTP_DATA_FORBIDDEN = "HTTP/1.1 403 Forbidden\r\nContent-Type: text/plain\r\nContent-Length: 13\r\n\r\n403 Forbidden"
-      HTTP_DATA_NOT_FOUND = "HTTP/1.1 404 Not Found\r\nContent-Type: text/plain\r\nContent-Length: 13\r\n\r\n404 Not Found"
-
       def initialize(options)
-        @livereload_js_path = options[:livereload_js_path]
+        @dispatcher = Dispatcher.new(options)
         super
       end
 
       def dispatch(data)
-        parser = Http::Parser.new
-        parser << data
-        # prepend with '.' to make request url usable as a file path
-        request_path = '.' + URI.parse(parser.request_url).path
-        request_path += '/index.html' if File.directory? request_path
-        if parser.http_method != 'GET' || parser.upgrade?
-          super # pass the request to websocket
-        else
-          _serve(request_path)
-        end
-      end
-
-      private
-
-      def _serve_file(path)
-        UI.debug "Serving file #{path}"
-
-        data = [
-          'HTTP/1.1 200 OK',
-          'Content-Type: %s',
-          'Content-Length: %s',
-          '',
-          '']
-        data = format(data * "\r\n", _content_type(path), File.size(path))
-        send_data(data)
-        stream_file_data(path).callback { close_connection_after_writing }
-      end
-
-      def _content_type(path)
-        case File.extname(path).downcase
-        when '.html', '.htm' then 'text/html'
-        when '.css' then 'text/css'
-        when '.js' then 'application/ecmascript'
-        when '.gif' then 'image/gif'
-        when '.jpeg', '.jpg' then 'image/jpeg'
-        when '.png' then 'image/png'
-        else; 'text/plain'
+        responses = @dispatcher.dispatch(data)
+
+        responses.each do |type, payload|
+          case type
+          when :default
+            super
+          when :data
+            send_data(payload)
+          when :close_write
+            close_connection_after_writing
+          when :file
+            path = payload
+            stream_file_data(path).callback { close_connection_after_writing }
+          else
+            fail "Unknown response type: #{type.inspect}"
+          end
         end
       end
-
-      def _livereload_js_path
-        @livereload_js_path
-      end
-
-      def _serve(path)
-        return _serve_file(_livereload_js_path) if path == './livereload.js'
-        data = _readable_file(path) ? HTTP_DATA_FORBIDDEN : HTTP_DATA_NOT_FOUND
-        send_data(data)
-        close_connection_after_writing
-      end
-
-      def _readable_file(path)
-        File.readable?(path) && !File.directory?(path)
-      end
     end
   end
 end

lib/guard/livereload/websocket/dispatcher.rb

@@ -0,0 +1,85 @@
+require 'guard/ui'
+require 'eventmachine'
+require 'em-websocket'
+require 'http/parser'
+require 'uri'
+
+module Guard
+  class LiveReload
+    class WebSocket < EventMachine::WebSocket::Connection
+      class Dispatcher
+        class Http
+          def self.build(header, message)
+            [
+              header,
+              'Content-Type: text/plain',
+              "Content-Length: #{message.size}",
+              '',
+              message
+            ].join("\r\n").freeze
+          end
+
+          FORBIDDEN = build('HTTP/1.1 403 Forbidden', '403 Forbidden')
+          NOT_FOUND = build('HTTP/1.1 404 Not Found', '404 Not Found')
+        end
+
+        def initialize(options)
+          @livereload_js_path = options[:livereload_js_path]
+        end
+
+        def dispatch(data)
+          parser = ::Http::Parser.new
+          parser << data
+          # prepend with '.' to make request url usable as a file path
+          request_path = '.' + URI.parse(parser.request_url).path
+          request_path += '/index.html' if File.directory? request_path
+          if parser.http_method != 'GET' || parser.upgrade?
+            [[:default, nil]]
+          else
+            _serve(request_path)
+          end
+        end
+
+        private
+
+        def _content_type(path)
+          case File.extname(path).downcase
+          when '.html', '.htm' then 'text/html'
+          when '.css' then 'text/css'
+          when '.js' then 'application/ecmascript'
+          when '.gif' then 'image/gif'
+          when '.jpeg', '.jpg' then 'image/jpeg'
+          when '.png' then 'image/png'
+          else; 'text/plain'
+          end
+        end
+
+        def _livereload_js_path
+          @livereload_js_path
+        end
+
+        def _serve(path)
+          if path == './livereload.js'
+            content_type = _content_type(path)
+            real_path = _livereload_js_path
+            data = _file_data_header(real_path, content_type)
+            return [[:data, data], [:file, real_path]]
+          end
+
+          data = _readable_file(path) ? Http::FORBIDDEN : Http::NOT_FOUND
+          [[:data, data], [:close_write, nil]]
+        end
+
+        def _readable_file(path)
+          File.readable?(path) && !File.directory?(path)
+        end
+
+        def _file_data_header(path, content_type)
+          UI.debug "Serving file #{path}"
+          data = ['HTTP/1.1 200 OK', 'Content-Type: %s', 'Content-Length: %s', '', '']
+          format(data * "\r\n", content_type, File.size(path))
+        end
+      end
+    end
+  end
+end

spec/lib/guard/livereload/websocket/dispatcher_spec.rb

@@ -0,0 +1,64 @@
+RSpec.describe Guard::LiveReload::WebSocket::Dispatcher do
+  let(:options) { { livereload_js_path: '/tmp/foo.js.123' } }
+  subject { described_class.new(options) }
+
+  def http_request(type, path)
+    [
+      "#{type} #{path} HTTP/1.1",
+      'Host: 127.0.0.1:35729',
+      'Connection: keep-alive',
+      'Accept: */*',
+      'User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2490.80 Safari/537.36',
+      'Referer: http://localhost:4000/foo.html',
+      'Accept-Encoding: gzip, deflate, sdch',
+      'Accept-Language: en-US,en;q=0.8,pl;q=0.6',
+      '',
+      ''
+    ].join("\r\n")
+  end
+
+  describe '#dispatch' do
+    context 'with a request for livereload.js' do
+      let(:data) { http_request('GET', '/livereload.js?ext=Chrome&extver=2.1.0') }
+
+      before do
+        allow(File).to receive(:size).with('/tmp/foo.js.123').and_return(123)
+      end
+
+      it 'sends livereload files' do
+        expected = "HTTP/1.1 200 OK\r\nContent-Type: application/ecmascript\r\nContent-Length: 123\r\n\r\n"
+        expect(subject.dispatch(data)).to eq([[:data, expected], [:file, '/tmp/foo.js.123']])
+      end
+    end
+
+    context 'with a non-GET request' do
+      let(:data) { http_request('DELETE', '/livereload.js?ext=Chrome&extver=2.1.0') }
+
+      it 'lets the socket process the request' do
+        expect(subject.dispatch(data)).to eq([[:default, nil]])
+      end
+    end
+
+    context 'with a request for a non-existing file' do
+      let(:data) { http_request('GET', '/nosuchfile.js?ext=Chrome&extver=2.1.0') }
+
+      it 'responds with a 404' do
+        expected = "HTTP/1.1 404 Not Found\r\nContent-Type: text/plain\r\nContent-Length: 13\r\n\r\n404 Not Found"
+        expect(subject.dispatch(data)).to eq([[:data, expected], [:close_write, nil]])
+      end
+    end
+
+    context 'with a request for file outside the project' do
+      let(:data) { http_request('GET', '/./../Rakefile?ext=Chrome&extver=2.1.0') }
+
+      before do
+        allow(File).to receive(:readable?).with('././../Rakefile').and_return(true)
+      end
+
+      it 'responds with a 403' do
+        expected = "HTTP/1.1 403 Forbidden\r\nContent-Type: text/plain\r\nContent-Length: 13\r\n\r\n403 Forbidden"
+        expect(subject.dispatch(data)).to eq([[:data, expected], [:close_write, nil]])
+      end
+    end
+  end
+end

spec/lib/guard/livereload/websocket_spec.rb

@@ -0,0 +1,103 @@
+RSpec.describe Guard::LiveReload::WebSocket do
+  let(:options) { { livereload_js_path: 'example_livereload.js' } }
+  let(:signature) { 123 }
+  subject { described_class.new(signature, options) }
+
+  let(:dispatcher) { instance_double(described_class::Dispatcher) }
+
+  before do
+    allow(described_class::Dispatcher).to receive(:new).with(options).and_return(dispatcher)
+  end
+
+  describe '#initialize' do
+    context 'with options' do
+      let(:options) { {} }
+      it 'passes options to dispatcher' do
+        expect(described_class::Dispatcher).to receive(:new).with(options)
+        subject
+      end
+    end
+  end
+
+  describe '#receive_data' do
+    let(:data) { 'foo' }
+
+    before do
+      allow(dispatcher).to receive(:dispatch).and_return(response)
+      allow(subject).to receive(:close_connection_after_writing)
+      allow(subject).to receive(:close_connection)
+      allow(subject).to receive(:send_data)
+    end
+
+    context 'with a request for allowed file' do
+      let(:response) { [[:data, 'foobar'], [:file, './foo.js']] }
+      let(:callback) { double(:callback) }
+
+      before do
+        allow(subject).to receive(:stream_file_data).and_return(callback)
+        allow(callback).to receive(:callback) { |&block| block.call }
+      end
+
+      it 'send the HTTP content info' do
+        expect(subject).to receive(:send_data).with('foobar')
+        subject.receive_data(data)
+      end
+
+      it 'streams the file' do
+        expect(subject).to receive(:stream_file_data).with('./foo.js').and_return(callback)
+        subject.receive_data(data)
+      end
+
+      it 'closes the stream' do
+        expect(subject).to receive(:close_connection_after_writing)
+        subject.receive_data(data)
+      end
+    end
+
+    context 'with a data response' do
+      let(:response) { [[:data, 'hello'], [:close_write, nil]] }
+
+      it 'responds with the data' do
+        expect(subject).to receive(:send_data).with('hello')
+        subject.receive_data(data)
+      end
+
+      it 'closes the stream' do
+        expect(subject).to receive(:close_connection_after_writing)
+        subject.receive_data(data)
+      end
+    end
+
+    context 'with partial data' do
+      let(:response) { [[:data, 'hello']] }
+
+      it 'responds with the data' do
+        expect(subject).to receive(:send_data).with('hello')
+        subject.receive_data(data)
+      end
+
+      it 'does not close the stream' do
+        expect(subject).to_not receive(:close_connection_after_writing)
+        subject.receive_data(data)
+      end
+    end
+
+    context 'with unhandled response' do
+      let(:response) { [[:default, nil]] }
+
+      it 'lets the socket process the request' do
+        subject.receive_data(data)
+      end
+
+      it 'does not send data' do
+        expect(subject).to_not receive(:send_data)
+        subject.receive_data(data)
+      end
+
+      it 'does not close the stream' do
+        expect(subject).to_not receive(:close_connection_after_writing)
+        subject.receive_data(data)
+      end
+    end
+  end
+end