Add test and fix some bugs

Signed-off-by: Yuta Iwama <ganmacs@gmail.com>

lib/fluent/plugin_helper/http_server/app.rb

@@ -50,6 +50,8 @@ def call(request)
               connect(request)
             when HttpServer::Methods::TRACE
               trace(request)
+            else
+              raise "Unknown method #{method}"
             end
           Protocol::HTTP::Response[*resp]
         rescue => e

lib/fluent/plugin_helper/http_server/compat/server.rb

@@ -50,6 +50,7 @@ def start(notify = nil)
 
           def stop
             @server.shutdown
+            @server.stop
           end
 
           HttpServer::Methods::ALL.map { |e| e.downcase.to_sym }.each do |name|

lib/fluent/plugin_helper/http_server/compat/webrick_handler.rb

@@ -23,8 +23,8 @@ module HttpServer
       module Compat
         class WebrickHandler
           # **opt is enough. but I wrote a signature explicitly for readability
-          def self.build(get: nil, head: nil, post: nil, put: nil, patch: nil, delete: nil, connect: nil)
-            opt = { get: get, head: head, post: post, put: put, patch: patch, delete: delete, connect: connect }
+          def self.build(get: nil, head: nil, post: nil, put: nil, patch: nil, delete: nil, connect: nil, options: nil, trace: nil)
+            opt = { get: get, head: head, post: post, put: put, patch: patch, delete: delete, connect: connect, options: options, trace: trace }
 
             Class.new(WEBrick::HTTPServlet::AbstractServlet) do
               HttpServer::Methods::ALL.each do |name|

lib/fluent/plugin_helper/http_server/request.rb

@@ -24,16 +24,17 @@ class Request
         attr_reader :path, :query_string
 
         def initialize(request)
+          @request = request
           path = request.path
           @path, @query_string = path.split('?', 2)
         end
 
         def query
-          CGI.parse(@query_string)
+          @query_string && CGI.parse(@query_string)
         end
 
         def body
-          request.body
+          @request.body && @request.body.read
         end
       end
     end

test/plugin_helper/http_server/test_app.rb

@@ -0,0 +1,67 @@
+require_relative '../../helper'
+require 'flexmock/test_unit'
+require 'net/http'
+require 'uri'
+
+begin
+  require 'fluent/plugin_helper/http_server/app'
+  skip = false
+rescue LoadError => _
+  skip = true
+end
+
+unless skip
+  class HtttpHelperAppTest < Test::Unit::TestCase
+    NULL_LOGGER = Logger.new(nil)
+
+    class DummyRounter
+      def initialize(table = {})
+        @table = table
+      end
+
+      def route!(method, path, _req)
+        r = @table.fetch(method).fetch(path)
+        [200, {}, r]
+      end
+    end
+
+    sub_test_case '#call' do
+      data(
+        'GET request' => 'GET',
+        'POST request' => 'POST',
+        'DELETE request' => 'DELETE',
+        'PUT request' => 'PUT',
+        'PATCH request' => 'PATCH',
+        'OPTION request' => 'OPTIONS',
+        'CONNECT request' => 'CONNECT',
+        'TRACE request' => 'TRACE',
+      )
+      test 'dispatch correct path' do |method|
+        r = DummyRounter.new(method.downcase.to_sym => { 'path/' => 'hi' })
+        app = Fluent::PluginHelper::HttpServer::App.new(r, NULL_LOGGER)
+        m = flexmock('request', method: method, path: 'path/')
+        r = app.call(m)
+        assert_equal(r.body.read, 'hi')
+        assert_equal(r.status, 200)
+      end
+
+      test 'HEAD: dispatch correct path' do |method|
+        r = DummyRounter.new(:head => { 'path/' => 'hi' })
+        app = Fluent::PluginHelper::HttpServer::App.new(r, NULL_LOGGER)
+        m = flexmock('request', method: method, path: 'path')
+        r = app.call(m)
+        assert_equal(r.body.read, '')
+        assert_equal(r.status, 200)
+      end
+
+      test 'if path does not end with `/`' do |method|
+        r = DummyRounter.new(:head => { 'path/' => 'hi' })
+        app = Fluent::PluginHelper::HttpServer::App.new(r, NULL_LOGGER)
+        m = flexmock('request', method: method, path: 'path')
+        r = app.call(m)
+        assert_equal(r.body.read, '')
+        assert_equal(r.status, 200)
+      end
+    end
+  end
+end

