Merge branch 'refactor/request-handling' into 'master'

refactor: restructure the way we handle requests to make it more consistent.

Requests are handled at a lot of different ways in different styles right now.
Let's make this more consistent and flexible to add email lookup at other leap providers.

See merge request !2

17 files changed, 319 insertions, 191 deletions

diff --git a/lib/nickserver/dispatcher.rb b/lib/nickserver/dispatcher.rb
new file mode 100644
index 0000000..8bcfd05
--- /dev/null
+++ b/lib/nickserver/dispatcher.rb
@@ -0,0 +1,65 @@
+#
+# Dispatcher
+#
+# Dispatch a request so it get's handled by the correct handler.
+#
+# The dispatcher hands a request to one handler after the other until one of
+# them responds.
+#
+# This is similar to the Chain of Responsibility patter but we iterate over the
+# 'handler_chain' array instead of a linked list.
+#
+# To change the order of handlers or add other handlers change the array in the
+# handler_chain function.
+#
+
+require 'nickserver/request'
+require 'nickserver/request_handlers/invalid_email_handler'
+require 'nickserver/request_handlers/local_email_handler'
+require 'nickserver/request_handlers/email_handler'
+require 'nickserver/request_handlers/fingerprint_handler'
+
+module Nickserver
+  class Dispatcher
+
+    def initialize(responder)
+      @responder = responder
+    end
+
+    def respond_to(params, headers)
+      request = Nickserver::Request.new params, headers
+      response = handle request
+      send_response response.status, response.content
+    end
+
+    protected
+
+    def handle(request)
+      handler_chain.each do |handler|
+        response = handler.call request
+        return response if response
+      end
+    rescue RuntimeError => exc
+      puts "Error: #{exc}"
+      puts exc.backtrace
+      ErrorResponse.new(exc.to_s)
+    end
+
+    def handler_chain
+      [
+        RequestHandlers::InvalidEmailHandler.new,
+        RequestHandlers::LocalEmailHandler.new,
+        RequestHandlers::EmailHandler.new,
+        RequestHandlers::FingerprintHandler.new,
+        Proc.new { Nickserver::Response.new(404, "Not Found\n") }
+      ]
+    end
+
+    def send_response(status = 200, content = '')
+      responder.respond status, content
+    end
+
+    attr_reader :responder
+
+  end
+end
diff --git a/lib/nickserver/error_response.rb b/lib/nickserver/error_response.rb
new file mode 100644
index 0000000..1065e4e
--- /dev/null
+++ b/lib/nickserver/error_response.rb
@@ -0,0 +1,11 @@
+require 'nickserver/response'
+
+module Nickserver
+  class ErrorResponse < Nickserver::Response
+    def initialize(message)
+      @status = 500
+      @message = message + "\n"
+    end
+
+  end
+end
diff --git a/lib/nickserver/invalid_source.rb b/lib/nickserver/invalid_source.rb
deleted file mode 100644
index dac245a..0000000
--- a/lib/nickserver/invalid_source.rb
+++ /dev/null
@@ -1,14 +0,0 @@
-#
-# This is a dummy source for invalid queries.
-# It simply always returns 500 and "Not a valid address"
-#
-
-module Nickserver
-  class InvalidSource
-
-    def query(nick)
-      yield 500, "Not a valid address"
-    end
-
-  end
-end
diff --git a/lib/nickserver/lookup.rb b/lib/nickserver/lookup.rb
deleted file mode 100644
index 105e77e..0000000
--- a/lib/nickserver/lookup.rb
+++ /dev/null
@@ -1,31 +0,0 @@
-require 'nickserver/invalid_source'
-
-module Nickserver
-  class Lookup
-
-    attr_reader :nick
-
-    def initialize(nick)
-      @nick = nick
-    end
-
-    def respond_with(responder)
-      query do |status, content|
-        responder.send_response status: status, content: content
-      end
-    end
-
-    protected
-
-    def query(&block)
-      source.query nick, &block
-    end
-
-    def source
-      if nick.invalid?  then Nickserver::InvalidSource
-      elsif nick.local? then Nickserver::Config.local_source
-      else Nickserver::Config.remote_source
-      end
-    end
-  end
-end
diff --git a/lib/nickserver/reel_server.rb b/lib/nickserver/reel_server.rb
index 6f05e0b..b681577 100644
--- a/lib/nickserver/reel_server.rb
+++ b/lib/nickserver/reel_server.rb
@@ -2,7 +2,7 @@ silence_warnings do
   require 'reel'
 end
 require 'nickserver/adapters/celluloid_http'
-require 'nickserver/request_handler'
+require 'nickserver/dispatcher'
 
 module Nickserver
   class ReelServer < Reel::Server::HTTP
