Merge branch 'feature/deal-with-network-failures' into 'master'

Feature/deal with network failures

Also activates the new nicknym lookup.

See merge request !5

10 files changed, 288 insertions, 61 deletions

diff --git a/.gitignore b/.gitignore
index 6dfed3c..22d0d82 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,3 +1 @@
-pkg
-Gemfile.lock
-NOTES
\ No newline at end of file
+vendor
diff --git a/lib/nickserver/dispatcher.rb b/lib/nickserver/dispatcher.rb
index 7a584e5..869f721 100644
--- a/lib/nickserver/dispatcher.rb
+++ b/lib/nickserver/dispatcher.rb
@@ -14,14 +14,17 @@
 #
 
 require 'nickserver/request'
+require 'nickserver/handler_chain'
 require 'nickserver/request_handlers/invalid_email_handler'
 require 'nickserver/request_handlers/local_email_handler'
+require 'nickserver/request_handlers/leap_email_handler'
 require 'nickserver/request_handlers/hkp_email_handler'
 require 'nickserver/request_handlers/fingerprint_handler'
 
 module Nickserver
   class Dispatcher
 
+
     def initialize(responder)
       @responder = responder
     end
@@ -35,10 +38,7 @@ module Nickserver
     protected
 
     def handle(request)
-      handler_chain.each do |handler|
-        response = handler.call request
-        return response if response
-      end
+      handler_chain.handle request
     rescue RuntimeError => exc
       puts "Error: #{exc}"
       puts exc.backtrace
@@ -46,13 +46,26 @@ module Nickserver
     end
 
     def handler_chain
-      [
-        RequestHandlers::InvalidEmailHandler,
+      @handler_chain ||= init_handler_chain
+    end
+
+    def init_handler_chain
+      chain = HandlerChain.new RequestHandlers::InvalidEmailHandler,
         RequestHandlers::LocalEmailHandler,
+        RequestHandlers::LeapEmailHandler,
         RequestHandlers::HkpEmailHandler,
         RequestHandlers::FingerprintHandler,
+        Proc.new {|_req| proxy_error_response },
         Proc.new { Nickserver::Response.new(404, "404 Not Found\n") }
-      ]
+      chain.continue_on HTTP::ConnectionError
+      return chain
+    end
+
+    def proxy_error_response
+      exception = handler_chain.rescued_exceptions.first
+      if exception
+        Nickserver::Response.new(502, exception.to_s)
+      end
     end
 
     def send_response(response)
diff --git a/lib/nickserver/handler_chain.rb b/lib/nickserver/handler_chain.rb
new file mode 100644
index 0000000..afc24a5
--- /dev/null
+++ b/lib/nickserver/handler_chain.rb
@@ -0,0 +1,48 @@
+#
+# Handler Chain
+#
+# A chain of handlers that respond to call. Invoking handle(*args) on the chain
+# will call the handlers with the given args until one of them returns a result
+# that is truethy (i.e. not false or nil).
+#
+# You can specify exception classes to rescue with
+#   handler_chain.continue_on ErrorClass1, ErrorClass2
+# These exceptions will be rescued and tracked. The chain will proceed even if
+# one handler raised the given exception. Afterwards you can inspect them with
+#   handler_chain.rescued_exceptions
+#
+
+module Nickserver
+  class HandlerChain
+
+    def initialize(*handlers)
+      @handlers = handlers
+      @exceptions_to_rescue = []
+      @rescued_exceptions = []
+    end
+
+    def continue_on(*exceptions)
+      self.exceptions_to_rescue += exceptions
+    end
+
+    def handle(*args)
+      result = nil
+      _handled_by = @handlers.find{|h| result = try_handler(h, *args)}
+      result
+    end
+
+    attr_reader :rescued_exceptions
+
+    protected
+
+    attr_writer :rescued_exceptions
+    attr_accessor :exceptions_to_rescue
+
+    def try_handler(handler, *args)
+      result = handler.call(*args)
+    rescue *exceptions_to_rescue
+      self.rescued_exceptions << $!
+      result = false
+    end
+  end
+end
diff --git a/lib/nickserver/hkp/source.rb b/lib/nickserver/hkp/source.rb
index e104aa8..82c94a0 100644
--- a/lib/nickserver/hkp/source.rb
+++ b/lib/nickserver/hkp/source.rb
@@ -19,7 +19,7 @@ module Nickserver; module Hkp
       if status == 200
         best = pick_best_key(response)
         get_key_by_fingerprint(best.keyid, nick)
-      else
+      elsif status != 404    # 404 means no key found and we proceed
         Nickserver::Response.new(status, response)
       end
     end