test/plugin_helper/http_server/test_route.rb

@@ -0,0 +1,34 @@
+require_relative '../../helper'
+require 'flexmock/test_unit'
+require 'net/http'
+require 'uri'
+
+begin
+  require 'fluent/plugin_helper/http_server/router'
+  skip = false
+rescue LoadError => _
+  skip = true
+end
+
+unless skip
+  class HtttpHelperRouterTest < Test::Unit::TestCase
+    sub_test_case '#mount' do
+      test 'mount with method and path' do
+        router = Fluent::PluginHelper::HttpServer::Router.new
+        router.mount(:get, 'path/', ->(req) { req })
+        assert_equal(router.route!(:get, 'path/', 'request'), 'request')
+      end
+
+      test 'use default app if not found' do
+        router = Fluent::PluginHelper::HttpServer::Router.new
+        req = flexmock('request', path: 'path/')
+        assert_equal(router.route!(:get, 'path/', req), [404, { 'Content-Type' => 'text/plain' }, "404 Not Found: #{req.path}\n"])
+      end
+
+      test 'default app is configurable' do
+        router = Fluent::PluginHelper::HttpServer::Router.new(->(req) { req })
+        assert_equal(router.route!(:get, 'path/', 'hello'), 'hello')
+      end
+    end
+  end
+end

test/plugin_helper/test_http_server_helper.rb