@@ -32,7 +32,7 @@ module Nickserver
     protected
 
     def handler_for(request)
-      RequestHandler.new(request, Nickserver::Adapters::CelluloidHttp.new)
+      Dispatcher.new(request)
     end
 
     def params(request)
diff --git a/lib/nickserver/request.rb b/lib/nickserver/request.rb
new file mode 100644
index 0000000..c21c280
--- /dev/null
+++ b/lib/nickserver/request.rb
@@ -0,0 +1,30 @@
+module Nickserver
+  class Request
+    def initialize(params, headers)
+      @params = params || {}
+      @headers = headers
+    end
+
+    def email
+      param("address")
+    end
+
+    def fingerprint
+      param("fingerprint")
+    end
+
+    def domain
+      host_header = headers['Host'] || ''
+      domain_part = host_header.split(':')[0] || ''
+      domain_part.strip.sub(/^nicknym\./, '')
+    end
+
+    protected
+
+    def param(key)
+      params[key] && params[key].first
+    end
+
+    attr_reader :params, :headers
+  end
+end
diff --git a/lib/nickserver/request_handler.rb b/lib/nickserver/request_handler.rb
deleted file mode 100644
index 856ec5d..0000000
--- a/lib/nickserver/request_handler.rb
+++ /dev/null
@@ -1,95 +0,0 @@
-require 'nickserver/hkp/source'
-require 'nickserver/couch_db/source'
-
-module Nickserver
-  class RequestHandler
-
-    def initialize(responder, adapter)
-      @responder = responder
-      @adapter = adapter
-    end
-
-    def respond_to(params, headers)
-      if params && params["address"] && params["address"].any?
-        by_email(params, headers)
-      elsif params && params["fingerprint"] && params["fingerprint"].any?
-        by_fingerprint(params)
-      else
-        send_not_found
-      end
-
-    rescue RuntimeError => exc
-      puts "Error: #{exc}"
-      puts exc.backtrace
-      send_error(exc.to_s)
-    end
-
-    protected
-
-    def by_email(params, headers)
-      email = EmailAddress.new(params["address"].first)
-      if email.invalid?
-        send_error("Not a valid address")
-      else
-        send_key(email, headers)
-      end
-    end
-
-    def by_fingerprint(params)
-      fingerprint = params["fingerprint"].first
-      if fingerprint.length == 40 && !fingerprint[/\H/]
-        source = Nickserver::Hkp::Source.new(adapter)
-        key_response = source.get_key_by_fingerprint(fingerprint)
-        send_response key_response.status, key_response.content
-      else
-        send_error('Fingerprint invalid: ' + fingerprint)
-      end
-    end
-
-    def send_key(email, headers)
-      if local_address?(email, headers)
-        source = Nickserver::CouchDB::Source.new(adapter)
-      else
-        source = Nickserver::Hkp::Source.new(adapter)
-      end
-      response = source.query(email)
-      send_response response.status, response.content
-    rescue MissingHostHeader
-      send_error("HTTP request must include a Host header.")
-    end
-
-    #
-    # Return true if the user address is for a user of this service provider.
-    # e.g. if the provider is example.org, then alice@example.org returns true.
-    #
-    # If 'domain' is not configured, we rely on the Host header of the HTTP request.
-    #
-    def local_address?(email, headers)
-      email.domain?(Config.domain || domain_from_headers(headers))
-    end
-
-    # no domain configured, use Host header
-    def domain_from_headers(headers)
-      host_header = headers['Host']
-      raise MissingHostHeader if host_header.nil?
-      host_header.split(':')[0].strip.sub(/^nicknym\./, '')
-    end
-
-    def send_error(msg = "not supported")
-      send_response 500, "500 #{msg}\n"
-    end
-
-    def send_not_found(msg = "Not Found")
-      send_response 404, "404 #{msg}\n"
-    end
-
-    def send_response(status = 200, content = '')
-      responder.respond status, content
-    end
-
-    attr_reader :responder, :adapter
-
-    class MissingHostHeader < StandardError
-    end
-  end
-end
diff --git a/lib/nickserver/request_handlers/email_handler.rb b/lib/nickserver/request_handlers/email_handler.rb
new file mode 100644
index 0000000..96fcaf3
--- /dev/null
+++ b/lib/nickserver/request_handlers/email_handler.rb
@@ -0,0 +1,22 @@
+require 'nickserver/email_address'
+require 'nickserver/hkp/source'
+
+module Nickserver
+  module RequestHandlers
+    class EmailHandler
+
+      def call(request)
+        return unless request.email
+        email = EmailAddress.new(request.email)
+        source.query(email)
+      end
+
+      protected
+
+      def source
+        Nickserver::Hkp::Source.new
+      end
+
+    end
+  end
+end
diff --git a/lib/nickserver/request_handlers/fingerprint_handler.rb b/lib/nickserver/request_handlers/fingerprint_handler.rb
new file mode 100644
index 0000000..3c04fcd
--- /dev/null
+++ b/lib/nickserver/request_handlers/fingerprint_handler.rb
@@ -0,0 +1,27 @@
+require 'nickserver/hkp/source'
+require 'nickserver/error_response'
+
+module Nickserver
+  module RequestHandlers
+    class FingerprintHandler
+
+      def call(request)
+        return unless request.fingerprint
+        handle_request(request)
+      end
+
+      protected
+
+      def handle_request(request)
+        fingerprint = request.fingerprint
+        if fingerprint.length == 40 && !fingerprint[/\H/]
+          source = Nickserver::Hkp::Source.new
+          source.get_key_by_fingerprint(fingerprint)
+        else
+          ErrorResponse.new('Fingerprint invalid: ' + fingerprint)
+        end
+      end
+
+    end
+  end
+end
diff --git a/lib/nickserver/request_handlers/invalid_email_handler.rb b/lib/nickserver/request_handlers/invalid_email_handler.rb
new file mode 100644
index 0000000..eaf8156
--- /dev/null
+++ b/lib/nickserver/request_handlers/invalid_email_handler.rb
@@ -0,0 +1,15 @@
+require 'nickserver/email_address'
+require 'nickserver/error_response'
+
+module Nickserver
+  module RequestHandlers
+    class InvalidEmailHandler
+      def call(request)
+        return unless request.email
+        email = EmailAddress.new(request.email)
+        ErrorResponse.new("Not a valid address") if email.invalid?
+      end
+
+    end
+  end
+end
diff --git a/lib/nickserver/request_handlers/local_email_handler.rb b/lib/nickserver/request_handlers/local_email_handler.rb
new file mode 100644
index 0000000..1f2abc2
--- /dev/null
+++ b/lib/nickserver/request_handlers/local_email_handler.rb
@@ -0,0 +1,32 @@
+require 'nickserver/email_address'
+require 'nickserver/error_response'
+require 'nickserver/couch_db/source'
+
+module Nickserver
+  module RequestHandlers
+    class LocalEmailHandler
+
+      def call(request)
+        return nil unless request.email
+        domain = Config.domain || request.domain
+        return missing_domain_response if domain.nil? || domain == ''
+        email = EmailAddress.new(request.email)
+        return nil unless email.domain?(domain)
+        source.query email
+      end
+
+      protected
+
+      attr_reader :domain
+
+      def source
+        Nickserver::CouchDB::Source.new
+      end
+
+      def missing_domain_response
+        ErrorResponse.new "HTTP request must include a Host header."
+      end
+
+    end
+  end
+end
diff --git a/lib/nickserver/source.rb b/lib/nickserver/source.rb
index b8135da..78f245b 100644
--- a/lib/nickserver/source.rb
+++ b/lib/nickserver/source.rb
@@ -1,13 +1,12 @@
 module Nickserver
   class Source
 