diff --git a/test/integration/dispatcher_test.rb b/test/integration/dispatcher_test.rb
index 60ad9d7..4f13e6b 100644
--- a/test/integration/dispatcher_test.rb
+++ b/test/integration/dispatcher_test.rb
@@ -1,48 +1,80 @@
 require 'test_helper'
 require 'nickserver/dispatcher'
 
+#
+# Test integration between the Dispatcher and the RequestHandlers
+#
+# Starting from a given request we test the interaction between the dispatcher
+# and the different RequestHandlers. There's a lot of combinations possible
+# and we only test a couple of them to ensure the parts work together well.
+#
+# This does not test the server. We stub and mock the sources. The nickserver
+# integration test covers these as well.
+#
+
 class Nickserver::DispatcherTest < Minitest::Test
 
   def test_empty_query
     handle
-    assert_response status: 404, content: "404 Not Found\n"
+    assert_response not_found
   end
 
   def test_invalid_query
     handle address: ['asdf']
-    assert_response status: 500, content: "500 Not a valid address\n"
+    assert_response error('Not a valid address')
+  end
+
+  def test_fingerprint_to_short
+    handle fingerprint: ['44F2F455E28']
+    assert_response error("Fingerprint invalid: 44F2F455E28")
+  end
+
+  def test_fingerprint_is_not_hex
+    handle fingerprint: ['X36E738D69173C13Z709E44F2F455E2824D18DDX']
+    assert_response error("Fingerprint invalid: X36E738D69173C13Z709E44F2F455E2824D18DDX")
   end
 
   def test_missing_domain
     handle address: ['valid@email.tld']
-    assert_response_from_hkp
+    stub_nicknym_not_available
+    hkp_source.expect :query, success, [Nickserver::EmailAddress]
+    assert_response success
   end
 
-  def test_email_from_hkp
+  def test_email_via_hkp
     handle address: ['valid@email.tld'], headers: { "Host" => "http://nickserver.me" }
-    assert_response_from_hkp
+    stub_nicknym_not_available
+    hkp_source.expect :query, success, [Nickserver::EmailAddress]
+    assert_response success
   end
 
-  def test_fingerprint_to_short
-    handle fingerprint: ['44F2F455E28']
-    assert_response status: 500, content: "500 Fingerprint invalid: 44F2F455E28\n"
+  def test_email_via_hkp_nicknym_unreachable
+    handle address: ['valid@email.tld'], headers: { "Host" => "http://nickserver.me" }
+    stub_nicknym_raises
+    hkp_source.expect :query, success, [Nickserver::EmailAddress]
+    assert_response success
   end
 
-  def test_fingerprint_is_not_hex
-    handle fingerprint: ['X36E738D69173C13Z709E44F2F455E2824D18DDX']
-    assert_response status: 500,
-      content: "500 Fingerprint invalid: X36E738D69173C13Z709E44F2F455E2824D18DDX\n"
+  def test_email_via_hkp_nicknym_unreachable
+    handle address: ['valid@email.tld'], headers: { "Host" => "http://nickserver.me" }
+    stub_nicknym_raises
+    hkp_source.expect :query, nil, [Nickserver::EmailAddress]
+    assert_response response(status: 502, content: "HTTP::ConnectionError")
+  end
+
+  def test_email_via_nicknym
+    handle address: ['valid@email.tld'], headers: { "Host" => "http://nickserver.me" }
+    nicknym_source.expect :available_for?, true, [String]
+    nicknym_source.expect :query, success, [Nickserver::EmailAddress]
+    assert_response success
   end
 
-  def test_get_key_with_fingerprint_from_hkp
+  def test_get_key_with_fingerprint
     handle fingerprint: ['E36E738D69173C13D709E44F2F455E2824D18DDF']
-    source = Minitest::Mock.new
-    source.expect :get_key_by_fingerprint,
-      Nickserver::Response.new(200, "fake key response"),
+    stub_nicknym_not_available
+    hkp_source.expect :get_key_by_fingerprint, success,
       ['E36E738D69173C13D709E44F2F455E2824D18DDF']
-    Nickserver::Hkp::Source.stub :new, source do
-      assert_response status: 200, content: "fake key response"
-    end
+    assert_response success
   end
 
   protected
@@ -52,20 +84,52 @@ class Nickserver::DispatcherTest < Minitest::Test
     @params = Hash[ params.map{ |k,v| [k.to_s, v] } ]
   end
 
-  def assert_response(args)
-    responder.expect :respond, nil, [args[:status], args[:content]]
-    dispatcher.respond_to @params, @headers
-    responder.verify
+  def assert_response(response)
+    Nickserver::Nicknym::Source.stub :new, nicknym_source do
+      Nickserver::Hkp::Source.stub :new, hkp_source do
+        responder.expect :respond, nil, [response.status, response.content]
+        dispatcher.respond_to @params, @headers
+        responder.verify
+      end
+    end
   end
 