@@ -0,0 +1,186 @@
+require_relative '../helper'
+require 'fluent/plugin_helper/http_server'
+require 'fluent/plugin/output'
+require 'fluent/event'
+require 'net/http'
+require 'uri'
+
+class HtttpHelperTest < Test::Unit::TestCase
+  PORT = unused_port
+  NULL_LOGGER = Logger.new(nil)
+
+  class Dummy < Fluent::Plugin::TestBase
+    helpers :http_server
+  end
+
+  def on_driver
+    Fluent::Test.setup
+    driver = Dummy.new
+    driver.start
+    driver.after_start
+
+    yield(driver)
+  ensure
+    unless driver.stopped?
+      driver.stop rescue nil
+    end
+
+    unless driver.before_shutdown?
+      driver.before_shutdown rescue nil
+    end
+
+    unless driver.shutdown?
+      driver.shutdown rescue nil
+    end
+
+    unless driver.after_shutdown?
+      driver.after_shutdown rescue nil
+    end
+
+    unless driver.closed?
+      driver.close rescue nil
+    end
+
+    unless driver.terminated?
+      driver.terminated rescue nil
+    end
+  end
+
+  %w[get head].each do |n|
+    define_method(n) do |uri, header = {}|
+      url = URI.parse(uri)
+      req = Net::HTTP.const_get(n.capitalize).new(url, header)
+      Net::HTTP.start(url.host, url.port) do |http|
+        http.request(req)
+      end
+    end
+  end
+
+  %w[post put patch delete options trace].each do |n|
+    define_method(n) do |uri, body = '', header = {}|
+      url = URI.parse(uri)
+      req = Net::HTTP.const_get(n.capitalize).new(url, header)
+      req.body = body
+      Net::HTTP.start(url.host, url.port) do |http|
+        http.request(req)
+      end
+    end
+  end
+
+  sub_test_case 'Create a HTTP server' do
+    test 'monunt given path' do
+      on_driver do |driver|
+        driver.create_http_server(addr: '127.0.0.1', port: PORT, logger: NULL_LOGGER) do |s|
+          s.get('/example/hello') { [200, { 'Content-Type' => 'text/plain' }, 'hello get'] }
+          s.post('/example/hello') { [200, { 'Content-Type' => 'text/plain' }, 'hello post'] }
+          s.head('/example/hello') { [200, { 'Content-Type' => 'text/plain' }, 'hello head'] }
+          s.put('/example/hello') { [200, { 'Content-Type' => 'text/plain' }, 'hello put'] }
+          s.patch('/example/hello') { [200, { 'Content-Type' => 'text/plain' }, 'hello patch'] }
+          s.delete('/example/hello') { [200, { 'Content-Type' => 'text/plain' }, 'hello delete'] }
+          s.trace('/example/hello') { [200, { 'Content-Type' => 'text/plain' }, 'hello trace'] }
+          s.options('/example/hello') { [200, { 'Content-Type' => 'text/plain' }, 'hello options'] }
+        end
+
+        resp = head("http://127.0.0.1:#{PORT}/example/hello")
+        assert_equal('200', resp.code)
+        assert_equal(nil, resp.body)
+        assert_equal('text/plain', resp['Content-Type'])
+
+        %w[get put post put delete options trace].each do |n|
+          resp = send(n, "http://127.0.0.1:#{PORT}/example/hello")
+          assert_equal('200', resp.code)
+          assert_equal("hello #{n}", resp.body)
+          assert_equal('text/plain', resp['Content-Type'])
+        end
+      end
+    end
+
+    test 'when path does not start with `/` or ends with `/`' do
+      on_driver do |driver|
+        driver.create_http_server(addr: '127.0.0.1', port: PORT, logger: NULL_LOGGER) do |s|
+          s.get('example/hello') { [200, { 'Content-Type' => 'text/plain' }, 'hello get'] }
+          s.get('/example/hello2/') { [200, { 'Content-Type' => 'text/plain' }, 'hello get'] }
+        end
+
+        resp = get("http://127.0.0.1:#{PORT}/example/hello")
+        assert_equal('404', resp.code)
+
+        resp = get("http://127.0.0.1:#{PORT}/example/hello2")
+        assert_equal('200', resp.code)
+      end
+    end
+
+    test 'when path is not found' do
+      on_driver do |driver|
+        driver.create_http_server(addr: '127.0.0.1', port: PORT, logger: NULL_LOGGER) do |s|
+          s.get('/example/hello') { [200, { 'Content-Type' => 'text/plain' }, 'hello get'] }
+        end
+
+        resp = get("http://127.0.0.1:#{PORT}/example/hello/not_found")
+        assert_equal('404', resp.code)
+      end
+    end
+
+    test 'params and body' do
+      on_driver do |driver|
+        driver.create_http_server(addr: '127.0.0.1', port: PORT, logger: NULL_LOGGER) do |s|
+          s.get('/example/hello') do |req|
+            assert_equal(req.query_string, nil)
+            assert_equal(req.body, nil)
+            [200, { 'Content-Type' => 'text/plain' }, 'hello get']
+          end
+
+          s.post('/example/hello') do |req|
+            assert_equal(req.query_string, nil)
+            assert_equal(req.body, 'this is body')
+            [200, { 'Content-Type' => 'text/plain' }, 'hello post']
+          end
+
+          s.get('/example/hello/params') do |req|
+            assert_equal(req.query_string, 'test=true')
+            assert_equal(req.body, nil)
+            [200, { 'Content-Type' => 'text/plain' }, 'hello get']
+          end
+
+          s.post('/example/hello/params') do |req|
+            assert_equal(req.query_string, 'test=true')
+            assert_equal(req.body, 'this is body')
+            [200, { 'Content-Type' => 'text/plain' }, 'hello post']
+          end
+        end
+
+        resp = get("http://127.0.0.1:#{PORT}/example/hello")
+        assert_equal('200', resp.code)
+
+        resp = post("http://127.0.0.1:#{PORT}/example/hello", 'this is body')
+        assert_equal('200', resp.code)
+
+        resp = get("http://127.0.0.1:#{PORT}/example/hello/params?test=true")
+        assert_equal('200', resp.code)
+
+        resp = post("http://127.0.0.1:#{PORT}/example/hello/params?test=true", 'this is body')
+        assert_equal('200', resp.code)
+      end
+    end
+
+    test 'must be call start and stop' do
+      on_driver do |driver|
+        require 'flexmock/test_unit'
+        server = flexmock('Server') { |watcher|
+          watcher.should_receive(:start).once.and_return do |que|
+            que.push(:start)
+          end
+          watcher.should_receive(:stop).once
+        }
+
+        stub(Fluent::PluginHelper::HttpServer::Server).new(anything) { server }
+
+        driver.create_http_server(addr: '127.0.0.1', port: PORT, logger: NULL_LOGGER) do
+          # nothing
+        end
+
+        driver.stop
+      end
+    end
+  end
+end