-    def initialize(adapter)
+    def initialize(adapter = Nickserver::Adapters::CelluloidHttp.new)
       @adapter = adapter
     end
 
     protected
 
     attr_reader :adapter
-
   end
 end
diff --git a/test/unit/request_handler_test.rb b/test/integration/dispatcher_test.rb
index 6c7a036..60d252b 100644
--- a/test/unit/request_handler_test.rb
+++ b/test/integration/dispatcher_test.rb
@@ -1,7 +1,7 @@
 require 'test_helper'
-require 'nickserver/request_handler'
+require 'nickserver/dispatcher'
 
-class Nickserver::RequestHandlerTest < Minitest::Test
+class Nickserver::DispatcherTest < Minitest::Test
 
   def test_empty_query
     handle
@@ -58,19 +58,16 @@ class Nickserver::RequestHandlerTest < Minitest::Test
 
   def assert_response(args)
     responder.expect :respond, nil, [args[:status], args[:content]]
-    handler.respond_to @params, @headers
+    dispatcher.respond_to @params, @headers
     responder.verify
   end
 
-  def handler
-    Nickserver::RequestHandler.new responder, adapter
+  def dispatcher
+    Nickserver::Dispatcher.new responder
   end
 
   def responder
     @responder ||= Minitest::Mock.new
   end
 
-  def adapter
-    @adapter ||= Minitest::Mock.new
-  end
 end