-  def assert_response_from_hkp
-    source = Minitest::Mock.new
-    source.expect :query, Nickserver::Response.new(200, "fake content"), [Nickserver::EmailAddress]
-    Nickserver::Hkp::Source.stub :new, source do
-      assert_response status: 200, content: "fake content"
+  def hkp_source
+    @hkp_source ||= Minitest::Mock.new
+  end
+
+  def stub_nicknym_not_available
+    def nicknym_source.available_for?(*_args)
+      false
     end
   end
 
+  def stub_nicknym_raises
+    def nicknym_source.available_for?(*_args)
+      raise HTTP::ConnectionError
+    end
+  end
+
+  def nicknym_source
+    @nicknym_source ||= Minitest::Mock.new
+  end
+
+  def success
+    response status: 200, content: "fake content"
+  end
+
+  def not_found
+    response status: 404, content: "404 Not Found\n"
+  end
+
+  def error(msg)
+    response status: 500, content: "500 #{msg}\n"
+  end
+
+  def response(options)
+    Nickserver::Response.new(options[:status], options[:content])
+  end
+
   def dispatcher
     Nickserver::Dispatcher.new responder
   end
diff --git a/test/integration/nickserver_test.rb b/test/integration/nickserver_test.rb
index e367e06..06d6e29 100644
--- a/test/integration/nickserver_test.rb
+++ b/test/integration/nickserver_test.rb
@@ -3,6 +3,13 @@ require 'support/http_stub_helper'
 require 'nickserver/server'
 require 'json'
 
+# Integration Test for the whole nickserver without network dependecy.
+#
+# These tests are meant to test the integration between the different
+# components of the nickserver from the ReelServer all the way down to
+# the different sources.
+# These tests do not test the low level network adapter, the daemonization
+# or the startup script.
 #
 # Some important notes to understanding these tests:
 #
@@ -29,6 +36,7 @@ class NickserverTest < Minitest::Test
   def test_GET_key_by_email_address_served_via_SKS
     uid    = 'cloudadmin@leap.se'
     key_id = 'E818C478D3141282F7590D29D041EB11B1647490'
+    stub_nicknym_available_response 'leap.se', status: 404
     stub_sks_vindex_reponse(uid, body: file_content(:leap_vindex_result))
     stub_sks_get_reponse(key_id, body: file_content(:leap_public_key))
 
diff --git a/test/remote/nicknym_source_test.rb b/test/remote/nicknym_source_test.rb
index 2be7251..b38a991 100644
--- a/test/remote/nicknym_source_test.rb
+++ b/test/remote/nicknym_source_test.rb
@@ -2,6 +2,9 @@ require 'test_helper'
 require 'nickserver/nicknym/source'
 require 'nickserver/email_address'
 
+#
+# Please note the Readme.md file in this directory
+#
 class RemoteNicknymSourceTest < Minitest::Test
 
   def setup
@@ -15,22 +18,28 @@ class RemoteNicknymSourceTest < Minitest::Test
   end
 
   def test_availablility_check
-    assert source.available_for? 'mail.bitmask.net'
+    source.available_for? 'mail.bitmask.net'
     refute source.available_for? 'dl.bitmask.net'   # not a provider
-    refute source.available_for? 'demo.bitmask.net' # provider without mx
+  rescue HTTP::ConnectionError => e
+    skip e.to_s
   end
 
   def test_successful_query
     response = source.query(email_with_key)
+    skip if response.status == 404
     json = JSON.parse response.content
-    assert_equal 200, response.status
     assert_equal email_with_key.to_s, json["address"]
     refute_empty json["openpgp"]
+  rescue HTTP::ConnectionError => e
+    skip e.to_s
   end
 
   def test_not_found
     response = source.query(email_without_key)
-    assert_equal 404, response.status
+    skip if response.status == 200
+    assert response.status == 404
+  rescue HTTP::ConnectionError => e
+    skip e.to_s
   end
 
   protected
diff --git a/test/support/http_stub_helper.rb b/test/support/http_stub_helper.rb
index 6b05f98..cb9b578 100644
--- a/test/support/http_stub_helper.rb
+++ b/test/support/http_stub_helper.rb
@@ -7,6 +7,11 @@ module HttpStubHelper
     adapter.verify
   end
 
+  def stub_nicknym_available_response(domain, response = {})
+    stub_http_request :get, "https://#{domain}/provider.json",
+      response: response
+  end
+
   def stub_sks_vindex_reponse(uid, response = {})
     stub_http_request :get, config.hkp_url,
       query: {op: 'vindex', search: uid, exact: 'on', options: 'mr', fingerprint: 'on'},