diff --git a/test/unit/invalid_source_test.rb b/test/unit/invalid_source_test.rb
deleted file mode 100644
index 37a38fc..0000000
--- a/test/unit/invalid_source_test.rb
+++ /dev/null
@@ -1,16 +0,0 @@
-require 'test_helper'
-require 'nickserver/invalid_source'
-
-class Nickserver::InvalidSourceTest < Minitest::Test
-
-  def test_query
-    adapter.query(nil) do |status, content|
-      assert_equal 500, status
-      assert_equal "Not a valid address", content
-    end
-  end
-
-  def adapter
-    Nickserver::InvalidSource.new
-  end
-end
diff --git a/test/unit/lookup_test.rb b/test/unit/lookup_test.rb
deleted file mode 100644
index ac827e0..0000000
--- a/test/unit/lookup_test.rb
+++ /dev/null
@@ -1,23 +0,0 @@
-require 'test_helper'
-require 'minitest/mock'
-require 'nickserver/lookup'
-
-class TestLookup < Nickserver::Lookup
-
-  def query
-    yield 200, 'yeah'
-  end
-
-end
-
-class LookupTest < Minitest::Test
-
-  def test_responding
-    responder = Minitest::Mock.new
-    responder.expect :send_response, nil,
-      [{status: 200, content: 'yeah'}]
-    lookup = TestLookup.new nil
-    lookup.respond_with responder
-    responder.verify
-  end
-end
diff --git a/test/unit/request_handlers/local_email_handler_test.rb b/test/unit/request_handlers/local_email_handler_test.rb
new file mode 100644
index 0000000..8f303ec
--- /dev/null
+++ b/test/unit/request_handlers/local_email_handler_test.rb
@@ -0,0 +1,65 @@
+require 'test_helper'
+require 'nickserver/request_handlers/local_email_handler'
+
+class LocalEmailHandlerTest < MiniTest::Test
+
+  def test_no_email
+    assert_refuses
+  end
+
+  def test_remote_email
+    assert_refuses email: 'me@remote.tld', domain: 'local.tld'
+  end
+
+  def test_local_email
+    assert_handles email: 'me@local.tld', domain: 'local.tld'
+  end
+
+  def test_missing_host_header
+    Nickserver::Config.stub :domain, nil do
+      assert_responds_with_error "HTTP request must include a Host header.",
+        email: 'me@local.tld'
+    end
+  end
+
+  protected
+
+  def handler
+    Nickserver::RequestHandlers::LocalEmailHandler.new
+  end
+
+  def source
+    source = Minitest::Mock.new
+    source.expect :query,
+      'response',
+      [Nickserver::EmailAddress]
+    source
+  end
+
+  def assert_handles(opts)
+    Nickserver::CouchDB::Source.stub :new, source do
+      assert_equal 'response', handle(request(opts))
+    end
+  end
+
+  def assert_responds_with_error(msg, opts)
+    response = handle(request(opts))
+    assert_equal 500, response.status
+    assert_equal "500 #{msg}\n", response.content
+  end
+
+  def assert_refuses(opts = {})
+    assert_nil handle(request(opts))
+  end
+
+  def handle(request)
+    handler.call(request)
+  end
+
+  def request(opts = {})
+    params = {'address' => [opts[:email]]}
+    headers = {'Host' => opts[:domain]}
+    Nickserver::Request.new params, headers
+  end
+
+end
diff --git a/test/unit/request_test.rb b/test/unit/request_test.rb
new file mode 100644
index 0000000..698a275
--- /dev/null
+++ b/test/unit/request_test.rb
@@ -0,0 +1,44 @@
+require 'test_helper'
+require 'nickserver/request'
+
+class Nickserver::RequestTest < Minitest::Test
+
+  def test_email
+    request = request_with_params address: fake_email
+    assert_equal fake_email, request.email
+  end
+
+  def test_blank_email
+    request = request_with_params
+    assert_equal nil, request.email
+  end
+
+  def test_fingerprint
+    request = request_with_params fingerprint: fake_fingerprint
+    assert_equal fake_fingerprint, request.fingerprint
+  end
+
+  def test_domain
+    request = Nickserver::Request.new Hash.new,
+      'Host' => ' nicknym.my.domain.tld:123'
+    assert_equal 'my.domain.tld', request.domain
+  end
+
+  protected
+
+  # params are encoded with strings as keys and arrays with the
+  # given value(s)
+  def request_with_params(params = {})
+    params = params.collect{|k,v| [k.to_s, Array(v)]}.to_h
+    Nickserver::Request.new params, {}
+  end
+
+  def fake_email
+    'test@domain.tld'
+  end
+
+  def fake_fingerprint
+    'F' * 40
+  end
+
+end