@@ -28,8 +33,9 @@ module HttpStubHelper
 
   def stub_http_request(verb, url, options = {})
     response = {status: 200, body: ""}.merge(options.delete(:response) || {})
+    options = nil if options == {}
     adapter.expect :get, [response[:status], response[:body]],
-      [url, options]
+      [url, options].compact
   end
 
   def adapter
diff --git a/test/unit/handler_chain_test.rb b/test/unit/handler_chain_test.rb
new file mode 100644
index 0000000..fae0418
--- /dev/null
+++ b/test/unit/handler_chain_test.rb
@@ -0,0 +1,68 @@
+require 'test_helper'
+require 'nickserver/handler_chain'
+
+class HandlerChainTest < Minitest::Test
+
+  def test_initialization
+    assert chain
+  end
+
+  def test_noop
+    assert_nil chain.handle
+  end
+
+  def test_triggering_handlers
+    handler_mock.expect :call, nil, [:a, :b]
+    chain handler_mock
+    chain.handle :a, :b
+    handler_mock.verify
+  end
+
+  def test_returns_handler_result
+    chain  handler_with_nil, handler_with_result
+    assert_equal :result, chain.handle
+  end
+
+  def test_raise_exception
+    chain handler_raising, handler_with_result
+    assert_raises RuntimeError do
+      chain.handle
+    end
+  end
+
+  def test_continue_on_exception
+    chain handler_raising, handler_with_result
+    chain.continue_on(RuntimeError)
+    assert_equal :result, chain.handle
+    assert_equal [RuntimeError], chain.rescued_exceptions.map(&:class)
+  end
+
+  def test_continue_on_exception_with_nil
+    chain handler_raising, handler_with_nil
+    chain.continue_on(RuntimeError)
+    assert_nil chain.handle
+    assert_equal [RuntimeError], chain.rescued_exceptions.map(&:class)
+  end
+
+  protected
+
+  def chain(*handlers)
+    @chain ||= Nickserver::HandlerChain.new(*handlers)
+  end
+
+  def handler_mock
+    @handler ||= Minitest::Mock.new
+  end
+
+  def handler_with_nil
+    Proc.new {}
+  end
+
+  def handler_with_result
+    Proc.new { :result }
+  end
+
+  def handler_raising(exception = RuntimeError)
+    Proc.new { raise exception }
+  end
+end
diff --git a/test/unit/nicknym/source_test.rb b/test/unit/nicknym/source_test.rb
index 76337d4..f8c9b60 100644
--- a/test/unit/nicknym/source_test.rb
+++ b/test/unit/nicknym/source_test.rb
@@ -8,31 +8,44 @@ class NicknymSourceTest < Minitest::Test
     assert source
   end
 
-  def test_available_for_domain
-    adapter.expect :get, [200, '{"services": ["mx"]}'],
-      ['https://leap_powered.tld/provider.json']
-    assert source.available_for?('leap_powered.tld')
-    adapter.verify
+  def test_available_for_domain_with_service_mx
+    assert available_on?(200, '{"services": ["mx"]}')
   end
 
-  def test_not_available_for_domain
-    adapter.expect :get, [404, nil],
-      ['https://remote.tld/provider.json']
-    assert !source.available_for?('remote.tld')
-    adapter.verify
+  def test_no_provider_json_means_no_nicknym
+    refute available_on?(404, 'blablabla')
+  end
+
+  def test_invalid_provider_json_means_no_nicknym
+    refute available_on?(200, 'blablabla')
+  end
+
+  def test_proxy_successful_query
+    assert proxies_query_response?(200, 'dummy body')
   end
 
-  def test_successful_query
-    adapter.expect :get, [200, 'dummy body'],
-      ['https://nicknym.leap_powered.tld:6425',
-       {query: {address: email_stub.to_s}}]
+  def test_proxy_query_not_found
+    assert proxies_query_response?(404, 'dummy body')
+  end
+
+  protected
+
+  def proxies_query_response?(status = 0, body = nil)
+    adapter.expect :get, [status, body],
+      ['https://nicknym.leap_powered.tld:6425', query: {address: email_stub.to_s}]
     response = source.query(email_stub)
-    assert_equal 200, response.status
-    assert_equal 'dummy body', response.content
+    assert_equal status, response.status
+    assert_equal body, response.content
     adapter.verify
   end
 
-  protected
+  def available_on?(status = 0, body = nil)
+    adapter.expect :get, [status, body],
+      ['https://remote.tld/provider.json']
+    available = source.available_for?('remote.tld')
+    adapter.verify
+    return available
+  end
 
   def source
     Nickserver::Nicknym::Source.new(adapter)